fix(812): classify the narrative sites by who-benefits; keep the detector's reach (#882)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 22s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 9m18s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m33s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Skipped
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m52s
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 22s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 9m18s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m33s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Skipped
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m52s
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
This commit was merged in pull request #882.
This commit is contained in:
@@ -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
|
||||
|
||||
+26
-28
@@ -819,7 +819,7 @@ checkout + detect and **reports `success` in seconds**.
|
||||
out shallow (`fetch-depth: 2`), and
|
||||
a shallow clone has **no `origin/<base>` 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
|
||||
|
||||
@@ -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/<area>/<topic>.md` (archived ones at `docs/decisions/archive/<area>/<topic>.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) |
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
+15
-16
File diff suppressed because one or more lines are too long
@@ -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:<short-sha>` 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 `:<short-sha>` — 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:<short-sha>` 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 `:<short-sha>` — 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 `:<sha>` 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
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user