Skip to content

fix(discover): preserve case terminators (;; ;& ;;&) in hook rewrite - #3202

Closed
mgwedd wants to merge 2 commits into
rtk-ai:developfrom
mgwedd:fix/hook-preserve-case-terminators
Closed

mgwedd wants to merge 2 commits into
rtk-ai:developfrom
mgwedd:fix/hook-preserve-case-terminators

Conversation

@mgwedd

@mgwedd mgwedd commented Jul 25, 2026 •

Copy link
Copy Markdown

Summary

  • The lexer tokenized ;; as two separate ; operators, and the rejoin step printed each one back as "; ", so any command that mixed a rewritable command with a case statement got corrupted into a bash syntax error.
  • ;;, ;&, and ;;& are now lexed as single operator tokens, using the same lookahead pattern already used for &&/||, and the rejoin step writes back the token's original value instead of reconstructing it character by character.
  • Fixes bug(hook): rewrite path turns ;; into ; ;, breaking every case statement (bash syntax error) #3197.

Test plan

  • cargo fmt --all && cargo clippy --all-targets && cargo test
  • Manual testing: rtk <command> output inspected

Ran the pre-commit gate locally. I also added a regression test, test_rewrite_preserves_case_terminators. It covers the three terminators (;;, ;&, ;;&) in a case statement. I checked this test fails against the old lexer (reproduces the ; ; corruption). It passes with the fix.

  • Reverted lexer change locally reproduces the ; ; corruption caught by test_rewrite_preserves_case_terminators; fix turns it green.
    quoted/escaped ;; (never operators), ; ; with space (stays two separators), ;;; (greedy-left ;; + ;), UTF-8 adjacent to ;; (offset), ;& at start/end of string.Invalid-bash inputs remain invalid, but don't become worse.
  • Atomic ;; yields byte matching segment boundaries to the old two-; tokens in split_for_permissions/split_on_operators, and case-arm bodies (e.g. a) rm -rf x;;) are still surfaced for permission checking.
  • End-to-end: issue repro an 18-snippet matrix (case/select/until, nested case, quoted ;;, find -exec ;, heredocs, pipes-in-arms, unicode) through rtk hook claude; rewrites pass bash -n (or the original was already invalid, e.g. ;;& on bash v3.2) and case bodies are preserved byte-verbatim. Pipes-inside-case and heredocs correctly fall back to passthrough.

Important: All PRs must target the develop branch (not master).
See CONTRIBUTING.md for details.

Possible follow-up

Not needed for this fix, but worth noting: the lexer handles multi-char operators with per-branch lookahead (| peeks for ||/|&, & for &&/&>, and now ; for ;;/;&/;;&). A table-driven longest-match over a single operator list might make future operators a one-line addition and keep the rejoin spacing rules in one place. The issue also suggested splicing rewritten segments back by byte offset, leaving untouched bytes verbatim — that would get rid of this whole class of rejoin bugs.

@CLAassistant

CLAassistant commented Jul 25, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

…rvive rewrite

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mgwedd
mgwedd force-pushed the fix/hook-preserve-case-terminators branch from 829007e to e81c098 Compare July 25, 2026 03:56
Adversarial coverage for the atomic ;; / ;& / ;;& fix (rtk-ai#3197):
- quoted and backslash-escaped semicolons never become operators
- spaced `; ;` stays two separators; `;;;` splits greedy-left
- UTF-8 before `;;` does not skew offset/segment math
- split_for_permissions still surfaces case-arm bodies for checking
- rewrite keeps quoted `;;` and find -exec \; byte-verbatim

Verified end-to-end: issue repro plus an 18-snippet matrix through
`rtk hook claude` with bash -n validation of every rewrite.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@KuSh

KuSh commented Sep 26, 2026

Copy link
Copy Markdown
Collaborator

Closing in favour of #4130, which makes the same lexer and rewrite change for #3197 and adds regression tests for case statements inside for, while, if and function blocks. Thanks for the careful analysis and the edge case matrix; review or extra cases are welcome on #4130, or reply here if your version covers something it misses.

@KuSh KuSh closed this Sep 26, 2026
@KuSh KuSh added the duplicate This issue or pull request already exists label Sep 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(hook): rewrite path turns ;; into ; ;, breaking every case statement (bash syntax error)

3 participants