diff --git a/.harness/docs/ARCHITECTURE.md b/.harness/docs/ARCHITECTURE.md index 3bb1cca..f8aa3d0 100644 --- a/.harness/docs/ARCHITECTURE.md +++ b/.harness/docs/ARCHITECTURE.md @@ -30,9 +30,9 @@ A stage that writes to the repository declares an optional `changed_files` key n 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. Since story-106 the declaration carries an optional third name, `discarded`, the run-directory directory a refused stage's own version of each reverted path is kept under; the implementer names `discarded-edits` and the tester `tester-discarded-edits`. The enforced set is no longer the stage's `may_not_create` prefixes read at the call site but whatever that stage's declared restrictions govern, and a refusal no longer stops the run. See "A stage may be confined to its lane, and a refused edit is undone" below. +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. Since story-106 the declaration carries an optional third name, `discarded`, the run-directory directory a refused stage's own version of each reverted path is kept under; the implementer names `discarded-edits`, the tester `tester-discarded-edits` and, since story-154, the documenter `documenter-discarded-edits`, with `documenter-revert-check-result.json` as its result. The enforced set is no longer the stage's `may_not_create` prefixes read at the call site but whatever that stage's declared restrictions govern, and a refusal no longer stops the run. See "A stage may be confined to its lane, and a refused edit is undone" below. -A stage may also declare an optional `may_only_change` key since story-106, the mirror image of `may_not_create`: a list of repository-relative prefixes its repository writes are **confined** to, where a path outside every named prefix is governed and a path at or beneath one of them is not. The tester declares `["{{tests_dir}}"]`, resolved from configuration exactly as the implementer's create restriction is, together with a `revert_check` of its own. No other stage declares it, and `workflows/refactor-workflow.json` declares neither key on any stage. Both keys are read through one derivation — `stage_restrictions` — which returns restrictions carrying their sense rather than bare prefixes, so no reader distinguishes the two. See "A stage may be confined to its lane, and a refused edit is undone" below. +A stage may also declare an optional `may_only_change` key since story-106, the mirror image of `may_not_create`: a list of repository-relative prefixes its repository writes are **confined** to, where a path outside every named prefix is governed and a path at or beneath one of them is not. The tester declares `["{{tests_dir}}"]`, resolved from configuration exactly as the implementer's create restriction is, together with a `revert_check` of its own. Since story-154 the documenter declares it too, as `["{{architecture_docs}}"]` with a `revert_check` of its own, in both shipped workflows; see "The documenter is confined to the configured documents, and a pass is granted what its findings name" below. Both keys are read through one derivation — `stage_restrictions` — which returns restrictions carrying their sense rather than bare prefixes, so no reader distinguishes the two. See "A stage may be confined to its lane, and a refused edit is undone" below. The verifier stage declares an optional `clean_clone` key, an object naming both things that check needs: `result`, the artifact it writes (`clean-clone-result.json`), and `retry_stage`, the stage a clean-clone failure routes to. The key is what turns the check on: the coordinator reads `stage.get("clean_clone")` and does nothing when it is absent, so removing the declaration disables the check with no change to orchestration code, and neither the artifact name nor the destination appears there. It was a bare string until story-028 needed a second name and widened it rather than adding a second key, exactly as `revert_check` was widened in story-019 — one declaration, so the check and its route cannot be switched on independently and disagree. Its route is not one of the verifier's retry categories: nothing *chooses* it, so it carries no `when` and is never rendered into a prompt, but it is held to the same pre-flight as the categories are. It sits on the verifier because the check runs on the verifier's passing verdict, in that stage's branch, before routing. Since story-045 the verifier is the *last* stage, so the tree the check clones already holds the documenter's edits — the declaration, the mechanics and the recorded result are unchanged, and only what is in the tree when it runs changed, as a consequence of the reorder rather than of an edit to the check. @@ -72,7 +72,7 @@ Two of them carry the routing decision, and both additions were properties on ex **Four schemas carry story-050's check, and one of them carries it twice.** `retry-guidance.schema.json`'s `current_focus` became an array of objects requiring `focus` and `satisfied_when`, and `retry-history.schema.json` declares the same shape inline, because this validator subset has no cross-file reference and a guidance recorded in history must validate identically to one at the run root — the duplication is now stated in the history object's own description so the two are edited together rather than drifting. `verification-result.schema.json` gained an optional `guidance_outcomes`, one object per entry of the guidance in force, each requiring the entry echoed verbatim in `guidance` and carrying an optional `unmet`; it is optional because a first verification has no guidance to answer. `self-route-result.schema.json` gained a fourth `failure` value, `defective-retry-guidance`, and its `artifacts` description widened to cover the artifacts a contradiction is *between* rather than only the required outputs a check found missing or stale. Both new optional fields state in their own descriptions why presence is the signal — the same `if`/`then` and `dependentRequired` gap `retry_target` records above — so the rule is where a reader meets the field. -**story-055 adds one optional field, one schema and two enum values.** `verification-result.schema.json` gained an optional `correctable_findings` array, each item requiring `location`, `finding`, `correction` and `category`. Its description states presence-as-signal by naming the same validator-subset limitation `retry_target`, `unfinishable_by_retry` and `guidance_outcomes.unmet` already record, rather than re-deriving it; that a correctable finding is a correction to words alone — a comment, a docstring, a schema description, a document — and never to behaviour, so nothing a pass changes may alter what any test asserts; and that `category` names a retry category the workflow declares, spelled as the `retry_routing` table declares it. `correction-pass.schema.json` is new and is coordinator-written, so it joins `clean-clone-result`, `retry-history`, `revert-check-result`, `self-route-result` and `claim-support-result` on the terms above: no stage's `schemas` map names it, no agent is asked to satisfy it, and the coordinator does not validate its own write at run time. It carries `pass`, `attempt`, `stage`, the `findings` copied verbatim and the coordinator's `statement`. Its finding shape is **declared inline rather than referenced**, and the description says so along with why — this validator subset has no cross-file reference, and a finding recorded here must validate identically to one in `verification-result.schema.json` — which is the treatment `retry-history` already gives the guidance shape it repeats. `execution-history.schema.json`'s `event` enum gained `correction-pass-routed` and `correction-pass-recorded`, the same one-line addition story-051 made for the same reason: a run's history is validated against that schema, so an event kind it does not declare fails, and reusing `note` is the repair story-051 records as wrong. +**story-055 adds one optional field, one schema and two enum values.** `verification-result.schema.json` gained an optional `correctable_findings` array, each item requiring `location`, `finding`, `correction` and `category` — and, since story-154, `path`, the repository-relative file the words are in, whose description says that naming it is what grants the stage the pass enters at that one file for that pass. Its description states presence-as-signal by naming the same validator-subset limitation `retry_target`, `unfinishable_by_retry` and `guidance_outcomes.unmet` already record, rather than re-deriving it; that a correctable finding is a correction to words alone — a comment, a docstring, a schema description, a document — and never to behaviour, so nothing a pass changes may alter what any test asserts; and that `category` names a retry category the workflow declares, spelled as the `retry_routing` table declares it. `correction-pass.schema.json` is new and is coordinator-written, so it joins `clean-clone-result`, `retry-history`, `revert-check-result`, `self-route-result` and `claim-support-result` on the terms above: no stage's `schemas` map names it, no agent is asked to satisfy it, and the coordinator does not validate its own write at run time. It carries `pass`, `attempt`, `stage`, the `findings` copied verbatim and the coordinator's `statement`. Its finding shape is **declared inline rather than referenced**, and the description says so along with why — this validator subset has no cross-file reference, and a finding recorded here must validate identically to one in `verification-result.schema.json` — which is the treatment `retry-history` already gives the guidance shape it repeats. `execution-history.schema.json`'s `event` enum gained `correction-pass-routed` and `correction-pass-recorded`, the same one-line addition story-051 made for the same reason: a run's history is validated against that schema, so an event kind it does not declare fails, and reusing `note` is the repair story-051 records as wrong. `story.schema.json` describes the full story shape — nested story fields, typed arrays, `technical_plan` as a known optional property — and the whole of it is consumed: it both *drives* the parse in `story_parser` and validates the parsed result. It is the one place the ambiguity YAML cannot resolve is resolved: whether `- some criterion: with a colon` is a string or a mapping is decided by `items.type` at the schema node being read, not by the syntax of the line. @@ -854,7 +854,7 @@ A verifier that noticed something real but too small to fail a run recorded it i **A passing verdict may now carry `correctable_findings`, and presence is the signal.** On a passing verdict the coordinator re-enters the workflow at the stage the `correction_pass` declaration names and runs through to verification again. The field is read only on a passing verdict, because a failed verdict already routes and a correction travels in the retry guidance it already writes. -**The entry stage is declared, not derived, since story-067.** It used to be the earliest stage in workflow order among the `retry_routing` destinations the findings' categories named — earliest so that a verdict naming two categories cost one pass rather than two. What that priced was every stage *after* the entry point: story-063's finding was a line wrap and its category was the implementer's; `.harness/stories/story-067.yaml` records what the pass then cost — an implementer invocation and two self-routes without producing the edit's artifacts, while a fully verified story sat blocked on whitespace. A pass now costs the declared stage and the stages after it, whatever category a finding carries. The shipped workflow names the documenter, and that is a fit rather than a compromise: every correction recorded so far — story-047's, story-048's, story-049's and story-063's — was prose, and the documenter is the one prose-writing stage no `may_not_create` declaration governs, so a reflow in `orchestration/story_coordinator.py` is an edit it is permitted and instructed to make. +**The entry stage is declared, not derived, since story-067.** It used to be the earliest stage in workflow order among the `retry_routing` destinations the findings' categories named — earliest so that a verdict naming two categories cost one pass rather than two. What that priced was every stage *after* the entry point: story-063's finding was a line wrap and its category was the implementer's; `.harness/stories/story-067.yaml` records what the pass then cost — an implementer invocation and two self-routes without producing the edit's artifacts, while a fully verified story sat blocked on whitespace. A pass now costs the declared stage and the stages after it, whatever category a finding carries. The shipped workflow names the documenter, and that is a fit rather than a compromise: every correction recorded so far — story-047's, story-048's, story-049's and story-063's — was prose, and the documenter was the one prose-writing stage no `may_not_create` declaration governed, so a reflow in `orchestration/story_coordinator.py` was an edit it was permitted and instructed to make. Since story-154 the documenter is confined to the configured architecture documents, and that reflow lands because the finding's required `path` grants the pass the one file it names — see "The documenter is confined to the configured documents, and a pass is granted what its findings name" below. **It is a retry's shape minus everything a retry spends.** No `retry_count` increment, no `attempts/attempt-N/` archive, no `retry-history.json` entry, no attempt number moving in any rendered prompt filename. The verdict that routed it is unchanged — the archived `verification/iteration-N.json` still records `passed` — and the guidance in force is **cleared** at the routing, for the reason the clean-clone reroute clears it: the passing verdict that routed the pass wrote no guidance, and a stale one read as in force would subject the next verification to a check about an attempt it is not judging. That is the same two-budget separation the self-route keeps, arrived at from the other side: a self-route has no verdict behind it, and a correction pass has a *passing* one. @@ -922,7 +922,7 @@ The added conjunct was one question: `(root / entry_path).exists()` is false. `e ## 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**. The declarations that do, all naming `{{tests_dir}}`: the story implementer's `may_not_create`, the story tester's `may_only_change` since story-106, and the refactor implementer's `suite_census` `paths` since story-070. +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**. The declarations naming `{{tests_dir}}`: the story implementer's `may_not_create`, the story tester's `may_only_change` since story-106, and the refactor implementer's `suite_census` `paths` since story-070. Since story-154 the documenter's `may_only_change` in both shipped workflows names `{{architecture_docs}}`, the second and only other referable key. **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. @@ -930,7 +930,7 @@ Since story-046 a workflow definition may name a value the target's own `.harnes **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. +**What is referable is a mapping of literal reads, and that is the narrowness.** `harness_config.workflow_token_values(config)` returns `tests_dir` and, since story-154, `architecture_docs`, each 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: the narrow token leaves every existing reader of a loaded workflow untouched, and a second key was added by naming it rather than by generalising. `{{branch_prefix}}` in a workflow is refused like any other unanswerable reference, and the refusal lists both referable keys. A token whose configured value is a list — `architecture_docs` is one — splices the list's entries into the declaration in place of the token entry, in configured order, and an empty list resolves the entry out exactly as an unset key does; see "The documenter is confined to the configured documents, and a pass is granted what its findings name" below. **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. @@ -1496,7 +1496,7 @@ The section above made the suite cheaper to run. This one lets one of the three The stage that writes the validation could edit the thing being validated, and nothing watched it do so. story-104's tester modified `prompts/prose-layer.md` — the file that story's own assertions were about — and its verifier recorded in `unverified` that it could not diff the change, because the tester's stage baseline covered the test directory alone. The edit survived only in a gitignored log. story-106 gives the tester the mirror image of the restriction the implementer carries, and changes what a refusal does. -**The confinement is a sense, not a second mechanism.** `may_only_change` names the prefixes a stage's repository writes are confined to; outside them a creation escalates on the ownership check and a modification or deletion is decided by the revert check, exactly as they are inside a `may_not_create` prefix. The tester declares `["{{tests_dir}}"]` and a `revert_check` beside it, and **the revert check is what brings the stage baseline with it** — which is the half that closes story-104's gap, because the tester's baseline then covers the tree it may not freely change. +**The confinement is a sense, not a second mechanism.** `may_only_change` names the prefixes a stage's repository writes are confined to; outside them a creation escalates on the ownership check and a modification or deletion is decided by the revert check, exactly as they are inside a `may_not_create` prefix. The tester declares `["{{tests_dir}}"]` and a `revert_check` beside it — and since story-154 the documenter declares `["{{architecture_docs}}"]` with its own, see the section below — and **the revert check is what brings the stage baseline with it** — which is the half that closes story-104's gap, because the tester's baseline then covers the tree it may not freely change. **One derivation, because there were nearly three.** Plan time read `stage_restrictions()`; the coordinator's enforcement site read `stage.get("may_not_create")` at the call site. Two readings of one rule that could disagree, and a second sense would have made it three. `stage_restrictions` now returns `StageRestriction` values — the stage, the prefix, the sense, the whole prefix list its declaration named, a `governs(path)` predicate and a `wording` property — and every reader goes through it or through the per-stage `restrictions_on(stage)`: the ownership check, `governed_edits`, the baseline capture, `stage_exception_problems`, both plan-time checks, and the planner and tester injection. **No declaration key and no sense conditional is read at the enforcement site.** `_SENSES` is the one place a sense's words are written, so a prompt, a plan-time refusal and a run-time escalation state the same restriction identically; adding a sense is adding a member of that mapping. @@ -1522,6 +1522,26 @@ The stage that writes the validation could edit the thing being validated, and n **The cost, named because a reader will meet it before they meet this paragraph.** The tester's baseline now captures the repository outside its confinement — on this repository that is the whole tree minus `tests/`, copied once per stage entry. That is what the confinement bringing its baseline with it buys, and it is what makes a tester edit outside its lane recoverable at all rather than readable only from a gitignored log. +## The documenter is confined to the configured documents, and a pass is granted what its findings name + +The documenter could write implementation code and the run recorded nothing about it. It sits after the tester, so its edits are not covered by the suite the coordinator ran, and before the verifier, so what it wrote is in the tree the verifier judges without being attributable to the stage that should have written it. story-154's brief cites story-148, whose documenter record named an orchestration module beside the architecture document on one attempt and a test module on the next; neither appeared in `events.log` or the completion report, and the verifier met them only as a changed-files list its prompt called "documentation files". Nothing governed it because nothing was declared: the documenter carried neither `may_only_change` nor `revert_check`, so there was no path on which the coordinator asked what it wrote. + +**Since story-154 the documenter carries the declaration and the check the tester carries, in both shipped workflows.** The stage declares `"may_only_change": ["{{architecture_docs}}"]` and a `revert_check` naming `documenter-revert-check-result.json` as its result, `stage-baseline` as its baseline directory and `documenter-discarded-edits` as its discarded directory — keyed exactly as the tester's, so `archivable_artifacts`, the archived check results and the stage baseline pick the artifact up with no code change. The shape is a confinement because the documenter's subject is a set of documents rather than a directory. Everything the section above says of the tester holds of the documenter from the declaration alone: a creation outside the configured documents escalates on the ownership check, a modification or deletion outside them is put to the revert check and undone and recorded when the suite is green without it, and a record naming only the configured documents runs no check and writes no artifact. **No stage name, artifact name or repository path was written into the coordinator's enforcement path for it**, and removing the declaration disables the check with no code change, as it does for the tester. + +**The prefix is a workflow token, resolved from `architecture_docs`.** `workflow_token_values` returns `architecture_docs` beside `tests_dir` — still a literal read per key, so the referable set stays two keys and `{{branch_prefix}}` is still refused, with the refusal now listing both. `_resolve_tokens` learned that a list-valued token splices its entries into the declaration in place of the token entry, so a target configuring two documents has a documenter confined to both, in configured order; an unset or empty value resolves the entry out of the list exactly as an unset `tests_dir` does, so a target that configures no architecture documents has a documenter with no confinement and `stage_restrictions` reports none for it. A literal path in the shipped workflow was never an option: a standing test refuses one, and it would be wrong for every other target. See "A workflow declaration may reference configuration" above. + +**A slash-less confinement prefix is one file.** `StageRestriction.governs` used `startswith` for a confinement, which would have read `.harness/docs/ARCHITECTURE.md.bak` as inside a confinement to `.harness/docs/ARCHITECTURE.md`. It now decides per prefix through `_inside`: a prefix ending in a slash is a directory and covers everything beneath it; any other prefix is exactly that file. That is the meaning `grant_covers` already gives a slash-less granted value and the meaning the `:(exclude,top)` pathspec the baseline capture builds already has, so `baseline_pathspecs` needed no change. A confinement to `tests/` still reports every path beneath it as not governed and every path outside it as governed; the create-restriction sense is untouched. + +**The confinement collides with the correction pass, and the collision is settled rather than left to fail quietly.** Since story-063 and story-067 the documenter, entered on a correction pass, corrects words wherever the verifier's finding names them — a docstring in orchestration source included — and `tests/test_correction_pass.py` pins that. The revert check permits an edit outside a stage's subject only when undoing it turns the suite red, and an edit to prose never does, so a documenter governed on the tester's terms alone would have every pass edit outside the architecture documents undone. **The resolution is that the verifier grants the pass its scope.** A correctable finding gained a required `path` — the repository-relative file its words are in — in `schemas/verification-result.schema.json` and in the inline copy of the finding shape in `schemas/correction-pass.schema.json`, and naming it is the grant. The coordinator already copies findings verbatim into the pass record, so what was granted is recorded by the act that granted it, and nothing parses `location` or any other prose to derive a path. A verifier that omits `path` fails schema validation and self-routes on the bookkeeping cause, so a pass can never quietly lose its grant. + +**The grant is read where the story's grants are read, and bounded by the record.** `correction_pass_grants(run_dir, stages, state, stage_name, attempt)` reads the current pass record off the run directory — the declared artifact keyed by `correction_pass_count`, read back off the file exactly as the prompt injection reads it, so what the checks exempt and what the prompt reports are one thing — and returns the `path` of every finding when the record's `stage` is the stage being checked and its `attempt` is the attempt now running. At the post-stage ownership and revert checks the exempt list is the story's own `stage_exceptions` grants plus those paths; the story grant keeps its meaning and its plan-time cross-check, and the pass grant is added beside it, neither replacing nor widening it. One event of kind `correction-pass-grant-applied` is appended per granted path, worded `correction-pass grant applied: may change ` beside the `stage exception applied` event so the two read alike in `events.log`, and `schemas/execution-history.schema.json`'s closed `event` enum gained the kind — the recurring bill a new event kind pays, reported by the implementer as a deviation since that schema was outside the story's declared scope. **The bounds:** a finding grants one file, never a directory, for one pass; an edit to any file no finding named stays governed on that same pass; a record naming another stage grants nothing; and a documenter re-entered by a later retry in the same run runs at a later attempt, so it inherits nothing from the earlier record. Nothing checks that the edit to a granted file was prose, which this repository already states is not tractable; the trust is the one a story grant extends, bounded by one file and one pass and judged by the verifier that runs again after it. + +**A granted edit does not survive a later attempt's revert of the same file, and that is the baseline rule rather than the grant.** The stage baseline is keyed by stage and first-seen content wins (`stage_baseline_dir` and `capture_stage_baseline`), so the baseline for a file the pass corrected is what the documenter's *first* invocation found — the pre-pass text. A documenter re-entered by a retry that edits that file again, ungranted, has the edit refused and the tree restored to that first-seen content: the retry's words go under `documenter-discarded-edits`, and the pass's words are neither in the tree nor under the discarded directory, while `events.log` still records the grant as applied on the pass. `tests/test_documenter_is_governed.py`'s later-attempt tests assert exactly this and say why. Whether a pass's granted edit ought to survive is a design question this story did not decide — its constraint was that the revert check's dispositions do not change — and the story's verifier recorded it under `unverified` for a brief. + +**The prompts say what the checks do.** `prompts/documenter.md`'s correction paragraph says the files the findings name are the documenter's to change for this pass, that the architecture documents always are, and that an edit to any other file is put to the revert check after the turn and undone if the suite is green without it — so a correction the pass cannot land is reported in `documentation-report.md` rather than attempted elsewhere; its sentence that the approved story artifact is never edited by a pass is kept. Both verifier prompts ask for `path` on each correctable finding and say what naming it does, and their documenter changed-files slot now describes the record as every repository file the documenter touched rather than as documentation files, which was the wording that let story-148's edits pass unremarked. + +**Two consequences follow and are accepted.** A documenter that creates a file outside the configured documents escalates on the ownership check, as a tester creating outside `tests/` does, and a story that needs it declares a `stage_exceptions` grant. And a story-063-shaped pass — a docstring reflow in orchestration source — lands only because the verifier named the file; the story-063 tests in `tests/test_correction_pass.py` now establish that the pass's grant permits the edit rather than that the entered stage carried no restriction, with no assertion weakened. **Deliberately left alone:** #247, which asks that an out-of-confinement edit the revert check finds necessary be routed rather than permitted, and which presumes a check exists to find one — this story is that check, and the routing question is not decided by it; and the claim-support check, which reads what the documenter wrote into a configured document and is unrelated to which files it may touch. + ## A stage records every file it touched A changed-files record is an attestation, and until story-111 nothing checked that it was complete. story-101's tester modified `tests/test_command_transport.py` and left it out of `tester-changed-files.json`; the correction pass asked the *documenter* to add it and the documenter refused, correctly — the record is another stage's output under `.harness/runs/`, a blocked path for every stage, and a documenter editing it would make the tester's account of its own work into something the documenter wrote. The verifier accepted the refusal and recorded the omission under `unverified`. The run ended with a record known to be wrong and structurally unfixable. @@ -2529,6 +2549,10 @@ story-139 moves a sorting a person did after filing into the sync branch of `tem the same record for the tester's own check, since story-106; one artifact per declaring stage, each named by that stage's own declaration + documenter-revert-check-result.json + the same record for the documenter's check, since + story-154; absent when its record named only the + configured architecture documents suite-run-result.json the record of the suite the coordinator ran after the declaring stage's turn ended, coordinator- written; absent when no stage declares a @@ -2618,11 +2642,13 @@ story-139 moves a sorting a person did after filing into the sync branch of `tem dir per stage, merged first-seen-wins across attempts discarded-edits/ since story-106, the stage's own version of every tester-discarded-edits/ path a refused revert check undid, at the content + documenter-discarded-edits/ the stage left, mirroring repository-relative paths; one dir per declaring stage, named by that stage's - revert_check declaration. Absent from a run in which - no check was refused, and from a stage whose - declaration names none + revert_check declaration (the documenter's since + story-154). Absent from a run in which no check + was refused, and from a stage whose declaration + names none attempts/attempt-1/ superseded attempt's artifacts, canonical filenames resumes/resume-1/ the entry a resume ended: everything above whose name is keyed by retry_count or by diff --git a/.harness/history/execution-log.jsonl b/.harness/history/execution-log.jsonl index 56b3ec9..bb9b1a8 100644 --- a/.harness/history/execution-log.jsonl +++ b/.harness/history/execution-log.jsonl @@ -90,3 +90,4 @@ {"story_id": "story-153", "status": "escalated", "retry_count": 1, "timestamp": "2026-09-16 12:27:50"} {"story_id": "story-153", "status": "resumed", "retry_count": 1, "timestamp": "2026-09-16 12:32:22"} {"story_id": "story-153", "status": "completed", "retry_count": 1, "timestamp": "2026-09-16 12:55:13"} +{"story_id": "story-154", "status": "completed", "retry_count": 1, "timestamp": "2026-09-16 17:32:45"} diff --git a/.harness/history/inspection-log.jsonl b/.harness/history/inspection-log.jsonl index 8d8b837..01ca51c 100644 --- a/.harness/history/inspection-log.jsonl +++ b/.harness/history/inspection-log.jsonl @@ -51,3 +51,7 @@ {"story_id": "story-153", "timestamp": "2026-09-16 12:27:41", "filed": 1, "dropped": 0, "routed": 3, "acted_on": 2} {"story_id": "story-153", "mode": "narrow", "cost_usd": 2.5583735, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-16 12:46:28", "findings": 1, "filed": 0, "dropped": 0, "dedupe_ran": true} {"story_id": "story-153", "timestamp": "2026-09-16 12:49:39", "filed": 1, "dropped": 0, "routed": 1, "acted_on": 0} +{"story_id": "story-154", "mode": "narrow", "cost_usd": 3.7946285, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-16 16:22:57", "findings": 4, "filed": 0, "dropped": 0, "dedupe_ran": true} +{"story_id": "story-154", "timestamp": "2026-09-16 17:01:19", "filed": 4, "dropped": 0, "routed": 4, "acted_on": 0} +{"story_id": "story-154", "mode": "narrow", "cost_usd": 3.3866405000000004, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-16 17:18:31", "findings": 6, "filed": 0, "dropped": 0, "dedupe_ran": true} +{"story_id": "story-154", "timestamp": "2026-09-16 17:24:21", "filed": 5, "dropped": 0, "routed": 6, "acted_on": 1} diff --git a/.harness/history/mandate-log.jsonl b/.harness/history/mandate-log.jsonl index 41aa24e..f1fbe37 100644 --- a/.harness/history/mandate-log.jsonl +++ b/.harness/history/mandate-log.jsonl @@ -59,3 +59,4 @@ {"story_id": "story-151", "timestamp": "2026-09-15 14:59:07", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false} {"story_id": "story-152", "timestamp": "2026-09-16 09:53:13", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false} {"story_id": "story-153", "timestamp": "2026-09-16 12:03:52", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false} +{"story_id": "story-154", "timestamp": "2026-09-16 14:56:07", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false} diff --git a/.harness/history/retry-log.jsonl b/.harness/history/retry-log.jsonl index d8b6c54..536b694 100644 --- a/.harness/history/retry-log.jsonl +++ b/.harness/history/retry-log.jsonl @@ -65,3 +65,4 @@ {"story_id": "story-152", "timestamp": "2026-09-16 10:28:50", "stage": "verifier", "retry_decision": "retry", "retry_category": "validation", "retry_stage": "tester"} {"story_id": "story-153", "timestamp": "2026-09-16 12:27:41", "stage": "verifier", "retry_decision": "retry", "retry_category": "validation", "retry_stage": "tester"} {"story_id": "story-153", "timestamp": "2026-09-16 12:27:49", "stage": "tester", "retry_stage": "tester"} +{"story_id": "story-154", "timestamp": "2026-09-16 17:01:19", "stage": "verifier", "retry_decision": "retry", "retry_category": "validation", "retry_stage": "tester"} diff --git a/.harness/stories/story-154.yaml b/.harness/stories/story-154.yaml new file mode 100644 index 0000000..e3ab06e --- /dev/null +++ b/.harness/stories/story-154.yaml @@ -0,0 +1,202 @@ +story: + id: story-154 + title: Every stage that may write outside its own subject is governed by a check that records what it wrote + description: | + Planned from the brief filed under 294. + + The documenter can write implementation code and the run records nothing + about it. On story-148 its changed-files record for the first attempt + named `.harness/docs/ARCHITECTURE.md` and `orchestration/story_inspection.py`, + and that module gained 75 lines across the run; on the second attempt it + edited a test module. Neither is reported anywhere: not in events.log, not + in the completion report, and to the verifier only as a changed-files list + its prompt describes as "documentation files". The documenter sits after + the tester, so its edits are not covered by the suite the coordinator ran, + and before the verifier, so what it wrote is in the tree the verifier + judges without being attributable to the stage that should have written + it. Nothing governs it because nothing was ever declared: the documenter + carries neither `may_only_change` nor `revert_check`, so there is no path + on which the coordinator asks what it wrote. + + This story gives the documenter the declaration and the check the tester + already carries, in both shipped workflows. The shape is a confinement, + because the documenter's subject is a set of documents rather than a + directory, and the prefix comes from configuration: the workflow declares + `"may_only_change": ["{{architecture_docs}}"]`, and the token resolver + that today answers only `{{tests_dir}}` learns that second key and splices + a list-valued one into the declaration. A literal path in a shipped + workflow is refused by a standing test and would be wrong for every other + target; an unset `architecture_docs` resolves the restriction out, as an + unset `tests_dir` does for the implementer. + + The confinement collides with the correction pass, and the collision is + settled here rather than left to fail quietly. Since story-063 and + story-067 the documenter, entered on a correction pass, corrects words + wherever the verifier's finding names them, a docstring in orchestration + source included, and `tests/test_correction_pass.py` pins that. The + revert check permits an edit outside a stage's subject only when undoing + it turns the suite red, and an edit to prose never does, so a documenter + governed on the tester's terms would have every pass edit outside the + architecture documents undone. The resolution is that the verifier grants + the pass its scope: a correctable finding gains a required `path`, the + repository-relative file its words are in, and naming it is the grant. + The coordinator already copies findings verbatim into the pass record, so + what was granted is recorded by the act that granted it. At the + documenter's post-stage checks the exempt list is the story's own grants + plus the path of every finding in the current pass record, when that + record names this stage and this attempt, and each granted path is an + event beside `stage exception applied`. A finding grants one file, never + a directory, for one pass; an edit to any file no finding named stays + governed, and a documenter re-entered by a later retry in the same run + does not inherit the grant. Nothing checks that the edit to a granted + file was prose, which this repository already states is not tractable; + the trust is the one a story grant extends, bounded by one file and one + pass and judged by the verifier that runs again after it. + + Two consequences follow and are accepted. A documenter that creates a + file outside the configured documents escalates on the ownership check, + as a tester creating outside `tests/` does, and a story that needs it + declares a `stage_exceptions` grant. A verifier that omits `path` fails + schema validation and self-routes on the bookkeeping cause, so a pass can + never quietly lose its grant. + + Deliberately left alone: #247, which asks that an out-of-confinement edit + the revert check finds necessary be routed rather than permitted, and + which presumes a check exists to find one; whether story-148's documenter + edits were right, which is settled by the shipped change and the green + suite; and the claim-support check, which reads what the documenter wrote + into a configured document and is unrelated to which files it may touch. + workflow: story-workflow + +tasks: + - In orchestration/harness_config.py, make `{{architecture_docs}}` a workflow token beside `{{tests_dir}}`. A token whose configured value is a list splices that list's entries into the declaration in place of the token entry; an unset or empty value resolves the entry out of the list exactly as an unset `tests_dir` does; a declared key outside the set, `branch_prefix` among them, stays unreferable so the set remains narrow rather than general. + - In workflows/story-workflow.json and workflows/refactor-workflow.json, give the documenter stage `"may_only_change": ["{{architecture_docs}}"]` and a `revert_check` declaration naming `documenter-revert-check-result.json` as its result, `stage-baseline` as its baseline directory and `documenter-discarded-edits` as its discarded directory, on the terms the story-workflow tester declares its own. + - In orchestration/story_coordinator.py, make StageRestriction.governs read a confinement prefix not ending in a slash as exactly that file - the meaning grant_covers already gives a slash-less value and the meaning the `:(exclude,top)` pathspec the baseline capture builds already has - so a path that merely shares the file's name as a prefix is governed rather than read as inside the confinement. A prefix ending in a slash keeps its directory meaning for both senses. + - In schemas/verification-result.schema.json, add `path` to a correctable finding's required fields: the repository-relative file the words to correct are in, and the grant the pass will run under. Say in its description that naming it is what lets the stage the pass enters at change that file. Make the same addition to the inline copy of the finding shape in schemas/correction-pass.schema.json, which that schema says is edited together with the verdict's. + - In orchestration/story_coordinator.py, at the post-stage ownership and revert checks, extend the exempt list a stage's checks read with the `path` of every finding in the current correction-pass record when that record's `stage` is the stage being checked and its `attempt` is the attempt now running. Append one event per granted path, of kind `correction-pass-grant-applied`, worded beside the existing `stage exception applied` event so the two read alike in events.log. Read the record off the run directory rather than passing it in memory, as the prompt injection of the same record does. + - In prompts/story-verifier.md and prompts/refactor-verifier.md, have the correctable-findings paragraph ask for `path` on each finding and say what naming it does - the stage the pass enters at may change that one file for that pass, and a finding that names no path is refused by the schema. Reword the documenter changed-files slot so it describes the record as every repository file the documenter touched rather than as documentation files. + - In prompts/documenter.md, have the correction paragraph say that the files the findings name are the documenter's to change for this pass, that the architecture documents always are, and that an edit to any other file is put to the revert check after the turn and undone if the suite is green without it - so a correction the pass cannot land is reported in documentation-report.md rather than attempted elsewhere. + - Tester: add a standing test module under tests/ holding the brief's assertion and its controls - a documenter whose record names a module outside the configured architecture documents has that edit decided by the revert check, undone and recorded when the suite is green without it and permitted and recorded when it is not; a documenter whose record names only the configured documents is unaffected and the run carries no revert event for it; a documenter creating a file outside the documents escalates on the ownership check; a file a correctable finding names by `path` is exempt from the revert check on the pass and the grant is in events.log; a documenter re-entered on a later attempt of the same run does not inherit that grant; and the token resolves the shipped declaration to the configured documents, to nothing when the key is unset, and to a several-entry list when several are configured. + - Tester: adapt the tests the change forces - the story-063 tests in tests/test_correction_pass.py, which asserted the entered stage carried no restriction and now assert the edit is permitted by the pass's grant; every correctable-finding fixture in the suite, which gains `path`; and the token-set tests in tests/test_configured_test_location.py, which state the referable set. + - Documenter: record in .harness/docs/ARCHITECTURE.md that the documenter is confined to the configured architecture documents under the same revert check as the tester, that the prefix is a workflow token resolved from `architecture_docs`, and that a correction pass is granted the files its findings name by the verifier's own verdict, with the bounds above. + +acceptance_criteria: + - Loading either shipped workflow against a configuration whose `architecture_docs` lists one document yields a documenter stage whose `may_only_change` is exactly that document's path; against a configuration listing two, both paths in configured order; against a configuration with the key unset or empty, no `may_only_change` entries at all and no restriction reported for the documenter by stage_restrictions. + - A workflow declaration referencing `{{branch_prefix}}` is still refused with UnresolvedWorkflowToken, and the refusal lists both `{{tests_dir}}` and `{{architecture_docs}}` as the referable set. + - A confinement restriction whose prefix is `.harness/docs/ARCHITECTURE.md` reports `.harness/docs/ARCHITECTURE.md.bak` as governed and `.harness/docs/ARCHITECTURE.md` as not governed; a confinement whose prefix is `tests/` still reports every path beneath it as not governed and every path outside it as governed. + - In a run under the story workflow whose documenter record names a module outside the configured documents, the coordinator runs the revert check on that path after the documenter's turn, writes documenter-revert-check-result.json, and appends the revert-check events for the documenter stage; when the suite passes with the edit reverted the working tree holds the baseline content, the stage's version is under documenter-discarded-edits, and the run continues; when the suite fails with it reverted the edit is kept and recorded as permitted. + - In a run whose documenter record names only paths in the configured documents, no revert check runs for the documenter, no documenter revert-check artifact is written, and the tree is as the documenter left it. + - In a run whose documenter record names a created file outside the configured documents and no grant covers it, the run escalates naming the documenter, the path and the confinement in the workflow's own wording. + - A verification-result.json carrying a correctable finding without `path` fails validation against schemas/verification-result.schema.json, and a correction-pass record carrying such a finding fails validation against schemas/correction-pass.schema.json; each schema's `path` description says what naming it grants. + - In a run that takes a correction pass whose finding names `path` orchestration/example.py, the documenter's edit to that file on the pass is not put to the revert check, an event of kind `correction-pass-grant-applied` naming the documenter and that path is appended before the checks run, and an edit the same documenter made to a second file no finding named is put to the revert check. + - In a run where the verifier, after a correction pass, fails the verdict and routes a retry that re-enters the documenter on the next attempt, that documenter invocation is granted nothing from the earlier pass record, and an edit it makes to the file that record named is put to the revert check. + - The rendered documenter prompt for a correction pass states that the files the findings name are the documenter's to change for that pass and that other edits outside the architecture documents are undone when the suite is green without them; both rendered verifier prompts ask for `path` on each correctable finding and describe the documenter changed-files record without the word "documentation" qualifying the files. + - tests/test_correction_pass.py's story-063 tests pass against the shipped workflows by establishing that the pass's grant permits the docstring edit in orchestration source, and no assertion in that module is weakened to do it. + - The planner-injection rendering of the story workflow lists the documenter's confinement in the workflow's own wording beside the implementer's and tester's restrictions. + - .harness/docs/ARCHITECTURE.md describes the documenter's confinement, the `{{architecture_docs}}` token, and the correction-pass grant, naming the event kind and the bounds on the grant. + +technical_plan: + implementation_steps: + - Read StageRestriction, restrictions_on, granted_paths and grant_covers at orchestration/story_coordinator.py:396-826, baseline_pathspecs and stage_baseline_requests around line 5081, the post-stage checks around line 9007, and the correction-pass routing around line 9389, so the grant is read where the story grants are read and nothing about the checks themselves changes. + - Extend workflow_token_values in orchestration/harness_config.py to return `architecture_docs` beside `tests_dir`, and teach _resolve_tokens that a list-valued token replaces its entry with the list's entries; keep the unset case resolving the entry out, and update the docstrings that say the mapping has one entry. + - Add the confinement and the revert_check declaration to the documenter stage of both workflow definitions, keyed as the tester's are so archivable_artifacts and the archived check results pick the new artifact up with no code change. + - Change StageRestriction.governs so a confinement prefix without a trailing slash matches only the identical path, and update its docstring; confirm baseline_pathspecs needs no change because a git pathspec already reads a slash-less value that way. + - Add `path` to the finding shape in both schemas, required, with a description that says it is the grant; check the schema inventory test still holds. + - Write a reader of the current pass record - the declared artifact keyed by state.correction_pass_count, read off the run directory - that returns the finding paths when the record's stage and attempt match the stage being checked and the running attempt, and join its result to the exempt list at the ownership and revert checks with one event per path. + - Edit the three prompts as the tasks describe, keeping the correction paragraph's existing sentence about the approved story artifact never being edited by a pass. + - Run the existing confinement, revert-check and correction-pass modules under the configured test command before handing over; the tester writes the new module and adapts the fixtures. + likely_file_changes: + - file: orchestration/harness_config.py + stage: implementer + reason: The workflow token resolver learns a second key and how to splice a list-valued one. + - file: workflows/story-workflow.json + stage: implementer + reason: The documenter declares the confinement and the revert check. + - file: workflows/refactor-workflow.json + stage: implementer + reason: The refactor workflow's documenter is in the same position and gets the same declaration. + - file: orchestration/story_coordinator.py + stage: implementer + reason: A slash-less confinement prefix is read as one file, and the correction-pass grant joins the exempt list at the post-stage checks with an event per path. + - file: schemas/verification-result.schema.json + stage: implementer + reason: A correctable finding requires `path`, the file its words are in and the grant the pass runs under. + - file: schemas/correction-pass.schema.json + stage: implementer + reason: The inline copy of the finding shape is edited together with the verdict's. + - file: prompts/story-verifier.md + stage: implementer + reason: The correctable-findings paragraph asks for `path` and the documenter changed-files slot stops calling the record documentation files. + - file: prompts/refactor-verifier.md + stage: implementer + reason: The same two edits for the refactor workflow's verifier. + - file: prompts/documenter.md + stage: implementer + reason: The correction paragraph says which files the pass may change and what happens to an edit elsewhere. + - file: tests/test_documenter_is_governed.py + stage: tester + reason: The standing module holding the brief's assertion and its controls. + - file: tests/test_correction_pass.py + stage: tester + reason: The story-063 tests assert the grant rather than the absence of a restriction, and its correctable-finding fixtures gain `path`. + - file: tests/test_configured_test_location.py + stage: tester + reason: The tests stating the referable token set now name two keys. + - file: tests/test_archived_check_results.py + stage: tester + reason: Its correctable-finding fixtures gain `path`. + - file: tests/test_inspection_findings_reach_the_story.py + stage: tester + reason: Its correctable-finding fixtures gain `path`. + - file: .harness/docs/ARCHITECTURE.md + stage: documenter + reason: Records the documenter's confinement, the token and the correction-pass grant. + +scope: + modify: + - orchestration/harness_config.py + - orchestration/story_coordinator.py + - workflows/story-workflow.json + - workflows/refactor-workflow.json + - schemas/verification-result.schema.json + - schemas/correction-pass.schema.json + - prompts/story-verifier.md + - prompts/refactor-verifier.md + - prompts/documenter.md + - tests/ + - .harness/docs/ARCHITECTURE.md + do_not_modify: + - orchestration/plan_validation.py + - orchestration/context_assembler.py + - orchestration/verification_runner.py + - hooks/ + - .harness/config.yaml + - schemas/story.schema.json + - schemas/changed-files.schema.json + +verification_requirements: + - Confirm the full suite passes under the configured test command. + - Confirm the new test module is a real check - that its documenter-edits-a-module case fails against the shipped workflows with the documenter's `may_only_change` and `revert_check` removed, and that its grant case fails with the coordinator's correction-pass grant removed. + - Confirm both shipped workflows load against .harness/config.yaml with the documenter's confinement resolved to `.harness/docs/ARCHITECTURE.md`, and that no shipped workflow carries a literal repository path. + - Confirm the correction-pass grant is bounded as the story says - one file per finding, in force only for the stage and attempt the pass record names, recorded as an event per path - and that an edit to a file no finding named is still put to the revert check on the pass. + - Confirm no existing assertion in tests/test_correction_pass.py, tests/test_confined_stage_edits.py, tests/test_revert_check.py or tests/test_stage_output_ownership.py was weakened, skipped or removed, and that the story-063 tests still establish that the pass's docstring edit in orchestration source lands. + - Confirm the ownership check, the revert check and its two dispositions run for the documenter off the declaration alone, with no documenter-specific branch in orchestration/story_coordinator.py. + - Confirm the claim-support check is unchanged and #247's routing question is not decided by anything this story changed. + +constraints: + - No stage name, artifact name or repository path is written into the coordinator's enforcement path for this; the documenter is governed because its declaration says so, and removing the declaration disables the check with no code change, as it does for the tester. + - The revert check's verdicts and dispositions are not changed. A refused documenter edit is undone and recorded rather than escalated, exactly as a refused tester edit is, and a check that could not run escalates as it does today. + - The correction-pass grant is conferred by the verifier's verdict alone. No agent writes a grant separately from a finding, and the coordinator does not parse `location` or any other prose to derive a path. + - A story-level stage_exceptions grant keeps its meaning and its plan-time cross-check; the pass grant is added beside it in the exempt list and does not replace or widen it. + - The token set stays narrow: two keys, each named as a literal read in workflow_token_values, and any other `{{key}}` is still refused. + - No existing test is weakened, skipped or removed to pass; fixtures gain the required field and nothing else. + - No wall-clock bound is added to any test. + +mandate: + source: + kind: human + conferred_at: 2026-09-16 14:56:07 + conferred_by: jerod.wilkerson <30474318+jerodw@users.noreply.github.com> + recorded_by: l5-plan + +brief_key: 294 diff --git a/orchestration/harness_config.py b/orchestration/harness_config.py index 49755e5..ae51da2 100644 --- a/orchestration/harness_config.py +++ b/orchestration/harness_config.py @@ -263,9 +263,17 @@ def workflow_token_values(config: dict) -> dict[str, str | None]: 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. + That the mapping has two entries, each named as a literal read, is the + narrowness: a declared key outside this set -- `branch_prefix` among them + -- stays unreferable, so the set is narrow rather than general. `tests_dir` + resolves to a single string; `architecture_docs` resolves to a list, whose + entries `_resolve_tokens` splices into the declaration in place of the + token entry. """ - return {"tests_dir": config.get("tests_dir")} + return { + "tests_dir": config.get("tests_dir"), + "architecture_docs": config.get("architecture_docs"), + } def workflow_names(harness_root: Path) -> tuple[str, ...]: @@ -330,12 +338,16 @@ def __init__(self, workflow: str, tokens: list[str]): 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. + A token whose configured value is a list splices that list's entries into + the declaration in place of the token entry; a token whose value is a + single string replaces the entry with that string. An unset or empty value + 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, and an empty + list splices in nothing for the same reason. 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) @@ -350,8 +362,12 @@ def _resolve_tokens(value, values: dict[str, str | None], unresolved: list[str]) name = match.group(1) if name not in values: unresolved.append(name) - elif values[name]: - resolved.append(values[name]) + continue + configured = values[name] + if isinstance(configured, list): + resolved.extend(configured) + elif configured: + resolved.append(configured) return resolved if isinstance(value, str): unresolved.extend(_WORKFLOW_TOKEN.findall(value)) diff --git a/orchestration/story_coordinator.py b/orchestration/story_coordinator.py index dfabe35..f1e52c4 100644 --- a/orchestration/story_coordinator.py +++ b/orchestration/story_coordinator.py @@ -403,6 +403,19 @@ def resolves_nothing(source: dict) -> dict | None: CONFINEMENT = "may_only_change" +def _inside(prefix: str, path: str) -> bool: + """Whether `path` is at or beneath a confinement prefix. + + A prefix ending in a slash is a directory and covers everything beneath it; + any other prefix is exactly one file and covers only the identical path. + The same slash convention `grant_covers` gives a granted value, so a + confinement and a grant agree about what a slash-less value means. + """ + if prefix.endswith("/"): + return path.startswith(prefix) + return path == prefix + + @dataclass(frozen=True) class StageRestriction: """One declared restriction on where a stage may write, and what it means. @@ -435,10 +448,20 @@ def governs(self, path: str) -> bool: restriction claim a path only its sibling covers. A confinement is the opposite and has to read the group, because being confined to two directories means being governed only outside *both*. + + A prefix not ending in a slash is exactly that file, not a directory + whose name it happens to be: `.harness/docs/ARCHITECTURE.md` confines a + stage to that one file, so `.harness/docs/ARCHITECTURE.md.bak` is + governed rather than read as inside it. That is the meaning `grant_covers` + already gives a slash-less value and the meaning the `:(exclude,top)` + pathspec the baseline capture builds already has; a prefix ending in a + slash keeps its directory meaning. A confinement decides `inside` per + prefix rather than by `startswith`, so a slash-less prefix admits only + the identical path. """ if self.sense == CREATE_RESTRICTION: return path.startswith(self.prefix) - return not any(path.startswith(prefix) for prefix in self.prefixes) + return not any(_inside(prefix, path) for prefix in self.prefixes) @property def wording(self) -> str: @@ -5933,6 +5956,48 @@ def correction_pass_result_file(artifact: str, number: int) -> str: return f"{stem}-{number}{dot}{extension}" +def correction_pass_grants( + run_dir: Path, stages: list[dict], state: RunState, stage_name: str, attempt: int +) -> list[str]: + """The files this pass's findings grant one stage to change for one attempt. + + A correctable finding names a `path`, and naming it is the grant: at the + stage the pass enters, the exempt list the ownership and revert checks read + gains that path, so a finding grants its stage the one file it names for + this pass. The record is read off the run directory rather than passed in + memory, exactly as the prompt injection of the same record is, so what the + checks exempt and what the prompt reports are one thing. + + The grant is bounded to the stage and the attempt the record names: a + documenter re-entered by a later retry in the same run runs at a later + attempt, so the attempt no longer matches and the grant is not inherited. + A record for another stage grants nothing here either. A run that has taken + no pass, a workflow that declares none, and a record that is absent or + unreadable each grant nothing. + """ + if not state.correction_pass_count: + return [] + declared = correction_pass_declaration(stages).get("result") + if not declared: + return [] + record = run_dir / correction_pass_result_file( + declared, state.correction_pass_count + ) + if not record.is_file(): + return [] + try: + parsed = json.loads(record.read_text(encoding="utf-8")) + except (OSError, ValueError): + return [] + if parsed.get("stage") != stage_name or parsed.get("attempt") != attempt: + return [] + return [ + finding["path"] + for finding in parsed.get("findings", []) + if finding.get("path") + ] + + def correction_pass_statement(stage_name: str) -> str: """What the coordinator tells the stage a correction pass routes to. @@ -9013,6 +9078,25 @@ def elapsed() -> float | None: kind="stage-exception-applied", stage=name, ) + # A correction pass grants the stage it enters the files its + # findings name, for that pass alone. The verifier's verdict copied + # verbatim into the pass record is the grant, so what was granted is + # recorded by the act that granted it; the exempt list here is the + # story's own grants plus the path of every finding in the current + # pass record, when that record names this stage and this attempt. + # Each granted path is an event beside `stage exception applied` so + # the two read alike in events.log. + pass_grants = correction_pass_grants( + run_dir, stages, state, name, attempt + ) + for granted in pass_grants: + append_event( + run_dir, + f"correction-pass grant applied: {name} may change {granted}", + kind="correction-pass-grant-applied", + stage=name, + ) + exempt = exempt + pass_grants ownership = _ownership_violation(run_dir, record_name, enforced, exempt) if ownership: return _escalate( diff --git a/prompts/documenter.md b/prompts/documenter.md index 8f4209c..e053e54 100644 --- a/prompts/documenter.md +++ b/prompts/documenter.md @@ -81,9 +81,14 @@ and fixable in the words alone. The coordinator wrote it, not an agent. Correct the words each finding names and nothing else: this changes prose and never behaviour, so nothing you change for it may alter what any test asserts. You are not asked to establish that — the clean-clone check the coordinator runs -after the pass confirms the correction changed no behaviour. The approved story -artifact is never edited by a pass; if a finding names it, correct nothing and -say so in your report. No retry was spent and the verdict still stands: +after the pass confirms the correction changed no behaviour. The files the +findings name are yours to change for this pass, and the architecture documents +always are. An edit to any other file is put to the revert check after your +turn ends, and undone if the suite is green without it — so if a correction +cannot land in a file you may change, report that in documentation-report.md +rather than attempting it elsewhere. The approved story artifact is never edited +by a pass; if a finding names it, correct nothing and say so in your report. No +retry was spent and the verdict still stands: {{correction_pass_result}} Retry guidance: diff --git a/prompts/planner.md b/prompts/planner.md index 500337b..d6bd863 100644 --- a/prompts/planner.md +++ b/prompts/planner.md @@ -22,7 +22,8 @@ Do not: - write the story artifact before the developer approves the plan. [Process] -1. Read .harness/docs/ARCHITECTURE.md and the standards in .harness/standards/. +1. Read the architecture documents the target's configuration names under + `architecture_docs`, and the standards in .harness/standards/. 2. Ask the developer any questions required to remove important ambiguity. Ask about decisions that are expensive or hard to reverse; apply sensible defaults to cheap, reversible ones and say you did. diff --git a/prompts/refactor-verifier.md b/prompts/refactor-verifier.md index c27f550..af843c3 100644 --- a/prompts/refactor-verifier.md +++ b/prompts/refactor-verifier.md @@ -114,6 +114,12 @@ can fix mechanically, and whose repair is to the words alone — a comment, a docstring, a schema description, a document. Name the retry category that owns that prose, spelled exactly as the categories above are: a category this workflow does not define escalates the run rather than being routed somewhere. +Give each finding a `path`, the repository-relative file the words to correct +are in. Naming it is what lets the stage the pass enters at change that one +file for that pass: the coordinator grants that stage the files the findings +name, and an edit to any other file is put to the revert check after the turn. +A finding that names no path is refused by the schema, because the pass would +have no scope to change the words in. What stays in unverified is what you could not check: evidence that was missing, a claim you had no way to settle, a question the artifacts available @@ -246,8 +252,8 @@ story scope, and note that edits to existing tests are within this stage's work under this workflow): {{changed_files}} -Documenter changed files (documenter's record — documentation files created -or modified by the documenter stage): +Documenter changed files (documenter's record — every repository file the +documenter stage touched): {{documenter_changed_files}} Documentation report (the documenter's account of what it wrote and why): diff --git a/prompts/story-verifier.md b/prompts/story-verifier.md index fde5534..8af3cf6 100644 --- a/prompts/story-verifier.md +++ b/prompts/story-verifier.md @@ -87,6 +87,12 @@ can fix mechanically, and whose repair is to the words alone — a comment, a docstring, a schema description, a document. Name the retry category that owns that prose, spelled exactly as the categories above are: a category this workflow does not define escalates the run rather than being routed somewhere. +Give each finding a `path`, the repository-relative file the words to correct +are in. Naming it is what lets the stage the pass enters at change that one +file for that pass: the coordinator grants that stage the files the findings +name, and an edit to any other file is put to the revert check after the turn. +A finding that names no path is refused by the schema, because the pass would +have no scope to change the words in. What stays in unverified is what you could not check: evidence that was missing, a claim you had no way to settle, a question the artifacts available @@ -219,8 +225,8 @@ the tester stage; treat them as expected additions of a later stage, not implementation scope violations): {{tester_changed_files}} -Documenter changed files (documenter's record — documentation files created -or modified by the documenter stage): +Documenter changed files (documenter's record — every repository file the +documenter stage touched): {{documenter_changed_files}} Documentation report (the documenter's account of what it wrote and why): diff --git a/schemas/correction-pass.schema.json b/schemas/correction-pass.schema.json index 24d6a47..9f9c4a7 100644 --- a/schemas/correction-pass.schema.json +++ b/schemas/correction-pass.schema.json @@ -22,8 +22,12 @@ "description": "The correctable findings the verdict carried, copied verbatim. The item shape is declared inline rather than referenced, because this validator subset has no cross-file reference and a finding recorded here must validate identically to one in verification-result.schema.json's correctable_findings — the duplication is stated here so the two are edited together rather than drifting.", "items": { "type": "object", - "required": ["location", "finding", "correction", "category"], + "required": ["path", "location", "finding", "correction", "category"], "properties": { + "path": { + "type": "string", + "description": "The repository-relative file the words to correct are in, as the verifier recorded it. Naming it is what lets the stage this pass enters at change that file: the coordinator adds the path of every finding in this record to that stage's exempt list, so the finding grants its stage the one file it names for this pass, while an edit to any file no finding named stays governed by the revert check." + }, "location": { "type": "string", "description": "Where the wording is, as the verifier recorded it." diff --git a/schemas/execution-history.schema.json b/schemas/execution-history.schema.json index c19437a..b77b363 100644 --- a/schemas/execution-history.schema.json +++ b/schemas/execution-history.schema.json @@ -37,6 +37,7 @@ "claim-support-checked", "correction-pass-routed", "correction-pass-recorded", + "correction-pass-grant-applied", "self-routed", "budget-stopped", "capacity-paused", diff --git a/schemas/verification-result.schema.json b/schemas/verification-result.schema.json index 2da3229..80f73c9 100644 --- a/schemas/verification-result.schema.json +++ b/schemas/verification-result.schema.json @@ -73,8 +73,12 @@ "description": "Findings you judge correct but too small to fail the run, that one stage can fix mechanically. Its presence is the signal that a correction pass should run: there is no separate key saying whether this field means anything, for the same limitation retry_target, unfinishable_by_retry and guidance_outcomes.unmet already record above — this validator subset has no if/then and no dependentRequired, so a flag with a sibling payload has a state where the flag is set and the payload is missing, while presence-as-signal has none. A correctable finding is a correction to words alone — a comment, a docstring, a schema description, a document — and never to behaviour: nothing a correction pass changes may alter what any test asserts about the system, and the suite must pass unchanged across a pass. A finding whose repair would change behaviour is a blocking issue or belongs in unverified, not here. It is read only on a passing verdict, because a failed verdict already routes and the correction travels in the retry guidance it already writes. It is optional because almost every verdict has no such finding.", "items": { "type": "object", - "required": ["location", "finding", "correction", "category"], + "required": ["path", "location", "finding", "correction", "category"], "properties": { + "path": { + "type": "string", + "description": "The repository-relative file the words to correct are in. Naming it is what lets the stage the pass enters at change that file: at the documenter's post-stage checks the exempt list is the story's own grants plus the path of every finding in the current pass record, so a finding grants its stage the one file it names, for that pass alone. An edit to any file no finding named stays governed by the revert check. This is why a correctable finding without a path fails schema validation — the pass would run with no scope to change the words in." + }, "location": { "type": "string", "description": "Where the wording is: the file, and the symbol, section or line within it that carries the words to correct." diff --git a/tests/test_archived_check_results.py b/tests/test_archived_check_results.py index 10ee8e3..5ffe6cb 100644 --- a/tests/test_archived_check_results.py +++ b/tests/test_archived_check_results.py @@ -290,6 +290,7 @@ def failing_verdict(attempt: int) -> dict: def finding(marker: str) -> dict: """A correctable finding whose words name the verdict that carried it.""" return { + "path": ".harness/docs/ARCHITECTURE.md", "location": ".harness/docs/ARCHITECTURE.md - the routing section", "finding": f"MARKER-{marker} the paragraph names a stage that was renamed", "correction": f"MARKER-{marker}-FIX name the stage the workflow declares", diff --git a/tests/test_baseline_honesty.py b/tests/test_baseline_honesty.py index 4bc2b3e..6aed173 100644 --- a/tests/test_baseline_honesty.py +++ b/tests/test_baseline_honesty.py @@ -2449,6 +2449,20 @@ def live_artifact_reading_modules() -> list[str]: "destination_or_when, test_the_template_declares_both_placeholders " "and test_the_role_layer_says_the_documenters_output_is_part_of_the_" "subject read prompts/story-verifier.md, the template this repository ships", + "test_documenter_is_governed.py": + "the shipped workflows are the whole of its subject: story-154's claim " + "is that *this deployment's* documenter is confined to the configured " + "architecture documents under the revert check, in both definitions, " + "and that a correction pass under the shipped verifier's declaration " + "is granted the files its findings name. That cannot be asked of a " + "built workflow, which would only say back what the module declared. " + "Every run it drives goes through the shipped story workflow against " + "a target repository it builds, with a real suite the revert check's " + "verdict comes from; the documenter stage, the artifact names and the " + "correction-pass entry are derived from the definitions rather than " + "written. It resolves the two schemas the finding's required path is " + "declared in and the shipped verifier and documenter templates for " + "the same reason: what this repository ships is the claim", "test_escalation_summary.py": "its workflow reads were converted by story-048; what remains is the " "retry ceiling this repository declares in rules/execution-rules.json, " diff --git a/tests/test_config_keys_are_obeyed.py b/tests/test_config_keys_are_obeyed.py index d98173c..ebd7e9e 100644 --- a/tests/test_config_keys_are_obeyed.py +++ b/tests/test_config_keys_are_obeyed.py @@ -1483,7 +1483,7 @@ def test_the_scan_does_not_count_a_subscript_through_a_variable(tmp_path): # reads keys by name: exactly the named reads are counted, and neither of # the two variables the mapping is built through joins them. read = keys_read_in(REPO_ROOT / "orchestration" / "harness_config.py") - assert read == {"tests_dir", "history_dir", "logs_dir"} + assert read == {"tests_dir", "architecture_docs", "history_dir", "logs_dir"} assert not read & {"key", "current_list"} @@ -2704,8 +2704,13 @@ def test_tests_dir_is_the_location_the_workflow_and_the_prompt_are_governed_at( run.config) restrictions = story_coordinator.stage_restrictions(workflow["stages"]) assert restrictions + # Every prefix the run enforces is a configured value: the test location + # under the stages that declare it, and — since story-154 — the configured + # architecture documents under the stage confined to them. Nothing here + # is the directory the harness used to assume. assert {restriction.prefix for restriction in restrictions} == { - "xyzzy-checks/"} + "xyzzy-checks/", *run.config["architecture_docs"]} + assert "tests/" not in {restriction.prefix for restriction in restrictions} 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. diff --git a/tests/test_configured_test_location.py b/tests/test_configured_test_location.py index 3c6f301..2441f9b 100644 --- a/tests/test_configured_test_location.py +++ b/tests/test_configured_test_location.py @@ -147,11 +147,30 @@ def restrictions_under(tests_dir: str | None, return story_coordinator.stage_restrictions(workflow["stages"]) +def location_token_restrictions(tests_dir: str | None, + harness_root: Path = REPO_ROOT) -> list: + """The restrictions the test-location token alone produces. + + Since story-154 the shipped definition resolves a second token, from + another configuration key, and the restrictions it produces are in every + load whatever `tests_dir` says. What this module is about is the test + location, so what it reads is the difference the key makes: the load with + the key set, less everything the load with it unset reports. Nothing here + names the other token or the key it resolves from. + """ + unaffected = restrictions_under(None, harness_root) + return [restriction for restriction in restrictions_under(tests_dir, + harness_root) + if restriction not in unaffected] + + def prefixes_under(tests_dir: str | None, harness_root: Path = REPO_ROOT) -> list[str]: - """Just the prefixes of the above, for an assertion about the value alone.""" + """Just the prefixes the test location produces, for an assertion about + the value alone.""" return [restriction.prefix - for restriction in restrictions_under(tests_dir, harness_root)] + for restriction in location_token_restrictions(tests_dir, + harness_root)] DEFINITION_TEXT = (REPO_ROOT / "workflows" / f"{WORKFLOW_NAME}.json").read_text( @@ -183,10 +202,11 @@ def test_every_declaration_naming_the_test_location_moves_with_the_config(): are read off the load rather than named here. """ for location in ("spec/", "__tests__/"): - restrictions = restrictions_under(location) + restrictions = location_token_restrictions(location) assert {restriction.prefix for restriction in restrictions} == {location} assert {restriction.sense for restriction in restrictions} == { - restriction.sense for restriction in restrictions_under(CONFIGURED)} + restriction.sense + for restriction in location_token_restrictions(CONFIGURED)} def test_a_different_configured_location_moves_the_restriction_with_it(): @@ -199,9 +219,20 @@ def test_a_different_configured_location_moves_the_restriction_with_it(): 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) != [] + `stage_restrictions` that stopped seeing anything. + + What the unset load still reports is what the other token resolves, and + it is asserted to be exactly what the set load reports beyond the test + location — so the key removes its own restrictions and touches nothing + else's. + """ + unset = restrictions_under(None) + assert [r for r in unset if r.sense == story_coordinator.CREATE_RESTRICTION] == [] + assert CONFIGURED not in {restriction.prefix for restriction in unset} + assert location_token_restrictions(CONFIGURED) != [] + assert [r for r in restrictions_under(CONFIGURED) if r not in unset] \ + == location_token_restrictions(CONFIGURED) + assert all(r in restrictions_under(CONFIGURED) for r in unset) def test_the_unset_key_removes_the_entry_rather_than_emptying_the_list_item(): @@ -232,8 +263,8 @@ def test_a_resolver_that_emptied_the_entry_is_reported_by_the_same_check(tmp_pat """ mutant = load_mutant( REPO_ROOT / "orchestration" / "harness_config.py", - [(" elif values[name]:\n resolved.append(values[name])", - " else:\n resolved.append(values[name] or \"\")")], + [(" elif configured:\n resolved.append(configured)", + " else:\n resolved.append(configured or \"\")")], name="harness_config_emptying_the_entry", tmp_path=tmp_path) workflow = mutant.load_workflow(REPO_ROOT, WORKFLOW_NAME, config_with(None)) @@ -917,33 +948,45 @@ def restriction_problems(artifact: Path, stages: list[dict]) -> list[str]: ) -def literal_stages(prefix: str) -> list[dict]: +def literal_stages(prefix: str, tmp_path: Path) -> 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. + Built from today's shipped declaration with the test-location 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. Only that token is replaced: + every other entry of every restriction, a later story's token among them, + is left as it ships and resolved through the same load the resolved side + goes through, so the two sides differ in the test location alone. """ definition = raw_definition() + token = "{{tests_dir}}" for stage in definition["stages"]: for key in (story_coordinator.CREATE_RESTRICTION, story_coordinator.CONFINEMENT): if key in stage: - stage[key] = [prefix] - return definition["stages"] + stage[key] = [prefix if entry == token else entry + for entry in stage[key]] + assert token not in json.dumps(definition) + harness = mirror_harness(tmp_path / "literal-harness", definition) + return harness_config.load_workflow( + harness, WORKFLOW_NAME, conftest.repository_config())["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, +def test_every_committed_story_artifact_validates_exactly_as_it_did_before( + tmp_path, +): + """All 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"] + literal_definition = literal_stages(location, tmp_path) + resolved = {a: restriction_problems(a, stages_at(location)) for a in artifacts} - literal = {a: restriction_problems(a, literal_stages(location)) + literal = {a: restriction_problems(a, literal_definition) for a in artifacts} assert resolved == literal @@ -1036,9 +1079,11 @@ 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 {restriction.prefix for restriction - in story_coordinator.stage_restrictions( - conftest.shipped_workflow()["stages"])} == {location} + assert {restriction.prefix + for restriction in location_token_restrictions(location)} == {location} + assert location in {restriction.prefix for restriction + in story_coordinator.stage_restrictions( + conftest.shipped_workflow()["stages"])} def test_a_newly_initialized_target_declares_a_location_too(): diff --git a/tests/test_correction_pass.py b/tests/test_correction_pass.py index 4cfb2ab..00a5954 100644 --- a/tests/test_correction_pass.py +++ b/tests/test_correction_pass.py @@ -55,7 +55,11 @@ with the declaration repaired, which creates all of it; * "the statement no longer tells the stage to establish the suite" sits beside the clause it used to carry, constructed here in the test, which the - same check reports. + same check reports; + * "story-063's edit in orchestration source is exempt from both checks + under the pass's grant" sits beside the same record with nothing granted, + which the shipped stage's confinement governs — so the permission is the + grant rather than a confinement that stopped governing anything. """ import json import shutil @@ -185,24 +189,28 @@ def destination_of(category: str) -> str: #: matched by its own words, which is what "the finding reached the stage" #: means. CHECKS_FINDING = { + "path": "tests/test_sample.py", "location": "tests/test_sample.py::test_the_sample - the module docstring", "finding": "MARKER-CHECKS the docstring says three assertions and four follow", "correction": "MARKER-CHECKS-FIX delete the count and say 'the assertions below'", "category": CHECKS_CATEGORY, } BEHAVIOUR_FINDING = { + "path": "src/app.py", "location": "src/app.py - the docstring of the sample function", "finding": "MARKER-BEHAVIOUR the docstring wraps mid-word", "correction": "MARKER-BEHAVIOUR-FIX rewrap the line", "category": BEHAVIOUR_CATEGORY, } RECORD_FINDING = { + "path": ".harness/docs/ARCHITECTURE.md", "location": ".harness/docs/ARCHITECTURE.md - the routing section", "finding": "MARKER-RECORD the paragraph names a stage that was renamed", "correction": "MARKER-RECORD-FIX name the stage the workflow declares today", "category": RECORD_CATEGORY, } UNKNOWN_FINDING = { + "path": "src/app.py", "location": "src/app.py - the module comment", "finding": "MARKER-UNKNOWN the comment describes behaviour the code lost", "correction": "MARKER-UNKNOWN-FIX describe what the code does now", @@ -1489,8 +1497,8 @@ def test_the_verification_result_schema_declares_the_field_as_optional(): assert field in VERDICT_SCHEMA["properties"] assert field not in VERDICT_SCHEMA.get("required", []) item = VERDICT_SCHEMA["properties"][field]["items"] - assert set(item["required"]) == {"location", "finding", "correction", - "category"} + assert set(item["required"]) == {"path", "location", "finding", + "correction", "category"} assert schema_validator.validate(passing_with(CHECKS_FINDING), VERDICT_SCHEMA) == [] assert schema_validator.validate(PASS, VERDICT_SCHEMA) == [] @@ -1510,8 +1518,12 @@ def test_a_verdict_whose_finding_omits_a_required_field_is_reported(): # the files the entered stage ordinarily writes, which is why the run that met # it categorised it elsewhere and re-entered elsewhere. Under the declared # entry it goes to the shipped stage regardless, so what has to hold is that -# the edit is one that stage is both permitted and instructed to make. The -# subject here is what this repository ships, which is why these read it. +# the edit is one that stage is both permitted and instructed to make. Since +# story-154 that stage is confined to the configured architecture documents, +# so "permitted" is no longer the absence of a restriction: it is the grant +# the pass carries, the `path` each finding names, read by the coordinator +# into the exempt list the ownership and revert checks share. The subject +# here is what this repository ships, which is why these read it. # -------------------------------------------------------------------------- BLOCKED_PATHS = json.loads( @@ -1539,9 +1551,9 @@ def test_the_edit_falls_under_no_blocked_path(): def test_the_stage_the_pass_enters_at_is_governed_by_no_create_restriction(): - """The second half. `may_not_create` is what stops a stage writing under a - prefix, and the stage the shipped declaration enters at declares none — - which is what makes the correction its own work rather than a compromise. + """The second half, part one. `may_not_create` is what stops a stage + writing under a prefix outright, and the stage the shipped declaration + enters at declares none. The control is the shipped stages that do declare one: without it, a coordinator that had renamed the key would read as unrestricted here. @@ -1554,6 +1566,84 @@ def test_the_stage_the_pass_enters_at_is_governed_by_no_create_restriction(): assert shipped_correction_stage()["name"] not in restricted +def story_063_finding() -> dict: + """A finding shaped as story-063's was, naming the file its words are in.""" + return {**BEHAVIOUR_FINDING, "path": STORY_063_PATH} + + +def test_the_stage_the_pass_enters_at_is_confined_and_the_pass_grants_the_edit( + tmp_path, +): + """The second half, part two. Since story-154 the entered stage *is* + confined, to the configured architecture documents, so story-063's + docstring edit in orchestration source is one its confinement governs — + and the edit still lands because the pass's grant permits it: the finding + names the file, and the coordinator adds every path the current pass + record names to the exempt list the ownership and revert checks read. + + Driven through the coordinator's own reader of the record rather than + argued from the prompt, against a record written the way the coordinator + writes one, keyed by the shipped declaration's artifact and pass count. + The control is the same record read for a later attempt, which grants + nothing, and the same path with no record at all, which the confinement + governs — so the permission is the grant rather than a confinement that + stopped governing anything. + """ + stage = shipped_correction_stage() + (confinement,) = [restriction for restriction + in story_coordinator.restrictions_on(stage)] + assert confinement.sense == story_coordinator.CONFINEMENT + assert confinement.governs(STORY_063_PATH) + + shipped = conftest.shipped_workflow() + declared = story_coordinator.correction_pass_declaration(shipped["stages"]) + run_dir = tmp_path / "run" + run_dir.mkdir() + write_json(run_dir / story_coordinator.correction_pass_result_file( + declared["result"], 1), { + "pass": 1, "attempt": 1, "stage": stage["name"], + "findings": [story_063_finding()], + "statement": story_coordinator.correction_pass_statement( + stage["name"]), + }) + state = story_coordinator.RunState(story_id="story-001", branch="story/story-001") + state.correction_pass_count = 1 + + granted = story_coordinator.correction_pass_grants( + run_dir, shipped["stages"], state, stage["name"], 1) + assert granted == [STORY_063_PATH] + assert story_coordinator.grant_covers(granted, STORY_063_PATH) + # And nothing beside it: the grant is the one file, not its directory. + assert not story_coordinator.grant_covers( + granted, f"orchestration/not-{STORY_063_LOCATION}") + # A later attempt of the same run inherits nothing from the record. + assert story_coordinator.correction_pass_grants( + run_dir, shipped["stages"], state, stage["name"], 2) == [] + + +def test_the_pass_grant_exempts_the_edit_from_both_checks(tmp_path): + """The two checks the grant is read by, asked directly with the entered + stage's own restrictions: with the grant the docstring edit is neither an + ownership violation nor a governed edit, and without it the revert check + would have decided it.""" + stage = shipped_correction_stage() + restrictions = story_coordinator.restrictions_on(stage) + run_dir = tmp_path / "run" + run_dir.mkdir() + write_json(run_dir / stage["changed_files"], + {"modified": [STORY_063_PATH], "created": [], "deleted": []}) + + granted = [story_063_finding()["path"]] + assert story_coordinator.governed_edits( + run_dir, stage["changed_files"], restrictions, granted).paths == () + assert story_coordinator._ownership_violation( + run_dir, stage["changed_files"], restrictions, granted) is None + # The control: the identical record with nothing granted is governed. + assert story_coordinator.governed_edits( + run_dir, stage["changed_files"], restrictions, []).paths \ + == (STORY_063_PATH,) + + def test_that_stage_is_instructed_to_correct_whatever_the_finding_names(): """"Instructed", read off the paragraph the stage meets: the instruction is about the words a finding names and says nothing about which file they diff --git a/tests/test_documenter_is_governed.py b/tests/test_documenter_is_governed.py new file mode 100644 index 0000000..b761d71 --- /dev/null +++ b/tests/test_documenter_is_governed.py @@ -0,0 +1,1053 @@ +"""The documenter is governed by the check that records what it wrote. + +Before story-154 the documenter could write implementation code and the run +recorded nothing about it: it carried neither `may_only_change` nor +`revert_check`, so there was no path on which the coordinator asked what it +wrote. It now declares both, in both shipped workflows, on the terms the tester +declares its own — and because the documenter's subject is a set of documents +rather than a directory, the confinement's prefix is a workflow token, +`{{architecture_docs}}`, resolved from the target's configuration. + +The subject here is what this repository ships, which is why the runs below go +through the shipped story workflow rather than a built one: the claim is that +*this deployment's* documenter is governed, and that cannot be asked of a +fixture. Every name is still derived — the documenter stage is found by the +token its confinement carries, the artifact names come off its declaration, +the correction-pass entry off the verifier's — and the target underneath is a +repository this module builds, with a real suite the revert check's verdict +comes from. + +The confinement collides with the correction pass, and the resolution is +asserted as runs: the verifier's verdict is the grant. A correctable finding +names a `path`, and at the documenter's post-stage checks the exempt list is +the story's own grants plus that path, for the stage and attempt the pass +record names and no other. + +Every absence asserted here carries a demonstration that it can fail: + + * "a documenter naming only the configured documents runs no revert check" + sits beside the same run naming a module outside them, which runs one; + * "an edit the suite is green without is undone" sits beside the identical + edit with the suite red without it, which is kept and permitted; + * "a granted path is not put to the revert check on the pass" sits beside a + second file the same documenter edited that no finding named, which is; + * "a later attempt inherits no grant" sits beside the pass itself, where the + same file is exempt and the grant is in events.log; + * "an unset key resolves the confinement out" sits beside the same load with + one and with two documents configured; + * "the verifier slot no longer calls the record documentation files" sits + beside the old wording, constructed here, which the same check reports. + +Nothing here invokes a model: every run goes through the fake runner below. +""" +from __future__ import annotations + +import json +import shlex +import subprocess +import sys +from pathlib import Path + +import pytest + +import conftest +import context_assembler +import harness_config +import schema_validator +import story_coordinator +import story_parser +from agent_runner import AgentResult + +REPO_ROOT = conftest.HARNESS_ROOT +STORY_ID = "story-001" + +#: The token a documenter's confinement carries in a shipped definition, and +#: the configuration key it resolves from. The one place the token is spelled; +#: the documenter stage is *found* by it below rather than named. +DOCS_KEY = "architecture_docs" +DOCS_TOKEN = f"{{{{{DOCS_KEY}}}}}" + +#: The workflow the runs below execute. Its name is a name `load_workflow` +#: takes, spelled here once because the subject of the runs is this +#: deployment's own definition. +RUN_WORKFLOW = "story-workflow" + +#: Both definitions this harness ships, read off the directory rather than +#: listed, so a third would join these assertions without an edit here. +SHIPPED = harness_config.workflow_names(REPO_ROOT) + +CONFINEMENT = story_coordinator.CONFINEMENT + + +def raw_definition(name: str) -> dict: + """A shipped definition as it ships — token and all.""" + return json.loads( + (REPO_ROOT / "workflows" / f"{name}.json").read_text(encoding="utf-8")) + + +def stage_confined_to_the_documents(definition: dict) -> dict: + """The one stage whose confinement is the architecture-documents token.""" + stages = [stage for stage in definition["stages"] + if DOCS_TOKEN in stage.get(CONFINEMENT, [])] + assert len(stages) == 1, [stage["name"] for stage in stages] + return stages[0] + + +RAW = raw_definition(RUN_WORKFLOW) +DOCUMENTING_STAGE = stage_confined_to_the_documents(RAW) +DOCUMENTING = DOCUMENTING_STAGE["name"] +DECLARATION = DOCUMENTING_STAGE["revert_check"] +ARTIFACT = DECLARATION["result"] +RECORD = DOCUMENTING_STAGE["changed_files"] +VERIFIER_STAGE = next(stage for stage in RAW["stages"] + if "correction_pass" in stage) +VERIFYING = VERIFIER_STAGE["name"] +CORRECTION = VERIFIER_STAGE["correction_pass"] +ROUTES = VERIFIER_STAGE["on_failure"]["retry_routing"] + +#: The retry category whose route re-enters the documenter, derived from the +#: table rather than spelled, so the "later attempt" run below is a real +#: backward retry to the stage under test. +(DOCUMENTING_CATEGORY,) = [category for category, route in ROUTES.items() + if route["stage"] == DOCUMENTING] + +#: The events the grant and the two revert dispositions append, as the +#: execution-history schema enumerates them. +GRANT_EVENT = "correction-pass-grant-applied" +STORY_GRANT_EVENT = "stage-exception-applied" +REVERTED_EVENT = "revert-check-reverted" +PERMITTED_EVENT = "revert-check-permitted" + +VERDICT_SCHEMA = schema_validator.load_schema("verification-result") +CORRECTION_SCHEMA = schema_validator.load_schema("correction-pass") +HISTORY_SCHEMA = schema_validator.load_schema("execution-history") +RULES = harness_config.load_rules(REPO_ROOT) + + +# -------------------------------------------------------------------------- +# The target: configured documents, a module outside them, a suite over it +# -------------------------------------------------------------------------- + +#: The fixture's own names, each stated once. The document is where the +#: documenter may write; the modules are outside it and so are where every +#: governed case lands; the suite is where the revert check's verdict comes +#: from. +DOCUMENT = ".harness/docs/ARCHITECTURE.md" +SECOND_DOCUMENT = ".harness/docs/DECISIONS.md" +SOURCE_DIR = "src/" +SUITE_DIR = "tests/" +APP = f"{SOURCE_DIR}app.py" +EXAMPLE = f"{SOURCE_DIR}example.py" +OTHER = f"{SOURCE_DIR}other.py" +NEW_MODULE = f"{SOURCE_DIR}helper.py" +NEW_TEST = f"{SUITE_DIR}test_shout.py" + +TEST_COMMAND = shlex.join([sys.executable, "-m", "pytest", + SUITE_DIR.rstrip("/"), "-q", "-p", "no:cacheprovider"]) + +APP_AT_HEAD = '''\ +def greet(name): + return f"hello, {name}" +''' + +#: The helper the suite needs once the test below exists: revert it and the +#: new test cannot import what it calls. +APP_WITH_HELPER = APP_AT_HEAD + ''' + +def shout(name): + return greet(name).upper() +''' + +TEST_APP = '''\ +from app import greet + + +def test_greet(): + assert greet("world") == "hello, world" +''' + +TEST_SHOUT = '''\ +from app import shout + + +def test_shout(): + assert shout("world") == "HELLO, WORLD" +''' + +EXAMPLE_AT_HEAD = '"""An example module. Its docstring say three things."""\n' +EXAMPLE_CORRECTED = '"""An example module. Its docstring says three things."""\n' +OTHER_AT_HEAD = '"""Another module, which no finding names."""\n' +OTHER_REWORDED = '"""Another module, reworded by a pass that was not granted it."""\n' +EXAMPLE_REWORDED_AGAIN = '"""An example module, reworded again on a retry."""\n' +DOCUMENT_AT_HEAD = "# Architecture\n" +DOCUMENT_UPDATED = "# Architecture\n\nThe documenter wrote this.\n" + +ROOT_CONFTEST = '''\ +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "src")) +''' + + +def config_text(documents: list[str]) -> str: + listed = "".join(f" - {document}\n" for document in documents) + return ( + f"workflow: {RUN_WORKFLOW}\n" + "branch_prefix: story/\n" + "permission_mode: acceptEdits\n" + "stories_dir: .harness/stories\n" + "runs_dir: .harness/runs\n" + "logs_dir: .harness/logs\n" + "standards_dir: .harness/standards\n" + f"{DOCS_KEY}:\n{listed}" + f"test_command: {TEST_COMMAND}\n" + f"tests_dir: {SUITE_DIR}\n" + ) + + +def write(path: Path, text: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + + +def write_json(path: Path, payload) -> None: + write(path, json.dumps(payload, indent=2) + "\n") + + +def build_target(root: Path) -> Path: + """A repository configured to run the shipped workflow, with a real suite.""" + for sub in (".harness/standards", ".harness/stories", ".harness/runs", + ".harness/logs"): + (root / sub).mkdir(parents=True) + write(root / ".harness" / "config.yaml", config_text([DOCUMENT])) + 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 / DOCUMENT, DOCUMENT_AT_HEAD) + write(root / "conftest.py", ROOT_CONFTEST) + write(root / APP, APP_AT_HEAD) + write(root / EXAMPLE, EXAMPLE_AT_HEAD) + write(root / OTHER, OTHER_AT_HEAD) + write(root / SUITE_DIR / "test_app.py", TEST_APP) + write(root / ".gitignore", ".pytest_cache/\n__pycache__/\n") + conftest.init_repository(root) + return root + + +@pytest.fixture +def target(tmp_path: Path) -> Path: + return build_target(tmp_path / "governed-documenter-target") + + +def append_to_story(target_root: Path, text: str) -> None: + path = target_root / ".harness" / "stories" / f"{STORY_ID}.yaml" + path.write_text(path.read_text(encoding="utf-8") + text, encoding="utf-8") + conftest.commit_setup(target_root, "the story this test runs") + + +def story_grant(path: str) -> str: + return ( + "\nstage_exceptions:\n" + f" - stage: {DOCUMENTING}\n" + f" create: {path}\n" + " reason: this story's own deliverable is that file\n" + ) + + +# -------------------------------------------------------------------------- +# The documenter's working-tree changes, each with the record for it +# -------------------------------------------------------------------------- + + +def only_the_document(tree: Path) -> dict: + write(tree / DOCUMENT, DOCUMENT_UPDATED) + return {"modified": [DOCUMENT], "created": [], "deleted": []} + + +def unforced_module_edit(tree: Path) -> dict: + """The document, and a module outside it that nothing in the suite needs.""" + write(tree / DOCUMENT, DOCUMENT_UPDATED) + write(tree / APP, APP_WITH_HELPER) + return {"modified": [DOCUMENT, APP], "created": [], "deleted": []} + + +def forced_module_edit(tree: Path) -> dict: + """The same module edit, with a test that needs it. + + The test is outside the confinement too, so the story running this case + grants it; the module edit is not granted, and reverting it alone is what + turns the suite red. + """ + write(tree / APP, APP_WITH_HELPER) + write(tree / NEW_TEST, TEST_SHOUT) + return {"modified": [APP], "created": [NEW_TEST], "deleted": []} + + +def creation_outside(tree: Path) -> dict: + write(tree / NEW_MODULE, "VALUE = 1\n") + return {"modified": [], "created": [NEW_MODULE], "deleted": []} + + +def the_named_file(tree: Path) -> dict: + """The correction the pass asked for, in the file the finding named.""" + write(tree / EXAMPLE, EXAMPLE_CORRECTED) + return {"modified": [EXAMPLE], "created": [], "deleted": []} + + +def the_named_file_again(tree: Path) -> dict: + """A later edit to the file the pass was granted, with different words, so + undoing it is observable beside the pass's edit standing.""" + write(tree / EXAMPLE, EXAMPLE_REWORDED_AGAIN) + return {"modified": [EXAMPLE], "created": [], "deleted": []} + + +def the_named_file_and_another(tree: Path) -> dict: + """The same correction, and a second file no finding named.""" + write(tree / EXAMPLE, EXAMPLE_CORRECTED) + write(tree / OTHER, OTHER_REWORDED) + return {"modified": [EXAMPLE, OTHER], "created": [], "deleted": []} + + +# -------------------------------------------------------------------------- +# Driving a run of the shipped workflow +# -------------------------------------------------------------------------- + +PASS = {"status": "passed", "blocking_issues": [], "unverified": [], + "retry_recommended": False} +EMPTY = {"modified": [], "created": [], "deleted": []} + +#: The finding a pass is routed on. Its `path` is the grant; its category is +#: one the shipped table declares, read off the table rather than spelled. +FINDING = { + "path": EXAMPLE, + "location": f"{EXAMPLE} - the module docstring", + "finding": "MARKER-FINDING the docstring says 'say' for 'says'", + "correction": "MARKER-FINDING-FIX write 'says'", + "category": DOCUMENTING_CATEGORY, +} + + +def passing_with(*findings: dict) -> dict: + return {**PASS, "correctable_findings": [dict(one) for one in findings]} + + +def failing_into(category: str) -> dict: + return { + "status": "failed", + "blocking_issues": [{ + "severity": "high", "issue": "the document is stale", + "location": DOCUMENT, + "required_behavior": "the document describes what shipped", + }], + "unverified": [], "retry_recommended": True, "retry_target": category, + } + + +class Runner: + """A fake agent runner that writes each stage's declared artifacts. + + Every artifact comes off the stage's declaration in the loaded workflow. + The documenter's working-tree change is the case's: `edits` is the list of + edit functions its invocations make, in order — None for an invocation + that changes nothing — so a run that re-enters the documenter can have it + do something different the second time. A pass runs in the attempt its + first entry ran in, and a record in one attempt accounts for everything + the stage changed in it, so the runs below that take a pass have the + first entry change nothing and the pass's record name the pass's edits + alone. Each + invocation's prompt is kept, so what a stage was told on a pass can be + read back beside what it was told on its first entry. + """ + + def __init__(self, target_root: Path, verdicts: list[dict], + edits: list | None = None): + self.target_root = Path(target_root) + self.run_dir = conftest.run_dir_for(self.target_root, STORY_ID) + self.stages = harness_config.load_workflow( + REPO_ROOT, RUN_WORKFLOW, harness_config.load_config(target_root) + )["stages"] + self.verdicts = list(verdicts) + self.edits = list(edits or []) + self.calls: list[str] = [] + self.prompts: dict[str, list[str]] = {} + + def __call__(self, prompt, *, stage, cwd=None, log_path=None, + permission_mode=None, model=None, allowed_tools=None, + max_budget_usd=None, suite_command=None, run_dir=None): + self.calls.append(stage) + self.prompts.setdefault(stage, []).append(prompt) + declaration = next(s for s in self.stages if s["name"] == stage) + edit = self.edits.pop(0) if (stage == DOCUMENTING and self.edits) else None + record = edit(Path(cwd)) if edit else dict(EMPTY) + for artifact in story_coordinator.required_artifacts(declaration): + self._write(artifact, declaration, record) + return AgentResult(ok=True, result_text=f"{stage} done") + + def _write(self, artifact: str, declaration: dict, record: dict) -> None: + path = self.run_dir / artifact + if artifact == conftest.VERIFICATION_RESULT: + verdict = conftest.answering_guidance(self.verdicts.pop(0), + self.run_dir) + write_json(path, verdict) + if verdict.get("retry_recommended"): + write_json(self.run_dir / conftest.RETRY_GUIDANCE, { + "current_focus": [{ + "focus": "bring the document up to date", + "satisfied_when": "it describes what shipped", + }], + "preserve_behavior": ["the existing behavior"], + "retry_scope": [DOCUMENT], + }) + elif artifact == declaration.get("changed_files"): + write_json(path, record) + elif artifact == conftest.TEST_RESULTS: + write_json(path, {"tests_written": 1}) + else: + write(path, f"{artifact} written.\n") + + +def drive(target_root: Path, verdicts: list[dict], edits: list | None = None): + """One run of the shipped workflow: exit code, runner, run directory.""" + runner = Runner(target_root, verdicts, edits) + code = story_coordinator.run_story(STORY_ID, REPO_ROOT, target_root, runner) + return code, runner, runner.run_dir + + +def read_json(path: Path): + return json.loads(path.read_text(encoding="utf-8")) + + +def history_of(run_dir: Path) -> list[dict]: + return read_json(run_dir / "execution-history.json") + + +def events_of_kind(run_dir: Path, kind: str) -> list[dict]: + """History entries of one kind; the schema keys the kind as `event` and + the wording as `message`.""" + return [entry for entry in history_of(run_dir) if entry.get("event") == kind] + + +def documenter_events(run_dir: Path, kind: str) -> list[dict]: + return [entry for entry in events_of_kind(run_dir, kind) + if entry.get("stage") == DOCUMENTING] + + +def events_log(run_dir: Path) -> str: + return (run_dir / "events.log").read_text(encoding="utf-8") + + +def pass_record(run_dir: Path, number: int = 1) -> dict: + return read_json(run_dir / story_coordinator.correction_pass_result_file( + CORRECTION["result"], number)) + + +def content(run_dir: Path, relative: str) -> str: + """A file in the tree the run worked in, which holds its run directory + under the configured runs_dir — so the tree is two levels above it.""" + return (run_dir.parents[2] / relative).read_text(encoding="utf-8") + + +# -------------------------------------------------------------------------- +# The fixture is what it claims to be +# -------------------------------------------------------------------------- + + +def restriction_declarations(definition: dict) -> list[str]: + """Every entry of every restriction declaration across a definition's + stages — the lists a literal path would have to be in to govern anything. + The prose beside them is not read: a `budget_reason` that names the + document by way of explanation predates this story and confines no one.""" + return [entry + for stage in definition["stages"] + for key in (CONFINEMENT, story_coordinator.CREATE_RESTRICTION) + for entry in stage.get(key, [])] + + +def test_the_documenter_is_found_by_its_token_in_both_shipped_workflows(): + """The premise: each shipped definition confines exactly one stage to the + documents, by the token and not by a literal path, and declares the revert + check beside it keyed as the tester's is.""" + for name in SHIPPED: + stage = stage_confined_to_the_documents(raw_definition(name)) + assert stage[CONFINEMENT] == [DOCS_TOKEN] + assert set(stage["revert_check"]) == {"result", "baseline", "discarded"} + assert DOCUMENT not in restriction_declarations(raw_definition(name)) + + +def test_a_literal_path_in_a_restriction_declaration_would_be_seen(): + """The control for the absence above: the same reader over a copy of the + shipped definition with the literal path planted in a restriction list + reports it, so "no literal path" is the declarations and not a reader + that stopped seeing them.""" + planted = raw_definition(RUN_WORKFLOW) + stage = stage_confined_to_the_documents(planted) + stage[CONFINEMENT] = [DOCUMENT] + assert DOCUMENT in restriction_declarations(planted) + # And the other sense is read too: planted there instead, still seen. + also = raw_definition(RUN_WORKFLOW) + implementer = next(stage for stage in also["stages"] + if story_coordinator.CREATE_RESTRICTION in stage) + implementer[story_coordinator.CREATE_RESTRICTION].append(DOCUMENT) + assert DOCUMENT in restriction_declarations(also) + + +def test_the_correction_pass_enters_at_the_documenter(): + """The grant cases below are about the stage a pass enters at, and the + documenter is that stage in the shipped definition.""" + assert story_coordinator.correction_pass_declaration( + RAW["stages"])["stage"] == DOCUMENTING + + +def test_the_target_suite_is_green_as_built_and_red_without_the_helper(tmp_path): + """What the revert check's two verdicts rest on: the suite as built passes, + and with the new test present and the helper absent it fails.""" + root = build_target(tmp_path / "probe") + assert subprocess.run(shlex.split(TEST_COMMAND), cwd=root).returncode == 0 + write(root / NEW_TEST, TEST_SHOUT) + assert subprocess.run(shlex.split(TEST_COMMAND), cwd=root).returncode != 0 + write(root / APP, APP_WITH_HELPER) + assert subprocess.run(shlex.split(TEST_COMMAND), cwd=root).returncode == 0 + + +# -------------------------------------------------------------------------- +# The token resolves the confinement out of configuration +# -------------------------------------------------------------------------- + + +def config_with(documents) -> dict: + """This repository's config with the documents set, or the key removed.""" + config = dict(conftest.repository_config()) + if documents is None: + config.pop(DOCS_KEY, None) + else: + config[DOCS_KEY] = list(documents) + return config + + +def documenter_under(name: str, documents) -> dict: + workflow = harness_config.load_workflow(REPO_ROOT, name, + config_with(documents)) + return next(stage for stage in workflow["stages"] + if stage["name"] == DOCUMENTING) + + +def documenter_restrictions_under(name: str, documents) -> list: + workflow = harness_config.load_workflow(REPO_ROOT, name, + config_with(documents)) + return [restriction for restriction + in story_coordinator.stage_restrictions(workflow["stages"]) + if restriction.stage == DOCUMENTING] + + +@pytest.mark.parametrize("name", SHIPPED) +def test_one_configured_document_resolves_to_exactly_that_path(name): + assert documenter_under(name, [DOCUMENT])[CONFINEMENT] == [DOCUMENT] + assert [r.prefix for r in documenter_restrictions_under(name, [DOCUMENT])] \ + == [DOCUMENT] + + +@pytest.mark.parametrize("name", SHIPPED) +def test_two_configured_documents_resolve_to_both_in_configured_order(name): + """A list-valued token splices its entries in place of the token entry.""" + both = [DOCUMENT, SECOND_DOCUMENT] + assert documenter_under(name, both)[CONFINEMENT] == both + assert documenter_under(name, list(reversed(both)))[CONFINEMENT] \ + == list(reversed(both)) + assert [r.prefix for r in documenter_restrictions_under(name, both)] == both + + +@pytest.mark.parametrize("name", SHIPPED) +@pytest.mark.parametrize("documents", [None, []], ids=["unset", "empty"]) +def test_an_unset_or_empty_key_resolves_the_confinement_out(name, documents): + """No entries at all rather than an empty string, and no restriction for + the documenter. Its control is the two loads above, identical but for the + key, which report one.""" + assert documenter_under(name, documents).get(CONFINEMENT, []) == [] + assert documenter_restrictions_under(name, documents) == [] + assert documenter_restrictions_under(name, [DOCUMENT]) != [] + + +def test_this_repository_resolves_the_shipped_confinement_to_its_own_document(): + for name in SHIPPED: + shipped = conftest.shipped_workflow(name=name) + documenter = next(s for s in shipped["stages"] if s["name"] == DOCUMENTING) + assert documenter[CONFINEMENT] == conftest.repository_config()[DOCS_KEY] + assert "{{" not in json.dumps(shipped) + + +def test_a_key_outside_the_referable_set_is_still_refused_naming_both(tmp_path): + """`branch_prefix` is a declared, set key and stays unreferable; the + refusal lists exactly the two tokens a declaration may carry.""" + definition = raw_definition(RUN_WORKFLOW) + stage_confined_to_the_documents(definition)[CONFINEMENT] = ["{{branch_prefix}}"] + harness = conftest.materialize_workflow(definition, tmp_path / "harness") + config = config_with([DOCUMENT]) + assert config.get("branch_prefix") + + with pytest.raises(harness_config.UnresolvedWorkflowToken) as raised: + harness_config.load_workflow(harness, RUN_WORKFLOW, config) + + assert raised.value.tokens == ["branch_prefix"] + (problem,) = raised.value.problems + assert "{{tests_dir}}" in problem + assert DOCS_TOKEN in problem + assert set(harness_config.workflow_token_values({})) == {"tests_dir", + DOCS_KEY} + + +def test_the_planner_injection_lists_the_documenters_confinement(): + """In the workflow's own wording, beside the other stages' restrictions.""" + rendered = context_assembler.workflow_context( + conftest.shipped_workflow(), RULES)["stage_path_restrictions"] + wordings = [restriction.wording for restriction in + story_coordinator.stage_restrictions( + conftest.shipped_workflow()["stages"])] + documenters = [w for w in wordings if w.startswith(f"{DOCUMENTING} ")] + assert documenters + for wording in wordings: + assert wording in rendered + assert len({w.split(" ")[0] for w in wordings}) >= 3 + + +# -------------------------------------------------------------------------- +# A slash-less confinement prefix is one file +# -------------------------------------------------------------------------- + + +def confinement(*prefixes: str): + (first, *_) = story_coordinator.restrictions_on( + {"name": DOCUMENTING, CONFINEMENT: list(prefixes)}) + return first + + +def test_a_file_prefix_governs_a_path_that_merely_extends_its_name(): + restriction = confinement(DOCUMENT) + assert restriction.governs(f"{DOCUMENT}.bak") + assert not restriction.governs(DOCUMENT) + assert restriction.governs(APP) + + +def test_a_directory_prefix_keeps_its_directory_meaning(): + restriction = confinement(SUITE_DIR) + assert not restriction.governs(f"{SUITE_DIR}test_app.py") + assert not restriction.governs(f"{SUITE_DIR}unit/test_deep.py") + assert restriction.governs(APP) + assert restriction.governs(SUITE_DIR.rstrip("/") + "-more/test.py") + + +def test_the_two_senses_agree_with_grant_covers(): + """What a slash-less confinement admits is what a slash-less grant covers, + so a plan-time grant and a run-time confinement cannot read one value two + ways.""" + for value, path in ((DOCUMENT, DOCUMENT), (DOCUMENT, f"{DOCUMENT}.bak"), + (SUITE_DIR, f"{SUITE_DIR}x.py"), (SUITE_DIR, APP)): + assert (not confinement(value).governs(path)) == \ + story_coordinator.grant_covers([value], path) + + +def test_a_baseline_pathspec_reads_a_file_prefix_as_that_file(tmp_path): + """The capture side of the same reading: a pathspec built from the file + prefix excludes the file and nothing sharing its name as a prefix, and one + built from a directory prefix excludes what is beneath it.""" + specs = story_coordinator.baseline_pathspecs( + story_coordinator.restrictions_on( + {"name": DOCUMENTING, CONFINEMENT: [DOCUMENT]})) + root = tmp_path / "pathspec-probe" + write(root / DOCUMENT, "doc\n") + write(root / f"{DOCUMENT}.bak", "bak\n") + write(root / APP, "app\n") + conftest.init_repository(root) + listed = subprocess.run(["git", "ls-files", "--", *specs], cwd=root, + capture_output=True, text=True, check=True + ).stdout.split() + assert f"{DOCUMENT}.bak" in listed + assert APP in listed + assert DOCUMENT not in listed + + +# -------------------------------------------------------------------------- +# A documenter edit outside the documents is decided by the revert check +# -------------------------------------------------------------------------- + + +def test_a_module_edit_the_suite_is_green_without_is_undone_and_recorded(target): + code, runner, run_dir = drive(target, [PASS], [unforced_module_edit]) + + assert code == 0, runner.calls + record = read_json(run_dir / ARTIFACT) + assert record["ran"] is True + assert record["paths"] == [APP] + assert record["permitted"] is False + # The tree holds the baseline content, the stage's version is kept under + # the discarded directory, and the document edit stands. + assert content(run_dir, APP) == APP_AT_HEAD + assert (run_dir / DECLARATION["discarded"] / APP).read_text( + encoding="utf-8") == APP_WITH_HELPER + assert content(run_dir, DOCUMENT) == DOCUMENT_UPDATED + (event,) = documenter_events(run_dir, REVERTED_EVENT) + assert APP in event["message"] + assert documenter_events(run_dir, PERMITTED_EVENT) == [] + assert runner.calls[-1] == VERIFYING + + +def test_a_module_edit_the_suite_is_red_without_is_kept_and_permitted(target): + """The control for the refusal above: the identical edit to the identical + path, with a test that needs it. The test is granted by the story so it + is not what the check decides; the module edit is, and reverting it alone + turns the suite red.""" + append_to_story(target, story_grant(NEW_TEST)) + + code, runner, run_dir = drive(target, [PASS], [forced_module_edit]) + + assert code == 0, runner.calls + record = read_json(run_dir / ARTIFACT) + assert record["ran"] is True + assert record["paths"] == [APP] + assert record["permitted"] is True + assert record["exit_code"] != 0 + assert content(run_dir, APP) == APP_WITH_HELPER + (event,) = documenter_events(run_dir, PERMITTED_EVENT) + assert APP in event["message"] + assert documenter_events(run_dir, REVERTED_EVENT) == [] + (granted,) = documenter_events(run_dir, STORY_GRANT_EVENT) + assert NEW_TEST in granted["message"] + + +def test_a_documenter_naming_only_the_documents_is_unaffected(target): + """No revert check, no artifact, no event, and the tree as the stage left + it. The two runs above are the control: the same machinery with a module + beside the document decides it.""" + code, runner, run_dir = drive(target, [PASS], [only_the_document]) + + assert code == 0, runner.calls + assert not (run_dir / ARTIFACT).exists() + assert documenter_events(run_dir, REVERTED_EVENT) == [] + assert documenter_events(run_dir, PERMITTED_EVENT) == [] + assert content(run_dir, DOCUMENT) == DOCUMENT_UPDATED + assert runner.calls[-1] == VERIFYING + + +def test_a_documenter_creating_outside_the_documents_escalates(target): + code, runner, run_dir = drive(target, [PASS], [creation_outside]) + + assert code == 2 + assert runner.calls[-1] == DOCUMENTING + (wording,) = [restriction.wording for restriction + in story_coordinator.stage_restrictions(runner.stages) + if restriction.stage == DOCUMENTING] + summary = (run_dir / "escalation-summary.md").read_text(encoding="utf-8") + for text in (events_log(run_dir), summary): + assert DOCUMENTING in text + assert NEW_MODULE in text + assert DOCUMENT in text + assert wording in text + + +def test_the_documenter_is_governed_by_its_declaration_alone(): + """Removing the declaration disables the check with no code change, and no + stage name or repository path is written into the enforcement path.""" + source = Path(story_coordinator.__file__).read_text(encoding="utf-8") + assert f'"{DOCUMENTING}"' not in source + assert f"'{DOCUMENTING}'" not in source + undeclared = {**DOCUMENTING_STAGE} + undeclared.pop(CONFINEMENT) + undeclared.pop("revert_check") + assert story_coordinator.restrictions_on(undeclared) == [] + assert story_coordinator.restrictions_on( + {**DOCUMENTING_STAGE, CONFINEMENT: [DOCUMENT]}) != [] + + +# -------------------------------------------------------------------------- +# The correction pass is granted the files its findings name +# -------------------------------------------------------------------------- + + +@pytest.fixture +def pass_run(target): + """A passing verdict carrying one finding naming EXAMPLE, then a pass on + which the documenter edits that file and one no finding named.""" + return drive(target, [passing_with(FINDING), PASS], + [None, the_named_file_and_another]) + + +def test_the_pass_record_names_the_documenter_and_the_attempt(pass_run): + _, runner, run_dir = pass_run + record = pass_record(run_dir) + assert record["stage"] == DOCUMENTING + assert record["attempt"] == 1 + assert [finding["path"] for finding in record["findings"]] == [EXAMPLE] + assert runner.calls.count(DOCUMENTING) == 2 + + +def test_the_named_file_is_exempt_and_the_other_is_decided(pass_run): + code, _, run_dir = pass_run + + assert code == 0 + record = read_json(run_dir / ARTIFACT) + assert record["paths"] == [OTHER] + assert record["permitted"] is False + (reverted,) = documenter_events(run_dir, REVERTED_EVENT) + assert OTHER in reverted["message"] + assert EXAMPLE not in reverted["message"] + + +def test_the_granted_edit_lands_and_the_other_is_undone(pass_run): + _, _, run_dir = pass_run + assert content(run_dir, EXAMPLE) == EXAMPLE_CORRECTED + assert content(run_dir, OTHER) == OTHER_AT_HEAD + assert (run_dir / DECLARATION["discarded"] / OTHER).read_text( + encoding="utf-8") == OTHER_REWORDED + + +def test_the_grant_is_an_event_before_the_checks_and_reads_like_a_story_grant( + pass_run, +): + _, _, run_dir = pass_run + history = history_of(run_dir) + kinds = [entry.get("event") for entry in history] + (grant,) = documenter_events(run_dir, GRANT_EVENT) + assert EXAMPLE in grant["message"] + assert DOCUMENTING in grant["message"] + assert kinds.index(GRANT_EVENT) < kinds.index(REVERTED_EVENT) + assert f"may change {EXAMPLE}" in events_log(run_dir) + assert schema_validator.validate(history, HISTORY_SCHEMA) == [] + + +def test_the_grant_is_one_file_and_not_its_directory(pass_run): + """A finding grants the file it names: OTHER sits in the same directory + and was decided, so the grant did not widen to `src/`.""" + _, _, run_dir = pass_run + grants = story_coordinator.correction_pass_grants( + run_dir, RAW["stages"], story_coordinator.load_state(run_dir), + DOCUMENTING, 1) + assert grants == [EXAMPLE] + assert not story_coordinator.grant_covers(grants, OTHER) + assert story_coordinator.grant_covers(grants, EXAMPLE) + + +def test_a_first_entry_before_the_pass_is_granted_nothing(target): + """The control for the grant: the documenter's first invocation, before + any pass, edits the same file and is decided on it.""" + code, _, run_dir = drive(target, [PASS], [the_named_file]) + + assert code == 0 + record = read_json(run_dir / ARTIFACT) + assert record["paths"] == [EXAMPLE] + assert record["permitted"] is False + assert documenter_events(run_dir, GRANT_EVENT) == [] + + +def test_a_record_for_another_stage_or_attempt_grants_nothing(pass_run): + _, _, run_dir = pass_run + state = story_coordinator.load_state(run_dir) + assert story_coordinator.correction_pass_grants( + run_dir, RAW["stages"], state, DOCUMENTING, 1) == [EXAMPLE] + assert story_coordinator.correction_pass_grants( + run_dir, RAW["stages"], state, DOCUMENTING, 2) == [] + assert story_coordinator.correction_pass_grants( + run_dir, RAW["stages"], state, VERIFYING, 1) == [] + + +@pytest.fixture +def retried_after_pass(target): + """A pass, then a failing verdict routing a retry that re-enters the + documenter on attempt 2, where it edits the file the pass record named.""" + return drive( + target, + [passing_with(FINDING), failing_into(DOCUMENTING_CATEGORY), PASS], + [None, the_named_file, the_named_file_again]) + + +def test_a_documenter_re_entered_on_a_later_attempt_inherits_no_grant( + retried_after_pass, +): + code, runner, run_dir = retried_after_pass + + assert code == 0, runner.calls + assert runner.calls.count(DOCUMENTING) == 3 + state = story_coordinator.load_state(run_dir) + assert state.retry_count == 1 + assert pass_record(run_dir)["attempt"] == 1 + # One grant, on the pass; none on the retry. + (grant,) = documenter_events(run_dir, GRANT_EVENT) + assert EXAMPLE in grant["message"] + # The retry's edit to the same file was decided and undone, and the + # retry's own words are under the discarded directory. What the tree + # holds afterwards is what the documenter's *first* invocation found: the + # stage baseline is keyed by stage and first-seen content wins, so a + # revert restores the tree from before any of the stage's edits, the + # pass's included. That the pass's words do not survive a reverted retry + # is the baseline rule and not this story's grant, which is bounded to the + # pass it was conferred on. + record = read_json(run_dir / ARTIFACT) + assert record["paths"] == [EXAMPLE] + assert record["permitted"] is False + (reverted,) = documenter_events(run_dir, REVERTED_EVENT) + assert EXAMPLE in reverted["message"] + assert content(run_dir, EXAMPLE) == EXAMPLE_AT_HEAD + assert (story_coordinator.stage_baseline_dir( + run_dir, DECLARATION["baseline"], DOCUMENTING) / EXAMPLE).read_text( + encoding="utf-8") == EXAMPLE_AT_HEAD + assert (run_dir / DECLARATION["discarded"] / EXAMPLE).read_text( + encoding="utf-8") == EXAMPLE_REWORDED_AGAIN + + +def test_the_pass_itself_did_grant_that_file(retried_after_pass): + """The control for the inheritance above: three documenter invocations, + and the documenter's events read grant, then reverted — the pass's edit + to the file was exempt and the retry's was decided. Read in order off the + history, so the one reverted event is shown to be the retry's rather than + the pass's. + + The tree afterwards cannot be the witness, because the retry's revert + restores the stage's first-seen baseline and the pass's words go with it. + What shows the grant was acted on is that the pass's attempt wrote no + revert-check record at all — its archive holds none, while the retry's + attempt holds the live one — and that what was discarded is the retry's + words rather than the pass's.""" + _, runner, run_dir = retried_after_pass + assert runner.calls.count(DOCUMENTING) == 3 + ordered = [entry["event"] for entry in history_of(run_dir) + if entry.get("stage") == DOCUMENTING + and entry.get("event") in (GRANT_EVENT, REVERTED_EVENT, + PERMITTED_EVENT)] + assert ordered == [GRANT_EVENT, REVERTED_EVENT] + assert pass_record(run_dir)["attempt"] == 1 + archived = story_coordinator.attempt_dir(run_dir, 1) + assert archived.is_dir() + assert not (archived / ARTIFACT).exists() + assert (run_dir / ARTIFACT).is_file() + assert (run_dir / DECLARATION["discarded"] / EXAMPLE).read_text( + encoding="utf-8") == EXAMPLE_REWORDED_AGAIN + assert content(run_dir, EXAMPLE) != EXAMPLE_REWORDED_AGAIN + + +# -------------------------------------------------------------------------- +# A finding without a path is refused by both schemas +# -------------------------------------------------------------------------- + + +def without_path(finding: dict) -> dict: + return {key: value for key, value in finding.items() if key != "path"} + + +def test_a_verdict_whose_finding_names_no_path_fails_the_schema(): + assert schema_validator.validate(passing_with(FINDING), VERDICT_SCHEMA) == [] + problems = schema_validator.validate(passing_with(without_path(FINDING)), + VERDICT_SCHEMA) + assert problems + assert any("path" in problem for problem in problems) + + +def test_a_pass_record_whose_finding_names_no_path_fails_the_schema(pass_run): + _, _, run_dir = pass_run + record = pass_record(run_dir) + assert schema_validator.validate(record, CORRECTION_SCHEMA) == [] + broken = {**record, "findings": [without_path(f) for f in record["findings"]]} + problems = schema_validator.validate(broken, CORRECTION_SCHEMA) + assert problems + assert any("path" in problem for problem in problems) + + +@pytest.mark.parametrize("schema, field", [ + (VERDICT_SCHEMA, "correctable_findings"), + (CORRECTION_SCHEMA, "findings"), +], ids=["verdict", "pass record"]) +def test_each_schema_says_what_naming_the_path_grants(schema, field): + item = schema["properties"][field]["items"] + assert "path" in item["required"] + description = item["properties"]["path"]["description"].lower() + assert "grant" in description + assert "revert check" in description + + +# -------------------------------------------------------------------------- +# The prompts +# -------------------------------------------------------------------------- + + +def flowed(text: str) -> str: + return " ".join(text.split()) + + +def rendered_template(target_root: Path, template: str, workflow: str) -> str: + """A shipped template rendered against the fixture target.""" + 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) + config = harness_config.load_config(target_root) + 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=RULES, + workflow=harness_config.load_workflow(REPO_ROOT, workflow, config), + retry_count=0, + ) + return context_assembler.render( + context_assembler.load_template(REPO_ROOT, template), context) + + +def test_the_documenter_prompt_on_a_pass_states_the_grant_and_the_check(pass_run): + _, runner, _ = pass_run + first, on_pass = runner.prompts[DOCUMENTING] + paragraph = flowed(on_pass) + assert "files the findings name are yours to change for this pass" in paragraph + assert "architecture documents always are" in paragraph + assert "put to the revert check" in paragraph + assert "undone if the suite is green without it" in paragraph + assert "approved story artifact is never edited" in paragraph + # The same template carried the same words on the first entry: the + # paragraph is the prompt's, and what the pass changes is the record + # injected beneath it. + assert "put to the revert check" in flowed(first) + assert FINDING["finding"] in on_pass + assert FINDING["finding"] not in first + + +def verifier_of(name: str) -> str: + """The verifier template a shipped workflow names.""" + return next(stage for stage in raw_definition(name)["stages"] + if "correction_pass" in stage)["prompt"] + + +def documenter_record_label(prompt: str) -> str: + """The label introducing the documenter changed-files slot.""" + marker = "Documenter changed files" + assert marker in prompt + start = prompt.index(marker) + return prompt[start:prompt.index(":", start + len(marker))] + + +#: What the slot used to say, written here so the check below is shown to +#: report it. +THE_OLD_LABEL = ("Documenter changed files (documenter's record — documentation " + "files created or modified by the documenter stage)") + + +def calls_them_documentation(label: str) -> bool: + return "documentation" in flowed(label).lower() + + +def test_the_check_for_the_old_wording_reports_it(): + assert calls_them_documentation(THE_OLD_LABEL) + + +@pytest.mark.parametrize("name", SHIPPED) +def test_each_rendered_verifier_prompt_asks_for_path_and_names_every_file( + name, target, +): + rendered = flowed(rendered_template(target, verifier_of(name), name)) + assert "Give each finding a `path`" in rendered + assert "the stage the pass enters at change that one file" in rendered + assert "finding that names no path is refused by the schema" in rendered + label = documenter_record_label(rendered) + assert not calls_them_documentation(label) + assert "every repository file" in flowed(label) + assert "{{" not in rendered diff --git a/tests/test_inspection_findings_reach_the_story.py b/tests/test_inspection_findings_reach_the_story.py index 8954cc9..6ab5d89 100644 --- a/tests/test_inspection_findings_reach_the_story.py +++ b/tests/test_inspection_findings_reach_the_story.py @@ -413,6 +413,7 @@ def passing_with(*findings: dict) -> dict: #: table declares, so a passing verdict carrying it spends a correction pass #: rather than escalating on a category the definition does not define. CORRECTABLE_FINDING = { + "path": CHANGED_FILE, "location": f"{CHANGED_FILE} - the docstring", "finding": "MARKER-CORRECTABLE the wording is stale", "correction": "MARKER-CORRECTABLE-FIX reword it", diff --git a/tests/test_plan_assignment_refusal.py b/tests/test_plan_assignment_refusal.py index 8e2267e..a3481eb 100644 --- a/tests/test_plan_assignment_refusal.py +++ b/tests/test_plan_assignment_refusal.py @@ -317,18 +317,44 @@ def test_a_grant_on_another_stage_does_not_suppress_it(): assert len(plan_validation.assignment_problems(other, STAGES, ABSENT_ROOT)) == 1 +def restrictions_of(stage: str) -> list: + return story_coordinator.restrictions_on( + next(declared for declared in STAGES if declared["name"] == stage)) + + +def candidate_paths_for(stage: str) -> list[str]: + """The paths a stage is asked about, on both sides of its own prefixes. + + The two fixed candidates sit outside and beneath the create restriction's + prefix. A confinement governs the outside, so a stage confined somewhere + neither candidate reaches — the documenter, since story-154, confined to + one configured document — would have nothing ungoverned to be assigned. + So each confinement the stage carries contributes the path inside it, read + off the loaded restriction rather than spelled: the prefix itself when it + names one file, and a file beneath it when it names a directory. + """ + inside = [] + for restriction in restrictions_of(stage): + if restriction.sense != story_coordinator.CONFINEMENT: + continue + prefix = restriction.prefix + inside.append(prefix if not prefix.endswith("/") + else f"{prefix}a_file_no_story_owns.py") + return [OUTSIDE_EVERY_PREFIX, STORY_031_FILE, *inside] + + def ungoverned_by(stage: str) -> str: """A path in this repository no restriction on `stage` governs. Chosen rather than written: "outside every prefix" used to be the same answer for every stage, and a confinement makes it the opposite answer — outside is exactly what a confinement governs. So each stage is asked which - of two candidate paths its own restrictions leave alone, and the pair is on - opposite sides of the declared prefixes so that one of them always is. + of the candidate paths its own restrictions leave alone, and the candidates + are on opposite sides of the declared prefixes so that one of them always + is. """ - restrictions = story_coordinator.restrictions_on( - next(declared for declared in STAGES if declared["name"] == stage)) - for candidate in (OUTSIDE_EVERY_PREFIX, STORY_031_FILE): + restrictions = restrictions_of(stage) + for candidate in candidate_paths_for(stage): if not any(restriction.governs(candidate) for restriction in restrictions): return candidate @@ -352,10 +378,9 @@ def test_a_file_a_restriction_on_that_stage_governs_is_reported(stage): """The control for the row above, over the stages that carry a restriction: the same two candidate paths, and the one its own restriction governs is the one that offends.""" - restrictions = story_coordinator.restrictions_on( - next(declared for declared in STAGES if declared["name"] == stage)) + restrictions = restrictions_of(stage) governed = next( - candidate for candidate in (OUTSIDE_EVERY_PREFIX, STORY_031_FILE) + candidate for candidate in candidate_paths_for(stage) if any(restriction.governs(candidate) for restriction in restrictions)) problems = plan_validation.assignment_problems( plan(entry(governed, stage)), STAGES, ABSENT_ROOT) diff --git a/tests/test_planner_injection.py b/tests/test_planner_injection.py index 877ae65..1fb41f2 100644 --- a/tests/test_planner_injection.py +++ b/tests/test_planner_injection.py @@ -650,9 +650,11 @@ def test_the_workflow_defines_the_four_expected_stages(): The one this module *builds* is what the renders below inject, and what is anchored of it is that it declares four distinct stages and a create restriction on one of them. The one this repository *deploys* is what the - end-to-end `l5-plan` case injects, and its four stages and its one - restriction are named here as they always were — that half is a claim about - what is deployed, which is why this module stays a declared reader of it. + end-to-end `l5-plan` case injects, and its four stages and its + restrictions are named here as they always were — since story-154 the + documenter's confinement on the configured document beside the + implementer's and tester's. That half is a claim about what is deployed, + which is why this module stays a declared reader of it. """ assert len(stage_names()) == 4 assert len(set(stage_names())) == len(stage_names()) @@ -675,7 +677,9 @@ def test_the_workflow_defines_the_four_expected_stages(): for restriction in story_coordinator.stage_restrictions( deployed["stages"])] == [ ("implementer", story_coordinator.CREATE_RESTRICTION, "tests/"), - ("tester", story_coordinator.CONFINEMENT, "tests/")] + ("tester", story_coordinator.CONFINEMENT, "tests/"), + ("documenter", story_coordinator.CONFINEMENT, + ".harness/docs/ARCHITECTURE.md")] def test_the_template_names_no_workflow_stage_of_its_own(): @@ -963,10 +967,14 @@ def test_l5_plan_injects_the_workflow_facts_into_the_session_prompt( assert argv is not None prompt = argv[argv.index("--append-system-prompt") + 1] # `l5-plan` resolves its own harness root, so the facts it injects are the - # ones this repository deploys rather than the ones built above. The - # expectation is read off that definition, so the assertion compares the - # render against what the run actually had to inject. - deployed = conftest.shipped_workflow(REPO_ROOT, "story-workflow") + # ones this repository deploys rather than the ones built above, resolved + # against the *project's* configuration — a restriction written as a token + # resolves to what that target configures, and one whose key the target + # leaves unset resolves out. The expectation is read off that same load, + # so the assertion compares the render against what the run actually had + # to inject. + deployed = harness_config.load_workflow( + REPO_ROOT, "story-workflow", harness_config.load_config(project)) deployed_names = [stage["name"] for stage in deployed["stages"]] deployed_restrictions = story_coordinator.stage_restrictions( deployed["stages"]) diff --git a/tests/test_refactor_workflow.py b/tests/test_refactor_workflow.py index 0186f2f..f1a2bbe 100644 --- a/tests/test_refactor_workflow.py +++ b/tests/test_refactor_workflow.py @@ -105,15 +105,21 @@ def test_it_declares_no_stage_that_authors_validation(): assert "tester" not in REFACTOR_NAMES -#: Every key that governs where a stage may write, together with the key that -#: turns the check deciding an edit outside it on. Read off the coordinator's +#: Every key that governs where a stage may write. Read off the coordinator's #: own vocabulary rather than spelled here, so a sense added to it is a sense #: the assertion below covers rather than one it silently stops watching. GOVERNING_KEYS = (story_coordinator.CREATE_RESTRICTION, - story_coordinator.CONFINEMENT, "revert_check") + story_coordinator.CONFINEMENT) -def test_no_refactor_stage_declares_any_key_that_governs_where_it_writes(): +def keys_governing_the_tests_directory(stage: dict) -> list[str]: + """The governing keys on `stage` whose declaration names the configured + tests directory — the one location a refactor's implementer must be free + to edit, and so the one guard this workflow exists to drop.""" + return [key for key in GOVERNING_KEYS if TESTS_DIR in stage.get(key, [])] + + +def test_no_refactor_stage_declares_a_key_governing_the_tests_directory(): """The declarations that rest on the assumption a refactor breaks. Over every stage rather than the implementer alone, and over every sense @@ -122,26 +128,46 @@ def test_no_refactor_stage_declares_any_key_that_governs_where_it_writes(): stage confined to the tests directory would be governed everywhere else, which is exactly the guard this workflow exists to drop. - The control is story-workflow, which declares each of them somewhere: if - the reading had stopped seeing a declaration at all, the two definitions - would look identical and the absence would mean nothing. + What this is not is a claim that no refactor stage is governed anywhere. + Since story-154 the documenter of both definitions is confined to the + configured architecture documents, under the revert check, because its + subject is those documents and a refactor changes nothing about that; the + guard a refactor drops is the one on the tests directory, and that is the + guard asserted absent here, on every stage and under every sense. + + The control is story-workflow, which declares each sense on the tests + directory somewhere: if the reading had stopped seeing a declaration at + all, the two definitions would look identical and the absence would mean + nothing. """ assert not [(stage["name"], key) for stage in REFACTOR_STAGES - for key in GOVERNING_KEYS if key in stage] + for key in keys_governing_the_tests_directory(stage)] for key in GOVERNING_KEYS: assert [stage["name"] for stage in STORY_WORKFLOW["stages"] - if key in stage], key + if key in keys_governing_the_tests_directory(stage)], key + # A refactor stage that does declare the revert check is one whose + # restriction names something other than the tests directory. + for stage in REFACTOR_STAGES: + if "revert_check" in stage: + assert keys_governing_the_tests_directory(stage) == [] + assert any(stage.get(key) for key in GOVERNING_KEYS), stage["name"] -def test_no_refactor_stage_is_governed_anywhere_by_the_shared_derivation(): +def test_no_refactor_stage_is_governed_on_the_tests_directory_by_the_shared_derivation(): """The same absence asked of the derivation the coordinator enforces from. Reading the keys is a statement about the file; this is a statement about what a run would enforce, which is what the criterion is about. Its control - is the shipped story workflow, whose stages do derive restrictions. + is the shipped story workflow, whose stages do derive restrictions on the + tests directory. And what the refactor workflow does derive is a subset of + what the story workflow derives, so the refactor drops guards and adds + none. """ - assert story_coordinator.stage_restrictions(REFACTOR_STAGES) == [] - assert story_coordinator.stage_restrictions(STORY_WORKFLOW["stages"]) != [] + refactor = story_coordinator.stage_restrictions(REFACTOR_STAGES) + story = story_coordinator.stage_restrictions(STORY_WORKFLOW["stages"]) + assert [r for r in refactor if r.prefix == TESTS_DIR] == [] + assert [r for r in story if r.prefix == TESTS_DIR] != [] + assert set(refactor) <= set(story) def test_the_refactor_implementer_declares_the_census_and_the_suite_run(): diff --git a/tests/test_shipped_workflow_is_valid.py b/tests/test_shipped_workflow_is_valid.py index 15b8b29..ee5ea77 100644 --- a/tests/test_shipped_workflow_is_valid.py +++ b/tests/test_shipped_workflow_is_valid.py @@ -445,9 +445,9 @@ def raw_definition() -> dict: def test_the_implementer_may_not_create_the_configured_tests_directory(): - """The separation the two-stage split exists for, and the one configuration - reference in this workflow: the restriction is written as `{{tests_dir}}` - and resolves to what this repository configures.""" + """The separation the two-stage split exists for: the restriction is + written as `{{tests_dir}}` and resolves to what this repository + configures.""" restrictions = story_coordinator.stage_restrictions(SHIPPED_STAGES) tests_dir = conftest.repository_config()["tests_dir"] creation = [restriction for restriction in restrictions @@ -462,13 +462,18 @@ def test_the_implementer_may_not_create_the_configured_tests_directory(): def confined_stage_declaration(definition: dict) -> dict: """The stage that writes the validation, out of a definition. - Found by the restriction that confines it rather than by its name, so this - names no stage of its own; the assertions below then read that stage's - other declarations. + Found by the restriction that confines it to the configured test location + rather than by its name, so this names no stage of its own; the assertions + below then read that stage's other declarations. Since story-154 the + definition carries a second confinement, the documenter's to the + configured architecture documents, which is why the confinement is picked + by its prefix rather than being the only one. """ + tests_dir = conftest.repository_config()["tests_dir"] confinements = [restriction for restriction in story_coordinator.stage_restrictions(SHIPPED_STAGES) - if restriction.sense == story_coordinator.CONFINEMENT] + if restriction.sense == story_coordinator.CONFINEMENT + and restriction.prefix == tests_dir] (confinement,) = confinements return next(stage for stage in definition["stages"] if stage["name"] == confinement.stage) @@ -481,10 +486,12 @@ def test_the_stage_that_writes_the_validation_is_confined_to_that_location(): exactly as the create restriction does — a target keeping its tests elsewhere is governed there under both with no workflow edit. """ + confined = confined_stage_declaration(raw_definition()) confinement = next( restriction for restriction in story_coordinator.stage_restrictions(SHIPPED_STAGES) - if restriction.sense == story_coordinator.CONFINEMENT) + if restriction.sense == story_coordinator.CONFINEMENT + and restriction.stage == confined["name"]) assert confinement.prefix == conftest.repository_config()["tests_dir"] assert confinement.stage != SHIPPED_NAMES[0] diff --git a/workflows/refactor-workflow.json b/workflows/refactor-workflow.json index 9cc5115..8dca539 100644 --- a/workflows/refactor-workflow.json +++ b/workflows/refactor-workflow.json @@ -29,6 +29,12 @@ "prompt": "documenter.md", "outputs": ["documentation-report.md", "documenter-changed-files.json"], "changed_files": "documenter-changed-files.json", + "may_only_change": ["{{architecture_docs}}"], + "revert_check": { + "result": "documenter-revert-check-result.json", + "baseline": "stage-baseline", + "discarded": "documenter-discarded-edits" + }, "may_not_run_suite": true, "max_self_routes": 1, "max_execution_cost_usd": 10, diff --git a/workflows/story-workflow.json b/workflows/story-workflow.json index b885a9a..ae921a4 100644 --- a/workflows/story-workflow.json +++ b/workflows/story-workflow.json @@ -53,6 +53,12 @@ "prompt": "documenter.md", "outputs": ["documentation-report.md", "documenter-changed-files.json"], "changed_files": "documenter-changed-files.json", + "may_only_change": ["{{architecture_docs}}"], + "revert_check": { + "result": "documenter-revert-check-result.json", + "baseline": "stage-baseline", + "discarded": "documenter-discarded-edits" + }, "may_not_run_suite": true, "max_self_routes": 1, "max_execution_cost_usd": 10,