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
19 changes: 17 additions & 2 deletions .harness/docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,24 @@ The resume branch does four things, in this order, before any state is written:

**The interrupted attempt is archived before the resumed stage runs**, under the attempt number it was written with (`state.retry_count + 1`, the same expression the retry branch uses). `interrupted_attempt_artifacts(stages, attempt)` is `archivable_artifacts(stages)` plus that attempt's rendered prompt filenames, one per stage — the prompts are the addition a resume needs, because a run re-entering under the same attempt number re-renders over the prompt the interrupted stage was actually given. Both lists come off the loaded workflow; no stage name and no artifact name is written in orchestration. An archive directory that already exists is a refusal, not an overwrite (see the story-010 bullet below).

**Resuming is inferred from the recorded status and from nothing else.** The guard adds only a refusal, for the one case where a resume is knowably pointless. `unchanged_since_escalation(state, story_text, target_root, harness_root)` makes three comparisons, each of which must be *establishable* before it says anything: the story artifact's SHA-256 against `state.story_digest`, the branch against `state.escalation_commit` with a clean `git status --porcelain`, and the harness's revision against `state.harness_revision`. It returns the evidence only when all three are establishable and identical, and an empty list otherwise — **anything it cannot establish counts as not-the-same**, so an absent digest, an escalation that committed nothing, an unreadable HEAD, or a harness root that is not a git repository produces no refusal rather than a false one. The refusal message names the escalation reason (read from `escalation-summary.md` by `escalation_reason`, for the message only — nothing routes on it) and says what to change: amend the story, change the code on the branch, or update the harness. Amending the story clears it. The digest informs that message; it never authorizes or triggers a resume, so an incidental edit to a story cannot silently restart a run.
**Resuming is inferred from the recorded status and from nothing else.** The guard adds only a refusal, for the one case where a resume is knowably pointless. `unchanged_since_escalation(state, story_text, target_root, harness_root)` makes three comparisons, each of which must be *establishable* before it says anything: the story artifact's SHA-256 against `state.story_digest`, the branch against `state.escalation_commit` with a clean `git status --porcelain`, and the harness against what the state recorded of it. It returns the evidence only when all three are establishable and identical, and an empty list otherwise — **anything it cannot establish counts as not-the-same**, so an absent digest, an escalation that committed nothing, an unreadable HEAD, or a harness root that is not a git repository produces no refusal rather than a false one. Since story-034 the third comparison has **two forms** and still counts as one comparison — see "When the harness is its own target" below. The refusal message names the escalation reason (read from `escalation-summary.md` by `escalation_reason`, for the message only — nothing routes on it) and says what to change: amend the story, change the code on the branch, or update the harness. Amending the story clears it. The digest informs that message; it never authorizes or triggers a resume, so an incidental edit to a story cannot silently restart a run.

One resumed event is appended through the existing `append_event` path, naming the stage, so the routing stays reconstructable from the log alone and `events.log` and `execution-history.json` stay two renderings of one write.

### When the harness is its own target

The guard was reasoned about for a **separate-repository deployment**, where a run touches the target and never the harness checkout, so the recorded revision is a fact the run cannot move and the third comparison compares what it was written to compare. This repository is the other configuration — the harness *is* the target — and there the recorded-revision comparison could never say "unchanged". `state.harness_revision` is written as the first act of `_escalate`, before `commit_escalated_work` and `commit_escalated_tree` move HEAD, so a shared-root resume always read a pre-escalation revision against a post-escalation one. The one shape where HEAD does not move — an escalation with a clean tree, which commits nothing — is already ended earlier by the empty `escalation_commit`. There was therefore **no path on which the guard could refuse in this repository**, and story-028's resume happened for that reason rather than because anything about it had changed. This is drift into a configuration the guard was not reasoned about in, not a design error in it.

`same_repository(target_root, harness_root)` decides the case, from `git rev-parse --show-toplevel` in each root through the existing `_git` helper, reporting shared only when **both** invocations succeed and their outputs are equal. A failing invocation on either side returns False — the not-established answer — which is the same one-directional bias `_revision`, `dirty_paths`, `completion_commits` and `base_problems` take, so a root that is not a git repository falls through to the revision comparison rather than producing a false refusal.

**Under a shared root the branch comparison stands for the harness one**, and the deferral is sound only *because* leg two ran first: it has established that the branch is exactly the escalation commit with nothing uncommitted, and under one tree that covers every change to the harness source. The third comparison's evidence line then names the shared checkout instead of a recorded revision — "the harness is the same checkout as the target, so it is covered by the branch comparison above". The shared-root branch therefore sits *after* leg two and could not sit before it; any earlier and it would assert something not yet established. Under separate roots the `state.harness_revision` comparison and its evidence line are unchanged character for character, so a separate-repository deployment reads exactly as it did.

**A tree hash over the harness's own source directories was considered and rejected.** It is a second definition of "has the harness changed" living beside the branch comparison that already answers it, and it would have to choose which directories count — a choice nothing else in the harness makes. **Moving where the revision is recorded was not attempted either**, and the reasoning should not be re-derived: any revision recorded before the escalation's commits is stale afterwards, and any recorded after cannot be inside the commit that records it — the same circularity the two-commit shape already solves for `escalation_commit`. Both rejections are written at the comparison itself as well as here.

