fix(server): return delegated waits on pending approvals

[gustavonline]

Problem

A child approval can leave delegate_task(mode: "wait") waiting while the child cannot progress. Questions already return control to the parent; approvals do not.

Change

Return pendingApproval with the request ID, kind and task-owned run ID. Require a pending, live record on that task’s unfinished run; exclude unrelated tasks, settled requests and stopped/disposed work. Preserve the task’s normal result and completion delivery.

Approval authority stays with the existing holder, who must inspect the fresh full request in the child thread. A pending record can outlive its provider callback.

Scope and approval

Focused blocking-wait bug from the maintainer-triaged remainder of #15082, submitted under the focused-bug exception.

This complements Lenoridev’s approval-notice offer and client attention #19630. It changes neither reactor/UI notices nor approval decisions, sign-in or remote delegation.

Verification

Native event/SQLite integration checks approval at entry and during a wait, task/run ownership, stale/other-task/Stop exclusions, and the same task’s later result with one parent delivery. The new completion wait uses an exact Deferred signal.

After resolving the comment-pruning conflicts with main 86534db4:

• 41/41 fresh tests: service 32, contracts 7, integration 2.
• Server and contracts types pass; scoped lint reports zero warnings/errors; all seven changed files pass formatting.
• Independent source review: ACCEPT. The merge preserves behavior and the exact Deferred synchronization.

Node 24.13.1 / pnpm 11.10.0, updated frozen dependencies including Effect 4.0.5. These checks use synthetic approval events and fake providers.

Lenoridev’s real Codex verification covers the previous head 1babdc3 combined with the separate approval-notice patch. Decisions came from a test client, not the parent. Real-provider or interactive-client acceptance of this merged head is not claimed. No runtime deployment.

Written with Claude Opus 5.5 (High, Fast off) through Claude Code in T3 Code. Independently reviewed with GPT-6.1-Sol (Max) through Codex in T3 Code.

[macroscopeapp]

Approvability

Verdict: Approved at 1babdc3

Macroscope's review found this PR approvable — This is a narrowly scoped delegated-wait bug fix that reports approval identity and returns control without deciding approvals, cancelling child work, or changing defaults. Extensive unit and integration coverage exercises ownership, stale-request, cancellation, retry, and eventual completion behavior.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: da186fd0-8fc2-4344-9e4a-d0c91d458cc3

📥 Commits

Reviewing files that changed from the base of the PR and between 1babdc3 and 08600ee.

📒 Files selected for processing: 4

• apps/server/src/mcp/OrchestratorMcpService.test.ts
• apps/server/src/mcp/OrchestratorMcpService.ts
• apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts
• packages/contracts/src/orchestratorMcp.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Delegated task waits and status responses now report qualifying pending approvals for unfinished task-owned runs. A wait can return while the child continues running. After approval resolution, the child can complete and deliver its result to the parent.

Changes: Delegated task approvals

Layer / File(s)	Summary
Pending approval result contract packages/contracts/src/orchestratorMcp.ts, packages/contracts/src/orchestratorMcp.test.ts, docs/orchestration-v2/orchestrator-mcp-server.md	The delegated-task result adds nullable pendingApproval details for the request ID, kind, and run ID. Contract tests cover valid and invalid approval kinds and the required field.
Approval ownership by task run apps/server/src/mcp/OrchestratorMcpService.ts, apps/server/src/mcp/OrchestratorMcpService.test.ts	The service maps pending approval requests to task-owned, unfinished runs. It excludes requests without qualifying ownership and selects the oldest qualifying request.
Wait, status, and completion flow apps/server/src/mcp/OrchestratorMcpService.ts, apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts, apps/server/src/mcp/toolkits/orchestrator/tools.ts, docs/orchestration-v2/orchestrator-mcp-server.md	Task responses and wait conditions include qualifying approvals. Integration coverage exercises approval resolution, task completion, and parent delivery. Tool descriptions and lifecycle documentation describe the approval handling boundary.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ChildRun as Child task run
  participant Service as OrchestratorMcpService
  participant Caller as delegate_task caller
  participant Holder as Approval holder
  participant Delivery as Parent delivery
  ChildRun->>Service: Record pending approval request
  Service->>Caller: Return pendingApproval and end wait
  Holder->>ChildRun: Resolve approval in child thread
  ChildRun->>Service: Complete task
  Service->>Delivery: Reserve parent delivery

Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 08600

A delegated task waiting on a child now returns early when that child has a pending approval of its own. The response identifies the request so the person who holds approval can act in the child thread, and the task's normal completion delivery is unchanged. No concrete defects were found, and the change appears ready to merge.

Security architecture risk: 🔵 Low · up to 08600

Approval notifications return control without giving the parent permission to approve. Ownership checks tie each notification to the delegated task’s unfinished work. Remaining uncertainty concerns real-provider recovery behavior and compatibility with consumers of the changed response.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The added disclosure is approval identity metadata for an app-owned delegated task belonging to the caller’s thread. Task lookup rejects arbitrary task IDs outside that ownership scope, and approval selection further restricts records to the task’s own unfinished runs.

Trust Boundaries and Controls

• observed — Existing thread-scoped invocation and active-provider-run checks remain in place. Receiving an approval ID does not add an approval decision operation: the pending-request handlers accept user-input questions and reject approval kinds.

Resilience and Maintainability Implications

• observed — Focused integration tests exercise eligibility exclusions, retry, stop, approval resolution, and subsequent task completion. The completion scenario asserts one parent delivery and a terminal result with pendingApproval null. This supports the tested transition sequence, not universal exactly-once behavior under real-provider failures or concurrent recovery.

Pre-merge checks: 4

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title is concise, uses conventional commit format, and clearly describes the main change: delegated waits now return when approvals are pending.
Description check		The description covers the problem, implementation, scope and approval basis, verification results, limitations, and tool usage. It is complete and aligned with the repository template.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[Lenoridev]

I verified this PR at 1babdc3682 with real Codex CLI parent/child sessions, combined with the local approval-notice patch 8b0623408e offered on #15082, on main 81493efb71 (which includes #19630).

• A mode: "wait" delegation returned in about four seconds against a 120-second budget: status: "running", hasPendingQuestion: false, a pendingApproval identifying the live command request and its run, and waitTimedOut: false. The parent asked the user once. After the test client approved the harmless fixture command, the child completed and the parent received its completion wake.
• Async delegation woke an idle parent. Two successive command approvals produced two distinct notices, then normal completion delivery. Resolving an approval while the parent was active cancelled the queued notice before delivery.
• Stop while the parent was active cancelled the queued notice and live child request, interrupted the child, and marked completion delivery disposed. No approval or completion message subsequently woke that parent.

The combined branch passed 104 focused server/contract/client tests plus both MCP toolkit integration tests (106 total), and scoped repository TypeScript/Effect checks. Approval decisions above came from the test client, through the same public RPC used by clients; the parent was not given approval authority. This was one-host provider/API acceptance, not interactive web, Electron, native-mobile, or remote acceptance.

For the sign-in remainder of #15082: an actual delegation to a configured Codex instance with an empty auth home returned provider_unavailable and explicit codex login instructions to the parent before creating a child. That preflight case already reaches the caller. A sign-in flow that pauses an existing child still needs a concrete reproducer; internal auth_refresh does not establish one.

Verification performed with GPT-6.1-Sol through the Codex harness.