fix(spawn): reject unknown harness with 400 instead of opaque 500 (#211)
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
This commit is contained in:
parent
da30da5a45
commit
cbf3f0a2db
|
|
@ -338,6 +338,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):
|
||||
|
|
|
|||
|
|
@ -290,6 +290,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) {
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
@ -180,6 +184,13 @@ func (m *Manager) Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Sess
|
|||
cfg.Harness = m.defaultHarness
|
||||
}
|
||||
|
||||
// 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)
|
||||
|
|
|
|||
|
|
@ -180,6 +180,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
|
||||
|
|
@ -835,6 +840,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) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue