From 794532928f76c197181bc80944e8be1f6e5eda9a Mon Sep 17 00:00:00 2001 From: Adam <2363879+adamdotdevin@users.noreply.github.com> Date: Mon, 9 Mar 2026 15:28:29 -0500 Subject: [PATCH] fix(app): terminal state corruption --- packages/app/src/context/terminal.test.ts | 43 ++++++++++ packages/app/src/context/terminal.tsx | 98 ++++++++++++++++------- 2 files changed, 112 insertions(+), 29 deletions(-) diff --git a/packages/app/src/context/terminal.test.ts b/packages/app/src/context/terminal.test.ts index a250de57c..6e07e0312 100644 --- a/packages/app/src/context/terminal.test.ts +++ b/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 }, + ], + }) + }) +}) diff --git a/packages/app/src/context/terminal.tsx b/packages/app/src/context/terminal.tsx index 4467495b7..a2807375f 100644 --- a/packages/app/src/context/terminal.tsx +++ b/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 { + 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() + 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, 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, 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),