chore(infra): tracking-file guard, LWDiD data canary, /push-pr-update range scan + changelog compiler hardening - #808
chore(infra): tracking-file guard, LWDiD data canary, /push-pr-update range scan + changelog compiler hardening#808igerber wants to merge 9 commits into
Conversation
…push-pr-update range scan Closes three TODO.md infra rows: - tests/test_tracking_files.py: CI guard for the TODO.md/DEFERRED.md split contract - no TODO table row points work at DEFERRED.md, rows cross-linking an M-xxx ledger id do not restate lifecycle state, and the three documented table shapes (TODO Actionable, DEFERRED blocker, DEFERRED Decision record) are enforced with per-row column counts. The parser handles escaped pipes, unspaced rows, and GFM no-leading-pipe tables, and fails loudly on malformed (non-rendering) tables. Wired into docs-tests.yml (path filters + step); excluded from rust-test.yml triggers. CLAUDE.md documents the Decision-record shape. - .github/workflows/lwdid-data-canary.yml: weekly cron + dispatch lane that fails loudly if load_prop99/load_walmart fall back to synthetic data (source != 'lwdid_ssc_ancillary'), then runs the LWDiD replication tests the canary de-gates. Test docstrings now point at the lane instead of the deleted TODO row. - .claude/commands/push-pr-update.md section 3b: restored the committed-range pattern scan via premerge_scan.py --range with the command's own COMPARISON_REF resolver; findings informational, scan-integrity failures (exit 3/4) stop-and-report. pre-merge-check.md splits premerge_scan and changelog_compile check into separate Bash calls so the first exit status is not masked. Also hardens the freshly merged changelog fragments compiler (#806), found during local review of this branch: - Malformed/duplicate release headers and link definitions refused on both fresh and exit-4 paths; exit-4 predecessor derived only from a canonical compare/tag link; --previous-version cross-check (bump-version.md now passes it) with interrupted-bump recovery kept; prev-anchor date validation with a documented legacy exemption. - Existing-target sections are validated with the full fragment grammar on the exit-4 path (orphan bullets / unknown-category bullets refuse). - Indented ATX headings (CommonMark accepts 1-3 leading spaces) are rejected by the fragment grammar - previously ' ## [9.9.9]' compiled into the changelog as a real H2 invisible to the header scanners. - Dirty guard catches deleted-but-committed fragments; compile rolls back changelog + fragments on a mid-flight unlink failure; strict kebab fragment-name grammar. Release note in changelog.d/20260830-infra-process-sweep.md (Internal).
PR ReviewOverall assessmentExecutive summary
MethodologyNo findings. The affected method is LWDiD validation only. Code Quality1. Indented changelog constructs bypass duplicate protection
2. Deleted-fragment enumeration fails open
3. Escaped-pipe parsing mishandles even backslash runs
PerformanceNo findings. MaintainabilityNo additional findings. Tech DebtNo findings. The removed TODO rows correspond to implemented work, and no new untracked deferral was introduced. SecurityNo findings. No secrets were observed, and workflow actions remain SHA-pinned. Documentation / TestsStale TODO reference
Test coverage is otherwise substantial. Path to Approval
|
…, backslash parity
- changelog_compile.py: refuse CommonMark-indented (1-3 space) ATX
headings and version-link definitions anywhere in CHANGELOG.md - the
duplicate/date scans are column-zero anchored, so ' ## [1.3.0]' or
' [1.3.0]: ...' rendered as real constructs while staying invisible to
every safeguard (reproduced: compile succeeded leaving two rendered
1.3.0 headings + two link definitions). Regressions on both the fresh
and exit-4 paths.
- changelog_compile.py: the deleted-fragment guard now fails CLOSED when
'git ls-tree -r HEAD' fails - previously a nonzero return silently
skipped the set-equality check and could compile an incomplete
release. Regression forces the enumeration to fail and asserts nothing
is written or consumed.
- test_tracking_files.py: pipe escaping is parity-based ('\|' literal,
'\\|' backslash + separator); the fixed-width (?<!\\) lookbehind got
even backslash runs wrong. Replaced with a backslash-run-counting
splitter (also fixes the same parity hole in the trailing-pipe trim);
parity tests for 1-4 backslash runs and trailing forms.
- lwdid-data-canary.yml: reword header comment to name the former
(now-deleted) TODO row.
|
🔁 AI review rerun (requested by @igerber) Head SHA: PR ReviewOverall assessmentExecutive summary
MethodologyNo findings. The affected surface is LWDiD validation only. Code Quality1. [Newly identified] CommonMark heading whitespace still bypasses release scans
2. Divider validation accepts non-rendering tables
PerformanceNo findings. MaintainabilityNo additional findings. Tech DebtNo findings. Removed TODO rows correspond to implemented work. SecurityNo findings. No secrets were observed; workflow actions remain SHA-pinned. Documentation / Tests3. [Newly identified]
|
…r, bump-version variable
- changelog_compile.py: the malformed-header scan now accepts ANY
CommonMark heading spacing and depth ('## [x]', a tab after the
hashes, '### [x]') instead of the single-space '## [' shape, so a
rendering-but-noncanonical release-like heading can no longer hide
from the duplicate/date safeguards; exact-canonical release and
Unreleased headers are the only exemptions. Regressions for
double-space, tab, and H3 shapes on both fresh and exit-4 paths
(real CHANGELOG audited: all 85 bracketed headings canonical).
- test_tracking_files.py: _is_divider now enforces the GFM delimiter
grammar per cell (':?-+:?') - an empty or colon-only delimiter cell
stops the whole table from rendering, so such a row no longer counts
as a divider and the well-formedness check raises. Tests for empty,
colon-only, and space delimiter cells.
- bump-version.md: OLD_VERSION is resolved inside the SAME Bash block
that passes --previous-version (variables do not persist across Bash
calls; the prose-only 'store as OLD_VERSION' step would have expanded
empty and failed every normal release compile).
|
🔁 AI review rerun (requested by @igerber) Head SHA: PR ReviewOverall assessmentExecutive summary
MethodologyNo findings. Code Quality1. [Newly identified] No-whitespace link definitions bypass duplicate checks
PerformanceNo findings. MaintainabilityNo findings. Tech DebtNo findings. The removed TODO rows correspond to implemented work. SecurityNo findings. Workflow permissions are read-only and actions remain SHA-pinned. Documentation / TestsNo findings. The release fragment follows the project convention. Path to Approval
|
changelog_compile.py: CommonMark permits '[1.3.0]:https://...' with no
whitespace after the colon (and resolves a label to its FIRST
definition), so the '\s+<dest>' link-definition scan missed such a
definition - a fresh compile could append the canonical definition while
the hidden earlier one kept controlling the rendered link, and the
exit-4 path could certify the pair. The duplicate/already-defined-target
scan now matches on the label alone ('^[X.Y.Z]:'). The predecessor-link
lookups keep their canonical '\]:\s+' form deliberately: a no-space
predecessor definition refuses as not-found (fail closed) since compiler
output always writes the space. Regressions: hidden no-space target
definition blocks the fresh compile (nothing written or consumed);
no-space duplicate blocks exit-4.
|
🔁 AI review rerun (requested by @igerber) Head SHA: PR ReviewOverall assessmentExecutive summary
MethodologyNo findings. The canary uses the SSC ancillary sources and Prop 99/Walmart targets documented in Code Quality1. [Newly identified] CommonMark container definitions bypass link validation
PerformanceNo findings. MaintainabilityNo findings. Tech DebtNo findings. The deleted TODO rows correspond to implemented work. SecurityNo separate findings. Actions are SHA-pinned and workflow permissions are read-only. Documentation / TestsNo separate findings. The required Path to Approval
|
Third same-class CommonMark-smuggling finding (indented defs, no-space
colons, now container/whitespace-normalized labels) - stop chasing
shapes and enforce the invariant directly: a version-label-plus-colon
('[X.Y.Z]:', any label spacing, any container) may exist ONLY as a
canonical column-zero definition, because CommonMark registers
definitions inside list items/blockquotes, whitespace-normalizes labels
('[ 1.3.0 ]' resolves as '1.3.0'), and resolves each label to its FIRST
definition - any smuggled form outranks what the compiler writes.
_noncanonical_version_link() enforces this in three places:
- the pre-branch CHANGELOG scan (covers fresh and exit-4 paths),
- the fragment grammar (a '- [1.3.0]:url' bullet refuses at check time),
- a backstop on the fully assembled text before any write (multi-line
labels can span the line-based fragment check).
Real CHANGELOG and changelog.d audited: no non-canonical shapes exist.
Regressions: whitespace-normalized label, list-item and nested-list
definitions on both compile paths; three fragment-body forms.
|
🔁 AI review rerun (requested by @igerber) Head SHA: PR ReviewOverall assessmentExecutive summary
MethodologyNo findings. The canary checks the Prop 99 and Walmart sources documented in Code Quality1. [Newly identified] Markdown code blocks can masquerade as canonical link definitions
PerformanceNo findings. MaintainabilityNo findings. Tech DebtNo findings. The removed TODO rows correspond to implemented work. SecurityNo findings. Workflow permissions are read-only and actions are SHA-pinned. Documentation / TestsNo separate findings. The required Path to Approval
|
Inverse of round 4's hazard: a column-zero '[1.2.0]: url' inside a fenced code or raw-HTML block LOOKS canonical to the line-based scans but does not render as a definition - the predecessor-anchor search could then insert the new comparison link inside that block, and exit-4 could certify a release whose header link never resolves. Rather than teach the scans Markdown block context, enforce the terminal invariant the file already satisfies (all 84 real definitions form one contiguous run at EOF): _link_block_violation() refuses any canonical-looking definition outside the terminal contiguous link block, where no fence/HTML context can exist without breaking the run. Runs on the pre-branch text (fresh + exit-4) and as a backstop on the assembled text before any write. Regressions: predecessor definition existing only inside a fenced block and only inside an HTML block, on both paths, asserting nothing is written or consumed.
|
🔁 AI review rerun (requested by @igerber) Head SHA: PR ReviewOverall assessmentExecutive summary
MethodologyNo findings. The canary checks the SSC ancillary sources documented in Code Quality1. Unterminated Markdown blocks can still masquerade as comparison links
PerformanceNo findings. MaintainabilityNo findings. Tech DebtNo findings. Removed backlog rows correspond to implemented work. SecurityNo findings. Workflow permissions remain read-only and actions are SHA-pinned. Documentation / TestsNo additional findings. The required Path to Approval
|
…entirely Round 5's terminal-link-block invariant assumed a block context cannot reach EOF; CommonMark lets an unterminated fence or <pre> stay open through EOF, swallowing the terminal link block itself while every line still matches the scans. Close the whole question structurally: the changelog may not contain ANY persistent block-context starter - fenced code (```/~~~ at 0-3 indent) or raw-HTML block types 1-5 (<pre/script/style/textarea>, processing instructions, declarations, CDATA), plus HTML comments not closed on the same line (the Unreleased pointer's closed form stays legal). Type-6/7 HTML blocks end at the first blank line and cannot reach the terminal block. With no such starter anywhere, no scan ever needs Markdown block context. Enforced pre-branch (fresh + exit-4) and on the assembled text before any write (a 1-space-indented fence in a fragment continuation would land at top level once compiled). Real CHANGELOG audited: zero starters, all comments self-closing. Regressions: unterminated fence and unterminated <pre> on both paths.
…e boundary A blank-line-terminated raw-HTML block (e.g. a bare '<div>') is not a persistent starter, but one sitting DIRECTLY above the definitions would swallow them. Requiring the terminal run to be immediately preceded by a blank line guarantees every type-6/7 block above has ended. Real CHANGELOG already satisfies it; regression on both compile paths.
|
🔁 AI review rerun (requested by @igerber) Head SHA: PR ReviewOverall assessmentExecutive summary
MethodologyNo findings. The canary verifies the SSC ancillary sources specified by the Registry. Code Quality1. CI accepts fragments that the release compiler refuses
PerformanceNo findings. MaintainabilityNo findings. Tech DebtNo findings. Removed TODO rows correspond to implemented work, and this finding is not tracked elsewhere. SecurityNo findings. Workflow permissions are read-only and actions are SHA-pinned. Documentation / TestsNo additional findings. The required Path to Approval
|
A fragment with an indented code fence (or a multi-line link label) passed 'check' at PR time - the line-based grammar cannot see either - then refused at RELEASE time via the assembled-output guards: a CI-approved fragment blocking the release is a fail-late trap. run_check now applies the same whole-text guards per fragment that the assembled output must satisfy (_block_context_violation + _noncanonical_version_link), so everything compile would refuse fails at PR time instead. changelog.d/README.md documents the restrictions (no fenced/raw-HTML blocks - use inline code; no version-link-like constructs). Regressions: indented fence, <pre> block, and multi-line numeric link label all fail check.
|
🔁 AI review rerun (requested by @igerber) Head SHA: PR ReviewOverall assessment✅ Looks good — no unmitigated P0 or P1 findings. Executive summary
MethodologyNo findings. The canary checks the Prop 99 and Walmart sources and replication suite specified in Code QualityNo findings. The prior validation-parity issue is addressed in PerformanceNo findings. MaintainabilityNo findings. Tech DebtNo findings. The removed TODO rows correspond to implemented guards and canary coverage. SecurityNo findings. Workflow permissions are read-only and third-party actions are SHA-pinned. Documentation / TestsNo findings. The prior cases now have regression coverage, the restrictions are documented, and the required |
1 participant
Summary
tests/test_tracking_files.py): CI-enforces the TODO.md/DEFERRED.md split — no TODO table row points work at DEFERRED.md, rows cross-linking anM-xxxledger id do not restate lifecycle state (best-effort tripwire, calibrated against every legitimate current row), and the three documented table shapes (TODO Actionable, DEFERRED blocker, DEFERRED Decision record) are enforced with per-row column counts. The parser honors escaped\|, unspaced rows, and GFM no-leading-pipe tables, and fails loudly on malformed (non-rendering) tables. Wired intodocs-tests.yml(path filters + dedicated step), excluded fromrust-test.ymltriggers; CLAUDE.md's Tracking-file map gains the Decision-record shape it enforces..github/workflows/lwdid-data-canary.yml): weekly cron +workflow_dispatchlane that fails loudly whenload_prop99/load_walmartfall back to synthetic data (attrs["source"] != "lwdid_ssc_ancillary"), then runstests/test_methodology_lwdid.py. Today a network/URL/sha regression at the SSC mirror only produces visible-but-green skips. Test docstrings now name the lane instead of the deleted TODO row./push-pr-update§3b range scan: restores the committed-range pattern scan viapremerge_scan.py --rangeusing the command's ownCOMPARISON_REFresolver; findings stay informational, scan-integrity failures (exit 3/4) are stop-and-report./pre-merge-checksplitspremerge_scanandchangelog_compile.py checkinto separate Bash calls so the first command's exit status is not masked.compare//releases/tag/link;--previous-versioncross-check (/bump-versionpasses it) preserving interrupted-bump recovery; prev-anchor date validation with a documented legacy exemption for historical dateless headers; existing-target sections validated with the full fragment grammar on the exit-4 path; indented ATX headings rejected (CommonMark accepts 1-3 leading spaces, so## [9.9.9]previously compiled into the changelog as a real H2 invisible to the column-zero header scanners); dirty guard catches deleted-but-committed fragments; compile rolls back changelog + fragment deletions on a mid-flight unlink failure; strict kebab fragment-name grammar.changelog.d/20260830-infra-process-sweep.md(### Internal).Methodology references (required if estimator / math changes)
Validation
tests/test_tracking_files.py(new, 44 tests),tests/test_changelog_fragments.py(+~25 regressions),tests/test_methodology_lwdid.py(docstring pointers only)changelog_compile.py check: OKon the live tree; canary workflow YAML validated; TODO.md/DEFERRED.md verified well-formed under the stricter parser before enforcementSecurity / privacy