Skip to content

feat(install): inject CMM code-discovery reminder into Claude subagents#632

Merged
DeusData merged 3 commits into
DeusData:mainfrom
halindrome:feat/subagent-cmm-startup-hook
Jun 29, 2026
Merged

feat(install): inject CMM code-discovery reminder into Claude subagents#632
DeusData merged 3 commits into
DeusData:mainfrom
halindrome:feat/subagent-cmm-startup-hook

Conversation

@halindrome

Copy link
Copy Markdown
Contributor

Closes #631

What

Subagents spawned via the Agent tool do not fire SessionStart, so the existing SessionStart reminder never reaches them — they start without the codebase-memory-mcp code-discovery guidance and fall back to grep/file-read.

This adds a Claude Code SubagentStart hook (matcher *) that injects a leaner variant of the protocol into every subagent via hookSpecificOutput.additionalContext, omitting the index_repository step (the parent session has already indexed the project).

How

  • cbm_install_subagent_reminder_script() generates cbm-subagent-reminder, which emits a static JSON literal. SubagentStart injects context only via JSON (unlike SessionStart, which takes plain stdout), and because the text is fixed there is no runtime escaping and no python3/jq dependency.
  • cbm_upsert_claude_subagent_hooks() / cbm_remove_claude_subagent_hooks() register/remove the SubagentStart entry through the existing idempotent upsert_hooks_json/remove_hooks_json helpers. Wired into install_claude_code_config() (incl. --plan) and the Claude uninstall path, alongside the SessionStart hook.

Why Claude-only

SubagentStart is Claude-Code-specific (added in Claude Code 2.0.43, command-type hooks only). It lives in the Claude installer path; Codex/Gemini/etc. have no equivalent subagent-start event and differ in subagent model. The guidance text is tool-neutral, so this keeps the agent-agnostic boundary intact — only the hook plumbing is Claude-specific (mirrors the existing Codex-vs-JSON-agent split). Older Claude Code ignores the unknown event key (fail-open); no version gate needed in code.

Testing

  • scripts/test.sh: full unit suite green (5684 passed), including a new cli_claude_subagent_hook test covering install shape, idempotent re-install, and clean removal.
  • scripts/lint.sh (clang-format) clean.
  • Verified the generated script emits valid JSON: parses, hookEventName = SubagentStart, contains search_graph, omits index_repository.
  • Tested against a real indexed project.

Notes for reviewers

  • One logical change (sibling to the SessionStart reminder), well under the 500-line guideline.
  • The reminder text intentionally drops the index step for subagents; index staleness is still surfaced as an advisory on CMM query results.

🤖 Generated with Claude Code

halindrome pushed a commit to halindrome/codebase-memory-mcp that referenced this pull request Jun 25, 2026
QA round 1 (PR DeusData#632) flagged that the SubagentStart hook uses matcher "*",
the same matcher a user is most likely to pick for their own catch-all
SubagentStart hook. Because is_cmm_hook_entry keyed ownership on the matcher
string alone, install would remove the user's "*" entry and replace it with
ours, and uninstall could remove the wrong one.

Add an optional `match_command_substr` to the upsert/remove args, threaded into
is_cmm_hook_entry: when set, an entry must ALSO carry a hooks[].command
containing that substring to be claimed as ours. The Claude SubagentStart hook
passes "cbm-subagent-reminder"; all existing callers leave it NULL, preserving
their matcher-only behavior unchanged. Install/uninstall now only ever touch
CMM's own entry and never clobber a foreign "*" hook.

Add cli_claude_subagent_hook_preserves_user_entry covering the case: a
pre-existing user "*" SubagentStart hook survives both install and uninstall.

Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qc6k8xVZekbbT7WkkWjNog
@halindrome

Copy link
Copy Markdown
Contributor Author

QA Round 1

Reviewer: Claude (pr-qa-reviewer, fresh session). Contract source: issue #631. Branch synced with base (upstream/main); no advance. Schema change: none. SAST: code scanning / Dependabot not enabled on this repo — skipped, non-blocking.

Contract Verification

Criterion Verdict Evidence
SubagentStart hook registered (matcher *) at install pass cbm_upsert_claude_subagent_hookshook_event="SubagentStart", matcher_str="*"; wired into install_claude_code_config.
Inject via hookSpecificOutput.additionalContext (JSON, not plain stdout) pass Generated cbm-subagent-reminder emits {"hookSpecificOutput":{"hookEventName":"SubagentStart","additionalContext":"…"}}; verified parses as JSON.
Leaner SessionStart variant; omits index_repository pass Emitted text has search_graph/trace_path/… and omits index_repository.
Static JSON literal — no runtime escaping, no python3/jq pass fprintf of a fixed string in a single-quoted heredoc; no interpolation, no external binary.
Claude installer path only; Codex/Gemini untouched; tool-neutral text pass Both calls inside install_claude_code_config + Claude uninstall path; Gemini path untouched.
Idempotent registration; uninstall removes it pass upsert_hooks_json removes the prior matching entry before appending; remove_hooks_json prunes the empty event key.
Advisory/non-blocking; older Claude ignores unknown key; no version gate pass No version gate; advisory. SubagentStart added in Claude Code 2.0.43.
Unit coverage: install shape, idempotency, removal pass cli_claude_subagent_hook asserts entry shape, idempotent count==1, removal.

Findings

Finding 1 — "*" matcher ownership collision (regression, minor, hypothetical) — FIXED
is_cmm_hook_entry keyed ownership on the matcher string alone. Since the hook uses matcher "*" — the matcher a user is most likely to pick for their own catch-all SubagentStart hook — install could remove/replace the user's entry and uninstall could remove the wrong one.
Resolution (commit fix(install): address QA round 1): added an optional match_command_substr to the upsert/remove args, threaded into is_cmm_hook_entry; when set, an entry must also carry a hooks[].command containing that substring to be claimed as ours. The SubagentStart hook passes "cbm-subagent-reminder"; all existing callers pass NULL (matcher-only behavior unchanged). New test cli_claude_subagent_hook_preserves_user_entry verifies a pre-existing user "*" SubagentStart hook survives both install and uninstall.

Finding 2 — no test on the generated script's JSON output (observation, minor) — accepted
The test covers settings.json registration but does not run the generated script to validate its JSON. Coverage gap, not a defect (JSON verified valid here and by the author). Left as-is to match the sibling SessionStart generator, which likewise has no output test; the three contract-mandated targets (install shape, idempotency, removal) are covered.

Summary

Severity Count Status
Critical 0
Major 0
Minor 2 1 fixed (Finding 1), 1 accepted (Finding 2)

No blocking defects. All acceptance criteria pass. Test suite: 5685 passed, including both subagent tests. scripts/lint.sh (clang-format) clean.


SAST review skipped

GitHub code scanning is not available / not enabled on this repo, and Dependabot alerts are unavailable (not enabled or token lacks security_events scope). No PR-scoped security delta to review. Non-blocking.


QA performed by Claude Code (claude-opus-4-8)

@halindrome

Copy link
Copy Markdown
Contributor Author

QA Round 2 — clean

Reviewer: Claude (pr-qa-reviewer, fresh session, independent re-review of the full diff). Contract source: issue #631. Branch synced with base (upstream/main, no advance). Schema change: none. SAST: code scanning not enabled — skipped, non-blocking.

Round 2 focused on verifying the round-1 fix (match_command_substr ownership hardening) and re-checking the whole diff.

Result: 0 blocking findings

The round-1 fix is correct and introduces no regressions:

  • NULL-path equivalence for existing callersis_cmm_hook_entry computes matcher_ok exactly as before and returns early when require_command_substr is NULL. All non-subagent callers (PreToolUse Grep|Glob, SessionStart startup/resume/clear/compact, Gemini BeforeTool) leave the field NULL → byte-equivalent behavior. Confirmed the helper has exactly two callers, both threaded correctly.
  • Command-aware path null-safe — missing/non-array hooks → not ours; missing/non-string command skipped; null guarded before strstr.
  • Idempotency preserved — re-install replaces, count == 1.
  • User-entry preservation + no incorrect key pruning — with a user "*" entry plus CMM's, uninstall removes only CMM's (matched by command substring); the SubagentStart key survives because the array is non-empty. cli_claude_subagent_hook_preserves_user_entry exercises both install and uninstall with meaningful assertions.
  • JSON output — single-line static literal via quoted heredoc; parses valid; index_repository absent (leaner variant); no python3/jq.

Contract Verification

Criterion Verdict
SubagentStart hook registered, matcher *, at install pass
Injects via hookSpecificOutput.additionalContext (JSON) pass
Leaner variant; omits index_repository pass
Static JSON literal; no runtime escaping / python3 / jq pass
Claude installer path only; Codex/Gemini unaffected; tool-neutral pass
Idempotent registration; uninstall removes it pass
Advisory/non-blocking; no version gate pass
Unit tests: install shape, idempotency, removal (+ user-entry preservation) pass

Pre-existing issue (non-blocking, NOT this PR)

cli_hook_gate_script_no_predictable_tmp_issue384 fails in a dev shell where CLAUDE_CONFIG_DIR is exported: it expects the gate script under tmpdir/.claude/hooks/…, but cbm_claude_config_dir honors $CLAUDE_CONFIG_DIR and redirects the write, so the read returns NULL. The PR does not touch cbm_install_hook_gate_script, cbm_claude_config_dir, or this test — it fails identically on upstream/main in the same shell. With CLAUDE_CONFIG_DIR unset the full suite is green (5685 passed).

Schema Change

None — diff touches only src/cli/cli.c, src/cli/cli.h, tests/test_cli.c.

Summary

Severity Count
Critical 0
Major 0
Minor (blocking) 0
Minor (observation, pre-existing) 1

Clean round. All acceptance criteria pass; round-1 fix verified. No fix commit this round.


QA performed by Claude Code (claude-opus-4-8)

@halindrome halindrome marked this pull request as ready for review June 25, 2026 17:57
@DeusData

Copy link
Copy Markdown
Owner

Hey, can you rebase on latest main? Otherwise LGTM

Subagents spawned via the Agent tool do not fire SessionStart, so the
existing SessionStart reminder never reaches them — they start without the
codebase-memory-mcp code-discovery guidance and fall back to grep/file-read.

Register a Claude Code SubagentStart hook (matcher "*") that injects a leaner
variant of the protocol via JSON additionalContext, omitting the
index_repository step since the parent session has already indexed the
project. SubagentStart injects context only via a JSON object on stdout, not
plain text, so the generated script emits a static JSON literal — no runtime
escaping and no python3/jq dependency. Advisory only; installed and removed
alongside the SessionStart hook in install_claude_code_config.

Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>
Verify the Claude SubagentStart reminder: install writes a SubagentStart
entry with a match-all matcher pointing at cbm-subagent-reminder, a second
upsert is idempotent (no duplicate entry), and removal leaves no SubagentStart
key behind.

Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>
QA round 1 (PR DeusData#632) flagged that the SubagentStart hook uses matcher "*",
the same matcher a user is most likely to pick for their own catch-all
SubagentStart hook. Because is_cmm_hook_entry keyed ownership on the matcher
string alone, install would remove the user's "*" entry and replace it with
ours, and uninstall could remove the wrong one.

Add an optional `match_command_substr` to the upsert/remove args, threaded into
is_cmm_hook_entry: when set, an entry must ALSO carry a hooks[].command
containing that substring to be claimed as ours. The Claude SubagentStart hook
passes "cbm-subagent-reminder"; all existing callers leave it NULL, preserving
their matcher-only behavior unchanged. Install/uninstall now only ever touch
CMM's own entry and never clobber a foreign "*" hook.

Add cli_claude_subagent_hook_preserves_user_entry covering the case: a
pre-existing user "*" SubagentStart hook survives both install and uninstall.

Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>
halindrome pushed a commit to halindrome/codebase-memory-mcp that referenced this pull request Jun 29, 2026
QA round 1 (PR DeusData#632) flagged that the SubagentStart hook uses matcher "*",
the same matcher a user is most likely to pick for their own catch-all
SubagentStart hook. Because is_cmm_hook_entry keyed ownership on the matcher
string alone, install would remove the user's "*" entry and replace it with
ours, and uninstall could remove the wrong one.

Add an optional `match_command_substr` to the upsert/remove args, threaded into
is_cmm_hook_entry: when set, an entry must ALSO carry a hooks[].command
containing that substring to be claimed as ours. The Claude SubagentStart hook
passes "cbm-subagent-reminder"; all existing callers leave it NULL, preserving
their matcher-only behavior unchanged. Install/uninstall now only ever touch
CMM's own entry and never clobber a foreign "*" hook.

Add cli_claude_subagent_hook_preserves_user_entry covering the case: a
pre-existing user "*" SubagentStart hook survives both install and uninstall.

Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qc6k8xVZekbbT7WkkWjNog
@halindrome halindrome force-pushed the feat/subagent-cmm-startup-hook branch from 53d9a43 to 682722e Compare June 29, 2026 14:18
@halindrome halindrome requested a review from DeusData as a code owner June 29, 2026 14:18
@DeusData

Copy link
Copy Markdown
Owner

Sorry @halindrome — I said "rebase, otherwise LGTM," but one extra heads-up so the rebase actually goes green: dco is red right now even though your commits are signed off.

The cause is a blank line between Signed-off-by: and the trailing Co-Authored-By: … / Claude-Session: lines — git's trailer parser only treats that final contiguous block as trailers, so it never sees the sign-off (git log --format='%(trailers:key=Signed-off-by,valueonly)' comes back empty on all three commits).

Cleanest fix while you rebase: drop the Co-Authored-By: Claude … / Claude-Session: lines (we keep commit history AI-attribution-free anyway) so Signed-off-by is the final trailer → DCO clears. The code itself is good to go — thanks for the careful work, and especially the cli_claude_subagent_hook_preserves_user_entry test that keeps us from clobbering a user's own * hook. 🙏

@DeusData

Copy link
Copy Markdown
Owner

Deeper review pass on top of the LGTM — merge-ready, no blockers. The generated cbm-subagent-reminder script is fully static and written only under the Claude config dir; the require_command_substr ownership change correctly preserves a user's own "*" hook on both install and uninstall (and cli_claude_subagent_hook_preserves_user_entry genuinely exercises that path — it'd fail under matcher-only ownership); the emitted JSON validates; and SubagentStart is confirmed a real, executed hook event. Two optional, non-blocking notes:

  • Test gapcbm_install_subagent_reminder_script (the function that writes the heredoc JSON) isn't covered; both new tests exercise only the settings.json upsert/remove. Since the whole feature hinges on that JSON literal being well-formed, a small test that runs the installer into a tmp HOME and asserts the written script parses as JSON with hookEventName == "SubagentStart" would guard against a future typo in it.
  • One empirical check (belt-and-suspenders for the "no non-functional element" principle) — worth confirming once that the additionalContext actually reaches a spawned subagent's context (install → spawn a Task subagent → see the guidance appears). The evidence says it works via the SessionStart-parity path; just a nice live confirmation since the binary's output-validation schema doesn't explicitly enumerate additionalContext for the *Start events.

Neither blocks merge. (The DCO/trailer note from my comment above is the only thing actually needed to get it green.) Thanks! 🙏

@halindrome halindrome force-pushed the feat/subagent-cmm-startup-hook branch from 682722e to 5255939 Compare June 29, 2026 20:32
@DeusData DeusData merged commit 63e3f2c into DeusData:main Jun 29, 2026
14 checks passed
@DeusData

Copy link
Copy Markdown
Owner

Merged — thanks @halindrome! 🙏 Really clean work, and I appreciate the fast turnaround dropping the trailers to clear DCO. The require_command_substr ownership refinement — and the cli_claude_subagent_hook_preserves_user_entry test that proves a user's own * hook survives both install and uninstall — is exactly the kind of care that makes a hook touching user settings safe to ship.

The two optional follow-ups from my review (a small test over the generated JSON, and a one-time confirm that the reminder actually lands in a spawned subagent's context) are nice-to-haves whenever you're back in this area — nothing urgent. Thanks again for the careful work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subagents don't receive the codebase-memory-mcp code-discovery reminder

3 participants