Skip to content

Add typed installation confirmation callbacks - #2763

Draft
gokhanarkan wants to merge 3 commits into
gokhanarkan/mcp-confirmation-sdk-handofffrom
gokhanarkan/typed-mcp-installation-sdk
Draft

gokhanarkan wants to merge 3 commits into
gokhanarkan/mcp-confirmation-sdk-handofffrom
gokhanarkan/typed-mcp-installation-sdk

Conversation

@gokhanarkan

@gokhanarkan gokhanarkan commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Scope and current intake

Use SDK commit 00f9c9af275afd85ce7dceb769f341558fac4c21, tree 5d201aaaf4741e9f66499bc36aef7556ae65d308. This is an intermediate dependency on #2762, based exactly on 9bb64152153fa9618d6f1373f0195a6248db1621; that prerequisite branch and PR are unchanged.

  • Repair generic Rust named-reference union generation for typed MCP package/remote choices and complete action-specific confirmation reviews, without handwritten DTO overlays.
  • Add nonblocking connection-global installations.confirm receivers to Rust and Node. Handlers return only explicit decisions; the SDK echoes the original challenge and fingerprint. Original operation/session metadata is never replaced by current-session guesses.
  • Distinguish numeric request cancellation from original connection closure and reject late results. Rust registers supported incoming request IDs synchronously before dispatch, preventing cancellation-before-registration races across separate channels.
  • Regenerate all six SDK projections through the normal facade and document the experimental receiver.

Consumer compatibility corrections and acceptance

Both corrections are normal follow-up commits; earlier history is preserved.

9333ce94 fixes the initial optional-trust regression in 12be097b. Previously opaque optional trust remains lossless raw JSON on catalogue candidates and installation reviews, allowing the host's existing bounded degradation policy to run. Required MCP choices remain typed and strict; standalone typed trust remains available after explicit host validation. No type-name exceptions or consumer casts.

00f9c9af fixes the nullable-reference regression exposed by actual App compilation on 9333ce94 (E0308 before tests). The producer defines context information, context attribution, workspace and subagent settings as nullable. A generic cycle-aware schema check preserves null through references, aliases and wrappers, restoring all seven affected fields to Option<T>. Missing/null/object handling and explicit required-field null serialisation are covered without removing App's None branches.

Actual App catalogue compatibility acceptance passed on 00f9c9af. App commit f82331ae2cc1e2e71c3dd5f92e157ae35ef930db, tree 245956fa517a4442800e88f0af95f97639f466eb, ran the full unchanged customize_catalog_search suite: 17 passed, 0 failed, 1 intentionally ignored child-harness entry used by the account-isolation parent. The log and source receipt were inspected. Malformed/future/overlimit trust degrades only metadata, inert rows remain, and bounds/redaction, authentication/account isolation, cancellation and the omitted sessionless selector pass. The original trust projector/assertions and core None handling are unchanged. Normal paired Rust/Node intake matched 195 immutable source blobs and retained genuine 1.0.89-1 snapshots.

This is targeted existing-consumer compatibility acceptance, not a complete installation/OAuth journey.

Producer provenance

Generated from the co-located schemas exported from committed github/copilot-agent-runtime@1a43282222653a8e73f2f67846f29467f32a6d2f, tree 975e20f2a77f1945a9ac3e4b500fd236059cdaf6:

Input SHA-256
api.schema.json 11ccd2013cb9ceb6f9e61056d34e4b25f5905098dc455c8ba4862d6574e90e43
session-events.schema.json b3bd26410b74e1ae25fcf8f00d3befce7765c2ea7a6a5a02334ea4e088e75d64

Reproduction: npm run generate -- --runtime-source checkout --schema-dir <immutable-schema-directory>.

The original transport-choice fixture exactly matches selected genuine CLI 1.0.89-1 definitions. Release pins/assets, protocol constants, dependency lockfiles and replay snapshots are unchanged.

Validation

Revision Focused proof
12be097b 217 Rust tests covering framed confirmation, concurrent/out-of-order decisions, cancellation-before-dispatch, late A/new B, peer loss, client-drop and ordinary callbacks; strict scoped Clippy, local-runtime check, all-feature rustdoc; 259 Node client/TCP/global/codegen tests with genuine prepared CLI 1.0.89-1, Node build/typecheck and format/lint.
9333ce94 Two causal optional-trust codegen reds, then 47 Rust and 59 light checks pass; strict scoped Clippy. Whole-search fixtures preserve malformed/future metadata and exact host-boundary payloads.
00f9c9af 51 Rust tests: API 23, catalogue 12, confirmation 10, choices 6; all seven nullable fields covered. Eight generic reference/wrapper/union codegen cases; 67 light checks, full Node source/test typecheck, introducing lint and pinned Rust formatting; strict scoped Clippy.

Normal all-six regeneration reproduces current output exactly: aggregate SHA-256 02e5fde8899919ab177d08ac9561c809129fd9d1a5ed2d2387a7511f1bb2faca. The nullable correction changes only seven Rust field declarations. Java uses an existing JDK 25 selected for the command only.

Causal failures are retained: missing Node runtime-package setup was resolved through documented prepare:runtime; missing JAVA_HOME through the existing JDK, with no pin/global changes. An initial TCP close fixture needed disposal of its already-closed server peer after observing the client's real closure signal. The initial nullable-codegen run exposed six semantic failures and one over-strict naming assertion; the control was corrected to preserve existing type-array naming. App's 9333ce94 compile failure preceded the successful 00f9c9af acceptance above.

