`pretooluse-merge-consent.sh` proves all three consent conditions at the moment
the merge tool is called. With `merge_when_checks_succeed=true`, condition (a) is
delegated to Gitea, which then merges whatever head is green at ITS merge time —
while (b) Done-when and (c) the head-referencing verdict were proven against the
head at SCHEDULING time. Every commit pushed in between merges unreviewed. The
gate is not bypassed; it is satisfied against a snapshot that stops being true.
Demonstrated as a controlled A/B rather than inferred, with a CI check left
pending so Gitea waits as it really does:
without a required verdict context: unreviewed commit B MERGED
with it: same sequence REFUSED
after reviewing B and posting it: merges — blocked, not deadlocked
NOTE the anecdote in #622 is wrong and is corrected in the docs rather than
repeated: PR #619 does carry `Review-verdict: MERGEABLE @ 02c82b35`, posted six
seconds before the merge, explicitly re-reviewing the follow-up commits. #622 was
filed off a lagging API read. The hole is real regardless, and structural —
nothing FORCED that re-review inside the window Gitea would have merged in. This
turns a property that held by diligence into one that holds by construction.
The fix is the sha, not a smarter check. A Gitea commit status belongs to exactly
one commit, so a status written for a parent cannot be inherited by a child that
did not exist yet. `review-verdict/h10` becomes a REQUIRED status check on main:
push a new head and the context is simply absent, which Gitea reads as
not-passing (verified against 1.25.4: missing AND pending both block, and
auto-merge re-checks the current head). It also covers merge paths the hook never
sees — Gitea UI, raw API, another agent's session.
- scripts/post-review-verdict.sh writes the comment and the status together so
they cannot drift, and re-reads head after commenting: if a commit landed
mid-flight it writes NO status and exits non-zero rather than retargeting the
verdict at a commit nobody read.
- .gitea/workflows/review-verdict.yml auto-passes the two exempt classes that
would otherwise deadlock — Renovate-authored (platformAutomerge) and docs-only
— and marks everything else pending. Exemptions are void when the PR touches
.claude/, .gitea/, .husky/, scripts/ or docker/ci/.
- The hook refuses to SCHEDULE an auto-merge unless that status is green on head,
and no longer claims "CI green" on the mwcs path, where it never read CI.
Two silent false negatives in the exemption path, both found by verifying rather
than reasoning, both fixed at BOTH call sites (workflow and the hook's
pre-existing docs-only carve-out):
- The files endpoint caps at 50 rows and IGNORES a larger `limit` — PR #619 has
194 changed files and `?limit=100` returns 50. A single page saw ZERO protected
paths there where the full enumeration finds ten. Both now page to exhaustion
and withhold the exemption if they cannot complete.
- A rename is ONE row whose `filename` is the destination, the source only in
`previous_filename`. Verified live: `.gitea/workflows/renovate.yml` ->
`docs/innocuous-note.md` presented as docs-only with no protected path visible.
Both now read BOTH sides.
Limits are documented, not papered over: base changes leave the head sha (and so
the verdict) unchanged, and a PR editing the workflow is judged by its own edited
copy — so PROTECTED is a guardrail against accident, not a tamper-proof control.
fixes#622
This is why `decisions lifecycle` went red, and it was NOT the known flake. I
came close to dismissing it as one for the second time this session, because an
earlier red on another branch genuinely was.
The dual-format parser imported PyYAML to read frontmatter. `decisions-guard`
does `setup-python` and installs NOTHING, so once the corpus was migrated every
record became unparseable there: ModuleNotFoundError, job fails. The same would
hit the Husky pre-commit hook and every contributor's machine.
Installing PyYAML in CI is the wrong fix: READING happens everywhere -- CI, the
hook, every dev -- while WRITING happens once, in a migration a human runs
deliberately. So the read path is now dependency-free and only
`migrate_decisions_split` (the writer) still imports yaml.
A hand-rolled parser is only safe if it provably matches the library that WROTE
the files, so `test_frontmatter_reader_matches_pyyaml_on_every_real_record`
compares the two field-by-field across all 169 real records (importorskip, so it
is skipped rather than failing where PyYAML is absent) with a >100-file guard
against near-vacuity. It is narrow by construction: the frontmatter is
machine-generated with default_flow_style=False and width=10**9, so every value
is a single-line scalar, and the reader bails to None on anything nested.
Verified by running all four affected entry points against a shim that makes
`import yaml` raise: validate --base/--head, catalog --check, the kickoff guard,
and the plain validate the pre-commit hook calls. All exit 0.
refs #610
Three Done-when items that were genuinely unfinished. I audited the checklist
before ticking it rather than after, and these were not done.
1. BODY-DIFF GUARD RE-KEYED FROM HEADING TO `key`. Heading-keying made a pure
RENAME look like a removal plus an unrelated addition, so retitling a record
failed CI as an "unlogged removal" -- a trap that has bitten this repo before.
Records are now MATCHED by key and REPORTED by heading (a key alone is not
enough for a human to find the record). Demotion, which is inherently about a
record losing its key, is detected on the heading axis instead, and a demoted
record is no longer double-reported as removed. Key-matching is also what lets
the comparison work straight through the legacy->frontmatter migration, since
`key` survives both the format change and the move between files.
Pinned by a test driving the REAL git-backed diff engine: retitle a record,
same key, byte-identical prose -> removed/rewritten/demoted all empty.
Mutation-verified: restoring heading-keying makes it fail with exactly
"a retitle was reported as a removal".
2. PATH <-> KEY ENFORCED BY THE VALIDATOR. The filename is derived from the key,
which is what makes one-active-per-key a filesystem property rather than a
check -- but only if the two cannot drift. Now an error when they disagree,
with legacy multi-record files explicitly exempt (they have no key-derived
path to match).
3. ONE-ACTIVE-PER-KEY PINNED AS A FILESYSTEM PROPERTY. Two records sharing a key
derive the same path, so the filesystem refuses the second; the migration
aborts on a destination collision rather than silently overwriting. Both are
now tested, the latter end-to-end through `main()`.
Also fixed a vacuous assertion ruff caught in my own new test -- `assert X or
True` is always true.
Note on verification: my first positive control for the guard reported it NOT
firing. That was the probe, not the code -- "load 340" appears in both the
`sources:` frontmatter field and the body, and replace(...,1) hit the metadata
copy, which is correctly token-free. Re-run against genuine body prose, the guard
fires with exit 1.
refs #610
168 records -> docs/decisions/records/<area>/<topic>.md (163 active, 23 dirs) and
docs/decisions/archive/<area>/<topic>.md (5 archived). The filename IS the key,
so one-active-record-per-key becomes a filesystem property rather than a
validator check, and supersession becomes a `git mv`.
WHY: the monolith was a concurrency problem before an aesthetic one. A
3,900-line append target made parallel sessions collide -- PR #605 and PR #614
both hit append-vs-append conflicts during routine rebases, and hand-resolving
those inside the corpus is exactly the operation the rationale-rewrite guard
exists to police.
HOW IT IS VERIFIED: a ~170-file diff cannot be meaningfully read, so correctness
does not rest on reading it. The parser was taught BOTH formats first, so the
body-diff guard parses the old form at the merge-base and the new form at head --
the migration validates itself, no bypass. The proof is a field-level equivalence
harness: 168 records before and after, zero lost, zero gained, zero field
mismatches, zero rationale bodies differing. Reviewers should scrutinise the
harness; it is the actual evidence.
What measuring caught that reading would not have:
- ~500 lines sit OUTSIDE any record -- decisions.md's lifecycle schema and each
topic file's preamble, mostly the only copy. Source files are kept and
stripped, never deleted. They also cannot be filed per-area: topic files hold
several areas and 4 of 23 areas span several files.
- Archive discovery was a non-recursive glob; after the split it found ZERO
archived records, surfacing as four bogus "supersedes points to unknown key"
errors rather than an obvious failure.
- ~32 live docs point into the corpus BY DATE, which the split dangles. Each
stripped file now ends with a generated "Records formerly in this file" index,
which also rescues the identical breadcrumbs in old issue comments.
- decisions.md's "In this file:" list was 97 same-file anchor bullets that the
split makes WRONG, not merely stale. Dropped; the generated index replaces
them with links that resolve.
The equivalence harness now runs against a checked-in FIXTURE, not the live
corpus. The earlier version migrated the real tree, which made it a one-shot:
the moment the migration landed there was nothing left to move and the tests
failed for reasons unrelated to the code. A fixture keeps them testing the
SCRIPT rather than the repo's current state.
Keys preserved verbatim, warts included: `sched` (12) and `scheduling` (1) remain
two directories for one concept. Renaming a key is not a move -- it changes
identity, breaks the equivalence proof, and invalidates MemPalace's per-key
drawers. Taxonomy normalisation is separate work.
refs #610
Both surfaced when main gained two records mid-flight.
- test_record_count_is_the_expected_166 hardcoded the total, so it failed the
moment a record landed on main -- a merge turning an unrelated test red. The
real invariant is before == after; the count only needs to prove the harness
isn't parsing a stub corpus, so it is now equality plus a floor.
- The migration's "lines preserved" figure silently absorbed the generated
where-did-it-go index once that was threaded into the preamble string, jumping
507 -> 759 with no new prose preserved. It now reports the two separately:
514 lines of original prose, plus 245 generated index lines. A number that
quietly changes meaning is worse than no number.
Live docs point into the corpus BY DATE -- "see `decisions.md` 2026-07-10" --
about 32 such references across 12 files, plus the same form in historical issue
comments. The split would dangle every one of them.
Each stripped file now ends with a generated "Records formerly in this file"
index: date, title, and a link to the record's new path. A reader following a
date pointer lands on the file it names and resolves from there. That is far
cheaper and less error-prone than rewriting 32 references by hand, and it also
covers the issue-comment breadcrumbs, which cannot be rewritten at all.
Caught while verifying it: the generated `## Records formerly in this file`
heading is itself an H2, so the record parser counted one legacy-unmigrated
record per stripped file -- the notice went 0 -> 6. Same treatment as the
existing `## Index` section: skip it by name. SKIP_HEADINGS moved to
decisions_lib as the single source of truth, since three modules now need it.
Found by reading the validator's notice output on a trial migration, not by
inspection -- the corpus still validated OK, so nothing else would have flagged it.
The migration is a MOVE, so correctness is provable rather than reviewable:
parse the corpus before, migrate, parse after, assert the Record sets are
identical field-for-field with byte-identical rationale.
scripts/migrate_decisions_split.py
166 records -> docs/decisions/records/<area>/<topic>.md (active) or
docs/decisions/archive/<area>/<topic>.md (superseded/retired), 26 directories.
Path is DERIVED from the key, so identity stays the key. Refuses to run if any
record lacks a key, and aborts on a destination collision.
Source files are KEPT, stripped to their narrative -- not deleted. 506 lines of
the corpus sit outside any record: decisions.md's lifecycle-schema header (the
status vocabulary, supersession rules, the edit-token contract) and each topic
file's preamble explaining why those records exist. For most of it that is the
only copy. It also cannot be filed per-area -- topic files hold several areas
and 4 of 23 areas span several files -- so the files themselves stay.
scripts/tests/test_migration_equivalence.py
Runs the real migration against a COPY of the real corpus in tmp_path, never
the working tree. Asserts: all 166 records survive with the same keys, every
metadata field round-trips, titles round-trip from headings, every rationale
body is byte-identical, path matches key, archived records land in the archive
wing, the legacy files keep their narrative, and no parseable record is left
behind in them.
Proven non-vacuous: corrupting one migrated record's prose is caught by the
byte-identical check, and deleting one is caught by the survival check.
One test-authoring note: an early assertion string-matched "## " to prove no
records were left in decisions.md. That is wrong -- the schema header quotes an
illustrative "## 2026-07-17 ..." example in prose. Whether records remain is a
PARSING question, so the parser-based leftover test is the real invariant.
Cross-family review (High + Medium + Low), all three reproduced before fixing:
High -- on a pull_request event actions/checkout lands on a synthetic merge commit whose
body the forge composes from the PR description, so a description ending in an example
marker armed a guard no author armed. Excluded merge commits from the range; merging main
into a PR branch is separately forbidden, so no author-written commit is skipped.
Medium -- a folded value (`no` + continuation ` yes`) was split into independent lines and
the continuation armed on its own, inverting the value the author wrote. Read with `unfold`
so the value is judged whole.
Low -- the module docstring promised fail-open while marker resolution deliberately fails
closed. The posture is right; the docstring was wrong. Documented as the one exception.
Both new negative controls verified red against the unfixed matcher. Decision record
amended to state the residual honestly rather than overclaim: a quoted example that is the
FINAL paragraph of an ordinary commit is a trailer by git's own grammar and does arm. What
the change buys is that discussing the marker can no longer disable anything.
fixes#609
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rationale-edit exemption was a substring test over the whole commit range, so any
message containing the literal marker armed it -- including prose explaining why no
marker was needed, which is how it fired live in PR #605: a green --base/--head run
that was vacuous on the body-diff dimension, in the one PR that hand-resolved a merge
conflict inside the corpus the guard exists to police.
Now read as an affirmative `Decisions-Edit:` git trailer. Git parses trailers only in
the final paragraph, so a quoted example message cannot arm it -- which matters because
this commit and its decision record both quote one. A non-affirmative value (`no`) does
not arm it either; the retired substring arms nothing and gets a ::warning:: nudge.
Tests: negative controls (prose mention, quoted mid-body trailer, `no` value, retired
substring) plus positive controls (trailer, uppercase, alongside Co-Authored-By). All
four negative controls verified red against the old matcher.
fixes#609
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1. `date.fromisoformat` is not a YYYY-MM-DD validator. On Python >= 3.11 it also
accepts ISO basic format ("20270101") and week dates ("2027-W01-1"), so a
malformed-looking value passed the blocking check — and which forms parse
depends on the interpreter, meaning the same corpus could validate differently
on a dev machine and on the runner (pr-checks.yml pins only python-version
'3.x'). Knock-on: the catalog's Review-due section sorts on the raw STRING, so
an accepted "20270101" sorted AFTER "2027-01-15" ('-' < '0'), contradicting the
section's own "sorted soonest-first" text. Gate on ^\d{4}-\d{2}-\d{2}$ first,
which fixes both — a fixed-width zero-padded form makes string sort == date sort.
2. A present-but-empty `stale-after:` was collapsed to None by `or None` in the
parser and then skipped by a truthiness guard in the validator, so it passed as
"absent" — a field that silently never fires, which is the exact failure mode
the blocking check exists to prevent. Keep "" distinct from None and test with
`is not None`.
3. `test_catalog_is_date_independent` was partly vacuous: with no date in either
render, both sides were trivially equal after the .replace(). It did still catch
an injected clock-derived marker, but it passed with the feature deleted. Assert
the dates are present.
4. The malformed-date check ran only over the active set, exempting archive
records. Staleness is moot there, but a typo is still a typo — check both wings.
Adds regression tests for each, plus a Review-due row for a topic-file record
(pinning the `../decisions.md` vs bare-filename link forms).
Evaluated the Open Knowledge Format (GoogleCloudPlatform/knowledge-catalog okf
v0.2, scaccogatto/okf-skills) as a replacement for our decision-record system and
rejected it: its conformance rules are deliberately permissive exactly where ours
are strict (broken links, unknown types and missing fields must all be tolerated;
`deprecated` points at no successor), and its stable identity is the file path,
which the breadcrumb rule tells agents not to trust.
Adopted two of its optional families instead, additively:
- `stale-after: YYYY-MM-DD` on the metadata line — marks a record asserting an
outside-world fact as due for re-confirmation. Absolute date, no TTL.
- `**Sources:**` in the metadata block — the evidence a record rests on, as
distinct from `Signals:` (recall keywords).
Neither is required; absence is never an error. A malformed `stale-after` is
blocking (it would silently never fire), but a past-due record is only a
non-blocking `::notice::` — going stale is the passage of time, not a defect in
whatever commit is under test. The catalog's new "Review due" section renders the
date only and never a clock-derived verdict, so it cannot drift `--check` red on a
calendar boundary with no commit touching the corpus.
No backfill: no existing record adopts either field here.
fixes#603
The decisions-guard validator enforced lifecycle metadata (key/status/since/
supersedes/superseded-by + reciprocal links) but not the **Signals:** line —
which is exactly what MemPalace's keyword recall matches on. A record without
it ingests with weak recall metadata and produces confident false-negatives
for the "MemPalace to find, file to confirm" retrieval workflow.
Add "signals" to REQUIRED_META so a migrated record with a missing or empty
**Signals:** line fails the same way a missing key does. All 114 active + 2
archive records already carry a Signals line, so this is non-breaking on the
current corpus. Archive records are intentionally out of scope (recall targets
the active corpus).
- scripts/decisions_validate.py: signals in REQUIRED_META (+ rationale comment)
- scripts/tests/test_decisions_validate.py: _rec() default + missing/empty/present cases
- docs/decisions.md: header + Enforcement note the requirement and why
fixes#545
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Exclude docs/decisions/retrieval-eval.md from active decision parsing
(_NON_DECISION_FILES); its `## N.` eval-question headings were being
miscounted as 7 legacy-unmigrated records.
- Add decisions_lib.metadata_line_count() + a decisions_validate guard
that fails a record with more than one `key:` metadata line, so a
stacked-metadata-block migration bug (which the parser silently
tolerated by reading only the first block) can't recur unnoticed.
TDD: test_duplicate_metadata_block_fails / test_single_metadata_block_passes.
- De-duplicate the 6 docs/decisions.md records left with two stacked
metadata blocks (scan.getoraddfolder-db-lookup #488,
scan.musicvideo-reconciliation #494, scan.jellyfin-mixed-content-library
#489, iptv.logo-drives-bug-preset #67, ffmpeg.qsv-decode-encode-split
#498, ci.small-lane-git-only server-management#639), merging the union
of Signals/paths/issues/Mechanics from both blocks and keeping the
richer Rule wording; rationale prose untouched.
- Fill in the deferred Q6b row in docs/decisions/retrieval-eval.md now
that startup.parallel-orientation is active in docs/decisions.md,
scoring it as a real active-vs-superseded question against the
archived docs.queue-state-gitea-tracker.
- Regenerate docs/decisions/README.md via build_decisions_catalog.py.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_anchor() mapped each space/hyphen to its own '-' without collapsing runs,
so the standard heading separator " — " (space, em-dash, space) produced a
double hyphen in every generated anchor. Since nearly every real decision
record heading uses that separator, this made the catalog emit a dead link
for essentially every row. Fix: after building the char list, collapse
consecutive '-' into one and strip leading/trailing '-' via re.sub, matching
how Goldmark/GitHub/Gitea sluggers behave.
Also fixed main() writing an extra trailing newline (want already ends in
"\n", then "+ \n" appended a second one) so docs/decisions/README.md now
ends with exactly one trailing newline; --check still compares via .strip().
Added test_anchor_collapses_em_dash_and_keeps_underscore to pin the anchor
behavior against the reported iptv.base_url case.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds scripts/build_decisions_catalog.py, which renders docs/decisions/README.md
as a compact table of only 'active' decision records (sorted by key), and its
test scripts/tests/test_build_catalog.py. Supports --check for CI drift
detection. No decision records are migrated yet, so the generated catalog is
currently empty (banner + header only) — expected at this stage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds scripts/decisions_validate.py: lifecycle invariant checks (unique
active key, key format, reciprocal supersession, removed-without-archive,
rationale-rewrite-without-token, catalog staleness, corpus budget) plus
git-diff helpers for merge-base-based CI checks. Deviates from the task
brief in one spot: REQUIRED_META narrowed to (key, status) — the brief's
(key, status, since, supersedes, superseded_by) makes its own
test_clean_corpus_passes fail, since since/supersedes/superseded_by
default to None on bare Record() instances built without going through
the markdown parser.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds scripts/decisions_lib.py, the shared parser for ErsatzTV decision
records (docs/decisions.md + docs/decisions/*.md). Parses H2 sections
into Record dataclasses, distinguishing migrated records (visible
metadata block: key/status/since/supersedes/superseded-by + Rule/
Signals/Mechanics) from legacy-unmigrated ones with no metadata line.
scripts/ is now an importable package (scripts/__init__.py,
scripts/tests/__init__.py) so later tools can `import scripts.decisions_lib`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
timothy
