From da0213fe1d5ee1910cb1a2f85897886b8a824664 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 10 Aug 2026 22:03:34 -0700 Subject: [PATCH] fix(codex): reconcile hooks for restored shells --- .../runtime-home-service.test.ts | 38 +++++++++ .../codex-accounts/runtime-home-service.ts | 35 ++++++++ .../codex/retained-codex-hook-state.test.ts | 45 ++++++++++ src/main/codex/retained-codex-hook-state.ts | 26 ++++++ src/main/index.ts | 16 +++- .../startup/desktop-startup-ordering.test.ts | 10 ++- .../restored-codex-hook-trust-report.md | 83 +++++++++++++++++++ 7 files changed, 248 insertions(+), 5 deletions(-) create mode 100644 src/main/codex/retained-codex-hook-state.test.ts create mode 100644 src/main/codex/retained-codex-hook-state.ts create mode 100644 validation-screenshots/restored-codex-hook-trust-report.md diff --git a/src/main/codex-accounts/runtime-home-service.test.ts b/src/main/codex-accounts/runtime-home-service.test.ts index 15a48821501..966d901f994 100644 --- a/src/main/codex-accounts/runtime-home-service.test.ts +++ b/src/main/codex-accounts/runtime-home-service.test.ts @@ -1206,6 +1206,44 @@ describe('CodexRuntimeHomeService', () => { } }) + it('resolves only Orca-owned homes used by live retained host shells', async () => { + const accountHome = createManagedAuth( + testState.userDataDir, + 'account-1', + createCodexAuthJson('managed@example.com', 'acct-managed', 'managed') + ) + const unownedHome = join(testState.fakeHomeDir, 'unowned-codex-home') + mkdirSync(unownedHome, { recursive: true }) + writeFileSync(join(unownedHome, '.orca-managed-home'), 'account-2\n', 'utf-8') + writePaneRegistry({ + 'shared-pane': { selectionKey: 'host', accountId: null, homeRoute: 'shared-home' }, + 'account-pane': { selectionKey: 'host', accountId: 'account-1', homeRoute: 'account-home' }, + 'unowned-pane': { selectionKey: 'host', accountId: 'account-2', homeRoute: 'account-home' }, + 'real-pane': { selectionKey: 'host', accountId: null, homeRoute: 'real-home' }, + 'wsl-pane': { selectionKey: 'wsl:Ubuntu', accountId: null, homeRoute: 'wsl-home' } + }) + const settings = createSettings({ + codexManagedAccounts: [ + createCodexAccountRecord('account-1', 'managed@example.com', 'acct-managed', accountHome), + createCodexAccountRecord('account-2', 'other@example.com', 'acct-other', unownedHome) + ] + }) + const store = createStore(settings) + const { CodexRuntimeHomeService } = await import('./runtime-home-service') + const service = new CodexRuntimeHomeService(store as never) + + expect( + service.getRetainedHostCodexHookHomePaths([ + 'shared-pane', + 'account-pane', + 'unowned-pane', + 'real-pane', + 'wsl-pane', + 'unknown-pane' + ]) + ).toEqual([getRuntimeCodexHomePath(), accountHome]) + }) + it('keeps pre-rollout shared-home panes authenticated on the real-home lane', async () => { const oldSystemAuth = createCodexAuthJson('system@example.com', 'acct-system', 'old-system') const systemAuth = createCodexAuthJson('system@example.com', 'acct-system', 'system') diff --git a/src/main/codex-accounts/runtime-home-service.ts b/src/main/codex-accounts/runtime-home-service.ts index 68d054f3571..2d4b8239aad 100644 --- a/src/main/codex-accounts/runtime-home-service.ts +++ b/src/main/codex-accounts/runtime-home-service.ts @@ -83,6 +83,7 @@ import { migrateLegacySharedAuthToPerAccountHome } from './legacy-shared-auth-mi import { CodexCredentialAbsenceGrace } from './codex-credential-absence-grace' import { syncLegacySharedCodexConfigForRetainedPanes } from './legacy-shared-config-compatibility' import { + getCodexPaneAccount, hasRecordedLegacySharedCodexPane, type CodexPaneHomeRoute } from '../codex/codex-pane-account-registry' @@ -545,6 +546,40 @@ export class CodexRuntimeHomeService { return this.isHostSystemDefaultRealHome() ? 'real-home' : 'shared-home' } + getRetainedHostCodexHookHomePaths(ptyIds: readonly string[]): string[] { + const settings = this.store.getSettings() + const homes = new Map() + for (const ptyId of ptyIds) { + const record = getCodexPaneAccount(ptyId) + if (!record || record.selectionKey !== 'host') { + continue + } + if ( + record.homeRoute === undefined || + record.homeRoute === 'shared-home' || + record.homeRoute === 'custom-home' + ) { + const homePath = this.getRuntimeHomePath() + homes.set(normalizeRuntimePathForComparison(homePath), homePath) + continue + } + if (record.homeRoute !== 'account-home' || !record.accountId) { + continue + } + const account = settings.codexManagedAccounts.find( + (candidate) => candidate.id === record.accountId + ) + if (!account || this.getWslManagedHomePath(account)) { + continue + } + const homePath = this.getTrustedSelfContainedManagedHomePath(account) + if (homePath) { + homes.set(normalizeRuntimePathForComparison(homePath), homePath) + } + } + return [...homes.values()] + } + // Why: the real-home hook installer flips this gate off when the trust-grant // client reports the host incapable, keeping that host byte-identical to the // managed lane instead of shipping status-blind panes. diff --git a/src/main/codex/retained-codex-hook-state.test.ts b/src/main/codex/retained-codex-hook-state.test.ts new file mode 100644 index 00000000000..642b8bc7476 --- /dev/null +++ b/src/main/codex/retained-codex-hook-state.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it, vi } from 'vitest' +import type { AgentHookInstallStatus } from '../../shared/agent-hook-types' +import { reconcileRetainedCodexHookHomes } from './retained-codex-hook-state' + +function status(state: 'installed' | 'not_installed' | 'error'): AgentHookInstallStatus { + return { + agent: 'codex', + state, + configPath: '/runtime/hooks.json', + managedHooksPresent: state === 'installed', + detail: state === 'error' ? 'failed' : null + } +} + +describe('retained Codex hook state', () => { + it('repairs Orca hooks before a retained shell can launch Codex', () => { + const install = vi.fn(() => status('installed')) + const refreshRuntimeUserHooks = vi.fn(() => status('not_installed')) + + reconcileRetainedCodexHookHomes({ + hookService: { install, refreshRuntimeUserHooks }, + hooksEnabled: true, + runtimeHomePaths: ['/orca/shared-home', '/orca/account-home'] + }) + + expect(install).toHaveBeenCalledTimes(2) + expect(install).toHaveBeenNthCalledWith(1, '/orca/shared-home') + expect(install).toHaveBeenNthCalledWith(2, '/orca/account-home') + expect(refreshRuntimeUserHooks).not.toHaveBeenCalled() + }) + + it('removes only Orca hooks from retained homes when hooks are disabled', () => { + const install = vi.fn(() => status('installed')) + const refreshRuntimeUserHooks = vi.fn(() => status('not_installed')) + + reconcileRetainedCodexHookHomes({ + hookService: { install, refreshRuntimeUserHooks }, + hooksEnabled: false, + runtimeHomePaths: ['/orca/shared-home'] + }) + + expect(refreshRuntimeUserHooks).toHaveBeenCalledWith('/orca/shared-home') + expect(install).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/codex/retained-codex-hook-state.ts b/src/main/codex/retained-codex-hook-state.ts new file mode 100644 index 00000000000..ccdf8c86af3 --- /dev/null +++ b/src/main/codex/retained-codex-hook-state.ts @@ -0,0 +1,26 @@ +import type { AgentHookInstallStatus } from '../../shared/agent-hook-types' + +type RetainedCodexHookService = { + install: (runtimeHomePath: string) => AgentHookInstallStatus + refreshRuntimeUserHooks: (runtimeHomePath: string) => AgentHookInstallStatus +} + +export function reconcileRetainedCodexHookHomes(args: { + hookService: RetainedCodexHookService + hooksEnabled: boolean + runtimeHomePaths: readonly string[] +}): void { + for (const runtimeHomePath of args.runtimeHomePaths) { + try { + const status = args.hooksEnabled + ? args.hookService.install(runtimeHomePath) + : args.hookService.refreshRuntimeUserHooks(runtimeHomePath) + if (status.state === 'error') { + console.warn('[codex-hook-service] failed to reconcile retained Codex home', status.detail) + } + } catch (error) { + // Why: a retained home repair is best-effort; daemon availability must not depend on a writable Codex config. + console.warn('[codex-hook-service] failed to reconcile retained Codex home', error) + } + } +} diff --git a/src/main/index.ts b/src/main/index.ts index 0a4be2562c8..0812a078c13 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -216,6 +216,7 @@ import { } from './codex-accounts/runtime-selection' import { normalizeClaudeRuntimeSelection } from './claude-accounts/runtime-selection' import { codexHookService, setSystemCodexHomeHookSweepSuppressed } from './codex/hook-service' +import { reconcileRetainedCodexHookHomes } from './codex/retained-codex-hook-state' import { ensureRealHomeCodexHookState, isRealHomeCodexHookLaneUsable @@ -917,11 +918,20 @@ function startTerminalRuntimeStartupServices(): Promise { await initDaemonPtyProvider(signal, { macosLoginSessionWatch: process.platform === 'darwin' && !isServeMode }) - if (codexRuntimeHome?.isHostSystemDefaultRealHome() && hasRecordedLegacySharedCodexPane()) { - const livePtyIds = await listLiveDaemonPtyIds() - if (livePtyIds) { + // Why: a retained shell keeps its launch-time Codex home even when the current routing lane changes. + const livePtyIds = await listLiveDaemonPtyIds() + if (livePtyIds && codexRuntimeHome) { + if (codexRuntimeHome.isHostSystemDefaultRealHome() && hasRecordedLegacySharedCodexPane()) { reconcileCodexPaneAccountsWithLivePtys(livePtyIds) } + const settings = store?.getSettings() + reconcileRetainedCodexHookHomes({ + hookService: codexHookService, + hooksEnabled: + isAgentStatusHooksEnabled(settings) && + settings?.disabledTuiAgents.includes('codex') !== true, + runtimeHomePaths: codexRuntimeHome.getRetainedHostCodexHookHomePaths(livePtyIds) + }) } // Why: retained shells can invoke Codex immediately after the startup gate. codexRuntimeHome?.reconcileLegacySharedHomeForRetainedPanes() diff --git a/src/main/startup/desktop-startup-ordering.test.ts b/src/main/startup/desktop-startup-ordering.test.ts index 001bc5252d1..42eb31617bc 100644 --- a/src/main/startup/desktop-startup-ordering.test.ts +++ b/src/main/startup/desktop-startup-ordering.test.ts @@ -105,12 +105,18 @@ describe('startup ordering', () => { const inventoryIndex = source.indexOf('await listLiveDaemonPtyIds()', daemonInitIndex) const reconciliation = 'codexRuntimeHome?.reconcileLegacySharedHomeForRetainedPanes()' const reconciliationIndex = source.indexOf(reconciliation, inventoryIndex) + const hookReconciliationIndex = source.indexOf( + 'reconcileRetainedCodexHookHomes({', + inventoryIndex + ) const serveIndex = source.indexOf('if (serveOptions) {', reconciliationIndex) const desktopIndex = source.indexOf('Promise.resolve(openMainWindow())', serveIndex) expect(daemonInitIndex).toBeGreaterThanOrEqual(0) - expect(routeGateIndex).toBeGreaterThan(daemonInitIndex) - expect(inventoryIndex).toBeGreaterThan(routeGateIndex) + expect(inventoryIndex).toBeGreaterThan(daemonInitIndex) + expect(routeGateIndex).toBeGreaterThan(inventoryIndex) + expect(hookReconciliationIndex).toBeGreaterThan(inventoryIndex) + expect(hookReconciliationIndex).toBeLessThan(reconciliationIndex) expect(reconciliationIndex).toBeGreaterThan(inventoryIndex) expect(serveIndex).toBeGreaterThan(reconciliationIndex) expect(desktopIndex).toBeGreaterThan(serveIndex) diff --git a/validation-screenshots/restored-codex-hook-trust-report.md b/validation-screenshots/restored-codex-hook-trust-report.md new file mode 100644 index 00000000000..b4a1621e78b --- /dev/null +++ b/validation-screenshots/restored-codex-hook-trust-report.md @@ -0,0 +1,83 @@ +# Restored-shell Codex hook-trust validation + +## Root cause + +Restored daemon shells became interactive after the daemon and hook server started, while managed Codex hook reconciliation remained an unawaited background task. A retained shell could therefore launch manually typed `codex` against its launch-time managed home before Orca refreshed that home's hook definitions and trusted hashes. The background repair then made persisted state and later `hooks/list` checks look healthy. + +The retained shell's launch-time home is independent of the currently selected Codex routing lane. Startup now inventories authoritative live daemon PTYs on every retained-shell startup, resolves only registered Orca-managed host homes, and reconciles those homes before the startup gate completes. + +## Security boundary + +- Shared and legacy managed homes resolve only to Orca's private runtime home. +- Per-account homes pass existing Orca ownership validation. +- Real-home, WSL, SSH/non-host, unknown, missing-account, and unowned paths are skipped. +- Enabled settings reinstall/repair only Orca-managed hooks. +- Disabled settings remove only Orca-managed entries while preserving user hooks. + +## Electron/CDP reproduction + +Build: + +```bash +pnpm run build:electron-vite +``` + +Isolated launch (the shortened profile path avoids the Unix-socket path limit): + +```bash +/usr/bin/env -u ELECTRON_RUN_AS_NODE -u CODEX_HOME -u ORCA_CODEX_HOME \ + PATH='/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin' \ + HOME='/tmp/orca-rce-060679/home' \ + USERPROFILE='/tmp/orca-rce-060679/home' \ + ORCA_E2E_USER_DATA_DIR='/tmp/orca-rce-060679' \ + ORCA_E2E_HOME_DIR='/tmp/orca-rce-060679/home' \ + ORCA_E2E_HEADFUL=1 NODE_ENV=development \ + '/node_modules/.pnpm/electron@43.1.0/node_modules/electron/dist/Electron.app/Contents/MacOS/Electron' \ + out/main/index.js --remote-debugging-port=9460 +``` + +Validation used `agent-browser --cdp 9460` only. A real floating-shell PTY was created, the Electron main process was terminated without stopping the daemon, its pane registry retained the managed shared-home route, and an Orca lifecycle-hook `trusted_hash` was made stale. A deterministic Codex fixture was added only after startup so it reported the same review/ready state from the managed config without allowing generic startup CLI detection to hide the race. + +Local capture filenames (uploaded to the PR as GitHub attachments; intentionally not committed): + +- Before: `restored-codex-hook-review-before.png` +- After: `restored-codex-hook-trusted-after.png` +- Fresh pane: `fresh-codex-hook-trusted.png` +- Disabled hooks after retained restart: `restored-codex-hooks-disabled.png` + +The pre-fix restart retained the stale hash and showed `Hooks need review`. The fixed restart repaired it before `awaitFirstWindowStartupServices()` resolved and the same retained PTY showed trusted/ready. A fresh pane reached normal Codex onboarding without a hook-review prompt. With `agentStatusHooksEnabled: false`, a retained restart left `hooks.json` as `{ "hooks": {} }` and `config.toml` with an empty `[hooks.state]`. + +## Automated validation + +```bash +pnpm exec vitest run \ + src/main/codex/retained-codex-hook-state.test.ts \ + src/main/startup/desktop-startup-ordering.test.ts \ + src/main/codex-accounts/runtime-home-service.test.ts + +pnpm run typecheck:node + +pnpm exec oxfmt --check \ + src/main/index.ts \ + src/main/codex-accounts/runtime-home-service.ts \ + src/main/codex-accounts/runtime-home-service.test.ts \ + src/main/codex/retained-codex-hook-state.ts \ + src/main/codex/retained-codex-hook-state.test.ts \ + src/main/startup/desktop-startup-ordering.test.ts + +pnpm exec oxlint \ + src/main/index.ts \ + src/main/codex-accounts/runtime-home-service.ts \ + src/main/codex-accounts/runtime-home-service.test.ts \ + src/main/codex/retained-codex-hook-state.ts \ + src/main/codex/retained-codex-hook-state.test.ts \ + src/main/startup/desktop-startup-ordering.test.ts + +git diff --check +``` + +Results: 113 focused tests passed; node typecheck, formatting, direct oxlint, diff check, and Electron build passed. The aggregate changed-code script was not used because Node 26's unsupported-engine warning breaks its JSON parser; direct oxlint covered all changed source files. + +## Remaining risk + +The repair deliberately covers only host-side Orca-managed runtime homes recorded for live PTYs. Folder workspaces use the same verified host-home path; real-home, WSL, SSH, and arbitrary user or repository hook paths are not auto-trusted, and their existing launch-specific flows remain unchanged.