diff --git a/docs/blazor-route-parity.md b/docs/blazor-route-parity.md index 7ffffb6e1..830134e70 100644 --- a/docs/blazor-route-parity.md +++ b/docs/blazor-route-parity.md @@ -49,9 +49,8 @@ parameterized routes. Together they cover every SPA-ready route (all rows former | `/settings/ui` | `/app/settings/general` | | `/settings/xmltv` | `/app/settings/xmltv` | -Note: **the settings sub-routes above are already redirected**, not Blazor-only as an earlier draft -of this table implied — `LegacyUiRedirects.cs` covers all seven `/settings/*` pages that have SPA -equivalents. There is no Blazor-only settings sub-route left except the ones with no SPA screen at +Note: **the settings sub-routes above are already redirected** — `LegacyUiRedirects.cs` covers all +seven `/settings/*` pages that have SPA equivalents. There is no Blazor-only settings sub-route left except the ones with no SPA screen at all (there are none currently — every Blazor `Settings/*.razor` page has both an SPA screen and a redirect). @@ -227,7 +226,7 @@ Nothing remains here. The playback troubleshooting screen (`/app/troubleshooting ### Media sources — #202 DONE (2026-07-11) -Nothing remains here. Found by the #91 cold review (2026-07-09): the SPA `/app/libraries` screen +Nothing remains here. As of the #91 sweep (2026-07-09): the SPA `/app/libraries` screen listed sources/libraries and could trigger scans, but source add/edit, connection parameters, path replacements, and library enable/disable had **no SPA UI and no write REST API** (`MediaSourcesController` was GET-only; "Add Source" was a hardcoded disabled stub) — a fresh diff --git a/docs/ci-cd.md b/docs/ci-cd.md index bf18b0b3c..5e9f4666f 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -819,7 +819,7 @@ checkout + detect and **reports `success` in seconds**. out shallow (`fetch-depth: 2`), and a shallow clone has **no `origin/` tracking ref and no merge-base** — so a three-dot `origin/main...HEAD` diff *errors*, the fail-safe returns `docs_only=false`, and the skip silently -never fires (the first cut shipped this bug — every docs-only PR still ran the full matrix; caught by +never fires (this bug shipped once — every docs-only PR still ran the full matrix; caught by #416's "verify on a real PR" box). The script therefore fetches the base and diffs against **`FETCH_HEAD`** (always written by `git fetch`, resolves in a shallow clone) with a **two-dot** tree diff (`git diff --no-renames FETCH_HEAD HEAD`) — no merge-base required. (`api-docs`/`format` avoided @@ -943,12 +943,12 @@ Two independent mechanisms hold it, and neither is redundant: **Why a job and not a step inside `build`.** A step cannot protect the job it lives in. `build` is what publishes, so a guard step there fails **open** if the runner drops it — and the defence ("the guard's own body has no opener, so it cannot be dropped") is circular when the only thing - enforcing that property is the same PR-only test being backstopped. This was the first design and - two independent reviews rejected it for exactly that. + enforcing that property is the same PR-only test being backstopped. That was the first design, and + it was rejected for exactly that. **Why it runs the real pytest and not a bespoke scanner.** The same first cut hand-parsed the - workflow YAML in stdlib Python, to avoid provisioning PyYAML on `build`'s bare runner. Review found - ~10 **false negatives** in that parser in one round — flow mappings (`{run: …}`), a quoted + workflow YAML in stdlib Python, to avoid provisioning PyYAML on `build`'s bare runner. That parser + yielded ~10 **false negatives** in one round — flow mappings (`{run: …}`), a quoted `"run":` key, aliases, multiline quoted scalars — making it strictly *weaker* than the check it backstopped, in the only direction that matters for a security gate. Running the existing test needs no second definition of "what is a `run:` body", so it has no drift surface at all. `scan` @@ -979,13 +979,13 @@ Two independent mechanisms hold it, and neither is redundant: command" (the step also runs the wiring tests; the script runs only the ban file) and not merely "fails if it passes": any other outcome — exit 5 because everything was deselected, exit 2 on a collection error, or a failure in some unrelated test — is a **refusal**, not a pass. That - exhaustiveness is the load-bearing part, because each weaker reading was a live bug in an earlier - draft: "non-zero means it noticed" vouched for the gate on a collection error, and the deselection - disarm this script exists to catch exits **5**, not 1. + exhaustiveness is the load-bearing part, because each weaker reading was a live bug here: + "non-zero means it noticed" vouched for the gate on a collection error, and the deselection disarm this script + exists to catch exits **5**, not 1. It runs in the **real checkout** — an isolated copy does not inherit the repo-root config a disarm - would live in, which is exactly how the first version of that script reported healthy while the - real invocation was deselected. The workflow file is + would live in, so a copy-based run reports healthy while the real invocation is deselected — which + has happened here. The workflow file is restored by an `EXIT` trap, and the step carries a `selfcheck` marker so dropping it reddens the job. @@ -1008,9 +1008,9 @@ Two independent mechanisms hold it, and neither is redundant: interpolation drop. It moves the terminal assumption — to fail open you must now drop the pytest step **and** the assert step, rather than either one alone. - Measuring a guard on this path does **not** require cutting a release, and an earlier draft here - claiming it would was simply wrong: `build` runs on every push to `main` - (`if: github.event_name != 'pull_request'`), and a `workflow_dispatch` on any other ref runs the + Measuring a guard on this path does **not** require cutting a release: `build` runs on every + push to `main` (`if: github.event_name != 'pull_request'`), and a `workflow_dispatch` on any + other ref runs the job while `Build and push` publishes nothing (its `push:` is gated on `main`/`v*`). That is how #767 was verified — see the decision record for the run ids. @@ -1078,8 +1078,8 @@ resolved identity on stdout**, where capture is not in question, and the answer this change's own run: `Marker identity: job=test run=1916 attempt=1 (from the runner)`, and the same for `migrations`. Both required jobs, on the lane that matters. -That measurement is what promoted it from warn-and-default to required, and it is why the residual -this paragraph used to describe — a rerun inheriting attempt 1's markers — no longer exists. The +That measurement is what promoted it from warn-and-default to required, and it is what rules out a +rerun inheriting attempt 1's markers. The identity line stays, as the standing evidence a future reader checks first if the keying is ever doubted again. @@ -1670,16 +1670,16 @@ re-measured either and is 1.25.4-dated too. Separately, after posting an exemption `success` the job re-reads the per-POST status history and, if a human `Review-verdict:` row appeared during the write window, overwrites its own status with -`pending` and logs an error. **That check runs only for a `success` write**, so "a human `BLOCKED` can -never be silently turned green" is too strong and is retracted: a generic `pending` write is not -verified, and a rejection it masks is re-derived green by a later run (ersatztv#849). The repair is +`pending` and logs an error. **That check runs only for a `success` write**, so it does not extend to +a generic `pending` write: that write is not verified, and a rejection it masks is re-derived green +by a later run (ersatztv#849). The repair is `pending`, never a copy of the human's verdict, which would attribute a human decision to the job. **Both `/statuses/{sha}` reads are PAGED** (ersatztv#763). The history is read twice — before the write for the high-water mark, after it for the race check — and `limit` clamps to `MAX_RESPONSE_ITEMS` (measured 50), so a single read of a busy head returned a partial list. -**Be precise about what that cost**, because ersatztv#763's own framing overstated it. Under the server +**Be precise about what that cost** — ersatztv#763's framing of it is too strong. Under the server default (`created_unix DESC`) page 1 holds the *newest* rows and ids are monotonic with `created_at`, so page 1 already carried both the true maximum id and every row newer than the mark — the only rows the post-write check selects on. A single-page read therefore missed a raced verdict only if **more @@ -1866,9 +1866,8 @@ Three things now hold the line, and they are deliberately different in kind: historical defect (`pr number`) and a nonexistent context, but **not** a syntactically invalid payload whose tokens are all known (`${{ github.ref == }}` passes), nor a renamed output (`steps.metadata.outputs.shortsha` passes — every token after the first is preceded by `.` and is - skipped), nor an unclosed opener. Catching those needs an expression parser. An earlier draft of - this section claimed it caught "a payload that cannot evaluate, wherever it sits"; that was false, - and the corrected claim is the one to rely on. + skipped), nor an unclosed opener. Catching those needs an expression parser — it does **not** + catch "a payload that cannot evaluate, wherever it sits". Worth knowing why nothing caught this for three days: every *other* workflow-shape test in that file reads `_code_lines()`, which strips comments. That is correct for what it was for, but it encodes the @@ -1894,8 +1893,8 @@ Two things kept it invisible, and both are worth generalising: `array`-only gate was never exercised by the suite either. With the double corrected and the old gate restored, **most of the fence suite fails** — 18 tests when first measured at `c710db4a1`, 21 once three more fence-dependent tests existed. The invariant is the point, not the count: they had - all been passing for the wrong reason. (Given as a range on purpose — an earlier draft cited a bare - "18", which was stale two commits later, inside a section about stale claims.) When a double claims + all been passing for the wrong reason. (Given as a range on purpose: a bare count goes stale within + a couple of commits, inside a section about stale claims.) When a double claims fidelity, that claim is a test assertion and needs re-measuring like any other. The type is now read as a value (`case` over `jq -r 'type'`) rather than through `jq -e`, whose @@ -1904,12 +1903,11 @@ walk. The regression test is parameterised over both shapes because both are liv An empty page — `null` OR `[]`, which take the SAME rule since ersatztv#803 — is accepted as exhaustion only from **page 2 on** — every real PR's first page carries events (spot-checked non-empty across #752/#753/#749/#739/#717; the counts are deliberately not recorded here -because timelines grow and an earlier draft's five figures were stale within days), so an empty first +because timelines grow and such figures go stale within days), so an empty first page is ANOMALOUS rather than EXHAUSTED, and the walk should not certify "no retarget happened" from a response it cannot explain. -**The same nil-slice shape bites `/commits/{sha}/status`** — a third instance, found by cold review of -the fix for the second. A head with no statuses yet returns +**The same nil-slice shape bites `/commits/{sha}/status`** — a third instance. A head with no statuses yet returns `{"state":"pending","total_count":0,"statuses":null}` (measured on PR #739's head). `read_existing_verdict` gated on `.statuses | type == "array"`, so it hit its `exit 1` and posted nothing at all — fail-closed, same user-visible outcome. `null` is now accepted there only when `total_count` is 0, so a body that @@ -1931,7 +1929,7 @@ proved empty: at the server-wide `MAX_RESPONSE_ITEMS`, **measured at 50** (`/issues?limit=100` returns 50). A response can therefore never carry 100 rows, and the comparison was **dead code**. The repo already documented that cap in `scripts/pr-changed-files.sh`, two test files and `ci.script-tests-job`; the - guard was written against 100 anyway, and a cold review caught it. Hardcoding 50 instead would + guard was written against 100 anyway. Hardcoding 50 instead would re-break the day the setting changes. So the job **asks the server, and only when it matters**: if the `review-verdict/h10` row is on page 1 diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 85dd76ade..fa44153fd 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -84,7 +84,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `docs.decision-one-file-per-record` | Each decision record is its own file at `docs/decisions/records//.md` (archived ones at `docs/decisions/archive//.md`) with YAML frontmatter; the filename IS the key, so one-active-record-per-key is a filesystem property rather than a validator check, and supersession is a `git mv`. | 2026-07-25 | [link](records/docs/decision-one-file-per-record.md) | | `docs.decision-optional-provenance` | Decision records gain two OPTIONAL fields — `stale-after: YYYY-MM-DD` on the metadata line and a `**Sources:**` line in the metadata block; the Open Knowledge Format (OKF) itself is NOT adopted as the record format. | 2026-07-25 | [link](records/docs/decision-optional-provenance.md) | | `docs.frontmatter-pyyaml-crosscheck` | `decisions_validate.py` runs `pyyaml_frontmatter_faults()` over every record-wing file: it loads the frontmatter with PyYAML and reports an ERROR when PyYAML rejects the document OR when any key's value differs from what the dependency-free `dl._read_frontmatter` read. PyYAML is the WRITER of these files (`migrate_decisions_split.render_record` emits them with `yaml.safe_dump`), so on any disagreement PyYAML is authoritative and the defect is in the FILE, not in either parser. The check is strictly additive: when PyYAML is not importable it is SKIPPED and `main()` says so with a `::notice::`, never silently — the read path stays dependency-free because `decisions-guard`, the Husky hooks and contributor machines install nothing. The comparison has exactly ONE implementation, called by both the validator and `test_frontmatter_reader_matches_pyyaml_on_every_real_record`, so the suite and the tool cannot drift on what "matches PyYAML" means. | 2026-08-04 | [link](records/docs/frontmatter-pyyaml-crosscheck.md) | -| `docs.no-session-narrative` | Every durable artifact — an in-repo `docs/` page, a skill, a README, a code comment, an Obsidian vault page — records the END STATE. The path to that end state goes in the commit message, the Gitea issue, or the issue's `## Closing record`; it does not go in the artifact. Concretely: **a review finding is answered in the commit message, and only the corrected claim enters the doc.** Naming the destination is load-bearing — "do not write it in the doc" with no home loses the knowledge, and this repo has the inverse failure on record too (#542, where a pruned narrative turned out to be the only copy). THE TEST IS WHO BENEFITS: if only the author's timeline explains why a sentence is there, it is narrative and belongs in the commit; if a reader who never saw the session would act differently knowing it, it is a finding and stays. Session narrative reads as: first person or session chronology ("I initially thought", "an earlier draft counted", "my first attempt returned 0"), a correction of a belief the reader never held ("this was wrong, actually X" where only X matters), relative time ("earlier today", "currently investigating"), or a blow-by-blow diagnosis standing in place of the conclusion. THE CARVE-OUT, which must be stated or the rule gets over-applied — reader-facing history that must survive: a decision record's `supersedes`/`superseded-by`; a dated measurement or an explicitly stated snapshot boundary; a TESTED-AND-REJECTED negative result, kept so nobody re-proposes it on plausibility; the *why* behind a non-obvious choice; and a trap together with its consequence. `docs/decisions/records/**` and `docs/decisions/archive/**` are exempt WHOLESALE: a record narrating how a rule was got wrong is carrying the rationale it exists to carry. ENFORCEMENT IS ADVISORY ONLY — `scripts/check-doc-narrative.py`, run non-blocking from the `docs-reminder` job over ADDED lines. It is a string predicate over prose and may never become a blocking gate. | 2026-08-21 | [link](records/docs/no-session-narrative.md) | +| `docs.no-session-narrative` | Every durable artifact — an in-repo `docs/` page, a skill, a README, a code comment, an Obsidian vault page — records the END STATE. The path to that end state goes in the commit message, the Gitea issue, or the issue's `## Closing record`; it does not go in the artifact. Concretely: **a review finding is answered in the commit message, and only the corrected claim enters the doc.** Naming the destination is load-bearing — "do not write it in the doc" with no home loses the knowledge, and this repo has the inverse failure on record too (#542, where a pruned narrative turned out to be the only copy). THE TEST IS WHO BENEFITS: if only the author's timeline explains why a sentence is there, it is narrative and belongs in the commit; if a reader who never saw the session would act differently knowing it, it is a finding and stays. Session narrative reads as: first person or session chronology ("I initially thought", "an earlier draft counted", "my first attempt returned 0"), a correction of a belief the reader never held ("this was wrong, actually X" where only X matters), relative time ("earlier today", "currently investigating"), or a blow-by-blow diagnosis standing in place of the conclusion. THE CARVE-OUT, which must be stated or the rule gets over-applied — reader-facing history that must survive: a decision record's `supersedes`/`superseded-by`; a dated measurement or an explicitly stated snapshot boundary; a TESTED-AND-REJECTED negative result, kept so nobody re-proposes it on plausibility; the *why* behind a non-obvious choice; and a trap together with its consequence. `docs/decisions/records/**` and `docs/decisions/archive/**` are exempt WHOLESALE: a record narrating how a rule was got wrong is carrying the rationale it exists to carry. `docs/superpowers/**` — the dated plans and specs — IS reached by the rule (#812), and is NOT exempt from the detector either. The narrative sites IDENTIFIED there were remediated with the rest of the corpus — identified, not all, since no sweep here claims closure over a phrasing space. The one surviving detector hit is a carve-out on its merits, keeping a rejected design together with the concrete harm that killed it. A wholesale detector exemption for that path was proposed and REJECTED, recorded so it is not re-proposed on plausibility: the argument was that all 35 files are frozen (measured 2026-08-29, before this change: nothing had been edited there since 2026-07-23), so sweeping them yields only false positives. But `--diff`, the mode CI runs, scans ADDED lines, and a frozen file contributes none — so the exemption buys nothing where the check actually runs, and what it WOULD suppress is any plan being written OR REVISED now — 15 of the 35 have more than one commit, so revisions do happen — which is exactly the case where the remedy (move it to the commit message) is still available. It costs the detector's only reach to buy quiet in a sweep a person runs deliberately. ENFORCEMENT IS ADVISORY ONLY — `scripts/check-doc-narrative.py`, run non-blocking from the `docs-reminder` job over ADDED lines. It is a string predicate over prose and may never become a blocking gate. | 2026-08-21 | [link](records/docs/no-session-narrative.md) | | `docs.record-wing-parse-guard` | `decisions_validate.py` asserts, per PATH, that every `*.md` under `docs/decisions/records/**` and `docs/decisions/archive/**` parses to exactly one record carrying a `key` — an ERROR, not a warning, since a file in the record wings that is not a record is a mistake by definition. A file sitting DIRECTLY in `archive/` is exempt only when it actually looks like a #610 stripped index — exactly one keyless record with a known generated heading — never merely by living there. The one other exemption, `archive/README.md`, is by exact RELATIVE PATH; nothing is ever exempt by BASENAME, since that would exempt the same filename in the active wing too. `_read_frontmatter` is deliberately NOT extended to accept YAML block scalars: every record value goes on ONE line, and the structural check is what makes that limitation loud instead of silent. | 2026-07-26 | [link](records/docs/record-wing-parse-guard.md) | | `docs.tracker-comment-retrofit` | When the knowledge exporter flags an over-cap tracker issue and excludes it from ingestion, triage its comments instead of assuming a retrofit is owed — and for each decision-shaped item check the **worked issue first**, because a tracker session comment is by construction a précis of the fuller closing record posted on the issue it narrates. Applied to #237 (111 comments) this yielded **zero** records, so server-management#642's "a fact found only in a #237 comment" retrieval row has no valid subject and its interim target (an already-migrated record) is permanent. | 2026-07-21 | [link](records/docs/tracker-comment-retrofit.md) | | `ffmpeg.external-logo-graphics-engine` | External-URL channel logos pass through to the graphics engine like any other watermark source; `WatermarkSelector` must never gate them on `File.Exists` (always false for a URL) and never route them through the ffmpeg-native overlay shortcut. | 2026-07-20 | [link](records/ffmpeg/external-logo-graphics-engine.md) | diff --git a/docs/decisions/records/docs/no-session-narrative.md b/docs/decisions/records/docs/no-session-narrative.md index df8cb9f83..005c417ff 100644 --- a/docs/decisions/records/docs/no-session-narrative.md +++ b/docs/decisions/records/docs/no-session-narrative.md @@ -5,9 +5,9 @@ status: active since: '2026-08-21' supersedes: none superseded-by: none -rule: 'Every durable artifact — an in-repo `docs/` page, a skill, a README, a code comment, an Obsidian vault page — records the END STATE. The path to that end state goes in the commit message, the Gitea issue, or the issue''s `## Closing record`; it does not go in the artifact. Concretely: **a review finding is answered in the commit message, and only the corrected claim enters the doc.** Naming the destination is load-bearing — "do not write it in the doc" with no home loses the knowledge, and this repo has the inverse failure on record too (#542, where a pruned narrative turned out to be the only copy). THE TEST IS WHO BENEFITS: if only the author''s timeline explains why a sentence is there, it is narrative and belongs in the commit; if a reader who never saw the session would act differently knowing it, it is a finding and stays. Session narrative reads as: first person or session chronology ("I initially thought", "an earlier draft counted", "my first attempt returned 0"), a correction of a belief the reader never held ("this was wrong, actually X" where only X matters), relative time ("earlier today", "currently investigating"), or a blow-by-blow diagnosis standing in place of the conclusion. THE CARVE-OUT, which must be stated or the rule gets over-applied — reader-facing history that must survive: a decision record''s `supersedes`/`superseded-by`; a dated measurement or an explicitly stated snapshot boundary; a TESTED-AND-REJECTED negative result, kept so nobody re-proposes it on plausibility; the *why* behind a non-obvious choice; and a trap together with its consequence. `docs/decisions/records/**` and `docs/decisions/archive/**` are exempt WHOLESALE: a record narrating how a rule was got wrong is carrying the rationale it exists to carry. ENFORCEMENT IS ADVISORY ONLY — `scripts/check-doc-narrative.py`, run non-blocking from the `docs-reminder` job over ADDED lines. It is a string predicate over prose and may never become a blocking gate.' -signals: 'no session narrative in docs · the reader never saw the earlier draft · who-benefits test · answer a review finding in the commit message not the doc · end state versus path to it · earlier draft · first version of this table · my first attempt · relative time in a doc · carve-out for dated measurement and snapshot boundary · tested-and-rejected negative result · decision records are exempt on purpose · advisory not blocking · string predicate over prose · paths: `scripts/check-doc-narrative.py`, `.gitea/workflows/pr-checks.yml`, `docs/handoffs/chicorytv-issue-queue.md`, `docs/defect-shapes-773.md` · issues: #784, #773, #767, #743, #542' -mechanics: 'The RULE is about every durable artifact; the DETECTOR''s population is narrower and is stated here so a row in it never reads as coverage it does not have. `scripts/check-doc-narrative.py` scans `docs/**/*.md` minus `docs/decisions/**` (exempt wholesale, in both modes), plus root-level `*.md`. Skills under `.claude/`, `web/`, and every other nested markdown file outside `docs/` are OUT of its scope and stay a human judgement. It warns and exits 0 on every path — bad argument, unresolvable ref, unreadable file, unhandled exception — asserted per argument shape in `scripts/tests/test_check_doc_narrative.py`, not only in prose, EXCEPT the unhandled-exception arm, which is a bare `except` no test exercises and is recorded as unproven rather than implied. The invariant is held at the JOB level too (`continue-on-error` on both steps): a script that returns 0 does not keep a job green if a setup action fails. Every knob DEMONSTRATED to break the parse has a row in `FORMAT_KNOBS` in the test file; the user and system config files are removed from the picture entirely so an unnamed one cannot reach it. Three separate review rounds each found ONE more knob turning a real hit into a clean-looking `scanned 0 file(s)`, so the third fix removed the surface rather than naming a fourth knob — a completeness claim over the knob space would be exactly the enumeration that failed three times. COVERAGE BOUNDARY, written once so it is not rediscovered one clause at a time, and naming the clauses rather than a category because a category is where the last mis-sort hid. MUTATION-PROVEN: the parse path (hunk state, the `\` marker, the `diff --git` reset, `splitlines`, the population count, `re.IGNORECASE`, the SCANNED-NOTHING return) and five pins — `core.quotePath=false`, `--find-renames`, `--dst-prefix`, `--no-ext-diff`, `--no-color`. UNPROVEN and defensive, recorded as such rather than implied to be covered: `-U0` (the context arm handles any `-U`, so removing it reddens nothing), the `GIT_CONFIG_GLOBAL`/`SYSTEM`/`NOSYSTEM` overrides (which exist for the UNNAMED knob and therefore cannot be witnessed — the script says so itself), `--src-prefix`, the `+++ /dev/null` deletion arm, the malformed-`@@` arm, the stderr relay, `git()`''s `OSError` return, `run_all`''s failure sentinel, `open(errors=)` and the top-level `except`. Both lists ENUMERATE; neither is a universal over the file, and a clause on neither list has simply not been measured. `--diff` scans only lines ADDED in the PR (with rename detection, so a `git mv` does not re-flag a file''s pre-existing content) and reports `SCANNED NOTHING` rather than a clean-looking line when it cannot resolve the base. `--all` sweeps the tracked corpus from `git ls-files`, never a filesystem walk (#778). Two consequences of added-lines-only are deliberate and stated so they are not mistaken for coverage: a file MOVED into the population (`web/x.md` to `docs/x.md`) is never scanned by any PR, and neither is anything already in the corpus. `--all` is the only thing that sees either, which is why the sweep is a task a person runs rather than a job.' +rule: 'Every durable artifact — an in-repo `docs/` page, a skill, a README, a code comment, an Obsidian vault page — records the END STATE. The path to that end state goes in the commit message, the Gitea issue, or the issue''s `## Closing record`; it does not go in the artifact. Concretely: **a review finding is answered in the commit message, and only the corrected claim enters the doc.** Naming the destination is load-bearing — "do not write it in the doc" with no home loses the knowledge, and this repo has the inverse failure on record too (#542, where a pruned narrative turned out to be the only copy). THE TEST IS WHO BENEFITS: if only the author''s timeline explains why a sentence is there, it is narrative and belongs in the commit; if a reader who never saw the session would act differently knowing it, it is a finding and stays. Session narrative reads as: first person or session chronology ("I initially thought", "an earlier draft counted", "my first attempt returned 0"), a correction of a belief the reader never held ("this was wrong, actually X" where only X matters), relative time ("earlier today", "currently investigating"), or a blow-by-blow diagnosis standing in place of the conclusion. THE CARVE-OUT, which must be stated or the rule gets over-applied — reader-facing history that must survive: a decision record''s `supersedes`/`superseded-by`; a dated measurement or an explicitly stated snapshot boundary; a TESTED-AND-REJECTED negative result, kept so nobody re-proposes it on plausibility; the *why* behind a non-obvious choice; and a trap together with its consequence. `docs/decisions/records/**` and `docs/decisions/archive/**` are exempt WHOLESALE: a record narrating how a rule was got wrong is carrying the rationale it exists to carry. `docs/superpowers/**` — the dated plans and specs — IS reached by the rule (#812), and is NOT exempt from the detector either. The narrative sites IDENTIFIED there were remediated with the rest of the corpus — identified, not all, since no sweep here claims closure over a phrasing space. The one surviving detector hit is a carve-out on its merits, keeping a rejected design together with the concrete harm that killed it. A wholesale detector exemption for that path was proposed and REJECTED, recorded so it is not re-proposed on plausibility: the argument was that all 35 files are frozen (measured 2026-08-29, before this change: nothing had been edited there since 2026-07-23), so sweeping them yields only false positives. But `--diff`, the mode CI runs, scans ADDED lines, and a frozen file contributes none — so the exemption buys nothing where the check actually runs, and what it WOULD suppress is any plan being written OR REVISED now — 15 of the 35 have more than one commit, so revisions do happen — which is exactly the case where the remedy (move it to the commit message) is still available. It costs the detector''s only reach to buy quiet in a sweep a person runs deliberately. ENFORCEMENT IS ADVISORY ONLY — `scripts/check-doc-narrative.py`, run non-blocking from the `docs-reminder` job over ADDED lines. It is a string predicate over prose and may never become a blocking gate.' +signals: 'no session narrative in docs · the reader never saw the earlier draft · who-benefits test · answer a review finding in the commit message not the doc · end state versus path to it · earlier draft · first version of this table · my first attempt · relative time in a doc · carve-out for dated measurement and snapshot boundary · tested-and-rejected negative result · decision records are exempt on purpose · advisory not blocking · string predicate over prose · does the rule reach docs/superpowers · dated plans and specs · a frozen file contributes no ADDED lines so --diff never sees it · exempting a path costs the detector''s only reach · detector population unchanged by #812 · paths: `scripts/check-doc-narrative.py`, `.gitea/workflows/pr-checks.yml`, `docs/handoffs/chicorytv-issue-queue.md`, `docs/defect-shapes-773.md` · issues: #784, #812, #773, #767, #743, #542' +mechanics: 'The RULE is about every durable artifact; the DETECTOR''s population is narrower and is stated here so a row in it never reads as coverage it does not have. `scripts/check-doc-narrative.py` scans `docs/**/*.md` minus `docs/decisions/**` (exempt wholesale, in both modes), plus root-level `*.md` — a population #812 left UNCHANGED after considering and rejecting a `docs/superpowers/**` exemption. The exemption test carries a hand-written sample path per prefix in `EXEMPT_PREFIXES` and asserts set equality against it, so a prefix added without a case reddens; it also asserts a NON-EXEMPT control HIT in the same invocation, because `hits(out) == set()` alone passes when the scan examined nothing — a scanned-zero run is indistinguishable from a working exemption. What that buys is bounded, and the bound is stated because three rounds of trying to widen it each failed: the test proves the scan EXAMINED something, NOT that a sample''s absence is attributable to the prefix rather than to its shape. A lone depth-1 control fell to a depth-1 population rule; a depth-3 control fell to a `count("/") <= 3` cap; minimal twins differing only in the renamed directory fall to a directory-NAME rule keyed on that rename. Each fix was locally correct and the sequence did not converge — the same shape as the withdrawn `test_review_verdict_vocabulary_parity.py` cited below — so the claim was retracted rather than a fourth finite control added. The residual that remains after all of it is the WIRING: every proof below is about `is_scanned_path`, and nothing in it establishes that the SCAN consults that function — a `continue` added at either call site re-exempts a path with the function untouched, measured to drop `--all` from 66 files to 31 with the oracle still green. A fixture under `docs/superpowers/` is asserted to reach the OUTPUT for exactly that reason, so the one re-exemption this record argues against is witnessed rather than assumed; the general residual is not closed. What closes the SHAPE question is a CLOSED-FORM proof: `is_scanned_path` is compared against an INDEPENDENT restatement of the population rule (written over path segments, not string prefixes) across every tracked `*.md` path AND a cross product of FOUR dimensions a population rule has been OBSERVED to key on here — depth, first segment, second segment, extension — since widened with basename, case and dotted directories. That list ENUMERATES; it is not a universal over the space of rules, and a dimension on neither list has simply not been measured. The real corpus alone is not enough, and that is measured: no scanned path carries more than three slashes today, so a `count("/") <= 3` cap is a no-op against it and would ship latent. The three controls'' defeating mutations (a depth-1 rule, a `count("/") <= 3` cap, a directory-NAME rule) all redden it, as do scan-nothing and a dropped prefix — and so do the basename, case, dotted-directory and extension mutants a later round added the corresponding axes for. Skills under `.claude/`, `web/`, and every other nested markdown file outside `docs/` are OUT of its scope and stay a human judgement. It warns and exits 0 on every path — bad argument, unresolvable ref, unreadable file, unhandled exception — asserted per argument shape in `scripts/tests/test_check_doc_narrative.py`, not only in prose, EXCEPT the unhandled-exception arm, which is a bare `except` no test exercises and is recorded as unproven rather than implied. The invariant is held at the JOB level too (`continue-on-error` on both steps): a script that returns 0 does not keep a job green if a setup action fails. Every knob DEMONSTRATED to break the parse has a row in `FORMAT_KNOBS` in the test file; the user and system config files are removed from the picture entirely so an unnamed one cannot reach it. Three separate review rounds each found ONE more knob turning a real hit into a clean-looking `scanned 0 file(s)`, so the third fix removed the surface rather than naming a fourth knob — a completeness claim over the knob space would be exactly the enumeration that failed three times. COVERAGE BOUNDARY, written once so it is not rediscovered one clause at a time, and naming the clauses rather than a category because a category is where the last mis-sort hid. MUTATION-PROVEN: the parse path (hunk state, the `\` marker, the `diff --git` reset, `splitlines`, the population count, `re.IGNORECASE`, the SCANNED-NOTHING return) and five pins — `core.quotePath=false`, `--find-renames`, `--dst-prefix`, `--no-ext-diff`, `--no-color`. UNPROVEN and defensive, recorded as such rather than implied to be covered: `-U0` (the context arm handles any `-U`, so removing it reddens nothing), the `GIT_CONFIG_GLOBAL`/`SYSTEM`/`NOSYSTEM` overrides (which exist for the UNNAMED knob and therefore cannot be witnessed — the script says so itself), `--src-prefix`, the `+++ /dev/null` deletion arm, the malformed-`@@` arm, the stderr relay, `git()`''s `OSError` return, `run_all`''s failure sentinel, `open(errors=)` and the top-level `except`. Both lists ENUMERATE; neither is a universal over the file, and a clause on neither list has simply not been measured. `--diff` scans only lines ADDED in the PR (with rename detection, so a `git mv` does not re-flag a file''s pre-existing content) and reports `SCANNED NOTHING` rather than a clean-looking line when it cannot resolve the base. `--all` sweeps the tracked corpus from `git ls-files`, never a filesystem walk (#778). Two consequences of added-lines-only are deliberate and stated so they are not mistaken for coverage: a file MOVED into the population (`web/x.md` to `docs/x.md`) is never scanned by any PR, and neither is anything already in the corpus. `--all` is the only thing that sees either, which is why the sweep is a task a person runs rather than a job.' --- The rule already existed, correctly stated, and scoped to exactly one file. The kickoff handoff said @@ -48,6 +48,29 @@ for it being wrong sometimes, and make that acceptable by not letting it block. **It is Python, not shell, and that is the same argument one level down.** A detector over diff output has to decide what each line IS, and deciding that from its prefix alone — without hunk state — is a string predicate too. Four distinct defects fell out of one bash implementation of it: the no-trailing-newline marker counted as content, an added line whose own text began `++ ` eaten by the `+++ ` header arm, `core.quotePath` hiding non-ASCII paths, and a final unterminated line dropped by `read`. Those are four sites of one mistake, so the mechanism was replaced rather than the sites patched one at a time. Anything that parses a diff here should parse it with hunk state or not at all. +**A path exemption was proposed for `docs/superpowers/**` and rejected (#812).** Recorded because +the argument for it is a good one and will be made again. Those 35 dated plans and specs were frozen +— measured 2026-08-29, before this change: nothing had been edited there since 2026-07-23, and none is +indexed in `docs/README.md` — so +the rule's remedy, *move it to the commit message*, is unavailable for a file nobody will edit +again, and a design doc recording a tried-and-rejected approach is doing the job a design doc exists +for. On that reading every hit in the path is a carve-out false positive, and a permanent false +positive is what makes an advisory check stop being read. + +What defeats it is the MODE. `--diff` is what CI runs, and it scans lines a PR **adds**; a frozen +file adds none, so it is never flagged there anyway and the exemption buys nothing where the check +actually runs. What it *would* suppress is a plan being **written or revised** now — the case in the path +where the remedy is still available, because that commit message is being written in the same act. The exemption +spends the detector's only reach to buy quiet in a sweep a person runs deliberately, and a sweep's one +explained hit is cheaper than a blind spot. Note that frozen and never-revised are different +claims: 15 of the 35 files have more than one commit, so revisions do happen — they simply all +predate the 2026-07-23 snapshot measured above. + +So the rule reaches `docs/superpowers/**` and the detector's population is unchanged. The narrative +sites IDENTIFIED there were remediated with the rest of the corpus — identified, not all — and the +one surviving hit is a genuine carve-out: it keeps a rejected design together with the concrete harm +that killed it. + **If this ever becomes a hook, it belongs at USER scope, not in this repo.** The rule is not ersatztv-specific — it is true of every repo — and an ersatztv-only hook would enforce it exactly where it was first noticed and nowhere else, which is the shape this record exists to close. diff --git a/docs/decisions/records/release/verdict-vocabulary-shared.md b/docs/decisions/records/release/verdict-vocabulary-shared.md index 68012363d..50f12cb39 100644 --- a/docs/decisions/records/release/verdict-vocabulary-shared.md +++ b/docs/decisions/records/release/verdict-vocabulary-shared.md @@ -118,6 +118,8 @@ finds separator-joined enumerations, it is better than a number because it can b not evidence that no other copy exists. The safety property does not rest on it — nothing outside `review-verdict-vocabulary.sh` classifies a word, and that is what makes these copies inert. -`docs/superpowers/**` is excluded as historical session artifacts (#812 is deciding whether the docs -rules reach them at all), and this record excludes itself, since the pattern above matches its own -text. +`docs/superpowers/**` is excluded as historical session artifacts by the ad-hoc `git ls-files` +sweep above, not by `review-verdict-vocabulary.sh`, which classifies a word and carries no path +exemption at all. #812 settled that the docs narrative rule does reach that path and left the +narrative detector's population unchanged. This record excludes itself, since the pattern above matches its +own text. diff --git a/docs/defect-shapes-773.md b/docs/defect-shapes-773.md index 11801fe8d..c398bc38a 100644 --- a/docs/defect-shapes-773.md +++ b/docs/defect-shapes-773.md @@ -672,8 +672,8 @@ necessary condition, not the finding. So these two zeroes remain genuinely ambiguous, and neither can be resolved without the thing that resolves it: a live browser session, or a real merge attempt. Manufacturing a merge to observe the merge guard is a worse idea than the gap it would close. Distinguishing the two meanings of zero -stays a human judgement — `testing.hook-reports-its-own-execution` says so, and an earlier draft of -this paragraph quietly contradicted it by treating "the script works" as "the wiring works". +stays a human judgement — `testing.hook-reports-its-own-execution` says so, and treating "the script +works" as "the wiring works" is the way that judgement goes wrong. **The measurement earned its keep on its first run.** `pretooluse-agent-model` was recorded deciding `no-op` on an `Agent` dispatch where `ask` was expected. Previously that would have been an diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index 443b2dc8c..02ce247b0 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -39,7 +39,7 @@ suite green, and every behavioural test passed throughout. **The definition above is the second one.** The first read "a named test disarms this guard's clause and asserts red", and three rows were graded `MUTATION` against it that do not disarm anything — they feed the real script an input only that clause rejects (empty stdin, a short page, a full first -page). Cold review caught it, and the objection is right twice over: those tests are the same +page). The objection to that is right twice over: those tests are the same species as ones graded `BEHAVIOUR-ONLY` eight rows away, so the column was being applied as a curve on the very day it was introduced. They are regraded. The surviving criterion is *witnessed*, not *plausible* — "removing this clause would surely redden that test" is an argument, and this table @@ -71,7 +71,7 @@ with have sat in these gaps. Treat anything not named here as unassessed, not as prove the population `pageSizeCallSites.guard.test.ts` derives (split because the `realgit` half needs the git BINARY, which the Docker web-build stage has not got). Note what that costs: this list is a HAND-WRITTEN mirror of a population nothing derives, so it goes stale silently and CI - stays green — #807 added the third entry, and only review caught that + stays green — #807 added the third entry, and nothing mechanical caught that `pageSizeCallSites.guard.test.ts` had become the only one named. Since #819 that guard derives its file population from the git index like the Python ones, which makes it read as covered by `test_guard_populations_derive_from_git.py`. It is not: that register @@ -164,7 +164,7 @@ The audit #806 asked for, recorded whichever way it came out, because "we looked | `test_hook_fire_log.py` | `.claude/hooks/*.sh` | **yes** — every hook must be instrumented | converted; an untracked scratch `.sh` used to demand instrumentation and redden the suite on that checkout alone | | `test_ci_image_pin_population.py` | `.gitea/workflows/*.yml` + `*.yaml` | **yes** — "docker-build is the ONLY workflow pinning the toolchain image" | converted, and `*.yaml` added: Gitea accepts both spellings, so a `.yaml` workflow was structurally invisible while the test read as covering all of them | | `test_workflow_job_guards.py` | `.gitea/workflows/*.y*ml` | **yes** — every tracked workflow's jobs must each declare a `CI_JOB_ROLE`, and the guard-declaring ones must match `## Workflow-job guards` both ways | index-derived from the start (ersatztv#786); registered in `test_guard_populations_derive_from_git.py` so the shared proof covers it | -| `test_pr_changed_files.py` | `.gitea/workflows/*.y*ml` | **yes** — "no OTHER workflow writes the review-verdict status", and since ersatztv#748 "every tracked workflow declares a `permissions:` block", with NO exemption list (the one `ci-image.yml` briefly needed was deleted when #744 landed, per #835) | converted. Not on #806's list and found by cold review on the pushed head: an untracked `.yaml` dropped in `.gitea/workflows/` reddened two guards while absent from the index — the issue's list of files to assess was a starting point, not the population | +| `test_pr_changed_files.py` | `.gitea/workflows/*.y*ml` | **yes** — "no OTHER workflow writes the review-verdict status", and since ersatztv#748 "every tracked workflow declares a `permissions:` block", with NO exemption list (the one `ci-image.yml` briefly needed was deleted when #744 landed, per #835) | converted. Not on #806's list: an untracked `.yaml` dropped in `.gitea/workflows/` reddened two guards while absent from the index — the issue's list of files to assess was a starting point, not the population | | `test_workflow_persist_credentials.py` | `.gitea/workflows/*.yml` + `*.yaml` | **yes** — every `actions/checkout` in every workflow must drop the persisted credential | derived from the index from the start (#835). Shipped with NO exemption list, which is why it waited for #744: the one non-compliant checkout would otherwise have bought a permanent entry that keeps passing after its reason expires | | `web/src/api/pageSizeCallSites.guard.test.ts` | `import.meta.glob` over `web/src/**/*.{ts,tsx,mts,cts}`, INTERSECTED with the git index | **yes** — an unregistered discovered site fails | converted under #819. The population is the glob INTERSECTED with the git index, and the walk/index divergence is asserted in BOTH directions WITHIN a shared scope: a key the index does not carry is dropped, and a tracked, on-disk, in-scope path that never reached the scanner FAILS rather than shrinking the population silently (that direction catches a dotfile, which the glob cannot match, and a disk/index spelling divergence, which `core.ignorecase` and NFD/NFC normalisation make permanent). The SCOPE itself cancels out of that equality — both sides call one `isInScopeSourcePath` — so narrowing it moves both sets together and no assertion comparing the two sides can see it. That is closed separately, by re-deriving the predicate from its two components over the whole tracked index (`the shared scope predicate is EXACTLY its two documented components`). Two mechanisms carry that. FIRST, the scope predicate is proved by a CLOSED-FORM restatement — it may share no helper, at any depth, with the predicate it checks, since anything shared sits on both sides of the comparison and cancels. FOUR earlier attempts were each measured going green while removing real files: a table of example paths (four of `src/`'s eight directories, missed a 23-file narrowing), a decomposition delegating its filename half to `isScannableSourceFileName` (11 files), one still sharing a `basename` helper (15 files, and blind to a planted call site), and one still sharing the tracked-file array every comparison was derived from (8 files, also blind). SECOND — because that last one showed a restatement cannot police the population it reads — the plugin runs a separate `git ls-files --others` query and the guard requires every in-scope walked path to appear in `tracked ∪ others`; narrowing `tracked` cannot suppress that, since it adds nothing to `others`. SIX residuals, NOT a claim of closure, each with its direction MEASURED by planting a real `pageSize` call site rather than reasoned about. FAIL-NOISY (they redden a checkout; they cannot hide a call site): (2) a scope term matching no tracked path today survives until the day it first matches one; (4) the WIRING in `listSourceFiles`, indistinguishable on a clean checkout where the walk set and the index set agree on every in-scope key (they are not literally equal — the index also carries `.css`, which the glob never yields), so no assertion COMPARING THE TWO POPULATIONS can tell which is passed; (5) `ABSENT_FROM_DISK` emptied. BLIND — each hid a planted `pageSize: 100` with the whole suite green: (1) a COORDINATED edit of the scope predicate and BOTH closed-form restatements, which is three sites and not two, since a second restatement guards `expectedTrackedSources` (measured: the two-site edit reddens, the three-site edit does not) — tolerated because a three-site policy edit is review-visible in a way a one-line slip is not; and (3) a misdescription by the plugin's THREE outputs — not only a mispartition of `tracked`/`others`, which preserves the union any consumer compares, but a FATTENED `absentFromDisk`, which subtracts real files from the hole-detection direction. The fattening is now caught wherever it overlaps a file the walk found (a walked path is on disk by construction, so it can never be legitimately absent); restricted to paths the walk cannot see anyway — a dotfile, a case-divergent name — it stays invisible from the consuming side and is answerable only by testing the derivation directly, and the real-git tests that do so catch an UNCONDITIONAL misdescription, not one keyed on a path pattern. Neither blind residual is tolerated for being harmless. A sixth is specific to WATCH mode and is stale in BOTH directions, so it belongs to neither group: the virtual module has no backing file, so the index is read once per dev-server lifetime while the glob refreshes. A file CREATED mid-session reddens the cross-check misleadingly (noisy); a file already UNTRACKED when the watcher started keeps that classification when staged mid-session, so its call sites are never scanned — MEASURED green across both phases while `npx vitest run` on the same tree reports `UNREGISTERED`, i.e. blind, and a window that did not exist before #819 because the population was then the walk. A watch-mode green is therefore not authoritative for this guard; restarting the watcher clears the noisy case and opens the blind one, so the remedy is `npm test -- --run`. Invalidating the module from `configureServer` was implemented, measured and REJECTED: it fixes the created-mid-session red and additionally blinds the create-then-stage sequence, while the already-untracked-then-staged sequence is blind either way, since `git add` fires no watcher event in either design. The index is read by `web/vite-plugins/trackedSourceFiles.ts` in Vite's own Node context and handed to the app project as the virtual module `virtual:etv-tracked-source-files`. That is what reaches the index without admitting `@types/node` to `tsconfig.app.json` — the obstacle that deferred this, since wiring those types in was tried and reverted (it leaked Node's `setTimeout` into the app project and broke three unrelated tests). The plugin throws rather than falling back to the unfiltered walk when git fails, reports zero files, or cannot learn the Vite root. Residual, stated: a file tracked but DELETED in the working tree is subtracted deliberately, since an unstaged deletion is a normal developer state and a guard red on one gets ignored (#806); it is distinguished from the hole cases above by an on-disk existence check, not conflated with them | | `test_ci_release_path_scan_job.py` | `.gitea/workflows/*.y*ml` + `scripts/**` | **no** — a fixture assembling a tmp harness, asserted about behaviour not membership | takes its file LIST from the index anyway, for hermeticity not completeness: `shutil.copytree` copied whatever was on disk, including untracked files and `scripts/__pycache__`, into a tree whose behaviour the probes then measure. Content still comes from the working tree. The copy is not a git repo, so the two files this step RUNS may not use the helper — see the fixture docstring | @@ -369,10 +369,9 @@ clause-provable and the entry is regraded. 44 guards, 6 tooling scripts, 22 proof files. **22 guards carry a mutation proof; 6 are behaviour-only; 16 have none.** These figures are asserted against the table by -`test_the_summary_counts_match_the_table` — they were wrong in the first draft (28/4/6/3/19 against -a table holding 27/5/6/3/18), because a hand-maintained summary of a table is a second copy of it, -which is the duplication family this change argues against. Both cold reviewers found the error -independently. +`test_the_summary_counts_match_the_table`, because a hand-maintained summary of a table is a second +copy of it — the duplication family this change argues against, and one that has already drifted +here. **One guard was WITHDRAWN rather than shipped**, and it is worth a line here because the inventory is where a future session will look for it. A `test_review_verdict_vocabulary_parity.py` asserted @@ -383,8 +382,8 @@ escaped it or made it red on a correct tree. It was deleted rather than patched symptom-keyed mistake, and this was the empirical demonstration. The underlying duplication is real and is now UNMITIGATED — tracked in #788, which fixes it by removing the second copy. -**What that file asserted, and where each part went** — enumerated because the first attempt at this -withdrawal silently dropped one and claimed otherwise, which is +**What that file asserted, and where each part went** — enumerated rather than summarised, because a +summarised removal silently drops an invariant and claims otherwise, which is `process.enumerate-workaround-behaviors-before-deleting` failing on its own removal: | Invariant it asserted | Where it is now | @@ -398,9 +397,9 @@ withdrawal silently dropped one and claimed otherwise, which is | A wrong write source makes the extractors disagree | Retired with the extractor; #788 | Two guards were ADDED in its place. They are **not** replacements for rows 2 and 3 and the table -above deliberately does not list them as such — an earlier draft did, and cold review was right that -relabelling a lost invariant as a narrower surviving one is the exact failure this table exists to -prevent, committed by the table: +above deliberately does not list them as such: relabelling a lost invariant as a narrower surviving +one is the exact failure this table exists to prevent — and it would be worst committed by the +table itself: | Added | What it pins | |---|---| @@ -455,7 +454,7 @@ Proofs: `test_REMOVING_the_pre_collection_isolation_LETS_an_IMPORT_TIME_snapshot arms as nested pytest sessions under a fake `HOME`, and `test_a_launch_NOT_CARRYING_the_isolated_log_dir_FAILS_THE_LAUNCH` drives the wrapper over both branches, an explicitly-wrong value, and a RELATIVE value made to land on a shared log by the -child's `cwd` — that last one found by cross-family review, which is also why the guard anchors a +child's `cwd` — that last one is also why the guard anchors a relative path at the launch's `cwd=` rather than at its own — each with a negative control. `test_the_resolver_agrees_with_the_SINKS_OWN_expansion` runs the Python resolver and `etv_hook_fire_log_dir` over the same environments every run, because a resolver that @@ -475,8 +474,8 @@ instrumenting `subprocess.Popen` for a full run and asking which launches resolv session-scoped fixtures — both outside any test, and so beyond an autouse function fixture's reach. The other 2 came from an environment built from scratch with the real `HOME`, the route the second layer exists for. Instrument `Popen` ALONE: `subprocess.run`, `call` and `check_output` all reach -it, so wrapping `run` as well counts every launch twice and doubles every figure — which is how the -first census here was wrong. That predicate also UNDERCOUNTS the from-scratch route, because such an environment +it, so wrapping `run` as well counts every launch twice and doubles every figure. That predicate +also UNDERCOUNTS the from-scratch route, because such an environment usually carries no `HOME` either and so resolves to the sink's other default, which it cannot see. Both agree on the finding that matters: almost none of it was the module-level-snapshot route ersatztv#809 names. @@ -491,7 +490,7 @@ reproduction still shows the mistake, not the consequence it used to have. The pin is `test_worktree_ownership_guard.py::test_driving_a_hook_LANDS_its_records_in_the_ISOLATED_dir`, which asserts the EFFECT — records land in the fixture's dir — rather than the shape of the fix. Its -predecessor asserted the helper's return value, and cold review showed that leaves the call site +predecessor asserted the helper's return value, which leaves the call site unguarded: `_env()` correct, one `env=` reverted, all tests green, records still leaking. Pinning to a hand-written revert rather than to the property is `verify-against-the-REAL-predecessor`. diff --git a/docs/remote-state-inventory.md b/docs/remote-state-inventory.md index b37d29220..00cd13728 100644 --- a/docs/remote-state-inventory.md +++ b/docs/remote-state-inventory.md @@ -41,7 +41,7 @@ artifact; the test keeps it from rotting. - **Class** — - `PINNED` — the read is bound to an immutable version identifier (a **full** commit sha, an image digest, a monotonic event count), and that binding still holds when the action runs. Two - shapes qualify, and the second was missing from the first wording: either the action + shapes qualify: either the action **re-validates** against the identifier immediately before committing (a snapshot nobody re-checks is not pinned — binding alone is never enough), **or** the check and the use are a single step over a value that cannot move, such as a workflow reading a full sha straight out of @@ -65,8 +65,8 @@ classifications differ; otherwise the strictest applies and the Note names the e | Site | Class | Note | |---|---|---| -| `.claude/hooks/pretooluse-merge-consent.sh` — head-sha reads (CI status, H10 status, verdict comments) | `UNSAFE-KNOWN` | Every comparison uses the **full** 40-char `.head.sha` (`${sha:0:7}` appears only in human-readable strings), which is the right identifier — but it is captured ONCE from the PR snapshot at the top of the hook. It WAS stale in two distinct ways; the first is closed. FIRST, within the run: every later check (CI status, H10 status, verdict comments) was evaluated against the captured sha, so a push landing mid-run — including across the docs-only enumeration's up-to-forty round trips — was checked against the commit it replaced. **Closed 2026-08-28 (#803)**, symmetrically with the base and at the same hoist: `.head.sha` is re-read from `prjson_now`, the response the base re-read already fetches, so the two axes cost one round trip between them and cannot describe two different instants; a moved head DENIES, matching the `stale` verdict class, while an unreadable one asks. SECOND, after the decision: the hook returns `allow` and a separate call performs the merge. "No async window" was the second overclaim cold review removed from this file. The one that remains is bounded the same way it always was — a head the verdict does not cover cannot inherit the sha-bound required status, so the server refuses it — and is not bounded by anything in this hook. Note the re-read narrows the window rather than erasing it: a push landing after the hoist still passes, and the hook deliberately does not re-read a second time (two reads move the window rather than closing it, the same rule the branch-protection block states). The merge API accepts an optional `head_commit_id`, which would make the call a true CAS; a PreToolUse hook cannot ADD that argument, only refuse without it, and requiring it changes every merge call's shape — tracked as follow-up rather than smuggled in here. Accepted meanwhile because the window is one tool call wide with no scheduler in it, and the server-side per-sha required check still refuses an unreviewed head. | -| `.claude/hooks/pretooluse-merge-consent.sh` — scheduled auto-merge (`merge_when_checks_succeed`) | `UNSAFE-KNOWN` | **Preflight, not a pin** — graded down by cold review, which was right: the hook's own comment concedes the branch-protection read pins nothing, so calling it `PINNED` contradicted this file's definition. What the hook proves is a snapshot; Gitea merges later; and since #778 it also verifies that `review-verdict/h10` is a required check, reading the repo's **full rule list** (never the by-name endpoint, which does no matching and knows nothing about precedence) — nothing can govern the base → deny, unreadable → ask, and a **glob rule that could govern it → ask**, because the hook deliberately does not reimplement Gitea's glob dialect. It also asks when two rule names fold equal, or when either name is non-ASCII, since Gitea's `EqualFold` is Unicode-aware and its rule precedence is not derivable here. That converts an unobserved assumption into an observed precondition and detects drift, but an admin can still weaken the protection *after* the read. Accepted, and the earlier wording here was **circular** — it said the residual was "closed one layer down" by the very branch protection an admin may have removed. It is not closed; it is BOUNDED, and the bound is a trust assumption that should be stated rather than dressed as a mechanism: everything on this path assumes repo-admin branch-protection config is not hostile. If protection is present at preflight and removed afterwards, an unreviewed head can merge, and nothing in this repo would detect it. What the check does buy is that the far commoner case — protection already weakened when the merge is attempted — stops being silent. A PreToolUse hook cannot add `head_commit_id` to the merge call, so it can never convert its own grant into a CAS — it can only refuse, which is what it now does. | +| `.claude/hooks/pretooluse-merge-consent.sh` — head-sha reads (CI status, H10 status, verdict comments) | `UNSAFE-KNOWN` | Every comparison uses the **full** 40-char `.head.sha` (`${sha:0:7}` appears only in human-readable strings), which is the right identifier — but it is captured ONCE from the PR snapshot at the top of the hook. It WAS stale in two distinct ways; the first is closed. FIRST, within the run: every later check (CI status, H10 status, verdict comments) was evaluated against the captured sha, so a push landing mid-run — including across the docs-only enumeration's up-to-forty round trips — was checked against the commit it replaced. **Closed 2026-08-28 (#803)**, symmetrically with the base and at the same hoist: `.head.sha` is re-read from `prjson_now`, the response the base re-read already fetches, so the two axes cost one round trip between them and cannot describe two different instants; a moved head DENIES, matching the `stale` verdict class, while an unreadable one asks. SECOND, after the decision: the hook returns `allow` and a separate call performs the merge. "No async window" would be an overclaim. The window that remains is bounded the same way it always was — a head the verdict does not cover cannot inherit the sha-bound required status, so the server refuses it — and is not bounded by anything in this hook. Note the re-read narrows the window rather than erasing it: a push landing after the hoist still passes, and the hook deliberately does not re-read a second time (two reads move the window rather than closing it, the same rule the branch-protection block states). The merge API accepts an optional `head_commit_id`, which would make the call a true CAS; a PreToolUse hook cannot ADD that argument, only refuse without it, and requiring it changes every merge call's shape — tracked as follow-up rather than smuggled in here. Accepted meanwhile because the window is one tool call wide with no scheduler in it, and the server-side per-sha required check still refuses an unreviewed head. | +| `.claude/hooks/pretooluse-merge-consent.sh` — scheduled auto-merge (`merge_when_checks_succeed`) | `UNSAFE-KNOWN` | **Preflight, not a pin** — the hook's own comment concedes the branch-protection read pins nothing, so calling it `PINNED` would contradict this file's definition. What the hook proves is a snapshot; Gitea merges later; and since #778 it also verifies that `review-verdict/h10` is a required check, reading the repo's **full rule list** (never the by-name endpoint, which does no matching and knows nothing about precedence) — nothing can govern the base → deny, unreadable → ask, and a **glob rule that could govern it → ask**, because the hook deliberately does not reimplement Gitea's glob dialect. It also asks when two rule names fold equal, or when either name is non-ASCII, since Gitea's `EqualFold` is Unicode-aware and its rule precedence is not derivable here. That converts an unobserved assumption into an observed precondition and detects drift, but an admin can still weaken the protection *after* the read. Accepted. Calling the residual "closed one layer down" by the very branch protection an admin may have removed would be **circular**: it is not closed; it is BOUNDED, and the bound is a trust assumption that should be stated rather than dressed as a mechanism: everything on this path assumes repo-admin branch-protection config is not hostile. If protection is present at preflight and removed afterwards, an unreviewed head can merge, and nothing in this repo would detect it. What the check does buy is that the far commoner case — protection already weakened when the merge is attempted — stops being silent. A PreToolUse hook cannot add `head_commit_id` to the merge call, so it can never convert its own grant into a CAS — it can only refuse, which is what it now does. | | `.claude/hooks/pretooluse-merge-consent.sh` — base-retarget detection | `UNSAFE-KNOWN` | `recorded_base` parsed from the H10 status description is compared against the PR's live `.base.ref` (#632), and it deliberately compares the base **ref** rather than `base.sha` — the tip moves on every unrelated merge, so comparing it would deadlock every open PR. The residual is the same ABA the enumerator has: a name can be retargeted away and back, and the comparison cannot see it. Accepted because the alternative that CAN see it is the monotonic event count, which lives in the workflow that writes the enforced status rather than in this advisory hook. | | `.claude/hooks/pretooluse-merge-consent.sh` — `## Done-when` issue-body read | `UNSAFE-KNOWN` | The issue body carries an `updated_at` that is not used, so a box unticked between the read and the merge is invisible. Accepted: the only actor who can edit the issue is the one requesting the merge, so this is a self-inflicted race with no adversary and no silent-failure mode. | | `.claude/hooks/pretooluse-merge-consent.sh` — guard-scope freshness `branch_protections` read (ersatztv#787) | `UNSAFE-KNOWN` | A SECOND, separate read of the same endpoint, and the first one on the IMMEDIATE merge path: the pre-existing row above fires only when arming a scheduled auto-merge. Read-then-act with nothing pinning it — branch protection can change between this read and the merge Gitea performs. Accepted because of what the read AUTHORIZES, which is nothing: every outcome is `ask` or silence, never a grant and never a deny, so a stale or raced read can only cost a prompt. It is also skipped entirely unless the merge target is the repo `.gitea/required-status-contexts.json` names. The reconciliation it performs is bounded to merges made through this hook — a web-UI merge never runs it. | @@ -96,13 +96,13 @@ classifications differ; otherwise the strictest applies and the Note names the e | Site | Class | Note | |---|---|---| -| `scripts/post-review-verdict.sh` — commit-status write | `PINNED` | Re-reads the PR and compares **both** `.head.sha` and `.base.ref` immediately before the POST, and `die`s (exit 1, no status written) on a mismatch **or on a field it cannot read**. That last clause is new: both comparisons were guarded by `[ -n "$x" ] &&`, so a well-formed 2xx body that merely omitted the field made the check a no-op and the status was posted having confirmed nothing — found by cold review on #778 and regression-tested against the real predecessor, since the redundant `-z` arm alone mutates green. Closes #706 and #632 for this path by read-compare-refuse, not by CAS: Gitea's status API offers no conditional write. Residual closed on the write ORDER since #792: the status is written first and the comment second, so a refusal can no longer leave a verdict comment with no status behind it — the only reachable half-state is a status with no comment, which the merge hook resolves as `ask` (`release.verdict-writes-status-before-comment`). | -| `scripts/pr-changed-files.sh` — paged file enumeration | `UNSAFE-KNOWN` | #707's fix, graded honestly after cold review: `.base.ref`, `.base.sha` and `.head.sha` are captured before paging and re-checked after, and any *observed* movement fails the whole enumeration closed rather than emitting a short list. But before-and-after equality is **ABA-vulnerable** — a `main → scratch → main` retarget during paging can return the same ref and, if nothing merged meanwhile, the same base sha, while the pages in between were diffed against the scratch base. The script's own comment says it narrows rather than erases; this row previously said "any movement fails", which was stronger than the code. Accepted here because the enumerator cannot close it alone, but be exact about what the caller-side fence does and does not cover: `ci.verdict-write-retarget-fence` counts `change_target_branch` events, so it catches the BASE alias and **nothing else**. A HEAD alias — a force-push `H1 -> H2 -> H1` during pagination — leaves the final `.head.sha` comparison equal while the middle pages were enumerated against `H2`, and NOTHING inside this script can see it. **Fenced at the enforced caller since #803** (2026-08-28): the same timeline walk now also counts `pull_push`, so a head that moved at all during classification withholds the write — subject to that walk's own terminator, which is defeatable by a page of filtered rows (**#870**, pre-existing and equally affecting the base axis). The grade stays `UNSAFE-KNOWN` for the SCRIPT, because the script's own guarantee is unchanged and one-way — a caller that does not run the fence inherits the alias. Enumeration bound to an immutable tree remains unavailable: re-probed at 1.27.1 on 2026-08-28, `compare/{base}...{head}` still returns no `files` key. | +| `scripts/post-review-verdict.sh` — commit-status write | `PINNED` | Re-reads the PR and compares **both** `.head.sha` and `.base.ref` immediately before the POST, and `die`s (exit 1, no status written) on a mismatch **or on a field it cannot read**. That last clause is load-bearing: guarding both comparisons with `[ -n "$x" ] &&` alone makes a well-formed 2xx body that merely omits the field a no-op, and the status is posted having confirmed nothing. Regression-tested against the real predecessor (#778), since the redundant-looking `-z` arm alone mutates green. Closes #706 and #632 for this path by read-compare-refuse, not by CAS: Gitea's status API offers no conditional write. Residual closed on the write ORDER since #792: the status is written first and the comment second, so a refusal can no longer leave a verdict comment with no status behind it — the only reachable half-state is a status with no comment, which the merge hook resolves as `ask` (`release.verdict-writes-status-before-comment`). | +| `scripts/pr-changed-files.sh` — paged file enumeration | `UNSAFE-KNOWN` | #707's fix: `.base.ref`, `.base.sha` and `.head.sha` are captured before paging and re-checked after, and any *observed* movement fails the whole enumeration closed rather than emitting a short list. But before-and-after equality is **ABA-vulnerable** — a `main → scratch → main` retarget during paging can return the same ref and, if nothing merged meanwhile, the same base sha, while the pages in between were diffed against the scratch base. The script's own comment says it narrows rather than erases — "any movement fails" would be stronger than the code. Accepted here because the enumerator cannot close it alone, but be exact about what the caller-side fence does and does not cover: `ci.verdict-write-retarget-fence` counts `change_target_branch` events, so it catches the BASE alias and **nothing else**. A HEAD alias — a force-push `H1 -> H2 -> H1` during pagination — leaves the final `.head.sha` comparison equal while the middle pages were enumerated against `H2`, and NOTHING inside this script can see it. **Fenced at the enforced caller since #803** (2026-08-28): the same timeline walk now also counts `pull_push`, so a head that moved at all during classification withholds the write — subject to that walk's own terminator, which is defeatable by a page of filtered rows (**#870**, pre-existing and equally affecting the base axis). The grade stays `UNSAFE-KNOWN` for the SCRIPT, because the script's own guarantee is unchanged and one-way — a caller that does not run the fence inherits the alias. Enumeration bound to an immutable tree remains unavailable: re-probed at 1.27.1 on 2026-08-28, `compare/{base}...{head}` still returns no `files` key. | | `scripts/select-queue.sh` — issue list, then per-issue `/dependencies` | `UNSAFE-KNOWN` | The open-issue list (labels, milestone, priority) is snapshotted once; per-candidate dependency reads happen seconds later and never re-read the issue's own labels, so an issue claimed `in-progress` in that gap still appears on the shortlist. Accepted: the script authorizes **no write**. The real gate is the four-way claim check in `process.parallel-session-claim`, which runs after selection and re-reads live state by construction. Tightening this would move a check that must be adversarial into a tool that is advisory. | | `scripts/ci-detect-already-validated.sh` — prior-head combined status | `UNSAFE-KNOWN` | Reads the PR head's status and emits `skip=true`, with nothing re-checking before the consuming job runs. Accepted and narrow: the skip elides only **re-running** test/migrations on a tree already validated; the `build` job still builds and pushes unconditionally, so no image ever ships from unvalidated source. | | `scripts/issue-qualification-audit.sh` — paged issue list, then a report | `UNSAFE-KNOWN` | Pages the open-issue list and reports which issues lack a `priority:` label, so like every paged read here its pages can straddle a change and the report can name a state no single instant held. Graded to match `select-queue.sh` rather than `N/A`: the two run the same shape, and the reason offered for accepting `select-queue.sh` — it authorizes no write — cannot simultaneously be the reason this one is out of the class. Accepted on the same terms: it is advisory, session-end, human-read, and the labels it prompts for are applied by hand afterwards. | | `scripts/security-scan.sh` — `docker pull`, then `docker run` the same tag | `UNSAFE-KNOWN` | Pull and run are two steps over a MUTABLE tag, which is the same shape the registry rows below were graded down for; "resolution and use are one step" overstated it. In practice the second step resolves against the local daemon, which holds the image the pull just placed, so a mid-window retag does not change what runs. Accepted on that, plus the scope: this boots a throwaway container and scans it, authorizing nothing. | -| `scripts/migration-smoke.sh` — `docker pull`, then `docker run` the same tag | `UNSAFE-KNOWN` | The same pull-then-run over a mutable tag as `security-scan.sh` above, and graded with it rather than left behind: its row previously said "Same shape" as a note that has since been rewritten to the opposite conclusion, so the backreference had quietly inverted. This one deserves the grade MORE, not less — `security-scan.sh` boots a throwaway container and authorizes nothing, while this is the pre-deploy migration smoke that gates a production stack recreation. Accepted on the same bound (the run resolves against the local daemon holding the image the pull just placed) plus its own stated operator-trust gap: the resolved image id is **reported** for a human rather than compared against a prior read. | +| `scripts/migration-smoke.sh` — `docker pull`, then `docker run` the same tag | `UNSAFE-KNOWN` | The same pull-then-run over a mutable tag as `security-scan.sh` above, and graded with it rather than left behind — spelled out rather than cross-referenced, because a backreference to another row's conclusion inverts silently when that row is regraded. This one deserves the grade MORE, not less — `security-scan.sh` boots a throwaway container and authorizes nothing, while this is the pre-deploy migration smoke that gates a production stack recreation. Accepted on the same bound (the run resolves against the local daemon holding the image the pull just placed) plus its own stated operator-trust gap: the resolved image id is **reported** for a human rather than compared against a prior read. | | `scripts/hook-fire-log.sh` | `N/A` | Entirely local: reads stdin and writes JSONL under the cache dir; the only `curl` in the file is in a comment. | | `scripts/e2e-local.sh` | `N/A` | No outbound call at all; readiness is a local log grep and a local port probe against a subprocess it started. | | `scripts/e2e-ui.sh` | `N/A` | Launches a local Chromium and runs specs against `http://localhost:$PORT`. | @@ -135,7 +135,7 @@ classifications differ; otherwise the strictest applies and the Note names the e | `scripts/ci-toolchain-image-resolves.sh` — registry manifest read for the pinned toolchain tag | `UNSAFE-KNOWN` | Reads a MUTABLE identifier (a registry tag) with nothing re-checking it before the `container:` jobs pull, so a tag deleted between the preflight and the pull is reported as present. Graded `UNSAFE-KNOWN` rather than `N/A` deliberately: nothing proceeds on the strength of the read — it can only turn its own job red, which is not nothing (the merge-consent hook denies on the COMBINED status, ersatztv#598) but is not authorization either — while a stale PASS is read by a human as "the image is fine", which is an assertion about remote state this file exists to grade. The residual is bounded by what it degrades to: a stale pass leaves exactly the pre-#772 behaviour (five jobs failing at pull), never anything that proceeds on the strength of the read. The opposite error is closed by the EXIT CODE rather than by wording: an unusable credential, an unverifiable answer (after retries) and an HTTP 200 whose body is not a manifest all FAIL the job. THE TRAP, since warning on those and exiting 0 is the natural way to write this check: a missing `curl`, a moved registry or a DNS change all land there, and a green-with-a-warning job is indistinguishable from a healthy pin forever after — "the check could not run" presenting as "the pin is fine", which is precisely what this row would then be asserting falsely. | | `scripts/set-provider.sh` | `N/A` | Reads no live remote state — sets local `dotnet user-secrets` values. | | `scripts/__init__.py` | `N/A` | Empty package marker — executes nothing. | -| `scripts/scripted-schedules/entrypoint.py` — `ScriptedScheduleApi.get_context(build_id)`, then `define_content` / `reset_playout` / `build_playout` against the same live server | `UNSAFE-KNOWN` | A genuine read-then-act over live ErsatzTV state, and the row cold review found missing when the population was still non-recursive. The context is fetched, handed to user-supplied script functions that mutate the playout, and re-fetched after a reset with nothing pinning either read — a concurrent build or edit between them is invisible. Accepted because it runs inside a single scripted-schedule build the server itself serialises per playout, and because the API exposes no version or ETag on the context to compare against; the honest bound is that the blast radius is one playout's content, reversible by rebuilding. | +| `scripts/scripted-schedules/entrypoint.py` — `ScriptedScheduleApi.get_context(build_id)`, then `define_content` / `reset_playout` / `build_playout` against the same live server | `UNSAFE-KNOWN` | A genuine read-then-act over live ErsatzTV state. The context is fetched, handed to user-supplied script functions that mutate the playout, and re-fetched after a reset with nothing pinning either read — a concurrent build or edit between them is invisible. Accepted because it runs inside a single scripted-schedule build the server itself serialises per playout, and because the API exposes no version or ETag on the context to compare against; the honest bound is that the blast radius is one playout's content, reversible by rebuilding. | | `scripts/macOS/bundle.sh` | `N/A` | Reads no live remote state — moves files and creates symlinks in a local app bundle. | | `scripts/macOS/sign.sh` | `N/A` | `codesign --timestamp` contacts Apple's timestamp server, but resolution and use are one step with no earlier check whose answer is later trusted — the same boundary as limit 1. | | `scripts/macOS/sign-dmg.sh` | `N/A` | Same shape as `sign.sh` — a timestamped `codesign` over a local DMG, no check-then-act over remote state. | @@ -146,14 +146,14 @@ classifications differ; otherwise the strictest applies and the Note names the e |---|---|---| | `.gitea/workflows/review-verdict.yml` — status read → status POST | `UNSAFE-KNOWN` | The residual this whole class reduces to. Gitea's status API has no ETag, no If-Match and no expected-previous-state, so read and write cannot be made one operation. Narrowed rather than claimed closed: a monotonic **event-count** fence refuses to write if the PR timeline's `change_target_branch` OR `pull_push` count moved (`ci.verdict-write-retarget-fence`; counts are used because the branch *name* and the head *sha* are both ABA-vulnerable — the head axis added by #803), and a high-water-mark re-read repairs a `success` posted over a human verdict back to `pending`. The file states the residual window explicitly rather than asserting safety. | | `.gitea/workflows/review-verdict.yml` — base-ref checkout | `PINNED` | `ref: ${{ github.event.pull_request.base.sha }}` — a full sha from the fixed event payload, so the PR head cannot supply the workflow definition that judges it. | -| `.gitea/workflows/review-verdict.yml` — changed-file enumeration | `UNSAFE-KNOWN` | Delegates to `scripts/pr-changed-files.sh` with the head sha and expected base, and therefore **inherits that row's residual, not a pin** — this row said "inherits that row's pins" while the row it points at was being graded down, which is exactly the stale-cross-reference a multi-round edit produces. Accepted on better terms than the enumerator alone — this is the one caller that also runs the monotonic event-count fence (`ci.verdict-write-retarget-fence`), and since #803 that fence covers BOTH axes: `change_target_branch` for the base alias and `pull_push` for the HEAD alias described in the enumerator's row (`H1 -> H2 -> H1` during pagination). This row said the head alias was unfenced here, which was true until 2026-08-28. What remains is the residual the fence shares with the base axis — a mutation landing between the final pre-write count and the POST (#849) — not an unwatched axis. | -| `.gitea/workflows/docker-build.yml` — CI toolchain image | `UNSAFE-KNOWN` | Graded down by cold review, correctly: this file's own definition of `PINNED` names an image **digest**, and `ersatztv-ci:` is a mutable **tag**. A registry tag can be repointed after `ci-image-pin` verifies it and before a job pulls it, and a 7-hex short sha is additionally collision-prone. Accepted rather than fixed here because the exposure needs write access to our own LAN registry — i.e. an attacker already inside the trust boundary — and two controls bound it: jobs never consume `:latest`, and `pr-checks.yml`'s `ci-image-pin` fails the build if the tag drifts from the last commit touching `docker/ci/`. Consuming `image@sha256:…` is the real fix and is the natural companion to **#772**, which already covers the availability half of this tag's weakness. | -| `.gitea/workflows/docker-build.yml` — release smoke pull | `UNSAFE-KNOWN` | Pulls `${IMAGE}:${SMOKE_SHORT_SHA}`, the tag this same job pushed moments earlier. The first draft called that `PINNED` on the strength of the job's `concurrency` group; cold review showed the group is **per-ref**, so a branch build and a tag build of the same commit sit in DIFFERENT groups and can publish the same `:` — the smoke step can therefore pull the other run's image. Accepted rather than fixed here because the fix is the same one-line change as the row above (pull `image@sha256:…`, propagated from the push step) and belongs with it; until then no registry row in this file claims to be pinned. | -| `.gitea/workflows/docker-build.yml` — `api-docs` / `format` base fetch | `UNSAFE-KNOWN` | Fetches the live base tip to diff generated artifacts, with nothing pinning it. Graded up from `N/A` by cold review, which was right that "advisory" was too quick: these are not branch-protection-required contexts, but the merge-consent hook reads Gitea's **combined** status, and a combined state that is not `success` blocks the auto-grant — so a wrong answer here does participate in merge consent. Accepted because the failure direction is benign: a base that advanced mid-job makes a generated artifact look stale and FAILS the job, costing a re-run, rather than passing something it should not. | +| `.gitea/workflows/review-verdict.yml` — changed-file enumeration | `UNSAFE-KNOWN` | Delegates to `scripts/pr-changed-files.sh` with the head sha and expected base, and therefore **inherits that row's residual, not a pin** — a cross-reference to another row's GRADE goes stale the moment that row is regraded, so this one names what it inherits. Accepted on better terms than the enumerator alone — this is the one caller that also runs the monotonic event-count fence (`ci.verdict-write-retarget-fence`), and since #803 that fence covers BOTH axes: `change_target_branch` for the base alias and `pull_push` for the HEAD alias described in the enumerator's row (`H1 -> H2 -> H1` during pagination). What remains is the residual the fence shares with the base axis — a mutation landing between the final pre-write count and the POST (#849) — not an unwatched axis. | +| `.gitea/workflows/docker-build.yml` — CI toolchain image | `UNSAFE-KNOWN` | This file's own definition of `PINNED` names an image **digest**, and `ersatztv-ci:` is a mutable **tag**. A registry tag can be repointed after `ci-image-pin` verifies it and before a job pulls it, and a 7-hex short sha is additionally collision-prone. Accepted rather than fixed here because the exposure needs write access to our own LAN registry — i.e. an attacker already inside the trust boundary — and two controls bound it: jobs never consume `:latest`, and `pr-checks.yml`'s `ci-image-pin` fails the build if the tag drifts from the last commit touching `docker/ci/`. Consuming `image@sha256:…` is the real fix and is the natural companion to **#772**, which already covers the availability half of this tag's weakness. | +| `.gitea/workflows/docker-build.yml` — release smoke pull | `UNSAFE-KNOWN` | Pulls `${IMAGE}:${SMOKE_SHORT_SHA}`, the tag this same job pushed moments earlier. The job's `concurrency` group does not make that a pin: the group is **per-ref**, so a branch build and a tag build of the same commit sit in DIFFERENT groups and can publish the same `:` — the smoke step can therefore pull the other run's image. Accepted rather than fixed here because the fix is the same one-line change as the row above (pull `image@sha256:…`, propagated from the push step) and belongs with it; until then no registry row in this file claims to be pinned. | +| `.gitea/workflows/docker-build.yml` — `api-docs` / `format` base fetch | `UNSAFE-KNOWN` | Fetches the live base tip to diff generated artifacts, with nothing pinning it. Grading this `N/A` on "advisory" is too quick: these are not branch-protection-required contexts, but the merge-consent hook reads Gitea's **combined** status, and a combined state that is not `success` blocks the auto-grant — so a wrong answer here does participate in merge consent. Accepted because the failure direction is benign: a base that advanced mid-job makes a generated artifact look stale and FAILS the job, costing a re-run, rather than passing something it should not. | | `.gitea/workflows/ci-image.yml` — verify the pushed image | `UNSAFE-KNOWN` | Pulls back the `:` tag it pushed in the immediately preceding step. Same demotion and same reason as the two rows above: a registry tag is not a digest, and anything able to write to the registry can repoint it between the push and the verify, which would make the verification confirm an image other than the one built. Accepted on the same terms — the exposure requires registry write access, i.e. an actor already inside the trust boundary — and the same fix applies. | | `.gitea/workflows/pr-checks.yml` — `prove-fix` | `PINNED` | `base`/`head` are full shas from the event payload, immune to later PR mutation. | | `.gitea/workflows/pr-checks.yml` — `ci-image-pin` | `N/A` | A purely local comparison — `git log` over the checked-out tree against a literal in `docker-build.yml`. It never queries the registry, which is exactly why it cannot bound the retagging the image rows above describe. | -| `.gitea/workflows/pr-checks.yml` — `docs-reminder` base fetch | `N/A` | Fetches the live base tip, but the result only selects the text of a non-blocking warning. The rationale has to be narrower than the first draft's "the job cannot fail and never reaches the combined status", which is false — any job's status joins the combined state, and runner or checkout failure can redden it. What is true, and is what earns the `N/A`: the fetch and diff are failure-swallowed, so the remote read cannot change this job's outcome, only the warning's wording. It draws no authorization from what it reads, this file's second `N/A` clause. Grouping it with `decisions-guard` over-demoted it, and the two have different classifications, so per the rule above they get separate rows. Since ersatztv#784 the job runs TWO advisory checks off this fetch (the parity-doc reminder and `scripts/check-doc-narrative.py`); the classification is unchanged because the second is advisory on the same terms — it exits 0 on every path, so a moved base still only changes warning text (its unproven arms are enumerated in `docs.no-session-narrative`). | +| `.gitea/workflows/pr-checks.yml` — `docs-reminder` base fetch | `N/A` | Fetches the live base tip, but the result only selects the text of a non-blocking warning. The rationale has to be narrower than "the job cannot fail and never reaches the combined status", which is false — any job's status joins the combined state, and runner or checkout failure can redden it. What is true, and is what earns the `N/A`: the fetch and diff are failure-swallowed, so the remote read cannot change this job's outcome, only the warning's wording. It draws no authorization from what it reads, this file's second `N/A` clause. Grouping it with `decisions-guard` over-demoted it, and the two have different classifications, so per the rule above they get separate rows. Since ersatztv#784 the job runs TWO advisory checks off this fetch (the parity-doc reminder and `scripts/check-doc-narrative.py`); the classification is unchanged because the second is advisory on the same terms — it exits 0 on every path, so a moved base still only changes warning text (its unproven arms are enumerated in `docs.no-session-narrative`). | | `.gitea/workflows/pr-checks.yml` — `decisions-guard` base fetch | `UNSAFE-KNOWN` | Fetches the live base tip and diffs against it, unpinned, and unlike `docs-reminder` this job CAN fail — so its answer reaches the combined status the merge hook reads and therefore participates in consent. Accepted on the same grounds as the `api-docs`/`format` row: a base that moved mid-job produces a spurious FAILURE and a re-run, never a spurious pass, and it re-fetches fresh on every trigger with no state carried between runs. | | `.gitea/workflows/renovate.yml` — `renovate/renovate:43` | `N/A` | Out of this class, in scope for a different one — see the limits below. | | `.gitea/workflows/dependency-scan.yml` — NuGet advisory query | `N/A` | `dotnet list package --vulnerable` queries a live advisory database and fails the job on the report marker in the same step, so there is no check-then-act split. Worth one line anyway: a green here means "no advisories **as of this run**", which is a dated claim about mutable remote data rather than a property of the tree — which is why the scan runs on a weekly schedule instead of only on PRs. | @@ -179,8 +179,8 @@ classifications differ; otherwise the strictest applies and the Note names the e that reads no remote state earns an explicit `N/A` rather than silently staying out. The first version filtered that scope by an outbound-network token list and argued the filter was - a scope choice rather than a population filter. Cold review rejected the distinction, and the - evidence settled it: the list omitted `git fetch`, which is this repo's most common remote read, + a scope choice rather than a population filter. That distinction does not survive the + evidence: the list omitted `git fetch`, which is this repo's most common remote read, so `prepush-rebase-check.sh` — which fetches `origin/main` and derives a **push decision** from it — was structurally invisible to a guard claiming to cover "every executable that reads live remote state", along with three others. The defence offered was that over-inclusion is the safe diff --git a/docs/superpowers/plans/2026-07-20-qsv-native-decode.md b/docs/superpowers/plans/2026-07-20-qsv-native-decode.md index 40e15336a..c1947a470 100644 --- a/docs/superpowers/plans/2026-07-20-qsv-native-decode.md +++ b/docs/superpowers/plans/2026-07-20-qsv-native-decode.md @@ -265,7 +265,7 @@ git -c core.hooksPath=/dev/null commit -m "feat(498): QsvHardwareAccelerationOpt - Consumes: `FFmpegState.QsvPreferNativeDecoder` (`bool?`, Task 2 — coerce with `!= false`); `QsvHardwareAccelerationOption(..., preferNativeDecoder)` (Task 3). - Produces: `DecoderVaapiToSoftware` (software-output implicit VA-API decoder); a QSV pipeline whose decode side is VA-API-to-software when the flag is set. -**Design note (READ — Fable review corrected the original approach):** The naive approach (set `DecoderHardwareAccelerationMode = Vaapi` with `-hwaccel_output_format vaapi`, reuse `DecoderVaapi`) is **BROKEN**: `DecoderVaapi` outputs *hardware* VA-API surfaces, and `QsvPipelineBuilder.SetScale` unconditionally chooses `ScaleQsvFilter` when the encoder is QSV — which, on hardware frames, emits a **bare `vpp_qsv`** (even for same-resolution content, via the "still need pixel format" branch at `ScaleQsvFilter.cs:41-47`) directly onto VA-API frames. ffmpeg does NOT auto-`hwmap` between hw contexts → the graph fails to configure on ~all content. `DeinterlaceQsvFilter` fails the same way on interlaced content. The `HardwareDownloadFilter` at `QsvPipelineBuilder.cs:204` runs *after* scale/deinterlace, too late to save it. +**Design note (READ — the naive approach below does not work):** The naive approach (set `DecoderHardwareAccelerationMode = Vaapi` with `-hwaccel_output_format vaapi`, reuse `DecoderVaapi`) is **BROKEN**: `DecoderVaapi` outputs *hardware* VA-API surfaces, and `QsvPipelineBuilder.SetScale` unconditionally chooses `ScaleQsvFilter` when the encoder is QSV — which, on hardware frames, emits a **bare `vpp_qsv`** (even for same-resolution content, via the "still need pixel format" branch at `ScaleQsvFilter.cs:41-47`) directly onto VA-API frames. ffmpeg does NOT auto-`hwmap` between hw contexts → the graph fails to configure on ~all content. `DeinterlaceQsvFilter` fails the same way on interlaced content. The `HardwareDownloadFilter` at `QsvPipelineBuilder.cs:204` runs *after* scale/deinterlace, too late to save it. **Correct approach — produce SOFTWARE frames after VA-API decode.** Emit `-hwaccel vaapi` WITHOUT `-hwaccel_output_format` (Task 3) so ffmpeg downloads decoded frames to system memory, and use a decoder whose `OutputFrameDataLocation` is `Software`. Then every QSV filter takes its proven software branch: `ScaleQsvFilter` → `format=nv12,hwupload=extra_hw_frames=N,vpp_qsv` (`ScaleQsvFilter.cs:83-86`); `DeinterlaceQsvFilter` → `format=nv12,hwupload=…,deinterlace_qsv` (`DeinterlaceQsvFilter.cs:17-18`); `SetTonemap` picks the software `TonemapFilter` because the decoder mode is not `Qsv` (`QsvPipelineBuilder.cs:705`). This is **exactly the graph the production error-loop paths already run** (`FFmpegLibraryProcessService.cs:782/923` — software decode + QSV encode), just with VA-API-accelerated tolerant decode instead of a CPU decoder. Trade-off (state honestly in the PR): a PCIe download/upload round-trip, NOT Jellyfin's zero-copy `hwmap=derive_device=qsv` (that filter does not exist in this codebase and is out of scope). We keep the QSV *encoder* and the tolerant *decoder*, which is the whole point of #498. diff --git a/docs/superpowers/plans/2026-07-20-unified-logo-bug.md b/docs/superpowers/plans/2026-07-20-unified-logo-bug.md index 15699ba3e..474ce4926 100644 --- a/docs/superpowers/plans/2026-07-20-unified-logo-bug.md +++ b/docs/superpowers/plans/2026-07-20-unified-logo-bug.md @@ -731,7 +731,7 @@ git commit -m "feat(67): show resolved bug geometry on the watermarks screen" ): T | null; ``` -**Behavior contract (read carefully — the first draft of this plan got this wrong):** +**Behavior contract (read carefully — the ticked-state rule below is a trap):** - **Ticked** iff the watermark that `draft.watermarkId` *points at* has `imageSource === 'ChannelLogo'`. Resolve it by **id lookup**, never by searching the list for the first `ChannelLogo` entry: `getWatermarks()` sorts by name (`web/src/api/pickers.ts:14`), so a search silently binds to the alphabetically-first logo-driven preset. Since the design explicitly invites users to create a second preset for per-channel geometry, that search would (a) show the toggle OFF for a channel that *is* logo-driven and (b) **repoint the channel** on tick — a data-changing action from a control documented as a pure reflection. - **Tick-on target** is `findLogoBugWatermark(...)`: prefer the preset named exactly `Channel Bug`, else the first `ChannelLogo` preset, else `null`. With several logo-driven presets `imageSource` alone cannot identify *the* default, so the name is a deterministic tiebreak — not the identification mechanism. - Loading and re-saving a channel without touching the toggle must leave `watermarkId` **byte-identical**, including when it points at a non-`ChannelLogo` watermark or at a `ChannelLogo` preset that is not the seeded one. @@ -1151,8 +1151,7 @@ shared row already delivers the user-visible behavior with no schema change. editor — quick-add creates through the API and then navigates to the editor, so the editor only ever loads an existing row. The editor toggle reflects the **referenced** watermark's `imageSource`; an earlier draft searched the list instead, which (because `getWatermarks()` sorts by name) would have - silently repointed channels bound to a non-first logo-driven preset. Caught in independent review - and pinned by a regression test. + silently repointed channels bound to a non-first logo-driven preset. Pinned by a regression test. - The builder flow is covered on **fresh installs only**, by stamping the preset onto the system channel templates the seed itself creates; existing installs' templates are never mutated. - Server-side create defaulting was rejected: making an omitted `watermarkId` mean "give me a diff --git a/docs/superpowers/plans/2026-07-21-decision-lifecycle-and-kickoff.md b/docs/superpowers/plans/2026-07-21-decision-lifecycle-and-kickoff.md index d9319e620..af7e6671d 100644 --- a/docs/superpowers/plans/2026-07-21-decision-lifecycle-and-kickoff.md +++ b/docs/superpowers/plans/2026-07-21-decision-lifecycle-and-kickoff.md @@ -22,7 +22,7 @@ Actions (`.gitea/workflows/docker-build.yml`), Husky hooks. - Work in the worktree `feat/520-521-decision-lifecycle` off `origin/main`. Never commit in `/Users/timothy/ersatztv`. -- **Two PRs, one arc** (Fable review #6; matches #521's "bounded topic batches, avoid a single +- **Two PRs, one arc** (matches #521's "bounded topic batches, avoid a single conflict-heavy rewrite"): - **PR1 — decision-lifecycle machinery** (Tasks 1, 2, 3, 5, 7): parser, validator, catalog generator, exemplar reconciliations (incl. dogfooding the append-only→lifecycle supersession), @@ -37,7 +37,7 @@ Actions (`.gitea/workflows/docker-build.yml`), Husky hooks. are cheap (a concurrently appended unmigrated entry just gets a metadata block on rebase). - Each PR: rebase-then-`merge_when_checks_succeed`. On rebase conflicts in generated artifacts (`docs/decisions/README.md`), REGENERATE — never hand-merge. -- **`[decisions-edit]` is kept narrow, not retired** (Fable review #3/#4): lifecycle *metadata* writes +- **`[decisions-edit]` is kept narrow, not retired**: lifecycle *metadata* writes (add record, set `superseded-by`, move to archive, regen catalog) are token-free — the validator proves them well-formed. A change to a record's *rationale prose* still requires `[decisions-edit]` in the commit range, enforced by the validator's body-diff (Task 2). This closes the silent-rewrite diff --git a/docs/superpowers/specs/2026-07-20-unified-logo-bug-design.md b/docs/superpowers/specs/2026-07-20-unified-logo-bug-design.md index 4b444427d..c4d6a5baf 100644 --- a/docs/superpowers/specs/2026-07-20-unified-logo-bug-design.md +++ b/docs/superpowers/specs/2026-07-20-unified-logo-bug-design.md @@ -101,8 +101,7 @@ user-editable name. Legal under frozen-additive `/api/v1`; also useful to MCP/ma reliably tell "new" from "existing". The default is therefore applied at the SPA's quick-add creation path — `ChannelsScreen.tsx:377`, which currently hardcodes `watermarkId: null` — by setting `watermarkId` to the seeded preset at create time. -- **The `ChannelBuilder` lineup flow is covered via template stamping** (revised after independent - review). It does not hardcode a watermark: it *inherits* `watermarkId` from the selected +- **The `ChannelBuilder` lineup flow is covered via template stamping.** It does not hardcode a watermark: it *inherits* `watermarkId` from the selected `ChannelTemplate` (`web/src/builder/ChannelBuilder.tsx:1025`, `:2046`). Because §3's seed stamps the preset onto templates **it is creating for the first time**, a fresh install gets the default through both creation flows with no user data mutated. On an existing install (including production) the @@ -116,7 +115,7 @@ user-editable name. Legal under frozen-additive `/api/v1`; also useful to MCP/ma list for the first `ChannelLogo` preset; because `getWatermarks()` sorts by name (`web/src/api/pickers.ts:14`), that silently repointed a channel to the alphabetically-first logo-driven preset the moment a second one existed — which this design's own trade-off invites - users to create. Caught in independent review. + users to create. - **Tick-on target is deterministic**: prefer the preset named `Channel Bug`, else the first `ChannelLogo` preset. With multiple logo-driven presets, `imageSource` alone cannot identify *the* default, so a name preference is unavoidable here; it is a tiebreak, not the identification @@ -146,10 +145,10 @@ Existing logo-upload and external-URL-precedence behavior (`docs/decisions.md:45 - **Vitest**: `BugPreview` geometry mapping across all 9 locations, plus opacity/size/margins. For the toggle: - **loading and re-saving an existing channel without touching the toggle leaves `watermarkId` - byte-identical** — the guarantee protecting the 43 production channels, and the test the first - draft of this plan promised but omitted; + byte-identical** — the guarantee protecting the 43 production channels, and a case the test + suite must cover explicitly; - with **two** `ChannelLogo` presets, a channel pointing at the non-first one still reads as ticked - and is not repointed (the review's blocker, pinned by a regression test); + and is not repointed (pinned by a regression test); - the creation path stamps the preset, and **degrades to `null` rather than aborting** when the watermark lookup fails. diff --git a/docs/superpowers/specs/2026-07-21-decision-lifecycle-and-kickoff-design.md b/docs/superpowers/specs/2026-07-21-decision-lifecycle-and-kickoff-design.md index a86665ee0..ae392f256 100644 --- a/docs/superpowers/specs/2026-07-21-decision-lifecycle-and-kickoff-design.md +++ b/docs/superpowers/specs/2026-07-21-decision-lifecycle-and-kickoff-design.md @@ -46,7 +46,7 @@ line, and retiring *moves* a record to archive. **Decision:** replace the line-level mechanic with a **lifecycle validator** that enforces the same *spirit* — rationale is never silently rewritten or deleted; every history touch is deliberate and -reviewable. The `[decisions-edit]` token is **kept narrow, not retired** (revised after Fable review): +reviewable. The `[decisions-edit]` token is **kept narrow, not retired**: routine lifecycle *metadata* writes (add record, set `superseded-by`, relocate to archive, regen catalog) are token-free and proven well-formed by the validator; a change to a record's *rationale prose* still requires `[decisions-edit]`, enforced by the validator's **body-diff**. Retiring the @@ -60,7 +60,7 @@ active version, without `[decisions-edit]` in the commit range**; supersession l and resolvable; status only moves active → superseded/retired. The no-vanish/body-diff checks run in CI over `merge-base(base,head)…head` across `decisions.md` **and** every topic file. -**Delivery: two PRs, one arc** (revised after Fable review). PR1 = machinery (parser, validator, +**Delivery: two PRs, one arc.** PR1 = machinery (parser, validator, catalog generator, guard swap, header rewrite, exemplar + append-only→lifecycle supersession); PR2 = full corpus migration + #520 startup rewrite + kickoff guard + retrieval-eval. Matches #521's "bounded topic batches, avoid a single conflict-heavy rewrite" and keeps each diff's conflict surface diff --git a/scripts/check-doc-narrative.py b/scripts/check-doc-narrative.py index 4cf7cdab9..061864f85 100755 --- a/scripts/check-doc-narrative.py +++ b/scripts/check-doc-narrative.py @@ -44,6 +44,16 @@ except Exception: # noqa: S110 — a stdout that cannot be reconfigured is not # `docs/decisions/**` is exempt WHOLESALE — a decision record narrating how a rule was got wrong is # carrying the rationale it exists to carry, so a detector that flagged it would fight the # convention it serves. +# +# `docs/superpowers/**` was CONSIDERED for this list in #812 and deliberately REJECTED. Recorded here +# so it is not re-proposed on plausibility: the argument was that those 35 dated plans are frozen +# (measured 2026-08-29, before this change: nothing edited there since 2026-07-23), so a sweep over +# them yields only carve-out false positives. But +# `--diff` — the mode CI runs — scans ADDED lines, and a frozen file contributes none, so exempting +# them buys nothing there. What it WOULD suppress is any plan being WRITTEN OR REVISED now — 15 of +# the 35 have more than one commit, so revisions do happen — which is precisely the case where the +# rule's remedy (move it to the commit message) is still available. The exemption costs the only +# reach the detector has and buys only quiet in a sweep a person runs deliberately. EXEMPT_PREFIXES = ("docs/decisions/",) # Session-narrative phrasings. Deliberately narrow: each is first person or names a revision of THIS diff --git a/scripts/tests/test_check_doc_narrative.py b/scripts/tests/test_check_doc_narrative.py index 863b30a96..03b23a1e6 100644 --- a/scripts/tests/test_check_doc_narrative.py +++ b/scripts/tests/test_check_doc_narrative.py @@ -12,6 +12,7 @@ number written into the test — a hand-written expectation is a second copy of from __future__ import annotations +import importlib.util import re import subprocess import sys @@ -260,20 +261,149 @@ def test_a_RENAME_does_not_re_flag_the_whole_pre_existing_file(repo: Path) -> No # --- the population ------------------------------------------------------------------------------ +# Hand-written, one entry per exempt prefix. NOT derived from the script — a population read out of +# the code under test proves only that the code agrees with itself. The completeness guard below is +# what stops a new prefix from shipping without a case here. +EXEMPT_SAMPLES = { + "docs/decisions/": ("docs/decisions/records/x/y.md", "docs/decisions/archive/x/y.md"), +} + + @pytest.mark.parametrize("mode", ["--all", "--diff"]) -def test_decision_records_are_exempt_in_BOTH_modes(repo: Path, mode: str) -> None: - for rel in ("docs/decisions/records/x/y.md", "docs/decisions/archive/x/y.md"): - p = repo / rel - p.parent.mkdir(parents=True, exist_ok=True) - p.write_text(f"x\n{NARRATIVE}\n", encoding="utf-8") +def test_every_exempt_genre_is_exempt_in_BOTH_modes(repo: Path, mode: str) -> None: + """`docs/decisions/**` is exempt WHOLESALE (#784). Both modes, because an exemption that held + only for `--all` would still flag the file on the PR that adds it. + + The CONTROLS are what make this a measurement. `hits(out) == set()` alone also passes when the + scan examined nothing at all — a scanned-zero run reads identical to a working exemption — so a + non-exempt file carrying the same marker must come back HIT in the same invocation. + + Each exempt sample is also paired with a MINIMAL TWIN — the same path with the exempt directory + renamed — which raises the cost of a structural bypass but does NOT close it. Stated as a limit + rather than a guarantee, because three successive attempts to close it were each defeated by the + next round: a lone depth-1 control fell to a depth-1 population rule; a depth-3 control fell to a + `count("/") <= 3` cap; the twins themselves fell to a directory-NAME rule keyed on the renamed + SECOND segment (`path.count("/") > 1 and "-" not in path.split("/", 2)[1]`), which skipped both + samples and kept both twins. It reddens here now, but only because the `docs/superpowers/` + fixture below happens to have a hyphen-free second segment — luck, not design, and not a + property this test establishes. + + So this test proves the scan EXAMINED something; it does NOT prove a sample's absence is + attributable to `EXEMPT_PREFIXES` rather than to its shape. Do not add a fourth finite control — + each of the three was locally correct and the sequence did not converge, the same shape as the + withdrawn `test_review_verdict_vocabulary_parity.py` (six rounds, then deleted) that + `docs.no-session-narrative` cites as the case for stopping. What DOES close it is a closed-form + proof rather than a fixture: + `test_the_population_agrees_with_an_INDEPENDENT_RESTATEMENT_over_the_REAL_corpus` below. + """ + for rels in EXEMPT_SAMPLES.values(): + for rel in rels: + f = repo / rel + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(f"x\n{NARRATIVE}\n", encoding="utf-8") + # The twin of `docs/decisions/records/x/y.md` is `docs/decisions-not/records/x/y.md`: identical + # in every structural respect, and NOT under the exempt prefix. + twins = {rel.replace(prefix, prefix[:-1] + "-not/", 1) for prefix, rels in EXEMPT_SAMPLES.items() for rel in rels} + # `docs/superpowers/` is deliberately NOT exempt (#812), and re-exempting it is the specific + # regression this change argues against. It is named here rather than left to the closed-form + # oracle, because that oracle proves a property of `is_scanned_path`, not that the SCAN consults + # it: a `continue` added in `run_all`/`run_diff` reinstates the exemption with the function + # untouched. Only a path that reaches the OUTPUT witnesses the wiring. + twins = twins | {"docs/superpowers/plans/2026-01-01-x.md"} + for rel in twins | {"docs/control.md"}: + control = repo / rel + control.parent.mkdir(parents=True, exist_ok=True) + control.write_text(f"x\n{NARRATIVE}\n", encoding="utf-8") commit(repo) out = run(repo, mode, "HEAD~1").stdout if mode == "--diff" else run(repo, mode).stdout - assert hits(out) == set() + assert hits(out) == {(rel, 2) for rel in twins | {"docs/control.md"}} + + +def test_the_population_agrees_with_an_INDEPENDENT_RESTATEMENT_over_the_REAL_corpus() -> None: + """CLOSED-FORM. This is what three successive finite controls could not buy, and it is why the + retraction in the exemption test's docstring stops at "not another fixture" rather than at + "unprovable". + + The rule as the record states it — `docs/**/*.md` minus `docs/decisions/**`, plus root-level + `*.md` — is restated below over path SEGMENTS rather than string prefixes, so the two are not + one expression copy-pasted, then compared against `is_scanned_path` on every tracked Markdown + path in this repo. A finite fixture can only witness the shapes someone thought to write down; + this witnesses every real path, so any population change that alters a verdict on one of them + reddens — the depth-1 rule and the directory-NAME rule among them. + + It is NOT sufficient on its own, and the arm below says why with the measurement: a + `count("/") <= 3` cap changes no real path's verdict today, so within this test only the + synthetic arm catches it. Do not trim that arm as belt-and-braces; it is the half that sees a + rule the corpus has no instance of yet. (The exemption test above happens to redden on that cap + too — its two minimal twins sit at four slashes — but that is incidental, not the arm carrying the claim.) + """ + + def restated(path: str) -> bool: + parts = path.split("/") + if not parts[-1].endswith(".md"): + return False + if len(parts) == 1: + return True # root-level *.md + if parts[0] != "docs": + return False # nested markdown outside docs/ is out of scope + return parts[1] != "decisions" # docs/** minus docs/decisions/** + + spec = importlib.util.spec_from_file_location("_cdn_pop", SCRIPT) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + + rc = subprocess.run(["git", "ls-files", "-z", "--", "*.md"], cwd=REPO, capture_output=True, text=True, check=True) + paths = [p for p in rc.stdout.split("\0") if p] + assert len(paths) > 100, f"corpus looks wrong: {len(paths)} paths" + + disagree = [p for p in paths if mod.is_scanned_path(p) != restated(p)] + assert not disagree, f"population disagrees with the restated rule on {len(disagree)}: {disagree[:8]}" + + # Anti-vacuity: an oracle that says False everywhere would agree with a scan-nothing mutant. + scanned = [p for p in paths if restated(p)] + exempt = [p for p in paths if p.startswith("docs/decisions/")] + assert len(scanned) > 50 and len(exempt) > 50, f"{len(scanned)} scanned / {len(exempt)} exempt" + + # The real corpus alone cannot see a rule that differs only on a shape it does not currently + # contain — no scanned path carries more than three slashes today, so a `count("/") <= 3` cap is + # a no-op against it and would ship latent. So the same oracle is also compared over a cross + # product of FOUR dimensions a population rule has been observed to key on here — depth, first + # segment, second segment, extension — widened with basename, case and dotted directories after + # a review round found a mutant keyed on each. This ENUMERATES; it is not a universal over the + # space of rules, and a dimension on neither list has simply not been measured. + heads = ["docs", "Docs", "web", "scripts", ".github", ""] + seconds = ["decisions", "superpowers", "other", "decisions-not", "Decisions", ".hidden", ""] + leaves = ["f", "index", "README"] + exts = [".md", ".markdown", ".MD", ".txt", ""] + synthetic = set() + for ext in exts: + for leaf in leaves: + for head in heads: + for second in seconds: + for depth in range(0, 3): + segs = [p for p in (head, second) if p] + ["x"] * depth + synthetic.add("/".join(segs + [leaf + ext])) + bad = [p for p in sorted(synthetic) if mod.is_scanned_path(p) != restated(p)] + assert not bad, f"population disagrees with the restated rule on {len(bad)} shapes: {bad[:8]}" + + +def test_every_exempt_prefix_has_a_case_above() -> None: + """Completeness: adding a prefix to `EXEMPT_PREFIXES` without a sample above reddens here, so the + exemption test cannot silently stop covering the population it claims to cover.""" + spec = importlib.util.spec_from_file_location("_cdn", SCRIPT) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + assert set(EXEMPT_SAMPLES) == set(mod.EXEMPT_PREFIXES) + for prefix, rels in EXEMPT_SAMPLES.items(): + assert all(r.startswith(prefix) for r in rels), prefix def test_markdown_outside_the_stated_population_is_not_scanned(repo: Path) -> None: - """The population is `docs/**/*.md` minus `docs/decisions/**`, plus root-level `*.md`. A skill - under `.claude/` is out of scope, and the record's `mechanics:` says so.""" + """The population is `docs/**/*.md` minus `EXEMPT_PREFIXES`, plus root-level `*.md`. A skill + under `.claude/` is out of scope, and the record's `mechanics:` says so — out of the DETECTOR, + still bound by the RULE.""" (repo / ".claude" / "skills" / "s").mkdir(parents=True) (repo / ".claude" / "skills" / "s" / "SKILL.md").write_text(f"{NARRATIVE}\n", encoding="utf-8") (repo / "README.md").write_text(f"{NARRATIVE}\n", encoding="utf-8")