diff --git a/.changeset/the-supervisor-changes-agents.md b/.changeset/the-supervisor-changes-agents.md new file mode 100644 index 00000000..8cba2bb9 --- /dev/null +++ b/.changeset/the-supervisor-changes-agents.md @@ -0,0 +1,9 @@ +--- +"@openspec-ui/core": minor +"@openspec-ui/webui": minor +"@openspec-ui/server": minor +"@openspec-ui/cli": minor +"openspec-ui-vscode": minor +--- + +The supervisor can act on a failed stage (the-supervisor-changes-agents, ADR 0039 decision 4). Under `supervisor.mode: "act"`, set in a change's own `harness.json` together with `autonomyLevel: "autonomous"` and more than one attempt per stage, a stage whose failure is likely to pass is attempted again on the same agent, and one that repeating cannot fix is moved to the first agent of `supervisor.fallback.` the change's policy allows: `allowProviderChange` for another provider, `allowCostIncrease` for any agent but the local model. Every repeat and move is said in the chain's events, recorded in the audit log, and costs an attempt; a chain says `failed` only for a stage it does not try again. Every registered agent now names its provider. Under `advise`, the last-run suggestion names the fallback `act` would use. A change's Harness Settings offers Act under Autonomous, with the fallback per stage, the allowances and a note on cost and providers; `openspec-ui-cli run` prints why a stage is attempted again. diff --git a/HARNESS.md b/HARNESS.md index 8200197e..c67e2af7 100644 --- a/HARNESS.md +++ b/HARNESS.md @@ -250,12 +250,15 @@ question. ### `supervisor` -`{ "mode"?: "advise" | "off", "silentAfterSeconds"?: , -"waitingAfterSeconds"?: }`. Optional; absent means -`advise`, 600 and 60. Accepted in both files, and a change's own object is -merged key by key over the global one: a change that turns it off keeps -the workspace's thresholds. Set the mode from either Harness Settings -view; the thresholds are set in the file. +`{ "mode"?: "advise" | "off" | "act", "silentAfterSeconds"?: , +"waitingAfterSeconds"?: , "fallback"?: { "": ["", ...] }, +"allowCostIncrease"?: , "allowProviderChange"?: }`. +Optional; absent means `advise`, 600 and 60, no fallback and nothing +allowed. Accepted in both files, and a change's own object is merged key +by key over the global one: a change that turns it off keeps the +workspace's thresholds. Set the mode from either Harness Settings view; +the thresholds are set in the file. `act`, `fallback` and the two +allowances are a change's own only — see [Act](#act) below. The supervisor ([ADR 0039](docs/adr/0039-the-supervisor-advises.md)) is rules over records that already exist. It is not a process and not a @@ -297,6 +300,54 @@ number in output is usually a duration or a count. The diagnosis quotes the line it was found in, so it can be checked. A failure matching none is diagnosed as unknown, and nothing more is said beneath it. +#### Act + +Under `act` (the-supervisor-changes-agents, ADR 0039 decision 4) the +supervisor also does something about a stage whose agent failed, from the +failure's diagnosis and the change's own policy: + +| Diagnosis | What it does | +| --- | --- | +| Repeating is likely to help (rate limit, service failure) | Attempts the stage again on the same agent | +| Repeating will not help (not installed, not signed in, blocked, no network) | Moves the stage to the first agent in `fallback.` that the policy allows and the stage has not run on in this chain | +| Not known | Nothing: the chain ends with the failure, as under `advise` | + +Every repeat or move is another attempt of the stage, so it is spent from +`maxStageAttempts`; with no attempt left the chain ends with the failure. +A moved stage runs its new agent with that agent's own defaults: the old +entry's model, effort, budget and custom agent are not carried over. + +The policy: + +- **`allowProviderChange`** — a move to an agent of another provider + (Anthropic, GitHub, OpenAI, Google, DeepSeek, or the local model). The + change's files and prompts then go to that provider. +- **`allowCostIncrease`** — a move to any agent but the local model. + Nothing here knows one service's price against another's, so every move + but one to the local model counts as one that may cost more. + +Both are false where absent, and a fallback the policy refuses is skipped +and named. `act` is refused in the workspace file, and in a change's file +that does not itself set `autonomyLevel: "autonomous"`: under a level that +asks a person, the person decides. A configuration whose `act` would have +one attempt per stage is refused when it resolves. A fallback list names +only stages that run an agent, only registered agents, never +`vscode-chat`, and no agent twice. + +What it does is always said and recorded: a progress line ("the supervisor +moved apply from copilot-cli-acp to claude-cli-acp: …"), the next +attempt's `stageStarted` naming the new agent with the reason, and an +audit `message` entry from `supervisor` carrying `supervisorDecision`. A +chain yields no `failed` event for a stage it goes on to attempt again. +Under `advise`, a change's last-run suggestion names the fallback `act` +would move the stage to, and changes nothing. + +In a change's Harness Settings, Act is offered only where the change's own +autonomy level is Autonomous; choosing it asks for a second attempt where +the change has one, and shows the fallback fields and the two allowances +with a note on cost and providers. `openspec-ui-cli run` prints the move, +and the reason under the next attempt's heading. + ### `branches` `{ "rebaseWhenBehind"?: , "followMain"?: }`. Optional; diff --git a/openspec/changes/the-supervisor-changes-agents/.openspec.yaml b/openspec/changes/the-supervisor-changes-agents/.openspec.yaml new file mode 100644 index 00000000..4adf1522 --- /dev/null +++ b/openspec/changes/the-supervisor-changes-agents/.openspec.yaml @@ -0,0 +1,4 @@ +schema: spec-driven +created: 2026-10-05 +follows: + - the-supervisor-advises diff --git a/openspec/changes/the-supervisor-changes-agents/design.md b/openspec/changes/the-supervisor-changes-agents/design.md new file mode 100644 index 00000000..d37428e1 --- /dev/null +++ b/openspec/changes/the-supervisor-changes-agents/design.md @@ -0,0 +1,126 @@ +## Context + +See `proposal.md` and ADR 0039. What exists after `the-supervisor-advises`: + +- Every failed run carries a `FailureDiagnosis` with `repeatHelps` + (`no`, `likely`, `unknown`), on its `failed` event and audit entry. +- `supervisor.mode` is `advise` or `off`; `act` is refused today. +- `HarnessChainRunner` runs each stage in an attempt loop bounded by + `maxStageAttempts`, and attempts a stage again only when a ceiling cut + it. A stage's `failed` event is yielded as it arrives, and it is the + chain's terminal event (ADR 0012): nothing may follow it. +- A stage's agent comes from `stepAgents.`. + +## Goals / Non-Goals + +**Goals:** + +- A chain under `act` survives a failure a repeat or another agent fixes. +- The policy is the person's, written in the change's own file, and every + move is said and recorded. + +**Non-Goals:** + +- Stopping a run that is still going. The supervisor acts only on a stage + that has already ended in failure; a silent run is still only pointed + out (ADR 0039 decision 3). +- Judging which agent is better. The order is the person's list. +- Moving a model or an effort with the agent: an entry's settings belong + to its agent, as `mergeStepAgent` already holds for a change of agent. +- Acting outside a chain: a single run or a delegated item is not moved. + +## Decisions + +1. **`act` needs three things in the change's own file.** + - `supervisor.mode: "act"` is refused in the global file, as + `autonomyLevel: "autonomous"` is: it widens what runs unattended. + - It is refused unless the same file sets `autonomyLevel: "autonomous"`: + under a level that asks a person, the person is the one deciding. + - It is refused unless the resolved `maxStageAttempts` is above 1: a + repeat or a move is an attempt of the stage, under the one ceiling + every reason shares (HARNESS.md, `maxStageAttempts`). A setting that + can never act is not accepted quietly. + +2. **The policy keys.** + - `supervisor.fallback`: `{ : [agentId, ...] }` for the stages + that run an agent. Each id must be a registered agent other than + `vscode-chat` (a chain cannot hand a retried stage to the editor's + chat), and appear once. Accepted in either file: a list says what may + be used, and only `act` uses it. + - `supervisor.allowCostIncrease`, `supervisor.allowProviderChange`: + booleans, absent meaning false, per change only (ADR 0018: what widens + authority is never inherited). + +3. **A provider per agent, in the registry.** `AgentDescriptor.provider`: + `anthropic` (claude), `github` (copilot), `openai` (codex), `google` + (gemini), `deepseek`, `local` (both local LLM agents), `vscode` + (vscode-chat). A move to another provider sends the change's files to + another company, which is the reason it needs its own allowance. + A move is a cost increase unless it is to a `local` agent: nothing here + knows one service's price against another's, and the safe reading of + "not known" is "may cost more". + +4. **The decision is a pure function in `supervisor.ts`.** + `superviseFailure({ stage, current, tried, diagnosis, supervisor })` + returns: + - `{ action: "repeat", agent: current }` where repeating is likely to + help; + - `{ action: "move", agent }` where it will not, naming the first agent + of the stage's fallback list that has not been tried in this chain's + stage and that the policy allows; + - `{ action: "none", why }` otherwise, saying which rule refused each + candidate ("claude-cli-acp is another provider, and + allowProviderChange is false"). + +5. **The chain holds a stage's failure until it is decided.** `runStage` + no longer yields a `failed` event that came from the stage's agent; it + keeps it on the chain's state and returns `failed`. The attempt loop + then: + - under `act`, with an attempt left and a decision to repeat or move, + yields a `progress` event saying what the supervisor did and why, + records an audit `message` entry from `supervisor`, and starts the + next attempt, whose `stageStarted` names the agent and carries the + reason in `previousAttemptReason`; + - otherwise yields the held `failed` event, unchanged, and the chain + ends as before. + So the terminal contract holds: a `failed` event is still the last + event of the chain it ends, and only one that ends it is yielded. + A moved stage runs its new agent with that agent's own defaults: the + model, effort, budget and custom agent of the old entry are not carried + over. + +6. **Under `advise`, the last-run suggestion names the fallback.** Where + the change's last run failed at a stage that cannot simply be repeated, + and the stage has a fallback the policy allows, the suggestion says + which agent `act` would move it to. It still changes nothing. + +7. **Surfaces.** + - The change's Harness Settings: the Supervisor choice gains Act, offered + only where the change's own autonomy level is Autonomous, with a note + on cost and providers; a field per stage for its fallback agents; the + two allowances. The global view does not offer Act or the allowances. + Choosing Act where the change would have one attempt per stage sets + `maxStageAttempts` to 2 in the change's file and says so in the note: + with one attempt Act could do nothing, and the configuration would be + refused when it resolves. A change whose autonomy level is moved away + from Autonomous while Act is chosen is told that saving will be + refused. + - The audit entry is a `message` from `supervisor` with + `supervisorDecision` (`action`, `stage`, `from`, `to`, `because`), so no + counter of runs reads it as one. + - The chain's timeline shows the `progress` line and the next stage's + reason, as it shows any repeated attempt. + - `openspec-ui-cli run` prints a repeated or moved stage's reason under + its heading. + +## Risks / Trade-offs + +- **A move can cost money.** It is off unless the change's file says + `act`, and a move to a paid agent needs `allowCostIncrease` as well. +- **A move can send the change to another company.** It needs + `allowProviderChange`. +- **A repeat of a rate-limited stage may hit the limit again.** It is one + attempt under `maxStageAttempts`, and the second failure ends the chain + if nothing is left. +- **Holding the failure delays it by the decision**, which is a pure + function over what the chain already holds. diff --git a/openspec/changes/the-supervisor-changes-agents/proposal.md b/openspec/changes/the-supervisor-changes-agents/proposal.md new file mode 100644 index 00000000..b9402e23 --- /dev/null +++ b/openspec/changes/the-supervisor-changes-agents/proposal.md @@ -0,0 +1,66 @@ +## Why + +[ADR 0039](../../../docs/adr/0039-the-supervisor-advises.md), decision 4, +and the owner's answers of 2026-10-04: the supervisor may change the agent +of a stage when one fails, where a change's configuration allows it, and +always says so. The policy is configured: which agents a stage falls back +to, and whether a fallback may cost more or go to another provider. + +`the-supervisor-advises` diagnoses a failure and says whether repeating it +can help. Today a chain that fails on "not signed in" or "not installed" +ends there, and one cut by a passing rate limit or server error ends there +too: `maxStageAttempts` repeats only a stage a ceiling cut. + +## What Changes + +- **`supervisor.mode: "act"`**, in a change's own `harness.json` only, and + only with `autonomyLevel: "autonomous"` and `maxStageAttempts` above 1. + Under `act`, when a stage fails, the chain asks the supervisor: + - repeating is likely to help (rate-limited, server error): the stage is + attempted again on the same agent; + - repeating will not help (not installed, not signed in, blocked, + network): the stage moves to the next agent of + `supervisor.fallback.` that the policy allows; + - otherwise, or with no attempt left, the chain fails as it does today. +- **The policy.** `supervisor.fallback`, an ordered list of agents per + stage, in either file. `supervisor.allowCostIncrease` and + `supervisor.allowProviderChange`, per change only, both false by default. + A move to an agent of another provider (Anthropic, GitHub, OpenAI, Google, + DeepSeek, the local model) needs the second; any move except to the local + model needs the first, since nothing here knows one service's price + against another's. +- **Always said.** The chain's timeline shows the move and why: "the + supervisor moved apply from copilot-cli-acp to claude-cli-acp: the agent + is not signed in". The audit log records it. The move is never silent, + and a policy that refused a fallback says which rule refused it. +- **Under `advise`**, a change whose last run cannot be repeated is told + which fallback agent the policy would allow. +- **From the UI.** A change's Harness Settings offer Act, with a warning + about cost and providers, the fallback agents per stage, and the two + allowances. + +## Capabilities + +### New Capabilities + +(none) + +### Modified Capabilities + +- `agentic-harness`: `act`, the fallback policy, and how a chain repeats + or moves a failed stage. +- `shared-ui`: the change's Harness Settings for them. +- `ci-cli`: `run` says when the supervisor moved or repeated a stage. + +## Impact + +- `packages/core`: `harness-config.ts` (the new keys and their rules), + `agents/registry.ts` (each agent's provider), `supervisor.ts` (the + decision), `harness-chain-runner.ts` (holding a stage's failure until the + decision, and acting on it). +- `packages/webui`: the change's Harness Settings. +- `packages/cli`: `render-run.ts`. +- `packages/extension`: regenerated schemas. +- `HARNESS.md`. +- No event kind is added: a move is a `progress` event and the next + `stageStarted`, which already carries the attempt and its reason. diff --git a/openspec/changes/the-supervisor-changes-agents/specs/agentic-harness/spec.md b/openspec/changes/the-supervisor-changes-agents/specs/agentic-harness/spec.md new file mode 100644 index 00000000..824e18fa --- /dev/null +++ b/openspec/changes/the-supervisor-changes-agents/specs/agentic-harness/spec.md @@ -0,0 +1,73 @@ +## ADDED Requirements + +### Requirement: The supervisor acts only where a change allows it + +`supervisor.mode: "act"` SHALL be accepted only in a change's own +`harness.json`, only where that file sets `autonomyLevel: "autonomous"`, +and only where the resolved `maxStageAttempts` is above 1. Otherwise it +SHALL be refused where the configuration resolves, naming the rule. + +`supervisor.allowCostIncrease` and `supervisor.allowProviderChange` SHALL +be accepted only in a change's own file and SHALL mean false where absent. +`supervisor.fallback` SHALL be accepted in either file, as an ordered list +of registered agents per agent stage, without `vscode-chat` and without a +repeated id. + +#### Scenario: Act in the global file + +- **WHEN** `openspec/agent-harness.json` sets `supervisor.mode: "act"` +- **THEN** resolving the configuration fails and says act is set per change + +#### Scenario: Act without an attempt to spend + +- **WHEN** a change's file sets `act` and `maxStageAttempts` resolves to 1 +- **THEN** resolving the configuration fails and names `maxStageAttempts` + +### Requirement: A failed stage is repeated or moved under act, and said + +Under `act`, when a chain's stage fails and an attempt is left, the chain +SHALL: + +- attempt the stage again on the same agent where the failure's diagnosis + says repeating is likely to help; +- move the stage to the first agent of `supervisor.fallback.` not + yet tried in it that the policy allows, where the diagnosis says + repeating will not help; +- otherwise end with the stage's failure, as without `act`. + +A move to an agent of another provider SHALL need +`allowProviderChange`, and a move to any agent that is not the local model +SHALL need `allowCostIncrease`. A moved stage SHALL run its new agent with +that agent's defaults. + +Every repeat and move SHALL be said in the chain's events and recorded in +the audit log, with the agent, the cause and, for a fallback refused, the +rule that refused it. A chain SHALL yield no `failed` event for a stage it +goes on to attempt again. + +#### Scenario: Not signed in, with a fallback allowed + +- **WHEN** under `act`, `apply` fails on `copilot-cli-acp` diagnosed as not + signed in, its fallback is `claude-cli-acp`, and both allowances are true +- **THEN** the chain says it moved apply to `claude-cli-acp` because the + agent is not signed in, records it, and attempts apply on + `claude-cli-acp` + +#### Scenario: A fallback the policy refuses + +- **WHEN** the same happens with `allowProviderChange` false +- **THEN** the chain ends with apply's failure, and says the fallback + `claude-cli-acp` was refused because it is another provider + +#### Scenario: A rate limit + +- **WHEN** under `act`, a stage fails diagnosed as rate-limited with an + attempt left +- **THEN** the stage is attempted again on the same agent, and the chain + says why + +#### Scenario: Advise names the fallback + +- **WHEN** under `advise`, a change's last run failed at apply for a cause + repeating cannot fix, and apply has an allowed fallback +- **THEN** the supervisor's suggestion names that agent and changes nothing diff --git a/openspec/changes/the-supervisor-changes-agents/specs/ci-cli/spec.md b/openspec/changes/the-supervisor-changes-agents/specs/ci-cli/spec.md new file mode 100644 index 00000000..8f6ddd36 --- /dev/null +++ b/openspec/changes/the-supervisor-changes-agents/specs/ci-cli/spec.md @@ -0,0 +1,12 @@ +## ADDED Requirements + +### Requirement: The terminal says when the supervisor moved a stage + +`openspec-ui-cli run` SHALL print, under a repeated or moved stage's +heading, why it was attempted again and on which agent. + +#### Scenario: A moved stage in the terminal + +- **WHEN** a chain run from the terminal moves apply to another agent +- **THEN** the heading names the new agent and the attempt, and the line + under it says the supervisor moved it and why diff --git a/openspec/changes/the-supervisor-changes-agents/specs/shared-ui/spec.md b/openspec/changes/the-supervisor-changes-agents/specs/shared-ui/spec.md new file mode 100644 index 00000000..4a9766c0 --- /dev/null +++ b/openspec/changes/the-supervisor-changes-agents/specs/shared-ui/spec.md @@ -0,0 +1,19 @@ +## ADDED Requirements + +### Requirement: A change's Harness Settings set what the supervisor may do + +A change's Harness Settings SHALL offer Act among the supervisor's modes +only where the change's own autonomy level is Autonomous, with a note that +under Act the supervisor may start another agent, which can cost money and +send the change's files to another provider. They SHALL offer each agent +stage's fallback agents and the two allowances, and SHALL save them with +the rest. The global Harness Settings SHALL NOT offer Act or the +allowances. + +#### Scenario: Turning Act on + +- **WHEN** a person sets a change's autonomy level to Autonomous, chooses + Act, names `claude-cli-acp` as apply's fallback and saves +- **THEN** the change's `harness.json` carries `supervisor.mode: "act"` and + `supervisor.fallback.apply: ["claude-cli-acp"]`, and the note about cost + and providers was shown beside the choice diff --git a/openspec/changes/the-supervisor-changes-agents/tasks.md b/openspec/changes/the-supervisor-changes-agents/tasks.md new file mode 100644 index 00000000..e702697c --- /dev/null +++ b/openspec/changes/the-supervisor-changes-agents/tasks.md @@ -0,0 +1,110 @@ +Requested by the owner on 2026-10-04 and 2026-10-05 (ADR 0039 decision 4): +the supervisor may change a stage's agent where a change's configuration +allows it, under a configured policy, and always says so. + +## 1. Core + +- [x] 1.1 `agents/registry.ts`: `AgentDescriptor.provider` for every agent + (design.md decision 3). Test in `registry.test.ts`: every agent has one. + Every registry entry has `provider`; `registry.test.ts`: 5 passed. +- [x] 1.2 `harness-config.ts` and `harness-config-schema.ts`: `act`, + `fallback`, `allowCostIncrease`, `allowProviderChange`, with the rules of + decisions 1 and 2; extension schemas regenerated. Tests in + `harness-config.test.ts`: act in the global file, without autonomous, + with one attempt, accepted with all three; a fallback with an unknown + agent, with `vscode-chat`, with a repeated id, for a stage without an + agent; the allowances refused in the global file. + `act`, `fallback` and the allowances, schema by scope, extension + schemas regenerated; `harness-config.test.ts`: 201 passed, six new under + "supervisor act". +- [x] 1.3 `supervisor.ts`: `superviseFailure` (decision 4). Test in + `supervisor.test.ts`: repeat on `likely`; move to the first allowed + untried fallback; a provider change refused and named; a cost increase + refused and named; a local fallback allowed without + `allowCostIncrease`; nothing tried twice; `unknown` and no fallback. + `superviseFailure` and `fallbackRefusal`; seven new cases in + `supervisor.test.ts`, 34 passed with `last-runs` and `registry`. After + 4.3, a move names the fallbacks it passed over and why ("passed over: + copilot-cli-acp may cost money, and allowCostIncrease is false"); + `supervisor.test.ts` 22 passed. +- [x] 1.4 `harness-chain-runner.ts`: the held failure and the act loop + (decision 5). Tests in the chain runner's tests: a stage moved to its + fallback, with the progress line, the audit entry, the new agent's + `stageStarted` and no `failed` before it; a repeat on a rate limit; a + refused fallback ending with the original failure; no attempt left + ending with it; `advise` unchanged. + `runStage` holds the agent's `failed`; `superviseHeldFailure` repeats, + moves or yields it; audit `supervisorDecision` on a `supervisor` message + entry. Five new cases in `harness-chain-runner.test.ts`; the core project + 2095 passed. Found in 4.3: `claude-cli-acp` says "Not logged in · Please + run /login" as an ACP message, not on stderr, so its failure was + diagnosed unknown and `act` could do nothing with it; `agent-runner.ts` + now adds what an ACP agent says to the tail it diagnoses. One new case in + `agent-runner.test.ts`, 29 passed. +- [x] 1.5 `supervisor.ts` / `pipeline-readings.ts`: under `advise`, the + last-run suggestion names the allowed fallback (decision 6). Test in + `supervisor.test.ts`. + `aboutLastRuns` adds the fallback sentence from the last run's agent + (`last-runs.ts` now reads it); two new cases in `supervisor.test.ts`. + +## 2. Surfaces + +- [x] 2.1 `packages/webui`: the change's Harness Settings (decision 7). + Tests in `ChangeHarnessSettingsView.test.tsx`: Act offered only under + Autonomous, the note shown, the fallback and the allowances saved; the + global view offers neither. + Act under Autonomous only, the warning, the fallback per stage, the + allowances, a second attempt asked for; three new cases in + `ChangeHarnessSettingsView.test.tsx`, one assertion in the global view's; + 65 passed. +- [x] 2.2 `packages/cli`: `render-run.ts` prints a stage's reason under a + repeated or moved heading. Test in `render-run.test.ts`. + The reason under the heading, not repeated after the supervisor's own + line; two new cases in `render-run.test.ts`, 15 passed. + +## 3. Documents + +- [x] 3.1 `HARNESS.md`: the supervisor's `act`, the policy, the providers, + what it never does. + `HARNESS.md`, `supervisor`, "Act". +- [x] 3.2 A changeset: core, webui, server, cli, extension, minor. + `.changeset/the-supervisor-changes-agents.md`. + +## 4. Checks + +- [x] 4.1 `npm run typecheck && npm run lint`, and every test project, each + on its own where the root run would exceed a background limit. + Typecheck clean; lint 0 errors (3 warnings, in lines this change did not + touch); the seven script tests pass. Core 2095, core-git-subprocess 67, + cli 201, webui 711, server 122, extension 502 passed. +- [x] 4.2 `openspec validate the-supervisor-changes-agents --strict`, and + the merge gate with the worktree's absolute path as `--cwd`. + Valid; the gate with `--cwd C:/Prog/.worktrees/OpenSpec-UI/the-supervisor-changes-agents` reported only 4.4 open (Human-only); after the owner closed it, ok. +- [x] 4.3 One live move end to end: a scratch change whose `apply` agent is + not on the PATH, under `act` with a fallback the policy allows, run with + `openspec-ui-cli run`; record the printed move, the audit entries, and + the fallback agent's run. + Record, 2026-10-05: the scratch repository of the-supervisor-advises 6.3, + change `greet-by-name`, autonomous, `maxStageAttempts: 2`, `apply` on + `gemini-cli` (not on the PATH), `supervisor` `act` with + `fallback.apply: ["copilot-cli-acp", "local-llm-acp"]` and only + `allowProviderChange`, the local LLM a stand-in server on 127.0.0.1. + `run` printed `▶ apply — gemini-cli`, then "· the supervisor moved apply + from gemini-cli to local-llm-acp: the agent is not installed: repeating + will not help (passed over: copilot-cli-acp may cost money, and + allowCostIncrease is false)", then `▶ apply — local-llm-acp (attempt 2)`; + the fallback wrote `hello.txt` and completed, and the chain went on to + verify, which ended it with the task still unchecked (the stand-in ticks + nothing). No `failed` event for the first attempt. The audit log holds + `gemini-cli` failed (agent-not-installed), a `supervisor` message entry + with `supervisorDecision` {move, apply, gemini-cli → local-llm-acp, the + cause}, then `local-llm-acp` started and completed at apply. With + `fallback.apply: ["claude-cli-acp"]` and both allowances, the move went to + `claude-cli-acp`, which is not signed in on this machine; after the + agent-runner fix it printed "the supervisor did not try apply again: the + agent is not signed in: repeating will not help, and every fallback for + apply has been tried" and ended with that diagnosis and its remedy. +- [x] 4.4 **Human-only**: in a change's Harness Settings in either host, see + Act offered under Autonomous with its note, and set a fallback. + Closed by the owner on 2026-10-05, ticked by hand in this change's + working directory. diff --git a/packages/cli/src/render-run.test.ts b/packages/cli/src/render-run.test.ts index 9c3a10e4..2b4fd19d 100644 --- a/packages/cli/src/render-run.test.ts +++ b/packages/cli/src/render-run.test.ts @@ -104,6 +104,34 @@ describe("RunTextRenderer", () => { expect(output).toContain("attempt 2"); }); + // the-supervisor-changes-agents 2.2 + it("says why a stage is attempted again, under its heading", () => { + const output = render([ + { + kind: "stageStarted", + runId: "r", + timestamp: at, + stage: "apply", + agentId: "claude-cli-acp", + attempt: 2, + previousAttemptReason: "stopped because it ran past timeout.perStageMs", + }, + ]); + + expect(output).toContain("▶ apply — claude-cli-acp (attempt 2)\n because stopped because it ran past timeout.perStageMs"); + }); + + it("does not repeat a reason the supervisor's line has just said", () => { + const moved = "the supervisor moved apply from copilot-cli-acp to claude-cli-acp: the agent is not signed in"; + const output = render([ + { kind: "progress", runId: "r", timestamp: at, message: moved }, + { kind: "stageStarted", runId: "r", timestamp: at, stage: "apply", agentId: "claude-cli-acp", attempt: 2, previousAttemptReason: moved }, + ]); + + expect(output.split(moved)).toHaveLength(2); + expect(output).toContain("▶ apply — claude-cli-acp (attempt 2)"); + }); + it("shows nothing for an update core reads nothing out of", () => { const renderer = new RunTextRenderer(); diff --git a/packages/cli/src/render-run.ts b/packages/cli/src/render-run.ts index 9cf17604..c5fb3545 100644 --- a/packages/cli/src/render-run.ts +++ b/packages/cli/src/render-run.ts @@ -50,6 +50,9 @@ export class RunTextRenderer { * reason: they are two statements and running them together shows one * that was never made. */ private streamingKind: AcpTextChunkKind | "stdout" | undefined; + /** The last progress line printed, so a stage attempted again for a + * reason a progress line has just said is not told twice. */ + private lastProgress: string | undefined; /** Renders one event, or `undefined` for one with nothing to show. */ render(event: Event): RenderedPiece { @@ -57,7 +60,13 @@ export class RunTextRenderer { case "stageStarted": { const attempt = event.attempt !== undefined && event.attempt > 1 ? ` (attempt ${event.attempt})` : ""; const agent = event.agentId ? ` — ${event.agentId}` : ""; - return this.line(`\n▶ ${event.stage}${agent}${attempt}`); + // Why the stage is attempted again: a ceiling that cut it, or the + // supervisor repeating or moving it (the-supervisor-changes-agents). + const reason = attempt !== "" && event.previousAttemptReason !== undefined && event.previousAttemptReason !== this.lastProgress + ? `\n because ${event.previousAttemptReason}` + : ""; + this.lastProgress = undefined; + return this.line(`\n▶ ${event.stage}${agent}${attempt}${reason}`); } case "stageCompleted": return this.line(`✓ ${event.stage} → ${event.nextStage}`); @@ -77,6 +86,7 @@ export class RunTextRenderer { case "stderr": return this.line(event.chunk.replace(/\r?\n$/, "")); case "progress": + this.lastProgress = event.message; return this.line(`· ${event.message}`); case "usageReported": { const parts: string[] = []; diff --git a/packages/core/src/agent-runner.test.ts b/packages/core/src/agent-runner.test.ts index 84cb9187..ef3a2713 100644 --- a/packages/core/src/agent-runner.test.ts +++ b/packages/core/src/agent-runner.test.ts @@ -427,6 +427,27 @@ describe("createAgentRunner — a failure's diagnosis", () => { expect(auditLog.entries.find((entry) => entry.outcome === "failed")?.diagnosis?.cause).toBe("not-signed-in"); }); + // the-supervisor-changes-agents 4.3: found live, on claude-cli-acp. + it("diagnoses from what an ACP agent said as a message", async () => { + const { adapter } = makeFakeAdapter(async function* (invocation, cmd) { + yield { kind: "started", runId: cmd.runId, timestamp: "t", command: "implement", cwd: workspaceRoot }; + yield { + kind: "agentUpdate", + runId: cmd.runId, + timestamp: "t", + update: { sessionUpdate: "agent_message_chunk", content: { type: "text", text: "Not logged in · Please run /login" } }, + }; + yield { kind: "failed", runId: cmd.runId, timestamp: "t", reason: "claude exited with code 1" }; + }); + const runner = createAgentRunner(adapter, { workspaceRoot, allowlist, auditLog: new InMemoryAuditLog() }); + + const events: Event[] = []; + for await (const event of runner.run(command("run-diag-acp"))) events.push(event); + + const failed = events.find((event) => event.kind === "failed"); + expect(failed?.kind === "failed" ? failed.diagnosis?.cause : undefined).toBe("not-signed-in"); + }); + it("diagnoses an adapter that throws", async () => { const { adapter } = makeFakeAdapter(async function* (invocation, cmd) { yield { kind: "started", runId: cmd.runId, timestamp: "t", command: "implement", cwd: workspaceRoot }; diff --git a/packages/core/src/agent-runner.ts b/packages/core/src/agent-runner.ts index 465999bb..12b4d122 100644 --- a/packages/core/src/agent-runner.ts +++ b/packages/core/src/agent-runner.ts @@ -18,6 +18,7 @@ import { prepareAgentContext, } from "./security.js"; import type { AgentUsage } from "./agent-usage.js"; +import { readAcpStreamedText } from "./acp-streamed-text.js"; import { diagnoseFailure, type FailureDiagnosis } from "./failure-diagnosis.js"; import { createGitWrapper } from "./git.js"; import type { Command, CommandKind, Event } from "./protocol.js"; @@ -321,6 +322,14 @@ export function createAgentRunner(adapter: AgentAdapter, options: AgentRunnerOpt if (event.kind === "stdout" || event.kind === "stderr") { outputTail = (outputTail + event.chunk).slice(-OUTPUT_TAIL_CHARS); } + // An ACP agent says why it could not work as a message, not on + // stderr: `claude` answers "Not logged in · Please run /login" that + // way, and a tail without it diagnosed nothing + // (the-supervisor-changes-agents 4.3). + if (event.kind === "agentUpdate") { + const said = readAcpStreamedText(event.update); + if (said?.kind === "agent_message_chunk") outputTail = (outputTail + said.text).slice(-OUTPUT_TAIL_CHARS); + } if (event.kind === "completed") lastSummary = event.summary; if (event.kind === "failed") { lastOutcome = "failed"; diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index 93d7f392..bb1fc2f6 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -12,6 +12,15 @@ import { LocalLlmAcpAdapter } from "./local-llm-acp.js"; import { LocalLlmAdapter } from "./local-llm.js"; describe("AGENT_REGISTRY", () => { + // the-supervisor-changes-agents 1.1: a fallback policy asks whether a move + // changes provider, so every agent says whose it is. + it("names a provider for every agent", () => { + const providers = new Set(["anthropic", "github", "openai", "google", "deepseek", "local"]); + for (const descriptor of AGENT_REGISTRY) expect(providers.has(descriptor.provider), descriptor.id).toBe(true); + expect(AGENT_REGISTRY.find((descriptor) => descriptor.id === "local-llm-acp")?.provider).toBe("local"); + expect(AGENT_REGISTRY.find((descriptor) => descriptor.id === "copilot-cli-acp")?.provider).toBe("github"); + }); + it("has unique ids", () => { const ids = AGENT_REGISTRY.map((a) => a.id); expect(new Set(ids).size).toBe(ids.length); diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index 0af4e35b..21e9ca41 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -18,10 +18,16 @@ // config entry itself ("dispatch stage to VS Code chat") and avoids // implying that a CLI process will run. +/** Who serves an agent's model: what a change's files are sent to when it + * runs. A move between two is a provider change (the-supervisor-changes-agents). */ +export type AgentProvider = "anthropic" | "github" | "openai" | "google" | "deepseek" | "local"; + export interface AgentDescriptor { /** Matches the `AgentAdapter.name` of the corresponding adapter. */ id: string; label: string; + /** Who serves its model (the-supervisor-changes-agents). */ + provider: AgentProvider; /** The CLI flag this adapter passes a model with (see harness-step-models * design.md). */ modelFlag?: string; @@ -38,25 +44,25 @@ export interface AgentDescriptor { } export const AGENT_REGISTRY: readonly AgentDescriptor[] = [ - { id: "claude-cli", label: "Claude CLI", modelFlag: "--model", customAgentFlag: "--agent" }, - { id: "copilot-cli", label: "GitHub Copilot CLI", modelFlag: "--model", customAgentFlag: "--agent" }, - { id: "codex-cli", label: "Codex CLI" }, - { id: "gemini-cli", label: "Gemini CLI" }, - { id: "local-llm", label: "Local LLM (OpenAI-compatible)", takesModel: true }, + { provider: "anthropic", id: "claude-cli", label: "Claude CLI", modelFlag: "--model", customAgentFlag: "--agent" }, + { provider: "github", id: "copilot-cli", label: "GitHub Copilot CLI", modelFlag: "--model", customAgentFlag: "--agent" }, + { provider: "openai", id: "codex-cli", label: "Codex CLI" }, + { provider: "google", id: "gemini-cli", label: "Gemini CLI" }, + { provider: "local", id: "local-llm", label: "Local LLM (OpenAI-compatible)", takesModel: true }, // Runs inside the product: nothing to install (ADR 0038). - { id: "local-llm-acp", label: "Local LLM agent (OpenAI-compatible, built in)", takesModel: true }, + { provider: "local", id: "local-llm-acp", label: "Local LLM agent (OpenAI-compatible, built in)", takesModel: true }, // ACP-flavored adapters (acp-agent-adapters) — additional entries, not // replacements for the four above (see this file's header comment). - { id: "copilot-cli-acp", label: "GitHub Copilot CLI (ACP)", modelFlag: "--model", customAgentFlag: "--agent" }, - { id: "gemini-cli-acp", label: "Gemini CLI (ACP)" }, - { id: "codex-cli-acp", label: "Codex CLI (ACP)" }, + { provider: "github", id: "copilot-cli-acp", label: "GitHub Copilot CLI (ACP)", modelFlag: "--model", customAgentFlag: "--agent" }, + { provider: "google", id: "gemini-cli-acp", label: "Gemini CLI (ACP)" }, + { provider: "openai", id: "codex-cli-acp", label: "Codex CLI (ACP)" }, // DeepSeek through its own CLI's ACP profile (deepseek-joins-as-an-acp-agent). - { id: "deepseek-cli-acp", label: "DeepSeek CLI (ACP)" }, + { provider: "deepseek", id: "deepseek-cli-acp", label: "DeepSeek CLI (ACP)" }, // Label states the limitation inline, not just in the picker's own // copy (webui's AiPanel.tsx) — see design.md's risk mitigation // "the UI presenting this adapter must say so explicitly ... not leave // it to be discovered" and claude-acp.ts's own header comment for why. - { id: "claude-cli-acp", label: "Claude CLI (ACP) — progress only, no permission gate", modelFlag: "--model", customAgentFlag: "--agent" }, + { provider: "anthropic", id: "claude-cli-acp", label: "Claude CLI (ACP) — progress only, no permission gate", modelFlag: "--model", customAgentFlag: "--agent" }, ]; /** Whether a stage may name a model for `descriptor`'s agent. */ diff --git a/packages/core/src/audit-runs.ts b/packages/core/src/audit-runs.ts index 82f50ce0..cebd5c1d 100644 --- a/packages/core/src/audit-runs.ts +++ b/packages/core/src/audit-runs.ts @@ -36,6 +36,11 @@ export const VERIFY_CHECKS_AGENT_NAME = "verify-checks"; * before it started (a-card-says-what-its-change-is-doing). */ export const CHAIN_ENDING_AGENT_NAME = "chain"; +/** The `agent` of a `message` entry saying what the supervisor did about a + * failed stage under `act` (the-supervisor-changes-agents). A message, so + * no counter of runs reads it as one. */ +export const SUPERVISOR_AGENT_NAME = "supervisor"; + /** Whether this entry records a run, as opposed to a fact about one. * * The checks pseudo-agent and a chain's ending are excluded. The diff --git a/packages/core/src/browser.ts b/packages/core/src/browser.ts index 2ac0c7fe..eba9f91a 100644 --- a/packages/core/src/browser.ts +++ b/packages/core/src/browser.ts @@ -10,7 +10,7 @@ // openspec/changes/standalone-app/tasks.md 2.1). export * from "./protocol.js"; -export { AGENT_REGISTRY, DEFAULT_AGENT_ID, type AgentDescriptor } from "./agents/registry.js"; +export { AGENT_REGISTRY, DEFAULT_AGENT_ID, type AgentDescriptor, type AgentProvider } from "./agents/registry.js"; export type { ChangeState } from "./change-state.js"; // Where a change stands, and the one word every surface shows for it. Two // leaves with no Node imports; the reading itself stays Node-only diff --git a/packages/core/src/harness-chain-runner.test.ts b/packages/core/src/harness-chain-runner.test.ts index 8245aa0e..55e71633 100644 --- a/packages/core/src/harness-chain-runner.test.ts +++ b/packages/core/src/harness-chain-runner.test.ts @@ -2122,6 +2122,152 @@ describe("HarnessChainRunner — a chain writes its own ending (a-card-says-what }); }); +describe("HarnessChainRunner — the supervisor acts on a failed stage (the-supervisor-changes-agents 1.4)", () => { + const notSignedIn = { cause: "not-signed-in", repeatHelps: "no", evidence: "Authentication required" } as const; + const rateLimited = { cause: "rate-limited", repeatHelps: "likely", evidence: "429 Too Many Requests" } as const; + + /** A runner per agent: the ones named in `failing` fail with the given + * diagnosis (only the first time, with `once`); every other completes. */ + function runnersByAgent( + failing: Record, + options: { once?: boolean } = {}, + ): { + resolve: (agentId: string | undefined) => AgentRunner; + calls: { agentId: string | undefined; command: Command }[]; + } { + const calls: { agentId: string | undefined; command: Command }[] = []; + const failedOnce = new Set(); + return { + calls, + resolve: (agentId) => ({ + async *run(command) { + if (command.kind === "cancel") return; + calls.push({ agentId, command }); + yield { kind: "started", runId: command.runId, timestamp: "t", command: command.kind, cwd: command.cwd }; + const diagnosis = agentId === undefined ? undefined : failing[agentId]; + if (agentId !== undefined && diagnosis !== undefined && !(options.once === true && failedOnce.has(agentId))) { + failedOnce.add(agentId); + yield { kind: "failed", runId: command.runId, timestamp: "t", reason: diagnosis.evidence, diagnosis }; + return; + } + yield { kind: "completed", runId: command.runId, timestamp: "t", summary: `${command.kind} done` }; + }, + }), + }; + } + + /** A change entered at verify, which runs on copilot-cli-acp. */ + async function verifyingChange(supervisor: Record, maxStageAttempts = 2): Promise { + const root = await temporaryRoot(); + await writeGlobalHarnessConfig(root, { autonomyLevel: "semi-autonomous" }); + await writeChangeHarnessConfig(root, "demo", { + autonomyLevel: "autonomous", + maxStageAttempts, + stepAgents: { verify: { agent: "copilot-cli-acp", model: "gpt-5" } }, + supervisor, + } as never); + mockStatus(true); + await writeTasks(root, 0, 3); + mockArchiveSucceeds(); + return root; + } + + async function drain(chain: InstanceType, root: string): Promise { + const events: Event[] = []; + for await (const event of chain.run(baseCommand(root))) events.push(event); + return events; + } + + function supervisorSaid(events: readonly Event[]): Event | undefined { + return events.find((event) => event.kind === "progress" && event.message.startsWith("the supervisor")); + } + + const moving = { + mode: "act", + fallback: { verify: ["claude-cli-acp"] }, + allowCostIncrease: true, + allowProviderChange: true, + }; + + it("moves a stage to its fallback, says and records it, and never says failed", async () => { + const root = await verifyingChange(moving); + const auditLog = new InMemoryAuditLog(); + const { resolve, calls } = runnersByAgent({ "copilot-cli-acp": notSignedIn }); + + const events = await drain(new HarnessChainRunner({ resolveRunner: resolve, auditLog }), root); + + expect(events.some((event) => event.kind === "failed")).toBe(false); + expect(events.at(-1)).toMatchObject({ kind: "completed" }); + const said = supervisorSaid(events); + expect(said).toMatchObject({ message: expect.stringContaining("moved verify from copilot-cli-acp to claude-cli-acp") }); + const started = events.filter((event) => event.kind === "stageStarted" && event.stage === "verify"); + expect(started).toEqual([ + expect.objectContaining({ agentId: "copilot-cli-acp" }), + expect.objectContaining({ agentId: "claude-cli-acp", attempt: 2, previousAttemptReason: expect.stringContaining("moved verify") }), + ]); + expect(events.indexOf(said as Event)).toBeLessThan(events.indexOf(started[1] as Event)); + // The new agent runs with its own defaults, not the old entry's model. + expect(calls.map((call) => [call.agentId, call.command.model])).toEqual([["copilot-cli-acp", "gpt-5"], ["claude-cli-acp", undefined]]); + expect(auditLog.entries.filter((entry) => entry.agent === "supervisor")).toEqual([expect.objectContaining({ + outcome: "message", + stage: "verify", + supervisorDecision: expect.objectContaining({ action: "move", from: "copilot-cli-acp", to: "claude-cli-acp" }), + })]); + }); + + it("repeats a rate-limited stage on the same agent", async () => { + const root = await verifyingChange({ mode: "act" }); + const { resolve, calls } = runnersByAgent({ "copilot-cli-acp": rateLimited }, { once: true }); + + const events = await drain(new HarnessChainRunner({ resolveRunner: resolve }), root); + + expect(events.some((event) => event.kind === "failed")).toBe(false); + expect(events.at(-1)).toMatchObject({ kind: "completed" }); + expect(supervisorSaid(events)).toMatchObject({ message: expect.stringContaining("repeats verify on copilot-cli-acp") }); + expect(calls.map((call) => call.agentId)).toEqual(["copilot-cli-acp", "copilot-cli-acp"]); + }); + + it("ends with the original failure, and says why, when the policy refuses the fallback", async () => { + const root = await verifyingChange({ ...moving, allowProviderChange: false }); + const { resolve, calls } = runnersByAgent({ "copilot-cli-acp": notSignedIn }); + + const events = await drain(new HarnessChainRunner({ resolveRunner: resolve }), root); + + expect(calls).toHaveLength(1); + expect(events.at(-1)).toMatchObject({ kind: "failed", diagnosis: notSignedIn }); + expect(events.filter((event) => event.kind === "failed")).toHaveLength(1); + expect(supervisorSaid(events)).toMatchObject({ + message: expect.stringContaining("claude-cli-acp is another provider (anthropic), and allowProviderChange is false"), + }); + }); + + it("ends with the original failure when no attempt is left", async () => { + const root = await verifyingChange({ mode: "act" }, 2); + const { resolve, calls } = runnersByAgent({ "copilot-cli-acp": rateLimited }); + + const events = await drain(new HarnessChainRunner({ resolveRunner: resolve }), root); + + expect(calls).toHaveLength(2); + expect(events.at(-1)).toMatchObject({ kind: "failed", diagnosis: rateLimited }); + expect(events.filter((event) => event.kind === "failed")).toHaveLength(1); + expect(supervisorSaid(events)).toBeDefined(); + expect(events.filter((event) => event.kind === "progress" && event.message.includes("maxStageAttempts: 2"))).toHaveLength(1); + }); + + it("leaves a failure under advise exactly as it was", async () => { + const root = await verifyingChange({ ...moving, mode: "advise" }); + const auditLog = new InMemoryAuditLog(); + const { resolve, calls } = runnersByAgent({ "copilot-cli-acp": notSignedIn }); + + const events = await drain(new HarnessChainRunner({ resolveRunner: resolve, auditLog }), root); + + expect(calls).toHaveLength(1); + expect(events.at(-1)).toMatchObject({ kind: "failed", diagnosis: notSignedIn }); + expect(supervisorSaid(events)).toBeUndefined(); + expect(auditLog.entries.some((entry) => entry.agent === "supervisor")).toBe(false); + }); +}); + describe("HarnessChainRunner — asked to stop (a-change-is-run-from-its-card 3.9)", () => { /** A stage runner a test feeds one event at a time. A cancel ends it the * way a real runner's does: its `cancelled` comes once the process is gone. */ diff --git a/packages/core/src/harness-chain-runner.ts b/packages/core/src/harness-chain-runner.ts index a575eae0..6b5516ab 100644 --- a/packages/core/src/harness-chain-runner.ts +++ b/packages/core/src/harness-chain-runner.ts @@ -44,6 +44,7 @@ import { normalizeStepAgent, readChangeHarnessConfig, resolveHarnessConfig, + resolveSupervisor, } from "./harness-config.js"; import { archiveChange, statusChange } from "./openspec.js"; import { @@ -51,7 +52,8 @@ import { isWaitingStep, runChainStep, } from "./chain-steps.js"; -import { CHAIN_ENDING_AGENT_NAME, VERIFY_CHECKS_AGENT_NAME } from "./audit-runs.js"; +import { CHAIN_ENDING_AGENT_NAME, SUPERVISOR_AGENT_NAME, VERIFY_CHECKS_AGENT_NAME } from "./audit-runs.js"; +import { superviseFailure } from "./supervisor.js"; import { isChainStepName, skipsStage, type ChainPart } from "./harness-stage.js"; import { untilStopBoundary } from "./stop-boundary.js"; import { DEFAULT_AGENT_ID } from "./agents/registry.js"; @@ -234,6 +236,18 @@ interface ChainState { /** The questions handed to the stage now running, waiting for what it * says at its end. */ awaitingAnswer: ChainMessage[]; + /** The `failed` event a stage's agent ended with, held by `runStage` until + * the attempt loop decides whether the supervisor repeats or moves the + * stage. Yielded unchanged where it does not, so a `failed` event is + * still the last of the chain it ends (ADR 0012), and never yielded for a + * stage that goes on (the-supervisor-changes-agents). */ + heldFailure?: Extract; + /** The agent the supervisor moved a stage to, which later attempts of it + * run on. */ + agentOverride: Map; + /** Every agent each stage has run on in this chain: a fallback is never + * tried twice. */ + triedAgents: Map; } function nowIso(): string { @@ -346,11 +360,25 @@ function failedEvent(runId: string, reason: string): Event { * same fallback `resolveRunner` applies (`default-runners.ts`), so the * name recorded here matches the `agent` the apply stage's own audit * entries carry and the two group together. */ -function resolvedApplyAgent(harnessConfig: HarnessConfig): string { +function resolvedApplyAgent(harnessConfig: HarnessConfig, state?: Pick): string { + const moved = state?.agentOverride.get("apply"); + if (moved !== undefined) return moved; const entry = harnessConfig.stepAgents.apply; return entry === undefined ? DEFAULT_AGENT_ID : normalizeStepAgent(entry).agent; } +/** The agent a stage runs on in this chain, as `stageStarted` and + * `checkpoint` name it: the one the supervisor moved it to, else its + * configured entry, else "" — for a stage with no agent, or none + * configured (the-supervisor-changes-agents). */ +function stageAgentOf(stage: ChainPart, harnessConfig: HarnessConfig, state: Pick): string { + if (isChainStepName(stage) || !isHarnessStepAgentStage(stage)) return ""; + const moved = state.agentOverride.get(stage); + if (moved !== undefined) return moved; + const entry = harnessConfig.stepAgents[stage]; + return entry === undefined ? "" : normalizeStepAgent(entry).agent; +} + function wildcardPatternToRegExp(pattern: string): RegExp { const escaped = pattern.replace(/[.+?^${}()|[\]\\]/g, "\\$&").replace(/\*/g, ".*"); return new RegExp(`^${escaped}$`); @@ -625,7 +653,15 @@ export class HarnessChainRunner { return; } - const state: ChainState = { cancelRequested: false, elapsedMs: 0, attemptsByStage: new Map(), pendingMessages: [], awaitingAnswer: [] }; + const state: ChainState = { + cancelRequested: false, + elapsedMs: 0, + attemptsByStage: new Map(), + pendingMessages: [], + awaitingAnswer: [], + agentOverride: new Map(), + triedAgents: new Map(), + }; this.active.set(command.runId, state); // The stage the chain is in, and how it ended, as its own events say. // Read here rather than at each place an ending is yielded, so no @@ -1135,11 +1171,7 @@ export class HarnessChainRunner { // "" for the stages that run no agent at all ("archive", "git"), // matching `checkpoint`'s `nextAgentId` convention rather than // inventing a second spelling for the same idea. - agentId: !isHarnessStepAgentStage(stage) - ? "" - : (harnessConfig.stepAgents[stage] === undefined - ? "" - : normalizeStepAgent(harnessConfig.stepAgents[stage]).agent), + agentId: stageAgentOf(stage, harnessConfig, state), // Absent on a first attempt, which is every chain that has not // come back here — a surface renders those exactly as it always // did. @@ -1196,6 +1228,19 @@ export class HarnessChainRunner { state.elapsedMs += Date.now() - stageStartedAt; } + // A failure from the stage's agent, which `runStage` held: under + // `act` the supervisor may try the stage again, and otherwise it is + // yielded here exactly as it came (the-supervisor-changes-agents). + const held = state.heldFailure; + state.heldFailure = undefined; + if (held !== undefined && outcome === "failed") { + if (yield* this.superviseHeldFailure(stage, held, harnessConfig, state, command, attempt, maxAttempts)) continue; + break; + } + // An agent that said `failed` and then ended some other way: said + // in the order it came, as before anything was held. + if (held !== undefined) yield held; + const cutByCeiling = outcome === "cancelled" && state.cancelReason !== undefined; if (!cutByCeiling) break; // The chain's own cancel flag was set by the timer calling @@ -1218,11 +1263,7 @@ export class HarnessChainRunner { runId, timestamp: nowIso(), stage, - agentId: !isHarnessStepAgentStage(stage) - ? "" - : (harnessConfig.stepAgents[stage] === undefined - ? "" - : normalizeStepAgent(harnessConfig.stepAgents[stage]).agent), + agentId: stageAgentOf(stage, harnessConfig, state), attempt: attempt + 1, previousAttemptReason: state.cancelReason, }; @@ -1378,11 +1419,7 @@ export class HarnessChainRunner { // rather than re-listing the two stage names here — see // harness-step-agent.ts's `HarnessStepAgentStage` comment for // why they are kept on one list, together. - nextAgentId: !isHarnessStepAgentStage(nextStage) - ? "" - : (harnessConfig.stepAgents[nextStage] === undefined - ? "" - : normalizeStepAgent(harnessConfig.stepAgents[nextStage]).agent), + nextAgentId: stageAgentOf(nextStage, harnessConfig, state), }; const checkpointOutcome = await checkpointPromise; if (checkpointOutcome === "cancelled") { @@ -1516,6 +1553,85 @@ export class HarnessChainRunner { * swallowed `"completed"` with `"checkpoint"`/`"stageCompleted"`. A * `"failed"`/`"cancelled"` stage outcome always ends the whole chain, so * it is forwarded as-is regardless of `hasNextStage`. */ + /** What the supervisor does about a stage whose agent failed + * (the-supervisor-changes-agents, design.md decision 5). Under `act`, with + * an attempt left and a repeat or a move its policy allows, it says so, + * records it, announces the next attempt and returns `true`; the attempt + * loop goes on. Otherwise it yields the failure, unchanged, and returns + * `false` — under `act` after saying why the stage was not tried again. */ + private *superviseHeldFailure( + stage: ChainStage, + failure: Extract, + harnessConfig: HarnessConfig, + state: ChainState, + command: Command, + attempt: number, + maxAttempts: number, + ): Generator { + const { runId, cwd, context } = command; + const supervisor = resolveSupervisor(harnessConfig.supervisor); + // A person's cancel or stop, or a stage that already spent past its + // ceiling, ends the chain as it did before `act` existed. + if ( + supervisor.mode !== "act" + || !isHarnessStepAgentStage(stage) + || state.cancelRequested + || state.stopRequest !== undefined + || describeStageOverspend(harnessConfig, state.lastStageUsage, stage) !== undefined + ) { + yield failure; + return false; + } + const current = stageAgentOf(stage, harnessConfig, state) || DEFAULT_AGENT_ID; + const decision = superviseFailure({ + stage, + current, + tried: state.triedAgents.get(stage) ?? [current], + diagnosis: failure.diagnosis, + supervisor, + }); + const goesOn = decision.action !== "none" && attempt < maxAttempts; + const because = decision.action === "none" + ? decision.why + : goesOn + ? decision.because + : `${decision.because}, but ${stage} has had ${attempt} attempt(s), the maximum configured (maxStageAttempts: ${maxAttempts})`; + const action = goesOn ? decision.action : "none"; + const to = goesOn && decision.action === "move" ? decision.agent : undefined; + const message = action === "move" + ? `the supervisor moved ${stage} from ${current} to ${to}: ${because}` + : action === "repeat" + ? `the supervisor repeats ${stage} on ${current}: ${because}` + : `the supervisor did not try ${stage} again: ${because}`; + yield { kind: "progress", runId, timestamp: nowIso(), message }; + this.deps.auditLog?.record({ + runId, + agent: SUPERVISOR_AGENT_NAME, + outcome: "message", + cwd, + timestamp: nowIso(), + changeDir: context.changeDir, + stage, + reason: message, + supervisorDecision: { action, stage, from: current, ...(to !== undefined ? { to } : {}), because }, + }); + if (!goesOn) { + yield failure; + return false; + } + if (to !== undefined) state.agentOverride.set(stage, to); + yield { + kind: "stageStarted", + runId, + timestamp: nowIso(), + stage, + agentId: stageAgentOf(stage, harnessConfig, state), + attempt: attempt + 1, + previousAttemptReason: message, + }; + return true; + } + private async *runStage( stage: ChainStage, hasNextStage: boolean, @@ -1576,7 +1692,7 @@ export class HarnessChainRunner { // Recorded before the gate below, which returns without invoking // the verifying agent — so the failing case, which records // nothing today, is exactly the one this must not miss. - this.recordVerifyChecks(command, verifyCheckOutcome, harnessConfig); + this.recordVerifyChecks(command, verifyCheckOutcome, harnessConfig, state); } catch (error) { yield failedEvent(runId, error instanceof Error ? error.message : String(error)); return "failed"; @@ -1595,15 +1711,24 @@ export class HarnessChainRunner { } } + // A stage the supervisor moved runs its new agent with that agent's own + // defaults: the old entry's model, effort, budget and custom agent were + // chosen for another agent (the-supervisor-changes-agents). const stepAgent = harnessConfig.stepAgents[stage]; - const { agent: agentId, model, effort, budget, customAgent } = stepAgent === undefined - ? { agent: undefined, model: undefined, effort: undefined, budget: undefined, customAgent: undefined } - : normalizeStepAgent(stepAgent); + const movedTo = state.agentOverride.get(stage); + const { agent: agentId, model, effort, budget, customAgent } = movedTo !== undefined + ? { agent: movedTo, model: undefined, effort: undefined, budget: undefined, customAgent: undefined } + : stepAgent === undefined + ? { agent: undefined, model: undefined, effort: undefined, budget: undefined, customAgent: undefined } + : normalizeStepAgent(stepAgent); const runner = this.deps.resolveRunner(agentId); if (!runner) { yield failedEvent(runId, `no agent available to run the "${stage}" stage`); return "failed"; } + const ranOn = agentId ?? DEFAULT_AGENT_ID; + const tried = state.triedAgents.get(stage) ?? []; + if (!tried.includes(ranOn)) state.triedAgents.set(stage, [...tried, ranOn]); // Only the "verify" stage's context carries a delta and/or an // "established checks" section — every other stage keeps the exact @@ -1750,7 +1875,14 @@ export class HarnessChainRunner { } } if (event.kind === "usageReported") state.lastStageUsage = event.usage; - if (event.kind === "failed") outcome = "failed"; + if (event.kind === "failed") { + // Held, not yielded: only the attempt loop knows whether the + // supervisor tries the stage again, and a stage that goes on must + // not have said `failed` (the-supervisor-changes-agents). + outcome = "failed"; + state.heldFailure = event; + continue; + } if (event.kind === "cancelled") { outcome = "cancelled"; // The runner reports that its process is gone; only the chain @@ -1916,6 +2048,7 @@ export class HarnessChainRunner { command: Command, outcome: MechanicalCheckRunOutcome, harnessConfig: HarnessConfig, + state: Pick, ): void { if (!outcome.ranAny) return; const failures = outcome.failed @@ -1929,7 +2062,7 @@ export class HarnessChainRunner { timestamp: nowIso(), changeDir: command.context.changeDir, stage: "verify", - checkedAgent: resolvedApplyAgent(harnessConfig), + checkedAgent: resolvedApplyAgent(harnessConfig, state), checksRan: outcome.passed.length + outcome.failed.length, checksFailed: outcome.failed.length, ...(failures.length > 0 ? { reason: failures } : {}), diff --git a/packages/core/src/harness-config-schema.ts b/packages/core/src/harness-config-schema.ts index 38e05ebe..4a3908f7 100644 --- a/packages/core/src/harness-config-schema.ts +++ b/packages/core/src/harness-config-schema.ts @@ -305,11 +305,26 @@ function topLevel(scope: HarnessSchemaScope): Record { supervisor: { type: "object", additionalProperties: false, - description: "What the supervisor points out about runs (ADR 0039). It suggests and changes nothing.", + description: "What the supervisor points out about runs, and, under act in a change's own file, what it may do when a stage fails (ADR 0039).", properties: { - mode: { type: "string", enum: [...SUPERVISOR_MODES], description: "advise (absent) or off." }, + mode: scope === "change" + ? { type: "string", enum: [...SUPERVISOR_MODES], description: "advise (absent), off, or act: repeat or move a failed stage, which needs autonomyLevel autonomous and maxStageAttempts above 1." } + : { type: "string", enum: SUPERVISOR_MODES.filter((mode) => mode !== "act"), description: "advise (absent) or off. act is set per change." }, silentAfterSeconds: positiveInteger("Seconds a live run may say nothing new before it is pointed out. Absent means 600."), waitingAfterSeconds: positiveInteger("Seconds a run may wait on a person before it is pointed out. Absent means 60."), + fallback: { + type: "object", + additionalProperties: false, + description: "The agents a stage may move to, in order, where its agent fails for a cause repeating cannot fix. Used only under act.", + properties: Object.fromEntries(STAGES.filter(isHarnessStepAgentStage).map((stage) => [stage, { + type: "array", + minItems: 1, + uniqueItems: true, + items: { type: "string", enum: AGENT_REGISTRY.map((agent) => agent.id) }, + }])), + }, + allowCostIncrease: perChangeOnly(scope, { type: "boolean", description: "Whether a move may be to an agent that is not the local model. Absent means false." }, "It may cost money."), + allowProviderChange: perChangeOnly(scope, { type: "boolean", description: "Whether a move may be to an agent of another provider. Absent means false." }, "It sends the change's files to another company."), }, }, allowAgentMessages: { type: "boolean", description: "Whether a run takes notes written by another run. Absent means false." }, diff --git a/packages/core/src/harness-config.test.ts b/packages/core/src/harness-config.test.ts index 8e9d3896..00300497 100644 --- a/packages/core/src/harness-config.test.ts +++ b/packages/core/src/harness-config.test.ts @@ -1002,7 +1002,14 @@ describe("supervisor", () => { it("advises with 600 and 60 seconds where nothing says otherwise", async () => { const root = await temporaryRoot(); expect(resolveSupervisor((await resolveHarnessConfig(root)).supervisor)) - .toEqual({ mode: "advise", silentAfterSeconds: 600, waitingAfterSeconds: 60 }); + .toEqual({ + mode: "advise", + silentAfterSeconds: 600, + waitingAfterSeconds: 60, + fallback: {}, + allowCostIncrease: false, + allowProviderChange: false, + }); }); it("is turned off by either file", async () => { @@ -1021,13 +1028,13 @@ describe("supervisor", () => { await writeChangeHarnessConfig(root, "demo", { supervisor: { silentAfterSeconds: 120 } }); expect(resolveSupervisor((await resolveHarnessConfig(root, "demo")).supervisor)) - .toEqual({ mode: "off", silentAfterSeconds: 120, waitingAfterSeconds: 30 }); + .toMatchObject({ mode: "off", silentAfterSeconds: 120, waitingAfterSeconds: 30 }); }); it("refuses a mode this version does not have, naming the field", async () => { const root = await temporaryRoot(); - await expect(writeChangeHarnessConfig(root, "demo", { supervisor: { mode: "act" as never } })) - .rejects.toThrow(/supervisor\.mode must be one of: advise, off/); + await expect(writeChangeHarnessConfig(root, "demo", { supervisor: { mode: "decide" as never } })) + .rejects.toThrow(/supervisor\.mode must be one of: advise, off, act/); }); it("refuses a threshold that is not a positive whole number, and a key it does not have", async () => { @@ -1041,6 +1048,71 @@ describe("supervisor", () => { }); }); +// the-supervisor-changes-agents 1.2 +describe("supervisor act", () => { + const act = { + mode: "act" as const, + fallback: { apply: ["claude-cli-acp", "local-llm"] }, + allowCostIncrease: true, + allowProviderChange: true, + }; + + it("is refused in the workspace's file", async () => { + const root = await temporaryRoot(); + await expect(writeGlobalHarnessConfig(root, { maxStageAttempts: 2, supervisor: { mode: "act" } })) + .rejects.toThrow(/supervisor\.mode "act" is set in a change's own harness\.json/); + }); + + it("is refused without autonomous in the same file", async () => { + const root = await temporaryRoot(); + await expect(writeChangeHarnessConfig(root, "demo", { maxStageAttempts: 2, supervisor: act })) + .rejects.toThrow(/needs autonomyLevel "autonomous" in the same file/); + await expect(writeChangeHarnessConfig(root, "demo", { autonomyLevel: "semi-autonomous", maxStageAttempts: 2, supervisor: act })) + .rejects.toThrow(/needs autonomyLevel "autonomous" in the same file/); + }); + + it("is refused where the stage has one attempt", async () => { + const root = await temporaryRoot(); + await writeChangeHarnessConfig(root, "demo", { autonomyLevel: "autonomous", supervisor: act }); + await expect(resolveHarnessConfig(root, "demo")).rejects.toThrow(/maxStageAttempts is 1/); + }); + + it("is accepted with autonomous, attempts and a fallback, and resolves to them", async () => { + const root = await temporaryRoot(); + await writeChangeHarnessConfig(root, "demo", { autonomyLevel: "autonomous", maxStageAttempts: 2, supervisor: act }); + expect(resolveSupervisor((await resolveHarnessConfig(root, "demo")).supervisor)).toEqual({ + mode: "act", + silentAfterSeconds: 600, + waitingAfterSeconds: 60, + fallback: { apply: ["claude-cli-acp", "local-llm"] }, + allowCostIncrease: true, + allowProviderChange: true, + }); + }); + + it("refuses a fallback with an unknown agent, the editor's chat, a repeated id, or a stage without an agent", async () => { + const root = await temporaryRoot(); + const write = (fallback: unknown) => writeChangeHarnessConfig(root, "demo", { + autonomyLevel: "autonomous", + maxStageAttempts: 2, + supervisor: { mode: "act", fallback } as never, + }); + await expect(write({ apply: ["no-such-agent"] })).rejects.toThrow(/not a registered agent/); + await expect(write({ apply: ["vscode-chat"] })).rejects.toThrow(/may not name vscode-chat/); + await expect(write({ apply: ["claude-cli-acp", "claude-cli-acp"] })).rejects.toThrow(/names "claude-cli-acp" twice/); + await expect(write({ archive: ["claude-cli-acp"] })).rejects.toThrow(/fallback\.archive names no stage that runs an agent/); + await expect(write({ apply: [] })).rejects.toThrow(/non-empty list/); + }); + + it("refuses the allowances in the workspace's file", async () => { + const root = await temporaryRoot(); + await expect(writeGlobalHarnessConfig(root, { supervisor: { allowCostIncrease: true } } as never)) + .rejects.toThrow(/supervisor\.allowCostIncrease is set in a change's own harness\.json/); + await expect(writeGlobalHarnessConfig(root, { supervisor: { allowProviderChange: true } } as never)) + .rejects.toThrow(/supervisor\.allowProviderChange is set in a change's own harness\.json/); + }); +}); + describe("every accepted key survives a round trip (config-keys-survive-a-round-trip)", () => { /** A representative value per accepted top-level key. * diff --git a/packages/core/src/harness-config.ts b/packages/core/src/harness-config.ts index 9d927ae0..5c0db640 100644 --- a/packages/core/src/harness-config.ts +++ b/packages/core/src/harness-config.ts @@ -86,9 +86,11 @@ export interface HarnessHints { enabled: boolean; } -/** What the supervisor does (ADR 0039, the-supervisor-advises). */ -export type HarnessSupervisorMode = "off" | "advise"; -export const SUPERVISOR_MODES: readonly HarnessSupervisorMode[] = ["advise", "off"]; +/** What the supervisor does (ADR 0039, the-supervisor-advises). `act` also + * repeats or moves a failed stage of a chain, within the change's policy + * (the-supervisor-changes-agents). */ +export type HarnessSupervisorMode = "off" | "advise" | "act"; +export const SUPERVISOR_MODES: readonly HarnessSupervisorMode[] = ["advise", "off", "act"]; /** The supervisor: rules over the runs' records that suggest and change * nothing. Every field optional; see `resolveSupervisor` for what absent @@ -101,27 +103,61 @@ export interface HarnessSupervisor { silentAfterSeconds?: number; /** How long a run may wait on a person before it is pointed out. */ waitingAfterSeconds?: number; + /** The agents a stage may move to, in order, where its agent fails for a + * cause repeating cannot fix. Used only under `act` + * (the-supervisor-changes-agents). */ + fallback?: Partial>; + /** Whether a move may be to an agent that is not the local model, which + * may cost money. Per change only; absent is false. */ + allowCostIncrease?: boolean; + /** Whether a move may be to an agent of another provider, which sends the + * change's files to another company. Per change only; absent is false. */ + allowProviderChange?: boolean; } export interface ResolvedSupervisor { mode: HarnessSupervisorMode; silentAfterSeconds: number; waitingAfterSeconds: number; + fallback: Partial>; + allowCostIncrease: boolean; + allowProviderChange: boolean; } -export const DEFAULT_SUPERVISOR: ResolvedSupervisor = { mode: "advise", silentAfterSeconds: 600, waitingAfterSeconds: 60 }; +export const DEFAULT_SUPERVISOR: ResolvedSupervisor = { + mode: "advise", + silentAfterSeconds: 600, + waitingAfterSeconds: 60, + fallback: {}, + allowCostIncrease: false, + allowProviderChange: false, +}; -/** The supervisor a configuration resolves to: `advise`, 600 and 60 where - * it says nothing, which is what every configuration written before the - * key existed says. */ +/** The supervisor a configuration resolves to: `advise`, 600 and 60, no + * fallback and nothing allowed where it says nothing, which is what every + * configuration written before the keys existed says. */ export function resolveSupervisor(value: HarnessSupervisor | undefined): ResolvedSupervisor { return { mode: value?.mode ?? DEFAULT_SUPERVISOR.mode, silentAfterSeconds: value?.silentAfterSeconds ?? DEFAULT_SUPERVISOR.silentAfterSeconds, waitingAfterSeconds: value?.waitingAfterSeconds ?? DEFAULT_SUPERVISOR.waitingAfterSeconds, + fallback: value?.fallback ?? {}, + allowCostIncrease: value?.allowCostIncrease === true, + allowProviderChange: value?.allowProviderChange === true, }; } +/** Why a resolved configuration's `act` cannot act, or `undefined`. Checked + * where the configuration resolves, because the attempts it spends may come + * from the other file (the-supervisor-changes-agents). */ +export function supervisorActProblem(config: Pick): string | undefined { + if (config.supervisor?.mode !== "act") return undefined; + const attempts = config.maxStageAttempts ?? 1; + return attempts > 1 + ? undefined + : `supervisor.mode "act" repeats or moves a failed stage as another attempt of it, and maxStageAttempts is ${attempts}: set it above 1 in this change's harness.json, or choose "advise"`; +} + /** A cost/token ceiling `HarnessChainRunner` checks before starting each * stage of a chain — see openspec/changes/agent-usage-accounting/design.md * and spec.md, "A configured budget stops work at stage boundaries". Both @@ -862,13 +898,19 @@ function assertValidHints(value: unknown): asserts value is HarnessHints | undef } } -const SUPERVISOR_KEYS = ["mode", "silentAfterSeconds", "waitingAfterSeconds"] as const; +const SUPERVISOR_KEYS = ["mode", "silentAfterSeconds", "waitingAfterSeconds", "fallback", "allowCostIncrease", "allowProviderChange"] as const; -/** Structural, and accepted in both files: advising writes nothing and - * spends nothing, so there is nothing a workspace may not decide. A mode - * this version does not know is refused rather than read as `advise`: a - * setting that cannot act is not accepted quietly (ADR 0039). */ -function assertValidSupervisor(value: unknown): asserts value is HarnessSupervisor | undefined { +/** Advising writes nothing and spends nothing, so a workspace may decide it. + * Acting may start another agent, so it is a change's own decision, made + * where the change runs unattended (ADR 0018's rule for what widens + * authority), and so are the two allowances. A mode this version does not + * know is refused rather than read as `advise`: a setting that cannot act is + * not accepted quietly (ADR 0039). */ +function assertValidSupervisor( + value: unknown, + isPerChangeFile: boolean, + autonomyLevel: HarnessAutonomyLevel | undefined, +): asserts value is HarnessSupervisor | undefined { if (value === undefined) return; if (typeof value !== "object" || value === null || Array.isArray(value)) { throw new InvalidHarnessConfigError("supervisor must be an object"); @@ -882,12 +924,55 @@ function assertValidSupervisor(value: unknown): asserts value is HarnessSupervis if (record.mode !== undefined && !SUPERVISOR_MODES.includes(record.mode as HarnessSupervisorMode)) { throw new InvalidHarnessConfigError(`supervisor.mode must be one of: ${SUPERVISOR_MODES.join(", ")}`); } + if (record.mode === "act") { + if (!isPerChangeFile) { + throw new InvalidHarnessConfigError('supervisor.mode "act" is set in a change\'s own harness.json, never for the whole workspace: it may start another agent unattended'); + } + if (autonomyLevel !== "autonomous") { + throw new InvalidHarnessConfigError('supervisor.mode "act" needs autonomyLevel "autonomous" in the same file: under a level that asks a person, the person decides'); + } + } for (const key of ["silentAfterSeconds", "waitingAfterSeconds"] as const) { const seconds = record[key]; if (seconds !== undefined && (typeof seconds !== "number" || !Number.isInteger(seconds) || seconds <= 0)) { throw new InvalidHarnessConfigError(`supervisor.${key} must be a positive whole number of seconds`); } } + for (const key of ["allowCostIncrease", "allowProviderChange"] as const) { + if (record[key] === undefined) continue; + if (typeof record[key] !== "boolean") throw new InvalidHarnessConfigError(`supervisor.${key} must be a boolean`); + if (!isPerChangeFile) { + throw new InvalidHarnessConfigError(`supervisor.${key} is set in a change's own harness.json, never for the whole workspace`); + } + } + if (record.fallback !== undefined) assertValidFallback(record.fallback); +} + +/** A stage's fallback agents: registered, not the editor's chat, each once, + * for a stage that runs an agent. */ +function assertValidFallback(value: unknown): void { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + throw new InvalidHarnessConfigError("supervisor.fallback must be an object of stages"); + } + for (const [stage, agents] of Object.entries(value as Record)) { + if (!(STAGES as readonly string[]).includes(stage) || !isHarnessStepAgentStage(stage as HarnessStage)) { + throw new InvalidHarnessConfigError(`supervisor.fallback.${stage} names no stage that runs an agent`); + } + if (!Array.isArray(agents) || agents.length === 0 || agents.some((agent) => typeof agent !== "string")) { + throw new InvalidHarnessConfigError(`supervisor.fallback.${stage} must be a non-empty list of agent ids`); + } + const seen = new Set(); + for (const agent of agents as string[]) { + if (agent === VSCODE_CHAT_STEP_AGENT_ID) { + throw new InvalidHarnessConfigError(`supervisor.fallback.${stage} may not name ${agent}: a chain cannot hand a stage it attempts again to the editor's chat`); + } + if (!AGENT_REGISTRY.some((descriptor) => descriptor.id === agent)) { + throw new InvalidHarnessConfigError(`supervisor.fallback.${stage} names "${agent}", which is not a registered agent`); + } + if (seen.has(agent)) throw new InvalidHarnessConfigError(`supervisor.fallback.${stage} names "${agent}" twice`); + seen.add(agent); + } + } } function assertValidCheckpoints( @@ -1097,7 +1182,7 @@ function assertValidHarnessConfigInput( assertValidReviewGate(input.reviewGate, isPerChangeFile); assertValidCheckpoints(input.checkpoints, isPerChangeFile); assertValidHints((input as { hints?: unknown }).hints); - assertValidSupervisor((input as { supervisor?: unknown }).supervisor); + assertValidSupervisor((input as { supervisor?: unknown }).supervisor, isPerChangeFile, input.autonomyLevel); assertValidMaxCost((input as { budget?: { maxCost?: unknown } }).budget?.maxCost); const allowAgentMessages = (input as { allowAgentMessages?: unknown }).allowAgentMessages; if (allowAgentMessages !== undefined && typeof allowAgentMessages !== "boolean") { @@ -1405,6 +1490,11 @@ export async function resolveHarnessConfig(workspaceRoot: string, changeName?: s } }); + // `act` spends attempts, which may be set in either file + // (the-supervisor-changes-agents). + const actProblem = supervisorActProblem(merged); + if (actProblem !== undefined) throw new InvalidHarnessConfigError(actProblem); + return merged; } diff --git a/packages/core/src/last-runs-facts.ts b/packages/core/src/last-runs-facts.ts index 382754d5..4bbde92e 100644 --- a/packages/core/src/last-runs-facts.ts +++ b/packages/core/src/last-runs-facts.ts @@ -19,6 +19,9 @@ export interface LastRun { /** What is known about why it failed, where it failed and the log says * (the-supervisor-advises). */ diagnosis?: FailureDiagnosis; + /** The agent the failed stage ran on, where it failed on one + * (the-supervisor-changes-agents). */ + agent?: string; /** The sum of what the run's entries reported spending; absent when none * reported a cost, which is not the same as costing nothing. */ costUsd?: number; diff --git a/packages/core/src/last-runs.test.ts b/packages/core/src/last-runs.test.ts index 5c83de25..13a0d0aa 100644 --- a/packages/core/src/last-runs.test.ts +++ b/packages/core/src/last-runs.test.ts @@ -53,6 +53,9 @@ describe("lastRunsOf", () => { stage: "verify", endedAt: "2026-09-14T10:06:00.010Z", reason: "verify: the agent gave up", + // The failed stage's agent, for the supervisor's fallback policy + // (the-supervisor-changes-agents). + agent: "claude-cli", costUsd: 0.75, }); }); diff --git a/packages/core/src/last-runs.ts b/packages/core/src/last-runs.ts index 89f14f03..4c356e2d 100644 --- a/packages/core/src/last-runs.ts +++ b/packages/core/src/last-runs.ts @@ -9,6 +9,8 @@ import { CHAIN_ENDING_AGENT_NAME, changeNameOf, isRunEntry } from "./audit-runs.js"; import { readFailureDiagnosis } from "./failure-diagnosis.js"; +import { AGENT_REGISTRY } from "./agents/registry.js"; + import { createGitWrapper, type GitWrapper } from "./git.js"; import type { LastRun, LastRunsReport } from "./last-runs-facts.js"; import { readRepositoryAuditEntries, type AuditReadCache } from "./repository-audit.js"; @@ -16,6 +18,10 @@ import type { AuditEntry } from "./security.js"; export type { LastRun, LastRunsReport } from "./last-runs-facts.js"; +/** The agents a run can be moved between: an entry from anything else is not + * the stage's agent. */ +const REGISTERED: ReadonlySet = new Set(AGENT_REGISTRY.map((descriptor) => descriptor.id)); + /** Lives for the process: the Pipeline asks on every survey, and an * unchanged log is not parsed twice. */ const logCache: AuditReadCache = new Map(); @@ -79,6 +85,11 @@ function endingOf(runId: string, group: readonly AuditEntry[]): LastRun | undefi ? undefined : readFailureDiagnosis(ending.diagnosis) ?? readFailureDiagnosis([...stageEntries].reverse().find((entry) => entry.outcome === "failed")?.diagnosis); + // The registered agent the run failed on, for a policy that asks whether a + // fallback is another provider (the-supervisor-changes-agents). + const agent = ending.outcome !== "failed" + ? undefined + : [...stageEntries].reverse().find((entry) => entry.outcome === "failed" && REGISTERED.has(entry.agent))?.agent; const costs = stageEntries .map((entry) => entry.usage?.costUsd) .filter((cost): cost is number => typeof cost === "number"); @@ -89,6 +100,7 @@ function endingOf(runId: string, group: readonly AuditEntry[]): LastRun | undefi ...(stage !== undefined ? { stage } : {}), ...(ending.reason !== undefined ? { reason: ending.reason } : {}), ...(diagnosis !== undefined ? { diagnosis } : {}), + ...(agent !== undefined ? { agent } : {}), ...(costs.length > 0 ? { costUsd: costs.reduce((sum, cost) => sum + cost, 0) } : {}), }; } diff --git a/packages/core/src/security.ts b/packages/core/src/security.ts index deee9624..b6396fe3 100644 --- a/packages/core/src/security.ts +++ b/packages/core/src/security.ts @@ -452,6 +452,17 @@ export interface AuditEntry { operatorMessage?: { messageId: string; kind: "note" | "ask"; from: string; stage: string }; /** A message addressed to this run that it did not take, and why. */ operatorMessageRefused?: { messageId: string; why: "unverified" | "stale" | "seen" | "author-not-allowed" }; + /** What the supervisor did about a failed stage under `act`, on an entry + * from `supervisor` whose outcome is `message` + * (the-supervisor-changes-agents): repeated it on the same agent, moved it + * to another, or left the failure as it was, with why. */ + supervisorDecision?: { + action: "repeat" | "move" | "none"; + stage: string; + from: string; + to?: string; + because: string; + }; } export interface AuditLog { diff --git a/packages/core/src/supervisor.test.ts b/packages/core/src/supervisor.test.ts index b440db73..6ee06336 100644 --- a/packages/core/src/supervisor.test.ts +++ b/packages/core/src/supervisor.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest"; import type { AgentStatusReport } from "./agent-status.js"; import { DEFAULT_SUPERVISOR, type ResolvedSupervisor } from "./harness-config.js"; import type { LastRun } from "./last-runs-facts.js"; -import { describeSpan, superviseRuns, type SuperviseInputs } from "./supervisor.js"; +import { describeSpan, superviseFailure, superviseRuns, type FailureFactsForDecision, type SuperviseInputs } from "./supervisor.js"; // the-supervisor-advises 3.2: pure over records, so every case is a record. @@ -184,3 +184,87 @@ describe("describeSpan", () => { expect(describeSpan(150 * MINUTE)).toBe("2 hours"); }); }); + +// the-supervisor-changes-agents 1.3 +describe("superviseFailure", () => { + const notSignedIn = { cause: "not-signed-in", repeatHelps: "no" } as const; + + function facts(partial: Partial & { policy?: Partial } = {}): FailureFactsForDecision { + return { + stage: "apply", + current: "copilot-cli-acp", + tried: partial.tried ?? [partial.current ?? "copilot-cli-acp"], + diagnosis: notSignedIn, + supervisor: { + fallback: { apply: ["claude-cli-acp", "local-llm-acp"] }, + allowCostIncrease: true, + allowProviderChange: true, + ...partial.policy, + }, + ...partial, + }; + } + + it("repeats on the same agent where repeating is likely to help", () => { + expect(superviseFailure(facts({ diagnosis: { cause: "rate-limited", repeatHelps: "likely" } }))) + .toEqual({ action: "repeat", agent: "copilot-cli-acp", because: expect.stringContaining("rate") }); + }); + + it("moves to the first fallback the policy allows that was not tried", () => { + expect(superviseFailure(facts())).toMatchObject({ action: "move", agent: "claude-cli-acp" }); + expect(superviseFailure(facts({ tried: ["copilot-cli-acp", "claude-cli-acp"] }))) + .toMatchObject({ action: "move", agent: "local-llm-acp" }); + }); + + it("refuses a provider change, and names the rule", () => { + const decision = superviseFailure(facts({ policy: { fallback: { apply: ["claude-cli-acp"] }, allowProviderChange: false } })); + expect(decision).toEqual({ action: "none", why: expect.stringContaining("claude-cli-acp is another provider (anthropic), and allowProviderChange is false") }); + }); + + it("refuses a cost increase, and names the rule", () => { + const decision = superviseFailure(facts({ policy: { fallback: { apply: ["copilot-cli"] }, allowCostIncrease: false } })); + expect(decision).toEqual({ action: "none", why: expect.stringContaining("copilot-cli may cost money, and allowCostIncrease is false") }); + }); + + it("allows the local model without allowCostIncrease, and names the fallback it passed over", () => { + expect(superviseFailure(facts({ policy: { allowCostIncrease: false } }))).toEqual({ + action: "move", + agent: "local-llm-acp", + because: expect.stringContaining("passed over: claude-cli-acp may cost money, and allowCostIncrease is false"), + }); + }); + + it("tries nothing twice", () => { + expect(superviseFailure(facts({ tried: ["copilot-cli-acp", "claude-cli-acp", "local-llm-acp"] }))) + .toEqual({ action: "none", why: expect.stringContaining("every fallback for apply has been tried") }); + }); + + it("does nothing where the cause is unknown, or the stage has no fallback", () => { + expect(superviseFailure(facts({ diagnosis: { cause: "unknown", repeatHelps: "unknown" } }))).toMatchObject({ action: "none" }); + expect(superviseFailure(facts({ diagnosis: undefined }))).toMatchObject({ action: "none" }); + expect(superviseFailure(facts({ policy: { fallback: {} } }))) + .toEqual({ action: "none", why: expect.stringContaining("apply names no fallback agent") }); + }); +}); + +// the-supervisor-changes-agents 1.5 +describe("superviseRuns — the fallback under advise", () => { + it("names the fallback act would move the stage to, and changes nothing", () => { + const hints = superviseRuns(inputs({ + lastRuns: { byChange: { demo: failed({ agent: "copilot-cli-acp" }) } }, + supervisor: { fallback: { apply: ["claude-cli-acp"] }, allowCostIncrease: true, allowProviderChange: true }, + })); + expect(hints).toHaveLength(1); + expect(hints[0]?.because).toContain("Its fallback for apply, claude-cli-acp, is allowed by this change's policy"); + expect(hints[0]?.because).toContain('under supervisor.mode "act" a chain would move the stage to it'); + expect(hints[0]?.commands).toEqual(["copilot"]); + }); + + it("says nothing of a fallback the policy refuses", () => { + const hints = superviseRuns(inputs({ + lastRuns: { byChange: { demo: failed({ agent: "copilot-cli-acp" }) } }, + supervisor: { fallback: { apply: ["claude-cli-acp"] } }, + })); + expect(hints[0]?.because).not.toContain("fallback"); + }); +}); diff --git a/packages/core/src/supervisor.ts b/packages/core/src/supervisor.ts index d09698f0..2c581579 100644 --- a/packages/core/src/supervisor.ts +++ b/packages/core/src/supervisor.ts @@ -21,8 +21,10 @@ // outside; it says how long, and what the run last said (ADR 0028). import type { AgentStatusReport } from "./agent-status.js"; -import { describeDiagnosis } from "./failure-diagnosis.js"; +import { AGENT_REGISTRY, type AgentProvider } from "./agents/registry.js"; +import { describeDiagnosis, type FailureDiagnosis } from "./failure-diagnosis.js"; import type { ResolvedSupervisor } from "./harness-config.js"; +import type { HarnessStepAgentStage } from "./harness-step-agent.js"; import type { Hint } from "./hints.js"; import type { LastRunsReport } from "./last-runs-facts.js"; import { describeWaiting } from "./worktree-survey-facts.js"; @@ -115,12 +117,22 @@ function aboutLastRuns(inputs: SuperviseInputs): Hint[] { if (inputs.supervisorFor(changeName).mode === "off") continue; const at = run.stage !== undefined ? ` at ${run.stage}` : ""; const evidence = diagnosis.evidence !== undefined ? ` It printed: "${diagnosis.evidence}".` : ""; + // The agent `act` would move the stage to, where the change's policy + // allows one; said, never done (the-supervisor-changes-agents). + const supervisor = inputs.supervisorFor(changeName); + const move = run.stage !== undefined && run.agent !== undefined && isHarnessStepAgentStageName(run.stage) + ? superviseFailure({ stage: run.stage, current: run.agent, tried: [run.agent], diagnosis, supervisor }) + : undefined; + const fallback = move?.action === "move" + ? ` Its fallback for ${run.stage}, ${move.agent}, is allowed by this change's policy${supervisor.mode === "act" ? "" : "; under supervisor.mode \"act\" a chain would move the stage to it"}.` + : ""; hints.push({ id: `last-run-cannot-be-repeated:${changeName}:${run.runId}`, kind: "last-run-cannot-be-repeated", subject: `${changeName}'s last run failed${at}, and repeating it will not help`, because: `${capitalised(describeDiagnosis(diagnosis))}.${evidence}` - + (diagnosis.remedy !== undefined ? ` ${diagnosis.remedy}` : ""), + + (diagnosis.remedy !== undefined ? ` ${diagnosis.remedy}` : "") + + fallback, commands: [...(diagnosis.commands ?? [])], }); } @@ -131,6 +143,89 @@ function capitalised(text: string): string { return text.length === 0 ? text : `${text[0]?.toUpperCase() ?? ""}${text.slice(1)}`; } +const AGENT_STAGES: ReadonlySet = new Set(["propose", "review", "apply", "verify"]); + +function isHarnessStepAgentStageName(stage: string): stage is HarnessStepAgentStage { + return AGENT_STAGES.has(stage); +} + +/** What the supervisor does about a failed stage under `act` + * (the-supervisor-changes-agents, ADR 0039 decision 4). */ +export type FailureDecision = + | { action: "repeat"; agent: string; because: string } + | { action: "move"; agent: string; because: string } + | { action: "none"; why: string }; + +export interface FailureFactsForDecision { + stage: HarnessStepAgentStage; + /** The agent the stage failed on. */ + current: string; + /** Every agent this stage has run on in this chain, the current one + * included: a fallback is never tried twice. */ + tried: readonly string[]; + diagnosis: FailureDiagnosis | undefined; + supervisor: Pick; +} + +function providerOf(agent: string): AgentProvider | undefined { + return AGENT_REGISTRY.find((descriptor) => descriptor.id === agent)?.provider; +} + +/** Why the policy refuses a move from `current` to `candidate`, or + * `undefined` where it allows it. Any move but one to the local model may + * cost more: nothing here knows one service's price against another's. */ +export function fallbackRefusal( + current: string, + candidate: string, + supervisor: Pick, +): string | undefined { + const from = providerOf(current); + const to = providerOf(candidate); + if (to === undefined) return `${candidate} is not a registered agent`; + if (to !== from && !supervisor.allowProviderChange) { + return `${candidate} is another provider (${to}), and allowProviderChange is false`; + } + if (to !== "local" && !supervisor.allowCostIncrease) { + return `${candidate} may cost money, and allowCostIncrease is false`; + } + return undefined; +} + +/** Repeat the stage, move it to a fallback, or leave the failure as it is. + * A pure function of what the chain already holds: the failure's + * diagnosis, the stage's fallback list and the change's policy. */ +export function superviseFailure(facts: FailureFactsForDecision): FailureDecision { + const { diagnosis } = facts; + if (diagnosis === undefined || diagnosis.repeatHelps === "unknown") { + return { action: "none", why: "the cause is not known, so the stage is neither repeated nor moved" }; + } + const cause = describeDiagnosis(diagnosis); + if (diagnosis.repeatHelps === "likely") { + return { action: "repeat", agent: facts.current, because: `${cause}` }; + } + const fallback = facts.supervisor.fallback[facts.stage] ?? []; + if (fallback.length === 0) { + return { action: "none", why: `${cause}, and ${facts.stage} names no fallback agent` }; + } + const refusals: string[] = []; + for (const candidate of fallback) { + if (facts.tried.includes(candidate)) continue; + const refusal = fallbackRefusal(facts.current, candidate, facts.supervisor); + // A fallback passed over on the way is named too: a move that skipped + // one says why, not only where it went. + if (refusal === undefined) { + return { action: "move", agent: candidate, because: refusals.length > 0 ? `${cause} (passed over: ${refusals.join("; ")})` : cause }; + } + refusals.push(refusal); + } + return { + action: "none", + why: refusals.length > 0 + ? `${cause}, and the policy refuses every fallback left: ${refusals.join("; ")}` + : `${cause}, and every fallback for ${facts.stage} has been tried`, + }; +} + /** What the supervisor points out, in the order a person deals with it: * runs first, since they are spending now, then changes whose last run * cannot simply be tried again. */ diff --git a/packages/extension/schemas/agent-harness.schema.json b/packages/extension/schemas/agent-harness.schema.json index a9ea76b7..ab6c114b 100644 --- a/packages/extension/schemas/agent-harness.schema.json +++ b/packages/extension/schemas/agent-harness.schema.json @@ -179,7 +179,7 @@ "supervisor": { "type": "object", "additionalProperties": false, - "description": "What the supervisor points out about runs (ADR 0039). It suggests and changes nothing.", + "description": "What the supervisor points out about runs, and, under act in a change's own file, what it may do when a stage fails (ADR 0039).", "properties": { "mode": { "type": "string", @@ -187,7 +187,7 @@ "advise", "off" ], - "description": "advise (absent) or off." + "description": "advise (absent) or off. act is set per change." }, "silentAfterSeconds": { "type": "integer", @@ -198,6 +198,105 @@ "type": "integer", "minimum": 1, "description": "Seconds a run may wait on a person before it is pointed out. Absent means 60." + }, + "fallback": { + "type": "object", + "additionalProperties": false, + "description": "The agents a stage may move to, in order, where its agent fails for a cause repeating cannot fix. Used only under act.", + "properties": { + "propose": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "enum": [ + "claude-cli", + "copilot-cli", + "codex-cli", + "gemini-cli", + "local-llm", + "local-llm-acp", + "copilot-cli-acp", + "gemini-cli-acp", + "codex-cli-acp", + "deepseek-cli-acp", + "claude-cli-acp" + ] + } + }, + "review": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "enum": [ + "claude-cli", + "copilot-cli", + "codex-cli", + "gemini-cli", + "local-llm", + "local-llm-acp", + "copilot-cli-acp", + "gemini-cli-acp", + "codex-cli-acp", + "deepseek-cli-acp", + "claude-cli-acp" + ] + } + }, + "apply": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "enum": [ + "claude-cli", + "copilot-cli", + "codex-cli", + "gemini-cli", + "local-llm", + "local-llm-acp", + "copilot-cli-acp", + "gemini-cli-acp", + "codex-cli-acp", + "deepseek-cli-acp", + "claude-cli-acp" + ] + } + }, + "verify": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "enum": [ + "claude-cli", + "copilot-cli", + "codex-cli", + "gemini-cli", + "local-llm", + "local-llm-acp", + "copilot-cli-acp", + "gemini-cli-acp", + "codex-cli-acp", + "deepseek-cli-acp", + "claude-cli-acp" + ] + } + } + } + }, + "allowCostIncrease": { + "not": {}, + "description": "Only a change's own harness.json may set this. It may cost money." + }, + "allowProviderChange": { + "not": {}, + "description": "Only a change's own harness.json may set this. It sends the change's files to another company." } } }, diff --git a/packages/extension/schemas/change-harness.schema.json b/packages/extension/schemas/change-harness.schema.json index 0c254205..4b10cbe0 100644 --- a/packages/extension/schemas/change-harness.schema.json +++ b/packages/extension/schemas/change-harness.schema.json @@ -322,15 +322,16 @@ "supervisor": { "type": "object", "additionalProperties": false, - "description": "What the supervisor points out about runs (ADR 0039). It suggests and changes nothing.", + "description": "What the supervisor points out about runs, and, under act in a change's own file, what it may do when a stage fails (ADR 0039).", "properties": { "mode": { "type": "string", "enum": [ "advise", - "off" + "off", + "act" ], - "description": "advise (absent) or off." + "description": "advise (absent), off, or act: repeat or move a failed stage, which needs autonomyLevel autonomous and maxStageAttempts above 1." }, "silentAfterSeconds": { "type": "integer", @@ -341,6 +342,105 @@ "type": "integer", "minimum": 1, "description": "Seconds a run may wait on a person before it is pointed out. Absent means 60." + }, + "fallback": { + "type": "object", + "additionalProperties": false, + "description": "The agents a stage may move to, in order, where its agent fails for a cause repeating cannot fix. Used only under act.", + "properties": { + "propose": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "enum": [ + "claude-cli", + "copilot-cli", + "codex-cli", + "gemini-cli", + "local-llm", + "local-llm-acp", + "copilot-cli-acp", + "gemini-cli-acp", + "codex-cli-acp", + "deepseek-cli-acp", + "claude-cli-acp" + ] + } + }, + "review": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "enum": [ + "claude-cli", + "copilot-cli", + "codex-cli", + "gemini-cli", + "local-llm", + "local-llm-acp", + "copilot-cli-acp", + "gemini-cli-acp", + "codex-cli-acp", + "deepseek-cli-acp", + "claude-cli-acp" + ] + } + }, + "apply": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "enum": [ + "claude-cli", + "copilot-cli", + "codex-cli", + "gemini-cli", + "local-llm", + "local-llm-acp", + "copilot-cli-acp", + "gemini-cli-acp", + "codex-cli-acp", + "deepseek-cli-acp", + "claude-cli-acp" + ] + } + }, + "verify": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "enum": [ + "claude-cli", + "copilot-cli", + "codex-cli", + "gemini-cli", + "local-llm", + "local-llm-acp", + "copilot-cli-acp", + "gemini-cli-acp", + "codex-cli-acp", + "deepseek-cli-acp", + "claude-cli-acp" + ] + } + } + } + }, + "allowCostIncrease": { + "type": "boolean", + "description": "Whether a move may be to an agent that is not the local model. Absent means false." + }, + "allowProviderChange": { + "type": "boolean", + "description": "Whether a move may be to an agent of another provider. Absent means false." } } }, diff --git a/packages/webui/src/components/ChangeHarnessSettingsView.test.tsx b/packages/webui/src/components/ChangeHarnessSettingsView.test.tsx index 9d71a43e..408cdb01 100644 --- a/packages/webui/src/components/ChangeHarnessSettingsView.test.tsx +++ b/packages/webui/src/components/ChangeHarnessSettingsView.test.tsx @@ -286,6 +286,79 @@ describe("ChangeHarnessSettingsView — saving", () => { expect("supervisor" in saved).toBe(false); }); + // the-supervisor-changes-agents 2.1 + it("offers Act only where the change itself is autonomous", async () => { + const api = createApi(); + await renderLoaded(api); + + expect(radios("Change supervisor").map((radio) => radio.value)).not.toContain("act"); + expect(screen.getByTestId("change-supervisor-note").textContent).toContain("Act is offered where this change's own autonomy level is Autonomous"); + choose("Change autonomy level", "autonomous"); + expect(radios("Change supervisor").map((radio) => radio.value)).toContain("act"); + }); + + it("says what Act may start, and saves the fallback, the allowances and a second attempt", async () => { + const api = createApi({ readChangeOverride: vi.fn().mockResolvedValue({ stepAgents: {}, autonomyLevel: "autonomous" }) }); + await renderLoaded(api); + + choose("Change supervisor", "act"); + expect(screen.getByTestId("change-supervisor-act-warning").textContent).toContain("it may cost money"); + expect(screen.getByTestId("change-supervisor-note").textContent).toContain("attempted 2 times"); + // The same lists as the stage table's agents: one per fallback, and one + // more to add the next; the editor's chat is not offered. + const add = () => screen.getByRole("combobox", { name: "Add a fallback for apply" }) as HTMLSelectElement; + expect([...add().options].map((option) => option.value)).not.toContain("vscode-chat"); + fireEvent.change(add(), { target: { value: "claude-cli-acp" } }); + fireEvent.change(add(), { target: { value: "local-llm-acp" } }); + expect(screen.getByRole("combobox", { name: "Fallback 1 for apply" })).toHaveValue("claude-cli-acp"); + expect([...add().options].map((option) => option.value)).not.toContain("claude-cli-acp"); + fireEvent.click(screen.getByLabelText(/Allow a move that may cost more/)); + fireEvent.click(screen.getByLabelText("Allow a move to another provider")); + fireEvent.click(saveButton()); + + await waitFor(() => expect(api.writeChangeOverride).toHaveBeenCalledWith("demo", expect.objectContaining({ + autonomyLevel: "autonomous", + maxStageAttempts: 2, + supervisor: { + mode: "act", + fallback: { apply: ["claude-cli-acp", "local-llm-acp"] }, + allowCostIncrease: true, + allowProviderChange: true, + }, + }))); + }); + + it("removes a fallback when its list is set back to none", async () => { + const api = createApi({ + readChangeOverride: vi.fn().mockResolvedValue({ + stepAgents: {}, + autonomyLevel: "autonomous", + maxStageAttempts: 2, + supervisor: { mode: "act", fallback: { apply: ["claude-cli-acp", "local-llm-acp"] } }, + }), + }); + await renderLoaded(api); + + fireEvent.change(screen.getByRole("combobox", { name: "Fallback 1 for apply" }), { target: { value: "" } }); + expect(screen.getByRole("combobox", { name: "Fallback 1 for apply" })).toHaveValue("local-llm-acp"); + fireEvent.change(screen.getByRole("combobox", { name: "Fallback 1 for apply" }), { target: { value: "" } }); + fireEvent.click(saveButton()); + + await waitFor(() => expect(api.writeChangeOverride).toHaveBeenCalledWith("demo", expect.objectContaining({ supervisor: { mode: "act" } }))); + }); + + it("says Act will be refused once the change is no longer autonomous", async () => { + const api = createApi({ + readChangeOverride: vi.fn().mockResolvedValue({ stepAgents: {}, autonomyLevel: "autonomous", maxStageAttempts: 3, supervisor: { mode: "act" } }), + }); + await renderLoaded(api); + + expect(chosen("Change supervisor")).toBe("act"); + expect(screen.queryByTestId("change-supervisor-act-refused")).toBeNull(); + choose("Change autonomy level", ""); + expect(screen.getByTestId("change-supervisor-act-refused").textContent).toContain("saving will be refused"); + }); + it("still removes autonomyLevel when it is set back to inherit", async () => { const api = createApi({ readChangeOverride: vi.fn().mockResolvedValue({ stepAgents: {}, autonomyLevel: "autonomous" }) }); await renderLoaded(api); diff --git a/packages/webui/src/components/ChangeHarnessSettingsView.tsx b/packages/webui/src/components/ChangeHarnessSettingsView.tsx index dfbf4e6a..7e9ec231 100644 --- a/packages/webui/src/components/ChangeHarnessSettingsView.tsx +++ b/packages/webui/src/components/ChangeHarnessSettingsView.tsx @@ -42,6 +42,7 @@ import { stepAgentsFromForms, STAGES, SUPERVISOR_NOTES, + SupervisorActFields, supervisorModeFrom, toForm, withSupervisorMode, @@ -239,6 +240,10 @@ export function ChangeHarnessSettingsView( const supervisorMode = supervisorModeFrom(override); const inheritedSupervisor = supervisorModeFrom(base) || "advise"; + // Act is this change's to choose only where this change itself runs + // unattended, and it spends attempts (the-supervisor-changes-agents). + const ownAutonomous = autonomyLevel === "autonomous"; + const stageAttempts = override?.maxStageAttempts ?? base.maxStageAttempts ?? 1; const inheritedLevel = autonomyLevelParts(base.autonomyLevel); const shownLevel = autonomyLevel === INHERIT ? inheritedLevel : autonomyLevelParts(autonomyLevel); @@ -384,18 +389,41 @@ export function ChangeHarnessSettingsView( setOverride((previous) => withSupervisorMode(previous ?? {}, mode))} + onChange={(mode) => setOverride((previous) => { + const next = withSupervisorMode(previous ?? {}, mode); + // A repeat or a move is another attempt of the stage, so + // Act with one attempt could do nothing: it asks for a + // second, and the note below says so. + return mode === "act" && (next.maxStageAttempts ?? base.maxStageAttempts ?? 1) <= 1 + ? { ...next, maxStageAttempts: 2 } + : next; + })} options={[ { value: INHERIT, label: "Inherit", title: `${inheritedSupervisor}, ${FROM_GLOBAL}` }, { value: "advise" as const, label: "Advise" }, { value: "off" as const, label: "Off" }, + ...(ownAutonomous || supervisorMode === "act" ? [{ value: "act" as const, label: "Act" }] : []), ]} />

