From dde72f85de60948947dc47478d3cce51c2617933 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Wed, 29 Jul 2026 15:23:06 -0700 Subject: [PATCH 01/21] fix(windows): separate updater from orchestration migration (#11405) * fix(windows): separate updater from orchestration migration * fix(terminal): attest adopted reveal identity --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- .github/workflows/win-update-e2e.yml | 1 - config/nsis/daemon-host-uninstall.nsh | 20 ----- config/reliability-gates.jsonc | 23 ++--- ...orchestration-seamless-update-migration.md | 36 ++------ src/main/cli/windows-launcher-asset.test.ts | 23 ----- .../src/hooks/ipc-events-test-harness.ts | 10 ++- ...obile-terminal-reveal-tab-adoption.test.ts | 16 +++- src/renderer/src/hooks/useIpcEvents.ts | 3 +- tools/win-update-e2e/README.md | 9 -- tools/win-update-e2e/cli-args.mjs | 4 - tools/win-update-e2e/installer-pair-proof.mjs | 35 -------- .../installer-pair-proof.test.mjs | 88 ------------------- tools/win-update-e2e/run.mjs | 31 ++----- 13 files changed, 44 insertions(+), 255 deletions(-) delete mode 100644 tools/win-update-e2e/installer-pair-proof.mjs delete mode 100644 tools/win-update-e2e/installer-pair-proof.test.mjs diff --git a/.github/workflows/win-update-e2e.yml b/.github/workflows/win-update-e2e.yml index 58a27b82d4c..21c788256b2 100644 --- a/.github/workflows/win-update-e2e.yml +++ b/.github/workflows/win-update-e2e.yml @@ -128,7 +128,6 @@ jobs: node tools/win-update-e2e/run.mjs ` --from "$env:FROM_EXE" ` --to "$env:TO_EXE" ` - --require-distinct-artifacts ` --expect "$env:EXPECT" ` --soak-seconds "$env:SOAK_SECONDS" 2>&1 | Tee-Object -FilePath $log exit $LASTEXITCODE diff --git a/config/nsis/daemon-host-uninstall.nsh b/config/nsis/daemon-host-uninstall.nsh index fc3ebf4642a..dc3a497ce67 100644 --- a/config/nsis/daemon-host-uninstall.nsh +++ b/config/nsis/daemon-host-uninstall.nsh @@ -13,26 +13,6 @@ ; The image name and the LOCALAPPDATA folder name must stay in sync with ; DAEMON_HOST_EXE_NAME and LOCAL_HOST_ROOT_NAME in ; src/main/daemon/daemon-host-relocation.ts. - -!include "getProcessInfo.nsh" -Var pid - -!macro customCheckAppRunning - ${if} ${isUpdated} - ; Why: an A-era packaged CLI may still hold Orca.exe while its exact native launcher waits. - nsExec::Exec `"$PowerShellPath" -NoProfile -NonInteractive -ExecutionPolicy Bypass -Command "& { $$ErrorActionPreference='Stop'; $$app=[IO.Path]::GetFullPath($$args[0]); $$launcher=[IO.Path]::GetFullPath($$args[1]); $$comparison=[StringComparison]::OrdinalIgnoreCase; $$children=@(Get-CimInstance Win32_Process | Where-Object { $$_.ExecutablePath -and [String]::Equals([IO.Path]::GetFullPath($$_.ExecutablePath), $$app, $$comparison) }); $$launcherProofs=@(); foreach ($$child in $$children) { $$parent=Get-CimInstance Win32_Process -Filter ('ProcessId=' + $$child.ParentProcessId) -ErrorAction SilentlyContinue; if ($$parent -and $$parent.ExecutablePath -and [String]::Equals([IO.Path]::GetFullPath($$parent.ExecutablePath), $$launcher, $$comparison)) { $$launcherProofs += [pscustomobject]@{ ProcessId=[int]$$parent.ProcessId; CreationDate=[string]$$parent.CreationDate } }; Stop-Process -Id $$child.ProcessId -Force -ErrorAction SilentlyContinue }; $$launcherDeadline=[DateTime]::UtcNow.AddSeconds(30); do { $$remaining=@($$launcherProofs | Where-Object { $$proof=$$_; $$current=Get-CimInstance Win32_Process -Filter ('ProcessId=' + $$proof.ProcessId) -ErrorAction SilentlyContinue; $$current -and $$current.ExecutablePath -and [String]::Equals([IO.Path]::GetFullPath($$current.ExecutablePath), $$launcher, $$comparison) -and [String]::Equals([string]$$current.CreationDate, $$proof.CreationDate, [StringComparison]::Ordinal) }); if ($$remaining.Count -eq 0) { break }; Start-Sleep -Milliseconds 100 } while ([DateTime]::UtcNow -lt $$launcherDeadline); if ($$remaining.Count -ne 0) { exit 20 }; $$lockDeadline=[DateTime]::UtcNow.AddSeconds(30); $$released=$$false; do { try { $$stream=[IO.File]::Open($$app, [IO.FileMode]::Open, [IO.FileAccess]::ReadWrite, [IO.FileShare]::None); $$stream.Dispose(); $$released=$$true } catch { Start-Sleep -Milliseconds 100 } } while (-not $$released -and [DateTime]::UtcNow -lt $$lockDeadline); if (-not $$released) { exit 21 } }" "$INSTDIR\Orca.exe" "$INSTDIR\resources\bin\orca.exe"` - Pop $0 - ${if} $0 != 0 - DetailPrint "Orca update release proof failed with status $0." - SetErrorLevel 2 - Quit - ${endIf} - ${else} - !insertmacro IS_POWERSHELL_AVAILABLE - !insertmacro _CHECK_APP_RUNNING - ${endIf} -!macroend - !macro customUnInstall ${ifNot} ${isUpdated} nsExec::Exec 'taskkill /F /IM orca-terminal-daemon.exe' diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 7d3bd1717ef..10829928880 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -5266,15 +5266,15 @@ "daemon", "ssh" ], - "coverageNotes": "Deterministic units cover authority-aware legacy formatting, exact legacy worker identity planning, local worker presentation, retained-output reads after adoption, reveal-failure warnings, stable-pane Run/Dispatch routing, the SSH in-process CLI fallback, federated non-reveal, and fail-closed distinct Windows package proof. Two isolated macOS Electron journeys launch fake Codex workers through the real RPC path and record append-only spawn/interruption ledgers. They assert immediate inactive presentation, one live agent PID, stable PTY/incarnation/tab/leaf/worktree/Task/Dispatch identity, and no interruption after workspace re-entry; the restart journey additionally removes renderer ownership, marks the Dispatch legacy, retains the daemon process across an app restart, and proves exact background adoption with readable ACK output and no resume replay. Distinct A/B artifacts plus live SSH, WSL, folder, remote-runtime, Linux, and Windows cutover journeys remain explicit gaps.", + "coverageNotes": "Deterministic units cover authority-aware legacy formatting, exact legacy worker identity planning, local worker presentation, retained-output reads after adoption, reveal-failure warnings, stable-pane Run/Dispatch routing, the SSH in-process CLI fallback, and federated non-reveal. Two isolated macOS Electron journeys launch fake Codex workers through the real RPC path and record append-only spawn/interruption ledgers. They assert immediate inactive presentation, one live agent PID, stable PTY/incarnation/tab/leaf/worktree/Task/Dispatch identity, and no interruption after workspace re-entry; the restart journey additionally removes renderer ownership, marks the Dispatch legacy, retains the daemon process across an app restart, and proves exact background adoption with readable ACK output and no resume replay. Distinct A/B artifacts plus live SSH, WSL, folder, remote-runtime, Linux, and Windows cutover journeys remain explicit gaps.", "motivatingLinks": [ "https://github.com/stablyai/orca/pull/11107#discussion_r3663321387" ], "invariant": "Starting a worker in the coordinator's current workspace must materialize one inactive terminal tab before worker-start returns, preserve coordinator focus, and remain exactly once after workspace re-entry. After an app update or restart, an exact live legacy worker must fence automatic provider resume, adopt its original PTY into its original background pane, retain readable output, and clear the resume record without spawning, writing, signalling, interrupting, replacing, or focusing the worker. An exact existing target workspace must receive a discoverable tab without stealing coordinator focus; if renderer reveal fails, worker-start must expose that the live worker remains background-only. Run and Dispatch checks must resolve through the caller's stable pane identity when a terminal handle is reminted, while a live handle outranks mismatched pane metadata, explicit legacy terminal inspection remains handle-scoped, and remote or headless worker presentation remains background-only.", - "oracle": "Drive Run create, Task create, and worker-start through production Electron runtimes with a deterministic Codex fixture. Require append-only ledgers with one still-live PID and no interruption, a visible inactive worker tab while the coordinator stays active, Run delivery through stable pane identity, and stable PTY/incarnation, tab, leaf, worktree, Task, and Dispatch across workspace re-entry. In a restart journey, retain the original daemon PTY and PID, remove renderer ownership, retain sleeping-session evidence, mark the Dispatch legacy, relaunch, and require exact inactive tab adoption, readable ACK output, cleared resume state, one spawn, and no resume argv or Conversation interrupted text after another workspace round trip. Unit tests separately assert authority-specific legacy affordances, exact identity and owner matching, retained-output fallback, pane-stable routing, federated non-activation, SSH fallback parity, and distinct packaged installer paths, versions, and hashes.", + "oracle": "Drive Run create, Task create, and worker-start through production Electron runtimes with a deterministic Codex fixture. Require append-only ledgers with one still-live PID and no interruption, a visible inactive worker tab while the coordinator stays active, Run delivery through stable pane identity, and stable PTY/incarnation, tab, leaf, worktree, Task, and Dispatch across workspace re-entry. In a restart journey, retain the original daemon PTY and PID, remove renderer ownership, retain sleeping-session evidence, mark the Dispatch legacy, relaunch, and require exact inactive tab adoption, readable ACK output, cleared resume state, one spawn, and no resume argv or Conversation interrupted text after another workspace round trip. Unit tests separately assert authority-specific legacy affordances, exact identity and owner matching, retained-output fallback, pane-stable routing, federated non-activation, and SSH fallback parity.", "commands": [ "pnpm exec vitest run --config config/vitest.config.ts src/cli/handlers/orchestration.test.ts src/cli/handlers/orchestration-check-identity.test.ts src/cli/handlers/orchestration-worker-cli.test.ts src/main/runtime/rpc/methods/orchestration.test.ts src/main/ssh/ssh-remote-orca-cli.test.ts", - "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orchestration/formatter.test.ts src/main/runtime/rpc/methods/orchestration-federation.test.ts tools/win-update-e2e/installer-pair-proof.test.mjs", + "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orchestration/formatter.test.ts src/main/runtime/rpc/methods/orchestration-federation.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orchestration/orchestration-legacy-worker-terminal-recovery.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts", "pnpm run test:e2e -- tests/e2e/orchestration-worker-terminal-visibility.spec.ts --workers=1", @@ -5291,8 +5291,7 @@ "src/main/runtime/rpc/methods/orchestration-federation.test.ts", "src/main/ssh/ssh-remote-orca-cli.test.ts", "tests/e2e/orchestration-worker-terminal-visibility.spec.ts", - "tests/e2e/orchestration-legacy-worker-restart-recovery.spec.ts", - "tools/win-update-e2e/installer-pair-proof.test.mjs" + "tests/e2e/orchestration-legacy-worker-restart-recovery.spec.ts" ], "assertionRefs": [ { @@ -5373,13 +5372,6 @@ "sleeping-session, resume-claim, and pending-startup state are cleared after adoption", "no second spawn, resume argv, input, signal, interruption, or duplicate tab occurs after workspace re-entry" ] - }, - { - "file": "tools/win-update-e2e/installer-pair-proof.test.mjs", - "assertions": [ - "distinct package proof records different paths, versions, and SHA-256 hashes", - "identical paths, bytes, versions, or unreadable versions fail closed" - ] } ], "evidenceRuns": [ @@ -5396,10 +5388,10 @@ "date": "2026-07-28", "runner": "local", "platform": "macos", - "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orchestration/formatter.test.ts src/main/runtime/rpc/methods/orchestration-federation.test.ts tools/win-update-e2e/installer-pair-proof.test.mjs", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orchestration/formatter.test.ts src/main/runtime/rpc/methods/orchestration-federation.test.ts", "result": "passed", "durationSeconds": 2.72, - "summary": "Three focused files passed with 39 tests, covering authority-aware legacy affordances, federated non-reveal, and fail-closed distinct packaged installer proof." + "summary": "Two focused files passed with 34 tests, covering authority-aware legacy affordances and federated non-reveal." }, { "date": "2026-07-28", @@ -5440,13 +5432,12 @@ "Collect 100 consecutive passes or 14 days of stable CI history on macOS, Linux, and Windows.", "Run distinct installed A/B artifacts through separate headed and headless paired-runtime cutover journeys, plus a git-independent folder workspace parameter.", "Add Docker SSH restart/reconnect proof and physical Windows WSL correct-distro/wrong-distro proof.", - "Gate a packaged Windows real-updater journey with a blocked packaged CLI ask, launcher status 75, exact native/WSL resume commands, and one question/reply.", "Attach saved intentional-break artifacts for hidden local presentation and dropped stable-pane delivery." ], "knownGaps": [ "No distinct installed A/B headed or headless paired-runtime cutover is attached; the restart journey relaunches the same build while preserving the daemon and agent.", "No Docker SSH restart/reconnect, physical Windows WSL distro-authority, or git-independent folder-workspace cutover journey is attached.", - "The Windows workflow now rejects non-distinct package inputs, but it does not yet drive the packaged updater with a blocked packaged CLI ask or prove status 75 and native/WSL resume.", + "Packaged Windows updater and uninstaller continuity is owned by separate updater reliability work and is not claimed by this gate.", "The Electron journey uses a deterministic fake Codex CLI rather than a real account.", "The local presentation journey tolerates its existing terminal-handle remint and proves continuity by PTY/incarnation/tab/leaf; byte-stable handle proof across a real A/B cutover awaits the runtime-authority/RPC implementation.", "The local Electron restart proof covers retained visible output from the same-build daemon checkpoint, not transcript recovery after a transport cut or distinct A/B runtime replacement." diff --git a/docs/reference/plans/2026-07-28-orchestration-seamless-update-migration.md b/docs/reference/plans/2026-07-28-orchestration-seamless-update-migration.md index b648e2601f5..18ceea5bff2 100644 --- a/docs/reference/plans/2026-07-28-orchestration-seamless-update-migration.md +++ b/docs/reference/plans/2026-07-28-orchestration-seamless-update-migration.md @@ -417,28 +417,10 @@ status. The command name comes only from the validated inherited than rendered. The launcher propagates status 75. No long wait starts on this path. -The candidate installer owns the backward edge: after the desktop has shut -down, it may terminate only remaining processes whose resolved executable path -is the exact installed Orca executable before replacement. This releases an -already-blocked A CLI that cannot know the new marker. Its accepted question -remains durable; retrying the exact old ask under B reconstructs the same -pending question and prints the resume command. If launcher restart -supervision can be proved, it may transparently reconnect instead, but -seamless install cannot depend on that optimization. - -After terminating the exact installed child, the installer waits boundedly for -its exact descendant native launcher to exit and for the installed executable -lock to clear. Replacement fails explicitly if either release cannot be -proved; it never proceeds from a broad process-name match or an assumed delay. - -A non-skipped packaged Windows A-to-B test uses the real launcher and updater, -cuts a blocked ask, verifies post-commit output flush and status 75 propagation -on native and packaged WSL marker-aware paths, including an executable -`orca-ide` WSL resume command. It separately proves the candidate installer -terminates an exact-path marker-unaware A child and observes the exact launcher -exit plus lock release. The test fails if either release is unknown. It then -installs B, resumes the same question, and records one question/reply. This -fallback is product behavior, not test-only guidance. +This compatibility path makes the accepted question recoverable without +claiming Windows updater or uninstaller process ownership. Generic executable +drain, replacement, and uninstall behavior remains separate updater +reliability work and requires its own Windows artifact proof. ## Rendering and authoritative guidance @@ -510,7 +492,7 @@ Task, and Dispatch; emits no provider-resume spawn or interruption; preserves coordinator focus; and remains inert after switching away and back. This candidate/candidate fixture covers the renderer regression deterministically but does not replace the distinct installed-A/candidate-B packaged cutover -journey below. +journey below. It does not claim packaged Windows updater behavior. Use distinct installed A and B artifacts plus an external append-only fake agent spawn ledger: @@ -558,11 +540,9 @@ Run separate remote journeys: - a git-independent folder workspace parameter through the same adoption journey. -The Windows test is a non-skipped PR/release gate. It installs distinct A and -candidate-B packages, drives the real packaged updater while a packaged -`orca.exe` ask is blocked, proves the launcher and child release, verifies A -and B hashes/versions differ, resumes the same question on B, and records -exactly one question and reply. +Packaged Windows updater continuity is outside this migration gate. A separate +updater change must prove candidate-built A/B install, executable release, +resume, and normal uninstall behavior on Windows before making that guarantee. ## Implementation slices diff --git a/src/main/cli/windows-launcher-asset.test.ts b/src/main/cli/windows-launcher-asset.test.ts index b615e88c88d..f491cb0771d 100644 --- a/src/main/cli/windows-launcher-asset.test.ts +++ b/src/main/cli/windows-launcher-asset.test.ts @@ -25,27 +25,4 @@ describe('packaged Windows CLI launcher asset', () => { expect(source).toContain('child.WaitForExit();') expect(source).toContain('return child.ExitCode;') }) - - it('requires exact updater process and executable release proof', () => { - const includePath = join(process.cwd(), 'config', 'nsis', 'daemon-host-uninstall.nsh') - const source = readFileSync(includePath, 'utf8') - - expect(source).toContain('!macro customCheckAppRunning') - expect(source).toContain('!include "getProcessInfo.nsh"') - expect(source).toContain('Var pid') - expect(source).toContain('[String]::Equals') - expect(source).toContain('"$INSTDIR\\Orca.exe" "$INSTDIR\\resources\\bin\\orca.exe"') - expect(source).toContain('CreationDate=[string]$$parent.CreationDate') - expect(source).toContain("'ProcessId=' + $$proof.ProcessId") - expect(source).toContain('[IO.FileShare]::None') - expect(source).toContain('$$released=$$false') - expect(source).toContain('$$released=$$true') - expect(source).not.toMatch(/(? & Pick ): HarnessStoreState { - return { + const state: HarnessStoreState = { createTab: vi.fn(() => ({ id: 'tab-minted' })), setActiveView: vi.fn(), setActiveWorktree: vi.fn(), @@ -87,6 +87,14 @@ export function createHarnessStoreState( }, ...overrides } + if (overrides.setTabLayout === undefined) { + state.setTabLayout = vi.fn( + (tabId: string, layout: HarnessStoreState['terminalLayoutsByTabId'][string]) => { + state.terminalLayoutsByTabId[tabId] = layout + } + ) + } + return state } /** Subscription no-ops for every listener useIpcEvents attaches beyond the ones under test. */ diff --git a/src/renderer/src/hooks/mobile-terminal-reveal-tab-adoption.test.ts b/src/renderer/src/hooks/mobile-terminal-reveal-tab-adoption.test.ts index 30130211c62..4f5afa434fd 100644 --- a/src/renderer/src/hooks/mobile-terminal-reveal-tab-adoption.test.ts +++ b/src/renderer/src/hooks/mobile-terminal-reveal-tab-adoption.test.ts @@ -7,6 +7,10 @@ import { const WORKTREE_ID = 'wt-1' +function revealIdentity(tabId: string) { + return { worktreeId: WORKTREE_ID, tabId, leafId: 'leaf-b', ptyId: 'pty-b' } +} + function revealSplitPaneFromMobile(harness: { createTerminal: (request: { requestId?: string @@ -49,7 +53,8 @@ describe('mobile terminal reveal tab adoption', () => { expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ requestId: 'mobile-reveal', tabId: 'tab-split', - title: 'codex' + title: 'codex', + identity: revealIdentity('tab-split') }) }) @@ -70,7 +75,8 @@ describe('mobile terminal reveal tab adoption', () => { expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ requestId: 'mobile-reveal', tabId: 'tab-split', - title: 'codex' + title: 'codex', + identity: revealIdentity('tab-split') }) }) @@ -99,7 +105,8 @@ describe('mobile terminal reveal tab adoption', () => { expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ requestId: 'mobile-reveal', tabId: 'tab-detached', - title: 'codex' + title: 'codex', + identity: revealIdentity('tab-detached') }) }) @@ -199,7 +206,8 @@ describe('mobile terminal reveal tab adoption', () => { expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ requestId: 'mobile-reveal', tabId: 'tab-detached', - title: 'codex' + title: 'codex', + identity: revealIdentity('tab-detached') }) }) diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 475608f4f3c..9d9acf18876 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -1582,11 +1582,12 @@ export function useIpcEvents(): void { requestBackgroundTerminalWorktreeMount({ worktreeId, tabIds: [tab.id] }) } if (requestId) { + // Why: attest the actual binding; recovery callers compare it with their expected identity. const identity = ptyId && tabId && leafId ? verifyTerminalRevealIdentity(useAppStore.getState(), { worktreeId, - tabId, + tabId: tab.id, leafId, ptyId }) diff --git a/tools/win-update-e2e/README.md b/tools/win-update-e2e/README.md index cc94aefc552..6c771bb1197 100644 --- a/tools/win-update-e2e/README.md +++ b/tools/win-update-e2e/README.md @@ -54,7 +54,6 @@ Or directly: `node tools/win-update-e2e/run.mjs --from ... --to ... --expect ... | `--install-dir ` | Isolated-install mode (see below) — install into `` | | `--asset-pattern ` | gh asset glob (default `*windows-setup.exe`) | | `--soak-seconds ` | Post-relaunch window-watch soak (default `180`) | -| `--require-distinct-artifacts` | Require distinct N/B paths, versions, and SHA-256 hashes | | `--keep-install` | Skip teardown/uninstall for debugging (ignored in isolated) | ### Profiles @@ -211,14 +210,6 @@ powershell -File tools/win-update-e2e/window-enum.ps1 ## Known limitations -- **This is installer/session scaffolding, not the orchestration cutover gate.** - `--require-distinct-artifacts` makes the existing Windows update journey fail - closed unless it received two provably different packages. A CI follow-up - must still drive the packaged updater with a blocked packaged `orca.exe` ask, - prove launcher status `75` and the exact native/WSL resume command, then - record one question and reply on B. That proof requires distinct installed - A/B artifacts and a physical Windows host with WSL; this harness does not - simulate either contract. - **Scrollback fidelity is best-effort.** A production build renders the terminal with WebGL, so xterm text is not reliably in the DOM and the e2e `SerializeAddon` is not exposed. When text cannot be read the check reports diff --git a/tools/win-update-e2e/cli-args.mjs b/tools/win-update-e2e/cli-args.mjs index e98ffda89eb..ed50412314e 100644 --- a/tools/win-update-e2e/cli-args.mjs +++ b/tools/win-update-e2e/cli-args.mjs @@ -52,9 +52,6 @@ Options: --keep-install Skip teardown/uninstall (leaves the app installed) --asset-pattern gh release asset glob (default: *windows-setup.exe) --soak-seconds Post-relaunch window watch duration (default: 180) - --require-distinct-artifacts - Fail unless N and N+1 have distinct paths, versions, - and SHA-256 hashes. Intended for compatibility gates. -h, --help Show this help ` @@ -74,7 +71,6 @@ export function parseArgs(argv) { installDir: takeValue(argv, '--install-dir'), keepInstall: argv.includes('--keep-install'), allowExistingInstall: argv.includes('--allow-existing-install'), - requireDistinctArtifacts: argv.includes('--require-distinct-artifacts'), usage: USAGE } diff --git a/tools/win-update-e2e/installer-pair-proof.mjs b/tools/win-update-e2e/installer-pair-proof.mjs deleted file mode 100644 index f97101c0a8b..00000000000 --- a/tools/win-update-e2e/installer-pair-proof.mjs +++ /dev/null @@ -1,35 +0,0 @@ -import { createHash } from 'node:crypto' -import { createReadStream } from 'node:fs' -import path from 'node:path' - -async function sha256(filePath) { - const hash = createHash('sha256') - for await (const chunk of createReadStream(filePath)) { - hash.update(chunk) - } - return hash.digest('hex') -} - -export async function proveDistinctInstallerPair({ fromPath, toPath, fromVersion, toVersion }) { - const [fromHash, toHash] = await Promise.all([sha256(fromPath), sha256(toPath)]) - const proof = { - from: { path: path.resolve(fromPath), version: fromVersion, sha256: fromHash }, - to: { path: path.resolve(toPath), version: toVersion, sha256: toHash } - } - const errors = [] - if (proof.from.path.toLowerCase() === proof.to.path.toLowerCase()) { - errors.push('base and candidate installer paths are identical') - } - if (fromHash === toHash) { - errors.push('base and candidate installer SHA-256 hashes are identical') - } - if (!fromVersion || !toVersion) { - errors.push('both installer versions must be readable') - } else if (fromVersion === toVersion) { - errors.push(`base and candidate installer versions are identical (${fromVersion})`) - } - if (errors.length > 0) { - throw new Error(`Distinct installer proof failed: ${errors.join('; ')}`) - } - return proof -} diff --git a/tools/win-update-e2e/installer-pair-proof.test.mjs b/tools/win-update-e2e/installer-pair-proof.test.mjs deleted file mode 100644 index 203170f5eef..00000000000 --- a/tools/win-update-e2e/installer-pair-proof.test.mjs +++ /dev/null @@ -1,88 +0,0 @@ -import { mkdtemp, rm, writeFile } from 'node:fs/promises' -import os from 'node:os' -import path from 'node:path' -import { afterEach, describe, expect, it } from 'vitest' -import { parseArgs } from './cli-args.mjs' -import { proveDistinctInstallerPair } from './installer-pair-proof.mjs' - -const scratchDirs = [] - -async function artifactPair(fromBody = 'artifact-a', toBody = 'artifact-b') { - const directory = await mkdtemp(path.join(os.tmpdir(), 'orca-installer-pair-proof-')) - scratchDirs.push(directory) - const fromPath = path.join(directory, 'orca-a.exe') - const toPath = path.join(directory, 'orca-b.exe') - await Promise.all([writeFile(fromPath, fromBody), writeFile(toPath, toBody)]) - return { fromPath, toPath } -} - -afterEach(async () => { - await Promise.all(scratchDirs.splice(0).map((directory) => rm(directory, { recursive: true }))) -}) - -describe('packaged installer pair proof', () => { - it('requires an explicit compatibility-gate flag', () => { - const opts = parseArgs([ - '--from', - 'a.exe', - '--to', - 'b.exe', - '--expect', - 'cold-restore', - '--require-distinct-artifacts' - ]) - - expect(opts.requireDistinctArtifacts).toBe(true) - }) - - it('records distinct paths, versions, and hashes', async () => { - const pair = await artifactPair() - - const proof = await proveDistinctInstallerPair({ - ...pair, - fromVersion: '1.4.130', - toVersion: '1.4.131' - }) - - expect(proof.from.path).not.toBe(proof.to.path) - expect(proof.from.version).not.toBe(proof.to.version) - expect(proof.from.sha256).not.toBe(proof.to.sha256) - }) - - it('fails closed for byte-identical artifacts at different paths', async () => { - const pair = await artifactPair('same-artifact', 'same-artifact') - - await expect( - proveDistinctInstallerPair({ - ...pair, - fromVersion: '1.4.130', - toVersion: '1.4.131' - }) - ).rejects.toThrow('SHA-256 hashes are identical') - }) - - it('fails closed for an identical path or version', async () => { - const pair = await artifactPair() - - await expect( - proveDistinctInstallerPair({ - fromPath: pair.fromPath, - toPath: pair.fromPath, - fromVersion: '1.4.130', - toVersion: '1.4.130' - }) - ).rejects.toThrow(/paths are identical.*hashes are identical.*versions are identical/) - }) - - it('fails closed when either package version is unavailable', async () => { - const pair = await artifactPair() - - await expect( - proveDistinctInstallerPair({ - ...pair, - fromVersion: null, - toVersion: '1.4.131' - }) - ).rejects.toThrow('both installer versions must be readable') - }) -}) diff --git a/tools/win-update-e2e/run.mjs b/tools/win-update-e2e/run.mjs index d9f8d8b5b10..7c8d8cde6c5 100644 --- a/tools/win-update-e2e/run.mjs +++ b/tools/win-update-e2e/run.mjs @@ -12,13 +12,7 @@ import { execFileSync } from 'node:child_process' import { assertWin32 } from './platform-guard.mjs' import { parseArgs } from './cli-args.mjs' import { preflight } from './preflight.mjs' -import { - getExeVersion, - resolveInstaller, - silentInstall, - silentUninstall -} from './installer-steps.mjs' -import { proveDistinctInstallerPair } from './installer-pair-proof.mjs' +import { resolveInstaller, silentInstall, silentUninstall } from './installer-steps.mjs' import { backupInstallState, restoreInstallState } from './registry-shortcut-backup.mjs' import { launchInstalledApp, @@ -183,24 +177,6 @@ async function runProof(ctx, args) { releaseTag: opts.fromRelease, assetPattern: opts.assetPattern }) - const toInstaller = resolveInstaller({ - localPath: opts.to, - releaseTag: opts.toRelease, - assetPattern: opts.assetPattern - }) - if (opts.requireDistinctArtifacts) { - const artifactProof = await proveDistinctInstallerPair({ - fromPath: fromInstaller, - toPath: toInstaller, - fromVersion: getExeVersion(fromInstaller), - toVersion: getExeVersion(toInstaller) - }) - log( - 'artifact-proof', - `A ${artifactProof.from.version} ${artifactProof.from.sha256}; ` + - `B ${artifactProof.to.version} ${artifactProof.to.sha256}` - ) - } log('install-base', `installing ${fromInstaller}`) const base = silentInstall(fromInstaller, { installDir }) // Track the install now (not only after the update at L238) so a failure @@ -254,6 +230,11 @@ async function runProof(ctx, args) { log('watch', `started (duration ${watchDuration}s) -> ${watchOut}`) // --- Update: install N+1 --- + const toInstaller = resolveInstaller({ + localPath: opts.to, + releaseTag: opts.toRelease, + assetPattern: opts.assetPattern + }) log('update', `installing ${toInstaller}`) const updated = silentInstall(toInstaller, { installDir }) // Record the exact dir the harness installed into so non-isolated teardown From 3f37e32e72c3150213ad3a04f8967b60785629d9 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:39:34 -0700 Subject: [PATCH 02/21] perf(main): move hang watchdog into a worker thread (#11344) Keep main-thread hang detection independent of the blocked Electron event loop while reducing watchdog memory from 47.1 MiB to 11.5 MiB. Preserve marker, recovery, and telemetry behavior with a bundled worker-thread entry. --- build-plugins/plain-node-entry-guard.ts | 1 - config/electron-builder.config.cjs | 1 - .../scripts/electron-builder-config.test.mjs | 8 +- electron.vite.config.ts | 5 +- ...s => hang-watchdog-detection-loop.test.ts} | 6 +- ...oop.ts => hang-watchdog-detection-loop.ts} | 10 +- .../hang-watchdog/hang-watchdog-entry-path.ts | 17 --- .../hang-watchdog-worker-path.test.ts | 26 ++++ .../hang-watchdog-worker-path.ts | 14 ++ .../hang-watchdog-worker-protocol.ts | 12 ++ .../main-thread-hang-watchdog-entry.test.ts | 3 - .../main-thread-hang-watchdog-entry.ts | 92 ++++++++----- .../main-thread-hang-watchdog.test.ts | 126 +++++++++++++----- .../main-thread-hang-watchdog.ts | 101 +++++++------- 14 files changed, 263 insertions(+), 159 deletions(-) rename src/main/hang-watchdog/{hang-watchdog-child-loop.test.ts => hang-watchdog-detection-loop.test.ts} (96%) rename src/main/hang-watchdog/{hang-watchdog-child-loop.ts => hang-watchdog-detection-loop.ts} (86%) delete mode 100644 src/main/hang-watchdog/hang-watchdog-entry-path.ts create mode 100644 src/main/hang-watchdog/hang-watchdog-worker-path.test.ts create mode 100644 src/main/hang-watchdog/hang-watchdog-worker-path.ts create mode 100644 src/main/hang-watchdog/hang-watchdog-worker-protocol.ts diff --git a/build-plugins/plain-node-entry-guard.ts b/build-plugins/plain-node-entry-guard.ts index 6638cca97c5..a7ad171a402 100644 --- a/build-plugins/plain-node-entry-guard.ts +++ b/build-plugins/plain-node-entry-guard.ts @@ -21,7 +21,6 @@ type OutputChunk = Rollup.OutputChunk const PLAIN_NODE_ENTRY_NAMES = [ 'daemon-entry', 'parcel-watcher-process-entry', - 'main-thread-hang-watchdog-entry', 'computer-sidecar', 'agent-hooks/managed-agent-hook-controls', 'codex/codex-app-server-grant-entry' diff --git a/config/electron-builder.config.cjs b/config/electron-builder.config.cjs index 91cb45da285..0a96abb6d24 100644 --- a/config/electron-builder.config.cjs +++ b/config/electron-builder.config.cjs @@ -153,7 +153,6 @@ module.exports = { 'out/main/plugin-host-entry.js', 'out/main/computer-sidecar.js', 'out/main/parcel-watcher-process-entry.js', - 'out/main/main-thread-hang-watchdog-entry.js', 'out/main/chunks/**', 'resources/**', 'node_modules/ws/**', diff --git a/config/scripts/electron-builder-config.test.mjs b/config/scripts/electron-builder-config.test.mjs index 9712e5edace..0a3908bdf5e 100644 --- a/config/scripts/electron-builder-config.test.mjs +++ b/config/scripts/electron-builder-config.test.mjs @@ -166,11 +166,9 @@ describe('electron-builder config', () => { ) }) - // Why: the watchdog only arms in packaged builds, and its ELECTRON_RUN_AS_NODE - // fork resolves the entry from app.asar.unpacked — inside the asar it never runs. - it('unpacks the forked main-thread hang-watchdog entry', () => { - expect(electronBuilderConfig.asarUnpack).toEqual( - expect.arrayContaining(['out/main/main-thread-hang-watchdog-entry.js']) + it('keeps the worker-thread hang watchdog inside app.asar', () => { + expect(electronBuilderConfig.asarUnpack).not.toContain( + 'out/main/main-thread-hang-watchdog-entry.js' ) }) diff --git a/electron.vite.config.ts b/electron.vite.config.ts index 7388211487a..6f21d967652 100644 --- a/electron.vite.config.ts +++ b/electron.vite.config.ts @@ -208,9 +208,8 @@ export const electronViteConfig: UserConfig = { // Why: forked with ELECTRON_RUN_AS_NODE so @parcel/watcher faults // can't take down the main process (issue #7547). 'parcel-watcher-process-entry': resolve('src/main/ipc/parcel-watcher-process-entry.ts'), - // Why: forked with ELECTRON_RUN_AS_NODE so it survives a deadlocked - // main thread (macOS 26 AppKit scene-update deadlock) and can record - // the stall for the next launch to report. + // Why: a worker thread survives the macOS 26 AppKit main-thread deadlock + // without paying for another Electron process. 'main-thread-hang-watchdog-entry': resolve( 'src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts' ), diff --git a/src/main/hang-watchdog/hang-watchdog-child-loop.test.ts b/src/main/hang-watchdog/hang-watchdog-detection-loop.test.ts similarity index 96% rename from src/main/hang-watchdog/hang-watchdog-child-loop.test.ts rename to src/main/hang-watchdog/hang-watchdog-detection-loop.test.ts index 8d390c04e8e..8d1c3ebfcd1 100644 --- a/src/main/hang-watchdog/hang-watchdog-child-loop.test.ts +++ b/src/main/hang-watchdog/hang-watchdog-detection-loop.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { createHangWatchdogChildLoop } from './hang-watchdog-child-loop' +import { createHangWatchdogDetectionLoop } from './hang-watchdog-detection-loop' const TIMEOUT_MS = 45_000 const CHECK_INTERVAL_MS = 5_000 @@ -8,7 +8,7 @@ function loopWithClock(startAt = 0) { let now = startAt const onHangDetected = vi.fn() const onHangResolved = vi.fn() - const loop = createHangWatchdogChildLoop({ + const loop = createHangWatchdogDetectionLoop({ timeoutMs: TIMEOUT_MS, checkIntervalMs: CHECK_INTERVAL_MS, now: () => now, @@ -18,7 +18,7 @@ function loopWithClock(startAt = 0) { return { loop, onHangDetected, onHangResolved, advance: (ms: number) => (now += ms) } } -describe('createHangWatchdogChildLoop', () => { +describe('createHangWatchdogDetectionLoop', () => { it('does not fire while heartbeats keep arriving', () => { const { loop, onHangDetected, advance } = loopWithClock() for (let i = 0; i < 100; i++) { diff --git a/src/main/hang-watchdog/hang-watchdog-child-loop.ts b/src/main/hang-watchdog/hang-watchdog-detection-loop.ts similarity index 86% rename from src/main/hang-watchdog/hang-watchdog-child-loop.ts rename to src/main/hang-watchdog/hang-watchdog-detection-loop.ts index 3a8ebb34fdf..9d45cba7797 100644 --- a/src/main/hang-watchdog/hang-watchdog-child-loop.ts +++ b/src/main/hang-watchdog/hang-watchdog-detection-loop.ts @@ -1,4 +1,4 @@ -export type HangWatchdogChildLoopConfig = { +export type HangWatchdogDetectionLoopConfig = { timeoutMs: number checkIntervalMs: number now: () => number @@ -7,14 +7,14 @@ export type HangWatchdogChildLoopConfig = { onHangResolved: (unresponsiveMs: number) => void } -export type HangWatchdogChildLoop = { +export type HangWatchdogDetectionLoop = { recordHeartbeat: () => void tick: () => void } -export function createHangWatchdogChildLoop( - config: HangWatchdogChildLoopConfig -): HangWatchdogChildLoop { +export function createHangWatchdogDetectionLoop( + config: HangWatchdogDetectionLoopConfig +): HangWatchdogDetectionLoop { let lastHeartbeatAt = config.now() let lastTickAt = config.now() let detected = false diff --git a/src/main/hang-watchdog/hang-watchdog-entry-path.ts b/src/main/hang-watchdog/hang-watchdog-entry-path.ts deleted file mode 100644 index 74f8b1d0089..00000000000 --- a/src/main/hang-watchdog/hang-watchdog-entry-path.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { existsSync } from 'node:fs' -import { join } from 'node:path' - -export function resolveHangWatchdogEntryPath( - appPath: string, - isPackaged: boolean, - pathExists: (candidate: string) => boolean = existsSync -): string { - // Why: ELECTRON_RUN_AS_NODE bypasses asar integration, so the packaged entry must be forked from app.asar.unpacked. - const basePath = isPackaged ? appPath.replace('app.asar', 'app.asar.unpacked') : appPath - const adjacentBuildEntry = join(basePath, 'main-thread-hang-watchdog-entry.js') - // Why: electron-vite's unpackaged appPath is already out/main; appending out/main again would silently disable the watchdog in dev and E2E builds. - if (!isPackaged && pathExists(adjacentBuildEntry)) { - return adjacentBuildEntry - } - return join(basePath, 'out', 'main', 'main-thread-hang-watchdog-entry.js') -} diff --git a/src/main/hang-watchdog/hang-watchdog-worker-path.test.ts b/src/main/hang-watchdog/hang-watchdog-worker-path.test.ts new file mode 100644 index 00000000000..3ade5b1cbf3 --- /dev/null +++ b/src/main/hang-watchdog/hang-watchdog-worker-path.test.ts @@ -0,0 +1,26 @@ +import { join } from 'node:path' +import { describe, expect, it, vi } from 'vitest' +import { resolveHangWatchdogWorkerPath } from './hang-watchdog-worker-path' + +describe('resolveHangWatchdogWorkerPath', () => { + it('resolves packaged workers inside app.asar', () => { + const appPath = join('/apps', 'orca', 'app.asar') + expect(resolveHangWatchdogWorkerPath(appPath, true)).toBe( + join(appPath, 'out', 'main', 'main-thread-hang-watchdog-entry.js') + ) + }) + + it('uses an adjacent entry when the dev app path is out/main', () => { + const appPath = join('/repo', 'out', 'main') + const adjacent = join(appPath, 'main-thread-hang-watchdog-entry.js') + const pathExists = vi.fn((candidate: string) => candidate === adjacent) + expect(resolveHangWatchdogWorkerPath(appPath, false, pathExists)).toBe(adjacent) + }) + + it('resolves through out/main from a dev project root', () => { + const appPath = join('/repo', 'orca') + expect(resolveHangWatchdogWorkerPath(appPath, false, () => false)).toBe( + join(appPath, 'out', 'main', 'main-thread-hang-watchdog-entry.js') + ) + }) +}) diff --git a/src/main/hang-watchdog/hang-watchdog-worker-path.ts b/src/main/hang-watchdog/hang-watchdog-worker-path.ts new file mode 100644 index 00000000000..7efc300cb45 --- /dev/null +++ b/src/main/hang-watchdog/hang-watchdog-worker-path.ts @@ -0,0 +1,14 @@ +import { existsSync } from 'node:fs' +import { join } from 'node:path' + +export function resolveHangWatchdogWorkerPath( + appPath: string, + isPackaged: boolean, + pathExists: (candidate: string) => boolean = existsSync +): string { + const adjacentBuildEntry = join(appPath, 'main-thread-hang-watchdog-entry.js') + if (!isPackaged && pathExists(adjacentBuildEntry)) { + return adjacentBuildEntry + } + return join(appPath, 'out', 'main', 'main-thread-hang-watchdog-entry.js') +} diff --git a/src/main/hang-watchdog/hang-watchdog-worker-protocol.ts b/src/main/hang-watchdog/hang-watchdog-worker-protocol.ts new file mode 100644 index 00000000000..14e6d007796 --- /dev/null +++ b/src/main/hang-watchdog/hang-watchdog-worker-protocol.ts @@ -0,0 +1,12 @@ +export const HANG_WATCHDOG_HEARTBEAT_INTERVAL_MS = 2_000 +export const HANG_WATCHDOG_TIMEOUT_MS = 45_000 +export const HANG_WATCHDOG_CHECK_INTERVAL_MS = 5_000 + +export type HangWatchdogWorkerData = { + parentPid: number + markerPath: string + timeoutMs: number + checkIntervalMs: number +} + +export type MainToHangWatchdogWorkerMessage = { type: 'heartbeat' } | { type: 'shutdown' } diff --git a/src/main/hang-watchdog/main-thread-hang-watchdog-entry.test.ts b/src/main/hang-watchdog/main-thread-hang-watchdog-entry.test.ts index 0f36e8acad9..2f307e3ce2b 100644 --- a/src/main/hang-watchdog/main-thread-hang-watchdog-entry.test.ts +++ b/src/main/hang-watchdog/main-thread-hang-watchdog-entry.test.ts @@ -67,9 +67,6 @@ describe('recordHangObservation', () => { }) }) - // Why: this is the safety contract of the whole PR. Observing a hang must never kill, relaunch, - // or exit — a false positive would SIGKILL a live main thread mid-write. If a future change - // reintroduces recovery, this test must fail loudly rather than ship silently. it('never spawns, signals, or exits', () => { recordHangObservation({ parentPid: 4242, diff --git a/src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts b/src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts index 8d45c725a72..1b100b73566 100644 --- a/src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts +++ b/src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts @@ -1,20 +1,12 @@ -// Forked with ELECTRON_RUN_AS_NODE from the main process. Watches heartbeats from the main -// thread; if they stop (e.g. the macOS 26 AppKit scene-update deadlock) it records a marker so -// the next launch can report the stall, and rewrites it if the heartbeats come back. -// -// Why no kill: a deadlocked main thread has already lost whatever sat in the persistence debounce -// window, so killing it recovers nothing the user could not recover by force-quitting. A false -// positive, though, would SIGKILL a live main thread that was merely blocked — most plausibly on -// I/O, i.e. exactly when writes are in flight. All of the downside sits in the misfire, so this -// measures first: `selfRecovered` counts the stalls a killer would have gotten wrong. -// -// Must never import electron. -import { createHangWatchdogChildLoop } from './hang-watchdog-child-loop' +import { isMainThread, parentPort, workerData } from 'node:worker_threads' +import { createHangWatchdogDetectionLoop } from './hang-watchdog-detection-loop' import { writeHangDetectionMarker } from './hang-detection-marker' +import type { + HangWatchdogWorkerData, + MainToHangWatchdogWorkerMessage +} from './hang-watchdog-worker-protocol' -const DEFAULT_TIMEOUT_MS = 45_000 -const DEFAULT_CHECK_INTERVAL_MS = 5_000 - +// Observation only: a false positive must never kill a live main thread mid-write. export function recordHangObservation(options: { parentPid: number markerPath: string @@ -36,37 +28,63 @@ export function recordHangObservation(options: { } } -function runWatchdog(parentPid: number): void { - const markerPath = process.env.ORCA_HANG_WATCHDOG_MARKER_PATH ?? '' - const timeoutMs = Number(process.env.ORCA_HANG_WATCHDOG_TIMEOUT_MS) || DEFAULT_TIMEOUT_MS - const checkIntervalMs = - Number(process.env.ORCA_HANG_WATCHDOG_CHECK_INTERVAL_MS) || DEFAULT_CHECK_INTERVAL_MS - - const loop = createHangWatchdogChildLoop({ - timeoutMs, - checkIntervalMs, +function runWatchdog(config: HangWatchdogWorkerData): void { + const port = parentPort + if (!port) { + return + } + const loop = createHangWatchdogDetectionLoop({ + timeoutMs: config.timeoutMs, + checkIntervalMs: config.checkIntervalMs, now: () => Date.now(), onHangDetected: (unresponsiveMs) => - recordHangObservation({ parentPid, markerPath, unresponsiveMs, selfRecovered: false }), + recordHangObservation({ + parentPid: config.parentPid, + markerPath: config.markerPath, + unresponsiveMs, + selfRecovered: false + }), // Why: rewriting the marker keeps one observation per stall rather than two rows to reconcile. onHangResolved: (unresponsiveMs) => - recordHangObservation({ parentPid, markerPath, unresponsiveMs, selfRecovered: true }) + recordHangObservation({ + parentPid: config.parentPid, + markerPath: config.markerPath, + unresponsiveMs, + selfRecovered: true + }) }) - process.on('message', (message) => { - const type = (message as { type?: string } | null)?.type - if (type === 'heartbeat') { + let checkTimer: ReturnType | null = setInterval( + () => loop.tick(), + config.checkIntervalMs + ) + port.on('message', (message: MainToHangWatchdogWorkerMessage) => { + if (message.type === 'heartbeat') { loop.recordHeartbeat() - } else if (type === 'shutdown') { - process.exit(0) + } else if (message.type === 'shutdown') { + if (checkTimer) { + clearInterval(checkTimer) + checkTimer = null + } + port.close() } }) - // Why: a normal parent exit closes the IPC channel; the watchdog must not outlive it and misfire. - process.on('disconnect', () => process.exit(0)) - setInterval(() => loop.tick(), checkIntervalMs) } -const configuredParentPid = Number(process.env.ORCA_HANG_WATCHDOG_PARENT_PID) -if (Number.isInteger(configuredParentPid) && configuredParentPid > 0) { - runWatchdog(configuredParentPid) +function isHangWatchdogWorkerData(value: unknown): value is HangWatchdogWorkerData { + const data = value as Partial | null + return ( + !!data && + Number.isInteger(data.parentPid) && + (data.parentPid ?? 0) > 0 && + typeof data.markerPath === 'string' && + Number.isFinite(data.timeoutMs) && + (data.timeoutMs ?? 0) > 0 && + Number.isFinite(data.checkIntervalMs) && + (data.checkIntervalMs ?? 0) > 0 + ) +} + +if (!isMainThread && isHangWatchdogWorkerData(workerData)) { + runWatchdog(workerData) } diff --git a/src/main/hang-watchdog/main-thread-hang-watchdog.test.ts b/src/main/hang-watchdog/main-thread-hang-watchdog.test.ts index b5c1063a915..eaaea54225b 100644 --- a/src/main/hang-watchdog/main-thread-hang-watchdog.test.ts +++ b/src/main/hang-watchdog/main-thread-hang-watchdog.test.ts @@ -1,7 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { join } from 'node:path' -const { forkMock, appMock } = vi.hoisted(() => ({ - forkMock: vi.fn(), +const { workerState, appMock } = vi.hoisted(() => ({ + workerState: { + calls: [] as unknown[][], + instance: null as object | null, + error: null as Error | null + }, appMock: { isPackaged: true, getAppPath: vi.fn(() => '/apps/orca/app.asar'), @@ -9,8 +14,16 @@ const { forkMock, appMock } = vi.hoisted(() => ({ } })) -vi.mock('node:child_process', () => ({ - fork: forkMock +vi.mock('node:worker_threads', () => ({ + Worker: class WorkerMock { + constructor(...args: unknown[]) { + workerState.calls.push(args) + if (workerState.error) { + throw workerState.error + } + return workerState.instance as WorkerMock + } + } })) vi.mock('electron', () => ({ @@ -29,28 +42,33 @@ function withPlatform(platform: NodeJS.Platform, run: () => T): T { } } -function fakeChild() { +function fakeWorker() { return { - connected: true, - stderr: { on: vi.fn() }, + postMessage: vi.fn(), + unref: vi.fn(), on: vi.fn(), - send: vi.fn(), - disconnect: vi.fn(), - kill: vi.fn() + once: vi.fn() } } describe('installMainThreadHangWatchdog', () => { beforeEach(() => { vi.useFakeTimers() - forkMock.mockReset() + workerState.calls = [] + workerState.instance = null + workerState.error = null appMock.on.mockReset() appMock.isPackaged = true delete process.env.ORCA_HANG_WATCHDOG_FORCE + delete process.env.ORCA_HANG_WATCHDOG_TIMEOUT_MS + delete process.env.ORCA_HANG_WATCHDOG_CHECK_INTERVAL_MS }) afterEach(() => { vi.useRealTimers() + delete process.env.ORCA_HANG_WATCHDOG_FORCE + delete process.env.ORCA_HANG_WATCHDOG_TIMEOUT_MS + delete process.env.ORCA_HANG_WATCHDOG_CHECK_INTERVAL_MS }) it('is a no-op off macOS', () => { @@ -60,7 +78,7 @@ describe('installMainThreadHangWatchdog', () => { expect( withPlatform('linux', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) ).toBeNull() - expect(forkMock).not.toHaveBeenCalled() + expect(workerState.calls).toHaveLength(0) }) it('is a no-op in unpackaged builds unless forced', () => { @@ -69,62 +87,98 @@ describe('installMainThreadHangWatchdog', () => { withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) ).toBeNull() process.env.ORCA_HANG_WATCHDOG_FORCE = '1' - const child = fakeChild() - forkMock.mockReturnValue(child) + const worker = fakeWorker() + workerState.instance = worker expect( withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) ).not.toBeNull() delete process.env.ORCA_HANG_WATCHDOG_FORCE }) - it('forks the watchdog as plain Node with pid, bundle, and marker config', () => { - const child = fakeChild() - forkMock.mockReturnValue(child) + it('starts a worker with pid, marker, and timing config', () => { + const worker = fakeWorker() + workerState.instance = worker const handle = withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' }) ) expect(handle).not.toBeNull() - const [, , options] = forkMock.mock.calls[0] - expect(options.env.ELECTRON_RUN_AS_NODE).toBe('1') - expect(options.env.ORCA_HANG_WATCHDOG_PARENT_PID).toBe(String(process.pid)) - expect(options.env.ORCA_HANG_WATCHDOG_MARKER_PATH).toContain('/ud') + const [workerPath, rawOptions] = workerState.calls[0] + const options = rawOptions as { + name: string + workerData: { + parentPid: number + markerPath: string + timeoutMs: number + checkIntervalMs: number + } + } + expect(workerPath).toBe( + join('/apps/orca/app.asar', 'out', 'main', 'main-thread-hang-watchdog-entry.js') + ) + expect(options.name).toBe('orca-main-thread-hang-watchdog') + expect(options.workerData).toMatchObject({ + parentPid: process.pid, + markerPath: join('/ud', 'main-thread-hang.json'), + timeoutMs: 45_000, + checkIntervalMs: 5_000 + }) + expect(worker.unref).toHaveBeenCalled() }) - it('sends heartbeats on an interval and shutdown+disconnect on stop', () => { - const child = fakeChild() - forkMock.mockReturnValue(child) + it('sends heartbeats on an interval and shutdown on stop', () => { + const worker = fakeWorker() + workerState.instance = worker const handle = withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' }) ) vi.advanceTimersByTime(6_000) - const heartbeats = child.send.mock.calls.filter(([m]) => m.type === 'heartbeat') + const heartbeats = worker.postMessage.mock.calls.filter(([m]) => m.type === 'heartbeat') expect(heartbeats.length).toBe(3) handle?.stop() - expect(child.send.mock.calls.some(([m]) => m.type === 'shutdown')).toBe(true) - expect(child.disconnect).toHaveBeenCalled() + expect(worker.postMessage.mock.calls.some(([m]) => m.type === 'shutdown')).toBe(true) - // Why: quit fires will-quit twice; a second stop must not resend or throw. handle?.stop() - const shutdowns = child.send.mock.calls.filter(([m]) => m.type === 'shutdown') + const shutdowns = worker.postMessage.mock.calls.filter(([m]) => m.type === 'shutdown') expect(shutdowns.length).toBe(1) vi.advanceTimersByTime(10_000) - const heartbeatsAfterStop = child.send.mock.calls.filter(([m]) => m.type === 'heartbeat') + const heartbeatsAfterStop = worker.postMessage.mock.calls.filter( + ([m]) => m.type === 'heartbeat' + ) expect(heartbeatsAfterStop.length).toBe(3) }) + it('passes test timing overrides to the worker', () => { + process.env.ORCA_HANG_WATCHDOG_TIMEOUT_MS = '900' + process.env.ORCA_HANG_WATCHDOG_CHECK_INTERVAL_MS = '100' + workerState.instance = fakeWorker() + withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) + const options = workerState.calls[0][1] as { + workerData: { timeoutMs: number; checkIntervalMs: number } + } + expect(options.workerData).toMatchObject({ timeoutMs: 900, checkIntervalMs: 100 }) + }) + it('registers stop on will-quit', () => { - const child = fakeChild() - forkMock.mockReturnValue(child) + workerState.instance = fakeWorker() withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) expect(appMock.on).toHaveBeenCalledWith('will-quit', expect.any(Function)) }) - it('returns null and stays inert when the fork itself fails', () => { - forkMock.mockImplementation(() => { - throw new Error('spawn failure') - }) + it('stops heartbeat work when the worker exits', () => { + const worker = fakeWorker() + workerState.instance = worker + withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) + const exitListener = worker.once.mock.calls.find(([event]) => event === 'exit')?.[1] + expect(exitListener).toEqual(expect.any(Function)) + exitListener() + vi.advanceTimersByTime(6_000) + expect(worker.postMessage).not.toHaveBeenCalled() + }) + + it('returns null and stays inert when worker construction fails', () => { + workerState.error = new Error('worker failure') expect( withPlatform('darwin', () => installMainThreadHangWatchdog({ userDataPath: '/ud' })) ).toBeNull() diff --git a/src/main/hang-watchdog/main-thread-hang-watchdog.ts b/src/main/hang-watchdog/main-thread-hang-watchdog.ts index f859f8b5745..46454429fc7 100644 --- a/src/main/hang-watchdog/main-thread-hang-watchdog.ts +++ b/src/main/hang-watchdog/main-thread-hang-watchdog.ts @@ -1,19 +1,25 @@ -import { fork, type ChildProcess } from 'node:child_process' +import { Worker } from 'node:worker_threads' import { app } from 'electron' -import { resolveHangWatchdogEntryPath } from './hang-watchdog-entry-path' import { hangDetectionMarkerPath } from './hang-detection-marker' - -const HEARTBEAT_INTERVAL_MS = 2_000 +import { resolveHangWatchdogWorkerPath } from './hang-watchdog-worker-path' +import { + HANG_WATCHDOG_CHECK_INTERVAL_MS, + HANG_WATCHDOG_HEARTBEAT_INTERVAL_MS, + HANG_WATCHDOG_TIMEOUT_MS, + type HangWatchdogWorkerData, + type MainToHangWatchdogWorkerMessage +} from './hang-watchdog-worker-protocol' export type MainThreadHangWatchdogHandle = { stop: () => void - child: ChildProcess + worker: Worker +} + +function positiveTiming(value: string | undefined, fallback: number): number { + const parsed = Number(value) + return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback } -// Why: macOS 26 scene-backed AppKit windows can deadlock the main thread inside -// FrontBoardServices with no crash and no event loop left to self-report. A plain-Node sibling -// process watches heartbeats and records the stall so the next launch can report it. It observes -// only — see main-thread-hang-watchdog-entry.ts for why it does not kill the parent. export function installMainThreadHangWatchdog(options: { userDataPath: string }): MainThreadHangWatchdogHandle | null { @@ -24,58 +30,57 @@ export function installMainThreadHangWatchdog(options: { if (!app.isPackaged && process.env.ORCA_HANG_WATCHDOG_FORCE !== '1') { return null } - const entryPath = resolveHangWatchdogEntryPath(app.getAppPath(), app.isPackaged) - let child: ChildProcess + const workerPath = resolveHangWatchdogWorkerPath(app.getAppPath(), app.isPackaged) + const workerData: HangWatchdogWorkerData = { + parentPid: process.pid, + markerPath: hangDetectionMarkerPath(options.userDataPath), + timeoutMs: positiveTiming(process.env.ORCA_HANG_WATCHDOG_TIMEOUT_MS, HANG_WATCHDOG_TIMEOUT_MS), + checkIntervalMs: positiveTiming( + process.env.ORCA_HANG_WATCHDOG_CHECK_INTERVAL_MS, + HANG_WATCHDOG_CHECK_INTERVAL_MS + ) + } + let worker: Worker try { - child = fork(entryPath, [], { - stdio: ['ignore', 'ignore', 'pipe', 'ipc'], - env: { - ...process.env, - ELECTRON_RUN_AS_NODE: '1', - ORCA_HANG_WATCHDOG_PARENT_PID: String(process.pid), - ORCA_HANG_WATCHDOG_MARKER_PATH: hangDetectionMarkerPath(options.userDataPath) - } + // Why: the worker survives an AppKit main-thread deadlock without another Electron process. + worker = new Worker(workerPath, { + name: 'orca-main-thread-hang-watchdog', + workerData }) } catch (error) { - console.error('[hang-watchdog] failed to fork watchdog process:', error) + console.error('[hang-watchdog] failed to start watchdog worker:', error) return null } - child.stderr?.on('data', (chunk: Buffer) => { - console.error('[hang-watchdog]', String(chunk).trimEnd()) + worker.on('error', (error) => { + console.error('[hang-watchdog] watchdog worker failed:', error) }) - // Why: the watchdog is a safety net — if it dies, run without it; a restart loop here must never affect the app. - child.on('error', () => {}) - const heartbeatTimer = setInterval(() => { - if (child.connected) { - try { - child.send({ type: 'heartbeat' }, () => {}) - } catch { - // Channel raced closed between the check and the send. - } - } - }, HEARTBEAT_INTERVAL_MS) let stopped = false + const postMessage = (message: MainToHangWatchdogWorkerMessage): void => { + if (stopped && message.type === 'heartbeat') { + return + } + try { + worker.postMessage(message) + } catch { + // The worker already exited. + } + } + const heartbeatTimer = setInterval(() => { + postMessage({ type: 'heartbeat' }) + }, HANG_WATCHDOG_HEARTBEAT_INTERVAL_MS) const stop = (): void => { if (stopped) { return } stopped = true clearInterval(heartbeatTimer) - if (child.connected) { - try { - child.send({ type: 'shutdown' }, () => {}) - } catch { - // Already disconnecting. - } - } - // Why: disconnect guarantees the child observes parent shutdown and exits instead of misreading quit as a hang. - try { - child.disconnect() - } catch { - // Channel already closed. - } + postMessage({ type: 'shutdown' }) } - // Why: will-quit fires twice during quit; stop is idempotent. + worker.once('exit', () => { + stopped = true + clearInterval(heartbeatTimer) + }) + worker.unref() app.on('will-quit', stop) - return { stop, child } + return { stop, worker } } From 827207784dc0f4ea14fb451672199f81c4680687 Mon Sep 17 00:00:00 2001 From: caioribeiroclw-pixel Date: Wed, 29 Jul 2026 22:40:25 +0000 Subject: [PATCH 03/21] Add OMP skill discovery source (#6422) * Add OMP skill discovery source * review: scope OMP discovery to the shared provider and cover orchestration - Drop the `SkillProvider` widening: `~/.omp/agent/skills` is a provider home like `~/.pi/agent/skills`, so it carries `agent-skills` and identifies OMP through the source `owner`. That keeps every `Record` (including the Skills page label map) at a zero diff. - Add the `omp-home` orchestration coverage location and map OMP to it, so the Agent coverage panel stops reporting a real OMP install as missing. * review: guard the Pi/OMP matcher anchors Both roots end in agent/skills, so dropping either leading segment let one agent's install mark the other with a green suite. Also corrects the OMP owner assertion's rationale: no OMP native-chat profile exists, so owner matters because a null owner leaks OMP-only skills into other pickers. --------- Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> --- src/main/skills/discovery.test.ts | 18 ++++++++++- src/main/skills/skill-discovery-sources.ts | 8 +++++ .../lib/orchestration-skill-coverage.test.ts | 31 +++++++++++++++++++ .../src/lib/orchestration-skill-coverage.ts | 8 +++++ 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/main/skills/discovery.test.ts b/src/main/skills/discovery.test.ts index a5d113aebe9..13aec60bf67 100644 --- a/src/main/skills/discovery.test.ts +++ b/src/main/skills/discovery.test.ts @@ -23,14 +23,17 @@ describe('skill discovery', () => { const home = join(root, 'home') const repo = join(root, 'repo') const codexSkill = join(home, '.codex', 'skills', 'review') + const ompSkill = join(home, '.omp', 'agent', 'skills', 'planning') const repoSkill = join(repo, '.claude', 'skills', 'docs') await mkdir(codexSkill, { recursive: true }) + await mkdir(ompSkill, { recursive: true }) await mkdir(repoSkill, { recursive: true }) await writeFile( join(codexSkill, 'SKILL.md'), ['---', 'name: code-review', 'description: Review code changes.', '---', ''].join('\n') ) await writeFile(join(repoSkill, 'SKILL.md'), '# Docs\n\nWrite project docs.') + await writeFile(join(ompSkill, 'SKILL.md'), '# Planning\n\nPlan OMP work.') const result = await discoverSkills({ homeDir: home, @@ -38,11 +41,18 @@ describe('skill discovery', () => { repos: [makeRepo(repo)] }) - expect(result.skills.map((skill) => skill.name).sort()).toEqual(['Docs', 'code-review']) + expect(result.skills.map((skill) => skill.name).sort()).toEqual([ + 'Docs', + 'Planning', + 'code-review' + ]) expect(result.skills.find((skill) => skill.name === 'code-review')?.providers).toEqual([ 'codex' ]) expect(result.skills.find((skill) => skill.name === 'Docs')?.providers).toEqual(['claude']) + expect(result.skills.find((skill) => skill.name === 'Planning')?.providers).toEqual([ + 'agent-skills' + ]) }) it('discovers the enabled Claude plugin version applicable to the project cwd', async () => { @@ -153,6 +163,7 @@ describe('skill discovery', () => { '/home/test/.grok/skills', '/home/test/.config/opencode/skills', '/home/test/.pi/agent/skills', + '/home/test/.omp/agent/skills', '/home/test/.gemini/skills', '/home/test/.gemini/antigravity/skills', '/home/test/.cursor/skills' @@ -165,6 +176,11 @@ describe('skill discovery', () => { expect(root.providers).toEqual(['agent-skills']) } } + // Why: the native-chat picker admits a root when its owner is null, so leaving + // OMP's home shared would leak OMP-only skills into every other agent's picker. + expect( + roots.find((root) => root.path.replace(/\\/g, '/') === '/home/test/.omp/agent/skills')?.owner + ).toBe('omp') }) it('does not add runtime-owned repository paths to local scan roots', () => { diff --git a/src/main/skills/skill-discovery-sources.ts b/src/main/skills/skill-discovery-sources.ts index f17d15e1ef2..ba0cb4f4742 100644 --- a/src/main/skills/skill-discovery-sources.ts +++ b/src/main/skills/skill-discovery-sources.ts @@ -131,6 +131,14 @@ export function buildSkillDiscoverySources( ['agent-skills'], 'pi' ), + source( + 'home-omp', + 'OMP home', + pathApi.join(home, '.omp', 'agent', 'skills'), + 'home', + ['agent-skills'], + 'omp' + ), source( 'home-gemini', 'Gemini home', diff --git a/src/renderer/src/lib/orchestration-skill-coverage.test.ts b/src/renderer/src/lib/orchestration-skill-coverage.test.ts index bcb68ff74b5..b4d287e3fe8 100644 --- a/src/renderer/src/lib/orchestration-skill-coverage.test.ts +++ b/src/renderer/src/lib/orchestration-skill-coverage.test.ts @@ -115,6 +115,11 @@ describe('orchestration skill agent coverage', () => { rootPath: '/Users/test/.pi/agent/skills', directoryPath: '/Users/test/.pi/agent/skills/orchestration' }, + { + agent: 'omp', + rootPath: '/Users/test/.omp/agent/skills', + directoryPath: '/Users/test/.omp/agent/skills/orchestration' + }, { agent: 'gemini', rootPath: '/Users/test/.gemini/skills', @@ -180,6 +185,32 @@ describe('orchestration skill agent coverage', () => { expect(agentHasOrchestrationSkill('gemini', antigravityInstall)).toBe(false) }) + it('keeps Pi and OMP distinct despite sharing the agent/skills tail', () => { + const piInstall = [ + skill({ + providers: ['agent-skills'], + sourceKind: 'home', + rootPath: '/Users/test/.pi/agent/skills', + directoryPath: '/Users/test/.pi/agent/skills/orchestration' + }) + ] + const ompInstall = [ + skill({ + providers: ['agent-skills'], + sourceKind: 'home', + rootPath: 'C:\\Users\\test\\.omp\\agent\\skills', + directoryPath: 'C:\\Users\\test\\.omp\\agent\\skills\\orchestration' + }) + ] + + // Why: `.pi/agent/skills` and `.omp/agent/skills` differ only in the leading + // segment, so dropping either anchor would cross-mark the other agent. + expect(agentHasOrchestrationSkill('pi', piInstall)).toBe(true) + expect(agentHasOrchestrationSkill('omp', piInstall)).toBe(false) + expect(agentHasOrchestrationSkill('omp', ompInstall)).toBe(true) + expect(agentHasOrchestrationSkill('pi', ompInstall)).toBe(false) + }) + it('marks Claude Agent Teams from ~/.claude/skills like Claude Code', () => { const skills = [ skill({ diff --git a/src/renderer/src/lib/orchestration-skill-coverage.ts b/src/renderer/src/lib/orchestration-skill-coverage.ts index 28300112837..8645081e082 100644 --- a/src/renderer/src/lib/orchestration-skill-coverage.ts +++ b/src/renderer/src/lib/orchestration-skill-coverage.ts @@ -12,6 +12,7 @@ export type OrchestrationSkillLocationId = | 'grok-home' | 'opencode-home' | 'pi-home' + | 'omp-home' | 'gemini-home' | 'antigravity-home' | 'cursor-home' @@ -69,6 +70,12 @@ const ORCHESTRATION_SKILL_LOCATIONS: readonly OrchestrationSkillLocationDefiniti isGlobalOrchestrationSkill(skill) && pathContainsSegments(skill.rootPath, ['.pi', 'agent', 'skills']) }, + { + id: 'omp-home', + matchesSkill: (skill) => + isGlobalOrchestrationSkill(skill) && + pathContainsSegments(skill.rootPath, ['.omp', 'agent', 'skills']) + }, { // Why: segment matching keeps `.gemini/skills` (Gemini CLI) distinct from the // sibling `.gemini/antigravity/skills`, so the two never cross-mark each other. @@ -102,6 +109,7 @@ const ORCHESTRATION_SKILL_LOCATION_IDS_BY_AGENT: Partial< grok: ['grok-home', 'agents-home'], opencode: ['opencode-home', 'agents-home'], pi: ['pi-home', 'agents-home'], + omp: ['omp-home', 'agents-home'], gemini: ['gemini-home', 'agents-home'], antigravity: ['antigravity-home', 'agents-home'], cursor: ['cursor-home', 'agents-home'] From b21f978c6db5f91e1e5b4fcf6c5ce74ce9b34420 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:53:42 -0700 Subject: [PATCH 04/21] fix(codex): restore five-hour usage window (#11415) * fix(codex): restore five-hour usage window * fix(codex): reuse backend reset credit metadata --- .../rate-limits/codex-fetcher-backend.test.ts | 40 ++++ .../codex-fetcher-session-supplement.test.ts | 174 ++++++++++++++++++ src/main/rate-limits/codex-fetcher.test.ts | 47 +++++ src/main/rate-limits/codex-fetcher.ts | 105 ++++++++--- ...x-rate-limit-window-classification.test.ts | 4 +- .../codex-rate-limit-window-classification.ts | 34 ++-- .../src/components/status-bar/StatusBar.tsx | 6 +- .../status-bar/UsageRosterPanel.test.tsx | 4 + .../status-bar/inline-usage-bars.test.tsx | 25 +++ 9 files changed, 392 insertions(+), 47 deletions(-) create mode 100644 src/main/rate-limits/codex-fetcher-session-supplement.test.ts diff --git a/src/main/rate-limits/codex-fetcher-backend.test.ts b/src/main/rate-limits/codex-fetcher-backend.test.ts index 9102e54ab09..3fefe5e3a2f 100644 --- a/src/main/rate-limits/codex-fetcher-backend.test.ts +++ b/src/main/rate-limits/codex-fetcher-backend.test.ts @@ -97,6 +97,46 @@ describe('Codex backend rate-limit requests', () => { ) }) + it('classifies a sole seven-day backend primary window as weekly', async () => { + readFileMock.mockResolvedValue( + JSON.stringify({ + tokens: { access_token: 'access-token', account_id: 'account-id' } + }) + ) + vi.mocked(fetch) + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + plan_type: 'plus', + rate_limit: { + primary_window: { + used_percent: 37, + limit_window_seconds: 7 * 24 * 60 * 60, + reset_at: 1_800_000_000 + }, + secondary_window: null + } + }) + } as Response) + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ available_count: 0, credits: [] }) + } as Response) + + await expect( + fetchCodexRateLimits({ + codexHomePath: '\\\\wsl.localhost\\Ubuntu\\home\\alice\\.local\\share\\orca\\account\\home' + }) + ).resolves.toMatchObject({ + session: null, + weekly: { usedPercent: 37, windowMinutes: 10_080, resetsAt: 1_800_000_000_000 }, + status: 'ok' + }) + + expect(childSpawnMock).not.toHaveBeenCalled() + expect(ptySpawnMock).not.toHaveBeenCalled() + }) + it('aborts callers while sharing one stalled backend auth read', async () => { let resolveRead!: (content: string) => void readFileMock.mockImplementation( diff --git a/src/main/rate-limits/codex-fetcher-session-supplement.test.ts b/src/main/rate-limits/codex-fetcher-session-supplement.test.ts new file mode 100644 index 00000000000..9bef59e96bd --- /dev/null +++ b/src/main/rate-limits/codex-fetcher-session-supplement.test.ts @@ -0,0 +1,174 @@ +import { EventEmitter } from 'node:events' +import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest' + +const { childSpawnMock, readFileMock } = vi.hoisted(() => ({ + childSpawnMock: vi.fn(), + readFileMock: vi.fn() +})) + +vi.mock('node:child_process', () => ({ spawn: childSpawnMock })) +vi.mock('node:fs/promises', () => ({ readFile: readFileMock })) +vi.mock('../codex-cli/command', () => ({ resolveCodexCommand: () => 'codex' })) +vi.mock('node-pty', () => ({ spawn: vi.fn() })) +vi.mock('./codex-auth-presence', () => ({ + probeCodexAuthPresence: vi.fn(async () => 'present') +})) + +import { fetchCodexRateLimits } from './codex-fetcher' + +function makeRpcChild(rateLimitResetCredits?: unknown) { + const child = new EventEmitter() as EventEmitter & { + stdout: EventEmitter + stderr: EventEmitter + stdin: { write: ReturnType } + kill: ReturnType + } + child.stdout = new EventEmitter() + child.stderr = new EventEmitter() + child.kill = vi.fn() + child.stdin = { + write: vi.fn((line: string) => { + const message = JSON.parse(line) as { id?: number; method?: string } + if (message.method === 'initialize') { + setTimeout(() => { + child.stdout.emit( + 'data', + Buffer.from(`${JSON.stringify({ jsonrpc: '2.0', id: message.id, result: {} })}\n`) + ) + }, 0) + } + if (message.method === 'account/rateLimits/read') { + setTimeout(() => { + child.stdout.emit( + 'data', + Buffer.from( + `${JSON.stringify({ + jsonrpc: '2.0', + id: message.id, + result: { + rateLimits: { + primary: { usedPercent: 22, windowDurationMins: 10_080 } + }, + ...(rateLimitResetCredits !== undefined ? { rateLimitResetCredits } : {}) + } + })}\n` + ) + ) + }, 0) + } + }) + } + return child +} + +function usageResponse(rateLimitResetCredits?: unknown): Response { + return { + ok: true, + json: async () => ({ + plan_type: 'pro', + rate_limit: { + primary_window: { + used_percent: 23, + limit_window_seconds: 7 * 24 * 60 * 60 + } + }, + ...(rateLimitResetCredits !== undefined + ? { rate_limit_reset_credits: rateLimitResetCredits } + : {}) + }) + } as Response +} + +function dedicatedCreditsResponse(): Response { + return { + ok: true, + json: async () => ({ + available_count: 2, + credits: [ + { + status: 'available', + expires_at: '2027-01-15T12:00:00Z', + granted_at: '2027-01-08T12:00:00Z' + } + ] + }) + } as Response +} + +async function fetchWeeklyOnly(rateLimitResetCredits?: unknown) { + childSpawnMock.mockReturnValue(makeRpcChild(rateLimitResetCredits)) + const resultPromise = fetchCodexRateLimits() + await vi.advanceTimersByTimeAsync(1) + await vi.advanceTimersByTimeAsync(1) + return resultPromise +} + +describe('Codex backend session supplement credits', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.clearAllMocks() + readFileMock.mockResolvedValue( + JSON.stringify({ + tokens: { access_token: 'access-token', account_id: 'account-id' } + }) + ) + vi.stubGlobal('fetch', vi.fn()) + }) + + afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + }) + + it('reuses complete zero-credit metadata from a weekly-only usage response', async () => { + vi.mocked(fetch).mockResolvedValue(usageResponse({ available_count: 0 })) + + await expect(fetchWeeklyOnly()).resolves.toMatchObject({ + session: null, + weekly: { usedPercent: 22, windowMinutes: 10_080 }, + rateLimitResetCredits: { availableCount: 0, nextExpiresAt: null } + }) + expect(fetch).toHaveBeenCalledTimes(1) + }) + + it.each([ + { name: 'null', credits: null }, + { name: 'invalid', credits: {} } + ])('preserves complete RPC credits when usage metadata is $name', async ({ credits }) => { + vi.mocked(fetch).mockResolvedValue(usageResponse(credits)) + + await expect( + fetchWeeklyOnly({ + availableCount: 1, + nextExpiresAt: 1_800_000_000 + }) + ).resolves.toMatchObject({ + rateLimitResetCredits: { + availableCount: 1, + nextExpiresAt: 1_800_000_000_000 + } + }) + expect(fetch).toHaveBeenCalledTimes(1) + }) + + it.each([ + { name: 'absent', credits: undefined }, + { name: 'incomplete', credits: { available_count: 2 } } + ])('uses the dedicated credits endpoint when usage metadata is $name', async ({ credits }) => { + vi.mocked(fetch) + .mockResolvedValueOnce(usageResponse(credits)) + .mockResolvedValueOnce(dedicatedCreditsResponse()) + + await expect(fetchWeeklyOnly()).resolves.toMatchObject({ + rateLimitResetCredits: { + availableCount: 2, + nextExpiresAt: Date.parse('2027-01-15T12:00:00Z') + } + }) + expect(fetch).toHaveBeenCalledTimes(2) + expect(vi.mocked(fetch).mock.calls.map(([url]) => url)).toEqual([ + 'https://chatgpt.com/backend-api/wham/usage', + 'https://chatgpt.com/backend-api/wham/rate-limit-reset-credits' + ]) + }) +}) diff --git a/src/main/rate-limits/codex-fetcher.test.ts b/src/main/rate-limits/codex-fetcher.test.ts index a00d86ffc9d..11057a1fc81 100644 --- a/src/main/rate-limits/codex-fetcher.test.ts +++ b/src/main/rate-limits/codex-fetcher.test.ts @@ -385,6 +385,53 @@ describe('fetchCodexRateLimits', () => { }) }) + it('fills a restored backend 5-hour window when RPC only reports weekly usage', async () => { + const rpcChild = makeRpcChild() + childSpawnMock.mockReturnValue(rpcChild) + respondToRpcRateLimitRead(rpcChild, { + primary: { usedPercent: 22, windowDurationMins: 10080 }, + secondary: null + }) + readFileMock.mockResolvedValue( + JSON.stringify({ + tokens: { access_token: 'access-token', account_id: 'account-id' } + }) + ) + vi.mocked(fetch).mockResolvedValue({ + ok: true, + json: async () => ({ + plan_type: 'plus', + rate_limit: { + primary_window: { + used_percent: 7, + limit_window_seconds: 5 * 60 * 60, + reset_at: 1_800_000_000 + }, + secondary_window: { + used_percent: 23, + limit_window_seconds: 7 * 24 * 60 * 60, + reset_at: 1_800_100_000 + } + }, + rate_limit_reset_credits: { + available_count: 1, + credits: [{ status: 'available', expires_at: '2027-01-15T12:00:00Z' }] + } + }) + } as Response) + + const resultPromise = fetchCodexRateLimits() + await vi.advanceTimersByTimeAsync(1) + await vi.advanceTimersByTimeAsync(1) + + await expect(resultPromise).resolves.toMatchObject({ + session: { usedPercent: 7, windowMinutes: 300, resetsAt: 1_800_000_000_000 }, + weekly: { usedPercent: 23, windowMinutes: 10080, resetsAt: 1_800_100_000_000 }, + status: 'ok' + }) + expect(fetch).toHaveBeenCalledTimes(1) + }) + it('does not map a duplicate session-duration window into the weekly slot', async () => { const rpcChild = makeRpcChild() childSpawnMock.mockReturnValue(rpcChild) diff --git a/src/main/rate-limits/codex-fetcher.ts b/src/main/rate-limits/codex-fetcher.ts index 28a97a37da6..d28fdee015e 100644 --- a/src/main/rate-limits/codex-fetcher.ts +++ b/src/main/rate-limits/codex-fetcher.ts @@ -15,8 +15,8 @@ import { classifyCodexRateLimitWindows, CODEX_SESSION_WINDOW_MINUTES, CODEX_WEEKLY_WINDOW_MINUTES, - type CodexRpcRateLimits, - type CodexRpcRateWindow + type CodexRateLimitWindowsSnapshot, + type CodexRateWindowSnapshot } from './codex-rate-limit-window-classification' import { resolveCodexCommand } from '../codex-cli/command' import { withMacTailscaleDnsHint } from '../network/macos-tailscale-dns-diagnostic' @@ -83,7 +83,7 @@ type RateLimitResetCredits = { // Why: the Codex app-server wraps rate limit data as { rateLimits: { primary, secondary, ... } }. type RpcRateLimitsResponse = { - rateLimits?: CodexRpcRateLimits | null + rateLimits?: CodexRateLimitWindowsSnapshot | null rateLimitResetCredits?: { availableCount?: number totalEarnedCount?: number @@ -292,6 +292,12 @@ function mapBackendRateLimitResetCredits( } } +function hasCompleteRateLimitResetCredits( + credits: RateLimitResetCredits | null | undefined +): boolean { + return Boolean(credits && (credits.availableCount === 0 || credits.nextExpiresAt != null)) +} + function createBackendRequestSignal( callerSignal?: AbortSignal, timeoutMs = BACKEND_TIMEOUT_MS @@ -392,8 +398,7 @@ async function withBackendRateLimitResetCredits( if ( options?.signal?.aborted || limits.provider !== 'codex' || - (limits.rateLimitResetCredits?.nextExpiresAt !== undefined && - limits.rateLimitResetCredits.nextExpiresAt !== null) + hasCompleteRateLimitResetCredits(limits.rateLimitResetCredits) ) { return limits } @@ -454,7 +459,7 @@ export async function consumeCodexRateLimitResetCredit(options: { } function mapRpcWindow( - raw: CodexRpcRateWindow | null | undefined, + raw: CodexRateWindowSnapshot | null | undefined, expectedWindowMinutes: number ): RateLimitWindow | null { if (!raw || typeof raw.usedPercent !== 'number' || !Number.isFinite(raw.usedPercent)) { @@ -489,27 +494,30 @@ function mapRpcWindow( } } -function mapBackendUsageWindow( - raw: BackendRateLimitWindow | null | undefined, - fallbackWindowMinutes: number -): RateLimitWindow | null { - const limitWindowSeconds = raw?.limit_window_seconds - // Why: match Codex backend-client's window_minutes_from_seconds — actual bucket duration, rounding partial minutes up. - const windowMinutes = +function backendWindowToSnapshot( + raw: BackendRateLimitWindow | null | undefined +): CodexRateWindowSnapshot | null { + if (!raw) { + return null + } + const limitWindowSeconds = raw.limit_window_seconds + const windowDurationMins = typeof limitWindowSeconds === 'number' && Number.isFinite(limitWindowSeconds) && limitWindowSeconds > 0 ? Math.ceil(limitWindowSeconds / 60) - : fallbackWindowMinutes - return mapRpcWindow( - raw - ? { - usedPercent: raw.used_percent, - resetsAt: raw.reset_at - } - : undefined, - windowMinutes - ) + : undefined + return { usedPercent: raw.used_percent, windowDurationMins, resetsAt: raw.reset_at } +} + +function snapshotWindowMinutes( + snapshot: CodexRateWindowSnapshot | null, + fallbackWindowMinutes: number +): number { + const duration = snapshot?.windowDurationMins + return typeof duration === 'number' && Number.isFinite(duration) && duration > 0 + ? duration + : fallbackWindowMinutes } async function fetchViaBackend( @@ -534,10 +542,20 @@ async function fetchViaBackend( if (typeof payload.plan_type !== 'string') { return null } + const classified = classifyCodexRateLimitWindows({ + primary: backendWindowToSnapshot(payload.rate_limit?.primary_window), + secondary: backendWindowToSnapshot(payload.rate_limit?.secondary_window) + }) return { provider: 'codex', - session: mapBackendUsageWindow(payload.rate_limit?.primary_window, 300), - weekly: mapBackendUsageWindow(payload.rate_limit?.secondary_window, 10080), + session: mapRpcWindow( + classified.session, + snapshotWindowMinutes(classified.session, CODEX_SESSION_WINDOW_MINUTES) + ), + weekly: mapRpcWindow( + classified.weekly, + snapshotWindowMinutes(classified.weekly, CODEX_WEEKLY_WINDOW_MINUTES) + ), // Surfaced for the status-bar Usage row (e.g. "Codex · Plus"). planType: payload.plan_type, ...(payload.rate_limit_reset_credits !== undefined @@ -552,6 +570,37 @@ async function fetchViaBackend( } } +async function withBackendSessionWindow( + limits: ProviderRateLimits, + options?: FetchCodexRateLimitsOptions +): Promise { + if (options?.signal?.aborted || limits.session || !limits.weekly) { + return limits + } + try { + const backend = await fetchViaBackend(options) + if (!backend) { + return limits + } + const rateLimitResetCredits = backend.rateLimitResetCredits ?? limits.rateLimitResetCredits + if (!backend.session) { + return rateLimitResetCredits === limits.rateLimitResetCredits + ? limits + : { ...limits, rateLimitResetCredits } + } + return { + ...limits, + session: backend.session, + weekly: backend.weekly ?? limits.weekly, + planType: backend.planType ?? limits.planType, + ...(rateLimitResetCredits !== undefined ? { rateLimitResetCredits } : {}), + updatedAt: backend.updatedAt + } + } catch { + return limits + } +} + // --------------------------------------------------------------------------- // RPC fetch — spawn `codex -s read-only -a untrusted app-server` // --------------------------------------------------------------------------- @@ -1133,7 +1182,8 @@ export async function fetchCodexRateLimits( return abortedCodexRateLimitResult() } if (rpcResult.status === 'ok' || rpcResult.status === 'unavailable') { - const withResetCredits = await withBackendRateLimitResetCredits(rpcResult, options) + const withSession = await withBackendSessionWindow(rpcResult, options) + const withResetCredits = await withBackendRateLimitResetCredits(withSession, options) return options?.signal?.aborted ? abortedCodexRateLimitResult() : withResetCredits } if (isCodexAuthError(rpcResult.error)) { @@ -1169,7 +1219,8 @@ export async function fetchCodexRateLimits( if (options?.signal?.aborted) { return abortedCodexRateLimitResult() } - const withResetCredits = await withBackendRateLimitResetCredits(ptyResult, options) + const withSession = await withBackendSessionWindow(ptyResult, options) + const withResetCredits = await withBackendRateLimitResetCredits(withSession, options) return options?.signal?.aborted ? abortedCodexRateLimitResult() : withResetCredits } catch (err) { if (options?.signal?.aborted) { diff --git a/src/main/rate-limits/codex-rate-limit-window-classification.test.ts b/src/main/rate-limits/codex-rate-limit-window-classification.test.ts index 6dff322bc38..826f1844ebe 100644 --- a/src/main/rate-limits/codex-rate-limit-window-classification.test.ts +++ b/src/main/rate-limits/codex-rate-limit-window-classification.test.ts @@ -1,10 +1,10 @@ import { describe, expect, it } from 'vitest' import { classifyCodexRateLimitWindows, - type CodexRpcRateLimits + type CodexRateLimitWindowsSnapshot } from './codex-rate-limit-window-classification' -function usedPercentByWindow(result: CodexRpcRateLimits | null): { +function usedPercentByWindow(result: CodexRateLimitWindowsSnapshot | null): { session: number | null weekly: number | null } { diff --git a/src/main/rate-limits/codex-rate-limit-window-classification.ts b/src/main/rate-limits/codex-rate-limit-window-classification.ts index 9c260cc52ae..cdf667b6306 100644 --- a/src/main/rate-limits/codex-rate-limit-window-classification.ts +++ b/src/main/rate-limits/codex-rate-limit-window-classification.ts @@ -4,27 +4,27 @@ export const CODEX_WEEKLY_WINDOW_MINUTES = 10080 // Why: tolerate the one-minute drift seen in older Codex bucket lengths without absorbing other durations. const CODEX_WINDOW_DURATION_TOLERANCE_MINUTES = 1 -export type CodexRpcRateWindow = { +export type CodexRateWindowSnapshot = { usedPercent?: unknown windowDurationMins?: unknown resetsAt?: unknown } -export type CodexRpcRateLimits = { - primary?: CodexRpcRateWindow | null - secondary?: CodexRpcRateWindow | null +export type CodexRateLimitWindowsSnapshot = { + primary?: CodexRateWindowSnapshot | null + secondary?: CodexRateWindowSnapshot | null } -type MappableCodexRpcRateWindow = CodexRpcRateWindow & { usedPercent: number } +type MappableCodexRateWindowSnapshot = CodexRateWindowSnapshot & { usedPercent: number } type CodexRateLimitWindowKind = 'session' | 'weekly' | null -function isMappableCodexRpcRateWindow( - raw: CodexRpcRateWindow | null | undefined -): raw is MappableCodexRpcRateWindow { +function isMappableCodexRateWindowSnapshot( + raw: CodexRateWindowSnapshot | null | undefined +): raw is MappableCodexRateWindowSnapshot { return typeof raw?.usedPercent === 'number' && Number.isFinite(raw.usedPercent) } -function classifyWindowDuration(raw: MappableCodexRpcRateWindow): CodexRateLimitWindowKind { +function classifyWindowDuration(raw: MappableCodexRateWindowSnapshot): CodexRateLimitWindowKind { const duration = raw.windowDurationMins if (typeof duration !== 'number' || !Number.isFinite(duration)) { return null @@ -40,14 +40,16 @@ function classifyWindowDuration(raw: MappableCodexRpcRateWindow): CodexRateLimit return null } -export function classifyCodexRateLimitWindows(result: CodexRpcRateLimits | null | undefined): { - session: MappableCodexRpcRateWindow | null - weekly: MappableCodexRpcRateWindow | null +export function classifyCodexRateLimitWindows( + result: CodexRateLimitWindowsSnapshot | null | undefined +): { + session: MappableCodexRateWindowSnapshot | null + weekly: MappableCodexRateWindowSnapshot | null } { - const primary = isMappableCodexRpcRateWindow(result?.primary) ? result.primary : null - const secondary = isMappableCodexRpcRateWindow(result?.secondary) ? result.secondary : null - let session: MappableCodexRpcRateWindow | null = null - let weekly: MappableCodexRpcRateWindow | null = null + const primary = isMappableCodexRateWindowSnapshot(result?.primary) ? result.primary : null + const secondary = isMappableCodexRateWindowSnapshot(result?.secondary) ? result.secondary : null + let session: MappableCodexRateWindowSnapshot | null = null + let weekly: MappableCodexRateWindowSnapshot | null = null for (const window of [primary, secondary]) { if (!window) { diff --git a/src/renderer/src/components/status-bar/StatusBar.tsx b/src/renderer/src/components/status-bar/StatusBar.tsx index 2c321685ff6..02978a47912 100644 --- a/src/renderer/src/components/status-bar/StatusBar.tsx +++ b/src/renderer/src/components/status-bar/StatusBar.tsx @@ -1232,10 +1232,12 @@ function VerboseProviderUsage({ return window !== null }) + const meterWindow = p.session ?? p.weekly + return ( <> - {p.session && !compact ? ( - + {meterWindow && !compact ? ( + ) : null} {visibleWindows.map((window, index) => ( diff --git a/src/renderer/src/components/status-bar/UsageRosterPanel.test.tsx b/src/renderer/src/components/status-bar/UsageRosterPanel.test.tsx index 54d1fb6cf9d..b1addca4dab 100644 --- a/src/renderer/src/components/status-bar/UsageRosterPanel.test.tsx +++ b/src/renderer/src/components/status-bar/UsageRosterPanel.test.tsx @@ -129,6 +129,10 @@ describe('UsageRow', () => { expect(mocks.useResetCountdownClock).toHaveBeenCalledOnce() expect(mocks.useResetCountdownClock).toHaveBeenCalledWith([sessionReset, weeklyReset]) expect(markup).toContain('Resets in 2m') + expect(markup).toContain('5h') + expect(markup).toContain('25%') + expect(markup).toContain('wk') + expect(markup).toContain('10%') }) }) diff --git a/src/renderer/src/components/status-bar/inline-usage-bars.test.tsx b/src/renderer/src/components/status-bar/inline-usage-bars.test.tsx index a956695c805..a6b78d6f849 100644 --- a/src/renderer/src/components/status-bar/inline-usage-bars.test.tsx +++ b/src/renderer/src/components/status-bar/inline-usage-bars.test.tsx @@ -99,6 +99,31 @@ describe('InlineUsageBars', () => { expect(markup).toContain('32% used now') }) + it('keeps the footer meter for weekly-only Codex usage', async () => { + const { ProviderSegment } = await import('./StatusBar') + const limits: ProviderRateLimits = { + provider: 'codex', + session: null, + weekly: { + usedPercent: 37, + windowMinutes: 10_080, + resetsAt: null, + resetDescription: null + }, + updatedAt: Date.now(), + error: null, + status: 'ok' + } + + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('w-[48px] h-[6px]') + expect(markup).toContain('width:37%') + expect(markup).toContain('37% used wk') + }) + it('shows remaining copy and remaining meter fill', async () => { mocks.usagePercentageDisplay = 'remaining' const { InlineUsageBars } = await import('./StatusBar') From 5c8013abaa87d6b7e21439ea68be02f635b71d8f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:56:30 -0700 Subject: [PATCH 05/21] fix(settings): reserve skill badges for attention (#11413) * fix(settings): reserve skill badges for attention * test(settings): cover hidden checking badge --- .../settings/SettingsSidebar.test.tsx | 31 ++++++++++--- .../components/settings/SettingsSidebar.tsx | 43 ++++++++----------- .../skills/SkillFreshnessStatusPill.test.tsx | 2 +- .../skills/SkillFreshnessStatusPill.tsx | 2 +- src/renderer/src/i18n/locales/en.json | 2 +- src/renderer/src/i18n/locales/es.json | 2 +- src/renderer/src/i18n/locales/ja.json | 2 +- src/renderer/src/i18n/locales/ko.json | 2 +- src/renderer/src/i18n/locales/zh.json | 2 +- 9 files changed, 50 insertions(+), 38 deletions(-) diff --git a/src/renderer/src/components/settings/SettingsSidebar.test.tsx b/src/renderer/src/components/settings/SettingsSidebar.test.tsx index 18e7668383a..75b5785be36 100644 --- a/src/renderer/src/components/settings/SettingsSidebar.test.tsx +++ b/src/renderer/src/components/settings/SettingsSidebar.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment happy-dom import { renderToStaticMarkup } from 'react-dom/server' -import { Bot, Mic, Network, Puzzle } from 'lucide-react' +import { Bot, GitBranch, Mic, Network, Puzzle } from 'lucide-react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { getDefaultSettings } from '../../../../shared/constants' import { SettingsSidebar } from './SettingsSidebar' @@ -71,6 +71,24 @@ function renderSidebar( icon: Bot, installStatus: 'up-to-date' }, + { + id: 'voice-loading', + title: 'Voice Loading', + icon: Mic, + installStatus: 'checking' + }, + { + id: 'linear', + title: 'Linear', + icon: GitBranch, + installStatus: 'update-available' + }, + { + id: 'ephemeral-vms', + title: 'Ephemeral VMs', + icon: Bot, + installStatus: 'needs-attention' + }, { id: 'plugins', title: 'Plugins', @@ -126,12 +144,15 @@ describe('SettingsSidebar', () => { expect(markup).toContain('--worktree-sidebar-foreground:#f0f4f8') }) - it('renders install state labels separately from static badges', () => { + it('reserves install state labels for actionable skill states', () => { const markup = renderSidebar() - expect(markup).toContain('Not installed') - expect(markup).toContain('Installed') - expect(markup).toContain('Up to date') + expect(markup).not.toContain('Not installed') + expect(markup).not.toContain('Installed') + expect(markup).not.toContain('Up to date') + expect(markup).not.toContain('Checking...') + expect(markup).toContain('Update available') + expect(markup).toContain('Review skill') expect(markup).toContain('Optional') }) diff --git a/src/renderer/src/components/settings/SettingsSidebar.tsx b/src/renderer/src/components/settings/SettingsSidebar.tsx index d9a0093f0b8..5c912b0512d 100644 --- a/src/renderer/src/components/settings/SettingsSidebar.tsx +++ b/src/renderer/src/components/settings/SettingsSidebar.tsx @@ -60,6 +60,17 @@ type SettingsSidebarProps = { ) => void } +type VisibleInstallStatus = Extract< + SettingsNavInstallStatus, + 'update-available' | 'needs-attention' +> + +function isVisibleInstallStatus( + status: SettingsNavInstallStatus | undefined +): status is VisibleInstallStatus { + return status === 'update-available' || status === 'needs-attention' +} + type SettingsSetupGuideRowProps = { progress: SettingsSetupGuideProgress setupActive: boolean @@ -146,17 +157,8 @@ export function SettingsSidebar({ ? 'bg-worktree-sidebar-accent font-medium text-worktree-sidebar-accent-foreground ring-1 ring-worktree-sidebar-ring/25' : 'text-worktree-sidebar-foreground/60 hover:bg-worktree-sidebar-accent/60 hover:text-worktree-sidebar-foreground' ) - const installStatusLabel = (status: SettingsNavInstallStatus): string => { + const installStatusLabel = (status: VisibleInstallStatus): string => { switch (status) { - case 'install': - return translate( - 'auto.components.settings.AgentSkillSetupPanel.5289300939', - 'Not installed' - ) - case 'installed': - return translate('auto.components.settings.AgentSkillSetupPanel.9fcebceb2a', 'Installed') - case 'up-to-date': - return translate('auto.components.skills.SkillFreshnessStatusPill.upToDate', 'Up to date') case 'update-available': return translate( 'auto.components.skills.SkillFreshnessStatusPill.updateAvailable', @@ -165,23 +167,12 @@ export function SettingsSidebar({ case 'needs-attention': return translate( 'auto.components.skills.SkillFreshnessStatusPill.needsAttention', - 'Needs attention' + 'Review skill' ) - case 'checking': - return translate('auto.components.settings.AgentSkillSetupPanel.68a468752e', 'Checking...') } } - const installStatusClassName = (status: SettingsNavInstallStatus): string => - cn( - 'ml-auto shrink-0 rounded-full border px-1.5 py-0.5 text-[10px] font-medium leading-none', - status === 'installed' || status === 'up-to-date' - ? 'border-status-success-border bg-status-success-background text-status-success' - : status === 'update-available' || status === 'needs-attention' - ? 'border-amber-500/40 bg-amber-500/10 text-amber-700 dark:text-amber-300' - : status === 'install' - ? 'border-foreground/15 bg-foreground/10 text-foreground' - : 'border-border/50 bg-muted/30 text-muted-foreground' - ) + const installStatusClassName = + 'ml-auto shrink-0 rounded-full border border-amber-500/40 bg-amber-500/10 px-1.5 py-0.5 text-[10px] font-medium leading-none text-amber-700 dark:text-amber-300' return (