From 7216026dfd1184e9c89f644742bebe1302bd79e4 Mon Sep 17 00:00:00 2001 From: codebanditssss Date: Sat, 13 Jun 2026 01:40:58 +0530 Subject: [PATCH] fix(spawn): reject unknown harness with 400 instead of opaque 500 An unknown --harness was only caught at the agent-registry lookup deep in Spawn, after the seed row and worktree were already created: the untyped error collapsed to INTERNAL_ERROR 500 and left a terminated orphan row. Validate the harness against the registry before any durable state is created and return a typed ErrUnknownHarness mapped to UNKNOWN_HARNESS (400). Sibling to #152 Bug 6 (unknown binary on PATH), which did not cover a harness with no registered adapter. Closes #210 --- backend/internal/service/session/service.go | 2 ++ .../internal/service/session/service_test.go | 1 + backend/internal/session_manager/manager.go | 11 ++++++++ .../internal/session_manager/manager_test.go | 28 +++++++++++++++++++ 4 files changed, 42 insertions(+) diff --git a/backend/internal/service/session/service.go b/backend/internal/service/session/service.go index 51e43396..12bcf436 100644 --- a/backend/internal/service/session/service.go +++ b/backend/internal/service/session/service.go @@ -318,6 +318,8 @@ func toAPIError(err error) error { return apierr.Conflict("SESSION_INCOMPLETE_HANDLE", "Session is missing runtime or workspace handles", nil) case errors.Is(err, sessionmanager.ErrProjectNotResolvable): 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, 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 9af3ceea..4367a374 100644 --- a/backend/internal/service/session/service_test.go +++ b/backend/internal/service/session/service_test.go @@ -245,6 +245,7 @@ func TestToAPIErrorMapsWorkspaceBranchSentinels(t *testing.T) { {"checked out elsewhere", fmt.Errorf("spawn mer-1: workspace: %w: \"x\" is checked out at \"/tmp\"", ports.ErrWorkspaceBranchCheckedOutElsewhere), apierr.KindConflict, "BRANCH_CHECKED_OUT_ELSEWHERE"}, {"not fetched", fmt.Errorf("spawn mer-1: workspace: %w: \"x\" has no local head", ports.ErrWorkspaceBranchNotFetched), apierr.KindInvalid, "BRANCH_NOT_FETCHED"}, {"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"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/backend/internal/session_manager/manager.go b/backend/internal/session_manager/manager.go index 9a563e96..483037d4 100644 --- a/backend/internal/session_manager/manager.go +++ b/backend/internal/session_manager/manager.go @@ -28,6 +28,10 @@ var ( // ErrProjectNotResolvable means the spawn's project has no usable repo // (unregistered, archived, or missing a path). The API maps it to a 400. ErrProjectNotResolvable = errors.New("session: project repo not resolvable") + // ErrUnknownHarness means the requested agent harness has no registered + // 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") ) // Env vars a spawned process reads to learn who it is. @@ -163,6 +167,13 @@ func (m *Manager) Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Sess // so a project can default workers to one agent and orchestrators to another. cfg.Harness = effectiveHarness(cfg.Harness, cfg.Kind, project.Config) + // Reject an unknown harness before any durable state is created. Doing this + // after CreateSession would leave a terminated orphan row and waste a + // worktree on a spawn that can never launch. + if _, ok := m.agents.Agent(cfg.Harness); !ok { + return domain.SessionRecord{}, fmt.Errorf("spawn: %w: %q", ErrUnknownHarness, cfg.Harness) + } + prompt, systemPrompt, err := m.buildSpawnTexts(ctx, cfg) if err != nil { return domain.SessionRecord{}, fmt.Errorf("spawn: prompt: %w", err) diff --git a/backend/internal/session_manager/manager_test.go b/backend/internal/session_manager/manager_test.go index cc7832e2..8631f9e9 100644 --- a/backend/internal/session_manager/manager_test.go +++ b/backend/internal/session_manager/manager_test.go @@ -175,6 +175,11 @@ type singleAgent struct{ agent ports.Agent } func (s singleAgent) Agent(domain.AgentHarness) (ports.Agent, bool) { return s.agent, true } +// missingAgents resolves no harness, simulating a typo'd or unregistered agent. +type missingAgents struct{} + +func (missingAgents) Agent(domain.AgentHarness) (ports.Agent, bool) { return nil, false } + type fakeWorkspace struct { createErr error destroyErr error @@ -729,6 +734,29 @@ func TestSpawn_RejectsMissingAgentBinary(t *testing.T) { } } +func TestSpawn_RejectsUnknownHarness(t *testing.T) { + st := newFakeStore() + rt := &fakeRuntime{} + ws := &fakeWorkspace{} + m := New(Deps{Runtime: rt, Agents: missingAgents{}, Workspace: ws, 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, Harness: "bogus"}) + if !errors.Is(err, ErrUnknownHarness) { + t.Fatalf("err = %v, want ErrUnknownHarness", err) + } + // The harness is rejected before any durable state is created — no seed row, + // no worktree — so an unknown harness never leaves an orphan behind. + if len(st.sessions) != 0 { + t.Fatalf("no session row should be created, got %d", len(st.sessions)) + } + if ws.lastCfg.SessionID != "" || ws.destroyed != 0 { + t.Fatal("workspace must not be created for an unknown harness") + } + if rt.created != 0 { + t.Fatal("runtime must not be created for an unknown harness") + } +} + // pathPinManager builds a manager whose Executable dep is stubbed, plus a // buffer capturing its log output, for the hook PATH pin tests. func pathPinManager(executable func() (string, error)) (*Manager, *fakeStore, *fakeRuntime, *bytes.Buffer) {