Skip to content

fix: wrap hook commands in bash -c for paths with spaces - #47

Open
carmandale wants to merge 1 commit into
parcadei:mainfrom
carmandale:fix/hooks-path-spaces-clean
Open

fix: wrap hook commands in bash -c for paths with spaces#47
carmandale wants to merge 1 commit into
parcadei:mainfrom
carmandale:fix/hooks-path-spaces-clean

Conversation

@carmandale

@carmandale carmandale commented Jan 11, 2026

Copy link
Copy Markdown

Summary

  • Wraps all 25 hooks using $CLAUDE_PROJECT_DIR in bash -c '...' to handle paths containing spaces
  • Fixes hook failures when project path contains spaces (e.g., "Groove Jones Dropbox")

Problem

When $CLAUDE_PROJECT_DIR contains spaces, hooks fail because Claude Code strips quotes before shell execution, causing word splitting:

python3 "/Users/dale/Groove Jones Dropbox/..."
# Becomes: python3 /Users/dale/Groove with "Jones" and "Dropbox/..." as separate args

Solution

Wrap commands in bash -c '...' so bash handles variable expansion with proper quoting:

// Before
"command": "python3 \"$CLAUDE_PROJECT_DIR/.claude/hooks/script.py\""

// After  
"command": "bash -c 'python3 \"$CLAUDE_PROJECT_DIR/.claude/hooks/script.py\"'"

Changes

  • .claude/settings.json - Wrapped 25 hooks using $CLAUDE_PROJECT_DIR
  • Left 12 hooks using $HOME unchanged (macOS home paths never have spaces)

Testing

  • JSON validates: python3 -c "import json; json.load(open('.claude/settings.json'))"
  • All CLAUDE_PROJECT_DIR hooks wrapped: grep 'CLAUDE_PROJECT_DIR' .claude/settings.json | grep -v "bash -c" | wc -l returns 0
  • Tested in actual Claude Code session with spaced path - hooks execute successfully

Risk

Low - Pure find-and-replace transformation. ~5ms overhead per hook is negligible.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Updated command execution configuration for internal hook system
    • Enhanced hook targeting with conditional execution refinement across multiple stages
    • Standardized path handling in configuration system

✏️ Tip: You can customize this high-level summary in your review settings.

Problem: Claude Code hooks fail when $CLAUDE_PROJECT_DIR contains
spaces (e.g., "Groove Jones Dropbox"). The shell splits the path
on spaces even when quoted in settings.json.

Solution: Wrap all commands using $CLAUDE_PROJECT_DIR in bash -c '...'
so bash handles variable expansion with proper quoting.

Changes:
- Wrapped 25 hooks using $CLAUDE_PROJECT_DIR with bash -c
- Preserved all timeout values
- Left $HOME hooks unchanged (no spaces in home paths)

Pattern applied:
  Before: python3 "$CLAUDE_PROJECT_DIR/.claude/hooks/script.py"
  After:  bash -c 'python3 "$CLAUDE_PROJECT_DIR/.claude/hooks/script.py"'
@coderabbitai

coderabbitai Bot commented Jan 11, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The .claude/settings.json configuration file was updated to wrap all external command invocations (python, node, bash) in bash -c statements with properly quoted paths. Matcher fields were added to multiple hook blocks across PreToolUse, SessionStart, UserPromptSubmit, PostToolUse, and Stop sections for refined conditional execution. A trailing newline was normalized at file's end.

Changes

Cohort / File(s) Summary
Configuration Execution Wrapper Updates
.claude/settings.json
Wrapped all external command invocations (statusLine.command and hooks) in bash -c '...' format with quoted paths. Examples: python3 \"$CLAUDE_PROJECT_DIR/.claude/scripts/status.py\", node \"$CLAUDE_PROJECT_DIR/.claude/hooks/dist/*.mjs\". Affected commands include pre-tool-use, session management, import validation, diagnostics, and other hook files.
Hook Matcher Field Additions
.claude/settings.json
Added matcher fields to hook blocks in PreToolUse (Read|Edit|Write, Read, Grep, Task, Edit), SessionStart (startup|resume, resume|compact|clear), UserPromptSubmit, PostToolUse, and Stop (Bash) sections for conditional hook application.
File Formatting
.claude/settings.json
Normalized trailing newline at end of file.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A config file danced in bash's embrace,
With matchers and quotes keeping each hook in place,
Commands now cradled in shells that care,
Settings refined with precision and flair!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: wrap hook commands in bash -c for paths with spaces' directly and specifically describes the main change: wrapping hook commands in bash -c to handle paths containing spaces.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
.claude/settings.json (1)

249-249: Redundant bash wrapping.

This command wraps bash inside bash -c, creating two bash processes. While functional, it could be simplified:

-"command": "bash -c 'bash \"$CLAUDE_PROJECT_DIR/.claude/plugins/braintrust-tracing/hooks/stop_hook.sh\"'"
+"command": "bash -c '\"$CLAUDE_PROJECT_DIR/.claude/plugins/braintrust-tracing/hooks/stop_hook.sh\"'"

Alternatively, if the script has a proper shebang, you could execute it directly. However, if explicit bash invocation is required for compatibility, the current approach is acceptable.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 395b7b2 and 3f60223.

⛔ Files ignored due to path filters (1)
  • .claude/hooks/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • .claude/settings.json
🔇 Additional comments (7)
.claude/settings.json (7)

4-4: LGTM: statusLine command properly wrapped.

The bash -c wrapping with escaped quotes correctly handles spaces in $CLAUDE_PROJECT_DIR.


12-76: LGTM: PreToolUse hooks properly wrapped.

All PreToolUse hooks referencing $CLAUDE_PROJECT_DIR are consistently wrapped in bash -c with proper quoting.


101-106: LGTM: SessionStart hooks properly wrapped.

All SessionStart hooks using $CLAUDE_PROJECT_DIR are correctly wrapped in bash -c with proper quoting.

Also applies to: 125-130


145-145: LGTM: UserPromptSubmit hooks properly wrapped.

All UserPromptSubmit hooks using $CLAUDE_PROJECT_DIR are correctly wrapped in bash -c.

Also applies to: 150-150, 155-155


176-186: LGTM: PostToolUse hooks properly wrapped.

All PostToolUse hooks using $CLAUDE_PROJECT_DIR are correctly wrapped in bash -c with proper quoting.

Also applies to: 224-224, 234-234


277-277: LGTM: Trailing newline added.

Good practice to end JSON files with a newline.


87-87: Address the inconsistency between $HOME and $CLAUDE_PROJECT_DIR path wrapping.

All $CLAUDE_PROJECT_DIR commands are wrapped in bash -c, but $HOME paths (lines 87, 97, 116, 141, 160, 171, 196, 205, 215, 263, 267, 271) are not. If the wrapping is necessary to prevent quote stripping when paths contain spaces, $HOME could face the same issue. Either wrap $HOME commands consistently, or document why they're exempt from this treatment.

@parcadei

Copy link
Copy Markdown
Owner

Hey @carmandale - nice fix! This properly handles the spaced paths issue.

There are merge conflicts with settings.json (we've made changes since your fork). Could you rebase on main and resolve the conflicts? Then we can merge.

Thanks!

— v3 bot 🤖

@manishin2050 manishin2050 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job

Have you tested..?

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