{supervisorMode === INHERIT ? `${inheritedSupervisor === "off" ? "Off" : "Advise"}, ${FROM_GLOBAL}. ${SUPERVISOR_NOTES[inheritedSupervisor]}` : SUPERVISOR_NOTES[supervisorMode]} + {supervisorMode !== "act" && !ownAutonomous + ? " Act is offered where this change's own autonomy level is Autonomous." + : ""} + {supervisorMode === "act" + ? ` Each stage may be attempted ${stageAttempts} times (maxStageAttempts).` + : ""}

+ {supervisorMode === "act" && !ownAutonomous ? ( +

+ Act needs this change's own autonomy level to be Autonomous: saving will be refused until it is, or until another mode is chosen. +

+ ) : null} + {supervisorMode === "act" ? ( + setOverride(next)} /> + ) : null} { await waitFor(() => expect(screen.getByLabelText("propose agent")).toHaveValue("claude-cli")); expect(chosen("Global supervisor")).toBe("advise"); + // Act is a change's own to choose (the-supervisor-changes-agents 2.1). + expect(radios("Global supervisor").map((radio) => radio.value)).toEqual(["advise", "off"]); + expect(screen.queryByTestId("change-supervisor-act")).toBeNull(); expect(saveButton()).toBeDisabled(); choose("Global supervisor", "off"); expect(screen.getByTestId("global-supervisor-note").textContent).toContain("Points out nothing about runs"); diff --git a/packages/webui/src/components/harness-settings-parts.tsx b/packages/webui/src/components/harness-settings-parts.tsx index 82325c55..d8259d13 100644 --- a/packages/webui/src/components/harness-settings-parts.tsx +++ b/packages/webui/src/components/harness-settings-parts.tsx @@ -393,8 +393,135 @@ export function withSupervisorMode>(config: T, export const SUPERVISOR_NOTES: Readonly> = { advise: "Points out a run that says nothing new or waits on a person, and a failure that repeating cannot fix. It suggests and changes nothing.", off: "Points out nothing about runs. Other suggestions are still made.", + act: "Advises, and when a stage fails it repeats it where repeating is likely to help, or moves it to a fallback agent this change allows. It says and records every move.", }; +/** Said beside Act, before anything is saved: what it may start unattended + * (the-supervisor-changes-agents). */ +export const SUPERVISOR_ACT_WARNING = "A move starts another agent without asking: it may cost money, and with another provider " + + "this change's files and prompts go to that provider. Each is allowed only below."; + +/** The stages a fallback can be named for: the ones that run an agent. */ +export const FALLBACK_STAGES: readonly HarnessStepAgentStage[] = ["propose", "review", "apply", "verify"]; + +/** The supervisor's policy keys as the form edits them. */ +export type SupervisorPolicyKey = "allowCostIncrease" | "allowProviderChange"; + +/** Lays a stage's fallback field over a configuration's `supervisor`. Empty + * removes the stage, and a fallback left with no stage is removed: the + * file says only what was chosen. What is typed is kept as typed; core's + * validator names an unknown id when the file is written. */ +export function withFallback>(config: T, stage: HarnessStepAgentStage, text: string): T { + const agents = text.split(",").map((part) => part.trim()).filter((part) => part.length > 0); + const { [stage]: _replaced, ...others } = config.supervisor?.fallback ?? {}; + const fallback = agents.length === 0 ? others : { ...others, [stage]: agents }; + const { fallback: _old, ...rest } = config.supervisor ?? {}; + const supervisor = Object.keys(fallback).length === 0 ? rest : { ...rest, fallback }; + const result = { ...config }; + if (Object.keys(supervisor).length === 0) delete result.supervisor; + else result.supervisor = supervisor; + return result; +} + +/** Lays an allowance over a configuration's `supervisor`. Unticked removes + * it: absent is false. */ +export function withSupervisorAllowance>(config: T, key: SupervisorPolicyKey, allowed: boolean): T { + const { [key]: _replaced, ...rest } = config.supervisor ?? {}; + const supervisor = allowed ? { ...rest, [key]: true } : rest; + const result = { ...config }; + if (Object.keys(supervisor).length === 0) delete result.supervisor; + else result.supervisor = supervisor; + return result; +} + +/** The agents a fallback can name, as the stage table's agent list names + * them: every registered agent, the editor's chat excepted — a chain cannot + * hand a stage to it. */ +const FALLBACK_AGENT_OPTIONS = STAGE_RUNNER_OPTIONS.filter((agent) => agent.id !== VSCODE_CHAT_STEP_AGENT_ID); + +/** A stage's fallback agents as the stage table picks an agent: one list + * per agent, in order, and one more to add the next. Choosing "(none)" in a + * list removes that agent. */ +function FallbackAgents( + { stage, agents, onChange }: { stage: HarnessStepAgentStage; agents: readonly string[]; onChange: (agents: string[]) => void }, +) { + const lists = [...agents, INHERIT]; + return ( +
+ Fallback for {stage} + + {lists.map((agent, index) => { + const chosen = FALLBACK_AGENT_OPTIONS.find((option) => option.id === agent); + const offered = FALLBACK_AGENT_OPTIONS.filter((option) => option.id === agent || !agents.includes(option.id)); + if (agent === INHERIT && offered.length === 0) return null; + return ( + + + + ); + })} + +
+ ); +} + +/** Act's own fields: a fallback per stage and the two allowances + * (the-supervisor-changes-agents). */ +export function SupervisorActFields( + { config, onChange }: { config: Partial; onChange: (next: Partial) => void }, +) { + return ( +
+

{SUPERVISOR_ACT_WARNING}

+ {FALLBACK_STAGES.map((stage) => ( + onChange(withFallback(config, stage, agents.join(", ")))} + /> + ))} + + +
+ ); +} + /** The columns of the stage table, in order. Each cell carries its column's * name, which the narrow layout draws above the field. */ export const STAGE_COLUMNS = ["Stage", "Agent", "Model", "Effort", "Max cost"] as const; diff --git a/packages/webui/src/shell-ui.ts b/packages/webui/src/shell-ui.ts index 57fce2b9..62b9f4be 100644 --- a/packages/webui/src/shell-ui.ts +++ b/packages/webui/src/shell-ui.ts @@ -1992,6 +1992,49 @@ export const shellThemeCss = ` color: var(--muted); } + /* The supervisor's Act fields: a fallback per stage, then the allowances + (the-supervisor-changes-agents). */ + .openspec-supervisor-act { + display: flex; + flex-direction: column; + gap: 6px; + width: 100%; + } + + .openspec-supervisor-fallback { + display: grid; + grid-template-columns: minmax(0, 9em) minmax(0, 1fr); + align-items: start; + gap: 8px; + font-size: 12px; + } + + .openspec-supervisor-fallback-stage { + line-height: 30px; + } + + .openspec-supervisor-fallback-agents { + display: flex; + flex-direction: column; + gap: 4px; + min-width: 0; + } + + /* The band's fields fill their column; a checkbox is not a field. */ + .openspec-supervisor-allowance { + display: flex; + align-items: center; + gap: 6px; + font-size: 12px; + } + + .openspec-metro .openspec-harness-band .openspec-supervisor-allowance input { + flex: none; + width: auto; + height: auto; + margin: 0; + } + .openspec-metro .badge.openspec-harness-gate { position: static; display: inline-block;