feat: surface interrupted subagent sessions for recovery#392
feat: surface interrupted subagent sessions for recovery#392SweetSophia wants to merge 1 commit into
Conversation
Greptile SummaryThis PR adds a session resumption recovery mechanism for delegated subagent tasks: when a Confidence Score: 5/5Safe to merge; the single finding is a defensive ordering concern in All remaining findings are P2. The prior P1 concerns (agent name never populated, sweepStale never called) have been properly addressed with the src/hooks/delegate-task-resume/detector.ts — parameter-error guard ordering Important Files Changed
Sequence DiagramsequenceDiagram
participant O as Orchestrator
participant TT as task tool
participant Hook as DelegateTaskResumeHook
participant Tracker as TaskSessionTracker
O->>TT: invoke task(description, prompt, ...)
Note over TT: session.created fires
TT->>Tracker: register(childSessionId, parentSessionId)
Note over TT: chat.message fires (first message)
TT->>Tracker: updateAgent(sessionId, "fixer")
alt Task interrupted / provider error
TT-->>Hook: tool.execute.after (empty output)
Hook->>Hook: detectInterruptedTask() → true
Hook->>Hook: parseTaskId() → "ses_abc"
Hook->>Tracker: markInterrupted("ses_abc")
Hook->>Tracker: get("ses_abc") → {agent:"fixer", status:"interrupted"}
Hook->>Hook: buildResumeGuidance()
Hook-->>O: output += "[task partial state available]\n task_id: ses_abc\n agent: @fixer"
O->>TT: (optionally) task(..., task_id="ses_abc")
else Task succeeds
TT-->>Hook: tool.execute.after (output with content)
Hook->>Hook: detectInterruptedTask() → false
Hook->>Tracker: markCompleted("ses_abc")
Hook-->>O: output unchanged
end
Note over TT: session.deleted fires
TT->>Tracker: cleanup(sessionId)
Reviews (2): Last reviewed commit: "fix: address PR review — agent name, swe..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Adds an informational “recovery note” mechanism so the orchestrator can resume interrupted delegated subagent sessions (via task_id) instead of losing partial state when a task tool result is empty/incomplete.
Changes:
- Introduces a
TaskSessionTrackerutility to track subagent sessions bytask_idand lifecycle status. - Adds a new
delegate-task-resumehook that detects interrupted/emptytaskresults, extractstask_id, and appends a[task partial state available]note. - Updates the orchestrator prompt with a new “Session Resumption” section and wires the new hook + tracker into plugin bootstrap.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/task-session-tracker.ts |
New tracker for subagent task sessions (status + metadata). |
src/utils/task-session-tracker.test.ts |
Unit tests for tracker behavior (register/status/cleanup/sweep). |
src/hooks/delegate-task-resume/detector.ts |
Detects interrupted outputs and parses task_id from tool output. |
src/hooks/delegate-task-resume/guidance.ts |
Builds the appended recovery note text. |
src/hooks/delegate-task-resume/hook.ts |
Hook wiring to append guidance and update tracker status. |
src/hooks/delegate-task-resume/index.ts |
Barrel exports for the new hook module. |
src/hooks/delegate-task-resume/index.test.ts |
Tests for detector/guidance and hook integration behavior. |
src/hooks/index.ts |
Exports createDelegateTaskResumeHook for plugin composition. |
src/index.ts |
Instantiates tracker + hook; wires into session.created, session.deleted, and tool.execute.after. |
src/agents/orchestrator.ts |
Adds “Session Resumption” section to orchestrator prompt. |
src/interview/interview.test.ts |
Formatting-only change (Biome). |
src/utils/system-collapse.test.ts |
Import ordering/formatting-only change (Biome). |
AGENTS.md |
Updates contributor/agent guidelines and build/test notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Lifecycle: | ||
| * register() — called when the `task` tool fires via tool.execute.after | ||
| * markInterrupted() / markCompleted() — update session status | ||
| * cleanup() — called on session.deleted event | ||
| */ |
There was a problem hiding this comment.
The header comment and register() docstring say register() is called from tool.execute.after, but in the current wiring it's called from the session.created event handler (src/index.ts). Please update the documentation to reflect the actual lifecycle to avoid future confusion/misuse.
| // Track subagent task sessions for potential resumption. | ||
| // Only register child sessions (parentID set) since those are | ||
| // subagent sessions created by the task tool. | ||
| if (taskSessionTracker && childSessionId && parentSessionId) { | ||
| taskSessionTracker.register(childSessionId, parentSessionId); | ||
| } |
There was a problem hiding this comment.
TaskSessionTracker.register supports storing an agent name for later inclusion in the recovery note, but the plugin currently registers child sessions without passing any agent info. As a result, the resume guidance will almost always omit the intended "Agent: @..." line even when the session is tracked. Consider deriving the agent from session.created properties (e.g. info.title if it contains the agent) or capturing it from the task tool arguments / a subagent.session.created event and passing it into register().
I noticed a couple of things that may be worth checking before merging: 1.
|
- import parseTaskIdFromTaskOutput from src/utils/task.ts - use shared line-based parser as canonical behavior - keep XML parsing only as fallback for compatibility - adds shared helper file to branch so PR #392 aligns with merged #390 Tests: bun test src/hooks/delegate-task-resume/index.test.ts src/utils/task-session-tracker.test.ts Typecheck: bun run tsc --noEmit Lint: bun run check:ci
|
#392 (comment) |
d79add6 to
c2da790
Compare
|
I rebased/reworked this PR on top of the current The PR now integrates the useful recovery behavior directly into the existing
Validated with:
|
Summary
task-session-managerinstead of adding a separate tracker/hook.task_idvalues, including empty<task_result>, provider errors, 429/rate-limit, timeout, quota and resource-exhaustion signals.[task partial state available]note to interrupted task tool output so the orchestrator can decide whether to resume the existing subagent session.<resumable_sessions>prompt injection and alias/session tracking as the single source of truth.What changed from the old PR shape
TaskSessionTrackerapproach.delegate-task-resumehook.task-session-managerandparseTaskIdFromTaskOutput()path.AGENTS.mdand unrelated formatting/test changes out of this PR.Safety / compatibility
task_id.Validation
bun testbun run typecheckbun run lintgit diff --check