diff --git a/.opencode/screenshots/terminal-final.png b/.opencode/screenshots/terminal-final.png new file mode 100644 index 00000000..ab8d0cbb Binary files /dev/null and b/.opencode/screenshots/terminal-final.png differ diff --git a/.opencode/screenshots/terminal-parallel-start.png b/.opencode/screenshots/terminal-parallel-start.png new file mode 100644 index 00000000..84c137ba Binary files /dev/null and b/.opencode/screenshots/terminal-parallel-start.png differ diff --git a/.opencode/screenshots/terminal-reset.png b/.opencode/screenshots/terminal-reset.png new file mode 100644 index 00000000..447a9da0 Binary files /dev/null and b/.opencode/screenshots/terminal-reset.png differ diff --git a/.opencode/screenshots/terminal-startup.png b/.opencode/screenshots/terminal-startup.png new file mode 100644 index 00000000..39296cd3 Binary files /dev/null and b/.opencode/screenshots/terminal-startup.png differ diff --git a/bun-patches/bun-pty@0.4.8.patch b/bun-patches/bun-pty@0.4.8.patch new file mode 100644 index 00000000..f65b3bba --- /dev/null +++ b/bun-patches/bun-pty@0.4.8.patch @@ -0,0 +1,77 @@ +diff --git a/src/terminal.ts b/src/terminal.ts +index ec248d46a939f8a09cd669e853cefb126922c80a..c0473bc625edda7be2ade987e8aa3bd99160ce67 100644 +--- a/src/terminal.ts ++++ b/src/terminal.ts +@@ -11,6 +11,7 @@ export const DEFAULT_COLS = 80; + export const DEFAULT_ROWS = 24; + export const DEFAULT_FILE = "sh"; + export const DEFAULT_NAME = "xterm"; ++const INITIAL_OUTPUT_BUFFER_LIMIT = 512 * 1024; + + /** + * Quote a string for shell-words compatible splitting on the Rust side. +@@ -136,6 +137,8 @@ export class Terminal implements IPty { + + private _readLoop = false; + private _closing = false; ++ private _hasDataSubscriber = false; ++ private _initialOutput = ""; + + // TextDecoder with streaming mode to properly handle UTF-8 across chunk boundaries + // Without this, multi-byte characters (like box-drawing ─) that span chunks become � +@@ -191,12 +194,29 @@ export class Terminal implements IPty { + } + + get onData() { +- return this._onData.event; ++ return (listener: (data: string) => void) => { ++ const disposable = this._onData.event(listener); ++ if (!this._hasDataSubscriber) { ++ this._hasDataSubscriber = true; ++ const initialOutput = this._initialOutput; ++ this._initialOutput = ""; ++ if (initialOutput) listener(initialOutput); ++ } ++ return disposable; ++ }; + } + get onExit() { + return this._onExit.event; + } + ++ private _emitData(data: string) { ++ if (this._hasDataSubscriber) { ++ this._onData.fire(data); ++ } else { ++ this._initialOutput = `${this._initialOutput}${data}`.slice(-INITIAL_OUTPUT_BUFFER_LIMIT); ++ } ++ } ++ + /* ------------- IO methods ------------- */ + + write(data: string) { +@@ -235,13 +255,13 @@ export class Terminal implements IPty { + // This prevents corruption when multi-byte chars span chunk boundaries + const decoded = this._decoder.decode(buf.subarray(0, n), { stream: true }); + if (decoded) { +- this._onData.fire(decoded); ++ this._emitData(decoded); + } + } else if (n === -2) { + // CHILD_EXITED - flush any remaining bytes in the decoder + const remaining = this._decoder.decode(); + if (remaining) { +- this._onData.fire(remaining); ++ this._emitData(remaining); + } + const exitCode = lib.symbols.bun_pty_get_exit_code(this.handle); + this._onExit.fire({ exitCode }); +@@ -250,7 +270,7 @@ export class Terminal implements IPty { + // error - flush decoder before breaking + const remaining = this._decoder.decode(); + if (remaining) { +- this._onData.fire(remaining); ++ this._emitData(remaining); + } + break; + } else { diff --git a/bun.lock b/bun.lock index 15b89095..2209fc6d 100644 --- a/bun.lock +++ b/bun.lock @@ -95,7 +95,7 @@ }, "packages/electron": { "name": "@openchamber/electron", - "version": "1.17.1", + "version": "1.17.2", "dependencies": { "@openchamber/web": "workspace:*", "better-sqlite3": "^12.10.0", @@ -132,7 +132,7 @@ }, "packages/ui": { "name": "@openchamber/ui", - "version": "1.17.1", + "version": "1.17.2", "dependencies": { "@aparajita/capacitor-secure-storage": "^8.0.0", "@base-ui/react": "^1.4.0", @@ -237,7 +237,7 @@ }, "packages/vscode": { "name": "openchamber", - "version": "1.17.1", + "version": "1.17.2", "dependencies": { "@openchamber/ui": "workspace:*", "@opencode-ai/sdk": "1.18.11", @@ -260,7 +260,7 @@ }, "packages/web": { "name": "@openchamber/web", - "version": "1.17.1", + "version": "1.17.2", "bin": { "openchamber": "./bin/cli.js", }, @@ -352,6 +352,7 @@ ], "patchedDependencies": { "@tanstack/virtual-core@3.17.3": "bun-patches/@tanstack+virtual-core+3.17.3.patch", + "bun-pty@0.4.8": "bun-patches/bun-pty@0.4.8.patch", }, "overrides": { "@codemirror/language": "6.12.2", diff --git a/package.json b/package.json index 3561555d..030fc1d0 100644 --- a/package.json +++ b/package.json @@ -176,6 +176,7 @@ "vite": "^7.1.2" }, "patchedDependencies": { - "@tanstack/virtual-core@3.17.3": "bun-patches/@tanstack+virtual-core+3.17.3.patch" + "@tanstack/virtual-core@3.17.3": "bun-patches/@tanstack+virtual-core+3.17.3.patch", + "bun-pty@0.4.8": "bun-patches/bun-pty@0.4.8.patch" } } diff --git a/packages/ui/src/components/chat/QueuedMessageChips.tsx b/packages/ui/src/components/chat/QueuedMessageChips.tsx index d49b1ae5..b64a3f4f 100644 --- a/packages/ui/src/components/chat/QueuedMessageChips.tsx +++ b/packages/ui/src/components/chat/QueuedMessageChips.tsx @@ -111,7 +111,15 @@ const EMPTY_QUEUE: QueuedMessage[] = []; export const QueuedMessageChips = memo(({ onEditMessage, onSendMessage }: QueuedMessageChipsProps) => { const { t } = useI18n(); const currentSessionId = useSessionUIStore((state) => state.currentSessionId); - const currentSessionDirectory = useSessionUIStore((state) => state.currentSessionDirectory); + // Must use the same resolution the composer used to build the queue key — + // reading currentSessionDirectory raw can key the chips to a different + // directory than the one the messages were queued under. + const currentSessionDirectory = useSessionUIStore( + React.useCallback( + (state) => (currentSessionId ? state.getDirectoryForSession(currentSessionId) : null), + [currentSessionId], + ), + ); const target = currentSessionId ? createMessageQueueTarget(currentSessionId, currentSessionDirectory) : null; const queueKey = target ? getMessageQueueKey(target) : null; const queuedMessages = useMessageQueueStore( diff --git a/packages/ui/src/components/chat/message/parts/ToolPart.test.ts b/packages/ui/src/components/chat/message/parts/ToolPart.test.ts index d31d076e..2042f014 100644 --- a/packages/ui/src/components/chat/message/parts/ToolPart.test.ts +++ b/packages/ui/src/components/chat/message/parts/ToolPart.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from 'bun:test'; +import { renderTerminalOutput } from './toolOutput'; import { readTaskTagSessionIdFromOutput } from './taskSessionIdParser'; import { tryParseJsonOutput } from '../toolRenderers'; import { getStreamingThrottleText } from '../../hooks/useStreamingTextThrottle'; @@ -7,18 +8,73 @@ import { getStreamingOutputAppend, getToolOutput } from './toolOutput'; import { getToolDescriptionFallback } from './toolRenderUtils'; describe('getToolOutput', () => { - test('prefers authoritative state output', () => { - expect(getToolOutput('bash', 'final output', 'streamed output')).toBe('final output'); - expect(getToolOutput('bash', '', 'streamed output')).toBe(''); + test('prefers state.output for completed tools', () => { + expect(getToolOutput('bash', 'final output', 'partial output', 'completed')).toBe('final output'); }); - test('falls back to streamed metadata output for bash', () => { - expect(getToolOutput('bash', undefined, 'streamed output')).toBe('streamed output'); - expect(getToolOutput('bash', undefined, '')).toBe(undefined); + test('normalizes completed bash state output while preserving final-output precedence', () => { + expect(getToolOutput('bash', '\u001B[32mFinal output\u001B[0m', 'partial output', 'completed')).toBe('Final output'); }); - test('does not expose metadata output for other tools', () => { - expect(getToolOutput('read', undefined, 'metadata output')).toBe(undefined); + test('falls back to metadata.output for bash tools without state output', () => { + expect(getToolOutput('bash', undefined, 'partial output', 'completed')).toBe('partial output'); + }); + + test('normalizes bash metadata output for completed state', () => { + expect(getToolOutput('bash', undefined, 'Progress 10%\r\u001B[2KProgress 90%', 'completed')).toBe('Progress 90%'); + }); + + test('does not normalize bash output while running', () => { + expect(getToolOutput('bash', '\u001B[32mRunning\u001B[0m', undefined, 'running')).toBe('\u001B[32mRunning\u001B[0m'); + expect(getToolOutput('bash', undefined, 'Progress\r\u001B[2K', 'running')).toBe('Progress\r\u001B[2K'); + }); + + test('ignores metadata.output for non-bash tools', () => { + expect(getToolOutput('read', undefined, 'partial output', 'completed')).toBe(undefined); + expect(getToolOutput('read', 'final output', 'partial output', 'completed')).toBe('final output'); + }); + + test('returns undefined when bash has no output', () => { + expect(getToolOutput('bash', undefined, undefined, 'completed')).toBe(undefined); + }); + + test('ignores empty metadata.output for bash', () => { + expect(getToolOutput('bash', undefined, '', 'completed')).toBe(undefined); + }); +}); + +describe('renderTerminalOutput', () => { + test('renders carriage-return progress updates as their latest value', () => { + expect(renderTerminalOutput('Downloading 10%\r\u001B[2KDownloading 90%')).toBe('Downloading 90%'); + }); + + test('removes ANSI styles while preserving the output text', () => { + expect(renderTerminalOutput('\u001B[32mComplete\u001B[0m\n')).toBe('Complete\n'); + }); + + test('applies cursor-up progress updates to the prior line', () => { + expect(renderTerminalOutput('First\nWorking\u001B[1A\r\u001B[2KDone\n')).toBe('Done\nWorking'); + }); + + test('CSI K erases from cursor to end of line', () => { + expect(renderTerminalOutput('Hello World\u001B[5G\u001B[K')).toBe('Hell'); + }); + + test('CSI 1 K erases from beginning of line through cursor, preserving suffix', () => { + expect(renderTerminalOutput('Hello World\u001B[6G\u001B[1K')).toBe(' World'); + }); + + test('CSI 2 K erases entire line', () => { + expect(renderTerminalOutput('Hello World\u001B[2K')).toBe(''); + }); + + test('handles large single-line output without quadratic slowdown', () => { + const largeLine = 'A'.repeat(50000) + '\u001B[0m'; + const start = performance.now(); + const result = renderTerminalOutput(largeLine); + const elapsed = performance.now() - start; + expect(result).toBe('A'.repeat(50000)); + expect(elapsed).toBeLessThan(1000); }); }); diff --git a/packages/ui/src/components/chat/message/parts/ToolPart.tsx b/packages/ui/src/components/chat/message/parts/ToolPart.tsx index eb3e0cf8..ee4983de 100644 --- a/packages/ui/src/components/chat/message/parts/ToolPart.tsx +++ b/packages/ui/src/components/chat/message/parts/ToolPart.tsx @@ -1362,7 +1362,7 @@ const ToolExpandedContent: React.FC = React.memo(({ const stateWithData = state as ToolStateWithMetadata; const metadata = stateWithData.metadata; const input = stateWithData.input; - const rawOutput = getToolOutput(part.tool, stateWithData.output, metadata?.output); + const rawOutput = getToolOutput(part.tool, stateWithData.output, metadata?.output, state.status); const hasStringOutput = typeof rawOutput === 'string' && rawOutput.length > 0; const rawOutputString = typeof rawOutput === 'string' ? rawOutput : ''; const isStreamingBash = part.tool === 'bash' && state.status === 'running'; diff --git a/packages/ui/src/components/chat/message/parts/toolOutput.ts b/packages/ui/src/components/chat/message/parts/toolOutput.ts index 88d1d0ab..5eb8dc91 100644 --- a/packages/ui/src/components/chat/message/parts/toolOutput.ts +++ b/packages/ui/src/components/chat/message/parts/toolOutput.ts @@ -1,14 +1,133 @@ +const ensureLine = (lines: string[][], row: number): void => { + while (lines.length <= row) { + lines.push([]); + } +}; + +const writeTerminalCharacter = (lines: string[][], row: number, column: number, character: string): void => { + ensureLine(lines, row); + const line = lines[row]; + while (line.length < column) { + line.push(' '); + } + line[column] = character; +}; + +export const renderTerminalOutput = (output: string): string => { + if (!output.includes('\u001B') && !output.includes('\r') && !output.includes('\b')) { + return output; + } + + const lines: string[][] = [[]]; + let row = 0; + let column = 0; + + for (let index = 0; index < output.length; index += 1) { + const character = output[index]; + + if (character === '\n') { + row += 1; + column = 0; + ensureLine(lines, row); + continue; + } + if (character === '\r') { + column = 0; + continue; + } + if (character === '\b') { + column = Math.max(0, column - 1); + continue; + } + if (character !== '\u001B') { + writeTerminalCharacter(lines, row, column, character); + column += 1; + continue; + } + + const nextCharacter = output[index + 1]; + if (nextCharacter === '[') { + const sequenceStart = index + 2; + let sequenceEnd = sequenceStart; + while (sequenceEnd < output.length && !/[\x40-\x7E]/.test(output[sequenceEnd])) { + sequenceEnd += 1; + } + if (sequenceEnd === output.length) { + break; + } + + const command = output[sequenceEnd]; + const parameters = output.slice(sequenceStart, sequenceEnd).split(';').map((value) => Number.parseInt(value, 10) || 0); + const count = parameters[0] || 1; + if (command === 'A') { + row = Math.max(0, row - count); + } else if (command === 'B') { + row += count; + ensureLine(lines, row); + } else if (command === 'C') { + column += count; + } else if (command === 'D') { + column = Math.max(0, column - count); + } else if (command === 'G') { + column = Math.max(0, count - 1); + } else if (command === 'H' || command === 'f') { + row = Math.max(0, (parameters[0] || 1) - 1); + column = Math.max(0, (parameters[1] || 1) - 1); + ensureLine(lines, row); + } else if (command === 'K') { + ensureLine(lines, row); + const line = lines[row]; + const mode = parameters[0]; + if (mode === 1) { + for (let i = 0; i <= column && i < line.length; i += 1) { + line[i] = ' '; + } + } else if (mode === 2) { + lines[row] = []; + } else { + line.length = Math.min(line.length, column); + } + } + index = sequenceEnd; + continue; + } + + if (nextCharacter === ']') { + const terminator = output.indexOf('\u0007', index + 2); + const stringTerminator = output.indexOf('\u001B\\', index + 2); + const end = terminator === -1 + ? stringTerminator + : stringTerminator === -1 + ? terminator + : Math.min(terminator, stringTerminator); + if (end === -1) { + break; + } + index = output[end] === '\u0007' ? end : end + 1; + continue; + } + + index += 1; + } + + return lines.map((line) => line.join('')).join('\n'); +}; + export const getToolOutput = ( tool: string, stateOutput: unknown, metadataOutput: unknown, + status?: string, ): string | undefined => { + const isBash = tool === 'bash'; + const shouldNormalize = isBash && status !== 'running'; + if (typeof stateOutput === 'string') { - return stateOutput; + return shouldNormalize ? renderTerminalOutput(stateOutput) : stateOutput; } - if (tool === 'bash' && typeof metadataOutput === 'string' && metadataOutput.length > 0) { - return metadataOutput; + if (isBash && typeof metadataOutput === 'string' && metadataOutput.length > 0) { + return shouldNormalize ? renderTerminalOutput(metadataOutput) : metadataOutput; } return undefined; diff --git a/packages/ui/src/components/terminal/TerminalViewport.tsx b/packages/ui/src/components/terminal/TerminalViewport.tsx index 90b834d3..cd86497f 100644 --- a/packages/ui/src/components/terminal/TerminalViewport.tsx +++ b/packages/ui/src/components/terminal/TerminalViewport.tsx @@ -18,6 +18,42 @@ import type { TerminalChunk } from '@/stores/useTerminalStore'; let ghosttyPromise: Promise | null = null; const loadGhostty = (): Promise => 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(({ const fitRef = React.useRef(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(null); + const provisionalSizeRef = React.useRef(null); const lastChunkRef = React.useRef(null); const writeQueueRef = React.useRef(''); const outputRewriteCarryRef = React.useRef(''); @@ -65,6 +102,14 @@ const TerminalViewport = React.forwardRef(({ 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(({ 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); diff --git a/packages/ui/src/components/views/TerminalView.tsx b/packages/ui/src/components/views/TerminalView.tsx index a373f880..c97f608e 100644 --- a/packages/ui/src/components/views/TerminalView.tsx +++ b/packages/ui/src/components/views/TerminalView.tsx @@ -26,6 +26,8 @@ type TerminalViewProps = { visible?: boolean; }; +const FALLBACK_TERMINAL_SIZE = { cols: 80, rows: 24 } as const; + export const TerminalView: React.FC = ({ visible }) => { const { t } = useI18n(); const { terminal, runtime } = useRuntimeAPIs(); @@ -109,7 +111,6 @@ export const TerminalView: React.FC = ({ visible }) => { const [isReconnectPending, setIsReconnectPending] = React.useState(false); const [activeModifier, setActiveModifier] = React.useState(null); const [isRestarting, setIsRestarting] = React.useState(false); - const [hasViewportSize, setHasViewportSize] = React.useState(false); const streamCleanupRef = React.useRef<(() => void) | null>(null); const activeTerminalIdRef = React.useRef(null); @@ -118,7 +119,7 @@ export const TerminalView: React.FC = ({ visible }) => { const directoryRef = React.useRef(effectiveDirectory); const terminalControllerRef = React.useRef(null); const lastViewportSizeRef = React.useRef<{ cols: number; rows: number } | null>(null); - const isTerminalVisibleRef = React.useRef(false); + const pendingTerminalCreatesRef = React.useRef(new Set()); const previewScanTailRef = React.useRef(''); const pendingPreviewProbeUrlsRef = React.useRef>(new Set()); const previewProbeGenerationRef = React.useRef(0); @@ -157,10 +158,6 @@ export const TerminalView: React.FC = ({ visible }) => { } }, [isTerminalVisible]); - React.useEffect(() => { - isTerminalVisibleRef.current = isTerminalVisible; - }, [isTerminalVisible]); - React.useEffect(() => { terminalIdRef.current = terminalSessionId; }, [terminalSessionId]); @@ -424,7 +421,7 @@ export const TerminalView: React.FC = ({ 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 = ({ 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 = ({ 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 = ({ 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 = ({ visible }) => { terminalLifecycle, activeTabId, hasOpenedTerminalViewport, - hasViewportSize, enableTabs, terminalHydrated, ensureDirectory, @@ -568,7 +589,7 @@ export const TerminalView: React.FC = ({ 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 = ({ 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 = ({ 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) { diff --git a/packages/ui/src/components/views/__tests__/terminalViewportRemount.test.ts b/packages/ui/src/components/views/__tests__/terminalViewportRemount.test.ts index f33bd2cd..d03379c1 100644 --- a/packages/ui/src/components/views/__tests__/terminalViewportRemount.test.ts +++ b/packages/ui/src/components/views/__tests__/terminalViewportRemount.test.ts @@ -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 ?? {})'); + }); }); diff --git a/packages/ui/src/hooks/useQueuedMessageAutoSend.test.ts b/packages/ui/src/hooks/useQueuedMessageAutoSend.test.ts index 6675b2a5..70c5eb43 100644 --- a/packages/ui/src/hooks/useQueuedMessageAutoSend.test.ts +++ b/packages/ui/src/hooks/useQueuedMessageAutoSend.test.ts @@ -29,12 +29,61 @@ mock.module('@/sync/session-ui-store', () => ({ import { buildQueuedAutoSendPayload, + createQueuedAutoSendRetryScheduler, getQueuedAutoSendRetryDelayMs, isQueuedAutoSendBackedOff, sendQueuedAutoSendPayload, shouldDispatchQueuedAutoSend, } from './useQueuedMessageAutoSend'; +describe('queued auto-send retry scheduler', () => { + test('wakes the queue when backoff expires', () => { + const callbacks = new Map void>(); + let nextTimer = 0; + let wakeups = 0; + const scheduler = createQueuedAutoSendRetryScheduler( + () => { wakeups += 1; }, + () => 1_000, + (callback, delay) => { + callbacks.set(++nextTimer, callback); + expect(delay).toBe(500); + return nextTimer as unknown as ReturnType; + }, + (timer) => { callbacks.delete(timer as unknown as number); }, + ); + + scheduler.schedule(1_500); + expect(callbacks.size).toBe(1); + callbacks.values().next().value?.(); + expect(wakeups).toBe(1); + }); + + test('keeps the earliest retry and cancels it on dispose', () => { + const callbacks = new Map void>(); + let nextTimer = 0; + const delays: number[] = []; + const scheduler = createQueuedAutoSendRetryScheduler( + () => undefined, + () => 1_000, + (callback, delay) => { + callbacks.set(++nextTimer, callback); + delays.push(delay); + return nextTimer as unknown as ReturnType; + }, + (timer) => { callbacks.delete(timer as unknown as number); }, + ); + + scheduler.schedule(3_000); + scheduler.schedule(4_000); + scheduler.schedule(2_000); + + expect(delays).toEqual([2_000, 1_000]); + expect(callbacks.size).toBe(1); + scheduler.dispose(); + expect(callbacks.size).toBe(0); + }); +}); + describe('shouldDispatchQueuedAutoSend', () => { test('dispatches only after an active session becomes idle', () => { expect(shouldDispatchQueuedAutoSend('busy', 'idle', false)).toBe(true); diff --git a/packages/ui/src/hooks/useQueuedMessageAutoSend.ts b/packages/ui/src/hooks/useQueuedMessageAutoSend.ts index ecb53e6b..bc327ebe 100644 --- a/packages/ui/src/hooks/useQueuedMessageAutoSend.ts +++ b/packages/ui/src/hooks/useQueuedMessageAutoSend.ts @@ -33,12 +33,47 @@ export const isQueuedAutoSendBackedOff = ( now: number, ): boolean => failure !== undefined && failure.messageId === messageId && now < failure.nextAttemptAt; -const hasRecentAbort = (sessionId: string): boolean => { +export const createQueuedAutoSendRetryScheduler = ( + onWake: () => void, + now: () => number = Date.now, + scheduleTimeout: (callback: () => void, delay: number) => ReturnType = setTimeout, + cancelTimeout: (timer: ReturnType) => void = clearTimeout, +) => { + let timer: ReturnType | null = null; + let scheduledAt: number | null = null; + + return { + schedule(retryAt: number) { + if (scheduledAt !== null && scheduledAt <= retryAt) return; + if (timer !== null) cancelTimeout(timer); + scheduledAt = retryAt; + timer = scheduleTimeout(() => { + timer = null; + scheduledAt = null; + onWake(); + }, Math.max(0, retryAt - now())); + }, + dispose() { + if (timer !== null) cancelTimeout(timer); + timer = null; + scheduledAt = null; + }, + }; +}; + +/** + * When the abort window is still open, returns the time it expires so the + * caller can wake the queue then. Returns `null` once sending is allowed + * again — a queued item must not wait for an unrelated state change to be + * retried after the window closes. + */ +const getAbortHoldUntil = (sessionId: string): number | null => { const abortRecord = useSessionUIStore.getState().sessionAbortFlags.get(sessionId); if (!abortRecord) { - return false; + return null; } - return Date.now() - abortRecord.timestamp < RECENT_ABORT_WINDOW_MS; + const holdUntil = abortRecord.timestamp + RECENT_ABORT_WINDOW_MS; + return Date.now() < holdUntil ? holdUntil : null; }; export const buildQueuedAutoSendPayload = (queue: QueuedMessage[]) => { @@ -149,6 +184,13 @@ export function useQueuedMessageAutoSend(enabledOrOptions?: boolean | { enabled? const sendFailuresRef = React.useRef>(new Map()); const previousStatusRef = React.useRef>(new Map()); const autoReviewBlockedSessionsRef = React.useRef>(new Set()); + const [retryTick, setRetryTick] = React.useState(0); + const retryScheduler = React.useMemo( + () => createQueuedAutoSendRetryScheduler(() => setRetryTick((value) => value + 1)), + [], + ); + + React.useEffect(() => () => retryScheduler.dispose(), [retryScheduler]); React.useEffect(() => { if (!enabled) { @@ -164,7 +206,9 @@ export function useQueuedMessageAutoSend(enabledOrOptions?: boolean | { enabled? if (inFlightSessionsRef.current.has(targetKey)) { return; } - if (hasRecentAbort(sessionId)) { + const abortHoldUntil = getAbortHoldUntil(sessionId); + if (abortHoldUntil !== null) { + retryScheduler.schedule(abortHoldUntil); return; } if (useAutoReviewStore.getState().isRunningForSession(sessionId)) { @@ -185,7 +229,8 @@ export function useQueuedMessageAutoSend(enabledOrOptions?: boolean | { enabled? const failure = sendFailuresRef.current.get(targetKey); if (failure && failure.messageId !== payload.queuedMessageId) { sendFailuresRef.current.delete(targetKey); - } else if (isQueuedAutoSendBackedOff(failure, payload.queuedMessageId, Date.now())) { + } else if (failure && isQueuedAutoSendBackedOff(failure, payload.queuedMessageId, Date.now())) { + retryScheduler.schedule(failure.nextAttemptAt); return; } @@ -195,6 +240,10 @@ export function useQueuedMessageAutoSend(enabledOrOptions?: boolean | { enabled? ? captured : resolveSessionSendConfig(sessionId); if (!resolved.providerID || !resolved.modelID) { + // Legacy queues may predate captured send configuration. Config + // hydration is asynchronous, so retry instead of stranding the item + // until an unrelated status or directory update happens. + retryScheduler.schedule(Date.now() + AUTO_SEND_RETRY_BASE_DELAY_MS); return; } @@ -213,11 +262,13 @@ export function useQueuedMessageAutoSend(enabledOrOptions?: boolean | { enabled? console.warn('[queue] queued auto-send failed:', error); const priorFailures = failure?.messageId === payload.queuedMessageId ? failure.failures : 0; const failures = priorFailures + 1; + const nextAttemptAt = Date.now() + getQueuedAutoSendRetryDelayMs(failures); sendFailuresRef.current.set(targetKey, { messageId: payload.queuedMessageId, failures, - nextAttemptAt: Date.now() + getQueuedAutoSendRetryDelayMs(failures), + nextAttemptAt, }); + retryScheduler.schedule(nextAttemptAt); } finally { inFlightSessionsRef.current.delete(targetKey); } @@ -257,5 +308,5 @@ export function useQueuedMessageAutoSend(enabledOrOptions?: boolean | { enabled? }); previousStatusRef.current = nextStatusMap; - }, [enabled, queuedMessages, sessionStatusRecord, autoReviewRuns, currentDirectory]); + }, [enabled, queuedMessages, sessionStatusRecord, autoReviewRuns, currentDirectory, retryTick, retryScheduler]); } diff --git a/packages/ui/src/lib/debug.ts b/packages/ui/src/lib/debug.ts index bafb8272..cece345d 100644 --- a/packages/ui/src/lib/debug.ts +++ b/packages/ui/src/lib/debug.ts @@ -1,12 +1,19 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { useSessionUIStore } from '@/sync/session-ui-store'; +import { useSessionUIStore, getRememberedSessionDirectory } from '@/sync/session-ui-store'; import { useDirectoryStore } from '@/stores/useDirectoryStore'; import { useProjectsStore } from '@/stores/useProjectsStore'; import { opencodeClient } from '@/lib/opencode/client'; import { checkIsGitRepository } from '@/lib/gitApi'; import { streamDebugEnabled } from '@/stores/utils/streamDebug'; import { copyTextToClipboard as copyPlainTextToClipboard } from '@/lib/clipboard'; -import { getSyncSessions, getSyncMessages, getSyncParts } from '@/sync/sync-refs'; +import { getSyncSessions, getSyncMessages, getSyncParts, getAllSyncSessions, getSyncSessionDirectory } from '@/sync/sync-refs'; +import { + describeSessionDirectorySources, + resolveSessionDirectoryFromSources, +} from '@/sync/session-directory-resolution'; +import { useSessionWorktreeStore } from '@/sync/session-worktree-store'; +import { getRecentSendFailures } from '@/sync/send-failure-log'; +import { getAttachedSessionDirectory } from '@/sync/session-worktree-contract'; import { useStreamingStore } from '@/sync/streaming'; import { runtimeFetch } from '@/lib/runtime-fetch'; import { getRegisteredRuntimeAPIs } from '@/contexts/runtimeAPIRegistry'; @@ -375,12 +382,107 @@ export const debugUtils = { openchamber: { settingsInfo, }, + // Empty is a meaningful answer here: it means no prompt was rejected in + // this session, so a "my message disappeared" report is not a rejected + // send and needs a different explanation. + recentSendFailures: getRecentSendFailures(), + currentSessionDirectoryResolution: sessionState.currentSessionId + ? this.diagnoseSessionDirectory(sessionState.currentSessionId) + : null, }; console.log('[DEBUG] App status snapshot:', report); return report; }, + /** + * Prompt sends that were rejected and rolled back in this app session. + * Newest first; empty means no send was rejected. + */ + getRecentSendFailures() { + const failures = getRecentSendFailures(); + if (failures.length === 0) { + console.log('[OK] No prompt sends were rejected in this session.'); + } else { + console.warn(`[ALERT] ${failures.length} rejected prompt send(s):`); + console.table(failures); + } + return failures; + }, + + /** + * Report how a session's directory is resolved, from every source, in + * precedence order. A send is routed by the winning value, so a disagreement + * here explains a prompt that vanishes without an error: it was posted + * against a directory that does not own the session. + */ + diagnoseSessionDirectory(sessionId?: string) { + const sessionState = useSessionUIStore.getState(); + const targetSessionId = sessionId ?? sessionState.currentSessionId; + + if (!targetSessionId) { + console.log('[ERROR] No session selected and no session id passed'); + return null; + } + + const attachment = getAttachedSessionDirectory( + useSessionWorktreeStore.getState().getAttachment(targetSessionId), + ); + const worktreeMetadata = sessionState.worktreeMetadata.get(targetSessionId)?.path ?? null; + const owningStoreDirectory = getSyncSessionDirectory(targetSessionId); + const sessionRecord = getAllSyncSessions().find((session) => session.id === targetSessionId); + const recordDirectory = (sessionRecord as { directory?: string | null } | undefined)?.directory ?? null; + const selected = targetSessionId === sessionState.currentSessionId + ? sessionState.currentSessionDirectory + : null; + + const remembered = getRememberedSessionDirectory(targetSessionId); + + const sources = { + attachment, + worktreeMetadata, + authoritative: owningStoreDirectory ?? recordDirectory, + selected, + remembered: remembered.runtime, + }; + + const resolution = resolveSessionDirectoryFromSources(sources); + const routedDirectory = sessionState.getDirectoryForSession(targetSessionId); + + const report = { + sessionId: targetSessionId, + isCurrentSession: targetSessionId === sessionState.currentSessionId, + routedDirectory, + resolvedFrom: resolution.source, + conflict: resolution.conflict, + sources: describeSessionDirectorySources(sources), + details: { + owningChildStore: owningStoreDirectory, + sessionRecordDirectory: recordDirectory, + sessionIndexed: Boolean(sessionRecord), + currentSessionDirectory: sessionState.currentSessionDirectory, + rememberedForRuntime: remembered.runtime, + persistedAcrossRestarts: remembered.persisted, + activeDirectory: useDirectoryStore.getState().currentDirectory ?? null, + opencodeClientDirectory: opencodeClient.getDirectory() ?? null, + }, + }; + + console.log('[DEBUG] Session directory resolution:', report); + if (resolution.conflict) { + console.warn( + `[ALERT] Directory sources disagree: using "${resolution.directory}" (${resolution.source}) ` + + `while "${resolution.conflict.directory}" came from ${resolution.conflict.source}.`, + ); + } else if (!routedDirectory) { + console.warn('[ALERT] No directory resolved for this session — sends fall back to the active directory.'); + } else { + console.log('[OK] All known sources agree on the session directory.'); + } + + return report; + }, + async buildDiagnosticsReport() { const report = await this.getAppStatus(); return JSON.stringify(report, null, 2); @@ -697,6 +799,8 @@ if (typeof window !== 'undefined') { console.log(' __opencodeDebug.getAllMessages(truncate?) - List all messages (truncate=true for short preview)'); console.log(' __opencodeDebug.truncateMessages(messages) - Truncate long fields in messages array'); console.log(' __opencodeDebug.getAppStatus() - Show app status snapshot'); + console.log(' __opencodeDebug.diagnoseSessionDirectory(sessionId?) - Show how the session directory is resolved'); + console.log(' __opencodeDebug.getRecentSendFailures() - List prompt sends that were rejected and rolled back'); console.log(' __opencodeDebug.checkLastMessage() - Check if last message is problematic'); console.log(' __opencodeDebug.findEmptyMessages() - Find all empty assistant messages'); console.log(' __opencodeDebug.showRetryHelp() - Show instructions for handling empty responses'); diff --git a/packages/ui/src/lib/terminalApi.test.ts b/packages/ui/src/lib/terminalApi.test.ts index b02aa0ac..7573984a 100644 --- a/packages/ui/src/lib/terminalApi.test.ts +++ b/packages/ui/src/lib/terminalApi.test.ts @@ -93,6 +93,157 @@ describe('terminal transport', () => { transport.dispose(); }); + test('invalidates URL auth when the current socket closes before opening', async () => { + const socket = new FakeSocket(); + let cleared = 0; + const transport = new TerminalTransport({ + refreshAuth: async () => '', + openSocket: () => socket, + clearUrlAuthToken: () => { cleared += 1; }, + }); + + const unsubscribe = transport.subscribe('term-1', { onEvent: () => {} }); + await tick(); + socket.close(); + await tick(); + + expect(cleared).toBe(1); + unsubscribe(); + transport.dispose(); + }); + + test('invalidates URL auth before retrying a pre-open socket error', async () => { + const socket = new FakeSocket(); + let cleared = 0; + const transport = new TerminalTransport({ + refreshAuth: async () => '', + openSocket: () => socket, + clearUrlAuthToken: () => { cleared += 1; }, + }); + + const unsubscribe = transport.subscribe('term-1', { onEvent: () => {} }); + await tick(); + socket.onerror?.(); + + expect(cleared).toBe(1); + unsubscribe(); + transport.dispose(); + }); + + test('does not let a cancelled opening reconnect a replacement subscription', async () => { + const sockets = [new FakeSocket(), new FakeSocket()]; + let socketIndex = 0; + const replacementEvents: string[] = []; + const transport = new TerminalTransport({ + refreshAuth: async () => '', + openSocket: () => sockets[socketIndex++]!, + }); + + const unsubscribeFirst = transport.subscribe('term-1', { onEvent: () => {} }); + await tick(); + unsubscribeFirst(); + + const unsubscribeReplacement = transport.subscribe('term-1', { + onEvent: (event) => replacementEvents.push(event.type), + }); + await tick(); + sockets[1]?.open(); + await tick(); + + expect(replacementEvents).not.toContain('reconnecting'); + unsubscribeReplacement(); + transport.dispose(); + }); + + test('starts a fresh reconnect sequence after every terminal has detached', async () => { + const firstEvents: number[] = []; + const replacementEvents: number[] = []; + const transport = new TerminalTransport({ + refreshAuth: async () => '', + openSocket: () => { throw new Error('offline'); }, + }); + + const unsubscribeFirst = transport.subscribe('term-1', { + onEvent: (event) => { + if (event.type === 'reconnecting' && typeof event.attempt === 'number') firstEvents.push(event.attempt); + }, + }); + await tick(); + await tick(); + expect(firstEvents).toEqual([1]); + + unsubscribeFirst(); + const unsubscribeReplacement = transport.subscribe('term-2', { + onEvent: (event) => { + if (event.type === 'reconnecting' && typeof event.attempt === 'number') replacementEvents.push(event.attempt); + }, + }); + await tick(); + await tick(); + + expect(replacementEvents).toEqual([1]); + unsubscribeReplacement(); + transport.dispose(); + }); + + test('waits a minute before reconnecting while hidden', async () => { + const originalSetTimeout = globalThis.setTimeout; + const originalDocument = Object.getOwnPropertyDescriptor(globalThis, 'document'); + const delays: number[] = []; + let transport: TerminalTransport | null = null; + + Object.defineProperty(globalThis, 'document', { + configurable: true, + value: { + visibilityState: 'hidden', + addEventListener: () => {}, + removeEventListener: () => {}, + }, + }); + globalThis.setTimeout = ((handler: TimerHandler, timeout?: number, ...args: unknown[]) => { + delays.push(Number(timeout ?? 0)); + if (timeout === 0) return originalSetTimeout(handler, 0, ...args); + return 0 as unknown as ReturnType; + }) as typeof setTimeout; + + try { + transport = new TerminalTransport({ + refreshAuth: async () => '', + openSocket: () => { throw new Error('offline'); }, + }); + transport.subscribe('term-1', { onEvent: () => {} }); + await tick(); + await tick(); + + expect(delays).toContain(60_000); + } finally { + transport?.dispose(); + globalThis.setTimeout = originalSetTimeout; + if (originalDocument) Object.defineProperty(globalThis, 'document', originalDocument); + else delete (globalThis as { document?: unknown }).document; + } + }); + + test('attaches a remaining same-terminal subscriber after the first one leaves', async () => { + const socket = new FakeSocket(); + const transport = new TerminalTransport({ refreshAuth: async () => '', openSocket: () => socket }); + + const unsubscribeOther = transport.subscribe('term-other', { onEvent: () => {} }); + await tick(); + socket.open(); + await tick(); + + const unsubscribeFirst = transport.subscribe('term-1', { onEvent: () => {} }); + const unsubscribeRemaining = transport.subscribe('term-1', { onEvent: () => {} }); + unsubscribeFirst(); + await tick(); + + expect(socket.sent.filter((message) => message.t === 'attach' && message.s === 'term-1')).toHaveLength(1); + unsubscribeRemaining(); + unsubscribeOther(); + transport.dispose(); + }); + test('releases replay projections when the last subscriber detaches', async () => { const socket = new FakeSocket(); const transport = new TerminalTransport({ refreshAuth: async () => '', openSocket: () => socket }); diff --git a/packages/ui/src/lib/terminalApi.ts b/packages/ui/src/lib/terminalApi.ts index 3e555bf7..4bfce9d5 100644 --- a/packages/ui/src/lib/terminalApi.ts +++ b/packages/ui/src/lib/terminalApi.ts @@ -3,7 +3,7 @@ import { openRuntimeWebSocket } from './relay/runtime-socket'; import type { RelayTunnelWebSocket } from './relay/tunnel-client'; import { runtimeFetch } from './runtime-fetch'; import { getRuntimeUrlResolver } from './runtime-url'; -import { refreshRuntimeUrlAuthToken } from './runtime-auth'; +import { clearRuntimeUrlAuthToken, refreshRuntimeUrlAuthToken } from './runtime-auth'; import { isTerminalShell } from './terminalShell'; type Message = Record & { t: string; s?: string; q?: number }; @@ -66,12 +66,12 @@ const trimProjection = (value: string): string => { type TerminalTransportDependencies = { refreshAuth: () => Promise; openSocket: () => RelayTunnelWebSocket; + clearUrlAuthToken?: () => void; }; export class TerminalTransport { private socket: RelayTunnelWebSocket | null = null; private opening: Promise | null = null; - private openingGeneration: number | null = null; private subscribers = new Map>(); private projections = new Map(); private reconnectTimer: ReturnType | null = null; @@ -85,6 +85,7 @@ export class TerminalTransport { constructor(private readonly dependencies: TerminalTransportDependencies = { refreshAuth: refreshRuntimeUrlAuthToken, openSocket: () => openRuntimeWebSocket(getRuntimeUrlResolver().websocket('/api/terminal/ws')), + clearUrlAuthToken: clearRuntimeUrlAuthToken, }) {} subscribe(sessionId: string, handlers: TerminalHandlers): () => void { @@ -100,7 +101,13 @@ export class TerminalTransport { handlers.onEvent({ type: 'snapshot', sequence: projection.sequence, data: projection.history, status: projection.status, exitCode: projection.exitCode, signal: projection.signal, runtime: projection.runtime, ptyBackend: projection.ptyBackend }); } const socketWasOpen = this.socket?.readyState === SOCKET_OPEN; - this.ensureConnected().then(() => { if (first && socketWasOpen && set.has(subscriber)) this.send({ t: 'attach', v: 3, s: sessionId }); }).catch((error) => { + this.ensureConnected().then(() => { + const current = this.subscribers.get(sessionId); + if (first && socketWasOpen && current === set && current.size > 0) { + this.send({ t: 'attach', v: 3, s: sessionId }); + } + }).catch((error) => { + if (!set.has(subscriber)) return; handlers.onError?.(error, false); this.scheduleReconnect(); }); @@ -114,6 +121,7 @@ export class TerminalTransport { } if (this.subscribers.size === 0) { this.cancelReconnect(); + this.failures = 0; if (this.socket?.readyState === SOCKET_OPEN) { // Healthy socket: hold it briefly so a tab switch can reattach to it. this.scheduleIdleClose(); @@ -121,6 +129,7 @@ export class TerminalTransport { } // Nothing to reuse, so abandon any dial that is still in flight. this.generation += 1; + this.opening = null; this.closeSocket(); } }; @@ -138,6 +147,7 @@ export class TerminalTransport { dispose(): void { this.disposed = true; this.generation += 1; + this.opening = null; this.subscribers.clear(); this.projections.clear(); if (this.reconnectTimer) clearTimeout(this.reconnectTimer); @@ -155,71 +165,89 @@ export class TerminalTransport { private async ensureConnected(): Promise { if (this.disposed) throw new Error('Terminal runtime changed'); if (this.socket?.readyState === SOCKET_OPEN) return; - if (this.opening && this.openingGeneration === this.generation) { + if (this.opening) { await this.opening; if (this.socket?.readyState === SOCKET_OPEN) return; return this.ensureConnected(); } - if (this.openingGeneration !== this.generation) { - this.opening = null; - this.openingGeneration = null; - } const generation = this.generation; const opening = (async () => { await this.dependencies.refreshAuth(); if (generation !== this.generation || this.disposed) throw new Error('Terminal runtime changed'); await new Promise((resolve, reject) => { - let settled = false; - let pendingSocket: RelayTunnelWebSocket | null = null; - const finish = (error?: Error) => { - if (settled) return; - settled = true; - clearTimeout(timeout); - if (error) reject(error); - else resolve(); - }; - const timeout = setTimeout(() => { - pendingSocket?.close(); - finish(new Error('Terminal connection timed out')); - }, 10_000); - try { - const socket = this.dependencies.openSocket(); - pendingSocket = socket; - socket.binaryType = 'arraybuffer'; - this.socket = socket; - socket.onopen = () => { - if (generation !== this.generation || this.disposed) { socket.close(); finish(new Error('Terminal runtime changed')); return; } - this.failures = 0; - this.send({ t: 'hello', v: 3 }); - for (const sessionId of this.subscribers.keys()) this.send({ t: 'attach', v: 3, s: sessionId }); - this.startKeepalive(); - finish(); + let settled = false; + let opened = false; + let authInvalidated = false; + let pendingSocket: RelayTunnelWebSocket | null = null; + const isCurrentSocket = () => ( + generation === this.generation && + !this.disposed && + pendingSocket !== null && + this.socket === pendingSocket + ); + const invalidatePreOpenAuth = () => { + if (authInvalidated || opened || !isCurrentSocket()) return; + authInvalidated = true; + this.dependencies.clearUrlAuthToken?.(); }; - socket.onmessage = (event) => void this.handleMessage(event.data); - socket.onerror = () => { - finish(new Error('Terminal WebSocket failed')); + const finish = (error?: Error) => { + if (settled) return; + settled = true; + clearTimeout(timeout); + if (error) reject(error); + else resolve(); + }; + const timeout = setTimeout(() => { + invalidatePreOpenAuth(); + pendingSocket?.close(); + finish(new Error('Terminal connection timed out')); + }, 10_000); + try { + const socket = this.dependencies.openSocket(); + pendingSocket = socket; + socket.binaryType = 'arraybuffer'; + this.socket = socket; + socket.onopen = () => { + if (!isCurrentSocket()) { socket.close(); finish(new Error('Terminal runtime changed')); return; } + opened = true; + this.failures = 0; + this.send({ t: 'hello', v: 3 }); + for (const sessionId of this.subscribers.keys()) this.send({ t: 'attach', v: 3, s: sessionId }); + this.startKeepalive(); + finish(); + }; + socket.onmessage = (event) => void this.handleMessage(event.data); + socket.onerror = () => { + const current = isCurrentSocket(); + if (current) invalidatePreOpenAuth(); + finish(new Error('Terminal WebSocket failed')); + if (current && this.subscribers.size > 0) this.scheduleReconnect(); + }; + socket.onclose = () => { + const current = isCurrentSocket(); + if (current) { + this.stopKeepalive(); + // An upgrade rejected before `open` commonly means the cached + // URL-scoped auth token is stale. Retrying it reaches the 8s + // backoff cap instead of minting a fresh token. + invalidatePreOpenAuth(); + } + if (this.socket === socket) this.socket = null; + finish(new Error('Terminal WebSocket closed')); + if (current && this.subscribers.size > 0) this.scheduleReconnect(); + }; + } catch (error) { + finish(error instanceof Error ? error : new Error('Terminal WebSocket failed')); if (!this.disposed && this.subscribers.size > 0) this.scheduleReconnect(); - }; - socket.onclose = () => { - if (this.socket === socket) this.socket = null; - this.stopKeepalive(); - finish(new Error('Terminal WebSocket closed')); - if (!this.disposed && this.subscribers.size > 0) this.scheduleReconnect(); - }; - } catch (error) { - finish(error instanceof Error ? error : new Error('Terminal WebSocket failed')); - if (!this.disposed && this.subscribers.size > 0) this.scheduleReconnect(); - } + } }); })(); this.opening = opening; - this.openingGeneration = generation; try { await opening; } finally { if (this.opening === opening) { this.opening = null; - this.openingGeneration = null; } } } @@ -279,7 +307,7 @@ export class TerminalTransport { if (this.reconnectTimer || this.disposed || this.subscribers.size === 0) return; this.failures += 1; const slow = (typeof document !== 'undefined' && document.visibilityState === 'hidden') || (typeof navigator !== 'undefined' && !navigator.onLine); - const delay = Math.min(500 * 2 ** Math.min(this.failures - 1, 10), slow ? 60_000 : 8_000); + const delay = slow ? 60_000 : Math.min(500 * 2 ** Math.min(this.failures - 1, 10), 8_000); for (const set of this.subscribers.values()) for (const sub of set) sub.handlers.onEvent({ type: 'reconnecting', attempt: this.failures, maxAttempts: Number.POSITIVE_INFINITY }); const wake = () => { if (typeof document !== 'undefined' && document.visibilityState === 'hidden') return; @@ -304,6 +332,7 @@ export class TerminalTransport { this.idleCloseTimer = null; if (this.disposed || this.subscribers.size > 0) return; this.generation += 1; + this.opening = null; this.closeSocket(); }, IDLE_SOCKET_GRACE_MS); } diff --git a/packages/ui/src/stores/useAgentsStore.ts b/packages/ui/src/stores/useAgentsStore.ts index 1412bc0a..4e7beef2 100644 --- a/packages/ui/src/stores/useAgentsStore.ts +++ b/packages/ui/src/stores/useAgentsStore.ts @@ -700,7 +700,9 @@ async function performConfigRefresh(options: { uiRefreshTasks.push(commandsStore.loadCommands().then(() => undefined)); } if (refreshSkills) { - invalidateSkillsLoadCache(currentDirectory); + // Match loadSkills cache key (active-project-first). Passing client/directory-store + // path here misses the key when those diverge after getRequestDirectory(). + invalidateSkillsLoadCache(); uiRefreshTasks.push(skillsStore.loadSkills().then(() => undefined)); uiRefreshTasks.push(skillsCatalogStore.loadCatalog({ refresh: true }).then(() => undefined)); } diff --git a/packages/ui/src/stores/useMultiRunStore.test.ts b/packages/ui/src/stores/useMultiRunStore.test.ts index b2a1bbb6..c01295f1 100644 --- a/packages/ui/src/stores/useMultiRunStore.test.ts +++ b/packages/ui/src/stores/useMultiRunStore.test.ts @@ -127,6 +127,7 @@ mock.module('./useGlobalSessionsStore', () => ({ })); mock.module('@/sync/sync-refs', () => ({ + getSyncSessionDirectory: () => null, registerSessionDirectory: (sessionID: string, directory: string) => { registeredDirectories.push({ sessionID, directory }); }, diff --git a/packages/ui/src/stores/useSkillsCatalogStore.ts b/packages/ui/src/stores/useSkillsCatalogStore.ts index 139348fb..6585d152 100644 --- a/packages/ui/src/stores/useSkillsCatalogStore.ts +++ b/packages/ui/src/stores/useSkillsCatalogStore.ts @@ -15,6 +15,7 @@ import type { import { invalidateSkillsLoadCache, refreshSkillsAfterOpenCodeRestart, useSkillsStore } from '@/stores/useSkillsStore'; import { opencodeClient } from '@/lib/opencode/client'; +import { useProjectsStore } from '@/stores/useProjectsStore'; import { startConfigUpdate, finishConfigUpdate, updateConfigUpdateMessage } from '@/lib/configUpdate'; import { runtimeFetch } from '@/lib/runtime-fetch'; @@ -45,20 +46,21 @@ const getSkillsCatalogCacheKey = (directory: string | null): string => { return directory?.trim() || DEFAULT_SKILLS_CATALOG_CACHE_KEY; }; -const getCurrentDirectory = (): string | null => { - const opencodeDirectory = opencodeClient.getDirectory(); - if (typeof opencodeDirectory === 'string' && opencodeDirectory.trim().length > 0) { - return opencodeDirectory; - } - +const getRequestDirectory = (): string | null => { try { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const store = (window as any).__zustand_directory_store__; - if (store) { - return store.getState().currentDirectory; + const projectsStore = useProjectsStore.getState(); + const activeProject = projectsStore.getActiveProject?.(); + + if (activeProject?.path?.trim()) { + return activeProject.path.trim(); } - } catch { - // ignore + + const clientDir = opencodeClient.getDirectory(); + if (clientDir?.trim()) { + return clientDir.trim(); + } + } catch (err) { + console.warn('[SkillsCatalogStore] Error resolving config directory:', err); } return null; @@ -118,7 +120,7 @@ export const useSkillsCatalogStore = create()( setSelectedSource: (id) => set({ selectedSourceId: id }), loadCatalog: async (options) => { - const currentDirectory = getCurrentDirectory(); + const currentDirectory = getRequestDirectory(); const cacheKey = getSkillsCatalogCacheKey(currentDirectory); const now = Date.now(); const loadedAt = skillsCatalogLastLoadedAt.get(cacheKey) ?? 0; @@ -222,7 +224,7 @@ export const useSkillsCatalogStore = create()( set({ isLoadingSource: true, lastCatalogError: null }); try { - const currentDirectory = getCurrentDirectory(); + const currentDirectory = getRequestDirectory(); const refresh = options?.refresh ? '&refresh=true' : ''; const queryParams = currentDirectory ? `?directory=${encodeURIComponent(currentDirectory)}&sourceId=${encodeURIComponent(sourceId)}${refresh}` @@ -293,7 +295,7 @@ export const useSkillsCatalogStore = create()( set({ isLoadingMore: true }); try { - const currentDirectory = getCurrentDirectory(); + const currentDirectory = getRequestDirectory(); const parts = [`sourceId=${encodeURIComponent(selectedSourceId)}`]; if (currentDirectory) { parts.push(`directory=${encodeURIComponent(currentDirectory)}`); @@ -355,7 +357,7 @@ export const useSkillsCatalogStore = create()( scanRepo: async (request) => { set({ isScanning: true, lastScanError: null, scanResults: null }); try { - const currentDirectory = getCurrentDirectory(); + const currentDirectory = getRequestDirectory(); const queryParams = currentDirectory ? `?directory=${encodeURIComponent(currentDirectory)}` : ''; const response = await runtimeFetch(`/api/config/skills/scan${queryParams}`, { @@ -391,7 +393,7 @@ export const useSkillsCatalogStore = create()( const directoryOverride = typeof options?.directory === 'string' && options.directory.trim().length > 0 ? options.directory.trim() : null; - const currentDirectory = directoryOverride ?? getCurrentDirectory(); + const currentDirectory = directoryOverride ?? getRequestDirectory(); const queryParams = currentDirectory ? `?directory=${encodeURIComponent(currentDirectory)}` : ''; const response = await runtimeFetch(`/api/config/skills/install${queryParams}`, { diff --git a/packages/ui/src/stores/useSkillsStore.test.ts b/packages/ui/src/stores/useSkillsStore.test.ts new file mode 100644 index 00000000..f75fb711 --- /dev/null +++ b/packages/ui/src/stores/useSkillsStore.test.ts @@ -0,0 +1,119 @@ +import { beforeEach, describe, expect, mock, test } from 'bun:test'; + +const activeProjectPath = '/workspace/project-with-agents-skills'; + +let runtimeFetchCalls: Array<{ url: string; headers?: HeadersInit }> = []; +let runtimeFetchImpl: (url: string, init?: RequestInit) => Promise = async () => ( + new Response(JSON.stringify({ skills: [] }), { + headers: { 'Content-Type': 'application/json' }, + }) +); +let getDirectoryImpl: () => string | undefined = () => undefined; + +const runtimeFetchMock = async (url: string, init?: RequestInit) => { + runtimeFetchCalls.push({ url: String(url), headers: init?.headers }); + return runtimeFetchImpl(url, init); +}; + +mock.module('@/lib/opencode/client', () => ({ + opencodeClient: { + getDirectory: () => getDirectoryImpl(), + checkHealth: async () => true, + }, +})); + +mock.module('@/stores/useProjectsStore', () => ({ + useProjectsStore: { + getState: () => ({ + getActiveProject: () => ({ path: activeProjectPath }), + }), + }, +})); + +mock.module('@/lib/runtime-fetch', () => ({ + runtimeFetch: runtimeFetchMock, +})); + +mock.module('@/lib/background-network', () => ({ + runBackgroundNetworkTask: async (task: () => Promise) => task(), +})); + +mock.module('@/lib/configUpdate', () => ({ + startConfigUpdate: mock(() => undefined), + finishConfigUpdate: mock(() => undefined), + updateConfigUpdateMessage: mock(() => undefined), +})); + +mock.module('@/lib/configSync', () => ({ + emitConfigChange: mock(() => undefined), + scopeMatches: mock(() => false), + subscribeToConfigChanges: mock(() => () => undefined), +})); + +mock.module('./utils/safeStorage', () => ({ + createDeferredSafeJSONStorage: () => ({ + getItem: async () => null, + setItem: async () => undefined, + removeItem: async () => undefined, + }), +})); + +const { invalidateSkillsLoadCache, useSkillsStore } = await import('./useSkillsStore'); + +describe('useSkillsStore directory resolution', () => { + beforeEach(() => { + runtimeFetchCalls = []; + getDirectoryImpl = () => undefined; + runtimeFetchImpl = async () => new Response(JSON.stringify({ + skills: [{ + name: 'repo-local-skill', + path: `${activeProjectPath}/.agents/skills/repo-local-skill/SKILL.md`, + scope: 'project', + source: 'agents', + sources: { md: { description: 'Repository local' } }, + }], + }), { + headers: { 'Content-Type': 'application/json' }, + }); + + invalidateSkillsLoadCache(activeProjectPath); + useSkillsStore.setState({ + selectedSkillName: null, + skills: [], + isLoading: false, + skillDraft: null, + }); + }); + + test('loadSkills scopes discovery to the active project even when client directory is unset', async () => { + const loaded = await useSkillsStore.getState().loadSkills(); + + expect(loaded).toBe(true); + expect(runtimeFetchCalls.length).toBe(1); + expect(runtimeFetchCalls[0]?.url).toContain(`directory=${encodeURIComponent(activeProjectPath)}`); + expect(useSkillsStore.getState().skills).toEqual([{ + name: 'repo-local-skill', + path: `${activeProjectPath}/.agents/skills/repo-local-skill/SKILL.md`, + scope: 'project', + source: 'agents', + description: 'Repository local', + group: undefined, + }]); + }); + + test('invalidateSkillsLoadCache() with no argument clears the active-project cache key used by loadSkills', async () => { + expect(await useSkillsStore.getState().loadSkills()).toBe(true); + expect(runtimeFetchCalls.length).toBe(1); + + // Wrong key: client-directory-first null maps to __default__, not the active project. + invalidateSkillsLoadCache(null); + expect(await useSkillsStore.getState().loadSkills()).toBe(true); + expect(runtimeFetchCalls.length).toBe(1); + + // Default resolution must match loadSkills (active project first). + invalidateSkillsLoadCache(); + expect(await useSkillsStore.getState().loadSkills()).toBe(true); + expect(runtimeFetchCalls.length).toBe(2); + expect(runtimeFetchCalls[1]?.url).toContain(`directory=${encodeURIComponent(activeProjectPath)}`); + }); +}); diff --git a/packages/ui/src/stores/useSkillsStore.ts b/packages/ui/src/stores/useSkillsStore.ts index 2ffe69dd..eab17af2 100644 --- a/packages/ui/src/stores/useSkillsStore.ts +++ b/packages/ui/src/stores/useSkillsStore.ts @@ -10,23 +10,29 @@ import { import { createDeferredSafeJSONStorage } from "./utils/safeStorage"; import { runtimeFetch } from "@/lib/runtime-fetch"; import { runBackgroundNetworkTask } from "@/lib/background-network"; +import { useProjectsStore } from "@/stores/useProjectsStore"; import { opencodeClient } from '@/lib/opencode/client'; -const getCurrentDirectory = (): string | null => { - const opencodeDirectory = opencodeClient.getDirectory(); - if (typeof opencodeDirectory === 'string' && opencodeDirectory.trim().length > 0) { - return opencodeDirectory; - } - +// Prefer the active project path so Settings/Skills discovery matches the +// project selector (and Commands/Agents). Falling back only to the session +// directory misses repository-local `.agents/skills` when the client directory +// is unset or points elsewhere while an active project exists. +const getRequestDirectory = (): string | null => { try { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const store = (window as any).__zustand_directory_store__; - if (store) { - return store.getState().currentDirectory; + const projectsStore = useProjectsStore.getState(); + const activeProject = projectsStore.getActiveProject?.(); + + if (activeProject?.path?.trim()) { + return activeProject.path.trim(); } - } catch { - // ignore + + const clientDir = opencodeClient.getDirectory(); + if (clientDir?.trim()) { + return clientDir.trim(); + } + } catch (err) { + console.warn('[SkillsStore] Error resolving config directory:', err); } return null; @@ -169,7 +175,7 @@ const getSkillsCacheKey = (directory: string | null): string => { return directory?.trim() || DEFAULT_SKILLS_CACHE_KEY; }; -export const invalidateSkillsLoadCache = (directory: string | null = getCurrentDirectory()) => { +export const invalidateSkillsLoadCache = (directory: string | null = getRequestDirectory()) => { skillsLastLoadedAt.delete(getSkillsCacheKey(directory)); }; @@ -198,8 +204,8 @@ export const useSkillsStore = create()( }, loadSkills: async () => { - const currentDirectory = getCurrentDirectory(); - const cacheKey = getSkillsCacheKey(currentDirectory); + const directory = getRequestDirectory(); + const cacheKey = getSkillsCacheKey(directory); const now = Date.now(); const loadedAt = skillsLastLoadedAt.get(cacheKey) ?? 0; const hasCachedSkills = get().skills.length > 0; @@ -220,9 +226,12 @@ export const useSkillsStore = create()( for (let attempt = 0; attempt < 3; attempt++) { try { - const queryParams = currentDirectory ? `?directory=${encodeURIComponent(currentDirectory)}` : ''; + const queryParams = directory ? `?directory=${encodeURIComponent(directory)}` : ''; - const response = await runBackgroundNetworkTask(() => runtimeFetch(`/api/config/skills${queryParams}`, { priority: 'low' })); + const response = await runBackgroundNetworkTask(() => runtimeFetch(`/api/config/skills${queryParams}`, { + priority: 'low', + headers: directory ? { 'x-opencode-directory': directory } : undefined, + })); if (!response.ok) { throw new Error(`Failed to list skills: ${response.status}`); } @@ -263,10 +272,12 @@ export const useSkillsStore = create()( getSkillDetail: async (name: string) => { try { - const currentDirectory = getCurrentDirectory(); - const queryParams = currentDirectory ? `?directory=${encodeURIComponent(currentDirectory)}` : ''; + const directory = getRequestDirectory(); + const queryParams = directory ? `?directory=${encodeURIComponent(directory)}` : ''; - const response = await runtimeFetch(`/api/config/skills/${encodeURIComponent(name)}${queryParams}`); + const response = await runtimeFetch(`/api/config/skills/${encodeURIComponent(name)}${queryParams}`, { + headers: directory ? { 'x-opencode-directory': directory } : undefined, + }); if (!response.ok) { return null; } @@ -291,12 +302,15 @@ export const useSkillsStore = create()( if (config.source) skillConfig.source = config.source; if (config.supportingFiles) skillConfig.supportingFiles = config.supportingFiles; - const currentDirectory = getCurrentDirectory(); - const queryParams = currentDirectory ? `?directory=${encodeURIComponent(currentDirectory)}` : ''; + const directory = getRequestDirectory(); + const queryParams = directory ? `?directory=${encodeURIComponent(directory)}` : ''; const response = await runtimeFetch(`/api/config/skills/${encodeURIComponent(config.name)}${queryParams}`, { method: 'POST', - headers: { 'Content-Type': 'application/json' }, + headers: { + 'Content-Type': 'application/json', + ...(directory ? { 'x-opencode-directory': directory } : {}), + }, body: JSON.stringify(skillConfig) }); @@ -307,7 +321,7 @@ export const useSkillsStore = create()( } const needsReload = payload?.requiresReload ?? false; - invalidateSkillsLoadCache(currentDirectory); + invalidateSkillsLoadCache(directory); if (needsReload) { requiresReload = true; await refreshSkillsAfterOpenCodeRestart({ @@ -342,12 +356,15 @@ export const useSkillsStore = create()( if (config.supportingFiles !== undefined) skillConfig.supportingFiles = config.supportingFiles; if (config.targetPath !== undefined) skillConfig.targetPath = config.targetPath; - const currentDirectory = getCurrentDirectory(); - const queryParams = currentDirectory ? `?directory=${encodeURIComponent(currentDirectory)}` : ''; + const directory = getRequestDirectory(); + const queryParams = directory ? `?directory=${encodeURIComponent(directory)}` : ''; const response = await runtimeFetch(`/api/config/skills/${encodeURIComponent(name)}${queryParams}`, { method: 'PATCH', - headers: { 'Content-Type': 'application/json' }, + headers: { + 'Content-Type': 'application/json', + ...(directory ? { 'x-opencode-directory': directory } : {}), + }, body: JSON.stringify(skillConfig) }); @@ -358,7 +375,7 @@ export const useSkillsStore = create()( } const needsReload = payload?.requiresReload ?? false; - invalidateSkillsLoadCache(currentDirectory); + invalidateSkillsLoadCache(directory); if (needsReload) { requiresReload = true; await refreshSkillsAfterOpenCodeRestart({ @@ -386,11 +403,12 @@ export const useSkillsStore = create()( startConfigUpdate("Deleting skill..."); let requiresReload = false; try { - const currentDirectory = getCurrentDirectory(); - const queryParams = currentDirectory ? `?directory=${encodeURIComponent(currentDirectory)}` : ''; + const directory = getRequestDirectory(); + const queryParams = directory ? `?directory=${encodeURIComponent(directory)}` : ''; const response = await runtimeFetch(`/api/config/skills/${encodeURIComponent(name)}${queryParams}`, { - method: 'DELETE' + method: 'DELETE', + headers: directory ? { 'x-opencode-directory': directory } : undefined, }); const payload = await response.json().catch(() => null); @@ -400,7 +418,7 @@ export const useSkillsStore = create()( } const needsReload = payload?.requiresReload ?? false; - invalidateSkillsLoadCache(currentDirectory); + invalidateSkillsLoadCache(directory); if (needsReload) { requiresReload = true; await refreshSkillsAfterOpenCodeRestart({ @@ -436,11 +454,12 @@ export const useSkillsStore = create()( readSupportingFile: async (skillName: string, filePath: string) => { try { - const currentDirectory = getCurrentDirectory(); - const queryParams = currentDirectory ? `&directory=${encodeURIComponent(currentDirectory)}` : ''; + const directory = getRequestDirectory(); + const queryParams = directory ? `&directory=${encodeURIComponent(directory)}` : ''; const response = await runtimeFetch( - `/api/config/skills/${encodeURIComponent(skillName)}/files/${encodeURIComponent(filePath)}?${queryParams.slice(1)}` + `/api/config/skills/${encodeURIComponent(skillName)}/files/${encodeURIComponent(filePath)}?${queryParams.slice(1)}`, + { headers: directory ? { 'x-opencode-directory': directory } : undefined }, ); if (!response.ok) { return null; @@ -455,14 +474,17 @@ export const useSkillsStore = create()( writeSupportingFile: async (skillName: string, filePath: string, content: string) => { try { - const currentDirectory = getCurrentDirectory(); - const queryParams = currentDirectory ? `?directory=${encodeURIComponent(currentDirectory)}` : ''; + const directory = getRequestDirectory(); + const queryParams = directory ? `?directory=${encodeURIComponent(directory)}` : ''; const response = await runtimeFetch( `/api/config/skills/${encodeURIComponent(skillName)}/files/${encodeURIComponent(filePath)}${queryParams}`, { method: 'PUT', - headers: { 'Content-Type': 'application/json' }, + headers: { + 'Content-Type': 'application/json', + ...(directory ? { 'x-opencode-directory': directory } : {}), + }, body: JSON.stringify({ content }) } ); @@ -475,12 +497,15 @@ export const useSkillsStore = create()( deleteSupportingFile: async (skillName: string, filePath: string) => { try { - const currentDirectory = getCurrentDirectory(); - const queryParams = currentDirectory ? `?directory=${encodeURIComponent(currentDirectory)}` : ''; + const directory = getRequestDirectory(); + const queryParams = directory ? `?directory=${encodeURIComponent(directory)}` : ''; const response = await runtimeFetch( `/api/config/skills/${encodeURIComponent(skillName)}/files/${encodeURIComponent(filePath)}${queryParams}`, - { method: 'DELETE' } + { + method: 'DELETE', + headers: directory ? { 'x-opencode-directory': directory } : undefined, + } ); return response.ok; diff --git a/packages/ui/src/sync/DOCUMENTATION.md b/packages/ui/src/sync/DOCUMENTATION.md index 93a475ff..78152f32 100644 --- a/packages/ui/src/sync/DOCUMENTATION.md +++ b/packages/ui/src/sync/DOCUMENTATION.md @@ -197,6 +197,30 @@ Directory stores also own session-keyed sidecar notification channels for permis Message sidecar consumers also filter targeted updates by purpose before notifying React. Suspended live-tail text/reasoning changes do not rebuild visible message records, but structural Task session identity changes bypass suspension so a parent can link a newly created subagent immediately. Assistant-only part changes do not rebuild user input history, and targeted updates that preserve authoritative part buckets do not recheck a session that is already renderable. Message replacements, removed final part buckets, and conservative resets always notify. +## Session directory resolution + +`session-directory-resolution.ts` owns the precedence used to answer "which directory does this session belong to". Every send, message fetch, message-queue key, and send-confirmation lookup is routed by that answer, so a wrong value is not a display problem: the prompt is posted against a directory that does not own the session, the request is rejected, and the optimistic message is rolled back with no visible error. + +Precedence, highest authority first: + +The discriminator is whether the server confirmed the path, not whether the value is local or synced. + +| Source | Meaning | +|---|---| +| `authoritative` | The child store that actually holds the session, then its own record | +| `selected` | Server-confirmed directory captured at selection; a guessed one is never passed | +| `attachment` | Worktree attachment recorded by this client; the *requested* path | +| `worktree-metadata` | Worktree captured when the session was created in one; the *requested* path | +| `remembered` | Per-runtime directory persisted across restarts | + +Rules: + +1. `getSyncSessionDirectory()` is the authoritative session→directory mapping: a session lives in exactly the child store for its directory, whether or not the server populated `session.directory`. `null` means "not indexed yet", never "no directory". +2. `attachment` and `worktreeMetadata` hold the worktree path this client asked for, before the server canonicalized it. They are a hint for a session sync has not indexed yet, never a correction of a confirmed directory — otherwise a stale local path re-creates the very mismatch this precedence exists to prevent. +3. Never persist or rank a guessed directory. `selectSession` may fall back to the active directory to keep routing usable, but that value is not written to runtime memory, not written to the last-active snapshot, and not passed as `selected` — a persisted guess outlives the race that produced it and survives reloads and restarts. +4. Components must not read `currentSessionDirectory` to build request or queue keys; use `getDirectoryForSession()` so every consumer resolves identically. +5. A disagreement between sources is logged once per session, and `__opencodeDebug.diagnoseSessionDirectory()` reports every source in precedence order. + ## Session action rules Session actions live in `session-actions.ts` and are the canonical place for SDK-calling session mutations that affect global session lists. @@ -207,6 +231,7 @@ Rules: 2. If an action targets a session by ID, resolve the **session's own directory**. Do not assume the current directory is correct. 3. `session-ui-store.ts` should delegate to `session-actions.ts` for these mutations instead of duplicating SDK calls. 4. Sending after a revert commits the new branch optimistically: remove the reverted tail and marker before inserting the new message, and restore both if the send is rejected. +5. After session creation, the directory returned by the server is authoritative over the requested draft directory. The server may canonicalize a worktree path, and the first prompt must use the same directory identity as the created session. Examples of global-store updates performed in `session-actions.ts`: diff --git a/packages/ui/src/sync/__tests__/issue-1637-2270.test.ts b/packages/ui/src/sync/__tests__/issue-1637-2270.test.ts index 5773f57e..5cd7f3b1 100644 --- a/packages/ui/src/sync/__tests__/issue-1637-2270.test.ts +++ b/packages/ui/src/sync/__tests__/issue-1637-2270.test.ts @@ -42,6 +42,7 @@ mock.module("../session-ui-store", () => ({ })) mock.module("../sync-refs", () => ({ + getSyncSessionDirectory: () => null, registerSessionDirectory: (sessionID: string, directory: string) => { registerSessionDirectoryCalls.push({ sessionID, directory }) }, diff --git a/packages/ui/src/sync/__tests__/issue-2039.test.ts b/packages/ui/src/sync/__tests__/issue-2039.test.ts index 8197823c..ef9f5439 100644 --- a/packages/ui/src/sync/__tests__/issue-2039.test.ts +++ b/packages/ui/src/sync/__tests__/issue-2039.test.ts @@ -4,11 +4,16 @@ import { togglePermissionAutoAccept } from "../../components/chat/permissionAuto const storage = new Map() const createSessionCalls: Array<{ title?: string; directory: string | null; parentID: string | null; metadata?: unknown }> = [] const permissionAutoAcceptCalls: Array<[string, boolean]> = [] +let createdSessionDirectory: string | undefined const getMockCalls = (fn: unknown): unknown[][] => ((fn as { mock?: { calls: unknown[][] } }).mock?.calls ?? []) mock.module("zustand", () => ({ - create: () => (initializer: (set: (patch: unknown | ((state: unknown) => unknown)) => void, get: () => unknown) => Record) => { + create: () => (initializer: ( + set: (patch: unknown | ((state: unknown) => unknown)) => void, + get: () => unknown, + api?: unknown, + ) => Record) => { let state: Record const get = () => state const set = (patch: unknown | ((current: Record) => unknown)) => { @@ -16,7 +21,12 @@ mock.module("zustand", () => ({ state = next && typeof next === "object" ? { ...state, ...(next as Record) } : state } - state = initializer(set, get) + state = initializer(set, get, { + setState: set, + getState: get, + getInitialState: get, + subscribe: () => () => undefined, + } as never) const store = ((selector?: (current: Record) => unknown) => ( typeof selector === "function" ? selector(state) : state @@ -53,6 +63,11 @@ const deferredStorage: Storage = { mock.module("@/stores/utils/safeStorage", () => ({ getDeferredSafeStorage: () => deferredStorage, + createDeferredSafeJSONStorage: () => ({ + getItem: async () => null, + setItem: async () => undefined, + removeItem: async () => undefined, + }), })) mock.module("@/lib/opencode/client", () => ({ @@ -224,15 +239,18 @@ mock.module("../sync-refs", () => ({ getSyncMessages: () => [], getSyncParts: () => [], getAllSyncSessions: () => [], + getSyncSessionDirectory: () => null, })) mock.module("../session-actions", () => ({ createSession: mock(async (title: string | undefined, directory: string | null, parentID: string | null, metadata?: unknown) => { createSessionCalls.push({ title, directory, parentID, metadata }) - return { id: "ses_issue_2039", directory } + return { id: "ses_issue_2039", directory: createdSessionDirectory ?? directory } }), deleteSession: mock(async () => true), + deleteSessions: mock(async () => ({ deletedIds: [], failedIds: [] })), archiveSession: mock(async () => true), + archiveSessions: mock(async () => ({ archivedIds: [], failedIds: [] })), updateSessionTitle: mock(async () => undefined), shareSession: mock(async () => undefined), unshareSession: mock(async () => undefined), @@ -242,6 +260,9 @@ mock.module("../session-actions", () => ({ unrevertSession: mock(async () => undefined), forkFromMessage: mock(async () => undefined), fetchMessagesForSession: mock(async () => undefined), + getSessionLastAssistantModel: () => null, + patchSessionMetadata: mock(async () => undefined), + abortCurrentOperation: mock(async () => undefined), })) const { materializeOpenDraftSession, useSessionUIStore } = await import("../session-ui-store") @@ -298,6 +319,7 @@ describe("issue 2039 draft auto-accept", () => { storage.clear() createSessionCalls.length = 0 permissionAutoAcceptCalls.length = 0 + createdSessionDirectory = undefined useSessionUIStore.setState({ currentSessionId: null, @@ -348,4 +370,45 @@ describe("issue 2039 draft auto-accept", () => { expect(createSessionCalls).toHaveLength(0) expect(permissionAutoAcceptCalls).toHaveLength(0) }) + + test("uses the server-authoritative directory after worktree session creation", async () => { + createdSessionDirectory = "/canonical/worktree" + useSessionUIStore.getState().openNewSessionDraft({ + directoryOverride: "/requested/worktree", + }) + + const result = await materializeOpenDraftSession({ + providerID: "provider", + modelID: "model", + }) + + expect(createSessionCalls[0]?.directory).toBe("/requested/worktree") + expect(result?.directory).toBe("/canonical/worktree") + expect(useSessionUIStore.getState().currentSessionDirectory).toBe("/canonical/worktree") + }) + + test("routes the session by the canonical directory, not the requested worktree path", async () => { + createdSessionDirectory = "/canonical/worktree" + useSessionUIStore.getState().openNewSessionDraft({ + directoryOverride: "/requested/worktree", + }) + + const created = await materializeOpenDraftSession({ + providerID: "provider", + modelID: "model", + }) + const sessionId = created?.sessionId ?? "" + + // The worktree attachment still holds the path this client asked for. The + // directory every send, queue key, and confirmation lookup is routed by + // must be the canonical one the server returned. + useSessionUIStore.getState().setWorktreeMetadata(sessionId, { + path: "/requested/worktree", + projectDirectory: "/repo", + branch: "feature", + label: "feature", + }) + + expect(useSessionUIStore.getState().getDirectoryForSession(sessionId)).toBe("/canonical/worktree") + }) }) diff --git a/packages/ui/src/sync/send-failure-log.ts b/packages/ui/src/sync/send-failure-log.ts new file mode 100644 index 00000000..5bbac589 --- /dev/null +++ b/packages/ui/src/sync/send-failure-log.ts @@ -0,0 +1,49 @@ +/** + * Recent prompt-send failures, kept in memory for diagnostics. + * + * A rejected send rolls the optimistic message back and, for transport-level + * failures, the composer stays silent by design. That makes a misrouted or + * refused prompt indistinguishable from "nothing happened" — the user has + * nothing to report beyond "it disappeared". + * + * This buffer gives the failure somewhere to live until someone asks for it, + * via the About dialog's diagnostics report or `__opencodeDebug`. It is + * in-memory only: never persisted, never sent anywhere, and dropped on reload. + */ + +const MAX_RECORDED_SEND_FAILURES = 20 +const MAX_REASON_LENGTH = 200 + +export type SendFailureRecord = { + at: number + sessionId: string + messageId: string + /** Directory the prompt was routed to — the value under suspicion. */ + directory: string | null + /** HTTP status, or null for a transport failure with no response. */ + status: number | null + /** Whether the send may still have been accepted server-side. */ + ambiguous: boolean + /** Whether a confirmation refetch ran and failed to find the message. */ + confirmationChecked: boolean + reason: string +} + +const records: SendFailureRecord[] = [] + +export function recordSendFailure(record: Omit & { reason: string }): void { + records.push({ + ...record, + reason: record.reason.slice(0, MAX_REASON_LENGTH), + at: Date.now(), + }) + if (records.length > MAX_RECORDED_SEND_FAILURES) { + records.splice(0, records.length - MAX_RECORDED_SEND_FAILURES) + } +} + +/** Newest first. */ +export function getRecentSendFailures(): SendFailureRecord[] { + return [...records].reverse() +} + diff --git a/packages/ui/src/sync/session-actions.test.ts b/packages/ui/src/sync/session-actions.test.ts index 85da9ffc..c9c4325a 100644 --- a/packages/ui/src/sync/session-actions.test.ts +++ b/packages/ui/src/sync/session-actions.test.ts @@ -245,6 +245,7 @@ mock.module("./session-deletion-cleanup", () => ({ })) mock.module("./sync-refs", () => ({ + getSyncSessionDirectory: () => null, registerSessionDirectory: (sessionID: string, directory: string) => { registeredSessionDirectories.push({ sessionID, directory }) }, diff --git a/packages/ui/src/sync/session-actions.ts b/packages/ui/src/sync/session-actions.ts index e310f3a0..5a6eab26 100644 --- a/packages/ui/src/sync/session-actions.ts +++ b/packages/ui/src/sync/session-actions.ts @@ -13,6 +13,7 @@ import { opencodeClient } from "@/lib/opencode/client" import { mergeSessionDirectoryMetadata, resolveGlobalSessionDirectory, useGlobalSessionsStore } from "@/stores/useGlobalSessionsStore" import { useConfigStore } from "@/stores/useConfigStore" import { registerSessionDirectory } from "./sync-refs" +import { recordSendFailure } from "./send-failure-log" import { isSyntheticPart } from "@/lib/messages/synthetic" import { materializeSessionSnapshots } from "./materialization" import { stripMessageDiffSnapshots, stripSessionDiffSnapshots } from "./sanitize" @@ -1176,7 +1177,9 @@ export async function optimisticSend(input: { try { await input.send(messageID) } catch (error) { - const acceptedRecords = isAmbiguousSendFailure(error) + const status = getErrorStatus(error) + const ambiguousFailure = isAmbiguousSendFailure(error) + const acceptedRecords = ambiguousFailure ? await fetchRecentSendConfirmationRecords(input.sessionId, messageID, targetDirectory) : null @@ -1190,6 +1193,24 @@ export async function optimisticSend(input: { return } + // The rollback below makes the user's message disappear with no other + // trace, and the composer intentionally stays silent for transport-level + // failures. Record the failure so the About dialog's diagnostics report can + // answer "it disappeared and nothing happened" with an actual cause. + // `reason` is truncated by the recorder: a rejected send echoes the + // provider/OpenCode response body, which this log has no reason to keep. + const failureRecord = { + sessionId: input.sessionId, + messageId: messageID, + directory: targetDirectory ?? null, + status, + ambiguous: ambiguousFailure, + confirmationChecked: ambiguousFailure, + reason: error instanceof Error ? error.message : String(error), + } + recordSendFailure(failureRecord) + console.warn("[session-actions] prompt send rejected; rolling back optimistic message", failureRecord) + // Rollback via optimistic infrastructure _optimisticRemove({ sessionID: input.sessionId, diff --git a/packages/ui/src/sync/session-directory-resolution.test.ts b/packages/ui/src/sync/session-directory-resolution.test.ts new file mode 100644 index 00000000..c901ca30 --- /dev/null +++ b/packages/ui/src/sync/session-directory-resolution.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, test } from 'bun:test'; + +import { + describeSessionDirectorySources, + resolveSessionDirectoryFromSources, +} from './session-directory-resolution'; + +const WORKTREE = '/repo/.worktrees/feature'; +const MAIN = '/repo'; + +describe('resolveSessionDirectoryFromSources', () => { + test('authoritative directory beats a selection-time fallback', () => { + const resolution = resolveSessionDirectoryFromSources({ + authoritative: WORKTREE, + selected: MAIN, + }); + + expect(resolution.directory).toBe(WORKTREE); + expect(resolution.source).toBe('authoritative'); + expect(resolution.conflict).toEqual({ source: 'selected', directory: MAIN }); + }); + + test('authoritative directory beats a directory persisted across restarts', () => { + const resolution = resolveSessionDirectoryFromSources({ + authoritative: WORKTREE, + remembered: MAIN, + }); + + expect(resolution.directory).toBe(WORKTREE); + expect(resolution.conflict).toEqual({ source: 'remembered', directory: MAIN }); + }); + + test('the indexed directory outranks a locally requested worktree path', () => { + // attachment/worktreeMetadata hold the path this client asked for, before + // the server canonicalized it. Letting them win would route prompts to a + // directory that no child store owns. + const resolution = resolveSessionDirectoryFromSources({ + attachment: '/requested/worktree', + worktreeMetadata: '/requested/worktree', + authoritative: WORKTREE, + }); + + expect(resolution.directory).toBe(WORKTREE); + expect(resolution.source).toBe('authoritative'); + expect(resolution.conflict).toEqual({ source: 'attachment', directory: '/requested/worktree' }); + }); + + test('a worktree attachment is used while the session is not indexed yet', () => { + // A guessed selection is not passed as `selected` at all, so the worktree + // assignment is the best available value during the bootstrap race. + const resolution = resolveSessionDirectoryFromSources({ + authoritative: null, + selected: null, + attachment: WORKTREE, + remembered: MAIN, + }); + + expect(resolution.directory).toBe(WORKTREE); + expect(resolution.source).toBe('attachment'); + expect(resolution.conflict).toEqual({ source: 'remembered', directory: MAIN }); + }); + + test('a server-confirmed selection outranks the requested worktree path', () => { + const resolution = resolveSessionDirectoryFromSources({ + authoritative: null, + selected: '/canonical/worktree', + attachment: '/requested/worktree', + worktreeMetadata: '/requested/worktree', + }); + + expect(resolution.directory).toBe('/canonical/worktree'); + expect(resolution.source).toBe('selected'); + }); + + test('falls back to the selection hint while the session is not indexed yet', () => { + const resolution = resolveSessionDirectoryFromSources({ + authoritative: null, + selected: WORKTREE, + }); + + expect(resolution.directory).toBe(WORKTREE); + expect(resolution.source).toBe('selected'); + expect(resolution.conflict).toBeNull(); + }); + + test('agreeing sources report no conflict', () => { + const resolution = resolveSessionDirectoryFromSources({ + authoritative: WORKTREE, + selected: WORKTREE, + remembered: WORKTREE, + }); + + expect(resolution.conflict).toBeNull(); + }); + + test('reports the first disagreeing source, not the last', () => { + const resolution = resolveSessionDirectoryFromSources({ + authoritative: WORKTREE, + selected: MAIN, + remembered: '/somewhere/else', + }); + + expect(resolution.conflict).toEqual({ source: 'selected', directory: MAIN }); + }); + + test('treats missing and blank values as unknown, never as a directory', () => { + const resolution = resolveSessionDirectoryFromSources({ + attachment: null, + worktreeMetadata: ' ', + authoritative: undefined, + selected: '', + }); + + expect(resolution.directory).toBeNull(); + expect(resolution.source).toBe('none'); + expect(resolution.conflict).toBeNull(); + }); +}); + +describe('describeSessionDirectorySources', () => { + test('lists populated sources in precedence order', () => { + expect(describeSessionDirectorySources({ + remembered: MAIN, + authoritative: WORKTREE, + selected: '', + })).toEqual([ + { source: 'authoritative', directory: WORKTREE }, + { source: 'remembered', directory: MAIN }, + ]); + }); +}); diff --git a/packages/ui/src/sync/session-directory-resolution.ts b/packages/ui/src/sync/session-directory-resolution.ts new file mode 100644 index 00000000..d8069176 --- /dev/null +++ b/packages/ui/src/sync/session-directory-resolution.ts @@ -0,0 +1,137 @@ +/** + * Session → directory resolution precedence. + * + * A session's directory decides which OpenCode project every send, message + * fetch, queue key, and confirmation lookup is routed to. Getting it wrong is + * not a cosmetic problem: the prompt is posted against a directory that does + * not own the session, the send is rejected, and the optimistic message is + * rolled back with no visible error. + * + * The precedence below is deliberate and ordered by authority, not by + * convenience: + * + * The ordering discriminator is **whether the server confirmed the path**, not + * whether the value is local or synced: + * + * 1. `authoritative` — the child store that actually holds the session, then + * the session's own record. Server-backed truth for an indexed session. + * 2. `selected` — the directory captured when the session was selected, but + * only when it came from a server response (the directory `createSession` + * returned, which may be a canonicalized form of what was requested). A + * selection that fell back to the active directory is a guess and is not + * passed here at all. + * 3. `attachment` / `worktreeMetadata` — the worktree this client assigned to + * the session. Both hold the *requested* path, before the server had a + * chance to canonicalize it, so they are a hint for a session sync has not + * indexed yet, never a correction of a confirmed one. + * 4. `remembered` — the per-runtime directory persisted across restarts. Last + * resort: it survives reloads, so a value written from a startup fallback + * would otherwise outlive the race that produced it. + * + * Routing a prompt by an unconfirmed path posts it against a directory that + * does not own the session, and the send is rejected. Moves need no exception: + * a session move updates the owning child store before any client-side value. + */ + +export type SessionDirectorySource = + | 'authoritative' + | 'selected' + | 'attachment' + | 'worktree-metadata' + | 'remembered' + | 'none' + +export type SessionDirectorySources = { + /** Directory of the child store that holds the session, or its own record. */ + authoritative?: string | null + /** Server-confirmed directory captured at selection. Never a guessed one. */ + selected?: string | null + /** Worktree attachment recorded for this session; the requested path. */ + attachment?: string | null + /** Worktree metadata captured when the session was created in a worktree. */ + worktreeMetadata?: string | null + /** Directory persisted for this runtime; may outlive the race that wrote it. */ + remembered?: string | null +} + +export type SessionDirectoryResolution = { + directory: string | null + source: SessionDirectorySource + /** + * Set when a lower-priority source disagrees with the winning one. This is + * the signature of the stale-directory bug: a persisted or selection-time + * fallback pointing at the parent repository while the session lives in a + * worktree. + */ + conflict: { source: SessionDirectorySource; directory: string } | null +} + +const RESOLUTION_ORDER: ReadonlyArray> = [ + 'authoritative', + 'selected', + 'attachment', + 'worktree-metadata', + 'remembered', +] + +const readSource = ( + sources: SessionDirectorySources, + source: Exclude, +): string | null => { + const value = source === 'attachment' + ? sources.attachment + : source === 'worktree-metadata' + ? sources.worktreeMetadata + : source === 'authoritative' + ? sources.authoritative + : source === 'selected' + ? sources.selected + : sources.remembered + if (typeof value !== 'string') return null + const trimmed = value.trim() + return trimmed.length > 0 ? trimmed : null +} + +/** + * Resolve a session directory from every known source, reporting which source + * won and whether a weaker source disagreed. + * + * Callers normalize paths before passing them in; this module only orders + * authority and never rewrites a path. + */ +export const resolveSessionDirectoryFromSources = ( + sources: SessionDirectorySources, +): SessionDirectoryResolution => { + let winner: { source: SessionDirectorySource; directory: string } | null = null + let conflict: { source: SessionDirectorySource; directory: string } | null = null + + for (const source of RESOLUTION_ORDER) { + const directory = readSource(sources, source) + if (!directory) continue + if (!winner) { + winner = { source, directory } + continue + } + if (!conflict && directory !== winner.directory) { + conflict = { source, directory } + } + } + + if (!winner) { + return { directory: null, source: 'none', conflict: null } + } + + return { directory: winner.directory, source: winner.source, conflict } +} + +/** Every source that carries a value, in precedence order. For diagnostics. */ +export const describeSessionDirectorySources = ( + sources: SessionDirectorySources, +): Array<{ source: SessionDirectorySource; directory: string }> => { + const described: Array<{ source: SessionDirectorySource; directory: string }> = [] + for (const source of RESOLUTION_ORDER) { + const directory = readSource(sources, source) + if (directory) described.push({ source, directory }) + } + return described +} diff --git a/packages/ui/src/sync/session-ui-store.ts b/packages/ui/src/sync/session-ui-store.ts index 33618f71..4f65e98b 100644 --- a/packages/ui/src/sync/session-ui-store.ts +++ b/packages/ui/src/sync/session-ui-store.ts @@ -40,7 +40,13 @@ import { getSyncMessages, getSyncParts, getDirectoryState, + getSyncSessionDirectory, } from "./sync-refs" +import { + resolveSessionDirectoryFromSources, + type SessionDirectoryResolution, + type SessionDirectorySources, +} from "./session-directory-resolution" import { markSessionViewed } from "./notification-store" import { setActiveSession } from "./sync-context" import { @@ -73,7 +79,7 @@ import { useSessionWorktreeStore } from "./session-worktree-store" import { getAttachedSessionDirectory } from "./session-worktree-contract" import { setSessionOpener } from "./session-navigation" import { getRuntimeKey } from "@/lib/runtime-switch" -import { clearLastActiveSession, persistLastActiveSession } from "./last-session-cache" +import { clearLastActiveSession, persistLastActiveSession, readLastActiveSession } from "./last-session-cache" import { persistWorktreeTopology, readPersistedWorktreeTopology } from "./worktree-topology-cache" import { rememberRuntimeLiveStatus } from "./runtime-live-memory" @@ -394,23 +400,108 @@ const getAttachmentForSession = (sessionId: string | null | undefined): SessionW return useSessionWorktreeStore.getState().getAttachment(sessionId) } +/** + * Authoritative directory for a session: the child store that holds it, and + * only then the session record's own fields. `null` means "not indexed yet", + * never "no directory" — callers must fall back rather than treat it as empty. + */ +const getAuthoritativeSessionDirectory = (sessionId: string): string | null => { + const owningDirectory = getSyncSessionDirectory(sessionId) + if (owningDirectory) return normalizePath(owningDirectory) + const target = getAllSyncSessions().find((s) => s.id === sessionId) + return target ? resolveDirectoryKey(target) : null +} + +/** + * Directory remembered for a session in this runtime, plus the one persisted + * across restarts. Exported for diagnostics: a stale persisted directory is the + * hardest source to observe and the one that survives reloads, so a report that + * cannot show it cannot rule it out. + */ +export const getRememberedSessionDirectory = (sessionId: string): { + runtime: string | null + persisted: string | null +} => { + const key = runtimeMemoryKey() + const runtimeMemory = runtimeSessionMemory.get(key) + const persisted = readLastActiveSession(key) + return { + runtime: runtimeMemory?.sessionId === sessionId ? normalizePath(runtimeMemory.directory) : null, + persisted: persisted?.sessionId === sessionId ? normalizePath(persisted.directory) : null, + } +} + +/** + * Session whose `currentSessionDirectory` is only the active directory, used + * because the session's own directory was not known at selection time. Such a + * value must never outrank a worktree assignment or reach persistence — it is + * a guess, not a selection. + */ +let guessedSelectionSessionId: string | null = null + +const collectSessionDirectorySources = ( + sessionId: string, + getWtMeta: (id: string) => WorktreeMetadata | undefined, + selected: string | null, +): SessionDirectorySources => ({ + authoritative: getAuthoritativeSessionDirectory(sessionId), + selected: sessionId === guessedSelectionSessionId ? null : normalizePath(selected), + attachment: getAttachedSessionDirectory(getAttachmentForSession(sessionId)), + worktreeMetadata: normalizePath(getWtMeta(sessionId)?.path ?? null), + remembered: getRememberedSessionDirectory(sessionId).runtime, +}) + +/** + * Conflicts already warned about, so a stale directory logs once instead of on + * every keystroke. Keyed by runtime *and* the exact pair of directories: the + * same session ID means a different thing in another runtime, and a conflict + * that reappears after being resolved is news worth logging again. Bounded so + * a long-lived session cannot grow it without limit. + */ +const reportedDirectoryConflicts = new Set() +const MAX_REPORTED_DIRECTORY_CONFLICTS = 200 + +const reportSessionDirectoryConflict = ( + sessionId: string, + resolution: SessionDirectoryResolution, +): void => { + if (!resolution.conflict) return + const conflictKey = JSON.stringify([ + runtimeMemoryKey(), + sessionId, + resolution.directory, + resolution.conflict.source, + resolution.conflict.directory, + ]) + if (reportedDirectoryConflicts.has(conflictKey)) return + if (reportedDirectoryConflicts.size >= MAX_REPORTED_DIRECTORY_CONFLICTS) { + reportedDirectoryConflicts.clear() + } + reportedDirectoryConflicts.add(conflictKey) + console.warn( + "[session-directory] session directory sources disagree; using the higher-authority one. " + + "Run __opencodeDebug.diagnoseSessionDirectory() for the full picture.", + { + sessionId, + using: resolution.source, + directory: resolution.directory, + conflictingSource: resolution.conflict.source, + conflictingDirectory: resolution.conflict.directory, + }, + ) +} + const resolveSessionDirectory = ( sessionId: string | null | undefined, getWtMeta: (id: string) => WorktreeMetadata | undefined, + selected: string | null = null, ): string | null => { if (!sessionId) return null - const attachmentDirectory = getAttachedSessionDirectory(getAttachmentForSession(sessionId)) - if (attachmentDirectory) return attachmentDirectory - const metaPath = getWtMeta(sessionId)?.path - if (typeof metaPath === "string" && metaPath.trim().length > 0) return normalizePath(metaPath) - const runtimeMemory = runtimeSessionMemory.get(runtimeMemoryKey()) - if (runtimeMemory?.sessionId === sessionId && runtimeMemory.directory) { - return normalizePath(runtimeMemory.directory) - } - const sessions = getAllSyncSessions() - const target = sessions.find((s) => s.id === sessionId) - if (!target) return null - return resolveDirectoryKey(target) + const resolution = resolveSessionDirectoryFromSources( + collectSessionDirectorySources(sessionId, getWtMeta, selected), + ) + reportSessionDirectoryConflict(sessionId, resolution) + return resolution.directory } const activateConfigForDirectory = async (directory: string | null | undefined): Promise => { @@ -504,13 +595,18 @@ export async function materializeOpenDraftSession(selection: { const created = await store.createSession(draft.title, draftDirectoryOverride, draft.parentID ?? null) if (!created?.id) throw new Error("Failed to create session") + // The server response is authoritative. It may canonicalize a requested + // worktree path (for example through a symlink or platform path casing). + // Sending with the pre-canonical draft path can target a different + // directory scope than the session that was just created. + const createdDirectory = normalizePath(created.directory ?? draftDirectoryOverride ?? null) + persistDraftTarget({ projectId: draftProjectId, - directory: normalizePath(draftDirectoryOverride ?? created.directory ?? null), + directory: createdDirectory, }) const draftSyntheticParts = draft.syntheticParts - const createdDirectory = normalizePath(draftDirectoryOverride ?? created.directory ?? null) const configState = useConfigStore.getState() void activateConfigForDirectory(createdDirectory).catch((error) => { console.warn("Failed to activate directory after creating session:", error) @@ -604,7 +700,13 @@ export const useSessionUIStore = create()((set, get) => ({ (sid) => get().worktreeMetadata.get(sid), ) const fallbackDir = opencodeClient.getDirectory() ?? directoryState.currentDirectory ?? null - const resolvedDir = (directoryHint ? normalizePath(directoryHint) : null) ?? sessionDir ?? fallbackDir + const knownDir = (directoryHint ? normalizePath(directoryHint) : null) ?? sessionDir + const resolvedDir = knownDir ?? fallbackDir + // `fallbackDir` is the active directory, not this session's directory. It + // keeps routing usable while the owning directory store bootstraps, but it + // must never be remembered: a persisted guess outlives the race that + // produced it and survives reloads and restarts. + const isGuessedDir = knownDir === null const projectsState = useProjectsStore.getState() const sessionProject = resolvedDir ? resolveProjectForSessionDirectory( @@ -617,12 +719,14 @@ export const useSessionUIStore = create()((set, get) => ({ // Set the directory together with the session id so chat hooks read the // same child store that send/SSE events will update during startup races. set({ currentSessionId: id, currentSessionDirectory: id ? resolvedDir ?? null : null }) - writeRuntimeSessionMemory(key, { sessionId: id, directory: resolvedDir ?? null }) + guessedSelectionSessionId = isGuessedDir && id ? id : null + const rememberedDir = isGuessedDir ? null : resolvedDir ?? null + writeRuntimeSessionMemory(key, { sessionId: id, directory: rememberedDir }) // Keep the last NON-null session per runtime across app restarts (cold // mobile launches reopen it after the instance reconnects). Going back to // a draft intentionally does not erase it. if (id) { - persistLastActiveSession(key, { sessionId: id, directory: resolvedDir ?? null }) + persistLastActiveSession(key, { sessionId: id, directory: rememberedDir }) } // Kick off the message fetch on the same tick, before React commits the @@ -1560,16 +1664,19 @@ export const useSessionUIStore = create()((set, get) => ({ }, getDirectoryForSession: (sessionId) => { - if (sessionId === get().currentSessionId && get().currentSessionDirectory) { - return get().currentSessionDirectory - } - const resolved = resolveSessionDirectory(sessionId, (sid) => get().worktreeMetadata.get(sid)) + // The selection-time directory participates in resolution, it does not + // short-circuit it. For a worktree session selected before its directory + // store finished bootstrapping, that value is a startup fallback pointing + // at the parent repository; letting it win would route every send, queue + // key, and send-confirmation lookup to a directory that does not own the + // session. + const selected = sessionId === get().currentSessionId ? get().currentSessionDirectory : null + const resolved = resolveSessionDirectory( + sessionId, + (sid) => get().worktreeMetadata.get(sid), + selected, + ) if (resolved) return resolved - const attachmentDirectory = getAttachedSessionDirectory(getAttachmentForSession(sessionId)) - if (attachmentDirectory) return attachmentDirectory - const sessions = getAllSyncSessions() - const session = sessions.find((s) => s.id === sessionId) - if (session) return resolveDirectoryKey(session) const globalStore = useGlobalSessionsStore.getState() const globalSession = [...globalStore.activeSessions, ...globalStore.archivedSessions] .find((s) => s.id === sessionId) @@ -1634,6 +1741,11 @@ export const useSessionUIStore = create()((set, get) => ({ setSessionDirectory: (sessionId, directory) => { const normalized = normalizePath(directory) + // Callers set this from a confirmed destination (a completed move, a + // created worktree), so the selection is no longer a guess. + if (sessionId === guessedSelectionSessionId) { + guessedSelectionSessionId = null + } if (sessionId === get().currentSessionId) { set({ currentSessionDirectory: normalized }) writeRuntimeSessionMemory(runtimeMemoryKey(), { sessionId, directory: normalized }) diff --git a/packages/ui/src/sync/sync-refs.ts b/packages/ui/src/sync/sync-refs.ts index 9b15f0d3..0d96911c 100644 --- a/packages/ui/src/sync/sync-refs.ts +++ b/packages/ui/src/sync/sync-refs.ts @@ -17,6 +17,7 @@ const configListeners = new Set<(directory: string, config: Config) => void>() let cachedSessionManager: ChildStoreManager | null = null let cachedSessionSlices = new Map() let cachedSessionsById = new Map() +let cachedSessionDirectoryById = new Map() export function setSyncRefs( _sdk: OpencodeClient, @@ -29,6 +30,7 @@ export function setSyncRefs( cachedSessionManager = null cachedSessionSlices = new Map() cachedSessionsById = new Map() + cachedSessionDirectoryById = new Map() } _directory = directory if (registerSessionDirectory) { @@ -103,20 +105,38 @@ export function getAllSyncSessionMap(): ReadonlyMap() const nextSessionsById = new Map() + const nextDirectoriesById = new Map() for (const [directory, store] of stores.children) { const sessions = store.getState().session nextSlices.set(directory, sessions) for (const session of sessions) { if (!session?.id) continue nextSessionsById.set(session.id, session) + nextDirectoriesById.set(session.id, directory) } } cachedSessionManager = stores cachedSessionSlices = nextSlices cachedSessionsById = nextSessionsById + cachedSessionDirectoryById = nextDirectoriesById return cachedSessionsById } +/** + * Directory of the child store that actually holds this session. + * + * This is the authoritative session→directory mapping: a session is present in + * exactly the store for the directory it belongs to, regardless of whether the + * server populated `session.directory` on the record itself. Returns `null` + * when no initialized child store contains the session, which means "unknown", + * never "no directory". + */ +export function getSyncSessionDirectory(sessionId: string): string | null { + if (!sessionId) return null + getAllSyncSessionMap() + return cachedSessionDirectoryById.get(sessionId) ?? null +} + /** Read messages for a session from current directory's child store */ export function getSyncMessages(sessionId: string, directory?: string) { return getDirectoryState(directory)?.message[sessionId] ?? [] diff --git a/packages/web/server/lib/opencode/DOCUMENTATION.md b/packages/web/server/lib/opencode/DOCUMENTATION.md index bb9fcdfc..7a389352 100644 --- a/packages/web/server/lib/opencode/DOCUMENTATION.md +++ b/packages/web/server/lib/opencode/DOCUMENTATION.md @@ -355,6 +355,10 @@ an authoritative loopback callback URL even when OpenChamber binds port `0`. - Skills config CRUD and metadata under `/api/config/skills*` - Skills catalog listing/source pagination, scan, and install routes - Supporting skill file read/write/delete routes + - Directory resolution prefers an explicit request directory, then soft-falls + back to the active project / `lastDirectory` so repository-local + `.agents/skills` and `.opencode/skills` remain discoverable when the client + omits `directory`. Requests without any project still list user-scoped skills. ## Public exports (proxy.js) - `registerOpenCodeProxy(app, dependencies)`: registers OpenCode proxy routes and middleware. diff --git a/packages/web/server/lib/opencode/skill-routes.js b/packages/web/server/lib/opencode/skill-routes.js index b66680a5..ad8dae59 100644 --- a/packages/web/server/lib/opencode/skill-routes.js +++ b/packages/web/server/lib/opencode/skill-routes.js @@ -200,9 +200,33 @@ export const registerSkillRoutes = (app, dependencies) => { return null; }; + // Prefer an explicit request directory, then soft-fallback to the active + // project / lastDirectory so repository-local skills stay visible when the + // client omits `directory` (create already used resolveProjectDirectory). + const resolveSkillsDirectory = async (req) => { + const optional = await resolveOptionalProjectDirectory(req); + if (optional.error) { + return optional; + } + if (optional.directory) { + return optional; + } + + try { + const fallback = await resolveProjectDirectory(req); + if (fallback.directory) { + return { directory: fallback.directory, error: null }; + } + } catch { + // ignore — listing user-scoped skills without a project is valid + } + + return { directory: null, error: null }; + }; + app.get('/api/config/skills', async (req, res) => { try { - const { directory, error } = await resolveOptionalProjectDirectory(req); + const { directory, error } = await resolveSkillsDirectory(req); if (error) { return res.status(400).json({ error }); } @@ -257,7 +281,7 @@ export const registerSkillRoutes = (app, dependencies) => { app.get('/api/config/skills/catalog/source', async (req, res) => { try { - const { directory, error } = await resolveOptionalProjectDirectory(req); + const { directory, error } = await resolveSkillsDirectory(req); if (error) { return res.status(400).json({ ok: false, error: { kind: 'invalidSource', message: error } }); } @@ -518,7 +542,7 @@ export const registerSkillRoutes = (app, dependencies) => { app.get('/api/config/skills/:name', async (req, res) => { try { const skillName = req.params.name; - const { directory, error } = await resolveOptionalProjectDirectory(req); + const { directory, error } = await resolveSkillsDirectory(req); if (error) { return res.status(400).json({ error }); } @@ -546,7 +570,7 @@ export const registerSkillRoutes = (app, dependencies) => { if (isUnsafeSkillRelativePath(filePath)) { return res.status(400).json({ error: 'Invalid file path' }); } - const { directory, error } = await resolveOptionalProjectDirectory(req); + const { directory, error } = await resolveSkillsDirectory(req); if (error) { return res.status(400).json({ error }); } @@ -579,7 +603,7 @@ export const registerSkillRoutes = (app, dependencies) => { const { scope, source: skillSource, ...config } = req.body; const { directory, error } = scope === SKILL_SCOPE.PROJECT ? await resolveProjectDirectory(req) - : await resolveOptionalProjectDirectory(req); + : await resolveSkillsDirectory(req); if (error || (scope === SKILL_SCOPE.PROJECT && !directory)) { return res.status(400).json({ error: error || 'Project skill creation requires a directory' }); } @@ -606,7 +630,7 @@ export const registerSkillRoutes = (app, dependencies) => { try { const skillName = req.params.name; const updates = req.body; - const { directory, error } = await resolveOptionalProjectDirectory(req); + const { directory, error } = await resolveSkillsDirectory(req); if (error) { return res.status(400).json({ error }); } @@ -637,7 +661,7 @@ export const registerSkillRoutes = (app, dependencies) => { return res.status(400).json({ error: 'Invalid file path' }); } const { content } = req.body; - const { directory, error } = await resolveOptionalProjectDirectory(req); + const { directory, error } = await resolveSkillsDirectory(req); if (error) { return res.status(400).json({ error }); } @@ -671,7 +695,7 @@ export const registerSkillRoutes = (app, dependencies) => { if (isUnsafeSkillRelativePath(filePath)) { return res.status(400).json({ error: 'Invalid file path' }); } - const { directory, error } = await resolveOptionalProjectDirectory(req); + const { directory, error } = await resolveSkillsDirectory(req); if (error) { return res.status(400).json({ error }); } @@ -701,7 +725,7 @@ export const registerSkillRoutes = (app, dependencies) => { app.delete('/api/config/skills/:name', async (req, res) => { try { const skillName = req.params.name; - const { directory, error } = await resolveOptionalProjectDirectory(req); + const { directory, error } = await resolveSkillsDirectory(req); if (error) { return res.status(400).json({ error }); } diff --git a/packages/web/server/lib/opencode/skill-routes.test.js b/packages/web/server/lib/opencode/skill-routes.test.js new file mode 100644 index 00000000..c83f7f84 --- /dev/null +++ b/packages/web/server/lib/opencode/skill-routes.test.js @@ -0,0 +1,157 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import express from 'express'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { registerSkillRoutes } from './skill-routes.js'; +import { + createSkill, + deleteSkill, + discoverSkills, + getSkillSources, + mergeDiscoveredSkills, + updateSkill, +} from './skills.js'; +import { + SKILL_DIR, + SKILL_SCOPE, + deleteSkillSupportingFile, + readSkillSupportingFile, + writeSkillSupportingFile, +} from './shared.js'; + +const createTempProject = () => { + const projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'oc-skill-routes-')); + fs.mkdirSync(path.join(projectRoot, '.git')); + return projectRoot; +}; + +const startSkillsApp = ({ projectRoot }) => { + const app = express(); + app.use(express.json()); + + registerSkillRoutes(app, { + fs, + path, + os, + resolveProjectDirectory: async () => ({ directory: projectRoot, error: null }), + resolveOptionalProjectDirectory: async (req) => { + const queryDirectory = Array.isArray(req.query?.directory) + ? req.query.directory[0] + : req.query?.directory; + if (!queryDirectory) { + return { directory: null, error: null }; + } + return { directory: String(queryDirectory), error: null }; + }, + readSettingsFromDisk: async () => ({}), + sanitizeSkillCatalogs: (value) => value, + isUnsafeSkillRelativePath: () => false, + refreshOpenCodeAfterConfigChange: async () => {}, + clientReloadDelayMs: 0, + buildOpenCodeUrl: () => 'http://127.0.0.1:9/', + getOpenCodeAuthHeaders: () => ({}), + getOpenCodePort: () => 0, + getSkillSources, + discoverSkills, + mergeDiscoveredSkills, + createSkill, + updateSkill, + deleteSkill, + readSkillSupportingFile, + writeSkillSupportingFile, + deleteSkillSupportingFile, + SKILL_SCOPE, + SKILL_DIR, + getCuratedSkillsSources: () => [], + getCacheKey: () => 'k', + getCachedScan: () => null, + setCachedScan: () => {}, + parseSkillRepoSource: () => ({ ok: false }), + scanSkillsRepository: async () => ({ ok: false }), + installSkillsFromRepository: async () => ({ ok: false }), + scanClawdHubPage: async () => ({ ok: false }), + installSkillsFromClawdHub: async () => ({ ok: false }), + isClawdHubSource: () => false, + getProfiles: () => [], + getProfile: () => null, + }); + + const server = app.listen(0); + const { port } = server.address(); + return { + baseUrl: `http://127.0.0.1:${port}`, + close: () => new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }), + }; +}; + +describe('skill-routes directory soft fallback', () => { + /** @type {string | null} */ + let projectRoot = null; + /** @type {{ close: () => Promise } | null} */ + let appHandle = null; + + afterEach(async () => { + if (appHandle) { + await appHandle.close(); + appHandle = null; + } + if (projectRoot) { + fs.rmSync(projectRoot, { recursive: true, force: true }); + projectRoot = null; + } + }); + + it('lists repository-local .agents skills after create even when list omits directory', async () => { + projectRoot = createTempProject(); + appHandle = startSkillsApp({ projectRoot }); + + const createResponse = await fetch(`${appHandle.baseUrl}/api/config/skills/repo-local-skill`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + description: 'Created without list directory', + instructions: 'Do the thing.', + scope: 'project', + source: 'agents', + }), + }); + expect(createResponse.status).toBe(200); + expect(fs.existsSync(path.join(projectRoot, '.agents', 'skills', 'repo-local-skill', 'SKILL.md'))).toBe(true); + + const listResponse = await fetch(`${appHandle.baseUrl}/api/config/skills`); + expect(listResponse.status).toBe(200); + const payload = await listResponse.json(); + expect(payload.skills.map((skill) => skill.name)).toContain('repo-local-skill'); + const skill = payload.skills.find((entry) => entry.name === 'repo-local-skill'); + expect(skill.scope).toBe('project'); + expect(skill.source).toBe('agents'); + }); + + it('lists manually created repository-local .agents skills via active-project fallback', async () => { + projectRoot = createTempProject(); + const skillDir = path.join(projectRoot, '.agents', 'skills', 'manual-repo-skill'); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync( + path.join(skillDir, 'SKILL.md'), + [ + '---', + 'name: manual-repo-skill', + 'description: Manual repository skill', + '---', + '', + 'Instructions', + '', + ].join('\n'), + 'utf8', + ); + + appHandle = startSkillsApp({ projectRoot }); + const listResponse = await fetch(`${appHandle.baseUrl}/api/config/skills`); + expect(listResponse.status).toBe(200); + const payload = await listResponse.json(); + expect(payload.skills.map((skill) => skill.name)).toContain('manual-repo-skill'); + }); +}); diff --git a/packages/web/server/lib/opencode/skills.test.js b/packages/web/server/lib/opencode/skills.test.js index 95f9c722..ffa3a6f2 100644 --- a/packages/web/server/lib/opencode/skills.test.js +++ b/packages/web/server/lib/opencode/skills.test.js @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest'; import fsPromises from 'fs/promises'; import os from 'os'; import path from 'path'; -import { getSkillSources, mergeDiscoveredSkills } from './skills.js'; +import { discoverSkills, getSkillSources, mergeDiscoveredSkills } from './skills.js'; describe('skills', () => { it('merges locally discovered skills missing from OpenCode live discovery', () => { @@ -24,6 +24,43 @@ describe('skills', () => { ]); }); + it('discovers repository-local .agents skills for the project directory', async () => { + const tempRoot = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'oc-project-agents-')); + const skillDir = path.join(tempRoot, '.agents', 'skills', 'repo-local-skill'); + const skillPath = path.join(skillDir, 'SKILL.md'); + + try { + await fsPromises.mkdir(skillDir, { recursive: true }); + await fsPromises.mkdir(path.join(tempRoot, '.git')); + await fsPromises.writeFile( + skillPath, + [ + '---', + 'name: repo-local-skill', + 'description: Repository-local agents skill', + '---', + '', + 'Use this skill in this repository.', + '', + ].join('\n'), + 'utf8', + ); + + const discovered = discoverSkills(tempRoot); + const match = discovered.find((skill) => skill.name === 'repo-local-skill'); + + expect(match).toEqual({ + name: 'repo-local-skill', + path: skillPath, + scope: 'project', + source: 'agents', + description: 'Repository-local agents skill', + }); + } finally { + await fsPromises.rm(tempRoot, { recursive: true, force: true }); + } + }); + it('resolves built-in OpenCode skill content without parsing virtual locations as files', () => { const sources = getSkillSources( 'customize-opencode', diff --git a/packages/web/server/lib/terminal/DOCUMENTATION.md b/packages/web/server/lib/terminal/DOCUMENTATION.md index 40196998..33fd9e54 100644 --- a/packages/web/server/lib/terminal/DOCUMENTATION.md +++ b/packages/web/server/lib/terminal/DOCUMENTATION.md @@ -10,11 +10,12 @@ - `attach` registers a connection for one terminal. One socket may attach to many terminals. - Every attach and reconnect begins with an authoritative `snapshot` containing bounded history and the current sequence. +- A current socket that closes or errors before its initial `open` invalidates its URL-scoped auth token before retrying, so retries mint a fresh token instead of backing off against a rejected upgrade. Hidden or offline clients wait 60 seconds and wake promptly on visibility/online recovery. - `output`, `exit`, and `restarted` carry monotonically increasing per-terminal sequences. Output carries raw live bytes plus replay-safe bytes with terminal query exchanges removed. - Attach registers before capturing the snapshot, buffers concurrent events, drops events represented by the snapshot sequence, then enters live delivery. - `write` always includes the terminal ID; sockets never have mutable single-terminal binding state. - `detach` removes only that attachment. -- Creation carries the active UI appearance. The PTY sets `COLORFGBG` and answers OSC 10, OSC 11, and Mode 2031 queries immediately, including queries emitted before a WebSocket attachment exists. Subscribed TUIs receive a Mode 2031 notification when the appearance changes. +- Creation carries the active UI appearance. The PTY sets `COLORFGBG` and answers OSC 10, OSC 11, Mode 2031, and primary-device-attribute queries immediately, including queries emitted before a WebSocket attachment exists. The DA1 fallback prevents Fish from waiting ten seconds for a renderer that cannot observe or answer its startup query. Subscribed TUIs receive a Mode 2031 notification when the appearance changes. HTTP remains the authenticated command plane for create, resize, appearance updates, restart, close, and force-kill. There is no SSE output or HTTP input compatibility path. @@ -23,6 +24,7 @@ HTTP remains the authenticated command plane for create, resize, appearance upda - IDs are client-provided or generated with `randomUUID()`. - Concurrent creates for one ID are single-flight only when working directory and shell preference match. Existing IDs cannot be reused for another working directory. - Dimensions are bounded to 1-1000 columns and 1-500 rows; input is capped at 64 KiB. +- A client may create before its renderer has mounted. It derives an initial size from the container and font metrics (falling back to 80x24 when unavailable), then sends a resize once Ghostty reports its final dimensions. This allows shell startup and renderer initialization to overlap. - PTY children explicitly clear `NODE_CHANNEL_FD`; daemon IPC descriptors are host-private and invalid after PTY descriptor cleanup. - PTY children also strip AppImage `ARGV0` (and other host-private shell vars such as `ELECTRON_RUN_AS_NODE`, `BASH_ENV`, `ENV`, `BASH_XTRACEFD`). An exported `ARGV0` makes zsh rewrite argv[0] for every external command, which breaks Python venv detection and other argv[0]/$0 consumers while leaving `/proc/self/exe` correct. On Linux, PTY spawn is wrapped with `env -u ARGV0` because `bun-pty` merges the native OS environ and would otherwise reintroduce `ARGV0` after a JS-only delete. - `GET /api/terminal/shells` reports shell IDs available on the active server using the same augmented PATH provided to spawned PTYs, plus whether each executable has a supported login-mode argument. `auto` preserves environment/platform fallback order; an explicit unavailable shell fails creation instead of silently running a different shell. Login mode is opt-in and uses only built-in arguments for known shells. Preference changes affect new sessions and explicit restarts, not running PTYs. diff --git a/packages/web/server/lib/terminal/runtime.js b/packages/web/server/lib/terminal/runtime.js index 9aa328d4..5e90ac93 100644 --- a/packages/web/server/lib/terminal/runtime.js +++ b/packages/web/server/lib/terminal/runtime.js @@ -150,7 +150,7 @@ export function createTerminalRuntime({ background: session.terminalBackground, foreground: session.terminalForeground, modeEnabled: session.themeModeEnabled, - }); + }, { respondToPrimaryDeviceAttributes: true }); session.pendingThemeControlSequence = theme.pending; session.themeModeEnabled = theme.modeEnabled; for (const response of theme.responses) session.process?.write(response); @@ -336,7 +336,7 @@ export function createTerminalRuntime({ session.process = spawned.process; session.backend = spawned.backend; session.shell = spawned.shell; session.loginShell = spawned.loginShell; session.cwd = cwd; session.cols = cols; session.rows = rows; session.history = ''; session.pendingHistoryControlSequence = ''; session.pendingThemeControlSequence = ''; session.themeModeEnabled = false; session.status = 'running'; session.exitCode = null; session.signal = null; session.eventQueue.length = 0; session.themeMode = themeMode === 'light' ? 'light' : 'dark'; session.terminalBackground = terminalBackground; session.terminalForeground = terminalForeground; - wire(session, spawned.process); void terminateProcess(oldProcess); publish(session, { t: 'restarted', history: '' }); + wire(session, spawned.process); void terminateProcess(oldProcess); publish(session, { t: 'restarted', history: '' }); }); pendingSessionRestarts.set(session.id, restart); try { diff --git a/packages/web/server/lib/terminal/runtime.test.js b/packages/web/server/lib/terminal/runtime.test.js index f0e2b622..8232be95 100644 --- a/packages/web/server/lib/terminal/runtime.test.js +++ b/packages/web/server/lib/terminal/runtime.test.js @@ -160,8 +160,8 @@ describe('terminal runtime', () => { expect(harness.processes[0].shell).toMatch(/\/env$/); expect(harness.processes[0].args.slice(0, 3)).toEqual(['-u', 'ARGV0', expect.any(String)]); } - harness.processes[0].emitData('\u001b[?2031h\u001b]10;?\u0007\u001b]11;?\u0007'); - expect(harness.processes[0].writes).toEqual(['\u001b]10;rgb:1b1b/1b1b/1b1b\u001b\\', '\u001b]11;rgb:fafa/f8f8/f0f0\u001b\\']); + harness.processes[0].emitData('\u001b[?2031h\u001b]10;?\u0007\u001b]11;?\u0007\u001b[0c'); + expect(harness.processes[0].writes).toEqual(['\u001b]10;rgb:1b1b/1b1b/1b1b\u001b\\', '\u001b]11;rgb:fafa/f8f8/f0f0\u001b\\', '\u001b[?1;2c']); const appearance = createResponse(); harness.routes.post.get('/api/terminal/:sessionId/appearance')({ params: { sessionId: 'term-1' }, body: { themeMode: 'dark' } }, appearance); diff --git a/packages/web/server/lib/terminal/theme-response.js b/packages/web/server/lib/terminal/theme-response.js index 3738675a..26662610 100644 --- a/packages/web/server/lib/terminal/theme-response.js +++ b/packages/web/server/lib/terminal/theme-response.js @@ -2,11 +2,21 @@ const MODE_SET = '\u001b[?2031h'; const MODE_RESET = '\u001b[?2031l'; const CAPABILITY_QUERY = '\u001b[?2031$p'; const MODE_QUERIES = ['\u001b[?996n', '\u001b[?997n']; +// Fish asks this before an unattached browser terminal can reply. +const PRIMARY_DEVICE_ATTRIBUTE_QUERIES = ['\u001b[c', '\u001b[0c']; +const PRIMARY_DEVICE_ATTRIBUTE_RESPONSE = '\u001b[?1;2c'; const OSC_QUERIES = [10, 11].flatMap((code) => [ { sequence: `\u001b]${code};?\u0007`, code }, { sequence: `\u001b]${code};?\u001b\\`, code }, ]); -const CONTROL_SEQUENCES = [MODE_SET, MODE_RESET, CAPABILITY_QUERY, ...MODE_QUERIES, ...OSC_QUERIES.map(({ sequence }) => sequence)]; +const CONTROL_SEQUENCES = [ + MODE_SET, + MODE_RESET, + CAPABILITY_QUERY, + ...MODE_QUERIES, + ...PRIMARY_DEVICE_ATTRIBUTE_QUERIES, + ...OSC_QUERIES.map(({ sequence }) => sequence), +]; const parseColor = (value) => { if (typeof value !== 'string') return null; @@ -28,7 +38,12 @@ const colorReport = (code, color) => { export const terminalThemeModeReport = (themeMode) => `\u001b[?997;${themeMode === 'light' ? 2 : 1}n`; -export const consumeTerminalThemeQueries = (pending, data, appearance) => { +export const consumeTerminalThemeQueries = ( + pending, + data, + appearance, + { respondToPrimaryDeviceAttributes = false } = {}, +) => { if (!pending && !data.includes('\u001b')) return { pending: '', responses: [], modeEnabled: appearance.modeEnabled === true }; const input = `${pending}${data}`; const responses = []; @@ -56,6 +71,15 @@ export const consumeTerminalThemeQueries = (pending, data, appearance) => { index += modeQuery.length - 1; continue; } + const primaryDeviceAttributeQuery = PRIMARY_DEVICE_ATTRIBUTE_QUERIES.find((query) => input.startsWith(query, index)); + if (primaryDeviceAttributeQuery && respondToPrimaryDeviceAttributes) { + // A shell can ask before any browser terminal is attached. Answer with a + // conservative VT100 DA1 response so Fish does not block startup for its + // ten-second query timeout while waiting for a renderer that cannot see it. + responses.push(PRIMARY_DEVICE_ATTRIBUTE_RESPONSE); + index += primaryDeviceAttributeQuery.length - 1; + continue; + } const oscQuery = OSC_QUERIES.find(({ sequence }) => input.startsWith(sequence, index)); if (oscQuery) { const response = colorReport(oscQuery.code, oscQuery.code === 10 ? appearance.foreground : appearance.background); diff --git a/packages/web/server/lib/terminal/theme-response.test.js b/packages/web/server/lib/terminal/theme-response.test.js index f67fe418..e45fdd9c 100644 --- a/packages/web/server/lib/terminal/theme-response.test.js +++ b/packages/web/server/lib/terminal/theme-response.test.js @@ -44,4 +44,27 @@ describe('terminal theme responses', () => { '\u001b]10;rgb:1b1b/1b1b/1b1b\u001b\\', ]); }); + + test('answers a primary device attribute query when the fallback is enabled', () => { + const attached = consumeTerminalThemeQueries('', '\u001b[0c', lightAppearance); + const unattached = consumeTerminalThemeQueries('', '\u001b[0c', lightAppearance, { + respondToPrimaryDeviceAttributes: true, + }); + + expect(attached.responses).toEqual([]); + expect(unattached.responses).toEqual(['\u001b[?1;2c']); + }); + + test('answers a primary device attribute query split across PTY chunks', () => { + const first = consumeTerminalThemeQueries('', '\u001b[0', lightAppearance, { + respondToPrimaryDeviceAttributes: true, + }); + const second = consumeTerminalThemeQueries(first.pending, 'c', { + ...lightAppearance, + modeEnabled: first.modeEnabled, + }, { respondToPrimaryDeviceAttributes: true }); + + expect(first.pending).toBe('\u001b[0'); + expect(second.responses).toEqual(['\u001b[?1;2c']); + }); });