Add post-tool and session-end hooks for Codex/Hermes/OpenCode#230
Add post-tool and session-end hooks for Codex/Hermes/OpenCode#230Gradata wants to merge 3 commits into
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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.
|
Caution Review failedPull request was closed or merged during review 📝 Walkthrough
WalkthroughAdds a generic hook-command builder and exported post-tool/session-end helpers; updates Codex, Hermes, OpenCode, and Claude‑Code adapters to idempotently install and uninstall hooks across pre-tool, post-tool, and session-end phases; expands tests to cover the new hook placements and uninstall behavior. ChangesThree-Phase Hook Installation Across All Adapters
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 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)
⚔️ Resolve merge conflicts
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 |
![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 `@Gradata/src/gradata/hooks/adapters/_base.py`:
- Around line 132-136: The hook_command_for_module function currently embeds
brain_dir verbatim which can be relative; change it to serialize an
absolute/canonical path (e.g., call brain_dir.resolve() or convert to an
absolute string before quoting) so the generated BRAIN_DIR is always an absolute
path; update hook_command_for_module to use the resolved/absolute Path when
building the f-string passed to shlex.quote(sys.executable) and BRAIN_DIR.
In `@Gradata/src/gradata/hooks/adapters/hermes.py`:
- Around line 166-181: The migration currently skips writing the config when no
new hook is appended because it only checks `added`; update the logic around the
loop that calls `_migrate_legacy_event(hooks, legacy_key, current_key)` to
detect whether any legacy->current migrations occurred (e.g., set a boolean flag
like `migrated = True` when `_migrate_legacy_event` moves entries or compare
pre/post state of `hooks`), and if either `added` is non-empty or `migrated` is
true, call `atomic_write_text(agent_config_path, _dump_simple_yaml(data))` and
return the appropriate InstallResult; this ensures migrations performed by
`_migrate_legacy_event` are persisted even when no new hook (`sig`) was
appended.
🪄 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: 4f364644-089b-4897-914c-1ac13562952d
📒 Files selected for processing (5)
Gradata/src/gradata/hooks/adapters/_base.pyGradata/src/gradata/hooks/adapters/codex.pyGradata/src/gradata/hooks/adapters/hermes.pyGradata/src/gradata/hooks/adapters/opencode.pyGradata/tests/test_hook_adapters.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 macos-latest / py3.12
- GitHub Check: pytest ubuntu-latest / py3.12
- GitHub Check: pytest macos-latest / py3.11
- GitHub Check: pytest windows-latest / py3.12
- GitHub Check: pytest (py3.11)
- GitHub Check: pytest (py3.12)
- GitHub Check: pytest windows-latest / py3.11
- GitHub Check: pytest ubuntu-latest / py3.11
🧰 Additional context used
📓 Path-based instructions (2)
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_hook_adapters.py
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/opencode.pyGradata/src/gradata/hooks/adapters/hermes.pyGradata/src/gradata/hooks/adapters/codex.py
🔇 Additional comments (4)
Gradata/tests/test_hook_adapters.py (1)
55-68: LGTM!Also applies to: 70-105, 107-123
Gradata/src/gradata/hooks/adapters/codex.py (1)
74-91: LGTM!Also applies to: 100-123, 128-188
Gradata/src/gradata/hooks/adapters/hermes.py (1)
245-295: LGTM!Gradata/src/gradata/hooks/adapters/opencode.py (1)
49-78: LGTM!Also applies to: 83-120
The 'gradata install --agent claude-code' was only installing the PreToolUse hook for inject_brain_rules. It was missing the PostToolUse hook for auto_correct (capture corrections from Edit|Write) and the Stop hook for session_close (graduation sweep). This means new users following the README got rule injection but no correction capture — the core value proposition was broken at install. _fix: - Added post_tool_hook_command + session_end_hook_command imports - Extracted _HOOK_ENTRIES list (event, matcher, cmd_fn) - install() now writes all 3 hooks - uninstall() scans all events (not just PreToolUse) - Idempotent: signature check across all events Blocks: GRA-1233 (critical), GRA-1215 (README publication), HN launch
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.
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 `@Gradata/src/gradata/hooks/adapters/claude_code.py`:
- Line 81: The current signature match uses a substring search on the entire
entry string ("if sig in str(existing)" and the analogous check at uninstall)
which can hit unrelated fields; update both places to inspect the hook id
directly by retrieving entry.get("hooks", [{}])[0].get("id") (or iterate hooks
if multiple) and compare that value to sig, then proceed with install/uninstall
logic using that equality check to avoid false positives.
- Around line 78-84: The current install() logic iterates over _HOOK_ENTRIES and
returns InstallResult("already_present") as soon as the signature is found in
any event, causing all-or-nothing idempotency and preventing repair of partially
missing hooks; change this to check each (_event, _matcher, _cmd_fn, _desc) in
_HOOK_ENTRIES against hooks.get(_event, []) and only skip installation for that
specific event when the signature exists, otherwise proceed to add the missing
hook entries for that event, and only return a final InstallResult summarizing
which hooks were added (or none) instead of early-returning when any single
match is found (references: _HOOK_ENTRIES, hooks, InstallResult, AGENT,
agent_config_path).
🪄 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: a4ae7df0-a64d-44f7-bcfc-1fdf31da499d
📒 Files selected for processing (1)
Gradata/src/gradata/hooks/adapters/claude_code.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
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/claude_code.py
🔇 Additional comments (5)
Gradata/src/gradata/hooks/adapters/claude_code.py (5)
15-15: LGTM!Also applies to: 17-17
57-63: LGTM!
132-159: LGTM!
101-101: ⚡ Quick winUpdate:
write_json()already performs atomic JSON writes viaatomic_write_text()
write_json()delegates toatomic_write_text(), which writes to a sibling temp file (with a pid-based.tmpsuffix), flushes+fsyncs the file, then usesos.replace()to atomically replace the target and fsyncs the parent directory—meeting the atomic-write guideline.
62-62: ⚡ Quick winConfirm Claude Code event name: using
"Stop"forsession_closeis correct.
Gradata/src/gradata/hooks/adapters/claude_code.pyregisters the session-end hook under event"Stop", andGradata/src/gradata/hooks/session_close.pyexplicitly setsHOOK_META["event"] = "Stop"and documents that it fires on every Stop (end of every turn) since Claude Code lacks a real session-end signal; the hook gates the expensive work accordingly.
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.
1 participant
Closes GRA-1163 and GRA-1233.
Summary
Verification