Run the pre-commit hook against the tree being committed, and only for commits

[Flix6x]

Description

.claude/hooks/pre-commit-check.sh runs pre-commit before a git commit and blocks the commit if
it fails. It got two things wrong, both because a PreToolUse hook runs in the project directory and
does not share the Bash tool's shell state.

It ran in the wrong working tree. The script never read its payload; it ran wherever the hook
process happened to start, which is always the primary checkout. Committing from a git worktree
therefore checked the wrong tree — and worse, a hook that rewrites files rewrote them over there. In
the case that prompted this, regenerating the OpenAPI spec left a stray version change in the
primary checkout while another agent had unrelated uncommitted work in it.

It now works out which tree the command targets, in the same priority order worktree-guard.sh
already uses for the same shared checkout: an explicit -C <path> (or --git-dir=), else a cd
earlier in the same command, else the working directory from the payload, else the project directory.
Whatever it lands on is resolved with git rev-parse --show-toplevel, so a linked worktree resolves
to its own root.

Reading the path out of the command text is the part that matters in practice. The shell's working
directory is reset between Bash calls, so cd <worktree> && git commit ... reports the primary
checkout in the payload's cwd; only the command text says where the commit is really going.

It ran on commands that were not commits. It is registered with "if": "Bash(git commit:*)", but
that settings-level matcher is not honoured by every Claude Code version — worktree-guard.sh says
as much in a comment and repeats its filter in the script. This one did not, so an unrelated command
paid for a full pre-commit run --all-files: it fired on a gh issue create, which is how the stray
spec change happened in the first place.

The filter now lives in the script too, anchored so that git commit-graph and a command that merely
quotes git commit in an issue or PR body are left alone. The cost of anchoring is that an oddly
wrapped commit (time git commit ...) goes unchecked, which is the safe direction for a hook that
only checks.

How to test

The hook is not covered by the test suite, so it was exercised by feeding it payloads directly. With
a deliberate lint error planted in the target worktree, the failure message names the directory the
hooks ran in:

Payload	Expected	Result
cd <worktree> && git commit, cwd = primary checkout	runs in the worktree	✅ pre-commit failed in .../fm-wt-hooks
git -C <worktree> commit	runs in the worktree	✅ same
git commit, cwd = worktree	runs in the worktree	✅ same
git commit, cwd = /tmp (not a repo)	falls back to the project dir	✅ ran in $CLAUDE_PROJECT_DIR
git -c user.name=x commit	still recognised as a commit	✅ ran
git commit-graph write	skipped	✅ exit 0
gh issue create --body "run git commit -s to sign off"	skipped	✅ exit 0, 8 ms instead of ~40 s
git commit on a clean worktree	allowed	✅ exit 0

Afterwards the primary checkout was confirmed to hold only the other agent's own uncommitted work,
with no spec change left behind.

Related items

Fallout from the review of #2472, where the hook fired on a gh issue create and regenerated
flexmeasures/ui/static/openapi-specs.json in the shared primary checkout.

---

Sign-off

• I agree to contribute to the project under Apache 2 License.
• To the best of my knowledge, the proposed patch is not based on code under GPL or another incompatible license.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LtMZ49GH6LaiY5MdgtfNe2

[read-the-docs-community]

Documentation build overview

📚 flexmeasures | 🛠️ Build #34390969 | 📁 Comparing 753a006 against latest (48705b5)

  🔍 Preview build  

1 file changed

± api/v3_0.html

[Copilot]

🟡 Changes recommended

The new command/target parsing has edge cases that can still trigger on non-commit commands and can mis-handle --git-dir/$HOME under set -u, risking incorrect hook behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens the .claude PreToolUse pre-commit hook so it only runs for real git commit invocations and runs pre-commit in the correct working tree (including linked worktrees), avoiding accidental changes in the shared primary checkout.

Changes:

• Parse the hook payload (jq) to extract the executed command and reported cwd, and resolve the actual git top-level directory to run hooks in.
• Add an in-script “commit-only” filter (to not rely on Claude Code’s settings-level matcher).
• Improve failure output to include the directory where pre-commit ran.

File summaries

