Canvas SDK: post-merge review followups (PR #1401)

dotnet/src/Canvas.cs

@@ -2,9 +2,11 @@
  *  Copyright (c) Microsoft Corporation. All rights reserved.
  *--------------------------------------------------------------------------------------------*/
 
+using System;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.Text.Json;
+using System.Text.Json.Nodes;
 using System.Text.Json.Serialization;
 using System.Threading;
 using System.Threading.Tasks;
@@ -55,19 +57,19 @@ public sealed class ExtensionInfo
     public string Name { get; set; } = string.Empty;
 }
 
-/// <summary>Structured error returned from canvas handlers.</summary>
+/// <summary>Structured exception returned from canvas handlers.</summary>
 /// <remarks>
 /// Throw this from <see cref="ICanvasHandler"/> implementations to surface a
 /// machine-readable error code to the runtime. Any other exception is wrapped
 /// in a generic <c>canvas_handler_error</c> envelope.
 /// </remarks>
 [Experimental(Diagnostics.Experimental)]
-public sealed class CanvasError : Exception
+public sealed class CanvasException : Exception
 {
-    /// <summary>Initializes a new <see cref="CanvasError"/>.</summary>
+    /// <summary>Initializes a new <see cref="CanvasException"/>.</summary>
     /// <param name="code">Machine-readable error code.</param>
     /// <param name="message">Human-readable message.</param>
-    public CanvasError(string code, string message) : base(message)
+    public CanvasException(string code, string message) : base(message)
     {
         Code = code;
     }
@@ -76,15 +78,15 @@ public CanvasError(string code, string message) : base(message)
     public string Code { get; }
 
     /// <summary>
-    /// Default error returned when a custom action has no handler.
+    /// Default exception returned when a custom action has no handler.
     /// </summary>
-    public static CanvasError NoHandler() => new(
+    public static CanvasException NoHandler() => new(
         "canvas_action_no_handler",
         "No handler implemented for this canvas action");
 }
 
 /// <summary>
-/// Internal helpers used by the session runtime to translate <see cref="CanvasError"/>
+/// Internal helpers used by the session runtime to translate <see cref="CanvasException"/>
 /// (and other handler-thrown exceptions) into structured JSON-RPC error responses.
 /// </summary>
 internal static class CanvasErrorHelpers
@@ -99,31 +101,17 @@ public static LocalRpcInvocationException HandlerError(string message) => Build(
         "canvas_handler_error",
         message);
 
-    public static LocalRpcInvocationException ToRpcException(CanvasError error) => Build(error.Code, error.Message);
+    public static LocalRpcInvocationException ToRpcException(CanvasException error) => Build(error.Code, error.Message);
 
     private static LocalRpcInvocationException Build(string code, string message)
     {
-        var json = JsonSerializer.Serialize(
-            new CanvasErrorPayload { Code = code, Message = message },
-            CanvasJsonContext.Default.CanvasErrorPayload);
-        using var doc = JsonDocument.Parse(json);
-        return new LocalRpcInvocationException(InternalError, message, doc.RootElement.Clone());
-    }
-
-    internal sealed class CanvasErrorPayload
-    {
-        [JsonPropertyName("code")]
-        public string Code { get; set; } = string.Empty;
-
-        [JsonPropertyName("message")]
-        public string Message { get; set; } = string.Empty;
+        JsonElement payload = JsonSerializer.SerializeToElement(
+            new JsonObject { ["code"] = code, ["message"] = message },
+            TypesJsonContext.Default.JsonObject);
+        return new LocalRpcInvocationException(InternalError, message, payload);
     }
 }
 
-[JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase)]
-[JsonSerializable(typeof(CanvasErrorHelpers.CanvasErrorPayload))]
-internal partial class CanvasJsonContext : JsonSerializerContext;
-
 /// <summary>
 /// Provider-side canvas lifecycle handler.
 /// </summary>
@@ -155,7 +143,7 @@ public interface ICanvasHandler
 
     /// <summary>
     /// Handle a non-lifecycle action declared by the canvas.
