diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 3c9678a5a8d..0c1789bf3ea 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -426,8 +426,12 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile - - name: Test Windows PowerShell CLI boundary - run: pnpm exec vitest run --config config/vitest.config.ts src/main/cli/wsl-cli-powershell-boundary.test.ts + - name: Test Windows-specific boundaries + run: >- + pnpm exec vitest run --config config/vitest.config.ts + src/main/cli/wsl-cli-powershell-boundary.test.ts + src/main/orca-profiles/profile-index-store.test.ts + src/shared/secure-file-fsync-flags.test.ts - name: Build package inputs run: pnpm run build:release diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 0702c1948dd..fab7be18ea0 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -17,21 +17,23 @@ "protection": "partial", "owner": "terminal-session-readiness", "layer": "posix-pty-and-shell-startup", - "surfaces": ["queued terminal startup commands", "zsh and bash startup files"], + "surfaces": ["queued terminal startup commands", "zsh, bash, and Fish startup output"], "platforms": ["macos", "linux"], "providers": ["local", "local-daemon", "ssh-daemon", "ssh-relay", "paired-runtime"], "coveredPlatforms": ["macos", "linux"], "coveredProviders": ["local", "local-daemon", "ssh-relay", "paired-runtime"], - "coverageNotes": "Real macOS node-pty integration covers zsh 5.9 .zshenv/.zprofile and bash 3.2 profile replacement shells, including exec -a names, same-shell and child-process silent reads, a blocking zle-line-init hook, non-shell readline replacements, and a non-shell image renamed zsh. An Ubuntu 20.04 container confirms compatible zsh/bash termios. Deterministic owner contracts cover local, daemon, and SSH relay identity stripping plus provider-owned delivery; explicit terminal-paste remains renderer-owned. Live isolated journeys cover a macOS headed desktop owner with paired web client, headless orca serve with paired web client, and a Docker Linux OpenSSH/Bash target with one readiness-time reconnect. Linux desktop, WSL/Windows, live zsh-over-SSH, and physical remote hosts remain gaps.", + "coverageNotes": "Real macOS node-pty integration covers zsh 5.9 .zshenv/.zprofile and bash 3.2 profile replacement shells, including exec -a names, same-shell and child-process silent reads, a blocking zle-line-init hook, non-shell readline replacements, and a non-shell image renamed zsh. A real Fish PTY proves marker stripping is byte-exact and does not render bracketed-paste control text. An Ubuntu 20.04 container confirms compatible zsh/bash termios. Deterministic owner contracts cover local, daemon, and SSH relay identity/readiness composition plus provider-owned delivery; explicit terminal-paste remains renderer-owned. Live isolated journeys cover a macOS headed desktop owner with paired web client, headless orca serve with paired web client, and a Docker Linux OpenSSH/Bash target with one readiness-time reconnect. Linux desktop, WSL/Windows, live zsh-over-SSH, and physical remote hosts remain gaps.", "motivatingLinks": [ "https://github.com/stablyai/orca/issues/13767", "https://linear.app/stably/issue/STA-4067", + "https://linear.app/stably/issue/STA-4068", "https://github.com/stablyai/orca/pull/14161" ], - "invariant": "Before user startup files run, the wrapper publishes its shell PID. If a later exec discards the ready marker, proof-based recovery releases queued startup input only after the launched shell's actual executable image with that exact PID owns the PTY foreground, emits its post-hook line-editor enable sequence, and exposes zle/readline termios; startup reads, line-init hooks, child foreground processes, and non-shell readline replacements cannot trigger that proof. The existing bounded compatibility timeout may enqueue input without proof, but must not publish readiness, lose or duplicate that input across reconnect, or execute it before the replacement shell can accept it.", - "oracle": "Run the real generated wrappers under node-pty with disposable zsh and bash startup files. Plain and exec -a replacement shells must receive one queued command at their first completed line editor without waiting for the 15-second timeout. The identical oracle must time out without the startup identity and prompt fallback. Same-shell reads, child-process reads, a blocking zle-line-init hook, sqlite3, and sqlite3 renamed zsh must not trigger prompt fallback. Relay owner coverage must strip the identity and release through the same prompt contract.", + "invariant": "Shell startup scanning preserves every non-Orca output byte in order across arbitrary PTY chunk boundaries. Before user startup files run, the wrapper publishes its shell PID. If a later exec discards the ready marker, proof-based recovery releases queued startup input only after the launched shell's actual executable image with that exact PID owns the PTY foreground, emits its post-hook line-editor enable sequence, and exposes zle/readline termios; startup reads, line-init hooks, child foreground processes, and non-shell readline replacements cannot trigger that proof. The existing bounded compatibility timeout may enqueue input without proof, but must not publish readiness, lose or duplicate that input across reconnect, or execute it before the replacement shell can accept it.", + "oracle": "Split Fish output immediately after the ready marker and after the following ESC introducer; local and daemon owners must emit the exact ESC [ ? 2004 h bytes and headless xterm must not render literal control text. Run the real generated wrappers under node-pty with disposable zsh, bash, and Fish startup files. Plain and exec -a replacement shells must receive one queued command at their first completed line editor without waiting for the 15-second timeout. The identical oracle must time out without the startup identity and prompt fallback. Same-shell reads, child-process reads, a blocking zle-line-init hook, sqlite3, and sqlite3 renamed zsh must not trigger prompt fallback. Relay owner coverage must use the same lossless scanner composition and release through the same prompt contract.", "commands": [ - "pnpm exec vitest run --config config/vitest.config.ts src/main/daemon/repro-13767-shell-ready-marker-lost-to-exec.test.ts src/main/line-editor-ready-output-scanner.test.ts src/main/shell-startup-identity-scanner.test.ts src/main/shell-prompt-readiness-probe.test.ts src/shared/pty-slave-line-discipline-echo.test.ts src/shared/shell-process-readiness.test.ts src/main/daemon/session.test.ts src/main/daemon/shell-ready.test.ts src/main/providers/local-pty-shell-ready.test.ts src/main/providers/local-pty-provider.test.ts src/relay/pty-shell-launch.test.ts src/relay/pty-handler.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts src/main/daemon/repro-13767-shell-ready-marker-lost-to-exec.test.ts src/main/line-editor-ready-output-scanner.test.ts src/main/shell-startup-identity-scanner.test.ts src/main/shell-startup-output-scanner.test.ts src/main/shell-prompt-readiness-probe.test.ts src/shared/pty-slave-line-discipline-echo.test.ts src/shared/shell-process-readiness.test.ts src/main/daemon/session.test.ts src/main/daemon/shell-ready.test.ts src/main/providers/local-pty-shell-ready.test.ts src/main/providers/local-pty-provider.test.ts src/relay/pty-shell-launch.test.ts src/relay/pty-handler.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts --maxWorkers=1 src/main/daemon/shell-ready.test.ts src/main/shell-startup-output-scanner.test.ts", "ORCA_E2E_SSH_DOCKER=1 SKIP_BUILD=1 pnpm exec playwright test tests/e2e/ssh-startup-exec-readiness.spec.ts --config tests/playwright.config.ts --project electron-headful --workers=1", "ORCA_E2E_WEB_CLIENT=1 SKIP_BUILD=1 pnpm exec playwright test tests/e2e/paired-startup-exec-readiness.spec.ts --config tests/playwright.config.ts --project electron-headless --project electron-headful --workers=1" ], @@ -39,6 +41,7 @@ "src/main/daemon/repro-13767-shell-ready-marker-lost-to-exec.test.ts", "src/main/line-editor-ready-output-scanner.test.ts", "src/main/shell-startup-identity-scanner.test.ts", + "src/main/shell-startup-output-scanner.test.ts", "src/main/shell-prompt-readiness-probe.test.ts", "src/shared/pty-slave-line-discipline-echo.test.ts", "src/shared/shell-process-readiness.test.ts", @@ -52,6 +55,26 @@ "tests/e2e/paired-startup-exec-readiness.spec.ts" ], "assertionRefs": [ + { + "file": "src/main/shell-startup-output-scanner.test.ts", + "assertions": [ + "Fish bracketed-paste output stays byte-exact across marker and ESC chunk splits", + "every byte after readiness is preserved across every chunk boundary, even when it resembles an identity marker", + "pre-ready identity and readiness markers strip across every chunk boundary without reordering prompt output", + "incomplete scanner prefixes drain in byte order" + ] + }, + { + "file": "src/main/daemon/session.test.ts", + "assertions": [ + "daemon output is byte-exact for both Fish chunk splits", + "headless xterm never renders literal bracketed-paste control text" + ] + }, + { + "file": "src/main/providers/local-pty-provider.test.ts", + "assertions": ["local PTY output is byte-exact for both Fish chunk splits"] + }, { "file": "src/main/daemon/repro-13767-shell-ready-marker-lost-to-exec.test.ts", "assertions": [ @@ -88,14 +111,23 @@ } ], "evidenceRuns": [ + { + "date": "2026-08-12", + "runner": "manual", + "platform": "linux", + "command": "pnpm exec vitest run --config config/vitest.config.ts --maxWorkers=1 src/main/daemon/shell-ready.test.ts src/main/shell-startup-output-scanner.test.ts", + "result": "passed", + "durationSeconds": 7, + "summary": "A clean Debian 12 arm64 container with real Fish 3.6 and node-pty passed exact marker-stripped output plus headless-xterm rendering." + }, { "date": "2026-08-12", "runner": "local", "platform": "macos", - "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/daemon/repro-13767-shell-ready-marker-lost-to-exec.test.ts src/main/line-editor-ready-output-scanner.test.ts src/main/shell-startup-identity-scanner.test.ts src/main/shell-prompt-readiness-probe.test.ts src/shared/pty-slave-line-discipline-echo.test.ts src/shared/shell-process-readiness.test.ts src/main/daemon/session.test.ts src/main/daemon/shell-ready.test.ts src/main/providers/local-pty-shell-ready.test.ts src/main/providers/local-pty-provider.test.ts src/relay/pty-shell-launch.test.ts src/relay/pty-handler.test.ts", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/daemon/repro-13767-shell-ready-marker-lost-to-exec.test.ts src/main/line-editor-ready-output-scanner.test.ts src/main/shell-startup-identity-scanner.test.ts src/main/shell-startup-output-scanner.test.ts src/main/shell-prompt-readiness-probe.test.ts src/shared/pty-slave-line-discipline-echo.test.ts src/shared/shell-process-readiness.test.ts src/main/daemon/session.test.ts src/main/daemon/shell-ready.test.ts src/main/providers/local-pty-shell-ready.test.ts src/main/providers/local-pty-provider.test.ts src/relay/pty-shell-launch.test.ts src/relay/pty-handler.test.ts", "result": "passed", - "durationSeconds": 4, - "summary": "All real exec, silent-read, line-init-hook, renamed-image, disabled-bracketed-paste, and relay-owner oracles plus identity, foreground, executable-image, protocol, and line-editor contracts passed." + "durationSeconds": 6, + "summary": "473 tests passed with one local Fish skip, including every-boundary scanner composition, real exec, silent-read, line-init-hook, renamed-image, disabled-bracketed-paste, and relay-owner oracles." }, { "date": "2026-08-12", @@ -126,11 +158,11 @@ }, "redGreenEvidence": { "status": "complete", - "evidence": "Daemon oracle SHA-256 b13b08006a38b2adc2acb51ae9342909514e55ab4822fac4fbb76cd49b344cc6 was byte-identical across runs: affected baseline 09ec516ae5 failed 4 of 14, while pre-PR main bc2e30000b and this PR passed 14 of 14. The unchanged live SSH oracle failed on bc2e30000b and with the ownership change reverted because the ledger stayed pending after reconnect, then passed on this PR. The exact reviewed base is 150cc6f50c; its intervening daemon scrollback change does not alter startup delivery." + "evidence": "Daemon oracle SHA-256 b13b08006a38b2adc2acb51ae9342909514e55ab4822fac4fbb76cd49b344cc6 was byte-identical across runs: affected baseline 09ec516ae5 failed 4 of 14, while pre-PR main bc2e30000b and this PR passed 14 of 14. The unchanged live SSH oracle failed on bc2e30000b and with the ownership change reverted because the ledger stayed pending after reconnect, then passed on this PR. Fish chunk oracle patch SHA-256 fd49a4d3d91bd5f0d10c1b1f44f4cfbd92cef9155a5505b415c8680184cdc765 was also byte-identical: scan target e4c278eb52, main 150cc6f50c, and candidate ae7f30318a with shared scanner integration disabled failed only the ESC-introducer split in local and daemon owners, while ae7f30318a passed all 163 cases plus headless-xterm rendering." }, "performanceBudget": { "required": true, - "evidence": "The normal ready-marker path cancels before external probing. A missing marker triggers a debounced stty read only after a line-editor enable sequence, followed by narrow PID status and executable-image inspection only when termios matches. Slow ordinary startup output triggers zero probes; rejected protocol emissions are capped at four probes per startup. There is no per-command hook or steady-state polling." + "evidence": "Two bounded linear scans partition identity ownership at the exact ready-marker boundary and retain only the existing bounded prefixes. The normal ready-marker path cancels before external probing. A missing marker triggers a debounced stty read only after a line-editor enable sequence, followed by narrow PID status and executable-image inspection only when termios matches. Slow ordinary startup output triggers zero probes; rejected protocol emissions are capped at four probes per startup. There is no per-command hook or steady-state polling." }, "promotionCriteria": [ "Collect CI and soak history for the Docker SSH and paired live journeys.", diff --git a/src/main/browser/browser-cookie-import-google-exclusion.test.ts b/src/main/browser/browser-cookie-import-google-exclusion.test.ts index 3cc329a14d7..431ca758f2f 100644 --- a/src/main/browser/browser-cookie-import-google-exclusion.test.ts +++ b/src/main/browser/browser-cookie-import-google-exclusion.test.ts @@ -120,7 +120,7 @@ describe('file import excludes the Google cookie family', () => { }) describe('native Chromium import excludes the Google cookie family', () => { - let clearStorageDataMock: ReturnType + let clearDataMock: ReturnType let cookiesGetMock: ReturnType let cookiesRemoveMock: ReturnType let cookiesSetMock: ReturnType @@ -134,7 +134,7 @@ describe('native Chromium import excludes the Google cookie family', () => { execFileSyncMock.mockImplementation(() => { throw new Error('OS browser version lookup unavailable') }) - clearStorageDataMock = vi.fn().mockResolvedValue(undefined) + clearDataMock = vi.fn().mockResolvedValue(undefined) cookiesGetMock = vi.fn().mockResolvedValue([]) cookiesRemoveMock = vi.fn().mockResolvedValue(undefined) cookiesSetMock = vi.fn().mockResolvedValue(undefined) @@ -145,7 +145,7 @@ describe('native Chromium import excludes the Google cookie family', () => { remove: cookiesRemoveMock, set: cookiesSetMock }, - clearStorageData: clearStorageDataMock + clearData: clearDataMock }) platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') }) @@ -166,7 +166,7 @@ describe('native Chromium import excludes the Google cookie family', () => { createChromiumCookieTestDatabase(targetCookiesPath, rows).close() } - it('never clears or reconstructs live Google cookies before importing', async () => { + it('bulk clears while excluding live Google cookies before importing', async () => { const sourceCookiesPath = seedSource([ { domain: '.google.com', name: 'SID', value: 'transplanted-sid' }, { domain: '.example.com', name: 'session', value: 'new' } @@ -186,9 +186,10 @@ describe('native Chromium import excludes the Google cookie family', () => { googleCookiesSkipped: 1, domains: ['example.com'] }) - expect(clearStorageDataMock).not.toHaveBeenCalled() - expect(cookiesRemoveMock).toHaveBeenCalledOnce() - expect(cookiesRemoveMock).toHaveBeenCalledWith('https://example.com/', 'stale') + expect(clearDataMock.mock.calls).toEqual([ + [{ dataTypes: ['cookies'], excludeOrigins: ['https://google.com'] }] + ]) + expect(cookiesRemoveMock).not.toHaveBeenCalled() expect(cookiesSetMock.mock.calls.map(([details]) => details.domain)).toEqual(['.example.com']) }) @@ -239,7 +240,7 @@ describe('native Chromium import excludes the Google cookie family', () => { domains: [] }) expect(execFileSyncMock).not.toHaveBeenCalled() - expect(clearStorageDataMock).not.toHaveBeenCalled() + expect(clearDataMock).not.toHaveBeenCalled() expect(cookiesSetMock).not.toHaveBeenCalled() }) @@ -284,12 +285,13 @@ describe('native Chromium import excludes the Google cookie family', () => { throw new Error('cookie store unavailable') } }) + clearDataMock.mockRejectedValue(new Error('storage busy')) const result = await importCookiesFromBrowser(chromeBrowser(sourceCookiesPath), 'persist:test') expect(result).toMatchObject({ ok: false }) expect(result.ok || result.reason).toContain('Could not clear existing cookies') - expect(clearStorageDataMock).not.toHaveBeenCalled() + expect(clearDataMock).toHaveBeenCalledOnce() expect(cookiesRemoveMock.mock.calls.map(([, name]) => name)).toEqual(['removed-first', 'stale']) expect(cookiesSetMock).not.toHaveBeenCalled() expect(setPendingCookieImportMock).not.toHaveBeenCalled() diff --git a/src/main/browser/browser-cookie-import-partition-rollback.electron.test.ts b/src/main/browser/browser-cookie-import-partition-rollback.electron.test.ts index 5ee11981251..bbd0d9a8a25 100644 --- a/src/main/browser/browser-cookie-import-partition-rollback.electron.test.ts +++ b/src/main/browser/browser-cookie-import-partition-rollback.electron.test.ts @@ -20,8 +20,10 @@ type CdpCookie = { name: string; value: string; partitionKey?: Record | undefined clearError: string | null + bulkClearCalls: number remainingChips: CdpCookie[] remainingPlain: CdpCookie[] + remainingExcluded: CdpCookie[] } const EXPECTED_PARTITION_KEY = { @@ -33,7 +35,7 @@ function buildFixtureMain(policyPath: string, resultPath: string): string { return ` const { app, BrowserWindow, session } = require('electron') const { writeFileSync } = require('node:fs') -const { removeAllCookiesExcept, isNonTransplantableCookieDomain } = require(${JSON.stringify(policyPath)}) +const { removeTransplantableCookies } = require(${JSON.stringify(policyPath)}) const resultPath = ${JSON.stringify(resultPath)} let currentStep = 'starting' const mark = (step) => { @@ -76,6 +78,9 @@ async function run() { await targetSession.cookies.set({ url: 'https://plain.example/', name: 'plain', value: 'stale', secure: true }) await targetSession.cookies.set({ url: 'https://victim.example/', name: 'victim', value: 'stale', secure: true }) + // Why: keep a live excluded cookie in the partial-failure fixture so the fallback proves it + // preserves that cookie while deleting the removable ones it can reach. + await targetSession.cookies.set({ url: 'https://accounts.google.com/', name: 'SID', value: 'live', secure: true }) mark('removable cookies set') const store = { @@ -88,9 +93,20 @@ async function run() { } } + // Why: the bulk clear is the ordinary path now, so rejecting it is what routes this fixture onto + // the per-cookie fallback where a partial failure — and the old rollback — could happen at all. + let bulkClearCalls = 0 + const clearSession = { + cookies: store, + clearData: async () => { + bulkClearCalls++ + throw new Error('forced bulk clear failure') + } + } + let clearError = null try { - await removeAllCookiesExcept(store, (cookie) => isNonTransplantableCookieDomain(cookie.domain ?? '')) + await removeTransplantableCookies(clearSession) } catch (error) { clearError = String(error?.message || error) } @@ -104,8 +120,10 @@ async function run() { writeFileSync(resultPath, JSON.stringify({ beforePartitionKey: beforeChips.partitionKey, clearError, + bulkClearCalls, remainingChips: project('chips-auth'), - remainingPlain: project('plain') + remainingPlain: project('plain'), + remainingExcluded: project('SID') })) debug.detach() window.destroy() @@ -164,6 +182,10 @@ describe('non-Google partitioned cookie under a failed Electron cookie clear', ( const result = await runFixture() expect(result.beforePartitionKey).toEqual(EXPECTED_PARTITION_KEY) + // Why (STA-4065): the fallback is only reachable once the bulk clear has been tried and failed; + // if it ever succeeded here the partial-failure assertions below would be vacuous. + expect(result.bulkClearCalls).toBe(1) + expect(result.remainingExcluded.map(({ name }) => name)).toEqual(['SID']) expect(result.clearError).toContain('Could not clear existing cookies') // STA-4061: rollback rebuilt this cookie through cookies.set, which drops partitionKey. expect( diff --git a/src/main/browser/browser-cookie-import-partition.electron.test.ts b/src/main/browser/browser-cookie-import-partition.electron.test.ts index 494ebbd9e2b..e844341e12f 100644 --- a/src/main/browser/browser-cookie-import-partition.electron.test.ts +++ b/src/main/browser/browser-cookie-import-partition.electron.test.ts @@ -74,6 +74,20 @@ async function run() { value: 'remove-me', secure: true }) + // Why: the excluded origin is HTTPS, so prove registrable-domain matching also keeps HTTP. + await targetSession.cookies.set({ + url: 'http://mail.google.com/', + name: 'http-google', + value: 'live-http-value', + secure: false + }) + await targetSession.cookies.set({ + url: 'https://google.com/', + domain: '.google.com', + name: 'domain-google', + value: 'live-domain-value', + secure: true + }) mark('ordinary cookie set') const beforeCookies = (await debug.sendCommand('Network.getAllCookies')).cookies @@ -189,6 +203,11 @@ describe('native Chromium excluded partition cookie under Electron', () => { ok: true, summary: { importedCookies: 1, domains: ['example.com'] } }) - expect(result.remainingNames).toEqual(['imported', 'partitioned-google']) + expect(result.remainingNames).toEqual([ + 'domain-google', + 'http-google', + 'imported', + 'partitioned-google' + ]) }, 90_000) }) diff --git a/src/main/browser/browser-cookie-import-policy.test.ts b/src/main/browser/browser-cookie-import-policy.test.ts index f007c07d8ff..51a1a6107d5 100644 --- a/src/main/browser/browser-cookie-import-policy.test.ts +++ b/src/main/browser/browser-cookie-import-policy.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest' +import { describe, expect, it, vi, type Mock } from 'vitest' import { DatabaseSync } from 'node:sqlite' import type { Cookie } from 'electron' import { @@ -6,7 +6,7 @@ import { isNonTransplantableCookieDomain, NON_TRANSPLANTABLE_HOST_KEY_SQL, normalizeCookieDomain, - removeAllCookiesExcept, + removeTransplantableCookies, replaceCookiesForImportedDomains } from './browser-cookie-import-policy' @@ -207,78 +207,193 @@ describe('NON_TRANSPLANTABLE_HOST_KEY_SQL', () => { }) }) -describe('removeAllCookiesExcept', () => { - it('never removes or reconstructs excluded Google cookies', async () => { - const get = vi - .fn() - .mockResolvedValue([ - cookie('.google.com', 'SID'), - cookie('accounts.google.com', 'ACCOUNT'), - cookie('.example.com', 'session'), - cookie('other.test', 'tracker', '/scoped') - ]) - const remove = vi.fn().mockResolvedValue(undefined) - const set = vi.fn().mockResolvedValue(undefined) +type CookieClearMocks = { + get: Mock + remove: Mock + set: Mock + clearData: Mock +} - await removeAllCookiesExcept({ get, remove }, (existingCookie) => - isNonTransplantableCookieDomain(existingCookie.domain ?? '') +describe('removeTransplantableCookies', () => { + const rejectingBulkClear = () => vi.fn().mockRejectedValue(new Error('storage busy')) + + function clearSession(cookies: Cookie[], overrides: Partial = {}) { + const store = { + get: vi.fn().mockResolvedValue(cookies), + remove: vi.fn().mockResolvedValue(undefined), + set: vi.fn().mockResolvedValue(undefined), + ...overrides + } + const clearData = overrides.clearData ?? vi.fn().mockResolvedValue(undefined) + return { + session: { cookies: store, clearData }, + get: store.get, + remove: store.remove, + set: store.set, + clearData + } + } + + // Why (STA-4065): the bulk call is the ordinary path even when the jar holds cookies to keep — + // excludeOrigins preserves the whole google.com family, verified against real Electron. + it('clears a jar holding Google cookies in one call that excludes them', async () => { + const { session, remove, set, clearData } = clearSession([ + cookie('.google.com', 'SID'), + cookie('accounts.google.com', 'ACCOUNT'), + cookie('.example.com', 'session'), + cookie('other.test', 'tracker', '/scoped') + ]) + + await removeTransplantableCookies(session) + + expect(clearData.mock.calls).toEqual([ + [{ dataTypes: ['cookies'], excludeOrigins: ['https://google.com'] }] + ]) + expect(remove).not.toHaveBeenCalled() + expect(set).not.toHaveBeenCalled() + }) + + it('bulk clears in one call when the jar holds nothing to preserve', async () => { + const { session, remove, set, clearData } = clearSession([ + cookie('.example.com', 'session'), + cookie('other.test', 'tracker', '/scoped'), + cookie('notgoogle.com', 'lookalike') + ]) + + await removeTransplantableCookies(session) + + expect(clearData.mock.calls).toEqual([ + [{ dataTypes: ['cookies'], excludeOrigins: ['https://google.com'] }] + ]) + expect(remove).not.toHaveBeenCalled() + expect(set).not.toHaveBeenCalled() + }) + + it('touches nothing when the jar is already empty', async () => { + const { session, remove, clearData } = clearSession([]) + + await removeTransplantableCookies(session) + + expect(clearData).not.toHaveBeenCalled() + expect(remove).not.toHaveBeenCalled() + }) + + it('bulk clears cookies the per-cookie path could never address', async () => { + const { session, clearData } = clearSession([ + { ...cookie('.example.com', 'session'), domain: '' } + ]) + + await removeTransplantableCookies(session) + + expect(clearData).toHaveBeenCalledOnce() + }) + + it('falls back to per-cookie removal when the bulk clear rejects', async () => { + const { session, remove, clearData } = clearSession( + [cookie('.example.com', 'session'), cookie('other.test', 'tracker')], + { clearData: rejectingBulkClear() } ) + await removeTransplantableCookies(session) + + expect(clearData).toHaveBeenCalledOnce() expect(remove.mock.calls).toEqual([ ['https://example.com/', 'session'], - ['https://other.test/scoped', 'tracker'] + ['https://other.test/', 'tracker'] ]) + }) + + it('re-reads the jar after a rejected bulk clear', async () => { + const beforeAttempt = [cookie('.removed.test', 'gone-before-fallback')] + const afterAttempt = [ + cookie('.google.com', 'SID'), + cookie('.survivor.test', 'survived'), + cookie('.arrived.test', 'arrived-during-clear') + ] + const get = vi.fn().mockResolvedValueOnce(beforeAttempt).mockResolvedValueOnce(afterAttempt) + const { session, remove } = clearSession(beforeAttempt, { + get, + clearData: rejectingBulkClear() + }) + + await removeTransplantableCookies(session) + + expect(get).toHaveBeenCalledTimes(2) + expect(remove.mock.calls).toEqual([ + ['https://survivor.test/', 'survived'], + ['https://arrived.test/', 'arrived-during-clear'] + ]) + }) + + // Why: the fallback carries the same exclusion as the bulk call, so a rejected clearData must + // not become the path that finally deletes a live Google session. + it('still preserves Google cookies on the per-cookie fallback', async () => { + const { session, remove, set } = clearSession( + [ + cookie('.google.com', 'SID'), + cookie('accounts.google.com', 'ACCOUNT'), + cookie('.example.com', 'session') + ], + { clearData: rejectingBulkClear() } + ) + + await removeTransplantableCookies(session) + + expect(remove.mock.calls).toEqual([['https://example.com/', 'session']]) expect(set).not.toHaveBeenCalled() }) // Why (STA-4061): reconstructing a removed cookie loses its partition key, and the snapshot // cannot say which cookies had one, so a failed clear must stay failed. it('never reconstructs removed cookies when another removal fails', async () => { - const get = vi - .fn() - .mockResolvedValue([ + const { session, remove, set } = clearSession( + [ cookie('.google.com', 'SID'), cookie('.example.com', 'first', '/one'), cookie('.example.com', 'second', '/two'), cookie('.example.com', 'third', '/three') - ]) - const remove = vi.fn().mockImplementation(async (_url: string, name: string) => { - if (name === 'second') { - throw new Error('store unavailable') + ], + { + clearData: rejectingBulkClear(), + remove: vi.fn().mockImplementation(async (_url: string, name: string) => { + if (name === 'second') { + throw new Error('store unavailable') + } + }) } - }) - const set = vi.fn().mockResolvedValue(undefined) + ) - await expect( - removeAllCookiesExcept({ get, remove }, (existingCookie) => - isNonTransplantableCookieDomain(existingCookie.domain ?? '') - ) - ).rejects.toThrow('the session was left partially cleared') + await expect(removeTransplantableCookies(session)).rejects.toThrow( + 'the session was left partially cleared' + ) expect(remove).toHaveBeenCalledTimes(3) expect(set).not.toHaveBeenCalled() }) it('bounds parallel removals so large cookie jars do not clear serially or fan out', async () => { - const get = vi - .fn() - .mockResolvedValue( - Array.from({ length: 12 }, (_, index) => cookie('.example.com', `${index}`)) - ) let releaseRemovals: (() => void) | undefined const removalsReleased = new Promise((resolve) => { releaseRemovals = resolve }) let active = 0 let maxActive = 0 - const remove = vi.fn().mockImplementation(async () => { - active++ - maxActive = Math.max(maxActive, active) - await removalsReleased - active-- - }) - const set = vi.fn().mockResolvedValue(undefined) + const { session, remove, set } = clearSession( + [ + cookie('.google.com', 'SID'), + ...Array.from({ length: 12 }, (_, index) => cookie('.example.com', `${index}`)) + ], + { + clearData: rejectingBulkClear(), + remove: vi.fn().mockImplementation(async () => { + active++ + maxActive = Math.max(maxActive, active) + await removalsReleased + active-- + }) + } + ) - const clearing = removeAllCookiesExcept({ get, remove }, () => false) + const clearing = removeTransplantableCookies(session) await vi.waitFor(() => expect(remove).toHaveBeenCalledTimes(8)) expect(maxActive).toBe(8) releaseRemovals?.() @@ -289,22 +404,26 @@ describe('removeAllCookiesExcept', () => { }) it('serializes cookies that share Electron removal coordinates', async () => { - const get = vi - .fn() - .mockResolvedValue([ - cookie('.example.com', 'session'), - { ...cookie('example.com', 'session'), hostOnly: true } - ]) let releaseFirst: (() => void) | undefined const firstReleased = new Promise((resolve) => { releaseFirst = resolve }) - const remove = vi - .fn() - .mockImplementationOnce(() => firstReleased) - .mockResolvedValueOnce(undefined) + const { session, remove } = clearSession( + [ + cookie('.google.com', 'SID'), + cookie('.example.com', 'session'), + { ...cookie('example.com', 'session'), hostOnly: true } + ], + { + clearData: rejectingBulkClear(), + remove: vi + .fn() + .mockImplementationOnce(() => firstReleased) + .mockResolvedValueOnce(undefined) + } + ) - const clearing = removeAllCookiesExcept({ get, remove }, () => false) + const clearing = removeTransplantableCookies(session) await vi.waitFor(() => expect(remove).toHaveBeenCalledOnce()) releaseFirst?.() await clearing diff --git a/src/main/browser/browser-cookie-import-policy.ts b/src/main/browser/browser-cookie-import-policy.ts index 27be9512218..6f81407f4a9 100644 --- a/src/main/browser/browser-cookie-import-policy.ts +++ b/src/main/browser/browser-cookie-import-policy.ts @@ -1,4 +1,4 @@ -import type { Cookie, Cookies } from 'electron' +import type { Cookie, Cookies, Session } from 'electron' import { parse as parseDomain } from 'psl' import { mapSettledWithConcurrency } from '../../shared/map-with-concurrency' @@ -59,11 +59,16 @@ export function normalizeCookieImportDomain(domain: string): string | null { // it is copied. Signing in directly inside Orca is the only path that produces a working // session, so an import must never write these cookies and never remove them either — the // live session is always more valuable than anything an import could put in its place. +// Entries must be canonical lowercase ASCII (punycode) registrable domains, never subdomains or +// public suffixes, because clearData derives one excluded origin and matches at that boundary. // Adding a site is one entry here. // youtube.com is deliberately NOT listed: YouTube accepts a transplanted session and re-issues // its cookies via the accounts.youtube.com relay, so excluding it would silently drop imports // users actually asked for. const NON_TRANSPLANTABLE_DOMAINS = ['google.com'] as const +const NON_TRANSPLANTABLE_CLEAR_EXCLUDED_ORIGINS = NON_TRANSPLANTABLE_DOMAINS.map( + (root) => `https://${root}` +) const COOKIE_CLEAR_CONCURRENCY = 8 export function isNonTransplantableCookieDomain(domain: string): boolean { @@ -196,20 +201,49 @@ export async function restoreImportedDomainCookies( } } +// Why (STA-4061): 'set' stays out so the lossy partition-dropping reconstruction cannot return. +export type CookieClearSession = { + cookies: Pick + clearData: Session['clearData'] +} + // Why: Electron cannot round-trip partition identity, so excluded cookies must never be removed. // Why (STA-4061): the same gap forbids rolling a partial clear back. cookies.get() omits // partitionKey and cookies.set() silently drops it, so every reconstruction is a coin flip that // can downgrade a partitioned (CHIPS) cookie into an unpartitioned one — and nothing in the // snapshot says which cookies are at risk. A partially cleared jar is a retryable import failure; // a downgraded cookie is unrecoverable auth-state corruption that survives restart. -export async function removeAllCookiesExcept( - store: Pick, - isExcluded: (cookie: Cookie) => boolean +// Why (STA-4065): the exclusion is module state rather than a parameter so the predicate and the +// origins the bulk clear preserves cannot drift apart — a caller-supplied predicate that disagreed +// with NON_TRANSPLANTABLE_DOMAINS would silently delete a cookie the bulk call is meant to keep. +export async function removeTransplantableCookies( + targetSession: CookieClearSession ): Promise { + const store = targetSession.cookies + const initialCookies = await store.get({}) + if (initialCookies.length === 0) { + return + } + + // Why (STA-4065): measured on Electron 43, excludeOrigins preserves the whole registrable + // family — host, leading-dot, subdomain, and partitioned Google cookies — so one call replaces a + // remove() per cookie even when the jar holds cookies to keep. That is the ordinary case here: + // this import exists for Google, so a Google cookie is usually present. + try { + await targetSession.clearData({ + dataTypes: ['cookies'], + excludeOrigins: NON_TRANSPLANTABLE_CLEAR_EXCLUDED_ORIGINS + }) + return + } catch { + // Why: a rejected bulk clear can still have changed the jar, so the fallback must act on the + // survivors rather than stale removal coordinates from before the attempt. + } + const existingCookies = await store.get({}) const removableGroups = new Map() for (const cookie of existingCookies) { - if (isExcluded(cookie)) { + if (isNonTransplantableCookieDomain(cookie.domain ?? '')) { continue } const domain = cookie.domain ? normalizeCookieDomain(cookie.domain) : null diff --git a/src/main/browser/browser-cookie-import-replacement.test.ts b/src/main/browser/browser-cookie-import-replacement.test.ts index 8b5c07bb057..db70922ccb5 100644 --- a/src/main/browser/browser-cookie-import-replacement.test.ts +++ b/src/main/browser/browser-cookie-import-replacement.test.ts @@ -168,7 +168,7 @@ describe('validated cookie replacement', () => { }) describe('native Chromium integrity-cookie accounting', () => { - let clearStorageDataMock: ReturnType + let clearDataMock: ReturnType let cookiesSetMock: ReturnType let tmpDir: string @@ -180,7 +180,7 @@ describe('native Chromium integrity-cookie accounting', () => { }) clearPendingCookieImportMock.mockClear() setPendingCookieImportMock.mockClear() - clearStorageDataMock = vi.fn().mockResolvedValue(undefined) + clearDataMock = vi.fn().mockResolvedValue(undefined) cookiesSetMock = vi.fn().mockResolvedValue(undefined) sessionFromPartitionMock.mockReset().mockReturnValue({ cookies: { @@ -189,7 +189,7 @@ describe('native Chromium integrity-cookie accounting', () => { remove: vi.fn().mockResolvedValue(undefined), set: cookiesSetMock }, - clearStorageData: clearStorageDataMock + clearData: clearDataMock }) }) @@ -252,7 +252,7 @@ describe('native Chromium integrity-cookie accounting', () => { googleCookiesSkipped: 2, domains: [] }) - expect(clearStorageDataMock).not.toHaveBeenCalled() + expect(clearDataMock).not.toHaveBeenCalled() expect(cookiesSetMock).not.toHaveBeenCalled() expect(setPendingCookieImportMock).not.toHaveBeenCalled() expect(clearPendingCookieImportMock).not.toHaveBeenCalled() diff --git a/src/main/browser/browser-cookie-import.test.ts b/src/main/browser/browser-cookie-import.test.ts index d2d4ffd4836..90514321d54 100644 --- a/src/main/browser/browser-cookie-import.test.ts +++ b/src/main/browser/browser-cookie-import.test.ts @@ -423,7 +423,7 @@ describe('importCookiesFromBrowser Chromium', () => { let cookiesSetMock: ReturnType let cookiesRemoveMock: ReturnType let cookiesFlushStoreMock: ReturnType - let clearStorageDataMock: ReturnType + let clearDataMock: ReturnType let setUserAgentMock: ReturnType beforeEach(() => { @@ -431,7 +431,7 @@ describe('importCookiesFromBrowser Chromium', () => { cookiesSetMock = vi.fn().mockResolvedValue(undefined) cookiesRemoveMock = vi.fn().mockResolvedValue(undefined) cookiesFlushStoreMock = vi.fn().mockResolvedValue(undefined) - clearStorageDataMock = vi.fn().mockResolvedValue(undefined) + clearDataMock = vi.fn().mockResolvedValue(undefined) setUserAgentMock = vi.fn() appGetPathMock.mockReset() appGetPathMock.mockReturnValue(join(tmpDir, 'userData')) @@ -450,7 +450,7 @@ describe('importCookiesFromBrowser Chromium', () => { remove: cookiesRemoveMock, flushStore: cookiesFlushStoreMock }, - clearStorageData: clearStorageDataMock, + clearData: clearDataMock, setUserAgent: setUserAgentMock }) }) @@ -511,7 +511,7 @@ describe('importCookiesFromBrowser Chromium', () => { ['', '-wal', '-shm'].map((suffix) => readFileSync(sourceCookiesPath + suffix)) ).toEqual(sourceFilesBefore) expect(cookiesRemoveMock).not.toHaveBeenCalled() - expect(clearStorageDataMock).not.toHaveBeenCalled() + expect(clearDataMock).not.toHaveBeenCalled() // Why: STA-3514 — imports must never impersonate the source browser; the // session keeps the engine UA the registry set at startup. expect(setUserAgentMock).not.toHaveBeenCalled() diff --git a/src/main/browser/browser-cookie-import.ts b/src/main/browser/browser-cookie-import.ts index 2a7bed19950..3c3a701f953 100644 --- a/src/main/browser/browser-cookie-import.ts +++ b/src/main/browser/browser-cookie-import.ts @@ -80,7 +80,7 @@ import { NON_TRANSPLANTABLE_HOST_KEY_SQL, normalizeCookieDomain, normalizeCookieImportDomain, - removeAllCookiesExcept, + removeTransplantableCookies, replaceCookiesForImportedDomains, restoreImportedDomainCookies, type CookieImportMode @@ -1780,9 +1780,7 @@ export async function importCookiesFromBrowser( // Why: clear stale cookies first; mixing them with the imported set makes sites reject the // session. Non-transplantable families are exempt — nothing was imported for them, and their // live session is the only one that works. - await removeAllCookiesExcept(targetSession.cookies, (cookie) => - isNonTransplantableCookieDomain(cookie.domain ?? '') - ) + await removeTransplantableCookies(targetSession) diag( ` cleared existing session cookies before loading ${decryptedCookies.length} imported cookies` ) diff --git a/src/main/daemon/session.test.ts b/src/main/daemon/session.test.ts index bc90bf5d97f..5769858f2cf 100644 --- a/src/main/daemon/session.test.ts +++ b/src/main/daemon/session.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { SESSION_FORCE_KILL_RETRY_MS, Session } from './session' +import { HeadlessEmulator } from './headless-emulator' import type { SessionState, ShellReadyState } from './types' import type { TuiAgent } from '../../shared/types' @@ -430,6 +431,29 @@ describe('Session', () => { expect(session.getSnapshot()?.snapshotAnsi).not.toContain('orca-shell-ready') }) + it.each([ + ['after the ready marker', ['\x1b]777;orca-shell-ready\x07', '\x1b[?2004hfish> ']], + ['after the ESC introducer', ['\x1b]777;orca-shell-ready\x07\x1b', '[?2004hfish> ']] + ])('preserves Fish bracketed-paste output split %s', (_boundary, chunks) => { + createSession({ shellReadySupported: true }) + const received: string[] = [] + session.attachClient({ + onData: (data) => received.push(data), + onExit: () => {} + }) + + for (const chunk of chunks) { + subprocess.simulateData(chunk) + } + + const output = received.join('') + expect(output).toBe('\x1b[?2004hfish> ') + const rendered = new HeadlessEmulator({ cols: 80, rows: 24 }) + expect(rendered.writeSync(output)).toBe(true) + expect(rendered.getVisibleLines().join('\n')).not.toContain('[?2004h') + rendered.dispose() + }) + it('publishes an absolute output sequence with live snapshots', () => { createSession() subprocess.simulateData('first') diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index 81343b39744..3771b3194b5 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -8,17 +8,11 @@ import { import { isValidPtySize, normalizePtySize } from './daemon-pty-size' import { PostReadyFlushGate } from './post-ready-flush-gate' import { - createShellReadyScanState, - drainShellReadyHeldBytes, - scanForShellReady, - type ShellReadyScanState -} from '../shell-ready-marker-scanner' -import { - createShellStartupIdentityScanState, - drainShellStartupIdentityHeldBytes, - scanForShellStartupIdentity, - type ShellStartupIdentityScanState -} from '../shell-startup-identity-scanner' + createShellStartupOutputScanState, + drainShellStartupOutputScanState, + scanShellStartupOutput, + type ShellStartupOutputScanState +} from '../shell-startup-output-scanner' import { createShellPromptReadinessProbe, type ShellPromptReadinessProbe @@ -136,8 +130,7 @@ export class Session { private preReadyStdinQueue: string[] = [] private releaseStartupDeviceAttributesResponder: (() => void) | null = null private startupDeviceAttributesQueryFilter: StartupDeviceAttributesQueryFilter | null = null - private shellReadyScanState: ShellReadyScanState | null = null - private shellStartupIdentityScanState: ShellStartupIdentityScanState | null = null + private shellStartupOutputScanState: ShellStartupOutputScanState | null = null private shellStartupPid: number | null = null private shellPromptReadinessProbe: ShellPromptReadinessProbe | null = null private shellReadyTimer: ReturnType | null = null @@ -184,8 +177,7 @@ export class Session { if (opts.shellReadySupported) { this._shellState = 'pending' - this.shellReadyScanState = createShellReadyScanState() - this.shellStartupIdentityScanState = createShellStartupIdentityScanState() + this.shellStartupOutputScanState = createShellStartupOutputScanState() // Why: `write` queues everything until the ready marker, including the renderer's DA1 // reply — and a shell that withholds its first prompt until DA1 is answered (fish) then // never emits the marker that would release it. Answer from the daemon, past the queue. @@ -636,7 +628,7 @@ export class Session { } this.shellPromptReadinessProbe?.dispose() this.shellPromptReadinessProbe = null - this.shellReadyScanState = null + this.shellStartupOutputScanState = null this.preReadyStdinQueue = [] this.postReadyFlushGate.clear() this.disposeSubprocessHandle() @@ -682,18 +674,13 @@ export class Session { } let releaseStartupDeviceAttributes = false - if (this._shellState === 'pending' && this.shellStartupIdentityScanState) { - const scanned = scanForShellStartupIdentity(this.shellStartupIdentityScanState, data) + if (this._shellState === 'pending' && this.shellStartupOutputScanState) { + const scanned = scanShellStartupOutput(this.shellStartupOutputScanState, data) data = scanned.output if (scanned.shellPid) { this.shellStartupPid = scanned.shellPid - this.shellStartupIdentityScanState = null } - } - if (this._shellState === 'pending' && this.shellReadyScanState) { - const scanned = scanForShellReady(this.shellReadyScanState, data) - data = scanned.output - if (scanned.matched) { + if (scanned.ready) { this.transitionToReady(scanned.postMarkerBytesObserved) releaseStartupDeviceAttributes = true } @@ -773,20 +760,11 @@ export class Session { } private releaseHeldShellReadyBytes(): string { - if (!this.shellReadyScanState && !this.shellStartupIdentityScanState) { + if (!this.shellStartupOutputScanState) { return '' } - let heldBytes = this.shellStartupIdentityScanState - ? drainShellStartupIdentityHeldBytes(this.shellStartupIdentityScanState) - : '' - this.shellStartupIdentityScanState = null - if (this.shellReadyScanState && heldBytes) { - heldBytes = scanForShellReady(this.shellReadyScanState, heldBytes).output - } - if (this.shellReadyScanState) { - heldBytes += drainShellReadyHeldBytes(this.shellReadyScanState) - } - this.shellReadyScanState = null + const heldBytes = drainShellStartupOutputScanState(this.shellStartupOutputScanState) + this.shellStartupOutputScanState = null // Why: scanning strips marker bytes before fan-out; if readiness never completes, release any held prefix before timeout/exit discards it. this.startupIngress.accept(heldBytes) return heldBytes @@ -813,8 +791,7 @@ export class Session { private transitionToReady(postMarkerBytesObserved = false): void { this._shellState = 'ready' - this.shellReadyScanState = null - this.shellStartupIdentityScanState = null + this.shellStartupOutputScanState = null this.shellPromptReadinessProbe?.dispose() this.shellPromptReadinessProbe = null if (this.shellReadyTimer) { diff --git a/src/main/daemon/shell-ready.test.ts b/src/main/daemon/shell-ready.test.ts index b7299343579..2d931e7206e 100644 --- a/src/main/daemon/shell-ready.test.ts +++ b/src/main/daemon/shell-ready.test.ts @@ -6,6 +6,12 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync import type * as ShellReadyModule from './shell-ready' import { getZshShellReadyMarkerRegistrationBlock } from '../shell-templates' import { fishRequirementViolation, resolveFishBinary } from '../../shared/fish-binary-requirement' +import { + createShellStartupOutputScanState, + drainShellStartupOutputScanState, + scanShellStartupOutput +} from '../shell-startup-output-scanner' +import { HeadlessEmulator } from './headless-emulator' async function importFreshShellReady(): Promise { vi.resetModules() @@ -296,6 +302,8 @@ describePosix('daemon shell-ready launch config', () => { } }) let output = '' + let scannedOutput = '' + const startupScanState = createShellStartupOutputScanState() let commandWritten = false let erasureProbeWritten = false let queryCarry = '' @@ -321,6 +329,7 @@ describePosix('daemon shell-ready launch config', () => { }, 50) proc.onData((chunk) => { output += chunk + scannedOutput += scanShellStartupOutput(startupScanState, chunk).output // Why: fish stalls its first prompt 10s waiting on these and re-queries // each prompt, so answer every occurrence — an unanswered query makes // fish swallow the post-marker command as its reply. @@ -350,9 +359,15 @@ describePosix('daemon shell-ready launch config', () => { clearTimeout(deadline) clearInterval(sentinelPoll) proc.kill() + scannedOutput += drainShellStartupOutputScanState(startupScanState) expect(output).toContain(SHELL_READY_MARKER_OUTPUT) expect(output.split(SHELL_READY_MARKER_OUTPUT)).toHaveLength(2) + expect(scannedOutput).toBe(output.replace(SHELL_READY_MARKER_OUTPUT, '')) + const rendered = new HeadlessEmulator({ cols: 80, rows: 24 }) + expect(rendered.writeSync(scannedOutput)).toBe(true) + expect(rendered.getVisibleLines().join('\n')).not.toContain('[?2004h') + rendered.dispose() expect(existsSync(sentinel)).toBe(true) // Why: asserts the erase directly rather than inferring it from the marker // count, which only holds once enough prompts have been drawn to expose it. diff --git a/src/main/providers/local-pty-provider.test.ts b/src/main/providers/local-pty-provider.test.ts index 889d75ac7ae..178b60d6caa 100644 --- a/src/main/providers/local-pty-provider.test.ts +++ b/src/main/providers/local-pty-provider.test.ts @@ -658,6 +658,23 @@ describe('LocalPtyProvider', () => { } }) + it.each([ + ['after the ready marker', ['\x1b]777;orca-shell-ready\x07', '\x1b[?2004hfish> ']], + ['after the ESC introducer', ['\x1b]777;orca-shell-ready\x07\x1b', '[?2004hfish> ']] + ])('preserves Fish bracketed-paste output split %s', async (_boundary, chunks) => { + process.env.SHELL = '/usr/bin/fish' + const received: string[] = [] + provider.configure({ onData: (_id, data) => received.push(data) }) + + await provider.spawn({ cols: 80, rows: 24, command: 'printf ready' }) + const dataCallback = mockProc.onData.mock.calls[0]?.[0] as (data: string) => void + for (const chunk of chunks) { + dataCallback(chunk) + } + + expect(received.join('')).toBe('\x1b[?2004hfish> ') + }) + it('releases held marker-prefix bytes when local shell readiness times out', async () => { vi.useFakeTimers() const onData = vi.fn() diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index 6601a56ac31..3026ded42a3 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -29,9 +29,6 @@ import { prepareMacosTccLoginShell } from './macos-tcc-login-shell' import { getMarkerlessShellLaunchConfig, getShellReadyLaunchConfig, - createShellReadyScanState, - drainShellReadyHeldBytes, - scanForShellReady, writeStartupCommandWhenShellReady, STARTUP_COMMAND_READY_MAX_WAIT_MS } from './local-pty-shell-ready' @@ -76,10 +73,10 @@ import { readPtySlavePath } from '../../shared/pty-slave-line-discipline-echo' import { - createShellStartupIdentityScanState, - drainShellStartupIdentityHeldBytes, - scanForShellStartupIdentity -} from '../shell-startup-identity-scanner' + createShellStartupOutputScanState, + drainShellStartupOutputScanState, + scanShellStartupOutput +} from '../shell-startup-output-scanner' import { createShellPromptReadinessProbe, type ShellPromptReadinessProbe @@ -961,11 +958,8 @@ export class LocalPtyProvider implements IPtyProvider { let shellReadyTimeout: ReturnType | null = null let shellStartupPid: number | null = null let shellPromptReadinessProbe: ShellPromptReadinessProbe | null = null - let shellStartupIdentityScanState = shellReadyLaunch?.supportsReadyMarker - ? createShellStartupIdentityScanState() - : null - const shellReadyScanState = shellReadyLaunch?.supportsReadyMarker - ? createShellReadyScanState() + let shellStartupOutputScanState = shellReadyLaunch?.supportsReadyMarker + ? createShellStartupOutputScanState() : null const shellReadyPromise = args.command ? new Promise((resolve) => { @@ -987,23 +981,17 @@ export class LocalPtyProvider implements IPtyProvider { resolve(signal) } const releaseHeldShellReadyBytes = (): void => { - if (!shellReadyScanState) { + if (!shellStartupOutputScanState) { return } - let heldBytes = shellStartupIdentityScanState - ? drainShellStartupIdentityHeldBytes(shellStartupIdentityScanState) - : '' - shellStartupIdentityScanState = null - if (heldBytes) { - heldBytes = scanForShellReady(shellReadyScanState, heldBytes).output - } - heldBytes += drainShellReadyHeldBytes(shellReadyScanState) + const heldBytes = drainShellStartupOutputScanState(shellStartupOutputScanState) + shellStartupOutputScanState = null if (heldBytes.length === 0) { return } startupIngress.accept(heldBytes) } - if (shellReadyScanState) { + if (shellStartupOutputScanState) { shellPromptReadinessProbe = createShellPromptReadinessProbe({ slavePath: readPtySlavePath(proc), shellPath, @@ -1048,18 +1036,13 @@ export class LocalPtyProvider implements IPtyProvider { const disposables: { dispose: () => void }[] = [] const onDataDisposable = proc.onData((rawData) => { let data = rawData - if (shellStartupIdentityScanState && resolveShellReady) { - const scanned = scanForShellStartupIdentity(shellStartupIdentityScanState, data) + if (shellStartupOutputScanState && resolveShellReady) { + const scanned = scanShellStartupOutput(shellStartupOutputScanState, data) data = scanned.output if (scanned.shellPid) { shellStartupPid = scanned.shellPid - shellStartupIdentityScanState = null } - } - if (shellReadyScanState && resolveShellReady) { - const scanned = scanForShellReady(shellReadyScanState, data) - data = scanned.output - if (scanned.matched) { + if (scanned.ready) { finishShellReady({ postMarkerBytesObserved: scanned.postMarkerBytesObserved }) } } diff --git a/src/main/shell-ready-marker-scanner.ts b/src/main/shell-ready-marker-scanner.ts index 0e6250e1c4e..a8b58499852 100644 --- a/src/main/shell-ready-marker-scanner.ts +++ b/src/main/shell-ready-marker-scanner.ts @@ -11,6 +11,10 @@ export type ShellReadyScanResult = { postMarkerBytesObserved: boolean } +type ShellReadyBoundaryScanResult = ShellReadyScanResult & { + postMarkerOutputIndex: number | null +} + export function createShellReadyScanState(): ShellReadyScanState { return { matchPos: 0, heldBytes: '' } } @@ -22,7 +26,10 @@ export function drainShellReadyHeldBytes(state: ShellReadyScanState): string { return heldBytes } -export function scanForShellReady(state: ShellReadyScanState, data: string): ShellReadyScanResult { +export function scanForShellReadyBoundary( + state: ShellReadyScanState, + data: string +): ShellReadyBoundaryScanResult { let output = '' for (let i = 0; i < data.length; i += 1) { @@ -49,7 +56,8 @@ export function scanForShellReady(state: ShellReadyScanState, data: string): She return { output: output + remaining, matched: true, - postMarkerBytesObserved: remaining.length > 0 + postMarkerBytesObserved: remaining.length > 0, + postMarkerOutputIndex: output.length } } else { output += state.heldBytes @@ -64,5 +72,15 @@ export function scanForShellReady(state: ShellReadyScanState, data: string): She } } - return { output, matched: false, postMarkerBytesObserved: false } + return { + output, + matched: false, + postMarkerBytesObserved: false, + postMarkerOutputIndex: null + } +} + +export function scanForShellReady(state: ShellReadyScanState, data: string): ShellReadyScanResult { + const { postMarkerOutputIndex: _, ...result } = scanForShellReadyBoundary(state, data) + return result } diff --git a/src/main/shell-startup-output-scanner.test.ts b/src/main/shell-startup-output-scanner.test.ts new file mode 100644 index 00000000000..1a0930b9395 --- /dev/null +++ b/src/main/shell-startup-output-scanner.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from 'vitest' +import { + createShellStartupOutputScanState, + drainShellStartupOutputScanState, + scanShellStartupOutput +} from './shell-startup-output-scanner' + +const READY_MARKER = '\x1b]777;orca-shell-ready\x07' +const IDENTITY_MARKER = '\x1b]777;orca-shell-start:12345\x07' + +function scanChunks(chunks: string[]): { + output: string + shellPid: number | null + ready: boolean + postMarkerBytesObserved: boolean +} { + const state = createShellStartupOutputScanState() + let output = '' + let shellPid: number | null = null + let ready = false + let postMarkerBytesObserved = false + + for (const chunk of chunks) { + const scanned = scanShellStartupOutput(state, chunk) + output += scanned.output + shellPid ??= scanned.shellPid + ready ||= scanned.ready + postMarkerBytesObserved ||= scanned.postMarkerBytesObserved + } + return { output, shellPid, ready, postMarkerBytesObserved } +} + +function expectEverySplitToProduce( + input: string, + expected: Omit, 'postMarkerBytesObserved'> +): void { + for (let split = 0; split <= input.length; split += 1) { + const { postMarkerBytesObserved: _, ...scanned } = scanChunks([ + input.slice(0, split), + input.slice(split) + ]) + expect(scanned, `split ${split}`).toEqual(expected) + } +} + +describe('shell startup output scanner', () => { + it.each([ + ['after the ready marker', [READY_MARKER, '\x1b[?2004hfish> ']], + ['after the ESC introducer', [`${READY_MARKER}\x1b`, '[?2004hfish> ']] + ])('preserves Fish output split %s', (_boundary, chunks) => { + const state = createShellStartupOutputScanState() + let output = '' + + for (const chunk of chunks) { + output += scanShellStartupOutput(state, chunk).output + } + + expect(output).toBe('\x1b[?2004hfish> ') + }) + + it('preserves every post-ready byte at every chunk boundary', () => { + expectEverySplitToProduce(`${READY_MARKER}${IDENTITY_MARKER}prompt`, { + output: `${IDENTITY_MARKER}prompt`, + shellPid: null, + ready: true + }) + expectEverySplitToProduce(`${READY_MARKER}\x1b[?2004hfish> `, { + output: '\x1b[?2004hfish> ', + shellPid: null, + ready: true + }) + }) + + it('strips pre-ready markers at every chunk boundary', () => { + expectEverySplitToProduce(`${IDENTITY_MARKER}${READY_MARKER}prompt`, { + output: 'prompt', + shellPid: 12345, + ready: true + }) + }) + + it('strips identity and readiness markers from one chunk', () => { + const state = createShellStartupOutputScanState() + const scanned = scanShellStartupOutput(state, `${IDENTITY_MARKER}${READY_MARKER}prompt`) + + expect(scanned).toEqual({ + output: 'prompt', + shellPid: 12345, + ready: true, + postMarkerBytesObserved: true + }) + }) + + it('counts an identity-held byte as post-marker output', () => { + const state = createShellStartupOutputScanState() + + expect(scanShellStartupOutput(state, `${READY_MARKER}\x1b`)).toEqual({ + output: '\x1b', + shellPid: null, + ready: true, + postMarkerBytesObserved: true + }) + }) + + it('drains every incomplete scanner prefix in byte order', () => { + const state = createShellStartupOutputScanState() + expect(scanShellStartupOutput(state, '\x1b]777;orca-shell-st').output).toBe('') + + expect(drainShellStartupOutputScanState(state)).toBe('\x1b]777;orca-shell-st') + }) + + it('drains simultaneous identity and readiness prefixes in input order', () => { + const state = createShellStartupOutputScanState() + expect(scanShellStartupOutput(state, '\x1b]777;orca-shell-st').output).toBe('') + expect(scanShellStartupOutput(state, '\x1b]777;orca-shell-rea').output).toBe('') + + expect(drainShellStartupOutputScanState(state)).toBe( + '\x1b]777;orca-shell-st\x1b]777;orca-shell-rea' + ) + }) +}) diff --git a/src/main/shell-startup-output-scanner.ts b/src/main/shell-startup-output-scanner.ts new file mode 100644 index 00000000000..b87626c24eb --- /dev/null +++ b/src/main/shell-startup-output-scanner.ts @@ -0,0 +1,68 @@ +import { + createShellReadyScanState, + drainShellReadyHeldBytes, + scanForShellReadyBoundary, + type ShellReadyScanState +} from './shell-ready-marker-scanner' +import { + createShellStartupIdentityScanState, + drainShellStartupIdentityHeldBytes, + scanForShellStartupIdentity, + type ShellStartupIdentityScanState +} from './shell-startup-identity-scanner' + +export type ShellStartupOutputScanState = { + ready: ShellReadyScanState + identity: ShellStartupIdentityScanState | null +} + +export type ShellStartupOutputScanResult = { + output: string + shellPid: number | null + ready: boolean + postMarkerBytesObserved: boolean +} + +export function createShellStartupOutputScanState(): ShellStartupOutputScanState { + return { + ready: createShellReadyScanState(), + identity: createShellStartupIdentityScanState() + } +} + +export function scanShellStartupOutput( + state: ShellStartupOutputScanState, + data: string +): ShellStartupOutputScanResult { + const readiness = scanForShellReadyBoundary(state.ready, data) + const postMarkerOutputIndex = readiness.postMarkerOutputIndex ?? readiness.output.length + const identityInput = readiness.output.slice(0, postMarkerOutputIndex) + let shellPid: number | null = null + let output = identityInput + if (state.identity) { + const identity = scanForShellStartupIdentity(state.identity, identityInput) + output = identity.output + shellPid = identity.shellPid + if (shellPid) { + state.identity = null + } + } + + if (readiness.matched && state.identity) { + output += drainShellStartupIdentityHeldBytes(state.identity) + state.identity = null + } + output += readiness.output.slice(postMarkerOutputIndex) + return { + output, + shellPid, + ready: readiness.matched, + postMarkerBytesObserved: readiness.postMarkerBytesObserved + } +} + +export function drainShellStartupOutputScanState(state: ShellStartupOutputScanState): string { + const output = state.identity ? drainShellStartupIdentityHeldBytes(state.identity) : '' + state.identity = null + return output + drainShellReadyHeldBytes(state.ready) +} diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index 6dd3cd8a7fc..e7c09df95de 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -27,19 +27,13 @@ import { } from '../shared/cross-platform-path' import { splitWorktreeId } from '../shared/worktree-id' import { PhysicalExitTracker } from '../shared/physical-exit-tracker' +import { SHELL_READY_MARKER_PREFIX } from '../main/shell-ready-marker-scanner' import { - createShellReadyScanState, - drainShellReadyHeldBytes, - scanForShellReady, - SHELL_READY_MARKER_PREFIX, - type ShellReadyScanState -} from '../main/shell-ready-marker-scanner' -import { - createShellStartupIdentityScanState, - drainShellStartupIdentityHeldBytes, - scanForShellStartupIdentity, - type ShellStartupIdentityScanState -} from '../main/shell-startup-identity-scanner' + createShellStartupOutputScanState, + drainShellStartupOutputScanState, + scanShellStartupOutput, + type ShellStartupOutputScanState +} from '../main/shell-startup-output-scanner' import { createShellPromptReadinessProbe, type ShellPromptReadinessProbe @@ -196,8 +190,7 @@ type ManagedStartupCommand = { providerDelivery: boolean delivered: boolean waitForShellReady: boolean - scanState: ShellReadyScanState | null - identityScanState: ShellStartupIdentityScanState | null + outputScanState: ShellStartupOutputScanState | null shellPid: number | null promptProbe: ShellPromptReadinessProbe | null timer: ReturnType | null @@ -658,17 +651,11 @@ export class PtyHandler { } private drainStartupScanBytes(startup: ManagedStartupCommand): string { - let heldBytes = startup.identityScanState - ? drainShellStartupIdentityHeldBytes(startup.identityScanState) - : '' - startup.identityScanState = null - if (startup.scanState && heldBytes) { - heldBytes = scanForShellReady(startup.scanState, heldBytes).output - } - if (startup.scanState) { - heldBytes += drainShellReadyHeldBytes(startup.scanState) - startup.scanState = null + if (!startup.outputScanState) { + return '' } + const heldBytes = drainShellStartupOutputScanState(startup.outputScanState) + startup.outputScanState = null return heldBytes } @@ -767,18 +754,13 @@ export class PtyHandler { } managed.pty.onData((data: string) => { const startup = managed.startupCommand - if (startup?.identityScanState && !startup.delivered) { - const scanned = scanForShellStartupIdentity(startup.identityScanState, data) + if (startup?.waitForShellReady && startup.outputScanState && !startup.delivered) { + const scanned = scanShellStartupOutput(startup.outputScanState, data) data = scanned.output if (scanned.shellPid) { startup.shellPid = scanned.shellPid - startup.identityScanState = null } - } - if (startup?.waitForShellReady && startup.scanState && !startup.delivered) { - const scanned = scanForShellReady(startup.scanState, data) - data = scanned.output - if (scanned.matched) { + if (scanned.ready) { if (startup.providerDelivery) { this.scheduleStartupCommandResolution(managed, STARTUP_COMMAND_WRITE_DELAY_MS) } else { @@ -1644,13 +1626,9 @@ export class PtyHandler { providerDelivery: shouldProviderDeliverCommand, delivered: false, waitForShellReady: shellLaunch.env.ORCA_SHELL_READY_MARKER === '1', - scanState: + outputScanState: shellLaunch.env.ORCA_SHELL_READY_MARKER === '1' - ? createShellReadyScanState() - : null, - identityScanState: - shellLaunch.env.ORCA_SHELL_READY_MARKER === '1' - ? createShellStartupIdentityScanState() + ? createShellStartupOutputScanState() : null, shellPid: null, promptProbe: null, diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 643b6497e5d..0795d6221a3 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -1416,7 +1416,9 @@ function App(): React.JSX.Element { hideWorkspacesFromOtherDevices, alwaysShowDefaultBranchWorkspace, showDotfilesByWorktree, - filterRepoIds, + // Why: the store keeps this readonly for identity stability, but PersistedUI crosses to + // main, which owns a mutable array — copy at the boundary rather than widening the wire type. + filterRepoIds: [...filterRepoIds], // Why (#9002): activeView is deliberately NOT included here. It used to // ride this same 150ms writer (#8265), which meant every top-level view // switch scheduled a full durable-state save. The narrow preference diff --git a/src/renderer/src/components/automations/AutomationEditorDialog.tsx b/src/renderer/src/components/automations/AutomationEditorDialog.tsx index 90eb5a9df47..6ac0c7bc66c 100644 --- a/src/renderer/src/components/automations/AutomationEditorDialog.tsx +++ b/src/renderer/src/components/automations/AutomationEditorDialog.tsx @@ -62,7 +62,7 @@ type AutomationEditorDialogProps = { canSave: boolean createTarget: AutomationCreateTarget repos: readonly Repo[] - projectHostSetups: ProjectHostSetup[] + projectHostSetups: readonly ProjectHostSetup[] automationYamlHooksByRepoKey: Record getAutomationHooksCacheKey: (repoId: string) => string repoMap: Map diff --git a/src/renderer/src/components/automations/AutomationEditorDialogFooter.tsx b/src/renderer/src/components/automations/AutomationEditorDialogFooter.tsx index ff5a67cc41b..160dac72d8b 100644 --- a/src/renderer/src/components/automations/AutomationEditorDialogFooter.tsx +++ b/src/renderer/src/components/automations/AutomationEditorDialogFooter.tsx @@ -32,7 +32,7 @@ type AutomationEditorDialogFooterProps = { isSaving: boolean canSave: boolean repos: readonly Repo[] - projectHostSetups: ProjectHostSetup[] + projectHostSetups: readonly ProjectHostSetup[] automationYamlHooksByRepoKey: Record getAutomationHooksCacheKey: (repoId: string) => string repoMap: Map diff --git a/src/renderer/src/components/automations/AutomationListExternalRows.tsx b/src/renderer/src/components/automations/AutomationListExternalRows.tsx index 25a9f137397..8cd712b09c2 100644 --- a/src/renderer/src/components/automations/AutomationListExternalRows.tsx +++ b/src/renderer/src/components/automations/AutomationListExternalRows.tsx @@ -36,6 +36,8 @@ import { AUTOMATIONS_TABLE_ROW_SELECTED_CLASS } from './automations-table-layout' import { isPortaledRowMenuClick, isRowActivationKey } from './automation-list-row-interaction' +import { getExternalAutomationLastRunSnapshot } from './automation-list-last-run' +import { AutomationListLastRunCell } from './AutomationListLastRunCell' import { AutomationListStatusCell } from './AutomationListStatusCell' import { translate } from '@/i18n/i18n' @@ -85,6 +87,7 @@ export function AutomationListExternalRows({ const nextRunLabel = entry.job.enabled ? formatExternalDate(entry.job.nextRunAt, relativeNow) : translate('auto.components.automations.AutomationsPage.paused', 'Paused') + const lastRunSnapshot = getExternalAutomationLastRunSnapshot(entry.job) return ( @@ -124,6 +127,7 @@ export function AutomationListExternalRows({ {nextRunLabel} + {providerLabel} diff --git a/src/renderer/src/components/automations/AutomationListFilterMenu.tsx b/src/renderer/src/components/automations/AutomationListFilterMenu.tsx new file mode 100644 index 00000000000..54efbc819a0 --- /dev/null +++ b/src/renderer/src/components/automations/AutomationListFilterMenu.tsx @@ -0,0 +1,192 @@ +import React from 'react' +import { ListFilter, X } from 'lucide-react' +import { Button } from '@/components/ui/button' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuLabel, + DropdownMenuRadioGroup, + DropdownMenuRadioItem, + DropdownMenuSeparator, + DropdownMenuTrigger +} from '@/components/ui/dropdown-menu' +import { translate } from '@/i18n/i18n' +import { + countAutomationListFilters, + EMPTY_AUTOMATION_LIST_FILTER, + type AutomationListFilter, + type AutomationListLastRunFilter, + type AutomationListStatusFilter +} from './automation-list-view' + +function FilterPill({ + label, + value, + onClear +}: { + label: string + value: string + onClear: () => void +}): React.JSX.Element { + return ( + + {label}: + {value} + + + ) +} + +export function AutomationListFilterPills({ + filter, + onChange +}: { + filter: AutomationListFilter + onChange: (next: AutomationListFilter) => void +}): React.JSX.Element | null { + const statusLabel = translate('auto.components.automations.AutomationsPage.tableStatus', 'Status') + const lastRunLabel = translate( + 'auto.components.automations.AutomationsPage.tableLastRun', + 'Last run' + ) + const statusValueLabel = + filter.status === 'enabled' + ? translate('auto.components.automations.AutomationDetail.eaa02014f8', 'Enabled') + : filter.status === 'paused' + ? translate('auto.components.automations.AutomationDetail.b09b2384fd', 'Paused') + : null + const lastRunValueLabel = + filter.lastRun === 'failed' + ? translate('auto.components.automations.AutomationListFilterMenu.failed', 'Failed') + : filter.lastRun === 'succeeded' + ? translate('auto.components.automations.AutomationListFilterMenu.succeeded', 'Succeeded') + : filter.lastRun === 'never' + ? translate('auto.components.automations.AutomationListFilterMenu.neverRan', 'Never ran') + : null + if (!statusValueLabel && !lastRunValueLabel) { + return null + } + return ( + <> + {statusValueLabel ? ( + onChange({ ...filter, status: 'all' })} + /> + ) : null} + {lastRunValueLabel ? ( + onChange({ ...filter, lastRun: 'all' })} + /> + ) : null} + + ) +} + +export function AutomationListFilterMenu({ + filter, + onChange +}: { + filter: AutomationListFilter + onChange: (next: AutomationListFilter) => void +}): React.JSX.Element { + const activeCount = countAutomationListFilters(filter) + const statusLabel = translate('auto.components.automations.AutomationsPage.tableStatus', 'Status') + const lastRunLabel = translate( + 'auto.components.automations.AutomationsPage.tableLastRun', + 'Last run' + ) + + return ( + + + + + + {statusLabel} + + onChange({ ...filter, status: value as AutomationListStatusFilter }) + } + > + + {translate('auto.components.automations.AutomationListFilterMenu.all', 'All')} + + + {translate('auto.components.automations.AutomationDetail.eaa02014f8', 'Enabled')} + + + {translate('auto.components.automations.AutomationDetail.b09b2384fd', 'Paused')} + + + + {lastRunLabel} + + onChange({ ...filter, lastRun: value as AutomationListLastRunFilter }) + } + > + + {translate('auto.components.automations.AutomationListFilterMenu.all', 'All')} + + + {translate('auto.components.automations.AutomationListFilterMenu.failed', 'Failed')} + + + {translate( + 'auto.components.automations.AutomationListFilterMenu.succeeded', + 'Succeeded' + )} + + + {translate( + 'auto.components.automations.AutomationListFilterMenu.neverRan', + 'Never ran' + )} + + + {activeCount > 0 ? ( + <> + + onChange(EMPTY_AUTOMATION_LIST_FILTER)}> + {translate( + 'auto.components.automations.AutomationListFilterMenu.clear', + 'Clear filters' + )} + + + ) : null} + + + ) +} diff --git a/src/renderer/src/components/automations/AutomationListLastRunCell.tsx b/src/renderer/src/components/automations/AutomationListLastRunCell.tsx new file mode 100644 index 00000000000..6b5ddf42d9c --- /dev/null +++ b/src/renderer/src/components/automations/AutomationListLastRunCell.tsx @@ -0,0 +1,37 @@ +import React from 'react' +import { cn } from '@/lib/utils' +import { + formatAutomationLastRunCell, + type AutomationLastRunSnapshot +} from './automation-list-last-run' + +export function AutomationListLastRunCell({ + snapshot, + now +}: { + snapshot: AutomationLastRunSnapshot + now: number +}): React.JSX.Element { + const cell = formatAutomationLastRunCell(snapshot, now) + const failed = cell.tone === 'failed' + return ( + + {cell.tone !== 'never' ? ( + + ) +} diff --git a/src/renderer/src/components/automations/AutomationListLocalRows.test.tsx b/src/renderer/src/components/automations/AutomationListLocalRows.test.tsx index b6c41db55af..221e0e45142 100644 --- a/src/renderer/src/components/automations/AutomationListLocalRows.test.tsx +++ b/src/renderer/src/components/automations/AutomationListLocalRows.test.tsx @@ -6,11 +6,13 @@ import userEvent from '@testing-library/user-event' import { afterEach, describe, expect, it, vi } from 'vitest' import type { Automation, + AutomationRun, ExternalAutomationJob, ExternalAutomationManager } from '../../../../shared/automations-types' import { AutomationListLocalRows } from './AutomationListLocalRows' import { AutomationListExternalRows } from './AutomationListExternalRows' +import { indexLatestAutomationRuns } from './automation-list-last-run' // Why: Tooltip needs a provider in the app; stub so rows render standalone. vi.mock('@/components/ui/tooltip', () => ({ @@ -84,16 +86,42 @@ function makeExternalManager(): ExternalAutomationManager { } } +function makeRun(overrides: Partial = {}): AutomationRun { + return { + id: 'run-1', + automationId: 'automation-1', + title: 'test', + scheduledFor: 1, + status: 'dispatch_failed', + trigger: 'scheduled', + workspaceId: 'worktree-1', + sessionKind: 'terminal', + chatSessionId: null, + terminalSessionId: null, + terminalPaneKey: null, + terminalPtyId: null, + outputSnapshot: null, + precheckResult: null, + usage: null, + error: 'boom', + startedAt: Date.now() - 8 * 60 * 60 * 1000, + dispatchedAt: Date.now() - 8 * 60 * 60 * 1000, + createdAt: Date.now() - 8 * 60 * 60 * 1000, + ...overrides + } +} + function renderLocalRows(handlers: { onSelect: (automationId: string) => void onDelete?: (automation: Automation) => void + runs?: AutomationRun[] }) { return render( { expect(onSelect).not.toHaveBeenCalled() }) + it('shows last-run status and relative time', () => { + renderLocalRows({ onSelect: vi.fn(), runs: [makeRun()] }) + expect(screen.getByText('Failed 8h ago')).toBeTruthy() + }) + it('still selects when the local row itself is clicked', async () => { const onSelect = vi.fn() const { container } = renderLocalRows({ onSelect }) diff --git a/src/renderer/src/components/automations/AutomationListLocalRows.tsx b/src/renderer/src/components/automations/AutomationListLocalRows.tsx index 97ec2c7bb5f..93019241d63 100644 --- a/src/renderer/src/components/automations/AutomationListLocalRows.tsx +++ b/src/renderer/src/components/automations/AutomationListLocalRows.tsx @@ -31,6 +31,8 @@ import type { ProjectHostSetup, Repo, Worktree } from '../../../../shared/types' import type { RuntimeStatus } from '../../../../shared/runtime-types' import type { TaskSourceHostAvailability } from '../task-source-context-summary' import type { AutomationHostTarget } from './automation-host-client' +import { getLocalAutomationLastRunSnapshot } from './automation-list-last-run' +import { AutomationListLastRunCell } from './AutomationListLastRunCell' import { formatAutomationDateTimeWithRelative } from './automation-page-parts' import { getAutomationTargetAvailability } from './automation-target-availability' import { getAgentLabel } from './automation-draft-model' @@ -48,7 +50,7 @@ export function AutomationListLocalRows({ automations, selectedId, isSelectedLocal, - runs, + lastRunByAutomationId, relativeNow, repoMap, worktreeMap, @@ -67,7 +69,7 @@ export function AutomationListLocalRows({ automations: readonly Automation[] selectedId: string | null | undefined isSelectedLocal: boolean - runs: readonly AutomationRun[] + lastRunByAutomationId: ReadonlyMap relativeNow: number repoMap: ReadonlyMap worktreeMap: ReadonlyMap @@ -86,19 +88,6 @@ export function AutomationListLocalRows({ onToggle: (automation: Automation) => void onDelete: (automation: Automation) => void }): React.JSX.Element { - // Why: one pass over runs instead of a full scan per rendered automation — - // this list re-renders on the relativeNow timer. - const lastRunByAutomationId = React.useMemo(() => { - const latest = new Map() - for (const run of runs) { - const existing = latest.get(run.automationId) - if (!existing || run.createdAt > existing.createdAt) { - latest.set(run.automationId, run) - } - } - return latest - }, [runs]) - return ( <> {automations.map((automation) => { @@ -132,6 +121,7 @@ export function AutomationListLocalRows({ ? (hostLabelById.get(hostId) ?? getExecutionHostLabel(hostId)) : getLocalExecutionHostLabel() const lastRun = lastRunByAutomationId.get(automation.id) + const lastRunSnapshot = getLocalAutomationLastRunSnapshot(automation, lastRun) const lastRunCost = lastRun?.usage?.status === 'known' ? formatAutomationCost(lastRun.usage.estimatedCostUsd) @@ -213,6 +203,7 @@ export function AutomationListLocalRows({ {nextRunLabel} + diff --git a/src/renderer/src/components/automations/AutomationListSortHeader.tsx b/src/renderer/src/components/automations/AutomationListSortHeader.tsx new file mode 100644 index 00000000000..2c24a344328 --- /dev/null +++ b/src/renderer/src/components/automations/AutomationListSortHeader.tsx @@ -0,0 +1,51 @@ +import React from 'react' +import { ArrowDown, ArrowUp } from 'lucide-react' +import { cn } from '@/lib/utils' +import { translate } from '@/i18n/i18n' +import type { AutomationListSort, AutomationListSortField } from './automation-list-view' + +export function AutomationListSortHeader({ + field, + label, + sort, + onSort +}: { + field: AutomationListSortField + label: string + sort: AutomationListSort | null + onSort: (field: AutomationListSortField) => void +}): React.JSX.Element { + const active = sort?.field === field + const direction = active ? sort.direction : null + // Why: one interpolated key per direction — word order and punctuation around + // the column name differ per language. + const sortedLabel = + direction === 'asc' + ? translate( + 'auto.components.automations.AutomationListSortHeader.sortedAscending', + '{{value0}}, sorted ascending', + { value0: label } + ) + : direction === 'desc' + ? translate( + 'auto.components.automations.AutomationListSortHeader.sortedDescending', + '{{value0}}, sorted descending', + { value0: label } + ) + : null + return ( + + ) +} diff --git a/src/renderer/src/components/automations/AutomationListTableHeader.tsx b/src/renderer/src/components/automations/AutomationListTableHeader.tsx new file mode 100644 index 00000000000..8adbd127727 --- /dev/null +++ b/src/renderer/src/components/automations/AutomationListTableHeader.tsx @@ -0,0 +1,50 @@ +import React from 'react' +import { cn } from '@/lib/utils' +import { translate } from '@/i18n/i18n' +import { + AUTOMATIONS_TABLE_GRID_CLASS, + AUTOMATIONS_TABLE_HEADER_CLASS +} from './automations-table-layout' +import { AutomationListSortHeader } from './AutomationListSortHeader' +import type { AutomationListSort, AutomationListSortField } from './automation-list-view' + +export function AutomationListTableHeader({ + sort, + onSort +}: { + sort: AutomationListSort | null + onSort: (field: AutomationListSortField) => void +}): React.JSX.Element { + return ( +
+ + + {translate('auto.components.automations.AutomationDetail.18763ded26', 'Schedule')} + + + {translate('auto.components.automations.AutomationsPage.tableProject', 'Project')} + + + {translate('auto.components.automations.AutomationDetail.578ff46987', 'Next run')} + + + {translate('auto.components.automations.AutomationsPage.tableStatus', 'Status')} + + {translate('auto.components.automations.AutomationDetail.2df8970cd5', 'Agent')} + + + {translate('auto.components.automations.AutomationsPage.tableActions', 'Actions')} + +
+ ) +} diff --git a/src/renderer/src/components/automations/AutomationListToolbar.tsx b/src/renderer/src/components/automations/AutomationListToolbar.tsx new file mode 100644 index 00000000000..31e55df9e04 --- /dev/null +++ b/src/renderer/src/components/automations/AutomationListToolbar.tsx @@ -0,0 +1,86 @@ +import React from 'react' +import { Plus, RefreshCw } from 'lucide-react' +import { Button } from '@/components/ui/button' +import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' +import { cn } from '@/lib/utils' +import { translate } from '@/i18n/i18n' +import { clampAutomationListSearchQueryInput } from './automation-list-search' +import type { AutomationListFilter } from './automation-list-view' +import { AutomationListFilterMenu, AutomationListFilterPills } from './AutomationListFilterMenu' +import { AutomationListSearchField } from './AutomationListSearchField' +import type { AutomationTemplate } from './automation-templates' + +export function AutomationListToolbar({ + listSearchQuery, + isListSearchQueryTooLarge, + onListSearchQueryChange, + filter, + onFilterChange, + onRefresh, + isRefreshing, + openCreateDialog +}: { + listSearchQuery: string + isListSearchQueryTooLarge: boolean + onListSearchQueryChange: (query: string) => void + filter: AutomationListFilter + onFilterChange: (filter: AutomationListFilter) => void + onRefresh: () => void + isRefreshing: boolean + openCreateDialog: (template?: AutomationTemplate) => void +}): React.JSX.Element { + return ( +
+
+
+ + onListSearchQueryChange(clampAutomationListSearchQueryInput(query)) + } + onClear={() => onListSearchQueryChange('')} + /> + + + + + + + {translate( + 'auto.components.automations.AutomationsPage.19a6e30eae', + 'Refresh automations' + )} + + +
+ +
+ +
+ ) +} diff --git a/src/renderer/src/components/automations/AutomationsListPanel.tsx b/src/renderer/src/components/automations/AutomationsListPanel.tsx index e3dd2b3c090..a792839885c 100644 --- a/src/renderer/src/components/automations/AutomationsListPanel.tsx +++ b/src/renderer/src/components/automations/AutomationsListPanel.tsx @@ -1,7 +1,6 @@ import React from 'react' -import { Plus, RefreshCw } from 'lucide-react' +import { Plus } from 'lucide-react' import { Button } from '@/components/ui/button' -import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { cn } from '@/lib/utils' import type { Automation, @@ -15,29 +14,35 @@ import type { ProjectHostSetup, Repo, Worktree } from '../../../../shared/types' import type { RuntimeStatus } from '../../../../shared/runtime-types' import type { TaskSourceHostAvailability } from '../task-source-context-summary' import type { AutomationHostTarget } from './automation-host-client' -import { clampAutomationListSearchQueryInput } from './automation-list-search' import type { AutomationPaneTab } from './automation-page-state' -import { AutomationListSearchField } from './AutomationListSearchField' import { getAutomationTemplates, type AutomationTemplate } from './automation-templates' -import type { ExternalAutomationListEntry } from './external-automation-list-entries' import { AutomationListLocalRows } from './AutomationListLocalRows' import { AutomationListExternalRows } from './AutomationListExternalRows' -import { - AUTOMATIONS_TABLE_CONTAINER_CLASS, - AUTOMATIONS_TABLE_GRID_CLASS, - AUTOMATIONS_TABLE_HEADER_CLASS -} from './automations-table-layout' +import { AUTOMATIONS_TABLE_CONTAINER_CLASS } from './automations-table-layout' import { translate } from '@/i18n/i18n' +import type { + AutomationListFilter, + AutomationListSort, + AutomationListSortField, + AutomationListViewItem +} from './automation-list-view' +import { AutomationListTableHeader } from './AutomationListTableHeader' +import { AutomationListToolbar } from './AutomationListToolbar' +import { indexLatestAutomationRuns } from './automation-list-last-run' type AutomationsListPanelProps = { hasListItems: boolean hasFilteredListItems: boolean isListSearchActive: boolean + isListFilterActive: boolean listSearchQuery: string isListSearchQueryTooLarge: boolean onListSearchQueryChange: (query: string) => void - filteredAutomations: readonly Automation[] - filteredExternalAutomationEntries: readonly ExternalAutomationListEntry[] + visibleItems: readonly AutomationListViewItem[] + listFilter: AutomationListFilter + onListFilterChange: (filter: AutomationListFilter) => void + listSort: AutomationListSort | null + onListSort: (field: AutomationListSortField) => void // Why: use explicit ids, not resolved detail selection (which falls back to the // first row) — list highlight should only mark a user-chosen row. selectedId: string | null @@ -79,11 +84,15 @@ export function AutomationsListPanel({ hasListItems, hasFilteredListItems, isListSearchActive, + isListFilterActive, listSearchQuery, isListSearchQueryTooLarge, onListSearchQueryChange, - filteredAutomations, - filteredExternalAutomationEntries, + visibleItems, + listFilter, + onListFilterChange, + listSort, + onListSort, selectedId, selectedExternalKey, runs, @@ -111,6 +120,10 @@ export function AutomationsListPanel({ onRefresh, isRefreshing }: AutomationsListPanelProps): React.JSX.Element { + // Why: one pass over runs for the whole list — each row renders its own + // component, so indexing per row would be O(rows × runs) on every re-render. + const lastRunByAutomationId = React.useMemo(() => indexLatestAutomationRuns(runs), [runs]) + return (
) : (
-
-
- - onListSearchQueryChange(clampAutomationListSearchQueryInput(query)) - } - onClear={() => onListSearchQueryChange('')} - /> - - - - - - {translate( - 'auto.components.automations.AutomationsPage.19a6e30eae', - 'Refresh automations' - )} - - -
- -
+
{hasFilteredListItems ? ( <> -
- - {translate('auto.components.automations.AutomationsPage.tableName', 'Name')} - - - {translate( - 'auto.components.automations.AutomationDetail.18763ded26', - 'Schedule' - )} - - - {translate( - 'auto.components.automations.AutomationsPage.tableProject', - 'Project' - )} - - - {translate( - 'auto.components.automations.AutomationDetail.578ff46987', - 'Next run' - )} - - - {translate('auto.components.automations.AutomationsPage.tableStatus', 'Status')} - - - {translate('auto.components.automations.AutomationDetail.2df8970cd5', 'Agent')} - - - {translate( - 'auto.components.automations.AutomationsPage.tableActions', - 'Actions' - )} - -
+
- { - selectExternalKey(null) - selectAutomationId(automationId) - onOpenDetail() - }} - onRunNow={runNow} - onEdit={openEditDialog} - onToggle={toggleAutomation} - onDelete={requestDeleteAutomation} - /> - { - selectAutomationId(null) - selectExternalKey(entryKey) - setActivePaneTab('overview') - onOpenDetail() - }} - onRequestAction={requestExternalAction} - onEdit={openEditExternalDialog} - /> + {visibleItems.map((item) => + item.kind === 'local' ? ( + { + selectExternalKey(null) + selectAutomationId(automationId) + onOpenDetail() + }} + onRunNow={runNow} + onEdit={openEditDialog} + onToggle={toggleAutomation} + onDelete={requestDeleteAutomation} + /> + ) : ( + { + selectAutomationId(null) + selectExternalKey(entryKey) + setActivePaneTab('overview') + onOpenDetail() + }} + onRequestAction={requestExternalAction} + onEdit={openEditExternalDialog} + /> + ) + )}
- ) : isListSearchActive ? ( + ) : isListSearchActive || isListFilterActive ? (
{translate( - 'auto.components.automations.AutomationsPage.noSearchMatches', - 'No automations match your search.' + 'auto.components.automations.AutomationsPage.noListMatches', + 'No automations match.' )}
) : null} diff --git a/src/renderer/src/components/automations/AutomationsPage.tsx b/src/renderer/src/components/automations/AutomationsPage.tsx index c52126ad670..b1fe9c4988b 100644 --- a/src/renderer/src/components/automations/AutomationsPage.tsx +++ b/src/renderer/src/components/automations/AutomationsPage.tsx @@ -106,6 +106,13 @@ import { import { buildExternalAutomationListEntries } from './external-automation-list-entries' import { shouldCloseDetailForLostSelection } from './automation-detail-selection' import { useAutomationListSearch } from './use-automation-list-search' +import { useAutomationListView } from './use-automation-list-view' +import { + EMPTY_AUTOMATION_LIST_FILTER, + nextAutomationListSort, + type AutomationListFilter, + type AutomationListSort +} from './automation-list-view' import { AutomationDeleteDialog, ExternalAutomationDeleteDialog } from './AutomationDeleteDialogs' import { AutomationsListPanel } from './AutomationsListPanel' import { AutomationsDetailPane } from './AutomationsDetailPane' @@ -173,6 +180,8 @@ export default function AutomationsPage(): React.JSX.Element { const [isLoading, setIsLoading] = useState(true) const [isSaving, setIsSaving] = useState(false) const [listSearchQuery, setListSearchQuery] = useState('') + const [listFilter, setListFilter] = useState(EMPTY_AUTOMATION_LIST_FILTER) + const [listSort, setListSort] = useState(null) const [createOpen, setCreateOpen] = useState(false) const [createTarget, setCreateTarget] = useState('orca') const [editingAutomationId, setEditingAutomationId] = useState(null) @@ -273,8 +282,7 @@ export default function AutomationsPage(): React.JSX.Element { isListSearchActive, filteredAutomations, filteredExternalAutomationEntries, - hasListItems, - hasFilteredListItems + hasListItems } = useAutomationListSearch({ listSearchQuery, automations, @@ -285,6 +293,21 @@ export default function AutomationsPage(): React.JSX.Element { selectAutomationId, selectExternalKey }) + const { + visibleItems, + isListFilterActive, + hasVisibleListItems: hasFilteredListItems + } = useAutomationListView({ + automations: filteredAutomations, + externalEntries: filteredExternalAutomationEntries, + runs, + filter: listFilter, + sort: listSort, + selectedId, + selectedExternalKey, + selectAutomationId, + selectExternalKey + }) const selectedExternal = externalAutomationEntries.find((entry) => entry.key === selectedExternalKey) ?? @@ -2015,11 +2038,15 @@ export default function AutomationsPage(): React.JSX.Element { hasListItems={hasListItems} hasFilteredListItems={hasFilteredListItems} isListSearchActive={isListSearchActive} + isListFilterActive={isListFilterActive} listSearchQuery={listSearchQuery} isListSearchQueryTooLarge={isListSearchQueryTooLarge} onListSearchQueryChange={setListSearchQuery} - filteredAutomations={filteredAutomations} - filteredExternalAutomationEntries={filteredExternalAutomationEntries} + visibleItems={visibleItems} + listFilter={listFilter} + onListFilterChange={setListFilter} + listSort={listSort} + onListSort={(field) => setListSort((current) => nextAutomationListSort(current, field))} selectedId={selectedId} selectedExternalKey={selectedExternalKey} runs={runs} diff --git a/src/renderer/src/components/automations/AutomationsPageSkeleton.tsx b/src/renderer/src/components/automations/AutomationsPageSkeleton.tsx index b8de919f1cc..172502b29a1 100644 --- a/src/renderer/src/components/automations/AutomationsPageSkeleton.tsx +++ b/src/renderer/src/components/automations/AutomationsPageSkeleton.tsx @@ -16,12 +16,14 @@ function TableRowSkeleton({ scheduleWidthClass, projectWidthClass, nextWidthClass, + lastRunWidthClass, statusWidthClass }: { nameWidthClass: string scheduleWidthClass: string projectWidthClass: string nextWidthClass: string + lastRunWidthClass: string statusWidthClass: string }): React.JSX.Element { return ( @@ -30,6 +32,7 @@ function TableRowSkeleton({ + @@ -44,6 +47,7 @@ const TABLE_ROW_SKELETONS = [ scheduleWidthClass: 'w-36', projectWidthClass: 'w-28', nextWidthClass: 'w-24', + lastRunWidthClass: 'w-20', statusWidthClass: 'w-16' }, { @@ -52,6 +56,7 @@ const TABLE_ROW_SKELETONS = [ scheduleWidthClass: 'w-28', projectWidthClass: 'w-32', nextWidthClass: 'w-20', + lastRunWidthClass: 'w-24', statusWidthClass: 'w-14' }, { @@ -60,6 +65,7 @@ const TABLE_ROW_SKELETONS = [ scheduleWidthClass: 'w-40', projectWidthClass: 'w-24', nextWidthClass: 'w-28', + lastRunWidthClass: 'w-16', statusWidthClass: 'w-16' }, { @@ -68,6 +74,7 @@ const TABLE_ROW_SKELETONS = [ scheduleWidthClass: 'w-32', projectWidthClass: 'w-36', nextWidthClass: 'w-24', + lastRunWidthClass: 'w-20', statusWidthClass: 'w-14' }, { @@ -76,6 +83,7 @@ const TABLE_ROW_SKELETONS = [ scheduleWidthClass: 'w-24', projectWidthClass: 'w-28', nextWidthClass: 'w-20', + lastRunWidthClass: 'w-24', statusWidthClass: 'w-16' }, { @@ -84,6 +92,7 @@ const TABLE_ROW_SKELETONS = [ scheduleWidthClass: 'w-36', projectWidthClass: 'w-20', nextWidthClass: 'w-24', + lastRunWidthClass: 'w-16', statusWidthClass: 'w-14' } ] as const @@ -100,7 +109,8 @@ export function AutomationsPageSkeleton(): React.JSX.Element { >
- + +
@@ -114,6 +124,7 @@ export function AutomationsPageSkeleton(): React.JSX.Element { + diff --git a/src/renderer/src/components/automations/automation-list-last-run.test.ts b/src/renderer/src/components/automations/automation-list-last-run.test.ts new file mode 100644 index 00000000000..7eb54e123a5 --- /dev/null +++ b/src/renderer/src/components/automations/automation-list-last-run.test.ts @@ -0,0 +1,168 @@ +import { describe, expect, it } from 'vitest' +import type { + Automation, + AutomationRun, + ExternalAutomationJob +} from '../../../../shared/automations-types' +import { + formatAutomationLastRunCell, + getExternalAutomationLastRunSnapshot, + getLocalAutomationLastRunSnapshot, + getToneForAutomationRunStatus, + indexLatestAutomationRuns +} from './automation-list-last-run' + +function makeAutomation(overrides: Partial = {}): Automation { + return { + id: 'automation-1', + name: 'Nightly', + prompt: 'run', + precheck: null, + agentId: 'codex', + projectId: 'repo-1', + executionTargetType: 'local', + executionTargetId: 'local', + schedulerOwner: 'local_host_service', + workspaceMode: 'existing', + workspaceId: 'worktree-1', + baseBranch: null, + reuseSession: false, + timezone: 'UTC', + rrule: 'FREQ=DAILY', + dtstart: 1, + enabled: true, + nextRunAt: 2, + missedRunPolicy: 'run_once_within_grace', + missedRunGraceMinutes: 720, + createdAt: 1, + updatedAt: 1, + ...overrides + } +} + +function makeRun(overrides: Partial = {}): AutomationRun { + return { + id: 'run-1', + automationId: 'automation-1', + title: 'Nightly', + scheduledFor: 10, + status: 'completed', + trigger: 'scheduled', + workspaceId: 'worktree-1', + sessionKind: 'terminal', + chatSessionId: null, + terminalSessionId: null, + terminalPaneKey: null, + terminalPtyId: null, + outputSnapshot: null, + precheckResult: null, + usage: null, + error: null, + startedAt: 20, + dispatchedAt: 30, + createdAt: 10, + ...overrides + } +} + +function makeJob(overrides: Partial = {}): ExternalAutomationJob { + return { + id: 'job-1', + managerId: 'manager-1', + provider: 'hermes', + name: 'Digest', + schedule: '0 9 * * 1-5', + rawSchedule: null, + enabled: true, + state: 'enabled', + prompt: null, + promptPreview: '', + nextRunAt: null, + lastRunAt: null, + lastStatus: null, + lastError: null, + workdir: null, + runCount: 0, + runs: [], + ...overrides + } +} + +describe('automation-list-last-run', () => { + it('indexes the newest run per automation', () => { + const latest = indexLatestAutomationRuns([ + makeRun({ id: 'old', createdAt: 10, dispatchedAt: 10 }), + makeRun({ id: 'new', createdAt: 50, dispatchedAt: 50, status: 'dispatch_failed' }), + makeRun({ id: 'other', automationId: 'automation-2', createdAt: 20 }) + ]) + expect(latest.get('automation-1')?.id).toBe('new') + expect(latest.get('automation-2')?.id).toBe('other') + }) + + it('classifies local run statuses', () => { + expect(getToneForAutomationRunStatus('dispatch_failed')).toBe('failed') + expect(getToneForAutomationRunStatus('completed')).toBe('succeeded') + expect(getToneForAutomationRunStatus('dispatched')).toBe('running') + expect(getToneForAutomationRunStatus('skipped_precheck')).toBe('skipped') + }) + + it('prefers the latest run over lastRunAt-only metadata', () => { + const snapshot = getLocalAutomationLastRunSnapshot( + makeAutomation({ lastRunAt: 5 }), + makeRun({ status: 'dispatch_failed', dispatchedAt: 40, createdAt: 40 }) + ) + expect(snapshot.tone).toBe('failed') + expect(snapshot.at).toBe(40) + expect(snapshot.statusLabel).toBe('Failed') + }) + + it('falls back to lastRunAt when no run history exists', () => { + const snapshot = getLocalAutomationLastRunSnapshot(makeAutomation({ lastRunAt: 99 }), undefined) + expect(snapshot).toEqual({ at: 99, tone: 'unknown', statusLabel: '' }) + }) + + it('treats missing local history as never run', () => { + expect(getLocalAutomationLastRunSnapshot(makeAutomation(), undefined).tone).toBe('never') + }) + + it('classifies external last-run status and errors', () => { + expect( + getExternalAutomationLastRunSnapshot( + makeJob({ lastRunAt: '2026-08-12T01:00:00Z', lastStatus: 'failed' }) + ) + ).toMatchObject({ tone: 'failed', statusLabel: 'Failed' }) + expect( + getExternalAutomationLastRunSnapshot( + makeJob({ lastRunAt: '2026-08-12T01:00:00Z', lastStatus: 'ok' }) + ) + ).toMatchObject({ tone: 'succeeded', statusLabel: 'Done' }) + expect( + getExternalAutomationLastRunSnapshot(makeJob({ lastError: 'boom', lastStatus: null })) + ).toMatchObject({ tone: 'failed', statusLabel: 'Failed' }) + expect( + getExternalAutomationLastRunSnapshot( + makeJob({ lastRunAt: '2026-08-12T01:00:00Z', lastStatus: 'ok', lastError: 'stale' }) + ) + ).toMatchObject({ tone: 'succeeded', statusLabel: 'Done' }) + expect(getExternalAutomationLastRunSnapshot(makeJob()).tone).toBe('never') + }) + + it('formats last-run cells as status plus relative time', () => { + const now = Date.parse('2026-08-12T10:00:00Z') + const failed = formatAutomationLastRunCell( + { at: now - 8 * 60 * 60 * 1000, tone: 'failed', statusLabel: 'Failed' }, + now + ) + expect(failed.text).toBe('Failed 8h ago') + expect(failed.tone).toBe('failed') + expect( + formatAutomationLastRunCell({ at: null, tone: 'never', statusLabel: '' }, now).text + ).toBe('Never') + const failedNoTime = formatAutomationLastRunCell( + { at: null, tone: 'failed', statusLabel: 'Failed' }, + now + ) + expect(failedNoTime.text).toBe('Failed') + expect(failedNoTime.tone).toBe('failed') + }) +}) diff --git a/src/renderer/src/components/automations/automation-list-last-run.ts b/src/renderer/src/components/automations/automation-list-last-run.ts new file mode 100644 index 00000000000..7d5127337fa --- /dev/null +++ b/src/renderer/src/components/automations/automation-list-last-run.ts @@ -0,0 +1,152 @@ +import { translate } from '@/i18n/i18n' +import type { + Automation, + AutomationRun, + AutomationRunStatus, + ExternalAutomationJob +} from '../../../../shared/automations-types' +import { + formatAutomationDateTime, + formatAutomationRelativeTime, + getAutomationRunStatusLabel +} from './automation-page-parts' + +export type AutomationLastRunTone = + | 'failed' + | 'succeeded' + | 'running' + | 'skipped' + | 'never' + | 'unknown' + +export type AutomationLastRunSnapshot = { + at: number | null + tone: AutomationLastRunTone + statusLabel: string +} + +const EXTERNAL_FAILED_STATUSES = new Set(['failed', 'fail', 'error', 'dispatch_failed']) +const EXTERNAL_SUCCEEDED_STATUSES = new Set([ + 'ok', + 'completed', + 'success', + 'succeeded', + 'done', + 'healthy' +]) + +export function indexLatestAutomationRuns( + runs: readonly AutomationRun[] +): ReadonlyMap { + const latest = new Map() + for (const run of runs) { + const existing = latest.get(run.automationId) + if (!existing || run.createdAt > existing.createdAt) { + latest.set(run.automationId, run) + } + } + return latest +} + +export function getAutomationRunLastRunAt(run: AutomationRun): number { + return run.dispatchedAt ?? run.startedAt ?? run.createdAt +} + +export function getToneForAutomationRunStatus(status: AutomationRunStatus): AutomationLastRunTone { + if (status === 'dispatch_failed') { + return 'failed' + } + if (status === 'completed') { + return 'succeeded' + } + if (status === 'pending' || status === 'dispatching' || status === 'dispatched') { + return 'running' + } + if (status.startsWith('skipped')) { + return 'skipped' + } + return 'unknown' +} + +export function getLocalAutomationLastRunSnapshot( + automation: Automation, + lastRun: AutomationRun | undefined +): AutomationLastRunSnapshot { + if (lastRun) { + return { + at: getAutomationRunLastRunAt(lastRun), + tone: getToneForAutomationRunStatus(lastRun.status), + statusLabel: getAutomationRunStatusLabel(lastRun.status) + } + } + if (automation.lastRunAt) { + return { + at: automation.lastRunAt, + tone: 'unknown', + statusLabel: '' + } + } + return { at: null, tone: 'never', statusLabel: '' } +} + +function parseExternalLastRunAt(value: string | null): number | null { + if (!value) { + return null + } + const parsed = Date.parse(value) + return Number.isFinite(parsed) ? parsed : null +} + +export function getExternalAutomationLastRunSnapshot( + job: ExternalAutomationJob +): AutomationLastRunSnapshot { + const at = parseExternalLastRunAt(job.lastRunAt) + const raw = job.lastStatus?.trim() ?? '' + const normalized = raw.toLowerCase() + if (!at && !raw && !job.lastError) { + return { at: null, tone: 'never', statusLabel: '' } + } + if (EXTERNAL_SUCCEEDED_STATUSES.has(normalized)) { + return { + at, + tone: 'succeeded', + statusLabel: translate('auto.components.automations.automation.list.last.run.done', 'Done') + } + } + if (EXTERNAL_FAILED_STATUSES.has(normalized) || Boolean(job.lastError)) { + return { + at, + tone: 'failed', + statusLabel: translate( + 'auto.components.automations.automation.list.last.run.failed', + 'Failed' + ) + } + } + if (raw) { + return { at, tone: 'unknown', statusLabel: raw } + } + return { at, tone: 'unknown', statusLabel: '' } +} + +export function formatAutomationLastRunCell( + snapshot: AutomationLastRunSnapshot, + now: number +): { text: string; title: string; tone: AutomationLastRunTone } { + const status = snapshot.statusLabel.trim() + if (snapshot.tone === 'never') { + const never = formatAutomationDateTime(null) + return { text: never, title: never, tone: 'never' } + } + if (snapshot.at == null) { + return { text: status || formatAutomationDateTime(null), title: status, tone: snapshot.tone } + } + const relative = formatAutomationRelativeTime(snapshot.at, now) + const absolute = formatAutomationDateTime(snapshot.at) + const text = status && relative ? `${status} ${relative}` : status || relative || absolute + return { + text, + title: status ? `${status} · ${absolute}` : absolute, + tone: snapshot.tone + } +} diff --git a/src/renderer/src/components/automations/automation-list-view.test.ts b/src/renderer/src/components/automations/automation-list-view.test.ts new file mode 100644 index 00000000000..01a0cfa7799 --- /dev/null +++ b/src/renderer/src/components/automations/automation-list-view.test.ts @@ -0,0 +1,167 @@ +import { describe, expect, it } from 'vitest' +import type { + Automation, + AutomationRun, + ExternalAutomationJob, + ExternalAutomationManager +} from '../../../../shared/automations-types' +import { + applyAutomationListView, + countAutomationListFilters, + isAutomationListFilterActive, + nextAutomationListSort +} from './automation-list-view' +import type { ExternalAutomationListEntry } from './external-automation-list-entries' + +function makeAutomation(overrides: Partial = {}): Automation { + return { + id: 'automation-1', + name: 'Zebra job', + prompt: 'run', + precheck: null, + agentId: 'codex', + projectId: 'repo-1', + executionTargetType: 'local', + executionTargetId: 'local', + schedulerOwner: 'local_host_service', + workspaceMode: 'existing', + workspaceId: 'worktree-1', + baseBranch: null, + reuseSession: false, + timezone: 'UTC', + rrule: 'FREQ=DAILY', + dtstart: 1, + enabled: true, + nextRunAt: 2, + missedRunPolicy: 'run_once_within_grace', + missedRunGraceMinutes: 720, + createdAt: 1, + updatedAt: 1, + ...overrides + } +} + +function makeRun(overrides: Partial = {}): AutomationRun { + return { + id: 'run-1', + automationId: 'automation-1', + title: 'Zebra job', + scheduledFor: 10, + status: 'completed', + trigger: 'scheduled', + workspaceId: 'worktree-1', + sessionKind: 'terminal', + chatSessionId: null, + terminalSessionId: null, + terminalPaneKey: null, + terminalPtyId: null, + outputSnapshot: null, + precheckResult: null, + usage: null, + error: null, + startedAt: 20, + dispatchedAt: 30, + createdAt: 10, + ...overrides + } +} + +function makeExternalEntry( + overrides: Partial = {} +): ExternalAutomationListEntry { + const manager: ExternalAutomationManager = { + id: 'manager-1', + provider: 'hermes', + label: 'Local Hermes', + targetLabel: 'Local', + target: { type: 'local' }, + status: 'available', + error: null, + canManage: true, + jobs: [] + } + const job: ExternalAutomationJob = { + id: 'job-1', + managerId: manager.id, + provider: 'hermes', + name: 'Alpha digest', + schedule: '0 9 * * 1-5', + rawSchedule: null, + enabled: true, + state: 'enabled', + prompt: null, + promptPreview: '', + nextRunAt: null, + lastRunAt: '2026-08-12T01:00:00Z', + lastStatus: 'failed', + lastError: null, + workdir: null, + runCount: 1, + runs: [], + ...overrides + } + return { key: `${manager.id}:${job.id}`, manager, job } +} + +describe('automation-list-view', () => { + it('counts and detects active filters', () => { + expect(isAutomationListFilterActive({ status: 'all', lastRun: 'all' })).toBe(false) + expect(isAutomationListFilterActive({ status: 'paused', lastRun: 'all' })).toBe(true) + expect(countAutomationListFilters({ status: 'paused', lastRun: 'failed' })).toBe(2) + }) + + it('toggles sort direction and defaults last run to newest first', () => { + expect(nextAutomationListSort(null, 'name')).toEqual({ field: 'name', direction: 'asc' }) + expect(nextAutomationListSort(null, 'lastRun')).toEqual({ field: 'lastRun', direction: 'desc' }) + expect(nextAutomationListSort({ field: 'name', direction: 'asc' }, 'name')).toEqual({ + field: 'name', + direction: 'desc' + }) + expect(nextAutomationListSort({ field: 'name', direction: 'asc' }, 'lastRun')).toEqual({ + field: 'lastRun', + direction: 'desc' + }) + }) + + it('filters by enabled state and last-run outcome', () => { + const items = applyAutomationListView({ + automations: [ + makeAutomation({ id: 'paused', name: 'Paused', enabled: false }), + makeAutomation({ id: 'ok', name: 'Healthy' }) + ], + externalEntries: [makeExternalEntry()], + runs: [ + makeRun({ automationId: 'paused', status: 'completed' }), + makeRun({ automationId: 'ok', status: 'dispatch_failed' }) + ], + filter: { status: 'enabled', lastRun: 'failed' }, + sort: null + }) + expect(items.map((item) => item.id)).toEqual(['ok', 'manager-1:job-1']) + }) + + it('sorts by name across local and external rows', () => { + const items = applyAutomationListView({ + automations: [makeAutomation({ name: 'Zebra job' })], + externalEntries: [makeExternalEntry({ name: 'Alpha digest' })], + runs: [], + filter: { status: 'all', lastRun: 'all' }, + sort: { field: 'name', direction: 'asc' } + }) + expect(items.map((item) => item.name)).toEqual(['Alpha digest', 'Zebra job']) + }) + + it('sorts by last run newest first and keeps never-run rows last', () => { + const items = applyAutomationListView({ + automations: [ + makeAutomation({ id: 'old', name: 'Old' }), + makeAutomation({ id: 'never', name: 'Never' }) + ], + externalEntries: [makeExternalEntry({ lastRunAt: '2026-08-12T09:00:00Z' })], + runs: [makeRun({ automationId: 'old', dispatchedAt: Date.parse('2026-08-11T09:00:00Z') })], + filter: { status: 'all', lastRun: 'all' }, + sort: { field: 'lastRun', direction: 'desc' } + }) + expect(items.map((item) => item.id)).toEqual(['manager-1:job-1', 'old', 'never']) + }) +}) diff --git a/src/renderer/src/components/automations/automation-list-view.ts b/src/renderer/src/components/automations/automation-list-view.ts new file mode 100644 index 00000000000..49d44aadb3d --- /dev/null +++ b/src/renderer/src/components/automations/automation-list-view.ts @@ -0,0 +1,191 @@ +import { getIntlLocale } from '@/i18n/i18n' +import type { Automation, AutomationRun } from '../../../../shared/automations-types' +import type { ExternalAutomationListEntry } from './external-automation-list-entries' +import { + getExternalAutomationLastRunSnapshot, + getLocalAutomationLastRunSnapshot, + indexLatestAutomationRuns, + type AutomationLastRunSnapshot +} from './automation-list-last-run' + +export type AutomationListStatusFilter = 'all' | 'enabled' | 'paused' +export type AutomationListLastRunFilter = 'all' | 'failed' | 'succeeded' | 'never' +export type AutomationListSortField = 'name' | 'lastRun' +export type AutomationListSortDirection = 'asc' | 'desc' + +export type AutomationListSort = { + field: AutomationListSortField + direction: AutomationListSortDirection +} + +export type AutomationListViewItem = + | { + kind: 'local' + id: string + name: string + enabled: boolean + lastRunAt: number | null + lastRun: AutomationLastRunSnapshot + automation: Automation + } + | { + kind: 'external' + id: string + name: string + enabled: boolean + lastRunAt: number | null + lastRun: AutomationLastRunSnapshot + entry: ExternalAutomationListEntry + } + +export type AutomationListFilter = { + status: AutomationListStatusFilter + lastRun: AutomationListLastRunFilter +} + +export const EMPTY_AUTOMATION_LIST_FILTER: AutomationListFilter = { + status: 'all', + lastRun: 'all' +} + +export function isAutomationListFilterActive(filter: AutomationListFilter): boolean { + return filter.status !== 'all' || filter.lastRun !== 'all' +} + +export function countAutomationListFilters(filter: AutomationListFilter): number { + return (filter.status !== 'all' ? 1 : 0) + (filter.lastRun !== 'all' ? 1 : 0) +} + +export function defaultAutomationListSortDirection( + field: AutomationListSortField +): AutomationListSortDirection { + return field === 'lastRun' ? 'desc' : 'asc' +} + +export function nextAutomationListSort( + current: AutomationListSort | null, + field: AutomationListSortField +): AutomationListSort { + if (current?.field !== field) { + return { field, direction: defaultAutomationListSortDirection(field) } + } + return { + field, + direction: current.direction === 'asc' ? 'desc' : 'asc' + } +} + +function matchesStatusFilter(enabled: boolean, filter: AutomationListStatusFilter): boolean { + if (filter === 'all') { + return true + } + return filter === 'enabled' ? enabled : !enabled +} + +function matchesLastRunFilter( + snapshot: AutomationLastRunSnapshot, + filter: AutomationListLastRunFilter +): boolean { + if (filter === 'all') { + return true + } + return snapshot.tone === filter +} + +export function buildAutomationListViewItems({ + automations, + externalEntries, + runs +}: { + automations: readonly Automation[] + externalEntries: readonly ExternalAutomationListEntry[] + runs: readonly AutomationRun[] +}): AutomationListViewItem[] { + const lastRunByAutomationId = indexLatestAutomationRuns(runs) + const locals: AutomationListViewItem[] = automations.map((automation) => { + const lastRun = getLocalAutomationLastRunSnapshot( + automation, + lastRunByAutomationId.get(automation.id) + ) + return { + kind: 'local', + id: automation.id, + name: automation.name, + enabled: automation.enabled, + lastRunAt: lastRun.at, + lastRun, + automation + } + }) + const externals: AutomationListViewItem[] = externalEntries.map((entry) => { + const lastRun = getExternalAutomationLastRunSnapshot(entry.job) + return { + kind: 'external', + id: entry.key, + name: entry.job.name, + enabled: entry.job.enabled, + lastRunAt: lastRun.at, + lastRun, + entry + } + }) + return [...locals, ...externals] +} + +export function filterAutomationListViewItems( + items: readonly AutomationListViewItem[], + filter: AutomationListFilter +): AutomationListViewItem[] { + if (!isAutomationListFilterActive(filter)) { + return [...items] + } + return items.filter( + (item) => + matchesStatusFilter(item.enabled, filter.status) && + matchesLastRunFilter(item.lastRun, filter.lastRun) + ) +} + +export function sortAutomationListViewItems( + items: readonly AutomationListViewItem[], + sort: AutomationListSort | null +): AutomationListViewItem[] { + if (!sort) { + return [...items] + } + const next = [...items] + const locale = getIntlLocale() + next.sort((left, right) => { + const compared = + sort.field === 'name' + ? left.name.localeCompare(right.name, locale, { sensitivity: 'base' }) + : (left.lastRunAt ?? 0) - (right.lastRunAt ?? 0) + if (compared !== 0) { + return sort.direction === 'asc' ? compared : -compared + } + return left.id.localeCompare(right.id) + }) + return next +} + +export function applyAutomationListView({ + automations, + externalEntries, + runs, + filter, + sort +}: { + automations: readonly Automation[] + externalEntries: readonly ExternalAutomationListEntry[] + runs: readonly AutomationRun[] + filter: AutomationListFilter + sort: AutomationListSort | null +}): AutomationListViewItem[] { + return sortAutomationListViewItems( + filterAutomationListViewItems( + buildAutomationListViewItems({ automations, externalEntries, runs }), + filter + ), + sort + ) +} diff --git a/src/renderer/src/components/automations/automations-table-layout.ts b/src/renderer/src/components/automations/automations-table-layout.ts index 17f3af46681..b32c09d9fac 100644 --- a/src/renderer/src/components/automations/automations-table-layout.ts +++ b/src/renderer/src/components/automations/automations-table-layout.ts @@ -1,8 +1,8 @@ /** Shared layout classes for the automations list table. * Matches AutomationRunHistory / ExternalAutomationRunTable / Tasks list tables. */ -// Name | Schedule | Project | Next run | Status | Agent | Actions +// Name | Schedule | Project | Next run | Last run | Status | Agent | Actions export const AUTOMATIONS_TABLE_GRID_CLASS = - 'grid grid-cols-[minmax(0,1.5fr)_minmax(7rem,10rem)_minmax(5rem,9rem)_minmax(10rem,1.2fr)_minmax(4.5rem,6rem)_2.5rem_2.5rem]' + 'grid grid-cols-[minmax(0,1.4fr)_minmax(7.5rem,10rem)_minmax(4.5rem,8rem)_minmax(8.5rem,1fr)_minmax(8rem,11rem)_minmax(4.5rem,6rem)_2.5rem_2.5rem]' export const AUTOMATIONS_TABLE_CONTAINER_CLASS = 'rounded-md border border-border/50 bg-muted/20' diff --git a/src/renderer/src/components/automations/use-automation-list-view.ts b/src/renderer/src/components/automations/use-automation-list-view.ts new file mode 100644 index 00000000000..f87550c4079 --- /dev/null +++ b/src/renderer/src/components/automations/use-automation-list-view.ts @@ -0,0 +1,98 @@ +import { useEffect, useMemo } from 'react' +import type { Automation, AutomationRun } from '../../../../shared/automations-types' +import type { ExternalAutomationListEntry } from './external-automation-list-entries' +import { + applyAutomationListView, + isAutomationListFilterActive, + type AutomationListFilter, + type AutomationListSort, + type AutomationListViewItem +} from './automation-list-view' + +export function useAutomationListView({ + automations, + externalEntries, + runs, + filter, + sort, + selectedId, + selectedExternalKey, + selectAutomationId, + selectExternalKey +}: { + automations: readonly Automation[] + externalEntries: readonly ExternalAutomationListEntry[] + runs: readonly AutomationRun[] + filter: AutomationListFilter + sort: AutomationListSort | null + selectedId: string | null + selectedExternalKey: string | null + selectAutomationId: (automationId: string | null) => void + selectExternalKey: (externalKey: string | null) => void +}): { + visibleItems: readonly AutomationListViewItem[] + isListFilterActive: boolean + hasVisibleListItems: boolean +} { + const isListFilterActive = isAutomationListFilterActive(filter) + const visibleItems = useMemo( + () => + applyAutomationListView({ + automations, + externalEntries, + runs, + filter, + sort + }), + [automations, externalEntries, filter, runs, sort] + ) + const hasVisibleListItems = visibleItems.length > 0 + + // Why: search already hides rows; filter/sort must keep highlight + detail on a + // still-visible row so the list never points at something the user cannot see. + useEffect(() => { + if (!isListFilterActive && !sort) { + return + } + const localVisible = + selectedExternalKey === null && + selectedId != null && + visibleItems.some((item) => item.kind === 'local' && item.id === selectedId) + const externalVisible = + selectedExternalKey != null && + visibleItems.some((item) => item.kind === 'external' && item.id === selectedExternalKey) + if (localVisible || externalVisible) { + return + } + const first = visibleItems[0] + if (!first) { + return + } + if (first.kind === 'local') { + if (selectedExternalKey !== null) { + selectExternalKey(null) + } + if (selectedId !== first.id) { + selectAutomationId(first.id) + } + return + } + if (selectedExternalKey !== first.id) { + selectExternalKey(first.id) + } + }, [ + isListFilterActive, + selectAutomationId, + selectExternalKey, + selectedExternalKey, + selectedId, + sort, + visibleItems + ]) + + return { + visibleItems, + isListFilterActive, + hasVisibleListItems + } +} diff --git a/src/renderer/src/components/right-sidebar/ai-vault-scope-paths.ts b/src/renderer/src/components/right-sidebar/ai-vault-scope-paths.ts index 7bc85b13093..c9ad8137fc9 100644 --- a/src/renderer/src/components/right-sidebar/ai-vault-scope-paths.ts +++ b/src/renderer/src/components/right-sidebar/ai-vault-scope-paths.ts @@ -3,7 +3,7 @@ import { normalizeRuntimePathForComparison } from '../../../../shared/cross-platform-path' import type { ProjectHostSetupProjection } from '../../../../shared/project-host-setup-projection' -import type { Worktree } from '../../../../shared/types' +import type { ProjectHostSetup, Worktree } from '../../../../shared/types' import { splitWorktreeIdForFilesystem } from '../../../../shared/worktree-id' export function deriveAiVaultWorkspaceScopePaths( @@ -88,10 +88,10 @@ export function deriveAiVaultScopeSessionPaths( function buildProjectSetupsByRepoId( projection?: ProjectHostSetupProjection -): Map { - const setupsByRepoId = new Map() +): Map { + const setupsByRepoId = new Map() for (const setup of projection?.setups ?? []) { - const setups = setupsByRepoId.get(setup.repoId) ?? [] + const setups: ProjectHostSetup[] = setupsByRepoId.get(setup.repoId) ?? [] setups.push(setup) setupsByRepoId.set(setup.repoId, setups) } diff --git a/src/renderer/src/components/sidebar/add-repo-skip-finalization.ts b/src/renderer/src/components/sidebar/add-repo-skip-finalization.ts index 4161f3bb6f9..7c9c69357ce 100644 --- a/src/renderer/src/components/sidebar/add-repo-skip-finalization.ts +++ b/src/renderer/src/components/sidebar/add-repo-skip-finalization.ts @@ -3,7 +3,7 @@ import { isDefaultBranchWorkspace } from './visible-worktrees' export type AddRepoSkipFinalizationState = { activeRepoId: string | null - filterRepoIds: string[] + filterRepoIds: readonly string[] showActiveOnly: boolean hideDefaultBranchWorkspace: boolean showSleepingWorkspaces: boolean diff --git a/src/renderer/src/components/sidebar/visible-worktrees.ts b/src/renderer/src/components/sidebar/visible-worktrees.ts index f85083db406..206d81dbe38 100644 --- a/src/renderer/src/components/sidebar/visible-worktrees.ts +++ b/src/renderer/src/components/sidebar/visible-worktrees.ts @@ -186,7 +186,7 @@ export function computeVisibleWorktreeIds( worktreesByRepo: Record, sortedIds: string[], opts: { - filterRepoIds: string[] + filterRepoIds: readonly string[] showSleepingWorkspaces: boolean tabsByWorktree: Record[]> | null ptyIdsByTabId: Record | null diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 30fd1eceb6d..6c5d86d11de 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -5781,6 +5781,11 @@ export function connectPanePty( reportRemoteRendererSerializerReady() } }, + onStreamRecovered: (): void => { + if (isCurrent()) { + markHiddenOutputRestoreNeeded() + } + }, onData: (data: string, meta?: PtyDataMeta): void => { if (isCurrent()) { dataCallback(data, meta, generation) diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index 9e5c1d89be2..1ba841bd823 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -104,6 +104,9 @@ type PtyCallbacks = { /** Called before an adopted PTY can publish buffered/live bytes. */ onReattachDetermined?: () => void onConnect?: () => void + /** A stream re-established after loss carries only new bytes, so the pane must + * re-pull the host's retained buffer or an idle/exited pane paints nothing. */ + onStreamRecovered?: () => void onDisconnect?: () => void onData?: (data: string, meta?: PtyDataMeta) => void onReplayData?: (data: string, meta?: PtyReplayDataMeta) => void diff --git a/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-outcomes.test.ts b/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-outcomes.test.ts index 0c9b286703c..0e98202aab3 100644 --- a/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-outcomes.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-outcomes.test.ts @@ -32,6 +32,7 @@ function leafIdForPane(paneId: number): string { type ConnectCallbacks = { onReattachDetermined?: () => void onConnect?: () => void + onStreamRecovered?: () => void onData?: ( data: string, meta?: { seq?: number; rawLength?: number; background?: boolean; droppedOutput?: boolean } @@ -379,6 +380,7 @@ type RemotePaneDrive = { disposable: { dispose: () => void } deliver: (data: string, seq: number) => void setOutputPaused: (paused: boolean) => void + recoverStream: () => void writtenChunks: () => string[] } @@ -396,9 +398,11 @@ async function connectHiddenRemoteAgentPane( const capturedOutputPauseCallback: { current: ((paused: boolean, supported: boolean) => void) | null } = { current: null } + const capturedStreamRecoveredCallback: { current: (() => void) | null } = { current: null } transport.connect.mockImplementation(async ({ callbacks }: { callbacks?: ConnectCallbacks }) => { capturedDataCallback.current = callbacks?.onData ?? null capturedOutputPauseCallback.current = callbacks?.onOutputPauseChanged ?? null + capturedStreamRecoveredCallback.current = callbacks?.onStreamRecovered ?? null return REMOTE_PTY_ID }) transportFactoryQueue.push(transport) @@ -415,6 +419,7 @@ async function connectHiddenRemoteAgentPane( disposable, deliver: (data, seq) => capturedDataCallback.current?.(data, { seq, rawLength: data.length }), setOutputPaused: (paused) => capturedOutputPauseCallback.current?.(paused, true), + recoverStream: () => capturedStreamRecoveredCallback.current?.(), writtenChunks: () => pane.terminal.write.mock.calls.map(([data]) => String(data)) } } @@ -618,6 +623,24 @@ describe('remote hidden-output restore outcomes', () => { resetAgentStartupDelayedDeliveryForTests() }) + it('[modern] repaints a recovered visible pane from the retained host buffer', async () => { + const serializeBuffer = vi.fn() + const serializeBufferOutcome = vi.fn().mockResolvedValue({ + availability: { kind: 'snapshot' }, + snapshot: HOST_SNAPSHOT + }) + const drive = await connectHiddenRemoteAgentPane(serializeBuffer, serializeBufferOutcome) + ;(drive.deps.isVisibleRef as { current: boolean }).current = true + + drive.recoverStream() + await flushAsyncTicks(20) + + expect(serializeBufferOutcome).toHaveBeenCalledTimes(1) + expect(drive.writtenChunks().join('')).toContain(HOST_SNAPSHOT_MARKER) + expect(serializeBuffer).not.toHaveBeenCalled() + drive.disposable.dispose() + }) + it('[modern] accepts an empty snapshot as successful recovery without a loss banner', async () => { const serializeBuffer = vi.fn() const serializeBufferOutcome = vi.fn().mockResolvedValue({ diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts index 0292d0b1749..4087cf59a15 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts @@ -5054,6 +5054,91 @@ describe('createRemoteRuntimePtyTransport', () => { transport.destroy?.() }) + it('re-arms the retained-buffer restore when a recovery subscribe replays no snapshot', async () => { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const onReplayData = vi.fn() + const onStreamRecovered = vi.fn() + const onConnect = vi.fn() + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + + await transport.connect({ + url: '', + callbacks: { onReplayData, onStreamRecovered, onConnect } + }) + await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) + const firstStreamId = latestSubscribePayload().streamId + emitSnapshot(firstStreamId, 'INITIAL_SNAPSHOT') + subscriptionCallbacks?.onResponse({ + ok: true, + result: { + type: 'subscribed', + streamId: firstStreamId, + capabilities: { outputPause: 1 } + } + }) + await vi.waitFor(() => expect(onConnect).toHaveBeenCalledTimes(1)) + // The first subscribe already carries the host snapshot; re-arming there would cost + // every pane a redundant restore request on open. + expect(onStreamRecovered).not.toHaveBeenCalled() + + subscriptionCallbacks?.onClose?.() + await vi.waitFor(() => expect(runtimeSubscribe).toHaveBeenCalledTimes(2)) + await vi.waitFor(() => + expect( + subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .filter((frame) => frame?.opcode === TerminalStreamOpcode.Subscribe) + ).toHaveLength(2) + ) + const reconnectStreamId = latestSubscribePayload().streamId + // An exited-but-preserved pane has nothing to push and will never emit live bytes, + // so without the re-arm the pane stays blank until a visibility flip. + emitSnapshot(reconnectStreamId, '') + subscriptionCallbacks?.onResponse({ + ok: true, + result: { + type: 'subscribed', + streamId: reconnectStreamId, + capabilities: { outputPause: 1 } + } + }) + + await vi.waitFor(() => expect(onStreamRecovered).toHaveBeenCalledTimes(1)) + expect(onReplayData.mock.calls.map((call) => call[0])).toEqual(['INITIAL_SNAPSHOT']) + + subscriptionCallbacks?.onClose?.() + await vi.waitFor(() => expect(runtimeSubscribe).toHaveBeenCalledTimes(3)) + await vi.waitFor(() => + expect( + subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .filter((frame) => frame?.opcode === TerminalStreamOpcode.Subscribe) + ).toHaveLength(3) + ) + const populatedReconnectStreamId = latestSubscribePayload().streamId + emitSnapshot(populatedReconnectStreamId, 'RECOVERY_SNAPSHOT') + subscriptionCallbacks?.onResponse({ + ok: true, + result: { + type: 'subscribed', + streamId: populatedReconnectStreamId, + capabilities: { outputPause: 1 } + } + }) + + await vi.waitFor(() => expect(onConnect).toHaveBeenCalledTimes(3)) + expect(onStreamRecovered).toHaveBeenCalledTimes(1) + expect(onReplayData.mock.calls.map((call) => call[0])).toEqual([ + 'INITIAL_SNAPSHOT', + 'RECOVERY_SNAPSHOT' + ]) + transport.destroy?.() + }) + it('backs off before retrying a capacity-rejected terminal stream', async () => { vi.useFakeTimers() try { diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index 8908b39c63c..6eff59d84ce 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -1735,6 +1735,7 @@ export function createRemoteRuntimePtyTransport( setAttachmentReady(false) let transportClosed = false let subscriptionAttached = false + let subscriptionSnapshotHadContent = false // Why: viewport handed to subscribe; a resize during the round-trip falls back to the refresh-only one-shot RPC, replayed through the stream below once current. const subscribedViewport = desiredViewport const isCurrentSubscription = (): boolean => @@ -1760,6 +1761,7 @@ export function createRemoteRuntimePtyTransport( onSnapshot: (data, meta) => { // Why: an empty snapshot can still carry a pending mid-escape tail that must replay so the next live chunk completes it. if ((data || meta?.pendingEscapeTailAnsi) && isCurrentSubscription()) { + subscriptionSnapshotHadContent = true if (subscribedPtyId && bufferPtyShutdownReplayData(subscribedPtyId, data)) { return } @@ -1806,6 +1808,12 @@ export function createRemoteRuntimePtyTransport( markRecoveryHealthy() emitRecoveryState() storedCallbacks.onConnect?.() + // Why: a recovery subscribe replays nothing when the host's push snapshot is + // empty (idle or exited pane), so ask for the retained buffer instead of + // waiting for bytes that an exited process will never send. + if (expectedRecoveryEpoch !== undefined && !subscriptionSnapshotHadContent) { + storedCallbacks.onStreamRecovered?.() + } storedCallbacks.onStatus?.('shell') }, onEnd: () => { diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 921f3ab76c5..edf9789cae0 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -14471,7 +14471,9 @@ "tableStatus": "Status", "tableActions": "Actions", "newAutomation": "New Automation", - "rowActions": "Automation actions" + "rowActions": "Automation actions", + "tableLastRun": "Last run", + "noListMatches": "No automations match." }, "AutomationsPageSkeleton": { "55527b7bcf": "Loading automations" @@ -14565,6 +14567,14 @@ "name": "Hourly maintenance check", "prompt": "Check for stuck work, stale generated files, failing validation, and anything that needs human attention. Report only actionable issues." } + }, + "list": { + "last": { + "run": { + "done": "Done", + "failed": "Failed" + } + } } }, "external": { @@ -14597,6 +14607,19 @@ }, "AutomationListLocalRows": { "c92c9463c6": "Automation actions" + }, + "AutomationListFilterMenu": { + "removeFilter": "Remove {{value0}} filter", + "failed": "Failed", + "succeeded": "Succeeded", + "neverRan": "Never ran", + "filters": "Filters", + "all": "All", + "clear": "Clear filters" + }, + "AutomationListSortHeader": { + "sortedAscending": "{{value0}}, sorted ascending", + "sortedDescending": "{{value0}}, sorted descending" } }, "agent": { diff --git a/src/renderer/src/lib/http-link-routing.ts b/src/renderer/src/lib/http-link-routing.ts index 4b4a6a22a23..3295183d504 100644 --- a/src/renderer/src/lib/http-link-routing.ts +++ b/src/renderer/src/lib/http-link-routing.ts @@ -40,7 +40,7 @@ type StoreAccessor = () => { setActiveWorktree: (worktreeId: string) => void createBrowserTab: (worktreeId: string, url: string, opts: { activate: boolean }) => unknown repos?: readonly LocalhostLinkRepo[] - projects?: LocalhostLinkProject[] + projects?: readonly LocalhostLinkProject[] worktreesByRepo?: Record allWorktrees?: () => LocalhostLinkWorktree[] workspacePortScan?: { result: WorkspacePortScanResult } | null diff --git a/src/renderer/src/store/slices/repo-filter-selection.ts b/src/renderer/src/store/slices/repo-filter-selection.ts new file mode 100644 index 00000000000..65ae6d344a8 --- /dev/null +++ b/src/renderer/src/store/slices/repo-filter-selection.ts @@ -0,0 +1,10 @@ +// Why: catalog refreshes re-filter this on every fetch; six identity-sensitive subscribers +// (App.tsx at the root among them) re-render on a new array even when nothing was pruned. +export function retainValidFilterRepoIds( + filterRepoIds: readonly string[], + validRepoIds: ReadonlySet +): readonly string[] { + return filterRepoIds.every((repoId) => validRepoIds.has(repoId)) + ? filterRepoIds + : filterRepoIds.filter((repoId) => validRepoIds.has(repoId)) +} diff --git a/src/renderer/src/store/slices/repo-identity-reconcile.ts b/src/renderer/src/store/slices/repo-identity-reconcile.ts index f382ba21e2a..f61107c2673 100644 --- a/src/renderer/src/store/slices/repo-identity-reconcile.ts +++ b/src/renderer/src/store/slices/repo-identity-reconcile.ts @@ -12,7 +12,7 @@ import { getRepoHostIdentity } from './repo-host-identity' // arrays). IPC structured-clone rebuilds those every fetch, and main's hydrateRepo always // reconstructs hookSettings — so a reference compare reports every repo as changed and no repo // ever reconciles. Compare nested plain values structurally; they are small sanitized records. -function areValuesEqual(a: unknown, b: unknown): boolean { +export function areValuesEqual(a: unknown, b: unknown): boolean { if (a === b) { return true } @@ -28,9 +28,11 @@ function areValuesEqual(a: unknown, b: unknown): boolean { ) } // Why: only plain records are safe to walk — anything exotic falls back to reference equality. + const aPrototype = Object.getPrototypeOf(a) + const bPrototype = Object.getPrototypeOf(b) if ( - Object.getPrototypeOf(a) !== Object.prototype || - Object.getPrototypeOf(b) !== Object.prototype + (aPrototype !== Object.prototype && aPrototype !== null) || + (bPrototype !== Object.prototype && bPrototype !== null) ) { return false } @@ -45,41 +47,35 @@ function areValuesEqual(a: unknown, b: unknown): boolean { ) } -function areReposEqual(a: Repo, b: Repo): boolean { - if (a === b) { - return true - } - const keys = Object.keys(a) as (keyof Repo)[] - if (keys.length !== Object.keys(b).length) { - return false - } - for (const key of keys) { - if (!Object.hasOwn(b, key)) { - return false - } - if (!areValuesEqual(a[key], b[key])) { - return false - } - } - return true -} - -export function reconcileFetchedRepos( - previous: readonly Repo[], - next: readonly Repo[] -): readonly Repo[] { - const previousById = new Map(previous.map((repo) => [getRepoHostIdentity(repo), repo])) +/** + * Reuses equal rows from `previous` — and the whole array when nothing moved — so a refetch that + * changed nothing leaves identity-keyed memos and store subscribers untouched. `getIdentity` must + * be the key the producing merge already dedups by, so it is unique within `next`. + */ +export function reconcileCatalogRows( + previous: readonly T[], + next: readonly T[], + getIdentity: (row: T) => string +): readonly T[] { + const previousByIdentity = new Map(previous.map((row) => [getIdentity(row), row])) let identical = next.length === previous.length - const reconciled = next.map((repo, index) => { - const existing = previousById.get(getRepoHostIdentity(repo)) - if (existing && areReposEqual(existing, repo)) { + const reconciled = next.map((row, index) => { + const existing = previousByIdentity.get(getIdentity(row)) + if (existing !== undefined && areValuesEqual(existing, row)) { if (existing !== previous[index]) { identical = false } return existing } identical = false - return repo + return row }) return identical ? previous : reconciled } + +export function reconcileFetchedRepos( + previous: readonly Repo[], + next: readonly Repo[] +): readonly Repo[] { + return reconcileCatalogRows(previous, next, getRepoHostIdentity) +} diff --git a/src/renderer/src/store/slices/repos-refresh-identity.test.ts b/src/renderer/src/store/slices/repos-refresh-identity.test.ts new file mode 100644 index 00000000000..7fd32cca5ed --- /dev/null +++ b/src/renderer/src/store/slices/repos-refresh-identity.test.ts @@ -0,0 +1,283 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Project, Repo } from '../../../../shared/types' +import { createTestStore } from './store-test-helpers' + +// Why: every field here is load-bearing. A scalar-only repo reconciles even when the structural +// compare is broken, which is exactly how an earlier version of this work shipped green and inert. +// addedAt must stay non-zero: project-host-setup-projection falls back to `repo.addedAt || now`, +// so a zero timestamp stamps Date.now() into every projection and nothing ever reconciles. +const repo: Repo = { + id: 'repo-1', + path: '/repo-1', + displayName: 'Repo 1', + badgeColor: '#000000', + addedAt: 1_700_000_000_000, + executionHostId: 'local', + kind: 'git', + repoIcon: { type: 'lucide', name: 'Box' }, + upstream: { owner: 'upstream-owner', repo: 'repo-1', host: 'github.com' }, + gitRemoteIdentity: { + canonicalKey: 'github.com/octocat/repo-1', + remoteName: 'origin', + remoteUrl: 'git@github.com:octocat/repo-1.git' + }, + hookSettings: { mode: 'auto', scripts: { setup: 'echo hi', archive: '' } }, + symlinkPaths: ['.env', 'node_modules'], + importedExternalWorktreePaths: [] +} + +const secondRepo: Repo = { + ...repo, + id: 'repo-2', + path: '/repo-2', + displayName: 'Repo 2', + addedAt: 1_700_000_001_000, + upstream: { owner: 'upstream-owner', repo: 'repo-2', host: 'github.com' }, + gitRemoteIdentity: { + canonicalKey: 'github.com/octocat/repo-2', + remoteName: 'origin', + remoteUrl: 'git@github.com:octocat/repo-2.git' + } +} + +const reposList = vi.fn() +const projectsList = vi.fn() +const listHostSetups = vi.fn() + +// Why: catalogs arrive over IPC, so every fetch must hand back freshly allocated objects — +// otherwise identity would match by accident and prove nothing. +function clone(value: T): T { + return structuredClone(value) +} + +function mockRepos(...rows: readonly Repo[]): void { + reposList.mockImplementation(async () => rows.map(clone)) +} + +beforeEach(() => { + reposList.mockReset() + projectsList.mockReset() + listHostSetups.mockReset() + mockRepos(repo) + projectsList.mockImplementation(async () => []) + listHostSetups.mockImplementation(async () => []) + + vi.stubGlobal('window', { + api: { + repos: { list: reposList }, + projects: { list: projectsList, listHostSetups } + }, + dispatchEvent: vi.fn() + }) +}) + +describe('repo catalog refresh identity', () => { + it('keeps the projects and host setups arrays and entries across a no-op refetch', async () => { + const store = createTestStore() + await store.getState().fetchRepos() + const projects = store.getState().projects + const setups = store.getState().projectHostSetups + expect(projects).toHaveLength(1) + expect(setups).toHaveLength(1) + + await store.getState().fetchRepos() + + expect(store.getState().projects).toBe(projects) + expect(store.getState().projects[0]).toBe(projects[0]) + expect(store.getState().projectHostSetups).toBe(setups) + expect(store.getState().projectHostSetups[0]).toBe(setups[0]) + }) + + it('lets a nested hookSettings change through', async () => { + const store = createTestStore() + await store.getState().fetchRepos() + const setups = store.getState().projectHostSetups + + mockRepos({ ...repo, hookSettings: { mode: 'override', scripts: { setup: '', archive: '' } } }) + await store.getState().fetchRepos() + + expect(store.getState().projectHostSetups).not.toBe(setups) + expect(store.getState().projectHostSetups[0]?.hookSettings?.mode).toBe('override') + }) + + it('lets a displayName change through on projects', async () => { + const store = createTestStore() + await store.getState().fetchRepos() + const projects = store.getState().projects + + mockRepos({ ...repo, displayName: 'Renamed' }) + await store.getState().fetchRepos() + + expect(store.getState().projects).not.toBe(projects) + expect(store.getState().projects[0]?.displayName).toBe('Renamed') + }) + + it('lets an array-field change through', async () => { + const store = createTestStore() + await store.getState().fetchRepos() + const repos = store.getState().repos + + mockRepos({ ...repo, symlinkPaths: ['.env'] }) + await store.getState().fetchRepos() + + expect(store.getState().repos).not.toBe(repos) + expect(store.getState().repos[0]?.symlinkPaths).toEqual(['.env']) + }) + + it('lets a nested gitRemoteIdentity change through', async () => { + const store = createTestStore() + await store.getState().fetchRepos() + const projects = store.getState().projects + + mockRepos({ + ...repo, + gitRemoteIdentity: { ...repo.gitRemoteIdentity!, remoteName: 'upstream' } + }) + await store.getState().fetchRepos() + + expect(store.getState().projects).not.toBe(projects) + expect(store.getState().projects[0]?.gitRemoteIdentity?.remoteName).toBe('upstream') + }) + + it('treats clearing localWindowsRuntimePreference as a change', async () => { + const store = createTestStore() + await store.getState().fetchRepos() + const projectId = store.getState().projects[0]!.id + const withPreference: Project = { + ...store.getState().projects[0]!, + localWindowsRuntimePreference: { kind: 'wsl', distro: 'Ubuntu' } + } + store.setState({ projects: [withPreference] }) + + await store.getState().fetchRepos() + + // Why: a local-host refresh is authoritative; an absent key must not read as unchanged. + expect(store.getState().projects[0]).not.toBe(withPreference) + expect(store.getState().projects[0]?.id).toBe(projectId) + expect(store.getState().projects[0]?.localWindowsRuntimePreference).toBeUndefined() + }) + + it('reuses an unchanged setup element while a sibling changes', async () => { + mockRepos(repo, secondRepo) + const store = createTestStore() + await store.getState().fetchRepos() + const setups = store.getState().projectHostSetups + expect(setups).toHaveLength(2) + + mockRepos(repo, { ...secondRepo, displayName: 'Repo 2 renamed' }) + await store.getState().fetchRepos() + + const next = store.getState().projectHostSetups + expect(next).not.toBe(setups) + expect(next[0]).toBe(setups[0]) + expect(next[1]).not.toBe(setups[1]) + }) + + it('does not reuse a setup that moved to a different execution host', async () => { + const store = createTestStore() + await store.getState().fetchRepos() + const setups = store.getState().projectHostSetups + expect(setups[0]?.hostId).toBe('local') + + // Why: the repo-derived fallback sets setup.id = repo.id, so the same id on a second host is + // the case that would silently splice the wrong host's routing metadata into the row. + mockRepos({ ...repo, executionHostId: 'ssh:host-a', connectionId: 'host-a' }) + await store.getState().fetchRepos() + + const next = store.getState().projectHostSetups + expect(next[0]).not.toBe(setups[0]) + expect(next[0]?.hostId).toBe('ssh:host-a') + }) + + it('reconciles both setups when one repo id exists on two hosts', async () => { + // Why: the repo-derived fallback sets setup.id = repo.id, so these two setups share an id and + // differ only by host. Keying the reconcile on setup.id instead of the owner key collapses them + // onto one slot and one of the two churns on every refresh. + const sshRepo: Repo = { ...repo, executionHostId: 'ssh:host-a', connectionId: 'host-a' } + mockRepos(repo, sshRepo) + const store = createTestStore() + await store.getState().fetchRepos() + const setups = store.getState().projectHostSetups + expect(setups).toHaveLength(2) + expect(new Set(setups.map((setup) => setup.id)).size).toBe(1) + + await store.getState().fetchRepos() + + expect(store.getState().projectHostSetups).toBe(setups) + expect(store.getState().projectHostSetups[0]).toBe(setups[0]) + expect(store.getState().projectHostSetups[1]).toBe(setups[1]) + }) + + it('drops a project and its setup when its repo disappears', async () => { + mockRepos(repo, secondRepo) + const store = createTestStore() + await store.getState().fetchRepos() + const projects = store.getState().projects + + mockRepos(repo) + await store.getState().fetchRepos() + + expect(store.getState().projects).not.toBe(projects) + expect(store.getState().projects).toHaveLength(1) + expect(store.getState().projectHostSetups).toHaveLength(1) + }) + + it('adds a project and its setup when a repo appears', async () => { + const store = createTestStore() + await store.getState().fetchRepos() + const projects = store.getState().projects + + mockRepos(repo, secondRepo) + await store.getState().fetchRepos() + + expect(store.getState().projects).not.toBe(projects) + expect(store.getState().projects).toHaveLength(2) + expect(store.getState().projectHostSetups).toHaveLength(2) + }) +}) + +describe('repo filter identity across catalog refreshes', () => { + it('keeps the filterRepoIds array when a refetch prunes nothing', async () => { + const store = createTestStore() + store.setState({ filterRepoIds: [repo.id] }) + const first = store.getState().filterRepoIds + + await store.getState().fetchRepos() + + // Why: App.tsx at the root and five sidebar consumers select this array by identity. + expect(store.getState().filterRepoIds).toBe(first) + }) + + it('still prunes a filtered repo id that no longer exists', async () => { + const store = createTestStore() + store.setState({ filterRepoIds: [repo.id, 'gone'] }) + + await store.getState().fetchRepos() + + expect(store.getState().filterRepoIds).toEqual([repo.id]) + }) + + it('prunes only the vanished id and reallocates', async () => { + mockRepos(repo) + const store = createTestStore() + store.setState({ filterRepoIds: [repo.id, secondRepo.id] }) + const first = store.getState().filterRepoIds + + await store.getState().fetchRepos() + + expect(store.getState().filterRepoIds).toEqual([repo.id]) + expect(store.getState().filterRepoIds).not.toBe(first) + }) + + it('keeps a filtered id whose repo merely moved to another host', async () => { + const store = createTestStore() + store.setState({ filterRepoIds: [repo.id] }) + const first = store.getState().filterRepoIds + + // Why: the filter is keyed on repo id, not host identity — a rehomed repo is not pruned. + mockRepos({ ...repo, executionHostId: 'ssh:host-a', connectionId: 'host-a' }) + await store.getState().fetchRepos() + + expect(store.getState().filterRepoIds).toBe(first) + }) +}) diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 49c53d12a7b..dbc55c8c320 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -47,7 +47,12 @@ import { getProjectGroupSubtreeIds } from '../../../../shared/project-groups' import { isPathInsideOrEqual } from '../../../../shared/cross-platform-path' import { getRepoIdFromWorktreeId } from '../../../../shared/worktree-id' import { selectProjectGroupRemovalTargets } from './project-group-removal-targets' -import { reconcileFetchedRepos } from './repo-identity-reconcile' +import { + areValuesEqual, + reconcileCatalogRows, + reconcileFetchedRepos +} from './repo-identity-reconcile' +import { retainValidFilterRepoIds } from './repo-filter-selection' import { mergeSshRepoReadoptions, reconcileReadoptedSshRepoRows, @@ -792,6 +797,81 @@ function getExplicitProjectHostIds( return hostIds } +function indexProjectHostSetupsByProjectId( + setups: readonly ProjectHostSetup[] +): Map { + const setupsByProjectId = new Map() + for (const setup of setups) { + const existing = setupsByProjectId.get(setup.projectId) + if (existing) { + existing.push(setup) + } else { + setupsByProjectId.set(setup.projectId, [setup]) + } + } + return setupsByProjectId +} + +function getProjectSourceRepos( + project: Project, + reposById: ReadonlyMap +): Repo[] { + const sourceRepos: Repo[] = [] + for (const repoId of project.sourceRepoIds) { + for (const repo of reposById.get(repoId) ?? []) { + sourceRepos.push(repo) + } + } + return sourceRepos +} + +// Why: the host-id resolvers rescan every setup and repo per project; feeding them the project's own slices keeps a catalog refresh linear. +function createProjectHostIdIndex( + setups: readonly ProjectHostSetup[], + reposById: ReadonlyMap, + resolveHostIds: ( + project: Project, + setups: readonly ProjectHostSetup[], + repos: readonly Repo[] + ) => Set +): (project: Project) => ReadonlySet { + const noSetups: readonly ProjectHostSetup[] = [] + const hostIdsByProject = new Map>() + let setupsByProjectId: Map | null = null + return (project) => { + const cached = hostIdsByProject.get(project) + if (cached) { + return cached + } + setupsByProjectId ??= indexProjectHostSetupsByProjectId(setups) + const hostIds = resolveHostIds( + project, + setupsByProjectId.get(project.id) ?? noSetups, + getProjectSourceRepos(project, reposById) + ) + hostIdsByProject.set(project, hostIds) + return hostIds + } +} + +// Why: mergePreviousProjectMetadata scans the whole catalog's repo key set; a view holding only this pair's repos keeps that scan per-project. +function restrictReposToProjectPair( + previous: Project, + current: Project, + reposById: ReadonlyMap +): Map { + const restricted = new Map() + for (const project of [previous, current]) { + for (const repoId of project.sourceRepoIds) { + const matches = reposById.get(repoId) + if (matches) { + restricted.set(repoId, matches) + } + } + } + return restricted +} + function mergeFetchedProjectCompatibilityForHost({ previous, fetched, @@ -819,78 +899,78 @@ function mergeFetchedProjectCompatibilityForHost({ const previousProjectById = new Map(previous.projects.map((project) => [project.id, project])) const reposById = getReposById(repos) const currentRepoIds = new Set(repos.map((repo) => repo.id)) - const projectHasHost = (project: Project, setups: readonly ProjectHostSetup[]): boolean => - getProjectHostIds(project, setups, repos).has(hostId) - const projectHasCurrentOwnerOutsideHost = (project: Project): boolean => - [...getExplicitProjectHostIds(project, projectHostSetups, repos)].some( - (ownerHostId) => ownerHostId !== hostId - ) + const fetchedProjectHostIds = createProjectHostIdIndex( + fetched.projectHostSetups, + reposById, + getProjectHostIds + ) + const previousProjectHostIds = createProjectHostIdIndex( + previous.projectHostSetups, + reposById, + getProjectHostIds + ) + const currentProjectOwnerHostIds = createProjectHostIdIndex( + projectHostSetups, + reposById, + getExplicitProjectHostIds + ) + const projectHasCurrentOwnerOutsideHost = (project: Project): boolean => { + for (const ownerHostId of currentProjectOwnerHostIds(project)) { + if (ownerHostId !== hostId) { + return true + } + } + return false + } const fetchedProjects = fetched.projects .filter((project) => { const previousProject = previousProjectById.get(project.id) // Why: repo-derived compatibility projects include every host; a one-host refresh should only reconcile or prune that host's ownership. return ( - projectHasHost(project, fetched.projectHostSetups) || - (previousProject ? projectHasHost(previousProject, previous.projectHostSetups) : false) + fetchedProjectHostIds(project).has(hostId) || + (previousProject ? previousProjectHostIds(previousProject).has(hostId) : false) ) }) .map((project) => { const previousProject = previousProjectById.get(project.id) return previousProject - ? mergePreviousProjectMetadata(previousProject, project, reposById, hostId) + ? mergePreviousProjectMetadata( + previousProject, + project, + restrictReposToProjectPair(previousProject, project, reposById), + hostId + ) : projectWithCurrentSourceRepoIds(project, currentRepoIds) }) const fetchedProjectIds = new Set(fetchedProjects.map((project) => project.id)) const preservedProjects = previous.projects.filter( (project) => !fetchedProjectIds.has(project.id) && - (!getProjectHostIds(project, previous.projectHostSetups, repos).has(hostId) || - projectHasCurrentOwnerOutsideHost(project)) + (!previousProjectHostIds(project).has(hostId) || projectHasCurrentOwnerOutsideHost(project)) ) + // Why: both merges always allocate (sourceRepoIds is rebuilt per project, and fetched setups + // arrive freshly cloned over IPC), so reconcile against `previous` to recover identity when a + // refresh changed nothing. Each key is what the producing merge already dedups by. return { - projects: mergeProjectCompatibilityProjects( - preservedProjects.map((project) => { - const sourceRepoIds = getSourceRepoIdsOutsideHost(project, reposById, hostId) - return sourceRepoIds.length === project.sourceRepoIds.length - ? project - : { ...project, sourceRepoIds } - }), - fetchedProjects + projects: reconcileCatalogRows( + previous.projects, + mergeProjectCompatibilityProjects( + preservedProjects.map((project) => { + const sourceRepoIds = getSourceRepoIdsOutsideHost(project, reposById, hostId) + return sourceRepoIds.length === project.sourceRepoIds.length + ? project + : { ...project, sourceRepoIds } + }), + fetchedProjects + ), + (project) => project.id ), - projectHostSetups - } -} - -function isPlainCatalogObject(value: unknown): value is Record { - if (typeof value !== 'object' || value === null) { - return false - } - const prototype = Object.getPrototypeOf(value) - return prototype === Object.prototype || prototype === null -} - -// Why: catalog fetches rebuild every entry from IPC, so identity alone never matches; -// structural equality is what lets an unchanged refetch stay a no-op. -function areCatalogEntriesEqual(a: unknown, b: unknown): boolean { - if (a === b) { - return true - } - if (Array.isArray(a) || Array.isArray(b)) { - return ( - Array.isArray(a) && - Array.isArray(b) && - a.length === b.length && - a.every((entry, index) => areCatalogEntriesEqual(entry, b[index])) + projectHostSetups: reconcileCatalogRows( + previous.projectHostSetups, + projectHostSetups, + getProjectHostSetupOwnerKey ) } - if (!isPlainCatalogObject(a) || !isPlainCatalogObject(b)) { - return false - } - const keys = Object.keys(a) - if (keys.length !== Object.keys(b).length) { - return false - } - return keys.every((key) => Object.hasOwn(b, key) && areCatalogEntriesEqual(a[key], b[key])) } // Why: returning `base` unchanged keeps referential-equality selectors quiet, so a @@ -912,7 +992,7 @@ function mergeByIdentity( changed = true continue } - if (areCatalogEntriesEqual(merged[index], entry)) { + if (areValuesEqual(merged[index], entry)) { continue } merged[index] = entry @@ -1203,7 +1283,7 @@ function filterSetupsForPrunedRepoRows( setups: readonly ProjectHostSetup[], mergedRepos: readonly Repo[], reconciledRepos: readonly Repo[] -): ProjectHostSetup[] { +): readonly ProjectHostSetup[] { const survivingOwners = new Set( reconciledRepos.map((repo) => `${getRepoExecutionHostId(repo)}:${repo.id}`) ) @@ -1212,12 +1292,15 @@ function filterSetupsForPrunedRepoRows( .filter((repo) => !survivingOwners.has(`${getRepoExecutionHostId(repo)}:${repo.id}`)) .map((repo) => `${getRepoExecutionHostId(repo)}:${repo.id}`) ) + // Why: this result feeds the compat merge as `previous`, so an unconditional copy would discard + // the identity that merge is about to try to preserve. if (prunedOwners.size === 0) { - return [...setups] + return setups } - return setups.filter( + const filtered = setups.filter( (setup) => !setup.repoId || !prunedOwners.has(`${setup.hostId}:${setup.repoId}`) ) + return filtered.length === setups.length ? setups : filtered } function reconcileReadoptedSshWorktreeState( @@ -1690,8 +1773,8 @@ function getFolderWorkspacePathStatusRequestSnapshotForRead( export type RepoSlice = { repos: readonly Repo[] - projects: Project[] - projectHostSetups: ProjectHostSetup[] + projects: readonly Project[] + projectHostSetups: readonly ProjectHostSetup[] projectGroups: readonly ProjectGroup[] folderWorkspaces: readonly FolderWorkspace[] folderWorkspacePathStatuses: Record @@ -2006,7 +2089,7 @@ export const createRepoSlice: StateCreator = (set, ? {} : { folderWorkspacePathStatuses: {} }), activeRepoId: s.activeRepoId && validRepoIds.has(s.activeRepoId) ? s.activeRepoId : null, - filterRepoIds: s.filterRepoIds.filter((projectId) => validRepoIds.has(projectId)), + filterRepoIds: retainValidFilterRepoIds(s.filterRepoIds, validRepoIds), setupScriptPromptDismissedRepoIds: filterSetupScriptPromptDismissalsToValidRepos( s.setupScriptPromptDismissedRepoIds, validRepoHostIdentities @@ -2090,7 +2173,7 @@ export const createRepoSlice: StateCreator = (set, ...reconcileReadoptedSshWorktreeState(s, s.pendingSshRepoReadoptions), ...mergedProjectCompatibility, activeRepoId: s.activeRepoId && validRepoIds.has(s.activeRepoId) ? s.activeRepoId : null, - filterRepoIds: s.filterRepoIds.filter((projectId) => validRepoIds.has(projectId)), + filterRepoIds: retainValidFilterRepoIds(s.filterRepoIds, validRepoIds), setupScriptPromptDismissedRepoIds: filterSetupScriptPromptDismissalsToValidRepos( s.setupScriptPromptDismissedRepoIds, validRepoHostIdentities @@ -2172,7 +2255,7 @@ export const createRepoSlice: StateCreator = (set, const validRepoHostIdentities = new Set(s.repos.map(getRepoHostIdentity)) return { activeRepoId: s.activeRepoId && validRepoIds.has(s.activeRepoId) ? s.activeRepoId : null, - filterRepoIds: s.filterRepoIds.filter((projectId) => validRepoIds.has(projectId)), + filterRepoIds: retainValidFilterRepoIds(s.filterRepoIds, validRepoIds), setupScriptPromptDismissedRepoIds: filterSetupScriptPromptDismissalsToValidRepos( s.setupScriptPromptDismissedRepoIds, validRepoHostIdentities diff --git a/src/renderer/src/store/slices/ui.ts b/src/renderer/src/store/slices/ui.ts index 1bfa1d68d77..dbbc2977965 100644 --- a/src/renderer/src/store/slices/ui.ts +++ b/src/renderer/src/store/slices/ui.ts @@ -904,8 +904,8 @@ export type UISlice = { showDotfilesByWorktree: Record setShowDotfilesForWorktree: (worktreeId: string, showDotfiles: boolean) => void toggleShowDotfilesForWorktree: (worktreeId: string) => void - filterRepoIds: string[] - setFilterRepoIds: (ids: string[]) => void + filterRepoIds: readonly string[] + setFilterRepoIds: (ids: readonly string[]) => void collapsedGroups: Set toggleCollapsedGroup: (key: string) => void worktreeCardProperties: WorktreeCardProperty[] diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 94a3a692360..9c1fc6691ec 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -6080,7 +6080,7 @@ export const createWorktreeSlice: StateCreator const reposChanged = survivingRepos.length !== s.repos.length // Why: a repoId-less setup on the removed host can still split a surviving project group, so drop every setup it owns. - const survivingSetups: AppState['projectHostSetups'] = [] + const survivingSetups: ProjectHostSetup[] = [] for (const setup of s.projectHostSetups) { if (isRemovedRuntimeHostId(setup.hostId, removed)) { if (setup.repoId) { diff --git a/src/shared/agent-hook-listener.test.ts b/src/shared/agent-hook-listener.test.ts index 298e7a55c79..9d30dc8a844 100644 --- a/src/shared/agent-hook-listener.test.ts +++ b/src/shared/agent-hook-listener.test.ts @@ -801,7 +801,7 @@ describe('shared agent-hook-listener', () => { expect(tool?.payload.interactivePrompt).toBeUndefined() }) - it('keeps OMP ask_user_question behavior on Pi-compatible events', () => { + it('maps OMP ask to blocked without publishing a native prompt', () => { const tool = normalizeHookPayload( state, 'omp', @@ -812,8 +812,8 @@ describe('shared agent-hook-listener', () => { env: 'production', version: '1', payload: { - hook_event_name: 'tool_call', - tool_name: 'ask_user_question', + hook_event_name: 'tool_execution_start', + tool_name: 'ask', tool_input: { questions: [ { @@ -827,9 +827,9 @@ describe('shared agent-hook-listener', () => { 'production' ) expect(tool?.payload).toMatchObject({ - state: 'working', + state: 'blocked', agentType: 'omp', - toolName: 'ask_user_question' + toolName: 'ask' }) expect(tool?.payload.interactivePrompt).toBeUndefined() }) diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index d24fa70e087..d7b7422db93 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -3847,13 +3847,14 @@ function normalizePiCompatibleEvent( return null } - // Why: gate on the event's own tool_name (not a merged snapshot) so a stale cached ask_user_question can't re-enter blocked. - const isPiAskUserQuestion = - agentType === 'pi' && - isAskUserQuestionTool(readString(hookPayload, 'tool_name')) && + // Why: gate on the event's own tool_name so a stale cached question can't re-enter blocked. + const toolName = readString(hookPayload, 'tool_name') + const isPiCompatibleAsk = + ((agentType === 'pi' && isAskUserQuestionTool(toolName)) || + (agentType === 'omp' && toolName === 'ask')) && (eventName === 'tool_call' || eventName === 'tool_execution_start') - const stateName = isPiAskUserQuestion + const stateName = isPiCompatibleAsk ? 'blocked' : eventName === 'before_agent_start' || eventName === 'agent_start' || diff --git a/src/shared/project-host-setup-projection.ts b/src/shared/project-host-setup-projection.ts index c0c129d643a..febf7a16018 100644 --- a/src/shared/project-host-setup-projection.ts +++ b/src/shared/project-host-setup-projection.ts @@ -13,8 +13,8 @@ type ProjectAccumulator = { } export type ProjectHostSetupProjection = { - projects: Project[] - setups: ProjectHostSetup[] + projects: readonly Project[] + setups: readonly ProjectHostSetup[] } function getProjectProviderIdentity( diff --git a/src/shared/secure-file-fsync-flags.test.ts b/src/shared/secure-file-fsync-flags.test.ts new file mode 100644 index 00000000000..1c167bffdad --- /dev/null +++ b/src/shared/secure-file-fsync-flags.test.ts @@ -0,0 +1,52 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import type * as NodeFs from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const openedPaths = vi.hoisted(() => [] as { path: string; flags: string | number }[]) + +vi.mock('node:fs', async () => { + const actual = await vi.importActual('node:fs') + return { + ...actual, + openSync: (path: NodeFs.PathLike, flags: string | number, mode?: NodeFs.Mode) => { + openedPaths.push({ path: String(path), flags }) + return actual.openSync(path, flags, mode) + } + } +}) + +import { bestEffortFsyncDirectorySync, fsyncFileSync } from './secure-file' + +const createdPaths: string[] = [] + +afterEach(() => { + openedPaths.length = 0 + for (const path of createdPaths.splice(0)) { + rmSync(path, { recursive: true, force: true }) + } +}) + +describe('secure file fsync flags', () => { + it('opens files read/write before fsync', () => { + const directory = mkdtempSync(join(tmpdir(), 'orca-file-fsync-')) + createdPaths.push(directory) + const path = join(directory, 'record.json') + writeFileSync(path, '{}') + + fsyncFileSync(path) + + expect(openedPaths).toEqual([{ path, flags: 'r+' }]) + }) + + const posixIt = process.platform === 'win32' ? it.skip : it + posixIt('opens directories read-only before fsync', () => { + const directory = mkdtempSync(join(tmpdir(), 'orca-directory-fsync-')) + createdPaths.push(directory) + + bestEffortFsyncDirectorySync(directory) + + expect(openedPaths).toEqual([{ path: directory, flags: 'r' }]) + }) +}) diff --git a/src/shared/secure-file.ts b/src/shared/secure-file.ts index e3c38c70e09..7b2f646c540 100644 --- a/src/shared/secure-file.ts +++ b/src/shared/secure-file.ts @@ -133,8 +133,8 @@ export function writeSecureFile( } } -export function fsyncFileSync(path: string): void { - const descriptor = openSync(path, 'r') +function fsyncPathSync(path: string, flags: 'r' | 'r+'): void { + const descriptor = openSync(path, flags) try { fsyncSync(descriptor) } finally { @@ -142,12 +142,17 @@ export function fsyncFileSync(path: string): void { } } +export function fsyncFileSync(path: string): void { + // FlushFileBuffers requires a write-capable handle on Windows. + fsyncPathSync(path, 'r+') +} + export function bestEffortFsyncDirectorySync(directory: string): void { if (process.platform === 'win32') { return } try { - fsyncFileSync(directory) + fsyncPathSync(directory, 'r') } catch (error) { if ( error instanceof Error &&