diff --git a/README.md b/README.md index 281cb376..7adf5678 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. App-wide worker and orchestrator defaults + are stored in daemon-backed settings. - **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 @@ -94,7 +95,7 @@ go build -o /tmp/ao ./cmd/ao # 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 -# Spawn a worker session running the default agent. +# Spawn a worker session running the configured default agent. /tmp/ao spawn --project your-repo --prompt "Refactor the auth module" # Inspect what's running. @@ -167,7 +168,6 @@ 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_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/adapters/runtime/zellij/zellij.go b/backend/internal/adapters/runtime/zellij/zellij.go index 938815f6..0e313abb 100644 --- a/backend/internal/adapters/runtime/zellij/zellij.go +++ b/backend/internal/adapters/runtime/zellij/zellij.go @@ -125,6 +125,11 @@ func (r *Runtime) Create(ctx context.Context, cfg ports.RuntimeConfig) (ports.Ru if cfg.WorkspacePath == "" { return ports.RuntimeHandle{}, errors.New("zellij runtime: workspace path is required") } + if stat, err := os.Stat(cfg.WorkspacePath); err != nil { + return ports.RuntimeHandle{}, fmt.Errorf("zellij runtime: workspace path %q: %w", cfg.WorkspacePath, err) + } else if !stat.IsDir() { + return ports.RuntimeHandle{}, fmt.Errorf("zellij runtime: workspace path %q is not a directory", cfg.WorkspacePath) + } if len(cfg.Argv) == 0 { return ports.RuntimeHandle{}, errors.New("zellij runtime: launch command is required") } diff --git a/backend/internal/adapters/runtime/zellij/zellij_test.go b/backend/internal/adapters/runtime/zellij/zellij_test.go index 784cda43..b69bd3ee 100644 --- a/backend/internal/adapters/runtime/zellij/zellij_test.go +++ b/backend/internal/adapters/runtime/zellij/zellij_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "os/exec" + "path/filepath" "reflect" "runtime" "strings" @@ -271,7 +272,7 @@ func TestCreateRejectsInvalidEnvKeys(t *testing.T) { r.runner = &fakeRunner{} _, err := r.Create(context.Background(), ports.RuntimeConfig{ SessionID: "sess-1", - WorkspacePath: "/tmp/ws", + WorkspacePath: t.TempDir(), Argv: []string{"echo", "ready"}, Env: map[string]string{"BAD KEY": "x"}, }) @@ -280,6 +281,19 @@ func TestCreateRejectsInvalidEnvKeys(t *testing.T) { } } +func TestCreateRejectsMissingWorkspacePath(t *testing.T) { + r := New(Options{Binary: "zellij-test", Timeout: time.Second, Shell: "/bin/zsh"}) + r.runner = &fakeRunner{} + _, err := r.Create(context.Background(), ports.RuntimeConfig{ + SessionID: "sess-1", + WorkspacePath: filepath.Join(t.TempDir(), "missing"), + Argv: []string{"echo", "ready"}, + }) + if err == nil || !strings.Contains(err.Error(), "workspace path") { + t.Fatalf("Create err = %v, want workspace path error", err) + } +} + func TestCreateStartsSessionAndDiscoversPane(t *testing.T) { fr := &fakeRunner{outputs: [][]byte{[]byte("zellij 0.44.3"), nil, nil, []byte(`[{"id":0,"is_plugin":true,"title":"zellij:tab-bar"},{"id":3,"is_plugin":false,"title":"agent"}]`)}} r := New(Options{Binary: "zellij-test", Timeout: time.Second, Shell: "/bin/zsh", SocketDir: "/tmp/zj", ConfigDir: "/tmp/cfg"}) @@ -287,7 +301,7 @@ func TestCreateStartsSessionAndDiscoversPane(t *testing.T) { handle, err := r.Create(context.Background(), ports.RuntimeConfig{ SessionID: "sess-1", - WorkspacePath: "/tmp/ws", + WorkspacePath: t.TempDir(), Argv: []string{"echo", "ready"}, Env: map[string]string{"AO_SESSION_ID": "sess-1"}, }) @@ -329,7 +343,7 @@ func TestCreateClearsStaleSessionBeforeCreating(t *testing.T) { if _, err := r.Create(context.Background(), ports.RuntimeConfig{ SessionID: "sess-1", - WorkspacePath: "/tmp/ws", + WorkspacePath: t.TempDir(), Argv: []string{"echo", "ready"}, }); err != nil { t.Fatalf("Create: %v", err) diff --git a/backend/internal/adapters/workspace/gitworktree/workspace.go b/backend/internal/adapters/workspace/gitworktree/workspace.go index d42b3363..d5d2dbb2 100644 --- a/backend/internal/adapters/workspace/gitworktree/workspace.go +++ b/backend/internal/adapters/workspace/gitworktree/workspace.go @@ -229,14 +229,56 @@ func (w *Workspace) existingWorktree(ctx context.Context, repo, path string, cfg if err != nil { return ports.WorkspaceInfo{}, false, err } - if rec, ok := findWorktree(records, path); ok { - branch := rec.Branch - if branch == "" { - branch = cfg.Branch + rec, ok := findWorktree(records, path) + if !ok { + return ports.WorkspaceInfo{}, false, nil + } + usable, err := registeredWorktreeUsable(rec) + if err != nil { + return ports.WorkspaceInfo{}, false, err + } + if !usable { + if _, err := w.run(ctx, w.binary, worktreePruneArgs(repo)...); err != nil { + return ports.WorkspaceInfo{}, false, fmt.Errorf("gitworktree: worktree prune stale %q: %w", path, err) + } + records, err = w.listRecords(ctx, repo) + if err != nil { + return ports.WorkspaceInfo{}, false, err + } + rec, ok = findWorktree(records, path) + if !ok { + return ports.WorkspaceInfo{}, false, nil + } + usable, err = registeredWorktreeUsable(rec) + if err != nil { + return ports.WorkspaceInfo{}, false, err } - return ports.WorkspaceInfo{Path: path, Branch: branch, SessionID: cfg.SessionID, ProjectID: cfg.ProjectID}, true, nil + if !usable { + return ports.WorkspaceInfo{}, false, fmt.Errorf("gitworktree: refusing to reuse stale registered worktree %q after prune", path) + } + } + branch := rec.Branch + if branch == "" { + branch = cfg.Branch + } + return ports.WorkspaceInfo{Path: path, Branch: branch, SessionID: cfg.SessionID, ProjectID: cfg.ProjectID}, true, nil +} + +func registeredWorktreeUsable(rec worktreeRecord) (bool, error) { + if rec.Prunable { + return false, nil + } + stat, err := os.Stat(rec.Path) + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("gitworktree: stat registered worktree %q: %w", rec.Path, err) + } + if !stat.IsDir() { + return false, fmt.Errorf("gitworktree: registered worktree %q is not a directory", rec.Path) } - return ports.WorkspaceInfo{}, false, nil + return true, nil } func (w *Workspace) addWorktree(ctx context.Context, repo, path, branch, baseBranch string) error { diff --git a/backend/internal/adapters/workspace/gitworktree/workspace_test.go b/backend/internal/adapters/workspace/gitworktree/workspace_test.go index 1fa0f611..b00a42d9 100644 --- a/backend/internal/adapters/workspace/gitworktree/workspace_test.go +++ b/backend/internal/adapters/workspace/gitworktree/workspace_test.go @@ -59,6 +59,15 @@ func TestBaseRefCandidates(t *testing.T) { } } +func containsJoined(values []string, want string) bool { + for _, value := range values { + if strings.Contains(value, want) { + return true + } + } + return false +} + func TestParseWorktreePorcelain(t *testing.T) { input := strings.Join([]string{ "worktree /repo", @@ -182,6 +191,9 @@ func TestCreateReusesRegisteredWorktreeAtExpectedPath(t *testing.T) { t.Fatalf("new: %v", err) } path := filepath.Join(ws.managedRoot, "proj", "orchestrator", "proj-orchestrator") + if err := os.MkdirAll(path, 0o750); err != nil { + t.Fatalf("mkdir path: %v", err) + } cfg := ports.WorkspaceConfig{ ProjectID: "proj", SessionID: "proj-1", @@ -211,6 +223,63 @@ func TestCreateReusesRegisteredWorktreeAtExpectedPath(t *testing.T) { } } +func TestCreatePrunesStaleRegisteredWorktreeBeforeAdd(t *testing.T) { + root := t.TempDir() + repo := t.TempDir() + ws, err := New(Options{ManagedRoot: root, RepoResolver: StaticRepoResolver{"proj": repo}}) + if err != nil { + t.Fatalf("new: %v", err) + } + path := filepath.Join(ws.managedRoot, "proj", "orchestrator", "proj-orchestrator") + cfg := ports.WorkspaceConfig{ + ProjectID: "proj", + SessionID: "proj-1", + Kind: domain.KindOrchestrator, + SessionPrefix: "proj", + Branch: "ao/proj-orchestrator", + } + + listCalls := 0 + var calls []string + ws.run = func(_ context.Context, _ string, args ...string) ([]byte, error) { + joined := strings.Join(args, " ") + calls = append(calls, joined) + switch { + case strings.Contains(joined, "check-ref-format"): + return nil, nil + case strings.Contains(joined, "worktree list --porcelain"): + listCalls++ + if listCalls == 1 { + return []byte("worktree " + path + "\nHEAD abc123\nbranch refs/heads/ao/proj-orchestrator\nprunable gitdir file points to non-existent location\n"), nil + } + return []byte("worktree " + repo + "\nHEAD root123\nbranch refs/heads/main\n"), nil + case strings.Contains(joined, "worktree prune"): + return nil, nil + case strings.Contains(joined, "rev-parse --verify --quiet refs/heads/ao/proj-orchestrator"): + return []byte("abc123\n"), nil + case strings.Contains(joined, "worktree add"): + return nil, nil + default: + t.Fatalf("unexpected git invocation: %v", args) + return nil, nil + } + } + + info, err := ws.Create(context.Background(), cfg) + if err != nil { + t.Fatalf("Create: %v", err) + } + if info.Path != path || info.Branch != "ao/proj-orchestrator" { + t.Fatalf("info = %#v, want path %q branch ao/proj-orchestrator", info, path) + } + if !containsJoined(calls, "worktree prune") { + t.Fatalf("calls missing prune: %#v", calls) + } + if !containsJoined(calls, "worktree add "+path+" ao/proj-orchestrator") { + t.Fatalf("calls missing worktree add: %#v", calls) + } +} + // TestValidateConfigRejectsPathEscapingIDs covers review item RB: filepath.Join // in managedPath cleans `..` segments before validateManagedPath sees them, so a // session id of "../other" would stay inside managedRoot while jumping projects. diff --git a/backend/internal/cli/dto_drift_e2e_test.go b/backend/internal/cli/dto_drift_e2e_test.go index 2488d8f4..2114d95e 100644 --- a/backend/internal/cli/dto_drift_e2e_test.go +++ b/backend/internal/cli/dto_drift_e2e_test.go @@ -62,8 +62,8 @@ func (f *fakeSessionService) Spawn(_ context.Context, cfg ports.SpawnConfig) (do }, nil } -func (f *fakeSessionService) SpawnOrchestrator(ctx context.Context, projectID domain.ProjectID, _ bool) (domain.Session, error) { - return f.Spawn(ctx, ports.SpawnConfig{ProjectID: projectID, Kind: domain.KindOrchestrator}) +func (f *fakeSessionService) SpawnOrchestrator(ctx context.Context, projectID domain.ProjectID, _ bool, harness domain.AgentHarness) (domain.Session, error) { + return f.Spawn(ctx, ports.SpawnConfig{ProjectID: projectID, Kind: domain.KindOrchestrator, Harness: harness}) } func (f *fakeSessionService) Get(context.Context, domain.SessionID) (domain.Session, error) { diff --git a/backend/internal/cli/spawn.go b/backend/internal/cli/spawn.go index efa4f3f1..b1c79e4f 100644 --- a/backend/internal/cli/spawn.go +++ b/backend/internal/cli/spawn.go @@ -44,7 +44,8 @@ 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, the project's role override, or the\n" + + "app-wide default agent configured in Settings. It uses 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 +121,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: app setting)") 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 e2a9386c..1296ea42 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -29,9 +29,6 @@ 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 = "claude-code" ) // DefaultAllowedOrigins are the browser origins the daemon's CORS boundary @@ -63,10 +60,6 @@ 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 string // AllowedOrigins are the browser origins granted CORS read access (see // DefaultAllowedOrigins). Overridden by AO_ALLOWED_ORIGINS. AllowedOrigins []string @@ -89,7 +82,6 @@ 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_ALLOWED_ORIGINS CORS origins, comma-separated (default DefaultAllowedOrigins) // // The bind host is not configurable: the daemon is loopback-only by design. @@ -99,7 +91,6 @@ func Load() (Config, error) { Port: DefaultPort, RequestTimeout: DefaultRequestTimeout, ShutdownTimeout: DefaultShutdownTimeout, - Agent: DefaultAgent, AllowedOrigins: DefaultAllowedOrigins, } @@ -130,10 +121,6 @@ func Load() (Config, error) { cfg.ShutdownTimeout = d } - if raw := os.Getenv("AO_AGENT"); raw != "" { - cfg.Agent = raw - } - if raw, ok := os.LookupEnv("AO_ALLOWED_ORIGINS"); ok && raw != "" { // Explicit override replaces the defaults entirely so a deployment can // also narrow the list. The "null" origin is rejected, never silently diff --git a/backend/internal/config/config_test.go b/backend/internal/config/config_test.go index 4ce22512..30be606f 100644 --- a/backend/internal/config/config_test.go +++ b/backend/internal/config/config_test.go @@ -10,7 +10,7 @@ import ( func TestLoadDefaults(t *testing.T) { // Clear every recognised var so we observe pure defaults regardless of the // surrounding environment. - for _, k := range []string{"AO_PORT", "AO_REQUEST_TIMEOUT", "AO_SHUTDOWN_TIMEOUT", "AO_RUN_FILE", "AO_DATA_DIR", "AO_AGENT", "AO_ALLOWED_ORIGINS"} { + for _, k := range []string{"AO_PORT", "AO_REQUEST_TIMEOUT", "AO_SHUTDOWN_TIMEOUT", "AO_RUN_FILE", "AO_DATA_DIR", "AO_ALLOWED_ORIGINS"} { t.Setenv(k, "") } diff --git a/backend/internal/daemon/daemon.go b/backend/internal/daemon/daemon.go index 747f5251..4c4f1e9f 100644 --- a/backend/internal/daemon/daemon.go +++ b/backend/internal/daemon/daemon.go @@ -18,6 +18,7 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/runfile" notificationsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/notification" projectsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/project" + settingssvc "github.com/aoagents/agent-orchestrator/backend/internal/service/settings" "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite" "github.com/aoagents/agent-orchestrator/backend/internal/terminal" ) @@ -94,9 +95,8 @@ func Run() error { lcStack.scmDone = startSCMObserver(ctx, store, lcStack.LCM, log) // 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 - // on the API. + // zellij runtime, a gitworktree workspace, the per-session agent resolver, + // and the agent messenger, then mount it on the API. sessionSvc, reviewSvc, err := startSession(cfg, runtimeAdapter, store, lcStack.LCM, messenger, log) if err != nil { stop() @@ -111,6 +111,7 @@ func Run() error { Projects: projectsvc.NewWithDeps(projectsvc.Deps{Store: store, Sessions: sessionSvc}), Sessions: sessionSvc, Reviews: reviewSvc, + Settings: settingssvc.New(store), Notifications: notifier, NotificationStream: notificationHub, CDC: store, diff --git a/backend/internal/daemon/lifecycle_wiring.go b/backend/internal/daemon/lifecycle_wiring.go index a66d3dea..45a9eb91 100644 --- a/backend/internal/daemon/lifecycle_wiring.go +++ b/backend/internal/daemon/lifecycle_wiring.go @@ -60,17 +60,10 @@ 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 +// 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, 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 - } - agents, err := buildAgentResolver(defaultAgent, log) + agents, err := buildAgentResolver(log) if err != nil { return nil, nil, err } @@ -87,15 +80,15 @@ 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, + AgentDefaults: store, + Logger: log, }) scmProvider, err := newGitHubSCMProvider(log) if err != nil { @@ -178,20 +171,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 misses: session +// spawns must resolve defaults before they reach the registry. 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 @@ -202,27 +190,19 @@ 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. -func buildAgentResolver(defaultAgent string, log *slog.Logger) (ports.AgentResolver, error) { - if defaultAgent == "" { - defaultAgent = config.DefaultAgent - } +// adapters. The session lane plugs this in when it mounts the controller-facing +// session service at the httpd APIDeps.Sessions slot. +func buildAgentResolver(log *slog.Logger) (ports.AgentResolver, error) { reg, err := buildAgentRegistry() if err != nil { return nil, err } - resolver := agentRegistry{reg: reg, defaultHarness: domain.AgentHarness(defaultAgent)} - if _, ok := resolver.Agent(""); !ok { - return nil, fmt.Errorf("configured default agent %q is not a registered adapter", defaultAgent) - } + resolver := agentRegistry{reg: reg} ids := make([]string, 0) for _, mf := range reg.Manifests() { ids = append(ids, mf.ID) } - log.Info("built per-session agent resolver", "default", defaultAgent, "registered", ids) + log.Info("built per-session agent resolver", "registered", ids) return resolver, nil } diff --git a/backend/internal/daemon/wiring_test.go b/backend/internal/daemon/wiring_test.go index 36e67344..ed5a9dfc 100644 --- a/backend/internal/daemon/wiring_test.go +++ b/backend/internal/daemon/wiring_test.go @@ -77,12 +77,11 @@ 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. +// real registry-backed per-session resolver: each concrete harness resolves to +// the matching registered adapter, while empty/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 + resolver, err := buildAgentResolver(log) if err != nil { t.Fatal(err) } @@ -113,7 +112,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 { @@ -130,6 +128,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/domain/agentdefaults.go b/backend/internal/domain/agentdefaults.go new file mode 100644 index 00000000..16129b02 --- /dev/null +++ b/backend/internal/domain/agentdefaults.go @@ -0,0 +1,43 @@ +package domain + +import "fmt" + +// AgentDefaults are the app-wide fallback harnesses used when a spawn does not +// name an explicit harness and the project has no role override. +type AgentDefaults struct { + DefaultWorkerAgent AgentHarness `json:"defaultWorkerAgent,omitempty" enum:"claude-code,codex,aider,opencode,grok,droid,amp,agy,crush,cursor,qwen,copilot,goose,auggie,continue,devin,cline,kimi,kiro,kilocode,vibe,pi,autohand"` + DefaultOrchestratorAgent AgentHarness `json:"defaultOrchestratorAgent,omitempty" enum:"claude-code,codex,aider,opencode,grok,droid,amp,agy,crush,cursor,qwen,copilot,goose,auggie,continue,devin,cline,kimi,kiro,kilocode,vibe,pi,autohand"` +} + +// HarnessFor returns the default harness for a session kind. Any non-worker +// role is treated as an orchestrator role because the domain only has those two +// concrete spawn roles today. +func (d AgentDefaults) HarnessFor(kind SessionKind) AgentHarness { + if kind == KindWorker || kind == "" { + return d.DefaultWorkerAgent + } + return d.DefaultOrchestratorAgent +} + +// Complete reports whether both app-wide defaults have been configured. +func (d AgentDefaults) Complete() bool { + return d.DefaultWorkerAgent != "" && d.DefaultOrchestratorAgent != "" +} + +// ValidateComplete rejects missing or unknown defaults. Settings writes use +// this strict path so the app never persists a half-configured default state. +func (d AgentDefaults) ValidateComplete() error { + if d.DefaultWorkerAgent == "" { + return fmt.Errorf("defaultWorkerAgent is required") + } + if !d.DefaultWorkerAgent.IsKnown() { + return fmt.Errorf("defaultWorkerAgent: unknown harness %q", d.DefaultWorkerAgent) + } + if d.DefaultOrchestratorAgent == "" { + return fmt.Errorf("defaultOrchestratorAgent is required") + } + if !d.DefaultOrchestratorAgent.IsKnown() { + return fmt.Errorf("defaultOrchestratorAgent: unknown harness %q", d.DefaultOrchestratorAgent) + } + return nil +} diff --git a/backend/internal/httpd/api.go b/backend/internal/httpd/api.go index 40b65d8a..2924624d 100644 --- a/backend/internal/httpd/api.go +++ b/backend/internal/httpd/api.go @@ -23,6 +23,7 @@ type APIDeps struct { Activity controllers.ActivityRecorder PRs prsvc.ActionManager Reviews reviewsvc.Manager + Settings controllers.SettingsService Notifications controllers.NotificationService NotificationStream controllers.NotificationStream CDC cdc.Source @@ -37,6 +38,7 @@ type API struct { sessions *controllers.SessionsController prs *controllers.PRsController reviews *controllers.ReviewsController + settings *controllers.SettingsController notifications *controllers.NotificationsController events *EventsController } @@ -56,6 +58,7 @@ func NewAPI(cfg config.Config, deps APIDeps) *API { }, prs: &controllers.PRsController{Svc: deps.PRs}, reviews: &controllers.ReviewsController{Svc: deps.Reviews}, + settings: &controllers.SettingsController{Svc: deps.Settings}, notifications: &controllers.NotificationsController{Svc: deps.Notifications, Stream: deps.NotificationStream}, events: &EventsController{Source: deps.CDC, Live: deps.Events}, } @@ -79,6 +82,7 @@ func (a *API) Register(root chi.Router) { a.sessions.Register(r) a.prs.Register(r) a.reviews.Register(r) + a.settings.Register(r) a.notifications.Register(r) // Sibling REST controllers plug in here. }) diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index a279d460..766810bc 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -1149,6 +1149,67 @@ paths: summary: Clean up terminated session workspaces tags: - sessions + /api/v1/settings/agents: + get: + operationId: getAgentDefaults + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/AgentDefaultsResponse' + description: OK + "500": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Internal Server Error + "501": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Implemented + summary: Get app-wide default agents + tags: + - settings + put: + operationId: setAgentDefaults + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/AgentDefaultsRequest' + required: true + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/AgentDefaultsResponse' + description: OK + "400": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Bad Request + "500": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Internal Server Error + "501": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Implemented + summary: Set app-wide default agents + tags: + - settings components: schemas: APIError: @@ -1195,6 +1256,120 @@ components: permissions: type: string type: object + AgentDefaultsRequest: + properties: + defaultOrchestratorAgent: + enum: + - claude-code + - codex + - aider + - opencode + - grok + - droid + - amp + - agy + - crush + - cursor + - qwen + - copilot + - goose + - auggie + - continue + - devin + - cline + - kimi + - kiro + - kilocode + - vibe + - pi + - autohand + type: string + defaultWorkerAgent: + enum: + - claude-code + - codex + - aider + - opencode + - grok + - droid + - amp + - agy + - crush + - cursor + - qwen + - copilot + - goose + - auggie + - continue + - devin + - cline + - kimi + - kiro + - kilocode + - vibe + - pi + - autohand + type: string + type: object + AgentDefaultsResponse: + properties: + configured: + type: boolean + defaultOrchestratorAgent: + enum: + - claude-code + - codex + - aider + - opencode + - grok + - droid + - amp + - agy + - crush + - cursor + - qwen + - copilot + - goose + - auggie + - continue + - devin + - cline + - kimi + - kiro + - kilocode + - vibe + - pi + - autohand + type: string + defaultWorkerAgent: + enum: + - claude-code + - codex + - aider + - opencode + - grok + - droid + - amp + - agy + - crush + - cursor + - qwen + - copilot + - goose + - auggie + - continue + - devin + - cline + - kimi + - kiro + - kilocode + - vibe + - pi + - autohand + type: string + required: + - configured + type: object ClaimPRRequest: properties: allowTakeover: @@ -1808,6 +1983,32 @@ components: properties: clean: type: boolean + harness: + enum: + - claude-code + - codex + - aider + - opencode + - grok + - droid + - amp + - agy + - crush + - cursor + - qwen + - copilot + - goose + - auggie + - continue + - devin + - cline + - kimi + - kiro + - kilocode + - vibe + - pi + - autohand + type: string projectId: type: string required: @@ -1903,6 +2104,8 @@ tags: name: prs - description: Code-review runs and findings name: reviews +- description: App-wide user settings + name: settings - description: Durable dashboard notifications name: notifications - description: Server-sent CDC event stream with durable replay diff --git a/backend/internal/httpd/apispec/specgen/build.go b/backend/internal/httpd/apispec/specgen/build.go index 2aeca734..a906710f 100644 --- a/backend/internal/httpd/apispec/specgen/build.go +++ b/backend/internal/httpd/apispec/specgen/build.go @@ -63,6 +63,8 @@ func Build() ([]byte, error) { "Pull-request actions (SCM lane)"), *(&openapi31.Tag{Name: "reviews"}).WithDescription( "Code-review runs and findings"), + *(&openapi31.Tag{Name: "settings"}).WithDescription( + "App-wide user settings"), *(&openapi31.Tag{Name: "notifications"}).WithDescription( "Durable dashboard notifications"), *(&openapi31.Tag{Name: "events"}).WithDescription( @@ -129,6 +131,7 @@ var schemaNames = map[string]string{ "DomainProjectConfig": "ProjectConfig", "DomainAgentConfig": "AgentConfig", "DomainRoleOverride": "RoleOverride", + "DomainAgentDefaults": "AgentDefaults", // httpd/controllers (wire envelopes) "ControllersListProjectsResponse": "ListProjectsResponse", "ControllersProjectResponse": "ProjectResponse", @@ -157,6 +160,8 @@ var schemaNames = map[string]string{ "ControllersSpawnOrchestratorRequest": "SpawnOrchestratorRequest", "ControllersSpawnOrchestratorResponse": "SpawnOrchestratorResponse", "ControllersOrchestratorResponse": "OrchestratorResponse", + "ControllersAgentDefaultsRequest": "AgentDefaultsRequest", + "ControllersAgentDefaultsResponse": "AgentDefaultsResponse", "ControllersListNotificationsQuery": "ListNotificationsQuery", "ControllersNotificationStreamQuery": "NotificationStreamQuery", "ControllersNotificationTarget": "NotificationTarget", @@ -258,10 +263,36 @@ func operations() []operation { ops = append(ops, sessionOperations()...) ops = append(ops, prOperations()...) ops = append(ops, reviewOperations()...) + ops = append(ops, settingsOperations()...) ops = append(ops, notificationOperations()...) return ops } +func settingsOperations() []operation { + return []operation{ + { + method: http.MethodGet, path: "/api/v1/settings/agents", id: "getAgentDefaults", tag: "settings", + summary: "Get app-wide default agents", + resps: []respUnit{ + {http.StatusOK, controllers.AgentDefaultsResponse{}}, + {http.StatusInternalServerError, envelope.APIError{}}, + {http.StatusNotImplemented, envelope.APIError{}}, + }, + }, + { + method: http.MethodPut, path: "/api/v1/settings/agents", id: "setAgentDefaults", tag: "settings", + summary: "Set app-wide default agents", + reqBody: controllers.AgentDefaultsRequest{}, + resps: []respUnit{ + {http.StatusOK, controllers.AgentDefaultsResponse{}}, + {http.StatusBadRequest, envelope.APIError{}}, + {http.StatusInternalServerError, envelope.APIError{}}, + {http.StatusNotImplemented, envelope.APIError{}}, + }, + }, + } +} + func notificationOperations() []operation { return []operation{ { diff --git a/backend/internal/httpd/controllers/dto.go b/backend/internal/httpd/controllers/dto.go index 2da2fe91..277a10b6 100644 --- a/backend/internal/httpd/controllers/dto.go +++ b/backend/internal/httpd/controllers/dto.go @@ -255,8 +255,9 @@ type OrchestratorIDParam struct { // SpawnOrchestratorRequest is the body of POST /api/v1/orchestrators. type SpawnOrchestratorRequest struct { - ProjectID domain.ProjectID `json:"projectId"` - Clean bool `json:"clean,omitempty"` + ProjectID domain.ProjectID `json:"projectId"` + Clean bool `json:"clean,omitempty"` + Harness domain.AgentHarness `json:"harness,omitempty" enum:"claude-code,codex,aider,opencode,grok,droid,amp,agy,crush,cursor,qwen,copilot,goose,auggie,continue,devin,cline,kimi,kiro,kilocode,vibe,pi,autohand"` } // SpawnOrchestratorResponse is the body of POST /api/v1/orchestrators. @@ -271,6 +272,17 @@ type OrchestratorResponse struct { ProjectName string `json:"projectName,omitempty"` } +// AgentDefaultsRequest is the body of PUT /api/v1/settings/agents. +type AgentDefaultsRequest struct { + domain.AgentDefaults +} + +// AgentDefaultsResponse is the body of GET/PUT /api/v1/settings/agents. +type AgentDefaultsResponse struct { + domain.AgentDefaults + Configured bool `json:"configured"` +} + // ListNotificationsQuery is the query string accepted by GET /api/v1/notifications. type ListNotificationsQuery struct { Status string `query:"status,omitempty" enum:"unread" description:"Notification status filter. V1 supports only unread."` diff --git a/backend/internal/httpd/controllers/sessions.go b/backend/internal/httpd/controllers/sessions.go index bc0a4504..7fbcb651 100644 --- a/backend/internal/httpd/controllers/sessions.go +++ b/backend/internal/httpd/controllers/sessions.go @@ -26,7 +26,7 @@ const ( type SessionService interface { List(ctx context.Context, filter sessionsvc.ListFilter) ([]domain.Session, error) Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Session, error) - SpawnOrchestrator(ctx context.Context, projectID domain.ProjectID, clean bool) (domain.Session, error) + SpawnOrchestrator(ctx context.Context, projectID domain.ProjectID, clean bool, harness domain.AgentHarness) (domain.Session, error) Get(ctx context.Context, id domain.SessionID) (domain.Session, error) Restore(ctx context.Context, id domain.SessionID) (domain.Session, error) Kill(ctx context.Context, id domain.SessionID) (bool, error) @@ -328,7 +328,7 @@ func (c *SessionsController) spawnOrchestrator(w http.ResponseWriter, r *http.Re envelope.WriteAPIError(w, r, http.StatusBadRequest, "bad_request", "PROJECT_ID_REQUIRED", "projectId is required", nil) return } - sess, err := c.Svc.SpawnOrchestrator(r.Context(), in.ProjectID, in.Clean) + sess, err := c.Svc.SpawnOrchestrator(r.Context(), in.ProjectID, in.Clean, in.Harness) if err != nil { envelope.WriteError(w, r, err) return diff --git a/backend/internal/httpd/controllers/sessions_test.go b/backend/internal/httpd/controllers/sessions_test.go index 3bc53b60..622a570d 100644 --- a/backend/internal/httpd/controllers/sessions_test.go +++ b/backend/internal/httpd/controllers/sessions_test.go @@ -57,7 +57,7 @@ func (f *fakeSessionService) Spawn(_ context.Context, cfg ports.SpawnConfig) (do return s, nil } -func (f *fakeSessionService) SpawnOrchestrator(ctx context.Context, projectID domain.ProjectID, clean bool) (domain.Session, error) { +func (f *fakeSessionService) SpawnOrchestrator(ctx context.Context, projectID domain.ProjectID, clean bool, harness domain.AgentHarness) (domain.Session, error) { if clean { active := true existing, err := f.List(ctx, sessionsvc.ListFilter{ProjectID: projectID, Active: &active, OrchestratorOnly: true}) @@ -70,7 +70,7 @@ func (f *fakeSessionService) SpawnOrchestrator(ctx context.Context, projectID do } } } - return f.Spawn(ctx, ports.SpawnConfig{ProjectID: projectID, Kind: domain.KindOrchestrator}) + return f.Spawn(ctx, ports.SpawnConfig{ProjectID: projectID, Kind: domain.KindOrchestrator, Harness: harness}) } func (f *fakeSessionService) Get(_ context.Context, id domain.SessionID) (domain.Session, error) { diff --git a/backend/internal/httpd/controllers/settings.go b/backend/internal/httpd/controllers/settings.go new file mode 100644 index 00000000..a79c17f0 --- /dev/null +++ b/backend/internal/httpd/controllers/settings.go @@ -0,0 +1,67 @@ +package controllers + +import ( + "context" + "net/http" + + "github.com/go-chi/chi/v5" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd/apispec" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd/envelope" +) + +// SettingsService is the controller-facing app settings contract. +type SettingsService interface { + GetAgentDefaults(ctx context.Context) (domain.AgentDefaults, error) + SetAgentDefaults(ctx context.Context, defaults domain.AgentDefaults) (domain.AgentDefaults, error) +} + +// SettingsController owns app-wide user settings routes. Nil keeps routes +// registered but returns OpenAPI-backed 501s. +type SettingsController struct { + Svc SettingsService +} + +// Register mounts the settings routes on the supplied router. +func (c *SettingsController) Register(r chi.Router) { + r.Get("/settings/agents", c.getAgentDefaults) + r.Put("/settings/agents", c.setAgentDefaults) +} + +func (c *SettingsController) getAgentDefaults(w http.ResponseWriter, r *http.Request) { + if c.Svc == nil { + apispec.NotImplemented(w, r, "GET", "/api/v1/settings/agents") + return + } + defaults, err := c.Svc.GetAgentDefaults(r.Context()) + if err != nil { + envelope.WriteError(w, r, err) + return + } + envelope.WriteJSON(w, http.StatusOK, AgentDefaultsResponse{ + AgentDefaults: defaults, + Configured: defaults.Complete(), + }) +} + +func (c *SettingsController) setAgentDefaults(w http.ResponseWriter, r *http.Request) { + if c.Svc == nil { + apispec.NotImplemented(w, r, "PUT", "/api/v1/settings/agents") + return + } + var in AgentDefaultsRequest + if err := decodeJSONStrict(r, &in); err != nil { + envelope.WriteAPIError(w, r, http.StatusBadRequest, "bad_request", "INVALID_JSON", "Invalid JSON body", nil) + return + } + defaults, err := c.Svc.SetAgentDefaults(r.Context(), in.AgentDefaults) + if err != nil { + envelope.WriteError(w, r, err) + return + } + envelope.WriteJSON(w, http.StatusOK, AgentDefaultsResponse{ + AgentDefaults: defaults, + Configured: defaults.Complete(), + }) +} diff --git a/backend/internal/httpd/controllers/settings_test.go b/backend/internal/httpd/controllers/settings_test.go new file mode 100644 index 00000000..efa4dd5a --- /dev/null +++ b/backend/internal/httpd/controllers/settings_test.go @@ -0,0 +1,80 @@ +package controllers_test + +import ( + "context" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + + "github.com/aoagents/agent-orchestrator/backend/internal/config" + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd/apierr" +) + +type fakeSettingsService struct { + defaults domain.AgentDefaults + err error + saved domain.AgentDefaults +} + +func (f *fakeSettingsService) GetAgentDefaults(context.Context) (domain.AgentDefaults, error) { + return f.defaults, f.err +} + +func (f *fakeSettingsService) SetAgentDefaults(_ context.Context, defaults domain.AgentDefaults) (domain.AgentDefaults, error) { + if f.err != nil { + return domain.AgentDefaults{}, f.err + } + f.saved = defaults + f.defaults = defaults + return defaults, nil +} + +func newSettingsTestServer(t *testing.T, svc *fakeSettingsService) *httptest.Server { + t.Helper() + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + srv := httptest.NewServer(httpd.NewRouterWithControl(config.Config{}, log, nil, httpd.APIDeps{Settings: svc}, httpd.ControlDeps{})) + t.Cleanup(srv.Close) + return srv +} + +func TestSettingsAPI_AgentDefaultsRoundTrip(t *testing.T) { + svc := &fakeSettingsService{} + srv := newSettingsTestServer(t, svc) + + body, status, _ := doRequest(t, srv, "GET", "/api/v1/settings/agents", "") + if status != http.StatusOK { + t.Fatalf("GET settings = %d, want 200; body=%s", status, body) + } + var got struct { + DefaultWorkerAgent string `json:"defaultWorkerAgent"` + DefaultOrchestratorAgent string `json:"defaultOrchestratorAgent"` + Configured bool `json:"configured"` + } + mustJSON(t, body, &got) + if got.Configured || got.DefaultWorkerAgent != "" || got.DefaultOrchestratorAgent != "" { + t.Fatalf("first-run settings = %#v, want empty incomplete defaults", got) + } + + body, status, _ = doRequest(t, srv, "PUT", "/api/v1/settings/agents", `{"defaultWorkerAgent":"codex","defaultOrchestratorAgent":"claude-code"}`) + if status != http.StatusOK { + t.Fatalf("PUT settings = %d, want 200; body=%s", status, body) + } + mustJSON(t, body, &got) + if !got.Configured || got.DefaultWorkerAgent != "codex" || got.DefaultOrchestratorAgent != "claude-code" { + t.Fatalf("saved settings = %#v", got) + } + if svc.saved.DefaultWorkerAgent != domain.HarnessCodex || svc.saved.DefaultOrchestratorAgent != domain.HarnessClaudeCode { + t.Fatalf("service saved = %+v", svc.saved) + } +} + +func TestSettingsAPI_ValidationError(t *testing.T) { + srv := newSettingsTestServer(t, &fakeSettingsService{err: apierr.Invalid("INVALID_AGENT_DEFAULTS", "defaultOrchestratorAgent is required", nil)}) + + body, status, _ := doRequest(t, srv, "PUT", "/api/v1/settings/agents", `{"defaultWorkerAgent":"codex"}`) + assertErrorCode(t, body, status, http.StatusBadRequest, "INVALID_AGENT_DEFAULTS") +} diff --git a/backend/internal/integration/lifecycle_sqlite_test.go b/backend/internal/integration/lifecycle_sqlite_test.go index 9a0a9960..7ff9b884 100644 --- a/backend/internal/integration/lifecycle_sqlite_test.go +++ b/backend/internal/integration/lifecycle_sqlite_test.go @@ -92,12 +92,18 @@ func newStack(t *testing.T) *stack { if err := store.UpsertProject(ctx, domain.ProjectRecord{ID: "mer", Path: "/repo/mer", RegisteredAt: time.Now()}); err != nil { t.Fatal(err) } + if err := store.SetAgentDefaults(ctx, domain.AgentDefaults{ + DefaultWorkerAgent: domain.HarnessClaudeCode, + DefaultOrchestratorAgent: domain.HarnessClaudeCode, + }); err != nil { + t.Fatal(err) + } msg := &captureMessenger{} lcm := lifecycle.New(store, msg) prm := prsvc.New(prsvc.Deps{Writer: store, Lifecycle: lcm}) rt := &stubRuntime{} ws := &stubWorkspace{} - mgr := sessionmanager.New(sessionmanager.Deps{Runtime: rt, Agents: stubAgents{}, Workspace: ws, Store: store, Messenger: msg, Lifecycle: lcm, LookPath: func(string) (string, error) { return "/usr/bin/true", nil }}) + mgr := sessionmanager.New(sessionmanager.Deps{Runtime: rt, Agents: stubAgents{}, Workspace: ws, Store: store, Messenger: msg, Lifecycle: lcm, LookPath: func(string) (string, error) { return "/usr/bin/true", nil }, AgentDefaults: store}) sm := sessionsvc.New(mgr, store) return &stack{store: store, sm: sm, lcm: lcm, prm: prm, rt: rt, ws: ws, msg: msg} } diff --git a/backend/internal/service/session/service.go b/backend/internal/service/session/service.go index 26a39fa0..26aaa1a8 100644 --- a/backend/internal/service/session/service.go +++ b/backend/internal/service/session/service.go @@ -158,7 +158,7 @@ func (s *Service) requireProject(ctx context.Context, id domain.ProjectID) error // true it first tears down any active orchestrator(s) for that project so the new // one is the only live coordinator — a business rule that belongs here, not in the // HTTP controller. -func (s *Service) SpawnOrchestrator(ctx context.Context, projectID domain.ProjectID, clean bool) (domain.Session, error) { +func (s *Service) SpawnOrchestrator(ctx context.Context, projectID domain.ProjectID, clean bool, harness domain.AgentHarness) (domain.Session, error) { if err := s.requireProject(ctx, projectID); err != nil { return domain.Session{}, err } @@ -174,7 +174,7 @@ func (s *Service) SpawnOrchestrator(ctx context.Context, projectID domain.Projec } } } - return s.Spawn(ctx, ports.SpawnConfig{ProjectID: projectID, Kind: domain.KindOrchestrator}) + return s.Spawn(ctx, ports.SpawnConfig{ProjectID: projectID, Kind: domain.KindOrchestrator, Harness: harness}) } // Restore relaunches a terminated session and returns the API-facing read model. @@ -341,6 +341,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.ErrDefaultAgentRequired): + return apierr.Invalid("DEFAULT_AGENT_REQUIRED", "Choose default agents in Settings before spawning sessions", 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 d9c2ec0b..c6567a26 100644 --- a/backend/internal/service/session/service_test.go +++ b/backend/internal/service/session/service_test.go @@ -140,11 +140,13 @@ type fakeCommander struct { killErr error cleanupErr error spawned bool + spawnedConfig ports.SpawnConfig killsAtSpawn int } func (f *fakeCommander) Spawn(_ context.Context, cfg ports.SpawnConfig) (domain.SessionRecord, error) { f.spawned = true + f.spawnedConfig = cfg f.killsAtSpawn = len(f.killed) return domain.SessionRecord{ID: "mer-9", ProjectID: cfg.ProjectID, Kind: cfg.Kind}, nil } @@ -237,7 +239,7 @@ func TestSpawnOrchestratorCleanKillsActiveOrchestratorsBeforeSpawn(t *testing.T) fc := &fakeCommander{} svc := &Service{manager: fc, store: st} - if _, err := svc.SpawnOrchestrator(context.Background(), "mer", true); err != nil { + if _, err := svc.SpawnOrchestrator(context.Background(), "mer", true, ""); err != nil { t.Fatalf("SpawnOrchestrator: %v", err) } @@ -249,6 +251,20 @@ func TestSpawnOrchestratorCleanKillsActiveOrchestratorsBeforeSpawn(t *testing.T) } } +func TestSpawnOrchestratorForwardsExplicitHarness(t *testing.T) { + st := newFakeStore() + st.projects["mer"] = domain.ProjectRecord{ID: "mer"} + fc := &fakeCommander{} + svc := &Service{manager: fc, store: st} + + if _, err := svc.SpawnOrchestrator(context.Background(), "mer", false, domain.HarnessCodex); err != nil { + t.Fatalf("SpawnOrchestrator: %v", err) + } + if fc.spawnedConfig.Kind != domain.KindOrchestrator || fc.spawnedConfig.Harness != domain.HarnessCodex { + t.Fatalf("spawn config = %+v, want orchestrator codex", fc.spawnedConfig) + } +} + // TestSpawnUnknownProjectReturns404 covers Bug 1: an HTTP spawn for an // unregistered projectId must surface PROJECT_NOT_FOUND (apierr.NotFound) // BEFORE any session row is created, so no orphan terminated row is left @@ -275,7 +291,7 @@ func TestSpawnOrchestratorUnknownProjectReturns404(t *testing.T) { fc := &fakeCommander{} svc := &Service{manager: fc, store: st} - _, err := svc.SpawnOrchestrator(context.Background(), "ghost", false) + _, err := svc.SpawnOrchestrator(context.Background(), "ghost", false, "") var e *apierr.Error if !errors.As(err, &e) || e.Kind != apierr.KindNotFound || e.Code != "PROJECT_NOT_FOUND" { t.Fatalf("err = %v, want apierr.NotFound PROJECT_NOT_FOUND", err) @@ -300,6 +316,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"}, + {"default agent required", fmt.Errorf("spawn: %w", sessionmanager.ErrDefaultAgentRequired), apierr.KindInvalid, "DEFAULT_AGENT_REQUIRED"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -320,7 +337,7 @@ func TestSpawnOrchestratorNoCleanSkipsKills(t *testing.T) { fc := &fakeCommander{} svc := &Service{manager: fc, store: st} - if _, err := svc.SpawnOrchestrator(context.Background(), "mer", false); err != nil { + if _, err := svc.SpawnOrchestrator(context.Background(), "mer", false, ""); err != nil { t.Fatalf("SpawnOrchestrator: %v", err) } if len(fc.killed) != 0 || !fc.spawned { diff --git a/backend/internal/service/settings/service.go b/backend/internal/service/settings/service.go new file mode 100644 index 00000000..925ff521 --- /dev/null +++ b/backend/internal/service/settings/service.go @@ -0,0 +1,46 @@ +package settings + +import ( + "context" + "fmt" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd/apierr" +) + +// Store is the persistence surface for app-wide user settings. +type Store interface { + GetAgentDefaults(ctx context.Context) (domain.AgentDefaults, bool, error) + SetAgentDefaults(ctx context.Context, defaults domain.AgentDefaults) error +} + +// Service owns validation and persistence for app-wide user settings. +type Service struct { + store Store +} + +// New wires a settings service over a store. +func New(store Store) *Service { + return &Service{store: store} +} + +// GetAgentDefaults returns configured defaults. Missing settings return the +// zero value so callers can distinguish first-run setup by Complete(). +func (s *Service) GetAgentDefaults(ctx context.Context) (domain.AgentDefaults, error) { + defaults, _, err := s.store.GetAgentDefaults(ctx) + if err != nil { + return domain.AgentDefaults{}, fmt.Errorf("get agent defaults: %w", err) + } + return defaults, nil +} + +// SetAgentDefaults validates and persists app-wide agent defaults. +func (s *Service) SetAgentDefaults(ctx context.Context, defaults domain.AgentDefaults) (domain.AgentDefaults, error) { + if err := defaults.ValidateComplete(); err != nil { + return domain.AgentDefaults{}, apierr.Invalid("INVALID_AGENT_DEFAULTS", err.Error(), nil) + } + if err := s.store.SetAgentDefaults(ctx, defaults); err != nil { + return domain.AgentDefaults{}, fmt.Errorf("set agent defaults: %w", err) + } + return defaults, nil +} diff --git a/backend/internal/service/settings/service_test.go b/backend/internal/service/settings/service_test.go new file mode 100644 index 00000000..eb83d3e7 --- /dev/null +++ b/backend/internal/service/settings/service_test.go @@ -0,0 +1,59 @@ +package settings + +import ( + "context" + "errors" + "testing" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd/apierr" +) + +type fakeStore struct { + defaults domain.AgentDefaults + ok bool + err error + saved domain.AgentDefaults +} + +func (f *fakeStore) GetAgentDefaults(context.Context) (domain.AgentDefaults, bool, error) { + return f.defaults, f.ok, f.err +} + +func (f *fakeStore) SetAgentDefaults(_ context.Context, defaults domain.AgentDefaults) error { + f.saved = defaults + return f.err +} + +func TestGetAgentDefaultsReturnsZeroWhenUnset(t *testing.T) { + got, err := New(&fakeStore{}).GetAgentDefaults(context.Background()) + if err != nil { + t.Fatalf("GetAgentDefaults: %v", err) + } + if got.Complete() { + t.Fatalf("defaults = %+v, want first-run incomplete defaults", got) + } +} + +func TestSetAgentDefaultsValidatesAndPersists(t *testing.T) { + store := &fakeStore{} + defaults := domain.AgentDefaults{ + DefaultWorkerAgent: domain.HarnessCodex, + DefaultOrchestratorAgent: domain.HarnessClaudeCode, + } + got, err := New(store).SetAgentDefaults(context.Background(), defaults) + if err != nil { + t.Fatalf("SetAgentDefaults: %v", err) + } + if got != defaults || store.saved != defaults { + t.Fatalf("got=%+v saved=%+v, want %+v", got, store.saved, defaults) + } +} + +func TestSetAgentDefaultsRejectsMissingValues(t *testing.T) { + _, err := New(&fakeStore{}).SetAgentDefaults(context.Background(), domain.AgentDefaults{DefaultWorkerAgent: domain.HarnessCodex}) + var apiErr *apierr.Error + if !errors.As(err, &apiErr) || apiErr.Kind != apierr.KindInvalid || apiErr.Code != "INVALID_AGENT_DEFAULTS" { + t.Fatalf("err = %v, want INVALID_AGENT_DEFAULTS", err) + } +} diff --git a/backend/internal/session_manager/manager.go b/backend/internal/session_manager/manager.go index 0e33d9b5..7ab1bff5 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") + // ErrDefaultAgentRequired means a spawn named no harness, the project had no + // role override, and the app-wide default for that role has not been set. + ErrDefaultAgentRequired = errors.New("session: default agent required") ) // Env vars a spawned process reads to learn who it is. @@ -76,6 +79,12 @@ type Store interface { DeleteSession(ctx context.Context, id domain.SessionID) (bool, error) } +// AgentDefaults is the app-wide settings surface needed to resolve a spawn that +// does not carry an explicit or project-level harness. +type AgentDefaults interface { + GetAgentDefaults(ctx context.Context) (domain.AgentDefaults, bool, error) +} + // Manager coordinates internal session spawn, restore, kill, and cleanup over // the outbound ports. User-facing read-model assembly lives in the service package. type Manager struct { @@ -86,11 +95,10 @@ 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 + // agentDefaults supplies app-wide defaults when a spawn names no harness and + // the project has no role override. + agentDefaults AgentDefaults + 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 +121,10 @@ 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 + // AgentDefaults stores app-wide defaults used when a spawn names no harness + // and the project has no role override. + AgentDefaults AgentDefaults + 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 +142,18 @@ 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, + agentDefaults: d.AgentDefaults, + 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 @@ -167,6 +173,27 @@ func New(d Deps) *Manager { return m } +func (m *Manager) defaultHarnessForKind(ctx context.Context, kind domain.SessionKind) (domain.AgentHarness, error) { + if m.agentDefaults == nil { + return "", ErrDefaultAgentRequired + } + defaults, ok, err := m.agentDefaults.GetAgentDefaults(ctx) + if err != nil { + return "", fmt.Errorf("agent defaults: %w", err) + } + if !ok { + return "", ErrDefaultAgentRequired + } + harness := defaults.HarnessFor(kind) + if harness == "" { + return "", ErrDefaultAgentRequired + } + if !harness.IsKnown() { + return "", fmt.Errorf("%w: %q", ErrUnknownHarness, harness) + } + return harness, nil +} + // Spawn creates the session row (which assigns the "{project}-{n}" id), then the // workspace and runtime, then reports completion to the LCM. If workspace // materialization fails the still-seed row is deleted outright; a later failure @@ -179,12 +206,12 @@ 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 + harness, err := m.defaultHarnessForKind(ctx, cfg.Kind) + if err != nil { + return domain.SessionRecord{}, fmt.Errorf("spawn: %w", err) + } + cfg.Harness = harness } // Reject an unknown harness before any durable state is created. Doing this @@ -308,7 +335,7 @@ 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. +// it stays empty so the app-wide default can be read from settings. func effectiveHarness(explicit domain.AgentHarness, kind domain.SessionKind, cfg domain.ProjectConfig) domain.AgentHarness { if explicit != "" { return explicit diff --git a/backend/internal/session_manager/manager_test.go b/backend/internal/session_manager/manager_test.go index d2474a2b..87ff33ed 100644 --- a/backend/internal/session_manager/manager_test.go +++ b/backend/internal/session_manager/manager_test.go @@ -85,6 +85,23 @@ func (f *fakeStore) GetDisplayPRFactsForSession(_ context.Context, id domain.Ses return domain.PRFacts{}, false, nil } +type fakeAgentDefaults struct { + defaults domain.AgentDefaults + ok bool + err error +} + +func (f fakeAgentDefaults) GetAgentDefaults(context.Context) (domain.AgentDefaults, bool, error) { + return f.defaults, f.ok, f.err +} + +func testAgentDefaults() fakeAgentDefaults { + return fakeAgentDefaults{ok: true, defaults: domain.AgentDefaults{ + DefaultWorkerAgent: domain.HarnessClaudeCode, + DefaultOrchestratorAgent: domain.HarnessClaudeCode, + }} +} + type fakeLCM struct { store *fakeStore completed int @@ -228,7 +245,11 @@ func newManager() (*Manager, *fakeStore, *fakeRuntime, *fakeWorkspace) { // Stub lookPath so the pre-launch agent-binary check passes; the fakeAgent // returns argv ["launch"] which is not a real binary on PATH. lookPath := func(string) (string, error) { return "/bin/true", nil } - m := New(Deps{Runtime: rt, Agents: fakeAgents{}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath}) + m := New(Deps{ + Runtime: rt, Agents: fakeAgents{}, Workspace: ws, Store: st, + Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath, + AgentDefaults: testAgentDefaults(), + }) return m, st, rt, ws } func seedTerminal(st *fakeStore, id domain.SessionID, meta domain.SessionMetadata) { @@ -251,7 +272,7 @@ func TestSpawn_ResolvesProjectConfig(t *testing.T) { rt := &fakeRuntime{} ws := &fakeWorkspace{} lookPath := func(string) (string, error) { return "/bin/true", nil } - m := New(Deps{Runtime: rt, Agents: singleAgent{agent: agent}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath}) + m := New(Deps{Runtime: rt, Agents: singleAgent{agent: agent}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath, AgentDefaults: testAgentDefaults()}) rec, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker}) if err != nil { @@ -284,24 +305,27 @@ 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) { +// TestSpawn_PersistsResolvedAppDefaultHarness locks the default resolution +// order: a spawn that names no harness persists the stored app default, while +// an explicit harness wins. +func TestSpawn_PersistsResolvedAppDefaultHarness(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 }, + AgentDefaults: fakeAgentDefaults{ok: true, defaults: domain.AgentDefaults{ + DefaultWorkerAgent: domain.HarnessCodex, + DefaultOrchestratorAgent: domain.HarnessClaudeCode, + }}, }) if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker}); err != nil { t.Fatal(err) } - if got := st.sessions["mer-1"].Harness; got != domain.HarnessClaudeCode { - t.Fatalf("unspecified harness = %q, want resolved default %q", got, domain.HarnessClaudeCode) + if got := st.sessions["mer-1"].Harness; got != domain.HarnessCodex { + t.Fatalf("unspecified harness = %q, want resolved default %q", got, domain.HarnessCodex) } if _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker, Harness: domain.HarnessCodex}); err != nil { @@ -312,6 +336,24 @@ func TestSpawn_PersistsResolvedDefaultHarness(t *testing.T) { } } +func TestSpawn_UnspecifiedHarnessRequiresConfiguredDefault(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 }, + }) + + _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker}) + if !errors.Is(err, ErrDefaultAgentRequired) { + t.Fatalf("err = %v, want ErrDefaultAgentRequired", err) + } + if len(st.sessions) != 0 { + t.Fatalf("spawn without defaults must not create a session row: %+v", st.sessions) + } +} + func TestSpawn_AssignsIDAndGoesIdle(t *testing.T) { m, st, rt, _ := newManager() s, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker, Prompt: "do it"}) @@ -471,7 +513,7 @@ func TestRestore_AppliesProjectAgentConfig(t *testing.T) { seedTerminal(st, "mer-1", domain.SessionMetadata{WorkspacePath: "/ws/mer-1", Branch: "b", AgentSessionID: "agent-x"}) agent := &recordingAgent{} lookPath := func(string) (string, error) { return "/bin/true", nil } - m := New(Deps{Runtime: &fakeRuntime{}, Agents: singleAgent{agent: agent}, Workspace: &fakeWorkspace{}, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath}) + m := New(Deps{Runtime: &fakeRuntime{}, Agents: singleAgent{agent: agent}, Workspace: &fakeWorkspace{}, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath, AgentDefaults: testAgentDefaults()}) if _, err := m.Restore(ctx, "mer-1"); err != nil { t.Fatal(err) @@ -565,7 +607,7 @@ func TestSpawn_ForwardsResolvedAgentConfigPermissions(t *testing.T) { }} agent := &recordingAgent{} lookPath := func(string) (string, error) { return "/bin/true", nil } - m := New(Deps{Runtime: &fakeRuntime{}, Agents: singleAgent{agent: agent}, Workspace: &fakeWorkspace{}, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath}) + m := New(Deps{Runtime: &fakeRuntime{}, Agents: singleAgent{agent: agent}, Workspace: &fakeWorkspace{}, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath, AgentDefaults: testAgentDefaults()}) _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker}) if err != nil { @@ -616,7 +658,7 @@ func TestSpawnWorker_AppendsActiveOrchestratorContact(t *testing.T) { rt := &fakeRuntime{} ws := &fakeWorkspace{} lookPath := func(string) (string, error) { return "/bin/true", nil } - m := New(Deps{Runtime: rt, Agents: singleAgent{agent: agent}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath}) + m := New(Deps{Runtime: rt, Agents: singleAgent{agent: agent}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath, AgentDefaults: testAgentDefaults()}) s, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker, Prompt: "do it"}) if err != nil { @@ -652,7 +694,7 @@ func TestSpawnWorker_SkipsTerminatedOrchestratorContact(t *testing.T) { rt := &fakeRuntime{} ws := &fakeWorkspace{} lookPath := func(string) (string, error) { return "/bin/true", nil } - m := New(Deps{Runtime: rt, Agents: singleAgent{agent: agent}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath}) + m := New(Deps{Runtime: rt, Agents: singleAgent{agent: agent}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath, AgentDefaults: testAgentDefaults()}) _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker, Prompt: "do it"}) if err != nil { @@ -670,7 +712,7 @@ func TestSpawnOrchestrator_UsesCoordinatorPrompt(t *testing.T) { rt := &fakeRuntime{} ws := &fakeWorkspace{} lookPath := func(string) (string, error) { return "/bin/true", nil } - m := New(Deps{Runtime: rt, Agents: singleAgent{agent: agent}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath}) + m := New(Deps{Runtime: rt, Agents: singleAgent{agent: agent}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath, AgentDefaults: testAgentDefaults()}) _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindOrchestrator}) if err != nil { @@ -843,7 +885,7 @@ func TestSpawn_RejectsMissingAgentBinary(t *testing.T) { notFound := func(name string) (string, error) { return "", fmt.Errorf("exec: %q: not found", name) } - m := New(Deps{Runtime: rt, Agents: fakeAgents{}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: notFound}) + m := New(Deps{Runtime: rt, Agents: fakeAgents{}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: notFound, AgentDefaults: testAgentDefaults()}) _, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker}) if !errors.Is(err, ports.ErrAgentBinaryNotFound) { @@ -894,7 +936,8 @@ func pathPinManager(executable func() (string, error)) (*Manager, *fakeStore, *f Runtime: rt, Agents: fakeAgents{}, Workspace: &fakeWorkspace{}, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: lookPath, Executable: executable, - Logger: slog.New(slog.NewTextHandler(logBuf, nil)), + AgentDefaults: testAgentDefaults(), + Logger: slog.New(slog.NewTextHandler(logBuf, nil)), }) return m, st, rt, logBuf } diff --git a/backend/internal/storage/sqlite/gen/models.go b/backend/internal/storage/sqlite/gen/models.go index 589bfed0..21fd48ed 100644 --- a/backend/internal/storage/sqlite/gen/models.go +++ b/backend/internal/storage/sqlite/gen/models.go @@ -12,6 +12,12 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/domain" ) +type AppSetting struct { + ID int64 + DefaultWorkerAgent domain.AgentHarness + DefaultOrchestratorAgent domain.AgentHarness +} + type ChangeLog struct { Seq int64 ProjectID domain.ProjectID diff --git a/backend/internal/storage/sqlite/gen/settings.sql.go b/backend/internal/storage/sqlite/gen/settings.sql.go new file mode 100644 index 00000000..c64130d1 --- /dev/null +++ b/backend/internal/storage/sqlite/gen/settings.sql.go @@ -0,0 +1,48 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: settings.sql + +package gen + +import ( + "context" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +const getAgentDefaults = `-- name: GetAgentDefaults :one +SELECT default_worker_agent, default_orchestrator_agent +FROM app_settings +WHERE id = 1 +` + +type GetAgentDefaultsRow struct { + DefaultWorkerAgent domain.AgentHarness + DefaultOrchestratorAgent domain.AgentHarness +} + +func (q *Queries) GetAgentDefaults(ctx context.Context) (GetAgentDefaultsRow, error) { + row := q.db.QueryRowContext(ctx, getAgentDefaults) + var i GetAgentDefaultsRow + err := row.Scan(&i.DefaultWorkerAgent, &i.DefaultOrchestratorAgent) + return i, err +} + +const upsertAgentDefaults = `-- name: UpsertAgentDefaults :exec +INSERT INTO app_settings (id, default_worker_agent, default_orchestrator_agent) +VALUES (1, ?, ?) +ON CONFLICT(id) DO UPDATE SET + default_worker_agent = excluded.default_worker_agent, + default_orchestrator_agent = excluded.default_orchestrator_agent +` + +type UpsertAgentDefaultsParams struct { + DefaultWorkerAgent domain.AgentHarness + DefaultOrchestratorAgent domain.AgentHarness +} + +func (q *Queries) UpsertAgentDefaults(ctx context.Context, arg UpsertAgentDefaultsParams) error { + _, err := q.db.ExecContext(ctx, upsertAgentDefaults, arg.DefaultWorkerAgent, arg.DefaultOrchestratorAgent) + return err +} diff --git a/backend/internal/storage/sqlite/migrations/0014_app_settings.sql b/backend/internal/storage/sqlite/migrations/0014_app_settings.sql new file mode 100644 index 00000000..d60d3c55 --- /dev/null +++ b/backend/internal/storage/sqlite/migrations/0014_app_settings.sql @@ -0,0 +1,17 @@ +-- +goose Up +-- +goose StatementBegin + +CREATE TABLE app_settings ( + id INTEGER PRIMARY KEY CHECK (id = 1), + default_worker_agent TEXT NOT NULL DEFAULT '' + CHECK (default_worker_agent IN ('', '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_orchestrator_agent TEXT NOT NULL DEFAULT '' + CHECK (default_orchestrator_agent IN ('', 'claude-code', 'codex', 'aider', 'opencode', 'grok', 'droid', 'amp', 'agy', 'crush', 'cursor', 'qwen', 'copilot', 'goose', 'auggie', 'continue', 'devin', 'cline', 'kimi', 'kiro', 'kilocode', 'vibe', 'pi', 'autohand')) +); + +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +DROP TABLE app_settings; +-- +goose StatementEnd diff --git a/backend/internal/storage/sqlite/queries/settings.sql b/backend/internal/storage/sqlite/queries/settings.sql new file mode 100644 index 00000000..2b0de0c7 --- /dev/null +++ b/backend/internal/storage/sqlite/queries/settings.sql @@ -0,0 +1,11 @@ +-- name: GetAgentDefaults :one +SELECT default_worker_agent, default_orchestrator_agent +FROM app_settings +WHERE id = 1; + +-- name: UpsertAgentDefaults :exec +INSERT INTO app_settings (id, default_worker_agent, default_orchestrator_agent) +VALUES (1, ?, ?) +ON CONFLICT(id) DO UPDATE SET + default_worker_agent = excluded.default_worker_agent, + default_orchestrator_agent = excluded.default_orchestrator_agent; diff --git a/backend/internal/storage/sqlite/store/settings_store.go b/backend/internal/storage/sqlite/store/settings_store.go new file mode 100644 index 00000000..d77752f9 --- /dev/null +++ b/backend/internal/storage/sqlite/store/settings_store.go @@ -0,0 +1,40 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "fmt" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite/gen" +) + +// GetAgentDefaults returns the app-wide spawn defaults. ok=false means the +// user has not configured them yet. +func (s *Store) GetAgentDefaults(ctx context.Context) (domain.AgentDefaults, bool, error) { + row, err := s.qr.GetAgentDefaults(ctx) + if errors.Is(err, sql.ErrNoRows) { + return domain.AgentDefaults{}, false, nil + } + if err != nil { + return domain.AgentDefaults{}, false, fmt.Errorf("get agent defaults: %w", err) + } + return domain.AgentDefaults{ + DefaultWorkerAgent: row.DefaultWorkerAgent, + DefaultOrchestratorAgent: row.DefaultOrchestratorAgent, + }, true, nil +} + +// SetAgentDefaults replaces the app-wide spawn defaults. +func (s *Store) SetAgentDefaults(ctx context.Context, defaults domain.AgentDefaults) error { + s.writeMu.Lock() + defer s.writeMu.Unlock() + if err := s.qw.UpsertAgentDefaults(ctx, gen.UpsertAgentDefaultsParams{ + DefaultWorkerAgent: defaults.DefaultWorkerAgent, + DefaultOrchestratorAgent: defaults.DefaultOrchestratorAgent, + }); err != nil { + return fmt.Errorf("set agent defaults: %w", err) + } + return nil +} diff --git a/backend/internal/storage/sqlite/store/settings_store_test.go b/backend/internal/storage/sqlite/store/settings_store_test.go new file mode 100644 index 00000000..e93a0b59 --- /dev/null +++ b/backend/internal/storage/sqlite/store/settings_store_test.go @@ -0,0 +1,37 @@ +package store_test + +import ( + "context" + "testing" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite" +) + +func TestAgentDefaultsRoundTrip(t *testing.T) { + st, err := sqlite.Open(t.TempDir()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = st.Close() }) + + ctx := context.Background() + if got, ok, err := st.GetAgentDefaults(ctx); err != nil || ok || got.Complete() { + t.Fatalf("initial defaults got=%+v ok=%v err=%v, want unset", got, ok, err) + } + + want := domain.AgentDefaults{ + DefaultWorkerAgent: domain.HarnessCodex, + DefaultOrchestratorAgent: domain.HarnessClaudeCode, + } + if err := st.SetAgentDefaults(ctx, want); err != nil { + t.Fatalf("SetAgentDefaults: %v", err) + } + got, ok, err := st.GetAgentDefaults(ctx) + if err != nil { + t.Fatalf("GetAgentDefaults: %v", err) + } + if !ok || got != want { + t.Fatalf("defaults got=%+v ok=%v, want %+v", got, ok, want) + } +} diff --git a/backend/internal/terminal/attachment.go b/backend/internal/terminal/attachment.go index 682f05eb..200917be 100644 --- a/backend/internal/terminal/attachment.go +++ b/backend/internal/terminal/attachment.go @@ -157,6 +157,9 @@ func (a *attachment) run(ctx context.Context) { start := time.Now() a.copyOut(p) _ = p.Close() + if a.isClosed() || ctx.Err() != nil { + return + } if time.Since(start) >= a.resetGrace { failures = 0 @@ -170,6 +173,9 @@ func (a *attachment) run(ctx context.Context) { if !a.backoff(ctx, failures) { return } + if a.isClosed() || ctx.Err() != nil { + return + } a.log.Debug("terminal re-attaching", "id", a.id, "failures", failures) } } diff --git a/backend/internal/terminal/attachment_test.go b/backend/internal/terminal/attachment_test.go index 476d03ac..2e1ca5f2 100644 --- a/backend/internal/terminal/attachment_test.go +++ b/backend/internal/terminal/attachment_test.go @@ -142,6 +142,31 @@ func TestAttachmentSkipsReattachOnCleanExit(t *testing.T) { } } +func TestAttachmentCloseStopsWithoutReattachBackoff(t *testing.T) { + src := &fakeSource{alive: true} + pty := newFakePTY() + sp := &fakeSpawner{ptys: []*fakePTY{pty}} + a := newTestAttachment(src, sp.spawn, nil, nil) + + done := make(chan struct{}) + go func() { + a.run(context.Background()) + close(done) + }() + eventually(t, time.Second, func() bool { return sp.calls() == 1 }) + + a.close() + + select { + case <-done: + case <-time.After(100 * time.Millisecond): + t.Fatal("attachment close waited for reattach backoff instead of stopping immediately") + } + if got := sp.calls(); got != 1 { + t.Fatalf("closed attachment reattached, got %d spawns", got) + } +} + // TestAttachmentNeverAttachesToDeadRuntime covers the resurrection bug: `zellij // attach` on a killed-but-cached session resurrects it, re-running the agent // command. An attachment whose runtime probes definitively dead must therefore diff --git a/backend/internal/terminal/manager.go b/backend/internal/terminal/manager.go index 7b22608b..3e75b7b1 100644 --- a/backend/internal/terminal/manager.go +++ b/backend/internal/terminal/manager.go @@ -225,6 +225,12 @@ func (c *connState) openTerminal(id string, rows, cols uint16) { var a *attachment a = newAttachment(id, ports.RuntimeHandle{ID: id}, c.mgr.src, c.mgr.spawn, func(data []byte) { + c.mu.Lock() + current := c.terms[id] + c.mu.Unlock() + if current != a { + return + } c.enqueue(serverMsg{ Ch: chTerminal, ID: id, @@ -366,6 +372,7 @@ func (c *connState) cleanup() { return } c.closed = true + c.cancel() attachments := make([]*attachment, 0, len(c.terms)) for _, a := range c.terms { attachments = append(attachments, a) diff --git a/backend/internal/terminal/manager_test.go b/backend/internal/terminal/manager_test.go index dc0730b8..26eb7ba6 100644 --- a/backend/internal/terminal/manager_test.go +++ b/backend/internal/terminal/manager_test.go @@ -112,6 +112,18 @@ func nextTerminal(t *testing.T, c *fakeConn) serverMsg { } } +func assertNoTerminalFrame(t *testing.T, c *fakeConn, typ string, d time.Duration) { + t.Helper() + select { + case m := <-c.out: + if m.Ch == chTerminal && m.Type == typ { + t.Fatalf("received unexpected terminal/%s frame", typ) + } + t.Fatalf("received unexpected frame %s/%s", m.Ch, m.Type) + case <-time.After(d): + } +} + // Opening a pane whose runtime is already dead must (1) send opened before // exited (the dead pane is reported, not errored) and (2) clear the conn's // entry, so a later open for the same id on this connection is still served @@ -170,6 +182,149 @@ func TestServeExitAfterOpenClearsEntryAllowingReopen(t *testing.T) { recv(t, conn, chTerminal, msgOpened, 2*time.Second) } +func TestServeSameSocketSwitchesTerminalByCloseThenOpen(t *testing.T) { + src := &fakeSource{alive: true} + p1, p2 := newFakePTY(), newFakePTY() + sp := &fakeSpawner{ptys: []*fakePTY{p1, p2}} + mgr := NewManager(src, nil, testLogger(), WithSpawn(sp.spawn), WithHeartbeat(0)) + defer mgr.Close() + + conn := newFakeConn() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go mgr.Serve(ctx, conn) + + conn.in <- clientMsg{Ch: chTerminal, ID: "t1", Type: msgOpen} + recv(t, conn, chTerminal, msgOpened, time.Second) + eventually(t, time.Second, func() bool { return sp.calls() == 1 }) + + conn.in <- clientMsg{Ch: chTerminal, ID: "t1", Type: msgClose} + eventually(t, time.Second, func() bool { + select { + case <-p1.closed: + return true + default: + return false + } + }) + assertNoTerminalFrame(t, conn, msgExited, 50*time.Millisecond) + + conn.in <- clientMsg{Ch: chTerminal, ID: "t2", Type: msgOpen} + msg := recv(t, conn, chTerminal, msgOpened, time.Second) + if msg.ID != "t2" { + t.Fatalf("opened id = %q, want t2", msg.ID) + } + eventually(t, time.Second, func() bool { return sp.calls() == 2 }) + select { + case <-p2.closed: + t.Fatal("opening t2 on the same socket must not immediately close t2") + default: + } +} + +func TestServeConnectionCleanupClosesAllOpenAttachments(t *testing.T) { + src := &fakeSource{alive: true} + p1, p2 := newFakePTY(), newFakePTY() + sp := &fakeSpawner{ptys: []*fakePTY{p1, p2}} + mgr := NewManager(src, nil, testLogger(), WithSpawn(sp.spawn), WithHeartbeat(0)) + defer mgr.Close() + + conn := newFakeConn() + ctx, cancel := context.WithCancel(context.Background()) + go mgr.Serve(ctx, conn) + + conn.in <- clientMsg{Ch: chTerminal, ID: "t1", Type: msgOpen} + recv(t, conn, chTerminal, msgOpened, time.Second) + conn.in <- clientMsg{Ch: chTerminal, ID: "t2", Type: msgOpen} + recv(t, conn, chTerminal, msgOpened, time.Second) + eventually(t, time.Second, func() bool { return sp.calls() == 2 }) + + cancel() + eventually(t, time.Second, func() bool { + select { + case <-p1.closed: + default: + return false + } + select { + case <-p2.closed: + return true + default: + return false + } + }) +} + +type latePTY struct { + out chan []byte + done chan struct{} + doneOnce sync.Once +} + +func newLatePTY() *latePTY { + return &latePTY{out: make(chan []byte, 4), done: make(chan struct{})} +} + +func (p *latePTY) push(b []byte) { p.out <- b } + +func (p *latePTY) finish() { p.doneOnce.Do(func() { close(p.done) }) } + +func (p *latePTY) Read(b []byte) (int, error) { + select { + case chunk := <-p.out: + return copy(b, chunk), nil + case <-p.done: + return 0, context.Canceled + } +} + +func (p *latePTY) Write(b []byte) (int, error) { return len(b), nil } + +func (p *latePTY) Resize(uint16, uint16) error { return nil } + +func (p *latePTY) Close() error { return nil } + +func TestServeDropsLateDataFromSupersededAttachment(t *testing.T) { + src := &fakeSource{alive: true} + oldPTY := newLatePTY() + newPTY := newFakePTY() + var mu sync.Mutex + ptys := []ptyProcess{oldPTY, newPTY} + calls := 0 + spawn := func(context.Context, []string, uint16, uint16) (ptyProcess, error) { + mu.Lock() + defer mu.Unlock() + p := ptys[calls] + calls++ + return p, nil + } + spawnCalls := func() int { + mu.Lock() + defer mu.Unlock() + return calls + } + mgr := NewManager(src, nil, testLogger(), WithSpawn(spawn), WithHeartbeat(0)) + defer mgr.Close() + defer oldPTY.finish() + + conn := newFakeConn() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go mgr.Serve(ctx, conn) + + conn.in <- clientMsg{Ch: chTerminal, ID: "t1", Type: msgOpen} + recv(t, conn, chTerminal, msgOpened, time.Second) + eventually(t, time.Second, func() bool { return spawnCalls() == 1 }) + + conn.in <- clientMsg{Ch: chTerminal, ID: "t1", Type: msgClose} + conn.in <- clientMsg{Ch: chTerminal, ID: "t1", Type: msgOpen} + recv(t, conn, chTerminal, msgOpened, time.Second) + eventually(t, time.Second, func() bool { return spawnCalls() == 2 }) + + oldPTY.push([]byte("stale output")) + assertNoTerminalFrame(t, conn, msgData, 50*time.Millisecond) +} + // An attachment that exits the moment it is opened (dead runtime) fires onExit // from its run goroutine, racing the reopen that follows the exited frame. The // identity-guarded delete in onExit must never evict a successor attachment @@ -278,6 +433,45 @@ func TestServeClosesConnOnReadEnd(t *testing.T) { } } +type cancelAwarePTY struct { + cancelled <-chan struct{} + sawCancel bool +} + +func (p *cancelAwarePTY) Read([]byte) (int, error) { return 0, context.Canceled } + +func (p *cancelAwarePTY) Write(b []byte) (int, error) { return len(b), nil } + +func (p *cancelAwarePTY) Resize(uint16, uint16) error { return nil } + +func (p *cancelAwarePTY) Close() error { + select { + case <-p.cancelled: + p.sawCancel = true + default: + } + return nil +} + +func TestCleanupCancelsConnectionBeforeClosingAttachments(t *testing.T) { + cancelled := make(chan struct{}) + pty := &cancelAwarePTY{cancelled: cancelled} + c := &connState{ + cancel: func() { close(cancelled) }, + conn: newFakeConn(), + out: make(chan serverMsg, defaultWriteBuffer), + terms: map[string]*attachment{ + "t1": {id: "t1", pty: pty}, + }, + } + + c.cleanup() + + if !pty.sawCancel { + t.Fatal("cleanup closed attachment PTY before cancelling the connection context") + } +} + // Each connection opening the same pane gets its OWN attach PTY — that is the // per-client model: zellij replays its init handshake + full repaint to every // fresh attach, so no client depends on bytes another client consumed. Output diff --git a/backend/sqlc.yaml b/backend/sqlc.yaml index 070b6916..bca92b99 100644 --- a/backend/sqlc.yaml +++ b/backend/sqlc.yaml @@ -80,6 +80,14 @@ sql: go_type: import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" type: "ProjectID" + - column: "app_settings.default_worker_agent" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "AgentHarness" + - column: "app_settings.default_orchestrator_agent" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "AgentHarness" - column: "session_worktrees.session_id" go_type: import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" diff --git a/frontend/src/api/schema.ts b/frontend/src/api/schema.ts index 0bb8bd4a..7383a752 100644 --- a/frontend/src/api/schema.ts +++ b/frontend/src/api/schema.ts @@ -400,6 +400,24 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/settings/agents": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** Get app-wide default agents */ + get: operations["getAgentDefaults"]; + /** Set app-wide default agents */ + put: operations["setAgentDefaults"]; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; } export type webhooks = Record; export interface components { @@ -424,6 +442,19 @@ export interface components { model?: string; permissions?: string; }; + AgentDefaultsRequest: { + /** @enum {string} */ + defaultOrchestratorAgent?: "claude-code" | "codex" | "aider" | "opencode" | "grok" | "droid" | "amp" | "agy" | "crush" | "cursor" | "qwen" | "copilot" | "goose" | "auggie" | "continue" | "devin" | "cline" | "kimi" | "kiro" | "kilocode" | "vibe" | "pi" | "autohand"; + /** @enum {string} */ + defaultWorkerAgent?: "claude-code" | "codex" | "aider" | "opencode" | "grok" | "droid" | "amp" | "agy" | "crush" | "cursor" | "qwen" | "copilot" | "goose" | "auggie" | "continue" | "devin" | "cline" | "kimi" | "kiro" | "kilocode" | "vibe" | "pi" | "autohand"; + }; + AgentDefaultsResponse: { + configured: boolean; + /** @enum {string} */ + defaultOrchestratorAgent?: "claude-code" | "codex" | "aider" | "opencode" | "grok" | "droid" | "amp" | "agy" | "crush" | "cursor" | "qwen" | "copilot" | "goose" | "auggie" | "continue" | "devin" | "cline" | "kimi" | "kiro" | "kilocode" | "vibe" | "pi" | "autohand"; + /** @enum {string} */ + defaultWorkerAgent?: "claude-code" | "codex" | "aider" | "opencode" | "grok" | "droid" | "amp" | "agy" | "crush" | "cursor" | "qwen" | "copilot" | "goose" | "auggie" | "continue" | "devin" | "cline" | "kimi" | "kiro" | "kilocode" | "vibe" | "pi" | "autohand"; + }; ClaimPRRequest: { allowTakeover?: null | boolean; pr: string; @@ -658,6 +689,8 @@ export interface components { }; SpawnOrchestratorRequest: { clean?: boolean; + /** @enum {string} */ + harness?: "claude-code" | "codex" | "aider" | "opencode" | "grok" | "droid" | "amp" | "agy" | "crush" | "cursor" | "qwen" | "copilot" | "goose" | "auggie" | "continue" | "devin" | "cline" | "kimi" | "kiro" | "kilocode" | "vibe" | "pi" | "autohand"; projectId: string; }; SpawnOrchestratorResponse: { @@ -2124,4 +2157,93 @@ export interface operations { }; }; }; + getAgentDefaults: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["AgentDefaultsResponse"]; + }; + }; + /** @description Internal Server Error */ + 500: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Not Implemented */ + 501: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + }; + }; + setAgentDefaults: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["AgentDefaultsRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["AgentDefaultsResponse"]; + }; + }; + /** @description Bad Request */ + 400: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Internal Server Error */ + 500: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Not Implemented */ + 501: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + }; + }; } diff --git a/frontend/src/renderer/components/AgentDefaultsDialog.test.tsx b/frontend/src/renderer/components/AgentDefaultsDialog.test.tsx new file mode 100644 index 00000000..7726669a --- /dev/null +++ b/frontend/src/renderer/components/AgentDefaultsDialog.test.tsx @@ -0,0 +1,94 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { getMock, putMock } = vi.hoisted(() => ({ + getMock: vi.fn(), + putMock: vi.fn(), +})); + +vi.mock("../lib/api-client", () => ({ + apiClient: { + GET: getMock, + PUT: putMock, + }, + apiErrorMessage: (error: unknown) => { + if (error instanceof Error) return error.message; + if (typeof error === "object" && error !== null && "message" in error) { + return String((error as { message: unknown }).message); + } + return "Request failed"; + }, +})); + +import { AgentDefaultsDialog } from "./AgentDefaultsDialog"; + +function renderDialog(open = false) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + const onOpenChange = vi.fn(); + render( + + + , + ); + return onOpenChange; +} + +async function chooseOption(trigger: HTMLElement, optionName: string) { + await userEvent.click(trigger); + await userEvent.click(await screen.findByRole("option", { name: optionName })); +} + +beforeEach(() => { + getMock.mockReset(); + putMock.mockReset(); +}); + +describe("AgentDefaultsDialog", () => { + it("opens on first run and saves selected defaults", async () => { + getMock.mockResolvedValue({ + data: { configured: false }, + error: undefined, + }); + putMock.mockResolvedValue({ + data: { + configured: true, + defaultWorkerAgent: "codex", + defaultOrchestratorAgent: "goose", + }, + error: undefined, + }); + const onOpenChange = renderDialog(false); + + expect(await screen.findByRole("dialog", { name: "Choose Default Agents" })).toBeInTheDocument(); + const save = screen.getByRole("button", { name: "Save defaults" }); + expect(save).toBeDisabled(); + + await chooseOption(screen.getByRole("combobox", { name: "Worker agent" }), "codex"); + await chooseOption(screen.getByRole("combobox", { name: "Orchestrator agent" }), "goose"); + await userEvent.click(save); + + await waitFor(() => expect(putMock).toHaveBeenCalledTimes(1)); + expect(putMock).toHaveBeenCalledWith("/api/v1/settings/agents", { + body: { defaultWorkerAgent: "codex", defaultOrchestratorAgent: "goose" }, + }); + expect(onOpenChange).toHaveBeenCalledWith(false); + }); + + it("stays hidden when configured and not explicitly opened", async () => { + getMock.mockResolvedValue({ + data: { + configured: true, + defaultWorkerAgent: "codex", + defaultOrchestratorAgent: "claude-code", + }, + error: undefined, + }); + renderDialog(false); + + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + }); +}); diff --git a/frontend/src/renderer/components/AgentDefaultsDialog.tsx b/frontend/src/renderer/components/AgentDefaultsDialog.tsx new file mode 100644 index 00000000..f2003211 --- /dev/null +++ b/frontend/src/renderer/components/AgentDefaultsDialog.tsx @@ -0,0 +1,160 @@ +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { Bot, X } from "lucide-react"; +import type { ReactNode } from "react"; +import { useEffect, useMemo, useState } from "react"; +import { agentDefaultsQueryKey, fetchAgentDefaults, saveAgentDefaults } from "../lib/agent-defaults"; +import { AGENT_OPTIONS, type AgentOption } from "../lib/agent-options"; +import { Button } from "./ui/button"; +import { Label } from "./ui/label"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "./ui/select"; + +const AGENT_SELECT_PLACEHOLDER = "__select_agent__"; + +type AgentDefaultsDialogProps = { + daemonReady: boolean; + open: boolean; + onOpenChange: (open: boolean) => void; +}; + +export function AgentDefaultsDialog({ daemonReady, open, onOpenChange }: AgentDefaultsDialogProps) { + const queryClient = useQueryClient(); + const query = useQuery({ + queryKey: agentDefaultsQueryKey, + queryFn: fetchAgentDefaults, + enabled: daemonReady, + }); + const firstRunRequired = + daemonReady && (query.isLoading || query.isError || (query.isSuccess && !query.data.configured)); + const visible = open || firstRunRequired; + const locked = firstRunRequired; + const [workerAgent, setWorkerAgent] = useState(""); + const [orchestratorAgent, setOrchestratorAgent] = useState(""); + + useEffect(() => { + if (!visible || !query.data) return; + setWorkerAgent(query.data.defaultWorkerAgent ?? ""); + setOrchestratorAgent(query.data.defaultOrchestratorAgent ?? ""); + }, [query.data, visible]); + + const canSave = daemonReady && workerAgent !== "" && orchestratorAgent !== ""; + const title = firstRunRequired ? "Choose Default Agents" : "Default Agents"; + const mutation = useMutation({ + mutationFn: () => + saveAgentDefaults({ + defaultWorkerAgent: workerAgent as AgentOption, + defaultOrchestratorAgent: orchestratorAgent as AgentOption, + }), + onSuccess: (defaults) => { + queryClient.setQueryData(agentDefaultsQueryKey, defaults); + onOpenChange(false); + }, + }); + + const statusText = useMemo(() => { + if (query.isLoading) return "Loading agent settings..."; + if (!daemonReady) return "Daemon is not ready."; + if (query.isError) return query.error instanceof Error ? query.error.message : "Could not load agent settings"; + if (mutation.isError) + return mutation.error instanceof Error ? mutation.error.message : "Could not save agent settings"; + return null; + }, [daemonReady, mutation.error, mutation.isError, query.error, query.isError, query.isLoading]); + + if (!visible) return null; + + const close = () => { + if (!locked) onOpenChange(false); + }; + + return ( +
+
{ + event.preventDefault(); + if (canSave) mutation.mutate(); + }} + role="dialog" + aria-modal="true" + > +
+
+
+
+

