Skip to content

fix(hooks): stop triggering full editable rebuild on every Edit/Write#661

Merged
zackees merged 2 commits into
mainfrom
fix/hook-no-project-cross-repo
Jun 20, 2026
Merged

fix(hooks): stop triggering full editable rebuild on every Edit/Write#661
zackees merged 2 commits into
mainfrom
fix/hook-no-project-cross-repo

Conversation

@zackees

@zackees zackees commented Jun 19, 2026

Copy link
Copy Markdown
Member

Summary

Two long-standing pain points with .claude/hooks:

  1. Every Claude Edit/Write hook fire was a soldr cargo build --release -p fbuild-cli. settings.json used uv run --script ci/hooks/<name>.py without --no-project. uv discovered the fbuild project and ran setuptools.build_meta.build_editable (which fbuild's pyproject wires to the native Rust build). So every file edit kicked off a release build of the CLI — minutes of latency per edit, plus a hard failure whenever the underlying soldr/zccache daemon flaked. All hook scripts use PEP 723 inline script metadata so they don't actually need the project installed.

  2. lint.py ran ./lint against files outside this repo. When editing files in a sibling repo's worktree (e.g. zccache's), detect_crate() read the worktree's crates/<x>/ segment and ran soldr cargo clippy -p <x> against the fbuild workspace. Either errored (no such package) or — when names collided — lint'd the wrong code.

Fix

  • Add --no-project to every hook command in .claude/settings.json.
  • lint.py: skip files outside PROJECT_ROOT. Invoke ./lint via sys.executable instead of uv run --script so the inner call doesn't trigger another editable rebuild.
  • .gitignore: .claude/tmp/ (scratch dir for hereoc'd issue bodies during Claude sessions).

No CI behavior change — cargo fmt --all --check + clippy still run via the Stop hook on workspace-level passes. Per-file Edit hooks now finish in milliseconds instead of minutes.

Test plan

  • Edit a .rs file inside this repo — hook fires fast, lints just that file
  • Edit a .rs file inside a sibling repo's worktree (e.g. ~/dev/zccache/.claude/worktrees/.../crates/zccache/src/...) — hook skips, no spurious clippy run against fbuild
  • uv run --no-project --script ci/hooks/lint.py < /dev/null — completes without trying to build fbuild

Refs: surfaced during the zccache#784 work session — the cross-repo edit case is exactly what was misbehaving.

Summary by CodeRabbit

  • Chores

    • Updated development environment configuration and build hooks.
    • Added temporary files directory to ignore list.
  • Refactor

    • Optimized lint execution to improve build performance by reducing unnecessary rebuild steps.
    • Enhanced lint request validation to handle edge cases.

Two long-standing pain points with `.claude/hooks` on this repo:

1. **Every hook fire was a `soldr cargo build --release -p fbuild-cli`.**
   The settings.json entries used `uv run --script ci/hooks/<name>.py`
   without `--no-project`. uv interpreted the cwd as the fbuild project
   root and ran `setuptools.build_meta.build_editable`, which fbuild's
   pyproject wires to the native fbuild-cli build. So every Edit/Write
   kicked off a release build of the CLI — minutes of latency per file
   change, and a hard failure whenever the underlying soldr/zccache
   daemon flaked (which is the very class of bug we're hitting in
   zccache#774 / zccache#784).

   Fix: add `--no-project` to every hook command. The hooks all use
   PEP 723 inline script metadata, so they don't actually need the
   project to be installed — uv was syncing it as a side-effect of
   project-discovery.

2. **`lint.py` ran fbuild's `./lint` against files outside this repo.**
   When editing files in a sibling repo's worktree (e.g. a `.claude/`
   worktree under `~/dev/zccache/`), `detect_crate()` read the
   worktree's `crates/<x>/` path segment and ran
   `soldr cargo clippy -p <x>` against the fbuild workspace. Either
   that errored (no such package) or — when names happened to collide
   — it lint'd the wrong code.

   Fix: gate `lint.py` on `file_path.startswith(PROJECT_ROOT)` so files
   outside this repo are no-ops. And invoke `./lint` via
   `sys.executable` instead of `uv run --script` so it doesn't trigger
   a SECOND editable rebuild on top of the outer one.

Also: `.claude/tmp/` (scratch for hereoc'd issue bodies / PR descriptions
during Claude sessions) added to `.gitignore`.

No CI behavior change — `cargo fmt --all --check` + clippy still run via
`Stop` hook on workspace-level passes. Per-file Edit hooks now finish in
milliseconds instead of minutes.
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@zackees, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 1 second. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 18ea6e68-02d2-45e7-9360-c0165ee5db96

📥 Commits

Reviewing files that changed from the base of the PR and between d329db4 and fef4d89.

📒 Files selected for processing (1)
  • ci/hooks/lint.py
📝 Walkthrough

Walkthrough

All Claude lifecycle hook commands in .claude/settings.json gain the --no-project flag on their uv run --script invocations. ci/hooks/lint.py adds a project-root boundary check and switches its lint subprocess call from uv run --script to sys.executable. .claude/tmp/ is added to .gitignore.

Changes

Claude Hook Hardening

Layer / File(s) Summary
Add --no-project to all hook commands
.claude/settings.json, .gitignore
All five lifecycle hook command strings (UserPromptSubmit, PreToolUse, PostToolUse, SessionStart, Stop) are updated to use uv run --no-project --script .... .claude/tmp/ is added to .gitignore.
lint.py: project-root guard and direct subprocess invocation
ci/hooks/lint.py
Adds a normpath check that returns early if file_path is outside the project root, and replaces the uv run --script lint invocation with sys.executable to skip editable-build overhead.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 Hop, hop, no project flag in sight,
The hooks now run with --no-project right,
The lint guard checks the root with care,
No stray worktrees shall sneak in there,
And .claude/tmp/ is tucked away tight! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main objective: preventing full editable rebuilds triggered by Edit/Write hooks.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/hook-no-project-cross-repo

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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ci/hooks/lint.py`:
- Around line 57-59: The boundary check using startswith() on line 58 is
performed on a non-canonicalized file_path, which allows paths containing
relative components like ".." to bypass the check and reference files outside
the repository. Canonicalize the file_path variable (by resolving it to an
absolute path with all relative components resolved) before performing the
startswith() comparison against project_root_norm to ensure only paths that
genuinely exist within the project root pass validation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 76746520-3ea9-4c6e-a8b2-59fa7ac7db6b

📥 Commits

Reviewing files that changed from the base of the PR and between 3e09b88 and d329db4.

📒 Files selected for processing (3)
  • .claude/settings.json
  • .gitignore
  • ci/hooks/lint.py

Comment thread ci/hooks/lint.py Outdated
@zackees zackees merged commit ac187a5 into main Jun 20, 2026
84 of 90 checks passed
@zackees zackees deleted the fix/hook-no-project-cross-repo branch June 20, 2026 02:06
@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Jun 20, 2026
zackees added a commit to zackees/template-python-rust-cmd that referenced this pull request Jun 22, 2026
Default uv only watches pyproject.toml for cache invalidation, so an
editable install silently leaves a stale `_native.pyd` after .rs edits.
`uv run` then executes against pre-edit Rust code with no warning — a
particularly silent footgun in a maturin-backed project where the .pyd
is invisible to the developer.

Add `[tool.uv] cache-keys` globs that cover Cargo.toml, Cargo.lock,
rust-toolchain.toml, and crates/**/*.rs / crates/**/Cargo.toml. Now
`uv run` triggers re-sync on real source changes.

Correctness fix, not raw speed — but the fast iteration loop depends
on it. Ported from FastLED/fbuild#661.

Refs #2 (item 7).

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant