From d4addfb13fbf066a621367fff755d12249bcbe7e Mon Sep 17 00:00:00 2001 From: Jerry <71265541+xukp20@users.noreply.github.com> Date: Thu, 16 Apr 2026 14:44:52 +0800 Subject: [PATCH] fix(codex): preserve workspace app-server options (#612) * fix(codex): preserve workspace app-server options * fix(core): tighten workspace agent option snapshots --- agent/codex/codex.go | 23 ++++++++++ core/engine.go | 37 ++++++++++----- core/engine_test.go | 105 +++++++++++++++++++++++++++++++++++++++++++ core/interfaces.go | 9 ++++ 4 files changed, 162 insertions(+), 12 deletions(-) diff --git a/agent/codex/codex.go b/agent/codex/codex.go index c191038..6a4a158 100644 --- a/agent/codex/codex.go +++ b/agent/codex/codex.go @@ -373,6 +373,29 @@ func (a *Agent) GetMode() string { return a.mode } +func (a *Agent) WorkspaceAgentOptions() map[string]any { + a.mu.RLock() + defer a.mu.RUnlock() + + opts := map[string]any{ + "mode": a.mode, + "backend": a.backend, + } + if a.model != "" { + opts["model"] = a.model + } + if a.reasoningEffort != "" { + opts["reasoning_effort"] = a.reasoningEffort + } + if a.appServerURL != "" { + opts["app_server_url"] = a.appServerURL + } + if a.codexHome != "" { + opts["codex_home"] = a.codexHome + } + return opts +} + // ── SkillProvider implementation ────────────────────────────── func (a *Agent) SkillDirs() []string { diff --git a/core/engine.go b/core/engine.go index 2f10f55..091a575 100644 --- a/core/engine.go +++ b/core/engine.go @@ -2125,18 +2125,27 @@ func (e *Engine) getOrCreateWorkspaceAgent(workspace string) (Agent, *SessionMan // Create a new agent instance with this workspace's work_dir opts := make(map[string]any) + if snapshotter, ok := e.agent.(WorkspaceAgentOptionSnapshotter); ok { + for k, v := range snapshotter.WorkspaceAgentOptions() { + opts[k] = v + } + } opts["work_dir"] = workspace // Copy model from original agent if possible - if ma, ok := e.agent.(interface{ GetModel() string }); ok { - if m := ma.GetModel(); m != "" { - opts["model"] = m + if _, ok := opts["model"]; !ok { + if ma, ok := e.agent.(interface{ GetModel() string }); ok { + if m := ma.GetModel(); m != "" { + opts["model"] = m + } } } // Copy permission mode - if ma, ok := e.agent.(interface{ GetMode() string }); ok { - if m := ma.GetMode(); m != "" { - opts["mode"] = m + if _, ok := opts["mode"]; !ok { + if ma, ok := e.agent.(interface{ GetMode() string }); ok { + if m := ma.GetMode(); m != "" { + opts["mode"] = m + } } } // Copy run_as_user (and run_as_env) for OS-level isolation. Without @@ -2145,14 +2154,18 @@ func (e *Engine) getOrCreateWorkspaceAgent(workspace string) (Agent, *SessionMan // above, not inherited from the project-level opts that main.go // already decorated. See cc-connect#496 and the cc-connect/core/runas.go // preamble for why run_as_user has to survive this copy. - if ma, ok := e.agent.(interface{ GetRunAsUser() string }); ok { - if u := ma.GetRunAsUser(); u != "" { - opts["run_as_user"] = u + if _, ok := opts["run_as_user"]; !ok { + if ma, ok := e.agent.(interface{ GetRunAsUser() string }); ok { + if u := ma.GetRunAsUser(); u != "" { + opts["run_as_user"] = u + } } } - if ma, ok := e.agent.(interface{ GetRunAsEnv() []string }); ok { - if env := ma.GetRunAsEnv(); len(env) > 0 { - opts["run_as_env"] = env + if _, ok := opts["run_as_env"]; !ok { + if ma, ok := e.agent.(interface{ GetRunAsEnv() []string }); ok { + if env := ma.GetRunAsEnv(); len(env) > 0 { + opts["run_as_env"] = env + } } } diff --git a/core/engine_test.go b/core/engine_test.go index 72f72f1..040818d 100644 --- a/core/engine_test.go +++ b/core/engine_test.go @@ -493,6 +493,32 @@ func (a *namedStubModelModeAgent) Name() string { return a.name } +type namedStubWorkspaceOptionAgent struct { + namedStubModelModeAgent + opts map[string]any + runAsUser string + runAsEnv []string +} + +func (a *namedStubWorkspaceOptionAgent) WorkspaceAgentOptions() map[string]any { + out := make(map[string]any, len(a.opts)) + for k, v := range a.opts { + out[k] = v + } + return out +} + +func (a *namedStubWorkspaceOptionAgent) GetRunAsUser() string { return a.runAsUser } + +func (a *namedStubWorkspaceOptionAgent) GetRunAsEnv() []string { + if len(a.runAsEnv) == 0 { + return nil + } + out := make([]string, len(a.runAsEnv)) + copy(out, a.runAsEnv) + return out +} + type stubWorkDirAgent struct { stubAgent workDir string @@ -3459,6 +3485,85 @@ func TestGetOrCreateWorkspaceAgent_InheritsActiveProvider(t *testing.T) { } } +func TestGetOrCreateWorkspaceAgent_InheritsSnapshotOptions(t *testing.T) { + agentName := "test-workspace-option-snapshot" + RegisterAgent(agentName, func(opts map[string]any) (Agent, error) { + snapshot := make(map[string]any, len(opts)) + for k, v := range opts { + snapshot[k] = v + } + return &namedStubWorkspaceOptionAgent{ + namedStubModelModeAgent: namedStubModelModeAgent{ + name: agentName, + stubModelModeAgent: stubModelModeAgent{ + model: "gpt-5.4", + mode: "yolo", + reasoningEffort: "high", + }, + }, + opts: snapshot, + }, nil + }) + + globalAgent := &namedStubWorkspaceOptionAgent{ + namedStubModelModeAgent: namedStubModelModeAgent{ + name: agentName, + stubModelModeAgent: stubModelModeAgent{ + model: "gpt-5.4", + mode: "yolo", + reasoningEffort: "high", + }, + }, + opts: map[string]any{ + "backend": "app_server", + "app_server_url": "ws://127.0.0.1:3846", + "codex_home": "/tmp/codex-home", + "reasoning_effort": "high", + "mode": "yolo", + "model": "gpt-5.4", + "run_as_user": "workspace-snapshot-user", + "run_as_env": []string{"SNAPSHOT_ONLY"}, + }, + runAsUser: "fallback-user", + runAsEnv: []string{"FALLBACK_ONLY"}, + } + e := NewEngine("test", globalAgent, []Platform{&stubPlatformEngine{n: "plain"}}, "", LangEnglish) + e.SetMultiWorkspace(t.TempDir(), filepath.Join(t.TempDir(), "bindings.json")) + + workspace := normalizeWorkspacePath(t.TempDir()) + wsAgentRaw, _, err := e.getOrCreateWorkspaceAgent(workspace) + if err != nil { + t.Fatalf("getOrCreateWorkspaceAgent returned error: %v", err) + } + + wsAgent, ok := wsAgentRaw.(*namedStubWorkspaceOptionAgent) + if !ok { + t.Fatalf("workspace agent type = %T, want *namedStubWorkspaceOptionAgent", wsAgentRaw) + } + if got := wsAgent.opts["backend"]; got != "app_server" { + t.Fatalf("workspace backend = %#v, want app_server", got) + } + if got := wsAgent.opts["app_server_url"]; got != "ws://127.0.0.1:3846" { + t.Fatalf("workspace app_server_url = %#v, want ws://127.0.0.1:3846", got) + } + if got := wsAgent.opts["codex_home"]; got != "/tmp/codex-home" { + t.Fatalf("workspace codex_home = %#v, want /tmp/codex-home", got) + } + if got := wsAgent.opts["reasoning_effort"]; got != "high" { + t.Fatalf("workspace reasoning_effort = %#v, want high", got) + } + if got := wsAgent.opts["work_dir"]; got != workspace { + t.Fatalf("workspace work_dir = %#v, want %q", got, workspace) + } + if got := wsAgent.opts["run_as_user"]; got != "workspace-snapshot-user" { + t.Fatalf("workspace run_as_user = %#v, want snapshot value", got) + } + gotRunAsEnv, _ := wsAgent.opts["run_as_env"].([]string) + if len(gotRunAsEnv) != 1 || gotRunAsEnv[0] != "SNAPSHOT_ONLY" { + t.Fatalf("workspace run_as_env = %#v, want snapshot value", wsAgent.opts["run_as_env"]) + } +} + func TestWorkspaceContext_PerChannelIndependence(t *testing.T) { agentName := "test-workspace-context-dir-override" RegisterAgent(agentName, func(opts map[string]any) (Agent, error) { diff --git a/core/interfaces.go b/core/interfaces.go index e088aa9..f0aea09 100644 --- a/core/interfaces.go +++ b/core/interfaces.go @@ -433,6 +433,15 @@ type ModeSwitcher interface { PermissionModes() []PermissionModeInfo } +// WorkspaceAgentOptionSnapshotter is an optional interface for agents that can +// export reusable constructor options needed to recreate an equivalent agent in +// a different workspace. Snapshot values should omit work_dir; the caller is +// responsible for setting the target workspace explicitly. Provider wiring and +// run_as propagation may still be handled separately by the engine. +type WorkspaceAgentOptionSnapshotter interface { + WorkspaceAgentOptions() map[string]any +} + // LiveModeSwitcher is an optional interface for running agent sessions that can // apply a mode change immediately without restarting the process. type LiveModeSwitcher interface {