diff --git a/.changeset/filetree-incremental-paint.md b/.changeset/filetree-incremental-paint.md new file mode 100644 index 000000000..658b8c0a8 --- /dev/null +++ b/.changeset/filetree-incremental-paint.md @@ -0,0 +1,5 @@ +--- +"@inkeep/open-knowledge": patch +--- + +Paint the sidebar file tree incrementally as the directory listing streams in, instead of withholding the whole level until the walk finishes. The `GET /api/documents` NDJSON walk is now applied to the tree per network chunk (additively, so folders not yet streamed are never pruned mid-stream), and the loading skeleton clears on the first batch. The authoritative prune + optimistic-merge reconcile still runs once as a single splice when the stream completes, so the final tree is unchanged. On a knowledge base with a large top-level directory the first rows appear sooner rather than after the entire level is enumerated. diff --git a/.changeset/terminal-launch-no-shell-history.md b/.changeset/terminal-launch-no-shell-history.md new file mode 100644 index 000000000..26619e674 --- /dev/null +++ b/.changeset/terminal-launch-no-shell-history.md @@ -0,0 +1,5 @@ +--- +"@inkeep/open-knowledge": patch +--- + +Desktop: launching an agent CLI from the docked terminal ("Open in Claude/Codex/Cursor/OpenCode") no longer pollutes your shell history. The launch command used to be typed into an interactive shell, so every launch — prompt and all — was recorded in `~/.zsh_history`, cluttering `↑`/`Ctrl-R` and writing document content in plaintext outside `.ok/`. The launch is now baked into the tab's shell spawn (`$SHELL -l -i -c '; exec $SHELL -l -i'`): the command runs without going through the line editor (so it is never recorded), PATH is unchanged (the same login-interactive shell is used), and the tab drops into a fresh interactive shell when the agent exits — so your own later commands still record normally. Applies to all four CLIs. diff --git a/packages/app/src/components/FileTree.showall-lazy.dom.test.tsx b/packages/app/src/components/FileTree.showall-lazy.dom.test.tsx index dc55c3a91..00e189268 100644 --- a/packages/app/src/components/FileTree.showall-lazy.dom.test.tsx +++ b/packages/app/src/components/FileTree.showall-lazy.dom.test.tsx @@ -376,6 +376,81 @@ describe('FileTree showAll lazy root seed', () => { }); }); + test('paints the first NDJSON chunk before the stream completes (incremental seed)', async () => { + let releaseRest: () => void = () => {}; + const gate = new Promise((resolve) => { + releaseRest = resolve; + }); + const encoder = new TextEncoder(); + showAllResponseFactory = () => + new Response( + new ReadableStream({ + async start(controller) { + controller.enqueue(encoder.encode(`${JSON.stringify(folderEntry('team', true))}\n`)); + await gate; + controller.enqueue( + encoder.encode( + `${JSON.stringify(docEntry('README'))}\n${JSON.stringify({ + type: 'complete', + truncated: false, + count: 2, + })}\n`, + ), + ); + controller.close(); + }, + }), + { status: 200, headers: { 'content-type': 'application/x-ndjson' } }, + ); + render(); + + await waitFor(() => expect([...model.items.keys()]).toEqual(['team/'])); + + expect(screen.queryByRole('status')).toBeNull(); + + releaseRest(); + + await waitFor(() => expect([...model.items.keys()].sort()).toEqual(['README.md', 'team/'])); + }); + + test('a first chunk of only hidden entries does not clear the skeleton (no empty-state flash)', async () => { + let releaseVisible: () => void = () => {}; + const gate = new Promise((resolve) => { + releaseVisible = resolve; + }); + const encoder = new TextEncoder(); + showAllResponseFactory = () => + new Response( + new ReadableStream({ + async start(controller) { + controller.enqueue(encoder.encode(`${JSON.stringify(folderEntry('.github', true))}\n`)); + await gate; + controller.enqueue( + encoder.encode( + `${JSON.stringify(folderEntry('docs', true))}\n${JSON.stringify({ + type: 'complete', + truncated: false, + count: 2, + })}\n`, + ), + ); + controller.close(); + }, + }), + { status: 200, headers: { 'content-type': 'application/x-ndjson' } }, + ); + render(); + + await waitFor(() => expect(fetchUrls).toContain(SHOW_ALL_DEPTH1_URL)); + await new Promise((resolve) => setTimeout(resolve, 30)); + expect(screen.queryByText(/No files yet|Create your first/i)).toBeNull(); + expect(screen.queryByRole('status')).not.toBeNull(); + expect(model.items.size).toBe(0); + + releaseVisible(); + await waitFor(() => expect([...model.items.keys()]).toEqual(['docs/'])); + }); + test('a server-emitted NDJSON error line surfaces its problem title, not the connectivity copy', async () => { const lines = [ JSON.stringify(folderEntry('team', true)), diff --git a/packages/app/src/components/FileTree.superseded-refresh.dom.test.tsx b/packages/app/src/components/FileTree.superseded-refresh.dom.test.tsx index c8437e842..c66d0350a 100644 --- a/packages/app/src/components/FileTree.superseded-refresh.dom.test.tsx +++ b/packages/app/src/components/FileTree.superseded-refresh.dom.test.tsx @@ -36,6 +36,27 @@ function abortableBodyResponse(signal: AbortSignal | null | undefined): Response }); } +function ndjsonFirstEntryThenAbort( + signal: AbortSignal | null | undefined, + firstLine: string, +): Response { + const encoder = new TextEncoder(); + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(encoder.encode(firstLine)); + const failWithAbort = () => + controller.error(new DOMException('The operation was aborted.', 'AbortError')); + if (!signal) return; + if (signal.aborted) failWithAbort(); + else signal.addEventListener('abort', failWithAbort, { once: true }); + }, + }); + return new Response(stream, { + status: 200, + headers: { 'content-type': 'application/x-ndjson' }, + }); +} + function deferredDocumentsResponse(): Promise { return new Promise((resolve) => { resolveTrailingDocuments = (body: unknown) => resolve(jsonResponse(body)); @@ -316,4 +337,26 @@ describe('FileTree superseded documents refresh', () => { ); expect(fetchFailedWarns).toEqual([]); }); + + test('NDJSON: a superseded mid-stream refresh drops its batch; the fresh listing wins', async () => { + documentsFetchPlan.push( + (signal) => ndjsonFirstEntryThenAbort(signal, `${JSON.stringify(docEntry('stale'))}\n`), + () => jsonResponse({ documents: [docEntry('fresh')], truncated: false }), + ); + render(); + + await waitFor(() => expect(documentsCallCount).toBe(1)); + await waitFor(() => expect([...model.items.keys()]).toEqual(['stale.md'])); + + window.dispatchEvent(new Event('focus')); + await waitFor(() => expect(documentsCallCount).toBe(2)); + + await waitFor(() => expect([...model.items.keys()]).toEqual(['fresh.md'])); + expect(model.items.has('stale.md')).toBe(false); + + const supersededWarns = consoleWarnSpy.mock.calls.filter( + (args) => typeof args[0] === 'string' && args[0].includes('[FileTree] fetch failed:'), + ); + expect(supersededWarns).toEqual([]); + }); }); diff --git a/packages/app/src/components/FileTree.tsx b/packages/app/src/components/FileTree.tsx index d204533a8..c46b4243e 100644 --- a/packages/app/src/components/FileTree.tsx +++ b/packages/app/src/components/FileTree.tsx @@ -213,7 +213,7 @@ import { import { OK_SIDEBAR_DRAG_MIME, serializeSidebarDragPayload } from '@/lib/sidebar-drag'; import { cn } from '@/lib/utils'; import { joinWorkspacePath } from '@/lib/workspace-paths'; -import { spliceLazyFolderChildren } from './file-tree-merge'; +import { mergeRootEntriesAdditive, spliceLazyFolderChildren } from './file-tree-merge'; import { OpenInAgentContextSubmenu } from './handoff/OpenInAgentContextSubmenu'; import { buildFolderHandoffInput, @@ -1881,9 +1881,23 @@ export function FileTree({ headers: SHOW_ALL_NDJSON_ACCEPT, }); if (isNdjsonResponse(res)) { - const { entries, truncated } = await consumeShowAllStream(res); - if (!active) return; const bypassClientDotDrop = showHiddenFilesRef.current; + let paintedFirstBatch = false; + const { entries, truncated } = await consumeShowAllStream(res, { + onBatch: (batch) => { + if (!active || controller.signal.aborted) return; + const batchEntries = filterVisibleEntries(toFileEntries(batch), bypassClientDotDrop); + if (batchEntries.length === 0) return; + setDocuments((prev) => mergeRootEntriesAdditive(prev, batchEntries)); + if (!paintedFirstBatch) { + paintedFirstBatch = true; + setError(null); + noteConnectivityRecovered(); + setLoading(false); + } + }, + }); + if (!active) return; const serverEntries = filterVisibleEntries(toFileEntries(entries), bypassClientDotDrop); setDocuments((prev) => spliceLazyFolderChildren(prev, '', serverEntries, recentLocalAddsRef.current), diff --git a/packages/app/src/components/SkillsSidebarSection.tsx b/packages/app/src/components/SkillsSidebarSection.tsx index cf3e08324..157c53a5e 100644 --- a/packages/app/src/components/SkillsSidebarSection.tsx +++ b/packages/app/src/components/SkillsSidebarSection.tsx @@ -162,13 +162,22 @@ function SkillFolderItem({ }) { const { t } = useLingui(); const [open, setOpen] = useState(false); + const [menuOpen, setMenuOpen] = useState(false); const active = activeDocName === skillLiveDocName(skill.scope, skill.name); return ( - + { + e.preventDefault(); + setMenuOpen(true); + }} + > {/* Display the prefix-stripped name (`open-knowledge-pack-X` → `X`) so @@ -196,7 +205,7 @@ function SkillFolderItem({ {/* The same context menu a file row gets (Reveal / Open with AI / Terminal / Copy Path / Duplicate / Rename / Delete), with Install/Uninstall in place of Hide — reuses SkillContextMenuItems. */} - + diff --git a/packages/app/src/components/TerminalPanel.launch.dom.test.tsx b/packages/app/src/components/TerminalPanel.launch.dom.test.tsx index 33209ceda..ef23ce7ea 100644 --- a/packages/app/src/components/TerminalPanel.launch.dom.test.tsx +++ b/packages/app/src/components/TerminalPanel.launch.dom.test.tsx @@ -72,11 +72,20 @@ const ON_PATH: CliReadiness = { onPath: 'present' }; function makeBridge(preflight: ClaudeReadiness = WIRED, cliReadiness: CliReadiness = ON_PATH) { const dataSubs: Array<(m: OkPtyData) => void> = []; const terminal = { - create: mock(async () => ({ ok: true as const, ptyId: 'pty-1' })), + create: mock(async (_opts: { cols: number; rows: number; launchCommand?: string }) => ({ + ok: true as const, + ptyId: 'pty-1', + })), input: mock((_id: string, _d: string) => {}), resize: mock(() => {}), kill: mock(async () => {}), drain: mock(() => {}), + adopt: mock( + async (): Promise<{ ok: true; replay: string } | { ok: false; reason: string }> => ({ + ok: true, + replay: '', + }), + ), onData: mock((cb: (m: OkPtyData) => void) => { dataSubs.push(cb); return mock(() => {}); @@ -101,18 +110,25 @@ function makeBridge(preflight: ClaudeReadiness = WIRED, cliReadiness: CliReadine const { TerminalPanel } = await import('./TerminalPanel'); -/** Inputs the user typed are forwarded verbatim; the launch is the only call - * that goes through `buildCliLaunchCommand`. Filter to the launch shape for a - * given binary prefix. */ -function launchWrites(inputMock: ReturnType, bin = 'claude'): string[] { +/** The `launchCommand` baked into the (single) `create` call, or undefined when + * none was passed (plain shell). The launch's only sanctioned transport. */ +function bakedLaunch(createMock: ReturnType): string | undefined { + const calls = createMock.mock.calls; + const last = calls.at(-1)?.[0] as { launchCommand?: string } | undefined; + return last?.launchCommand; +} + +/** Any `terminal.input` write that looks like a baked launch command — must stay + * empty: the launch rides `create`, never the line editor (the whole fix). */ +function launchInputWrites(inputMock: ReturnType): string[] { return inputMock.mock.calls .map((c) => c[1] as string) - .filter((d) => typeof d === 'string' && d.startsWith(`${bin} `)); + .filter((d) => typeof d === 'string' && /^(claude|codex|cursor-agent|opencode) /.test(d)); } const CLAUDE_PRE = `--settings '{"enabledMcpjsonServers":["open-knowledge"]}'`; -describe('TerminalPanel "Open in terminal" launch', () => { +describe('TerminalPanel "Open in terminal" launch (baked into the PTY spawn)', () => { beforeEach(() => { (globalThis as { ResizeObserver: unknown }).ResizeObserver = MockResizeObserver; }); @@ -120,363 +136,194 @@ describe('TerminalPanel "Open in terminal" launch', () => { cleanup(); }); - test("writes `claude --settings '' ''` exactly once on running, after first output", async () => { - const { bridge, terminal, pushData } = makeBridge(WIRED); + test("bakes `claude --settings '' ''` into create — no `\\r`, never via input", async () => { + const { bridge, terminal } = makeBridge(WIRED); const prompt = "Let's work on `foo.md` using OpenKnowledge."; - const { rerender } = render( - , - ); - - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.claudePreflight).toHaveBeenCalled()); - - await waitFor(() => expect(launchWrites(terminal.input).length).toBe(1)); - expect(launchWrites(terminal.input)[0]).toBe( - `claude ${CLAUDE_PRE} 'Let'\\''s work on \`foo.md\` using OpenKnowledge.'\r`, - ); - - rerender(); - await act(async () => { - await Promise.resolve(); - }); - expect(launchWrites(terminal.input).length).toBe(1); - }); - - test('does NOT write a launch before the shell is running', async () => { - const terminal = { - create: mock(() => new Promise(() => {})), - input: mock(() => {}), - resize: mock(() => {}), - kill: mock(async () => {}), - drain: mock(() => {}), - onData: mock(() => mock(() => {})), - onExit: mock(() => mock(() => {})), - claudePreflight: mock(async () => WIRED), - cliPreflight: mock(async () => ON_PATH), - rewireClaudeMcp: mock(async () => WIRED), - }; - const bridge = { - terminal, - shell: { openExternal: mock(async () => {}) }, - config: { e2eSmoke: false }, - } as unknown as OkDesktopBridge; + render(); - render(); await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); - await act(async () => { - await Promise.resolve(); - }); - expect(launchWrites(terminal.input).length).toBe(0); + expect(bakedLaunch(terminal.create)).toBe( + `claude ${CLAUDE_PRE} 'Let'\\''s work on \`foo.md\` using OpenKnowledge.'`, + ); + expect(bakedLaunch(terminal.create)).not.toContain('\r'); + expect(launchInputWrites(terminal.input)).toEqual([]); }); - test('does NOT write the command when claude is not found (the banner handles remediation)', async () => { - const { bridge, terminal, pushData } = makeBridge({ - claude: 'not-found', - mcp: 'needs-rewire', - }); + test('spawns a plain shell (no launchCommand) when claude is not found, and surfaces the banner', async () => { + const { bridge, terminal } = makeBridge({ claude: 'not-found', mcp: 'needs-rewire' }); render(); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.claudePreflight).toHaveBeenCalled()); - await act(async () => { - await Promise.resolve(); - }); - - expect(launchWrites(terminal.input).length).toBe(0); + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(bakedLaunch(terminal.create)).toBeUndefined(); + expect(launchInputWrites(terminal.input)).toEqual([]); + await screen.findByText(/Claude Code \(claude\) isn't installed/); }); - test('launches when claude is present even if OK tools need a rewire (bare — no pre-approval)', async () => { - const { bridge, terminal, pushData } = makeBridge({ - claude: 'present', - mcp: 'needs-rewire', - }); + test('bakes a BARE claude command (no pre-approval) when claude is present but OK tools need a rewire', async () => { + const { bridge, terminal } = makeBridge({ claude: 'present', mcp: 'needs-rewire' }); render(); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.claudePreflight).toHaveBeenCalled()); - - await waitFor(() => expect(launchWrites(terminal.input).length).toBe(1)); - expect(launchWrites(terminal.input)[0]).toBe("claude 'hi'\r"); + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(bakedLaunch(terminal.create)).toBe("claude 'hi'"); + expect(bakedLaunch(terminal.create)).not.toContain('--settings'); }); test("does NOT pre-approve when the project MCP entry is not OK's own (mcpPreApprovable false)", async () => { - const { bridge, terminal, pushData } = makeBridge(WIRED_FOREIGN_PROJECT); + const { bridge, terminal } = makeBridge(WIRED_FOREIGN_PROJECT); render(); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.claudePreflight).toHaveBeenCalled()); - - await waitFor(() => expect(launchWrites(terminal.input).length).toBe(1)); - expect(launchWrites(terminal.input)[0]).toBe("claude 'hi'\r"); - expect(launchWrites(terminal.input)[0]).not.toContain('--settings'); + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(bakedLaunch(terminal.create)).toBe("claude 'hi'"); + expect(bakedLaunch(terminal.create)).not.toContain('--settings'); }); - test('re-verifies pre-approval at LAUNCH time, not mount time (TOCTOU)', async () => { - const dataSubs: Array<(m: OkPtyData) => void> = []; - let preflightCalls = 0; - const terminal = { - create: mock(async () => ({ ok: true as const, ptyId: 'pty-1' })), - input: mock((_id: string, _d: string) => {}), - resize: mock(() => {}), - kill: mock(async () => {}), - drain: mock(() => {}), - onData: mock((cb: (m: OkPtyData) => void) => { - dataSubs.push(cb); - return mock(() => {}); - }), - onExit: mock(() => mock(() => {})), - claudePreflight: mock(async () => (preflightCalls++ === 0 ? WIRED : WIRED_FOREIGN_PROJECT)), - cliPreflight: mock(async () => ON_PATH), - rewireClaudeMcp: mock(async () => WIRED), - }; - const bridge = { - terminal, - shell: { openExternal: mock(async () => {}) }, - config: { e2eSmoke: false }, - } as unknown as OkDesktopBridge; - const pushData = (m: OkPtyData) => { - for (const f of dataSubs) f(m); - }; - + test('verifies pre-approval at LAUNCH time (the bake gates on the fresh preflight, not a stale snapshot)', async () => { + const { bridge, terminal } = makeBridge(WIRED_FOREIGN_PROJECT); render(); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(launchWrites(terminal.input).length).toBe(1)); - expect(launchWrites(terminal.input)[0]).toBe("claude 'hi'\r"); - expect(launchWrites(terminal.input)[0]).not.toContain('--settings'); - expect(terminal.claudePreflight.mock.calls.length).toBeGreaterThanOrEqual(2); + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(bakedLaunch(terminal.create)).toBe("claude 'hi'"); + expect(terminal.claudePreflight).toHaveBeenCalled(); }); - test('a launch-time preflight REJECTION suppresses the write + surfaces the readiness banner (parity with codex/cursor)', async () => { - const dataSubs: Array<(m: OkPtyData) => void> = []; - let calls = 0; - const terminal = { - create: mock(async () => ({ ok: true as const, ptyId: 'pty-1' })), - input: mock((_id: string, _d: string) => {}), - resize: mock(() => {}), - kill: mock(async () => {}), - drain: mock(() => {}), - onData: mock((cb: (m: OkPtyData) => void) => { - dataSubs.push(cb); - return mock(() => {}); - }), - onExit: mock(() => mock(() => {})), - claudePreflight: mock(async () => { - if (calls++ === 0) return WIRED; - throw new Error('ipc boom'); - }), - cliPreflight: mock(async () => ON_PATH), - rewireClaudeMcp: mock(async () => WIRED), - }; - const bridge = { - terminal, - shell: { openExternal: mock(async () => {}) }, - config: { e2eSmoke: false }, - } as unknown as OkDesktopBridge; - const pushData = (m: OkPtyData) => { - for (const f of dataSubs) f(m); - }; - - render(); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.claudePreflight).toHaveBeenCalledTimes(2)); - await act(async () => { - await Promise.resolve(); + test('a claude launch-preflight REJECTION spawns a plain shell + surfaces the banner (no command-not-found)', async () => { + const { bridge, terminal } = makeBridge(); + terminal.claudePreflight = mock(async () => { + throw new Error('ipc boom'); }); + render(); - expect(launchWrites(terminal.input).length).toBe(0); + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(bakedLaunch(terminal.create)).toBeUndefined(); + expect(launchInputWrites(terminal.input)).toEqual([]); await screen.findByText(/Claude Code \(claude\) isn't installed/); }); - test('a same-nonce effect re-run during the launch preflight window does not drop the launch', async () => { - const dataSubs: Array<(m: OkPtyData) => void> = []; - let calls = 0; - const deferreds: Array<(r: ClaudeReadiness) => void> = []; - const terminal = { - create: mock(async () => ({ ok: true as const, ptyId: 'pty-1' })), - input: mock((_id: string, _d: string) => {}), - resize: mock(() => {}), - kill: mock(async () => {}), - drain: mock(() => {}), - onData: mock((cb: (m: OkPtyData) => void) => { - dataSubs.push(cb); - return mock(() => {}); - }), - onExit: mock(() => mock(() => {})), - claudePreflight: mock(() => - calls++ === 0 - ? Promise.resolve(WIRED) - : new Promise((resolve) => { - deferreds.push(resolve); - }), - ), - cliPreflight: mock(async () => ON_PATH), - rewireClaudeMcp: mock(async () => WIRED), - }; - const bridge = { - terminal, - shell: { openExternal: mock(async () => {}) }, - config: { e2eSmoke: false }, - } as unknown as OkDesktopBridge; - const pushData = (m: OkPtyData) => { - for (const f of dataSubs) f(m); - }; - - const { rerender } = render( - , - ); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(deferreds.length).toBe(1)); - - rerender(); - await waitFor(() => expect(deferreds.length).toBe(2)); - - act(() => { - for (const resolve of deferreds) resolve(WIRED); + test('claude launch-time verdict UNKNOWN spawns a plain shell + surfaces the banner (unknown→not-found for display)', async () => { + const { bridge, terminal } = makeBridge({ + claude: 'unknown', + mcp: 'needs-rewire', + mcpPreApprovable: false, }); + render(); - await waitFor(() => expect(launchWrites(terminal.input).length).toBe(1)); - expect(launchWrites(terminal.input)[0]).toBe(`claude ${CLAUDE_PRE} 'hi'\r`); - }); - - test('a new nonce fires a second launch (repeat "Open in terminal" while running)', async () => { - const { bridge, terminal, pushData } = makeBridge(WIRED); - const { rerender } = render( - , - ); - - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(launchWrites(terminal.input).length).toBe(1)); - - rerender( - , - ); - await waitFor(() => expect(launchWrites(terminal.input).length).toBe(2)); - expect(launchWrites(terminal.input)[1]).toBe(`claude ${CLAUDE_PRE} 'second'\r`); + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(bakedLaunch(terminal.create)).toBeUndefined(); + expect(launchInputWrites(terminal.input)).toEqual([]); + await screen.findByText(/Claude Code \(claude\) isn't installed/); }); - test('codex launch probes cliPreflight and writes the codex command', async () => { - const { bridge, terminal, pushData } = makeBridge(WIRED, ON_PATH); + test('codex launch probes cliPreflight and bakes the codex command', async () => { + const { bridge, terminal } = makeBridge(WIRED, ON_PATH); render(); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.cliPreflight).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(terminal.cliPreflight).toHaveBeenCalledTimes(1); expect(terminal.cliPreflight.mock.calls[0]?.[0]).toBe('codex'); - - await waitFor(() => expect(launchWrites(terminal.input, 'codex').length).toBe(1)); - expect(launchWrites(terminal.input, 'codex')[0]).toBe("codex 'hi'\r"); + expect(bakedLaunch(terminal.create)).toBe("codex 'hi'"); + expect(launchInputWrites(terminal.input)).toEqual([]); }); - test('cursor launch writes the cursor-agent command (the agent CLI, not the editor)', async () => { - const { bridge, terminal, pushData } = makeBridge(WIRED, ON_PATH); + test('cursor launch bakes the cursor-agent command (the agent CLI, not the editor)', async () => { + const { bridge, terminal } = makeBridge(WIRED, ON_PATH); render(); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.cliPreflight).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(bakedLaunch(terminal.create)).toBe("cursor-agent 'hi'"); + }); - await waitFor(() => expect(launchWrites(terminal.input, 'cursor-agent').length).toBe(1)); - expect(launchWrites(terminal.input, 'cursor-agent')[0]).toBe("cursor-agent 'hi'\r"); + test('opencode launch bakes the --prompt form (positional is the project dir)', async () => { + const { bridge, terminal } = makeBridge(WIRED, ON_PATH); + render(); + + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(bakedLaunch(terminal.create)).toBe("opencode --prompt 'hi'"); }); - test('codex not on PATH: suppresses the write and surfaces the missing-CLI banner', async () => { - const { bridge, terminal, pushData } = makeBridge(WIRED, { onPath: 'not-found' }); + test('codex not on PATH: spawns a plain shell + surfaces the missing-CLI banner', async () => { + const { bridge, terminal } = makeBridge(WIRED, { onPath: 'not-found' }); render(); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.cliPreflight).toHaveBeenCalledTimes(1)); - + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(bakedLaunch(terminal.create)).toBeUndefined(); await screen.findByText(/Codex \(codex\) isn't installed/); - expect(launchWrites(terminal.input, 'codex').length).toBe(0); + expect(launchInputWrites(terminal.input)).toEqual([]); }); - test('cursor probe UNKNOWN re-probes once; still-unknown suppresses + shows the banner', async () => { - const { bridge, terminal, pushData } = makeBridge(WIRED, { onPath: 'unknown' }); + test('cursor probe UNKNOWN re-probes once; still-unknown spawns plain + shows the banner', async () => { + const { bridge, terminal } = makeBridge(WIRED, { onPath: 'unknown' }); render(); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.cliPreflight).toHaveBeenCalledTimes(2)); + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(terminal.cliPreflight).toHaveBeenCalledTimes(2); + expect(bakedLaunch(terminal.create)).toBeUndefined(); await screen.findByText(/Cursor \(cursor-agent\) isn't installed/); - expect(launchWrites(terminal.input, 'cursor-agent').length).toBe(0); }); - test('cursor probe UNKNOWN then PRESENT on re-probe: launches with the preserved prompt', async () => { + test('cursor probe UNKNOWN then PRESENT on re-probe: bakes the preserved prompt', async () => { let calls = 0; - const { bridge, terminal, pushData } = makeBridge(WIRED); + const { bridge, terminal } = makeBridge(WIRED); terminal.cliPreflight = mock(async () => { calls += 1; return calls === 1 ? { onPath: 'unknown' as const } : { onPath: 'present' as const }; }); render(); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.cliPreflight).toHaveBeenCalledTimes(2)); - await waitFor(() => expect(launchWrites(terminal.input, 'cursor-agent').length).toBe(1)); - expect(launchWrites(terminal.input, 'cursor-agent')[0]).toBe("cursor-agent 'hi'\r"); + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(terminal.cliPreflight).toHaveBeenCalledTimes(2); + expect(bakedLaunch(terminal.create)).toBe("cursor-agent 'hi'"); }); - test('cliPreflight IPC rejection suppresses the write (no raw command-not-found)', async () => { - const { bridge, terminal, pushData } = makeBridge(WIRED); + test('cliPreflight IPC rejection spawns plain + surfaces the banner (no raw command-not-found)', async () => { + const { bridge, terminal } = makeBridge(WIRED); terminal.cliPreflight = mock(async () => { throw new Error('ipc channel closed'); }); render(); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.cliPreflight).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(bakedLaunch(terminal.create)).toBeUndefined(); await screen.findByText(/Codex \(codex\) isn't installed/); - expect(launchWrites(terminal.input, 'codex').length).toBe(0); + expect(launchInputWrites(terminal.input)).toEqual([]); }); - test('claude present at mount but not-found on the fresh recheck: suppresses the write', async () => { - let calls = 0; - const { bridge, terminal, pushData } = makeBridge(WIRED); - terminal.claudePreflight = mock(async () => { - calls += 1; - return calls === 1 - ? WIRED - : { claude: 'not-found' as const, mcp: 'needs-rewire' as const, mcpPreApprovable: false }; - }); - render(); + test('an adopted (rehydrated) session does NOT re-bake its launch', async () => { + const { bridge, terminal } = makeBridge(WIRED); + render( + , + ); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.claudePreflight).toHaveBeenCalledTimes(2)); + await waitFor(() => expect(terminal.adopt).toHaveBeenCalledTimes(1)); await act(async () => { await Promise.resolve(); }); - expect(launchWrites(terminal.input).length).toBe(0); - await screen.findByText(/Claude Code \(claude\) isn't installed/); + expect(terminal.create).not.toHaveBeenCalled(); + expect(launchInputWrites(terminal.input)).toEqual([]); }); - test('claude present at mount but UNKNOWN on the fresh recheck: suppresses + surfaces the banner', async () => { - let calls = 0; - const { bridge, terminal, pushData } = makeBridge(WIRED); - terminal.claudePreflight = mock(async () => { - calls += 1; - return calls === 1 - ? WIRED - : { claude: 'unknown' as const, mcp: 'needs-rewire' as const, mcpPreApprovable: false }; - }); - render(); + test('a FAILED adoption (survivor gone) falls through to a plain shell — does NOT re-bake the launch', async () => { + const { bridge, terminal } = makeBridge(WIRED); + terminal.adopt = mock( + async (): Promise<{ ok: true; replay: string } | { ok: false; reason: string }> => ({ + ok: false, + reason: 'unknown-session', + }), + ); + render( + , + ); - await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); - act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); - await waitFor(() => expect(terminal.claudePreflight).toHaveBeenCalledTimes(2)); - expect(launchWrites(terminal.input).length).toBe(0); - await screen.findByText(/Claude Code \(claude\) isn't installed/); + await waitFor(() => expect(terminal.adopt).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(terminal.create).toHaveBeenCalledTimes(1)); + expect(bakedLaunch(terminal.create)).toBeUndefined(); + expect(launchInputWrites(terminal.input)).toEqual([]); }); }); diff --git a/packages/app/src/components/TerminalPanel.tsx b/packages/app/src/components/TerminalPanel.tsx index 27e3e65d2..1eb5edb9f 100644 --- a/packages/app/src/components/TerminalPanel.tsx +++ b/packages/app/src/components/TerminalPanel.tsx @@ -1,6 +1,6 @@ import '@xterm/xterm/css/xterm.css'; -import { buildCliLaunchCommand, type TerminalCli } from '@inkeep/open-knowledge-core'; +import { buildCliLaunchArgString, type TerminalCli } from '@inkeep/open-knowledge-core'; import { useLingui } from '@lingui/react/macro'; import { FitAddon } from '@xterm/addon-fit'; import { Unicode11Addon } from '@xterm/addon-unicode11'; @@ -76,6 +76,8 @@ interface TerminalSessionProps { readonly onExit?: (info: { readonly exitCode: number; readonly signal: number | null }) => void; readonly onTitleChange?: (title: string) => void; readonly onRestart: () => void; + /** "Open in terminal" launch intent — baked into the PTY spawn when present + * (preflight-gated). See {@link TerminalPanelProps.launch}. */ readonly launch?: TerminalLaunchIntent | null; /** Surviving PTY to adopt on mount instead of spawning a fresh shell; `null` * spawns fresh (the normal path). */ @@ -99,11 +101,8 @@ function TerminalSession({ const initialResolvedThemeRef = useRef(resolvedTheme); const [status, setStatus] = useState('starting'); const [readiness, setReadiness] = useState(null); - const [preflightDone, setPreflightDone] = useState(false); const [exitInfo, setExitInfo] = useState(null); const ptyIdRef = useRef(null); - const [firstOutputSeen, setFirstOutputSeen] = useState(false); - const lastLaunchedNonceRef = useRef(null); const [missingCli, setMissingCli] = useState(null); useEffect(() => { @@ -232,7 +231,6 @@ function TerminalSession({ unsubData = bridge.terminal.onData((msg) => { if (msg.ptyId !== ptyId) return; - if (!cancelled) setFirstOutputSeen(true); term.write(msg.data, () => bridge.terminal.drain(msg.ptyId, msg.data.length)); }); @@ -251,17 +249,58 @@ function TerminalSession({ term.focus(); - bridge.terminal - .claudePreflight() - .then((readinessResult) => { - if (!cancelled) setReadiness(readinessResult); - }) - .catch((err) => { - console.warn('[terminal] claude readiness preflight failed', err); - }) - .finally(() => { - if (!cancelled) setPreflightDone(true); - }); + const launchOwnsReadiness = launch !== null && adoptPtyId === null; + if (!launchOwnsReadiness) { + bridge.terminal + .claudePreflight() + .then((readinessResult) => { + if (!cancelled) setReadiness(readinessResult); + }) + .catch((err) => { + console.warn('[terminal] claude readiness preflight failed', err); + }); + } + }; + + const resolveLaunchCommand = async ( + intent: TerminalLaunchIntent, + ): Promise => { + if (intent.cli === 'claude') { + try { + const fresh = await bridge.terminal.claudePreflight(); + if (fresh.claude === 'present') { + if (!cancelled) setReadiness(fresh); + return buildCliLaunchArgString('claude', intent.prompt, { + mcpPreApprove: fresh.mcpPreApprovable === true, + }); + } + if (!cancelled) { + setReadiness( + fresh.claude === 'not-found' + ? fresh + : { claude: 'not-found', mcp: fresh.mcp, mcpPreApprovable: false }, + ); + } + } catch (err) { + console.warn('[terminal] claude launch preflight failed', err); + if (!cancelled) { + setReadiness({ claude: 'not-found', mcp: 'needs-rewire', mcpPreApprovable: false }); + } + } + return undefined; + } + try { + let res = await bridge.terminal.cliPreflight(intent.cli); + if (res.onPath === 'unknown') { + if (cancelled) return undefined; + res = await bridge.terminal.cliPreflight(intent.cli); + } + if (res.onPath === 'present') return buildCliLaunchArgString(intent.cli, intent.prompt); + } catch (err) { + console.warn('[terminal] cliPreflight failed', { cli: intent.cli, err }); + } + if (!cancelled) setMissingCli(intent.cli); + return undefined; }; void (async () => { @@ -282,9 +321,15 @@ function TerminalSession({ } } + let launchCommand: string | undefined; + if (launch !== null && adoptPtyId === null) { + launchCommand = await resolveLaunchCommand(launch); + if (cancelled) return; + } + let result: Awaited>; try { - result = await bridge.terminal.create({ cols: term.cols, rows: term.rows }); + result = await bridge.terminal.create({ cols: term.cols, rows: term.rows, launchCommand }); } catch (err) { console.error('[terminal] create() failed:', err); if (cancelled) return; @@ -326,7 +371,7 @@ function TerminalSession({ .kill(ptyId) .catch((err) => console.warn('[terminal] kill on unmount failed:', err)); }; - }, [bridge, adoptPtyId]); + }, [bridge, adoptPtyId, launch]); useEffect(() => { const term = termRef.current; @@ -334,97 +379,6 @@ function TerminalSession({ term.options.theme = xtermThemeForMode(resolvedTheme); }, [resolvedTheme]); - useEffect(() => { - if (launch === null) return; - if (adoptPtyId !== null) return; - if (status !== 'running') return; - if (!firstOutputSeen) return; - if (lastLaunchedNonceRef.current === launch.nonce) return; - const ptyId = ptyIdRef.current; - if (ptyId === null) return; - - const nonce = launch.nonce; - let cancelled = false; - - if (launch.cli === 'claude') { - if (!preflightDone) return; - setMissingCli(null); - if (readiness?.claude === 'not-found') { - lastLaunchedNonceRef.current = nonce; // banner handles remediation; consume - return; - } - const { prompt } = launch; - const writeClaude = (mcpPreApprove: boolean) => { - if (cancelled) return; - const livePtyId = ptyIdRef.current; - if (livePtyId === null) return; - bridge.terminal.input( - livePtyId, - buildCliLaunchCommand('claude', prompt, { mcpPreApprove }), - ); - lastLaunchedNonceRef.current = nonce; // commit only after the write lands - }; - void bridge.terminal - .claudePreflight() - .then((fresh) => { - if (cancelled) return; - if (fresh.claude === 'present') { - writeClaude(fresh.mcpPreApprovable === true); - return; - } - setReadiness( - fresh.claude === 'not-found' - ? fresh - : { claude: 'not-found', mcp: fresh.mcp, mcpPreApprovable: false }, - ); - lastLaunchedNonceRef.current = nonce; - }) - .catch((err) => { - if (cancelled) return; - console.warn('[terminal] claude pre-approval recheck failed', { nonce, err }); - setReadiness({ claude: 'not-found', mcp: 'needs-rewire', mcpPreApprovable: false }); - lastLaunchedNonceRef.current = nonce; - }); - return () => { - cancelled = true; - }; - } - - setMissingCli(null); - const { cli, prompt } = launch; - const writeLaunch = () => { - if (cancelled) return; - const livePtyId = ptyIdRef.current; - if (livePtyId === null) return; - bridge.terminal.input(livePtyId, buildCliLaunchCommand(cli, prompt)); - lastLaunchedNonceRef.current = nonce; // commit only after the write lands - }; - const suppress = () => { - if (cancelled) return; - setMissingCli(cli); - lastLaunchedNonceRef.current = nonce; // banner handles remediation; consume - }; - void (async () => { - try { - let res = await bridge.terminal.cliPreflight(cli); - if (res.onPath === 'unknown') { - if (cancelled) return; - res = await bridge.terminal.cliPreflight(cli); - } - if (cancelled) return; - if (res.onPath === 'present') writeLaunch(); - else suppress(); - } catch (err) { - if (cancelled) return; - console.warn('[terminal] cliPreflight failed', { cli, err }); - suppress(); - } - })(); - return () => { - cancelled = true; - }; - }, [bridge, launch, status, firstOutputSeen, preflightDone, readiness, adoptPtyId]); - return (
{status === 'running' && readiness ? ( diff --git a/packages/app/src/components/file-tree-merge.test.ts b/packages/app/src/components/file-tree-merge.test.ts index ed02ad725..8dcc051ef 100644 --- a/packages/app/src/components/file-tree-merge.test.ts +++ b/packages/app/src/components/file-tree-merge.test.ts @@ -24,6 +24,7 @@ import { describe, expect, test } from 'bun:test'; import { mergeAndPruneRecentLocalAdds, + mergeRootEntriesAdditive, STALE_REFRESH_PRESERVE_WINDOW_MS, spliceLazyFolderChildren, } from './file-tree-merge'; @@ -248,3 +249,36 @@ describe('spliceLazyFolderChildren', () => { expect(recentAdds.has('just-created.md')).toBe(true); }); }); + +describe('mergeRootEntriesAdditive', () => { + test('empty current: returns a copy of the incoming entries', () => { + const incoming = [doc('a'), folder('team')]; + const result = mergeRootEntriesAdditive([], incoming); + expect(result).toEqual(incoming); + expect(result).not.toBe(incoming); + }); + + test('empty incoming: returns a copy of the current entries unchanged', () => { + const current = [doc('a')]; + const result = mergeRootEntriesAdditive(current, []); + expect(result).toEqual(current); + expect(result).not.toBe(current); + }); + + test('unions new entries onto the current set, preserving order', () => { + const result = mergeRootEntriesAdditive([doc('a')], [doc('b'), folder('team')]); + expect(result).toEqual([doc('a'), doc('b'), folder('team')]); + }); + + test('de-dupes by tree path — the existing entry wins on collision', () => { + const current = [doc('a', '2026-01-01T00:00:00.000Z')]; + const incoming = [doc('a', '2099-12-31T00:00:00.000Z'), doc('b')]; + const result = mergeRootEntriesAdditive(current, incoming); + expect(result).toEqual([doc('a', '2026-01-01T00:00:00.000Z'), doc('b')]); + }); + + test('a folder and a document collide only when their tree paths match', () => { + const result = mergeRootEntriesAdditive([folder('team')], [folder('team'), doc('team')]); + expect(result).toEqual([folder('team'), doc('team')]); + }); +}); diff --git a/packages/app/src/components/file-tree-merge.ts b/packages/app/src/components/file-tree-merge.ts index 1af5dce30..4c8af9e0c 100644 --- a/packages/app/src/components/file-tree-merge.ts +++ b/packages/app/src/components/file-tree-merge.ts @@ -30,6 +30,22 @@ export function mergeAndPruneRecentLocalAdds( return [...serverEntries, ...preservedLocal]; } +export function mergeRootEntriesAdditive( + currentEntries: readonly FileEntry[], + incomingEntries: readonly FileEntry[], +): FileEntry[] { + if (incomingEntries.length === 0) return [...currentEntries]; + const seen = new Set(currentEntries.map((entry) => fileEntryToTreePath(entry))); + const merged = [...currentEntries]; + for (const entry of incomingEntries) { + const treePath = fileEntryToTreePath(entry); + if (seen.has(treePath)) continue; + seen.add(treePath); + merged.push(entry); + } + return merged; +} + export function spliceLazyFolderChildren( currentEntries: readonly FileEntry[], folderTreePath: string, diff --git a/packages/app/src/lib/desktop-bridge-types.ts b/packages/app/src/lib/desktop-bridge-types.ts index fc30ea13f..9c1fa4c7c 100644 --- a/packages/app/src/lib/desktop-bridge-types.ts +++ b/packages/app/src/lib/desktop-bridge-types.ts @@ -734,7 +734,11 @@ export interface OkDesktopBridge { }; terminal: { - create(opts: { cols: number; rows: number }): Promise; + create(opts: { + cols: number; + rows: number; + launchCommand?: string; + }): Promise; input(ptyId: string, data: string): void; resize(ptyId: string, cols: number, rows: number): void; kill(ptyId: string): Promise; diff --git a/packages/app/src/lib/show-all-stream.test.ts b/packages/app/src/lib/show-all-stream.test.ts index 996e82e32..a6022e42c 100644 --- a/packages/app/src/lib/show-all-stream.test.ts +++ b/packages/app/src/lib/show-all-stream.test.ts @@ -135,4 +135,35 @@ describe('consumeShowAllStream', () => { const { entries } = await consumeShowAllStream(ndjsonResponse(body)); expect(entries.map((e) => e.docName)).toEqual(['beta']); }); + + test('onBatch delivers each chunk’s entries incrementally before completion', async () => { + const res = chunkedResponse([ + docLine('alpha') + docLine('beta'), + docLine('gamma') + completeLine(false, 3), + ]); + const batches: string[][] = []; + const { entries } = await consumeShowAllStream(res, { + onBatch: (batch) => batches.push(batch.map((e) => e.docName)), + }); + expect(batches).toEqual([['alpha', 'beta'], ['gamma']]); + expect(entries.map((e) => e.docName)).toEqual(['alpha', 'beta', 'gamma']); + }); + + test('onBatch is not called for a chunk carrying only the terminal complete line', async () => { + const res = chunkedResponse([docLine('alpha'), completeLine(false, 1)]); + const batchSizes: number[] = []; + await consumeShowAllStream(res, { onBatch: (batch) => batchSizes.push(batch.length) }); + expect(batchSizes).toEqual([1]); + }); + + test('a throw from onBatch propagates out of consumeShowAllStream', async () => { + const res = chunkedResponse([docLine('alpha'), completeLine(false, 1)]); + await expect( + consumeShowAllStream(res, { + onBatch: () => { + throw new Error('consumer blew up'); + }, + }), + ).rejects.toThrow('consumer blew up'); + }); }); diff --git a/packages/app/src/lib/show-all-stream.ts b/packages/app/src/lib/show-all-stream.ts index 58f672f31..9af11d325 100644 --- a/packages/app/src/lib/show-all-stream.ts +++ b/packages/app/src/lib/show-all-stream.ts @@ -23,13 +23,22 @@ function isControlEvent(value: unknown): value is ControlEvent { return typeof value === 'object' && value !== null && 'type' in value; } -export async function consumeShowAllStream(res: Response): Promise { +export interface ConsumeShowAllStreamOptions { + onBatch?: (batch: DocumentListEntry[]) => void; +} + +export async function consumeShowAllStream( + res: Response, + options: ConsumeShowAllStreamOptions = {}, +): Promise { const body = res.body; if (!body) throw new ShowAllStreamError('Show All Files stream had no response body.'); + const { onBatch } = options; const reader = body.getReader(); const decoder = new TextDecoder(); const entries: DocumentListEntry[] = []; + let pendingBatch: DocumentListEntry[] = []; let truncated = false; let buffer = ''; @@ -57,12 +66,20 @@ export async function consumeShowAllStream(res: Response): Promise { + if (!onBatch || pendingBatch.length === 0) return; + const batch = pendingBatch; + pendingBatch = []; + onBatch(batch); + }; + try { let done = false; while (!done) { @@ -79,9 +96,11 @@ export async function consumeShowAllStream(res: Response): Promise; + create(opts: { + cols: number; + rows: number; + launchCommand?: string; + }): Promise; input(ptyId: string, data: string): void; resize(ptyId: string, cols: number, rows: number): void; kill(ptyId: string): Promise; diff --git a/packages/core/src/handoff/index.ts b/packages/core/src/handoff/index.ts index 1ee579584..5476ea045 100644 --- a/packages/core/src/handoff/index.ts +++ b/packages/core/src/handoff/index.ts @@ -21,6 +21,7 @@ export { } from './prompt-composer.ts'; export { buildClaudeLaunchCommand, + buildCliLaunchArgString, buildCliLaunchCommand, shellSingleQuote, TERMINAL_CLI_IDS, diff --git a/packages/core/src/handoff/terminal-launch.test.ts b/packages/core/src/handoff/terminal-launch.test.ts index 70909e6d2..4863f323c 100644 --- a/packages/core/src/handoff/terminal-launch.test.ts +++ b/packages/core/src/handoff/terminal-launch.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'bun:test'; import { MCP_SERVER_NAME } from '../constants/mcp.ts'; import { buildClaudeLaunchCommand, + buildCliLaunchArgString, buildCliLaunchCommand, shellSingleQuote, TERMINAL_CLI_IDS, @@ -97,6 +98,29 @@ describe('buildCliLaunchCommand', () => { }); }); +describe('buildCliLaunchArgString', () => { + it('is the launch command WITHOUT the trailing carriage return', () => { + for (const cli of TERMINAL_CLI_IDS) { + const arg = buildCliLaunchArgString(cli, 'hi', { mcpPreApprove: true }); + expect(arg.endsWith('\r')).toBe(false); + expect(`${arg}\r`).toBe(buildCliLaunchCommand(cli, 'hi', { mcpPreApprove: true })); + } + }); + + it('keeps the fixed per-CLI shape (registry bin + single-quoted prompt)', () => { + expect(buildCliLaunchArgString('claude', 'hi')).toBe("claude 'hi'"); + expect(buildCliLaunchArgString('codex', 'hi')).toBe("codex 'hi'"); + expect(buildCliLaunchArgString('cursor', 'hi')).toBe("cursor-agent 'hi'"); + expect(buildCliLaunchArgString('opencode', 'hi')).toBe("opencode --prompt 'hi'"); + }); + + it('keeps an injection payload inert and contained in the prompt arg', () => { + const arg = buildCliLaunchArgString('claude', "'; rm -rf / #"); + expect(arg).toBe("claude ''\\''; rm -rf / #'"); + expect(arg.endsWith("''\\''; rm -rf / #'")).toBe(true); + }); +}); + describe('claude MCP pre-approval', () => { it('is OFF by default and only added for claude when opted in', () => { expect(buildCliLaunchCommand('claude', 'hi')).not.toContain('--settings'); diff --git a/packages/core/src/handoff/terminal-launch.ts b/packages/core/src/handoff/terminal-launch.ts index b82ef6933..d2b873273 100644 --- a/packages/core/src/handoff/terminal-launch.ts +++ b/packages/core/src/handoff/terminal-launch.ts @@ -14,7 +14,7 @@ export interface TerminalCliInfo { readonly bin: string; /** Claude's MCP pre-approval fragment, inserted verbatim between `` and * the prompt ONLY when the caller passes `mcpPreApprove: true` (see - * {@link buildCliLaunchCommand}) — i.e. after the launch site has verified the + * {@link buildCliLaunchArgString}) — i.e. after the launch site has verified the * project's `open-knowledge` `.mcp.json` entry is OK's own. An already-shell- * safe fragment (NOT re-quoted); registry-fixed, never user input. Claude-only; * omit for CLIs with no pre-approval. */ @@ -28,7 +28,7 @@ export interface TerminalCliInfo { /** Flag that carries the starting prompt for CLIs whose POSITIONAL argument is * NOT the prompt. OpenCode's positional is the project directory, so its * prompt must be passed as `--prompt ''`; claude/codex/cursor take the - * prompt positionally (omit this). When set, {@link buildCliLaunchCommand} + * prompt positionally (omit this). When set, {@link buildCliLaunchArgString} * inserts it immediately before the quoted prompt. */ readonly promptFlag?: string; } @@ -77,7 +77,7 @@ export interface BuildCliLaunchOptions { readonly mcpPreApprove?: boolean; } -export function buildCliLaunchCommand( +export function buildCliLaunchArgString( cli: TerminalCli, prompt: string, opts: BuildCliLaunchOptions = {}, @@ -86,7 +86,15 @@ export function buildCliLaunchCommand( const preApprove = opts.mcpPreApprove === true && info.mcpPreApproveArg ? `${info.mcpPreApproveArg} ` : ''; const promptFlag = info.promptFlag ? `${info.promptFlag} ` : ''; - return `${info.bin} ${preApprove}${promptFlag}${shellSingleQuote(prompt)}\r`; + return `${info.bin} ${preApprove}${promptFlag}${shellSingleQuote(prompt)}`; +} + +export function buildCliLaunchCommand( + cli: TerminalCli, + prompt: string, + opts: BuildCliLaunchOptions = {}, +): string { + return `${buildCliLaunchArgString(cli, prompt, opts)}\r`; } export function buildClaudeLaunchCommand(prompt: string, opts: BuildCliLaunchOptions = {}): string { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7eb3a7c3a..23ad52a40 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -376,6 +376,7 @@ export { assertNeverUrnIpcLookup, buildClaudeLaunchCommand, buildClaudeUrl, + buildCliLaunchArgString, buildCliLaunchCommand, buildCodexUrl, buildCursorUrl, diff --git a/packages/desktop/src/main/index.ts b/packages/desktop/src/main/index.ts index f8f4a2142..a2d6a45b5 100644 --- a/packages/desktop/src/main/index.ts +++ b/packages/desktop/src/main/index.ts @@ -1583,6 +1583,7 @@ function registerIpcHandlers() { projectRoot: projectPath, cols: clampPtyDimension(opts.cols, DEFAULT_PTY_COLS), rows: clampPtyDimension(opts.rows, DEFAULT_PTY_ROWS), + launchCommand: opts.launchCommand, }); }); handle('ok:pty:input', async (event, req) => { diff --git a/packages/desktop/src/main/terminal-manager.ts b/packages/desktop/src/main/terminal-manager.ts index 761ff961c..119427677 100644 --- a/packages/desktop/src/main/terminal-manager.ts +++ b/packages/desktop/src/main/terminal-manager.ts @@ -39,6 +39,7 @@ interface TerminalCreateRequest { projectRoot: string | null; cols: number; rows: number; + launchCommand?: string; } interface TerminalAddressedRequest { @@ -297,6 +298,7 @@ export function createTerminalManager(deps: TerminalManagerDeps): TerminalManage cwd: req.projectRoot, cols: req.cols, rows: req.rows, + launchCommand: req.launchCommand, }); return { ok: true, ptyId }; }, diff --git a/packages/desktop/src/shared/bridge-contract.ts b/packages/desktop/src/shared/bridge-contract.ts index 51359856e..d2be31e19 100644 --- a/packages/desktop/src/shared/bridge-contract.ts +++ b/packages/desktop/src/shared/bridge-contract.ts @@ -730,7 +730,11 @@ export interface OkDesktopBridge { }; terminal: { - create(opts: { cols: number; rows: number }): Promise; + create(opts: { + cols: number; + rows: number; + launchCommand?: string; + }): Promise; input(ptyId: string, data: string): void; resize(ptyId: string, cols: number, rows: number): void; kill(ptyId: string): Promise; diff --git a/packages/desktop/src/shared/ipc-channels.ts b/packages/desktop/src/shared/ipc-channels.ts index 254507cae..8502c0371 100644 --- a/packages/desktop/src/shared/ipc-channels.ts +++ b/packages/desktop/src/shared/ipc-channels.ts @@ -476,7 +476,7 @@ export interface RequestChannels { }; 'ok:pty:create': { - args: [opts: { cols: number; rows: number }]; + args: [opts: { cols: number; rows: number; launchCommand?: string }]; result: OkPtyCreateResult; }; 'ok:pty:input': { diff --git a/packages/desktop/src/utility/pty-host.ts b/packages/desktop/src/utility/pty-host.ts index 6b38bacd3..a36ed1c76 100644 --- a/packages/desktop/src/utility/pty-host.ts +++ b/packages/desktop/src/utility/pty-host.ts @@ -9,6 +9,7 @@ export interface PtyCreateMessage { cols: number; rows: number; shell?: string; + launchCommand?: string; } interface PtyInputMessage { type: 'input'; @@ -104,7 +105,10 @@ function asIncomingMessage(raw: unknown): PtyHostIncomingMessage | null { if (typeof m.ptyId !== 'string' || m.ptyId.length === 0) return null; switch (m.type) { case 'create': - return typeof m.cwd === 'string' && typeof m.cols === 'number' && typeof m.rows === 'number' + return typeof m.cwd === 'string' && + typeof m.cols === 'number' && + typeof m.rows === 'number' && + (m.launchCommand === undefined || typeof m.launchCommand === 'string') ? (raw as PtyHostIncomingMessage) : null; case 'input': @@ -132,6 +136,12 @@ export function resolveShell(env: Record, override?: return typeof shell === 'string' && shell.length > 0 ? shell : DARWIN_FALLBACK_SHELL; } +export function buildShellArgs(shell: string, launchCommand?: string): string[] { + if (launchCommand === undefined || launchCommand.length === 0) return ['-l', '-i']; + const quotedShell = `'${shell.replace(/'/g, "'\\''")}'`; + return ['-l', '-i', '-c', `${launchCommand}; exec ${quotedShell} -l -i`]; +} + export function buildShellEnv( parentEnv: Record, ): Record { @@ -176,7 +186,7 @@ export function setupPtyHost(deps: SetupPtyHostDeps): PtyHostHandle { const shellEnv = buildShellEnv(env); let pty: PtyProcessLike; try { - pty = deps.spawn(shell, ['-l', '-i'], { + pty = deps.spawn(shell, buildShellArgs(shell, message.launchCommand), { name: 'xterm-256color', cols: message.cols, rows: message.rows, diff --git a/packages/desktop/tests/utility/pty-host.test.ts b/packages/desktop/tests/utility/pty-host.test.ts index 35919bb0b..c869c5f3f 100644 --- a/packages/desktop/tests/utility/pty-host.test.ts +++ b/packages/desktop/tests/utility/pty-host.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from 'bun:test'; import { + buildShellArgs, buildShellEnv, type HostReapProcess, installHostReaping, @@ -134,6 +135,18 @@ describe('setupPtyHost — create', () => { expect(h.spawnCalls[0]?.options.rows).toBe(24); }); + test('bakes a launch command into a non-history `-c` spawn with an interactive exec tail', () => { + const h = makeHarness({ env: { SHELL: '/bin/zsh', PATH: '/usr/bin' } }); + h.fire(CREATE({ launchCommand: "claude 'do the thing'" })); + expect(h.spawnCalls[0]?.file).toBe('/bin/zsh'); + expect(h.spawnCalls[0]?.args).toEqual([ + '-l', + '-i', + '-c', + "claude 'do the thing'; exec '/bin/zsh' -l -i", + ]); + }); + test('falls back to /bin/zsh when SHELL is unset', () => { const h = makeHarness({ env: { PATH: '/usr/bin' } }); h.fire(CREATE()); @@ -495,6 +508,28 @@ describe('setupPtyHost — incoming message validation (asIncomingMessage guard) expect(logger.warnings.length).toBeGreaterThan(0); }); + test('accepts a create with a string launchCommand and bakes it', () => { + const h = makeHarness(); + h.fireRaw({ + type: 'create', + ptyId: 'p1', + cwd: '/x', + cols: 80, + rows: 24, + launchCommand: "x 'y'", + }); + expect(h.spawnCalls).toHaveLength(1); + expect(h.spawnCalls[0]?.args).toEqual(['-l', '-i', '-c', "x 'y'; exec '/bin/zsh' -l -i"]); + }); + + test('drops a create whose launchCommand is present but not a string', () => { + const logger = makeLogger(); + const h = makeHarness({ logger }); + h.fireRaw({ type: 'create', ptyId: 'p1', cwd: '/x', cols: 80, rows: 24, launchCommand: 123 }); + expect(h.spawnCalls).toHaveLength(0); + expect(logger.warnings.length).toBeGreaterThan(0); + }); + test('an unknown message type lands in the default warn branch (forward-compat)', () => { const logger = makeLogger(); const h = makeHarness({ logger }); @@ -504,6 +539,31 @@ describe('setupPtyHost — incoming message validation (asIncomingMessage guard) }); }); +describe('buildShellArgs', () => { + test('a plain tab is the bare login interactive shell', () => { + expect(buildShellArgs('/bin/zsh')).toEqual(['-l', '-i']); + expect(buildShellArgs('/bin/zsh', '')).toEqual(['-l', '-i']); + }); + + test('a launch bakes the command on `-c` and execs into a fresh interactive shell', () => { + expect(buildShellArgs('/bin/zsh', "codex 'hi'")).toEqual([ + '-l', + '-i', + '-c', + "codex 'hi'; exec '/bin/zsh' -l -i", + ]); + }); + + test('single-quotes the shell path in the exec tail (space/quote-safe)', () => { + expect(buildShellArgs("/odd path/o'sh", "claude 'x'")).toEqual([ + '-l', + '-i', + '-c', + "claude 'x'; exec '/odd path/o'\\''sh' -l -i", + ]); + }); +}); + describe('buildShellEnv', () => { test('strips markers, drops undefined, preserves the rest, marks the desktop terminal', () => { const env = buildShellEnv({