fix(hooks): quote $CLAUDE_PROJECT_DIR-prefixed hook commands#2104
Conversation
Claude Code (and Factory Droid) run hook commands via a shell, so an unquoted $CLAUDE_PROJECT_DIR/FACTORY_PROJECT_DIR substitution word-splits when the project path contains a space, breaking the hook. Wrap the generated substitution in double quotes, and teach the import-side stripping logic to recognize both the new quoted form and the legacy unquoted form. Closes #2097
The previous fix wrapped the entire substituted command (including any trailing arguments) in one pair of double quotes, turning e.g. `./script.sh --fix` into a single shell word and breaking hook commands that take arguments, on both Claude Code and Factory Droid. Quote only the `$CLAUDE_PROJECT_DIR`/`$FACTORY_PROJECT_DIR` variable itself so a spaced project path stays one word while trailing arguments remain outside the quotes and still split normally. Update the import-side stripping and add regression tests covering argument-bearing commands for both tools.
dyoshikawa
left a comment
dyoshikawa
left a comment
There was a problem hiding this comment.
The fix correctly resolves the exact reproduction in #2097 (a bare, no-argument dot-relative script with a space in the project path), and the stripCommandPrefix refactor (early-return guard, shared escapedVar) is a nice cleanup. But I think there's a real regression here that's worth fixing before merge: quoting the whole command string instead of just the path segment breaks any hook that has trailing arguments, independent of whether the project path has spaces at all. I verified this empirically with /bin/sh -c — details in the inline comment.
| (!converterConfig.prefixDotRelativeCommandsOnly || trimmedCommand.startsWith(".")); | ||
|
|
||
| // Only the variable itself is quoted (not the whole command) so a project path | ||
| // containing a space can't be word-split by the shell, while any trailing |
There was a problem hiding this comment.
This quotes the entire remainder of the command, not just the projectDirVar/path token. For a canonical hook command like .rulesync/hooks/lint.sh --fix, this now emits the single string "$CLAUDE_PROJECT_DIR/.rulesync/hooks/lint.sh --fix" (quote characters included). When the target tool runs that via /bin/sh -c "<command>", the double quotes suppress word-splitting across the whole thing, so after $CLAUDE_PROJECT_DIR expansion the shell treats path and arguments as one un-splittable word — argv[0] becomes a literal path containing a space, which doesn't exist, and the hook fails with the same class of error this PR is fixing.
I checked this against a real shell: pre-PR (unquoted), .rulesync/hooks/lint.sh --fix splits correctly into lint.sh + --fix even with no space in the path. Post-PR (quoted), the same command fails with not found. So this trades a narrow bug (spaces in the project path, no-arg commands only) for a broader one (any dot-relative command with trailing arguments, regardless of path spaces) — and issue #2097's own suggested fix called this out explicitly: quote only the $CLAUDE_PROJECT_DIR/<path> token so trailing arguments are preserved outside the quotes.
Worth noting Factory Droid (factorydroid-hooks.ts) doesn't set prefixDotRelativeCommandsOnly, so there shouldPrefix is true for any command not starting with $ — meaning this same failure mode hits a much wider set of commands there, not just dot-relative ones.
None of the new tests cover a command with trailing arguments (both only test bare, no-arg scripts), so this wouldn't be caught by pnpm cicheck as currently written. Adding a case like .rulesync/hooks/lint.sh --fix to the export test, and asserting the quotes wrap only the path (e.g. "$CLAUDE_PROJECT_DIR/.rulesync/hooks/lint.sh" --fix), would both prove the fix and guard against this regressing again.
The Claude Code hooks e2e assertion checked for the literal substring $CLAUDE_PROJECT_DIR/, which no longer occurs now that only the variable itself is quoted. Assert on the actual generated command value directly instead of a JSON.stringify substring match.
|
@dyoshikawa Thank you! |
2 participants
Summary
/bin/sh -c "<command>". The./→$CLAUDE_PROJECT_DIR/-style rewrite emitted an unquoted substitution, so a project path containing a space gets word-split and the hook fails withNo such file or directory.$CLAUDE_PROJECT_DIR/...(and$FACTORY_PROJECT_DIR/...) substitution in double quotes at export time.stripCommandPrefixto recognize both the new quoted form and the legacy unquoted form, so round-tripping existing generated files keeps working.$are left untouched, as before.Test plan
pnpm cicheck(full lint/typecheck/test/docs/gitignore/schema/spelling/secrets suite) passes./-relative command on importCloses #2097