fix(hooks): stop rewriting ${CLAUDE_PROJECT_DIR} hook bindings to baked absolute paths - #109
Conversation
…ked absolute paths
The SessionStart hook health check compared the installed command
byte-for-byte against a canonicalized absolute --path pin, so a committed
portable form (--path "\${CLAUDE_PROJECT_DIR}") classified as Stale and any
`loomweave install` / `doctor --fix` rewrote the tracked settings file to a
hard-coded path — breaking downstream CI (elspeth's
test_repository_hooks_bind_to_claude_project_dir) and silently pointing
linked worktrees at the main checkout.
Now: a functionally-current entry (templated \$CLAUDE_PROJECT_DIR /
\${CLAUDE_PROJECT_DIR}, bare or double-quoted, or a literal pin resolving to
this project root) classifies Present and is left byte-for-byte untouched
(the installer short-circuits before parsing, so foreign hook entries are
not re-serialized either). When a write genuinely is needed, the emitted
command binds --path "\${CLAUDE_PROJECT_DIR}" instead of baking the resolved
path. Single-quoted templates stay Stale — single quotes suppress expansion,
so that entry is broken.
Closes clarion-3fbb9cdfcd.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates Loomweave’s Claude Code SessionStart hook installer/state-check logic to avoid rewriting ${CLAUDE_PROJECT_DIR}-templated hook commands into baked absolute paths, preventing unnecessary churn in tracked .claude/settings.json files and improving compatibility across checkouts and linked worktrees.
Changes:
- Introduces
hook_command_is_current()to treat${CLAUDE_PROJECT_DIR}(and equivalent forms) and current-project absolute pins as “Present” without rewriting. - Switches installer output to a portable
--path "${CLAUDE_PROJECT_DIR}"command (DESIRED_HOOK_COMMAND) instead of embedding an absolute project path. - Adds regression tests ensuring templated hooks remain byte-for-byte untouched and validating equivalence classification.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let unquoted = arg | ||
| .strip_prefix('\'') | ||
| .and_then(|s| s.strip_suffix('\'')) | ||
| .or_else(|| arg.strip_prefix('"').and_then(|s| s.strip_suffix('"'))) | ||
| .unwrap_or(arg); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84c29bb5c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | "${CLAUDE_PROJECT_DIR}" | ||
| | "$CLAUDE_PROJECT_DIR" |
There was a problem hiding this comment.
When an existing setting uses the bare ${CLAUDE_PROJECT_DIR}/$CLAUDE_PROJECT_DIR form, this classifies it as Present, so install_session_start_hook returns without rewriting it. In shell-form hooks Claude runs the command through a shell that tokenizes and expands variables (docs), so a project path containing spaces is split and --path receives the wrong value; only the double-quoted placeholder forms should be treated as current, or the installer should repair bare forms to DESIRED_HOOK_COMMAND.
Useful? React with 👍 / 👎.
Summary
Reported from ~/elspeth: the hook installer/repair rewrote a committed
.claude/settings.jsonentryloomweave hook session-start --path "${CLAUDE_PROJECT_DIR}"into a hard-coded--path '/home/john/elspeth', breaking elspeth's CI testtest_repository_hooks_bind_to_claude_project_dirand re-serializing (key-reordering) foreign hook entries in the same file.Root cause:
session_start_hook_state/ the install merge compared the installed command byte-for-byte againstdesired_hook_command()'s canonicalized absolute pin, so the functionally-identical templated form classifiedStaleand got rewritten by anyloomweave install/doctor --fix.Fix
hook_command_is_current()accepts the templated$CLAUDE_PROJECT_DIR/${CLAUDE_PROJECT_DIR}forms (bare or double-quoted; single-quoted stays Stale since single quotes suppress expansion) and any literal pin resolving to the current project root. Such entries classifyPresentand are left byte-for-byte untouched — the installer short-circuits before parsing, so foreign entries aren't re-serialized on the no-op path.--path "${CLAUDE_PROJECT_DIR}"instead of a baked absolute path — correct for tracked settings shared across checkouts and for linked worktrees.Present(no churn wave across existing installs).Residual (split to follow-up clarion-1bbde98622): a genuine rewrite still round-trips the file through
serde_json::Valuewithoutpreserve_order, alphabetizing foreign entries' keys. Enablingpreserve_orderworkspace-wide is unsafe (feature unification would change canonical JSON bytes under SEI/HMAC/golden hashing), so surgical writes need their own change.Closes clarion-3fbb9cdfcd.
Testing
legisentry whose key order must survive), legacy current-project pin no-op, single-quoted template Stale, equivalence-form matrix, emission binds${CLAUDE_PROJECT_DIR}.-D warnings, build, nextest (2313 passed), doc-D warnings, deny, ruff/ruff-format/mypy --strict/pytest (317 passed).🤖 Generated with Claude Code