feat: add selective subagent memory context - #479
Conversation
📝 WalkthroughWalkthroughClaude Code and Codex now support ChangesSubagentStart context delivery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SubagentRuntime
participant ContextHook
participant NowledgeContext
SubagentRuntime->>ContextHook: SubagentStart payload
ContextHook->>NowledgeContext: Load bounded context for eligible role
NowledgeContext-->>ContextHook: Context or failure
ContextHook-->>SubagentRuntime: Routing guidance or additionalContext
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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 `@nowledge-mem-claude-code-plugin/tests/test_nmem_hook_subagent.py`:
- Around line 31-51: Isolate the default-policy tests, including
test_selected_subagent_injects_bounded_context_and_boundary and the referenced
tests, from the process environment by removing NMEM_SUBAGENT_CONTEXT_TYPES
before loading the module. Use a scoped fixture or per-test environment cleanup
so each test consistently exercises the module’s default context-type policy.
In `@nowledge-mem-codex-plugin/hooks/nmem-context.py`:
- Line 254: Update the SystemExit statement in the exception-handling flow of
nmem-context.py to explicitly suppress the active exception context by raising
it from None, preserving the existing fail-open exit status.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 098aff85-61a1-4a8c-9136-c5ef8f7c2c3f
📒 Files selected for processing (19)
.claude-plugin/marketplace.jsonintegrations.jsonnowledge-mem-claude-code-plugin/.claude-plugin/plugin.jsonnowledge-mem-claude-code-plugin/CHANGELOG.mdnowledge-mem-claude-code-plugin/README.mdnowledge-mem-claude-code-plugin/hooks/hooks.jsonnowledge-mem-claude-code-plugin/scripts/nmem-hook-subagent.pynowledge-mem-claude-code-plugin/tests/test_nmem_hook_subagent.pynowledge-mem-codex-plugin/.codex-plugin/plugin.jsonnowledge-mem-codex-plugin/AGENTS.mdnowledge-mem-codex-plugin/CHANGELOG.mdnowledge-mem-codex-plugin/README.mdnowledge-mem-codex-plugin/hooks/hooks.jsonnowledge-mem-codex-plugin/hooks/nmem-context.pynowledge-mem-codex-plugin/scripts/install_hooks.pynowledge-mem-codex-plugin/scripts/validate-plugin.mjsnowledge-mem-codex-plugin/tests/test_codex_plugin.pyshared/behavioral-guidance.mdtests/plugin_e2e/test_key_plugins_e2e.py
| def test_selected_subagent_injects_bounded_context_and_boundary(): | ||
| module = _load_module() | ||
| stdout = io.StringIO() | ||
| oversized_context = "context-内容\n" * 1000 | ||
|
|
||
| with mock.patch.object(module, "_load_context", return_value=oversized_context), \ | ||
| mock.patch.object(module.sys, "stdout", stdout): | ||
| assert module.main( | ||
| {"hook_event_name": "SubagentStart", "agent_type": "Plan"} | ||
| ) == 0 | ||
|
|
||
| output = json.loads(stdout.getvalue())["hookSpecificOutput"] | ||
| additional_context = output["additionalContext"] | ||
| assert output["hookEventName"] == "SubagentStart" | ||
| assert "isolated subagent context" in additional_context | ||
| assert "memory_search` / `thread_search" in additional_context | ||
| assert "nmem --json m search" in additional_context | ||
| assert "Do not distill speculative" in additional_context | ||
| assert "Current Nowledge context" in additional_context | ||
| assert "context truncated for subagent" in additional_context | ||
| assert len(additional_context.encode("utf-8")) <= module.SUBAGENT_CONTEXT_MAX_BYTES |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Isolate NMEM_SUBAGENT_CONTEXT_TYPES in tests that require defaults.
These tests read the real process environment. If the test runner sets NMEM_SUBAGENT_CONTEXT_TYPES, the default-policy assertions can fail or test a different policy.
Clear this variable for each default-policy test, or add an autouse fixture that removes it before loading the module.
Also applies to: 54-69, 71-87, 89-101, 103-120
🤖 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 `@nowledge-mem-claude-code-plugin/tests/test_nmem_hook_subagent.py` around
lines 31 - 51, Isolate the default-policy tests, including
test_selected_subagent_injects_bounded_context_and_boundary and the referenced
tests, from the process environment by removing NMEM_SUBAGENT_CONTEXT_TYPES
before loading the module. Use a scoped fixture or per-test environment cleanup
so each test consistently exercises the module’s default context-type policy.
| agent_type == "explorer" | ||
| and agent_type not in _subagent_context_types() | ||
| ): | ||
| raise SystemExit(0) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Suppress the handled exception before exiting.
Line 254 raises SystemExit while another exception is active. Add from None so the intentional fail-open exit does not retain an exception chain. This resolves Ruff B904.
Proposed fix
- raise SystemExit(0)
+ raise SystemExit(0) from None📝 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.
| raise SystemExit(0) | |
| raise SystemExit(0) from None |
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 254-254: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 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 `@nowledge-mem-codex-plugin/hooks/nmem-context.py` at line 254, Update the
SystemExit statement in the exception-handling flow of nmem-context.py to
explicitly suppress the active exception context by raising it from None,
preserving the existing fail-open exit status.
Source: Linters/SAST tools
|
Thanks @hawkingrei, merged. I kept the shape of the contribution and added a focused cleanup pass: subagent hook tests now isolate their env defaults, and the Codex hook fail-open path exits cleanly without noisy chained exceptions. The Claude/Codex targeted tests and Codex plugin validator are green. |
2 participants
Summary
SubagentStartbootstrap hooks for Claude Code and Codex.NMEM_SUBAGENT_CONTEXT_TYPESallowlist.Motivation
Non-fork subagents run with isolated context, so they cannot rely on the parent session's injected Context Bundle. Loading the complete bundle for every small exploration or implementation task would add latency, token cost, and attention noise. This change provides a role-aware bootstrap while keeping simple agents lightweight.
Implementation
Claude Code
Plan,code-reviewer,architect,researcher.Exploreis a no-op by default.Codex
planner,code-reviewer,architect,researcher.exploreris a no-op by default.defaultandworkerreceive retrieval routing without a context read.agent_typerole values and enables the packaged hook state during setup.Setting
NMEM_SUBAGENT_CONTEXT_TYPESreplaces the host default. An empty value disables full Context Bundle injection while retaining routing for non-explorer roles.User impact
Context-heavy planning, review, architecture, and research subagents start with bounded cross-tool context. Simple exploration and execution agents avoid unnecessary Mem reads while retaining an explicit path to search prior decisions when the task requires it.
Validation
git diff --checkvalidationRelated issues
N/A
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores