From f8b553b7d5f028c4e85d80855d72f343348cb05f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:19:18 -0700 Subject: [PATCH] fix(agent-hooks): skip unavailable agent homes (#11442) * fix(agent-hooks): skip unavailable agent homes * refactor(agent-hooks): separate Pi and OMP home fix * test(agent-hooks): update merged protocol harnesses * fix(agent-hooks): avoid redundant reconciliation * fix(agent-hooks): harden reconciliation and detection * test(agent-hooks): cover settings reconciliation * fix(agent-hooks): hydrate PATH for paired clients --- config/tsconfig.cli.json | 3 + src/cli/handlers/agent-hooks.ts | 20 +- .../agent-hooks/install-telemetry.test.ts | 146 ++--------- src/main/agent-hooks/install-telemetry.ts | 39 +-- .../local-agent-cli-presence.test.ts | 212 ++++++++++++++++ .../agent-hooks/local-agent-cli-presence.ts | 200 +++++++++++++++ .../managed-agent-hook-controls.test.ts | 151 +++++++++++ .../managed-agent-hook-controls.ts | 239 +++++++++++------- .../managed-agent-hook-registry.ts | 71 ++++++ .../managed-hook-detection-commands.test.ts | 24 ++ .../managed-hook-detection-commands.ts | 42 +++ .../managed-hook-local-filesystem.test.ts | 4 +- src/main/agent-hooks/managed-hook-runtime.ts | 5 +- .../remote-hook-service-installers.test.ts | 14 + .../remote-managed-hook-installers.ts | 8 +- src/main/agent-hooks/wsl-hook-fs-adapter.ts | 28 +- src/main/agent-hooks/wsl-hook-relay-deps.ts | 3 + .../wsl-hook-relay-live.integration.test.ts | 9 +- .../wsl-hook-relay-manager.test.ts | 59 +++-- .../agent-hooks/wsl-hook-relay-manager.ts | 18 +- .../agent-hooks/wsl-hook-relay-state-key.ts | 3 + src/main/index.ts | 17 +- src/main/ipc/settings.test.ts | 54 ++++ src/main/ipc/settings.ts | 26 +- src/main/runtime/orca-runtime.test.ts | 85 ++++++- src/main/runtime/orca-runtime.ts | 92 +++++-- src/main/runtime/rpc/methods/client-ui.ts | 4 +- .../ssh-relay-session-managed-hooks.test.ts | 131 ++++++++++ src/main/ssh/ssh-relay-session.test.ts | 59 ++--- src/main/ssh/ssh-relay-session.ts | 35 ++- src/relay/managed-hook-installer.test.ts | 46 +++- src/relay/managed-hook-installer.ts | 18 +- src/relay/wsl-agent-hook-relay.ts | 2 + src/shared/agent-hook-relay.ts | 3 + src/shared/agent-hook-types.ts | 9 +- .../managed-agent-command-token.test.ts | 43 ++++ src/shared/managed-agent-command-token.ts | 65 +++++ src/shared/managed-agent-hook-targets.ts | 32 +++ src/shared/tui-agent-selection.test.ts | 14 +- src/shared/tui-agent-selection.ts | 6 + 40 files changed, 1656 insertions(+), 383 deletions(-) create mode 100644 src/main/agent-hooks/local-agent-cli-presence.test.ts create mode 100644 src/main/agent-hooks/local-agent-cli-presence.ts create mode 100644 src/main/agent-hooks/managed-agent-hook-controls.test.ts create mode 100644 src/main/agent-hooks/managed-agent-hook-registry.ts create mode 100644 src/main/agent-hooks/managed-hook-detection-commands.test.ts create mode 100644 src/main/agent-hooks/managed-hook-detection-commands.ts create mode 100644 src/main/agent-hooks/wsl-hook-relay-state-key.ts create mode 100644 src/main/ssh/ssh-relay-session-managed-hooks.test.ts create mode 100644 src/shared/managed-agent-command-token.test.ts create mode 100644 src/shared/managed-agent-command-token.ts create mode 100644 src/shared/managed-agent-hook-targets.ts diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 60d08561403..7e972280832 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -9,7 +9,9 @@ "../src/main/agent-hooks/hooks-json-read.ts", "../src/main/agent-hooks/installer-utils.ts", "../src/main/agent-hooks/installer-utils-remote.ts", + "../src/main/agent-hooks/local-agent-cli-presence.ts", "../src/main/agent-hooks/managed-agent-hook-controls.ts", + "../src/main/agent-hooks/managed-agent-hook-registry.ts", "../src/main/amp/hook-service.ts", "../src/main/antigravity/hook-service.ts", "../src/main/claude/hook-settings.ts", @@ -68,6 +70,7 @@ "../src/main/kimi/kimi-hook-config-toml.ts", "../src/main/openclaude/hook-service.ts", "../src/main/rolling-file-backup.ts", + "../src/main/startup/hydrate-shell-path.ts", "../src/main/runtime/runtime-metadata.ts", "../src/main/win32-utils.ts" ], diff --git a/src/cli/handlers/agent-hooks.ts b/src/cli/handlers/agent-hooks.ts index 03f9c24c9f3..1eb5ed418eb 100644 --- a/src/cli/handlers/agent-hooks.ts +++ b/src/cli/handlers/agent-hooks.ts @@ -12,7 +12,7 @@ import { } from '../runtime-client' import type { AgentHookInstallStatus } from '../../shared/agent-hook-types' import { getDefaultPersistedState } from '../../shared/constants' -import type { PersistedState } from '../../shared/types' +import type { GlobalSettings, PersistedState } from '../../shared/types' import { applyAgentStatusHooksEnabled, getManagedAgentHookStatuses @@ -75,7 +75,10 @@ function readEnabledFromDisk(): boolean { return state.settings?.agentStatusHooksEnabled !== false } -function updateEnabledOnDisk(enabled: boolean): string { +function updateEnabledOnDisk(enabled: boolean): { + settingsPath: string + settings: Pick +} { const dataPath = getDataPath() const state = readPersistedState(dataPath) state.settings = { @@ -84,7 +87,13 @@ function updateEnabledOnDisk(enabled: boolean): string { agentStatusHooksEnabled: enabled } writePersistedState(dataPath, state) - return dataPath + return { + settingsPath: dataPath, + settings: { + agentCmdOverrides: state.settings.agentCmdOverrides ?? {}, + disabledTuiAgents: state.settings.disabledTuiAgents ?? [] + } + } } async function updateRunningRuntime(client: RuntimeClient, enabled: boolean): Promise { @@ -134,10 +143,11 @@ async function setAgentHooksEnabled( enabled: boolean ): Promise { const updatedRuntime = await updateRunningRuntime(client, enabled) - const settingsPath = updatedRuntime ? getDataPath() : updateEnabledOnDisk(enabled) + const offlineUpdate = updatedRuntime ? null : updateEnabledOnDisk(enabled) + const settingsPath = offlineUpdate?.settingsPath ?? getDataPath() const statuses = updatedRuntime ? getManagedAgentHookStatuses() - : applyAgentStatusHooksEnabled(enabled) + : await applyAgentStatusHooksEnabled(enabled, offlineUpdate?.settings) return { enabled, settingsPath, diff --git a/src/main/agent-hooks/install-telemetry.test.ts b/src/main/agent-hooks/install-telemetry.test.ts index 9af8a321d4f..e5f669bbe4d 100644 --- a/src/main/agent-hooks/install-telemetry.test.ts +++ b/src/main/agent-hooks/install-telemetry.test.ts @@ -1,155 +1,39 @@ -// 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). - import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' 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(() => {}) }) + afterEach(() => { 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('records the agent and truncated error message', () => { + recordManagedHookInstallFailure('codex', new Error('x'.repeat(500))) expect(trackMock).toHaveBeenCalledTimes(1) - expect(trackMock).toHaveBeenCalledWith('agent_hook_install_failed', { - agent: 'codex', - error_message: 'codex config malformed' - }) - }) - - 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) - } - ] - ]) - 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' - } - ] - ]) - expect(trackMock).toHaveBeenCalledWith('agent_hook_install_failed', { - agent: 'cursor', - error_message: 'cursor string failure' - }) - }) - - it('serializes thrown objects through JSON.stringify', () => { - runManagedHookInstallers([ - [ - 'cursor', - () => { - throw { code: 'EACCES', path: '/tmp' } - } - ] - ]) - expect(trackMock).toHaveBeenCalledTimes(1) - expect(trackMock).toHaveBeenCalledWith('agent_hook_install_failed', { - agent: 'cursor', - error_message: '{"code":"EACCES","path":"/tmp"}' - }) - }) - - it('does not throw when an installer throws undefined (regression for JSON.stringify undefined return)', () => { - expect(() => - runManagedHookInstallers([ - [ - 'cursor', - () => { - throw undefined - } - ] - ]) - ).not.toThrow() - expect(trackMock).toHaveBeenCalledTimes(1) - const [eventName, props] = trackMock.mock.calls[0] as [string, { error_message: string }] + const [eventName, props] = trackMock.mock.calls[0] as [ + string, + { agent: string; error_message: string } + ] expect(eventName).toBe('agent_hook_install_failed') - expect(typeof props.error_message).toBe('string') + expect(props.agent).toBe('codex') + expect(props.error_message).toHaveLength(200) }) - it('continues running later installers when track itself throws (telemetry must not break fail-open)', () => { - const codex = vi.fn() + it('handles non-Error values and telemetry failures', () => { trackMock.mockImplementationOnce(() => { - throw new Error('telemetry blew up') + throw new Error('telemetry failed') }) - expect(() => - runManagedHookInstallers([ - [ - 'claude', - () => { - throw new Error('claude failed') - } - ], - ['codex', codex] - ]) - ).not.toThrow() - expect(codex).toHaveBeenCalledTimes(1) + + expect(() => recordManagedHookInstallFailure('cursor', { code: 'EACCES' })).not.toThrow() }) }) diff --git a/src/main/agent-hooks/install-telemetry.ts b/src/main/agent-hooks/install-telemetry.ts index 658872f7039..be904a30bc8 100644 --- a/src/main/agent-hooks/install-telemetry.ts +++ b/src/main/agent-hooks/install-telemetry.ts @@ -1,21 +1,8 @@ -// 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. - import type { HookInstallAgent } from '../../shared/telemetry-events' import { track } from '../telemetry/client' -// Why: install errors are about config-file shape (malformed JSON, ACL -// denial), not user content — but messages can include paths or stack -// fragments. The 200-char cap matches `agentHookInstallFailedSchema.error_message` -// in `src/shared/telemetry-events.ts`; the validator drops overlength values, -// so truncation must happen here at the call site. const ERROR_MESSAGE_MAX_LEN = 200 -export type ManagedHookInstaller = readonly [HookInstallAgent, () => void] - function describeError(error: unknown): string { if (error instanceof Error) { return error.message @@ -31,23 +18,13 @@ function describeError(error: unknown): string { } } -export function runManagedHookInstallers(installers: readonly ManagedHookInstaller[]): void { - for (const [agent, install] of installers) { - try { - install() - } catch (error) { - console.error(`[agent-hooks] Failed to install ${agent} managed hooks:`, error) - // 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. - try { - track('agent_hook_install_failed', { - agent, - error_message: describeError(error).slice(0, ERROR_MESSAGE_MAX_LEN) - }) - } catch (telemetryError) { - console.error('[agent-hooks] Failed to record install-failure telemetry:', telemetryError) - } - } +export function recordManagedHookInstallFailure(agent: HookInstallAgent, error: unknown): void { + try { + track('agent_hook_install_failed', { + agent, + error_message: describeError(error).slice(0, ERROR_MESSAGE_MAX_LEN) + }) + } catch (telemetryError) { + console.error('[agent-hooks] Failed to record install-failure telemetry:', telemetryError) } } diff --git a/src/main/agent-hooks/local-agent-cli-presence.test.ts b/src/main/agent-hooks/local-agent-cli-presence.test.ts new file mode 100644 index 00000000000..15f474a9cc7 --- /dev/null +++ b/src/main/agent-hooks/local-agent-cli-presence.test.ts @@ -0,0 +1,212 @@ +import { chmodSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { ManagedAgentHookTarget } from '../../shared/managed-agent-hook-targets' +import { detectLocalManagedAgentCliPresence } from './local-agent-cli-presence' + +const codexTarget: ManagedAgentHookTarget = { + agent: 'codex', + tuiAgent: 'codex', + executableCandidates: ['codex'] +} + +const claudeTarget: ManagedAgentHookTarget = { + agent: 'claude', + tuiAgent: 'claude', + executableCandidates: ['claude'] +} + +describe('detectLocalManagedAgentCliPresence', () => { + let tmpDir: string | null = null + + afterEach(() => { + if (tmpDir) { + rmSync(tmpDir, { recursive: true, force: true }) + tmpDir = null + } + }) + + it('scans a deduped PATH once for all agent candidates', async () => { + const probe = vi.fn(async (filePath: string) => filePath.endsWith('/bin/codex')) + const result = await detectLocalManagedAgentCliPresence( + [codexTarget, claudeTarget], + { agentCmdOverrides: {} }, + { + pathEnv: ['/bin', '/bin', '/usr/bin'].join(':'), + pathDelimiter: ':', + fileProbe: { isExecutableFile: probe }, + platform: 'linux' + } + ) + + expect(result.codex?.state).toBe('found') + expect(result.claude?.state).toBe('missing') + expect(probe.mock.calls.map(([filePath]) => filePath)).toEqual([ + '/bin/codex', + '/bin/claude', + '/usr/bin/claude' + ]) + }) + + it('uses executable override paths as positive evidence', async () => { + const probe = vi.fn(async (filePath: string) => filePath === '/custom/bin/codex') + const result = await detectLocalManagedAgentCliPresence( + [codexTarget], + { agentCmdOverrides: { codex: '/custom/bin/codex --profile work' } }, + { + pathEnv: '', + pathDelimiter: ':', + fileProbe: { isExecutableFile: probe }, + platform: 'linux' + } + ) + + expect(result.codex?.state).toBe('found') + expect(probe).toHaveBeenCalledWith('/custom/bin/codex') + }) + + it('preserves Windows override separators', async () => { + const overridePath = 'C:\\My Tools\\claude.cmd' + const probe = vi.fn(async (filePath: string) => filePath === overridePath) + const result = await detectLocalManagedAgentCliPresence( + [claudeTarget], + { agentCmdOverrides: { claude: `"${overridePath}" --flag` } }, + { + pathEnv: '', + pathDelimiter: ';', + fileProbe: { isExecutableFile: probe }, + platform: 'win32' + } + ) + + expect(result.claude?.state).toBe('found') + expect(probe).toHaveBeenCalledWith(overridePath) + }) + + it('expands Windows home-relative override paths with Windows separators', async () => { + const overridePath = 'C:\\Users\\orca\\bin\\claude.cmd' + const probe = vi.fn(async (filePath: string) => filePath === overridePath) + const result = await detectLocalManagedAgentCliPresence( + [claudeTarget], + { agentCmdOverrides: { claude: '~\\bin\\claude.cmd --flag' } }, + { + pathEnv: '', + fileProbe: { isExecutableFile: probe }, + platform: 'win32', + homeDir: 'C:\\Users\\orca' + } + ) + + expect(result.claude?.state).toBe('found') + expect(probe).toHaveBeenCalledWith(overridePath) + }) + + it('expands home-relative override paths', async () => { + const probe = vi.fn(async (filePath: string) => filePath === '/home/orca/bin/codex') + const result = await detectLocalManagedAgentCliPresence( + [codexTarget], + { agentCmdOverrides: { codex: '~/bin/codex --profile work' } }, + { + pathEnv: '', + pathDelimiter: ':', + fileProbe: { isExecutableFile: probe }, + platform: 'linux', + homeDir: '/home/orca' + } + ) + + expect(result.codex?.state).toBe('found') + expect(probe).toHaveBeenCalledWith('/home/orca/bin/codex') + }) + + it('reports relative override paths as unknown', async () => { + const probe = vi.fn(async () => true) + const result = await detectLocalManagedAgentCliPresence( + [codexTarget], + { agentCmdOverrides: { codex: 'bin/codex --profile work' } }, + { + pathEnv: '', + pathDelimiter: ':', + fileProbe: { isExecutableFile: probe }, + platform: 'linux' + } + ) + + expect(result.codex).toEqual({ state: 'unknown' }) + expect(probe).not.toHaveBeenCalled() + }) + + it('honors PATHEXT for Windows PATH candidates', async () => { + const probe = vi.fn(async (filePath: string) => filePath === 'C:\\Tools\\codex.CMD') + const result = await detectLocalManagedAgentCliPresence( + [codexTarget], + { agentCmdOverrides: {} }, + { + pathEnv: 'C:\\Other;C:\\Tools', + pathExt: '.EXE;.CMD', + fileProbe: { isExecutableFile: probe }, + platform: 'win32' + } + ) + + expect(result.codex?.state).toBe('found') + expect(probe.mock.calls.map(([filePath]) => filePath)).toEqual([ + 'C:\\Other\\codex.EXE', + 'C:\\Other\\codex.CMD', + 'C:\\Tools\\codex.EXE', + 'C:\\Tools\\codex.CMD' + ]) + }) + + it('warns and uses the inherited PATH when shell hydration throws', async () => { + const warning = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + const result = await detectLocalManagedAgentCliPresence( + [codexTarget], + { agentCmdOverrides: {} }, + { + pathEnv: '', + pathDelimiter: ':', + fileProbe: { isExecutableFile: vi.fn(async () => false) }, + platform: 'linux', + shouldHydrateShellPath: true, + hydratePath: vi.fn(async () => { + throw new Error('shell unavailable') + }) + } + ) + + expect(result.codex?.state).toBe('missing') + expect(warning).toHaveBeenCalledWith( + '[agent-hooks] Shell PATH hydration failed; using inherited PATH:', + expect.objectContaining({ message: 'shell unavailable' }) + ) + } finally { + warning.mockRestore() + } + }) + + it.runIf(process.platform !== 'win32')( + 'accepts executable symlinks and rejects broken symlinks', + async () => { + tmpDir = mkdtempSync(join(tmpdir(), 'orca-cli-presence-')) + const binDir = join(tmpDir, 'bin') + mkdirSync(binDir) + const targetPath = join(tmpDir, 'codex-real') + writeFileSync(targetPath, '#!/bin/sh\n') + chmodSync(targetPath, 0o755) + symlinkSync(targetPath, join(binDir, 'codex')) + symlinkSync(join(tmpDir, 'missing'), join(binDir, 'claude')) + + const result = await detectLocalManagedAgentCliPresence( + [codexTarget, claudeTarget], + { agentCmdOverrides: {} }, + { pathEnv: binDir, pathDelimiter: ':', platform: process.platform } + ) + + expect(result.codex?.state).toBe('found') + expect(result.claude?.state).toBe('missing') + } + ) +}) diff --git a/src/main/agent-hooks/local-agent-cli-presence.ts b/src/main/agent-hooks/local-agent-cli-presence.ts new file mode 100644 index 00000000000..256e6520456 --- /dev/null +++ b/src/main/agent-hooks/local-agent-cli-presence.ts @@ -0,0 +1,200 @@ +import { constants } from 'node:fs' +import { access, stat } from 'node:fs/promises' +import { homedir } from 'node:os' +import path from 'node:path' +import type { AgentHookTarget } from '../../shared/agent-hook-types' +import { + extractExecutableToken, + hasPathSeparatorToken, + isSafeExecutableBasename +} from '../../shared/managed-agent-command-token' +import type { ManagedAgentHookTarget } from '../../shared/managed-agent-hook-targets' +import type { GlobalSettings } from '../../shared/types' +import { hydrateShellPath, mergePathSegments } from '../startup/hydrate-shell-path' + +export type LocalCliPresenceState = 'found' | 'missing' | 'unknown' +export type LocalCliPresenceByAgent = Partial< + Record +> + +type FileProbe = { + isExecutableFile: (filePath: string) => Promise +} + +type HydrationResult = + | { ok: true; segments: string[] } + | { ok: false; segments: []; failureReason?: string } + +type DetectOptions = { + pathEnv?: string + platform?: NodeJS.Platform + pathDelimiter?: string + pathExt?: string + fileProbe?: FileProbe + hydratePath?: () => Promise + shouldHydrateShellPath?: boolean + homeDir?: string +} + +type CommandOverrideSettings = Partial> | null | undefined + +const DEFAULT_WINDOWS_EXTENSIONS = ['.COM', '.EXE', '.BAT', '.CMD'] + +function pathApiForPlatform(platform: NodeJS.Platform) { + return platform === 'win32' ? path.win32 : path.posix +} + +async function isExecutableFile(filePath: string, platform: NodeJS.Platform): Promise { + try { + const fileStat = await stat(filePath) + if (!fileStat.isFile()) { + return false + } + if (platform === 'win32') { + return true + } + await access(filePath, constants.X_OK) + return true + } catch { + return false + } +} + +function pathEntries(pathEnv: string, delimiter: string): string[] { + return [...new Set(pathEnv.split(delimiter).filter(Boolean))] +} + +function windowsPathExts(value: string | undefined): string[] { + const source = value?.length ? value : DEFAULT_WINDOWS_EXTENSIONS.join(';') + return [ + ...new Set( + source + .split(';') + .map((part) => part.trim()) + .filter(Boolean) + .map((part) => (part.startsWith('.') ? part : `.${part}`)) + .map((part) => part.toUpperCase()) + ) + ] +} + +function candidateFileNames( + candidate: string, + platform: NodeJS.Platform, + pathExt?: string +): string[] { + if (platform !== 'win32' || pathApiForPlatform(platform).extname(candidate)) { + return [candidate] + } + return windowsPathExts(pathExt).map((suffix) => `${candidate}${suffix}`) +} + +function overrideTokenForAgent( + settings: CommandOverrideSettings, + target: ManagedAgentHookTarget, + platform: NodeJS.Platform +): string | null { + return extractExecutableToken(settings?.agentCmdOverrides?.[target.tuiAgent], { platform }) +} + +function expandHomePathToken(token: string, platform: NodeJS.Platform, homeDir: string): string { + if (token === '~') { + return homeDir + } + if (token.startsWith('~/') || (platform === 'win32' && token.startsWith('~\\'))) { + return pathApiForPlatform(platform).join(homeDir, token.slice(2)) + } + return token +} + +async function probePathCandidate( + candidate: string, + dirs: readonly string[], + platform: NodeJS.Platform, + fileProbe: FileProbe, + pathExt?: string +): Promise { + if (!isSafeExecutableBasename(candidate)) { + return false + } + for (const dir of dirs) { + for (const fileName of candidateFileNames(candidate, platform, pathExt)) { + if (await fileProbe.isExecutableFile(pathApiForPlatform(platform).join(dir, fileName))) { + return true + } + } + } + return false +} + +function isPlatformAbsolutePath(candidate: string, platform: NodeJS.Platform): boolean { + return platform === 'win32' ? path.win32.isAbsolute(candidate) : path.posix.isAbsolute(candidate) +} + +async function maybeHydrateShellPath(options: DetectOptions): Promise { + if (!options.shouldHydrateShellPath) { + return + } + try { + const result = await (options.hydratePath ?? hydrateShellPath)() + if (result.ok) { + mergePathSegments(result.segments) + } + } catch (error) { + // Detection failure must never permit config mutation. + console.warn('[agent-hooks] Shell PATH hydration failed; using inherited PATH:', error) + } +} + +export async function detectLocalManagedAgentCliPresence( + targets: readonly ManagedAgentHookTarget[], + settings: CommandOverrideSettings, + options: DetectOptions = {} +): Promise { + await maybeHydrateShellPath(options) + const platform = options.platform ?? process.platform + const delimiter = options.pathDelimiter ?? pathApiForPlatform(platform).delimiter + const dirs = pathEntries(options.pathEnv ?? process.env.PATH ?? '', delimiter) + const homeDir = options.homeDir ?? homedir() + const fileProbe = options.fileProbe ?? { + isExecutableFile: (filePath: string) => isExecutableFile(filePath, platform) + } + const candidates = new Set() + for (const target of targets) { + for (const candidate of target.executableCandidates) { + if (!hasPathSeparatorToken(candidate)) { + candidates.add(candidate) + } + } + const override = overrideTokenForAgent(settings, target, platform) + if (override && !hasPathSeparatorToken(override)) { + candidates.add(override) + } + } + const found = new Set() + for (const candidate of candidates) { + if (await probePathCandidate(candidate, dirs, platform, fileProbe, options.pathExt)) { + found.add(candidate) + } + } + const result: LocalCliPresenceByAgent = {} + for (const target of targets) { + const override = overrideTokenForAgent(settings, target, platform) + if (override && hasPathSeparatorToken(override)) { + const expanded = expandHomePathToken(override, platform, homeDir) + if (!isPlatformAbsolutePath(expanded, platform)) { + result[target.agent] = { state: 'unknown' } + continue + } + result[target.agent] = (await fileProbe.isExecutableFile(expanded)) + ? { state: 'found' } + : { state: 'missing' } + continue + } + const targetCandidates = [...target.executableCandidates, ...(override ? [override] : [])] + result[target.agent] = targetCandidates.some((candidate) => found.has(candidate)) + ? { state: 'found' } + : { state: 'missing' } + } + return result +} diff --git a/src/main/agent-hooks/managed-agent-hook-controls.test.ts b/src/main/agent-hooks/managed-agent-hook-controls.test.ts new file mode 100644 index 00000000000..f87fd8a06dd --- /dev/null +++ b/src/main/agent-hooks/managed-agent-hook-controls.test.ts @@ -0,0 +1,151 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + detect: vi.fn(), + installClaude: vi.fn(), + installCodex: vi.fn(), + removeClaude: vi.fn(), + removeCodex: vi.fn(), + statusClaude: vi.fn(), + statusCodex: vi.fn() +})) + +vi.mock('./local-agent-cli-presence', () => ({ + detectLocalManagedAgentCliPresence: mocks.detect +})) + +vi.mock('./managed-agent-hook-registry', () => ({ + MANAGED_AGENT_HOOK_INSTALLERS: [ + ['claude', mocks.installClaude], + ['codex', mocks.installCodex] + ], + MANAGED_AGENT_HOOK_REMOVERS: [ + ['claude', mocks.removeClaude], + ['codex', mocks.removeCodex] + ], + MANAGED_AGENT_HOOK_STATUS_READERS: [ + ['claude', mocks.statusClaude], + ['codex', mocks.statusCodex] + ] +})) + +import { + applyAgentStatusHooksEnabled, + installManagedAgentHooks +} from './managed-agent-hook-controls' + +function status(agent: 'claude' | 'codex', state: 'installed' | 'not_installed') { + return { + agent, + state, + configPath: `/${agent}`, + managedHooksPresent: state === 'installed', + detail: null + } as const +} + +describe('managed agent hook controls', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.installClaude.mockReturnValue(status('claude', 'installed')) + mocks.installCodex.mockReturnValue(status('codex', 'installed')) + mocks.removeClaude.mockReturnValue(status('claude', 'not_installed')) + mocks.removeCodex.mockReturnValue(status('codex', 'not_installed')) + }) + + it('installs only agents with positively detected CLIs', async () => { + mocks.detect.mockResolvedValue({ + claude: { state: 'missing' }, + codex: { state: 'found' } + }) + + const results = await installManagedAgentHooks({ agentCmdOverrides: {} }) + + expect(mocks.installClaude).not.toHaveBeenCalled() + expect(mocks.installCodex).toHaveBeenCalledTimes(1) + expect(results).toEqual([ + expect.objectContaining({ + agent: 'claude', + state: 'skipped', + skipReason: 'cli_not_found' + }), + expect.objectContaining({ agent: 'codex', state: 'installed' }) + ]) + }) + + it('fails closed when CLI detection rejects', async () => { + mocks.detect.mockRejectedValue(new Error('detection unavailable')) + + const results = await installManagedAgentHooks({ agentCmdOverrides: {} }) + + expect(mocks.installClaude).not.toHaveBeenCalled() + expect(mocks.installCodex).not.toHaveBeenCalled() + expect(results).toEqual([ + expect.objectContaining({ + agent: 'claude', + state: 'skipped', + skipReason: 'cli_presence_unknown' + }), + expect.objectContaining({ + agent: 'codex', + state: 'skipped', + skipReason: 'cli_presence_unknown' + }) + ]) + }) + + it('removes disabled agents without probing or reinstalling them', async () => { + mocks.detect.mockResolvedValue({ codex: { state: 'found' } }) + + const results = await applyAgentStatusHooksEnabled(true, { + agentCmdOverrides: {}, + disabledTuiAgents: ['claude'] + }) + + expect(mocks.removeClaude).toHaveBeenCalledTimes(1) + expect(mocks.installClaude).not.toHaveBeenCalled() + expect(mocks.installCodex).toHaveBeenCalledTimes(1) + expect(results).toEqual([ + expect.objectContaining({ agent: 'claude', state: 'not_installed' }), + expect.objectContaining({ agent: 'codex', state: 'installed' }) + ]) + }) + + it('does not install an agent disabled while detection was running', async () => { + mocks.detect.mockResolvedValue({ + claude: { state: 'found' }, + codex: { state: 'found' } + }) + + await installManagedAgentHooks( + { agentCmdOverrides: {} }, + { shouldContinue: (agent) => agent !== 'claude' } + ) + + expect(mocks.installClaude).not.toHaveBeenCalled() + expect(mocks.installCodex).toHaveBeenCalledTimes(1) + }) + + it('does not remove an agent enabled by a newer settings update', async () => { + mocks.detect.mockResolvedValue({ codex: { state: 'found' } }) + + await applyAgentStatusHooksEnabled( + true, + { + agentCmdOverrides: {}, + disabledTuiAgents: ['claude'] + }, + { shouldContinue: () => true } + ) + + expect(mocks.removeClaude).not.toHaveBeenCalled() + }) + + it('removes every managed hook when the global setting is off', async () => { + await applyAgentStatusHooksEnabled(false) + + expect(mocks.detect).not.toHaveBeenCalled() + expect(mocks.removeClaude).toHaveBeenCalledTimes(1) + expect(mocks.removeCodex).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/agent-hooks/managed-agent-hook-controls.ts b/src/main/agent-hooks/managed-agent-hook-controls.ts index e3070afed3e..a13450b0e21 100644 --- a/src/main/agent-hooks/managed-agent-hook-controls.ts +++ b/src/main/agent-hooks/managed-agent-hook-controls.ts @@ -1,75 +1,34 @@ -import type { AgentHookInstallStatus } from '../../shared/agent-hook-types' -import type { HookInstallAgent } from '../../shared/telemetry-events' +import type { AgentHookInstallStatus, AgentHookTarget } from '../../shared/agent-hook-types' +import { + getManagedAgentHookTarget, + isManagedAgentHookTarget +} from '../../shared/managed-agent-hook-targets' +import { normalizeDisabledTuiAgents } from '../../shared/tui-agent-selection' import type { GlobalSettings } from '../../shared/types' -import { ampHookService } from '../amp/hook-service' -import { antigravityHookService } from '../antigravity/hook-service' -import { claudeHookService } from '../claude/hook-service' -import { codexHookService } from '../codex/hook-service' -import { copilotHookService } from '../copilot/hook-service' -import { cursorHookService } from '../cursor/hook-service' -import { droidHookService } from '../droid/hook-service' -import { commandCodeHookService } from '../command-code/hook-service' -import { geminiHookService } from '../gemini/hook-service' -import { devinHookService } from '../devin/hook-service' -import { grokHookService } from '../grok/hook-service' -import { hermesHookService } from '../hermes/hook-service' -import { kimiHookService } from '../kimi/hook-service' -import { openClaudeHookService } from '../openclaude/hook-service' +import { detectLocalManagedAgentCliPresence } from './local-agent-cli-presence' +import { + MANAGED_AGENT_HOOK_INSTALLERS, + MANAGED_AGENT_HOOK_REMOVERS, + MANAGED_AGENT_HOOK_STATUS_READERS, + type ManagedAgentHookInstaller +} from './managed-agent-hook-registry' -export type ManagedAgentHookInstaller = readonly [HookInstallAgent, () => void] -type ManagedHookRemover = readonly [HookInstallAgent, () => AgentHookInstallStatus] -type ManagedHookStatusReader = readonly [HookInstallAgent, () => AgentHookInstallStatus] +export { MANAGED_AGENT_HOOK_INSTALLERS } from './managed-agent-hook-registry' -export const MANAGED_AGENT_HOOK_INSTALLERS: readonly ManagedAgentHookInstaller[] = [ - ['claude', () => claudeHookService.install()], - ['openclaude', () => openClaudeHookService.install()], - ['codex', () => codexHookService.install()], - ['gemini', () => geminiHookService.install()], - ['antigravity', () => antigravityHookService.install()], - ['amp', () => ampHookService.install()], - ['cursor', () => cursorHookService.install()], - ['droid', () => droidHookService.install()], - ['command-code', () => commandCodeHookService.install()], - ['grok', () => grokHookService.install()], - ['copilot', () => copilotHookService.install()], - ['hermes', () => hermesHookService.install()], - ['devin', () => devinHookService.install()], - ['kimi', () => kimiHookService.install()] -] +type ManagedHookSettings = Partial< + Pick +> | null -const LOCAL_MANAGED_HOOK_REMOVERS: readonly ManagedHookRemover[] = [ - ['claude', () => claudeHookService.remove()], - ['openclaude', () => openClaudeHookService.remove()], - ['codex', () => codexHookService.remove()], - ['gemini', () => geminiHookService.remove()], - ['antigravity', () => antigravityHookService.remove()], - ['amp', () => ampHookService.remove()], - ['cursor', () => cursorHookService.remove()], - ['droid', () => droidHookService.remove()], - ['command-code', () => commandCodeHookService.remove()], - ['grok', () => grokHookService.remove()], - ['copilot', () => copilotHookService.remove()], - ['hermes', () => hermesHookService.remove()], - ['devin', () => devinHookService.remove()], - ['kimi', () => kimiHookService.remove()] -] +type InstallOptions = { + shouldHydrateShellPath?: boolean + onInstallError?: (agent: AgentHookTarget, error: unknown) => void + shouldContinue?: (agent: AgentHookTarget) => boolean + agents?: readonly AgentHookTarget[] +} -const LOCAL_MANAGED_HOOK_STATUS_READERS: readonly ManagedHookStatusReader[] = [ - ['claude', () => claudeHookService.getStatus()], - ['openclaude', () => openClaudeHookService.getStatus()], - ['codex', () => codexHookService.getStatus()], - ['gemini', () => geminiHookService.getStatus()], - ['antigravity', () => antigravityHookService.getStatus()], - ['amp', () => ampHookService.getStatus()], - ['cursor', () => cursorHookService.getStatus()], - ['droid', () => droidHookService.getStatus()], - ['grok', () => grokHookService.getStatus()], - ['command-code', () => commandCodeHookService.getStatus()], - ['copilot', () => copilotHookService.getStatus()], - ['hermes', () => hermesHookService.getStatus()], - ['devin', () => devinHookService.getStatus()], - ['kimi', () => kimiHookService.getStatus()] -] +type RemoveOptions = { + agents?: readonly AgentHookTarget[] +} export function isAgentStatusHooksEnabled( settings: Pick | null | undefined @@ -77,17 +36,7 @@ export function isAgentStatusHooksEnabled( return settings?.agentStatusHooksEnabled !== false } -export function installManagedAgentHooks(): void { - for (const [agent, install] of MANAGED_AGENT_HOOK_INSTALLERS) { - try { - install() - } catch (error) { - console.warn(`[agent-hooks] Failed to install ${agent} managed hooks:`, error) - } - } -} - -function errorStatus(agent: HookInstallAgent, error: unknown): AgentHookInstallStatus { +function errorStatus(agent: AgentHookTarget, error: unknown): AgentHookInstallStatus { return { agent, state: 'error', @@ -97,8 +46,110 @@ function errorStatus(agent: HookInstallAgent, error: unknown): AgentHookInstallS } } -export function removeManagedAgentHooks(): AgentHookInstallStatus[] { - return LOCAL_MANAGED_HOOK_REMOVERS.map(([agent, remove]) => { +function skippedStatus( + agent: AgentHookTarget, + skipReason: NonNullable, + detail: string +): AgentHookInstallStatus { + return { + agent, + state: 'skipped', + configPath: '', + managedHooksPresent: false, + detail, + skipReason + } +} + +function selectedInstallers(options: InstallOptions): readonly ManagedAgentHookInstaller[] { + if (!options.agents) { + return MANAGED_AGENT_HOOK_INSTALLERS + } + const allowed = new Set(options.agents) + return MANAGED_AGENT_HOOK_INSTALLERS.filter(([agent]) => allowed.has(agent)) +} + +function runInstaller( + entry: ManagedAgentHookInstaller, + onInstallError: InstallOptions['onInstallError'] +): AgentHookInstallStatus { + const [agent, install] = entry + try { + return 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) + } + return errorStatus(agent, error) + } +} + +export async function installManagedAgentHooks( + settings: ManagedHookSettings = null, + options: InstallOptions = {} +): Promise { + const installers = selectedInstallers(options) + const disabled = new Set(normalizeDisabledTuiAgents(settings?.disabledTuiAgents)) + const enabledInstallers = installers.filter(([agent]) => !disabled.has(agent)) + const targets = enabledInstallers.flatMap(([agent]) => { + const target = getManagedAgentHookTarget(agent) + return target ? [target] : [] + }) + let presenceByAgent + try { + presenceByAgent = await detectLocalManagedAgentCliPresence(targets, settings, { + shouldHydrateShellPath: options.shouldHydrateShellPath + }) + } catch (error) { + const detail = error instanceof Error ? error.message : String(error) + return installers.map(([agent]) => + disabled.has(agent) + ? skippedStatus(agent, 'agent_disabled', 'Agent is disabled in Settings.') + : skippedStatus(agent, 'cli_presence_unknown', detail) + ) + } + + const results: AgentHookInstallStatus[] = [] + for (const entry of installers) { + const [agent] = entry + if (disabled.has(agent)) { + results.push(skippedStatus(agent, 'agent_disabled', 'Agent is disabled in Settings.')) + continue + } + if (options.shouldContinue && !options.shouldContinue(agent)) { + results.push( + skippedStatus( + agent, + 'hooks_disabled', + 'Agent status hooks were disabled before install completed.' + ) + ) + continue + } + const presence = presenceByAgent[agent] + if (presence?.state !== 'found') { + results.push( + skippedStatus( + agent, + presence?.state === 'unknown' ? 'cli_presence_unknown' : 'cli_not_found', + 'CLI not found; managed hook install skipped.' + ) + ) + continue + } + results.push(runInstaller(entry, options.onInstallError)) + } + return results +} + +export function removeManagedAgentHooks(options: RemoveOptions = {}): AgentHookInstallStatus[] { + 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) { @@ -108,7 +159,7 @@ export function removeManagedAgentHooks(): AgentHookInstallStatus[] { } export function getManagedAgentHookStatuses(): AgentHookInstallStatus[] { - return LOCAL_MANAGED_HOOK_STATUS_READERS.map(([agent, getStatus]) => { + return MANAGED_AGENT_HOOK_STATUS_READERS.map(([agent, getStatus]) => { try { return getStatus() } catch (error) { @@ -117,10 +168,26 @@ export function getManagedAgentHookStatuses(): AgentHookInstallStatus[] { }) } -export function applyAgentStatusHooksEnabled(enabled: boolean): AgentHookInstallStatus[] { - if (enabled) { - installManagedAgentHooks() - return getManagedAgentHookStatuses() +export async function applyAgentStatusHooksEnabled( + enabled: boolean, + settings: ManagedHookSettings = null, + options: InstallOptions = {} +): Promise { + if (!enabled) { + return removeManagedAgentHooks() } - return removeManagedAgentHooks() + const disabled = normalizeDisabledTuiAgents(settings?.disabledTuiAgents).filter( + isManagedAgentHookTarget + ) + const installed = await installManagedAgentHooks(settings, options) + const disabledToRemove = options.shouldContinue + ? disabled.filter((agent) => !options.shouldContinue?.(agent)) + : disabled + if (disabledToRemove.length === 0) { + return installed + } + const removed = new Map( + 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 new file mode 100644 index 00000000000..1c17da381ce --- /dev/null +++ b/src/main/agent-hooks/managed-agent-hook-registry.ts @@ -0,0 +1,71 @@ +import type { AgentHookInstallStatus } from '../../shared/agent-hook-types' +import type { HookInstallAgent } from '../../shared/telemetry-events' +import { ampHookService } from '../amp/hook-service' +import { antigravityHookService } from '../antigravity/hook-service' +import { claudeHookService } from '../claude/hook-service' +import { codexHookService } from '../codex/hook-service' +import { commandCodeHookService } from '../command-code/hook-service' +import { copilotHookService } from '../copilot/hook-service' +import { cursorHookService } from '../cursor/hook-service' +import { devinHookService } from '../devin/hook-service' +import { droidHookService } from '../droid/hook-service' +import { geminiHookService } from '../gemini/hook-service' +import { grokHookService } from '../grok/hook-service' +import { hermesHookService } from '../hermes/hook-service' +import { kimiHookService } from '../kimi/hook-service' +import { openClaudeHookService } from '../openclaude/hook-service' + +export type ManagedAgentHookInstaller = readonly [HookInstallAgent, () => AgentHookInstallStatus] +export type ManagedAgentHookRemover = readonly [HookInstallAgent, () => AgentHookInstallStatus] +export type ManagedAgentHookStatusReader = readonly [HookInstallAgent, () => AgentHookInstallStatus] + +export const MANAGED_AGENT_HOOK_INSTALLERS: readonly ManagedAgentHookInstaller[] = [ + ['claude', () => claudeHookService.install()], + ['openclaude', () => openClaudeHookService.install()], + ['codex', () => codexHookService.install()], + ['gemini', () => geminiHookService.install()], + ['antigravity', () => antigravityHookService.install()], + ['amp', () => ampHookService.install()], + ['cursor', () => cursorHookService.install()], + ['droid', () => droidHookService.install()], + ['command-code', () => commandCodeHookService.install()], + ['grok', () => grokHookService.install()], + ['copilot', () => copilotHookService.install()], + ['hermes', () => hermesHookService.install()], + ['devin', () => devinHookService.install()], + ['kimi', () => kimiHookService.install()] +] + +export const MANAGED_AGENT_HOOK_REMOVERS: readonly ManagedAgentHookRemover[] = [ + ['claude', () => claudeHookService.remove()], + ['openclaude', () => openClaudeHookService.remove()], + ['codex', () => codexHookService.remove()], + ['gemini', () => geminiHookService.remove()], + ['antigravity', () => antigravityHookService.remove()], + ['amp', () => ampHookService.remove()], + ['cursor', () => cursorHookService.remove()], + ['droid', () => droidHookService.remove()], + ['command-code', () => commandCodeHookService.remove()], + ['grok', () => grokHookService.remove()], + ['copilot', () => copilotHookService.remove()], + ['hermes', () => hermesHookService.remove()], + ['devin', () => devinHookService.remove()], + ['kimi', () => kimiHookService.remove()] +] + +export const MANAGED_AGENT_HOOK_STATUS_READERS: readonly ManagedAgentHookStatusReader[] = [ + ['claude', () => claudeHookService.getStatus()], + ['openclaude', () => openClaudeHookService.getStatus()], + ['codex', () => codexHookService.getStatus()], + ['gemini', () => geminiHookService.getStatus()], + ['antigravity', () => antigravityHookService.getStatus()], + ['amp', () => ampHookService.getStatus()], + ['cursor', () => cursorHookService.getStatus()], + ['droid', () => droidHookService.getStatus()], + ['grok', () => grokHookService.getStatus()], + ['command-code', () => commandCodeHookService.getStatus()], + ['copilot', () => copilotHookService.getStatus()], + ['hermes', () => hermesHookService.getStatus()], + ['devin', () => devinHookService.getStatus()], + ['kimi', () => kimiHookService.getStatus()] +] diff --git a/src/main/agent-hooks/managed-hook-detection-commands.test.ts b/src/main/agent-hooks/managed-hook-detection-commands.test.ts new file mode 100644 index 00000000000..abcb24d3969 --- /dev/null +++ b/src/main/agent-hooks/managed-hook-detection-commands.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from 'vitest' +import { + buildManagedHookDetectionCommands, + detectedManagedHookAgents +} from './managed-hook-detection-commands' + +describe('managed hook detection commands', () => { + it('omits disabled agents and includes safe command overrides', () => { + const commands = buildManagedHookDetectionCommands( + { + disabledTuiAgents: ['claude'], + agentCmdOverrides: { codex: '/opt/codex custom' } + }, + 'linux' + ) + + expect(commands.some((command) => command.id === 'claude')).toBe(false) + expect(commands).toContainEqual({ id: 'codex', cmd: '/opt/codex' }) + }) + + it('maps detected TUI ids back to managed hook targets', () => { + expect(detectedManagedHookAgents(['codex', 'opencode', 'droid'])).toEqual(['codex', 'droid']) + }) +}) diff --git a/src/main/agent-hooks/managed-hook-detection-commands.ts b/src/main/agent-hooks/managed-hook-detection-commands.ts new file mode 100644 index 00000000000..ba84c901bb5 --- /dev/null +++ b/src/main/agent-hooks/managed-hook-detection-commands.ts @@ -0,0 +1,42 @@ +import type { AgentHookTarget } from '../../shared/agent-hook-types' +import { + extractExecutableToken, + isSafeOverrideExecutableToken +} from '../../shared/managed-agent-command-token' +import { MANAGED_AGENT_HOOK_TARGETS } from '../../shared/managed-agent-hook-targets' +import { normalizeDisabledTuiAgents } from '../../shared/tui-agent-selection' +import type { GlobalSettings } from '../../shared/types' +import type { TuiAgentDetectionCommand } from '../ipc/tui-agent-detection-commands' + +export type ManagedHookDetectionSettings = Partial< + Pick +> | null + +export function buildManagedHookDetectionCommands( + settings: ManagedHookDetectionSettings, + platform: NodeJS.Platform +): TuiAgentDetectionCommand[] { + const disabled = new Set(normalizeDisabledTuiAgents(settings?.disabledTuiAgents)) + return MANAGED_AGENT_HOOK_TARGETS.filter((target) => !disabled.has(target.tuiAgent)).flatMap( + (target) => { + const commands = new Set(target.executableCandidates) + const override = extractExecutableToken(settings?.agentCmdOverrides?.[target.tuiAgent], { + platform + }) + if (override && isSafeOverrideExecutableToken(override)) { + commands.add(override) + } + return [...commands].map((cmd) => ({ id: target.tuiAgent, cmd })) + } + ) +} + +export function detectedManagedHookAgents(values: unknown): AgentHookTarget[] { + if (!Array.isArray(values)) { + return [] + } + const detected = new Set(values.filter((value): value is string => typeof value === 'string')) + return MANAGED_AGENT_HOOK_TARGETS.filter((target) => detected.has(target.tuiAgent)).map( + (target) => target.agent + ) +} diff --git a/src/main/agent-hooks/managed-hook-local-filesystem.test.ts b/src/main/agent-hooks/managed-hook-local-filesystem.test.ts index 91a1ca62eb6..f6dcf14471a 100644 --- a/src/main/agent-hooks/managed-hook-local-filesystem.test.ts +++ b/src/main/agent-hooks/managed-hook-local-filesystem.test.ts @@ -6,9 +6,11 @@ import { installRemoteManagedAgentHooks } from './remote-managed-hook-installers import { createManagedHookLocalFilesystem } from './managed-hook-local-filesystem' const tempHomes: string[] = [] +const tempRoot = process.platform === 'win32' ? tmpdir() : '/tmp' async function createTempHome(): Promise { - const home = await mkdtemp(join(tmpdir(), 'orca-managed-hooks-')) + // Why: mkdir-p probes ancestors; macOS's per-user temp directory can contain hundreds of thousands of entries. + const home = await mkdtemp(join(tempRoot, 'orca-managed-hooks-')) tempHomes.push(home) return home } diff --git a/src/main/agent-hooks/managed-hook-runtime.ts b/src/main/agent-hooks/managed-hook-runtime.ts index b79c3b2186d..bdd7195b50d 100644 --- a/src/main/agent-hooks/managed-hook-runtime.ts +++ b/src/main/agent-hooks/managed-hook-runtime.ts @@ -3,6 +3,7 @@ import { basename } from 'node:path' import { homedir, userInfo } from 'node:os' import { promisify } from 'node:util' import { installRemoteManagedAgentHooks } from './remote-managed-hook-installers' +import type { AgentHookTarget } from '../../shared/agent-hook-types' import { createManagedHookLocalFilesystem } from './managed-hook-local-filesystem' import { withManagedHookInstallLock } from './managed-hook-install-lock' import { @@ -75,6 +76,7 @@ export async function resolveRelayGrokHome(home: string, signal?: AbortSignal): export async function installManagedHooks(options?: { signal?: AbortSignal hostKeyFingerprint?: string + agents?: readonly AgentHookTarget[] }): Promise { options?.signal?.throwIfAborted() const home = homedir() @@ -93,7 +95,8 @@ export async function installManagedHooks(options?: { home, { grokHomeDir, - signal: options?.signal + signal: options?.signal, + agents: options?.agents } ) return { diff --git a/src/main/agent-hooks/remote-hook-service-installers.test.ts b/src/main/agent-hooks/remote-hook-service-installers.test.ts index 89124fe86a8..2b2cfcdcf38 100644 --- a/src/main/agent-hooks/remote-hook-service-installers.test.ts +++ b/src/main/agent-hooks/remote-hook-service-installers.test.ts @@ -713,6 +713,20 @@ describe('remote hook service installers', () => { expect(byAgent.get('copilot')).toBe('installed') }) + it('installs only positively detected remote agents', async () => { + const { sftp, fs } = createFakeSftp() + + const results = await installRemoteManagedAgentHooks(sftp, '/home/dev', { + agents: ['codex'] + }) + + expect(results.map((result) => result.agent)).toEqual(['codex']) + const paths = [...fs.files.keys(), ...fs.dirs] + for (const unusedHome of ['.factory', '.gemini', '.grok', '.hermes', '.commandcode']) { + expect(paths.some((path) => path.includes(`/home/dev/${unusedHome}`))).toBe(false) + } + }) + it('stops before the next installer when its relay request is cancelled', async () => { const controller = new AbortController() const claudeInstall = vi diff --git a/src/main/agent-hooks/remote-managed-hook-installers.ts b/src/main/agent-hooks/remote-managed-hook-installers.ts index 7f9e97f2f9f..0fdf0c47da4 100644 --- a/src/main/agent-hooks/remote-managed-hook-installers.ts +++ b/src/main/agent-hooks/remote-managed-hook-installers.ts @@ -1,5 +1,5 @@ import type { SFTPWrapper } from 'ssh2' -import type { AgentHookInstallStatus } from '../../shared/agent-hook-types' +import type { AgentHookInstallStatus, AgentHookTarget } from '../../shared/agent-hook-types' import { ampHookService } from '../amp/hook-service' import { claudeHookService } from '../claude/hook-service' import { codexHookService } from '../codex/hook-service' @@ -26,6 +26,8 @@ export type RemoteManagedHookInstallOptions = { /** Stops before starting the next installer when the owning relay request * is cancelled. Individual filesystem mutations remain atomic. */ signal?: AbortSignal + /** Positively detected and enabled agents allowed to mutate config. */ + agents?: readonly AgentHookTarget[] } type RemoteManagedHookInstaller = readonly [ @@ -80,7 +82,11 @@ export async function installRemoteManagedAgentHooks( options?: RemoteManagedHookInstallOptions ): Promise { const results: AgentHookInstallStatus[] = [] + const allowedAgents = options?.agents ? new Set(options.agents) : null for (const [agent, install] of REMOTE_MANAGED_HOOK_INSTALLERS) { + if (allowedAgents && !allowedAgents.has(agent)) { + continue + } // Why: relay requests can disappear during reconnect; do not start more // user-config mutations after their client has gone away. options?.signal?.throwIfAborted() diff --git a/src/main/agent-hooks/wsl-hook-fs-adapter.ts b/src/main/agent-hooks/wsl-hook-fs-adapter.ts index d47dd400199..9578fda1d10 100644 --- a/src/main/agent-hooks/wsl-hook-fs-adapter.ts +++ b/src/main/agent-hooks/wsl-hook-fs-adapter.ts @@ -6,6 +6,11 @@ import type { SFTPWrapper } from 'ssh2' import type { installRemoteManagedAgentHooks } from './remote-managed-hook-installers' +import { + buildManagedHookDetectionCommands, + detectedManagedHookAgents, + type ManagedHookDetectionSettings +} from './managed-hook-detection-commands' import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' import { wslCodexRuntimeHomeForGuestHome } from '../pty/codex-home-wsl-env' import { WSL_HOOK_FS_METHODS, type WslFsResult } from '../../shared/wsl-hook-relay-contract' @@ -18,11 +23,30 @@ export async function installWslGuestHooks(options: { guestHome: string distro: string installHooks: typeof installRemoteManagedAgentHooks + settings: ManagedHookDetectionSettings warn: (message: string) => void }): Promise { - const { mux, guestHome, distro, installHooks, warn } = options + const { mux, guestHome, distro, installHooks, settings, warn } = options + let agents + try { + const detected = (await mux.request('preflight.detectAgents', { + commands: buildManagedHookDetectionCommands(settings, 'linux') + })) as { agents?: unknown } + agents = detectedManagedHookAgents(detected?.agents) + } catch (error) { + warn( + `[agent-hooks] WSL agent detection for '${distro}' failed: ${ + error instanceof Error ? error.message : String(error) + }` + ) + return + } + if (agents.length === 0) { + return + } const results = await installHooks(createWslHookSftpAdapter(mux), guestHome, { - codexHomeDir: wslCodexRuntimeHomeForGuestHome(guestHome) + codexHomeDir: wslCodexRuntimeHomeForGuestHome(guestHome), + agents }) const failed = results.filter((r) => r.state === 'error').length if (failed > 0) { diff --git a/src/main/agent-hooks/wsl-hook-relay-deps.ts b/src/main/agent-hooks/wsl-hook-relay-deps.ts index cbeb5477d9b..9d6fbccde26 100644 --- a/src/main/agent-hooks/wsl-hook-relay-deps.ts +++ b/src/main/agent-hooks/wsl-hook-relay-deps.ts @@ -5,6 +5,7 @@ import { createHash } from 'node:crypto' import { readFileSync } from 'node:fs' import { agentHookServer } from './server' +import type { ManagedHookDetectionSettings } from './managed-hook-detection-commands' import { installRemoteManagedAgentHooks } from './remote-managed-hook-installers' import { getOpenCodePluginSource } from '../opencode/hook-service' import type { PluginSources } from '../../relay/plugin-overlay' @@ -59,6 +60,7 @@ export type WslHookRelayManagerDeps = { waitForSentinel: typeof waitForWslRelaySentinel ingest: (envelope: Record, connectionId: string) => void installHooks: typeof installRemoteManagedAgentHooks + managedHookSettings: () => ManagedHookDetectionSettings /** Plugin source strings shipped to the guest relay so an Orca update needn't redeploy the relay bundle. */ pluginSources: () => PluginSources warn: (message: string) => void @@ -89,6 +91,7 @@ export const defaultWslHookRelayDeps: WslHookRelayManagerDeps = { connectionId ), installHooks: installRemoteManagedAgentHooks, + managedHookSettings: () => null, // Why: only OpenCode is in scope for WSL now; the payload shape stays identical to SSH so Pi/OMP are additive later. pluginSources: () => ({ opencodePluginSource: getOpenCodePluginSource() }), warn: (message) => console.warn(message), diff --git a/src/main/agent-hooks/wsl-hook-relay-live.integration.test.ts b/src/main/agent-hooks/wsl-hook-relay-live.integration.test.ts index 89b0987a582..ab6d6a03e45 100644 --- a/src/main/agent-hooks/wsl-hook-relay-live.integration.test.ts +++ b/src/main/agent-hooks/wsl-hook-relay-live.integration.test.ts @@ -7,7 +7,6 @@ // exercises minus the wsl.exe byte transport (validated separately on-rig). import { execFileSync, spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs' -import { tmpdir } from 'node:os' import { join } from 'node:path' import { createServer } from 'node:net' import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest' @@ -59,7 +58,7 @@ describe.skipIf(process.platform === 'win32')( }) it('delivers a Claude hook POST from the live relay into ingestRemote and installs guest hooks', async () => { - fakeHome = mkdtempSync(join(tmpdir(), 'wsl-live-home-')) + fakeHome = mkdtempSync(join('/tmp', 'wsl-live-home-')) const preferredPort = await pickFreePort() const version = readFileSync(join(BUNDLE_DIR, '.version'), 'utf8').trim() @@ -102,6 +101,12 @@ describe.skipIf(process.platform === 'win32')( envelope as Parameters[0], connectionId ), + managedHookSettings: () => ({ + agentCmdOverrides: { + claude: process.execPath, + codex: process.execPath + } + }), warn: (message) => warns.push(message), transientRetryDelayMs: 1 }) diff --git a/src/main/agent-hooks/wsl-hook-relay-manager.test.ts b/src/main/agent-hooks/wsl-hook-relay-manager.test.ts index 0eaf850b63a..39a9736a526 100644 --- a/src/main/agent-hooks/wsl-hook-relay-manager.test.ts +++ b/src/main/agent-hooks/wsl-hook-relay-manager.test.ts @@ -3,7 +3,6 @@ // the per-distro relay manager state machine with fault injection. import { EventEmitter } from 'node:events' import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs' -import { tmpdir } from 'node:os' import { join } from 'node:path' import type { ChildProcessWithoutNullStreams } from 'node:child_process' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -34,9 +33,13 @@ function createGuestHarness(): GuestHarness { const clientDataCallbacks: ((data: Buffer) => void)[] = [] const closeCallbacks: (() => void)[] = [] const transport: MultiplexerTransport = { - write: (data) => { - setImmediate(() => relayFeed?.(data)) + write: (data, onSettled) => { + setImmediate(() => { + relayFeed?.(data) + onSettled?.({ ok: true }) + }) }, + supportsWriteSettlement: true, onData: (cb) => { clientDataCallbacks.push(cb) }, @@ -44,21 +47,24 @@ function createGuestHarness(): GuestHarness { closeCallbacks.push(cb) } } - const guestDispatcher = new RelayDispatcher((data: Buffer) => { - setImmediate(() => { - for (const cb of clientDataCallbacks) { - cb(data) - } - }) - }) + const guestDispatcher = new RelayDispatcher( + (data: Buffer, onSettled) => { + setImmediate(() => { + for (const cb of clientDataCallbacks) { + cb(data) + } + onSettled({ ok: true }) + }) + }, + { supportsWriteCallback: true } + ) relayFeed = (data) => guestDispatcher.feed(data) const mux = new SshChannelMultiplexer(transport) return { transport, guestDispatcher, mux } } -// Why skipIf: the fs bridge runs inside the Linux guest and is POSIX-only by -// design (posix.resolve). On a Windows dev host tmpdir() yields C:\ paths the -// bridge correctly refuses; Windows coverage comes from the live rig runs. +// Why skipIf: the fs bridge runs inside the Linux guest and is POSIX-only; +// Windows coverage comes from the live rig runs. describe.skipIf(process.platform === 'win32')( 'createWslHookSftpAdapter over the guest fs bridge', () => { @@ -66,7 +72,7 @@ describe.skipIf(process.platform === 'win32')( let harness: GuestHarness beforeEach(() => { - home = mkdtempSync(join(tmpdir(), 'wsl-guest-home-')) + home = mkdtempSync(join('/tmp', 'wsl-guest-home-')) harness = createGuestHarness() registerWslHookFsHandlers(harness.guestDispatcher, home) }) @@ -166,13 +172,19 @@ describe('WslHookRelayManager', () => { return child as unknown as ChildProcessWithoutNullStreams & { emitClose: () => void } } - function guestTransport(registerInstallPlugins = true): MultiplexerTransport { + function guestTransport( + options: { registerInstallPlugins?: boolean; detectedAgents?: string[] } = {} + ): MultiplexerTransport { + const { registerInstallPlugins = true, detectedAgents = ['codex'] } = options const harness = createGuestHarness() harnesses.push(harness) registerWslHookFsHandlers(harness.guestDispatcher, home) harness.guestDispatcher.onRequest(AGENT_HOOK_REQUEST_REPLAY_METHOD, async () => ({ replayed: 0 })) + harness.guestDispatcher.onRequest('preflight.detectAgents', async () => ({ + agents: detectedAgents + })) // A guest bundle predating the plugin overlay omits this handler (-32601). if (registerInstallPlugins) { harness.guestDispatcher.onRequest(AGENT_HOOK_INSTALL_PLUGINS_METHOD, async () => ({ @@ -212,6 +224,7 @@ describe('WslHookRelayManager', () => { waitForSentinel: vi.fn(async () => guestTransport()), ingest: vi.fn(), installHooks: vi.fn(async () => []), + managedHookSettings: () => null, pluginSources: () => ({ opencodePluginSource: '// opencode plugin source' }), warn: vi.fn(), transientRetryDelayMs: 1, @@ -228,7 +241,8 @@ describe('WslHookRelayManager', () => { expect(deps.spawnRelay).toHaveBeenCalledTimes(1) // Codex is the one agent whose home Orca redirects for WSL sessions. expect(deps.installHooks).toHaveBeenCalledWith(expect.anything(), home, { - codexHomeDir: `${home}/.local/share/orca/codex-runtime-home/home` + codexHomeDir: `${home}/.local/share/orca/codex-runtime-home/home`, + agents: ['codex'] }) expect(manager.getGuestEndpointFilePath('Ubuntu')).toBe( @@ -261,7 +275,7 @@ describe('WslHookRelayManager', () => { }) it('leaves the overlay dir null when the guest bundle lacks the installPlugins handler', async () => { - const waitForSentinel = vi.fn(async () => guestTransport(false)) + const waitForSentinel = vi.fn(async () => guestTransport({ registerInstallPlugins: false })) const { manager, deps } = createManager({ waitForSentinel }) manager.ensureForDistro('Ubuntu') // Connect still completes (hooks install); the -32601 is swallowed silently. @@ -272,6 +286,17 @@ describe('WslHookRelayManager', () => { manager.disposeAll() }) + it('does not mutate managed agent homes when no WSL agents are detected', async () => { + const waitForSentinel = vi.fn(async () => guestTransport({ detectedAgents: [] })) + const { manager, deps } = createManager({ waitForSentinel }) + + manager.ensureForDistro('Ubuntu') + await vi.waitFor(() => expect(manager.getOpenCodeOverlayDir('Ubuntu')).toBe(opencodeOverlayDir)) + + expect(deps.installHooks).not.toHaveBeenCalled() + manager.disposeAll() + }) + it('resolves the default distro for null and dedupes it with the explicit name', async () => { const { manager, deps } = createManager({}) manager.ensureForDistro(null) diff --git a/src/main/agent-hooks/wsl-hook-relay-manager.ts b/src/main/agent-hooks/wsl-hook-relay-manager.ts index 0d3817791f7..9971ed04f49 100644 --- a/src/main/agent-hooks/wsl-hook-relay-manager.ts +++ b/src/main/agent-hooks/wsl-hook-relay-manager.ts @@ -18,6 +18,7 @@ import { } from './wsl-hook-relay-deps' import { wireWslRelayLink } from './wsl-hook-relay-link' import { WslRelayRecovery } from './wsl-hook-relay-recovery' +import { wslHookRelayStateKey } from './wsl-hook-relay-state-key' import { requestGuestOpenCodeOverlayDir } from './wsl-guest-plugin-install' import { SshChannelMultiplexer, type MultiplexerTransport } from '../ssh/ssh-channel-multiplexer' import { AGENT_HOOK_REQUEST_REPLAY_METHOD } from '../../shared/agent-hook-relay' @@ -44,10 +45,6 @@ type DistroState = { lastInstallAt?: number } -function distroKey(distro: string): string { - return distro.trim().toLowerCase() -} - export class WslHookRelayManager { private deps: WslHookRelayManagerDeps private recovery: WslRelayRecovery @@ -62,12 +59,12 @@ export class WslHookRelayManager { isDistroRunning: (distro) => this.deps.isDistroRunning(distro), warn: (message) => this.deps.warn(message), isDisposed: () => this.disposed, - isCurrent: (state) => this.states.get(distroKey(state.distro)) === state, + isCurrent: (state) => this.states.get(wslHookRelayStateKey(state.distro)) === state, restart: (distro) => this.ensureForDistro(distro), dropState: (state) => { // Why: identity-guarded — a fresh ensure() may own this key by now; // deleting by key alone would orphan its live relay child. - const key = distroKey(state.distro) + const key = wslHookRelayStateKey(state.distro) if (this.states.get(key) === state) { this.states.delete(key) } @@ -75,6 +72,10 @@ export class WslHookRelayManager { }) } + setManagedHookSettingsResolver(resolve: WslHookRelayManagerDeps['managedHookSettings']): void { + this.deps.managedHookSettings = resolve + } + /** Fire-and-forget from every WSL PTY spawn-env build; errors breadcrumb. */ ensureForDistro(distro: string | null): void { if (this.disposed || this.deps.platform() !== 'win32' || !this.deps.remoteHooksEnabled()) { @@ -89,7 +90,7 @@ export class WslHookRelayManager { private stateFor(distro: string | null): DistroState | undefined { // Empty key never matches a real (non-empty) distro state. - return this.states.get(distroKey(distro ?? this.defaultDistro ?? '')) + return this.states.get(wslHookRelayStateKey(distro ?? this.defaultDistro ?? '')) } /** Guest endpoint file path once known; null before first connect @@ -120,7 +121,7 @@ export class WslHookRelayManager { if (!distro || this.disposed) { return } - const key = distroKey(distro) + const key = wslHookRelayStateKey(distro) const existing = this.states.get(key) if (existing) { if (existing.phase === 'running') { @@ -280,6 +281,7 @@ export class WslHookRelayManager { guestHome, distro: state.distro, installHooks: this.deps.installHooks, + settings: this.deps.managedHookSettings(), warn: this.deps.warn }) // Why: ship OpenCode's status plugin and record the guest overlay dir the diff --git a/src/main/agent-hooks/wsl-hook-relay-state-key.ts b/src/main/agent-hooks/wsl-hook-relay-state-key.ts new file mode 100644 index 00000000000..45962f22c3a --- /dev/null +++ b/src/main/agent-hooks/wsl-hook-relay-state-key.ts @@ -0,0 +1,3 @@ +export function wslHookRelayStateKey(distro: string): string { + return distro.trim().toLowerCase() +} diff --git a/src/main/index.ts b/src/main/index.ts index 1bc94103213..5dbd92065f0 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -43,7 +43,7 @@ import { initObservability, shutdownObservability } from './observability' import { registerMobileHandlers } from './ipc/mobile' import { initTelemetry, shutdownTelemetry, trackAppOpenedOnce, track } from './telemetry/client' import { classifyError } from './telemetry/classify-error' -import { runManagedHookInstallers } from './agent-hooks/install-telemetry' +import { recordManagedHookInstallFailure } from './agent-hooks/install-telemetry' import { indexPersistedPaneKeyPtyIds, isLocalExecutionHost, @@ -51,8 +51,8 @@ import { sweepRestoredSubagentsWithoutLiveAgent } from './agent-hooks/restored-subagent-liveness-sweep' import { + applyAgentStatusHooksEnabled, isAgentStatusHooksEnabled, - MANAGED_AGENT_HOOK_INSTALLERS, removeManagedAgentHooks } from './agent-hooks/managed-agent-hook-controls' import { initCohortClassifier } from './telemetry/cohort-classifier' @@ -1997,6 +1997,7 @@ void app.whenReady().then(async () => { const activeOrcaProfile = ensureActiveOrcaProfile() store = new Store({ dataFile: activeOrcaProfile.dataFile }) + wslHookRelayManager.setManagedHookSettingsResolver(() => store?.getSettings() ?? null) logStartupMilestone('store-loaded') // Why: apply initial fallback WSL distro from store settings for global git/CLI calls. setDefaultWslDistroOverride(store.getSettings().terminalWindowsWslDistro ?? null) @@ -2557,7 +2558,17 @@ void app.whenReady().then(async () => { 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())) { - runManagedHookInstallers(MANAGED_AGENT_HOOK_INSTALLERS) + const managedHookStore = store + void applyAgentStatusHooksEnabled(true, managedHookStore.getSettings(), { + shouldHydrateShellPath: app.isPackaged && process.platform !== 'win32', + onInstallError: recordManagedHookInstallFailure, + shouldContinue: (agent) => { + const settings = managedHookStore.getSettings() + return isAgentStatusHooksEnabled(settings) && !settings.disabledTuiAgents.includes(agent) + } + }).catch((error) => { + console.warn('[agent-hooks] failed to reconcile managed hooks on startup:', error) + }) } else { removeManagedAgentHooks() } diff --git a/src/main/ipc/settings.test.ts b/src/main/ipc/settings.test.ts index 71ca362f08e..f91c6699850 100644 --- a/src/main/ipc/settings.test.ts +++ b/src/main/ipc/settings.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi, beforeEach } from 'vitest' const { applyAppIconMock, + applyAgentStatusHooksEnabledMock, applyElectronProxySettingsMock, browserWindowGetAllWindowsMock, handleMock, @@ -13,6 +14,7 @@ const { rebuildAppMenuMock } = vi.hoisted(() => ({ applyAppIconMock: vi.fn(), + applyAgentStatusHooksEnabledMock: vi.fn(), applyElectronProxySettingsMock: vi.fn(), browserWindowGetAllWindowsMock: vi.fn(), handleMock: vi.fn(), @@ -47,6 +49,10 @@ vi.mock('../app-icon', () => ({ applyAppIcon: applyAppIconMock })) +vi.mock('../agent-hooks/managed-agent-hook-controls', () => ({ + applyAgentStatusHooksEnabled: applyAgentStatusHooksEnabledMock +})) + vi.mock('../worktree-root-preparation', () => ({ prepareLocalWorktreeRootsForRepos: prepareLocalWorktreeRootsForReposMock })) @@ -81,6 +87,7 @@ describe('registerSettingsHandlers', () => { handleMock.mockClear() onMock.mockClear() applyAppIconMock.mockClear() + applyAgentStatusHooksEnabledMock.mockReset().mockResolvedValue([]) applyElectronProxySettingsMock.mockClear() applyElectronProxySettingsMock.mockResolvedValue({ source: 'settings' }) previewGhosttyImportMock.mockClear() @@ -121,6 +128,53 @@ describe('registerSettingsHandlers', () => { expect(event.returnValue).toEqual({ terminalMainSideEffectAuthority: false }) }) + it('does not reconcile hooks when the disabled-agent set is unchanged', async () => { + const before = { + agentStatusHooksEnabled: true, + disabledTuiAgents: ['codex', 'claude'] + } + store.getSettings.mockReturnValue(before) + store.updateSettings.mockReturnValue({ + ...before, + disabledTuiAgents: ['claude', 'codex'] + }) + registerSettingsHandlers(store as never) + const handler = handleMock.mock.calls.find((call) => call[0] === 'settings:set')?.[1] as ( + event: typeof settingsInvokeEvent, + args: { disabledTuiAgents: string[] } + ) => Promise + + await handler(settingsInvokeEvent, { disabledTuiAgents: ['claude', 'codex'] }) + + expect(applyAgentStatusHooksEnabledMock).not.toHaveBeenCalled() + }) + + it('reconciles hooks when the disabled-agent set changes', async () => { + const before = { + agentStatusHooksEnabled: true, + disabledTuiAgents: ['codex', 'claude'] + } + const updated = { + ...before, + disabledTuiAgents: ['claude'] + } + store.getSettings.mockReturnValue(before) + store.updateSettings.mockReturnValue(updated) + registerSettingsHandlers(store as never) + const handler = handleMock.mock.calls.find((call) => call[0] === 'settings:set')?.[1] as ( + event: typeof settingsInvokeEvent, + args: { disabledTuiAgents: string[] } + ) => Promise + + await handler(settingsInvokeEvent, { disabledTuiAgents: ['claude'] }) + + expect(applyAgentStatusHooksEnabledMock).toHaveBeenCalledWith( + true, + updated, + expect.objectContaining({ shouldContinue: expect.any(Function) }) + ) + }) + it('rejects durable Active Server writes through generic settings:set', async () => { store.getSettings.mockReturnValue({ activeRuntimeEnvironmentId: null }) store.updateSettings.mockReturnValue({ activeRuntimeEnvironmentId: null }) diff --git a/src/main/ipc/settings.ts b/src/main/ipc/settings.ts index 0166f4ad5ee..c5be32dd32c 100644 --- a/src/main/ipc/settings.ts +++ b/src/main/ipc/settings.ts @@ -11,6 +11,7 @@ import { SETTINGS_CHANGED_WHITELIST, type SettingsChangedKey } from '../../share import type { AgentAwakeService } from '../agent-awake-service' import { sanitizeFloatingWorkspaceDirectorySetting } from './floating-workspace-directory' import { applyAgentStatusHooksEnabled } from '../agent-hooks/managed-agent-hook-controls' +import { recordManagedHookInstallFailure } from '../agent-hooks/install-telemetry' import { applyElectronProxySettings } from '../network/proxy-settings' import { normalizeProxyBypassRules, normalizeProxyUrl } from '../../shared/network-proxy' import { normalizeAppIconId } from '../../shared/app-icon' @@ -23,6 +24,7 @@ import { prepareLocalWorktreeRootsForRepos } from '../worktree-root-preparation' import { scheduleCurrentWorktreeBaseDirectoryWatcherSync } from './worktree-base-directory-watcher' import { applyPRBotAuthorOverride } from '../../shared/pr-bot-author-overrides' import { resolveEnvironment } from '../../shared/runtime-environment-store' +import { haveSameDisabledTuiAgents } from '../../shared/tui-agent-selection' // Why: the whitelist is the source-of-truth for which keys we emit on. Casting // to a Set once at module load lets the IPC handler's per-key membership @@ -148,14 +150,26 @@ export function registerSettingsHandlers( if ('keepComputerAwakeWhileAgentsRun' in sanitizedArgs) { agentAwakeService?.setEnabled(result.keepComputerAwakeWhileAgentsRun) } - if ( - 'agentStatusHooksEnabled' in sanitizedArgs && - before.agentStatusHooksEnabled !== result.agentStatusHooksEnabled - ) { + const hookSettingChanged = + ('agentStatusHooksEnabled' in sanitizedArgs && + before.agentStatusHooksEnabled !== result.agentStatusHooksEnabled) || + ('disabledTuiAgents' in sanitizedArgs && + !haveSameDisabledTuiAgents(before.disabledTuiAgents, result.disabledTuiAgents)) + if (hookSettingChanged) { try { - applyAgentStatusHooksEnabled(result.agentStatusHooksEnabled) + await applyAgentStatusHooksEnabled(result.agentStatusHooksEnabled, result, { + shouldHydrateShellPath: app.isPackaged && process.platform !== 'win32', + onInstallError: recordManagedHookInstallFailure, + shouldContinue: (agent) => { + const settings = store.getSettings() + return ( + settings.agentStatusHooksEnabled !== false && + !settings.disabledTuiAgents.includes(agent) + ) + } + }) } catch (error) { - console.warn('[settings] failed to apply agentStatusHooksEnabled:', error) + console.warn('[settings] failed to reconcile managed agent hooks:', error) } } if ('uiLanguage' in sanitizedArgs && before.uiLanguage !== result.uiLanguage) { diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 8fe931323fd..72663c7f908 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -182,7 +182,7 @@ const electronMocks = vi.hoisted(() => { BrowserWindow: { fromId: vi.fn((_id: number): unknown => null) }, webContents: { fromId: vi.fn((_id: number): unknown => null) }, ipcMain, - app: { getPath: vi.fn(() => '/tmp') } + app: { getPath: vi.fn(() => '/tmp'), isPackaged: false } } }) @@ -255,6 +255,7 @@ const { addGitHubIssueCommentMock, listGitHubLabelsMock, listGitHubAssignableUsersMock, + applyAgentStatusHooksEnabledMock, detectInstalledAgentsWithShellPathHydrationMock, detectRemoteAgentsMock, markCodexProjectTrustedMock, @@ -361,6 +362,7 @@ const { addGitHubIssueCommentMock: vi.fn(), listGitHubLabelsMock: vi.fn(), listGitHubAssignableUsersMock: vi.fn(), + applyAgentStatusHooksEnabledMock: vi.fn(), detectInstalledAgentsWithShellPathHydrationMock: vi.fn(), detectRemoteAgentsMock: vi.fn(), markCodexProjectTrustedMock: vi.fn(), @@ -434,6 +436,10 @@ vi.mock('../ipc/preflight', () => ({ detectRemoteAgents: detectRemoteAgentsMock })) +vi.mock('../agent-hooks/managed-agent-hook-controls', () => ({ + applyAgentStatusHooksEnabled: applyAgentStatusHooksEnabledMock +})) + vi.mock('../agent-trust-presets', () => ({ markCodexProjectTrusted: markCodexProjectTrustedMock, markCopilotFolderTrusted: markCopilotFolderTrustedMock, @@ -617,6 +623,7 @@ vi.mock('../git/git-username', async () => { function resetRuntimeTestMocks(): void { resetPlatform() + electronMocks.app.isPackaged = false clearConfiguredWorktreeSharedDirectoriesCacheForTests() _resetTerminalViewAttributesForTest() advertisedUrlWatcher.clear() @@ -663,6 +670,7 @@ function resetRuntimeTestMocks(): void { }) muxRequestMock.mockReset() muxRequestMock.mockResolvedValue(undefined) + applyAgentStatusHooksEnabledMock.mockReset().mockResolvedValue([]) getActiveMultiplexerMock.mockReset() getActiveMultiplexerMock.mockReturnValue({ request: muxRequestMock, notify: vi.fn() }) vi.mocked(createSetupRunnerScript).mockReset() @@ -1665,7 +1673,7 @@ describe('OrcaRuntimeService', () => { expect(updateSettings).not.toHaveBeenCalled() }) - it('accepts runtime-backed setting updates from paired clients', () => { + it('accepts runtime-backed setting updates from paired clients', async () => { let settings = { ...store.getSettings(), experimentalNewWorktreeCardStyle: false, @@ -1684,7 +1692,7 @@ describe('OrcaRuntimeService', () => { } as never) expect( - runtime.updateClientSettings({ + await runtime.updateClientSettings({ experimentalNewWorktreeCardStyle: true, compactWorktreeCards: true, minimaxGroupId: 'group-42', @@ -1713,6 +1721,77 @@ describe('OrcaRuntimeService', () => { }) }) + it('reconciles hooks only when paired-client hook settings change', async () => { + electronMocks.app.isPackaged = true + let settings = { + ...store.getSettings(), + agentStatusHooksEnabled: true, + disabledTuiAgents: ['codex', 'claude'] + } + const updateSettings = vi.fn((updates: Partial) => { + settings = { ...settings, ...updates } + return settings + }) + const runtime = new OrcaRuntimeService({ + ...store, + getSettings: () => settings, + updateSettings + } as never) + + await runtime.updateClientSettings({ disabledTuiAgents: ['claude', 'codex'] }) + expect(applyAgentStatusHooksEnabledMock).not.toHaveBeenCalled() + + await runtime.updateClientSettings({ disabledTuiAgents: ['claude'] }) + expect(applyAgentStatusHooksEnabledMock).toHaveBeenCalledOnce() + expect(applyAgentStatusHooksEnabledMock).toHaveBeenCalledWith( + true, + expect.objectContaining({ disabledTuiAgents: ['claude'] }), + expect.objectContaining({ + shouldContinue: expect.any(Function), + shouldHydrateShellPath: process.platform !== 'win32' + }) + ) + }) + + it('serializes paired-client hook reconciliation and reads current settings', async () => { + let settings = { + ...store.getSettings(), + agentStatusHooksEnabled: true, + disabledTuiAgents: ['codex', 'claude'] + } + const updateSettings = vi.fn((updates: Partial) => { + settings = { ...settings, ...updates } + return settings + }) + const firstReconciliation = deferred<[]>() + applyAgentStatusHooksEnabledMock + .mockImplementationOnce(() => firstReconciliation.promise) + .mockResolvedValueOnce([]) + const runtime = new OrcaRuntimeService({ + ...store, + getSettings: () => settings, + updateSettings + } as never) + + const first = runtime.updateClientSettings({ disabledTuiAgents: ['claude'] }) + await vi.waitFor(() => expect(applyAgentStatusHooksEnabledMock).toHaveBeenCalledOnce()) + const second = runtime.updateClientSettings({ disabledTuiAgents: [] }) + + expect(applyAgentStatusHooksEnabledMock).toHaveBeenCalledOnce() + const firstOptions = applyAgentStatusHooksEnabledMock.mock.calls[0]?.[2] + expect(firstOptions?.shouldContinue?.('claude')).toBe(true) + + firstReconciliation.resolve([]) + await Promise.all([first, second]) + + expect(applyAgentStatusHooksEnabledMock).toHaveBeenCalledTimes(2) + expect(applyAgentStatusHooksEnabledMock).toHaveBeenLastCalledWith( + true, + expect.objectContaining({ disabledTuiAgents: [] }), + expect.objectContaining({ shouldContinue: expect.any(Function) }) + ) + }) + it('rejects relative paths for runtime nested repo scan/import', async () => { const runtime = new OrcaRuntimeService({ ...store, diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 8376ca52f08..161314dbb16 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -386,7 +386,11 @@ import { isExpectedAgentProcess, recognizeAgentProcess } from '../../shared/agent-process-recognition' -import { isTuiAgentEnabled, pickTuiAgent } from '../../shared/tui-agent-selection' +import { + haveSameDisabledTuiAgents, + isTuiAgentEnabled, + pickTuiAgent +} from '../../shared/tui-agent-selection' import { resolveTuiAgentLaunchArgs, resolveTuiAgentLaunchEnv @@ -406,6 +410,7 @@ import { } from '../agent-trust-presets' import { markRemoteAgentWorkspaceTrusted } from '../remote-agent-trust-presets' import { applyAgentStatusHooksEnabled } from '../agent-hooks/managed-agent-hook-controls' +import { recordManagedHookInstallFailure } from '../agent-hooks/install-telemetry' import { isWindowsAbsolutePathLike, isPathInsideOrEqual, @@ -533,7 +538,7 @@ import { } from '../../shared/claude-agent-teams-tmux-compat' import { joinWorktreeRelativePath } from './runtime-relative-paths' import { collectMemorySnapshot } from '../memory/collector' -import { BrowserWindow, ipcMain, Notification } from 'electron' +import { app, BrowserWindow, ipcMain, Notification } from 'electron' import type { AgentBrowserBridge } from '../browser/agent-browser-bridge' import type { BrowserBackend } from '../browser/browser-backend' import { BrowserError } from '../browser/cdp-bridge' @@ -2601,6 +2606,8 @@ export class OrcaRuntimeService { private readonly runtimeId = randomUUID() private readonly startedAt = Date.now() private readonly store: RuntimeStore | null + private managedHookReconciliationGeneration = 0 + private managedHookReconciliationTail: Promise = Promise.resolve() private readonly orchestrationEnvironmentTransport: OrchestrationEnvironmentTransport | null private readonly orchestrationFederationTimers = new Map>() private readonly orchestrationFederationSyncs = new Map>() @@ -3346,7 +3353,34 @@ export class OrcaRuntimeService { } } - updateClientSettings( + private reconcileManagedAgentHooks(): Promise { + const generation = ++this.managedHookReconciliationGeneration + const reconciliation = this.managedHookReconciliationTail.then(async () => { + if (generation !== this.managedHookReconciliationGeneration) { + return + } + const settings = this.store?.getSettings() + if (!settings) { + return + } + await applyAgentStatusHooksEnabled(settings.agentStatusHooksEnabled !== false, settings, { + shouldHydrateShellPath: app.isPackaged && process.platform !== 'win32', + onInstallError: recordManagedHookInstallFailure, + shouldContinue: (agent) => { + const current = this.store?.getSettings() + return ( + current !== undefined && + current.agentStatusHooksEnabled !== false && + !current.disabledTuiAgents?.includes(agent) + ) + } + }) + }) + this.managedHookReconciliationTail = reconciliation.catch(() => {}) + return reconciliation + } + + async updateClientSettings( updates: Pick< Partial, | 'agentStatusHooksEnabled' @@ -3366,36 +3400,42 @@ export class OrcaRuntimeService { | 'minimaxUsageModels' | 'prBotAuthorOverrides' > - ): Pick< - GlobalSettings, - | 'defaultTuiAgent' - | 'disabledTuiAgents' - | 'agentCmdOverrides' - | 'agentDefaultArgs' - | 'agentDefaultEnv' - | 'agentStatusHooksEnabled' - | 'defaultTaskSource' - | 'defaultTaskViewPreset' - | 'visibleTaskProviders' - | 'defaultRepoSelection' - | 'defaultLinearTeamSelection' - | 'githubProjects' - | 'experimentalNewWorktreeCardStyle' - | 'compactWorktreeCards' - | 'minimaxGroupId' - | 'minimaxUsageModels' - | 'prBotAuthorOverrides' + ): Promise< + Pick< + GlobalSettings, + | 'defaultTuiAgent' + | 'disabledTuiAgents' + | 'agentCmdOverrides' + | 'agentDefaultArgs' + | 'agentDefaultEnv' + | 'agentStatusHooksEnabled' + | 'defaultTaskSource' + | 'defaultTaskViewPreset' + | 'visibleTaskProviders' + | 'defaultRepoSelection' + | 'defaultLinearTeamSelection' + | 'githubProjects' + | 'experimentalNewWorktreeCardStyle' + | 'compactWorktreeCards' + | 'minimaxGroupId' + | 'minimaxUsageModels' + | 'prBotAuthorOverrides' + > > { if (!this.store?.getSettings || !this.store.updateSettings) { throw new Error('runtime_unavailable') } - const before = this.store.getSettings().agentStatusHooksEnabled !== false + const beforeSettings = this.store.getSettings() + const before = beforeSettings.agentStatusHooksEnabled !== false this.store.updateSettings(updates, { notifyListeners: true }) + const settings = this.store.getSettings() if ( - typeof updates.agentStatusHooksEnabled === 'boolean' && - before !== updates.agentStatusHooksEnabled + (typeof updates.agentStatusHooksEnabled === 'boolean' && + before !== updates.agentStatusHooksEnabled) || + (updates.disabledTuiAgents !== undefined && + !haveSameDisabledTuiAgents(beforeSettings.disabledTuiAgents, settings.disabledTuiAgents)) ) { - applyAgentStatusHooksEnabled(updates.agentStatusHooksEnabled) + await this.reconcileManagedAgentHooks() } return this.getClientSettings() } diff --git a/src/main/runtime/rpc/methods/client-ui.ts b/src/main/runtime/rpc/methods/client-ui.ts index 17ebfade6aa..a10f1b66708 100644 --- a/src/main/runtime/rpc/methods/client-ui.ts +++ b/src/main/runtime/rpc/methods/client-ui.ts @@ -20,7 +20,9 @@ export const CLIENT_UI_METHODS: RpcMethod[] = [ defineMethod({ name: 'settings.update', params: SettingsUpdate, - handler: (params, { runtime }) => ({ settings: runtime.updateClientSettings(params) }) + handler: async (params, { runtime }) => ({ + settings: await runtime.updateClientSettings(params) + }) }), defineMethod({ name: 'settings.getTerminalQuickCommands', diff --git a/src/main/ssh/ssh-relay-session-managed-hooks.test.ts b/src/main/ssh/ssh-relay-session-managed-hooks.test.ts new file mode 100644 index 00000000000..95492aa3a3a --- /dev/null +++ b/src/main/ssh/ssh-relay-session-managed-hooks.test.ts @@ -0,0 +1,131 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, + AGENT_HOOK_INSTALL_PLUGINS_METHOD +} from '../../shared/agent-hook-relay' +import { SshRelaySession } from './ssh-relay-session' +import type { SshConnection } from './ssh-connection' +import { createMockDeps, mockDeploySuccess } from './ssh-relay-session-test-fixtures' + +const { muxRequestMock, openConsumerSessionMock } = vi.hoisted(() => ({ + muxRequestMock: vi.fn(), + openConsumerSessionMock: vi.fn() +})) + +vi.mock('./ssh-relay-deploy', () => ({ deployAndLaunchRelay: vi.fn() })) +vi.mock('./ssh-relay-deploy-helpers', () => ({ execCommand: vi.fn().mockResolvedValue('') })) +vi.mock('./ssh-pty-consumer-session', () => ({ + SSH_PTY_SOURCE_WINDOW_SU: 256 * 1024, + openSshPtyConsumerSession: openConsumerSessionMock +})) +vi.mock('./ssh-channel-multiplexer', () => ({ + SshChannelMultiplexer: class MockSshChannelMultiplexer { + notify = vi.fn() + notifyWithSettlement = vi.fn() + request = muxRequestMock + onNotification = vi.fn().mockReturnValue(() => {}) + onNotificationByMethod = vi.fn().mockReturnValue(() => {}) + onRequest = vi.fn().mockReturnValue(() => {}) + onDispose = vi.fn().mockReturnValue(() => {}) + dispose = vi.fn() + isDisposed = vi.fn().mockReturnValue(false) + } +})) +vi.mock('../providers/ssh-pty-provider', () => ({ + isSshPtyNotFoundError: vi.fn(() => false), + isSshPtyIdentityMismatchError: vi.fn(() => false), + SshPtyProvider: class MockSshPtyProvider { + onData = vi.fn().mockReturnValue(() => {}) + onReplay = vi.fn().mockReturnValue(() => {}) + onExit = vi.fn().mockReturnValue(() => {}) + dispose = vi.fn() + } +})) +vi.mock('../providers/ssh-filesystem-provider', () => ({ + SshFilesystemProvider: class MockSshFilesystemProvider { + dispose = vi.fn() + } +})) +vi.mock('../providers/ssh-git-provider', () => ({ + SshGitProvider: class MockSshGitProvider {} +})) +vi.mock('../ipc/pty', () => ({ + registerSshPtyProvider: vi.fn(), + unregisterSshPtyProvider: vi.fn(), + getSshPtyProvider: vi.fn(), + getPtyIdsForConnection: vi.fn().mockReturnValue([]), + clearPtyOwnershipForConnection: vi.fn(), + clearProviderPtyState: vi.fn(), + deletePtyOwnership: vi.fn(), + setPtyOwnership: vi.fn(), + restorePtyIncarnation: vi.fn(), + isCurrentPtyExit: vi.fn(() => true) +})) +vi.mock('../providers/ssh-filesystem-dispatch', () => ({ + registerSshFilesystemProvider: vi.fn(), + unregisterSshFilesystemProvider: vi.fn(), + getSshFilesystemProvider: vi.fn() +})) +vi.mock('../providers/ssh-git-dispatch', () => ({ + registerSshGitProvider: vi.fn(), + unregisterSshGitProvider: vi.fn() +})) + +const { registerSshPtyProvider } = await import('../ipc/pty') + +describe('SshRelaySession managed hooks', () => { + beforeEach(() => { + vi.clearAllMocks() + process.env.ORCA_FEATURE_REMOTE_AGENT_HOOKS = '1' + openConsumerSessionMock.mockImplementation(async (_mux, options) => ({ + mode: 'legacy-fallback', + clientInstanceId: options.clientInstanceId, + serverBuildId: 'test-relay-build' + })) + mockDeploySuccess() + }) + + it('installs only detected hooks without blocking provider registration', async () => { + muxRequestMock.mockImplementation(async (method: string) => { + if (method === 'preflight.detectAgents') { + return { agents: ['codex'] } + } + return method === AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD + ? { installers: 1, errors: 0 } + : { ok: true } + }) + const { mockStore, mockPortForward, getMainWindow } = createMockDeps() + const sftp = vi.fn() + const connection = { + sftp, + getHostKeyFingerprint: vi.fn(() => 'SHA256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA') + } as unknown as SshConnection + const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) + + await session.establish(connection) + await vi.waitFor(() => + expect(muxRequestMock).toHaveBeenCalledWith(AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, { + hostKeyFingerprint: 'SHA256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA', + agents: ['codex'] + }) + ) + + const managedIndex = muxRequestMock.mock.calls.findIndex( + ([method]) => method === AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD + ) + const pluginsIndex = muxRequestMock.mock.calls.findIndex( + ([method]) => method === AGENT_HOOK_INSTALL_PLUGINS_METHOD + ) + expect(muxRequestMock.mock.calls[pluginsIndex]?.[1]).toMatchObject({ + piExtensionSource: expect.stringContaining('/hook/pi'), + ompExtensionSource: expect.stringContaining('/hook/omp') + }) + expect(sftp).not.toHaveBeenCalled() + expect(muxRequestMock.mock.invocationCallOrder[pluginsIndex]).toBeLessThan( + vi.mocked(registerSshPtyProvider).mock.invocationCallOrder[0] + ) + expect(vi.mocked(registerSshPtyProvider).mock.invocationCallOrder[0]).toBeLessThan( + muxRequestMock.mock.invocationCallOrder[managedIndex] + ) + }) +}) diff --git a/src/main/ssh/ssh-relay-session.test.ts b/src/main/ssh/ssh-relay-session.test.ts index 40b38a75b52..ad563d002b5 100644 --- a/src/main/ssh/ssh-relay-session.test.ts +++ b/src/main/ssh/ssh-relay-session.test.ts @@ -207,50 +207,12 @@ describe('SshRelaySession', () => { expect(registerSshGitProvider).toHaveBeenCalledWith('target-1', expect.anything()) }) - it('installs all managed hooks in one relay RPC before plugins and PTY registration', async () => { - process.env.ORCA_FEATURE_REMOTE_AGENT_HOOKS = '1' - muxRequestMock.mockResolvedValue({ installers: 14, errors: 0 }) - const { mockStore, mockPortForward, getMainWindow } = createMockDeps() - const sftp = vi.fn() - const mockConn = { - sftp, - getHostKeyFingerprint: vi.fn(() => 'SHA256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA') - } as unknown as SshConnection - const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) - - await session.establish(mockConn) - - const managedHookCalls = muxRequestMock.mock.calls.filter( - ([method]) => method === AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD - ) - const managedHookCallIndex = muxRequestMock.mock.calls.findIndex( - ([method]) => method === AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD - ) - const installPluginsCallIndex = muxRequestMock.mock.calls.findIndex( - ([method]) => method === AGENT_HOOK_INSTALL_PLUGINS_METHOD - ) - expect(managedHookCalls).toEqual([ - [ - AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, - { hostKeyFingerprint: 'SHA256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA' } - ] - ]) - expect(installPluginsCallIndex).toBeGreaterThanOrEqual(0) - const installPluginsParams = muxRequestMock.mock.calls[installPluginsCallIndex]?.[1] - expect(installPluginsParams).toMatchObject({ - piExtensionSource: expect.stringContaining('/hook/pi'), - ompExtensionSource: expect.stringContaining('/hook/omp') - }) - expect(sftp).not.toHaveBeenCalled() - expect(managedHookCallIndex).toBeLessThan(installPluginsCallIndex) - expect(muxRequestMock.mock.invocationCallOrder[installPluginsCallIndex]).toBeLessThan( - vi.mocked(registerSshPtyProvider).mock.invocationCallOrder[0] - ) - }) - it('continues provider registration when the relay managed-hook request fails', async () => { process.env.ORCA_FEATURE_REMOTE_AGENT_HOOKS = '1' muxRequestMock.mockImplementation(async (method: string) => { + if (method === 'preflight.detectAgents') { + return { agents: ['codex'] } + } if (method === AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD) { throw new Error('runtime unavailable') } @@ -260,6 +222,12 @@ describe('SshRelaySession', () => { const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) await session.establish({} as SshConnection) + await vi.waitFor(() => + expect(muxRequestMock).toHaveBeenCalledWith( + AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, + expect.anything() + ) + ) expect(registerSshPtyProvider).toHaveBeenCalledWith('target-1', expect.anything()) expect( @@ -270,6 +238,9 @@ describe('SshRelaySession', () => { it('suppresses expected managed-hook teardown errors during disconnect', async () => { process.env.ORCA_FEATURE_REMOTE_AGENT_HOOKS = '1' muxRequestMock.mockImplementation(async (method: string) => { + if (method === 'preflight.detectAgents') { + return { agents: ['codex'] } + } if (method === AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD) { throw Object.assign(new Error('request disposed'), { code: 'DISPOSED' }) } @@ -281,6 +252,12 @@ describe('SshRelaySession', () => { try { await session.establish({} as SshConnection) + await vi.waitFor(() => + expect(muxRequestMock).toHaveBeenCalledWith( + AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, + expect.anything() + ) + ) expect(registerSshPtyProvider).toHaveBeenCalledWith('target-1', expect.anything()) expect(warn.mock.calls.flat().join(' ')).not.toContain('relay managed hook install failed') diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index 0af4f59969b..aef993f8eef 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -18,6 +18,10 @@ import { SshFilesystemProvider } from '../providers/ssh-filesystem-provider' import { SshGitProvider } from '../providers/ssh-git-provider' import { agentHookServer } from '../agent-hooks/server' import { isAgentStatusHooksEnabled } from '../agent-hooks/managed-agent-hook-controls' +import { + buildManagedHookDetectionCommands, + detectedManagedHookAgents +} from '../agent-hooks/managed-hook-detection-commands' import { AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, AGENT_HOOK_INSTALL_PLUGINS_METHOD, @@ -697,11 +701,6 @@ export class SshRelaySession { return false } - await this.installManagedHooksOnRemote(mux) - if (shouldContinue && !shouldContinue()) { - return false - } - await this.installPluginsOnRelay(mux) if (shouldContinue && !shouldContinue()) { return false @@ -833,6 +832,7 @@ export class SshRelaySession { this.wireUpPtyEvents(ptyProvider, mux, providerGeneration) this.wireUpAgentHookEvents(mux) this.wireUpRemoteWorkspaceEvents(mux) + void this.installManagedHooksOnRemote(mux, shouldContinue) return true } @@ -926,9 +926,15 @@ export class SshRelaySession { }) } - // Why: hooks must exist before PTY spawn; relay-local work keeps all managed installs to one SSH round trip. - private async installManagedHooksOnRemote(mux: SshChannelMultiplexer): Promise { - if (!isRemoteAgentHooksEnabled() || !this.areAgentStatusHooksEnabled()) { + private async installManagedHooksOnRemote( + mux: SshChannelMultiplexer, + shouldContinue?: () => boolean + ): Promise { + if ( + !isRemoteAgentHooksEnabled() || + !this.areAgentStatusHooksEnabled() || + (shouldContinue && !shouldContinue()) + ) { return } if ( @@ -940,8 +946,19 @@ export class SshRelaySession { } try { + const store = this.store as { getSettings?: Store['getSettings'] } + const detected = (await mux.request('preflight.detectAgents', { + commands: buildManagedHookDetectionCommands(store.getSettings?.() ?? null, 'linux') + })) as { agents?: unknown } + const agents = detectedManagedHookAgents(detected?.agents) + if (agents.length === 0 || (shouldContinue && !shouldContinue())) { + return + } const hostKeyFingerprint = this.requireReadyConnection().getHostKeyFingerprint?.() - const params = hostKeyFingerprint ? { hostKeyFingerprint } : {} + const params = { + ...(hostKeyFingerprint ? { hostKeyFingerprint } : {}), + agents + } const result = (await mux.request(AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD, params)) as { errors?: unknown } diff --git a/src/relay/managed-hook-installer.test.ts b/src/relay/managed-hook-installer.test.ts index a060114ffcc..f33086e871f 100644 --- a/src/relay/managed-hook-installer.test.ts +++ b/src/relay/managed-hook-installer.test.ts @@ -27,11 +27,14 @@ describe('registerManagedHookInstaller', () => { const installManagedHooks = vi.fn().mockResolvedValue({ installers: 14, errors: 0 }) const handler = captureHandler(() => ({ installManagedHooks })) - await expect(handler({}, context(controller.signal))).resolves.toEqual({ + await expect(handler({ agents: ['codex'] }, context(controller.signal))).resolves.toEqual({ installers: 14, errors: 0 }) - expect(installManagedHooks).toHaveBeenCalledWith({ signal: controller.signal }) + expect(installManagedHooks).toHaveBeenCalledWith({ + signal: controller.signal, + agents: ['codex'] + }) }) it('does not load or start the runtime for an already-cancelled request', async () => { @@ -51,13 +54,44 @@ describe('registerManagedHookInstaller', () => { const handler = captureHandler(() => ({ installManagedHooks })) const fingerprint = 'SHA256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA' - await handler({ hostKeyFingerprint: fingerprint }, context()) - await handler({ hostKeyFingerprint: 'ssh://untrusted-host' }, context()) + await handler({ hostKeyFingerprint: fingerprint, agents: ['codex'] }, context()) + await handler({ hostKeyFingerprint: 'ssh://untrusted-host', agents: ['codex'] }, context()) expect(installManagedHooks).toHaveBeenNthCalledWith(1, { signal: undefined, - hostKeyFingerprint: fingerprint + hostKeyFingerprint: fingerprint, + agents: ['codex'] }) - expect(installManagedHooks).toHaveBeenNthCalledWith(2, { signal: undefined }) + expect(installManagedHooks).toHaveBeenNthCalledWith(2, { + signal: undefined, + agents: ['codex'] + }) + }) + + it('fails closed when the detected agent allowlist is omitted', async () => { + const installManagedHooks = vi.fn().mockResolvedValue({ installers: 0, errors: 0 }) + const handler = captureHandler(() => ({ installManagedHooks })) + + await handler({}, context()) + + expect(installManagedHooks).toHaveBeenCalledWith({ + signal: undefined, + agents: [] + }) + }) + + it('validates, deduplicates, and forwards the detected agent allowlist', async () => { + const installManagedHooks = vi.fn().mockResolvedValue({ installers: 1, errors: 0 }) + const handler = captureHandler(() => ({ installManagedHooks })) + + await handler({ agents: ['codex', 'codex'] }, context()) + + expect(installManagedHooks).toHaveBeenCalledWith({ + signal: undefined, + agents: ['codex'] + }) + await expect(handler({ agents: ['unknown'] }, context())).rejects.toThrow( + 'invalid_managed_hook_agents' + ) }) }) diff --git a/src/relay/managed-hook-installer.ts b/src/relay/managed-hook-installer.ts index a27a6b8bf49..4bb67da2460 100644 --- a/src/relay/managed-hook-installer.ts +++ b/src/relay/managed-hook-installer.ts @@ -4,6 +4,8 @@ import { type AgentHookInstallManagedHooksParams } from '../shared/agent-hook-relay' import type { RelayDispatcher, RequestContext } from './dispatcher' +import type { AgentHookTarget } from '../shared/agent-hook-types' +import { isManagedAgentHookTarget } from '../shared/managed-agent-hook-targets' export type ManagedHookInstallSummary = { installers: number @@ -14,6 +16,7 @@ export type ManagedHookRuntime = { installManagedHooks: (options?: { signal?: AbortSignal hostKeyFingerprint?: string + agents?: readonly AgentHookTarget[] }) => Promise } @@ -27,6 +30,17 @@ function readHostKeyFingerprint(params: unknown): string | undefined { : undefined } +function readAgents(params: unknown): AgentHookTarget[] { + const raw = (params as Partial | null)?.agents + if (raw === undefined) { + return [] + } + if (!Array.isArray(raw) || !raw.every(isManagedAgentHookTarget)) { + throw new Error('invalid_managed_hook_agents') + } + return [...new Set(raw)] +} + let managedHookRuntime: ManagedHookRuntime | null = null function loadManagedHookRuntime(): ManagedHookRuntime { @@ -47,9 +61,11 @@ export function registerManagedHookInstaller( async (params, context: RequestContext): Promise => { context.signal?.throwIfAborted() const hostKeyFingerprint = readHostKeyFingerprint(params) + const agents = readAgents(params) return await loadRuntime().installManagedHooks({ signal: context.signal, - ...(hostKeyFingerprint ? { hostKeyFingerprint } : {}) + ...(hostKeyFingerprint ? { hostKeyFingerprint } : {}), + agents }) } ) diff --git a/src/relay/wsl-agent-hook-relay.ts b/src/relay/wsl-agent-hook-relay.ts index 534c388e07c..384fbc2c303 100644 --- a/src/relay/wsl-agent-hook-relay.ts +++ b/src/relay/wsl-agent-hook-relay.ts @@ -18,6 +18,7 @@ import { RelayAgentHookServer } from './agent-hook-server' import { registerWslHookFsHandlers } from './wsl-hook-fs-bridge' import { PluginOverlayManager } from './plugin-overlay' import { createInstallPluginsHandler } from './wsl-install-plugins-handler' +import { PreflightHandler } from './preflight-handler' import { AGENT_HOOK_INSTALL_PLUGINS_METHOD, AGENT_HOOK_NOTIFICATION_METHOD, @@ -73,6 +74,7 @@ async function main(): Promise { envelope as unknown as Record ) }) + new PreflightHandler(dispatcher) dispatcher.onRequest(AGENT_HOOK_REQUEST_REPLAY_METHOD, async () => ({ replayed: hookServer.replayCachedPayloadsForPanes() diff --git a/src/shared/agent-hook-relay.ts b/src/shared/agent-hook-relay.ts index 9cbd6c18faa..71e7953170a 100644 --- a/src/shared/agent-hook-relay.ts +++ b/src/shared/agent-hook-relay.ts @@ -24,6 +24,7 @@ import type { ParsedAgentStatusPayload } from './agent-status-types' import type { AgentProviderSessionMetadata } from './agent-session-resume' +import type { AgentHookTarget } from './agent-hook-types' // Why: the local hook server knows the discriminator from URL pathname routing // (`/hook/`); the relay equally must tag each forwarded notification @@ -115,6 +116,8 @@ export const AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD = 'agent_hook.installManage export type AgentHookInstallManagedHooksParams = { /** SHA-256 fingerprint of the server key negotiated by Orca's SSH transport. */ hostKeyFingerprint?: string + /** Positively detected and enabled agents allowed to mutate remote config. */ + agents: readonly AgentHookTarget[] } /** Feature-flag env var. Read once at process start by Orca and the relay. diff --git a/src/shared/agent-hook-types.ts b/src/shared/agent-hook-types.ts index 707a89ffda3..5319dffceaf 100644 --- a/src/shared/agent-hook-types.ts +++ b/src/shared/agent-hook-types.ts @@ -21,7 +21,13 @@ export const AGENT_HOOK_TARGETS = [ ] as const export type AgentHookTarget = (typeof AGENT_HOOK_TARGETS)[number] -export type AgentHookInstallState = 'installed' | 'not_installed' | 'partial' | 'error' +export type AgentHookInstallState = 'installed' | 'not_installed' | 'partial' | 'error' | 'skipped' + +export type AgentHookInstallSkipReason = + | 'agent_disabled' + | 'cli_not_found' + | 'cli_presence_unknown' + | 'hooks_disabled' export type AgentHookInstallStatus = { agent: AgentHookTarget @@ -29,6 +35,7 @@ export type AgentHookInstallStatus = { configPath: string managedHooksPresent: boolean detail: string | null + skipReason?: AgentHookInstallSkipReason } // Why: bumped whenever the managed script's request shape changes. The diff --git a/src/shared/managed-agent-command-token.test.ts b/src/shared/managed-agent-command-token.test.ts new file mode 100644 index 00000000000..b58e9c7ac4e --- /dev/null +++ b/src/shared/managed-agent-command-token.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest' +import { + extractExecutableToken, + hasPathSeparatorToken, + isSafeExecutableBasename, + isSafeOverrideExecutableToken +} from './managed-agent-command-token' + +describe('managed agent command tokens', () => { + it('extracts quoted and escaped POSIX executable paths', () => { + expect(extractExecutableToken('"/opt/Agent Tools/codex" --flag', { platform: 'linux' })).toBe( + '/opt/Agent Tools/codex' + ) + expect(extractExecutableToken('/opt/Agent\\ Tools/codex --flag', { platform: 'linux' })).toBe( + '/opt/Agent Tools/codex' + ) + }) + + it('preserves Windows path separators', () => { + expect( + extractExecutableToken('"C:\\Program Files\\Claude\\claude.exe" --flag', { + platform: 'win32' + }) + ).toBe('C:\\Program Files\\Claude\\claude.exe') + }) + + it('distinguishes safe basenames from path tokens', () => { + expect(isSafeExecutableBasename('claude-code_1.2+')).toBe(true) + expect(isSafeExecutableBasename('../claude')).toBe(false) + expect(isSafeExecutableBasename('claude;echo')).toBe(false) + expect(hasPathSeparatorToken('C:\\Tools\\claude.exe')).toBe(true) + expect(hasPathSeparatorToken('/opt/codex')).toBe(true) + expect(hasPathSeparatorToken('codex')).toBe(false) + }) + + it('rejects traversal, control characters, and shell syntax in override paths', () => { + expect(isSafeOverrideExecutableToken('~/bin/codex')).toBe(true) + expect(isSafeOverrideExecutableToken('C:\\Program Files\\Claude\\claude.exe')).toBe(true) + expect(isSafeOverrideExecutableToken('../bin/codex')).toBe(false) + expect(isSafeOverrideExecutableToken('/opt/codex;echo')).toBe(false) + expect(isSafeOverrideExecutableToken('/opt/codex\0')).toBe(false) + }) +}) diff --git a/src/shared/managed-agent-command-token.ts b/src/shared/managed-agent-command-token.ts new file mode 100644 index 00000000000..4c767547a44 --- /dev/null +++ b/src/shared/managed-agent-command-token.ts @@ -0,0 +1,65 @@ +const QUOTES = new Set(['"', "'"]) + +type ExtractExecutableTokenOptions = { + platform?: NodeJS.Platform +} + +export function extractExecutableToken( + command: string | null | undefined, + options: ExtractExecutableTokenOptions = {} +): string | null { + const input = command?.trim() + if (!input) { + return null + } + const backslashEscapes = (options.platform ?? process.platform) !== 'win32' + let index = 0 + let quote: string | null = null + let token = '' + while (index < input.length) { + const char = input[index] + if (quote) { + if (char === quote) { + quote = null + } else if (char === '\\' && quote === '"' && backslashEscapes && index + 1 < input.length) { + index += 1 + token += input[index] + } else { + token += char + } + } else if (QUOTES.has(char)) { + quote = char + } else if (/\s/.test(char)) { + break + } else if (char === '\\' && backslashEscapes && index + 1 < input.length) { + index += 1 + token += input[index] + } else { + token += char + } + index += 1 + } + return token.length > 0 ? token : null +} + +export function hasPathSeparatorToken(token: string): boolean { + return token.includes('/') || token.includes('\\') +} + +export function isSafeExecutableBasename(token: string): boolean { + return /^[A-Za-z0-9._+-]+$/.test(token) +} + +export function isSafeOverrideExecutableToken(token: string): boolean { + if (token.includes('\0')) { + return false + } + if (!hasPathSeparatorToken(token)) { + return isSafeExecutableBasename(token) + } + return ( + !token.includes('..') && + !/[|&;<>(){}[\]$`"'*!?]/.test(token) && + /^[A-Za-z0-9._+\-/:\\~ ]+$/.test(token) + ) +} diff --git a/src/shared/managed-agent-hook-targets.ts b/src/shared/managed-agent-hook-targets.ts new file mode 100644 index 00000000000..a32dc63d411 --- /dev/null +++ b/src/shared/managed-agent-hook-targets.ts @@ -0,0 +1,32 @@ +import { AGENT_HOOK_TARGETS, type AgentHookTarget } from './agent-hook-types' +import { getTuiAgentDetectCommands, TUI_AGENT_CONFIG } from './tui-agent-config' +import type { TuiAgent } from './types' + +export type ManagedAgentHookTarget = { + agent: AgentHookTarget + tuiAgent: TuiAgent + executableCandidates: readonly string[] +} + +function target(agent: AgentHookTarget): ManagedAgentHookTarget { + return { + agent, + tuiAgent: agent, + executableCandidates: getTuiAgentDetectCommands(TUI_AGENT_CONFIG[agent]) + } +} + +export const MANAGED_AGENT_HOOK_TARGETS: readonly ManagedAgentHookTarget[] = + AGENT_HOOK_TARGETS.map(target) + +export function getManagedAgentHookTarget( + agent: AgentHookTarget +): ManagedAgentHookTarget | undefined { + return MANAGED_AGENT_HOOK_TARGETS.find((entry) => entry.agent === agent) +} + +export function isManagedAgentHookTarget(value: unknown): value is AgentHookTarget { + return ( + typeof value === 'string' && MANAGED_AGENT_HOOK_TARGETS.some((entry) => entry.agent === value) + ) +} diff --git a/src/shared/tui-agent-selection.test.ts b/src/shared/tui-agent-selection.test.ts index 50f106b845e..c92f5e62d37 100644 --- a/src/shared/tui-agent-selection.test.ts +++ b/src/shared/tui-agent-selection.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import { normalizeDisabledTuiAgents, pickTuiAgent } from './tui-agent-selection' +import { + haveSameDisabledTuiAgents, + normalizeDisabledTuiAgents, + pickTuiAgent +} from './tui-agent-selection' describe('pickTuiAgent', () => { it('uses an installed preferred agent', () => { @@ -30,3 +34,11 @@ describe('normalizeDisabledTuiAgents', () => { ]) }) }) + +describe('haveSameDisabledTuiAgents', () => { + it('compares the normalized disabled-agent sets', () => { + expect(haveSameDisabledTuiAgents(['codex', 'claude'], ['claude', 'codex'])).toBe(true) + expect(haveSameDisabledTuiAgents(['codex', 'unknown'], ['codex'])).toBe(true) + expect(haveSameDisabledTuiAgents(['codex'], ['claude'])).toBe(false) + }) +}) diff --git a/src/shared/tui-agent-selection.ts b/src/shared/tui-agent-selection.ts index 48e10305aa5..0256c0526e9 100644 --- a/src/shared/tui-agent-selection.ts +++ b/src/shared/tui-agent-selection.ts @@ -79,6 +79,12 @@ export function normalizeDisabledTuiAgents(value: unknown): TuiAgent[] { return [...seen] } +export function haveSameDisabledTuiAgents(left: unknown, right: unknown): boolean { + const leftSet = new Set(normalizeDisabledTuiAgents(left)) + const rightSet = new Set(normalizeDisabledTuiAgents(right)) + return leftSet.size === rightSet.size && [...leftSet].every((agent) => rightSet.has(agent)) +} + export function isTuiAgentEnabled(agent: TuiAgent, disabled?: Iterable | null): boolean { return !normalizeDisabledTuiAgents(disabled).includes(agent) }