diff --git a/config/build-plugins/plain-node-entry-guard.ts b/config/build-plugins/plain-node-entry-guard.ts index 441536e7edb..c87e2548a53 100644 --- a/config/build-plugins/plain-node-entry-guard.ts +++ b/config/build-plugins/plain-node-entry-guard.ts @@ -24,8 +24,7 @@ const PLAIN_NODE_ENTRY_NAMES = [ 'parcel-watcher-process-entry', 'computer-sidecar', 'wsl-transcript-fs-process-entry', - 'agent-hooks/managed-agent-hook-controls', - 'codex/codex-app-server-grant-entry' + 'agent-hooks/managed-agent-hook-controls' ] as const // Entries executed as worker threads of the main process. Electron's module is diff --git a/config/knip.json b/config/knip.json index 565fb68bc0a..9af0b8d1a74 100644 --- a/config/knip.json +++ b/config/knip.json @@ -14,7 +14,6 @@ "src/main/ports/port-scan-command-worker-entry.ts", "src/main/ipc/parcel-watcher-process-entry.ts", "src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts", - "src/main/codex/codex-app-server-grant-entry.ts", "src/main/agent-hooks/managed-agent-hook-controls.ts", "src/main/claude-accounts/keychain.ts", "src/renderer/src/main.tsx", diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index e5cf60dd5a7..eb278257159 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -30,8 +30,6 @@ "../src/main/codex/codex-app-server-capability-cache.ts", "../src/main/codex/codex-app-server-capability-signal.ts", "../src/main/codex/codex-app-server-client.ts", - "../src/main/codex/codex-app-server-grant-bridge.ts", - "../src/main/codex/codex-app-server-grant-envelope.ts", "../src/main/codex/codex-app-server-session.ts", "../src/main/codex/codex-config-mirror.ts", "../src/main/codex/codex-config-path-reference-rewrite.ts", @@ -40,6 +38,7 @@ "../src/main/codex/codex-config-settings-upsert.ts", "../src/main/codex/codex-home-paths.ts", "../src/main/codex/codex-managed-home-resource-copy-marker.ts", + "../src/main/codex/codex-managed-trust-grant-plan.ts", "../src/main/codex/codex-path-observation.ts", "../src/main/codex/codex-hook-identity.ts", "../src/main/codex/codex-hook-trust-grant.ts", @@ -48,6 +47,7 @@ "../src/main/codex/codex-state-db.ts", "../src/main/codex/codex-trust-identity.ts", "../src/main/codex/codex-trust-config-rollback.ts", + "../src/main/codex/codex-trust-config-mutation-queue.ts", "../src/main/codex/codex-trust-grant-telemetry.ts", "../src/main/codex/codex-trust-grant-host.ts", "../src/main/codex/codex-trust-grant-ledger.ts", diff --git a/electron.vite.config.ts b/electron.vite.config.ts index 982677811c5..f81dbb93041 100644 --- a/electron.vite.config.ts +++ b/electron.vite.config.ts @@ -239,12 +239,6 @@ export const electronViteConfig: UserConfig = { 'main-thread-hang-watchdog-entry': resolve( 'src/main/hang-watchdog/main-thread-hang-watchdog-entry.ts' ), - // Why: run under ELECTRON_RUN_AS_NODE while the caller blocks on - // spawnSync — codex app-server trust grants need a live event loop - // but must finish before a Codex pane launch proceeds. - 'codex/codex-app-server-grant-entry': resolve( - 'src/main/codex/codex-app-server-grant-entry.ts' - ), // Why: electron-vite cleans out/main in dev. The dev CLI imports // this path for `orca agent hooks ...`, so it must survive rebuilds. 'agent-hooks/managed-agent-hook-controls': resolve( diff --git a/src/cli/handlers/agent-hooks.ts b/src/cli/handlers/agent-hooks.ts index 7e21c4973a2..57419518d4b 100644 --- a/src/cli/handlers/agent-hooks.ts +++ b/src/cli/handlers/agent-hooks.ts @@ -208,7 +208,7 @@ async function setAgentHooksEnabled( export const AGENT_HOOK_HANDLERS: Record = { 'agent hooks prepare-codex': async ({ client }) => { const settings = await readHookSettings(client) - prepareManagedCodexHomeBeforeShellLaunch({ + await prepareManagedCodexHomeBeforeShellLaunch({ userDataPath: getDefaultUserDataPath(), hooksEnabled: settings.agentStatusHooksEnabled && !settings.disabledTuiAgents.includes('codex') diff --git a/src/main/agent-hooks/managed-agent-hook-controls.ts b/src/main/agent-hooks/managed-agent-hook-controls.ts index 043cc1bb1ea..64d426ae072 100644 --- a/src/main/agent-hooks/managed-agent-hook-controls.ts +++ b/src/main/agent-hooks/managed-agent-hook-controls.ts @@ -86,14 +86,14 @@ function selectedInstallers(options: InstallOptions): readonly ManagedAgentHookI return MANAGED_AGENT_HOOK_INSTALLERS.filter(([agent]) => allowed.has(agent)) } -function runInstaller( +async function runInstaller( entry: ManagedAgentHookInstaller, onInstallError: InstallOptions['onInstallError'], userInitiated?: boolean -): AgentHookInstallStatus { +): Promise { const [agent, install] = entry try { - return install({ userInitiated }) + return await install({ userInitiated }) } catch (error) { console.error(`[agent-hooks] Failed to install ${agent} managed hooks:`, error) try { @@ -177,22 +177,27 @@ export async function installManagedAgentHooks( ) continue } - results.push(runInstaller(entry, options.onInstallError, options.userInitiated)) + results.push(await runInstaller(entry, options.onInstallError, options.userInitiated)) } return results } -export function removeManagedAgentHooks(options: RemoveOptions = {}): AgentHookInstallStatus[] { +export async function removeManagedAgentHooks( + options: RemoveOptions = {} +): Promise { const allowed = options.agents ? new Set(options.agents) : null - return MANAGED_AGENT_HOOK_REMOVERS.filter( - ([agent]) => allowed === null || allowed.has(agent) - ).map(([agent, remove]) => { - try { - return remove() - } catch (error) { - return errorStatus(agent, error) + const results: AgentHookInstallStatus[] = [] + for (const [agent, remove] of MANAGED_AGENT_HOOK_REMOVERS) { + if (allowed !== null && !allowed.has(agent)) { + continue } - }) + try { + results.push(await remove()) + } catch (error) { + results.push(errorStatus(agent, error)) + } + } + return results } export async function removeManagedAgentHooksAsync( @@ -228,7 +233,7 @@ export async function applyAgentStatusHooksEnabled( options: InstallOptions = {} ): Promise { if (!enabled) { - return removeManagedAgentHooks() + return await removeManagedAgentHooks() } const disabled = normalizeDisabledTuiAgents(settings?.disabledTuiAgents).filter( isManagedAgentHookTarget @@ -241,7 +246,10 @@ export async function applyAgentStatusHooksEnabled( return installed } const removed = new Map( - removeManagedAgentHooks({ agents: disabledToRemove }).map((status) => [status.agent, status]) + (await removeManagedAgentHooks({ agents: disabledToRemove })).map((status) => [ + status.agent, + status + ]) ) return installed.map((status) => removed.get(status.agent) ?? status) } diff --git a/src/main/agent-hooks/managed-agent-hook-registry.ts b/src/main/agent-hooks/managed-agent-hook-registry.ts index c94d367bd79..5c462f1794a 100644 --- a/src/main/agent-hooks/managed-agent-hook-registry.ts +++ b/src/main/agent-hooks/managed-agent-hook-registry.ts @@ -15,13 +15,21 @@ import { hermesHookService } from '../hermes/hook-service' import { kimiHookService } from '../kimi/hook-service' import { openClaudeHookService } from '../openclaude/hook-service' +// Why (#16441): Codex's installer awaits a codex app-server trust-grant session +// instead of blocking the main thread on spawnSync. Widening the tuple keeps the +// other thirteen agent services synchronous — the shared loop already awaits. export type ManagedAgentHookInstallOptions = { userInitiated?: boolean } export type ManagedAgentHookInstaller = readonly [ HookInstallAgent, - (options?: ManagedAgentHookInstallOptions) => AgentHookInstallStatus + ( + options?: ManagedAgentHookInstallOptions + ) => AgentHookInstallStatus | Promise ] export type ManagedAgentHookScriptRefresher = readonly [HookInstallAgent, () => Promise] -export type ManagedAgentHookRemover = readonly [HookInstallAgent, () => AgentHookInstallStatus] +export type ManagedAgentHookRemover = readonly [ + HookInstallAgent, + () => AgentHookInstallStatus | Promise +] export type ManagedAgentHookAsyncRemover = readonly [ HookInstallAgent, () => Promise diff --git a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts index c52d6656013..40237141d24 100644 --- a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts +++ b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts @@ -209,11 +209,14 @@ async function generatePosixScripts(): Promise> { return scripts } -function withPlatform(platform: NodeJS.Platform, run: () => T): T { +// Why: the Codex installer awaits an app-server trust-grant session, so the +// override has to stay pinned across the await instead of being restored by a +// synchronous `finally` while the install is still running. +async function withPlatform(platform: NodeJS.Platform, run: () => T | Promise): Promise { const original = Object.getOwnPropertyDescriptor(process, 'platform') Object.defineProperty(process, 'platform', { configurable: true, value: platform }) try { - return run() + return await run() } finally { if (original) { Object.defineProperty(process, 'platform', original) @@ -222,7 +225,7 @@ function withPlatform(platform: NodeJS.Platform, run: () => T): T { } describe('Windows managed hook stdin structure', () => { - it('exits immediately when Orca env is missing and keeps drain for other failures', () => { + it('exits immediately when Orca env is missing and keeps drain for other failures', async () => { const home = mkdtempSync(join(tmpdir(), 'orca-hook-stdin-windows-')) homedirMock.mockReturnValue(home) const previousGrokHome = process.env.GROK_HOME @@ -230,9 +233,9 @@ describe('Windows managed hook stdin structure', () => { delete process.env.GROK_HOME delete process.env.KIMI_CODE_HOME try { - withPlatform('win32', () => { + await withPlatform('win32', async () => { for (const entry of LOCAL_INSTALLERS) { - expect(entry.install().state, `${entry.agent} install status`).toBe('installed') + expect((await entry.install()).state, `${entry.agent} install status`).toBe('installed') } }) const hooksDir = join(home, '.orca', 'agent-hooks') @@ -317,7 +320,7 @@ describe('Windows managed hook stdin structure', () => { try { const gitBash = findGitBash() for (const entry of LOCAL_INSTALLERS) { - expect(entry.install().state, `${entry.agent} install status`).toBe('installed') + expect((await entry.install()).state, `${entry.agent} install status`).toBe('installed') } const hooksDir = join(home, '.orca', 'agent-hooks') const mainScripts = readdirSync(hooksDir).filter( diff --git a/src/main/agent-hooks/windows-hook-post-interpreter.test.ts b/src/main/agent-hooks/windows-hook-post-interpreter.test.ts index 0e79c125c2e..09377f3adf5 100644 --- a/src/main/agent-hooks/windows-hook-post-interpreter.test.ts +++ b/src/main/agent-hooks/windows-hook-post-interpreter.test.ts @@ -56,11 +56,14 @@ const BATCH_SCRIPT_INSTALLERS = [ { agent: 'grok', install: () => new GrokHookService().install() } ] as const -function withPlatform(platform: NodeJS.Platform, run: () => T): T { +// Why: the Codex installer awaits an app-server trust-grant session, so the +// override has to stay pinned across the await instead of being restored by a +// synchronous `finally` while the install is still running. +async function withPlatform(platform: NodeJS.Platform, run: () => T | Promise): Promise { const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') Object.defineProperty(process, 'platform', { configurable: true, value: platform }) try { - return run() + return await run() } finally { if (originalPlatform) { Object.defineProperty(process, 'platform', originalPlatform) @@ -93,10 +96,10 @@ describe('Windows managed hook post interpreter', () => { home = '' }) - it('posts through curl.exe from every managed batch script, spawning no interpreter', () => { - const scripts = withPlatform('win32', () => { + it('posts through curl.exe from every managed batch script, spawning no interpreter', async () => { + const scripts = await withPlatform('win32', async () => { for (const entry of BATCH_SCRIPT_INSTALLERS) { - expect(entry.install().state, `${entry.agent} install status`).toBe('installed') + expect((await entry.install()).state, `${entry.agent} install status`).toBe('installed') } const hooksDir = join(home, '.orca', 'agent-hooks') return readdirSync(hooksDir) diff --git a/src/main/agent-trust-presets.test.ts b/src/main/agent-trust-presets.test.ts index 2a979c60e27..f5377be86c4 100644 --- a/src/main/agent-trust-presets.test.ts +++ b/src/main/agent-trust-presets.test.ts @@ -40,6 +40,8 @@ vi.mock('node:os', async () => { const { markCodexProjectTrusted, markCopilotFolderTrusted, markCursorWorkspaceTrusted } = await import('./agent-trust-presets') +const { runExclusivelyForCodexTrustConfig } = + await import('./codex/codex-trust-config-mutation-queue') beforeEach(() => { testState.fakeHomeDir = mkdtempSync(join(tmpdir(), 'orca-trust-presets-')) @@ -137,7 +139,32 @@ describe('markCopilotFolderTrusted', () => { }) describe('markCodexProjectTrusted', () => { - it('trusts the main repository root for a linked worktree without reading commondir', () => { + // Why (#16441): a hook install/grant holds this file across an awaited + // app-server session; an unqueued write here lands inside its + // capture->restore window and is silently reverted. + it('queues behind an in-flight Codex trust-config mutation', async () => { + const workspace = mkdtempSync(join(tmpdir(), 'orca-codex-ws-')) + const configPath = join(testState.fakeHomeDir, '.codex', 'config.toml') + let releaseGrant!: () => void + const grantHoldingTheFile = new Promise((resolve) => { + releaseGrant = resolve + }) + try { + const held = runExclusivelyForCodexTrustConfig(configPath, () => grantHoldingTheFile) + const marked = markCodexProjectTrusted(workspace) + await Promise.resolve() + expect(existsSync(configPath)).toBe(false) + + releaseGrant() + await held + await marked + expect(readFileSync(configPath, 'utf-8')).toContain('trust_level = "trusted"') + } finally { + rmSync(workspace, { recursive: true, force: true }) + } + }) + + it('trusts the main repository root for a linked worktree without reading commondir', async () => { const fixtureRoot = mkdtempSync(join(tmpdir(), 'orca-codex-linked-ws-')) const repository = join(fixtureRoot, 'repo') const workspace = join(fixtureRoot, 'worktrees', 'feature') @@ -148,7 +175,7 @@ describe('markCodexProjectTrusted', () => { writeFileSync(join(workspace, '.git'), `gitdir: ${worktreeGitDir}\n`, 'utf-8') writeFileSync(join(worktreeGitDir, 'gitdir'), join(workspace, '.git'), 'utf-8') - markCodexProjectTrusted(workspace) + await markCodexProjectTrusted(workspace) const repositoryRoot = realpathSync.native(repository) const workspaceRoot = realpathSync.native(workspace) @@ -171,7 +198,7 @@ describe('markCodexProjectTrusted', () => { } }) - it('does not broaden trust through arbitrary or adversarial Git metadata', () => { + it('does not broaden trust through arbitrary or adversarial Git metadata', async () => { const fixtureRoot = mkdtempSync(join(tmpdir(), 'orca-codex-untrusted-gitdir-')) const workspace = join(fixtureRoot, 'workspace') const arbitraryGitDir = join(fixtureRoot, 'metadata', 'feature') @@ -183,12 +210,12 @@ describe('markCodexProjectTrusted', () => { writeFileSync(join(workspace, '.git'), `gitdir: ${arbitraryGitDir}\n`, 'utf-8') writeFileSync(join(arbitraryGitDir, 'commondir'), join(unrelatedRoot, '.git'), 'utf-8') - markCodexProjectTrusted(workspace) + await markCodexProjectTrusted(workspace) const structuredGitDir = join(unrelatedRoot, '.git', 'worktrees', 'feature') mkdirSync(structuredGitDir, { recursive: true }) writeFileSync(join(workspace, '.git'), `gitdir: ${structuredGitDir}\n`, 'utf-8') writeFileSync(join(structuredGitDir, 'gitdir'), join(unrelatedRoot, '.git'), 'utf-8') - markCodexProjectTrusted(workspace) + await markCodexProjectTrusted(workspace) const written = readFileSync(join(testState.fakeHomeDir, '.codex', 'config.toml'), 'utf-8') expect(written).toContain( @@ -202,11 +229,11 @@ describe('markCodexProjectTrusted', () => { } }) - it('writes ~/.codex/config.toml with the project marked trusted', () => { + it('writes ~/.codex/config.toml with the project marked trusted', async () => { const workspace = mkdtempSync(join(tmpdir(), 'orca-codex-ws-')) try { const realpath = realpathSync.native(workspace) - markCodexProjectTrusted(workspace) + await markCodexProjectTrusted(workspace) const configPath = join(testState.fakeHomeDir, '.codex', 'config.toml') const runtimeConfigPath = join( testState.userDataDir, @@ -227,7 +254,7 @@ describe('markCodexProjectTrusted', () => { } }) - it('preserves existing config keys and updates an existing project block', () => { + it('preserves existing config keys and updates an existing project block', async () => { const workspace = mkdtempSync(join(tmpdir(), 'orca-codex-ws-')) const realpath = realpathSync.native(workspace) try { @@ -260,7 +287,7 @@ describe('markCodexProjectTrusted', () => { 'utf-8' ) - markCodexProjectTrusted(workspace) + await markCodexProjectTrusted(workspace) const written = readFileSync(join(codexDir, 'config.toml'), 'utf-8') const runtimeWritten = readFileSync(join(runtimeCodexDir, 'config.toml'), 'utf-8') diff --git a/src/main/agent-trust-presets.ts b/src/main/agent-trust-presets.ts index 3b441d6d1fe..16c728eba55 100644 --- a/src/main/agent-trust-presets.ts +++ b/src/main/agent-trust-presets.ts @@ -4,6 +4,7 @@ import { basename, dirname, join, resolve } from 'node:path' import { writeFileAtomically } from './codex-accounts/fs-utils' import { getOrcaManagedCodexHomePath } from './codex/codex-home-paths' import { upsertProjectTrustLevel } from './codex/config-toml-trust' +import { runExclusivelyForCodexTrustConfig } from './codex/codex-trust-config-mutation-queue' export type AgentTrustPreset = 'cursor' | 'copilot' | 'codex' @@ -108,13 +109,21 @@ export function markCopilotFolderTrusted(workspacePath: string): void { * Verified against codex-rs/tui/src/onboarding/trust_directory.rs and * codex-rs/core/src/config/config_tests.rs in the Codex CLI source. */ -export function markCodexProjectTrusted(workspacePath: string): void { +export function markCodexProjectTrusted(workspacePath: string): Promise { const absPath = resolveCodexProjectTrustRoot(workspacePath) - const configPath = join(homedir(), '.codex', 'config.toml') - upsertProjectTrustLevel(configPath, absPath, 'trusted') + const systemTomlPath = join(homedir(), '.codex', 'config.toml') // Why: Orca-launched Codex runs with an Orca-owned CODEX_HOME, so the trust // preset must also update the runtime config Codex will actually read. - upsertProjectTrustLevel(join(getOrcaManagedCodexHomePath(), 'config.toml'), absPath, 'trusted') + const runtimeTomlPath = join(getOrcaManagedCodexHomePath(), 'config.toml') + // Why (#16441): hook installs now await a codex app-server grant, so an + // unqueued write here can land inside their capture->restore window and be + // reverted. Same runtime-before-system lock order the installer takes. + return runExclusivelyForCodexTrustConfig(runtimeTomlPath, () => + runExclusivelyForCodexTrustConfig(systemTomlPath, async () => { + upsertProjectTrustLevel(systemTomlPath, absPath, 'trusted') + upsertProjectTrustLevel(runtimeTomlPath, absPath, 'trusted') + }) + ) } function resolveCodexProjectTrustRoot(workspacePath: string): string { diff --git a/src/main/codex-accounts/runtime-home-service-per-account-migration.test.ts b/src/main/codex-accounts/runtime-home-service-per-account-migration.test.ts index 20b2ef75839..53736a11beb 100644 --- a/src/main/codex-accounts/runtime-home-service-per-account-migration.test.ts +++ b/src/main/codex-accounts/runtime-home-service-per-account-migration.test.ts @@ -95,7 +95,7 @@ describe('CodexRuntimeHomeService per-account takeover composition', () => { const config = readFileSync(join(account.managedHomePath, 'config.toml'), 'utf8') expect(config).toContain('model = "fixture-model"') expect(config).not.toContain('[hooks.state') - expect(hookService.install(account.managedHomePath).state).toBe('installed') + expect((await hookService.install(account.managedHomePath)).state).toBe('installed') expect(readFileSync(join(account.managedHomePath, 'hooks.json'), 'utf8')).toContain( process.platform === 'win32' ? 'codex-hook.cmd' : 'codex-hook.sh' ) diff --git a/src/main/codex/codex-app-server-capability-cache.test.ts b/src/main/codex/codex-app-server-capability-cache.test.ts index c6dea6e0031..676c3f8191b 100644 --- a/src/main/codex/codex-app-server-capability-cache.test.ts +++ b/src/main/codex/codex-app-server-capability-cache.test.ts @@ -21,30 +21,40 @@ describe('CodexAppServerCapabilityCache', () => { ) }) - it('falls back on the first unsupported probe and skips the probe on later calls', () => { + it('falls back on the first unsupported probe and skips the probe on later calls', async () => { const cache = new CodexAppServerCapabilityCache() - const firstPreferred = vi.fn(() => { - throw unsupportedError - }) - expect( - cache.runWithFallbackSync('native', firstPreferred, () => 'first-fallback', isUnsupported, 5) - ).toBe('first-fallback') + const firstPreferred = vi.fn(() => Promise.reject(unsupportedError)) + await expect( + cache.runWithFallback( + 'native', + firstPreferred, + () => Promise.resolve('first-fallback'), + isUnsupported + ) + ).resolves.toBe('first-fallback') expect(firstPreferred).toHaveBeenCalledTimes(1) - // Why: probes are synchronous on the main thread, so they can never - // overlap — back-to-back calls inside the retry window are the - // "concurrent probe" equivalent and must share the first probe's result. - const laterPreferred = vi.fn(() => 'unexpected-preferred') - expect( - cache.runWithFallbackSync('native', laterPreferred, () => 'cached-fallback', isUnsupported, 6) - ).toBe('cached-fallback') - expect( - cache.runWithFallbackSync('native', laterPreferred, () => 'cached-fallback', isUnsupported, 7) - ).toBe('cached-fallback') + const laterPreferred = vi.fn(() => Promise.resolve('unexpected-preferred')) + await expect( + cache.runWithFallback( + 'native', + laterPreferred, + () => Promise.resolve('cached-fallback'), + isUnsupported + ) + ).resolves.toBe('cached-fallback') + await expect( + cache.runWithFallback( + 'native', + laterPreferred, + () => Promise.resolve('cached-fallback'), + isUnsupported + ) + ).resolves.toBe('cached-fallback') expect(laterPreferred).not.toHaveBeenCalled() }) - it('isolates capability state per execution host', () => { + it('isolates capability state per execution host', async () => { const cache = new CodexAppServerCapabilityCache() cache.rememberUnsupported('wsl:Ubuntu', 1_000) @@ -52,63 +62,148 @@ describe('CodexAppServerCapabilityCache', () => { expect(cache.shouldTry('native', 1_001)).toBe(true) expect(cache.shouldTry('wsl:Debian', 1_001)).toBe(true) - const nativePreferred = vi.fn(() => 'native-result') - expect( - cache.runWithFallbackSync('native', nativePreferred, () => 'unexpected', isUnsupported, 1_001) - ).toBe('native-result') + const nativePreferred = vi.fn(() => Promise.resolve('native-result')) + await expect( + cache.runWithFallback( + 'native', + nativePreferred, + () => Promise.resolve('unexpected'), + isUnsupported + ) + ).resolves.toBe('native-result') expect(nativePreferred).toHaveBeenCalledTimes(1) }) - it('drops known support when a later call reports the capability unsupported', () => { + it('drops known support when a later call reports the capability unsupported', async () => { const cache = new CodexAppServerCapabilityCache() - expect( - cache.runWithFallbackSync( + await expect( + cache.runWithFallback( 'native', - () => 'supported', - () => 'unexpected', - isUnsupported, - 1 + () => Promise.resolve('supported'), + () => Promise.resolve('unexpected'), + isUnsupported ) - ).toBe('supported') + ).resolves.toBe('supported') expect(cache.isKnownSupported('native')).toBe(true) - expect( - cache.runWithFallbackSync( + await expect( + cache.runWithFallback( 'native', - () => { - throw unsupportedError - }, - () => 'fallback', - isUnsupported, - 2 + () => Promise.reject(unsupportedError), + () => Promise.resolve('fallback'), + isUnsupported ) - ).toBe('fallback') + ).resolves.toBe('fallback') expect(cache.isKnownSupported('native')).toBe(false) - const laterPreferred = vi.fn(() => 'unexpected-preferred') - expect( - cache.runWithFallbackSync('native', laterPreferred, () => 'cached-fallback', isUnsupported, 3) - ).toBe('cached-fallback') + const laterPreferred = vi.fn(() => Promise.resolve('unexpected-preferred')) + await expect( + cache.runWithFallback( + 'native', + laterPreferred, + () => Promise.resolve('cached-fallback'), + isUnsupported + ) + ).resolves.toBe('cached-fallback') expect(laterPreferred).not.toHaveBeenCalled() }) - it('rethrows transient errors without marking the host unsupported', () => { + it('rethrows transient errors without marking the host unsupported', async () => { const cache = new CodexAppServerCapabilityCache() const transient = new Error('spawn ETIMEDOUT') - expect(() => - cache.runWithFallbackSync( + await expect( + cache.runWithFallback( 'native', - () => { - throw transient - }, - () => 'unexpected-fallback', - isUnsupported, - 1 + () => Promise.reject(transient), + () => Promise.resolve('unexpected-fallback'), + isUnsupported ) - ).toThrow(transient) + ).rejects.toBe(transient) expect(cache.shouldTry('native', 2)).toBe(true) }) + // Why (#16441): grants no longer block the main thread, so two pane launches + // can reach a cold host at once. Without dedupe each one pays its own + // app-server session against a codex that has no such RPC surface. + it('dedupes concurrent probes on one host to a single app-server session', async () => { + const cache = new CodexAppServerCapabilityCache() + let releaseProbe!: (error: unknown) => void + const preferred = vi.fn( + () => + new Promise((_resolve, reject) => { + releaseProbe = reject + }) + ) + const first = cache.runWithFallback( + 'native', + preferred, + () => Promise.resolve('fallback'), + isUnsupported + ) + const second = cache.runWithFallback( + 'native', + preferred, + () => Promise.resolve('fallback'), + isUnsupported + ) + await Promise.resolve() + releaseProbe(unsupportedError) + + await expect(first).resolves.toBe('fallback') + await expect(second).resolves.toBe('fallback') + expect(preferred).toHaveBeenCalledTimes(1) + }) + + it('lets a waiter run its own work once the in-flight probe reports support', async () => { + const cache = new CodexAppServerCapabilityCache() + let releaseProbe!: (value: string) => void + const firstPreferred = vi.fn( + () => + new Promise((resolve) => { + releaseProbe = resolve + }) + ) + const secondPreferred = vi.fn(() => Promise.resolve('second')) + const first = cache.runWithFallback( + 'native', + firstPreferred, + () => Promise.resolve('fallback'), + isUnsupported + ) + const second = cache.runWithFallback( + 'native', + secondPreferred, + () => Promise.resolve('fallback'), + isUnsupported + ) + await Promise.resolve() + releaseProbe('first') + + await expect(first).resolves.toBe('first') + await expect(second).resolves.toBe('second') + expect(secondPreferred).toHaveBeenCalledTimes(1) + }) + + it('isolates in-flight probes per host so a cold WSL distro never waits on native', async () => { + const cache = new CodexAppServerCapabilityCache() + const nativePreferred = vi.fn(() => new Promise(() => {})) + void cache.runWithFallback( + 'native', + nativePreferred, + () => Promise.resolve('fallback'), + isUnsupported + ) + const wslPreferred = vi.fn(() => Promise.resolve('wsl-result')) + await expect( + cache.runWithFallback( + 'wsl:Ubuntu', + wslPreferred, + () => Promise.resolve('fallback'), + isUnsupported + ) + ).resolves.toBe('wsl-result') + }) + it('builds host keys that keep WSL distros apart', () => { expect(getCodexAppServerHostKey({ kind: 'native' })).toBe('native') expect(getCodexAppServerHostKey({ kind: 'wsl', distro: 'Ubuntu' })).toBe('wsl:Ubuntu') diff --git a/src/main/codex/codex-app-server-capability-cache.ts b/src/main/codex/codex-app-server-capability-cache.ts index 85816c9cf0c..ae6c175f043 100644 --- a/src/main/codex/codex-app-server-capability-cache.ts +++ b/src/main/codex/codex-app-server-capability-cache.ts @@ -1,3 +1,5 @@ +import { CapabilityProbeCache } from '../../shared/capability-probe-cache' + // Why: suppress a known-missing RPC surface without pinning it forever — an // in-place codex upgrade during a long Orca session self-heals after the // interval, mirroring GitCapabilityCache's rationale. @@ -14,70 +16,14 @@ export function getCodexAppServerHostKey( } /** - * Capability cache for the codex app-server trust-grant RPC pair, modeled on - * GitCapabilityCache but with a synchronous runner: the grant client blocks - * the main thread by design (launch prep), so probes cannot overlap — the - * unsupported mark alone is what keeps later installs off the dead probe. + * Capability cache for the codex app-server trust-grant RPC pair. The grant + * client runs off the main thread's critical path, so two pane launches can + * probe the same host at once; the shared probe dedupe is what keeps a cold + * host to one app-server session instead of one per concurrent launch. */ -export class CodexAppServerCapabilityCache { - private readonly retryAfterByHost = new Map() - private readonly supportedHosts = new Set() - - shouldTry(hostKey: CodexAppServerHostKey, nowMs = Date.now()): boolean { - const retryAfterMs = this.retryAfterByHost.get(hostKey) - if (retryAfterMs === undefined) { - return true - } - if (nowMs < retryAfterMs) { - return false - } - this.retryAfterByHost.delete(hostKey) - return true - } - - isKnownSupported(hostKey: CodexAppServerHostKey): boolean { - return this.supportedHosts.has(hostKey) - } - - rememberUnsupported(hostKey: CodexAppServerHostKey, nowMs = Date.now()): void { - this.supportedHosts.delete(hostKey) - this.retryAfterByHost.set(hostKey, nowMs + CODEX_APP_SERVER_CAPABILITY_RETRY_INTERVAL_MS) - } - - rememberSupported(hostKey: CodexAppServerHostKey): void { - this.retryAfterByHost.delete(hostKey) - this.supportedHosts.add(hostKey) - } - - runWithFallbackSync( - hostKey: CodexAppServerHostKey, - runPreferred: () => T, - runFallback: () => T, - isUnsupportedError: (error: unknown) => boolean, - nowMs = Date.now() - ): T { - if (!this.supportedHosts.has(hostKey) && !this.shouldTry(hostKey, nowMs)) { - return runFallback() - } - try { - const result = runPreferred() - this.rememberSupported(hostKey) - return result - } catch (error) { - // Why: only a positive absence signal (unknown method / missing - // subcommand) marks unsupported. Transient spawn failures, timeouts, - // and RPC errors fall back once without poisoning the capability. - if (!isUnsupportedError(error)) { - throw error - } - this.rememberUnsupported(hostKey, nowMs) - return runFallback() - } - } - - clear(): void { - this.retryAfterByHost.clear() - this.supportedHosts.clear() +export class CodexAppServerCapabilityCache extends CapabilityProbeCache { + constructor() { + super(CODEX_APP_SERVER_CAPABILITY_RETRY_INTERVAL_MS) } } diff --git a/src/main/codex/codex-app-server-client.test.ts b/src/main/codex/codex-app-server-client.test.ts index 121aebe7255..30d4941ad57 100644 --- a/src/main/codex/codex-app-server-client.test.ts +++ b/src/main/codex/codex-app-server-client.test.ts @@ -13,10 +13,6 @@ import { type CodexHookTrustGrantRequest } from './codex-app-server-client' import { killCodexAppServerProcessTree, runCodexAppServerSession } from './codex-app-server-session' -import { - resolveCodexGrantEntryPath, - runCodexHookTrustGrantSessionSync -} from './codex-app-server-grant-bridge' // Stub codex app-server speaking the same JSONL protocol: initialize → // initialized → hooks/list → config/batchWrite → hooks/list. Scenario-driven @@ -470,106 +466,3 @@ describe('runCodexHookTrustGrantSession', () => { expect(isCodexAppServerUnsupportedError(error)).toBe(false) }) }) - -describe('runCodexHookTrustGrantSessionSync', () => { - function writeEntryFixture(source: string): string { - const root = mkdtempSync(join(tmpdir(), 'orca-codex-entry-')) - tempRoots.push(root) - const entryPath = join(root, 'grant-entry.cjs') - writeFileSync(entryPath, source) - return entryPath - } - - const baseRequest: CodexHookTrustGrantRequest = { - invocation: { command: 'codex', cliPath: null, args: ['app-server'], timeoutMs: 1_000 }, - hooksListCwd: '/tmp', - expectedTrustKeys: ['k'], - managedCommand: MANAGED_COMMAND - } - - it('returns the entry envelope result and passes the request over stdin', () => { - const entryPath = writeEntryFixture(` - let input = '' - process.stdin.setEncoding('utf8') - process.stdin.on('data', (chunk) => { input += chunk }) - process.stdin.on('end', () => { - const request = JSON.parse(input) - process.stdout.write(JSON.stringify({ - ok: true, - result: { - outcome: 'granted', - wroteTrust: true, - entries: [{ key: request.expectedTrustKeys[0], normalizedKey: request.expectedTrustKeys[0], trustedHash: 'sha256:x' }] - } - }) + '\\n') - }) - `) - const result = runCodexHookTrustGrantSessionSync(baseRequest, { entryPath }) - expect(result).toMatchObject({ outcome: 'granted', wroteTrust: true }) - }) - - it('rethrows unsupported envelopes as the unsupported error class', () => { - const entryPath = writeEntryFixture(` - process.stdin.resume() - process.stdin.on('end', () => { - process.stdout.write(JSON.stringify({ ok: false, errorName: 'CodexAppServerUnsupportedError', message: 'no app-server', unsupported: true }) + '\\n') - }) - `) - expect(() => runCodexHookTrustGrantSessionSync(baseRequest, { entryPath })).toThrow( - CodexAppServerUnsupportedError - ) - }) - - it('fails with a clear error when the entry produces no result', () => { - const entryPath = writeEntryFixture( - `process.stdin.resume(); process.stdin.on('end', () => process.exit(7))` - ) - expect(() => runCodexHookTrustGrantSessionSync(baseRequest, { entryPath })).toThrow( - /produced no result \(exit 7\)/ - ) - }) - - it('classifies the spawnSync deadline as a typed timeout', () => { - const entryPath = writeEntryFixture(`setInterval(() => {}, 1000)`) - const request = { - ...baseRequest, - invocation: { ...baseRequest.invocation, timeoutMs: 20 } - } - expect(() => - runCodexHookTrustGrantSessionSync(request, { entryPath, timeoutMarginMs: 20 }) - ).toThrow(CodexAppServerTimeoutError) - }) -}) - -describe('resolveCodexGrantEntryPath', () => { - const entryName = 'codex-app-server-grant-entry.js' - - it('finds the sibling entry from emitted main and chunk directories', () => { - const mainDir = join('/opt', 'orca', 'out', 'main') - expect( - resolveCodexGrantEntryPath( - (candidate) => candidate === join(mainDir, 'codex', entryName), - mainDir - ) - ).toBe(join(mainDir, 'codex', entryName)) - - const chunkDir = join(mainDir, 'chunks') - expect( - resolveCodexGrantEntryPath( - (candidate) => candidate === join(mainDir, 'codex', entryName), - chunkDir - ) - ).toBe(join(mainDir, 'codex', entryName)) - }) - - it('redirects app.asar to unpacked without double-unpacking an existing path', () => { - const resourcesDir = join('/Applications', 'Orca.app', 'Contents', 'Resources') - const expected = join(resourcesDir, 'app.asar.unpacked', 'out', 'main', 'codex', entryName) - for (const archiveDir of ['app.asar', 'app.asar.unpacked']) { - const moduleDir = join(resourcesDir, archiveDir, 'out', 'main', 'chunks') - expect(resolveCodexGrantEntryPath((candidate) => candidate === expected, moduleDir)).toBe( - expected - ) - } - }) -}) diff --git a/src/main/codex/codex-app-server-client.ts b/src/main/codex/codex-app-server-client.ts index cdc2c4d20ff..8c95562e66c 100644 --- a/src/main/codex/codex-app-server-client.ts +++ b/src/main/codex/codex-app-server-client.ts @@ -41,8 +41,8 @@ export type CodexGrantedHookTrust = { trustedHash: string } -/** Closed verify-failure taxonomy — crosses the grant-bridge JSON envelope, so - * telemetry never has to parse the free-form `reason` diagnostics string. */ +/** Closed verify-failure taxonomy, so telemetry never has to parse the + * free-form `reason` diagnostics string. */ export type CodexTrustGrantSessionVerifyClass = | 'list-mismatch' | 'post-grant-untrusted' diff --git a/src/main/codex/codex-app-server-grant-bridge.ts b/src/main/codex/codex-app-server-grant-bridge.ts deleted file mode 100644 index c85c4a1c479..00000000000 --- a/src/main/codex/codex-app-server-grant-bridge.ts +++ /dev/null @@ -1,144 +0,0 @@ -import { spawnSync } from 'node:child_process' -import { existsSync } from 'node:fs' -import { join } from 'node:path' -import { - CodexAppServerTimeoutError, - CodexAppServerUnsupportedError, - type CodexHookTrustGrantRequest, - type CodexHookTrustGrantSessionResult -} from './codex-app-server-client' -import type { - CodexAppServerEntryRequest, - CodexAppServerEntryResult, - GrantEntryEnvelope -} from './codex-app-server-grant-envelope' -import type { - CodexUserHookTrustRebaseRequest, - CodexUserHookTrustRebaseResult -} from './codex-user-hook-trust-rebase-client' - -// Why: hook install/refresh is synchronous launch prep — a Codex pane must -// not start before its trust is settled — but a stdio JSON-RPC session needs -// a live event loop. This bridge blocks the caller on spawnSync of a bundled -// ELECTRON_RUN_AS_NODE entry (same pattern as the daemon and parcel-watcher -// entries) that runs the session and reports one JSON envelope on stdout. - -const GRANT_ENTRY_FILE_NAME = 'codex-app-server-grant-entry.js' -// Why: spawnSync must outlive the session deadline so the entry's own timeout -// (and its result envelope) win the race; the margin only reaps a hung entry. -const GRANT_ENTRY_TIMEOUT_MARGIN_MS = 5_000 -const GRANT_ENTRY_MAX_BUFFER_BYTES = 16 * 1024 * 1024 - -export function resolveCodexGrantEntryPath( - pathExists: (candidate: string) => boolean = existsSync, - moduleDir = __dirname -): string | null { - // Why: resolved from __dirname (not electron's app paths) so this module - // stays loadable in plain-node CLI entries — the build guard rejects any - // electron require reachable from them. The emitted bridge chunk sits in - // out/main or out/main/chunks, so the entry is one or two levels up. - // ELECTRON_RUN_AS_NODE bypasses asar integration, so packaged builds must - // run the copy under app.asar.unpacked (out/main/codex/** is asarUnpacked). - const toUnpackedDir = (dir: string): string => - dir.replace(/([\\/])app\.asar(?=([\\/]|$))/, '$1app.asar.unpacked') - const baseDirs = [moduleDir, join(moduleDir, '..')].map(toUnpackedDir) - for (const baseDir of baseDirs) { - const candidate = join(baseDir, 'codex', GRANT_ENTRY_FILE_NAME) - if (pathExists(candidate)) { - return candidate - } - } - return null -} - -export type RunGrantSessionSyncOptions = { - entryPath?: string - nodeCommand?: string - /** Test-only override; production keeps enough margin for child cleanup. */ - timeoutMarginMs?: number -} - -/** - * Blocking wrapper for the grant session. Hook install/refresh is synchronous - * launch prep (pane launch must not proceed until trust is settled), and a - * stdio JSON-RPC session needs a live event loop — so the session runs in a - * short-lived ELECTRON_RUN_AS_NODE child (same pattern as the daemon and - * parcel-watcher entries) while the caller blocks on spawnSync. spawnSync - * always reaps the entry; a killed entry closes the codex child's stdin, - * which makes codex app-server exit on EOF. - */ -export function runCodexHookTrustGrantSessionSync( - request: CodexHookTrustGrantRequest, - options: RunGrantSessionSyncOptions = {} -): CodexHookTrustGrantSessionResult { - return runCodexAppServerEntrySync(request, options) as CodexHookTrustGrantSessionResult -} - -export function runCodexUserHookTrustRebaseSessionSync( - request: CodexUserHookTrustRebaseRequest, - options: RunGrantSessionSyncOptions = {} -): CodexUserHookTrustRebaseResult { - return runCodexAppServerEntrySync(request, options) as CodexUserHookTrustRebaseResult -} - -function runCodexAppServerEntrySync( - request: CodexAppServerEntryRequest, - options: RunGrantSessionSyncOptions -): CodexAppServerEntryResult { - const entryPath = options.entryPath ?? resolveCodexGrantEntryPath() - if (!entryPath) { - throw new Error('codex trust-grant entry bundle not found') - } - const spawned = spawnSync(options.nodeCommand ?? process.execPath, [entryPath], { - input: JSON.stringify(request), - encoding: 'utf8', - timeout: - request.invocation.timeoutMs + (options.timeoutMarginMs ?? GRANT_ENTRY_TIMEOUT_MARGIN_MS), - killSignal: 'SIGKILL', - maxBuffer: GRANT_ENTRY_MAX_BUFFER_BYTES, - windowsHide: true, - env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' } - }) - if ((spawned.error as NodeJS.ErrnoException | undefined)?.code === 'ETIMEDOUT') { - // Why: spawnSync reports its own deadline through error.code before the - // signal field; preserve the typed timeout so cooldown diagnostics work. - throw new CodexAppServerTimeoutError( - `codex trust-grant entry exceeded ${request.invocation.timeoutMs}ms session deadline` - ) - } - if (spawned.error) { - throw spawned.error - } - if (spawned.signal) { - throw new CodexAppServerTimeoutError( - `codex trust-grant entry killed by ${spawned.signal} after ${request.invocation.timeoutMs}ms deadline` - ) - } - const lines = (spawned.stdout ?? '').split('\n').filter((line) => line.trim().length > 0) - const lastLine = lines.at(-1) - let envelope: GrantEntryEnvelope | null = null - if (lastLine) { - try { - envelope = JSON.parse(lastLine) as GrantEntryEnvelope - } catch { - envelope = null - } - } - if (!envelope) { - throw new Error( - `codex trust-grant entry produced no result (exit ${spawned.status ?? 'unknown'})${ - spawned.stderr ? `: ${spawned.stderr.trim().slice(0, 400)}` : '' - }` - ) - } - if (!envelope.ok) { - if (envelope.unsupported) { - throw new CodexAppServerUnsupportedError(envelope.message) - } - if (envelope.errorName === 'CodexAppServerTimeoutError') { - throw new CodexAppServerTimeoutError(envelope.message) - } - throw new Error(envelope.message) - } - return envelope.result -} diff --git a/src/main/codex/codex-app-server-grant-entry.ts b/src/main/codex/codex-app-server-grant-entry.ts deleted file mode 100644 index 8e16892d279..00000000000 --- a/src/main/codex/codex-app-server-grant-entry.ts +++ /dev/null @@ -1,79 +0,0 @@ -// Forked (ELECTRON_RUN_AS_NODE) child that runs one codex app-server -// trust-grant session. The parent blocks on spawnSync because hook -// install/refresh must finish before a Codex pane launch proceeds, while the -// JSONL RPC session itself needs a live event loop. Reads the request JSON -// from stdin, writes a single result-envelope JSON line to stdout, and never -// imports electron (see PLAIN_NODE_ENTRY_NAMES in the build guard). -import { - buildGrantEntryEnvelope, - type CodexAppServerEntryRequest -} from './codex-app-server-grant-envelope' -import { writeSync } from 'node:fs' -import { runCodexHookTrustGrantSession } from './codex-app-server-client' -import { runCodexUserHookTrustRebaseSession } from './codex-user-hook-trust-rebase-client' - -const HARD_EXIT_MARGIN_MS = 2_000 - -async function readStdin(): Promise { - const chunks: Buffer[] = [] - for await (const chunk of process.stdin) { - chunks.push(chunk as Buffer) - } - return Buffer.concat(chunks).toString('utf8') -} - -async function main(): Promise { - const raw = await readStdin() - let request: CodexAppServerEntryRequest - try { - request = JSON.parse(raw) as CodexAppServerEntryRequest - } catch (error) { - process.stdout.write( - `${JSON.stringify({ - ok: false, - errorName: 'Error', - message: `invalid trust-grant request JSON: ${error instanceof Error ? error.message : String(error)}` - })}\n` - ) - return - } - // Why: backstop for a session whose own deadline failed to fire (clock - // suspend mid-session); exiting closes the codex child's stdio so it - // exits on EOF instead of orphaning. - const hardExit = setTimeout(() => { - // Why: process.exit() does not flush asynchronous stdout pipes; write the - // timeout envelope synchronously so the parent can classify the fallback. - writeSync( - process.stdout.fd, - `${JSON.stringify({ - ok: false, - errorName: 'CodexAppServerTimeoutError', - message: `trust-grant entry hard deadline (${request.invocation.timeoutMs + HARD_EXIT_MARGIN_MS}ms) elapsed` - })}\n` - ) - process.exit(3) - }, request.invocation.timeoutMs + HARD_EXIT_MARGIN_MS) - const run = - 'operation' in request - ? runCodexUserHookTrustRebaseSession(request) - : runCodexHookTrustGrantSession(request) - const envelope = await buildGrantEntryEnvelope(run) - clearTimeout(hardExit) - process.stdout.write(`${JSON.stringify(envelope)}\n`) -} - -void main().then( - () => { - process.exitCode = 0 - }, - (error: unknown) => { - process.stdout.write( - `${JSON.stringify({ - ok: false, - errorName: error instanceof Error ? error.name : 'Error', - message: error instanceof Error ? error.message : String(error) - })}\n` - ) - process.exitCode = 0 - } -) diff --git a/src/main/codex/codex-app-server-grant-envelope.ts b/src/main/codex/codex-app-server-grant-envelope.ts deleted file mode 100644 index d5b133c2975..00000000000 --- a/src/main/codex/codex-app-server-grant-envelope.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { - isCodexAppServerUnsupportedError, - type CodexHookTrustGrantRequest, - type CodexHookTrustGrantSessionResult -} from './codex-app-server-client' -import type { - CodexUserHookTrustRebaseRequest, - CodexUserHookTrustRebaseResult -} from './codex-user-hook-trust-rebase-client' - -export type CodexAppServerEntryRequest = - | CodexHookTrustGrantRequest - | CodexUserHookTrustRebaseRequest - -export type CodexAppServerEntryResult = - | CodexHookTrustGrantSessionResult - | CodexUserHookTrustRebaseResult - -export type GrantEntryEnvelope = - | { ok: true; result: CodexAppServerEntryResult } - | { ok: false; errorName: string; message: string; unsupported?: boolean } - -export function buildGrantEntryEnvelope( - run: Promise -): Promise { - return run.then( - (result) => ({ ok: true as const, result }), - (error: unknown) => ({ - ok: false as const, - errorName: error instanceof Error ? error.name : 'Error', - message: error instanceof Error ? error.message : String(error), - ...(isCodexAppServerUnsupportedError(error) ? { unsupported: true as const } : {}) - }) - ) -} diff --git a/src/main/codex/codex-hook-trust-grant.test.ts b/src/main/codex/codex-hook-trust-grant.test.ts index e1ca7c41076..5292be120c1 100644 --- a/src/main/codex/codex-hook-trust-grant.test.ts +++ b/src/main/codex/codex-hook-trust-grant.test.ts @@ -45,7 +45,7 @@ beforeEach(() => { afterEach(() => { vi.useRealTimers() - _internals.setGrantSessionRunnerSync(null) + _internals.setGrantSessionRunner(null) setCodexTrustGrantTelemetry(() => {}) codexAppServerCapabilityCache.clear() if (previousUserDataPath === undefined) { @@ -97,28 +97,30 @@ function grantedSessionResult(entries: CodexTrustEntry[], hashPrefix = 'sha256:c } describe('grantManagedCodexHookTrust', () => { - it('does not let a short trust RPC claim an incomplete session index', () => { + it('does not let a short trust RPC claim an incomplete session index', async () => { const sessions = join(runtimeHomeDir, 'sessions') mkdirSync(sessions, { recursive: true }) for (let index = 0; index < 100; index += 1) { writeFileSync(join(sessions, `${index}.jsonl`), '{}\n') } const runner = vi.fn() - _internals.setGrantSessionRunnerSync(runner) + _internals.setGrantSessionRunner(runner) - expect(grantManagedCodexHookTrust(buildPlan([managedEntry('stop')]))).toMatchObject({ + expect(await grantManagedCodexHookTrust(buildPlan([managedEntry('stop')]))).toMatchObject({ lane: 'fallback', reason: 'retry-cached' }) expect(runner).not.toHaveBeenCalled() }) - it('returns granted entries with codex-verbatim hashes and records the ledger', () => { + it('returns granted entries with codex-verbatim hashes and records the ledger', async () => { const entries = [managedEntry('session_start'), managedEntry('stop')] - const runner = vi.fn((_request: CodexHookTrustGrantRequest) => grantedSessionResult(entries)) - _internals.setGrantSessionRunnerSync(runner) + const runner = vi.fn(async (_request: CodexHookTrustGrantRequest) => + grantedSessionResult(entries) + ) + _internals.setGrantSessionRunner(runner) - const outcome = grantManagedCodexHookTrust(buildPlan(entries)) + const outcome = await grantManagedCodexHookTrust(buildPlan(entries)) expect(outcome.lane).toBe('rpc') if (outcome.lane !== 'rpc') { return @@ -139,20 +141,22 @@ describe('grantManagedCodexHookTrust', () => { expect(getCodexTrustGrantDiagnostics()).toMatchObject({ granted: 1, fellBack: 0 }) }) - it('builds a default-home grant invocation without an inherited CODEX_HOME', () => { + it('builds a default-home grant invocation without an inherited CODEX_HOME', async () => { const entries = [managedEntry('stop')] - const runner = vi.fn((_request: CodexHookTrustGrantRequest) => grantedSessionResult(entries)) - _internals.setGrantSessionRunnerSync(runner) + const runner = vi.fn(async (_request: CodexHookTrustGrantRequest) => + grantedSessionResult(entries) + ) + _internals.setGrantSessionRunner(runner) expect( - grantManagedCodexHookTrust({ ...buildPlan(entries), useDefaultCodexHome: true }) + await grantManagedCodexHookTrust({ ...buildPlan(entries), useDefaultCodexHome: true }) ).toMatchObject({ lane: 'rpc' }) const invocation = runner.mock.calls[0]![0]!.invocation expect(invocation.env?.CODEX_HOME).toBeUndefined() expect(invocation.envToDelete).toContain('CODEX_HOME') }) - it('removes equivalent Windows fallback keys before the RPC writes canonical trust', () => { + it('removes equivalent Windows fallback keys before the RPC writes canonical trust', async () => { const entry: CodexTrustEntry = { ...managedEntry('stop'), sourcePath: String.raw`C:\Users\Alice\.codex\hooks.json` @@ -162,23 +166,23 @@ describe('grantManagedCodexHookTrust', () => { expect(readHookTrustEntries(plan.tomlPath).get(computeTrustKey(entry))?.trustedHash).toBe( computeTrustedHash(entry) ) - const runner = vi.fn(() => { + const runner = vi.fn(async () => { expect(readHookTrustEntries(plan.tomlPath).has(computeTrustKey(entry))).toBe(false) return grantedSessionResult([entry]) }) - _internals.setGrantSessionRunnerSync(runner) + _internals.setGrantSessionRunner(runner) - expect(grantManagedCodexHookTrust(plan)).toMatchObject({ lane: 'rpc' }) + expect(await grantManagedCodexHookTrust(plan)).toMatchObject({ lane: 'rpc' }) expect(runner).toHaveBeenCalledTimes(1) }) - it('skips the RPC session while the ledger grant still holds, and re-grants on config drift', () => { + it('skips the RPC session while the ledger grant still holds, and re-grants on config drift', async () => { const entries = [managedEntry('session_start')] - const runner = vi.fn(() => grantedSessionResult(entries)) - _internals.setGrantSessionRunnerSync(runner) + const runner = vi.fn(async () => grantedSessionResult(entries)) + _internals.setGrantSessionRunner(runner) const plan = buildPlan(entries) - const first = grantManagedCodexHookTrust(plan) + const first = await grantManagedCodexHookTrust(plan) expect(first.lane).toBe('rpc') expect(runner).toHaveBeenCalledTimes(1) @@ -187,92 +191,95 @@ describe('grantManagedCodexHookTrust', () => { upsertHookTrustEntries(plan.tomlPath, [ { ...entries[0], trustedHash: 'sha256:codex-session_start' } ]) - const second = grantManagedCodexHookTrust(plan) + const second = await grantManagedCodexHookTrust(plan) expect(second.lane).toBe('rpc') expect(runner).toHaveBeenCalledTimes(1) expect(getCodexTrustGrantDiagnostics()).toMatchObject({ granted: 1, ledgerHits: 1 }) // Config drift (user wiped the trust entry) must re-run the session. upsertHookTrustEntries(plan.tomlPath, [{ ...entries[0], trustedHash: 'sha256:wiped' }]) - const third = grantManagedCodexHookTrust(plan) + const third = await grantManagedCodexHookTrust(plan) expect(third.lane).toBe('rpc') expect(runner).toHaveBeenCalledTimes(2) }) - it('re-grants when the managed hook identity changes', () => { + it('re-grants when the managed hook identity changes', async () => { const entries = [managedEntry('session_start')] - const runner = vi.fn(() => grantedSessionResult(entries)) - _internals.setGrantSessionRunnerSync(runner) + const runner = vi.fn(async () => grantedSessionResult(entries)) + _internals.setGrantSessionRunner(runner) const plan = buildPlan(entries) - grantManagedCodexHookTrust(plan) + await grantManagedCodexHookTrust(plan) upsertHookTrustEntries(plan.tomlPath, [ { ...entries[0], trustedHash: 'sha256:codex-session_start' } ]) const changedEntries = [{ ...entries[0], timeoutSec: 99 }] - const changedRunner = vi.fn(() => grantedSessionResult(changedEntries)) - _internals.setGrantSessionRunnerSync(changedRunner) - const outcome = grantManagedCodexHookTrust(buildPlan(changedEntries)) + const changedRunner = vi.fn(async () => grantedSessionResult(changedEntries)) + _internals.setGrantSessionRunner(changedRunner) + const outcome = await grantManagedCodexHookTrust(buildPlan(changedEntries)) expect(outcome.lane).toBe('rpc') expect(changedRunner).toHaveBeenCalledTimes(1) }) - it('marks the host unsupported only for the unsupported error class', () => { + it('marks the host unsupported only for the unsupported error class', async () => { const entries = [managedEntry('session_start')] - const runner = vi.fn((): CodexHookTrustGrantSessionResult => { + const runner = vi.fn((): Promise => { throw new CodexAppServerUnsupportedError('no such method') }) - _internals.setGrantSessionRunnerSync(runner) + _internals.setGrantSessionRunner(runner) const plan = buildPlan(entries) - expect(grantManagedCodexHookTrust(plan)).toMatchObject({ + expect(await grantManagedCodexHookTrust(plan)).toMatchObject({ lane: 'fallback', reason: 'unsupported' }) expect(runner).toHaveBeenCalledTimes(1) // Cached: the second install skips the probe entirely. - expect(grantManagedCodexHookTrust(plan)).toMatchObject({ + expect(await grantManagedCodexHookTrust(plan)).toMatchObject({ lane: 'fallback', reason: 'unsupported-cached' }) expect(runner).toHaveBeenCalledTimes(1) }) - it('backs off transient failures without poisoning the capability', () => { + it('backs off transient failures without poisoning the capability', async () => { vi.useFakeTimers() vi.setSystemTime(1_000) const entries = [managedEntry('session_start')] - const runner = vi.fn((): CodexHookTrustGrantSessionResult => { + const runner = vi.fn((): Promise => { throw new Error('spawn ETIMEDOUT') }) - _internals.setGrantSessionRunnerSync(runner) + _internals.setGrantSessionRunner(runner) const plan = buildPlan(entries) - expect(grantManagedCodexHookTrust(plan)).toMatchObject({ lane: 'fallback', reason: 'error' }) - expect(grantManagedCodexHookTrust(plan)).toMatchObject({ + expect(await grantManagedCodexHookTrust(plan)).toMatchObject({ + lane: 'fallback', + reason: 'error' + }) + expect(await grantManagedCodexHookTrust(plan)).toMatchObject({ lane: 'fallback', reason: 'retry-cached' }) expect(runner).toHaveBeenCalledTimes(1) expect(codexAppServerCapabilityCache.shouldTry('native')).toBe(true) - runner.mockImplementation(() => grantedSessionResult(entries)) + runner.mockImplementation(async () => grantedSessionResult(entries)) vi.setSystemTime(1_000 + CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS) - expect(grantManagedCodexHookTrust(plan)).toMatchObject({ lane: 'rpc' }) + expect(await grantManagedCodexHookTrust(plan)).toMatchObject({ lane: 'rpc' }) expect(runner).toHaveBeenCalledTimes(2) }) - it('falls back on verify-failed without marking unsupported', () => { + it('falls back on verify-failed without marking unsupported', async () => { const entries = [managedEntry('session_start')] - const runner = vi.fn(() => ({ + const runner = vi.fn(async () => ({ outcome: 'verify-failed' as const, reason: 'missing entries', reasonClass: 'list-mismatch' as const })) - _internals.setGrantSessionRunnerSync(runner) + _internals.setGrantSessionRunner(runner) - expect(grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ + expect(await grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ lane: 'fallback', reason: 'verify-failed' }) @@ -280,53 +287,56 @@ describe('grantManagedCodexHookTrust', () => { expect(getCodexTrustGrantDiagnostics()).toMatchObject({ verifyFailed: 1 }) }) - it('rejects duplicate granted keys instead of treating another key as covered', () => { + it('rejects duplicate granted keys instead of treating another key as covered', async () => { const entries = [managedEntry('session_start'), managedEntry('stop')] const duplicated = grantedSessionResult([entries[0]!, entries[0]!]) - _internals.setGrantSessionRunnerSync(() => duplicated) + _internals.setGrantSessionRunner(async () => duplicated) - expect(grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ + expect(await grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ lane: 'fallback', reason: 'verify-failed' }) expect(readCodexTrustGrantLedgerHome(runtimeHomeDir)).toBeNull() }) - it('keeps grant and fallback outcomes stable when telemetry throws', () => { + it('keeps grant and fallback outcomes stable when telemetry throws', async () => { const entries = [managedEntry('session_start')] setCodexTrustGrantTelemetry(() => { throw new Error('telemetry unavailable') }) - _internals.setGrantSessionRunnerSync(() => grantedSessionResult(entries)) + _internals.setGrantSessionRunner(async () => grantedSessionResult(entries)) - expect(grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ lane: 'rpc' }) + expect(await grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ lane: 'rpc' }) process.env.ORCA_DISABLE_CODEX_TRUST_RPC = '1' - expect(grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ + expect(await grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ lane: 'fallback', reason: 'disabled' }) }) - it('restores exact config bytes before fallback after a mutating RPC error', () => { + it('restores exact config bytes before fallback after a mutating RPC error', async () => { const entries = [managedEntry('session_start')] const plan = buildPlan(entries) const original = '# user formatting\r\n[hooks]\r\n' mkdirSync(runtimeHomeDir, { recursive: true }) writeFileSync(plan.tomlPath, original) - _internals.setGrantSessionRunnerSync(() => { + _internals.setGrantSessionRunner(async () => { writeFileSync(plan.tomlPath, '[hooks.state."rpc-partial"]\ntrusted_hash = "changed"\n') throw new Error('post-write transport failure') }) - expect(grantManagedCodexHookTrust(plan)).toMatchObject({ lane: 'fallback', reason: 'error' }) + expect(await grantManagedCodexHookTrust(plan)).toMatchObject({ + lane: 'fallback', + reason: 'error' + }) expect(readFileSync(plan.tomlPath, 'utf8')).toBe(original) }) - it('removes an RPC-created config before fallback when none existed', () => { + it('removes an RPC-created config before fallback when none existed', async () => { const entries = [managedEntry('session_start')] const plan = buildPlan(entries) mkdirSync(runtimeHomeDir, { recursive: true }) - _internals.setGrantSessionRunnerSync(() => { + _internals.setGrantSessionRunner(async () => { writeFileSync(plan.tomlPath, '[hooks.state."rpc-partial"]\ntrusted_hash = "changed"\n') return { outcome: 'verify-failed', @@ -335,32 +345,116 @@ describe('grantManagedCodexHookTrust', () => { } }) - expect(grantManagedCodexHookTrust(plan)).toMatchObject({ + expect(await grantManagedCodexHookTrust(plan)).toMatchObject({ lane: 'fallback', reason: 'verify-failed' }) expect(existsSync(plan.tomlPath)).toBe(false) }) - it('honors the ops kill switch env flag', () => { + it('honors the ops kill switch env flag', async () => { process.env.ORCA_DISABLE_CODEX_TRUST_RPC = '1' const entries = [managedEntry('session_start')] - const runner = vi.fn(() => grantedSessionResult(entries)) - _internals.setGrantSessionRunnerSync(runner) + const runner = vi.fn(async () => grantedSessionResult(entries)) + _internals.setGrantSessionRunner(runner) - expect(grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ + expect(await grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ lane: 'fallback', reason: 'disabled' }) expect(runner).not.toHaveBeenCalled() }) - it('builds a WSL invocation that runs codex inside the distro', () => { + // Why (#16441): the grant used to run through spawnSync, so two grants on one + // config.toml were impossible by construction. Now they must queue — an + // interleaved capture/restore pair resurrects trust the other run removed. + it('serializes concurrent grants that share one config.toml', async () => { const entries = [managedEntry('session_start')] - const runner = vi.fn((_request: CodexHookTrustGrantRequest) => grantedSessionResult(entries)) - _internals.setGrantSessionRunnerSync(runner) + const plan = buildPlan(entries) + let inFlight = 0 + let maxInFlight = 0 + const releases: (() => void)[] = [] + _internals.setGrantSessionRunner(async () => { + inFlight += 1 + maxInFlight = Math.max(maxInFlight, inFlight) + await new Promise((resolve) => releases.push(resolve)) + inFlight -= 1 + return grantedSessionResult(entries) + }) - const outcome = grantManagedCodexHookTrust({ + const first = grantManagedCodexHookTrust(plan) + const second = grantManagedCodexHookTrust(plan) + await vi.waitFor(() => expect(releases).toHaveLength(1)) + releases[0]!() + await first + await vi.waitFor(() => expect(releases).toHaveLength(2)) + releases[1]!() + await second + + expect(maxInFlight).toBe(1) + }) + + it('lets grants on different config.toml paths overlap', async () => { + const entries = [managedEntry('session_start')] + const otherHome = join(userDataDir, 'codex-accounts', 'other', 'home') + mkdirSync(otherHome, { recursive: true }) + // Why: the probe dedupe only holds the first session on an unproven host. + // A known-supported host must keep its intended launch concurrency. + codexAppServerCapabilityCache.rememberSupported('native') + let inFlight = 0 + let maxInFlight = 0 + const releases: (() => void)[] = [] + _internals.setGrantSessionRunner(async () => { + inFlight += 1 + maxInFlight = Math.max(maxInFlight, inFlight) + await new Promise((resolve) => releases.push(resolve)) + inFlight -= 1 + return grantedSessionResult(entries) + }) + + const first = grantManagedCodexHookTrust(buildPlan(entries)) + const second = grantManagedCodexHookTrust({ + ...buildPlan(entries), + runtimeHomePath: otherHome, + tomlPath: join(otherHome, 'config.toml') + }) + await vi.waitFor(() => expect(releases).toHaveLength(2)) + releases.forEach((release) => release()) + await Promise.all([first, second]) + + expect(maxInFlight).toBe(2) + }) + + it('dedupes the capability probe when concurrent grants hit an unsupported host', async () => { + const entries = [managedEntry('session_start')] + const otherHome = join(userDataDir, 'codex-accounts', 'other', 'home') + mkdirSync(otherHome, { recursive: true }) + const releases: ((error: unknown) => void)[] = [] + const runner = vi.fn(() => new Promise((_resolve, reject) => releases.push(reject))) + _internals.setGrantSessionRunner(runner) + + const first = grantManagedCodexHookTrust(buildPlan(entries)) + const second = grantManagedCodexHookTrust({ + ...buildPlan(entries), + runtimeHomePath: otherHome, + tomlPath: join(otherHome, 'config.toml') + }) + await vi.waitFor(() => expect(releases).toHaveLength(1)) + releases[0]!(new CodexAppServerUnsupportedError('no such method')) + + expect(await first).toMatchObject({ lane: 'fallback', reason: 'unsupported' }) + expect(await second).toMatchObject({ lane: 'fallback', reason: 'unsupported-cached' }) + expect(runner).toHaveBeenCalledTimes(1) + }) + + it('builds a WSL invocation that runs codex inside the distro', async () => { + const entries = [managedEntry('session_start')] + const runner = vi.fn(async (_request: CodexHookTrustGrantRequest) => + grantedSessionResult(entries) + ) + _internals.setGrantSessionRunner(runner) + + const outcome = await grantManagedCodexHookTrust({ ...buildPlan(entries), host: { kind: 'wsl', distro: 'Ubuntu', linuxRuntimeHome: '/home/alice/.codex-runtime' } }) @@ -384,32 +478,32 @@ describe('trust-grant telemetry detail', () => { return events } - it('attributes the plan lane on granted events', () => { + it('attributes the plan lane on granted events', async () => { const events = captureTelemetry() const entries = [managedEntry('session_start')] - _internals.setGrantSessionRunnerSync(() => grantedSessionResult(entries)) + _internals.setGrantSessionRunner(async () => grantedSessionResult(entries)) - expect(grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ lane: 'rpc' }) + expect(await grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ lane: 'rpc' }) expect(events).toEqual([{ outcome: 'granted', hostKind: 'native', lane: 'real-home' }]) }) - it('reports the managed lane independently of host kind', () => { + it('reports the managed lane independently of host kind', async () => { const events = captureTelemetry() const entries = [managedEntry('session_start')] - _internals.setGrantSessionRunnerSync(() => grantedSessionResult(entries)) + _internals.setGrantSessionRunner(async () => grantedSessionResult(entries)) - grantManagedCodexHookTrust({ ...buildPlan(entries), telemetryLane: 'managed' }) + await grantManagedCodexHookTrust({ ...buildPlan(entries), telemetryLane: 'managed' }) expect(events).toEqual([{ outcome: 'granted', hostKind: 'native', lane: 'managed' }]) }) - it('classifies error fallbacks on the wire', () => { + it('classifies error fallbacks on the wire', async () => { const events = captureTelemetry() const entries = [managedEntry('session_start')] - _internals.setGrantSessionRunnerSync(() => { + _internals.setGrantSessionRunner(async () => { throw new Error('spawn codex ENOENT') }) - expect(grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ + expect(await grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ lane: 'fallback', reason: 'error' }) @@ -424,16 +518,16 @@ describe('trust-grant telemetry detail', () => { ]) }) - it('carries the session verify class through the fallback event', () => { + it('carries the session verify class through the fallback event', async () => { const events = captureTelemetry() const entries = [managedEntry('session_start')] - _internals.setGrantSessionRunnerSync(() => ({ + _internals.setGrantSessionRunner(async () => ({ outcome: 'verify-failed' as const, reason: 'post-grant verify left 1 entries untrusted', reasonClass: 'post-grant-untrusted' as const })) - grantManagedCodexHookTrust(buildPlan(entries)) + await grantManagedCodexHookTrust(buildPlan(entries)) expect(events).toEqual([ { outcome: 'verify_failed', @@ -445,12 +539,12 @@ describe('trust-grant telemetry detail', () => { ]) }) - it('classifies module-detected verify failures', () => { + it('classifies module-detected verify failures', async () => { const events = captureTelemetry() const entries = [managedEntry('session_start'), managedEntry('stop')] - _internals.setGrantSessionRunnerSync(() => grantedSessionResult([entries[0]!, entries[0]!])) + _internals.setGrantSessionRunner(async () => grantedSessionResult([entries[0]!, entries[0]!])) - grantManagedCodexHookTrust(buildPlan(entries)) + await grantManagedCodexHookTrust(buildPlan(entries)) expect(events).toEqual([ { outcome: 'verify_failed', diff --git a/src/main/codex/codex-hook-trust-grant.ts b/src/main/codex/codex-hook-trust-grant.ts index 9142a590ab3..f886dbb6344 100644 --- a/src/main/codex/codex-hook-trust-grant.ts +++ b/src/main/codex/codex-hook-trust-grant.ts @@ -1,5 +1,6 @@ import { isCodexAppServerUnsupportedError, + runCodexHookTrustGrantSession, type CodexHookTrustGrantRequest, type CodexHookTrustGrantSessionResult } from './codex-app-server-client' @@ -10,55 +11,50 @@ import { type CodexTrustGrantTelemetryLane, type CodexTrustGrantVerifyClass } from './codex-trust-grant-telemetry' -import { runCodexHookTrustGrantSessionSync } from './codex-app-server-grant-bridge' import { codexAppServerCapabilityCache, - getCodexAppServerHostKey + getCodexAppServerHostKey, + type CodexAppServerHostKey } from './codex-app-server-capability-cache' import { writeCodexTrustGrantLedgerHome, type CodexTrustGrantBinaryStamp, type CodexTrustGrantLedgerEntry } from './codex-trust-grant-ledger' -import { - computeTrustKey, - computeTrustedHash, - normalizeHookTrustKeyForLookup, - readHookTrustEntries, - removeHookTrustEntries, - type CodexTrustEntry -} from './config-toml-trust' -import { getCodexHookTrustSignature } from './codex-hook-identity' +import type { CodexTrustEntry } from './config-toml-trust' import { captureCodexTrustConfig, restoreCodexTrustConfig } from './codex-trust-config-rollback' +import { runExclusivelyForCodexTrustConfig } from './codex-trust-config-mutation-queue' import { - readCodexTrustGrantLedgerHomeMatchingStamp, resolveCodexTrustGrantHost, - type CodexTrustGrantHost + type ResolvedCodexTrustGrantHost } from './codex-trust-grant-host' +import { + buildExpectedEntries, + findLedgerGrant, + removeSelfComputedTrustBeforeGrant, + type CodexManagedTrustGrantPlan, + type ExpectedManagedEntry +} from './codex-managed-trust-grant-plan' import { isCodexStateDbBackfillPending } from './codex-state-db' // Why: a transiently hung app-server must not block launch prep on every pane. // The legacy lane remains available while a short, host-scoped cooldown runs. export const CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS = 5 * 60_000 -/** Ops escape hatch (not a setting): forces the unchanged fallback lane. */ +/** + * Ops escape hatch (not a setting): forces the unchanged fallback lane for the + * *managed* grant only. + * + * Scope, because the name reads broader than it is: the real-home rebase + * (`mutateRealHomeHooksPreservingUserTrust`) still runs its own inspect/repair + * app-server sessions when Orca's insertion shifts a user's hook positions, and + * does not read this flag. That is unchanged from before the grant went async — + * those sessions simply used to block the main thread instead. Widening the flag + * to cover the rebase is a follow-up, not something this constant already does. + */ const DISABLE_ENV_FLAG = 'ORCA_DISABLE_CODEX_TRUST_RPC' -export type CodexManagedTrustGrantPlan = { - /** Host-visible runtime home path (UNC for WSL) — ledger key + config reads. */ - runtimeHomePath: string - /** Host-visible config.toml path holding the trust entries. */ - tomlPath: string - /** Exact command string written to the managed hooks.json entries. */ - managedCommand: string - /** Managed trust identities Orca just wrote (no trustedHash). */ - managedEntries: readonly CodexTrustEntry[] - host: CodexTrustGrantHost - telemetryLane: CodexTrustGrantTelemetryLane - /** Match a pane where CODEX_HOME is absent instead of an explicit managed home. */ - useDefaultCodexHome?: boolean -} - +export type { CodexManagedTrustGrantPlan } export type { CodexTrustGrantFallbackReason, CodexTrustGrantTelemetryLane } export type CodexManagedTrustGrantOutcome = @@ -77,11 +73,14 @@ const transientRetryAfterByHost = new Map() export const getCodexTrustGrantDiagnostics = (): CodexTrustGrantDiagnostics => ({ ...diagnostics }) -type GrantSessionRunnerSync = ( +type GrantSessionRunner = ( request: CodexHookTrustGrantRequest -) => CodexHookTrustGrantSessionResult +) => Promise -let runSessionSync: GrantSessionRunnerSync = runCodexHookTrustGrantSessionSync +// Why (#16441): the session runs in-process on the main thread's event loop. +// It used to be forked through spawnSync purely to donate an event loop to a +// deliberately-blocked parent, which froze the window for the whole deadline. +let runSession: GrantSessionRunner = runCodexHookTrustGrantSession function fallback( plan: CodexManagedTrustGrantPlan, @@ -109,60 +108,141 @@ function fallback( return { lane: 'fallback', reason } } -type ExpectedManagedEntry = { - entry: CodexTrustEntry - normalizedKey: string - signature: string +function startTransientCooldown(hostKey: CodexAppServerHostKey): void { + transientRetryAfterByHost.set(hostKey, Date.now() + CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS) } -function buildExpectedEntries(plan: CodexManagedTrustGrantPlan): ExpectedManagedEntry[] { - return plan.managedEntries.map((entry) => ({ - entry, - normalizedKey: normalizeHookTrustKeyForLookup(computeTrustKey(entry)), - signature: getCodexHookTrustSignature(entry) - })) +type GrantAttempt = { + plan: CodexManagedTrustGrantPlan + expected: ExpectedManagedEntry[] + hostKey: CodexAppServerHostKey + currentStamp: CodexTrustGrantBinaryStamp | null + configSnapshot: ReturnType + startedAtMs: number } -function removeSelfComputedTrustBeforeGrant(plan: CodexManagedTrustGrantPlan): void { - const trustStates = readHookTrustEntries(plan.tomlPath) - const ownedKeys = plan.managedEntries - .map((entry) => { - const key = computeTrustKey(entry) - return trustStates.get(key)?.trustedHash === computeTrustedHash(entry) ? key : null - }) - .filter((key): key is string => key !== null) - if (ownedKeys.length > 0) { - removeHookTrustEntries(plan.tomlPath, ownedKeys) +/** Post-session verification, ledger persistence and telemetry. Never throws for + * a verify failure — every rejection is a rolled-back fallback. */ +function completeGrant( + attempt: GrantAttempt, + result: CodexHookTrustGrantSessionResult +): CodexManagedTrustGrantOutcome { + const { plan, expected, hostKey, configSnapshot } = attempt + const rejectGrant = ( + detail: unknown, + verifyClass: CodexTrustGrantVerifyClass + ): CodexManagedTrustGrantOutcome => { + restoreCodexTrustConfig(plan.tomlPath, configSnapshot) + startTransientCooldown(hostKey) + return fallback(plan, 'verify-failed', detail, verifyClass) } + if (result.outcome === 'verify-failed') { + return rejectGrant(result.reason, result.reasonClass) + } + + const byNormalizedKey = new Map(expected.map((item) => [item.normalizedKey, item])) + const seenNormalizedKeys = new Set() + const grantedEntries: CodexTrustEntry[] = [] + const ledgerRecord: Record = {} + for (const granted of result.entries) { + const match = byNormalizedKey.get(granted.normalizedKey) + if (!match) { + return rejectGrant(`unexpected granted key ${granted.key}`, 'unexpected-key') + } + if (seenNormalizedKeys.has(granted.normalizedKey)) { + return rejectGrant(`duplicate granted key ${granted.key}`, 'duplicate-key') + } + seenNormalizedKeys.add(granted.normalizedKey) + grantedEntries.push({ ...match.entry, trustedHash: granted.trustedHash }) + ledgerRecord[granted.normalizedKey] = { + signature: match.signature, + trustedHash: granted.trustedHash + } + } + if (seenNormalizedKeys.size !== expected.length) { + return rejectGrant('granted entry set did not cover expected entries', 'coverage') + } + transientRetryAfterByHost.delete(hostKey) + try { + writeCodexTrustGrantLedgerHome(plan.runtimeHomePath, { + binary: attempt.currentStamp, + entries: ledgerRecord + }) + } catch (error) { + // Why: a ledger write failure only costs an extra session next launch. + console.warn('[codex-trust-grant] failed to persist grant ledger', error) + } + diagnostics.granted += 1 + console.log( + `[codex-trust-grant] granted ${grantedEntries.length} managed hook entries via codex app-server ` + + `(host=${plan.host.kind}, wrote=${result.wroteTrust}, ${Date.now() - attempt.startedAtMs}ms)` + ) + emitCodexTrustGrantTelemetry({ + outcome: 'granted', + hostKind: plan.host.kind, + lane: plan.telemetryLane + }) + return { lane: 'rpc', entries: grantedEntries } } -function findLedgerGrant( +async function runGrantAttempt( plan: CodexManagedTrustGrantPlan, expected: ExpectedManagedEntry[], - currentStamp: CodexTrustGrantBinaryStamp | null -): CodexTrustEntry[] | null { - const home = readCodexTrustGrantLedgerHomeMatchingStamp(plan.runtimeHomePath, currentStamp) - if (!home) { - return null + resolvedHost: ResolvedCodexTrustGrantHost, + hostKey: CodexAppServerHostKey +): Promise { + // Why: the RPC may rewrite config.toml before a later RPC fails. Restore its + // exact pre-session bytes before the legacy lane runs so every fallback has + // the same input and output as the pre-RPC implementation. + const attempt: GrantAttempt = { + plan, + expected, + hostKey, + currentStamp: resolvedHost.binaryStamp, + configSnapshot: captureCodexTrustConfig(plan.tomlPath), + startedAtMs: Date.now() } - let trustStates: ReturnType + let unsupportedError: unknown try { - trustStates = readHookTrustEntries(plan.tomlPath) - } catch { - return null + return await codexAppServerCapabilityCache.runWithFallback( + hostKey, + async () => { + removeSelfComputedTrustBeforeGrant(plan) + return completeGrant( + attempt, + await runSession( + resolvedHost.buildRequest({ + runtimeHomePath: plan.runtimeHomePath, + managedCommand: plan.managedCommand, + expectedTrustKeys: expected.map(({ normalizedKey }) => normalizedKey), + useDefaultCodexHome: plan.useDefaultCodexHome + }) + ) + ) + }, + async () => { + if (unsupportedError === undefined) { + // Why: a concurrent launch's probe proved the surface missing while + // this one waited behind it; nothing was mutated, so nothing to undo. + return fallback(plan, 'unsupported-cached') + } + restoreCodexTrustConfig(plan.tomlPath, attempt.configSnapshot) + transientRetryAfterByHost.delete(hostKey) + return fallback(plan, 'unsupported', unsupportedError) + }, + (error) => { + if (!isCodexAppServerUnsupportedError(error)) { + return false + } + unsupportedError = error + return true + } + ) + } catch (error) { + restoreCodexTrustConfig(plan.tomlPath, attempt.configSnapshot) + startTransientCooldown(hostKey) + return fallback(plan, 'error', error) } - const entries: CodexTrustEntry[] = [] - for (const { entry, normalizedKey, signature } of expected) { - const recorded = home.entries[normalizedKey] - if (!recorded || recorded.signature !== signature) { - return null - } - if (trustStates.get(normalizedKey)?.trustedHash !== recorded.trustedHash) { - return null - } - entries.push({ ...entry, trustedHash: recorded.trustedHash }) - } - return entries } /** @@ -173,9 +253,9 @@ function findLedgerGrant( * throws: any unexpected failure is a fallback, because hook install is * best-effort launch prep. */ -export function grantManagedCodexHookTrust( +export async function grantManagedCodexHookTrust( plan: CodexManagedTrustGrantPlan -): CodexManagedTrustGrantOutcome { +): Promise { try { if (process.env[DISABLE_ENV_FLAG] === '1') { return fallback(plan, 'disabled') @@ -184,9 +264,8 @@ export function grantManagedCodexHookTrust( return fallback(plan, 'no-managed-entries') } const expected = buildExpectedEntries(plan) - const resolvedHost = resolveCodexTrustGrantHost(plan.host) - const currentStamp = resolvedHost.binaryStamp - const ledgerEntries = findLedgerGrant(plan, expected, currentStamp) + const resolvedHost = await resolveCodexTrustGrantHost(plan.host) + const ledgerEntries = findLedgerGrant(plan, expected, resolvedHost.binaryStamp) if (ledgerEntries !== null) { diagnostics.ledgerHits += 1 return { lane: 'rpc', entries: ledgerEntries } @@ -207,131 +286,17 @@ export function grantManagedCodexHookTrust( } transientRetryAfterByHost.delete(hostKey) } - - const startedAtMs = Date.now() - // Why: the RPC may rewrite config.toml before a later RPC fails. Restore - // its exact pre-session bytes before the legacy lane runs so every fallback - // has the same input and output as the pre-RPC implementation. - const configSnapshot = captureCodexTrustConfig(plan.tomlPath) - let result: CodexHookTrustGrantSessionResult - try { - // Why: Windows fallback writes equivalent separator variants that Codex's - // canonical RPC key may not overwrite, leaving conflicting logical trust. - removeSelfComputedTrustBeforeGrant(plan) - result = runSessionSync( - resolvedHost.buildRequest({ - runtimeHomePath: plan.runtimeHomePath, - managedCommand: plan.managedCommand, - expectedTrustKeys: expected.map(({ normalizedKey }) => normalizedKey), - useDefaultCodexHome: plan.useDefaultCodexHome - }) - ) - } catch (error) { - restoreCodexTrustConfig(plan.tomlPath, configSnapshot) - if (isCodexAppServerUnsupportedError(error)) { - transientRetryAfterByHost.delete(hostKey) - codexAppServerCapabilityCache.rememberUnsupported(hostKey) - return fallback(plan, 'unsupported', error) - } - transientRetryAfterByHost.set( - hostKey, - Date.now() + CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS - ) - return fallback(plan, 'error', error) - } - // Why: the RPC surface answered, even if our entries were not verifiable — - // remember support so a later drift event retries the preferred lane. - codexAppServerCapabilityCache.rememberSupported(hostKey) - if (result.outcome === 'verify-failed') { - restoreCodexTrustConfig(plan.tomlPath, configSnapshot) - transientRetryAfterByHost.set( - hostKey, - Date.now() + CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS - ) - return fallback(plan, 'verify-failed', result.reason, result.reasonClass) - } - - const byNormalizedKey = new Map(expected.map((item) => [item.normalizedKey, item])) - const seenNormalizedKeys = new Set() - const grantedEntries: CodexTrustEntry[] = [] - const ledgerRecord: Record = {} - for (const granted of result.entries) { - const match = byNormalizedKey.get(granted.normalizedKey) - if (!match) { - restoreCodexTrustConfig(plan.tomlPath, configSnapshot) - transientRetryAfterByHost.set( - hostKey, - Date.now() + CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS - ) - return fallback( - plan, - 'verify-failed', - `unexpected granted key ${granted.key}`, - 'unexpected-key' - ) - } - if (seenNormalizedKeys.has(granted.normalizedKey)) { - restoreCodexTrustConfig(plan.tomlPath, configSnapshot) - transientRetryAfterByHost.set( - hostKey, - Date.now() + CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS - ) - return fallback( - plan, - 'verify-failed', - `duplicate granted key ${granted.key}`, - 'duplicate-key' - ) - } - seenNormalizedKeys.add(granted.normalizedKey) - grantedEntries.push({ ...match.entry, trustedHash: granted.trustedHash }) - ledgerRecord[granted.normalizedKey] = { - signature: match.signature, - trustedHash: granted.trustedHash - } - } - if (seenNormalizedKeys.size !== expected.length) { - restoreCodexTrustConfig(plan.tomlPath, configSnapshot) - transientRetryAfterByHost.set( - hostKey, - Date.now() + CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS - ) - return fallback( - plan, - 'verify-failed', - 'granted entry set did not cover expected entries', - 'coverage' - ) - } - transientRetryAfterByHost.delete(hostKey) - try { - writeCodexTrustGrantLedgerHome(plan.runtimeHomePath, { - binary: currentStamp, - entries: ledgerRecord - }) - } catch (error) { - // Why: a ledger write failure only costs an extra session next launch. - console.warn('[codex-trust-grant] failed to persist grant ledger', error) - } - diagnostics.granted += 1 - console.log( - `[codex-trust-grant] granted ${grantedEntries.length} managed hook entries via codex app-server ` + - `(host=${plan.host.kind}, wrote=${result.wroteTrust}, ${Date.now() - startedAtMs}ms)` + return await runExclusivelyForCodexTrustConfig(plan.tomlPath, () => + runGrantAttempt(plan, expected, resolvedHost, hostKey) ) - emitCodexTrustGrantTelemetry({ - outcome: 'granted', - hostKind: plan.host.kind, - lane: plan.telemetryLane - }) - return { lane: 'rpc', entries: grantedEntries } } catch (error) { return fallback(plan, 'error', error) } } export const _internals = { - setGrantSessionRunnerSync(runner: GrantSessionRunnerSync | null): void { - runSessionSync = runner ?? runCodexHookTrustGrantSessionSync + setGrantSessionRunner(runner: GrantSessionRunner | null): void { + runSession = runner ?? runCodexHookTrustGrantSession }, resetDiagnostics(): void { diagnostics.granted = 0 diff --git a/src/main/codex/codex-managed-trust-grant-plan.ts b/src/main/codex/codex-managed-trust-grant-plan.ts new file mode 100644 index 00000000000..e816b0cbff5 --- /dev/null +++ b/src/main/codex/codex-managed-trust-grant-plan.ts @@ -0,0 +1,90 @@ +import type { CodexTrustGrantTelemetryLane } from './codex-trust-grant-telemetry' +import { + readCodexTrustGrantLedgerHomeMatchingStamp, + type CodexTrustGrantHost +} from './codex-trust-grant-host' +import type { CodexTrustGrantBinaryStamp } from './codex-trust-grant-ledger' +import { getCodexHookTrustSignature } from './codex-hook-identity' +import { + computeTrustKey, + computeTrustedHash, + normalizeHookTrustKeyForLookup, + readHookTrustEntries, + removeHookTrustEntries, + type CodexTrustEntry +} from './config-toml-trust' + +export type CodexManagedTrustGrantPlan = { + /** Host-visible runtime home path (UNC for WSL) — ledger key + config reads. */ + runtimeHomePath: string + /** Host-visible config.toml path holding the trust entries. */ + tomlPath: string + /** Exact command string written to the managed hooks.json entries. */ + managedCommand: string + /** Managed trust identities Orca just wrote (no trustedHash). */ + managedEntries: readonly CodexTrustEntry[] + host: CodexTrustGrantHost + telemetryLane: CodexTrustGrantTelemetryLane + /** Match a pane where CODEX_HOME is absent instead of an explicit managed home. */ + useDefaultCodexHome?: boolean +} + +export type ExpectedManagedEntry = { + entry: CodexTrustEntry + normalizedKey: string + signature: string +} + +export function buildExpectedEntries(plan: CodexManagedTrustGrantPlan): ExpectedManagedEntry[] { + return plan.managedEntries.map((entry) => ({ + entry, + normalizedKey: normalizeHookTrustKeyForLookup(computeTrustKey(entry)), + signature: getCodexHookTrustSignature(entry) + })) +} + +/** Windows fallback writes equivalent separator variants that Codex's canonical + * RPC key may not overwrite, leaving conflicting logical trust behind. */ +export function removeSelfComputedTrustBeforeGrant(plan: CodexManagedTrustGrantPlan): void { + const trustStates = readHookTrustEntries(plan.tomlPath) + const ownedKeys = plan.managedEntries + .map((entry) => { + const key = computeTrustKey(entry) + return trustStates.get(key)?.trustedHash === computeTrustedHash(entry) ? key : null + }) + .filter((key): key is string => key !== null) + if (ownedKeys.length > 0) { + removeHookTrustEntries(plan.tomlPath, ownedKeys) + } +} + +/** Entries a prior grant already recorded for this exact binary and config + * state, or null when the RPC session has to run again. */ +export function findLedgerGrant( + plan: CodexManagedTrustGrantPlan, + expected: ExpectedManagedEntry[], + currentStamp: CodexTrustGrantBinaryStamp | null +): CodexTrustEntry[] | null { + const home = readCodexTrustGrantLedgerHomeMatchingStamp(plan.runtimeHomePath, currentStamp) + if (!home) { + return null + } + let trustStates: ReturnType + try { + trustStates = readHookTrustEntries(plan.tomlPath) + } catch { + return null + } + const entries: CodexTrustEntry[] = [] + for (const { entry, normalizedKey, signature } of expected) { + const recorded = home.entries[normalizedKey] + if (!recorded || recorded.signature !== signature) { + return null + } + if (trustStates.get(normalizedKey)?.trustedHash !== recorded.trustedHash) { + return null + } + entries.push({ ...entry, trustedHash: recorded.trustedHash }) + } + return entries +} diff --git a/src/main/codex/codex-real-home-hook-install.test.ts b/src/main/codex/codex-real-home-hook-install.test.ts index b108a0b5250..ed0a184163c 100644 --- a/src/main/codex/codex-real-home-hook-install.test.ts +++ b/src/main/codex/codex-real-home-hook-install.test.ts @@ -87,7 +87,7 @@ beforeEach(() => { }) afterEach(() => { - rebaseInternals.setSessionRunnerSync(null) + rebaseInternals.setSessionRunner(null) rebaseInternals.resetRetryState() rmSync(fakeHomeDir, { recursive: true, force: true }) rmSync(userDataDir, { recursive: true, force: true }) @@ -100,10 +100,30 @@ afterEach(() => { }) describe('ensureRealHomeCodexHookState (install)', () => { - it('creates hooks.json with the Orca entry in every managed event for a fresh home', () => { + // Why (#16441): the ensure chain is process-wide; a rejection that escapes it + // would return the same rejected promise to every later pane launch, with no + // retry and no cooldown recovery. + it('recovers from a home-resolution failure instead of poisoning later ensures', async () => { + grantSucceeds() + homedirMock.mockImplementationOnce(() => { + throw new Error('home unavailable') + }) + + await expect( + ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + ).resolves.toBe('unavailable') + await expect( + ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir }) + ).resolves.toBe('removed') + }) + + it('creates hooks.json with the Orca entry in every managed event for a fresh home', async () => { grantSucceeds() - const lane = ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + const lane = await ensureRealHomeCodexHookState({ + hooksEnabled: true, + userDataPath: userDataDir + }) expect(lane).toBe('installed') const material = getCodexManagedHookInstallMaterial() @@ -121,7 +141,7 @@ describe('ensureRealHomeCodexHookState (install)', () => { expect(plan.managedEntries.every((entry) => entry.groupIndex === 0)).toBe(true) }) - it('keeps a symlinked default home logical in the keys sent to Codex', () => { + it('keeps a symlinked default home logical in the keys sent to Codex', async () => { grantSucceeds() const logicalHome = join(fakeHomeDir, '.codex') const targetHome = join(fakeHomeDir, 'dotfiles-codex') @@ -129,9 +149,9 @@ describe('ensureRealHomeCodexHookState (install)', () => { mkdirSync(targetHome) symlinkSync(targetHome, logicalHome, process.platform === 'win32' ? 'junction' : 'dir') - expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( - 'installed' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + ).toBe('installed') const plan = grantMock.mock.calls[0]![0] as CodexManagedTrustGrantPlan expect( @@ -139,7 +159,7 @@ describe('ensureRealHomeCodexHookState (install)', () => { ).toBe(true) }) - it('keeps the managed lane for unknown top-level fields Codex cannot load', () => { + it('keeps the managed lane for unknown top-level fields Codex cannot load', async () => { grantSucceeds() const userConfig = { hooks: { @@ -151,7 +171,10 @@ describe('ensureRealHomeCodexHookState (install)', () => { const original = `${JSON.stringify(userConfig, null, 2)}\n` writeFileSync(getRealHooksJsonPath(), original, 'utf-8') - const lane = ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + const lane = await ensureRealHomeCodexHookState({ + hooksEnabled: true, + userDataPath: userDataDir + }) expect(lane).toBe('unavailable') expect(readFileSync(getRealHooksJsonPath(), 'utf-8')).toBe(original) @@ -161,7 +184,7 @@ describe('ensureRealHomeCodexHookState (install)', () => { ) }) - it('appends LAST and preserves user entries and trust positions', () => { + it('appends LAST and preserves user entries and trust positions', async () => { grantSucceeds() const userConfig = { hooks: { @@ -172,9 +195,9 @@ describe('ensureRealHomeCodexHookState (install)', () => { const original = `${JSON.stringify(userConfig, null, 2)}\n` writeFileSync(getRealHooksJsonPath(), original, 'utf-8') - expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( - 'installed' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + ).toBe('installed') const config = readRealHooksJson() expect(config.hooks?.Stop).toHaveLength(2) @@ -190,7 +213,7 @@ describe('ensureRealHomeCodexHookState (install)', () => { // Why: ordinary Windows CI tokens cannot create file symlinks without Developer Mode. it.skipIf(process.platform === 'win32')( 'updates a symlinked hooks.json target without replacing the symlink', - () => { + async () => { grantSucceeds() const dotfilesDir = join(fakeHomeDir, 'dotfiles') const targetPath = join(dotfilesDir, 'hooks.json') @@ -202,78 +225,87 @@ describe('ensureRealHomeCodexHookState (install)', () => { ) symlinkSync(targetPath, getRealHooksJsonPath()) - expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( - 'installed' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + ).toBe('installed') expect(lstatSync(getRealHooksJsonPath()).isSymbolicLink()).toBe(true) expect(JSON.parse(readFileSync(targetPath, 'utf-8')).hooks.Stop).toHaveLength(2) } ) - it('keeps the managed lane and original bytes when the pristine backup cannot be created', () => { + it('keeps the managed lane and original bytes when the pristine backup cannot be created', async () => { grantSucceeds() const original = `${JSON.stringify({ hooks: { Stop: [] } }, null, 2)}\n` writeFileSync(getRealHooksJsonPath(), original, 'utf-8') writeFileSync(join(userDataDir, 'codex-real-home-hooks'), 'blocks backup directory', 'utf-8') - expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( - 'unavailable' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + ).toBe('unavailable') expect(readFileSync(getRealHooksJsonPath(), 'utf-8')).toBe(original) expect(grantMock).not.toHaveBeenCalled() }) - it.skipIf(process.platform === 'win32')('preserves restrictive hooks.json permissions', () => { - grantSucceeds() - writeFileSync(getRealHooksJsonPath(), '{ "hooks": {} }\n', 'utf-8') - chmodSync(getRealHooksJsonPath(), 0o600) - - expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( - 'installed' - ) - - expect(statSync(getRealHooksJsonPath()).mode & 0o777).toBe(0o600) - }) - it.skipIf(process.platform === 'win32')( - 'restores restrictive hooks.json permissions after grant fallback', - () => { - grantUnavailable() + 'preserves restrictive hooks.json permissions', + async () => { + grantSucceeds() writeFileSync(getRealHooksJsonPath(), '{ "hooks": {} }\n', 'utf-8') chmodSync(getRealHooksJsonPath(), 0o600) - expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( - 'unavailable' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + ).toBe('installed') expect(statSync(getRealHooksJsonPath()).mode & 0o777).toBe(0o600) } ) - it('rolls the file back byte-exactly when the grant lane is unavailable', () => { + it.skipIf(process.platform === 'win32')( + 'restores restrictive hooks.json permissions after grant fallback', + async () => { + grantUnavailable() + writeFileSync(getRealHooksJsonPath(), '{ "hooks": {} }\n', 'utf-8') + chmodSync(getRealHooksJsonPath(), 0o600) + + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + ).toBe('unavailable') + + expect(statSync(getRealHooksJsonPath()).mode & 0o777).toBe(0o600) + } + ) + + it('rolls the file back byte-exactly when the grant lane is unavailable', async () => { grantUnavailable() const userRaw = `${JSON.stringify({ hooks: { Stop: [{ hooks: [{ type: 'command', command: 'mine.sh' }] }] } }, null, 2)}\n` writeFileSync(getRealHooksJsonPath(), userRaw, 'utf-8') - const lane = ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + const lane = await ensureRealHomeCodexHookState({ + hooksEnabled: true, + userDataPath: userDataDir + }) expect(lane).toBe('unavailable') expect(getRealHomeCodexHookLane()).toBe('unavailable') expect(readFileSync(getRealHooksJsonPath(), 'utf-8')).toBe(userRaw) }) - it('removes a freshly created hooks.json when the grant lane is unavailable', () => { + it('removes a freshly created hooks.json when the grant lane is unavailable', async () => { grantUnavailable() - const lane = ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + const lane = await ensureRealHomeCodexHookState({ + hooksEnabled: true, + userDataPath: userDataDir + }) expect(lane).toBe('unavailable') expect(existsSync(getRealHooksJsonPath())).toBe(false) }) - it('surfaces rollback failures to the retry boundary', () => { + it('surfaces rollback failures to the retry boundary', async () => { const warning = vi.spyOn(console, 'warn').mockImplementation(() => {}) grantMock.mockImplementation(() => { rmSync(getRealHooksJsonPath()) @@ -281,9 +313,9 @@ describe('ensureRealHomeCodexHookState (install)', () => { return { lane: 'fallback', reason: 'unsupported' } }) - expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( - 'unavailable' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + ).toBe('unavailable') expect(warning).toHaveBeenCalledWith( '[codex-real-home-hooks] ensure failed; staying on managed lane:', @@ -291,43 +323,49 @@ describe('ensureRealHomeCodexHookState (install)', () => { ) }) - it('does no hook-file or grant work on repeated unsupported launches', () => { + it('does no hook-file or grant work on repeated unsupported launches', async () => { grantUnavailable() - expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( - 'unavailable' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + ).toBe('unavailable') expect(existsSync(getRealHooksJsonPath())).toBe(false) - expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( - 'unavailable' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + ).toBe('unavailable') expect(grantMock).toHaveBeenCalledTimes(1) expect(existsSync(getRealHooksJsonPath())).toBe(false) }) - it('leaves an unparseable hooks.json untouched and keeps the managed lane', () => { + it('leaves an unparseable hooks.json untouched and keeps the managed lane', async () => { writeFileSync(getRealHooksJsonPath(), '{not json', 'utf-8') - const lane = ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + const lane = await ensureRealHomeCodexHookState({ + hooksEnabled: true, + userDataPath: userDataDir + }) expect(lane).toBe('unavailable') expect(readFileSync(getRealHooksJsonPath(), 'utf-8')).toBe('{not json') expect(grantMock).not.toHaveBeenCalled() }) - it('is idempotent: a second ensure keeps a single appended entry per event', () => { + it('is idempotent: a second ensure keeps a single appended entry per event', async () => { grantSucceeds() - ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) const firstRaw = readFileSync(getRealHooksJsonPath(), 'utf-8') - const lane = ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + const lane = await ensureRealHomeCodexHookState({ + hooksEnabled: true, + userDataPath: userDataDir + }) expect(lane).toBe('installed') expect(readFileSync(getRealHooksJsonPath(), 'utf-8')).toBe(firstRaw) }) - it('keeps later user hook trust positions stable when reconciling an existing install', () => { + it('keeps later user hook trust positions stable when reconciling an existing install', async () => { grantSucceeds() const userBefore = { hooks: [{ type: 'command', command: 'before.sh' }] } writeFileSync( @@ -335,15 +373,15 @@ describe('ensureRealHomeCodexHookState (install)', () => { `${JSON.stringify({ hooks: { Stop: [userBefore] } }, null, 2)}\n`, 'utf-8' ) - ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) const installed = readRealHooksJson() const userAfter = { hooks: [{ type: 'command', command: 'after.sh' }] } installed.hooks!.Stop!.push(userAfter) writeFileSync(getRealHooksJsonPath(), `${JSON.stringify(installed, null, 2)}\n`, 'utf-8') - expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( - 'installed' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + ).toBe('installed') const reconciled = readRealHooksJson().hooks?.Stop expect(reconciled?.[0]).toEqual(userBefore) @@ -352,17 +390,17 @@ describe('ensureRealHomeCodexHookState (install)', () => { expect(plan.managedEntries.find((entry) => entry.eventLabel === 'stop')?.groupIndex).toBe(1) }) - it("keeps later user handler trust positions stable inside Orca's hook group", () => { + it("keeps later user handler trust positions stable inside Orca's hook group", async () => { grantSucceeds() - ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) const installed = readRealHooksJson() const userAfter = { type: 'command', command: 'after.sh' } installed.hooks!.Stop![0]!.hooks!.push(userAfter) writeFileSync(getRealHooksJsonPath(), `${JSON.stringify(installed, null, 2)}\n`, 'utf-8') - expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( - 'installed' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + ).toBe('installed') expect(readRealHooksJson().hooks?.Stop?.[0]?.hooks?.[1]).toEqual(userAfter) const plan = grantMock.mock.calls.at(-1)![0] as CodexManagedTrustGrantPlan @@ -372,37 +410,37 @@ describe('ensureRealHomeCodexHookState (install)', () => { }) describe('ensureRealHomeCodexHookState (opt-out sweep)', () => { - it('keeps the managed lane when hooks.json cannot be read', () => { + it('keeps the managed lane when hooks.json cannot be read', async () => { mkdirSync(getRealHooksJsonPath()) - expect(ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir })).toBe( - 'unavailable' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir }) + ).toBe('unavailable') }) - it('keeps the managed lane when hooks.json is malformed', () => { + it('keeps the managed lane when hooks.json is malformed', async () => { writeFileSync(getRealHooksJsonPath(), '{ not json', 'utf-8') - expect(ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir })).toBe( - 'unavailable' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir }) + ).toBe('unavailable') expect(readFileSync(getRealHooksJsonPath(), 'utf-8')).toBe('{ not json') }) - it('rebases trust when a user appended hooks after Orca installed', () => { + it('rebases trust when a user appended hooks after Orca installed', async () => { grantSucceeds() const before = { type: 'command', command: 'before.sh' } writeFileSync( getRealHooksJsonPath(), `${JSON.stringify({ hooks: { Stop: [{ hooks: [before] }] } }, null, 2)}\n` ) - ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) const installed = readRealHooksJson() const after = { type: 'command', command: 'after.sh' } installed.hooks!.Stop!.push({ hooks: [after] }) writeFileSync(getRealHooksJsonPath(), `${JSON.stringify(installed, null, 2)}\n`) const operations: string[] = [] - rebaseInternals.setSessionRunnerSync((request) => { + rebaseInternals.setSessionRunner(async (request) => { operations.push(request.operation) if (request.operation === 'inspect-user-hook-trust') { expect(readRealHooksJson().hooks?.Stop?.[2]?.hooks?.[0]?.command).toBe('after.sh') @@ -420,21 +458,21 @@ describe('ensureRealHomeCodexHookState (opt-out sweep)', () => { return { outcome: 'repaired', repaired: 1 } }) - expect(ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir })).toBe( - 'removed' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir }) + ).toBe('removed') expect(operations).toEqual(['inspect-user-hook-trust', 'repair-user-hook-trust']) expect(readRealHooksJson().hooks?.Stop).toEqual([{ hooks: [before] }, { hooks: [after] }]) }) - it('aborts without writing when hooks.json changes during the trust inspection', () => { + it('aborts without writing when hooks.json changes during the trust inspection', async () => { grantSucceeds() const before = { type: 'command', command: 'before.sh' } writeFileSync( getRealHooksJsonPath(), `${JSON.stringify({ hooks: { Stop: [{ hooks: [before] }] } }, null, 2)}\n` ) - ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) const installed = readRealHooksJson() const after = { type: 'command', command: 'after.sh' } installed.hooks!.Stop!.push({ hooks: [after] }) @@ -443,7 +481,7 @@ describe('ensureRealHomeCodexHookState (opt-out sweep)', () => { writeFileSync(getRealConfigTomlPath(), userTrustToml, 'utf-8') const concurrentSave = `${JSON.stringify({ hooks: { Stop: [{ hooks: [before] }] } }, null, 2)}\n` const operations: string[] = [] - rebaseInternals.setSessionRunnerSync((request) => { + rebaseInternals.setSessionRunner(async (request) => { operations.push(request.operation) // A user save (or a second Orca instance) lands while the RPC runs. writeFileSync(getRealHooksJsonPath(), concurrentSave, 'utf-8') @@ -458,16 +496,16 @@ describe('ensureRealHomeCodexHookState (opt-out sweep)', () => { } }) - expect(ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir })).toBe( - 'unavailable' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir }) + ).toBe('unavailable') expect(operations).toEqual(['inspect-user-hook-trust']) expect(readFileSync(getRealHooksJsonPath(), 'utf-8')).toBe(concurrentSave) expect(readFileSync(getRealConfigTomlPath(), 'utf-8')).toBe(userTrustToml) }) - it('removes only Orca entries and reports the removed lane', () => { + it('removes only Orca entries and reports the removed lane', async () => { grantSucceeds() const userStop = { matcher: 'deploy-*', @@ -478,10 +516,13 @@ describe('ensureRealHomeCodexHookState (opt-out sweep)', () => { `${JSON.stringify({ hooks: { Stop: [userStop] } }, null, 2)}\n`, 'utf-8' ) - ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + await ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) expect(readRealHooksJson().hooks?.Stop).toHaveLength(2) - const lane = ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir }) + const lane = await ensureRealHomeCodexHookState({ + hooksEnabled: false, + userDataPath: userDataDir + }) expect(lane).toBe('removed') const config = readRealHooksJson() @@ -495,14 +536,17 @@ describe('ensureRealHomeCodexHookState (opt-out sweep)', () => { } }) - it('no-ops the sweep when the real home has no hooks.json', () => { - const lane = ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir }) + it('no-ops the sweep when the real home has no hooks.json', async () => { + const lane = await ensureRealHomeCodexHookState({ + hooksEnabled: false, + userDataPath: userDataDir + }) expect(lane).toBe('removed') expect(existsSync(getRealHooksJsonPath())).toBe(false) }) - it('removes only hash-proven Orca trust from a mixed hook group', () => { + it('removes only hash-proven Orca trust from a mixed hook group', async () => { const material = getCodexManagedHookInstallMaterial() const userCommand = 'my-user-hook.sh' writeFileSync( @@ -544,9 +588,9 @@ describe('ensureRealHomeCodexHookState (opt-out sweep)', () => { ] writeFileSync(getRealConfigTomlPath(), upsertHookTrustEntriesInContent('', entries), 'utf-8') - expect(ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir })).toBe( - 'removed' - ) + expect( + await ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir }) + ).toBe('removed') expect(readRealHooksJson().hooks?.Stop).toEqual([ { hooks: [{ type: 'command', command: userCommand }] } diff --git a/src/main/codex/codex-real-home-hook-install.ts b/src/main/codex/codex-real-home-hook-install.ts index 678526e4e60..f5ab1b279c7 100644 --- a/src/main/codex/codex-real-home-hook-install.ts +++ b/src/main/codex/codex-real-home-hook-install.ts @@ -1,8 +1,5 @@ -import { existsSync, mkdirSync, readFileSync, statSync, unlinkSync } from 'node:fs' -import { join } from 'node:path' -import { writeFileAtomically } from '../codex-accounts/fs-utils' +import { statSync } from 'node:fs' import { - buildManagedCommandHook, createManagedCommandMatcher, MANAGED_HOOK_TIMEOUT_SECONDS, readHooksJsonWithRaw, @@ -13,6 +10,14 @@ import { type HooksConfig } from '../agent-hooks/installer-utils' import { resolveHooksJsonWritePath } from '../agent-hooks/hook-config-write-path' +import { + assertHooksJsonGeneration, + backupRealHomeHooksJsonOnce, + getRealHomeConfigTomlPath, + getRealHomeHooksJsonPath, + reconcileManagedHookDefinition, + restoreRealHomeHooksJson +} from './codex-real-home-hooks-json' import { getCodexManagedScriptFileName } from './codex-hook-identity' import { CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS, @@ -25,6 +30,7 @@ import { getSystemCodexHomePath } from './codex-home-paths' import type { CodexTrustEntry } from './config-toml-trust' import { restoreCodexTrustConfig } from './codex-trust-config-rollback' import { mutateRealHomeHooksPreservingUserTrust } from './codex-user-hook-trust-rebase' +import { runExclusivelyForCodexTrustConfig } from './codex-trust-config-mutation-queue' /** * Real-home Codex hook lane for the system-default selection (flag ON). @@ -42,6 +48,7 @@ export type RealHomeCodexHookLane = 'pending' | 'installed' | 'unavailable' | 'r let currentLane: RealHomeCodexHookLane = 'pending' let installRetryAfterMs = 0 +let ensureInFlight: Promise = Promise.resolve(currentLane) export function getRealHomeCodexHookLane(): RealHomeCodexHookLane { return currentLane @@ -56,52 +63,43 @@ export function isRealHomeCodexHookLaneUsable(): boolean { return currentLane !== 'unavailable' } -function getRealHomeHooksJsonPath(): string { - return join(getSystemCodexHomePath(), 'hooks.json') -} - -function getRealHomeConfigTomlPath(): string { - return join(getSystemCodexHomePath(), 'config.toml') -} - -/** Orca-side state dir; nothing extra is ever written into the user's ~/.codex. */ -function getRealHomeHookStateDir(userDataPath: string): string { - return join(userDataPath, 'codex-real-home-hooks') -} - -function assertHooksJsonGeneration( - hooksJsonPath: string, - hooksWritePath: string, - expectedRaw: string | null -): void { - const currentRaw = existsSync(hooksJsonPath) ? readFileSync(hooksJsonPath, 'utf-8') : null - if (currentRaw !== expectedRaw || resolveHooksJsonWritePath(hooksJsonPath) !== hooksWritePath) { - // Why: the pre-mutation RPC can overlap a user's editor save. Abort rather - // than atomically replacing a newer file with the stale parsed snapshot. - throw new Error('Codex hooks.json changed while Orca prepared its trust repair') - } -} - /** * Ensures the real-home hook state matches the settings: installs and trusts - * the Orca status hook when enabled, sweeps it when opted out. Idempotent and - * synchronous (launch prep); repeat calls are cheap — an unchanged hooks.json - * write no-ops and a valid grant ledger skips the RPC session entirely. + * the Orca status hook when enabled, sweeps it when opted out. Idempotent; + * repeat calls are cheap — an unchanged hooks.json write no-ops and a valid + * grant ledger skips the RPC session entirely. * Never throws: any failure logs and leaves the host on the managed lane. */ export function ensureRealHomeCodexHookState(args: { hooksEnabled: boolean userDataPath: string -}): RealHomeCodexHookLane { +}): Promise { // Why: the grant client caches failed probes, but mutating and rolling back - // hooks.json before consulting it still adds synchronous work to every pane. + // hooks.json before consulting it still adds work to every pane launch. if (args.hooksEnabled && currentLane === 'unavailable' && Date.now() < installRetryAfterMs) { - return currentLane + return Promise.resolve(currentLane) } + // Why: this mutates the user's real ~/.codex and the module's lane state. + // Concurrent pane launches must not interleave two of them, and the shared + // config.toml lane keeps the rebase + grant pair atomic against the managed + // installer's legacy sweep of the same file. + const run = (): Promise => runRealHomeCodexHookEnsure(args) + // Why both handlers: a rejected predecessor must not poison every later + // ensure for the process' lifetime. + ensureInFlight = ensureInFlight.then(run, run) + return ensureInFlight +} + +async function runRealHomeCodexHookEnsure(args: { + hooksEnabled: boolean + userDataPath: string +}): Promise { try { - currentLane = args.hooksEnabled - ? installRealHomeCodexHook(args.userDataPath) - : sweepRealHomeCodexHook() + // Why inside the try: resolving the real home can throw too, and this + // function is the module's "never throws" boundary. + currentLane = await runExclusivelyForCodexTrustConfig(getRealHomeConfigTomlPath(), () => + args.hooksEnabled ? installRealHomeCodexHook(args.userDataPath) : sweepRealHomeCodexHook() + ) if (!args.hooksEnabled || currentLane === 'installed') { installRetryAfterMs = 0 } @@ -115,7 +113,7 @@ export function ensureRealHomeCodexHookState(args: { return currentLane } -function installRealHomeCodexHook(userDataPath: string): RealHomeCodexHookLane { +async function installRealHomeCodexHook(userDataPath: string): Promise { const material = getCodexManagedHookInstallMaterial() const hooksJsonPath = getRealHomeHooksJsonPath() const hooksWritePath = resolveHooksJsonWritePath(hooksJsonPath) @@ -175,7 +173,7 @@ function installRealHomeCodexHook(userDataPath: string): RealHomeCodexHookLane { backupRealHomeHooksJsonOnce(userDataPath, previousRaw) // Why: unknown top-level fields belong to the user (other managers' // metadata); unlike the managed-home writer, preserve them verbatim. - const trustConfigSnapshot = mutateRealHomeHooksPreservingUserTrust({ + const trustConfigSnapshot = await mutateRealHomeHooksPreservingUserTrust({ sourcePath: hooksJsonPath, runtimeHomePath: getSystemCodexHomePath(), tomlPath: getRealHomeConfigTomlPath(), @@ -190,7 +188,7 @@ function installRealHomeCodexHook(userDataPath: string): RealHomeCodexHookLane { restoreHooks: () => restoreRealHomeHooksJson(hooksWritePath, previousRaw, previousMode) }) - const grant = grantManagedCodexHookTrust({ + const grant = await grantManagedCodexHookTrust({ runtimeHomePath: getSystemCodexHomePath(), tomlPath: getRealHomeConfigTomlPath(), managedCommand: material.command, @@ -223,53 +221,13 @@ function installRealHomeCodexHook(userDataPath: string): RealHomeCodexHookLane { return 'unavailable' } -function reconcileManagedHookDefinition( - current: HookDefinition[], - isManagedCommand: (command: string | undefined) => boolean, - command: string -): { definitions: HookDefinition[]; groupIndex: number; handlerIndex: number } { - const directCommandKeys = ['command', 'bash', 'powershell'] as const - const hasManagedDirectCommand = current.some((definition) => - directCommandKeys.some((key) => isManagedCommand(definition[key])) - ) - const nestedLocations = current.flatMap((definition, groupIndex) => - Array.isArray(definition.hooks) - ? definition.hooks.flatMap((hook, handlerIndex) => - isManagedCommand(hook.command) ? [{ groupIndex, handlerIndex }] : [] - ) - : [] - ) - if (!hasManagedDirectCommand && nestedLocations.length === 1) { - const { groupIndex, handlerIndex } = nestedLocations[0]! - const definition = current[groupIndex]! - const hasDirectCommand = directCommandKeys.some((key) => typeof definition[key] === 'string') - if (definition.matcher === undefined && !hasDirectCommand) { - const definitions = [...current] - // Why: users can append groups or handlers after Orca's first install. - // Reusing the exact slot preserves all later positional trust keys. - const hooks = [...definition.hooks!] - hooks[handlerIndex] = buildManagedCommandHook(command) - definitions[groupIndex] = { ...definition, hooks } - return { definitions, groupIndex, handlerIndex } - } - } - - const cleaned = removeManagedCommands(current, isManagedCommand) - // Why: first install appends LAST so no existing user trust position shifts. - return { - definitions: [...cleaned, { hooks: [buildManagedCommandHook(command)] }], - groupIndex: cleaned.length, - handlerIndex: 0 - } -} - function getInstallRetryAfterMs(reason: CodexTrustGrantFallbackReason): number { return reason === 'unsupported' || reason === 'unsupported-cached' || reason === 'disabled' ? Number.POSITIVE_INFINITY : Date.now() + CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS } -function sweepRealHomeCodexHook(): RealHomeCodexHookLane { +async function sweepRealHomeCodexHook(): Promise { const hooksJsonPath = getRealHomeHooksJsonPath() // Why: single read — the pre-write generation guard must compare against // the exact bytes this sweep's parse came from. @@ -306,7 +264,7 @@ function sweepRealHomeCodexHook(): RealHomeCodexHookLane { if (removedAny) { const hooksWritePath = resolveHooksJsonWritePath(hooksJsonPath) const previousMode = statSync(hooksWritePath).mode - mutateRealHomeHooksPreservingUserTrust({ + await mutateRealHomeHooksPreservingUserTrust({ sourcePath: hooksJsonPath, runtimeHomePath: getSystemCodexHomePath(), tomlPath: getRealHomeConfigTomlPath(), @@ -345,41 +303,10 @@ function sweepRealHomeCodexHook(): RealHomeCodexHookLane { return 'removed' } -/** One-time pristine copy of the user's file, kept under Orca's userData. */ -function backupRealHomeHooksJsonOnce(userDataPath: string, previousRaw: string | null): void { - if (previousRaw === null) { - return - } - const backupDir = getRealHomeHookStateDir(userDataPath) - const backupPath = join(backupDir, 'hooks.json.pre-orca') - if (existsSync(backupPath)) { - return - } - // Why: this lane mutates the user's real Codex home. If the required - // pristine recovery copy cannot be created, keep the managed lane intact. - mkdirSync(backupDir, { recursive: true }) - writeFileAtomically(backupPath, previousRaw, { mode: 0o600 }) -} - -function restoreRealHomeHooksJson( - hooksJsonPath: string, - previousRaw: string | null, - previousMode?: number -): void { - if (previousRaw === null) { - if (existsSync(hooksJsonPath)) { - unlinkSync(hooksJsonPath) - } - return - } - // Why: rollback is part of the safety boundary. Use the shared atomic - // writer so Windows file-lock retries and failed-temp cleanup are covered. - writeFileAtomically(hooksJsonPath, previousRaw, { mode: previousMode }) -} - export const _internals = { setLaneForTesting(lane: RealHomeCodexHookLane): void { currentLane = lane installRetryAfterMs = 0 + ensureInFlight = Promise.resolve(lane) } } diff --git a/src/main/codex/codex-real-home-hooks-json.ts b/src/main/codex/codex-real-home-hooks-json.ts new file mode 100644 index 00000000000..0b9f63003bb --- /dev/null +++ b/src/main/codex/codex-real-home-hooks-json.ts @@ -0,0 +1,115 @@ +import { existsSync, mkdirSync, readFileSync, unlinkSync } from 'node:fs' +import { join } from 'node:path' +import { writeFileAtomically } from '../codex-accounts/fs-utils' +import { + buildManagedCommandHook, + removeManagedCommands, + type HookDefinition +} from '../agent-hooks/installer-utils' +import { resolveHooksJsonWritePath } from '../agent-hooks/hook-config-write-path' +import { getSystemCodexHomePath } from './codex-home-paths' + +/** The user's real `~/.codex` hook files, plus the guards and rollback the + * real-home lane needs before it is allowed to mutate them. */ +export function getRealHomeHooksJsonPath(): string { + return join(getSystemCodexHomePath(), 'hooks.json') +} + +export function getRealHomeConfigTomlPath(): string { + return join(getSystemCodexHomePath(), 'config.toml') +} + +/** Orca-side state dir; nothing extra is ever written into the user's ~/.codex. */ +function getRealHomeHookStateDir(userDataPath: string): string { + return join(userDataPath, 'codex-real-home-hooks') +} + +export function assertHooksJsonGeneration( + hooksJsonPath: string, + hooksWritePath: string, + expectedRaw: string | null +): void { + const currentRaw = existsSync(hooksJsonPath) ? readFileSync(hooksJsonPath, 'utf-8') : null + if (currentRaw !== expectedRaw || resolveHooksJsonWritePath(hooksJsonPath) !== hooksWritePath) { + // Why: the pre-mutation RPC can overlap a user's editor save. Abort rather + // than atomically replacing a newer file with the stale parsed snapshot. + throw new Error('Codex hooks.json changed while Orca prepared its trust repair') + } +} + +/** One-time pristine copy of the user's file, kept under Orca's userData. */ +export function backupRealHomeHooksJsonOnce( + userDataPath: string, + previousRaw: string | null +): void { + if (previousRaw === null) { + return + } + const backupDir = getRealHomeHookStateDir(userDataPath) + const backupPath = join(backupDir, 'hooks.json.pre-orca') + if (existsSync(backupPath)) { + return + } + // Why: this lane mutates the user's real Codex home. If the required + // pristine recovery copy cannot be created, keep the managed lane intact. + mkdirSync(backupDir, { recursive: true }) + writeFileAtomically(backupPath, previousRaw, { mode: 0o600 }) +} + +export function restoreRealHomeHooksJson( + hooksJsonPath: string, + previousRaw: string | null, + previousMode?: number +): void { + if (previousRaw === null) { + if (existsSync(hooksJsonPath)) { + unlinkSync(hooksJsonPath) + } + return + } + // Why: rollback is part of the safety boundary. Use the shared atomic + // writer so Windows file-lock retries and failed-temp cleanup are covered. + writeFileAtomically(hooksJsonPath, previousRaw, { mode: previousMode }) +} + +/** Places Orca's managed hook in `definitions`, reusing its existing slot when + * one is unambiguous so no later user trust position shifts. */ +export function reconcileManagedHookDefinition( + current: HookDefinition[], + isManagedCommand: (command: string | undefined) => boolean, + command: string +): { definitions: HookDefinition[]; groupIndex: number; handlerIndex: number } { + const directCommandKeys = ['command', 'bash', 'powershell'] as const + const hasManagedDirectCommand = current.some((definition) => + directCommandKeys.some((key) => isManagedCommand(definition[key])) + ) + const nestedLocations = current.flatMap((definition, groupIndex) => + Array.isArray(definition.hooks) + ? definition.hooks.flatMap((hook, handlerIndex) => + isManagedCommand(hook.command) ? [{ groupIndex, handlerIndex }] : [] + ) + : [] + ) + if (!hasManagedDirectCommand && nestedLocations.length === 1) { + const { groupIndex, handlerIndex } = nestedLocations[0]! + const definition = current[groupIndex]! + const hasDirectCommand = directCommandKeys.some((key) => typeof definition[key] === 'string') + if (definition.matcher === undefined && !hasDirectCommand) { + const definitions = [...current] + // Why: users can append groups or handlers after Orca's first install. + // Reusing the exact slot preserves all later positional trust keys. + const hooks = [...definition.hooks!] + hooks[handlerIndex] = buildManagedCommandHook(command) + definitions[groupIndex] = { ...definition, hooks } + return { definitions, groupIndex, handlerIndex } + } + } + + const cleaned = removeManagedCommands(current, isManagedCommand) + // Why: first install appends LAST so no existing user trust position shifts. + return { + definitions: [...cleaned, { hooks: [buildManagedCommandHook(command)] }], + groupIndex: cleaned.length, + handlerIndex: 0 + } +} diff --git a/src/main/codex/codex-trust-config-concurrent-launch.test.ts b/src/main/codex/codex-trust-config-concurrent-launch.test.ts new file mode 100644 index 00000000000..d08d7a54969 --- /dev/null +++ b/src/main/codex/codex-trust-config-concurrent-launch.test.ts @@ -0,0 +1,410 @@ +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { + CodexHookTrustGrantRequest, + CodexHookTrustGrantSessionResult +} from './codex-app-server-client' +import type { CodexManagedTrustGrantPlan } from './codex-hook-trust-grant' +import type { CodexTrustEntry } from './config-toml-trust' + +const testState = { + fakeHomeDir: '', + userDataDir: '', + previousUserDataPath: undefined as string | undefined +} + +vi.mock('node:os', async () => { + // eslint-disable-next-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import() + const actual = await vi.importActual('node:os') + return { ...actual, homedir: () => testState.fakeHomeDir } +}) + +const { CodexAppServerUnsupportedError } = await import('./codex-app-server-client') +const { codexAppServerCapabilityCache } = await import('./codex-app-server-capability-cache') +const { _internals, grantManagedCodexHookTrust } = await import('./codex-hook-trust-grant') +const { markCodexProjectTrusted } = await import('../agent-trust-presets') +const { setCodexTrustGrantTelemetry } = await import('./codex-trust-grant-telemetry') +const { + computeTrustKey, + computeTrustedHash, + normalizeHookTrustKeyForLookup, + readHookTrustEntries, + upsertHookTrustEntries +} = await import('./config-toml-trust') + +let runtimeHomeDir: string + +beforeEach(() => { + testState.fakeHomeDir = mkdtempSync(join(tmpdir(), 'orca-concurrent-home-')) + testState.userDataDir = mkdtempSync(join(tmpdir(), 'orca-concurrent-userdata-')) + testState.previousUserDataPath = process.env.ORCA_USER_DATA_PATH + process.env.ORCA_USER_DATA_PATH = testState.userDataDir + runtimeHomeDir = join(testState.userDataDir, 'codex-runtime-home', 'home') + mkdirSync(runtimeHomeDir, { recursive: true }) + writeFileSync(join(runtimeHomeDir, 'hooks.json'), '{"hooks":{}}\n', 'utf-8') + mkdirSync(join(testState.fakeHomeDir, '.codex'), { recursive: true }) + codexAppServerCapabilityCache.clear() + _internals.resetDiagnostics() +}) + +afterEach(() => { + _internals.setGrantSessionRunner(null) + setCodexTrustGrantTelemetry(() => {}) + codexAppServerCapabilityCache.clear() + if (testState.previousUserDataPath === undefined) { + delete process.env.ORCA_USER_DATA_PATH + } else { + process.env.ORCA_USER_DATA_PATH = testState.previousUserDataPath + } + delete process.env.ORCA_DISABLE_CODEX_TRUST_RPC + rmSync(testState.fakeHomeDir, { recursive: true, force: true }) + rmSync(testState.userDataDir, { recursive: true, force: true }) +}) + +const MANAGED_COMMAND = "/bin/sh '/tmp/orca/codex-hook.sh'" + +function managedEntry(eventLabel: CodexTrustEntry['eventLabel']): CodexTrustEntry { + return { + sourcePath: join(runtimeHomeDir, 'hooks.json'), + eventLabel, + groupIndex: 0, + handlerIndex: 0, + command: MANAGED_COMMAND, + timeoutSec: 10 + } +} + +function buildPlan( + entries: CodexTrustEntry[], + overrides: Partial = {} +): CodexManagedTrustGrantPlan { + return { + runtimeHomePath: runtimeHomeDir, + tomlPath: join(runtimeHomeDir, 'config.toml'), + managedCommand: MANAGED_COMMAND, + managedEntries: entries, + host: { kind: 'native' }, + telemetryLane: 'real-home', + ...overrides + } +} + +const tick = (): Promise => new Promise((resolve) => setTimeout(resolve, 0)) + +/** Stands in for codex app-server: really writes the trust entries into + * config.toml across an await, like the RPC does. */ +function writingSessionRunner(args: { + tomlPath: string + entries: CodexTrustEntry[] + hashPrefix: string + gate?: Promise + outcome?: 'granted' | 'verify-failed' +}) { + return async ( + _request: CodexHookTrustGrantRequest + ): Promise => { + const granted = args.entries.map((entry) => { + const key = computeTrustKey(entry) + return { + key, + normalizedKey: normalizeHookTrustKeyForLookup(key), + trustedHash: `${args.hashPrefix}${entry.eventLabel}` + } + }) + await tick() + upsertHookTrustEntries( + args.tomlPath, + args.entries.map((entry, index) => ({ ...entry, trustedHash: granted[index].trustedHash })) + ) + if (args.gate) { + await args.gate + } + if (args.outcome === 'verify-failed') { + return { + outcome: 'verify-failed', + reason: 'listed hash mismatch', + reasonClass: 'post-grant-mismatch' + } + } + return { outcome: 'granted', wroteTrust: true, entries: granted } + } +} + +describe('two Codex pane launches against one config.toml', () => { + it('does not let a failing launch roll back a concurrent launch that already succeeded', async () => { + // Why warm: on a cold host the shared capability probe incidentally + // serializes the two launches. Once the host is known-supported that + // dedupe is bypassed and the per-file lane is the only thing left. + codexAppServerCapabilityCache.rememberSupported('native') + const tomlPath = join(runtimeHomeDir, 'config.toml') + const entries = [managedEntry('session_start')] + let sessionsInFlight = 0 + let maxSessionsInFlight = 0 + let call = 0 + let releaseFirst!: () => void + const firstGate = new Promise((resolve) => { + releaseFirst = resolve + }) + + _internals.setGrantSessionRunner(async (request) => { + sessionsInFlight += 1 + maxSessionsInFlight = Math.max(maxSessionsInFlight, sessionsInFlight) + call += 1 + const isFirst = call === 1 + try { + return await writingSessionRunner({ + tomlPath, + entries, + hashPrefix: isFirst ? 'sha256:doomed-' : 'sha256:survivor-', + gate: isFirst ? firstGate : undefined, + outcome: isFirst ? 'verify-failed' : 'granted' + })(request) + } finally { + sessionsInFlight -= 1 + } + }) + + const doomed = grantManagedCodexHookTrust(buildPlan(entries)) + const survivor = grantManagedCodexHookTrust(buildPlan(entries)) + await tick() + await tick() + releaseFirst() + + expect(await doomed).toMatchObject({ lane: 'fallback', reason: 'verify-failed' }) + expect(await survivor).toMatchObject({ lane: 'rpc' }) + // The doomed run's rollback must not resurrect the pre-grant file over + // the entries the survivor legitimately wrote. + const trust = readHookTrustEntries(tomlPath) + const key = normalizeHookTrustKeyForLookup(computeTrustKey(entries[0])) + expect(trust.get(key)?.trustedHash).toBe('sha256:survivor-session_start') + expect(maxSessionsInFlight).toBe(1) + }) + + it('keeps a concurrent markCodexProjectTrusted write out of a grant rollback window', async () => { + codexAppServerCapabilityCache.rememberSupported('native') + const tomlPath = join(runtimeHomeDir, 'config.toml') + const entries = [managedEntry('session_start')] + const workspace = mkdtempSync(join(tmpdir(), 'orca-concurrent-ws-')) + let releaseSession!: () => void + const sessionGate = new Promise((resolve) => { + releaseSession = resolve + }) + + _internals.setGrantSessionRunner( + writingSessionRunner({ + tomlPath, + entries, + hashPrefix: 'sha256:doomed-', + gate: sessionGate, + outcome: 'verify-failed' + }) + ) + + try { + const grant = grantManagedCodexHookTrust(buildPlan(entries)) + // Let the grant capture config.toml and start its session. + await tick() + await tick() + const marked = markCodexProjectTrusted(workspace) + await tick() + // The lane must hold the preset write back until rollback has run. + expect(readFileSync(tomlPath, 'utf-8')).not.toContain('trust_level') + + releaseSession() + expect(await grant).toMatchObject({ lane: 'fallback', reason: 'verify-failed' }) + await marked + + expect(readFileSync(tomlPath, 'utf-8')).toContain('trust_level = "trusted"') + } finally { + rmSync(workspace, { recursive: true, force: true }) + } + }) +}) + +describe('concurrent capability probes against a cold host', () => { + it('shares one app-server session between two launches on different config files', async () => { + const secondHome = join(testState.userDataDir, 'second-runtime-home') + mkdirSync(secondHome, { recursive: true }) + writeFileSync(join(secondHome, 'hooks.json'), '{"hooks":{}}\n', 'utf-8') + const entries = [managedEntry('session_start')] + let sessions = 0 + let releaseProbe!: () => void + const probeGate = new Promise((resolve) => { + releaseProbe = resolve + }) + _internals.setGrantSessionRunner(async () => { + sessions += 1 + await probeGate + throw new CodexAppServerUnsupportedError('hooks/grantTrust: method not found') + }) + + const first = grantManagedCodexHookTrust(buildPlan(entries)) + const second = grantManagedCodexHookTrust( + buildPlan([{ ...entries[0], sourcePath: join(secondHome, 'hooks.json') }], { + runtimeHomePath: secondHome, + tomlPath: join(secondHome, 'config.toml') + }) + ) + await tick() + await tick() + expect(sessions).toBe(1) + releaseProbe() + + expect(await first).toMatchObject({ lane: 'fallback', reason: 'unsupported' }) + expect(await second).toMatchObject({ lane: 'fallback', reason: 'unsupported-cached' }) + expect(sessions).toBe(1) + }) + + it('leaves the waiter config.toml untouched when the shared probe reports unsupported', async () => { + const secondHome = join(testState.userDataDir, 'second-runtime-home') + mkdirSync(secondHome, { recursive: true }) + writeFileSync(join(secondHome, 'hooks.json'), '{"hooks":{}}\n', 'utf-8') + const waiterToml = join(secondHome, 'config.toml') + const waiterEntry = { + ...managedEntry('session_start'), + sourcePath: join(secondHome, 'hooks.json') + } + // Self-computed trust the fallback lane already wrote for this pane. + upsertHookTrustEntries(waiterToml, [ + { ...waiterEntry, trustedHash: computeTrustedHash(waiterEntry) } + ]) + const before = readFileSync(waiterToml, 'utf-8') + + let releaseProbe!: () => void + const probeGate = new Promise((resolve) => { + releaseProbe = resolve + }) + _internals.setGrantSessionRunner(async () => { + await probeGate + throw new CodexAppServerUnsupportedError('hooks/grantTrust: method not found') + }) + + const first = grantManagedCodexHookTrust(buildPlan([managedEntry('session_start')])) + const waiter = grantManagedCodexHookTrust( + buildPlan([waiterEntry], { runtimeHomePath: secondHome, tomlPath: waiterToml }) + ) + await tick() + releaseProbe() + await first + expect(await waiter).toMatchObject({ lane: 'fallback', reason: 'unsupported-cached' }) + expect(readFileSync(waiterToml, 'utf-8')).toBe(before) + }) +}) + +describe('host-scoped transient cooldown', () => { + // Why: the cooldown lives outside the per-file lane, so a failure on one + // pane's config.toml has to suppress every other pane on that host and + // nothing on a different one. + it('suppresses a second config.toml on the same host but not another host', async () => { + const secondHome = join(testState.userDataDir, 'second-runtime-home') + mkdirSync(secondHome, { recursive: true }) + writeFileSync(join(secondHome, 'hooks.json'), '{"hooks":{}}\n', 'utf-8') + const entries = [managedEntry('session_start')] + let calls = 0 + _internals.setGrantSessionRunner(() => { + calls += 1 + throw new Error('spawn ETIMEDOUT') + }) + + expect(await grantManagedCodexHookTrust(buildPlan(entries))).toMatchObject({ + lane: 'fallback', + reason: 'error' + }) + expect( + await grantManagedCodexHookTrust( + buildPlan([{ ...entries[0], sourcePath: join(secondHome, 'hooks.json') }], { + runtimeHomePath: secondHome, + tomlPath: join(secondHome, 'config.toml') + }) + ) + ).toMatchObject({ lane: 'fallback', reason: 'retry-cached' }) + expect(calls).toBe(1) + + // A WSL distro runs its own codex binary; the native cooldown must not reach it. + expect( + await grantManagedCodexHookTrust( + buildPlan(entries, { + host: { kind: 'wsl', distro: 'Ubuntu', linuxRuntimeHome: '/home/u/.codex' } + }) + ) + ).toMatchObject({ lane: 'fallback', reason: 'error' }) + expect(calls).toBe(2) + }) + + // Why: the cooldown check runs before the lane, so a launch already admitted + // can succeed after a sibling failed. That proof of health must clear the + // sibling's cooldown instead of suppressing the host for five more minutes. + it('lets a concurrent success clear a cooldown a sibling failure just set', async () => { + codexAppServerCapabilityCache.rememberSupported('native') + const secondHome = join(testState.userDataDir, 'second-runtime-home') + mkdirSync(secondHome, { recursive: true }) + writeFileSync(join(secondHome, 'hooks.json'), '{"hooks":{}}\n', 'utf-8') + const entries = [managedEntry('session_start')] + const okEntry = { ...entries[0], sourcePath: join(secondHome, 'hooks.json') } + const okToml = join(secondHome, 'config.toml') + const okPlan = buildPlan([okEntry], { runtimeHomePath: secondHome, tomlPath: okToml }) + + let releaseFailure!: () => void + const failureGate = new Promise((resolve) => { + releaseFailure = resolve + }) + let sessions = 0 + _internals.setGrantSessionRunner(async (request) => { + sessions += 1 + if (request.hooksListCwd === runtimeHomeDir) { + await failureGate + throw new Error('spawn ETIMEDOUT') + } + return writingSessionRunner({ + tomlPath: okToml, + entries: [okEntry], + hashPrefix: 'sha256:ok-' + })(request) + }) + + const failing = grantManagedCodexHookTrust(buildPlan(entries)) + const succeeding = grantManagedCodexHookTrust(okPlan) + releaseFailure() + expect(await failing).toMatchObject({ lane: 'fallback', reason: 'error' }) + expect(await succeeding).toMatchObject({ lane: 'rpc' }) + + // A later launch on the same host must reach the RPC, not the cooldown. + // The ledger is shared across runtime homes, so clear it to force a session. + rmSync(join(testState.userDataDir, 'codex-runtime-home', 'trust-grant-ledger.json'), { + force: true + }) + expect(sessions).toBe(2) + expect(await grantManagedCodexHookTrust(okPlan)).toMatchObject({ lane: 'rpc' }) + expect(sessions).toBe(3) + }) +}) + +describe('reentrancy under concurrency', () => { + it('completes a grant nested inside an installer that already holds both lanes', async () => { + const { runExclusivelyForCodexTrustConfig } = + await import('./codex-trust-config-mutation-queue') + const entries = [managedEntry('session_start')] + const tomlPath = join(runtimeHomeDir, 'config.toml') + const systemToml = join(testState.fakeHomeDir, '.codex', 'config.toml') + _internals.setGrantSessionRunner( + writingSessionRunner({ tomlPath, entries, hashPrefix: 'sha256:nested-' }) + ) + const workspace = mkdtempSync(join(tmpdir(), 'orca-nested-ws-')) + try { + // Installer lock order: runtime then system, with a grant and a preset + // write nested inside both. + const outcome = await runExclusivelyForCodexTrustConfig(tomlPath, () => + runExclusivelyForCodexTrustConfig(systemToml, async () => { + await markCodexProjectTrusted(workspace) + return grantManagedCodexHookTrust(buildPlan(entries)) + }) + ) + expect(outcome).toMatchObject({ lane: 'rpc' }) + expect(readFileSync(tomlPath, 'utf-8')).toContain('trust_level = "trusted"') + } finally { + rmSync(workspace, { recursive: true, force: true }) + } + }, 5000) +}) diff --git a/src/main/codex/codex-trust-config-mutation-queue.test.ts b/src/main/codex/codex-trust-config-mutation-queue.test.ts new file mode 100644 index 00000000000..e2d775b4ed7 --- /dev/null +++ b/src/main/codex/codex-trust-config-mutation-queue.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from 'vitest' +import { runExclusivelyForCodexTrustConfig } from './codex-trust-config-mutation-queue' + +function deferred(): { promise: Promise; resolve: () => void; reject: (e: unknown) => void } { + let resolve!: () => void + let reject!: (e: unknown) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } +} + +describe('runExclusivelyForCodexTrustConfig', () => { + // Why: the grant lane runs inside the installer that already owns the file; + // a non-reentrant lane would queue it behind itself and never settle. + it('passes through a nested acquire of a lane the caller already holds', async () => { + const nested = await runExclusivelyForCodexTrustConfig('/a/config.toml', () => + runExclusivelyForCodexTrustConfig('/a/config.toml', () => Promise.resolve('inner')) + ) + expect(nested).toBe('inner') + }) + + it('still queues an unrelated lane acquired from inside another lane', async () => { + const gate = deferred() + let innerRan = false + const blocking = runExclusivelyForCodexTrustConfig('/b/config.toml', () => gate.promise) + const nested = runExclusivelyForCodexTrustConfig('/a/config.toml', () => + runExclusivelyForCodexTrustConfig('/b/config.toml', () => { + innerRan = true + return Promise.resolve() + }) + ) + await Promise.resolve() + expect(innerRan).toBe(false) + gate.resolve() + await blocking + await nested + expect(innerRan).toBe(true) + }) + + it('runs one mutation at a time per config.toml', async () => { + const order: string[] = [] + const first = deferred() + const second = deferred() + + const a = runExclusivelyForCodexTrustConfig('/home/.codex/config.toml', async () => { + order.push('a:start') + await first.promise + order.push('a:end') + return 'a' + }) + const b = runExclusivelyForCodexTrustConfig('/home/.codex/config.toml', async () => { + order.push('b:start') + await second.promise + order.push('b:end') + return 'b' + }) + + await Promise.resolve() + expect(order).toEqual(['a:start']) + first.resolve() + await a + second.resolve() + await b + expect(order).toEqual(['a:start', 'a:end', 'b:start', 'b:end']) + }) + + it('keeps distinct config.toml paths independent', async () => { + const gate = deferred() + let secondRan = false + const blocked = runExclusivelyForCodexTrustConfig('/a/config.toml', () => gate.promise) + await runExclusivelyForCodexTrustConfig('/b/config.toml', async () => { + secondRan = true + }) + expect(secondRan).toBe(true) + gate.resolve() + await blocked + }) + + it('keeps the queue alive after a rejected mutation', async () => { + const failing = runExclusivelyForCodexTrustConfig('/a/config.toml', () => + Promise.reject(new Error('grant blew up')) + ) + await expect(failing).rejects.toThrow('grant blew up') + await expect( + runExclusivelyForCodexTrustConfig('/a/config.toml', () => Promise.resolve('next')) + ).resolves.toBe('next') + }) + + // Why: normalized keys, so a Windows caller passing the other separator or + // case must still land in the same lane as the run it has to wait for. + it('serializes equivalent paths that differ only in normalization', async () => { + const gate = deferred() + let secondStarted = false + const blocked = runExclusivelyForCodexTrustConfig( + String.raw`C:\Users\Alice\.codex\config.toml`, + () => gate.promise + ) + const queued = runExclusivelyForCodexTrustConfig('C:/Users/Alice/.codex/config.toml', () => { + secondStarted = true + return Promise.resolve() + }) + await Promise.resolve() + expect(secondStarted).toBe(false) + gate.resolve() + await blocked + await queued + expect(secondStarted).toBe(true) + }) +}) diff --git a/src/main/codex/codex-trust-config-mutation-queue.ts b/src/main/codex/codex-trust-config-mutation-queue.ts new file mode 100644 index 00000000000..08aa3db4316 --- /dev/null +++ b/src/main/codex/codex-trust-config-mutation-queue.ts @@ -0,0 +1,46 @@ +import { AsyncLocalStorage } from 'node:async_hooks' +import { normalizeRuntimePathForComparison } from '../../shared/cross-platform-path' + +const tailByTomlPath = new Map>() +// Why: the grant lane runs inside the installer that already owns the file. +// AsyncLocalStorage survives awaits, so the inner acquire can see the outer +// one and pass through instead of queueing behind itself forever. +const heldKeys = new AsyncLocalStorage>() + +/** + * Serializes everything that mutates one Codex `config.toml` — hook installs, + * trust grants, and user-hook rebases — as a single lane per file. + * + * Why (#16441): these used to block the main thread, so two of them could + * never be in flight at once. Now that they await, a second run could write + * the file between another run's capture and its restore-on-failure, undoing + * a mutation that run never made and resurrecting trust it deliberately + * removed. + */ +export function runExclusivelyForCodexTrustConfig( + tomlPath: string, + run: () => Promise +): Promise { + const key = normalizeRuntimePathForComparison(tomlPath) + const held = heldKeys.getStore() + if (held?.has(key)) { + return run() + } + const owned = new Set(held ?? []) + owned.add(key) + const enter = (): Promise => heldKeys.run(owned, run) + const previous = tailByTomlPath.get(key) ?? Promise.resolve() + // Why both handlers: a rejected predecessor must not cancel the queue. + const result = previous.then(enter, enter) + const tail = result.then( + () => undefined, + () => undefined + ) + tailByTomlPath.set(key, tail) + void tail.then(() => { + if (tailByTomlPath.get(key) === tail) { + tailByTomlPath.delete(key) + } + }) + return result +} diff --git a/src/main/codex/codex-trust-grant-host.test.ts b/src/main/codex/codex-trust-grant-host.test.ts index 5e23add099e..5ad415e9779 100644 --- a/src/main/codex/codex-trust-grant-host.test.ts +++ b/src/main/codex/codex-trust-grant-host.test.ts @@ -1,11 +1,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { execFileSyncMock, resolveCodexCommandMock } = vi.hoisted(() => ({ - execFileSyncMock: vi.fn(), +const { runProcessMock, resolveCodexCommandMock } = vi.hoisted(() => ({ + runProcessMock: vi.fn(), resolveCodexCommandMock: vi.fn() })) -vi.mock('node:child_process', () => ({ execFileSync: execFileSyncMock })) +vi.mock('../../shared/child-process/run-process', () => ({ runProcess: runProcessMock })) vi.mock('../codex-cli/command', () => ({ resolveCodexCommand: resolveCodexCommandMock @@ -14,23 +14,28 @@ vi.mock('../codex-cli/command', () => ({ import { resolveCodexTrustGrantHost } from './codex-trust-grant-host' beforeEach(() => { - execFileSyncMock.mockReset() + runProcessMock.mockReset() // Stand in for the guest shell: rc banner first, then the payload inside the // command's own fence. The identity script execs, so no closing fence is written. - execFileSyncMock.mockImplementation((_command: string, args: string[]) => { - const nonce = /__ORCA_WSL_CAPTURE_BEGIN_([^_]+)__/.exec(String(args.at(-1)))?.[1] ?? '' - return ( - 'To run a command as administrator (user "root"), use "sudo ".\n\n' + - `__ORCA_WSL_CAPTURE_BEGIN_${nonce}__/home/alice/.local/bin/codex\ncodex-cli 1.2.3\n` - ) + runProcessMock.mockImplementation((spec: { args: string[] }) => { + const nonce = /__ORCA_WSL_CAPTURE_BEGIN_([^_]+)__/.exec(String(spec.args.at(-1)))?.[1] ?? '' + return Promise.resolve({ + code: 0, + signal: null, + timedOut: false, + stderr: '', + stdout: + 'To run a command as administrator (user "root"), use "sudo ".\n\n' + + `__ORCA_WSL_CAPTURE_BEGIN_${nonce}__/home/alice/.local/bin/codex\ncodex-cli 1.2.3\n` + }) }) resolveCodexCommandMock.mockReset() resolveCodexCommandMock.mockReturnValue(process.execPath) }) describe('resolveCodexTrustGrantHost', () => { - it('resolves the native command once for both the binary stamp and request', () => { - const host = resolveCodexTrustGrantHost({ kind: 'native' }) + it('resolves the native command once for both the binary stamp and request', async () => { + const host = await resolveCodexTrustGrantHost({ kind: 'native' }) const input = { runtimeHomePath: '/tmp/codex-home', managedCommand: '/bin/sh codex-hook.sh', @@ -43,11 +48,11 @@ describe('resolveCodexTrustGrantHost', () => { // Why: PATH/version-manager scans are synchronous launch-path I/O. Reusing // the resolved command keeps one grant at one scan regardless of consumers. expect(resolveCodexCommandMock).toHaveBeenCalledTimes(1) - expect(execFileSyncMock).not.toHaveBeenCalled() + expect(runProcessMock).not.toHaveBeenCalled() }) - it('builds WSL requests without scanning the native PATH', () => { - const host = resolveCodexTrustGrantHost({ + it('builds WSL requests without scanning the native PATH', async () => { + const host = await resolveCodexTrustGrantHost({ kind: 'wsl', distro: 'Ubuntu', linuxRuntimeHome: '/home/alice/.codex-runtime' @@ -65,11 +70,33 @@ describe('resolveCodexTrustGrantHost', () => { version: 'codex-cli 1.2.3' }) expect(request.invocation.command).toBe('wsl.exe') - expect(execFileSyncMock).toHaveBeenCalledWith( - 'wsl.exe', - expect.arrayContaining(['-d', 'Ubuntu', '--exec', 'sh', '-c']), - expect.objectContaining({ encoding: 'utf-8', timeout: 5_000, windowsHide: true }) + // Why (#16441): the identity probe runs through the shared async runner — + // an execFileSync here froze the Electron main thread for its full timeout. + expect(runProcessMock).toHaveBeenCalledWith( + expect.objectContaining({ + program: 'wsl.exe', + args: expect.arrayContaining(['-d', 'Ubuntu', '--exec', 'sh', '-c']), + timeoutMs: 5_000 + }) ) expect(resolveCodexCommandMock).not.toHaveBeenCalled() }) + + it('drops the stamp when the guest probe fails instead of trusting partial stdout', async () => { + runProcessMock.mockResolvedValue({ + code: 127, + signal: null, + timedOut: false, + stdout: '', + stderr: 'codex not found' + }) + + const host = await resolveCodexTrustGrantHost({ + kind: 'wsl', + distro: 'Ubuntu', + linuxRuntimeHome: '/home/alice/.codex-runtime' + }) + + expect(host.binaryStamp).toBeNull() + }) }) diff --git a/src/main/codex/codex-trust-grant-host.ts b/src/main/codex/codex-trust-grant-host.ts index c74f8f19af9..9228fad43ce 100644 --- a/src/main/codex/codex-trust-grant-host.ts +++ b/src/main/codex/codex-trust-grant-host.ts @@ -1,4 +1,4 @@ -import { execFileSync } from 'node:child_process' +import { runProcess } from '../../shared/child-process/run-process' import { resolveCodexCommand } from '../codex-cli/command' import { getSpawnArgsForWindows } from '../win32-utils' import { @@ -36,10 +36,18 @@ export type ResolvedCodexTrustGrantHost = { buildRequest: (input: CodexTrustGrantRequestInput) => CodexHookTrustGrantRequest } -export function resolveCodexTrustGrantHost(host: CodexTrustGrantHost): ResolvedCodexTrustGrantHost { +/** + * Resolves the host that runs the codex binary for a grant session. + * + * Async because the WSL identity probe shells into the distro; #16441 measured + * a 15s main-thread stall when launch prep did this work synchronously. + */ +export async function resolveCodexTrustGrantHost( + host: CodexTrustGrantHost +): Promise { if (host.kind === 'wsl') { return { - binaryStamp: buildWslCodexBinaryStamp(host.distro), + binaryStamp: await buildWslCodexBinaryStamp(host.distro), buildRequest: (input) => ({ invocation: { command: 'wsl.exe', @@ -55,6 +63,10 @@ export function resolveCodexTrustGrantHost(host: CodexTrustGrantHost): ResolvedC } } + return resolveNativeCodexTrustGrantHost() +} + +export function resolveNativeCodexTrustGrantHost(): ResolvedCodexTrustGrantHost { // Why: command resolution scans PATH/version-manager directories. Resolve // once per grant and reuse it for both the binary stamp and invocation. const command = resolveCodexCommand() @@ -81,19 +93,24 @@ export function resolveCodexTrustGrantHost(host: CodexTrustGrantHost): ResolvedC } } -function buildWslCodexBinaryStamp(distro: string): CodexTrustGrantBinaryStamp | null { +async function buildWslCodexBinaryStamp( + distro: string +): Promise { try { // Why: WSL PATH resolution happens inside the distro's login shell. The // resolved path plus CLI version detects upgrades without assuming UNC access. const probe = buildWslCodexIdentityProbe(distro) - const stdout = execFileSync('wsl.exe', probe.args, { - encoding: 'utf-8', - timeout: WSL_CODEX_AVAILABILITY_TIMEOUT_MS, - windowsHide: true + const result = await runProcess({ + program: 'wsl.exe', + args: probe.args, + timeoutMs: WSL_CODEX_AVAILABILITY_TIMEOUT_MS }) + if (result.code !== 0 || result.timedOut) { + return null + } // Why: the split below is positional, so login-shell rc output ahead of the // payload would silently become the "path" and destabilize the stamp. - const output = probe.readStdout(stdout) + const output = probe.readStdout(result.stdout) if (output === null) { return null } @@ -114,9 +131,10 @@ export function readCodexTrustGrantLedgerHomeMatchingStamp( return home && binaryStampsMatch(home.binary, currentStamp) ? home : null } -export function readCurrentCodexTrustGrantLedgerHome( - runtimeHomePath: string, - host: CodexTrustGrantHost +/** Native-only: the WSL stamp needs a subprocess, and status reads must stay + * synchronous for the hook-status readers that never target a distro. */ +export function readCurrentNativeCodexTrustGrantLedgerHome( + runtimeHomePath: string ): CodexTrustGrantLedgerHome | null { try { const home = readCodexTrustGrantLedgerHome(runtimeHomePath) @@ -125,7 +143,7 @@ export function readCurrentCodexTrustGrantLedgerHome( // and version-manager scan when there is no recorded stamp to validate. return null } - return binaryStampsMatch(home.binary, resolveCodexTrustGrantHost(host).binaryStamp) + return binaryStampsMatch(home.binary, resolveNativeCodexTrustGrantHost().binaryStamp) ? home : null } catch { diff --git a/src/main/codex/codex-trust-grant-main-thread-boundary.test.ts b/src/main/codex/codex-trust-grant-main-thread-boundary.test.ts new file mode 100644 index 00000000000..4b88d95991c --- /dev/null +++ b/src/main/codex/codex-trust-grant-main-thread-boundary.test.ts @@ -0,0 +1,60 @@ +import { existsSync, readFileSync, readdirSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' + +/** + * Ratchet for stablyai/orca#16441. + * + * Codex hook trust used to be granted by blocking the Electron main thread on + * `spawnSync` of a bundled ELECTRON_RUN_AS_NODE entry, for the whole + * app-server deadline: 15s native, 35s WSL, ~45s on the three-session real-home + * path. The window showed "Not Responding" during cold start and pane launch. + * + * The subprocess only ever existed to donate an event loop to a deliberately + * blocked parent, so this guards the shape of the fix rather than one call + * site: nothing on the trust-grant lane may start a child process + * synchronously, and the forked entry must stay gone. + */ +const CODEX_DIR = __dirname + +const SYNC_SPAWN_PATTERN = /\b(?:spawnSync|execSync|execFileSync|runProcessSync)\s*[(<]/ + +/** Drop comments so the prose explaining the old idiom is not an offender. */ +function codeText(contents: string): string { + return contents + .split('\n') + .filter((line) => !/^\s*(?:\/\/|\/\*|\*)/.test(line)) + .join('\n') +} + +function listCodexSourceFiles(): string[] { + return readdirSync(CODEX_DIR).filter((name) => name.endsWith('.ts') && !name.endsWith('.test.ts')) +} + +describe('codex trust grant main-thread boundary', () => { + it('starts no child process synchronously anywhere in the codex module', () => { + const offenders = listCodexSourceFiles().filter((name) => + SYNC_SPAWN_PATTERN.test(codeText(readFileSync(join(CODEX_DIR, name), 'utf8'))) + ) + expect(offenders).toEqual([]) + }) + + it('keeps the forked grant entry and its blocking bridge deleted', () => { + for (const name of [ + 'codex-app-server-grant-bridge.ts', + 'codex-app-server-grant-entry.ts', + 'codex-app-server-grant-envelope.ts' + ]) { + expect(existsSync(join(CODEX_DIR, name))).toBe(false) + } + }) + + it('keeps the trust-grant lane on async entry points', () => { + const grant = readFileSync(join(CODEX_DIR, 'codex-hook-trust-grant.ts'), 'utf8') + expect(grant).toContain('export async function grantManagedCodexHookTrust(') + const host = readFileSync(join(CODEX_DIR, 'codex-trust-grant-host.ts'), 'utf8') + expect(host).toContain('export async function resolveCodexTrustGrantHost(') + const realHome = readFileSync(join(CODEX_DIR, 'codex-real-home-hook-install.ts'), 'utf8') + expect(realHome).toContain('}): Promise {') + }) +}) diff --git a/src/main/codex/codex-trust-grant-telemetry.ts b/src/main/codex/codex-trust-grant-telemetry.ts index 25265219ba8..aa40c17cba3 100644 --- a/src/main/codex/codex-trust-grant-telemetry.ts +++ b/src/main/codex/codex-trust-grant-telemetry.ts @@ -16,10 +16,9 @@ export type CodexTrustGrantFallbackReason = | 'retry-cached' | 'error' -/** Closed classification of `reason: 'error'` fallbacks. Errors cross the - * grant-bridge envelope as message text (only timeout/unsupported keep their - * name), so classes are matched on the bounded message shapes each layer - * produces — never forwarded raw. */ +/** Closed classification of `reason: 'error'` fallbacks. Only timeout and + * unsupported carry a stable error name, so the rest are matched on the + * bounded message shapes each layer produces — never forwarded raw. */ export type CodexTrustGrantErrorClass = | 'binary-missing' | 'timeout' diff --git a/src/main/codex/codex-user-hook-trust-rebase.test.ts b/src/main/codex/codex-user-hook-trust-rebase.test.ts index 89f25176d50..c3e0f760f69 100644 --- a/src/main/codex/codex-user-hook-trust-rebase.test.ts +++ b/src/main/codex/codex-user-hook-trust-rebase.test.ts @@ -27,7 +27,7 @@ beforeEach(() => { }) afterEach(() => { - _internals.setSessionRunnerSync(null) + _internals.setSessionRunner(null) _internals.resetRetryState() codexAppServerCapabilityCache.clear() rmSync(root, { recursive: true, force: true }) @@ -38,18 +38,18 @@ function command(command: string): HookCommandConfig { } describe('real-home user hook trust rebasing', () => { - it('writes directly without reading config or spawning Codex when user positions stay stable', () => { + it('writes directly without reading config or spawning Codex when user positions stay stable', async () => { const user = command('user-hook') const orca = command('orca-hook') const before = { Stop: [{ hooks: [user] }] } const after = { Stop: [{ hooks: [user] }, { hooks: [orca] }] } let wroteHooks = false - _internals.setSessionRunnerSync(() => { + _internals.setSessionRunner(() => { throw new Error('stable positions must not open an app-server session') }) expect( - mutateRealHomeHooksPreservingUserTrust({ + await mutateRealHomeHooksPreservingUserTrust({ sourcePath: hooksPath, runtimeHomePath: root, tomlPath: configPath, @@ -67,7 +67,7 @@ describe('real-home user hook trust rebasing', () => { expect(existsSync(configPath)).toBe(false) }) - it('finds multiple shifted user hooks, including a handler from a mixed group', () => { + it('finds multiple shifted user hooks, including a handler from a mixed group', async () => { const orca = command('orca-hook') const first = command('first-user') const second = command('second-user') @@ -98,7 +98,7 @@ describe('real-home user hook trust rebasing', () => { ]) }) - it('carries only previously trusted states into the repair request', () => { + it('carries only previously trusted states into the repair request', async () => { const orca = command('orca-hook') const trusted = command('trusted-user') const untrusted = command('untrusted-user') @@ -107,7 +107,7 @@ describe('real-home user hook trust rebasing', () => { writeFileSync(hooksPath, `${JSON.stringify({ hooks: before }, null, 2)}\n`) writeFileSync(configPath, '# original config\n') const requests: CodexUserHookTrustRebaseRequest[] = [] - _internals.setSessionRunnerSync((request) => { + _internals.setSessionRunner(async (request) => { requests.push(request) if (request.operation === 'inspect-user-hook-trust') { return { @@ -123,7 +123,7 @@ describe('real-home user hook trust rebasing', () => { return { outcome: 'repaired', repaired: 1 } }) - mutateRealHomeHooksPreservingUserTrust({ + await mutateRealHomeHooksPreservingUserTrust({ sourcePath: hooksPath, runtimeHomePath: root, tomlPath: configPath, @@ -147,14 +147,14 @@ describe('real-home user hook trust rebasing', () => { } }) - it('marks the host unsupported and skips further codex sessions', () => { + it('marks the host unsupported and skips further codex sessions', async () => { const orca = command('orca-hook') const user = command('user-hook') const before = { Stop: [{ hooks: [orca] }, { hooks: [user] }] } const after = { Stop: [{ hooks: [user] }] } writeFileSync(configPath, '# config\n') let sessions = 0 - _internals.setSessionRunnerSync(() => { + _internals.setSessionRunner(async () => { sessions += 1 throw new CodexAppServerUnsupportedError('unrecognized subcommand app-server') }) @@ -172,20 +172,22 @@ describe('real-home user hook trust rebasing', () => { } } - expect(() => mutateRealHomeHooksPreservingUserTrust(args)).toThrow('unrecognized subcommand') - expect(() => mutateRealHomeHooksPreservingUserTrust(args)).toThrow('marked unsupported') + await expect(mutateRealHomeHooksPreservingUserTrust(args)).rejects.toThrow( + 'unrecognized subcommand' + ) + await expect(mutateRealHomeHooksPreservingUserTrust(args)).rejects.toThrow('marked unsupported') expect(sessions).toBe(1) expect(codexAppServerCapabilityCache.shouldTry('native')).toBe(false) }) - it('cools down after a transient session failure instead of retrying every launch prep', () => { + it('cools down after a transient session failure instead of retrying every launch prep', async () => { const orca = command('orca-hook') const user = command('user-hook') const before = { Stop: [{ hooks: [orca] }, { hooks: [user] }] } const after = { Stop: [{ hooks: [user] }] } writeFileSync(configPath, '# config\n') let sessions = 0 - _internals.setSessionRunnerSync(() => { + _internals.setSessionRunner(async () => { sessions += 1 throw new Error('pre-mutation hooks/list reported 0 of 1 moved user hooks') }) @@ -203,14 +205,16 @@ describe('real-home user hook trust rebasing', () => { } } - expect(() => mutateRealHomeHooksPreservingUserTrust(args)).toThrow('0 of 1 moved user hooks') - expect(() => mutateRealHomeHooksPreservingUserTrust(args)).toThrow('cooling down') + await expect(mutateRealHomeHooksPreservingUserTrust(args)).rejects.toThrow( + '0 of 1 moved user hooks' + ) + await expect(mutateRealHomeHooksPreservingUserTrust(args)).rejects.toThrow('cooling down') expect(sessions).toBe(1) // Why: a transient failure must not poison the shared capability signal. expect(codexAppServerCapabilityCache.shouldTry('native')).toBe(true) }) - it('restores both files byte-exactly when post-mutation repair fails', () => { + it('restores both files byte-exactly when post-mutation repair fails', async () => { const orca = command('orca-hook') const user = command('user-hook') const before = { Stop: [{ hooks: [orca] }, { hooks: [user] }] } @@ -220,7 +224,7 @@ describe('real-home user hook trust rebasing', () => { const originalConfig = '# user formatting\r\nmodel = "x"\r\n' writeFileSync(hooksPath, originalHooks) writeFileSync(configPath, originalConfig) - _internals.setSessionRunnerSync((request) => { + _internals.setSessionRunner(async (request) => { if (request.operation === 'inspect-user-hook-trust') { return { outcome: 'inspected', @@ -236,7 +240,7 @@ describe('real-home user hook trust rebasing', () => { throw new Error('repair transport failed') }) - expect(() => + await expect( mutateRealHomeHooksPreservingUserTrust({ sourcePath: hooksPath, runtimeHomePath: root, @@ -246,7 +250,7 @@ describe('real-home user hook trust rebasing', () => { writeHooks: () => writeFileSync(hooksPath, `${JSON.stringify({ hooks: after })}\n`), restoreHooks: () => writeFileSync(hooksPath, originalHooks) }) - ).toThrow('repair transport failed') + ).rejects.toThrow('repair transport failed') expect(readFileSync(hooksPath, 'utf-8')).toBe(originalHooks) expect(readFileSync(configPath, 'utf-8')).toBe(originalConfig) }) diff --git a/src/main/codex/codex-user-hook-trust-rebase.ts b/src/main/codex/codex-user-hook-trust-rebase.ts index 0020e106c2f..67a69afce6e 100644 --- a/src/main/codex/codex-user-hook-trust-rebase.ts +++ b/src/main/codex/codex-user-hook-trust-rebase.ts @@ -4,7 +4,7 @@ import { getCodexAppServerHostKey, type CodexAppServerHostKey } from './codex-app-server-capability-cache' -import { runCodexUserHookTrustRebaseSessionSync } from './codex-app-server-grant-bridge' +import { runCodexUserHookTrustRebaseSession } from './codex-user-hook-trust-rebase-client' import { isCodexAppServerUnsupportedError } from './codex-app-server-session' import { CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS } from './codex-hook-trust-grant' import { createCodexHookTrustEntry } from './codex-hook-identity' @@ -14,6 +14,7 @@ import { restoreCodexTrustConfig, type CodexTrustConfigSnapshot } from './codex-trust-config-rollback' +import { runExclusivelyForCodexTrustConfig } from './codex-trust-config-mutation-queue' import { computeTrustKey, type CodexTrustEntry } from './config-toml-trust' import type { CodexUserHookTrustRebaseRequest, @@ -23,11 +24,13 @@ import type { type HooksByEvent = Record -type RebaseSessionRunnerSync = ( +type RebaseSessionRunner = ( request: CodexUserHookTrustRebaseRequest -) => CodexUserHookTrustRebaseResult +) => Promise -let runSessionSync: RebaseSessionRunnerSync = runCodexUserHookTrustRebaseSessionSync +// Why (#16441): the session runs in-process; forking it through spawnSync +// froze the main thread for the whole app-server deadline on every install. +let runSession: RebaseSessionRunner = runCodexUserHookTrustRebaseSession // Why: launch prep re-runs the callers on every pane spawn. A host stuck // without a usable rebase lane (old CLI, unmatched keys) must not pay a codex @@ -129,12 +132,25 @@ export function mutateRealHomeHooksPreservingUserTrust(args: { afterHooks: HooksByEvent writeHooks: () => void restoreHooks: () => void -}): CodexTrustConfigSnapshot | null { +}): Promise { const moves = getMovedCodexUserHookTrust(args.sourcePath, args.beforeHooks, args.afterHooks) if (moves.length === 0) { args.writeHooks() - return null + return Promise.resolve(null) } + // Why: capture/mutate/restore on one config.toml is not reentrant. + return runExclusivelyForCodexTrustConfig(args.tomlPath, () => rebaseMovedUserTrust(args, moves)) +} + +async function rebaseMovedUserTrust( + args: { + runtimeHomePath: string + tomlPath: string + writeHooks: () => void + restoreHooks: () => void + }, + moves: CodexUserHookTrustMove[] +): Promise { const hostKey = getCodexAppServerHostKey({ kind: 'native' }) if (!codexAppServerCapabilityCache.shouldTry(hostKey)) { throw new Error('codex app-server is marked unsupported on this host; trust rebase skipped') @@ -148,7 +164,7 @@ export function mutateRealHomeHooksPreservingUserTrust(args: { } const snapshot = captureCodexTrustConfig(args.tomlPath) - const baseRequest = resolveCodexTrustGrantHost({ kind: 'native' }).buildRequest({ + const baseRequest = (await resolveCodexTrustGrantHost({ kind: 'native' })).buildRequest({ runtimeHomePath: args.runtimeHomePath, managedCommand: '', expectedTrustKeys: [], @@ -158,7 +174,7 @@ export function mutateRealHomeHooksPreservingUserTrust(args: { // without shifting a user's positional trust key. let inspected: CodexUserHookTrustRebaseResult try { - inspected = runSessionSync({ + inspected = await runSession({ operation: 'inspect-user-hook-trust', invocation: baseRequest.invocation, hooksListCwd: baseRequest.hooksListCwd, @@ -177,7 +193,7 @@ export function mutateRealHomeHooksPreservingUserTrust(args: { try { args.writeHooks() hooksWritten = true - const repaired = runSessionSync({ + const repaired = await runSession({ operation: 'repair-user-hook-trust', invocation: baseRequest.invocation, hooksListCwd: baseRequest.hooksListCwd, @@ -197,8 +213,8 @@ export function mutateRealHomeHooksPreservingUserTrust(args: { } export const _internals = { - setSessionRunnerSync(runner: RebaseSessionRunnerSync | null): void { - runSessionSync = runner ?? runCodexUserHookTrustRebaseSessionSync + setSessionRunner(runner: RebaseSessionRunner | null): void { + runSession = runner ?? runCodexUserHookTrustRebaseSession }, resetRetryState(): void { rebaseRetryAfterByHost.clear() diff --git a/src/main/codex/hook-service-legacy-cleanup.test.ts b/src/main/codex/hook-service-legacy-cleanup.test.ts index 7093e0242d4..f2b941944c0 100644 --- a/src/main/codex/hook-service-legacy-cleanup.test.ts +++ b/src/main/codex/hook-service-legacy-cleanup.test.ts @@ -54,7 +54,7 @@ function legacyManagedHookCommand(): string { } describe('CodexHookService', () => { - it('removes legacy Orca-managed hooks from system ~/.codex during install', () => { + it('removes legacy Orca-managed hooks from system ~/.codex during install', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') const legacyCommand = legacyManagedHookCommand() @@ -102,7 +102,7 @@ describe('CodexHookService', () => { 'utf-8' ) - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') const systemHooks = JSON.parse(readFileSync(systemHooksPath, 'utf-8')) as { hooks: Record @@ -117,7 +117,7 @@ describe('CodexHookService', () => { expect(systemToml).not.toContain(':session_start:0:0') }) - it('removes very large legacy Orca-managed hook lists from system ~/.codex', () => { + it('removes very large legacy Orca-managed hook lists from system ~/.codex', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') const legacyCommand = legacyManagedHookCommand() @@ -136,7 +136,7 @@ describe('CodexHookService', () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) try { - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') expect(warnSpy).not.toHaveBeenCalledWith( '[codex-hook-service] failed to clean legacy Codex hooks', @@ -151,18 +151,18 @@ describe('CodexHookService', () => { expect(systemHooks.hooks.Stop).toBeUndefined() }, 30_000) - it('removes the legacy Orca Codex profile file when it only contains managed hooks', () => { + it('removes the legacy Orca Codex profile file when it only contains managed hooks', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const profilePath = join(systemCodexHome, 'orca-agent-status.config.toml') mkdirSync(systemCodexHome, { recursive: true }) writeFileSync(profilePath, LEGACY_ORCA_PROFILE_LINES.join('\n'), 'utf-8') - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') expect(existsSync(profilePath)).toBe(false) }) - it('removes only the legacy Orca block from a user-edited Codex profile file', () => { + it('removes only the legacy Orca block from a user-edited Codex profile file', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const profilePath = join(systemCodexHome, 'orca-agent-status.config.toml') mkdirSync(systemCodexHome, { recursive: true }) @@ -172,7 +172,7 @@ describe('CodexHookService', () => { 'utf-8' ) - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') const profileConfig = readFileSync(profilePath, 'utf-8') expect(profileConfig).toContain('model = "gpt-5.5"') @@ -180,7 +180,7 @@ describe('CodexHookService', () => { expect(profileConfig).not.toContain('codex-hook') }) - it('cleans legacy system and profile hooks when runtime hooks.json is malformed during remove', () => { + it('cleans legacy system and profile hooks when runtime hooks.json is malformed during remove', async () => { const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') mkdirSync(managedCodexHome, { recursive: true }) writeFileSync(join(managedCodexHome, 'hooks.json'), '{not json', 'utf-8') @@ -209,7 +209,7 @@ describe('CodexHookService', () => { ) writeFileSync(profilePath, LEGACY_ORCA_PROFILE_LINES.join('\n'), 'utf-8') - const status = new CodexHookService().remove() + const status = await new CodexHookService().remove() expect(status.state).toBe('error') expect(status.detail).toBe('Could not parse Codex hooks.json') @@ -221,7 +221,7 @@ describe('CodexHookService', () => { expect(existsSync(profilePath)).toBe(false) }) - it('sanitizes runtime hooks.json metadata during remove even without managed hooks', () => { + it('sanitizes runtime hooks.json metadata during remove even without managed hooks', async () => { const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') const managedHooksPath = join(managedCodexHome, 'hooks.json') mkdirSync(managedCodexHome, { recursive: true }) @@ -244,7 +244,7 @@ describe('CodexHookService', () => { 'utf-8' ) - const status = new CodexHookService().remove() + const status = await new CodexHookService().remove() expect(status.state).toBe('not_installed') const hooksConfig = JSON.parse(readFileSync(managedHooksPath, 'utf-8')) as { @@ -256,7 +256,7 @@ describe('CodexHookService', () => { expect(hooksConfig.hooks.Stop).toEqual([{ hooks: [{ type: 'command', command: 'user-hook' }] }]) }) - it('cleans duplicate Codex hook representations while keeping status hooks in runtime CODEX_HOME', () => { + it('cleans duplicate Codex hook representations while keeping status hooks in runtime CODEX_HOME', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') const systemTomlPath = join(systemCodexHome, 'config.toml') @@ -307,7 +307,7 @@ describe('CodexHookService', () => { writeFileSync(legacyProfilePath, LEGACY_ORCA_PROFILE_LINES.join('\n'), 'utf-8') const service = new CodexHookService() - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') const managedHooksPath = join(managedCodexHome, 'hooks.json') diff --git a/src/main/codex/hook-service-managed-install.test.ts b/src/main/codex/hook-service-managed-install.test.ts index 72852e45b55..5d0021f9499 100644 --- a/src/main/codex/hook-service-managed-install.test.ts +++ b/src/main/codex/hook-service-managed-install.test.ts @@ -28,6 +28,7 @@ vi.mock('os', async (importOriginal) => { }) import { CodexHookService } from './hook-service' +import { runExclusivelyForCodexTrustConfig } from './codex-trust-config-mutation-queue' const WINDOWS_POWERSHELL_LAUNCHER = /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -WindowStyle Hidden -EncodedCommand \S+$/ @@ -48,7 +49,58 @@ function localManagedCodexEvents(): string[] { } describe('CodexHookService', () => { - it('installs PermissionRequest with trust so Codex approval prompts reach Orca', () => { + // Why (#16441): install promotes in-Orca approvals into ~/.codex/config.toml + // and mirrors that file into the managed home, so holding only the runtime + // lane still lets it land inside a real-home grant's capture->restore window. + it('waits for an in-flight mutation of the system config.toml', async () => { + const systemCodexHome = join(homes.tmpHome, '.codex') + mkdirSync(systemCodexHome, { recursive: true }) + writeFileSync(join(systemCodexHome, 'config.toml'), 'approval_policy = "on-request"\n', 'utf-8') + const managedHooksJsonPath = join(homes.userDataDir, 'codex-runtime-home', 'home', 'hooks.json') + let releaseGrant!: () => void + const grantHoldingSystemConfig = new Promise((resolve) => { + releaseGrant = resolve + }) + const held = runExclusivelyForCodexTrustConfig( + join(systemCodexHome, 'config.toml'), + () => grantHoldingSystemConfig + ) + + const install = new CodexHookService().install() + await new Promise((resolve) => setImmediate(resolve)) + expect(existsSync(managedHooksJsonPath)).toBe(false) + + releaseGrant() + await held + await expect(install).resolves.toMatchObject({ state: 'installed' }) + expect(existsSync(managedHooksJsonPath)).toBe(true) + }) + + it('makes the user-hook refresh wait for the system config.toml too', async () => { + const systemCodexHome = join(homes.tmpHome, '.codex') + mkdirSync(systemCodexHome, { recursive: true }) + writeFileSync(join(systemCodexHome, 'config.toml'), 'approval_policy = "on-request"\n', 'utf-8') + const managedHooksJsonPath = join(homes.userDataDir, 'codex-runtime-home', 'home', 'hooks.json') + let releaseGrant!: () => void + const held = runExclusivelyForCodexTrustConfig( + join(systemCodexHome, 'config.toml'), + () => + new Promise((resolve) => { + releaseGrant = resolve + }) + ) + + const refresh = new CodexHookService().refreshRuntimeUserHooks() + await new Promise((resolve) => setImmediate(resolve)) + expect(existsSync(managedHooksJsonPath)).toBe(false) + + releaseGrant() + await held + await refresh + expect(existsSync(managedHooksJsonPath)).toBe(true) + }) + + it('installs PermissionRequest with trust so Codex approval prompts reach Orca', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') mkdirSync(systemCodexHome, { recursive: true }) writeFileSync( @@ -57,7 +109,7 @@ describe('CodexHookService', () => { 'utf-8' ) - const status = new CodexHookService().install() + const status = await new CodexHookService().install() expect(status.state).toBe('installed') @@ -77,7 +129,7 @@ describe('CodexHookService', () => { expect(trustConfig).toContain(':permission_request:0:0') }) - it('installs managed hooks + trust into a per-account self-contained home, not the shared mirror', () => { + it('installs managed hooks + trust into a per-account self-contained home, not the shared mirror', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') mkdirSync(systemCodexHome, { recursive: true }) writeFileSync(join(systemCodexHome, 'config.toml'), 'approval_policy = "on-request"\n', 'utf-8') @@ -86,7 +138,7 @@ describe('CodexHookService', () => { mkdirSync(perAccountHome, { recursive: true }) writeFileSync(join(perAccountHome, '.orca-managed-home'), 'account-1\n', 'utf-8') - const status = new CodexHookService().install(perAccountHome) + const status = await new CodexHookService().install(perAccountHome) expect(status.state).toBe('installed') // Hooks + trust land in THIS account's home. @@ -106,7 +158,7 @@ describe('CodexHookService', () => { expect(existsSync(join(systemCodexHome, 'hooks.json'))).toBe(false) }) - it('drops plugin manager metadata from runtime hooks.json during install', () => { + it('drops plugin manager metadata from runtime hooks.json during install', async () => { const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') mkdirSync(managedCodexHome, { recursive: true }) writeFileSync( @@ -122,7 +174,7 @@ describe('CodexHookService', () => { 'utf-8' ) - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') const hooksConfig = JSON.parse(readFileSync(join(managedCodexHome, 'hooks.json'), 'utf-8')) as { hooks: Record @@ -138,7 +190,7 @@ describe('CodexHookService', () => { // `cmd.exe /C` never sees the raw script path. it.skipIf(process.platform !== 'win32')( 'wraps the managed hook command when the profile path contains a space (#6078)', - () => { + async () => { const spaceHome = join(tmpdir(), 'orca home with spaces') mkdirSync(spaceHome, { recursive: true }) homedirMock.mockReturnValue(spaceHome) @@ -146,7 +198,7 @@ describe('CodexHookService', () => { const systemCodexHome = join(spaceHome, '.codex') mkdirSync(systemCodexHome, { recursive: true }) - const status = new CodexHookService().install() + const status = await new CodexHookService().install() expect(status.state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') @@ -168,7 +220,7 @@ describe('CodexHookService', () => { // plausible paths. Keep those rare cases on the encoded launcher from #6078. it.skipIf(process.platform !== 'win32')( 'keeps the encoded launcher when the profile path contains cmd metacharacters', - () => { + async () => { const metacharHome = join(tmpdir(), 'orca %ORCA_TEST% ^ home') mkdirSync(metacharHome, { recursive: true }) homedirMock.mockReturnValue(metacharHome) @@ -176,7 +228,7 @@ describe('CodexHookService', () => { const systemCodexHome = join(metacharHome, '.codex') mkdirSync(systemCodexHome, { recursive: true }) - const status = new CodexHookService().install() + const status = await new CodexHookService().install() expect(status.state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') @@ -199,8 +251,8 @@ describe('CodexHookService', () => { // speed that Codex 0.140's synchronous "Running hook" rows expose. it.skipIf(process.platform !== 'win32')( 'launches the managed .cmd directly when the profile path is cmd-safe', - () => { - const status = new CodexHookService().install() + async () => { + const status = await new CodexHookService().install() expect(status.state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') @@ -227,7 +279,7 @@ describe('CodexHookService', () => { it.skipIf(process.platform !== 'win32')( 'posts hook payloads via the curl-based managed script preserving UTF-8 and spaced metadata', async () => { - new CodexHookService().install() + await new CodexHookService().install() const scriptPath = join(homedir(), '.orca', 'agent-hooks', 'codex-hook.cmd') expect(existsSync(scriptPath)).toBe(true) @@ -297,7 +349,7 @@ describe('CodexHookService', () => { } ) - it('keeps hooks isolated by Orca userData instead of mutating system ~/.codex', () => { + it('keeps hooks isolated by Orca userData instead of mutating system ~/.codex', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') const existingSystemHooks = '{"hooks":{"Stop":[{"hooks":[{"command":"user-hook"}]}]}}\n' @@ -314,7 +366,7 @@ describe('CodexHookService', () => { throw new Error(`unexpected app.getPath(${name})`) }) process.env.ORCA_USER_DATA_PATH = devUserDataDir - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') getPathMock.mockImplementation((name: string) => { if (name === 'userData') { @@ -323,7 +375,7 @@ describe('CodexHookService', () => { throw new Error(`unexpected app.getPath(${name})`) }) process.env.ORCA_USER_DATA_PATH = prodUserDataDir - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') const devHooksPath = join(devUserDataDir, 'codex-runtime-home', 'home', 'hooks.json') const prodHooksPath = join(prodUserDataDir, 'codex-runtime-home', 'home', 'hooks.json') diff --git a/src/main/codex/hook-service-runtime-trust-repair.test.ts b/src/main/codex/hook-service-runtime-trust-repair.test.ts index ce8c660af67..5adc4b202b4 100644 --- a/src/main/codex/hook-service-runtime-trust-repair.test.ts +++ b/src/main/codex/hook-service-runtime-trust-repair.test.ts @@ -33,7 +33,7 @@ import { CodexHookService } from './hook-service' const homes = setupCodexHookHomes(homedirMock, getPathMock) describe('CodexHookService', () => { - it('removes managed trust entries when userData resolves through a symlink', () => { + it('removes managed trust entries when userData resolves through a symlink', async () => { const linkedUserDataDir = join(homes.tmpHome, 'linked-user-data') symlinkSync( homes.userDataDir, @@ -43,14 +43,14 @@ describe('CodexHookService', () => { process.env.ORCA_USER_DATA_PATH = linkedUserDataDir const service = new CodexHookService() - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const linkedManagedCodexHome = join(linkedUserDataDir, 'codex-runtime-home', 'home') const linkedHooksPath = join(linkedManagedCodexHome, 'hooks.json') let runtimeToml = readFileSync(join(linkedManagedCodexHome, 'config.toml'), 'utf-8') expect(runtimeToml).toContain(hookTrustHeader(`${linkedHooksPath}:permission_request:0:0`)) - const status = service.remove() + const status = await service.remove() expect(status.state).toBe('not_installed') runtimeToml = readFileSync(join(linkedManagedCodexHome, 'config.toml'), 'utf-8') @@ -58,9 +58,9 @@ describe('CodexHookService', () => { expect(runtimeToml).not.toContain(':stop:0:0') }) - it('removes legacy managed trust entries hashed before hook timeouts existed', () => { + it('removes legacy managed trust entries hashed before hook timeouts existed', async () => { const service = new CodexHookService() - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') const managedHooksPath = join(managedCodexHome, 'hooks.json') @@ -88,13 +88,13 @@ describe('CodexHookService', () => { 'utf-8' ) - expect(service.remove().state).toBe('not_installed') + expect((await service.remove()).state).toBe('not_installed') const runtimeToml = readFileSync(runtimeTomlPath, 'utf-8') expect(runtimeToml).not.toContain(':permission_request:0:0') }) - it('mirrors system Codex config while preserving runtime hook trust on hook install', () => { + it('mirrors system Codex config while preserving runtime hook trust on hook install', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') mkdirSync(systemCodexHome, { recursive: true }) writeFileSync(join(systemCodexHome, 'config.toml'), 'model = "system-model"\n', 'utf-8') @@ -114,7 +114,7 @@ describe('CodexHookService', () => { 'utf-8' ) - const status = new CodexHookService().install() + const status = await new CodexHookService().install() expect(status.state).toBe('installed') const trustConfig = readFileSync(join(managedCodexHome, 'config.toml'), 'utf-8') @@ -128,9 +128,9 @@ describe('CodexHookService', () => { it.skipIf(process.platform !== 'win32')( 'treats legacy forward-slash runtime trust keys as installed before canonicalizing on reinstall', - () => { + async () => { const service = new CodexHookService() - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') const managedHooksPath = join(managedCodexHome, 'hooks.json') @@ -154,7 +154,7 @@ describe('CodexHookService', () => { expect(legacyToml).toContain(legacyPermissionHeader) expect(service.getStatus().state).toBe('installed') - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const repairedToml = readFileSync(runtimeTomlPath, 'utf-8') expect(repairedToml).not.toContain(legacyPermissionHeader) @@ -163,13 +163,13 @@ describe('CodexHookService', () => { } ) - it('repairs duplicate managed PermissionRequest trust tables on restart install', () => { + it('repairs duplicate managed PermissionRequest trust tables on restart install', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') mkdirSync(systemCodexHome, { recursive: true }) writeFileSync(join(systemCodexHome, 'config.toml'), 'model = "system-model"\n', 'utf-8') const service = new CodexHookService() - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') const managedHooksPath = join(managedCodexHome, 'hooks.json') @@ -209,7 +209,7 @@ describe('CodexHookService', () => { // Why: preserving `enabled = false` is the repair contract; status can be // partial because the user-disabled managed hook remains disabled. - expect(['installed', 'partial']).toContain(service.install().state) + expect(['installed', 'partial']).toContain((await service.install()).state) const repairedToml = readFileSync(runtimeTomlPath, 'utf-8') expect(repairedToml.split(permissionRequestHeader)).toHaveLength(2) @@ -219,7 +219,7 @@ describe('CodexHookService', () => { expect(repairedToml).toContain('model = "system-model"') }) - it('preserves runtime-only project trust while honoring system project untrust', () => { + it('preserves runtime-only project trust while honoring system project untrust', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') mkdirSync(systemCodexHome, { recursive: true }) writeFileSync( @@ -247,7 +247,7 @@ describe('CodexHookService', () => { 'utf-8' ) - const status = new CodexHookService().install() + const status = await new CodexHookService().install() expect(status.state).toBe('installed') const trustConfig = readFileSync(join(managedCodexHome, 'config.toml'), 'utf-8') diff --git a/src/main/codex/hook-service-test-harness.ts b/src/main/codex/hook-service-test-harness.ts index 2893d3a12ba..f6a450a263d 100644 --- a/src/main/codex/hook-service-test-harness.ts +++ b/src/main/codex/hook-service-test-harness.ts @@ -7,6 +7,17 @@ import { getCodexExplicitHomeHookSourcePath, normalizeCodexHookSourcePath } from './config-toml-trust' +import { _internals as grantInternals } from './codex-hook-trust-grant' +import { _internals as rebaseInternals } from './codex-user-hook-trust-rebase' + +// Why (#16441): the grant/rebase sessions now run in-process instead of in a +// forked bundle that never existed under vitest. Without this stub these +// suites spawn the developer's real `codex app-server`, so they pass in CI +// (no codex installed) and fail on any machine that has one. Stand in for the +// missing binary so the fallback lane is exercised either way. +function stubMissingCodexBinary(): never { + throw Object.assign(new Error('spawn codex ENOENT'), { code: 'ENOENT' }) +} export type CodexHookHomes = { tmpHome: string @@ -14,6 +25,19 @@ export type CodexHookHomes = { } /** Mutable holder: fields are re-pointed at fresh temp dirs by the registered beforeEach. */ +/** Applies the stub above; for suites that build their own temp homes. */ +export function stubCodexTrustSessionsForTests(): void { + grantInternals.setGrantSessionRunner(stubMissingCodexBinary) + rebaseInternals.setSessionRunner(stubMissingCodexBinary) +} + +export function restoreCodexTrustSessionsForTests(): void { + grantInternals.setGrantSessionRunner(null) + grantInternals.resetDiagnostics() + rebaseInternals.setSessionRunner(null) + rebaseInternals.resetRetryState() +} + export function setupCodexHookHomes( homedirMock: Mock<() => string>, getPathMock: Mock<(name: string) => string> @@ -27,6 +51,7 @@ export function setupCodexHookHomes( previousUserDataPath = process.env.ORCA_USER_DATA_PATH process.env.ORCA_USER_DATA_PATH = homes.userDataDir homedirMock.mockReturnValue(homes.tmpHome) + stubCodexTrustSessionsForTests() getPathMock.mockImplementation((name: string) => { if (name === 'userData') { return homes.userDataDir @@ -36,6 +61,7 @@ export function setupCodexHookHomes( }) afterEach(() => { + restoreCodexTrustSessionsForTests() rmSync(homes.tmpHome, { recursive: true, force: true }) rmSync(homes.userDataDir, { recursive: true, force: true }) if (previousUserDataPath === undefined) { diff --git a/src/main/codex/hook-service-trust-grant.test.ts b/src/main/codex/hook-service-trust-grant.test.ts index 7814994c758..1b4ab0cd7ac 100644 --- a/src/main/codex/hook-service-trust-grant.test.ts +++ b/src/main/codex/hook-service-trust-grant.test.ts @@ -75,8 +75,8 @@ beforeEach(() => { }) afterEach(() => { - rebaseInternals.setSessionRunnerSync(null) - trustGrantInternals.setGrantSessionRunnerSync(null) + rebaseInternals.setSessionRunner(null) + trustGrantInternals.setGrantSessionRunner(null) trustGrantInternals.resetDiagnostics() codexAppServerCapabilityCache.clear() if (previousDisableTrustRpc === undefined) { @@ -123,7 +123,7 @@ function writeCodexLikeTrust(configPath: string, entries: CodexTrustEntry[]): vo function installCodexLikeGrantRunner(): ReturnType { const codexHash = (key: string): string => `sha256:codex-${parseTrustKey(key)?.eventLabel ?? 'unknown'}` - const runner = vi.fn((request: CodexHookTrustGrantRequest) => { + const runner = vi.fn(async (request: CodexHookTrustGrantRequest) => { const codexHome = request.invocation.env?.CODEX_HOME expect(codexHome).toBeTruthy() const entries: CodexTrustEntry[] = request.expectedTrustKeys.map((key) => { @@ -145,7 +145,7 @@ function installCodexLikeGrantRunner(): ReturnType { })) } }) - trustGrantInternals.setGrantSessionRunnerSync(runner) + trustGrantInternals.setGrantSessionRunner(runner) return runner } @@ -154,11 +154,11 @@ function prepareSystemHome(): void { } describe('CodexHookService app-server trust grant lane', () => { - it('treats Codex hashes as authoritative and records the verified grant', () => { + it('treats Codex hashes as authoritative and records the verified grant', async () => { prepareSystemHome() const runner = installCodexLikeGrantRunner() - const status = new CodexHookService().install() + const status = await new CodexHookService().install() expect(status.state).toBe('installed') expect(runner).toHaveBeenCalledTimes(1) @@ -177,15 +177,15 @@ describe('CodexHookService app-server trust grant lane', () => { expect(Object.keys(readCodexTrustGrantLedgerHome(managedHome)!.entries)).toHaveLength(8) }) - it('keeps config byte-stable and skips the session on a repeat ledger hit', () => { + it('keeps config byte-stable and skips the session on a repeat ledger hit', async () => { prepareSystemHome() const runner = installCodexLikeGrantRunner() const service = new CodexHookService() - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const managedHome = join(userDataDir, 'codex-runtime-home', 'home') const firstToml = readFileSync(join(managedHome, 'config.toml')) - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') expect(runner).toHaveBeenCalledTimes(1) // Why: each launch validates the binary stamp once; getStatus reuses the // just-verified grant instead of repeating PATH/version-manager scans. @@ -193,7 +193,7 @@ describe('CodexHookService app-server trust grant lane', () => { expect(readFileSync(join(managedHome, 'config.toml'))).toEqual(firstToml) }) - it('retries ledger-proven real-home trust cleanup after the hook is already gone', () => { + it('retries ledger-proven real-home trust cleanup after the hook is already gone', async () => { prepareSystemHome() const systemHome = join(tmpHome, '.codex') const hooksPath = join(systemHome, 'hooks.json') @@ -223,7 +223,7 @@ describe('CodexHookService app-server trust grant lane', () => { }) installCodexLikeGrantRunner() - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') expect(readHookTrustEntries(configPath).has(trustKey)).toBe(false) expect(readCodexTrustGrantLedgerHome(systemHome)).toBeNull() @@ -232,7 +232,7 @@ describe('CodexHookService app-server trust grant lane', () => { // Why: ordinary Windows CI tokens cannot create file symlinks without Developer Mode. it.skipIf(process.platform === 'win32')( 'keeps a real-home symlink and rebases later user trust during flag-off cleanup', - () => { + async () => { prepareSystemHome() const systemHome = join(tmpHome, '.codex') const hooksPath = join(systemHome, 'hooks.json') @@ -256,7 +256,7 @@ describe('CodexHookService app-server trust grant lane', () => { ) symlinkSync(targetPath, hooksPath) const operations: string[] = [] - rebaseInternals.setSessionRunnerSync((request) => { + rebaseInternals.setSessionRunner(async (request) => { operations.push(request.operation) if (request.operation === 'inspect-user-hook-trust') { return { @@ -273,7 +273,7 @@ describe('CodexHookService app-server trust grant lane', () => { }) installCodexLikeGrantRunner() - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') expect(lstatSync(hooksPath).isSymbolicLink()).toBe(true) expect(JSON.parse(readFileSync(targetPath, 'utf-8')).hooks.Stop).toEqual([ @@ -285,7 +285,7 @@ describe('CodexHookService app-server trust grant lane', () => { it.skipIf(process.platform === 'win32')( 'preserves restrictive real-home hooks permissions during flag-off cleanup', - () => { + async () => { prepareSystemHome() const hooksPath = join(tmpHome, '.codex', 'hooks.json') const material = getCodexManagedHookInstallMaterial() @@ -296,17 +296,17 @@ describe('CodexHookService app-server trust grant lane', () => { chmodSync(hooksPath, 0o600) installCodexLikeGrantRunner() - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') expect(statSync(hooksPath).mode & 0o777).toBe(0o600) } ) - it('does not accept a ledger hash after the recorded Codex binary stamp changes', () => { + it('does not accept a ledger hash after the recorded Codex binary stamp changes', async () => { prepareSystemHome() installCodexLikeGrantRunner() const service = new CodexHookService() - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const managedHome = join(userDataDir, 'codex-runtime-home', 'home') const ledger = readCodexTrustGrantLedgerHome(managedHome)! writeCodexTrustGrantLedgerHome(managedHome, { @@ -320,16 +320,16 @@ describe('CodexHookService app-server trust grant lane', () => { }) }) - it('upgrades self-computed trust in place without duplicate logical entries', () => { + it('upgrades self-computed trust in place without duplicate logical entries', async () => { prepareSystemHome() const service = new CodexHookService() process.env.ORCA_DISABLE_CODEX_TRUST_RPC = '1' - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const managedHome = join(userDataDir, 'codex-runtime-home', 'home') delete process.env.ORCA_DISABLE_CODEX_TRUST_RPC installCodexLikeGrantRunner() - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const upgraded = readFileSync(join(managedHome, 'config.toml'), 'utf-8') // Why: the legacy Windows fallback intentionally writes slash variants; // duplicate detection is about the normalized trust identity. @@ -350,7 +350,7 @@ describe('CodexHookService app-server trust grant lane', () => { expect(upgraded).toContain('sha256:codex-session_start') }) - it('leaves user trust byte-untouched while granting managed entries', () => { + it('leaves user trust byte-untouched while granting managed entries', async () => { prepareSystemHome() const managedHome = join(userDataDir, 'codex-runtime-home', 'home') mkdirSync(managedHome, { recursive: true }) @@ -362,35 +362,35 @@ describe('CodexHookService app-server trust grant lane', () => { writeFileSync(join(managedHome, 'config.toml'), `${userBlock}\n`) installCodexLikeGrantRunner() - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') expect(readFileSync(join(managedHome, 'config.toml'), 'utf-8')).toContain(userBlock) }) - it('keeps the forced fallback on self-computed writes', () => { + it('keeps the forced fallback on self-computed writes', async () => { prepareSystemHome() process.env.ORCA_DISABLE_CODEX_TRUST_RPC = '1' const runner = vi.fn() - trustGrantInternals.setGrantSessionRunnerSync(runner) + trustGrantInternals.setGrantSessionRunner(runner) const service = new CodexHookService() - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') expect(service.getStatus().state).toBe('installed') expect(runner).not.toHaveBeenCalled() expect(resolveCodexCommandMock).not.toHaveBeenCalled() }) - it('restores exact config bytes before fallback after a mutating RPC failure', () => { + it('restores exact config bytes before fallback after a mutating RPC failure', async () => { prepareSystemHome() const service = new CodexHookService() process.env.ORCA_DISABLE_CODEX_TRUST_RPC = '1' - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const managedHome = join(userDataDir, 'codex-runtime-home', 'home') const baseline = readFileSync(join(managedHome, 'config.toml')) delete process.env.ORCA_DISABLE_CODEX_TRUST_RPC rmSync(managedHome, { recursive: true, force: true }) trustGrantInternals.resetDiagnostics() - const runner = vi.fn((request: CodexHookTrustGrantRequest) => { + const runner = vi.fn(async (request: CodexHookTrustGrantRequest) => { const codexHome = request.invocation.env?.CODEX_HOME writeFileSync( join(codexHome!, 'config.toml'), @@ -398,9 +398,9 @@ describe('CodexHookService app-server trust grant lane', () => { ) throw new Error('transport failed after config/batchWrite') }) - trustGrantInternals.setGrantSessionRunnerSync(runner) + trustGrantInternals.setGrantSessionRunner(runner) - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') expect(runner).toHaveBeenCalledTimes(1) expect(readFileSync(join(managedHome, 'config.toml'))).toEqual(baseline) }) diff --git a/src/main/codex/hook-service-user-hook-mirroring.test.ts b/src/main/codex/hook-service-user-hook-mirroring.test.ts index d06fec0b3a7..f13953a6051 100644 --- a/src/main/codex/hook-service-user-hook-mirroring.test.ts +++ b/src/main/codex/hook-service-user-hook-mirroring.test.ts @@ -73,10 +73,10 @@ function markHookTrustDisabled(toml: string, header: string): string { } describe('CodexHookService', () => { - it('preserves mirrored user hooks when the system hooks file cannot be read', () => { + it('preserves mirrored user hooks when the system hooks file cannot be read', async () => { const service = new CodexHookService() const { systemHooksPath, managedHooksPath } = seedSystemUserHook('user-hook') - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const systemBefore = readFileSync(systemHooksPath, 'utf-8') const before = readFileSync(managedHooksPath, 'utf-8') @@ -84,7 +84,7 @@ describe('CodexHookService', () => { mkdirSync(systemHooksPath) for (const retry of [() => service.install(), () => service.refreshRuntimeUserHooks()]) { - expect(retry()).toMatchObject({ + expect(await retry()).toMatchObject({ state: 'error', detail: 'Could not read system Codex hooks.json' }) @@ -93,16 +93,16 @@ describe('CodexHookService', () => { rmSync(systemHooksPath, { recursive: true }) writeFileSync(systemHooksPath, systemBefore, 'utf-8') - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') expect(readRuntimeHookCommands(managedHooksPath)).toContain('user-hook') }) it.each(['absent', 'malformed'] as const)( 'rebuilds mirrored user hooks when the system source is %s', - (sourceState) => { + async (sourceState) => { const service = new CodexHookService() const { systemHooksPath, managedHooksPath } = seedSystemUserHook('stale-user-hook') - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') if (sourceState === 'absent') { rmSync(systemHooksPath) @@ -110,12 +110,12 @@ describe('CodexHookService', () => { writeFileSync(systemHooksPath, '{ not json', 'utf-8') } - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') expect(readRuntimeHookCommands(managedHooksPath)).not.toContain('stale-user-hook') } ) - it('mirrors trusted system user hook approvals into the runtime CODEX_HOME', () => { + it('mirrors trusted system user hook approvals into the runtime CODEX_HOME', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') mkdirSync(systemCodexHome, { recursive: true }) @@ -163,7 +163,7 @@ describe('CodexHookService', () => { 'utf-8' ) - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') const managedHooksPath = join(managedCodexHome, 'hooks.json') @@ -183,7 +183,7 @@ describe('CodexHookService', () => { expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:0:0`, true)) }) - it('runs managed PostToolUse status before mirrored user hooks', () => { + it('runs managed PostToolUse status before mirrored user hooks', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') mkdirSync(systemCodexHome, { recursive: true }) @@ -210,7 +210,7 @@ describe('CodexHookService', () => { 'utf-8' ) - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') const managedHooksPath = join(managedCodexHome, 'hooks.json') @@ -231,7 +231,7 @@ describe('CodexHookService', () => { expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:post_tool_use:0:0`, true)) }) - it('mirrors system user hook approvals when the system trust indices are stale', () => { + it('mirrors system user hook approvals when the system trust indices are stale', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') mkdirSync(systemCodexHome, { recursive: true }) @@ -272,7 +272,7 @@ describe('CodexHookService', () => { 'utf-8' ) - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') const managedHooksPath = join(managedCodexHome, 'hooks.json') @@ -284,7 +284,7 @@ describe('CodexHookService', () => { expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:1:0`, true)) }) - it('skips plugin-placeholder system hooks when mirroring into runtime CODEX_HOME', () => { + it('skips plugin-placeholder system hooks when mirroring into runtime CODEX_HOME', async () => { const pluginCommands = [ 'node "${CLAUDE_PLUGIN_ROOT}/scripts/on-stop.mjs"', 'node "${CLAUDE_PLUGIN_DATA}/scripts/on-stop.mjs"', @@ -340,7 +340,7 @@ describe('CodexHookService', () => { 'utf-8' ) - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') const managedHooksPath = join(managedCodexHome, 'hooks.json') @@ -368,7 +368,7 @@ describe('CodexHookService', () => { } }) - it('mirrors compact-event user hook approvals and disabled trust entries', () => { + it('mirrors compact-event user hook approvals and disabled trust entries', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') mkdirSync(systemCodexHome, { recursive: true }) @@ -409,7 +409,7 @@ describe('CodexHookService', () => { 'utf-8' ) - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') const managedHooksPath = join(managedCodexHome, 'hooks.json') @@ -430,7 +430,7 @@ describe('CodexHookService', () => { expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:post_compact:0:0`, true)) }) - it('removes runtime user hook trust after system approval is revoked', () => { + it('removes runtime user hook trust after system approval is revoked', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') mkdirSync(systemCodexHome, { recursive: true }) @@ -456,7 +456,7 @@ describe('CodexHookService', () => { ) const service = new CodexHookService() - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') const managedHooksPath = join(managedCodexHome, 'hooks.json') @@ -466,14 +466,14 @@ describe('CodexHookService', () => { ) writeFileSync(join(systemCodexHome, 'config.toml'), 'model = "system-model"\n', 'utf-8') - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const runtimeToml = readFileSync(join(managedCodexHome, 'config.toml'), 'utf-8') expect(runtimeToml).not.toContain(runtimeUserTrustHeader) expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:stop:0:0`)) }) - it('refreshes mirrored system user hooks when the system hooks file changes', () => { + it('refreshes mirrored system user hooks when the system hooks file changes', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') mkdirSync(systemCodexHome, { recursive: true }) @@ -486,7 +486,7 @@ describe('CodexHookService', () => { ) const service = new CodexHookService() - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') writeFileSync( systemHooksPath, @@ -495,7 +495,7 @@ describe('CodexHookService', () => { })}\n`, 'utf-8' ) - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const managedHooksPath = join(homes.userDataDir, 'codex-runtime-home', 'home', 'hooks.json') const runtimeHooks = JSON.parse(readFileSync(managedHooksPath, 'utf-8')) as { @@ -509,7 +509,7 @@ describe('CodexHookService', () => { expect(stopCommands).not.toContain('user-hook-old') }) - it('refreshes runtime user hooks without installing Orca-managed hooks', () => { + it('refreshes runtime user hooks without installing Orca-managed hooks', async () => { const systemCodexHome = join(homes.tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') mkdirSync(systemCodexHome, { recursive: true }) @@ -537,7 +537,7 @@ describe('CodexHookService', () => { ) const service = new CodexHookService() - expect(service.install().state).toBe('installed') + expect((await service.install()).state).toBe('installed') const managedCodexHome = join(homes.userDataDir, 'codex-runtime-home', 'home') const managedHooksPath = join(managedCodexHome, 'hooks.json') const runtimeTomlPath = join(managedCodexHome, 'config.toml') @@ -559,7 +559,7 @@ describe('CodexHookService', () => { 'utf-8' ) - const status = service.refreshRuntimeUserHooks() + const status = await service.refreshRuntimeUserHooks() expect(status.state).toBe('not_installed') expect(status.managedHooksPresent).toBe(false) diff --git a/src/main/codex/hook-service-wsl-runtime.test.ts b/src/main/codex/hook-service-wsl-runtime.test.ts index dbc51f39b35..8e892e69147 100644 --- a/src/main/codex/hook-service-wsl-runtime.test.ts +++ b/src/main/codex/hook-service-wsl-runtime.test.ts @@ -82,7 +82,7 @@ function expectedManagedCommand(scriptPath: string): string { } describe('Codex WSL runtime hook install', () => { - it('plans WSL hook files with Linux command and trust paths', () => { + it('plans WSL hook files with Linux command and trust paths', async () => { const runtimeHome = '\\\\wsl.localhost\\Ubuntu\\home\\alice\\.local\\share\\orca\\codex-runtime-home\\home' @@ -100,7 +100,7 @@ describe('Codex WSL runtime hook install', () => { }) }) - it('plans WSL hooks when the distro home is mounted on a Windows drive', () => { + it('plans WSL hooks when the distro home is mounted on a Windows drive', async () => { const runtimeHome = 'D:\\wsl-home\\.local\\share\\orca\\codex-runtime-home\\home' expect( @@ -121,7 +121,7 @@ describe('Codex WSL runtime hook install', () => { }) }) - it('uses WSL-canonical paths for hook commands and trust keys', () => { + it('uses WSL-canonical paths for hook commands and trust keys', async () => { const runtimeHome = '\\\\wsl.localhost\\Ubuntu\\home\\alias\\.local\\share\\orca\\codex-runtime-home\\home' const canonicalHome = '/home/alice/.local/share/orca/codex-runtime-home/home' @@ -141,7 +141,7 @@ describe('Codex WSL runtime hook install', () => { expect(plan?.configPath).toBe(pathWin32.join(runtimeHome, 'hooks.json')) }) - it('removes managed trust when the WSL canonical path changes', () => { + it('removes managed trust when the WSL canonical path changes', async () => { const plan = createTestPlan() writeFileSync(plan.configPath, '{"hooks":{}}\n', 'utf-8') writeFileSync(plan.tomlPath, '', 'utf-8') @@ -151,7 +151,7 @@ describe('Codex WSL runtime hook install', () => { commandScriptPath: '/old/home/.orca/agent-hooks/codex-hook.sh', trustConfigPath: '/old/home/hooks.json' } - expect(_internals.installManagedHooksIntoWslRuntime(oldPlan).state).toBe('installed') + expect((await _internals.installManagedHooksIntoWslRuntime(oldPlan)).state).toBe('installed') const oldCommand = expectedManagedCommand(oldPlan.commandScriptPath) const oldKey = computeTrustKey(getManagedTrustEntry(oldPlan, oldCommand)) @@ -160,7 +160,7 @@ describe('Codex WSL runtime hook install', () => { commandScriptPath: '/new/home/.orca/agent-hooks/codex-hook.sh', trustConfigPath: '/new/home/hooks.json' } - expect(_internals.installManagedHooksIntoWslRuntime(newPlan).state).toBe('installed') + expect((await _internals.installManagedHooksIntoWslRuntime(newPlan)).state).toBe('installed') const newCommand = expectedManagedCommand(newPlan.commandScriptPath) const newKey = computeTrustKey(getManagedTrustEntry(newPlan, newCommand)) const trustEntries = readHookTrustEntries(plan.tomlPath) @@ -171,7 +171,7 @@ describe('Codex WSL runtime hook install', () => { it.skipIf(process.platform === 'win32')( 'drains stdin when the WSL runtime script is missing', - () => { + async () => { const basePlan = createTestPlan() const plan = { ...basePlan, @@ -180,7 +180,7 @@ describe('Codex WSL runtime hook install', () => { writeFileSync(plan.configPath, '{"hooks":{}}\n', 'utf-8') writeFileSync(plan.tomlPath, '', 'utf-8') - expect(_internals.installManagedHooksIntoWslRuntime(plan).state).toBe('installed') + expect((await _internals.installManagedHooksIntoWslRuntime(plan)).state).toBe('installed') const installed = JSON.parse(readFileSync(plan.configPath, 'utf-8')) as HooksConfig const command = installed.hooks.UserPromptSubmit[0]?.hooks?.[0]?.command expect(command).toBe(expectedManagedCommand(plan.commandScriptPath)) @@ -193,20 +193,20 @@ describe('Codex WSL runtime hook install', () => { } ) - it('sweeps all managed WSL trust for disable or confirmed absence', () => { + it('sweeps all managed WSL trust for disable or confirmed absence', async () => { // Why: disable and confirmed absence intentionally pass []. Transient // unavailability must NOT use this path — last known-good trust remains. const plan = createTestPlan() writeFileSync(plan.configPath, '{"hooks":{}}\n', 'utf-8') writeFileSync(plan.tomlPath, '', 'utf-8') - expect(_internals.installManagedHooksIntoWslRuntime(plan).state).toBe('installed') + expect((await _internals.installManagedHooksIntoWslRuntime(plan)).state).toBe('installed') _internals.removeStaleWslRuntimeManagedHookTrustEntries(plan.tomlPath, []) expect(readHookTrustEntries(plan.tomlPath).size).toBe(0) }) - it('reconciles only current, conclusive WSL path settlements', () => { + it('reconciles only current, conclusive WSL path settlements', async () => { expect( _internals.getWslHookReconciliationAction({ settlement: { status: 'unavailable' }, @@ -272,7 +272,7 @@ describe('Codex WSL runtime hook install', () => { ).toBe('reinstall') }) - it('generates a POSIX hook that bridges WSL loopback failures through Windows curl', () => { + it('generates a POSIX hook that bridges WSL loopback failures through Windows curl', async () => { const script = _internals.getManagedScript('posix') expect(script).toContain('load_hook_endpoint()') expect(script).toContain('"set ORCA_AGENT_HOOK_TOKEN="*)') @@ -287,7 +287,7 @@ describe('Codex WSL runtime hook install', () => { it.skipIf(process.platform === 'win32')( 'refreshes stale hook coordinates from a Windows endpoint file', - () => { + async () => { const plan = createTestPlan() const root = dirname(plan.configPath) const endpointPath = join(root, 'endpoint.cmd') @@ -339,7 +339,7 @@ describe('Codex WSL runtime hook install', () => { it.skipIf(process.platform === 'win32')( 'uses the Windows curl discovered from the WSL PATH after loopback fails', - () => { + async () => { const plan = createTestPlan() const root = dirname(plan.configPath) const binDir = join(root, 'bin') @@ -378,7 +378,7 @@ describe('Codex WSL runtime hook install', () => { } ) - it('installs trusted WSL hooks and removes only Orca entries when disabled', () => { + it('installs trusted WSL hooks and removes only Orca entries when disabled', async () => { const plan = createTestPlan() const userCommand = '/bin/sh /home/alice/user-hook.sh' writeFileSync( @@ -408,7 +408,7 @@ describe('Codex WSL runtime hook install', () => { 'utf-8' ) - expect(_internals.installManagedHooksIntoWslRuntime(plan).state).toBe('installed') + expect((await _internals.installManagedHooksIntoWslRuntime(plan)).state).toBe('installed') const installed = JSON.parse(readFileSync(plan.configPath, 'utf-8')) as HooksConfig expect(Object.keys(installed.hooks).sort()).toEqual([...managedEvents].sort()) @@ -457,7 +457,7 @@ describe('Codex WSL runtime hook install app-server grant lane', () => { }) afterEach(() => { - trustGrantInternals.setGrantSessionRunnerSync(null) + trustGrantInternals.setGrantSessionRunner(null) trustGrantInternals.resetDiagnostics() codexAppServerCapabilityCache.clear() if (previousUserDataPath === undefined) { @@ -467,12 +467,12 @@ describe('Codex WSL runtime hook install app-server grant lane', () => { } }) - it('grants WSL managed trust through codex inside the distro instead of self-computed writes', () => { + it('grants WSL managed trust through codex inside the distro instead of self-computed writes', async () => { const plan = createTestPlan() writeFileSync(plan.configPath, '{"hooks":{}}\n', 'utf-8') writeFileSync(plan.tomlPath, '', 'utf-8') - const runner = vi.fn((request: CodexHookTrustGrantRequest) => { + const runner = vi.fn(async (request: CodexHookTrustGrantRequest) => { // Simulate codex's side: write trusted_hash blocks the way its config // writer would, then report the entries trusted. upsertHookTrustEntries( @@ -496,9 +496,9 @@ describe('Codex WSL runtime hook install app-server grant lane', () => { })) } }) - trustGrantInternals.setGrantSessionRunnerSync(runner) + trustGrantInternals.setGrantSessionRunner(runner) - expect(_internals.installManagedHooksIntoWslRuntime(plan).state).toBe('installed') + expect((await _internals.installManagedHooksIntoWslRuntime(plan)).state).toBe('installed') expect(runner).toHaveBeenCalledTimes(1) const request = runner.mock.calls[0]![0]! @@ -519,7 +519,7 @@ describe('Codex WSL runtime hook install app-server grant lane', () => { ) }) - it('keeps the unchanged self-computed lane when the WSL grant falls back', () => { + it('keeps the unchanged self-computed lane when the WSL grant falls back', async () => { const plan = createTestPlan() writeFileSync(plan.configPath, '{"hooks":{}}\n', 'utf-8') writeFileSync(plan.tomlPath, '', 'utf-8') @@ -527,9 +527,9 @@ describe('Codex WSL runtime hook install app-server grant lane', () => { const runner = vi.fn(() => { throw new Error('wsl.exe not reachable') }) - trustGrantInternals.setGrantSessionRunnerSync(runner) + trustGrantInternals.setGrantSessionRunner(runner) - expect(_internals.installManagedHooksIntoWslRuntime(plan).state).toBe('installed') + expect((await _internals.installManagedHooksIntoWslRuntime(plan)).state).toBe('installed') expect(runner).toHaveBeenCalledTimes(1) const command = expectedManagedCommand(plan.commandScriptPath) @@ -540,12 +540,12 @@ describe('Codex WSL runtime hook install app-server grant lane', () => { }) }) - it('uses the previous ledger to remove stale Codex hashes after a canonical path change', () => { + it('uses the previous ledger to remove stale Codex hashes after a canonical path change', async () => { const basePlan = createTestPlan() writeFileSync(basePlan.configPath, '{"hooks":{}}\n', 'utf-8') writeFileSync(basePlan.tomlPath, '', 'utf-8') let staleKeyExpectedRemoved: string | null = null - const runner = vi.fn((request: CodexHookTrustGrantRequest) => { + const runner = vi.fn(async (request: CodexHookTrustGrantRequest) => { if (staleKeyExpectedRemoved) { expect(readHookTrustEntries(basePlan.tomlPath).has(staleKeyExpectedRemoved)).toBe(false) } @@ -571,7 +571,7 @@ describe('Codex WSL runtime hook install app-server grant lane', () => { })) } }) - trustGrantInternals.setGrantSessionRunnerSync(runner) + trustGrantInternals.setGrantSessionRunner(runner) const oldPlan = { ...basePlan, @@ -579,7 +579,7 @@ describe('Codex WSL runtime hook install app-server grant lane', () => { trustConfigPath: '/old/home/hooks.json', linuxRuntimeHome: '/old/home' } - expect(_internals.installManagedHooksIntoWslRuntime(oldPlan).state).toBe('installed') + expect((await _internals.installManagedHooksIntoWslRuntime(oldPlan)).state).toBe('installed') const oldKey = computeTrustKey( getManagedTrustEntry(oldPlan, expectedManagedCommand(oldPlan.commandScriptPath)) ) @@ -591,7 +591,7 @@ describe('Codex WSL runtime hook install app-server grant lane', () => { trustConfigPath: '/new/home/hooks.json', linuxRuntimeHome: '/new/home' } - expect(_internals.installManagedHooksIntoWslRuntime(newPlan).state).toBe('installed') + expect((await _internals.installManagedHooksIntoWslRuntime(newPlan)).state).toBe('installed') const newKey = computeTrustKey( getManagedTrustEntry(newPlan, expectedManagedCommand(newPlan.commandScriptPath)) ) diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index 270906012b5..93dfaf12f41 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -73,7 +73,8 @@ import { snapshotCodexRuntimeHookTrustProvenance } from './hook-trust-promotion' import { grantManagedCodexHookTrust } from './codex-hook-trust-grant' -import { readCurrentCodexTrustGrantLedgerHome } from './codex-trust-grant-host' +import { runExclusivelyForCodexTrustConfig } from './codex-trust-config-mutation-queue' +import { readCurrentNativeCodexTrustGrantLedgerHome } from './codex-trust-grant-host' import { getCodexLedgerTrustedHash, readCodexTrustGrantLedgerHomeForReconciliation, @@ -585,7 +586,30 @@ function removeSystemManagedHookTrustEntries(systemHomePath: string, hooksJsonPa }) } -function cleanupLegacySystemManagedHooks(): void { +// Why (#16441): these sequences mutate the runtime config.toml *and* the +// system one — approval promotion, the system-config sync and the legacy sweep +// all touch ~/.codex/config.toml — so holding only the runtime lane still lets +// a real-home grant's capture->restore window swallow their writes. Lock order +// is always runtime-before-system; every other holder acquires it that way too. +function runExclusivelyForRuntimeAndSystemTrustConfig( + runtimeHomePath: string, + run: () => Promise +): Promise { + return runExclusivelyForCodexTrustConfig(getCodexConfigTomlPath(runtimeHomePath), () => + runExclusivelyForCodexTrustConfig(getSystemCodexConfigTomlPath(), run) + ) +} + +function cleanupLegacySystemManagedHooks(): Promise { + // Why: shares the real-home lane with ensureRealHomeCodexHookState — both + // capture, mutate and roll back the user's ~/.codex/config.toml. + return runExclusivelyForCodexTrustConfig( + getSystemCodexConfigTomlPath(), + sweepLegacySystemManagedHooks + ) +} + +async function sweepLegacySystemManagedHooks(): Promise { if (systemCodexHomeHookSweepSuppressed()) { return } @@ -650,7 +674,7 @@ function cleanupLegacySystemManagedHooks(): void { // Remove only stale Orca hook entries and preserve other managers' metadata. const hooksWritePath = resolveHooksJsonWritePath(legacyConfigPath) const previousMode = statSync(hooksWritePath).mode - mutateRealHomeHooksPreservingUserTrust({ + await mutateRealHomeHooksPreservingUserTrust({ sourcePath: legacyConfigPath, runtimeHomePath: systemHomePath, tomlPath: getSystemCodexConfigTomlPath(), @@ -717,9 +741,9 @@ function cleanupLegacyCodexProfileHooks(): void { } } -function cleanupLegacyManagedHookRepresentations(): void { +async function cleanupLegacyManagedHookRepresentations(): Promise { try { - cleanupLegacySystemManagedHooks() + await cleanupLegacySystemManagedHooks() cleanupLegacyCodexProfileHooks() } catch (error) { console.warn('[codex-hook-service] failed to clean legacy Codex hooks', error) @@ -859,9 +883,20 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { ].join('\n') } +// Why (#16441): the grant inside awaits a codex app-server session, so a +// concurrent pane launch could write this config.toml between this run's +// capture and its restore. One lane per file keeps the sequence atomic. function installManagedHooksIntoWslRuntime( plan: CodexWslRuntimeHookInstallPlan -): AgentHookInstallStatus { +): Promise { + return runExclusivelyForCodexTrustConfig(plan.tomlPath, () => + installManagedHooksIntoWslRuntimeExclusively(plan) + ) +} + +async function installManagedHooksIntoWslRuntimeExclusively( + plan: CodexWslRuntimeHookInstallPlan +): Promise { const config = readHooksJson(plan.configPath) if (!config) { return { @@ -924,7 +959,7 @@ function installManagedHooksIntoWslRuntime( trustEntries, previousLedgerHome ? [previousLedgerHome] : [] ) - const grant = grantManagedCodexHookTrust({ + const grant = await grantManagedCodexHookTrust({ runtimeHomePath, tomlPath: plan.tomlPath, managedCommand: command, @@ -1050,17 +1085,24 @@ export class CodexHookService { return generation } - installForRuntimeHome( + async installForRuntimeHome( runtimeHomePath: string | null | undefined, target?: CodexWslRuntimeHookTarget - ): AgentHookInstallStatus | null { + ): Promise { const generation = this.supersedeWslReconciliation(runtimeHomePath) let installedTrustConfigPath: string | null = null - // Why: JS is single-threaded, so the synchronous install below finishes - // before any async `wsl.exe` settlement callback runs — this flag is - // always set by the time the callback reads it. let installSucceeded = false - const onCanonicalPathSettled = (settlement: WslCanonicalPathSettlement): void => { + // Why: the install below now awaits a codex app-server session, so a + // settlement callback can land mid-install. This gate keeps reconciliation + // reading the finished install's flags, as it did when the install was + // synchronous and no callback could interleave with it. + let markPrimaryInstallSettled!: () => void + let reconciliationChain = new Promise((resolve) => { + markPrimaryInstallSettled = resolve + }) + const reconcileSettledWslCanonicalPath = async ( + settlement: WslCanonicalPathSettlement + ): Promise => { if (!runtimeHomePath) { return } @@ -1097,7 +1139,7 @@ export class CodexHookService { if (!resolvedPlan) { return } - const status = installManagedHooksIntoWslRuntime(resolvedPlan) + const status = await installManagedHooksIntoWslRuntime(resolvedPlan) if (status.state === 'error') { console.warn('[codex-hook-service] failed to reconcile WSL hook path', status.detail) return @@ -1105,6 +1147,13 @@ export class CodexHookService { installedTrustConfigPath = resolvedPlan.trustConfigPath installSucceeded = status.state === 'installed' } + const onCanonicalPathSettled = (settlement: WslCanonicalPathSettlement): void => { + const run = (): Promise => reconcileSettledWslCanonicalPath(settlement) + reconciliationChain = reconciliationChain.then(run, run) + void reconciliationChain.catch((error: unknown) => { + console.warn('[codex-hook-service] failed to reconcile WSL hook path', error) + }) + } const wslPlan = createCodexWslRuntimeHookInstallPlan( runtimeHomePath, target, @@ -1112,9 +1161,13 @@ export class CodexHookService { onCanonicalPathSettled ) installedTrustConfigPath = wslPlan?.trustConfigPath ?? null - const status = wslPlan ? installManagedHooksIntoWslRuntime(wslPlan) : null - installSucceeded = status?.state === 'installed' - return status + try { + const status = wslPlan ? await installManagedHooksIntoWslRuntime(wslPlan) : null + installSucceeded = status?.state === 'installed' + return status + } finally { + markPrimaryInstallSettled() + } } refreshRuntimeUserHooksForRuntimeHome( @@ -1169,7 +1222,7 @@ export class CodexHookService { // hashes or wrote fallback hashes. Re-resolving PATH here doubles sync launch work. const ledgerHome = recentGrantEntries === null - ? readCurrentCodexTrustGrantLedgerHome(runtimeHomePath, { kind: 'native' }) + ? readCurrentNativeCodexTrustGrantLedgerHome(runtimeHomePath) : null const recentGrantHashes = new Map() for (const entry of recentGrantEntries ?? []) { @@ -1273,7 +1326,16 @@ export class CodexHookService { // Why: runtimeHomePath defaults to the shared managed mirror, but a managed // account launching against its own self-contained CODEX_HOME passes that // per-account home so hooks.json/config.toml/trust land where codex reads. - install(runtimeHomePath: string = getOrcaManagedCodexHomePath()): AgentHookInstallStatus { + install( + runtimeHomePath: string = getOrcaManagedCodexHomePath() + ): Promise { + // Why: same lane as the grant it performs — see installManagedHooksIntoWslRuntime. + return runExclusivelyForRuntimeAndSystemTrustConfig(runtimeHomePath, () => + this.installExclusively(runtimeHomePath) + ) + } + + private async installExclusively(runtimeHomePath: string): Promise { const configPath = getConfigPath(runtimeHomePath) const scriptPath = getManagedScriptPath() // Why: must run before this install rewrites hooks.json/config.toml — @@ -1375,7 +1437,7 @@ export class CodexHookService { // then carry Codex's verbatim hashes into stale cleanup so it cannot // delete what Codex just wrote. Mirrored user trust keeps its existing // verbatim-carry lane either way. - const grant = grantManagedCodexHookTrust({ + const grant = await grantManagedCodexHookTrust({ runtimeHomePath, tomlPath, managedCommand: command, @@ -1410,12 +1472,7 @@ export class CodexHookService { } } snapshotCodexRuntimeHookTrustProvenance(runtimeHomePath) - try { - cleanupLegacySystemManagedHooks() - cleanupLegacyCodexProfileHooks() - } catch (error) { - console.warn('[codex-hook-service] failed to clean legacy Codex hooks', error) - } + await cleanupLegacyManagedHookRepresentations() return this.getStatusAfterInstall(recentGrantEntries, runtimeHomePath) } @@ -1535,7 +1592,15 @@ export class CodexHookService { refreshRuntimeUserHooks( runtimeHomePath: string = getOrcaManagedCodexHomePath() - ): AgentHookInstallStatus { + ): Promise { + return runExclusivelyForRuntimeAndSystemTrustConfig(runtimeHomePath, () => + this.refreshRuntimeUserHooksExclusively(runtimeHomePath) + ) + } + + private async refreshRuntimeUserHooksExclusively( + runtimeHomePath: string + ): Promise { const configPath = getConfigPath(runtimeHomePath) // Why: same as install() — capture in-Orca approvals before this refresh // rewrites the runtime files they are keyed against. @@ -1543,7 +1608,7 @@ export class CodexHookService { const config = readHooksJson(configPath) if (!config) { // Why: disabled launch prep once called remove(); preserve that legacy cleanup even when runtime hooks.json is malformed. - cleanupLegacyManagedHookRepresentations() + await cleanupLegacyManagedHookRepresentations() return { agent: 'codex', state: 'error', @@ -1592,17 +1657,23 @@ export class CodexHookService { } snapshotCodexRuntimeHookTrustProvenance(runtimeHomePath) - cleanupLegacyManagedHookRepresentations() + await cleanupLegacyManagedHookRepresentations() return this.getStatus(runtimeHomePath) } - remove(): AgentHookInstallStatus { + remove(): Promise { + return runExclusivelyForRuntimeAndSystemTrustConfig(getOrcaManagedCodexHomePath(), () => + this.removeExclusively() + ) + } + + private async removeExclusively(): Promise { const configPath = getConfigPath() const configExists = existsSync(configPath) const config = readHooksJson(configPath) if (!config) { // Why: a malformed hooks.json shouldn't strand old hooks in ~/.codex or the legacy profile after disabling. - cleanupLegacyManagedHookRepresentations() + await cleanupLegacyManagedHookRepresentations() return { agent: 'codex', state: 'error', @@ -1635,7 +1706,7 @@ export class CodexHookService { // Why: drop trust entries so config.toml doesn't accumulate dead [hooks.state] blocks across install/remove cycles. removeRuntimeManagedHookTrustEntries(configPath) - cleanupLegacyManagedHookRepresentations() + await cleanupLegacyManagedHookRepresentations() return this.getStatus() } diff --git a/src/main/codex/hook-trust-promotion.test.ts b/src/main/codex/hook-trust-promotion.test.ts index 0f65b73b515..56a772bc1f4 100644 --- a/src/main/codex/hook-trust-promotion.test.ts +++ b/src/main/codex/hook-trust-promotion.test.ts @@ -39,6 +39,10 @@ vi.mock('os', async (importOriginal) => { } }) +import { + restoreCodexTrustSessionsForTests, + stubCodexTrustSessionsForTests +} from './hook-service-test-harness' import { CodexHookService } from './hook-service' let tmpHome: string @@ -50,6 +54,7 @@ beforeEach(() => { userDataDir = mkdtempSync(join(tmpdir(), 'orca-codex-user-data-')) previousUserDataPath = process.env.ORCA_USER_DATA_PATH process.env.ORCA_USER_DATA_PATH = userDataDir + stubCodexTrustSessionsForTests() homedirMock.mockReturnValue(tmpHome) getPathMock.mockImplementation((name: string) => { if (name === 'userData') { @@ -60,6 +65,7 @@ beforeEach(() => { }) afterEach(() => { + restoreCodexTrustSessionsForTests() rmSync(tmpHome, { recursive: true, force: true }) rmSync(userDataDir, { recursive: true, force: true }) if (previousUserDataPath === undefined) { @@ -130,7 +136,7 @@ function readSystemToml(): string { } describe('codex hook trust write-back promotion', () => { - it('mirrors default-home trust when the .codex directory is a symlink', () => { + it('mirrors default-home trust when the .codex directory is a symlink', async () => { const targetHome = join(tmpHome, 'dotfiles-codex') mkdirSync(targetHome) symlinkSync(targetHome, systemCodexDir(), process.platform === 'win32' ? 'junction' : 'dir') @@ -141,7 +147,7 @@ describe('codex hook trust write-back promotion', () => { upsertHookTrustEntriesInContent('', [systemEntry]) ) - expect(new CodexHookService().install().state).toBe('installed') + expect((await new CodexHookService().install()).state).toBe('installed') const runtimeTrust = readHookTrustEntries(join(runtimeHomeDir(), 'config.toml')) expect(runtimeTrust.get(computeTrustKey(runtimeUserStopEntry()))?.trustedHash).toBe( @@ -149,10 +155,10 @@ describe('codex hook trust write-back promotion', () => { ) }) - it('keeps an in-Orca approval of a user hook across launches and promotes it to ~/.codex', () => { + it('keeps an in-Orca approval of a user hook across launches and promotes it to ~/.codex', async () => { writeSystemUserHook() const service = new CodexHookService() - service.install() + await service.install() // The mirrored user hook has no system trust yet, so no runtime trust // entry exists for it — Codex would show it as pending review. @@ -163,7 +169,7 @@ describe('codex hook trust write-back promotion', () => { simulateCodexApproval(runtimeUserStopEntry()) const approvedHash = computeTrustedHash(runtimeUserStopEntry()) - service.install() + await service.install() // Approval survives the relaunch instead of being wiped as stale… expect(readHookTrustEntries(runtimeTomlPath).get(approvalKey)?.trustedHash).toBe(approvedHash) @@ -177,15 +183,15 @@ describe('codex hook trust write-back promotion', () => { // Steady state: another launch with no external changes rewrites nothing. const systemTomlAfterPromotion = readSystemToml() const runtimeTomlAfterPromotion = readFileSync(runtimeTomlPath, 'utf-8') - service.install() + await service.install() expect(readSystemToml()).toBe(systemTomlAfterPromotion) expect(readFileSync(runtimeTomlPath, 'utf-8')).toBe(runtimeTomlAfterPromotion) }) - it('never promotes trust for the Orca-managed status hook into ~/.codex', () => { + it('never promotes trust for the Orca-managed status hook into ~/.codex', async () => { writeSystemUserHook() const service = new CodexHookService() - service.install() + await service.install() // Simulate Codex rewriting the managed Stop hook's trust entry (as an // approval after hash drift would). @@ -206,20 +212,20 @@ describe('codex hook trust write-back promotion', () => { { hash: 'sha256:codex-corrected-managed-hash' } ) - service.install() + await service.install() expect(readSystemToml()).not.toContain(managedCommand) expect(readSystemToml()).not.toContain('codex-corrected-managed-hash') }) - it('does not resurrect trust the user revoked in ~/.codex/config.toml', () => { + it('does not resurrect trust the user revoked in ~/.codex/config.toml', async () => { writeSystemUserHook() // Pre-trust the hook in the system config, as a terminal Codex session would. const systemTomlPath = join(systemCodexDir(), 'config.toml') writeFileSync(systemTomlPath, upsertHookTrustEntriesInContent('', [systemUserStopEntry()])) const service = new CodexHookService() - service.install() + await service.install() const runtimeTomlPath = join(runtimeHomeDir(), 'config.toml') const approvalKey = computeTrustKey(runtimeUserStopEntry()) @@ -227,19 +233,19 @@ describe('codex hook trust write-back promotion', () => { // User revokes in the system config; the runtime copy must not win. writeFileSync(systemTomlPath, '') - service.install() + await service.install() expect(readHookTrustEntries(runtimeTomlPath).get(approvalKey)).toBeUndefined() expect(readSystemToml()).not.toContain('[hooks.state.') }) - it('promotes an in-Orca disable of a mirrored user hook back to the system config', () => { + it('promotes an in-Orca disable of a mirrored user hook back to the system config', async () => { writeSystemUserHook() const systemTomlPath = join(systemCodexDir(), 'config.toml') writeFileSync(systemTomlPath, upsertHookTrustEntriesInContent('', [systemUserStopEntry()])) const service = new CodexHookService() - service.install() + await service.install() // User disables the hook via /hooks inside Orca-launched Codex. const runtimeTomlPath = join(runtimeHomeDir(), 'config.toml') @@ -251,7 +257,7 @@ describe('codex hook trust write-back promotion', () => { upsertHookTrustEntriesInContent(runtimeToml, [{ ...runtimeUserStopEntry(), enabled: false }]) ) - service.install() + await service.install() const systemState = readHookTrustEntries(systemTomlPath).get( computeTrustKey(systemUserStopEntry()) @@ -261,17 +267,17 @@ describe('codex hook trust write-back promotion', () => { expect(readHookTrustEntries(runtimeTomlPath).get(approvalKey)?.enabled).toBe(false) }) - it('carries a Codex-written hash verbatim when it differs from the reproduced hash', () => { + it('carries a Codex-written hash verbatim when it differs from the reproduced hash', async () => { // Simulates Codex changing its trust hash algorithm: the approval hash in // the runtime config no longer matches computeTrustedHash's output. writeSystemUserHook() const service = new CodexHookService() - service.install() + await service.install() const driftedHash = 'sha256:codex-next-gen-hash-orca-cannot-reproduce' simulateCodexApproval(runtimeUserStopEntry(), { hash: driftedHash }) - service.install() + await service.install() const runtimeTomlPath = join(runtimeHomeDir(), 'config.toml') const approvalKey = computeTrustKey(runtimeUserStopEntry()) @@ -283,24 +289,24 @@ describe('codex hook trust write-back promotion', () => { ).toBe(driftedHash) // And the launch after that still keeps it. - service.install() + await service.install() expect(readHookTrustEntries(runtimeTomlPath).get(approvalKey)?.trustedHash).toBe(driftedHash) }) - it('does not touch ~/.codex on the first launch after upgrading (no provenance yet)', () => { + it('does not touch ~/.codex on the first launch after upgrading (no provenance yet)', async () => { // Simulates an existing install: runtime home fully materialized by a // build without provenance snapshots, managed hooks only. const service = new CodexHookService() - service.install() + await service.install() rmSync(join(runtimeHomeDir(), '.orca-hook-trust-provenance.json'), { force: true }) - service.install() + await service.install() expect(readSystemToml()).toBe('') expect(existsSync(join(systemCodexDir(), 'config.toml'))).toBe(false) }) - it('re-promoting mirrored trust without provenance is a no-op on ~/.codex', () => { + it('re-promoting mirrored trust without provenance is a no-op on ~/.codex', async () => { // Existing install with a system-trusted user hook, upgraded to this // build: the mirrored runtime entry has no provenance, so promotion must // sit out this launch and leave the system config byte-identical. @@ -308,16 +314,16 @@ describe('codex hook trust write-back promotion', () => { const systemTomlPath = join(systemCodexDir(), 'config.toml') writeFileSync(systemTomlPath, upsertHookTrustEntriesInContent('', [systemUserStopEntry()])) const service = new CodexHookService() - service.install() + await service.install() rmSync(join(runtimeHomeDir(), '.orca-hook-trust-provenance.json'), { force: true }) const systemTomlBefore = readSystemToml() - service.install() + await service.install() expect(readSystemToml()).toBe(systemTomlBefore) }) - it('does not resurrect trust revoked in ~/.codex before the first provenance snapshot', () => { + it('does not resurrect trust revoked in ~/.codex before the first provenance snapshot', async () => { // Old build mirrored a system-trusted hook into the runtime home; the // user then revoked it in ~/.codex/config.toml and upgraded to this // build. The stale runtime mirror must not be mistaken for an approval. @@ -325,11 +331,11 @@ describe('codex hook trust write-back promotion', () => { const systemTomlPath = join(systemCodexDir(), 'config.toml') writeFileSync(systemTomlPath, upsertHookTrustEntriesInContent('', [systemUserStopEntry()])) const service = new CodexHookService() - service.install() + await service.install() rmSync(join(runtimeHomeDir(), '.orca-hook-trust-provenance.json'), { force: true }) writeFileSync(systemTomlPath, '') - service.install() + await service.install() expect(readSystemToml()).not.toContain('[hooks.state.') expect( @@ -339,21 +345,21 @@ describe('codex hook trust write-back promotion', () => { ).toBeUndefined() }) - it('does not flip a hook the user disabled in ~/.codex back to enabled after upgrading', () => { + it('does not flip a hook the user disabled in ~/.codex back to enabled after upgrading', async () => { // Old build mirrored the hook enabled=true; the user then set // enabled = false in ~/.codex/config.toml and upgraded to this build. writeSystemUserHook() const systemTomlPath = join(systemCodexDir(), 'config.toml') writeFileSync(systemTomlPath, upsertHookTrustEntriesInContent('', [systemUserStopEntry()])) const service = new CodexHookService() - service.install() + await service.install() rmSync(join(runtimeHomeDir(), '.orca-hook-trust-provenance.json'), { force: true }) writeFileSync( systemTomlPath, upsertHookTrustEntriesInContent('', [{ ...systemUserStopEntry(), enabled: false }]) ) - service.install() + await service.install() expect( readHookTrustEntries(systemTomlPath).get(computeTrustKey(systemUserStopEntry()))?.enabled @@ -365,13 +371,13 @@ describe('codex hook trust write-back promotion', () => { ).toBe(false) }) - it('promotes one approval to every identical system hook collapsed by deduping', () => { + it('promotes one approval to every identical system hook collapsed by deduping', async () => { writeSystemUserHook([USER_HOOK_COMMAND, USER_HOOK_COMMAND]) const service = new CodexHookService() - service.install() + await service.install() simulateCodexApproval(runtimeUserStopEntry()) - service.install() + await service.install() const systemTrust = readHookTrustEntries(join(systemCodexDir(), 'config.toml')) const approvedHash = computeTrustedHash(runtimeUserStopEntry()) @@ -379,16 +385,16 @@ describe('codex hook trust write-back promotion', () => { expect(systemTrust.get(computeTrustKey(systemUserStopEntry(1)))?.trustedHash).toBe(approvedHash) }) - it('skips promotion when the approved hook no longer exists in ~/.codex/hooks.json', () => { + it('skips promotion when the approved hook no longer exists in ~/.codex/hooks.json', async () => { writeSystemUserHook() const service = new CodexHookService() - service.install() + await service.install() simulateCodexApproval(runtimeUserStopEntry()) // User deletes the hook from their system hooks.json before relaunching. writeFileSync(join(systemCodexDir(), 'hooks.json'), JSON.stringify({ hooks: {} })) - service.install() + await service.install() expect(readSystemToml()).not.toContain('[hooks.state.') // The runtime copy of the deleted hook (and its approval) is cleaned up. @@ -398,10 +404,10 @@ describe('codex hook trust write-back promotion', () => { ).toBeUndefined() }) - it('promotes approvals recorded while status hooks are disabled (refresh path)', () => { + it('promotes approvals recorded while status hooks are disabled (refresh path)', async () => { writeSystemUserHook() const service = new CodexHookService() - service.refreshRuntimeUserHooks() + await service.refreshRuntimeUserHooks() // Without the managed status hook, the mirrored user hook sits at group 0. const refreshedRuntimeEntry: CodexTrustEntry = { @@ -411,7 +417,7 @@ describe('codex hook trust write-back promotion', () => { simulateCodexApproval(refreshedRuntimeEntry) const approvedHash = computeTrustedHash(refreshedRuntimeEntry) - service.refreshRuntimeUserHooks() + await service.refreshRuntimeUserHooks() expect( readHookTrustEntries(join(systemCodexDir(), 'config.toml')).get( diff --git a/src/main/codex/managed-home-shell-preflight.test.ts b/src/main/codex/managed-home-shell-preflight.test.ts index 38cd9b7d146..8cc519d3c79 100644 --- a/src/main/codex/managed-home-shell-preflight.test.ts +++ b/src/main/codex/managed-home-shell-preflight.test.ts @@ -35,7 +35,7 @@ describe('managed Codex shell preflight', () => { ).toBe(home) }) - it('accepts a marker-proven account home and installs only while hooks are enabled', () => { + it('accepts a marker-proven account home and installs only while hooks are enabled', async () => { const userDataPath = makeRoot() const home = join(userDataPath, 'codex-accounts', 'account-1', 'home') mkdirSync(home, { recursive: true }) @@ -50,12 +50,22 @@ describe('managed Codex shell preflight', () => { const env = { CODEX_HOME: home, ORCA_CODEX_HOME: home } expect( - prepareManagedCodexHomeBeforeShellLaunch({ userDataPath, hooksEnabled: true, env, install }) + await prepareManagedCodexHomeBeforeShellLaunch({ + userDataPath, + hooksEnabled: true, + env, + install + }) ).toMatchObject({ state: 'installed' }) expect(install).toHaveBeenCalledWith(home) expect( - prepareManagedCodexHomeBeforeShellLaunch({ userDataPath, hooksEnabled: false, env, install }) + await prepareManagedCodexHomeBeforeShellLaunch({ + userDataPath, + hooksEnabled: false, + env, + install + }) ).toBeNull() expect(install).toHaveBeenCalledTimes(1) }) diff --git a/src/main/codex/managed-home-shell-preflight.ts b/src/main/codex/managed-home-shell-preflight.ts index 35dd3a68a70..d94b31e23d7 100644 --- a/src/main/codex/managed-home-shell-preflight.ts +++ b/src/main/codex/managed-home-shell-preflight.ts @@ -77,12 +77,20 @@ export function resolveManagedCodexShellPreflightHome( return resolveAccountManagedHome(codexHome, userDataPath) } -export function prepareManagedCodexHomeBeforeShellLaunch(args: { +/** + * Shell-startup preflight for a managed CODEX_HOME. + * + * Async because the Codex install awaits an app-server trust-grant session + * in-process. The old lane forked that session through spawnSync purely to + * borrow an event loop; the CLI already has one, so awaiting here removes a + * whole ELECTRON_RUN_AS_NODE process from every managed-home shell launch. + */ +export async function prepareManagedCodexHomeBeforeShellLaunch(args: { env?: ShellPreflightEnvironment userDataPath: string hooksEnabled: boolean - install?: (runtimeHomePath: string) => AgentHookInstallStatus -}): AgentHookInstallStatus | null { + install?: (runtimeHomePath: string) => AgentHookInstallStatus | Promise +}): Promise { if (!args.hooksEnabled) { return null } diff --git a/src/main/codex/retained-codex-hook-state.test.ts b/src/main/codex/retained-codex-hook-state.test.ts index 642b8bc7476..4dcdb9dc9a0 100644 --- a/src/main/codex/retained-codex-hook-state.test.ts +++ b/src/main/codex/retained-codex-hook-state.test.ts @@ -13,11 +13,11 @@ function status(state: 'installed' | 'not_installed' | 'error'): AgentHookInstal } describe('retained Codex hook state', () => { - it('repairs Orca hooks before a retained shell can launch Codex', () => { + it('repairs Orca hooks before a retained shell can launch Codex', async () => { const install = vi.fn(() => status('installed')) const refreshRuntimeUserHooks = vi.fn(() => status('not_installed')) - reconcileRetainedCodexHookHomes({ + await reconcileRetainedCodexHookHomes({ hookService: { install, refreshRuntimeUserHooks }, hooksEnabled: true, runtimeHomePaths: ['/orca/shared-home', '/orca/account-home'] @@ -29,11 +29,11 @@ describe('retained Codex hook state', () => { expect(refreshRuntimeUserHooks).not.toHaveBeenCalled() }) - it('removes only Orca hooks from retained homes when hooks are disabled', () => { + it('removes only Orca hooks from retained homes when hooks are disabled', async () => { const install = vi.fn(() => status('installed')) const refreshRuntimeUserHooks = vi.fn(() => status('not_installed')) - reconcileRetainedCodexHookHomes({ + await reconcileRetainedCodexHookHomes({ hookService: { install, refreshRuntimeUserHooks }, hooksEnabled: false, runtimeHomePaths: ['/orca/shared-home'] diff --git a/src/main/codex/retained-codex-hook-state.ts b/src/main/codex/retained-codex-hook-state.ts index ccdf8c86af3..f3e6304e390 100644 --- a/src/main/codex/retained-codex-hook-state.ts +++ b/src/main/codex/retained-codex-hook-state.ts @@ -1,20 +1,31 @@ import type { AgentHookInstallStatus } from '../../shared/agent-hook-types' type RetainedCodexHookService = { - install: (runtimeHomePath: string) => AgentHookInstallStatus - refreshRuntimeUserHooks: (runtimeHomePath: string) => AgentHookInstallStatus + install: (runtimeHomePath: string) => AgentHookInstallStatus | Promise + refreshRuntimeUserHooks: ( + runtimeHomePath: string + ) => AgentHookInstallStatus | Promise } -export function reconcileRetainedCodexHookHomes(args: { +/** + * Repairs the hook state of Codex homes that retained shells still point at. + * + * Why not on the startup critical path (#16441): each home can run a codex + * app-server trust-grant session, so N retained homes used to mean N sequential + * multi-second blocks before the first window could paint. Callers start this + * and move on — the repair only matters before a retained shell's next Codex + * invocation, which cannot happen until the daemon provider is already serving. + */ +export async function reconcileRetainedCodexHookHomes(args: { hookService: RetainedCodexHookService hooksEnabled: boolean runtimeHomePaths: readonly string[] -}): void { +}): Promise { for (const runtimeHomePath of args.runtimeHomePaths) { try { const status = args.hooksEnabled - ? args.hookService.install(runtimeHomePath) - : args.hookService.refreshRuntimeUserHooks(runtimeHomePath) + ? await args.hookService.install(runtimeHomePath) + : await args.hookService.refreshRuntimeUserHooks(runtimeHomePath) if (status.state === 'error') { console.warn('[codex-hook-service] failed to reconcile retained Codex home', status.detail) } diff --git a/src/main/index.ts b/src/main/index.ts index 0f7e44742a3..249700bcd16 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -1073,12 +1073,17 @@ function startTerminalRuntimeStartupServices(): WindowsDesktopStartupServices { if (livePtyIds) { reconcileCodexPaneAccountsWithLivePtys(livePtyIds) const settings = store?.getSettings() - reconcileRetainedCodexHookHomes({ + // Why (#16441): each retained home can run a codex app-server grant + // session. Awaiting them here delayed the first window by N sessions; + // a retained shell cannot invoke Codex before this provider serves. + void reconcileRetainedCodexHookHomes({ hookService: codexHookService, hooksEnabled: isAgentStatusHooksEnabled(settings) && settings?.disabledTuiAgents.includes('codex') !== true, runtimeHomePaths: codexRuntimeHome.getRetainedHostCodexHookHomePaths(livePtyIds) + }).catch((error: unknown) => { + console.warn('[codex-hook-service] retained Codex home reconcile failed:', error) }) } } @@ -1139,24 +1144,24 @@ function bindTerminalRuntimeStartupServices( localPtyProviderStartupReady = services.then((value) => value.localPtyProviderReady) } -function prepareCodexRuntimeHomeForLaunch( +async function prepareCodexRuntimeHomeForLaunch( target?: CodexAccountSelectionTarget, launchEnv?: NodeJS.ProcessEnv, launchContext?: CodexHomeLaunchContext -): string | null { +): Promise { if ( target?.runtime !== 'wsl' && launchContext?.launchAgent === 'codex' && launchContext.workspacePath ) { try { - // Why: renderer quick-launch cannot await trust IPC before its PTY mounts; launch prep runs synchronously before every recognized Codex spawn. - markCodexProjectTrusted(launchContext.workspacePath) + // Why: renderer quick-launch cannot await trust IPC before its PTY mounts; launch prep runs before every recognized Codex spawn. + await markCodexProjectTrusted(launchContext.workspacePath) } catch (error) { console.warn('[codex-project-trust] failed to pre-mark launch workspace:', error) } } - const ensureRealHomeHooksIfSelected = (): boolean => { + const ensureRealHomeHooksIfSelected = async (): Promise => { if ( target?.runtime === 'wsl' || !codexRuntimeHome!.isHostSystemDefaultRealHomeSelected(launchEnv) @@ -1167,13 +1172,13 @@ function prepareCodexRuntimeHomeForLaunch( // and trusted by codex's own app-server grant — in the real ~/.codex before // the pane spawns. An incapable grant flips the lane gate so the launch // below falls back to the managed home instead of a status-blind pane. - ensureRealHomeCodexHookState({ + await ensureRealHomeCodexHookState({ hooksEnabled: isAgentStatusHooksEnabled(store?.getSettings()), userDataPath: app.getPath('userData') }) return true } - let realHomeHooksPrepared = ensureRealHomeHooksIfSelected() + let realHomeHooksPrepared = await ensureRealHomeHooksIfSelected() // Why: a ManagedCodexHomeTemporarilyUnavailableError must escape uncaught — // the fallbacks below all key off `null`, which means "system default", so // swallowing the refusal would launch the wrong account (#STA-4422). @@ -1184,7 +1189,7 @@ function prepareCodexRuntimeHomeForLaunch( // Why: launch prep can reject an untrusted managed home and clear its // selection. Establish hook capability for that newly selected lane, then // re-resolve if the capability gate rejects it. - realHomeHooksPrepared = ensureRealHomeHooksIfSelected() + realHomeHooksPrepared = await ensureRealHomeHooksIfSelected() if (realHomeHooksPrepared) { runtimeHomePath = codexRuntimeHome!.prepareForCodexLaunch(target, launchEnv, { unavailableManagedHomePath: launchContext?.unavailableManagedHomePath @@ -1207,12 +1212,12 @@ function prepareCodexRuntimeHomeForLaunch( try { // Why: honor the persisted off switch so post-startup launches can't reinstall removed hooks. const status = hooksEnabled - ? (codexHookService.installForRuntimeHome(runtimeHomePath, hookTarget) ?? + ? ((await codexHookService.installForRuntimeHome(runtimeHomePath, hookTarget)) ?? // Why: a managed account's launch home is its own self-contained // CODEX_HOME, so hooks/trust must install there, not the shared mirror. - codexHookService.install(runtimeHomePath ?? undefined)) + (await codexHookService.install(runtimeHomePath ?? undefined))) : (codexHookService.refreshRuntimeUserHooksForRuntimeHome(runtimeHomePath, hookTarget) ?? - codexHookService.refreshRuntimeUserHooks(runtimeHomePath ?? undefined)) + (await codexHookService.refreshRuntimeUserHooks(runtimeHomePath ?? undefined))) if (status.state === 'error') { console.warn( `[codex-hook-service] failed to ${ @@ -1306,7 +1311,7 @@ async function prepareCodexSessionResumeForLaunch(args: { if (args.workspacePath) { try { - markCodexProjectTrusted(args.workspacePath) + await markCodexProjectTrusted(args.workspacePath) } catch (error) { console.warn('[codex-project-trust] failed to pre-mark resumed workspace:', error) } @@ -1317,11 +1322,14 @@ async function prepareCodexSessionResumeForLaunch(args: { const hooksEnabled = isAgentStatusHooksEnabled(settingsStore.getSettings()) try { if (isSystemHome) { - ensureRealHomeCodexHookState({ hooksEnabled, userDataPath: app.getPath('userData') }) + await ensureRealHomeCodexHookState({ + hooksEnabled, + userDataPath: app.getPath('userData') + }) } else if (hooksEnabled) { - codexHookService.install(resumeHome) + await codexHookService.install(resumeHome) } else { - codexHookService.refreshRuntimeUserHooks(resumeHome) + await codexHookService.refreshRuntimeUserHooks(resumeHome) } } catch (error) { // Why: hook repair is best-effort; session provenance must still win over the currently selected home. @@ -3060,30 +3068,40 @@ void app.whenReady().then(async () => { console.warn('[worktrees] Failed to sweep leftover worktree directories:', error) }) nativeTheme.themeSource = store.getSettings().theme ?? 'system' - if (codexRuntimeHome.isHostSystemDefaultRealHomeSelected()) { - // Why: establish capability before managed-hook reconciliation so an - // incapable host re-arms and completes the legacy real-home sweep now. - ensureRealHomeCodexHookState({ - hooksEnabled: isAgentStatusHooksEnabled(store.getSettings()), - userDataPath: app.getPath('userData') - }) - } + // Why (#16441): the real-home grant runs a codex app-server session. It stays + // ordered before managed-hook reconciliation — an incapable host must re-arm + // and complete the legacy real-home sweep first — but awaiting it inline + // stalled app init behind that session, so chain instead of blocking. + const realHomeCodexHookState = codexRuntimeHome.isHostSystemDefaultRealHomeSelected() + ? ensureRealHomeCodexHookState({ + hooksEnabled: isAgentStatusHooksEnabled(store.getSettings()), + userDataPath: app.getPath('userData') + }).catch((error: unknown) => { + console.warn('[codex-real-home-hooks] startup ensure failed:', error) + }) + : Promise.resolve() if (shouldInstallManagedHooks(is.dev)) { // Why: check the persisted off switch before any auto-install so removed hooks don't silently reappear on launch. if (isAgentStatusHooksEnabled(store.getSettings())) { const managedHookStore = store - void applyAgentStatusHooksEnabled(true, managedHookStore.getSettings(), { - shouldHydrateShellPath: app.isPackaged, - onInstallError: recordManagedHookInstallFailure, - shouldContinue: (agent) => { - const settings = managedHookStore.getSettings() - return shouldContinueManagedHookStartup(isQuitting, settings, agent) - } - }).catch((error) => { - console.warn('[agent-hooks] failed to reconcile managed hooks on startup:', error) - }) + void realHomeCodexHookState + .then(() => + applyAgentStatusHooksEnabled(true, managedHookStore.getSettings(), { + shouldHydrateShellPath: app.isPackaged, + onInstallError: recordManagedHookInstallFailure, + shouldContinue: (agent) => { + const settings = managedHookStore.getSettings() + return shouldContinueManagedHookStartup(isQuitting, settings, agent) + } + }) + ) + .catch((error: unknown) => { + console.warn('[agent-hooks] failed to reconcile managed hooks on startup:', error) + }) } else { - removeManagedAgentHooks() + void removeManagedAgentHooks().catch((error: unknown) => { + console.warn('[agent-hooks] failed to remove managed hooks on startup:', error) + }) } } // Why: process-gone metrics only see survivors; retain a recent whole-app diff --git a/src/main/ipc/pty/host-env/codex-home.ts b/src/main/ipc/pty/host-env/codex-home.ts index ae825e31e78..7abde336181 100644 --- a/src/main/ipc/pty/host-env/codex-home.ts +++ b/src/main/ipc/pty/host-env/codex-home.ts @@ -114,8 +114,10 @@ type ManagedCodexAuthResolutionArgs = { getSettings: () => GlobalSettings | undefined requiredCodexHomePath?: string target: CodexAccountSelectionTarget - resolveCurrent: () => string | null - resolveAfterUnavailable: (unavailableManagedHomePath: string) => string | null + resolveCurrent: () => string | null | Promise + resolveAfterUnavailable: ( + unavailableManagedHomePath: string + ) => string | null | Promise } export function resolveCodexHomeAfterManagedAuthReadiness( @@ -150,7 +152,7 @@ async function continueCodexHomeAfterManagedAuthWait( if (args.requiredCodexHomePath) { return selectedCodexHomePath } - const currentCodexHomePath = args.resolveCurrent() + const currentCodexHomePath = await args.resolveCurrent() if (codexHomeSelectionsEqual(selectedCodexHomePath, currentCodexHomePath)) { return selectedCodexHomePath } @@ -172,7 +174,7 @@ async function continueCodexHomeAfterManagedAuthWait( if (args.requiredCodexHomePath) { throw new Error(CODEX_RESUME_AUTH_UNAVAILABLE_MESSAGE) } - selectedCodexHomePath = args.resolveAfterUnavailable(selectedCodexHomePath!) + selectedCodexHomePath = await args.resolveAfterUnavailable(selectedCodexHomePath!) if (attempt === 1) { break } diff --git a/src/main/ipc/pty/host-env/codex-resume.ts b/src/main/ipc/pty/host-env/codex-resume.ts index a32aefe9057..62e199e09c3 100644 --- a/src/main/ipc/pty/host-env/codex-resume.ts +++ b/src/main/ipc/pty/host-env/codex-resume.ts @@ -103,14 +103,14 @@ export function resolveCodexResumeLaunch( }) } -export function reconcileSharedRuntimeResumeHome( +export async function reconcileSharedRuntimeResumeHome( resumeHome: Extract, - resolveCurrentHome: () => string | null -): string { + resolveCurrentHome: () => string | null | Promise +): Promise { if (!resumeHome.reconcileSharedRuntimeAuth) { return resumeHome.codexHomePath } - const currentHome = resolveCurrentHome() + const currentHome = await resolveCurrentHome() if (!codexHomePathsEqual(currentHome, resumeHome.codexHomePath)) { throw new Error(CODEX_RESUME_AUTH_UNAVAILABLE_MESSAGE) } diff --git a/src/main/ipc/pty/host-env/types.ts b/src/main/ipc/pty/host-env/types.ts index 61ff335b43d..298949d615a 100644 --- a/src/main/ipc/pty/host-env/types.ts +++ b/src/main/ipc/pty/host-env/types.ts @@ -39,11 +39,14 @@ export type CodexHomeLaunchContext = { unavailableManagedHomePath?: string } +// Why (#16441): Codex launch prep grants hook trust through a codex app-server +// session. It resolves asynchronously so the Electron main thread stays +// responsive; every consumer already runs inside an async spawn path. export type GetSelectedCodexHomePath = ( target?: CodexAccountSelectionTarget, launchEnv?: NodeJS.ProcessEnv, launchContext?: CodexHomeLaunchContext -) => string | null +) => string | null | Promise export type PrepareCodexSessionResume = (args: { providerSession: AgentProviderSessionMetadata diff --git a/src/main/ipc/pty/ipc/spawn-env-codex.ts b/src/main/ipc/pty/ipc/spawn-env-codex.ts index 655a848e980..42550f6732d 100644 --- a/src/main/ipc/pty/ipc/spawn-env-codex.ts +++ b/src/main/ipc/pty/ipc/spawn-env-codex.ts @@ -51,24 +51,23 @@ export async function assemblePtyIpcSpawnCodexEnv(ctx: PtyIpcSpawnState): Promis // Why: declared after the strip so a local-provider spawn cannot capture the // pre-strip env — only the daemon branch below re-derives this from baseEnv. ctx.env = ctx.baseEnv + const selectLaunchCodexHome = async (): Promise => + (await ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.baseEnv, { + workspacePath: ctx.cwd, + launchAgent: isTuiAgent(args.launchAgent) ? args.launchAgent : undefined + })) ?? null ctx.selectedCodexHomePath = !ctx.preAdoptedStablePane && !args.connectionId ? getCompatibleSelectedCodexHomePath( ctx.codexSelectionTarget, codexResumeHome - ? ctx.deps.reconcileSharedRuntimeResumeHome(codexResumeHome, () => + ? await ctx.deps.reconcileSharedRuntimeResumeHome(codexResumeHome, async () => getCompatibleSelectedCodexHomePath( ctx.codexSelectionTarget, - ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.baseEnv, { - workspacePath: ctx.cwd, - launchAgent: isTuiAgent(args.launchAgent) ? args.launchAgent : undefined - }) ?? null + await selectLaunchCodexHome() ) ) - : (ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.baseEnv, { - workspacePath: ctx.cwd, - launchAgent: isTuiAgent(args.launchAgent) ? args.launchAgent : undefined - }) ?? null) + : await selectLaunchCodexHome() ) : null if (!ctx.preAdoptedStablePane && args.launchAgent === 'codex' && args.sessionId === undefined) { @@ -77,22 +76,22 @@ export async function assemblePtyIpcSpawnCodexEnv(ctx: PtyIpcSpawnState): Promis getSettings: () => ctx.deps.getSettings?.(), requiredCodexHomePath: codexResumeHome?.codexHomePath, target: ctx.codexSelectionTarget, - resolveCurrent: () => + resolveCurrent: async () => getCompatibleSelectedCodexHomePath( ctx.codexSelectionTarget, - ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.baseEnv, { + (await ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.baseEnv, { workspacePath: ctx.cwd, launchAgent: 'codex' - }) ?? null + })) ?? null ), - resolveAfterUnavailable: (unavailableManagedHomePath) => + resolveAfterUnavailable: async (unavailableManagedHomePath) => getCompatibleSelectedCodexHomePath( ctx.codexSelectionTarget, - ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.baseEnv, { + (await ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.baseEnv, { workspacePath: ctx.cwd, launchAgent: 'codex', unavailableManagedHomePath - }) ?? null + })) ?? null ) }) ctx.selectedCodexHomePath = resolution instanceof Promise ? await resolution : resolution diff --git a/src/main/ipc/pty/ipc/spawn-types.ts b/src/main/ipc/pty/ipc/spawn-types.ts index 68e1cc62227..af5abe564ed 100644 --- a/src/main/ipc/pty/ipc/spawn-types.ts +++ b/src/main/ipc/pty/ipc/spawn-types.ts @@ -110,8 +110,8 @@ export type PtySpawnIpcDeps = { ) => Promise reconcileSharedRuntimeResumeHome: ( resumeHome: Extract, - resolveCurrent: () => string | null - ) => string + resolveCurrent: () => string | null | Promise + ) => Promise stripSequencedStartupResumeArgv: | undefined>( env: T, launch: CodexResumeLaunch diff --git a/src/main/ipc/pty/provider/local-configure.ts b/src/main/ipc/pty/provider/local-configure.ts index 2051d937c94..50f3c3e4016 100644 --- a/src/main/ipc/pty/provider/local-configure.ts +++ b/src/main/ipc/pty/provider/local-configure.ts @@ -38,7 +38,7 @@ export function configureLocalPtyProvider(args: { getWindowsPowerShellImplementation: () => getSettings ? (getSettings()?.terminalWindowsPowerShellImplementation ?? 'auto') : undefined, pwshAvailable: () => isPwshAvailableAsync(), - buildSpawnEnv: (id, baseEnv, ctx) => { + buildSpawnEnv: async (id, baseEnv, ctx) => { const codexSelectionTarget: CodexAccountSelectionTarget = ctx?.isWsl === true ? { runtime: 'wsl', wslDistro: ctx.wslDistro ?? null } @@ -47,10 +47,10 @@ export function configureLocalPtyProvider(args: { codexSelectionTarget, ctx?.codexHomePathOverride ? ctx.codexHomePathOverride.value - : (getSelectedCodexHomePath?.(codexSelectionTarget, baseEnv, { + : ((await getSelectedCodexHomePath?.(codexSelectionTarget, baseEnv, { workspacePath: ctx?.cwd, launchAgent: ctx?.launchAgent - }) ?? null) + })) ?? null) ) const skipCodexHomeEnv = ctx?.isWsl === true && !selectedCodexHomePath const ptySettings = getSettings?.() diff --git a/src/main/ipc/pty/runtime/controller-deps.ts b/src/main/ipc/pty/runtime/controller-deps.ts index 4abebc3b28c..da6c45ce5c5 100644 --- a/src/main/ipc/pty/runtime/controller-deps.ts +++ b/src/main/ipc/pty/runtime/controller-deps.ts @@ -40,8 +40,8 @@ export type PtyRuntimeControllerDeps = { noCodexResumeLaunch: (command: string | undefined) => CodexResumeLaunch reconcileSharedRuntimeResumeHome: ( resumeHome: Extract, - resolveCurrent: () => string | null - ) => string + resolveCurrent: () => string | null | Promise + ) => Promise stripSequencedStartupResumeArgv: | undefined>( env: T, launch: CodexResumeLaunch diff --git a/src/main/ipc/pty/runtime/spawn-preflight.ts b/src/main/ipc/pty/runtime/spawn-preflight.ts index 45c3ea1172f..f1ce7df7633 100644 --- a/src/main/ipc/pty/runtime/spawn-preflight.ts +++ b/src/main/ipc/pty/runtime/spawn-preflight.ts @@ -171,24 +171,23 @@ export async function prepareRuntimePtySpawn( if (args.preAllocatedHandle) { ctx.env = { ...ctx.env, ORCA_TERMINAL_HANDLE: args.preAllocatedHandle } } + const selectLaunchCodexHome = async (): Promise => + (await ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.env, { + workspacePath: ctx.cwd, + launchAgent: isTuiAgent(args.launchAgent) ? args.launchAgent : undefined + })) ?? null ctx.selectedCodexHomePath = !ctx.preAdoptedStablePane && !args.connectionId ? getCompatibleSelectedCodexHomePath( ctx.codexSelectionTarget, codexResumeHome - ? ctx.deps.reconcileSharedRuntimeResumeHome(codexResumeHome, () => + ? await ctx.deps.reconcileSharedRuntimeResumeHome(codexResumeHome, async () => getCompatibleSelectedCodexHomePath( ctx.codexSelectionTarget, - ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.env, { - workspacePath: ctx.cwd, - launchAgent: isTuiAgent(args.launchAgent) ? args.launchAgent : undefined - }) ?? null + await selectLaunchCodexHome() ) ) - : (ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.env, { - workspacePath: ctx.cwd, - launchAgent: isTuiAgent(args.launchAgent) ? args.launchAgent : undefined - }) ?? null) + : await selectLaunchCodexHome() ) : null if ( @@ -201,22 +200,22 @@ export async function prepareRuntimePtySpawn( getSettings: () => ctx.deps.getSettings?.(), requiredCodexHomePath: codexResumeHome?.codexHomePath, target: ctx.codexSelectionTarget, - resolveCurrent: () => + resolveCurrent: async () => getCompatibleSelectedCodexHomePath( ctx.codexSelectionTarget, - ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.env, { + (await ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.env, { workspacePath: ctx.cwd, launchAgent: 'codex' - }) ?? null + })) ?? null ), - resolveAfterUnavailable: (unavailableManagedHomePath) => + resolveAfterUnavailable: async (unavailableManagedHomePath) => getCompatibleSelectedCodexHomePath( ctx.codexSelectionTarget, - ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.env, { + (await ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.env, { workspacePath: ctx.cwd, launchAgent: 'codex', unavailableManagedHomePath - }) ?? null + })) ?? null ) }) ctx.selectedCodexHomePath = resolution instanceof Promise ? await resolution : resolution diff --git a/src/main/providers/local-pty-provider-spawn-session.test.ts b/src/main/providers/local-pty-provider-spawn-session.test.ts index b5ec1706225..c4ef90816ed 100644 --- a/src/main/providers/local-pty-provider-spawn-session.test.ts +++ b/src/main/providers/local-pty-provider-spawn-session.test.ts @@ -317,6 +317,30 @@ describe('LocalPtyProvider', () => { expect(spawnMock).toHaveBeenCalledOnce() }) + // Why (#16441): the Codex hook install and trust grant moved into + // buildSpawnEnv, so the env build is now the long await before node-pty + // exists — shutdown must be able to cancel the session id during it. + it('does not spawn after shutdown cancels a pending spawn during the env build', async () => { + spawnMock.mockClear() + let finishEnvBuild!: (env: Record) => void + const buildSpawnEnv = vi.fn( + (_id: string, baseEnv: Record) => + new Promise>((resolve) => { + finishEnvBuild = () => resolve(baseEnv) + }) + ) + const envProvider = new LocalPtyProvider({ buildSpawnEnv }) + + const spawn = envProvider.spawn({ cols: 80, rows: 24, sessionId: 'env-build-session' }) + const canceledSpawn = expect(spawn).rejects.toThrow('PTY spawn canceled: env-build-session') + await vi.waitFor(() => expect(buildSpawnEnv).toHaveBeenCalledOnce()) + + await envProvider.shutdown('env-build-session', { immediate: true }) + finishEnvBuild({}) + await canceledSpawn + expect(spawnMock).not.toHaveBeenCalled() + }) + it('coalesces a concurrent same-session-id spawn before launching a redundant shell (F3)', async () => { spawnMock.mockClear() const procA = { ...mockProc, pid: 1001 } diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index 4cdab4df4af..d8ddc7cd902 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -373,17 +373,19 @@ function allocatePtyId(sessionId: string | undefined): string { return id } -async function prepareLocalPtySpawn(id: string): Promise { +/** Awaits pre-launch work that shutdown must be able to cancel: no node-pty + * process exists yet, so cancellation can only be observed after the await. */ +async function awaitCancelableLocalPtySpawn(id: string, operation: T | Promise): Promise { const pendingSpawn: PendingLocalPtySpawn = { canceled: false } const pending = pendingLocalPtySpawns.get(id) ?? new Set() pending.add(pendingSpawn) pendingLocalPtySpawns.set(id, pending) try { - // Why: shutdown must be able to cancel a stable session id during the async macOS capability probe, before node-pty exists. - await prepareMacosTccLoginShell() + const result = await operation if (pendingSpawn.canceled) { throw new Error(`PTY spawn canceled: ${id}`) } + return result } finally { pending.delete(pendingSpawn) if (pending.size === 0) { @@ -534,7 +536,10 @@ export type LocalPtyProviderOptions = { isWsl?: boolean wslDistro?: string | null } - ) => Record + // Why (#16441): Codex launch prep grants hook trust through a codex + // app-server session. `spawn` already awaits, so returning a promise keeps + // the Electron main thread responsive instead of blocking on spawnSync. + ) => Record | Promise> /** Whether worktree-scoped shell history is enabled; when true (or absent) with a worktreeId, HISTFILE is scoped per-worktree. */ isHistoryEnabled?: () => boolean /** Why: COMSPEC is always cmd.exe, so this callback injects the user's persisted shell preference. Undefined when none set. */ @@ -743,16 +748,21 @@ export class LocalPtyProvider implements IPtyProvider { const isWslShell = Boolean(wslInfo) || pathWin32.basename(shellPath).toLowerCase() === 'wsl.exe' const launchWslDistro = isWslShell ? (launchWslContext?.distro ?? null) : null + // Why (#16441): building the env now awaits Codex hook installs and trust + // grants, so shutdown must be able to cancel this session id here too. const finalEnv = this.opts.buildSpawnEnv - ? this.opts.buildSpawnEnv(id, spawnEnv, { - command: args.command, - launchAgent: args.launchAgent, - codexHomePathOverride: args.codexHomePathOverride, - cwd, - shellPath, - isWsl: isWslShell, - wslDistro: launchWslDistro - }) + ? await awaitCancelableLocalPtySpawn( + id, + this.opts.buildSpawnEnv(id, spawnEnv, { + command: args.command, + launchAgent: args.launchAgent, + codexHomePathOverride: args.codexHomePathOverride, + cwd, + shellPath, + isWsl: isWslShell, + wslDistro: launchWslDistro + }) + ) : spawnEnv // Why: app-level env hooks can re-add scrubbed vars; delete last so shims like Claude Agent Teams keep their PATH. for (const key of args.envToDelete ?? []) { @@ -914,7 +924,8 @@ export class LocalPtyProvider implements IPtyProvider { primaryLaunchEnvKeys = Object.keys(shellLaunch.env) } - await prepareLocalPtySpawn(id) + // Why: the async macOS capability probe runs before node-pty exists. + await awaitCancelableLocalPtySpawn(id, prepareMacosTccLoginShell()) if (args.signal?.aborted) { throw new Error('client_disconnected') } diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 54c6b711173..32373ee98cc 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -24483,7 +24483,20 @@ export class OrcaRuntimeService { } } - private markLocalWorkspaceTrustedForAgent(agent: TuiAgent, workspacePath: string): void { + private markWorkspaceTrustedForAgent( + agent: TuiAgent, + connectionId: string | null | undefined, + workspacePath: string + ): Promise { + return connectionId + ? this.markRemoteWorkspaceTrustedForAgent(agent, connectionId, workspacePath) + : this.markLocalWorkspaceTrustedForAgent(agent, workspacePath) + } + + private async markLocalWorkspaceTrustedForAgent( + agent: TuiAgent, + workspacePath: string + ): Promise { const preset = TUI_AGENT_CONFIG[agent].preflightTrust if (!preset) { return @@ -24494,7 +24507,9 @@ export class OrcaRuntimeService { } else if (preset === 'copilot') { markCopilotFolderTrusted(workspacePath) } else if (preset === 'codex') { - markCodexProjectTrusted(workspacePath) + // Why: the Codex write queues behind any in-flight hook grant, so the + // agent must not launch until it has actually landed. + await markCodexProjectTrusted(workspacePath) } } catch { // Best-effort: the user can still accept the agent trust prompt manually. @@ -25019,7 +25034,7 @@ export class OrcaRuntimeService { try { const startupTrustAgent = effectiveDraftPaste?.agent ?? effectiveCreatedWithAgent if (startupTrustAgent) { - this.markLocalWorkspaceTrustedForAgent(startupTrustAgent, worktree.path) + await this.markLocalWorkspaceTrustedForAgent(startupTrustAgent, worktree.path) } const terminal = await this.createTerminal(`id:${worktree.id}`, { command: effectiveStartup.command, @@ -25812,7 +25827,7 @@ export class OrcaRuntimeService { // session later, matching `orca terminal create` background semantics. const startupTrustAgent = effectiveDraftPaste?.agent ?? effectiveCreatedWithAgent if (startupTrustAgent) { - this.markLocalWorkspaceTrustedForAgent(startupTrustAgent, worktreePath) + await this.markLocalWorkspaceTrustedForAgent(startupTrustAgent, worktreePath) } const terminal = await this.createTerminal(`id:${worktree.id}`, { command: sequencedStartup.command, @@ -28298,11 +28313,7 @@ export class OrcaRuntimeService { return opts } - if (workspace.connectionId) { - await this.markRemoteWorkspaceTrustedForAgent(agent, workspace.connectionId, workspace.path) - } else { - this.markLocalWorkspaceTrustedForAgent(agent, workspace.path) - } + await this.markWorkspaceTrustedForAgent(agent, workspace.connectionId, workspace.path) return { ...opts, @@ -28436,15 +28447,7 @@ export class OrcaRuntimeService { if (!startup) { throw new Error('agent_session_identity_required') } - if (workspace.connectionId) { - await this.markRemoteWorkspaceTrustedForAgent( - request.agent, - workspace.connectionId, - workspace.path - ) - } else { - this.markLocalWorkspaceTrustedForAgent(request.agent, workspace.path) - } + await this.markWorkspaceTrustedForAgent(request.agent, workspace.connectionId, workspace.path) if (_caller.signal?.aborted) { throw new Error('client_disconnected') } @@ -28605,15 +28608,7 @@ export class OrcaRuntimeService { if (!startup) { throw new Error('agent_session_identity_required') } - if (workspace.connectionId) { - await this.markRemoteWorkspaceTrustedForAgent( - request.agent, - workspace.connectionId, - workspace.path - ) - } else { - this.markLocalWorkspaceTrustedForAgent(request.agent, workspace.path) - } + await this.markWorkspaceTrustedForAgent(request.agent, workspace.connectionId, workspace.path) if (caller.signal?.aborted) { throw new Error('client_disconnected') } @@ -29245,11 +29240,7 @@ export class OrcaRuntimeService { throw new Error('Repository for the selected workspace is no longer available.') } const startup = this.buildStartupForAgent(repo, opts.agent, opts.prompt) - if (repo.connectionId) { - await this.markRemoteWorkspaceTrustedForAgent(opts.agent, repo.connectionId, worktree.path) - } else { - this.markLocalWorkspaceTrustedForAgent(opts.agent, worktree.path) - } + await this.markWorkspaceTrustedForAgent(opts.agent, repo.connectionId, worktree.path) return await this.createTerminal(`id:${worktree.id}`, { command: startup.startup.command, env: startup.startup.env, @@ -29638,15 +29629,7 @@ export class OrcaRuntimeService { if (opts.agentPrompt && startupPlan.followupPrompt) { throw new Error(`Agent ${opts.agent} does not support startup prompt quick commands.`) } - if (workspace.connectionId) { - await this.markRemoteWorkspaceTrustedForAgent( - opts.agent, - workspace.connectionId, - workspace.path - ) - } else { - this.markLocalWorkspaceTrustedForAgent(opts.agent, workspace.path) - } + await this.markWorkspaceTrustedForAgent(opts.agent, workspace.connectionId, workspace.path) return { command: startupPlan.launchCommand, env: startupPlan.env, diff --git a/src/main/text-generation/commit-message-agent-environment.ts b/src/main/text-generation/commit-message-agent-environment.ts index de83bcc3559..3c5523bc420 100644 --- a/src/main/text-generation/commit-message-agent-environment.ts +++ b/src/main/text-generation/commit-message-agent-environment.ts @@ -4,7 +4,9 @@ import { readShellStartupEnvVar } from '../pty/shell-startup-env' import { parseWslUncPath } from '../../shared/wsl-paths' export type CommitMessageAgentEnvironmentResolvers = { - prepareForCodexLaunch?: (target?: CommitMessageAgentRuntimeTarget) => string | null + prepareForCodexLaunch?: ( + target?: CommitMessageAgentRuntimeTarget + ) => string | null | Promise prepareForClaudeLaunch?: ( target?: CommitMessageAgentRuntimeTarget ) => Promise @@ -100,7 +102,7 @@ export async function prepareLocalCommitMessageAgentEnv( try { if (agentId === 'codex' && resolvers.prepareForCodexLaunch) { - const codexHomePath = resolvers.prepareForCodexLaunch(target) + const codexHomePath = await resolvers.prepareForCodexLaunch(target) const wslCodexHome = codexHomePath ? parseWslUncPath(codexHomePath) : null if (target?.runtime === 'wsl') { const codexHomeForTarget = wslCodexHome?.linuxPath ?? null diff --git a/src/shared/capability-probe-cache.ts b/src/shared/capability-probe-cache.ts new file mode 100644 index 00000000000..0a571dcf4da --- /dev/null +++ b/src/shared/capability-probe-cache.ts @@ -0,0 +1,128 @@ +/** + * Optimistic capability probing with a bounded retry window and in-flight + * probe dedupe. + * + * Extracted from GitCapabilityCache so every host-capability cache in the tree + * gets the same three behaviors: probe once, remember only a positive absence + * signal, and let a concurrent caller wait on the probe already running rather + * than starting a duplicate one. + */ +export type CapabilityProbeOutcome = 'supported' | 'unsupported' | 'unknown' + +export class CapabilityProbeCache { + private readonly retryAfterByCapability = new Map() + private readonly probesByCapability = new Map>() + private readonly supportedCapabilities = new Set() + + constructor(private readonly retryIntervalMs: number) {} + + shouldTry(capability: TCapability, nowMs = Date.now()): boolean { + const retryAfterMs = this.retryAfterByCapability.get(capability) + if (retryAfterMs === undefined) { + return true + } + if (nowMs < retryAfterMs) { + return false + } + this.retryAfterByCapability.delete(capability) + return true + } + + isKnownSupported(capability: TCapability): boolean { + return this.supportedCapabilities.has(capability) + } + + rememberSupported(capability: TCapability): void { + this.retryAfterByCapability.delete(capability) + this.supportedCapabilities.add(capability) + } + + rememberUnsupported(capability: TCapability, nowMs = Date.now()): void { + // Why: optimistic probes preserve newer behavior, but repeating a known + // failure on every poll/search wastes subprocesses and trace space. + this.supportedCapabilities.delete(capability) + this.retryAfterByCapability.set(capability, nowMs + this.retryIntervalMs) + } + + async runWithFallback( + capability: TCapability, + runPreferred: () => Promise, + runFallback: () => Promise, + isUnsupportedError: (error: unknown) => boolean + ): Promise { + if (this.supportedCapabilities.has(capability)) { + // Why: supported commands are real work, not disposable probes. Let + // sibling repo/SSH calls retain their intended concurrency. + return this.runPreferredOrFallback(capability, runPreferred, runFallback, isUnsupportedError) + } + if (!this.shouldTry(capability)) { + return runFallback() + } + + const inFlightProbe = this.probesByCapability.get(capability) + if (inFlightProbe) { + const outcome = await inFlightProbe + if (outcome === 'unsupported' || !this.shouldTry(capability)) { + return runFallback() + } + return this.runPreferredOrFallback(capability, runPreferred, runFallback, isUnsupportedError) + } + + let settleProbe!: (outcome: CapabilityProbeOutcome) => void + const probe = new Promise((resolve) => { + settleProbe = resolve + }) + this.probesByCapability.set(capability, probe) + try { + return await this.runPreferredOrFallback( + capability, + runPreferred, + runFallback, + isUnsupportedError, + settleProbe + ) + } finally { + if (this.probesByCapability.get(capability) === probe) { + this.probesByCapability.delete(capability) + } + // Backstop: `isUnsupportedError` or `rememberUnsupported` can throw + // before the settle below them runs; waiters must not hang behind it. + settleProbe('unknown') + } + } + + clear(): void { + this.retryAfterByCapability.clear() + this.probesByCapability.clear() + this.supportedCapabilities.clear() + } + + private async runPreferredOrFallback( + capability: TCapability, + runPreferred: () => Promise, + runFallback: () => Promise, + isUnsupportedError: (error: unknown) => boolean, + settleProbe?: (outcome: CapabilityProbeOutcome) => void + ): Promise { + try { + const result = await runPreferred() + // A preferred callback can detect a weaker positive signal (old Git's + // exit-zero option echo) and remember it as unsupported, so do not + // overwrite that stronger signal. + const outcome = this.retryAfterByCapability.has(capability) ? 'unsupported' : 'supported' + if (outcome === 'supported') { + this.supportedCapabilities.add(capability) + } + settleProbe?.(outcome) + return result + } catch (error) { + if (!isUnsupportedError(error)) { + settleProbe?.('unknown') + throw error + } + this.rememberUnsupported(capability) + settleProbe?.('unsupported') + return runFallback() + } + } +} diff --git a/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt b/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt index fcf8258d6b3..03e06eb374f 100644 --- a/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt +++ b/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt @@ -44,10 +44,8 @@ src/main/claude-accounts/keychain.ts src/main/codex-accounts/runtime-home-service.ts src/main/codex-accounts/service.ts src/main/codex/codex-app-server-client.ts -src/main/codex/codex-app-server-grant-bridge.ts src/main/codex/codex-app-server-session.ts src/main/codex/codex-state-db-backfill-recovery.ts -src/main/codex/codex-trust-grant-host.ts src/main/codex/codex-wsl-hook-install-plan.ts src/main/computer/desktop-script-provider-bridge.ts src/main/computer/macos-computer-use-permission-status.ts diff --git a/src/shared/git-capability-cache.ts b/src/shared/git-capability-cache.ts index 9e3d4ce89e0..06d0e957c74 100644 --- a/src/shared/git-capability-cache.ts +++ b/src/shared/git-capability-cache.ts @@ -1,3 +1,5 @@ +import { CapabilityProbeCache } from './capability-probe-cache' + // Why: suppress hot-loop failures while still detecting an in-place Git // upgrade during a long Orca session without requiring a restart. export const GIT_CAPABILITY_RETRY_INTERVAL_MS = 30 * 60_000 @@ -10,107 +12,8 @@ export type GitCapability = | 'rev-parse-path-format' | 'worktree-list-z' -type GitCapabilityProbeOutcome = 'supported' | 'unsupported' | 'unknown' - -export class GitCapabilityCache { - private readonly retryAfterByCapability = new Map() - private readonly probesByCapability = new Map>() - private readonly supportedCapabilities = new Set() - - shouldTry(capability: GitCapability, nowMs = Date.now()): boolean { - const retryAfterMs = this.retryAfterByCapability.get(capability) - if (retryAfterMs === undefined) { - return true - } - if (nowMs < retryAfterMs) { - return false - } - this.retryAfterByCapability.delete(capability) - return true - } - - rememberUnsupported(capability: GitCapability, nowMs = Date.now()): void { - // Why: optimistic probes preserve newer Git behavior, but repeating a - // known failure on every poll/search wastes subprocesses and trace space. - this.supportedCapabilities.delete(capability) - this.retryAfterByCapability.set(capability, nowMs + GIT_CAPABILITY_RETRY_INTERVAL_MS) - } - - async runWithFallback( - capability: GitCapability, - runPreferred: () => Promise, - runFallback: () => Promise, - isUnsupportedError: (error: unknown) => boolean - ): Promise { - if (this.supportedCapabilities.has(capability)) { - // Why: supported commands are real work, not disposable probes. Let - // sibling repo/SSH calls retain their intended concurrency. - return this.runPreferredOrFallback(capability, runPreferred, runFallback, isUnsupportedError) - } - if (!this.shouldTry(capability)) { - return runFallback() - } - - const inFlightProbe = this.probesByCapability.get(capability) - if (inFlightProbe) { - const outcome = await inFlightProbe - if (outcome === 'unsupported' || !this.shouldTry(capability)) { - return runFallback() - } - return this.runPreferredOrFallback(capability, runPreferred, runFallback, isUnsupportedError) - } - - let settleProbe!: (outcome: GitCapabilityProbeOutcome) => void - const probe = new Promise((resolve) => { - settleProbe = resolve - }) - this.probesByCapability.set(capability, probe) - try { - return await this.runPreferredOrFallback( - capability, - runPreferred, - runFallback, - isUnsupportedError, - settleProbe - ) - } finally { - if (this.probesByCapability.get(capability) === probe) { - this.probesByCapability.delete(capability) - } - } - } - - clear(): void { - this.retryAfterByCapability.clear() - this.probesByCapability.clear() - this.supportedCapabilities.clear() - } - - private async runPreferredOrFallback( - capability: GitCapability, - runPreferred: () => Promise, - runFallback: () => Promise, - isUnsupportedError: (error: unknown) => boolean, - settleProbe?: (outcome: GitCapabilityProbeOutcome) => void - ): Promise { - try { - const result = await runPreferred() - // A preferred callback can detect old Git's exit-zero option echo and - // remember it as unsupported, so do not overwrite that stronger signal. - const outcome = this.retryAfterByCapability.has(capability) ? 'unsupported' : 'supported' - if (outcome === 'supported') { - this.supportedCapabilities.add(capability) - } - settleProbe?.(outcome) - return result - } catch (error) { - if (!isUnsupportedError(error)) { - settleProbe?.('unknown') - throw error - } - this.rememberUnsupported(capability) - settleProbe?.('unsupported') - return runFallback() - } +export class GitCapabilityCache extends CapabilityProbeCache { + constructor() { + super(GIT_CAPABILITY_RETRY_INTERVAL_MS) } }