Add typed installation confirmation callbacks - #2763
gokhanarkan wants to merge 3 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
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 = ( |
There was a problem hiding this comment.
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.
This comment has been minimized.
This comment has been minimized.
Cross-SDK Consistency ReviewScope of PR #2763: regenerates schemas/protocol code for all six SDKs (new Findings✅ Consistent (schema/protocol layer): The generated types for the new
This mirrors the existing Left inline comments on
|
There was a problem hiding this comment.
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…
Scope and current intake
Use SDK commit
00f9c9af275afd85ce7dceb769f341558fac4c21, tree5d201aaaf4741e9f66499bc36aef7556ae65d308. This is an intermediate dependency on #2762, based exactly on9bb64152153fa9618d6f1373f0195a6248db1621; that prerequisite branch and PR are unchanged.installations.confirmreceivers 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.Consumer compatibility corrections and acceptance
Both corrections are normal follow-up commits; earlier history is preserved.
9333ce94fixes the initial optional-trust regression in12be097b. 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.00f9c9affixes the nullable-reference regression exposed by actual App compilation on9333ce94(E0308before 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 toOption<T>. Missing/null/object handling and explicit required-fieldnullserialisation are covered without removing App'sNonebranches.Actual App catalogue compatibility acceptance passed on
00f9c9af. App commitf82331ae2cc1e2e71c3dd5f92e157ae35ef930db, tree245956fa517a4442800e88f0af95f97639f466eb, ran the full unchangedcustomize_catalog_searchsuite: 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 coreNonehandling are unchanged. Normal paired Rust/Node intake matched 195 immutable source blobs and retained genuine1.0.89-1snapshots.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, tree975e20f2a77f1945a9ac3e4b500fd236059cdaf6:api.schema.json11ccd2013cb9ceb6f9e61056d34e4b25f5905098dc455c8ba4862d6574e90e43session-events.schema.jsonb3bd26410b74e1ae25fcf8f00d3befce7765c2ea7a6a5a02334ea4e088e75d64Reproduction:
npm run generate -- --runtime-source checkout --schema-dir <immutable-schema-directory>.The original transport-choice fixture exactly matches selected genuine CLI
1.0.89-1definitions. Release pins/assets, protocol constants, dependency lockfiles and replay snapshots are unchanged.Validation
12be097b1.0.89-1, Node build/typecheck and format/lint.9333ce9400f9c9afNormal 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; missingJAVA_HOMEthrough 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's9333ce94compile failure preceded the successful00f9c9afacceptance 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,discoverandplanInstall. 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.