fix: cap session-start/subagent-start hook latency (#221)#271
Conversation
Two hook scripts blocked Claude Code's startup waiting on REST responses they didn't actually need: - `session-start` awaited a 5000ms POST and discarded the response when `AGENTMEMORY_INJECT_CONTEXT=false` (the default). Pure latency. - `subagent-start` had a `// fire and forget` comment but the code awaited a 2000ms POST. Pure latency. Under fan-out (Slack-bot orchestrators, multi-agent harnesses, fanned `claude -p` jobs) the awaited timeouts stack and feed back into the engine; the reporter hit a positive feedback loop that OOM-killed iii-engine. Fix: - `session-start` — fire-and-forget when `INJECT_CONTEXT=false`. Cap the inject path at 1500ms (down from 5000ms) so a slow server can't block the agent indefinitely when stdout is actually consumed. - `subagent-start` — actually fire-and-forget, matching the existing comment. Cap at 800ms. Verified live against a black-hole TCP listener (accepts, never replies): - session-start (no inject): 5.05s → 0.85s - session-start (inject): 5.05s → 1.55s - subagent-start: 2.05s → 0.87s Built artifacts in `plugin/scripts/` regenerated via `npx tsdown`. Closes #221.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR refactors session-start and subagent-start hooks to eliminate blocking waits on slow/unreachable REST endpoints when responses are discarded. Separate timeout constants are introduced, request setup is consolidated into reusable variables, and control flow splits into non-blocking telemetry paths (fire-and-forget with 800ms timeout) and awaited context-injection paths (1500ms timeout for session-start). ChangesHook Timeout and Fire-and-Forget Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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)
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: 1
🤖 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 `@src/hooks/subagent-start.ts`:
- Line 51: The code constructs timestamps inline for the request body; capture a
single timestamp once at the start of the subagent-start handler (declare const
timestamp = new Date().toISOString()) and reuse that variable wherever a
timestamp is needed (e.g., the timestamp property in the request body) instead
of calling new Date().toISOString() multiple times.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b9d3051-01f9-48bb-bb20-d24a840ba625
📒 Files selected for processing (4)
plugin/scripts/session-start.mjsplugin/scripts/subagent-start.mjssrc/hooks/session-start.tssrc/hooks/subagent-start.ts
| sessionId, | ||
| project: data.cwd || process.cwd(), | ||
| cwd: data.cwd || process.cwd(), | ||
| timestamp: new Date().toISOString(), |
There was a problem hiding this comment.
Capture timestamp once at the top of the function.
The timestamp is created inline within the request body. As per coding guidelines, capture timestamps once with new Date().toISOString() and reuse instead of calling Date multiple times.
📝 Proposed fix
const sessionId = (data.session_id as string) || "unknown";
+ const timestamp = new Date().toISOString();
fetch(`${REST_URL}/agentmemory/observe`, {
method: "POST",
headers: authHeaders(),
body: JSON.stringify({
hookType: "subagent_start",
sessionId,
project: data.cwd || process.cwd(),
cwd: data.cwd || process.cwd(),
- timestamp: new Date().toISOString(),
+ timestamp,
data: {
agent_id: data.agent_id,
agent_type: data.agent_type,
},
}),As per coding guidelines, "Capture timestamps once with new Date().toISOString() and reuse instead of calling Date multiple times".
📝 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.
| timestamp: new Date().toISOString(), | |
| const sessionId = (data.session_id as string) || "unknown"; | |
| const timestamp = new Date().toISOString(); | |
| fetch(`${REST_URL}/agentmemory/observe`, { | |
| method: "POST", | |
| headers: authHeaders(), | |
| body: JSON.stringify({ | |
| hookType: "subagent_start", | |
| sessionId, | |
| project: data.cwd || process.cwd(), | |
| cwd: data.cwd || process.cwd(), | |
| timestamp, | |
| data: { | |
| agent_id: data.agent_id, | |
| agent_type: data.agent_type, | |
| }, | |
| }), |
🤖 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 `@src/hooks/subagent-start.ts` at line 51, The code constructs timestamps
inline for the request body; capture a single timestamp once at the start of the
subagent-start handler (declare const timestamp = new Date().toISOString()) and
reuse that variable wherever a timestamp is needed (e.g., the timestamp property
in the request body) instead of calling new Date().toISOString() multiple times.
Three reliability fixes from #269/#270/#271: - search/recall surfaces saved memories (closes #265) - MCP shim proxies full server tool set (closes #234) - session/subagent hooks no longer block startup (closes #221) Also fixes packages/mcp version drift — was stuck at 0.9.4 through v0.9.5, now lockstepped with main.
|
Confirming this works in production after upgrading our plugin install to v0.9.9. Verified locally against a black-hole TCP listener (accepts, never replies): Matches the PR description. Thanks for the rapid turnaround — this closes the OOM-loop we hit on 2026-05-01 (originally filed as #221). |
2 participants
Summary
session-start.tsawaited a 5000ms POST and discarded the response wheneverAGENTMEMORY_INJECT_CONTEXT=false(the default since 0.8.10) — pure blocking latency on every Claude Code session start.subagent-start.tshad a// fire and forgetcomment but the code awaited a 2000ms POST.claude -pjobs) the awaited timeouts stack and create the positive feedback loop the reporter described — leading to iii-engine OOM-kill.Fix
session-start— fire-and-forget on the telemetry path. Cap the inject path at 1500ms (down from 5000ms) so a slow server can't block the agent indefinitely when stdout is consumed.subagent-start— actually fire-and-forget, matching the existing comment. Cap at 800ms.Live e2e (black-hole TCP listener: accepts, never replies)
session-start(no inject)session-start(inject=true)subagent-startNotes
plugin/scripts/regenerated vianpx tsdown.isSdkChildContextguard is intentionally inlined in each hook source (matches the established pattern: everytsdownhook entry compiles to a single self-contained.mjs).Closes #221.
Summary by CodeRabbit