Compare commits

...
Author SHA1 Message Date
timothy 1ef581403c fix(609): close round-5 test gaps and a prose misattribution
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 11s
PR Gates / Docs update reminder (pull_request) Successful in 14s
PR Gates / decisions lifecycle (pull_request) Successful in 19s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 1m23s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 29s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 6m20s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 21m10s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 21m57s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Fable's round-5 review ran the 16 tmp_path tests that no prior round could
execute (16/16 pass) and mutation-tested every fix. Six of seven reverts were
killed; one survived, which is finding 1.

1. The exact-arity refusal in `_token_armed` had ZERO coverage -- deleting it
   passed all 55 tests, because nothing fed malformed git-log output to that
   function. Now pinned by a test that stubs `_run` with 2-field and 4-field
   output and asserts refusal, plus a 3-field control proving the refusal is
   about arity rather than the token. Verified the new test kills the mutation.

2. `test_integration_separator_in_subject_cannot_inject` did not actually pin
   the NUL framing: with `\x1f` framing restored and the arity check kept, it
   still passed, because one or two injected separators break arity and get
   absorbed. Added a case with THREE separators, which restores a multiple-of-3
   arity and would false-arm under that revert -- so it pins the framing itself.

3. The decision record attributed the "old git echoes the trailers atom" case to
   the arity check. Wrong: an echoed atom is one well-formed field, so arity
   cannot catch it -- that case is handled by the `git --version` capability
   probe. Corrected in the record.

Not changed: review also noted subject matching is now case-insensitive, so
`[DECISIONS-EDIT]` arms where the old substring check was case-sensitive.
Deliberate and harmless -- arming still requires typing the token.
2026-07-25 18:25:26 +02:00
timothy 990d32f31a fix(609): anchor and bound the git version probe
Both round-4 findings, both in the version regex introduced in round 3. Both
reproduced against the old code and confirmed closed.

1. The regex was UNANCHORED, so the first dotted number anywhere in the output
   won. `wrapper 2026.1; git version 2.20.1` read as 2026.1 -> True, enabling
   trailer parsing on a git that cannot expand the atom, whose verbatim echo then
   reads as a non-empty trailer and FALSELY ARMS. Now anchored to the canonical
   `git version X.Y` prefix.

2. Digits were unbounded, so a pathological version string raised ValueError
   instead of returning the documented safe False -- Python refuses int()
   conversion of a literal over 4300 digits. Digits are now bounded to 5 each,
   plus a try/except that the bounded regex should make unreachable.

Adds seven probe cases: the wrapper-prefix and multiline-shim strings, Apple git,
an rc suffix, a three-digit major, and the 5000-digit pathological input.
2026-07-25 18:25:26 +02:00
timothy 851dca2596 fix(609): close round-3 review findings
All four LOW; no HIGH remained. The subject_of fix from round 2 was confirmed
correct across every message shape and all 38 historical commits.

1. The old-git compat check was a VALUE sentinel: it blanked any trailer whose
   value happened to equal the atom string, so a legitimate
   `Decisions-Edit: %(trailers:key=Decisions-Edit,valueonly)` was silently
   discarded. Replaced with a capability probe on `git --version` (>= 2.22).
   Detecting by version instead of by sniffing output removes the collision
   class entirely rather than narrowing it. Unknown/unparseable version resolves
   to False -- trailers ignored, subject-only matching -- which is the safe
   direction: a trailer-only token not arming is an annoyance, whereas reading an
   unexpanded atom as a value would falsely arm and disable the guard.

2. The compat test never called `_token_armed`, so it pinned nothing -- deleting
   the guard would have left it green. Replaced with three tests that drive the
   real function through a stubbed `_run`, covering old git (trailers ignored),
   modern git (trailer arms), a tokened subject surviving an unusable trailer,
   and version-string parsing incl. unparseable input. Proven non-vacuous:
   forcing the probe True makes the old-git test fail.

3. `_repo()` still ignored return codes from init/config/base-commit and never
   checked that the base sha resolved, so a rejected base could leave it
   returning ("", <root sha>) and negative range tests would pass vacuously. All
   commands are now checked and the base sha is asserted to be a full 40 chars.

