fix(server): switching providers no longer revokes the other provider's T3 tool access by Alb11747 · Pull Request #19413 · pingdotgg/t3code

Alb11747 · 2026-10-14T12:05:43Z

Switching providers replaced a thread’s single MCP credential. Provider processes that still held the old token then received 401 errors or used a token scoped to the wrong provider instance.

Keep credentials by thread and provider instance. Every adapter reads its own credential; rotation and release revoke only the matching credential. Terminal archive/delete cleanup still revokes all credentials for the thread. Active-run ownership checks for mutations remain unchanged.

Thanks @Alb11747 for the reproduction and original fix in #19413, following the maintainer direction on #16565 and the failure reported in #18852. The maintainer takeover preserves the author’s commits, merges current main, consolidates Codex reads through its existing helper, and replaces the original regression with a switch-back scenario.

Verification on Linux / Node 24:

PR-added regression tests: 1 → 1 in the existing manager suite; 66 manager tests pass. The replacement covers A → B → A with two instances of the same driver, captured adapter credentials, runtime reuse, independent close/reissue, and thread-wide cleanup. Without the fix, opening B revokes A’s captured token and the test fails.
One fresh parallel adversarial round (Claude Opus 5.5 and GPT-6 Sol) found no actionable code defects. The historical OpenCode registration collision finding was rechecked and remains inapplicable: managed instances own separate servers; shared external connections skip registration.
999 tests pass across 14 focused lifecycle, authorization and adapter suites. Both terminal-deletion routing tests also pass. Scoped server, provider-core, ACP, Cursor, Muse, OpenCode and Pi typechecks pass; changed-file lint, formatting and whitespace checks pass (existing warnings only).
Before/after MCP HTTP proof: production manager, credential services, HTTP authentication and tool guards on isolated state. Main fails with HTTP 401 when the original A client calls after B opens. Head passes all 16 observations, including original-client access after switch-back, independent revocation, reissue, mutation ownership refusal, and terminal-detach revocation of both credentials.

The HTTP proof uses provider adapter fixtures and a controlled active-run shell. Authenticated Claude/Codex turns, UI switching and archive/delete command dispatch were not exercised live; these still need a human check. macOS, Windows, iOS, Android and Electron desktop were not verified. There is no visual UI change, so screenshots do not add evidence. Credential-lifetime changes still need human review; automated correctness checks do not supply that approval.

Tests: Codex GPT-6 Astra (high). Implementation and server verification: Codex GPT-6.1 Sol (high). Adversarial review: Claude Opus 5.5 and Codex GPT-6 Sol, through T3 Code.

Fixes #16565

chatgpt-codex-connector · 2026-10-14T12:05:56Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-18T19:36:29.416149Z	`2d41178`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

macroscopeapp · 2026-10-14T12:08:01Z

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR changes production MCP credential storage, provider scoping, and revocation behavior so authorization tokens can remain valid across provider switches. Because it affects authentication-sensitive runtime behavior and credential lifetime, human review is warranted.

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

coderabbitai · 2026-10-14T12:17:54Z

No actionable comments were generated in the recent review.

Walkthrough

MCP session configurations are now stored and selected by thread and provider instance. Provider adapters pass the instance ID when reading configurations. Session preparation and cleanup target the matching provider credential.

Changes: Provider-scoped MCP credentials

Layer / File(s)	Summary
Per-provider MCP session storage `packages/provider-core/src/server/McpProviderSessions.ts`	MCP session storage keeps configurations by thread and provider instance. Reads require both IDs, and `clear` can remove configurations by provider session ID.
Provider session credential lifecycle `apps/server/src/orchestration-v2/ProviderSessionManager.ts`, `apps/server/src/orchestration-v2/ProviderSessionManager.test.ts`	Credential validation and cleanup target the provider session. Another attached session on the same provider instance can hold the credential. Tests cover separate credentials for two provider instances on one thread and verify that closing one instance leaves the other credential in place.
Provider adapter lookups `apps/server/src/orchestration-v2/Adapters/`, `packages/provider-acp/src/server/adapter.ts`, `packages/provider-cursor/src/server/adapter.ts`, `packages/provider-muse/src/server/adapter.ts`, `packages/provider-opencode/src/server/`, `packages/provider-pi/src/server/adapter.ts`	Adapters pass the provider instance ID when reading MCP sessions for a thread.

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

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

