From c712df2a41c0627e276f774eb8081c46b330cb84 Mon Sep 17 00:00:00 2001 From: serafin-garcia Date: Fri, 26 Jun 2026 18:04:54 -0700 Subject: [PATCH] fix(open-knowledge): docked terminal never prints raw "command not found" on launch (#2208) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(open-knowledge): docked terminal never prints raw "command not found" on launch An "Open in terminal" launch now writes the ` ''` command only when a PATH probe confirms the CLI is present. On a flaky `unknown` probe it re-probes once; a `not-found` verdict, a still-`unknown` re-probe, or an IPC-level probe failure all suppress the write and surface the existing missing-CLI banner instead — so a missing binary never dumps a raw `command not found` into the terminal. Covers codex/cursor/ opencode (via cliPreflight) and claude (gated on the fresh claudePreflight recheck). * fix(open-knowledge): claude launch IPC-rejection surfaces the readiness banner A claude launch-time claudePreflight IPC rejection suppressed the write silently — no banner, unlike the codex/cursor/opencode suppress() path which shows the missing-CLI banner on an unconfirmed probe. Surface the readiness banner on the claude rejection too so the user gets feedback after clicking "Open in terminal" instead of a silent no-op. Addresses reviewer asymmetry note. * fix(open-knowledge): claude launch always surfaces feedback on unconfirmed probe Close the remaining silent-no-op gap: when the fresh claudePreflight recheck returns `unknown` (not just `not-found`), the launch was suppressed but the readiness banner hides for `unknown` by design — leaving the user with no feedback after clicking "Open in terminal". Map an unconfirmed verdict to not-found for DISPLAY so the banner always shows, matching the IPC-catch path and the codex/cursor/opencode suppress() banner. Also include `nonce` in the claude catch diagnostic log for correlation parity with the codex/cursor catch. Addresses reviewer threads (MAJOR: silent suppress on unconfirmed claude probe; nit: nonce in catch log). * refactor(open-knowledge): guard re-probe on cancel + assert banner in fresh-not-found test Two reviewer Consider items: (1) bail before the codex/cursor/opencode re-probe when the effect was cancelled during the first probe, so the second cliPreflight doesn't go out over a torn-down channel (avoids a wasted round-trip + a spurious StrictMode warn); (2) assert the readiness banner appears in the claude fresh-not-found suppression test so it verifies both halves of the contract (suppress + feedback), matching its three sibling tests. * refactor(open-knowledge): guard codex/cursor catch warn on cancel (claude-catch parity) The codex/cursor/opencode cliPreflight catch logged unconditionally even after the effect was cancelled; the claude catch already early-returns on `cancelled`. Add the same guard so a post-teardown rejection doesn't emit log noise. State was already safe (suppress() guards internally) — this is log-hygiene parity only. --------- GitOrigin-RevId: 80f0f826f83701af581bf6f6dac24c2be73d00bf --- ...erminal-launch-no-raw-command-not-found.md | 5 ++ .../TerminalPanel.launch.dom.test.tsx | 76 +++++++++++++++++-- packages/app/src/components/TerminalPanel.tsx | 51 +++++++++---- 3 files changed, 108 insertions(+), 24 deletions(-) create mode 100644 .changeset/terminal-launch-no-raw-command-not-found.md diff --git a/.changeset/terminal-launch-no-raw-command-not-found.md b/.changeset/terminal-launch-no-raw-command-not-found.md new file mode 100644 index 00000000..e89cd1c7 --- /dev/null +++ b/.changeset/terminal-launch-no-raw-command-not-found.md @@ -0,0 +1,5 @@ +--- +"@inkeep/open-knowledge-app": patch +--- + +Docked terminal: an _Open in terminal_ launch no longer prints a raw `command not found`. The launch gate now writes the ` ''` command only when a PATH probe confirms the CLI is present. On a flaky `unknown` probe it re-probes once; a `not-found` verdict, a still-`unknown` re-probe, or an IPC-level probe failure all suppress the write and surface the existing missing-CLI banner instead. This applies to Codex / Cursor / OpenCode (via `cliPreflight`) and to Claude (gated on the fresh `claudePreflight` recheck it already runs). The trade-off is a rare false-negative — an installed CLI whose probe flakes twice won't auto-launch — in exchange for a guaranteed-clean terminal. diff --git a/packages/app/src/components/TerminalPanel.launch.dom.test.tsx b/packages/app/src/components/TerminalPanel.launch.dom.test.tsx index bdd8927d..33209ced 100644 --- a/packages/app/src/components/TerminalPanel.launch.dom.test.tsx +++ b/packages/app/src/components/TerminalPanel.launch.dom.test.tsx @@ -252,7 +252,7 @@ describe('TerminalPanel "Open in terminal" launch', () => { expect(terminal.claudePreflight.mock.calls.length).toBeGreaterThanOrEqual(2); }); - test('a launch-time preflight REJECTION falls back to a bare launch (security fail-safe)', async () => { + 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 = { @@ -285,10 +285,13 @@ describe('TerminalPanel "Open in terminal" launch', () => { 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(); + }); - 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(launchWrites(terminal.input).length).toBe(0); + 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 () => { @@ -397,16 +400,34 @@ describe('TerminalPanel "Open in terminal" launch', () => { expect(launchWrites(terminal.input, 'codex').length).toBe(0); }); - test('cursor probe UNKNOWN does not block the launch (parity with claude unknown)', async () => { + test('cursor probe UNKNOWN re-probes once; still-unknown suppresses + shows the banner', async () => { const { bridge, terminal, pushData } = 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 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 () => { + let calls = 0; + const { bridge, terminal, pushData } = 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"); }); - test('cliPreflight IPC rejection fail-opens: the launch is still written (the .catch path)', async () => { + test('cliPreflight IPC rejection suppresses the write (no raw command-not-found)', async () => { const { bridge, terminal, pushData } = makeBridge(WIRED); terminal.cliPreflight = mock(async () => { throw new Error('ipc channel closed'); @@ -416,7 +437,46 @@ describe('TerminalPanel "Open in terminal" launch', () => { await waitFor(() => expect(terminal.onData).toHaveBeenCalledTimes(1)); act(() => pushData({ ptyId: 'pty-1', data: '$ ' })); await waitFor(() => expect(terminal.cliPreflight).toHaveBeenCalledTimes(1)); - await waitFor(() => expect(launchWrites(terminal.input, 'codex').length).toBe(1)); - expect(launchWrites(terminal.input, 'codex')[0]).toBe("codex 'hi'\r"); + await screen.findByText(/Codex \(codex\) isn't installed/); + expect(launchWrites(terminal.input, 'codex').length).toBe(0); + }); + + 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(); + + 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(); + }); + expect(launchWrites(terminal.input).length).toBe(0); + await screen.findByText(/Claude Code \(claude\) isn't installed/); + }); + + 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(); + + 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/); }); }); diff --git a/packages/app/src/components/TerminalPanel.tsx b/packages/app/src/components/TerminalPanel.tsx index a1003166..86f9483f 100644 --- a/packages/app/src/components/TerminalPanel.tsx +++ b/packages/app/src/components/TerminalPanel.tsx @@ -336,10 +336,24 @@ function TerminalSession({ }; void bridge.terminal .claudePreflight() - .then((fresh) => writeClaude(fresh.mcpPreApprovable === true)) + .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) => { - console.warn('[terminal] claude pre-approval recheck failed', { err }); - writeClaude(false); + 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; @@ -355,22 +369,27 @@ function TerminalSession({ bridge.terminal.input(livePtyId, buildCliLaunchCommand(cli, prompt)); lastLaunchedNonceRef.current = nonce; // commit only after the write lands }; - void bridge.terminal - .cliPreflight(cli) - .then((res) => { - if (cancelled) return; - if (res.onPath === 'not-found') { - setMissingCli(cli); - lastLaunchedNonceRef.current = nonce; // banner handles remediation; consume - return; + 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); } - writeLaunch(); - }) - .catch((err) => { + if (cancelled) return; + if (res.onPath === 'present') writeLaunch(); + else suppress(); + } catch (err) { if (cancelled) return; console.warn('[terminal] cliPreflight failed', { cli, err }); - writeLaunch(); - }); + suppress(); + } + })(); return () => { cancelled = true; };