fix(app): terminal state corruption

Author: adamdotdevin

packages/app/src/context/terminal.test.ts

@@ -2,6 +2,7 @@ import { beforeAll, describe, expect, mock, test } from "bun:test"
 
 let getWorkspaceTerminalCacheKey: (dir: string) => string
 let getLegacyTerminalStorageKeys: (dir: string, legacySessionID?: string) => string[]
+let migrateTerminalState: (value: unknown) => unknown
 
 beforeAll(async () => {
   mock.module("@solidjs/router", () => ({
@@ -17,6 +18,7 @@ beforeAll(async () => {
   const mod = await import("./terminal")
   getWorkspaceTerminalCacheKey = mod.getWorkspaceTerminalCacheKey
   getLegacyTerminalStorageKeys = mod.getLegacyTerminalStorageKeys
+  migrateTerminalState = mod.migrateTerminalState
 })
 
 describe("getWorkspaceTerminalCacheKey", () => {
@@ -37,3 +39,44 @@ describe("getLegacyTerminalStorageKeys", () => {
     ])
   })
 })
+
+describe("migrateTerminalState", () => {
+  test("drops invalid terminals and restores a valid active terminal", () => {
+    expect(
+      migrateTerminalState({
+        active: "missing",
+        all: [
+          null,
+          { id: "one", title: "Terminal 2" },
+          { id: "one", title: "duplicate", titleNumber: 9 },
+          { id: "two", title: "logs", titleNumber: 4, rows: 24, cols: 80 },
+          { title: "no-id" },
+        ],
+      }),
+    ).toEqual({
+      active: "one",
+      all: [
+        { id: "one", title: "Terminal 2", titleNumber: 2 },
+        { id: "two", title: "logs", titleNumber: 4, rows: 24, cols: 80 },
+      ],
+    })
+  })
+
+  test("keeps a valid active id", () => {
+    expect(
+      migrateTerminalState({
+        active: "two",
+        all: [
+          { id: "one", title: "Terminal 1" },
+          { id: "two", title: "shell", titleNumber: 7 },
+        ],
+      }),
+    ).toEqual({
+      active: "two",
+      all: [
+        { id: "one", title: "Terminal 1", titleNumber: 1 },
+        { id: "two", title: "shell", titleNumber: 7 },
+      ],
+    })
+  })
+})

packages/app/src/context/terminal.tsx

@@ -20,6 +20,71 @@ export type LocalPTY = {
 const WORKSPACE_KEY = "__workspace__"
 const MAX_TERMINAL_SESSIONS = 20
 
+function record(value: unknown): value is Record<string, unknown> {
+  return typeof value === "object" && value !== null && !Array.isArray(value)
+}
+
+function text(value: unknown) {
+  return typeof value === "string" ? value : undefined
+}
+
+function num(value: unknown) {
+  return typeof value === "number" && Number.isFinite(value) ? value : undefined
+}
+
+function numberFromTitle(title: string) {
+  const match = title.match(/^Terminal (\d+)$/)
+  if (!match) return
+  const value = Number(match[1])
+  if (!Number.isFinite(value) || value <= 0) return
+  return value
+}
+
+function pty(value: unknown): LocalPTY | undefined {
+  if (!record(value)) return
+
+  const id = text(value.id)
+  if (!id) return
+
+  const title = text(value.title) ?? ""
+  const number = num(value.titleNumber)
+  const rows = num(value.rows)
+  const cols = num(value.cols)
+  const buffer = text(value.buffer)
+  const scrollY = num(value.scrollY)
+  const cursor = num(value.cursor)
+
+  return {
+    id,
+    title,
+    titleNumber: number && number > 0 ? number : (numberFromTitle(title) ?? 0),
+    ...(rows !== undefined ? { rows } : {}),
+    ...(cols !== undefined ? { cols } : {}),
+    ...(buffer !== undefined ? { buffer } : {}),
+    ...(scrollY !== undefined ? { scrollY } : {}),
+    ...(cursor !== undefined ? { cursor } : {}),
+  }
+}
+
+export function migrateTerminalState(value: unknown) {
+  if (!record(value)) return value
+
+  const seen = new Set<string>()
+  const all = (Array.isArray(value.all) ? value.all : []).flatMap((item) => {
+    const next = pty(item)
+    if (!next || seen.has(next.id)) return []
+    seen.add(next.id)
+    return [next]
+  })
+
+  const active = text(value.active)
+
+  return {
+    active: active && seen.has(active) ? active : all[0]?.id,
+    all,
+  }
+}
+
 export function getWorkspaceTerminalCacheKey(dir: string) {
   return `${dir}:${WORKSPACE_KEY}`
 }
@@ -71,16 +136,11 @@ export function clearWorkspaceTerminals(dir: string, sessionIDs?: string[], plat
 function createWorkspaceTerminalSession(sdk: ReturnType<typeof useSDK>, dir: string, legacySessionID?: string) {
   const legacy = getLegacyTerminalStorageKeys(dir, legacySessionID)
 
-  const numberFromTitle = (title: string) => {
-    const match = title.match(/^Terminal (\d+)$/)
-    if (!match) return
-    const value = Number(match[1])
-    if (!Number.isFinite(value) || value <= 0) return
-    return value
-  }
-
   const [store, setStore, _, ready] = persisted(
-    Persist.workspace(dir, "terminal", legacy),
+    {
+      ...Persist.workspace(dir, "terminal", legacy),
+      migrate: migrateTerminalState,
+    },
     createStore<{
       active?: string
       all: LocalPTY[]
@@ -128,26 +188,6 @@ function createWorkspaceTerminalSession(sdk: ReturnType<typeof useSDK>, dir: str
   })
   onCleanup(unsub)
 
-  const meta = { migrated: false }
-
-  createEffect(() => {
-    if (!ready()) return
-    if (meta.migrated) return
-    meta.migrated = true
-
-    setStore("all", (all) => {
-      const next = all.map((pty) => {
-        const direct = Number.isFinite(pty.titleNumber) && pty.titleNumber > 0 ? pty.titleNumber : undefined
-        if (direct !== undefined) return pty
-        const parsed = numberFromTitle(pty.title)
-        if (parsed === undefined) return pty
-        return { ...pty, titleNumber: parsed }
-      })
-      if (next.every((pty, index) => pty === all[index])) return all
-      return next
-    })
-  })
-
   return {
     ready,
     all: createMemo(() => store.all),