diff --git a/.harness/docs/ARCHITECTURE.md b/.harness/docs/ARCHITECTURE.md index ca9e31b..84e81d6 100644 --- a/.harness/docs/ARCHITECTURE.md +++ b/.harness/docs/ARCHITECTURE.md @@ -372,7 +372,8 @@ story-031 escalated seventeen minutes into its implementer on a conflict that wa revert-check-result.json the revert check's record, coordinator-written; absent when the stage's record named no governed path stage-baseline/ what the tree held under a stage's governed prefixes - implementer-attempt-1/ before that stage ran; file copies, one dir per attempt + implementer/ when that stage first ran; file copies, one dir per + stage, merged first-seen-wins across attempts attempts/attempt-1/ superseded attempt's artifacts, canonical filenames retry-history.json one entry per retry taken; absent when none was completion-report.md or escalation-summary.md @@ -385,12 +386,12 @@ Three artifacts describe a retry and none of them substitutes for another. `retr `clean-clone-result.json` is absent from the archive for a different reason, and it is a known cost rather than a decision. `archivable_artifacts` reads the three places a stage names artifacts — `outputs`, `changed_files`, and the keys of `schemas` — and the clean-clone declaration is none of those, so a second attempt's record overwrites the first's. The event stream still carries that a first clean-clone failure happened; only its captured output is lost. Widening the declaration is the fix if that ever costs a debugging session. `revert-check-result.json` is absent from the archive for exactly the same reason and on the same terms — a known cost carried deliberately, not an oversight — though it bites less: the revert check escalates rather than retrying, so a run that writes the record twice is a run that was permitted the first time. -`stage-baseline/` is the odd one in this directory, and it is worth saying exactly what it is and is not. It holds **file copies, not JSON**: one directory per stage and attempt, `-attempt-N/`, mirroring repository-relative paths, holding the content of every file under that stage's declared `may_not_create` prefixes as it stood immediately before the stage agent was invoked. N is the same attempt number the rendered prompts and `attempts/attempt-N/` use, so `prompt-implementer-attempt-2.md`, `attempts/attempt-2/` and `stage-baseline/implementer-attempt-2/` describe one attempt from three sides — what the stage was asked, what it produced, and what it started from. Both the directory name and the stage it is captured for come off the workflow declaration; `capture_stage_baseline` names no stage and no prefix. +`stage-baseline/` is the odd one in this directory, and it is worth saying exactly what it is and is not. It holds **file copies, not JSON**: one directory per **stage**, `/`, mirroring repository-relative paths, holding the content of every file under that stage's declared `may_not_create` prefixes as it stood when that stage *first* ran. It carries no attempt component, and since story-037 no attempt number is derived for it anywhere — the directory answers "what did this stage find", a question the run has one answer to however many attempts it takes. Both the directory name and the stage it is captured for come off the workflow declaration; `stage_baseline_dir(run_dir, baseline, stage_name)` and `capture_stage_baseline(run_dir, target_root, baseline, stage_name, prefixes)` name no stage and no prefix. 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 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. +- **First seen wins, per path.** A capture merges into the stage's directory: a path the baseline already holds keeps the content it was first captured with, and a path new since the last capture is added at its current content. Nothing is ever overwritten. 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 this rule is the whole of what prevents it. **Per path rather than per directory**, and the difference is not cosmetic: reusing an earlier capture's directory whole leaves a governed path that first *appeared* between two invocations of the stage — a test file another stage created in the meantime — absent from the baseline, and a path absent from the baseline is **deleted** in the clone rather than restored, because absent means it did not exist when the stage started. The merge holds both halves; per-directory reuse holds only the first. See the story-037 bullet under "Decisions and constraints" for the run that made this concrete. - **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. @@ -460,6 +461,9 @@ It is narrower than Chapter 18's **checkpoints**, and the difference is the reas - 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 -- ` 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; 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 baseline is what the *stage* first found, and keying it by attempt made it what the *attempt* found — which is a different question, and the wrong one.** story-019 got the baseline right and story-037 got its scope right. Captured per stage *and per attempt*, the second attempt's capture already held the first attempt's edits, so reverting the second attempt's work rolled the tree back only as far as the first attempt's and decided the stage against itself. story-036 is the observed case and its run directory is the evidence: attempt 2's capture held 999 lines of `tests/test_story_022_validation.py` against attempt 1's 951 and 806 of `tests/test_coordinator_contract.py` against 800, the reverted clone's suite passed at 1958, and the run escalated on "the suite still passes with those edits reverted" — a permission refused on coverage the check had never actually tested. Any retry that edits a file it also edited on the previous attempt meets this, so it blocked retries generally, exactly as the HEAD baseline did one bullet up. **This is the second time the revert check reverted to the wrong state and the first time it was not the choice of state but the choice of key**; when a check has a "compared against what" parameter, the keying of that parameter is part of the semantics and not bookkeeping. With the directory keyed by stage alone the attempt number decides nothing, so it is gone from both signatures rather than left reading as if it does. +- **The fix had to be a per-path merge, and reusing the earlier attempt's directory wholesale would have traded one defect for the other.** The two cases pull in opposite directions: a path this stage edited on an earlier attempt must keep the content it had *before* that attempt (else the stage is decided against itself, story-036), and a path that first appeared between two invocations of this stage must be present at the content this stage first met it with (else the clone *deletes* it, since absent from the baseline means it did not exist when the stage started — story-019's motivating case, a test file another stage created in the meantime). Per-directory reuse holds the first and breaks the second. First-seen-wins per path holds both, and it is one rule rather than two, which is why the capture and not the check is where it lives. **Everything else about the check is untouched**: the decision rule is still permitted exactly when reverting makes the suite fail, only the state it reverts to changed; `_build_clone`'s restore semantics including the deletion of an unheld governed path are unchanged; the captured file set is still tracked-plus-untracked under each declared prefix; the directory is still created even when it records nothing, so a stage that declares the check with no baseline still escalates naming that; and the baseline is still evidence, absent from `state.json` and read by no routing decision. +- story-037's own run was not governed by the change it makes, for the eighth time in this pattern's history, by the stale-import cause rather than the stale-workflow one: the coordinator process imported `story_coordinator` before the keying changed in it, so this run's own baseline is `stage-baseline/implementer-attempt-1/`. Enforcement begins with the next run. Expected, stated in the story's constraints, and not a defect. - **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. diff --git a/orchestration/story_coordinator.py b/orchestration/story_coordinator.py index 2e59ac0..caa7462 100644 --- a/orchestration/story_coordinator.py +++ b/orchestration/story_coordinator.py @@ -1290,16 +1290,16 @@ def _clean_clone_failures(output: str) -> str: # -------------------------------------------------------------------------- -def stage_baseline_dir( - run_dir: Path, baseline: str, stage_name: str, attempt: int -) -> Path: - """Where one stage's pre-stage content for one attempt is kept. +def stage_baseline_dir(run_dir: Path, baseline: str, stage_name: str) -> Path: + """Where one stage's pre-stage content is kept. The directory name comes off the loaded workflow declaration; only the - keying by stage and attempt is written here, and it is the same attempt - number the rendered prompt filename and `attempts/attempt-N/` use. + keying is written here, and it is by stage alone because the baseline is + what that stage first found. An attempt-keyed directory made the second + attempt of a stage decide against the first attempt's own edits, which is + not the question the revert check asks. """ - return run_dir / baseline / f"{stage_name}-attempt-{attempt}" + return run_dir / baseline / stage_name def capture_stage_baseline( @@ -1307,7 +1307,6 @@ def capture_stage_baseline( target_root: Path, baseline: str, stage_name: str, - attempt: int, prefixes: list[str], ) -> Path: """Record what the tree held under a stage's governed prefixes before it ran. @@ -1318,9 +1317,19 @@ def capture_stage_baseline( committed is captured. Tracked files alone would miss exactly that file, which is the whole reason this exists. - Capture once, reuse afterwards: a directory already recorded for this stage - and attempt is returned untouched, so a re-entered stage is decided against - the state it originally found rather than against its own completed edits. + First seen wins, per path rather than per directory: a path the baseline + already holds keeps the content it was first captured with, and a path new + since the last capture is added at its current content. So a re-entered + stage is decided against what it originally found rather than against its + own completed edits. + + The merge is per path because reusing the earlier capture's directory whole + would get the second half wrong. A governed path that first appears between + two invocations of this stage — a test file another stage created in the + meantime — would be absent from the baseline, and a path absent from the + baseline is deleted in the clone rather than restored, because absent means + it did not exist when the stage started. + 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. @@ -1328,10 +1337,8 @@ def capture_stage_baseline( It names no stage and no prefix: both come from the loaded workflow. The result is evidence — nothing routes on it, and it is not in state.json. """ - directory = stage_baseline_dir(run_dir, baseline, stage_name, attempt) - if directory.exists(): - return directory - directory.mkdir(parents=True) + directory = stage_baseline_dir(run_dir, baseline, stage_name) + directory.mkdir(parents=True, exist_ok=True) for prefix in prefixes: listed = _git( target_root, @@ -1348,6 +1355,8 @@ def capture_stage_baseline( if not source.is_file(): continue destination = directory / rel + if destination.exists(): + continue destination.parent.mkdir(parents=True, exist_ok=True) shutil.copy2(source, destination) return directory @@ -2505,7 +2514,6 @@ def elapsed() -> float | None: target_root, declaration["baseline"], name, - attempt, stage.get("may_not_create", []), ) if declaration diff --git a/tests/test_story_017_validation.py b/tests/test_story_017_validation.py index 83d8609..5f018d5 100644 --- a/tests/test_story_017_validation.py +++ b/tests/test_story_017_validation.py @@ -442,7 +442,7 @@ def baseline_of(target_root: Path, scratch: Path, prefix: str = PREFIX) -> Path: Taken before the edit under test, which is where the coordinator takes it. """ return story_coordinator.capture_stage_baseline( - scratch, target_root, BASELINE, "stage", 1, [prefix]) + scratch, target_root, BASELINE, "stage", [prefix]) def suite_in(directory: Path) -> int: diff --git a/tests/test_story_019_validation.py b/tests/test_story_019_validation.py index cea443f..631d2e8 100644 --- a/tests/test_story_019_validation.py +++ b/tests/test_story_019_validation.py @@ -421,18 +421,18 @@ def run(target_root: Path, harness: Path, edits: dict | None = None, return code, runner -def baseline_at(target_root: Path, stage: str, attempt: int, +def baseline_at(target_root: Path, stage: str, story_id: str = "story-001") -> Path: - """The baseline the coordinator captured for one stage and attempt.""" + """The baseline the coordinator captured for one stage.""" return story_coordinator.stage_baseline_dir( - run_dir_of(target_root, story_id), BASELINE, stage, attempt) + run_dir_of(target_root, story_id), BASELINE, stage) def capture(target_root: Path, scratch: Path, prefix: str = PREFIX, - stage: str = "stage", attempt: int = 1) -> Path: + stage: str = "stage") -> Path: """Capture a baseline the way the coordinator captures one, into scratch.""" return story_coordinator.capture_stage_baseline( - scratch, target_root, BASELINE, stage, attempt, [prefix]) + scratch, target_root, BASELINE, stage, [prefix]) def suite_in(directory: Path) -> int: @@ -622,7 +622,7 @@ def test_a_forced_edit_to_a_file_created_earlier_in_the_run_is_permitted( assert record["permitted"] is True assert record["exit_code"] != 0 assert record["paths"] == ["tests/test_new.py"] - assert record["baseline"].endswith("implementer-attempt-2") + assert record["baseline"].endswith("implementer") def test_the_run_records_which_run_created_path_was_reverted_and_why( @@ -684,7 +684,7 @@ def test_the_baseline_of_the_retry_holds_the_file_the_tester_left( is the condition the check had to survive. """ assert run(target, harness_root, RETRY_SHAPE, [FAIL, PASS])[0] == 0 - captured = baseline_at(target, "implementer", 2) / "tests" / "test_new.py" + captured = baseline_at(target, "implementer") / "tests" / "test_new.py" assert captured.read_text() == TEST_NEW_BROKEN tracked = git(target, "ls-tree", "-r", "--name-only", "HEAD^", "--", @@ -965,26 +965,28 @@ def unbuildable(*args, **kwargs): # -------------------------------------------------------------------------- -def test_a_second_capture_for_the_same_stage_and_attempt_does_not_overwrite( +def test_a_second_capture_for_the_same_stage_does_not_overwrite( target, tmp_path, ): scratch = tmp_path / "run" - first = capture(target, scratch, stage="implementer", attempt=1) + first = capture(target, scratch, stage="implementer") assert (first / "tests" / "test_app.py").read_text() == TEST_APP_AT_HEAD forced_repair(target, run_dir_of(target)) - again = capture(target, scratch, stage="implementer", attempt=1) + again = capture(target, scratch, stage="implementer") assert again == first assert (again / "tests" / "test_app.py").read_text() == TEST_APP_AT_HEAD - # The control: a different attempt is a different baseline, and it does - # see the edit — so the reuse above is the keying, not a capture that has - # stopped reading the tree. - later = capture(target, scratch, stage="implementer", attempt=2) + # The control: a fresh capture, into a run directory holding no baseline + # for this stage, does see the edit — so the reuse above is the first-seen + # rule, not a capture that has stopped reading the tree. Since story-037 + # the directory is keyed by stage alone, so a fresh run directory is what + # distinguishes it rather than a different attempt number. + later = capture(target, tmp_path / "fresh", stage="implementer") assert (later / "tests" / "test_app.py").read_text() == TEST_APP_REPAIRED -def test_the_attempt_directory_exists_even_when_it_captures_nothing( +def test_the_baseline_directory_exists_even_when_it_captures_nothing( target, tmp_path, ): """Its existence answers "was a baseline taken", so an empty capture and @@ -1015,10 +1017,10 @@ def test_a_re_entered_stage_is_decided_against_the_baseline_it_first_found( assert resumed.calls[0] == "implementer" record = record_of(target) assert record["permitted"] is True - assert record["baseline"].endswith("implementer-attempt-1") + assert record["baseline"].endswith("implementer") # The baseline it decided against is the one taken before the first # invocation, not the state the interrupted stage left behind. - captured = baseline_at(target, "implementer", 1) / "tests" / "test_app.py" + captured = baseline_at(target, "implementer") / "tests" / "test_app.py" assert captured.read_text() == TEST_APP_AT_HEAD @@ -1069,7 +1071,7 @@ def test_with_the_declaration_the_same_run_captures_and_escalates( assert code == 2 run_dir = run_dir_of(target) assert (run_dir / ARTIFACT).exists() - assert (run_dir / BASELINE / "implementer-attempt-1").is_dir() + assert (run_dir / BASELINE / "implementer").is_dir() def test_moving_the_declaration_moves_the_capture_and_the_check( @@ -1091,9 +1093,9 @@ def test_moving_the_declaration_moves_the_capture_and_the_check( "tester": [module_only]}) assert code == 2 run_dir = run_dir_of(target) - assert (run_dir / BASELINE / "tester-attempt-1").is_dir() - assert not (run_dir / BASELINE / "implementer-attempt-1").exists() - assert (run_dir / BASELINE / "tester-attempt-1" / "src" / "app.py").is_file() + assert (run_dir / BASELINE / "tester").is_dir() + assert not (run_dir / BASELINE / "implementer").exists() + assert (run_dir / BASELINE / "tester" / "src" / "app.py").is_file() assert record_of(target)["paths"] == ["src/app.py"] @@ -1149,7 +1151,7 @@ def test_state_json_gains_no_field_and_never_names_the_baseline( # 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 # never took one. - assert (run_dir_of(target) / BASELINE / "implementer-attempt-2").is_dir() + assert (run_dir_of(target) / BASELINE / "implementer").is_dir() def test_nothing_in_run_story_routes_on_the_baseline(): diff --git a/tests/test_story_020_validation.py b/tests/test_story_020_validation.py index bcd8915..533ddf2 100644 --- a/tests/test_story_020_validation.py +++ b/tests/test_story_020_validation.py @@ -1547,16 +1547,19 @@ def test_removing_the_summary_costs_the_refusal_a_sentence_and_no_decision( def test_a_resumed_stage_reuses_the_baseline_recorded_for_it( - target, harness_root, + target, harness_root, tmp_path, ): """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. + The control is a fresh capture taken after the edit into a scratch run + directory, which holds the edited content — so the reuse above is the + keying rather than a capture that has stopped reading the tree. It takes + the fresh capture into a scratch directory because since story-037 the + baseline is keyed by stage alone, and an attempt number distinguishes + nothing. """ class Incomplete(Runner): """An implementer that edits the tree and writes no artifacts, so the @@ -1572,7 +1575,7 @@ def __call__(self, prompt, *, stage, **kwargs): 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) + run_dir, BASELINE, IMPLEMENTER_STAGE["name"]) assert (captured / "tests" / "test_existing.py").read_text() == TEST_AT_HEAD change_the_code(target) @@ -1583,7 +1586,7 @@ def __call__(self, prompt, *, stage, **kwargs): 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, + tmp_path / "scratch-run", target, BASELINE, IMPLEMENTER_STAGE["name"], IMPLEMENTER_STAGE["may_not_create"]) assert (recaptured / "tests" / "test_existing.py").read_text() != TEST_AT_HEAD diff --git a/tests/test_story_037_validation.py b/tests/test_story_037_validation.py new file mode 100644 index 0000000..d59d588 --- /dev/null +++ b/tests/test_story_037_validation.py @@ -0,0 +1,1172 @@ +"""Independent validation for story-037: a stage's baseline is what that stage +first found. + +The subject is a *baseline*, so almost nothing here is asserted from source. A +target repository with a real pytest suite is built under tmp_path, fake stage +agents edit its working tree, and the coordinator is run. Whether an edit is +permitted is then whatever the suite does in a clone with that edit restored — +the same question the check asks, answered by running it. + +The defect is reproduced before the fix is asserted. story-036's run escalated +on "the suite still passes with those edits reverted" because the implementer's +attempt-2 baseline already held attempt 1's edits, so reverting attempt 2 rolled +the tree back only as far as attempt 1's and the suite passed. +`pre_story_coordinator` reconstructs that code — today's coordinator with the +attempt component and the capture-once-per-directory early exit put back — and +`test_the_pre_story_code_escalates_on_the_same_run` drives the *identical* run +through it and watches it escalate for exactly that reason. Every claim below +about the two-attempt case being decided honestly is read against that +reproduction. + +Every absence asserted here carries a control: + + * "the retry's edits are permitted" sits beside the same run through the + pre-story code, which escalates — so a check that permitted everything + could not produce both; + * "the fix did not blanket-permit retries" is its own run, in which the + retry's unforced edit lands on a path the stage did *not* touch on attempt + 1 and is escalated — so the permission above is about what the baseline + holds, not about being on a retry; + * "a capture does not overwrite what the baseline holds" sits beside a fresh + capture into another run directory, which does see the edit, and beside a + path new since the first capture, which the same call adds; + * "the baseline path carries no attempt component" is paired with the + pre-story path built by the same function in the mutant, which does; + * "no attempt number is derived for the baseline" is a scan paired with the + pre-story source it does match; + * "a governed path present in the baseline is restored" sits beside the same + clone built from a baseline lacking that path, which *deletes* it and + returns the opposite verdict; + * "the resumed stage captures nothing new" is paired with a fresh capture + taken at the moment of the resume, which holds the changed content; + * "the baseline is not in state.json" is paired with the run directory that + does hold it; + * "no stage name or declared name is in the code" is paired with the names + that legitimately are. + +Nothing here invokes a model: every run goes through a fake agent runner and +every clone source is a local filesystem path. +""" +import inspect +import json +import re +import shlex +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import (BASELINE as PRE_STORY_BOUND, STORY, first_retry_route, + function_source_at, load_mutant, story_diff) + +import harness_config +import schema_validator +import story_coordinator +from agent_runner import AgentResult + +REPO_ROOT = Path(__file__).resolve().parents[1] +ORCHESTRATION = REPO_ROOT / "orchestration" +COORDINATOR_REL = "orchestration/story_coordinator.py" +COORDINATOR_PATH = REPO_ROOT / COORDINATOR_REL + +WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +IMPLEMENTER_STAGE = next(s for s in WORKFLOW["stages"] if s["name"] == "implementer") + +#: Both names are read off the declaration, never spelled here, for the same +#: reason the coordinator may not spell them. +DECLARATION = IMPLEMENTER_STAGE["revert_check"] +ARTIFACT = DECLARATION["result"] +BASELINE = DECLARATION["baseline"] +PREFIX = IMPLEMENTER_STAGE["may_not_create"][0] + +SCHEMA_STEM = "revert-check-result" + +PASS = {"status": "passed", "blocking_issues": [], "unverified": [], + "retry_recommended": False} +FAIL = { + "status": "failed", + "blocking_issues": [{ + "severity": "high", + "issue": "the validation written this run does not hold", + "location": "tests/", + "required_behavior": "the suite passes", + }], + "unverified": [], + "retry_recommended": True, + "retry_target": first_retry_route(WORKFLOW)[0], +} + +TEST_COMMAND = shlex.join([sys.executable, "-m", "pytest", "tests", "-q", + "-p", "no:cacheprovider"]) + +CONFIG = f"""\ +project: suite-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: {TEST_COMMAND} +""" + +# -------------------------------------------------------------------------- +# The target repository: a real module and a real suite over it. +# -------------------------------------------------------------------------- + +APP_AT_HEAD = '''\ +def greet(name): + return f"hello, {name}" +''' + +#: The rename attempt 1 makes. The pre-existing test cannot survive it, so the +#: test edit that accompanies it is forced. +APP_RENAMED = '''\ +def salute(name): + return f"hello, {name}" +''' + +APP_ADDITIVE = APP_AT_HEAD + ''' + +def shout(name): + return greet(name).upper() +''' + +TEST_APP_AT_HEAD = '''\ +from app import greet + + +def test_greet(): + assert greet("world") == "hello, world" +''' + +TEST_APP_REPAIRED = '''\ +from app import salute + + +def test_greet(): + assert salute("world") == "hello, world" +''' + +#: Coverage nothing forced, appended by attempt 2 on top of attempt 1's repair. +#: It passes against the tree as attempt 1 left it and as attempt 2 leaves it, +#: so reverting *it alone* costs nothing — which is the whole reason the +#: pre-story code escalated on this shape. +FREE_COVERAGE = ''' + +def test_salute_again(): + assert salute("again") == "hello, again" +''' + +TEST_APP_WITH_FREE_COVERAGE = TEST_APP_REPAIRED + FREE_COVERAGE + +TESTS_CONFTEST_AT_HEAD = '''\ +import pytest +''' + +#: An unused fixture: appended to tests/conftest.py it is an edit nothing +#: forced, on a path attempt 1 never touched. +TESTS_CONFTEST_WITH_FIXTURE = TESTS_CONFTEST_AT_HEAD + ''' + +@pytest.fixture +def unused(): + return 42 +''' + +#: The file the tester creates during the run, in its broken form: the case a +#: governed path first appearing *between* two invocations of the implementer +#: reduces to. +TEST_NEW_BROKEN = '''\ +from app import shout + + +def test_shout(): + assert shout("world") == "hello, world" +''' + +TEST_NEW_REPAIRED = '''\ +from app import shout + + +def test_shout(): + assert shout("world") == "HELLO, WORLD" +''' + +ROOT_CONFTEST = '''\ +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "src")) +''' + + +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) + + +@pytest.fixture +def target(tmp_path: Path) -> Path: + """A target repository whose configured test command is a real suite.""" + root = tmp_path / "suite-target" + 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" / "story-001.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 / "conftest.py", ROOT_CONFTEST) + write(root / "src" / "app.py", APP_AT_HEAD) + write(root / "tests" / "conftest.py", TESTS_CONFTEST_AT_HEAD) + write(root / "tests" / "test_app.py", TEST_APP_AT_HEAD) + write(root / ".gitignore", ".pytest_cache/\n__pycache__/\n") + 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) + return root + + +@pytest.fixture +def harness_root() -> Path: + return REPO_ROOT + + +# -------------------------------------------------------------------------- +# The stage edits, each paired with the record that describes it. +# -------------------------------------------------------------------------- + +NO_CHANGES = {"modified": [], "created": [], "deleted": []} + + +def unchanged(root: Path, run_dir: Path) -> dict: + return dict(NO_CHANGES) + + +def module_only(root: Path, run_dir: Path) -> dict: + """An implementer change naming no path under the governed prefix.""" + write(root / "src" / "app.py", APP_ADDITIVE) + return {"modified": ["src/app.py"], "created": [], "deleted": []} + + +def forced_repair(root: Path, run_dir: Path) -> dict: + """Attempt 1: a rename the pre-existing test cannot survive, and its repair.""" + write(root / "src" / "app.py", APP_RENAMED) + write(root / "tests" / "test_app.py", TEST_APP_REPAIRED) + return {"modified": ["src/app.py", "tests/test_app.py"], "created": [], + "deleted": []} + + +def appends_free_coverage(root: Path, run_dir: Path) -> dict: + """Attempt 2: more of the same governed file, and nothing forced this time. + + This is story-036's shape reduced: a retry that edits a file it also edited + on the previous attempt. + """ + write(root / "tests" / "test_app.py", TEST_APP_WITH_FREE_COVERAGE) + return {"modified": ["tests/test_app.py"], "created": [], "deleted": []} + + +def appends_an_unused_fixture(root: Path, run_dir: Path) -> dict: + """Attempt 2 on a governed path attempt 1 never touched, unforced.""" + write(root / "tests" / "conftest.py", TESTS_CONFTEST_WITH_FIXTURE) + return {"modified": ["tests/conftest.py"], "created": [], "deleted": []} + + +def creates_the_broken_new_test(root: Path, run_dir: Path) -> dict: + """The tester writes a governed file this run, and gets it wrong. + + Untracked and uncommitted for the whole of the run: the coordinator commits + once, at _complete. + """ + write(root / "tests" / "test_new.py", TEST_NEW_BROKEN) + return {"modified": [], "created": ["tests/test_new.py"], "deleted": []} + + +def repairs_the_new_test(root: Path, run_dir: Path) -> dict: + """The retried implementer fixes the file the tester wrote this run.""" + write(root / "tests" / "test_new.py", TEST_NEW_REPAIRED) + return {"modified": ["tests/test_new.py"], "created": [], "deleted": []} + + +def repair_then_discard_the_baseline(root: Path, run_dir: Path) -> dict: + """A forced repair whose baseline is gone by the time the check looks.""" + record = forced_repair(root, run_dir) + directory = run_dir / BASELINE + if directory.is_dir(): + shutil.rmtree(directory) + return record + + +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. + + `edits` maps a stage to the list of edits it makes, one per invocation; the + last entry repeats. `verdicts` is the verifier's, read the same way. + `interrupt` names a stage invocation after which the runner raises, which + is how a run is left with status running for the resume case below. + """ + + def __init__(self, target_root: Path, edits: dict | None = None, + verdicts: list | None = None, interrupt: tuple | None = None, + story_id: str = "story-001"): + self.target_root = target_root + self.run_dir = target_root / ".harness" / "runs" / story_id + self.edits = edits or {} + self.verdicts = verdicts or [PASS] + self.interrupt = interrupt + self.calls: list[str] = [] + + def _nth(self, sequence: list, index: int): + return sequence[min(index, len(sequence) - 1)] + + def _record(self, stage: str) -> dict: + seen = self.calls.count(stage) - 1 + edit = self._nth(self.edits.get(stage, [unchanged]), seen) + return edit(self.target_root, self.run_dir) + + def __call__(self, prompt, *, stage, cwd=None, log_path=None, + permission_mode=None, model=None, allowed_tools=None): + self.calls.append(stage) + if stage == "implementer": + write_json(self.run_dir / "changed-files.json", self._record(stage)) + write(self.run_dir / "implementation-summary.md", "Did it.\n") + elif stage == "tester": + record = self._record(stage) + write_json(self.run_dir / "test-results.json", { + "status": "passed", "tests_written": 1, "tests_run": 2, + "tests_passed": 2, "tests_failed": 0, "failures": [], + }) + write_json(self.run_dir / "tester-changed-files.json", record) + elif stage == "verifier": + seen = self.calls.count(stage) - 1 + write_json(self.run_dir / "verification-result.json", + self._nth(self.verdicts, seen)) + elif stage == "documenter": + write(self.run_dir / "documentation-report.md", "Nothing.\n") + if self.interrupt == (stage, self.calls.count(stage)): + raise KeyboardInterrupt(f"{stage} interrupted") + return AgentResult(ok=True, result_text=f"{stage} done") + + +def run_dir_of(target_root: Path, story_id: str = "story-001") -> 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 record_of(target_root: Path, artifact: str = ARTIFACT) -> dict: + return json.loads((run_dir_of(target_root) / artifact).read_text()) + + +def evidence(target_root: Path) -> tuple[str, str]: + run_dir = run_dir_of(target_root) + return ((run_dir / "events.log").read_text(), + (run_dir / "escalation-summary.md").read_text()) + + +def run(target_root: Path, harness: Path, edits: dict | None = None, + verdicts: list | None = None, interrupt: tuple | None = None, + coordinator=story_coordinator) -> tuple[int, Runner]: + """One run, through the real coordinator or through a mutant of it.""" + runner = Runner(target_root, edits, verdicts, interrupt) + code = coordinator.run_story("story-001", harness, target_root, runner) + return code, runner + + +def baseline_at(target_root: Path, stage: str = "implementer") -> Path: + """The baseline the coordinator captured for one stage.""" + return story_coordinator.stage_baseline_dir( + run_dir_of(target_root), BASELINE, stage) + + +def capture(target_root: Path, scratch: Path, prefix: str = PREFIX, + stage: str = "implementer") -> Path: + """Capture a baseline the way the coordinator captures one, into scratch.""" + return story_coordinator.capture_stage_baseline( + scratch, target_root, BASELINE, stage, [prefix]) + + +def contents_of(directory: Path) -> dict: + """Every file the baseline holds, keyed by its repository-relative path.""" + return {path.relative_to(directory).as_posix(): path.read_text() + for path in sorted(directory.rglob("*")) if path.is_file()} + + +def suite_in(directory: Path) -> int: + return subprocess.run( + [sys.executable, "-m", "pytest", "tests", "-q", "-p", "no:cacheprovider"], + cwd=directory, capture_output=True, text=True, + ).returncode + + +def scratch_suite(root: Path, app: str, test_app: str) -> int: + """The target's suite reconstructed at a given pair of contents.""" + write(root / "conftest.py", ROOT_CONFTEST) + write(root / "src" / "app.py", app) + write(root / "tests" / "conftest.py", TESTS_CONFTEST_AT_HEAD) + write(root / "tests" / "test_app.py", test_app) + return suite_in(root) + + +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) + + +# -------------------------------------------------------------------------- +# The pre-story coordinator, reconstructed by putting back what this story +# removed: the attempt component of the baseline path, and the capture-once +# early exit that returned an existing directory untouched. +# +# Built out of today's source rather than loaded from git history, because a +# coordinator recovered from history runs against today's workflow, schemas and +# config and stops running as soon as any of those legitimately changes — which +# is why `conftest.load_mutant` takes a working-tree path. +# -------------------------------------------------------------------------- + +NEW_DIR_SIGNATURE = \ + "def stage_baseline_dir(run_dir: Path, baseline: str, stage_name: str) -> Path:" +OLD_DIR_SIGNATURE = ( + "def stage_baseline_dir(\n" + " run_dir: Path, baseline: str, stage_name: str, attempt: int\n" + ") -> Path:" +) + +NEW_DIR_RETURN = " return run_dir / baseline / stage_name\n" +OLD_DIR_RETURN = ' return run_dir / baseline / f"{stage_name}-attempt-{attempt}"\n' + +NEW_CAPTURE_SIGNATURE = """def capture_stage_baseline( + run_dir: Path, + target_root: Path, + baseline: str, + stage_name: str, + prefixes: list[str], +) -> Path:""" +OLD_CAPTURE_SIGNATURE = """def capture_stage_baseline( + run_dir: Path, + target_root: Path, + baseline: str, + stage_name: str, + attempt: int, + prefixes: list[str], +) -> Path:""" + +NEW_CAPTURE_HEAD = """ directory = stage_baseline_dir(run_dir, baseline, stage_name) + directory.mkdir(parents=True, exist_ok=True) +""" +OLD_CAPTURE_HEAD = """ directory = stage_baseline_dir(run_dir, baseline, stage_name, attempt) + if directory.exists(): + return directory + directory.mkdir(parents=True) +""" + +NEW_FIRST_SEEN = """ destination = directory / rel + if destination.exists(): + continue +""" +OLD_FIRST_SEEN = """ destination = directory / rel +""" + +NEW_CALL_SITE = """ declaration["baseline"], + name, + stage.get("may_not_create", []), +""" +OLD_CALL_SITE = """ declaration["baseline"], + name, + attempt, + stage.get("may_not_create", []), +""" + +PRE_STORY_SUBSTITUTIONS = ( + (NEW_DIR_SIGNATURE, OLD_DIR_SIGNATURE), + (NEW_DIR_RETURN, OLD_DIR_RETURN), + (NEW_CAPTURE_SIGNATURE, OLD_CAPTURE_SIGNATURE), + (NEW_CAPTURE_HEAD, OLD_CAPTURE_HEAD), + (NEW_FIRST_SEEN, OLD_FIRST_SEEN), + (NEW_CALL_SITE, OLD_CALL_SITE), +) + + +def pre_story_coordinator(tmp_path: Path): + """Today's coordinator with the attempt keying and capture-once put back.""" + return load_mutant(COORDINATOR_PATH, list(PRE_STORY_SUBSTITUTIONS), + name="coordinator_before_story_037", tmp_path=tmp_path) + + +def pre_story_source() -> str: + """The same reconstruction as text, for the source-level controls below.""" + source = COORDINATOR_PATH.read_text(encoding="utf-8") + for old, new in PRE_STORY_SUBSTITUTIONS: + assert old in source, old + source = source.replace(old, new, 1) + return source + + +# -------------------------------------------------------------------------- +# The premises: what makes "permitted" and "escalated" mean anything here +# -------------------------------------------------------------------------- + +#: The run shape story-036 escalated on, reduced: the implementer edits a +#: governed file on attempt 1 and edits it further on attempt 2. +TWO_ATTEMPT_SHAPE = {"implementer": [forced_repair, appends_free_coverage]} + + +def test_reverting_the_stages_whole_edit_to_the_governed_file_fails_the_suite( + tmp_path, +): + """The premise under "permitted": restoring tests/test_app.py to what the + implementer *first* found leaves a test calling a function the renamed + module no longer has, so the suite fails and the aggregate edit is forced. + + The control is the same tree with the stage's own final content, which + passes — so the failure is the revert and not a broken scratch tree. + """ + assert scratch_suite(tmp_path / "reverted", APP_RENAMED, TEST_APP_AT_HEAD) != 0 + assert scratch_suite(tmp_path / "intact", APP_RENAMED, + TEST_APP_WITH_FREE_COVERAGE) == 0 + + +def test_reverting_only_the_second_attempts_edit_costs_nothing(tmp_path): + """The premise under the pre-story escalation: attempt 2's appended + coverage passes against the tree before and after it, so a check reverting + to attempt 1's content sees a passing suite and calls the edit unforced. + + This is precisely what the attempt-keyed baseline made the check ask. + """ + assert scratch_suite(tmp_path / "attempt-1", APP_RENAMED, + TEST_APP_REPAIRED) == 0 + assert scratch_suite(tmp_path / "attempt-2", APP_RENAMED, + TEST_APP_WITH_FREE_COVERAGE) == 0 + + +# -------------------------------------------------------------------------- +# The defect, reproduced: the pre-story code escalates on this run +# -------------------------------------------------------------------------- + + +def test_the_pre_story_code_escalates_on_the_same_run(target, harness_root, tmp_path): + """story-036's failure, driven rather than argued. + + The attempt-2 capture holds attempt 1's edits, so reverting attempt 2's + edit rolls the tree back only as far as attempt 1's, the suite passes, and + the run escalates on "the suite still passes with those edits reverted" — + unforced coverage the check never actually tested. + """ + coordinator = pre_story_coordinator(tmp_path) + code, runner = run(target, harness_root, TWO_ATTEMPT_SHAPE, [FAIL, PASS], + coordinator=coordinator) + + assert code == 2 + assert state_of(target)["status"] == "escalated" + assert runner.calls == ["implementer", "tester", "verifier", "implementer"] + + record = record_of(target) + assert record["ran"] is True + assert record["permitted"] is False + assert record["exit_code"] == 0 # the suite passed with it reverted + assert record["paths"] == ["tests/test_app.py"] + + _, summary = evidence(target) + assert "still passes with those edits reverted" in summary + + # And the reason it asked the wrong question: two attempt-keyed + # directories, the second already holding attempt 1's content. + run_dir = run_dir_of(target) + assert (run_dir / BASELINE / "implementer-attempt-2" / "tests" + / "test_app.py").read_text() == TEST_APP_REPAIRED + assert not (run_dir / BASELINE / "implementer").exists() + + +# -------------------------------------------------------------------------- +# The fix: a re-entered stage is decided against what it first found +# -------------------------------------------------------------------------- + + +def test_the_second_attempts_edit_to_a_file_it_also_edited_is_permitted( + target, harness_root, +): + """The story's first two acceptance criteria, end to end and as one run. + + The identical run the pre-story code escalated on completes: the check + restores tests/test_app.py to what the implementer found when it *first* + ran, the suite fails, and the edit is permitted. + """ + code, runner = run(target, harness_root, TWO_ATTEMPT_SHAPE, [FAIL, PASS]) + + assert code == 0 + assert state_of(target)["status"] == "completed" + assert runner.calls == ["implementer", "tester", "verifier", + "implementer", "tester", "verifier", "documenter"] + + record = record_of(target) + assert record["ran"] is True + assert record["permitted"] is True + assert record["exit_code"] != 0 # the suite failed with it reverted + assert record["paths"] == ["tests/test_app.py"] + + +def test_the_baseline_that_decision_was_made_against_holds_the_original_content( + target, harness_root, +): + """What the check decided against, read off the run directory: the content + the governed path held before the stage's *first* invocation, not the + content attempt 1 left. + + The control is the tree the run ended on, which holds neither — so this is + a statement about the baseline rather than about a capture that happens to + agree with whatever is on disk. + """ + assert run(target, harness_root, TWO_ATTEMPT_SHAPE, [FAIL, PASS])[0] == 0 + + captured = baseline_at(target) / "tests" / "test_app.py" + assert captured.read_text() == TEST_APP_AT_HEAD + assert captured.read_text() != TEST_APP_REPAIRED + assert (target / "tests" / "test_app.py").read_text() \ + == TEST_APP_WITH_FREE_COVERAGE + + +def test_a_retry_editing_a_path_it_did_not_touch_before_is_still_escalated( + target, harness_root, +): + """The control for the permission above, and the reason it is not a + blanket exemption for retries. + + The same two-attempt run, with attempt 2's unforced edit landing on a + governed path attempt 1 never touched. The baseline holds that path at what + the stage first found it as — which is also what attempt 1 left it as — so + reverting costs nothing, the suite passes, and the run escalates. Being on + a retry buys nothing; what the baseline holds decides. + """ + code, runner = run(target, harness_root, + {"implementer": [forced_repair, appends_an_unused_fixture]}, + [FAIL, PASS]) + + assert code == 2 + assert state_of(target)["status"] == "escalated" + assert runner.calls == ["implementer", "tester", "verifier", "implementer"] + + record = record_of(target) + assert record["ran"] is True + assert record["permitted"] is False + assert record["paths"] == ["tests/conftest.py"] + _, summary = evidence(target) + assert "still passes with those edits reverted" in summary + + +# -------------------------------------------------------------------------- +# A governed path that first appears between two invocations of the stage +# -------------------------------------------------------------------------- + +#: The implementer's first invocation touches no governed path; the tester then +#: creates one; the retried implementer repairs it. The path exists in neither +#: HEAD nor the stage's first capture. +BETWEEN_ATTEMPTS_SHAPE = { + "implementer": [module_only, repairs_the_new_test], + "tester": [creates_the_broken_new_test, unchanged], +} + + +def test_a_path_created_between_two_invocations_is_captured_at_what_it_met( + target, harness_root, +): + """The merge's other half: the file the tester created after the + implementer's first invocation is added to the implementer's one baseline + directory, at the content the tester left, and the check decides the + retry's repair of it as permitted — restored rather than deleted. + + The control is the file the first invocation *did* capture, which is still + at its original content in the same directory: the merge added a path + without disturbing one already held. + """ + code, _ = run(target, harness_root, BETWEEN_ATTEMPTS_SHAPE, [FAIL, PASS]) + assert code == 0 + + baseline = baseline_at(target) + assert (baseline / "tests" / "test_new.py").read_text() == TEST_NEW_BROKEN + assert (baseline / "tests" / "test_app.py").read_text() == TEST_APP_AT_HEAD + + record = record_of(target) + assert record["ran"] is True + assert record["permitted"] is True + assert record["paths"] == ["tests/test_new.py"] + + # It had no version at HEAD for the whole of the run: HEAD^ is where the + # run started, and the run's own commit is _complete's, made after the + # check had already decided. The control is the path that did have one. + assert git(target, "cat-file", "-e", "HEAD^:tests/test_new.py", + check=False).returncode != 0 + assert git(target, "cat-file", "-e", "HEAD^:tests/test_app.py", + check=False).returncode == 0 + + +def test_reusing_the_earlier_capture_whole_would_have_deleted_that_path( + target, tmp_path, +): + """The control for the test above, and the reason the merge is per path. + + A baseline taken before the tester created the file — which is what + reusing the earlier attempt's directory wholesale would have handed the + check — does not hold `tests/test_new.py`, so `_build_clone` *deletes* it + rather than restoring it, and the suite then passes: the same repair the + merged baseline permits would be escalated. + """ + module_only(target, run_dir_of(target)) + stale = capture(target, tmp_path / "before-the-tester") + creates_the_broken_new_test(target, run_dir_of(target)) + merged = capture(target, tmp_path / "after-the-tester") + repairs_the_new_test(target, run_dir_of(target)) + + assert not (stale / "tests" / "test_new.py").exists() + assert (merged / "tests" / "test_new.py").read_text() == TEST_NEW_BROKEN + + deleting = story_coordinator.run_clean_clone( + target, TEST_COMMAND, None, tmp_path / "deleting", + revert=["tests/test_new.py"], baseline=stale) + restoring = story_coordinator.run_clean_clone( + target, TEST_COMMAND, None, tmp_path / "restoring", + revert=["tests/test_new.py"], baseline=merged) + + assert deleting.ran is True and deleting.exit_code == 0 # escalates + assert restoring.ran is True and restoring.exit_code != 0 # permits + assert not (Path(deleting.clone_path) / "tests" / "test_new.py").exists() + assert (Path(restoring.clone_path) / "tests" / "test_new.py").read_text() \ + == TEST_NEW_BROKEN + + +# -------------------------------------------------------------------------- +# A capture never overwrites what the baseline already holds +# -------------------------------------------------------------------------- + + +def test_a_second_capture_keeps_what_the_first_recorded_and_adds_what_is_new( + target, tmp_path, +): + """Both halves of first-seen-wins, in one call. + + The control is a fresh capture into a run directory holding no baseline for + this stage, which does see the edit — so the preservation above is the + first-seen rule rather than a capture that has stopped reading the tree. + """ + scratch = tmp_path / "run" + first = capture(target, scratch) + assert (first / "tests" / "test_app.py").read_text() == TEST_APP_AT_HEAD + assert not (first / "tests" / "test_new.py").exists() + + forced_repair(target, run_dir_of(target)) + creates_the_broken_new_test(target, run_dir_of(target)) + again = capture(target, scratch) + + assert again == first + assert (again / "tests" / "test_app.py").read_text() == TEST_APP_AT_HEAD + assert (again / "tests" / "test_new.py").read_text() == TEST_NEW_BROKEN + + fresh = capture(target, tmp_path / "fresh") + assert (fresh / "tests" / "test_app.py").read_text() == TEST_APP_REPAIRED + + +def test_the_pre_story_capture_would_have_dropped_the_path_new_since_the_first( + target, tmp_path, +): + """The control for the adding half above, which is the half a wholesale + reuse of the earlier capture gets wrong. + + The pre-story capture returned an existing directory untouched, so the same + two calls against the same directory leave `tests/test_new.py` out of the + baseline entirely — and a governed path the baseline does not hold is + deleted in the clone rather than restored. + """ + coordinator = pre_story_coordinator(tmp_path) + scratch = tmp_path / "run" + + def pre_story_capture() -> Path: + return coordinator.capture_stage_baseline( + scratch, target, BASELINE, "implementer", 1, [PREFIX]) + + first = pre_story_capture() + assert (first / "tests" / "test_app.py").read_text() == TEST_APP_AT_HEAD + + creates_the_broken_new_test(target, run_dir_of(target)) + again = pre_story_capture() + + assert again == first + assert not (again / "tests" / "test_new.py").exists() + # The control on the control: today's capture, over the same tree and the + # same directory, does add it. + assert (capture(target, tmp_path / "today") / "tests" + / "test_new.py").read_text() == TEST_NEW_BROKEN + + +def test_today_neither_function_accepts_the_attempt_the_pre_story_ones_required( + target, tmp_path, +): + """The behavioural form of the signature change: the pre-story call, made + with the attempt argument it required, is a TypeError today. + + The control is the same call against the pre-story reconstruction, which + accepts it — so the rejection below is about today's signature rather than + about a call that was malformed either way. + """ + coordinator = pre_story_coordinator(tmp_path) + assert coordinator.capture_stage_baseline( + tmp_path / "before", target, BASELINE, "implementer", 1, [PREFIX]).is_dir() + assert coordinator.stage_baseline_dir(tmp_path / "before", BASELINE, + "implementer", 1).name.endswith("-1") + + with pytest.raises(TypeError): + story_coordinator.capture_stage_baseline( + tmp_path / "now", target, BASELINE, "implementer", 1, [PREFIX]) + with pytest.raises(TypeError): + story_coordinator.stage_baseline_dir(tmp_path / "now", BASELINE, + "implementer", 1) + + +def test_the_baseline_directory_exists_even_when_it_captures_nothing( + target, tmp_path, +): + """The merge replaced an early exit that also created the directory, so + the property is re-asserted here: its existence answers "was a baseline + taken", and an absent one is the distinct condition the check refuses on. + + The control is the same call over a prefix that has files. + """ + empty = capture(target, tmp_path / "run", prefix="nothing-here/") + assert empty.is_dir() + assert list(empty.rglob("*")) == [] + assert (capture(target, tmp_path / "run", stage="other") / "tests").is_dir() + + +def test_the_captured_set_is_still_tracked_plus_untracked_under_the_prefix( + target, tmp_path, +): + """A file created earlier in the run and never committed is still captured. + + The control is the tracked listing, which does not contain it: a capture + built on `--cached` alone would have held nothing for that path, and the + restore would have deleted it. + """ + creates_the_broken_new_test(target, run_dir_of(target)) + baseline = capture(target, tmp_path / "run") + + assert set(contents_of(baseline)) == { + "tests/conftest.py", "tests/test_app.py", "tests/test_new.py"} + tracked = git(target, "ls-files", "--", PREFIX).stdout.split() + assert "tests/test_new.py" not in tracked + assert "tests/test_app.py" in tracked + + +# -------------------------------------------------------------------------- +# A resumed run reuses the stage's stored baseline +# -------------------------------------------------------------------------- + + +def test_a_resumed_stage_captures_nothing_new_and_decides_against_the_stored_one( + target, harness_root, tmp_path, +): + """A run interrupted inside the implementer and resumed there. The stage is + re-entered with its edits already in the tree; the merge records nothing, + and the reused baseline is what makes the decision honest. + + The control is a fresh capture taken at the moment of the resume, which + holds the interrupted stage's edit — so an overwriting capture would have + produced a different baseline and reversed the decision. + """ + with pytest.raises(KeyboardInterrupt): + run(target, harness_root, {"implementer": [forced_repair]}, + interrupt=("implementer", 1)) + assert state_of(target)["status"] == "running" + assert (target / "tests" / "test_app.py").read_text() == TEST_APP_REPAIRED + + stored = contents_of(baseline_at(target)) + assert stored["tests/test_app.py"] == TEST_APP_AT_HEAD + fresh = contents_of(capture(target, tmp_path / "at-the-resume")) + assert fresh["tests/test_app.py"] == TEST_APP_REPAIRED + + code, resumed = run(target, harness_root, {"implementer": [forced_repair]}) + assert code == 0 + assert resumed.calls[0] == "implementer" + assert contents_of(baseline_at(target)) == stored + + record = record_of(target) + assert record["permitted"] is True + assert record["baseline"] == str(baseline_at(target)) + + +# -------------------------------------------------------------------------- +# The path carries no attempt component, and neither signature takes one +# -------------------------------------------------------------------------- + + +def test_neither_function_takes_an_attempt_argument(): + """The control is the parameter list each function does take, asserted + whole: an absence read off a signature that had stopped being resolved + could not also match these.""" + assert list(inspect.signature(story_coordinator.stage_baseline_dir) + .parameters) == ["run_dir", "baseline", "stage_name"] + assert list(inspect.signature(story_coordinator.capture_stage_baseline) + .parameters) == ["run_dir", "target_root", "baseline", + "stage_name", "prefixes"] + + +def test_the_baseline_path_is_the_declared_name_and_the_stage_and_nothing_else( + tmp_path, +): + """The control is the same function in the pre-story coordinator, which + builds the attempt-keyed path from the same three arguments — so the + absence below is about today's keying and not about a comparison that + could not differ. + """ + run_dir = tmp_path / "run" + now = story_coordinator.stage_baseline_dir(run_dir, BASELINE, "implementer") + assert now == run_dir / BASELINE / "implementer" + assert now.relative_to(run_dir).parts == (BASELINE, "implementer") + assert "attempt" not in str(now.relative_to(run_dir)) + + before = pre_story_coordinator(tmp_path).stage_baseline_dir( + run_dir, BASELINE, "implementer", 2) + assert "attempt" in str(before.relative_to(run_dir)) + + +def _baseline_regions(module_source: str) -> dict[str, str]: + """The three places an attempt number for the baseline could be derived: + the keying function, the capture, and the capture's call site in run_story. + """ + keying, _, rest = module_source.partition("def capture_stage_baseline(") + capture, _, after = rest.partition("\n@dataclass") + _, _, call_site = after.partition('declaration = stage.get("revert_check")') + return { + "stage_baseline_dir": executable_source( + "def stage_baseline_dir(" + + keying.rpartition("def stage_baseline_dir(")[2]), + "capture_stage_baseline": executable_source(capture), + "the call site": executable_source( + call_site.partition("result = runner(")[0]), + } + + +def test_no_attempt_number_is_derived_for_the_baseline_anywhere(): + """Neither function names an attempt, and neither does the capture's call + site in run_story. + + The scan is paired with the pre-story source, in which all three regions do + name one — so a scan that had stopped seeing anything could not pass both + halves. + """ + today = _baseline_regions(COORDINATOR_PATH.read_text(encoding="utf-8")) + before = _baseline_regions(pre_story_source()) + + for where, body in today.items(): + assert "stage_baseline_dir" in body or "baseline" in body, where + assert "attempt" not in body, where + for where, body in before.items(): + assert "attempt" in body, where # the scan can fail + + +# -------------------------------------------------------------------------- +# The record names the stage-keyed directory, and the refusal is unchanged +# -------------------------------------------------------------------------- + + +def test_the_records_baseline_field_names_the_stage_keyed_directory( + target, harness_root, +): + assert run(target, harness_root, TWO_ATTEMPT_SHAPE, [FAIL, PASS])[0] == 0 + record = record_of(target) + run_dir = run_dir_of(target) + + assert record["baseline"] == str(run_dir / BASELINE / "implementer") + assert Path(record["baseline"]).is_dir() + assert "attempt" not in Path(record["baseline"]).relative_to(run_dir).as_posix() + assert schema_validator.validate( + record, schema_validator.load_schema(SCHEMA_STEM)) == [] + + +def test_a_stage_declaring_the_check_with_no_baseline_still_escalates_naming_it( + target, harness_root, +): + """Unchanged and still reachable: the merge creates the directory rather + than requiring it, so a run whose baseline is removed after the capture + still reaches the refusal.""" + code, _ = run(target, harness_root, + {"implementer": [repair_then_discard_the_baseline]}) + assert code == 2 + record = record_of(target) + assert record["ran"] is False + assert "permitted" not in record + assert "baseline" not in record + assert BASELINE in record["reason"] + _, summary = evidence(target) + assert "could not run" in summary + + +def test_the_same_run_with_its_baseline_intact_decides(target, harness_root): + """The control for the refusal above: the identical edit, and the only + difference is that the baseline is still there.""" + code, _ = run(target, harness_root, {"implementer": [forced_repair]}) + assert code == 0 + record = record_of(target) + assert record["ran"] is True + assert record["permitted"] is True + assert BASELINE in record["baseline"] + + +# -------------------------------------------------------------------------- +# The decision rule and the restore semantics are untouched +# -------------------------------------------------------------------------- + + +def test_the_check_and_the_clone_builder_are_byte_for_byte_pre_story(): + """Only the state reverted to changed. The control is the capture, in the + same file at the same bound, which did change — so a comparison that had + stopped resolving anything could not pass both halves. + """ + def before(name: str) -> str: + return function_source_at(COORDINATOR_REL, name, + validation_file=Path(__file__), + bound=PRE_STORY_BOUND, repo=REPO_ROOT) + + for name in ("revert_check", "_build_clone", "_revert_check_permitted", + "run_clean_clone", "governed_edits"): + assert before(name) == inspect.getsource( + getattr(story_coordinator, name)), name + + assert before("capture_stage_baseline") \ + != inspect.getsource(story_coordinator.capture_stage_baseline) + assert before("stage_baseline_dir") \ + != inspect.getsource(story_coordinator.stage_baseline_dir) + + +def test_a_forced_edit_and_an_unforced_one_on_a_single_attempt_still_decide( + target, harness_root, +): + """The decision rule, exercised at the shape that has nothing to do with + retries: permitted exactly when reverting makes the suite fail.""" + code, _ = run(target, harness_root, {"implementer": [forced_repair]}) + assert code == 0 + assert record_of(target)["permitted"] is True + + +def test_an_unforced_edit_on_a_single_attempt_is_still_escalated( + target, harness_root, +): + code, _ = run(target, harness_root, + {"implementer": [appends_an_unused_fixture]}) + assert code == 2 + record = record_of(target) + assert record["permitted"] is False + assert record["paths"] == ["tests/conftest.py"] + + +# -------------------------------------------------------------------------- +# The baseline is evidence, never state; and the code names nothing +# -------------------------------------------------------------------------- + + +def test_state_json_gains_no_field_and_never_names_the_baseline( + target, harness_root, +): + assert run(target, harness_root, TWO_ATTEMPT_SHAPE, [FAIL, PASS])[0] == 0 + state_text = (run_dir_of(target) / "state.json").read_text() + fields = set(json.loads(state_text)) + + 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 and not about a run that took none. + assert baseline_at(target).is_dir() + + +def test_nothing_in_run_story_routes_on_the_baseline(): + """It is consumed to build a clone and nowhere else. The control is the + verdict, which *is* branched on.""" + 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("decided") + assert routing("baseline") == [] + + +def test_no_stage_name_no_prefix_and_neither_declared_name_is_in_the_code(): + """story-019's property, preserved over the changed source. Docstrings and + comments are stripped first: prose may name what code may not. The control + is the name that legitimately does appear.""" + body = executable_source(COORDINATOR_PATH.read_text(encoding="utf-8")) + assert PREFIX not in body + assert ARTIFACT not in body + assert BASELINE not in body + for stage in WORKFLOW["stages"]: + if stage["name"] == "verifier": + continue # the verifier routing branch predates this story + assert stage["name"] not in body, stage["name"] + assert "state.json" in body # a name the code owns + + +def test_the_capture_names_no_stage_and_no_prefix(): + body = executable_source( + inspect.getsource(story_coordinator.capture_stage_baseline)) + assert "ls-files" in body # the stripping kept the code + assert PREFIX not in body + assert BASELINE not in body + for stage in WORKFLOW["stages"]: + assert stage["name"] not in body, stage["name"] + + +def test_the_baseline_still_carries_no_schema_and_no_manifest_entry(): + manifest = json.loads( + (REPO_ROOT / "schemas" / "manifest.json").read_text(encoding="utf-8")) + assert BASELINE not in manifest["schemas"] + assert not (REPO_ROOT / "schemas" / f"{BASELINE}.schema.json").exists() + # The control: the artifact declared beside it does carry one. + assert SCHEMA_STEM in manifest["schemas"] + + +# -------------------------------------------------------------------------- +# What this story left alone +# -------------------------------------------------------------------------- + + +def test_this_story_edited_no_schema_no_workflow_and_no_story_artifact(): + """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 untouched in (".harness/stories/", "schemas/", "workflows/", "prompts/", + "orchestration/context_assembler.py", + "orchestration/agent_runner.py", + "orchestration/plan_validation.py"): + assert story_diff([untouched], validation_file=validation) == "", untouched + assert story_diff([COORDINATOR_REL], validation_file=validation) != ""