Skip to content

Commit 40dbc51

Browse files
edburnsedburns
authored
Merge branch 'main' into edburns/dd-3012834-experimental-annotation
2 parents a890c8b + 391bcd4 commit 40dbc51

10 files changed

Lines changed: 506 additions & 26 deletions

File tree

‎dotnet/src/Session.cs‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public SessionCapabilities Capabilities
132132
/// </summary>
133133
/// <remarks>
134134
/// Populated from the most recent <c>session.resume</c> response and live
135-
/// <c>session.canvas.opened</c> events.
135+
/// <c>session.canvas.opened</c> and <c>session.canvas.closed</c> events.
136136
/// </remarks>
137137
[Experimental(Diagnostics.Experimental)]
138138
public IReadOnlyList<OpenCanvasInstance> OpenCanvases => _openCanvases;
@@ -892,6 +892,19 @@ internal void SetOpenCanvases(IList<OpenCanvasInstance>? canvases)
892892

893893
private void UpdateOpenCanvasesFromEvent(SessionEvent sessionEvent)
894894
{
895+
if (sessionEvent is SessionCanvasClosedEvent closedEvent)
896+
{
897+
var closedInstanceId = closedEvent.Data.InstanceId;
898+
if (string.IsNullOrEmpty(closedInstanceId))
899+
{
900+
_logger.LogWarning("failed to deserialize session.canvas.closed payload");
901+
return;
902+
}
903+
904+
RemoveOpenCanvas(closedInstanceId);
905+
return;
906+
}
907+
895908
if (sessionEvent is not SessionCanvasOpenedEvent canvasEvent)
896909
return;
897910

@@ -931,6 +944,12 @@ private void UpsertOpenCanvas(OpenCanvasInstance canvas)
931944
_openCanvases = canvases.AsReadOnly();
932945
}
933946

947+
private void RemoveOpenCanvas(string instanceId)
948+
{
949+
var canvases = _openCanvases.Where(open => open.InstanceId != instanceId).ToList();
950+
_openCanvases = canvases.AsReadOnly();
951+
}
952+
934953
internal void SetCanvasHandler(ICanvasHandler? handler)
935954
{
936955
ClientSessionApis.Canvas = handler is null ? null : new CanvasHandlerAdapter(handler);

‎dotnet/test/Unit/CanvasTests.cs‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,93 @@ public void SessionCanvasOpenedEvent_UpdatesOpenCanvasSnapshots()
229229
canvas => Assert.Equal("logs-1", canvas.InstanceId));
230230
}
231231

232+
[Fact]
233+
public void SessionCanvasClosedEvent_RemovesOpenCanvasSnapshots()
234+
{
235+
var session = CreateSession();
236+
237+
DispatchEvent(session, new SessionCanvasOpenedEvent
238+
{
239+
Id = Guid.NewGuid(),
240+
Timestamp = DateTimeOffset.UtcNow,
241+
Data = new SessionCanvasOpenedData
242+
{
243+
Availability = CanvasOpenedAvailability.Ready,
244+
CanvasId = "counter",
245+
ExtensionId = "project:counter",
246+
InstanceId = "counter-1",
247+
Title = "Counter",
248+
Reopen = false,
249+
}
250+
});
251+
DispatchEvent(session, new SessionCanvasOpenedEvent
252+
{
253+
Id = Guid.NewGuid(),
254+
Timestamp = DateTimeOffset.UtcNow,
255+
Data = new SessionCanvasOpenedData
256+
{
257+
Availability = CanvasOpenedAvailability.Ready,
258+
CanvasId = "logs",
259+
ExtensionId = "project:logs",
260+
InstanceId = "logs-1",
261+
Title = "Logs",
262+
Reopen = false,
263+
}
264+
});
265+
266+
Assert.Collection(
267+
session.OpenCanvases,
268+
canvas => Assert.Equal("counter-1", canvas.InstanceId),
269+
canvas => Assert.Equal("logs-1", canvas.InstanceId));
270+
271+
// Closing one instance removes it; the other remains.
272+
DispatchEvent(session, new SessionCanvasClosedEvent
273+
{
274+
Id = Guid.NewGuid(),
275+
Timestamp = DateTimeOffset.UtcNow,
276+
Data = new SessionCanvasClosedData
277+
{
278+
CanvasId = "counter",
279+
ExtensionId = "project:counter",
280+
InstanceId = "counter-1",
281+
}
282+
});
283+
284+
Assert.Collection(
285+
session.OpenCanvases,
286+
canvas => Assert.Equal("logs-1", canvas.InstanceId));
287+
288+
// Closing an absent instance is a no-op (idempotent).
289+
DispatchEvent(session, new SessionCanvasClosedEvent
290+
{
291+
Id = Guid.NewGuid(),
292+
Timestamp = DateTimeOffset.UtcNow,
293+
Data = new SessionCanvasClosedData
294+
{
295+
CanvasId = "counter",
296+
ExtensionId = "project:counter",
297+
InstanceId = "counter-1",
298+
}
299+
});
300+
301+
// A closed event with an empty instance id leaves the snapshot intact.
302+
DispatchEvent(session, new SessionCanvasClosedEvent
303+
{
304+
Id = Guid.NewGuid(),
305+
Timestamp = DateTimeOffset.UtcNow,
306+
Data = new SessionCanvasClosedData
307+
{
308+
CanvasId = "logs",
309+
ExtensionId = "project:logs",
310+
InstanceId = "",
311+
}
312+
});
313+
314+
Assert.Collection(
315+
session.OpenCanvases,
316+
canvas => Assert.Equal("logs-1", canvas.InstanceId));
317+
}
318+
232319
[Fact]
233320
public void ExtensionInfo_Serializes_SourceAndName()
234321
{

‎go/session.go‎

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ func (s *Session) WorkspacePath() string {
100100

101101
// OpenCanvases returns the open-canvas snapshot last reported by the runtime.
102102
// The snapshot is populated from session.resume and live session.canvas.opened
103-
// events. The returned slice is a copy and is safe to mutate by the caller.
103+
// and session.canvas.closed events. The returned slice is a copy and is safe to
104+
// mutate by the caller.
104105
func (s *Session) OpenCanvases() []rpc.OpenCanvasInstance {
105106
s.openCanvasesMu.RLock()
106107
defer s.openCanvasesMu.RUnlock()
@@ -130,27 +131,44 @@ func (s *Session) upsertOpenCanvas(canvas rpc.OpenCanvasInstance) {
130131
s.openCanvases = append(s.openCanvases, canvas)
131132
}
132133

133-
func (s *Session) updateOpenCanvasesFromEvent(event SessionEvent) {
134-
data, ok := event.Data.(*SessionCanvasOpenedData)
135-
if !ok {
136-
return
134+
func (s *Session) removeOpenCanvas(instanceID string) {
135+
s.openCanvasesMu.Lock()
136+
defer s.openCanvasesMu.Unlock()
137+
filtered := make([]rpc.OpenCanvasInstance, 0, len(s.openCanvases))
138+
for _, canvas := range s.openCanvases {
139+
if canvas.InstanceID != instanceID {
140+
filtered = append(filtered, canvas)
141+
}
137142
}
138-
if data.InstanceID == "" || data.CanvasID == "" || data.ExtensionID == "" || data.Availability == "" {
139-
fmt.Printf("failed to deserialize session.canvas.opened payload\n")
140-
return
143+
s.openCanvases = filtered
144+
}
145+
146+
func (s *Session) updateOpenCanvasesFromEvent(event SessionEvent) {
147+
switch data := event.Data.(type) {
148+
case *SessionCanvasOpenedData:
149+
if data.InstanceID == "" || data.CanvasID == "" || data.ExtensionID == "" || data.Availability == "" {
150+
fmt.Printf("failed to deserialize session.canvas.opened payload\n")
151+
return
152+
}
153+
s.upsertOpenCanvas(rpc.OpenCanvasInstance{
154+
Availability: rpc.CanvasInstanceAvailability(data.Availability),
155+
CanvasID: data.CanvasID,
156+
ExtensionID: data.ExtensionID,
157+
ExtensionName: data.ExtensionName,
158+
Input: data.Input,
159+
InstanceID: data.InstanceID,
160+
Reopen: data.Reopen,
161+
Status: data.Status,
162+
Title: data.Title,
163+
URL: data.URL,
164+
})
165+
case *SessionCanvasClosedData:
166+
if data.InstanceID == "" {
167+
fmt.Printf("failed to deserialize session.canvas.closed payload\n")
168+
return
169+
}
170+
s.removeOpenCanvas(data.InstanceID)
141171
}
142-
s.upsertOpenCanvas(rpc.OpenCanvasInstance{
143-
Availability: rpc.CanvasInstanceAvailability(data.Availability),
144-
CanvasID: data.CanvasID,
145-
ExtensionID: data.ExtensionID,
146-
ExtensionName: data.ExtensionName,
147-
Input: data.Input,
148-
InstanceID: data.InstanceID,
149-
Reopen: data.Reopen,
150-
Status: data.Status,
151-
Title: data.Title,
152-
URL: data.URL,
153-
})
154172
}
155173

156174
func (s *Session) registerCanvasHandler(handler CanvasHandler) {

‎go/session_test.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,72 @@ func TestSession_Capabilities(t *testing.T) {
659659
t.Fatalf("expected stale availability, got %q", open[0].Availability)
660660
}
661661
})
662+
663+
t.Run("session.canvas.closed event removes open canvas snapshots", func(t *testing.T) {
664+
session, cleanup := newTestSession()
665+
defer cleanup()
666+
667+
session.dispatchEvent(SessionEvent{
668+
Data: &SessionCanvasOpenedData{
669+
ExtensionID: "project:counter",
670+
CanvasID: "counter",
671+
InstanceID: "counter-1",
672+
Title: ptr("Counter"),
673+
Availability: CanvasOpenedAvailabilityReady,
674+
},
675+
})
676+
session.dispatchEvent(SessionEvent{
677+
Data: &SessionCanvasOpenedData{
678+
ExtensionID: "project:logs",
679+
CanvasID: "logs",
680+
InstanceID: "logs-1",
681+
Title: ptr("Logs"),
682+
Availability: CanvasOpenedAvailabilityReady,
683+
},
684+
})
685+
686+
if open := session.OpenCanvases(); len(open) != 2 {
687+
t.Fatalf("expected 2 open canvases, got %d", len(open))
688+
}
689+
690+
// Closing one instance removes it; the other remains.
691+
session.dispatchEvent(SessionEvent{
692+
Data: &SessionCanvasClosedData{
693+
ExtensionID: "project:counter",
694+
CanvasID: "counter",
695+
InstanceID: "counter-1",
696+
},
697+
})
698+
open := session.OpenCanvases()
699+
if len(open) != 1 || open[0].InstanceID != "logs-1" {
700+
t.Fatalf("expected only logs-1 to remain, got %+v", open)
701+
}
702+
703+
// Closing an absent instance is a no-op (idempotent).
704+
session.dispatchEvent(SessionEvent{
705+
Data: &SessionCanvasClosedData{
706+
ExtensionID: "project:counter",
707+
CanvasID: "counter",
708+
InstanceID: "counter-1",
709+
},
710+
})
711+
open = session.OpenCanvases()
712+
if len(open) != 1 || open[0].InstanceID != "logs-1" {
713+
t.Fatalf("idempotent close should leave logs-1, got %+v", open)
714+
}
715+
716+
// A closed event missing instanceID leaves the snapshot intact.
717+
session.dispatchEvent(SessionEvent{
718+
Data: &SessionCanvasClosedData{
719+
ExtensionID: "project:logs",
720+
CanvasID: "logs",
721+
},
722+
})
723+
open = session.OpenCanvases()
724+
if len(open) != 1 || open[0].InstanceID != "logs-1" {
725+
t.Fatalf("invalid close should leave logs-1, got %+v", open)
726+
}
727+
})
662728
}
663729

664730
// waitForCapability polls Session.Capabilities() until predicate matches or timeout.

‎nodejs/src/session.ts‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,8 @@ export class CopilotSession {
507507
this._capabilities = { ...this._capabilities, ...event.data };
508508
} else if (event.type === "session.canvas.opened") {
509509
this.upsertOpenCanvasFromEvent(event.data);
510+
} else if (event.type === "session.canvas.closed") {
511+
this.removeOpenCanvasFromEvent(event.data);
510512
}
511513
}
512514

@@ -518,6 +520,25 @@ export class CopilotSession {
518520
this.upsertOpenCanvas(data);
519521
}
520522

523+
private removeOpenCanvasFromEvent(data: unknown): void {
524+
if (
525+
!data ||
526+
typeof data !== "object" ||
527+
typeof (data as { instanceId?: unknown }).instanceId !== "string" ||
528+
(data as { instanceId: string }).instanceId.length === 0
529+
) {
530+
console.warn("failed to deserialize session.canvas.closed payload");
531+
return;
532+
}
533+
this.removeOpenCanvas((data as { instanceId: string }).instanceId);
534+
}
535+
536+
private removeOpenCanvas(instanceId: string): void {
537+
this.openCanvasInstances = this.openCanvasInstances.filter(
538+
(open) => open.instanceId !== instanceId
539+
);
540+
}
541+
521542
private upsertOpenCanvas(instance: OpenCanvasInstance): void {
522543
const index = this.openCanvasInstances.findIndex(
523544
(open) => open.instanceId === instance.instanceId
@@ -851,8 +872,8 @@ export class CopilotSession {
851872
/**
852873
* Snapshot of canvas instances currently known to be open for this session.
853874
* Populated from the `session.resume` response and live `session.canvas.opened`
854-
* events. Returns a defensive copy — mutating the returned array has no effect
855-
* on the session.
875+
* and `session.canvas.closed` events. Returns a defensive copy — mutating the
876+
* returned array has no effect on the session.
856877
*/
857878
get openCanvases(): OpenCanvasInstance[] {
858879
return [...this.openCanvasInstances];

‎nodejs/test/client.test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,69 @@ describe("CopilotClient", () => {
317317
warn.mockRestore();
318318
});
319319

320+
it("removes open canvases on live session.canvas.closed events", () => {
321+
const session = new CopilotSession("session-1", {} as any);
322+
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
323+
324+
(session as any)._dispatchEvent({
325+
type: "session.canvas.opened",
326+
data: {
327+
extensionId: "project:counter",
328+
canvasId: "counter",
329+
instanceId: "counter-1",
330+
title: "Counter",
331+
reopen: false,
332+
availability: "ready",
333+
},
334+
});
335+
(session as any)._dispatchEvent({
336+
type: "session.canvas.opened",
337+
data: {
338+
extensionId: "project:logs",
339+
canvasId: "logs",
340+
instanceId: "logs-1",
341+
title: "Logs",
342+
reopen: false,
343+
availability: "ready",
344+
},
345+
});
346+
expect(session.openCanvases.map((canvas) => canvas.instanceId)).toEqual([
347+
"counter-1",
348+
"logs-1",
349+
]);
350+
351+
// Closing one instance removes it; the other remains.
352+
(session as any)._dispatchEvent({
353+
type: "session.canvas.closed",
354+
data: {
355+
extensionId: "project:counter",
356+
canvasId: "counter",
357+
instanceId: "counter-1",
358+
},
359+
});
360+
expect(session.openCanvases.map((canvas) => canvas.instanceId)).toEqual(["logs-1"]);
361+
362+
// Closing an absent instance is a no-op (idempotent).
363+
(session as any)._dispatchEvent({
364+
type: "session.canvas.closed",
365+
data: {
366+
extensionId: "project:counter",
367+
canvasId: "counter",
368+
instanceId: "counter-1",
369+
},
370+
});
371+
expect(session.openCanvases.map((canvas) => canvas.instanceId)).toEqual(["logs-1"]);
372+
373+
// A closed event missing instanceId warns and leaves the snapshot intact.
374+
(session as any)._dispatchEvent({
375+
type: "session.canvas.closed",
376+
data: { extensionId: "project:logs", canvasId: "logs" },
377+
});
378+
expect(warn).toHaveBeenCalledWith("failed to deserialize session.canvas.closed payload");
379+
expect(session.openCanvases.map((canvas) => canvas.instanceId)).toEqual(["logs-1"]);
380+
warn.mockRestore();
381+
});
382+
320383
it("returns canvas_action_no_handler when no per-action handler is registered", async () => {
321384
const canvas = createCanvas({
322385
id: "counter",

0 commit comments

Comments
 (0)