fix(terminal): start PTY before viewport mounts without dropping output or startup replies
Terminal creation no longer waits for the Ghostty viewport to report its size: it starts the PTY immediately with a container/font-derived provisional size (falling back to 80x24), then resizes once the real viewport dimensions are known, with a dedupe guard while sizing settles. Starting the shell earlier means it can emit device/theme queries before a browser terminal is attached to answer them, so the server now answers primary device attribute queries itself (Fish blocks ~10s on this at startup) and bun-pty buffers output emitted before a data subscriber attaches. Also fixes a few WebSocket transport reconnect races surfaced by session creation now overlapping renderer setup.
This commit is contained in:
@@ -421,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);
|
||||
@@ -487,18 +487,27 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
) {
|
||||
void terminal.resize({ sessionId: session.sessionId, ...viewportSize }).catch(() => {});
|
||||
}
|
||||
terminalId = session.sessionId;
|
||||
// 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) {
|
||||
if (!cancelled) {
|
||||
setConnectionError(
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: t('terminalView.error.startSessionFailed')
|
||||
);
|
||||
setIsFatalError(true);
|
||||
setIsReconnectPending(false);
|
||||
setConnecting(directory, tabId, false);
|
||||
}
|
||||
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);
|
||||
@@ -706,9 +715,7 @@ 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 };
|
||||
} else if (previous.cols !== cols || previous.rows !== rows) {
|
||||
if (!previous || previous.cols !== cols || previous.rows !== rows) {
|
||||
lastViewportSizeRef.current = { cols, rows };
|
||||
}
|
||||
if (!isTerminalVisible) {
|
||||
@@ -716,9 +723,7 @@ export const TerminalView: React.FC<TerminalViewProps> = ({ visible }) => {
|
||||
}
|
||||
const terminalId = terminalIdRef.current;
|
||||
if (!terminalId) return;
|
||||
void terminal.resize({ sessionId: terminalId, cols, rows }).catch(() => {
|
||||
|
||||
});
|
||||
void terminal.resize({ sessionId: terminalId, cols, rows }).catch(() => {});
|
||||
},
|
||||
[isTerminalVisible, terminal]
|
||||
);
|
||||
@@ -812,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) {
|
||||
|
||||
@@ -27,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', () => {
|
||||
@@ -80,6 +68,32 @@ describe('terminal viewport remount guard', () => {
|
||||
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(() => {');
|
||||
|
||||
Reference in New Issue
Block a user