pingdotgg/t3code#28935 — Reports parent_not_active after a provider handoff and identifies stale provider-scoped MCP credentials as a possible cause. This change updates credential selection and cleanup by provider instance.

Possibly related PRs

pingdotgg/t3code#29803 — Its Claude MCP App operations use MCP session configuration, and this change scopes Claude session reads by provider instance.
pingdotgg/t3code#25661 — Its Claude device-tool capability lookup uses the thread’s MCP session, which this change scopes by provider instance.

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to a5699

The change scopes tool credentials per provider instance, so switching providers no longer revokes the other provider's access. No concrete merge-blocking defect remains. Authenticated provider switching was not exercised live, which normal verification can cover.

Architecture risk: Low · up to 2d411

The change affects 7 systems.

Changed systems: apps/server, packages/provider-cursor, packages/provider-opencode, packages/provider-acp, packages/provider-core, packages/provider-muse, packages/provider-pi

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

observed — apps/server (service) was modified; 4 changed files map to changed impact.
observed — packages/provider-cursor (library) was modified; 1 changed file maps to changed impact.
observed — packages/provider-opencode (library) was modified; 2 changed files map to changed impact.
observed — packages/provider-acp (library) was modified; 1 changed file maps to changed impact.

Before / after behavior

observed — Modified behavior in apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts: readMcpSession now supplies adapterOptions.instanceId to mcpSessions.read for non-null thread IDs; null thread IDs still return undefined.
observed — Modified behavior in apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts: Turn startup now includes adapterOptions.instanceId when reading the MCP session for the turn’s thread.
observed — Modified behavior in apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts: Thread creation now includes adapterOptions.instanceId when reading the MCP session for the new thread.
observed — Modified behavior in apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts: Thread forking now includes adapterOptions.instanceId when reading the MCP session for the target thread.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: switching providers no longer revokes the other provider’s T3 tool access.
Description check		The description explains the problem, implementation, scope, linked issues and approval context, verification results, limitations, and required human review. It provides substantially complete inform…
Linked Issues check		The pull request satisfies the coding requirements in `#16565`. `McpProviderSessions` stores credentials by thread and provider instance. The provider adapters read credentials with their own provider i…
Out of Scope Changes check		The changes stay within `#16565`. The modified manager, credential store, adapter lookups, lifecycle cleanup, and regression tests directly implement provider-scoped MCP credentials for reused sessions.…

Full details: Description check

Explanation

The description explains the problem, implementation, scope, linked issues and approval context, verification results, limitations, and required human review. It provides substantially complete information for the template.

Full details: Linked Issues check

Explanation

The pull request satisfies the coding requirements in #16565. McpProviderSessions stores credentials by thread and provider instance. The provider adapters read credentials with their own provider instance IDs. ProviderSessionManager revokes only the matching credential during rotation and release, while terminal cleanup can revoke all thread credentials. The manager regression test covers provider switching, same-driver instances, credential isolation, independent cleanup, reissue, and thread-wide cleanup. Active-run mutation ownership checks remain unchanged.

Full details: Out of Scope Changes check

Explanation

The changes stay within #16565. The modified manager, credential store, adapter lookups, lifecycle cleanup, and regression tests directly implement provider-scoped MCP credentials for reused sessions. No unrelated functional change is established by the pull request summary.

Finishing Touches

Generate unit tests (beta)

Create a new PR

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

Recent review info

Run configuration

Source mode: Auto

Repository: pingdotgg/t3code

Branch: main

File: .coderabbit.config.ts

Commit: a03d9bc

Layers: Repository TS config

