Skip to content

fix(hooks): scan every apply_patch span in a bash command - #21

Draft
yannelli wants to merge 1 commit into
mainfrom
yannelli/fix-apply-patch-spans-b88c
Draft

yannelli wants to merge 1 commit into
mainfrom
yannelli/fix-apply-patch-spans-b88c

Conversation

@yannelli

@yannelli yannelli commented Sep 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

Random file sampling (shuf --random-source=/dev/urandom) landed on apply-patch.mjs and codex-tests.mjs. Both had real bugs.

Production: extractPatch only kept the first *** Begin Patch … *** End Patch span. A Bash command with two apply_patch heredocs scanned the first file and ignored the rest, so a long comment in the second patch would not be denied. parseApplyPatch also kept attaching + lines after *** End Patch to the last file, so trailing junk could be treated as file content.

Test: the multi-file “reason names the violating file” check printed FAIL but never incremented the suite failure count, so that assertion could not fail CI.

Rebased onto main after the main-only branch flow, so Conventional PR no longer treats this as a dev → main release PR.

Behavior

  • Every Begin/End Patch span in a shell command is extracted and scanned.
  • parseApplyPatch closes the current file on *** Begin Patch / *** End Patch, so + lines after the terminator are ignored.
  • The violating-file assertion now uses the shared ok/bad counters.

Checks

Ran locally after the rebase:

node scripts/check.mjs
node --test test/*.test.mjs
env -u HOME -u USERPROFILE -u XDG_CONFIG_HOME node plugins/concise/test/run-tests.mjs
node scripts/coverage.mjs "$NODE_V8_COVERAGE"

scripts/check.mjs reported valid syntax/JSON/versions. node --test passed 193/193. Plugin tests: 1811 passed, 0 failed. Line coverage 100.00% (5854/5854). check-pr.mjs accepts this title against main.

Migration

None.

Open in Web Open in Cursor 

@yannelli
yannelli marked this pull request as ready for review September 15, 2026 22:23
@yannelli
yannelli marked this pull request as draft September 22, 2026 02:34
@yannelli
yannelli changed the base branch from dev to main October 5, 2026 05:42
@yannelli yannelli closed this Oct 5, 2026
@yannelli yannelli reopened this Oct 5, 2026
@yannelli yannelli closed this Oct 5, 2026
@yannelli yannelli reopened this Oct 5, 2026
extractPatch only kept the first Begin/End Patch block, so a second
heredoc in the same Bash call skipped comment checks. parseApplyPatch
also kept reading + lines after End Patch into the last file.

Co-authored-by: Ryan Yannelli <ryanyannelli@gmail.com>
@cursor
cursor Bot force-pushed the yannelli/fix-apply-patch-spans-b88c branch from 54569c0 to cc756aa Compare October 5, 2026 05:46
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.

1 participant