Review fixes: guarantee Codex launch approval promotion, dedupe installer loop, honest Windows-remote presence

- Launch-prep no longer drops #7896 approval promotion when the presence
  gate skips the managed install (false-'missing' PATH probe or relative
  command override). Promotion now runs on every Codex launch; only the
  managed-config write stays presence-gated. Guarded by a new predicate test.
- Remove the now-dead duplicated installer loop + errorStatus from
  install-telemetry (production uses the CLI-safe copy in controls); keep the
  electron-coupled recorder there and test it directly. Add fail-open loop
  coverage to controls where the loop actually lives.
- Relay Windows presence reports 'unknown' (honest) instead of a false
  'missing'; drop the unused 'unsupported_remote_platform' skip reason.

config/reliability-gates.jsonc

@@ -2596,7 +2596,7 @@
       "invariant": "Startup, Settings, runtime RPC, offline CLI, and SSH enable paths must not mutate third-party managed hook config without positive CLI presence or a resolvable explicit command override; disabling hooks remains ungated so stale Orca-managed hooks can be removed.",
       "oracle": "Presence-gated install calls only installers with found local or remote CLI evidence; missing or unknown CLIs return skipped statuses and do not write config; status reads remain config-authoritative; SSH all-missing/unknown presence avoids opening SFTP.",
       "commands": [
-        "pnpm exec vitest run --config config/vitest.config.ts src/main/agent-hooks/managed-agent-hook-manifest.test.ts src/main/agent-hooks/local-agent-cli-presence.test.ts src/main/agent-hooks/managed-agent-hook-controls.test.ts src/main/agent-hooks/install-telemetry.test.ts src/main/agent-hooks/remote-managed-hook-installers.test.ts src/main/ssh/ssh-relay-session-agent-hook-presence.test.ts src/relay/agent-hook-cli-presence.test.ts src/relay/agent-hook-cli-presence-timeout.test.ts src/cli/handlers/agent-hooks.test.ts src/main/runtime/rpc/methods/client-ui.test.ts src/main/runtime/orca-runtime.test.ts"
+        "pnpm exec vitest run --config config/vitest.config.ts src/main/agent-hooks/managed-agent-hook-manifest.test.ts src/main/agent-hooks/local-agent-cli-presence.test.ts src/main/agent-hooks/managed-agent-hook-controls.test.ts src/main/agent-hooks/install-telemetry.test.ts src/main/agent-hooks/remote-managed-hook-installers.test.ts src/main/ssh/ssh-relay-session-agent-hook-presence.test.ts src/relay/agent-hook-cli-presence.test.ts src/relay/agent-hook-cli-presence-timeout.test.ts src/cli/handlers/agent-hooks.test.ts src/main/runtime/rpc/methods/client-ui.test.ts src/main/runtime/orca-runtime.test.ts src/main/codex/codex-launch-hook-upkeep.test.ts"
       ],
       "testFiles": [
         "src/main/agent-hooks/managed-agent-hook-manifest.test.ts",
@@ -2609,7 +2609,8 @@
         "src/relay/agent-hook-cli-presence-timeout.test.ts",
         "src/cli/handlers/agent-hooks.test.ts",
         "src/main/runtime/rpc/methods/client-ui.test.ts",
-        "src/main/runtime/orca-runtime.test.ts"
+        "src/main/runtime/orca-runtime.test.ts",
+        "src/main/codex/codex-launch-hook-upkeep.test.ts"
       ],
       "assertionRefs": [
         {
@@ -2642,7 +2643,7 @@
           "date": "2026-07-05",
           "runner": "local",
           "platform": "macos",
-          "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/agent-hooks/managed-agent-hook-manifest.test.ts src/main/agent-hooks/local-agent-cli-presence.test.ts src/main/agent-hooks/managed-agent-hook-controls.test.ts src/main/agent-hooks/install-telemetry.test.ts src/main/agent-hooks/remote-managed-hook-installers.test.ts src/main/ssh/ssh-relay-session-agent-hook-presence.test.ts src/relay/agent-hook-cli-presence.test.ts src/relay/agent-hook-cli-presence-timeout.test.ts src/cli/handlers/agent-hooks.test.ts src/main/runtime/rpc/methods/client-ui.test.ts src/main/runtime/orca-runtime.test.ts",
+          "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/agent-hooks/managed-agent-hook-manifest.test.ts src/main/agent-hooks/local-agent-cli-presence.test.ts src/main/agent-hooks/managed-agent-hook-controls.test.ts src/main/agent-hooks/install-telemetry.test.ts src/main/agent-hooks/remote-managed-hook-installers.test.ts src/main/ssh/ssh-relay-session-agent-hook-presence.test.ts src/relay/agent-hook-cli-presence.test.ts src/relay/agent-hook-cli-presence-timeout.test.ts src/cli/handlers/agent-hooks.test.ts src/main/runtime/rpc/methods/client-ui.test.ts src/main/runtime/orca-runtime.test.ts src/main/codex/codex-launch-hook-upkeep.test.ts",
           "result": "passed",
           "durationSeconds": 12,
           "summary": "11 test files passed, 639 tests passed locally for managed hook CLI presence gating."

src/main/agent-hooks/install-telemetry.test.ts

@@ -1,7 +1,7 @@
-// Pins the contract between `runManagedHookInstallers` and the
-// `agent_hook_install_failed` telemetry event: each catch must fire `track`
-// with the correct agent label and a truncated error_message, and one
-// installer's failure must not stop the others (fail-open semantics).
+// Pins the contract of `recordManagedHookInstallFailure`: it fires the
+// `agent_hook_install_failed` telemetry event with the correct agent label and a
+// truncated error_message, and it swallows any throw from `track` so a broken
+// telemetry client can never break the fail-open installer loop that calls it.
 
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
@@ -9,9 +9,9 @@ const { trackMock } = vi.hoisted(() => ({ trackMock: vi.fn() }))
 
 vi.mock('../telemetry/client', () => ({ track: trackMock }))
 
-import { runManagedHookInstallers } from './install-telemetry'
+import { recordManagedHookInstallFailure } from './install-telemetry'
 
-describe('runManagedHookInstallers', () => {
+describe('recordManagedHookInstallFailure', () => {
   beforeEach(() => {
     trackMock.mockReset()
     vi.spyOn(console, 'error').mockImplementation(() => {})
@@ -20,27 +20,8 @@ describe('runManagedHookInstallers', () => {
     vi.restoreAllMocks()
   })
 
-  it('runs every installer when none throw and never calls track', () => {
-    const claude = vi.fn()
-    const codex = vi.fn()
-    runManagedHookInstallers([
-      ['claude', claude],
-      ['codex', codex]
-    ])
-    expect(claude).toHaveBeenCalledTimes(1)
-    expect(codex).toHaveBeenCalledTimes(1)
-    expect(trackMock).not.toHaveBeenCalled()
-  })
-
-  it('fires agent_hook_install_failed with the correct agent label when an installer throws', () => {
-    runManagedHookInstallers([
-      [
-        'codex',
-        () => {
-          throw new Error('codex config malformed')
-        }
-      ]
-    ])
+  it('fires agent_hook_install_failed with the correct agent label', () => {
+    recordManagedHookInstallFailure('codex', new Error('codex config malformed'))
 
     expect(trackMock).toHaveBeenCalledTimes(1)
     expect(trackMock).toHaveBeenCalledWith('agent_hook_install_failed', {
@@ -49,52 +30,17 @@ describe('runManagedHookInstallers', () => {
     })
   })
 
-  it('continues running later installers after an earlier one throws (fail-open)', () => {
-    const codex = vi.fn()
-    const gemini = vi.fn()
-    runManagedHookInstallers([
-      [
-        'claude',
-        () => {
-          throw new Error('claude failed')
-        }
-      ],
-      ['codex', codex],
-      ['gemini', gemini]
-    ])
-    expect(codex).toHaveBeenCalledTimes(1)
-    expect(gemini).toHaveBeenCalledTimes(1)
-    expect(trackMock).toHaveBeenCalledTimes(1)
-    expect(trackMock).toHaveBeenCalledWith(
-      'agent_hook_install_failed',
-      expect.objectContaining({ agent: 'claude' })
-    )
-  })
-
   it('truncates error_message to 200 chars', () => {
-    const longMessage = 'x'.repeat(500)
-    runManagedHookInstallers([
-      [
-        'gemini',
-        () => {
-          throw new Error(longMessage)
-        }
-      ]
-    ])
+    recordManagedHookInstallFailure('gemini', new Error('x'.repeat(500)))
+
     expect(trackMock).toHaveBeenCalledTimes(1)
     const [, props] = trackMock.mock.calls[0] as [string, { error_message: string }]
     expect(props.error_message.length).toBe(200)
   })
 
   it('handles non-Error throws', () => {
-    runManagedHookInstallers([
-      [
-        'cursor',
-        () => {
-          throw 'cursor string failure'
-        }
-      ]
-    ])
+    recordManagedHookInstallFailure('cursor', 'cursor string failure')
+
     expect(trackMock).toHaveBeenCalledWith('agent_hook_install_failed', {
       agent: 'cursor',
       error_message: 'cursor string failure'
@@ -102,14 +48,8 @@ describe('runManagedHookInstallers', () => {
   })
 
   it('serializes thrown objects through JSON.stringify', () => {
-    runManagedHookInstallers([
-      [
-        'cursor',
-        () => {
-          throw { code: 'EACCES', path: '/tmp' }
-        }
-      ]
-    ])
+    recordManagedHookInstallFailure('cursor', { code: 'EACCES', path: '/tmp' })
+
     expect(trackMock).toHaveBeenCalledTimes(1)
     expect(trackMock).toHaveBeenCalledWith('agent_hook_install_failed', {
       agent: 'cursor',
@@ -117,39 +57,20 @@ describe('runManagedHookInstallers', () => {
     })
   })
 
-  it('does not throw when an installer throws undefined (regression for JSON.stringify undefined return)', () => {
-    expect(() =>
-      runManagedHookInstallers([
-        [
-          'cursor',
-          () => {
-            throw undefined
-          }
-        ]
-      ])
-    ).not.toThrow()
+  it('does not throw on an undefined error (regression for JSON.stringify undefined)', () => {
+    expect(() => recordManagedHookInstallFailure('cursor', undefined)).not.toThrow()
     expect(trackMock).toHaveBeenCalledTimes(1)
     const [eventName, props] = trackMock.mock.calls[0] as [string, { error_message: string }]
     expect(eventName).toBe('agent_hook_install_failed')
     expect(typeof props.error_message).toBe('string')
   })
 
-  it('continues running later installers when track itself throws (telemetry must not break fail-open)', () => {
-    const codex = vi.fn()
+  it('swallows a throw from track so the caller stays fail-open', () => {
     trackMock.mockImplementationOnce(() => {
       throw new Error('telemetry blew up')
     })
     expect(() =>
-      runManagedHookInstallers([
-        [
-          'claude',
-          () => {
-            throw new Error('claude failed')
-          }
-        ],
-        ['codex', codex]
-      ])
+      recordManagedHookInstallFailure('claude', new Error('claude failed'))
     ).not.toThrow()
-    expect(codex).toHaveBeenCalledTimes(1)
   })
 })

src/main/agent-hooks/install-telemetry.ts

@@ -1,11 +1,10 @@
-// Per-agent managed-hook installer with fail-open semantics and PostHog
-// attribution. Lifted out of `src/main/index.ts` so the loop is unit-testable
-// without standing up the full Electron startup graph — the catch site needs
-// the agent label to fire `agent_hook_install_failed`, and the previous
-// closure-style loop lost it.
+// PostHog attribution for managed-hook install failures. Kept out of the
+// CLI-safe `managed-agent-hook-controls` module (and out of `src/main/index.ts`)
+// because `track` pulls in the Electron telemetry client: main-process callers
+// inject `recordManagedHookInstallFailure` into the installer loop, while the
+// offline CLI path leaves it unset so it never imports Electron.
 
 import type { HookInstallAgent } from '../../shared/telemetry-events'
-import type { AgentHookInstallStatus } from '../../shared/agent-hook-types'
 import { track } from '../telemetry/client'
 
 // Why: install errors are about config-file shape (malformed JSON, ACL
@@ -15,7 +14,6 @@ import { track } from '../telemetry/client'
 // so truncation must happen here at the call site.
 const ERROR_MESSAGE_MAX_LEN = 200
 
-export type ManagedHookInstaller = readonly [HookInstallAgent, () => AgentHookInstallStatus]
 export type ManagedHookInstallErrorRecorder = (agent: HookInstallAgent, error: unknown) => void
 
 function describeError(error: unknown): string {
@@ -33,20 +31,10 @@ function describeError(error: unknown): string {
   }
 }
 
-function errorStatus(agent: HookInstallAgent, error: unknown): AgentHookInstallStatus {
-  return {
-    agent,
-    state: 'error',
-    configPath: '',
-    managedHooksPresent: false,
-    detail: describeError(error)
-  }
-}
-
 export function recordManagedHookInstallFailure(agent: HookInstallAgent, error: unknown): void {
   // Why: telemetry must not break fail-open. A throw inside `track` (e.g.
   // a corrupted settings store the resolveConsent path reads from) would
-  // otherwise abort the for-loop and skip later agents' installers.
+  // otherwise abort the installer loop and skip later agents' installers.
   try {
     track('agent_hook_install_failed', {
       agent,
@@ -56,24 +44,3 @@ export function recordManagedHookInstallFailure(agent: HookInstallAgent, error:
     console.error('[agent-hooks] Failed to record install-failure telemetry:', telemetryError)
   }
 }
-
-export function runManagedHookInstallers(
-  installers: readonly ManagedHookInstaller[],
-  onInstallError: ManagedHookInstallErrorRecorder = recordManagedHookInstallFailure
-): AgentHookInstallStatus[] {
-  const results: AgentHookInstallStatus[] = []
-  for (const [agent, install] of installers) {
-    try {
-      results.push(install())
-    } catch (error) {
-      console.error(`[agent-hooks] Failed to install ${agent} managed hooks:`, error)
-      try {
-        onInstallError(agent, error)
-      } catch (telemetryError) {
-        console.error('[agent-hooks] Failed to record install-failure telemetry:', telemetryError)
-      }
-      results.push(errorStatus(agent, error))
-    }
-  }
-  return results
-}

src/main/agent-hooks/managed-agent-hook-controls.test.ts

@@ -1,4 +1,4 @@
-import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
 const { installCodex, installClaude, removeCodex, removeClaude, statusCodex, statusClaude } =
   vi.hoisted(() => ({
@@ -47,6 +47,8 @@ import {
 describe('managed agent hook controls', () => {
   beforeEach(() => {
     vi.clearAllMocks()
+    // Fail-open paths log to console.error on install failure; keep test output clean.
+    vi.spyOn(console, 'error').mockImplementation(() => {})
     installCodex.mockReturnValue({
       agent: 'codex',
       state: 'installed',
@@ -91,6 +93,10 @@ describe('managed agent hook controls', () => {
     })
   })
 
+  afterEach(() => {
+    vi.restoreAllMocks()
+  })
+
   it('installs only agents with launchable CLIs and returns skipped attempt results', async () => {
     detectLocalManagedAgentCliPresenceMock.mockResolvedValue({
       codex: { state: 'found' },
@@ -155,6 +161,47 @@ describe('managed agent hook controls', () => {
     expect(results).toEqual([expect.objectContaining({ agent: 'codex', state: 'installed' })])
   })
 
+  it('is fail-open: an installer that throws still runs the others and reports an error status', async () => {
+    detectLocalManagedAgentCliPresenceMock.mockResolvedValue({
+      codex: { state: 'found' },
+      claude: { state: 'found' }
+    })
+    installCodex.mockImplementation(() => {
+      throw new Error('codex config malformed')
+    })
+    const onInstallError = vi.fn()
+
+    const results = await installManagedAgentHooks({ agentCmdOverrides: {} }, { onInstallError })
+
+    expect(installClaude).toHaveBeenCalledTimes(1)
+    expect(onInstallError).toHaveBeenCalledWith('codex', expect.any(Error))
+    expect(results).toEqual([
+      expect.objectContaining({ agent: 'codex', state: 'error', detail: 'codex config malformed' }),
+      expect.objectContaining({ agent: 'claude', state: 'installed' })
+    ])
+  })
+
+  it('keeps installing later agents even when the error recorder itself throws', async () => {
+    detectLocalManagedAgentCliPresenceMock.mockResolvedValue({
+      codex: { state: 'found' },
+      claude: { state: 'found' }
+    })
+    installCodex.mockImplementation(() => {
+      throw new Error('codex failed')
+    })
+    const onInstallError = vi.fn(() => {
+      throw new Error('telemetry blew up')
+    })
+
+    await expect(
+      installManagedAgentHooks({ agentCmdOverrides: {} }, { onInstallError })
+    ).resolves.toEqual([
+      expect.objectContaining({ agent: 'codex', state: 'error' }),
+      expect.objectContaining({ agent: 'claude', state: 'installed' })
+    ])
+    expect(installClaude).toHaveBeenCalledTimes(1)
+  })
+
   it('keeps status reads config-authoritative instead of returning skipped states', () => {
     expect(getManagedAgentHookStatuses()).toEqual([
       expect.objectContaining({ agent: 'codex', state: 'installed' }),

src/main/codex/codex-launch-hook-upkeep.test.ts

@@ -0,0 +1,26 @@
+import { describe, expect, it } from 'vitest'
+import type { AgentHookInstallState } from '../../shared/agent-hook-types'
+import { codexLaunchNeedsDirectApprovalPromotion } from './codex-launch-hook-upkeep'
+
+function status(state: AgentHookInstallState): { state: AgentHookInstallState } {
+  return { state }
+}
+
+describe('codexLaunchNeedsDirectApprovalPromotion', () => {
+  it('promotes directly when the managed install skipped codex (presence-gated out)', () => {
+    // Regression: a false-'missing' PATH probe or relative override must not drop
+    // #7896 approval promotion on a real Codex launch.
+    expect(codexLaunchNeedsDirectApprovalPromotion(status('skipped'))).toBe(true)
+  })
+
+  it('promotes directly when no codex status came back at all', () => {
+    expect(codexLaunchNeedsDirectApprovalPromotion(undefined)).toBe(true)
+  })
+
+  it.each<AgentHookInstallState>(['installed', 'partial', 'error', 'not_installed'])(
+    'does not double-promote when install() already ran (state=%s)',
+    (state) => {
+      expect(codexLaunchNeedsDirectApprovalPromotion(status(state))).toBe(false)
+    }
+  )
+})

src/main/codex/codex-launch-hook-upkeep.ts

@@ -0,0 +1,15 @@
+import type { AgentHookInstallStatus } from '../../shared/agent-hook-types'
+
+// Why: the per-launch Codex path runs the presence-gated managed install, but
+// that install can be skipped (a false-'missing' PATH probe or a relative
+// command override reporting 'unknown'). #7896 approval promotion must still run
+// on every launch, so when the managed install did not run install() itself the
+// caller must promote directly. A completed install() (installed/partial/error)
+// already promoted before touching config, so promoting again would be wasted
+// work — and falling back to refreshRuntimeUserHooks() would wrongly strip the
+// managed hooks a false-'missing' can't see.
+export function codexLaunchNeedsDirectApprovalPromotion(
+  codexStatus: Pick<AgentHookInstallStatus, 'state'> | undefined
+): boolean {
+  return !codexStatus || codexStatus.state === 'skipped'
+}

src/main/codex/hook-service.ts

@@ -838,6 +838,14 @@ export class CodexHookService {
     return { agent: 'codex', state, configPath, managedHooksPresent, detail }
   }
 
+  // Why: exposes only the #7896 approval-promotion side effect so the per-launch
+  // path can guarantee it runs even when presence-gating skips the managed
+  // install. Promotion is idempotent, so callers that also run install() (which
+  // promotes internally) stay correct without double work.
+  promoteRuntimeHookApprovals(): void {
+    promoteCodexRuntimeHookApprovalsToSystem()
+  }
+
   install(): AgentHookInstallStatus {
     const configPath = getConfigPath()
     const scriptPath = getManagedScriptPath()

src/main/index.ts

@@ -36,6 +36,7 @@ import {
   removeManagedAgentHooks
 } from './agent-hooks/managed-agent-hook-controls'
 import { recordManagedHookInstallFailure } from './agent-hooks/install-telemetry'
+import type { AgentHookInstallStatus } from '../shared/agent-hook-types'
 import { initCohortClassifier } from './telemetry/cohort-classifier'
 import { initOnboardingCohortClassifier } from './telemetry/onboarding-cohort-classifier'
 import { resolveConsent } from './telemetry/consent'
@@ -122,6 +123,7 @@ import {
 } from './codex-accounts/runtime-selection'
 import { normalizeClaudeRuntimeSelection } from './claude-accounts/runtime-selection'
 import { codexHookService } from './codex/hook-service'
+import { codexLaunchNeedsDirectApprovalPromotion } from './codex/codex-launch-hook-upkeep'
 import { ClaudeAccountService } from './claude-accounts/service'
 import { ClaudeRuntimeAuthService } from './claude-accounts/runtime-auth-service'
 import {
@@ -699,20 +701,29 @@ async function startServeAgentHookServer(): Promise<void> {
 async function maintainCodexLaunchHooks(): Promise<void> {
   const hooksEnabled = isAgentStatusHooksEnabled(store?.getSettings())
   try {
-    const statuses = hooksEnabled
-      ? await installManagedAgentHooks(store?.getSettings(), {
-          shouldHydrateShellPath: app.isPackaged && process.platform !== 'win32',
-          onInstallError: recordManagedHookInstallFailure,
-          agents: ['codex']
-        })
-      : [codexHookService.refreshRuntimeUserHooks()]
-    const failedStatus = statuses.find((status) => status.state === 'error')
-    if (failedStatus) {
+    let launchStatus: AgentHookInstallStatus | undefined
+    if (hooksEnabled) {
+      const statuses = await installManagedAgentHooks(store?.getSettings(), {
+        shouldHydrateShellPath: app.isPackaged && process.platform !== 'win32',
+        onInstallError: recordManagedHookInstallFailure,
+        agents: ['codex']
+      })
+      launchStatus = statuses.find((status) => status.agent === 'codex')
+      // Why: presence-gating can skip the managed install; #7896 approval
+      // promotion must still run on every Codex launch, so promote directly when
+      // install() itself never ran (a completed install() already promoted).
+      if (codexLaunchNeedsDirectApprovalPromotion(launchStatus)) {
+        codexHookService.promoteRuntimeHookApprovals()
+      }
+    } else {
+      launchStatus = codexHookService.refreshRuntimeUserHooks()
+    }
+    if (launchStatus?.state === 'error') {
       console.warn(
         `[codex-hook-service] failed to ${
           hooksEnabled ? 'refresh' : 'refresh user'
         } runtime hooks before launch`,
-        failedStatus.detail
+        launchStatus.detail
       )
     }
   } catch (error) {

src/relay/agent-hook-cli-presence.ts

@@ -217,8 +217,13 @@ export async function detectAgentHookCliPresence(
   const request = parseRequest(params)
   const presence: AgentHookCliPresenceResponse['presence'] = {}
   if (process.platform === 'win32') {
+    // Why: the probe below is POSIX-only (login-shell PATH + X_OK), so on a
+    // Windows relay we can't determine presence — report 'unknown' rather than a
+    // false 'missing', which would wrongly claim the CLI is absent. Callers still
+    // skip install (only positive 'found' installs). Reachable only defensively:
+    // SSH sessions short-circuit Windows remotes before requesting presence.
     for (const agent of request.agents) {
-      presence[agent] = { state: 'missing' }
+      presence[agent] = { state: 'unknown' }
     }
     return { presence }
   }

src/shared/agent-hook-types.ts

@@ -28,7 +28,6 @@ export type AgentHookInstallSkipReason =
   | 'cli_presence_unknown'
   | 'hooks_disabled'
   | 'remote_presence_unavailable'
-  | 'unsupported_remote_platform'
   | 'remote_hook_unsupported'
 
 export type AgentHookInstallStatus = {