Synced from GitHub
fix(server): stop shipping delegated task results twice#19759
Delegated task responses repeated the full result in both summary and latestTerminalSummary, and repeated the context-transfer ID too. This wastes context each time an agent checks a completed task.
The shared response builder now compares each pair independently and sets the repeating latest-terminal field to null on exact equality. Distinct follow-up results and transfer IDs remain available; terminal run identity, status, pending work, and the existing response shape are preserved. The task-status tool explains these semantics.
Continues @bmdavis419's work in #19759, brought onto current main without rewriting the author's commits. Replaced the 114-line mock fixture with the existing runtime replay, parameterized for identical and distinct follow-ups. The replay also proves that identical text can still carry a distinct transfer ID.
Verification:
- Before: 31 passed, 3 failed on duplicate fields. After: all 34 pass in the same service/integration files. An additional 44 HTTP, stdio, contract, and tool tests pass. Server typecheck, scoped lint/format, and whitespace checks pass.
- Merged-baseline test counts stay at 34: service 32 → 31; integration 2 → 3. No new test files.
- One full-diff review round by Claude Opus 5.5 and GPT-6-Sol: both found no actionable issues. Their reviewed content is committed unchanged.
- Live headless Linux app: real Codex parent and GPT-6-Astra child completed the same transport probe before and after. Inspected the persisted MCP tool output itself, in addition to the agent's displayed report: before JSON, after JSON. No native client or provider-adapter code changes.
Before — base server repeats both values:
Before: live MCP task status repeats summary text and its transfer ID
After — current server keeps the stable fields and nulls the duplicates:
After: live MCP task status retains the result once and nulls repeating fields
Not verified: macOS, Windows, iOS, Android, or Electron desktop. Distinct and identical follow-ups are covered by the deterministic Codex replay; the live probe covers the completed task's own result.
Implemented through T3 Code's Codex harness with GPT-6.1-Sol; tests by GPT-6-Astra; independent reviews by GPT-6-Sol and Claude Opus 5.5 through T3 Code.
Checks
20 successful · 13 skipped
Added 1 commit
- fix(server): stop shipping delegated task results twicea0c7284
Synced from GitHub
Approvability
Verdict: Approved at d0553b3
Macroscope's review found this PR approvable — The change is a narrowly scoped server fix that removes duplicated delegated-task result data while preserving distinct follow-up results, identifiers, statuses, and the existing response schema. Production behavior is isolated to response shaping, with targeted integration coverage and no configuration, deployment, security, billing, or static-analysis changes.
No code changes detected at d673033. Prior analysis still applies.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
Thread transfer impact
✅ Thread transfer remains within every enforced ceiling.
| Provider | Metric | Main baseline | This PR | Impact | PR ceiling | |
|---|---|---|---|---|---|---|
| Codex | Total thread wire | 5.0 KiB | 5.0 KiB | 0 B (0.0%) | 6.8 KiB | ✅ |
| Codex | Thread snapshot wire | 3.8 KiB | 3.8 KiB | 0 B (0.0%) | 4.9 KiB | ✅ |
| Codex | Live turn WebSocket wire | 1.2 KiB | 1.2 KiB | 0 B (0.0%) | 2.0 KiB | ✅ |
| Codex | Live turn WebSocket decoded | 20.9 KiB | 20.9 KiB | 0 B (0.0%) | 29.3 KiB | ✅ |
| Codex | Live turn messages | 2 | 2 | 0 (0.0%) | 8 | ✅ |
| Claude | Total thread wire | 5.0 KiB | 5.0 KiB | −7 B (−0.1%) | 6.8 KiB | ✅ |
| Claude | Thread snapshot wire | 3.8 KiB | 3.8 KiB | 0 B (0.0%) | 4.9 KiB | ✅ |
| Claude | Live turn WebSocket wire | 1.2 KiB | 1.2 KiB | −7 B (−0.6%) | 2.0 KiB | ✅ |
| Claude | Live turn WebSocket decoded | 21.2 KiB | 21.2 KiB | 0 B (0.0%) | 29.3 KiB | ✅ |
| Claude | Live turn messages | 2 | 2 | 0 (0.0%) | 8 | ✅ |
Baseline: 673c5e3 · PR result: d673033 · Source CI: success
Scenario and decoded snapshot size
10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
- Codex decoded thread snapshot: 108.5 KiB
- Claude decoded thread snapshot: 108.8 KiB
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.
Synced from GitHub
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:
5df17afe-1e46-4a85-b802-aa54922d1eba
📥 Commits
Reviewing files that changed from the base of the PR and between a0c72844d855dd1242f73603a5807be091d224b3 and d6730339ca8e322f378d5c15c58881de6f622cd1.
📒 Files selected for processing: 3
apps/server/src/mcp/OrchestratorMcpService.tsapps/server/src/mcp/OrchestratorMcpToolkit.integration.test.tsapps/server/src/mcp/toolkits/orchestrator/tools.ts
🚧 Files skipped from review as they are similar to previous changes: 1
- apps/server/src/mcp/toolkits/orchestrator/tools.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
📝 Walkthrough
readTasknow returns latest-terminal summary and transfer ID only when they differ from current result values. Integration replay tests cover distinct and identical follow-up results, andTaskStatusTooldocuments the nullability rule.Changes: Task status result deduplication
Layer / File(s) Summary Compare latest-terminal and current result values
apps/server/src/mcp/OrchestratorMcpService.tsreadTaskderives the latest terminal summary and transfer ID. It returnsnullfor values that duplicate the current result.Test and describe duplicate-value behavior
apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts,apps/server/src/mcp/toolkits/orchestrator/tools.tsIntegration replay tests cover distinct and identical follow-up results and verify status values. The TaskStatusTooldescription documents the nullability rules.Priority: ⬇️ Low
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers:
juliusmarminge
Merge Risk: ⚪ Minimal · up to d6730
Task status suppresses duplicate result metadata while retaining distinct values. No merge-blocking issue was established.
Security architecture risk: ⚪ Minimal · up to d6730
The change removes duplicate response values without changing task ownership, access checks, result delivery, or terminal-run identity. Distinct follow-up transfer IDs remain available even when result text is identical. No material security risk was identified in the reviewed change.
Retained concerns No architecture-level concerns identified.
Security review details
Security Blast Radius
- inferred — The changed behavior affects responses for delegated tasks already accessible to the invoking parent thread. It does not expand the set of task owners, transfer recipients, or credential capabilities accepted by this path.
Trust Boundaries and Controls
- observed — The task lookup requires orchestration capability and an app-owned task belonging to the invoking thread. Result-transfer lookup also constrains the source child thread and target parent thread before exposing transfer metadata.
- observed — The graph-labelled public entrypoint is an unexported integration-test helper. Its follow-up text argument comes from the parameterized test and is substituted into a local replay fixture; the specialist source comparison found no production caller in either base or head.
Pre-merge checks: Passed 4
Check name Status Explanation Description check Passed The description clearly states the problem, implementation, scope, verification results, and unverified platforms. It does not use the template's Problem, Change, and Scope and approval headings, but … Title check Passed The title is concise, conventional, and accurately describes the main server change: preventing delegated task results from being sent twice. Linked Issues check Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request. Full details: Description check
Explanation
The description clearly states the problem, implementation, scope, verification results, and unverified platforms. It does not use the template's Problem, Change, and Scope and approval headings, but it includes the required information in clear prose and links the related pull request.
✨ Finishing Touches
📝 Generate docstrings
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
- Commit to this branch
- 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.
Taking over this one. I’ll carry the exact-value deduplication fix onto current main, trim the tests, and run independent reviews before pushing back to this PR.
Added 2 commits
Synced from GitHub
Added 1 commit
- Merge remote-tracking branch 'origin/main' into fix/dedupe-delegated-task-resultsd673033