Skip to content

fix(hooks): two enforcement papercuts - commit-graph and wrapped dashes#494

Merged
SUaDtL merged 3 commits into
mainfrom
fix/485-git-subcommand-matching
Jul 26, 2026
Merged

fix(hooks): two enforcement papercuts - commit-graph and wrapped dashes#494
SUaDtL merged 3 commits into
mainfrom
fix/485-git-subcommand-matching

Conversation

@SUaDtL

@SUaDtL SUaDtL commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

What

Two enforcement papercuts, both shared-core changes projected to all three hosts. Grouped because they share the version bump — a fourth contended ca-pi number for two small independent fixes is churn.

#485git commit-graph was gated as git commit

COMMIT_RE spelled the subcommand commit\b, and a word boundary sits between commit and -, so every commit-* verb matched.

The annoyance was not the cost. The gate told the operator to run the crypto-compliance gate for a command that writes no objects, creates no commit, and can introduce neither a crypto nor a secret change — leaving two escapes, both wrong: record a security-gate pass that certifies nothing, or reach for /ca:override, which is precisely the override-invented-to-cover-a-coverage-hole that ADR-0022 and #308 exist to prevent. It also bites at the worst possible moment, since a stale commit-graph is what a batch of --delete-branch merges leaves behind — which is how it was found.

The exclusion is an explicit ALLOW of the one verb proven safe, not an allowlist of gated ones, so the failure direction stays closed: a commit-* name the matcher has never seen is still gated. commit-tree deliberately stays in scope — it creates a commit object, and a crafted commit plus update-ref is a real path around H-01.

AC-4: the sibling-matcher audit

push has no push-* subcommand to over-match onto. git add--interactive genuinely does stage, so add\b reaching it is correct rather than a false positive. Neither needs the treatment, and both were left alone.

What the audit did surface is recorded in the code and knowingly not fixed: these matchers scan command text, so a git command quoted inside another command's arguments is gated as if it were being run — a grep "git add" trips H-03, which happened twice during this work. Exempting quoted occurrences would be a real bypass, not a loosening: sh -c "git add ." executes what it quotes. Telling the two apart needs actual shell tokenization, and over-blocking a mention is the fail-CLOSED side of that trade (ORCHESTRATOR §2). Left as-is with the reasoning in place, so the next reader doesn't "fix" it unsafely.

#484 — a separator dash at a line-wrap boundary was invisible

The detector read one line at a time and required word characters on both sides of the dash on the same line, so a separator at a soft-wrap boundary scored zero in both directions:

...one of three states —          ← right-hand span on the next line
listed below.

...the heavyweight audit
— checkpoints are the lean sweep.  ← left-hand span on the previous line

All three fixtures are real misses, found by hand in the site's own codearbiter-directory.md while fixing #338's 118 flagged lines — violations a reader sees that the gate did not report.

It now scans a paragraph at a time. _prose_only still runs per line and before the join, so inline code, URLs, ranges, and the definition-list lead-in are stripped exactly as they were, and each finding still carries the line of its own dash rather than the paragraph's. Joining stops at every block boundary — blank line, list item, heading, table row, thematic break, fence — and a table row is never joined at all, because lending a neighbouring row's words to a lone N/A marker is the exact thing the cell split exists to prevent. A range split by the wrap (12– / 18) is re-checked on the join, since the same-line exemption cannot reach it.

Verification

Unit tests are the easy part for a detector this widely wired; the risk is regression on real prose and vacuous boundary tests. Both were checked.

Monotone, and measured to be. Across the 23 in-scope prose files: no finding the old detector reported was lost, and 19 were gained. Every one sampled is a genuine wrap separator, e.g. CONTRIBUTING.md:121, CHANGELOG.md:42.

Nothing new blocks. check_site_voice.py — the blocking CI gate — is still clean on all 36 authored site pages. H-13 is a remind(), so the 19 are reminders on a doc edit, not a new block on files like CHANGELOG.md.

The boundary tests are not vacuous. They assert [], so they would pass against an implementation that never joins. Mutating _starts_new_block to always return False (join across every boundary) turns 7 of them red, so they genuinely constrain the implementation rather than just agreeing with it.

