-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(desktop+config): AO_KEEP_DAEMON keep-alive + per-role worker env profile (claude-code) #2621
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ import ( | |
| "github.com/aoagents/agent-orchestrator/backend/internal/adapters" | ||
| "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/agentbase" | ||
| "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/binaryutil" | ||
| "github.com/aoagents/agent-orchestrator/backend/internal/domain" | ||
| "github.com/aoagents/agent-orchestrator/backend/internal/ports" | ||
| ) | ||
|
|
||
|
|
@@ -184,6 +185,9 @@ func (p *Plugin) GetLaunchCommand(ctx context.Context, cfg ports.LaunchConfig) ( | |
| cmd = append(cmd, "--append-system-prompt", systemPrompt) | ||
| } | ||
|
|
||
| appendMCPFlags(&cmd, cfg.Config.MCP) | ||
| appendPluginFlags(&cmd, cfg.Config.PluginDirs) | ||
|
|
||
| if cfg.Prompt != "" { | ||
| cmd = append(cmd, "--", cfg.Prompt) | ||
| } | ||
|
|
@@ -228,6 +232,12 @@ func (p *Plugin) GetRestoreCommand(ctx context.Context, cfg ports.RestoreConfig) | |
| if err := ctx.Err(); err != nil { | ||
| return nil, false, err | ||
| } | ||
| // Defense-in-depth, symmetric with GetLaunchCommand: the config was | ||
| // validated on write, but re-check here so a config mutated later (by a | ||
| // bug or a different code path) is caught at restore too, not only launch. | ||
| if err := cfg.Config.Validate(); err != nil { | ||
| return nil, false, fmt.Errorf("claude-code: %w", err) | ||
| } | ||
|
|
||
| sessionID := strings.TrimSpace(cfg.Session.Metadata[ports.MetadataKeyAgentSessionID]) | ||
| if sessionID == "" && cfg.Session.ID != "" { | ||
|
|
@@ -256,6 +266,11 @@ func (p *Plugin) GetRestoreCommand(ctx context.Context, cfg ports.RestoreConfig) | |
| // re-appended or a restored orchestrator loses its role. | ||
| cmd = append(cmd, "--append-system-prompt", systemPrompt) | ||
| } | ||
| // MCP/plugin flags are also rebuilt from flags on resume (they are not part | ||
| // of the transcript), so re-apply them or a restored worker loses its scoped | ||
| // MCP set and plugins. | ||
| appendMCPFlags(&cmd, cfg.Config.MCP) | ||
| appendPluginFlags(&cmd, cfg.Config.PluginDirs) | ||
| cmd = append(cmd, "--resume", sessionID) | ||
| return cmd, true, nil | ||
| } | ||
|
|
@@ -454,6 +469,49 @@ func appendToolFlags(cmd *[]string, allowed, disallowed []string) { | |
| } | ||
| } | ||
|
|
||
| // appendMCPFlags emits claude-code's per-session MCP flags. Each MCPConfig | ||
| // entry is passed to the repeatable --mcp-config as-is (a JSON string or a path | ||
| // to a JSON file — both accepted by the CLI). Strict adds --strict-mcp-config | ||
| // so the session ignores every other MCP source, isolating the worker. A nil | ||
| // MCPConfig emits nothing, so an unset config inherits the global MCP set as | ||
| // before. Strict alone (empty Configs) is valid: it means "no MCP at all". | ||
| func appendMCPFlags(cmd *[]string, mcp *domain.MCPConfig) { | ||
| if mcp == nil { | ||
| return | ||
| } | ||
| for _, c := range mcp.Configs { | ||
| if c = strings.TrimSpace(c); c != "" { | ||
| *cmd = append(*cmd, "--mcp-config", c) | ||
| } | ||
| } | ||
| if mcp.Strict { | ||
| *cmd = append(*cmd, "--strict-mcp-config") | ||
| } | ||
| } | ||
|
|
||
| // appendPluginFlags emits --plugin-dir / --plugin-url for each entry. An | ||
| // http(s):// entry maps to --plugin-url (a fetched zip); any other value is | ||
| // treated as a local path and mapped to --plugin-dir. Both flags are repeatable, | ||
| // so one is emitted per entry. Empty/whitespace entries are skipped. | ||
| func appendPluginFlags(cmd *[]string, dirs []string) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] These flags add plugins; they do not scope the set a worker can load. User/project/global plugins and standalone skills remain active, so a configured worker still carries the orchestrator's globally enabled capabilities. That leaves the central least-privilege requirement from #2195 unimplemented. Anthropic documents Please add an isolation mechanism for other plugin/skill sources, or narrow the feature and PR claims so they do not promise plugin scoping. |
||
| for _, d := range dirs { | ||
| if d = strings.TrimSpace(d); d == "" { | ||
| continue | ||
| } | ||
| if isPluginURL(d) { | ||
| *cmd = append(*cmd, "--plugin-url", d) | ||
| } else { | ||
| *cmd = append(*cmd, "--plugin-dir", d) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // isPluginURL reports whether s is an http(s) plugin URL rather than a local | ||
| // path, deciding --plugin-url vs --plugin-dir. | ||
| func isPluginURL(s string) bool { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. URL schemes are case-insensitive, but this only recognizes lowercase prefixes. A valid |
||
| return strings.HasPrefix(s, "http://") || strings.HasPrefix(s, "https://") | ||
| } | ||
|
|
||
| // claudeBinarySpec locates the claude binary: PATH first, then the native | ||
| // installer's locations, npm global, Homebrew, and the claude-managed dir. | ||
| var claudeBinarySpec = binaryutil.BinarySpec{ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -95,6 +95,41 @@ func TestBuildProjectConfigTrackerIntakeFlags(t *testing.T) { | |
| } | ||
| } | ||
|
|
||
| // TestBuildProjectConfigWorkerProfileFlags: the worker MCP/plugin flags are | ||
| // assembled into the per-role AgentConfig that the daemon validates and | ||
| // claude-code maps to --mcp-config / --strict-mcp-config / --plugin-dir[-url]. | ||
| func TestBuildProjectConfigWorkerProfileFlags(t *testing.T) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This bypasses Cobra and only tests the options-to-struct helper. The repository requires command tests for CLI changes; please add an |
||
| got, err := buildProjectConfig(projectSetConfigOptions{ | ||
| workerMCPConfig: []string{`{"a":1}`, "/p/m.json"}, | ||
| workerStrictMCP: true, | ||
| workerPluginDir: []string{"/local/plugin", "https://example.com/p.zip"}, | ||
| }) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| mcp := got.Worker.AgentConfig.MCP | ||
| if mcp == nil || !mcp.Strict || len(mcp.Configs) != 2 || mcp.Configs[0] != `{"a":1}` || mcp.Configs[1] != "/p/m.json" { | ||
| t.Fatalf("worker MCP = %#v, want strict with 2 configs", mcp) | ||
| } | ||
| if len(got.Worker.AgentConfig.PluginDirs) != 2 || got.Worker.AgentConfig.PluginDirs[1] != "https://example.com/p.zip" { | ||
| t.Fatalf("worker pluginDirs = %#v", got.Worker.AgentConfig.PluginDirs) | ||
| } | ||
| } | ||
|
|
||
| // TestBuildProjectConfigWorkerStrictMCPAlone: --worker-strict-mcp without any | ||
| // --worker-mcp-config is a valid isolation request (empty MCP set), so it must | ||
| // still build a present MCPConfig with Strict=true rather than dropping it. | ||
| func TestBuildProjectConfigWorkerStrictMCPAlone(t *testing.T) { | ||
| got, err := buildProjectConfig(projectSetConfigOptions{workerStrictMCP: true}) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| mcp := got.Worker.AgentConfig.MCP | ||
| if mcp == nil || !mcp.Strict || len(mcp.Configs) != 0 { | ||
| t.Fatalf("worker MCP = %#v, want strict isolation with empty configs", mcp) | ||
| } | ||
| } | ||
|
|
||
| func TestProjectList_Success(t *testing.T) { | ||
| cfg := setConfigEnv(t) | ||
| srv, capture := projectServer(t, http.StatusOK, `{"projects":[{"id":"zeta","name":"Zeta","sessionPrefix":"zeta"},{"id":"alpha","name":"Alpha","sessionPrefix":"alpha","resolveError":"config missing"}]}`) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] This pins the opposite of issue #2195's priority-one MCP behavior. The issue says workers should default to no MCP and opt in explicitly, but a nil config returns without
--strict-mcp-config, so every globally configured MCP server remains available. The new ‘unset inherits global’ test makes that mismatch deliberate.The issue also contains broader compatibility wording, so please resolve the acceptance semantics explicitly. If least privilege is still the requirement, workers need a strict-empty default plus an explicit way to request inheritance.