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

@@ -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

packages/dsh-plugin-browserskill/src/client/ObservationOverlay.module.css

@@ -79,6 +79,8 @@
   overflow: hidden;
 }
 
+/* Fade only on first mount. The <img> 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%;

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 (
     <div
@@ -158,7 +158,7 @@ function StripItem(props: {
         onClick={() => onTogglePin(obs.sessionId)}
       >
         <span className={css["strip-thumb"]}>
-          {thumb?.status === "ready" && thumb.url !== undefined ? (
+          {thumb?.url !== undefined ? (
             <img src={thumb.url} alt="" />
           ) : (
             <span className={css["strip-thumb-empty"]} />
@@ -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 <img> 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}
       </div>
       <div className={css.stage}>
-        {thumb?.status === "ready" && thumb.url !== undefined ? (
+        {displayUrl !== undefined ? (
           <img
-            key={thumb.url}
             className={css.thumb}
-            src={thumb.url}
+            src={displayUrl}
             alt={`session ${focus?.sessionId ?? ""} view`}
           />
         ) : (

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<Record<string, ThumbnailState>>;
+  /**
+   * 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<Record<string, ThumbnailState>>;
   /** 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<string, SessionObservation>();
   private thumbs = new Map<string, ThumbnailState>();
-  /** sessionId → the attachment id currently displayed for it (leak guard). */
+  /** sessionId → the attachment id currently advertised for it. */
   private readonly thumbBySession = new Map<string, string>();
+  /** sessionId → last successfully decoded frame (held across the next load). */
+  private readonly lastReady = new Map<string, { attachmentId: string; url: string }>();
   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<string, ThumbnailState> {
+    const frames: Record<string, ThumbnailState> = {};
+    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();
       },
       () => {

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<string>((resolve) => {
+          resolveNext = resolve;
+        });
+      }
+      return `blob:${id}`;
+    });
+    render(<ObservationOverlay store={h.store} />);
+    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 <img> 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(<ObservationOverlay store={h.store} />);

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" }]);