Review profile: CHILL

Plan: Advanced

Run ID: `25d41350-e8c6-41da-9a29-1c054f5b25ef`

Commits

Reviewing files that changed from the base of the PR and between `2d41178` and `a569956`.

Files selected for processing: 11

`apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts`

`apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts`

`apps/server/src/orchestration-v2/ProviderSessionManager.test.ts`

`apps/server/src/orchestration-v2/ProviderSessionManager.ts`

`packages/provider-acp/src/server/adapter.ts`

`packages/provider-core/src/server/McpProviderSessions.ts`

`packages/provider-cursor/src/server/adapter.ts`

`packages/provider-muse/src/server/adapter.ts`

`packages/provider-opencode/src/server/adapter.ts`

`packages/provider-opencode/src/server/v2/adapter.ts`

`packages/provider-pi/src/server/adapter.ts`

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

…'s T3 tool access Each thread had a single MCP credential slot. Preparing a session for one provider revoked every credential on the thread and replaced the slot, so a provider session still attached from before the switch kept calling T3 tools with a revoked token (401 "Authentication required"), or read a token scoped to the other provider (parent_not_active). Credentials are now kept per thread and provider instance. Rotation revokes only the credential being replaced, and session release only defers revocation to other live sessions of the same provider instance. Fixes pingdotgg#16565 Co-Authored-By: Claude Opus 5.5 <[email protected]> Upstream-PR: pingdotgg#19413

Alb11747 · 2026-10-14T12:37:51Z

Checked CodeRabbit's read-lifetime note against the source. McpToolAccess.reads already permits reads without active-run ownership; assertLiveCaller still requires the calling provider to own the active run for mutations. Keeping each still-live provider's credential usable across switches is the intended fix for #16565. Adding active-provider checks to reads would introduce a separate authorization policy, so I am leaving that outside this PR. Release still revokes the relevant credential, and archive/delete revoke all thread credentials.

Upstream checks passed on 8fb4634. Fork checks for this head have also been dispatched: https://github.com/Alb11747/t3code/actions/runs/38565217378 (pending).

…'s T3 tool access Each thread had a single MCP credential slot. Preparing a session for one provider revoked every credential on the thread and replaced the slot, so a provider session still attached from before the switch kept calling T3 tools with a revoked token (401 "Authentication required"), or read a token scoped to the other provider (parent_not_active). Credentials are now kept per thread and provider instance. Rotation revokes only the credential being replaced, and session release only defers revocation to other live sessions of the same provider instance. Fixes pingdotgg#16565 Co-Authored-By: Claude Opus 5.5 <[email protected]> Upstream-PR: pingdotgg#19413

…r-provider

…'s T3 tool access Each thread had a single MCP credential slot. Preparing a session for one provider revoked every credential on the thread and replaced the slot, so a provider session still attached from before the switch kept calling T3 tools with a revoked token (401 "Authentication required"), or read a token scoped to the other provider (parent_not_active). Credentials are now kept per thread and provider instance. Rotation revokes only the credential being replaced, and session release only defers revocation to other live sessions of the same provider instance. Fixes pingdotgg#16565 Co-Authored-By: Claude Opus 5.5 <[email protected]> Upstream-PR: pingdotgg#19413

…r-provider # Conflicts: # packages/provider-cursor/src/server/adapter.ts

…r-provider

…'s T3 tool access Each thread had a single MCP credential slot. Preparing a session for one provider revoked every credential on the thread and replaced the slot, so a provider session still attached from before the switch kept calling T3 tools with a revoked token (401 "Authentication required"), or read a token scoped to the other provider (parent_not_active). Credentials are now kept per thread and provider instance. Rotation revokes only the credential being replaced, and session release only defers revocation to other live sessions of the same provider instance. Fixes pingdotgg#16565 Co-Authored-By: Claude Opus 5.5 <[email protected]> Upstream-PR: pingdotgg#19413

