From 9854ee2f454a0363ac68ed13b3af914bc3e14884 Mon Sep 17 00:00:00 2001 From: BB-fat <1056871944@qq.com> Date: Mon, 17 Aug 2026 07:27:28 +0000 Subject: [PATCH] fix(dsh-plugin-browserskill): stop observation overlay from flashing between frames Hold the last ready thumbnail until the next one decodes, and swap the img src in place instead of remounting through a placeholder. --- packages/dsh-plugin-browserskill/README.md | 3 +- .../src/client/ObservationOverlay.module.css | 2 + .../src/client/ObservationOverlay.tsx | 15 ++-- .../src/client/observation-store.ts | 90 ++++++++++++++++--- .../tests/client/observation-overlay.test.tsx | 25 ++++++ .../tests/client/observation-store.test.ts | 41 ++++++++- 6 files changed, 157 insertions(+), 19 deletions(-) diff --git a/packages/dsh-plugin-browserskill/README.md b/packages/dsh-plugin-browserskill/README.md index a0b0ddb..57db443 100644 --- a/packages/dsh-plugin-browserskill/README.md +++ b/packages/dsh-plugin-browserskill/README.md @@ -109,7 +109,8 @@ shell's theme cannot bleed back in. - **Lifecycle**: hidden while the plugin owns no sessions; appears on the first `browser_session_start`; disappears when all sessions stop (or the plugin unloads). - **Focus view**: status row (green/idle/red dot + session + action + mm:ss), the latest page - frame (refreshes every ~1.5s while active, ~8s when idle, keeps the last frame on errors), + frame (refreshes every ~1.5s while active, ~8s when idle; the last good frame stays on + stage while the next one loads, and is kept on errors so the card does not flash), and the action row (Interrupt + Pop out). - **Interrupt**: one click kills the in-flight bsk command of the focus session (same semantics as the chat Stop button — the current action fails, the agent run may continue). Strip items diff --git a/packages/dsh-plugin-browserskill/src/client/ObservationOverlay.module.css b/packages/dsh-plugin-browserskill/src/client/ObservationOverlay.module.css index 5095811..288478a 100644 --- a/packages/dsh-plugin-browserskill/src/client/ObservationOverlay.module.css +++ b/packages/dsh-plugin-browserskill/src/client/ObservationOverlay.module.css @@ -79,6 +79,8 @@ overflow: hidden; } +/* Fade only on first mount. The is reused across frames (src swap, + no React key) so this animation must not replay on every breath. */ .thumb { max-width: 100%; max-height: 100%; diff --git a/packages/dsh-plugin-browserskill/src/client/ObservationOverlay.tsx b/packages/dsh-plugin-browserskill/src/client/ObservationOverlay.tsx index 8c0f845..e6b82a3 100644 --- a/packages/dsh-plugin-browserskill/src/client/ObservationOverlay.tsx +++ b/packages/dsh-plugin-browserskill/src/client/ObservationOverlay.tsx @@ -139,7 +139,7 @@ function StripItem(props: { useEffect(() => { store.ensureThumbnail(thumbId); }, [store, thumbId]); - const thumb = thumbId !== undefined ? store.getSnapshot().thumbnails[thumbId] : undefined; + const thumb = store.getSnapshot().displayFrames[obs.sessionId]; const state = obs.dead === true ? "dead" : statusOf(obs); return (
onTogglePin(obs.sessionId)} > - {thumb?.status === "ready" && thumb.url !== undefined ? ( + {thumb?.url !== undefined ? ( ) : ( @@ -222,7 +222,11 @@ function OverlayBody(props: { useEffect(() => { store.ensureThumbnail(thumbId); }, [store, thumbId]); - const thumb = thumbId !== undefined ? store.getSnapshot().thumbnails[thumbId] : undefined; + // Paint the last good frame while the next attachment decodes — swapping + // to a placeholder (and remounting with a fade) is what made the + // card flash on every breath / action-end capture. + const thumb = focus !== undefined ? store.getSnapshot().displayFrames[focus.sessionId] : undefined; + const displayUrl = thumb?.url; const canInterrupt = available && @@ -266,11 +270,10 @@ function OverlayBody(props: { ) : null}
- {thumb?.status === "ready" && thumb.url !== undefined ? ( + {displayUrl !== undefined ? ( {`session ) : ( diff --git a/packages/dsh-plugin-browserskill/src/client/observation-store.ts b/packages/dsh-plugin-browserskill/src/client/observation-store.ts index aeefa7d..e180956 100644 --- a/packages/dsh-plugin-browserskill/src/client/observation-store.ts +++ b/packages/dsh-plugin-browserskill/src/client/observation-store.ts @@ -3,7 +3,9 @@ * SSE increment stream (with a state refetch on every (re)open — events that * fired while the stream was down are otherwise lost forever), on-demand * thumbnail blob loading through the session-authorized readAttachment path, - * and the interrupt call. All I/O is injected so tests never touch a network. + * and the interrupt call. The last ready blob per session is held until its + * replacement decodes so the overlay can swap frames in place. All I/O is + * injected so tests never touch a network. */ import type { ObservationEvent, SessionObservation } from "../observation"; @@ -38,6 +40,12 @@ export interface OverlaySnapshot { /** Whether the SSE increment stream is currently subscribed. */ readonly subscribed: boolean; readonly thumbnails: Readonly>; + /** + * sessionId → the frame the overlay should paint. While a newer attachment + * is still loading (or failed), this keeps the last good URL so the card + * never drops to a placeholder between breaths. + */ + readonly displayFrames: Readonly>; /** False when the host reports the browser/daemon as unreachable. */ readonly available: boolean; } @@ -55,14 +63,17 @@ function revoke(url: string | undefined): void { export class ObservationClientStore { private sessions = new Map(); private thumbs = new Map(); - /** sessionId → the attachment id currently displayed for it (leak guard). */ + /** sessionId → the attachment id currently advertised for it. */ private readonly thumbBySession = new Map(); + /** sessionId → last successfully decoded frame (held across the next load). */ + private readonly lastReady = new Map(); private listeners = new Set<() => void>(); private events: EventSourceLike | undefined; private snapshot: OverlaySnapshot = { sessions: [], subscribed: false, thumbnails: {}, + displayFrames: {}, available: true, }; private available = true; @@ -82,11 +93,43 @@ export class ObservationClientStore { sessions: [...this.sessions.values()], subscribed: this.events !== undefined, thumbnails: Object.fromEntries(this.thumbs), + displayFrames: this.buildDisplayFrames(), available: this.available, }; for (const listener of [...this.listeners]) listener(); } + /** Frame the overlay should paint for one session (last good while next loads). */ + private frameFor(sessionId: string): ThumbnailState | undefined { + const attachmentId = this.thumbBySession.get(sessionId); + const current = attachmentId !== undefined ? this.thumbs.get(attachmentId) : undefined; + if (current?.status === "ready" && current.url !== undefined) return current; + const held = this.lastReady.get(sessionId); + if (held !== undefined) { + return { + status: current?.status === "error" ? "error" : "ready", + url: held.url, + }; + } + return current; + } + + private buildDisplayFrames(): Record { + const frames: Record = {}; + for (const sessionId of this.sessions.keys()) { + const frame = this.frameFor(sessionId); + if (frame !== undefined) frames[sessionId] = frame; + } + return frames; + } + + private sessionOf(attachmentId: string): string | undefined { + for (const [sessionId, id] of this.thumbBySession) { + if (id === attachmentId) return sessionId; + } + return undefined; + } + /** (Re)pull the full state: initial load and every SSE (re)open. */ private refreshState(): void { void this.deps @@ -141,28 +184,42 @@ export class ObservationClientStore { for (const thumb of this.thumbs.values()) revoke(thumb.url); this.thumbs.clear(); this.thumbBySession.clear(); + this.lastReady.clear(); this.sessions.clear(); this.publish(); } - /** Forget one session's tracked thumbnail, revoking its blob URL. */ + /** Forget one session's tracked + held frames, revoking their blob URLs. */ private dropThumb(sessionId: string): void { const attachmentId = this.thumbBySession.get(sessionId); - if (attachmentId === undefined) return; + const held = this.lastReady.get(sessionId); this.thumbBySession.delete(sessionId); - revoke(this.thumbs.get(attachmentId)?.url); - this.thumbs.delete(attachmentId); + this.lastReady.delete(sessionId); + if (attachmentId !== undefined) { + revoke(this.thumbs.get(attachmentId)?.url); + this.thumbs.delete(attachmentId); + } + if (held !== undefined && held.attachmentId !== attachmentId) { + revoke(held.url); + this.thumbs.delete(held.attachmentId); + } } /** - * Track the frame a session currently shows; replacing a frame revokes the - * old blob URL immediately — blob URLs must not accumulate one per frame. + * Track the frame a session currently advertises. The last *ready* blob is + * kept until the replacement decodes — dropping it on the upsert is what + * made the overlay flash a placeholder between breaths. */ private trackThumb(sessionId: string, attachmentId: string | undefined): void { const previous = this.thumbBySession.get(sessionId); if (previous === attachmentId) return; - this.dropThumb(sessionId); if (attachmentId !== undefined) this.thumbBySession.set(sessionId, attachmentId); + else this.thumbBySession.delete(sessionId); + const heldId = this.lastReady.get(sessionId)?.attachmentId; + if (previous !== undefined && previous !== heldId) { + revoke(this.thumbs.get(previous)?.url); + this.thumbs.delete(previous); + } } private apply(event: ObservationEvent): void { @@ -171,6 +228,7 @@ export class ObservationClientStore { for (const thumb of this.thumbs.values()) revoke(thumb.url); this.thumbs.clear(); this.thumbBySession.clear(); + this.lastReady.clear(); } else if (event.type === "remove" && event.session !== undefined) { this.sessions.delete(event.session.sessionId); this.dropThumb(event.session.sessionId); @@ -185,8 +243,9 @@ export class ObservationClientStore { /** * Ensure a thumbnail load is in flight for one attachment reference. New - * frames replace the old URL; failures keep the last good frame (the caller - * renders `status: 'error'` as a small badge over the old image). + * frames replace the old URL only after they decode; failures keep the last + * good frame (the caller renders `status: 'error'` as a small badge over + * the old image). */ ensureThumbnail(attachmentId: string | undefined): void { if (attachmentId === undefined || this.thumbs.has(attachmentId)) return; @@ -200,6 +259,15 @@ export class ObservationClientStore { return; } this.thumbs.set(attachmentId, { status: "ready", url }); + const sessionId = this.sessionOf(attachmentId); + if (sessionId !== undefined) { + const previous = this.lastReady.get(sessionId); + if (previous !== undefined && previous.attachmentId !== attachmentId) { + revoke(previous.url); + this.thumbs.delete(previous.attachmentId); + } + this.lastReady.set(sessionId, { attachmentId, url }); + } this.publish(); }, () => { diff --git a/packages/dsh-plugin-browserskill/tests/client/observation-overlay.test.tsx b/packages/dsh-plugin-browserskill/tests/client/observation-overlay.test.tsx index f3a5c22..ea8ab77 100644 --- a/packages/dsh-plugin-browserskill/tests/client/observation-overlay.test.tsx +++ b/packages/dsh-plugin-browserskill/tests/client/observation-overlay.test.tsx @@ -92,6 +92,31 @@ describe("ObservationOverlay", () => { await screen.findByRole("img", { name: "session s1 view" }); }); + it("keeps the current frame on stage while the next thumbnail loads", async () => { + let resolveNext: ((url: string) => void) | undefined; + const h = makeHarness([{ ...BUSY, thumbnailAttachmentId: "att-1" }]); + h.loadImage.mockImplementation(async (id: string) => { + if (id === "att-2") { + return new Promise((resolve) => { + resolveNext = resolve; + }); + } + return `blob:${id}`; + }); + render(); + const img = await screen.findByRole("img", { name: "session s1 view" }); + expect(img.getAttribute("src")).toBe("blob:att-1"); + h.push([{ ...BUSY, thumbnailAttachmentId: "att-2" }]); + await waitFor(() => expect(h.loadImage).toHaveBeenCalledWith("att-2")); + // Same node, same src — no placeholder flash, no remount/fade. + expect(screen.queryByText("waiting for page")).toBeNull(); + expect(screen.getByRole("img", { name: "session s1 view" })).toBe(img); + expect(img.getAttribute("src")).toBe("blob:att-1"); + resolveNext?.("blob:att-2"); + await waitFor(() => expect(img.getAttribute("src")).toBe("blob:att-2")); + expect(screen.getByRole("img", { name: "session s1 view" })).toBe(img); + }); + it("interrupt: disabled while idle, active call posts and shows progress", async () => { const h = makeHarness([{ sessionId: "s1", action: "idle", since: Date.now() }]); render(); diff --git a/packages/dsh-plugin-browserskill/tests/client/observation-store.test.ts b/packages/dsh-plugin-browserskill/tests/client/observation-store.test.ts index b0c7714..1451bf2 100644 --- a/packages/dsh-plugin-browserskill/tests/client/observation-store.test.ts +++ b/packages/dsh-plugin-browserskill/tests/client/observation-store.test.ts @@ -119,7 +119,7 @@ describe("thumbnail blob URL lifecycle", () => { return revoke; } - it("replacing a frame revokes the old blob URL and drops its entry", async () => { + it("holds the last ready frame until the replacement decodes", async () => { const revoke = withRevokeSpy(); const { store, esInstances } = harness([{ ...OBS_IDLE, thumbnailAttachmentId: "a1" }]); store.start(); @@ -132,11 +132,50 @@ describe("thumbnail blob URL lifecycle", () => { type: "upsert", session: { ...OBS_IDLE, thumbnailAttachmentId: "a2" }, }); + // Old blob stays painted so the overlay does not flash a placeholder. + expect(store.getSnapshot().thumbnails["a1"]).toEqual({ + status: "ready", + url: "blob:url-a1", + }); + expect(store.getSnapshot().displayFrames.s1).toEqual({ + status: "ready", + url: "blob:url-a1", + }); + expect(revoke).not.toHaveBeenCalled(); + store.ensureThumbnail("a2"); + await vi.waitFor(() => + expect(store.getSnapshot().displayFrames.s1).toEqual({ + status: "ready", + url: "blob:url-a2", + }), + ); expect(store.getSnapshot().thumbnails["a1"]).toBeUndefined(); expect(revoke).toHaveBeenCalledWith("blob:url-a1"); store.stop(); }); + it("keeps the last good frame in displayFrames when the next load fails", async () => { + const { store, esInstances, loadImage } = harness([ + { ...OBS_IDLE, thumbnailAttachmentId: "a1" }, + ]); + store.start(); + await vi.waitFor(() => expect(store.getSnapshot().sessions).toHaveLength(1)); + store.ensureThumbnail("a1"); + await vi.waitFor(() => expect(store.getSnapshot().thumbnails["a1"].status).toBe("ready")); + loadImage.mockRejectedValueOnce(new Error("nope")); + emit(esInstances[0], { + type: "upsert", + session: { ...OBS_IDLE, thumbnailAttachmentId: "a2" }, + }); + store.ensureThumbnail("a2"); + await vi.waitFor(() => expect(store.getSnapshot().thumbnails["a2"]?.status).toBe("error")); + expect(store.getSnapshot().displayFrames.s1).toEqual({ + status: "error", + url: "blob:url-a1", + }); + store.stop(); + }); + it("remove and reset revoke the session's tracked frame", async () => { const revoke = withRevokeSpy(); const { store, esInstances } = harness([{ ...OBS_IDLE, thumbnailAttachmentId: "a1" }]);