Skip to content

Canvas SDK E2E tests across all 5 languages#1422

Closed
jmoseley wants to merge 4 commits into
mainfrom
jmoseley/canvas-e2e-tests-all-sdks
Closed

Canvas SDK E2E tests across all 5 languages#1422
jmoseley wants to merge 4 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

This comment has been minimized.

Resolved add/add conflicts in canvas E2E test files by keeping this PR's
test suite (the 5 tests addressing stephentoub's review feedback: session
disposal + nullable fix). PR #1413 (which landed on main) added a different
set of canvas E2E tests focused on `session.canvas.list` API; those remain
on main without duplication here.

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

Cross-SDK Consistency Review ✅

Reviewed for feature parity and API consistency across Node.js, Python, Go, .NET, and Rust.

All 5 languages have E2E coverage for the same 5 canvas scenarios (open round-trip, action invoke, close, error surfacing, resumeSession with openCanvases).

Notable change: dotnet/src/Canvas.csToRpcException message prefix

The .NET change (prefixing the error code into the message string) is a language-specific workaround for a RemoteRpcException limitation — it's not a consistency gap. The PR description is explicit about this and includes a TODO. For comparison:

SDK How canvas_action_no_handler code is observable
Rust error.data.code (structured data)
Go (*jsonrpc2.Error).Data → unmarshal code field
Python error.data["code"]
Node.js N/A — API enforces handlers at declaration time; runtime pre-validates action names instead
.NET ex.Message contains "canvas_action_no_handler: ..." (workaround, TODO to fix properly)

Each language's test exercises the appropriate observable surface. No cross-SDK consistency issues found.

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

@jmoseley
Copy link
Copy Markdown
Contributor Author

Closing as superseded by #1413, which landed canvas E2E coverage across all 5 SDKs (Rust/Node/Python/Go/.NET) on the new codegen-aligned canvas API. That PR covers the core scenarios this one was opened for: declaring a canvas, dispatching canvas.open, canvas.action.invoke, canvas.close, and observing listOpen/OpenCanvases.

After the merge from main, this PR's tests no longer compile against the new API surface (removed exports like CanvasActionContext, CanvasOpenResponse, ExtensionInfo, etc.), and rewriting them on the new API would largely duplicate #1413.

Two scenarios from the original ask are not yet covered by #1413 and remain as potential follow-up work:

  1. Unhandled-action error path — verifying canvas.action.invoke for an unknown action returns {"code":"canvas_action_no_handler"} (reachable in Rust/Python/Go/.NET via default handler impls; not reachable in Node where the API requires per-action handlers).
  2. Resume rehydration — resuming a session with openCanvases populated and verifying the rehydrated state is observable via listOpen() / OpenCanvases.

These can be added in a small follow-up PR on top of #1413's foundation if desired.

@jmoseley jmoseley closed this May 26, 2026
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