Skip to content

fix(server): discard failed ACP MCP disconnect bodies - #14420

Open
kiiwee wants to merge 1 commit into
pingdotgg:mainfrom
kiiwee:fix/v2-acp-mcp-disconnect-cleanup-20260928
Open

kiiwee wants to merge 1 commit into
pingdotgg:mainfrom
kiiwee:fix/v2-acp-mcp-disconnect-cleanup-20260928

Conversation

@kiiwee

@kiiwee kiiwee commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

An unsuccessful MCP-over-ACP disconnect returns an error without cancelling its HTTP response body. Repeated failures can retain transport resources.

Discard the response body before checking its status, preserving the existing typed error and 404 handling.

Validation: focused AcpMcpOverAcpBridge suite (4 passing tests), changed-file type-aware lint/typecheck, formatting, and diff check. The new regression fails for an uncancelled HTTP 500 body and covers HTTP 404, success, and connection removal. Scoped checks retain one pre-existing require-yield warning on the unchanged bridge factory.

Targets the Orchestrator V2 branch, not main.

Implementation: Grok 4.6 Build via Grok CLI, with Codex-supervised fixture refinement and scoped verification. Independent review: Claude Opus 5.5 (high) via Claude Code (claude -c). Publication ops: GPT-6.1 Sol (xhigh) via Codex.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 30, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 30, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 0bf061a

Macroscope's review found this PR approvable — This is a localized server bug fix that releases failed disconnect response bodies without changing the existing error or 404 semantics. The accompanying tests cover the affected statuses and connection cleanup, with no product-default, schema, deployment, or static-analysis changes.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 1, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 1, 2026 04:04

Dismissing prior approval to re-evaluate 0ab5563

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 1, 2026
@juliusmarminge
juliusmarminge deleted the branch pingdotgg:main October 2, 2026 19:23
@juliusmarminge juliusmarminge added the triage:keep-open Keeps this PR open despite not necessarily passing the contribution guide fully label Oct 2, 2026
@juliusmarminge juliusmarminge reopened this Oct 2, 2026
@juliusmarminge
juliusmarminge changed the base branch from t3code/codex-turn-mapping to main October 2, 2026 20:38
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 2, 2026 20:39

Dismissing prior approval to re-evaluate 0ab5563

@macroscopeapp

macroscopeapp Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Macroscope skipped reviewing this pull request. Per-review cost limit exceeded (workspace setting).

This review would cost an estimated $15.63, which exceeds your per-review limit of $15.00.

The top 3 files driving up this estimate:

File Diff Size Estimate
packages/contracts/src/orchestrationV2.ts 127.11KB $2.13
apps/server/src/mcp/OrchestratorMcpService.ts 75.70KB $1.27
apps/server/src/orchestration-v2/EventSink.ts 33.91KB $0.57

Tip

To get this pull request reviewed, you can:

  1. Comment @macroscope-app on this PR to request a manual review (monthly spend limits still apply).
  2. Exclude the file(s) above from review by adding a pattern to your .macroscope/ignore.md — note that creating this file replaces Macroscope's built-in default ignores rather than extending them.
  3. Raise your cost limit in your workspace billing settings.

Turn off this reminder going forward

@juliusmarminge
juliusmarminge force-pushed the fix/v2-acp-mcp-disconnect-cleanup-20260928 branch from 0ab5563 to 0bf061a Compare October 2, 2026 20:54
@coderabbitai

coderabbitai Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 46 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 639a14a1-5d88-41fc-b271-968d8d437b84

📥 Commits

Reviewing files that changed from the base of the PR and between cc1e634 and 0bf061a.

📒 Files selected for processing (2)
  • apps/server/src/mcp/AcpMcpOverAcpBridge.test.ts
  • apps/server/src/mcp/AcpMcpOverAcpBridge.ts
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Hi! We are cleaning up open PRs, and this one appears to have been created with an older model (grok 4.6). If this change is really important, we recommend rebuilding the PR with a newer model if possible.

@maria-rcks maria-rcks closed this Oct 11, 2026
@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Reopening, this was closed by mistake. Sorry for the noise!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:XS 0-9 changed lines (additions + deletions). triage:keep-open Keeps this PR open despite not necessarily passing the contribution guide fully vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants