fix: sync CLAUDE.md test counts and dependency description with codebase#43
fix: sync CLAUDE.md test counts and dependency description with codebase#43
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughConverted many hook denial messages to a structured "WHAT / FIX / REF" remediation format and adjusted worktree path/command suggestions; updated CLAUDE.md punctuation, test totals/breakdown, and noted Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Update test count to 231 (178 unit + 4 doc + 17 integration + 10 proptest + 22 memory). Mention rusqlite as memory crate dependency. Fix trailing period on H-4 purity bullet.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
CLAUDE.md (1)
233-234: Clarify dependency policy scope to avoid cross-crate ambiguity.Line 233 currently reads as a global dependency list before adding a memory exception. Consider explicitly scoping it to
muzzle-hooks(allowlist) vsmuzzle-memory(includesrusqlite) in one sentence so readers don’t misapply the “no network/proc-macro” constraint across crates.Based on learnings: the 5-dependency allowlist in this doc applies only to the hooks crate, while other workspace crates maintain separate dependency sets.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 233 - 234, Clarify that the five-dependency allowlist ("serde", "serde_json", "regex", "flate2", "libc") applies only to the muzzle-hooks crate: change the sentence so it explicitly states "muzzle-hooks allows these 5 crates" and then add a follow-up clause that the memory crate (muzzle-memory) additionally includes "rusqlite" and that other workspace crates may maintain separate dependency policies (i.e., the "no async runtime, no network deps, no proc macros" constraint is scoped to muzzle-hooks, not globally).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@CLAUDE.md`:
- Around line 233-234: Clarify that the five-dependency allowlist ("serde",
"serde_json", "regex", "flate2", "libc") applies only to the muzzle-hooks crate:
change the sentence so it explicitly states "muzzle-hooks allows these 5 crates"
and then add a follow-up clause that the memory crate (muzzle-memory)
additionally includes "rusqlite" and that other workspace crates may maintain
separate dependency policies (i.e., the "no async runtime, no network deps, no
proc macros" constraint is scoped to muzzle-hooks, not globally).
…ective 22) Restructure all hook denial messages to follow the harness engineering WHAT/FIX/REF format so the agent can self-repair without human intervention: - lib.rs: WORKTREE_MISSING now includes WHAT/FIX/REF - sandbox.rs: REDIRECT and fallback BLOCKED messages use WHAT/FIX/REF - gitcheck.rs: all 8 git safety blocks + worktree enforcement use WHAT/FIX/REF with pointers to CLAUDE.md and docs/architecture.md Reference: harness.md Pillar 2c — custom linters with remediation messages.
Greptile SummaryThis PR is a documentation and error-message formatting clean-up. It corrects the test count in Confidence Score: 5/5Safe to merge; changes are documentation and error-message copy with no logic changes, and all relevant tests pass. The only substantive Rust change is reformatting string literals — no logic, regex, or API surface is altered. The WORKTREE_MISSING: sentinel prefix (used by integration tests) is preserved, and the REDIRECT: message relied on by sandbox unit tests is untouched. The single P2 comment (FR-GS-8 mentioning origin/main only) is a copy nit that does not affect functionality or test coverage. hooks/src/gitcheck.rs — FR-GS-8 message nit (origin/master variant not mentioned), otherwise no files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Agent issues Bash/Write tool call] --> B{permissions hook}
B --> C{git command?}
C -->|yes| D[check_git_safety\nFR-GS-1..8]
D -->|blocked| E[WHAT/FIX/REF denial\ne.g. force-push, push-to-main\nreset --hard origin/main]
D -->|ok| F[check_worktree_enforcement]
F -->|blocked| G[WHAT/FIX/REF denial\ne.g. bare checkout on main]
F -->|ok| H[Allow]
C -->|no - file write| I[check_path_with_context\nsandbox.rs]
I --> J{worktree active?}
J -->|yes - worktree path| K{persistent config?}
K -->|yes| L[REDIRECT: to main checkout\nunchanged message]
K -->|no| H
J -->|yes - main checkout path| M{worktree dir exists?}
M -->|no| N[WORKTREE_MISSING:repo\nWHAT/FIX/REF]
M -->|yes| O[WHAT/FIX/REF denial\nredirect to worktree path]
J -->|no worktrees| P[WHAT/FIX/REF denial\nno worktree for session]
E --> Q[Agent self-repairs\nusing FIX guidance]
G --> Q
N --> Q
O --> Q
P --> Q
Reviews (4): Last reviewed commit: "fix: apply rustfmt to sandbox remediatio..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ## Testing | ||
|
|
||
| 219 tests (166 unit + 5 doc + 13 integration + 10 proptest + 25 memory) plus 4 fuzz targets. | ||
| 231 tests (178 unit + 4 doc + 17 integration + 10 proptest + 22 memory) plus 4 fuzz targets. |
There was a problem hiding this comment.
cargo test output
The PR description itself states "cargo test — all 246 tests pass", yet the updated line documents 231. That's a gap of 15 unaccounted tests (246 − 231). The five tests in hooks/tests/claude_md.rs only verify binary count, architecture-tree completeness, dependency count/names, and Makefile targets — none of them assert this total, so the mismatch won't be caught automatically.
If the intention is to keep the breakdown as the authoritative count (e.g., the claude_md integration tests are intentionally excluded), it would be worth a brief clarifying note, or at least reconciling the number so readers aren't confused by the delta.
There was a problem hiding this comment.
Good catch. The 231 count was wrong — it missed the claude_md integration tests (5), memory integration tests (4), and a separate proptest binary (10). Fixed to 246 with the full breakdown: 188 hooks unit + 5 claude_md + 17 integration + 10 proptest + 22 memory unit + 4 memory integration.
Greptile correctly identified the count (231) didn't match the actual cargo test output (246). Missing: claude_md (5), memory integration (4), separate proptest binary (10). Correct: 188 hooks unit + 5 claude_md + 17 integration + 10 proptest + 22 memory unit + 4 memory integration = 246.
frits-v
force-pushed
the
fix/claude-md-sync
branch
from
4e92acd to
e7eca24
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/src/gitcheck.rs`:
- Around line 162-164: Update the FR-GS-8 message string in hooks::gitcheck (the
literal that currently mentions "origin/main") to be branch-agnostic so it
matches the blocked patterns (both origin/main and origin/master); e.g., change
the specific "origin/main" text to a generic "origin/main or origin/master" or
simply "origin/<main|master>" and keep the suggested fixes (git stash / git
reset --soft) and reference intact so the message accurately reflects the rule.
- Around line 84-165: The denial message literals in hooks/src/gitcheck.rs (the
GitResult::Block returns for checks like RE_PUSH_TO_MAIN, RE_REFSPEC_MAIN,
RE_DELETE_MAIN, RE_DELETE_REFSPEC, RE_NO_VERIFY, RE_FOLLOW_TAGS,
RE_DELETE_SEMVER_TAG, RE_DELETE_REMOTE_TAG, and RE_HARD_RESET) are not
rustfmt-compliant; run cargo fmt (or rustfmt) to reformat the updated strings
and commit the formatted file so the file passes `cargo fmt -- --check`; if
multiline concatenation/escaping prevents rustfmt from producing stable output,
rewrite the message literals in a rustfmt-friendly form (single-line strings or
raw strings) inside those GitResult::Block(...) calls before formatting.
In `@hooks/src/sandbox.rs`:
- Around line 166-173: The denial message currently uses a multi-line string
literal that violates rustfmt; replace the multiline literal with a single
formatted string expression so fmt can pass: build the message with format!(...)
when constructing PathDecision::Deny and interpolate wt_path (which you already
compute with wt_path = format!(...)) into that format string (e.g.
PathDecision::Deny(format!("WHAT: ... FIX: Use the worktree path instead: {}",
wt_path))). Ensure the parentheses and indentation match rustfmt style; apply
the same fix for the similar block around lines 198-200.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0430a96e-23d0-4432-9d82-145f7b3ac474
📒 Files selected for processing (4)
CLAUDE.mdhooks/src/gitcheck.rshooks/src/lib.rshooks/src/sandbox.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- CLAUDE.md
| "WHAT: git reset --hard origin/main discards all local work. \ | ||
| FIX: Use `git stash` or `git reset --soft` instead. \ | ||
| REF: CLAUDE.md#key-design-decisions".into(), |
There was a problem hiding this comment.
Make FR-GS-8 message match the actual blocked patterns.
Line 162 currently says origin/main, but the rule blocks both origin/main and origin/master. The text should be branch-agnostic to avoid misleading remediation.
💡 Suggested wording tweak
- "WHAT: git reset --hard origin/main discards all local work. \
+ "WHAT: git reset --hard origin/(main|master) discards all local work. \
FIX: Use `git stash` or `git reset --soft` instead. \
REF: CLAUDE.md#key-design-decisions".into(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "WHAT: git reset --hard origin/main discards all local work. \ | |
| FIX: Use `git stash` or `git reset --soft` instead. \ | |
| REF: CLAUDE.md#key-design-decisions".into(), | |
| "WHAT: git reset --hard origin/(main|master) discards all local work. \ | |
| FIX: Use `git stash` or `git reset --soft` instead. \ | |
| REF: CLAUDE.md#key-design-decisions".into(), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/src/gitcheck.rs` around lines 162 - 164, Update the FR-GS-8 message
string in hooks::gitcheck (the literal that currently mentions "origin/main") to
be branch-agnostic so it matches the blocked patterns (both origin/main and
origin/master); e.g., change the specific "origin/main" text to a generic
"origin/main or origin/master" or simply "origin/<main|master>" and keep the
suggested fixes (git stash / git reset --soft) and reference intact so the
message accurately reflects the rule.
|
Superseded by #44 which includes these changes |
1 participant
Summary
rusqliteas memory crate dependency in Dependencies sectionAll changes verified by
cargo test --test claude_md(5/5 passing).Test plan
cargo test --test claude_md— all 5 assertions passcargo test— all 246 tests passcargo clippy— no warnings🤖 Generated with Claude Code