Skip to content

fix(server): cap concurrent ACP MCP connections - #14432

Closed
kiiwee wants to merge 1 commit into
pingdotgg:mainfrom
kiiwee:fix/v2-acp-mcp-connect-cap-20260928
Closed

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

Conversation

@kiiwee

@kiiwee kiiwee commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Concurrent MCP-over-ACP connects can each observe an available slot before allocating their connection IDs, allowing the bridge to exceed its existing 16-connection cap.

Allocate the ID and mutex before checking capacity, then check and insert without yielding. This preserves the limit without reserving slots for interrupted allocations.

Validation: focused AcpMcpOverAcpBridge.test.ts suite (5 passing tests), changed-file lint/typecheck, formatting, and diff check. Deterministic regressions cover 17 overlapping connects, freeing and reusing a slot, and interrupted allocation. 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 c558f44

Macroscope's review found this PR approvable — An existing 16-connection guard is made atomic across concurrent ACP connects, with regression coverage for overlap, interruption, and slot reuse. The production change is confined to bridge bookkeeping and introduces no schema, default, deployment, or static-analysis configuration 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:07

Dismissing prior approval to re-evaluate 8385071

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:38

Dismissing prior approval to re-evaluate 8385071

@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

Replayed the contribution onto current main.
@juliusmarminge
juliusmarminge force-pushed the fix/v2-acp-mcp-connect-cap-20260928 branch from 8385071 to c558f44 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: 82bbd9b7-0259-408d-8371-cf500992659f

📥 Commits

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

📒 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.

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