test_hook_guards.py 146 assertions / 0 failed (4 new, RED-proven first: commit-graph write and commit-graph verify were blocked before the fix). test_sloplib.py 47 tests OK (14 new). test_hooklib, test_hooks_cold_install, test_migration_backstop, test_pre_read, test_ci_impact all green.

Versions

ca-pi advances 0.1.27 → 0.1.30: #491 claims 0.1.28 and #493 claims 0.1.29, and two branches must never claim one ca-pi number. Merge after both. ca (2.9.1) and ca-codex (0.3.0) carry no tag yet, so their entries join the open sections.

Closes #485
Closes #484

https://claude.ai/code/session_01Y8UPz5RZwgnda3NbAVfd6L

SUaDtL added 3 commits July 26, 2026 09:42
Both are gates misreading text they were never meant to match, both were found
by hitting them during the sprint, and both are shared-core changes projected to
all three hosts. Grouped because they share the version bump: a fourth
contended ca-pi number for two small independent fixes is churn.

#485 - `git commit-graph` was gated as `git commit`. COMMIT_RE spelled the
subcommand `commit\b`, and a word boundary sits between `commit` and `-`, so
every `commit-*` verb matched. The annoyance was not the cost. The gate told the
operator to run the crypto-compliance gate for a command that writes no objects,
creates no commit, and can introduce neither a crypto nor a secret change -
leaving two escapes, both wrong: record a security-gate pass that certifies
nothing, or reach for `/ca:override`, which is precisely the override-invented-
to-cover-a-coverage-hole that ADR-0022 and #308 exist to prevent. It also bites
at the worst possible moment, since a stale commit-graph is what a batch of
`--delete-branch` merges leaves behind - which is how it was found.

The exclusion is an explicit ALLOW of the one verb proven safe, not an allowlist
of gated ones, so the failure direction stays closed: a `commit-*` name the
matcher has never seen is still gated. `commit-tree` deliberately stays in scope
- it creates a commit object, and a crafted commit plus `update-ref` is a real
path around H-01.

AC-4 asked for an audit of the sibling matchers. `push` has no `push-*`
subcommand to over-match onto, and `git add--interactive` genuinely does stage,
so `add\b` reaching it is correct rather than a false positive. What the audit
did surface is recorded in the code and knowingly NOT fixed: these matchers scan
command TEXT, so a git command quoted inside another command's arguments is
gated as if it were being run - a `grep "git add"` trips H-03, twice during this
work. Exempting quoted occurrences would be a real bypass, not a loosening:
`sh -c "git add ."` executes what it quotes. Telling the two apart needs actual
shell tokenization, and over-blocking a mention is the fail-CLOSED side of that
trade (ORCHESTRATOR §2).

#484 - the prose-separator-dash detector read one line at a time. It required
word characters on both sides of the dash ON THE SAME LINE, so a separator at a
soft-wrap boundary scored zero in BOTH directions: the right-hand span on the
next line (`...one of three states —` / `listed below.`) or the left-hand span on
the previous one (`...the heavyweight audit` / `— checkpoints are the lean
sweep.`). All three fixtures are real misses found by hand in the site's own
`codearbiter-directory.md` while fixing #338's 118 flagged lines.

It now scans a paragraph at a time. `_prose_only` still runs per line and before
the join, so inline code, URLs, ranges and the definition-list lead-in are
stripped exactly as they were, and each finding still carries the line of its own
dash rather than the paragraph's. Joining stops at every block boundary - blank
line, list item, heading, table row, thematic break, fence - and a table row is
never joined at all, because lending a neighbouring row's words to a lone N/A
marker is the exact thing the cell split exists to prevent. A range split BY the
wrap (`12–` / `18`) is re-checked on the join, since the same-line exemption
cannot reach it.

Verification beyond the unit tests, which is where the risk actually lives for a
detector this widely wired: the change is monotone by construction, and measured
to be - across the 23 in-scope prose files, no finding the old detector reported
was lost, and 19 were gained. Every one sampled is a genuine wrap separator. The
blocking CI gate (`check_site_voice.py`) is still clean on all 36 authored site
pages, and H-13 is a `remind()`, so the 19 are reminders on a doc edit rather
than a new block on files like CHANGELOG.md.

ca-pi advances 0.1.27 -> 0.1.30: #491 claims 0.1.28 and #493 claims 0.1.29, and
two branches must never claim one ca-pi number. ca (2.9.1) and ca-codex (0.3.0)
carry no tag yet, so their entries join the open sections.

Closes #485
Closes #484

Claude-Session: https://claude.ai/code/session_01Y8UPz5RZwgnda3NbAVfd6L
The first cut of the paragraph join rebuilt the joined prefix and suffix strings
for every line of every paragraph, so scanning one N-line paragraph did O(N^2)
string construction. Measured on a single long paragraph: 45ms at 3000 lines and
481ms at 10000, against 4ms and 13ms for the old per-line scan. This runs on
every write and edit through H-13, so that is the wrong direction to move a hook
that was previously free.

The fix is to pass the surrounding context PRE-REDUCED instead of as text. The
dash test only ever asks two questions of the lines around it: is there a word
character further out, and is the immediately adjacent text a digit (the
numeric-range exemption). The first is two accumulating passes per paragraph;
the second needs only the nearest neighbour. Now 4.8ms and 15.5ms - a 9x and 31x
improvement, and linear.

The accumulation deliberately spans the WHOLE paragraph rather than peeking at
the neighbouring line, which would have been the tempting simplification. A line
that is entirely inline code strips to whitespace under `_prose_only`, so the
real continuation can sit two lines from the dash; a one-line lookahead misses
it. `test_context_reaches_past_a_line_that_strips_to_nothing` pins exactly that,
and mutating the accumulation to an adjacent-only lookahead kills it - so the
test constrains the reduction rather than merely agreeing with it.

Behaviour is unchanged, and proven so rather than assumed: across all 741
Markdown files in the repository the linear implementation and the quadratic one
it replaces produce byte-identical findings - 4997 of them, zero mismatches.
The site-voice gate stays clean on all 36 authored pages.

Refs #484

Claude-Session: https://claude.ai/code/session_01Y8UPz5RZwgnda3NbAVfd6L
@SUaDtL
SUaDtL merged commit ff5f5eb into main Jul 26, 2026
73 of 75 checks passed
@SUaDtL
SUaDtL deleted the fix/485-git-subcommand-matching branch July 26, 2026 14:18
SUaDtL added a commit that referenced this pull request Jul 26, 2026
…ows (#496)

The pi security-evidence test fails on windows-latest with "Test timed out in
5000ms" - a harness timeout, not an assertion. Seen three times on 2026-07-26
across two branches and both pinned Pi runtimes, while ubuntu-latest and
macos-latest passed every one of those runs. It fails on main too (run
30203161506), so it is not branch-specific.

The script is not the slow part: `test_pi_security.py --contract-only` runs in
about 70ms, and it imports nothing this repository changed recently. What does
not fit in 5s is a cold CPython process launch on a loaded Windows runner, and
the whole cost lands inside the test's budget because the spawn is synchronous.
The deadline was provisioned for the script's runtime when what it has to cover
is a process creation.

So the timeout is what is wrong, not the code under test, and it is now
platform-aware rather than uniformly larger - a genuine hang on the fast
platforms still surfaces as a hang. Both tests in the file that spawn Python
synchronously get it, so the second does not resurface separately (AC-4). No
assertion is touched: the secret-bearing ambient-value checks are unchanged.

This is not cosmetic. `[GATE ] | [REPO] | Merge readiness` aggregates this
suite, and the release preflight requires that gate green for the EXACT commit
being tagged (#385), so a flake here blocks the release lane for that SHA and
trains readers to re-run a red gate without reading it. It cost two merge
attempts on PR #494.

No version bump: plugins/ca-pi/tools/** is outside the shipped payload
(payload_scope.py), and this changes a test only.

Closes #495

Claude-Session: https://claude.ai/code/session_01Y8UPz5RZwgnda3NbAVfd6L

Co-authored-by: SUaDtL <SUaDtL@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pre-bash H-09b/H-10b treat git commit-graph as a commit Anti-slop dash detector misses a separator at a line-wrap boundary

1 participant