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: 9 additions & 1 deletion .harness/docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ Thin entry points only; no orchestration logic. `l5-init`, `l5-plan`, `l5-run`,

`l5-plan` is the exception to "the coordinator renders prompts": the planner is not a workflow stage, so the script loads `prompts/planner.md` with `context_assembler.load_template`, renders it with `context_assembler.render`, and passes the rendered text to `--append-system-prompt`. The render context is `schema_context(HARNESS_ROOT)` merged with `workflow_context(workflow, rules)`: since story-009, `l5-plan` locates the target repository like every other run script (`find_target_root`, same no-config refusal, no session started), loads the target's config, loads a workflow and the execution rules from HARNESS_ROOT, and injects the stage list, per-stage create restrictions, and blocked paths alongside the story schema. Since story-069 *which* workflow is `[--workflow <name>]`, parsed ahead of the request text the way `--base` is; the script also sets `workflow_name` in the render context from the name it passed to `load_workflow`, so the prompt cannot name a workflow other than the one it was rendered against, and a name with no definition refuses before the session starts. See "A run executes the workflow its work item names" below. The flag used to default to the workflow the config names; since story-072 it does not, and **the script reads the configured `workflow` key on no path** — without the flag the name comes from a proposal a developer confirmed, and without a terminal to confirm at, the invocation is refused. See "The planner proposes the workflow it plans for" below. Since story-096 the same loop also parses `--brief <key>`, which names a brief to plan *instead of* giving the request as text and is refused in combination with it; where it is given, the request the session receives is what `brief_fetch.render` made of the fetched brief and the workflow decision has a proposal already — see "l5-plan plans from a brief" below. Requiring a target was not a loss — a planner that cannot see the project cannot list `.harness/stories/` to assign the next story number either. All lookup, loading, and rendering stays in orchestration; the script wires them together. Since story-104 `l5-assist` renders `prompts/assist.md` the same way, through the same `load_template`/`render` pair, so the shared prose partial reaches it too — see "A second shared partial joined it" above. Since story-099 `l5-assist` also passes `--plugin-dir` naming the shipped `plugin/` directory and exports `L5_HARNESS_ROOT` before the exec, so the session it starts has the harness's skills and can name an entry point without a path relative to wherever the developer was standing. It keeps its `os.execvp`: it has nothing to do after the session, which is what `l5-plan` gave the exec up for.

Since story-023 the script no longer ends in `os.execvp`. It runs the session as a **child process with stdin, stdout and stderr inherited** — the developer's terminal is the session's terminal — so the script outlives the session and can commit what the session wrote. The argument list handed to `claude` is byte for byte what the exec passed, and everything before the session (target-root discovery, config loading, workflow and rules loading, the render context, the rendered planner prompt) is unchanged; the exec was replaced, not the script around it. The session's exit status becomes the script's, a child reaped with a negative return code is translated to the shell's `128 + signal`, and a `KeyboardInterrupt` in the parent is 130 — so both routes to an interrupt exit 130 and both still commit an artifact already on disk. The only new logic in the script is a `report()` function that turns `plan_commit`'s return values into stdout lines and a status; the snapshot, the commit, the push and the remote resolution live in `orchestration/plan_commit.py`, per "thin entry points only". `l5-run`, `l5-status`, `l5-assist` and `l5-init` keep the process model they had.
Since story-023 the script no longer ends in `os.execvp`. It runs the session as a **child process with stdin, stdout and stderr inherited** — the developer's terminal is the session's terminal — so the script outlives the session and can commit what the session wrote. The argument list handed to `claude` is byte for byte what the exec passed, and everything before the session (target-root discovery, config loading, workflow and rules loading, the render context, the rendered planner prompt) is unchanged; the exec was replaced, not the script around it. The session's exit status becomes the script's, a child reaped with a negative return code is translated to the shell's `128 + signal`, and a `KeyboardInterrupt` in the parent is 130 — so both routes to an interrupt exit 130 and both still commit an artifact already on disk. **A third landing was missed until story-150, and it is worth reading before adding another act to this script.** The `try` that caught `KeyboardInterrupt` covered the session subprocess alone, so an interrupt arriving in everything the script does *afterwards* — committing, stamping, validating, publishing, or waiting at either of the script's own prompts — left `main` through an uncaught `KeyboardInterrupt`, CPython restored the default handler and re-raised, and the process **died by the signal rather than exiting**. A shell shows 130 for both ends, which is why it stood from story-023 to story-150 unnoticed; anything that reads the process's return code instead — a test, a script, the coordinator — sees `-SIGINT` for one and `130` for the other. The status normalisation and the `report(...)` call are now inside a second `try` whose `except KeyboardInterrupt` prints one line and exits `INTERRUPTED_EXIT_CODE`, which is where 130 is named once for both sites. Nothing on that path is undone, retried or completed: whatever the acts above the interrupt had already committed, stamped, pushed or published stands as they left it, and the acts below it did not happen — which is what dying by the signal already meant. The guard is at the call site in `main` rather than inside `report`, so the scan that holds `report` to writing and removing nothing (below) still reads the same function. **The general point for a later act: an act added inside `report` inherits this handling, and one added after `sys.exit` would not exist to inherit it — but an act added *outside* both `try` blocks would reopen exactly this hole.** The only new logic in the script is a `report()` function that turns `plan_commit`'s return values into stdout lines and a status; the snapshot, the commit, the push and the remote resolution live in `orchestration/plan_commit.py`, per "thin entry points only". `l5-run`, `l5-status`, `l5-assist` and `l5-init` keep the process model they had.

