Skip to content

feat: add mcpOAuthTokenStorage support across all SDKs#1326

Merged
MackinnonBuck merged 16 commits into
mainfrom
mackinnonbuck/per-session-mcp-oauth-sdk
May 28, 2026
Merged

feat: add mcpOAuthTokenStorage support across all SDKs#1326
MackinnonBuck merged 16 commits into
mainfrom
mackinnonbuck/per-session-mcp-oauth-sdk

Conversation

@MackinnonBuck
Copy link
Copy Markdown
Collaborator

@MackinnonBuck MackinnonBuck commented May 18, 2026

Motivation

The runtime now accepts mcpOAuthTokenStorage on session.create and session.resume requests to control whether MCP OAuth tokens use the OS keychain or an in-memory store. SDK consumers need this property exposed so they can opt into safe multitenant behavior where tokens are isolated per session.

Part of https://github.com/github/copilot-agent-runtime/issues/7151

Approach

Adds mcpOAuthTokenStorage to the session config types and wires it through to the JSON-RPC request payload in all five language SDKs. The SDK default is "in-memory" (not "persistent") -- this intentionally differs from the runtime default to ensure multitenant consumers get isolation by default.

Per-SDK details:

  • Node.js -- property on SessionConfig interface with "persistent" | "in-memory" union type; added to ResumeSessionConfig Pick type; applied with ?? "in-memory" in both payload builders
  • Python -- Literal["persistent", "in-memory"] field in both TypedDicts; keyword-only parameter with docstring in both client methods; applied with or "in-memory"
  • Go -- MCPOAuthTokenStorage string on public config structs and internal wire request structs (follows Go initialism convention, consistent with MCPServers); default applied in client wiring with empty-string check
  • .NET -- McpOAuthTokenStorageMode enum using [JsonConverter(typeof(JsonStringEnumConverter<T>))] + [JsonStringEnumMemberName] (matching established pattern); property on both config classes with copy constructors; wire records and pass-through updated; added to [JsonSerializable] context
  • Rust -- Option<String> field with #[serde(skip_serializing_if)]; builder methods; Default / new set Some("in-memory"); Debug impls updated

Tests

  • Rust: Default and builder composition assertions in existing type tests
  • .NET: McpOAuthTokenStorage added to SessionConfig_Clone_CopiesAllProperties test
  • Go: Wire serialization tests for both createSessionRequest and resumeSessionRequest (verifies correct JSON key and omitempty behavior)

Notes for reviewers

  • The property is always sent on the wire (never omitted when defaulted), so the runtime always receives an explicit value from the SDK.

MackinnonBuck and others added 2 commits May 18, 2026 12:40
Add the mcpOAuthTokenStorage protocol property to session creation and
resume flows in all five language SDKs (Node.js, Python, Go, .NET, Rust).

When set to "in-memory", the runtime uses an in-memory MCP OAuth token
store instead of the OS keychain. The SDK defaults to "in-memory" for
safe multitenant behavior.

- Node.js: Add to SessionConfig interface and ResumeSessionConfig Pick type
- Python: Add to both TypedDicts and client methods with docstrings
- Go: Add to config structs, wire request structs, and client wiring
- .NET: Add McpOAuthTokenStorageMode enum with JsonStringEnumConverter,
  update config classes, copy constructors, wire records, and serialization
  context
- Rust: Add field, builder methods, Default/new impls, and Debug impls

Tests:
- Rust: Assert defaults and builder composition in existing type tests
- .NET: Add property to SessionConfig_Clone_CopiesAllProperties test
- Go: Add wire serialization tests for both request types

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow Go naming convention for initialisms (consistent with MCPServers).
Also fixes JSON tags that were accidentally changed from camelCase wire
format during the rename.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@MackinnonBuck MackinnonBuck marked this pull request as ready for review May 18, 2026 19:49
@MackinnonBuck MackinnonBuck requested a review from a team as a code owner May 18, 2026 19:49
Copilot AI review requested due to automatic review settings May 18, 2026 19:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR exposes a new session configuration option, mcpOAuthTokenStorage, across all SDKs so callers can choose whether MCP OAuth tokens are stored persistently (OS keychain) or isolated in-memory per session, defaulting to "in-memory" for safer multitenant usage.

Changes:

  • Adds mcpOAuthTokenStorage to session create/resume config types in Rust, Python, Node.js, Go, and .NET.
  • Wires the value through to the JSON-RPC payload builders in each SDK, defaulting to "in-memory" when not provided.
  • Extends Go/.NET/Rust tests to cover defaulting, cloning, and JSON key serialization.
Show a summary per file
File Description
rust/src/types.rs Adds mcp_oauth_token_storage fields + builders + defaults for create/resume configs and extends Rust type tests.
python/copilot/session.py Extends SessionConfig / ResumeSessionConfig TypedDicts with mcp_oauth_token_storage Literal type.
python/copilot/client.py Adds mcp_oauth_token_storage parameters to create_session/resume_session and always sends mcpOAuthTokenStorage with default.
nodejs/src/types.ts Adds mcpOAuthTokenStorage to SessionConfig and includes it in ResumeSessionConfig Pick.
nodejs/src/client.ts Always sends mcpOAuthTokenStorage on create/resume payloads, defaulting to "in-memory".
go/types.go Adds MCPOAuthTokenStorage to public configs and wire request structs (JSON key mcpOAuthTokenStorage).
go/client.go Defaults MCPOAuthTokenStorage to "in-memory" when unset, for both create and resume requests.
go/client_test.go Adds serialization tests for mcpOAuthTokenStorage JSON key presence/omission behavior.
dotnet/test/Unit/CloneTests.cs Ensures McpOAuthTokenStorage is copied by SessionConfig clone/copy constructor test.
dotnet/src/Types.cs Introduces McpOAuthTokenStorageMode enum and adds config properties for create/resume configs.
dotnet/src/Client.cs Wires McpOAuthTokenStorage through create/resume request records and serialization context.

