diff --git a/Cargo.lock b/Cargo.lock index 032faa7..9a89d6b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -275,7 +275,7 @@ dependencies = [ [[package]] name = "eval-magic" -version = "0.10.0" +version = "0.11.0" dependencies = [ "anyhow", "assert_cmd", diff --git a/Cargo.toml b/Cargo.toml index 01d1a70..288a5a1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "eval-magic" -version = "0.10.0" +version = "0.11.0" edition = "2024" description = "One-stop CLI for running skill evals — measure whether an agent skill actually shifts behavior." license = "MIT" diff --git a/README.md b/README.md index 1c2eb49..5825b17 100644 --- a/README.md +++ b/README.md @@ -123,9 +123,9 @@ Each eval case runs once per condition and repetition in its own clean Git repos receive the same codebase, task, and overlays; only the condition under test changes. Assertions can combine LLM judgment with runner-owned command checks, transcript checks, and final diff limits. Multi-turn evals resume one native harness session so follow-up answers remain part of the same -conversation, whether the turns are scripted or derived by a responder. An eval can start that -session in the harness's native plan mode and continue in act mode once the plan is approved -(`eval-magic docs conversations`). +conversation, whether the turns are scripted or derived by a responder. An eval can start in the +harness's native plan mode — implementing the plan once approved, or stopping with the plan as its +output — or be handed a plan written beforehand to carry out (`eval-magic docs conversations`). Most harness features are declared in TOML descriptors. See the current registry and resolved data instead of relying on a static compatibility table: diff --git a/docs/claude-notes.md b/docs/claude-notes.md index 1ce3d8e..9572842 100644 --- a/docs/claude-notes.md +++ b/docs/claude-notes.md @@ -89,9 +89,19 @@ scratch repository with one bug: `permissionMode: bypassPermissions` in `init`, keeps the same `session_id`, and edits succeed: the session leaves plan mode. The Claude Code documentation states the converse — a `-p --resume` stays in plan mode only when `--permission-prompt-tool` is passed and no `--permission-mode` is. -- The plan file lands in the operator's `~/.claude/plans`, as in any session. The write guard and - the stray-write audit allow that root; eval-magic keeps the copy the judge reads as - `outputs/plan.md`. +- The plan file lands in the operator's `~/.claude/plans`, as in any session — one directory + shared by every concurrent task, and by the operator's own sessions. That is not a collision: + the plan text is read from the write's `content` argument in the round's own transcript, not + from the file, so tasks cannot read each other's plans. The write guard and the stray-write + audit allow that root; eval-magic keeps the copy the judge reads as `outputs/plan.md`. +- **Plan mode refuses writes into the task environment**, which is why the plan artifact cannot be + a file eval-magic asks the agent to write there. The harness-neutral half of the contract is the + agent's final message: `src/cli/run/plan_prompt.rs` asks every planning round to close with its + complete plan, and that is the `final_message` signal a harness without a plan file falls back + to. On Claude Code the plan file wins, so the fallback is a safety net rather than the path. +- **`--append-system-prompt` is deliberately not used** to steer where the plan is written. There + is no flag that relocates `~/.claude/plans`, and a harness-specific instruction would give + Claude Code a contract no other harness could honor. Relaxing the default closes the common case, not the class — a deny rule, a managed setting, or an operator-overridden mode still refuses calls — so refusals are detected and reported rather than diff --git a/docs/guides/byoh.md b/docs/guides/byoh.md index 32d9627..b4d61c1 100644 --- a/docs/guides/byoh.md +++ b/docs/guides/byoh.md @@ -199,10 +199,12 @@ template you already declared, so it works on any harness that can resume. See A harness with a native read-only planning mode declares `[plan_mode]`: `plan_args` and `act_args` fill a `{mode_args}` slot in both dispatch templates, and `[plan_mode.plan_file]` names -the file the harness writes its plan to, when it writes one. Verify each value the way you verify a -resume: one headless turn dispatched with the planning arguments must refuse an edit, and the same -session resumed with the act arguments must make it. `eval-magic harness show claude-code` is a -worked descriptor; the eval side is in `eval-magic docs conversations`. +the file the harness writes its plan to, when it writes one. Omitting `plan_file` costs nothing — +the planning round's final message is then the plan, because eval-magic's own dispatch prompt asks +every planning round to close with it. Verify each value the way you verify a resume: one headless +turn dispatched with the planning arguments must refuse an edit, and the same session resumed with +the act arguments must make it. `eval-magic harness show claude-code` is a worked descriptor; the +eval side is in `eval-magic docs conversations`. When a shadow preflight reports a live copy, isolate every initial and resumed eval-agent dispatch before setting `isolates_live_sources = true`. The per-harness remedies and verification procedure diff --git a/docs/guides/conversations.md b/docs/guides/conversations.md index 1cc9d64..2292d13 100644 --- a/docs/guides/conversations.md +++ b/docs/guides/conversations.md @@ -110,7 +110,6 @@ dispatch call for different fixes. | `completed` | The responder judged the agent finished. The run stops rather than burning its remaining turns. | | `stopped`, `responder_cannot_answer` | The responder produced no usable reply. `responder_outcome.cause` says why. | | `stopped`, `max_turns_reached` | The agent was still asking at the bound. | -| `stopped`, `plan_not_presented` | A plan-mode session ended its planning phase with no plan to approve. | | `timed_out` | The task outran `dispatch --timeout`. | A `stopped` conversation is recorded, not failed: `dispatch` exits zero and @@ -192,16 +191,24 @@ approves it. An eval declares that shape with `plan_mode`: "id": "add-request-caching", "prompt": "Requests to the pricing API are slow. Can you add caching?", "expected_output": "A working cache with the pricing endpoint under 100ms.", - "plan_mode": true, - "responder": { "type": "llm" } + "plan_mode": true } ``` +`plan_mode` takes one of three values: + +| Value | Shape | +| --- | --- | +| `true`, or `"plan_then_act"` | Plan, approve, implement — the two phases below. | +| `"plan_only"` | Plan and stop. The plan is the run's whole output. | +| `false`, or omitted | Not a plan-mode eval. | + The session runs in two phases, in one native session: 1. **Planning.** The opening prompt is dispatched in the harness's native plan - mode, so the agent explores read-only. If it asks a question, the responder - answers it and the session stays in plan mode. + mode, so the agent explores read-only. If it asks a question and the eval + declares a `responder`, the responder answers and the session stays in plan + mode. 2. **Implementation.** Once the agent has presented its plan, the runner approves it with one fixed message and resumes the same session in act mode. The message is always `The plan is approved. Implement it now.` From @@ -212,19 +219,46 @@ The approval is fixed rather than judged, so the transition is identical in every run and both arms: what varies between conditions is the plan and the implementation, never how the runner reacted to them. +A `"plan_only"` eval runs the first phase and stops there. Nothing is approved +and no act round is dispatched, so the working tree stays clean and +`outputs/plan.md` is what the judge grades. Use it for a skill that only shapes +how the plan is written — running the implementation would spend tokens on work +the eval does not measure. Scripted `turns` are rejected on a plan-only eval, +since the session ends before any of them could be delivered; a `responder` is +still allowed, and answers the agent's questions while it plans. + +### What the agent is told + +A planning round's dispatch prompt carries instructions eval-magic writes +itself, identical in both arms: + +- the session starts in planning mode, so read and explore but do not edit; +- end the turn with the complete plan as the final message — the whole plan + text, not a summary of it and not a pointer to where it lives; +- what follows the plan, which is the one thing the two shapes disagree on. + +Harnesses word their own planning modes differently and some say nothing about +how a plan should be presented. These lines are the part that reads the same +everywhere, and they are what makes the final message a reliable place to +recover a plan from. + ### How the runner knows the plan is ready -Two signals, tried in this order: +Three signals, tried in this order: | Signal | When it applies | | --- | --- | | `plan_file` | The harness writes the plan it presents to a file, and its descriptor declares where (`[plan_mode.plan_file]`). A planning round that wrote one has presented its plan, and the file's content is the plan. Claude Code writes to `~/.claude/plans`. | | `responder` | The eval declares a `responder`, which is told the agent is planning. Its `done` verdict means the plan is ready, and the agent's last message is the plan. | +| `final_message` | Neither of the above was available. The planning round's final message is the plan, which is what the dispatch prompt asked the agent to close with. | -A harness that writes no plan file needs the responder to decide, so `run` -rejects a plan-mode eval there unless it declares one. With a plan file the -responder is optional; if the agent never writes one and there is no responder -to ask, the run stops with `plan_not_presented`. +The last signal always fires, so a plan-mode eval needs no responder on any +harness and every planning phase produces a `plan.md`. What the responder buys +is a planning phase that can run for more than one round: without one, the +agent's first closing message ends the phase, so an agent that spent its turn +asking a question has that question recorded as its plan. `conversation.json`'s +`plan.signal` says which signal fired, so a run resting on `final_message` is +distinguishable from one that wrote a real plan file. `eval-magic harness list` shows `plan-mode` for every harness that can start a session in plan mode. `run` rejects a plan-mode eval on one that cannot, before @@ -232,17 +266,19 @@ any environment is built. ### What is recorded -- `outputs/plan.md` holds the approved plan, and the judge evidence bundle - renders it in a section of its own. +- `outputs/plan.md` holds the plan, and the judge evidence bundle renders it in + a section of its own. - Every user message in `conversation.json` carries `mode` (`plan` or `act`), and the approval turn carries `origin.runner: plan_approval`. -- `conversation.json`'s `plan` names the round the plan was presented in, the - round the approval opened, and which signal fired. +- `conversation.json`'s `plan` names the round the plan was presented in, which + signal fired, and the round the approval opened. A plan-only run has no + approval round, so it records no `approved_in_round`. - A responder's `max_turns` is one budget across both phases: planning-phase answers count toward it. -- A plan-mode run whose session never left the planning phase — whatever +- A plan-then-act run whose session never left the planning phase — whatever stopped it — is counted per condition in `benchmark.json`'s - `validity_warnings`, because it never attempted the task. + `validity_warnings`, because it never attempted the task. A plan-only run + presents a plan, so it is not one of those. - An agent that tries to edit while planning is refused by the mode itself. That refusal is recorded in `permission-denials.json` as behavioral evidence and marked `plan_mode_attributed`, but it raises no validity warning: the @@ -251,3 +287,47 @@ any environment is built. Where the harness writes its plan file, the write guard and the stray-write audit allow that root beside the task environment. The plan file lands where the harness puts it in any session; `plan.md` is the copy the judge reads. + +## Starting from a plan someone already wrote + +The mirror of a plan-only eval: `plan_source` hands the agent a finished plan +and asks it to carry the plan out. The path is relative to the skill's `evals/` +directory, under `files_root` when the eval sets one: + +```json +{ + "id": "execute-cache-plan", + "prompt": "Implement the approved plan.", + "expected_output": "A working cache matching the plan.", + "plan_source": "plans/add-request-caching.md" +} +``` + +The file's text is spliced into the dispatch prompt ahead of the request, +framed as already approved. The session is an ordinary act-mode dispatch, so +this needs nothing from the harness: it works on every harness, including one +whose descriptor declares no `[plan_mode]`. `dispatch.json` records the path +each task's plan came from; the text itself is in `dispatch-prompt.txt`. + +`plan_source` and `plan_mode` are mutually exclusive — a run either writes its +own plan or is handed one. + +To carry a plan-only campaign's output into an executing campaign, copy the +run's `outputs/plan.md` into the executing skill's `evals/` directory and name +it in `plan_source`. Which plan to carry across is a judgement about the +comparison you are making, so the copy is deliberate rather than automatic. + +The alternative, when the plan should be a file the agent reads rather than +prompt context, is the `files` overlay: + +```json +{ + "id": "execute-cache-plan", + "files": ["PLAN.md"], + "prompt": "Implement the plan in PLAN.md.", + "expected_output": "A working cache matching the plan." +} +``` + +That stages `PLAN.md` in the task repository, where it is part of the baseline +and so does not count against a `diff_scope` assertion. diff --git a/docs/opencode-notes.md b/docs/opencode-notes.md index ef7ede2..ef4fffc 100644 --- a/docs/opencode-notes.md +++ b/docs/opencode-notes.md @@ -77,8 +77,10 @@ bug (free model `opencode/nemotron-3.5-lightning-free`): agent makes, so it belongs to `act_args` only. - `opencode run --session --agent build --auto` resumed the same session and edited the file. `--agent build` is explicit so a resumed session does not inherit the plan agent. -- OpenCode writes no plan file, so the descriptor declares no `[plan_mode.plan_file]`; a plan-mode - eval on OpenCode needs a `responder`, whose `done` verdict in the planning phase approves the plan. +- OpenCode writes no plan file, so the descriptor declares no `[plan_mode.plan_file]`. The + planning round's final message is the plan there — the dispatch prompt asks every planning round + to close with it — so a plan-mode eval on OpenCode needs no `responder`. Declaring one still + buys a planning phase of more than one round, with its `done` verdict approving the plan. ## Write guard diff --git a/docs/progressive-enhancements.md b/docs/progressive-enhancements.md index d9a30e2..c375cc1 100644 --- a/docs/progressive-enhancements.md +++ b/docs/progressive-enhancements.md @@ -426,15 +426,27 @@ scan config-declared `skills.paths`/`skills.urls` sources. *Why harness-specific:* the read-only planning mode is the harness's own — a permission mode for Claude Code, a built-in agent for OpenCode — and so is the way the agent presents its plan. -*What it unlocks:* evals that declare `plan_mode: true`. The driver dispatches the opening round -with the planning arguments, lets the agent present a plan, approves it with one fixed message, and -resumes the same session with the act arguments; the eval's `turns` or `responder` then proceed as -usual. The approved plan is saved as `outputs/plan.md` and rendered in the judge evidence bundle. -`plan_file` is the deterministic signal that the plan was presented; without one the eval's -responder decides, which is why `run` requires a responder on a harness without a plan file. +*What it unlocks:* evals that declare `plan_mode`. The driver dispatches the opening round with the +planning arguments, lets the agent present a plan, and saves it as `outputs/plan.md`, rendered in +the judge evidence bundle. `plan_mode: true` then approves the plan with one fixed message and +resumes the same session with the act arguments, where the eval's `turns` or `responder` proceed as +usual; `plan_mode: "plan_only"` stops at the plan, making it the run's whole output. + +The descriptor supplies only half the capability. The other half is harness-neutral and lives in +the dispatch prompt (`src/cli/run/plan_prompt.rs`): a planning round is told it cannot edit and +that its final message must carry the complete plan. That is what makes the third signal below +work, and it is the part a new harness inherits without declaring anything. + +Three signals mark a plan as presented, tried in order: the `plan_file` write, a responder's `done` +verdict, and the planning round's final message. The last always fires, so no eval needs a +responder to reach a plan and every planning phase produces an artifact. `conversation.json`'s +`plan.signal` records which one did, so a plan resting on the final message stays distinguishable +from one read out of a native plan file. *Fallback:* none. `run` rejects a plan-mode eval for a harness without `[plan_mode]`, before any environment is built, the way it rejects multi-turn evals for a harness without `[conversation]`. +The eval-side `plan_source` field is not this capability: it splices a pre-written plan into an +ordinary act-mode dispatch and needs no descriptor support at all. *Descriptor fields:* the `[plan_mode]` table — `plan_args` and `act_args` fill the `{mode_args}` slot that both `dispatch.exec_template` and `conversation.resume_exec_template` must carry (the act diff --git a/harnesses/template.toml b/harnesses/template.toml index 58cd1ec..c5fa5dc 100644 --- a/harnesses/template.toml +++ b/harnesses/template.toml @@ -190,7 +190,9 @@ label = "{label}" ## [plan_mode.plan_file] names the file the harness writes its plan to: writes under root are ## allowed by the write guard and the stray-write audit, a plan-phase round that wrote one has ## presented its plan, and that write's content_field is the plan artifact. Omit plan_file when -## the harness writes no plan file; the eval's responder then decides when the plan is ready. +## the harness writes no plan file: the eval's responder then decides when the plan is ready, and +## without one the planning round's final message is the plan — eval-magic's own prompt asks every +## planning round to close with it, so a plan artifact is produced either way. ## VERIFY: run one headless turn in the planning mode and confirm edits are refused; resume it ## with the act arguments and confirm that same session edits. See `eval-magic harness show ## claude-code` for a plan_file example and `eval-magic docs conversations` for the eval side. diff --git a/schema/conversation.schema.json b/schema/conversation.schema.json index b848183..a31ab08 100644 --- a/schema/conversation.schema.json +++ b/schema/conversation.schema.json @@ -115,9 +115,9 @@ }, "planRecord": { "type": "object", - "required": ["presented_in_round", "approved_in_round", "signal"], + "required": ["presented_in_round", "signal"], "additionalProperties": false, - "description": "How a plan-mode session moved from planning to implementation. Absent unless the eval declared plan_mode and a plan was approved.", + "description": "How a plan-mode session moved from planning to implementation. Absent unless the eval declared plan_mode and a plan was presented.", "properties": { "presented_in_round": { "type": "integer", @@ -127,12 +127,12 @@ "approved_in_round": { "type": "integer", "minimum": 2, - "description": "The round the runner's fixed approval opened in act mode." + "description": "The round the runner's fixed approval opened in act mode. Absent on a plan_only run, which presents its plan and stops without one." }, "signal": { "type": "string", - "enum": ["plan_file", "responder"], - "description": "What marked the plan as presented: the harness wrote its declared plan file (plan_file), or the responder judged the agent finished planning (responder)." + "enum": ["plan_file", "responder", "final_message"], + "description": "What marked the plan as presented, in the order they are tried: the harness wrote its declared plan file (plan_file), the responder judged the agent finished planning (responder), or neither was available and the planning round's final message was taken as the plan (final_message)." }, "artifact_path": { "type": "string", diff --git a/schema/evals.schema.json b/schema/evals.schema.json index ba6bfb7..c2b9974 100644 --- a/schema/evals.schema.json +++ b/schema/evals.schema.json @@ -125,9 +125,12 @@ "description": "Derives each follow-up turn from what the agent just said, instead of scripting them. Mutually exclusive with turns; declaring neither preserves one-shot dispatch. Requires a harness with native conversation resume, exactly as turns does." }, "plan_mode": { - "type": "boolean", + "anyOf": [ + { "type": "boolean" }, + { "enum": ["plan_then_act", "plan_only"] } + ], "default": false, - "description": "Start the session in the harness's native plan mode. The agent explores read-only and presents a plan; eval-magic approves it in the same session with one fixed message and continues in act mode, where turns or a responder proceed as usual. Requires a harness whose descriptor declares [plan_mode] (`eval-magic harness list` shows plan-mode). A harness without a [plan_mode.plan_file] also requires a responder, which decides when the plan is ready. See `eval-magic docs conversations`." + "description": "Start the session in the harness's native plan mode. The agent explores read-only and presents a plan; the runner saves it as outputs/plan.md. `true` (or \"plan_then_act\") then approves the plan in the same session with one fixed message and continues in act mode, where turns or a responder proceed as usual; \"plan_only\" stops there, making the plan the run's whole output. The plan is recognized from the harness's own plan file, else a responder's `done` verdict, else the planning round's final message — the dispatch prompt asks every planning round to close with its complete plan. Requires a harness whose descriptor declares [plan_mode] (`eval-magic harness list` shows plan-mode). See `eval-magic docs conversations`." }, "expected_output": { "type": "string", @@ -139,6 +142,11 @@ "items": { "type": "string" }, "description": "Codebase overlay destination paths relative to the task repository. Sources use the same paths beneath the skill's evals/ directory, or beneath files_root when set. Applied after the codebase and condition skills are staged. The normalized first component may not be .git (case-insensitive) because root Git metadata is runner-owned; nested .git paths are allowed." }, + "plan_source": { + "type": "string", + "minLength": 1, + "description": "A plan written before the run, as a path relative to the skill's evals/ directory (beneath files_root when set). Its text is spliced into the dispatch prompt as an already-approved plan and the session runs in ordinary act mode, so this works on every harness — including one whose descriptor declares no [plan_mode]. Mutually exclusive with plan_mode: a run either writes its own plan or is handed one. To carry a plan-only campaign's output forward, copy its outputs/plan.md into the executing skill's evals/ directory and name it here. See `eval-magic docs conversations`." + }, "files_root": { "type": "string", "minLength": 1, diff --git a/schema/run-record.schema.json b/schema/run-record.schema.json index a801335..b97f3fc 100644 --- a/schema/run-record.schema.json +++ b/schema/run-record.schema.json @@ -232,9 +232,9 @@ }, "planRecord": { "type": "object", - "required": ["presented_in_round", "approved_in_round", "signal"], + "required": ["presented_in_round", "signal"], "additionalProperties": false, - "description": "How a plan-mode session moved from planning to implementation. Absent unless the eval declared plan_mode and a plan was approved.", + "description": "How a plan-mode session moved from planning to implementation. Absent unless the eval declared plan_mode and a plan was presented.", "properties": { "presented_in_round": { "type": "integer", @@ -244,12 +244,12 @@ "approved_in_round": { "type": "integer", "minimum": 2, - "description": "The round the runner's fixed approval opened in act mode." + "description": "The round the runner's fixed approval opened in act mode. Absent on a plan_only run, which presents its plan and stops without one." }, "signal": { "type": "string", - "enum": ["plan_file", "responder"], - "description": "What marked the plan as presented: the harness wrote its declared plan file (plan_file), or the responder judged the agent finished planning (responder)." + "enum": ["plan_file", "responder", "final_message"], + "description": "What marked the plan as presented, in the order they are tried: the harness wrote its declared plan file (plan_file), the responder judged the agent finished planning (responder), or neither was available and the planning round's final message was taken as the plan (final_message)." }, "artifact_path": { "type": "string", diff --git a/src/cli/args.rs b/src/cli/args.rs index 84cf200..1a67a94 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -378,8 +378,10 @@ pub(crate) enum Commands { /// agent turn and one small responder dispatch per round, up to its /// `max_turns` bound. A `plan_mode` case starts each session in the /// harness's native plan mode and adds one runner-authored approval turn - /// before implementation; it needs the `plan-mode` capability - /// (`eval-magic harness list`). Each `llm_judge` assertion creates its effective sample + /// before implementation; `plan_mode: "plan_only"` stops at the plan + /// instead, costing one round. Both need the `plan-mode` capability + /// (`eval-magic harness list`). A `plan_source` case costs one ordinary + /// act-mode round and needs no capability. Each `llm_judge` assertion creates its effective sample /// count of judge tasks per condition and repetition. Review the printed run /// summary and obtain confirmation before spending model usage. /// @@ -437,12 +439,13 @@ pub(crate) enum Commands { /// cause: the run ended mid-task, so read its last assistant message before /// trusting it. See `eval-magic docs conversations`. /// - /// A `plan_mode` task runs its opening round in the harness's plan mode, - /// approves the presented plan with one fixed message, and continues the - /// same session in act mode; the approved plan is saved as `outputs/plan.md`. - /// A planning phase that ends with no plan to approve is recorded as - /// `plan_not_presented` and warned about: that run never reached - /// implementation. + /// A `plan_mode` task runs its opening round in the harness's plan mode + /// and saves the presented plan as `outputs/plan.md`. It then approves the + /// plan with one fixed message and continues the same session in act mode, + /// unless the eval declared `plan_mode: "plan_only"`, which stops there + /// with the plan as its output. The plan is read from the harness's own + /// plan file, else a responder's verdict, else the planning round's final + /// message; `conversation.json` records which. /// /// Nested Codex sandboxes: if the same generated task command succeeds in /// an ordinary terminal with equivalent inputs and configuration, but @@ -677,8 +680,9 @@ pub(crate) enum Commands { /// /// Extend the seed in `evals/evals.json`: `turns` scripts same-session /// follow-ups and `responder` derives them instead, `plan_mode` starts the - /// session in the harness's plan mode (see - /// `eval-magic docs conversations`), `files_root` resolves overlay sources + /// session in the harness's plan mode and `plan_source` hands it a plan + /// written beforehand (see `eval-magic docs conversations`), + /// `files_root` resolves overlay sources /// applied at the codebase root, and a per-eval `runs` value overrides /// `run --runs`. Add /// assertions after the first iteration, then check the file with diff --git a/src/cli/run/conversation.rs b/src/cli/run/conversation.rs index af4a596..86d78d0 100644 --- a/src/cli/run/conversation.rs +++ b/src/cli/run/conversation.rs @@ -44,7 +44,10 @@ pub enum TaskOutcome { Completed { delivered_followups: u32, source: TurnSource, - /// The round the runner's plan approval opened, for a plan-mode task. + /// The round whose output was taken as the plan, for a plan-mode task. + plan_presented_in_round: Option, + /// The round the runner's plan approval opened. `None` on a plan-only + /// task, which presents its plan and stops without one. plan_approved_in_round: Option, }, Stopped { @@ -94,6 +97,13 @@ impl TaskOutcome { "completed with {delivered_followups} follow-up turn(s), plan approved in round \ {round}" ), + // A plan-only task delivers nothing and approves nothing, so + // without this its whole output would go unmentioned. + Self::Completed { + plan_presented_in_round: Some(round), + plan_approved_in_round: None, + .. + } => format!("completed — plan presented in round {round}, saved as outputs/plan.md"), Self::Completed { delivered_followups: 0, .. @@ -200,7 +210,8 @@ pub fn run_task( // one-shot. Other one-shot tasks never resume, and a harness may support // them without declaring `[conversation]` at all (cline does). let plan_mode = task.plan_mode; - let mut mode = if plan_mode { + let starts_planning = plan_mode.starts_in_plan_mode(); + let mut mode = if starts_planning { SessionMode::Plan } else { SessionMode::Act @@ -211,13 +222,13 @@ pub fn run_task( SessionMode::Plan => anyhow!("harness declares no plan-mode dispatch command"), SessionMode::Act => anyhow!("harness declares no initial dispatch command"), })?; - let needs_resume = plan.delivers_followups() || plan_mode; + let needs_resume = plan.delivers_followups() || plan_mode.implements_the_plan(); let resume_templates = if needs_resume { Some(ResumeTemplates { act: adapter .cli_resume_command_in_mode(SessionMode::Act, guard, agent_model, agent_env) .ok_or_else(|| anyhow!("harness declares no native conversation resume command"))?, - plan: plan_mode + plan: starts_planning .then(|| { adapter .cli_resume_command_in_mode( @@ -234,7 +245,7 @@ pub fn run_task( None }; // The plan file only matters while planning; `home` resolves its `~`. - let plan_file = plan_mode.then(|| adapter.plan_file()).flatten(); + let plan_file = starts_planning.then(|| adapter.plan_file()).flatten(); let home = std::env::home_dir(); if overwrite && conversation_path.exists() { fs::remove_file(&conversation_path).with_context(|| { @@ -269,7 +280,7 @@ pub fn run_task( round: 1, text: task.user_prompt.clone(), origin: None, - mode: plan_mode.then_some(mode), + mode: starts_planning.then_some(mode), }]; let first_outputs = base_outputs.join("turn-1"); let initial_command = render_command( @@ -374,12 +385,18 @@ pub fn run_task( PlanDecision::Approve(approved) => { let plan_path = base_outputs.join("plan.md"); write_atomic(&plan_path, approved.text.clone())?; + let implements = plan_mode.implements_the_plan(); plan_record = Some(PlanRecord { presented_in_round: last_round, - approved_in_round: last_round.saturating_add(1), + approved_in_round: implements.then(|| last_round.saturating_add(1)), signal: approved.signal, artifact_path: Some(artifact_path(&plan_path)), }); + if !implements { + // A plan-only eval is finished: the plan is the output, + // so there is nothing to approve and no act round. + break; + } ( PLAN_APPROVAL_PROMPT.to_string(), Some(TurnOrigin::Runner { @@ -438,7 +455,7 @@ pub fn run_task( round, text: prompt.clone(), origin, - mode: plan_mode.then_some(mode), + mode: starts_planning.then_some(mode), }); delivered_followups = delivered_followups.saturating_add(1); @@ -580,10 +597,14 @@ fn write_conversation( ConversationStatus::Completed => TaskOutcome::Completed { delivered_followups: conversation.delivered_followups, source, + plan_presented_in_round: conversation + .plan + .as_ref() + .map(|plan| plan.presented_in_round), plan_approved_in_round: conversation .plan .as_ref() - .map(|plan| plan.approved_in_round), + .and_then(|plan| plan.approved_in_round), }, ConversationStatus::Stopped => TaskOutcome::Stopped { before_followup: conversation.stopped_before_followup.unwrap_or_default(), @@ -749,7 +770,7 @@ fn write_atomic(path: &Path, body: String) -> anyhow::Result<()> { mod tests { use std::collections::BTreeMap; - use super::{execute_round, final_text_for_round}; + use super::{TaskOutcome, TurnSource, execute_round, final_text_for_round}; use crate::adapters::TranscriptSummary; use crate::adapters::cli_command::shell_quote_arg; @@ -771,6 +792,36 @@ mod tests { ); } + /// A plan-only run completes with no follow-ups and no approval, so a bare + /// "completed" would hide the one thing it produced. The summary says the + /// plan was presented and where it landed. + #[test] + fn a_plan_only_outcome_says_the_plan_was_presented() { + let outcome = TaskOutcome::Completed { + delivered_followups: 0, + source: TurnSource::Scripted, + plan_presented_in_round: Some(1), + plan_approved_in_round: None, + }; + let summary = outcome.summary(); + assert!(summary.contains("plan presented in round 1"), "{summary}"); + assert!(summary.contains("outputs/plan.md"), "{summary}"); + assert!(!summary.contains("approved"), "{summary}"); + } + + /// A plan-then-act run still reports its approval round, unchanged. + #[test] + fn a_plan_then_act_outcome_still_names_the_approval_round() { + let outcome = TaskOutcome::Completed { + delivered_followups: 1, + source: TurnSource::Scripted, + plan_presented_in_round: Some(1), + plan_approved_in_round: Some(2), + }; + let summary = outcome.summary(); + assert!(summary.contains("plan approved in round 2"), "{summary}"); + } + #[test] fn final_text_for_round_returns_the_parser_result() { let summary = TranscriptSummary { diff --git a/src/cli/run/conversation/plan_phase.rs b/src/cli/run/conversation/plan_phase.rs index 320bd0b..b82ae9c 100644 --- a/src/cli/run/conversation/plan_phase.rs +++ b/src/cli/run/conversation/plan_phase.rs @@ -90,10 +90,13 @@ pub(super) fn decide( return PlanDecision::Approve(presented); } let Some((policy, runtime)) = responder else { - return PlanDecision::Stop { - reason: ConversationStopReason::PlanNotPresented, - responder: None, - }; + // Nothing else can say the plan is ready, so the round's closing + // message is it. The dispatch prompt asked the agent for exactly that, + // which is what makes this a signal rather than a guess. + return PlanDecision::Approve(PresentedPlan { + text: consultation.final_message.to_string(), + signal: PlanSignal::FinalMessage, + }); }; let verdict = runtime.consult(followup, consultation, previous_reply); match next_from_verdict(policy, consultation.prior_replies.len() as u32, verdict) { @@ -122,7 +125,7 @@ mod tests { use super::*; use crate::adapters::TranscriptSummary; use crate::adapters::descriptor::PlanFileSection; - use crate::core::{ConversationStopReason, PlanSignal, ToolInvocation}; + use crate::core::{PlanSignal, ToolInvocation}; fn plan_file() -> PlanFileSection { PlanFileSection { @@ -221,20 +224,23 @@ mod tests { assert_eq!(presented.text, "The plan: fix it."); } + /// The last rung of the ladder. With no plan file to read and no responder + /// to ask, the planning round's final message *is* the plan — the dispatch + /// prompt told the agent to end the turn with it — so the run gets an + /// inspectable `plan.md` instead of stopping empty-handed. #[test] - fn without_a_plan_file_or_responder_the_phase_stops_plan_not_presented() { + fn without_a_plan_file_or_responder_the_final_message_is_the_plan() { let consultation = Consultation { task_prompt: "Add caching.", prior_replies: &[], - final_message: "Which file?", + final_message: "1. Add an LRU.\n2. Test it.\n", planning: true, }; - let PlanDecision::Stop { reason, responder } = decide(None, None, 1, &consultation, None) - else { - panic!("nothing can approve a plan nobody presented"); + let PlanDecision::Approve(approved) = decide(None, None, 1, &consultation, None) else { + panic!("the final message stands in for a plan file the harness never writes"); }; - assert_eq!(reason, ConversationStopReason::PlanNotPresented); - assert!(responder.is_none()); + assert_eq!(approved.text, "1. Add an LRU.\n2. Test it.\n"); + assert_eq!(approved.signal, PlanSignal::FinalMessage); } #[test] diff --git a/src/cli/run/dispatch.rs b/src/cli/run/dispatch.rs index 285e8ce..957d61c 100644 --- a/src/cli/run/dispatch.rs +++ b/src/cli/run/dispatch.rs @@ -16,7 +16,7 @@ use crate::adapters::{CliManifestContext, adapter_for}; use crate::core::fs::artifact_path; use crate::core::{ AvailableSkill, CodebaseRecord, ConditionSkill, Eval, GuardPolicyConfig, Harness, - POSIX_TOOLING_REQUIREMENT, ResponderPolicy, ScriptedTurn, SkillSource, + POSIX_TOOLING_REQUIREMENT, PlanMode, ResponderPolicy, ScriptedTurn, SkillSource, }; use super::RunError; @@ -89,10 +89,16 @@ pub struct DispatchTask { /// Fully expanded command policy used by the live guard and post-run audit. #[serde(default)] pub guard_policy: GuardPolicyConfig, - /// Whether the session starts in the harness's native plan mode. Absent - /// unless the eval declares it, so other tasks serialize as before. - #[serde(default, skip_serializing_if = "std::ops::Not::not")] - pub plan_mode: bool, + /// Whether the session starts in the harness's native plan mode, and what + /// follows the plan. Absent unless the eval declares it, so other tasks + /// serialize as before. + #[serde(default, skip_serializing_if = "PlanMode::is_off")] + pub plan_mode: PlanMode, + /// The eval-relative file the supplied plan was read from, when the eval + /// declared a `plan_source`. Provenance only: the plan text itself is in + /// `dispatch-prompt.txt`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub plan_source: Option, #[serde(default, skip_serializing)] pub dispatch_prompt: String, } @@ -117,7 +123,12 @@ pub struct DispatchTaskOpts<'a> { pub files: Vec, pub turns: Option<&'a [ScriptedTurn]>, /// The eval's `plan_mode` declaration. - pub plan_mode: bool, + pub plan_mode: PlanMode, + /// The already-approved plan this eval was handed, read from its + /// `plan_source`. Spliced into the prompt ahead of the user request. + pub plan_text: Option<&'a str>, + /// Where that plan was read from, recorded on the task for provenance. + pub plan_source: Option<&'a str>, pub outputs_dir: &'a str, pub cond_dir: &'a str, pub bootstrap_content: Option<&'a str>, @@ -218,12 +229,25 @@ pub fn build_dispatch_task(opts: &DispatchTaskOpts) -> Result Result> = HashMap::new(); + // An eval handed a written plan carries its text into the dispatch prompt, + // so it is read once here rather than per condition and repetition. + let mut plan_text_by_eval: HashMap<&str, String> = HashMap::new(); for ev in &r.selected_evals { let dests = overlay_file_pairs(ev, &ctx.skill_subdir)? .into_iter() .map(|(dest, _source)| dest) .collect(); overlay_files_by_eval.insert(ev.id.as_str(), dests); + if let Some(plan) = plan_source_text(ev, &ctx.skill_subdir)? { + plan_text_by_eval.insert(ev.id.as_str(), plan); + } } // A single group keeps the `group` key off each task (>1 group tags them); @@ -258,6 +264,8 @@ pub(super) fn write_dispatch( turns: ev.turns.as_deref(), responder: ev.responder.as_ref(), plan_mode: ev.plan_mode, + plan_text: plan_text_by_eval.get(ev.id.as_str()).map(String::as_str), + plan_source: ev.plan_source.as_deref(), outputs_dir: &outputs_dir_str, cond_dir: &run_dir_str, bootstrap_content: staged.bootstrap_content.as_deref(), diff --git a/src/cli/run/orchestrate/mod.rs b/src/cli/run/orchestrate/mod.rs index 4b28bc8..e700404 100644 --- a/src/cli/run/orchestrate/mod.rs +++ b/src/cli/run/orchestrate/mod.rs @@ -246,14 +246,14 @@ pub fn command_run(ctx: &RunContext, opts: &RunOptions) -> Result<(), RunError> } } - // Plan mode is a native capability with no fallback, and the approved plan - // is implemented by resuming the session, so it is gated here too. A - // harness that writes no plan file has no signal of its own for "the plan - // is ready", so those evals have to bring a responder to decide it. + // Plan mode is a native capability with no fallback, and the plan phase is + // driven by resuming the session, so it is gated here too. No responder is + // required: a harness that writes no plan file still closes its planning + // round with a final message, which the dispatch prompt asks to be the plan. let plan_mode_evals: Vec<&str> = resolved .selected_evals .iter() - .filter(|eval| eval.plan_mode) + .filter(|eval| eval.plan_mode.starts_in_plan_mode()) .map(|eval| eval.id.as_str()) .collect(); if !plan_mode_evals.is_empty() { @@ -268,22 +268,6 @@ pub fn command_run(ctx: &RunContext, opts: &RunOptions) -> Result<(), RunError> plan_mode_evals.join(", ") ))); } - if adapter.plan_file().is_none() { - let without_responder: Vec<&str> = resolved - .selected_evals - .iter() - .filter(|eval| eval.plan_mode && eval.responder.is_none()) - .map(|eval| eval.id.as_str()) - .collect(); - if !without_responder.is_empty() { - return Err(RunError::msg(format!( - "--harness {label} needs a responder on plan-mode evals ({}): its descriptor \ - declares no [plan_mode.plan_file], so only a responder can tell when the plan \ - is ready for approval (`eval-magic docs conversations`)", - without_responder.join(", ") - ))); - } - } } // The harness preflight enforces the runner-ready dispatch/transcript @@ -360,15 +344,31 @@ fn print_run_plan(ctx: &RunContext, opts: &RunOptions, r: &Resolved) { source.resolved_path.as_deref().unwrap_or(&source.source) ); } - let plan_mode_evals = r + // The two shapes are counted apart: "plan mode" alone no longer says + // whether the run implements anything, and that is what an operator + // reading the plan needs to know. + let then_act = r .selected_evals .iter() - .filter(|eval| eval.plan_mode) + .filter(|eval| eval.plan_mode.implements_the_plan()) .count(); - if plan_mode_evals > 0 { + let plan_only = r + .selected_evals + .iter() + .filter(|eval| { + eval.plan_mode.starts_in_plan_mode() && !eval.plan_mode.implements_the_plan() + }) + .count(); + if then_act > 0 { + println!( + " plan mode: {then_act} plan-then-act eval(s) start in the harness's native plan \ + mode and continue in act mode once the plan is approved" + ); + } + if plan_only > 0 { println!( - " plan mode: {plan_mode_evals} eval(s) start in the harness's native plan mode and \ - continue in act mode once the plan is approved" + " plan mode: {plan_only} plan-only eval(s) start in the harness's native plan mode \ + and stop once the plan is presented, with outputs/plan.md as the output" ); } if r.selected_evals.len() != r.total_evals { diff --git a/src/cli/run/overlays.rs b/src/cli/run/overlays.rs index 860b047..577c69d 100644 --- a/src/cli/run/overlays.rs +++ b/src/cli/run/overlays.rs @@ -87,6 +87,30 @@ pub fn overlay_file_pairs( Ok(pairs) } +/// Read the plan an eval was handed, when it declares one. The path resolves +/// exactly where an overlay source does — beneath `/evals/`, under +/// `files_root` when one is set — and the text goes into the dispatch prompt +/// rather than into the task environment, so it is context the agent is given +/// rather than a file it has to be told to find. +pub fn plan_source_text(eval: &Eval, skill_dir: &Path) -> Result, RunError> { + let Some(relative) = eval.plan_source.as_deref() else { + return Ok(None); + }; + validate_task_relative_path(relative)?; + let mut source_root = skill_dir.join("evals"); + if let Some(root) = eval.files_root.as_deref() { + validate_files_root_rel(root)?; + source_root = source_root.join(root); + } + let source = source_root.join(relative); + fs::read_to_string(&source).map(Some).map_err(|error| { + RunError::msg(format!( + "plan source not found: {} ({error})", + source.display() + )) + }) +} + /// Resolve a command check's held-out setup paths without copying them. pub fn setup_file_pairs( check: &AssertionCommandCheck, @@ -132,6 +156,7 @@ pub fn copy_overlay_files( #[cfg(test)] mod tests { use super::*; + use crate::core::PlanMode; fn eval_with_files(files: &[&str]) -> Eval { Eval { @@ -147,10 +172,92 @@ mod tests { codebase: None, responder: None, guard: None, - plan_mode: false, + plan_mode: PlanMode::Off, + plan_source: None, } } + /// A `plan_source` resolves beneath the skill's evals directory, exactly + /// where an overlay file would, and its text is read verbatim. + #[test] + fn plan_source_reads_the_named_file_under_evals() { + let tmp = tempfile::TempDir::new().unwrap(); + let skill_dir = tmp.path().join("skill"); + fs::create_dir_all(skill_dir.join("evals/plans")).unwrap(); + fs::write( + skill_dir.join("evals/plans/add-cache.md"), + "1. Add an LRU\n2. Test it\n", + ) + .unwrap(); + + let mut eval = eval_with_files(&[]); + eval.plan_source = Some("plans/add-cache.md".to_string()); + + assert_eq!( + plan_source_text(&eval, &skill_dir).unwrap().as_deref(), + Some("1. Add an LRU\n2. Test it\n") + ); + } + + /// `files_root` moves the plan source with the overlay files, so an eval + /// that keeps its fixtures in a subdirectory keeps its plan there too. + #[test] + fn plan_source_honors_files_root() { + let tmp = tempfile::TempDir::new().unwrap(); + let skill_dir = tmp.path().join("skill"); + fs::create_dir_all(skill_dir.join("evals/fixtures")).unwrap(); + fs::write(skill_dir.join("evals/fixtures/plan.md"), "the plan\n").unwrap(); + + let mut eval = eval_with_files(&[]); + eval.files_root = Some("fixtures".to_string()); + eval.plan_source = Some("plan.md".to_string()); + + assert_eq!( + plan_source_text(&eval, &skill_dir).unwrap().as_deref(), + Some("the plan\n") + ); + } + + /// An eval that declares no plan source reads nothing, and one that names + /// a file that is not there says so by name rather than dispatching a task + /// whose prompt is quietly missing its plan. + #[test] + fn plan_source_is_optional_and_missing_files_are_named() { + let tmp = tempfile::TempDir::new().unwrap(); + let skill_dir = tmp.path().join("skill"); + fs::create_dir_all(skill_dir.join("evals")).unwrap(); + + assert!( + plan_source_text(&eval_with_files(&[]), &skill_dir) + .unwrap() + .is_none() + ); + + let mut missing = eval_with_files(&[]); + missing.plan_source = Some("nope.md".to_string()); + let error = plan_source_text(&missing, &skill_dir) + .unwrap_err() + .to_string(); + assert!(error.contains("plan source"), "error was: {error}"); + assert!(error.contains("nope.md"), "error was: {error}"); + } + + /// The containment rules that hold for overlay sources hold here: a plan + /// source may not climb out of the skill's evals directory. + #[test] + fn plan_source_may_not_escape_the_evals_directory() { + let tmp = tempfile::TempDir::new().unwrap(); + let skill_dir = tmp.path().join("skill"); + fs::create_dir_all(skill_dir.join("evals")).unwrap(); + + let mut escaping = eval_with_files(&[]); + escaping.plan_source = Some("../outside.md".to_string()); + let error = plan_source_text(&escaping, &skill_dir) + .unwrap_err() + .to_string(); + assert!(error.contains("relative"), "error was: {error}"); + } + #[test] fn overlay_pairs_resolve_without_copying() { let tmp = tempfile::TempDir::new().unwrap(); diff --git a/src/cli/run/plan_prompt.rs b/src/cli/run/plan_prompt.rs new file mode 100644 index 0000000..8473ca8 --- /dev/null +++ b/src/cli/run/plan_prompt.rs @@ -0,0 +1,87 @@ +//! Plan-mode guidance shared by dispatch prompt assembly. +//! +//! A harness's own planning mode already tells the agent how it presents a +//! plan, and each one says something different — Claude Code points at +//! `~/.claude/plans`, OpenCode's `plan` agent says nothing of the kind. These +//! lines are the part eval-magic can rely on everywhere: the agent closes its +//! planning turn with the whole plan, so the driver has a plan to save as +//! `outputs/plan.md` whatever the harness does. + +use crate::core::PlanMode; + +pub(super) fn push_instructions(lines: &mut Vec, plan_mode: PlanMode) { + if plan_mode.is_off() { + return; + } + lines.push( + "- This session starts in the harness's planning mode: read and explore the task environment, but do not edit files." + .to_string(), + ); + lines.push( + "- End this turn with your complete plan as your final message — the whole plan text, not a summary of it and not a pointer to where it lives." + .to_string(), + ); + lines.push( + if plan_mode.implements_the_plan() { + "- You will be told when the plan is approved; implement it in this same session then." + } else { + "- The plan is the deliverable: this task ends once you have presented it, and there is nothing to implement." + } + .to_string(), + ); +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The two lines every planning round gets, whatever the harness and + /// whatever follows the plan. + #[test] + fn both_plan_shapes_ask_for_the_whole_plan_in_the_final_message() { + for plan_mode in [PlanMode::PlanThenAct, PlanMode::PlanOnly] { + let mut lines = Vec::new(); + push_instructions(&mut lines, plan_mode); + let rendered = lines.join("\n"); + assert!( + rendered.contains("planning mode"), + "{plan_mode:?}: {rendered}" + ); + assert!( + rendered.contains("do not edit files"), + "{plan_mode:?}: {rendered}" + ); + assert!( + rendered.contains("complete plan as your final message"), + "{plan_mode:?}: {rendered}" + ); + } + } + + /// What follows the plan is the one thing the two shapes disagree on, and + /// the agent is told which it is so it does not sit waiting for an approval + /// that never comes. + #[test] + fn each_shape_says_what_follows_the_plan() { + let mut then_act = Vec::new(); + push_instructions(&mut then_act, PlanMode::PlanThenAct); + let then_act = then_act.join("\n"); + assert!(then_act.contains("approved"), "{then_act}"); + assert!(!then_act.contains("nothing to implement"), "{then_act}"); + + let mut plan_only = Vec::new(); + push_instructions(&mut plan_only, PlanMode::PlanOnly); + let plan_only = plan_only.join("\n"); + assert!(plan_only.contains("nothing to implement"), "{plan_only}"); + assert!(!plan_only.contains("approved"), "{plan_only}"); + } + + /// An eval outside plan mode contributes nothing, so its prompt is + /// byte-identical to what it was before this module existed. + #[test] + fn a_non_plan_mode_eval_contributes_nothing() { + let mut lines = Vec::new(); + push_instructions(&mut lines, PlanMode::Off); + assert!(lines.is_empty()); + } +} diff --git a/src/core/types.rs b/src/core/types.rs index ac697af..d81a79c 100644 --- a/src/core/types.rs +++ b/src/core/types.rs @@ -135,11 +135,91 @@ pub struct Eval { /// config-level policy rather than extending it. #[serde(default, skip_serializing_if = "Option::is_none")] pub guard: Option, - /// Start the session in the harness's native plan mode and continue it in - /// act mode once the presented plan is approved. Appended last so an eval - /// that declares none serializes exactly as it did before the field existed. - #[serde(default, skip_serializing_if = "std::ops::Not::not")] - pub plan_mode: bool, + /// Whether the session starts in the harness's native plan mode, and what + /// happens once the plan is presented. Appended last so an eval that + /// declares none serializes exactly as it did before the field existed. + #[serde(default, skip_serializing_if = "PlanMode::is_off")] + pub plan_mode: PlanMode, + /// A plan written before the run, named as a path beneath `/evals/` + /// (under [`Self::files_root`] when one is set). Its text is spliced into + /// the dispatch prompt as an already-approved plan and the session runs in + /// act mode, so this needs no plan-mode capability from the harness. + /// Mutually exclusive with [`Self::plan_mode`]. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub plan_source: Option, +} + +/// What an eval's `plan_mode` declaration asks for. +/// +/// Tri-state on the wire, but only one spelling is new. `false` and an absent +/// key are [`Self::Off`]; `true` is [`Self::PlanThenAct`] and serializes back as +/// `true`, so every evals.json and dispatch.json written before this existed +/// round-trips byte-identically. `"plan_only"` is the addition: the plan is the +/// deliverable and the session ends once it is presented. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum PlanMode { + /// Not a plan-mode eval: the session opens in act mode. + #[default] + Off, + /// Plan, approve, then implement in the same session. + PlanThenAct, + /// Plan and stop. `outputs/plan.md` is the run's output. + PlanOnly, +} + +impl PlanMode { + /// Whether the opening round is dispatched with the harness's plan + /// arguments. True for both plan-mode shapes. + pub fn starts_in_plan_mode(self) -> bool { + !self.is_off() + } + + /// Whether the runner approves the presented plan and resumes the session + /// in act mode. False for a plan-only eval, which stops at the plan. + pub fn implements_the_plan(self) -> bool { + matches!(self, Self::PlanThenAct) + } + + /// Whether the eval declares no plan mode. The `skip_serializing_if` + /// predicate that keeps the key off a non-plan-mode eval. + pub fn is_off(&self) -> bool { + matches!(self, Self::Off) + } +} + +impl Serialize for PlanMode { + fn serialize(&self, serializer: S) -> Result { + match self { + Self::Off => serializer.serialize_bool(false), + Self::PlanThenAct => serializer.serialize_bool(true), + Self::PlanOnly => serializer.serialize_str("plan_only"), + } + } +} + +impl<'de> Deserialize<'de> for PlanMode { + fn deserialize>(deserializer: D) -> Result { + // Untagged rather than hand-rolled visitor: the accepted shapes are a + // bool and two strings, and the error below is clearer than serde's + // "data did not match any variant" for a misspelled shape. + #[derive(Deserialize)] + #[serde(untagged)] + enum Declared { + Shorthand(bool), + Named(String), + } + match Declared::deserialize(deserializer)? { + Declared::Shorthand(false) => Ok(Self::Off), + Declared::Shorthand(true) => Ok(Self::PlanThenAct), + Declared::Named(name) => match name.as_str() { + "plan_then_act" => Ok(Self::PlanThenAct), + "plan_only" => Ok(Self::PlanOnly), + other => Err(serde::de::Error::custom(format!( + "unknown plan_mode {other:?}: expected true, false, \"plan_then_act\", or \"plan_only\"" + ))), + }, + } + } } /// Authored shell-command allowances for a guarded eval run. @@ -635,15 +715,20 @@ pub struct ConversationRecord { pub struct PlanRecord { /// The plan-phase round whose output was taken as the plan. pub presented_in_round: u32, - /// The round the runner's fixed approval opened in act mode. - pub approved_in_round: u32, + /// The round the runner's fixed approval opened in act mode. Absent on a + /// plan-only run, which presents its plan and stops without one. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub approved_in_round: Option, pub signal: PlanSignal, /// Where the plan text was saved. #[serde(default, skip_serializing_if = "Option::is_none")] pub artifact_path: Option, } -/// What marked a plan as presented. +/// What marked a plan as presented. Tried in this order: a native plan file is +/// the most specific evidence, a responder's judgement the next, and the +/// planning round's final message the fallback that always exists — the +/// dispatch prompt tells a planning agent to close its turn with the plan. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum PlanSignal { @@ -651,6 +736,9 @@ pub enum PlanSignal { PlanFile, /// The responder judged the agent finished planning. Responder, + /// Neither of the above was available, so the planning round's final + /// message was taken as the plan. + FinalMessage, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -679,8 +767,9 @@ pub enum ConversationStopReason { /// reached. A bounded conversation, not a failed one. MaxTurnsReached, /// A plan-mode session ended its planning phase without presenting a plan - /// the runner could approve: the harness's plan file was never written and - /// the eval declared no responder to decide otherwise. + /// the runner could approve. Retained for reading artifacts written before + /// the planning round's final message became the last plan signal: a + /// planning phase now always has a plan to record, so nothing produces this. PlanNotPresented, } @@ -882,7 +971,8 @@ mod tests { codebase: None, responder: None, guard: None, - plan_mode: false, + plan_mode: PlanMode::Off, + plan_source: None, }; let out = serde_json::to_value(&eval).unwrap(); assert!(out.get("files").is_none()); @@ -960,7 +1050,7 @@ mod tests { let plan = PlanRecord { presented_in_round: 1, - approved_in_round: 2, + approved_in_round: Some(2), signal: PlanSignal::PlanFile, artifact_path: Some("outputs/plan.md".into()), }; @@ -973,6 +1063,22 @@ mod tests { "artifact_path": "outputs/plan.md" }) ); + // A plan-only run has no approval round, so the key is absent rather + // than carrying a round that never happened. + let plan_only = PlanRecord { + presented_in_round: 1, + approved_in_round: None, + signal: PlanSignal::FinalMessage, + artifact_path: Some("outputs/plan.md".into()), + }; + assert_eq!( + serde_json::to_value(&plan_only).unwrap(), + json!({ + "presented_in_round": 1, + "signal": "final_message", + "artifact_path": "outputs/plan.md" + }) + ); assert_eq!( serde_json::to_value(ConversationStopReason::PlanNotPresented).unwrap(), json!("plan_not_presented") @@ -983,18 +1089,90 @@ mod tests { ); } + /// `plan_mode` is tri-state on the wire but its two older spellings are + /// unchanged: an eval that declares nothing serializes without the key, and + /// one that declares `true` serializes back as `true`. Only `"plan_only"` + /// is new, so no existing evals.json or dispatch.json shifts a byte. #[test] - fn plan_mode_true_round_trips() { + fn plan_mode_round_trips_every_spelling() { + let cases = [ + (json!(true), PlanMode::PlanThenAct, Some(json!(true))), + ( + json!("plan_then_act"), + PlanMode::PlanThenAct, + Some(json!(true)), + ), + ( + json!("plan_only"), + PlanMode::PlanOnly, + Some(json!("plan_only")), + ), + (json!(false), PlanMode::Off, None), + ]; + for (declared, expected, serialized) in cases { + let eval: Eval = serde_json::from_value(json!({ + "id": "e1", + "prompt": "p", + "expected_output": "o", + "plan_mode": declared.clone() + })) + .unwrap_or_else(|error| panic!("{declared} should parse: {error}")); + assert_eq!(eval.plan_mode, expected, "parsed {declared}"); + let out = serde_json::to_value(&eval).unwrap(); + assert_eq!( + out.get("plan_mode"), + serialized.as_ref(), + "serialized {declared}" + ); + } + } + + /// An eval that declares no plan mode keeps serializing exactly as it did + /// before the field was tri-state. + #[test] + fn plan_mode_absent_stays_absent() { let eval: Eval = serde_json::from_value(json!({ "id": "e1", "prompt": "p", - "expected_output": "o", - "plan_mode": true + "expected_output": "o" })) .unwrap(); - assert!(eval.plan_mode); - let out = serde_json::to_value(&eval).unwrap(); - assert_eq!(out["plan_mode"], Value::Bool(true)); + assert_eq!(eval.plan_mode, PlanMode::Off); + assert!( + serde_json::to_value(&eval) + .unwrap() + .get("plan_mode") + .is_none() + ); + } + + /// A misspelled shape is rejected at parse time, naming both spellings, so + /// the author fixes it before a campaign is built rather than after. + #[test] + fn plan_mode_rejects_an_unknown_spelling() { + let error = serde_json::from_value::(json!({ + "id": "e1", + "prompt": "p", + "expected_output": "o", + "plan_mode": "planning" + })) + .expect_err("an unknown plan_mode spelling is an error") + .to_string(); + assert!(error.contains("plan_then_act"), "{error}"); + assert!(error.contains("plan_only"), "{error}"); + } + + /// The two questions the rest of the code asks: does this session open in + /// plan mode, and does it go on to implement what it planned? + #[test] + fn plan_mode_answers_the_two_phase_questions() { + assert!(!PlanMode::Off.starts_in_plan_mode()); + assert!(PlanMode::PlanThenAct.starts_in_plan_mode()); + assert!(PlanMode::PlanOnly.starts_in_plan_mode()); + + assert!(!PlanMode::Off.implements_the_plan()); + assert!(PlanMode::PlanThenAct.implements_the_plan()); + assert!(!PlanMode::PlanOnly.implements_the_plan()); } #[test] diff --git a/src/pipeline/grade/evidence.rs b/src/pipeline/grade/evidence.rs index aa10626..216831b 100644 --- a/src/pipeline/grade/evidence.rs +++ b/src/pipeline/grade/evidence.rs @@ -379,11 +379,14 @@ fn render_plan(run_record: &RunRecord, outputs_dir: &Path) -> Option { PLAN_BYTE_LIMIT, &artifact_path(&plan_path), ); + let outcome = match plan.approved_in_round { + Some(round) => format!("approved in round {round}"), + None => "the session ended after planning".to_string(), + }; Some(Rendered { content: format!( - "Presented in round {}; approved in round {} (signal: `{}`).\n\n{}", + "Presented in round {}; {outcome} (signal: `{}`).\n\n{}", plan.presented_in_round, - plan.approved_in_round, serialized_label(&plan.signal), body.content ), @@ -709,6 +712,56 @@ mod tests { ); } + /// A plan-only run has no approval round, so the header says the session + /// ended after planning rather than naming a round that never happened. + /// The judge reads the same plan text either way. + #[test] + fn a_plan_only_run_renders_its_plan_without_an_approval_round() { + let temp = tempfile::TempDir::new().unwrap(); + let run_dir = temp.path().join("eval-add-cache/with_skill"); + let outputs_dir = run_dir.join("outputs"); + fs::create_dir_all(&outputs_dir).unwrap(); + fs::write(outputs_dir.join("plan.md"), "1. Add an LRU\n").unwrap(); + + let mut record = serde_json::to_value(realistic_record()).unwrap(); + record["conversation"]["plan"] = json!({ + "presented_in_round": 1, + "signal": "final_message", + "artifact_path": outputs_dir.join("plan.md").to_string_lossy() + }); + let record: RunRecord = serde_json::from_value(record).unwrap(); + + let bundle = build_evidence_bundle( + &record, + &run_dir.join("run.json"), + &outputs_dir, + &run_dir.join("judge-evidence.md"), + ) + .unwrap(); + + assert!(bundle.content.contains("## Plan"), "{}", bundle.content); + assert!( + bundle.content.contains("Presented in round 1"), + "{}", + bundle.content + ); + assert!( + bundle.content.contains("the session ended after planning"), + "{}", + bundle.content + ); + assert!( + !bundle.content.contains("approved in round"), + "no round approved this plan: {}", + bundle.content + ); + assert!( + bundle.content.contains("1. Add an LRU"), + "{}", + bundle.content + ); + } + /// The plan artifact lives where the driver wrote it — inside the task /// environment's outputs — which is not the directory the judge stage /// resolves for raw harness outputs. The record's own path is what locates diff --git a/src/validation/evals.rs b/src/validation/evals.rs index 03ce668..dff943f 100644 --- a/src/validation/evals.rs +++ b/src/validation/evals.rs @@ -21,6 +21,7 @@ pub fn validate_evals_config(config: &Value, source: &str) -> Result Result<(), Ok(()) } +/// Reject the three ways an eval's plan declarations can be self-contradictory: +/// a `plan_mode` the schema does not recognize, a `plan_source` beside a +/// `plan_mode` (a run either writes its own plan or is handed one), and a +/// plan-only eval that also scripts follow-up turns — that session ends the +/// moment the plan is presented, so every scripted turn would be dropped +/// without a word. Each is reported by name rather than left to the schema. +fn validate_plan_declarations(config: &Value, source: &str) -> Result<(), ValidationError> { + let evals = config.get("evals").and_then(Value::as_array); + for (index, eval) in evals.into_iter().flatten().enumerate() { + let id = || { + eval.get("id") + .and_then(Value::as_str) + .map_or_else(|| format!("evals[{index}]"), str::to_string) + }; + let invalid = |message: String| ValidationError::InvalidConfig { + path: source.to_string(), + message, + }; + let plan_mode = eval.get("plan_mode"); + // Checked before the schema for the same reason the clash below is: + // the schema states the shapes as an `anyOf`, which reports the value + // as matching none of them without ever naming what would have. + if let Some(Value::String(name)) = plan_mode + && !matches!(name.as_str(), "plan_then_act" | "plan_only") + { + return Err(invalid(format!( + "eval '{}': unknown plan_mode {name:?}; expected true, false, \"plan_then_act\", \ + or \"plan_only\"", + id() + ))); + } + let starts_in_plan_mode = match plan_mode { + Some(Value::Bool(declared)) => *declared, + Some(Value::String(_)) => true, + _ => false, + }; + if starts_in_plan_mode && eval.get("plan_source").is_some() { + return Err(invalid(format!( + "eval '{}': declares both 'plan_mode' and 'plan_source'; a run either writes its \ + own plan or is handed one, not both", + id() + ))); + } + let plan_only = plan_mode.and_then(Value::as_str) == Some("plan_only"); + if plan_only && eval.get("turns").is_some() { + return Err(invalid(format!( + "eval '{}': declares both 'plan_mode: plan_only' and 'turns'; a plan-only run \ + ends once the plan is presented, so no scripted turn could be delivered. Use \ + 'plan_mode: true' to implement the plan, or drop the turns", + id() + ))); + } + } + Ok(()) +} + fn validate_codebase(source: &str, label: &str, value: &Value) -> Result<(), ValidationError> { // A non-object is a plain type error the schema words perfectly well. let Some(fields) = value.as_object() else { @@ -602,6 +659,91 @@ mod tests { assert!(error.contains("turns"), "{error}"); } + /// A plan-only eval stops the moment its plan is presented, so scripted + /// follow-ups could never be delivered. Saying so beats silently dropping + /// turns the author wrote. + #[test] + fn rejects_plan_only_with_scripted_turns() { + let mut config = base(); + config["evals"][0]["plan_mode"] = json!("plan_only"); + config["evals"][0]["turns"] = json!([{ "prompt": "go on", "deliver_when": "always" }]); + + let error = validate_evals_config(&config, "evals.json") + .unwrap_err() + .to_string(); + + assert!(error.contains("plan_only"), "{error}"); + assert!(error.contains("turns"), "{error}"); + } + + /// A responder is still welcome on a plan-only eval: it answers the agent's + /// questions during the planning phase, which is the phase that runs. + #[test] + fn accepts_plan_only_with_a_responder() { + let mut config = base(); + config["evals"][0]["plan_mode"] = json!("plan_only"); + config["evals"][0]["responder"] = json!({ "type": "llm" }); + + validate_evals_config(&config, "evals.json") + .expect("a plan-only eval may have a responder"); + } + + /// Plan-then-act still scripts follow-ups after the approval, so it keeps + /// working exactly as it did. + #[test] + fn accepts_plan_then_act_with_scripted_turns() { + let mut config = base(); + config["evals"][0]["plan_mode"] = json!(true); + config["evals"][0]["turns"] = json!([{ "prompt": "go on", "deliver_when": "always" }]); + + validate_evals_config(&config, "evals.json") + .expect("a plan-then-act eval scripts turns after its approval"); + } + + /// A run either writes its own plan or is handed one. Declaring both asks + /// the agent to plan from scratch while holding a plan it was already given. + #[test] + fn rejects_plan_source_together_with_plan_mode() { + let mut config = base(); + config["evals"][0]["plan_mode"] = json!(true); + config["evals"][0]["plan_source"] = json!("plans/add-cache.md"); + + let error = validate_evals_config(&config, "evals.json") + .unwrap_err() + .to_string(); + + assert!(error.contains("plan_source"), "{error}"); + assert!(error.contains("plan_mode"), "{error}"); + } + + /// A misspelled `plan_mode` is the schema's error to report, so the + /// exclusivity check must not claim the eval declared plan mode and mask it. + #[test] + fn an_unknown_plan_mode_spelling_is_reported_as_such_beside_a_plan_source() { + let mut config = base(); + config["evals"][0]["plan_mode"] = json!("planning"); + config["evals"][0]["plan_source"] = json!("plans/add-cache.md"); + + let error = validate_evals_config(&config, "evals.json") + .unwrap_err() + .to_string(); + + assert!( + error.contains("plan_then_act") || error.contains("plan_only"), + "the spelling is the problem to report: {error}" + ); + } + + /// On its own, a plan source is an ordinary act-mode eval. + #[test] + fn accepts_a_plan_source_without_plan_mode() { + let mut config = base(); + config["evals"][0]["plan_source"] = json!("plans/add-cache.md"); + + validate_evals_config(&config, "evals.json") + .expect("an eval handed a plan runs in act mode"); + } + /// A bound of zero would dispatch turn 1 and refuse to answer anything, /// which is a one-shot eval written the long way round. #[test] diff --git a/tests/cli/basics.rs b/tests/cli/basics.rs index e8d358f..b91c662 100644 --- a/tests/cli/basics.rs +++ b/tests/cli/basics.rs @@ -174,7 +174,9 @@ fn dispatch_help_documents_conversation_verification() { .stdout(contains("duration_ms")) .stdout(contains("responder consultations, judges, queueing")) .stdout(contains("timed_out")) - .stdout(contains("plan_not_presented")); + // Both plan-mode shapes and where the plan comes from. + .stdout(contains("outputs/plan.md")) + .stdout(contains("plan_only")); } #[test] diff --git a/tests/cli/docs.rs b/tests/cli/docs.rs index db23c40..1447c85 100644 --- a/tests/cli/docs.rs +++ b/tests/cli/docs.rs @@ -159,8 +159,14 @@ fn docs_conversations_keeps_the_plan_mode_contract() { .stdout(contains("\"plan_mode\": true")) .stdout(contains("plan-mode")) .stdout(contains("The plan is approved. Implement it now.")) + // All three plan-ready signals, in the order the runner tries them. .stdout(contains("plan_file")) - .stdout(contains("plan_not_presented")) + .stdout(contains("responder")) + .stdout(contains("final_message")) + // Both plan-mode shapes, and the pre-written-plan alternative. + .stdout(contains("plan_only")) + .stdout(contains("plan_then_act")) + .stdout(contains("plan_source")) .stdout(contains("plan_approval")) .stdout(contains("plan_mode_attributed")) .stdout(contains("plan.md")); diff --git a/tests/run/plan_mode.rs b/tests/run/plan_mode.rs index e3f89f0..378f831 100644 --- a/tests/run/plan_mode.rs +++ b/tests/run/plan_mode.rs @@ -64,18 +64,14 @@ fn a_plan_mode_eval_rejects_a_harness_without_native_plan_mode() { ); } -/// OpenCode writes no plan file, so nothing but a responder can tell when the -/// agent has presented its plan: a plan-mode eval there must declare one. +/// OpenCode writes no plan file, but the dispatch prompt asks every planning +/// round to close with its plan, so the final message is the signal and a +/// responder is optional there as it is on Claude Code. #[test] -fn a_plan_mode_eval_without_a_responder_needs_a_plan_file_signal() { +fn a_plan_mode_eval_needs_no_responder_on_a_harness_without_a_plan_file() { let tmp = tempfile::TempDir::new().unwrap(); let (skill_dir, cwd) = setup(tmp.path(), PLAN_MODE_EVALS); - run_dry(&skill_dir, &cwd, "opencode").failure().stderr( - contains("responder") - .and(contains("plan-first")) - .and(contains("plan_file")) - .and(contains("opencode")), - ); + run_dry(&skill_dir, &cwd, "opencode").success(); let with_responder = PLAN_MODE_EVALS.replace( "\"plan_mode\": true", @@ -95,7 +91,7 @@ fn a_plan_mode_eval_is_announced_and_recorded_per_task() { let (skill_dir, cwd) = setup(tmp.path(), PLAN_MODE_EVALS); run_dry(&skill_dir, &cwd, "claude-code") .success() - .stdout(contains("plan mode: 1 eval")); + .stdout(contains("plan mode: 1 plan-then-act eval")); let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); let tasks = dispatch["tasks"].as_array().unwrap(); @@ -109,6 +105,23 @@ fn a_plan_mode_eval_is_announced_and_recorded_per_task() { } } +/// The run plan tells the operator which shape each plan-mode eval is, because +/// "plan mode" alone no longer says whether the run implements anything. +#[test] +fn the_run_plan_names_each_plan_mode_shape() { + let both_shapes = PLAN_MODE_EVALS.replace( + r#"{ "id": "plain", "prompt": "review this MR", "expected_output": "a review" }"#, + r#"{ "id": "plan-only", "prompt": "how would you cache this?", + "expected_output": "a plan", "plan_mode": "plan_only" }"#, + ); + let tmp = tempfile::TempDir::new().unwrap(); + let (skill_dir, cwd) = setup(tmp.path(), &both_shapes); + + run_dry(&skill_dir, &cwd, "claude-code") + .success() + .stdout(contains("1 plan-then-act").and(contains("1 plan-only"))); +} + // ── The plan phase, driven end to end ───────────────────────────────────────── // // Each test below swaps the frozen descriptor's two templates for one POSIX stub @@ -289,7 +302,7 @@ fn conversation_of(cwd: &Path) -> (Value, Value) { (task, conversation) } -const PLAN_ONLY: &str = r#"{ +const ONE_PLAN_EVAL: &str = r#"{ "skill_name": "mr-review", "evals": [{ "id": "plan-first", @@ -302,7 +315,7 @@ const PLAN_ONLY: &str = r#"{ #[test] fn a_presented_plan_is_approved_and_the_session_continues_in_act_mode() { let tmp = tempfile::TempDir::new().unwrap(); - let (skill_dir, cwd, rounds) = prepare(tmp.path(), PLAN_ONLY, "claude-code", &[]); + let (skill_dir, cwd, rounds) = prepare(tmp.path(), ONE_PLAN_EVAL, "claude-code", &[]); let plan = "1. Fix add()\n2. Add a regression test\n"; round(&rounds, "initial", "plan", None, &plan_write_events(plan)); round( @@ -348,37 +361,76 @@ fn a_presented_plan_is_approved_and_the_session_continues_in_act_mode() { ); } +/// With no plan file written and no responder to ask, the planning round's +/// final message is the plan. The run gets an inspectable `plan.md` and +/// continues into act mode rather than stopping empty-handed. #[test] -fn a_plan_phase_without_a_plan_file_and_no_responder_stops_plan_not_presented() { +fn a_plan_phase_without_a_plan_file_or_responder_takes_the_final_message() { let tmp = tempfile::TempDir::new().unwrap(); - let (skill_dir, cwd, rounds) = prepare(tmp.path(), PLAN_ONLY, "claude-code", &[]); + let (skill_dir, cwd, rounds) = prepare(tmp.path(), ONE_PLAN_EVAL, "claude-code", &[]); + let plan = "1. Add an LRU in pricing.py\n2. Cover eviction with a test\n"; + round(&rounds, "initial", "plan", None, &[init(), result(plan)]); round( &rounds, - "initial", - "plan", - None, - &[init(), result("Which module owns the pricing client?")], + "resume-2", + "bypassPermissions", + Some(APPROVAL), + &edit_events("Implemented."), ); - dispatch(&skill_dir, &cwd, "claude-code", tmp.path()) - .success() - .stderr(contains("without presenting a plan")); + dispatch(&skill_dir, &cwd, "claude-code", tmp.path()).success(); + + let (task, conversation) = conversation_of(&cwd); + assert_eq!(conversation["status"], "completed", "{conversation}"); + assert!(conversation.get("stop_reason").is_none(), "{conversation}"); + assert_eq!(conversation["plan"]["signal"], "final_message"); + assert_eq!(conversation["plan"]["presented_in_round"], 1); + assert_eq!(conversation["plan"]["approved_in_round"], 2); + let outputs = Path::new(task["outputs_dir"].as_str().unwrap()); + assert_eq!(fs::read_to_string(outputs.join("plan.md")).unwrap(), plan); +} + +/// A `plan_only` eval stops at the plan: `plan.md` is the whole output, no +/// approval is sent, and the session never enters act mode. +#[test] +fn a_plan_only_eval_stops_once_the_plan_is_presented() { + let tmp = tempfile::TempDir::new().unwrap(); + let evals = ONE_PLAN_EVAL.replace("\"plan_mode\": true", "\"plan_mode\": \"plan_only\""); + let (skill_dir, cwd, rounds) = prepare(tmp.path(), &evals, "claude-code", &[]); + let plan = "1. Fix add()\n2. Add a regression test\n"; + round(&rounds, "initial", "plan", None, &plan_write_events(plan)); + + dispatch(&skill_dir, &cwd, "claude-code", tmp.path()).success(); let (task, conversation) = conversation_of(&cwd); - assert_eq!(conversation["status"], "stopped", "{conversation}"); - assert_eq!(conversation["stop_reason"], "plan_not_presented"); - assert_eq!(conversation["stopped_before_followup"], 1); - assert!(conversation.get("plan").is_none()); - assert_eq!(conversation["events"].as_array().unwrap().len(), 1); + assert_eq!(conversation["status"], "completed", "{conversation}"); + assert_eq!(conversation["delivered_followups"], 0); + assert_eq!(conversation["plan"]["presented_in_round"], 1); + assert!( + conversation["plan"].get("approved_in_round").is_none(), + "no round approved a plan-only run: {conversation}" + ); + assert_eq!(conversation["plan"]["signal"], "plan_file"); + let events = conversation["events"].as_array().unwrap(); + assert_eq!(events.len(), 1, "only the seeded prompt: {conversation}"); + assert_eq!(events[0]["mode"], "plan"); + let outputs = Path::new(task["outputs_dir"].as_str().unwrap()); - assert!(!outputs.join("turn-2").exists()); - assert!(!outputs.join("plan.md").exists()); + assert_eq!(fs::read_to_string(outputs.join("plan.md")).unwrap(), plan); + assert!( + !outputs.join("turn-2").exists(), + "a plan-only eval never resumes in act mode" + ); + + // The prompt the agent read says the plan is the deliverable. + let prompt = fs::read_to_string(task["dispatch_prompt_path"].as_str().unwrap()).unwrap(); + assert!(prompt.contains("nothing to implement"), "{prompt}"); } #[test] fn the_responder_answers_plan_phase_questions_until_the_plan_is_presented() { let tmp = tempfile::TempDir::new().unwrap(); - let evals = PLAN_ONLY.replace( + let evals = ONE_PLAN_EVAL.replace( "\"plan_mode\": true", "\"plan_mode\": true, \"responder\": { \"type\": \"llm\" }", ); @@ -446,7 +498,7 @@ fn the_responder_answers_plan_phase_questions_until_the_plan_is_presented() { #[test] fn scripted_turns_follow_the_approval() { let tmp = tempfile::TempDir::new().unwrap(); - let evals = PLAN_ONLY.replace( + let evals = ONE_PLAN_EVAL.replace( "\"plan_mode\": true", "\"plan_mode\": true, \"turns\": [{ \"prompt\": \"Also add a metric.\", \"deliver_when\": \"always\" }]", ); @@ -501,7 +553,7 @@ fn scripted_turns_follow_the_approval() { #[test] fn a_signal_less_harness_approves_on_the_responders_done_verdict() { let tmp = tempfile::TempDir::new().unwrap(); - let evals = PLAN_ONLY.replace( + let evals = ONE_PLAN_EVAL.replace( "\"plan_mode\": true", "\"plan_mode\": true, \"responder\": { \"type\": \"llm\" }", ); @@ -634,7 +686,7 @@ act_args = " --permission-mode act" #[test] fn the_write_guard_allows_the_declared_plan_file_root() { let tmp = tempfile::TempDir::new().unwrap(); - let (skill_dir, cwd) = setup(tmp.path(), PLAN_ONLY); + let (skill_dir, cwd) = setup(tmp.path(), ONE_PLAN_EVAL); skill_eval() .current_dir(&cwd) .env("HOME", tmp.path()) @@ -668,3 +720,121 @@ fn the_write_guard_allows_the_declared_plan_file_root() { "{roots:?}" ); } + +/// A plan-only eval may still declare a responder, which answers the agent's +/// questions while it plans. The planning phase then runs for more than one +/// round — resuming the session in plan mode — and stops at the plan. +#[test] +fn a_plan_only_eval_may_plan_across_several_rounds_with_a_responder() { + let tmp = tempfile::TempDir::new().unwrap(); + let evals = ONE_PLAN_EVAL.replace( + "\"plan_mode\": true", + "\"plan_mode\": \"plan_only\", \"responder\": { \"type\": \"llm\" }", + ); + let (skill_dir, cwd, rounds) = prepare( + tmp.path(), + &evals, + "claude-code", + &["--responder-model", "test-responder-model"], + ); + round( + &rounds, + "initial", + "plan", + None, + &[ + init(), + result("Redis (needs a service) or an in-memory LRU (recommended)?"), + ], + ); + fs::write( + rounds.join("verdict-1.json"), + json!({"verdict": "answer", "reply": "The in-memory LRU, please.", + "rationale": "took the recommended option"}) + .to_string(), + ) + .unwrap(); + let plan = "1. Add an in-memory LRU\n2. Cover eviction\n"; + round( + &rounds, + "resume-2", + "plan", + Some("The in-memory LRU, please."), + &plan_write_events(plan), + ); + + dispatch(&skill_dir, &cwd, "claude-code", tmp.path()).success(); + + let (task, conversation) = conversation_of(&cwd); + assert_eq!(conversation["status"], "completed", "{conversation}"); + assert_eq!(conversation["plan"]["presented_in_round"], 2); + assert!( + conversation["plan"].get("approved_in_round").is_none(), + "{conversation}" + ); + let modes: Vec<&str> = conversation["events"] + .as_array() + .unwrap() + .iter() + .map(|event| event["mode"].as_str().unwrap()) + .collect(); + assert_eq!(modes, ["plan", "plan"], "{conversation}"); + + let outputs = Path::new(task["outputs_dir"].as_str().unwrap()); + assert_eq!(fs::read_to_string(outputs.join("plan.md")).unwrap(), plan); + assert!(!outputs.join("turn-3").exists(), "no act round follows"); +} + +// ── Handing an eval a plan that was written earlier ─────────────────────────── + +const SUPPLIED_PLAN: &str = r#"{ + "skill_name": "mr-review", + "evals": [{ + "id": "execute-plan", + "prompt": "Implement the approved plan.", + "expected_output": "the plan carried out", + "plan_source": "plans/add-cache.md" + }] +}"#; + +/// A `plan_source` needs nothing from the harness — the session is ordinary act +/// mode — so it reaches Codex, whose descriptor declares no `[plan_mode]` at +/// all. The plan text lands in the prompt the agent reads, ahead of the +/// request, and the task records where it came from. +#[test] +fn a_supplied_plan_reaches_a_harness_without_plan_mode() { + let tmp = tempfile::TempDir::new().unwrap(); + let (skill_dir, cwd) = setup(tmp.path(), SUPPLIED_PLAN); + let plan = "1. Add an LRU to pricing.py\n2. Cover eviction with a test\n"; + let plans = skill_dir.join("mr-review/evals/plans"); + fs::create_dir_all(&plans).unwrap(); + fs::write(plans.join("add-cache.md"), plan).unwrap(); + + run_dry(&skill_dir, &cwd, "codex").success(); + + let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); + let tasks = dispatch["tasks"].as_array().unwrap(); + assert_eq!(tasks.len(), 2, "one eval × two conditions"); + for task in tasks { + assert_eq!(task["plan_source"], "plans/add-cache.md", "{task}"); + assert!( + task.get("plan_mode").is_none(), + "a supplied plan is act mode: {task}" + ); + let prompt = fs::read_to_string(task["dispatch_prompt_path"].as_str().unwrap()).unwrap(); + assert!(prompt.contains("already written and approved"), "{prompt}"); + assert!(prompt.contains("1. Add an LRU to pricing.py"), "{prompt}"); + } +} + +/// A named plan that is not on disk fails the run rather than dispatching a +/// prompt whose plan is quietly missing. +#[test] +fn a_missing_plan_source_fails_the_run() { + let tmp = tempfile::TempDir::new().unwrap(); + let (skill_dir, cwd) = setup(tmp.path(), SUPPLIED_PLAN); + + run_dry(&skill_dir, &cwd, "codex") + .failure() + .stderr(contains("plan source").and(contains("add-cache.md"))); +}