Compare commits
6
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
1ef581403c | ||
|
|
990d32f31a | ||
|
|
851dca2596 | ||
|
|
aa4a8fb849 | ||
|
|
243bec708d | ||
|
|
4596603020 |
@@ -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.
|
||||
|
||||
@@ -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) |
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user