Follow-up from advisory review threads on #687 (skills/pr-batch/bin/pr-route-provenance, tests in skills/pr-batch/bin/pr-route-provenance-test.rb). None of these blocks #687. Each was reproduced against head d9dbd54f8385c35b02627d4e2738bae299a4cfc3.
1. Marker discovery is not fence-aware and is implemented three times
apply_to_body (raw body.scan(START_MARKER) / body.sub(...)), validate_managed_section!, and validate_applied_body! each re-implement the "zero or exactly one ordered managed section" check with raw string scans. agent_details_closing_index already walks fenced blocks, but marker discovery does not.
Reproduced: a PR body whose only marker pair sits inside a ```markdown example (no live section) gets the example's inner text replaced by the generated table, which then renders as literal text inside the fence. When a live section also exists, apply refuses with "zero or exactly one ordered managed section" even though the body is well-formed.
Direction: one fence-aware helper (for example managed_section_span(text)) that skips fenced blocks and returns the single live span or raises, used by all three call sites.
2. Bounded ledger gaps
commit_rows caps the number of rows at MAX_COMMIT_ROWS but not the size of a row: every wave that attributes the same commit is concatenated into one cell. Reproduced: 30 receipts attributing one commit produce a single 2,251-character row with 30 fragments. A squash workflow with many waves grows without bound and can push the body past GitHub's 65,536-character limit, at which point the PATCH fails.wave_rows omits the middle waves above MAX_WAVE_ROWS (20), but commit_rows still cites them by number. Reproduced: with 25 receipts, the commit table says "Wave 15 ..." while the waves table shows only waves 1-10, the omission row, and 16-25.
Direction: cap fragments per commit cell (first/last N plus an "M more waves remain in the receipts" note), and either flag cited-but-omitted waves in the commit table or list the omitted wave numbers in the omission row.
3. Exact duplicate receipts are accepted
validated_receipts only checks batch/target uniqueness. Reproduced: passing the same receipt file twice (--receipt a.json --receipt a.json) renders two identical waves and attributes the commit to both, so one execution counts as two pieces of route evidence.
Direction: reject receipts whose canonical_json is identical before sorting and numbering.
4. Disposition enumeration is duplicated
MEASUREMENT_DISPOSITIONS (skills/pr-batch/bin/pr-route-provenance lines 15-21) hardcodes the same five keys as ValidateExecutionProvenance::DISPOSITIONS (bin/validate-execution-provenance lines 40-46). Reproduced: adding a value to the validator lets the receipt validate, then route_row raises KeyError (the CLI exits 1 with "Error: key not found"). Fail-closed, but drift is only detected at render time.
Direction: assert at load time or in the test file that MEASUREMENT_DISPOSITIONS.keys.sort == ValidateExecutionProvenance::DISPOSITIONS.sort, or derive the map from the validator's list with an explicit default.
5. Cleanup while in the file
display(value) is identical to code(value). Fold the call sites into code, or give display a distinct rendering if one was intended.
6. GitHubClient parity with skills/post-merge-audit/bin/completed-batch-audit-receipt
PrRouteProvenance::GitHubClient#gh_api (skills/pr-batch/bin/pr-route-provenance lines 50-62) differs from the sibling gh_api in skills/post-merge-audit/bin/completed-batch-audit-receipt (lines 1495-1532):
- No subprocess timeout: the sibling uses
capture_process(command, input:, timeout: gh_timeout_seconds) with a configurable default; this client calls Open3.capture3 with no bound, so a stalled gh api blocks the integration closeout indefinitely.
- No
--hostname: the sibling takes an explicit host for GitHub Enterprise; this client always targets github.com.
The missing force_encoding("UTF-8") before JSON.parse is a separate defect being fixed on #687 itself and is not tracked here.
Direction: reuse the sibling's bounded capture and host handling, or lift both into a shared helper.
Done when
apply_to_body ignores marker pairs inside fenced blocks (tests: fenced example plus live section updates only the live section; fenced example alone appends a new section and leaves the example intact), and the three ordered-section checks share one helper.- Wave fragments per commit cell are capped and cited-but-omitted waves are explicit; a test covers more than 20 waves attributing one commit.
- Duplicate canonical receipts are rejected, with a test.
- A test fails if the two disposition lists drift.
gh_api has a bounded timeout with a test, and the host is configurable or explicitly documented as github.com-only.ruby skills/pr-batch/bin/pr-route-provenance-test.rb and rubocop on both files pass.
Follow-up from advisory review threads on #687 (
skills/pr-batch/bin/pr-route-provenance, tests inskills/pr-batch/bin/pr-route-provenance-test.rb). None of these blocks #687. Each was reproduced against headd9dbd54f8385c35b02627d4e2738bae299a4cfc3.1. Marker discovery is not fence-aware and is implemented three times
apply_to_body(rawbody.scan(START_MARKER)/body.sub(...)),validate_managed_section!, andvalidate_applied_body!each re-implement the "zero or exactly one ordered managed section" check with raw string scans.agent_details_closing_indexalready walks fenced blocks, but marker discovery does not.Reproduced: a PR body whose only marker pair sits inside a ```markdown example (no live section) gets the example's inner text replaced by the generated table, which then renders as literal text inside the fence. When a live section also exists,
applyrefuses with "zero or exactly one ordered managed section" even though the body is well-formed.Direction: one fence-aware helper (for example
managed_section_span(text)) that skips fenced blocks and returns the single live span or raises, used by all three call sites.2. Bounded ledger gaps
commit_rowscaps the number of rows atMAX_COMMIT_ROWSbut not the size of a row: every wave that attributes the same commit is concatenated into one cell. Reproduced: 30 receipts attributing one commit produce a single 2,251-character row with 30 fragments. A squash workflow with many waves grows without bound and can push the body past GitHub's 65,536-character limit, at which point the PATCH fails.wave_rowsomits the middle waves aboveMAX_WAVE_ROWS(20), butcommit_rowsstill cites them by number. Reproduced: with 25 receipts, the commit table says "Wave 15 ..." while the waves table shows only waves 1-10, the omission row, and 16-25.Direction: cap fragments per commit cell (first/last N plus an "M more waves remain in the receipts" note), and either flag cited-but-omitted waves in the commit table or list the omitted wave numbers in the omission row.
3. Exact duplicate receipts are accepted
validated_receiptsonly checks batch/target uniqueness. Reproduced: passing the same receipt file twice (--receipt a.json --receipt a.json) renders two identical waves and attributes the commit to both, so one execution counts as two pieces of route evidence.Direction: reject receipts whose
canonical_jsonis identical before sorting and numbering.4. Disposition enumeration is duplicated
MEASUREMENT_DISPOSITIONS(skills/pr-batch/bin/pr-route-provenancelines 15-21) hardcodes the same five keys asValidateExecutionProvenance::DISPOSITIONS(bin/validate-execution-provenancelines 40-46). Reproduced: adding a value to the validator lets the receipt validate, thenroute_rowraisesKeyError(the CLI exits 1 with "Error: key not found"). Fail-closed, but drift is only detected at render time.Direction: assert at load time or in the test file that
MEASUREMENT_DISPOSITIONS.keys.sort == ValidateExecutionProvenance::DISPOSITIONS.sort, or derive the map from the validator's list with an explicit default.5. Cleanup while in the file
display(value)is identical tocode(value). Fold the call sites intocode, or givedisplaya distinct rendering if one was intended.6. GitHubClient parity with
skills/post-merge-audit/bin/completed-batch-audit-receiptPrRouteProvenance::GitHubClient#gh_api(skills/pr-batch/bin/pr-route-provenancelines 50-62) differs from the siblinggh_apiinskills/post-merge-audit/bin/completed-batch-audit-receipt(lines 1495-1532):capture_process(command, input:, timeout: gh_timeout_seconds)with a configurable default; this client callsOpen3.capture3with no bound, so a stalledgh apiblocks the integration closeout indefinitely.--hostname: the sibling takes an explicit host for GitHub Enterprise; this client always targets github.com.The missing
force_encoding("UTF-8")beforeJSON.parseis a separate defect being fixed on #687 itself and is not tracked here.Direction: reuse the sibling's bounded capture and host handling, or lift both into a shared helper.
Done when
apply_to_bodyignores marker pairs inside fenced blocks (tests: fenced example plus live section updates only the live section; fenced example alone appends a new section and leaves the example intact), and the three ordered-section checks share one helper.gh_apihas a bounded timeout with a test, and the host is configurable or explicitly documented as github.com-only.ruby skills/pr-batch/bin/pr-route-provenance-test.rbandrubocopon both files pass.