File	Description
.claude/hooks/pre-commit-check.sh	Parse payload, detect commits, resolve correct repo root/worktree, and run pre-commit there

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The current -C/--work-tree extraction truncates quoted paths containing spaces, which can still cause pre-commit to run in the wrong checkout (undermining the PR’s main goal).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

.claude/hooks/pre-commit-check.sh:21

• Several newly added comment lines break mid-phrase (e.g. ending with “such as”, “a”, “the”), which violates the repo rule that comments/docstrings should only line-break after punctuation. Please reflow this header block so each physical line ends with punctuation (comma/semicolon/colon/period).

This issue also appears on line 32 of the same file.

.claude/hooks/pre-commit-check.sh:36

• The new comment blocks elsewhere in this file also break lines mid-phrase (e.g. around the commit_re explanation and target-selection explanation). To follow the repo’s comment/docstring line-break rule, please reflow those blocks too so every line ends after punctuation.

# Self-filter: only act on `git commit`. Global options such as `-C <path>` or
# `-c foo=bar` may sit between `git` and the verb, so allow a run of option
# words (each optionally followed by its value) first. The verb has to be
# followed by whitespace or the end of the command, so that `git commit-graph`
# is left alone, and `git` has to start the command or follow a `;`, `&` or `|`,

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The current $-path handling can still cause pre-commit to run in the wrong checkout (reintroducing the original shared-primary side effects) instead of failing open when the target path is unresolvable.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The current parsing still has concrete edge cases that can reintroduce false positives or unintended skipping due to quoting/flag extraction behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

.claude/hooks/pre-commit-check.sh:56

• value_after uses a greedy sed pattern (.*$1...) which selects the last occurrence of a flag in the whole command line. Combined with scanning $command for -C/--work-tree/cd, this can misinterpret flags appearing later in the command (or in unrelated segments) and then skip checks unexpectedly. Prefer (1) detecting these tokens on the unquoted command and within the same git … commit segment, and (2) extracting the first match of the flag/value pair.

value_after() {
  echo "$command" | sed -nE "s/.*$1(\"[^\"]*\"|'[^']*'|[^[:space:];&|]+).*/\1/p" | head -n1
}

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

New/updated docstrings in pre_commit_target.py violate the repo’s “wrap only after punctuation” convention and should be reflowed for consistency.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

.claude/hooks/pre_commit_target.py:80

• The target_directory docstring wraps mid-phrase (e.g. line breaks after "process" and "a"), which breaks the repo convention to wrap docstrings/comments only after punctuation. Reflow so each physical line ends with punctuation.

    """The directory whose hooks should run for this command, or None to run none.

    None means either that the command does not commit, or that it points at a tree this process
    cannot resolve: a path built from a shell variable set in an earlier call, or one that is not a
    checkout. Running the hooks somewhere else would check the wrong files, and any hook that

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[Flix6x]

Round 5 found no code findings — only the docstring line-break convention, which is the repo's documented rule rather than a matter of taste, so it is applied in 68dec1a. Every prose line in pre_commit_target.py now ends at punctuation, checked mechanically over the file rather than line by line, and the 12 parsing cases still pass.

That is where I am stopping the review loop. Five rounds, and the shape of them is the useful part of the record:

• Round 1 — three findings: a quoted body could still look like a command, --git-dir names a git directory rather than a working tree, and ${HOME} could abort the hook under set -u.
• Round 2 — a quoted path containing a space was truncated at the space.
• Round 3 — a path the hook could not resolve fell through to a guess, instead of skipping.
• Round 4 — an escaped quote inside a quoted body, and greedy extraction picking the last -C in the line.
• Round 5 — docstring reflow only.

Rounds 1 to 4 were all correct, all distinct, and all landed on the same failure: the hooks running in the shared primary checkout, which is the bug this PR exists to fix. Four different strings fooling four generations of regular expression is what prompted 5ad5e91, which tokenises the command with shlex instead of matching it, and that is the change I would want reviewed by a person rather than iterated on further.

[Copilot]

🟡 Changes recommended

The new target-resolution logic can still pick the wrong checkout for commands combining cd ... with relative git -C/--work-tree (and for multi-cd sequences), which can reintroduce running hooks in the primary checkout.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The changes are narrowly scoped to hook behavior, the parsing approach is robust (token-based rather than regex), and the fail-open behavior avoids the prior risk of running hooks in the wrong checkout.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite