From d3542f23bb0763f856c85c80d1263a66ddfde5f4 Mon Sep 17 00:00:00 2001 From: "jerod.wilkerson" <30474318+jerodw@users.noreply.github.com> Date: Sat, 8 Aug 2026 14:42:03 -0600 Subject: [PATCH] story-020: Resume an escalated run, and commit its work when it escalates Implemented by the l5 harness story workflow. --- .harness/docs/ARCHITECTURE.md | 60 +- orchestration/story_coordinator.py | 447 +++++++- scripts/l5-run | 16 +- tests/test_coordinator_contract.py | 9 + tests/test_story_010_validation.py | 8 +- tests/test_story_012_validation.py | 10 +- tests/test_story_019_validation.py | 12 +- tests/test_story_020_validation.py | 1695 ++++++++++++++++++++++++++++ 8 files changed, 2236 insertions(+), 21 deletions(-) create mode 100644 tests/test_story_020_validation.py diff --git a/.harness/docs/ARCHITECTURE.md b/.harness/docs/ARCHITECTURE.md index 51610c1..683ffcd 100644 --- a/.harness/docs/ARCHITECTURE.md +++ b/.harness/docs/ARCHITECTURE.md @@ -68,7 +68,7 @@ The drift source that paragraph used to name is closed: `planner.md` no longer s ### Orchestration (`orchestration/`) -- `story_coordinator.py` — the Story Coordinator. Loads the workflow definition, story artifact, and rules; creates the story branch and run directory; loops: determine stage → assemble context → render prompt → invoke agent → save artifacts → update state → route (advance, retry, or escalate). Post-stage checks run in a fixed order: required artifacts present → declared artifacts match their schemas → changed-files record clear of blocked paths → stage output ownership → the revert check. Schema validation sits in the middle deliberately, so a malformed `changed-files.json` escalates with a validation error naming the field rather than raising out of the blocked-paths check that reads the same file. Ownership runs last, on the same record, after that record is known to be well-formed and clear of blocked paths. `_ownership_violation` returns a frozen `OwnershipViolation(path, prefix)`, and the escalation reason names stage, path, and prefix in both `events.log` and `escalation-summary.md`. `_granted_prefixes` subtracts the story's grants from the enforced list at check time, and each applied grant is appended to `events.log` as `stage exception applied: may create `, so routing stays reconstructable from the log alone. The revert check sits immediately after, in the same block, reusing that already-narrowed `enforced` list rather than recomputing it: `governed_edits(run_dir, record_name, prefixes)` returns a frozen `GovernedEdits(paths, prefixes)` holding the sorted `modified` and `deleted` entries under any of them plus the prefixes that matched, and it names no stage, no prefix and no artifact. When `paths` is empty nothing at all happens — no clone, no suite, no artifact — because a check that can say nothing should not run. Otherwise `revert_check(run_dir, target_root, config, artifact, paths, baseline)` is shaped exactly like `clean_clone_check`: `tempfile.mkdtemp` scratch, the shared `run_clean_clone` with `revert` set to the governed paths, `shutil.rmtree` in a `finally` whatever the result, and `RevertCheckResult.as_record()` written under the declared artifact name. `permitted` is `result.exit_code != 0`. Two conditions stop it from running and neither permits: a stage that declares the check with no baseline captured (decided *before* any clone is attempted, so the reason names the missing directory rather than surfacing as a generic clone failure) and a clone that cannot be built. A check that did not run escalates naming the reason; `permitted` false escalates naming the stage, the prefixes and the paths; `permitted` true appends `_revert_check_permitted` and falls through to the existing advance. Both escalations go through `_escalate`, which does not touch `retry_count`. `_build_clone` and `run_clean_clone` gained a `revert` parameter for this and, in story-019, the `baseline` the reverted content is restored *from*; both default to reverting nothing, so the clean-clone check is untouched. When `revert` is non-empty the restore runs inside the clone *after* the working-tree diff is applied and the untracked files are copied and *before* `git add -A`, so the clone commits those paths as the stage found them while every other change is present: the baseline's copy is copied over the clone's for each governed path it holds, and each governed path it does **not** hold is deleted in the clone, because a path absent from the baseline did not exist when the stage started. Deleting rather than skipping is the point — skipping decides nothing and would report a permission the check never established, which is the assertion-that-cannot-fail failure mode `tests/test_baseline_honesty.py` exists to prevent. Naming paths with no baseline raises `RuntimeError` naming them. No code path in the check reverts to HEAD any longer. Pre-flight story reading is one function, `read_story(story_text)`: load `story.schema.json`, parse with it, validate against it, and return a frozen `StoryReading` carrying both the `parsed` story (`None` when parsing failed) and the `problems` list. It runs above the run-directory creation and the branch checkout, so a rejection is an exit-1 refusal leaving no run directory, no `state.json`, no log, and no new branch — and no agent invoked. It is called exactly once per run, and the parse it returns is the run's only reading of the artifact: `reading.parsed` is threaded into every `build_context` call and into `_complete`, which takes the completion-report title and commit-message subject from `story["story"]["title"]` rather than scanning lines. A missing title is a loud `KeyError`; the schema marks it required and the run cannot reach `_complete` without having validated. `read_story` stays schema conformance only. Whether a story's `stage_exceptions` mean anything against the workflow *this run loaded* is a separate question the schema cannot answer, so it is a separate function — `stage_exception_problems(story, stages)`, called from `run_story` beside `read_story` and above the run-directory creation. It refuses an exception naming a stage the workflow does not define, and one granting a prefix that stage was never restricted on: an exception that grants nothing is a planning error, not a harmless one. Matching is exact — a grant must name a prefix appearing verbatim in the stage's `may_not_create` list, so `create: tests` against a declared `tests/` refuses rather than silently granting part of it. Both refusals print through one extracted `_refuse(story_path, problems)`, so the refusal shape (exit 1, one message per problem, nothing created) is a single code path rather than two copies of one. The retry branch archives before it increments: `archive_attempt(run_dir, archivable_artifacts(stages), state.retry_count + 1)` copies the superseded attempt's artifacts under `attempts/attempt-N/` — see the archive decisions below. `append_event(run_dir, message, *, kind, stage, artifacts, duration_seconds, verifier_outcome, retry_decision, retry_reason)` is the run's single event write path: the prose message stays positional and is what the `events.log` line is built from, and the *same call* appends one structured entry to `execution-history.json`. `load_history(run_dir)` is the read side, called only by `append_event` for the next sequence number. `run_story` captures `stage_started_at = time.monotonic()` at the stage-started event and reads it through a local `elapsed()` at every event that ends a stage, so a completed stage's entry carries a duration the log only made derivable; `_escalate` forwards whatever structured fields an escalation has and tags its entry `escalated`. The clean-clone check is the last thing the verifier branch does on a passing verdict: `clean_clone_check(run_dir, target_root, config, artifact)` builds a scratch clone with `tempfile.mkdtemp`, runs `run_clean_clone`, removes the scratch directory in a `finally` whatever the result, and writes the returned `CleanCloneResult.as_record()` to the run directory under the declared artifact name. `_build_clone` does `git clone --no-hardlinks` from the target's filesystem path, applies the target's tracked edits as `git diff --binary HEAD` piped to `git apply`, copies the untracked-but-not-ignored files from `git ls-files --others --exclude-standard`, then commits inside the clone; the target repository is only read. `_link_interpreter_roots` links the top-level directory of each configured interpreter path into the clone and appends those names to the clone's `.git/info/exclude`, because a virtualenv is gitignored and therefore absent from a fresh clone, and a `.gitignore` entry for a directory does not cover a symlink standing in its place. Zero exit appends `_clean_clone_passed` and falls through to the existing advance; non-zero takes the retry path the verification-failed branch already takes — `archive_attempt` above the increment, then increment, save, `_clean_clone_failed`, and `index = stage_names.index(stage["on_failure"]["retry_stage"])` — or the existing escalation path at the ceiling, with `_clean_clone_failures` collapsing the output's `FAILED` lines into the one-line reason. Both events are module-level helpers rather than inline calls, for a reason worth keeping: `tests/test_story_011_validation.py` proves its own non-vacuity by deleting the first `retry_decision="retry",` line at the verification-failed branch's indentation, and an inline clean-clone branch nests deeper and sits earlier in the file, so its line *contains* that indented text and the mutation lands there instead of where it was aimed. +- `story_coordinator.py` — the Story Coordinator. Loads the workflow definition, story artifact, and rules; creates the story branch and run directory *or resumes an existing one*; loops: determine stage → assemble context → render prompt → invoke agent → save artifacts → update state → route (advance, retry, or escalate). `run_story` takes an optional keyword-only `start_stage` overriding where execution enters — the recorded stage on a resume, `stage_names[0]` on a fresh run. It is named `start_stage` rather than `stage` because `stage` is the loop's name for the stage being executed. A `start_stage` the loaded workflow does not define is refused above everything else, in the same shape as the other pre-flight refusals: exit 1, one message naming the stages the workflow does define, nothing created and no agent invoked. See "Resuming a run" below for the resume branch, the escalation commit and the guard. Post-stage checks run in a fixed order: required artifacts present → declared artifacts match their schemas → changed-files record clear of blocked paths → stage output ownership → the revert check. Schema validation sits in the middle deliberately, so a malformed `changed-files.json` escalates with a validation error naming the field rather than raising out of the blocked-paths check that reads the same file. Ownership runs last, on the same record, after that record is known to be well-formed and clear of blocked paths. `_ownership_violation` returns a frozen `OwnershipViolation(path, prefix)`, and the escalation reason names stage, path, and prefix in both `events.log` and `escalation-summary.md`. `_granted_prefixes` subtracts the story's grants from the enforced list at check time, and each applied grant is appended to `events.log` as `stage exception applied: may create `, so routing stays reconstructable from the log alone. The revert check sits immediately after, in the same block, reusing that already-narrowed `enforced` list rather than recomputing it: `governed_edits(run_dir, record_name, prefixes)` returns a frozen `GovernedEdits(paths, prefixes)` holding the sorted `modified` and `deleted` entries under any of them plus the prefixes that matched, and it names no stage, no prefix and no artifact. When `paths` is empty nothing at all happens — no clone, no suite, no artifact — because a check that can say nothing should not run. Otherwise `revert_check(run_dir, target_root, config, artifact, paths, baseline)` is shaped exactly like `clean_clone_check`: `tempfile.mkdtemp` scratch, the shared `run_clean_clone` with `revert` set to the governed paths, `shutil.rmtree` in a `finally` whatever the result, and `RevertCheckResult.as_record()` written under the declared artifact name. `permitted` is `result.exit_code != 0`. Two conditions stop it from running and neither permits: a stage that declares the check with no baseline captured (decided *before* any clone is attempted, so the reason names the missing directory rather than surfacing as a generic clone failure) and a clone that cannot be built. A check that did not run escalates naming the reason; `permitted` false escalates naming the stage, the prefixes and the paths; `permitted` true appends `_revert_check_permitted` and falls through to the existing advance. Both escalations go through `_escalate`, which does not touch `retry_count`. Since story-020 `_escalate` takes `target_root` and `harness_root` as required keywords — it commits and it records the harness revision — so every escalation site forwards both; a new escalation that forgets them is a `TypeError` rather than a run that quietly leaves its work uncommitted. `_build_clone` and `run_clean_clone` gained a `revert` parameter for this and, in story-019, the `baseline` the reverted content is restored *from*; both default to reverting nothing, so the clean-clone check is untouched. When `revert` is non-empty the restore runs inside the clone *after* the working-tree diff is applied and the untracked files are copied and *before* `git add -A`, so the clone commits those paths as the stage found them while every other change is present: the baseline's copy is copied over the clone's for each governed path it holds, and each governed path it does **not** hold is deleted in the clone, because a path absent from the baseline did not exist when the stage started. Deleting rather than skipping is the point — skipping decides nothing and would report a permission the check never established, which is the assertion-that-cannot-fail failure mode `tests/test_baseline_honesty.py` exists to prevent. Naming paths with no baseline raises `RuntimeError` naming them. No code path in the check reverts to HEAD any longer. Pre-flight story reading is one function, `read_story(story_text)`: load `story.schema.json`, parse with it, validate against it, and return a frozen `StoryReading` carrying both the `parsed` story (`None` when parsing failed) and the `problems` list. It runs above the run-directory creation and the branch checkout, so a rejection is an exit-1 refusal leaving no run directory, no `state.json`, no log, and no new branch — and no agent invoked. It is called exactly once per run, and the parse it returns is the run's only reading of the artifact: `reading.parsed` is threaded into every `build_context` call and into `_complete`, which takes the completion-report title and commit-message subject from `story["story"]["title"]` rather than scanning lines. A missing title is a loud `KeyError`; the schema marks it required and the run cannot reach `_complete` without having validated. `read_story` stays schema conformance only. Whether a story's `stage_exceptions` mean anything against the workflow *this run loaded* is a separate question the schema cannot answer, so it is a separate function — `stage_exception_problems(story, stages)`, called from `run_story` beside `read_story` and above the run-directory creation. It refuses an exception naming a stage the workflow does not define, and one granting a prefix that stage was never restricted on: an exception that grants nothing is a planning error, not a harmless one. Matching is exact — a grant must name a prefix appearing verbatim in the stage's `may_not_create` list, so `create: tests` against a declared `tests/` refuses rather than silently granting part of it. Both refusals print through one extracted `_refuse(story_path, problems)`, so the refusal shape (exit 1, one message per problem, nothing created) is a single code path rather than two copies of one. The retry branch archives before it increments: `archive_attempt(run_dir, archivable_artifacts(stages), state.retry_count + 1)` copies the superseded attempt's artifacts under `attempts/attempt-N/` — see the archive decisions below. `append_event(run_dir, message, *, kind, stage, artifacts, duration_seconds, verifier_outcome, retry_decision, retry_reason)` is the run's single event write path: the prose message stays positional and is what the `events.log` line is built from, and the *same call* appends one structured entry to `execution-history.json`. `load_history(run_dir)` is the read side, called only by `append_event` for the next sequence number. `run_story` captures `stage_started_at = time.monotonic()` at the stage-started event and reads it through a local `elapsed()` at every event that ends a stage, so a completed stage's entry carries a duration the log only made derivable; `_escalate` forwards whatever structured fields an escalation has and tags its entry `escalated`. The clean-clone check is the last thing the verifier branch does on a passing verdict: `clean_clone_check(run_dir, target_root, config, artifact)` builds a scratch clone with `tempfile.mkdtemp`, runs `run_clean_clone`, removes the scratch directory in a `finally` whatever the result, and writes the returned `CleanCloneResult.as_record()` to the run directory under the declared artifact name. `_build_clone` does `git clone --no-hardlinks` from the target's filesystem path, applies the target's tracked edits as `git diff --binary HEAD` piped to `git apply`, copies the untracked-but-not-ignored files from `git ls-files --others --exclude-standard`, then commits inside the clone; the target repository is only read. `_link_interpreter_roots` links the top-level directory of each configured interpreter path into the clone and appends those names to the clone's `.git/info/exclude`, because a virtualenv is gitignored and therefore absent from a fresh clone, and a `.gitignore` entry for a directory does not cover a symlink standing in its place. Zero exit appends `_clean_clone_passed` and falls through to the existing advance; non-zero takes the retry path the verification-failed branch already takes — `archive_attempt` above the increment, then increment, save, `_clean_clone_failed`, and `index = stage_names.index(stage["on_failure"]["retry_stage"])` — or the existing escalation path at the ceiling, with `_clean_clone_failures` collapsing the output's `FAILED` lines into the one-line reason. Both events are module-level helpers rather than inline calls, for a reason worth keeping: `tests/test_story_011_validation.py` proves its own non-vacuity by deleting the first `retry_decision="retry",` line at the verification-failed branch's indentation, and an inline clean-clone branch nests deeper and sits earlier in the file, so its line *contains* that indented text and the mutation lands there instead of where it was aimed. - `story_parser.py` — lexer plus schema-directed interpreter for the story artifact. **The story dialect is not YAML**; see the module docstring before reaching for `yaml.safe_load`, which reads committed artifacts differently and wrongly. The lexer produces line/indent/content records, drops blank lines and full-line comments, consumes a `key: |` block scalar body whole (so blank and `#`-shaped lines *inside* it survive), and rejects tab indentation. The interpreter dispatches on the schema node's `type`, consulting structure only where the schema is silent. Under `items.type == "string"` a `- ` item is the verbatim remainder of its line, colons included; under `items.type == "object"` the same syntax parses into key/value pairs. Scalars are never coerced — every value is a `str`. A single `StoryParseError` carries line, expectation, and finding, rendering as `line 12: expected …, found …`. - `schema_validator.py` — `schemas_dir`, `load_schema`, `shipped_schemas`, `unsupported_keywords`, and `validate(instance, schema) -> list[str]`. `shipped_schemas(harness_root=None) -> tuple[str, ...]` reads `schemas/manifest.json` through `schemas_dir`, so the override behaves identically to `load_schema`'s, and raises `ValueError` on anything short of a well-formed non-empty list of strings. A deliberately small JSON Schema subset — `type`, `required`, `properties`, `items`, `enum` — because the harness is standard library only. `validate` walks the whole schema first and raises `ValueError` if any keyword outside that subset appears anywhere in it, so a schema can never claim a constraint the validator silently drops. Errors carry a tracked JSON path, the expectation, and the found value: `$.blocking_issues[0].severity: expected one of ["high", "medium", "low"], found string ("critical")`. - `context_assembler.py` — builds each stage's runtime context from the story artifact, prior stage artifacts, retry state, and architecture documents, and renders it into the prompt template. `build_context` takes both the raw `story_text` and the required keyword-only `story` (the parsed artifact from `read_story`); it never reads the artifact itself. `{{story}}` is `story_text` verbatim, and `{{acceptance_criteria}}` comes from the parsed list via `_dashed_lines`, which renders one `- `-prefixed criterion per line and returns `None` for an absent or empty list. `{{stage_exceptions}}` follows the same convention through `_exception_lines`: one dash-prefixed line per grant naming the stage, the granted path, and the reason, `None` when the story declares none. `render()` is single-pass: `re.sub` does not re-scan substituted text, so a placeholder injected by one substitution is not itself resolved. `build_context` therefore resolves the shared `prompts/harness-layer.md` partial as a **two-pass render** — it renders that partial (including the partial's own `{{blocked_paths}}` placeholder) against the assembled context first, then stores the already-resolved text as the `harness_layer` context value for injection into stage templates. When the partial is absent, `harness_layer` is left unset and renders as `None`. The schema placeholders come from `schema_context(harness_root) -> dict[str, str]`, a public function of the same module: it globs `harness_root/schemas/*.schema.json` and exposes each file's text under the stem with hyphens replaced by underscores plus `_schema` (`verification-result.schema.json` → `{{verification_result_schema}}`). `build_context` merges it with `update` at the point the inline loop used to run, before the two-pass render, so the values are available to any template. A new schema file becomes an injectable placeholder with no code change. The glob appears exactly once in the module because it has two callers: `build_context` for workflow stages, and `l5-plan` for the planner template, which no coordinator renders. `workflow_context(workflow, rules) -> dict[str, str | None]` sits beside `schema_context` for the same reason: it maps the loaded workflow's stage names to `{{workflow_stages}}`, each stage's `may_not_create` declarations to `{{stage_create_restrictions}}` (`" may not create files under "`, one line per pair), and the rules' `blocked_paths` to `{{blocked_paths}}`, all through the shared `_dashed_lines` helper — `build_context`'s own `blocked_paths` rendering goes through the same helper, so the harness-layer partial and the planner render the list identically. `_dashed_lines` returns `None` for an empty list, and `render()` maps `None` to the literal `None`, so the empty-list edge changes no rendered prompt. @@ -86,7 +86,7 @@ Headless agents cannot answer permission prompts, so `.harness/config.yaml` carr ### Scripts (`scripts/`) -Thin entry points only; no orchestration logic. `l5-init`, `l5-plan`, `l5-run`, `l5-assist`, `l5-status`. Each resolves HARNESS_ROOT from its own location, adds `orchestration/` to `sys.path`, and locates the target repository through `harness_config.find_target_root` — one shared walk-up to the nearest `.harness/config.yaml`, not a per-script copy — before delegating to its orchestration module. (`l5-init` is the exception by design: it is *given* the directory to initialize and has nothing to find.) +Thin entry points only; no orchestration logic. `l5-init`, `l5-plan`, `l5-run`, `l5-assist`, `l5-status`. `l5-run` takes ` [--stage ]`: it parses the option and passes it to `run_story` as `start_stage`, and that is the whole of its involvement — the validation of the stage name against the loaded workflow happens in orchestration, because only the coordinator has loaded the workflow. Each resolves HARNESS_ROOT from its own location, adds `orchestration/` to `sys.path`, and locates the target repository through `harness_config.find_target_root` — one shared walk-up to the nearest `.harness/config.yaml`, not a per-script copy — before delegating to its orchestration module. (`l5-init` is the exception by design: it is *given* the directory to initialize and has nothing to find.) `l5-plan` is the exception to "the coordinator renders prompts": the planner is not a workflow stage, so the script loads `prompts/planner.md` with `context_assembler.load_template`, renders it with `context_assembler.render`, and passes the rendered text to `--append-system-prompt`. The render context is `schema_context(HARNESS_ROOT)` merged with `workflow_context(workflow, rules)`: since story-009, `l5-plan` locates the target repository like every other run script (`find_target_root`, same no-config refusal, no session started), loads the target's config, loads the workflow the config names (default `story-workflow`) and the execution rules from HARNESS_ROOT, and injects the stage list, per-stage create restrictions, and blocked paths alongside the story schema. Requiring a target was not a loss — a planner that cannot see the project cannot list `.harness/stories/` to assign the next story number either. All lookup, loading, and rendering stays in orchestration; the script wires them together and stays a single `os.execvp` into an interactive session. `l5-assist` reads its template raw; sharing the render path with it is a later story. @@ -112,10 +112,49 @@ Thin entry points only; no orchestration logic. `l5-init`, `l5-plan`, `l5-run`, ↓ completed (completion-report.md) +An escalation is not the end of the run. `l5-run ` on a run whose +`state.json` says `escalated` **resumes** it at the recorded stage; only +`completed` still refuses. + + escalated ──l5-run [--stage ]──→ running, at the recorded stage + │ (or the one --stage names) + └──unchanged story, tree and harness──→ refused + +## Resuming a run + +Chapter 18 treats a crash and an escalation as the same problem, and its recovery pseudocode branches on both statuses identically: restore the artifacts, set `current_stage` from the recorded state, continue. `run_story` already did that for a `running` run; story-020 narrowed the already-ended refusal so it fires on `completed` alone and let an `escalated` run take the same path. Nothing was added to the resume mechanism — the artifacts and the state are already on disk, which is exactly what Chapter 18 names as the mechanism, and is why `.harness/requests/README.md`'s decision not to write checkpoints still stands after this story rather than being reopened by it. + +The resume branch does four things, in this order, before any state is written: evaluate the unchanged guard, set the stage, archive the interrupted attempt, then set `status` back to `running` and save. Order is the point — a guard that ran after the state was rewritten would be deciding about a run that no longer exists. + +**Nothing is reinitialized.** `retry_count` and `verification_iterations` key the rendered-prompt and verification-iteration filenames (`prompt--attempt-N.md`, `verification/iteration-N.json`), so a resume that reset them would write over the escalated attempt's evidence — the evidence the refusal it replaces existed to protect. Carrying them forward is what makes preserving state and preserving evidence the same act. + +**The interrupted attempt is archived before the resumed stage runs**, under the attempt number it was written with (`state.retry_count + 1`, the same expression the retry branch uses). `interrupted_attempt_artifacts(stages, attempt)` is `archivable_artifacts(stages)` plus that attempt's rendered prompt filenames, one per stage — the prompts are the addition a resume needs, because a run re-entering under the same attempt number re-renders over the prompt the interrupted stage was actually given. Both lists come off the loaded workflow; no stage name and no artifact name is written in orchestration. An archive directory that already exists is a refusal, not an overwrite (see the story-010 bullet below). + +**Resuming is inferred from the recorded status and from nothing else.** The guard adds only a refusal, for the one case where a resume is knowably pointless. `unchanged_since_escalation(state, story_text, target_root, harness_root)` makes three comparisons, each of which must be *establishable* before it says anything: the story artifact's SHA-256 against `state.story_digest`, the branch against `state.escalation_commit` with a clean `git status --porcelain`, and the harness's revision against `state.harness_revision`. It returns the evidence only when all three are establishable and identical, and an empty list otherwise — **anything it cannot establish counts as not-the-same**, so an absent digest, an escalation that committed nothing, an unreadable HEAD, or a harness root that is not a git repository produces no refusal rather than a false one. The refusal message names the escalation reason (read from `escalation-summary.md` by `escalation_reason`, for the message only — nothing routes on it) and says what to change: amend the story, change the code on the branch, or update the harness. Amending the story clears it. The digest informs that message; it never authorizes or triggers a resume, so an incidental edit to a story cannot silently restart a run. + +One resumed event is appended through the existing `append_event` path, naming the stage, so the routing stays reconstructable from the log alone and `events.log` and `execution-history.json` stay two renderings of one write. + +### The escalation commit + +Resume cannot recover what the harness did not preserve, which is why the commit is part of this story rather than a neighbour of it. `_complete` ended a successful run with `git add -A` and a commit; `_escalate` committed nothing, so the one terminal state a developer most needs to inspect, resume or hand to another agent was the one left in the working tree, surviving exactly until someone checked out another branch — a normal thing to do while deciding what to do about an escalation. story-018's escalation left seven files and 1174 insertions uncommitted on `story/story-018`, preserved by a hand-written commit (`e2f401d`) the harness should have made. + +An escalation now leaves **two** commits, and the second is not bookkeeping for its own sake: + +1. `commit_escalated_work(target_root, state, reason, run_dir=...)` commits the run's own record of the escalation — `state.json` and both renderings of the event stream — and returns its sha, which is what `state.escalation_commit` records. It returns `""` on a clean tree, and then nothing further is committed: an escalation with nothing to commit records no commit and is not an error. +2. `commit_escalated_tree`, invoked by the `_commits_the_tree_it_ends_on` decorator on `_escalate` after `_escalate` has written everything including the summary, runs `git add -A` and commits the work on top. That commit is the branch tip. + +Two commits rather than one because **a commit cannot carry its own sha**: the content is hashed into the identity being recorded. In a repository that tracks its run directory, a clean tree requires the committed `state.json` to equal the on-disk one, and the on-disk one records the sha of the commit containing it — a fixed point of the hash over its own tree. Splitting the record from the work is what lets `state.json` be both committed and carry a real sha. Both commits use `--allow-empty`, so the branch shape is the same in a repository that ignores its run directory as in one that tracks it, which is what lets one undo command be right in both. The decorator exists to put the work commit *last* without moving `_escalate`'s own writing: the escalation summary's construction and write stay byte for byte what they were, which the "do not change `escalation-summary.md`'s content" constraint required. + +Consequences worth knowing: `ESCALATION_UNDO_COMMAND` is `git reset --mixed HEAD~2`, named in the commit body, and `unchanged_since_escalation` compares the recorded commit against `HEAD~1` rather than `HEAD`, because the escalation commit is the tip's parent. + +The subject is `l5 escalated: stopped at `, built from `ESCALATION_COMMIT_MARKER`. `_complete`'s is `: `, so a subject leading with a marker cannot be read as a completion in `git log --oneline`, in a PR title, or by anyone scanning the branch. The body says outright that the commit is a holding place rather than a decision about the work, carries the escalation reason, and names the undo command. `_complete` is untouched — same message, same contents, same behaviour. + ## Run directory anatomy .harness/runs/story-001/ - state.json current stage, status, retry_count, branch + state.json current stage, status, retry_count, branch, + and the three resume fields: story_digest, + escalation_commit, harness_revision events.log append-only stage/retry/escalation events execution-history.json the same events, structured; one entry per log line implementation-summary.md @@ -146,11 +185,11 @@ Three artifacts describe a retry and none of them substitutes for another. `retr Four properties of it are load-bearing: - **The file set is `git ls-files --cached --others --exclude-standard`**, the same tracked-plus-untracked set `_build_clone` carries into a clone — not tracked files alone. A file an earlier stage of this run created and never committed (the coordinator commits once, at `_complete`) has no HEAD version, and capturing it is the whole reason this exists. -- **Capture once, reuse afterwards.** A directory already recorded for this stage and attempt is returned untouched. This is what stops a re-entered stage from snapshotting its own completed edits, which would make the revert a no-op, the suite pass, and the check report a permission it never established. The rule is stated here rather than inside the check because `.harness/requests/06-resume-continuity.md` inherits it rather than adding a second mechanism — answering "what did the tree look like before this stage ran" twice is how this repository has repeatedly ended up with one fact in two places. +- **Capture once, reuse afterwards.** A directory already recorded for this stage and attempt is returned untouched. This is what stops a re-entered stage from snapshotting its own completed edits, which would make the revert a no-op, the suite pass, and the check report a permission it never established. The rule is stated here rather than inside the check because resume inherits it rather than adding a second mechanism — answering "what did the tree look like before this stage ran" twice is how this repository has repeatedly ended up with one fact in two places. story-020 is that inheritance in practice and added nothing: a resumed run re-enters at the recorded stage with that stage's work already in the working tree, so a fresh capture there would snapshot exactly the edits the check is meant to revert, and the capture-once rule is the whole of what prevents it. - **The directory is created even when it captures nothing**, so its existence answers "was a baseline taken" and its absence is a distinct, reportable condition rather than an empty capture. A stage declaring the check with no baseline captured escalates naming that. - **It is evidence, never state.** Nothing in orchestration routes on it — no branch in `run_story` reads it to decide where execution goes; the revert check consumes it only to build a clone. It is absent from `state.json`, whose field set story-019 left unchanged. It carries **no schema and no `schemas/manifest.json` entry**, because it is a directory of file copies rather than a JSON artifact, and the inventory asserts exact set equality in both directions — an entry with no schema file behind it would fail it. -It is narrower than Chapter 18's **checkpoints**, and the difference is the reason this is not that. A checkpoint is a known-good execution boundary a run can be resumed from; `stage-baseline/` records content for a bounded set of paths so one check can ask one question, and records no resumable point. `.harness/requests/README.md`'s decision not to write checkpoints stands — this does not fulfil it, partially or otherwise. +It is narrower than Chapter 18's **checkpoints**, and the difference is the reason this is not that. A checkpoint is a known-good execution boundary a run can be resumed from; `stage-baseline/` records content for a bounded set of paths so one check can ask one question, and records no resumable point. `.harness/requests/README.md`'s decision not to write checkpoints stands — this does not fulfil it, partially or otherwise. story-020 shipped resume without one, which is the evidence that judgement was right: it resumes from `state.json` and the run-directory artifacts that already exist, and a checkpoint would have been a third record of where execution stands. ## Decisions and constraints @@ -175,7 +214,7 @@ It is narrower than Chapter 18's **checkpoints**, and the difference is the reas - The archive is evidence, never state. Nothing routes on anything under `attempts/`; `state.json` remains the coordinator's only routing source, and `archive_attempt`'s return value is discarded at the call site (it exists so a test can assert what was archived). `verification/iteration-N.json` and the root `verification-result.json` are untouched — the archive adds evidence rather than replacing the verifier's. - One archive point, not four. The copy happens in the verifier's retry branch of `run_story`, immediately **above** `state.retry_count += 1`, rather than as a branch in each stage's write path. That is the only place where "the root artifacts describe a superseded attempt" is known to be true, and it is why the directory is created at the first retry and never in advance. Placing it above the increment is what makes `state.retry_count + 1` — the same expression the rendered-prompt filename uses — name the attempt that just ended rather than the one about to start. - `archivable_artifacts(stages)` names no artifact and no stage: it takes the union of each stage's `outputs`, its `changed_files` record, and the keys of its `schemas` map, reading exactly the three places the coordinator already reads artifact names from, sorted for determinism. A workflow that declares a new stage artifact gets it archived with no change to `orchestration/story_coordinator.py`; `tests/test_story_010_validation.py` proves this with a workflow definition the repository does not ship. An artifact the attempt did not write is skipped rather than failing the archive, matching how `_schema_violation` skips an absent conditional artifact. As with the workflow-loaded blocked paths and ownership prefixes, the general rule holds: what the coordinator routes on or records comes off the loaded definition, not out of orchestration code. -- Open question story-010 leaves: `archive_attempt` uses `mkdir(exist_ok=True)` and `shutil.copy2`, so a resumed run whose `attempts/attempt-N/` already exists would overwrite it. No acceptance criterion covered that case and no test exercises it. It matters only for resume, which the harness does not yet support. +- Open question story-010 left, answered by story-020: `archive_attempt` uses `mkdir(exist_ok=True)` and `shutil.copy2`, so a resumed run whose `attempts/attempt-N/` already exists would overwrite it. The resume **refuses** rather than overwriting, naming the directory and saying to move or remove it — the archive is the evidence a resume exists to preserve, so writing over it is the one thing the feature must not do. `archive_attempt` itself is unchanged; the check sits in the resume branch, which is the only caller that can meet an existing directory (the retry branch creates attempt N for the first time). The directory name now has one home, `attempt_dir(run_dir, attempt)`, which both the archive and the resume derive it from, so `"attempts"` appears exactly once in the coordinator. - A retry record is appended wherever a retry is actually *taken*, which is not the same as "where a verification failed". story-012 was planned against one rerouting path and three escalation paths; story-014 had since added a second reroute, the clean-clone failure. The governing sentence is the task's — one entry each time a retry is taken — so both reroute paths append, and all three escalation paths (the two after a failed verification, and the clean-clone one at the ceiling) append nothing, because no retry follows them. Leaving the clean-clone reroute out would make the record quietly disagree with `state.retry_count`, which is the one number a reader would check it against. The call sits immediately after `archive_attempt` and **above** `state.retry_count += 1`, for the archive's own reason: `state.retry_count + 1` then names the attempt that just ended, matching `attempts/attempt-N/` and `prompt-implementer-attempt-N.md`. - **Presence of a conditional artifact at the run root is not evidence that this attempt wrote it.** Nothing clears `retry-guidance.json` between attempts — `context_assembler` injects it into the retried implementer's context from exactly there — so a reroute that writes no guidance (the clean-clone path, which follows a *passing* verdict) would inherit the previous attempt's if the record simply read the file. The test that holds is a before/after signature: `artifact_signatures(run_dir, artifacts)` takes `(st_mtime_ns, st_size)` per artifact, `run_story` snapshots it immediately before invoking the stage agent, and `artifacts_written_since(...)` returns those that appeared or changed. Two alternatives were rejected and are worth not re-proposing: deleting the guidance after archiving breaks the retried implementer's context, and gating on `verdict["status"]` hard-codes "guidance accompanies a failure" into orchestration where the snapshot asks the general question and names no artifact. Expect this shape for any future conditional artifact whose authorship, not existence, is the question. - `conditional_artifacts(stage)` — the keys of a stage's `schemas` map minus its `outputs` and its `changed_files` record — is how the coordinator locates the artifacts a stage *may* write but need not, without an artifact name appearing in orchestration code. For the verifier that set is exactly the retry guidance. Same rule as `archivable_artifacts`, blocked paths and ownership prefixes: what the coordinator reads comes off the loaded workflow definition. @@ -204,12 +243,19 @@ It is narrower than Chapter 18's **checkpoints**, and the difference is the reas - **The line the harness draws under `tests/` is not a path prefix, it is reverting.** Four story artifacts asserted in prose that the implementer's record lists nothing under `tests/`. The harness enforced something narrower — `may_not_create`, creation only — and never escalated on the difference, so the prose added no enforcement while being sometimes impossible to satisfy: a legitimate implementer change can break an existing test and the suite has to stay green. Every finding the sentence produced was a deviation from prose rather than a defect (story-011 had to add a schema to two inventories asserting exact set equality, story-013 could not carry the rule at all because deleting those inventories was its deliverable, story-012 shipped a schema that turned five story-013 assertions red and recorded the deviation as not clearable by a retry). Three separate causes, each removed by its own story, each followed by another, and nobody could enumerate the next. The distinction being reached for was never "must not touch `tests/`" but "must not author its own validation", and a path prefix conflates two acts: authoring coverage, which must not happen in the implementer, and keeping existing validation runnable, which is maintenance. Reverting separates them exactly and with no judgement — **maintenance is by definition the edit without which the suite fails** — so an edit under a governed prefix is permitted iff reverting it makes the suite fail. Do not reintroduce the prefix rule in prose beside the check. - The revert check is story-014's clone operation with the governed paths *restored to the state the stage found them in* instead of applied, and there is deliberately no second clone builder. `run_clean_clone` stays the single build-a-clone-and-run-the-suite path and both checks go through it. The added `revert` parameter defaults to reverting nothing, so the clean-clone check's artifact, events and routing are unchanged — which is the property to re-establish after any future edit to that path, because two clone builders would drift the same way two write paths for the run's history would. - **The baseline for "was this edit forced" is the stage, not HEAD.** story-017 shipped the check reverting with `git checkout HEAD -- <paths>` and named "a governed path with no HEAD version" among the cases it refuses on — the right stance for a clone it cannot build, and that stance stays. It misjudged the frequency: story-018 escalated on it on the first retry after the check shipped, and the shape is ordinary rather than exceptional. A clean-clone or verification failure is frequently a defect in a test written *this run*, the retry routes to the implementer, and the implementer edits that test — a file with no version at HEAD, because the coordinator commits once at `_complete`. Every such retry escalated, so the check blocked retries generally. story-019 replaced the baseline with the pre-stage tree state, which is the question the check was always asking: for a file that predates the story that state *is* its HEAD content and nothing changed; for a file the tester created earlier in the run it is what the tester left. The fix was deliberately **not** to skip paths lacking a HEAD version — skipping decides nothing and reports a permission the check never established. No code path in the check reverts to HEAD any longer, and no path is skipped for lacking a HEAD version. -- story-019 was not governed by the change it makes, for the third time in this pattern's history (story-007 with `may_not_create`, story-014 with `clean_clone`, story-017 with `revert_check`): the coordinator loads the workflow definition at run start, so the object-form declaration its implementer wrote was not in the definition its own run held, and its own revert check still reverted from HEAD. Expected, not a defect, and recovery is by hand because resume does not exist yet. The check could still run on that run because both files its implementer touched under `tests/` exist at HEAD. +- story-019 was not governed by the change it makes, for the third time in this pattern's history (story-007 with `may_not_create`, story-014 with `clean_clone`, story-017 with `revert_check`): the coordinator loads the workflow definition at run start, so the object-form declaration its implementer wrote was not in the definition its own run held, and its own revert check still reverted from HEAD. Expected, not a defect; recovery was by hand there because resume did not exist yet, and since story-020 the equivalent recovery is a resume. The check could still run on that run because both files its implementer touched under `tests/` exist at HEAD. - **The check decides on the whole set of governed paths in one run of the suite, and the limit is stated where the check is defined** — in `story_coordinator.py`'s module docstring and in `revert-check-result.schema.json`'s own `description`, following the narrowness `tests/test_baseline_honesty.py` states about itself. A set containing one forced repair is therefore permitted *in full*, added coverage in the other files of that set included, and a single file mixing a forced repair with added coverage is not caught at all. The record's `paths` field names exactly what was reverted, so the artifact reports what it checked rather than claiming discrimination it does not have. Per-file or per-hunk reversion was considered and is not the fix to reach for first: the negative-control case it would leave open — an assertion weakened as part of an otherwise-forced repair, where reverting still fails — is not closable by any granularity of reverting, and belongs to `prompts/verifier.md` and the verifier reading the diff, which it has done correctly in every recorded instance. - A revert-check refusal escalates immediately without incrementing `retry_count`, matching the ownership violation it sits beside and for the same reason: the stage did not fail at its work, it produced something that is not its to produce, and rerunning the same instructions would produce it again. The reason names the stage, the prefixes and the governed paths in both `events.log` and `escalation-summary.md`. A permission is not silent either — one `revert-check-permitted` event is appended, so a run shows *why* an implementer was allowed into a governed prefix rather than only that it was. - A check that cannot run escalates naming why; it never permits by default. Two conditions reach that state — a clone that genuinely cannot be built, and a stage that declares the check with no baseline captured — and `revert_check` records `ran: false` with the reason rather than treating either as evidence of maintenance — the same stance the clean-clone check takes toward an unresolvable configured interpreter. `permitted` is *absent* from the record in that case rather than `false`, because a check that could not run permitted nothing and refused nothing; the validator subset has no union keyword, so absence is the honest encoding, as it is for the optional history fields. - `revert-check-result.json` is evidence, never state. Nothing in orchestration reads it back — the routing is driven by the returned `RevertCheckResult`, exactly as `clean-clone-result.json` and `retry-history.json` and everything under `attempts/` route nothing. No stage name, no prefix and no artifact name introduced by this check appears in `orchestration/story_coordinator.py`; all three come off the loaded workflow definition and the story, per the standing rule. - **Making a bad requirement harmless is not the same as stopping it being written, and the two are separate stories.** story-017 makes the over-strict sentence inert: once reverting decides permission, a verification requirement restating the stricter rule has nothing to adjudicate, because the coordinator has already permitted or refused the edit before the verifier is asked. The offending sentence is composed fresh by the planner into each story and is stored nowhere, so there was nothing to delete. Catching it *at plan time*, when the artifact is written, is what `.harness/requests/11-plan-time-validation.md` proposes and is not done here. The one line added to `planner.md`'s `[Workflow facts]` is a courtesy toward that day, not the enforcement — a written rule in an injected document has already been tried in this repository and failed, which is what `tests/test_baseline_honesty.py` exists because of. - **Adding a stage declaration to the shipped workflow breaks the end-to-end tests of every check already declared on that stage, and the repair is to mirror the workflow, not to weaken the test.** `tests/test_story_007_validation.py`'s two ownership tests drive real `run_story` calls whose subject is that the ownership rule reads `created` alone; against the shipped workflow they now also pick up the new `revert_check` key, and their synthetic records name paths with no version at HEAD, so the revert check correctly escalates with *could not run* and the runs no longer complete. story-017 repaired them with an `ownership_only` helper built on that file's existing `mirror_harness`, which runs each against a copy of the workflow with the implementer's `revert_check` popped; every assertion and every record is unchanged and the docstring says why. That is the pattern to reuse: a test whose subject is one check is run against a workflow declaring only that check, so the next declaration does not silently retarget it. The general form is the story-013 ripple bullet above — a per-story test that goes red on landing gets repointed, never relaxed — and demonstrating the repair was *forced* is the same revert the check itself performs. +- **A fact `state.json` does not carry is added to `state.json`; it does not get a shadow file.** story-020 needed three facts a resume cannot derive — the story artifact as the run first read it (`story_digest`), the commit the escalation made (`escalation_commit`), and the harness revision at the moment it escalated (`harness_revision`) — and put all three on `RunState` rather than beside it. Each is defaulted, so `RunState(**json)` still loads a state file written before this story, and **empty means "not established" at every reader** rather than being a value to compare against. That single convention is what makes the resume guard's bias correct by construction instead of by three separate checks. The story added no new artifact at all: the run directory already holds every stage artifact and the attempt archive, and no second record of where execution stands was created. +- `story_digest(story_text)` is taken from the same text `read_story` was handed, so the digest and the run's one reading describe one artifact — the same "exactly one mechanism reads a story artifact" rule, applied to hashing it. It is recorded when the run's state is first created, and it feeds a refusal *message*. It does not authorize a resume, does not trigger one, and no line in `run_story` routes on it. +- **The resume guard refuses only on positive evidence of sameness.** Every comparison in `unchanged_since_escalation` must be establishable before it can contribute, and a comparison that cannot be made returns the empty list rather than a partial verdict. The bias is deliberate and one-directional: under-refusing costs a wasted run the developer asked for, over-refusing blocks a resume the developer needs, and only the second is a failure the harness inflicts. A harness root that is not a git repository is the case to keep working — it produces no refusal. +- **Neither terminal commit establishes that the tree it commits is the tree the run produced.** Both `_complete`'s and `_escalate`'s `git add -A` stage whatever the working tree holds, and the escalation does that on a tree that is by definition unfinished. story-020 **states** this limit rather than closing it — in the coordinator, where the commits are made, and in a test worded as a property of the coordinator rather than of one commit. `.harness/requests/commit-only-what-the-run-produced.md` is the story that closes it and lands after this one by decision, at which point that assertion is repointed rather than deleted. A stated limit a reader meets where the behaviour is, and a test that goes red when the statement stops being true, is the shape to reuse for any limit a story knowingly leaves open. +- **Do not exempt a resumed run from the future clean-tree pre-flight, and no rule about it is written here.** Which runs that check applies to is the clean-tree story's decision — a fresh run and a resume of an *escalated* run, whose tree this story leaves clean at escalation, but not a resume of a *crashed* run, whose dirty tree is that run's own unfinished work. Deciding it here by accident is the failure both requests were written to avoid. +- A narrowness in the guard, recorded rather than fixed: `unchanged_since_escalation` runs before `_checkout_story_branch`, so its `HEAD~1` comparison reads whatever branch the developer is standing on rather than `state.branch`. A developer who escalates, checks out `main` to think about it — the act this story exists to make safe — and then re-runs the story gets a resume rather than the refusal. Whether that is a miss depends on reading "the target tree" as the branch (the technical plan says "the branch's HEAD", which would make it a one-line change to `f"{state.branch}~1"`) or as the working tree (which after the checkout genuinely has changed). It is left as it is because the story's stated bias is toward under-refusing, and it is written down here so the next reader decides it deliberately. +- Landing story-020 turned three existing assertions red and each was **repointed, not relaxed** — the standing ripple rule. `tests/test_story_019_validation.py` asserted `state.json`'s field set exactly; its subject was that the *baseline* added no field, so it now asserts the story-019 fields are a subset and that no field names the baseline. `tests/test_story_012_validation.py` compared two runs' whole state; it now compares field by field excluding `escalation_commit`, because two copies of one repository escalate to different shas for reasons unrelated to routing, and the exclusion is a named set so a future field is included by default. `tests/test_story_010_validation.py` asserted `"attempts"` appears once in the coordinator and inside `archive_attempt`; the literal moved to `attempt_dir`, so it now asserts the literal is in the helper and that `archive_attempt` calls it — the guarantee is unchanged and now exact. `tests/test_coordinator_contract.py` is the deliberate edit rather than a ripple: the new fields and the fact that `escalated` is an *ending* status but no longer a final one are changes to the standing contract, which is what that file exists to make explicit. - Verification rules never change between retries; retries narrow scope, they do not restart the workflow. - Capacity exhaustion (rate limits) is a reason to wait, not to fail; budget ceilings are a reason to stop. diff --git a/orchestration/story_coordinator.py b/orchestration/story_coordinator.py index 7ad28dd..df895c6 100644 --- a/orchestration/story_coordinator.py +++ b/orchestration/story_coordinator.py @@ -17,6 +17,8 @@ class of edit, it does not audit one. """ from __future__ import annotations +import functools +import hashlib import json import re import shlex @@ -44,6 +46,19 @@ class RunState: retry_count: int = 0 verification_iterations: int = 0 artifacts: list[str] = field(default_factory=list) + # Everything below is defaulted so RunState(**json) still loads a state + # file written before these fields existed, and empty means "not + # established" everywhere it is read. + #: The story artifact as this run first read it, so a refusal can say + #: whether it has been amended since. It informs a message; it authorizes + #: nothing. + story_digest: str = "" + #: The commit _escalate made on the story branch, empty when there was + #: nothing to commit. Tells an escalation the harness committed from one a + #: developer committed from one where nothing was committed. + escalation_commit: str = "" + #: The harness revision at the moment the run escalated. + harness_revision: str = "" @dataclass(frozen=True) @@ -75,6 +90,18 @@ def read_story(story_text: str, harness_root: Path | None = None) -> StoryReadin return StoryReading(parsed, schema_validator.validate(parsed, schema)) +def story_digest(story_text: str) -> str: + """A digest of the story artifact exactly as the run was given it. + + Taken from the same text read_story was handed, so the digest and the + reading describe one artifact. It is recorded on state.json at run start + for one purpose: a refusal to resume can say whether the story has been + amended since the run escalated. It never authorizes or triggers a resume — + an incidental edit to a story must not silently restart a run. + """ + return hashlib.sha256(story_text.encode("utf-8")).hexdigest() + + def stage_exception_problems(story: dict, stages: list[dict]) -> list[str]: """Cross-check a story's stage exceptions against the loaded workflow. @@ -201,6 +228,18 @@ def _git(target_root: Path, *args: str) -> subprocess.CompletedProcess: ) +def _revision(root: Path, revision: str = "HEAD") -> str: + """The revision `root` is at, or "" when that cannot be established. + + Empty is the honest answer for a directory that is not a git repository, + and every reader treats it as not-established rather than as a value to + compare — which is what keeps the resume guard from refusing on evidence + it does not have. + """ + result = _git(root, "rev-parse", revision) + return result.stdout.strip() if result.returncode == 0 else "" + + def _checkout_story_branch(target_root: Path, branch: str) -> None: exists = _git(target_root, "rev-parse", "--verify", branch).returncode == 0 args = ["checkout", branch] if exists else ["checkout", "-b", branch] @@ -292,6 +331,16 @@ def archivable_artifacts(stages: list[dict]) -> list[str]: return sorted(names) +def attempt_dir(run_dir: Path, attempt: int) -> Path: + """Where one attempt's superseded artifacts are kept. + + The one place the archive directory is named. Both readers derive it from + here: the archive that writes it, and the resume that refuses when it + already exists rather than writing over the evidence in it. + """ + return run_dir / "attempts" / f"attempt-{attempt}" + + def archive_attempt(run_dir: Path, artifacts: list[str], attempt: int) -> list[str]: """Copy the superseded attempt's artifacts into attempts/attempt-N/. @@ -302,7 +351,7 @@ def archive_attempt(run_dir: Path, artifacts: list[str], attempt: int) -> list[s conditional artifact is skipped by _schema_violation. Returns the names actually archived. """ - destination = run_dir / "attempts" / f"attempt-{attempt}" + destination = attempt_dir(run_dir, attempt) destination.mkdir(parents=True, exist_ok=True) archived = [] for artifact in artifacts: @@ -314,6 +363,22 @@ def archive_attempt(run_dir: Path, artifacts: list[str], attempt: int) -> list[s return archived +def interrupted_attempt_artifacts(stages: list[dict], attempt: int) -> list[str]: + """What a resumed run would write over if the interrupted attempt stayed. + + The stage artifacts archive_attempt already derives from the workflow, plus + that attempt's rendered prompts. The prompts are the addition a resume + needs: a resumed run carries retry_count forward — it must, since resetting + it would overwrite the escalated attempt's verification iteration — so it + re-renders under the same attempt number and would write over the prompt + the interrupted stage was actually given. No stage name and no artifact + name is written here; both come off the loaded workflow. + """ + return archivable_artifacts(stages) + [ + f"prompt-{stage['name']}-attempt-{attempt}.md" for stage in stages + ] + + def conditional_artifacts(stage: dict) -> list[str]: """The artifacts a stage may write but is not required to. @@ -1021,6 +1086,216 @@ def _revert_check_permitted( ) +# -------------------------------------------------------------------------- +# Ending a run so it can be resumed +# +# A successful run's work is durable: _complete commits it. An escalated run's +# was not, so it lived in the working tree and survived exactly until someone +# checked out another branch — a normal thing to do while deciding what to do +# about an escalation. Resume cannot recover what the harness did not preserve, +# so the escalation commits too. +# +# It commits with the same looseness _complete has, and the limit is stated +# rather than closed here: `git add -A` stages whatever is in the working tree, +# not what the run produced, and an escalation does that on a tree that is by +# definition unfinished. Closing it is a separate story, deliberately after +# this one. +# +# The escalation ends in *two* commits, and the second one is not bookkeeping +# for its own sake. Everything the escalation writes — state.json, both +# renderings of the event stream, the summary — has to be inside the commit, +# because a repository that tracks its run directory is otherwise left dirty by +# the very writes that record the escalation, and the checkout this exists to +# make safe is refused. But state.json records the sha of the commit it is +# committed in, and no commit can contain its own sha: the content is hashed +# into the identity being recorded. So the work is committed first, the sha of +# that commit is written to state.json, and a second commit carries the record +# on top. It is made even when it has nothing to add — a repository that +# ignores its run directory has nothing to record — so the branch an escalation +# leaves always has the same shape and the undo command named in the message +# can name one revision. +# -------------------------------------------------------------------------- + +#: Leads the escalation commit's subject. _complete's subject is +#: "<story-id>: <title>", so a subject beginning with a marker naming what the +#: commit is cannot be read as a completion in `git log --oneline`, in a PR +#: title, or by anyone scanning the branch. +ESCALATION_COMMIT_MARKER = "l5 escalated:" + +#: How the escalation commit's changes are put back in the working tree. Named +#: in the body, because a developer deciding what to do about an escalation +#: should not have to work it out. Two revisions, because an escalation that +#: commits makes two commits — the work and the record of it — and both belong +#: back in the tree. +ESCALATION_UNDO_COMMAND = "git reset --mixed HEAD~2" + + +def escalation_commit_message(state: RunState, reason: str) -> str: + """The escalation commit's message: what it is, why, and how to undo it. + + The subject names the stage execution stopped at. The body says outright + that this is a holding place rather than a decision about the work, carries + the escalation reason, and names the command that returns the changes to + the working tree. + """ + stage = state.current_stage or "no stage" + return ( + f"{ESCALATION_COMMIT_MARKER} {state.story_id} stopped at {stage}\n" + f"\n" + f"The run escalated and this commit is a holding place for what it " + f"left in the working tree, so the work survives a checkout of another " + f"branch. It is not a decision about that work: the story did not " + f"finish and nothing here has been accepted or reviewed.\n" + f"\n" + f"Escalation reason: {reason}\n" + f"\n" + f"To put these changes back in the working tree:\n" + f" {ESCALATION_UNDO_COMMAND}\n" + ) + + +def commit_escalated_work( + target_root: Path, + state: RunState, + reason: str, + *, + run_dir: Path | None = None, +) -> str: + """Open the escalation's commit of what the run left, and name it. + + This is the first of the two commits an escalation makes: the run's own + record of the escalation — state.json, both renderings of the event stream + — so that the sha it returns can be written into state.json and committed, + with the work, by `commit_escalated_tree` on top of it. A commit cannot + carry its own sha, and that is the whole reason the record and the work are + two commits rather than one. + + Returns the commit, or "" when the escalated run left nothing to commit at + all — an escalation with a clean tree records no commit, commits nothing + further, and is not an error. It establishes nothing about the tree it + commits, exactly as _complete does not: both stage whatever the working + tree holds. + + `--allow-empty`, because a repository that ignores its run directory has no + record to commit here and must still leave the same two-commit shape: the + undo command named in the message names one revision and has to be right in + both shapes. + """ + if not _git(target_root, "status", "--porcelain").stdout.strip(): + return "" + if run_dir is not None: + _git(target_root, "add", "-A", "--", str(run_dir)) + committed = _git( + target_root, + "commit", + "--allow-empty", + "-m", + escalation_commit_message(state, reason), + ) + return _revision(target_root) if committed.returncode == 0 else "" + + +def commit_escalated_tree(target_root: Path, state: RunState, reason: str) -> None: + """Commit the work the escalated run left, on top of its record. + + The second of the two commits, and the branch tip an escalation leaves. It + carries the same message as the commit it sits on, because it is the same + escalation: a reader scanning the branch should meet the escalation rather + than a bookkeeping entry. + """ + _git(target_root, "add", "-A") + _git( + target_root, + "commit", + "--allow-empty", + "-m", + escalation_commit_message(state, reason), + ) + + +def escalation_reason(run_dir: Path) -> str | None: + """The reason the escalation summary recorded, for a message only. + + Nothing routes on this. The resume guard decides entirely from state.json, + and this is read afterwards so a refusal can say what the run escalated + for; a missing or reshaped summary costs the message a sentence and changes + no decision. + """ + path = run_dir / "escalation-summary.md" + if not path.is_file(): + return None + text = path.read_text(encoding="utf-8") + if "## Reason" not in text: + return None + return text.split("## Reason", 1)[1].split("##", 1)[0].strip() or None + + +def unchanged_since_escalation( + state: RunState, story_text: str, target_root: Path, harness_root: Path +) -> list[str]: + """Evidence that resuming would reach the same point the same way. + + Three comparisons, each of which must be *establishable* before it can say + anything: the story artifact against the digest recorded at run start, the + branch against the escalation commit the harness made, and the harness + against the revision recorded when the run escalated. + + Returns the evidence only when all three are establishable and identical, + and an empty list otherwise. Anything the guard cannot establish counts as + not-the-same, so an absent digest, an escalation that committed nothing, a + target whose HEAD cannot be read, or a harness root that is not a git + repository produces no refusal rather than a false one. + """ + if not state.story_digest or state.story_digest != story_digest(story_text): + return [] + evidence = [ + "the story artifact is byte for byte the one the escalated run read" + ] + + # The escalation commit is the branch tip's parent, not the tip: the record + # of its sha is committed on top of it, because a commit cannot carry its + # own sha. A branch a developer has committed on since therefore fails this + # comparison, which is what it is for. + if ( + not state.escalation_commit + or _revision(target_root, "HEAD~1") != state.escalation_commit + ): + return [] + porcelain = _git(target_root, "status", "--porcelain") + if porcelain.returncode != 0 or porcelain.stdout.strip(): + return [] + evidence.append( + f"branch {state.branch} is exactly the escalation commit " + f"{state.escalation_commit[:12]}, with nothing uncommitted" + ) + + if not state.harness_revision or _revision(harness_root) != state.harness_revision: + return [] + evidence.append( + f"the harness is still at revision {state.harness_revision[:12]}" + ) + return evidence + + +def _resume_refusal( + story_path: Path, run_dir: Path, state: RunState, evidence: list[str] +) -> str: + reason = escalation_reason(run_dir) + lines = [ + f"{state.story_id} escalated at stage {state.current_stage} and nothing " + f"establishable has changed since:", + *(f" - {item}" for item in evidence), + ] + if reason: + lines.append(f"It escalated because: {reason}") + lines.append( + f"Resuming now would reach the same point the same way. Amend " + f"{story_path}, change the code on branch {state.branch}, or update the " + f"harness, then run the story again." + ) + return "\n".join(lines) + + def _refuse(story_path: Path, problems: list[str]) -> int: """The one pre-flight refusal path: exit 1, one message per problem.""" print(f"{story_path} is not a valid story artifact:", file=sys.stderr) @@ -1033,14 +1308,69 @@ def _refuse(story_path: Path, problems: list[str]) -> int: return 1 -def _escalate(run_dir: Path, state: RunState, reason: str, **event_fields) -> int: +def _commits_the_tree_it_ends_on(escalate): + """Commit the escalated work after the escalation has finished writing. + + The work commit has to be the last thing that happens: every file the + escalation writes — state.json, both renderings of the event stream, the + escalation summary — belongs inside it, or a repository that tracks its run + directory is left dirty by the very writes that record the escalation, and + the checkout this story exists to make safe is refused. Wrapping is how the + ordering is expressed without moving the escalation's own writing around, + whose last act is the summary a separate request owns. + """ + + @functools.wraps(escalate) + def escalate_and_commit( + run_dir: Path, + state: RunState, + reason: str, + *, + target_root: Path, + harness_root: Path, + **event_fields, + ) -> int: + code = escalate( + run_dir, + state, + reason, + target_root=target_root, + harness_root=harness_root, + **event_fields, + ) + if state.escalation_commit: + commit_escalated_tree(target_root, state, reason) + return code + + return escalate_and_commit + + +@_commits_the_tree_it_ends_on +def _escalate( + run_dir: Path, + state: RunState, + reason: str, + *, + target_root: Path, + harness_root: Path, + **event_fields, +) -> int: """End the run, recording the escalation in both renderings. A run that failed must be as reconstructable as one that passed, so the history ends with this entry. Callers forward whatever structured fields the escalation has — the stage's elapsed time, the verifier's outcome, the decision that routed here — through event_fields. + + The run's record of the escalation is committed here, and the sha of that + commit is written into state.json for the commit that follows to carry — + the wrapper above commits the tree once this has written everything, so + that the escalation's own evidence is inside the commit and the tree it + leaves is clean. The harness revision is recorded here too, for the same + reader: a resume can then tell whether the harness itself has changed + since. """ + state.harness_revision = _revision(harness_root) state.status = "escalated" save_state(run_dir, state) append_event( @@ -1050,6 +1380,11 @@ def _escalate(run_dir: Path, state: RunState, reason: str, **event_fields) -> in stage=state.current_stage or None, **event_fields, ) + state.escalation_commit = commit_escalated_work( + target_root, state, reason, run_dir=run_dir + ) + if state.escalation_commit: + save_state(run_dir, state) summary = ( f"# {state.story_id} Escalation Summary\n\n" f"## Status\nEscalated\n\n" @@ -1098,13 +1433,35 @@ def run_story( harness_root: Path, target_root: Path, runner=agent_runner.run_agent, + start_stage: str | None = None, ) -> int: + """Execute one story, from a fresh run or from where a run left off. + + `start_stage` overrides where execution enters — the recorded stage on a + resume, the workflow's first stage on a fresh run. It is named + `start_stage` rather than `stage` because `stage` is the loop's name for + the stage being executed, and one name for two things is how this + repository has repeatedly confused itself. + """ config = harness_config.load_config(target_root) workflow = harness_config.load_workflow(harness_root, config.get("workflow", "story-workflow")) rules = harness_config.load_rules(harness_root) stages = workflow["stages"] stage_names = [s["name"] for s in stages] + # A stage the developer named overrides where execution enters, which is + # what makes a resume useful: an escalation caused by an amended story is + # re-entered at the implementer rather than at the verifier that recorded + # it. Refused above everything else, in the shape the other pre-flight + # refusals take — exit 1, one message, nothing created and no agent run. + if start_stage is not None and start_stage not in stage_names: + print( + f"'{start_stage}' is not a stage the loaded workflow defines. " + f"{workflow['name']} defines: {', '.join(stage_names)}.", + file=sys.stderr, + ) + return 1 + story_path = target_root / config.get("stories_dir", ".harness/stories") / f"{story_id}.yaml" if not story_path.is_file(): print(f"No story artifact at {story_path}. Run l5-plan first.", file=sys.stderr) @@ -1133,12 +1490,14 @@ def run_story( log_path = target_root / config.get("logs_dir", ".harness/logs") / f"{story_id}.log" state = load_state(run_dir) - if state and state.status != "running": + if state and state.status == "completed": # Name the branch as well as the run directory. _checkout_story_branch # reuses an existing branch rather than resetting it, so deleting only # the run directory re-runs the story on top of the finished work — the # implementer opens a repository where the story is already done, and - # the run reports success having changed nothing. + # the run reports success having changed nothing. This guard is about + # finished work; an escalated run resumes below, because its work is + # not finished and its evidence is what the resume exists to keep. print( f"{story_id} already ended with status '{state.status}'. " f"Inspect {run_dir} to review it.\n" @@ -1149,6 +1508,50 @@ def run_story( ) return 1 if state: + # A resume, of a crashed run or an escalated one. Chapter 18 treats + # the two identically and so does this: restore nothing, because the + # artifacts and the state are already here, and continue at the + # recorded stage. Nothing is reinitialized — retry_count and + # verification_iterations key the rendered prompt and verification + # iteration filenames, so resetting them would overwrite the evidence + # of the attempt being resumed. + if state.status == "escalated": + # Resuming is inferred from the recorded status and from nothing + # else. What the guard adds is a refusal in the one case where a + # resume is knowably pointless: the story, the tree and the harness + # are all establishably what they were when the run escalated. + evidence = unchanged_since_escalation( + state, story_text, target_root, harness_root + ) + if evidence: + print( + _resume_refusal(story_path, run_dir, state, evidence), + file=sys.stderr, + ) + return 1 + if start_stage: + state.current_stage = start_stage + if state.status == "escalated": + # The interrupted attempt is archived before the resumed stage + # runs, under the attempt number it was written with. Refuse rather + # than overwrite: the archive is the evidence a resume exists to + # preserve, and story-010 recorded exactly this case as open. + attempt = state.retry_count + 1 + destination = attempt_dir(run_dir, attempt) + if destination.exists(): + print( + f"{destination} already holds an archived attempt, and " + f"resuming {story_id} would write attempt {attempt} over " + f"it. Move or remove it if that attempt is not worth " + f"keeping, then run the story again.", + file=sys.stderr, + ) + return 1 + archive_attempt( + run_dir, interrupted_attempt_artifacts(stages, attempt), attempt + ) + state.status = "running" + save_state(run_dir, state) append_event( run_dir, f"resumed at stage {state.current_stage}", @@ -1157,7 +1560,14 @@ def run_story( ) else: branch = config.get("branch_prefix", "story/") + story_id - state = RunState(story_id=story_id, branch=branch, current_stage=stage_names[0]) + state = RunState( + story_id=story_id, + branch=branch, + current_stage=start_stage or stage_names[0], + # Recorded from the same text read_story was given, so the digest + # and the run's one reading describe one artifact. + story_digest=story_digest(story_text), + ) save_state(run_dir, state) append_event( run_dir, f"workflow started for {story_id}", kind="workflow-started" @@ -1237,7 +1647,12 @@ def elapsed() -> float | None: ) if not result.ok: return _escalate( - run_dir, state, f"{name} agent process failed", duration_seconds=elapsed() + run_dir, + state, + f"{name} agent process failed", + target_root=target_root, + harness_root=harness_root, + duration_seconds=elapsed(), ) missing = [out for out in stage.get("outputs", []) if not (run_dir / out).is_file()] @@ -1246,6 +1661,8 @@ def elapsed() -> float | None: run_dir, state, f"{name} did not produce required artifacts: {', '.join(missing)}", + target_root=target_root, + harness_root=harness_root, duration_seconds=elapsed(), ) @@ -1255,6 +1672,8 @@ def elapsed() -> float | None: run_dir, state, f"{name} wrote an invalid artifact: {violation}", + target_root=target_root, + harness_root=harness_root, duration_seconds=elapsed(), ) @@ -1266,6 +1685,8 @@ def elapsed() -> float | None: run_dir, state, f"{name} modified blocked path: {violation}", + target_root=target_root, + harness_root=harness_root, duration_seconds=elapsed(), ) @@ -1290,6 +1711,8 @@ def elapsed() -> float | None: state, f"{name} created {ownership.path}, which it declared it " f"must not create under {ownership.prefix}", + target_root=target_root, + harness_root=harness_root, duration_seconds=elapsed(), ) @@ -1323,6 +1746,8 @@ def elapsed() -> float | None: state, f"the revert check on {name}'s edits ({listed}) under " f"{prefixes} could not run: {decided.result.reason}", + target_root=target_root, + harness_root=harness_root, duration_seconds=elapsed(), ) if not decided.permitted: @@ -1332,6 +1757,8 @@ def elapsed() -> float | None: f"{name} edited {listed} under {prefixes}, which it " f"declared it must not create under, and the suite " f"still passes with those edits reverted", + target_root=target_root, + harness_root=harness_root, duration_seconds=elapsed(), ) _revert_check_permitted(run_dir, name, revert_artifact, edits) @@ -1367,6 +1794,8 @@ def elapsed() -> float | None: run_dir, state, f"the clean-clone check could not run: {clean.reason}", + target_root=target_root, + harness_root=harness_root, duration_seconds=elapsed(), ) if clean.exit_code != 0: @@ -1377,6 +1806,8 @@ def elapsed() -> float | None: state, f"the clean-clone check failed and retries are " f"exhausted: {failures}", + target_root=target_root, + harness_root=harness_root, duration_seconds=elapsed(), retry_decision="escalate", retry_reason=( @@ -1457,6 +1888,8 @@ def elapsed() -> float | None: run_dir, state, "verification failed and retries are exhausted", + target_root=target_root, + harness_root=harness_root, duration_seconds=elapsed(), verifier_outcome=verdict.get("status"), retry_decision="escalate", @@ -1467,6 +1900,8 @@ def elapsed() -> float | None: run_dir, state, "verification failed and the verifier did not recommend a retry", + target_root=target_root, + harness_root=harness_root, duration_seconds=elapsed(), verifier_outcome=verdict.get("status"), retry_decision="escalate", diff --git a/scripts/l5-run b/scripts/l5-run index 3b2f7cf..e7efb2b 100755 --- a/scripts/l5-run +++ b/scripts/l5-run @@ -1,9 +1,13 @@ #!/usr/bin/env python3 """Execute an approved story through the story workflow. -Usage: l5-run <story-id> +Usage: l5-run <story-id> [--stage <stage>] The target repository is found by walking up from the current directory to the nearest .harness/config.yaml. + +A run whose state.json says it escalated resumes at the recorded stage. +--stage overrides that with a stage the loaded workflow defines; the +coordinator refuses one it does not. """ import sys from pathlib import Path @@ -16,11 +20,17 @@ import story_coordinator # noqa: E402 def main() -> int: - if len(sys.argv) != 2: + argv = sys.argv[1:] + stage = None + if len(argv) == 3 and argv[1] == "--stage": + argv, stage = argv[:1], argv[2] + if len(argv) != 1: print(__doc__.strip(), file=sys.stderr) return 1 target_root = harness_config.find_target_root(Path.cwd()) - return story_coordinator.run_story(sys.argv[1], HARNESS_ROOT, target_root) + return story_coordinator.run_story( + argv[0], HARNESS_ROOT, target_root, start_stage=stage + ) if __name__ == "__main__": diff --git a/tests/test_coordinator_contract.py b/tests/test_coordinator_contract.py index 349ef81..55d2980 100644 --- a/tests/test_coordinator_contract.py +++ b/tests/test_coordinator_contract.py @@ -38,6 +38,9 @@ # Every status the coordinator may write. A run starts `running` and ends in # one of ENDING_STATUSES; the source check below fails if a fourth appears. +# Ending is not the same as final: since story-020 an `escalated` run can be +# resumed, which returns its status to `running`. `completed` is the one a +# rerun still refuses. ENDING_STATUSES = {"completed", "escalated"} STATUSES = {"running", *ENDING_STATUSES} @@ -97,6 +100,12 @@ def state_contract_problems(state: dict) -> list[str]: "retry_count": int, "verification_iterations": int, "artifacts": list, + # story-020's resume fields. Each defaults to empty, which is what a + # state file written before this story loads as and what every reader + # treats as "not established". + "story_digest": str, + "escalation_commit": str, + "harness_revision": str, } declared = {f.name for f in dataclasses.fields(story_coordinator.RunState)} problems = [] diff --git a/tests/test_story_010_validation.py b/tests/test_story_010_validation.py index fe62e80..98968f0 100644 --- a/tests/test_story_010_validation.py +++ b/tests/test_story_010_validation.py @@ -475,6 +475,12 @@ def test_no_reader_was_pointed_at_the_archive(): assembler = Path(context_assembler.__file__).read_text(encoding="utf-8") assert "attempts" not in assembler coordinator = Path(story_coordinator.__file__).read_text(encoding="utf-8") + # story-020 gave the directory its own helper, because the resume has to + # refuse an attempt directory that already exists and naming it a second + # time is how one fact ends up in two places. The guarantee is unchanged + # and now exact: one literal in the module, in the helper both the archive + # and the resume derive the directory from. body = _archive_code_body("archive_attempt") assert coordinator.count('"attempts"') == 1 - assert "attempts" in body + assert "attempts" in _archive_code_body("attempt_dir") + assert "attempt_dir(" in body diff --git a/tests/test_story_012_validation.py b/tests/test_story_012_validation.py index cd8fd20..8eb0285 100644 --- a/tests/test_story_012_validation.py +++ b/tests/test_story_012_validation.py @@ -653,7 +653,15 @@ def test_a_run_whose_retry_history_keeps_disappearing_routes_identically( run_dir, control_dir = run_dir_of(target_root), run_dir_of(control_root) assert runner.calls == control.calls - assert read_state(run_dir) == read_state(control_dir) + # Every field but the escalation commit, which since story-020 records the + # commit each escalation makes on its own branch: these are two copies of + # one repository, so the two shas differ for a reason that has nothing to + # do with routing. Compared field by field so a new field is included by + # default rather than needing to be added here. + volatile = {"escalation_commit"} + assert ({k: v for k, v in read_state(run_dir).items() if k not in volatile} + == {k: v for k, v in read_state(control_dir).items() + if k not in volatile}) assert _log_messages(run_dir) == _log_messages(control_dir) # And the control run did keep a full history, so the comparison above is # between a run missing the artifact and one that had it. diff --git a/tests/test_story_019_validation.py b/tests/test_story_019_validation.py index 919255e..1587c77 100644 --- a/tests/test_story_019_validation.py +++ b/tests/test_story_019_validation.py @@ -1124,8 +1124,12 @@ def test_the_capture_names_no_stage_and_no_prefix(): # The baseline is evidence, never state # -------------------------------------------------------------------------- -#: state.json's fields, written here rather than read off the dataclass that -#: produces them — a comparison against its own source could not fail. +#: state.json's fields as of story-019, written here rather than read off the +#: dataclass that produces them — a comparison against its own source could not +#: fail. A later story may add a field for its own reasons: story-020 added +#: three so a resume can tell what has changed since a run escalated. What this +#: story claims is narrower and is what the assertion below now states — the +#: *baseline* added none, and none of the fields that arrived later names it. STATE_FIELDS = {"story_id", "branch", "status", "current_stage", "retry_count", "verification_iterations", "artifacts"} @@ -1135,7 +1139,9 @@ def test_state_json_gains_no_field_and_never_names_the_baseline( ): assert run(target, harness_root, RETRY_SHAPE, [FAIL, PASS])[0] == 0 state_text = (run_dir_of(target) / "state.json").read_text() - assert set(json.loads(state_text)) == STATE_FIELDS + fields = set(json.loads(state_text)) + assert STATE_FIELDS <= fields + assert [name for name in fields if "baseline" in name] == [] assert BASELINE not in state_text # The control: the run this state describes did capture a baseline, so # the absence above is about state.json rather than about a run that diff --git a/tests/test_story_020_validation.py b/tests/test_story_020_validation.py new file mode 100644 index 0000000..336ea74 --- /dev/null +++ b/tests/test_story_020_validation.py @@ -0,0 +1,1695 @@ +"""Independent validation for story-020: resuming an escalated run, and +committing its work when it escalates. + +The subject is a *terminal state that is no longer terminal*, so almost +nothing here is asserted from source. A target repository is built under +tmp_path, fake stage agents drive it into an escalation, and the coordinator +is then run again against the run directory the escalation left. What resume +does is whatever the second run does to that directory. + +The property the story exists to guarantee is checked the way the story words +it — escalate, check out another branch, and look at that branch — rather than +by reading the commit the escalation made. + +Every absence asserted here carries a demonstration that the same check can +report the violation it exists to catch: + + * "the other branch is untouched after an escalation" sits beside the same + checkout performed with the work left uncommitted, which does carry it + across — so the escalation's commit is what the assertion is about; + * "the escalation subject is not a completion's" is a pattern paired with a + real completion subject, which it does match; + * "the escalated attempt's verification iteration is unmodified" sits beside + the identical resume with the counters reset, which overwrites it; + * "the resume refuses when nothing changed" sits beside the same resume with + one input changed, which proceeds — once per input, so no comparison can + be the only one carrying the decision; + * "an unknown stage starts no agent and creates nothing" sits beside the + same call with a stage the workflow does define; + * "nothing routes on the escalation summary, the archive or the baseline" is + a run with all three removed, which routes identically, plus a scan whose + control is the field that *is* routed on; + * "a pre-story state.json still loads" is written by the pre-story module + itself, read out of git, and paired with a field neither module declares, + which still fails to load. + +The one place this file asserts a *positive* fact about a limit rather than a +guarantee is `test_neither_terminal_commit_establishes_what_it_commits`. That +limit is stated rather than closed by this story, and the test is written so +that closing it turns the test red rather than leaving it quietly true. + +Nothing here invokes a model: every run goes through a fake agent runner and +every clone source is a local filesystem path. +""" +import importlib.machinery +import importlib.util +import inspect +import json +import re +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import story_commit_range, story_diff + +import harness_config +import story_coordinator +from agent_runner import AgentResult + +REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] +WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +STAGE_NAMES = [stage["name"] for stage in WORKFLOW["stages"]] +VERIFIER_STAGE = next(s for s in WORKFLOW["stages"] if "on_failure" in s) +RETRY_STAGE = VERIFIER_STAGE["on_failure"]["retry_stage"] +IMPLEMENTER_STAGE = next(s for s in WORKFLOW["stages"] + if "revert_check" in s) +BASELINE = IMPLEMENTER_STAGE["revert_check"]["baseline"] + +STORY_ID = "story-001" +STORY_TITLE = "Sample story for coordinator tests" +DEFAULT_BRANCH = "main" + +PASS = {"status": "passed", "blocking_issues": [], "unverified": [], + "retry_recommended": False} + + +def failing(attempt: int, *, retry: bool) -> dict: + """A failing verdict whose text names the attempt that produced it. + + Every attempt's verdict differs, so an iteration file that has been + written over by a later attempt is distinguishable from one that was not. + """ + return { + "status": "failed", + "blocking_issues": [{ + "severity": "high", + "issue": f"attempt {attempt} did not implement the sample behavior", + "location": f"src/attempt_{attempt}.py", + "required_behavior": f"the sample behavior exists after attempt {attempt}", + }], + "unverified": [], + "retry_recommended": retry, + } + + +#: The two escalation reasons the verifier routing produces, spelled here only +#: as the shapes the tests below drive; the reason text itself is always read +#: back off the run rather than compared with a literal. +FAIL_RETRY = failing(1, retry=True) +FAIL_FINAL = failing(2, retry=False) +FAIL_AT_ONCE = failing(1, retry=False) + +STORY = f"""\ +story: + id: {STORY_ID} + title: {STORY_TITLE} + description: | + A stand-in story used to exercise the workflow deterministically. + +tasks: + - do the sample work + +acceptance_criteria: + - the sample behavior exists + - existing behavior is preserved + +scope: + modify: + - src/ + do_not_modify: + - rules/ + +verification_requirements: + - confirm the sample behavior + +constraints: + - preserve existing behavior +""" + +CONFIG = """\ +project: resume-target +workflow: story-workflow +branch_prefix: story/ +permission_mode: acceptEdits +stories_dir: .harness/stories +runs_dir: .harness/runs +logs_dir: .harness/logs +standards_dir: .harness/standards +architecture_docs: + - .harness/docs/ARCHITECTURE.md +test_command: echo tests-ok +""" + +APP_AT_HEAD = "print('hello')\n" +TEST_AT_HEAD = "def test_nothing():\n assert True\n" + + +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 git(root: Path, *args: str, check: bool = True) -> subprocess.CompletedProcess: + return subprocess.run(["git", "-C", str(root), *args], + capture_output=True, text=True, check=check) + + +def init_repo(root: Path) -> None: + subprocess.run(["git", "init", "-q"], cwd=root, check=True) + subprocess.run(["git", "config", "user.email", "t@example.com"], cwd=root, check=True) + subprocess.run(["git", "config", "user.name", "T"], cwd=root, check=True) + subprocess.run(["git", "add", "-A"], cwd=root, check=True) + subprocess.run(["git", "commit", "-q", "-m", "initial"], cwd=root, check=True) + # A deterministic name for the branch a developer checks out *away* to. + subprocess.run(["git", "branch", "-M", DEFAULT_BRANCH], cwd=root, check=True) + + +def build_target(root: Path, gitignore: str = "") -> Path: + for sub in (".harness/standards", ".harness/stories", ".harness/runs", + ".harness/logs", ".harness/docs"): + (root / sub).mkdir(parents=True) + write(root / ".harness" / "config.yaml", CONFIG) + write(root / ".harness" / "stories" / f"{STORY_ID}.yaml", STORY) + write(root / ".harness" / "standards" / "coding.md", "# Coding\n- simple\n") + write(root / ".harness" / "standards" / "testing.md", "# Testing\n- test it\n") + write(root / ".harness" / "docs" / "ARCHITECTURE.md", "# Architecture\n") + write(root / "src" / "app.py", APP_AT_HEAD) + write(root / "tests" / "test_existing.py", TEST_AT_HEAD) + if gitignore: + write(root / ".gitignore", gitignore) + init_repo(root) + return root + + +@pytest.fixture +def target(tmp_path: Path) -> Path: + """A target repository whose run directory is tracked, as a project's is.""" + return build_target(tmp_path / "resume-target") + + +@pytest.fixture +def quiet_target(tmp_path: Path) -> Path: + """The same repository with its run directory ignored. + + The one shape in which an escalation can find nothing to commit: with the + run directory tracked, every run dirties the tree by writing state.json. + """ + return build_target(tmp_path / "quiet-target", gitignore=".harness/runs/\n") + + +@pytest.fixture +def harness_root() -> Path: + return REPO_ROOT + + +# -------------------------------------------------------------------------- +# The stage edits and the fake runner +# -------------------------------------------------------------------------- + + +def unchanged(root: Path, attempt: int) -> dict: + return {"modified": [], "created": [], "deleted": []} + + +def edits_the_module(root: Path, attempt: int) -> dict: + write(root / "src" / "app.py", APP_AT_HEAD + f"print('attempt {attempt}')\n") + return {"modified": ["src/app.py"], "created": [], "deleted": []} + + +def creates_a_module(root: Path, attempt: int) -> dict: + write(root / "src" / f"attempt_{attempt}.py", f"value = {attempt}\n") + return {"modified": [], "created": [f"src/attempt_{attempt}.py"], "deleted": []} + + +class Runner: + """A fake agent runner: each stage writes its artifacts, and a stage + holding an edit also makes that edit in the target's working tree. + + It records, at the entry to every stage, which attempt directories already + existed — which is how "archived *before* the resumed stage runs" is + checked as a fact about the run rather than about its final state. + """ + + def __init__(self, target_root: Path, edits: dict | None = None, + verdicts: list | None = None, story_id: str = STORY_ID): + self.target_root = target_root + self.run_dir = target_root / ".harness" / "runs" / story_id + self.edits = edits or {} + self.verdicts = verdicts or [PASS] + self.calls: list[str] = [] + #: (stage, the attempt directories present when the stage started) + self.archives_seen: list[tuple[str, list[str]]] = [] + self.verdicts_written: list[dict] = [] + + def _nth(self, sequence: list, index: int): + return sequence[min(index, len(sequence) - 1)] + + def _edit(self, stage: str, attempt: int) -> dict: + seen = self.calls.count(stage) - 1 + edit = self._nth(self.edits.get(stage, [unchanged]), seen) + return edit(self.target_root, attempt) + + def __call__(self, prompt, *, stage, cwd=None, log_path=None, + permission_mode=None, model=None, allowed_tools=None): + self.calls.append(stage) + archives = self.run_dir / "attempts" + self.archives_seen.append( + (stage, sorted(p.name for p in archives.glob("*")) if archives.is_dir() else []) + ) + attempt = max(1, self.calls.count(RETRY_STAGE)) + + if stage == "implementer": + write_json(self.run_dir / "changed-files.json", self._edit(stage, attempt)) + write(self.run_dir / "implementation-summary.md", + f"Implemented on attempt {attempt}.\n") + elif stage == "tester": + write_json(self.run_dir / "test-results.json", { + "status": "passed", "tests_written": 1, "tests_run": 1, + "tests_passed": 1, "tests_failed": 0, "failures": [], + }) + write_json(self.run_dir / "tester-changed-files.json", + self._edit(stage, attempt)) + elif stage == "verifier": + seen = self.calls.count(stage) - 1 + verdict = self._nth(self.verdicts, seen) + self.verdicts_written.append(verdict) + write_json(self.run_dir / "verification-result.json", verdict) + elif stage == "documenter": + write(self.run_dir / "documentation-report.md", "Nothing.\n") + return AgentResult(ok=True, result_text=f"{stage} done") + + +def run_dir_of(target_root: Path, story_id: str = STORY_ID) -> Path: + return target_root / ".harness" / "runs" / story_id + + +def state_of(target_root: Path) -> dict: + return json.loads((run_dir_of(target_root) / "state.json").read_text()) + + +def write_state(target_root: Path, **changes) -> None: + """Rewrite state.json in place, the way an inspecting developer would.""" + path = run_dir_of(target_root) / "state.json" + state = json.loads(path.read_text()) + state.update(changes) + path.write_text(json.dumps(state, indent=2) + "\n", encoding="utf-8") + + +def strip_new_fields(target_root: Path) -> dict: + """Reduce state.json to the fields it carried before this story. + + The pre-story coordinator loads state.json through `RunState(**json)`, so + it cannot read a file carrying fields it does not declare. Handing it the + pre-story form is what lets the two modules be run against one run + directory; the fields removed are not read by anything being compared. + Returns the original so the caller can put it back. + """ + path = run_dir_of(target_root) / "state.json" + original = json.loads(path.read_text()) + path.write_text( + json.dumps({key: value for key, value in original.items() + if key not in NEW_FIELDS}, indent=2) + "\n", + encoding="utf-8", + ) + return original + + +def restore_state(target_root: Path, original: dict) -> None: + (run_dir_of(target_root) / "state.json").write_text( + json.dumps(original, indent=2) + "\n", encoding="utf-8") + + +def run(target_root: Path, harness: Path = REPO_ROOT, edits: dict | None = None, + verdicts: list | None = None, runner: Runner | None = None, + start_stage: str | None = None) -> tuple[int, Runner]: + runner = runner or Runner(target_root, edits, verdicts) + code = story_coordinator.run_story( + STORY_ID, harness, target_root, runner, start_stage=start_stage) + return code, runner + + +#: The run shape that escalates at the verifier with no retry taken. +AT_ONCE = ([FAIL_AT_ONCE], {"implementer": [edits_the_module]}) +#: The run shape that retries once and then escalates, so the resumed run has +#: a non-zero retry count and a second verification iteration to carry. +AFTER_A_RETRY = ([FAIL_RETRY, FAIL_FINAL], + {"implementer": [creates_a_module, creates_a_module]}) + + +def escalate(target_root: Path, harness: Path = REPO_ROOT, + shape: tuple = AT_ONCE) -> Runner: + verdicts, edits = shape + code, runner = run(target_root, harness, edits, verdicts) + assert code == 2, "the shape was meant to escalate" + assert state_of(target_root)["status"] == "escalated" + return runner + + +def change_the_code(target_root: Path) -> None: + """One of the three things the refusal message tells a developer to do. + + Used to clear the guard in tests whose subject is what the resume *does*, + so that clearing it is a single, named act rather than a side effect. + """ + write(target_root / "src" / "app.py", APP_AT_HEAD + "print('by hand')\n") + + +def amend_the_story(target_root: Path) -> None: + write(target_root / ".harness" / "stories" / f"{STORY_ID}.yaml", + STORY + " - and keep the sample behavior working\n") + + +def subject_of(root: Path, revision: str = "HEAD") -> str: + return git(root, "log", "-1", "--format=%s", revision).stdout.strip() + + +def body_of(root: Path, revision: str = "HEAD") -> str: + return git(root, "log", "-1", "--format=%b", revision).stdout + + +def files_in(root: Path, revision: str = "HEAD") -> list[str]: + return git(root, "show", "--name-only", "--format=", revision).stdout.split() + + +def messages(target_root: Path) -> list[str]: + log = (run_dir_of(target_root) / "events.log").read_text() + return [line.split("] ", 1)[1] for line in log.splitlines() if "] " in line] + + +def history(target_root: Path) -> list[dict]: + return json.loads( + (run_dir_of(target_root) / "execution-history.json").read_text()) + + +def executable_source(text: str) -> str: + """Strip docstrings and comment lines; prose may name what code may not.""" + kept, in_docstring = [], False + for line in text.splitlines(): + stripped = line.lstrip() + if stripped.startswith('"""') or stripped.startswith("'''"): + if not (len(stripped) > 3 and stripped.rstrip().endswith('"""') + and stripped.rstrip() != '"""'): + in_docstring = not in_docstring + continue + if in_docstring or stripped.startswith("#"): + continue + kept.append(line) + return "\n".join(kept) + + +def pre_story(path: str) -> str: + """A repository file as it stood before this story's own run. + + Resolved through the shared range in conftest.py rather than as HEAD, so + the comparison survives this story's own commit. + """ + revision = story_commit_range(Path(__file__)).baseline + return git(REPO_ROOT, "show", f"{revision}:{path}").stdout + + +def pre_story_coordinator(tmp_path: Path): + """The coordinator as it stood before this story, loaded as its own module. + + Every claim of the form "this story did not change X" is made against this + rather than against a phrase written here, so the control for each is the + thing the story *did* change, compared the same way. + """ + module_path = tmp_path / "pre_story_coordinator.py" + write(module_path, pre_story("orchestration/story_coordinator.py")) + loader = importlib.machinery.SourceFileLoader( + "pre_story_coordinator", str(module_path)) + spec = importlib.util.spec_from_loader(loader.name, loader) + module = importlib.util.module_from_spec(spec) + # Registered before execution because `@dataclass` resolves a field's + # annotations through sys.modules[cls.__module__], which is None for a + # module that has been created but never registered. + sys.modules[loader.name] = module + try: + loader.exec_module(module) + finally: + sys.modules.pop(loader.name, None) + return module + + +# -------------------------------------------------------------------------- +# An escalated run's work survives a checkout +# -------------------------------------------------------------------------- + + +def test_an_escalated_run_leaves_a_clean_working_tree(target, harness_root): + """The first half of the property the story exists to guarantee. + + The control is `test_the_same_checkout_carries_uncommitted_work_across` + below, which shows what an unclean tree costs at the checkout. + """ + escalate(target, harness_root) + + assert git(target, "rev-parse", "--abbrev-ref", "HEAD").stdout.strip() \ + == f"story/{STORY_ID}" + assert (target / "src" / "app.py").read_text() != APP_AT_HEAD + assert git(target, "status", "--porcelain").stdout.strip() == "" + + +def test_an_escalated_run_leaves_another_branch_untouched(target, harness_root): + """The property checked the way the story words it: escalate, check out + another branch, and look at that branch. + + Checking out is the act the story exists to make safe, so the checkout's + own exit code is asserted before what it left behind — a checkout that + aborts is a stronger failure than one that carries work across, and it is + invisible to an assertion that only reads the tree afterwards. + """ + escalate(target, harness_root) + + checkout = git(target, "checkout", DEFAULT_BRANCH, check=False) + + assert checkout.returncode == 0, checkout.stderr + assert git(target, "rev-parse", "--abbrev-ref", "HEAD").stdout.strip() \ + == DEFAULT_BRANCH + assert (target / "src" / "app.py").read_text() == APP_AT_HEAD + assert git(target, "status", "--porcelain").stdout.strip() == "" + + +def test_the_escalations_own_evidence_is_committed_with_the_work( + target, harness_root, +): + """What the escalation commit has to carry for the two above to hold. + + state.json, the two renderings of the event stream and the escalation + summary are written by `_escalate` itself, and they are the evidence the + resume reads. Each is compared as content rather than as presence: an + earlier stage's state.json is in the commit whatever the ordering, and a + stale copy of it would satisfy a check that only asked whether the path + was there. + + The control is the artifact written before the escalation began, whose + committed content does match — so this is about *when* the commit is made + rather than about the commit missing the run directory. + """ + escalate(target, harness_root) + run_dir = run_dir_of(target) + run_relative = f".harness/runs/{STORY_ID}" + + def committed(name: str) -> str: + return git(target, "show", f"HEAD:{run_relative}/{name}", + check=False).stdout + + assert committed("changed-files.json") \ + == (run_dir / "changed-files.json").read_text() # the control + for evidence in ("state.json", "events.log", "execution-history.json", + "escalation-summary.md"): + assert committed(evidence) == (run_dir / evidence).read_text(), evidence + + +def test_the_same_checkout_carries_uncommitted_work_across(target): + """The control for the assertion above, and the failure story-018 hit. + + Nothing about the checkout is different — only that the work was left in + the working tree, which is what an escalation used to do. + """ + git(target, "checkout", "-q", "-b", f"story/{STORY_ID}") + write(target / "src" / "app.py", APP_AT_HEAD + "print('uncommitted')\n") + + git(target, "checkout", DEFAULT_BRANCH) + + assert (target / "src" / "app.py").read_text() != APP_AT_HEAD + assert git(target, "status", "--porcelain").stdout.strip() != "" + + +def test_the_escalation_commit_is_recorded_on_state_and_is_on_the_branch( + target, harness_root, +): + """What the recorded commit has to be for anything to read it. + + Asserted as "a real commit, on the story branch, directly under the tree + the escalation ends on" rather than as "the branch tip". The field is + written into the state.json that the commit it names contains, and a commit + cannot name itself: its sha is hashed from a tree that would have to hold + that sha already. So the escalation records its own state and event stream + in one commit, whose sha this field carries, and commits the work on top — + and what the field exists for, telling an escalation the harness committed + from one a developer committed from one where nothing was committed, an + ancestor establishes exactly as a tip would. + + The relationship is asserted rather than left implicit because two other + things are derived from it: the guard compares HEAD~1 and the undo command + names HEAD~2. A recorded commit that were two back, or on no branch at all, + would leave both silently wrong while this field still looked populated. + """ + escalate(target, harness_root) + state = state_of(target) + commit = state["escalation_commit"] + + assert commit + assert git(target, "cat-file", "-t", commit).stdout.strip() == "commit" + assert git(target, "merge-base", "--is-ancestor", commit, + f"story/{STORY_ID}", check=False).returncode == 0 + assert git(target, "rev-parse", "HEAD~1").stdout.strip() == commit + assert state["harness_revision"] == git( + REPO_ROOT, "rev-parse", "HEAD").stdout.strip() + assert state["story_digest"] == story_coordinator.story_digest( + (target / ".harness" / "stories" / f"{STORY_ID}.yaml").read_text()) + + # The control for the ancestry assertion: a commit made elsewhere, which + # the same check reports as not on the story branch. + git(target, "checkout", "-q", DEFAULT_BRANCH) + write(target / "elsewhere.txt", "not on the story branch\n") + git(target, "add", "-A") + git(target, "commit", "-q", "-m", "a commit on another branch") + assert git(target, "merge-base", "--is-ancestor", + git(target, "rev-parse", "HEAD").stdout.strip(), + f"story/{STORY_ID}", check=False).returncode != 0 + + +def test_the_escalation_adds_exactly_the_commits_its_undo_command_names( + target, harness_root, +): + """The one number two other things depend on, read off the branch. + + The undo command's revision count and the number of commits an escalation + adds are the same fact written twice, and nothing else notices if they + drift: an undo naming one revision too few would leave a commit behind and + still put work in the tree, which the undo test alone cannot see. Both are + read off the run here and compared with each other. + + Every commit added is also checked to read as an escalation, so a developer + scanning `git log --oneline` meets the escalation on each line rather than + a bookkeeping entry on one of them. The control is a real completion + subject, which the same pattern does match. + """ + escalate(target, harness_root) + added = git(target, "rev-list", f"{DEFAULT_BRANCH}..story/{STORY_ID}" + ).stdout.split() + named = re.search(r"HEAD~(\d+)\s*$", + story_coordinator.ESCALATION_UNDO_COMMAND) + + assert named, story_coordinator.ESCALATION_UNDO_COMMAND + assert int(named.group(1)) == len(added) + for revision in added: + subject = subject_of(target, revision) + assert subject.startswith(story_coordinator.ESCALATION_COMMIT_MARKER) + assert not COMPLETION_SUBJECT.match(subject) + + completed = build_target(target.parent / "completed-for-count") + code, _ = run(completed, harness_root, {"implementer": [edits_the_module]}) + assert code == 0 + assert COMPLETION_SUBJECT.match(subject_of(completed)) # the control + + +# -------------------------------------------------------------------------- +# The escalation commit's form, and the way back out of it +# -------------------------------------------------------------------------- + +#: `_complete`'s subject form: the story id, a colon, the title. Written as a +#: pattern so the escalation subject is tested against the *shape* a reader +#: would mistake it for rather than against one completion's prose. +COMPLETION_SUBJECT = re.compile(rf"^{re.escape(STORY_ID)}: \S") + + +def test_the_escalation_subject_cannot_be_read_as_a_completion( + target, harness_root, +): + """Asserted on form. The control is a real completion subject from the + same story id, which the same pattern does match — so the pattern is + capable of reporting the confusion it is checking for.""" + escalate(target, harness_root) + escalation = subject_of(target) + + assert escalation.startswith(story_coordinator.ESCALATION_COMMIT_MARKER) + assert not COMPLETION_SUBJECT.match(escalation) + assert VERIFIER_STAGE["name"] in escalation # where execution stopped + + completed = build_target(target.parent / "completed-target") + code, _ = run(completed, harness_root, {"implementer": [edits_the_module]}) + assert code == 0 + assert COMPLETION_SUBJECT.match(subject_of(completed)) # the control + + +def test_the_escalation_body_names_the_reason_and_the_way_back( + target, harness_root, +): + escalate(target, harness_root) + reason = json.loads( + (run_dir_of(target) / "verification-result.json").read_text())["status"] + summary = (run_dir_of(target) / "escalation-summary.md").read_text() + recorded = summary.split("## Reason", 1)[1].split("##", 1)[0].strip() + body = body_of(target) + + assert recorded in body + assert reason == "failed" # the run really did fail + assert story_coordinator.ESCALATION_UNDO_COMMAND in body + assert "not a decision" in body or "holding place" in body + + +def test_the_named_undo_command_actually_returns_the_changes_to_the_tree( + target, harness_root, +): + """The body's instruction, executed rather than read. A command that named + the wrong revision would leave the tree clean or the branch short.""" + escalate(target, harness_root) + commit = state_of(target)["escalation_commit"] + edited = (target / "src" / "app.py").read_text() + assert edited != APP_AT_HEAD + + undo = story_coordinator.ESCALATION_UNDO_COMMAND.split() + subprocess.run(undo, cwd=target, check=True, capture_output=True) + + assert git(target, "rev-parse", "HEAD").stdout.strip() != commit + assert (target / "src" / "app.py").read_text() == edited + assert "src/app.py" in git(target, "status", "--porcelain").stdout + + +def test_the_undo_command_is_right_in_a_repository_that_ignores_its_runs( + quiet_target, harness_root, +): + """The same command, executed in the other repository shape. + + One command is named in every escalation's body, so it has to be right + wherever the escalation happens — and the two shapes differ in exactly what + there is to commit: a tracked run directory gives the escalation's own + record something to carry, an ignored one gives it nothing. If the shapes + left different numbers of commits, this command would be right in one of + them and would either strand a commit or unwind past the run in the other. + + The control is the same command in the tracked shape, one test above. + """ + escalate(quiet_target, harness_root) + started_from = git(quiet_target, "rev-parse", DEFAULT_BRANCH).stdout.strip() + edited = (quiet_target / "src" / "app.py").read_text() + assert edited != APP_AT_HEAD + + subprocess.run(story_coordinator.ESCALATION_UNDO_COMMAND.split(), + cwd=quiet_target, check=True, capture_output=True) + + assert git(quiet_target, "rev-parse", "HEAD").stdout.strip() == started_from + assert (quiet_target / "src" / "app.py").read_text() == edited + assert "src/app.py" in git(quiet_target, "status", "--porcelain").stdout + + +def test_an_escalation_with_nothing_to_commit_records_none_and_does_not_fail( + quiet_target, harness_root, +): + """A repository whose run directory is ignored, and stages that touch + nothing: the tree at escalation is clean. + + The control is the same run in the same repository with one edit, which + does record a commit — so the empty record above is about there being + nothing to commit rather than about the commit never being attempted. + """ + head_before = git(quiet_target, "rev-parse", "HEAD").stdout.strip() + escalate(quiet_target, harness_root, ([FAIL_AT_ONCE], {})) + + assert state_of(quiet_target)["escalation_commit"] == "" + assert git(quiet_target, "rev-parse", "HEAD").stdout.strip() == head_before + + with_an_edit = build_target(quiet_target.parent / "quiet-edited", + gitignore=".harness/runs/\n") + escalate(with_an_edit, harness_root, AT_ONCE) + assert state_of(with_an_edit)["escalation_commit"] != "" + + +def test_commit_escalated_work_returns_empty_on_a_clean_tree(target, tmp_path): + """The same fact at the level below the run, so the end-to-end result is + read against a direct call. The control is the identical call one edit + later.""" + state = story_coordinator.RunState( + story_id=STORY_ID, branch=f"story/{STORY_ID}", current_stage="verifier") + head = git(target, "rev-parse", "HEAD").stdout.strip() + + assert story_coordinator.commit_escalated_work(target, state, "nothing") == "" + assert git(target, "rev-parse", "HEAD").stdout.strip() == head + + write(target / "src" / "app.py", APP_AT_HEAD + "print('now something')\n") + made = story_coordinator.commit_escalated_work(target, state, "something") + assert made == git(target, "rev-parse", "HEAD").stdout.strip() + assert made != head + + +# -------------------------------------------------------------------------- +# _complete is not what this story is fixing +# -------------------------------------------------------------------------- + + +def test_the_completion_commit_is_byte_for_byte_the_code_it_was(tmp_path): + """Same message, same contents, same behavior, stated as sameness of the + function that produces them. The control is `_escalate`, compared the same + way against the same pre-story module, which did change.""" + before = pre_story_coordinator(tmp_path) + assert inspect.getsource(story_coordinator._complete) \ + == inspect.getsource(before._complete) + assert inspect.getsource(story_coordinator._escalate) \ + != inspect.getsource(before._escalate) + + +def test_a_successful_run_still_commits_its_work_under_the_completion_subject( + target, harness_root, +): + """The behavioral half: a run that never escalates makes exactly one + commit, in the completion form, carrying the story's work.""" + head_before = git(target, "rev-parse", "HEAD").stdout.strip() + code, _ = run(target, harness_root, {"implementer": [edits_the_module]}) + assert code == 0 + + assert COMPLETION_SUBJECT.match(subject_of(target)) + assert STORY_TITLE in subject_of(target) + assert not subject_of(target).startswith( + story_coordinator.ESCALATION_COMMIT_MARKER) + assert git(target, "rev-list", "--count", f"{head_before}..HEAD").stdout.strip() == "1" + assert "src/app.py" in files_in(target) + assert (run_dir_of(target) / "completion-report.md").is_file() + + +# -------------------------------------------------------------------------- +# Which runs resume and which still refuse +# -------------------------------------------------------------------------- + + +def test_an_escalated_run_resumes_at_the_recorded_stage(target, harness_root): + escalate(target, harness_root) + assert state_of(target)["current_stage"] == VERIFIER_STAGE["name"] + change_the_code(target) + + code, resumed = run(target, harness_root, verdicts=[PASS]) + + assert code == 0 + assert resumed.calls == [VERIFIER_STAGE["name"], "documenter"] + assert state_of(target)["status"] == "completed" + + +def test_a_completed_run_still_refuses_with_the_message_it_always_had( + target, harness_root, tmp_path, capsys, +): + """The refusal is not compared with a phrase written here. The pre-story + coordinator is run against the same finished run directory and its stderr + is compared byte for byte with the current one.""" + code, _ = run(target, harness_root, {"implementer": [edits_the_module]}) + assert code == 0 + capsys.readouterr() + + again = story_coordinator.run_story( + STORY_ID, harness_root, target, Runner(target)) + now = capsys.readouterr().err + before_module = pre_story_coordinator(tmp_path) + strip_new_fields(target) + was = before_module.run_story(STORY_ID, harness_root, target, Runner(target)) + then = capsys.readouterr().err + + assert again == was == 1 + assert now == then + assert str(run_dir_of(target)) in now + assert f"story/{STORY_ID}" in now + assert "completed" in now + + +def test_the_completed_refusal_starts_no_agent_and_the_escalated_one_does( + target, harness_root, capsys, +): + """The narrowing, as behavior: the same coordinator, the same run + directory, and the only difference is the status recorded in it.""" + escalate(target, harness_root) + change_the_code(target) + write_state(target, status="completed") + + refused = Runner(target) + assert story_coordinator.run_story( + STORY_ID, harness_root, target, refused) == 1 + assert refused.calls == [] + + write_state(target, status="escalated") + code, resumed = run(target, harness_root, verdicts=[PASS]) + assert code == 0 + assert resumed.calls != [] + + +def test_the_pre_story_coordinator_refused_the_run_this_one_resumes( + target, harness_root, tmp_path, capsys, +): + """What "narrowed" means, read against the code it narrowed. The same + escalated run directory: the pre-story coordinator refuses it and starts + nothing, the current one resumes it.""" + escalate(target, harness_root) + change_the_code(target) + before_module = pre_story_coordinator(tmp_path) + + refused = Runner(target) + original = strip_new_fields(target) + assert before_module.run_story( + STORY_ID, harness_root, target, refused) == 1 + assert refused.calls == [] + restore_state(target, original) + + code, resumed = run(target, harness_root, verdicts=[PASS]) + assert code == 0 + assert resumed.calls[0] == VERIFIER_STAGE["name"] + + +def test_a_crashed_run_still_resumes_exactly_as_it_did(target, harness_root): + """The status this story did not touch. A run left `running` resumes with + no guard and no archive — the archive belongs to the escalation path, and + a crashed run's interrupted attempt is its own unfinished work.""" + crashed = Runner(target, {"implementer": [edits_the_module]}) + run_dir_of(target).mkdir(parents=True, exist_ok=True) + story_coordinator.save_state( + run_dir_of(target), + story_coordinator.RunState(story_id=STORY_ID, branch=f"story/{STORY_ID}", + status="running", current_stage="tester"), + ) + + code, _ = run(target, harness_root, runner=crashed, verdicts=[PASS]) + + assert code == 0 + assert crashed.calls[0] == "tester" + assert not (run_dir_of(target) / "attempts").exists() + + +# -------------------------------------------------------------------------- +# The counters are carried forward, and the escalated attempt survives +# -------------------------------------------------------------------------- + + +def test_a_resumed_run_carries_the_counters_and_preserves_the_attempt( + target, harness_root, +): + """The whole point of not reinitializing: the escalated attempt's rendered + prompt and its verification iteration are still there, unmodified, after + the resumed run has finished.""" + escalate(target, harness_root, AFTER_A_RETRY) + escalated = state_of(target) + assert escalated["retry_count"] == 1 + assert escalated["verification_iterations"] == 2 + + run_dir = run_dir_of(target) + prompt = (run_dir / "prompt-verifier-attempt-2.md").read_text() + iteration_1 = (run_dir / "verification" / "iteration-1.json").read_text() + iteration_2 = (run_dir / "verification" / "iteration-2.json").read_text() + + change_the_code(target) + code, resumed = run(target, harness_root, verdicts=[PASS]) + assert code == 0 + + state = state_of(target) + assert state["retry_count"] == 1 + assert state["verification_iterations"] == 3 + assert (run_dir / "verification" / "iteration-1.json").read_text() == iteration_1 + assert (run_dir / "verification" / "iteration-2.json").read_text() == iteration_2 + assert json.loads( + (run_dir / "verification" / "iteration-3.json").read_text()) == PASS + # The rendered prompt of the interrupted attempt, kept where the archive + # put it: the resumed stage re-renders under the same attempt number and + # writes over the copy at the run root, which is why the archive exists. + assert (story_coordinator.attempt_dir(run_dir, 2) + / "prompt-verifier-attempt-2.md").read_text() == prompt + assert resumed.calls == [VERIFIER_STAGE["name"], "documenter"] + + +def test_resetting_the_counters_would_have_overwritten_that_evidence( + target, harness_root, +): + """The control for the test above, and the reason carrying the counters + forward is a constraint rather than a convenience. + + The same escalated run, resumed with `verification_iterations` reset the + way a reinitializing resume would leave it: the resumed verifier writes + iteration-1.json, and the escalated attempt's verdict is gone. + """ + escalate(target, harness_root) + run_dir = run_dir_of(target) + iteration_1 = json.loads((run_dir / "verification" / "iteration-1.json").read_text()) + assert iteration_1 == FAIL_AT_ONCE + + change_the_code(target) + write_state(target, verification_iterations=0) + code, _ = run(target, harness_root, verdicts=[PASS]) + + assert code == 0 + assert json.loads( + (run_dir / "verification" / "iteration-1.json").read_text()) == PASS + assert not (run_dir / "verification" / "iteration-2.json").exists() + + +# -------------------------------------------------------------------------- +# The interrupted attempt is archived, and never written over +# -------------------------------------------------------------------------- + + +def test_the_interrupted_attempt_is_archived_before_the_resumed_stage_runs( + target, harness_root, +): + """Observed at the entry to the resumed stage rather than after the run, + so "before" is a fact about ordering.""" + escalate(target, harness_root) + run_dir = run_dir_of(target) + assert not (run_dir / "attempts").exists() + escalated_verdict = (run_dir / "verification-result.json").read_text() + escalated_prompt = (run_dir / "prompt-verifier-attempt-1.md").read_text() + + change_the_code(target) + code, resumed = run(target, harness_root, verdicts=[PASS]) + assert code == 0 + + first_stage, archives_at_entry = resumed.archives_seen[0] + assert first_stage == VERIFIER_STAGE["name"] + assert archives_at_entry == ["attempt-1"] + + archived = story_coordinator.attempt_dir(run_dir, 1) + assert (archived / "verification-result.json").read_text() == escalated_verdict + assert (archived / "prompt-verifier-attempt-1.md").read_text() == escalated_prompt + # The stage artifacts the workflow declares are archived too, so the + # archive is the attempt rather than the prompts alone. + assert (archived / "changed-files.json").is_file() + assert (archived / "test-results.json").is_file() + + +def test_the_archive_carries_every_stage_artifact_the_workflow_declares(): + """The archive list is derived, not written: the prompts are the addition, + and the control is that every declared stage artifact is still in it.""" + stages = WORKFLOW["stages"] + listed = story_coordinator.interrupted_attempt_artifacts(stages, 3) + + for artifact in story_coordinator.archivable_artifacts(stages): + assert artifact in listed + for stage in stages: + assert f"prompt-{stage['name']}-attempt-3.md" in listed + assert f"prompt-{stages[0]['name']}-attempt-2.md" not in listed + + +def test_a_resume_whose_archive_directory_exists_refuses_naming_it( + target, harness_root, capsys, +): + """The case story-010 recorded as open. The control is the identical + resume in a repository where the directory does not exist, which proceeds + — so the refusal is about the directory rather than about resuming.""" + escalate(target, harness_root) + change_the_code(target) + occupied = story_coordinator.attempt_dir(run_dir_of(target), 1) + occupied.mkdir(parents=True) + write(occupied / "verification-result.json", "hand-written evidence\n") + capsys.readouterr() + + blocked = Runner(target) + code = story_coordinator.run_story(STORY_ID, harness_root, target, blocked) + message = capsys.readouterr().err + + assert code == 1 + assert blocked.calls == [] + assert str(occupied) in message + assert (occupied / "verification-result.json").read_text() \ + == "hand-written evidence\n" + assert state_of(target)["status"] == "escalated" + + elsewhere = build_target(target.parent / "unoccupied") + escalate(elsewhere, harness_root) + change_the_code(elsewhere) + proceeded, runner = run(elsewhere, harness_root, verdicts=[PASS]) + assert proceeded == 0 + assert runner.calls != [] + + +# -------------------------------------------------------------------------- +# The stage argument +# -------------------------------------------------------------------------- + + +def test_a_stage_argument_overrides_the_recorded_stage(target, harness_root): + """The control is the same resume with no argument, which enters at the + stage state.json records — so the override is the argument's doing.""" + escalate(target, harness_root) + assert state_of(target)["current_stage"] == VERIFIER_STAGE["name"] + change_the_code(target) + + code, overridden = run(target, harness_root, verdicts=[PASS], + edits={"implementer": [edits_the_module]}, + start_stage=RETRY_STAGE) + + assert code == 0 + assert overridden.calls[0] == RETRY_STAGE + assert overridden.calls == [RETRY_STAGE, "tester", VERIFIER_STAGE["name"], + "documenter"] + + elsewhere = build_target(target.parent / "no-override") + escalate(elsewhere, harness_root) + change_the_code(elsewhere) + assert run(elsewhere, harness_root, verdicts=[PASS])[1].calls[0] \ + == VERIFIER_STAGE["name"] + + +def test_a_stage_the_workflow_does_not_define_refuses_and_creates_nothing( + target, harness_root, capsys, +): + undefined = "reviewer" + assert undefined not in STAGE_NAMES + runner = Runner(target) + + code = story_coordinator.run_story( + STORY_ID, harness_root, target, runner, start_stage=undefined) + message = capsys.readouterr().err + + assert code == 1 + assert runner.calls == [] + assert not run_dir_of(target).exists() + assert undefined in message + for name in STAGE_NAMES: + assert name in message + # The control: the same call naming a stage the workflow does define runs. + accepted = Runner(target, {"implementer": [edits_the_module]}) + assert story_coordinator.run_story( + STORY_ID, harness_root, target, accepted, start_stage=STAGE_NAMES[0]) == 0 + assert accepted.calls != [] + assert run_dir_of(target).is_dir() + + +def test_a_fresh_run_started_at_a_later_stage_records_that_stage( + target, harness_root, +): + """The argument is not resume-only: on a fresh run it is where the + workflow is entered. The control is a fresh run with no argument, which + enters at the first stage.""" + code, started = run(target, harness_root, verdicts=[PASS], + start_stage=VERIFIER_STAGE["name"]) + assert code == 0 + assert started.calls == [VERIFIER_STAGE["name"], "documenter"] + + elsewhere = build_target(target.parent / "fresh-default") + assert run(elsewhere, harness_root, verdicts=[PASS])[1].calls[0] \ + == STAGE_NAMES[0] + + +def load_l5_run(): + loader = importlib.machinery.SourceFileLoader( + "l5_run_script", str(REPO_ROOT / "scripts" / "l5-run")) + spec = importlib.util.spec_from_loader(loader.name, loader) + module = importlib.util.module_from_spec(spec) + loader.exec_module(module) + return module + + +def test_l5_run_passes_the_stage_through_and_decides_nothing_itself( + target, monkeypatch, +): + """The script stays a thin entry point: it forwards whatever it was given, + including a stage the workflow does not define, and the refusal is the + coordinator's. The control is the invocation with no argument, which + forwards None rather than a default of its own.""" + script = load_l5_run() + seen: list[dict] = [] + + def spy(story_id, harness_root, target_root, runner=None, start_stage=None): + seen.append({"story_id": story_id, "start_stage": start_stage}) + return 0 + + monkeypatch.setattr(story_coordinator, "run_story", spy) + monkeypatch.setattr(harness_config, "find_target_root", lambda cwd: target) + + monkeypatch.setattr(sys, "argv", ["l5-run", STORY_ID, "--stage", RETRY_STAGE]) + assert script.main() == 0 + monkeypatch.setattr(sys, "argv", ["l5-run", STORY_ID, "--stage", "reviewer"]) + assert script.main() == 0 + monkeypatch.setattr(sys, "argv", ["l5-run", STORY_ID]) + assert script.main() == 0 + + assert [call["start_stage"] for call in seen] == [RETRY_STAGE, "reviewer", None] + assert {call["story_id"] for call in seen} == {STORY_ID} + + +def test_l5_run_still_refuses_an_argument_list_it_cannot_read(target, monkeypatch): + script = load_l5_run() + started: list = [] + monkeypatch.setattr(story_coordinator, "run_story", + lambda *a, **k: started.append(a) or 0) + monkeypatch.setattr(harness_config, "find_target_root", lambda cwd: target) + + for argv in (["l5-run"], ["l5-run", STORY_ID, "extra"], + ["l5-run", STORY_ID, "--stage"]): + monkeypatch.setattr(sys, "argv", argv) + assert script.main() == 1, argv + assert started == [] + + +# -------------------------------------------------------------------------- +# The unchanged guard, in both directions +# -------------------------------------------------------------------------- + + +def guard(target_root: Path, harness: Path = REPO_ROOT, + story_text: str | None = None) -> list[str]: + state = story_coordinator.load_state(run_dir_of(target_root)) + if story_text is None: + story_text = (target_root / ".harness" / "stories" + / f"{STORY_ID}.yaml").read_text() + return story_coordinator.unchanged_since_escalation( + state, story_text, target_root, harness) + + +#: Every guard test below runs against `quiet_target`, whose run directory is +#: ignored, because that is the only shape in which an escalation currently +#: leaves the clean tree the guard's second comparison requires. The tracked +#: shape — the one the harness itself sets up — is the subject of +#: `test_the_refusal_fires_in_a_repository_that_tracks_its_run_directory`. + + +def test_a_resume_with_nothing_changed_refuses_naming_the_reason_and_the_fix( + quiet_target, harness_root, capsys, +): + escalate(quiet_target, harness_root) + summary = (run_dir_of(quiet_target) / "escalation-summary.md").read_text() + reason = summary.split("## Reason", 1)[1].split("##", 1)[0].strip() + capsys.readouterr() + + refused = Runner(quiet_target) + code = story_coordinator.run_story( + STORY_ID, harness_root, quiet_target, refused) + message = capsys.readouterr().err + + assert code == 1 + assert refused.calls == [] + assert reason in message + assert str(quiet_target / ".harness" / "stories" / f"{STORY_ID}.yaml") in message + assert f"story/{STORY_ID}" in message + assert state_of(quiet_target)["status"] == "escalated" + assert not (run_dir_of(quiet_target) / "attempts").exists() + + +def test_the_refusal_fires_in_a_repository_that_tracks_its_run_directory( + target, harness_root, capsys, +): + """The same refusal in the shape `l5-init` produces, where the run + directory is a tracked part of the repository — this repository's own + `.harness/runs/` is tracked, and so is the target fixture's. + + The control is `test_a_resume_with_nothing_changed_refuses_...` above, + which is the identical run in the identical state with the run directory + ignored: if the guard's logic were wrong, that one would fail too. + """ + escalate(target, harness_root) + capsys.readouterr() + + refused = Runner(target) + code = story_coordinator.run_story(STORY_ID, harness_root, target, refused) + + assert code == 1 + assert refused.calls == [] + + +def test_amending_the_story_clears_the_refusal(quiet_target, harness_root): + """The story is the input the guard names first, and amending it is the + response the message asks for. The control is the run immediately above, + which refused with the same run directory and the story untouched.""" + escalate(quiet_target, harness_root) + assert guard(quiet_target, harness_root) != [] + + amend_the_story(quiet_target) + assert guard(quiet_target, harness_root) == [] + + code, resumed = run(quiet_target, harness_root, verdicts=[PASS]) + assert code == 0 + assert resumed.calls[0] == VERIFIER_STAGE["name"] + + +def test_each_input_alone_clears_the_guard(quiet_target, harness_root): + """One comparison at a time, so no single input can be the only one + carrying the decision. The positive case is asserted first and is the + control for all three.""" + target = quiet_target + escalate(target, harness_root) + story_text = (target / ".harness" / "stories" / f"{STORY_ID}.yaml").read_text() + assert len(guard(target, harness_root)) == 3 + + # The story artifact, changed with the tree and the harness untouched. + assert guard(target, harness_root, story_text + "\n# amended\n") == [] + + # The harness, at a revision the run did not record. + assert guard(target, harness_root) != [] + write_state(target, harness_revision="0" * 40) + assert guard(target, harness_root) == [] + write_state(target, harness_revision=git(REPO_ROOT, "rev-parse", "HEAD").stdout.strip()) + assert guard(target, harness_root) != [] + + # The tree, dirtied without moving HEAD. + change_the_code(target) + assert guard(target, harness_root) == [] + + +def test_a_developer_commit_after_the_escalation_clears_the_guard( + quiet_target, harness_root, +): + """The distinction the recorded commit exists to draw, driven rather than + reasoned about: an escalation the harness committed, and the same run after + a developer has committed on top of it. + + The tree is clean in both cases and the story and harness are untouched in + both, so the branch comparison is the only thing that can tell them apart. + It matters here more than it reads: the guard compares the recorded commit + against the tip's *parent*, and a developer commit moves the tip — the + parent it then finds is the escalation's own work commit, not the recorded + one. The control is the assertion immediately before the commit is made, + which does refuse. + """ + target = quiet_target + escalate(target, harness_root) + assert guard(target, harness_root) != [] # the control + + change_the_code(target) + git(target, "add", "-A") + git(target, "commit", "-q", "-m", "a developer's fix on the story branch") + assert git(target, "status", "--porcelain").stdout.strip() == "" + + assert guard(target, harness_root) == [] + + +def test_the_guard_refuses_only_on_evidence_it_can_establish( + quiet_target, harness_root, +): + """Anything not established counts as not-the-same. Each of the three + records is emptied in turn, which is what a state file written before this + story, an escalation that committed nothing, and a harness that is not a + repository all look like.""" + target = quiet_target + escalate(target, harness_root) + assert guard(target, harness_root) != [] # the control + + for field in ("story_digest", "escalation_commit", "harness_revision"): + recorded = state_of(target)[field] + write_state(target, **{field: ""}) + assert guard(target, harness_root) == [], field + write_state(target, **{field: recorded}) + assert guard(target, harness_root) != [] + + +def test_a_harness_root_that_is_not_a_repository_produces_no_false_refusal( + quiet_target, tmp_path, +): + """The escalation records "" for a harness it cannot resolve, and the + guard reads that as not-established rather than as a value to compare. + + The control is the identical run against the real harness root, which does + refuse — so what differs is the establishability of the harness revision. + """ + target = quiet_target + fake = tmp_path / "harness-not-a-repo" + (fake / "workflows").mkdir(parents=True) + for shared in ("prompts", "schemas", "rules"): + (fake / shared).symlink_to(REPO_ROOT / shared) + write(fake / "workflows" / "story-workflow.json", + json.dumps(WORKFLOW, indent=2)) + assert story_coordinator._revision(fake) == "" + assert story_coordinator._revision(REPO_ROOT) != "" + + escalate(target, fake) + assert state_of(target)["harness_revision"] == "" + assert guard(target, fake) == [] + + code, resumed = run(target, fake, verdicts=[PASS]) + assert code == 0 + assert resumed.calls[0] == VERIFIER_STAGE["name"] + + elsewhere = build_target(target.parent / "real-harness", + gitignore=".harness/runs/\n") + escalate(elsewhere, REPO_ROOT) + assert guard(elsewhere, REPO_ROOT) != [] # the control + + +def test_the_digest_neither_authorizes_nor_triggers_a_resume( + target, harness_root, capsys, +): + """An amended story does not restart a finished run, and does not make a + run resumable that its status does not. Routing is on the status alone. + + The control is the same amended story against the same run directory with + the status put back, which does resume. + """ + escalate(target, harness_root) + amend_the_story(target) + write_state(target, status="completed") + capsys.readouterr() + + refused = Runner(target) + assert story_coordinator.run_story( + STORY_ID, harness_root, target, refused) == 1 + assert refused.calls == [] + assert "completed" in capsys.readouterr().err + + write_state(target, status="escalated") + assert run(target, harness_root, verdicts=[PASS])[1].calls != [] + + +def test_no_line_in_run_story_routes_on_the_digest(harness_root): + """The digest informs a message; it is never a branch, a reroute or a + return. The control is the status, which is routed on in the same + function — so a scan that had stopped matching would fail here first.""" + lines = executable_source( + inspect.getsource(story_coordinator.run_story)).splitlines() + + def routing(name: str) -> list[str]: + return [line for line in lines if name in line + and re.search(r"\b(if|elif|else|return|continue)\b|index\s*=", line)] + + assert routing("status") + assert routing("digest") == [] + + +# -------------------------------------------------------------------------- +# state.json is the only routing source +# -------------------------------------------------------------------------- + + +def test_the_resumed_stage_comes_from_state_json(target, harness_root): + """Changed in state.json and nowhere else, the resume enters somewhere + else. Nothing else in the run directory was touched.""" + escalate(target, harness_root) + assert state_of(target)["current_stage"] == VERIFIER_STAGE["name"] + change_the_code(target) + write_state(target, current_stage="documenter") + + code, resumed = run(target, harness_root) + + assert code == 0 + assert resumed.calls == ["documenter"] + + +def test_nothing_routes_on_the_summary_the_archive_or_the_baseline( + target, harness_root, +): + """All three removed from the run directory, and the resume routes + identically to the run below that keeps them. + + The escalation summary is read for the refusal message only, so removing + it costs that message a sentence; here the guard is already cleared and it + costs nothing at all. + """ + escalate(target, harness_root) + run_dir = run_dir_of(target) + change_the_code(target) + (run_dir / "escalation-summary.md").unlink() + subprocess.run(["rm", "-rf", str(run_dir / BASELINE)], check=True) + + code, stripped = run(target, harness_root, verdicts=[PASS]) + + assert code == 0 + assert stripped.calls == [VERIFIER_STAGE["name"], "documenter"] + + intact = build_target(target.parent / "intact") + escalate(intact, harness_root) + change_the_code(intact) + code, kept = run(intact, harness_root, verdicts=[PASS]) + assert code == 0 + assert kept.calls == stripped.calls + + +def test_removing_the_summary_costs_the_refusal_a_sentence_and_no_decision( + quiet_target, harness_root, capsys, +): + """The other half: with the guard *not* cleared, the run still refuses + with the summary gone — the decision came from state.json — and only the + reason drops out of the message.""" + target = quiet_target + escalate(target, harness_root) + summary = (run_dir_of(target) / "escalation-summary.md").read_text() + reason = summary.split("## Reason", 1)[1].split("##", 1)[0].strip() + capsys.readouterr() + + assert story_coordinator.run_story(STORY_ID, harness_root, target, + Runner(target)) == 1 + with_summary = capsys.readouterr().err + + (run_dir_of(target) / "escalation-summary.md").unlink() + assert story_coordinator.run_story(STORY_ID, harness_root, target, + Runner(target)) == 1 + without_summary = capsys.readouterr().err + + assert reason in with_summary + assert reason not in without_summary + assert f"story/{STORY_ID}" in without_summary + + +def test_a_resumed_stage_reuses_the_baseline_recorded_for_it( + target, harness_root, +): + """story-019's capture-once-reuse rule, inherited rather than + re-implemented. A run escalated *inside* the implementer is resumed there + with that stage's edits already in the tree; the baseline it decides + against is the one taken before the stage first ran. + + The control is the same baseline recaptured after the edit, which holds + the edited content — so the reuse above is the keying rather than a + capture that has stopped reading the tree. + """ + class Incomplete(Runner): + """An implementer that edits the tree and writes no artifacts, so the + run escalates inside the stage that made the edit.""" + + def __call__(self, prompt, *, stage, **kwargs): + self.calls.append(stage) + write(self.target_root / "tests" / "test_existing.py", + TEST_AT_HEAD + "\n\ndef test_added():\n assert True\n") + return AgentResult(ok=True, result_text="no artifacts") + + assert story_coordinator.run_story( + STORY_ID, harness_root, target, Incomplete(target)) == 2 + run_dir = run_dir_of(target) + captured = story_coordinator.stage_baseline_dir( + run_dir, BASELINE, IMPLEMENTER_STAGE["name"], 1) + assert (captured / "tests" / "test_existing.py").read_text() == TEST_AT_HEAD + + change_the_code(target) + code, _ = run(target, harness_root, verdicts=[PASS], + edits={"implementer": [edits_the_module]}) + assert code == 0 + + assert (captured / "tests" / "test_existing.py").read_text() == TEST_AT_HEAD + recaptured = story_coordinator.capture_stage_baseline( + run_dir, target, BASELINE, IMPLEMENTER_STAGE["name"], 99, + IMPLEMENTER_STAGE["may_not_create"]) + assert (recaptured / "tests" / "test_existing.py").read_text() != TEST_AT_HEAD + + +# -------------------------------------------------------------------------- +# The event stream reconstructs the whole run +# -------------------------------------------------------------------------- + + +def test_one_resumed_event_names_the_stage_in_both_renderings( + target, harness_root, +): + escalate(target, harness_root) + change_the_code(target) + run(target, harness_root, verdicts=[PASS]) + + resumed = [line for line in messages(target) if line.startswith("resumed")] + entries = [entry for entry in history(target) if entry["event"] == "resumed"] + + assert len(resumed) == 1 + assert VERIFIER_STAGE["name"] in resumed[0] + assert len(entries) == 1 + assert entries[0]["message"] == resumed[0] + assert entries[0]["stage"] == VERIFIER_STAGE["name"] + + +def test_the_whole_run_including_the_escalation_and_the_resume_reconstructs( + target, harness_root, +): + """One stream, one run: the escalation and the resume are both in it, in + order, and the sequence numbers are contiguous across the two invocations. + + The control is the ordering assertion itself — a stream that had lost the + escalation or the resume could not satisfy it. + """ + escalate(target, harness_root, AFTER_A_RETRY) + change_the_code(target) + assert run(target, harness_root, verdicts=[PASS])[0] == 0 + + entries = history(target) + assert [entry["sequence"] for entry in entries] == list(range(1, len(entries) + 1)) + kinds = [entry["event"] for entry in entries] + for kind in ("workflow-started", "verification-failed", "escalated", + "resumed", "verification-passed", "story-completed"): + assert kind in kinds, kind + assert kinds.index("escalated") < kinds.index("resumed") + assert kinds.index("resumed") < kinds.index("story-completed") + assert kinds[-1] == "story-completed" + assert [entry["message"] for entry in entries] == messages(target) + + assert (run_dir_of(target) / "completion-report.md").is_file() + assert COMPLETION_SUBJECT.match(subject_of(target)) + # The escalation commit is left in place; the completion commits on top. + escalation = state_of(target)["escalation_commit"] + assert git(target, "merge-base", "--is-ancestor", escalation, "HEAD", + check=False).returncode == 0 + + +# -------------------------------------------------------------------------- +# state.json's new fields, and the files written before them +# -------------------------------------------------------------------------- + +#: The fields state.json carried before this story, read out of the pre-story +#: dataclass in the test below rather than written here. +NEW_FIELDS = {"story_digest", "escalation_commit", "harness_revision"} + + +def test_a_state_file_written_before_this_story_still_loads(target, tmp_path): + """Loaded rather than reasoned about: the pre-story module writes the + file, and the current module reads it. + + The control is a field neither module declares, which still fails to load + — so the tolerance above is the defaults rather than a loader that has + stopped checking anything. + """ + before_module = pre_story_coordinator(tmp_path) + run_dir = run_dir_of(target) + run_dir.mkdir(parents=True) + before_module.save_state(run_dir, before_module.RunState( + story_id=STORY_ID, branch=f"story/{STORY_ID}", status="escalated", + current_stage=VERIFIER_STAGE["name"], retry_count=1, + verification_iterations=2)) + + written = json.loads((run_dir / "state.json").read_text()) + assert set(written) & NEW_FIELDS == set() + + loaded = story_coordinator.load_state(run_dir) + assert loaded.status == "escalated" + assert loaded.retry_count == 1 + assert loaded.verification_iterations == 2 + assert (loaded.story_digest, loaded.escalation_commit, + loaded.harness_revision) == ("", "", "") + + (run_dir / "state.json").write_text( + json.dumps({**written, "invented": "field"}, indent=2), encoding="utf-8") + with pytest.raises(TypeError): + story_coordinator.load_state(run_dir) + + +def test_a_run_escalated_before_this_story_resumes_without_a_false_refusal( + target, harness_root, tmp_path, +): + """The upgrade path, end to end: a run left escalated by the pre-story + coordinator has none of the three records, so the guard establishes + nothing and the resume proceeds.""" + escalate(target, harness_root) + run_dir = run_dir_of(target) + old = {key: value for key, value in state_of(target).items() + if key not in NEW_FIELDS} + (run_dir / "state.json").write_text(json.dumps(old, indent=2) + "\n", + encoding="utf-8") + + code, resumed = run(target, harness_root, verdicts=[PASS]) + + assert code == 0 + assert resumed.calls[0] == VERIFIER_STAGE["name"] + + +def test_the_new_fields_are_written_by_every_run_and_default_to_empty( + target, harness_root, +): + """A fresh run records the digest at the start and leaves the other two + empty until it ends. The control is the escalated run, where all three are + populated.""" + code, _ = run(target, harness_root, {"implementer": [edits_the_module]}) + assert code == 0 + completed = state_of(target) + assert NEW_FIELDS <= set(completed) + assert completed["story_digest"] + assert completed["escalation_commit"] == "" + assert completed["harness_revision"] == "" + + elsewhere = build_target(target.parent / "escalated-fields") + escalate(elsewhere, harness_root) + escalated = state_of(elsewhere) + assert all(escalated[field] for field in NEW_FIELDS) + + +# -------------------------------------------------------------------------- +# The limit this story states rather than closes +# -------------------------------------------------------------------------- + + +def test_neither_terminal_commit_establishes_what_it_commits( + target, harness_root, +): + """A property of the coordinator, not of one commit: both terminal commits + stage the working tree rather than the run's own work. + + This is the one test here that asserts a limit rather than a guarantee, so + it is written to go *red* when the limit closes: + `.harness/requests/commit-only-what-the-run-produced.md` is the story that + closes it, and when it lands the stray file below stops being committed + and both halves fail. Repoint them there rather than deleting them. + + The control is the same tree committed the narrow way — only the paths the + stage recorded — which does not carry the stray file. That is what makes + the assertion a statement about what the coordinator stages rather than a + statement about the file existing at all. + """ + stray = "stray-nothing-produced-this.txt" + write(target / stray, "no stage wrote this\n") + escalate(target, harness_root) + + assert stray in files_in(target) + + # The control: staged the narrow way, the same tree yields no stray file. + narrow = build_target(target.parent / "narrow") + write(narrow / stray, "no stage wrote this\n") + edits_the_module(narrow, 1) + git(narrow, "add", "--", "src/app.py") + git(narrow, "commit", "-q", "-m", "only what was recorded") + assert "src/app.py" in files_in(narrow) + assert stray not in files_in(narrow) + + +def test_the_completion_commit_stages_the_same_way(target, harness_root): + """The other half of the same limit, so the statement is about both + terminal commits and neither can close alone without this going red.""" + stray = "stray-nothing-produced-this.txt" + write(target / stray, "no stage wrote this\n") + + code, _ = run(target, harness_root, {"implementer": [edits_the_module]}) + + assert code == 0 + assert stray in files_in(target) + + +def test_the_coordinator_states_that_limit_where_the_commits_are_made(): + """Stated in the coordinator as a property of the coordinator, so the + story that closes it repoints one statement rather than hunting for + prose. The control is that the same search finds nothing in a rendering + with the prose stripped out.""" + source = (REPO_ROOT / "orchestration" / "story_coordinator.py").read_text( + encoding="utf-8") + stated = [line for line in source.splitlines() + if "working tree" in line and ("add -A" in line or "stage" in line)] + assert stated + assert [line for line in executable_source(source).splitlines() + if "working tree" in line and "add -A" in line] == [] + + +# -------------------------------------------------------------------------- +# What this story left alone +# -------------------------------------------------------------------------- + + +def test_this_story_edited_no_blocked_path_and_added_no_artifact(harness_root): + """The control is the file the story did edit: if the diff resolution had + stopped seeing anything, the last assertion would fail too.""" + validation = Path(__file__) + for path in ("rules/", "workflows/", "schemas/", "prompts/", + ".harness/stories/"): + assert story_diff([path], validation_file=validation) == "", path + assert story_diff(["orchestration/story_coordinator.py"], + validation_file=validation) != "" + + +def test_the_escalation_summary_is_the_text_it_was(tmp_path): + """A separate request owns its content, sequenced after this story. The + control is `_escalate`'s own source, which did change in the same file.""" + before_module = pre_story_coordinator(tmp_path) + summary_body = inspect.getsource(story_coordinator._escalate).split( + "summary = (", 1)[1] + before_body = inspect.getsource(before_module._escalate).split( + "summary = (", 1)[1] + assert summary_body == before_body + assert inspect.getsource(story_coordinator._escalate) \ + != inspect.getsource(before_module._escalate)