Copilot's findings

  • Files reviewed: 11/11 changed files
  • Comments generated: 2

Comment thread go/client_test.go
Comment thread go/client_test.go Outdated
@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by SDK Consistency Review Agent for issue #1326 · ● 968.2K

Comment thread nodejs/src/client.ts Outdated
Comment thread python/copilot/client.py Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

MackinnonBuck and others added 2 commits May 18, 2026 13:27
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

…sion-mcp-oauth-sdk

# Conflicts:
#	dotnet/src/Types.cs
#	nodejs/src/client.ts
#	nodejs/src/types.ts
#	python/copilot/client.py
#	python/copilot/session.py
#	rust/src/types.rs
Comment thread python/test_client.py Fixed
Comment thread python/test_client.py Fixed
Comment thread python/test_client.py Fixed
Comment thread python/test_client.py Fixed
@github-actions

This comment has been minimized.

…orage tests

Replace SubprocessConfig positional arg (removed in main) with
keyword-only connection=RuntimeConnection.for_stdio(path=CLI_PATH),
matching the pattern used by all other tests in the file.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

Add mcpOAuthTokenStorage field to SessionConfig, ResumeSessionConfig,
CreateSessionRequest, and ResumeSessionRequest. The SessionRequestBuilder
defaults to "in-memory" for safe multitenant behavior, consistent with
all other SDK implementations.

Includes 6 unit tests covering default and explicit value forwarding for
both create and resume paths, plus null config handling.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

…sion-mcp-oauth-sdk

# Conflicts:
#	go/types.go
@github-actions

This comment has been minimized.

Update the Python MCP OAuth request interception tests to forward keyword arguments introduced by the merged create_session path, and add focused .NET clone tests so McpOAuth coverage can be validated directly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

MackinnonBuck and others added 2 commits May 28, 2026 11:28
Previously, all SDKs unconditionally defaulted mcpOAuthTokenStorage to
"in-memory". With the client mode system from PR #1428, the safe
multitenant default now only applies in "empty" mode. In "copilot-cli"
mode, the property is not sent, letting the runtime default to
"persistent" (backward compatible).

Changes across all 6 SDKs:
- Node.js: configDefaultsForMode() sets default for empty mode only
- Python: _mcp_oauth_token_storage_default() helper checks mode
- Go: applyConfigDefaultsForMode/applyResumeDefaultsForMode set default
- .NET: ApplyConfigDefaultsForMode() uses ??= for empty mode
- Rust: Default impls return None; session.rs applies for empty mode
- Java: CopilotClient applies default in empty mode blocks

Tests updated to verify mode-specific behavior in all SDKs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sion-mcp-oauth-sdk

# Conflicts:
#	java/src/test/java/com/github/copilot/SessionRequestBuilderTest.java
@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by SDK Consistency Review Agent for issue #1326 · ● 5.8M

Comment thread rust/src/types.rs Outdated
The doc comments incorrectly stated the default came from
SessionConfig::default(). Updated to reflect that the in-memory default
is applied at the client level when ClientMode::Empty is active.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

Use Literal type instead of str to match the parameter type in client.py,
fixing the red-knot invalid-assignment diagnostic.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

Empty mode requires base_directory to avoid falling back to ~/.copilot.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅

This PR adds mcpOAuthTokenStorage consistently across all six SDK implementations. Here's a summary of the cross-SDK alignment:

SDK Config type Default applied Wire field
Node.js SessionConfigBase.mcpOAuthTokenStorage?: "persistent" | "in-memory" "in-memory" in configDefaultsForMode() for "empty" mode mcpOAuthTokenStorage
Python mcp_oauth_token_storage: Literal["persistent", "in-memory"] | None "in-memory" via _mcp_oauth_token_storage_default() for "empty" mode mcpOAuthTokenStorage
Go MCPOAuthTokenStorage string on config structs "in-memory" with empty-string check in client wiring for empty mode mcpOAuthTokenStorage
.NET McpOAuthTokenStorageMode enum with [JsonStringEnumMemberName] McpOAuthTokenStorageMode.InMemory via ??= in ApplyConfigDefaultsForMode() mcpOAuthTokenStorage
Rust Option<String> field with serde skip Some("in-memory") in Default/new mcpOAuthTokenStorage
Java String mcpOAuthTokenStorage on SessionConfig/ResumeSessionConfig "in-memory" applied in CopilotClient when getMcpOAuthTokenStorage() == null in empty mode mcpOAuthTokenStorage

All SDKs:

  • Expose the property on both create and resume config types
  • Apply the same "in-memory" default for empty mode (SDK default intentionally differs from runtime default for safe multitenant behavior)
  • Pass the value through to the wire payload
  • Include tests covering the default and explicit override cases

No cross-SDK inconsistencies found. 🎉

Generated by SDK Consistency Review Agent for issue #1326 · ● 4.5M ·

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants