[Story 3.3] Child completion callback and parent wakeup

[edelauna]

Part of #358 (Epic 3: TaskScheduler + Subtask Fan-out).

Depends on: Story 3.2d (Webview-side task-scoping guard — must land before fan-out is first exercised in user-visible UI).

Relates to #126 (concurrent conversation sidebar — this story completes the fan-out loop).

Context

When a child completes, the parent needs to receive the result and resume. Currently this is embedded entirely in reopenParentFromDelegation (disk-based: reads messages from disk, injects tool_result, reopens parent instance). For fan-out, the parent may still be running and its state is live in memory — disk I/O is unnecessary and would race against the live instance. Two injection paths are needed: one per parent state.

This story assumes Stories 3.2b-3.2d have already established task isolation:

• 3.2b: parent and child execution contexts do not leak into each other during child setup
• 3.2c: non-focused tasks do not post into the single webview
• 3.2d: the webview rejects cross-task messages/state and uses per-task sequence guards

Developer Notes

In src/core/task/TaskScheduler.ts:

• Register an onComplete callback per child via RooCodeEventName.TaskCompleted or TaskDelegationCompleted.
• On child completion: (1) release semaphore permit; (2) check if parent is still active in TaskRegistry (fan-out path) or was suspended (sequential path); (3) call the appropriate injection path.

In src/core/webview/ClineProvider.ts:

• Sequential path (parent was suspended): Existing reopenParentFromDelegation flow — reads messages from disk, injects tool_result, reopens parent Task instance. No change to this path.
• Fan-out path (parent is still running): In-memory injection directly into the parent's live userMessageContent array via pushToolResultToUserContent(). Critical: the child completion callback must NOT call parent.setUserMessageContentReady(true). The parent's own dispatch loop is the sole owner of userMessageContentReady — it sets it to true only after all tool results for the current turn are collected. This follows the Single Writer Principle.
• Parent lookup must be by explicit parentTaskId, not by getCurrentTask(). During fan-out the focused task may be the child or another live task; during sequential delegation the parent may be persisted but not live.

Add /** @invariant Single Writer — only the parent's dispatch loop may set this to true. See Single Writer Principle (Thompson, 2011). */ JSDoc at the userMessageContentReady field in Task.ts.

Files: src/core/task/TaskScheduler.ts, src/core/webview/ClineProvider.ts

Tests (extend src/core/task/__tests__/TaskScheduler.spec.ts and delegation tests):

• Child completion releases semaphore permit (semaphore.available increases by 1).
• Sequential path: assert reopenParentFromDelegation is called with correct completionResultSummary.
• Fan-out path: assert pushToolResultToUserContent is called on live parent instance and userMessageContentReady is NOT set by the callback.
• Fan-out path: assert that if the parent has in-flight tools, userMessageContentReady remains false after child completion injection.
• Fan-out path with child focused: completion still injects into the parent by parentTaskId, not into whichever task is current.
• Semaphore permit restored on child abort and on child unhandled crash (the finally guard from Story 3.1 covers this).
• Completed child Task instances removed from activeTasks map after callback.
• Orphan handling: Parent aborts while child is in-flight → assert abortTask(true) called on the orphaned child, semaphore permit released, child removed from activeTasks.

Acceptance Criteria

• Parent receives completionResultSummary as a tool_result in its next API call regardless of which path is taken.
• No memory leak: completed task instances are removed from the scheduler's map.
• Correct injection path selected based on parent's live vs suspended state.
• Completion injection is scoped by explicit parent/child ids and does not depend on focused task.
• Orphan handling: TaskScheduler listens for RooCodeEventName.TaskAborted on the parent and calls abortTask(true) on orphaned children.
• Invariant: Only the parent's dispatch loop sets userMessageContentReady = true. No external callback may set this flag.

Out of Scope

• Story [Story 3.2b] Fan-out: parent stays active while child runs #369 owns parent/child execution-context isolation and delegation rollback safety.
• Story [Story 3.2c] Extension-side task-scoping guard #370 owns extension-host post suppression and live-task focus switching.
• Story [Story 3.2d] Webview-side task-scoping guard + per-task seq #371 owns webview-side rejection and per-task sequence handling.

[edelauna]

Observed a live instance of this exact race in CI, on the merge-queue run for an unrelated PR (#867, xAI provider changes — confirmed no overlap with subtask/delegation code):

https://github.com/Zoo-Code-Org/Zoo-Code/actions/runs/29504558797/job/87641832328

Test: apps/vscode-e2e/src/suite/subtasks.test.ts — "child completing on its first response returns to parent"

Failure:

AssertionError [ERR_ASSERTION]: Parent should resume after the child completes on its first response
+ actual - expected
+ 'Fast child completed'
- 'Fast parent resumed'

The parent's final message ended up being the child's own completion text instead of the parent's post-resume message — consistent with a parent-resume/child-completion ordering race rather than a hard failure (76/77 other e2e tests passed in the same run, and the xAI-specific tests were all green). Re-running the same commit would likely pass, which points at this story's callback/wakeup path rather than anything test-specific.

Logging here since this is the concrete mechanism this story (child completion callback and parent wakeup) is meant to fix — figured a real repro link would be useful when this gets picked up, since PR #94 originally unskipped this test as a regression anchor for exactly this kind of race, not because it's expected to be flake-free until the underlying fix (this story) lands.

[edelauna]

Note from implementing Story 3.2b (#369): the review on that story caught a TOCTOU bug in permit handling that this story's semaphore-release step looks likely to reintroduce.

The bug (3.2b): the fan-out decision originally checked taskScheduler.available > 0 and only actually acquired the permit several awaits later (mode switch, task creation, history persistence). In between, a concurrent delegation could consume the last permit, leaving the "fan-out" parent live but the child queued instead of actually concurrent — the availability check was stale by the time it was acted on. Fix was TaskScheduler.tryReserve(): reserve the permit atomically at decision time (no await between the isLocked() check and the acquire), then thread that specific reservation through to the eventual run, with explicit release-on-rollback if the flow errors before the task starts.

Why it's relevant here: the dev notes describe, per child completion: "(1) release semaphore permit; (2) check if parent is still active in TaskRegistry... (3) call the appropriate injection path." That's release-then-decide, with awaits (disk read for the sequential path, or pushToolResultToUserContent for the fan-out path) between steps 1 and 3. Once the permit is released in step 1, a different delegation's tryReserve() can immediately claim it — before this callback has finished routing the completion result to the correct parent. That's not obviously harmful (the two operations may be independent), but it's worth deciding deliberately rather than by accident, especially combined with the orphan-handling / TaskAborted listener also described here, since permit release and injection-path selection both touch scheduler/registry state that a newly-admitted task could observe mid-transition.

Suggest either: (a) decide and begin the injection path before releasing the permit, or (b) confirm explicitly that releasing early is intentional and safe given what a newly-admitted task can observe, and add a test exercising a concurrent tryReserve() racing the release.