SDK: Align canvas with codegen pipeline, add e2e tests#1413
SDK: Align canvas with codegen pipeline, add e2e tests#1413SteveSandersonMS wants to merge 7 commits into
Conversation
Replace hand-wired canvas provider request handlers with the codegen-generated CanvasHandler interface from the client session API pipeline. - Remove manual canvas.open/close/invokeAction onRequest handlers from client.ts - Remove dispatchCanvasProviderRequest and manual type guards from canvas.ts - Wire canvas dispatch through generated CanvasHandler in session.registerCanvases() - Replace manual CanvasOpenResponse/CanvasHostContext/CanvasOpenContext/ CanvasActionContext/CanvasLifecycleContext/CanvasJsonSchema with re-exports from codegen (single source of truth from runtime Zod schemas) - Add 4 canvas e2e tests: list, open, invokeAction, close lifecycle Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Replace hand-wired canvas RPC handlers with codegen-generated CanvasHandler across all four remaining SDK languages, matching the Node implementation: - Replace manual context/response types with codegen type aliases - Remove hand-wired canvas.open/close/action.invoke handlers - Wire canvas dispatch through ClientSessionApiHandlers - Add canvas e2e tests (list, open, invokeAction, close) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1413 · ● 5.3M
There was a problem hiding this comment.
Pull request overview
This PR migrates canvas provider callbacks (canvas.* server→client requests) away from hand-wired dispatch and into the codegen “client session API handlers” pipeline, and adds new canvas E2E coverage across SDKs.
Changes:
- Replace manual canvas request wiring/dispatch with codegen-generated
CanvasHandler/register*ClientSessionApiHandlersplumbing (Node/Python/Go/.NET) and new Rustcanvas_dispatchmodule. - Replace hand-maintained canvas wire-contract types with codegen re-exports / aliases across SDKs.
- Add new canvas E2E tests + a new replay snapshot scenario.
Show a summary per file
| File | Description |
|---|---|
| test/snapshots/canvas/canvas_list_discovers_declared_canvases.yaml | Adds replay snapshot baseline for canvas scenario. |
| rust/tests/session_test.rs | Updates provider callback method name to canvas.invokeAction. |
| rust/tests/e2e/canvas.rs | Adds Rust canvas E2E test for listing declared canvases. |
| rust/tests/e2e.rs | Registers new Rust canvas E2E module. |
| rust/src/types.rs | Updates docs to reflect canvas.invokeAction naming. |
| rust/src/session.rs | Routes canvas.* requests to centralized dispatch module. |
| rust/src/lib.rs | Registers new internal canvas_dispatch module. |
| rust/src/generated/api_types.rs | Adds generated canvas rpc method constants and provider request/result types. |
| rust/src/canvas.rs | Replaces manual canvas types with generated aliases/re-exports. |
| rust/src/canvas_dispatch.rs | New internal dispatcher for provider-side canvas.* inbound requests. |
| python/test_canvas.py | Updates unit tests to validate codegen type aliases + adapter wiring. |
| python/e2e/test_canvas_e2e.py | Adds Python canvas E2E coverage (list/open/invoke/close). |
| python/copilot/session.py | Registers codegen canvas handler adapter on sessions. |
| python/copilot/client.py | Removes manual canvas.* request handlers in favor of codegen registration. |
| python/copilot/canvas.py | Switches public canvas context/response types to codegen aliases and trims manual helpers/types. |
| python/copilot/init.py | Updates public exports for new canvas type names/aliases. |
| nodejs/test/e2e/canvas.e2e.test.ts | Adds Node canvas E2E tests (list/open/invoke/close). |
| nodejs/test/client.test.ts | Updates unit tests to use clientSessionApis.canvas instead of direct dispatch helpers. |
| nodejs/src/session.ts | Wires clientSessionApis.canvas adapter from registered canvases. |
| nodejs/src/generated/rpc.ts | Adds codegen canvas provider request/result types + handler registrations. |
| nodejs/src/client.ts | Removes manual connection.onRequest(canvas.*) wiring and type guards; relies on codegen registration. |
| nodejs/src/canvas.ts | Re-exports codegen canvas wire-contract types; removes manual duplicates. |
| go/types.go | Updates canvas method naming; removes ExtensionInfo from session configs. |
| go/session.go | Adds Go canvas client-session adapter implementing generated rpc.CanvasHandler. |
| go/rpc/zrpc.go | Adds generated canvas provider request types + handler registration for canvas.*. |
| go/internal/e2e/canvas_e2e_test.go | Adds Go canvas E2E coverage in a single end-to-end test. |
| go/client.go | Removes manual canvas.* request handlers and stops sending extensionInfo in create/resume. |
| go/canvas.go | Replaces manual canvas wire-contract types with generated aliases; removes old helper structs. |
| go/canvas_test.go | Updates Go unit tests to validate new adapter-based dispatch and JSON round-trip. |
| dotnet/test/Unit/CanvasTests.cs | Updates unit tests to use generated canvas provider types. |
| dotnet/test/E2E/CanvasE2ETests.cs | Adds .NET canvas E2E tests for list/open/invoke/close. |
| dotnet/src/Types.cs | Updates docs and JSON source-gen registration for generated canvas types. |
| dotnet/src/Session.cs | Installs generated canvas handler adapter via ClientSessionApis.Canvas. |
| dotnet/src/Generated/Rpc.cs | Adds generated canvas provider request/result types + ClientSessionApiHandlers.Canvas. |
| dotnet/src/Client.cs | Removes manual canvas.* RPC methods; relies on codegen registration. |
| dotnet/src/Canvas.cs | Removes manual canvas context/response types; updates ICanvasHandler to use generated request/result types. |
Copilot's findings
Files not reviewed (1)
- go/rpc/zrpc.go: Language not supported
Comments suppressed due to low confidence (1)
python/copilot/canvas.py:43
CanvasHostCapabilitieswas removed from the public surface without a backwards-compatible alias. Other SDKs keep the old name as an alias to the generatedCanvasHostContextCapabilitiestype, so existing users importingCanvasHostCapabilitieswill now break. Consider addingCanvasHostCapabilities = CanvasHostContextCapabilitiesand re-exporting it in__all__(andcopilot/__init__.py) to preserve the public API while still using the codegen type.
from .generated.rpc import (
CanvasAction,
CanvasHostContext,
CanvasHostContextCapabilities,
CanvasJsonSchema,
CanvasProviderCloseRequest,
CanvasProviderInvokeActionRequest,
CanvasProviderOpenRequest,
CanvasProviderOpenResult,
OpenCanvasInstance,
)
__all__ = [
"CanvasAction",
"CanvasActionContext",
"CanvasDeclaration",
"CanvasError",
"CanvasHandler",
"CanvasHostContext",
"CanvasHostContextCapabilities",
"CanvasJsonSchema",
"CanvasLifecycleContext",
"CanvasOpenContext",
"CanvasOpenResponse",
"ExtensionInfo",
"OpenCanvasInstance",
]
- Files reviewed: 32/37 changed files
- Comments generated: 7
| async def open(self, params: CanvasProviderOpenRequest) -> CanvasProviderOpenResult: | ||
| try: | ||
| return await self._handler.on_open(params) | ||
| except CanvasError as err: | ||
| raise JsonRpcError(-32603, err.message, data=err.to_envelope()) from err | ||
|
|
||
| async def close(self, params: CanvasProviderCloseRequest) -> None: | ||
| try: | ||
| await self._handler.on_close(params) | ||
| except CanvasError as err: | ||
| raise JsonRpcError(-32603, err.message, data=err.to_envelope()) from err | ||
|
|
||
| async def invoke_action(self, params: CanvasProviderInvokeActionRequest) -> Any: | ||
| try: | ||
| return await self._handler.on_action(params) | ||
| except CanvasError as err: | ||
| raise JsonRpcError(-32603, err.message, data=err.to_envelope()) from err |
There was a problem hiding this comment.
Not addressing — this is a pre-existing behavior and the other SDKs don't wrap unexpected exceptions with a structured envelope either.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…(Node) The TypeScript codegen was computing groupExperimental but not using it to add @experimental JSDoc to handler interfaces. C#, Python, and Go already did this correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1413 · ● 6.5M
This comment has been minimized.
This comment has been minimized.
Adds 3 new e2e tests matching the Node/C# canvas test coverage: - canvas_open_round_trip: verifies server->client open dispatch and response - canvas_invoke_action_round_trip: verifies action dispatch with input/result - canvas_close_round_trip: verifies close dispatch and list_open cleanup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
- Remove friendly type aliases (CanvasOpenContext, CanvasOpenResponse, etc.)
from all 5 languages; use codegen names (CanvasProviderOpenRequest,
CanvasProviderOpenResult, etc.) directly. Canvas is experimental and
days old, so no backward-compat concern.
- Fix C# codegen: opaque results (x-opaque-json) now emit 'object' instead
of a named wrapper type, preventing double-wrapping of invokeAction result.
- Fix Go codegen: opaque results now emit 'any' instead of a named struct
pointer, preventing the same double-wrapping issue.
- Restore accidentally removed ExtensionInfo from Go SessionConfig,
ResumeSessionConfig, and wire structs.
- Fix Go canvas.close adapter to return nil (null on wire) instead of
empty struct ({} on wire).
- Update all tests (unit + e2e) across all languages.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Resolved — the PR now covers all 5 SDKs consistently. The initial commit was Node-only; subsequent commits added C#, Python, Go, and Rust. |
|
Thanks — follow-up commit drops all aliases across all languages and fixes opaque result codegen for C# and Go. |
This comment has been minimized.
This comment has been minimized.
Node: catch CanvasError and generic exceptions in canvas adapter,
throw ResponseError with {code, message} data envelope.
Python: catch generic exceptions (not just CanvasError) in
_CanvasHandlerAdapter, wrap as canvas_handler_error envelope.
All 5 SDKs now consistently produce structured error envelopes
for canvas handler failures.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cross-SDK Consistency Review ✅This PR updates all five active SDK implementations (Node.js, Python, Go, .NET, Rust) in a consistent manner. The pattern is uniform across languages:
Java: Canvas support does not exist in the Java SDK (pre-existing gap, not introduced by this PR). E2E tests: Added for Node.js, Python, Go, and .NET. Rust already had canvas e2e tests prior to this PR and they use the codegen types, consistent with this change. No cross-SDK consistency concerns found.
|
Summary
Replaces hand-wired canvas provider request handlers with the codegen-generated
CanvasHandlerinterface from the client session API pipeline. Canvas server→client calls now flow through the same codegen machinery as sessionFs, eliminating manual type definitions and dispatch logic.Ship together with the companion runtime PR that adds canvas to the codegen schema.
What changed
Removed manual dispatch (client.ts)
connection.onRequest(canvas.*)registrationshandleCanvasProviderRequestandhandleCanvasActionInvokeRequestmethodsisCanvasProviderRequestParamsandisCanvasActionInvokeParamstype guardsAll replaced by the generated
registerClientSessionApiHandlerswhich routes canvas calls automatically.Replaced manual types with codegen re-exports (canvas.ts)
6 manually-defined types totaling ~40 fields have been replaced with re-exports from codegen:
CanvasJsonSchemaCanvasHostContextCanvasOpenResponseCanvasProviderOpenResultCanvasOpenContextCanvasProviderOpenRequestCanvasActionContextCanvasProviderInvokeActionRequestCanvasLifecycleContextCanvasProviderCloseRequestZero manual wire-contract type definitions remain. The public export names are preserved as aliases so downstream consumers don't need to change imports.
Simplified handler dispatch (session.ts)
registerCanvases()now populatesclientSessionApis.canvaswith aCanvasHandlerimplementation. Since the context types ARE the codegen request types, handler dispatch passesparamsdirectly — no manual field-by-field destructuring.New e2e tests
4 canvas e2e tests covering the full server→client round-trip:
session.canvas.listBehavioral changes
None. The public API surface (
createCanvas,Canvas, all exported types) is preserved. Theinputfield type on context types changes fromunknownto{ [k: string]: unknown } | undefined(matching the codegen output), which is a compatible narrowing.