Merge pull request #2592 from openchamber/terminal-open-debug
fix(terminal): start PTY before viewport mounts without dropping output or replies
This commit is contained in:
@@ -18,6 +18,42 @@ import type { TerminalChunk } from '@/stores/useTerminalStore';
|
||||
let ghosttyPromise: Promise<Ghostty> | null = null;
|
||||
const loadGhostty = (): Promise<Ghostty> => ghosttyPromise ??= Ghostty.load();
|
||||
|
||||
type TerminalSize = { cols: number; rows: number };
|
||||
|
||||
const getProvisionalTerminalSize = (
|
||||
container: HTMLDivElement,
|
||||
fontFamily: string,
|
||||
fontSize: number,
|
||||
): TerminalSize | null => {
|
||||
if (typeof window === 'undefined' || typeof document === 'undefined') return null;
|
||||
|
||||
const context = document.createElement('canvas').getContext('2d');
|
||||
if (!context || container.clientWidth < 24 || container.clientHeight < 24) return null;
|
||||
|
||||
context.font = `${fontSize}px ${fontFamily}`;
|
||||
const metrics = context.measureText('M');
|
||||
const cellWidth = Math.ceil(metrics.width);
|
||||
const cellHeight = Math.ceil(
|
||||
(metrics.actualBoundingBoxAscent || fontSize * 0.8) +
|
||||
(metrics.actualBoundingBoxDescent || fontSize * 0.2),
|
||||
) + 2;
|
||||
if (cellWidth < 1 || cellHeight < 1) return null;
|
||||
|
||||
const style = window.getComputedStyle(container);
|
||||
const horizontalPadding =
|
||||
(Number.parseInt(style.paddingLeft, 10) || 0) +
|
||||
(Number.parseInt(style.paddingRight, 10) || 0);
|
||||
const verticalPadding =
|
||||
(Number.parseInt(style.paddingTop, 10) || 0) +
|
||||
(Number.parseInt(style.paddingBottom, 10) || 0);
|
||||
|
||||
// Match Ghostty FitAddon's 15px scrollbar reservation and minimum dimensions.
|
||||
return {
|
||||
cols: Math.max(2, Math.floor((container.clientWidth - horizontalPadding - 15) / cellWidth)),
|
||||
rows: Math.max(1, Math.floor((container.clientHeight - verticalPadding) / cellHeight)),
|
||||
};
|
||||
};
|
||||
|
||||
export type TerminalController = {
|
||||
focus: () => void;
|
||||
fit: () => void;
|
||||
@@ -47,7 +83,8 @@ const TerminalViewport = React.forwardRef<TerminalController, Props>(({
|
||||
const fitRef = React.useRef<FitAddon | null>(null);
|
||||
const inputRef = React.useRef(onInput);
|
||||
const resizeRef = React.useRef(onResize);
|
||||
const lastSizeRef = React.useRef<{ cols: number; rows: number } | null>(null);
|
||||
const lastSizeRef = React.useRef<TerminalSize | null>(null);
|
||||
const provisionalSizeRef = React.useRef<TerminalSize | null>(null);
|
||||
const lastChunkRef = React.useRef<number | null>(null);
|
||||
const writeQueueRef = React.useRef('');
|
||||
const outputRewriteCarryRef = React.useRef('');
|
||||
@@ -65,6 +102,14 @@ const TerminalViewport = React.forwardRef<TerminalController, Props>(({
|
||||
visibleRef.current = isVisible;
|
||||
safeResetRef.current = getGhosttySafeResetSequence(theme.background);
|
||||
|
||||
React.useLayoutEffect(() => {
|
||||
const container = containerRef.current;
|
||||
if (!container) return;
|
||||
const size = getProvisionalTerminalSize(container, fontFamily, fontSize);
|
||||
provisionalSizeRef.current = size;
|
||||
if (size) resizeRef.current(size.cols, size.rows);
|
||||
}, [fontFamily, fontSize]);
|
||||
|
||||
const fit = React.useCallback(() => {
|
||||
const container = containerRef.current;
|
||||
const terminal = terminalRef.current;
|
||||
@@ -168,7 +213,10 @@ const TerminalViewport = React.forwardRef<TerminalController, Props>(({
|
||||
|
||||
loadGhostty().then((ghostty) => {
|
||||
if (disposed) return;
|
||||
terminal = new GhosttyTerminal(getGhosttyTerminalOptions(fontFamily, fontSize, theme, ghostty, false));
|
||||
terminal = new GhosttyTerminal({
|
||||
...getGhosttyTerminalOptions(fontFamily, fontSize, theme, ghostty, false),
|
||||
...(provisionalSizeRef.current ?? {}),
|
||||
});
|
||||
const fitAddon = new FitAddon();
|
||||
terminal.loadAddon(fitAddon);
|
||||
terminal.open(container);
|
||||
|
||||
@@ -26,6 +26,8 @@ type TerminalViewProps = {
|
||||
visible?: boolean;
|
||||
};
|
||||
|
||||
const FALLBACK_TERMINAL_SIZE = { cols: 80, rows: 24 } as const;
|
||||
|
||||
export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
const { t } = useI18n();
|
||||
const { terminal, runtime } = useRuntimeAPIs();
|
||||
@@ -109,7 +111,6 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
const [isReconnectPending, setIsReconnectPending] = React.useState(false);
|
||||
const [activeModifier, setActiveModifier] = React.useState<Modifier | null>(null);
|
||||
const [isRestarting, setIsRestarting] = React.useState(false);
|
||||
const [hasViewportSize, setHasViewportSize] = React.useState(false);
|
||||
|
||||
const streamCleanupRef = React.useRef<(() => void) | null>(null);
|
||||
const activeTerminalIdRef = React.useRef<string | null>(null);
|
||||
@@ -118,7 +119,7 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
const directoryRef = React.useRef<string | null>(effectiveDirectory);
|
||||
const terminalControllerRef = React.useRef<TerminalController | null>(null);
|
||||
const lastViewportSizeRef = React.useRef<{ cols: number; rows: number } | null>(null);
|
||||
const isTerminalVisibleRef = React.useRef(false);
|
||||
const pendingTerminalCreatesRef = React.useRef(new Set<string>());
|
||||
const previewScanTailRef = React.useRef('');
|
||||
const pendingPreviewProbeUrlsRef = React.useRef<Set<string>>(new Set());
|
||||
const previewProbeGenerationRef = React.useRef(0);
|
||||
@@ -157,10 +158,6 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
}
|
||||
}, [isTerminalVisible]);
|
||||
|
||||
React.useEffect(() => {
|
||||
isTerminalVisibleRef.current = isTerminalVisible;
|
||||
}, [isTerminalVisible]);
|
||||
|
||||
React.useEffect(() => {
|
||||
terminalIdRef.current = terminalSessionId;
|
||||
}, [terminalSessionId]);
|
||||
@@ -424,7 +421,7 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
}
|
||||
|
||||
const tab = state.tabs.find((t) => t.id === tabId) ?? state.tabs[0];
|
||||
let terminalId = tab?.terminalSessionId ?? null;
|
||||
const terminalId = tab?.terminalSessionId ?? null;
|
||||
const terminalLifecycle = tab?.lifecycle ?? 'idle';
|
||||
const isActionTab = Boolean(tab?.label?.startsWith('Action:'));
|
||||
const buffer = useTerminalStore.getState().getBuffer(directory, tabId);
|
||||
@@ -441,11 +438,17 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
return;
|
||||
}
|
||||
|
||||
const size = lastViewportSizeRef.current;
|
||||
if (!size && isTerminalVisibleRef.current) {
|
||||
const createKey = `${directory}\u0000${tabId}`;
|
||||
if (pendingTerminalCreatesRef.current.has(createKey)) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Launch the shell while Ghostty is still loading and fitting.
|
||||
// The backend accepts 80x24, then receives the measured size as
|
||||
// soon as the viewport is ready.
|
||||
const initialSize = lastViewportSizeRef.current ?? FALLBACK_TERMINAL_SIZE;
|
||||
pendingTerminalCreatesRef.current.add(createKey);
|
||||
|
||||
setConnectionError(null);
|
||||
setIsFatalError(false);
|
||||
setIsReconnectPending(false);
|
||||
@@ -454,8 +457,8 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
const session = await terminal.createSession({
|
||||
cwd: directory,
|
||||
sessionId: tabId,
|
||||
cols: size?.cols,
|
||||
rows: size?.rows,
|
||||
cols: initialSize.cols,
|
||||
rows: initialSize.rows,
|
||||
shell: terminalShell,
|
||||
loginShell: terminalLoginShell,
|
||||
...terminalAppearanceRef.current,
|
||||
@@ -476,19 +479,38 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
|
||||
setTabSessionId(directory, tabId, session.sessionId);
|
||||
if (!stillActive) return;
|
||||
terminalId = session.sessionId;
|
||||
} catch (error) {
|
||||
if (!cancelled) {
|
||||
setConnectionError(
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: t('terminalView.error.startSessionFailed')
|
||||
);
|
||||
setIsFatalError(true);
|
||||
setIsReconnectPending(false);
|
||||
setConnecting(directory, tabId, false);
|
||||
|
||||
const viewportSize = lastViewportSizeRef.current;
|
||||
if (
|
||||
viewportSize &&
|
||||
(viewportSize.cols !== initialSize.cols || viewportSize.rows !== initialSize.rows)
|
||||
) {
|
||||
void terminal.resize({ sessionId: session.sessionId, ...viewportSize }).catch(() => {});
|
||||
}
|
||||
// Storing the session ID reruns this effect. Let that next
|
||||
// effect own stream startup: starting here would be torn
|
||||
// down immediately by this effect's cleanup.
|
||||
return;
|
||||
} catch (error) {
|
||||
const owningTab = useTerminalStore.getState().getDirectoryState(directory)?.tabs.find((entry) => entry.id === tabId);
|
||||
if (!owningTab || owningTab.terminalSessionId) return;
|
||||
|
||||
setConnecting(directory, tabId, false);
|
||||
// Strict Mode replaces the first effect while its create
|
||||
// request is pending. `cancelled` therefore does not mean
|
||||
// this tab stopped owning the request; use current store
|
||||
// ownership so a rejected create cannot leave it spinning.
|
||||
if (directoryRef.current !== directory || activeTabIdRef.current !== tabId) return;
|
||||
setConnectionError(
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: t('terminalView.error.startSessionFailed')
|
||||
);
|
||||
setIsFatalError(true);
|
||||
setIsReconnectPending(false);
|
||||
return;
|
||||
} finally {
|
||||
pendingTerminalCreatesRef.current.delete(createKey);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -513,7 +535,6 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
terminalLifecycle,
|
||||
activeTabId,
|
||||
hasOpenedTerminalViewport,
|
||||
hasViewportSize,
|
||||
enableTabs,
|
||||
terminalHydrated,
|
||||
ensureDirectory,
|
||||
@@ -568,7 +589,7 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
resetTerminalPreviewScan();
|
||||
|
||||
try {
|
||||
const size = lastViewportSizeRef.current ?? { cols: 80, rows: 24 };
|
||||
const size = lastViewportSizeRef.current ?? FALLBACK_TERMINAL_SIZE;
|
||||
const restarted = await terminal.restartSession(originalSessionId, { cwd: effectiveDirectory, shell: terminalShell, loginShell: terminalLoginShell, ...size, ...terminalAppearanceRef.current });
|
||||
const owningTab = useTerminalStore.getState().getDirectoryState(effectiveDirectory)?.tabs.find((tab) => tab.id === tabId);
|
||||
if (owningTab?.terminalSessionId !== originalSessionId) return;
|
||||
@@ -694,22 +715,17 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
const handleViewportResize = React.useCallback(
|
||||
(cols: number, rows: number) => {
|
||||
const previous = lastViewportSizeRef.current;
|
||||
if (!previous) {
|
||||
lastViewportSizeRef.current = { cols, rows };
|
||||
if (!terminalIdRef.current) setHasViewportSize(true);
|
||||
} else if (previous.cols !== cols || previous.rows !== rows) {
|
||||
if (!previous || previous.cols !== cols || previous.rows !== rows) {
|
||||
lastViewportSizeRef.current = { cols, rows };
|
||||
}
|
||||
if (!isTerminalVisibleRef.current) {
|
||||
if (!isTerminalVisible) {
|
||||
return;
|
||||
}
|
||||
const terminalId = terminalIdRef.current;
|
||||
if (!terminalId) return;
|
||||
void terminal.resize({ sessionId: terminalId, cols, rows }).catch(() => {
|
||||
|
||||
});
|
||||
void terminal.resize({ sessionId: terminalId, cols, rows }).catch(() => {});
|
||||
},
|
||||
[terminal]
|
||||
[isTerminalVisible, terminal]
|
||||
);
|
||||
|
||||
const handleModifierToggle = React.useCallback(
|
||||
@@ -801,11 +817,7 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
// here tore down and rebuilt the Ghostty terminal (WASM VT + canvas + font
|
||||
// atlas) a second time the moment `createSession` resolved, doubling the cost
|
||||
// of every terminal open. Session changes are handled by the chunk replay path.
|
||||
const terminalViewportKey = React.useMemo(() => {
|
||||
const directoryPart = effectiveDirectory ?? 'no-dir';
|
||||
const tabPart = activeTabId ?? 'no-tab';
|
||||
return `${directoryPart}::${tabPart}`;
|
||||
}, [effectiveDirectory, activeTabId]);
|
||||
const terminalViewportKey = `${effectiveDirectory ?? 'no-dir'}::${activeTabId ?? 'no-tab'}`;
|
||||
|
||||
React.useEffect(() => {
|
||||
if (!isTerminalVisible || useTouchTerminalInput) {
|
||||
|
||||
@@ -2,8 +2,8 @@
|
||||
* Regression guard for slow terminal opening on Linux.
|
||||
*
|
||||
* `TerminalViewport` is keyed by `terminalViewportKey`. That key used to include
|
||||
* the PTY session id, which is null until `createSession` resolves. Because the
|
||||
* viewport must mount first to report its size before a session can be created,
|
||||
* the PTY session id, which is null until `createSession` resolves. Historically,
|
||||
* the viewport had to mount first to report its size before session creation, so
|
||||
* every terminal open built a Ghostty terminal (WASM VT + 2D canvas renderer +
|
||||
* font atlas), threw it away when the session id arrived, and built a second one.
|
||||
* The same churn repeated on reconnect and on every incidental session-id change,
|
||||
@@ -12,6 +12,8 @@
|
||||
*
|
||||
* Viewport identity must therefore be directory + tab only. Session changes are
|
||||
* handled by the chunk replay path, which resets the existing terminal in place.
|
||||
* New sessions start concurrently with a container-derived size (or 80x24) and
|
||||
* resize after their viewport fits.
|
||||
*/
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { readFileSync } from 'node:fs';
|
||||
@@ -25,27 +27,15 @@ const terminalViewportSource = readFileSync(
|
||||
'utf-8',
|
||||
);
|
||||
|
||||
const viewportKeyBlock = (() => {
|
||||
const start = terminalViewSource.indexOf('const terminalViewportKey = React.useMemo(');
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
const end = terminalViewSource.indexOf('}, [', start);
|
||||
expect(end).toBeGreaterThan(start);
|
||||
return terminalViewSource.slice(start, terminalViewSource.indexOf(');', end));
|
||||
})();
|
||||
const viewportKeyDeclaration = terminalViewSource
|
||||
.split('\n')
|
||||
.find((line) => line.includes('const terminalViewportKey =')) ?? '';
|
||||
|
||||
describe('terminal viewport remount guard', () => {
|
||||
test('viewport identity excludes the PTY session id', () => {
|
||||
expect(viewportKeyBlock).toContain('effectiveDirectory');
|
||||
expect(viewportKeyBlock).toContain('activeTabId');
|
||||
expect(viewportKeyBlock).not.toContain('terminalSessionId');
|
||||
});
|
||||
|
||||
test('viewport key memo does not depend on the PTY session id', () => {
|
||||
const dependencyStart = terminalViewSource.indexOf('}, [', terminalViewSource.indexOf('const terminalViewportKey'));
|
||||
const dependencies = terminalViewSource.slice(dependencyStart, terminalViewSource.indexOf(']', dependencyStart));
|
||||
expect(dependencies).toContain('effectiveDirectory');
|
||||
expect(dependencies).toContain('activeTabId');
|
||||
expect(dependencies).not.toContain('terminalSessionId');
|
||||
expect(viewportKeyDeclaration).toContain('effectiveDirectory');
|
||||
expect(viewportKeyDeclaration).toContain('activeTabId');
|
||||
expect(viewportKeyDeclaration).not.toContain('terminalSessionId');
|
||||
});
|
||||
|
||||
test('replay discontinuities reset the terminal in place instead of remounting it', () => {
|
||||
@@ -61,4 +51,53 @@ describe('terminal viewport remount guard', () => {
|
||||
expect(terminalViewSource).toContain('getBuffer(');
|
||||
expect(terminalViewSource).not.toContain('activeTab?.bufferChunks');
|
||||
});
|
||||
|
||||
test('starts the PTY before Ghostty reports its first viewport size', () => {
|
||||
expect(terminalViewSource).toContain('const FALLBACK_TERMINAL_SIZE = { cols: 80, rows: 24 } as const;');
|
||||
expect(terminalViewSource).toContain('const initialSize = lastViewportSizeRef.current ?? FALLBACK_TERMINAL_SIZE;');
|
||||
expect(terminalViewSource).not.toContain('if (!size && isTerminalVisibleRef.current)');
|
||||
expect(terminalViewSource).toContain('cols: initialSize.cols');
|
||||
expect(terminalViewSource).toContain('rows: initialSize.rows');
|
||||
expect(terminalViewSource).toContain('void terminal.resize({ sessionId: session.sessionId, ...viewportSize })');
|
||||
expect(terminalViewSource).toContain('if (!isTerminalVisible) {');
|
||||
expect(terminalViewSource).not.toContain('isTerminalVisibleRef');
|
||||
});
|
||||
|
||||
test('deduplicates create attempts while the viewport layout settles', () => {
|
||||
expect(terminalViewSource).toContain('pendingTerminalCreatesRef.current.has(createKey)');
|
||||
expect(terminalViewSource).toContain('pendingTerminalCreatesRef.current.delete(createKey)');
|
||||
});
|
||||
|
||||
test('lets the session-ID effect own stream startup after creating a tab', () => {
|
||||
const createStart = terminalViewSource.indexOf('if (!terminalId) {');
|
||||
const createEnd = terminalViewSource.indexOf('if (!terminalId || cancelled) return;', createStart);
|
||||
expect(createStart).toBeGreaterThan(-1);
|
||||
expect(createEnd).toBeGreaterThan(createStart);
|
||||
const createBlock = terminalViewSource.slice(createStart, createEnd);
|
||||
|
||||
expect(createBlock).toContain('setTabSessionId(directory, tabId, session.sessionId);');
|
||||
expect(createBlock).toContain('Let that next');
|
||||
expect(createBlock).not.toContain('startStream(');
|
||||
});
|
||||
|
||||
test('clears a current tab from connecting when a strict-mode create rejects', () => {
|
||||
const createStart = terminalViewSource.indexOf('if (!terminalId) {');
|
||||
const catchStart = terminalViewSource.indexOf('} catch (error) {', createStart);
|
||||
const catchEnd = terminalViewSource.indexOf('} finally {', catchStart);
|
||||
expect(catchStart).toBeGreaterThan(createStart);
|
||||
expect(catchEnd).toBeGreaterThan(catchStart);
|
||||
const catchBlock = terminalViewSource.slice(catchStart, catchEnd);
|
||||
|
||||
expect(catchBlock).toContain('owningTab.terminalSessionId');
|
||||
expect(catchBlock).toContain('activeTabIdRef.current !== tabId');
|
||||
expect(catchBlock).toContain('setConnecting(directory, tabId, false);');
|
||||
expect(catchBlock).not.toContain('if (!cancelled)');
|
||||
});
|
||||
|
||||
test('derives the initial PTY size before Ghostty mounts', () => {
|
||||
expect(terminalViewportSource).toContain('const getProvisionalTerminalSize');
|
||||
expect(terminalViewportSource).toContain('React.useLayoutEffect(() => {');
|
||||
expect(terminalViewportSource).toContain('resizeRef.current(size.cols, size.rows)');
|
||||
expect(terminalViewportSource).toContain('...(provisionalSizeRef.current ?? {})');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user