fix(gui-extensions): close lifetime and storage gaps found in review

Release SSH resources as each is acquired, follow re-authenticated server data in the browser model, share one main storage cache per key, clear stale createLatest errors, restrict main provide to declared Ipcs, add createKeyed equals for object sources, and derive the review mode instead of overwriting the stored choice.
Hona · Oct 4, 2026 · Oct 2, 2026 · Oct 2, 2026 · Oct 2, 2026 · Oct 2, 2026
commit 7c33ac1456ee5d528a38e4f80f1d7565f9c2af36
@@ -12,11 +12,13 @@ for (const view of ["desktop", "mobile"] as const) {
   test(`offers Git initialization for a project without VCS (${view})`, async ({ page }) => {
     if (view === "mobile") await page.setViewportSize({ width: 390, height: 844 })
     const requests: { directory: string; provider?: string }[] = []
+
     const workspace = await openSession(page, {
       name: "ReviewWithoutGit",
       project: { vcs: undefined },
       onVcsInit: (input) => requests.push(input),
     })
+
     if (view === "desktop") await page.getByRole("button", { name: "Toggle review" }).click()
     else await page.getByRole("tablist", { name: "Session view" }).getByRole("tab", { name: "Changes" }).click()
 
@@ -35,6 +37,7 @@ test("open file tab browses, searches, and tracks missing files", async ({ page
   const searches: { query: string; dirs?: string; limit?: number }[] = []
   const directory = "C:/OpenCode/ReviewOpenFile"
   const files = Array.from({ length: 80 }, (_, index) => `file-${String(index).padStart(2, "0")}.ts`)
+
   const workspace = await openSession(page, {
     name: "ReviewOpenFile",
     vcsDiff: [fileDiff("src/changed.ts")],
@@ -49,6 +52,7 @@ test("open file tab browses, searches, and tracks missing files", async ({ page
     fileContent: (path) => ({ type: "text", content: `contents:${path}` }),
     findFiles: (input) => {
       searches.push(input)
+
       return input.query === "nested" ? [fileNode(directory, "src/nested.ts")] : []
     },
     seed: {
@@ -90,6 +94,8 @@ test("open file tab browses, searches, and tracks missing files", async ({ page
     route.fulfill({
       status: 404,
       headers: { "access-control-allow-origin": "*" },
+      // SAFETY: the server's wire body for a missing file, which the client decodes into its own error.
+      // oxlint-disable-next-line anti-slop-effect/no-manual-tagged-construction -- see SAFETY above
       json: { _tag: "FileNotFoundError", path: "README.md", message: "File not found: README.md" },
     }),
   )
@@ -142,9 +148,10 @@ test("open file tab browses, searches, and tracks missing files", async ({ page
     .toBeLessThanOrEqual(1)
   const scrolled = await viewport.evaluate((element) => element.scrollTop)
   expect(scrolled).toBeGreaterThan(0)
-  await root.evaluate((element) => ((element as HTMLElement & { e2eProbe?: string }).e2eProbe = "original"))
+  await root.evaluate((element) => void (element.dataset.e2eProbe = "original"))
+
   const expectSidebarKept = async () => {
-    expect(await root.evaluate((element) => (element as HTMLElement & { e2eProbe?: string }).e2eProbe)).toBe("original")
+    expect(await root.evaluate((element) => element.dataset.e2eProbe)).toBe("original")
     await expect.poll(() => viewport.evaluate((element) => element.scrollTop)).toBe(scrolled)
   }
 
@@ -269,7 +276,9 @@ test("file tree expands Windows paths and scrolls long names in both directions"
           { ...fileNode(directory, longPath), path: `frontend\\${longFilename}` },
         ]
       }
+
       if (path) return []
+
       return [
         { ...fileNode(directory, "frontend", "directory"), name: "", path: "frontend\\" },
         fileNode(directory, "README.md"),
@@ -371,6 +380,7 @@ test("a restored file tab keeps the file tree on Files Changed", async ({ page }
 
 test("rereads a file each time an artifact link opens it", async ({ page }) => {
   const file = { content: "first draft" }
+
   const messages = [
     { id: "msg_prompt", type: "user", text: "Write the shopping list", time: { created: 1 } },
     {
@@ -382,6 +392,7 @@ test("rereads a file each time an artifact link opens it", async ({ page }) => {
       time: { created: 2, completed: 3 },
     },
   ] satisfies SessionMessageInfo[]
+
   await openSession(page, {
     name: "ArtifactReopen",
     fileList: () => [],
@@ -405,6 +416,9 @@ test("rereads a file each time an artifact link opens it", async ({ page }) => {
   await expect(panel.getByText("first draft", { exact: true })).toHaveCount(0)
 })
 
+/** What the image case's frame probe records on the window. */
+type ImageProbe = { problems: string[]; frames: number; running: boolean }
+
 test("image files keep the review panel painted while loading", async ({ page }) => {
   await page.setViewportSize({ width: 960, height: 900 })
   const directory = "C:/OpenCode/ReviewImage"
@@ -418,12 +432,16 @@ test("image files keep the review panel painted while loading", async ({ page })
       if (path !== image) return undefined
       read.resolve()
       await release.promise
+
       return { type: "binary", content: "iVBORw0KGgo=", encoding: "base64", mimeType: "image/png" }
     },
     fileList: (path) => {
       if (!path) return [fileNode(directory, "assets", "directory"), fileNode(directory, "src", "directory")]
+
       if (path === "assets") return [fileNode(directory, image)]
+
       if (path === "src") return [fileNode(directory, "src/example.ts")]
+
       return []
     },
   })
@@ -432,27 +450,37 @@ test("image files keep the review panel painted while loading", async ({ page })
 
   // Records every painted frame from before the click until after the image read completes.
   await page.evaluate(() => {
-    const probe = { problems: [] as string[], frames: 0, running: true }
+    const probe: ImageProbe = { problems: [], frames: 0, running: true }
+
     const sample = () => {
       const panel = document.querySelector<HTMLElement>('#review-panel [data-component="session-review-v2"]')
       const rect = panel?.getBoundingClientRect()
+
       const hit =
         rect?.width && rect.height ? document.elementFromPoint(rect.x + rect.width / 2, rect.y + rect.height / 2) : null
+
       const frame = [
         !panel?.checkVisibility() && "hidden",
         !panel?.textContent?.trim() && "blank",
         !(hit && document.querySelector("#review-panel")?.contains(hit)) && "outside the review panel",
         hit && getComputedStyle(hit).backgroundColor === "rgb(0, 0, 0)" && "black",
-      ].filter((problem) => typeof problem === "string")
+      ].filter((problem) => problem !== false && problem !== null)
+
       probe.problems.push(...frame)
       probe.frames += 1
+
       if (probe.running) requestAnimationFrame(sample)
     }
-    ;(window as Window & { e2eImageProbe?: typeof probe }).e2eImageProbe = probe
+
+    // The evaluates below read it.
+    Object.assign(window, { e2eImageProbe: probe })
     requestAnimationFrame(sample)
   })
+
   const frames = () =>
-    page.evaluate(() => (window as Window & { e2eImageProbe?: { frames: number } }).e2eImageProbe!.frames)
+    // SAFETY: the evaluate above stored the probe on the window before any frame is counted.
+    page.evaluate(() => (window as Window & { e2eImageProbe?: ImageProbe }).e2eImageProbe!.frames)
+
   const response = page.waitForResponse((item) => item.url().includes(`/api/fs/read/${image}`))
   await page.getByRole("button", { name: /preview\.png/ }).click()
   await read.promise
@@ -465,8 +493,10 @@ test("image files keep the review panel painted while loading", async ({ page })
   await expect.poll(frames).toBeGreaterThan(loaded)
   expect(
     await page.evaluate(() => {
-      const probe = (window as Window & { e2eImageProbe?: { problems: string[]; running: boolean } }).e2eImageProbe!
+      // SAFETY: the evaluate above stored the probe on the window.
+      const probe = (window as Window & { e2eImageProbe?: ImageProbe }).e2eImageProbe!
       probe.running = false
+
       return probe.problems
     }),
   ).toEqual([])
@@ -478,6 +508,7 @@ test("restores review state and the side-panel tab per session", async ({ page }
     { id: "ses_review_state_b", title: "Beta review state" },
     { id: "ses_review_state_c", title: "Gamma review state" },
   ]
+
   const directory = "C:/OpenCode/ReviewState"
   await openSession(page, {
     name: "ReviewState",
@@ -493,10 +524,13 @@ test("restores review state and the side-panel tab per session", async ({ page }
   const panel = page.locator("#review-panel")
   const review = panel.locator("#session-side-panel-review-tab")
   const toggle = page.getByRole("button", { name: "Toggle review" })
+
   const selectedTab = (name: string) =>
     expect(panel.getByRole("tab", { name, exact: true })).toHaveAttribute("aria-selected", "true")
+
   const selectedFile = (file: string) =>
     expect(page.locator('[data-slot="session-review-v2-file-name"]')).toHaveText(file)
+
   const switchSession = async (title: string) => {
     await page.locator("[data-titlebar-tab-slot]", { hasText: title }).click()
     await expectSessionTitle(page, title)
@@ -603,6 +637,7 @@ test("keeps the review state a session stored before extensions", async ({ page
   })
   const mode = (name: string) => page.getByRole("button", { name, exact: true })
   const changes = page.getByRole("tablist", { name: "Session view", exact: true }).getByRole("tab", { name: "Changes" })
+
   const trigger = (file: string) =>
     page
       .locator(`[data-component="session-review"] [data-file="${file}"]`)
@@ -643,8 +678,10 @@ test("desktop review waits for the session's stored mode before it loads changes
   const working: string[] = []
   page.on("request", (request) => {
     const url = new URL(request.url())
+
     if (url.pathname.endsWith("/vcs/diff") && url.searchParams.get("mode") === "working") working.push(request.url())
   })
+
   const desktop = (hold?: string) =>
     `/e2e/utils/settings-wsl.html?${new URLSearchParams({
       server: SERVER,
@@ -653,6 +690,7 @@ test("desktop review waits for the session's stored mode before it loads changes
       path: `/server/${base64Encode("sidecar")}/session/${id}`,
       ...(hold && { hold }),
     })}`
+
   const mode = (name: string) => page.getByRole("button", { name, exact: true })
   const file = page.locator('[data-slot="session-review-v2-file-name"]')
   const toggle = page.getByRole("button", { name: "Toggle review", exact: true })
@@ -686,6 +724,36 @@ test("desktop review waits for the session's stored mode before it loads changes
   expect(working).toEqual([])
 })
 
+test("a chosen mode the session stops offering shows as Git, and comes back when it is offered again", async ({
+  page,
+}) => {
+  // The mock server reads the branches on each request, so a reload sees the session's branch change.
+  const vcs = { current: "feature", default: "dev" }
+  await openSession(page, {
+    name: "ReviewModeBack",
+    vcs,
+    vcsDiff: ({ mode }) => (mode === "branch" ? [fileDiff("beta.ts")] : [fileDiff("gamma.ts")]),
+  })
+  const mode = (name: string) => page.getByRole("button", { name, exact: true })
+  const file = page.locator('[data-slot="session-review-v2-file-name"]')
+
+  await page.getByRole("button", { name: "Toggle review", exact: true }).click()
+  await mode("Git changes").click()
+  await page.getByRole("option", { name: "Branch changes" }).click()
+  await expect(file).toHaveText("beta.ts")
+
+  // Back on the default branch, Branch is not offered.
+  vcs.current = "dev"
+  await page.reload()
+  await expect(mode("Git changes")).toBeVisible()
+  await expect(file).toHaveText("gamma.ts")
+
+  vcs.current = "feature"
+  await page.reload()
+  await expect(mode("Branch changes")).toBeVisible()
+  await expect(file).toHaveText("beta.ts")
+})
+
 for (const direction of ["ltr", "rtl"] as const) {
   test(`review toggle stays at the header edge in ${direction}`, async ({ page }) => {
     await page.setViewportSize({ width: 1000, height: 900 })
@@ -708,6 +776,7 @@ for (const direction of ["ltr", "rtl"] as const) {
     await expect
       .poll(async () => {
         const box = (await panel.boundingBox())!
+
         return (
           closed.x >= box.x &&
           closed.x + closed.width <= box.x + box.width &&
@@ -719,6 +788,7 @@ for (const direction of ["ltr", "rtl"] as const) {
     await expect
       .poll(async () => {
         const box = await panel.locator('[data-slot="session-side-panel-actions"]').boundingBox()
+
         return box ? box.y + box.height / 2 : undefined
       })
       .toBe(closed.y + closed.height / 2)
@@ -731,14 +801,17 @@ for (const direction of ["ltr", "rtl"] as const) {
     // Pause in the same task as each click so even the first painted state can be inspected.
     for (const opened of [true, false]) {
       await toggle.evaluate((element) => {
-        ;(element as HTMLButtonElement).click()
+        // SAFETY: `toggle` is the review toggle, a button.
+        const button = element as HTMLButtonElement
+        button.click()
         document
           .getAnimations()
           .filter((animation) => animation.timeline instanceof DocumentTimeline)
           .forEach((animation) => animation.pause())
       })
       await expect(toggle).toHaveAttribute("aria-expanded", String(opened))
       await expect(panel).toHaveAttribute("aria-hidden", String(!opened))
+
       for (const progress of [0.25, 0.8]) await expectHeaderClearOfToggle(page, toggle, progress)
       await page.evaluate(() =>
         document
@@ -747,6 +820,7 @@ for (const direction of ["ltr", "rtl"] as const) {
           .forEach((animation) => animation.finish()),
       )
     }
+
     await expect(panel).toBeHidden()
   })
 }
@@ -764,6 +838,7 @@ async function statusInset(status: Locator, side: "left" | "right") {
   return status.evaluate((element, side) => {
     const viewport = element.closest<HTMLElement>(".scroll-view__viewport")!.getBoundingClientRect()
     const box = element.getBoundingClientRect()
+
     return side === "right" ? viewport.right - box.right : box.left - viewport.left
   }, side)
 }
@@ -781,18 +856,22 @@ function filesystemEvent(directory: string, file: string, event: "add" | "change
 async function expectHeaderClearOfToggle(page: Page, toggle: Locator, progress: number) {
   const geometry = await page.locator('[data-slot="session-chat-panel"]').evaluate((chat, progress) => {
     const row = chat.parentElement!
+
     const animations = row
       .getAnimations({ subtree: true })
       .filter((animation) => animation.timeline instanceof DocumentTimeline)
+
     const width = animations.find(
       (animation) => animation instanceof CSSTransition && animation.transitionProperty === "width",
     )!
+
     animations.forEach((animation) => {
       animation.pause()
       animation.currentTime = Number(width.effect!.getTiming().duration) * progress
     })
     const chatBounds = chat.getBoundingClientRect()
     const panelBounds = document.querySelector("#review-panel")!.getBoundingClientRect()
+
     return {
       row: row.getBoundingClientRect().width,
       panelWidth: panelBounds.width,
@@ -804,8 +883,11 @@ async function expectHeaderClearOfToggle(page: Page, toggle: Locator, progress:
       panels: chatBounds.width + panelBounds.width + parseFloat(getComputedStyle(row).columnGap),
     }
   }, progress)
+
   expect(geometry.gap).toBeCloseTo(8, 1)
+
   if (geometry.panelWidth > 0) expect(Math.abs(geometry.row - geometry.panels)).toBeLessThanOrEqual(1)
+
   if (progress === 0.25) {
     expect(geometry.contentOpacity).toBeGreaterThan(0)
     expect(geometry.contentOpacity).toBeLessThan(1)

@@ -130,6 +130,61 @@ describe("extension primitives", () => {
     dispose()
   })
 
+  test("createKeyed with equals keys an object source by the fields it compares", () => {
+    const [read, write] = createSignal({ directory: "a", loading: true })
+    const log: string[] = []
+
+    const dispose = createRoot((dispose) => {
+      createKeyed(
+        () => ({ directory: read().directory, loading: read().loading }),
+        (value) => {
+          log.push(`run ${value.directory}`)
+          onCleanup(() => log.push(`end ${value.directory}`))
+        },
+        { equals: (previous, next) => previous.directory === next.directory },
+      )
+
+      return dispose
+    })
+
+    // The listing loads, which is the same key; then the directory changes.
+    write({ directory: "a", loading: false })
+    write({ directory: "b", loading: true })
+    expect(log).toEqual(["run a", "end a", "run b"])
+    dispose()
+  })
+
+  test("createLatest clears the error when the next request starts and while the source has no value", async () => {
+    const [key, setKey] = createSignal<string | undefined>("a")
+    const requests = new Map<string, PromiseWithResolvers<string>>()
+
+    const root = createRoot((dispose) => ({
+      dispose,
+      latest: createLatest(key, (value) => {
+        const reply = Promise.withResolvers<string>()
+
+        requests.set(value, reply)
+
+        return reply.promise
+      }),
+    }))
+
+    const error = () => (root.latest.error instanceof Error ? root.latest.error.message : root.latest.error)
+    const seen: unknown[] = []
+    requests.get("a")?.reject(new Error("a failed"))
+    await Bun.sleep(0)
+    seen.push(error())
+    setKey("b")
+    seen.push(error())
+    requests.get("b")?.reject(new Error("b failed"))
+    await Bun.sleep(0)
+    seen.push(error())
+    setKey(undefined)
+    seen.push(error())
+    expect(seen).toEqual(["a failed", undefined, "b failed", undefined])
+    root.dispose()
+  })
+
   test("createLatest aborts the previous request and drops its late reply", async () => {
     const [key, setKey] = createSignal<string | undefined>("a")
     const requests = new Map<string, { signal: AbortSignal; reply: PromiseWithResolvers<string> }>()