Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions .harness/docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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, `<stage>-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**, `<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.

Expand Down Expand Up @@ -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 -- <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; 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.
Expand Down
40 changes: 24 additions & 16 deletions orchestration/story_coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -1290,24 +1290,23 @@ 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(
run_dir: Path,
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.
Expand All @@ -1318,20 +1317,28 @@ 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.

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,
Expand All @@ -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
Expand Down Expand Up @@ -2505,7 +2514,6 @@ def elapsed() -> float | None:
target_root,
declaration["baseline"],
name,
attempt,
stage.get("may_not_create", []),
)
if declaration
Expand Down
2 changes: 1 addition & 1 deletion tests/test_story_017_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading
Loading