…ed provider session A thread that ran on provider instance A, switched to B, and came back to A reused A's still-attached session. The attach path skipped credential preparation for an already-attached thread, so the thread's MCP slot kept the credential B had issued. Every orchestration call from the returning A turn then failed with parent_not_active, because the credential named instance B. Before a turn uses an already-attached session, the manager now checks the thread's credential slot. When the slot is scoped to another provider instance, it prepares a credential for this instance and records it on the session, as a new attachment would. This is the approach of upstream pingdotgg#25369 (originally pingdotgg#17096), not the per-instance credentials of pingdotgg#19413 or the detach-on-switch of pingdotgg#27206. Fixes pingdotgg#16565 and case 1 of pingdotgg#23006, including the usage-limit failover between two instances of one provider. Not covered: one credential slot per thread remains, so preparing the returning instance's credential revokes the other instance's. A provider whose native MCP client keeps the token it started with (a Codex thread still loaded in the shared app-server) gets 401 after a switch and back (pingdotgg#18852). That needs per-instance credentials (pingdotgg#19413). Written with Claude Opus 5.5 in Claude Code (T3 Code). Co-Authored-By: Claude Opus 5.5 <[email protected]>

…r-provider

AI-assisted; verified with the tests and tools listed above.

juliusmarminge · 2026-11-11T06:57:52Z

The automated pipeline stopped on this pull request and will not merge it:

the capture step could not show the problem on the base commit, so there is nothing to prove fixed.

On both commits: start a Claude thread and call orchestrator_capabilities; complete the turn; switch the same thread to Codex and call orchestrator_capabilities; return to Claude and call orchestrator_capabilities followed by delegate_task for a child replying OK; return to Codex within its retained-session window and call list_thread_pull_requests. All calls succeeded on both commits. Paired videos repeat the return-to-Claude delegation and return-to-Codex tool call.

It stays open for a person to finish or close.

bompus · 2026-11-11T07:07:54Z

Reproduction the capture step missed. The pipeline's script only calls tools after each provider switch; the failure is that the T3 tool credential for the thread is revoked when the thread returns to a provider (A → B → A), which a successful call right after the switch does not show. On upstream main 543ee27, with this PR's ProviderSessionManager.test.ts applied, cd apps/server && vp test run src/orchestration-v2/ProviderSessionManager.test.ts fails 1 of 58: 'preserves MCP access through A → B → A and isolates cleanup' (assert at line 2622: the token resolves to undefined for the thread). On this PR's head a569956 all 58 pass.

…ed provider session A thread that ran on provider instance A, switched to B, and came back to A reused A's still-attached session. The attach path skipped credential preparation for an already-attached thread, so the thread's MCP slot kept the credential B had issued. Every orchestration call from the returning A turn then failed with parent_not_active, because the credential named instance B. Before a turn uses an already-attached session, the manager now checks the thread's credential slot. When the slot is scoped to another provider instance, it prepares a credential for this instance and records it on the session, as a new attachment would. This is the approach of upstream pingdotgg#25369 (originally pingdotgg#17096), not the per-instance credentials of pingdotgg#19413 or the detach-on-switch of pingdotgg#27206. Fixes pingdotgg#16565 and case 1 of pingdotgg#23006, including the usage-limit failover between two instances of one provider. Not covered: one credential slot per thread remains, so preparing the returning instance's credential revokes the other instance's. A provider whose native MCP client keeps the token it started with (a Codex thread still loaded in the shared app-server) gets 401 after a switch and back (pingdotgg#18852). That needs per-instance credentials (pingdotgg#19413). Written with Claude Opus 5.5 in Claude Code (T3 Code). Co-Authored-By: Claude Opus 5.5 <[email protected]>

t3dotgg · 2026-11-12T10:16:56Z

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Closing. This PR has been open for more than 3 weeks, and main has moved a long way since then. If the change still matters, open a fresh PR from current main.

