Skip to content

Canvas SDK E2E tests across all 5 languages#1422

Open
jmoseley wants to merge 3 commits into
mainfrom
jmoseley/canvas-e2e-tests-all-sdks
Open

Canvas SDK E2E tests across all 5 languages#1422
jmoseley wants to merge 3 commits into
mainfrom
jmoseley/canvas-e2e-tests-all-sdks

Conversation

@jmoseley
Copy link
Copy Markdown
Contributor

Follow-up to #1401 (per stephentoub's review comment) adding real-runtime E2E coverage for canvas SDK support in Rust, Node, Python, Go, and .NET.

What's covered

Each language gains 5 tests against the bundled copilot-cli subprocess:

  1. canvas.open round-trip — host session.rpc.canvas.open → runtime → declaring provider's CanvasHandler.onOpen → result back to caller.
  2. canvas.action.invoke round-trip — per-action handler routed correctly; return value flows through verbatim.
  3. canvas.close round-trip — handler's onClose fires.
  4. canvas_action_no_handler — declaring an action without a matching handler surfaces the structured error code (where reachable through the language's API).
  5. resumeSession with openCanvases — rehydrated state observable via the session's openCanvases accessor.

Design

No CAPI traffic needed. Snapshots under test/snapshots/canvas/ are empty (conversations: []). The trigger is the host-side RPC surface — when an SDK declares a canvas and then calls session.rpc.canvas.*, the runtime dispatches back to that same SDK's handler, exercising the full loop deterministically.

Notable change

dotnet/src/Canvas.cs: CanvasErrorHelpers.ToRpcException now prefixes the error code in the message string. The .NET JSON-RPC client doesn't surface the JSON-RPC error.data payload to callers, so without this the structured code (e.g. canvas_action_no_handler) would be unobservable from the receiving side. Matches how the Rust SDK already exposes canvas error codes.

Validation

All passing locally:

  • Node: npx vitest run test/e2e/canvas.e2e.test.ts — 5/5
  • Python: uv run pytest e2e/test_canvas_e2e.py — 5/5
  • Go: go test ./internal/e2e/ -run Canvas — 5/5
  • Rust: cargo test --features test-support --test e2e canvas — 5/5
  • .NET: dotnet test --filter "FullyQualifiedName~Canvas" — 14/14

Follow-up to #1401 (per stephentoub's review comment) adding real-runtime
E2E coverage for canvas SDK support in Rust, Node, Python, Go, and .NET.

Each language gains 5 tests exercising the full host ↔ runtime ↔ provider
loop against the bundled `copilot-cli` subprocess:

1. `canvas.open` round-trip — host-side `session.rpc.canvas.open`
   dispatches back to the declaring provider's `CanvasHandler`.
2. `canvas.action.invoke` round-trip — per-action handler is routed and
   its return value flows back to the caller verbatim.
3. `canvas.close` round-trip — handler's onClose is invoked.
4. `canvas_action_no_handler` — declaring an action without a handler
   surfaces the structured error code (where reachable).
5. `resumeSession` with `openCanvases` — rehydrated state is observable
   via the session's openCanvases accessor.

No CAPI traffic is required; snapshots under `test/snapshots/canvas/` are
empty (`conversations: []`) and the trigger is purely host-side RPC, so
the suites are deterministic.

.NET: `CanvasErrorHelpers.ToRpcException` now prefixes the error code in
the message string, because the .NET JSON-RPC client doesn't surface the
JSON-RPC `error.data` payload to callers — without this the structured
code (e.g. `canvas_action_no_handler`) is unobservable on the receiving
side. Matches how the Rust SDK already exposes canvas error codes.

Refs #1401 (comment)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 25, 2026 20:21
@jmoseley jmoseley requested a review from a team as a code owner May 25, 2026 20:21
Per review feedback, document why the canvas error code is prefixed
into the message string so future maintainers know the long-term fix
is to plumb error.data through RemoteRpcException.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jmoseley jmoseley requested a review from stephentoub May 25, 2026 20:23
Comment thread dotnet/test/E2E/CanvasE2ETests.cs Fixed
@github-actions

This comment has been minimized.

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

Adds deterministic, real-runtime end-to-end coverage for the Canvas SDK ↔ runtime dispatch loop across Rust, Node, Python, Go, and .NET, using the bundled copilot-cli subprocess and empty CAPI snapshots (no chat traffic) to validate the host-side session.rpc.canvas.* round-trips.

Changes:

  • Add new Canvas E2E test suites in Rust / Node / Python / Go / .NET covering open, invokeAction, close, missing-handler error, and resume seeding.
  • Add test/snapshots/canvas/*.yaml snapshot fixtures (models list + empty conversations) for the new tests across differing per-language snapshot name sanitization.
  • Update .NET canvas RPC error conversion to prefix the structured error code into the top-level exception message.
Show a summary per file
File Description
test/snapshots/canvas/seedsopencanvasesonresumefromruntime.yaml Empty snapshot fixture for a canvas resume-seeding scenario
test/snapshots/canvas/seeds_opencanvases_on_resume_from_the_runtime_resume_response.yaml Empty snapshot fixture for Node-style sanitized test name
test/snapshots/canvas/seeds_open_canvases_on_resume.yaml Empty snapshot fixture for Go-style subtest name
test/snapshots/canvas/seeds_open_canvases_on_resume_from_the_runtime_resume_response.yaml Empty snapshot fixture for Rust/Python-style test name
test/snapshots/canvas/returnscanvasactionnohandlerfordeclaredactionwithouthandler.yaml Empty snapshot fixture for .NET camelCase method name sanitization
test/snapshots/canvas/returns_canvas_action_no_handler.yaml Empty snapshot fixture for Go-style subtest name
test/snapshots/canvas/returns_canvas_action_no_handler_when_the_declared_action_has_no_handler.yaml Empty snapshot fixture for Node-style test name
test/snapshots/canvas/returns_canvas_action_no_handler_when_declared_action_has_no_handler.yaml Empty snapshot fixture for Rust/Python-style test name variant
test/snapshots/canvas/rejects_invokeaction_for_an_action_the_canvas_did_not_declare.yaml Empty snapshot fixture for Node “unknown action” E2E case
test/snapshots/canvas/dispatchescanvasopentoproviderhandler.yaml Empty snapshot fixture for .NET camelCase method name sanitization
test/snapshots/canvas/dispatchescanvasclosetoonclosehandler.yaml Empty snapshot fixture for .NET camelCase method name sanitization
test/snapshots/canvas/dispatchescanvasactioninvoketohandler.yaml Empty snapshot fixture for .NET camelCase method name sanitization
test/snapshots/canvas/dispatches_canvas_open.yaml Empty snapshot fixture for Go-style subtest name
test/snapshots/canvas/dispatches_canvas_open_to_the_provider_handler.yaml Empty snapshot fixture for Rust/Python/Node-style test name
test/snapshots/canvas/dispatches_canvas_close.yaml Empty snapshot fixture for Go-style subtest name
test/snapshots/canvas/dispatches_canvas_close_to_the_provider_onclose_handler.yaml Empty snapshot fixture for Node-style test name (“onClose”)
test/snapshots/canvas/dispatches_canvas_close_to_the_provider_on_close_handler.yaml Empty snapshot fixture for Rust/Python-style test name (“on_close”)
test/snapshots/canvas/dispatches_canvas_action_invoke.yaml Empty snapshot fixture for Go-style subtest name
test/snapshots/canvas/dispatches_canvas_action_invoke_to_the_per_action_handler.yaml Empty snapshot fixture for Rust/Python/Node-style test name
rust/tests/e2e/canvas.rs New Rust Canvas E2E coverage for dispatch + resume seeding
rust/tests/e2e.rs Registers the new Rust canvas E2E module
python/e2e/test_canvas_e2e.py New Python Canvas E2E test module covering dispatch + resume seeding
nodejs/test/e2e/canvas.e2e.test.ts New Node Canvas E2E test module covering dispatch + resume seeding
go/internal/e2e/canvas_e2e_test.go New Go Canvas E2E test suite covering dispatch + resume seeding
dotnet/test/E2E/CanvasE2ETests.cs New .NET Canvas E2E tests covering dispatch + resume seeding
dotnet/src/Canvas.cs Adjusts .NET canvas error-to-RPC conversion to prefix code into message

Copilot's findings

Comments suppressed due to low confidence (1)

dotnet/src/Canvas.cs:214

  • ToRpcException now prefixes the code into the JSON-RPC message, but Build() reuses that same string for the structured error.data.message payload. If the goal is only to make the code observable to .NET callers (who don't surface error.data), consider keeping error.data.message as the original CanvasError.Message and only prefixing the top-level exception message (e.g., by letting Build accept separate values for RPC message vs payload message). This preserves the structured payload semantics and keeps it closer to the other SDKs’ {code,message} envelope.
    // Code is prefixed into the message because RemoteRpcException does not currently
    // surface the JSON-RPC error.data payload to callers, so the structured code (e.g.
    // "canvas_action_no_handler") would otherwise be unobservable on the receiving side.
    // TODO: plumb error.data through RemoteRpcException and drop the prefix here.
    public static LocalRpcInvocationException ToRpcException(CanvasError error) => Build(error.Code, $"{error.Code}: {error.Message}");

    private static LocalRpcInvocationException Build(string code, string message)
    {
        var json = JsonSerializer.Serialize(
  • Files reviewed: 26/26 changed files
  • Comments generated: 5

Comment thread dotnet/test/E2E/CanvasE2ETests.cs
Comment thread nodejs/test/e2e/canvas.e2e.test.ts
Comment thread python/e2e/test_canvas_e2e.py Outdated
Comment thread go/internal/e2e/canvas_e2e_test.go
Comment thread go/internal/e2e/canvas_e2e_test.go
Address review feedback on PR #1422:
- Dispose every created/resumed session so the shared E2E client doesn't
  leak runtime resources across tests and so per-test workdir cleanup is
  not racing live sessions:
  - .NET: `await using var session = ...` for create + resume
  - Node: `try/finally` with `await session.disconnect()`
  - Python: `try/finally` with `await session.disconnect()`
  - Go: `t.Cleanup(func() { _ = session.Disconnect() })` inside the
    shared `createCanvasSession` helper and on the resumed session
- .NET: extract `result.Result` into a local and use the null-forgiving
  operator so the nullable dereference warning at line 53 is silenced
  without changing behavior.

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

Cross-SDK Consistency Review ✅

This PR maintains excellent cross-SDK consistency across the 5 languages that support the canvas API (Node.js, Python, Go, .NET, Rust):

  • Test parity: All 5 SDKs gain the same 5 E2E test scenarios (canvas open, action invoke, close, no-handler error, resume with openCanvases).
  • Production change (dotnet/src/Canvas.cs): The ToRpcException fix is a .NET-specific workaround for the StreamJsonRpc library not surfacing error.data to callers. The PR description correctly notes this mirrors how Rust already exposes error codes. No equivalent change is needed in Node, Python, or Go since those runtimes surface structured error data natively.

Pre-existing gap (outside scope of this PR)

The Java SDK has no canvas support at all — no Canvas.java, no canvas handlers, no E2E tests. This is a pre-existing gap not introduced by this PR. If canvas support is planned for Java, a follow-up issue to track that work would be useful.

Generated by SDK Consistency Review Agent for issue #1422 · ● 6.9M ·

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.

2 participants