4. docs/decisions.md line 65 still said "append it, as every prior use does".
   37 of 38 append; docs(434) is mid-subject.
2026-07-25 18:25:26 +02:00
timothy aa4a8fb849 fix(609): close round-2 review findings
HIGH -- `subject_of` used `lstrip("\n")`, so it returned the first NON-EMPTY
line. `git commit --cleanup=verbatim` accepts a message that begins with a blank
line and `%B` returns it raw, so body prose on line 2 was promoted to "subject"
and armed the token. Now literally line 1: an empty first line yields "", which
arms nothing -- failing toward the guard running.

LOW -- the old-git compat guard was a PREFIX match (`startswith("%(trailers")`)
that also `continue`d before the subject was evaluated. So a legitimate trailer
value beginning with that text was discarded, and worse, a perfectly good tokened
SUBJECT was thrown away because of its trailer field. Now an exact match against
the full atom, neutralising only the trailer and leaving the subject honoured.

LOW -- docstrings still said every historical use "appends" the token. Of the 38
uses, 37 append and `docs(434)` is mid-subject.

LOW (plausible) -- the `_repo` test helper ignored every git return code, so a
rejected commit would leave HEAD at base and every negative assertion would pass
vacuously. Return codes are now checked and HEAD is asserted to have moved.

Adds a regression test for the verbatim leading-blank-line case and one pinning
the compat guard to an exact atom match.
2026-07-25 18:25:26 +02:00
timothy 243bec708d fix(609): close two false-arm holes found in cross-family review
Codex review of the first attempt found both, and both were in the git plumbing
that my unit tests never touched -- they only exercised the pure predicate.

1. HIGH: git's `%s` is the first PARAGRAPH, not the first line. It joins
   consecutive non-blank lines with spaces, so
     `fix: harmless subject`
     `This explains [decisions-edit] on line two.`
   came back as ONE line containing the token and armed it -- the exact
   false-arm this change exists to prevent. The first line is now taken from
   `%B` via `subject_of()`.

2. HIGH: the in-band `\x1f`/`\x1e` field separators were injectable. A subject
   containing a literal `\x1f` was split at the wrong place and its tail read as
   a trailer, arming the token. Framing is now NUL, which git forbids inside a
   commit message and which therefore cannot be injected, with exact-arity
   parsing (fields must be a multiple of three) that refuses to arm otherwise.

Also from the same review:
- Refuse to arm on a `%(trailers:...)` atom echoed literally by a git older than
  2.22, which would otherwise read as a non-empty trailer (exit 0, so `_run`
  returns it rather than None).
- Record corrected: 38 subject-tokened commits in ancestry, not "twenty"; and it
  no longer claims a blanket fail-safe -- `_token_armed` failing is safe, but the
  surrounding `_diff_findings` fails open earlier on an unresolvable merge-base,
  skipping every check. That predates this change.

Adds 8 integration tests that drive `_token_armed` against a real throwaway git
repo -- the gap that let both defects pass. Verified non-vacuous by
reconstructing the old implementation in memory: it arms on both inputs, the new
one does not.

Note `--format` uses git's `%x00` escape, not a literal NUL: a NUL in argv raises
ValueError from subprocess, which broke every diff-engine test until fixed.
2026-07-25 18:25:26 +02:00
timothy 4596603020 fix(609): scope the decisions edit token to the subject line or a trailer
The token was armed by a bare substring match over every commit message in the
range, so a commit that merely DESCRIBED the mechanism armed it and skipped the
entire `if not token:` block -- all three rationale-rewrite comparisons (active
survivors, active->archive laundering, archive survivors). `removed` and `demoted`
still ran, so the job printed `decisions-validate: OK` while doing nothing. It
bit in PR#605, which had hand-resolved an append-vs-append conflict inside
docs/decisions.md -- precisely the operation the guard exists to police.

Now recognized in exactly two places:
  * the commit SUBJECT line -- the established form. All twenty prior tokened
    commits append it to the subject (or place it mid-subject, as docs(434)
    does); none put it on its own line, so the obvious "own-line only" rule
    would have broken every historical use.
  * a `Decisions-Edit: <reason>` git trailer -- the forward-looking form, which
    can carry a reason the bracketed marker cannot.

Fail-open posture unchanged: unresolvable git means the token reads unarmed, so
the guard still runs.

Verified by measuring the guard rather than reading a green check -- a positive
control over the real corpus across all three placements: no token fires (exit 1),
subject token suppresses (exit 0), body-only mention fires (exit 1). Plus an
end-to-end matcher test against a throwaway git repo covering the established
form, mid-subject placement, the trailer, a merge commit quoting a tokened PR
title, a multi-commit range, and an unresolvable ref.

fixes #609
2026-07-25 18:25:26 +02:00
4 changed files with 414 additions and 4 deletions
+51
View File
@@ -61,6 +61,15 @@ lifecycle writes (adding a new active record, relocating a superseded/retired re
updating metadata fields, regenerating the catalog) are token-free; the validator proves they're
legitimate structurally instead of gating on the token.
**Where the token counts (ersatztv#609).** It is recognized in a commit's **subject line** (the
established form — all 38 prior uses carry it there, 37 appended) or as a
**`Decisions-Edit: <reason>`** git trailer. It is *not* recognized anywhere else in the message
body, so you can safely write *about*
the mechanism in a commit body without arming it. That scoping is the fix for a real failure: the
check used to be a bare substring over the whole message, so a commit merely describing the token
armed it and silently suppressed every rationale-rewrite comparison — the job still printed
`decisions-validate: OK` while doing nothing.
---
## Index
@@ -3918,3 +3927,45 @@ owner's sweep **destroyed** the row the other library still served. This is the
rather than *destroys* that shared row. The unrecoverable data loss is gone; the shared-row limitation is a
whole-app property, not a music-video one, and is tracked separately as **#606**. #496's Done-when was
amended to this parity wording rather than ticked as literally written.
## 2026-07-25 — The decisions edit token is recognized only in the subject line or a trailer, never in body prose (#609)
`key: docs.decision-edit-token-scope` · `status: active` · `since: 2026-07-25` · `supersedes: none` · `superseded-by: none`
**Rule:** `scripts/decisions_validate.py` arms `[decisions-edit]` only from a commit's **subject line** or a **`Decisions-Edit:` git trailer** — never from anywhere else in the message body, so a commit that merely *describes* the token cannot silently disable the rationale-rewrite guard.
**Signals:** decisions-edit token, edit token armed, bare substring match, guard no-op, green but vacuous, body-diff suppressed, subject line, Decisions-Edit trailer, token_armed_in · paths: scripts/decisions_validate.py · issues: #609, #603, #605
**Mechanics:** `scripts/decisions_validate.py``token_armed_in` / `_token_armed`; docs/decisions.md → header "Where the token counts"
**Sources:** PR#605, where a commit message explaining why no token was needed armed it and suppressed all three rationale comparisons; caught in adversarial review, not by CI
**The failure was a gate reading green while doing nothing.** The old check was
`EDIT_TOKEN.lower() in git log --format=%B mb..head`, a bare substring over every commit message in
the range. Any mention armed it — including prose *about* the mechanism — and arming it skips the
entire `if not token:` block: all three rationale comparisons (active survivors, active→archive
laundering, archive survivors). `removed` and `demoted` still ran, so the job printed
`decisions-validate: OK` and looked healthy. It bit in the PR that introduced `stale-after` (#603),
and it bit in the worst possible place: that PR had hand-resolved an append-vs-append conflict inside
`docs/decisions.md`, which is precisely the operation the guard exists to police.
**Scoping to the subject preserves every historical use.** All 38 prior tokened commits in `main`'s
ancestry carry `[decisions-edit]` in the subject — usually appended, sometimes mid-subject as
`docs(434):` does; none put it on its own line, so an "own-line only" rule — the obvious first
instinct — would have broken all of them. The `Decisions-Edit:` trailer is added alongside as the
forward-looking form because it can carry a *reason*, which the bracketed marker cannot.
**The subject is computed, not taken from git's `%s`.** That atom is the first *paragraph*: it joins
consecutive non-blank lines with spaces, so `fix: harmless` followed by a second line mentioning the
token would arm it. The first line is taken from `%B` instead. Fields are framed with **NUL**, which
git forbids inside a commit message and is therefore the one separator a message cannot inject — an
in-band `\x1f` let a subject containing that byte be split at the wrong place and its tail read as a
trailer. Parsing demands exact arity (a multiple of three fields) and refuses to arm on anything
else — defence in depth against truncated output. A git too old to expand `%(trailers:…)` is a
*separate* concern handled by a capability probe on `git --version` (>= 2.22), not by the arity
check: an echoed atom is still one well-formed field, so arity alone would not catch it.
**Unarmed is the safe default for this function**: if git can't be interrogated the token reads
unarmed, so the rationale comparisons still run. Note the *surrounding* `_diff_findings` fails open
separately and earlier — an unresolvable merge-base returns no findings at all, skipping every diff
check including this one. That predates this record and is unchanged by it.
**The general lesson is the one already recorded for CI behaviour**: a check that can silently
no-op is worse than no check, because its green reads as coverage. Verify a guard by *measuring that
it fires* — a positive control that mutates a record and expects a red — not by observing that the
job passed.
+1
View File
@@ -61,6 +61,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
| `concurrency.replace-all-contract` | Replace-all aggregate PUTs carry a uniform plain `int Version` concurrency token (EF `.IsConcurrencyToken()`), checked pre-save and enforced by the EF UPDATE guard, returning 412 (not 409) on a stale `If-Match`. | 2026-07-11 | [link](optimistic-concurrency.md#2026-07-11--optimistic-concurrency-contract-for-replace-all-puts-253-pr1-infra--block-reference) |
| `concurrency.schedule-item-child-identity` | `PUT /api/schedules/{id}/items` reconciles by an optional round-tripped child `Id` (null/absent/0 ⇒ new item), never by array position, so fill-group/shuffle state follows the logical item across reorders; an unknown or duplicate id is rejected 422 (checked after the §7a `CheckVersion`, so 412 precedes 422). | 2026-07-11 | [link](optimistic-concurrency.md#2026-07-11--stable-child-identity-for-schedule-item-replace-259-split-from-252253) |
| `docs.convention-docs-session-start` | Docs-first, not source-first: conventions (api-conventions, spa-conventions, e2e-local, blazor-route-parity, domain-model, decisions, README) are read from docs, not reverse-engineered from code, via `docs/README.md`'s task-signal map — only the sections it points to for the task at hand, not the whole set. Each doc is updated in the same PR that changes what it documents, replacing deferred/follow-up doc updates. | 2026-07-07 | [link](../decisions.md#2026-07-07--convention-docs-read-at-session-start-updated-in-pr) |
| `docs.decision-edit-token-scope` | `scripts/decisions_validate.py` arms `[decisions-edit]` only from a commit's **subject line** or a **`Decisions-Edit:` git trailer** — never from anywhere else in the message body, so a commit that merely *describes* the token cannot silently disable the rationale-rewrite guard. | 2026-07-25 | [link](../decisions.md#2026-07-25--the-decisions-edit-token-is-recognized-only-in-the-subject-line-or-a-trailer-never-in-body-prose-609) |
| `docs.decision-lifecycle` | every decision `##` record (active or archived) carries a 5-field metadata block (`key`, `status`, `since`, `supersedes`, `superseded-by`) checked by `scripts/decisions_validate.py`; a record is never deleted or line-edited to reverse a call — it is moved to `docs/decisions/archive/` with `status: superseded`/`retired` and a reciprocal `superseded-by`/`supersedes` key pair to its replacement. | 2026-07-21 | [link](../decisions.md#2026-07-21--decision-records-carry-a-lifecycle-schema-validated-by-a-script-append-only-by-diff-is-retired-521) |
| `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](../decisions.md#2026-07-25--okf-evaluated-and-rejected-as-a-replacement-two-of-its-optional-fields-adopted-603) |
| `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](../decisions.md#2026-07-21--check-the-worked-issue-before-the-decision-corpus-a-closed-trackers-comments-need-no-retrofit-524) |
+103 -4
View File
@@ -231,6 +231,108 @@ def _archive_records_at(ref: str):
return by_heading
_TRAILER_KEY = "Decisions-Edit"
# NUL framing, not \x1f/\x1e. git forbids NUL inside a commit message, so it is the ONLY separator
# a crafted (or merely odd) message cannot inject: a subject containing a literal \x1f would
# otherwise be split at the wrong place and its tail read as a trailer — falsely ARMING the token,
# which is the dangerous direction because arming silently disables the guard.
_NUL = "\x00" # what we SPLIT the output on
_NUL_FMT = "%x00" # what goes in the --format string; a literal NUL in argv raises ValueError
_FIELDS_PER_COMMIT = 3
# git < 2.22 cannot expand %(trailers:key=…,valueonly) and echoes the atom verbatim with exit 0.
# Detect that by VERSION, not by sniffing the output for the atom text: a value-based sentinel can
# collide with a legitimate trailer whose value happens to be that exact string, silently discarding
# a real token (#609 round 3).
_MIN_TRAILER_GIT = (2, 22)
# ANCHORED to the canonical `git version X.Y` prefix, and digits are BOUNDED. Unanchored, the first
# dotted number anywhere wins — `wrapper 2026.1; git version 2.20.1` would read as 2026.1 and enable
# trailers on a git that cannot expand them, which is the falsely-arming direction. Unbounded digits
# are also unsafe: Python raises ValueError converting an int literal over 4300 digits.
_GIT_VERSION_RE = re.compile(r"^git version (\d{1,5})\.(\d{1,5})")
def token_armed_in(subject: str, trailer: str) -> bool:
"""Whether ONE commit legitimately carries the edit token.
Recognized in exactly two places:
* the commit SUBJECT (first line) — the established form; all 38 historical uses carry the
token in the subject (37 appended, `docs(434)` mid-subject), so this stays compatible.
* a `Decisions-Edit:` git trailer — the forward-looking form, which can carry a reason.
Deliberately NOT the rest of the body. The old check was a bare substring over the whole
message, so a commit that merely *described* the token armed it and silently suppressed every
rationale-rewrite comparison for the PR — a gate reporting green while doing nothing, and it
bit hardest in a PR that hand-resolved a conflict inside the corpus the guard protects (#609).
"""
if EDIT_TOKEN.lower() in subject.lower():
return True
return bool(trailer.strip())
def subject_of(body: str) -> str:
"""The commit's true first line — literally line 1, even when line 1 is blank.
NOT git's `%s`: that atom is the first *paragraph*, joining consecutive non-blank lines with
spaces, so `fix: harmless\\nprose about [decisions-edit]\\n\\n…` would come back as one line
containing the token and arm it.
And NOT "the first non-blank line": `git commit --cleanup=verbatim` accepts a message beginning
with a blank line, and `%B` returns it raw. Skipping leading blanks would promote body prose to
subject and arm on it. An empty first line yields "", which arms nothing — the safe direction.
"""
return body.split("\n", 1)[0].strip()
def _git_supports_trailer_atom() -> bool:
"""Whether this git expands `%(trailers:key=…,valueonly)` (2.22+). Unknown → False.
Unknown resolving to False means trailers are ignored and matching falls back to the subject,
which is the safe direction: a genuine trailer-only token silently not arming is an annoyance,
whereas treating an unexpanded atom as a value would falsely arm and disable the guard.
"""
out = _run(["git", "--version"])
if not out:
return False
m = _GIT_VERSION_RE.match(out.strip())
if not m:
return False
try:
return (int(m.group(1)), int(m.group(2))) >= _MIN_TRAILER_GIT
except ValueError: # belt and braces; the bounded regex should make this unreachable
return False
def _token_armed(mb: str, head: str) -> bool:
"""True if ANY commit in mb..head carries the token. Anything unexpected → False (guard runs)."""
out = _run(
[
"git",
"log",
f"--format=%H{_NUL_FMT}%B{_NUL_FMT}%(trailers:key={_TRAILER_KEY},valueonly){_NUL_FMT}",
f"{mb}..{head}",
]
)
if out is None:
# On git trouble the token is NOT armed, so the guard still runs. Failing the other way
# would silently disable it — the bug this whole function exists to fix.
return False
trailers_usable = _git_supports_trailer_atom()
parts = out.split(_NUL)
if parts and not parts[-1].strip():
parts.pop() # trailing inter-record newline after the final NUL
if len(parts) % _FIELDS_PER_COMMIT != 0:
# Exact arity, not best-effort. NUL cannot occur in a commit message, so a bad count means
# the output was truncated or the format atom wasn't understood — refuse to arm.
return False
for i in range(0, len(parts), _FIELDS_PER_COMMIT):
body, trailer = parts[i + 1], parts[i + 2]
# On a git too old to expand the atom, ignore the trailer field entirely and fall back to
# subject-only matching. The SUBJECT is always honoured either way.
if token_armed_in(subject_of(body), trailer if trailers_usable else ""):
return True
return False
def _rationale(rec) -> str:
"""Record body with the contiguous top metadata block stripped, whitespace-normalized.
@@ -261,10 +363,7 @@ def _diff_findings(base: str, head: str) -> tuple[list[str], list[str], list[str
file=sys.stderr,
)
return [], [], []
if EDIT_TOKEN.lower() in (_run(["git", "log", "--format=%B", f"{mb}..{head}"]) or "").lower():
token = True
else:
token = False
token = _token_armed(mb, head)
base_active = _records_at(mb, _active_paths_at(mb))
head_active = _records_at(head, _active_paths_at(head))
+259
View File
@@ -478,3 +478,262 @@ def test_malformed_stale_after_on_an_archive_record_is_caught():
arch = _rec(key="a.old", status="superseded", superseded_by="a.b", stale_after="soon")
errs = _v([_rec(key="a.b", supersedes="a.old")], archive_records=[arch], archive_keys={"a.old"})
assert any("not a YYYY-MM-DD date" in e for e in errs)
# ---- edit-token scoping (ersatztv#609) ----
def test_token_armed_by_subject_the_established_form():
"""All 38 historical uses carry it in the subject (37 appended, one mid-subject)."""
for subj in (
"fix(460): write null LastScan on disable-sync [decisions-edit]",
"docs(434): [decisions-edit] update field-values decision",
"fix(529): address review [DECISIONS-EDIT]",
"Merge pull request 'fix(460): ... [decisions-edit]' (#601) from x into main",
):
assert dv.token_armed_in(subj, ""), subj
def test_token_armed_by_trailer():
assert dv.token_armed_in("docs: correct a stale figure", "corrected the 2026-07-17 load number")
def test_token_NOT_armed_by_body_prose():
"""The #609 defect: a commit DESCRIBING the token silently disarmed the whole guard."""
for subj in (
"docs(603): correct the record's own no-backfill claim",
"fix(609): scope the edit token to the subject line",
):
assert not dv.token_armed_in(subj, ""), subj
def test_token_not_armed_by_empty_or_whitespace_trailer():
assert not dv.token_armed_in("docs: something", "")
assert not dv.token_armed_in("docs: something", " \n ")
# ---- _token_armed INTEGRATION against real git (ersatztv#609 review round 2) ----
#
# The unit tests above only exercise `token_armed_in`. Both false-arm defects found in review
# (git's `%s` folding the first PARAGRAPH, and an injectable in-band separator) lived in the git
# plumbing and passed those tests untouched. These drive the real command.
def _repo(tmp_path, messages: list[str]) -> tuple[str, str]:
"""Build a throwaway repo; return (base_sha, head_sha)."""
import subprocess
def g(*a, **kw):
return subprocess.run(["git", *a], cwd=tmp_path, capture_output=True, text=True, **kw)
for cmd in (("init", "-q", "."), ("config", "user.email", "t@e"), ("config", "user.name", "t")):
assert g(*cmd).returncode == 0, f"setup failed: {cmd}"
(tmp_path / "f").write_text("base\n")
assert g("add", "-A").returncode == 0
assert g("commit", "-qm", "base").returncode == 0, "base commit rejected"
base = g("rev-parse", "HEAD").stdout.strip()
assert len(base) == 40, f"base sha not resolved ({base!r}) — range tests would be vacuous"
for i, msg in enumerate(messages):
(tmp_path / "f").write_text(f"{i}\n")
assert g("add", "-A").returncode == 0
r = g("commit", "-q", "-F", "-", input=msg)
assert r.returncode == 0, f"commit rejected, test would pass vacuously: {r.stderr}"
head = g("rev-parse", "HEAD").stdout.strip()
assert head != base, "no commit landed — every assertion below would be vacuous"
return base, head
def _armed(tmp_path, message: str) -> bool:
import os
base, head = _repo(tmp_path, [message])
cwd = os.getcwd()
os.chdir(tmp_path)
try:
return dv._token_armed(base, head)
finally:
os.chdir(cwd)
def test_integration_subject_token_arms(tmp_path):
assert _armed(tmp_path, "fix(460): write null LastScan [decisions-edit]\n\nbody\n")
def test_integration_trailer_arms(tmp_path):
assert _armed(tmp_path, "docs: correct a figure\n\nwhy\n\nDecisions-Edit: the load number was wrong\n")
def test_integration_body_prose_does_not_arm(tmp_path):
assert not _armed(tmp_path, "docs: explain it\n\nThe check matches [decisions-edit] as a substring.\n")
def test_integration_second_line_of_first_paragraph_does_not_arm(tmp_path):
"""git's %s is the first PARAGRAPH, not the first line — it folds line 2 in with a space.
Using %s directly, this message arms the token. It must not: line 2 is body prose.
"""
assert not _armed(tmp_path, "fix: harmless subject\nThis explains [decisions-edit] on line two.\n\nbody\n")
def test_integration_separator_in_subject_cannot_inject(tmp_path):
"""An in-band \\x1f separator was injectable; NUL cannot appear in a commit message."""
assert not _armed(tmp_path, "docs: harmless\x1fsuffix\n\nbody\n")
assert not _armed(tmp_path, "docs: harmless\x1e\x1fsuffix\n\nbody\n")
def test_integration_empty_trailer_does_not_arm(tmp_path):
assert not _armed(tmp_path, "docs: something\n\nbody\n\nDecisions-Edit:\n")
def test_integration_only_a_later_commit_carries_it(tmp_path):
import os
base, head = _repo(tmp_path, ["chore: unrelated\n", "fix: real correction [decisions-edit]\n"])
cwd = os.getcwd()
os.chdir(tmp_path)
try:
assert dv._token_armed(base, head)
finally:
os.chdir(cwd)
def test_integration_unresolvable_refs_do_not_arm(tmp_path):
"""Guard must still RUN when git can't answer; arming on failure would disable it."""
assert not _armed(tmp_path, "docs: x\n") or True # build a repo first
import os
cwd = os.getcwd()
os.chdir(tmp_path)
try:
assert dv._token_armed("nope1", "nope2") is False
finally:
os.chdir(cwd)
def test_integration_leading_blank_line_does_not_promote_body_to_subject(tmp_path):
"""`--cleanup=verbatim` allows a message starting blank; %B returns it raw.
Skipping leading blanks would promote body prose to "subject" and arm on it.
"""
import subprocess
d = tmp_path
subprocess.run(["git", "init", "-q", "."], cwd=d)
subprocess.run(["git", "config", "user.email", "t@e"], cwd=d)
subprocess.run(["git", "config", "user.name", "t"], cwd=d)
(d / "f").write_text("base\n")
subprocess.run(["git", "add", "-A"], cwd=d)
subprocess.run(["git", "commit", "-qm", "base"], cwd=d)
base = subprocess.run(["git", "rev-parse", "HEAD"], cwd=d, capture_output=True, text=True).stdout.strip()
(d / "f").write_text("x\n")
subprocess.run(["git", "add", "-A"], cwd=d)
msg = "\nThis body prose mentions [decisions-edit] and must not arm.\n"
r = subprocess.run(
["git", "commit", "-q", "--cleanup=verbatim", "-F", "-"], cwd=d, input=msg, text=True, capture_output=True
)
assert r.returncode == 0, f"commit rejected, test would be vacuous: {r.stderr}"
head = subprocess.run(["git", "rev-parse", "HEAD"], cwd=d, capture_output=True, text=True).stdout.strip()
assert head != base
import os
cwd = os.getcwd()
os.chdir(d)
try:
assert dv.subject_of(msg) == ""
assert not dv._token_armed(base, head)
finally:
os.chdir(cwd)
def test_old_git_falls_back_to_subject_only(monkeypatch):
"""Exercises the real `_token_armed` branch, not just the predicate.
Round-3 review caught that the previous version of this test never called `_token_armed`, so
deleting the guard entirely would have left it green.
"""
calls = {}
def fake_run(args):
if args[:2] == ["git", "--version"]:
return calls["version"]
# one commit: sha, body, trailer — the trailer field is the UNEXPANDED atom, as an old
# git would emit it verbatim.
return "sha\x00docs: plain subject\n\x00%(trailers:key=Decisions-Edit,valueonly)\x00"
monkeypatch.setattr(dv, "_run", fake_run)
calls["version"] = "git version 2.20.1\n" # too old: trailers ignored, subject-only
assert not dv._token_armed("a", "b"), "unexpanded atom must not be read as a trailer value"
calls["version"] = "git version 2.39.5\n" # new enough: the field is a real value
assert dv._token_armed("a", "b"), "a modern git's trailer value must still arm"
def test_old_git_still_honours_a_tokened_subject(monkeypatch):
def fake_run(args):
if args[:2] == ["git", "--version"]:
return "git version 2.20.1\n"
return "sha\x00fix: real [decisions-edit]\n\x00%(trailers:key=Decisions-Edit,valueonly)\x00"
monkeypatch.setattr(dv, "_run", fake_run)
assert dv._token_armed("a", "b"), "subject must arm even when trailers are unusable"
def test_git_version_probe_parses_and_fails_safe(monkeypatch):
for out, want in [
("git version 2.39.5\n", True),
("git version 2.22.0\n", True),
("git version 2.21.9\n", False),
("git version 3.0.0\n", True),
("", False),
(None, False),
("not a version string", False),
# round 4: unanchored matching let a wrapper's own version win, enabling trailers on a git
# that cannot expand them — the falsely-arming direction.
("wrapper 2026.1; git version 2.20.1", False),
("some-shim 9.9\ngit version 2.39.5", False),
("git version 2.39.5 (Apple Git-154)", True),
("git version 2.40.0.rc1", True),
("git version 123.4.5", True),
# round 4: unbounded digits raised ValueError instead of returning the documented False
("git version " + "9" * 5000 + ".1", False),
]:
monkeypatch.setattr(dv, "_run", lambda _a, _o=out: _o)
assert dv._git_supports_trailer_atom() is want, out
def test_malformed_arity_refuses_to_arm(monkeypatch):
"""Round-5 mutation testing: deleting the arity check passed the whole suite.
Reachable only via truncated `git log` output (NUL cannot appear in a commit message), and the
failure direction is safe — refuse to arm, so the guard still runs. Pinned so a refactor can't
drop it silently.
"""
def stub(out):
def fake_run(args):
return "git version 2.39.5\n" if args[:2] == ["git", "--version"] else out
return fake_run
# 2 fields instead of 3 (truncated mid-record), with the token present in the subject:
monkeypatch.setattr(dv, "_run", stub("sha\x00fix: real [decisions-edit]\n\x00"))
assert not dv._token_armed("a", "b"), "malformed arity must refuse to arm"
# 4 fields — an extra separator
monkeypatch.setattr(dv, "_run", stub("sha\x00fix: real [decisions-edit]\n\x00\x00extra\x00"))
assert not dv._token_armed("a", "b"), "malformed arity must refuse to arm"
# exactly 3 → the same token DOES arm, proving the refusal above is about arity, not the token
monkeypatch.setattr(dv, "_run", stub("sha\x00fix: real [decisions-edit]\n\x00\x00"))
assert dv._token_armed("a", "b"), "well-formed arity with a tokened subject must arm"
def test_separator_injection_needs_more_than_arity_to_be_caught(tmp_path):
"""Strengthens the injection test: THREE \\x1f's restore a multiple-of-3 arity.
Under the old in-band framing a subject with three separators would parse as well-formed and
its tail could be read as a trailer. With NUL framing the bytes are inert, so this pins the
framing itself rather than leaning on the arity check to absorb it.
"""
assert not _armed(tmp_path, "docs: a\x1fb\x1fc\x1fd\n\nbody\n")