…ed provider session A thread that ran on provider instance A, switched to B, and came back to A reused A's still-attached session. The attach path skipped credential preparation for an already-attached thread, so the thread's MCP slot kept the credential B had issued. Every orchestration call from the returning A turn then failed with parent_not_active, because the credential named instance B. Before a turn uses an already-attached session, the manager now checks the thread's credential slot. When the slot is scoped to another provider instance, it prepares a credential for this instance and records it on the session, as a new attachment would. This is the approach of upstream pingdotgg#25369 (originally pingdotgg#17096), not the per-instance credentials of pingdotgg#19413 or the detach-on-switch of pingdotgg#27206. Fixes pingdotgg#16565 and case 1 of pingdotgg#23006, including the usage-limit failover between two instances of one provider. Not covered: one credential slot per thread remains, so preparing the returning instance's credential revokes the other instance's. A provider whose native MCP client keeps the token it started with (a Codex thread still loaded in the shared app-server) gets 401 after a switch and back (pingdotgg#18852). That needs per-instance credentials (pingdotgg#19413). Written with Claude Opus 5.5 in Claude Code (T3 Code). Co-Authored-By: Claude Opus 5.5 <[email protected]>

This was referenced Oct 14, 2026

fix(server): refresh MCP clients when reusing a provider session #18884

Closed

fix(server): T3 tools work again after a thread switches back to a reused provider session #17096

Closed

github-actions Bot added vouch:unvouched Author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 14, 2026

Merge remote-tracking branch 'origin/main' into fix/mcp-credential-pe…

2574539

…r-provider

juliusmarminge added 2 commits October 20, 2026 05:09

Merge remote-tracking branch 'origin/main' into fix/mcp-credential-pe…

93bc501

…r-provider # Conflicts: # packages/provider-cursor/src/server/adapter.ts

Merge remote-tracking branch 'origin/main' into fix/mcp-credential-pe…

d1ced32

…r-provider

tcrapts mentioned this pull request Oct 29, 2026

fix(server): T3 write tools keep working after switching a thread's provider and back #27206

Closed

juliusmarminge added 2 commits November 11, 2026 06:22

Merge remote-tracking branch 'origin/main' into fix/mcp-credential-pe…

1a791d1

…r-provider

test(server): keep the provider switch test on the session manager

a569956

AI-assisted; verified with the tests and tools listed above.

t3dotgg closed this Nov 12, 2026

Alb11747 mentioned this pull request Nov 12, 2026

fix(server): switching providers no longer revokes the other provider's T3 tool access #35331

Closed

Navigation Menu

Repository navigation

fix(server): switching providers no longer revokes the other provider's T3 tool access - #19413

fix(server): switching providers no longer revokes the other provider's T3 tool access#19413
Alb11747 wants to merge 8 commits into
pingdotgg:mainfrom
Alb11747:fix/mcp-credential-per-provider

Alb11747 commented Oct 14, 2026 •

edited by juliusmarminge

Loading

Uh oh!

chatgpt-codex-connector Bot commented Oct 14, 2026 •

edited

Loading

Uh oh!

macroscopeapp Bot commented Oct 14, 2026

Uh oh!

coderabbitai Bot commented Oct 14, 2026 •

edited

Loading

Uh oh!

Alb11747 commented Oct 14, 2026

Uh oh!

juliusmarminge commented Nov 11, 2026

Uh oh!

bompus commented Nov 11, 2026

Uh oh!

t3dotgg commented Nov 12, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Alb11747 commented Oct 14, 2026 • edited by juliusmarminge Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

macroscopeapp Bot commented Oct 14, 2026

Approvability

Uh oh!

coderabbitai Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Alb11747 commented Oct 14, 2026

Uh oh!

juliusmarminge commented Nov 11, 2026

Uh oh!

bompus commented Nov 11, 2026

Uh oh!

t3dotgg commented Nov 12, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Alb11747 commented Oct 14, 2026 •

edited by juliusmarminge

Loading

chatgpt-codex-connector Bot commented Oct 14, 2026 •

edited

Loading

coderabbitai Bot commented Oct 14, 2026 •

edited

Loading