From 6eda9cdb27ce7354f2261f860a8a5e9faf9255ff Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 13 Jul 2026 05:32:31 -0700 Subject: [PATCH 01/23] fix(ssh): bound healthy relay watcher processes (#8556) --- config/reliability-gates.jsonc | 16 +-- .../relay-filesystem-watch-registry.test.ts | 101 +++++++++++++++++- src/relay/relay-watcher-process-pool.ts | 3 + 3 files changed, 112 insertions(+), 8 deletions(-) diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index fc02e5fb135..6a9ba8db139 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -3924,14 +3924,14 @@ "remote-runtime", "ssh" ], - "coverageNotes": "Deterministic tests cover a four-child healthy runtime pool plus at most four bounded quarantine children, one quarantine attempt per watch lifetime, queue-aware cancellation, bounded initial and crash-resubscribe crawls, physical desktop-install abort, stale-generation rejection, per-child stat bounds, final-overflow RPC delivery, and renderer eviction before terminal callbacks can retry. A built-entry macOS harness drives the real supervisor, kills its child with SIGSEGV, then requires automatic resubscription and a later event. SSH now bundles the same child boundary beside relay.js, requires both artifacts for install completeness, and preserves registration-owned same-root cancellation. A built-relay harness kills that child and requires the original relay PID and existing PTY to survive before overflow and precise events resume.", + "coverageNotes": "Deterministic tests cover a four-child healthy runtime pool, a one-child healthy SSH relay pool, and at most four bounded quarantine children, with one quarantine attempt per watch lifetime, queue-aware cancellation, bounded initial and crash-resubscribe crawls, physical desktop-install abort, stale-generation rejection, per-child stat bounds, final-overflow RPC delivery, and renderer eviction before terminal callbacks can retry. The relay policy test maps a standard repository's base, Git common directory, and worktree roots to one healthy supervisor, then proves a shared-child failure recovers those roots in separate quarantine supervisors with overflow and resumed events. A built-entry macOS harness drives the real supervisor, kills its child with SIGSEGV, then requires automatic resubscription and a later event. SSH bundles the same child boundary beside relay.js, requires both artifacts for install completeness, and preserves registration-owned same-root cancellation. A built-relay harness kills that child and requires the original relay PID and existing PTY to survive before overflow and precise events resume.", "motivatingLinks": [ "https://github.com/stablyai/orca/issues/5308", "https://github.com/stablyai/orca/issues/7547", "https://github.com/stablyai/orca/issues/8212" ], - "invariant": "Local runtime and SSH relay roots use at most four shared watcher children while healthy and at most four additional quarantine children after shard failure. Cancelling queued setup removes only that request; cancelling an active crawl restarts one shard and restores its healthy records. Setup deadlines start only when the child owns the crawl; the runtime RPC transport owns the normal 15-second initial budget, while a 60-second recovery deadline quarantines a root that would otherwise pin its shard. A crash-fused shared shard moves affected roots into the bounded quarantine pool, and a second fused quarantine ends those watch lifetimes instead of spawning replacements indefinitely. Every interrupted setup emits overflow after recovery, event delivery stays bounded, and the SSH relay, PTYs, Git, and other filesystem operations never load the watcher native addon or share its fault boundary.", - "oracle": "Fill the four-child healthy pool with multiple roots, inject IPC disconnect-before-exit, queued and active setup cancellation, initial and crash-resubscribe timeouts, and shard-level terminal failure, then assert healthy roots are restored, affected roots spread across no more than four quarantine supervisors, a second quarantine fuse creates no replacement, stale generations stop delivering, and existing same-root clients receive overflow plus later events without error/end. Abort a pending desktop install and require worktree close and app shutdown to settle before the native crawl resolves. Verify relay build hashes and remote install probes require both relay.js and relay-watcher.js. Launch the built relay with a real PTY and Parcel watch, kill only the watcher child, and require the same relay PID and PTY to answer before a replacement child delivers overflow and a post-crash event.", + "invariant": "Local runtime roots use at most four shared watcher children while healthy, while each SSH relay process uses at most one; either pool uses at most four additional quarantine children after shard failure. Cancelling queued setup removes only that request; cancelling an active crawl restarts one shard and restores its healthy records. Setup deadlines start only when the child owns the crawl; the runtime RPC transport owns the normal 15-second initial budget, while a 60-second recovery deadline quarantines a root that would otherwise pin its shard. A crash-fused shared shard moves affected roots into the bounded quarantine pool, and a second fused quarantine ends those watch lifetimes instead of spawning replacements indefinitely. Every interrupted setup emits overflow after recovery, event delivery stays bounded, and each SSH target's relay, PTYs, Git, and other filesystem operations remain isolated from other targets and never load the watcher native addon or share its fault boundary.", + "oracle": "Fill the four-child healthy runtime pool with multiple roots, and map a standard SSH repository's base, Git common directory, and worktree roots to one relay supervisor. Inject IPC disconnect-before-exit, queued and active setup cancellation, initial and crash-resubscribe timeouts, and shard-level terminal failure, then assert healthy roots are restored, affected roots spread across no more than four quarantine supervisors, a second quarantine fuse creates no replacement, stale generations stop delivering, and existing same-root clients receive overflow plus later events without error/end. Abort a pending desktop install and require worktree close and app shutdown to settle before the native crawl resolves. Verify relay build hashes and remote install probes require both relay.js and relay-watcher.js. Launch the built relay with a real PTY and Parcel watch, kill only the watcher child, and require the same relay PID and PTY to answer before a replacement child delivers overflow and a post-crash event.", "commands": [ "pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/parcel-watcher-process.test.ts src/main/ipc/runtime-watcher-process-pool.test.ts src/main/ipc/parcel-watcher-process-entry.test.ts src/main/ipc/parcel-watcher-event-delivery.test.ts src/main/ipc/filesystem-watcher-local-unsubscribe.test.ts src/main/ipc/filesystem-watcher-wsl.test.ts src/main/runtime/file-watcher-host.test.ts src/main/runtime/rpc/methods/files.test.ts src/main/providers/ssh-filesystem-provider.test.ts src/renderer/src/runtime/runtime-file-client.test.ts", "pnpm run build:electron-vite", @@ -4043,6 +4043,8 @@ { "file": "src/relay/relay-filesystem-watch-registry.test.ts", "assertions": [ + "each SSH host keeps its own healthy watcher supervisor while a standard repository's three distinct roots share that one child", + "shared-child failure moves those roots into separate quarantine supervisors with overflow and resumed events", "replacement emits overflow before later ordered events", "terminal shard failure preserves same-root clients through recovery", "pending setup aborts only after the last same-root client leaves", @@ -4155,11 +4157,11 @@ }, "redGreenEvidence": { "status": "partial", - "evidence": "Red tests reproduced the reviewed failures: IPC disconnect-before-exit orphaned existing roots, queue-admission deadlines cascaded shard replacement, a fused shard created one child per root, destructive desktop cleanup waited indefinitely for native setup, and synchronous renderer retry joined a terminal stream. Earlier red coverage also reproduced missing-entry fail-open, terminal recovery loss, stale same-root lease deletion, and unbounded per-child stat lanes. The bounded quarantine and cancellation rewrite passes those contracts, and the real-supervisor built-entry harness survives SIGSEGV with automatic replacement. CI/soak history is still absent." + "evidence": "Red tests reproduced the reviewed failures: IPC disconnect-before-exit orphaned existing roots, queue-admission deadlines cascaded shard replacement, a fused shard created one child per root, a standard SSH repository created three healthy watcher supervisors, destructive desktop cleanup waited indefinitely for native setup, and synchronous renderer retry joined a terminal stream. Earlier red coverage also reproduced missing-entry fail-open, terminal recovery loss, stale same-root lease deletion, and unbounded per-child stat lanes. The bounded quarantine and cancellation rewrite passes those contracts, the relay-specific healthy width test reduces the three-root case to one supervisor, and the real-supervisor built-entry harness survives SIGSEGV with automatic replacement. CI/soak history is still absent." }, "performanceBudget": { "required": true, - "evidence": "Healthy operation is capped at four shared child processes instead of one child per root; based on the prior 51.9 MiB one-child measurement this bounds the unremeasured eight-root healthy ceiling near 207.6 MiB rather than the rejected 388.7 MiB per-root design. Degraded operation adds at most four quarantine children, distributes affected roots by least load, disposes empty slots, and permits no replacement after a quarantine shard itself fuses. Each child caps directory stats at eight across its assigned roots (at most 32 healthy or 64 total logical stat operations), each subscription retains at most one active plus one bounded pending batch, and no polling or serve-process stat fanout is added." + "evidence": "Local runtime operation remains capped at four shared child processes, while each SSH relay is capped at one healthy child instead of spreading a standard repository's three roots across three children. Based on the measured 51.9 MiB per child, normal SSH watcher overhead is bounded near 51.9 MiB rather than 155.7 MiB before relay and PTY overhead. Degraded operation adds at most four quarantine children, distributes affected roots by least load, disposes empty slots, and permits no replacement after a quarantine shard itself fuses. Each child caps directory stats at eight across its assigned roots (at most 32 healthy or 64 total logical stat operations for local runtime; 8 healthy or 40 total per SSH relay), each subscription retains at most one active plus one bounded pending batch, and no polling or serve-process stat fanout is added." }, "promotionCriteria": [ "Run the built-entry SIGSEGV harness in soak for at least 100 consecutive passes or 14 days on macOS.", @@ -4168,11 +4170,11 @@ ], "knownGaps": [ "The fault harness bundles the real source supervisor and drives the built child, but it does not launch a packaged orca serve pairing session or exercise app.asar path resolution.", - "The four-child healthy and four-child quarantine memory ceilings are inferred from the earlier one-child measurement; direct degraded-state RSS plus sustained six-writer event-loop-delay and CPU soak evidence is still absent.", + "The one-child SSH healthy and four-child quarantine memory ceilings are inferred from the earlier one-child measurement; direct degraded-state RSS plus sustained six-writer event-loop-delay and CPU soak evidence is still absent.", "Linux native watcher fault containment is source-equivalent but not live fault-injected in this evidence run.", "Windows runtime watches bypass this path; SSH registration ownership is deterministic-contract tested but not live-relay fault-injected." ], - "demotionRule": "Demote or quarantine if the fault harness flakes without a product or harness bug, if healthy operation exceeds four runtime watcher children, if total healthy plus quarantine operation exceeds eight, if quarantine children outlive their roots or repeat after fusing, if event delivery becomes unbounded, or if metadata/stat work returns to the serve process." + "demotionRule": "Demote or quarantine if the fault harness flakes without a product or harness bug, if healthy operation exceeds four local runtime watcher children or one child in any SSH relay, if total healthy plus quarantine operation exceeds eight locally or five in an SSH relay, if quarantine children outlive their roots or repeat after fusing, if event delivery becomes unbounded, or if metadata/stat work returns to the serve process." } ] } diff --git a/src/relay/relay-filesystem-watch-registry.test.ts b/src/relay/relay-filesystem-watch-registry.test.ts index 5f1e46eecdd..7c505ae652d 100644 --- a/src/relay/relay-filesystem-watch-registry.test.ts +++ b/src/relay/relay-filesystem-watch-registry.test.ts @@ -1,7 +1,8 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { join } from 'node:path' import { tmpdir } from 'node:os' import { WatcherProcessFailure } from '../main/ipc/parcel-watcher-process-failure' +import { WatcherProcessSupervisor } from '../main/ipc/parcel-watcher-process-supervisor' import type { WatcherProcessCallback, WatcherProcessHooks, @@ -17,6 +18,29 @@ type InstalledWatch = { unsubscribe: ReturnType Promise>> } +type InstalledSupervisorWatch = InstalledWatch & { dir: string } + +function stubRelayWatcherSupervisors() { + const installed = new Map() + vi.spyOn(WatcherProcessSupervisor.prototype, 'subscribe').mockImplementation(function ( + this: WatcherProcessSupervisor, + dir, + callback, + _options, + hooks = {} + ) { + const watches = installed.get(this) ?? [] + const unsubscribe = vi.fn(async () => undefined) + watches.push({ dir, callback, hooks, unsubscribe }) + installed.set(this, watches) + return Promise.resolve({ unsubscribe }) + }) + const dispose = vi + .spyOn(WatcherProcessSupervisor.prototype, 'dispose') + .mockImplementation(() => undefined) + return { installed, dispose } +} + class FakeWatcherPool { readonly installed: InstalledWatch[] = [] readonly dispose = vi.fn() @@ -148,6 +172,81 @@ describe('RelayFilesystemWatchRegistry', () => { }) describe('createRelayWatcherProcessPool', () => { + afterEach(() => vi.restoreAllMocks()) + + it('uses one healthy watcher process per SSH host for each standard repo', async () => { + const { installed } = stubRelayWatcherSupervisors() + const firstHostPool = createRelayWatcherProcessPool() + const secondHostPool = createRelayWatcherProcessPool() + const repoBase = join(tmpdir(), 'ssh-repo') + const roots = [repoBase, join(repoBase, '.git'), join(repoBase, 'worktree')] + try { + for (const root of roots) { + await firstHostPool.subscribe(root, vi.fn(), {}, {}) + await secondHostPool.subscribe(root, vi.fn(), {}, {}) + } + + expect( + Array.from(installed.values()).map((watches) => watches.map(({ dir }) => dir)) + ).toEqual([roots, roots]) + } finally { + firstHostPool.dispose() + secondHostPool.dispose() + } + }) + + it('quarantines and recovers each standard-repo root after the shared child fails', async () => { + const { installed, dispose } = stubRelayWatcherSupervisors() + const pool = createRelayWatcherProcessPool() + const dispatcher = createDispatcher() + const registry = new RelayFilesystemWatchRegistry( + dispatcher as unknown as RelayDispatcher, + pool + ) + const repoBase = join(tmpdir(), 'ssh-repo-recovery') + const roots = [repoBase, join(repoBase, '.git'), join(repoBase, 'worktree')] + try { + for (const root of roots) { + await registry.watch(root, context(1)) + } + const healthyWatches = Array.from(installed.values())[0] + const failure = new WatcherProcessFailure( + 'file watcher process crashed repeatedly', + 'supervisor', + 'supervisor_crash_fuse' + ) + + for (const watch of healthyWatches) { + watch.hooks.onTerminalError?.(failure) + } + await Promise.resolve() + + const recoveredWatches = Array.from(installed.values()).slice(1) + expect(dispose).toHaveBeenCalledTimes(1) + expect(recoveredWatches.map((watches) => watches.map(({ dir }) => dir))).toEqual( + roots.map((root) => [root]) + ) + expect(dispatcher.notify.mock.calls.slice(0, roots.length)).toEqual( + roots.map((root) => ['fs.changed', { events: [{ kind: 'overflow', absolutePath: root }] }]) + ) + + for (const watches of recoveredWatches) { + const [{ callback, dir }] = watches + callback(null, [{ type: 'update', path: join(dir, 'recovered.txt') }]) + } + expect(dispatcher.notify.mock.calls.slice(roots.length)).toEqual( + roots.map((root) => [ + 'fs.changed', + { + events: [{ kind: 'update', absolutePath: join(root, 'recovered.txt') }] + } + ]) + ) + } finally { + registry.dispose() + } + }) + it('fails closed instead of loading the native watcher in the relay process', async () => { const previousVitest = process.env.VITEST process.env.VITEST = 'true' diff --git a/src/relay/relay-watcher-process-pool.ts b/src/relay/relay-watcher-process-pool.ts index af6cb8285e2..e0093b9a166 100644 --- a/src/relay/relay-watcher-process-pool.ts +++ b/src/relay/relay-watcher-process-pool.ts @@ -15,6 +15,9 @@ export function createRelayWatcherProcessPool( entryPath = getRelayWatcherProcessEntryPath() ): RelayWatcherProcessPool { return new RuntimeWatcherProcessPool({ + // Why: one SSH workspace normally installs three roots, and each remote + // watcher child carries substantial Node/native-addon process overhead. + maxSharedSupervisors: 1, createSupervisor: () => new WatcherProcessSupervisor({ entryPath, From 17f032dbd554a0ac237dbaa67c48dbabff06d8b2 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 13 Jul 2026 05:39:52 -0700 Subject: [PATCH 02/23] Fix WSL orchestration CLI preambles (#8561) --- src/main/ipc/pty.test.ts | 40 ++++++++++--- src/main/ipc/pty.ts | 16 ++++- src/main/pty/wsl-orca-env.test.ts | 2 + src/main/pty/wsl-orca-env.ts | 1 + src/main/runtime/orca-runtime.ts | 43 +++++++++++++- .../runtime/orchestration/cli-command.test.ts | 59 +++++++++++++++++++ src/main/runtime/orchestration/cli-command.ts | 25 ++++++++ .../runtime/orchestration/coordinator.test.ts | 7 +++ src/main/runtime/orchestration/coordinator.ts | 6 ++ .../runtime/orchestration/preamble.test.ts | 11 +++- src/main/runtime/orchestration/preamble.ts | 7 ++- .../runtime/rpc/methods/orchestration.test.ts | 16 +++++ src/main/runtime/rpc/methods/orchestration.ts | 14 +++-- 13 files changed, 228 insertions(+), 19 deletions(-) create mode 100644 src/main/runtime/orchestration/cli-command.test.ts create mode 100644 src/main/runtime/orchestration/cli-command.ts diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 35d386eb473..4838711803d 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -1702,6 +1702,7 @@ describe('registerPtyHandlers', () => { // (the GNOME screen reader) inside Orca-managed terminals (#7904). expect(entries.indexOf(shimDir)).toBeGreaterThanOrEqual(0) expect(entries.indexOf(shimDir)).toBeLessThan(entries.indexOf('/usr/bin')) + expect(env.ORCA_CLI_COMMAND).toBeUndefined() } finally { Object.defineProperty(process, 'platform', { configurable: true, @@ -1801,10 +1802,13 @@ describe('registerPtyHandlers', () => { // Why: runtime-created spawns (e.g. the mobile-create materialize path) // must thread the same {tabId, leafId} so the catch-path rescue can find // and keep their live PTY (#7587). - expect(runtime.registerPty).toHaveBeenCalledWith(expect.any(String), 'wt-runtime', null, { - tabId: 'tab-1', - leafId - }) + expect(runtime.registerPty).toHaveBeenCalledWith( + expect.any(String), + 'wt-runtime', + null, + { tabId: 'tab-1', leafId }, + false + ) }) it('uses the owning project WSL runtime for runtime-created daemon PTYs', async () => { @@ -1859,6 +1863,13 @@ describe('registerPtyHandlers', () => { expect(spawnOptions.shellOverride).toBe('wsl.exe') expect(spawnOptions.terminalWindowsWslDistro).toBe('Ubuntu') expect(spawnOptions.terminalWindowsPowerShellImplementation).toBe('auto') + expect(runtime.registerPty).toHaveBeenCalledWith( + expect.any(String), + 'repo-1::C:\\repo', + null, + undefined, + true + ) }) }) @@ -4558,10 +4569,13 @@ describe('registerPtyHandlers', () => { // Why: this is the load-bearing wiring for #7587 — the runtime can only back a // stalled mobile create from a live spawn if the spawn threads {tabId, leafId}. - expect(runtime.registerPty).toHaveBeenCalledWith(expect.any(String), 'wt-1', null, { - tabId: 'tab-1', - leafId - }) + expect(runtime.registerPty).toHaveBeenCalledWith( + expect.any(String), + 'wt-1', + null, + { tabId: 'tab-1', leafId }, + false + ) }) it('omits the pane identity from registerPty when the leafId is not a terminal leaf (#7587)', async () => { @@ -4590,7 +4604,13 @@ describe('registerPtyHandlers', () => { // Why: legacy numeric pane ids (`pane:N`) are not terminal leaf ids, so the // spawn seam must not fabricate a binding for them (registerPty would ignore // it anyway); this pins that the seam passes a clean `undefined`. - expect(runtime.registerPty).toHaveBeenCalledWith(expect.any(String), 'wt-1', null, undefined) + expect(runtime.registerPty).toHaveBeenCalledWith( + expect.any(String), + 'wt-1', + null, + undefined, + false + ) }) it('refreshes native Agent Teams env when captured teammate mode lives in launch args', async () => { @@ -6069,10 +6089,12 @@ describe('registerPtyHandlers', () => { expect(spawnCall[0]).toBe('wsl.exe') expect(env.ORCA_TERMINAL_HANDLE).toBe('term_wsl') expect(env.ORCA_USER_DATA_PATH).toBe('/tmp/orca-user-data') + expect(env.ORCA_CLI_COMMAND).toBe('orca-ide') expect(env.WSLENV?.split(':')).toEqual( expect.arrayContaining([ 'ORCA_TERMINAL_HANDLE/u', 'ORCA_USER_DATA_PATH/p', + 'ORCA_CLI_COMMAND/u', 'ORCA_AGENT_HOOK_PORT/u', 'ORCA_AGENT_HOOK_TOKEN/u', 'ORCA_OMP_SOURCE_AGENT_DIR/p', diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 305ebabfc14..0dd30b24b1e 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -980,8 +980,14 @@ export function buildPtyHostEnv( // Why: WSL shells need the managed userData root for shell-ready wrappers; dev-mode terminals need the same export so `orca` targets the live dev instance. if (opts.isWsl) { baseEnv.ORCA_USER_DATA_PATH = opts.userDataPath - } else if (!opts.isPackaged) { - baseEnv.ORCA_USER_DATA_PATH ??= opts.userDataPath + // Why: managed WSL registration deliberately uses `orca-ide`; exposing + // that literal keeps agent guidance scoped to WSL without a bare-orca shim. + baseEnv.ORCA_CLI_COMMAND = opts.isPackaged ? 'orca-ide' : 'orca-dev' + } else { + if (!opts.isPackaged) { + baseEnv.ORCA_USER_DATA_PATH ??= opts.userDataPath + } + delete baseEnv.ORCA_CLI_COMMAND } // Why: dev mode needs the launcher PATH override so `orca` resolves to the dev build instead of the production binary at /usr/local/bin/orca. if (!opts.isPackaged) { @@ -3234,6 +3240,9 @@ export function registerPtyHandlers( args.tabId.length <= 512 && metadataLeafId !== null ? { tabId: args.tabId, leafId: metadataLeafId } + : undefined, + !args.connectionId + ? shouldSkipCodexHomeEnvForWindowsShell(daemonShellOverride, cwd) : undefined ) } @@ -4282,6 +4291,9 @@ export function registerPtyHandlers( args.tabId.length <= 512 && metadataLeafId !== null ? { tabId: args.tabId, leafId: metadataLeafId } + : undefined, + !args.connectionId + ? shouldSkipCodexHomeEnvForWindowsShell(effectiveShellOverride, cwd) : undefined ) } diff --git a/src/main/pty/wsl-orca-env.test.ts b/src/main/pty/wsl-orca-env.test.ts index 550295460ad..fe0d94ab9cb 100644 --- a/src/main/pty/wsl-orca-env.test.ts +++ b/src/main/pty/wsl-orca-env.test.ts @@ -24,6 +24,7 @@ describe('addOrcaWslInteropEnv', () => { const env: Record = { ORCA_TERMINAL_HANDLE: 'term_wsl', ORCA_USER_DATA_PATH: 'C:\\Users\\jin\\AppData\\Roaming\\Orca', + ORCA_CLI_COMMAND: 'orca-ide', ORCA_OMP_STATUS_EXTENSION: 'C:\\Users\\jin\\.omp\\agent\\extensions\\orca-agent-status.ts', ORCA_PANE_KEY: 'tab-1:leaf-1', ORCA_TAB_ID: 'tab-1', @@ -38,6 +39,7 @@ describe('addOrcaWslInteropEnv', () => { expect(env.WSLENV).toContain('ORCA_TERMINAL_HANDLE/u') expect(env.WSLENV).toContain('ORCA_USER_DATA_PATH/p') + expect(env.WSLENV).toContain('ORCA_CLI_COMMAND/u') expect(env.WSLENV).toContain('ORCA_OMP_STATUS_EXTENSION/p') expect(env.WSLENV).toContain('ORCA_PANE_KEY/u') expect(env.WSLENV).toContain('ORCA_TAB_ID/u') diff --git a/src/main/pty/wsl-orca-env.ts b/src/main/pty/wsl-orca-env.ts index 161621e8c0f..3f136e0a284 100644 --- a/src/main/pty/wsl-orca-env.ts +++ b/src/main/pty/wsl-orca-env.ts @@ -24,6 +24,7 @@ export function addOrcaWslInteropEnv(env: Record): void { const passthroughEntries = [ 'ORCA_TERMINAL_HANDLE/u', 'ORCA_USER_DATA_PATH/p', + 'ORCA_CLI_COMMAND/u', 'ORCA_PANE_KEY/u', 'ORCA_TAB_ID/u', 'ORCA_WORKTREE_ID/u', diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 293075176e9..09185299c82 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -469,7 +469,9 @@ import { resolveLocalProjectRuntimeForRepo, resolveLocalProjectRuntimesForRepos } from '../project-runtime-git-options' +import { resolveLocalProjectRuntimeForWorktreeId } from '../local-project-runtime-resolution' import type { ProjectExecutionRuntimeResolution } from '../../shared/project-execution-runtime' +import { resolveTerminalOrchestrationCliCommand } from './orchestration/cli-command' import { getLocalWorktreePathAccess, removeLocalWorktreePath, @@ -976,6 +978,9 @@ type RuntimePtyWorktreeRecord = { ptyId: string worktreeId: string connectionId: string | null + // Why: a Windows host can own both native and WSL panes; preamble command + // selection must follow the pane that executes it, not process.platform. + isWsl: boolean | null // Why: background CLI PTYs can outlive a failed renderer reveal. Preserve the // spawn-time tab/pane identity so later reveals can adopt under the env key. tabId: string | null @@ -5529,7 +5534,8 @@ export class OrcaRuntimeService { ptyId: string, worktreeId: string, connectionId: string | null = null, - binding?: { tabId: string; leafId: string } + binding?: { tabId: string; leafId: string }, + isWsl?: boolean ): void { // Why: record the renderer pane identity at spawn time so a stalled graph // sync can't hide that a live PTY already backs a pending mobile create. @@ -5540,6 +5546,7 @@ export class OrcaRuntimeService { this.recordPtyWorktree(ptyId, worktreeId, { connected: true, connectionId, + ...(isWsl !== undefined ? { isWsl } : {}), ...(binding && paneKey ? { tabId: binding.tabId, paneKey } : {}) }) // Why: the renderer's own PTY spawn is the reliable signal that the pending @@ -7234,6 +7241,27 @@ export class OrcaRuntimeService { return pty ? { worktreeId: pty.worktreeId, connectionId: pty.connectionId } : null } + getTerminalOrchestrationCliCommand(handle: string): 'orca' | 'orca-ide' { + let pty: RuntimePtyWorktreeRecord | null = null + try { + const ptyId = this.resolveLeafForHandle(handle)?.ptyId + pty = ptyId ? (this.ptysById.get(ptyId) ?? null) : null + } catch { + return 'orca' + } + if (!pty) { + return 'orca' + } + return resolveTerminalOrchestrationCliCommand({ + connectionId: pty.connectionId, + isWsl: pty.isWsl, + worktreeId: pty.worktreeId, + projectRuntime: this.store + ? resolveLocalProjectRuntimeForWorktreeId(this.requireStore(), pty.worktreeId) + : undefined + }) + } + hasRecentTerminalOutputPath(handle: string, pathText: string, absolutePath: string): boolean { const ptyId = this.resolveLeafForHandle(handle)?.ptyId const recentOutput = ptyId ? this.recentPtyOutputById.get(ptyId) : null @@ -20057,7 +20085,14 @@ export class OrcaRuntimeService { state: Partial< Pick< RuntimePtyWorktreeRecord, - 'connected' | 'lastOutputAt' | 'preview' | 'tabId' | 'paneKey' | 'title' | 'connectionId' + | 'connected' + | 'lastOutputAt' + | 'preview' + | 'tabId' + | 'paneKey' + | 'title' + | 'connectionId' + | 'isWsl' > > = {} ): RuntimePtyWorktreeRecord { @@ -20068,6 +20103,7 @@ export class OrcaRuntimeService { ptyId, worktreeId, connectionId: state.connectionId ?? parseAppSshPtyId(ptyId)?.connectionId ?? null, + isWsl: state.isWsl ?? null, tabId: state.tabId ?? null, paneKey: state.paneKey ?? null, launchConfig: null, @@ -20109,6 +20145,9 @@ export class OrcaRuntimeService { if (state.connectionId !== undefined) { pty.connectionId = state.connectionId } + if (state.isWsl !== undefined) { + pty.isWsl = state.isWsl + } if (state.tabId !== undefined) { pty.tabId = state.tabId } diff --git a/src/main/runtime/orchestration/cli-command.test.ts b/src/main/runtime/orchestration/cli-command.test.ts new file mode 100644 index 00000000000..1d07d335c8a --- /dev/null +++ b/src/main/runtime/orchestration/cli-command.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from 'vitest' +import { resolveTerminalOrchestrationCliCommand } from './cli-command' + +describe('resolveTerminalOrchestrationCliCommand', () => { + it('uses orca-ide for a pane recorded as WSL', () => { + expect( + resolveTerminalOrchestrationCliCommand({ + connectionId: null, + isWsl: true, + worktreeId: 'repo::C:\\repo' + }) + ).toBe('orca-ide') + }) + + it('uses project runtime and WSL paths when restored pane metadata is unavailable', () => { + expect( + resolveTerminalOrchestrationCliCommand({ + connectionId: null, + isWsl: null, + worktreeId: 'repo::C:\\repo', + projectRuntime: { + status: 'resolved', + runtime: { + kind: 'wsl', + hostPlatform: 'wsl', + projectId: 'project', + distro: 'Ubuntu', + reason: 'project-override', + cacheKey: 'project:wsl:Ubuntu' + } + } + }) + ).toBe('orca-ide') + expect( + resolveTerminalOrchestrationCliCommand({ + connectionId: null, + isWsl: null, + worktreeId: 'repo::\\\\wsl.localhost\\Ubuntu\\home\\alice\\repo' + }) + ).toBe('orca-ide') + }) + + it('preserves native and SSH bare-orca commands', () => { + expect( + resolveTerminalOrchestrationCliCommand({ + connectionId: null, + isWsl: false, + worktreeId: 'repo::/home/alice/repo' + }) + ).toBe('orca') + expect( + resolveTerminalOrchestrationCliCommand({ + connectionId: 'ssh-1', + isWsl: null, + worktreeId: 'repo::\\\\wsl.localhost\\Ubuntu\\home\\alice\\repo' + }) + ).toBe('orca') + }) +}) diff --git a/src/main/runtime/orchestration/cli-command.ts b/src/main/runtime/orchestration/cli-command.ts new file mode 100644 index 00000000000..c0f4883f1e4 --- /dev/null +++ b/src/main/runtime/orchestration/cli-command.ts @@ -0,0 +1,25 @@ +import type { ProjectExecutionRuntimeResolution } from '../../../shared/project-execution-runtime' +import { isWslUncPath } from '../../../shared/wsl-paths' +import { splitWorktreeIdForFilesystem } from '../../../shared/worktree-id' + +export type OrchestrationCliCommand = 'orca' | 'orca-ide' + +export function resolveTerminalOrchestrationCliCommand(args: { + connectionId: string | null + isWsl: boolean | null | undefined + worktreeId: string + projectRuntime?: ProjectExecutionRuntimeResolution +}): OrchestrationCliCommand { + if (args.connectionId) { + return 'orca' + } + if (args.isWsl !== null && args.isWsl !== undefined) { + return args.isWsl ? 'orca-ide' : 'orca' + } + if (args.projectRuntime?.status === 'resolved' && args.projectRuntime.runtime.kind === 'wsl') { + return 'orca-ide' + } + + const worktreePath = splitWorktreeIdForFilesystem(args.worktreeId)?.worktreePath + return worktreePath && isWslUncPath(worktreePath) ? 'orca-ide' : 'orca' +} diff --git a/src/main/runtime/orchestration/coordinator.test.ts b/src/main/runtime/orchestration/coordinator.test.ts index 8563c3a1e87..cb532b71e36 100644 --- a/src/main/runtime/orchestration/coordinator.test.ts +++ b/src/main/runtime/orchestration/coordinator.test.ts @@ -21,6 +21,7 @@ function createMockRuntime(): CoordinatorRuntime & { createdTerminalOptions: { title?: string }[] probeDriftCalls: string[] probeDriftResult: DriftResult + cliCommand: 'orca' | 'orca-ide' setProbeDrift(result: DriftResult): void throwProbeDrift: Error | null } { @@ -36,6 +37,7 @@ function createMockRuntime(): CoordinatorRuntime & { createdTerminalOptions: [] as { title?: string }[], probeDriftCalls: [] as string[], probeDriftResult: null as DriftResult, + cliCommand: 'orca' as 'orca' | 'orca-ide', throwProbeDrift: null as Error | null, setProbeDrift(result: DriftResult): void { mock.probeDriftResult = result @@ -63,6 +65,9 @@ function createMockRuntime(): CoordinatorRuntime & { throw mock.throwProbeDrift } return mock.probeDriftResult + }, + getTerminalOrchestrationCliCommand() { + return mock.cliCommand } } return mock @@ -121,6 +126,7 @@ describe('Coordinator', () => { it('dispatches a ready task to an available terminal', async () => { db = new OrchestrationDb(':memory:') const runtime = createMockRuntime() + runtime.cliCommand = 'orca-ide' runtime.terminals = [{ handle: 'term_a', worktreeId: 'wt1', connected: true, writable: true }] const task = db.createTask({ spec: 'implement feature' }) @@ -147,6 +153,7 @@ describe('Coordinator', () => { expect(result.status).toBe('completed') expect(result.completedTasks).toContain(task.id) expect(runtime.sentMessages.length).toBeGreaterThan(0) + expect(runtime.sentMessages[0].text).toContain('orca-ide orchestration send') }) it('records the assignee pane key when the runtime can resolve one', async () => { diff --git a/src/main/runtime/orchestration/coordinator.ts b/src/main/runtime/orchestration/coordinator.ts index 73307eddd35..04f0788577c 100644 --- a/src/main/runtime/orchestration/coordinator.ts +++ b/src/main/runtime/orchestration/coordinator.ts @@ -32,6 +32,9 @@ export type CoordinatorRuntime = { // Why: optional so lightweight runtime fakes keep compiling; when present, // dispatch records the remint-stable pane identity of the assignee. getTerminalPaneKey?(handle: string): string | null + // Why: Windows can host native and WSL workers concurrently, so the + // worker pane—not the coordinator process—selects the packaged CLI name. + getTerminalOrchestrationCliCommand?(handle: string): 'orca' | 'orca-ide' } // Why (§3.1): single threshold, no warn/refuse split. Coordinator picked 20 @@ -486,6 +489,9 @@ export class Coordinator { coordinatorHandle: this.opts.coordinatorHandle, workerHandle: targetHandle, devMode: process.env.ORCA_USER_DATA_PATH?.includes('orca-dev'), + ...(this.runtime.getTerminalOrchestrationCliCommand + ? { cliCommand: this.runtime.getTerminalOrchestrationCliCommand(targetHandle) } + : {}), // Why (§3.2): drift section fires only when behind > 0. The preamble // builder gates on this itself; passing the object unconditionally lets // the coordinator stay dumb about the display rule. diff --git a/src/main/runtime/orchestration/preamble.test.ts b/src/main/runtime/orchestration/preamble.test.ts index 198437e4cd7..ef133551034 100644 --- a/src/main/runtime/orchestration/preamble.test.ts +++ b/src/main/runtime/orchestration/preamble.test.ts @@ -159,7 +159,7 @@ describe('buildDispatchPreamble', () => { }) it('uses orca-dev CLI when devMode is true', () => { - const result = buildDispatchPreamble(baseParams({ devMode: true })) + const result = buildDispatchPreamble(baseParams({ devMode: true, cliCommand: 'orca-ide' })) expect(result).toContain('orca-dev orchestration send') expect(result).toContain('orca-dev orchestration check') expect(result).toContain('orca-dev orchestration ask') @@ -175,6 +175,15 @@ describe('buildDispatchPreamble', () => { expect(result).toContain('orca orchestration check') }) + it('uses the exact orca-ide command for packaged WSL workers', () => { + const result = buildDispatchPreamble(baseParams({ cliCommand: 'orca-ide' })) + + expect(result).toContain('orca-ide orchestration send') + expect(result).toContain('orca-ide orchestration check') + expect(result).toContain('orca-ide orchestration ask') + expect(result).not.toMatch(/(^|\s)orca orchestration/m) + }) + it('appends a BASE DRIFT section when baseDrift.behind > 0', () => { const result = buildDispatchPreamble({ taskId: 'task_x', diff --git a/src/main/runtime/orchestration/preamble.ts b/src/main/runtime/orchestration/preamble.ts index 9aaa488318f..a8012c56854 100644 --- a/src/main/runtime/orchestration/preamble.ts +++ b/src/main/runtime/orchestration/preamble.ts @@ -1,3 +1,5 @@ +import type { OrchestrationCliCommand } from './cli-command' + export type PreambleParams = { taskId: string // Why: completion and heartbeat payloads attribute activity to a specific @@ -10,6 +12,9 @@ export type PreambleParams = { coordinatorHandle: string workerHandle: string devMode?: boolean + // Why: packaged WSL panes install the scoped launcher as `orca-ide`; + // other execution hosts keep their existing bare `orca` bridge. + cliCommand?: OrchestrationCliCommand // Why: populated by the coordinator's dispatch pre-flight (§3.1) only // when the target worktree is behind its tracking remote. When absent // or when `behind === 0`, the preamble emits no drift section. Callers @@ -42,7 +47,7 @@ export function buildDispatchPreamble(params: PreambleParams): string { // Why: in dev mode, agents must use orca-dev to connect to the dev runtime's // socket. Without this, agents inside the dev Electron app would call the // production CLI and talk to the wrong Orca instance (Section 6.4). - const cli = params.devMode ? 'orca-dev' : 'orca' + const cli = params.devMode ? 'orca-dev' : (params.cliCommand ?? 'orca') const postDoneInstructions = buildPostWorkerDoneInstructions({ cli, workerKind: params.workerKind ?? 'prompt-returning-agent' diff --git a/src/main/runtime/rpc/methods/orchestration.test.ts b/src/main/runtime/rpc/methods/orchestration.test.ts index fc7536c2e4f..95339cb3190 100644 --- a/src/main/runtime/rpc/methods/orchestration.test.ts +++ b/src/main/runtime/rpc/methods/orchestration.test.ts @@ -1369,6 +1369,22 @@ describe('orchestration RPC methods', () => { ) }) + it('uses the target pane CLI command for the returned preamble', async () => { + setup() + const task = db.createTask({ spec: 'work' }) + vi.spyOn(runtime, 'getTerminalOrchestrationCliCommand').mockReturnValue('orca-ide') + + const result = (await call('orchestration.dispatch', { + task: task.id, + to: 'term_wsl', + returnPreamble: true + })) as { preamble: string } + + expect(runtime.getTerminalOrchestrationCliCommand).toHaveBeenCalledWith('term_wsl') + expect(result.preamble).toContain('orca-ide orchestration send') + expect(result.preamble).not.toMatch(/(^|\s)orca orchestration/m) + }) + it('injects preamble through the agent prompt path instead of raw terminal send', async () => { setup() const task = db.createTask({ spec: 'line one\nline two' }) diff --git a/src/main/runtime/rpc/methods/orchestration.ts b/src/main/runtime/rpc/methods/orchestration.ts index 9caaf1ac57a..bd2721d1b99 100644 --- a/src/main/runtime/rpc/methods/orchestration.ts +++ b/src/main/runtime/rpc/methods/orchestration.ts @@ -495,7 +495,10 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [ taskSpec: task.spec, coordinatorHandle: params.from ?? 'coordinator', workerHandle: params.to ?? 'worker', - devMode: params.devMode + devMode: params.devMode, + ...(params.to + ? { cliCommand: runtime.getTerminalOrchestrationCliCommand(params.to) } + : {}) }) return { dispatch: null, injected: false, dryRun: true, preamble } } @@ -540,7 +543,8 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [ taskSpec: task.spec, coordinatorHandle: params.from ?? 'coordinator', workerHandle: to, - devMode: params.devMode + devMode: params.devMode, + cliCommand: runtime.getTerminalOrchestrationCliCommand(to) }) let injected = false @@ -583,6 +587,7 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [ if (!task) { throw new Error(`Task not found: ${params.task}`) } + const workerHandle = ctx?.assignee_handle ?? 'worker' const preamble = buildDispatchPreamble({ taskId: task.id, // Why: prefer the existing dispatch context's id if we have one @@ -591,8 +596,9 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [ dispatchId: ctx?.id ?? 'ctx_preview', taskSpec: task.spec, coordinatorHandle: params.from ?? 'coordinator', - workerHandle: ctx?.assignee_handle ?? 'worker', - devMode: params.devMode + workerHandle, + devMode: params.devMode, + ...(ctx ? { cliCommand: runtime.getTerminalOrchestrationCliCommand(workerHandle) } : {}) }) return { dispatch: ctx ?? null, preamble } } From 6091995e75ea5b3bd88137b306dff081ab45233c Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 13 Jul 2026 05:40:02 -0700 Subject: [PATCH 03/23] perf(worktree-watcher): skip head-identity reads on index-only bursts (#8558) An index rewrite cannot move HEAD, yet every quiet local Git-common status burst was calling readGitCommonHeadIdentities, which scans the primary checkout plus every linked worktree sequentially (~1,500 file reads for 500 worktrees) even when only index files changed. Split the conflated git-status signal into two: index churn stays `gitStatusRepoIds`, while logs/HEAD (and other reflog head moves) become a distinct `headIdentityRepoIds` signal. Source Control is still notified for both, but readGitCommonHeadIdentities now runs only for true head triggers (and structural ticks, which re-baseline silently). Index-only bursts do zero head-identity reads. Adds deterministic tests: index-only bursts read zero head identities across linked and primary checkouts; logs/HEAD still refreshes identities for both; plus coalescing and debounce coverage. --- ...orktree-base-directory-change-collector.ts | 75 ++++--- ...rktree-base-directory-event-filter.test.ts | 96 ++++++-- .../worktree-base-directory-event-filter.ts | 70 ++++-- .../worktree-base-directory-watcher.test.ts | 210 ++++++++++++++++-- .../ipc/worktree-base-directory-watcher.ts | 102 +++++---- 5 files changed, 413 insertions(+), 140 deletions(-) diff --git a/src/main/ipc/worktree-base-directory-change-collector.ts b/src/main/ipc/worktree-base-directory-change-collector.ts index 829aa8c3b82..168b32b48ad 100644 --- a/src/main/ipc/worktree-base-directory-change-collector.ts +++ b/src/main/ipc/worktree-base-directory-change-collector.ts @@ -13,35 +13,55 @@ export type WorktreeBaseCollectedChanges = { overflow: boolean structureRepoIds: string[] gitStatusRepoIds: string[] + headIdentityRepoIds: string[] +} + +type ChangeBuckets = { + structureRepoIds: Set + gitStatusRepoIds: Set + headIdentityRepoIds: Set +} + +function emptyBuckets(): ChangeBuckets { + return { + structureRepoIds: new Set(), + gitStatusRepoIds: new Set(), + headIdentityRepoIds: new Set() + } } function emptyChanges(): WorktreeBaseCollectedChanges { - return { overflow: false, structureRepoIds: [], gitStatusRepoIds: [] } + return { + overflow: false, + structureRepoIds: [], + gitStatusRepoIds: [], + headIdentityRepoIds: [] + } } function addMatchingChange( target: WorktreeBaseWatchTarget, event: WorktreeBaseWatcherEvent, - structureRepoIds: Set, - gitStatusRepoIds: Set + buckets: ChangeBuckets ): void { const change = classifyWorktreeBaseChange(target, event) for (const repoId of change.structureRepoIds) { - structureRepoIds.add(repoId) + buckets.structureRepoIds.add(repoId) } for (const repoId of change.gitStatusRepoIds) { - gitStatusRepoIds.add(repoId) + buckets.gitStatusRepoIds.add(repoId) + } + for (const repoId of change.headIdentityRepoIds) { + buckets.headIdentityRepoIds.add(repoId) } } -function toCollectedChanges( - structureRepoIds: Set, - gitStatusRepoIds: Set -): WorktreeBaseCollectedChanges { +function toCollectedChanges(buckets: ChangeBuckets): WorktreeBaseCollectedChanges { return { overflow: false, - structureRepoIds: [...structureRepoIds], - gitStatusRepoIds: [...gitStatusRepoIds] + structureRepoIds: [...buckets.structureRepoIds], + gitStatusRepoIds: [...buckets.gitStatusRepoIds], + headIdentityRepoIds: [...buckets.headIdentityRepoIds] } } @@ -49,47 +69,30 @@ export function collectLocalWorktreeBaseChanges( target: WorktreeBaseWatchTarget, events: WorktreeBaseWatcherEvent[] ): WorktreeBaseCollectedChanges { - const structureRepoIds = new Set() - const gitStatusRepoIds = new Set() + const buckets = emptyBuckets() for (const event of events) { - addMatchingChange(target, event, structureRepoIds, gitStatusRepoIds) + addMatchingChange(target, event, buckets) } - return toCollectedChanges(structureRepoIds, gitStatusRepoIds) + return toCollectedChanges(buckets) } export function collectRemoteWorktreeBaseChanges( target: WorktreeBaseWatchTarget, events: FsChangeEvent[] ): WorktreeBaseCollectedChanges { - const structureRepoIds = new Set() - const gitStatusRepoIds = new Set() + const buckets = emptyBuckets() for (const event of events) { if (event.kind === 'overflow') { return { ...emptyChanges(), overflow: true } } if (event.kind === 'rename') { if (event.oldAbsolutePath) { - addMatchingChange( - target, - { type: 'delete', path: event.oldAbsolutePath }, - structureRepoIds, - gitStatusRepoIds - ) + addMatchingChange(target, { type: 'delete', path: event.oldAbsolutePath }, buckets) } - addMatchingChange( - target, - { type: 'create', path: event.absolutePath }, - structureRepoIds, - gitStatusRepoIds - ) + addMatchingChange(target, { type: 'create', path: event.absolutePath }, buckets) continue } - addMatchingChange( - target, - { type: event.kind, path: event.absolutePath }, - structureRepoIds, - gitStatusRepoIds - ) + addMatchingChange(target, { type: event.kind, path: event.absolutePath }, buckets) } - return toCollectedChanges(structureRepoIds, gitStatusRepoIds) + return toCollectedChanges(buckets) } diff --git a/src/main/ipc/worktree-base-directory-event-filter.test.ts b/src/main/ipc/worktree-base-directory-event-filter.test.ts index 2a87b9f0b25..330be8b925c 100644 --- a/src/main/ipc/worktree-base-directory-event-filter.test.ts +++ b/src/main/ipc/worktree-base-directory-event-filter.test.ts @@ -25,13 +25,21 @@ describe('matchingWorktreeBaseRepoIds (git-common)', () => { type: 'update', path: join(COMMON_DIR, 'worktrees', 'wt-a', 'HEAD') }) - ).toEqual({ structureRepoIds: ['repo-1'], gitStatusRepoIds: [] }) + ).toEqual({ + structureRepoIds: ['repo-1'], + gitStatusRepoIds: [], + headIdentityRepoIds: [] + }) expect( classifyWorktreeBaseChange(target, { type: 'create', path: join(COMMON_DIR, 'worktrees', 'wt-b') }) - ).toEqual({ structureRepoIds: ['repo-1'], gitStatusRepoIds: [] }) + ).toEqual({ + structureRepoIds: ['repo-1'], + gitStatusRepoIds: [], + headIdentityRepoIds: [] + }) expect( matchingWorktreeBaseRepoIds(target, { type: 'delete', @@ -44,12 +52,26 @@ describe('matchingWorktreeBaseRepoIds (git-common)', () => { const target = makeGitCommonTarget() for (const file of ['HEAD', 'packed-refs']) { expect( - classifyWorktreeBaseChange(target, { type: 'update', path: join(COMMON_DIR, file) }) - ).toEqual({ structureRepoIds: ['repo-1'], gitStatusRepoIds: [] }) + classifyWorktreeBaseChange(target, { + type: 'update', + path: join(COMMON_DIR, file) + }) + ).toEqual({ + structureRepoIds: ['repo-1'], + gitStatusRepoIds: [], + headIdentityRepoIds: [] + }) } expect( - classifyWorktreeBaseChange(target, { type: 'update', path: join(COMMON_DIR, 'index') }) - ).toEqual({ structureRepoIds: [], gitStatusRepoIds: ['repo-1'] }) + classifyWorktreeBaseChange(target, { + type: 'update', + path: join(COMMON_DIR, 'index') + }) + ).toEqual({ + structureRepoIds: [], + gitStatusRepoIds: ['repo-1'], + headIdentityRepoIds: [] + }) }) it('classifies linked-worktree index as status-only', () => { @@ -59,16 +81,24 @@ describe('matchingWorktreeBaseRepoIds (git-common)', () => { type: 'update', path: join(COMMON_DIR, 'worktrees', 'wt-a', 'index') }) - ).toEqual({ structureRepoIds: [], gitStatusRepoIds: ['repo-1'] }) + ).toEqual({ + structureRepoIds: [], + gitStatusRepoIds: ['repo-1'], + headIdentityRepoIds: [] + }) expect( classifyWorktreeBaseChange(target, { type: 'update', path: join(COMMON_DIR, 'worktrees', 'wt-a', 'index.lock') }) - ).toEqual({ structureRepoIds: [], gitStatusRepoIds: [] }) + ).toEqual({ + structureRepoIds: [], + gitStatusRepoIds: [], + headIdentityRepoIds: [] + }) }) - it('classifies HEAD reflog appends as status-only for linked and primary checkouts', () => { + it('classifies HEAD reflog appends as head-identity triggers for linked and primary checkouts', () => { const target = makeGitCommonTarget() // commit --amend / reset --soft move HEAD without touching index or HEAD. expect( @@ -76,26 +106,42 @@ describe('matchingWorktreeBaseRepoIds (git-common)', () => { type: 'update', path: join(COMMON_DIR, 'worktrees', 'wt-a', 'logs', 'HEAD') }) - ).toEqual({ structureRepoIds: [], gitStatusRepoIds: ['repo-1'] }) + ).toEqual({ + structureRepoIds: [], + gitStatusRepoIds: [], + headIdentityRepoIds: ['repo-1'] + }) expect( classifyWorktreeBaseChange(target, { type: 'update', path: join(COMMON_DIR, 'logs', 'HEAD') }) - ).toEqual({ structureRepoIds: [], gitStatusRepoIds: ['repo-1'] }) + ).toEqual({ + structureRepoIds: [], + gitStatusRepoIds: [], + headIdentityRepoIds: ['repo-1'] + }) // Per-ref reflogs churn on fetches and stay ignored. expect( classifyWorktreeBaseChange(target, { type: 'update', path: join(COMMON_DIR, 'logs', 'refs', 'heads', 'main') }) - ).toEqual({ structureRepoIds: [], gitStatusRepoIds: [] }) + ).toEqual({ + structureRepoIds: [], + gitStatusRepoIds: [], + headIdentityRepoIds: [] + }) expect( classifyWorktreeBaseChange(target, { type: 'update', path: join(COMMON_DIR, 'worktrees', 'wt-a', 'logs', 'refs', 'heads', 'main') }) - ).toEqual({ structureRepoIds: [], gitStatusRepoIds: [] }) + ).toEqual({ + structureRepoIds: [], + gitStatusRepoIds: [], + headIdentityRepoIds: [] + }) }) it('classifies worktree-scoped config as structural for sparse-flag freshness', () => { @@ -105,13 +151,21 @@ describe('matchingWorktreeBaseRepoIds (git-common)', () => { type: 'update', path: join(COMMON_DIR, 'worktrees', 'wt-a', 'config.worktree') }) - ).toEqual({ structureRepoIds: ['repo-1'], gitStatusRepoIds: [] }) + ).toEqual({ + structureRepoIds: ['repo-1'], + gitStatusRepoIds: [], + headIdentityRepoIds: [] + }) expect( classifyWorktreeBaseChange(target, { type: 'create', path: join(COMMON_DIR, 'config.worktree') }) - ).toEqual({ structureRepoIds: ['repo-1'], gitStatusRepoIds: [] }) + ).toEqual({ + structureRepoIds: ['repo-1'], + gitStatusRepoIds: [], + headIdentityRepoIds: [] + }) }) it('classifies Windows-shaped linked metadata paths', () => { @@ -126,13 +180,21 @@ describe('matchingWorktreeBaseRepoIds (git-common)', () => { type: 'update', path: win32.join(commonDir, 'worktrees', 'wt a', 'gitdir') }) - ).toEqual({ structureRepoIds: ['repo-1'], gitStatusRepoIds: [] }) + ).toEqual({ + structureRepoIds: ['repo-1'], + gitStatusRepoIds: [], + headIdentityRepoIds: [] + }) expect( classifyWorktreeBaseChange(target, { type: 'update', path: win32.join(commonDir, 'worktrees', 'wt a', 'index') }) - ).toEqual({ structureRepoIds: [], gitStatusRepoIds: ['repo-1'] }) + ).toEqual({ + structureRepoIds: [], + gitStatusRepoIds: ['repo-1'], + headIdentityRepoIds: [] + }) }) it('ignores non-status common-dir churn', () => { diff --git a/src/main/ipc/worktree-base-directory-event-filter.ts b/src/main/ipc/worktree-base-directory-event-filter.ts index 7f161e33823..551af5a6568 100644 --- a/src/main/ipc/worktree-base-directory-event-filter.ts +++ b/src/main/ipc/worktree-base-directory-event-filter.ts @@ -11,6 +11,10 @@ type WorktreeBaseWatcherEvent = { export type WorktreeBaseChangeClass = { structureRepoIds: string[] gitStatusRepoIds: string[] + // logs/HEAD and other true head-move triggers: notify Source Control like + // status churn, but distinct so only these re-read head identities. An index + // rewrite cannot move HEAD, so it must never land here. + headIdentityRepoIds: string[] } export type WorktreeBaseWatchKind = 'base' | 'git-common' @@ -98,9 +102,10 @@ const GIT_COMMON_PRIMARY_STATUS_FILES = new Set(['index']) const GIT_COMMON_LINKED_STRUCTURAL_FILES = new Set(['HEAD', 'gitdir', 'locked', 'config.worktree']) const GIT_COMMON_LINKED_STATUS_FILES = new Set(['index']) -// `logs/HEAD` is the status-only trigger for head moves that rewrite no +// `logs/HEAD` is the head-identity trigger for head moves that rewrite no // watched leaf (commit --amend, reset --soft): every ref update through a -// checkout appends there, while `git status` churn never touches it. +// checkout appends there, while `git status` churn never touches it. It is +// kept separate from index churn so only these events re-read head identities. function isHeadLogParts(parts: string[], offset: number): boolean { return parts.length === offset + 2 && parts[offset] === 'logs' && parts[offset + 1] === 'HEAD' } @@ -109,6 +114,36 @@ function allRepoIds(target: WorktreeBaseWatchTarget): string[] { return [...target.repos.keys()] } +const NO_CHANGE: WorktreeBaseChangeClass = { + structureRepoIds: [], + gitStatusRepoIds: [], + headIdentityRepoIds: [] +} + +function structuralChange(repoIds: string[]): WorktreeBaseChangeClass { + return { + structureRepoIds: repoIds, + gitStatusRepoIds: [], + headIdentityRepoIds: [] + } +} + +function gitStatusChange(repoIds: string[]): WorktreeBaseChangeClass { + return { + structureRepoIds: [], + gitStatusRepoIds: repoIds, + headIdentityRepoIds: [] + } +} + +function headIdentityChange(repoIds: string[]): WorktreeBaseChangeClass { + return { + structureRepoIds: [], + gitStatusRepoIds: [], + headIdentityRepoIds: repoIds + } +} + // Why: Git records linked worktrees under the common dir's `worktrees` // metadata, which is lower churn than watching checkout contents. function classifyGitCommonEvent( @@ -117,54 +152,49 @@ function classifyGitCommonEvent( ): WorktreeBaseChangeClass { const parts = pathRelativeToWorktreeWatchRoot(target.path, event.path) if (!parts) { - return { structureRepoIds: [], gitStatusRepoIds: [] } + return NO_CHANGE } const repoIds = allRepoIds(target) if (parts.length === 1) { if (parts[0] === 'worktrees') { - return { structureRepoIds: repoIds, gitStatusRepoIds: [] } + return structuralChange(repoIds) } if (GIT_COMMON_PRIMARY_STRUCTURAL_FILES.has(parts[0])) { - return { structureRepoIds: repoIds, gitStatusRepoIds: [] } + return structuralChange(repoIds) } if (GIT_COMMON_PRIMARY_STATUS_FILES.has(parts[0])) { - return { structureRepoIds: [], gitStatusRepoIds: repoIds } + return gitStatusChange(repoIds) } - return { structureRepoIds: [], gitStatusRepoIds: [] } + return NO_CHANGE } if (parts[0] !== 'worktrees') { if (isHeadLogParts(parts, 0)) { - return { structureRepoIds: [], gitStatusRepoIds: repoIds } + return headIdentityChange(repoIds) } - return { structureRepoIds: [], gitStatusRepoIds: [] } + return NO_CHANGE } if (parts.length === 2) { - return event.type === 'update' - ? { structureRepoIds: [], gitStatusRepoIds: [] } - : { structureRepoIds: repoIds, gitStatusRepoIds: [] } + return event.type === 'update' ? NO_CHANGE : structuralChange(repoIds) } if (parts.length === 3) { if (GIT_COMMON_LINKED_STRUCTURAL_FILES.has(parts[2])) { - return { structureRepoIds: repoIds, gitStatusRepoIds: [] } + return structuralChange(repoIds) } if (GIT_COMMON_LINKED_STATUS_FILES.has(parts[2])) { - return { structureRepoIds: [], gitStatusRepoIds: repoIds } + return gitStatusChange(repoIds) } } if (isHeadLogParts(parts, 2)) { - return { structureRepoIds: [], gitStatusRepoIds: repoIds } + return headIdentityChange(repoIds) } - return { structureRepoIds: [], gitStatusRepoIds: [] } + return NO_CHANGE } function classifyBaseEvent( target: WorktreeBaseWatchTarget, event: WorktreeBaseWatcherEvent ): WorktreeBaseChangeClass { - return { - structureRepoIds: matchingBaseRepoIds(target, event.path, event.type), - gitStatusRepoIds: [] - } + return structuralChange(matchingBaseRepoIds(target, event.path, event.type)) } export function classifyWorktreeBaseChange( diff --git a/src/main/ipc/worktree-base-directory-watcher.test.ts b/src/main/ipc/worktree-base-directory-watcher.test.ts index 7f580421aaa..c560b5e51aa 100644 --- a/src/main/ipc/worktree-base-directory-watcher.test.ts +++ b/src/main/ipc/worktree-base-directory-watcher.test.ts @@ -116,7 +116,10 @@ describe('worktree base directory watcher', () => { emit(WORKTREE_ROOT, [ { type: 'create', path: join(WORKTREE_ROOT, 'project', 'external-5104') }, - { type: 'create', path: join(WORKTREE_ROOT, 'project', 'external-5104', '.git') } + { + type: 'create', + path: join(WORKTREE_ROOT, 'project', 'external-5104', '.git') + } ]) await vi.advanceTimersByTimeAsync(300) @@ -133,7 +136,10 @@ describe('worktree base directory watcher', () => { ) emit(WORKTREE_ROOT, [ - { type: 'create', path: join(WORKTREE_ROOT, 'project', 'external-5104', '.git') } + { + type: 'create', + path: join(WORKTREE_ROOT, 'project', 'external-5104', '.git') + } ]) destroyed = true await vi.advanceTimersByTimeAsync(300) @@ -145,7 +151,10 @@ describe('worktree base directory watcher', () => { await syncWorktreeBaseDirectoryWatchers(makeStore([makeRepo()]) as never, makeWindow() as never) emit(WORKTREE_ROOT, [ - { type: 'update', path: join(WORKTREE_ROOT, 'project', 'existing', 'src', 'file.ts') } + { + type: 'update', + path: join(WORKTREE_ROOT, 'project', 'existing', 'src', 'file.ts') + } ]) await vi.advanceTimersByTimeAsync(300) @@ -156,7 +165,10 @@ describe('worktree base directory watcher', () => { await syncWorktreeBaseDirectoryWatchers(makeStore([makeRepo()]) as never, makeWindow() as never) emit(PROJECT_GIT_COMMON_DIR, [ - { type: 'create', path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'gitdir') } + { + type: 'create', + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'gitdir') + } ]) await vi.advanceTimersByTimeAsync(300) @@ -167,9 +179,18 @@ describe('worktree base directory watcher', () => { await syncWorktreeBaseDirectoryWatchers(makeStore([makeRepo()]) as never, makeWindow() as never) emit(PROJECT_GIT_COMMON_DIR, [ - { type: 'create', path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'index') }, - { type: 'update', path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'index') }, - { type: 'delete', path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'index') } + { + type: 'create', + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'index') + }, + { + type: 'update', + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'index') + }, + { + type: 'delete', + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'index') + } ]) await vi.advanceTimersByTimeAsync(300) @@ -182,8 +203,14 @@ describe('worktree base directory watcher', () => { await syncWorktreeBaseDirectoryWatchers(makeStore([makeRepo()]) as never, makeWindow() as never) emit(PROJECT_GIT_COMMON_DIR, [ - { type: 'update', path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'HEAD') }, - { type: 'create', path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'locked') } + { + type: 'update', + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'HEAD') + }, + { + type: 'create', + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'locked') + } ]) await vi.advanceTimersByTimeAsync(300) @@ -192,17 +219,25 @@ describe('worktree base directory watcher', () => { expect(notifyWorktreeGitStatusMetadataChanged).not.toHaveBeenCalled() }) - it('emits head identities for status-only head moves without structural fanout', async () => { + it('emits head identities for a linked reflog head move without structural fanout', async () => { const linkedWorktree = absolutePath('workspace', 'worktrees', 'project', 'external-5104') vi.mocked(readGitCommonHeadIdentities).mockResolvedValue([ - { worktreePath: linkedWorktree, head: 'aaa111', branch: 'refs/heads/feature' } + { + worktreePath: linkedWorktree, + head: 'aaa111', + branch: 'refs/heads/feature' + } ]) await syncWorktreeBaseDirectoryWatchers(makeStore([makeRepo()]) as never, makeWindow() as never) await vi.advanceTimersByTimeAsync(0) // External commit --amend: only logs/HEAD moves, no index write. vi.mocked(readGitCommonHeadIdentities).mockResolvedValue([ - { worktreePath: linkedWorktree, head: 'bbb222', branch: 'refs/heads/feature' } + { + worktreePath: linkedWorktree, + head: 'bbb222', + branch: 'refs/heads/feature' + } ]) emit(PROJECT_GIT_COMMON_DIR, [ { @@ -217,50 +252,181 @@ describe('worktree base directory watcher', () => { expect(notifyWorktreeGitStatusMetadataChanged).toHaveBeenCalledTimes(1) expect(notifyWorktreeHeadIdentitiesChanged).toHaveBeenCalledTimes(1) expect(notifyWorktreeHeadIdentitiesChanged).toHaveBeenCalledWith(expect.anything(), 'repo-1', [ - { worktreePath: linkedWorktree, head: 'bbb222', branch: 'refs/heads/feature' } + { + worktreePath: linkedWorktree, + head: 'bbb222', + branch: 'refs/heads/feature' + } ]) }) - it('does not emit head identities when a status-only burst leaves heads unchanged', async () => { + it('makes zero head-identity reads on an index-only burst across linked and primary checkouts', async () => { const linkedWorktree = absolutePath('workspace', 'worktrees', 'project', 'external-5104') vi.mocked(readGitCommonHeadIdentities).mockResolvedValue([ - { worktreePath: linkedWorktree, head: 'aaa111', branch: 'refs/heads/feature' } + { + worktreePath: linkedWorktree, + head: 'aaa111', + branch: 'refs/heads/feature' + } ]) await syncWorktreeBaseDirectoryWatchers(makeStore([makeRepo()]) as never, makeWindow() as never) await vi.advanceTimersByTimeAsync(0) + // Subscribe baselines heads once; the index burst must add no further reads. + vi.mocked(readGitCommonHeadIdentities).mockClear() emit(PROJECT_GIT_COMMON_DIR, [ - { type: 'update', path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'index') } + { + type: 'create', + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'index') + }, + { + type: 'update', + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'index') + }, + { type: 'update', path: join(PROJECT_GIT_COMMON_DIR, 'index') } ]) await vi.advanceTimersByTimeAsync(300) await vi.advanceTimersByTimeAsync(0) - expect(notifyWorktreeGitStatusMetadataChanged).toHaveBeenCalledTimes(1) + // An index rewrite cannot move HEAD, so the linked-worktree scan is skipped. + expect(readGitCommonHeadIdentities).not.toHaveBeenCalled() expect(notifyWorktreeHeadIdentitiesChanged).not.toHaveBeenCalled() + // Source Control still refreshes for the status churn, coalesced to one call. + expect(notifyWorktreeGitStatusMetadataChanged).toHaveBeenCalledTimes(1) + expect(notifyWorktreesChanged).not.toHaveBeenCalled() + }) + + it('emits head identities for a primary-checkout reflog head move', async () => { + vi.mocked(readGitCommonHeadIdentities).mockResolvedValue([ + { worktreePath: PROJECT_ROOT, head: 'aaa111', branch: 'refs/heads/main' } + ]) + await syncWorktreeBaseDirectoryWatchers(makeStore([makeRepo()]) as never, makeWindow() as never) + await vi.advanceTimersByTimeAsync(0) + + vi.mocked(readGitCommonHeadIdentities).mockResolvedValue([ + { worktreePath: PROJECT_ROOT, head: 'bbb222', branch: 'refs/heads/main' } + ]) + emit(PROJECT_GIT_COMMON_DIR, [ + { type: 'update', path: join(PROJECT_GIT_COMMON_DIR, 'logs', 'HEAD') } + ]) + await vi.advanceTimersByTimeAsync(300) + await vi.advanceTimersByTimeAsync(0) + + expect(notifyWorktreesChanged).not.toHaveBeenCalled() + expect(notifyWorktreeHeadIdentitiesChanged).toHaveBeenCalledWith(expect.anything(), 'repo-1', [ + { + worktreePath: PROJECT_ROOT, + head: 'bbb222', + branch: 'refs/heads/main' + } + ]) + }) + + it('coalesces an index and reflog burst into one head read and one status refresh', async () => { + const linkedWorktree = absolutePath('workspace', 'worktrees', 'project', 'external-5104') + vi.mocked(readGitCommonHeadIdentities).mockResolvedValue([ + { + worktreePath: linkedWorktree, + head: 'aaa111', + branch: 'refs/heads/feature' + } + ]) + await syncWorktreeBaseDirectoryWatchers(makeStore([makeRepo()]) as never, makeWindow() as never) + await vi.advanceTimersByTimeAsync(0) + vi.mocked(readGitCommonHeadIdentities).mockClear() + + // reset --soft rewrites the index and appends logs/HEAD in the same burst. + vi.mocked(readGitCommonHeadIdentities).mockResolvedValue([ + { + worktreePath: linkedWorktree, + head: 'bbb222', + branch: 'refs/heads/feature' + } + ]) + emit(PROJECT_GIT_COMMON_DIR, [ + { + type: 'update', + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'index') + }, + { + type: 'update', + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'logs', 'HEAD') + } + ]) + await vi.advanceTimersByTimeAsync(300) + await vi.advanceTimersByTimeAsync(0) + + expect(readGitCommonHeadIdentities).toHaveBeenCalledTimes(1) + expect(notifyWorktreeGitStatusMetadataChanged).toHaveBeenCalledTimes(1) + expect(notifyWorktreeHeadIdentitiesChanged).toHaveBeenCalledTimes(1) + expect(notifyWorktreesChanged).not.toHaveBeenCalled() + }) + + it('debounces successive reflog events into a single head read', async () => { + const linkedWorktree = absolutePath('workspace', 'worktrees', 'project', 'external-5104') + vi.mocked(readGitCommonHeadIdentities).mockResolvedValue([ + { + worktreePath: linkedWorktree, + head: 'aaa111', + branch: 'refs/heads/feature' + } + ]) + await syncWorktreeBaseDirectoryWatchers(makeStore([makeRepo()]) as never, makeWindow() as never) + await vi.advanceTimersByTimeAsync(0) + vi.mocked(readGitCommonHeadIdentities).mockClear() + + const reflog = { + type: 'update' as const, + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'logs', 'HEAD') + } + emit(PROJECT_GIT_COMMON_DIR, [reflog]) + // Second burst arrives before the debounce window elapses and resets it. + await vi.advanceTimersByTimeAsync(100) + emit(PROJECT_GIT_COMMON_DIR, [reflog]) + await vi.advanceTimersByTimeAsync(300) + await vi.advanceTimersByTimeAsync(0) + + expect(readGitCommonHeadIdentities).toHaveBeenCalledTimes(1) + expect(notifyWorktreeGitStatusMetadataChanged).toHaveBeenCalledTimes(1) }) it('re-baselines head identities silently on structural notifications', async () => { const linkedWorktree = absolutePath('workspace', 'worktrees', 'project', 'external-5104') vi.mocked(readGitCommonHeadIdentities).mockResolvedValue([ - { worktreePath: linkedWorktree, head: 'aaa111', branch: 'refs/heads/feature' } + { + worktreePath: linkedWorktree, + head: 'aaa111', + branch: 'refs/heads/feature' + } ]) await syncWorktreeBaseDirectoryWatchers(makeStore([makeRepo()]) as never, makeWindow() as never) await vi.advanceTimersByTimeAsync(0) // Branch switch: structural path owns the refresh via the full listing. vi.mocked(readGitCommonHeadIdentities).mockResolvedValue([ - { worktreePath: linkedWorktree, head: 'ccc333', branch: 'refs/heads/other' } + { + worktreePath: linkedWorktree, + head: 'ccc333', + branch: 'refs/heads/other' + } ]) emit(PROJECT_GIT_COMMON_DIR, [ - { type: 'update', path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'HEAD') } + { + type: 'update', + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'HEAD') + } ]) await vi.advanceTimersByTimeAsync(300) await vi.advanceTimersByTimeAsync(0) expect(notifyWorktreeHeadIdentitiesChanged).not.toHaveBeenCalled() - // A later status-only event diffs against the re-baselined heads. + // A later reflog event diffs against the re-baselined heads, so an + // unchanged head is not re-reported. emit(PROJECT_GIT_COMMON_DIR, [ - { type: 'update', path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'index') } + { + type: 'update', + path: join(PROJECT_GIT_COMMON_DIR, 'worktrees', 'external-5104', 'logs', 'HEAD') + } ]) await vi.advanceTimersByTimeAsync(300) await vi.advanceTimersByTimeAsync(0) diff --git a/src/main/ipc/worktree-base-directory-watcher.ts b/src/main/ipc/worktree-base-directory-watcher.ts index d62eb2384ba..23352d688f1 100644 --- a/src/main/ipc/worktree-base-directory-watcher.ts +++ b/src/main/ipc/worktree-base-directory-watcher.ts @@ -25,6 +25,7 @@ type ActiveWatch = WorktreeBaseWatchTarget & { notifyTimer: ReturnType | null pendingStructureRepoIds: Set pendingGitStatusRepoIds: Set + pendingHeadIdentityRepoIds: Set headIdentityRefresh: WorktreeHeadIdentityRefreshState disposed: boolean } @@ -35,13 +36,17 @@ let syncGeneration = 0 let scheduledSync: ReturnType | null = null let latestSyncContext: { mainWindow: BrowserWindow; store: Store } | null = null -function scheduleNotification( - watch: ActiveWatch, - changes: { structureRepoIds?: readonly string[]; gitStatusRepoIds?: readonly string[] } -): void { +function clearPendingRepoIds(watch: ActiveWatch): void { + watch.pendingStructureRepoIds.clear() + watch.pendingGitStatusRepoIds.clear() + watch.pendingHeadIdentityRepoIds.clear() +} + +type PendingNotificationInput = Partial> + +function scheduleNotification(watch: ActiveWatch, changes: PendingNotificationInput): void { if (watch.disposed || watch.mainWindow.isDestroyed()) { - watch.pendingStructureRepoIds.clear() - watch.pendingGitStatusRepoIds.clear() + clearPendingRepoIds(watch) return } for (const repoId of changes.structureRepoIds ?? []) { @@ -50,37 +55,41 @@ function scheduleNotification( for (const repoId of changes.gitStatusRepoIds ?? []) { watch.pendingGitStatusRepoIds.add(repoId) } + for (const repoId of changes.headIdentityRepoIds ?? []) { + watch.pendingHeadIdentityRepoIds.add(repoId) + } if (watch.notifyTimer) { clearTimeout(watch.notifyTimer) } watch.notifyTimer = setTimeout(() => { watch.notifyTimer = null if (watch.disposed || watch.mainWindow.isDestroyed()) { - watch.pendingStructureRepoIds.clear() - watch.pendingGitStatusRepoIds.clear() + clearPendingRepoIds(watch) return } const pendingStructure = [...watch.pendingStructureRepoIds] - const pendingGitStatus = [...watch.pendingGitStatusRepoIds].filter( - (repoId) => !watch.pendingStructureRepoIds.has(repoId) + const hasHeadIdentity = watch.pendingHeadIdentityRepoIds.size > 0 + // Source Control refreshes on both index churn and head moves; structural + // repos already refresh via the authoritative listing, so drop them here. + const sourceControlRepoIds = new Set( + [...watch.pendingGitStatusRepoIds, ...watch.pendingHeadIdentityRepoIds].filter( + (repoId) => !watch.pendingStructureRepoIds.has(repoId) + ) ) - watch.pendingStructureRepoIds.clear() - watch.pendingGitStatusRepoIds.clear() + // Structural ticks refresh silently (emit=false): the authoritative listing + // already reported them, so this only re-baselines ahead of later head diffs. + const emitHeadIdentities = pendingStructure.length === 0 + clearPendingRepoIds(watch) for (const repoId of pendingStructure) { notifyWorktreesChanged(watch.mainWindow, repoId) } - for (const repoId of pendingGitStatus) { + for (const repoId of sourceControlRepoIds) { notifyWorktreeGitStatusMetadataChanged(watch.mainWindow, repoId) } - if (supportsHeadIdentityRefresh(watch)) { - // Emit only when no structural notification fired this tick: structural - // paths already run the authoritative worktree listing; the silent pass - // just re-baselines so later status-only diffs stay accurate. - void refreshWorktreeHeadIdentities( - watch, - watch.headIdentityRefresh, - pendingStructure.length === 0 - ) + // Only re-read head identities for true head triggers: an index rewrite + // cannot move HEAD, so status-only bursts skip the linked-worktree scan. + if (supportsHeadIdentityRefresh(watch) && (pendingStructure.length > 0 || hasHeadIdentity)) { + void refreshWorktreeHeadIdentities(watch, watch.headIdentityRefresh, emitHeadIdentities) } }, WATCH_DEBOUNCE_MS) } @@ -92,7 +101,9 @@ function supportsHeadIdentityRefresh(watch: ActiveWatch): boolean { } function hasCollectedChanges(changes: WorktreeBaseCollectedChanges): boolean { - return changes.structureRepoIds.length > 0 || changes.gitStatusRepoIds.length > 0 + return [changes.structureRepoIds, changes.gitStatusRepoIds, changes.headIdentityRepoIds].some( + (ids) => ids.length > 0 + ) } function handleLocalWatchEvents( @@ -131,6 +142,24 @@ function handleRemoteWatchEvents( } } +function createActiveWatch( + target: WorktreeBaseWatchTarget, + mainWindow: BrowserWindow, + subscription: ActiveWatch['subscription'] +): ActiveWatch { + return { + ...target, + mainWindow, + subscription, + notifyTimer: null, + pendingStructureRepoIds: new Set(), + pendingGitStatusRepoIds: new Set(), + pendingHeadIdentityRepoIds: new Set(), + headIdentityRefresh: createWorktreeHeadIdentityRefreshState(), + disposed: false + } +} + async function subscribeTarget( target: WorktreeBaseWatchTarget, mainWindow: BrowserWindow @@ -148,16 +177,9 @@ async function subscribeTarget( } handleRemoteWatchEvents(currentWatch, events) }) - activeWatch = { - ...target, - mainWindow, - subscription: { unsubscribe: async () => unwatch() }, - notifyTimer: null, - pendingStructureRepoIds: new Set(), - pendingGitStatusRepoIds: new Set(), - headIdentityRefresh: createWorktreeHeadIdentityRefreshState(), - disposed: false - } + activeWatch = createActiveWatch(target, mainWindow, { + unsubscribe: async () => unwatch() + }) return activeWatch } @@ -176,16 +198,7 @@ async function subscribeTarget( handleLocalWatchEvents(currentWatch, null, events) } ) - activeWatch = { - ...target, - mainWindow, - subscription, - notifyTimer: null, - pendingStructureRepoIds: new Set(), - pendingGitStatusRepoIds: new Set(), - headIdentityRefresh: createWorktreeHeadIdentityRefreshState(), - disposed: false - } + activeWatch = createActiveWatch(target, mainWindow, subscription) if (supportsHeadIdentityRefresh(activeWatch)) { // Baseline eagerly so the first status-only signal — possibly hours after // subscribe — diffs against subscribe-time heads instead of silently @@ -231,8 +244,7 @@ async function removeWatch(key: string): Promise { if (watch.notifyTimer) { clearTimeout(watch.notifyTimer) } - watch.pendingStructureRepoIds.clear() - watch.pendingGitStatusRepoIds.clear() + clearPendingRepoIds(watch) await watch.subscription.unsubscribe().catch((error) => { console.warn(`[worktree-base-watcher] failed to unwatch ${watch.path}:`, error) }) From 13ed697cc215dbe706008daf17e84db22aa8d4e5 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 13 Jul 2026 05:40:09 -0700 Subject: [PATCH 04/23] Fix windows serve wsl barrier (#8559) * fix(startup): bound serve WSL reconciliation wait * test(startup): cover WSL barrier fail-open on early reconciliation rejection Co-authored-by: Orca * feat(serve): surface managed WSL reconciliation status to headless clients Expose reconciliation state ('pending'|'settled'|'failed') in the orca_server_ready payload and a wsl-cli-barrier startup milestone, so headless/SSH agents can tell the fail-open barrier outlived its budget and a WSL PTY launch may still race an un-migrated registration. Co-authored-by: Orca --------- Co-authored-by: Orca --- src/main/index.ts | 20 ++++- .../startup/desktop-startup-ordering.test.ts | 26 ++++++- ...cli-reconciliation-startup-barrier.test.ts | 73 ++++++++++++++----- .../wsl-cli-reconciliation-startup-barrier.ts | 6 +- 4 files changed, 97 insertions(+), 28 deletions(-) diff --git a/src/main/index.ts b/src/main/index.ts index b76964ae872..5196b7eac01 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -229,6 +229,10 @@ const recoveryReloadInFlight = createWebContentsTimedFlag() let firstWindowStartupServicesReady: Promise = Promise.resolve() let managedWslCliReconciliationReady: Promise = Promise.resolve() let managedWslCliStartupBarrierReady: Promise = Promise.resolve() +// Why: the serve barrier fails open at its budget, so headless clients need the +// reconciliation state at ready time to know a WSL PTY launch may still race an +// un-migrated registration. 'settled' covers the off-Windows no-op fast path. +let managedWslCliReconciliationStatus: 'pending' | 'settled' | 'failed' = 'settled' // Why: GPU child crashes clustered right after launch indicate a broken driver; // track them so Orca can move this build onto software rendering. const gpuLaunchTimeMs = Date.now() @@ -1384,6 +1388,9 @@ async function printServeReady(options: ServeOptions): Promise { type: 'orca_server_ready', runtimeId: runtime.getRuntimeId(), endpoint, + // Why: the WSL reconciliation barrier fails open, so 'pending' warns clients + // it outlived the startup budget and a WSL PTY launch may still race a repair. + managedWslCliReconciliation: managedWslCliReconciliationStatus, pairing: pairing.available ? { url: pairing.pairingUrl, @@ -1610,6 +1617,7 @@ app.whenReady().then(async () => { // Why: managed WSL launchers live outside the Windows app bundle, so keep // their launcher and bridge contract synchronized across app updates. + managedWslCliReconciliationStatus = 'pending' managedWslCliReconciliationReady = reconcileManagedWslCliRegistrations({ isPackaged: app.isPackaged, userDataPath: getCanonicalUserDataPath(), @@ -1625,8 +1633,10 @@ app.whenReady().then(async () => { console.log(`[wsl-cli] Repaired managed registration in ${result.distro}.`) } } + managedWslCliReconciliationStatus = 'settled' }) .catch((error) => { + managedWslCliReconciliationStatus = 'failed' console.warn( '[wsl-cli] Managed registration reconciliation discovery failed:', error instanceof Error ? error.message : String(error) @@ -2090,9 +2100,13 @@ app.whenReady().then(async () => { } if (serveOptions) { - // Why: headless serve has no renderer startup barrier, so settle managed - // WSL command reconciliation before exposing its runtime transport. - await managedWslCliReconciliationReady + // Why: give managed WSL launchers a brief chance to migrate before headless + // PTYs become reachable without letting slow repairs withhold all RPC readiness. + logStartupMilestone('wsl-cli-barrier-start') + await managedWslCliStartupBarrierReady + logStartupMilestone('wsl-cli-barrier-resolved', { + reconciliation: managedWslCliReconciliationStatus + }) await startServeAgentHookServer() registerHeadlessPtyRuntime( runtime, diff --git a/src/main/startup/desktop-startup-ordering.test.ts b/src/main/startup/desktop-startup-ordering.test.ts index d99177285af..b9adfc6b8f2 100644 --- a/src/main/startup/desktop-startup-ordering.test.ts +++ b/src/main/startup/desktop-startup-ordering.test.ts @@ -2,7 +2,7 @@ import { readFileSync } from 'node:fs' import { join } from 'node:path' import { describe, expect, it } from 'vitest' -describe('desktop startup ordering', () => { +describe('startup ordering', () => { it('passes the startup barrier into PTY handlers without blocking window creation', () => { const source = readFileSync(join(process.cwd(), 'src/main/index.ts'), 'utf8') const attachStart = source.indexOf('attachMainWindowServices(') @@ -24,7 +24,7 @@ describe('desktop startup ordering', () => { expect(Math.max(rpcStartIndex, legacyRpcStartIndex)).toBeGreaterThanOrEqual(0) }) - it('shows the desktop window without waiting for WSL registration reconciliation', () => { + it('bounds WSL reconciliation before serve RPC while leaving desktop startup independent', () => { const source = readFileSync(join(process.cwd(), 'src/main/index.ts'), 'utf8') const barrierStart = source.indexOf("ipcMain.handle('app:awaitFirstWindowStartupServices'") const barrierEnd = source.indexOf("ipcMain.handle(\n 'app:startupDiagnostic'", barrierStart) @@ -43,12 +43,32 @@ describe('desktop startup ordering', () => { expect(serveStart).toBeGreaterThan(reconciliationStart) expect(serveEnd).toBeGreaterThan(serveStart) expect(desktopWindowStart).toBeGreaterThan(reconciliationStart) - expect(serveStartup).toContain('await managedWslCliReconciliationReady') + expect(serveStartup).toContain('await managedWslCliStartupBarrierReady') + expect(serveStartup).not.toContain('await managedWslCliReconciliationReady') + expect(serveStartup.indexOf('await managedWslCliStartupBarrierReady')).toBeLessThan( + serveStartup.indexOf('await runtimeRpc.start()') + ) expect(desktopStartup).not.toContain('await managedWslCliReconciliationReady') expect(barrier).toContain('managedWslCliStartupBarrierReady') expect(barrier).not.toContain('managedWslCliReconciliationReady') }) + it('exposes managed WSL reconciliation status to headless serve clients and diagnostics', () => { + const source = readFileSync(join(process.cwd(), 'src/main/index.ts'), 'utf8') + + // Why: the barrier fails open, so the serve-ready payload must carry the + // reconciliation state and the bounded wait must be traceable via a milestone. + const readyStart = source.indexOf("type: 'orca_server_ready'") + const readyEnd = source.indexOf('pairing: pairing.available', readyStart) + const readyPayload = source.slice(readyStart, readyEnd) + expect(readyPayload).toContain('managedWslCliReconciliation: managedWslCliReconciliationStatus') + + expect(source).toContain("managedWslCliReconciliationStatus = 'pending'") + expect(source).toContain("managedWslCliReconciliationStatus = 'settled'") + expect(source).toContain("managedWslCliReconciliationStatus = 'failed'") + expect(source).toContain("logStartupMilestone('wsl-cli-barrier-resolved'") + }) + it('does not run the rate-limit quota fetch before the first window can show results', () => { const source = readFileSync(join(process.cwd(), 'src/main/index.ts'), 'utf8') const attachIndex = source.indexOf('rateLimits.attach(window)') diff --git a/src/main/startup/wsl-cli-reconciliation-startup-barrier.test.ts b/src/main/startup/wsl-cli-reconciliation-startup-barrier.test.ts index e59201e6a02..b2a56a415e8 100644 --- a/src/main/startup/wsl-cli-reconciliation-startup-barrier.test.ts +++ b/src/main/startup/wsl-cli-reconciliation-startup-barrier.test.ts @@ -30,13 +30,13 @@ describe('createWslCliReconciliationStartupBarrier', () => { } }) - it('fails open when reconciliation exceeds the startup budget', async () => { + it('fails open immediately when reconciliation rejects before the budget expires', async () => { vi.useFakeTimers() - let resolveReconciliation!: () => void + let rejectReconciliation!: (error: Error) => void try { - const reconciliation = new Promise((resolve) => { - resolveReconciliation = resolve + const reconciliation = new Promise((_resolve, reject) => { + rejectReconciliation = reject }) const barrier = createWslCliReconciliationStartupBarrier(reconciliation) let barrierSettled = false @@ -44,38 +44,73 @@ describe('createWslCliReconciliationStartupBarrier', () => { barrierSettled = true }) - await vi.advanceTimersByTimeAsync(WSL_CLI_RECONCILIATION_STARTUP_BUDGET_MS - 1) + await vi.advanceTimersByTimeAsync(1) expect(barrierSettled).toBe(false) - await vi.advanceTimersByTimeAsync(1) + // Why: a fast WSL discovery failure should release the barrier via its catch + // branch without waiting out the budget, and must clear the pending timer. + rejectReconciliation(new Error('WSL discovery failed')) await expect(barrier).resolves.toBeUndefined() - resolveReconciliation() - await reconciliation + expect(vi.getTimerCount()).toBe(0) } finally { vi.useRealTimers() } }) - it('leaves reconciliation running after the startup budget expires', async () => { + it('lets serve reach RPC readiness at budget while reconciliation remains pending', async () => { vi.useFakeTimers() - let resolveWork!: () => void - let completed = false + let resolveReconciliation!: () => void + let reconciliationCompleted = false try { - const work = new Promise((resolve) => { - resolveWork = resolve + const reconciliation = new Promise((resolve) => { + resolveReconciliation = resolve }).then(() => { - completed = true + reconciliationCompleted = true }) - const barrier = createWslCliReconciliationStartupBarrier(work, { timeoutMs: 10 }) + const barrier = createWslCliReconciliationStartupBarrier(reconciliation) + let rpcReady = false + const serveRpcReadiness = barrier.then(() => { + rpcReady = true + }) + + await vi.advanceTimersByTimeAsync(WSL_CLI_RECONCILIATION_STARTUP_BUDGET_MS - 1) + expect(rpcReady).toBe(false) + expect(reconciliationCompleted).toBe(false) + + await vi.advanceTimersByTimeAsync(1) + await expect(serveRpcReadiness).resolves.toBeUndefined() + expect(rpcReady).toBe(true) + expect(reconciliationCompleted).toBe(false) + + resolveReconciliation() + await reconciliation + expect(reconciliationCompleted).toBe(true) + } finally { + vi.useRealTimers() + } + }) + + it('preserves eventual reconciliation error reporting after the budget expires', async () => { + vi.useFakeTimers() + let rejectReconciliation!: (error: Error) => void + const reportedErrors: string[] = [] + + try { + const reconciliation = new Promise((_resolve, reject) => { + rejectReconciliation = reject + }).catch((error) => { + reportedErrors.push(error instanceof Error ? error.message : String(error)) + }) + const barrier = createWslCliReconciliationStartupBarrier(reconciliation, { timeoutMs: 10 }) await vi.advanceTimersByTimeAsync(10) await expect(barrier).resolves.toBeUndefined() - expect(completed).toBe(false) + expect(reportedErrors).toEqual([]) - resolveWork() - await work - expect(completed).toBe(true) + rejectReconciliation(new Error('WSL discovery failed')) + await reconciliation + expect(reportedErrors).toEqual(['WSL discovery failed']) } finally { vi.useRealTimers() } diff --git a/src/main/startup/wsl-cli-reconciliation-startup-barrier.ts b/src/main/startup/wsl-cli-reconciliation-startup-barrier.ts index 5f70ad7eeb0..056f225b8de 100644 --- a/src/main/startup/wsl-cli-reconciliation-startup-barrier.ts +++ b/src/main/startup/wsl-cli-reconciliation-startup-barrier.ts @@ -5,7 +5,7 @@ type WslCliReconciliationStartupBarrierOptions = { } /** - * Briefly gates restored terminals while managed WSL registrations reconcile. + * Briefly gates terminal startup while managed WSL registrations reconcile. */ export function createWslCliReconciliationStartupBarrier( reconciliation: Promise, @@ -22,8 +22,8 @@ export function createWslCliReconciliationStartupBarrier( } }) - // Why: reconciliation may outlive a slow or unavailable WSL distro; restored - // terminals should wait briefly without turning WSL discovery into an app hang. + // Why: reconciliation may outlive a slow or unavailable WSL distro; terminal + // startup should wait briefly without turning WSL discovery into an app hang. return Promise.race([ settled, new Promise((resolve) => { From 9a77f56671957d145892acf4935ce22d0e413f5f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 13 Jul 2026 12:41:58 +0000 Subject: [PATCH 05/23] release: v1.4.138-rc.7 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a062d03ca1f..dff61b9eed2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "orca", - "version": "1.4.138-rc.6", + "version": "1.4.138-rc.7", "description": "Next-gen IDE for parallel agentic development", "homepage": "https://github.com/stablyai/orca", "author": "stablyai", From 7b3103f8909848a35934c2749607c50f70485963 Mon Sep 17 00:00:00 2001 From: fsdwen Date: Mon, 13 Jul 2026 21:50:18 +0800 Subject: [PATCH 06/23] =?UTF-8?q?fix(i18n):=20correct=20'run'=20translatio?= =?UTF-8?q?ns=20from=20'=E8=B7=91=E6=AD=A5'=20to=20'=E6=89=A7=E8=A1=8C'=20?= =?UTF-8?q?in=20zh.json=20(#8554)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #8553 '跑步' (jogging) was used in computing/automation contexts where the correct term is '执行' (execute). 11 entries fixed across: - TabBarQuickCommandsButton: Run - Scripts settings: When to run - quick.commands.search: run (keyword tag) - ReviewPRViewAnimatedVisual: Running - AutomationRunHistory: Run (column header) - AutomationRunPageFrame: Back to runs - AutomationsPage: Runs, run - ExternalAutomationManagers: run - ExternalAutomationRunTable: Runs, run Note: key e66091daf4 already correctly translated as '运行'. --- src/renderer/src/i18n/locales/zh.json | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index dbf29920404..44e1e44a579 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -2818,7 +2818,7 @@ "e8e1a52edb": "删除“{{value0}}”？", "37e1bb90ce": "运行：{{value0}}", "77ac113df0": "开始 {{value0}}: {{value1}}", - "7b1c9d6ae1": "跑步", + "7b1c9d6ae1": "执行", "c781f992e4": "destructive", "be8f0ff166": "删除", "f3a8c2d1e7": "搜索快捷命令...", @@ -5967,7 +5967,7 @@ "b91a0f297d": "在归档工作树之前运行的本地和共享脚本。", "9a100323ff": "存档脚本", "21fb607a87": "创建新工作树时的默认行为。", - "793dcee97d": "何时跑步", + "793dcee97d": "何时执行", "63e1783173": "当安装脚本可用时选择默认行为。", "fb6bebcf7e": "何时运行安装程序", "30d555acd2": "创建新工作树后运行的本地和共享脚本。", @@ -8028,7 +8028,7 @@ "b86c727100": "新项目管理", "b949a7c0a0": "PNPM", "0b78c4a165": "启动", - "2d8aff42be": "跑步", + "2d8aff42be": "执行", "1c5bdcd0f2": "存储库", "89d2a9ad9f": "仓库", "f58b92a48f": "项目", @@ -10915,7 +10915,7 @@ "ca36f7b27c": "通过", "25f6838e43": "Lint", "2ef0b97954": "类型检查", - "8ed213397c": "跑步", + "8ed213397c": "执行中", "d340c052fb": "核实", "9a097cae12": "1 待定", "2f37142229": "压缩合并", @@ -12343,14 +12343,14 @@ "13988187b3": "token", "86a248187e": "花费", "149c0b49c7": "工作区", - "8faaa00726": "跑步", + "8faaa00726": "执行", "53fc5f07ab": "运行历史", "a00e38d1a3": "不适用", "fdb3caa8fb": "已知的" }, "AutomationRunPageFrame": { "40a511bed4": "运行上下文", - "33741dd973": "返回跑步" + "33741dd973": "返回执行" }, "AutomationSchedulePicker": { "22359b186a": "上午或下午", @@ -12374,7 +12374,7 @@ "2695883141": "删除", "c3a28c9793": "选择一个自动化来查看运行。", "295698292f": "重新运行", - "0e110a3469": "跑步", + "0e110a3469": "执行", "bb1b2cd31e": "概述", "97ff587ee3": "连接此源以检查远程配置文件中的 Hermes 自动化。", "aaa007846f": "来源不可用", @@ -12437,7 +12437,7 @@ "dd0bc7a1ba": "每次运行新的", "7b2e285552": "SSH 连接在此客户端中不可用。", "d441032f7e": "暂停", - "5918020edc": "跑步", + "5918020edc": "执行", "a21f6c33ad": "自动化来源已刷新。", "53f06f0ad5": "重试来源", "pendingAutomationMissing": "自动化已不可用。", @@ -12468,7 +12468,7 @@ "766abf833c": "Hermes", "bf5f67b590": "爱马仕", "e66091daf4": "运行", - "8e9165af08": "跑步", + "8e9165af08": "执行", "2b0adbce21": "已暂停", "b3feba84c7": "当前", "92405f1431": "不可用", @@ -12485,11 +12485,11 @@ "be551397ca": "状态", "a813df9808": "预览", "d4b34feb66": "运行时间", - "2d4388a908": "跑步", + "2d4388a908": "执行", "9c080765ff": "尚未找到 Hermes 运行信息。", "8ea934cacf": "正在加载运行...", "d5527d8fe7": "运行", - "872d032d05": "跑步" + "872d032d05": "执行" }, "HermesCronOutputView": { "e27c716b43": "提示词", From 9a2379211e7bf17b5075a13b352d6deb8c3f113c Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 13 Jul 2026 11:23:28 -0700 Subject: [PATCH 07/23] Search worktrees by cached review metadata (#8505) * feat(renderer): search worktrees by review metadata * fix(renderer): scope review search by execution host --- .../src/components/WorktreeJumpPalette.tsx | 39 +++- .../worktree-checks-review-index.test.ts | 180 +++++++++++++++ .../cmd-j/worktree-checks-review-index.ts | 84 +++++++ .../worktree-palette-cache-inputs.test.ts | 42 ++++ .../cmd-j/worktree-palette-cache-inputs.ts | 29 +++ src/renderer/src/i18n/locales/en.json | 1 + src/renderer/src/i18n/locales/es.json | 1 + src/renderer/src/i18n/locales/ja.json | 1 + src/renderer/src/i18n/locales/ko.json | 1 + src/renderer/src/i18n/locales/zh.json | 1 + .../src/lib/worktree-palette-review-match.ts | 47 ++++ .../src/lib/worktree-palette-search.test.ts | 209 ++++++++++++++++++ .../src/lib/worktree-palette-search.ts | 57 ++--- 13 files changed, 655 insertions(+), 37 deletions(-) create mode 100644 src/renderer/src/components/cmd-j/worktree-checks-review-index.test.ts create mode 100644 src/renderer/src/components/cmd-j/worktree-checks-review-index.ts create mode 100644 src/renderer/src/components/cmd-j/worktree-palette-cache-inputs.test.ts create mode 100644 src/renderer/src/components/cmd-j/worktree-palette-cache-inputs.ts create mode 100644 src/renderer/src/lib/worktree-palette-review-match.ts diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index 8b1ee8fd01d..9b6fcef5b29 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -104,6 +104,9 @@ import { getCmdJQuickActions, CREATE_WORKSPACE_QUICK_ACTION_ID } from '@/components/cmd-j/quick-actions' +import { buildWorktreeChecksReviewIndex } from '@/components/cmd-j/worktree-checks-review-index' +import { selectWorktreePaletteCacheInputs } from '@/components/cmd-j/worktree-palette-cache-inputs' +import { getRepoHostIdentity } from '@/store/slices/repo-host-identity' import { getComposerEligibleRepos, resolveComposerGitRepoId @@ -382,8 +385,9 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { const agentStatusEpoch = useAppStore((s) => visible || statusInputsLingering ? s.agentStatusEpoch : 0 ) - const prCache = useAppStore((s) => s.prCache) - const issueCache = useAppStore((s) => s.issueCache) + const { prCache, issueCache, hostedReviewCache } = useAppStore( + useShallow((s) => selectWorktreePaletteCacheInputs(s, visible || statusInputsLingering)) + ) const migrationUnsupportedByPtyId = useAppStore((s) => s.migrationUnsupportedByPtyId) const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) const activeTabType = useAppStore((s) => s.activeTabType) @@ -439,6 +443,10 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { const preserveCreateLookupOnCloseRef = useRef(false) const repoMap = useMemo(() => new Map(repos.map((r) => [r.id, r])), [repos]) + const repoByHostIdentity = useMemo( + () => new Map(repos.map((repo) => [getRepoHostIdentity(repo), repo])), + [repos] + ) const hostLabelOverrides = useMemo(() => getHostDisplayLabelOverrides(settings), [settings]) // Why: host badges only appear when more than one execution host exists; reuse // the same registry the sidebar host-scope strip builds so labels stay in sync. @@ -622,6 +630,18 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { [browserSortedWorktrees] ) + const checksReviewByWorktree = useMemo( + () => + buildWorktreeChecksReviewIndex({ + worktrees: allWorktrees, + repoByHostIdentity, + prCache, + hostedReviewCache, + settings + }), + [allWorktrees, hostedReviewCache, prCache, repoByHostIdentity, settings] + ) + const worktreeMatches = useMemo( () => searchWorktrees( @@ -630,9 +650,18 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { repoMap, prCache, issueCache, - getWorkspacePortsByWorktreeId(workspacePortScan) + getWorkspacePortsByWorktreeId(workspacePortScan), + checksReviewByWorktree ), - [sortedWorktrees, deferredQuery, repoMap, prCache, issueCache, workspacePortScan] + [ + sortedWorktrees, + deferredQuery, + repoMap, + prCache, + issueCache, + workspacePortScan, + checksReviewByWorktree + ] ) const browserPageEntries = useMemo(() => { @@ -2364,5 +2393,7 @@ function getPaletteSupportingTextLabel( return translate('worktreeJumpPalette.matchLabel.port', 'Port') case 'pr': return translate('worktreeJumpPalette.matchLabel.pr', 'PR') + case 'mr': + return translate('worktreeJumpPalette.matchLabel.mr', 'MR') } } diff --git a/src/renderer/src/components/cmd-j/worktree-checks-review-index.test.ts b/src/renderer/src/components/cmd-j/worktree-checks-review-index.test.ts new file mode 100644 index 00000000000..78889b1d2db --- /dev/null +++ b/src/renderer/src/components/cmd-j/worktree-checks-review-index.test.ts @@ -0,0 +1,180 @@ +import { describe, expect, it } from 'vitest' +import type { HostedReviewInfo } from '../../../../shared/hosted-review' +import type { PRInfo, Repo, Worktree } from '../../../../shared/types' +import { getGitHubPRCacheKey } from '@/store/slices/github-cache-key' +import { getHostedReviewCacheKey } from '@/store/slices/hosted-review-cache-identity' +import { getRepoHostIdentity } from '@/store/slices/repo-host-identity' +import { buildWorktreeChecksReviewIndex } from './worktree-checks-review-index' + +const repo: Repo = { + id: 'repo-1', + path: '/remote/orca', + displayName: 'orca', + badgeColor: '#000000', + addedAt: 0, + executionHostId: 'ssh:staging' +} + +const worktree: Worktree = { + id: 'worktree-1', + repoId: repo.id, + path: '/remote/orca-worktrees/search', + head: 'abc123', + branch: 'refs/heads/feature/search', + isBare: false, + isMainWorktree: false, + displayName: 'Search reviews', + comment: '', + linkedIssue: null, + linkedPR: 42, + linkedLinearIssue: null, + hostId: 'ssh:staging', + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0 +} + +function makePR(): PRInfo { + return { + number: 42, + title: 'Search worktrees by their pull requests', + state: 'open', + url: 'https://github.com/acme/orca/pull/42', + checksStatus: 'success', + updatedAt: '2026-07-12T00:00:00Z', + mergeable: 'MERGEABLE' + } +} + +function makeGitLabReview(): HostedReviewInfo { + return { + provider: 'gitlab', + number: 17, + title: 'Search worktrees by merge request', + state: 'open', + url: 'https://gitlab.com/acme/orca/-/merge_requests/17', + status: 'pending', + updatedAt: '2026-07-12T00:00:00Z', + mergeable: 'UNKNOWN' + } +} + +describe('buildWorktreeChecksReviewIndex', () => { + it('reads the same host-scoped GitHub PR cache entry as Checks', () => { + const key = getGitHubPRCacheKey( + repo.path, + repo.id, + 'feature/search', + null, + repo.connectionId, + repo.executionHostId, + true + ) + + const reviews = buildWorktreeChecksReviewIndex({ + worktrees: [worktree], + repoByHostIdentity: new Map([[getRepoHostIdentity(repo), repo]]), + prCache: { [key]: { data: makePR(), fetchedAt: 1 } }, + hostedReviewCache: {}, + settings: null + }) + + expect(reviews.get(worktree)).toMatchObject({ + provider: 'github', + number: 42, + title: 'Search worktrees by their pull requests' + }) + }) + + it('uses the GitLab review selected by Checks instead of stale GitHub metadata', () => { + const gitLabWorktree = { ...worktree, linkedGitLabMR: 17 } + const prKey = getGitHubPRCacheKey( + repo.path, + repo.id, + 'feature/search', + null, + repo.connectionId, + repo.executionHostId, + true + ) + const reviewKey = getHostedReviewCacheKey( + repo.path, + 'feature/search', + null, + repo.id, + repo.connectionId, + repo.executionHostId, + true + ) + const gitLabReview = makeGitLabReview() + + const reviews = buildWorktreeChecksReviewIndex({ + worktrees: [gitLabWorktree], + repoByHostIdentity: new Map([[getRepoHostIdentity(repo), repo]]), + prCache: { [prKey]: { data: makePR(), fetchedAt: 1 } }, + hostedReviewCache: { [reviewKey]: { data: gitLabReview, fetchedAt: 1 } }, + settings: null + }) + + expect(reviews.get(gitLabWorktree)).toBe(gitLabReview) + }) + + it('records when a non-GitHub link suppresses stale GitHub metadata before its review loads', () => { + const gitLabWorktree = { ...worktree, linkedGitLabMR: 17 } + const prKey = getGitHubPRCacheKey( + repo.path, + repo.id, + 'feature/search', + null, + repo.connectionId, + repo.executionHostId, + true + ) + + const reviews = buildWorktreeChecksReviewIndex({ + worktrees: [gitLabWorktree], + repoByHostIdentity: new Map([[getRepoHostIdentity(repo), repo]]), + prCache: { [prKey]: { data: makePR(), fetchedAt: 1 } }, + hostedReviewCache: {}, + settings: null + }) + + expect(reviews.has(gitLabWorktree)).toBe(true) + expect(reviews.get(gitLabWorktree)).toBeNull() + }) + + it('keeps same-id worktrees isolated across execution hosts', () => { + const localRepo: Repo = { + ...repo, + path: '/local/orca', + executionHostId: 'local' + } + const localWorktree: Worktree = { ...worktree, hostId: 'local' } + const sshWorktree: Worktree = { ...worktree, hostId: 'ssh:staging' } + const sshKey = getGitHubPRCacheKey( + repo.path, + repo.id, + 'feature/search', + null, + repo.connectionId, + repo.executionHostId, + true + ) + + const reviews = buildWorktreeChecksReviewIndex({ + worktrees: [localWorktree, sshWorktree], + repoByHostIdentity: new Map([ + [getRepoHostIdentity(localRepo), localRepo], + [getRepoHostIdentity(repo), repo] + ]), + prCache: { [sshKey]: { data: makePR(), fetchedAt: 1 } }, + hostedReviewCache: {}, + settings: null + }) + + expect(reviews.has(localWorktree)).toBe(false) + expect(reviews.get(sshWorktree)).toMatchObject({ provider: 'github', number: 42 }) + }) +}) diff --git a/src/renderer/src/components/cmd-j/worktree-checks-review-index.ts b/src/renderer/src/components/cmd-j/worktree-checks-review-index.ts new file mode 100644 index 00000000000..507eee5543d --- /dev/null +++ b/src/renderer/src/components/cmd-j/worktree-checks-review-index.ts @@ -0,0 +1,84 @@ +import { branchName } from '@/lib/git-utils' +import { getGitHubPRCacheKey } from '@/store/slices/github-cache-key' +import { getHostedReviewCacheKey } from '@/store/slices/hosted-review-cache-identity' +import { getRepoHostIdentityForParts } from '@/store/slices/repo-host-identity' +import type { AppState } from '@/store/types' +import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host' +import type { HostedReviewInfo } from '../../../../shared/hosted-review' +import type { Repo, Worktree } from '../../../../shared/types' +import { selectChecksPanelReview } from '../right-sidebar/checks-panel-review' + +type WorktreeChecksReviewIndexArgs = { + worktrees: readonly Worktree[] + repoByHostIdentity: ReadonlyMap + prCache: AppState['prCache'] | null + hostedReviewCache: AppState['hostedReviewCache'] | null + settings: AppState['settings'] +} + +export function buildWorktreeChecksReviewIndex({ + worktrees, + repoByHostIdentity, + prCache, + hostedReviewCache, + settings +}: WorktreeChecksReviewIndexArgs): Map { + const reviews = new Map() + if (!prCache || !hostedReviewCache) { + return reviews + } + + for (const worktree of worktrees) { + const repo = repoByHostIdentity.get( + getRepoHostIdentityForParts(worktree.repoId, worktree.hostId ?? LOCAL_EXECUTION_HOST_ID) + ) + if (!repo) { + continue + } + const branch = branchName(worktree.branch) + const prKey = getGitHubPRCacheKey( + repo.path, + repo.id, + branch, + settings, + repo.connectionId, + repo.executionHostId, + true + ) + const hostedReviewKey = getHostedReviewCacheKey( + repo.path, + branch, + settings, + repo.id, + repo.connectionId, + repo.executionHostId, + true + ) + // Why: Cmd+J should expose exactly the review metadata Checks has already + // resolved, without starting another provider lookup from the search path. + const review = selectChecksPanelReview({ + hostedReview: hostedReviewCache[hostedReviewKey]?.data, + pr: prCache[prKey]?.data, + linkedGitLabMR: worktree.linkedGitLabMR ?? null, + linkedBitbucketPR: worktree.linkedBitbucketPR ?? null, + linkedAzureDevOpsPR: worktree.linkedAzureDevOpsPR ?? null, + linkedGiteaPR: worktree.linkedGiteaPR ?? null + }) + if (review) { + // Why: persisted IDs can be identical across execution hosts; the search + // scope preserves these object references while sorting and filtering. + reviews.set(worktree, review) + } else if ( + worktree.linkedGitLabMR != null || + worktree.linkedBitbucketPR != null || + worktree.linkedAzureDevOpsPR != null || + worktree.linkedGiteaPR != null + ) { + // Why: an empty Checks selection for a non-GitHub link is authoritative; + // omitting it would let Cmd+J surface stale GitHub metadata as a fallback. + reviews.set(worktree, null) + } + } + + return reviews +} diff --git a/src/renderer/src/components/cmd-j/worktree-palette-cache-inputs.test.ts b/src/renderer/src/components/cmd-j/worktree-palette-cache-inputs.test.ts new file mode 100644 index 00000000000..2e9254e86bf --- /dev/null +++ b/src/renderer/src/components/cmd-j/worktree-palette-cache-inputs.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from 'vitest' +import type { AppState } from '@/store/types' +import { + EMPTY_WORKTREE_PALETTE_CACHE_INPUTS, + selectWorktreePaletteCacheInputs +} from './worktree-palette-cache-inputs' + +describe('selectWorktreePaletteCacheInputs', () => { + it('stays referentially stable across review cache writes while the palette is closed', () => { + const emptyState: Pick = { + prCache: {}, + issueCache: {}, + hostedReviewCache: {} + } + const filledState: Pick = { + prCache: { pr: { data: null, fetchedAt: 1 } }, + issueCache: { issue: { data: null, fetchedAt: 1 } }, + hostedReviewCache: { review: { data: null, fetchedAt: 1 } } + } + + const before = selectWorktreePaletteCacheInputs(emptyState, false) + const after = selectWorktreePaletteCacheInputs(filledState, false) + + expect(before).toBe(EMPTY_WORKTREE_PALETTE_CACHE_INPUTS) + expect(after).toBe(before) + }) + + it('returns live cache identities while the palette is visible', () => { + const state = { + prCache: {}, + issueCache: {}, + hostedReviewCache: {} + } as Pick + + const selected = selectWorktreePaletteCacheInputs(state, true) + + expect(selected).toEqual(state) + expect(selected.prCache).toBe(state.prCache) + expect(selected.issueCache).toBe(state.issueCache) + expect(selected.hostedReviewCache).toBe(state.hostedReviewCache) + }) +}) diff --git a/src/renderer/src/components/cmd-j/worktree-palette-cache-inputs.ts b/src/renderer/src/components/cmd-j/worktree-palette-cache-inputs.ts new file mode 100644 index 00000000000..1a1c234f12d --- /dev/null +++ b/src/renderer/src/components/cmd-j/worktree-palette-cache-inputs.ts @@ -0,0 +1,29 @@ +import type { AppState } from '@/store/types' + +export type WorktreePaletteCacheInputs = { + prCache: AppState['prCache'] | null + issueCache: AppState['issueCache'] | null + hostedReviewCache: AppState['hostedReviewCache'] | null +} + +export const EMPTY_WORKTREE_PALETTE_CACHE_INPUTS: WorktreePaletteCacheInputs = Object.freeze({ + prCache: null, + issueCache: null, + hostedReviewCache: null +}) + +export function selectWorktreePaletteCacheInputs( + state: Pick, + active: boolean +): WorktreePaletteCacheInputs { + // Why: the palette stays mounted while closed; cache replacement from Checks + // must not rerender it when no search results are visible. + if (!active) { + return EMPTY_WORKTREE_PALETTE_CACHE_INPUTS + } + return { + prCache: state.prCache, + issueCache: state.issueCache, + hostedReviewCache: state.hostedReviewCache + } +} diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 40492be7b71..d6d8725c2d8 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -73,6 +73,7 @@ "matchLabel": { "comment": "Comment", "issue": "Issue", + "mr": "MR", "port": "Port", "pr": "PR" } diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index f909b7687b9..a2bbc411665 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -73,6 +73,7 @@ "matchLabel": { "comment": "Comentario", "issue": "Issue", + "mr": "MR", "port": "Puerto", "pr": "PR" } diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 33bc07fbfd1..8ce317e4ae4 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -73,6 +73,7 @@ "matchLabel": { "comment": "コメント", "issue": "Issue", + "mr": "MR", "port": "ポート", "pr": "PR" } diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 67187a4b21c..37326de084a 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -73,6 +73,7 @@ "matchLabel": { "comment": "댓글", "issue": "이슈", + "mr": "MR", "port": "포트", "pr": "PR" } diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 44e1e44a579..060903dce0e 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -73,6 +73,7 @@ "matchLabel": { "comment": "评论", "issue": "议题", + "mr": "MR", "port": "端口", "pr": "PR" } diff --git a/src/renderer/src/lib/worktree-palette-review-match.ts b/src/renderer/src/lib/worktree-palette-review-match.ts new file mode 100644 index 00000000000..9945722a366 --- /dev/null +++ b/src/renderer/src/lib/worktree-palette-review-match.ts @@ -0,0 +1,47 @@ +import type { HostedReviewInfo } from '../../../shared/hosted-review' + +type SearchableReview = Pick +type WorktreePaletteReviewMatch = { + labelKind: 'pr' | 'mr' + text: string + matchRange: { start: number; end: number } +} + +export function matchWorktreePaletteReview( + review: SearchableReview, + query: string, + numericQuery: string +): WorktreePaletteReviewMatch | null { + const isMergeRequest = review.provider === 'gitlab' + const numberPrefix = isMergeRequest ? 'MR !' : 'PR #' + const hasPullRequestSigil = query.startsWith('#') + const hasMergeRequestSigil = query.startsWith('!') + const sigilMatchesProvider = + (!hasPullRequestSigil && !hasMergeRequestSigil) || + (hasPullRequestSigil && !isMergeRequest) || + (hasMergeRequestSigil && isMergeRequest) + const reviewNumericQuery = hasMergeRequestSigil ? query.slice(1) : numericQuery + const reviewNumberIndex = sigilMatchesProvider + ? String(review.number).indexOf(reviewNumericQuery) + : -1 + if (reviewNumericQuery && reviewNumberIndex !== -1) { + return { + labelKind: isMergeRequest ? 'mr' : 'pr', + text: `${numberPrefix}${review.number}`, + matchRange: { + start: numberPrefix.length + reviewNumberIndex, + end: numberPrefix.length + reviewNumberIndex + reviewNumericQuery.length + } + } + } + + const titleIndex = review.title.toLowerCase().indexOf(query) + if (titleIndex === -1) { + return null + } + return { + labelKind: isMergeRequest ? 'mr' : 'pr', + text: review.title, + matchRange: { start: titleIndex, end: titleIndex + query.length } + } +} diff --git a/src/renderer/src/lib/worktree-palette-search.test.ts b/src/renderer/src/lib/worktree-palette-search.test.ts index ff63dd75ba5..7059ee4ad51 100644 --- a/src/renderer/src/lib/worktree-palette-search.test.ts +++ b/src/renderer/src/lib/worktree-palette-search.test.ts @@ -5,6 +5,7 @@ import { isWorktreePaletteQueryTooLarge } from './worktree-palette-query-bounds' import type { Repo, Worktree } from '../../../shared/types' +import type { HostedReviewInfo } from '../../../shared/hosted-review' function makeWorktree(overrides: Partial = {}): Worktree { return { @@ -172,6 +173,214 @@ describe('worktree-palette-search', () => { }) }) + it('matches the GitLab review title and number already selected by Checks', () => { + const review: HostedReviewInfo = { + provider: 'gitlab', + number: 17, + title: 'Reuse checks tab review metadata', + state: 'open', + url: 'https://gitlab.com/acme/orca/-/merge_requests/17', + status: 'success', + updatedAt: '2026-07-12T00:00:00Z', + mergeable: 'MERGEABLE' + } + const worktree = makeWorktree() + const reviews = new Map([[worktree, review]]) + + const titleResults = searchWorktrees( + [worktree], + 'checks tab', + repoMap, + null, + null, + undefined, + reviews + ) + const numberResults = searchWorktrees( + [worktree], + '!17', + repoMap, + null, + null, + undefined, + reviews + ) + + expect(titleResults[0].supportingText).toEqual({ + labelKind: 'mr', + text: review.title, + matchRange: { start: 6, end: 16 } + }) + expect(numberResults[0].supportingText).toEqual({ + labelKind: 'mr', + text: 'MR !17', + matchRange: { start: 4, end: 6 } + }) + }) + + it('does not search stale GitHub cache metadata when Checks selected another review', () => { + const review: HostedReviewInfo = { + provider: 'gitlab', + number: 17, + title: 'Current merge request', + state: 'open', + url: 'https://gitlab.com/acme/orca/-/merge_requests/17', + status: 'success', + updatedAt: '2026-07-12T00:00:00Z', + mergeable: 'MERGEABLE' + } + const staleWorktree = makeWorktree({ + branch: 'refs/heads/feature/palette-refresh', + linkedPR: 99 + }) + const reviews = new Map([[staleWorktree, review]]) + + expect( + searchWorktrees( + [staleWorktree], + 'stale github title', + repoMap, + { + '/repo/orca::feature/palette-refresh': { + data: { number: 99, title: 'Stale GitHub title' } + } + }, + null, + undefined, + reviews + ) + ).toEqual([]) + + expect( + searchWorktrees([staleWorktree], '#99', repoMap, null, null, undefined, reviews) + ).toEqual([]) + }) + + it('does not search stale GitHub metadata while a linked non-GitHub review is loading', () => { + const stalePRCache = { + '/repo/orca::feature/palette-refresh': { + data: { number: 99, title: 'Stale GitHub title' } + } + } + const staleWorktree = makeWorktree({ + branch: 'refs/heads/feature/palette-refresh', + linkedPR: 99, + linkedGitLabMR: 17 + }) + const authoritativeEmptyReviews = new Map([ + [staleWorktree, null] + ]) + + expect( + searchWorktrees( + [staleWorktree], + 'stale github title', + repoMap, + stalePRCache, + null, + undefined, + authoritativeEmptyReviews + ) + ).toEqual([]) + expect( + searchWorktrees( + [staleWorktree], + '#99', + repoMap, + stalePRCache, + null, + undefined, + authoritativeEmptyReviews + ) + ).toEqual([]) + }) + + it('keeps review matches isolated between same-id worktrees on different hosts', () => { + const localWorktree = makeWorktree({ hostId: 'local' }) + const sshWorktree = makeWorktree({ hostId: 'ssh:staging' }) + const review: HostedReviewInfo = { + provider: 'gitlab', + number: 17, + title: 'Remote-only merge request', + state: 'open', + url: 'https://gitlab.com/acme/orca/-/merge_requests/17', + status: 'success', + updatedAt: '2026-07-12T00:00:00Z', + mergeable: 'MERGEABLE' + } + + const results = searchWorktrees( + [localWorktree, sshWorktree], + 'remote-only', + repoMap, + null, + null, + undefined, + new Map([[sshWorktree, review]]) + ) + + expect(results).toHaveLength(1) + expect(results[0].supportingText?.text).toBe(review.title) + }) + + it('scopes PR and MR number sigils to their providers', () => { + const gitHubReview: HostedReviewInfo = { + provider: 'github', + number: 42, + title: 'GitHub pull request', + state: 'open', + url: 'https://github.com/acme/orca/pull/42', + status: 'success', + updatedAt: '2026-07-12T00:00:00Z', + mergeable: 'MERGEABLE' + } + const gitLabReview: HostedReviewInfo = { + provider: 'gitlab', + number: 17, + title: 'GitLab merge request', + state: 'open', + url: 'https://gitlab.com/acme/orca/-/merge_requests/17', + status: 'success', + updatedAt: '2026-07-12T00:00:00Z', + mergeable: 'MERGEABLE' + } + const gitHubWorktree = makeWorktree() + const gitLabWorktree = makeWorktree() + + expect( + searchWorktrees( + [gitHubWorktree], + '!42', + repoMap, + null, + null, + undefined, + new Map([[gitHubWorktree, gitHubReview]]) + ) + ).toEqual([]) + expect( + searchWorktrees( + [gitLabWorktree], + '#17', + repoMap, + null, + null, + undefined, + new Map([[gitLabWorktree, gitLabReview]]) + ) + ).toEqual([]) + expect( + searchWorktrees( + [makeWorktree({ linkedIssue: 42 })], + '!42', + repoMap, + null, + null, + new Map([['wt-1', [{ port: 42 }]]]) + ) + ).toEqual([]) + }) + it('preserves input order when query matches a repo name', () => { const worktrees = [ makeWorktree({ diff --git a/src/renderer/src/lib/worktree-palette-search.ts b/src/renderer/src/lib/worktree-palette-search.ts index e7a5933050b..f8dce99582b 100644 --- a/src/renderer/src/lib/worktree-palette-search.ts +++ b/src/renderer/src/lib/worktree-palette-search.ts @@ -1,8 +1,10 @@ import { branchName } from '@/lib/git-utils' import { issueCacheKey as getIssueCacheKey } from '@/store/slices/github' +import type { HostedReviewInfo } from '../../../shared/hosted-review' import type { Repo, Worktree } from '../../../shared/types' import { extractWorktreePaletteCommentSnippet } from './worktree-palette-comment-snippet' import { isWorktreePaletteQueryTooLarge } from './worktree-palette-query-bounds' +import { matchWorktreePaletteReview } from './worktree-palette-review-match' export type MatchRange = { start: number; end: number } @@ -16,7 +18,7 @@ export type PaletteMatchedField = | 'port' export type PaletteSupportingText = { - labelKind: 'comment' | 'pr' | 'issue' | 'port' + labelKind: 'comment' | 'pr' | 'mr' | 'issue' | 'port' text: string matchRange: MatchRange | null } @@ -69,7 +71,8 @@ export function searchWorktrees( repoMap: Map, prCache: Record | null, issueCache: Record | null, - workspacePortsByWorktreeId?: Map + workspacePortsByWorktreeId?: Map, + checksReviewByWorktree?: ReadonlyMap ): PaletteSearchResult[] { if (isWorktreePaletteQueryTooLarge(query)) { return [] @@ -199,42 +202,30 @@ export function searchWorktrees( } const repo = repoMap.get(worktree.repoId) + const checksReview = checksReviewByWorktree?.get(worktree) + const hasChecksReviewEntry = checksReview !== undefined + if (checksReview) { + const supportingText = matchWorktreePaletteReview(checksReview, q, numericQuery) + if (supportingText) { + results.push(makeResult(worktree.id, 'pr', { supportingText })) + continue + } + } + const prKey = repo ? `${repo.path}::${branch}` : '' - const pr = prKey && prCache ? prCache[prKey]?.data : undefined + const pr = !hasChecksReviewEntry && prKey && prCache ? prCache[prKey]?.data : undefined if (pr) { - const prText = `PR #${pr.number}` - const prNumberIndex = String(pr.number).indexOf(numericQuery) - if (prNumberIndex !== -1) { - results.push( - makeResult(worktree.id, 'pr', { - supportingText: { - labelKind: 'pr', - text: prText, - matchRange: { - start: 'PR #'.length + prNumberIndex, - end: 'PR #'.length + prNumberIndex + numericQuery.length - } - } - }) - ) + const supportingText = matchWorktreePaletteReview( + { ...pr, provider: 'github' }, + q, + numericQuery + ) + if (supportingText) { + results.push(makeResult(worktree.id, 'pr', { supportingText })) continue } - - const prTitleIndex = pr.title.toLowerCase().indexOf(q) - if (prTitleIndex !== -1) { - results.push( - makeResult(worktree.id, 'pr', { - supportingText: { - labelKind: 'pr', - text: pr.title, - matchRange: { start: prTitleIndex, end: prTitleIndex + q.length } - } - }) - ) - continue - } - } else if (worktree.linkedPR != null) { + } else if (!hasChecksReviewEntry && worktree.linkedPR != null) { const prText = `PR #${worktree.linkedPR}` const prNumberIndex = String(worktree.linkedPR).indexOf(numericQuery) if (prNumberIndex !== -1) { From 527c692b71402d3c26a0c26721d6fa9f8c28e47d Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 13 Jul 2026 12:42:55 -0700 Subject: [PATCH 08/23] Improve git pull on remote (#8524) * Fall back to a merge when a divergent pull has no reconciliation strateg - Git 2.27+ refuses `git pull` on divergent branches unless pull.rebase or pull.ff is configured. Retry with `--no-rebase` (Git's historical default) so pulls succeed out of the box on fresh hosts. - Skip the fallback whenever the caller already specified a reconciliation strategy (e.g. --ff-only, --rebase) so explicit policies still fail as expected on divergence. - Applied identically in the local git pull path and the relay/SSH git handler so both surfaces behave the same way. * Refactor divergent-pull merge fallback into shared helper Extracts the retry-as-merge logic (duplicated between local git and relay SSH pull paths) into `runPullWithDivergenceFallback` in git-remote-error.ts, so both callers share one implementation and test coverage. --- src/main/git/remote.test.ts | 84 ++++++++++++++++++ src/main/git/remote.ts | 17 ++-- .../git-handler-pull-reconciliation.test.ts | 28 +++++- src/relay/git-handler.ts | 52 ++++++----- src/shared/git-remote-error.test.ts | 88 ++++++++++++++++++- src/shared/git-remote-error.ts | 47 ++++++++++ 6 files changed, 284 insertions(+), 32 deletions(-) diff --git a/src/main/git/remote.test.ts b/src/main/git/remote.test.ts index 166db56079d..61b95d0e50d 100644 --- a/src/main/git/remote.test.ts +++ b/src/main/git/remote.test.ts @@ -363,6 +363,90 @@ describe('git remote operations', () => { ]) }) + it('retries a divergent pull as a merge when no strategy is configured', async () => { + const divergentError = new Error( + 'Command failed: git pull\n' + 'fatal: Need to specify how to reconcile divergent branches.' + ) + gitExecFileAsyncMock + // First attempt: plain pull rejects with git's reconciliation error. + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockRejectedValueOnce(divergentError) + // Fallback attempt: pull --no-rebase (merge) succeeds. + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await gitPull('/repo') + + expect(gitExecFileAsyncMock.mock.calls).toEqual([ + [['symbolic-ref', '--quiet', '--short', 'HEAD'], { cwd: '/repo' }], + [['rev-parse', '--abbrev-ref', 'HEAD@{u}'], { cwd: '/repo' }], + [['pull'], { cwd: '/repo' }], + [['symbolic-ref', '--quiet', '--short', 'HEAD'], { cwd: '/repo' }], + [['rev-parse', '--abbrev-ref', 'HEAD@{u}'], { cwd: '/repo' }], + [['pull', '--no-rebase'], { cwd: '/repo' }] + ]) + }) + + it('does not retry a fast-forward-only pull that fails on divergence', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockRejectedValueOnce( + new Error('Command failed: git pull\nfatal: Not possible to fast-forward, aborting.') + ) + + await expect(gitFastForward('/repo')).rejects.toThrow('Not possible to fast-forward') + // No fallback attempt: only the three probe/pull calls ran. + expect(gitExecFileAsyncMock.mock.calls).toHaveLength(3) + }) + + it('retries a divergent pushTarget pull as a merge when no strategy is configured', async () => { + const divergentError = new Error( + 'Command failed: git pull\n' + 'fatal: Need to specify how to reconcile divergent branches.' + ) + gitExecFileAsyncMock + // First attempt: validate the target, then the plain pull rejects. + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockRejectedValueOnce(divergentError) + // Fallback attempt: re-validate, then pull --no-rebase (merge) succeeds. + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await gitPull('/repo', { remoteName: 'fork', branchName: 'feature/fix' }) + + // The merge flag is spliced ahead of the positional remote/branch args. + expect(gitExecFileAsyncMock.mock.calls).toEqual([ + [['check-ref-format', '--branch', 'feature/fix'], { cwd: '/repo' }], + [['pull', 'fork', 'feature/fix'], { cwd: '/repo' }], + [['check-ref-format', '--branch', 'feature/fix'], { cwd: '/repo' }], + [['pull', '--no-rebase', 'fork', 'feature/fix'], { cwd: '/repo' }] + ]) + }) + + it('surfaces a normalized error and does not loop when the merge fallback itself fails', async () => { + const divergentError = new Error( + 'Command failed: git pull\n' + 'fatal: Need to specify how to reconcile divergent branches.' + ) + const mergeConflictError = new Error( + 'Command failed: git pull --no-rebase\nCONFLICT (content): Merge conflict in file.txt' + ) + gitExecFileAsyncMock + // First attempt fails with the reconciliation error. + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockRejectedValueOnce(divergentError) + // The single merge fallback then fails on a real conflict. + .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) + .mockResolvedValueOnce({ stdout: 'origin/feature\n', stderr: '' }) + .mockRejectedValueOnce(mergeConflictError) + + await expect(gitPull('/repo')).rejects.toThrow() + // At-most-once retry: probe+pull, then probe+fallback-pull — no further attempts. + expect(gitExecFileAsyncMock.mock.calls).toHaveLength(6) + }) + it('pulls the same-name origin branch for legacy base-tracking worktrees', async () => { gitExecFileAsyncMock .mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' }) diff --git a/src/main/git/remote.ts b/src/main/git/remote.ts index ab7fba775f9..5c3f40cafe0 100644 --- a/src/main/git/remote.ts +++ b/src/main/git/remote.ts @@ -1,4 +1,7 @@ -import { normalizeGitErrorMessage } from '../../shared/git-remote-error' +import { + normalizeGitErrorMessage, + runPullWithDivergenceFallback +} from '../../shared/git-remote-error' import { resolveEffectiveGitUpstream } from '../../shared/git-effective-upstream' import { gitRefTargetsBranchOnRemote } from '../../shared/git-remote-branch-name' import { resolveGitRemoteRebaseSource } from '../../shared/git-rebase-source' @@ -214,11 +217,11 @@ async function gitPullWithArgs( pushTarget?: GitPushTarget, options: GitRuntimeOptions = {} ): Promise { - try { + const runPull = async (effectiveArgs: string[]): Promise => { if (pushTarget) { const target = await validateGitPushTarget(worktreePath, pushTarget, options) await gitExecFileAsync( - ['pull', ...pullArgs, target.remoteName, target.branchName], + ['pull', ...effectiveArgs, target.remoteName, target.branchName], gitOptionsForWorktree(worktreePath, options) ) return @@ -230,13 +233,17 @@ async function gitPullWithArgs( // Why: legacy Orca branches may still track origin/main while pushes // target origin/. Pull the same effective branch the UI reports. await gitExecFileAsync( - ['pull', ...pullArgs, upstream.remoteName, upstream.branchName], + ['pull', ...effectiveArgs, upstream.remoteName, upstream.branchName], gitOptionsForWorktree(worktreePath, options) ) return } - await gitExecFileAsync(['pull', ...pullArgs], gitOptionsForWorktree(worktreePath, options)) + await gitExecFileAsync(['pull', ...effectiveArgs], gitOptionsForWorktree(worktreePath, options)) + } + + try { + await runPullWithDivergenceFallback(pullArgs, runPull) } catch (error) { throw new Error(normalizeGitErrorMessage(error, 'pull')) } diff --git a/src/relay/git-handler-pull-reconciliation.test.ts b/src/relay/git-handler-pull-reconciliation.test.ts index 1820faa0fec..5db00d16202 100644 --- a/src/relay/git-handler-pull-reconciliation.test.ts +++ b/src/relay/git-handler-pull-reconciliation.test.ts @@ -86,13 +86,33 @@ describe('GitHandler pull reconciliation', () => { return consumerDir } - it('explains how to configure a pull policy when divergent branches have no strategy', async () => { + it('falls back to a merge when divergent branches have no configured strategy', async () => { const consumerDir = createDivergentFixture() - await expect(dispatcher.callRequest('git.pull', { worktreePath: consumerDir })).rejects.toThrow( - 'Pull needs a Git pull policy for divergent branches. Configure one for this repository' - ) + await expect( + dispatcher.callRequest('git.pull', { worktreePath: consumerDir }) + ).resolves.not.toThrow() + // Merge reconciliation yields a two-parent commit and both sides' files. + const parentRefs = execGit(consumerDir, ['log', '-1', '--pretty=%P']).trim().split(/\s+/) + expect(parentRefs).toHaveLength(2) + expect(existsSync(path.join(consumerDir, 'remote.txt'))).toBe(true) + expect(existsSync(path.join(consumerDir, 'local.txt'))).toBe(true) + expect(execGit(consumerDir, ['status', '--short'])).toBe('') + }, 15_000) + + it('preserves configured fast-forward-only pull semantics on divergent branches', async () => { + const consumerDir = createDivergentFixture() + execGit(consumerDir, ['config', 'pull.ff', 'only']) + + // Why: an explicit ff-only policy must still fail on divergence rather than + // getting silently reconciled by the merge fallback. + await expect( + dispatcher.callRequest('git.pull', { worktreePath: consumerDir }) + ).rejects.toThrow() + + const parentRefs = execGit(consumerDir, ['log', '-1', '--pretty=%P']).trim().split(/\s+/) + expect(parentRefs).toHaveLength(1) expect(execGit(consumerDir, ['status', '--short'])).toBe('') }, 15_000) diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index c6709cd3f2a..510dd60f84b 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -44,7 +44,11 @@ import { gitExecMutatesRepository } from '../shared/git-exec-mutation' import { detectConflictOperation, getStatusOp } from './git-handler-status-ops' import { checkIgnoredPathsOp } from './git-handler-check-ignore' import { resolveRelayPushTarget } from './git-handler-push-target' -import { isNoUpstreamError, normalizeGitErrorMessage } from '../shared/git-remote-error' +import { + isNoUpstreamError, + normalizeGitErrorMessage, + runPullWithDivergenceFallback +} from '../shared/git-remote-error' import { upstreamOnlyCommitsArePatchEquivalent } from '../shared/git-upstream-status' import { assertGitPushTargetShape } from '../shared/git-push-target-validation' import { getPublishTargetStatus, type GitCommandRunner } from '../shared/git-publish-target-status' @@ -1021,29 +1025,33 @@ export class GitHandler { private async pullWithArgs(params: Record, pullArgs: string[]) { this.clearGitMutationReadCaches() const worktreePath = params.worktreePath as string + const runPull = async (effectiveArgs: string[]): Promise => { + if (params.pushTarget !== undefined) { + assertGitPushTargetShape(params.pushTarget) + const pushTarget = params.pushTarget as GitPushTarget + await this.git(['check-ref-format', '--branch', pushTarget.branchName], worktreePath) + await this.git( + ['pull', ...effectiveArgs, pushTarget.remoteName, pushTarget.branchName], + worktreePath + ) + return + } + const upstream = await resolveEffectiveGitUpstream((args) => this.git(args, worktreePath)) + if (upstream && !upstream.isConfiguredUpstream) { + // Why: legacy Orca branches may still track origin/main while pushes + // target origin/. Pull the same effective branch the UI reports. + await this.git( + ['pull', ...effectiveArgs, upstream.remoteName, upstream.branchName], + worktreePath + ) + return + } + await this.git(['pull', ...effectiveArgs], worktreePath) + } + try { try { - if (params.pushTarget !== undefined) { - assertGitPushTargetShape(params.pushTarget) - const pushTarget = params.pushTarget as GitPushTarget - await this.git(['check-ref-format', '--branch', pushTarget.branchName], worktreePath) - await this.git( - ['pull', ...pullArgs, pushTarget.remoteName, pushTarget.branchName], - worktreePath - ) - return - } - const upstream = await resolveEffectiveGitUpstream((args) => this.git(args, worktreePath)) - if (upstream && !upstream.isConfiguredUpstream) { - // Why: legacy Orca branches may still track origin/main while pushes - // target origin/. Pull the same effective branch the UI reports. - await this.git( - ['pull', ...pullArgs, upstream.remoteName, upstream.branchName], - worktreePath - ) - return - } - await this.git(['pull', ...pullArgs], worktreePath) + await runPullWithDivergenceFallback(pullArgs, runPull) } catch (error) { // Why: mirror the local gitPull normalization so SSH users see the same // actionable messages instead of raw git stderr. diff --git a/src/shared/git-remote-error.test.ts b/src/shared/git-remote-error.test.ts index d76d386db11..0a1e6be56e6 100644 --- a/src/shared/git-remote-error.test.ts +++ b/src/shared/git-remote-error.test.ts @@ -1,8 +1,12 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { formatSubmodulePushFailureDetail, + isDivergentPullReconciliationError, isNoUpstreamError, - normalizeGitErrorMessage + MERGE_RECONCILIATION_PULL_ARGS, + normalizeGitErrorMessage, + pullArgsSpecifyReconciliation, + runPullWithDivergenceFallback } from './git-remote-error' afterEach(() => { @@ -147,3 +151,85 @@ describe('isNoUpstreamError', () => { expect(isNoUpstreamError(error)).toBe(false) }) }) + +describe('isDivergentPullReconciliationError', () => { + it('detects git 2.27+ divergent-branch reconciliation failures', () => { + const error = new Error( + 'Command failed: git pull\n' + + 'hint: You have divergent branches and need to specify how to reconcile them.\n' + + 'fatal: Need to specify how to reconcile divergent branches.' + ) + + expect(isDivergentPullReconciliationError(error)).toBe(true) + }) + + it('does not match a fast-forward-only abort on divergent branches', () => { + const error = new Error( + 'Command failed: git pull\nfatal: Not possible to fast-forward, aborting.' + ) + + expect(isDivergentPullReconciliationError(error)).toBe(false) + }) + + it('returns false for non-Error values', () => { + expect(isDivergentPullReconciliationError('divergent branches')).toBe(false) + }) +}) + +describe('pullArgsSpecifyReconciliation', () => { + it('is false when no strategy flag is present', () => { + expect(pullArgsSpecifyReconciliation([])).toBe(false) + expect(pullArgsSpecifyReconciliation(['origin', 'main'])).toBe(false) + }) + + it('is true for any explicit reconciliation flag', () => { + expect(pullArgsSpecifyReconciliation(['--ff-only'])).toBe(true) + expect(pullArgsSpecifyReconciliation(['--rebase'])).toBe(true) + expect(pullArgsSpecifyReconciliation(['--no-rebase'])).toBe(true) + expect(pullArgsSpecifyReconciliation(['--rebase=interactive'])).toBe(true) + expect(pullArgsSpecifyReconciliation(['-r'])).toBe(true) + }) +}) + +describe('runPullWithDivergenceFallback', () => { + const divergentError = new Error( + 'Command failed: git pull\n' + + 'hint: You have divergent branches and need to specify how to reconcile them.\n' + + 'fatal: Need to specify how to reconcile divergent branches.' + ) + + it('retries with merge reconciliation args on a policy error', async () => { + const calls: string[][] = [] + const runPull = vi.fn(async (effectiveArgs: string[]) => { + calls.push(effectiveArgs) + if (effectiveArgs.length === 0) { + throw divergentError + } + }) + + await runPullWithDivergenceFallback([], runPull) + + expect(calls).toEqual([[], [...MERGE_RECONCILIATION_PULL_ARGS]]) + expect(runPull).toHaveBeenCalledTimes(2) + }) + + it('does not retry when pull args already specify reconciliation', async () => { + const runPull = vi.fn(async () => { + throw divergentError + }) + + await expect(runPullWithDivergenceFallback(['--ff-only'], runPull)).rejects.toBe(divergentError) + expect(runPull).toHaveBeenCalledTimes(1) + expect(runPull).toHaveBeenCalledWith(['--ff-only']) + }) + + it('rethrows non-divergence errors without retrying', async () => { + const otherError = new Error('fatal: Not possible to fast-forward, aborting.') + const runPull = vi.fn(async () => { + throw otherError + }) + + await expect(runPullWithDivergenceFallback([], runPull)).rejects.toBe(otherError) + expect(runPull).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/shared/git-remote-error.ts b/src/shared/git-remote-error.ts index 42cf210292f..d2171f8a6a2 100644 --- a/src/shared/git-remote-error.ts +++ b/src/shared/git-remote-error.ts @@ -20,6 +20,15 @@ const NORMALIZED_SUBMODULE_PUSH_FAILURE_PATTERN = /(?:^|:\s)((?:Submodule '[^'\n]+'|A submodule) (?:has remote changes\. Pull inside the submodule, then try again\.|could not be pushed\. Resolve the submodule push error, then try again\.))(?:$|\s)/i const DIVERGENT_PULL_RECONCILIATION_PATTERN = /Need to specify how to reconcile divergent branches|divergent branches and need to specify how to reconcile them/i +// Why: any of these already pin a reconciliation strategy, so the merge +// fallback must not override an explicit caller/user choice (e.g. --ff-only). +const RECONCILIATION_PULL_ARG_PATTERN = + /^(--rebase|--no-rebase|--ff-only|--ff|--no-ff|--merge|-r)(=|$)/ +// Why: merge is Git's historical pre-2.27 default. Falling back to it lets +// divergent pulls reconcile on fresh hosts that never configured pull.rebase +// or pull.ff, instead of failing outright. `--no-rebase` predates the 2.25 +// baseline, so it is safe across all supported Git binaries. +export const MERGE_RECONCILIATION_PULL_ARGS = ['--no-rebase'] export function stripCredentialsFromMessage(message: string): string { return message.replace(USERPASS_URL_PATTERN, '$1').replace(HTTPS_TOKEN_URL_PATTERN, '$1') @@ -81,6 +90,44 @@ function* iterateLinesFromEnd(value: string): Generator { yield value.slice(0, lineEnd) } +// Why: a fresh host may lack any pull.rebase/pull.ff policy, so Git 2.27+ +// refuses to reconcile divergent branches. Detect that specific failure so the +// caller can retry with an explicit merge instead of surfacing a config error. +export function isDivergentPullReconciliationError(error: unknown): boolean { + if (!(error instanceof Error)) { + return false + } + return DIVERGENT_PULL_RECONCILIATION_PATTERN.test(stripCredentialsFromMessage(error.message)) +} + +// Whether the pull already specifies how to reconcile (rebase/merge/ff-only), +// in which case the caller's choice must win over the merge fallback. +export function pullArgsSpecifyReconciliation(pullArgs: string[]): boolean { + return pullArgs.some((arg) => RECONCILIATION_PULL_ARG_PATTERN.test(arg)) +} + +// Why: on hosts with no pull.rebase/pull.ff policy, Git 2.27+ refuses to +// reconcile divergent branches. Retry as a merge (Git's historical default) so +// pulls succeed out of the box; callers that already forced a strategy — or +// users who configured rebase — never reach this fallback. +// Not routed through GitCapabilityCache: this is per-repo config/branch state, +// not a stable host capability, and only fires on an actual divergence error, +// so there is nothing host-scoped to cache. +export async function runPullWithDivergenceFallback( + pullArgs: string[], + runPull: (effectiveArgs: string[]) => Promise +): Promise { + try { + await runPull(pullArgs) + } catch (error) { + if (!pullArgsSpecifyReconciliation(pullArgs) && isDivergentPullReconciliationError(error)) { + await runPull([...MERGE_RECONCILIATION_PULL_ARGS, ...pullArgs]) + return + } + throw error + } +} + export type GitRemoteOperation = 'push' | 'pull' | 'fetch' | 'upstream' export function normalizeGitErrorMessage(error: unknown, operation?: GitRemoteOperation): string { From dc4fb2aa0350f654f42e629d4b94d35ea484d002 Mon Sep 17 00:00:00 2001 From: Kaynan Sampaio de Camargo <33468632+kaynansc@users.noreply.github.com> Date: Mon, 13 Jul 2026 12:45:46 -0700 Subject: [PATCH 09/23] fix(native-chat): retry not-yet-flushed transcripts instead of settling into a permanent error (#8418) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(native-chat): retry not-yet-flushed transcripts instead of settling into a permanent error A freshly-created session's transcript .jsonl lands on disk seconds to minutes after the process starts. Native chat's one-shot read raced that first flush: a miss became a permanent "No transcript found" error and the live-tail subscription silently degraded to a no-op, so the pane never recovered even after the file appeared. - transcript-reader/read-cache: mark the miss with notFound so callers can tell "not flushed yet" from a real parse/IO error (never cached). - transcript-watch: poll resolve+install (500ms backoff, 5s cap) for the subscription's lifetime instead of returning a dead no-op watcher. - use-native-chat-live-session: retry a notFound read with backoff for up to 60s while staying in the loading state, and let live appends render over a stale initial-read error. Fixes #8401 Claude-Session: https://claude.ai/code/session_01HA5g3X7wCakBttBpDru9Fp * fix(native-chat): address CodeRabbit review — ENOENT retryable, content over spinner, blank-id guard, unref poll timer - transcript-reader: an ENOENT after a successful resolve is the same first-flush/rotation race as an unresolved path — mark it notFound. - use-native-chat-live-session: live appends landing mid-retry render instead of the loading state (mirrors the stale-error gate). - transcript-watch: bail out for a blank session id with no explicit file (nothing to resolve-poll), and unref the poll timer so headless serve shutdown is never held open by an unresolvable session. Claude-Session: https://claude.ai/code/session_01HA5g3X7wCakBttBpDru9Fp --------- Co-authored-by: kaynan --- .../native-chat/transcript-read-cache.test.ts | 13 ++ src/main/native-chat/transcript-read-cache.ts | 4 +- .../native-chat/transcript-reader.test.ts | 26 ++- src/main/native-chat/transcript-reader.ts | 13 +- src/main/native-chat/transcript-watch.test.ts | 82 +++++++++ src/main/native-chat/transcript-watch.ts | 173 +++++++++++++++--- src/preload/api-types.ts | 6 +- .../use-native-chat-live-session.test.ts | 126 +++++++++++++ .../use-native-chat-live-session.ts | 92 +++++++--- .../e2e/native-chat-first-flush-race.spec.ts | 165 +++++++++++++++++ 10 files changed, 649 insertions(+), 51 deletions(-) create mode 100644 tests/e2e/native-chat-first-flush-race.spec.ts diff --git a/src/main/native-chat/transcript-read-cache.test.ts b/src/main/native-chat/transcript-read-cache.test.ts index 6f0fc39acb6..0d61f25f48e 100644 --- a/src/main/native-chat/transcript-read-cache.test.ts +++ b/src/main/native-chat/transcript-read-cache.test.ts @@ -110,6 +110,19 @@ describe('readNativeChatTranscriptCached', () => { expect('error' in result && result.error).toBeTruthy() }) + // Why: a just-created session's transcript can take up to minutes to exist on + // disk (#8401) — the miss must be marked notFound so watch/renderer callers + // retry instead of settling into a permanent error, and it must never be + // cached (a real error already isn't cached; this locks in the same for a miss). + it('marks a resolve miss as notFound and does not cache it', async () => { + await seedSession('present-2', 1) + const first = await readNativeChatTranscriptCached('claude', 'absent-2') + expect('error' in first && first.notFound).toBe(true) + expect(readSpy).not.toHaveBeenCalled() + const second = await readNativeChatTranscriptCached('claude', 'absent-2') + expect(second).not.toBe(first) + }) + // Why: two worktrees can present the SAME (agent, sessionId) via different // transcript files — e.g. the same session resumed into a second worktree, // which writes a new transcript file. Keying the cache by sessionId let one diff --git a/src/main/native-chat/transcript-read-cache.ts b/src/main/native-chat/transcript-read-cache.ts index 9f4df5bde87..8943c223dd2 100644 --- a/src/main/native-chat/transcript-read-cache.ts +++ b/src/main/native-chat/transcript-read-cache.ts @@ -91,7 +91,9 @@ export async function readNativeChatTranscriptCached( ): Promise { const filePath = await resolveSessionFilePath(agent, sessionId, { transcriptPath }) if (!filePath) { - return { error: `No transcript found for ${agent} session ${sessionId}` } + // Not cached (see below): a not-yet-flushed transcript should be re-checked + // on the next call, not pinned as a settled miss (#8401). + return { error: `No transcript found for ${agent} session ${sessionId}`, notFound: true } } const key = cacheKey(agent, filePath) diff --git a/src/main/native-chat/transcript-reader.test.ts b/src/main/native-chat/transcript-reader.test.ts index c6c2223c81f..cd87f2bdd59 100644 --- a/src/main/native-chat/transcript-reader.test.ts +++ b/src/main/native-chat/transcript-reader.test.ts @@ -242,19 +242,41 @@ describe('readNativeChatTranscript (codex)', () => { }) describe('readNativeChatTranscript (errors)', () => { - it('returns an error for an unreadable/missing file without throwing', async () => { + // Why: ENOENT after a successful resolve is the same first-flush/rotation + // race as an unresolved path (#8401) — it must stay retry-worthy. + it('marks an ENOENT on a directly-passed path as notFound (vanished after resolve)', async () => { const result = await readNativeChatTranscript('claude', 'sess', { filePath: join(tmpdir(), 'orca-native-chat-does-not-exist.jsonl') }) expect('error' in result).toBe(true) + if ('error' in result) { + expect(result.notFound).toBe(true) + } }) - it('returns an error when no transcript can be resolved', async () => { + it('returns a real read error (no notFound) when the path exists but is unreadable', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-native-chat-unreadable-')) + tempRoots.push(root) + // A directory instead of a file fails the read with a non-ENOENT error. + const result = await readNativeChatTranscript('claude', 'sess', { filePath: root }) + expect('error' in result).toBe(true) + if ('error' in result) { + expect(result.notFound).toBeUndefined() + } + }) + + // Why: a just-created Claude Code session's transcript can take up to minutes + // to exist on disk (#8401) — the miss must be marked retry-worthy so callers + // above (cache, watch, renderer) don't settle into a permanent error. + it('marks an unresolved session as notFound so callers know to retry', async () => { const root = await mkdtemp(join(tmpdir(), 'orca-native-chat-noresolve-')) tempRoots.push(root) const result = await readNativeChatTranscript('claude', 'missing', { claudeProjectsDir: join(root, 'empty') }) expect('error' in result).toBe(true) + if ('error' in result) { + expect(result.notFound).toBe(true) + } }) }) diff --git a/src/main/native-chat/transcript-reader.ts b/src/main/native-chat/transcript-reader.ts index 1dbbd7dd82a..e2ea727c809 100644 --- a/src/main/native-chat/transcript-reader.ts +++ b/src/main/native-chat/transcript-reader.ts @@ -9,7 +9,11 @@ import { } from './transcript-line-decoders' import { decodeTranscriptStream } from './transcript-stream-lines' -export type ReadTranscriptResult = { messages: NativeChatMessage[] } | { error: string } +export type ReadTranscriptResult = + | { messages: NativeChatMessage[] } + // notFound marks a retry-worthy miss (transcript not flushed to disk yet, + // #8401) as opposed to a real parse/IO error callers surface immediately. + | { error: string; notFound?: true } export type ReadTranscriptOptions = ResolveSessionFileOptions & { /** Resolve directly to this file, skipping path discovery (used by tests). */ @@ -30,7 +34,7 @@ export async function readNativeChatTranscript( ): Promise { const filePath = options.filePath ?? (await resolveSessionFilePath(agent, sessionId, options)) if (!filePath) { - return { error: `No transcript found for ${agent} session ${sessionId}` } + return { error: `No transcript found for ${agent} session ${sessionId}`, notFound: true } } try { if (agent === 'claude') { @@ -44,6 +48,11 @@ export async function readNativeChatTranscript( } return { error: `Unsupported agent for native chat transcript: ${agent}` } } catch (err) { + // Why: ENOENT after a successful resolve is the same first-flush/rotation + // race as an unresolved path — keep it retry-worthy (#8401). + if ((err as NodeJS.ErrnoException | null)?.code === 'ENOENT') { + return { error: errorMessage(err), notFound: true } + } return { error: errorMessage(err) } } } diff --git a/src/main/native-chat/transcript-watch.test.ts b/src/main/native-chat/transcript-watch.test.ts index 55009521a99..4ff621d05bc 100644 --- a/src/main/native-chat/transcript-watch.test.ts +++ b/src/main/native-chat/transcript-watch.test.ts @@ -24,6 +24,14 @@ async function tempFile(initial: string): Promise { return filePath } +// A path inside a fresh temp dir with nothing written yet — simulates a +// just-created session whose agent hasn't flushed its first JSONL line (#8401). +async function pendingFilePath(): Promise { + const root = await mkdtemp(join(tmpdir(), 'orca-native-chat-watch-pending-')) + tempRoots.push(root) + return join(root, 'rollout.jsonl') +} + function claudeLine(uuid: string, role: 'user' | 'assistant', text: string): string { return `${JSON.stringify({ type: role, @@ -237,3 +245,77 @@ describe('subscribeNativeChatTranscript', () => { sub.unsubscribe() }) }) + +// Regression for #8401: Claude Code (and other agents) can take from ~3s to +// minutes to flush a brand-new session's first JSONL line, so the file +// genuinely doesn't exist when native chat subscribes. Before this fix, +// subscribeNativeChatTranscript returned a permanent no-op the instant the +// file was missing and never recovered once it appeared. +describe('subscribeNativeChatTranscript (resolve-poll for a not-yet-created file, #8401)', () => { + it('keeps retrying resolve+install and tails the file once it is created', async () => { + const filePath = await pendingFilePath() + const seen: NativeChatMessage[] = [] + + const sub = await subscribeNativeChatTranscript({ + agent: 'claude', + sessionId: 'ignored', + filePath, + onAppend: (messages) => seen.push(...messages), + debounceMs: 5, + resolvePollIntervalMs: 20 + }) + + // Nothing installed yet — the file doesn't exist on disk. + expect(getActiveNativeChatWatcherCount()).toBe(0) + + // The agent flushes its first turn well after subscribe. + await new Promise((resolve) => setTimeout(resolve, 50)) + await writeFile(filePath, claudeLine('u-1', 'user', 'hello')) + + await waitFor(() => seen.some((m) => m.id === 'u-1')) + expect(getActiveNativeChatWatcherCount()).toBe(1) + + sub.unsubscribe() + expect(getActiveNativeChatWatcherCount()).toBe(0) + }) + + it('returns a no-op (no resolve poll) for a blank session id with no explicit file', async () => { + const before = getActiveNativeChatWatcherCount() + const sub = await subscribeNativeChatTranscript({ + agent: 'claude', + sessionId: ' ', + onAppend: () => {}, + debounceMs: 5, + resolvePollIntervalMs: 10 + }) + + // An unresolvable target must not spin the resolve poll forever. + await new Promise((resolve) => setTimeout(resolve, 60)) + expect(getActiveNativeChatWatcherCount()).toBe(before) + sub.unsubscribe() + sub.unsubscribe() + expect(getActiveNativeChatWatcherCount()).toBe(before) + }) + + it('unsubscribing during the poll phase leaves no watcher or timer alive', async () => { + const filePath = await pendingFilePath() + const before = getActiveNativeChatWatcherCount() + + const sub = await subscribeNativeChatTranscript({ + agent: 'claude', + sessionId: 'ignored', + filePath, + onAppend: () => {}, + debounceMs: 5, + resolvePollIntervalMs: 20 + }) + + // Unsubscribe while still polling — the file is never created in this test. + sub.unsubscribe() + expect(getActiveNativeChatWatcherCount()).toBe(before) + + // Give any stray timer a chance to fire; it must not install a watcher. + await new Promise((resolve) => setTimeout(resolve, 100)) + expect(getActiveNativeChatWatcherCount()).toBe(before) + }) +}) diff --git a/src/main/native-chat/transcript-watch.ts b/src/main/native-chat/transcript-watch.ts index 4bbd934f0eb..1f55c33124a 100644 --- a/src/main/native-chat/transcript-watch.ts +++ b/src/main/native-chat/transcript-watch.ts @@ -29,6 +29,10 @@ export type SubscribeNativeChatTranscriptArgs = ResolveSessionFileOptions & { filePath?: string /** Coalesce window for rapid fs.watch events (ms). Defaults to 40ms. */ debounceMs?: number + /** Overrides the resolve-poll interval (see subscribeViaResolvePoll) so tests + * don't wait out the production backoff. Production ignores this and backs + * off from 500ms to a 5s cap. */ + resolvePollIntervalMs?: number } export type NativeChatTranscriptSubscription = { @@ -115,26 +119,17 @@ async function readAppendedMessages( } /** - * Subscribe to live appends on an agent's transcript file. Returns an - * unsubscribe fn that tears the watcher down completely. - * - * Handles file rotation/replacement: when the file shrinks (a new session id - * resolved to a smaller/newer file, or the file was truncated), the offset is - * reset to 0 so the replacement's content is read from the top. + * Install a live-tail watcher on an already-resolved file path. Returns null + * (rather than throwing) when `watch()` fails — e.g. the file vanished between + * resolve and install — so the caller can fall back to the resolve-poll below + * instead of surfacing a hard error. */ -export async function subscribeNativeChatTranscript( - args: SubscribeNativeChatTranscriptArgs -): Promise { - const { agent, sessionId, onAppend, debounceMs } = args - const decode = lineDecoderForAgent(agent) - const filePath = args.filePath ?? (await resolveSessionFilePath(agent, sessionId, args)) - - if (!filePath || !decode) { - // Nothing watchable — return a no-op teardown so callers can unconditionally - // unsubscribe without null-checks. - return { unsubscribe: () => {} } - } - +function installTranscriptWatcher( + filePath: string, + decode: (line: string, fallbackId: string) => NativeChatMessage | null, + onAppend: (messages: NativeChatMessage[]) => void, + debounceMs?: number +): NativeChatTranscriptSubscription | null { // Why: seed the offset at 0 so the FIRST drain re-reads the whole file. This // closes the read/subscribe race — a turn appended between the caller's // readSession EOF and the watcher install is still emitted. Re-emitted lines @@ -161,12 +156,12 @@ export async function subscribeNativeChatTranscript( do { pendingReadRequested = false try { - const currentSize = await fileSize(filePath!) + const currentSize = await fileSize(filePath) if (currentSize < offset) { // Rotation/replacement/truncation: re-read from the top. offset = 0 } - const { messages, consumedTo } = await readAppendedMessages(filePath!, offset, decode!) + const { messages, consumedTo } = await readAppendedMessages(filePath, offset, decode) offset = consumedTo if (!closed && messages.length > 0) { onAppend(messages) @@ -200,8 +195,8 @@ export async function subscribeNativeChatTranscript( try { watcher = watch(filePath, scheduleDrain) } catch { - // File vanished between resolve and watch — return a no-op teardown. - return { unsubscribe: () => {} } + // File vanished between resolve and watch. + return null } activeWatcherCount++ @@ -225,3 +220,135 @@ export async function subscribeNativeChatTranscript( } } } + +/** One resolve+install attempt. Returns null when the file isn't resolvable + * yet, or vanished between resolve and `watch()` — either case is retried by + * the resolve-poll rather than treated as a hard failure. */ +async function attemptInstall( + args: SubscribeNativeChatTranscriptArgs, + decode: (line: string, fallbackId: string) => NativeChatMessage | null +): Promise { + const filePath = args.filePath ?? (await resolveSessionFilePath(args.agent, args.sessionId, args)) + if (!filePath) { + return null + } + return installTranscriptWatcher(filePath, decode, args.onAppend, args.debounceMs) +} + +// Why: Claude Code (and other agents) can take from ~3s to minutes to flush a +// brand-new session's first JSONL line (#8401) — resolveSessionFilePath +// genuinely has nothing to find yet. Poll for it instead of going deaf; each +// attempt is a cheap glob, so the cost of polling is negligible next to the +// gap it covers. +const INITIAL_RESOLVE_POLL_MS = 500 +const MAX_RESOLVE_POLL_MS = 5_000 + +/** + * Background retry loop for a transcript that hasn't been resolvable yet. + * Returns a subscription immediately (per subscribeNativeChatTranscript's + * contract); the loop keeps retrying resolve+install until it succeeds or + * unsubscribe() cancels it. + */ +function subscribeViaResolvePoll( + args: SubscribeNativeChatTranscriptArgs, + decode: (line: string, fallbackId: string) => NativeChatMessage | null +): NativeChatTranscriptSubscription { + let closed = false + let installed: NativeChatTranscriptSubscription | null = null + let pollTimer: ReturnType | null = null + let delay = args.resolvePollIntervalMs ?? INITIAL_RESOLVE_POLL_MS + + function scheduleAttempt(): void { + if (closed) { + return + } + pollTimer = setTimeout(() => { + pollTimer = null + void runAttempt() + }, delay) + // Why: never hold the event loop open (headless `orca serve` shutdown) for + // a session that may genuinely never resolve. + pollTimer.unref?.() + // Only back off in production; a test-supplied interval stays fixed so + // tests resolve in bounded, predictable time. + if (args.resolvePollIntervalMs === undefined) { + delay = Math.min(delay * 2, MAX_RESOLVE_POLL_MS) + } + } + + async function runAttempt(): Promise { + if (closed) { + return + } + let result: NativeChatTranscriptSubscription | null + try { + result = await attemptInstall(args, decode) + } catch { + // Why: a transient resolve failure (EACCES/EIO during the glob) must not + // kill the poll loop with an unhandled rejection — retry like a miss. + result = null + } + if (closed) { + // unsubscribe() ran while this attempt was in flight. + result?.unsubscribe() + return + } + if (result) { + installed = result + return + } + scheduleAttempt() + } + + scheduleAttempt() + + return { + unsubscribe: () => { + if (closed) { + return + } + closed = true + if (pollTimer) { + clearTimeout(pollTimer) + pollTimer = null + } + installed?.unsubscribe() + installed = null + } + } +} + +/** + * Subscribe to live appends on an agent's transcript file. Returns an + * unsubscribe fn that tears the watcher down completely. + * + * Handles file rotation/replacement: when the file shrinks (a new session id + * resolved to a smaller/newer file, or the file was truncated), the offset is + * reset to 0 so the replacement's content is read from the top. + * + * When the transcript isn't resolvable yet (a just-created session whose + * agent hasn't flushed its first JSONL line, #8401), returns the subscription + * immediately and keeps retrying resolve+install in the background rather + * than returning a no-op that never recovers. + */ +export async function subscribeNativeChatTranscript( + args: SubscribeNativeChatTranscriptArgs +): Promise { + const decode = lineDecoderForAgent(args.agent) + if (!decode) { + // Nothing watchable — return a no-op teardown so callers can unconditionally + // unsubscribe without null-checks. + return { unsubscribe: () => {} } + } + // Why: a blank session id (and no explicit file) can never resolve — bail out + // instead of resolve-polling an unresolvable target forever. + if (!args.filePath && !args.sessionId.trim()) { + return { unsubscribe: () => {} } + } + + const installed = await attemptInstall(args, decode) + if (installed) { + return installed + } + return subscribeViaResolvePoll(args, decode) +} diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 5784d5f77c7..599c1a8f81b 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -813,7 +813,11 @@ export type AiVaultApi = { onWindowFocused: (callback: () => void) => () => void } -export type NativeChatReadSessionResult = { messages: NativeChatMessage[] } | { error: string } +// notFound marks a miss caused by the transcript not existing on disk yet +// (retry-worthy), as opposed to a real read/parse error (#8401). +export type NativeChatReadSessionResult = + | { messages: NativeChatMessage[] } + | { error: string; notFound?: true } /** Messages appended to a live-tailed transcript since the previous emit. */ export type NativeChatAppendedMessages = NativeChatMessage[] diff --git a/src/renderer/src/components/native-chat/use-native-chat-live-session.test.ts b/src/renderer/src/components/native-chat/use-native-chat-live-session.test.ts index c2b6e5d7e1b..f60ce34d3c8 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-live-session.test.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-live-session.test.ts @@ -293,3 +293,129 @@ describe('useNativeChatLiveSession — transport routing', () => { expect(latest?.messages.map((m) => m.id)).not.toContain('stale') }) }) + +// Regression for #8401: a just-created Claude Code session's transcript can +// take up to minutes to exist on disk, so the first readSession commonly +// misses. Before this fix, the hook settled into a permanent 'error' phase +// on that first miss and never recovered. +describe('useNativeChatLiveSession — notFound retry (#8401)', () => { + const AGENT = 'claude' as const + const SESSION = 'sess-notfound' + const PANE = 'pane-notfound' + const roots: Root[] = [] + let latest: NativeChatLiveSession | null = null + + function Probe(props: UseNativeChatLiveSessionArgs): null { + latest = useNativeChatLiveSession(props) + return null + } + + async function render(props: UseNativeChatLiveSessionArgs): Promise { + const container = document.createElement('div') + const root = createRoot(container) + roots.push(root) + await act(async () => { + root.render(createElement(Probe, props)) + await Promise.resolve() + await Promise.resolve() + }) + return root + } + + beforeEach(() => { + useAppStore.setState({ agentStatusByPaneKey: {} }) + }) + + afterEach(() => { + for (const root of roots.splice(0)) { + act(() => root.unmount()) + } + latest = null + vi.clearAllMocks() + resetMockTransports() + vi.useRealTimers() + }) + + it('retries a notFound miss with backoff and settles into ready without ever exposing an error', async () => { + vi.useFakeTimers() + const transport = getMockTransport('env-1') + transport.readSession + .mockResolvedValueOnce({ error: 'No transcript found', notFound: true }) + .mockResolvedValueOnce({ messages: [assistant('a-1', 'hello')] }) + + await render({ paneKey: PANE, agent: AGENT, sessionId: SESSION, runtimeEnvironmentId: 'env-1' }) + expect(latest?.status).toBe('loading') + + // First backoff step (1s) fires the second readSession, which resolves. + await act(async () => { + await vi.advanceTimersByTimeAsync(1_000) + }) + + expect(latest?.status).not.toBe('error') + expect(transport.readSession).toHaveBeenCalledTimes(2) + expect(latest?.messages.map((m) => m.id)).toContain('a-1') + }) + + it('surfaces an error once the ~60s retry window is exhausted', async () => { + vi.useFakeTimers() + const transport = getMockTransport('env-1') + transport.readSession.mockResolvedValue({ error: 'No transcript found', notFound: true }) + + await render({ paneKey: PANE, agent: AGENT, sessionId: SESSION, runtimeEnvironmentId: 'env-1' }) + expect(latest?.status).toBe('loading') + + await act(async () => { + await vi.advanceTimersByTimeAsync(70_000) + }) + + expect(latest?.status).toBe('error') + expect(latest?.error).toBe('No transcript found') + }) + + it('renders live-appended content instead of loading while the read is still retrying', async () => { + vi.useFakeTimers() + const transport = getMockTransport('env-1') + transport.readSession.mockResolvedValue({ error: 'No transcript found', notFound: true }) + let onAppended: ((messages: NativeChatMessage[]) => void) | null = null + transport.subscribe.mockImplementationOnce( + (_args: unknown, cb: (m: NativeChatMessage[]) => void) => { + onAppended = cb + return transport.unsubscribe + } + ) + + await render({ paneKey: PANE, agent: AGENT, sessionId: SESSION, runtimeEnvironmentId: 'env-1' }) + expect(latest?.status).toBe('loading') + + // The watcher's first drain lands mid-retry — content must win over the spinner. + await act(async () => { + onAppended?.([assistant('a-early', 'landed during retry')]) + }) + + expect(latest?.status).not.toBe('loading') + expect(latest?.messages.map((m) => m.id)).toContain('a-early') + }) + + it('renders live-appended content even when the initial read settled into a permanent error', async () => { + const transport = getMockTransport('env-1') + transport.readSession.mockResolvedValueOnce({ error: 'unreadable transcript' }) + let onAppended: ((messages: NativeChatMessage[]) => void) | null = null + transport.subscribe.mockImplementationOnce( + (_args: unknown, cb: (m: NativeChatMessage[]) => void) => { + onAppended = cb + return transport.unsubscribe + } + ) + + await render({ paneKey: PANE, agent: AGENT, sessionId: SESSION, runtimeEnvironmentId: 'env-1' }) + expect(latest?.status).toBe('error') + + await act(async () => { + onAppended?.([assistant('a-late', 'landed late')]) + }) + + expect(latest?.status).not.toBe('error') + expect(latest?.error).toBeUndefined() + expect(latest?.messages.map((m) => m.id)).toContain('a-late') + }) +}) diff --git a/src/renderer/src/components/native-chat/use-native-chat-live-session.ts b/src/renderer/src/components/native-chat/use-native-chat-live-session.ts index 8832b6e1966..384ae724769 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-live-session.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-live-session.ts @@ -76,6 +76,17 @@ function nextSubscriptionId(): string { return `native-chat-${subscriptionCounter}-${Date.now()}` } +// Why: a brand-new session's transcript can take seconds to minutes to appear +// on disk (#8401), so a `notFound` miss retries — 1s/2s/4s/8s then every 10s — +// until the window below elapses. +const NOTFOUND_RETRY_DELAYS_MS = [1_000, 2_000, 4_000, 8_000] +const NOTFOUND_RETRY_FIXED_DELAY_MS = 10_000 +const NOTFOUND_RETRY_WINDOW_MS = 60_000 + +function notFoundRetryDelayMs(attempt: number): number { + return NOTFOUND_RETRY_DELAYS_MS[attempt] ?? NOTFOUND_RETRY_FIXED_DELAY_MS +} + type ReadState = | { phase: 'loading' } | { phase: 'ready'; messages: NativeChatMessage[] } @@ -161,31 +172,50 @@ export function useNativeChatLiveSession( } let cancelled = false + let retryTimer: ReturnType | null = null + const retryStartedAt = Date.now() + // Re-bound as a plain const: TS doesn't retain the `!sessionId` narrowing + // above inside a nested function declaration (it's hoisted, so the + // narrowing can't be proven to hold at every call site). + const activeSessionId = sessionId limitRef.current = NATIVE_CHAT_INITIAL_LIMIT setRead({ phase: 'loading' }) replaceList(appendMergerRef.current, []) setAppended([]) setHasMore(false) - void transport - .readSession(agent, sessionId, limitRef.current, transcriptPath ?? undefined) - .then((result) => { - if (cancelled) { - return - } - if (result && 'error' in result) { - setRead({ phase: 'error', error: result.error }) - return - } - const messages = result?.messages ?? [] - setRead({ phase: 'ready', messages }) - setHasMore(hasMoreNativeChatHistory(messages.length, limitRef.current)) - }) - .catch((err: unknown) => { - if (!cancelled) { - setRead({ phase: 'error', error: err instanceof Error ? err.message : String(err) }) - } - }) + function loadSession(attempt: number): void { + void transport + .readSession(agent, activeSessionId, limitRef.current, transcriptPath ?? undefined) + .then((result) => { + if (cancelled) { + return + } + if (result && 'error' in result) { + // A not-yet-flushed transcript: stay in 'loading' and retry with + // backoff instead of settling into a permanent error (#8401). + if (result.notFound && Date.now() - retryStartedAt < NOTFOUND_RETRY_WINDOW_MS) { + retryTimer = setTimeout(() => { + retryTimer = null + loadSession(attempt + 1) + }, notFoundRetryDelayMs(attempt)) + return + } + setRead({ phase: 'error', error: result.error }) + return + } + const messages = result?.messages ?? [] + setRead({ phase: 'ready', messages }) + setHasMore(hasMoreNativeChatHistory(messages.length, limitRef.current)) + }) + .catch((err: unknown) => { + if (!cancelled) { + setRead({ phase: 'error', error: err instanceof Error ? err.message : String(err) }) + } + }) + } + + loadSession(0) const subscriptionId = nextSubscriptionId() const unsubscribe = transport.subscribe( @@ -208,6 +238,10 @@ export function useNativeChatLiveSession( return () => { cancelled = true + if (retryTimer) { + clearTimeout(retryTimer) + retryTimer = null + } // Desktop returns a sync unsubscribe fn; the web RPC bridge returns a // Promise instead (and can't deliver streaming callbacks). Calling a // Promise as a function crashed the whole chat view, so resolve it first @@ -304,9 +338,23 @@ export function useNativeChatLiveSession( sessionId, agent, hookState, - loading: read.phase === 'loading', - ...(read.phase === 'error' ? { error: read.error } : {}) + // Why: a watcher append (fix for #8401) can land content while the read is + // still retrying ('loading') or after it settled into 'error' — in both + // cases showing the live content beats a spinner or a stale error, so each + // override only applies while there is nothing appended to render. + loading: read.phase === 'loading' && appended.length === 0, + ...(read.phase === 'error' && appended.length === 0 ? { error: read.error } : {}) }) return { ...session, hasMore, loadingEarlier, loadEarlier } - }, [assembledMessages, read, sessionId, agent, hookState, hasMore, loadingEarlier, loadEarlier]) + }, [ + assembledMessages, + read, + sessionId, + agent, + hookState, + hasMore, + loadingEarlier, + loadEarlier, + appended + ]) } diff --git a/tests/e2e/native-chat-first-flush-race.spec.ts b/tests/e2e/native-chat-first-flush-race.spec.ts new file mode 100644 index 00000000000..15ec43dc843 --- /dev/null +++ b/tests/e2e/native-chat-first-flush-race.spec.ts @@ -0,0 +1,165 @@ +import { randomUUID } from 'node:crypto' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import type { Page } from '@stablyai/playwright-test' +import { test, expect } from './helpers/orca-app' +import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { waitForActivePaneHookDescriptor, waitForActiveTerminalManager } from './helpers/terminal' +import type { GlobalSettings } from '../../src/shared/types' + +const LOADING_TITLE = 'Loading conversation…' +const ERROR_TITLE = 'Could not load conversation' + +async function enableNativeChatSetting(page: Page): Promise { + await page.evaluate(async () => { + const nextSettings = await window.api.settings.set({ experimentalNativeChat: true }) + window.__store?.setState({ settings: nextSettings as GlobalSettings }) + }) +} + +// Why: seeding agentStatusByPaneKey directly (rather than posting a real +// `/hook/claude` event) mirrors the technique agent-session-quit-resume.spec.ts +// uses to stay hermetic — it exercises the identical store → NativeChatView +// path a real Claude Code hook would drive, without an installed CLI. +async function seedClaudeProviderSession( + page: Page, + args: { paneKey: string; worktreeId: string; sessionId: string; transcriptPath: string } +): Promise { + await page.evaluate(({ paneKey, worktreeId, sessionId, transcriptPath }) => { + window.__store + ?.getState() + .setAgentStatus( + paneKey, + { state: 'working', prompt: 'e2e first-flush race probe', agentType: 'claude' }, + 'Claude', + undefined, + { worktreeId }, + { providerSession: { key: 'session_id', id: sessionId, transcriptPath } } + ) + }, args) +} + +// Why: toggleTabViewMode keys off the *unified* tab id, which can differ from +// the terminal tab id embedded in paneKey — resolve it the same way +// TerminalPane.tsx does before calling the store action a real toggle/shortcut +// would use. +async function toggleTerminalTabToChatView( + page: Page, + args: { tabId: string; worktreeId: string } +): Promise { + await page.evaluate(({ tabId, worktreeId }) => { + const store = window.__store + if (!store) { + throw new Error('Store unavailable') + } + const state = store.getState() + const unifiedTab = (state.unifiedTabsByWorktree[worktreeId] ?? []).find( + (tab) => tab.contentType === 'terminal' && tab.entityId === tabId + ) + if (!unifiedTab) { + throw new Error('Unified terminal tab not found for chat toggle') + } + state.toggleTabViewMode(unifiedTab.id) + }, args) +} + +function claudeTranscriptLines(args: { + sessionId: string + userText: string + assistantText: string +}): string { + // Why: distinct timestamps keep the rendered order deterministic (a tie is + // broken by uuid, which would put the assistant turn first). + const userTime = new Date() + const assistantTime = new Date(userTime.getTime() + 2_000) + const lines = [ + { + sessionId: args.sessionId, + uuid: `${args.sessionId}-user`, + timestamp: userTime.toISOString(), + type: 'user', + message: { role: 'user', content: [{ type: 'text', text: args.userText }] } + }, + { + sessionId: args.sessionId, + uuid: `${args.sessionId}-assistant`, + timestamp: assistantTime.toISOString(), + type: 'assistant', + message: { model: 'claude-opus-4', content: [{ type: 'text', text: args.assistantText }] } + } + ] + return `${lines.map((line) => JSON.stringify(line)).join('\n')}\n` +} + +test.describe('Native chat first-flush transcript race (#8401)', () => { + test('stays in loading (never errors) until a not-yet-flushed transcript appears, then hydrates live', async ({ + orcaPage + }, testInfo) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage, 30_000) + + const descriptor = await waitForActivePaneHookDescriptor(orcaPage) + const [tabId] = descriptor.paneKey.split(':') + const sessionId = `e2e-first-flush-${randomUUID()}` + + // Why: a real Claude Code session flushes its first JSONL line up to + // minutes after launch (#8401) — this directory intentionally has no file + // yet when the pane resolves its providerSession. + const scratchDir = mkdtempSync(path.join(os.tmpdir(), 'orca-e2e-native-chat-')) + const transcriptPath = path.join(scratchDir, `${sessionId}.jsonl`) + + const screenshotDir = path.join( + process.cwd(), + 'validation-screenshots', + `native-chat-first-flush-race-${Date.now()}` + ) + mkdirSync(screenshotDir, { recursive: true }) + await testInfo.attach('validation-screenshot-dir', { + body: screenshotDir, + contentType: 'text/plain' + }) + + try { + await enableNativeChatSetting(orcaPage) + await seedClaudeProviderSession(orcaPage, { + paneKey: descriptor.paneKey, + worktreeId: descriptor.worktreeId, + sessionId, + transcriptPath + }) + await toggleTerminalTabToChatView(orcaPage, { tabId, worktreeId: descriptor.worktreeId }) + + await expect(orcaPage.locator('[data-native-chat-root="true"]')).toBeVisible({ + timeout: 15_000 + }) + await expect(orcaPage.getByText(LOADING_TITLE)).toBeVisible({ timeout: 10_000 }) + await expect(orcaPage.getByText(ERROR_TITLE)).toHaveCount(0) + await orcaPage.screenshot({ + path: path.join(screenshotDir, '01-loading-no-error.png') + }) + + // Why: a short real delay proves the first readSession attempt already + // hit the not-yet-flushed file (returning notFound) and the renderer's + // backoff retry — not a lucky first read — is what picks it up below. + await orcaPage.waitForTimeout(1_500) + await expect(orcaPage.getByText(ERROR_TITLE)).toHaveCount(0) + + const userText = 'Explain the native chat first-flush race fix for #8401' + const assistantText = + 'The main process now retries a not-yet-flushed transcript instead of caching a permanent miss.' + writeFileSync(transcriptPath, claudeTranscriptLines({ sessionId, userText, assistantText })) + + await expect(orcaPage.getByText(userText)).toBeVisible({ timeout: 30_000 }) + await expect(orcaPage.getByText(assistantText)).toBeVisible({ timeout: 30_000 }) + await expect(orcaPage.getByText(ERROR_TITLE)).toHaveCount(0) + await orcaPage.screenshot({ + path: path.join(screenshotDir, '02-hydrated.png') + }) + } finally { + rmSync(scratchDir, { recursive: true, force: true }) + } + }) +}) From fc458f2b300a6c5b5e8bd1966ca99545ea1df6dd Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 13 Jul 2026 12:47:17 -0700 Subject: [PATCH 10/23] Remove finished and killed claude subagents from the sidebar roster (#8522) * Remove finished and killed claude subagents from the sidebar roster Finished one-shot subagents stayed in the pane roster as permanent idle rows (dozens per ultracode/workflow session), and a subagent killed without its SubagentStop hook stayed 'working' forever, pinning the pane working. SubagentStop now removes one-shot entries (teammates only idle: they are alive and resumable), and a lead Stop's present background_tasks list is treated as authoritative for non-teammates - unlisted entries are removed. Teammates are identified by their name-embedding agent ids (a-, verified against live hook captures). * Fix Claude subagent roster reconciliation * Drop named workflow lanes that share the teammate id shape Workflow/named one-shot agents report name-embedding lifecycle ids (afinder-C-, agent_type = the label), indistinguishable by shape from resumable teammates - so the roster retained them as idle rows (observed live: a 32-row 12h-old pile). Unlike teammates they ARE listed id-exact as subagent-typed background tasks, including inside their own SubagentStop payload. Use that corroboration to remove them on stop, reclassify task-listed teammate-shaped entries as one-shots, and reap teammate-shaped leftovers when a complete inventory lists no teammate-typed task at all (a teams session always lists its teammates, even idle ones). --- src/shared/agent-hook-listener.test.ts | 78 +++++- src/shared/agent-hook-listener.ts | 21 +- src/shared/claude-subagent-roster.test.ts | 245 +++++++++++++++--- src/shared/claude-subagent-roster.ts | 148 ++++++++--- .../claude-subagent-row-lifecycle.test.ts | 231 +++++++++++++++++ 5 files changed, 641 insertions(+), 82 deletions(-) create mode 100644 src/shared/claude-subagent-row-lifecycle.test.ts diff --git a/src/shared/agent-hook-listener.test.ts b/src/shared/agent-hook-listener.test.ts index a593d3bcae9..efafcaa5b06 100644 --- a/src/shared/agent-hook-listener.test.ts +++ b/src/shared/agent-hook-listener.test.ts @@ -26,6 +26,7 @@ import { clearGrokSessionPathLookupCacheForTests, findGrokChatHistoryBySessionId } from './grok-session-paths' +import { AGENT_STATUS_MAX_SUBAGENTS } from './agent-status-types' import { makePaneKey } from './stable-pane-id' const LEAF_ID = '11111111-1111-4111-8111-111111111111' @@ -2309,9 +2310,9 @@ describe('shared agent-hook-listener', () => { const stopped = claudeEvent({ hook_event_name: 'SubagentStop', agent_id: 'r1' }) expect(stopped?.payload.state).toBe('done') - expect(stopped?.payload.subagents).toEqual([ - expect.objectContaining({ id: 'r1', state: 'idle' }) - ]) + // Why: a finished one-shot leaves the sidebar instead of squatting as a + // permanent idle row for the rest of the session. + expect(stopped?.payload.subagents).toBeUndefined() }) it('keeps gating on tracked children when background_tasks is absent (older Claude)', () => { @@ -2408,6 +2409,35 @@ describe('shared agent-hook-listener', () => { ]) }) + it('keeps a teammate whose name differs from its configured agent type', () => { + claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'spawn reviewer' }) + claudeEvent({ + hook_event_name: 'SubagentStart', + agent_id: 'areviewer-6d3cb5b52120b7bf', + agent_type: 'security-reviewer' + }) + + // Why: teammate name and agent type are separate Agent-tool inputs; the + // lifecycle id embeds the former while the hook reports the latter. + claudeEvent({ + hook_event_name: 'SubagentStop', + agent_id: 'areviewer-6d3cb5b52120b7bf', + agent_type: 'security-reviewer' + }) + const idled = claudeEvent({ + hook_event_name: 'TeammateIdle', + teammate_name: 'reviewer', + team_name: 'session-x' + }) + expect(idled?.payload.subagents).toEqual([ + expect.objectContaining({ + id: 'areviewer-6d3cb5b52120b7bf', + agentType: 'security-reviewer', + state: 'idle' + }) + ]) + }) + it('scopes subagent rosters per pane', () => { claudeEvent( { hook_event_name: 'SubagentStart', agent_id: 'a1', agent_type: 'general-purpose' }, @@ -2555,7 +2585,7 @@ describe('shared agent-hook-listener', () => { expect(stopped?.payload.state).toBe('done') }) - it('demotes a snapshot-seeded child missing from a present background_tasks list', () => { + it('removes a snapshot-seeded child missing from a present background_tasks list', () => { seedClaudeSubagentRosterFromSnapshots(state, PANE_KEY, [ { id: 'a77', state: 'working', startedAt: 1000, agentType: 'general-purpose' } ]) @@ -2569,9 +2599,7 @@ describe('shared agent-hook-listener', () => { ] }) expect(stop?.payload.state).toBe('done') - expect(stop?.payload.subagents).toEqual([ - expect.objectContaining({ id: 'a77', state: 'idle' }) - ]) + expect(stop?.payload.subagents).toBeUndefined() }) it('keeps a snapshot-seeded child working while background_tasks still lists it', () => { @@ -2589,6 +2617,29 @@ describe('shared agent-hook-listener', () => { ]) }) + it('keeps a live child omitted by the background task snapshot cap', () => { + claudeEvent({ + hook_event_name: 'SubagentStart', + agent_id: 'alive-after-cap', + agent_type: 'general-purpose' + }) + const stop = claudeEvent({ + hook_event_name: 'Stop', + background_tasks: Array.from({ length: AGENT_STATUS_MAX_SUBAGENTS + 1 }, (_, index) => ({ + id: index === AGENT_STATUS_MAX_SUBAGENTS ? 'alive-after-cap' : `a${index}`, + type: 'subagent', + status: 'running' + })) + }) + + // Why: the inventory was capped before this id, so omission cannot + // prove the lifecycle-tracked child finished or was killed. + expect(stop?.payload.subagents).toContainEqual( + expect.objectContaining({ id: 'alive-after-cap', state: 'working' }) + ) + expect(stop?.payload.state).toBe('working') + }) + it('does not adopt a known child turn-boundary event as the lead state', () => { claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'go' }) claudeEvent({ @@ -2654,7 +2705,12 @@ describe('shared agent-hook-listener', () => { it('seeds the roster from persisted snapshots so a teammate-bearing Stop keeps child rows', () => { seedClaudeSubagentRosterFromSnapshots(state, PANE_KEY, [ - { id: 'aprobe2-abc', state: 'idle', startedAt: 1000, agentType: 'probe2' } + { + id: 'aprobe2-6d3cb5b52120b7bf', + state: 'idle', + startedAt: 1000, + agentType: 'security-reviewer' + } ]) claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'after restart' }) const stop = claudeEvent({ @@ -2665,7 +2721,11 @@ describe('shared agent-hook-listener', () => { }) expect(stop?.payload.state).toBe('done') expect(stop?.payload.subagents).toEqual([ - expect.objectContaining({ id: 'aprobe2-abc', state: 'idle' }) + expect.objectContaining({ + id: 'aprobe2-6d3cb5b52120b7bf', + agentType: 'security-reviewer', + state: 'idle' + }) ]) }) diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index ce3e2f720a4..1e3728eaef7 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -38,8 +38,9 @@ import { claudeRosterHasWorkingSubagent, claudeRosterToSnapshots, claudeTeammateIdMatchesName, + finishClaudeSubagent, foldClaudeBackgroundTasksIntoRoster, - markClaudeSubagentIdle, + isClaudeTeammateLifecycleId, markClaudeTeammateIdleByName, readClaudeBackgroundAgentTasks, upsertWorkingClaudeSubagent, @@ -2411,7 +2412,14 @@ function normalizeClaudeSubagentLifecycleEvent( Date.now() ) } else { - markClaudeSubagentIdle(roster, agentId) + // Why: SubagentStop carries the session's task inventory; a stopping + // agent listed id-exact as a subagent task is a workflow/named one-shot + // (teammate lifecycle ids never appear there), not a resumable teammate. + const stopTasks = readClaudeBackgroundAgentTasks(hookPayload) + finishClaudeSubagent(roster, agentId, { + listedAsSubagentTask: + stopTasks.present && stopTasks.tasks.some((task) => !task.teammate && task.id === agentId) + }) // Why: a blocked child that dies (killed, errored) without another tool // event would otherwise pin its permission/question wait on the pane // forever — nothing else references that agent again. @@ -2449,9 +2457,13 @@ export function seedClaudeSubagentRosterFromSnapshots( startedAt: snapshot.startedAt, agentType: snapshot.agentType, description: snapshot.description, + // Why: teammate name and agent type can differ, but the provider id + // shape survives persistence and keeps the row across restart folds. + ...(isClaudeTeammateLifecycleId(snapshot.id) ? { teammate: true as const } : {}), // Why: the seed can be a phantom (child finished while Orca was down, // its SubagentStop lost). Let a PRESENT background_tasks list that - // omits the id demote it instead of gating the pane 'working' forever. + // omits the id remove it (or demote a teammate) instead of gating the + // pane 'working' forever. backgroundTasksAuthoritative: true }) } @@ -2609,7 +2621,8 @@ function normalizeClaudeEvent( foldClaudeBackgroundTasksIntoRoster( getOrCreateClaudeSubagentRoster(state, paneKey), backgroundTasks.tasks, - Date.now() + Date.now(), + { inventoryComplete: !backgroundTasks.truncated } ) } } diff --git a/src/shared/claude-subagent-roster.test.ts b/src/shared/claude-subagent-roster.test.ts index e8a4cb6cb1e..99cd9bbb8dc 100644 --- a/src/shared/claude-subagent-roster.test.ts +++ b/src/shared/claude-subagent-roster.test.ts @@ -4,8 +4,8 @@ import { claudeRosterHasWorkingSubagent, claudeRosterToSnapshots, claudeTeammateIdMatchesName, + finishClaudeSubagent, foldClaudeBackgroundTasksIntoRoster, - markClaudeSubagentIdle, markClaudeTeammateIdleByName, readClaudeBackgroundAgentTasks, upsertWorkingClaudeSubagent, @@ -13,55 +13,136 @@ import { } from './claude-subagent-roster' describe('claude-subagent-roster', () => { - it('tracks spawn and stop as working → idle', () => { + it('removes a finished one-shot subagent on stop', () => { const roster: ClaudeSubagentRoster = new Map() upsertWorkingClaudeSubagent(roster, 'a1', { agentType: 'general-purpose' }, 100) expect(claudeRosterHasWorkingSubagent(roster)).toBe(true) - markClaudeSubagentIdle(roster, 'a1') - expect(claudeRosterHasWorkingSubagent(roster)).toBe(false) - expect(claudeRosterToSnapshots(roster)).toEqual([ - { - id: 'a1', - state: 'idle', - startedAt: 100, - agentType: 'general-purpose', - description: undefined - } - ]) + // Why: retaining finished one-shots as idle rows piled up dozens of dead + // "Idle - general-purpose" sidebar rows over a long workflow session. + finishClaudeSubagent(roster, 'a1') + expect(roster.size).toBe(0) + expect(claudeRosterToSnapshots(roster)).toBeUndefined() }) - it('re-marks an idle subagent working without resetting startedAt', () => { + it('idles a teammate on stop instead of removing it', () => { const roster: ClaudeSubagentRoster = new Map() - upsertWorkingClaudeSubagent(roster, 'a1', {}, 100) - markClaudeSubagentIdle(roster, 'a1') - upsertWorkingClaudeSubagent(roster, 'a1', { description: 'round two' }, 200) - expect(roster.get('a1')).toMatchObject({ + upsertWorkingClaudeSubagent(roster, 'aprobe1-6d3cb5b5', { agentType: 'probe1' }, 100) + finishClaudeSubagent(roster, 'aprobe1-6d3cb5b5') + expect(roster.get('aprobe1-6d3cb5b5')).toMatchObject({ state: 'idle', teammate: true }) + }) + + it('removes a finished workflow lane despite its teammate-shaped id when task-listed', () => { + const roster: ClaudeSubagentRoster = new Map() + // Why: workflow lanes get name-embedding ids (afinder-C-) like + // teammates, but their SubagentStop payload lists them id-exact as a + // subagent task — real teammate lifecycle ids never appear there. + upsertWorkingClaudeSubagent( + roster, + 'afinder-C-5d713c0781b7f8d2', + { agentType: 'finder-C' }, + 100 + ) + finishClaudeSubagent(roster, 'afinder-C-5d713c0781b7f8d2', { listedAsSubagentTask: true }) + expect(roster.size).toBe(0) + }) + + it('reclassifies a task-listed teammate-shaped entry as a one-shot', () => { + const roster: ClaudeSubagentRoster = new Map() + upsertWorkingClaudeSubagent( + roster, + 'av1-streaming-0b1c2d3e', + { agentType: 'v1-streaming' }, + 100 + ) + foldClaudeBackgroundTasksIntoRoster( + roster, + [ + { + id: 'av1-streaming-0b1c2d3e', + agentType: 'v1-streaming', + description: undefined, + running: true, + teammate: false + }, + { + id: 'tteam1', + agentType: undefined, + description: undefined, + running: true, + teammate: true + } + ], + 200 + ) + // A later stop without its own inventory still removes it: the fold + // already proved the id is a task id, not a teammate. + finishClaudeSubagent(roster, 'av1-streaming-0b1c2d3e') + expect(roster.has('av1-streaming-0b1c2d3e')).toBe(false) + }) + + it('removes teammate-shaped leftovers when a complete inventory lists no teammates', () => { + const roster: ClaudeSubagentRoster = new Map() + upsertWorkingClaudeSubagent(roster, 'acr-triage-1-c5a0588e', { agentType: 'cr-triage-1' }, 100) + finishClaudeSubagent(roster, 'acr-triage-1-c5a0588e') + expect(roster.get('acr-triage-1-c5a0588e')).toMatchObject({ state: 'idle' }) + upsertWorkingClaudeSubagent(roster, 'afix-main-11223344', { agentType: 'fix-main' }, 150) + + // Why: a teams session lists its teammates (even idle) as teammate-typed + // tasks; an inventory with none proves these name-shaped rows are dead + // workflow lanes, not resumable teammates. + foldClaudeBackgroundTasksIntoRoster( + roster, + [ + { + id: 'aunrelated0000001', + agentType: 'general-purpose', + description: undefined, + running: true, + teammate: false + } + ], + 200 + ) + expect(roster.has('acr-triage-1-c5a0588e')).toBe(false) + expect(roster.has('afix-main-11223344')).toBe(false) + expect(roster.has('aunrelated0000001')).toBe(true) + }) + + it('re-marks an idle teammate working without resetting startedAt', () => { + const roster: ClaudeSubagentRoster = new Map() + upsertWorkingClaudeSubagent(roster, 'aprobe1-6d3cb5b5', { agentType: 'probe1' }, 100) + finishClaudeSubagent(roster, 'aprobe1-6d3cb5b5') + upsertWorkingClaudeSubagent(roster, 'aprobe1-6d3cb5b5', { description: 'round two' }, 200) + expect(roster.get('aprobe1-6d3cb5b5')).toMatchObject({ state: 'working', startedAt: 100, description: 'round two' }) }) - it('ignores unknown ids on markClaudeSubagentIdle', () => { + it('ignores unknown ids on finishClaudeSubagent', () => { const roster: ClaudeSubagentRoster = new Map() - markClaudeSubagentIdle(roster, 'ghost') + finishClaudeSubagent(roster, 'ghost') expect(roster.size).toBe(0) }) it('caps roster size, evicting the oldest idle entry first', () => { const roster: ClaudeSubagentRoster = new Map() - for (let i = 0; i < AGENT_STATUS_MAX_SUBAGENTS; i++) { + upsertWorkingClaudeSubagent(roster, 'aprobe1-6d3cb5b5', { agentType: 'probe1' }, 0) + for (let i = 1; i < AGENT_STATUS_MAX_SUBAGENTS; i++) { upsertWorkingClaudeSubagent(roster, `a${i}`, {}, i) } // Why: all working — a new spawn cannot evict live children and is dropped. upsertWorkingClaudeSubagent(roster, 'overflow', {}, 999) expect(roster.has('overflow')).toBe(false) - markClaudeSubagentIdle(roster, 'a3') + // Why: only teammates hold idle entries now; an idle teammate is the + // eviction pool when a burst of new spawns hits the cap. + finishClaudeSubagent(roster, 'aprobe1-6d3cb5b5') upsertWorkingClaudeSubagent(roster, 'replacement', {}, 1000) expect(roster.has('replacement')).toBe(true) - expect(roster.has('a3')).toBe(false) + expect(roster.has('aprobe1-6d3cb5b5')).toBe(false) expect(roster.size).toBe(AGENT_STATUS_MAX_SUBAGENTS) }) @@ -105,11 +186,21 @@ describe('claude-subagent-roster', () => { expect(readClaudeBackgroundAgentTasks({ background_tasks: 'nope' }).present).toBe(false) }) + it('marks a background task inventory truncated after the snapshot cap', () => { + const tasks = Array.from({ length: AGENT_STATUS_MAX_SUBAGENTS + 1 }, (_, index) => ({ + id: `a${index}`, + type: 'subagent', + status: 'running' + })) + const result = readClaudeBackgroundAgentTasks({ background_tasks: tasks }) + expect(result.tasks).toHaveLength(AGENT_STATUS_MAX_SUBAGENTS) + expect(result.truncated).toBe(true) + }) + it('folds background_tasks in without trusting ambiguous entries', () => { const roster: ClaudeSubagentRoster = new Map() upsertWorkingClaudeSubagent(roster, 'a1', {}, 100) - markClaudeSubagentIdle(roster, 'a1') - upsertWorkingClaudeSubagent(roster, 'ateam-xyz', { agentType: 'reviewer' }, 150) + upsertWorkingClaudeSubagent(roster, 'ateam-6d3cb5b5', { agentType: 'security-reviewer' }, 150) foldClaudeBackgroundTasksIntoRoster( roster, @@ -137,7 +228,56 @@ describe('claude-subagent-roster', () => { expect(roster.size).toBe(2) // Why: id-exact matches are one-shot subagents whose run state IS reliable. expect(roster.get('a1')).toMatchObject({ state: 'working', description: 'review loop' }) - expect(roster.get('ateam-xyz')).toMatchObject({ state: 'working', agentType: 'reviewer' }) + // Why: the working lifecycle-tracked teammate is not listed by id, but + // omission proves nothing for teammates — it must survive the fold. + expect(roster.get('ateam-6d3cb5b5')).toMatchObject({ + state: 'working', + agentType: 'security-reviewer' + }) + }) + + it('removes an id-matched task reported not running', () => { + const roster: ClaudeSubagentRoster = new Map() + upsertWorkingClaudeSubagent(roster, 'a1', { agentType: 'general-purpose' }, 100) + foldClaudeBackgroundTasksIntoRoster( + roster, + [{ id: 'a1', agentType: undefined, description: undefined, running: false, teammate: false }], + 200 + ) + expect(roster.size).toBe(0) + }) + + it('removes a killed one-shot missing from a present list', () => { + const roster: ClaudeSubagentRoster = new Map() + // Why: a running one-shot is always listed id-exact at a lead Stop, so a + // working non-teammate missing from the list is dead (SubagentStop lost); + // keeping it pinned the pane 'working' forever. + upsertWorkingClaudeSubagent(roster, 'akilled0000000001', { agentType: 'general-purpose' }, 100) + foldClaudeBackgroundTasksIntoRoster( + roster, + [ + { id: 'other', agentType: undefined, description: undefined, running: true, teammate: true } + ], + 200 + ) + expect(roster.size).toBe(0) + }) + + it('retains an unlisted live child when the background task inventory was truncated', () => { + const roster: ClaudeSubagentRoster = new Map() + upsertWorkingClaudeSubagent(roster, 'alive-after-cap', {}, 100) + const parsed = readClaudeBackgroundAgentTasks({ + background_tasks: Array.from({ length: AGENT_STATUS_MAX_SUBAGENTS + 1 }, (_, index) => ({ + id: index === AGENT_STATUS_MAX_SUBAGENTS ? 'alive-after-cap' : `a${index}`, + type: 'subagent', + status: 'running' + })) + }) + + foldClaudeBackgroundTasksIntoRoster(roster, parsed.tasks, 200, { + inventoryComplete: !parsed.truncated + }) + expect(roster.has('alive-after-cap')).toBe(true) }) it('recreates unmatched running one-shot subagents after a listener restart', () => { @@ -174,17 +314,14 @@ describe('claude-subagent-roster', () => { expect(roster.size).toBe(0) }) - it('demotes task-id-authoritative entries missing from a present list', () => { + it('removes non-teammate authoritative entries and keeps live teammates on omission', () => { const roster: ClaudeSubagentRoster = new Map() - // Why: seeded/bt-sourced ids ARE task ids; absence from a present list - // proves the task finished. Lifecycle-tracked ids (teammates) prove - // nothing by absence and must keep their state. roster.set('a-phantom', { state: 'working', startedAt: 100, backgroundTasksAuthoritative: true }) - upsertWorkingClaudeSubagent(roster, 'ateam-xyz', { agentType: 'reviewer' }, 150) + upsertWorkingClaudeSubagent(roster, 'ateam-6d3cb5b5', { agentType: 'security-reviewer' }, 150) foldClaudeBackgroundTasksIntoRoster( roster, @@ -193,11 +330,33 @@ describe('claude-subagent-roster', () => { ], 200 ) - expect(roster.get('a-phantom')).toMatchObject({ state: 'idle' }) - expect(roster.get('ateam-xyz')).toMatchObject({ state: 'working' }) + expect(roster.has('a-phantom')).toBe(false) + expect(roster.get('ateam-6d3cb5b5')).toMatchObject({ state: 'working' }) }) - it('marks fold-recreated entries as task-id-authoritative for later folds', () => { + it('demotes a seeded teammate phantom missing from a present list to idle', () => { + const roster: ClaudeSubagentRoster = new Map() + // Why: a teammate seeded from a pre-restart snapshot may be dead, but its + // task id never appears in background_tasks — demote instead of delete so + // a live idle teammate keeps its row while a phantom stops gating the pane. + roster.set('aprobe1-6d3cb5b5', { + state: 'working', + startedAt: 100, + agentType: 'probe1', + teammate: true, + backgroundTasksAuthoritative: true + }) + foldClaudeBackgroundTasksIntoRoster( + roster, + [ + { id: 'other', agentType: undefined, description: undefined, running: true, teammate: true } + ], + 200 + ) + expect(roster.get('aprobe1-6d3cb5b5')).toMatchObject({ state: 'idle', teammate: true }) + }) + + it('removes fold-recreated entries missing from a later present list', () => { const roster: ClaudeSubagentRoster = new Map() foldClaudeBackgroundTasksIntoRoster( roster, @@ -211,13 +370,21 @@ describe('claude-subagent-roster', () => { ], 200 ) - expect(roster.get('a9')).toMatchObject({ state: 'idle' }) + expect(roster.has('a9')).toBe(false) }) - it('stops demoting an entry once live activity re-tracks it', () => { + it('keeps a re-tracked working teammate missing from a present list', () => { const roster: ClaudeSubagentRoster = new Map() - roster.set('a-seeded', { state: 'working', startedAt: 100, backgroundTasksAuthoritative: true }) - upsertWorkingClaudeSubagent(roster, 'a-seeded', {}, 150) + roster.set('aprobe1-6d3cb5b5', { + state: 'working', + startedAt: 100, + agentType: 'probe1', + teammate: true, + backgroundTasksAuthoritative: true + }) + // Why: live activity clears the authoritative flag; a busy teammate must + // not be demoted by a Stop that (as always) omits its lifecycle id. + upsertWorkingClaudeSubagent(roster, 'aprobe1-6d3cb5b5', { agentType: 'probe1' }, 150) foldClaudeBackgroundTasksIntoRoster( roster, @@ -226,7 +393,7 @@ describe('claude-subagent-roster', () => { ], 200 ) - expect(roster.get('a-seeded')).toMatchObject({ state: 'working' }) + expect(roster.get('aprobe1-6d3cb5b5')).toMatchObject({ state: 'working' }) }) it('matches teammate ids by name only up to the hyphen-free suffix', () => { diff --git a/src/shared/claude-subagent-roster.ts b/src/shared/claude-subagent-roster.ts index b4b4f7ef946..b0b945a76e2 100644 --- a/src/shared/claude-subagent-roster.ts +++ b/src/shared/claude-subagent-roster.ts @@ -14,12 +14,18 @@ export type TrackedClaudeSubagent = { description?: string state: 'working' | 'idle' startedAt: number - /** The id came from background_tasks or a persisted snapshot, not live - * lifecycle events, so a PRESENT list omitting it proves the task is gone - * (a phantom seeded before restart would otherwise gate the pane 'working' - * forever — teams sessions never send an empty list). Cleared once live - * activity re-tracks the id, so a seeded-but-alive teammate is demoted at - * most until its next tool event. */ + /** Known agent-teams teammate: its id embeds its name (`a-`) + * while one-shot ids are hyphen-free (`a`). Teammates are long-lived — + * SubagentStop means "finished a task", not "gone" — and their lifecycle + * ids never appear in background_tasks, so omission proves nothing. */ + teammate?: true + /** The id came from a persisted snapshot or background_tasks, not live + * lifecycle events. Only matters for teammate-shaped seeds now: a PRESENT + * list omitting a seeded-working teammate demotes it to idle so a phantom + * seeded before restart can't gate the pane 'working' forever (teams + * sessions never send an empty list). Cleared once live activity re-tracks + * the id. Non-teammate entries omitted from a present list are removed + * outright regardless of this flag. */ backgroundTasksAuthoritative?: boolean } @@ -36,6 +42,14 @@ export type ClaudeBackgroundAgentTask = { teammate: boolean } +/** Agent-team lifecycle ids are `a-`. The teammate name + * and agent type are independent spawn fields, so the id shape is the only + * reliable discriminator available on SubagentStart/SubagentStop hooks. */ +export function isClaudeTeammateLifecycleId(id: string): boolean { + const separator = id.lastIndexOf('-') + return separator > 1 && id.startsWith('a') && /^[0-9a-f]+$/i.test(id.slice(separator + 1)) +} + export function upsertWorkingClaudeSubagent( roster: ClaudeSubagentRoster, id: string, @@ -45,11 +59,15 @@ export function upsertWorkingClaudeSubagent( if (id.length === 0 || id.length > CLAUDE_SUBAGENT_ID_MAX_LENGTH) { return } + const teammate = isClaudeTeammateLifecycleId(id) const existing = roster.get(id) if (existing) { existing.state = 'working' existing.agentType = fields.agentType ?? existing.agentType existing.description = fields.description ?? existing.description + if (teammate) { + existing.teammate = true + } // Why: live activity proves the lifecycle stream owns this id again; // background_tasks absence must stop demoting it (teammate ids never // appear there). The fold re-tags its own recreations after this call. @@ -63,7 +81,8 @@ export function upsertWorkingClaudeSubagent( state: 'working', startedAt: now, agentType: fields.agentType, - description: fields.description + description: fields.description, + ...(teammate ? { teammate: true as const } : {}) }) } @@ -83,11 +102,31 @@ function evictOldestIdleClaudeSubagent(roster: ClaudeSubagentRoster): boolean { return true } -export function markClaudeSubagentIdle(roster: ClaudeSubagentRoster, id: string): void { +/** SubagentStop: a finished one-shot subagent leaves the sidebar immediately — + * retaining it as an idle row made long workflow/ultracode sessions pile up + * dozens of dead rows. Teammates only idle (alive + resumable): SubagentStop + * fires each time a teammate finishes a task, not just at shutdown. + * + * Workflow/named one-shots share the teammate id shape (`a