+ {title} +

+

+ {firstRunRequired ? "Required before spawning sessions." : "Used when a project has no role override."} +

+
+ {!locked && ( + + )} +
+ +
+ + + + + + +
+ +
+ + {statusText} + + +
+
+
+ ); +} + +function AgentSelect({ id, value, onChange }: { id: string; value: string; onChange: (value: string) => void }) { + return ( + + ); +} + +function Field({ label, htmlFor, children }: { label: string; htmlFor: string; children: ReactNode }) { + return ( +
+ + {children} +
+ ); +} diff --git a/frontend/src/renderer/components/ProjectSettingsForm.tsx b/frontend/src/renderer/components/ProjectSettingsForm.tsx index 673204e1..9e2dbef8 100644 --- a/frontend/src/renderer/components/ProjectSettingsForm.tsx +++ b/frontend/src/renderer/components/ProjectSettingsForm.tsx @@ -1,8 +1,9 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useState } from "react"; import type { components } from "../../api/schema"; -import { apiClient, apiErrorMessage } from "../lib/api-client"; import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; +import { apiClient, apiErrorMessage } from "../lib/api-client"; +import { AGENT_OPTIONS } from "../lib/agent-options"; 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" }, @@ -251,14 +249,14 @@ 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. + // "" sentinel → app default; Select can't hold an empty value, so map it. return (