Skip to content

Commit 2c8a0d5

Browse files
jmoseleyCopilot
andcommitted
Address PR review comments
- Rust+Node canvas dispatch: require instanceId for lifecycle verbs and custom actions (was silently defaulting to empty string). - Rust hostExtension.invoke: validate envelope.session_id matches the session handling the request; return a structured session_mismatch error envelope on mismatch. - Node handleHostExtensionInvoke: return { ok:false, error } envelopes for invalid payloads, missing sessions, and unsupported inner methods instead of throwing (which would have surfaced as JSON-RPC transport errors and broken the runtime-side contract). - Re-export CanvasInstanceRehydrate from @github/copilot-sdk/extension so extension consumers can type ResumeSessionConfig.openCanvasInstances without reaching into internal paths. - Fix canvas test that called canvas.open without instance_id. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent cf39cfd commit 2c8a0d5

5 files changed

Lines changed: 73 additions & 10 deletions

File tree

nodejs/src/canvas.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,12 @@ export async function dispatchCanvasAction(
350350
case RESERVED_CANVAS_ACTIONS.focus:
351351
case RESERVED_CANVAS_ACTIONS.close:
352352
case RESERVED_CANVAS_ACTIONS.reload: {
353+
if (!params.instanceId) {
354+
throw new CanvasError(
355+
"canvas_missing_instance_id",
356+
`Lifecycle verb '${params.actionName}' requires an instanceId`
357+
);
358+
}
353359
const hook =
354360
params.actionName === RESERVED_CANVAS_ACTIONS.focus
355361
? canvas.onFocus
@@ -360,7 +366,7 @@ export async function dispatchCanvasAction(
360366
const ctx: CanvasLifecycleContext = {
361367
sessionId,
362368
canvasId: params.canvasId,
363-
instanceId: params.instanceId ?? "",
369+
instanceId: params.instanceId,
364370
};
365371
await hook(ctx);
366372
return undefined;
@@ -369,10 +375,16 @@ export async function dispatchCanvasAction(
369375
if (!canvas.onAction) {
370376
throw CanvasError.noHandler();
371377
}
378+
if (!params.instanceId) {
379+
throw new CanvasError(
380+
"canvas_missing_instance_id",
381+
`Action '${params.actionName}' requires an instanceId`
382+
);
383+
}
372384
return canvas.onAction({
373385
sessionId,
374386
canvasId: params.canvasId,
375-
instanceId: params.instanceId ?? "",
387+
instanceId: params.instanceId,
376388
actionName: params.actionName,
377389
input: params.input,
378390
});

nodejs/src/client.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2150,22 +2150,46 @@ export class CopilotClient {
21502150
sessionId: string;
21512151
request: { id?: string; method: string; params?: unknown };
21522152
}): Promise<{ ok: true; result: unknown } | { ok: false; error: CanvasError }> {
2153+
const invalidEnvelope = (message: string) =>
2154+
({
2155+
ok: false as const,
2156+
error: {
2157+
code: "invalid_payload",
2158+
message,
2159+
name: "CanvasError",
2160+
} as CanvasError,
2161+
}) satisfies { ok: false; error: CanvasError };
2162+
21532163
if (!params || typeof params.sessionId !== "string" || !params.request) {
2154-
throw new Error("Invalid hostExtension.invoke payload");
2164+
return invalidEnvelope("Invalid hostExtension.invoke payload");
21552165
}
21562166
const session = this.sessions.get(params.sessionId);
21572167
if (!session) {
2158-
throw new Error(`Session not found: ${params.sessionId}`);
2168+
return {
2169+
ok: false,
2170+
error: {
2171+
code: "session_not_found",
2172+
message: `Session not found: ${params.sessionId}`,
2173+
name: "CanvasError",
2174+
} as CanvasError,
2175+
};
21592176
}
21602177
const { method, params: inner } = params.request;
21612178
// Only `canvas.action.invoke` is accepted as an inner method; reject
21622179
// anything else explicitly so misrouted calls don't silently no-op.
21632180
if (method !== "canvas.action.invoke") {
2164-
throw new Error(`Unsupported hostExtension.invoke method: ${method}`);
2181+
return {
2182+
ok: false,
2183+
error: {
2184+
code: "unsupported_method",
2185+
message: `hostExtension.invoke only supports canvas.action.invoke, got '${method}'`,
2186+
name: "CanvasError",
2187+
} as CanvasError,
2188+
};
21652189
}
21662190
const actionParams = inner as CanvasActionInvokeParams;
21672191
if (!actionParams || typeof actionParams.canvasId !== "string") {
2168-
throw new Error("Invalid canvas.action.invoke params: missing canvasId");
2192+
return invalidEnvelope("Invalid canvas.action.invoke params: missing canvasId");
21692193
}
21702194
const canvas = session.getCanvas(actionParams.canvasId);
21712195
if (!canvas) {

nodejs/src/extension.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export {
1717
type CanvasActionContext,
1818
type CanvasAgentActionDeclaration,
1919
type CanvasDeclaration,
20+
type CanvasInstanceRehydrate,
2021
type CanvasLifecycleContext,
2122
type CanvasOpenContext,
2223
type CanvasOpenResponse,

rust/src/canvas.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -430,13 +430,18 @@ pub async fn dispatch_canvas_invoke(
430430
}
431431
Ok(Value::Null)
432432
}
433-
_ => {
434-
let instance_id = params.instance_id.unwrap_or_default();
433+
other => {
434+
let instance_id = params.instance_id.ok_or_else(|| {
435+
CanvasError::new(
436+
"canvas_missing_instance_id",
437+
format!("Action '{other}' requires an instanceId"),
438+
)
439+
})?;
435440
let ctx = CanvasActionContext {
436441
session_id,
437442
canvas_id: params.canvas_id,
438443
instance_id,
439-
action_name: params.action_name,
444+
action_name: other.to_string(),
440445
input: params.input,
441446
};
442447
handler.on_action(ctx).await
@@ -729,7 +734,7 @@ mod tests {
729734
SessionId::from("s1"),
730735
CanvasInvokeParams {
731736
canvas_id: "dup".into(),
732-
instance_id: None,
737+
instance_id: Some("inst-1".into()),
733738
action_name: "canvas.open".into(),
734739
input: Value::Null,
735740
toolbar: None,

rust/src/session.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1848,6 +1848,27 @@ async fn handle_request(
18481848
}
18491849
};
18501850

1851+
if envelope.session_id != sid {
1852+
let result = serde_json::json!({
1853+
"ok": false,
1854+
"error": {
1855+
"code": "session_mismatch",
1856+
"message": format!(
1857+
"hostExtension.invoke session id '{}' does not match this session '{}'",
1858+
envelope.session_id, sid
1859+
),
1860+
},
1861+
});
1862+
let rpc_response = JsonRpcResponse {
1863+
jsonrpc: "2.0".to_string(),
1864+
id: request.id,
1865+
result: Some(result),
1866+
error: None,
1867+
};
1868+
let _ = client.send_response(&rpc_response).await;
1869+
return;
1870+
}
1871+
18511872
if envelope.request.method != "canvas.action.invoke" {
18521873
let result = serde_json::json!({
18531874
"ok": false,

0 commit comments

Comments
 (0)