From 075d1bdeec932210d602d58a621e7ffc73c1c2bb Mon Sep 17 00:00:00 2001 From: whoisasx Date: Sun, 21 Jun 2026 04:37:09 +0530 Subject: [PATCH 1/5] feat: require explicit project agents --- README.md | 10 +- backend/internal/cli/dto_drift_e2e_test.go | 11 ++ backend/internal/cli/project.go | 31 ++-- backend/internal/cli/project_test.go | 2 +- backend/internal/cli/spawn.go | 4 +- backend/internal/config/config.go | 12 +- backend/internal/daemon/daemon.go | 2 +- backend/internal/daemon/lifecycle_wiring.go | 46 +++--- backend/internal/daemon/wiring_test.go | 7 +- backend/internal/service/session/service.go | 2 + .../internal/service/session/service_test.go | 1 + backend/internal/session_manager/manager.go | 56 ++++--- .../internal/session_manager/manager_test.go | 55 +++++-- .../components/CreateProjectAgentSheet.tsx | 119 +++++++++++++++ .../components/ProjectSettingsForm.test.tsx | 35 +++++ .../components/ProjectSettingsForm.tsx | 68 ++++----- .../src/renderer/components/Sidebar.test.tsx | 40 ++++- frontend/src/renderer/components/Sidebar.tsx | 138 +++++++++++------- frontend/src/renderer/lib/agent-options.ts | 25 ++++ frontend/src/renderer/lib/shell-context.ts | 2 +- frontend/src/renderer/routes/_shell.tsx | 28 +++- 21 files changed, 495 insertions(+), 199 deletions(-) create mode 100644 frontend/src/renderer/components/CreateProjectAgentSheet.tsx create mode 100644 frontend/src/renderer/lib/agent-options.ts diff --git a/README.md b/README.md index 281cb376..91f76593 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,8 @@ progress (what's shipped vs. in flight) see [`docs/STATUS.md`](docs/STATUS.md). `opencode`, `aider`, `amp`, `goose`, `copilot`, `grok`, `qwen`, `kimi`, `crush`, `cline`, `droid`, `devin`, `auggie`, `continue`, `kiro`, `kilocode`, and more), registered through a shared registry with common - activity-dispatch / hook utilities. The default is set by `AO_AGENT`. + activity-dispatch / hook utilities. Worker and orchestrator defaults are set + per project. - **Isolated workspaces.** Worker and orchestrator sessions spawn into their own `git worktree` (`backend/internal/adapters/workspace/gitworktree/`), launched inside a `zellij` runtime adapter (`backend/internal/adapters/runtime/`) so @@ -92,9 +93,10 @@ go build -o /tmp/ao ./cmd/ao # Register a local git repo as a project. The id defaults to the lowercased # base of --path; pass --id explicitly when the directory name doesn't match. -/tmp/ao project add --path /path/to/your/repo --id your-repo --name your-repo +/tmp/ao project add --path /path/to/your/repo --id your-repo --name your-repo \ + --worker-agent codex --orchestrator-agent codex -# Spawn a worker session running the default agent. +# Spawn a worker session running the project's worker agent. /tmp/ao spawn --project your-repo --prompt "Refactor the auth module" # Inspect what's running. @@ -167,7 +169,7 @@ exposing it beyond loopback would be a security regression. | `AO_SHUTDOWN_TIMEOUT` | `10s` | Graceful-shutdown hard cap. | | `AO_RUN_FILE` | `/agent-orchestrator/running.json` | PID + port handshake path. | | `AO_DATA_DIR` | `/agent-orchestrator/data` | SQLite DB, WAL files, managed state. | -| `AO_AGENT` | `claude-code` | Default agent adapter id used by `ao spawn`. | +| `AO_AGENT` | `claude-code` | Compatibility agent adapter id validated at daemon startup. | | `AO_SESSION_ID` | _(unset)_ | Set inside spawned sessions; read by `ao send` and `ao hooks`. | | `GITHUB_TOKEN` | _(unset)_ | Used by the GitHub SCM and tracker adapters. Falls back to `gh auth token`. | diff --git a/backend/internal/cli/dto_drift_e2e_test.go b/backend/internal/cli/dto_drift_e2e_test.go index 2488d8f4..8b3a91ef 100644 --- a/backend/internal/cli/dto_drift_e2e_test.go +++ b/backend/internal/cli/dto_drift_e2e_test.go @@ -222,6 +222,8 @@ func TestE2E_SpawnAndProjectAddDTORoundTrip(t *testing.T) { "--path", "/repo/mer", "--id", "demo", "--name", "Demo", + "--worker-agent", "codex", + "--orchestrator-agent", "claude-code", "--as-workspace", }) if err := root.Execute(); err != nil { @@ -238,6 +240,15 @@ func TestE2E_SpawnAndProjectAddDTORoundTrip(t *testing.T) { if got.Name == nil || *got.Name != "Demo" { t.Errorf("Name = %v, want %q", got.Name, "Demo") } + if got.Config == nil { + t.Fatal("Config = nil, want role agent config") + } + if got.Config.Worker.Harness != domain.HarnessCodex { + t.Errorf("Config.Worker.Harness = %q, want codex", got.Config.Worker.Harness) + } + if got.Config.Orchestrator.Harness != domain.HarnessClaudeCode { + t.Errorf("Config.Orchestrator.Harness = %q, want claude-code", got.Config.Orchestrator.Harness) + } if !got.AsWorkspace { t.Errorf("AsWorkspace = false, want true (CLI json:\"asWorkspace\" vs AddInput)") } diff --git a/backend/internal/cli/project.go b/backend/internal/cli/project.go index 0de4b947..ff74dec8 100644 --- a/backend/internal/cli/project.go +++ b/backend/internal/cli/project.go @@ -15,10 +15,12 @@ import ( ) type projectAddOptions struct { - path string - id string - name string - asWorkspace bool + path string + id string + name string + workerAgent string + orchestratorAgent string + asWorkspace bool } type projectListOptions struct { @@ -37,10 +39,11 @@ type projectRemoveOptions struct { // addProjectRequest mirrors the daemon's project AddInput body for // POST /api/v1/projects. projectId and name are optional (pointers omit them). type addProjectRequest struct { - Path string `json:"path"` - ProjectID *string `json:"projectId,omitempty"` - Name *string `json:"name,omitempty"` - AsWorkspace bool `json:"asWorkspace,omitempty"` + Path string `json:"path"` + ProjectID *string `json:"projectId,omitempty"` + Name *string `json:"name,omitempty"` + Config *projectConfig `json:"config,omitempty"` + AsWorkspace bool `json:"asWorkspace,omitempty"` } type projectSummary struct { @@ -58,7 +61,7 @@ type projectDetails struct { Path string `json:"path"` Repo string `json:"repo"` DefaultBranch string `json:"defaultBranch"` - DefaultHarness string `json:"agent,omitempty"` + Agent string `json:"agent,omitempty"` Config *projectConfig `json:"config,omitempty"` WorkspaceRepos []workspaceRepoDetails `json:"workspaceRepos,omitempty"` ResolveError string `json:"resolveError,omitempty"` @@ -226,6 +229,12 @@ func newProjectAddCommand(ctx *commandContext) *cobra.Command { if opts.name != "" { req.Name = &opts.name } + if opts.workerAgent != "" || opts.orchestratorAgent != "" { + req.Config = &projectConfig{ + Worker: roleOverride{Agent: opts.workerAgent}, + Orchestrator: roleOverride{Agent: opts.orchestratorAgent}, + } + } var res projectResult if err := ctx.postJSON(cmd.Context(), "projects", req, &res); err != nil { return err @@ -238,6 +247,8 @@ func newProjectAddCommand(ctx *commandContext) *cobra.Command { f.StringVar(&opts.path, "path", "", "Absolute path to the local git repo (required)") f.StringVar(&opts.id, "id", "", "Project id (default: derived by the daemon from the path)") f.StringVar(&opts.name, "name", "", "Display name") + f.StringVar(&opts.workerAgent, "worker-agent", "", "Default worker session agent") + f.StringVar(&opts.orchestratorAgent, "orchestrator-agent", "", "Default orchestrator session agent") f.BoolVar(&opts.asWorkspace, "as-workspace", false, "Register a parent folder as a workspace project (root-as-repo plus direct child repos)") return cmd } @@ -443,7 +454,7 @@ func writeProjectDetails(cmd *cobra.Command, res projectGetResult) error { {label: "path", value: p.Path}, {label: "repo", value: p.Repo}, {label: "default branch", value: p.DefaultBranch}, - {label: "default harness", value: p.DefaultHarness}, + {label: "agent", value: p.Agent}, {label: "config", value: formatProjectConfig(p.Config)}, {label: "resolve error", value: p.ResolveError}, } diff --git a/backend/internal/cli/project_test.go b/backend/internal/cli/project_test.go index 98892dc9..eea7c924 100644 --- a/backend/internal/cli/project_test.go +++ b/backend/internal/cli/project_test.go @@ -107,7 +107,7 @@ func TestProjectGet_Success(t *testing.T) { if capture.method != http.MethodGet || capture.path != "/api/v1/projects/demo" { t.Fatalf("request = %s %s, want GET /api/v1/projects/demo", capture.method, capture.path) } - for _, want := range []string{"Project demo (ok)", "name: Demo", "path: /repo/demo", "default branch: main", "default harness: codex"} { + for _, want := range []string{"Project demo (ok)", "name: Demo", "path: /repo/demo", "default branch: main", "agent: codex"} { if !strings.Contains(out, want) { t.Fatalf("output missing %q:\n%s", want, out) } diff --git a/backend/internal/cli/spawn.go b/backend/internal/cli/spawn.go index efa4f3f1..b5cc7174 100644 --- a/backend/internal/cli/spawn.go +++ b/backend/internal/cli/spawn.go @@ -44,7 +44,7 @@ func newSpawnCommand(ctx *commandContext) *cobra.Command { Use: "spawn", Short: "Spawn a worker agent session in a registered project", Long: "Spawn a worker agent session in a registered project.\n\n" + - "The session runs the chosen agent (default: the daemon's AO_AGENT) in a\n" + + "The session runs the chosen agent in a\n" + "fresh git worktree. Register the project first with `ao project add`.", Args: noArgs, RunE: func(cmd *cobra.Command, args []string) error { @@ -120,7 +120,7 @@ func newSpawnCommand(ctx *commandContext) *cobra.Command { return pflag.NormalizedName(name) }) f.StringVar(&opts.project, "project", "", "Project id to spawn the session in (required)") - f.StringVar(&opts.harness, "harness", "", "Agent harness / --agent: claude-code, codex, aider, opencode, grok, droid, amp, agy, crush, cursor, qwen, copilot, goose, auggie, continue, devin, cline, kimi, kiro, kilocode, vibe, pi, autohand (default: the daemon's AO_AGENT)") + f.StringVar(&opts.harness, "harness", "", "Agent harness / --agent: claude-code, codex, aider, opencode, grok, droid, amp, agy, crush, cursor, qwen, copilot, goose, auggie, continue, devin, cline, kimi, kiro, kilocode, vibe, pi, autohand (default: project worker.agent; required if the project has none)") f.StringVar(&opts.branch, "branch", "", "Branch for the session worktree (default: ao//root)") f.StringVar(&opts.prompt, "prompt", "", "Initial prompt for the agent") f.StringVar(&opts.issue, "issue", "", "Issue id to associate with the session") diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index dd94193d..45cef3f5 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -29,8 +29,9 @@ const ( // DefaultShutdownTimeout is the hard cap on graceful shutdown. After this // the process exits even if connections are still draining. DefaultShutdownTimeout = 10 * time.Second - // DefaultAgent is the agent adapter id the daemon wires when AO_AGENT is - // unset. It matches the claude-code adapter's manifest id. + // DefaultAgent is the compatibility value used when AO_AGENT is unset. The + // daemon validates it at startup, but worker/orchestrator spawns resolve from + // explicit requests or project role config instead of falling back to it. DefaultAgent = "claude-code" // DefaultTelemetryPostHogHost is the default PostHog ingestion host when // remote telemetry is enabled and AO_TELEMETRY_POSTHOG_HOST is unset. @@ -85,9 +86,8 @@ type Config struct { // DataDir is the directory holding durable SQLite state: DB and WAL files. // It is created on first use by the storage layer. DataDir string - // Agent is the id of the agent adapter the daemon wires into the Session - // Manager (see DefaultAgent). Selected by AO_AGENT; startSession fails fast - // if no adapter with this id is registered. + // Agent is the compatibility agent adapter id selected by AO_AGENT; + // startSession fails fast if no adapter with this id is registered. Agent string // AllowedOrigins are the browser origins granted CORS read access (see // DefaultAllowedOrigins). Overridden by AO_ALLOWED_ORIGINS. @@ -113,7 +113,7 @@ func (c Config) Addr() string { // AO_SHUTDOWN_TIMEOUT shutdown deadline (Go duration > 0, default 10s) // AO_RUN_FILE running.json path (default ~/.ao/running.json) // AO_DATA_DIR durable state dir (default ~/.ao/data) -// AO_AGENT agent adapter id (default claude-code) +// AO_AGENT compatibility agent id (default claude-code) // AO_ALLOWED_ORIGINS CORS origins, comma-separated (default DefaultAllowedOrigins) // AO_TELEMETRY_EVENTS local event capture off|on (default off) // AO_TELEMETRY_METRICS local metric capture off|on (default off) diff --git a/backend/internal/daemon/daemon.go b/backend/internal/daemon/daemon.go index e9aac689..7ea50c3a 100644 --- a/backend/internal/daemon/daemon.go +++ b/backend/internal/daemon/daemon.go @@ -116,7 +116,7 @@ func Run() error { // Wire the controller-facing session service over the same store + LCM, the // zellij runtime, a gitworktree workspace, the per-session agent resolver - // (AO_AGENT default, validated here), and the agent messenger, then mount it + // (AO_AGENT validated here for compatibility), and the agent messenger, then mount it // on the API. sessionSvc, reviewSvc, err := startSession(cfg, runtimeAdapter, store, lcStack.LCM, messenger, telemetrySink, log) if err != nil { diff --git a/backend/internal/daemon/lifecycle_wiring.go b/backend/internal/daemon/lifecycle_wiring.go index 5d79ec31..7e11c34f 100644 --- a/backend/internal/daemon/lifecycle_wiring.go +++ b/backend/internal/daemon/lifecycle_wiring.go @@ -60,12 +60,9 @@ func (l *lifecycleStack) Stop() { // startSession builds the controller-facing session service: a session manager // over the real zellij runtime, a per-session gitworktree workspace, the shared -// store + LCM, the per-session agent resolver (AO_AGENT default), and the -// agent messenger. The returned service is mounted at httpd APIDeps.Sessions. +// store + LCM, the per-session agent resolver, and the agent messenger. The +// returned service is mounted at httpd APIDeps.Sessions. func startSession(cfg config.Config, runtime *zellij.Runtime, store *sqlite.Store, lcm *lifecycle.Manager, messenger ports.AgentMessenger, telemetry ports.EventSink, log *slog.Logger) (*sessionsvc.Service, reviewsvc.Manager, error) { - // Resolve the default agent once and share it with both the resolver (which - // launches it for an unspecified harness) and the session manager (which - // persists it onto the seed row), so the stored harness matches what runs. defaultAgent := cfg.Agent if defaultAgent == "" { defaultAgent = config.DefaultAgent @@ -87,15 +84,14 @@ func startSession(cfg config.Config, runtime *zellij.Runtime, store *sqlite.Stor return nil, nil, fmt.Errorf("session workspace: %w", err) } mgr := sessionmanager.New(sessionmanager.Deps{ - Runtime: runtime, - Agents: agents, - Workspace: ws, - Store: store, - Messenger: messenger, - Lifecycle: lcm, - DataDir: cfg.DataDir, - DefaultHarness: domain.AgentHarness(defaultAgent), - Logger: log, + Runtime: runtime, + Agents: agents, + Workspace: ws, + Store: store, + Messenger: messenger, + Lifecycle: lcm, + DataDir: cfg.DataDir, + Logger: log, }) scmProvider, err := newGitHubSCMProvider(log) if err != nil { @@ -179,20 +175,15 @@ func buildAgentRegistry() (*adapters.Registry, error) { // agentRegistry adapts the generic adapter Registry to ports.AgentResolver: it // maps a session's harness onto the registered adapter of the same id and -// asserts that adapter drives an agent. An empty harness falls back to the -// daemon's configured default (AO_AGENT), so a spawn that names no harness still -// gets a real agent. +// asserts that adapter drives an agent. Empty harnesses are invalid at the +// session manager boundary and deliberately do not resolve here. type agentRegistry struct { - reg *adapters.Registry - defaultHarness domain.AgentHarness + reg *adapters.Registry } var _ ports.AgentResolver = agentRegistry{} func (a agentRegistry) Agent(harness domain.AgentHarness) (ports.Agent, bool) { - if harness == "" { - harness = a.defaultHarness - } adapter, ok := a.reg.Get(string(harness)) if !ok { return nil, false @@ -203,10 +194,9 @@ func (a agentRegistry) Agent(harness domain.AgentHarness) (ports.Agent, bool) { // buildAgentResolver constructs the per-session agent resolver the Session // Manager consumes (sessionmanager.Deps.Agents): a registry of the shipped -// adapters plus the configured default harness. It fails fast if the default -// does not resolve, so a typo'd AO_AGENT surfaces at startup. The session lane -// plugs this in when it mounts the controller-facing session service at the -// httpd APIDeps.Sessions slot. +// adapters. It still validates AO_AGENT at startup for compatibility with the +// config surface, but worker/orchestrator spawns must provide a resolved +// harness before calling Agent. func buildAgentResolver(defaultAgent string, log *slog.Logger) (ports.AgentResolver, error) { if defaultAgent == "" { defaultAgent = config.DefaultAgent @@ -215,8 +205,8 @@ func buildAgentResolver(defaultAgent string, log *slog.Logger) (ports.AgentResol if err != nil { return nil, err } - resolver := agentRegistry{reg: reg, defaultHarness: domain.AgentHarness(defaultAgent)} - if _, ok := resolver.Agent(""); !ok { + resolver := agentRegistry{reg: reg} + if _, ok := resolver.Agent(domain.AgentHarness(defaultAgent)); !ok { return nil, fmt.Errorf("configured default agent %q is not a registered adapter", defaultAgent) } ids := make([]string, 0) diff --git a/backend/internal/daemon/wiring_test.go b/backend/internal/daemon/wiring_test.go index 95dc523b..a3acd553 100644 --- a/backend/internal/daemon/wiring_test.go +++ b/backend/internal/daemon/wiring_test.go @@ -79,8 +79,7 @@ func TestWiring_WriteFlowsToBroadcaster(t *testing.T) { // TestWiring_AgentResolverResolvesRealAdapters asserts buildAgentResolver wires a // real registry-backed per-session resolver: each harness resolves to the -// matching registered adapter, an empty harness falls back to the AO_AGENT -// default, and an unknown harness misses. +// matching registered adapter, while empty and unknown harnesses miss. func TestWiring_AgentResolverResolvesRealAdapters(t *testing.T) { log := slog.New(slog.NewTextHandler(io.Discard, nil)) resolver, err := buildAgentResolver("", log) // empty default → claude-code @@ -114,7 +113,6 @@ func TestWiring_AgentResolverResolvesRealAdapters(t *testing.T) { {domain.HarnessVibe, "vibe"}, {domain.HarnessPi, "pi"}, {domain.HarnessAutohand, "autohand"}, - {"", config.DefaultAgent}, // empty harness falls back to the AO_AGENT default } { agent, ok := resolver.Agent(tc.harness) if !ok { @@ -131,6 +129,9 @@ func TestWiring_AgentResolverResolvesRealAdapters(t *testing.T) { if _, ok := resolver.Agent("definitely-not-an-agent"); ok { t.Fatal("unknown harness resolved to an agent; want a miss") } + if _, ok := resolver.Agent(""); ok { + t.Fatal("empty harness resolved to an agent; want a miss") + } } // TestWiring_StartSessionBuildsSessionService asserts the daemon's startSession diff --git a/backend/internal/service/session/service.go b/backend/internal/service/session/service.go index bbcb32c1..b2b48940 100644 --- a/backend/internal/service/session/service.go +++ b/backend/internal/service/session/service.go @@ -452,6 +452,8 @@ func toAPIError(err error) error { return apierr.Invalid("PROJECT_NOT_RESOLVABLE", "Project is not registered or has no repo — register it with `ao project add`", nil) case errors.Is(err, sessionmanager.ErrUnknownHarness): return apierr.Invalid("UNKNOWN_HARNESS", err.Error(), nil) + case errors.Is(err, sessionmanager.ErrMissingHarness): + return apierr.Invalid("AGENT_REQUIRED", err.Error(), nil) case errors.Is(err, ports.ErrWorkspaceBranchCheckedOutElsewhere): return apierr.Conflict("BRANCH_CHECKED_OUT_ELSEWHERE", err.Error(), nil) case errors.Is(err, ports.ErrWorkspaceBranchNotFetched): diff --git a/backend/internal/service/session/service_test.go b/backend/internal/service/session/service_test.go index ee114108..9432148b 100644 --- a/backend/internal/service/session/service_test.go +++ b/backend/internal/service/session/service_test.go @@ -432,6 +432,7 @@ func TestToAPIErrorMapsWorkspaceBranchSentinels(t *testing.T) { {"invalid branch", fmt.Errorf("spawn mer-1: workspace: %w: \"bad!!\" (exit 1)", ports.ErrWorkspaceBranchInvalid), apierr.KindInvalid, "INVALID_BRANCH"}, {"agent binary not found", fmt.Errorf("spawn mer-1: %w", ports.ErrAgentBinaryNotFound), apierr.KindInvalid, "AGENT_BINARY_NOT_FOUND"}, {"unknown harness", fmt.Errorf("spawn: %w: %q", sessionmanager.ErrUnknownHarness, "bogus"), apierr.KindInvalid, "UNKNOWN_HARNESS"}, + {"missing harness", fmt.Errorf("spawn: %w: configure project worker.agent or pass --harness", sessionmanager.ErrMissingHarness), apierr.KindInvalid, "AGENT_REQUIRED"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/backend/internal/session_manager/manager.go b/backend/internal/session_manager/manager.go index b79f938a..19a54812 100644 --- a/backend/internal/session_manager/manager.go +++ b/backend/internal/session_manager/manager.go @@ -32,6 +32,9 @@ var ( // adapter. The API maps it to a 400 so a typo'd `--harness` is a validation // error, not an opaque 500. ErrUnknownHarness = errors.New("session: unknown agent harness") + // ErrMissingHarness means neither the spawn request nor the project's role + // config selected an agent. Worker/orchestrator spawns must be explicit. + ErrMissingHarness = errors.New("session: agent harness required") ) // Env vars a spawned process reads to learn who it is. @@ -86,11 +89,7 @@ type Manager struct { messenger ports.AgentMessenger lcm lifecycleRecorder dataDir string - // defaultHarness is the daemon's configured default agent (AO_AGENT). A spawn - // that names no harness resolves to it before the seed row is written, so the - // stored/returned harness matches the agent the resolver actually launches. - defaultHarness domain.AgentHarness - clock func() time.Time + clock func() time.Time // lookPath is exec.LookPath in production; tests substitute a stub so // they don't need real binaries on PATH. Returns ports.ErrAgentBinaryNotFound // when the binary is missing so the sentinel propagates through toAPIError. @@ -113,12 +112,7 @@ type Deps struct { // DataDir is exported to spawned agents as AO_DATA_DIR so their hook // commands can open the same store. DataDir string - // DefaultHarness is the daemon's configured default agent (AO_AGENT), used to - // resolve a spawn that names no harness. Wiring passes config.DefaultAgent; - // left empty, an unspecified harness stays empty (the resolver still defaults - // it at launch, but the record won't reflect the real agent). - DefaultHarness domain.AgentHarness - Clock func() time.Time + Clock func() time.Time // LookPath overrides exec.LookPath for the pre-launch agent-binary check. // Production wiring leaves this nil and the manager defaults to // exec.LookPath; tests inject a stub so they need not seed real binaries. @@ -136,18 +130,17 @@ type Deps struct { // time.Now when Deps.Clock is nil. func New(d Deps) *Manager { m := &Manager{ - runtime: d.Runtime, - agents: d.Agents, - workspace: d.Workspace, - store: d.Store, - messenger: d.Messenger, - lcm: d.Lifecycle, - dataDir: d.DataDir, - defaultHarness: d.DefaultHarness, - clock: d.Clock, - lookPath: d.LookPath, - executable: d.Executable, - logger: d.Logger, + runtime: d.Runtime, + agents: d.Agents, + workspace: d.Workspace, + store: d.Store, + messenger: d.Messenger, + lcm: d.Lifecycle, + dataDir: d.DataDir, + clock: d.Clock, + lookPath: d.LookPath, + executable: d.Executable, + logger: d.Logger, } if m.clock == nil { // UTC so spawn-stamped CreatedAt/UpdatedAt match every other session @@ -179,12 +172,8 @@ func (m *Manager) Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Sess // A per-project role override picks the harness when the spawn names none, // so a project can default workers to one agent and orchestrators to another. cfg.Harness = effectiveHarness(cfg.Harness, cfg.Kind, project.Config) - // Resolve an unspecified harness to the daemon default BEFORE the seed row is - // written, so the stored/returned harness matches the agent the resolver - // launches (otherwise a default-agent session persists an empty harness and - // the UI can't tell which agent is running). if cfg.Harness == "" { - cfg.Harness = m.defaultHarness + return domain.SessionRecord{}, fmt.Errorf("spawn: %w: configure project %s.agent or pass --harness", ErrMissingHarness, roleConfigName(cfg.Kind)) } // Reject an unknown harness before any durable state is created. Doing this @@ -307,8 +296,8 @@ func (m *Manager) loadProject(ctx context.Context, projectID domain.ProjectID) ( } // effectiveHarness resolves the harness for a spawn: an explicit harness wins; -// otherwise the project's role override for the session kind applies; otherwise -// it stays empty so the daemon's global default (AO_AGENT) is used downstream. +// otherwise the project's role override for the session kind applies. Empty is +// invalid for new worker/orchestrator launches and is rejected by Spawn. func effectiveHarness(explicit domain.AgentHarness, kind domain.SessionKind, cfg domain.ProjectConfig) domain.AgentHarness { if explicit != "" { return explicit @@ -319,6 +308,13 @@ func effectiveHarness(explicit domain.AgentHarness, kind domain.SessionKind, cfg return "" } +func roleConfigName(kind domain.SessionKind) string { + if kind == domain.KindOrchestrator { + return "orchestrator" + } + return "worker" +} + // effectiveAgentConfig merges the role override's agent config over the // project's base agent config; set override fields win. func effectiveAgentConfig(kind domain.SessionKind, cfg domain.ProjectConfig) ports.AgentConfig { diff --git a/backend/internal/session_manager/manager_test.go b/backend/internal/session_manager/manager_test.go index 869467f3..e3a8afd4 100644 --- a/backend/internal/session_manager/manager_test.go +++ b/backend/internal/session_manager/manager_test.go @@ -223,6 +223,7 @@ func (m *fakeMessenger) Send(_ context.Context, _ domain.SessionID, msg string) func newManager() (*Manager, *fakeStore, *fakeRuntime, *fakeWorkspace) { st := newFakeStore() + st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: testRoleAgents()} rt := &fakeRuntime{} ws := &fakeWorkspace{} // Stub lookPath so the pre-launch agent-binary check passes; the fakeAgent @@ -231,6 +232,12 @@ func newManager() (*Manager, *fakeStore, *fakeRuntime, *fakeWorkspace) { m := New(Deps{Runtime: rt, Agents: fakeAgents{}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath}) return m, st, rt, ws } +func testRoleAgents() domain.ProjectConfig { + return domain.ProjectConfig{ + Worker: domain.RoleOverride{Harness: domain.HarnessClaudeCode}, + Orchestrator: domain.RoleOverride{Harness: domain.HarnessClaudeCode}, + } +} func seedTerminal(st *fakeStore, id domain.SessionID, meta domain.SessionMetadata) { st.sessions[id] = domain.SessionRecord{ID: id, ProjectID: "mer", Metadata: meta, IsTerminated: true, Activity: domain.Activity{State: domain.ActivityExited}} } @@ -273,10 +280,11 @@ func TestSpawn_ResolvesProjectConfig(t *testing.T) { t.Fatal("runtime env missing AO_SESSION_ID") } - // A project with no stored config yields a zero AgentConfig (adapter defaults). + // A project with no stored config yields a zero AgentConfig (adapter defaults) + // when the spawn explicitly names its agent. st.projects["bare"] = domain.ProjectRecord{ID: "bare"} agent.lastConfig = ports.AgentConfig{Model: "stale"} - if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "bare", Kind: domain.KindWorker}); err != nil { + if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "bare", Kind: domain.KindWorker, Harness: domain.HarnessCodex}); err != nil { t.Fatal(err) } if !agent.lastConfig.IsZero() { @@ -284,30 +292,38 @@ func TestSpawn_ResolvesProjectConfig(t *testing.T) { } } -// TestSpawn_PersistsResolvedDefaultHarness locks the fix for the mislabelled -// agent: a spawn that names no harness must persist the daemon's default agent -// (so the API/UI report what actually runs), while an explicit harness wins. -func TestSpawn_PersistsResolvedDefaultHarness(t *testing.T) { +func TestSpawn_RejectsMissingRoleHarness(t *testing.T) { st := newFakeStore() st.projects["mer"] = domain.ProjectRecord{ID: "mer"} m := New(Deps{ Runtime: &fakeRuntime{}, Agents: fakeAgents{}, Workspace: &fakeWorkspace{}, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, - LookPath: func(string) (string, error) { return "/bin/true", nil }, - DefaultHarness: domain.HarnessClaudeCode, + LookPath: func(string) (string, error) { return "/bin/true", nil }, }) - if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker}); err != nil { - t.Fatal(err) + if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker}); !errors.Is(err, ErrMissingHarness) { + t.Fatalf("worker err = %v, want ErrMissingHarness", err) } - if got := st.sessions["mer-1"].Harness; got != domain.HarnessClaudeCode { - t.Fatalf("unspecified harness = %q, want resolved default %q", got, domain.HarnessClaudeCode) + if len(st.sessions) != 0 { + t.Fatalf("missing worker harness must not create a session row, got %d", len(st.sessions)) + } + if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindOrchestrator}); !errors.Is(err, ErrMissingHarness) { + t.Fatalf("orchestrator err = %v, want ErrMissingHarness", err) } +} +func TestSpawn_ExplicitHarnessWinsWithoutProjectRoleHarness(t *testing.T) { + st := newFakeStore() + st.projects["mer"] = domain.ProjectRecord{ID: "mer"} + m := New(Deps{ + Runtime: &fakeRuntime{}, Agents: fakeAgents{}, Workspace: &fakeWorkspace{}, Store: st, + Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, + LookPath: func(string) (string, error) { return "/bin/true", nil }, + }) if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker, Harness: domain.HarnessCodex}); err != nil { t.Fatal(err) } - if got := st.sessions["mer-2"].Harness; got != domain.HarnessCodex { + if got := st.sessions["mer-1"].Harness; got != domain.HarnessCodex { t.Fatalf("explicit harness = %q, want %q", got, domain.HarnessCodex) } } @@ -561,7 +577,10 @@ func TestSpawn_ForwardsResolvedAgentConfigPermissions(t *testing.T) { st := newFakeStore() st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: domain.ProjectConfig{ AgentConfig: domain.AgentConfig{Permissions: domain.PermissionModeAuto}, - Worker: domain.RoleOverride{AgentConfig: domain.AgentConfig{Permissions: domain.PermissionModeBypassPermissions}}, + Worker: domain.RoleOverride{ + Harness: domain.HarnessClaudeCode, + AgentConfig: domain.AgentConfig{Permissions: domain.PermissionModeBypassPermissions}, + }, }} agent := &recordingAgent{} lookPath := func(string) (string, error) { return "/bin/true", nil } @@ -610,6 +629,7 @@ func TestRestore_ForwardsResolvedAgentConfigPermissions(t *testing.T) { func TestSpawnWorker_AppendsActiveOrchestratorContact(t *testing.T) { st := newFakeStore() + st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: testRoleAgents()} st.num = 1 st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Kind: domain.KindOrchestrator} agent := &recordingAgent{} @@ -646,6 +666,7 @@ func TestSpawnWorker_AppendsActiveOrchestratorContact(t *testing.T) { func TestSpawnWorker_SkipsTerminatedOrchestratorContact(t *testing.T) { st := newFakeStore() + st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: testRoleAgents()} st.num = 1 st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Kind: domain.KindOrchestrator, IsTerminated: true} agent := &recordingAgent{} @@ -666,6 +687,7 @@ func TestSpawnWorker_SkipsTerminatedOrchestratorContact(t *testing.T) { func TestSpawnOrchestrator_UsesCoordinatorPrompt(t *testing.T) { st := newFakeStore() + st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: testRoleAgents()} agent := &recordingAgent{} rt := &fakeRuntime{} ws := &fakeWorkspace{} @@ -878,6 +900,7 @@ func TestRollbackSpawn_FallsBackToKillForLiveRow(t *testing.T) { // reaper later mistakes for a live session. func TestSpawn_RejectsMissingAgentBinary(t *testing.T) { st := newFakeStore() + st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: testRoleAgents()} rt := &fakeRuntime{} ws := &fakeWorkspace{} notFound := func(name string) (string, error) { @@ -927,6 +950,7 @@ func TestSpawn_RejectsUnknownHarness(t *testing.T) { // buffer capturing its log output, for the hook PATH pin tests. func pathPinManager(executable func() (string, error)) (*Manager, *fakeStore, *fakeRuntime, *bytes.Buffer) { st := newFakeStore() + st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: testRoleAgents()} rt := &fakeRuntime{} logBuf := &bytes.Buffer{} lookPath := func(string) (string, error) { return "/bin/true", nil } @@ -1018,7 +1042,8 @@ func TestSpawn_ProjectPATHIsPinBase(t *testing.T) { daemonExe := filepath.Join(t.TempDir(), "ao") m, st, rt, _ := pathPinManager(func() (string, error) { return daemonExe, nil }) st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: domain.ProjectConfig{ - Env: map[string]string{"PATH": "/proj/bin"}, + Env: map[string]string{"PATH": "/proj/bin"}, + Worker: domain.RoleOverride{Harness: domain.HarnessClaudeCode}, }} if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker}); err != nil { t.Fatal(err) diff --git a/frontend/src/renderer/components/CreateProjectAgentSheet.tsx b/frontend/src/renderer/components/CreateProjectAgentSheet.tsx new file mode 100644 index 00000000..462fe15b --- /dev/null +++ b/frontend/src/renderer/components/CreateProjectAgentSheet.tsx @@ -0,0 +1,119 @@ +import { useEffect, useState } from "react"; +import { AGENT_OPTIONS } from "../lib/agent-options"; +import { Button } from "./ui/button"; +import { Label } from "./ui/label"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "./ui/select"; +import { Sheet, SheetContent, SheetDescription, SheetFooter, SheetHeader, SheetTitle } from "./ui/sheet"; + +export type CreateProjectAgentSelection = { + workerAgent: string; + orchestratorAgent: string; +}; + +type CreateProjectAgentSheetProps = { + error?: string | null; + isCreating: boolean; + onOpenChange: (open: boolean) => void; + onSubmit: (selection: CreateProjectAgentSelection) => Promise; + open: boolean; + path: string | null; +}; + +export function CreateProjectAgentSheet({ + error, + isCreating, + onOpenChange, + onSubmit, + open, + path, +}: CreateProjectAgentSheetProps) { + const [workerAgent, setWorkerAgent] = useState(""); + const [orchestratorAgent, setOrchestratorAgent] = useState(""); + const canSubmit = workerAgent !== "" && orchestratorAgent !== "" && !isCreating; + + useEffect(() => { + if (!open) { + setWorkerAgent(""); + setOrchestratorAgent(""); + } + }, [open, path]); + + return ( + !isCreating && onOpenChange(next)}> + + + Project agents + {path ?? ""} + +
{ + event.preventDefault(); + if (!canSubmit) return; + void onSubmit({ workerAgent, orchestratorAgent }); + }} + > +
+ + + {error &&

{error}

} +
+ + + + +
+ + + ); +} + +export function RequiredAgentField({ + id, + label, + onChange, + placeholder, + value, +}: { + id: string; + label: string; + onChange: (value: string) => void; + placeholder: string; + value: string; +}) { + return ( +
+ + +
+ ); +} diff --git a/frontend/src/renderer/components/ProjectSettingsForm.test.tsx b/frontend/src/renderer/components/ProjectSettingsForm.test.tsx index 4e387c98..99569a81 100644 --- a/frontend/src/renderer/components/ProjectSettingsForm.test.tsx +++ b/frontend/src/renderer/components/ProjectSettingsForm.test.tsx @@ -149,6 +149,10 @@ describe("ProjectSettingsForm", () => { path: "/repo/project-one", repo: "", defaultBranch: "main", + config: { + worker: { agent: "codex" }, + orchestrator: { agent: "claude-code" }, + }, }, }, error: undefined, @@ -165,4 +169,35 @@ describe("ProjectSettingsForm", () => { expect(await screen.findByText("invalid permissions")).toBeInTheDocument(); expect(screen.queryByText("Saved.")).not.toBeInTheDocument(); }); + + it("requires worker and orchestrator agents for existing projects missing role config", async () => { + getMock.mockResolvedValue({ + data: { + status: "ok", + project: { + id: "proj-1", + name: "Project One", + kind: "single_repo", + path: "/repo/project-one", + repo: "", + defaultBranch: "main", + config: {}, + }, + }, + error: undefined, + }); + + renderSettings(); + + expect(await screen.findByText("Worker and orchestrator agents are required.")).toBeInTheDocument(); + expect(screen.getByRole("combobox", { name: "Default worker agent" })).toHaveTextContent("Select worker agent"); + expect(screen.getByRole("combobox", { name: "Default orchestrator agent" })).toHaveTextContent( + "Select orchestrator agent", + ); + + await userEvent.click(screen.getByRole("button", { name: "Save changes" })); + + expect(await screen.findAllByText("Worker and orchestrator agents are required.")).toHaveLength(2); + expect(putMock).not.toHaveBeenCalled(); + }); }); diff --git a/frontend/src/renderer/components/ProjectSettingsForm.tsx b/frontend/src/renderer/components/ProjectSettingsForm.tsx index 673204e1..959f6e29 100644 --- a/frontend/src/renderer/components/ProjectSettingsForm.tsx +++ b/frontend/src/renderer/components/ProjectSettingsForm.tsx @@ -3,6 +3,7 @@ import { useState } from "react"; import type { components } from "../../api/schema"; import { apiClient, apiErrorMessage } from "../lib/api-client"; import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; +import { RequiredAgentField } from "./CreateProjectAgentSheet"; import { DashboardSubhead } from "./DashboardSubhead"; import { Button } from "./ui/button"; import { Card, CardContent, CardHeader, CardTitle } from "./ui/card"; @@ -12,9 +13,6 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from ". type Project = components["schemas"]["Project"]; type ProjectConfig = components["schemas"]["ProjectConfig"]; -// Agents the daemon registers. Empty = "use the daemon default". -const AGENT_OPTIONS = ["claude-code", "codex", "opencode", "amp", "goose", "kiro"] as const; - const PERMISSION_MODE_OPTIONS = [ { value: "default", label: "Default" }, { value: "accept-edits", label: "Accept edits" }, @@ -78,6 +76,8 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje reviewerHarness: config.reviewers?.[0]?.harness ?? "", }); const [savedAt, setSavedAt] = useState(null); + const [validationError, setValidationError] = useState(null); + const missingRequiredAgent = form.workerAgent === "" || form.orchestratorAgent === ""; const mutation = useMutation({ mutationFn: async () => { @@ -87,8 +87,8 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje ...config, defaultBranch: form.defaultBranch || undefined, sessionPrefix: form.sessionPrefix || undefined, - worker: blankToUndefined({ ...config.worker, agent: form.workerAgent || undefined }), - orchestrator: blankToUndefined({ ...config.orchestrator, agent: form.orchestratorAgent || undefined }), + worker: { ...config.worker, agent: form.workerAgent }, + orchestrator: { ...config.orchestrator, agent: form.orchestratorAgent }, agentConfig: blankToUndefined({ ...config.agentConfig, model: form.model || undefined, @@ -104,6 +104,7 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje }, onSuccess: () => { setSavedAt(Date.now()); + setValidationError(null); void queryClient.invalidateQueries({ queryKey: ["project", projectId] }); onSaved(); }, @@ -114,6 +115,12 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje className="mx-auto flex max-w-2xl flex-col gap-4" onSubmit={(event) => { event.preventDefault(); + setSavedAt(null); + if (missingRequiredAgent) { + setValidationError("Worker and orchestrator agents are required."); + return; + } + setValidationError(null); mutation.mutate(); }} > @@ -159,20 +166,23 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje Agents - - setForm((f) => ({ ...f, workerAgent: v }))} - /> - - - setForm((f) => ({ ...f, orchestratorAgent: v }))} - /> - + setForm((f) => ({ ...f, workerAgent: v }))} + /> + setForm((f) => ({ ...f, orchestratorAgent: v }))} + /> + {missingRequiredAgent && ( +

