Skip to content

fix: claude command detecting issue for windows - #3648

Closed
0xhossein80 wants to merge 1 commit into
rtk-ai:developfrom
0xhossein80:develop
Closed

0xhossein80 wants to merge 1 commit into
rtk-ai:developfrom
0xhossein80:develop

Conversation

@0xhossein80

Copy link
Copy Markdown

This fix is according to issue #3632
I changed the shell_split function structure to make it easier to detect commands and better for both Windows and Unix systems:

pub fn shell_split(input: &str) -> Vec<String> {
    input
        .replace('"', "")
        .split_whitespace()
        .map(|s| s.to_string())
        .collect()
}

I updated the test for is_claude_hook_command() at src/hooks/mod.rs:26 to check Windows commands too.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@KuSh

KuSh commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Thanks for digging into this. The rtk.exe half of this landed on develop in #3552 (merged 2026-09-13) and was extended in #3008 (merged 2026-09-18): the shared is_rtk_binary helper in src/hooks/mod.rs now matches both rtk and rtk.exe, with Windows positive and negative tests alongside it. The shell_split half can no longer apply, because #3704 (merged 2026-09-04) rebuilt shell_split on the shared tokenize and coalesce_words primitives that the rewrite and discover paths also use, so the hand-rolled scanner this PR replaces is no longer in the file. The case from #3632 is still real on develop: an unquoted Windows path such as C:\Users\x\rtk.exe hook claude has its backslashes consumed as escapes by resolve_word_text, so the hook command is not recognised, and a narrow fix inside the shared lexer (or a platform-aware binary-name compare) would be very welcome as a fresh PR. Closing this one as it stands, but please comment or reopen if you think a part of it was missed.

@KuSh KuSh closed this Sep 19, 2026
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.

3 participants