Outliving the session is what made **plan-time validation** possible, and story-025 put it between the two acts the script already performed. `report()` takes the loaded workflow's `stages` (which `main` already had) and calls `plan_validation.artifact_problems` after `plan_commit.new_artifacts` and before `plan_commit.commit_artifacts`, so an artifact that pre-flight would refuse is never committed. On problems it prints each artifact's problems through `story_coordinator.refuse_bad_story`, says it committed nothing and that the artifacts remain in the working tree, and returns 1 — `sys.exit(status or committing)` is unchanged, so the session's own status still wins and an interrupt still exits 130. It **refuses rather than warns**, for all three classes: a warning at plan time is a warning nobody sees, because the session that could have repaired it has ended. Nothing is committed, pushed, deleted or rewritten on that path, and no automatic repair is attempted — the developer must see the problems and decide, and the artifact stays exactly where the session wrote it, which is the state to fix and re-run from. When one of several new artifacts fails, **none** is committed: the commit is one commit for the session's artifacts and a partial one would invent a split the session did not have. The no-new-artifact early return above it is untouched — a session that wrote nothing still says it committed nothing and exits on the session's own status. The script gained no decision and no composition; both live in `plan_validation`. **The check holding that guarantee reads `report` rather than the whole script, since story-076.** It scans for `.unlink(`, `.write_text(`, `shutil.` and `os.remove`, and scanning the whole file was a sound proxy for "a refused artifact is left as the session wrote it" only while the script wrote and removed nothing *anywhere* — which ended when phase one's answer and transcript moved into the script's own hands. The subject is now the function that validates and refuses, resolved through `conftest.function_source`; the pattern set, the strictness and the non-vacuity control are unchanged. A later story adding a write elsewhere in `l5-plan` should scope the check to the function whose guarantee it is rather than widen the pattern set, for the same reason: a whole-file scan over a file that legitimately writes reports the wrong thing in both directions. story-030 put a second check in the same gap, immediately below validation and still above the commit: the base check, which asks whether the branch about to receive the plan artifact is the base at all. Same shape, same refusal helper, same "committed nothing, the artifacts remain in the working tree" line — see "The base a story branches from" below. There is deliberately **no `l5-validate`** and no separate command a developer or the planner agent must remember to run: forgetting it returns the harness to the behaviour the story removed, which is also why `planner.md` gained no step telling the planner to validate its own output — the prompt paragraph is a courtesy and the script's check is the enforcement.

Expand Down Expand Up @@ -1426,6 +1426,14 @@ The section above left the suite parallel and said that a test failing under loa

**That rewrite needs a control, because the assertion it replaced was doing two jobs.** An outcome carrying an exit status is consistent with a helper that waits on the fetch's *output* too, unless something shows the two waits coming apart. So `test_that_same_subject_waited_on_through_its_output_does_time_out` drives the identical subject — written once by `escapes_into_its_own_session` rather than reproduced, so "identical" is a fact rather than a resemblance — through `Popen` and `communicate(timeout=...)` under the short bound, and observes the expiry. Its soundness is load-independent for the same reason the kill halves are: the escaped descendant holds the inherited pipes far past that bound, and a loaded machine holds them longer. The `CLASSIFICATION` entry for the module stays `BOUNDS_ONLY_SLOWNESS` and its comment now argues this pair as well as the kill cases, because that comment is where a later reader weighs the classification.

**A fourth route is to remove the race rather than to decide around it, and story-150 is the pattern.** `tests/test_plan_commit.py::test_an_interrupt_still_commits_what_was_written_and_exits_130` drove a sequence and then trusted the order it happened in: it signalled the process group and only *then* wrote the two answers the interrupted invocation needs into the pty behind it, so the result rested on an interleaving the scheduler chose. Neither a bound nor an artifact was the subject here — the answers were simply written too late to be certain of arriving, and an answer already sitting in a terminal is there whenever the child looks, with nothing about when it looks able to change what it finds. `run_plan_on_a_pty` therefore takes an optional `reply` written into the master **between `pty.openpty` and `Popen`**, defaulting to writing nothing so every existing caller starts a child with an empty terminal exactly as before — including the one in `tests/test_the_approval_is_observed.py`, which writes its answers behind the child on purpose because the gap that opens is what it measures. This is the arrangement `tests/conftest.py`'s `a_terminal_for_stdin` already makes for every other module that drives `l5-plan` through a terminal, and the reason is the same one. **The general rule: an input the subject will need is put where it is waiting before the subject exists, not written after the subject is running.**

