Add Claude Code hook capture for tool and lifecycle events - #1067
Conversation
|
Added focused Go unit tests for the Claude Code hook helpers. The new tests cover default-on scrubbing, tool include/exclude precedence, bounded Validation:
|
wayyoungboy
left a comment
There was a problem hiding this comment.
Thanks for adding coverage for the newer Claude Code hook events. I found one blocking install-path regression and two data-flow/documentation issues. I cannot submit a formal request-changes review from this account on this PR, so leaving this as a review comment instead.
Findings:
- Blocking: marketplace/Git installs can lose
hooks/bin/powermem-hook-*because the root marketplace source still points at./apps/claude-code-plugin, while this PR removes the tracked binaries and ignoreshooks/bin/;hooks/run-hook.shthen exits successfully when the binary is missing. - Lifecycle metadata stores the entire scrubbed
raw_payloadby default for subagent/task events. Real lifecycle payloads can include final assistant text, task details, and session/background task fields, and this path is not bounded or allowlisted like the tool/precompact summaries. - The troubleshooting section still says writes come from
SessionEnd/PostCompact, but the default hook table now includes write traffic fromPreCompact, tool success/failure, and lifecycle hooks during the session. That under-discloses the new default data-flow/privacy surface.
Validation I ran:
go test ./...inapps/claude-code-pluginpython -m py_compile tests/regression/test_claude_hook_no_llm.pyafter activating the project virtual environmentPOWERMEM_HOOK_BIN=apps/claude-code-plugin/hooks/bin/powermem-hook-linux-amd64 python tests/regression/test_claude_hook_no_llm.pyafter activating the project virtual environmentbash apps/claude-code-plugin/scripts/build-hook-binaries.shgo test -count=1 ./...inapps/claude-code-pluginbash apps/claude-code-plugin/scripts/package-plugin.shand checked the zip contains the hook binariesgit diff --check
| @@ -1,2 +1,3 @@ | |||
| dist/ | |||
| *.zip | |||
| hooks/bin/ | |||
There was a problem hiding this comment.
Blocking: after ignoring and deleting hooks/bin, the marketplace install path still pulls the plugin from the Git source tree (.claude-plugin/marketplace.json uses ./apps/claude-code-plugin). A Git or marketplace install will not include generated binaries, and hooks/run-hook.sh exits successfully when hooks/bin/powermem-hook-* is absent. The release zip path works because the packaging script builds binaries, but marketplace installs get inert hooks unless the binaries are shipped another way or the hook builds/locates them at install/runtime.
| "schema_version": 1, | ||
| "scrub_mode": hookScrubEnabled(), | ||
| "infer_mode": infer, | ||
| "raw_payload": rawPayload, |
There was a problem hiding this comment.
This stores the entire scrubbed lifecycle payload in memory metadata by default. Real SubagentStop payloads include final assistant content and background task/session fields, and task payloads can contain detailed task descriptions. Unlike the bounded tool and precompact summaries, raw_payload is not bounded or allowlisted, so one lifecycle event can persist arbitrarily large or sensitive text. Please store only selected link fields and bounded summaries, or make raw payload capture opt-in.
|
Re-review status for the current head: the prior review findings appear addressed in the latest update.
Current checks are green. I do not see a new blocking issue from this static re-review. I am not marking this LGTM because this PR is authored from the |
|
Did a focused re-review of current head
Local validation I could run here:
Local validation I could not complete here:
I do not see a new blocker from the static pass, but I would still rely on CI or a maintainer environment with Go installed for the final Go/regression signal before merge. Generated with ax. |
|
Verified successfully in a Go environment. LGTM. |
…#1067) * Add Claude Code hook event capture * Add unit tests for Claude hook helpers * Add Claude hook session and failure events * Fix Claude hook lifecycle handoff privacy * fix: address claude hook review findings
Summary
SessionStart,PostToolUse,PostToolUseFailure,PreCompact,Stop, subagent, and task lifecycle events with bounded summaries and no-LLM regression coverageSessionStartsearch scrubbing,POWERMEM_HOOK_SECRET_ACTION=blockhandling for detached worker handoff, and parent-side scrub/block before lifecycle worker payload files are writtentask_subject,task_description,teammate_name, andteam_namehooks/bin/powermem-hook-*binaries committed so Git/marketplace installs and release/plugin zips include runnable native hooksraw_payloadmetadata and keep bounded, allowlisted lifecycle link fields plus numeric token usageFixes #1040
Fixes #1041
Fixes #1042
Fixes #1043
Fixes #1044
Fixes #1045
Testing
go test ./...fromapps/claude-code-plugingo test -count=1 ./...fromapps/claude-code-pluginpython -m py_compile tests/regression/test_claude_hook_no_llm.pyPOWERMEM_HOOK_BIN=apps/claude-code-plugin/hooks/bin/powermem-hook-linux-amd64 python tests/regression/test_claude_hook_no_llm.pybash apps/claude-code-plugin/scripts/build-hook-binaries.shbash apps/claude-code-plugin/scripts/package-plugin.shunzip -l apps/claude-code-plugin/dist/powermem-claude-code-plugin-0.1.0.zip | rg 'hooks/bin/powermem-hook'git archive --format=tar HEAD apps/claude-code-plugin | tar -tf - | rg 'apps/claude-code-plugin/hooks/bin/powermem-hook'git diff --check