fix: suppress stop notifications for subagent sessions#22
Conversation
Fetch the session on session.idle to check for parentID. If the session is a child (subagent) session, return early without sending a stop notification. Falls through on fetch failure so notifications are never silently dropped. Bump version to 0.1.7.
|
@harryalbert |
Can do! Feel free to tag me as a reviewer as well going forward (easier to track on my end) |
| const session = await client.session.get({ | ||
| path: { id: sessionId }, | ||
| }) | ||
| if (session.data?.parentID) return |
There was a problem hiding this comment.
Why did you only make this change for the session.idle event? Feels like we might want it for other events too (like prompt_submit, session_start, tool_complete, etc)
There was a problem hiding this comment.
@harryalbert
Good point. I originally scoped this to session.idle because that was the noisy notification I observed, but the same subagent semantics should apply across all session-scoped notifications.
I've updated the PR to extract a shared isSubagentSession() helper and apply it to every notification handler: session.created, session.idle, permission.updated, permission.replied, permission.asked, chat.message, tool.execute.before, and tool.execute.after.
One thing I considered but left out: each handler independently calls client.session.get() to check parentID. Since parentID is immutable for a session's lifetime, a simple Map<string, boolean> cache would eliminate redundant lookups when multiple events fire for the same session in quick succession (e.g., tool.execute.before → tool.execute.after → session.idle). I left it out for now since it's a minor optimization and the current approach is correct. Happy to add it if you'd prefer.
aqeelat
force-pushed
the
fix/subagent-idle-status
branch
2 times, most recently
from
f8cc4fb to
853cafe
Compare
harryalbert
left a comment
harryalbert
left a comment
There was a problem hiding this comment.
Thanks for iterating! Just had a few more thoughts
| async function isSubagentSession(sessionId?: string): Promise<boolean> { | ||
| if (!sessionId) return false | ||
| try { | ||
| const session = await client.session.get({ |
There was a problem hiding this comment.
you mentioned this in your comment, but I think this is worth caching if it doesn't change. Would rather avoid an async call for every hook (especially frequently activated hooks like tool.execute.after) if we can
There was a problem hiding this comment.
@harryalbert sorry i had already worked on it but forgot to push.
| switch (event.type) { | ||
| case "session.created": { | ||
| const sessionId = event.properties.info.id | ||
| if (await isSubagentSession(sessionId)) return |
There was a problem hiding this comment.
instead of adding this boundary at every hook (which feels brittle in case we add more hooks and forget to add the check for any of those new hooks), I'd rather this just be one unified gate that we check before surfacing a notification. We could define a wrapper around warpNotify (something like maybeWarpNotify, which would live inside index.ts) that checks the sub-agent predicate and, if it passes, internally calls warpNotify.
aqeelat
force-pushed
the
fix/subagent-idle-status
branch
from
853cafe to
5107303
Compare
aqeelat
force-pushed
the
fix/subagent-idle-status
branch
from
5107303 to
c935ffa
Compare
|
@harryalbert Both suggestions are addressed in the latest push I added a |
2 participants
What
Subagent sessions (e.g. from the Task tool) fire
session.idlewhen they complete, causing duplicatestopnotifications in Warp.How
On
session.idle, fetch the session viaclient.session.get()and checkparentID. If set, return early — nostopnotification is sent for child sessions.If the fetch fails, the notification is sent anyway to avoid silently dropping it.
Trade-off
This adds one API call (
session.get) persession.idleevent. Since idle fires once per session completion, the overhead is negligible.