fix(dashboard): close terminal bootstrap residuals from the terminal review

- Windows clients no longer force isReady(true) on mount: the validation
  effect owns isReady on every path, so persisted tabs are server-validated
  before xterm connects (previously Windows could attach to a dead session
  first and recover via 4004).
- isWindowsBrowserClient() matches 'Windows NT' minus 'Windows Phone' so
  phone UAs are not needlessly denied first-tab auto-create.
- normalizeActiveTab() now also collapses MULTIPLE active tabs to the first
  active one (zero-active was already handled), and readTabsFromStorage
  drops malformed persisted entries individually instead of discarding the
  whole payload's valid siblings via the outer catch.
- Regression tests for all four paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-23 19:49:43 -07:00
parent faeb491ea4
commit fbaf3c56d2
2 changed files with 141 additions and 8 deletions

View File

@@ -376,6 +376,60 @@ describe("useTerminalSessions", () => {
expect(result.current.autoCreateDisabled).toBe(false);
});
it("does not disable auto-create for Windows Phone user agents", async () => {
// The guard exists for desktop wt.exe dialogs; a bare "Windows" substring
// also matched Windows Phone UAs and needlessly denied them auto-create.
setUserAgent(
"Mozilla/5.0 (Windows Phone 10.0; Android 6.0.1; Microsoft; Lumia 950) Edge/14.14263",
);
const { result } = renderHook(() => useTerminalSessions(TEST_PROJECT_ID));
await waitFor(() => {
expect(result.current.autoCreateDisabled).toBe(false);
expect(result.current.tabs.length).toBe(1);
});
expect(mockCreateTerminalSession).toHaveBeenCalledTimes(1);
});
it("waits for server validation before isReady on Windows with persisted tabs", async () => {
// The Windows branch used to force isReady(true) on mount, letting xterm
// connect to a persisted-but-dead session before validation pruned it.
setUserAgent("Mozilla/5.0 (Windows NT 10.0; Win64; x64) Chrome/126.0");
const storedTabs = [
{ id: "tab-dead", sessionId: "session-dead", title: "bash", isActive: true, createdAt: 1 },
];
localStorageMock.getItem.mockImplementation((key: string) =>
key === TERMINAL_TABS_KEY ? JSON.stringify(storedTabs) : null,
);
let resolveList: (sessions: unknown[]) => void = () => {};
mockListTerminalSessions.mockImplementation(
() =>
new Promise((resolve) => {
resolveList = resolve as unknown as (sessions: unknown[]) => void;
}),
);
const { result } = renderHook(() => useTerminalSessions(TEST_PROJECT_ID));
// Validation is still in flight: Windows must NOT short-circuit isReady.
await act(async () => {
await new Promise((resolve) => setTimeout(resolve, 10));
});
expect(result.current.isReady).toBe(false);
// Server says the persisted session is gone — validation prunes it.
await act(async () => {
resolveList([]);
});
await waitFor(() => {
expect(result.current.isReady).toBe(true);
});
expect(result.current.tabs.length).toBe(0);
expect(mockCreateTerminalSession).not.toHaveBeenCalled();
});
});
describe("persisted tab restore normalization", () => {
@@ -430,6 +484,53 @@ describe("useTerminalSessions", () => {
expect(result.current.activeTab).not.toBeNull();
expect(result.current.activeTab?.id).toBe("tab-a");
});
it("collapses multiple active tabs to the first active one", async () => {
// Several active-styled tabs render at once while only the first receives
// input; the inconsistency also persists back to storage.
const storedTabs = [
{ id: "tab-a", sessionId: "session-a", title: "bash", isActive: false, createdAt: 1 },
{ id: "tab-b", sessionId: "session-b", title: "zsh", isActive: true, createdAt: 2 },
{ id: "tab-c", sessionId: "session-c", title: "fish", isActive: true, createdAt: 3 },
];
localStorageMock.getItem.mockImplementation((key: string) =>
key === TERMINAL_TABS_KEY ? JSON.stringify(storedTabs) : null,
);
mockListTerminalSessions.mockRejectedValue(new Error("server unreachable"));
const { result } = renderHook(() => useTerminalSessions(TEST_PROJECT_ID));
await waitFor(() => {
expect(result.current.isReady).toBe(true);
});
expect(result.current.tabs.filter((tab) => tab.isActive)).toHaveLength(1);
expect(result.current.activeTab?.id).toBe("tab-b");
});
it("drops malformed persisted entries without discarding valid sibling tabs", async () => {
// One null/shape-less element used to throw inside normalization and the
// outer catch returned [], silently discarding the valid tabs beside it.
const storedPayload = [
null,
{ bogus: true },
{ id: "tab-good", sessionId: "session-good", title: "bash", isActive: false, createdAt: 1 },
];
localStorageMock.getItem.mockImplementation((key: string) =>
key === TERMINAL_TABS_KEY ? JSON.stringify(storedPayload) : null,
);
mockListTerminalSessions.mockResolvedValue([{ id: "session-good" }] as never);
const { result } = renderHook(() => useTerminalSessions(TEST_PROJECT_ID));
await waitFor(() => {
expect(result.current.isReady).toBe(true);
});
expect(result.current.tabs).toHaveLength(1);
expect(result.current.activeTab?.id).toBe("tab-good");
expect(mockCreateTerminalSession).not.toHaveBeenCalled();
});
});
describe("bootstrap sequencing (FN-7686)", () => {

View File

@@ -90,7 +90,13 @@ instead of an infinite "Starting terminal..." spinner that only the tab-strip
"+" button escapes.
*/
function isWindowsBrowserClient(): boolean {
return typeof window !== "undefined" && window.navigator.userAgent.includes("Windows");
if (typeof window === "undefined") return false;
const ua = window.navigator.userAgent;
// FNXC:Terminal 2026-07-23-21:00: match desktop Windows only. Every real
// desktop Windows browser (including Chromium's frozen/reduced UA) carries
// "Windows NT"; a bare "Windows" substring also matched Windows Phone UAs,
// which have no wt.exe to guard against and were needlessly denied auto-create.
return ua.includes("Windows NT") && !ua.includes("Windows Phone");
}
function terminalTabsStorageKey(storageScope?: string): string {
@@ -108,10 +114,20 @@ tie-break (activate the first tab) for BOTH the storage-read boundary and the
server-validation success branch so the two paths cannot drift.
*/
function normalizeActiveTab(tabs: TerminalTab[]): TerminalTab[] {
if (tabs.length === 0 || tabs.some((tab) => tab.isActive)) {
return tabs;
}
return tabs.map((tab, i) => ({ ...tab, isActive: i === 0 }));
if (tabs.length === 0) return tabs;
const activeCount = tabs.reduce((count, tab) => (tab.isActive ? count + 1 : count), 0);
if (activeCount === 1) return tabs;
/*
FNXC:Terminal 2026-07-23-21:00:
Zero active tabs wedges the "Starting terminal..." spinner (activeTab drives
the whole modal); MULTIPLE active tabs render several active-styled tabs while
only the first receives input, and the inconsistency persists back to storage.
Collapse both cases to exactly one active tab: the first currently-active one,
or the first tab when none is active.
*/
const firstActiveIndex = tabs.findIndex((tab) => tab.isActive);
const activeIndex = firstActiveIndex === -1 ? 0 : firstActiveIndex;
return tabs.map((tab, i) => ({ ...tab, isActive: i === activeIndex }));
}
function readTabsFromStorage(projectId?: string, storageScope?: string): TerminalTab[] {
@@ -120,11 +136,20 @@ function readTabsFromStorage(projectId?: string, storageScope?: string): Termina
try {
const stored = getScopedItem(terminalTabsStorageKey(storageScope), projectId);
if (stored) {
const parsed = JSON.parse(stored) as TerminalTab[];
const parsed = JSON.parse(stored) as unknown;
if (!Array.isArray(parsed)) return [];
// Drop malformed entries individually instead of letting one null/shape-less
// element throw and discard the payload's valid sibling tabs via the outer catch.
const validTabs = parsed.filter(
(tab): tab is TerminalTab =>
!!tab &&
typeof tab === "object" &&
typeof (tab as TerminalTab).id === "string" &&
typeof (tab as TerminalTab).sessionId === "string",
);
// Normalize here (not only in server validation) because the
// validation-FAILURE path keeps tabs exactly as read from storage.
return normalizeActiveTab(parsed);
return normalizeActiveTab(validTabs);
}
} catch {
// Ignore localStorage errors
@@ -326,8 +351,15 @@ export function useTerminalSessions(projectId?: string, options: UseTerminalSess
// (wt.exe) and produce native "Help" version dialogs. Users can still create a terminal
// explicitly from the UI.
useEffect(() => {
/*
FNXC:Terminal 2026-07-23-21:00:
The Windows skip must NOT force isReady(true) here: the validation effect
above already sets isReady on every path (zero-tabs skip, success, failure),
and forcing it on mount let Windows clients connect xterm to persisted tabs
BEFORE server validation had pruned dead sessions. Skipping auto-create is
the only Windows-specific behavior this effect owns.
*/
if (isWindowsBrowserClient()) {
setIsReady(true);
return;
}
if (tabs.length === 0 && isReady && serverAvailable && !bootstrapError) {