diff --git a/.harness/config.yaml b/.harness/config.yaml index 8c3fd89..8ffeb42 100644 --- a/.harness/config.yaml +++ b/.harness/config.yaml @@ -15,6 +15,12 @@ standards_dir: .harness/standards architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: .venv/bin/python -m pytest tests/ -q +# Where this repository's tests live. It is what the workflow's {{tests_dir}} +# token resolves to, so the stage restricted from creating tests is restricted +# here and the stage that writes them is told to write them here. There is no +# default: leave it unset and the target declares no test directory at all, and +# the restriction resolves out of the workflow entirely. +tests_dir: tests/ # The executable the clean-clone check runs the suite under. Deliberately an # older environment than the one the developer works in, and the one CI # exercises, so an incompatibility is found before CI rather than by it. It diff --git a/.harness/docs/ARCHITECTURE.md b/.harness/docs/ARCHITECTURE.md index afaa677..83e1a14 100644 --- a/.harness/docs/ARCHITECTURE.md +++ b/.harness/docs/ARCHITECTURE.md @@ -24,7 +24,7 @@ l5 is a level 3 agentic harness: a story execution system. The workflow defines A stage that writes to the repository declares an optional `changed_files` key naming its changed-files record: the implementer declares `changed-files.json`, the tester declares `tester-changed-files.json`, and since story-044 the documenter declares `documenter-changed-files.json`. After any stage with this declaration completes, the coordinator checks that record against `blocked_paths` and escalates on violation — enforcement is driven by the workflow definition, with no stage names hard-coded in the coordinator. Adding the documenter proved that: the diff that enabled the check touched no file under `orchestration/`, only the stage's `outputs`, `changed_files` and `schemas` entries in the workflow and the prompt paragraph asking for the record. The cost landed instead in the fixtures — the record became a required artifact, so every fake runner in `tests/` that drives the shipped workflow through the documenter had to start writing it. The declaration turns on the required-artifact check and the schema check along with the blocked-path check; it does not bring `may_not_create`, a revert check or a stage baseline, which remain separate declarations the documenter does not carry. All three records share one schema definition (`modified`/`created`/`deleted` arrays), not three copies. The documenter's record was written outward and read by nothing while the documenter ran last; since story-045 the verifier runs after it and receives it through `{{documenter_changed_files}}`, exactly as it receives the implementer's through `{{changed_files}}`, so all three records are now both checked and read. -A stage may also declare an optional `may_not_create` key, a list of repository-relative path prefixes it is not allowed to add files under. The implementer declares `["tests/"]`; no other stage does. After a stage that declares both `changed_files` and `may_not_create`, the coordinator reads that stage's own record and escalates when any entry in its **`created`** array falls under a declared prefix. `modified` and `deleted` are not examined by *this* check — the rule is about independence, not about directories: an implementer must be able to update an existing test whose call site its own signature change broke, but validation it authors itself checks what it built rather than what was asked. Since story-017 those two arrays are decided by the revert check below rather than left unexamined. As with blocked paths, no stage name and no prefix appears in orchestration code; both are read off the stage dict. +A stage may also declare an optional `may_not_create` key, a list of repository-relative path prefixes it is not allowed to add files under. The implementer declares `["{{tests_dir}}"]` — since story-046 the prefix is a **reference to the target's configuration** rather than a literal directory, resolved when the definition loads (see "A workflow declaration may reference configuration" below); for this repository it resolves to `tests/`, which is what the declaration used to say. No other stage declares the key. After a stage that declares both `changed_files` and `may_not_create`, the coordinator reads that stage's own record and escalates when any entry in its **`created`** array falls under a declared prefix. `modified` and `deleted` are not examined by *this* check — the rule is about independence, not about directories: an implementer must be able to update an existing test whose call site its own signature change broke, but validation it authors itself checks what it built rather than what was asked. Since story-017 those two arrays are decided by the revert check below rather than left unexamined. As with blocked paths, no stage name and no prefix appears in orchestration code; both are read off the stage dict. A stage may also declare an optional `revert_check` key, an object naming both artifacts that check needs: `result`, the record it writes, and `baseline`, the run-directory directory its pre-stage content is captured into. The implementer declares `{"result": "revert-check-result.json", "baseline": "stage-baseline"}`. One key still turns the whole thing on, exactly as `clean_clone` does on the verifier: the coordinator reads `stage.get("revert_check")` and does nothing when it is absent, so removing the declaration disables the capture *and* the check with no change to orchestration code and neither name appears there. It was a bare string until story-019 needed a second name and widened it rather than adding a second key — one declaration, so the capture and the check cannot be switched on independently and disagree. The check runs immediately after the ownership check, inside the same `changed_files` block, on the same record, the **same enforced prefix list** and the **same exemption**. Since story-032 that list is the stage's `may_not_create` declarations *unshortened*, and the story's grants are carried beside it as a set of exempt paths rather than subtracted from it; both checks skip a path `grant_covers` covers. A story holding a `stage_exception` is therefore not subject to the revert check on the granted path either, from reuse rather than from a second subtraction — the property is unchanged, only its mechanism moved. See "Work a stage cannot own" below. @@ -60,7 +60,7 @@ Since story-032 the `create` value is **a path at or beneath** one of that stage `technical_plan` deliberately carries no `type` keyword. story-001 and story-002 write it as a free-form block scalar and story-003 onward write it as a structured object; the validator subset has no union keyword, and those artifacts are committed and unedited. Omitting `type` keeps every nested constraint biting — `schema_validator` applies `properties`/`required` only when the value is a dict — so a malformed *object* form is still rejected while a block scalar is accepted. The reason is recorded in that property's own `description`. -One file in the inventory declares a *contract* rather than an artifact shape. Since story-039 the set of configuration keys the harness reads is declared in **`schemas/harness-config.schema.json`** — one property per key, thirteen of them (`allowed_tools`, `architecture_docs`, `base_branch`, `branch_prefix`, `logs_dir`, `model`, `permission_mode`, `runs_dir`, `standards_dir`, `stories_dir`, `test_command`, `verification_runner`, `workflow`), each typed as `load_config` produces it and described by what it governs and what it falls back to. It is in `schemas/manifest.json` like every other file there, and passes the same parametrized draft-2020-12, unsupported-keyword and no-`additionalProperties` checks, but no stage is asked to satisfy it because no agent produces a config file. `harness_config.declared_config_keys(harness_root=None)` is its **only reader**, resolving it relative to its own module through `schema_validator.load_schema` so `schemas/` keeps one reader, and raising `ValueError` naming the path on a missing, unparseable or wrong-shaped schema rather than degrading to an empty or partial tuple — a degraded return would make the coverage below vacuous instead of red. +One file in the inventory declares a *contract* rather than an artifact shape. Since story-039 the set of configuration keys the harness reads is declared in **`schemas/harness-config.schema.json`** — one property per key, fourteen of them since story-046 (`allowed_tools`, `architecture_docs`, `base_branch`, `branch_prefix`, `logs_dir`, `model`, `permission_mode`, `runs_dir`, `standards_dir`, `stories_dir`, `test_command`, `tests_dir`, `verification_runner`, `workflow`), each typed as `load_config` produces it and described by what it governs and what it falls back to. `tests_dir` carries no default, and unlike `test_command` — which is also read without a fallback, but whose absence simply stops two checks — its **absence is itself a declaration**: set, it is the prefix the workflow's `{{tests_dir}}` token resolves to; unset, the target declares no test directory and the restriction resolves out of the definition entirely — see "A workflow declaration may reference configuration" below. It is in `schemas/manifest.json` like every other file there, and passes the same parametrized draft-2020-12, unsupported-keyword and no-`additionalProperties` checks, but no stage is asked to satisfy it because no agent produces a config file. `harness_config.declared_config_keys(harness_root=None)` is its **only reader**, resolving it relative to its own module through `schema_validator.load_schema` so `schemas/` keeps one reader, and raising `ValueError` naming the path on a missing, unparseable or wrong-shaped schema rather than degrading to an empty or partial tuple — a degraded return would make the coverage below vacuous instead of red. Since story-043 the declaration is also a **run-time check, and it is strict**: `harness_config.undeclared_config_problems(config, harness_root=None)` calls `declared_config_keys` and returns one problem per key a loaded config carries that the schema does not declare, in the order the config carries them. `run_story` calls it immediately after `load_config` and refuses through `_refuse_undeclared_config_keys` and the shared `refuse()` — **above** the workflow load and above every other pre-flight, so a refused run creates no run directory, no state file, no log, no branch, and invokes no agent. Only key *names* are examined; no value is validated, coerced or constrained, and `load_config`'s parsing is untouched, so a comment naming an unknown key is stripped before any key is seen and refuses nothing. `tests/test_undeclared_config_keys.py` holds the refusal's coverage — it replaced the retired-key module story-041 added and this story deleted, whose subject no longer exists — and includes a sweep asserting that no fixture configuration under `tests/` carries an undeclared key, which is what keeps the rest of the suite runnable under the strict rule. @@ -70,7 +70,7 @@ Since story-043 the declaration is also a **run-time check, and it is strict**: **`project` was removed rather than declared**, from `.harness/config.yaml`, `templates/config.yaml`, the `{project}` substitution in `scripts/l5-init`, and every fixture configuration under `tests/`. It was the one key a shipped config carried that nothing reads, and under the strict rule it would have refused this repository's own config. Declaring it was not available: the declared set *is* the set of keys the harness reads, and story-039's three coverage checks require every declared key to be read, to carry a proof, and for that proof to go red when its read is replaced by its fallback — none of which a key nothing reads can satisfy. Adding a key to `templates/config.yaml` therefore now means adding it to the schema and giving it a proof, or not adding it at all. -What the declaration is for is coverage, and the coverage is **set equality in both directions**, twice, in `tests/test_config_keys_are_obeyed.py`. Against `KEY_PROOFS` — a key declared with no proof fails, a proof naming an undeclared key fails — and against an AST scan of `orchestration/` and `scripts/` collecting every `config.get("...")` and `config["..."]`, extensionless scripts included, so a key the harness reads and the schema does not declare fails. Neither comparison is against a second maintained list. Every declared key carries a proof that **varies** it: the value is one the harness would never pick (each carries the token `xyzzy`, and an assertion checks that none coincides with the key's default or with this repository's own configured value), and ten keys are proven behaviourally while `model`, `permission_mode` and `allowed_tools` are proven on the invocation built for a fake runner, because those three are handed to the agent runner and observable nowhere else. `KEY_PROOFS` records which of the two each key gets, so a reader knows what *proven* means for it. A mutation control then replaces each key's read with its fallback literal in a throwaway copy of `orchestration/` and requires that key's proof to go red there, so a proof that sets a key and asserts nothing about its effect cannot survive. This subsumes but does not retire story-028's absence assertions: proving a literal is *absent* and proving the configured value *governs* are different claims, and both are kept. +What the declaration is for is coverage, and the coverage is **set equality in both directions**, twice, in `tests/test_config_keys_are_obeyed.py`. Against `KEY_PROOFS` — a key declared with no proof fails, a proof naming an undeclared key fails — and against an AST scan of `orchestration/` and `scripts/` collecting every `config.get("...")` and `config["..."]`, extensionless scripts included, so a key the harness reads and the schema does not declare fails. Neither comparison is against a second maintained list. Every declared key carries a proof that **varies** it: the value is one the harness would never pick (each carries the token `xyzzy`, and an assertion checks that none coincides with the key's default or with this repository's own configured value), and eleven keys are proven behaviourally while `model`, `permission_mode` and `allowed_tools` are proven on the invocation built for a fake runner, because those three are handed to the agent runner and observable nowhere else. `tests_dir`'s proof is behavioural in both of the places the key reaches: a fixture configuring `xyzzy-checks/` is governed there, observed through `stage_restrictions` and through the rendered tester prompt, with the workflow definition asserted to name no directory of its own. `KEY_PROOFS` records which of the two each key gets, so a reader knows what *proven* means for it. A mutation control then replaces each key's read with its fallback literal in a throwaway copy of `orchestration/` and requires that key's proof to go red there, so a proof that sets a key and asserts nothing about its effect cannot survive. This subsumes but does not retire story-028's absence assertions: proving a literal is *absent* and proving the configured value *governs* are different claims, and both are kept. ### Prompts (`prompts/`) @@ -82,7 +82,7 @@ A prompt states a boundary; it does not hold it. `implementer.md`'s do-not list `{{self_route_result}}` is the same move for the same reason, one step further out. It sits in the runtime state layer of all four workflow-stage templates — `implementer.md`, `tester.md`, `verifier.md` and `documenter.md` — because any stage can fail mechanically, and each says in one sentence that the coordinator wrote it rather than an agent and that no verifier judged the work. It renders as `None` for every stage that did not self-route, including a stage running after another stage self-routed earlier in the same run. -`verifier.md` takes the same treatment for the routing categories it must choose between: `{{retry_routes}}` injects one line per declared category — the category, its destination stage and its `when` description — beside the verification-result schema, and the surrounding prose says only that a recommended retry must name one of them in `retry_target` and that the coordinator escalates on a missing or unknown one. The template **names no category**, so adding one to `workflows/story-workflow.json` changes the rendered prompt with no edit to any prompt file. `tester.md` gained `{{retry_guidance}}` and `{{retry_state}}` in the same story: the tester can now be a retry destination, and until then it had neither, because it could only ever be arrived at going forward. +`verifier.md` takes the same treatment for the routing categories it must choose between: `{{retry_routes}}` injects one line per declared category — the category, its destination stage and its `when` description — beside the verification-result schema, and the surrounding prose says only that a recommended retry must name one of them in `retry_target` and that the coordinator escalates on a missing or unknown one. The template **names no category**, so adding one to `workflows/story-workflow.json` changes the rendered prompt with no edit to any prompt file. `tester.md` gained `{{retry_guidance}}` and `{{retry_state}}` in the same story: the tester can now be a retry destination, and until then it had neither, because it could only ever be arrived at going forward. It gained `{{tests_dir}}` in story-046 for the same reason `verifier.md` names no category: the template now names no test directory, so changing the target's configured location changes the rendered prompt with `tester.md` unedited, and `prompts/` as a whole carries no target directory name and no test-framework filename. story-045 is the first real test of both halves of that. `documenter.md` gained `{{retry_guidance}}` for exactly the reason `tester.md` did — becoming a retry destination brought it under the standing rule that every stage a route can name must be able to say it is on a retry — and the third category reached the verifier's prompt with **no edit to `verifier.md`'s routing prose**, which is the property story-028 landed and nothing had exercised until a third route existed. What `verifier.md` did gain is the new subject rather than the new category: two runtime-state placeholders, `{{documenter_changed_files}}` and `{{documentation_report}}`, and one responsibility line saying the documenter's output is part of what the verifier evaluates. The template still names no category, no destination and no `when`. @@ -94,12 +94,12 @@ The drift source that paragraph used to name is closed: `planner.md` no longer s ### Orchestration (`orchestration/`) -- `story_coordinator.py` — the Story Coordinator. Loads the workflow definition, story artifact, and rules; creates the story branch and run directory *or resumes an existing one*; loops: determine stage → assemble context → render prompt → invoke agent → save artifacts → update state → route (advance, retry, or escalate). `run_story` takes an optional keyword-only `start_stage` overriding where execution enters — the recorded stage on a resume, `stage_names[0]` on a fresh run. It is named `start_stage` rather than `stage` because `stage` is the loop's name for the stage being executed. A `start_stage` the loaded workflow does not define is refused above everything else, in the same shape as the other pre-flight refusals: exit 1, one message naming the stages the workflow does define, nothing created and no agent invoked. See "Resuming a run" below for the resume branch, the escalation commit and the guard. Post-stage checks run in a fixed order: required artifacts present → required artifacts written by *this* attempt → declared artifacts match their schemas → changed-files record clear of blocked paths → stage output ownership → the revert check. The freshness check sits immediately after the presence check and never before it, so an artifact that is genuinely absent keeps its own missing-artifacts reason and the two stay distinguishable; see "An artifact's presence is not evidence of its authorship" below. Those first two, together with a failed agent process above them, are the three **mechanical** failures, and since story-036 each is routed through `self_route` rather than escalating outright: the stage runs again in place while it has unspent budget, and escalates with the same reason it always used when it has none. The checks after them — schema violations, blocked paths, ownership, the revert check — are unchanged and still escalate. See "A stage that failed mechanically runs again" below. Schema validation sits in the middle deliberately, so a malformed `changed-files.json` escalates with a validation error naming the field rather than raising out of the blocked-paths check that reads the same file. Ownership runs last, on the same record, after that record is known to be well-formed and clear of blocked paths. `_ownership_violation` returns a frozen `OwnershipViolation(path, prefix)`, and the escalation reason names stage, path, and prefix in both `events.log` and `escalation-summary.md`. `granted_paths(story, stage_name)` reads the story's grants for that stage and `grant_covers(granted, path)` decides whether any of them covers a path; the enforced list is passed whole and the grants are handed alongside it as an exemption, so `_ownership_violation(run_dir, record_name, prefixes, granted)` skips a covered path rather than the prefix being removed. Each grant is appended to `events.log` as `stage exception applied: may create `, one per grant whatever its granularity, so routing stays reconstructable from the log alone. The revert check sits immediately after, in the same block, reusing that same `enforced` list *and* that same exemption rather than recomputing either: `governed_edits(run_dir, record_name, prefixes, granted)` returns a frozen `GovernedEdits(paths, prefixes)` holding the sorted `modified` and `deleted` entries under any of the prefixes and covered by no grant, plus the prefixes that matched, and it names no stage, no prefix and no artifact. When `paths` is empty nothing at all happens — no clone, no suite, no artifact — because a check that can say nothing should not run. Otherwise `revert_check(run_dir, target_root, config, artifact, paths, baseline)` is shaped exactly like `clean_clone_check`: `tempfile.mkdtemp` scratch, the shared `run_clean_clone` with `revert` set to the governed paths, `shutil.rmtree` in a `finally` whatever the result, and `RevertCheckResult.as_record()` written under the declared artifact name. `permitted` is `result.exit_code != 0`. Two conditions stop it from running and neither permits: a stage that declares the check with no baseline captured (decided *before* any clone is attempted, so the reason names the missing directory rather than surfacing as a generic clone failure) and a clone that cannot be built. A check that did not run escalates naming the reason; `permitted` false escalates naming the stage, the prefixes and the paths; `permitted` true appends `_revert_check_permitted` and falls through to the existing advance. Both escalations go through `_escalate`, which does not touch `retry_count`. Since story-020 `_escalate` takes `target_root` and `harness_root` as required keywords — it commits and it records the harness revision — so every escalation site forwards both; a new escalation that forgets them is a `TypeError` rather than a run that quietly leaves its work uncommitted. `_build_clone` and `run_clean_clone` gained a `revert` parameter for this and, in story-019, the `baseline` the reverted content is restored *from*; both default to reverting nothing, so the clean-clone check is untouched. When `revert` is non-empty the restore runs inside the clone *after* the working-tree diff is applied and the untracked files are copied and *before* `git add -A`, so the clone commits those paths as the stage found them while every other change is present: the baseline's copy is copied over the clone's for each governed path it holds, and each governed path it does **not** hold is deleted in the clone, because a path absent from the baseline did not exist when the stage started. Deleting rather than skipping is the point — skipping decides nothing and would report a permission the check never established, which is the assertion-that-cannot-fail failure mode `tests/test_baseline_honesty.py` exists to prevent. Naming paths with no baseline raises `RuntimeError` naming them. No code path in the check reverts to HEAD any longer. Pre-flight story reading is one function, `read_story(story_text)`: load `story.schema.json`, parse with it, validate against it, and return a frozen `StoryReading` carrying both the `parsed` story (`None` when parsing failed) and the `problems` list. It runs above the run-directory creation and the branch checkout, so a rejection is an exit-1 refusal leaving no run directory, no `state.json`, no log, and no new branch — and no agent invoked. It is called exactly once per run, and the parse it returns is the run's only reading of the artifact: `reading.parsed` is threaded into every `build_context` call and into `_complete`, which takes the completion-report title and commit-message subject from `story["story"]["title"]` rather than scanning lines. A missing title is a loud `KeyError`; the schema marks it required and the run cannot reach `_complete` without having validated. `read_story` stays schema conformance only. Whether a story's `stage_exceptions` mean anything against the workflow *this run loaded* is a separate question the schema cannot answer, so it is a separate function — `stage_exception_problems(story, stages)`, called from `run_story` beside `read_story` and above the run-directory creation. It refuses an exception naming a stage the workflow does not define, and one granting a value beneath no prefix that stage was restricted on: an exception that grants nothing is a planning error, not a harmless one. Matching was exact equality against the declared prefixes until story-032 widened it to *containment* — a grant may name the whole prefix, a directory beneath it, or a single file beneath it — while both refusal messages read as they always did, so `create: tests` against a declared `tests/` still refuses rather than silently granting part of it. The stage → restricted-prefix mapping it needs is derived by `stage_restrictions(stages) -> list[tuple[str, str]]`, sitting immediately above it, which returns the workflow's (stage, prefix) create-restriction pairs in declared order; `stage_exception_problems` builds its per-stage mapping from that helper (taking the stage-name set from `stages` directly, so a stage declaring no `may_not_create` is still known to exist and the "names a stage the workflow does not define" branch is unaffected). The derivation exists once because story-025's plan-time strictness check needs the same pairs, and a second copy would be a second answer to "what does the workflow restrict". A third pre-flight joins them since story-021, the clean-tree check: `dirty_paths(target_root)` runs one `git status --porcelain` and returns the sorted, deduplicated paths, stripping the status codes, reducing a rename's `old -> new` to the new path, and unquoting a quoted path. It only *reads* the target — no commit, no branch, no stash, no index change — and a root that is not a git repository, or a git that fails for any other reason, reports nothing dirty, the same one-directional bias `unchanged_since_escalation` takes. Untracked files count, because a file no stage produced is exactly what `git add -A` would absorb; ignored files do not, which is why a gitignored `.harness/runs/` is not what the check is about. See "The tree a run starts from" below for which runs it applies to and why. A fourth joins them since story-027, the finished-branch check: `story_branch(config, story_id)` derives the branch name from the configured prefix in one place, and `completion_commits(target_root, branch, story_id)` returns one `" "` line per commit reachable from that branch that a finished run of this story made. It sits **above** the clean-tree check, so a developer whose tree is also dirty meets the refusal that makes the run pointless before the one that makes it unaccountable, rather than in two round trips. See "The branch a run starts from" below for the evidence it uses and why. A fifth joins them since story-030, the base check: `resolve_base(target_root, config, base)` settles what a story branch is cut from, and `base_problems(target_root, base, declared)` returns what refuses a run that would cut it from somewhere else. It sits between the finished-branch check and the clean-tree check, gated on the story branch not already existing, and `branch_behind(target_root, branch, base)` feeds the note an *existing* branch gets instead of a refusal. Both entry points read those two functions — see "The base a story branches from" below. A sixth joins them since story-028, and it is the only one that asks about the *definition* rather than about the target repository: `retry_routing_problems(stages)` sits beside `stage_exception_problems` and refuses a declared route whose destination the workflow does not define, and one whose destination does not sit strictly before the stage declaring it — see "Where a retry goes" below. A seventh joins them since story-036 and asks the same kind of question about the definition: `self_route_problems(stages)` sits beside `retry_routing_problems` and refuses a stage whose declared `max_self_routes` is not a non-negative integer — a budget that is not a count cannot be spent, and every run under that definition would meet it. All seven refusals print through one extracted `refuse(header, problems, guidance)`, so the refusal shape (exit 1, one message per problem, nothing created) is a single code path rather than a copy per reason; what differs between them is the sentence above the list and the sentence below it, which is what the six thin callers — `refuse_bad_story`, `_refuse_dirty_tree`, `_refuse_finished_branch`, `_refuse_base`, `_refuse_bad_routing` and `_refuse_bad_self_routes` — supply. `refuse` and `refuse_bad_story` are **public** since story-025, because plan time and pre-flight must print a given defect identically: `l5-plan` reports a failing artifact through `refuse_bad_story`, so the same defect in the same artifact produces the same text whether it is caught when the artifact is written or when it is run, from one function rather than from two that agree today. `_refuse_dirty_tree` stays private — it is a run-time refusal with no plan-time counterpart — and a helper is promoted when a second caller outside the coordinator actually needs it, not on principle. `_refuse_base` is the one place that rule is currently bent and it is recorded rather than glossed: it *does* have a caller outside the coordinator — `l5-plan` prints its base refusal through it, which is what makes the two entry points unable to word the same defect differently — and it kept its underscore. The decision functions the script reads, `resolve_base` and `base_problems`, are public; promote `_refuse_base` to match them in the story that has another reason to touch it. `load_state(run_dir)` moved above the run-directory creation for this, since the clean-tree check decides from the state the run is starting from and `load_state` reads a file rather than requiring a directory; nothing else about the resume decision moved with it. The retry branch archives before it increments: `archive_attempt(run_dir, archivable_artifacts(stages), state.retry_count + 1)` copies the superseded attempt's artifacts under `attempts/attempt-N/` — see the archive decisions below. `append_event(run_dir, message, *, kind, stage, artifacts, duration_seconds, verifier_outcome, retry_decision, retry_reason, retry_category, retry_stage)` is the run's single event write path: the prose message stays positional and is what the `events.log` line is built from, and the *same call* appends one structured entry to `execution-history.json`. `load_history(run_dir)` is the read side, called only by `append_event` for the next sequence number. `run_story` captures `stage_started_at = time.monotonic()` at the stage-started event and reads it through a local `elapsed()` at every event that ends a stage, so a completed stage's entry carries a duration the log only made derivable; `_escalate` forwards whatever structured fields an escalation has and tags its entry `escalated`. The clean-clone check is the last thing the verifier branch does on a passing verdict: `clean_clone_check(run_dir, target_root, config, artifact)` builds a scratch clone with `tempfile.mkdtemp`, runs `run_clean_clone`, removes the scratch directory in a `finally` whatever the result, and writes the returned `CleanCloneResult.as_record()` to the run directory under the declared artifact name. `_build_clone` does `git clone --no-local` from the target's filesystem path — over git's normal transport, see "The clone is built over the normal transport" below — applies the target's tracked edits as `git diff --binary HEAD` piped to `git apply`, copies the untracked-but-not-ignored files from `git ls-files --others --exclude-standard`, then commits inside the clone; the target repository is only read. `_link_interpreter_roots` links the top-level directory of each configured interpreter path into the clone and appends those names to the clone's `.git/info/exclude`, because a virtualenv is gitignored and therefore absent from a fresh clone, and a `.gitignore` entry for a directory does not cover a symlink standing in its place. Zero exit appends `_clean_clone_passed` and falls through to the existing advance; non-zero takes the retry path the verification-failed branch already takes — `archive_attempt` above the increment, then increment, save, `_clean_clone_failed`, and `index = stage_names.index(destination)` — or the existing escalation path at the ceiling, with `_clean_clone_failures` collapsing the output's `FAILED` lines into the one-line reason. Since story-028 the destination on that path is `clean_clone["retry_stage"]`, read off the widened declaration, rather than borrowed from the verifier's own table. Both events are module-level helpers rather than inline calls, for a reason worth keeping: `tests/test_execution_history.py` proves its own non-vacuity by deleting the first `retry_decision="retry",` line at the verification-failed branch's indentation, and an inline clean-clone branch nests deeper and sits earlier in the file, so its line *contains* that indented text and the mutation lands there instead of where it was aimed. +- `story_coordinator.py` — the Story Coordinator. Loads the workflow definition, story artifact, and rules; creates the story branch and run directory *or resumes an existing one*; loops: determine stage → assemble context → render prompt → invoke agent → save artifacts → update state → route (advance, retry, or escalate). `run_story` takes an optional keyword-only `start_stage` overriding where execution enters — the recorded stage on a resume, `stage_names[0]` on a fresh run. It is named `start_stage` rather than `stage` because `stage` is the loop's name for the stage being executed. A `start_stage` the loaded workflow does not define is refused above everything else, in the same shape as the other pre-flight refusals: exit 1, one message naming the stages the workflow does define, nothing created and no agent invoked. See "Resuming a run" below for the resume branch, the escalation commit and the guard. Post-stage checks run in a fixed order: required artifacts present → required artifacts written by *this* attempt → declared artifacts match their schemas → changed-files record clear of blocked paths → stage output ownership → the revert check. The freshness check sits immediately after the presence check and never before it, so an artifact that is genuinely absent keeps its own missing-artifacts reason and the two stay distinguishable; see "An artifact's presence is not evidence of its authorship" below. Those first two, together with a failed agent process above them, are the three **mechanical** failures, and since story-036 each is routed through `self_route` rather than escalating outright: the stage runs again in place while it has unspent budget, and escalates with the same reason it always used when it has none. The checks after them — schema violations, blocked paths, ownership, the revert check — are unchanged and still escalate. See "A stage that failed mechanically runs again" below. Schema validation sits in the middle deliberately, so a malformed `changed-files.json` escalates with a validation error naming the field rather than raising out of the blocked-paths check that reads the same file. Ownership runs last, on the same record, after that record is known to be well-formed and clear of blocked paths. `_ownership_violation` returns a frozen `OwnershipViolation(path, prefix)`, and the escalation reason names stage, path, and prefix in both `events.log` and `escalation-summary.md`. `granted_paths(story, stage_name)` reads the story's grants for that stage and `grant_covers(granted, path)` decides whether any of them covers a path; the enforced list is passed whole and the grants are handed alongside it as an exemption, so `_ownership_violation(run_dir, record_name, prefixes, granted)` skips a covered path rather than the prefix being removed. Each grant is appended to `events.log` as `stage exception applied: may create `, one per grant whatever its granularity, so routing stays reconstructable from the log alone. The revert check sits immediately after, in the same block, reusing that same `enforced` list *and* that same exemption rather than recomputing either: `governed_edits(run_dir, record_name, prefixes, granted)` returns a frozen `GovernedEdits(paths, prefixes)` holding the sorted `modified` and `deleted` entries under any of the prefixes and covered by no grant, plus the prefixes that matched, and it names no stage, no prefix and no artifact. When `paths` is empty nothing at all happens — no clone, no suite, no artifact — because a check that can say nothing should not run. Otherwise `revert_check(run_dir, target_root, config, artifact, paths, baseline)` is shaped exactly like `clean_clone_check`: `tempfile.mkdtemp` scratch, the shared `run_clean_clone` with `revert` set to the governed paths, `shutil.rmtree` in a `finally` whatever the result, and `RevertCheckResult.as_record()` written under the declared artifact name. `permitted` is `result.exit_code != 0`. Two conditions stop it from running and neither permits: a stage that declares the check with no baseline captured (decided *before* any clone is attempted, so the reason names the missing directory rather than surfacing as a generic clone failure) and a clone that cannot be built. A check that did not run escalates naming the reason; `permitted` false escalates naming the stage, the prefixes and the paths; `permitted` true appends `_revert_check_permitted` and falls through to the existing advance. Both escalations go through `_escalate`, which does not touch `retry_count`. Since story-020 `_escalate` takes `target_root` and `harness_root` as required keywords — it commits and it records the harness revision — so every escalation site forwards both; a new escalation that forgets them is a `TypeError` rather than a run that quietly leaves its work uncommitted. `_build_clone` and `run_clean_clone` gained a `revert` parameter for this and, in story-019, the `baseline` the reverted content is restored *from*; both default to reverting nothing, so the clean-clone check is untouched. When `revert` is non-empty the restore runs inside the clone *after* the working-tree diff is applied and the untracked files are copied and *before* `git add -A`, so the clone commits those paths as the stage found them while every other change is present: the baseline's copy is copied over the clone's for each governed path it holds, and each governed path it does **not** hold is deleted in the clone, because a path absent from the baseline did not exist when the stage started. Deleting rather than skipping is the point — skipping decides nothing and would report a permission the check never established, which is the assertion-that-cannot-fail failure mode `tests/test_baseline_honesty.py` exists to prevent. Naming paths with no baseline raises `RuntimeError` naming them. No code path in the check reverts to HEAD any longer. Pre-flight story reading is one function, `read_story(story_text)`: load `story.schema.json`, parse with it, validate against it, and return a frozen `StoryReading` carrying both the `parsed` story (`None` when parsing failed) and the `problems` list. It runs above the run-directory creation and the branch checkout, so a rejection is an exit-1 refusal leaving no run directory, no `state.json`, no log, and no new branch — and no agent invoked. It is called exactly once per run, and the parse it returns is the run's only reading of the artifact: `reading.parsed` is threaded into every `build_context` call and into `_complete`, which takes the completion-report title and commit-message subject from `story["story"]["title"]` rather than scanning lines. A missing title is a loud `KeyError`; the schema marks it required and the run cannot reach `_complete` without having validated. `read_story` stays schema conformance only. Whether a story's `stage_exceptions` mean anything against the workflow *this run loaded* is a separate question the schema cannot answer, so it is a separate function — `stage_exception_problems(story, stages)`, called from `run_story` beside `read_story` and above the run-directory creation. It refuses an exception naming a stage the workflow does not define, and one granting a value beneath no prefix that stage was restricted on: an exception that grants nothing is a planning error, not a harmless one. Matching was exact equality against the declared prefixes until story-032 widened it to *containment* — a grant may name the whole prefix, a directory beneath it, or a single file beneath it — while both refusal messages read as they always did, so `create: tests` against a declared `tests/` still refuses rather than silently granting part of it. The stage → restricted-prefix mapping it needs is derived by `stage_restrictions(stages) -> list[tuple[str, str]]`, sitting immediately above it, which returns the workflow's (stage, prefix) create-restriction pairs in declared order; `stage_exception_problems` builds its per-stage mapping from that helper (taking the stage-name set from `stages` directly, so a stage declaring no `may_not_create` is still known to exist and the "names a stage the workflow does not define" branch is unaffected). The derivation exists once because story-025's plan-time strictness check needs the same pairs, and a second copy would be a second answer to "what does the workflow restrict". A third pre-flight joins them since story-021, the clean-tree check: `dirty_paths(target_root)` runs one `git status --porcelain` and returns the sorted, deduplicated paths, stripping the status codes, reducing a rename's `old -> new` to the new path, and unquoting a quoted path. It only *reads* the target — no commit, no branch, no stash, no index change — and a root that is not a git repository, or a git that fails for any other reason, reports nothing dirty, the same one-directional bias `unchanged_since_escalation` takes. Untracked files count, because a file no stage produced is exactly what `git add -A` would absorb; ignored files do not, which is why a gitignored `.harness/runs/` is not what the check is about. See "The tree a run starts from" below for which runs it applies to and why. A fourth joins them since story-027, the finished-branch check: `story_branch(config, story_id)` derives the branch name from the configured prefix in one place, and `completion_commits(target_root, branch, story_id)` returns one `" "` line per commit reachable from that branch that a finished run of this story made. It sits **above** the clean-tree check, so a developer whose tree is also dirty meets the refusal that makes the run pointless before the one that makes it unaccountable, rather than in two round trips. See "The branch a run starts from" below for the evidence it uses and why. A fifth joins them since story-030, the base check: `resolve_base(target_root, config, base)` settles what a story branch is cut from, and `base_problems(target_root, base, declared)` returns what refuses a run that would cut it from somewhere else. It sits between the finished-branch check and the clean-tree check, gated on the story branch not already existing, and `branch_behind(target_root, branch, base)` feeds the note an *existing* branch gets instead of a refusal. Both entry points read those two functions — see "The base a story branches from" below. A sixth joins them since story-028, and it is the only one that asks about the *definition* rather than about the target repository: `retry_routing_problems(stages)` sits beside `stage_exception_problems` and refuses a declared route whose destination the workflow does not define, and one whose destination does not sit strictly before the stage declaring it — see "Where a retry goes" below. A seventh joins them since story-036 and asks the same kind of question about the definition: `self_route_problems(stages)` sits beside `retry_routing_problems` and refuses a stage whose declared `max_self_routes` is not a non-negative integer — a budget that is not a count cannot be spent, and every run under that definition would meet it. All seven refusals print through one extracted `refuse(header, problems, guidance)`, so the refusal shape (exit 1, one message per problem, nothing created) is a single code path rather than a copy per reason; story-046's unresolved-token refusal joins them on the same terms without being an eighth `*_problems` function — the defect is raised by `load_workflow` as `UnresolvedWorkflowToken` carrying `problems` already in that shape, and `_refuse_unresolved_workflow_token` is one more thin caller; what differs between them is the sentence above the list and the sentence below it, which is what the six thin callers — `refuse_bad_story`, `_refuse_dirty_tree`, `_refuse_finished_branch`, `_refuse_base`, `_refuse_bad_routing` and `_refuse_bad_self_routes` — supply. `refuse` and `refuse_bad_story` are **public** since story-025, because plan time and pre-flight must print a given defect identically: `l5-plan` reports a failing artifact through `refuse_bad_story`, so the same defect in the same artifact produces the same text whether it is caught when the artifact is written or when it is run, from one function rather than from two that agree today. `_refuse_dirty_tree` stays private — it is a run-time refusal with no plan-time counterpart — and a helper is promoted when a second caller outside the coordinator actually needs it, not on principle. `_refuse_base` is the one place that rule is currently bent and it is recorded rather than glossed: it *does* have a caller outside the coordinator — `l5-plan` prints its base refusal through it, which is what makes the two entry points unable to word the same defect differently — and it kept its underscore. The decision functions the script reads, `resolve_base` and `base_problems`, are public; promote `_refuse_base` to match them in the story that has another reason to touch it. `load_state(run_dir)` moved above the run-directory creation for this, since the clean-tree check decides from the state the run is starting from and `load_state` reads a file rather than requiring a directory; nothing else about the resume decision moved with it. The retry branch archives before it increments: `archive_attempt(run_dir, archivable_artifacts(stages), state.retry_count + 1)` copies the superseded attempt's artifacts under `attempts/attempt-N/` — see the archive decisions below. `append_event(run_dir, message, *, kind, stage, artifacts, duration_seconds, verifier_outcome, retry_decision, retry_reason, retry_category, retry_stage)` is the run's single event write path: the prose message stays positional and is what the `events.log` line is built from, and the *same call* appends one structured entry to `execution-history.json`. `load_history(run_dir)` is the read side, called only by `append_event` for the next sequence number. `run_story` captures `stage_started_at = time.monotonic()` at the stage-started event and reads it through a local `elapsed()` at every event that ends a stage, so a completed stage's entry carries a duration the log only made derivable; `_escalate` forwards whatever structured fields an escalation has and tags its entry `escalated`. The clean-clone check is the last thing the verifier branch does on a passing verdict: `clean_clone_check(run_dir, target_root, config, artifact)` builds a scratch clone with `tempfile.mkdtemp`, runs `run_clean_clone`, removes the scratch directory in a `finally` whatever the result, and writes the returned `CleanCloneResult.as_record()` to the run directory under the declared artifact name. `_build_clone` does `git clone --no-local` from the target's filesystem path — over git's normal transport, see "The clone is built over the normal transport" below — applies the target's tracked edits as `git diff --binary HEAD` piped to `git apply`, copies the untracked-but-not-ignored files from `git ls-files --others --exclude-standard`, then commits inside the clone; the target repository is only read. `_link_interpreter_roots` links the top-level directory of each configured interpreter path into the clone and appends those names to the clone's `.git/info/exclude`, because a virtualenv is gitignored and therefore absent from a fresh clone, and a `.gitignore` entry for a directory does not cover a symlink standing in its place. Zero exit appends `_clean_clone_passed` and falls through to the existing advance; non-zero takes the retry path the verification-failed branch already takes — `archive_attempt` above the increment, then increment, save, `_clean_clone_failed`, and `index = stage_names.index(destination)` — or the existing escalation path at the ceiling, with `_clean_clone_failures` collapsing the output's `FAILED` lines into the one-line reason. Since story-028 the destination on that path is `clean_clone["retry_stage"]`, read off the widened declaration, rather than borrowed from the verifier's own table. Both events are module-level helpers rather than inline calls, for a reason worth keeping: `tests/test_execution_history.py` proves its own non-vacuity by deleting the first `retry_decision="retry",` line at the verification-failed branch's indentation, and an inline clean-clone branch nests deeper and sits earlier in the file, so its line *contains* that indented text and the mutation lands there instead of where it was aimed. - `story_parser.py` — lexer plus schema-directed interpreter for the story artifact. **The story dialect is not YAML**; see the module docstring before reaching for `yaml.safe_load`, which reads committed artifacts differently and wrongly. The lexer produces line/indent/content records, drops blank lines and full-line comments, consumes a `key: |` block scalar body whole (so blank and `#`-shaped lines *inside* it survive), and rejects tab indentation. The interpreter dispatches on the schema node's `type`, consulting structure only where the schema is silent. Under `items.type == "string"` a `- ` item is the verbatim remainder of its line, colons included; under `items.type == "object"` the same syntax parses into key/value pairs. Scalars are never coerced — every value is a `str`. A single `StoryParseError` carries line, expectation, and finding, rendering as `line 12: expected …, found …`. - `schema_validator.py` — `schemas_dir`, `load_schema`, `shipped_schemas`, `unsupported_keywords`, and `validate(instance, schema) -> list[str]`. `shipped_schemas(harness_root=None) -> tuple[str, ...]` reads `schemas/manifest.json` through `schemas_dir`, so the override behaves identically to `load_schema`'s, and raises `ValueError` on anything short of a well-formed non-empty list of strings. A deliberately small JSON Schema subset — `type`, `required`, `properties`, `items`, `enum` — because the harness is standard library only. `validate` walks the whole schema first and raises `ValueError` if any keyword outside that subset appears anywhere in it, so a schema can never claim a constraint the validator silently drops. Errors carry a tracked JSON path, the expectation, and the found value: `$.blocking_issues[0].severity: expected one of ["high", "medium", "low"], found string ("critical")`. -- `context_assembler.py` — builds each stage's runtime context from the story artifact, prior stage artifacts, retry state, and architecture documents, and renders it into the prompt template. `build_context` takes the raw `story_text`, the required keyword-only `story` (the parsed artifact from `read_story`), and — since story-028 — the required keyword-only `workflow`, whose `workflow_context` it merges into the assembled context; it never reads either artifact itself. The workflow was the one thing every stage's context was built without, which is why the verifier could not be told what the coordinator routes on. Making it required rather than optional was deliberate: a default would let a call site silently render a verifier prompt with no categories in it. `{{story}}` is `story_text` verbatim, and `{{acceptance_criteria}}` comes from the parsed list via `_dashed_lines`, which renders one `- `-prefixed criterion per line and returns `None` for an absent or empty list. `{{stage_exceptions}}` follows the same convention through `_exception_lines`: one dash-prefixed line per grant naming the stage, the granted path, and the reason, `None` when the story declares none. `render()` is single-pass: `re.sub` does not re-scan substituted text, so a placeholder injected by one substitution is not itself resolved. `build_context` therefore resolves the shared `prompts/harness-layer.md` partial as a **two-pass render** — it renders that partial (including the partial's own `{{blocked_paths}}` placeholder) against the assembled context first, then stores the already-resolved text as the `harness_layer` context value for injection into stage templates. When the partial is absent, `harness_layer` is left unset and renders as `None`. The schema placeholders come from `schema_context(harness_root) -> dict[str, str]`, a public function of the same module: it globs `harness_root/schemas/*.schema.json` and exposes each file's text under the stem with hyphens replaced by underscores plus `_schema` (`verification-result.schema.json` → `{{verification_result_schema}}`). `build_context` merges it with `update` at the point the inline loop used to run, before the two-pass render, so the values are available to any template. A new schema file becomes an injectable placeholder with no code change. The glob appears exactly once in the module because it has two callers: `build_context` for workflow stages, and `l5-plan` for the planner template, which no coordinator renders. `workflow_context(workflow, rules) -> dict[str, str | None]` sits beside `schema_context` for the same reason: it maps the loaded workflow's stage names to `{{workflow_stages}}`, each stage's `may_not_create` declarations to `{{stage_create_restrictions}}` (`" may not create files under "`, one line per pair), the rules' `blocked_paths` to `{{blocked_paths}}`, and the workflow's declared retry routes to `{{retry_routes}}` (`" -> : "`, one line per declared category), all through the shared `_dashed_lines` helper — `build_context`'s own `blocked_paths` rendering goes through the same helper, so the harness-layer partial and the planner render the list identically. `_dashed_lines` returns `None` for an empty list, and `render()` maps `None` to the literal `None`, so the empty-list edge changes no rendered prompt. `retry_routes(stages) -> list[RetryRoute]` sits above it as the single derivation of the workflow's `(declared_by, category, stage, when)` triples, in the same spirit as `stage_restrictions`. It lives in this module rather than in the coordinator because the coordinator imports this module and not the reverse, and its two readers — the coordinator's pre-flight check on the table and the rendering just above — therefore read one answer to "what does this workflow route". The rendering exists once: there is no second function turning workflow routes into prompt text. `config_context(config) -> dict[str, str | None]` joins the pair since story-035, mapping the target config's `allowed_tools` to `{{allowed_tools}}` through the same `_dashed_lines` helper, and `build_context` merges it beside `workflow_context` from a new **optional** keyword-only `allowed_tools` argument — optional where `workflow` is required, deliberately, because a call that omits it must render exactly what it rendered before the argument existed. The coordinator passes `config.get("allowed_tools")` at its one call site. See "Tool allowlist" above for why the grants are injected rather than restated in prose. Since story-045 the context dict also carries `documentation_report` (from `documentation-report.md`) and `documenter_changed_files` (from `documenter-changed-files.json`), read through the same `_read` helper as the artifacts beside them, so an absent artifact renders as `None` rather than failing — which is what a stage running before the documenter, and this repository's own story-045 run, get. `self_route_result` joins it since story-036 on identical terms — optional, keyword-only, defaulting to `None`, exposed under a `self_route_result` key and rendering as `None` when nothing applies — so a call that omits it renders exactly what it rendered before the argument existed. The coordinator reads the value back off the evidence artifact it has just written rather than passing it along in memory, so what the prompt says and what the run directory records are one thing. +- `context_assembler.py` — builds each stage's runtime context from the story artifact, prior stage artifacts, retry state, and architecture documents, and renders it into the prompt template. `build_context` takes the raw `story_text`, the required keyword-only `story` (the parsed artifact from `read_story`), and — since story-028 — the required keyword-only `workflow`, whose `workflow_context` it merges into the assembled context; it never reads either artifact itself. The workflow was the one thing every stage's context was built without, which is why the verifier could not be told what the coordinator routes on. Making it required rather than optional was deliberate: a default would let a call site silently render a verifier prompt with no categories in it. `{{story}}` is `story_text` verbatim, and `{{acceptance_criteria}}` comes from the parsed list via `_dashed_lines`, which renders one `- `-prefixed criterion per line and returns `None` for an absent or empty list. `{{stage_exceptions}}` follows the same convention through `_exception_lines`: one dash-prefixed line per grant naming the stage, the granted path, and the reason, `None` when the story declares none. `render()` is single-pass: `re.sub` does not re-scan substituted text, so a placeholder injected by one substitution is not itself resolved. `build_context` therefore resolves the shared `prompts/harness-layer.md` partial as a **two-pass render** — it renders that partial (including the partial's own `{{blocked_paths}}` placeholder) against the assembled context first, then stores the already-resolved text as the `harness_layer` context value for injection into stage templates. When the partial is absent, `harness_layer` is left unset and renders as `None`. The schema placeholders come from `schema_context(harness_root) -> dict[str, str]`, a public function of the same module: it globs `harness_root/schemas/*.schema.json` and exposes each file's text under the stem with hyphens replaced by underscores plus `_schema` (`verification-result.schema.json` → `{{verification_result_schema}}`). `build_context` merges it with `update` at the point the inline loop used to run, before the two-pass render, so the values are available to any template. A new schema file becomes an injectable placeholder with no code change. The glob appears exactly once in the module because it has two callers: `build_context` for workflow stages, and `l5-plan` for the planner template, which no coordinator renders. `workflow_context(workflow, rules) -> dict[str, str | None]` sits beside `schema_context` for the same reason: it maps the loaded workflow's stage names to `{{workflow_stages}}`, each stage's `may_not_create` declarations to `{{stage_create_restrictions}}` (`" may not create files under "`, one line per pair), the rules' `blocked_paths` to `{{blocked_paths}}`, and the workflow's declared retry routes to `{{retry_routes}}` (`" -> : "`, one line per declared category), all through the shared `_dashed_lines` helper — `build_context`'s own `blocked_paths` rendering goes through the same helper, so the harness-layer partial and the planner render the list identically. `_dashed_lines` returns `None` for an empty list, and `render()` maps `None` to the literal `None`, so the empty-list edge changes no rendered prompt. `retry_routes(stages) -> list[RetryRoute]` sits above it as the single derivation of the workflow's `(declared_by, category, stage, when)` triples, in the same spirit as `stage_restrictions`. It lives in this module rather than in the coordinator because the coordinator imports this module and not the reverse, and its two readers — the coordinator's pre-flight check on the table and the rendering just above — therefore read one answer to "what does this workflow route". The rendering exists once: there is no second function turning workflow routes into prompt text. `config_context(config) -> dict[str, str | None]` joins the pair since story-035, mapping the target config's `allowed_tools` to `{{allowed_tools}}` through the same `_dashed_lines` helper, and `build_context` merges it beside `workflow_context` from a new **optional** keyword-only `allowed_tools` argument — optional where `workflow` is required, deliberately, because a call that omits it must render exactly what it rendered before the argument existed. The coordinator passes `config.get("allowed_tools")` at its one call site. See "Tool allowlist" above for why the grants are injected rather than restated in prose. Since story-046 `config_context` maps a second fact, `tests_dir` → `{{tests_dir}}`, and `build_context` calls it as `config_context({**config, "allowed_tools": allowed_tools})`: the grants keep arriving as their own optional argument so an omitting call renders what it always rendered, while every other configured fact comes off the config the caller has already passed. The reasoning is the same one that put the grants here — the stage that writes tests is told where they go by the very configuration the create restriction is resolved from, so the prompt cannot drift from the rule. Since story-045 the context dict also carries `documentation_report` (from `documentation-report.md`) and `documenter_changed_files` (from `documenter-changed-files.json`), read through the same `_read` helper as the artifacts beside them, so an absent artifact renders as `None` rather than failing — which is what a stage running before the documenter, and this repository's own story-045 run, get. `self_route_result` joins it since story-036 on identical terms — optional, keyword-only, defaulting to `None`, exposed under a `self_route_result` key and rendering as `None` when nothing applies — so a call that omits it renders exactly what it rendered before the argument existed. The coordinator reads the value back off the evidence artifact it has just written rather than passing it along in memory, so what the prompt says and what the run directory records are one thing. - `agent_runner.py` — invokes `claude -p` headlessly (`--permission-mode acceptEdits --output-format stream-json --verbose`, prompt on stdin), streams raw output to the run's log, and returns the agent's final result text. Since story-035 it also passes `--settings` on every stage invocation, registering the shipped deny-only Bash guard: `hooks_dir(harness_root=None)` resolves `hooks/` relative to this module the way `schema_validator` resolves schemas, and `guard_settings` reads `hooks/settings.json` and substitutes the guard's absolute path for the declaration's `{guard_path}`. An absent or unreadable declaration, or a missing guard file, returns `None` and the stage runs without the hook — the guard is the net and the allowlist is the gate, so failing to register it must not stop a run. The settings are resolved *here* rather than passed in, so `run_agent`'s signature is unchanged and the fake runners the suite injects need not know the hook exists. See "Tool allowlist" above for what the guard decides. -- `harness_config.py` — loads `.harness/config.yaml` (a deliberately small YAML subset parsed directly, keeping the harness dependency-free), workflow definitions, and execution rules. Also owns `find_target_root(start) -> Path`: the walk-up from a starting directory to the nearest ancestor containing `.harness/config.yaml`, exiting 1 with `No .harness/config.yaml found here or above. Run l5-init first.` when none exists. That loop appears exactly once in the repository — `l5-run`, `l5-plan`, and `l5-status` all call it (story-009 extracted it from `l5-run`, which `l5-status` had copied byte-for-byte). `l5-init`'s config check is a different thing: a non-walking existence probe on a directory it was explicitly given. +- `harness_config.py` — loads `.harness/config.yaml` (a deliberately small YAML subset parsed directly, keeping the harness dependency-free), workflow definitions, and execution rules. Since story-046 `load_workflow` takes the loaded config as a required third argument and resolves the definition's `{{key}}` configuration references against it — `workflow_token_values`, `_resolve_tokens` and `UnresolvedWorkflowToken` are that mechanism, described under "A workflow declaration may reference configuration" below. Also owns `find_target_root(start) -> Path`: the walk-up from a starting directory to the nearest ancestor containing `.harness/config.yaml`, exiting 1 with `No .harness/config.yaml found here or above. Run l5-init first.` when none exists. That loop appears exactly once in the repository — `l5-run`, `l5-plan`, and `l5-status` all call it (story-009 extracted it from `l5-run`, which `l5-status` had copied byte-for-byte). `l5-init`'s config check is a different thing: a non-walking existence probe on a directory it was explicitly given. - `plan_commit.py` — the decision behind `l5-plan`'s post-session commit, kept out of the script so it is testable without spawning `claude`. Every function returns what happened rather than printing it. `snapshot(stories_dir)` is every file under the configured stories directory right now (a directory that does not exist yet snapshots as empty, so a repository's first story appears like any other); `new_artifacts(stories_dir, before)` is what appeared since. **Appearance is the whole test** — nothing here reads the session's exit status, and a session that only edited an existing artifact yields nothing. `commit_artifacts` runs `git add -- ` then `git commit -m -- `: the pathspec on the *commit* as well as the add is what keeps unrelated dirty work — and whatever the developer had already staged — out of it. There is no `git add -A` anywhere in this module. `commit_subject` is `Plan story-NNN: ` for one readable artifact, `Plan story-NNN` on any parse failure, and `Plan story-NNN, story-MMM` when one session added more than one, committed together because splitting them would invent an order the session did not have. The title is read through `story_coordinator.read_story` — the run's one reading of a story artifact, per that rule below — not through a second `story_parser.parse` call, so the commit message describes the artifact the way the run that executes it will. `current_branch`, `resolve_remote` and `push_commit` resolve the remote from `branch.<name>.remote`, fall back to `origin` when the branch tracks nothing, and report rather than attempt when neither exists; the push is `git push <remote> HEAD`, so a branch with no upstream is pushed under its own name without writing tracking configuration into the developer's repository as a side effect of planning. Nothing in the module rolls back, amends or resets: a failed push leaves the commit exactly where it is. It chooses, creates and switches no branch — the commit lands on whatever branch the developer was on, as the planner's own commit did. - `plan_validation.py` — the decision behind `l5-plan`'s plan-time validation, added by story-025 and kept out of the script for the same reason `plan_commit` is. It **returns** the problems it found; it never prints them, never repairs an artifact and never deletes one. `artifact_problems(artifacts, stages, root) -> dict[Path, list[str]]` is the composition function (`root` is the target root, required and defaulted nowhere, forwarded to the one check below that reads the filesystem): one `story_coordinator.read_story` call per artifact, its parse handed to `story_coordinator.stage_exception_problems` and to `strictness_problems`, keyed by path and holding only the artifacts with problems, so an empty mapping is the whole of "these may be committed". An artifact `read_story` has something to say about yields that and nothing further — `run_story`'s own pre-flight order rather than a second one, because a story that failed to parse has no parse for the later checks and one that failed its *schema* has one whose shape they may not assume (`stage_exception_problems` indexes `exception["stage"]`; `strictness_problems` splits each entry as a string). @@ -443,19 +443,47 @@ The added conjunct is one question: `(root / entry_path).exists()` is false. `ex **This story's own plan was not governed by the check it adds**, for the eighth time in this pattern's history, and the cause is a new one worth naming: the artifact was written and committed by an `l5-plan` session that ran before `assignment_problems` existed. It happens to be clean — it assigns its test module to the stage that owns validation, which is the convention this story writes down — but it was not checked. Enforcement begins with the next plan, which is one step earlier than the stale-workflow and stale-import cases, since the artifact rather than the run is what escapes. +## A workflow declaration may reference configuration + +Since story-046 a workflow definition may name a value the target's own `.harness/config.yaml` supplies, written as `{{key}}` and **only as a whole list entry**. Exactly one declaration does: the implementer's `may_not_create` is `["{{tests_dir}}"]`. + +**The resolution happens once, when the definition loads.** `harness_config.load_workflow(harness_root, name, config)` walks the parsed definition through `_resolve_tokens` and returns a dict in which the token is already the configured string, so `stage_restrictions` and its six readers — the ownership check, the revert check's governed edits, the stage-baseline capture, `stage_exception_problems`, and both of `plan_validation`'s checks — read exactly what they read when the declaration was a literal, and not one of them gained a config parameter or a resolution step. For this repository the loaded definition is identical in value to the one that shipped before, which is why "this repository's behaviour is unchanged" is a checkable claim rather than an intention. + +**`config` is a required positional, not a defaulted one.** A caller that omitted it would load a definition with the configured entries silently missing — that is, with the create restriction gone — which is a quieter wrong answer than a `TypeError`, and the restriction disappearing silently is precisely the failure this mechanism must not introduce. Both callers pass the config they had already loaded: `story_coordinator.run_story` and `scripts/l5-plan`. + +**An unset key resolves the entry *out of the list*, and never to an empty string.** A restriction whose prefix is `""` is a prefix every path is under, the exact opposite of what the absence means. A target that declares no `tests_dir` therefore has an implementer with no create restriction at all — `stage_restrictions` reports no pair for it — which is what a Go project, or any target with no single test directory, is. The rule is stated in the key's own schema description as well as here. + +**What is referable is a one-entry mapping, and that is the narrowness.** `harness_config.workflow_token_values(config)` returns `{"tests_dir": config.get("tests_dir")}`, written as a literal read per key rather than a lookup by whatever name the definition carries — so the keys this resolution reads are visible to a reader and to `test_config_keys_are_obeyed`'s AST scan exactly like every other configured key. A general mechanism letting any declaration reach any config key was considered and rejected: one key needs this, and the narrow token leaves every existing reader of a loaded workflow untouched. `{{branch_prefix}}` in a workflow is refused like any other unanswerable reference. + +**An unanswerable reference refuses the run at pre-flight.** `load_workflow` raises `UnresolvedWorkflowToken`, carrying `problems` in the shape every pre-flight refusal enumerates, and `run_story` catches it and prints through `_refuse_unresolved_workflow_token` and the shared `refuse()`: the header names the workflow, each problem names the token and lists what a declaration may reference. It sits where the workflow load already sat — after the undeclared-config-key refusal and above the run directory, `state.json`, the log, the branch and any agent invocation — so a refused run creates nothing. A token-shaped string anywhere a list entry cannot be dropped from is collected as unresolved too, rather than silently substituted, because there is nowhere else an unset value could go. + +**The prompt renders the same value rather than restating it.** `context_assembler.config_context` maps `tests_dir` beside `allowed_tools`, and `build_context` passes `{**config, "allowed_tools": allowed_tools}` — the grants keep arriving as their own optional argument so a call that omits them renders what it always rendered, while every other configured fact comes off the config the caller already passed. `prompts/tester.md` renders `{{tests_dir}}`, so changing the config changes the rendered prompt with no prompt edit. Its baseline-resolution sentence was rewritten in the same move to say *use the shared baseline resolution the existing validation already provides* rather than naming the file this repository happens to keep it in — a prompt states what the tester should do, not this repository's filename. + +**Both shipped configs declare it explicitly.** `.harness/config.yaml` and `templates/config.yaml` each set `tests_dir: tests/` with a comment saying what its absence means, so a newly initialized target states its test location rather than inheriting one. + ## No target-stack literal in harness source The harness runs against any repository. Its only tie to a target's language, toolchain or directory layout belongs in `.harness/`, which is that target's own configuration. An audit on 2026-08-15 found five ties outside it: a Python snippet executed as a version probe and a configuration key whose name is a language, both in `orchestration/story_coordinator.py`; the `may_not_create` line of `workflows/story-workflow.json`; and two lines of prose in `prompts/tester.md` naming a pytest layout. **The rule is a scan, not a paragraph, and the choice has a record.** Two rules of exactly this shape were written down here before: the `git diff HEAD` baseline rule was documented *and* injected into every stage prompt and shipped five more times, and the git-history-loader rule was documented and shipped three more times. Both stopped when a scan landed. Two of the five ties also sit in a prompt, which no fixture reaches — `prompts/tester.md` is prose an agent reads — so "every configurable value is proven configurable" could vary every key in existence and still not notice a pytest filename sitting in it. Only a reader or a scan sees that, and readers had missed it since the file was written. -**Where the two halves live.** The declaration and the scan are `orchestration/harness_source.py` (above); the judgement is `tests/test_no_target_stack_in_harness_source.py`. The module reports mentions and judges none of them — the test module holds **two allowlists**, keyed by repository-relative path and the exact text of the matched line rather than by line number, so an unrelated edit above a tie does not churn the list and read as burn-down. `TEMPORARY_TIES` (3 entries since story-041, 36 before it) holds the mentions that name or assume a target's stack or layout. `PERMANENT_MENTIONS` (8 entries since story-043, 9 before it) holds the mentions that are the opposite of a tie — a docstring saying every scalar `story_parser` produces is a Python `str` is a fact about this code, and the shebang each `scripts/l5-*` entry point carries names the interpreter *this* harness runs under — and **every entry carries a one-line reason**, so the permanent half is a judgement on the record rather than a suppression list. The ninth entry was `orchestration/harness_config.py`, whose `RETIRED_CONFIG_KEYS` spelled a retired key; story-043 deleted the mapping, and the allowlist entry had to go with it or the two-way set equality would fail. Only two entries name a file under `orchestration/` now, both docstrings about this code: `story_parser.py`'s type-coercion note and `story_coordinator.py`'s `self_route_problems`. The two lists together equal exactly what the scan reports against this repository (11 distinct `(path, line text)` pairs from 12 findings — `prompts/tester.md:47` matches both rules), asserted in both directions and asserted disjoint. The classification rule sits in the module docstring beside the declaration so the next one is not re-argued. +**Where the two halves live.** The declaration and the scan are `orchestration/harness_source.py` (above); the judgement is `tests/test_no_target_stack_in_harness_source.py`. The module reports mentions and judges none of them — the test module holds **two allowlists**, keyed by repository-relative path and the exact text of the matched line rather than by line number, so an unrelated edit above a tie does not churn the list and read as burn-down. `TEMPORARY_TIES` (**empty since story-046**; 3 entries after story-041, 36 before it) holds the mentions that name or assume a target's stack or layout. `PERMANENT_MENTIONS` (8 entries since story-043, 9 before it) holds the mentions that are the opposite of a tie — a docstring saying every scalar `story_parser` produces is a Python `str` is a fact about this code, and the shebang each `scripts/l5-*` entry point carries names the interpreter *this* harness runs under — and **every entry carries a one-line reason**, so the permanent half is a judgement on the record rather than a suppression list. The ninth entry was `orchestration/harness_config.py`, whose `RETIRED_CONFIG_KEYS` spelled a retired key; story-043 deleted the mapping, and the allowlist entry had to go with it or the two-way set equality would fail. Only two entries name a file under `orchestration/` now, both docstrings about this code: `story_parser.py`'s type-coercion note and `story_coordinator.py`'s `self_route_problems`. The two lists together equal exactly what the scan reports against this repository — since story-046 that is `PERMANENT_MENTIONS` alone, 8 pairs — asserted in both directions, and every reported entry asserted to land on **exactly one** of them. Disjointness used to be its own assertion, intersecting the two lists directly; with `TEMPORARY_TIES` empty that intersection has one possible answer, so story-046 replaced it with the partition — a question this repository can still fail, since an entry on neither list is a new tie. (Before story-046 it was 11 distinct `(path, line text)` pairs from 12 findings, `prompts/tester.md:47` matching both rules.) The classification rule sits in the module docstring beside the declaration so the next one is not re-argued. -**Why they are two lists and not one.** `TEMPORARY_TIES` reaching empty is the completion signal for `.harness/requests/the-interpreter-is-not-assumed-to-be-python.md` and `.harness/requests/the-test-location-comes-from-configuration.md` — the two stories queued behind story-040, which fix the ties it grandfathers. Merge the two lists and the signal is gone — a list that stops shrinking cannot be told from work that finished. +**Why they are two lists and not one.** `TEMPORARY_TIES` reaching empty was the completion signal for `.harness/requests/the-interpreter-is-not-assumed-to-be-python.md` and `.harness/requests/the-test-location-comes-from-configuration.md` — the two stories queued behind story-040, which fix the ties it grandfathers. Merge the two lists and the signal is gone — a list that stops shrinking cannot be told from work that finished. The signal has now fired; the split stays, because the next tie a scan finds goes in one list or the other and the same question has to be answerable then. **story-040 fixes none of the five**, deliberately. `orchestration/story_coordinator.py`, `workflows/story-workflow.json` and `prompts/tester.md` carry no edit on its branch, and no run-time behaviour changed. -**The burn-down has since run once, and it removed exactly its own entries.** story-041 took off every `orchestration/story_coordinator.py` entry — the version probe, the record's interpreter-shaped fields, and the retired configuration key with the prose explaining it — together with every entry in `schemas/clean-clone-result.schema.json`, `schemas/revert-check-result.schema.json` and `schemas/harness-config.schema.json`. **What remains temporary is the two `prompts/tester.md` lines and the `workflows/story-workflow.json` restriction, and nothing else**, which is the completion signal for `the-test-location-comes-from-configuration` alone. The coordinator came off that module's untouched tuple and both of its entries came off `AUDITED_TIES` — asking the scan for a tie by name only works while the tie is there. In their place `REPAIRED_SCHEMAS` and `test_the_repaired_schemas_carry_no_mention_at_all` assert the scan reports *nothing* in the three schemas, and `test_the_coordinator_carries_no_temporary_tie` asserts the same for the coordinator in the weaker form the surviving permanent mention allows. Both read the claim off the scan rather than off the diff, so an entry removed while its mention is still in source goes red. One mention does survive in the coordinator, and deliberately: `self_route_problems`'s docstring naming the language *this harness* is written in, which the classification rule calls the opposite of a tie. +**The burn-down has since run once, and it removed exactly its own entries.** story-041 took off every `orchestration/story_coordinator.py` entry — the version probe, the record's interpreter-shaped fields, and the retired configuration key with the prose explaining it — together with every entry in `schemas/clean-clone-result.schema.json`, `schemas/revert-check-result.schema.json` and `schemas/harness-config.schema.json`. **What remained temporary after it was the two `prompts/tester.md` lines and the `workflows/story-workflow.json` restriction, and nothing else** — the completion signal for `the-test-location-comes-from-configuration` alone, which story-046 then fired. The coordinator came off that module's untouched tuple and both of its entries came off `AUDITED_TIES` — asking the scan for a tie by name only works while the tie is there. In their place `REPAIRED_SCHEMAS` and `test_the_repaired_schemas_carry_no_mention_at_all` assert the scan reports *nothing* in the three schemas, and `test_the_coordinator_carries_no_temporary_tie` asserts the same for the coordinator in the weaker form the surviving permanent mention allows. Both read the claim off the scan rather than off the diff, so an entry removed while its mention is still in source goes red. One mention does survive in the coordinator, and deliberately: `self_route_problems`'s docstring naming the language *this harness* is written in, which the classification rule calls the opposite of a tie. + +**The burn-down is finished.** story-046 removed the last three temporary entries — the workflow's create restriction, now the `{{tests_dir}}` token resolved from configuration when the definition loads, and the two lines of `prompts/tester.md` prose, which now render the configured location and describe the shared baseline resolution without naming the file this repository keeps it in. `harness_source.scan()` over this repository reports exactly the eight `PERMANENT_MENTIONS` and nothing else, which is story-040's completion signal for the whole burn-down. All five ties the 2026-08-15 audit found are repaired. + +**An empty list is the shape an assertion goes vacuous against, and no assertion in that module rests on reading it.** Emptying `TEMPORARY_TIES` took six assertions vacuous, in two waves, and every one was replaced rather than deleted or skipped. + +The first wave is the ones that *iterated* the list. `AUDITED_TIES` and the story-diff pair are gone; in their place `REPAIRED_FILES` and `test_the_repaired_files_carry_no_mention_at_all` assert the scan reports **nothing at all** in `workflows/story-workflow.json` and `prompts/tester.md`, and `test_the_burn_down_is_complete` asserts the scan reports nothing outside `PERMANENT_MENTIONS` — both read off the scan, not off the list. The control is `HISTORICAL_TIES`: the three exact lines that used to sit in those files, kept verbatim and replanted one at a time into a throwaway copy, where the same scan must report each and the same two-directional comparison must call it unexpected. Without that, "the scan reports nothing there" would be satisfied just as happily by a scan that had stopped reading the file. The stale-entry control moved onto a permanent entry, since no temporary one is left to remove. + +The second wave is subtler and was caught only in verification: three assertions still *named* the empty list to decide something — `reported & set(TEMPORARY_TIES) == set()`, `entry not in TEMPORARY_TIES`, and a whole test asserting the two lists disjoint. None iterates it, so none reads as a loop over nothing; each is simply a comparison against the empty set, which cannot go red. Two sat beside a live `PERMANENT_MENTIONS` assertion, so their tests survived and only the lines were dead — the shape that is hardest to see, because the test still fails when the behaviour breaks and the reader credits the wrong assertion for it. The replacement is `classification_problems(findings, temporary=None, permanent=None)`: exclusivity and coverage asked as one question, whether the two lists classify each reported entry **exactly once**. An entry on neither is a new tie, which this repository can still produce, so the question stays answerable with one list empty; and because the lists are parameters, a control hands the same check an entry sitting on both and observes it object. `test_the_coordinator_carries_no_temporary_tie` and `test_a_legitimate_mention_is_permanent_and_never_a_tie` now call it in place of their dead lines, each still carrying its own replant control. + +**The lesson worth keeping is the detection rule**, since the next story to empty a burn-down list meets the same thing: a list going empty makes every assertion that *mentions* it suspect, not just every loop over it, and the check is whether the assertion can still fail for some state of the repository rather than whether it still reads like an assertion. The repair is to re-ask the question against something that can still be non-empty — here, what the scan reports — and if a question is genuinely answered elsewhere, to say so in the module where the assertion used to be, so a later reader does not read the removal as a weakening. **What the scan does not catch, stated where a reader meets it.** `STACK_TOKENS` is a guess about languages nobody has tried and is incomplete by construction — a Ruby `Gemfile` or an Elixir `mix.exs` is reported by nothing here, which the suite demonstrates by planting both and finding them invisible. It is worth having anyway because it catches the *shape* of the mistake, which recurs, rather than every instance of it. The layout half **cannot read `orchestration/` at all**: this repository's own suite is called `tests/`, so in Python source a `tests/` literal cannot be told from an honest reference to it, and the half therefore reads only the target-facing files where a path can only mean a target's. `.harness/` and `tests/` are outside the scan entirely. The module is exempt from its own scan **by name and by nothing else** — the token list would otherwise report itself — and a second file carrying the same tokens is reported. And the scan is not tamper-proof: deleting it alongside a forced repair is not caught here at any granularity, and no assertion in the module implies otherwise. Every one of those limits is asserted by reading the module's text rather than by trusting it to be there. diff --git a/orchestration/context_assembler.py b/orchestration/context_assembler.py index 8d24ee8..2a8ca8c 100644 --- a/orchestration/context_assembler.py +++ b/orchestration/context_assembler.py @@ -113,14 +113,20 @@ def workflow_context(workflow: dict, rules: dict) -> dict[str, str | None]: def config_context(config: dict) -> dict[str, str | None]: - """Map the target config's grants to their injectable placeholder name. + """Map the target config's own facts to their injectable placeholder names. The granted Bash commands are rendered from the target's own configuration rather than restated in prose, so what a stage is told it may run cannot - drift from what is actually permitted. A config declaring no allowed_tools - renders as None, the optional-placeholder convention. + drift from what is actually permitted. The same reasoning puts the test + location here: the stage that writes tests is told where they go by the + configuration the restriction is resolved from, so changing the config + changes the rendered prompt with no prompt edit. A config declaring + neither renders as None, the optional-placeholder convention. """ - return {"allowed_tools": _dashed_lines(config.get("allowed_tools"))} + return { + "allowed_tools": _dashed_lines(config.get("allowed_tools")), + "tests_dir": config.get("tests_dir"), + } def _read(path: Path) -> str | None: @@ -258,7 +264,11 @@ def build_context( # a stage rendered with no categories in it would be a defect, while a # stage rendered with no granted list is exactly what every call site # rendered before this existed, so omitting it must change nothing. - context.update(config_context({"allowed_tools": allowed_tools})) + # `allowed_tools` still arrives as its own argument rather than off the + # config, so a call that omits it renders exactly what it rendered before + # the argument existed; every other configured fact this renders comes off + # the config the caller already passed. + context.update(config_context({**config, "allowed_tools": allowed_tools})) # Two-pass render: resolve the shared harness-layer partial (including its # own {{blocked_paths}} placeholder) against the assembled context before diff --git a/orchestration/harness_config.py b/orchestration/harness_config.py index 936836c..ceae8de 100644 --- a/orchestration/harness_config.py +++ b/orchestration/harness_config.py @@ -7,6 +7,7 @@ from __future__ import annotations import json +import re import sys from pathlib import Path @@ -117,9 +118,94 @@ def declared_config_keys(harness_root: Path | None = None) -> tuple[str, ...]: return tuple(properties) -def load_workflow(harness_root: Path, name: str) -> dict: +#: A workflow declaration references configuration as `{{key}}`, and only as +#: a whole list entry. A general mechanism -- any declaration reaching any +#: config key -- was considered and rejected: one key needs this, and a +#: narrow token leaves every existing reader of a loaded workflow untouched. +_WORKFLOW_TOKEN = re.compile(r"\{\{([A-Za-z_][A-Za-z0-9_]*)\}\}") + + +def workflow_token_values(config: dict) -> dict[str, str | None]: + """The configuration a workflow declaration may reference, by token name. + + Written as a literal read per key rather than a lookup by whatever name + the definition happens to carry, so the keys this resolution reads are + visible to a reader -- and to the scan that holds the declared set equal + to the set the harness reads -- exactly like every other configured key. + That the mapping has one entry is the narrowness, stated in code. + """ + return {"tests_dir": config.get("tests_dir")} + + +class UnresolvedWorkflowToken(ValueError): + """A loaded workflow carries a reference the configuration cannot answer. + + Carries `problems` in the shape every pre-flight refusal enumerates, so + the coordinator turns it into a refusal rather than composing its own + wording for it. + """ + + def __init__(self, workflow: str, tokens: list[str]): + self.workflow = workflow + self.tokens = tokens + referable = ", ".join(f"{{{{{name}}}}}" for name in workflow_token_values({})) + self.problems = [ + f"'{{{{{token}}}}}' is not a configuration reference the harness " + f"resolves; a workflow declaration may reference {referable}, and " + f"only as a whole list entry" + for token in tokens + ] + super().__init__("; ".join(self.problems)) + + +def _resolve_tokens(value, values: dict[str, str | None], unresolved: list[str]): + """Substitute every `{{key}}` list entry, dropping the ones with no value. + + An unset key resolves the entry *out of the list* rather than to an empty + string: a restriction whose prefix is "" is a prefix every path is under, + which is the opposite of the "this target declares none" the absence + means. Every other token-shaped string -- one naming a key outside the + narrow set, or a resolvable one somewhere a list entry cannot be dropped + from -- is collected as unresolved for the caller to refuse on. + """ + if isinstance(value, dict): + return {key: _resolve_tokens(item, values, unresolved) + for key, item in value.items()} + if isinstance(value, list): + resolved = [] + for item in value: + match = _WORKFLOW_TOKEN.fullmatch(item) if isinstance(item, str) else None + if match is None: + resolved.append(_resolve_tokens(item, values, unresolved)) + continue + name = match.group(1) + if name not in values: + unresolved.append(name) + elif values[name]: + resolved.append(values[name]) + return resolved + if isinstance(value, str): + unresolved.extend(_WORKFLOW_TOKEN.findall(value)) + return value + + +def load_workflow(harness_root: Path, name: str, config: dict) -> dict: + """The workflow definition, with its configuration references resolved. + + Resolution happens once, when the definition loads, so `stage_restrictions` + and every reader of a loaded workflow reads exactly what it read when the + declaration was a literal directory. `config` is required rather than + defaulted: a caller that omitted it would silently load a definition with + the configured entries missing, which is a quieter wrong answer than a + TypeError. + """ path = harness_root / "workflows" / f"{name}.json" - return json.loads(path.read_text(encoding="utf-8")) + definition = json.loads(path.read_text(encoding="utf-8")) + unresolved: list[str] = [] + resolved = _resolve_tokens(definition, workflow_token_values(config), unresolved) + if unresolved: + raise UnresolvedWorkflowToken(name, unresolved) + return resolved def load_rules(harness_root: Path) -> dict: diff --git a/orchestration/story_coordinator.py b/orchestration/story_coordinator.py index b9de586..12b353b 100644 --- a/orchestration/story_coordinator.py +++ b/orchestration/story_coordinator.py @@ -2330,6 +2330,24 @@ def _refuse_bad_self_routes(workflow: dict, problems: list[str]) -> int: ) +def _refuse_unresolved_workflow_token( + unresolved: harness_config.UnresolvedWorkflowToken, +) -> int: + """Refuse a workflow that references configuration the harness cannot answer. + + Thin, like every other caller of `refuse`. The header names the workflow + and each problem names the token, so the pair a developer needs is in the + message rather than one of them being left to inference. + """ + return refuse( + f"Workflow '{unresolved.workflow}' references configuration the harness " + f"cannot resolve:", + unresolved.problems, + "Fix the workflow definition's configuration references before running " + "a story under it.", + ) + + def _refuse_undeclared_config_keys(target_root: Path, problems: list[str]) -> int: """Refuse a run whose configuration carries a key the harness does not read. @@ -2553,7 +2571,17 @@ def run_story( if undeclared: return _refuse_undeclared_config_keys(target_root, undeclared) - workflow = harness_config.load_workflow(harness_root, config.get("workflow", "story-workflow")) + # The definition may reference the target's configuration, so it is loaded + # against the config that has just been read. A reference the config + # cannot answer is a defect in the definition that every run under it + # carries, so it is refused here, beside the other pre-flight refusals and + # above everything a run creates: no run directory, no state.json, no log, + # no branch, and no agent invoked. + workflow_name = config.get("workflow", "story-workflow") + try: + workflow = harness_config.load_workflow(harness_root, workflow_name, config) + except harness_config.UnresolvedWorkflowToken as unresolved_token: + return _refuse_unresolved_workflow_token(unresolved_token) rules = harness_config.load_rules(harness_root) stages = workflow["stages"] stage_names = [s["name"] for s in stages] diff --git a/prompts/tester.md b/prompts/tester.md index 84a5fc8..7608475 100644 --- a/prompts/tester.md +++ b/prompts/tester.md @@ -16,7 +16,7 @@ Do not: - weaken, skip, or delete existing tests, or - decide whether the workflow may continue (the verifier owns that decision). -New tests belong in tests/ and become permanent repository assets. +New tests belong in {{tests_dir}} and become permanent repository assets. Name a validation module for the behaviour it validates, so that a reader looking for that behaviour finds the module by its name rather than by @@ -44,7 +44,8 @@ Baselines resolved out of git are the recurring instance of this. Do not resolve one as `HEAD` or as the working tree against the repository root: the coordinator commits the working tree at the end of a successful run, so those comparisons go vacuously green the moment the story commits. Use the -shared resolution in `tests/conftest.py`. +shared baseline resolution the existing validation already provides rather +than writing a second one beside it. When you finish, write these files to the run directory at {{run_dir}}: diff --git a/schemas/harness-config.schema.json b/schemas/harness-config.schema.json index 8a131d2..cfc8f70 100644 --- a/schemas/harness-config.schema.json +++ b/schemas/harness-config.schema.json @@ -47,6 +47,10 @@ "type": "string", "description": "Directory, relative to the target root, holding approved story artifacts. Defaults to .harness/stories. Also the directory l5-plan snapshots to decide what a planning session produced." }, + "tests_dir": { + "type": "string", + "description": "Where a target's tests live, as a repository-relative path prefix ending in a slash. Set, it is the prefix the workflow's {{tests_dir}} token resolves to, so the stage restricted from creating tests is restricted there and the stage that writes them is told to write them there. There is no default: unset, the target declares no test directory at all, the token resolves out of the list it appears in entirely rather than becoming an empty prefix that would match every path, and the restriction does not exist for that target." + }, "test_command": { "type": "string", "description": "The command the clean-clone and revert checks run inside a scratch clone. Read without a fallback: a target that omits it cannot run either check." diff --git a/scripts/l5-plan b/scripts/l5-plan index 1c43cfc..b3fe008 100755 --- a/scripts/l5-plan +++ b/scripts/l5-plan @@ -41,7 +41,7 @@ def main() -> None: target_root = harness_config.find_target_root(Path.cwd()) config = harness_config.load_config(target_root) workflow = harness_config.load_workflow( - HARNESS_ROOT, config.get("workflow", "story-workflow") + HARNESS_ROOT, config.get("workflow", "story-workflow"), config ) rules = harness_config.load_rules(HARNESS_ROOT) # The planner is not a workflow stage, so no coordinator renders its diff --git a/templates/config.yaml b/templates/config.yaml index e86d2eb..a883f06 100644 --- a/templates/config.yaml +++ b/templates/config.yaml @@ -9,6 +9,14 @@ standards_dir: .harness/standards architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: {test_command} +# Where this repository's tests live, as a path prefix ending in a slash. It is +# what the workflow's {{tests_dir}} token resolves to: the stage restricted from +# creating tests is restricted here, and the stage that writes them is told to +# write them here. A starter value like every other line in this file — change +# it to wherever this repository actually keeps its tests. There is no default: +# delete the key and this repository declares no test directory at all, and the +# restriction resolves out of the workflow entirely. +tests_dir: tests/ # Bash commands stage agents may run without prompting. Headless agents # cannot answer permission prompts. Read-only search and inspection is # granted broadly: a denial costs a turn and buys nothing, because the diff --git a/tests/conftest.py b/tests/conftest.py index 269c6fe..e45361b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,6 +12,73 @@ HARNESS_ROOT = Path(__file__).resolve().parents[1] sys.path.insert(0, str(HARNESS_ROOT / "orchestration")) +import harness_config # noqa: E402 + + +# -------------------------------------------------------------------------- +# Loading the shipped workflow the way a run loads it. +# +# A workflow declaration may reference the target's configuration -- the +# implementer's create restriction is the token `{{tests_dir}}` -- and the +# reference is resolved when the definition loads. A module that wants the +# definition a run of *this* repository executes therefore has to load it +# against *this* repository's configuration, which is what these two do. A +# module that learns the restricted prefix must learn it this way rather than +# by reading `workflows/story-workflow.json` as text, where it would find the +# token rather than the value. +# -------------------------------------------------------------------------- + + +def repository_config(root: Path = HARNESS_ROOT) -> dict: + """This repository's own `.harness/config.yaml`, loaded.""" + return harness_config.load_config(root) + + +#: `load_workflow` gained a required `config` argument when a workflow +#: declaration became able to reference configuration. A module that recovers +#: an entry point out of git to compare its behaviour against today's is +#: comparing the change that story made, not the arity of a call that story +#: never touched, so the recovered call site is repointed — minimally, at the +#: one line, keeping the recovered code otherwise byte for byte what it was. +#: Without it the recovered script raises TypeError and the comparison stops +#: being about its own subject. +HISTORICAL_WORKFLOW_LOADS = ( + # scripts/l5-plan + ('harness_config.load_workflow(\n' + ' HARNESS_ROOT, config.get("workflow", "story-workflow")\n' + ' )', + 'harness_config.load_workflow(\n' + ' HARNESS_ROOT, config.get("workflow", "story-workflow"), config\n' + ' )'), + # orchestration/story_coordinator.py + ('harness_config.load_workflow(harness_root, ' + 'config.get("workflow", "story-workflow"))', + 'harness_config.load_workflow(harness_root, ' + 'config.get("workflow", "story-workflow"), config)'), +) + + +def repointed_at_todays_signature(source: str) -> str: + """Recovered source, with each historical `load_workflow` call repointed. + + Only that call is touched, and only where it appears; everything else the + revision carried is byte for byte what it was. + """ + for old, new in HISTORICAL_WORKFLOW_LOADS: + source = source.replace(old, new) + return source + + +def shipped_workflow(root: Path = HARNESS_ROOT, + name: str = "story-workflow") -> dict: + """The named workflow under `root`, resolved against this repository's config. + + `root` is a harness root, which is this repository unless a test has + mirrored one; the configuration stays this repository's, because a + mirrored harness root has no target configuration of its own. + """ + return harness_config.load_workflow(root, name, repository_config()) + # -------------------------------------------------------------------------- # The one honest baseline resolution the per-story validation files share. @@ -451,6 +518,7 @@ def _committed(repo: Path, relative: str) -> bool: architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: echo tests-ok +tests_dir: tests/ """ diff --git a/tests/test_artifact_schemas.py b/tests/test_artifact_schemas.py index a71610a..d219b6c 100644 --- a/tests/test_artifact_schemas.py +++ b/tests/test_artifact_schemas.py @@ -15,6 +15,7 @@ import context_assembler import harness_config +import conftest import schema_validator import story_coordinator import story_parser @@ -44,7 +45,7 @@ #: into a harness copy holding only orchestration/, schemas/ and tests/, #: where workflows/ does not exist. def workflow_definition() -> dict: - return harness_config.load_workflow( + return conftest.shipped_workflow( Path(context_assembler.__file__).resolve().parents[1], "story-workflow") diff --git a/tests/test_attempt_archiving.py b/tests/test_attempt_archiving.py index b187a2b..a96efd1 100644 --- a/tests/test_attempt_archiving.py +++ b/tests/test_attempt_archiving.py @@ -15,6 +15,7 @@ import pytest from conftest import commit_setup, first_retry_route, story_diff +import conftest import context_assembler import harness_config @@ -28,7 +29,7 @@ #: escalates rather than routing it, so every failing verdict below #: carries one. RETRY_CATEGORY, RETRY_STAGE = first_retry_route( - harness_config.load_workflow(REPO_ROOT, "story-workflow")) + conftest.shipped_workflow(REPO_ROOT, "story-workflow")) PASS = {"status": "passed", "blocking_issues": [], "unverified": [], "retry_recommended": False} @@ -261,7 +262,7 @@ def stage_attempt_directory(name: str) -> bool: """ stem, sep, number = name.rpartition("-attempt-") stages = {stage["name"] for stage - in harness_config.load_workflow(REPO_ROOT, "story-workflow")["stages"]} + in conftest.shipped_workflow(REPO_ROOT, "story-workflow")["stages"]} return bool(sep) and stem in stages and number.isdigit() diff --git a/tests/test_clean_clone_check.py b/tests/test_clean_clone_check.py index 794f28d..5034119 100644 --- a/tests/test_clean_clone_check.py +++ b/tests/test_clean_clone_check.py @@ -47,6 +47,7 @@ import story_coordinator from agent_runner import AgentResult from conftest import first_retry_route, load_mutant, story_diff +import conftest #: The two stories this module validates, as `conftest.STORY_ORIGINS` #: declares them. Every story-range call below names one of these, because a @@ -57,8 +58,7 @@ REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] COORDINATOR_PATH = Path(story_coordinator.__file__) COORDINATOR_SOURCE = COORDINATOR_PATH.read_text(encoding="utf-8") -WORKFLOW = json.loads( - (REPO_ROOT / "workflows" / "story-workflow.json").read_text(encoding="utf-8")) +WORKFLOW = conftest.shipped_workflow() VERIFIER_STAGE = next(s for s in WORKFLOW["stages"] if s["name"] == "verifier") #: Since story-028 the clean-clone declaration names both artifacts of the #: check — the result it writes and the stage a failure routes to — so the diff --git a/tests/test_config_keys_are_obeyed.py b/tests/test_config_keys_are_obeyed.py index 2740883..d0cde78 100644 --- a/tests/test_config_keys_are_obeyed.py +++ b/tests/test_config_keys_are_obeyed.py @@ -98,6 +98,7 @@ "standards_dir": ".harness/xyzzy-standards", "stories_dir": ".harness/xyzzy-stories", "test_command": "xyzzy-runner --all", + "tests_dir": "xyzzy-checks/", "verification_runner": "/xyzzy/bin/interpreter", "workflow": "xyzzy-workflow", } @@ -119,6 +120,7 @@ "standards_dir": ".harness/standards", "stories_dir": ".harness/stories", "test_command": None, + "tests_dir": None, "verification_runner": None, "workflow": "story-workflow", } @@ -182,6 +184,9 @@ def node_id(self) -> str: "test_command": Proof( "test_test_command_is_the_command_the_clean_clone_path_builds", BEHAVIOURAL), + "tests_dir": Proof( + "test_tests_dir_is_the_location_the_workflow_and_the_prompt_are_governed_at", + BEHAVIOURAL), "verification_runner": Proof( "test_verification_runner_is_the_executable_the_check_resolves", BEHAVIOURAL), @@ -269,6 +274,14 @@ def node_id(self) -> str: 'config.get("test_command")', HARDCODED_TEST_COMMAND), ), + "tests_dir": ( + ("orchestration/harness_config.py", + 'config.get("tests_dir")', + "None"), + ("orchestration/context_assembler.py", + 'config.get("tests_dir")', + "None"), + ), "verification_runner": ( ("orchestration/story_coordinator.py", 'config.get("verification_runner")', @@ -548,11 +561,12 @@ def clean_clone_record(run: Run) -> dict: EXPECTED_KEYS = ( "allowed_tools", "architecture_docs", "base_branch", "branch_prefix", "logs_dir", "model", "permission_mode", "runs_dir", "standards_dir", - "stories_dir", "test_command", "verification_runner", "workflow", + "stories_dir", "test_command", "tests_dir", "verification_runner", + "workflow", ) -def test_declared_config_keys_returns_exactly_the_thirteen_names(): +def test_declared_config_keys_returns_exactly_the_declared_names(): assert set(DECLARED) == set(EXPECTED_KEYS) assert len(DECLARED) == len(EXPECTED_KEYS) @@ -594,7 +608,7 @@ def test_declared_config_keys_raises_rather_than_returning_a_partial_tuple( assert "harness-config.schema.json" in str(raised.value) -def test_the_same_reader_returns_the_thirteen_names_from_a_copied_schema(tmp_path): +def test_the_same_reader_returns_the_declared_names_from_a_copied_schema(tmp_path): """The positive control for the six refusals above. Each of them asserts a raise. That says nothing about whether the reader @@ -782,7 +796,12 @@ def test_the_scan_does_not_count_a_subscript_through_a_variable(tmp_path): " return config\n", encoding="utf-8") assert keys_read_under([planted]) == set() - assert keys_read_in(REPO_ROOT / "orchestration" / "harness_config.py") == set() + # And over the real module, which both builds the mapping that way *and* + # reads one key by name: exactly the literal read is counted, and neither + # of the two variables the mapping is built through joins it. + read = keys_read_in(REPO_ROOT / "orchestration" / "harness_config.py") + assert read == {"tests_dir"} + assert not read & {"key", "current_list"} # -------------------------------------------------------------------------- @@ -936,10 +955,35 @@ def test_workflow_names_the_definition_the_run_actually_executes(tmp_path): AUDIT_STAGE] assert (run.run_dir / AUDIT_ARTIFACT).is_file() assert AUDIT_STAGE not in [ - stage["name"] for stage in harness_config.load_workflow( + stage["name"] for stage in conftest.shipped_workflow( REPO_ROOT, "story-workflow")["stages"]] +def test_tests_dir_is_the_location_the_workflow_and_the_prompt_are_governed_at( + tmp_path): + """The configured location governs both halves, observed rather than read. + + The fixture configures its tests somewhere no harness would guess, and the + workflow definition names no directory at all — it carries the token. So + the restriction the coordinator enforces can only have come from the + configuration, and the same value has to reach the stage that writes the + tests, or the stage is told to write them somewhere the coordinator does + not govern. + """ + run = complete_run(tmp_path) + workflow = harness_config.load_workflow(run.harness, FIXTURE_WORKFLOW, + run.config) + assert story_coordinator.stage_restrictions(workflow["stages"]) == [ + ("implementer", "xyzzy-checks/")] + assert "xyzzy-checks/" in run.prompt_for("tester") + # The definition itself names no directory: what the restriction resolves + # to is the configuration's answer and nothing else. + definition = (run.harness / "workflows" / f"{FIXTURE_WORKFLOW}.json" + ).read_text(encoding="utf-8") + assert "xyzzy-checks/" not in definition + assert "{{tests_dir}}" in definition + + def test_test_command_is_the_command_the_clean_clone_path_builds(tmp_path): run = complete_run(tmp_path) assert "xyzzy-runner --all" in run.prompt_for("implementer") diff --git a/tests/test_configured_test_location.py b/tests/test_configured_test_location.py new file mode 100644 index 0000000..24fc906 --- /dev/null +++ b/tests/test_configured_test_location.py @@ -0,0 +1,1013 @@ +"""Independent validation for story-046: the test location comes from +configuration. + +The harness used to write a target repository's test layout into its own +definitions — `workflows/story-workflow.json` declared the implementer's create +restriction as the literal `tests/`, and `prompts/tester.md` told every tester +that new tests belong there. A target keeping its tests anywhere else was +governed in the wrong place, and a target with no test directory at all could +not be expressed. + +Written from the story's acceptance criteria rather than from the +implementation, and almost entirely by *exercising* the configured location +rather than by reading the code that resolves it. The recurring fixture +configures a target's tests at `xyzzy-spec/`, a location no harness would guess +and this repository does not contain, so a check observed to fire there can +only have learned it from the configuration. + +Four altitudes: + + * **the resolution.** `harness_config.load_workflow` is driven directly with + configs that set the key, omit it, and answer nothing, and what comes back + is read through `story_coordinator.stage_restrictions` rather than out of + the definition file. + * **the refusal.** A workflow carrying a reference the config cannot answer + is run through the real `story_coordinator.run_story`, and what the refusal + *left behind* is read off the tree. + * **the enforcement.** The ownership check, the stage baseline, the revert + check and the grant validation are each exercised against the configured + location by running the coordinator, with a real suite under `xyzzy-spec/` + for the two that need one. + * **the prompt.** The tester prompt is rendered against two different configs + with `prompts/tester.md` itself unedited between the renderings. + +Every absence asserted here carries a demonstration that it can fail: + + * "an unset key leaves no create restriction" sits beside the same load with + the key set, which reports the pair; + * "no resolved restriction is ever the empty string" sits beside a mutant + resolver that substitutes an empty entry instead of dropping it, which the + same check reports — and beside the demonstration that an empty prefix + would govern every path; + * "the refused run created no run directory, no state file, no log, no branch + and invoked no agent" sits beside the same fixture whose workflow carries a + resolvable reference, where the same five observations find all five; + * "creating under the configured location escalates" is paired with the same + record naming `tests/` — the location the harness used to assume — which + does *not* escalate, so the governance is shown to have moved rather than + merely to exist; + * "the stage baseline holds the configured location" sits beside the same + baseline directory asserted to hold nothing under `tests/`; + * "a grant outside the configured location is refused" sits beside grants at + and beneath it, which are accepted; + * "the rendered tester prompt carries no unresolved placeholder" sits beside + the placeholder the prompt does carry, resolved to the configured value and + to a different one; + * "every committed story artifact passes plan-time validation" sits beside a + copy of one of them carrying a planted defect, which the same validation + reports. + +Nothing here invokes a model: every run goes through the fake runner below and +every suite that runs is a handful of local files. +""" +import json +import shlex +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +import conftest +from conftest import commit_setup, load_mutant + +import context_assembler +import harness_config +import plan_validation +import schema_validator +import story_coordinator +import story_parser +from agent_runner import AgentResult + +REPO_ROOT = Path(harness_config.__file__).resolve().parents[1] +STORIES_DIR = REPO_ROOT / ".harness" / "stories" +WORKFLOW_NAME = "story-workflow" +STORY_ID = "story-001" + +#: The configured location every fixture below uses. Deliberately a name this +#: repository does not contain and no harness would guess: a check observed +#: firing here cannot have learned the location from anywhere but the config. +CONFIGURED = "xyzzy-spec/" + +#: The location the harness used to assume, kept as the paired control. A +#: record naming this one under a target configured at CONFIGURED must go +#: *un*governed, or "the restriction moved" would be indistinguishable from +#: "the restriction is enforced in both places". +ASSUMED = "tests/" + +#: A reference no configuration answers. `branch_prefix` is a real, declared +#: config key, so this is the strongest form of the unanswerable case: the +#: story rejected a general mechanism, and a declared key that is nonetheless +#: not referable is what "narrow" means. +UNANSWERABLE = "branch_prefix" + +PASS_VERDICT = {"status": "passed", "blocking_issues": [], "unverified": [], + "retry_recommended": False} +EMPTY_RECORD = {"modified": [], "created": [], "deleted": []} + +#: A runner that exists everywhere this suite runs, so a control run's +#: clean-clone check resolves it. +WORKING_RUNNER = "/bin/echo" + + +# -------------------------------------------------------------------------- +# Loading the shipped definition against a config of this module's choosing +# -------------------------------------------------------------------------- + + +def config_with(tests_dir: str | None) -> dict: + """This repository's config with its test location set, or removed. + + Built off the real config rather than from a literal so the load below + differs from a real one in exactly the key under test. + """ + config = dict(conftest.repository_config()) + if tests_dir is None: + config.pop("tests_dir", None) + else: + config["tests_dir"] = tests_dir + return config + + +def restrictions_under(tests_dir: str | None, + harness_root: Path = REPO_ROOT) -> list[tuple[str, str]]: + """What the coordinator would enforce for a target configured this way. + + Read through `story_coordinator.stage_restrictions`, which is the one + derivation every reader of a loaded workflow goes through, rather than out + of the definition file — where the value is not written at all. + """ + workflow = harness_config.load_workflow(harness_root, WORKFLOW_NAME, + config_with(tests_dir)) + return story_coordinator.stage_restrictions(workflow["stages"]) + + +DEFINITION_TEXT = (REPO_ROOT / "workflows" / f"{WORKFLOW_NAME}.json").read_text( + encoding="utf-8") + + +# -------------------------------------------------------------------------- +# 1. The token resolves out of configuration +# -------------------------------------------------------------------------- + + +def test_a_configured_location_is_the_restriction_the_coordinator_enforces(): + """The definition names no directory, so the pair can only have come from + the configuration.""" + assert restrictions_under(CONFIGURED) == [("implementer", CONFIGURED)] + assert CONFIGURED not in DEFINITION_TEXT + assert "{{tests_dir}}" in DEFINITION_TEXT + + +def test_a_different_configured_location_moves_the_restriction_with_it(): + """The same definition, a different config, a different restriction — with + nothing on disk changed between the two loads.""" + assert restrictions_under("spec/") == [("implementer", "spec/")] + assert restrictions_under("__tests__/") == [("implementer", "__tests__/")] + + +def test_a_target_declaring_no_test_location_carries_no_create_restriction(): + """The absence, with its control in the same test: the identical load with + the key set reports the pair, so "no pair" is the unset key and not a + `stage_restrictions` that stopped seeing anything.""" + assert restrictions_under(None) == [] + assert restrictions_under(CONFIGURED) != [] + + +def test_the_unset_key_removes_the_entry_rather_than_emptying_the_list_item(): + """`may_not_create` resolves to an empty list, not to a list holding an + empty string. Read off the loaded stage itself, because that is what every + reader of the definition other than `stage_restrictions` looks at.""" + workflow = harness_config.load_workflow(REPO_ROOT, WORKFLOW_NAME, + config_with(None)) + implementer = next(s for s in workflow["stages"] if s["name"] == "implementer") + assert implementer.get("may_not_create", []) == [] + assert "may_not_create" not in implementer or implementer["may_not_create"] == [] + + +@pytest.mark.parametrize("tests_dir", [CONFIGURED, "spec/", None]) +def test_no_resolved_restriction_is_ever_the_empty_string(tests_dir): + assert "" not in [prefix for _, prefix in restrictions_under(tests_dir)] + + +def test_a_resolver_that_emptied_the_entry_is_reported_by_the_same_check(tmp_path): + """The control for the absence above. + + The failure it guards against is not hypothetical: an unset key resolving + to `""` leaves a restriction whose prefix every path in the repository + starts with. A mutant resolver that substitutes the empty entry instead of + dropping it is loaded here, the same definition is loaded through it with + the key unset, and the same check reports the empty prefix — and the + prefix is shown to match a path it has no business matching. + """ + mutant = load_mutant( + REPO_ROOT / "orchestration" / "harness_config.py", + [(" elif values[name]:\n resolved.append(values[name])", + " else:\n resolved.append(values[name] or \"\")")], + name="harness_config_emptying_the_entry", tmp_path=tmp_path) + + workflow = mutant.load_workflow(REPO_ROOT, WORKFLOW_NAME, config_with(None)) + prefixes = [p for _, p in story_coordinator.stage_restrictions(workflow["stages"])] + + assert "" in prefixes, prefixes + # And that is what makes it wrong rather than merely odd. + assert "src/app.py".startswith("") + + +def test_the_narrow_set_does_not_admit_an_arbitrary_declared_config_key(tmp_path): + """The story rejected a general mechanism. `branch_prefix` is a declared, + set config key and is still not referable, which is what makes the + resolution narrow rather than general.""" + harness = mirror_harness(tmp_path, referencing(UNANSWERABLE)) + config = config_with(CONFIGURED) + assert config.get(UNANSWERABLE) # the key really is set + + with pytest.raises(harness_config.UnresolvedWorkflowToken) as raised: + harness_config.load_workflow(harness, WORKFLOW_NAME, config) + + assert raised.value.workflow == WORKFLOW_NAME + assert raised.value.tokens == [UNANSWERABLE] + assert any(UNANSWERABLE in problem for problem in raised.value.problems) + + +# -------------------------------------------------------------------------- +# 2. A harness root carrying a doctored definition +# -------------------------------------------------------------------------- + + +def write_json(path: Path, payload) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + + +def raw_definition() -> dict: + """The shipped definition with its references *unresolved*. + + Read as JSON rather than through `load_workflow`, because what these + fixtures need is the declaration as it ships — token and all — to write + back out into a harness root of their own. + """ + return json.loads(DEFINITION_TEXT) + + +def referencing(key: str) -> dict: + """The shipped definition with the implementer's restriction pointed at + another configuration key.""" + definition = raw_definition() + for stage in definition["stages"]: + if stage["name"] == "implementer": + stage["may_not_create"] = ["{{%s}}" % key] + return definition + + +def mirror_harness(tmp_path: Path, definition: dict) -> Path: + """A harness root identical to this one but for its workflow definition.""" + fake = tmp_path / "harness" + (fake / "workflows").mkdir(parents=True) + for shared in ("prompts", "schemas", "rules"): + (fake / shared).symlink_to(REPO_ROOT / shared) + write_json(fake / "workflows" / f"{WORKFLOW_NAME}.json", definition) + return fake + + +# -------------------------------------------------------------------------- +# 3. The fixture target, configured somewhere the harness would never guess +# -------------------------------------------------------------------------- + + +def set_config(target_root: Path, **overrides) -> None: + """Rewrite the target's config keys, adding those it does not carry. + + Committed, because story-021's clean-tree pre-flight refuses a run whose + target tree holds work no stage produced, and a test's own configuration is + part of the repository the run starts *from*. + """ + path = target_root / ".harness" / "config.yaml" + lines = path.read_text(encoding="utf-8").splitlines() + for key, value in overrides.items(): + rendered = f"{key}: {value}" + for index, line in enumerate(lines): + if line.startswith(f"{key}:"): + lines[index] = rendered + break + else: + lines.append(rendered) + path.write_text("\n".join(lines) + "\n", encoding="utf-8") + commit_setup(target_root, "configure the target for this test") + + +def drop_config(target_root: Path, *keys: str) -> None: + path = target_root / ".harness" / "config.yaml" + kept = [line for line in path.read_text(encoding="utf-8").splitlines() + if not any(line.startswith(f"{key}:") for key in keys)] + path.write_text("\n".join(kept) + "\n", encoding="utf-8") + commit_setup(target_root, "remove config keys for this test") + + +@pytest.fixture +def elsewhere(target_root: Path) -> Path: + """The shared target, configuring its tests at a location no harness would + guess, and with a resolvable verification runner so a control run + completes.""" + set_config(target_root, tests_dir=CONFIGURED, + verification_runner=WORKING_RUNNER) + return target_root + + +class Runner: + """A fake agent runner whose per-stage changed-files record is the input. + + It records every stage it was asked to run, which is how "no agent was + invoked" becomes a fact about the coordinator rather than the absence of a + log nobody wrote. + """ + + def __init__(self, target_root: Path, story_id: str = STORY_ID, *, + records: dict[str, dict] | None = None): + self.target_root = target_root + self.run_dir = target_root / ".harness" / "runs" / story_id + self.records = records or {} + self.calls: list[str] = [] + + def _record(self, stage: str) -> dict: + return self.records.get(stage, dict(EMPTY_RECORD)) + + def __call__(self, prompt, *, stage, cwd=None, log_path=None, + permission_mode=None, model=None, allowed_tools=None): + self.calls.append(stage) + self.prompts = getattr(self, "prompts", {}) + self.prompts[stage] = prompt + if log_path is not None: + Path(log_path).parent.mkdir(parents=True, exist_ok=True) + with open(log_path, "a", encoding="utf-8") as handle: + handle.write(f"===== stage: {stage} =====\n") + if stage == "implementer": + write_json(self.run_dir / "changed-files.json", self._record(stage)) + (self.run_dir / "implementation-summary.md").write_text( + "Did it.\n", encoding="utf-8") + elif stage == "tester": + write_json(self.run_dir / "test-results.json", { + "status": "passed", "tests_written": 1, "tests_run": 1, + "tests_passed": 1, "tests_failed": 0, "failures": [], + }) + write_json(self.run_dir / "tester-changed-files.json", + self._record(stage)) + elif stage == "documenter": + (self.run_dir / "documentation-report.md").write_text( + "Nothing.\n", encoding="utf-8") + write_json(self.run_dir / "documenter-changed-files.json", + self._record(stage)) + elif stage == "verifier": + write_json(self.run_dir / "verification-result.json", PASS_VERDICT) + return AgentResult(ok=True, result_text=f"{stage} done") + + +def git(root: Path, *args: str) -> str: + return subprocess.run(["git", "-C", str(root), *args], + capture_output=True, text=True, check=True).stdout + + +def branches(root: Path) -> set[str]: + return set(git(root, "branch", "--format=%(refname:short)").split()) + + +def evidence(target_root: Path, story_id: str = STORY_ID) -> tuple[str, str]: + run_dir = target_root / ".harness" / "runs" / story_id + return ((run_dir / "events.log").read_text(encoding="utf-8"), + (run_dir / "escalation-summary.md").read_text(encoding="utf-8")) + + +def run(target_root: Path, harness_root: Path, **kwargs): + runner = Runner(target_root, **kwargs) + code = story_coordinator.run_story(STORY_ID, harness_root, target_root, + runner) + return code, runner, runner.run_dir + + +# -------------------------------------------------------------------------- +# 4. The pre-flight refusal +# -------------------------------------------------------------------------- + + +def test_a_workflow_referencing_configuration_the_config_cannot_answer_is_refused( + elsewhere, tmp_path, capsys, +): + harness = mirror_harness(tmp_path, referencing(UNANSWERABLE)) + + code, _, _ = run(elsewhere, harness) + + refusal = capsys.readouterr().err + assert code == 1 + assert f"{{{{{UNANSWERABLE}}}}}" in refusal, refusal + assert WORKFLOW_NAME in refusal, refusal + + +def test_that_refusal_leaves_no_run_directory_no_state_no_log_no_branch_no_agent( + elsewhere, tmp_path, +): + """Read off the refused target's tree, as the story asks, rather than off + the exit status. Its control is the next test, which makes the same five + observations of the same fixture under a definition that resolves.""" + harness = mirror_harness(tmp_path, referencing(UNANSWERABLE)) + before = branches(elsewhere) + + code, runner, run_dir = run(elsewhere, harness) + + assert code == 1 + assert not run_dir.exists() + assert not (run_dir / "state.json").exists() + assert not (elsewhere / ".harness" / "logs" / f"{STORY_ID}.log").exists() + assert branches(elsewhere) == before + assert runner.calls == [] + + +def test_the_same_fixture_under_a_resolvable_definition_creates_all_five( + elsewhere, tmp_path, +): + """The control the five absences above need: the identical mirror carrying + the shipped definition, whose one reference this config does answer.""" + harness = mirror_harness(tmp_path, raw_definition()) + before = branches(elsewhere) + + code, runner, run_dir = run(elsewhere, harness) + + assert code == 0, runner.calls + assert run_dir.is_dir() + assert json.loads((run_dir / "state.json").read_text( + encoding="utf-8"))["status"] == "completed" + assert (elsewhere / ".harness" / "logs" / f"{STORY_ID}.log").is_file() + assert branches(elsewhere) - before == {f"story/{STORY_ID}"} + assert runner.calls == ["implementer", "tester", "documenter", "verifier"] + + +# -------------------------------------------------------------------------- +# 5. The ownership check governs the configured location +# -------------------------------------------------------------------------- + + +def test_a_stage_creating_under_the_configured_location_escalates(elsewhere, + harness_root): + created = f"{CONFIGURED}test_new.py" + code, runner, _ = run(elsewhere, harness_root, records={ + "implementer": {"modified": [], "created": [created], "deleted": []}, + }) + + assert code == 2 + assert runner.calls == ["implementer"] + events, summary = evidence(elsewhere) + for text in (events, summary): + assert "implementer" in text + assert created in text + assert CONFIGURED in text + + +def test_the_same_stage_creating_under_the_location_the_harness_used_to_assume_does_not( + elsewhere, harness_root, +): + """The control for the escalation above, and the whole point of the story: + under a target configured at `xyzzy-spec/`, `tests/` is an ordinary + directory. If both escalated, the restriction would not have *moved*.""" + code, runner, _ = run(elsewhere, harness_root, records={ + "implementer": {"modified": [], "created": [f"{ASSUMED}test_new.py"], + "deleted": []}, + }) + + assert code == 0, runner.calls + assert runner.calls == ["implementer", "tester", "documenter", "verifier"] + + +def test_a_target_declaring_no_test_location_governs_nothing(elsewhere, + harness_root): + """The absence at run time: with the key removed the implementer may create + under both locations. Its control is the two tests above, which are the + same fixture and the same records with the key set.""" + drop_config(elsewhere, "tests_dir") + + code, runner, _ = run(elsewhere, harness_root, records={ + "implementer": {"modified": [], + "created": [f"{CONFIGURED}test_new.py", + f"{ASSUMED}test_new.py"], + "deleted": []}, + }) + + assert code == 0, runner.calls + assert runner.calls == ["implementer", "tester", "documenter", "verifier"] + + +# -------------------------------------------------------------------------- +# 6. The grant validation reads the configured location +# -------------------------------------------------------------------------- + + +def stages_at(tests_dir: str | None) -> list[dict]: + return harness_config.load_workflow(REPO_ROOT, WORKFLOW_NAME, + config_with(tests_dir))["stages"] + + +def granting(create: str) -> dict: + return {"stage_exceptions": [ + {"stage": "implementer", "create": create, + "reason": "the deliverable is the suite"}]} + + +@pytest.mark.parametrize("granted", [ + CONFIGURED, + f"{CONFIGURED}test_the_thing.py", + f"{CONFIGURED}unit/", +], ids=["the whole location", "a file beneath it", "a directory beneath it"]) +def test_a_grant_at_or_beneath_the_configured_location_is_accepted(granted): + assert story_coordinator.stage_exception_problems( + granting(granted), stages_at(CONFIGURED)) == [] + + +@pytest.mark.parametrize("granted", [ASSUMED, "src/helpers.py"], + ids=["the assumed location", "somewhere else entirely"]) +def test_a_grant_outside_the_configured_location_is_refused(granted): + """The control for the acceptances above sits in the same pair of tests: + the identical call shape, differing only in whether the granted path falls + under what the configuration declared.""" + problems = story_coordinator.stage_exception_problems( + granting(granted), stages_at(CONFIGURED)) + assert len(problems) == 1, problems + assert granted in problems[0] + + +def test_under_an_unset_location_no_grant_is_accepted_at_all(): + """A stage restricted from nothing was never restricted from creating the + granted path, so the grant means nothing and is refused — the same answer + the check has always given for an ungoverned grant.""" + problems = story_coordinator.stage_exception_problems( + granting(CONFIGURED), stages_at(None)) + assert len(problems) == 1, problems + assert CONFIGURED in problems[0] + + +# -------------------------------------------------------------------------- +# 7. The stage baseline and the revert check, over a real suite +# -------------------------------------------------------------------------- + +TEST_COMMAND = shlex.join([sys.executable, "-m", "pytest", CONFIGURED.rstrip("/"), + "-q", "-p", "no:cacheprovider"]) + +APP_AT_HEAD = '''\ +def greet(name): + return f"hello, {name}" +''' + +APP_RENAMED = '''\ +def salute(name): + return f"hello, {name}" +''' + +SPEC_AT_HEAD = '''\ +from app import greet + + +def test_greet(): + assert greet("world") == "hello, world" +''' + +SPEC_REPAIRED = '''\ +from app import salute + + +def test_greet(): + assert salute("world") == "hello, world" +''' + +ROOT_CONFTEST = '''\ +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "src")) +''' + + +@pytest.fixture +def suite_target(tmp_path: Path) -> Path: + """A target whose tests live at the configured location and really run. + + Everything the coordinator's revert check needs is here — a module, a suite + over it, and a configured command that runs that suite — with the suite + sitting at `xyzzy-spec/` rather than at the name the harness used to + assume. + """ + root = tmp_path / "elsewhere-target" + for sub in (".harness/standards", ".harness/stories", ".harness/runs", + ".harness/logs", ".harness/docs"): + (root / sub).mkdir(parents=True) + write(root / ".harness" / "config.yaml", f"""\ +workflow: {WORKFLOW_NAME} +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} +tests_dir: {CONFIGURED} +""") + write(root / ".harness" / "stories" / f"{STORY_ID}.yaml", conftest.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 / CONFIGURED / "test_app.py", SPEC_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 + + +def write(path: Path, text: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + + +class RenamingRunner(Runner): + """An implementer whose rename forces the edit to the suite beside it. + + The repair under the configured location is exactly the shape the revert + check exists to permit: revert it and the suite stops compiling, so the + edit was maintenance the change forced rather than validation the + implementer authored. + """ + + def __call__(self, prompt, *, stage, **kwargs): + if stage == "implementer": + write(self.target_root / "src" / "app.py", APP_RENAMED) + write(self.target_root / CONFIGURED / "test_app.py", SPEC_REPAIRED) + self.records["implementer"] = { + "modified": ["src/app.py", f"{CONFIGURED}test_app.py"], + "created": [], "deleted": [], + } + return super().__call__(prompt, stage=stage, **kwargs) + + +def implementer_declaration() -> dict: + stage = next(s for s in stages_at(CONFIGURED) if s["name"] == "implementer") + return stage["revert_check"] + + +def test_the_stage_baseline_is_captured_over_the_configured_location(suite_target, + harness_root): + """What the tree held under the *configured* prefix before the stage ran. + + Asserted as an exact set rather than a containment, so it says both what + the capture followed and what it did not sweep in: exactly the one file at + the configured location, and nothing from `src/` — which the same stage + modified in the same record and which is not governed. The paired control + is `test_the_same_target_with_no_configured_location_reverts_nothing`, + where the identical run with the one config line removed captures nothing. + """ + runner = RenamingRunner(suite_target) + code = story_coordinator.run_story(STORY_ID, harness_root, suite_target, + runner) + assert code == 0, runner.calls + + declaration = implementer_declaration() + baseline = story_coordinator.stage_baseline_dir( + runner.run_dir, declaration["baseline"], "implementer") + captured = sorted(str(p.relative_to(baseline)) + for p in baseline.rglob("*") if p.is_file()) + + assert captured == [f"{CONFIGURED}test_app.py"] + # And what it holds is what the tree held *before* the stage, not after. + assert (baseline / CONFIGURED / "test_app.py").read_text( + encoding="utf-8") == SPEC_AT_HEAD + + +def test_an_edit_under_the_configured_location_reaches_the_revert_check( + suite_target, harness_root, +): + """The check ran, it reverted the path at the configured location, and it + decided on the suite that actually ran there.""" + runner = RenamingRunner(suite_target) + code = story_coordinator.run_story(STORY_ID, harness_root, suite_target, + runner) + assert code == 0, runner.calls + + result = json.loads( + (runner.run_dir / implementer_declaration()["result"]).read_text( + encoding="utf-8")) + schema_validator.validate(result, schema_validator.load_schema( + "revert-check-result")) + + assert result["ran"] is True, result + assert result["paths"] == [f"{CONFIGURED}test_app.py"], result + assert result["permitted"] is True, result + # The source edit is not governed, so the check is shown to have narrowed + # to the configured location rather than reverting the whole record. + assert "src/app.py" not in result["paths"] + + +def test_the_same_target_with_no_configured_location_reverts_nothing( + suite_target, harness_root, +): + """The control for both assertions above: remove the one config line and + the identical run captures an empty baseline and reverts nothing, because + the implementer is no longer governed anywhere.""" + drop_config(suite_target, "tests_dir") + + runner = RenamingRunner(suite_target) + code = story_coordinator.run_story(STORY_ID, harness_root, suite_target, + runner) + assert code == 0, runner.calls + + declaration = implementer_declaration() + baseline = story_coordinator.stage_baseline_dir( + runner.run_dir, declaration["baseline"], "implementer") + assert [p for p in baseline.rglob("*") if p.is_file()] == [] + + # With nothing governed there is nothing to revert, so the check has + # nothing to decide and writes no result at all — where the same run with + # the key set writes one naming the path at the configured location. + assert not (runner.run_dir / declaration["result"]).exists() + + +# -------------------------------------------------------------------------- +# 8. The rendered tester prompt +# -------------------------------------------------------------------------- + +TESTER_PROMPT = REPO_ROOT / "prompts" / "tester.md" + + +def rendered_tester_prompt(target_root: Path, config: dict) -> str: + story_text = (target_root / ".harness" / "stories" + / f"{STORY_ID}.yaml").read_text(encoding="utf-8") + run_dir = target_root / ".harness" / "runs" / STORY_ID + run_dir.mkdir(parents=True, exist_ok=True) + context = context_assembler.build_context( + story_text=story_text, + story=story_parser.parse(story_text, + schema_validator.load_schema("story")), + run_dir=run_dir, + target_root=target_root, + harness_root=REPO_ROOT, + config=config, + rules=harness_config.load_rules(REPO_ROOT), + workflow=harness_config.load_workflow(REPO_ROOT, WORKFLOW_NAME, config), + retry_count=0, + ) + return context_assembler.render( + context_assembler.load_template(REPO_ROOT, "tester.md"), context) + + +def test_the_rendered_tester_prompt_names_the_configured_location(target_root): + config = {**harness_config.load_config(target_root), "tests_dir": CONFIGURED} + rendered = rendered_tester_prompt(target_root, config) + + assert CONFIGURED in rendered + assert "{{" not in rendered + # The template really did carry a placeholder here, so the value in the + # rendering is an injection and not prose that happens to match. + assert "{{tests_dir}}" in TESTER_PROMPT.read_text(encoding="utf-8") + + +def test_changing_the_configured_location_changes_the_rendered_prompt(target_root): + """With `prompts/tester.md` unedited between the two renderings — asserted + by reading the file's bytes before and after, so "no prompt edit" is + observed rather than assumed.""" + base = harness_config.load_config(target_root) + before = TESTER_PROMPT.read_bytes() + + # Two locations neither of which is a substring of the other, so "the + # other one is absent" is a real observation rather than an accident of + # spelling. + one, other = "xyzzy-spec/", "plugh-probes/" + first = rendered_tester_prompt(target_root, {**base, "tests_dir": one}) + second = rendered_tester_prompt(target_root, {**base, "tests_dir": other}) + + assert TESTER_PROMPT.read_bytes() == before + assert first != second + assert one in first and one not in second + assert other in second and other not in first + + +def test_a_target_declaring_no_test_location_renders_the_optional_placeholder( + target_root, +): + """The optional-placeholder convention: nothing to inject renders as None + rather than as an empty string or a leftover token.""" + config = {k: v for k, v in harness_config.load_config(target_root).items() + if k != "tests_dir"} + rendered = rendered_tester_prompt(target_root, config) + + assert "{{" not in rendered + assert "New tests belong in None" in rendered + + +#: What the two replaced sentences said, and the check each one is caught by. +#: Written here so the control below can put them back rather than argue that +#: the absence above is meaningful. +REPLACED_PROSE = ( + "New tests belong in tests/ and become permanent repository assets.", + "Use the shared resolution in `tests/conftest.py`.", +) + +#: A target layout name and a test-framework filename, neither of which a +#: harness may state on a target's behalf. +FORBIDDEN_IN_PROMPTS = ("tests/", "conftest.py", "pytest") + + +def forbidden_in(text: str) -> list[str]: + return [name for name in FORBIDDEN_IN_PROMPTS if name in text] + + +def test_the_tester_prompt_names_no_directory_and_no_framework_filename(): + """The prose the story replaced, gone from the template it was in.""" + assert forbidden_in(TESTER_PROMPT.read_text(encoding="utf-8")) == [] + + +@pytest.mark.parametrize("prose", REPLACED_PROSE) +def test_the_same_check_reports_the_replaced_prose_put_back(prose): + """The control for the absence above: each replaced sentence appended to a + rendering of the same template is reported by the same check, so the empty + list is the prose being gone rather than the check having stopped reading + anything.""" + restored = TESTER_PROMPT.read_text(encoding="utf-8") + "\n" + prose + "\n" + assert forbidden_in(restored) != [] + + +def test_no_prompt_in_the_repository_names_either(): + """The criterion is stated over `prompts/`, not over the tester prompt + alone. Its control is the two tests above, which run the same check over + the same kind of text and report.""" + offending = {path.name: forbidden_in(path.read_text(encoding="utf-8")) + for path in sorted((REPO_ROOT / "prompts").glob("*.md"))} + assert {name: found for name, found in offending.items() if found} == {} + assert len(offending) >= 5, offending + + +# -------------------------------------------------------------------------- +# 9. Every committed story artifact validates exactly as it did before +# +# Three of plan-time validation's checks read the restriction this story moved +# into configuration: the stage-exception cross-check, the strictness check and +# the assignment check. They are what is run here, over *every* committed +# artifact rather than a sample. +# +# The fourth thing `artifact_problems` does — `read_story`'s schema pass — is +# deliberately not asserted clean. Plan-time validation has only ever run on +# the artifacts a planning session *adds*, and this repository's earliest +# stories predate fields the story schema has since required, so they do not +# pass it today and did not pass it before this story either. Asserting they do +# would be asserting something this story neither caused nor could fix, and the +# comparison below is the honest form of "unchanged": the same artifacts, +# through the same three checks, give the identical answer whether the prefix +# arrives as a resolved token or as the literal the definition used to carry. +# -------------------------------------------------------------------------- + + +def committed_artifacts() -> list[Path]: + return sorted(STORIES_DIR.glob("*.yaml")) + + +def restriction_problems(artifact: Path, stages: list[dict]) -> list[str]: + """The three plan-time checks that read `stage_restrictions`, for one + artifact. Nothing here names a stage or a prefix; both come off `stages`.""" + reading = story_coordinator.read_story( + artifact.read_text(encoding="utf-8")) + return ( + story_coordinator.stage_exception_problems(reading.parsed, stages) + + plan_validation.strictness_problems(reading.parsed, stages) + + plan_validation.assignment_problems(reading.parsed, stages, REPO_ROOT) + ) + + +def literal_stages(prefix: str) -> list[dict]: + """The definition as it read before this story: the prefix spelled out. + + Built from today's shipped declaration with the token replaced by the + literal, so the comparison is against the pre-story shape reconstructed + from what ships rather than recovered from history — which keeps it honest + once this story commits. + """ + definition = raw_definition() + for stage in definition["stages"]: + if "may_not_create" in stage: + stage["may_not_create"] = [prefix] + return definition["stages"] + + +def test_every_committed_story_artifact_validates_exactly_as_it_did_before(): + """All 46 of them, not a sample — including every one whose scope, + do_not_modify or stage_exceptions names `tests/`.""" + artifacts = committed_artifacts() + assert len(artifacts) > 40, len(artifacts) + location = conftest.repository_config()["tests_dir"] + + resolved = {a: restriction_problems(a, stages_at(location)) + for a in artifacts} + literal = {a: restriction_problems(a, literal_stages(location)) + for a in artifacts} + + assert resolved == literal + + +def test_that_comparison_reports_a_difference_when_there_is_one(): + """The control for the equality above, which is an absence of difference: + resolve the token somewhere else and the same comparison goes red, so the + equality is the restriction being unchanged rather than the comparison + having stopped looking at anything.""" + artifacts = committed_artifacts() + location = conftest.repository_config()["tests_dir"] + + resolved = {a: restriction_problems(a, stages_at(location)) + for a in artifacts} + elsewhere = {a: restriction_problems(a, stages_at(CONFIGURED)) + for a in artifacts} + + assert resolved != elsewhere + + +def test_every_committed_grant_is_still_accepted_against_the_resolved_prefix(): + """The criterion's own words: the artifacts whose `stage_exceptions` name + `tests/` are accepted with the value resolved rather than literal. + + The grants are counted rather than assumed, so "all accepted" cannot be + true of an empty set. + """ + location = conftest.repository_config()["tests_dir"] + stages = stages_at(location) + granting_artifacts, refused = [], {} + for artifact in committed_artifacts(): + reading = story_coordinator.read_story( + artifact.read_text(encoding="utf-8")) + if not reading.parsed.get("stage_exceptions"): + continue + granting_artifacts.append(artifact) + problems = story_coordinator.stage_exception_problems( + reading.parsed, stages) + if problems: + refused[artifact.name] = problems + + assert len(granting_artifacts) >= 5, [a.name for a in granting_artifacts] + assert refused == {} + + +def test_the_same_check_refuses_a_planted_grant(tmp_path): + """The control for the empty mapping above. + + A copy of a real artifact is given a grant naming a path outside the + configured location, and the same check reports it — so "no grant refused" + is the grants being sound rather than `stage_exception_problems` having + stopped looking. + """ + source = committed_artifacts()[-1] + planted = tmp_path / source.name + planted.write_text( + source.read_text(encoding="utf-8") + + "\nstage_exceptions:\n" + " - stage: implementer\n" + " create: src/somewhere-else.py\n" + " reason: it is not under the configured location\n", + encoding="utf-8") + + reading = story_coordinator.read_story( + planted.read_text(encoding="utf-8")) + problems = story_coordinator.stage_exception_problems( + reading.parsed, stages_at(conftest.repository_config()["tests_dir"])) + + assert len(problems) == 1, problems + assert "src/somewhere-else.py" in problems[0] + + +# -------------------------------------------------------------------------- +# 10. This repository declares its own location, and is unchanged by it +# -------------------------------------------------------------------------- + + +def test_the_key_is_declared_and_the_reader_returns_it(): + assert "tests_dir" in harness_config.declared_config_keys() + schema = json.loads( + (REPO_ROOT / "schemas" / "harness-config.schema.json").read_text( + encoding="utf-8")) + description = schema["properties"]["tests_dir"]["description"] + # It says what a set value governs and what its absence means. + assert "unset" in description.lower() + + +def test_this_repository_declares_the_directory_the_workflow_used_to_name(): + location = conftest.repository_config()["tests_dir"] + assert location == ASSUMED + assert (REPO_ROOT / location).is_dir() + assert story_coordinator.stage_restrictions( + conftest.shipped_workflow()["stages"]) == [("implementer", location)] + + +def test_a_newly_initialized_target_declares_a_location_too(): + template = (REPO_ROOT / "templates" / "config.yaml").read_text( + encoding="utf-8") + declared = [line for line in template.splitlines() + if line.startswith("tests_dir:")] + assert declared, template + assert declared[0].split(":", 1)[1].strip() diff --git a/tests/test_context_assembler.py b/tests/test_context_assembler.py index 04015e8..9528397 100644 --- a/tests/test_context_assembler.py +++ b/tests/test_context_assembler.py @@ -3,6 +3,7 @@ import context_assembler import harness_config +import conftest import schema_validator import story_parser @@ -10,7 +11,7 @@ #: The loaded workflow build_context has taken as a required argument #: since story-028, which injects the workflow's own facts — its stages, #: its create restrictions, its retry routes — into every stage prompt. -WORKFLOW = harness_config.load_workflow( +WORKFLOW = conftest.shipped_workflow( Path(context_assembler.__file__).resolve().parents[1], "story-workflow") diff --git a/tests/test_contract_assertions_bite.py b/tests/test_contract_assertions_bite.py index b745795..1b99b29 100644 --- a/tests/test_contract_assertions_bite.py +++ b/tests/test_contract_assertions_bite.py @@ -74,6 +74,13 @@ def mutant_repo(tmp_path: Path, replacements: list[tuple[str, str]]) -> Path: (root / "tests").mkdir() for name in ("conftest.py", CONTRACT_FILE.name): shutil.copy(TESTS_DIR / name, root / "tests" / name) + # `.harness/` is not symlinked, so a run started in here cannot write into + # the real one. Its config file is copied in all the same: a workflow + # declaration may reference configuration, so a root with no config has no + # answer for the reference and cannot load the definition at all. + (root / ".harness").mkdir() + shutil.copy(REPO_ROOT / ".harness" / "config.yaml", + root / ".harness" / "config.yaml") path = root / "orchestration" / "story_coordinator.py" source = path.read_text(encoding="utf-8") diff --git a/tests/test_coordinator_contract.py b/tests/test_coordinator_contract.py index cc6abe9..b1fbc3d 100644 --- a/tests/test_coordinator_contract.py +++ b/tests/test_coordinator_contract.py @@ -32,10 +32,10 @@ import story_coordinator from agent_runner import AgentResult from conftest import first_retry_route +import conftest REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] -WORKFLOW = json.loads( - (REPO_ROOT / "workflows" / "story-workflow.json").read_text(encoding="utf-8")) +WORKFLOW = conftest.shipped_workflow() STAGE_NAMES = [stage["name"] for stage in WORKFLOW["stages"]] #: The retry category a failing verdict names, read off the loaded workflow. diff --git a/tests/test_documenter_before_verification.py b/tests/test_documenter_before_verification.py index cb6e337..37100c0 100644 --- a/tests/test_documenter_before_verification.py +++ b/tests/test_documenter_before_verification.py @@ -67,10 +67,11 @@ import story_coordinator from agent_runner import AgentResult from conftest import BASELINE, ENDPOINT, repository_file_at +import conftest REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(REPO_ROOT, "story-workflow") STAGE_NAMES = [stage["name"] for stage in WORKFLOW["stages"]] #: The stage that declares the routing table, found by the declaration @@ -178,6 +179,7 @@ def failing(category: str) -> dict: architecture_docs: - {doc} test_command: {test_command} +tests_dir: tests/ """ diff --git a/tests/test_escalation_resume.py b/tests/test_escalation_resume.py index f43c574..a749581 100644 --- a/tests/test_escalation_resume.py +++ b/tests/test_escalation_resume.py @@ -60,13 +60,14 @@ from conftest import (BASELINE as BASELINE_BOUND, ENDPOINT, first_retry_route, function_source_at, load_script, repository_file_at, story_commit_range, story_diff) +import conftest import harness_config import story_coordinator from agent_runner import AgentResult REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(REPO_ROOT, "story-workflow") STAGE_NAMES = [stage["name"] for stage in WORKFLOW["stages"]] VERIFIER_STAGE = next(s for s in WORKFLOW["stages"] if "on_failure" in s) #: Since story-028 the route is a category-keyed table rather than a constant, @@ -161,6 +162,7 @@ def failing(attempt: int, *, retry: bool) -> dict: architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: echo tests-ok +tests_dir: tests/ """ APP_AT_HEAD = "print('hello')\n" diff --git a/tests/test_escalation_summary.py b/tests/test_escalation_summary.py index fe3910f..29f01e7 100644 --- a/tests/test_escalation_summary.py +++ b/tests/test_escalation_summary.py @@ -51,13 +51,14 @@ from conftest import (BASELINE, ENDPOINT, first_retry_route, function_source_at, story_diff, story_commit_range) +import conftest import harness_config import story_coordinator from agent_runner import AgentResult REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(REPO_ROOT, "story-workflow") STAGE_NAMES = [stage["name"] for stage in WORKFLOW["stages"]] VERIFIER_STAGE = next(s for s in WORKFLOW["stages"] if "on_failure" in s) #: Since story-028 the route is a category-keyed table rather than a constant, @@ -142,6 +143,7 @@ def failing(attempt: int, *, retry: bool) -> dict: architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: echo tests-ok +tests_dir: tests/ """ APP_AT_HEAD = "print('hello')\n" diff --git a/tests/test_execution_history.py b/tests/test_execution_history.py index 8b5fe1b..f7d17ab 100644 --- a/tests/test_execution_history.py +++ b/tests/test_execution_history.py @@ -36,10 +36,10 @@ import story_coordinator from agent_runner import AgentResult from conftest import commit_setup, first_retry_route, load_mutant, story_diff +import conftest REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] -WORKFLOW = json.loads( - (REPO_ROOT / "workflows" / "story-workflow.json").read_text(encoding="utf-8")) +WORKFLOW = conftest.shipped_workflow() STAGE_NAMES = [stage["name"] for stage in WORKFLOW["stages"]] #: Since story-028 a recommended retry must name a category the workflow's #: retry_routing table defines, or the coordinator escalates rather than diff --git a/tests/test_foreign_work_refusal.py b/tests/test_foreign_work_refusal.py index d9ca992..8da6597 100644 --- a/tests/test_foreign_work_refusal.py +++ b/tests/test_foreign_work_refusal.py @@ -143,6 +143,7 @@ def failing(attempt: int, *, retry: bool) -> dict: architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: echo tests-ok +tests_dir: tests/ """ GITIGNORE = ".harness/runs/\n.harness/logs/\n" diff --git a/tests/test_harness_layer_extraction.py b/tests/test_harness_layer_extraction.py index 4cda6ee..09caeed 100644 --- a/tests/test_harness_layer_extraction.py +++ b/tests/test_harness_layer_extraction.py @@ -12,6 +12,7 @@ import context_assembler import harness_config +import conftest import schema_validator import story_parser @@ -21,7 +22,7 @@ #: The loaded workflow build_context has taken as a required argument #: since story-028, which injects the workflow's own facts — its stages, #: its create restrictions, its retry routes — into every stage prompt. -WORKFLOW = harness_config.load_workflow( +WORKFLOW = conftest.shipped_workflow( Path(context_assembler.__file__).resolve().parents[1], "story-workflow") diff --git a/tests/test_no_target_stack_in_harness_source.py b/tests/test_no_target_stack_in_harness_source.py index 20cbaa6..743450c 100644 --- a/tests/test_no_target_stack_in_harness_source.py +++ b/tests/test_no_target_stack_in_harness_source.py @@ -8,16 +8,20 @@ * **the two lists.** `TEMPORARY_TIES` and `PERMANENT_MENTIONS` below are asserted to equal exactly what `harness_source.scan()` reports against - this repository, in both directions, and to share no entry. Each entry - is keyed by repository-relative path and the exact text of the matched - line rather than by a line number, so an unrelated edit above a tie - does not churn the list and look like the burn-down. - * **the audited ties that are still grandfathered.** Asserted present by - name, by running the scan rather than by reading the list — a scan that - cannot see the ties that motivated it has not been shown to work. The - audit's other two both sat in `orchestration/story_coordinator.py` and - were repaired by the-interpreter-is-not-assumed-to-be-python, which is - also why "no tie was fixed" now guards two files rather than three. + this repository, in both directions, and to classify every reported + entry exactly once. Each entry is keyed by repository-relative path and + the exact text of the matched line rather than by a line number, so an + unrelated edit above a tie does not churn the list and look like the + burn-down. + * **the burn-down, now complete.** `TEMPORARY_TIES` is empty: the audit's + five ties are all repaired — two in `orchestration/story_coordinator.py` + by the-interpreter-is-not-assumed-to-be-python, and the remaining three + in `workflows/story-workflow.json` and `prompts/tester.md` by + the-test-location-comes-from-configuration. What is asserted is that the + scan reports *nothing* in either repaired file, by running the scan + rather than by reading the empty list, with each historical tie replanted + in a throwaway copy and reported there — a scan that cannot see the ties + that motivated it has not been shown to work. * **the matcher.** The four boundary cases the story names are constructed and run through the real `scan`, not reasoned about. * **the stated limits.** Read out of `orchestration/harness_source.py`'s @@ -34,6 +38,12 @@ with a tie planted in a file that is clean today, where the same comparison reports an unexpected entry, and beside one with a known tie removed, where it reports a stale entry; + * "every reported entry is classified exactly once" sits beside an entry + handed to the same check on both lists, and beside a planted tie that is + on neither, both of which it reports; + * "the coordinator carries nothing but its one permanent mention" sits + beside a coordinator tie replanted in a throwaway copy, which the same + checks report; * "nothing under `.harness/` or `tests/` is reported" sits beside ties planted in both, and beside the same tie planted in a scanned directory, which is reported; @@ -45,9 +55,10 @@ none, which the same check reports; * "the module states its limits" sits beside a rendering of that module with each stated limit stripped out, which the same check reports; - * "no tie was fixed" is resolved through the shared story range in - `tests/conftest.py` and sits beside a synthetic history whose run commit - edits one of the three files, which the same comparison reports. + * "the repaired files carry no mention at all" sits beside each of the + three historical ties replanted in a throwaway copy of the file it used + to sit in, which the same scan reports and which turns the same + two-directional list comparison red. Nothing here invokes a model, and nothing here writes to this repository. """ @@ -58,22 +69,30 @@ import pytest import harness_source -from conftest import story_diff -from test_shared_baseline_resolution import committed_story REPO_ROOT = Path(harness_source.__file__).resolve().parents[1] DECLARING_MODULE = "orchestration/harness_source.py" -VALIDATION_REL = "tests/test_no_target_stack_in_harness_source.py" -#: The files whose ties are still grandfathered below, and which no story -#: since has edited. `orchestration/story_coordinator.py` was among them -#: until the-interpreter-is-not-assumed-to-be-python repaired its ties, which -#: is what taking it off this tuple records. -UNTOUCHED = ( +#: The files the burn-down repaired last, and which the scan must now report +#: nothing at all in. `orchestration/story_coordinator.py` was repaired ahead +#: of them by the-interpreter-is-not-assumed-to-be-python, and is asserted +#: separately below because one permanent mention legitimately survives there. +REPAIRED_FILES = ( "workflows/story-workflow.json", "prompts/tester.md", ) +#: The exact lines that used to sit in those files, kept verbatim so each can +#: be replanted. This is what makes "the scan reports nothing there" an +#: assertion rather than a claim about where the scan happens to be looking: +#: put the tie back and the same scan reports it. +HISTORICAL_TIES = ( + ("workflows/story-workflow.json", ' "may_not_create": ["tests/"],'), + ("prompts/tester.md", + "New tests belong in tests/ and become permanent repository assets."), + ("prompts/tester.md", "shared resolution in `tests/conftest.py`."), +) + # ========================================================================== # The two lists @@ -94,24 +113,28 @@ # ========================================================================== -TEMPORARY_TIES: frozenset[tuple[str, str]] = frozenset({ - # --- Two lines of prose in a prompt naming a pytest layout. --------- - ('prompts/tester.md', - 'New tests belong in tests/ and become permanent repository assets.'), - ('prompts/tester.md', - 'shared resolution in `tests/conftest.py`.'), - - # --- The workflow restriction naming a directory in the target. ----- - ('workflows/story-workflow.json', - ' "may_not_create": ["tests/"],'), -}) - -#: What the-interpreter-is-not-assumed-to-be-python removed from the list -#: above: every `orchestration/story_coordinator.py` entry — the version -#: probe, the record's interpreter-shaped fields, and the retired -#: configuration key with the prose explaining it — together with every -#: entry in the three schemas that story names. What is left is the -#: completion signal for the-test-location-comes-from-configuration alone. +TEMPORARY_TIES: frozenset[tuple[str, str]] = frozenset() + +#: Empty, and that is the completion signal for the whole burn-down. +#: the-interpreter-is-not-assumed-to-be-python removed every +#: `orchestration/story_coordinator.py` entry — the version probe, the +#: record's interpreter-shaped fields, and the retired configuration key with +#: the prose explaining it — together with every entry in the three schemas it +#: names. the-test-location-comes-from-configuration removed the last three: +#: the workflow's create restriction, which is now the token `{{tests_dir}}` +#: resolved from configuration when the definition loads, and the two lines of +#: prose in the tester prompt, which now render the configured location and +#: describe the shared baseline resolution without naming the file this +#: repository keeps it in. +#: +#: An empty list is exactly the shape an assertion goes vacuous against, so no +#: assertion below rests on reading it. `list_problems` and +#: `classification_problems` both take it as one of two lists and would report +#: an entry landing on neither, which is how a new tie surfaces; what is +#: asserted about this repository is that the scan reports nothing outside +#: `PERMANENT_MENTIONS`, and every one of those assertions carries a control +#: that replants a historical tie, or hands the same check an entry it must +#: object to, and observes it reported. #: Each entry carries a one-line reason saying why that mention is not a @@ -145,22 +168,6 @@ } -#: The ties the 2026-08-15 audit found that are still grandfathered, each -#: identified by the file it sits in and a fragment of the line, so the scan -#: is asked for them by name rather than being read off the list above. The -#: audit's other two — the version probe and the retired configuration key, -#: both in `orchestration/story_coordinator.py` — are gone from the source, -#: so asking the scan for them by name would now be asking it for something -#: that is not there. -AUDITED_TIES = ( - ("the may_not_create restriction naming a directory", - "workflows/story-workflow.json", '"may_not_create": ["tests/"]', 9), - ("prompts/tester.md line 19", - "prompts/tester.md", "New tests belong in tests/", 19), - ("prompts/tester.md line 47", - "prompts/tester.md", "tests/conftest.py", 47), -) - #: The mentions that are honest sentences rather than ties. A rule that #: cannot tell these from a tie gets turned off within two stories. LEGITIMATE_MENTIONS = ( @@ -209,6 +216,33 @@ def list_problems(findings) -> list[str]: return problems +def classification_problems(findings, temporary=None, + permanent=None) -> list[str]: + """Every reported entry the two lists do not classify exactly once. + + Exclusivity and coverage are one question -- the lists partition what the + scan reports -- and asking it this way keeps it answerable while + `TEMPORARY_TIES` is empty: an entry on neither list is a new tie, which is + a failure this repository can still produce. The lists are parameters so a + control can hand the same check an entry sitting on both. + """ + temporary = set(TEMPORARY_TIES if temporary is None else temporary) + permanent = set(PERMANENT_MENTIONS if permanent is None else permanent) + problems = [] + for entry in sorted(reported_entries(findings)): + on = [name for name, listed in (("temporary", temporary), + ("permanent", permanent)) + if entry in listed] + if len(on) == 1: + continue + where = " and ".join(on) if on else "neither list" + problems.append( + f"{entry[0]} is on {where}, so its classification says nothing " + f"-- {entry[1].strip()!r}" + ) + return problems + + def reasonless(mentions: dict) -> list[tuple[str, str]]: """Every permanent entry whose reason is missing or not one line.""" return sorted(entry for entry, reason in mentions.items() @@ -284,31 +318,46 @@ def test_the_throwaway_root_reports_what_this_repository_does(throwaway, here): # ========================================================================== -# The five ties that motivated the story +# The five ties that motivated the story, all now repaired # ========================================================================== -@pytest.mark.parametrize("label,path,fragment,line_number", - AUDITED_TIES, ids=[t[0] for t in AUDITED_TIES]) -def test_the_scan_reports_each_audited_tie(here, label, path, fragment, - line_number): - """Asked of the scan by name rather than read off the list. A scan that - cannot see the ties that motivated it has not been shown to work.""" - matches = findings_for(here, path, fragment) - assert matches, f"{label}: nothing reported in {path} matching {fragment!r}" - if line_number is not None: - assert line_number in {f.line_number for f in matches}, ( - label, sorted(f.line_number for f in matches)) +def test_the_burn_down_is_complete(here): + """The completion signal, stated as a fact about the scan rather than + about the list: nothing this repository's harness source says is a tie.""" + assert TEMPORARY_TIES == frozenset() + assert reported_entries(here) <= set(PERMANENT_MENTIONS) -@pytest.mark.parametrize("label,path,fragment,line_number", - AUDITED_TIES, ids=[t[0] for t in AUDITED_TIES]) -def test_each_audited_tie_is_a_temporary_tie(here, label, path, fragment, - line_number): - for finding in findings_for(here, path, fragment): - entry = (finding.path, finding.line) - assert entry in TEMPORARY_TIES, (label, entry) - assert entry not in PERMANENT_MENTIONS, (label, entry) +@pytest.mark.parametrize("repaired", REPAIRED_FILES) +def test_the_repaired_files_carry_no_mention_at_all(here, repaired): + """Read off the scan rather than off the empty list: the workflow no + longer names a directory in the target and the tester prompt no longer + names a layout or a test-framework filename.""" + assert [f for f in here if f.path == repaired] == [] + + +@pytest.mark.parametrize("path,tie", HISTORICAL_TIES, + ids=[f"{p}:{t[:28]}" for p, t in HISTORICAL_TIES]) +def test_replanting_a_historical_tie_is_reported_by_the_same_scan(throwaway, + path, tie): + """The control for the two assertions above, which are both absences. + + Each historical tie is put back verbatim into a throwaway copy of the file + it used to sit in, and the same scan has to report it and the same + two-directional comparison has to call it unexpected. Without this, "the + scan reports nothing there" would be satisfied just as happily by a scan + that had stopped reading the file at all. + """ + assert not [f for f in harness_source.scan(throwaway) if f.path == path], \ + f"{path} still carries a mention, so the replant proves nothing" + + append(throwaway, path, "\n" + tie + "\n") + + assert [f for f in harness_source.scan(throwaway) if f.path == path] + problems = list_problems(harness_source.scan(throwaway)) + assert len(problems) == 1, problems + assert problems[0].startswith(f"unexpected: {path}:") #: The three schemas the-interpreter-is-not-assumed-to-be-python names. @@ -328,14 +377,37 @@ def test_the_repaired_schemas_carry_no_mention_at_all(here, schema): assert [f for f in here if f.path == schema] == [] +COORDINATOR = "orchestration/story_coordinator.py" + +#: One of the ties the-interpreter-is-not-assumed-to-be-python removed from +#: the coordinator, kept verbatim so the absence below can be shown to fail. +HISTORICAL_COORDINATOR_TIE = " version = platform.python_version()" + + def test_the_coordinator_carries_no_temporary_tie(here): """Every grandfathered tie in the coordinator is repaired. What the scan still reports there is the one permanent mention describing this harness's own implementation language, which is the opposite of a tie.""" - reported = {(f.path, f.line) for f in here - if f.path == "orchestration/story_coordinator.py"} - assert reported & set(TEMPORARY_TIES) == set() - assert reported <= set(PERMANENT_MENTIONS) + coordinator = [f for f in here if f.path == COORDINATOR] + assert coordinator, \ + f"the scan reports nothing at all in {COORDINATOR}, so it is not " \ + "looking there and this proves nothing" + assert not classification_problems(coordinator) + assert reported_entries(coordinator) <= set(PERMANENT_MENTIONS) + + +def test_a_tie_replanted_in_the_coordinator_is_reported_as_one(throwaway): + """The control for the absence above: the version probe put back verbatim + into a throwaway copy, where the same scan reports it, it lands on neither + list, and it is no longer inside `PERMANENT_MENTIONS`.""" + append(throwaway, COORDINATOR, "\n" + HISTORICAL_COORDINATOR_TIE + "\n") + + coordinator = [f for f in harness_source.scan(throwaway) + if f.path == COORDINATOR] + problems = classification_problems(coordinator) + assert len(problems) == 1, problems + assert problems[0].startswith(f"{COORDINATOR} is on neither list") + assert not reported_entries(coordinator) <= set(PERMANENT_MENTIONS) @pytest.mark.parametrize("path,fragment", LEGITIMATE_MENTIONS) @@ -346,10 +418,10 @@ def test_a_legitimate_mention_is_permanent_and_never_a_tie(here, path, language this harness is written in, not any target's.""" matches = findings_for(here, path, fragment) assert matches, (path, fragment) + assert not classification_problems(matches) for finding in matches: entry = (finding.path, finding.line) assert entry in PERMANENT_MENTIONS, entry - assert entry not in TEMPORARY_TIES, entry # ========================================================================== @@ -363,8 +435,37 @@ def test_the_two_lists_are_exactly_what_the_scan_reports(here): assert not list_problems(here), "\n".join(list_problems(here)) -def test_the_two_lists_share_no_entry(): - assert not set(TEMPORARY_TIES) & set(PERMANENT_MENTIONS) +def test_every_reported_entry_is_classified_exactly_once(here): + """What "the two lists share no entry" became once `TEMPORARY_TIES` went + empty: intersecting an empty set is a question with one possible answer, + while partitioning what the scan reports is one this repository can still + fail -- a mention that is a tie and a permanent mention at once, or a new + one that is neither.""" + assert not classification_problems(here), \ + "\n".join(classification_problems(here)) + + +def test_an_entry_on_both_lists_is_reported_by_the_same_check(here): + """The exclusivity half of the control: a mention the scan really reports, + handed to the same check as temporary *and* permanent.""" + both = sorted(reported_entries(here) & set(PERMANENT_MENTIONS))[0] + problems = classification_problems(here, temporary={both}) + assert len(problems) == 1, problems + assert problems[0].startswith(f"{both[0]} is on temporary and permanent") + + +def test_an_entry_on_neither_list_is_reported_by_the_same_check(throwaway): + """The coverage half: a tie planted in a file that is clean today lands on + neither list, and the same check reports it.""" + clean = "prompts/documenter.md" + assert not [f for f in harness_source.scan(throwaway) if f.path == clean], \ + f"{clean} is no longer clean, so it cannot serve as the plant site" + + append(throwaway, clean, "\nThe target is built with gradle.\n") + + problems = classification_problems(harness_source.scan(throwaway)) + assert len(problems) == 1, problems + assert problems[0].startswith(f"{clean} is on neither list") def test_every_permanent_mention_carries_a_reason(): @@ -404,9 +505,8 @@ def test_a_list_entry_left_behind_after_its_tie_is_removed_turns_the_lists_red( ): """The other direction: a file that stops violating must be taken off the list, or the burn-down counts work that is already done.""" - removed = ('workflows/story-workflow.json', - ' "may_not_create": ["tests/"],') - assert removed in TEMPORARY_TIES + removed = ('scripts/l5-status', '#!/usr/bin/env python3') + assert removed in PERMANENT_MENTIONS text = (throwaway / removed[0]).read_text(encoding="utf-8") assert removed[1] + "\n" in text @@ -414,7 +514,7 @@ def test_a_list_entry_left_behind_after_its_tie_is_removed_turns_the_lists_red( problems = list_problems(harness_source.scan(throwaway)) assert len(problems) == 1, problems - assert problems[0].startswith("stale: workflows/story-workflow.json") + assert problems[0].startswith("stale: scripts/l5-status") # ========================================================================== @@ -573,30 +673,10 @@ def test_the_stated_incompleteness_is_true_rather_than_modest(throwaway): # ========================================================================== -# The story fixed nothing, and this run changed nothing +# The declaration is still a declaration: nothing runs it # ========================================================================== -def test_no_tie_was_fixed_by_this_story(): - """The three files carrying the audited ties are untouched on this - story's branch. Resolved through the shared story range in - `tests/conftest.py`, never as HEAD against this repository.""" - assert story_diff(list(UNTOUCHED), - validation_file=Path(__file__)).strip() == "" - - -@pytest.mark.parametrize("guarded", UNTOUCHED) -def test_the_same_comparison_reports_a_story_that_did_edit_one(tmp_path, - guarded): - """The control for the assertion above, over the shape this repository - cannot be in while these tests run: a story already committed, whose own - run commit rewrote the guarded file.""" - root = committed_story(tmp_path, VALIDATION_REL, guarded, violate="modify", - name=f"violating-{Path(guarded).name}") - assert story_diff([guarded], validation_file=root / VALIDATION_REL, - repo=root).strip() != "" - - def readers_of_the_scan(root: Path) -> list[str]: """Every module under `root`'s orchestration/ that names the scan.""" declaring = Path(DECLARING_MODULE).name diff --git a/tests/test_plan_assignment_refusal.py b/tests/test_plan_assignment_refusal.py index 6a82b98..a2788aa 100644 --- a/tests/test_plan_assignment_refusal.py +++ b/tests/test_plan_assignment_refusal.py @@ -86,6 +86,7 @@ import pytest from conftest import load_mutant, load_script +import conftest from test_revert_check import ( # noqa: F401 - fixtures used by name APP_ADDITIVE, @@ -133,7 +134,7 @@ # Everything about the workflow is read off the workflow. # -------------------------------------------------------------------------- -WORKFLOW = harness_config.load_workflow(HARNESS_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(HARNESS_ROOT, "story-workflow") STAGES = WORKFLOW["stages"] STAGE_NAMES = [stage["name"] for stage in STAGES] RESTRICTIONS = story_coordinator.stage_restrictions(STAGES) diff --git a/tests/test_plan_commit.py b/tests/test_plan_commit.py index 2ec44dd..9378388 100644 --- a/tests/test_plan_commit.py +++ b/tests/test_plan_commit.py @@ -61,7 +61,8 @@ import pytest -from conftest import BASELINE, repository_file_at, story_commit_range +from conftest import (BASELINE, repointed_at_todays_signature, + repository_file_at, story_commit_range) HARNESS_ROOT = Path(__file__).resolve().parents[1] L5_PLAN = HARNESS_ROOT / "scripts" / "l5-plan" @@ -86,6 +87,7 @@ architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: echo tests-ok +tests_dir: tests/ """ ARTIFACT = """\ @@ -669,7 +671,7 @@ def pre_story_script(tmp_path: Path) -> Path: the old script loads the same config, workflow, rules and template the new one does without anything being written into the repository. """ - source = pre_story_text("scripts/l5-plan") + source = repointed_at_todays_signature(pre_story_text("scripts/l5-plan")) root = tmp_path / "pre-story-harness" (root / "scripts").mkdir(parents=True) for name in ("orchestration", "prompts", "schemas", "workflows", "rules"): diff --git a/tests/test_plan_time_validation.py b/tests/test_plan_time_validation.py index 0ac398c..ebd4dbc 100644 --- a/tests/test_plan_time_validation.py +++ b/tests/test_plan_time_validation.py @@ -59,6 +59,7 @@ from conftest import (BASELINE, NothingToCompareAgainst, load_script, repository_file_at, story_commit_range) +import conftest from test_plan_commit import ( ARTIFACT, Planning, @@ -91,7 +92,7 @@ # below agree with a copy of the workflow rather than with the workflow. # -------------------------------------------------------------------------- -WORKFLOW = harness_config.load_workflow(HARNESS_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(HARNESS_ROOT, "story-workflow") STAGES = WORKFLOW["stages"] STAGE_NAMES = [stage["name"] for stage in STAGES] RESTRICTIONS = story_coordinator.stage_restrictions(STAGES) @@ -219,10 +220,14 @@ def pre_story_harness(tmp_path: Path) -> Path: continue os.symlink(module, root / "orchestration" / module.name) (root / "orchestration" / "story_coordinator.py").write_text( - show("orchestration/story_coordinator.py"), encoding="utf-8") + conftest.repointed_at_todays_signature( + show("orchestration/story_coordinator.py")), + encoding="utf-8") for script in ("l5-plan", "l5-run"): written = root / "scripts" / script - written.write_text(show(f"scripts/{script}"), encoding="utf-8") + written.write_text( + conftest.repointed_at_todays_signature(show(f"scripts/{script}")), + encoding="utf-8") written.chmod(0o755) return root diff --git a/tests/test_planner_injection.py b/tests/test_planner_injection.py index b9862a4..3ad07ca 100644 --- a/tests/test_planner_injection.py +++ b/tests/test_planner_injection.py @@ -41,6 +41,7 @@ import pytest from conftest import story_diff +import conftest import context_assembler import harness_config @@ -85,7 +86,7 @@ #: The loaded workflow build_context has taken as a required argument #: since story-028, which injects the workflow's own facts — its stages, #: its create restrictions, its retry routes — into every stage prompt. -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(REPO_ROOT, "story-workflow") def planner_template() -> str: @@ -251,7 +252,10 @@ def captured_plan_argv(tmp_path: Path) -> list[str]: refusal path is covered by story-009's half of this module.""" (tmp_path / ".harness").mkdir() (tmp_path / ".harness" / "config.yaml").write_text( - "workflow: story-workflow\n", encoding="utf-8" + # tests_dir is what the implementer's create restriction + # resolves to, so a target that declares none has no + # restriction for the planner prompt to carry. + "workflow: story-workflow\ntests_dir: tests/\n", encoding="utf-8" ) bin_dir = tmp_path / "bin" bin_dir.mkdir() @@ -431,7 +435,7 @@ def test_every_committed_story_artifact_still_parses(): def workflow() -> dict: - return harness_config.load_workflow(REPO_ROOT, "story-workflow") + return conftest.shipped_workflow(REPO_ROOT, "story-workflow") def rules() -> dict: @@ -686,7 +690,10 @@ def test_l5_plan_injects_the_workflow_facts_into_the_session_prompt( project = tmp_path / "project" (project / ".harness").mkdir(parents=True) (project / ".harness" / "config.yaml").write_text( - "workflow: story-workflow\n", encoding="utf-8" + # tests_dir is what the implementer's create restriction + # resolves to, so a target that declares none has no + # restriction for the planner prompt to carry. + "workflow: story-workflow\ntests_dir: tests/\n", encoding="utf-8" ) nested = project / "src" / "deep" nested.mkdir(parents=True) diff --git a/tests/test_required_output_freshness.py b/tests/test_required_output_freshness.py index 5f6751b..6a3c285 100644 --- a/tests/test_required_output_freshness.py +++ b/tests/test_required_output_freshness.py @@ -51,11 +51,11 @@ import story_coordinator import test_retry_history as story012 from conftest import first_retry_route +import conftest from agent_runner import AgentResult REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] -WORKFLOW = json.loads( - (REPO_ROOT / "workflows" / "story-workflow.json").read_text(encoding="utf-8")) +WORKFLOW = conftest.shipped_workflow() COORDINATOR_SOURCE = Path(story_coordinator.__file__).read_text(encoding="utf-8") #: Read off the loaded workflow rather than written here, so this file names @@ -143,6 +143,7 @@ def failing(attempt: int) -> dict: architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: echo tests-ok +tests_dir: tests/ """ diff --git a/tests/test_rerun_refusal.py b/tests/test_rerun_refusal.py index f602454..5deea0c 100644 --- a/tests/test_rerun_refusal.py +++ b/tests/test_rerun_refusal.py @@ -133,6 +133,7 @@ architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: echo tests-ok +tests_dir: tests/ """ GITIGNORE = ".harness/runs/\n.harness/logs/\n" diff --git a/tests/test_resume_guard.py b/tests/test_resume_guard.py index 470442e..0ec4c5a 100644 --- a/tests/test_resume_guard.py +++ b/tests/test_resume_guard.py @@ -54,6 +54,7 @@ from conftest import (BASELINE as BASELINE_BOUND, ENDPOINT, function_source, function_source_at, load_mutant) +import conftest import harness_config import story_coordinator @@ -62,7 +63,7 @@ REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] COORDINATOR_REL = "orchestration/story_coordinator.py" COORDINATOR_PATH = REPO_ROOT / COORDINATOR_REL -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(REPO_ROOT, "story-workflow") VERIFIER_STAGE = next(s for s in WORKFLOW["stages"] if "on_failure" in s) STORY_ID = "story-001" @@ -125,6 +126,7 @@ architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: echo tests-ok +tests_dir: tests/ """ APP_AT_HEAD = "print('hello')\n" diff --git a/tests/test_retry_history.py b/tests/test_retry_history.py index 9b347c0..d798552 100644 --- a/tests/test_retry_history.py +++ b/tests/test_retry_history.py @@ -29,6 +29,7 @@ import pytest from conftest import first_retry_route, load_mutant, story_diff +import conftest import context_assembler import run_status @@ -37,8 +38,7 @@ from agent_runner import AgentResult REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] -WORKFLOW = json.loads( - (REPO_ROOT / "workflows" / "story-workflow.json").read_text(encoding="utf-8")) +WORKFLOW = conftest.shipped_workflow() #: Read off the loaded workflow rather than written here, so this file names #: no stage the definition does not. diff --git a/tests/test_retry_routing.py b/tests/test_retry_routing.py index 84ff0d1..3e32c25 100644 --- a/tests/test_retry_routing.py +++ b/tests/test_retry_routing.py @@ -64,12 +64,13 @@ import story_coordinator from agent_runner import AgentResult from conftest import load_mutant +import conftest REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] COORDINATOR_PATH = REPO_ROOT / "orchestration" / "story_coordinator.py" ASSEMBLER_PATH = REPO_ROOT / "orchestration" / "context_assembler.py" -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(REPO_ROOT, "story-workflow") STAGE_NAMES = [stage["name"] for stage in WORKFLOW["stages"]] #: The stage that declares the routing table, found by the declaration rather @@ -170,6 +171,7 @@ def failing(target=OMITTED, *, retry: bool = True) -> dict: architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: {test_command} +tests_dir: tests/ """ diff --git a/tests/test_revert_baseline.py b/tests/test_revert_baseline.py index dd8e6bc..6747a4d 100644 --- a/tests/test_revert_baseline.py +++ b/tests/test_revert_baseline.py @@ -48,6 +48,7 @@ from conftest import (BASELINE as BASELINE_BOUND, STORY, first_retry_route, repository_file_at, story_commit_range, story_diff) +import conftest import harness_config import schema_validator @@ -57,7 +58,7 @@ REPO_ROOT = Path(__file__).resolve().parents[1] ORCHESTRATION = REPO_ROOT / "orchestration" -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_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 @@ -102,6 +103,7 @@ architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: {TEST_COMMAND} +tests_dir: tests/ """ # -------------------------------------------------------------------------- @@ -454,7 +456,7 @@ def mirror_harness(tmp_path: Path, workflow: dict) -> Path: def loaded_workflow() -> dict: - return harness_config.load_workflow(REPO_ROOT, "story-workflow") + return conftest.shipped_workflow(REPO_ROOT, "story-workflow") def executable_source(text: str) -> str: diff --git a/tests/test_revert_check.py b/tests/test_revert_check.py index d0a3b38..96c5515 100644 --- a/tests/test_revert_check.py +++ b/tests/test_revert_check.py @@ -43,6 +43,7 @@ import pytest from conftest import STORY, story_diff +import conftest import context_assembler import harness_config @@ -55,7 +56,7 @@ STORIES_DIR = REPO_ROOT / ".harness" / "stories" TESTS_DIR = REPO_ROOT / "tests" -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(REPO_ROOT, "story-workflow") IMPLEMENTER_STAGE = next(s for s in WORKFLOW["stages"] if s["name"] == "implementer") #: The artifact name and the governed prefix are read off the workflow, never #: spelled here, for the same reason the coordinator may not spell them. Since @@ -86,6 +87,7 @@ architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: {TEST_COMMAND} +tests_dir: tests/ """ # -------------------------------------------------------------------------- @@ -374,7 +376,7 @@ def mirror_harness(tmp_path: Path, workflow: dict) -> Path: def loaded_workflow() -> dict: - return harness_config.load_workflow(REPO_ROOT, "story-workflow") + return conftest.shipped_workflow(REPO_ROOT, "story-workflow") def append_to_story(target_root: Path, text: str) -> None: diff --git a/tests/test_self_routing_retry.py b/tests/test_self_routing_retry.py index 2324d7d..cca59c4 100644 --- a/tests/test_self_routing_retry.py +++ b/tests/test_self_routing_retry.py @@ -68,6 +68,7 @@ import pytest from conftest import BASELINE, load_mutant, repository_file_at +import conftest # The suite target — a real module under a real pytest suite — and the helpers # that drive edits into it are story-017's. Reused rather than copied so a @@ -104,7 +105,7 @@ # Everything about the workflow is read off the workflow # -------------------------------------------------------------------------- -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(REPO_ROOT, "story-workflow") STAGES = WORKFLOW["stages"] STAGE_NAMES = [stage["name"] for stage in STAGES] @@ -263,6 +264,7 @@ def test_the_no_model_guard_fires_when_a_model_is_invoked(tmp_path): architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: {test_command} +tests_dir: tests/ """ @@ -761,7 +763,7 @@ def test_a_consecutive_failure_past_the_budget_escalates( # The same run, with the budget removed from that same stage. harness = probe_harness(Path(subject).parent, f"nobudget-{failure}", without_budget(BUDGETED)) - workflow = harness_config.load_workflow(harness, f"nobudget-{failure}") + workflow = conftest.shipped_workflow(harness, f"nobudget-{failure}") control = build_target(Path(subject).parent / f"control-{failure}", workflow=workflow["name"]) spec = build(BUDGETED, BUDGET + 1) @@ -973,7 +975,7 @@ def test_a_stage_that_self_routed_does_not_spend_another_stages_budget( """ other = BUDGETLESS[0] harness = probe_harness(tmp_path, "two-budgets", with_budget(other, 1)) - workflow = harness_config.load_workflow(harness, "two-budgets") + workflow = conftest.shipped_workflow(harness, "two-budgets") target_root = build_target(tmp_path / "two-budget-target", workflow="two-budgets") @@ -1466,7 +1468,7 @@ def test_a_boundary_violation_at_a_budgeted_stage_still_escalates( harness = probe_harness(tmp_path, f"nobudget-{name}", without_budget(BUDGETED)) - workflow = harness_config.load_workflow(harness, f"nobudget-{name}") + workflow = conftest.shipped_workflow(harness, f"nobudget-{name}") control = build_target(tmp_path / f"boundary-control-{name}", workflow=workflow["name"]) assert drive(control, harness, hooks={BUDGETED: [hook]}, @@ -1486,7 +1488,7 @@ def budgeted_clean_clone(tmp_path): name = "budgeted-clean-clone" harness = probe_harness(tmp_path, name, with_budget(CLEAN_CLONE_STAGE["name"], 1)) - return harness, harness_config.load_workflow(harness, name) + return harness, conftest.shipped_workflow(harness, name) def test_a_clean_clone_that_cannot_run_still_escalates_at_a_budgeted_stage( @@ -1604,7 +1606,7 @@ def test_the_same_run_under_a_sound_budget_creates_all_of_it(tmp_path): target, same runner, with a budget the check accepts.""" harness = probe_harness(tmp_path, "sound-budget", with_budget(BUDGETED, 0)) target_root = build_target(tmp_path / "target-sound", workflow="sound-budget") - workflow = harness_config.load_workflow(harness, "sound-budget") + workflow = conftest.shipped_workflow(harness, "sound-budget") runner = Runner(target_root, workflow=workflow) assert story_coordinator.run_story( @@ -1624,7 +1626,7 @@ def test_a_declared_budget_of_zero_escalates_like_declaring_nothing(tmp_path): """Zero is a deliberate declaration of no budget: accepted at pre-flight, and spent nowhere.""" harness = probe_harness(tmp_path, "zero-budget", with_budget(BUDGETED, 0)) - workflow = harness_config.load_workflow(harness, "zero-budget") + workflow = conftest.shipped_workflow(harness, "zero-budget") target_root = build_target(tmp_path / "target-zero", workflow="zero-budget") code, runner = drive(target_root, harness, {BUDGETED: [CRASH]}, @@ -1649,7 +1651,7 @@ def test_the_pre_flight_is_what_refuses_the_bad_budget(tmp_path): harness = probe_harness(tmp_path, "unchecked-budget", with_budget(BUDGETED, -1)) - workflow = harness_config.load_workflow(harness, "unchecked-budget") + workflow = conftest.shipped_workflow(harness, "unchecked-budget") target_root = build_target(tmp_path / "target-unchecked", workflow="unchecked-budget") runner = Runner(target_root, workflow=workflow) diff --git a/tests/test_shared_baseline_resolution.py b/tests/test_shared_baseline_resolution.py index 7b72746..4a133e0 100644 --- a/tests/test_shared_baseline_resolution.py +++ b/tests/test_shared_baseline_resolution.py @@ -146,7 +146,7 @@ def expected_test_names(current_rel: str) -> set[str]: #: The loaded workflow build_context has taken as a required argument #: since story-028, which injects the workflow's own facts — its stages, #: its create restrictions, its retry routes — into every stage prompt. -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(REPO_ROOT, "story-workflow") def git(root: Path, *args: str) -> str: @@ -830,10 +830,15 @@ def test_a_well_written_absence_assertion_survives_the_check(): # -------------------------------------------------------------------------- +#: The third phrase used to be the path this repository keeps the shared +#: resolution at. The prompt ships to any target, so it now names the thing +#: rather than the file, and this reads the sentence that survived: the +#: instruction is still to use the shared resolution rather than to write a +#: second one, which is the whole of what the guidance was for. TESTER_GUIDANCE = [ "An assertion that claims an absence needs a negative control", "demonstrate that it can fail", - "tests/conftest.py", + "shared baseline resolution", ] VERIFIER_GUIDANCE = [ "absence", diff --git a/tests/test_single_story_reader.py b/tests/test_single_story_reader.py index 32fe68e..27995b5 100644 --- a/tests/test_single_story_reader.py +++ b/tests/test_single_story_reader.py @@ -22,6 +22,7 @@ import story_parser from agent_runner import AgentResult from conftest import commit_setup +import conftest REPO_ROOT = Path(__file__).resolve().parents[1] CORPUS = REPO_ROOT.joinpath(".harness", "stories") @@ -73,7 +74,7 @@ #: The loaded workflow build_context has taken as a required argument #: since story-028, which injects the workflow's own facts — its stages, #: its create restrictions, its retry routes — into every stage prompt. -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(REPO_ROOT, "story-workflow") def parse(story_text: str) -> dict: diff --git a/tests/test_stage_baseline.py b/tests/test_stage_baseline.py index 7e2b09f..44087a7 100644 --- a/tests/test_stage_baseline.py +++ b/tests/test_stage_baseline.py @@ -61,6 +61,7 @@ from conftest import (BASELINE as PRE_STORY_BOUND, ENDPOINT, STORY, first_retry_route, function_source_at, load_mutant, story_diff) +import conftest import harness_config import schema_validator @@ -72,7 +73,7 @@ COORDINATOR_REL = "orchestration/story_coordinator.py" COORDINATOR_PATH = REPO_ROOT / COORDINATOR_REL -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_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 @@ -113,6 +114,7 @@ architecture_docs: - .harness/docs/ARCHITECTURE.md test_command: {TEST_COMMAND} +tests_dir: tests/ """ # -------------------------------------------------------------------------- diff --git a/tests/test_stage_output_ownership.py b/tests/test_stage_output_ownership.py index da760b2..77ebbb8 100644 --- a/tests/test_stage_output_ownership.py +++ b/tests/test_stage_output_ownership.py @@ -19,6 +19,7 @@ import pytest from conftest import commit_setup, story_diff +import conftest import context_assembler import harness_config @@ -40,7 +41,7 @@ #: The loaded workflow build_context has taken as a required argument #: since story-028, which injects the workflow's own facts — its stages, #: its create restrictions, its retry routes — into every stage prompt. -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(REPO_ROOT, "story-workflow") def write_json(path: Path, payload: dict) -> None: @@ -136,7 +137,7 @@ def exception_block(stage: str, create: str, reason: str = "the deliverable is t def workflow_stages(harness_root: Path) -> list[dict]: - return harness_config.load_workflow(harness_root, "story-workflow")["stages"] + return conftest.shipped_workflow(harness_root, "story-workflow")["stages"] def executable_source(text: str) -> str: @@ -231,7 +232,7 @@ def ownership_only(tmp_path: Path, harness_root: Path) -> Path: ownership does not escalate on a modification or a deletion. The revert check's own behavior on those records is story-017's to demonstrate. """ - workflow = harness_config.load_workflow(harness_root, "story-workflow") + workflow = conftest.shipped_workflow(harness_root, "story-workflow") for stage in workflow["stages"]: stage.pop("revert_check", None) return mirror_harness(tmp_path, harness_root, workflow) @@ -353,7 +354,7 @@ def test_moving_the_declaration_moves_the_enforcement(target_root, harness_root, """The strongest form of "no stage name and no prefix in the code": give the coordinator a workflow it has never seen, declaring a different prefix on a different stage, and the rule follows the declaration.""" - workflow = harness_config.load_workflow(harness_root, "story-workflow") + workflow = conftest.shipped_workflow(harness_root, "story-workflow") for stage in workflow["stages"]: stage.pop("may_not_create", None) if stage["name"] == "tester": @@ -378,7 +379,7 @@ def test_moving_the_declaration_moves_the_enforcement(target_root, harness_root, def test_a_workflow_declaring_nothing_enforces_nothing(target_root, harness_root, tmp_path): - workflow = harness_config.load_workflow(harness_root, "story-workflow") + workflow = conftest.shipped_workflow(harness_root, "story-workflow") for stage in workflow["stages"]: stage.pop("may_not_create", None) fake_root = mirror_harness(tmp_path, harness_root, workflow) @@ -468,7 +469,7 @@ def test_an_exception_does_not_lift_the_rule_for_another_stage(target_root, tmp_path): """The grant is per stage: a workflow restricting two stages and a story granting one leaves the other restricted.""" - workflow = harness_config.load_workflow(harness_root, "story-workflow") + workflow = conftest.shipped_workflow(harness_root, "story-workflow") for stage in workflow["stages"]: if stage["name"] in ("implementer", "tester"): stage["may_not_create"] = ["tests/"] diff --git a/tests/test_stage_tool_grants.py b/tests/test_stage_tool_grants.py index 679e6e9..e1f49a7 100644 --- a/tests/test_stage_tool_grants.py +++ b/tests/test_stage_tool_grants.py @@ -53,6 +53,7 @@ from conftest import (BASELINE, HARNESS_ROOT, load_mutant, repository_file_at) +import conftest import agent_runner import context_assembler @@ -69,7 +70,7 @@ TEMPLATE_CONFIG_REL = "templates/config.yaml" HARNESS_LAYER_REL = "prompts/harness-layer.md" -WORKFLOW = harness_config.load_workflow(HARNESS_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(HARNESS_ROOT, "story-workflow") # --------------------------------------------------------------------------- @@ -704,18 +705,20 @@ def test_config_context_maps_allowed_tools_through_the_shared_helper(): """AC12: dash-prefixed lines, and rendered by _dashed_lines rather than by a second copy of it — shown by replacing the helper and seeing the result change, which no independent formatting could do.""" - assert context_assembler.config_context({"allowed_tools": GRANTS}) == { - "allowed_tools": "- Bash(grep:*)\n- Bash(git show:*)"} - assert context_assembler.config_context({"allowed_tools": GRANTS}) == { - "allowed_tools": context_assembler._dashed_lines(GRANTS)} + # Keyed on the grants alone rather than on the whole mapping: + # config_context maps every configured fact a prompt renders, so another + # key joining it must not read as this one having changed. + rendered = context_assembler.config_context({"allowed_tools": GRANTS}) + assert rendered["allowed_tools"] == "- Bash(grep:*)\n- Bash(git show:*)" + assert rendered["allowed_tools"] == context_assembler._dashed_lines(GRANTS) def test_config_context_renders_none_when_the_config_declares_no_grants(): """AC12's absence half, controlled by the populated case above: the same call with grants renders them, so None is about the config.""" - assert context_assembler.config_context({}) == {"allowed_tools": None} - assert context_assembler.config_context({"allowed_tools": []}) == { - "allowed_tools": None} + assert context_assembler.config_context({})["allowed_tools"] is None + assert context_assembler.config_context( + {"allowed_tools": []})["allowed_tools"] is None assert context_assembler.config_context( {"allowed_tools": GRANTS})["allowed_tools"] is not None @@ -725,7 +728,7 @@ def test_config_context_uses_the_shared_dashed_lines_helper(monkeypatch): monkeypatch.setattr(context_assembler, "_dashed_lines", lambda items: "SENTINEL") assert context_assembler.config_context( - {"allowed_tools": GRANTS}) == {"allowed_tools": "SENTINEL"} + {"allowed_tools": GRANTS})["allowed_tools"] == "SENTINEL" def _context(target_root: Path, **extra) -> dict: @@ -769,7 +772,8 @@ def test_omitting_the_argument_renders_what_it_rendered_before_this_story( history, so it stays honest when the story commits.""" without_the_merge = load_mutant( HARNESS_ROOT / "orchestration" / "context_assembler.py", - [(" context.update(config_context({\"allowed_tools\": allowed_tools}))", + [(" context.update(config_context({**config, \"allowed_tools\": " + "allowed_tools}))", " pass # the merge this story added, removed")], name="context_assembler_before_story_035", tmp_path=tmp_path) @@ -777,8 +781,11 @@ def test_omitting_the_argument_renders_what_it_rendered_before_this_story( before = without_the_merge.build_context(**_context(target_root)) assert today["allowed_tools"] is None + # Every key the merge contributes is excluded, not the grants alone: the + # merge is one call, so removing it removes all of them at once. + merged = set(context_assembler.config_context({})) assert {key: value for key, value in today.items() - if key != "allowed_tools"} == before + if key not in merged} == before # The control: supplying the argument *does* change the render, so the # equality above is a statement about omission rather than about the merge diff --git a/tests/test_story_coordinator.py b/tests/test_story_coordinator.py index 13a67c9..64528a9 100644 --- a/tests/test_story_coordinator.py +++ b/tests/test_story_coordinator.py @@ -8,6 +8,7 @@ import story_coordinator from agent_runner import AgentResult from conftest import first_retry_route +import conftest REPO_ROOT = Path(story_coordinator.__file__).resolve().parents[1] @@ -17,7 +18,7 @@ #: escalates rather than routing it, so every failing verdict below #: carries one. RETRY_CATEGORY, RETRY_STAGE = first_retry_route( - harness_config.load_workflow(REPO_ROOT, "story-workflow")) + conftest.shipped_workflow(REPO_ROOT, "story-workflow")) def write_json(path: Path, payload: dict) -> None: diff --git a/tests/test_undeclared_config_keys.py b/tests/test_undeclared_config_keys.py index d73f37b..932dcd4 100644 --- a/tests/test_undeclared_config_keys.py +++ b/tests/test_undeclared_config_keys.py @@ -261,17 +261,20 @@ def test_every_problem_names_the_offending_key_and_lists_the_declared_set(): assert name in problem, (name, problem) -def test_the_declared_set_is_the_thirteen_the_schema_carries_and_no_more(): - """The story's constraint that this change adds no key and removes none. +def test_the_declared_set_is_what_the_schema_carries_and_no_more(): + """This story's constraint was that it added no key and removed none; a + later story adding one is a change to the schema, not to this refusal. Read out of the schema file itself rather than out of the function that reads it, so the two are compared rather than one restating the other. + The count is not written here: `tests/test_config_keys_are_obeyed.py` + owns the declared set and holds it to a proof per key, and a second copy + of the number here would only ever go red for that story's reason. """ schema = json.loads( (REPO_ROOT / "schemas" / "harness-config.schema.json").read_text( encoding="utf-8")) assert tuple(schema["properties"]) == harness_config.declared_config_keys() - assert len(schema["properties"]) == 13, sorted(schema["properties"]) assert RETIRED not in schema["properties"] assert "project" not in schema["properties"] diff --git a/tests/test_validation_module_naming.py b/tests/test_validation_module_naming.py index 8e463fe..d21eb4b 100644 --- a/tests/test_validation_module_naming.py +++ b/tests/test_validation_module_naming.py @@ -75,7 +75,7 @@ import plan_validation # noqa: E402 import story_coordinator # noqa: E402 -WORKFLOW = harness_config.load_workflow(REPO_ROOT, "story-workflow") +WORKFLOW = conftest.shipped_workflow(REPO_ROOT, "story-workflow") STAGES = WORKFLOW["stages"] #: The committed corpus. Reached with `joinpath` rather than the `/` operator diff --git a/workflows/story-workflow.json b/workflows/story-workflow.json index fa3ef32..4942b2f 100644 --- a/workflows/story-workflow.json +++ b/workflows/story-workflow.json @@ -6,7 +6,7 @@ "prompt": "implementer.md", "outputs": ["changed-files.json", "implementation-summary.md"], "changed_files": "changed-files.json", - "may_not_create": ["tests/"], + "may_not_create": ["{{tests_dir}}"], "max_self_routes": 1, "revert_check": { "result": "revert-check-result.json",