sdk: Expose githubMcpToolConfig across languages - #2112
Conversation
Adds the `githubMcpToolConfig` session option to `session.create` and `session.resume` in all six SDKs, forwarding the built-in GitHub MCP server settings the runtime already accepts: `enableAllTools`, `additionalToolsets`, `additionalTools`, `enableInsidersMode`, and the new `disableFormDeferral`. `disableFormDeferral` lets autonomous SDK workflows opt out of MCP App form deferral, so form-backed GitHub write tools execute directly instead of returning an awaiting-form stub. It requires MCP Apps to be enabled for the session. The option is omitted from the wire payload entirely when unset, so existing consumers see no change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Exposes the runtime’s githubMcpToolConfig option on session create/resume across multiple SDKs so callers can configure the built-in GitHub MCP server (notably disableFormDeferral) without dropping to raw payloads.
Changes:
- Added a GitHub MCP tool config type in each SDK and threaded it through
session.create/session.resume. - Ensured the option is omitted from wire payloads when unset (with tests in each language).
- Added language-specific helpers/builders/serialization to map SDK shapes to the runtime wire format.
Show a summary per file
| File | Description |
|---|---|
| rust/src/wire.rs | Adds github_mcp_tool_config to create/resume wire structs with omit-when-None behavior. |
| rust/src/types.rs | Introduces GitHubMcpToolConfig + builders; wires it through session configs and adds serde tests. |
| python/test_client.py | Adds pytest coverage verifying create/resume forwarding of githubMcpToolConfig. |
| python/copilot/session.py | Adds GitHubMcpToolConfig TypedDict for the public Python API. |
| python/copilot/client.py | Adds github_mcp_tool_config kwargs + wire conversion helper and forwards in payload. |
| python/copilot/init.py | Re-exports GitHubMcpToolConfig from the package root. |
| nodejs/test/client.test.ts | Adds tests for forwarding and omitting githubMcpToolConfig. |
| nodejs/src/types.ts | Adds GitHubMcpToolConfig interface and exposes it on SessionConfigBase. |
| nodejs/src/index.ts | Exports GitHubMcpToolConfig from package root. |
| nodejs/src/client.ts | Forwards githubMcpToolConfig in create/resume requests. |
| java/src/test/java/com/github/copilot/SessionRequestBuilderTest.java | Adds create/resume + omission serialization coverage for githubMcpToolConfig. |
| java/src/main/java/com/github/copilot/rpc/SessionConfig.java | Adds GitHubMcpToolConfig to session config + accessors + clone propagation. |
| java/src/main/java/com/github/copilot/rpc/ResumeSessionRequest.java | Adds githubMcpToolConfig JSON field to resume request. |
| java/src/main/java/com/github/copilot/rpc/ResumeSessionConfig.java | Adds GitHubMcpToolConfig to resume config + accessors + clone propagation. |
| java/src/main/java/com/github/copilot/rpc/GitHubMcpToolConfig.java | New bean defining the GitHub MCP tool config wire shape. |
| java/src/main/java/com/github/copilot/rpc/CreateSessionRequest.java | Adds githubMcpToolConfig JSON field to create request. |
| java/src/main/java/com/github/copilot/SessionRequestBuilder.java | Threads config into create/resume request builders. |
| go/types.go | Adds GitHubMCPToolConfig and threads it into session config + request wire structs. |
| go/client_test.go | Adds JSON serialization tests for create/resume/omitted behavior. |
| go/client.go | Passes config through to create/resume request payloads. |
| dotnet/test/Unit/SerializationTests.cs | Adds serialization coverage for create/resume + omit-when-unset. |
| dotnet/src/Types.cs | Adds GitHubMcpToolConfig and deep-copies it in SessionConfigBase copy ctor. |
| dotnet/src/Client.cs | Threads config into internal create/resume request records. |
Review details
- Files reviewed: 23/23 changed files
- Comments generated: 6
- Review effort level: Low
- node: omit githubMcpToolConfig for null as well as undefined - python: document github_mcp_tool_config on create_session/resume_session - go: assert json.Unmarshal succeeds in the omitted-when-unset subtest - java: call the non-deprecated buildCreateRequest(config, sessionId) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Comments suppressed due to low confidence (4)
java/src/main/java/com/github/copilot/SessionRequestBuilder.java:179
- Both builders unconditionally call
setGitHubMcpToolConfig(...), including when the config value isnull. This relies on global Jackson settings (or class-level@JsonInclude) to keep the field omitted; if serialization settings change, this could start emitting\"githubMcpToolConfig\": nulland violate the API contract of omitting when unset. Prefer only calling the setter when non-null (or calling an explicitclear.../leaving it untouched) to make omission behavior robust.
request.setGitHubMcpToolConfig(config.getGitHubMcpToolConfig());
java/src/main/java/com/github/copilot/SessionRequestBuilder.java:304
- Both builders unconditionally call
setGitHubMcpToolConfig(...), including when the config value isnull. This relies on global Jackson settings (or class-level@JsonInclude) to keep the field omitted; if serialization settings change, this could start emitting\"githubMcpToolConfig\": nulland violate the API contract of omitting when unset. Prefer only calling the setter when non-null (or calling an explicitclear.../leaving it untouched) to make omission behavior robust.
request.setGitHubMcpToolConfig(config.getGitHubMcpToolConfig());
python/copilot/client.py:351
- The helper includes a field whenever the key exists, even if the value is
None, which would serialize asnullon the wire. Since the intent is to omit unset fields while still preserving explicitFalse, consider checkingvalue is not Nonebefore setting each output key (this still forwardsFalsecorrectly). This avoids sendingnullvalues that may be rejected or interpreted differently by the runtime.
def _github_mcp_tool_config_to_wire(config: Mapping[str, Any]) -> dict[str, Any]:
"""Convert a ``GitHubMcpToolConfig`` mapping to wire format."""
wire: dict[str, Any] = {}
if "enable_all_tools" in config:
wire["enableAllTools"] = config["enable_all_tools"]
if "additional_toolsets" in config:
wire["additionalToolsets"] = config["additional_toolsets"]
if "additional_tools" in config:
wire["additionalTools"] = config["additional_tools"]
if "enable_insiders_mode" in config:
wire["enableInsidersMode"] = config["enable_insiders_mode"]
if "disable_form_deferral" in config:
wire["disableFormDeferral"] = config["disable_form_deferral"]
return wire
python/test_client.py:533
- Python adds forwarding coverage for
github_mcp_tool_config, but there’s no test asserting the key is omitted when unset/None (which is part of the PR’s stated contract and is covered in the other SDKs). Add a pytest case that callscreate_session()/resume_session()withoutgithub_mcp_tool_configand asserts\"githubMcpToolConfig\"is absent from the captured params.
@pytest.mark.asyncio
async def test_create_and_resume_session_forward_github_mcp_tool_config(self):
client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH))
await client.start()
try:
captured = {}
async def mock_request(method, params, **kwargs):
captured[method] = params
if method in ("session.create", "session.resume"):
result = {"sessionId": params.get("sessionId") or "session-1"}
callback = kwargs.get("on_response_inline")
if callback is not None:
callback(result)
return result
return {}
client._client.request = mock_request
config = {
"enable_all_tools": True,
"additional_toolsets": ["repos"],
"additional_tools": ["get_issue"],
"enable_insiders_mode": True,
"disable_form_deferral": True,
}
session = await client.create_session(github_mcp_tool_config=config)
await client.resume_session(session.session_id, github_mcp_tool_config=config)
- Files reviewed: 23/23 changed files
- Comments generated: 0 new
- Review effort level: Low
There was a problem hiding this comment.
Review details
Comments suppressed due to low confidence (4)
python/test_client.py:545
- The new coverage validates forwarding on create/resume, but it doesn’t cover the “omitted-when-unset” behavior for Python (which the PR description claims for every language). Add a test that calls
create_session()/resume_session()withoutgithub_mcp_tool_configand assertsgithubMcpToolConfigis absent from the outgoing params.
@pytest.mark.asyncio
async def test_create_and_resume_session_forward_github_mcp_tool_config(self):
client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH))
await client.start()
try:
captured = {}
async def mock_request(method, params, **kwargs):
captured[method] = params
if method in ("session.create", "session.resume"):
result = {"sessionId": params.get("sessionId") or "session-1"}
callback = kwargs.get("on_response_inline")
if callback is not None:
callback(result)
return result
return {}
client._client.request = mock_request
config = {
"enable_all_tools": True,
"additional_toolsets": ["repos"],
"additional_tools": ["get_issue"],
"enable_insiders_mode": True,
"disable_form_deferral": True,
}
session = await client.create_session(github_mcp_tool_config=config)
await client.resume_session(session.session_id, github_mcp_tool_config=config)
expected = {
"enableAllTools": True,
"additionalToolsets": ["repos"],
"additionalTools": ["get_issue"],
"enableInsidersMode": True,
"disableFormDeferral": True,
}
assert captured["session.create"]["githubMcpToolConfig"] == expected
assert captured["session.resume"]["githubMcpToolConfig"] == expected
finally:
await client.force_stop()
rust/src/types.rs:5941
- The omission assertion only covers the create wire path. To match the “omitted-when-unset” behavior on both create and resume, add the equivalent assertion for
ResumeSessionConfigwhengithub_mcp_tool_configis not set (i.e., ensuregithubMcpToolConfigis absent in the resume wire JSON too).
let (unset_wire, _) = SessionConfig::default()
.into_wire(Some(SessionId::from("github-mcp-unset")))
.expect("default config has no duplicate handlers");
assert!(
serde_json::to_value(&unset_wire)
.unwrap()
.get("githubMcpToolConfig")
.is_none()
);
go/client_test.go:2425
- The “unset is omitted” test only checks
createSessionRequest. Add the same omission check forresumeSessionRequest(with justSessionIDset) to ensuregithubMcpToolConfigalso omits correctly on the resume payload.
t.Run("unset is omitted", func(t *testing.T) {
data, err := json.Marshal(createSessionRequest{})
if err != nil {
t.Fatalf("Failed to marshal: %v", err)
}
var payload map[string]any
if err := json.Unmarshal(data, &payload); err != nil {
t.Fatalf("Failed to unmarshal: %v", err)
}
if _, ok := payload["githubMcpToolConfig"]; ok {
t.Fatal("Expected githubMcpToolConfig to be omitted")
}
})
java/src/test/java/com/github/copilot/SessionRequestBuilderTest.java:980
- This test checks omission when unset for create only. Add an assertion that
buildResumeRequest("session-2", new ResumeSessionConfig())also serializes without"githubMcpToolConfig"to fully cover the resume omission path.
@Test
void githubMcpToolConfigIsMappedAndSerializedForCreateAndResume() throws Exception {
var config = new GitHubMcpToolConfig().setEnableAllTools(true).setAdditionalToolsets(List.of("repos"))
.setAdditionalTools(List.of("get_issue")).setEnableInsidersMode(true).setDisableFormDeferral(true);
var createRequest = SessionRequestBuilder.buildCreateRequest(new SessionConfig().setGitHubMcpToolConfig(config),
"session-1");
var resumeRequest = SessionRequestBuilder.buildResumeRequest("session-1",
new ResumeSessionConfig().setGitHubMcpToolConfig(config));
assertSame(config, createRequest.getGitHubMcpToolConfig());
assertSame(config, resumeRequest.getGitHubMcpToolConfig());
var mapper = JsonRpcClient.getObjectMapper();
assertTrue(mapper.writeValueAsString(createRequest).contains("\"githubMcpToolConfig\""));
assertTrue(mapper.writeValueAsString(resumeRequest).contains("\"githubMcpToolConfig\""));
assertFalse(
mapper.writeValueAsString(SessionRequestBuilder.buildCreateRequest(new SessionConfig(), "session-2"))
.contains("\"githubMcpToolConfig\""));
}
- Files reviewed: 23/23 changed files
- Comments generated: 0 new
- Review effort level: Low
MackinnonBuck
left a comment
There was a problem hiding this comment.
Looks great overall - if we could add some E2E tests for this, that would be awesome.
Exercise the writable built-in GitHub MCP endpoint from Node and Go so regressions that drop githubMcpToolConfig fail end to end. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
10c79b1 to
cdb0b9f
Compare
Replace snapshot-specific replay behavior with a reusable captured-request endpoint and assert the writable GitHub MCP route and headers from Node and Go E2E tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Suppressed comments (4)
dotnet/src/Types.cs:3040
- The copy constructor assigns collection expressions (
[.. other...]) intoIList<string>?properties, which will typically produce arrays. Arrays implementIList<T>but are fixed-size, so callers modifyingAdditionalToolsets/AdditionalToolsafter cloning can hitNotSupportedException. Prefer copying into a mutableList<string>(or align the property types toIReadOnlyList<string>if immutability is intended).
GitHubMcpToolConfig = other.GitHubMcpToolConfig is null
? null
: new GitHubMcpToolConfig
{
EnableAllTools = other.GitHubMcpToolConfig.EnableAllTools,
AdditionalToolsets = other.GitHubMcpToolConfig.AdditionalToolsets is not null
? [.. other.GitHubMcpToolConfig.AdditionalToolsets]
: null,
AdditionalTools = other.GitHubMcpToolConfig.AdditionalTools is not null
? [.. other.GitHubMcpToolConfig.AdditionalTools]
: null,
EnableInsidersMode = other.GitHubMcpToolConfig.EnableInsidersMode,
DisableFormDeferral = other.GitHubMcpToolConfig.DisableFormDeferral,
};
java/src/main/java/com/github/copilot/SessionRequestBuilder.java:179
- The builder unconditionally calls
setGitHubMcpToolConfig(...)even when the config isnull. This makes omission behavior dependent on ObjectMapper null-handling. To guarantee the field is omitted when unset (and match the stated wire behavior), only call the setter whenconfig.getGitHubMcpToolConfig()is non-null (apply the same pattern inbuildResumeRequest).
if (config.isEnableMcpApps()) {
request.setRequestMcpApps(true);
}
request.setGitHubMcpToolConfig(config.getGitHubMcpToolConfig());
rust/src/types.rs:846
#[serde(default)]is redundant onOption<T>fields because Serde already treats missingOptionfields asNone. Removing thedefaultattribute reduces annotation noise while preserving the same serialization/deserialization behavior (theskip_serializing_ifis sufficient here).
#[serde(default, skip_serializing_if = "Option::is_none")]
pub enable_all_tools: Option<bool>,
/// Additional GitHub MCP toolsets to enable.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub additional_toolsets: Option<Vec<String>>,
/// Additional GitHub MCP tools to enable.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub additional_tools: Option<Vec<String>>,
/// Whether GitHub MCP insiders mode is enabled.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub enable_insiders_mode: Option<bool>,
rust/src/types.rs:851
#[serde(default)]is redundant onOption<T>fields because Serde already treats missingOptionfields asNone. Removing thedefaultattribute reduces annotation noise while preserving the same serialization/deserialization behavior (theskip_serializing_ifis sufficient here).
#[serde(default, skip_serializing_if = "Option::is_none")]
pub disable_form_deferral: Option<bool>,
- Files reviewed: 27/27 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
Review details
Suppressed comments (4)
java/src/main/java/com/github/copilot/SessionRequestBuilder.java:179
- The builder unconditionally calls
setGitHubMcpToolConfig(...), even when the value isnull. This makes omission of the field depend on ObjectMapper null-handling configuration and is less robust than explicitly setting the field only when non-null. Consider guarding these assignments (create + resume) with aconfig.getGitHubMcpToolConfig() != nullcheck so the request object stays "unset" when not provided.
if (config.isEnableMcpApps()) {
request.setRequestMcpApps(true);
}
request.setGitHubMcpToolConfig(config.getGitHubMcpToolConfig());
java/src/main/java/com/github/copilot/SessionRequestBuilder.java:304
- The builder unconditionally calls
setGitHubMcpToolConfig(...), even when the value isnull. This makes omission of the field depend on ObjectMapper null-handling configuration and is less robust than explicitly setting the field only when non-null. Consider guarding these assignments (create + resume) with aconfig.getGitHubMcpToolConfig() != nullcheck so the request object stays "unset" when not provided.
if (config.isEnableMcpApps()) {
request.setRequestMcpApps(true);
}
request.setGitHubMcpToolConfig(config.getGitHubMcpToolConfig());
go/internal/e2e/testharness/proxy.go:213
GetRequests()decodes the response body without checkingresp.StatusCode. If the proxy returns a non-200 (or an HTML/error payload), decoding will fail with a low-signal error. Consider checking forresp.StatusCode == http.StatusOKand returning a clearer error (ideally including the status and a short response body excerpt).
// GetRequests retrieves all captured outbound HTTP requests from the proxy.
func (p *CapiProxy) GetRequests() ([]CapturedRequest, error) {
p.mu.Lock()
url := p.proxyURL
p.mu.Unlock()
if url == "" {
return nil, fmt.Errorf("proxy not started")
}
resp, err := http.Get(url + "/requests")
if err != nil {
return nil, fmt.Errorf("failed to get requests: %w", err)
}
defer resp.Body.Close()
var requests []CapturedRequest
if err := json.NewDecoder(resp.Body).Decode(&requests); err != nil {
return nil, fmt.Errorf("failed to decode requests: %w", err)
}
return requests, nil
}
nodejs/test/e2e/harness/CapiProxy.ts:128
getRequests()assumes the response is always JSON and does not verifyresponse.ok. When the proxy is stopped or returns an error, this will throw a less actionable JSON parsing error. Consider checkingresponse.ok(and possibly content-type) and throwing a clearer error that includesresponse.status(and optionally response text).
async getRequests(): Promise<CapturedRequest[]> {
const response = await fetch(`${this.proxyUrl}/requests`, { method: "GET" });
return await response.json();
}
- Files reviewed: 30/30 changed files
- Comments generated: 0 new
- Review effort level: Lite
MRayermannMSFT
left a comment
There was a problem hiding this comment.
E2E tests were added as requested by @MackinnonBuck . 👍
Why
The runtime accepts a
githubMcpToolConfigoption onsession.create/session.resumeto configure the built-in GitHub MCP server, but no SDK exposed it, so SDK consumers could not reach it.The motivating case is the new
disableFormDeferralcontrol. MCP Apps are useful, but having issue and pull request tools open an interactive form instead of completing the requested operation is disruptive for autonomous SDK workflows, where the caller expects the tool to just run.disableFormDeferrallets those consumers keep MCP Apps result views while form-backed write tools execute directly.Companion changes:
mcp_apps_disable_form_deferralfeature flaggithubMcpToolConfig.disableFormDeferraland forwards the flags viaX-MCP-FeaturesWhat
Adds
githubMcpToolConfigto session create and resume across all six SDKs, carrying the settings the runtime already accepts:enableAllToolsbooladditionalToolsetsstring[]additionalToolsstring[]enableInsidersModebooldisableFormDeferralboolPer language:
GitHubMcpToolConfiginterface onSessionConfigBase, exported from the package rootGitHubMcpToolConfigTypedDictplus agithub_mcp_tool_configkeyword argument, converted by a_github_mcp_tool_config_to_wirehelper matching the existing_memory_to_wire/_tool_search_to_wireconventionGitHubMCPToolConfigstruct onSessionConfig/ResumeSessionConfigGitHubMcpToolConfigwith builder methods, wired throughSessionCreateWire/SessionResumeWireGitHubMcpToolConfigbean threaded throughSessionRequestBuilderGitHubMcpToolConfigclass onSessionConfigBase, including the copy constructordisableFormDeferralrequires MCP Apps to be enabled for the session; on its own it is inert. The whole option is omitted from the wire payload when unset, so existing consumers see no behavior change.Testing
Every language gets create/resume/omitted-when-unset coverage.
Run locally:
go build,go vet, andTestSessionRequests_GitHubMCPToolConfigpasscargo clippy --libcleantypecheck,format:check,lint(0 errors), and both new tests passNot run locally (no toolchain on this machine; relying on CI):
_github_mcp_tool_config_to_wirehelper was verified in isolation for the full, empty, and explicit-falsecases, butpytestneeds Python 3.11+ /uv