test(install): snapshot Claude Code lifecycle hooks#227
Conversation
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 Walkthrough
WalkthroughThe PR extends the Claude Code adapter to manage five coordinated hook lifecycles during installation and uninstallation. It adds command helper functions to the base adapter module, expands the Claude Code install/uninstall logic to handle multiple lifecycles, and provides comprehensive snapshot and acceptance tests to validate the implementation. ChangesClaude Code multi-lifecycle hook support
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.22.0)OpenGrep fatal error (exit code 2): �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Gradata/tests/test_install_claude_code_snapshot.py (1)
124-283: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd uninstall coverage for all five lifecycles.
This PR expands uninstall semantics materially; add a deterministic test that verifies full removal, user-hook preservation, and idempotent second uninstall.
Suggested test skeleton
+def test_uninstall_claude_code_removes_all_lifecycles_and_is_idempotent(tmp_path: Path) -> None: + brain = tmp_path / "brain" + brain.mkdir() + config_path = tmp_path / ".claude" / "settings.json" + config_path.parent.mkdir(parents=True) + config_path.write_text("{}", encoding="utf-8") + + adapter = get_adapter("claude-code") + install_result = adapter.install(brain, config_path) + assert install_result.action in ("added", "already_present") + + uninstall_result = adapter.uninstall(brain, config_path) + assert uninstall_result.action == "removed", uninstall_result.message + + settings = json.loads(config_path.read_text(encoding="utf-8")) + hooks = settings.get("hooks", {}) + for lifecycle in ("PreToolUse", "PostToolUse", "Stop", "PreCompact", "UserPromptSubmit"): + assert lifecycle not in hooks + + uninstall_again = adapter.uninstall(brain, config_path) + assert uninstall_again.action == "already_present"
🤖 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 `@Gradata/src/gradata/hooks/adapters/claude_code.py`:
- Around line 94-99: The PostToolUse hook currently uses matcher "Edit|Write"
which misses Claude's "MultiEdit" tool; update the matcher on the PostToolUse
hook entry (the dict containing "matcher", "hooks", the
auto_correct_command(brain_dir) call and "id": sig) to include "MultiEdit"
(e.g., "Edit|Write|MultiEdit") so auto_correct_command is invoked for MultiEdit
events as well.
In `@Gradata/tests/test_install_claude_code_snapshot.py`:
- Around line 98-109: The normalization currently hardcodes "/tmp" in the three
re.sub calls against the serialized variable which makes snapshots
platform-dependent; update those patterns to be platform-agnostic by
constructing the regex using the runtime temp directory (tempfile.gettempdir())
escaped with re.escape and use that escaped string in the patterns (or
alternately replace "/tmp" with a generic path-segment matcher like [^\" ]+
where appropriate) so the three places modifying serialized (the
BRAIN_DIR=/.../brain replacement, the Python executable normalization line, and
the hook signature ID replacement) work on non-Linux temp paths.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 21c2a075-7b09-47ea-b2f0-1039154eae79
📒 Files selected for processing (4)
Gradata/src/gradata/hooks/adapters/_base.pyGradata/src/gradata/hooks/adapters/claude_code.pyGradata/tests/snapshots/install_claude_code_settings.jsonGradata/tests/test_install_claude_code_snapshot.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: pytest (py3.12)
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest (py3.11)
- GitHub Check: pytest windows-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.11
- GitHub Check: pytest macos-latest / py3.12
🧰 Additional context used
📓 Path-based instructions (2)
Gradata/src/**/*.py
📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Gradata/src/**/*.py: Prefersentence-transformersfor local embeddings,google-genaifor Gemini embeddings,cryptographyfor AES-GCM encrypted system.db,bm25sfor BM25 rule ranking, andmem0aifor external memory adapters — guard all optional dependency imports withtry / except ImportErrorat the call site, never at module level
Maintain strict layering: Layer 0 (Primitives: _types.py, _db.py, _events.py, _paths.py, _file_lock.py; Patterns: contrib/patterns/) must never import from Layer 1 (Enhancements: enhancements/, rules/) or Layer 2 (Public API: brain.py, cli.py, daemon.py, mcp_server.py)
Never use bareexcept: pass— use typed exceptions or at minimumlogger.warning(...)withexc_info=Trueto avoid silent failure in a memory product
Never import from out-of-scope sibling directories../Sprites/or../Hausgem/withingradata/*code — that is a layering bug
Never leak private-sibling paths into public docs/code — no references to../Sprites/,../Hausgem/, email addresses, OneDrive paths, or Sprites-specific examples from insidegradata/*
Use atomic-write helper when writing JSON files to prevent corruption from mid-write crashes
Files:
Gradata/src/gradata/hooks/adapters/_base.pyGradata/src/gradata/hooks/adapters/claude_code.py
Gradata/tests/**/*.py
📄 CodeRabbit inference engine (Gradata/AGENTS.md)
Gradata/tests/**/*.py: SetBRAIN_DIRenvironment variable viatmp_pathin conftest.py for test isolation — ensure_paths.pymodule cache refreshes when callingBrain.init()directly inside tests
Add unit tests intests/test_*.pyfor every CI push without LLM calls (deterministic); mark integration tests with@pytest.mark.integrationand skip them by default (they hit real LLM APIs)
Files:
Gradata/tests/test_install_claude_code_snapshot.py
🔇 Additional comments (3)
Gradata/src/gradata/hooks/adapters/_base.py (1)
139-164: LGTM!Gradata/src/gradata/hooks/adapters/claude_code.py (1)
166-188: LGTM!Gradata/tests/snapshots/install_claude_code_settings.json (1)
1-62: LGTM!
| "matcher": "Edit|Write", | ||
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": auto_correct_command(brain_dir), | ||
| "id": sig, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Claude Code hooks matcher rules, does "Edit|Write" match only exact tool names (excluding "MultiEdit")?
💡 Result:
Yes. In Claude Code hook matcher rules for tool events (e.g., PreToolUse/PostToolUse), the matcher value is evaluated against the tool name. If the matcher contains only letters/digits/underscore and | (like "Edit|Write"), it is treated as an exact string match (or | -separated list of exact strings), not as a regex; so it matches exactly the tools named Edit and Write, and it would not match a tool named MultiEdit. [1][2]
Citations:
Fix PostToolUse matcher to include MultiEdit
PostToolUse is wired with matcher "Edit|Write", but Claude matcher evaluation is exact tool-name matching (| is a list of exact strings), so it won’t match the tool "MultiEdit". This can skip auto_correct for valid Claude MultiEdit events.
Proposed fix
- "matcher": "Edit|Write",
+ "matcher": "Edit|MultiEdit|Write",📝 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.
| "matcher": "Edit|Write", | |
| "hooks": [ | |
| { | |
| "type": "command", | |
| "command": auto_correct_command(brain_dir), | |
| "id": sig, | |
| "matcher": "Edit|MultiEdit|Write", | |
| "hooks": [ | |
| { | |
| "type": "command", | |
| "command": auto_correct_command(brain_dir), | |
| "id": sig, |
🤖 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 `@Gradata/src/gradata/hooks/adapters/claude_code.py` around lines 94 - 99, The
PostToolUse hook currently uses matcher "Edit|Write" which misses Claude's
"MultiEdit" tool; update the matcher on the PostToolUse hook entry (the dict
containing "matcher", "hooks", the auto_correct_command(brain_dir) call and
"id": sig) to include "MultiEdit" (e.g., "Edit|Write|MultiEdit") so
auto_correct_command is invoked for MultiEdit events as well.
| serialized = re.sub(r"BRAIN_DIR=/tmp/[^ ]+/brain", "BRAIN_DIR=__BRAIN_DIR__", serialized) | ||
| # Normalize: Python executable path differs across dev machines and CI. | ||
| serialized = re.sub( | ||
| r"BRAIN_DIR=__BRAIN_DIR__ [^\" ]+ -m gradata\.hooks\.", | ||
| "BRAIN_DIR=__BRAIN_DIR__ __PYTHON__ -m gradata.hooks.", | ||
| serialized, | ||
| ) | ||
| # Normalize: hook signature ID | ||
| serialized = re.sub( | ||
| r'"gradata:claude-code:/tmp/[^"]+brain"', | ||
| '"gradata:claude-code:__BRAIN_DIR__"', | ||
| serialized, |
There was a problem hiding this comment.
Make snapshot normalization path-agnostic (not /tmp-specific).
The regex currently assumes Linux-style temp paths; this will fail on other platforms and can produce flaky snapshot tests.
Proposed fix
- serialized = re.sub(r"BRAIN_DIR=/tmp/[^ ]+/brain", "BRAIN_DIR=__BRAIN_DIR__", serialized)
+ serialized = re.sub(
+ r"BRAIN_DIR=(?:'[^']*'|\"[^\"]*\"|[^ ]+)",
+ "BRAIN_DIR=__BRAIN_DIR__",
+ serialized,
+ )
@@
- serialized = re.sub(
- r'"gradata:claude-code:/tmp/[^"]+brain"',
- '"gradata:claude-code:__BRAIN_DIR__"',
- serialized,
- )
+ serialized = re.sub(
+ r'"gradata:claude-code:[^"]+"',
+ '"gradata:claude-code:__BRAIN_DIR__"',
+ serialized,
+ )📝 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.
| serialized = re.sub(r"BRAIN_DIR=/tmp/[^ ]+/brain", "BRAIN_DIR=__BRAIN_DIR__", serialized) | |
| # Normalize: Python executable path differs across dev machines and CI. | |
| serialized = re.sub( | |
| r"BRAIN_DIR=__BRAIN_DIR__ [^\" ]+ -m gradata\.hooks\.", | |
| "BRAIN_DIR=__BRAIN_DIR__ __PYTHON__ -m gradata.hooks.", | |
| serialized, | |
| ) | |
| # Normalize: hook signature ID | |
| serialized = re.sub( | |
| r'"gradata:claude-code:/tmp/[^"]+brain"', | |
| '"gradata:claude-code:__BRAIN_DIR__"', | |
| serialized, | |
| serialized = re.sub( | |
| r"BRAIN_DIR=(?:'[^']*'|\"[^\"]*\"|[^ ]+)", | |
| "BRAIN_DIR=__BRAIN_DIR__", | |
| serialized, | |
| ) | |
| # Normalize: Python executable path differs across dev machines and CI. | |
| serialized = re.sub( | |
| r"BRAIN_DIR=__BRAIN_DIR__ [^\" ]+ -m gradata\.hooks\.", | |
| "BRAIN_DIR=__BRAIN_DIR__ __PYTHON__ -m gradata.hooks.", | |
| serialized, | |
| ) | |
| # Normalize: hook signature ID | |
| serialized = re.sub( | |
| r'"gradata:claude-code:[^"]+"', | |
| '"gradata:claude-code:__BRAIN_DIR__"', | |
| serialized, | |
| ) |
🤖 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 `@Gradata/tests/test_install_claude_code_snapshot.py` around lines 98 - 109,
The normalization currently hardcodes "/tmp" in the three re.sub calls against
the serialized variable which makes snapshots platform-dependent; update those
patterns to be platform-agnostic by constructing the regex using the runtime
temp directory (tempfile.gettempdir()) escaped with re.escape and use that
escaped string in the patterns (or alternately replace "/tmp" with a generic
path-segment matcher like [^\" ]+ where appropriate) so the three places
modifying serialized (the BRAIN_DIR=/.../brain replacement, the Python
executable normalization line, and the hook signature ID replacement) work on
non-Linux temp paths.
Summary
.claude/settings.jsonoutput and idempotence coverageTest Plan
PYTHONPATH=src python3 -m pytest tests/test_install_claude_code_snapshot.py tests/test_hook_adapters.py -qCloses GRA-1211
Part of #206