fix(source-control): local PR-body gates check all four contract sections (0.55.11)

[kyle-sexton]

Closes #3206

Summary

The shared PR-body linkage validator only checked a closing keyword and ## Related, so both local pre-checks allowed bodies the pinned pr-issue-linkage reusable rejects. Local gates now require the same four contract sections CI does.

Fix

pr-linkage-validator.sh generalizes related_section() into section_content() over Summary, Fix, Verification, and Related, using the existing heading-level semantics (a nested ### is content). Each missing or empty section is its own LINKAGE_PROBLEMS entry. Both hook surfaces pick this up from the shared core; their blocked-message remedies now list all four sections so following them produces a body CI accepts. The no-issue-marker path and update_pull_request without body are unchanged.

Verification

bash plugins/source-control/hooks/pr-body-linkage-gate.test.sh — passed: 137 failed: 0

bash plugins/source-control/hooks/pr-linkage-mcp-gate.test.sh — pass=25 fail=0

New cases: the #3205 body (linkage + Summary + Verification + Related, no Fix) is blocked; a nested ### inside each required section is content. check-changelog-parity.sh --check-bump origin/main passes (0.55.6).

Related

• fix(docs-hygiene): correct audit-noise's stated negation limitation (0.21.2) #3205 — the PR whose pr-issue-linkage failure is the reproduction
• fix(ci): correct the pr-issue-linkage header's claims about the pinned reusable #3209 — comment-only header correction on the same reusable pin; no functional overlap
• .github/workflows/pr-issue-linkage.yml — pins the upstream reusable at v0.14.2

[claude]

Claude finished @kyle-sexton's task in 56s —— View job

---

Security review complete ✅

• Gather context (PR diff, changed files)
• Apply skip-gate check
• Review changed files for security-relevant issues
• Post findings / no-issues summary

Skip-gate check: PR is open, and the diff does have a narrow security-adjacent surface (a PreToolUse gate that decides whether to block gh pr create/gh pr edit and MCP create_pull_request/update_pull_request calls), so I read all 8 changed files in full rather than skipping.

Scope: All changes are in plugins/source-control/: pr-linkage-validator.sh (shared core), pr-body-linkage-gate.sh and pr-linkage-mcp-gate.sh (the two consuming hooks), their two test files, CHANGELOG.md, README.md, and the plugin.json version bump/option descriptions.

Findings: None. This PR only tightens an existing local validation gate — it generalizes related_section() into section_content(body, heading) and loops it over four hardcoded headings (REQUIRED_SECTIONS=(Summary Fix Verification Related)) instead of just Related. Relevant checks I made:

• The heading_lc value interpolated into the bash regex at pr-linkage-validator.sh#L125 comes only from the static REQUIRED_SECTIONS array, never from the PR body or other attacker-controlled input — no regex-injection surface.
• The gate's effect is strictly more restrictive (blocks a superset of bodies it used to block), so there's no way this change weakens the gate into allowing something CI would reject — the failure mode of a gate bug here would be over-blocking, not a bypass.
• No new command execution, eval, network calls, credential/token handling, or file-path handling was introduced.
• No GitHub Actions workflow files are touched, so no permissions/trigger/pinning concerns apply.
• Test-file changes are fixture expansion only (adding the four sections to existing fixtures plus new cases for the fix(docs-hygiene): correct audit-noise's stated negation limitation (0.21.2) #3205 repro and nested ### handling) — no new execution surface.

No security concerns to report on this PR.

[github-actions]

Last security-reviewed head: 8208c925ec2c64f093063b70b95b8b723e27eeba. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b33908049

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 3m 14s —— View job

---

Code review complete

• Gather context (diff, CLAUDE.md, REVIEW.md, skip gate)
• Review changed hook scripts and tests for correctness
• Check for security-lane overlap (scope out if applicable)
• Post findings as inline comments + summary

No blocking findings. This is a clean, well-scoped fix.

What I checked

• pr-linkage-validator.sh — related_section() → section_content(body, heading) generalization is correct: the start-of-section scan (^##[[:space:]]+${heading_lc}$) and the terminator scan (lvl <= 2 closes, deeper headings are content) preserve the original heading-level semantics exactly, just parameterized. linkage::problems() now loops REQUIRED_SECTIONS=(Summary Fix Verification Related) and reports each missing/empty section independently, which matches the stated goal (surface every problem in one pass rather than block-fix-reblock).
• Both hook surfaces (pr-body-linkage-gate.sh, pr-linkage-mcp-gate.sh) — block-message remedies were updated in lockstep to list all four sections, so following the remedy produces a body the real pr-issue-linkage reusable accepts. Header comments ("two halves" → "five requirements") were updated consistently across both files.
• Test coverage — both .test.sh files got a shared SECTIONS fixture threaded through the existing cases (closing-keyword shapes, HTML-comment stripping, locale/CRLF cases, perf case), plus a dedicated ISSUE_3205/NESTED case each reproducing the exact reported regression (Summary+Verification+Related, no Fix) and confirming a nested ### inside every one of the four sections is still content, not a terminator. Coverage looks complete for the change's surface area.
• Docs/version — plugin.json description, README.md options table, and CHANGELOG.md are all updated consistently with the 0.55.5 → 0.55.6 bump.
• Cross-check: /source-control:pull-request create's own pre-create gate (§2.4.2.2 in create.md) uses a separately config-driven REQUIRED_SECTIONS (a different, intentionally generic seam per the pr-body-linkage-gate.sh SCOPE GUARD comment). I verified this repo's own .claude/source-control.md already sets pr_body_required_sections to Summary, Fix, Verification, Related — matching the hardcoded contract this PR adds — so there's no drift between the skill's gate and the two hooks this PR touches in this repo.

Not independently verified

I could not execute shell scripts in this sandbox (test-run and syntax-check commands were denied), so the test-suite pass counts in the PR description (137/0 and 25/0) are author-claimed, not independently confirmed by me. I did trace the logic and new test fixtures by hand and they're internally consistent with the code changes.

[github-actions]

Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @cursor[bot]'s task in 3m 33s —— View job

---

Code review complete

• Gather context (PR diff, CLAUDE.md, REVIEW.md, skip gate)
• Review changed hook scripts and tests for correctness
• Check for security-lane overlap (scope out if applicable)
• Post findings as inline comments + summary

No blocking findings.

This lane last reviewed commit 3b33908 (the original four-section change). The branch has since gained a follow-up commit, 92963ab — mask_markdown_code in pr-linkage-validator.sh — which directly resolves the one substantive finding raised in this thread: Codex's P2 comment on pr-linkage-validator.sh:125 (a required heading matched inside a fenced/indented/inline code sample). I traced the new masking logic against CommonMark's fence-close and inline-code-span rules line by line:

• Fence open/close (backtick vs. tilde, closing run ≥ opening length, closing line content-free, a backtick fence's info string rejecting an embedded backtick) all match spec.
• Indented-code detection (4 spaces / tab) is checked before inline-span scanning, so it takes precedence the same way CommonMark's block parser does.
• Inline code-span masking correctly removes the entire span (delimiters included) only when a matching same-length backtick run is found later on the line; otherwise the opening run is emitted literally and scanning resumes past it — matching CommonMark's non-backtracking algorithm.
• Masked lines are blanked rather than deleted, so line-based heading/section-boundary logic in section_content() is unaffected by the extra pass.

Test coverage for the fix is direct: fenced-fix.md / indented-fix.md / inline-fix.md / real-plus-fenced-fix.md in pr-body-linkage-gate.test.sh reproduce exactly the scenario Codex described (a real Summary/Verification/Related plus only a templated ## Fix) and assert it now blocks, plus a FENCED_FIX case added to the MCP-surface test. CHANGELOG.md/README.md/plugin.json were bumped to 0.55.7 consistently to cover both fixes.

Two minor, non-blocking observations (not filing as inline comments — neither rises to block-worthy):

• The PR title/description still say "(0.55.6)" and the Verification section's test counts (137/25) predate the masking-fix commit, which adds several more test cases — worth a title/description touch-up before merge, but doesn't affect correctness.
• I could not execute the test suite in this sandbox (shell execution was denied, consistent with the prior review pass on this PR), so the pass counts remain author-claimed; I verified correctness by tracing the logic and fixtures by hand instead.

No security-relevant surface changed since the prior security-lane pass (still a local-gate tightening only, no new command execution/eval/network/credential handling).

[claude]

Claude finished @kyle-sexton's task in 7m 40s —— View job

---

Security review complete

• Gather context (PR diff, custom instructions)
• Invoke /review:security-review per custom instructions
• Post findings / no-issues summary

Skip-gate check: This head (505b2e9) is not fully covered by the prior security-lane pass, which last reviewed commit 3b33908 — before the follow-up mask_markdown_code commit (66017d6) landed. That commit adds ~80 lines of new Markdown-parsing logic on a security-adjacent surface (the PreToolUse gate deciding whether to block gh pr create/gh pr edit and MCP create_pull_request/update_pull_request), so I reviewed it in full rather than skipping. The only change since the code-review lane's last pass (bfc1ce0-equivalent state) is 505b2e9, which just trims 4 CHANGELOG lines — no functional surface.

Finding: 1 IMPORTANT (posted inline)

Superlinear inline-code-span scan in mask_markdown_code() can push the hook past its declared 15s timeout — which this repo's own ADR 0004 documents as failing the gate open.
plugins/source-control/hooks/pr-linkage-validator.sh#L189-L218

The new inline-backtick-span scanner rescans the entire rest of the line from each opening backtick run's position looking for a same-length closing run, with no bound on the search distance. A single crafted line built from strictly-increasing, all-distinct backtick-run lengths never finds a match for any opening run, so cost grows as O(L^1.5) rather than O(L) like every other per-line scan in this file. Within GitHub's ~65536-char PR-body ceiling, that's plausibly enough bash-level work to exceed the 15s timeout both consuming hooks declare in hooks.json. This repo has already measured and documented that a killed PreToolUse hook fails open ("the tool call proceeds unguarded" — docs/adr/0004-...md:146-149), so a PR body with one crafted long line could make this gate pass not because the body satisfies the four-section contract, but because masking never finishes in time. Real CI (pr-issue-linkage) would still catch a non-compliant body, so this defeats the local pre-check rather than achieving a full bypass — but it undercuts the exact hardening this PR is adding, via a fail-open mechanism the repo already treats as a known risk class elsewhere (guardrails/hooks/block-exported-msys-pathconv.sh's "a hook that hangs is worse than one that misses").

I could not execute a live timing PoC in this lane's restricted sandbox (only gh pr diff/view/comment/review, Read, Glob, Grep are available) — this is a traced-by-hand-simulation finding, not one confirmed by execution. Full detail and a suggested fix direction (bound the inner search distance) are in the inline comment.

Everything else in the diff (the Summary/Fix/Verification/Related heading loop, fence-open/close handling, both hook surfaces' remedy messages, test fixtures, docs/version bump) is unchanged from what the prior security and code-review passes already vetted, and I found no other new security-relevant surface: no new command execution, eval, network calls, credential handling, or GitHub Actions workflow changes.

[claude]

Claude finished @kyle-sexton's task in 1m 38s —— View job

---

Security review complete

• Gather context (PR diff, prior review history, skip-gate check)
• Determine what changed since the last security-lane pass
• Review any new security-relevant surface
• Post findings / no-issues summary

Skip-gate check: The prior security-lane pass (23:30:41Z) reviewed head 505b2e9 in full — including the mask_markdown_code() addition — and filed one IMPORTANT finding. The only commit since then is 2efc6d0 ("fold the 0.55.9 Fixed bullets"), which touches only plugins/source-control/CHANGELOG.md — removing a duplicate ### Fixed heading and merging two bullet lists. I confirmed this by diffing that commit directly: no hook script, test, or plugin.json changed. No new security-relevant surface, so no full re-review was needed.

Status of the outstanding finding

The previously-filed IMPORTANT finding is still open in the current code and has not been addressed by this branch:

Superlinear inline-code-span scan in mask_markdown_code() can push the hook past its declared timeout, which this repo's own ADR 0004 documents as failing the gate open.
plugins/source-control/hooks/pr-linkage-validator.sh#L189-L218

For each opening backtick run, the inner while ((j < len)) loop rescans the rest of the line looking for a same-length closing run with no bound on distance. A crafted line built from strictly-increasing, all-distinct backtick-run lengths never finds a match for any opening run, giving O(L^1.5) work per line instead of the O(L) every other scan in this file achieves. Within GitHub's 65536-char PR-body ceiling that's plausibly enough bash-level work to exceed the 15s timeout both pr-body-linkage-gate.sh and pr-linkage-mcp-gate.sh declare in hooks.json — and a killed PreToolUse hook fails open per docs/adr/0004-rightsize-instruction-surfaces-by-incumbent-first-arbitration.md:146-149. Impact is limited to defeating the local pre-check (real pr-issue-linkage CI still catches a non-compliant body), not a full bypass, but it undercuts the exact hardening this PR adds. Suggested direction: bound the inner search distance (e.g., cap scanned distance or max backtick-run length considered) instead of an unbounded rescan.

I'm not re-filing this as a new inline comment since the code is byte-identical to what was already reviewed and commented on inline at 23:38:24Z — flagging its unresolved status here so it isn't lost before merge.

No other findings. No new command execution, eval, network calls, credential handling, or GitHub Actions workflow changes were introduced by the docs-only commit reviewed here.
(branch: cursor/3206-pr-body-linkage-cfcf)

[claude]

Claude finished @kyle-sexton's task in 1m 45s —— View job

---

Security review complete

• Gather context (PR diff, prior review history, skip-gate check)
• Determine what changed since the last full security-lane pass
• Review any new security-relevant surface
• Post findings / no-issues summary

Skip-gate check: This head (de49d2e) diffs cleanly against origin/main to the same 8 files this PR has always touched (+237/-76, matching the PR's declared stats). The last full security-lane pass reviewed head 505b2e9 (including mask_markdown_code()) and filed one IMPORTANT finding; a follow-up pass then confirmed 2efc6d0 changed only CHANGELOG.md. I diffed plugins/source-control/hooks/ between 505b2e9 and the current head directly and it is byte-identical — the two commits since the last full pass (fcff4f7→5975238 equivalents folded into changelog history, plus the final changelog-fold commit) touch only CHANGELOG.md and the plugin.json version bump (0.55.9 → 0.55.11), no hook script or test file. No new security-relevant surface, so no full re-review was needed.

Status of the outstanding finding

The previously-filed IMPORTANT finding is still open and unchanged in the current code:

Superlinear inline-code-span scan in mask_markdown_code() can push the hook past its declared timeout, which this repo's own ADR 0004 documents as failing the gate open.
plugins/source-control/hooks/pr-linkage-validator.sh#L189-L218

For each opening backtick run, the inner while ((j < len)) loop (L195-L207) rescans the rest of the line looking for a same-length closing backtick run with no bound on distance. A crafted line built from strictly-increasing, all-distinct backtick-run lengths never finds a match for any opening run, giving O(L^1.5) work per line instead of the O(L) every other scan in this file achieves. Within GitHub's 65536-char PR-body ceiling that's plausibly enough bash-level work to exceed the 15s timeout both pr-body-linkage-gate.sh and pr-linkage-mcp-gate.sh declare in hooks.json — and a killed PreToolUse hook fails open per docs/adr/0004-rightsize-instruction-surfaces-by-incumbent-first-arbitration.md:146-149. Impact is limited to defeating the local pre-check (real pr-issue-linkage CI still catches a non-compliant body), not a full bypass, but it undercuts the exact hardening this PR adds. Suggested direction: bound the inner search distance (e.g., cap scanned distance or max backtick-run length considered) instead of an unbounded rescan.

Not re-filing as a new inline comment since the code is byte-identical to what's already commented on inline — flagging its unresolved status here so it isn't lost before merge.

No other findings. No new command execution, eval, network calls, credential handling, or GitHub Actions workflow changes since the last full pass.
(branch: cursor/3206-pr-body-linkage-cfcf)

[claude]

Claude finished @kyle-sexton's task in 7m 38s —— View job

---

Security review complete

• Gather context (PR diff, prior review history, skip-gate check)
• Invoke /review:security-review and follow its instructions
• Determine what changed since the last full security-lane pass
• Review any new security-relevant surface
• Post findings / no-issues summary

Skip-gate check: The last full security-lane pass reviewed head de49d2e (byte-identical hook code to 505b2e9, which added mask_markdown_code() and drew one outstanding IMPORTANT timing finding). Since then, git log de49d2e..52a6028 contains exactly one new commit — 52a6028 "pair inline-code spans in one linear collect pass" — which rewrites the inline-backtick-span matching in mask_markdown_code() to fix that reported O(L^1.5) timing issue. That's a real change to the same security-adjacent PreToolUse gate, so I reviewed it in full.

Finding: 1 IMPORTANT (posted inline)

The collect-then-pair rewrite mis-masks nested, differing-length backtick spans, leaking their gap text into the unanchored has_linkage() keyword scan.
plugins/source-control/hooks/pr-linkage-validator.sh#L208-L231

The new algorithm collects every backtick run on a line, then pairs each with the next unused same-length run and records spans_start/spans_end in the order each pairing completes, not in position order. For a properly nested pair of differing-length runs — outer opens, an inner same-length pair opens+closes fully inside it, outer closes (the ordinary code idiom for escaping a literal backtick) — the inner span always closes first and lands at spans_start[0], while the outer span (whose start position is earlier) lands at spans_start[1]. The render loop walks i monotonically checking only the current sidx, so once it advances past the inner span it can never fire the outer span whose start it already passed — the outer span's delimiters and surrounding gap text get emitted literally instead of masked.

Concretely: See the format: closes #5 x for reference. is one CommonMark inline code span (the outer matches the nearest equal-length run, so everything inside — including the inner x — is code). This algorithm masks the inner x `` (it closes first) but leaks the outer span's gap text closes #5 verbatim. `has_linkage()` then matches that leaked text with an unanchored ERE over the whole body, so a PR body with no genuine `Closes/Fixes/Resolves #N` — just an ordinary doc-style aside using nested backticks — would satisfy the closing-keyword requirement locally. (`section_content()`'s heading scan is line-anchored and isn't reachable the same way, since the outer run's own backtick characters always remain a literal prefix on the leaked line.)

Impact is the same class as the timing finding this exact commit fixed: it defeats this local pre-check rather than achieving a full bypass, since real pr-issue-linkage CI (assumed to implement correct CommonMark masking) would still mask the whole span and reject the PR. But it reopens the gap this PR exists to close, via an unremarkable nesting pattern, and isn't covered by the new superlinear-ticks.md test (which only exercises all-distinct, never-matching run lengths for the timing fix — not a nested/matching-but-differing-length case). Suggested direction in the inline comment: sort spans by start position before the render pass, or use a stack-based nearest-match walk that mirrors CommonMark's actual algorithm.

I could not execute a live PoC in this lane's restricted sandbox (Bash execution and file writes were denied) — this is traced by hand-simulating the algorithm step-by-step for the construction above, not confirmed by execution.

Everything else

Unchanged from what prior passes already vetted: the Summary/Fix/Verification/Related heading loop, fence-open/close handling, both hook surfaces' remedy messages, docs/version bump. No new command execution, eval, network calls, credential handling, or GitHub Actions workflow changes.
(branch: cursor/3206-pr-body-linkage-cfcf)

[claude]

Claude finished @kyle-sexton's task in 6m 4s —— View job

---

Security review complete

• Gather context (PR diff, prior review history, skip-gate check)
• Invoke /review:security-review and follow its instructions
• Determine what changed since the last full security-lane pass
• Review any new security-relevant surface
• Post findings / no-issues summary

Skip-gate check: The prior full security-lane pass reviewed head 52a6028 (the closer-first pairing-order bug) and filed one IMPORTANT finding. Since then, git log 52a6028..ff8ceb5 contains exactly one new commit — ff8ceb5 "keep outermost nested inline-code spans" — which is the fix for that finding, rewriting mask_markdown_code()'s span selection in pr-linkage-validator.sh plus two new test cases. That's a real change to the same security-adjacent PreToolUse gate, so I reviewed it in full.

Finding: 1 IMPORTANT (posted inline)

The sort-and-keep-outermost fix resolves the previously-reported matched-inner-pair case, but the underlying collect-then-pair loop still leaves stale pending state for a same-length run that gets absorbed as content inside an already-closed, differing-length span — corrupting a later, independent same-length pair on the same line and leaking its content unmasked into has_linkage().
plugins/source-control/hooks/pr-linkage-validator.sh#L207-L252

Concrete construction (traced by hand step-by-step through the pairing loop):

Use `` ` `` because `closes #5` is shown.

The outer `` `` is the canonical CommonMark idiom for escaping a literal backtick — it contains exactly **one** (odd count) inner single-backtick run. When that inner run'spending[1]entry is never cleared after the outer span closes, the *next* real single-backtick pair on the line (``closes #5``) gets mis-paired with the stale entry instead of its true partner. The sort/prune step correctly discards the resulting bogus crossing span, but that also discards the only computation that touched thecloses #5span's opener — and its closer is left dangling, unpaired. Net effect: the real, non-overlapping ``closes #5`` code span is never masked at all, andcloses #5survives verbatim into the bodylinkage::problems()hands tohas_linkage()'s unanchored regex ([L281](https://github.com/melodic-software/claude-code-plugins/blob/ff8ceb5ffa78e4ec61e70b66311b1df1fe99c50a/plugins/source-control/hooks/pr-linkage-validator.sh#L281)), satisfying the closing-keyword requirement with no genuine Closes #N` line anywhere in the body.

This isn't a contrived edge case — the escaped-backtick idiom is common in exactly the kind of PR that discusses backtick-masking logic (like this one), so a body with real Summary/Fix/Verification/Related sections, one such aside, and an unrelated inline-code span later on the same line could pass this local gate's linkage check without a real link. Impact stays in the same class already fixed twice on this branch: it defeats the local pre-check rather than fully bypassing linkage (real pr-issue-linkage CI, assumed to implement correct CommonMark masking, would still catch it). Not covered by this commit's new nested-ticks.md/nested-ticks-plus-real.md cases, which only exercise a matched (even-count) inner pair, not an odd/unbalanced same-length run trapped inside a differing-length span.

Full trace and a suggested fix direction (clear any pending entry whose run index falls strictly inside a span being closed) are in the inline comment. I could not execute a live PoC in this lane's sandbox (Bash execution was denied for scripted verification) — this is a hand-traced finding, not one confirmed by execution.

Everything else

Unchanged from what prior passes already vetted: the Summary/Fix/Verification/Related heading loop, fence-open/close handling, the O(L^1.5)-timing fix from the prior round, both hook surfaces' remedy messages, docs/version bump. No new command execution, eval, network calls, credential handling, or GitHub Actions workflow changes.
(branch: cursor/3206-pr-body-linkage-cfcf)

[claude]

Claude finished @kyle-sexton's task in 2m 26s —— View job

---

Security review complete

• Gather context (PR diff, prior review history, skip-gate check)
• Invoke /review:security-review and follow its instructions
• Determine what changed since the last full security-lane pass
• Review any new security-relevant surface
• Post findings / no-issues summary

Skip-gate check: The last full security-lane pass reviewed head ff8ceb5 (the sort-and-keep-outermost fix) and filed one IMPORTANT finding: the collect-then-pair loop's per-length pending map isn't invalidated for runs consumed as content inside an already-closed span, so an odd-count same-length run nested inside a differing-length span (the common `` `` escaped-backtick idiom) strands its own real trailing pair unmasked. Current head8573e01("drop closing-keyword literals from comments") diffs againstff8ceb5 in exactly two files — [pr-body-linkage-gate.test.sh](https://github.com/melodic-software/claude-code-plugins/blob/8573e01445d018c964cf2ab333dfb77eaf2936c4/plugins/source-control/hooks/pr-body-linkage-gate.test.sh) and [pr-linkage-validator.sh](https://github.com/melodic-software/claude-code-plugins/blob/8573e01445d018c964cf2ab333dfb77eaf2936c4/plugins/source-control/hooks/pr-linkage-validator.sh) — and both diffs are comment-text-only edits (replacing a literal "closes #N"example inside code comments with generic wording), confirmed viagit diff ff8ceb5..8573e01: the hunk touches only comment lines, no line-count shift, no logic change. mask_markdown_code()`'s actual pairing/sort/prune code is byte-identical to what the prior pass already reviewed and commented on inline. No new security-relevant surface, so no full re-review was needed.

Status of the outstanding finding

Still open and unaddressed. I hand-traced the exact construction from the prior finding against the current (unchanged) code at pr-linkage-validator.sh#L207-L252 to confirm it still reproduces:

Use `` ` `` because `closes #5` is shown.

Walking the pairing loop: the outer `` `` (len-2 run … len-1 run … len-2 run) pairs correctly and is kept after the sort/prune step. But pairing the len-2 close at index 2 also consumes the len-1pendingentry that was set by the *inner* lone backtick at index 1 — wait, more precisely: after the outer pair closes, the stalepending[1]entry (from the escaped-backtick run) is never cleared, so the *next* real len-1 opener (the one starting ``closes #5``) wrongly pairs with it instead of its true partner, producing a crossing span the sort/prune step correctly discards — but that discard also throws away the only pairing that touched the true closer, leaving it dangling and unpaired. Net result: the real, non-overlapping ``closes #5`` span is never masked at all, andcloses #5(with backtick-delimited neighbors, whichKEYWORD_ERE's [^a-z0-9_]boundaries still match) survives into the bodyhas_linkage()scans with its unanchored regex — satisfying the closing-keyword requirement with no genuineCloses/Fixes/Resolves #N` anywhere in the PR body.

This still isn't a contrived case: `` `` is the canonical CommonMark idiom for showing a literal backtick, and remains untested — the newnested-ticks.md/nested-ticks-plus-real.mdcases (unchanged sinceff8ceb5) only exercise a *matched* inner pair (`` x ``, even count), not the *odd*-count case that trips the stale-pending` bug.

I could not execute a live PoC in this lane's sandbox (Bash was denied for scripted verification of the trace above) — this is hand-traced, not confirmed by execution. Not re-filing a new inline comment since the flagged code is byte-identical, at the same line numbers, to what's already commented on inline in this thread — restating its status here so it isn't lost before merge. Suggested direction (unchanged from the prior review round): when closing a span, also drop any pending entry whose run index falls strictly inside the span being closed, since those runs were consumed as content and can never legitimately pair with anything outside it.

Everything else

Unchanged from what prior passes already vetted: the Summary/Fix/Verification/Related heading loop, fence-open/close handling, the O(L^1.5) timing fix, the matched-inner-pair nesting fix, both hook surfaces' remedy messages, docs/version bump. No new command execution, eval, network calls, credential handling, or GitHub Actions workflow changes.
(branch: cursor/3206-pr-body-linkage-cfcf)

[claude]

Claude finished @kyle-sexton's task in 4m 29s —— View job

---

Security review complete

• Gather context (PR diff, prior review history, skip-gate check)
• Invoke /review:security-review and follow its instructions
• Determine what changed since the last full security-lane pass
• Review any new security-relevant surface
• Post findings / no-issues summary

Skip-gate check: The last full security-lane pass reviewed head ff8ceb5 and filed one IMPORTANT finding (stale pending-map state in the collect-then-pair loop letting a same-length run inside an escaped-backtick idiom strand a later real pair). Since then, git log ff8ceb5..8208c92 has two commits: 8573e01 (comment-wording only — confirmed via git diff ff8ceb5..8573e01 -- plugins/source-control/hooks/pr-linkage-validator.sh, a one-line comment edit, no logic change) and 8208c92 ("pair inline code left-to-right like CommonMark"), which rewrites mask_markdown_code()'s span-matching in pr-linkage-validator.sh#L153-L269. That's a real change to the same security-adjacent PreToolUse gate, so I reviewed it in full.

Finding: previously-reported IMPORTANT issue is resolved

8208c92 replaces the per-length pending-map pairing with a different algorithm: walk backtick runs left to right, and for each not-yet-consumed opener, scan forward for the first not-yet-consumed run of the same length; everything strictly between the two is marked consumed ("absorbed as content") in the same step (L221-L252). This is the standard CommonMark code-span algorithm (leftmost opener consumes through the nearest same-length closer; intervening backtick runs of any length become content, never available to pair with anything outside), and it structurally can't reproduce either previously-reported bug:

• Nested differing-length runs ( x ): the outer pair's search finds its closer directly (the inner run is skipped over, not the target), so the whole nested construct becomes one span in a single step — no closer-first ordering exists to orphan the outer span.
• The escaped-tick idiom trapping a later pair ( `` `` followed later by ``closes feat(hook-telemetry): marketplace-wide telemetry contract + markdown-formatter producer #5``): there's no persistent per-length map to go stale. The lone inner tick from the escaped-tick idiom gets marked "used" (absorbed) the moment the outer pair closes, in the *same* iteration — it's never visited as an opener afterward, so it can never steal the later, unrelated same-length pair. I hand-traced this exact construction against the new code and thecloses feat(hook-telemetry): marketplace-wide telemetry contract + markdown-formatter producer #5` span is correctly found and masked.

I also re-verified the complexity property the original P2/IMPORTANT timing finding was about, since this is a different algorithm than the one that fixed it last round: the search-forward loop's cost is bounded by O(nruns) for any span that resolves (disjoint, non-overlapping intervals across the whole line, since absorbed runs are skipped in O(1) on their outer-loop visit), and by O(nruns²) only for runs that never find a match at all — which requires all-distinct run lengths, and distinct lengths 1..n cost ~n²/2 characters to produce, capping nruns at O(√L). Both cases bound total work to O(L), matching every other per-line scan in this file. This isn't just asserted — I worked through the disjoint-interval argument by hand for several adversarial shapes (all-distinct lengths, mirrored long-distance pairs, nested widely-spaced fillers) and none produce worse than O(L).

Both new test fixtures directly targeting this — escaped-tick.md (pr-body-linkage-gate.test.sh) and ESCAPED_TICK (pr-linkage-mcp-gate.test.sh) — exercise exactly the construction from the prior finding and assert it still blocks. CHANGELOG.md was updated in the same commit to describe the new algorithm accurately, replacing the now-inaccurate "sorted by start / reduced to outermost" description. No other files (plugin.json, README.md) changed since the last pass.

I could not execute the test suite or a live PoC in this lane's sandbox (Bash script execution required approval that wasn't granted) — this finding-resolution assessment is based on hand-tracing the algorithm and reasoning about its complexity bound, not execution. Given two prior rounds each found a real bug in a plausible-looking fix, I'd treat "tests pass" here as author-claimed until someone runs the suite, but the algorithm itself is now a textbook-correct implementation rather than an ad hoc patch, which is a meaningfully different (and more trustworthy) shape of fix than the previous two iterations.

Everything else

Unchanged from what prior passes already vetted: the Summary/Fix/Verification/Related heading loop, fence-open/close handling, both hook surfaces' remedy messages, docs/version bump. No new command execution, eval, network calls, credential handling, or GitHub Actions workflow changes.

No new findings to file inline.
(branch: cursor/3206-pr-body-linkage-cfcf)