**Pre-loading removes a race and does not by itself make a status deterministic, which is why that case still carries a precondition.** With the answers waiting, a machine that starves the invocation past the stub's sleep and past the script's own post-session work leaves nothing for the signal to reach — and a signal that reaches nothing is a run that never conducted the case rather than an answer to it. The case reads that off the invocation rather than off a clock: `process.poll()` before the `killpg`, and a `ProcessLookupError` out of it, each report through `machine_load.inconclusive`. The module's `CLASSIFICATION` entry stays `CARRIES_PRECONDITIONS`. Both of that case's assertions are exactly as they were — the status is 130 and the commit holds exactly the one artifact the session wrote — and the stub's thirty-second sleep is unchanged, because shortening it would tighten the race rather than remove it.

**Reaching a point *inside* a subject is done by what the subject printed, not by waiting a chosen number of seconds.** The new case covering the script's post-session landing has to interrupt `l5-plan` after the session has ended, and `wait_for_the_pty_to_say` gets it there: a line the script prints on its way into a question is the script saying it is at that question, so a slow machine changes when that line arrives rather than whether it does. The helper tells two endings apart, and the distinction is the same one the paragraph on preconditions draws. Not reaching the line inside the deadline is `machine_load.inconclusive` — the question went unasked. Reaching **end of output** without the line is an `AssertionError`, because no amount of load makes a script skip a question it is written to ask.

**Where the case's cost actually goes is recorded in the case, because the previous comment invited the wrong conclusion.** The comment read as though the case were expensive from waiting the stub's thirty-second sleep out. The signal goes to the process **group**, so it reaches the stub as well, and the stub dies of it at once. Essentially all of the run is spent *before* the signal, getting the session to a point where there is something to interrupt at all — the base refresh, the id reservation pushed to the remote, the worktree, the build-state link, the workflow load, the prompt render, and the stub's own start. story-150's measurements of that split are in a run directory and reach no clone; what the repository holds is the account in the case's docstring in `tests/test_plan_commit.py`, which is where the next reader of that case will be standing.

**The ripple landed where a nested suite is assembled by copying files.** `tests/conftest.py` now imports from `tests/machine_load.py`, so every module that builds a throwaway tests directory has to copy the helper beside the conftest or the copy cannot import at all — `test_contract_assertions_bite.py`, `test_schema_inventory_location.py` and the two copied-tests tuples in `test_config_keys_are_obeyed.py` each gained the name. That is the recurring shape of giving `tests/conftest.py` a new import: the modules that will redden are the ones enumerating which test files a constructed harness gets, and a plan touching the conftest should expect them rather than discover them. `tests/conftest.py` also inserts its own directory on `sys.path` explicitly rather than relying on pytest having done it, because a ceiling that silently failed to install is exactly the quiet disappearance it exists to prevent.

## The revert check asks one test first
Expand Down
1 change: 1 addition & 0 deletions .harness/history/execution-log.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,4 @@
{"story_id": "story-147", "status": "completed", "retry_count": 1, "timestamp": "2026-09-14 18:43:39"}
{"story_id": "story-148", "status": "completed", "retry_count": 1, "timestamp": "2026-09-14 21:49:01"}
{"story_id": "story-149", "status": "completed", "retry_count": 0, "timestamp": "2026-09-15 09:31:50"}
{"story_id": "story-150", "status": "completed", "retry_count": 0, "timestamp": "2026-09-15 12:47:39"}
1 change: 1 addition & 0 deletions .harness/history/inspection-log.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@
{"story_id": "story-148", "mode": "narrow", "cost_usd": 2.7776145, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-14 21:04:24", "findings": 2, "filed": 0, "dropped": 0, "dedupe_ran": true}
{"story_id": "story-148", "mode": "narrow", "cost_usd": 3.7401380000000004, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-14 21:37:12", "findings": 4, "filed": 0, "dropped": 0, "dedupe_ran": true}
{"story_id": "story-149", "mode": "narrow", "cost_usd": 1.749655, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-15 09:20:14", "findings": 1, "filed": 0, "dropped": 0, "dedupe_ran": true}
{"story_id": "story-150", "mode": "narrow", "cost_usd": 2.4072125, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-15 12:37:54", "findings": 1, "filed": 0, "dropped": 0, "dedupe_ran": true}
1 change: 1 addition & 0 deletions .harness/history/mandate-log.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,4 @@
{"story_id": "story-147", "timestamp": "2026-09-14 17:45:08", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false}
{"story_id": "story-148", "timestamp": "2026-09-14 20:41:55", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false}
{"story_id": "story-149", "timestamp": "2026-09-15 08:36:13", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false}
{"story_id": "story-150", "timestamp": "2026-09-15 12:12:00", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false}
Loading
Loading