Deliberate delivery limits

This remains a draft source-runtime dependency, not a complete installer or released-runtime capability claim. The producer registers bound search/planning and the global confirmation callback, but its MCP callable table still contains only config, discover and planInstall. Preparation, apply, removal/control and owned OAuth calls remain absent and are not invented here.

Full runtime-to-human-to-same-operation installation integration requires the later committed producer. Default pinned-schema freshness CI still requires a genuine runtime release containing this contract before normal release integration; this PR does not weaken that check or replace the published CLI pin. All-six generation does not mean all six languages have a functional installation receiver.

@github-actions

This comment has been minimized.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

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 #2763 · copilot · sonnet50 · 37 AIC · ⌖ 11.7 AIC · ⊞ 8.1K

* Registering this callback does not enable installation capabilities.
* @experimental
*/
export type InstallationConfirmationHandler = (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR adds a full connection-global "installation confirmation" feature (InstallationConfirmationHandler / installationConfirmationHandler client option, adapter wiring to installations.confirm) for Node.js and Rust, but the equivalent handler wiring is missing in Python, Go, .NET, and Java. Those SDKs did receive the regenerated InstallationConfirmationRequest/InstallationDecision/InstallationConfirmationResponse types (via the schema/codegen refresh), but there is no installation_confirmation_handler/InstallationConfirmationHandler option or adapter analogous to the existing extensionLaunchProvider/onGitHubTelemetry connection-global handler pattern used in those SDKs.

Since this is marked @experimental, it may be intentional to land it first in Node.js/Rust and follow up separately for the other four SDKs — but flagging it here so the follow-up isn't lost, since the pattern (client-global handler registered on the connection) is well established in Go/.NET/Java/Python for extensionLaunchProvider and onGitHubTelemetry.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review

Scope of PR #2763: regenerates schemas/protocol code for all six SDKs (new installations.confirm RPC, FusionAttribution, MCP catalog/plan install types, nullable-field preservation fixes, etc.) and adds an ergonomic, connection-global installation confirmation handler on top of the generated InstallationsHandler protocol.

Findings

✅ Consistent (schema/protocol layer): The generated types for the new installations.confirm RPC (InstallationConfirmationRequest/Response, InstallationDecision, InstallationReview, McpInstallationReview, catalog/MCP-plan types, FusionAttribution, etc.) are regenerated correctly across all six languages (nodejs/src/generated/rpc.ts, python/copilot/generated/rpc.py, go/rpc/zrpc.go, dotnet/src/Generated/Rpc.cs, java/sdk/src/generated/..., rust/src/generated/api_types.rs). Nullable-vs-optional schema handling and MCP plan transport discriminators look aligned across languages.

⚠️ Inconsistent (ergonomic API layer): The PR adds a higher-level, connection-global InstallationConfirmationHandler wrapper — with request/connection cancellation context, adapter wiring in the client, and CopilotClientOptions/ClientOptions fields — for Node.js (nodejs/src/installationConfirmation.ts, client.ts, types.ts) and Rust (rust/src/installation_confirmation.rs, ClientOptions::with_installation_confirmation_handler) only. Go, .NET, Java, and Python receive only the raw generated InstallationsHandler protocol interface with no equivalent ergonomic wrapper, option field, or client wiring.

This mirrors the existing extensionLaunchProvider connection-global handler feature, which is implemented consistently in all six SDKs (Go ExtensionLaunchProvider, .NET IExtensionLaunchProviderHandler, Java ExtensionLaunchProviderAdapter, Python extension_launch_provider), so there's a clear established pattern to follow for parity.

Left inline comments on nodejs/src/installationConfirmation.ts and rust/src/lib.rs with suggestions. Since the feature is explicitly marked @experimental, this may be an intentional phased rollout — if so, consider noting that in the PR description so reviewers/consumers of the other four SDKs aren't left wondering why the option is missing.

Generated by SDK Consistency Review Agent for #2763 · copilot · sonnet50 · 61.4 AIC · ⌖ 11.9 AIC · ⊞ 8.1K · ◷

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

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 #2763 · copilot · sonnet50 · 61.4 AIC · ⌖ 11.9 AIC · ⊞ 8.1K

Comments that could not be inline-anchored

nodejs/src/installationConfirmation.ts:52

This new ergonomic wrapper (createInstallationConfirmationAdapter + InstallationConfirmationHandler, plus the matching CopilotClientOptions.installationConfirmationHandler in client.ts/types.ts) is only added for Node.js and Rust (rust/src/installation_confirmation.rs, ClientOptions::with_installation_confirmation_handler). Go, .NET, Java, and Python only receive the generated low-level InstallationsHandler/installations.confirm protocol types (e.g. go/rpc/zrpc.go, `dotnet/s…

rust/src/lib.rs:22

Same cross-SDK parity gap noted on the Node.js side: installation_confirmation_handler / with_installation_confirmation_handler is added here (Rust) and in Node.js, but not in Go, .NET, Java, or Python. Those four SDKs only get the generated InstallationsHandler protocol interface, without an ergonomic connection-global wrapper or cancellation context analogous to InstallationConfirmationContext. Since extensionLaunchProvider (the closest precedent for a connection-global handler) has…

This branch has not been deployed

No deployments
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.

1 participant