Nothing about the structure moved. The function still makes exactly **three** comparisons, still returns evidence only when all three are establishable and identical, and still refuses only on positive evidence; the third simply has two forms. `_escalate`'s assignment of `state.harness_revision`, both escalation commits, `_revision` and `_resume_refusal`'s guidance sentence are untouched, and **no flag, environment variable or configuration key** that skips or weakens the guard exists.

What changes for a developer working in this repository: a story-028-shaped escalation — clean tree, story unamended, no new commit — **stops resuming and starts refusing**. `--stage` is the intended override, already exists and is already named in the refusal message, so nothing new is needed, but the working habit changes. The escalation summary's standing promise, that the resume is refused while the story artifact, the branch and the harness are all unchanged, becomes true in this repository for the first time.

### The escalation commit

Resume cannot recover what the harness did not preserve, which is why the commit is part of this story rather than a neighbour of it. `_complete` ended a successful run with `git add -A` and a commit; `_escalate` committed nothing, so the one terminal state a developer most needs to inspect, resume or hand to another agent was the one left in the working tree, surviving exactly until someone checked out another branch — a normal thing to do while deciding what to do about an escalation. story-018's escalation left seven files and 1174 insertions uncommitted on `story/story-018`, preserved by a hand-written commit (`e2f401d`) the harness should have made.
Expand Down Expand Up @@ -444,7 +458,8 @@ It is narrower than Chapter 18's **checkpoints**, and the difference is the reas
- **Adding a stage declaration to the shipped workflow breaks the end-to-end tests of every check already declared on that stage, and the repair is to mirror the workflow, not to weaken the test.** `tests/test_story_007_validation.py`'s two ownership tests drive real `run_story` calls whose subject is that the ownership rule reads `created` alone; against the shipped workflow they now also pick up the new `revert_check` key, and their synthetic records name paths with no version at HEAD, so the revert check correctly escalates with *could not run* and the runs no longer complete. story-017 repaired them with an `ownership_only` helper built on that file's existing `mirror_harness`, which runs each against a copy of the workflow with the implementer's `revert_check` popped; every assertion and every record is unchanged and the docstring says why. That is the pattern to reuse: a test whose subject is one check is run against a workflow declaring only that check, so the next declaration does not silently retarget it. The general form is the story-013 ripple bullet above — a per-story test that goes red on landing gets repointed, never relaxed — and demonstrating the repair was *forced* is the same revert the check itself performs.
- **A fact `state.json` does not carry is added to `state.json`; it does not get a shadow file.** story-020 needed three facts a resume cannot derive — the story artifact as the run first read it (`story_digest`), the commit the escalation made (`escalation_commit`), and the harness revision at the moment it escalated (`harness_revision`) — and put all three on `RunState` rather than beside it. Each is defaulted, so `RunState(**json)` still loads a state file written before this story, and **empty means "not established" at every reader** rather than being a value to compare against. That single convention is what makes the resume guard's bias correct by construction instead of by three separate checks. The story added no new artifact at all: the run directory already holds every stage artifact and the attempt archive, and no second record of where execution stands was created.
- `story_digest(story_text)` is taken from the same text `read_story` was handed, so the digest and the run's one reading describe one artifact — the same "exactly one mechanism reads a story artifact" rule, applied to hashing it. It is recorded when the run's state is first created, and it feeds a refusal *message*. It does not authorize a resume, does not trigger one, and no line in `run_story` routes on it.
- **The resume guard refuses only on positive evidence of sameness.** Every comparison in `unchanged_since_escalation` must be establishable before it can contribute, and a comparison that cannot be made returns the empty list rather than a partial verdict. The bias is deliberate and one-directional: under-refusing costs a wasted run the developer asked for, over-refusing blocks a resume the developer needs, and only the second is a failure the harness inflicts. A harness root that is not a git repository is the case to keep working — it produces no refusal.
- **The resume guard refuses only on positive evidence of sameness.** Every comparison in `unchanged_since_escalation` must be establishable before it can contribute, and a comparison that cannot be made returns the empty list rather than a partial verdict. The bias is deliberate and one-directional: under-refusing costs a wasted run the developer asked for, over-refusing blocks a resume the developer needs, and only the second is a failure the harness inflicts. A harness root that is not a git repository is the case to keep working — it produces no refusal. story-034 extended that bias rather than bending it: `same_repository` reports not-shared whenever either `git rev-parse --show-toplevel` fails, and the guard then falls through to the comparison it made before.
- **A guard that cannot fire in the configuration it is developed under is a guard nobody is testing.** `unchanged_since_escalation` was correct for a separate-repository deployment and structurally unable to refuse in this one, and the gap surfaced only as a resume nobody questioned. The repair was to recognise the shared-root case and let an existing comparison stand for the redundant one, not to add a fourth comparison or a second definition of "changed" — see "When the harness is its own target" above for the rejected tree hash and the rejected relocation of `state.harness_revision`. **When a check reads a fact the run itself writes, ask which deployment it was reasoned about in before trusting it in another.**
- **Both terminal commits now establish what they commit, and neither was changed to do it.** story-020 stated the limit — `git add -A` stages whatever the working tree holds — in the coordinator where the commits are made and in a test worded as a property of the coordinator rather than of one commit, and story-021 closed it from the other end: the clean-tree pre-flight establishes the tree *before* any stage runs, so staging everything and staging what the run produced are the same set. `_complete` and `_escalate` are byte-for-byte unchanged. The stated limit that survives is the pre-flight's one exclusion, a resumed crashed run, stated in the same place and driven by the same kind of test. **A stated limit a reader meets where the behaviour is, and a test that goes red when the statement stops being true, is the shape to reuse for any limit a story knowingly leaves open** — and the corollary story-020 predicted held: the assertion was repointed to the guarantee that now holds, keeping its subject, its strictness and its stray file, rather than deleted.
- **Deferring a decision to the story that owns it worked.** story-020 deliberately wrote no rule about which runs the future clean-tree pre-flight applies to, so that deciding it by accident could not happen; story-021 then decided it in one place, recorded above under "The tree a run starts from". Two requests planned together, with the shared question named in both and answered in neither until the second landed, is the shape that avoided it.
- A narrowness in the guard, recorded rather than fixed: `unchanged_since_escalation` runs before `_checkout_story_branch`, so its `HEAD~1` comparison reads whatever branch the developer is standing on rather than `state.branch`. A developer who escalates, checks out `main` to think about it — the act this story exists to make safe — and then re-runs the story gets a resume rather than the refusal. Whether that is a miss depends on reading "the target tree" as the branch (the technical plan says "the branch's HEAD", which would make it a one-line change to `f"{state.branch}~1"`) or as the working tree (which after the checkout genuinely has changed). It is left as it is because the story's stated bias is toward under-refusing, and it is written down here so the next reader decides it deliberately.
Expand Down
50 changes: 50 additions & 0 deletions orchestration/story_coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,23 @@ def _revision(root: Path, revision: str = "HEAD") -> str:
return result.stdout.strip() if result.returncode == 0 else ""


def same_repository(target_root: Path, harness_root: Path) -> bool:
"""Whether both roots are one git repository.

Decided from `git rev-parse --show-toplevel` in each root: shared only when
both invocations succeed and report the same path. A failing invocation on
either side is the not-established answer and returns False, which is the
same one-directional bias `_revision`, `dirty_paths`, `completion_commits`
and `base_problems` already take — a root that is not a git repository
reports not-shared rather than a false shared.
"""
target = _git(target_root, "rev-parse", "--show-toplevel")
harness = _git(harness_root, "rev-parse", "--show-toplevel")
if target.returncode != 0 or harness.returncode != 0:
return False
return target.stdout.strip() == harness.stdout.strip()


def dirty_paths(target_root: Path) -> list[str]:
"""The paths `git status --porcelain` reports as uncommitted, sorted.

Expand Down Expand Up @@ -1874,6 +1891,24 @@ def unchanged_since_escalation(
not-the-same, so an absent digest, an escalation that committed nothing, a
target whose HEAD cannot be read, or a harness root that is not a git
repository produces no refusal rather than a false one.

The third comparison has two forms, decided by whether the harness and the
target are one checkout. When they are separate — the deployment the guard
was written for — it is the recorded-revision comparison, unchanged. When
they are one tree, deferring to the branch comparison is what is sound:
leg two has already established that the branch is exactly the escalation
commit with nothing uncommitted, and under one tree that covers every
change to the harness source, so the recorded revision would answer a
question already answered. It could not answer it anyway — the revision is
recorded as the first act of `_escalate`, before the two escalation commits
move HEAD, so a shared-root comparison of it always differs and the guard
can never refuse.

A tree hash over the harness's own source directories was considered and
not taken. It is a second definition of "has the harness changed" living
beside the branch comparison that already answers it, and it would have to
choose which directories count — a choice nothing else in the harness
makes.
"""
if not state.story_digest or state.story_digest != story_digest(story_text):
return []
Expand All @@ -1898,6 +1933,21 @@ def unchanged_since_escalation(
f"{state.escalation_commit[:12]}, with nothing uncommitted"
)

# Under one checkout the branch comparison above is the harness comparison:
# it established that the branch is exactly the escalation commit with
# nothing uncommitted, which covers every change to the harness source. The
# recorded revision cannot serve here — it is recorded before the escalation
# commits move HEAD, so it always differs — and a tree hash over the
# harness's source directories was rejected rather than reached for: it is a
# second definition of "has the harness changed" beside the one already
# answering it, and it would have to choose which directories count.
if same_repository(target_root, harness_root):
evidence.append(
"the harness is the same checkout as the target, so it is covered "
"by the branch comparison above"
)
return evidence

if not state.harness_revision or _revision(harness_root) != state.harness_revision:
return []
evidence.append(
Expand Down
Loading
Loading