From 7407d5d42d0f47c0cac5da10ab2cc31e36411448 Mon Sep 17 00:00:00 2001 From: "jerod.wilkerson" <30474318+jerodw@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:56:19 -0600 Subject: [PATCH 1/3] Record the mandate conferred for story-155 --- .harness/history/mandate-log.jsonl | 1 + 1 file changed, 1 insertion(+) diff --git a/.harness/history/mandate-log.jsonl b/.harness/history/mandate-log.jsonl index f1fbe37..c70ef7e 100644 --- a/.harness/history/mandate-log.jsonl +++ b/.harness/history/mandate-log.jsonl @@ -60,3 +60,4 @@ {"story_id": "story-152", "timestamp": "2026-09-16 09:53:13", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false} {"story_id": "story-153", "timestamp": "2026-09-16 12:03:52", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false} {"story_id": "story-154", "timestamp": "2026-09-16 14:56:07", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false} +{"story_id": "story-155", "timestamp": "2026-09-16 18:56:19", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false} From 5124283013b231268c747b2518436a21ceb49cc5 Mon Sep 17 00:00:00 2001 From: "jerod.wilkerson" <30474318+jerodw@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:56:19 -0600 Subject: [PATCH 2/3] Plan story-155: A finding about the story's own change is fixed by the stage that owns it, whatever that stage is --- .harness/stories/story-155.yaml | 179 ++++++++++++++++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 .harness/stories/story-155.yaml diff --git a/.harness/stories/story-155.yaml b/.harness/stories/story-155.yaml new file mode 100644 index 0000000..55ac32e --- /dev/null +++ b/.harness/stories/story-155.yaml @@ -0,0 +1,179 @@ +story: + id: story-155 + title: A finding about the story's own change is fixed by the stage that owns it, whatever that stage is + description: | + Planned from the brief filed under 321. + + A verifier that wants a finding about the story's own change fixed has + two routes today, and most findings fit neither. A correctable finding + re-enters at the stage the correction_pass declaration names -- the + documenter in both shipped workflows -- and is bounded to the words + alone. A blocking issue fails the verdict, spends a retry and archives + the attempt. There is nothing between them, so a finding whose fix is + one line in a file the documenter does not own can be had only by + failing the whole run. story-154 routed six such findings and fixed + one; among the five it filed were a prompt never told about the + confinement the story gave its stage, and a slot one verifier prompt + has and its sibling lacks -- both the implementer's file, both not + prose-only, both plainly part of the story they were filed from. + + This story adds a third route, the repair pass: a passing verdict may + carry repairable_findings, each naming a path, a location, the finding, + the correction and a retry category; the coordinator looks the category + up in the verifier stage's retry_routing table exactly as it looks up a + failed verdict's retry_target, re-enters the workflow at the stage the + table names, and runs forward to verification again. It has the + correction pass's cost and the retry routing's reach: nothing a retry + spends is spent -- no retry_count increment, no attempts/attempt-N/ + archive, no retry-history entry, and the passing verdict that routed it + stands -- and the stage entered is granted the paths its findings name + for that attempt alone and is told the finding and the correction and + nothing else. It is bounded by a budget of its own, declared on the + verifier stage as repair_pass beside correction_pass, with a reason + where a reader meets the number. + + Because a repair pass may change behaviour, it is built on a verified + tree: it routes only after the clean-clone check on the routing verdict + has passed, and the verification it returns to runs the check again on + the repaired tree. The correction pass is untouched -- still prose-only, + still entering at the documenter, still its own budget -- and the + verifier's right to decline a finding is untouched too: a routed finding + the verdict does not name is still filed as a brief by story-152's + accounting, and a repairable finding that names a slug counts as acted + on by that same accounting. + workflow: story-workflow + +tasks: + - Add a repair_pass declaration to the verifier stage of both shipped workflow definitions, shaped like correction_pass without a stage key (the stage comes off the retry_routing table), with result "repair-pass.json", budget 2, and a budget_reason recording the judgement behind the number. + - Add repairable_findings to schemas/verification-result.schema.json, an optional array of objects requiring path, location, finding, correction and category, with descriptions saying what the field is for, that it is read only on a passing verdict, that presence is the signal, and that a category is checked against the retry_routing table with the strictness an unrecognised retry_target gets. + - In orchestration/story_coordinator.py, read the declaration off the loaded workflow wherever a stage carries it, hold it to a pre-flight beside correction_pass_problems (a budget that is not a positive integer, or a declaration on a stage that declares no retry_routing table, is refused before any run state is created), and add repair_pass_count to RunState defaulting to zero so an existing state.json still loads and a resumed run cannot spend the pass again. + - In the verifier branch, on a passing verdict, after the clean-clone check has passed and before the run completes, route a repair pass when the verdict carries repairable_findings and the budget is unspent, resolving each finding's category through the stage's retry_routing table, escalating on a category the table does not define (naming the offending categories and the ones the workflow defines), entering at the earliest stage in workflow order among the resolved stages, writing a numbered pass record (pass, attempt, entry stage, the findings each with its resolved stage, and the coordinator's statement), clearing guidance in force, spending no retry, archiving no attempt and appending no retry-history entry. When the budget is spent, record the findings in an event and complete the run as the correction pass does. + - Grant each stage entered during a repair pass the paths of the findings whose category resolves to it, for that attempt alone, on the same exempt list the ownership check and the revert check read, with one event per granted path beside the correction-pass grant events. + - Inject the current pass record into every stage prompt through a repair_pass_result context key in orchestration/context_assembler.py and a {{repair_pass_result}} slot in prompts/story-implementer.md, prompts/story-tester.md, prompts/documenter.md, prompts/refactor-implementer.md, prompts/story-verifier.md and prompts/refactor-verifier.md, described the way the correction-pass slot is. + - In prompts/story-verifier.md and prompts/refactor-verifier.md, add a section beside the correctable-findings one bounding what may go in repairable_findings, a finding judged correct, too small to fail the run, fixable by one stage in the file it names, whose fix is not to the words alone, naming the category that owns the fix spelled as the retry_routing table declares it, and saying that a repair pass is not a licence to revisit the work. + - Extend orchestration/story_inspection.verdict_locations to read repairable_findings beside blocking_issues and correctable_findings, so a repairable finding naming a routed finding's slug counts as acted on and the remainder are still filed, and word the repair-pass re-entry event with routed_among as the correction-pass event is. + - Write tests under tests/ covering the routing, the bound, the budget, the pre-flight, the accounting and both shipped workflows. + - Document the repair pass in .harness/docs/ARCHITECTURE.md, beside the correction-pass section, including why it routes after the clean-clone check and what the budget is set from. + +acceptance_criteria: + - A passing verdict carrying one repairable finding whose category the retry_routing table maps to the implementer re-enters the workflow at the implementer, and afterwards retry_count is unchanged, no attempts/attempt-N/ directory was written, retry-history.json gained no entry, and the archived verdict still records passed. + - The re-entry happens only after the clean-clone check on the routing verdict has run and exited 0; a verdict whose clean-clone check fails takes the existing clean-clone retry route and no repair pass record is written for it. + - The verification the repair pass returns to runs the clean-clone check again. + - The stage entered is granted, for that attempt only, exactly the paths of the findings whose category resolves to it, so an edit to a granted path is exempt from both the ownership check and the revert check and an edit to any other governed path is not; a later attempt of the same stage inherits no grant. + - A repairable finding naming a category the workflow's retry_routing table does not define escalates the run through _escalate, with retry_count untouched and no attempt archived, naming the offending category and the categories the workflow defines. + - When repair_pass_count has reached the declared budget, a passing verdict still carrying repairable findings appends an event naming each finding and the run completes with no further stage invocation. + - A workflow declaring repair_pass with a budget that is not a positive integer, or on a stage that declares no retry_routing table, is refused at pre-flight before any run state is created, with a message naming the problem. + - A workflow declaring no repair_pass key routes a passing verdict carrying repairable_findings nowhere, and its events.log is unchanged from what it was before this story. + - A routed inspection finding whose slug is named in the location of a repairable finding is counted as acted on by the accounting and is not filed; a routed finding no verdict entry names is still filed as a brief. + - Both shipped workflow definitions declare repair_pass with budget 2 and a budget_reason, and both pass the workflow validation the suite already holds them to. + - Every stage prompt named in the tasks carries a {{repair_pass_result}} slot, and the context assembler renders the current pass record into it for the stages a pass enters and None for a run that took none. + - The verification-result schema accepts a verdict carrying repairable_findings whose entries carry path, location, finding, correction and category, and refuses an entry missing any of them. + - Every existing test passes unchanged; the correction pass's behaviour, budget and prose-only bound are as they were. + +technical_plan: + implementation_steps: + - Add the repair_pass declaration to the verifier stage of workflows/story-workflow.json and workflows/refactor-workflow.json, with budget 2 and a budget_reason that records why two rather than one (a separate story to carry a leftover finding costs more than a second pass in the run that found it) and why the pass is bounded (it may change behaviour, so it carries the findings and no guidance, and the verification it returns to runs the clean-clone check on the result). + - Add repairable_findings to schemas/verification-result.schema.json beside correctable_findings, reusing that entry's shape and cross-referencing it in the description. + - In orchestration/story_coordinator.py add repair_pass_count to RunState; add repair_pass_declaration, repair_pass_result_file, repair_pass_grants (keyed on stage and attempt, reading each finding's resolved stage off the record), repair_pass_statement, repair_destination (walks the findings against the routes table, returns the entry stage as the earliest in workflow order among the resolved stages, or the unknown categories), repair_pass_problems for pre-flight, and the routed and recorded event writers; register the pre-flight beside correction_pass_problems in the workflow-loading path. + - In the verifier branch, after the clean-clone block succeeds, add the repair-pass block; on routing set the index to the entry stage and continue, exactly as the correction-pass block does; extend the exempt list at the post-stage checks with repair_pass_grants beside correction_pass_grants; read the current record into a repair_pass_result context value beside correction_pass_result. + - Add repair_pass_result to orchestration/context_assembler.py's context and the slot to the six stage prompts; add the bounding section to both verifier prompts. + - Extend story_inspection.verdict_locations with the new key. + - Tester writes tests/test_repair_pass.py holding the criteria above with a fake runner, plus the accounting and shipped-workflow assertions, adding to existing modules where a corpus test already holds the thing (the shipped-workflow validity test, the verdict-locations tests). + - Documenter adds the section to .harness/docs/ARCHITECTURE.md and updates the correction-pass section's mention that there is nothing between a correctable finding and a blocking issue. + likely_file_changes: + - file: orchestration/story_coordinator.py + stage: implementer + reason: the repair pass declaration reader, pre-flight, state field, routing block, grants, statement, events and prompt injection + - file: orchestration/story_inspection.py + stage: implementer + reason: verdict_locations reads repairable_findings so the accounting counts a named slug as acted on + - file: orchestration/context_assembler.py + stage: implementer + reason: the repair_pass_result context key + - file: schemas/verification-result.schema.json + stage: implementer + reason: the repairable_findings field + - file: workflows/story-workflow.json + stage: implementer + reason: the repair_pass declaration on the verifier stage with budget and reason + - file: workflows/refactor-workflow.json + stage: implementer + reason: the same declaration for the refactor workflow + - file: prompts/story-verifier.md + stage: implementer + reason: the section bounding repairable_findings and the repair-pass slot + - file: prompts/refactor-verifier.md + stage: implementer + reason: the same section and slot for the refactor verifier + - file: prompts/story-implementer.md + stage: implementer + reason: the repair-pass slot so an entered implementer is told what it was entered to fix + - file: prompts/story-tester.md + stage: implementer + reason: the repair-pass slot + - file: prompts/documenter.md + stage: implementer + reason: the repair-pass slot + - file: prompts/refactor-implementer.md + stage: implementer + reason: the repair-pass slot + - file: tests/test_repair_pass.py + stage: tester + reason: the routing, bound, budget, pre-flight, grant and accounting assertions for the repair pass + - file: tests/test_shipped_workflow_is_valid.py + stage: tester + reason: both shipped workflows declare repair_pass and validate + - file: .harness/docs/ARCHITECTURE.md + stage: documenter + reason: the repair pass section and the correction-pass section's statement that no route existed between it and a blocking issue + +scope: + modify: + - orchestration/story_coordinator.py + - orchestration/story_inspection.py + - orchestration/context_assembler.py + - schemas/verification-result.schema.json + - workflows/story-workflow.json + - workflows/refactor-workflow.json + - prompts/story-verifier.md + - prompts/refactor-verifier.md + - prompts/story-implementer.md + - prompts/story-tester.md + - prompts/documenter.md + - prompts/refactor-implementer.md + - tests/ + - .harness/docs/ARCHITECTURE.md + do_not_modify: + - The correction pass: its prose-only bound, its declared entry stage, its budget and its result artifact stay as they are + - The verifier's right to decline a finding, and story-152's accounting that files a declined routed finding as a brief + - orchestration/agent_runner.py + - scripts/ + - hooks/ + - The findings story-154 filed as briefs; this story builds the route and does not fix them + +verification_requirements: + - Confirm from the tests and a fake-runner run that a passing verdict carrying a repairable finding routed to the implementer re-enters there with retry_count, attempts/ and retry-history.json untouched and the archived verdict still passed. + - Confirm the repair-pass block sits after the clean-clone check in the verifier branch and that the check runs again on the verification the pass returns to. + - Confirm the grant is bounded to the paths the findings name, the stage they resolve to and the attempt of the pass, and that both the ownership check and the revert check read it. + - Confirm an undefined category escalates without spending a retry, and a spent budget records the findings and completes the run. + - Confirm the accounting counts a repairable finding naming a routed slug as acted on and still files an unnamed one. + - Confirm both shipped workflows declare repair_pass with budget 2 and a budget_reason, and that the whole suite passes in the clean clone. + - Confirm the correction pass's tests pass unchanged and its prose-only bound is unaltered in both verifier prompts, the schema and the coordinator's statement. + +constraints: + - No stage name, category name or artifact name for the repair pass is written into coordinator logic; each comes off the loaded workflow definition, as every other route does. + - The coordinator stays deterministic; no model call is added outside agent_runner.py. + - The repair pass spends nothing a retry spends; a run that declares no repair_pass key behaves exactly as before. + - A repair pass routes only after the clean-clone check on the routing verdict has passed. + - The correction pass remains prose-only and is not widened; the two passes are independent mechanisms with independent budgets. + - Every routing decision the repair pass makes is reconstructable from state.json and events.log. + - Python 3.10+, standard library only; existing tests are neither weakened nor skipped. + +mandate: + source: + kind: human + conferred_at: 2026-09-16 18:56:19 + conferred_by: jerod.wilkerson <30474318+jerodw@users.noreply.github.com> + recorded_by: l5-plan + +brief_key: 321 From 3bd930455e698a4938e203f58446047c44686638 Mon Sep 17 00:00:00 2001 From: "jerod.wilkerson" <30474318+jerodw@users.noreply.github.com> Date: Wed, 16 Sep 2026 20:18:27 -0600 Subject: [PATCH 3/3] story-155: A finding about the story's own change is fixed by the stage that owns it, whatever that stage is Implemented by the l5 harness story workflow. --- .harness/docs/ARCHITECTURE.md | 41 +- .harness/history/execution-log.jsonl | 1 + .harness/history/inspection-log.jsonl | 6 + .harness/history/retry-log.jsonl | 2 + orchestration/context_assembler.py | 8 + orchestration/story_coordinator.py | 410 +++- orchestration/story_inspection.py | 11 +- prompts/documenter.md | 11 + prompts/refactor-implementer.md | 11 + prompts/refactor-verifier.md | 31 + prompts/story-implementer.md | 11 + prompts/story-tester.md | 11 + prompts/story-verifier.md | 31 + schemas/execution-history.schema.json | 3 + schemas/verification-result.schema.json | 30 + tests/test_baseline_honesty.py | 13 + tests/test_coordinator_contract.py | 6 + tests/test_escalation_resume.py | 3 +- ...est_inspection_findings_reach_the_story.py | 95 + tests/test_repair_pass.py | 1836 +++++++++++++++++ tests/test_shipped_workflow_is_valid.py | 62 + workflows/refactor-workflow.json | 5 + workflows/story-workflow.json | 5 + 23 files changed, 2634 insertions(+), 9 deletions(-) create mode 100644 tests/test_repair_pass.py diff --git a/.harness/docs/ARCHITECTURE.md b/.harness/docs/ARCHITECTURE.md index f8aa3d0..2ed821c 100644 --- a/.harness/docs/ARCHITECTURE.md +++ b/.harness/docs/ARCHITECTURE.md @@ -36,7 +36,7 @@ A stage may also declare an optional `may_only_change` key since story-106, the The verifier stage declares an optional `clean_clone` key, an object naming both things that check needs: `result`, the artifact it writes (`clean-clone-result.json`), and `retry_stage`, the stage a clean-clone failure routes to. The key is what turns the check on: the coordinator reads `stage.get("clean_clone")` and does nothing when it is absent, so removing the declaration disables the check with no change to orchestration code, and neither the artifact name nor the destination appears there. It was a bare string until story-028 needed a second name and widened it rather than adding a second key, exactly as `revert_check` was widened in story-019 — one declaration, so the check and its route cannot be switched on independently and disagree. Its route is not one of the verifier's retry categories: nothing *chooses* it, so it carries no `when` and is never rendered into a prompt, but it is held to the same pre-flight as the categories are. It sits on the verifier because the check runs on the verifier's passing verdict, in that stage's branch, before routing. Since story-045 the verifier is the *last* stage, so the tree the check clones already holds the documenter's edits — the declaration, the mechanics and the recorded result are unchanged, and only what is in the tree when it runs changed, as a consequence of the reorder rather than of an edit to the check. -The verifier stage also declares an optional `correction_pass` key since story-055, an object naming the artifact the coordinator writes (`result`, `correction-pass.json`), the budget of passes a run may take (`budget`, `1`), and since story-067 the stage a pass enters at (`stage`, the documenter). It is shaped exactly like the `clean_clone` declaration beside it — `stage` is that declaration's `retry_stage` under another name — and turns the mechanism on the same way: the coordinator reads the declaration and does nothing when it is absent, so deleting the key disables the whole correction pass with no change to orchestration code, and neither the artifact name nor the destination appears there. The entry stage was derived rather than declared until story-067: it was the earliest stage in workflow order among the `retry_routing` destinations the findings' categories named, which is why a one-word fix could cost an implementer. The table is still read — an undeclared category still escalates — but it no longer chooses where a pass goes, so the declaration now carries a destination of its own. Being declared makes it something a definition can be wrong about, so it is held to a pre-flight (`correction_pass_problems`) like the routing table and the self-route budgets are: a `stage` the loaded workflow does not define is refused before any run state is created, naming the undefined stage and the stages the workflow does define. `correction_pass_declaration(stages)` finds it wherever a stage carries it rather than looking it up by stage name. See "A minor, correct finding has somewhere to go" below. +The verifier stage also declares an optional `correction_pass` key since story-055, an object naming the artifact the coordinator writes (`result`, `correction-pass.json`), the budget of passes a run may take (`budget`, `1`), and since story-067 the stage a pass enters at (`stage`, the documenter). It is shaped exactly like the `clean_clone` declaration beside it — `stage` is that declaration's `retry_stage` under another name — and turns the mechanism on the same way: the coordinator reads the declaration and does nothing when it is absent, so deleting the key disables the whole correction pass with no change to orchestration code, and neither the artifact name nor the destination appears there. The entry stage was derived rather than declared until story-067: it was the earliest stage in workflow order among the `retry_routing` destinations the findings' categories named, which is why a one-word fix could cost an implementer. The table is still read — an undeclared category still escalates — but it no longer chooses where a pass goes, so the declaration now carries a destination of its own. Being declared makes it something a definition can be wrong about, so it is held to a pre-flight (`correction_pass_problems`) like the routing table and the self-route budgets are: a `stage` the loaded workflow does not define is refused before any run state is created, naming the undefined stage and the stages the workflow does define. `correction_pass_declaration(stages)` finds it wherever a stage carries it rather than looking it up by stage name. See "A minor, correct finding has somewhere to go" below. Since story-155 the same stage also declares `repair_pass` beside it — `result` (`repair-pass.json`), `budget` (`2`) and a `budget_reason`, and deliberately no `stage`: a repairable finding's category is resolved through the `retry_routing` table below, so the pass enters whichever stage owns the file. Its pre-flight, `repair_pass_problems`, refuses a non-positive budget and a declaration on a stage that carries no routing table. See "A finding about the story's own change is fixed by the stage that owns it" below. The tester stage declares an optional `suite_run` key since story-066, an object naming the artifact the coordinator writes (`result`, `suite-run-result.json`) and nothing else. It is shaped like the `clean_clone` and `correction_pass` declarations above it and turns the mechanism on the same way: the coordinator reads `stage.get("suite_run")` off the loaded stage dict and does nothing when it is absent, so a target whose stage should keep running its own suite declares nothing and is left exactly as it was before the key existed, with no change to orchestration and no stage name at the call site. `suite_run_declaration(stages)` finds it wherever a stage carries it, in the shape `correction_pass_declaration` established. It declares no route, and since story-137 it declares none for a stronger reason than it did before: the failure route was a self-route back into the declaring stage, which needed no destination named; now a red suite is carried forward to the next stage and judged by the verifier, so there is no destination for the declaration to name. See "The coordinator runs the suite a stage declares" below. @@ -878,6 +878,30 @@ A verifier that noticed something real but too small to fail a run recorded it i **The companion half is the rule, not the mechanism.** `prompts/prose-layer.md` — described under "Prompts" above — carries the redundancy rule that removes the defect class at the source rather than routing it after the fact. The two landed together because a destination for a defect and a habit that stops producing it are answers to different halves of the same question. +**What this route cannot carry.** A correction pass is prose-only and enters at the documenter, so a finding whose fix is one line in a file the documenter does not own — a prompt slot, a schema description, a coordinator branch — fits neither this route nor a blocking issue. Until story-155 it could be had only by failing the whole run, and story-154's verifier filed five such findings as briefs instead. The route between the two is the repair pass, in the next section; the correction pass is unchanged by it. + +## A finding about the story's own change is fixed by the stage that owns it + +A verifier that wanted a finding about the story's own change fixed had two routes, and most findings fit neither. A correctable finding re-enters at the declared correction stage — the documenter in both shipped workflows — and is bounded to the words alone. A blocking issue fails the verdict, spends a retry and archives the attempt. `.harness/stories/story-154.yaml`'s verifier routed six findings about that story's change and fixed one; among the five it filed were a prompt never told about the confinement the story gave its stage, and a slot one verifier prompt had and its sibling lacked — both the implementer's files, neither prose-only, both plainly part of the story they were filed from. story-155 adds the third route. + +**A passing verdict may carry `repairable_findings`, each naming a path, a location, the finding, the correction and a retry category, and presence is the signal.** `schemas/verification-result.schema.json` declares the array beside `correctable_findings`, requiring all five keys. The coordinator looks each finding's category up in the verifier stage's `retry_routing` table exactly as it looks up a failed verdict's `retry_target` — `repair_destination(findings, routes, stage_names)` walks the findings against the table and returns a `RepairRouting`: the entry stage, each finding with its resolved `stage` added, or the categories the table does not define. The entry stage is the **earliest in workflow order** among the resolved stages, so a verdict naming an implementer category and a tester category costs one pass that runs forward through both. That is the derivation story-067 took *off* the correction pass, and it is right here for the reason it was wrong there: a correction is prose and the cheapest stage that writes prose was always the answer, while a repair is fixed by whichever stage owns the file, and only the table knows which. + +**It has the correction pass's cost and the retry routing's reach.** No `retry_count` increment, no `attempts/attempt-N/` archive, no `retry-history.json` entry; the archived `verification/iteration-N.json` still records `passed`, and `routed_category` and `routed_stage` are cleared rather than set because what the entered stages are told comes off the record and not off the retry placeholders. The guidance in force is cleared at the routing, for the reason the correction block clears it. The declaration is `repair_pass` on the verifier stage beside `correction_pass`, shaped like it but **without a `stage` key** — the stage comes off the routing table — with `result` (`repair-pass.json`), `budget` and a `budget_reason`. Both shipped workflows declare a budget of two, and the reason recorded beside the number is the correction pass's arithmetic taken one step further: a pass fixes the findings a verification raised, the verification after it can find one the first findings did not reach, and a separate story to carry a leftover finding costs more than a second pass in the run that found it. `RunState.repair_pass_count` defaults to `0` so an existing `state.json` loads, and is cumulative for the run like `correction_pass_count` and unlike `self_route_count`: the resume that zeroes the per-entry counts leaves both pass counts alone, so a resumed run cannot spend either a second time. When the budget is spent, a passing verdict still carrying repairable findings names them in a `repair-pass-recorded` event and the run completes with no further invocation — the bound is what makes termination a property rather than a hope, and the findings are where a developer meets them. + +**It routes after the clean-clone check, where the correction pass routes before it, and the difference is the whole justification.** The correction-pass section above records that reversing story-055's order — pass first, check once on the verification the pass returns to — is defensible only while a pass changes prose alone, and names the condition under which the original order would have to be restored: a pass ever allowed to change behaviour. A repair pass is that pass. So the repair block in `run_story`'s verifier branch sits **below** the clean-clone block: a verdict whose clean-clone check fails takes the existing clean-clone retry route and never reaches it, a verdict whose check passes routes the repair on a tree known clean-clone-good, and the verification the pass returns to runs the check again on the repaired tree. A run taking a repair therefore runs the suite in a fresh clone twice, and that is the price of letting the pass change behaviour rather than a cost to be optimised away. The block gates on the `repair_pass` declaration and the findings, not on the presence of a `clean_clone` declaration: a workflow declaring no clean-clone check has no check to wait for. + +**Each entered stage is granted the paths of the findings whose category resolved to it, for that attempt alone.** `repair_pass_grants(run_dir, stages, state, stage_name, attempt)` reads the current pass record off the run directory — `repair_pass_result_file` delegates to `correction_pass_result_file`, so the record is keyed by the pass number the same way — and returns the `path` of each finding whose recorded `stage` is the stage being checked, when the record's `attempt` is the attempt now running. At the post-stage checks the exempt list is the story's `stage_exceptions` grants plus the correction-pass grants plus these, and both the ownership check and the revert check read that one list, so an edit to a granted path is exempt from both and an edit to any other governed path is not. One `repair-pass-grant-applied` event is appended per granted path beside the correction-pass grant events. The bounds are the correction grant's: one file per finding, never a directory; one stage per finding, so an implementer finding grants the tester nothing even though the tester runs after it; one attempt, so a stage re-entered by a later retry inherits nothing. The stage is told the finding and the correction and nothing else — `repair_pass_statement` says the coordinator wrote it, that no retry was spent, that the verdict stands, that a stage none of the findings resolved to is running only because the workflow runs forward from the entry stage and has nothing to repair, and that a repair pass is not a licence to revisit the work. + +**A repair pass makes no new attempt, and the changed-files completeness check is decided per attempt.** `capture_tree_signature` is first-seen-wins within an attempt — see "A stage records every file it touched" below — and a repair pass increments no attempt counter, so a stage re-entered by a pass is compared against the tree as it stood before that stage's *first* invocation of the attempt. Its changed-files record must therefore name every file the attempt changed, the earlier edits beside the repaired one, or the omission is reported and costs the bookkeeping self-route the stage declares; the pass statement tells the entered stage to make the correction and nothing else and does not say this. This story's own run found it: the tester's fake writing stage recorded only the pass's edit on re-entry and the four grant cases failed on the completeness check rather than on the grant, and the verifier's verdict for attempt 1 reported it as an unverified cost a real stage may pay in production. The fixture was corrected; the coordinator was not, because the first-seen rule is what keeps a re-entered stage's omission reportable. + +**An undefined category escalates rather than being absorbed, above the budget comparison.** A finding naming a category the table does not define is a bug in what the verifier produced, and gets the strictness an unrecognised `retry_target` gets: `_escalate`, naming the offending categories and the ones the workflow defines, with `retry_count` untouched and nothing archived. Being a declaration, `repair_pass` is held to a pre-flight of its own — `repair_pass_problems`, beside `correction_pass_problems`, refusing through `_refuse_bad_repair_pass` — and a budget that is not a positive integer, or a declaration on a stage that declares no `retry_routing` table, is refused before any run state is created. The second case is the one the missing `stage` key creates: with nowhere but the table to send a finding, a stage without a table has nowhere to send it. + +**The record is an artifact, injected the way the correction record is.** Each pass writes `repair-pass-N.json` at the run-directory root carrying the pass number, the attempt, the entry stage, the findings each with its resolved stage, and the statement. The coordinator reads it back off the file into a `repair_pass_result` context key in `orchestration/context_assembler.py`, and every stage prompt a pass can enter — `story-implementer.md`, `story-tester.md`, `documenter.md`, `refactor-implementer.md` and both verifier prompts — carries a `{{repair_pass_result}}` slot described the way the correction-pass slot is; a run that took no pass renders `None`. The writing-stage slot names no check by name, because the turn-ending control forbids naming the checks outside the passage it cuts. Both verifier prompts gain a section beside the correctable-findings one bounding what may go in the field: a finding judged correct, too small to fail the run, fixable by one stage in the file it names, whose fix is *not* to the words alone, carrying the category spelled as the `retry_routing` table declares it. + +**Story-152's accounting counts a repairable finding as acting on the routed finding it names.** `verdict_locations` in `orchestration/story_inspection.py` reads `repairable_findings` beside `blocking_issues` and `correctable_findings`, so a routed inspection finding whose slug appears in a repairable finding's location is acted on and not filed, and a routed finding no entry names is still filed as a brief — the verifier's right to decline is untouched. The `repair-pass-routed` event carries the `routed_among` clause the correction-pass event carries, so `events.log` can tell a repair spent on the Inspector's reading from one spent on the verifier's own. + +**No stage name, category or artifact name is written into the coordinator for it**, and deleting the key disables it: a workflow declaring no `repair_pass` routes a passing verdict carrying `repairable_findings` nowhere, and its `events.log` is what it was before the key existed. `schemas/execution-history.schema.json`'s closed enum gained `repair-pass-routed`, `repair-pass-recorded` and `repair-pass-grant-applied` — the recurring bill a new event kind pays, reported as a deviation since that schema was outside the story's declared scope, as it was for stories 055, 100, 101, 106, 131 and 154. Two tests that pin the `RunState` field set exactly gained the new field with its reason. **Not decided here:** the findings story-154 filed as briefs stay filed; this story builds the route and does not walk it. + ## Work a stage cannot own story-031 escalated seventeen minutes into its implementer on a conflict that was fully decidable before the run started. Its plan named a file under a restricted prefix in `likely_file_changes` with the stage the workflow restricts from creating there, and declared no grant lifting it. The implementer changed exactly the one file the plan named; the harness refused the result correctly. Both halves of the conflict are literals — one in the story artifact, one in the workflow definition — and nothing about it depends on what any agent would later write. @@ -2078,7 +2102,7 @@ The two sections above build an inspection that fires on every completed story a The section above hands the Inspector's findings about the story's own change to the verifier and files nothing about them, and the verifier's prompt permits it to decline a finding that is correct but too small to fail a run. Until story-152 nothing read the findings artifact after the verdict, so a declined finding was neither fixed nor filed — it had been *deleted*, by a mechanism built to make sure findings reach someone — and the summary line had already reported it as answered. story-151's run lost two findings this way, one of them a defect worth a brief on its own. story-152 accounts for every routed finding after the verdict: what the verdict acted on is counted as acted on, and everything else is sent where a declined finding went before story-147, to the backlog as a brief. -**Acted on means the verdict named the slug, and nothing else counts.** `acted_on_by(findings, verdict)` in `orchestration/story_inspection.py` splits the routed findings on whether the `location` of any of the verdict's `blocking_issues` or `correctable_findings` names the finding's `slug` as a whole token — `names_a_slug` tokenises the location so that a slug which is a prefix of another slug is not the other. File-level matching is deliberately not used: a correctable finding about a docstring in a file would otherwise count as acting on a defect the Inspector found in the same file, which is the silent loss this section exists to close, one layer down. The consequence is stated in both verifier prompts where the findings are rendered: an entry raised from a routed finding names that finding's slug in its location, and a finding acted on without naming it is filed anyway — a duplicate a person closes rather than a loss. A finding carrying no slug at all is remainder for the same reason. The pure pieces — `acted_on_by`, `names_a_slug`, `verdict_locations`, `entries_among_the_routed` — are held by the tests directly, the way the partition tests hold `about_the_change`, and `about_the_change` itself is untouched: the file-level attribution still decides *whose* a finding is, and the slug rule decides only what became of it. +**Acted on means the verdict named the slug, and nothing else counts.** `acted_on_by(findings, verdict)` in `orchestration/story_inspection.py` splits the routed findings on whether the `location` of any of the verdict's `blocking_issues`, `correctable_findings` or, since story-155, `repairable_findings` names the finding's `slug` as a whole token — `names_a_slug` tokenises the location so that a slug which is a prefix of another slug is not the other. File-level matching is deliberately not used: a correctable finding about a docstring in a file would otherwise count as acting on a defect the Inspector found in the same file, which is the silent loss this section exists to close, one layer down. The consequence is stated in both verifier prompts where the findings are rendered: an entry raised from a routed finding names that finding's slug in its location, and a finding acted on without naming it is filed anyway — a duplicate a person closes rather than a loss. A finding carrying no slug at all is remainder for the same reason. The pure pieces — `acted_on_by`, `names_a_slug`, `verdict_locations`, `entries_among_the_routed` — are held by the tests directly, the way the partition tests hold `about_the_change`, and `about_the_change` itself is untouched: the file-level attribution still decides *whose* a finding is, and the slug rule decides only what became of it. **The remainder is filed through the one filing call, from the artifact already in the run directory.** `account_after_verdict(run_dir, target_root, config, harness_root, story_id, *, artifact, attempt, verdict) -> None` reads the attempt's findings artifact, splits it, resolves the bound through `inspection.bounds(config)`, and hands the remainder to `inspection.file_findings` with the resolved floor and cap — so the severity floor, the brief cap, the named drop reasons, the dedupe and the queue are exactly the ones every other producer uses, and story-114's test that reads over every filing call in the module holds over this one too. No Inspector is invoked: the findings come from what the pre-stage inspection wrote, and the agent runner is not on this path. It is total in the shape both existing entry points are — returns nothing, raises on no path, whole body under a guard — and a failure inside it (an unreadable artifact, a queue that cannot be reached) is printed and recorded and changes neither the verdict's routing, the run's status nor its exit code. The coordinator calls it in the verifier branch after the verdict is archived and before the passed/failed routing, only where the stage's own `inspection` declaration is present, and reads the artifact name off that declaration; the same scan that holds the pre-stage call site to "called once, read from by nothing that routes" holds this one. @@ -2086,7 +2110,7 @@ The section above hands the Inspector's findings about the story's own change to **An attempt that routed nothing is not accounted, which is a deviation from the plan and deliberate.** The plan asked for a record saying nothing was routed. A findings artifact that is absent, records `ran` false, or carries no findings now writes no accounting artifact and no event, because doing otherwise would put a line into the `events.log` of every run under a workflow whose file cap is unset — and story-100's property, that the key unset leaves `events.log` byte-for-byte what it was, forbids exactly that. Where findings were routed the artifact and the event are always written, so the once-per-attempt rule holds where there is anything to hold. -**Three lines, each true when written.** The inspection-time summary says `routed to this story: N`. After the verdict, an event of kind `inspection-accounted` says of the routed findings how many the verdict acted on, how many were filed as briefs and how many dropped, with each drop reason named with its count as the inspection line names its own. And the two re-entry events — the correction-pass-routed line and the verification-failed retry line — carry a clause, composed by `routed_among` in the coordinator through the same `names_a_slug` rule, saying how many of the findings they carry are among the ones routed to this story, so a reader of `events.log` can tell a re-entry spent on the Inspector's reading from one spent on the verifier's own. That clause is empty when nothing was routed, so a workflow declaring no inspection writes both events exactly as before. The accounting reaches `inspection-log.jsonl` as a second kind of record beside the inspection's own: `schemas/cross-run-history.schema.json` names `inspection-accounted` in that log's enum and declares `routed` and `acted_on` beside the counts, with `acted_on` present even at zero because none acted on is an answer; it carries no `mode`, no `cost_usd`, no `invocations` and no `dedupe_ran`, because it made no invocation and asked no query, and the log's description says those fields are how the two kinds are told apart since the kind itself is not projected. `execution-history.schema.json` had to declare the kind and the two fields as well — its enum is closed and its optional properties are held equal to `append_event`'s keywords in both directions — which is the recurring bill this document records for stories 055, 100, 101, 106 and 131, paid again. +**Three lines, each true when written.** The inspection-time summary says `routed to this story: N`. After the verdict, an event of kind `inspection-accounted` says of the routed findings how many the verdict acted on, how many were filed as briefs and how many dropped, with each drop reason named with its count as the inspection line names its own. And the re-entry events — the correction-pass-routed line, the repair-pass-routed line since story-155, and the verification-failed retry line — carry a clause, composed by `routed_among` in the coordinator through the same `names_a_slug` rule, saying how many of the findings they carry are among the ones routed to this story, so a reader of `events.log` can tell a re-entry spent on the Inspector's reading from one spent on the verifier's own. That clause is empty when nothing was routed, so a workflow declaring no inspection writes both events exactly as before. The accounting reaches `inspection-log.jsonl` as a second kind of record beside the inspection's own: `schemas/cross-run-history.schema.json` names `inspection-accounted` in that log's enum and declares `routed` and `acted_on` beside the counts, with `acted_on` present even at zero because none acted on is an answer; it carries no `mode`, no `cost_usd`, no `invocations` and no `dedupe_ran`, because it made no invocation and asked no query, and the log's description says those fields are how the two kinds are told apart since the kind itself is not projected. `execution-history.schema.json` had to declare the kind and the two fields as well — its enum is closed and its optional properties are held equal to `append_event`'s keywords in both directions — which is the recurring bill this document records for stories 055, 100, 101, 106 and 131, paid again. **A filed finding is not shown to the verifier twice.** The dedupe every producer files through runs inside `inspection.inspect_scope`, before `about_the_change` partitions what was found. So a finding the verifier declined on attempt 1 and the accounting filed is, if the retry's inspection finds it again, dropped as already queued before it could be routed — attempt 2 routes nothing about it, is not accounted for it, and the queue holds it once. That is the right outcome for a backlog item, and it means a routed finding gets exactly one reading by the verifier before it becomes the tracker's. @@ -2589,6 +2613,15 @@ story-139 moves a sorting a person did after filing into the sync branch of `tem whose passing verdict carried no correctable finding, and from every run under a workflow that declares no correction pass + repair-pass-N.json since story-155, one per repair pass, + coordinator-written and keyed by the pass + number; carries the entry stage and each + finding with the stage its category resolved + to, which is what the grant is read from; + absent from a run whose passing verdict + carried no repairable finding, and from every + run under a workflow that declares no + repair pass story-inspection-findings-N.json since story-147, the pre-stage inspection's findings about the files this story itself @@ -2681,7 +2714,7 @@ Three artifacts describe a retry and none of them substitutes for another. `retr `archive_attempt` also **follows a copied record's own pointer**. After copying an artifact it reads the copy as JSON, and when the copy is an object carrying a non-empty string `output_path` resolving to an existing file inside the run directory, it copies that file into `attempts/attempt-N/` under its basename and rewrites `output_path` **in the archived copy alone** to the archived file — so reading an archived record leads to the output of the attempt that record belongs to rather than to whatever the next attempt wrote over it, which is the whole-output file story-066 added for the self-routed stage to diagnose from. The pointer is followed off the record's own field, never off a filename or a naming convention, so a record of a shape declared later is followed for free. The live copies are untouched on both counts: the root record keeps its contents and its canonical name, the output file is copied rather than moved, and a record that is not JSON, is not an object, carries no `output_path`, or whose `output_path` names nothing that exists inside the run directory is copied verbatim as before — a pointer resolving *outside* the run directory names a file this archive has no claim on and is left pointing where it points. The companion's name joins the record's in what the archive returns. An artifact the superseded attempt did not produce is still skipped rather than failing the archive, which is the usual case for `clean-clone-result.json`, written only after a passing verdict. -**A correction pass's record needs no archiving, and story-075 established that rather than changing it.** `correction_pass_result_file` keys each record by the pass number, so the file on disk is `correction-pass-1.json` rather than the declared `correction-pass.json`; and `correction_pass_count` is only ever incremented — a retry does not reset it, and the resume that zeroes `retry_count`, `verification_iterations`, `self_route_count` and `entry_cost_usd` deliberately leaves it alone, because the budget is one pass per run and a resumed run must not spend it twice. Together those mean no correction-pass record is ever overwritten, so none needs preserving; the declared name the new collection picks up carries no pass number and therefore names no file on disk, and is skipped like any other absent artifact. Nothing about the correction pass changed. +**A correction pass's record needs no archiving, and story-075 established that rather than changing it.** `correction_pass_result_file` keys each record by the pass number, so the file on disk is `correction-pass-1.json` rather than the declared `correction-pass.json`; and `correction_pass_count` is only ever incremented — a retry does not reset it, and the resume that zeroes `retry_count`, `verification_iterations`, `self_route_count` and `entry_cost_usd` deliberately leaves it alone, because the budget is one pass per run and a resumed run must not spend it twice. Together those mean no correction-pass record is ever overwritten, so none needs preserving; the declared name the new collection picks up carries no pass number and therefore names no file on disk, and is skipped like any other absent artifact. Nothing about the correction pass changed. The repair pass's record since story-155 is keyed and counted the same way — `repair_pass_result_file` delegates to `correction_pass_result_file`, and `repair_pass_count` is likewise never reset — so it needs no archiving for the same reason. `stage-baseline/` is the odd one in this directory, and it is worth saying exactly what it is and is not. It holds **file copies, not JSON**: one directory per **stage**, `/`, mirroring repository-relative paths, holding the content of every file the prefixes that stage's declarations ask a baseline for cover — since story-106 whatever its restrictions govern for the revert check, which is at or beneath each `may_not_create` prefix and everywhere *outside* a `may_only_change` one, and since story-070 its `suite_census` `paths` — as it stood when that stage *first* ran. It carries no attempt component, and since story-037 no attempt number is derived for it anywhere — the directory answers "what did this stage find", a question the run has one answer to however many attempts it takes. Both the directory name and the stage it is captured for come off the workflow declaration; `stage_baseline_dir(run_dir, baseline, stage_name)` and `capture_stage_baseline(run_dir, target_root, baseline, stage_name, prefixes, *, accounted_for)` name no stage and no prefix. diff --git a/.harness/history/execution-log.jsonl b/.harness/history/execution-log.jsonl index bb9b1a8..d62bc1c 100644 --- a/.harness/history/execution-log.jsonl +++ b/.harness/history/execution-log.jsonl @@ -91,3 +91,4 @@ {"story_id": "story-153", "status": "resumed", "retry_count": 1, "timestamp": "2026-09-16 12:32:22"} {"story_id": "story-153", "status": "completed", "retry_count": 1, "timestamp": "2026-09-16 12:55:13"} {"story_id": "story-154", "status": "completed", "retry_count": 1, "timestamp": "2026-09-16 17:32:45"} +{"story_id": "story-155", "status": "completed", "retry_count": 2, "timestamp": "2026-09-16 20:18:27"} diff --git a/.harness/history/inspection-log.jsonl b/.harness/history/inspection-log.jsonl index 01ca51c..292477e 100644 --- a/.harness/history/inspection-log.jsonl +++ b/.harness/history/inspection-log.jsonl @@ -55,3 +55,9 @@ {"story_id": "story-154", "timestamp": "2026-09-16 17:01:19", "filed": 4, "dropped": 0, "routed": 4, "acted_on": 0} {"story_id": "story-154", "mode": "narrow", "cost_usd": 3.3866405000000004, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-16 17:18:31", "findings": 6, "filed": 0, "dropped": 0, "dedupe_ran": true} {"story_id": "story-154", "timestamp": "2026-09-16 17:24:21", "filed": 5, "dropped": 0, "routed": 6, "acted_on": 1} +{"story_id": "story-155", "mode": "narrow", "cost_usd": 2.9324584999999996, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-16 19:35:09", "findings": 3, "filed": 0, "dropped": 0, "dedupe_ran": true} +{"story_id": "story-155", "timestamp": "2026-09-16 19:40:22", "filed": 3, "dropped": 0, "routed": 3, "acted_on": 0} +{"story_id": "story-155", "mode": "narrow", "cost_usd": 3.4972214999999998, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-16 19:54:22", "findings": 5, "filed": 0, "dropped": 0, "dedupe_ran": true} +{"story_id": "story-155", "timestamp": "2026-09-16 19:56:29", "filed": 5, "dropped": 0, "routed": 5, "acted_on": 0} +{"story_id": "story-155", "mode": "narrow", "cost_usd": 3.163644, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-16 20:08:58", "findings": 5, "filed": 0, "dropped": 0, "dedupe_ran": true} +{"story_id": "story-155", "timestamp": "2026-09-16 20:12:38", "filed": 5, "dropped": 0, "routed": 5, "acted_on": 0} diff --git a/.harness/history/retry-log.jsonl b/.harness/history/retry-log.jsonl index 536b694..ea630ab 100644 --- a/.harness/history/retry-log.jsonl +++ b/.harness/history/retry-log.jsonl @@ -66,3 +66,5 @@ {"story_id": "story-153", "timestamp": "2026-09-16 12:27:41", "stage": "verifier", "retry_decision": "retry", "retry_category": "validation", "retry_stage": "tester"} {"story_id": "story-153", "timestamp": "2026-09-16 12:27:49", "stage": "tester", "retry_stage": "tester"} {"story_id": "story-154", "timestamp": "2026-09-16 17:01:19", "stage": "verifier", "retry_decision": "retry", "retry_category": "validation", "retry_stage": "tester"} +{"story_id": "story-155", "timestamp": "2026-09-16 19:40:22", "stage": "verifier", "retry_decision": "retry", "retry_category": "validation", "retry_stage": "tester"} +{"story_id": "story-155", "timestamp": "2026-09-16 19:56:29", "stage": "verifier", "retry_decision": "retry", "retry_category": "validation", "retry_stage": "tester"} diff --git a/orchestration/context_assembler.py b/orchestration/context_assembler.py index eed750e..59c8569 100644 --- a/orchestration/context_assembler.py +++ b/orchestration/context_assembler.py @@ -250,6 +250,7 @@ def build_context( allowed_tools: list[str] | None = None, self_route_result: str | None = None, correction_pass_result: str | None = None, + repair_pass_result: str | None = None, suite_run_result: str | None = None, revert_check_result: str | None = None, inspection_findings: str | None = None, @@ -350,6 +351,13 @@ def build_context( # coordinator's to compose. Defaulted to None so a call that omits it # renders exactly what it rendered before. "correction_pass_result": correction_pass_result, + # A stage the workflow re-entered because a passing verdict, and the + # clean-clone check on it, carried repairable findings is told so by + # the coordinator's own record of that pass — the findings each with + # the stage its category resolved to. Passed in for the reason the + # correction-pass record beside it is, and defaulted to None so a call + # that omits it renders exactly what it rendered before. + "repair_pass_result": repair_pass_result, # The coordinator's own record of the suite it ran after a stage's turn # ended: the exit status of a subprocess it owns, rather than an # agent's assertion about a run it made itself. Passed in for the diff --git a/orchestration/story_coordinator.py b/orchestration/story_coordinator.py index f1e52c4..3f4944f 100644 --- a/orchestration/story_coordinator.py +++ b/orchestration/story_coordinator.py @@ -113,6 +113,15 @@ class RunState: #: spends, so this is a separate count and neither reads nor writes #: retry_count. correction_pass_count: int = 0 + #: How many repair passes this run has taken. Cumulative for the run like + #: correction_pass_count, and for the same reason: the budget the verifier + #: stage declares is per run, and the count is what makes a repair the + #: verifier keeps rejecting terminate rather than cycle. Saved and restored + #: with the rest of the state so a resumed run cannot spend the pass again, + #: and defaulted so a state file written before it existed still loads. A + #: repair pass spends nothing a retry spends and neither reads nor writes + #: retry_count. + repair_pass_count: int = 0 #: Which entry of the run is now executing: zero for the first, one after #: the first resume, and so on. A run is re-entered rather than restarted, #: and each re-entry opens an entry whose counter-keyed artifacts are kept @@ -694,6 +703,43 @@ def correction_pass_problems(stages: list[dict]) -> list[str]: return [] +def repair_pass_problems(stages: list[dict]) -> list[str]: + """Check the repair-pass declaration against the workflow that carries it. + + Beside `correction_pass_problems`, and for its reason: a declaration every + run under the definition would trip on is refused before any run state is + created rather than at the one moment everything else went right. Two + things a definition can be wrong about here. The budget must be a positive + integer, because a budget that is not a count cannot be spent and a budget + of zero is a declaration that should have been left out. And the stage + declaring the pass must declare a retry_routing table, because a repair + pass names no entry stage of its own: each finding's category is looked up + in that table exactly as a failed verdict's retry_target is, so a stage + with no table has nowhere to send a repair. A workflow carrying no + declaration is not checked — that is how the mechanism is switched off. + No stage name is written here; it comes off the loaded definition. + """ + problems: list[str] = [] + for stage in stages: + declaration = stage.get("repair_pass") + if not declaration: + continue + budget = declaration.get("budget") + if isinstance(budget, bool) or not isinstance(budget, int) or budget < 1: + problems.append( + f"stage '{stage['name']}' declares a repair_pass budget of " + f"{budget!r}, which is not a positive integer" + ) + routes = (stage.get("on_failure") or {}).get("retry_routing") or {} + if not routes: + problems.append( + f"stage '{stage['name']}' declares a repair_pass but no " + f"retry_routing table, so a repairable finding's category " + f"has nothing to resolve against" + ) + return problems + + def _is_a_ceiling(value) -> bool: """Whether a declared ceiling is a value a ceiling can take. @@ -6163,6 +6209,228 @@ def _correction_pass_recorded( ) +# -------------------------------------------------------------------------- +# A finding about the story's own change is fixed by the stage that owns it +# +# A verifier that wants a finding fixed had two routes, and most findings fit +# neither: a correctable finding enters at the declared correction stage and +# is bounded to the words alone, and a blocking issue fails the verdict and +# spends a retry. A finding whose fix is one line in a file the correction +# stage does not own could be had only by failing the whole run, so story-154 +# filed five such findings as briefs instead. +# +# The repair pass is the third route. A passing verdict may carry +# `repairable_findings`, each naming a path, a location, the finding, the +# correction and a retry category. The category is looked up in the verifier +# stage's retry_routing table exactly as a failed verdict's retry_target is, +# the workflow re-enters at the earliest stage in workflow order among the +# resolved stages, and runs forward to verification again. It has the +# correction pass's cost and the retry routing's reach: nothing a retry spends +# is spent — no retry_count increment, no attempts/attempt-N/ archive, no +# retry-history entry, and the passing verdict that routed it stands — and +# each stage entered is granted the paths of the findings whose category +# resolves to it, for that attempt alone. +# +# Because a repair may change behaviour, it is built on a verified tree: it +# routes only after the clean-clone check on the routing verdict has passed, +# and the verification it returns to runs the check again. The budget is +# declared on the verifier stage beside correction_pass, and the declaration +# carries no stage key because the stage comes off the routing table. Nothing +# below writes a stage name, a category or an artifact name. + + +def repair_pass_declaration(stages: list[dict]) -> dict: + """The workflow's repair-pass declaration, wherever a stage carries it. + + In `correction_pass_declaration`'s shape and for its reason: read off the + loaded definition rather than looked up by stage name, and a workflow that + declares it nowhere disables the mechanism entirely. + """ + for stage in stages: + declaration = stage.get("repair_pass") + if declaration: + return declaration + return {} + + +def repair_pass_result_file(artifact: str, number: int) -> str: + """Where one repair pass's record is written, keyed by the pass number. + + Keyed for the reason `correction_pass_result_file` keys: the budget is + more than one, so a second pass would otherwise write over the first's + record. + """ + return correction_pass_result_file(artifact, number) + + +def repair_pass_grants( + run_dir: Path, stages: list[dict], state: RunState, stage_name: str, attempt: int +) -> list[str]: + """The files this pass's findings grant one stage to change for one attempt. + + Read off the current pass record exactly as `correction_pass_grants` reads + the correction record, so what the checks exempt and what the prompt + reports are one thing. The difference is that a repair pass enters more + than one stage: the record carries each finding with the stage its + category resolved to, and a stage is granted only the paths of the + findings that resolved to it. Bounded to the attempt the record names, so + a stage re-entered by a later retry in the same run inherits nothing. A + run that has taken no pass, a workflow that declares none, and a record + that is absent or unreadable each grant nothing. + """ + if not state.repair_pass_count: + return [] + declared = repair_pass_declaration(stages).get("result") + if not declared: + return [] + record = run_dir / repair_pass_result_file(declared, state.repair_pass_count) + if not record.is_file(): + return [] + try: + parsed = json.loads(record.read_text(encoding="utf-8")) + except (OSError, ValueError): + return [] + if parsed.get("attempt") != attempt: + return [] + return [ + finding["path"] + for finding in parsed.get("findings", []) + if isinstance(finding, dict) + and finding.get("stage") == stage_name + and finding.get("path") + ] + + +def repair_pass_statement(entry_stage: str) -> str: + """What the coordinator tells the stages a repair pass re-enters. + + A passing verdict writes no retry guidance, so there is no agent-authored + guidance behind this and the statement says so, as the correction-pass + statement does. What the stage most needs to know is that it is told the + findings and the corrections and nothing else, that only the files those + findings name are its to change on this pass, and that this is not a + licence to revisit the work. + """ + return ( + f"The verification passed, and the clean-clone check on that verdict " + f"passed too. It also recorded findings it judges correct, too small " + f"to fail the run, and fixable by one stage in the file each names, " + f"and the coordinator has re-entered the workflow at {entry_stage} so " + f"they are fixed by the stage that owns them rather than filed for a " + f"later story. This is the coordinator's own statement, not an " + f"agent's: a passing verdict writes no retry guidance, so nothing here " + f"was authored by a verifier beyond the findings themselves. This is " + f"not a retry and no retry budget was spent — the retry count is " + f"unchanged, no attempt was archived, and the passing verdict stands. " + f"Each finding below names the stage its category resolved to; if " + f"none names yours, you are running because the workflow runs forward " + f"from the entry stage, and you have nothing to repair. Make the " + f"correction each of your findings names, in the file it names, and " + f"nothing else: those files are yours to change for this pass, and an " + f"edit to any other governed file is put to the revert check after " + f"your turn. A repair pass is not a licence to revisit the work. The " + f"verification it returns to runs the clean-clone check again on the " + f"repaired tree." + ) + + +@dataclass(frozen=True) +class RepairRouting: + """Where a verdict's repairable findings send execution, or why they cannot. + + `stage` is the entry stage: the earliest in workflow order among the + stages the findings' categories resolved to, so a verdict naming two + categories costs one pass that runs forward through both. `resolved` is + each finding with its resolved stage added, in the order the verdict + carried them, so the pass record can say which stage owns which fix. + `unknown` holds the categories the table does not define, and it is + non-empty exactly when the run must escalate instead of routing — the + strictness an unrecognised retry_target already gets. + """ + + stage: str | None + resolved: list[dict] + unknown: list[str] + + +def repair_destination( + findings: list[dict], routes: dict, stage_names: list[str] +) -> RepairRouting: + """Resolve each finding's category through the routing table. + + The table is the same one a failed verdict's retry_target is looked up + in, so the categories a repair pass may name and the stages it may enter + are exactly the ones the workflow already declares for a retry. No + category name and no stage name is written here. + """ + unknown: list[str] = [] + resolved: list[dict] = [] + for finding in findings: + category = finding.get("category") + route = routes.get(category) if isinstance(category, str) else None + if route is None: + if category not in unknown: + unknown.append(category) + continue + resolved.append({**finding, "stage": route.get("stage")}) + if unknown: + return RepairRouting(None, [], unknown) + stages = {one["stage"] for one in resolved} + entry = next((name for name in stage_names if name in stages), None) + return RepairRouting(entry, resolved, []) + + +def _repair_pass_routed( + run_dir: Path, + stage_name: str, + destination: str, + findings: list[dict], + number: int, + budget: int, + routed_slugs=(), +) -> None: + """Record a repair pass, and where its findings came from. + + Worded with `routed_among` as the correction-pass event is, so a reader of + events.log can tell a re-entry spent on the Inspector's reading from one + spent on the verifier's own. + """ + append_event( + run_dir, + f"repair pass {number} of {budget}: verification passed and the " + f"clean-clone check passed, carrying {len(findings)} repairable " + f"finding(s); re-entering at {destination}" + + routed_among(findings, routed_slugs), + kind="repair-pass-routed", + stage=stage_name, + retry_stage=destination, + ) + + +def _repair_pass_recorded( + run_dir: Path, stage_name: str, findings: list[dict], budget: int +) -> None: + """Record findings a spent budget leaves unrepaired, and complete the run. + + The bound is what makes termination a property rather than a hope: a fix + the verifier keeps rejecting cannot cycle. A later verdict still carrying + findings is not an error and routes nowhere, so the findings are named in + the event stream, where a developer meets them, and the run completes. + """ + named = "; ".join( + f"{finding.get('location')}: {finding.get('finding')}" + for finding in findings + ) + append_event( + run_dir, + f"repair pass budget of {budget} is spent; verification passed " + f"still carrying repairable findings, which are recorded here and " + f"left unrepaired: {named}", + kind="repair-pass-recorded", + stage=stage_name, + ) + + # -------------------------------------------------------------------------- # Ending a run so it can be resumed # @@ -7020,6 +7288,22 @@ def _refuse_bad_correction_pass(workflow: dict, problems: list[str]) -> int: ) +def _refuse_bad_repair_pass(workflow: dict, problems: list[str]) -> int: + """Refuse a workflow whose repair pass cannot be spent or cannot route. + + Thin, like every other caller of `refuse`. The definition is wrong, not the + story and not the tree, so the guidance points at the file that has to + change. + """ + return refuse( + f"Workflow '{workflow['name']}' declares a repair pass that cannot " + f"be taken:", + problems, + "Fix the workflow definition's repair_pass declaration before running " + "a story under it.", + ) + + def _refuse_bad_cost_ceilings(workflow: dict, problems: list[str]) -> int: """Refuse a workflow whose declared cost ceiling is not a number. @@ -7894,6 +8178,14 @@ def run_story( if entry_problems: return _refuse_bad_correction_pass(workflow, entry_problems) + # And the same pre-flight for the repair pass. Its budget has to be a + # count that can be spent, and the stage declaring it has to declare the + # retry_routing table a repairable finding's category is resolved through, + # or a passing verdict carrying one would have nowhere to send it. + repair_problems = repair_pass_problems(stages) + if repair_problems: + return _refuse_bad_repair_pass(workflow, repair_problems) + # And the same pre-flight for the two cost ceilings. A ceiling that is not # a number can be neither compared against a spend nor handed to an # invocation, so a definition that cannot work is refused before a stage is @@ -8588,6 +8880,20 @@ def elapsed() -> float | None: if record.is_file(): correction_pass_result = record.read_text(encoding="utf-8") + # The same for a repair pass: the current pass record, read back off + # the artifact the coordinator wrote, into every stage prompt, so the + # stage a pass entered is told the findings and the corrections and + # nothing else. None for every stage of a run that took none. + repair_pass_result = None + if state.repair_pass_count: + declared = repair_pass_declaration(stages).get("result") + if declared: + record = run_dir / repair_pass_result_file( + declared, state.repair_pass_count + ) + if record.is_file(): + repair_pass_result = record.read_text(encoding="utf-8") + # The coordinator's record of the suite it ran after some stage's turn # ended. Read back off the artifact rather than passed along in memory, # for the reason the two records above are, and named off the @@ -8642,6 +8948,7 @@ def elapsed() -> float | None: allowed_tools=stage_tools, self_route_result=self_route_result, correction_pass_result=correction_pass_result, + repair_pass_result=repair_pass_result, suite_run_result=suite_run_result, revert_check_result="\n".join(revert_records) or None, inspection_findings=inspection_findings, @@ -9096,7 +9403,22 @@ def elapsed() -> float | None: kind="correction-pass-grant-applied", stage=name, ) - exempt = exempt + pass_grants + # A repair pass grants each stage it enters the files of the + # findings whose category resolved to that stage, for that attempt + # alone, on the same exempt list and the same terms: the record + # copies the findings with their resolved stages, so what was + # granted is recorded by the act that granted it. + repair_grants = repair_pass_grants( + run_dir, stages, state, name, attempt + ) + for granted in repair_grants: + append_event( + run_dir, + f"repair-pass grant applied: {name} may change {granted}", + kind="repair-pass-grant-applied", + stage=name, + ) + exempt = exempt + pass_grants + repair_grants ownership = _ownership_violation(run_dir, record_name, enforced, exempt) if ownership: return _escalate( @@ -9604,6 +9926,92 @@ def elapsed() -> float | None: index = stage_names.index(destination) continue _clean_clone_passed(run_dir, name, artifact) + + # A finding about the story's own change whose fix is not to + # the words alone has somewhere to go, and it goes there only + # once the tree is known clean-clone-good. This block sits + # below the clean-clone check deliberately, where the + # correction block sits above it: a repair may change + # behaviour, so it is built on a verified tree and the + # verification it returns to runs the check again on the + # repaired one. A verdict whose clean-clone check failed has + # already taken the retry route above and never reaches here. + # Both names come off the declaration, so removing the key + # disables the whole mechanism with no change here. + repair = stage.get("repair_pass") or {} + repair_artifact = repair.get("result") + repairable = verdict.get("repairable_findings") or [] + if repair_artifact and repairable: + routing = repair_destination(repairable, routes, stage_names) + if routing.unknown: + # Above the budget comparison, for the reason the + # correction block's escalation is: a finding naming a + # category the workflow does not define is a bug in + # what the verifier produced. Through _escalate, so + # retry_count is untouched, and above archive_attempt, + # so no attempts/attempt-N/ is written. + named = ", ".join(f"'{one}'" for one in routing.unknown) + return _escalate( + run_dir, + state, + f"a repairable finding named {named}, which is " + f"not a retry category {workflow['name']} defines; " + f"it defines: {declared}", + target_root=target_root, + harness_root=harness_root, + duration_seconds=elapsed(), + verifier_outcome=verdict.get("status"), + retry_decision="escalate", + retry_reason=( + f"a repairable finding named the unknown " + f"category {named}" + ), + ) + repair_budget = repair["budget"] + if state.repair_pass_count >= repair_budget: + _repair_pass_recorded( + run_dir, name, repairable, repair_budget + ) + else: + state.repair_pass_count += 1 + record = { + "pass": state.repair_pass_count, + "attempt": attempt, + "stage": routing.stage, + "findings": routing.resolved, + "statement": repair_pass_statement(routing.stage), + } + ( + run_dir + / repair_pass_result_file( + repair_artifact, state.repair_pass_count + ) + ).write_text( + json.dumps(record, indent=2) + "\n", encoding="utf-8" + ) + # Cleared for the reason the correction block clears + # it: the passing verdict that routed this wrote no + # guidance, so none is in force for the verification + # the pass runs back into. + state.guidance_in_force = [] + save_state(run_dir, state) + _repair_pass_routed( + run_dir, + name, + routing.stage, + repairable, + state.repair_pass_count, + repair_budget, + routed_slugs, + ) + # Nothing a retry spends is spent: retry_count is + # untouched, archive_attempt is not reached, and no + # retry-history entry is appended. The pass names no + # retry category and no retry destination; what each + # stage is told comes off the record just written. + routed_category, routed_stage = None, None + index = stage_names.index(routing.stage) + continue elif verdict.get("retry_recommended") and not target: # Above the ceiling comparison deliberately: a verdict that # cannot be routed is a bug in what the verifier produced, and diff --git a/orchestration/story_inspection.py b/orchestration/story_inspection.py index fd8d61e..4017baa 100644 --- a/orchestration/story_inspection.py +++ b/orchestration/story_inspection.py @@ -1292,13 +1292,18 @@ def _slug_of(finding) -> str: def verdict_locations(verdict) -> tuple: - """Every location the verdict names, on a blocking issue or a correctable - finding, as the verdict wrote it. + """Every location the verdict names, on a blocking issue, a correctable + finding or a repairable finding, as the verdict wrote it. + + A repairable finding counts for the reason a correctable one does: it is + an entry the verdict raised to have something acted on, so a routed + finding whose slug it names was acted on rather than declined, and is + not filed as a brief. """ if not isinstance(verdict, dict): return () named: list = [] - for key in ("blocking_issues", "correctable_findings"): + for key in ("blocking_issues", "correctable_findings", "repairable_findings"): for entry in verdict.get(key) or (): if isinstance(entry, dict): named.append(entry.get("location")) diff --git a/prompts/documenter.md b/prompts/documenter.md index e053e54..30c04d0 100644 --- a/prompts/documenter.md +++ b/prompts/documenter.md @@ -91,6 +91,17 @@ by a pass; if a finding names it, correct nothing and say so in your report. No retry was spent and the verdict still stands: {{correction_pass_result}} +Repair pass — present only when you are running because a passing +verification, on a tree whose suite also passed where the code ships, recorded +findings it judges correct, too small to fail the run, and fixable by one stage +in the file each names. The coordinator wrote it, not an agent. Each finding +names the stage its category resolved to; make the correction each of your +findings names, in the file it names, and nothing else — those files are yours +to change for this pass, and an edit to any other governed file is undone +after your turn if the suite is green without it. It is not a licence to +revisit the work. No retry was spent and the verdict still stands: +{{repair_pass_result}} + Retry guidance: {{retry_guidance}} diff --git a/prompts/refactor-implementer.md b/prompts/refactor-implementer.md index cd02e61..5180294 100644 --- a/prompts/refactor-implementer.md +++ b/prompts/refactor-implementer.md @@ -140,5 +140,16 @@ behaviour, so nothing you change for it may alter what any test asserts, and the suite must pass unchanged. No retry was spent and the verdict still stands: {{correction_pass_result}} +Repair pass — present only when you are running because a passing +verification, on a tree whose suite also passed where the code ships, recorded +findings it judges correct, too small to fail the run, and fixable by one stage +in the file each names. The coordinator wrote it, not an agent. Each finding +names the stage its category resolved to; make the correction each of your +findings names, in the file it names, and nothing else — those files are yours +to change for this pass, and an edit to any other governed file is undone +after your turn if the suite is green without it. It is not a licence to +revisit the work. No retry was spent and the verdict still stands: +{{repair_pass_result}} + Retry state: {{retry_state}} diff --git a/prompts/refactor-verifier.md b/prompts/refactor-verifier.md index af843c3..af42762 100644 --- a/prompts/refactor-verifier.md +++ b/prompts/refactor-verifier.md @@ -137,6 +137,29 @@ at is instructed to leave it alone, so such a finding has nowhere to be acted on. The passing verdict stands: recording a finding does not fail the run, spends no retry, and leaves the verdict that carried it recorded as passed. +A finding can also be correct, too small to fail the run, and not fixable in +the words alone — one line of code, a slot one prompt has and its sibling +lacks, a declaration a stage was never told about. Record such a finding in +repairable_findings. What belongs there is a finding you judge correct, whose +fix is one stage's to make in the one file the finding names, and whose fix is +not to prose alone — that is what separates it from a correctable finding, +which is bounded to words and enters at the declared correction stage +whatever category it carries. Name the category that owns the fix, spelled +exactly as the categories above are: unlike a correctable finding's category, +this one chooses where the pass goes, because the coordinator resolves it +through the routing table above to the stage that owns the file and re-enters +there, running forward to verification again. A category this workflow does +not define escalates the run. Give each finding a `path`: naming it grants the +owning stage that one file for that pass, and an edit to any other governed +file stays governed. A repair pass is routed only after the clean-clone check +on your verdict has passed, because a repair may change behaviour and is built +on a verified tree, and the verification it returns to runs the check again. +A repair pass is not a licence to revisit the work: it carries the findings +and the corrections and nothing else, spends no retry, archives no attempt, +and leaves the verdict that carried it recorded as passed. A finding whose fix +you cannot state for one stage in one file is a blocking issue if it matters +and nothing if it does not. + retry-guidance.json, written only when status is "failed" and a retry is recommended. It must satisfy this schema: @@ -317,5 +340,13 @@ they were routed to. This is not a retry: no retry budget was spent and the verdict that routed it still stands as passed: {{correction_pass_result}} +Repair pass — present only when a passing verdict of this run, and the +clean-clone check on it, carried repairable findings and the workflow +re-entered to act on them. The coordinator wrote it, not an agent, and it +names the findings each with the stage its category resolved to. This is not +a retry: no retry budget was spent and the verdict that routed it still stands +as passed. The clean-clone check runs again on this verification: +{{repair_pass_result}} + Retry state: {{retry_state}} diff --git a/prompts/story-implementer.md b/prompts/story-implementer.md index a778f57..a0f774d 100644 --- a/prompts/story-implementer.md +++ b/prompts/story-implementer.md @@ -120,5 +120,16 @@ behaviour, so nothing you change for it may alter what any test asserts, and the suite must pass unchanged. No retry was spent and the verdict still stands: {{correction_pass_result}} +Repair pass — present only when you are running because a passing +verification, on a tree whose suite also passed where the code ships, recorded +findings it judges correct, too small to fail the run, and fixable by one stage +in the file each names. The coordinator wrote it, not an agent. Each finding +names the stage its category resolved to; make the correction each of your +findings names, in the file it names, and nothing else — those files are yours +to change for this pass, and an edit to any other governed file is undone +after your turn if the suite is green without it. It is not a licence to +revisit the work. No retry was spent and the verdict still stands: +{{repair_pass_result}} + Retry state: {{retry_state}} diff --git a/prompts/story-tester.md b/prompts/story-tester.md index c1b9e5d..bec0ea5 100644 --- a/prompts/story-tester.md +++ b/prompts/story-tester.md @@ -193,5 +193,16 @@ behaviour, so nothing you change for it may alter what any test asserts, and the suite must pass unchanged. No retry was spent and the verdict still stands: {{correction_pass_result}} +Repair pass — present only when you are running because a passing +verification, on a tree whose suite also passed where the code ships, recorded +findings it judges correct, too small to fail the run, and fixable by one stage +in the file each names. The coordinator wrote it, not an agent. Each finding +names the stage its category resolved to; make the correction each of your +findings names, in the file it names, and nothing else — those files are yours +to change for this pass, and an edit to any other governed file is undone +after your turn if the suite is green without it. It is not a licence to +revisit the work. No retry was spent and the verdict still stands: +{{repair_pass_result}} + Retry state: {{retry_state}} diff --git a/prompts/story-verifier.md b/prompts/story-verifier.md index 8af3cf6..21f0dc1 100644 --- a/prompts/story-verifier.md +++ b/prompts/story-verifier.md @@ -110,6 +110,29 @@ at is instructed to leave it alone, so such a finding has nowhere to be acted on. The passing verdict stands: recording a finding does not fail the run, spends no retry, and leaves the verdict that carried it recorded as passed. +A finding can also be correct, too small to fail the run, and not fixable in +the words alone — one line of code, a slot one prompt has and its sibling +lacks, a declaration a stage was never told about. Record such a finding in +repairable_findings. What belongs there is a finding you judge correct, whose +fix is one stage's to make in the one file the finding names, and whose fix is +not to prose alone — that is what separates it from a correctable finding, +which is bounded to words and enters at the declared correction stage +whatever category it carries. Name the category that owns the fix, spelled +exactly as the categories above are: unlike a correctable finding's category, +this one chooses where the pass goes, because the coordinator resolves it +through the routing table above to the stage that owns the file and re-enters +there, running forward to verification again. A category this workflow does +not define escalates the run. Give each finding a `path`: naming it grants the +owning stage that one file for that pass, and an edit to any other governed +file stays governed. A repair pass is routed only after the clean-clone check +on your verdict has passed, because a repair may change behaviour and is built +on a verified tree, and the verification it returns to runs the check again. +A repair pass is not a licence to revisit the work: it carries the findings +and the corrections and nothing else, spends no retry, archives no attempt, +and leaves the verdict that carried it recorded as passed. A finding whose fix +you cannot state for one stage in one file is a blocking issue if it matters +and nothing if it does not. + retry-guidance.json, written only when status is "failed" and a retry is recommended. It must satisfy this schema: @@ -308,5 +331,13 @@ they were routed to. This is not a retry: no retry budget was spent and the verdict that routed it still stands as passed: {{correction_pass_result}} +Repair pass — present only when a passing verdict of this run, and the +clean-clone check on it, carried repairable findings and the workflow +re-entered to act on them. The coordinator wrote it, not an agent, and it +names the findings each with the stage its category resolved to. This is not +a retry: no retry budget was spent and the verdict that routed it still stands +as passed. The clean-clone check runs again on this verification: +{{repair_pass_result}} + Retry state: {{retry_state}} diff --git a/schemas/execution-history.schema.json b/schemas/execution-history.schema.json index b77b363..1a17747 100644 --- a/schemas/execution-history.schema.json +++ b/schemas/execution-history.schema.json @@ -38,6 +38,9 @@ "correction-pass-routed", "correction-pass-recorded", "correction-pass-grant-applied", + "repair-pass-routed", + "repair-pass-recorded", + "repair-pass-grant-applied", "self-routed", "budget-stopped", "capacity-paused", diff --git a/schemas/verification-result.schema.json b/schemas/verification-result.schema.json index 80f73c9..c6f8bfc 100644 --- a/schemas/verification-result.schema.json +++ b/schemas/verification-result.schema.json @@ -98,6 +98,36 @@ } } }, + "repairable_findings": { + "type": "array", + "description": "Findings you judge correct but too small to fail the run, whose fix is not to the words alone and belongs to one stage in the file each names. This is the third route between a correctable finding and a blocking issue: a correctable finding enters at the declared correction stage and is bounded to prose, and a blocking issue fails the verdict and spends a retry, while a repairable finding re-enters the workflow at the stage that owns it — the coordinator looks each finding's category up in this stage's retry_routing table exactly as it looks up a failed verdict's retry_target, enters at the earliest stage in workflow order among the resolved stages, and runs forward to verification again. Nothing a retry spends is spent: no retry is consumed, no attempt is archived, and the passing verdict that carried it stands. Its presence is the signal that a repair pass should run, for the same limitation retry_target, unfinishable_by_retry and correctable_findings record: this validator subset has no if/then and no dependentRequired, so presence-as-signal is the only shape with no state where the signal is set and the payload is missing. It is read only on a passing verdict, and only after the clean-clone check on that verdict has passed, because a repair may change behaviour and is therefore built on a verified tree; a failed verdict already routes and the correction travels in the retry guidance it writes. A category the workflow's retry_routing table does not define escalates the run naming the categories it does, with the strictness an unrecognised retry_target gets. It is optional because almost every verdict has no such finding, and it is bounded by the repair_pass budget the workflow declares.", + "items": { + "type": "object", + "required": ["path", "location", "finding", "correction", "category"], + "properties": { + "path": { + "type": "string", + "description": "The repository-relative file the fix is in. Naming it is the grant: the stage the finding's category resolves to is granted this one file for that pass alone, on the same exempt list the ownership check and the revert check read, and an edit to any other governed file stays governed. A finding names one file; a fix spanning two files is two findings, or a blocking issue." + }, + "location": { + "type": "string", + "description": "Where the defect is: the file, and the symbol, section or line within it. Where the finding was raised from one the pre-stage inspection routed to this story, name that finding's slug here, so the accounting counts it as acted on rather than filing it as a brief." + }, + "finding": { + "type": "string", + "description": "What is wrong as it stands." + }, + "correction": { + "type": "string", + "description": "What the fix is, specifically enough that the owning stage can make it without re-deciding what the finding meant. The stage entered is told the finding and the correction and nothing else." + }, + "category": { + "type": "string", + "description": "The retry category that owns the fix, spelled exactly as the workflow's retry_routing table declares it. Unlike a correctable finding's category, this one chooses where the pass goes: it is resolved through that table to the stage that owns the file, and one the workflow does not declare escalates the run naming the categories it does." + } + } + } + }, "unfinishable_by_retry": { "type": "string", "description": "The verifier's judgement that no number of retries closes the gap between what remains and what an attempt delivers, together with the split it recommends between this story and a follow-on. Its presence on a failed verdict ends the run in escalation at that verification, before the retry count is compared to the ceiling: the budget is left unspent and the retry count untouched, because the judgement is that spending them would not finish the work. Its content is both the judgement and the escalation's recorded reason, so the developer reads the verifier's own words rather than a reason the coordinator composed. It is one string rather than a boolean with a sibling reason for the same limitation retry_target records above: the validator subset has no if/then and no dependentRequired, so 'reason required when the flag is true' is inexpressible, and presence-as-signal leaves no state where the signal is set and the reason is missing. It is optional because almost every verdict has no such judgement to make, and it is read only on a failed verdict — a passing verification routes nowhere, so nothing there could act on it." diff --git a/tests/test_baseline_honesty.py b/tests/test_baseline_honesty.py index 6aed173..279f926 100644 --- a/tests/test_baseline_honesty.py +++ b/tests/test_baseline_honesty.py @@ -2567,6 +2567,19 @@ def live_artifact_reading_modules() -> list[str]: "comparison is between the definitions this repository deploys, and a " "built workflow could not make it. The prompts, the schemas and the " "census it reads are shipped artifacts on the same terms", + "test_repair_pass.py": + "its runs are driven against a workflow it builds, where the " + "declaration, the categories, the confinement and the artifact name " + "are inputs. What remains has what is shipped as its subject: both " + "shipped definitions declaring repair_pass on their verifier, which is " + "story-155's acceptance criterion and which a built definition could " + "only say back to itself; every prompt a shipped stage names carrying " + "the slot, rendered through the shipped story workflow because what " + "reaches the agent is the claim; the shipped verifier prompts, which " + "are where a verifier reads what may go in the field and so the only " + "place its bound can be asserted; the schemas this repository ships " + "for the verification result and the execution history; and the retry " + "ceiling in rules/execution-rules.json", "test_retry_history.py": "its workflow reads were converted by story-048; what remains is the " "retry ceiling this repository declares in rules/execution-rules.json " diff --git a/tests/test_coordinator_contract.py b/tests/test_coordinator_contract.py index e6d0c51..7480fdf 100644 --- a/tests/test_coordinator_contract.py +++ b/tests/test_coordinator_contract.py @@ -199,6 +199,12 @@ def state_contract_problems(state: dict) -> list[str]: # bound is one pass per run and that bound is what makes the mechanism # terminate. "correction_pass_count": int, + # story-155's repair-pass counter: how many repair passes this run has + # taken, bounded by the budget the verifier stage declares. Defaulted + # like the fields above, and zero means none has been taken. Cumulative + # over the run for the reason correction_pass_count is, and saved with + # the state so a resumed run cannot spend the pass again. + "repair_pass_count": int, # story-062's entry index: which entry of the run is now executing, # zero for the first and one higher after each resume. Defaulted like # the fields above, so a state file written before it existed reads as diff --git a/tests/test_escalation_resume.py b/tests/test_escalation_resume.py index 67d9144..f1607dc 100644 --- a/tests/test_escalation_resume.py +++ b/tests/test_escalation_resume.py @@ -2461,7 +2461,8 @@ def test_the_whole_run_including_the_escalation_and_the_resume_reconstructs( #: noticing still turns the assertion below red. FIELDS_ADDED_SINCE = {"self_route_count", "bookkeeping_self_route_count", "guidance_in_force", - "correction_pass_count", "resume_count", + "correction_pass_count", "repair_pass_count", + "resume_count", "entry_cost_usd", "stopped_on_cost", "workflow", "unshadowed_suite_failure"} diff --git a/tests/test_inspection_findings_reach_the_story.py b/tests/test_inspection_findings_reach_the_story.py index 6ab5d89..b52385e 100644 --- a/tests/test_inspection_findings_reach_the_story.py +++ b/tests/test_inspection_findings_reach_the_story.py @@ -1726,6 +1726,65 @@ def test_a_routed_finding_named_by_slug_is_acted_on_and_by_file_alone_is_not(): assert remainder == [OWN_FINDING] +#: story-155's third route, categorised for the fixture's one category so the +#: entry is one a run could route. Its shape is the correctable entry's. +REPAIRABLE_FINDING = { + "path": CHANGED_FILE, + "location": f"{CHANGED_FILE} - the helper", + "finding": "MARKER-REPAIRABLE the helper misses a case", + "correction": "MARKER-REPAIRABLE-FIX handle the case", + "category": RETRY_CATEGORY, +} + + +def repairing(*findings: dict) -> dict: + return {**PASS, "repairable_findings": [dict(one) for one in findings]} + + +def test_a_routed_finding_named_by_slug_in_a_repairable_finding_is_acted_on_too(): + """story-155: a repairable finding is an entry the verdict raised to have + something acted on, so a routed finding whose slug it names counts as + acted on exactly as one a correctable finding or a blocking issue names — + and by file alone it does not, which is the control that says the rule + is the slug and not the file the repairable finding happens to name. + """ + acted, remainder = story_inspection.acted_on_by( + [OWN_FINDING], repairing(naming_the_slug(OWN_FINDING, REPAIRABLE_FINDING))) + assert acted == [OWN_FINDING] + assert remainder == [] + + acted, remainder = story_inspection.acted_on_by( + [OWN_FINDING], repairing(REPAIRABLE_FINDING)) + assert REPAIRABLE_FINDING["location"].startswith(CHANGED_FILE), \ + "the control names the file the finding is about" + assert acted == [] + assert remainder == [OWN_FINDING] + + # One routed finding named across the three kinds of entry, and the other + # in none of them: the remainder is still filed. + acted, remainder = story_inspection.acted_on_by( + [OWN_FINDING, BOTH_FINDING], + {**passing_with(CORRECTABLE_FINDING), + "repairable_findings": [naming_the_slug(OWN_FINDING, REPAIRABLE_FINDING)]}) + assert acted == [OWN_FINDING] + assert remainder == [BOTH_FINDING] + + +def test_verdict_locations_reads_all_three_kinds_of_entry_and_nothing_else(): + """The reader the slug rule is built on, directly: every location a + blocking issue, a correctable finding or a repairable finding carries, in + that order, and none from a key it does not read.""" + verdict = { + "blocking_issues": [{"location": "one"}], + "correctable_findings": [{"location": "two"}], + "repairable_findings": [{"location": "three"}, "not an entry"], + "unverified": [{"location": "four"}], + } + assert story_inspection.verdict_locations(verdict) == ("one", "two", "three") + assert story_inspection.verdict_locations({"repairable_findings": []}) == () + assert story_inspection.verdict_locations("not a verdict") == () + + def test_the_slug_rule_keeps_every_finding_and_names_one_slug_at_a_time(): """Nothing is dropped between the two halves, and naming one routed finding's slug acts on that finding and no other — so two findings in one @@ -1889,6 +1948,42 @@ def test_a_routed_finding_the_verdict_names_by_file_alone_is_filed( assert any(OWN_FINDING["body"] in entry for entry in seen) +def test_a_repairable_finding_naming_the_slug_is_acting_on_the_finding( + driven, filings): + """story-155's kind of entry, driven as a run. The fixture's workflow + declares no repair pass, so the verdict routes nowhere and the run + completes on it — which is the point: the accounting reads what the + verdict raised, whether or not a route acted on it, and a routed finding + a repairable finding names by slug is acted on and filed by nobody.""" + run = driven(findings=[OWN_FINDING], + verdicts=[repairing(naming_the_slug(OWN_FINDING, + REPAIRABLE_FINDING))]) + + assert run.code == 0 + assert run.state["status"] == "completed" + recorded = accounting(run) + assert (recorded["routed"], recorded["acted_on"], recorded["filed"]) == \ + (1, 1, 0) + assert recorded["acted_on_slugs"] == [OWN_FINDING["slug"]] + assert routed_markers(run) == [] + assert filings_of(filings, OWN_FINDING) == 0 + + +def test_a_repairable_finding_naming_the_file_alone_leaves_it_filed( + driven, filings): + """The control: the same run with the repairable finding naming the file + and no slug, and the routed finding is filed.""" + run = driven(findings=[OWN_FINDING], + verdicts=[repairing(REPAIRABLE_FINDING)]) + + assert run.code == 0 + recorded = accounting(run) + assert (recorded["routed"], recorded["acted_on"], recorded["filed"]) == \ + (1, 0, 1) + assert routed_markers(run) == [OWN_FINDING["body"]] + assert filings_of(filings, OWN_FINDING) == 1 + + def test_a_blocking_issue_naming_the_slug_is_acting_on_the_finding(driven): """The other kind of entry the rule reads. A failed verdict whose blocking issue names the routed finding's slug acts on it on attempt 1, so nothing diff --git a/tests/test_repair_pass.py b/tests/test_repair_pass.py new file mode 100644 index 0000000..c708afb --- /dev/null +++ b/tests/test_repair_pass.py @@ -0,0 +1,1836 @@ +"""A finding about the story's own change is fixed by the stage that owns it. + +A verifier that wants a finding fixed had two routes, and most findings fit +neither. A correctable finding re-enters at the stage the `correction_pass` +declaration names and is bounded to the words alone; a blocking issue fails the +verdict and spends a retry. The repair pass is the third route: a passing +verdict may carry `repairable_findings`, each naming a path, a location, the +finding, the correction and a retry category, and the coordinator resolves the +category through the verifier stage's `retry_routing` table — exactly as it +resolves a failed verdict's `retry_target` — re-enters at the earliest stage in +workflow order among the resolved stages, and runs forward to verification +again. It has the correction pass's cost and the retry routing's reach: no +`retry_count`, no `attempts/attempt-N/` archive, no `retry-history.json` entry, +and the passing verdict that routed it still stands. Each stage entered is +granted the paths of the findings whose category resolved to it, for that +attempt alone. + +Because a repair may change behaviour, it is built on a verified tree: it +routes only after the clean-clone check on the routing verdict has passed, and +the verification it returns to runs the check again. That is the opposite +ordering from the correction pass, which routes before the check, and both +orderings are driven here as runs rather than argued from source. + +Every claim below is read off what a real run wrote: the stages the fake +runner was invoked for, the event stream, `state.json`, the run directory and +the prompts the coordinator rendered. Nothing here invokes a model. + +The workflow those runs execute is built by `tests/conftest.py`'s builder and +materialized into a harness root this module owns, for the reason story-048 +established: the subject is *the mechanism*, and the stage list, the retry +categories and the artifact names are inputs to it. Every name below still +comes off a definition rather than being spelled at an assertion — it is the +fixture's definition rather than the shipped one. The cases whose subject +genuinely is what this repository ships — both shipped definitions declare the +pass, every shipped prompt carries the slot, the shipped verifier prompts bound +the field — read the shipped artifacts and say so. + +Every absence asserted here carries a demonstration that it can fail: + + * "the pass spends no retry budget" — `retry_count` unchanged, no attempt + archive, no retry-history entry — sits beside `retry_run`, a run through + the same fixture that does spend all three; + * "the guidance in force is empty at the entry to the repaired stage" sits + beside a retry-routed entry to the *same* stage, where it is not empty; + * "a workflow declaring no repair pass runs a verdict carrying findings as + it ran one carrying none" is a comparison of two runs under the + non-declaring workflow, and sits beside the same comparison under the + declaring workflow, which must report a difference; + * "no stage is invoked past the budget" sits beside the exhausted verdict's + findings being recorded as an event, so a spent budget is distinguishable + from a mechanism that never ran; + * "an undeclared category escalates" sits beside a declared one that routes; + * "the clean-clone check ran before the pass" sits beside a run whose check + fails, which takes the retry route and writes no pass record; + * "a granted edit is neither reverted nor an ownership violation" sits beside + the same edit with nothing granted, which is reverted or escalates, and + beside the same stage re-entered on a later attempt, which inherits nothing; + * "no name this mechanism routes on is written in orchestration source" sits + beside the same names being present in the workflow this module built; + * "a refused declaration created no run state" sits beside the same harness + with the declaration repaired, which creates all of it; + * "the schema accepts a well-formed finding" sits beside an entry missing + each required field in turn, which it refuses. +""" +import json +import re +import shutil +import subprocess +from collections import Counter +from pathlib import Path + +import pytest + +import context_assembler +import harness_config +import schema_validator +import story_coordinator +import story_parser +from agent_runner import AgentResult +from conftest import StageRef, workflow_stage +import conftest + +REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] +COORDINATOR_SOURCE = Path(story_coordinator.__file__).read_text(encoding="utf-8") + +#: The artifact the fixture's declaration names. Deliberately not the name this +#: repository deploys: the record reaching the run directory under this name is +#: what says the coordinator reads the name off the declaration rather than +#: carrying one of its own. +REPAIR_ARTIFACT = "repair-probe.json" +CORRECTION_ARTIFACT = "correction-probe.json" + +#: The revert check the writing stage declares, so a repair pass's grant to +#: that stage is observable at run level: an ungranted edit outside its +#: confinement is decided by the check, a granted one is exempt from it. +REVERT_ARTIFACT = "revert-probe.json" +CHECKED = {"result": REVERT_ARTIFACT, "baseline": "stage-baseline", + "discarded": "discarded"} + +#: Where the writing stage is confined to. Its ordinary edit is inside it; +#: the files a repair finding grants below are outside it. +CONFINED_TO = "src/" +ORDINARY_FILE = "src/app.py" + +#: The workflow these runs execute. Four stages, because "the entry stage is +#: the earliest among the resolved stages" needs categories routing to more +#: than one stage, and "the correction pass is untouched" needs the two passes +#: declared side by side on one verifier. The clean-clone declaration is here +#: because the repair pass routes only after that check, which is an ordering +#: this module asserts. The correction pass names the last writing stage and +#: a budget of one, so the two budgets can be shown to be spent separately. +WORKFLOW = conftest.build_workflow( + workflow_stage( + outputs=(conftest.CHANGED_FILES, conftest.IMPLEMENTATION_SUMMARY), + changed_files=conftest.CHANGED_FILES, + schemas={conftest.CHANGED_FILES: "changed-files"}, + may_only_change=(CONFINED_TO,), + revert_check=CHECKED), + workflow_stage( + outputs=(conftest.TEST_RESULTS, conftest.TESTER_CHANGED_FILES), + changed_files=conftest.TESTER_CHANGED_FILES, + schemas={conftest.TEST_RESULTS: "test-results", + conftest.TESTER_CHANGED_FILES: "changed-files"}), + workflow_stage( + outputs=(conftest.DOCUMENTATION_REPORT, + conftest.DOCUMENTER_CHANGED_FILES), + changed_files=conftest.DOCUMENTER_CHANGED_FILES, + schemas={conftest.DOCUMENTER_CHANGED_FILES: "changed-files"}), + workflow_stage( + name=conftest.VERIFYING_STAGE, + outputs=(conftest.VERIFICATION_RESULT,), + schemas={conftest.VERIFICATION_RESULT: "verification-result", + conftest.RETRY_GUIDANCE: "retry-guidance"}, + clean_clone={"result": conftest.CLEAN_CLONE_RESULT, + "retry_stage": StageRef(0)}, + correction_pass={"result": CORRECTION_ARTIFACT, "budget": 1, + "stage": StageRef(2)}, + repair_pass={"result": REPAIR_ARTIFACT, "budget": 2}, + retry_routing={ + "the-behaviour": {"stage": StageRef(0), + "when": "the behaviour the story asked for is missing"}, + "the-checks": {"stage": StageRef(1), + "when": "the validation does not hold the behaviour"}, + "the-record": {"stage": StageRef(2), + "when": "the documents do not describe what shipped"}, + }), + escalation_rules={"max_retries_exceeded": {"action": "escalate"}}, + name="repair-pass-workflow", +) + +STAGE_NAMES = [stage["name"] for stage in WORKFLOW["stages"]] +WRITING, VALIDATING, DOCUMENTING, VERIFYING = STAGE_NAMES + +VERIFIER_STAGE = next(s for s in WORKFLOW["stages"] if "repair_pass" in s) +ROUTES = VERIFIER_STAGE["on_failure"]["retry_routing"] +BEHAVIOUR_CATEGORY, CHECKS_CATEGORY, RECORD_CATEGORY = list(ROUTES) +REPAIR = VERIFIER_STAGE["repair_pass"] +ARTIFACT = REPAIR["result"] +BUDGET = REPAIR["budget"] +CORRECTION = VERIFIER_STAGE["correction_pass"] +CORRECTION_ENTRY = CORRECTION["stage"] + +#: A category the fixture deliberately does not declare, which is the whole of +#: what makes it undeclared. Asserted rather than assumed below. +UNDECLARED_CATEGORY = "a-category-this-workflow-does-not-declare" + +MAX_RETRIES = json.loads( + (REPO_ROOT / "rules" / "execution-rules.json").read_text(encoding="utf-8") +)["max_retries"] + +VERDICT_SCHEMA = schema_validator.load_schema("verification-result") +HISTORY_SCHEMA = schema_validator.load_schema("execution-history") + +#: The field a repairable finding is recorded under, and the field the +#: correction pass reads. Spelled once each; every verdict built below and +#: every schema assertion derives from these. +REPAIRABLE = "repairable_findings" +CORRECTABLE = "correctable_findings" +REQUIRED_OF_A_FINDING = {"path", "location", "finding", "correction", "category"} + +#: The events this mechanism appends, as the execution-history schema +#: enumerates them, beside the ones it is asserted against. +ROUTED = "repair-pass-routed" +RECORDED = "repair-pass-recorded" +GRANT_EVENT = "repair-pass-grant-applied" +CORRECTION_ROUTED = "correction-pass-routed" +CLEAN_CLONE_PASSED = "clean-clone-passed" +CLEAN_CLONE_FAILED = "clean-clone-failed" +REVERTED_EVENT = "revert-check-reverted" +PERMITTED_EVENT = "revert-check-permitted" + + +def destination_of(category: str) -> str: + """Where the fixture's table routes a category. Read, never written.""" + return ROUTES[category]["stage"] + + +#: Files outside the writing stage's confinement that a repair finding may +#: grant it. Committed into the target by `governed_target` below, so a revert +#: has a baseline to restore them to. +NOTES = "NOTES.md" +OTHER_NOTES = "OTHER-NOTES.md" +NEW_FILE = "docs/NEW.md" +NOTES_AT_HEAD = "# Notes\n\nThe notes say three things.\n" +NOTES_REPAIRED = "# Notes\n\nThe notes say three things, and now say them.\n" +NOTES_REWORDED_AGAIN = "# Notes\n\nReworded again on a retry.\n" +OTHER_NOTES_AT_HEAD = "# Other notes\n" +OTHER_NOTES_REWORDED = "# Other notes, reworded by a pass not granted them.\n" + +#: Distinctive text so a search of a rendered prompt is looking for these +#: findings rather than for any sentence about repairs. A finding is matched by +#: its own words, which is what "the finding reached the stage" means. +BEHAVIOUR_FINDING = { + "path": NOTES, + "location": f"{NOTES} - the second paragraph", + "finding": "MARKER-BEHAVIOUR the notes say three things and name none", + "correction": "MARKER-BEHAVIOUR-FIX say them", + "category": BEHAVIOUR_CATEGORY, +} +CHECKS_FINDING = { + "path": "tests/test_sample.py", + "location": "tests/test_sample.py::test_the_sample - the absence assertion", + "finding": "MARKER-CHECKS the absence assertion has no negative control", + "correction": "MARKER-CHECKS-FIX add the control beside it", + "category": CHECKS_CATEGORY, +} +RECORD_FINDING = { + "path": ".harness/docs/ARCHITECTURE.md", + "location": ".harness/docs/ARCHITECTURE.md - the routing section", + "finding": "MARKER-RECORD the section names a stage that was renamed", + "correction": "MARKER-RECORD-FIX name the stage the workflow declares today", + "category": RECORD_CATEGORY, +} +UNKNOWN_FINDING = { + "path": "src/app.py", + "location": "src/app.py - the module comment", + "finding": "MARKER-UNKNOWN the comment describes behaviour the code lost", + "correction": "MARKER-UNKNOWN-FIX describe what the code does now", + "category": UNDECLARED_CATEGORY, +} + +#: One finding per declared category, keyed by the category the finding itself +#: carries rather than by a second spelling of it. +FINDING_FOR = {finding["category"]: finding + for finding in (BEHAVIOUR_FINDING, CHECKS_FINDING, RECORD_FINDING)} + +#: A correctable finding, for the cases about the two passes being +#: independent. Categorised for a category that routes somewhere other than +#: the declared correction entry, so a run that entered where the category +#: points would be visibly different from one that entered where the +#: declaration points. +CORRECTABLE_FINDING = { + "path": "src/app.py", + "location": "src/app.py - the docstring of the sample function", + "finding": "MARKER-CORRECTABLE the docstring wraps mid-word", + "correction": "MARKER-CORRECTABLE-FIX rewrap the line", + "category": BEHAVIOUR_CATEGORY, +} + +PASS = {"status": "passed", "blocking_issues": [], "unverified": [], + "retry_recommended": False} + + +def repairing(*findings: dict) -> dict: + return {**PASS, REPAIRABLE: [dict(f) for f in findings]} + + +def correcting(*findings: dict) -> dict: + return {**PASS, CORRECTABLE: [dict(f) for f in findings]} + + +def failing_into(category: str) -> dict: + return { + "status": "failed", + "blocking_issues": [{ + "severity": "high", "issue": "sample behavior missing", + "location": "src/app.py", + "required_behavior": "sample behavior exists", + }], + "unverified": [], "retry_recommended": True, "retry_target": category, + } + + +# -------------------------------------------------------------------------- +# Fixture plumbing +# -------------------------------------------------------------------------- + + +#: The context fields the coordinator injects the two pass records under. +#: `conftest.BUILT_PROMPT_FIELDS` predates them and the fixture says a module +#: needing a field it does not list passes its own template, which is what +#: this does. Spelled once, here, and derived from here by every assertion +#: that looks for a record in a prompt. +REPAIR_FIELD = "repair_pass_result" +CORRECTION_FIELD = "correction_pass_result" + +PROMPTS = { + name: (conftest.built_stage_prompt(name) + + f"{CORRECTION_FIELD}:\n{{{{{CORRECTION_FIELD}}}}}\n\n" + + f"{REPAIR_FIELD}:\n{{{{{REPAIR_FIELD}}}}}\n") + for name in STAGE_NAMES +} + + +@pytest.fixture +def configured_workflow() -> str: + """Point the shared target fixture at the definition built above.""" + return WORKFLOW["name"] + + +@pytest.fixture +def harness_root(tmp_path: Path) -> Path: + """A harness root carrying that definition, so every case below drives a + real coordinator loading a real file.""" + return conftest.materialize_workflow(WORKFLOW, + tmp_path / "repair-pass-harness", + prompts=PROMPTS) + + +@pytest.fixture +def governed_target(target_root: Path) -> Path: + """The shared target with the files a repair finding may grant, committed + so the writing stage's baseline holds them and a revert has something to + restore.""" + write(target_root / NOTES, NOTES_AT_HEAD) + write(target_root / OTHER_NOTES, OTHER_NOTES_AT_HEAD) + conftest.commit_setup(target_root, "the files a repair finding may name") + return target_root + + +def write(path: Path, text: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + + +def write_json(path: Path, payload) -> None: + write(path, json.dumps(payload, indent=2) + "\n") + + +def read_json(path: Path): + return json.loads(path.read_text(encoding="utf-8")) + + +def run_dir_of(target_root: Path, story_id: str = "story-001") -> Path: + return conftest.run_dir_for(target_root, story_id) + + +def history_of(run_dir: Path) -> list[dict]: + return read_json(run_dir / "execution-history.json") + + +def events_of(run_dir: Path) -> list[str]: + return [entry["event"] for entry in history_of(run_dir)] + + +def events_of_kind(run_dir: Path, kind: str, stage: str | None = None) -> list[dict]: + return [entry for entry in history_of(run_dir) + if entry["event"] == kind + and (stage is None or entry.get("stage") == stage)] + + +def read_state(run_dir: Path) -> dict: + return read_json(run_dir / "state.json") + + +def record_path(run_dir: Path, number: int = 1, artifact: str = ARTIFACT) -> Path: + return run_dir / story_coordinator.repair_pass_result_file(artifact, number) + + +def record_of(run_dir: Path, number: int = 1, artifact: str = ARTIFACT) -> dict: + return read_json(record_path(run_dir, number, artifact)) + + +def rendered_prompt(run_dir: Path, stage: str, attempt: int = 1) -> str: + """The prompt one stage was given, read back off the run directory, + through the coordinator's own name-shaping function.""" + return (run_dir / story_coordinator.prompt_file(stage, attempt)).read_text( + encoding="utf-8") + + +def paths_named_in(message: str, candidates: tuple[str, ...]) -> set[str]: + """Which of the candidate paths a message names as a whole token. A + plain substring test cannot tell NOTES.md from the tail of OTHER-NOTES.md, + so a name counts only when nothing that could be part of a path — a + word character, a dot or a hyphen — sits on either side of it.""" + return {candidate for candidate in candidates + if re.search(rf"(? str: + """A file in the tree the run worked in, which holds its run directory + under the configured runs_dir — so the tree is two levels above it.""" + return (run_dir.parents[2] / relative).read_text(encoding="utf-8") + + +def clean_clone_events(run_dir: Path) -> list[str]: + return [event for event in events_of(run_dir) + if event in (CLEAN_CLONE_PASSED, CLEAN_CLONE_FAILED)] + + +# -------------------------------------------------------------------------- +# The writing stage's edits on a pass, each with the record for it +# -------------------------------------------------------------------------- + + +def the_named_file(tree: Path) -> dict: + """The repair the pass asked for, in the file the finding named.""" + write(tree / NOTES, NOTES_REPAIRED) + return {"modified": [NOTES], "created": [], "deleted": []} + + +def the_named_file_again(tree: Path) -> dict: + """A later edit to the same file with different words, so undoing it is + observable.""" + write(tree / NOTES, NOTES_REWORDED_AGAIN) + return {"modified": [NOTES], "created": [], "deleted": []} + + +def the_named_file_and_another(tree: Path) -> dict: + """The same repair, and a second governed file no finding named.""" + write(tree / NOTES, NOTES_REPAIRED) + write(tree / OTHER_NOTES, OTHER_NOTES_REWORDED) + return {"modified": [NOTES, OTHER_NOTES], "created": [], "deleted": []} + + +def a_new_file(tree: Path) -> dict: + """A file created outside the confinement, which the ownership check + decides rather than the revert check.""" + write(tree / NEW_FILE, "created on a repair pass\n") + return {"modified": [], "created": [NEW_FILE], "deleted": []} + + +class Runner: + """A fake agent runner that writes each stage's declared artifacts. + + It records, per invocation, the prompt it was handed and the guidance the + coordinator had in force at that moment — read off `state.json` through + the shared helper at the moment of the call, because "the guidance in + force *at the entry to* the stage" is a question about that instant. + + `edits` is the list of edit functions the writing stage's invocations + make, in order — None for an invocation that changes nothing beyond its + ordinary `src/app.py` — so a run that re-enters the writing stage can have + it do something different the second time. Every invocation's record + names the ordinary file beside whatever the edit function touched: the + first invocation changed it, a repair pass makes no new attempt, and the + changed-files completeness check's tree signature is first-seen-wins + within an attempt — so a re-entered invocation that recorded the pass's + edit alone would be reported for the ordinary file still held by the + tree, and the run would stop before any grant was decided. + """ + + def __init__(self, target_root: Path, verdicts: list[dict], + edits: list | None = None, story_id: str = "story-001"): + self.target_root = target_root + self.run_dir = run_dir_of(target_root, story_id) + self.verdicts = list(verdicts) + self.edits = list(edits or []) + self.calls: list[str] = [] + self.prompts: dict[str, list[str]] = {} + self.guidance: dict[str, list[list[str]]] = {} + + def __call__(self, prompt, *, stage, cwd, log_path=None, + permission_mode=None, model=None, allowed_tools=None, + max_budget_usd=None, run_dir=None, **declared): + self.calls.append(stage) + self.prompts.setdefault(stage, []).append(prompt) + self.guidance.setdefault(stage, []).append( + conftest.guidance_in_force(self.run_dir)) + if stage == WRITING: + edit = self.edits.pop(0) if self.edits else None + if edit: + record = edit(Path(cwd)) + record["modified"] = [ORDINARY_FILE] + [ + path for path in record["modified"] if path != ORDINARY_FILE] + else: + (Path(cwd) / ORDINARY_FILE).write_text( + "print('hello')\n# the story's change\n", encoding="utf-8") + record = {"modified": [ORDINARY_FILE], "created": [], + "deleted": []} + write_json(self.run_dir / conftest.CHANGED_FILES, record) + (self.run_dir / conftest.IMPLEMENTATION_SUMMARY).write_text( + "Did the work.\n", encoding="utf-8") + elif stage == VALIDATING: + write_json(self.run_dir / conftest.TEST_RESULTS, + {"tests_written": 2}) + write_json(self.run_dir / conftest.TESTER_CHANGED_FILES, + {"modified": [], "created": ["tests/test_app.py"], + "deleted": []}) + elif stage == DOCUMENTING: + (self.run_dir / conftest.DOCUMENTATION_REPORT).write_text( + "No changes needed.\n", encoding="utf-8") + write_json(self.run_dir / conftest.DOCUMENTER_CHANGED_FILES, + {"modified": [], "created": [], "deleted": []}) + elif stage == VERIFYING: + verdict = conftest.answering_guidance( + self.verdicts.pop(0), self.run_dir) + write_json(self.run_dir / conftest.VERIFICATION_RESULT, verdict) + if verdict["status"] == "failed": + write_json(self.run_dir / conftest.RETRY_GUIDANCE, { + "current_focus": [{ + "focus": "fix the sample behavior", + "satisfied_when": "the sample behavior exists", + }], + "preserve_behavior": ["existing behavior"], + "retry_scope": ["src/app.py"], + }) + return AgentResult(ok=True, result_text=f"{stage} done") + + +def drive(target_root: Path, harness: Path, verdicts: list[dict], + edits: list | None = None): + """One run, returning its exit code, its runner and its run directory.""" + runner = Runner(target_root, verdicts, edits) + code = story_coordinator.run_story( + "story-001", harness, target_root, runner) + return code, runner, run_dir_of(target_root) + + +def probe_harness(tmp_path: Path, target_root: Path, name: str, mutate) -> Path: + """A harness root carrying the built definition with the verifier mutated, + and `target_root` configured to run it — the idiom the correction-pass and + clean-clone modules use for their own declarations.""" + workflow = json.loads(json.dumps(WORKFLOW)) + for stage in workflow["stages"]: + if stage["name"] == VERIFYING: + mutate(stage) + workflow["name"] = name + root = conftest.materialize_workflow(workflow, tmp_path / name, + prompts=PROMPTS) + configure(target_root, workflow=name) + return root + + +def second_target(target_root: Path, tmp_path: Path) -> Path: + """A copy of the target repository, so two coordinators can be compared: + a run refuses to re-run a story in a target that has already run it.""" + other = tmp_path / "second-target" + shutil.copytree(target_root, other) + return other + + +def configure(target_root: Path, **overrides) -> None: + """Rewrite the target's config keys, adding those it does not carry.""" + path = target_root / ".harness" / "config.yaml" + lines = path.read_text(encoding="utf-8").splitlines() + for key, value in overrides.items(): + rendered = f"{key}: {value}" + for index, line in enumerate(lines): + if line.startswith(f"{key}:"): + lines[index] = rendered + break + else: + lines.append(rendered) + path.write_text("\n".join(lines) + "\n", encoding="utf-8") + conftest.commit_setup(target_root, "configure the target for this test") + + +def entry_shape(entry: str) -> list[str]: + """The stages a run entering at `entry` invokes: all of them, then that one + and everything after it.""" + return [*STAGE_NAMES, *STAGE_NAMES[STAGE_NAMES.index(entry):]] + + +@pytest.fixture +def repaired_run(target_root, harness_root): + """The central run: a passing verdict carrying one repairable finding whose + category the table maps to the first stage. The workflow re-enters there + and runs forward to verification again, which passes carrying nothing.""" + return drive(target_root, harness_root, + [repairing(BEHAVIOUR_FINDING), PASS]) + + +@pytest.fixture +def retry_run(target_root, harness_root): + """The control every "spends nothing" assertion needs: a run that spends. + A failed verdict naming the same category routes a retry to the same + stage, so the two runs differ in the mechanism under test and in nothing + else.""" + return drive(target_root, harness_root, + [failing_into(BEHAVIOUR_CATEGORY), PASS]) + + +@pytest.fixture +def unrepaired_run(target_root, harness_root): + """A passing verdict carrying no findings, under the declaring workflow.""" + return drive(target_root, harness_root, [PASS]) + + +# -------------------------------------------------------------------------- +# The fixture is what it claims to be +# -------------------------------------------------------------------------- + + +def test_the_undeclared_category_really_is_undeclared(): + assert UNDECLARED_CATEGORY not in ROUTES + assert set(ROUTES) == {BEHAVIOUR_CATEGORY, CHECKS_CATEGORY, RECORD_CATEGORY} + assert set(FINDING_FOR) == set(ROUTES) + + +def test_the_declaration_names_no_entry_stage_of_its_own(): + """The premise "the stage comes off the routing table" rests on: the + declaration carries a result and a budget and nothing that chooses a + stage, where the correction pass beside it does.""" + assert "stage" not in REPAIR + assert "stage" in CORRECTION + assert BUDGET >= 2, "the budget cases need more than one pass" + + +def test_the_categories_route_to_more_than_one_stage_and_none_to_the_verifier(): + """The premise "earliest in workflow order" rests on: if every category + routed to one stage there would be nothing to order, and a category + routing to the verifier would re-enter a pass that runs nothing before + judging itself.""" + destinations = {destination_of(category) for category in ROUTES} + assert len(destinations) >= 2 + assert VERIFYING not in destinations + for destination in destinations: + assert destination in STAGE_NAMES + + +def test_the_granted_files_are_outside_the_writing_stages_confinement(): + """What makes the grant observable: the files the findings name are + governed for the stage they resolve to, and the ordinary edit is not.""" + (confinement,) = story_coordinator.restrictions_on(WORKFLOW["stages"][0]) + assert confinement.sense == story_coordinator.CONFINEMENT + assert confinement.governs(NOTES) + assert confinement.governs(OTHER_NOTES) + assert confinement.governs(NEW_FILE) + assert not confinement.governs("src/app.py") + assert destination_of(BEHAVIOUR_FINDING["category"]) == WRITING + + +# -------------------------------------------------------------------------- +# The routed pass +# -------------------------------------------------------------------------- + + +def test_the_stage_the_category_resolves_to_runs_again_within_the_same_run( + repaired_run, +): + code, runner, run_dir = repaired_run + entry = destination_of(BEHAVIOUR_FINDING["category"]) + + assert code == 0 + assert runner.calls == entry_shape(entry) + assert read_state(run_dir)["status"] == "completed" + assert (run_dir / "completion-report.md").is_file() + + +def test_the_run_records_the_pass_in_its_own_event_stream(repaired_run): + _, _, run_dir = repaired_run + routed = events_of_kind(run_dir, ROUTED) + assert len(routed) == 1 + assert routed[0]["stage"] == VERIFYING + assert routed[0]["retry_stage"] == destination_of(BEHAVIOUR_CATEGORY) + assert schema_validator.validate(history_of(run_dir), HISTORY_SCHEMA) == [] + + +def test_the_record_the_coordinator_wrote_is_in_the_run_directory(repaired_run): + """The numbered record: the pass, the attempt, the entry stage, each + finding with the stage its category resolved to, and the statement.""" + _, _, run_dir = repaired_run + record = record_of(run_dir) + + assert record["pass"] == 1 + assert record["attempt"] == 1 + assert record["stage"] == destination_of(BEHAVIOUR_CATEGORY) + assert record["findings"] == [ + {**BEHAVIOUR_FINDING, "stage": destination_of(BEHAVIOUR_CATEGORY)}] + assert record["statement"] == story_coordinator.repair_pass_statement( + record["stage"]) + + +def test_the_finding_reaches_the_entered_stage_in_its_rendered_prompt( + repaired_run, +): + _, runner, run_dir = repaired_run + entry = destination_of(BEHAVIOUR_CATEGORY) + prompt = rendered_prompt(run_dir, entry) + + for words in (BEHAVIOUR_FINDING["location"], BEHAVIOUR_FINDING["finding"], + BEHAVIOUR_FINDING["correction"]): + assert words in prompt, words + assert "{{" not in prompt + # And in the control: the first rendering of the same stage carried + # nothing of it, so the finding is there because the pass put it there. + first, second = runner.prompts[entry] + assert BEHAVIOUR_FINDING["finding"] not in first + assert BEHAVIOUR_FINDING["finding"] in second + + +def test_every_stage_the_pass_runs_forward_through_is_told_of_it(repaired_run): + """The record is injected into every stage prompt, not only the entered + one: a stage running because the workflow runs forward from the entry is + told it has nothing to repair rather than left to guess why it is running.""" + _, runner, _ = repaired_run + entry = destination_of(BEHAVIOUR_CATEGORY) + for stage in STAGE_NAMES[STAGE_NAMES.index(entry):]: + first, second = runner.prompts[stage] + assert BEHAVIOUR_FINDING["finding"] not in first, stage + assert BEHAVIOUR_FINDING["finding"] in second, stage + + +def test_the_statement_says_what_the_pass_is_and_is_not(): + """What a stage most needs to know, read off the coordinator's own + statement: that no retry was spent, that only the named files are its to + change, that it is not a licence to revisit the work, and that the + clean-clone check runs again.""" + statement = story_coordinator.repair_pass_statement(WRITING) + assert WRITING in statement + assert "not a retry" in statement + assert "no retry budget was spent" in statement + assert "not a licence to revisit the work" in statement + assert "clean-clone check" in statement + assert "nothing else" in statement + + +# -------------------------------------------------------------------------- +# The pass spends nothing a retry spends +# -------------------------------------------------------------------------- + + +def test_the_pass_leaves_the_retry_budget_untouched(repaired_run): + _, _, run_dir = repaired_run + state = read_state(run_dir) + assert state["retry_count"] == 0 + assert state["repair_pass_count"] == 1 + assert not (run_dir / "attempts").exists() + assert not (run_dir / "retry-history.json").exists() + + +def test_a_run_that_does_spend_the_retry_budget_shows_all_three(retry_run): + """The control beside it: each absence above is asserted here as a + presence, against a run through the same fixture that routed a retry to + the same stage.""" + code, _, run_dir = retry_run + assert code == 0 + state = read_state(run_dir) + assert state["retry_count"] == 1 + assert state["repair_pass_count"] == 0 + assert (run_dir / "attempts" / "attempt-1").is_dir() + assert (run_dir / "retry-history.json").is_file() + + +def test_the_pass_takes_no_attempt_number_of_its_own(repaired_run): + _, _, run_dir = repaired_run + entry = destination_of(BEHAVIOUR_CATEGORY) + assert record_of(run_dir)["attempt"] == 1 + assert (run_dir / story_coordinator.prompt_file(entry, 1)).is_file() + assert not (run_dir / story_coordinator.prompt_file(entry, 2)).exists() + + +def test_the_pass_does_not_turn_the_passing_verdict_into_a_failing_one( + repaired_run, +): + """Read off the archived iteration, which is the verdict as it was judged + rather than as the run directory's live artifact ended up.""" + _, _, run_dir = repaired_run + archived = read_json(run_dir / "verification" / "iteration-1.json") + assert archived["status"] == "passed" + assert archived[REPAIRABLE] == [BEHAVIOUR_FINDING] + assert read_state(run_dir)["verification_iterations"] == 2 + + +def test_a_failed_verdict_archives_as_failed(retry_run): + """Control: the archive records the verdict's own status.""" + _, _, run_dir = retry_run + archived = read_json(run_dir / "verification" / "iteration-1.json") + assert archived["status"] == "failed" + + +def test_a_run_that_takes_no_pass_counts_none(unrepaired_run): + _, _, run_dir = unrepaired_run + assert read_state(run_dir)["repair_pass_count"] == 0 + assert not record_path(run_dir).exists() + + +def test_a_state_file_written_before_the_field_existed_still_loads(tmp_path): + """The field is defaulted so a resumed run's state.json, written before + this story, reads as having taken no pass — and a saved count survives a + round trip so a resumed run cannot spend the pass again.""" + run_dir = tmp_path / "run" + run_dir.mkdir() + state = story_coordinator.RunState(story_id="story-001", + branch="story/story-001") + story_coordinator.save_state(run_dir, state) + written = read_state(run_dir) + del written["repair_pass_count"] + write_json(run_dir / "state.json", written) + assert story_coordinator.load_state(run_dir).repair_pass_count == 0 + + state.repair_pass_count = BUDGET + story_coordinator.save_state(run_dir, state) + assert story_coordinator.load_state(run_dir).repair_pass_count == BUDGET + + +# -------------------------------------------------------------------------- +# The guidance in force +# -------------------------------------------------------------------------- + + +def test_no_guidance_is_in_force_at_the_entry_to_the_repaired_stage( + repaired_run, +): + _, runner, _ = repaired_run + assert runner.guidance[destination_of(BEHAVIOUR_CATEGORY)] == [[], []] + + +def test_a_retry_routed_to_the_same_stage_does_carry_guidance(retry_run): + """The control beside it: the same stage, entered the other way, has + guidance in force.""" + _, runner, _ = retry_run + entries = runner.guidance[destination_of(BEHAVIOUR_CATEGORY)] + assert entries[0] == [] + assert entries[1] != [] + + +# -------------------------------------------------------------------------- +# The clean-clone check runs before the pass, and again after it +# -------------------------------------------------------------------------- + + +def test_the_pass_routes_only_after_the_clean_clone_check_has_passed( + repaired_run, +): + """Ordering off the event stream: the check on the routing verdict passed + first, then the pass routed, then the check on the repaired tree passed + again. Two passing verdicts, two checks.""" + _, _, run_dir = repaired_run + events = events_of(run_dir) + assert read_state(run_dir)["verification_iterations"] == 2 + assert clean_clone_events(run_dir) == [CLEAN_CLONE_PASSED, CLEAN_CLONE_PASSED] + first_check = events.index(CLEAN_CLONE_PASSED) + second_check = events.index(CLEAN_CLONE_PASSED, first_check + 1) + assert first_check < events.index(ROUTED) < second_check + assert events.index(ROUTED) < events.index("story-completed") + + +def test_a_run_that_takes_no_pass_runs_the_check_once(unrepaired_run): + """The control for "twice": one passing verdict, one check, where it + always was — after the verdict and before the run completes.""" + _, _, run_dir = unrepaired_run + events = events_of(run_dir) + assert clean_clone_events(run_dir) == [CLEAN_CLONE_PASSED] + assert events.index("verification-passed") < events.index(CLEAN_CLONE_PASSED) + assert events.index(CLEAN_CLONE_PASSED) < events.index("story-completed") + assert ROUTED not in events + + +#: A suite that fails wherever it runs, so the clean-clone check on the +#: routing verdict fails and the retry route above the repair block is what +#: the run takes. +SUITE_ALWAYS_RED = "sh -c 'exit 1'" + + +def test_a_verdict_whose_clean_clone_check_fails_takes_the_retry_route_instead( + target_root, harness_root, +): + """The other half of the ordering: a verdict carrying a repairable finding + whose check fails never reaches the repair block. It routes as any + clean-clone failure does — a retry, spent — and no pass record is written + for it. Enough verdicts are supplied for the check to fail on every one + until the ceiling is reached.""" + configure(target_root, test_command=SUITE_ALWAYS_RED) + code, _, run_dir = drive( + target_root, harness_root, + [repairing(BEHAVIOUR_FINDING), *[PASS] * (MAX_RETRIES + 2)]) + events = events_of(run_dir) + + assert code == 2 + assert CLEAN_CLONE_FAILED in events + assert CLEAN_CLONE_PASSED not in events + assert ROUTED not in events + assert not record_path(run_dir).exists() + failure = events_of_kind(run_dir, CLEAN_CLONE_FAILED)[0] + assert failure["retry_decision"] == "retry" + assert failure["retry_stage"] == VERIFIER_STAGE["clean_clone"]["retry_stage"] + state = read_state(run_dir) + assert state["retry_count"] >= 1 + assert state["repair_pass_count"] == 0 + + +def test_the_same_verdict_under_a_green_suite_does_route(repaired_run): + """The control: the identical verdict with the check passing is the one + that routed above, so the absence is the failed check's doing.""" + _, _, run_dir = repaired_run + assert ROUTED in events_of(run_dir) + assert record_path(run_dir).is_file() + + +# -------------------------------------------------------------------------- +# The destination comes off the routing table +# -------------------------------------------------------------------------- + + +@pytest.mark.parametrize("category", sorted(ROUTES)) +def test_every_declared_category_enters_at_the_stage_its_route_names( + target_root, harness_root, category, +): + """Driven as a run per category rather than by calling the routing + function, because what the story changed is where a run goes. Unlike the + correction pass, the category chooses.""" + code, runner, run_dir = drive( + target_root, harness_root, [repairing(FINDING_FOR[category]), PASS]) + + assert code == 0 + assert record_of(run_dir)["stage"] == destination_of(category) + assert runner.calls == entry_shape(destination_of(category)) + for stage in STAGE_NAMES[:STAGE_NAMES.index(destination_of(category))]: + assert runner.calls.count(stage) == 1, stage + + +def test_two_categories_enter_once_at_the_earliest_of_their_stages( + target_root, harness_root, +): + """A verdict naming two categories takes one pass that enters at the + earliest stage in workflow order among the resolved ones and runs forward + through the other. Each finding in the record carries its own stage, in + the order the verdict recorded them.""" + later, earlier = RECORD_FINDING, CHECKS_FINDING + assert STAGE_NAMES.index(destination_of(later["category"])) > \ + STAGE_NAMES.index(destination_of(earlier["category"])), "the premise" + + code, runner, run_dir = drive( + target_root, harness_root, [repairing(later, earlier), PASS]) + + assert code == 0 + record = record_of(run_dir) + assert record["stage"] == destination_of(earlier["category"]) + assert record["findings"] == [ + {**later, "stage": destination_of(later["category"])}, + {**earlier, "stage": destination_of(earlier["category"])}, + ] + assert runner.calls == entry_shape(destination_of(earlier["category"])) + assert events_of(run_dir).count(ROUTED) == 1 + + +def test_the_same_category_enters_at_the_declared_stage_under_the_correction_pass( + target_root, harness_root, +): + """The contrast that says the two passes are different mechanisms: the + same category, carried as a correctable finding, enters at the stage the + correction declaration names rather than at the one the category routes + to — and spends the correction budget, not the repair one.""" + category = CHECKS_FINDING["category"] + assert destination_of(category) != CORRECTION_ENTRY, "the premise" + code, runner, run_dir = drive( + target_root, harness_root, [correcting(CHECKS_FINDING), PASS]) + + assert code == 0 + assert runner.calls == entry_shape(CORRECTION_ENTRY) + state = read_state(run_dir) + assert state["correction_pass_count"] == 1 + assert state["repair_pass_count"] == 0 + assert not record_path(run_dir).exists() + + +def test_repair_destination_resolves_each_finding_and_orders_the_entry(): + """The routing function directly, over the fixture's own table, so the + two halves of the answer — each finding's stage and the entry — are + pinned to the table rather than to whichever a run meets first.""" + routing = story_coordinator.repair_destination( + [RECORD_FINDING, CHECKS_FINDING], ROUTES, STAGE_NAMES) + assert routing.unknown == [] + assert routing.stage == destination_of(CHECKS_CATEGORY) + assert [one["stage"] for one in routing.resolved] == [ + destination_of(RECORD_CATEGORY), destination_of(CHECKS_CATEGORY)] + + refused = story_coordinator.repair_destination( + [CHECKS_FINDING, UNKNOWN_FINDING], ROUTES, STAGE_NAMES) + assert refused.stage is None + assert refused.resolved == [] + assert refused.unknown == [UNDECLARED_CATEGORY] + + +# -------------------------------------------------------------------------- +# The two passes are independent +# -------------------------------------------------------------------------- + + +def test_a_correction_pass_and_a_repair_pass_spend_their_own_budgets( + target_root, harness_root, +): + """One run that takes both: a correction pass on the first verdict, a + repair pass on the verification it returned to. Each count is one, the + retry count is zero, and both records are in the run directory.""" + code, runner, run_dir = drive( + target_root, harness_root, + [correcting(CORRECTABLE_FINDING), repairing(BEHAVIOUR_FINDING), PASS]) + + assert code == 0 + state = read_state(run_dir) + assert state["correction_pass_count"] == 1 + assert state["repair_pass_count"] == 1 + assert state["retry_count"] == 0 + assert (run_dir / story_coordinator.correction_pass_result_file( + CORRECTION_ARTIFACT, 1)).is_file() + assert record_path(run_dir).is_file() + events = events_of(run_dir) + assert events.index(CORRECTION_ROUTED) < events.index(ROUTED) + assert runner.calls == [*STAGE_NAMES, + *STAGE_NAMES[STAGE_NAMES.index(CORRECTION_ENTRY):], + *STAGE_NAMES[STAGE_NAMES.index( + destination_of(BEHAVIOUR_CATEGORY)):]] + + +def test_the_correction_pass_is_still_bounded_to_the_words_and_its_own_stage(): + """The correction pass's statement and declaration are as they were: it + still says never behaviour, still names the clean-clone check as running + after it, and its declaration still carries its own stage. The repair + statement says the opposite about ordering, which is the difference.""" + correction = story_coordinator.correction_pass_statement(CORRECTION_ENTRY) + repair = story_coordinator.repair_pass_statement(WRITING) + assert "never behaviour" in correction + assert "never behaviour" not in repair + assert "clean-clone check on that verdict passed" in repair + assert CORRECTION["stage"] == CORRECTION_ENTRY + + +# -------------------------------------------------------------------------- +# A workflow declaring no repair pass routes nowhere +# -------------------------------------------------------------------------- + + +def shape_of(code: int, runner: Runner, run_dir: Path) -> tuple: + """What "the same run" means: the exit code, the stages invoked, the event + stream and the retry count, compared whole.""" + return (code, tuple(runner.calls), tuple(events_of(run_dir)), + read_state(run_dir)["retry_count"]) + + +def without_the_declaration(tmp_path: Path, target_root: Path, + name: str = "no-repair-pass") -> Path: + return probe_harness(tmp_path, target_root, name, + lambda stage: stage.pop("repair_pass")) + + +def test_a_workflow_declaring_no_repair_pass_runs_a_verdict_with_findings_as_one_without( + target_root, tmp_path, +): + """The criterion: with the key absent, a verdict carrying repairable + findings routes nowhere and its event stream is the one a verdict + carrying none produces.""" + other = second_target(target_root, tmp_path) + plain = shape_of(*drive(target_root, + without_the_declaration(tmp_path, target_root), + [PASS])) + carrying = shape_of(*drive(other, + without_the_declaration(tmp_path, other, + "no-repair-pass-other"), + [repairing(BEHAVIOUR_FINDING)])) + assert plain == carrying + assert plain[1] == tuple(STAGE_NAMES) + assert not any(e.startswith("repair-pass") for e in plain[2]) + + +def test_that_comparison_reports_a_difference_under_the_declaring_workflow( + target_root, harness_root, tmp_path, +): + """Control: the same two verdicts under the declaring workflow differ in + the stages invoked and in the event stream, so the equality above is a + fact about the missing key rather than about a comparison that cannot + see the pass.""" + other = second_target(target_root, tmp_path) + plain = shape_of(*drive(target_root, harness_root, [PASS])) + carrying = shape_of(*drive(other, harness_root, + [repairing(BEHAVIOUR_FINDING), PASS])) + assert plain != carrying + assert plain[1] != carrying[1] + assert plain[2] != carrying[2] + + +def test_removing_the_declaration_writes_no_record_and_completes( + target_root, tmp_path, +): + harness = without_the_declaration(tmp_path, target_root) + code, runner, run_dir = drive(target_root, harness, + [repairing(BEHAVIOUR_FINDING)]) + assert code == 0 + assert runner.calls == STAGE_NAMES + assert read_state(run_dir)["status"] == "completed" + assert read_state(run_dir)["repair_pass_count"] == 0 + assert not record_path(run_dir).exists() + assert clean_clone_events(run_dir) == [CLEAN_CLONE_PASSED] + + +# -------------------------------------------------------------------------- +# The budget +# -------------------------------------------------------------------------- + + +@pytest.fixture +def exhausted_run(target_root, harness_root): + """Every verdict of this run carries a finding, so the bound is the only + thing that ends it.""" + return drive(target_root, harness_root, + [repairing(BEHAVIOUR_FINDING)] * (BUDGET + 1)) + + +def test_the_declared_budget_bounds_the_passes_per_run(exhausted_run): + code, runner, run_dir = exhausted_run + assert code == 0 + state = read_state(run_dir) + assert state["repair_pass_count"] == BUDGET + assert state["retry_count"] == 0 + assert events_of(run_dir).count(ROUTED) == BUDGET + assert state["status"] == "completed" + assert runner.calls.count(VERIFYING) == BUDGET + 1 + + +def test_no_stage_is_invoked_past_the_budget(exhausted_run): + """The verdict after the last pass carries findings and routes nowhere: + no stage runs again, and the run completes without a further invocation.""" + _, runner, _ = exhausted_run + counted = Counter(runner.calls) + assert max(counted.values()) == BUDGET + 1 + + +def test_the_spent_budgets_findings_are_recorded_in_the_event_stream( + exhausted_run, +): + """The bound is not silence: what the run declines to repair is named + where a developer meets it, once, on the verdict the budget refused.""" + _, _, run_dir = exhausted_run + recorded = events_of_kind(run_dir, RECORDED) + assert len(recorded) == 1 + assert BEHAVIOUR_FINDING["finding"] in recorded[0]["message"] + assert BEHAVIOUR_FINDING["location"] in recorded[0]["message"] + events = events_of(run_dir) + assert events.index(RECORDED) < events.index("story-completed") + assert schema_validator.validate(history_of(run_dir), HISTORY_SCHEMA) == [] + + +def test_each_pass_writes_its_own_numbered_record_and_the_refused_one_writes_none( + exhausted_run, +): + _, _, run_dir = exhausted_run + for number in range(1, BUDGET + 1): + assert record_of(run_dir, number)["pass"] == number, number + assert not record_path(run_dir, BUDGET + 1).exists() + + +def test_a_run_within_the_budget_records_nothing_as_refused(repaired_run): + """The control for the recorded event: a run that took one pass and whose + next verdict carried nothing has no refusal to record.""" + _, _, run_dir = repaired_run + assert events_of_kind(run_dir, RECORDED) == [] + + +# -------------------------------------------------------------------------- +# An undeclared category escalates +# -------------------------------------------------------------------------- + + +@pytest.fixture +def unknown_category_run(target_root, harness_root): + return drive(target_root, harness_root, [repairing(UNKNOWN_FINDING)]) + + +def test_a_finding_naming_an_undeclared_category_escalates_the_run( + unknown_category_run, +): + code, runner, run_dir = unknown_category_run + assert code == 2 + assert runner.calls == STAGE_NAMES + assert read_state(run_dir)["status"] == "escalated" + assert not (run_dir / "completion-report.md").exists() + + +def test_the_escalation_names_the_unknown_category_and_the_declared_ones( + unknown_category_run, +): + _, _, run_dir = unknown_category_run + entry = history_of(run_dir)[-1] + assert entry["event"] == "escalated" + assert UNDECLARED_CATEGORY in entry["message"] + for category in ROUTES: + assert category in entry["message"], category + assert UNDECLARED_CATEGORY in (run_dir / "escalation-summary.md").read_text( + encoding="utf-8") + + +def test_that_escalation_spends_no_retry_budget_and_archives_nothing( + unknown_category_run, +): + _, _, run_dir = unknown_category_run + state = read_state(run_dir) + assert state["retry_count"] == 0 + assert state["repair_pass_count"] == 0 + assert not (run_dir / "attempts").exists() + assert not (run_dir / "retry-history.json").exists() + assert not record_path(run_dir).exists() + + +def test_the_escalation_comes_after_the_clean_clone_check_passed( + unknown_category_run, +): + """The block sits below the check, so even the refusal is made on a + verified tree: the check ran and passed before the category was looked + up.""" + _, _, run_dir = unknown_category_run + events = events_of(run_dir) + assert clean_clone_events(run_dir) == [CLEAN_CLONE_PASSED] + assert events.index(CLEAN_CLONE_PASSED) < events.index("escalated") + + +def test_a_finding_naming_a_declared_category_routes_instead(repaired_run): + code, _, run_dir = repaired_run + assert code == 0 + assert "escalated" not in events_of(run_dir) + + +def test_one_undeclared_category_among_declared_ones_still_escalates( + target_root, harness_root, +): + """A verdict that could have been routed on its other finding is refused + rather than partially obeyed.""" + code, _, run_dir = drive( + target_root, harness_root, + [repairing(BEHAVIOUR_FINDING, UNKNOWN_FINDING)]) + assert code == 2 + assert UNDECLARED_CATEGORY in history_of(run_dir)[-1]["message"] + assert ROUTED not in events_of(run_dir) + assert read_state(run_dir)["repair_pass_count"] == 0 + + +# -------------------------------------------------------------------------- +# The grant: the paths the findings name, the stage they resolve to, the +# attempt of the pass +# -------------------------------------------------------------------------- + + +@pytest.fixture +def granted_run(governed_target, harness_root): + """A repair pass to the writing stage on which it edits the file the + finding named and one no finding named.""" + return drive(governed_target, harness_root, + [repairing(BEHAVIOUR_FINDING), PASS], + [None, the_named_file_and_another]) + + +def test_the_granted_edit_lands_and_the_other_is_undone(granted_run): + """The named file holds the pass's words; the other holds what the stage + found, its words are under the discarded directory, and the revert record + names it alone.""" + code, _, run_dir = granted_run + assert code == 0 + assert content(run_dir, NOTES) == NOTES_REPAIRED + assert content(run_dir, OTHER_NOTES) == OTHER_NOTES_AT_HEAD + record = read_json(run_dir / REVERT_ARTIFACT) + assert record["paths"] == [OTHER_NOTES] + assert record["permitted"] is False + (reverted,) = events_of_kind(run_dir, REVERTED_EVENT, WRITING) + # The message names the ungranted file and not the granted one, matched + # as whole tokens because the granted name is a substring of the other. + assert paths_named_in(reverted["message"], (NOTES, OTHER_NOTES)) == {OTHER_NOTES} + # Control for the absence half: the same matcher does see the granted + # file when a message names it beside the other, so the set above being + # short of NOTES.md is not the matcher failing to look. + assert paths_named_in( + f"the suite passes with {NOTES} and {OTHER_NOTES} reverted", + (NOTES, OTHER_NOTES)) == {NOTES, OTHER_NOTES} + assert (run_dir / CHECKED["discarded"] / OTHER_NOTES).read_text( + encoding="utf-8") == OTHER_NOTES_REWORDED + + +def test_the_grant_is_an_event_before_the_checks(granted_run): + _, _, run_dir = granted_run + kinds = events_of(run_dir) + (grant,) = events_of_kind(run_dir, GRANT_EVENT) + assert grant["stage"] == WRITING + assert NOTES in grant["message"] + assert WRITING in grant["message"] + assert kinds.index(GRANT_EVENT) < kinds.index(REVERTED_EVENT) + assert f"may change {NOTES}" in (run_dir / "events.log").read_text( + encoding="utf-8") + assert schema_validator.validate(history_of(run_dir), HISTORY_SCHEMA) == [] + + +def test_a_first_entry_before_the_pass_is_granted_nothing( + governed_target, harness_root, +): + """The control for the grant: the writing stage's first invocation, + before any pass, edits the same file and is decided on it.""" + code, _, run_dir = drive(governed_target, harness_root, [PASS], + [the_named_file]) + assert code == 0 + record = read_json(run_dir / REVERT_ARTIFACT) + assert record["paths"] == [NOTES] + assert record["permitted"] is False + assert content(run_dir, NOTES) == NOTES_AT_HEAD + assert events_of_kind(run_dir, GRANT_EVENT) == [] + + +def test_the_grant_reader_is_bounded_to_the_stage_and_the_attempt(granted_run): + _, _, run_dir = granted_run + state = story_coordinator.load_state(run_dir) + stages = WORKFLOW["stages"] + granted = story_coordinator.repair_pass_grants( + run_dir, stages, state, WRITING, 1) + assert granted == [NOTES] + assert story_coordinator.grant_covers(granted, NOTES) + assert not story_coordinator.grant_covers(granted, OTHER_NOTES) + # A later attempt of the same stage, and another stage on the same + # attempt, are each granted nothing. + assert story_coordinator.repair_pass_grants( + run_dir, stages, state, WRITING, 2) == [] + for other in STAGE_NAMES: + if other != WRITING: + assert story_coordinator.repair_pass_grants( + run_dir, stages, state, other, 1) == [], other + + +def test_each_entered_stage_is_granted_only_its_own_findings_paths(tmp_path): + """Two findings resolving to two stages in one record: each stage is + granted the path of the finding that resolved to it and not the other's. + Against a record written the way the coordinator writes one.""" + run_dir = tmp_path / "run" + run_dir.mkdir() + routing = story_coordinator.repair_destination( + [RECORD_FINDING, CHECKS_FINDING], ROUTES, STAGE_NAMES) + write_json(record_path(run_dir), { + "pass": 1, "attempt": 1, "stage": routing.stage, + "findings": routing.resolved, + "statement": story_coordinator.repair_pass_statement(routing.stage), + }) + state = story_coordinator.RunState(story_id="story-001", + branch="story/story-001") + state.repair_pass_count = 1 + stages = WORKFLOW["stages"] + + assert story_coordinator.repair_pass_grants( + run_dir, stages, state, destination_of(RECORD_CATEGORY), 1) \ + == [RECORD_FINDING["path"]] + assert story_coordinator.repair_pass_grants( + run_dir, stages, state, destination_of(CHECKS_CATEGORY), 1) \ + == [CHECKS_FINDING["path"]] + assert story_coordinator.repair_pass_grants( + run_dir, stages, state, destination_of(BEHAVIOUR_CATEGORY), 1) == [] + # The control: a state that has taken no pass reads the same record as + # granting nothing, so the grant is the pass's and not the file's. + state.repair_pass_count = 0 + assert story_coordinator.repair_pass_grants( + run_dir, stages, state, destination_of(RECORD_CATEGORY), 1) == [] + + +def test_the_grant_exempts_the_edit_from_both_checks_and_nothing_else(tmp_path): + """The two checks the grant is read by, asked directly with the writing + stage's own restrictions: with the grant the named file is neither a + governed edit nor an ownership violation, an ungranted governed file is + still governed, and with nothing granted the named file is governed too.""" + stage = WORKFLOW["stages"][0] + restrictions = story_coordinator.restrictions_on(stage) + run_dir = tmp_path / "run" + run_dir.mkdir() + write_json(run_dir / stage["changed_files"], + {"modified": [NOTES, OTHER_NOTES], "created": [], + "deleted": []}) + + granted = [BEHAVIOUR_FINDING["path"]] + assert story_coordinator.governed_edits( + run_dir, stage["changed_files"], restrictions, granted).paths \ + == (OTHER_NOTES,) + assert story_coordinator._ownership_violation( + run_dir, stage["changed_files"], restrictions, granted) is None + assert story_coordinator.governed_edits( + run_dir, stage["changed_files"], restrictions, []).paths \ + == (NOTES, OTHER_NOTES) + + write_json(run_dir / stage["changed_files"], + {"modified": [], "created": [NEW_FILE], "deleted": []}) + assert story_coordinator._ownership_violation( + run_dir, stage["changed_files"], restrictions, [NEW_FILE]) is None + violation = story_coordinator._ownership_violation( + run_dir, stage["changed_files"], restrictions, []) + assert violation is not None + assert violation.path == NEW_FILE + + +def test_a_granted_creation_passes_the_ownership_check_and_an_ungranted_one_stops_the_run( + governed_target, harness_root, tmp_path, +): + """The ownership check at run level: the same creation on a pass, once + granted by a finding naming it and once not.""" + other = second_target(governed_target, tmp_path) + granted = {**BEHAVIOUR_FINDING, "path": NEW_FILE, + "location": f"{NEW_FILE} - the whole file"} + code, _, run_dir = drive(governed_target, harness_root, + [repairing(granted), PASS], [None, a_new_file]) + assert code == 0 + assert read_state(run_dir)["status"] == "completed" + assert content(run_dir, NEW_FILE) == "created on a repair pass\n" + (grant,) = events_of_kind(run_dir, GRANT_EVENT) + assert NEW_FILE in grant["message"] + + code, _, run_dir = drive(other, harness_root, + [repairing(BEHAVIOUR_FINDING), PASS], + [None, a_new_file]) + assert code == 2 + assert read_state(run_dir)["status"] == "escalated" + assert NEW_FILE in history_of(run_dir)[-1]["message"] + + +@pytest.fixture +def retried_after_pass(governed_target, harness_root): + """A pass, then a failing verdict routing a retry that re-enters the + writing stage on attempt 2, where it edits the file the pass named.""" + return drive( + governed_target, harness_root, + [repairing(BEHAVIOUR_FINDING), failing_into(BEHAVIOUR_CATEGORY), PASS], + [None, the_named_file, the_named_file_again]) + + +def test_a_stage_re_entered_on_a_later_attempt_inherits_no_grant( + retried_after_pass, +): + """One grant, on the pass; none on the retry, whose edit to the same file + is decided and undone. Read in order off the history, so the one reverted + event is shown to be the retry's rather than the pass's.""" + code, runner, run_dir = retried_after_pass + assert code == 0, runner.calls + assert runner.calls.count(WRITING) == 3 + state = story_coordinator.load_state(run_dir) + assert state.retry_count == 1 + assert state.repair_pass_count == 1 + assert record_of(run_dir)["attempt"] == 1 + + ordered = [entry["event"] for entry in history_of(run_dir) + if entry.get("stage") == WRITING + and entry.get("event") in (GRANT_EVENT, REVERTED_EVENT, + PERMITTED_EVENT)] + assert ordered == [GRANT_EVENT, REVERTED_EVENT] + (reverted,) = events_of_kind(run_dir, REVERTED_EVENT, WRITING) + assert NOTES in reverted["message"] + assert (run_dir / CHECKED["discarded"] / NOTES).read_text( + encoding="utf-8") == NOTES_REWORDED_AGAIN + assert content(run_dir, NOTES) != NOTES_REWORDED_AGAIN + # The pass's attempt wrote no revert record; the retry's holds the live one. + archived = story_coordinator.attempt_dir(run_dir, 1) + assert archived.is_dir() + assert not (archived / REVERT_ARTIFACT).exists() + assert (run_dir / REVERT_ARTIFACT).is_file() + + +# -------------------------------------------------------------------------- +# Neither a stage name nor a category nor an artifact name lives in the source +# -------------------------------------------------------------------------- + + +def test_no_name_this_mechanism_routes_on_is_written_in_orchestration_source(): + """Every name comes off the loaded definition. The control is the same + names being present in the workflow this module built.""" + definition = json.dumps(WORKFLOW) + names = [ARTIFACT, *ROUTES, + *(n for n in STAGE_NAMES if n != conftest.VERIFYING_STAGE)] + for name in names: + assert name not in COORDINATOR_SOURCE, name + assert name in definition, name + + +def test_the_artifact_name_travels_from_the_declaration_to_the_run_directory( + target_root, tmp_path, +): + renamed = "a-differently-named-repair-record.json" + assert renamed not in COORDINATOR_SOURCE + harness = probe_harness( + tmp_path, target_root, "renamed-repair-pass", + lambda stage: stage.__setitem__( + "repair_pass", {**stage["repair_pass"], "result": renamed})) + + code, _, run_dir = drive(target_root, harness, + [repairing(BEHAVIOUR_FINDING), PASS]) + + assert code == 0 + assert record_path(run_dir, 1, renamed).is_file() + assert not record_path(run_dir, 1, ARTIFACT).exists() + + +def test_the_budget_comes_off_the_declaration_too(target_root, tmp_path): + """A budget of one under a definition that otherwise declares two: one + pass, and the second verdict's findings recorded rather than routed.""" + harness = probe_harness( + tmp_path, target_root, "one-repair-pass", + lambda stage: stage.__setitem__( + "repair_pass", {**stage["repair_pass"], "budget": 1})) + + code, runner, run_dir = drive( + target_root, harness, [repairing(BEHAVIOUR_FINDING)] * 2) + + assert code == 0 + assert events_of(run_dir).count(ROUTED) == 1 + assert events_of(run_dir).count(RECORDED) == 1 + assert read_state(run_dir)["repair_pass_count"] == 1 + assert runner.calls.count(VERIFYING) == 2 + + +# -------------------------------------------------------------------------- +# Pre-flight: a declaration that cannot be spent or cannot route refuses the run +# -------------------------------------------------------------------------- + + +def created_nothing(target_root: Path, branch: str) -> list[str]: + """What a refused run must not have left behind, as a list of violations, + so the same statement can be made of the run that is supposed to create + all of it — which is the control.""" + run_dir = run_dir_of(target_root) + problems = [] + if run_dir.exists(): + problems.append("a run directory exists") + if (run_dir / "state.json").exists(): + problems.append("state.json was written") + if (run_dir / "events.log").exists(): + problems.append("an event stream was written") + tree = conftest.run_root_for(target_root, "story-001") + standing = head_of(tree) if tree.is_dir() else head_of(target_root) + if standing != branch: + problems.append(f"the repository was left on {standing}") + return problems + + +def head_of(target_root: Path) -> str: + return subprocess.run( + ["git", "-C", str(target_root), "rev-parse", "--abbrev-ref", "HEAD"], + capture_output=True, text=True, check=True).stdout.strip() + + +def budgeted(budget): + """A mutation setting the declaration's budget to `budget`.""" + return lambda stage: stage.__setitem__( + "repair_pass", {**stage["repair_pass"], "budget": budget}) + + +def without_a_routing_table(stage: dict) -> None: + """A mutation leaving the declaration in place with no table to resolve + a category through.""" + stage.pop("on_failure") + + +@pytest.mark.parametrize("budget", [0, -1, "two", 1.5, True, None], + ids=["zero", "negative", "a word", "a fraction", + "a bool", "null"]) +def test_a_budget_that_is_not_a_positive_integer_is_refused_before_any_run_state( + target_root, tmp_path, capsys, budget, +): + before = head_of(target_root) + harness = probe_harness(tmp_path, target_root, "bad-repair-budget", + budgeted(budget)) + runner = Runner(target_root, [PASS]) + + code = story_coordinator.run_story( + "story-001", harness, target_root, runner) + + message = capsys.readouterr().err + assert code == 1 + assert "repair_pass" in message + assert "positive integer" in message + assert VERIFYING in message + assert runner.calls == [] + assert created_nothing(target_root, before) == [] + + +def test_a_declaration_on_a_stage_with_no_routing_table_is_refused_too( + target_root, tmp_path, capsys, +): + before = head_of(target_root) + harness = probe_harness(tmp_path, target_root, "no-routing-table", + without_a_routing_table) + runner = Runner(target_root, [PASS]) + + code = story_coordinator.run_story( + "story-001", harness, target_root, runner) + + message = capsys.readouterr().err + assert code == 1 + assert "retry_routing" in message + assert VERIFYING in message + assert runner.calls == [] + assert created_nothing(target_root, before) == [] + + +def test_the_same_harness_with_the_declaration_repaired_creates_all_of_it( + target_root, tmp_path, +): + """The control for `created_nothing`: the same probe-built harness with + the budget repaired creates the run directory, the state, the event + stream and the branch.""" + before = head_of(target_root) + harness = probe_harness(tmp_path, target_root, "repaired-budget", + budgeted(BUDGET)) + code, runner, _ = drive(target_root, harness, [PASS]) + + assert code == 0 + assert runner.calls == STAGE_NAMES + assert created_nothing(target_root, before) == [ + "a run directory exists", + "state.json was written", + "an event stream was written", + "the repository was left on story/story-001", + ] + + +def test_the_check_reports_both_problems_over_a_stage_list_that_is_not_a_workflow(): + """The same defects said directly, over stage lists that are not a real + workflow, so the accepted and refused declarations are pinned to the + declarations rather than to whichever a run meets first.""" + problems = story_coordinator.repair_pass_problems([ + {"name": "alpha"}, + {"name": "beta", "repair_pass": {"result": ARTIFACT, "budget": 0}}]) + assert len(problems) == 2 + assert all("beta" in problem for problem in problems) + assert any("positive integer" in problem for problem in problems) + assert any("retry_routing" in problem for problem in problems) + + accepted = [{"name": "alpha"}, + {"name": "beta", "repair_pass": {"result": ARTIFACT, "budget": 1}, + "on_failure": {"retry_routing": { + "a-category": {"stage": "alpha", "when": "whenever"}}}}] + assert story_coordinator.repair_pass_problems(accepted) == [] + + +def test_the_check_accepts_a_workflow_that_declares_no_repair_pass(): + """The switch, at pre-flight: a definition carrying no declaration is not + checked.""" + assert story_coordinator.repair_pass_problems( + [{"name": "alpha"}, {"name": "beta"}]) == [] + assert story_coordinator.repair_pass_problems(WORKFLOW["stages"]) == [] + + +# -------------------------------------------------------------------------- +# The schema +# -------------------------------------------------------------------------- + + +def test_the_verification_result_schema_declares_the_field_as_optional(): + """Presence is the signal: the field is not required, and each entry + requires exactly the five keys the record copies.""" + assert REPAIRABLE in VERDICT_SCHEMA["properties"] + assert REPAIRABLE not in VERDICT_SCHEMA.get("required", []) + item = VERDICT_SCHEMA["properties"][REPAIRABLE]["items"] + assert set(item["required"]) == REQUIRED_OF_A_FINDING + assert schema_validator.validate(repairing(BEHAVIOUR_FINDING), + VERDICT_SCHEMA) == [] + assert schema_validator.validate( + repairing(BEHAVIOUR_FINDING, CHECKS_FINDING), VERDICT_SCHEMA) == [] + assert schema_validator.validate(PASS, VERDICT_SCHEMA) == [] + + +@pytest.mark.parametrize("missing", sorted(REQUIRED_OF_A_FINDING)) +def test_an_entry_missing_any_required_field_is_refused(missing): + """Control: the schema check above must be able to fail, on each field.""" + broken = repairing({k: v for k, v in BEHAVIOUR_FINDING.items() + if k != missing}) + assert schema_validator.validate(broken, VERDICT_SCHEMA), missing + + +def test_the_schema_description_says_when_the_field_is_read(): + """The other place the verifier is told what the field is for: read only + on a passing verdict, after the clean-clone check, presence as the + signal, and a category checked with retry_target's strictness.""" + description = VERDICT_SCHEMA["properties"][REPAIRABLE]["description"] + assert "passing verdict" in description + assert "clean-clone check" in description + assert "presence" in description.lower() + assert "retry_routing" in description + assert "retry_target" in description + assert "escalates" in description + category = VERDICT_SCHEMA["properties"][REPAIRABLE]["items"][ + "properties"]["category"]["description"] + assert "retry_routing" in category + + +# -------------------------------------------------------------------------- +# The record reaches every stage prompt through the context assembler +# -------------------------------------------------------------------------- + + +def shipped_definitions() -> dict[str, dict]: + """Every workflow this repository ships, loaded the way a run loads it. + The subject here is what ships, which is why these read it.""" + return {name: conftest.shipped_workflow(REPO_ROOT, name) + for name in harness_config.workflow_names(REPO_ROOT)} + + +def shipped_prompts() -> list[str]: + """Every prompt template a shipped stage names, derived from the + definitions rather than listed.""" + return sorted({stage["prompt"] for definition in shipped_definitions().values() + for stage in definition["stages"]}) + + +def test_every_shipped_stage_prompt_carries_the_slot(): + """Derived from both shipped definitions: every stage of every shipped + workflow is told of a pass, because a pass runs forward through every + stage after the one it enters.""" + slot = f"{{{{{REPAIR_FIELD}}}}}" + prompts = shipped_prompts() + assert prompts + for prompt in prompts: + text = (REPO_ROOT / "prompts" / prompt).read_text(encoding="utf-8") + assert text.count(slot) == 1, prompt + assert f"{{{{{CORRECTION_FIELD}}}}}" in text, prompt + + +def build_shipped_context(target_root: Path, **passes) -> dict: + """A context built the way the coordinator builds one, against the + shipped workflow, with whichever pass records the caller supplies.""" + harness_root = REPO_ROOT + config = harness_config.load_config(target_root) + rules = harness_config.load_rules(harness_root) + story_text = (target_root / ".harness" / "stories" / "story-001.yaml" + ).read_text(encoding="utf-8") + run_dir = target_root / ".harness" / "runs" / "story-001" + run_dir.mkdir(parents=True, exist_ok=True) + return context_assembler.build_context( + story_text=story_text, + story=story_parser.parse(story_text, + schema_validator.load_schema("story")), + run_dir=run_dir, + target_root=target_root, + harness_root=harness_root, + config=config, + rules=rules, + workflow=conftest.shipped_workflow(REPO_ROOT, "story-workflow"), + retry_count=0, + **passes, + ) + + +def test_the_assembler_renders_the_record_into_the_slot_and_none_without_it( + target_root, +): + """The record text supplied is what the slot renders, in every shipped + template; omitted, the slot renders None and nothing else moves.""" + record = json.dumps({"pass": 1, "attempt": 1, "stage": "a-stage", + "findings": [BEHAVIOUR_FINDING], + "statement": "MARKER-STATEMENT"}) + with_record = build_shipped_context(target_root, repair_pass_result=record) + without = build_shipped_context(target_root) + assert with_record[REPAIR_FIELD] == record + assert without[REPAIR_FIELD] is None + + for prompt in shipped_prompts(): + template = context_assembler.load_template(REPO_ROOT, prompt) + rendered = context_assembler.render(template, with_record) + assert BEHAVIOUR_FINDING["finding"] in rendered, prompt + assert "MARKER-STATEMENT" in rendered, prompt + assert "{{" not in rendered, prompt + plain = context_assembler.render(template, without) + assert BEHAVIOUR_FINDING["finding"] not in plain, prompt + assert "{{" not in plain, prompt + + +def test_the_rendered_prompt_of_a_run_that_took_a_pass_carries_the_record_verbatim( + repaired_run, +): + """End to end, off the run directory: the record the coordinator wrote is + the text the entered stage's prompt carries after the slot's label.""" + _, _, run_dir = repaired_run + entry = destination_of(BEHAVIOUR_CATEGORY) + prompt = rendered_prompt(run_dir, entry) + assert record_path(run_dir).read_text(encoding="utf-8") in prompt + assert f"{REPAIR_FIELD}:\nNone" not in prompt + + +def test_the_rendered_prompt_of_a_run_that_took_none_renders_none( + unrepaired_run, +): + _, _, run_dir = unrepaired_run + for stage in STAGE_NAMES: + assert f"{REPAIR_FIELD}:\nNone" in rendered_prompt(run_dir, stage), stage + + +# -------------------------------------------------------------------------- +# What this repository ships: the declaration and the bound +# -------------------------------------------------------------------------- + + +def shipped_verifier_prompts() -> list[str]: + """The prompt of the stage carrying the declaration, in each shipped + definition, so the bound is read where the verifier reads it.""" + prompts = [] + for definition in shipped_definitions().values(): + (stage,) = [stage for stage in definition["stages"] + if stage.get("repair_pass")] + prompts.append(stage["prompt"]) + return prompts + + +def flowed(text: str) -> str: + """`text` with its line wrapping removed, so a clause that straddles a + newline is still found.""" + return " ".join(text.split()) + + +def bounding_section(prompt: str) -> str: + """What the shipped verifier prompt says from the start of the paragraph + that first names the field to the next artifact it introduces. Bounded + at the paragraph rather than at the mention, because the sentence that + says what such a finding is precedes the sentence that names where it + goes.""" + text = (REPO_ROOT / "prompts" / prompt).read_text(encoding="utf-8") + assert REPAIRABLE in text, prompt + mention = text.index(REPAIRABLE) + start = text.rindex("\n\n", 0, mention) + end = text.index("retry-guidance.json", mention) + return flowed(text[start:end]) + + +@pytest.mark.parametrize("prompt", shipped_verifier_prompts()) +def test_each_shipped_verifier_prompt_bounds_what_may_go_in_the_field(prompt): + """What a verifier has to know before it records one: judged correct, too + small to fail the run, one stage in the file it names, not to the words + alone, the category spelled as the table declares it, routed after the + clean-clone check, and not a licence to revisit the work.""" + section = bounding_section(prompt) + assert "too small to fail the run" in section + assert "one stage" in section + assert "words alone" in section + assert "not to prose alone" in section + assert "category" in section + assert "spelled exactly as" in section + assert "clean-clone check" in section + assert "not a licence to revisit the work" in section + assert "escalates" in section + + +def test_the_bound_is_stated_beside_the_correctable_findings_one(): + """Beside, not instead: both shipped verifier prompts still carry the + correctable-findings section, and the correctable one still says words + and never behaviour.""" + for prompt in shipped_verifier_prompts(): + text = flowed((REPO_ROOT / "prompts" / prompt).read_text(encoding="utf-8")) + assert text.index(CORRECTABLE) < text.index(REPAIRABLE), prompt + assert "never behaviour" in text, prompt diff --git a/tests/test_shipped_workflow_is_valid.py b/tests/test_shipped_workflow_is_valid.py index ee5ea77..508f69f 100644 --- a/tests/test_shipped_workflow_is_valid.py +++ b/tests/test_shipped_workflow_is_valid.py @@ -1248,6 +1248,68 @@ def test_this_deployment_declares_a_clean_clone_check_and_a_revert_check(): assert declaration["baseline"], stage["name"] +#: The key a stage declares a repair pass under, the key beside its budget +#: that records the judgement, and the budget this deployment chose. Spelled +#: once here: the mechanism is tests/test_repair_pass.py's question against a +#: built definition, and what *this* deployment declares is asked here. +REPAIR_PASS_KEY = "repair_pass" +REPAIR_PASS_REASON_KEY = "budget" + REASON_SUFFIX +REPAIR_PASS_BUDGET = 2 + + +@pytest.mark.parametrize("name", sorted(shipped_definitions())) +def test_every_shipped_definition_declares_a_repair_pass_it_can_take(name): + """story-155: both shipped definitions declare the pass, on the one stage + that judges, with the budget this deployment chose and a reason beside it + that states the number. The declaration names no stage of its own — the + stage comes off the retry_routing table the same stage declares, which + the pre-flight requires and which is asserted here through it. + """ + definition = shipped_definitions()[name] + declaring = [stage for stage in definition["stages"] + if stage.get(REPAIR_PASS_KEY)] + assert len(declaring) == 1, [stage["name"] for stage in declaring] + (stage,) = declaring + declaration = stage[REPAIR_PASS_KEY] + + assert stage["name"] == conftest.VERIFYING_STAGE + assert story_coordinator.repair_pass_problems(definition["stages"]) == [], name + assert declaration["budget"] == REPAIR_PASS_BUDGET + assert "stage" not in declaration + assert declaration["result"].endswith(".json") + assert states_the_number(declaration[REPAIR_PASS_REASON_KEY], + declaration["budget"]), name + assert stage["on_failure"]["retry_routing"], name + # And it sits beside the correction pass rather than replacing it: the + # third route is added to the two, not substituted for one. + assert "correction_pass" in stage, name + + +@pytest.mark.parametrize("declaration, table", [ + ({"result": "r.json", "budget": 0}, True), + ({"result": "r.json", "budget": "two"}, True), + ({"result": "r.json", "budget": True}, True), + ({"result": "r.json", "budget": 1}, False), +], ids=["a budget of zero", "a budget that is a word", "a budget that is a bool", + "no routing table to resolve through"]) +def test_repair_pass_problems_reports_a_declaration_the_pre_flight_refuses( + declaration, table, +): + """The control for the clean sweep above, built rather than mutated from + what this repository deploys: a budget that is not a positive integer, + and a declaration on a stage with no table, are each reported.""" + routing = {"the-work": {"stage": StageRef(0), "when": "when it is missing"}} + built = build_workflow( + workflow_stage(), + workflow_stage(name=conftest.VERIFYING_STAGE, + repair_pass=declaration, + **({"retry_routing": routing} if table else {})), + name="a-refused-repair-pass-workflow") + problems = story_coordinator.repair_pass_problems(built["stages"]) + assert len(problems) == 1, problems + assert conftest.VERIFYING_STAGE in problems[0] + + #: The key a stage declares a pre-stage inspection under, and the key inside it #: that names the run-directory artifact the inspection writes. Spelled once #: here and derived from by everything below, so this module carries no second diff --git a/workflows/refactor-workflow.json b/workflows/refactor-workflow.json index 8dca539..88e25b7 100644 --- a/workflows/refactor-workflow.json +++ b/workflows/refactor-workflow.json @@ -71,6 +71,11 @@ "budget": 1, "stage": "documenter" }, + "repair_pass": { + "result": "repair-pass.json", + "budget": 2, + "budget_reason": "Two rather than one, on the correction pass's arithmetic: a pass fixes the findings a verification raises, and the verification after it can find one the first findings did not reach, and a separate story to carry a leftover finding costs more than a second pass in the run that found it. Bounded at all because a repair pass may change behaviour, which is why it carries the findings and no guidance, routes only after the clean-clone check on the routing verdict has passed, and returns to a verification that runs the check again on the repaired tree. The stage comes off the retry_routing table below rather than off this declaration, so a finding is fixed by the stage that owns its file. Nothing reads this key; it records the judgement where a reader of the definition meets the budget." + }, "on_failure": { "retry_routing": { "implementation": { diff --git a/workflows/story-workflow.json b/workflows/story-workflow.json index ae921a4..6bbb940 100644 --- a/workflows/story-workflow.json +++ b/workflows/story-workflow.json @@ -96,6 +96,11 @@ "budget_reason": "Two, raised from one after story-086. A pass corrects the locations its findings name, and the verification after it can find the same claim standing somewhere the first findings did not reach: story-086's budget went on an exclusivity claim in .harness/docs/ARCHITECTURE.md, and the next verification found the same claim in a test docstring with nothing left to spend, so it shipped recorded and uncorrected. One budget covers the findings a verification raises; the second covers what correcting them reveals. This is a ceiling on how many times the words may be revisited and not a licence to keep going: a pass changes prose and never behaviour, so neither pass can move what any test asserts. Nothing reads this key; it records the judgement where a reader of the definition meets the budget.", "stage": "documenter" }, + "repair_pass": { + "result": "repair-pass.json", + "budget": 2, + "budget_reason": "Two rather than one, on the correction pass's arithmetic: a pass fixes the findings a verification raises, and the verification after it can find one the first findings did not reach, and a separate story to carry a leftover finding costs more than a second pass in the run that found it. Bounded at all because a repair pass may change behaviour, which is why it carries the findings and no guidance, routes only after the clean-clone check on the routing verdict has passed, and returns to a verification that runs the check again on the repaired tree. The stage comes off the retry_routing table below rather than off this declaration, so a finding is fixed by the stage that owns its file. Nothing reads this key; it records the judgement where a reader of the definition meets the budget." + }, "on_failure": { "retry_routing": { "implementation": {