Worker and orchestrator agents are required.

+ )} {mutation.isPending ? "Saving…" : "Save changes"} + {validationError && {validationError}} {mutation.isError && ( {mutation.error instanceof Error ? mutation.error.message : "Save failed"} @@ -250,25 +261,6 @@ function PermissionModeSelect({ ); } -function AgentSelect({ id, value, onChange }: { id: string; value: string; onChange: (value: string) => void }) { - // "" sentinel → daemon default; Select can't hold an empty value, so map it. - return ( - - ); -} - function ReviewerSelect({ id, value, onChange }: { id: string; value: string; onChange: (value: string) => void }) { return ( - + diff --git a/frontend/src/renderer/components/ProjectSettingsForm.tsx b/frontend/src/renderer/components/ProjectSettingsForm.tsx index 959f6e29..1c86bb85 100644 --- a/frontend/src/renderer/components/ProjectSettingsForm.tsx +++ b/frontend/src/renderer/components/ProjectSettingsForm.tsx @@ -171,6 +171,7 @@ function SettingsBody({ project, projectId, onSaved }: { project: Project; proje value={form.workerAgent} placeholder="Select worker agent" label="Default worker agent" + invalid={validationError !== null && form.workerAgent === ""} onChange={(v) => setForm((f) => ({ ...f, workerAgent: v }))} /> setForm((f) => ({ ...f, orchestratorAgent: v }))} /> {missingRequiredAgent && (