-    /// Default: throws <see cref="CanvasError.NoHandler"/>.
+    /// Default: throws <see cref="CanvasException.NoHandler"/>.
     /// </summary>
     Task<object?> OnActionAsync(CanvasProviderInvokeActionRequest context, CancellationToken cancellationToken);
 }
@@ -180,5 +168,5 @@ public virtual Task OnCloseAsync(CanvasProviderCloseRequest context, Cancellatio
 
     /// <inheritdoc />
     public virtual Task<object?> OnActionAsync(CanvasProviderInvokeActionRequest context, CancellationToken cancellationToken)
-        => Task.FromException<object?>(CanvasError.NoHandler());
+        => Task.FromException<object?>(CanvasException.NoHandler());
 }

dotnet/src/JsonRpc.cs

@@ -486,6 +486,13 @@ private async Task HandleIncomingMethodAsync(string methodName, JsonElement mess
             }
             catch (Exception ex) when (ex is not OperationCanceledException)
             {
+                // `InvokeHandlerAsync` dispatches handlers via reflection
+                // (`Delegate.DynamicInvoke` / `MethodInfo.Invoke`), which wraps
+                // any exception thrown inside the user-supplied handler in a
+                // `TargetInvocationException`. Unwrap so we surface the original
+                // failure (e.g. `LocalRpcInvocationException`, `CanvasException`)
+                // to the JSON-RPC error response instead of the reflection
+                // wrapper.
                 var actual = ex is TargetInvocationException tie && tie.InnerException != null ? tie.InnerException : ex;
                 if (_logger.IsEnabled(LogLevel.Debug))
                 {

dotnet/src/Session.cs

@@ -892,15 +892,12 @@ internal void SetCanvasHandler(ICanvasHandler? handler)
         ClientSessionApis.Canvas = handler is null ? null : new CanvasHandlerAdapter(handler);
     }
 
+    private static readonly JsonElement NullJsonElement = JsonDocument.Parse("null").RootElement.Clone();
+
     private static JsonElement SerializeActionResult(object? value)
     {
         var element = CopilotClient.ToJsonElementForWire(value);
-        if (element.HasValue)
-        {
-            return element.Value;
-        }
-        using var doc = JsonDocument.Parse("null");
-        return doc.RootElement.Clone();
+        return element ?? NullJsonElement;
     }
 #pragma warning restore GHCP001
 
@@ -912,7 +909,7 @@ public async Task<CanvasProviderOpenResult> OpenAsync(CanvasProviderOpenRequest
             {
                 return await handler.OnOpenAsync(request, cancellationToken).ConfigureAwait(false);
             }
-            catch (CanvasError ce)
+            catch (CanvasException ce)
             {
                 throw CanvasErrorHelpers.ToRpcException(ce);
             }
@@ -928,7 +925,7 @@ public async Task CloseAsync(CanvasProviderCloseRequest request, CancellationTok
             {
                 await handler.OnCloseAsync(request, cancellationToken).ConfigureAwait(false);
             }
-            catch (CanvasError ce)
+            catch (CanvasException ce)
             {
                 throw CanvasErrorHelpers.ToRpcException(ce);
             }
@@ -945,7 +942,7 @@ public async Task<object> InvokeActionAsync(CanvasProviderInvokeActionRequest re
                 var result = await handler.OnActionAsync(request, cancellationToken).ConfigureAwait(false);
                 return SerializeActionResult(result);
             }
-            catch (CanvasError ce)
+            catch (CanvasException ce)
             {
                 throw CanvasErrorHelpers.ToRpcException(ce);
             }

dotnet/src/Types.cs

@@ -9,6 +9,7 @@
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Text.Json;
+using System.Text.Json.Nodes;
 using System.Text.Json.Serialization;
 
 namespace GitHub.Copilot;
@@ -3085,6 +3086,7 @@ public sealed class SystemMessageTransformRpcResponse
 [JsonSerializable(typeof(ToolResultObject))]
 [JsonSerializable(typeof(JsonElement))]
 [JsonSerializable(typeof(JsonElement?))]
+[JsonSerializable(typeof(JsonObject))]
 [JsonSerializable(typeof(object))]
 [JsonSerializable(typeof(Dictionary<string, object>))]
 [JsonSerializable(typeof(string[]))]

dotnet/test/Unit/CanvasTests.cs

@@ -86,18 +86,18 @@ public async Task CanvasHandlerBase_DefaultOnClose_Completes()
     }
 
     [Fact]
-    public async Task CanvasHandlerBase_DefaultOnAction_ThrowsNoHandlerCanvasError()
+    public async Task CanvasHandlerBase_DefaultOnAction_ThrowsNoHandlerCanvasException()
     {
         var handler = new TestHandler();
-        var ex = await Assert.ThrowsAsync<CanvasError>(
+        var ex = await Assert.ThrowsAsync<CanvasException>(
             () => handler.OnActionAsync(new CanvasProviderInvokeActionRequest(), CancellationToken.None));
         Assert.Equal("canvas_action_no_handler", ex.Code);
     }
 
     [Fact]
-    public void CanvasError_NoHandler_HasExpectedCode()
+    public void CanvasException_NoHandler_HasExpectedCode()
     {
-        var err = CanvasError.NoHandler();
+        var err = CanvasException.NoHandler();
         Assert.Equal("canvas_action_no_handler", err.Code);
         Assert.False(string.IsNullOrEmpty(err.Message));
     }

go/canvas.go

@@ -14,6 +14,9 @@ import (
 
 // CanvasDeclaration is the declarative metadata for a single canvas, sent over
 // the wire on `session.create` / `session.resume`.
+//
+// Experimental: CanvasDeclaration is part of an experimental wire-protocol
+// surface and may change or be removed in future SDK or CLI releases.
 type CanvasDeclaration struct {
 	// ID is the canvas identifier, unique within the declaring connection.
 	ID string `json:"id"`
@@ -29,6 +32,9 @@ type CanvasDeclaration struct {
 
 // ExtensionInfo carries stable extension identity for session participants
 // that provide canvases.
+//
+// Experimental: ExtensionInfo is part of an experimental wire-protocol
+// surface and may change or be removed in future SDK or CLI releases.
 type ExtensionInfo struct {
 	// Source is the extension namespace/source, e.g. "github-app".
 	Source string `json:"source"`
@@ -41,6 +47,9 @@ type ExtensionInfo struct {
 // Wire envelope:
 //
 //	{ "code": "<code>", "message": "<message>" }
+//
+// Experimental: CanvasError is part of an experimental wire-protocol
+// surface and may change or be removed in future SDK or CLI releases.
 type CanvasError struct {
 	// Code is the machine-readable error code.
 	Code string `json:"code"`
@@ -79,6 +88,9 @@ func CanvasErrorNoHandler() *CanvasError {
 //
 // Embed CanvasHandlerDefaults to inherit no-op defaults for OnClose and a
 // "no handler" error for OnAction.
+//
+// Experimental: CanvasHandler is part of an experimental wire-protocol
+// surface and may change or be removed in future SDK or CLI releases.
 type CanvasHandler interface {
 	OnOpen(ctx context.Context, c rpc.CanvasProviderOpenRequest) (rpc.CanvasProviderOpenResult, error)
 	OnClose(ctx context.Context, c rpc.CanvasProviderCloseRequest) error
@@ -94,6 +106,9 @@ type CanvasHandler interface {
 //	    copilot.CanvasHandlerDefaults
 //	}
 //	func (h *myHandler) OnOpen(ctx context.Context, c rpc.CanvasProviderOpenRequest) (rpc.CanvasProviderOpenResult, error) { ... }
+//
+// Experimental: CanvasHandlerDefaults is part of an experimental wire-protocol
+// surface and may change or be removed in future SDK or CLI releases.
 type CanvasHandlerDefaults struct{}
 
 // OnClose returns nil by default.

nodejs/src/canvas.ts

@@ -10,7 +10,11 @@ import type {
     CanvasProviderOpenResult,
 } from "./generated/rpc.js";
 
-export type { CanvasJsonSchema, CanvasHostContext } from "./generated/rpc.js";
+export type {
+    CanvasJsonSchema,
+    CanvasHostContext,
+    CanvasHostContextCapabilities,
+} from "./generated/rpc.js";
 
 /**
  * Extension-owned canvases declared via
@@ -21,6 +25,9 @@ export type { CanvasJsonSchema, CanvasHostContext } from "./generated/rpc.js";
  * pipeline. The SDK routes those requests by `canvasId` to the in-process
  * handlers bound by `createCanvas`. Re-opening with an existing `instanceId`
  * is how the host focuses an existing panel; reload is a renderer-only concern.
+ *
+ * @experimental Canvas types are part of an experimental wire-protocol surface
+ * and may change or be removed in future SDK or CLI releases.
  */
 
 /**
@@ -31,6 +38,9 @@ export type { CanvasJsonSchema, CanvasHostContext } from "./generated/rpc.js";
  *
  * Names MUST NOT start with `canvas.` — that prefix is reserved for
  * lifecycle verbs.
+ *
+ * @experimental This type is part of an experimental wire-protocol surface
+ * and may change or be removed in future SDK or CLI releases.
  */
 export interface CanvasAction {
     /** Action identifier, unique within the canvas. */
@@ -46,6 +56,9 @@ export interface CanvasAction {
 /**
  * Declarative metadata for a single canvas, serialized over the wire on
  * `session.create` / `session.resume`.
+ *
+ * @experimental This type is part of an experimental wire-protocol surface
+ * and may change or be removed in future SDK or CLI releases.
  */
 export interface CanvasDeclaration {
     /** Canvas id, unique within the declaring connection. */
@@ -60,7 +73,12 @@ export interface CanvasDeclaration {
     actions?: Omit<CanvasAction, "handler">[];
 }
 
-/** Structured error returned from canvas handlers. */
+/**
+ * Structured error returned from canvas handlers.
+ *
+ * @experimental This class is part of an experimental wire-protocol surface
+ * and may change or be removed in future SDK or CLI releases.
+ */
 export class CanvasError extends Error {
     constructor(
         public readonly code: string,
@@ -82,6 +100,9 @@ export class CanvasError extends Error {
 /**
  * Options accepted by {@link createCanvas}. Combines the declarative
  * {@link CanvasDeclaration} fields with the in-process handler closures.
+ *
+ * @experimental This interface is part of an experimental wire-protocol surface
+ * and may change or be removed in future SDK or CLI releases.
  */
 export interface CanvasOptions {
     /** @see CanvasDeclaration.id */
@@ -119,6 +140,9 @@ export interface CanvasOptions {
  * ergonomics) where other SDKs (Rust, Python, Go, .NET) expose a single
  * `CanvasHandler` per session that switches on `canvasId`. Both shapes target
  * the same JSON-RPC wire protocol; the divergence is API ergonomics only.
+ *
+ * @experimental This class is part of an experimental wire-protocol surface
+ * and may change or be removed in future SDK or CLI releases.
  */
 export class Canvas {
     readonly declaration: CanvasDeclaration;
@@ -156,6 +180,9 @@ export class Canvas {
  * `DefineTool`'s co-location ergonomics) where other SDKs (Rust, Python, Go,
  * .NET) expose a single `CanvasHandler` per session that switches on
  * `canvasId`. Both shapes target the same JSON-RPC wire protocol.
+ *
+ * @experimental This function is part of an experimental wire-protocol surface
+ * and may change or be removed in future SDK or CLI releases.
  */
 export function createCanvas(options: CanvasOptions): Canvas {
     return new Canvas(options);

nodejs/src/index.ts

@@ -18,6 +18,7 @@ export {
     type CanvasAction,
     type CanvasDeclaration,
     type CanvasHostContext,
+    type CanvasHostContextCapabilities,
     type CanvasJsonSchema,
     type CanvasOptions,
 } from "./canvas.js";