Skip to content

refactor(provider-core): MCP provider sessions live in a McpProviderSessions service - #17446

Merged
juliusmarminge merged 1 commit into
t3/provider-core-servicesfrom
t3/provider-mcp-sessions
Oct 9, 2026
Merged

juliusmarminge merged 1 commit into
t3/provider-core-servicesfrom
t3/provider-mcp-sessions

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

The per-thread T3 MCP credentials lived in a module-level Map in provider-core/server/mcpSession.ts, behind set/read/clearMcpProviderSession functions that any module could call. That's hidden global state, and it leaked between tests.

This PR adds @t3tools/provider-core/server/McpProviderSessions, a service with set, read and clear. The server provides it once, so ProviderSessionManager and every adapter share one instance.

  • Adapters yield it and read the session once per launch. The pure option builders (claudeMcpQueryOverrides, codexThreadRuntimeParams, cursorMcpServers, acpMcpContext) take the session they're given instead of looking it up.
  • makeClaudeAdapterV2 and makeCodexAdapterV2 are now Effect.fn, so they can yield the service.
  • Each adapter and driver env type lists McpProviderSessions. Tests get a fresh instance per layer, which replaces the old manual set/clear on the global.

Part of the provider-package audit (stack #17428).

Model: Claude Opus 5.5 via Claude Code in T3 Code.

🤖 Generated with Claude Code


Devin Review

@juliusmarminge
juliusmarminge added this pull request to stack #17428 October 9, 2026 08:25
@juliusmarminge
juliusmarminge marked this pull request as ready for review October 9, 2026 08:25
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 9, 2026
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 9, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR replaces global per-thread MCP credential state with an Effect service and threads it through the session manager and numerous provider launch and runtime paths. Because the shared-infrastructure refactor changes credential lifecycle and authentication-sensitive behavior across 48 files, human review is warranted.

No code changes detected at 19ba124. Prior analysis still applies.

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

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 5.0 KiB — 6.8 KiB ✅
Codex Thread snapshot wire — 3.8 KiB — 4.9 KiB ✅
Codex Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Codex Live turn WebSocket decoded — 20.9 KiB — 29.3 KiB ✅
Codex Live turn messages — 2 — 8 ✅
Claude Total thread wire — 5.0 KiB — 6.8 KiB ✅
Claude Thread snapshot wire — 3.8 KiB — 4.9 KiB ✅
Claude Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Claude Live turn WebSocket decoded — 21.2 KiB — 29.3 KiB ✅
Claude Live turn messages — 1 — 8 ✅

Baseline: unavailable · PR result: 19ba124 · 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.

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 24875f82-e03f-43d9-bf3f-167ca760fee7

📥 Commits

Reviewing files that changed from the base of the PR and between 519937b and 19ba124.


📒 Files selected for processing (1)
  • apps/server/src/server.ts

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



📝 Walkthrough

Walkthrough

The pull request adds an Effect-based McpProviderSessions service, removes the synchronous session registry, and updates provider adapters, orchestration code, runtime layers, and tests to use the service.

Changes

MCP provider session migration

Layer / File(s) Summary
Service and adapter integration
packages/provider-core/src/server/McpProviderSessions.ts, packages/provider-core/src/server/mcpSession.ts, packages/provider-core/package.json, packages/provider-*/src/server/adapter.ts, apps/server/src/orchestration-v2/Adapters/*AdapterV2.ts
The new service stores, reads, and clears thread-scoped MCP configuration. Adapters read sessions through the service or receive resolved session configuration directly. The legacy registry is removed.
Credential management and runtime wiring
apps/server/src/orchestration-v2/ProviderSessionManager.ts, apps/server/src/orchestration-v2/testkit/ProviderReplayHarness.ts, apps/server/src/server.ts, apps/server/src/provider/Drivers/*
Provider session preparation and cleanup use effectful service operations. The runtime and replay harness provide the service to dependent layers.
Tests and fixtures
apps/server/src/orchestration-v2/**/*.test.ts, apps/server/src/mcp/*.test.ts, apps/server/src/provider/**/*.test.ts, packages/provider-*/src/server/*.test.ts
Test environments provide the service. Tests and fixtures register, read, and clear sessions through it, and adapter construction uses Effect where required.

Priority: ➖ Normal

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

Change: Refactor

Suggested reviewers: t3dotgg


Merge Risk: ⚪ Minimal · up to 19ba1

The server shares the MCP session service across the manager and adapters, with no identified user-facing or operational issue from this wiring change. It is ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check Warning The description explains the problem and the implementation, but it omits the required Scope and approval and Verification sections. It does not provide approval evidence, focused test results, or che… Add a Scope and approval section with the linked issue or explicit maintainer approval, and add a Verification section with the focused tests or manual checks run, observed results, and any checks not performed.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the main change: moving MCP provider sessions into the McpProviderSessions service.
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 explains the problem and the implementation, but it omits the required Scope and approval and Verification sections. It does not provide approval evidence, focused test results, or checks that were not run.



  • Fix all pre-merge checks with AI
✨ 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.

…essions service

The per-thread MCP credentials were a module-level Map in mcpSession.ts. The
session manager now writes them through McpProviderSessions and adapters yield
it, reading the session once per launch and handing it to their pure option
builders. The server provides one instance; tests get a fresh one per layer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@juliusmarminge
juliusmarminge force-pushed the t3/provider-mcp-sessions branch from 519937b to 19ba124 Compare October 9, 2026 16:08
@juliusmarminge
juliusmarminge merged commit 784b562 into main Oct 9, 2026
30 checks passed
@juliusmarminge
juliusmarminge deleted the t3/provider-mcp-sessions branch October 9, 2026 16:15
kshanxs added a commit to kshanxs/t3code that referenced this pull request Oct 9, 2026
Resolve the import conflict in AntigravityDriver.ts with the
McpProviderSessions refactor (pingdotgg#17446).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Oct 9, 2026
## What's Changed
* fix(web): Local environment switch stays reachable after turning it off by @ScottN-PV in pingdotgg/t3code#17359
* fix(web): keep chat banners inside the lane beside the docked details card by @macodev00 in pingdotgg/t3code#17094
* fix(web): settled and snoozed lines line up with the messages above them by @RakshithBhat03 in pingdotgg/t3code#17191
* fix(web): distinguish project filter from new project by @voltcrash in pingdotgg/t3code#12113
* feat(web): assign a thread details panel shortcut by @maria-rcks in pingdotgg/t3code#16694
* fix(web): chat content keeps pace with sidebar resizing by @flamboh in pingdotgg/t3code#17383
* refactor(provider-core): expose model metadata through a ModelCatalog port by @juliusmarminge in pingdotgg/t3code#17417
* refactor(provider-core): follow the Effect service conventions throughout by @juliusmarminge in pingdotgg/t3code#17427
* refactor(provider-core): latest-version lookups go through a ProviderLatestVersions service by @juliusmarminge in pingdotgg/t3code#17434
* refactor(provider-core): MCP provider sessions live in a McpProviderSessions service by @juliusmarminge in pingdotgg/t3code#17446
* refactor(provider): bring opencode, muse, pi, core and testing in line with Effect conventions by @juliusmarminge in pingdotgg/t3code#17542
* refactor(provider-acp): ACP, ACP Registry and Grok follow the Effect service conventions by @juliusmarminge in pingdotgg/t3code#17544
* refactor(provider-cursor): follow the Effect service conventions by @juliusmarminge in pingdotgg/t3code#17545
* fix(marketing): use app wordmark in header by @voltcrash in pingdotgg/t3code#13240
* fix(web): pr merge actions stay visible while the stack refreshes by @maria-rcks in pingdotgg/t3code#17559

## New Contributors
* @voltcrash made their first contribution in pingdotgg/t3code#12113

**Full Changelog**: pingdotgg/t3code@v0.0.46-nightly.20261009.2873...v0.0.46-nightly.20261009.2886

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.46-nightly.20261009.2886
github-actions Bot added a commit to davidvanderklay/t3code-flake that referenced this pull request Oct 9, 2026
## What's Changed
* fix(web): Local environment switch stays reachable after turning it off by @ScottN-PV in pingdotgg/t3code#17359
* fix(web): keep chat banners inside the lane beside the docked details card by @macodev00 in pingdotgg/t3code#17094
* fix(web): settled and snoozed lines line up with the messages above them by @RakshithBhat03 in pingdotgg/t3code#17191
* fix(web): distinguish project filter from new project by @voltcrash in pingdotgg/t3code#12113
* feat(web): assign a thread details panel shortcut by @maria-rcks in pingdotgg/t3code#16694
* fix(web): chat content keeps pace with sidebar resizing by @flamboh in pingdotgg/t3code#17383
* refactor(provider-core): expose model metadata through a ModelCatalog port by @juliusmarminge in pingdotgg/t3code#17417
* refactor(provider-core): follow the Effect service conventions throughout by @juliusmarminge in pingdotgg/t3code#17427
* refactor(provider-core): latest-version lookups go through a ProviderLatestVersions service by @juliusmarminge in pingdotgg/t3code#17434
* refactor(provider-core): MCP provider sessions live in a McpProviderSessions service by @juliusmarminge in pingdotgg/t3code#17446
* refactor(provider): bring opencode, muse, pi, core and testing in line with Effect conventions by @juliusmarminge in pingdotgg/t3code#17542
* refactor(provider-acp): ACP, ACP Registry and Grok follow the Effect service conventions by @juliusmarminge in pingdotgg/t3code#17544
* refactor(provider-cursor): follow the Effect service conventions by @juliusmarminge in pingdotgg/t3code#17545
* fix(marketing): use app wordmark in header by @voltcrash in pingdotgg/t3code#13240
* fix(web): pr merge actions stay visible while the stack refreshes by @maria-rcks in pingdotgg/t3code#17559

## New Contributors
* @voltcrash made their first contribution in pingdotgg/t3code#12113

**Full Changelog**: pingdotgg/t3code@v0.0.46-nightly.20261009.2873...v0.0.46-nightly.20261009.2886

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.46-nightly.20261009.2886
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:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant