Repository navigation
Conversation
…pts, atomic pace-file writes
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds persistent nudge cooldown tracking, per-rule coaching cooldown serialization and enforcement, and a new PostToolFailure CLI hook that classifies failures and emits vent guidance. ChangesCooldown and failure hooks
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant HookHost
participant CLIHook
participant FailureHook
participant Vent
participant NudgePace
HookHost->>CLIHook: invoke PostToolFailure
CLIHook->>FailureHook: dispatch resolved agent
FailureHook->>FailureHook: parse and classify failure
FailureHook->>NudgePace: check and record cooldown
FailureHook->>Vent: build vent guidance context
FailureHook-->>HookHost: print hookSpecificOutput
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/nudge_pace.rs`:
- Around line 24-70: Protect the entire load–prune–insert–save sequence in
mark_fired with an exclusive file lock scoped to the nudge-pace state file,
reusing the existing RulesLock locking pattern from coaching/store.rs. Acquire
the lock before load() and release it only after save() completes, while
preserving the current best-effort behavior and pruning logic.
- Around line 60-70: Update mark_fired so timestamp pruning does not use the
hardcoded 86,400-second retention window; retain entries for at least the
maximum cooldown supported by configured coaching rules, including arbitrary
cooldown_secs values parsed by parse_rule_file and consumed by
rule_bodies_for_tool/rule_bodies_for_prompt. Preserve should_fire behavior for
both coaching rules and vent-nudge keys while avoiding premature removal during
unrelated mark_fired calls.
🪄 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 Plus
Run ID: fbfaaba0-26c1-42e7-b844-0820d2e6d07e
📒 Files selected for processing (8)
src/cli/hook.rssrc/coaching/mod.rssrc/coaching/rule.rssrc/coaching/store.rssrc/hook.rssrc/main.rssrc/nudge_pace.rssrc/vent/capture.rs
PostToolUseFailure was still registered as the old 15s prompt-type hook in wire_claude_code(), so the new classify()-based command hook this branch added was never reachable from a real settings.json. add_hook_entry now takes an optional matcher so the command-hook path can carry the same Bash|Edit|Write scoping the prompt hook had; add_prompt_hook_entry and the now-unused VENT_JUDGE_PROMPT are removed.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/init.rs (1)
857-863: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the matcher structurally.
This only checks for the command substring. Parse the settings JSON and assert the
PostToolUseFailureentry hasmatcher: "Bash|Edit|Write"so a regression cannot broaden failure-hook execution to every tool.🤖 Prompt for 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. In `@src/init.rs` around lines 857 - 863, Update the test function wire_claude_code_wires_post_tool_use_failure_command_hook to parse settings.json as JSON and inspect the PostToolUseFailure hook entry structurally. Assert its matcher equals "Bash|Edit|Write" while retaining verification of the command hook marker.
🤖 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 `@src/init.rs`:
- Around line 236-270: Update the hook wiring around add_hook_entry for
PostToolUseFailure to detect and remove the known legacy prompt entry before
adding the deterministic hook, preventing both entries from running on upgraded
installs. Add an upgrade test that seeds the legacy PostToolUseFailure prompt
entry and verifies it is removed and replaced by the marked hook
post-tool-failure entry.
---
Nitpick comments:
In `@src/init.rs`:
- Around line 857-863: Update the test function
wire_claude_code_wires_post_tool_use_failure_command_hook to parse settings.json
as JSON and inspect the PostToolUseFailure hook entry structurally. Assert its
matcher equals "Bash|Edit|Write" while retaining verification of the command
hook marker.
🪄 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 Plus
Run ID: 3f633c0a-2324-4324-bfeb-3bc2d48b3d0e
📒 Files selected for processing (2)
src/init.rssrc/rule_text.rs
💤 Files with no reviewable changes (1)
- src/rule_text.rs
…cooldowns Two issues flagged by CodeRabbit on the initial post-tool-failure hook PR: - mark_fired's load-mutate-save wasn't serialized across processes, so two hook processes racing on it (even for different keys) could each load() before the other's save() lands, silently dropping a fired-timestamp. Added PaceLock, same create_new-sentinel pattern as coaching's RulesLock. - mark_fired pruned any entry older than a hardcoded 24h regardless of the key's own cooldown_secs, which coaching rules can set arbitrarily high via their '# Cooldown:' header. Raised the prune window to 30 days so it can't outrun a configured cooldown.
add_hook_entry only skips adding when its own marker is already present, so an install wired by an older agentflare version (still carrying the 'genuine FRICTION' prompt-type hook) would get the new command hook added beside it instead of replacing it -- both firing on every tool failure. remove_hook_entries_matching() strips the legacy entry before the new one is wired.
getappz
enabled auto-merge (squash)
July 29, 2026 04:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/nudge_pace.rs: a shared cooldown-based pacing primitive (should_fire/mark_fired, flat JSON state file, fail-open, atomic writes) so nudge text doesn't repeat in full on every matching call.cooldown_secsheader field;rule_bodies_for_tool/rule_bodies_for_promptare now paced through it (falling back to a 300s default). Enforced/MANDATORY rules are explicitly never paced.PostToolUseFailureconverts from a 15s LLM-prompt hook (a model call on every Bash/Edit/Write failure) to a deterministicagentflare hook post-tool-failurecommand hook, classifying via the existingvent::classify::classify()heuristic — no model call — and gated by the same pacing primitive.Fixes from the final whole-branch review
hookSpecificOutput.permissionDecisionReason, which per the hooks schema only applies toPreToolUse.PostToolUseFailure's only event-specific field isadditionalContext— the nudge was being silently dropped while the cooldown was still burned. Fixed and live-verified end-to-end (confirmed the nudge actually reaches context on a real failure).is_interrupt: true) are no longer classified as friction.nudge_pace's state-file write is now atomic (tmp file + rename), since it runs from short-lived hook processes that can be killed mid-write under their timeout.cooldown_secsis actually honored, not just the default.Test plan
cargo build --workspace --all-features— cleancargo clippy --workspace --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedantic— cleancargo fmt --check— cleancargo test --workspace— 893/893 passingPostToolUseFailurestdin payload field name (error), confirmed sub-second hook latency (vs. the old 15s LLM call), and confirmed theadditionalContextnudge genuinely surfaces to the model on a real tool failure.Summary by CodeRabbit