From 5cec2c2dfcae0700ecfee5b7e1d67200bb74d375 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:07:08 -0700 Subject: [PATCH 01/10] test: preserve Docker context in isolated VM recipes (#18884) --- tests/e2e/ephemeral-vm-provisioned-root.spec.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/e2e/ephemeral-vm-provisioned-root.spec.ts b/tests/e2e/ephemeral-vm-provisioned-root.spec.ts index 0dc51224bb3..394868e63be 100644 --- a/tests/e2e/ephemeral-vm-provisioned-root.spec.ts +++ b/tests/e2e/ephemeral-vm-provisioned-root.spec.ts @@ -1,6 +1,6 @@ import { execFileSync } from 'node:child_process' import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' -import { tmpdir } from 'node:os' +import { homedir, tmpdir } from 'node:os' import path from 'node:path' import { expect, test } from './helpers/orca-app' import { ensureDockerSshRelayImage } from './helpers/docker-ssh-relay-image' @@ -136,6 +136,8 @@ async function addRecipeRepo(page: Parameters[0], re function seedRecipeRepo(repoPath: string, target: DockerSshRelayTarget): string { const createScript = path.join(repoPath, 'create.sh') const destroyScript = path.join(repoPath, 'destroy.sh') + // The recipe's isolated HOME must still address the engine that owns the fixture container. + const docker = `docker --config ${shellQuote(process.env.DOCKER_CONFIG ?? path.join(homedir(), '.docker'))}` writeFileSync( createScript, `#!/usr/bin/env bash @@ -145,8 +147,8 @@ set -euo pipefail [ -n "\${ORCA_REPO_REF:-}" ] [ -n "\${ORCA_REPO_REF_HEAD:-}" ] [ -n "\${ORCA_REPO_BRANCH:-}" ] -docker exec ${shellQuote(target.containerName)} git -C ${shellQuote(DOCKER_SSH_RELAY_REMOTE_REPO_PATH)} cat-file -e "$ORCA_REPO_REF_HEAD^{commit}" -docker exec ${shellQuote(target.containerName)} git -C ${shellQuote(DOCKER_SSH_RELAY_REMOTE_REPO_PATH)} checkout -B "$ORCA_REPO_BRANCH" "$ORCA_REPO_REF_HEAD" >&2 +${docker} exec ${shellQuote(target.containerName)} git -C ${shellQuote(DOCKER_SSH_RELAY_REMOTE_REPO_PATH)} cat-file -e "$ORCA_REPO_REF_HEAD^{commit}" +${docker} exec ${shellQuote(target.containerName)} git -C ${shellQuote(DOCKER_SSH_RELAY_REMOTE_REPO_PATH)} checkout -B "$ORCA_REPO_BRANCH" "$ORCA_REPO_REF_HEAD" >&2 node -e 'console.log(JSON.stringify({schemaVersion:2,checkoutMode:"provisioned-root",connection:{type:"ssh",projectRoot:process.argv[1],target:{label:"Docker provisioned root",host:process.argv[2],port:Number(process.argv[3]),username:"root",identityFile:process.argv[4],identitiesOnly:true}}}))' ${shellQuote(DOCKER_SSH_RELAY_REMOTE_REPO_PATH)} ${shellQuote(target.host)} ${target.port} ${shellQuote(target.identityFile)} ` ) @@ -155,7 +157,7 @@ node -e 'console.log(JSON.stringify({schemaVersion:2,checkoutMode:"provisioned-r `#!/usr/bin/env bash set -euo pipefail cat >/dev/null -docker rm -f ${shellQuote(target.containerName)} >/dev/null +${docker} rm -f ${shellQuote(target.containerName)} >/dev/null ` ) chmodSync(createScript, 0o755) From cd70048092bb9665a88f32252772901eb9a14ec0 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:10:43 -0700 Subject: [PATCH 02/10] Fix favicon retention across same-origin navigations (#18879) * fix: retain favicons across same-origin navigations Move favicon clearing from did-start-loading to did-start-navigation and only clear when origin changes. Chromium re-announces favicons only when the icon URL list changes, so clearing on every load orphans same-origin navigations. Extract favicon URL validation into a shared module. * fix: drop favicon on cross-origin redirects When a same-origin navigation redirects to a different origin, the favicon should be cleared to prevent stale icons from displaying the wrong site's identity. --- .../src/components/browser-favicon.test.tsx | 57 +++++++ .../src/components/browser-favicon.tsx | 28 ++-- .../describe-page/browser-favicon-url.test.ts | 90 +++++++++++ .../describe-page/browser-favicon-url.ts | 63 ++++++++ .../bind-browser-page-webview-listeners.ts | 3 + .../browser-page-favicon-retention.test.ts | 149 ++++++++++++++++++ .../browser-page-webview-loading-handlers.ts | 6 +- ...rowser-page-webview-navigation-handlers.ts | 45 +++++- .../src/components/tab-bar/BrowserTab.tsx | 1 + 9 files changed, 415 insertions(+), 27 deletions(-) create mode 100644 src/renderer/src/components/browser-favicon.test.tsx create mode 100644 src/renderer/src/components/browser-pane/describe-page/browser-favicon-url.test.ts create mode 100644 src/renderer/src/components/browser-pane/describe-page/browser-favicon-url.ts create mode 100644 src/renderer/src/components/browser-pane/host-guest/browser-page-favicon-retention.test.ts diff --git a/src/renderer/src/components/browser-favicon.test.tsx b/src/renderer/src/components/browser-favicon.test.tsx new file mode 100644 index 00000000000..362cf15b46f --- /dev/null +++ b/src/renderer/src/components/browser-favicon.test.tsx @@ -0,0 +1,57 @@ +// @vitest-environment happy-dom +import { createElement } from 'react' +import { cleanup, fireEvent, render } from '@testing-library/react' +import { afterEach, expect, it } from 'vitest' +import { BrowserFavicon } from './browser-favicon' + +afterEach(cleanup) + +const faviconUrl = 'https://example.test/favicon.ico' +const icon = (loading = false, url: string | null = faviconUrl) => + createElement(BrowserFavicon, { faviconUrl: url, loading }) + +it('retries a failed icon after a same-origin reload completes', () => { + const view = render(icon()) + fireEvent.error(view.container.querySelector('img')!) + expect(view.container.querySelector('img')).toBeNull() + view.rerender(icon(true)) + expect(view.container.querySelector('img')).toBeNull() + view.rerender(icon(false)) + expect(view.container.querySelector('img')?.getAttribute('src')).toBe(faviconUrl) + + fireEvent.error(view.container.querySelector('img')!) + view.rerender(icon(false)) + expect(view.container.querySelector('img')).toBeNull() + view.rerender(icon(true)) + view.rerender(icon(false)) + expect(view.container.querySelector('img')).not.toBeNull() +}) + +it('keeps a working image mounted throughout a reload', () => { + const view = render(icon()) + const image = view.container.querySelector('img') + view.rerender(icon(true)) + expect(view.container.querySelector('img')).toBe(image) + view.rerender(icon(false)) + expect(view.container.querySelector('img')).toBe(image) +}) + +it('retries an image that failed during initial loading when loading finishes', () => { + const view = render(icon(true)) + fireEvent.error(view.container.querySelector('img')!) + view.rerender(icon(false)) + expect(view.container.querySelector('img')).not.toBeNull() +}) + +it('still resets failures when the favicon URL changes or clears', () => { + const view = render(icon()) + fireEvent.error(view.container.querySelector('img')!) + view.rerender(icon(false, null)) + view.rerender(icon()) + expect(view.container.querySelector('img')).not.toBeNull() + fireEvent.error(view.container.querySelector('img')!) + view.rerender(icon(false, 'https://other.test/favicon.ico')) + expect(view.container.querySelector('img')?.getAttribute('src')).toBe( + 'https://other.test/favicon.ico' + ) +}) diff --git a/src/renderer/src/components/browser-favicon.tsx b/src/renderer/src/components/browser-favicon.tsx index ecde4065f03..92b14a6ad64 100644 --- a/src/renderer/src/components/browser-favicon.tsx +++ b/src/renderer/src/components/browser-favicon.tsx @@ -1,34 +1,30 @@ import { useState } from 'react' import { Globe } from 'lucide-react' import { cn } from '@/lib/utils' - -function displayableFaviconUrl(faviconUrl: string | null | undefined): string | null { - const trimmed = faviconUrl?.trim() - if (!trimmed) { - return null - } - if (trimmed.startsWith('data:image/')) { - return trimmed - } - try { - const url = new URL(trimmed) - return url.protocol === 'http:' || url.protocol === 'https:' ? trimmed : null - } catch { - return null - } -} +import { displayableFaviconUrl } from './browser-pane/describe-page/browser-favicon-url' export function BrowserFavicon({ faviconUrl, + loading = false, className, fallbackClassName }: { faviconUrl: string | null | undefined + loading?: boolean className?: string fallbackClassName?: string }): React.JSX.Element { const displayUrl = displayableFaviconUrl(faviconUrl) const [failedUrl, setFailedUrl] = useState(null) + const [previousLoading, setPreviousLoading] = useState(loading) + + // Retry after navigation settles, when cookies and connectivity may have recovered. + if (previousLoading !== loading) { + setPreviousLoading(loading) + if (!loading) { + setFailedUrl(null) + } + } // Why: reset during render on any favicon identity change — including a clear to null while // a page loads — so navigating back to the same url retries instead of keeping the fallback. diff --git a/src/renderer/src/components/browser-pane/describe-page/browser-favicon-url.test.ts b/src/renderer/src/components/browser-pane/describe-page/browser-favicon-url.test.ts new file mode 100644 index 00000000000..4c295309174 --- /dev/null +++ b/src/renderer/src/components/browser-pane/describe-page/browser-favicon-url.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from 'vitest' +import { + browserNavigationLeavesFaviconOrigin, + displayableFaviconUrl, + pickDisplayableFaviconUrl +} from './browser-favicon-url' + +describe('displayableFaviconUrl', () => { + it('accepts http, https and image data urls', () => { + expect(displayableFaviconUrl('https://github.com/favicon.ico')).toBe( + 'https://github.com/favicon.ico' + ) + expect(displayableFaviconUrl('http://127.0.0.1:8765/favicon.ico')).toBe( + 'http://127.0.0.1:8765/favicon.ico' + ) + expect(displayableFaviconUrl(' data:image/png;base64,AAAA ')).toBe( + 'data:image/png;base64,AAAA' + ) + }) + + it('rejects the empty-icon sentinel and non-web schemes', () => { + expect(displayableFaviconUrl('data:,')).toBeNull() + expect(displayableFaviconUrl('chrome-extension://abc/icon.png')).toBeNull() + expect(displayableFaviconUrl('file:///tmp/icon.png')).toBeNull() + expect(displayableFaviconUrl('not a url')).toBeNull() + expect(displayableFaviconUrl(null)).toBeNull() + expect(displayableFaviconUrl(' ')).toBeNull() + }) +}) + +describe('pickDisplayableFaviconUrl', () => { + it('skips leading entries that cannot render', () => { + expect(pickDisplayableFaviconUrl(['data:,', 'https://example.com/icon.png'])).toBe( + 'https://example.com/icon.png' + ) + }) + + it('keeps the declaration order among usable entries', () => { + expect( + pickDisplayableFaviconUrl([ + 'https://github.githubassets.com/favicons/favicon.png', + 'https://github.githubassets.com/favicons/favicon.svg' + ]) + ).toBe('https://github.githubassets.com/favicons/favicon.png') + }) + + it('reports nothing for an absent or unusable list', () => { + expect(pickDisplayableFaviconUrl(undefined)).toBeNull() + expect(pickDisplayableFaviconUrl([])).toBeNull() + expect(pickDisplayableFaviconUrl(['data:,'])).toBeNull() + }) +}) + +describe('browserNavigationLeavesFaviconOrigin', () => { + it('keeps the icon across a same-origin navigation', () => { + expect( + browserNavigationLeavesFaviconOrigin( + 'https://github.com/alibaba/jvm-sandbox', + 'https://github.com/btraceio/btrace' + ) + ).toBe(false) + }) + + it('drops the icon when the origin changes', () => { + expect( + browserNavigationLeavesFaviconOrigin('https://github.com/nodejs/node', 'https://x.com/home') + ).toBe(true) + }) + + it('treats scheme and port as part of the origin', () => { + expect( + browserNavigationLeavesFaviconOrigin('http://localhost:3000/', 'http://localhost:4000/') + ).toBe(true) + expect( + browserNavigationLeavesFaviconOrigin('http://example.com/', 'https://example.com/') + ).toBe(true) + }) + + it('drops the icon when the destination cannot carry one', () => { + expect(browserNavigationLeavesFaviconOrigin('https://github.com/', 'about:blank')).toBe(true) + expect( + browserNavigationLeavesFaviconOrigin('https://github.com/', 'file:///tmp/report.html') + ).toBe(true) + }) + + it('keeps the icon when the document being left is unknown', () => { + expect(browserNavigationLeavesFaviconOrigin(null, 'https://github.com/nodejs/node')).toBe(false) + expect(browserNavigationLeavesFaviconOrigin('about:blank', 'https://github.com/')).toBe(false) + }) +}) diff --git a/src/renderer/src/components/browser-pane/describe-page/browser-favicon-url.ts b/src/renderer/src/components/browser-pane/describe-page/browser-favicon-url.ts new file mode 100644 index 00000000000..8e1c442068d --- /dev/null +++ b/src/renderer/src/components/browser-pane/describe-page/browser-favicon-url.ts @@ -0,0 +1,63 @@ +// Why this lives apart from the : Chromium only emits `page-favicon-updated` when a document's +// icon URL list *changes*, so both the chrome that renders an icon and the guest listeners that +// decide when to drop one have to agree on what counts as a usable icon and as a new site. + +export function displayableFaviconUrl(faviconUrl: string | null | undefined): string | null { + const trimmed = faviconUrl?.trim() + if (!trimmed) { + return null + } + // Why not a plain `data:` check: Chromium reports `data:,` for a page that declares no icon. + if (trimmed.startsWith('data:image/')) { + return trimmed + } + try { + const url = new URL(trimmed) + return url.protocol === 'http:' || url.protocol === 'https:' ? trimmed : null + } catch { + return null + } +} + +export function pickDisplayableFaviconUrl(favicons: readonly string[] | undefined): string | null { + // Why not favicons[0]: the first entry can be a `data:,` sentinel or a non-web scheme while a + // later entry is a real icon. + for (const candidate of favicons ?? []) { + const displayable = displayableFaviconUrl(candidate) + if (displayable) { + return displayable + } + } + return null +} + +function faviconOrigin(rawUrl: string | null | undefined): string | null { + if (!rawUrl) { + return null + } + try { + const url = new URL(rawUrl) + return url.protocol === 'http:' || url.protocol === 'https:' ? url.origin : null + } catch { + return null + } +} + +// Why the two sides are treated asymmetrically: a destination with no icon of its own (about:blank, +// file://, a doc preview) must drop the previous site's icon, but an unknown *origin* — a freshly +// attached guest that hasn't committed a document yet — is not evidence the icon is stale, and +// clearing there would strand a restored tab on the globe until its first paint. +export function browserNavigationLeavesFaviconOrigin( + fromUrl: string | null | undefined, + toUrl: string | null | undefined +): boolean { + const to = faviconOrigin(toUrl) + if (to === null) { + return true + } + const from = faviconOrigin(fromUrl) + if (from === null) { + return false + } + return from !== to +} diff --git a/src/renderer/src/components/browser-pane/host-guest/bind-browser-page-webview-listeners.ts b/src/renderer/src/components/browser-pane/host-guest/bind-browser-page-webview-listeners.ts index e80409fa821..f8699324889 100644 --- a/src/renderer/src/components/browser-pane/host-guest/bind-browser-page-webview-listeners.ts +++ b/src/renderer/src/components/browser-pane/host-guest/bind-browser-page-webview-listeners.ts @@ -116,6 +116,7 @@ export function bindBrowserPageWebviewListeners({ const { handleDidStartNavigation, + handleDidRedirectNavigation, handleFullDidNavigate, handleDidNavigateInPage, handleTitleUpdate, @@ -149,6 +150,7 @@ export function bindBrowserPageWebviewListeners({ webview.addEventListener('focus', dismissAddressBarSuggestions) webview.addEventListener('did-start-loading', handleDidStartLoading) webview.addEventListener('did-start-navigation', handleDidStartNavigation) + webview.addEventListener('did-redirect-navigation', handleDidRedirectNavigation) webview.addEventListener('did-stop-loading', handleDidStopLoading) // Why: close find only on full 'did-navigate', not the shared handler, which also fires on SPA in-page hash/pushState changes. const handleFindCloseOnNavigate = (): void => { @@ -186,6 +188,7 @@ export function bindBrowserPageWebviewListeners({ webview.removeEventListener('focus', dismissAddressBarSuggestions) webview.removeEventListener('did-start-loading', handleDidStartLoading) webview.removeEventListener('did-start-navigation', handleDidStartNavigation) + webview.removeEventListener('did-redirect-navigation', handleDidRedirectNavigation) webview.removeEventListener('did-stop-loading', handleDidStopLoading) webview.removeEventListener('did-navigate', handleFullDidNavigate) webview.removeEventListener('did-navigate', handleFindCloseOnNavigate) diff --git a/src/renderer/src/components/browser-pane/host-guest/browser-page-favicon-retention.test.ts b/src/renderer/src/components/browser-pane/host-guest/browser-page-favicon-retention.test.ts new file mode 100644 index 00000000000..924e7834319 --- /dev/null +++ b/src/renderer/src/components/browser-pane/host-guest/browser-page-favicon-retention.test.ts @@ -0,0 +1,149 @@ +import { describe, expect, it, vi } from 'vitest' +import { createBrowserPageWebviewNavigationHandlers } from './browser-page-webview-navigation-handlers' +import { createBrowserPageWebviewLoadingHandlers } from './browser-page-webview-loading-handlers' +import type { BrowserTabPageState } from '../describe-page/browser-page-types' + +const TAB_ID = 'tab-1' +const GITHUB_ICON = 'https://github.githubassets.com/favicons/favicon.png' + +function createHarness(startUrl: string) { + const updates: BrowserTabPageState[] = [] + const committedUrl = { current: startUrl } + const webview = { + getURL: () => committedUrl.current, + getTitle: () => 'title', + canGoBack: () => false, + canGoForward: () => false, + src: startUrl + } as unknown as Electron.WebviewTag + const faviconUrlRef = { current: null as string | null } + const onUpdatePageStateRef = { + current: (_tabId: string, next: BrowserTabPageState) => { + updates.push(next) + } + } + const ref = (value: T) => ({ current: value }) + const navigation = createBrowserPageWebviewNavigationHandlers({ + webview, + browserTabId: TAB_ID, + browserTabUrl: startUrl, + recoveryNavigationValidationRef: ref(null), + activeLoadFailureRef: ref(null), + // Why the destination, not the current document: Orca-driven navigations set this ref before + // assigning src, which is exactly the case the origin check must not read it for. + lastKnownWebviewUrlRef: ref(startUrl), + addressBarInputRef: ref(null), + onSetUrlRef: ref(vi.fn()), + onUpdatePageStateRef, + addBrowserHistoryEntryRef: ref(vi.fn()), + faviconUrlRef, + setAddressBarValue: vi.fn(), + annotationViewportBridgeTokenRef: ref('token'), + setBrowserOverlayViewport: vi.fn() + }) + const loading = createBrowserPageWebviewLoadingHandlers({ + webview, + browserTabId: TAB_ID, + faviconUrlRef, + browserTabUrlRef: ref(startUrl), + addressBarValueRef: ref(startUrl), + addressBarInputRef: ref(null), + activeLoadFailureRef: ref(null), + lastKnownWebviewUrlRef: ref(startUrl), + trackNextLoadingEventRef: ref(true), + keepAddressBarFocusRef: ref(false), + recoveryNavigationValidationRef: ref(null), + clearBrowserPageAnnotationsRef: ref(vi.fn()), + onUpdatePageStateRef, + onSetUrlRef: ref(vi.fn()), + setPendingAnnotationPayload: vi.fn(), + setBrowserOverlayViewport: vi.fn(), + setAddressBarValue: vi.fn(), + focusAddressBarNow: () => false + }) + + const navigateTo = (url: string): void => { + loading.handleDidStartLoading() + navigation.handleDidStartNavigation({ + isMainFrame: true, + isInPlace: false, + url + } as Electron.DidStartNavigationEvent) + committedUrl.current = url + } + + return { faviconUrlRef, updates, navigation, navigateTo, committedUrl } +} + +describe('favicon retention across navigations', () => { + it('keeps the icon when Chromium will not re-announce it for a same-origin load', () => { + const harness = createHarness('https://github.com/alibaba/jvm-sandbox') + harness.navigation.handleFaviconUpdate({ favicons: [GITHUB_ICON] }) + expect(harness.faviconUrlRef.current).toBe(GITHUB_ICON) + + // Chromium emits no page-favicon-updated here: the icon URL list is unchanged. + harness.navigateTo('https://github.com/btraceio/btrace') + + expect(harness.faviconUrlRef.current).toBe(GITHUB_ICON) + expect(harness.updates.some((update) => update.faviconUrl === null)).toBe(false) + }) + + it('drops the icon when the navigation leaves the origin', () => { + const harness = createHarness('https://github.com/nodejs/node') + harness.navigation.handleFaviconUpdate({ favicons: [GITHUB_ICON] }) + + harness.navigateTo('https://x.com/home') + + expect(harness.faviconUrlRef.current).toBeNull() + expect(harness.updates.at(-1)).toEqual({ faviconUrl: null }) + }) + + it('drops the icon when a same-origin navigation redirects to another origin', () => { + const harness = createHarness('https://github.com/nodejs/node') + harness.navigation.handleFaviconUpdate({ favicons: [GITHUB_ICON] }) + harness.navigateTo('https://github.com/login') + + harness.navigation.handleDidRedirectNavigation({ + isMainFrame: true, + isInPlace: false, + url: 'https://example.com/after-login' + } as Electron.DidRedirectNavigationEvent) + + expect(harness.faviconUrlRef.current).toBeNull() + expect(harness.updates.at(-1)).toEqual({ faviconUrl: null }) + }) + + it('does not clear on a same-document navigation', () => { + const harness = createHarness('https://github.com/nodejs/node') + harness.navigation.handleFaviconUpdate({ favicons: [GITHUB_ICON] }) + + harness.navigation.handleDidStartNavigation({ + isMainFrame: true, + isInPlace: true, + url: 'https://example.com/' + } as Electron.DidStartNavigationEvent) + + expect(harness.faviconUrlRef.current).toBe(GITHUB_ICON) + }) + + it('reports loading without touching the icon on did-start-loading', () => { + const harness = createHarness('https://github.com/nodejs/node') + harness.navigation.handleFaviconUpdate({ favicons: [GITHUB_ICON] }) + harness.updates.length = 0 + + harness.navigateTo('https://github.com/nodejs/undici') + + expect(harness.updates).toEqual([{ loading: true }]) + }) + + it('takes the first renderable icon rather than the first declared one', () => { + const harness = createHarness('https://example.com/') + harness.navigation.handleFaviconUpdate({ + favicons: ['data:,', 'https://example.com/icon.png'] + }) + expect(harness.faviconUrlRef.current).toBe('https://example.com/icon.png') + + harness.navigation.handleFaviconUpdate({ favicons: ['data:,'] }) + expect(harness.faviconUrlRef.current).toBeNull() + }) +}) diff --git a/src/renderer/src/components/browser-pane/host-guest/browser-page-webview-loading-handlers.ts b/src/renderer/src/components/browser-pane/host-guest/browser-page-webview-loading-handlers.ts index f263354e8c5..b6887f119eb 100644 --- a/src/renderer/src/components/browser-pane/host-guest/browser-page-webview-loading-handlers.ts +++ b/src/renderer/src/components/browser-pane/host-guest/browser-page-webview-loading-handlers.ts @@ -78,10 +78,10 @@ export function createBrowserPageWebviewLoadingHandlers({ if (!trackNextLoadingEventRef.current) { return } - faviconUrlRef.current = null + // Why the favicon isn't cleared here: it is dropped on the cross-origin did-start-navigation + // instead, because Chromium won't re-announce an unchanged icon for a same-origin load. onUpdatePageStateRef.current(browserTabId, { - loading: true, - faviconUrl: null + loading: true }) } diff --git a/src/renderer/src/components/browser-pane/host-guest/browser-page-webview-navigation-handlers.ts b/src/renderer/src/components/browser-pane/host-guest/browser-page-webview-navigation-handlers.ts index dbb47ae4acb..240e763cf63 100644 --- a/src/renderer/src/components/browser-pane/host-guest/browser-page-webview-navigation-handlers.ts +++ b/src/renderer/src/components/browser-pane/host-guest/browser-page-webview-navigation-handlers.ts @@ -13,6 +13,10 @@ import { isChromiumErrorPage, toDisplayUrl } from '../describe-page/browser-page-url-display' +import { + browserNavigationLeavesFaviconOrigin, + pickDisplayableFaviconUrl +} from '../describe-page/browser-favicon-url' import type { BrowserPageNavigateEvent, BrowserPageRecoveryNavigationValidation, @@ -41,6 +45,7 @@ export type BrowserPageWebviewNavigationHandlersArgs = { export type BrowserPageWebviewNavigationHandlers = { handleDidStartNavigation: (event: Electron.DidStartNavigationEvent) => void + handleDidRedirectNavigation: (event: Electron.DidRedirectNavigationEvent) => void handleFullDidNavigate: (event: BrowserPageNavigateEvent) => void handleDidNavigateInPage: (event: BrowserPageNavigateEvent) => void handleTitleUpdate: (event: { title?: string }) => void @@ -64,6 +69,28 @@ export function createBrowserPageWebviewNavigationHandlers({ annotationViewportBridgeTokenRef, setBrowserOverlayViewport }: BrowserPageWebviewNavigationHandlersArgs): BrowserPageWebviewNavigationHandlers { + const clearFaviconIfOriginChanges = ( + event: Electron.DidStartNavigationEvent | Electron.DidRedirectNavigationEvent + ): void => { + if (!event.isMainFrame || event.isInPlace || !event.url) { + return + } + const browserStartedUrl = redactKagiSessionToken(event.url) + const startedUrl = normalizeBrowserNavigationUrl(browserStartedUrl) ?? browserStartedUrl + // Why getURL() and not lastKnownWebviewUrlRef: Orca-driven navigations point that ref at the + // destination before assigning src, so it can't identify the document being left. + let committedUrl: string | null = null + try { + committedUrl = webview.getURL() || null + } catch { + // Why: a guest that hasn't attached yet rejects getURL(); an unknown origin keeps the icon. + } + if (browserNavigationLeavesFaviconOrigin(committedUrl, startedUrl)) { + faviconUrlRef.current = null + onUpdatePageStateRef.current(browserTabId, { faviconUrl: null }) + } + } + const handleDidStartNavigation = (event: Electron.DidStartNavigationEvent): void => { if (!event.isMainFrame || event.isInPlace || !event.url) { return @@ -74,6 +101,14 @@ export function createBrowserPageWebviewNavigationHandlers({ if (pendingRecoveryNavigation?.targetUrl === startedUrl) { pendingRecoveryNavigation.started = true } + // Why here and not on did-start-loading: Chromium re-announces a favicon only when the icon URL + // list changes, so clearing on every load strands same-origin navigations with no icon and no + // event that would ever restore one. + clearFaviconIfOriginChanges(event) + } + + const handleDidRedirectNavigation = (event: Electron.DidRedirectNavigationEvent): void => { + clearFaviconIfOriginChanges(event) } const handleDidNavigate = ( @@ -136,14 +171,7 @@ export function createBrowserPageWebviewNavigationHandlers({ } const handleFaviconUpdate = (event: { favicons?: string[] }): void => { - const faviconUrl = event.favicons?.[0] ?? null - faviconUrlRef.current = - faviconUrl && - (faviconUrl.startsWith('https://') || - faviconUrl.startsWith('http://') || - faviconUrl.startsWith('data:image/')) - ? faviconUrl - : null + faviconUrlRef.current = pickDisplayableFaviconUrl(event.favicons) onUpdatePageStateRef.current(browserTabId, { faviconUrl: faviconUrlRef.current }) } @@ -175,6 +203,7 @@ export function createBrowserPageWebviewNavigationHandlers({ return { handleDidStartNavigation, + handleDidRedirectNavigation, handleFullDidNavigate, handleDidNavigateInPage, handleTitleUpdate, diff --git a/src/renderer/src/components/tab-bar/BrowserTab.tsx b/src/renderer/src/components/tab-bar/BrowserTab.tsx index b72f27287f9..507b738318a 100644 --- a/src/renderer/src/components/tab-bar/BrowserTab.tsx +++ b/src/renderer/src/components/tab-bar/BrowserTab.tsx @@ -191,6 +191,7 @@ export default function BrowserTab({ muted-foreground made the icon read as "disabled" in practice. */} From dce5ebd83da1ab2fb613d063b8df5af8d95e00cb Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:18:22 -0700 Subject: [PATCH 03/10] test: isolate native crash restoration and refresh stale fixtures (#18883) * test: isolate native crash restoration and seed current integration facts * test: await scoped GitLab preflight before URL transition checks --- tests/e2e/electron-home-isolation.spec.ts | 3 +- tests/e2e/feature-wall.spec.ts | 67 +++++++++++-------- .../github-url-smart-input-transition.spec.ts | 21 +++--- tests/e2e/helpers/electron-launch-args.ts | 4 ++ .../helpers/electron-launch-args.unit.test.ts | 13 +++- 5 files changed, 63 insertions(+), 45 deletions(-) diff --git a/tests/e2e/electron-home-isolation.spec.ts b/tests/e2e/electron-home-isolation.spec.ts index 65aa1b7dc10..2fae6f42eec 100644 --- a/tests/e2e/electron-home-isolation.spec.ts +++ b/tests/e2e/electron-home-isolation.spec.ts @@ -1,4 +1,5 @@ import type { ElectronApplication } from '@stablyai/playwright-test' +import { realpathSync } from 'node:fs' import path from 'node:path' import { expect, test } from './helpers/orca-app' @@ -23,7 +24,7 @@ async function readElectronHomeState(electronApp: ElectronApplication) { // HOME boundary and that real-home routing lands inside the disposable profile. test('isolates Electron and Codex from the developer home by default', async ({ electronApp }) => { const state = await readElectronHomeState(electronApp) - const expectedHome = path.join(state.userDataDir!, 'home') + const expectedHome = realpathSync.native(path.join(state.userDataDir!, 'home')) expect(state.appHome).toBe(expectedHome) expect(state.nodeHome).toBe(expectedHome) diff --git a/tests/e2e/feature-wall.spec.ts b/tests/e2e/feature-wall.spec.ts index 428ce5d7996..fb422ec8bf8 100644 --- a/tests/e2e/feature-wall.spec.ts +++ b/tests/e2e/feature-wall.spec.ts @@ -179,9 +179,40 @@ test.describe('Feature tour modal', () => { }) test('does not pre-check configured workflows until the user visits them', async ({ - orcaPage + orcaPage, + electronApp }) => { - await orcaPage.evaluate(() => { + await electronApp.evaluate( + ({ ipcMain }, preflightStatus) => { + ipcMain.removeHandler('preflight:check') + ipcMain.handle('preflight:check', () => preflightStatus) + ipcMain.removeHandler('linear:status') + ipcMain.handle('linear:status', () => ({ connected: false, viewer: null })) + ipcMain.removeHandler('jira:status') + ipcMain.handle('jira:status', () => ({ connected: false, viewer: null })) + }, + { + git: { installed: true }, + gh: { installed: true, authenticated: true }, + glab: { installed: false, authenticated: false }, + bitbucket: { configured: false, authenticated: false, account: null }, + azureDevOps: { + configured: false, + authenticated: false, + account: null, + baseUrl: null, + tokenConfigured: false + }, + gitea: { + configured: false, + authenticated: false, + account: null, + baseUrl: null, + tokenConfigured: false + } + } + ) + await orcaPage.evaluate(async () => { for (const key of [ 'orca.featureWall.visitedWorkflows.v1', 'orca.featureWall.visitedAgentSteps.v1', @@ -198,32 +229,12 @@ test.describe('Feature tour modal', () => { if (!store) { throw new Error('window.__store is not available') } - store.setState({ - preflightStatus: { - git: { installed: true }, - gh: { installed: true, authenticated: true }, - glab: { installed: false, authenticated: false }, - bitbucket: { configured: false, authenticated: false, account: null }, - azureDevOps: { - configured: false, - authenticated: false, - account: null, - baseUrl: null, - tokenConfigured: false - }, - gitea: { - configured: false, - authenticated: false, - account: null, - baseUrl: null, - tokenConfigured: false - } - }, - preflightStatusChecked: true, - preflightStatusLoading: false, - linearStatus: { connected: false, viewer: null }, - linearStatusChecked: true - }) + // Seed through the status actions so each result gets the current execution context. + await Promise.all([ + store.getState().refreshPreflightStatus({ force: true }), + store.getState().checkLinearConnection(true), + store.getState().checkJiraConnection() + ]) store.getState().openModal('feature-wall', { source: 'help_menu' }) }) diff --git a/tests/e2e/github-url-smart-input-transition.spec.ts b/tests/e2e/github-url-smart-input-transition.spec.ts index e078007bd78..f042c4ef676 100644 --- a/tests/e2e/github-url-smart-input-transition.spec.ts +++ b/tests/e2e/github-url-smart-input-transition.spec.ts @@ -203,6 +203,12 @@ async function installHeldGitLabLookup( __releaseGitLabUrlLookup?: () => void } fixture.__gitlabUrlLookupStarted = false + ipcMain.removeHandler('preflight:check') + ipcMain.handle('preflight:check', () => ({ + git: { installed: true }, + gh: { installed: true, authenticated: true }, + glab: { installed: true, authenticated: true } + })) ipcMain.removeHandler('gitlab:listMRs') ipcMain.handle('gitlab:listMRs', () => ({ items: [wrongItem], @@ -221,23 +227,12 @@ async function installHeldGitLabLookup( }, { wrongItem: GITLAB_WRONG_ITEM, targetItem: GITLAB_TARGET_ITEM } ) - await page.evaluate(() => { + await page.evaluate(async () => { const store = window.__store if (!store) { throw new Error('window.__store is not available') } - const state = store.getState() - if (!state.preflightStatusContextKey) { - throw new Error('preflight context is not ready') - } - store.setState({ - preflightStatus: { - git: state.preflightStatus?.git ?? { installed: true }, - gh: state.preflightStatus?.gh ?? { installed: true, authenticated: true }, - glab: { installed: true, authenticated: true } - }, - preflightStatusChecked: true - }) + await store.getState().refreshPreflightStatus({ force: true }) }) } diff --git a/tests/e2e/helpers/electron-launch-args.ts b/tests/e2e/helpers/electron-launch-args.ts index fc2ff1e81aa..9128a5fb551 100644 --- a/tests/e2e/helpers/electron-launch-args.ts +++ b/tests/e2e/helpers/electron-launch-args.ts @@ -7,6 +7,10 @@ export function getOrcaElectronLaunchArgs(mainPath: string, headful: boolean): s // these Chromium switches startup can block before the first renderer target. const keychainArgs = process.platform === 'darwin' ? ['--password-store=basic', '--use-mock-keychain'] : [] + if (process.platform === 'darwin') { + // Crash tests must not block later launches on AppKit's saved-window recovery dialog. + return [...keychainArgs, appPath, '-ApplePersistenceIgnoreState', 'YES'] + } if (headful || process.platform !== 'linux') { return [...keychainArgs, appPath] } diff --git a/tests/e2e/helpers/electron-launch-args.unit.test.ts b/tests/e2e/helpers/electron-launch-args.unit.test.ts index ed981951f14..636299fbc4e 100644 --- a/tests/e2e/helpers/electron-launch-args.unit.test.ts +++ b/tests/e2e/helpers/electron-launch-args.unit.test.ts @@ -8,10 +8,17 @@ describe('getOrcaElectronLaunchArgs', () => { const mainPath = join(root, 'out', 'main', 'index.js') const args = getOrcaElectronLaunchArgs(mainPath, true) - expect(args.at(-1)).toBe(root) if (process.platform === 'darwin') { - expect(args.slice(0, -1)).toEqual(['--password-store=basic', '--use-mock-keychain']) + expect(args).toEqual([ + '--password-store=basic', + '--use-mock-keychain', + root, + '-ApplePersistenceIgnoreState', + 'YES' + ]) + } else { + expect(args.at(-1)).toBe(root) } - expect(getOrcaElectronLaunchArgs(mainPath, false).at(-1)).toBe(root) + expect(getOrcaElectronLaunchArgs(mainPath, false)).toContain(root) }) }) From 2afc8b55ef41de499962ffb9b08760dce34144a7 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:34:35 -0700 Subject: [PATCH 04/10] test: pin worker visibility fixture command and handle (#18897) --- ...tration-worker-terminal-visibility.spec.ts | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/e2e/orchestration-worker-terminal-visibility.spec.ts b/tests/e2e/orchestration-worker-terminal-visibility.spec.ts index af603c8ca1f..32b0013ff5a 100644 --- a/tests/e2e/orchestration-worker-terminal-visibility.spec.ts +++ b/tests/e2e/orchestration-worker-terminal-visibility.spec.ts @@ -11,6 +11,10 @@ import { waitForSessionReady } from './helpers/store' import { waitForActivePaneHookDescriptor, waitForActivePanePtyId } from './helpers/terminal' +import { + buildFakeAgentCommandOverride, + FAKE_AGENT_WINDOWS_SHELL +} from './helpers/fake-agent-command-override' import { RuntimeClient } from '../../src/cli/runtime-client' import type { RuntimeTerminalListResult, RuntimeTerminalRead } from '../../src/shared/runtime-types' @@ -111,6 +115,22 @@ test('worker-start preserves one live inactive worker across workspace re-entry' electronApp }) => { await waitForSessionReady(orcaPage) + await orcaPage.evaluate( + async ({ command, windowsShell }) => { + const state = window.__store!.getState() + await state.updateSettings({ + agentCmdOverrides: { ...state.settings?.agentCmdOverrides, codex: command }, + terminalWindowsShell: windowsShell + }) + }, + { + command: buildFakeAgentCommandOverride( + path.join(fakeCliDir, process.platform === 'win32' ? 'codex.cmd' : 'codex') + ), + windowsShell: FAKE_AGENT_WINDOWS_SHELL + } + ) + const worktreeId = await waitForActiveWorktree(orcaPage) await ensureTerminalVisible(orcaPage) const coordinatorTabId = await getActiveTabId(orcaPage) @@ -160,7 +180,7 @@ test('worker-start preserves one live inactive worker across workspace re-entry' const terminals = await client.call('terminal.list') const workerTerminal = terminals.result.terminals.find( - (terminal) => terminal.title === 'Codex Ready' + (terminal) => terminal.handle === workerHandle ) expect(workerTerminal?.tabId).toBeTruthy() expect(workerTerminal?.leafId).toBeTruthy() From 51eed5a1bc6e7593076d6fe1ee3e911db6a3493b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:35:45 -0700 Subject: [PATCH 05/10] feat(cli): report SSH host platforms (#18896) * feat(cli): report SSH host platforms * feat(cli): include SSH connection status * fix(cli): preserve unknown SSH connection state --- docs/site/content/docs/cli/reference.mdx | 2 +- src/cli/format.ts | 20 +++++++- src/cli/handlers/environment.ts | 13 ++++- src/cli/host-selector-alternatives.test.ts | 18 +++++++ src/cli/host-selector-alternatives.ts | 38 ++++++++++++++- .../index-local-command-routing-flags.test.ts | 5 +- src/cli/specs/environment.ts | 2 + src/main/runtime/rpc/methods/ssh.test.ts | 48 +++++++++++++++++-- src/main/runtime/rpc/methods/ssh.ts | 17 +++++-- src/shared/ssh-types.ts | 11 ++++- 10 files changed, 157 insertions(+), 17 deletions(-) diff --git a/docs/site/content/docs/cli/reference.mdx b/docs/site/content/docs/cli/reference.mdx index 0f24ca34192..5cbf19b82f9 100644 --- a/docs/site/content/docs/cli/reference.mdx +++ b/docs/site/content/docs/cli/reference.mdx @@ -63,7 +63,7 @@ List every machine the current Orca host can target and the selector for each on orca host list --json ``` -The result includes this machine, its registered [SSH targets](/docs/ssh), and paired [Remote Orca Servers](/docs/remote-servers). Use `--host local` for this machine, `--host ssh:` for an SSH target, and `--environment ` for a paired server. SSH labels and paired-server names also resolve when they are unique; use the IDs from `host list` when names collide. If you put a machine name on the wrong selector, Orca reports the matching machine and the flag to use instead of returning an empty result. +The result includes this machine, its registered [SSH targets](/docs/ssh), and paired [Remote Orca Servers](/docs/remote-servers). Use `--host local` for this machine, `--host ssh:` for an SSH target, and `--environment ` for a paired server. SSH rows include the detected remote platform (`linux`, `darwin`, or `win32`) after the target connects; older or disconnected targets report `platform unknown`. They also include `connected` and, when known, the SSH lifecycle `connectionStatus`. SSH labels and paired-server names also resolve when they are unique; use the IDs from `host list` when names collide. If you put a machine name on the wrong selector, Orca reports the matching machine and the flag to use instead of returning an empty result. ## Runtime commands diff --git a/src/cli/format.ts b/src/cli/format.ts index 0a297138364..1487a69eea0 100644 --- a/src/cli/format.ts +++ b/src/cli/format.ts @@ -220,6 +220,9 @@ export type HostListEntry = { name: string id: string selector: string + platform?: string + connected?: boolean + connectionStatus?: string } // Why: the selector column is the point of this command — the name alone is what callers already @@ -231,10 +234,25 @@ export function formatHostList(result: { hosts: HostListEntry[] }): string { environment: 'orca server' } return result.hosts - .map((host) => `${kindLabel[host.kind].padEnd(11)} ${host.name} -> ${host.selector}`) + .map( + (host) => + `${kindLabel[host.kind].padEnd(11)} ${host.name} ${host.platform ?? 'platform unknown'} ${formatHostConnection(host)} -> ${host.selector}` + ) .join('\n') } +function formatHostConnection(host: HostListEntry): string { + if (host.kind !== 'ssh') { + return '' + } + if (host.connected === undefined) { + return `connection unknown${host.connectionStatus ? ` (${host.connectionStatus})` : ''}` + } + return host.connected + ? `connected${host.connectionStatus ? ` (${host.connectionStatus})` : ''}` + : `not connected${host.connectionStatus ? ` (${host.connectionStatus})` : ''}` +} + export function formatCliStatus(status: CliStatusResult): string { return [ ...(status.target && status.target.kind === 'environment' diff --git a/src/cli/handlers/environment.ts b/src/cli/handlers/environment.ts index 37b437af2c3..181b2947f98 100644 --- a/src/cli/handlers/environment.ts +++ b/src/cli/handlers/environment.ts @@ -50,10 +50,19 @@ export const ENVIRONMENT_HANDLERS: Record = { kind: 'ssh' as const, name: target.label, id: target.id, - selector: `--host ssh:${target.id}` + selector: `--host ssh:${target.id}`, + ...(target.connected === undefined ? {} : { connected: target.connected }), + ...(target.connectionStatus ? { connectionStatus: target.connectionStatus } : {}), + ...(target.remotePlatform ? { platform: target.remotePlatform } : {}) })) const hosts = [ - { kind: 'local' as const, name: 'this machine', id: 'local', selector: '--host local' }, + { + kind: 'local' as const, + name: 'this machine', + id: 'local', + selector: '--host local', + platform: process.platform + }, ...sshTargets, ...environments ] diff --git a/src/cli/host-selector-alternatives.test.ts b/src/cli/host-selector-alternatives.test.ts index bc4fadd05c1..9460931a083 100644 --- a/src/cli/host-selector-alternatives.test.ts +++ b/src/cli/host-selector-alternatives.test.ts @@ -118,6 +118,24 @@ describe('listSshTargets', () => { expect(call).toHaveBeenCalledWith('ssh.listTargets') }) + it('enriches legacy target rows from host-owned connection state', async () => { + const { RuntimeClientError } = await import('./runtime/types.js') + const call = vi.fn(async (method: string) => { + if (method === 'ssh.listTargetSummaries') { + throw new RuntimeClientError('method_not_found', 'Unknown method') + } + if (method === 'ssh.getState') { + return { result: { state: { status: 'connected', remotePlatform: 'win32' } } } + } + return { result: { targets: SSH_TARGETS } } + }) + + await expect(listSshTargets({ call } as unknown as RuntimeClient)).resolves.toEqual([ + { ...SSH_TARGETS[0], connected: true, connectionStatus: 'connected', remotePlatform: 'win32' } + ]) + expect(call).toHaveBeenCalledWith('ssh.getState', { targetId: SSH_TARGETS[0].id }) + }) + // Why: this only ever runs to enrich an error we are already reporting; a failure here must // not replace that error with a confusing one about SSH enumeration. it('returns nothing rather than masking the error it was enriching', async () => { diff --git a/src/cli/host-selector-alternatives.ts b/src/cli/host-selector-alternatives.ts index fd5abecbc18..f42fec88aee 100644 --- a/src/cli/host-selector-alternatives.ts +++ b/src/cli/host-selector-alternatives.ts @@ -1,6 +1,12 @@ import type { RuntimeClient } from './runtime-client' -export type SshTargetSummary = { id: string; label: string } +export type SshTargetSummary = { + id: string + label: string + remotePlatform?: 'linux' | 'darwin' | 'win32' + connected?: boolean + connectionStatus?: string +} export type EnvironmentSummary = { id: string; name: string } export type HostAlternatives = { @@ -106,7 +112,7 @@ export async function listSshTargets(client: RuntimeClient): Promise('ssh.listTargets') - return legacy.result.targets + return await enrichLegacySshTargetStates(client, legacy.result.targets) } catch { return [] } @@ -115,6 +121,34 @@ export async function listSshTargets(client: RuntimeClient): Promise { + return Promise.all( + targets.map(async (target) => { + try { + const response = await client.call<{ + state: { + status?: string + remotePlatform?: 'linux' | 'darwin' | 'win32' + } | null + }>('ssh.getState', { targetId: target.id }) + const state = response.result.state + return { + ...target, + ...(state?.status === undefined + ? {} + : { connected: state.status === 'connected', connectionStatus: state.status }), + ...(state?.remotePlatform === undefined ? {} : { remotePlatform: state.remotePlatform }) + } + } catch { + return target + } + }) + ) +} + // Why: `--host ssh:` was never validated, so an unknown target answered ok:true with an // empty list — the same silent wrong-machine answer that `runtime:` ids used to give. And since // target ids are machine-generated (`ssh--`), the label a caller actually diff --git a/src/cli/index-local-command-routing-flags.test.ts b/src/cli/index-local-command-routing-flags.test.ts index b8db44915d2..1a195cc52ff 100644 --- a/src/cli/index-local-command-routing-flags.test.ts +++ b/src/cli/index-local-command-routing-flags.test.ts @@ -48,7 +48,7 @@ import { main } from './index' import { okFixture, queueFixtures } from './test-fixtures' import { pairRuntimeEnvironment, useWorktreeAwarenessEnvironment } from './index-test-harness' -const SSH_TARGET = { id: 'ssh-1777360569033-yvz2mp', label: 'openclaw' } +const SSH_TARGET = { id: 'ssh-1777360569033-yvz2mp', label: 'openclaw', remotePlatform: 'win32' } /** Every SSH-target lookup answers with the one target only this machine's runtime knows about. */ function queueSshTargetLookups(count: number): void { @@ -82,6 +82,9 @@ describe('runtime-selector flags on locally pinned CLI commands', () => { SSH_TARGET.id, 'env-m4air' ]) + expect( + printed.result.hosts.find((host: { id: string }) => host.id === SSH_TARGET.id).platform + ).toBe('win32') // The tell: `runtimeId: local` is only honest if no routed client was ever built. expect(runtimeClientConstructorMock).toHaveBeenCalledWith(null, null) }) diff --git a/src/cli/specs/environment.ts b/src/cli/specs/environment.ts index 7bf90615270..64efbe802b9 100644 --- a/src/cli/specs/environment.ts +++ b/src/cli/specs/environment.ts @@ -10,6 +10,8 @@ export const ENVIRONMENT_COMMAND_SPECS: CommandSpec[] = [ notes: [ 'Answers "what can I target and what do I pass" in one place: this machine, the SSH targets registered on it, and the Orca servers paired with it.', 'The three kinds are reached differently. A paired Orca server is a connection, selected with --environment . An SSH target is a machine the connected Orca host reaches, selected with --host ssh:. Passing one where the other belongs is the most common way to get an empty or missing-host answer.', + 'SSH rows include the detected remote platform after that target has connected (linux, darwin, or win32); disconnected or older targets report platform unknown.', + 'SSH rows also include whether the target is currently connected and its lifecycle status when known.', "SSH targets are read from this machine's own Orca runtime, so this lists that machine's targets and not another server's. Run `orca host list` on the other machine to see the targets registered there.", '--environment and --pairing-code are rejected rather than ignored: paired servers come from this machine\u2019s pairing store, so a routed answer would describe two machines at once.' ], diff --git a/src/main/runtime/rpc/methods/ssh.test.ts b/src/main/runtime/rpc/methods/ssh.test.ts index 450375e48f8..04293d3cca4 100644 --- a/src/main/runtime/rpc/methods/ssh.test.ts +++ b/src/main/runtime/rpc/methods/ssh.test.ts @@ -116,6 +116,34 @@ describe('ssh RPC methods', () => { } ] listRegisteredSshTargetsMock.mockReturnValueOnce(targets) + getRegisteredSshStateMock.mockReturnValueOnce({ status: 'connected', remotePlatform: 'win32' }) + const runtime = { getRuntimeId: () => 'test-runtime' } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: SSH_METHODS }) + + const response = await dispatcher.dispatch(makeRequest('ssh.listTargetSummaries')) + + expect(response).toMatchObject({ + ok: true, + result: { + targets: [ + { + id: 'ssh-1', + label: 'Dev box', + connected: true, + connectionStatus: 'connected', + remotePlatform: 'win32' + } + ] + } + }) + expect(JSON.stringify(response)).not.toContain('dev.internal') + expect(JSON.stringify(response)).not.toContain('/secret/key') + expect(JSON.stringify(response)).not.toContain('bastion') + }) + + it('does not invent a platform before the SSH host has been detected', async () => { + listRegisteredSshTargetsMock.mockReturnValueOnce([{ id: 'ssh-1', label: 'Dev box' }]) + getRegisteredSshStateMock.mockReturnValueOnce(undefined) const runtime = { getRuntimeId: () => 'test-runtime' } as unknown as OrcaRuntimeService const dispatcher = new RpcDispatcher({ runtime, methods: SSH_METHODS }) @@ -125,9 +153,23 @@ describe('ssh RPC methods', () => { ok: true, result: { targets: [{ id: 'ssh-1', label: 'Dev box' }] } }) - expect(JSON.stringify(response)).not.toContain('dev.internal') - expect(JSON.stringify(response)).not.toContain('/secret/key') - expect(JSON.stringify(response)).not.toContain('bastion') + expect(JSON.stringify(response)).not.toContain('remotePlatform') + }) + + it('reports disconnected lifecycle states without calling them connected', async () => { + listRegisteredSshTargetsMock.mockReturnValueOnce([{ id: 'ssh-1', label: 'Dev box' }]) + getRegisteredSshStateMock.mockReturnValueOnce({ status: 'reconnecting' }) + const runtime = { getRuntimeId: () => 'test-runtime' } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: SSH_METHODS }) + + const response = await dispatcher.dispatch(makeRequest('ssh.listTargetSummaries')) + + expect(response).toMatchObject({ + ok: true, + result: { + targets: [{ id: 'ssh-1', connected: false, connectionStatus: 'reconnecting' }] + } + }) }) it('redacts the legacy target response for older clients', async () => { diff --git a/src/main/runtime/rpc/methods/ssh.ts b/src/main/runtime/rpc/methods/ssh.ts index 2e0a4e4f4ab..e6cb6b47b50 100644 --- a/src/main/runtime/rpc/methods/ssh.ts +++ b/src/main/runtime/rpc/methods/ssh.ts @@ -15,11 +15,18 @@ const SshTarget = z.object({ // Why: `generation` stays optional on the wire — an old server simply omits it and its rows key on target id alone. function listRegisteredSshTargetSummaries(): SshTargetSummary[] { - return listRegisteredSshTargets().map(({ id, label, generation }) => ({ - id, - label, - ...(generation === undefined ? {} : { generation }) - })) + return listRegisteredSshTargets().map(({ id, label, generation }) => { + const state = getRegisteredSshState(id) + const remotePlatform = state?.remotePlatform + return { + id, + label, + ...(generation === undefined ? {} : { generation }), + connected: state?.status === 'connected', + ...(state?.status === undefined ? {} : { connectionStatus: state.status }), + ...(remotePlatform === undefined ? {} : { remotePlatform }) + } + }) } export const SSH_METHODS: RpcMethod[] = [ diff --git a/src/shared/ssh-types.ts b/src/shared/ssh-types.ts index f234b1c578a..566bc5e17f5 100644 --- a/src/shared/ssh-types.ts +++ b/src/shared/ssh-types.ts @@ -65,8 +65,15 @@ export type SshTarget = { export type SshTargetCreateInput = Omit export type SshTargetUpdateInput = Partial -/** Public target identity safe to mirror to a paired client. */ -export type SshTargetSummary = Pick +/** Public target identity and observed host metadata safe to mirror to a paired client. */ +export type SshTargetSummary = Pick & { + /** The SSH host's OS, when it has connected and the relay has detected it. */ + remotePlatform?: SshRemotePlatform + /** Whether the target currently has a host-owned connected SSH lifecycle. */ + connected?: boolean + /** Current SSH lifecycle state, when the desktop has one for this target. */ + connectionStatus?: SshConnectionStatus +} /** Identity of a removed SSH target, recorded so that re-adding the same host * can re-point orphaned repos/worktrees from the old (deleted) target id to From 7bec98466bb314aa213c7ed7302e40451ea304dd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 15:04:52 -0700 Subject: [PATCH 06/10] test: canonicalize setup fixture paths before worktree lookup (#18912) --- tests/e2e/setup-script-import.spec.ts | 4 ++-- ...script-prompt-unreadable-orca-yaml.spec.ts | 19 ++++++++----------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/tests/e2e/setup-script-import.spec.ts b/tests/e2e/setup-script-import.spec.ts index 180b34f85aa..340258c884b 100644 --- a/tests/e2e/setup-script-import.spec.ts +++ b/tests/e2e/setup-script-import.spec.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'node:child_process' -import { mkdirSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, realpathSync, rmSync, writeFileSync } from 'node:fs' import path from 'node:path' import type { Locator, Page } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' @@ -108,7 +108,7 @@ async function addAndActivateRepo(page: Page, repoPath: string): Promise state.setActiveWorktree(worktree.id) state.setSidebarOpen(true) return addedRepo.id - }, repoPath) + }, realpathSync.native(repoPath)) } async function openRepoSettings(page: Page, repoId: string): Promise { diff --git a/tests/e2e/setup-script-prompt-unreadable-orca-yaml.spec.ts b/tests/e2e/setup-script-prompt-unreadable-orca-yaml.spec.ts index 151a4b02bbc..199694b961a 100644 --- a/tests/e2e/setup-script-prompt-unreadable-orca-yaml.spec.ts +++ b/tests/e2e/setup-script-prompt-unreadable-orca-yaml.spec.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'node:child_process' -import { mkdirSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, realpathSync, rmSync, writeFileSync } from 'node:fs' import path from 'node:path' import type { ElectronApplication, Page } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' @@ -112,17 +112,10 @@ async function addRepoAndActivateMainWorktree( if (!store) { throw new Error('window.__store is not available') } - const normalize = (value: string): string => - value.startsWith('/private/var/') ? value.slice('/private'.length) : value - const state = store.getState() const worktrees = state.worktreesByRepo[targetRepoId] ?? [] - const mainWorktree = worktrees.find( - (entry) => normalize(entry.path) === normalize(targetRepoPath) - ) - const featureWorktree = worktrees.find( - (entry) => normalize(entry.path) === normalize(targetFeaturePath) - ) + const mainWorktree = worktrees.find((entry) => entry.path === targetRepoPath) + const featureWorktree = worktrees.find((entry) => entry.path === targetFeaturePath) if (!mainWorktree || !featureWorktree) { throw new Error( `Missing worktrees for ${targetRepoPath}: ${worktrees.map((entry) => entry.path).join(', ')}` @@ -145,7 +138,11 @@ async function addRepoAndActivateMainWorktree( featureWorktreeId: featureWorktree.id } }, - { targetRepoId: repoId, targetRepoPath: repoPath, targetFeaturePath: featureWorktreePath } + { + targetRepoId: repoId, + targetRepoPath: realpathSync.native(repoPath), + targetFeaturePath: realpathSync.native(featureWorktreePath) + } ) } From d7767fb1960507a7ae3fc47d5858b06f8887bd6b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 15:08:22 -0700 Subject: [PATCH 07/10] perf(worktree): remove redundant creation and terminal startup work (#18793) * perf(worktree): remove redundant creation and terminal startup work * test(worktree): cover optimized creation call signatures Preserve explicit branch adoption, WSL callback routing and sparse cleanup expectations. * perf: preserve user Git checkout worker settings * perf(git): skip malformed remote base probes * perf(cli): avoid loading other agent hooks for Codex preflight * fix(build): retain Codex preflight entry for packaged CLI * test(ssh): wait for replacement PTY before lease recovery input * test(ssh): verify recovered shell execution and lease ownership * test(electron): reap isolated macOS crash reporters on teardown * test: allow either observed self-exit snapshot ordering * test: capture frozen-host input recovery evidence --- config/reliability-gates.jsonc | 96 ++++++++++++++++++ electron.vite.config.ts | 3 + src/cli/handlers/agent-hooks.test.ts | 5 +- src/cli/handlers/agent-hooks.ts | 10 +- .../claude-stream-json-connection.test.ts | 11 ++- src/main/git/repo-branch-conflict.test.ts | 74 +++++++++++++- src/main/git/repo-branch-conflict.ts | 34 +++++-- src/main/git/runner-wsl-direct-read.test.ts | 28 ++++++ .../git/worktree-add-creation-config.test.ts | 4 +- .../worktree-add-local-base-refresh.test.ts | 5 +- ...worktree-add-local-base-suggestion.test.ts | 5 +- src/main/git/worktree-add.ts | 15 ++- ...rktree-create-preparation-real-wsl.test.ts | 77 +++++++++++++++ src/main/git/worktree-create-preparation.ts | 41 +++++--- .../git/worktree-preparation-base-oid.test.ts | 98 +++++++++++++++++++ src/main/ipc/worktree-logic-wsl.test.ts | 21 ++++ src/main/ipc/worktree-logic.ts | 9 +- src/main/ipc/worktree-remote.ts | 42 +++++--- .../ipc/worktrees-local-create-flow.test.ts | 36 +++++-- src/main/ipc/worktrees-test-module-mocks.ts | 23 +++-- src/main/ipc/worktrees-test-runtime-stub.ts | 2 + .../local-pty-provider-spawn-session.test.ts | 7 +- src/main/providers/local-pty-spawn-state.ts | 2 + src/main/runtime/fetch-remote-cache.test.ts | 13 ++- ...orca-runtime-refresh-repo-worktree-scan.ts | 5 + .../local-worktree-creation-part-02.spec.ts | 12 ++- .../local-worktree-creation.spec.ts | 6 +- ...orktree-removal-and-reconciliation.spec.ts | 13 ++- ...runtime-local-worktree-create-candidate.ts | 26 +++-- .../runtime-remote-fetch-controller.ts | 2 +- ...rktree-scan-admin-fingerprint-gate.test.ts | 16 +++ .../terminal-pane/ipc-pty-connect-result.ts | 4 + ...tion-deferred-reattach-live-output.test.ts | 35 +++++++ .../pty-connection/apply-reattach-payload.ts | 7 ++ .../pty-transport-connect-spawn.test.ts | 18 ++++ .../terminal-pane-manager-options.ts | 6 ++ .../lib/pane-manager/pane-lifecycle.test.ts | 28 +++++- .../src/lib/pane-manager/pane-lifecycle.ts | 6 +- .../pane-manager-pane-creation.ts | 2 +- .../lib/pane-manager/pane-manager-types.ts | 1 + .../src/lib/pane-manager/pane-split-close.ts | 2 +- src/shared/git-binary-compatibility.test.ts | 27 +++++ .../e2e/helpers/electron-crashpad-cleanup.ts | 46 +++++++++ .../electron-crashpad-cleanup.unit.test.ts | 45 +++++++++ .../e2e/helpers/electron-process-shutdown.ts | 2 + .../helpers/ssh-recovery-input-observation.ts | 53 ++++++++++ ...ssh-docker-transport-drop-recovery.spec.ts | 62 +++++++++--- 47 files changed, 971 insertions(+), 114 deletions(-) create mode 100644 src/main/git/worktree-create-preparation-real-wsl.test.ts create mode 100644 src/main/git/worktree-preparation-base-oid.test.ts create mode 100644 tests/e2e/helpers/electron-crashpad-cleanup.ts create mode 100644 tests/e2e/helpers/electron-crashpad-cleanup.unit.test.ts create mode 100644 tests/e2e/helpers/ssh-recovery-input-observation.ts diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 2dd39ad7c3f..d48a239354f 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -10,6 +10,102 @@ } }, "gates": [ + { + "id": "terminal-output.prestarted-shell-snapshot-adoption", + "title": "Prestarted shell adoption paints covered output once", + "maturity": "experimental", + "protection": "partial", + "owner": "terminal-runtime", + "layer": "renderer-transport-and-live-electron", + "surfaces": [ + "backend-created first terminal", + "daemon snapshot adoption", + "deferred live output" + ], + "platforms": ["macos", "linux", "windows"], + "providers": ["local", "daemon", "wsl", "ssh", "remote-runtime"], + "coveredPlatforms": ["macos", "linux", "windows"], + "coveredProviders": ["local", "daemon", "wsl"], + "coverageNotes": "macOS daemon-backed Electron journey verifies same PID and terminal identity plus rendered output. Focused renderer contracts pass on Linux, Windows and WSL. Neighboring SSH model and replay contracts pass locally; no new live SSH or paired-runtime journey.", + "motivatingLinks": [ + "https://github.com/user-attachments/assets/e8c6d1dc-6150-4c3d-b55a-3d12efefdd04", + "https://github.com/user-attachments/assets/b0328f88-34ac-4d51-8119-9efe17072435" + ], + "invariant": "Adopting a prestarted terminal preserves its existing process and paints snapshot-covered startup output once while retaining subsequent live output. Missing sequence proof or blank snapshots must not authorize dropping output.", + "oracle": "Pass snapshot sequence and proven zero keyboard flags through real IPC transport projection. Deliver snapshot-covered and newer output before reattach resolves; drain replay parse callbacks and require one startup marker and the newer output. Repeat with no sequence and blank snapshot to retain unproven bytes. In Electron select a prestarted workspace, type a generated marker and compare PID and stable terminal identities before and after.", + "commands": [ + "pnpm test src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts src/renderer/src/components/terminal-pane/pty-connection-deferred-reattach-live-output.test.ts", + "pnpm test src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts src/renderer/src/components/terminal-pane/pty-connection-deferred-reattach-live-output.test.ts src/renderer/src/components/terminal-pane/pty-connection-hidden-snapshot-live-overlap.test.ts src/renderer/src/components/terminal-pane/pty-connection-replay-payload-handling.test.ts src/renderer/src/components/terminal-pane/pty-connection/reattach-payload-ssh-reconnect-model-paint.test.ts", + "pnpm test src/renderer/src/components/terminal-pane/pty-connection src/renderer/src/components/terminal-pane/pty-transport" + ], + "testFiles": [ + "src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts", + "src/renderer/src/components/terminal-pane/pty-connection-deferred-reattach-live-output.test.ts" + ], + "assertionRefs": [ + { + "file": "src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts", + "assertions": [ + "zero and nonzero snapshot sequence and proven zero keyboard flags survive IPC projection" + ] + }, + { + "file": "src/renderer/src/components/terminal-pane/pty-connection-deferred-reattach-live-output.test.ts", + "assertions": [ + "startup output covered by the snapshot is painted once", + "new output remains visible", + "legacy unsequenced and blank snapshots retain bytes" + ] + } + ], + "evidenceRuns": [ + { + "date": "2026-09-04", + "runner": "local", + "platform": "macos", + "command": "pnpm test src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts src/renderer/src/components/terminal-pane/pty-connection-deferred-reattach-live-output.test.ts src/renderer/src/components/terminal-pane/pty-connection-hidden-snapshot-live-overlap.test.ts src/renderer/src/components/terminal-pane/pty-connection-replay-payload-handling.test.ts src/renderer/src/components/terminal-pane/pty-connection/reattach-payload-ssh-reconnect-model-paint.test.ts", + "result": "passed", + "durationSeconds": 2.34, + "summary": "5 suites / 48 tests pass. Focused 2-suite runs independently pass 27 tests on Linux, Windows and WSL." + }, + { + "date": "2026-09-04", + "runner": "local", + "platform": "macos", + "command": "pnpm test src/renderer/src/components/terminal-pane/pty-connection src/renderer/src/components/terminal-pane/pty-transport", + "result": "passed", + "durationSeconds": 5.67, + "summary": "Broader connection/transport gate: 77 files and 813 tests passed, including neighboring restore, reconnect, input and replay behavior. Log: artifacts/worktree-create/orca-draft-replay-broader-gate.log." + } + ], + "runtimeBudget": { + "p95Seconds": 15, + "scope": "focused renderer transport and deferred-adoption contracts" + }, + "flakeHistory": { + "status": "unknown", + "evidence": "Focused local and remote runs pass; no CI soak history." + }, + "redGreenEvidence": { + "status": "partial", + "evidence": "Metadata tests fail before forwarding. Corrected parse-draining regression observes two startup markers when the baseline installation is removed, and one after restoration. Initial missing-live-output failure was a harness parse-drain omission and is not red proof. Before/fixed Electron screenshots show duplicate/single startup output." + }, + "performanceBudget": { + "required": true, + "evidence": "Reuses existing snapshot baseline reconciliation with no new scan, timer or subprocess. Corrected daemon-backed rendered trial reaches replay at 116.6 ms and generated keyboard output at 177 ms after selecting the prestarted workspace. This measures selection/adoption, not ordinary composer creation." + }, + "promotionCriteria": [ + "Meet manifest CI and soak policy.", + "Retain intentional-break and rendered identity/output proof.", + "Exercise live SSH and paired-runtime snapshot adoption before claiming full provider coverage." + ], + "knownGaps": [ + "Composer draft creation and cancellation are not implemented by this gate.", + "No new live SSH, Windows or WSL UI run; remote evidence is focused contract tests.", + "Mixed-version snapshots without sequence proof intentionally retain legacy behavior." + ], + "demotionRule": "Keep experimental or demote if adoption duplicates covered output, drops newer or unproven output, changes terminal ownership, or flakes without explanation." + }, { "id": "cmd-j-tabs.host-qualified-candidate-ownership", "title": "Cmd-J tab candidates retain execution-host ownership", diff --git a/electron.vite.config.ts b/electron.vite.config.ts index 4ed4641cde1..90dc637c204 100644 --- a/electron.vite.config.ts +++ b/electron.vite.config.ts @@ -253,6 +253,9 @@ export const electronViteConfig: UserConfig = { 'agent-hooks/managed-agent-hook-controls': resolve( 'src/main/agent-hooks/managed-agent-hook-controls.ts' ), + 'codex/managed-home-shell-preflight': resolve( + 'src/main/codex/managed-home-shell-preflight.ts' + ), // Why: account import mutates the user's macOS Keychain from the CLI. 'claude-accounts/keychain': resolve('src/main/claude-accounts/keychain.ts') }, diff --git a/src/cli/handlers/agent-hooks.test.ts b/src/cli/handlers/agent-hooks.test.ts index 4fcc186b0d8..279a8900bec 100644 --- a/src/cli/handlers/agent-hooks.test.ts +++ b/src/cli/handlers/agent-hooks.test.ts @@ -56,7 +56,10 @@ vi.mock('../runtime-client', () => { vi.mock('../../main/agent-hooks/managed-agent-hook-controls', () => ({ applyAgentStatusHooksEnabled: applyAgentStatusHooksEnabledMock, - getManagedAgentHookStatuses: getManagedAgentHookStatusesMock, + getManagedAgentHookStatuses: getManagedAgentHookStatusesMock +})) + +vi.mock('../../main/codex/managed-home-shell-preflight', () => ({ prepareManagedCodexHomeBeforeShellLaunch: prepareManagedCodexHomeBeforeShellLaunchMock })) diff --git a/src/cli/handlers/agent-hooks.ts b/src/cli/handlers/agent-hooks.ts index bf44211b4a7..4fcfe64f9b7 100644 --- a/src/cli/handlers/agent-hooks.ts +++ b/src/cli/handlers/agent-hooks.ts @@ -15,11 +15,7 @@ import { getDefaultPersistedState } from '../../shared/constants' import { normalizeDisabledTuiAgents } from '../../shared/tui-agent-selection' import type { GlobalSettings } from '../../shared/global-settings-types' import type { PersistedState } from '../../shared/persisted-state-types' -import { - applyAgentStatusHooksEnabled, - getManagedAgentHookStatuses, - prepareManagedCodexHomeBeforeShellLaunch -} from '../../main/agent-hooks/managed-agent-hook-controls' +import { prepareManagedCodexHomeBeforeShellLaunch } from '../../main/codex/managed-home-shell-preflight' type AgentHookCommandResult = { enabled: boolean @@ -194,6 +190,8 @@ async function setAgentHooksEnabled( client: RuntimeClient, enabled: boolean ): Promise { + const { applyAgentStatusHooksEnabled, getManagedAgentHookStatuses } = + await import('../../main/agent-hooks/managed-agent-hook-controls.js') const updatedRuntime = await updateRunningRuntime(client, enabled) const offlineUpdate = updatedRuntime ? null : updateEnabledOnDisk(enabled) const settingsPath = offlineUpdate?.settingsPath ?? getDataPath() @@ -234,6 +232,8 @@ export const AGENT_HOOK_HANDLERS: Record = { }) }, 'agent hooks status': async ({ json }) => { + const { getManagedAgentHookStatuses } = + await import('../../main/agent-hooks/managed-agent-hook-controls.js') const result: AgentHookCommandResult = { enabled: readHookSettingsFromDisk().agentStatusHooksEnabled, settingsPath: getDataPath(), diff --git a/src/main/claude/claude-stream-json-connection.test.ts b/src/main/claude/claude-stream-json-connection.test.ts index c4f1f9a6fca..eb69a66a897 100644 --- a/src/main/claude/claude-stream-json-connection.test.ts +++ b/src/main/claude/claude-stream-json-connection.test.ts @@ -566,7 +566,7 @@ describe('Claude stream-json connection', () => { ) }) - it('reports a self-exit with its status and stderr, and leaves its tree unverifiable', async () => { + it('reports a self-exit with its status, stderr, and observed tree verdict', async () => { const scenario = scriptScenario([{ stderr: 'claude: not signed in\n' }, { exit: 1 }]) let exit: Error | null = null const connection = await open(launchFor(scenario), { @@ -579,10 +579,11 @@ describe('Claude stream-json connection', () => { // The status and stderr are the only diagnostic a refused start leaves behind. expect((exit as unknown as Error).message).toMatch(/exited \(code 1\): claude: not signed in/) expect(connection.closed).toBe(true) - // The root's exit is first-hand, but it left before a descendant snapshot - // could be armed, so close() has no tree proof to offer and says so. - await expect(connection.close()).resolves.toBe(false) - expect(connection.exitVerdict).toEqual({ root: 'exited', tree: 'unverifiable' }) + // Stderr-triggered capture can win or lose the race with this real child's exit. + const closed = await connection.close() + expect(connection.exitVerdict.root).toBe('exited') + expect(['exited', 'unverifiable']).toContain(connection.exitVerdict.tree) + expect(closed).toBe(connection.exitVerdict.tree === 'exited') }) it.runIf(process.platform !== 'win32')( diff --git a/src/main/git/repo-branch-conflict.test.ts b/src/main/git/repo-branch-conflict.test.ts index 873c8bd3340..82bd1e65c9d 100644 --- a/src/main/git/repo-branch-conflict.test.ts +++ b/src/main/git/repo-branch-conflict.test.ts @@ -21,7 +21,7 @@ describe('getBranchConflictKindViaExec', () => { await expect(getBranchConflictKindViaExec(exec, 'feature/fix')).resolves.toBe('remote') expect(calls).toEqual([ - ['rev-parse', '--verify', 'refs/heads/feature/fix'], + ['rev-parse', '--verify', '--quiet', 'refs/heads/feature/fix'], ['remote'], ['show-ref', '--verify', '--quiet', '--', 'refs/remotes/foo/bar/feature/fix'], ['show-ref', '--verify', '--quiet', '--', 'refs/remotes/origin/feature/fix'] @@ -41,7 +41,10 @@ describe('getBranchConflictKindViaExec', () => { await expect( getBranchConflictKindViaExec(exec, 'feature/fix', 'origin/feature/fix') ).resolves.toBeNull() - expect(calls).toEqual([['rev-parse', '--verify', 'refs/heads/feature/fix'], ['remote']]) + expect(calls).toEqual([ + ['rev-parse', '--verify', '--quiet', 'refs/heads/feature/fix'], + ['remote'] + ]) }) it('keeps longest configured remote-name matching semantics', async () => { @@ -176,7 +179,7 @@ describe('getBranchConflictKindViaExec batched remote probe', () => { getBranchConflictKindViaExec(exec, 'feature', undefined, {}, batched) ).resolves.toBeNull() expect(calls).toEqual([ - ['rev-parse', '--verify', 'refs/heads/feature'], + ['rev-parse', '--verify', '--quiet', 'refs/heads/feature'], ['remote'], ['cat-file', '--batch-check'] ]) @@ -254,3 +257,68 @@ describe('getBranchConflictKindViaExec batched remote probe', () => { expect(calls.filter((argv) => argv[0] === 'show-ref')).toHaveLength(3) }) }) + +describe('branch conflict with existing-branch adoption', () => { + const absent = () => Object.assign(new Error('missing'), { code: 1, stderr: '' }) + + it('skips adoption and its commit probe for a proven missing local ref', async () => { + const exec = vi.fn(async (argv: string[]) => { + if (argv[0] === 'rev-parse') { + throw absent() + } + return { stdout: '' } + }) + const adopt = vi.fn(async () => false) + await expect( + getBranchConflictKindViaExec(exec, 'new', undefined, {}, undefined, adopt) + ).resolves.toBeNull() + expect(adopt).not.toHaveBeenCalled() + expect(exec).toHaveBeenCalledTimes(2) + }) + + it('allows an existing branch without querying remote refs', async () => { + const exec = vi.fn(async () => ({ stdout: 'a'.repeat(40) })) + const adopt = vi.fn(async () => true) + await expect( + getBranchConflictKindViaExec(exec, 'existing', undefined, {}, undefined, adopt) + ).resolves.toBeNull() + expect(adopt).toHaveBeenCalledOnce() + expect(exec).toHaveBeenCalledOnce() + }) + + it('retains conflicts for refs whose objects cannot be adopted as commits', async () => { + const exec = vi.fn(async () => ({ stdout: 'a'.repeat(40) })) + const adopt = vi.fn(async () => false) + await expect( + getBranchConflictKindViaExec(exec, 'dangling', undefined, {}, undefined, adopt) + ).resolves.toBe('local') + expect(adopt).toHaveBeenCalledOnce() + expect(exec).toHaveBeenCalledTimes(2) + }) + + it.each([ + Object.assign(new Error('transport'), { code: 1, stderr: 'transport failed' }), + Object.assign(new Error('timeout'), { code: 'ETIMEDOUT' }) + ])('still attempts adoption after an undecided ref probe: %s', async (error) => { + const exec = vi.fn(async () => { + throw error + }) + const adopt = vi.fn(async () => true) + await expect( + getBranchConflictKindViaExec(exec, 'existing', undefined, {}, undefined, adopt) + ).resolves.toBeNull() + expect(adopt).toHaveBeenCalledOnce() + }) + + it('rechecks a ref that disappeared while adoption was running', async () => { + const exec = vi + .fn() + .mockResolvedValueOnce({ stdout: 'a'.repeat(40) }) + .mockRejectedValueOnce(absent()) + .mockResolvedValueOnce({ stdout: '' }) + await expect( + getBranchConflictKindViaExec(exec, 'removed', undefined, {}, undefined, async () => false) + ).resolves.toBeNull() + expect(exec).toHaveBeenCalledTimes(3) + }) +}) diff --git a/src/main/git/repo-branch-conflict.ts b/src/main/git/repo-branch-conflict.ts index 5c6e03b94b0..f22fd97f99e 100644 --- a/src/main/git/repo-branch-conflict.ts +++ b/src/main/git/repo-branch-conflict.ts @@ -3,6 +3,7 @@ import { gitExecOptions, type LocalGitExecOptions } from './repo-default-base-re import { gitExecFileAsync } from './runner' import { isSafeGitRefName } from '../../shared/git-status-upstream-ref' import { + isShowRefNoMatchError, probeAnyExactRef, probeAnyExactRefBatched, type ExactRefProbeExec, @@ -29,16 +30,17 @@ function canQueryRemoteBranchName(branchName: string): boolean { return !branchName.startsWith('-') && isSafeGitRefName(`refs/heads/${branchName}`) } -async function hasGitRefAsync( +async function probeLocalBranchRef( exec: ExactRefProbeExec, ref: string, options: ExactRefProbeExecOptions -): Promise { +): Promise<'present' | 'absent' | 'unknown'> { try { - const { stdout } = await runGit(exec, ['rev-parse', '--verify', ref], options) - return stdout.trim().length > 0 - } catch { - return false + // Quiet absence avoids retrying the WSL probe through a login shell. + const { stdout } = await runGit(exec, ['rev-parse', '--verify', '--quiet', ref], options) + return stdout.trim().length > 0 ? 'present' : 'unknown' + } catch (error) { + return isShowRefNoMatchError(error) ? 'absent' : 'unknown' } } @@ -105,7 +107,8 @@ export async function getBranchConflictKindViaExec( branchName: string, allowedBaseRef?: string, options: ExactRefProbeExecOptions = {}, - batchedExec?: ExactRefProbeStdinExec + batchedExec?: ExactRefProbeStdinExec, + allowLocalBranch?: () => Promise ): Promise { if (!canQueryRemoteBranchName(branchName)) { return null @@ -114,7 +117,16 @@ export async function getBranchConflictKindViaExec( // are quiet, so introducing a smaller implicit cap would only make a large // remote configuration look like a missing conflict. const probeOptions: ExactRefProbeExecOptions = options - if (await hasGitRefAsync(exec, `refs/heads/${branchName}`, probeOptions)) { + const localRef = `refs/heads/${branchName}` + let presence = await probeLocalBranchRef(exec, localRef, probeOptions) + if (allowLocalBranch && presence !== 'absent') { + if (await allowLocalBranch()) { + return null + } + // Adoption can span ref changes; preserve the fresh conflict check after it fails. + presence = await probeLocalBranchRef(exec, localRef, probeOptions) + } + if (presence === 'present') { return 'local' } @@ -142,7 +154,8 @@ export function getBranchConflictKind( path: string, branchName: string, allowedBaseRef?: string, - options: LocalGitExecOptions = {} + options: LocalGitExecOptions = {}, + allowLocalBranch?: () => Promise ): Promise { const execOptions = gitExecOptions(path, options) const runLocalGit = ( @@ -168,7 +181,8 @@ export function getBranchConflictKind( // one `show-ref` subprocess per remote -- the exact cost the batch exists to remove. // `show-ref --verify --quiet` prints nothing and is read by exit code, so it needs // no fence; the capture wrapper preserves the payload's exit status either way. - (argv, commandOptions) => runLocalGit(argv, commandOptions, true) + (argv, commandOptions) => runLocalGit(argv, commandOptions, true), + allowLocalBranch ) } diff --git a/src/main/git/runner-wsl-direct-read.test.ts b/src/main/git/runner-wsl-direct-read.test.ts index 284cda55718..e7ea423f78e 100644 --- a/src/main/git/runner-wsl-direct-read.test.ts +++ b/src/main/git/runner-wsl-direct-read.test.ts @@ -17,6 +17,7 @@ vi.mock('../observability/instrumentation', () => ({ })) vi.mock('../diagnostics/main-thread-churn-probe', () => ({ recordSubprocessSpawn: vi.fn() })) +import { getBranchConflictKind } from './repo-branch-conflict' import { pendingWslDirectGitReadEnvironment } from './command-runner/git-command-resolution' import { gitExecFileAsync, gitSpawn, gitStreamStdout } from './runner' import { @@ -584,6 +585,33 @@ describe('WSL direct Git reads', () => { }) }) + it('checks a missing branch conflict without retrying through a login shell', async () => { + await withPlatform('win32', async () => { + seedWslGitReadEnvironmentForTests(DISTRO, LOGIN_ENVIRONMENT) + execFileMock.mockImplementation((_command, args: string[], _options, callback) => { + const child = createMockChild() + queueMicrotask(() => { + const missingRef = args.join(' ').includes('rev-parse') + const quiet = args.includes('--quiet') + const code = missingRef ? (quiet ? 1 : 128) : 0 + callback?.( + code ? Object.assign(new Error('missing ref'), { code }) : null, + '', + missingRef && !quiet ? 'fatal: Needed a single revision' : '' + ) + child.emit('close', code, null) + }) + return child + }) + + await expect( + getBranchConflictKind(String.raw`\\wsl.localhost\Ubuntu\repo`, 'new-feature') + ).resolves.toBeNull() + expect(execFileMock).toHaveBeenCalledTimes(2) + expect(execFileMock.mock.calls[0]?.[1]).toContain('--quiet') + }) + }) + it('keeps the fast path when direct and login Git both report an expected failure', async () => { await withPlatform('win32', async () => { seedWslGitReadEnvironmentForTests(DISTRO, LOGIN_ENVIRONMENT) diff --git a/src/main/git/worktree-add-creation-config.test.ts b/src/main/git/worktree-add-creation-config.test.ts index 44394b7728d..ff44ca7ac6d 100644 --- a/src/main/git/worktree-add-creation-config.test.ts +++ b/src/main/git/worktree-add-creation-config.test.ts @@ -199,7 +199,7 @@ describe('addWorktree', () => { }) const worktreeAddCall = gitExecFileAsyncMock.mock.calls.find( - ([argv]) => Array.isArray(argv) && argv[0] === 'worktree' && argv[1] === 'add' + ([argv]) => Array.isArray(argv) && argv.includes('worktree') && argv.includes('add') ) expect(worktreeAddCall?.[1]).toMatchObject({ timeout: WORKTREE_ADD_TIMEOUT_MS }) expect(WORKTREE_ADD_TIMEOUT_MS).toBeGreaterThan(0) @@ -214,7 +214,7 @@ describe('addWorktree', () => { }) const worktreeAddCall = gitExecFileAsyncMock.mock.calls.find( - ([argv]) => Array.isArray(argv) && argv[0] === 'worktree' && argv[1] === 'add' + ([argv]) => Array.isArray(argv) && argv.includes('worktree') && argv.includes('add') ) expect(worktreeAddCall?.[1]).toMatchObject({ timeout: 600_000 }) }) diff --git a/src/main/git/worktree-add-local-base-refresh.test.ts b/src/main/git/worktree-add-local-base-refresh.test.ts index dba4303647c..1ba2e6c16b8 100644 --- a/src/main/git/worktree-add-local-base-refresh.test.ts +++ b/src/main/git/worktree-add-local-base-refresh.test.ts @@ -1,5 +1,5 @@ // addWorktree: fast-forwarding the local base ref (reset --hard / update-ref) and its safety bailouts. -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { gitExecFileAsyncMock, @@ -32,11 +32,14 @@ import { registerWorktreeSuiteHooks } from './worktree-test-harness' registerWorktreeSuiteHooks() describe('addWorktree', () => { + afterEach(() => vi.restoreAllMocks()) const resolveCreationBaseConfigWrite = () => { gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // config --local --replace-all branch..base } beforeEach(() => { + // These branch-safety assertions use POSIX argv; Windows flags have separate coverage. + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') gitExecFileAsyncMock.mockReset() gitExecFileSyncMock.mockReset() translateWslOutputPathsMock.mockClear() diff --git a/src/main/git/worktree-add-local-base-suggestion.test.ts b/src/main/git/worktree-add-local-base-suggestion.test.ts index a65f77dd77d..3b449c339f6 100644 --- a/src/main/git/worktree-add-local-base-suggestion.test.ts +++ b/src/main/git/worktree-add-local-base-suggestion.test.ts @@ -1,5 +1,5 @@ // addWorktree: advisory local-base-ref update suggestions when the refresh setting is off. -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { gitExecFileAsyncMock, @@ -32,7 +32,10 @@ import { registerWorktreeSuiteHooks } from './worktree-test-harness' registerWorktreeSuiteHooks() describe('addWorktree', () => { + afterEach(() => vi.restoreAllMocks()) beforeEach(() => { + // These branch-safety assertions use POSIX argv; Windows flags have separate coverage. + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') gitExecFileAsyncMock.mockReset() gitExecFileSyncMock.mockReset() translateWslOutputPathsMock.mockClear() diff --git a/src/main/git/worktree-add.ts b/src/main/git/worktree-add.ts index ea6ec704b46..380f3a3cc34 100644 --- a/src/main/git/worktree-add.ts +++ b/src/main/git/worktree-add.ts @@ -12,7 +12,7 @@ import { getLocalBaseRefUpdateSuggestionForWorktreeCreate, refreshLocalBaseRefForWorktreeCreate } from './worktree-base-refresh' -import { hasWorktreeBaseCommitRef } from './worktree-base-ref-probe' +import { resolveWorktreeBaseCommitOid } from './worktree-base-ref-probe' import type { AddWorktreeOptions, AddWorktreeResult, @@ -23,6 +23,7 @@ import { bumpWorktreeScanGeneration } from './worktree-scan-cache' export type WorktreeAddBaseContext = AddWorktreeResult & { effectiveBase: string + effectiveBaseOid?: string } export async function resolveWorktreeAddBaseContext( @@ -31,9 +32,11 @@ export async function resolveWorktreeAddBaseContext( refreshLocalBaseRef: boolean, options: AddWorktreeOptions ): Promise { - const effectiveBase = await resolveWorktreeAddBaseRef(baseBranch, (qualifiedRef) => - hasWorktreeBaseCommitRef(repoPath, qualifiedRef, options) - ) + let effectiveBaseOid: string | null = null + const effectiveBase = await resolveWorktreeAddBaseRef(baseBranch, async (qualifiedRef) => { + effectiveBaseOid = await resolveWorktreeBaseCommitOid(repoPath, qualifiedRef, options) + return effectiveBaseOid !== null + }) const localBaseRefRefresh = refreshLocalBaseRef ? await refreshLocalBaseRefForWorktreeCreate( repoPath, @@ -55,6 +58,10 @@ export async function resolveWorktreeAddBaseContext( : undefined return { effectiveBase, + // Refresh/suggestion work can span ref changes; only reuse the immediate resolution probe. + ...(!refreshLocalBaseRef && !options.suggestLocalBaseRefUpdate && effectiveBaseOid + ? { effectiveBaseOid } + : {}), ...(localBaseRefRefresh ? { localBaseRefRefresh } : {}), ...(localBaseRefUpdateSuggestion ? { localBaseRefUpdateSuggestion } : {}) } diff --git a/src/main/git/worktree-create-preparation-real-wsl.test.ts b/src/main/git/worktree-create-preparation-real-wsl.test.ts new file mode 100644 index 00000000000..8ccaf3c23bf --- /dev/null +++ b/src/main/git/worktree-create-preparation-real-wsl.test.ts @@ -0,0 +1,77 @@ +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { expect, it } from 'vitest' +import { createWorktreePreparationLockReason } from '../../shared/worktree/create-preparation' +import { gitExecFileAsync } from './runner' +import { + discardPreparedWorktree, + finalizePreparedWorktree, + prepareWorktreeCreateCheckout +} from './worktree-create-preparation' + +// Opt in on Windows with a running distro; all Git commands use the production WSL router. +const wslDistro = process.env.ORCA_TEST_WSL_DISTRO + +it.skipIf(process.platform !== 'win32' || !wslDistro)( + 'prepares, retargets, moves and cleans up a real WSL checkout from Windows', + async () => { + const fixtureParent = process.env.ORCA_TEST_WSL_ROOT ?? `\\\\wsl.localhost\\${wslDistro}\\tmp` + const root = await mkdtemp(join(fixtureParent, 'orca-create-route-')) + const repoPath = join(root, 'repo') + const preparedPath = join(root, 'prepared checkout') + const finalPath = join(root, 'final checkout') + const options = { wslDistro, timeout: 60_000 } + const git = async (cwd: string, args: string[]): Promise => + (await gitExecFileAsync(args, { cwd, ...options })).stdout.trim() + + try { + await mkdir(repoPath) + await git(repoPath, ['init', '--quiet']) + expect(await git(repoPath, ['rev-parse', '--show-toplevel'])).toMatch(/^\/(?!\/)/) + await git(repoPath, ['symbolic-ref', 'HEAD', 'refs/heads/main']) + await git(repoPath, ['config', 'user.name', 'Test User']) + await git(repoPath, ['config', 'user.email', 'test@example.com']) + await writeFile(join(repoPath, 'version.txt'), 'one\n') + await git(repoPath, ['add', 'version.txt']) + await git(repoPath, ['commit', '--quiet', '-m', 'initial']) + await prepareWorktreeCreateCheckout( + repoPath, + preparedPath, + 'main', + createWorktreePreparationLockReason('real-wsl-test'), + options + ) + expect(await git(repoPath, ['worktree', 'list', '--porcelain'])).toContain( + 'locked orca-create-preparation:v1:' + ) + + await writeFile(join(repoPath, 'version.txt'), 'two\n') + await git(repoPath, ['commit', '--quiet', '-am', 'advance base']) + const target = await git(repoPath, ['rev-parse', 'HEAD']) + await finalizePreparedWorktree( + repoPath, + preparedPath, + finalPath, + 'feature/routed', + 'main', + false, + options + ) + expect(await git(finalPath, ['rev-parse', 'HEAD'])).toBe(target) + expect(await git(finalPath, ['symbolic-ref', '--short', 'HEAD'])).toBe('feature/routed') + expect(await git(finalPath, ['status', '--porcelain'])).toBe('') + expect(await readFile(join(finalPath, 'version.txt'), 'utf8')).toBe('two\n') + expect(await git(finalPath, ['config', '--get', 'branch.feature/routed.base'])).toBe( + 'refs/heads/main' + ) + expect(await git(repoPath, ['worktree', 'list', '--porcelain'])).not.toContain('locked ') + await discardPreparedWorktree(repoPath, finalPath, options) + expect( + (await git(repoPath, ['worktree', 'list', '--porcelain'])).match(/^worktree /gm) + ).toHaveLength(1) + } finally { + await rm(root, { recursive: true, force: true }) + } + }, + 120_000 +) diff --git a/src/main/git/worktree-create-preparation.ts b/src/main/git/worktree-create-preparation.ts index b60dc01ec33..78713957690 100644 --- a/src/main/git/worktree-create-preparation.ts +++ b/src/main/git/worktree-create-preparation.ts @@ -195,25 +195,38 @@ export async function finalizePreparedWorktree( } try { return await runWithGitReadCacheInvalidation(async () => { - const baseContext = await resolveWorktreeAddBaseContext( - repoPath, - baseBranch, - refreshLocalBaseRef, - finalizeGitOptions - ) - const [targetHeadResult, preparedHeadResult] = await Promise.all([ - gitExecFileAsync( - ['rev-parse', '--verify', `${baseContext.effectiveBase}^{commit}`], - gitExecOptions(repoPath, finalizeGitOptions) - ), + const [targetResult, preparedResult] = await Promise.allSettled([ + (async () => { + const baseContext = await resolveWorktreeAddBaseContext( + repoPath, + baseBranch, + refreshLocalBaseRef, + finalizeGitOptions + ) + const targetHead = + baseContext.effectiveBaseOid ?? + ( + await gitExecFileAsync( + ['rev-parse', '--verify', `${baseContext.effectiveBase}^{commit}`], + gitExecOptions(repoPath, finalizeGitOptions) + ) + ).stdout.trim() + return { baseContext, targetHead } + })(), gitExecFileAsync( ['rev-parse', '--verify', 'HEAD'], gitExecOptions(preparedPath, finalizeGitOptions) ) ]) - const { stdout: targetHeadOutput } = targetHeadResult - const targetHead = targetHeadOutput.trim() - const { stdout: preparedHeadOutput } = preparedHeadResult + // Settle both reads before failure cleanup can remove the prepared checkout. + if (targetResult.status === 'rejected') { + throw targetResult.reason + } + if (preparedResult.status === 'rejected') { + throw preparedResult.reason + } + const { baseContext, targetHead } = targetResult.value + const preparedHeadOutput = preparedResult.value.stdout if (preparedHeadOutput.trim() !== targetHead) { await gitExecFileAsync( [...windowsLongPathGitArgs(preparedPath), 'reset', '--hard', targetHead], diff --git a/src/main/git/worktree-preparation-base-oid.test.ts b/src/main/git/worktree-preparation-base-oid.test.ts new file mode 100644 index 00000000000..5864b9d305a --- /dev/null +++ b/src/main/git/worktree-preparation-base-oid.test.ts @@ -0,0 +1,98 @@ +import { beforeEach, expect, it, vi } from 'vitest' + +const gitExec = vi.hoisted(() => vi.fn()) +vi.mock('./runner', () => ({ gitExecFileAsync: gitExec })) +vi.mock('./worktree-base-refresh', () => ({ + refreshLocalBaseRefForWorktreeCreate: vi.fn(), + getLocalBaseRefUpdateSuggestionForWorktreeCreate: vi.fn() +})) +vi.mock('./status', () => ({ runWithGitReadCacheInvalidation: (run: () => unknown) => run() })) +vi.mock('./wsl-linked-worktree-git-routing', () => ({ + invalidateWslLinkedWorktreeGitRouting: vi.fn() +})) + +import { finalizePreparedWorktree } from './worktree-create-preparation' + +const originalOid = '1'.repeat(40) +const refreshedOid = '2'.repeat(40) + +beforeEach(() => { + gitExec.mockReset().mockImplementation(async (args: string[]) => ({ + stdout: + args[0] === 'rev-parse' + ? args.includes('--quiet') || args.at(-1) === 'HEAD' + ? originalOid + : refreshedOid + : '' + })) +}) + +it('reuses the current base-resolution oid and preserves WSL routing', async () => { + await finalizePreparedWorktree('/repo', '/prepared', '/final', 'feature', 'main', false, { + wslDistro: 'Ubuntu', + timeout: 8000 + }) + const revisions = gitExec.mock.calls.filter(([args]) => args[0] === 'rev-parse') + expect(revisions.map(([args]) => args)).toEqual([ + ['rev-parse', '--verify', '--quiet', 'refs/heads/main^{commit}'], + ['rev-parse', '--verify', 'HEAD'] + ]) + expect(gitExec.mock.calls.find(([args]) => args.includes('checkout'))?.[0]).toContain(originalOid) + expect(gitExec.mock.calls.some(([args]) => args.includes('reset'))).toBe(false) + for (const [, options] of gitExec.mock.calls) { + expect(options).toMatchObject({ wslDistro: 'Ubuntu', timeout: 8000 }) + } +}) + +it.each([ + { base: 'refs/heads/main', refresh: false, options: {} }, + { base: 'main', refresh: true, options: {} }, + { base: 'main', refresh: false, options: { suggestLocalBaseRefUpdate: true } } +])('re-reads the target for $base, refresh=$refresh, options=$options', async (test) => { + await finalizePreparedWorktree( + '/repo', + '/prepared', + '/final', + 'feature', + test.base, + test.refresh, + test.options + ) + expect(gitExec).toHaveBeenCalledWith( + ['rev-parse', '--verify', 'refs/heads/main^{commit}'], + expect.objectContaining({ cwd: '/repo' }) + ) + expect(gitExec.mock.calls.find(([args]) => args.includes('reset'))?.[0]).toContain(refreshedOid) + expect(gitExec.mock.calls.find(([args]) => args.includes('checkout'))?.[0]).toContain( + refreshedOid + ) +}) + +it('starts both independent probes before either resolves and settles them before failure', async () => { + let resolveBase!: (value: { stdout: string }) => void + let rejectPrepared!: (reason: Error) => void + gitExec.mockImplementation((args: string[]) => { + if (args.includes('--quiet')) { + return new Promise((resolve) => (resolveBase = resolve)) + } + if (args.at(-1) === 'HEAD') { + return new Promise((_, reject) => (rejectPrepared = reject)) + } + return Promise.resolve({ stdout: '' }) + }) + let settled = false + const error = new Error('prepared HEAD unreadable') + const result = finalizePreparedWorktree('/repo', '/prepared', '/final', 'feature', 'main') + const checked = expect(result).rejects.toBe(error) + void result.then( + () => (settled = true), + () => (settled = true) + ) + await vi.waitFor(() => expect(gitExec).toHaveBeenCalledTimes(2)) + rejectPrepared(error) + await Promise.resolve() + expect(settled).toBe(false) + resolveBase({ stdout: originalOid }) + await checked + expect(gitExec.mock.calls.some(([args]) => args.includes('move'))).toBe(false) +}) diff --git a/src/main/ipc/worktree-logic-wsl.test.ts b/src/main/ipc/worktree-logic-wsl.test.ts index c30c387263e..c21c20e0050 100644 --- a/src/main/ipc/worktree-logic-wsl.test.ts +++ b/src/main/ipc/worktree-logic-wsl.test.ts @@ -16,6 +16,7 @@ vi.mock('../wsl', () => ({ import { computeWorktreePath, computeWorktreePathAsync, + computeWorkspaceRootAsync, getWorktreePathSettings } from './worktree-logic' import { @@ -32,6 +33,26 @@ describe('computeWorktreePath WSL layout', () => { parseWslPathMock.mockReset() }) + it('reuses an asynchronously resolved root for every name candidate without a sync probe', async () => { + parseWslPathMock.mockReturnValue({ distro: 'Ubuntu', linuxPath: '/home/jin/repo' }) + const repoPath = String.raw`\\wsl.localhost\Ubuntu\home\jin\repo` + const home = String.raw`\\wsl.localhost\Ubuntu\home\jin` + const settings = { workspaceDir: 'C:\\workspaces', nestWorkspaces: true } + let resolveHome!: (home: string) => void + getWslHomeAsyncMock.mockReturnValue(new Promise((resolve) => (resolveHome = resolve))) + const pendingRoot = computeWorkspaceRootAsync(repoPath, settings) + expect(getWslHomeMock).not.toHaveBeenCalled() + resolveHome(home) + const root = await pendingRoot + for (const name of ['feature', 'feature-2', 'feature-3']) { + expect(computeWorktreePath(name, repoPath, settings, root)).toBe( + win32.join(home, 'orca', 'workspaces', 'repo', name) + ) + } + expect(getWslHomeAsyncMock).toHaveBeenCalledExactlyOnceWith('Ubuntu') + expect(getWslHomeMock).not.toHaveBeenCalled() + }) + it('places WSL repo worktrees under the distro home workspace root', () => { parseWslPathMock.mockReturnValue({ distro: 'Ubuntu', diff --git a/src/main/ipc/worktree-logic.ts b/src/main/ipc/worktree-logic.ts index 7a8fe175c89..744572f6e37 100644 --- a/src/main/ipc/worktree-logic.ts +++ b/src/main/ipc/worktree-logic.ts @@ -103,12 +103,13 @@ export function ensurePathWithinWorkspace(targetPath: string, workspaceDir: stri export function computeWorktreePath( sanitizedName: string, repoPath: string, - settings: WorktreePathSettings + settings: WorktreePathSettings, + workspaceRoot?: string ): string { return computeWorktreePathFromWorkspaceRoot( sanitizedName, repoPath, - computeWorkspaceRoot(repoPath, settings), + workspaceRoot ?? computeWorkspaceRoot(repoPath, settings), settings.nestWorkspaces ) } @@ -130,7 +131,7 @@ function computeWorktreePathFromWorkspaceRoot( } /** Async twin of computeWorktreePath. Same result; resolves the WSL home without blocking the main - * thread, so callers off the create path never freeze the app on a stopped distro. */ + * thread, so callers never freeze the app on a stopped distro. */ export async function computeWorktreePathAsync( sanitizedName: string, repoPath: string, @@ -147,7 +148,7 @@ export async function computeWorktreePathAsync( /** Async twin of computeWorkspaceRoot. Same result; the WSL home probe spawns `wsl.exe`, so * background preparation uses this variant rather than blocking the Electron main thread for up * to the probe timeout. The sync twin below still serves callers that cannot await (allowed-roots - * resolution, the create click, CLI create, watch targets, worktree trash). */ + * resolution, CLI create, watch targets, worktree trash). */ export async function computeWorkspaceRootAsync( repoPath: string, settings: { workspaceDir: string; wslMirrorDistro?: string } diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 27eaa9264db..ef65f2fcb53 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -87,7 +87,7 @@ import { computeValidatedBranchName, computeWorktreePath, computeRemoteWorktreePath, - computeWorkspaceRoot, + computeWorkspaceRootAsync, ensurePathWithinWorkspace, getWorktreeCreationLayout, getWorktreePathSettings, @@ -2444,7 +2444,7 @@ export async function createLocalWorktree( emitCreateWorktreeProgress(mainWindow, 'fetching', args.creationId) } } - const workspaceRoot = computeWorkspaceRoot(repo.path, worktreePathSettings) + const workspaceRoot = await computeWorkspaceRootAsync(repo.path, worktreePathSettings) // Why: this validation doesn't depend on remote refs, so it can overlap a required remote-tracking base refresh. const primarySetupScript = getEffectiveHooks(repo)?.scripts.setup @@ -2530,19 +2530,33 @@ export async function createLocalWorktree( username, localWorktreeGitOptions ) - checkoutExistingBranch = await canCheckoutExistingLocalBranch( - repo.path, - branchName, - baseBranch, - localWorktreeGitOptions - ) - if (checkoutExistingBranch && !selectedExistingLocalBranchName) { - // Why: suffix retries may need a new path, but an existing-branch checkout must keep the user-selected branch, not a sibling. - selectedExistingLocalBranchName = branchName + const tryExistingBranch = async (): Promise => { + checkoutExistingBranch = await canCheckoutExistingLocalBranch( + repo.path, + branchName, + baseBranch, + localWorktreeGitOptions + ) + return checkoutExistingBranch } + // Explicit branch selections retain the adoption-first path. + const preferExistingBranch = Boolean( + args.branchNameOverride || selectedExistingLocalBranchName + ) + checkoutExistingBranch = preferExistingBranch && (await tryExistingBranch()) lastBranchConflictKind = checkoutExistingBranch ? null - : await getBranchConflictKind(repo.path, branchName, baseBranch, localWorktreeGitOptions) + : await getBranchConflictKind( + repo.path, + branchName, + baseBranch, + localWorktreeGitOptions, + preferExistingBranch ? undefined : tryExistingBranch + ) + if (checkoutExistingBranch && !selectedExistingLocalBranchName) { + // Path retries must retain the adopted branch. + selectedExistingLocalBranchName = branchName + } const allowedPushTargetRemoteConflict = lastBranchConflictKind && isAllowedPushTargetRemoteConflict(lastBranchConflictKind, branchName, args) @@ -2604,7 +2618,7 @@ export async function createLocalWorktree( } worktreePath = ensurePathWithinWorkspace( - computeWorktreePath(effectiveSanitizedName, repo.path, worktreePathSettings), + computeWorktreePath(effectiveSanitizedName, repo.path, worktreePathSettings, workspaceRoot), workspaceRoot ) if (existsSync(worktreePath)) { @@ -3010,6 +3024,8 @@ export async function createLocalWorktree( } }) + // Startup resolves the new id before lifecycle notifications invalidate runtime caches. + runtime?.invalidateWorktreeCatalog?.(repo.id) const stagedStartup = await timing.time('spawn_startup_terminal', () => spawnLocalStartupAndSetupTerminals({ runtime, diff --git a/src/main/ipc/worktrees-local-create-flow.test.ts b/src/main/ipc/worktrees-local-create-flow.test.ts index abc711bbd9b..fc57c6969b6 100644 --- a/src/main/ipc/worktrees-local-create-flow.test.ts +++ b/src/main/ipc/worktrees-local-create-flow.test.ts @@ -2,6 +2,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { resolve } from 'node:path' import type { CreateWorktreeResult } from '../../shared/worktree/create-types' import { resolveRegisteredWorktreePath } from './registered-worktree-roots-cache' +import { computeWorkspaceRootAsync } from './worktree-logic' +import type * as WorktreeLogic from './worktree-logic' import { listWorktreesMock, describeCreatedWorktreeMock, @@ -78,11 +80,13 @@ vi.mock('../setup-hook-env-vars', async (importOriginal) => (await importOriginal()) as Record ) ) -vi.mock('./worktree-logic', async (importOriginal) => - (await import('./worktrees-test-module-mocks')).worktreeLogicModuleMock( - (await importOriginal()) as Record - ) -) +vi.mock('./worktree-logic', async (importOriginal) => { + const actual = await importOriginal() + return { + ...(await import('./worktrees-test-module-mocks')).worktreeLogicModuleMock(actual), + computeWorkspaceRootAsync: vi.fn(actual.computeWorkspaceRootAsync) + } +}) vi.mock('../terminal-history-deletion', async () => (await import('./worktrees-test-module-mocks')).terminalHistoryDeletionModuleMock() ) @@ -409,15 +413,23 @@ describe('registerWorktreeHandlers', () => { } ]) - await handlers['worktrees:create'](null, { + const root = Promise.withResolvers() + vi.mocked(computeWorkspaceRootAsync).mockReturnValueOnce(root.promise) + const create = handlers['worktrees:create'](null, { repoId: 'repo-1', name: 'feature' }) - expect(computeWorktreePathMock).toHaveBeenCalledWith('feature', '/workspace/repo', { - nestWorkspaces: false, - workspaceDir: '../worktrees' - }) + await vi.waitFor(() => expect(computeWorkspaceRootAsync).toHaveBeenCalled()) + expect(addWorktreeMock).not.toHaveBeenCalled() + root.resolve('/workspace/worktrees') + await create + expect(computeWorktreePathMock).toHaveBeenCalledWith( + 'feature', + '/workspace/repo', + { nestWorkspaces: false, workspaceDir: '../worktrees' }, + '/workspace/worktrees' + ) expect(addWorktreeMock).toHaveBeenCalledWith( '/workspace/repo', '../worktrees/feature', @@ -689,6 +701,10 @@ describe('registerWorktreeHandlers', () => { expect(setupCommand).toBe('bash /workspace/repo/.git/orca/setup-runner.sh') expect(result.setup).toBeUndefined() expect(result.startupTerminal).toEqual({ spawned: true, surface: 'visible' }) + expect(runtimeStub.invalidateWorktreeCatalog).toHaveBeenCalledWith('repo-1') + expect(runtimeStub.invalidateWorktreeCatalog.mock.invocationCallOrder[0]).toBeLessThan( + runtimeStub.createTerminal.mock.invocationCallOrder[0] + ) expect(result.timing?.phases.map((phase) => phase.phase)).toEqual( expect.arrayContaining([ 'git_worktree_add', diff --git a/src/main/ipc/worktrees-test-module-mocks.ts b/src/main/ipc/worktrees-test-module-mocks.ts index a1925d1ce72..8d2787fcf2f 100644 --- a/src/main/ipc/worktrees-test-module-mocks.ts +++ b/src/main/ipc/worktrees-test-module-mocks.ts @@ -1,4 +1,5 @@ import { type Mock, vi } from 'vitest' +import type { computeWorktreePath } from './worktree-logic' import type { HandlerMap } from './worktrees-test-ipc-surface' /** Loose signature: one mock stands in for many unrelated module exports. */ @@ -78,13 +79,7 @@ export const resolveSetupRunnerShellMock: ModuleMock = vi.fn() export const runHookMock: ModuleMock = vi.fn() export const hasHooksFileMock: ModuleMock = vi.fn() export const loadHooksMock: ModuleMock = vi.fn() -export const computeWorktreePathMock: Mock< - ( - sanitizedName: string, - repoPath: string, - settings: { nestWorkspaces: boolean; workspaceDir: string } - ) => string -> = vi.fn() +export const computeWorktreePathMock: Mock = vi.fn() export const ensurePathWithinWorkspaceMock: StringArgMock = vi.fn() export const gitExecFileAsyncMock: GitArgvMock = vi.fn() export const getSshGitProviderMock: StringArgMock = vi.fn() @@ -138,7 +133,19 @@ export const gitRepoModuleMock = () => ({ resolveDefaultBaseRefWithLocalGit: resolveDefaultBaseRefWithLocalGitMock, resolveDefaultBaseRefViaExec: resolveDefaultBaseRefViaExecMock, getDefaultRemote: getDefaultRemoteMock, - getBranchConflictKind: getBranchConflictKindMock + getBranchConflictKind: async ( + repoPath: string, + branch: string, + base?: string, + options?: { wslDistro?: string }, + allowLocalBranch?: () => Promise + ) => { + // These handler tests stub ref presence; policy tests cover the absent-ref fast path. + if (allowLocalBranch && (await allowLocalBranch())) { + return null + } + return getBranchConflictKindMock(repoPath, branch, base, options) + } }) export const githubClientModuleMock = () => ({ diff --git a/src/main/ipc/worktrees-test-runtime-stub.ts b/src/main/ipc/worktrees-test-runtime-stub.ts index 647d4801d13..bb2f33cb05e 100644 --- a/src/main/ipc/worktrees-test-runtime-stub.ts +++ b/src/main/ipc/worktrees-test-runtime-stub.ts @@ -12,6 +12,7 @@ export type WorktreeRuntimeStub = { clearOptimisticReconcileToken: ReturnType resolveManagedMrBase: ReturnType createTerminal: ReturnType + invalidateWorktreeCatalog: ReturnType splitTerminal: ReturnType notifyWorktreesChangedForRemoteClients: ReturnType closeFileWatchersForRemoval: ReturnType @@ -38,6 +39,7 @@ export function createWorktreeRuntimeStub(): WorktreeRuntimeStub { title: null, surface: 'visible' }), + invalidateWorktreeCatalog: vi.fn(), splitTerminal: vi.fn().mockResolvedValue({ handle: 'term-setup', tabId: 'tab-startup', diff --git a/src/main/providers/local-pty-provider-spawn-session.test.ts b/src/main/providers/local-pty-provider-spawn-session.test.ts index 7513d9c72cb..9dfa08d81bf 100644 --- a/src/main/providers/local-pty-provider-spawn-session.test.ts +++ b/src/main/providers/local-pty-provider-spawn-session.test.ts @@ -172,6 +172,7 @@ describe('LocalPtyProvider', () => { expect(second).toEqual({ id: 'serve-session-1', + incarnationId: first.incarnationId, pid: 12345, isReattach: true, // Why published: this attach really moved the PTY, unlike daemon/relay attach, so main @@ -220,7 +221,11 @@ describe('LocalPtyProvider', () => { attachOnly: true }) - expect(result).toMatchObject({ id: first.id, isReattach: true }) + expect(result).toMatchObject({ + id: first.id, + incarnationId: first.incarnationId, + isReattach: true + }) expect(spawnMock).not.toHaveBeenCalled() }) diff --git a/src/main/providers/local-pty-spawn-state.ts b/src/main/providers/local-pty-spawn-state.ts index d41f858cf98..2ab145f6c39 100644 --- a/src/main/providers/local-pty-spawn-state.ts +++ b/src/main/providers/local-pty-spawn-state.ts @@ -1,6 +1,7 @@ import type { PtySpawnResult } from './types' import { pendingLocalPtySpawns, + ptyIncarnations, ptyProcesses, ptyWslDistroById, type PendingLocalPtySpawn @@ -60,6 +61,7 @@ export function reattachLocalPty(id: string, cols: number, rows: number): PtySpa } return { id, + ...(ptyIncarnations.has(id) ? { incarnationId: ptyIncarnations.get(id) } : {}), pid: existing.pid, ...(ptyWslDistroById.has(id) ? { wslDistro: ptyWslDistroById.get(id) ?? null } : {}), isReattach: true, diff --git a/src/main/runtime/fetch-remote-cache.test.ts b/src/main/runtime/fetch-remote-cache.test.ts index 11cd2e0260d..5f0b8e249d6 100644 --- a/src/main/runtime/fetch-remote-cache.test.ts +++ b/src/main/runtime/fetch-remote-cache.test.ts @@ -176,8 +176,17 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { expect(caches.fetchLastCompletedAt.has('/repo/cache-0::origin')).toBe(false) }) - it.each(['main', 'a'.repeat(40), 'refs/remotes/main', ''])( - 'does not launch Git for a base without a remote/branch separator: %s', + it.each([ + 'main', + 'a'.repeat(40), + 'refs/remotes/main', + '', + 'origin/', + '/main', + 'refs/remotes/origin/', + 'refs/remotes//main' + ])( + 'does not launch Git for a base without both remote and branch components: %s', async (base) => { const runtime = new OrcaRuntimeService(null) await expect(runtime.resolveRemoteTrackingBase('/repo/e', base)).resolves.toBeNull() diff --git a/src/main/runtime/orca-runtime-refresh-repo-worktree-scan.ts b/src/main/runtime/orca-runtime-refresh-repo-worktree-scan.ts index 1964b5fdd2f..2df3f7b9f17 100644 --- a/src/main/runtime/orca-runtime-refresh-repo-worktree-scan.ts +++ b/src/main/runtime/orca-runtime-refresh-repo-worktree-scan.ts @@ -153,6 +153,11 @@ export class OrcaRuntimeWithRefreshRepoWorktreeScan extends OrcaRuntimeWithListK } } + invalidateWorktreeCatalog(repoId: string): void { + this.invalidateResolvedWorktreeCache() + this.invalidateWorktreeScanCacheForRepo(repoId) + } + protected invalidateSshWorktreeScanCacheInternal(targetId: string): void { const repos = this.store?.getRepos() ?? [] const affectedRepos = repos.filter((repo) => getRepoSshConnectionId(repo) === targetId) diff --git a/src/main/runtime/orca-runtime-tests/local-worktree-creation-part-02.spec.ts b/src/main/runtime/orca-runtime-tests/local-worktree-creation-part-02.spec.ts index fe6e8bfe3da..a12ffabcd61 100644 --- a/src/main/runtime/orca-runtime-tests/local-worktree-creation-part-02.spec.ts +++ b/src/main/runtime/orca-runtime-tests/local-worktree-creation-part-02.spec.ts @@ -50,7 +50,13 @@ describe('OrcaRuntimeService', () => { pushTarget: { remoteName: 'origin', branchName: 'feature/fix' } }) - expect(getBranchConflictKind).toHaveBeenCalledWith(TEST_REPO_PATH, 'feature/fix', 'abc123') + expect(getBranchConflictKind).toHaveBeenCalledWith( + TEST_REPO_PATH, + 'feature/fix', + 'abc123', + {}, + undefined + ) expect(getPRForBranchMock).toHaveBeenCalledWith(TEST_REPO_PATH, 'feature/fix') expect(addWorktree).toHaveBeenCalledWith( TEST_REPO_PATH, @@ -165,7 +171,9 @@ describe('OrcaRuntimeService', () => { expect(getBranchConflictKind).toHaveBeenCalledWith( TEST_REPO_PATH, 'feature/bitbucket', - 'abc123' + 'abc123', + {}, + undefined ) expect(getHostedReviewForBranchMock).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/src/main/runtime/orca-runtime-tests/local-worktree-creation.spec.ts b/src/main/runtime/orca-runtime-tests/local-worktree-creation.spec.ts index 17fe670a09c..3dcd2d0584c 100644 --- a/src/main/runtime/orca-runtime-tests/local-worktree-creation.spec.ts +++ b/src/main/runtime/orca-runtime-tests/local-worktree-creation.spec.ts @@ -533,10 +533,14 @@ describe('OrcaRuntimeService', () => { branchNameOverride: 'feature/something' }) + // Why: an explicit branch override adopts the local branch before the conflict + // probe, so no lazy adoption callback is handed to getBranchConflictKind. expect(getBranchConflictKind).toHaveBeenCalledWith( TEST_REPO_PATH, 'feature/something', - 'origin/feature/something' + 'origin/feature/something', + {}, + undefined ) expect(addWorktree).toHaveBeenCalledWith( TEST_REPO_PATH, diff --git a/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation.spec.ts index 65da9caccde..630eaebf007 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation.spec.ts @@ -376,7 +376,18 @@ describe('OrcaRuntimeService', () => { TEST_REPO_PATH, 'runtime-wsl', 'origin/main', - { wslDistro: 'Ubuntu' } + { wslDistro: 'Ubuntu' }, + expect.any(Function) + ) + // Why: the lazy adoption callback is only invoked when the conflict probe + // sees a local ref, so drive it here to prove adoption also routes via WSL. + const adoptLocalBranch = vi + .mocked(getBranchConflictKind) + .mock.calls.findLast((call) => call[1] === 'runtime-wsl')?.[4] + await expect(adoptLocalBranch?.()).resolves.toBe(false) + expect(gitSpy).toHaveBeenCalledWith( + ['rev-parse', '--verify', '--quiet', 'refs/heads/runtime-wsl^{commit}'], + { cwd: TEST_REPO_PATH, wslDistro: 'Ubuntu' } ) expect(getPRForBranchMock).toHaveBeenCalledWith( TEST_REPO_PATH, diff --git a/src/main/runtime/runtime-local-worktree-create-candidate.ts b/src/main/runtime/runtime-local-worktree-create-candidate.ts index 6c454666148..9de955c5cd1 100644 --- a/src/main/runtime/runtime-local-worktree-create-candidate.ts +++ b/src/main/runtime/runtime-local-worktree-create-candidate.ts @@ -110,23 +110,31 @@ export async function resolveRuntimeLocalWorktreeCreateCandidate(args: { args.username, args.localWorktreeGitOptions ) - checkoutExistingBranch = await canCheckoutExistingLocalBranch( - args.repo.path, - branchName, - args.baseBranch, - ...args.localWorktreeGitOptionArgs - ) - if (checkoutExistingBranch && !selectedExistingLocalBranchName) { - selectedExistingLocalBranchName = branchName + const tryExistingBranch = async (): Promise => { + checkoutExistingBranch = await canCheckoutExistingLocalBranch( + args.repo.path, + branchName, + args.baseBranch, + ...args.localWorktreeGitOptionArgs + ) + return checkoutExistingBranch } + const preferExistingBranch = Boolean( + args.request.branchNameOverride || selectedExistingLocalBranchName + ) + checkoutExistingBranch = preferExistingBranch && (await tryExistingBranch()) branchConflictKind = checkoutExistingBranch ? null : await getBranchConflictKind( args.repo.path, branchName, args.baseBranch, - ...args.localWorktreeGitOptionArgs + args.localWorktreeGitOptions, + preferExistingBranch ? undefined : tryExistingBranch ) + if (checkoutExistingBranch && !selectedExistingLocalBranchName) { + selectedExistingLocalBranchName = branchName + } const allowedPushTargetRemoteConflict = branchConflictKind && isAllowedPushTargetRemoteConflict(branchConflictKind, branchName, args.request) diff --git a/src/main/runtime/runtime-remote-fetch-controller.ts b/src/main/runtime/runtime-remote-fetch-controller.ts index f40f25f6c82..dbcc240525b 100644 --- a/src/main/runtime/runtime-remote-fetch-controller.ts +++ b/src/main/runtime/runtime-remote-fetch-controller.ts @@ -231,7 +231,7 @@ export class RuntimeRemoteFetchController { ? baseBranch.slice(remoteRefPrefix.length) : baseBranch // A remote-tracking base needs both a configured remote and a branch component. - if (!shortBaseBranch.includes('/')) { + if (shortBaseBranch.indexOf('/') <= 0 || shortBaseBranch.endsWith('/')) { return null } let remotes: string[] diff --git a/src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts b/src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts index 3750884f84f..7dfe0144406 100644 --- a/src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts +++ b/src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts @@ -273,6 +273,22 @@ describe('worktree scan admin-fingerprint gate', () => { } }) + it('resolves a just-created id after invalidation even within both cache TTLs', async () => { + const { runtime, list } = makeRuntime() + listWorktreesStrictMock.mockResolvedValueOnce([ + { path: REPO_PATH, head: 'abc', branch: 'main', isBare: false, isMainWorktree: true } + ]) + await list() + await expect(runtime.showManagedWorktree(`id:${WORKTREE_ID}`)).rejects.toThrow( + 'selector_not_found' + ) + runtime.invalidateWorktreeCatalog(REPO_ID) + await expect(runtime.showManagedWorktree(`id:${WORKTREE_ID}`)).resolves.toMatchObject({ + id: WORKTREE_ID + }) + expect(scanCount()).toBe(2) + }) + it('scans when the probe cannot describe the repo', async () => { vi.useFakeTimers() try { diff --git a/src/renderer/src/components/terminal-pane/ipc-pty-connect-result.ts b/src/renderer/src/components/terminal-pane/ipc-pty-connect-result.ts index 3dda2f83fec..bcfc0d3f67c 100644 --- a/src/renderer/src/components/terminal-pane/ipc-pty-connect-result.ts +++ b/src/renderer/src/components/terminal-pane/ipc-pty-connect-result.ts @@ -16,6 +16,10 @@ export function projectIpcPtyConnectResult( snapshot: spawnResult.snapshot, snapshotCols: spawnResult.snapshotCols, snapshotRows: spawnResult.snapshotRows, + ...(spawnResult.snapshotSeq !== undefined ? { snapshotSeq: spawnResult.snapshotSeq } : {}), + ...(spawnResult.snapshotKittyKeyboardFlags !== undefined + ? { snapshotKittyKeyboardFlags: spawnResult.snapshotKittyKeyboardFlags } + : {}), ...(spawnResult.snapshotPrefixAnsi !== undefined ? { snapshotPrefixAnsi: spawnResult.snapshotPrefixAnsi } : {}), diff --git a/src/renderer/src/components/terminal-pane/pty-connection-deferred-reattach-live-output.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-deferred-reattach-live-output.test.ts index fa110cdfe0e..ed38f6a31a4 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-deferred-reattach-live-output.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-deferred-reattach-live-output.test.ts @@ -231,6 +231,41 @@ describe('connectPanePty', () => { expect(transport.sendInput).not.toHaveBeenCalled() }) + it.each([ + { kind: 'covered backlog', snapshot: 'startup\r\n', seq: 9, count: 1 }, + { kind: 'legacy unsequenced snapshot', snapshot: 'startup\r\n', seq: undefined, count: 2 }, + { kind: 'blank snapshot', snapshot: '\x1b[2J', seq: 9, count: 1 } + ])( + 'preserves output while reconciling $kind on daemon adoption', + async ({ snapshot, seq, count }) => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('tab-pty') + transport.connect.mockImplementation( + async ({ sessionId, callbacks }: { sessionId?: string; callbacks?: ConnectCallbacks }) => { + callbacks?.onData?.('startup\r\n', { seq: 9, rawLength: 9 }) + callbacks?.onData?.('new output\r\n', { seq: 21, rawLength: 12 }) + return { id: sessionId, snapshot, snapshotSeq: seq } + } + ) + transportFactoryQueue.push(transport) + const pane = createPane(1) + const { writes, parseCallbacks } = captureCallbackTerminalWrites(pane) + const deps = createDeps({ + isVisibleRef: { current: true }, + restoredLeafId: LEAF_1, + restoredPtyIdByLeafId: { [LEAF_1]: 'tab-pty' } + }) + connectPanePty(pane as never, createManager(1) as never, deps as never) + await flushAsyncTicks(20) + for (let step = 0; step < 40; step += 1) { + parseCallbacks.shift()?.() + await flushAsyncTicks(2) + } + expect(writes.join('').match(/startup/g)).toHaveLength(count) + expect(writes.join('')).toContain('new output') + } + ) + it('drains live bytes after transport confirms an explicit reattach', async () => { const { connectPanePty } = await import('./pty-connection') const { deliverTerminalDataWithDeferredCredit } = diff --git a/src/renderer/src/components/terminal-pane/pty-connection/apply-reattach-payload.ts b/src/renderer/src/components/terminal-pane/pty-connection/apply-reattach-payload.ts index cb0806900af..ad64d458711 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/apply-reattach-payload.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/apply-reattach-payload.ts @@ -100,6 +100,13 @@ export function createReattachPayloadHandlers( // Why last: re-arm the dangling mid-escape after the reset (whose ESC would abort it) so the live continuation completes it (#7329). session.writeReplayData(ctx.connectResult.pendingEscapeTailAnsi) } + // The initial attach backlog can contain bytes already painted by this snapshot. + session.setRestoredSnapshotBaseline( + ctx.ptyId, + { seq: ctx.connectResult.snapshotSeq }, + restoredSnapshotPaintsPrintableContent({ data: daemonSnapshotReplay }) + ) + session.recordRendererOrderedSeq({ seq: ctx.connectResult.snapshotSeq }) session.sendFocusedReattachFocusInAfterReplay(ctx.ptyId, ctx.attemptGeneration) if (ctx.connectResult.coldRestore) { // Snapshot superseded the cold-restore payload; ack so the daemon doesn't redeliver it. diff --git a/src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts index 66dca98b360..138059f372b 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts @@ -26,6 +26,24 @@ describe('createIpcPtyTransport', () => { restorePtySpecWindow(originalWindow) }) + it.each([0, 420])( + 'preserves snapshot sequence and keyboard proof %s across IPC reattach', + async (seq) => { + const { createIpcPtyTransport } = await import('./pty-transport') + vi.mocked(window.api.pty.spawn).mockResolvedValue({ + id: 'existing', + isReattach: true, + snapshot: 'ready', + snapshotSeq: seq, + snapshotKittyKeyboardFlags: 0 + }) + const transport = createIpcPtyTransport({}) + const result = await transport.connect({ url: '', sessionId: 'existing', callbacks: {} }) + expect(result).toMatchObject({ snapshotSeq: seq, snapshotKittyKeyboardFlags: 0 }) + transport.detach?.() + } + ) + it('leaves title tracking to the PTY data stream (no OpenCode IPC channel)', async () => { // Why: the OpenCode status IPC channel is gone (now the agent-hooks server), so the transport has no per-agent status callback. const { createIpcPtyTransport } = await import('./pty-transport') diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-manager-options.ts b/src/renderer/src/components/terminal-pane/terminal-pane-manager-options.ts index 08eab4c0cf5..128b715a3c5 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-manager-options.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-manager-options.ts @@ -1,6 +1,7 @@ import type { IDisposable } from '@xterm/xterm' import type { PaneManagerOptions } from '@/lib/pane-manager/pane-manager' import { useAppStore } from '@/store' +import { resolveTerminalLigaturesEnabled } from '../../../../shared/terminal-ligatures' import { resolveTerminalFontWeights } from '../../../../shared/terminal-fonts' import { normalizeTerminalLineHeight } from '../../../../shared/terminal-line-height-settings' import { normalizeDesktopTerminalScrollbackRows } from '../../../../shared/terminal-scrollback-policy' @@ -102,6 +103,11 @@ export function createTerminalPaneManagerOptions( }, resolveExternalPaneDropTarget, onExternalPaneDrop, + terminalLigaturesEnabled: () => + resolveTerminalLigaturesEnabled( + settingsRef.current?.terminalLigatures, + settingsRef.current?.terminalFontFamily + ), terminalOptions: () => { const currentSettings = settingsRef.current const terminalFontWeights = resolveTerminalFontWeights( diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts index 3a174253d9c..e84638c33ea 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts @@ -7,7 +7,7 @@ import { primeTerminalWebglAddon, resetTerminalWebglSuggestion } from './pane-webgl-renderer' -import { attachLigatures, disposePane, openTerminal } from './pane-lifecycle' +import { attachLigatures, disposePane, openTerminal, setLigaturesEnabled } from './pane-lifecycle' import { ensureArabicShapingJoinerForText } from './terminal-arabic-shaping-joiner' import { buildDefaultTerminalOptions, @@ -531,6 +531,7 @@ describe('openTerminal — addon and provider wiring', () => { }), attachCustomWheelEventHandler: vi.fn(), onWriteParsed: vi.fn(() => ({ dispose: vi.fn() })), + refresh: vi.fn(), write: vi.fn(() => { events.push('write') }), @@ -588,6 +589,31 @@ describe('openTerminal — addon and provider wiring', () => { // unicode v11 is activated (still on default v6 width tables), wide chars // lay out as single cells. The bug surfaces as the broken `?`-style glyphs // users saw on worktree switch. + it('builds one initial WebGL atlas with ligatures and still rebuilds on a live toggle', async () => { + await primeTerminalWebglAddon() + resetTerminalWebglSuggestion() + vi.mocked(WebglAddon).mockClear() + webglMock.dispose.mockClear() + vi.stubGlobal('navigator', { platform: 'MacIntel', userAgent: 'Macintosh' }) + const { pane } = createOpenTerminalHarness() + pane.terminalGpuAcceleration = 'auto' + pane.gpuRenderingEnabled = true + + openTerminal(pane, true) + expect(pane.ligaturesAddon).not.toBeNull() + expect(pane.webglAddon).not.toBeNull() + const addons = vi.mocked(pane.terminal.loadAddon).mock.calls.map(([addon]) => addon) + expect(addons.indexOf(pane.ligaturesAddon!)).toBeLessThan(addons.indexOf(pane.webglAddon!)) + setLigaturesEnabled(pane, true) + expect(WebglAddon).toHaveBeenCalledTimes(1) + expect(webglMock.dispose).not.toHaveBeenCalled() + + setLigaturesEnabled(pane, false) + expect(WebglAddon).toHaveBeenCalledTimes(2) + expect(webglMock.dispose).toHaveBeenCalledTimes(1) + expect(pane.ligaturesAddon).toBeNull() + }) + it('activates unicode 11 before any caller-driven write would be possible', () => { const { pane, events } = createOpenTerminalHarness() diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts index 3c89a6ec723..cf4b50783d1 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts @@ -28,7 +28,7 @@ import { installTerminalImeCandidateAnchor } from './terminal-ime-candidate-anch export { createPaneDOM } from './pane-dom-creation' /** Open terminal into its container and load addons. Must be called after the container is in the DOM. */ -export function openTerminal(pane: ManagedPaneInternal): void { +export function openTerminal(pane: ManagedPaneInternal, ligaturesEnabled = false): void { const { terminal, container, @@ -100,6 +100,10 @@ export function openTerminal(pane: ManagedPaneInternal): void { pane.focusClassSyncCleanup = attachDomRendererFocusClassSync(terminal.element) + // Configure the first atlas with ligatures instead of immediately rebuilding it. + if (ligaturesEnabled) { + attachLigatures(pane) + } if (pane.gpuRenderingEnabled) { attachWebgl(pane) } diff --git a/src/renderer/src/lib/pane-manager/pane-manager-pane-creation.ts b/src/renderer/src/lib/pane-manager/pane-manager-pane-creation.ts index 483f49a1f55..c4c3a28c4ca 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager-pane-creation.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager-pane-creation.ts @@ -18,7 +18,7 @@ export function createInitialManagedPane( overflow: 'hidden' }) host.root.appendChild(pane.container) - openTerminal(pane) + openTerminal(pane, host.options.terminalLigaturesEnabled?.()) host.setActivePaneId(pane.id) applyPaneOpacity(host.panes.values(), host.getActivePaneId(), host.getStyleOptions()) diff --git a/src/renderer/src/lib/pane-manager/pane-manager-types.ts b/src/renderer/src/lib/pane-manager/pane-manager-types.ts index 7b23c177236..00637ae7f97 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager-types.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager-types.ts @@ -63,6 +63,7 @@ export type PaneManagerOptions = { resolveExternalPaneDropTarget?: PaneExternalDropResolver onExternalPaneDrop?: PaneExternalDropHandler terminalOptions?: (paneId: number) => Partial + terminalLigaturesEnabled?: () => boolean terminalTuiScrollSensitivity?: () => number | undefined onLinkClick?: (paneId: number, event: MouseEvent | undefined, url: string) => void /** Resolved per hover so link-routing setting changes apply without recreating panes. */ diff --git a/src/renderer/src/lib/pane-manager/pane-split-close.ts b/src/renderer/src/lib/pane-manager/pane-split-close.ts index df725156655..c5c71b03be6 100644 --- a/src/renderer/src/lib/pane-manager/pane-split-close.ts +++ b/src/renderer/src/lib/pane-manager/pane-split-close.ts @@ -141,7 +141,7 @@ function openSplitPane( newPane: ManagedPaneInternal, cwd?: string ): void { - openTerminal(newPane) + openTerminal(newPane, args.managerOptions.terminalLigaturesEnabled?.()) applyPaneOpacity(args.panes.values(), newPane.id, args.styleOptions) applyDividerStyles(args.root, args.styleOptions) newPane.terminal.focus() diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index 6387f200b4c..fb8161b9f90 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -111,6 +111,33 @@ describeBinaryCompatibility('real Git binary compatibility', () => { } }) + it('quietly distinguishes present and absent branch refs', async () => { + const head = (await runGit(['rev-parse', 'HEAD'])).stdout.trim() + await runGit(['branch', 'quiet-probe-present', head]) + await expect( + runGit(['rev-parse', '--verify', '--quiet', 'refs/heads/quiet-probe-present']) + ).resolves.toMatchObject({ stdout: `${head}\n`, stderr: '' }) + await expect( + runGit(['rev-parse', '--verify', '--quiet', 'refs/heads/quiet-probe-absent']) + ).rejects.toMatchObject({ code: 1, stdout: '', stderr: '' }) + }) + + it('distinguishes an absent branch from a ref pointing at a missing object', async () => { + const missingObject = 'a'.repeat(40) + const refPath = join(repoPath, '.git', 'refs', 'heads', 'quiet-probe-dangling') + await writeFile(refPath, `${missingObject}\n`) + try { + await expect( + runGit(['rev-parse', '--verify', '--quiet', 'refs/heads/quiet-probe-dangling']) + ).resolves.toMatchObject({ stdout: `${missingObject}\n`, stderr: '' }) + await expect( + runGit(['rev-parse', '--verify', '--quiet', 'refs/heads/quiet-probe-dangling^{commit}']) + ).rejects.toMatchObject({ code: 1, stdout: '', stderr: '' }) + } finally { + await rm(refPath) + } + }) + it('recognizes worktree-list and rev-parse compatibility boundaries', async () => { await expectPreferredOrRecognizedFallback( ['worktree', 'list', '--porcelain', '-z'], diff --git a/tests/e2e/helpers/electron-crashpad-cleanup.ts b/tests/e2e/helpers/electron-crashpad-cleanup.ts new file mode 100644 index 00000000000..8ec1a98b26b --- /dev/null +++ b/tests/e2e/helpers/electron-crashpad-cleanup.ts @@ -0,0 +1,46 @@ +import { execFileSync } from 'node:child_process' +import path from 'node:path' + +function ownsCrashpad(command: string, userDataDir: string): boolean { + return ( + command.includes('/chrome_crashpad_handler ') && + command.includes(` --database=${path.join(userDataDir, 'Crashpad')} `) + ) +} + +export function cleanupE2ECrashpad(userDataDir: string): void { + if (process.platform !== 'darwin') { + return + } + + // macOS reparents Crashpad before app exit; its inherited stderr can keep Playwright open. + try { + const table = execFileSync('ps', ['-axo', 'pid=,command='], { + encoding: 'utf8', + timeout: 5_000 + }) + for (const row of table.split('\n')) { + const match = row.match(/^\s*(\d+)\s+(.+)$/) + if (!match || !ownsCrashpad(match[2], userDataDir)) { + continue + } + const pid = Number(match[1]) + if (!Number.isSafeInteger(pid) || pid <= 1) { + continue + } + try { + const command = execFileSync('ps', ['-p', String(pid), '-o', 'command='], { + encoding: 'utf8', + timeout: 5_000 + }) + if (ownsCrashpad(command, userDataDir)) { + process.kill(pid, 'SIGTERM') + } + } catch { + // The test-owned reporter may already have exited. + } + } + } catch { + // Cleanup remains best-effort when process enumeration is unavailable. + } +} diff --git a/tests/e2e/helpers/electron-crashpad-cleanup.unit.test.ts b/tests/e2e/helpers/electron-crashpad-cleanup.unit.test.ts new file mode 100644 index 00000000000..cdf552e2548 --- /dev/null +++ b/tests/e2e/helpers/electron-crashpad-cleanup.unit.test.ts @@ -0,0 +1,45 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { execFileSync } from 'node:child_process' +import path from 'node:path' +import { cleanupE2ECrashpad } from './electron-crashpad-cleanup' + +vi.mock('node:child_process', () => ({ execFileSync: vi.fn() })) + +const profile = '/tmp/test profile' +const database = path.join(profile, 'Crashpad') +const reporter = `/Electron Framework/Helpers/chrome_crashpad_handler --database=${database} --annotation=prod=Electron` + +afterEach(() => vi.restoreAllMocks()) + +describe('test-owned macOS Crashpad cleanup', () => { + it('terminates only the reporter for the exact temporary profile after rechecking ownership', () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + const kill = vi.spyOn(process, 'kill').mockReturnValue(true) + vi.mocked(execFileSync) + .mockReturnValueOnce( + `111 ${reporter}\n222 ${reporter.replace('Crashpad ', 'Crashpad-old ')}\n333 ${reporter.replace('test profile', 'another profile')}\n444 /bin/echo --database=${database} \n` + ) + .mockReturnValueOnce(reporter) + cleanupE2ECrashpad(profile) + expect(kill).toHaveBeenCalledExactlyOnceWith(111, 'SIGTERM') + expect(execFileSync).toHaveBeenLastCalledWith('ps', ['-p', '111', '-o', 'command='], { + encoding: 'utf8', + timeout: 5_000 + }) + }) + + it('does not signal a PID whose ownership changed after enumeration', () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + const kill = vi.spyOn(process, 'kill').mockReturnValue(true) + vi.mocked(execFileSync).mockReturnValueOnce(`111 ${reporter}`).mockReturnValueOnce('/bin/sh') + cleanupE2ECrashpad(profile) + expect(kill).not.toHaveBeenCalled() + }) + + it.each(['win32', 'linux'] as const)('does not enumerate processes on %s', (platform) => { + vi.spyOn(process, 'platform', 'get').mockReturnValue(platform) + vi.mocked(execFileSync).mockClear() + cleanupE2ECrashpad(profile) + expect(execFileSync).not.toHaveBeenCalled() + }) +}) diff --git a/tests/e2e/helpers/electron-process-shutdown.ts b/tests/e2e/helpers/electron-process-shutdown.ts index 48ddb60bf43..f9b642a676e 100644 --- a/tests/e2e/helpers/electron-process-shutdown.ts +++ b/tests/e2e/helpers/electron-process-shutdown.ts @@ -2,6 +2,7 @@ import type { ChildProcess } from 'node:child_process' import { execFileSync } from 'node:child_process' import { existsSync, readFileSync, readdirSync } from 'node:fs' import path from 'node:path' +import { cleanupE2ECrashpad } from './electron-crashpad-cleanup' import type { ElectronApplication } from '@stablyai/playwright-test' const GRACEFUL_CLOSE_TIMEOUT_MS = 10_000 @@ -238,4 +239,5 @@ export async function cleanupE2EDaemons(userDataDir: string): Promise { for (const pid of readDaemonPidFiles(userDataDir)) { await forceKillPidTree(pid) } + cleanupE2ECrashpad(userDataDir) } diff --git a/tests/e2e/helpers/ssh-recovery-input-observation.ts b/tests/e2e/helpers/ssh-recovery-input-observation.ts new file mode 100644 index 00000000000..06b4bd7de3e --- /dev/null +++ b/tests/e2e/helpers/ssh-recovery-input-observation.ts @@ -0,0 +1,53 @@ +import type { Page, TestInfo } from '@playwright/test' +import type { RuntimeTerminalListResult } from '../../../src/shared/runtime-types' + +export async function attachSshRecoveryInputObservation( + page: Page, + testInfo: TestInfo, + targetId: string, + originalPtyId: string, + label: string +): Promise { + const observation = await page.evaluate( + async ({ targetId, originalPtyId }) => { + const state = window.__store?.getState() + const panes = [...(window.__paneManagers?.entries() ?? [])].flatMap(([tabId, manager]) => + manager.getPanes().map((pane) => ({ + tabId, + leafId: pane.leafId, + ptyId: pane.container.dataset.ptyId, + active: manager.getActivePane()?.id === pane.id + })) + ) + let timer: ReturnType | undefined + try { + const runtime = await Promise.race([ + window.api.runtime + .call({ method: 'terminal.list', params: { limit: 50, includeVisualLayouts: false } }) + .then((response) => + response.ok + ? { terminals: (response.result as RuntimeTerminalListResult).terminals } + : { error: response.error } + ), + new Promise<{ error: string }>((resolve) => { + timer = setTimeout(() => resolve({ error: 'Observation timed out' }), 1000) + }) + ]) + return { + originalPtyId, + authority: state?.sshConnectionStates.get(targetId), + activeWorktreeId: state?.activeWorktreeId, + panes, + runtime + } + } finally { + clearTimeout(timer) + } + }, + { targetId, originalPtyId } + ) + await testInfo.attach(`ssh-input-${label}.json`, { + body: JSON.stringify(observation, null, 2), + contentType: 'application/json' + }) +} diff --git a/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts b/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts index 42ac316790c..c64761ede80 100644 --- a/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts +++ b/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts @@ -4,6 +4,7 @@ import type { ElectronApplication } from '@playwright/test' import { test, expect } from './helpers/orca-app' import { DEFAULT_LOCAL_ORCA_PROFILE_ID } from '../../src/shared/orca-profiles' import { sshRemotePtyLeaseAllowsReattach, type SshRemotePtyLease } from '../../src/shared/ssh-types' +import { toRelaySshPtyId } from '../../src/shared/ssh-pty-id' import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' import { execInTerminal, @@ -29,6 +30,8 @@ import { withStalledDockerSshRelayTarget } from './helpers/docker-ssh-relay-faults' +import { attachSshRecoveryInputObservation } from './helpers/ssh-recovery-input-observation' + const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1' /** @@ -338,16 +341,21 @@ test.describe('SSH transport drop recovery', () => { const generations: string[][] = [] for (let generation = 1; generation <= 5; generation++) { - const predecessor = await waitForActivePanePtyId(orcaPage, 60_000) + const previousPtyId = await waitForActivePanePtyId(orcaPage, 60_000) await recoverDockerSshRelayAfterFault(orcaPage, remote.targetId, () => { - expect(killDockerSshRelayDaemon(target!)).toBeGreaterThan(0) + expect( + killDockerSshRelayDaemon(target!), + 'no relay process was found to kill' + ).toBeGreaterThan(0) }) - await expect - .poll(() => waitForActivePanePtyId(orcaPage, 60_000), { timeout: 120_000 }) - .not.toBe(predecessor) await waitForActiveTerminalManager(orcaPage, 120_000) - // The pane must be usable again before the count is meaningful: recovery is what mints the - // successor lease that retires the generation before it. + // Transport status can still be connected while the pane retains its old binding. + await expect + .poll(() => waitForActivePanePtyId(orcaPage, 60_000).catch(() => previousPtyId), { + timeout: 120_000, + message: `pane kept its old PTY binding after relay kill ${generation}` + }) + .not.toBe(previousPtyId) const ptyId = await waitForActivePanePtyId(orcaPage, 120_000) const markerSuffix = `${generation}_${Date.now()}` const marker = `LEASE_GEN_${markerSuffix}` @@ -356,15 +364,14 @@ test.describe('SSH transport drop recovery', () => { try { await expect - .poll(() => readReattachablePtyIds(userDataDir, remote.targetId).length, { + .poll(() => readReattachablePtyIds(userDataDir, remote.targetId), { timeout: 60_000 }) - .toBe(1) + .toEqual([toRelaySshPtyId(remote.targetId, ptyId)]) } catch (error) { - // Why re-thrown with the rows: the count alone cannot say WHICH predecessor stayed - // reattachable, and the user-data dir is torn down before the report is read. + // Preserve lease ownership diagnostics before the user-data directory is removed. throw new Error( - `reattachable lease count never settled at 1 in generation ${generation}; leases: ${describeSshLeases(userDataDir, remote.targetId)}`, + `reattachable leases never settled at the active PTY ${ptyId} in generation ${generation}; leases: ${describeSshLeases(userDataDir, remote.targetId)}`, { cause: error } ) } @@ -432,6 +439,7 @@ test.describe('SSH transport drop recovery', () => { test('accepts input again after a frozen host resumes', async ({ orcaPage }, testInfo) => { test.slow() let target: DockerSshRelayTarget | null = null + let observationTarget: { targetId: string; ptyId: string } | undefined try { target = startDockerSshRelayTarget(testInfo) enableDockerSshRelayTargetShellTitle(target) @@ -444,6 +452,18 @@ test.describe('SSH transport drop recovery', () => { await waitForActiveTerminalManager(orcaPage, 60_000) const ptyId = await waitForActivePanePtyId(orcaPage, 60_000) + observationTarget = { targetId: remote.targetId, ptyId } + const beforeSuffix = Date.now() + await execInTerminal(orcaPage, ptyId, `printf 'STALL_BEFORE_%s\\n' ${beforeSuffix}`) + await waitForTerminalOutput(orcaPage, `STALL_BEFORE_${beforeSuffix}`, 60_000) + await attachSshRecoveryInputObservation( + orcaPage, + testInfo, + remote.targetId, + ptyId, + 'before-freeze' + ) + await recoverDockerSshRelayAfterFault(orcaPage, remote.targetId, async () => { await withStalledDockerSshRelayTarget(target!, async () => { await orcaPage.waitForTimeout(30_000) @@ -454,7 +474,25 @@ test.describe('SSH transport drop recovery', () => { const afterSuffix = Date.now() const afterMarker = `STALL_AFTER_${afterSuffix}` await execInTerminal(orcaPage, ptyId, `printf 'STALL_AFTER_%s\\n' ${afterSuffix}`) + await attachSshRecoveryInputObservation( + orcaPage, + testInfo, + remote.targetId, + ptyId, + 'after-write' + ) await waitForTerminalOutput(orcaPage, afterMarker, 60_000) + } catch (error) { + if (observationTarget) { + await attachSshRecoveryInputObservation( + orcaPage, + testInfo, + observationTarget.targetId, + observationTarget.ptyId, + 'failure-before-cleanup' + ).catch(() => undefined) + } + throw error } finally { if (target) { clearDockerSshRelayFaults(target) From 9faa27c5f4e3f476393aff1e17429242483e8038 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 15:12:19 -0700 Subject: [PATCH 08/10] test: align desktop platform oracles with native behavior (#18915) --- .../right-sidebar-windows-titlebar.spec.ts | 42 ++++-------- tests/e2e/settings-agent-awake.spec.ts | 68 ++++++++++++++----- 2 files changed, 64 insertions(+), 46 deletions(-) diff --git a/tests/e2e/right-sidebar-windows-titlebar.spec.ts b/tests/e2e/right-sidebar-windows-titlebar.spec.ts index 1d6d4b8981f..ce39d7de28d 100644 --- a/tests/e2e/right-sidebar-windows-titlebar.spec.ts +++ b/tests/e2e/right-sidebar-windows-titlebar.spec.ts @@ -6,41 +6,19 @@ type RightSidebarHeaderGeometry = { stripTop: number closeTop: number titlebarActivityButtonCount: number + activityButtonCount: number firstButtonCenterHitsFirst: boolean lastButtonCenterHitsLast: boolean } -test.describe('Right sidebar Windows titlebar spacing', () => { - test('top activity buttons render inside the sidebar instead of the titlebar', async ({ - orcaPage - }) => { - await orcaPage.addInitScript(() => { - const userAgent = - 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 Chrome/146 Safari/537.36' - Object.defineProperty(navigator, 'userAgent', { - get: () => userAgent, - configurable: true - }) - }) - await orcaPage.reload({ waitUntil: 'domcontentloaded' }) - await orcaPage.waitForFunction(() => Boolean(window.__store), null, { timeout: 30_000 }) +test.describe('Right sidebar native titlebar spacing', () => { + test('top activity buttons follow the native desktop chrome layout', async ({ orcaPage }) => { await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) await ensureTerminalVisible(orcaPage) - await expect - .poll( - async () => - orcaPage.evaluate(() => ({ - hasWindowsUserAgent: navigator.userAgent.includes('Windows'), - hasWindowsTitlebarChrome: Boolean(document.querySelector('.window-controls')) - })), - { - timeout: 5_000, - message: 'Renderer did not switch to the Windows titlebar branch' - } - ) - .toEqual({ hasWindowsUserAgent: true, hasWindowsTitlebarChrome: true }) + const hasDesktopWindowChrome = process.platform !== 'darwin' + expect(await orcaPage.evaluate(() => window.api.platform.get().platform)).toBe(process.platform) await orcaPage.evaluate(() => { const store = window.__store @@ -95,6 +73,7 @@ test.describe('Right sidebar Windows titlebar spacing', () => { stripTop: stripRect.top, closeTop: closeRect.top, titlebarActivityButtonCount, + activityButtonCount: activityButtons.length, firstButtonCenterHitsFirst: elementAtFirstCenter !== null && firstButton.contains(elementAtFirstCenter), lastButtonCenterHitsLast: @@ -117,8 +96,13 @@ test.describe('Right sidebar Windows titlebar spacing', () => { .toBe(true) expect(headerGeometry).not.toBeNull() - expect(headerGeometry!.titlebarActivityButtonCount).toBe(0) - expect(headerGeometry!.stripTop).toBeGreaterThanOrEqual(headerGeometry!.headerBottom) + if (hasDesktopWindowChrome) { + expect(headerGeometry!.titlebarActivityButtonCount).toBe(0) + expect(headerGeometry!.stripTop).toBeGreaterThanOrEqual(headerGeometry!.headerBottom) + } else { + expect(headerGeometry!.titlebarActivityButtonCount).toBe(headerGeometry!.activityButtonCount) + expect(headerGeometry!.stripTop).toBeLessThan(headerGeometry!.headerBottom) + } expect(headerGeometry!.closeTop).toBeLessThan(headerGeometry!.headerBottom) expect(headerGeometry!.firstButtonCenterHitsFirst).toBe(true) expect(headerGeometry!.lastButtonCenterHitsLast).toBe(true) diff --git a/tests/e2e/settings-agent-awake.spec.ts b/tests/e2e/settings-agent-awake.spec.ts index 8a2ad840a14..ebea82a1241 100644 --- a/tests/e2e/settings-agent-awake.spec.ts +++ b/tests/e2e/settings-agent-awake.spec.ts @@ -1,4 +1,5 @@ import { randomUUID } from 'node:crypto' +import { runProcess } from '../../src/shared/child-process/run-process' import type { ElectronApplication, Page } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' import { waitForSessionReady } from './helpers/store' @@ -104,6 +105,19 @@ async function readPowerSaveBlockerProbe( }) } +async function readMacosSleepAssertionPids(electronApp: ElectronApplication): Promise { + const result = await runProcess({ + program: '/usr/bin/pgrep', + args: ['-P', String(electronApp.process().pid), '-f', '^/usr/bin/caffeinate -i -s$'], + maxOutputBytes: 4_096 + }) + if (result.code === 1) { + return [] + } + expect(result.code, result.stderr).toBe(0) + return result.stdout.trim().split(/\s+/).filter(Boolean).map(Number) +} + async function postCodexHookEvent( electronApp: ElectronApplication, options: { @@ -176,7 +190,9 @@ test.describe('Agent awake setting', () => { electronApp, orcaPage }) => { - await installPowerSaveBlockerProbe(electronApp) + if (process.platform !== 'darwin') { + await installPowerSaveBlockerProbe(electronApp) + } await setKeepAwake(orcaPage, true) const tabId = 'e2e-awake-tab' @@ -187,24 +203,33 @@ test.describe('Agent awake setting', () => { eventName: 'UserPromptSubmit' }) - await expect - .poll(async () => await readPowerSaveBlockerProbe(electronApp), { - timeout: 5_000, - message: 'powerSaveBlocker did not start for the working agent' - }) - .toEqual( - expect.objectContaining({ - activeIds: expect.arrayContaining([expect.any(Number)]), - starts: expect.arrayContaining([ - expect.objectContaining({ type: 'prevent-display-sleep' }) - ]) + await expect( + orcaPage.getByRole('button', { name: 'Keep computer awake, Agent · Active' }) + ).toBeVisible() + let startedIds: number[] = [] + if (process.platform === 'darwin') { + // macOS uses an app-owned caffeinate assertion instead of Electron's display blocker. + await expect + .poll(() => readMacosSleepAssertionPids(electronApp), { timeout: 5_000 }) + .not.toEqual([]) + } else { + await expect + .poll(async () => await readPowerSaveBlockerProbe(electronApp), { + timeout: 5_000, + message: 'powerSaveBlocker did not start for the working agent' }) - ) + .toEqual( + expect.objectContaining({ + activeIds: expect.arrayContaining([expect.any(Number)]), + starts: expect.arrayContaining([ + expect.objectContaining({ type: 'prevent-display-sleep' }) + ]) + }) + ) - const startedIds = (await readPowerSaveBlockerProbe(electronApp)).starts.map( - (start) => start.id - ) - expect(startedIds.length).toBeGreaterThan(0) + startedIds = (await readPowerSaveBlockerProbe(electronApp)).starts.map((start) => start.id) + expect(startedIds.length).toBeGreaterThan(0) + } await postCodexHookEvent(electronApp, { paneKey, @@ -212,6 +237,15 @@ test.describe('Agent awake setting', () => { eventName: 'Stop' }) + await expect( + orcaPage.getByRole('button', { name: 'Keep computer awake, Agent · Inactive' }) + ).toBeVisible() + if (process.platform === 'darwin') { + await expect + .poll(() => readMacosSleepAssertionPids(electronApp), { timeout: 5_000 }) + .toEqual([]) + return + } await expect .poll(async () => await readPowerSaveBlockerProbe(electronApp), { timeout: 5_000, From 239e3c7e0ba5b41545f440089f76e4b696385abd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 15:29:46 -0700 Subject: [PATCH 09/10] test: select seeded workspace and confirm sidebar reveal (#18921) --- tests/e2e/worktree-scroll-to-current.spec.ts | 24 +++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/e2e/worktree-scroll-to-current.spec.ts b/tests/e2e/worktree-scroll-to-current.spec.ts index 19c51005cfe..d61d96847a0 100644 --- a/tests/e2e/worktree-scroll-to-current.spec.ts +++ b/tests/e2e/worktree-scroll-to-current.spec.ts @@ -39,22 +39,30 @@ test.describe('Reveal active workspace button', () => { // the "outside the virtualized window" test below. test('clears sidebar filters before revealing a hidden current workspace', async ({ - orcaPage + orcaPage, + testRepoPath }) => { await prepareSidebarForScrollTest(orcaPage) - const renderedOptions = orcaPage.locator('[data-worktree-sidebar] [role="option"]') - await expect(renderedOptions).toHaveCount(2) - - const targetId = await renderedOptions.last().getAttribute('data-worktree-id') + // Other specs can add worktrees to the shared repository before this test runs. + const targetId = await orcaPage.evaluate((repoPath) => { + const state = window.__store!.getState() + const repo = state.repos.find((candidate) => candidate.path === repoPath) + return repo + ? state.worktreesByRepo[repo.id]?.find( + (worktree) => worktree.branch === 'refs/heads/e2e-secondary' + )?.id + : undefined + }, testRepoPath) if (!targetId) { - throw new Error('Bottom workspace row did not expose a data-worktree-id') + throw new Error('Seeded secondary worktree is missing') } const targetRows = orcaPage.locator( `[data-worktree-sidebar] [data-worktree-id=${JSON.stringify(targetId)}]` ) const targetRow = targetRows.first() + await expect(targetRows.and(orcaPage.getByRole('option'))).toHaveCount(1) const revealButton = orcaPage.getByRole('button', { name: 'Reveal active workspace' }) await orcaPage.evaluate((targetId) => { @@ -92,6 +100,10 @@ test.describe('Reveal active workspace button', () => { // contract under test is that reveal clears the filter (asserted below). await revealButton.click() + await orcaPage + .getByRole('dialog', { name: 'Reveal hidden workspace?' }) + .getByRole('button', { name: 'Clear filters and reveal' }) + .click() await expect(targetRow).toBeVisible() await expect(targetRow).toHaveAttribute('data-scroll-reveal-highlight', 'true') From 471a5f4aa795aaca11ea1a1a7b8dc17bd1330915 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 5 Sep 2026 15:33:04 -0700 Subject: [PATCH 10/10] feat(native-chat): model Codex MCP and web-search items instead of leaking opcodes (#18763) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(native-chat): model Codex MCP and web-search items instead of leaking opcodes Codex's app-server sends 19 thread-item types; the structured translator handled six. The rest fell through to a generic gray `codex · item:` row, even though the disposition table's own comment says it exists so a new item type cannot leak like that — the table had one entry. Give `mcpToolCall` and `webSearch` real tool-call bodies, and chrome `sleep`, which carries only a duration and renders as nothing in Codex's own TUI. `subAgentActivity` and `collabAgentToolCall` deliberately keep their generic rows. They arrive in real sessions today and are currently the only visible sign a subagent is running; hiding them before the subagent UI lands would render minutes of work as an idle turn. Tests pin that they stay visible. MCP tool names pass through verbatim when they contain `:`, `.`, `/` or `__`, so `mcp__server__tool` survives instead of being title-cased into nonsense. * fix(native-chat): keep Codex MCP tool identity and web-search results on the row Four fixes to the Codex MCP / web-search item bodies: - Drop the title-casing display name. `get_forecast` became `Get Forecast`, which no longer matches the raw snake_case identifiers that the diff renderer, question parsers, and tool-input previews dispatch on, and does not match how the Claude lane or the sibling `shell`/`apply_patch`/`web_search` bodies name a tool. The row name is now `server/tool` verbatim, the bare `tool` when no server is given, and `mcp` when the item names no tool at all. Server-qualifying also stops an MCP tool that happens to be called `apply_patch` from hijacking the diff renderer. - Pass the MCP call's own `arguments` as the tool input instead of wrapping it in `{server, tool, arguments}`. Row-label derivation only reads top-level keys, so the wrapper degraded every MCP row to a truncated raw JSON blob. A non-object `arguments` stays addressable under a key rather than being dropped; an absent one becomes null, which labels as empty rather than `{}`. - Carry a web search's `results` as the call output, bounded like every other inline payload and omitted when there are none. They were being dropped entirely, which showed less than the generic fallback row it replaced. - No streaming branches were added for these two item types: the Codex delta stream is a closed set of six methods that neither can reach, so such branches would be unreachable. * fix(native-chat): label Codex web searches and argument-less MCP calls A row label is derived from top-level `input` keys only, so a webSearch whose detail lives inside `action` — an opened page, an in-page find, or a bare `other` — fell through to the raw JSON of the whole input, as did the empty `query` Codex leaves on a completed search. Hoist the action's `url`, `pattern` and `type` beside the query, keep the full `action` object so the expanded detail loses nothing, and emit no input at all for the start frame. An MCP tool that takes no arguments sends `arguments: {}`, which passed straight through and labelled the row a literal `{}`; treat it as absent so the row reads as a bare `server/tool`. Split the durable-identity half of the item translator into `codex-thread-item-identity.ts`, re-exported so every existing import is unchanged, to keep both files under the max-lines cap. --------- Co-authored-by: Merge Sim --- src/main/codex/codex-command-action-class.ts | 71 +++++ src/main/codex/codex-item-field-readers.ts | 45 +++ .../codex-structured-item-translation.test.ts | 235 ++++++++++++++- .../codex-structured-item-translation.ts | 278 +++++++----------- src/main/codex/codex-thread-item-identity.ts | 65 ++++ .../provider-frame-disposition.test.ts | 48 +++ .../provider-frame-disposition.ts | 7 +- 7 files changed, 567 insertions(+), 182 deletions(-) create mode 100644 src/main/codex/codex-command-action-class.ts create mode 100644 src/main/codex/codex-item-field-readers.ts create mode 100644 src/main/codex/codex-thread-item-identity.ts diff --git a/src/main/codex/codex-command-action-class.ts b/src/main/codex/codex-command-action-class.ts new file mode 100644 index 00000000000..81360691ef9 --- /dev/null +++ b/src/main/codex/codex-command-action-class.ts @@ -0,0 +1,71 @@ +import { readRecord, readString } from './codex-item-field-readers' +import type { CodexThreadItem } from './codex-thread-item-identity' + +/** + * Codex's own classification of a shell call: the tool name to show, and the + * fields worth lifting into `input` for the shared label helper (a file target, + * a search term, a scanned root). A `Map`, not an object — an object index + * answers `__proto__` with a truthy non-string. Every other action type stays an + * unclassified `shell` row. + * + * Nothing is invented for a field Codex sends as null: a stand-in path is a + * claim about a target, and the label helper turns any path into a file link. + */ +type CommandActionClass = { + name: string + /** Action field to the `input` key it lifts to. A scan root and a listed + * directory lift to `directory`, never `path`: the label helper reads `path` + * as a file target, which mobile turns into a tappable open-file link. */ + keys: Readonly> +} + +const COMMAND_ACTION_CLASSES = new Map([ + ['read', { name: 'read', keys: { path: 'path' } }], + ['search', { name: 'search', keys: { query: 'query', path: 'directory' } }], + ['listFiles', { name: 'list', keys: { path: 'directory' } }] +]) + +/** The one class every classified `commandActions` entry agrees on, with the + * fields they all agree on; null leaves the row exactly as a Codex that sends no + * classification renders it. `cat a.txt && ls src` classifies as two different + * things, and naming that row after either would drop the other, so it stays a + * `shell` row that shows the whole command. */ +export function commandActionFacts( + item: CodexThreadItem +): { name: string; fields: Record } | null { + const actions = item.commandActions + if (!Array.isArray(actions)) { + return null + } + let matched: { class: CommandActionClass; fields: Record } | null = null + for (const action of actions) { + const record = readRecord(action) + const type = readString(record, 'type') + const classified = type === null ? undefined : COMMAND_ACTION_CLASSES.get(type) + if (classified === undefined) { + continue + } + if (matched === null) { + const fields: Record = {} + for (const [source, lifted] of Object.entries(classified.keys)) { + const value = readString(record, source) + if (value !== null) { + fields[lifted] = value + } + } + matched = { class: classified, fields } + continue + } + if (matched.class.name !== classified.name) { + return null + } + // The same class twice keeps the class, but only a target both entries name. + for (const [source, lifted] of Object.entries(matched.class.keys)) { + const kept = matched.fields[lifted] + if (kept !== undefined && readString(record, source) !== kept) { + delete matched.fields[lifted] + } + } + } + return matched === null ? null : { name: matched.class.name, fields: matched.fields } +} diff --git a/src/main/codex/codex-item-field-readers.ts b/src/main/codex/codex-item-field-readers.ts new file mode 100644 index 00000000000..bbe2551615a --- /dev/null +++ b/src/main/codex/codex-item-field-readers.ts @@ -0,0 +1,45 @@ +// Field readers for the loosely-typed records Codex sends on thread items. + +export function readRecord(value: unknown): Record { + return typeof value === 'object' && value !== null ? (value as Record) : {} +} + +export function readString(source: Record, key: string): string | null { + const value = source[key] + return typeof value === 'string' && value.length > 0 ? value : null +} + +export function readFirstString( + source: Record, + keys: readonly string[] +): string | null { + for (const key of keys) { + const value = readString(source, key) + if (value !== null) { + return value + } + } + return null +} + +export function readTextContent(source: Record, key: string): string | null { + const direct = readString(source, key) + if (direct) { + return direct + } + const value = source[key] + if (!Array.isArray(value)) { + return null + } + const parts = value.flatMap((part) => { + if (typeof part === 'string') { + return part.length > 0 ? [part] : [] + } + if (typeof part !== 'object' || part === null) { + return [] + } + const text = readString(part as Record, 'text') + return text ? [text] : [] + }) + return parts.length > 0 ? parts.join('\n') : null +} diff --git a/src/main/codex/codex-structured-item-translation.test.ts b/src/main/codex/codex-structured-item-translation.test.ts index 1d64158cb60..2558f4b60de 100644 --- a/src/main/codex/codex-structured-item-translation.test.ts +++ b/src/main/codex/codex-structured-item-translation.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from 'vitest' import { agentJournalItemKey } from '../../shared/agent-session-journal-item-key' -import { createToolInputDisplay } from '../../shared/native-chat-tool-summary' +import { + briefToolArg, + createToolInputDisplay, + describeToolInput +} from '../../shared/native-chat-tool-summary' import { codexItemBody, codexItemIdentity, @@ -14,6 +18,13 @@ import { type CodexThreadItem } from './codex-structured-item-translation' +/** The tool-call input a Codex item lands on, which is what the row label and + * the collapsed run header are both derived from. */ +function toolCallInput(item: CodexThreadItem): unknown { + const body = codexItemBody(item) + return body !== null && body.kind === 'tool-call' ? body.input : null +} + const THREAD_ID = 'thread-abc' const TURN_ID = 'turn-1' @@ -572,10 +583,226 @@ describe('codex item bodies', () => { }) expect(codexItemBody({ type: 'reasoning', id: 'r' })).toBeNull() expect(codexItemBody({ type: 'agentMessage', id: 'm', text: '' })).toBeNull() - expect(codexItemBody({ type: 'webSearch', id: 'w' })).toMatchObject({ + expect(codexItemBody({ type: 'somethingCodexAddedLater', id: 'x' })).toMatchObject({ kind: 'status', - text: 'codex · item:webSearch', - providerFrame: { provider: 'codex', kind: 'item:webSearch' } + text: 'codex · item:somethingCodexAddedLater', + providerFrame: { provider: 'codex', kind: 'item:somethingCodexAddedLater' } + }) + }) + + it('gives an mcp tool call a typed body with its own arguments as input', () => { + expect( + codexItemBody({ + type: 'mcpToolCall', + id: 'mcp-1', + server: 'weather', + tool: 'get_forecast', + status: 'completed', + arguments: { city: 'Oslo' }, + result: { content: [{ type: 'text', text: '12C' }] } + }) + ).toEqual({ + kind: 'tool-call', + // Server-qualified, and the arguments stay top level so the row label can + // read `query`/`command`/`file_path` out of them. + name: 'weather/get_forecast', + input: { city: 'Oslo' }, + state: 'completed', + output: { head: '12C', byteLength: 3, truncated: false, digest: expect.any(String) } + }) + }) + + it('passes an mcp tool name through with no casing transform', () => { + // Downstream dispatch is exact-match on raw identifiers, so every shape — + // bare snake_case included — has to survive byte-identical. + for (const tool of ['get_forecast', 'mcp__server__tool', 'ns.tool', 'urn:tool', 'listTools']) { + expect( + codexItemBody({ type: 'mcpToolCall', id: 'm', tool, status: 'inProgress' }), + tool + ).toMatchObject({ kind: 'tool-call', name: tool, state: 'running' }) + expect( + codexItemBody({ type: 'mcpToolCall', id: 'm', server: 'srv', tool, status: 'inProgress' }), + tool + ).toMatchObject({ kind: 'tool-call', name: `srv/${tool}`, state: 'running' }) + } + }) + + it('falls back to the bare tool, then to `mcp`, when the item is under-specified', () => { + expect( + codexItemBody({ type: 'mcpToolCall', id: 'm', tool: 'get_forecast', status: 'inProgress' }) + ).toMatchObject({ name: 'get_forecast' }) + expect( + codexItemBody({ type: 'mcpToolCall', id: 'm', server: '', tool: 'ping', status: 'completed' }) + ).toMatchObject({ name: 'ping' }) + expect( + codexItemBody({ type: 'mcpToolCall', id: 'm', server: 'weather', status: 'completed' }) + ).toMatchObject({ name: 'mcp' }) + }) + + it('keeps non-object mcp arguments addressable and empty ones off the label', () => { + // `arguments` is arbitrary JSON upstream; a scalar or array must still reach + // the row rather than being dropped or unwrapped into a bare value. + expect( + codexItemBody({ type: 'mcpToolCall', id: 'm', tool: 't', arguments: 'raw text' }) + ).toMatchObject({ input: { arguments: 'raw text' } }) + expect( + codexItemBody({ type: 'mcpToolCall', id: 'm', tool: 't', arguments: [1, 2] }) + ).toMatchObject({ input: { arguments: [1, 2] } }) + // `arguments` is required on the wire, so `{}` — not an absent key — is what + // an argument-less MCP tool sends, and passing it through labels the row `{}`. + expect(codexItemBody({ type: 'mcpToolCall', id: 'm', tool: 't', arguments: {} })).toEqual({ + kind: 'tool-call', + name: 't', + input: null, + state: 'running' + }) + expect(codexItemBody({ type: 'mcpToolCall', id: 'm', tool: 't' })).toMatchObject({ + input: null + }) + expect( + codexItemBody({ type: 'mcpToolCall', id: 'm', tool: 't', arguments: null }) + ).toMatchObject({ input: null }) + }) + + it('renders an argument-less mcp call as a bare server/tool row', () => { + const input = toolCallInput({ + type: 'mcpToolCall', + id: 'm', + server: 'srv', + tool: 'list_tools', + arguments: {} + }) + expect(describeToolInput(input)).toBe('') + expect(briefToolArg(input)).toBe('') + }) + + it('reports an mcp error as a failed call carrying the server message', () => { + expect( + codexItemBody({ + type: 'mcpToolCall', + id: 'mcp-2', + server: 's', + tool: 'ping', + status: 'completed', + error: { message: 'server unreachable' } + }) + ).toMatchObject({ + kind: 'tool-call', + name: 's/ping', + state: 'failed', + output: { head: 'server unreachable', truncated: false } + }) + }) + + it('models a web search as a tool call that runs until codex sends the action', () => { + // The start frame Codex actually emits: empty query, no action. Nothing is + // labelable yet, so the input is absent rather than a hull of null keys. + expect(codexItemBody({ type: 'webSearch', id: 'w', query: '', action: null })).toEqual({ + kind: 'tool-call', + name: 'web_search', + input: null, + state: 'running' + }) + expect( + codexItemBody({ + type: 'webSearch', + id: 'w', + query: 'orca release notes', + action: { type: 'search', query: 'orca release notes', queries: null }, + results: null + }) + ).toEqual({ + kind: 'tool-call', + name: 'web_search', + input: { + query: 'orca release notes', + description: 'search', + action: { type: 'search', query: 'orca release notes', queries: null } + }, + state: 'completed' + }) + }) + + it('carries the web search hits as the call output', () => { + const results = [{ title: 'Orca 1.0', url: 'https://example.com/notes' }] + expect( + codexItemBody({ + type: 'webSearch', + id: 'w', + query: 'orca release notes', + action: { type: 'search', query: 'orca release notes', queries: null }, + results + }) + ).toMatchObject({ + kind: 'tool-call', + name: 'web_search', + state: 'completed', + output: { head: JSON.stringify(results), truncated: false } + }) + // Nothing to show is no output block at all, not an empty one. + for (const empty of [undefined, null, []]) { + expect( + codexItemBody({ + type: 'webSearch', + id: 'w', + query: 'q', + action: { type: 'search' }, + results: empty + }), + String(empty) + ).not.toHaveProperty('output') + } + }) + + it('labels every web search shape without falling back to raw JSON', () => { + // Both the row label and the run header read top-level input keys only, so a + // shape whose detail sits inside `action` renders as the input's raw JSON. + const url = 'https://example.com/docs/page' + const shapes: [string, unknown, string, string][] = [ + ['started', null, '', ''], + [ + 'search', + { type: 'search', query: 'a sample query', queries: null }, + 'a sample query', + 'a sample query' + ], + ['openPage', { type: 'openPage', url }, url, ''], + [ + 'findInPage', + { type: 'findInPage', url, pattern: 'a needle' }, + 'a sample query', + 'a sample query' + ], + ['other', { type: 'other' }, 'other', ''] + ] + for (const [name, action, label, brief] of shapes) { + // Codex leaves the item's own `query` empty on most completed searches. + const query = name === 'search' || name === 'findInPage' ? 'a sample query' : '' + const input = toolCallInput({ type: 'webSearch', id: 'w', query, action }) + expect(describeToolInput(input), name).toBe(label) + expect(briefToolArg(input), name).toBe(brief) + } + }) + + it('leaves subagent items on the generic row until a real renderer exists', () => { + expect( + codexJournalItem({ + type: 'subAgentActivity', + id: 'a-1', + kind: 'started', + agentThreadId: 'thread-child', + agentPath: '/root/list_directory' + }) + ).toMatchObject({ + handled: false, + body: { kind: 'status', providerFrame: { kind: 'item:subAgentActivity' } } + }) + }) + + it('drops the sleep item, which codex itself renders as nothing', () => { + expect(codexJournalItem({ type: 'sleep', id: 's-1', durationMs: 20_000 })).toEqual({ + body: null, + handled: true }) }) diff --git a/src/main/codex/codex-structured-item-translation.ts b/src/main/codex/codex-structured-item-translation.ts index b3609e076f5..ad08525a5f5 100644 --- a/src/main/codex/codex-structured-item-translation.ts +++ b/src/main/codex/codex-structured-item-translation.ts @@ -1,7 +1,4 @@ -import type { - AgentJournalItemBody, - AgentJournalItemIdentity -} from '../../shared/agent-session-journal-types' +import type { AgentJournalItemBody } from '../../shared/agent-session-journal-types' import type { NativeChatBlock } from '../../shared/native-chat-types' import { boundInlineText, @@ -9,116 +6,27 @@ import { DEFAULT_JOURNAL_PAYLOAD_LIMITS } from '../native-chat/agent-session-journal/journal-payload-bounds' import { unhandledProviderFrameJournalItem } from '../native-chat/agent-session-wire/unhandled-provider-frame' -import type { CodexTurnOrdinals } from './codex-turn-ordinals' +import { commandActionFacts } from './codex-command-action-class' +import { + readFirstString, + readRecord, + readString, + readTextContent +} from './codex-item-field-readers' +import type { CodexThreadItem } from './codex-thread-item-identity' +export { + codexItemIdentity, + isCodexMessageItemType, + readCodexThreadItem, + type CodexThreadItem +} from './codex-thread-item-identity' export { CodexTurnOrdinals, MAX_CODEX_TURN_ORDINAL_BYTES, MAX_CODEX_TURN_ORDINAL_ENTRIES } from './codex-turn-ordinals' -// Codex thread items → journal item bodies and durable identities. -// -// THE ORDINAL RULE, and why it is not "index within the turn". Codex renumbers -// item ids positionally on resume (`item-1`…`item-N` across the whole thread), -// and a resumed turn does NOT contain every item the live turn emitted — -// reasoning and command execution are dropped from persisted history. Numbering -// by live position would therefore shift every message after the first tool -// call and hand the user a duplicate of the assistant's answer after a resume. -// -// So the ordinal counts MESSAGE items only, and the same projection is applied -// to the live stream and to a resumed turn's item list. Any other item type — -// including ones this build does not model — is skipped identically on both -// sides, which is what makes the key survive a Codex release that adds one. - -/** Only these carry a durable `(threadId, turnId, ordinal)` identity. */ -const CODEX_MESSAGE_ITEM_TYPES = new Set(['userMessage', 'agentMessage']) - -export type CodexThreadItem = { - type: string - id: string - [key: string]: unknown -} - -export function isCodexMessageItemType(type: string): boolean { - return CODEX_MESSAGE_ITEM_TYPES.has(type) -} - -export function readCodexThreadItem(value: unknown): CodexThreadItem | null { - if (typeof value !== 'object' || value === null) { - return null - } - const record = value as Record - return typeof record.type === 'string' && typeof record.id === 'string' - ? (record as CodexThreadItem) - : null -} - -function readRecord(value: unknown): Record { - return typeof value === 'object' && value !== null ? (value as Record) : {} -} - -/** - * Durable identity for a Codex item, or null for one that has none. - * - * Non-message items fall back to the `orca` namespace keyed by the Codex item - * id. That id is unstable across resume, so those rows are live-session detail - * that a recovered journal simply will not contain — which is correct: Codex - * itself does not persist them either. - */ -export function codexItemIdentity(input: { - threadId: string - turnId: string | null - item: CodexThreadItem - ordinals: CodexTurnOrdinals -}): AgentJournalItemIdentity { - const { item, turnId } = input - if (turnId && isCodexMessageItemType(item.type)) { - return { - provider: 'codex', - threadId: input.threadId, - turnId, - ordinal: input.ordinals.ordinalFor(input.threadId, turnId, item.id) - } - } - return { provider: 'orca', clientMessageId: `codex-item:${input.threadId}:${item.id}` } -} - -function readString(source: Record, key: string): string | null { - const value = source[key] - return typeof value === 'string' && value.length > 0 ? value : null -} - -function readFirstString(source: Record, keys: readonly string[]): string | null { - for (const key of keys) { - const value = readString(source, key) - if (value !== null) { - return value - } - } - return null -} - -function readTextContent(source: Record, key: string): string | null { - const direct = readString(source, key) - if (direct) { - return direct - } - const value = source[key] - if (!Array.isArray(value)) { - return null - } - const parts = value.flatMap((part) => { - if (typeof part === 'string') { - return part.length > 0 ? [part] : [] - } - if (typeof part !== 'object' || part === null) { - return [] - } - const text = readString(part as Record, 'text') - return text ? [text] : [] - }) - return parts.length > 0 ? parts.join('\n') : null -} +// Codex thread items → journal item bodies. /** `userMessage` carries structured content parts; `agentMessage` a flat text. */ export function codexMessageBlocks(item: CodexThreadItem): NativeChatBlock[] { @@ -175,75 +83,6 @@ export type CodexJournalItem = { handled: boolean } -/** - * Codex's own classification of a shell call: the tool name to show, and the - * fields worth lifting into `input` for the shared label helper (a file target, - * a search term, a scanned root). A `Map`, not an object — an object index - * answers `__proto__` with a truthy non-string. Every other action type stays an - * unclassified `shell` row. - * - * Nothing is invented for a field Codex sends as null: a stand-in path is a - * claim about a target, and the label helper turns any path into a file link. - */ -type CommandActionClass = { - name: string - /** Action field to the `input` key it lifts to. A scan root and a listed - * directory lift to `directory`, never `path`: the label helper reads `path` - * as a file target, which mobile turns into a tappable open-file link. */ - keys: Readonly> -} - -const COMMAND_ACTION_CLASSES = new Map([ - ['read', { name: 'read', keys: { path: 'path' } }], - ['search', { name: 'search', keys: { query: 'query', path: 'directory' } }], - ['listFiles', { name: 'list', keys: { path: 'directory' } }] -]) - -/** The one class every classified `commandActions` entry agrees on, with the - * fields they all agree on; null leaves the row exactly as a Codex that sends no - * classification renders it. `cat a.txt && ls src` classifies as two different - * things, and naming that row after either would drop the other, so it stays a - * `shell` row that shows the whole command. */ -function commandActionFacts( - item: CodexThreadItem -): { name: string; fields: Record } | null { - const actions = item.commandActions - if (!Array.isArray(actions)) { - return null - } - let matched: { class: CommandActionClass; fields: Record } | null = null - for (const action of actions) { - const record = readRecord(action) - const type = readString(record, 'type') - const classified = type === null ? undefined : COMMAND_ACTION_CLASSES.get(type) - if (classified === undefined) { - continue - } - if (matched === null) { - const fields: Record = {} - for (const [source, lifted] of Object.entries(classified.keys)) { - const value = readString(record, source) - if (value !== null) { - fields[lifted] = value - } - } - matched = { class: classified, fields } - continue - } - if (matched.class.name !== classified.name) { - return null - } - // The same class twice keeps the class, but only a target both entries name. - for (const [source, lifted] of Object.entries(matched.class.keys)) { - const kept = matched.fields[lifted] - if (kept !== undefined && readString(record, source) !== kept) { - delete matched.fields[lifted] - } - } - } - return matched === null ? null : { name: matched.class.name, fields: matched.fields } -} - function commandItem(item: CodexThreadItem): CodexJournalItem { const output = readFirstString(item, ['aggregatedOutput', 'aggregated_output']) const bounded = output === null ? null : boundInlineText(output, DEFAULT_JOURNAL_PAYLOAD_LIMITS) @@ -296,6 +135,85 @@ function fileChangeItem(item: CodexThreadItem): CodexJournalItem { } } +/** The tool name reaches the row verbatim — downstream dispatch (diff renderer, + * question parsers, input previews) matches raw identifiers, so any casing + * transform would silently miss them. `server/` qualifies it so two servers + * exposing the same tool stay distinguishable and neither shadows a built-in. */ +function mcpToolCallName(item: CodexThreadItem): string { + const tool = readString(item, 'tool') + const server = readString(item, 'server') + return tool === null ? 'mcp' : server === null ? tool : `${server}/${tool}` +} + +/** Row-label derivation only reads top-level keys, so the call's own arguments + * have to be the input itself. `arguments` is arbitrary JSON upstream: a + * non-object stays addressable under a key rather than being dropped, while a + * no-argument call — `{}` on the wire, the shape every argument-less MCP tool + * sends — becomes null so the row reads as a bare `server/tool` instead of a + * literal `{}`. */ +function mcpToolArguments(value: unknown): unknown { + if (typeof value !== 'object' || value === null) { + return value === null || value === undefined ? null : { arguments: value } + } + return Array.isArray(value) ? { arguments: value } : Object.keys(value).length > 0 ? value : null +} + +function mcpToolCallItem(item: CodexThreadItem): CodexJournalItem { + const failure = readString(readRecord(item.error), 'message') + const text = failure ?? readTextContent(readRecord(item.result), 'content') + const bounded = text === null ? null : boundInlineText(text, DEFAULT_JOURNAL_PAYLOAD_LIMITS) + return { + body: { + kind: 'tool-call', + name: mcpToolCallName(item), + input: boundToolInput(mcpToolArguments(item.arguments), DEFAULT_JOURNAL_PAYLOAD_LIMITS), + state: failure === null ? commandState(item) : 'failed', + ...(bounded === null ? {} : { output: bounded.bounded }) + }, + handled: true + } +} + +/** A row label is read off top-level keys only, so the action's own labelable + * fields are hoisted beside the query while `action` stays whole for the + * expanded detail. The action `type` lands on `description`, the lowest-ranked + * label key, so it names only an action that carries nothing better. */ +function webSearchInput(item: CodexThreadItem): Record | null { + const action = readRecord(item.action) + const fields: [string, unknown][] = [ + ['url', readString(action, 'url')], + ['pattern', readString(action, 'pattern')], + ['description', readString(action, 'type')], + ['action', item.action ?? null] + ] + const query = readString(item, 'query') ?? readString(action, 'query') + const present = fields.filter(([, value]) => value !== null) + // A blank `query` is the run header's "this call has no brief argument" + // signal; drop the key and the header stands the row's raw JSON in for one. + return query === null && present.length === 0 + ? null + : { query: query ?? '', ...Object.fromEntries(present) } +} + +/** `webSearch` carries no status: Codex starts it with an empty query and a null + * action, then sends the action, so `action` is the completion signal — a + * completed item's own `query` is routinely still empty. The hits arrive on + * `results` and are the call's output. */ +function webSearchItem(item: CodexThreadItem): CodexJournalItem { + const hits = Array.isArray(item.results) && item.results.length > 0 ? item.results : null + const bounded = hits && boundInlineText(JSON.stringify(hits), DEFAULT_JOURNAL_PAYLOAD_LIMITS) + return { + body: { + kind: 'tool-call', + name: 'web_search', + input: boundToolInput(webSearchInput(item), DEFAULT_JOURNAL_PAYLOAD_LIMITS), + state: item.action === null || item.action === undefined ? 'running' : 'completed', + ...(bounded === null ? {} : { output: bounded.bounded }) + }, + handled: true + } +} + /** * Journal body for a Codex item, or null for one with nothing to render. * @@ -319,6 +237,12 @@ export function codexJournalItem(item: CodexThreadItem): CodexJournalItem { if (item.type === 'fileChange') { return fileChangeItem(item) } + if (item.type === 'mcpToolCall') { + return mcpToolCallItem(item) + } + if (item.type === 'webSearch') { + return webSearchItem(item) + } if (item.type === 'reasoning' || item.type === 'plan') { const text = readTextContent(item, 'text') ?? diff --git a/src/main/codex/codex-thread-item-identity.ts b/src/main/codex/codex-thread-item-identity.ts new file mode 100644 index 00000000000..0488e5c00e9 --- /dev/null +++ b/src/main/codex/codex-thread-item-identity.ts @@ -0,0 +1,65 @@ +import type { AgentJournalItemIdentity } from '../../shared/agent-session-journal-types' +import type { CodexTurnOrdinals } from './codex-turn-ordinals' + +// Codex thread items → durable journal identities. +// +// THE ORDINAL RULE, and why it is not "index within the turn". Codex renumbers +// item ids positionally on resume (`item-1`…`item-N` across the whole thread), +// and a resumed turn does NOT contain every item the live turn emitted — +// reasoning and command execution are dropped from persisted history. Numbering +// by live position would therefore shift every message after the first tool +// call and hand the user a duplicate of the assistant's answer after a resume. +// +// So the ordinal counts MESSAGE items only, and the same projection is applied +// to the live stream and to a resumed turn's item list. Any other item type — +// including ones this build does not model — is skipped identically on both +// sides, which is what makes the key survive a Codex release that adds one. + +/** Only these carry a durable `(threadId, turnId, ordinal)` identity. */ +const CODEX_MESSAGE_ITEM_TYPES = new Set(['userMessage', 'agentMessage']) + +export type CodexThreadItem = { + type: string + id: string + [key: string]: unknown +} + +export function isCodexMessageItemType(type: string): boolean { + return CODEX_MESSAGE_ITEM_TYPES.has(type) +} + +export function readCodexThreadItem(value: unknown): CodexThreadItem | null { + if (typeof value !== 'object' || value === null) { + return null + } + const record = value as Record + return typeof record.type === 'string' && typeof record.id === 'string' + ? (record as CodexThreadItem) + : null +} + +/** + * Durable identity for a Codex item, or null for one that has none. + * + * Non-message items fall back to the `orca` namespace keyed by the Codex item + * id. That id is unstable across resume, so those rows are live-session detail + * that a recovered journal simply will not contain — which is correct: Codex + * itself does not persist them either. + */ +export function codexItemIdentity(input: { + threadId: string + turnId: string | null + item: CodexThreadItem + ordinals: CodexTurnOrdinals +}): AgentJournalItemIdentity { + const { item, turnId } = input + if (turnId && isCodexMessageItemType(item.type)) { + return { + provider: 'codex', + threadId: input.threadId, + turnId, + ordinal: input.ordinals.ordinalFor(input.threadId, turnId, item.id) + } + } + return { provider: 'orca', clientMessageId: `codex-item:${input.threadId}:${item.id}` } +} diff --git a/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts b/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts index 22bd645d8a6..9860aaa81d8 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts @@ -112,4 +112,52 @@ describe('provider frame classification catalog', () => { // An item type nobody has dispositioned still falls through visibly. expect(classifyProviderFrame('codex', 'item:futureThing', {})).toBe('timeline-substantive') }) + + it('chromes the one unmodelled codex item type that carries no content', () => { + expect(classifyProviderFrame('codex', 'item:sleep', { id: 's', durationMs: 20_000 })).toBe( + 'status-chrome' + ) + // Payload inspection still outranks the item catalog, so chroming a type + // cannot swallow one that reports a failure. + expect(classifyProviderFrame('codex', 'item:sleep', { id: 's', status: 'failed' })).toBe( + 'error-surface' + ) + }) + + it('keeps subagent items visible — the only evidence a spawned agent is working', () => { + expect( + classifyProviderFrame('codex', 'item:subAgentActivity', { + id: 'a-1', + kind: 'started', + agentThreadId: 'thread-child', + agentPath: '/root/list_directory' + }) + ).toBe('timeline-substantive') + expect( + classifyProviderFrame('codex', 'item:collabAgentToolCall', { + id: 'c-1', + tool: 'spawn', + status: 'inProgress', + senderThreadId: 'thread-root', + receiverThreadIds: ['thread-child'], + agentsStates: {} + }) + ).toBe('timeline-substantive') + }) + + it('leaves content-bearing codex item types on the visible fallback', () => { + // Each carries text or a path a user would want: review output, the image + // the agent looked at or generated, injected hook prompt text. + for (const type of [ + 'imageView', + 'imageGeneration', + 'enteredReviewMode', + 'exitedReviewMode', + 'hookPrompt' + ]) { + expect(classifyProviderFrame('codex', `item:${type}`, { id: 'i' }), type).toBe( + 'timeline-substantive' + ) + } + }) }) diff --git a/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts b/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts index 474b1385a4f..f05f4cd4c6c 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts @@ -197,7 +197,12 @@ function hasProviderError(payload: unknown): boolean { const CODEX_ITEM_CLASSIFICATIONS: Record = { // The `thread/compacted` notification is already chrome; its item form is the // same event and must not read as a mysterious opcode row. - contextCompaction: 'status-chrome' + contextCompaction: 'status-chrome', + // `{id, durationMs}` and nothing else — Codex's own transcript renders it as + // nothing at all. Every other item type this build does not model carries text + // a user would want (review output, an image path, hook prompt text, subagent + // progress), so those keep their visible fallback row. + sleep: 'status-chrome' } function notificationKind(kind: string): string {