From cbe5ea8f34fb3b684fd801ba80cf5eb993aa5a2b Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 7 Aug 2026 10:14:16 -0700 Subject: [PATCH] refactor: simplify retired-Gemini cleanup Collapse the settings migration from eleven shape-specific walkers into one key-matched depth-first pass (219 -> 104 lines), and trim the managed-hook cleanup to its read/strip/write core (120 -> 87 lines). Fixes two bugs the rewrite exposed: commitMessageAi kept its `enabled` flag (only automations disable on a retired agent), and already-disabled automations no longer re-trigger a save. Co-authored-by: Orca --- config/electron-builder.config.cjs | 1 - .../verify-agent-hook-stdin-lifecycle.mjs | 3 +- config/tsconfig.cli.json | 2 +- .../session/mobile-terminal-tab-agent.test.ts | 18 +- .../managed-agent-hook-controls.ts | 16 ++ src/main/agent-hooks/managed-hook-runtime.ts | 7 +- .../remote-managed-hook-installers.ts | 7 + .../retired-gemini-hook-cleanup.test.ts | 142 +++++++++++ .../retired-gemini-hook-cleanup.ts | 87 +++++++ src/main/agent-hooks/wsl-hook-fs-adapter.ts | 15 +- .../wsl-hook-relay-manager.test.ts | 39 +++- src/main/automations/headless-dispatch.ts | 77 ++++++ .../headless-workspace-create.test.ts | 2 + .../automations/headless-workspace-create.ts | 8 +- src/main/automations/service.test.ts | 58 +++++ src/main/automations/service.ts | 88 ++----- src/main/index.ts | 11 +- src/main/ipc/agent-hooks.test.ts | 3 - src/main/persistence.ts | 5 + .../retired-agent-settings-cleanup.test.ts | 60 ++++- src/main/retired-agent-settings-cleanup.ts | 220 ++++++------------ ...stration-worker-launch-preferences.test.ts | 6 - .../rpc/methods/worktree-schemas.test.ts | 22 +- .../runtime/rpc/methods/worktree-schemas.ts | 29 ++- .../ssh-relay-session-managed-hooks.test.ts | 26 +++ src/main/ssh/ssh-relay-session.ts | 5 +- .../automations/AutomationDetail.tsx | 6 +- .../automations/AutomationsPage.tsx | 9 +- .../automations/automation-draft-model.ts | 7 +- .../automation-host-client.test.ts | 2 +- .../automation-source-availability.ts | 91 ++++++++ .../automation-target-availability.ts | 91 +------- .../dashboard-worktree-launch-options.test.ts | 5 +- ...ote-hidden-output-restore-outcomes.test.ts | 2 - ...t-restore-unavailable-banner.repro.test.ts | 2 - .../src/hooks/useAutomationDispatchEvents.ts | 25 +- src/renderer/src/i18n/locales/en.json | 5 + .../lib/launch-agent-background-session.ts | 3 +- src/shared/agent-icons/gemini.png | Bin 1445 -> 0 bytes src/shared/automations-types.ts | 14 +- src/shared/tui-agent-config.ts | 9 + src/shared/tui-agent-draft-launch.ts | 97 ++++++++ src/shared/tui-agent-selection.test.ts | 14 ++ src/shared/tui-agent-selection.ts | 6 +- src/shared/tui-agent-session-option-props.ts | 8 + src/shared/tui-agent-startup.test.ts | 11 + src/shared/tui-agent-startup.ts | 91 +------- 47 files changed, 1016 insertions(+), 439 deletions(-) create mode 100644 src/main/agent-hooks/retired-gemini-hook-cleanup.test.ts create mode 100644 src/main/agent-hooks/retired-gemini-hook-cleanup.ts create mode 100644 src/renderer/src/components/automations/automation-source-availability.ts delete mode 100644 src/shared/agent-icons/gemini.png create mode 100644 src/shared/tui-agent-draft-launch.ts create mode 100644 src/shared/tui-agent-session-option-props.ts diff --git a/config/electron-builder.config.cjs b/config/electron-builder.config.cjs index b01c26997e1..be4554c6771 100644 --- a/config/electron-builder.config.cjs +++ b/config/electron-builder.config.cjs @@ -184,7 +184,6 @@ module.exports = { 'out/main/copilot/**', 'out/main/cursor/**', 'out/main/droid/**', - 'out/main/gemini/**', 'out/main/grok/**', 'out/main/hermes/**', 'out/main/daemon-entry.js', diff --git a/config/scripts/verify-agent-hook-stdin-lifecycle.mjs b/config/scripts/verify-agent-hook-stdin-lifecycle.mjs index 187b5aa3fc2..47a605a8377 100644 --- a/config/scripts/verify-agent-hook-stdin-lifecycle.mjs +++ b/config/scripts/verify-agent-hook-stdin-lifecycle.mjs @@ -26,13 +26,12 @@ const MANAGED_SCRIPTS = [ ['cursor-hook.sh', 'cursor'], ['devin-hook.sh', 'devin'], ['droid-hook.sh', 'droid'], - ['gemini-hook.sh', 'gemini'], ['grok-hook.sh', 'grok'], ['kimi-hook.sh', 'kimi'], ['openclaude-hook.sh', 'claude'] ] -const REQUIRED_JSON_STDOUT = new Set(['antigravity-hook.sh', 'copilot-hook.sh', 'gemini-hook.sh']) +const REQUIRED_JSON_STDOUT = new Set(['antigravity-hook.sh', 'copilot-hook.sh']) function parseArgs(argv) { const result = { home: process.env.HOME ?? '', minMtime: 0 } diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 1911a355225..6dc67e5e370 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -14,6 +14,7 @@ "../src/main/agent-hooks/managed-agent-hook-registry.ts", "../src/main/agent-hooks/managed-hook-script-refresh.ts", "../src/main/agent-hooks/runtime-home-hook-command.ts", + "../src/main/agent-hooks/retired-gemini-hook-cleanup.ts", "../src/main/amp/hook-service.ts", "../src/main/antigravity/hook-service.ts", "../src/main/claude/hook-settings.ts", @@ -65,7 +66,6 @@ "../src/main/copilot/hook-service.ts", "../src/main/cursor/hook-service.ts", "../src/main/droid/hook-service.ts", - "../src/main/gemini/hook-service.ts", "../src/main/grok/hook-service.ts", "../src/main/grok/windows-grok-hook-script.ts", "../src/main/devin/hook-settings.ts", diff --git a/mobile/src/session/mobile-terminal-tab-agent.test.ts b/mobile/src/session/mobile-terminal-tab-agent.test.ts index db6d95c228e..a1ea94974af 100644 --- a/mobile/src/session/mobile-terminal-tab-agent.test.ts +++ b/mobile/src/session/mobile-terminal-tab-agent.test.ts @@ -55,8 +55,10 @@ describe('resolveMobileTerminalTabAgentId', () => { ).toBe('codex') }) - it('falls back to explicit terminal titles when hook identity is unavailable', () => { - expect(resolveMobileTerminalTabAgentId(terminalTab('✦ Gemini CLI'))).toBe('gemini') + it('does not map retired Gemini title decorations to an agent id', () => { + // Gemini CLI is no longer a TuiAgent; title fallback must not invent an identity. + expect(resolveMobileTerminalTabAgentId(terminalTab('✦ Gemini CLI'))).toBeNull() + expect(resolveMobileTerminalTabAgentId(terminalTab('Codex ready'))).toBe('codex') }) it('does not let title fallback override launch identity', () => { @@ -76,8 +78,18 @@ describe('resolveMobileTerminalTabAgentId', () => { }) describe('getMobileSessionTabTitle', () => { + it('keeps retired Gemini titles unstripped when no agent identity is known', () => { + // No icon without identity; leave the raw title (including decoration) alone. + expect(getMobileSessionTabTitle(terminalTab('✦ Gemini CLI'))).toBe('✦ Gemini CLI') + }) + it('strips leading agent decorations when an icon is shown', () => { - expect(getMobileSessionTabTitle(terminalTab('✦ Gemini CLI'))).toBe('Gemini CLI') + expect(getMobileSessionTabTitle(terminalTab('Codex ready', { agentType: 'codex' }))).toBe( + 'Codex ready' + ) + expect(getMobileSessionTabTitle(terminalTab('✳ investigating', { agentType: 'claude' }))).toBe( + 'investigating' + ) }) it('falls back for glyph-only agent titles on mobile', () => { diff --git a/src/main/agent-hooks/managed-agent-hook-controls.ts b/src/main/agent-hooks/managed-agent-hook-controls.ts index 6f9f969668c..ee6e8ac3f87 100644 --- a/src/main/agent-hooks/managed-agent-hook-controls.ts +++ b/src/main/agent-hooks/managed-agent-hook-controls.ts @@ -13,6 +13,7 @@ import { MANAGED_AGENT_HOOK_STATUS_READERS, type ManagedAgentHookInstaller } from './managed-agent-hook-registry' +import { removeRetiredGeminiManagedHooksLocal } from './retired-gemini-hook-cleanup' export { MANAGED_AGENT_HOOK_INSTALLERS } from './managed-agent-hook-registry' export { prepareManagedCodexHomeBeforeShellLaunch } from '../codex/managed-home-shell-preflight' @@ -111,6 +112,12 @@ export async function installManagedAgentHooks( settings: ManagedHookSettings = null, options: InstallOptions = {} ): Promise { + // Why: Gemini CLI is gone; strip leftover managed hooks that 404 on /hook/gemini. + try { + removeRetiredGeminiManagedHooksLocal() + } catch (error) { + console.warn('[agent-hooks] Failed to clean retired Gemini managed hooks:', error) + } await refreshExistingManagedScripts(options) const installers = selectedInstallers(options) const disabled = new Set(normalizeDisabledTuiAgents(settings?.disabledTuiAgents)) @@ -167,6 +174,15 @@ export async function installManagedAgentHooks( } export function removeManagedAgentHooks(options: RemoveOptions = {}): AgentHookInstallStatus[] { + // Why: full uninstall must also clear retired Gemini entries that no longer + // have a remover in MANAGED_AGENT_HOOK_REMOVERS. + if (!options.agents) { + try { + removeRetiredGeminiManagedHooksLocal() + } catch (error) { + console.warn('[agent-hooks] Failed to clean retired Gemini managed hooks:', error) + } + } const allowed = options.agents ? new Set(options.agents) : null return MANAGED_AGENT_HOOK_REMOVERS.filter( ([agent]) => allowed === null || allowed.has(agent) diff --git a/src/main/agent-hooks/managed-hook-runtime.ts b/src/main/agent-hooks/managed-hook-runtime.ts index e995467ee7d..39df17291f3 100644 --- a/src/main/agent-hooks/managed-hook-runtime.ts +++ b/src/main/agent-hooks/managed-hook-runtime.ts @@ -85,7 +85,12 @@ export async function installManagedHooks(options?: { return { installers: 0, errors: 0 } } const home = homedir() - const grokHomeDir = await resolveRelayGrokHome(home, options?.signal) + // Why: an empty allowlist installs nothing (the request only carries retired-agent + // cleanup), so skip the login-shell GROK_HOME probe that install would need. + const grokHomeDir = + options?.agents?.length === 0 + ? defaultGrokHome(home) + : await resolveRelayGrokHome(home, options?.signal) options?.signal?.throwIfAborted() const hostIdentity = scopeManagedHookHostIdentity( await readManagedHookHostIdentity(), diff --git a/src/main/agent-hooks/remote-managed-hook-installers.ts b/src/main/agent-hooks/remote-managed-hook-installers.ts index b2283987171..1c47b10a99d 100644 --- a/src/main/agent-hooks/remote-managed-hook-installers.ts +++ b/src/main/agent-hooks/remote-managed-hook-installers.ts @@ -13,6 +13,7 @@ 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 { removeRetiredGeminiManagedHooksRemote } from './retired-gemini-hook-cleanup' export type RemoteManagedHookInstallOptions = { /** Explicit CODEX_HOME dir for redirected runtimes (WSL managed runtime @@ -80,6 +81,12 @@ export async function installRemoteManagedAgentHooks( remoteHome: string, options?: RemoteManagedHookInstallOptions ): Promise { + // Why: strip retired Gemini CLI hooks on remote homes the same way as local. + try { + await removeRetiredGeminiManagedHooksRemote(sftp, remoteHome) + } catch (error) { + console.warn('[agent-hooks] Failed to clean retired remote Gemini managed hooks:', error) + } // Why: omit/empty allowlist must never mean "install every agent" — that // recreates config homes for CLIs the user never installed (issue #11641). const allowedAgents = new Set(options?.agents ?? []) diff --git a/src/main/agent-hooks/retired-gemini-hook-cleanup.test.ts b/src/main/agent-hooks/retired-gemini-hook-cleanup.test.ts new file mode 100644 index 00000000000..2c96257cfe4 --- /dev/null +++ b/src/main/agent-hooks/retired-gemini-hook-cleanup.test.ts @@ -0,0 +1,142 @@ +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync, existsSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { homedirMock } = vi.hoisted(() => ({ + homedirMock: vi.fn<() => string>() +})) + +vi.mock('os', async () => { + const actual = (await vi.importActual('os')) as Record + return { + ...actual, + homedir: homedirMock + } +}) + +import { removeRetiredGeminiManagedHooksLocal } from './retired-gemini-hook-cleanup' + +describe('removeRetiredGeminiManagedHooksLocal', () => { + let homeDir: string + + beforeEach(() => { + homeDir = mkdtempSync(join(tmpdir(), 'orca-gemini-hook-cleanup-')) + homedirMock.mockReturnValue(homeDir) + }) + + afterEach(() => { + vi.clearAllMocks() + rmSync(homeDir, { recursive: true, force: true }) + }) + + it('removes managed gemini-hook commands and deletes the managed script', () => { + const settingsDir = join(homeDir, '.gemini') + const hooksDir = join(homeDir, '.orca', 'agent-hooks') + mkdirSync(settingsDir, { recursive: true }) + mkdirSync(hooksDir, { recursive: true }) + const scriptPath = join(hooksDir, 'gemini-hook.sh') + writeFileSync(scriptPath, '#!/bin/sh\nprintf "{}\\n"\n', 'utf8') + writeFileSync( + join(settingsDir, 'settings.json'), + JSON.stringify({ + hooks: { + BeforeAgent: [ + { + hooks: [ + { + type: 'command', + command: `/bin/sh '${scriptPath}'` + } + ] + }, + { + hooks: [{ type: 'command', command: '/usr/local/bin/user-hook' }] + } + ], + AfterTool: [ + { + hooks: [ + { + type: 'command', + command: `if [ -f '${scriptPath}' ]; then /bin/sh '${scriptPath}'; fi` + } + ] + } + ] + }, + theme: 'dark' + }), + 'utf8' + ) + + removeRetiredGeminiManagedHooksLocal() + + const settings = JSON.parse(readFileSync(join(settingsDir, 'settings.json'), 'utf8')) as { + hooks: Record + theme: string + } + expect(settings.theme).toBe('dark') + expect(settings.hooks.AfterTool).toBeUndefined() + expect(settings.hooks.BeforeAgent).toEqual([ + { hooks: [{ type: 'command', command: '/usr/local/bin/user-hook' }] } + ]) + expect(existsSync(scriptPath)).toBe(false) + }) + + it('deletes the Windows script variants the command matcher also strips', () => { + const hooksDir = join(homeDir, '.orca', 'agent-hooks') + mkdirSync(hooksDir, { recursive: true }) + for (const fileName of ['gemini-hook.sh', 'gemini-hook.cmd', 'gemini-hook.ps1']) { + writeFileSync(join(hooksDir, fileName), 'noop', 'utf8') + } + writeFileSync(join(hooksDir, 'claude-hook.ps1'), 'keep', 'utf8') + + removeRetiredGeminiManagedHooksLocal() + + expect(existsSync(join(hooksDir, 'gemini-hook.sh'))).toBe(false) + expect(existsSync(join(hooksDir, 'gemini-hook.cmd'))).toBe(false) + expect(existsSync(join(hooksDir, 'gemini-hook.ps1'))).toBe(false) + expect(existsSync(join(hooksDir, 'claude-hook.ps1'))).toBe(true) + }) + + it('is a no-op when no managed gemini hooks are present', () => { + const settingsDir = join(homeDir, '.gemini') + mkdirSync(settingsDir, { recursive: true }) + writeFileSync( + join(settingsDir, 'settings.json'), + JSON.stringify({ + hooks: { + BeforeAgent: [{ hooks: [{ type: 'command', command: '/usr/local/bin/user-hook' }] }] + } + }), + 'utf8' + ) + + removeRetiredGeminiManagedHooksLocal() + + const settings = JSON.parse(readFileSync(join(settingsDir, 'settings.json'), 'utf8')) as { + hooks: Record + } + expect(settings.hooks).toEqual({ + BeforeAgent: [{ hooks: [{ type: 'command', command: '/usr/local/bin/user-hook' }] }] + }) + }) + + it('does not touch Antigravity hooks under ~/.gemini/config', () => { + const antigravityDir = join(homeDir, '.gemini', 'config') + mkdirSync(antigravityDir, { recursive: true }) + const antigravityHooks = { + hooks: { + PreToolUse: [{ command: '/bin/sh antigravity-hook.sh' }] + } + } + writeFileSync(join(antigravityDir, 'hooks.json'), JSON.stringify(antigravityHooks), 'utf8') + + removeRetiredGeminiManagedHooksLocal() + + expect(JSON.parse(readFileSync(join(antigravityDir, 'hooks.json'), 'utf8'))).toEqual( + antigravityHooks + ) + }) +}) diff --git a/src/main/agent-hooks/retired-gemini-hook-cleanup.ts b/src/main/agent-hooks/retired-gemini-hook-cleanup.ts new file mode 100644 index 00000000000..42a00f60b5b --- /dev/null +++ b/src/main/agent-hooks/retired-gemini-hook-cleanup.ts @@ -0,0 +1,87 @@ +// Why: Gemini CLI was removed, but managed hook entries remain in +// ~/.gemini/settings.json and ~/.orca/agent-hooks/gemini-hook.* and still POST +// to /hook/gemini (404). Antigravity intentionally lives under ~/.gemini too — +// only strip commands that match the managed gemini-hook script name. + +import { unlinkSync } from 'node:fs' +import { homedir } from 'node:os' +import { join } from 'node:path' +import type { SFTPWrapper } from 'ssh2' +import { + createManagedCommandMatcher, + getSharedManagedScriptPath, + readHooksJson, + removeManagedCommands, + writeHooksJson, + type HooksConfig +} from './installer-utils' +import { readHooksJsonRemote, writeHooksJsonRemote } from './installer-utils-remote' + +// Why: installs emitted .sh (POSIX) or .cmd/.ps1 (Windows); sweep every variant. +const SCRIPT_FILES = ['gemini-hook.sh', 'gemini-hook.cmd', 'gemini-hook.ps1'] as const +const isManagedCommand = createManagedCommandMatcher('gemini-hook.sh') + +/** Returns the rewritten config, or null when the file holds no managed Gemini hooks. */ +function stripManagedGeminiHooks(config: HooksConfig | null): HooksConfig | null { + const hooks = config?.hooks + if (!hooks || typeof hooks !== 'object') { + return null + } + const nextHooks = { ...hooks } + let changed = false + for (const [eventName, definitions] of Object.entries(hooks)) { + if (!Array.isArray(definitions)) { + continue + } + const cleaned = removeManagedCommands(definitions, isManagedCommand) + if (cleaned.length === definitions.length) { + continue + } + changed = true + if (cleaned.length === 0) { + delete nextHooks[eventName] + } else { + nextHooks[eventName] = cleaned + } + } + return changed ? { ...config, hooks: nextHooks } : null +} + +/** Idempotent local upgrade cleanup for retired Gemini CLI managed hooks. */ +export function removeRetiredGeminiManagedHooksLocal(): void { + const configPath = join(homedir(), '.gemini', 'settings.json') + const cleaned = stripManagedGeminiHooks(readHooksJson(configPath)) + if (cleaned) { + writeHooksJson(configPath, cleaned) + } + for (const fileName of SCRIPT_FILES) { + try { + unlinkSync(getSharedManagedScriptPath(fileName)) + } catch { + // Missing file or permission error; settings cleanup already stops the POSTs. + } + } +} + +/** Idempotent remote (SSH) cleanup mirroring local remove for Gemini CLI hooks. */ +export async function removeRetiredGeminiManagedHooksRemote( + sftp: SFTPWrapper, + remoteHome: string +): Promise { + const home = remoteHome.replace(/\/$/, '') + const configPath = `${home}/.gemini/settings.json` + try { + const cleaned = stripManagedGeminiHooks(await readHooksJsonRemote(sftp, configPath)) + if (cleaned) { + await writeHooksJsonRemote(sftp, configPath, cleaned) + } + } catch { + // Best-effort: do not block remote hook install for other agents. + } + for (const fileName of SCRIPT_FILES) { + // Missing file or permission errors are non-fatal for upgrade cleanup. + await new Promise((resolve) => { + sftp.unlink(`${home}/.orca/agent-hooks/${fileName}`, () => resolve()) + }) + } +} diff --git a/src/main/agent-hooks/wsl-hook-fs-adapter.ts b/src/main/agent-hooks/wsl-hook-fs-adapter.ts index 9578fda1d10..c41a2d1d0d8 100644 --- a/src/main/agent-hooks/wsl-hook-fs-adapter.ts +++ b/src/main/agent-hooks/wsl-hook-fs-adapter.ts @@ -6,6 +6,7 @@ import type { SFTPWrapper } from 'ssh2' import type { installRemoteManagedAgentHooks } from './remote-managed-hook-installers' +import { removeRetiredGeminiManagedHooksRemote } from './retired-gemini-hook-cleanup' import { buildManagedHookDetectionCommands, detectedManagedHookAgents, @@ -27,6 +28,18 @@ export async function installWslGuestHooks(options: { warn: (message: string) => void }): Promise { const { mux, guestHome, distro, installHooks, settings, warn } = options + const sftp = createWslHookSftpAdapter(mux) + // Why: retired-agent cleanup must not depend on detecting a *supported* agent — + // a guest with only the retired CLI would keep posting to a dead hook endpoint. + try { + await removeRetiredGeminiManagedHooksRemote(sftp, guestHome) + } catch (error) { + warn( + `[agent-hooks] WSL retired Gemini hook cleanup for '${distro}' failed: ${ + error instanceof Error ? error.message : String(error) + }` + ) + } let agents try { const detected = (await mux.request('preflight.detectAgents', { @@ -44,7 +57,7 @@ export async function installWslGuestHooks(options: { if (agents.length === 0) { return } - const results = await installHooks(createWslHookSftpAdapter(mux), guestHome, { + const results = await installHooks(sftp, guestHome, { codexHomeDir: wslCodexRuntimeHomeForGuestHome(guestHome), agents }) 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 e6a3a727fb4..4f46fe98bab 100644 --- a/src/main/agent-hooks/wsl-hook-relay-manager.test.ts +++ b/src/main/agent-hooks/wsl-hook-relay-manager.test.ts @@ -2,7 +2,7 @@ // bridge (including a full run of the unchanged remote hook installers), and // the per-distro relay manager state machine with fault injection. import { EventEmitter } from 'node:events' -import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { join } from 'node:path' import type { ChildProcessWithoutNullStreams } from 'node:child_process' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -10,7 +10,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { RelayDispatcher } from '../../relay/dispatcher' import { registerWslHookFsHandlers } from '../../relay/wsl-hook-fs-bridge' import { SshChannelMultiplexer, type MultiplexerTransport } from '../ssh/ssh-channel-multiplexer' -import { createWslHookSftpAdapter } from './wsl-hook-fs-adapter' +import { createWslHookSftpAdapter, installWslGuestHooks } from './wsl-hook-fs-adapter' import { installRemoteManagedAgentHooks, REMOTE_MANAGED_HOOK_INSTALLER_AGENTS @@ -123,6 +123,41 @@ describe.skipIf(process.platform === 'win32')( expect(outside).toBeInstanceOf(Error) }) + it('strips retired Gemini hooks even when agent detection never succeeds', async () => { + // Detection has no handler in this harness, so it fails like a guest with + // none of the supported CLIs — cleanup must still have run by then. + mkdirSync(join(home, '.gemini'), { recursive: true }) + mkdirSync(join(home, '.orca', 'agent-hooks'), { recursive: true }) + const scriptPath = join(home, '.orca', 'agent-hooks', 'gemini-hook.sh') + writeFileSync(scriptPath, 'noop', 'utf8') + writeFileSync( + join(home, '.gemini', 'settings.json'), + JSON.stringify({ + hooks: { + BeforeAgent: [{ hooks: [{ type: 'command', command: `/bin/sh '${scriptPath}'` }] }] + }, + theme: 'dark' + }), + 'utf8' + ) + const installHooks = vi.fn() + + await installWslGuestHooks({ + mux: harness.mux, + guestHome: home, + distro: 'Ubuntu', + installHooks: installHooks as never, + settings: null, + warn: () => {} + }) + + const settings = JSON.parse(readFileSync(join(home, '.gemini', 'settings.json'), 'utf8')) + expect(settings.hooks).toEqual({}) + expect(settings.theme).toBe('dark') + expect(existsSync(scriptPath)).toBe(false) + expect(installHooks).not.toHaveBeenCalled() + }) + it('runs the unchanged remote managed hook installers against a WSL guest home', async () => { const adapter = createWslHookSftpAdapter(harness.mux) const results = await installRemoteManagedAgentHooks(adapter, home, { diff --git a/src/main/automations/headless-dispatch.ts b/src/main/automations/headless-dispatch.ts index 359673bc573..7915fe0d58c 100644 --- a/src/main/automations/headless-dispatch.ts +++ b/src/main/automations/headless-dispatch.ts @@ -1,8 +1,15 @@ import type { Automation, + AutomationDispatchResult, + AutomationPrecheckResult, AutomationRun, AutomationRunOutputSnapshot } from '../../shared/automations-types' +import { + didAutomationPrecheckPass, + formatAutomationPrecheckFailure +} from '../../shared/automation-precheck' +import type { Store } from '../persistence' import type { AutomationRunTargetResult } from './run-target-resolution' const MAX_HEADLESS_OUTPUT_SNAPSHOT_CHARS = 256 * 1024 @@ -71,3 +78,73 @@ export function createHeadlessAutomationOutputSnapshotBuffer(): { } } } + +/** Serve-mode run dispatch: precheck, launch through the dispatcher, then persist + * the launch target and its (possibly late) completion. */ +export async function requestHeadlessAutomationDispatch(args: { + store: Pick + dispatch: HeadlessAutomationDispatcher + automation: Automation + run: AutomationRun + target: Extract + runPrecheck: () => Promise + markDispatchResult: (result: AutomationDispatchResult) => Promise +}): Promise { + const { store, dispatch, automation, run, target, markDispatchResult } = args + const precheckResult = + run.trigger === 'scheduled' && automation.precheck ? await args.runPrecheck() : null + if (precheckResult && !didAutomationPrecheckPass(precheckResult)) { + return store.updateAutomationRun({ + runId: run.id, + status: 'skipped_precheck', + workspaceId: automation.workspaceId, + precheckResult, + error: formatAutomationPrecheckFailure(precheckResult) + }) + } + try { + const launch = await dispatch({ automation, run, target }) + const launchRunTarget = { + workspaceId: launch.workspaceId, + workspaceDisplayName: launch.workspaceDisplayName ?? null, + terminalSessionId: launch.terminalSessionId, + terminalPaneKey: launch.terminalPaneKey ?? null, + terminalPtyId: launch.terminalPtyId ?? null + } + const updated = store.updateAutomationRun({ + runId: run.id, + status: 'dispatched', + ...launchRunTarget, + error: null + }) + if (launch.completion) { + void launch.completion + .then((completion) => + markDispatchResult({ + runId: run.id, + status: completion.status, + ...launchRunTarget, + precheckResult, + outputSnapshot: completion.outputSnapshot ?? null, + error: completion.error ?? null + }) + ) + .catch((error) => + markDispatchResult({ + runId: run.id, + status: 'dispatch_failed', + ...launchRunTarget, + error: error instanceof Error ? error.message : String(error) + }) + ) + } + return updated + } catch (error) { + return store.updateAutomationRun({ + runId: run.id, + status: 'dispatch_failed', + workspaceId: automation.workspaceId, + error: error instanceof Error ? error.message : String(error) + }) + } +} diff --git a/src/main/automations/headless-workspace-create.test.ts b/src/main/automations/headless-workspace-create.test.ts index 3e25ad87440..e3ae487be26 100644 --- a/src/main/automations/headless-workspace-create.test.ts +++ b/src/main/automations/headless-workspace-create.test.ts @@ -55,6 +55,7 @@ describe('headless automation workspace create args', () => { it('stamps automation provenance for serve-mode new-per-run workspaces', () => { const args = buildHeadlessAutomationWorktreeCreateArgs({ automation, + agentId: 'codex', run: { id: 'run-1', title: 'Nightly review run', @@ -93,6 +94,7 @@ describe('headless automation workspace create args', () => { it('falls back to skip for legacy automations without a saved setup decision', () => { const args = buildHeadlessAutomationWorktreeCreateArgs({ automation: { ...automation, setupDecision: undefined }, + agentId: 'codex', run: { id: 'run-1', title: 'Nightly review run', diff --git a/src/main/automations/headless-workspace-create.ts b/src/main/automations/headless-workspace-create.ts index 2047a105c44..6eb5aafef48 100644 --- a/src/main/automations/headless-workspace-create.ts +++ b/src/main/automations/headless-workspace-create.ts @@ -1,6 +1,7 @@ import type { Automation, AutomationRun } from '../../shared/automations-types' import { buildAutomationWorkspaceProvenance } from '../../shared/automation-workspace-provenance' import type { Repo } from '../../shared/repo-types' +import type { TuiAgent } from '../../shared/tui-agent' import type { OrcaRuntimeService } from '../runtime/orca-runtime' type HeadlessAutomationRunForWorkspace = Pick @@ -23,11 +24,14 @@ export function buildHeadlessAutomationWorkspaceName( export function buildHeadlessAutomationWorktreeCreateArgs({ automation, + agentId, run, repo, createdAt = Date.now() }: { automation: Automation + /** Passed separately: `automation.agentId` is null once its agent is retired. */ + agentId: TuiAgent run: HeadlessAutomationRunForWorkspace repo: Repo createdAt?: number @@ -38,8 +42,8 @@ export function buildHeadlessAutomationWorktreeCreateArgs({ baseBranch: automation.baseBranch ?? undefined, setupDecision: automation.setupDecision ?? 'skip', activate: false, - createdWithAgent: automation.agentId, - startupAgent: automation.agentId, + createdWithAgent: agentId, + startupAgent: agentId, startupPrompt: automation.prompt, telemetrySource: 'unknown', automationProvenance: buildAutomationWorkspaceProvenance(automation, run, repo, createdAt) diff --git a/src/main/automations/service.test.ts b/src/main/automations/service.test.ts index 9509362b50c..bd22ea6c71e 100644 --- a/src/main/automations/service.test.ts +++ b/src/main/automations/service.test.ts @@ -5,6 +5,7 @@ import { tmpdir } from 'node:os' import type { Repo } from '../../shared/repo-types' import { toRuntimeExecutionHostId } from '../../shared/execution-host' import { AutomationService } from './service' +import { AUTOMATION_MISSING_AGENT_MESSAGE, type Automation } from '../../shared/automations-types' const testState = { dir: '' } @@ -26,6 +27,12 @@ async function createStore() { return new Store() } +/** Mirrors the load-time retired-agent cleanup, which nulls agentId in the profile. */ +function clearPersistedAgentId(store: Awaited>, id: string): void { + const state = (store as unknown as { state: { automations: Automation[] } }).state + state.automations.find((entry) => entry.id === id)!.agentId = null +} + const makeRepo = (overrides: Partial = {}): Repo => ({ id: 'r1', path: '/repo', @@ -121,6 +128,57 @@ describe('AutomationService', () => { ) }) + it('skips dispatch for an automation whose retired agent was cleared', async () => { + vi.setSystemTime(new Date('2026-05-13T08:00:00Z')) + const store = await createStore() + store.addRepo(makeRepo()) + const automation = store.createAutomation({ + name: 'Retired agent check', + prompt: 'Check the repo', + agentId: 'claude', + projectId: 'r1', + workspaceMode: 'existing', + workspaceId: 'wt1', + timezone: 'UTC', + rrule: 'FREQ=DAILY;BYHOUR=9;BYMINUTE=0', + dtstart: new Date('2026-05-14T00:00:00Z').getTime() + }) + clearPersistedAgentId(store, automation.id) + const send = vi.fn() + const service = new AutomationService(store, { tickMs: 60_000 }) + service.setWebContents({ isDestroyed: () => false, send } as never) + service.setRendererReady() + + const run = await service.runNow(automation.id) + + expect(run.status).toBe('skipped_unavailable') + expect(run.error).toBe(AUTOMATION_MISSING_AGENT_MESSAGE) + expect(send).not.toHaveBeenCalled() + }) + + it('keeps an agentless automation paused when a client tries to re-enable it', async () => { + vi.setSystemTime(new Date('2026-05-13T08:00:00Z')) + const store = await createStore() + store.addRepo(makeRepo()) + const automation = store.createAutomation({ + name: 'Retired agent check', + prompt: 'Check the repo', + agentId: 'claude', + projectId: 'r1', + workspaceMode: 'existing', + workspaceId: 'wt1', + timezone: 'UTC', + rrule: 'FREQ=DAILY;BYHOUR=9;BYMINUTE=0', + dtstart: new Date('2026-05-14T00:00:00Z').getTime() + }) + clearPersistedAgentId(store, automation.id) + + expect(store.updateAutomation(automation.id, { enabled: true }).enabled).toBe(false) + expect( + store.updateAutomation(automation.id, { agentId: 'codex', enabled: true }) + ).toMatchObject({ agentId: 'codex', enabled: true }) + }) + it('skips dispatch when the selected project host setup is gone', async () => { vi.setSystemTime(new Date('2026-05-13T08:00:00Z')) const store = await createStore() diff --git a/src/main/automations/service.ts b/src/main/automations/service.ts index 91b70ff6088..b9dbcd33df9 100644 --- a/src/main/automations/service.ts +++ b/src/main/automations/service.ts @@ -1,6 +1,7 @@ import type { WebContents } from 'electron' import type { Store } from '../persistence' import { + AUTOMATION_MISSING_AGENT_MESSAGE, isFinalAutomationRunStatus, type Automation, type AutomationDispatchRequest, @@ -13,12 +14,12 @@ import type { CodexUsageStore } from '../codex-usage/store' import { runAutomationPrecheck } from './precheck-runner' import { resolveAutomationRunTarget, type AutomationRunTargetResult } from './run-target-resolution' import { collectAutomationRunUsage } from './run-usage-collection' -import type { HeadlessAutomationDispatcher } from './headless-dispatch' -import { clearAutomationDispatchTokens, createAutomationDispatchToken } from './dispatch-tokens' import { - didAutomationPrecheckPass, - formatAutomationPrecheckFailure -} from '../../shared/automation-precheck' + requestHeadlessAutomationDispatch, + type HeadlessAutomationDispatcher +} from './headless-dispatch' +import { clearAutomationDispatchTokens, createAutomationDispatchToken } from './dispatch-tokens' +import { isTuiAgent } from '../../shared/tui-agent-config' const DEFAULT_TICK_MS = 60 * 1000 @@ -211,6 +212,16 @@ export class AutomationService { automation: Automation, run: AutomationRun ): Promise { + // Why: a retired agent leaves agentId null (see retired-agent-settings-cleanup). + // Fail the run here with an actionable reason instead of at agent launch. + if (!isTuiAgent(automation.agentId)) { + return this.store.updateAutomationRun({ + runId: run.id, + status: 'skipped_unavailable', + workspaceId: automation.workspaceId, + error: AUTOMATION_MISSING_AGENT_MESSAGE + }) + } const target = resolveAutomationRunTarget(this.store, automation, { allowRemoteHostScheduling: this.allowRemoteHostScheduling }) @@ -254,63 +265,14 @@ export class AutomationService { run: AutomationRun, target: Extract ): Promise { - const precheckResult = - run.trigger === 'scheduled' && automation.precheck - ? await this.runPrecheck(automation.id, run.id) - : null - if (precheckResult && !didAutomationPrecheckPass(precheckResult)) { - return this.store.updateAutomationRun({ - runId: run.id, - status: 'skipped_precheck', - workspaceId: automation.workspaceId, - precheckResult, - error: formatAutomationPrecheckFailure(precheckResult) - }) - } - try { - const launch = await this.headlessDispatcher!({ automation, run, target }) - const launchRunTarget = { - workspaceId: launch.workspaceId, - workspaceDisplayName: launch.workspaceDisplayName ?? null, - terminalSessionId: launch.terminalSessionId, - terminalPaneKey: launch.terminalPaneKey ?? null, - terminalPtyId: launch.terminalPtyId ?? null - } - const updated = this.store.updateAutomationRun({ - runId: run.id, - status: 'dispatched', - ...launchRunTarget, - error: null - }) - if (launch.completion) { - void launch.completion - .then((completion) => - this.markDispatchResult({ - runId: run.id, - status: completion.status, - ...launchRunTarget, - precheckResult, - outputSnapshot: completion.outputSnapshot ?? null, - error: completion.error ?? null - }) - ) - .catch((error) => - this.markDispatchResult({ - runId: run.id, - status: 'dispatch_failed', - ...launchRunTarget, - error: error instanceof Error ? error.message : String(error) - }) - ) - } - return updated - } catch (error) { - return this.store.updateAutomationRun({ - runId: run.id, - status: 'dispatch_failed', - workspaceId: automation.workspaceId, - error: error instanceof Error ? error.message : String(error) - }) - } + return await requestHeadlessAutomationDispatch({ + store: this.store, + dispatch: this.headlessDispatcher!, + automation, + run, + target, + runPrecheck: () => this.runPrecheck(automation.id, run.id), + markDispatchResult: (result) => this.markDispatchResult(result) + }) } } diff --git a/src/main/index.ts b/src/main/index.ts index 7d23dedd1d7..a89002eed2a 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -279,6 +279,8 @@ import { setUnreadDockBadgeCount } from './dock/unread-badge' import { AutomationService } from './automations/service' import { createHeadlessAutomationOutputSnapshotBuffer } from './automations/headless-dispatch' import { buildHeadlessAutomationWorktreeCreateArgs } from './automations/headless-workspace-create' +import { AUTOMATION_MISSING_AGENT_MESSAGE } from '../shared/automations-types' +import { isTuiAgent } from '../shared/tui-agent-config' import { AgentAwakeService } from './agent-awake-service' import { normalizeComputerAwakeMode } from '../shared/computer-awake-mode' import { registerSystemResumeBroadcast } from './system-resume-broadcast' @@ -2567,6 +2569,12 @@ void app.whenReady().then(async () => { allowRemoteHostScheduling: isServeMode, headlessDispatcher: isServeMode ? async ({ automation, run, target }) => { + // Why: the service already skips agentless runs; this keeps the launch + // paths below narrowed to a real agent instead of guarding each one. + const agentId = automation.agentId + if (!isTuiAgent(agentId)) { + throw new Error(AUTOMATION_MISSING_AGENT_MESSAGE) + } const terminalSnapshotLimit = 2_000 let terminalHandle: string let terminalSessionId: string | null = null @@ -2579,6 +2587,7 @@ void app.whenReady().then(async () => { const created = await runtimeService.createManagedWorktree({ ...buildHeadlessAutomationWorktreeCreateArgs({ automation, + agentId, run, repo: target.repo }) @@ -2602,7 +2611,7 @@ void app.whenReady().then(async () => { const terminal = await runtimeService.launchAgentTerminal( `id:${automation.workspaceId}`, { - agent: automation.agentId, + agent: agentId, prompt: automation.prompt, title: run.title } diff --git a/src/main/ipc/agent-hooks.test.ts b/src/main/ipc/agent-hooks.test.ts index 48440713cd0..c6cc8e0043e 100644 --- a/src/main/ipc/agent-hooks.test.ts +++ b/src/main/ipc/agent-hooks.test.ts @@ -69,9 +69,6 @@ vi.mock('../openclaude/hook-service', () => ({ vi.mock('../codex/hook-service', () => ({ codexHookService: { getStatus: vi.fn(() => ({ agent: 'codex', state: 'absent' })) } })) -vi.mock('../gemini/hook-service', () => ({ - geminiHookService: { getStatus: vi.fn(() => ({ agent: 'gemini', state: 'absent' })) } -})) vi.mock('../antigravity/hook-service', () => ({ antigravityHookService: { getStatus: vi.fn(() => ({ agent: 'antigravity', state: 'absent' })) } })) diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 499c3a12a6c..0a863d7f015 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -5374,9 +5374,14 @@ export class Store { const dtstart = updates.dtstart ?? current.dtstart const scheduleChanged = updates.rrule !== undefined || updates.dtstart !== undefined const workspaceMode = updates.workspaceMode ?? current.workspaceMode + const agentId = updates.agentId ?? current.agentId const updated: Automation = { ...current, ...updates, + agentId, + // Why: an automation whose agent was retired has no agent to dispatch to, + // so it stays paused until the user picks a new one. + enabled: agentId !== null && (updates.enabled ?? current.enabled), name: updates.name !== undefined ? updates.name.trim() || 'Untitled automation' : current.name, precheck: Object.hasOwn(updates, 'precheck') diff --git a/src/main/retired-agent-settings-cleanup.test.ts b/src/main/retired-agent-settings-cleanup.test.ts index ca510479b9f..1ba7155b7fb 100644 --- a/src/main/retired-agent-settings-cleanup.test.ts +++ b/src/main/retired-agent-settings-cleanup.test.ts @@ -27,12 +27,11 @@ describe('cleanRetiredAgentReferences', () => { const state = profile({ settings: { defaultTuiAgent: 'gemini' } }) expect(cleanRetiredAgentReferences(state)).toBe(true) expect(state.settings.defaultTuiAgent).toBeNull() - }) - it('preserves blank as an explicit shell-only preference', () => { - const state = profile({ settings: { defaultTuiAgent: 'blank' } }) - expect(cleanRetiredAgentReferences(state)).toBe(false) - expect(state.settings.defaultTuiAgent).toBe('blank') + // 'blank' is an explicit shell-only preference, not a retired agent. + const blank = profile({ settings: { defaultTuiAgent: 'blank' } }) + expect(cleanRetiredAgentReferences(blank)).toBe(false) + expect(blank.settings.defaultTuiAgent).toBe('blank') }) it('drops retired keys from agent-keyed launch settings', () => { @@ -84,12 +83,19 @@ describe('cleanRetiredAgentReferences', () => { }) }) - it('clears the legacy commitMessageAi agent', () => { + it('clears the legacy commitMessageAi agent without switching the feature off', () => { const state = profile({ - settings: { commitMessageAi: { agentId: 'gemini', selectedModelByAgent: { gemini: 'x' } } } + settings: { + commitMessageAi: { enabled: true, agentId: 'gemini', selectedModelByAgent: { gemini: 'x' } } + } }) expect(cleanRetiredAgentReferences(state)).toBe(true) - expect(state.settings.commitMessageAi).toEqual({ agentId: null, selectedModelByAgent: {} }) + // A cleared agent falls back to the default here, unlike automations. + expect(state.settings.commitMessageAi).toEqual({ + enabled: true, + agentId: null, + selectedModelByAgent: {} + }) }) it('drops the removed Gemini CLI OAuth toggle', () => { @@ -118,6 +124,44 @@ describe('cleanRetiredAgentReferences', () => { ).toBeNull() }) + it('disables and clears automations that still target a retired agent', () => { + const state = profile({ + automations: [ + { id: 'a1', name: 'Gemini nightly', agentId: 'gemini', enabled: true }, + { id: 'a2', name: 'Claude hourly', agentId: 'claude', enabled: true } + ] + }) + expect(cleanRetiredAgentReferences(state)).toBe(true) + expect(state.automations).toEqual([ + { id: 'a1', name: 'Gemini nightly', agentId: null, enabled: false }, + { id: 'a2', name: 'Claude hourly', agentId: 'claude', enabled: true } + ]) + }) + + it('is sticky for already-disabled retired automations that still hold the agent id', () => { + const state = profile({ + automations: [{ id: 'a1', agentId: 'gemini', enabled: false }] + }) + expect(cleanRetiredAgentReferences(state)).toBe(true) + expect(state.automations).toEqual([{ id: 'a1', agentId: null, enabled: false }]) + expect(cleanRetiredAgentReferences(state)).toBe(false) + }) + + it('drops a retired createdWithAgent so reopening cannot seed a missing agent', () => { + const state = profile({ + worktreeMeta: { + wt1: { createdWithAgent: 'gemini', isPinned: true }, + wt2: { createdWithAgent: 'codex' } + }, + folderWorkspaces: [{ id: 'fw1', createdWithAgent: 'gemini' }] + }) + expect(cleanRetiredAgentReferences(state)).toBe(true) + expect(state.worktreeMeta.wt1).toEqual({ isPinned: true }) + expect(state.worktreeMeta.wt2.createdWithAgent).toBe('codex') + expect(state.folderWorkspaces[0]).toEqual({ id: 'fw1' }) + expect(cleanRetiredAgentReferences(state)).toBe(false) + }) + it('tolerates a profile missing every optional collection', () => { expect(cleanRetiredAgentReferences({} as PersistedState)).toBe(false) }) diff --git a/src/main/retired-agent-settings-cleanup.ts b/src/main/retired-agent-settings-cleanup.ts index 30205f06731..e76682f1dbb 100644 --- a/src/main/retired-agent-settings-cleanup.ts +++ b/src/main/retired-agent-settings-cleanup.ts @@ -1,158 +1,87 @@ // Why: a removed agent id outlives its code inside saved profiles. Most readers // guard with isTuiAgent(), but `defaultTuiAgent` preselects the launch target -// verbatim and Source Control AI copies its `agentId` into every action recipe -// without validating it — both would point at an agent that no longer exists. -// Scrub the profile once at the load boundary so no downstream reader has to. +// verbatim, Source Control AI copies its `agentId` into every action recipe, +// and automations dispatch with a raw agentId — all would point at an agent +// that no longer exists. Scrub the profile once at the load boundary so no +// downstream reader has to. import type { PersistedState } from '../shared/persisted-state-types' /** Agent ids Orca no longer ships. Keep an entry until profiles predating its removal are gone. */ -const RETIRED_TUI_AGENT_IDS: readonly string[] = ['gemini'] +const RETIRED_AGENTS: readonly unknown[] = ['gemini'] /** Status-bar providers Orca no longer publishes usage for. */ -const RETIRED_STATUS_BAR_ITEM_IDS: readonly string[] = ['gemini', 'antigravity'] +const RETIRED_STATUS_BAR_ITEMS: readonly unknown[] = ['gemini', 'antigravity'] /** GlobalSettings keys owned solely by a retired agent. */ -const RETIRED_SETTINGS_KEYS: readonly string[] = ['geminiCliOAuthEnabled'] +const RETIRED_KEYS: readonly string[] = ['geminiCliOAuthEnabled'] -type MutableRecord = Record +/** Single-agent fields. null reads as "fall back to the default" everywhere. */ +const AGENT_ID_KEYS: readonly string[] = ['agentId', 'defaultTuiAgent'] -function asRecord(value: unknown): MutableRecord | null { - return value && typeof value === 'object' && !Array.isArray(value) - ? (value as MutableRecord) - : null +// Why: named explicitly rather than detected, so a repo, host, or worktree +// literally called "gemini" is never mistaken for an agent-keyed map. +const AGENT_KEYED_MAPS: readonly string[] = [ + 'agentCmdOverrides', + 'agentDefaultArgs', + 'agentDefaultEnv', + 'selectedModelByAgent', + 'discoveredModelsByAgent' +] +const AGENT_KEYED_MAPS_BY_HOST: readonly string[] = [ + 'selectedModelByAgentByHost', + 'discoveredModelsByAgentByHost' +] + +type Rec = Record + +function asRecord(value: unknown): Rec | null { + return value && typeof value === 'object' && !Array.isArray(value) ? (value as Rec) : null } -function isRetiredAgent(value: unknown): boolean { - return typeof value === 'string' && RETIRED_TUI_AGENT_IDS.includes(value) -} - -/** Drops retired agent ids used as keys of an agent-keyed record. */ -function dropRetiredAgentKeys(value: unknown): boolean { +function dropRetiredKeys(value: unknown): void { const record = asRecord(value) - if (!record) { - return false + for (const key of Object.keys(record ?? {})) { + if (RETIRED_AGENTS.includes(key)) { + delete record![key] + } } - let changed = false - for (const key of Object.keys(record)) { - if (isRetiredAgent(key)) { +} + +/** + * One depth-first pass over the profile. Every shape that stores an agent id + * does it under one of the key names above, at some nesting depth that differs + * per shape (settings, per-repo overrides, action recipes, automations), so + * matching on the key rather than the path covers them all. + */ +function scrub(node: unknown): void { + if (Array.isArray(node)) { + node.forEach(scrub) + return + } + const record = asRecord(node) + if (!record) { + return + } + for (const [key, value] of Object.entries(record)) { + if (RETIRED_KEYS.includes(key)) { delete record[key] - changed = true + } else if (AGENT_ID_KEYS.includes(key) && RETIRED_AGENTS.includes(value)) { + record[key] = null + } else if (key === 'createdWithAgent' && RETIRED_AGENTS.includes(value)) { + delete record[key] + } else if (key === 'disabledTuiAgents' && Array.isArray(value)) { + record[key] = value.filter((agent) => !RETIRED_AGENTS.includes(agent)) + } else if (key === 'statusBarItems' && Array.isArray(value)) { + record[key] = value.filter((item) => !RETIRED_STATUS_BAR_ITEMS.includes(item)) + } else if (AGENT_KEYED_MAPS.includes(key)) { + dropRetiredKeys(value) + } else if (AGENT_KEYED_MAPS_BY_HOST.includes(key)) { + Object.values(asRecord(value) ?? {}).forEach(dropRetiredKeys) + } else { + scrub(value) } } - return changed -} - -/** Same, one level deeper: host key -> agent-keyed record. */ -function dropRetiredAgentKeysByHost(value: unknown): boolean { - const record = asRecord(value) - if (!record) { - return false - } - let changed = false - for (const perHost of Object.values(record)) { - changed = dropRetiredAgentKeys(perHost) || changed - } - return changed -} - -/** Clears a retired `agentId` to null, which every reader treats as "fall back to the default". */ -function clearRetiredAgentId(value: unknown): boolean { - const record = asRecord(value) - if (!record || !isRetiredAgent(record.agentId)) { - return false - } - record.agentId = null - return true -} - -function cleanModelSelections(value: unknown): boolean { - const record = asRecord(value) - if (!record) { - return false - } - let changed = dropRetiredAgentKeys(record.selectedModelByAgent) - changed = dropRetiredAgentKeysByHost(record.selectedModelByAgentByHost) || changed - changed = dropRetiredAgentKeys(record.discoveredModelsByAgent) || changed - changed = dropRetiredAgentKeysByHost(record.discoveredModelsByAgentByHost) || changed - return changed -} - -function cleanActionRecipes(value: unknown): boolean { - const record = asRecord(value) - if (!record) { - return false - } - let changed = false - for (const recipe of Object.values(record)) { - changed = clearRetiredAgentId(recipe) || changed - } - return changed -} - -/** Covers both GlobalSettings.sourceControlAi and the per-repo override shape. */ -function cleanSourceControlAi(value: unknown): boolean { - const record = asRecord(value) - if (!record) { - return false - } - let changed = clearRetiredAgentId(record) - changed = cleanModelSelections(record) || changed - changed = cleanActionRecipes(record.actions) || changed - changed = cleanActionRecipes(record.launchActionDefaults) || changed - changed = cleanActionRecipes(record.actionOverrides) || changed - const modelOverrides = asRecord(record.modelOverridesByOperation) - for (const override of Object.values(modelOverrides ?? {})) { - changed = cleanModelSelections(override) || changed - } - return changed -} - -function cleanSettings(value: unknown): boolean { - const settings = asRecord(value) - if (!settings) { - return false - } - let changed = false - for (const key of RETIRED_SETTINGS_KEYS) { - if (key in settings) { - delete settings[key] - changed = true - } - } - if (isRetiredAgent(settings.defaultTuiAgent)) { - settings.defaultTuiAgent = null - changed = true - } - if (Array.isArray(settings.disabledTuiAgents)) { - const kept = settings.disabledTuiAgents.filter((agent) => !isRetiredAgent(agent)) - if (kept.length !== settings.disabledTuiAgents.length) { - settings.disabledTuiAgents = kept - changed = true - } - } - changed = dropRetiredAgentKeys(settings.agentCmdOverrides) || changed - changed = dropRetiredAgentKeys(settings.agentDefaultArgs) || changed - changed = dropRetiredAgentKeys(settings.agentDefaultEnv) || changed - changed = clearRetiredAgentId(settings.commitMessageAi) || changed - changed = cleanModelSelections(settings.commitMessageAi) || changed - changed = cleanSourceControlAi(settings.sourceControlAi) || changed - return changed -} - -function cleanUi(value: unknown): boolean { - const ui = asRecord(value) - if (!ui || !Array.isArray(ui.statusBarItems)) { - return false - } - const kept = ui.statusBarItems.filter( - (item) => typeof item !== 'string' || !RETIRED_STATUS_BAR_ITEM_IDS.includes(item) - ) - if (kept.length === ui.statusBarItems.length) { - return false - } - ui.statusBarItems = kept - return true } /** @@ -160,13 +89,16 @@ function cleanUi(value: unknown): boolean { * can flag the profile for a re-save. */ export function cleanRetiredAgentReferences(state: PersistedState): boolean { - let changed = cleanSettings(state.settings) - changed = cleanUi(state.ui) || changed - for (const repo of state.repos ?? []) { - changed = cleanSourceControlAi(repo?.sourceControlAi) || changed + const before = JSON.stringify(state) + // Why: automations dispatch without the isTuiAgent() guard most readers apply, + // so clearing the id alone would let them run on whatever agent is default. + // Scoped to automations on purpose — commitMessageAi also has an `enabled` + // flag, but there a cleared agent must not switch the whole feature off. + for (const automation of state.automations ?? []) { + if (automation && RETIRED_AGENTS.includes(automation.agentId)) { + automation.enabled = false + } } - for (const setup of state.projectHostSetups ?? []) { - changed = cleanSourceControlAi(setup?.sourceControlAi) || changed - } - return changed + scrub(state) + return JSON.stringify(state) !== before } diff --git a/src/main/runtime/rpc/methods/orchestration-worker-launch-preferences.test.ts b/src/main/runtime/rpc/methods/orchestration-worker-launch-preferences.test.ts index fc33c89cdda..371462d039f 100644 --- a/src/main/runtime/rpc/methods/orchestration-worker-launch-preferences.test.ts +++ b/src/main/runtime/rpc/methods/orchestration-worker-launch-preferences.test.ts @@ -116,12 +116,6 @@ describe('orchestration worker launch preferences', () => { ).toThrow('does not support launch-time model selection') }) - it('does not expose deprecated Gemini model selection to worker-start', () => { - expect(() => - resolveWorkerLaunchPreferences({ agent: 'gemini', model: 'gemini-3-pro-preview' }) - ).toThrow('does not support launch-time model selection') - }) - it('rejects preferences when reusing an existing terminal', () => { expect(() => assertWorkerLaunchPreferencesCreateTerminal({ diff --git a/src/main/runtime/rpc/methods/worktree-schemas.test.ts b/src/main/runtime/rpc/methods/worktree-schemas.test.ts index 5737e2c4c8d..22bb1b6980b 100644 --- a/src/main/runtime/rpc/methods/worktree-schemas.test.ts +++ b/src/main/runtime/rpc/methods/worktree-schemas.test.ts @@ -11,15 +11,31 @@ describe('worktree RPC schemas', () => { ).toBe(false) }) - it('rejects invalid startup agent values', () => { + it('strips a retired startup agent together with its prompt', () => { + // A mixed-version client can still send startupAgent:'gemini' plus a prompt. + // Stripping the agent alone left the prompt orphaned and failed the create. const parsed = WorktreeCreate.safeParse({ repo: 'repo-1', name: 'agent-startup', - startupAgent: 'wat', + startupAgent: 'gemini', startupPrompt: 'hi' }) - expect(parsed.success).toBe(false) + expect(parsed.success).toBe(true) + expect(parsed.data?.startupAgent).toBeUndefined() + expect(parsed.data?.startupPrompt).toBeUndefined() + }) + + it('keeps a supported startup agent and its prompt', () => { + const parsed = WorktreeCreate.parse({ + repo: 'repo-1', + name: 'agent-startup', + startupAgent: 'claude', + startupPrompt: 'hi' + }) + + expect(parsed.startupAgent).toBe('claude') + expect(parsed.startupPrompt).toBe('hi') }) it('rejects startup prompts without startup agents', () => { diff --git a/src/main/runtime/rpc/methods/worktree-schemas.ts b/src/main/runtime/rpc/methods/worktree-schemas.ts index d401ddd414d..529ae2826bc 100644 --- a/src/main/runtime/rpc/methods/worktree-schemas.ts +++ b/src/main/runtime/rpc/methods/worktree-schemas.ts @@ -15,16 +15,31 @@ import { TaskSourceContextSchema } from '../../../../shared/task-source-context- import { WorkspaceLinkedItemSchema } from '../../../../shared/workspace-linked-item-schema' import { isWorkspaceLinkedItemSourceContextMatch } from '../../../../shared/workspace-linked-item-source-context' +// Why: mixed client/host versions can still send retired ids (e.g. gemini). +// Strip to undefined like createdWithAgent so worktree-create is not rejected. const OptionalTuiAgent = z .unknown() - .superRefine((value, ctx) => { - if (value !== undefined && !isTuiAgent(value)) { - ctx.addIssue({ code: z.ZodIssueCode.custom, message: 'Unknown TUI agent' }) - } - }) .transform((value): TuiAgent | undefined => (isTuiAgent(value) ? value : undefined)) .optional() +/** + * Why: `startupPrompt` only travels with a `startupAgent`, so a stripped retired + * agent must take its prompt with it. Otherwise the leftover prompt trips the + * "startupPrompt requires startupAgent" refinement below and the create fails + * instead of degrading to a plain shell. + */ +function dropRetiredStartupAgentPair(value: unknown): unknown { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return value + } + const params = value as Record + if (params.startupAgent === undefined || isTuiAgent(params.startupAgent)) { + return value + } + const { startupAgent: _startupAgent, startupPrompt: _startupPrompt, ...rest } = params + return rest +} + const AutomationWorkspaceProvenanceRequest = z.object({ automationId: z.string(), automationRunId: z.string(), @@ -97,7 +112,7 @@ function assertLinkedWorkItemSourceContextMatch( } } -export const WorktreeCreate = z +const WorktreeCreateParams = z .object({ repo: z .unknown() @@ -212,6 +227,8 @@ export const WorktreeCreate = z } }) +export const WorktreeCreate = z.preprocess(dropRetiredStartupAgentPair, WorktreeCreateParams) + export const WorktreePrefetchCreateBase = z.object({ repo: z .unknown() diff --git a/src/main/ssh/ssh-relay-session-managed-hooks.test.ts b/src/main/ssh/ssh-relay-session-managed-hooks.test.ts index 93723965f92..e93e62cef5f 100644 --- a/src/main/ssh/ssh-relay-session-managed-hooks.test.ts +++ b/src/main/ssh/ssh-relay-session-managed-hooks.test.ts @@ -128,4 +128,30 @@ describe('SshRelaySession managed hooks', () => { muxRequestMock.mock.invocationCallOrder[managedIndex] ) }) + + it('still requests managed hooks with no detected agents so retired hooks get cleaned', async () => { + muxRequestMock.mockImplementation(async (method: string) => { + if (method === 'preflight.detectAgents') { + return { agents: [] } + } + return method === AGENT_HOOK_INSTALL_MANAGED_HOOKS_METHOD + ? { installers: 0, errors: 0 } + : { ok: true } + }) + const { mockStore, mockPortForward, getMainWindow } = createMockDeps() + const connection = { + sftp: vi.fn(), + 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: [] + }) + ) + }) }) diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index 820eb7e1360..acd1da6505e 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -1292,7 +1292,10 @@ export class SshRelaySession { commands: buildManagedHookDetectionCommands(store.getSettings?.() ?? null, 'linux') })) as { agents?: unknown } const agents = detectedManagedHookAgents(detected?.agents) - if (agents.length === 0 || (shouldContinue && !shouldContinue())) { + // Why: an empty agent list still goes to the host — the request also runs + // retired-agent hook cleanup, which a host with none of the supported CLIs + // installed would otherwise never get. + if (shouldContinue && !shouldContinue()) { return } const hostKeyFingerprint = this.requireReadyConnection().getHostKeyFingerprint?.() diff --git a/src/renderer/src/components/automations/AutomationDetail.tsx b/src/renderer/src/components/automations/AutomationDetail.tsx index b0ffad51b3f..daeea0fee25 100644 --- a/src/renderer/src/components/automations/AutomationDetail.tsx +++ b/src/renderer/src/components/automations/AutomationDetail.tsx @@ -3,7 +3,7 @@ import { Pencil, Pause, Play, Trash2 } from 'lucide-react' import { Button } from '@/components/ui/button' import { Badge } from '@/components/ui/badge' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' -import { getAgentCatalog, AgentIcon } from '@/lib/agent-catalog' +import { AgentIcon } from '@/lib/agent-catalog' import type { Automation, AutomationRun } from '../../../../shared/automations-types' import { formatAutomationSchedule } from '../../../../shared/automation-schedules' import { formatAutomationPrecheckTimeout } from '../../../../shared/automation-precheck' @@ -15,6 +15,7 @@ import { } from './automation-usage-model' import type { AutomationTargetAvailability } from './automation-target-availability' import { getAutomationSourceDisplay } from './automation-source-display' +import { getAgentLabel } from './automation-draft-model' import { translate } from '@/i18n/i18n' type AutomationDetailProps = { @@ -125,8 +126,7 @@ export function AutomationDetail({ : usageSummary.unavailableRuns > 0 ? 'Unavailable' : 'No runs' - const agentLabel = - getAgentCatalog().find((agent) => agent.id === automation.agentId)?.label ?? automation.agentId + const agentLabel = getAgentLabel(automation.agentId) const runLocationLabel = automation.workspaceMode === 'new_per_run' ? (automation.baseBranch ?? projectDefaultBaseRef ?? 'Project default') diff --git a/src/renderer/src/components/automations/AutomationsPage.tsx b/src/renderer/src/components/automations/AutomationsPage.tsx index 4445cf21eeb..6e7cecb5270 100644 --- a/src/renderer/src/components/automations/AutomationsPage.tsx +++ b/src/renderer/src/components/automations/AutomationsPage.tsx @@ -19,6 +19,7 @@ import type { AutomationRun, AutomationUpdateInput } from '../../../../shared/automations-types' +import { AUTOMATION_MISSING_AGENT_MESSAGE } from '../../../../shared/automations-types' import { getAutomationRunRepoId } from '../../../../shared/automation-run-identity' import { getLocalExecutionHostLabel, @@ -1123,7 +1124,8 @@ export default function AutomationsPage(): React.JSX.Element { const nextDraft: AutomationDraft = { name: latest.name, prompt: latest.prompt, - agentId: latest.agentId, + // Preselect the default agent when a retired-agent cleanup cleared this one. + agentId: latest.agentId ?? defaultAgent, projectId: getAutomationRunRepoId(latest), workspaceMode: latest.workspaceMode, workspaceId: latest.workspaceId ?? '', @@ -1514,6 +1516,11 @@ export default function AutomationsPage(): React.JSX.Element { } const toggleAutomation = async (automation: Automation): Promise => { + // Why: resuming without an agent would only schedule runs that fail at dispatch. + if (!automation.enabled && !automation.agentId) { + toast.error(AUTOMATION_MISSING_AGENT_MESSAGE) + return + } await updateAutomationForTarget( automation, { enabled: !automation.enabled }, diff --git a/src/renderer/src/components/automations/automation-draft-model.ts b/src/renderer/src/components/automations/automation-draft-model.ts index 827f84543a4..cade9f612b0 100644 --- a/src/renderer/src/components/automations/automation-draft-model.ts +++ b/src/renderer/src/components/automations/automation-draft-model.ts @@ -1,4 +1,5 @@ import { getAgentCatalog } from '@/lib/agent-catalog' +import { translate } from '@/i18n/i18n' import type { AutomationPrecheck } from '../../../../shared/automations-types' import { buildAutomationCronSchedule } from '../../../../shared/automation-schedules' import type { Worktree } from '../../../../shared/worktree/types' @@ -47,6 +48,10 @@ export function buildHermesCronSchedule(draft: AutomationDraft): string { }) } -export function getAgentLabel(agentId: string): string { +export function getAgentLabel(agentId: string | null): string { + if (!agentId) { + // A retired-agent cleanup cleared it; the user has to pick a replacement. + return translate('auto.components.automations.automation.draft.model.a0f61bd7c9', 'No agent') + } return getAgentCatalog().find((agent) => agent.id === agentId)?.label ?? agentId } diff --git a/src/renderer/src/components/automations/automation-host-client.test.ts b/src/renderer/src/components/automations/automation-host-client.test.ts index e9a5bc03bc2..9afc2d57923 100644 --- a/src/renderer/src/components/automations/automation-host-client.test.ts +++ b/src/renderer/src/components/automations/automation-host-client.test.ts @@ -90,7 +90,7 @@ describe('automation host client', () => { name: automation.name, prompt: automation.prompt, precheck: null, - agentId: automation.agentId, + agentId: 'codex', runContext: automation.runContext, projectId: automation.projectId, workspaceMode: automation.workspaceMode, diff --git a/src/renderer/src/components/automations/automation-source-availability.ts b/src/renderer/src/components/automations/automation-source-availability.ts new file mode 100644 index 00000000000..8ba34020792 --- /dev/null +++ b/src/renderer/src/components/automations/automation-source-availability.ts @@ -0,0 +1,91 @@ +import type { TaskSourceContext } from '../../../../shared/task-source-context' +import type { TaskSourceHostAvailability } from '../task-source-context-summary' +import type { AutomationTargetAvailability } from './automation-target-availability' + +function unavailable( + reason: Exclude, + message: string +): AutomationTargetAvailability { + return { canRunNow: false, reason, message } +} + +/** Blocks a manual run whose saved task-source host cannot serve it right now. */ +export function getAutomationSourceAvailability( + sourceContext: TaskSourceContext | null | undefined, + sourceHostAvailability: readonly TaskSourceHostAvailability[] | undefined +): AutomationTargetAvailability | null { + if (!sourceContext) { + return null + } + const availability = sourceHostAvailability?.find( + (entry) => entry.hostId === sourceContext.hostId + ) + if (!availability) { + return null + } + const providerLabel = getAutomationSourceProviderLabel(sourceContext.provider) + switch (availability.reason) { + case undefined: + break + case 'missing-provider-auth': + return unavailable( + 'source-auth-needed', + `Connect the saved ${providerLabel} source account before running manually.` + ) + case 'unavailable-source-tool': + return unavailable( + 'source-tool-unavailable', + `Install or configure the ${providerLabel} source tool before running manually.` + ) + case 'unsupported-provider': + case 'missing-task-source-capability': + return unavailable( + 'source-provider-unsupported', + `The saved ${providerLabel} source is not supported on this automation host.` + ) + case 'checking-task-source-capability': + return unavailable( + 'source-host-unavailable', + `Checking the saved ${providerLabel} source host before running manually.` + ) + } + if ( + availability.health === 'disconnected' || + availability.health === 'blocked' || + availability.health === 'error' || + availability.status === 'disconnected' || + availability.status === 'auth-failed' || + availability.status === 'reconnection-failed' || + availability.status === 'error' + ) { + return unavailable( + 'source-host-unavailable', + `Reconnect the saved ${providerLabel} source host before running manually.` + ) + } + if ( + availability.health === 'connecting' || + availability.status === 'connecting' || + availability.status === 'deploying-relay' || + availability.status === 'reconnecting' + ) { + return unavailable( + 'source-host-unavailable', + `The saved ${providerLabel} source host is still connecting.` + ) + } + return null +} + +function getAutomationSourceProviderLabel(provider: TaskSourceContext['provider']): string { + switch (provider) { + case 'github': + return 'GitHub' + case 'gitlab': + return 'GitLab' + case 'linear': + return 'Linear' + case 'jira': + return 'Jira' + } +} diff --git a/src/renderer/src/components/automations/automation-target-availability.ts b/src/renderer/src/components/automations/automation-target-availability.ts index 40a6b3dc8f6..5b0beadc43d 100644 --- a/src/renderer/src/components/automations/automation-target-availability.ts +++ b/src/renderer/src/components/automations/automation-target-availability.ts @@ -1,4 +1,7 @@ -import type { Automation } from '../../../../shared/automations-types' +import { + AUTOMATION_MISSING_AGENT_MESSAGE, + type Automation +} from '../../../../shared/automations-types' import { getRepoExecutionHostId, parseExecutionHostId } from '../../../../shared/execution-host' import { describeRuntimeCompatBlock, @@ -10,12 +13,12 @@ import { } from '../../../../shared/protocol-version' import type { AutomationHostTarget } from './automation-host-client' import type { SshConnectionState } from '../../../../shared/ssh-types' -import type { TaskSourceContext } from '../../../../shared/task-source-context' import type { RuntimeStatus } from '../../../../shared/runtime-types' import type { ProjectHostSetup } from '../../../../shared/project-types' import type { Repo } from '../../../../shared/repo-types' import type { Worktree } from '../../../../shared/worktree/types' import type { TaskSourceHostAvailability } from '../task-source-context-summary' +import { getAutomationSourceAvailability } from './automation-source-availability' export type AutomationTargetAvailability = | { @@ -26,6 +29,7 @@ export type AutomationTargetAvailability = | { canRunNow: false reason: + | 'missing-agent' | 'missing-project' | 'missing-project-host-setup' | 'project-host-setup-not-ready' @@ -69,6 +73,9 @@ export function getAutomationTargetAvailability({ automationHostTarget, sourceHostAvailability }: AutomationTargetAvailabilityArgs): AutomationTargetAvailability { + if (!automation.agentId) { + return unavailable('missing-agent', AUTOMATION_MISSING_AGENT_MESSAGE) + } if (!repo) { return unavailable('missing-project', 'The target project is no longer available.') } @@ -184,86 +191,6 @@ function repoHostMatchesRunContext( return targetHostId !== null && getRepoExecutionHostId(repo) === targetHostId } -function getAutomationSourceAvailability( - sourceContext: TaskSourceContext | null | undefined, - sourceHostAvailability: readonly TaskSourceHostAvailability[] | undefined -): AutomationTargetAvailability | null { - if (!sourceContext) { - return null - } - const availability = sourceHostAvailability?.find( - (entry) => entry.hostId === sourceContext.hostId - ) - if (!availability) { - return null - } - const providerLabel = getAutomationSourceProviderLabel(sourceContext.provider) - switch (availability.reason) { - case undefined: - break - case 'missing-provider-auth': - return unavailable( - 'source-auth-needed', - `Connect the saved ${providerLabel} source account before running manually.` - ) - case 'unavailable-source-tool': - return unavailable( - 'source-tool-unavailable', - `Install or configure the ${providerLabel} source tool before running manually.` - ) - case 'unsupported-provider': - case 'missing-task-source-capability': - return unavailable( - 'source-provider-unsupported', - `The saved ${providerLabel} source is not supported on this automation host.` - ) - case 'checking-task-source-capability': - return unavailable( - 'source-host-unavailable', - `Checking the saved ${providerLabel} source host before running manually.` - ) - } - if ( - availability.health === 'disconnected' || - availability.health === 'blocked' || - availability.health === 'error' || - availability.status === 'disconnected' || - availability.status === 'auth-failed' || - availability.status === 'reconnection-failed' || - availability.status === 'error' - ) { - return unavailable( - 'source-host-unavailable', - `Reconnect the saved ${providerLabel} source host before running manually.` - ) - } - if ( - availability.health === 'connecting' || - availability.status === 'connecting' || - availability.status === 'deploying-relay' || - availability.status === 'reconnecting' - ) { - return unavailable( - 'source-host-unavailable', - `The saved ${providerLabel} source host is still connecting.` - ) - } - return null -} - -function getAutomationSourceProviderLabel(provider: TaskSourceContext['provider']): string { - switch (provider) { - case 'github': - return 'GitHub' - case 'gitlab': - return 'GitLab' - case 'linear': - return 'Linear' - case 'jira': - return 'Jira' - } -} - function getRuntimeAutomationAvailability( environmentId: string, runtimeStatusByEnvironmentId: diff --git a/src/renderer/src/components/dashboard/dashboard-worktree-launch-options.test.ts b/src/renderer/src/components/dashboard/dashboard-worktree-launch-options.test.ts index c160b4dd37c..c44dd27d4ae 100644 --- a/src/renderer/src/components/dashboard/dashboard-worktree-launch-options.test.ts +++ b/src/renderer/src/components/dashboard/dashboard-worktree-launch-options.test.ts @@ -67,10 +67,11 @@ describe('buildDashboardWorktreeLaunchOptions', () => { disabledTuiAgents: ['claude'] } as LaunchState['settings'] }), - [card(), card({ paneKey: 'pane-2', agentType: 'gemini' })] + [card(), card({ paneKey: 'pane-2', agentType: 'droid' })] ) - expect(options).toEqual({ 'worktree-1': ['codex', 'gemini'] }) + // droid is undetected locally but proven by a running pane, so it stays offerable. + expect(options).toEqual({ 'worktree-1': ['codex', 'droid'] }) }) it('publishes detected launch choices for workspaces without cards', () => { diff --git a/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-outcomes.test.ts b/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-outcomes.test.ts index 0e98202aab3..f030f61261a 100644 --- a/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-outcomes.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-outcomes.test.ts @@ -90,10 +90,8 @@ vi.mock('@/store', () => ({ vi.mock('@/lib/agent-status', async (importOriginal) => { const actual = await importOriginal>() - const isGeminiTerminalTitle = actual.isGeminiTerminalTitle as (title: string) => boolean return { ...actual, - isGeminiTerminalTitle: vi.fn((title: string) => isGeminiTerminalTitle(title)), isClaudeAgent: vi.fn(() => false), detectAgentStatusFromTitle: vi.fn(() => null) } diff --git a/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-unavailable-banner.repro.test.ts b/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-unavailable-banner.repro.test.ts index ea6f69c1ab7..5e829880e64 100644 --- a/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-unavailable-banner.repro.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-hidden-output-restore-unavailable-banner.repro.test.ts @@ -111,10 +111,8 @@ vi.mock('@/store', () => ({ vi.mock('@/lib/agent-status', async (importOriginal) => { const actual = await importOriginal>() - const isGeminiTerminalTitle = actual.isGeminiTerminalTitle as (title: string) => boolean return { ...actual, - isGeminiTerminalTitle: vi.fn((title: string) => isGeminiTerminalTitle(title)), isClaudeAgent: vi.fn(() => false), detectAgentStatusFromTitle: vi.fn(() => null) } diff --git a/src/renderer/src/hooks/useAutomationDispatchEvents.ts b/src/renderer/src/hooks/useAutomationDispatchEvents.ts index a7741ec959e..e7b92a90425 100644 --- a/src/renderer/src/hooks/useAutomationDispatchEvents.ts +++ b/src/renderer/src/hooks/useAutomationDispatchEvents.ts @@ -8,9 +8,10 @@ import { findReusableAutomationSession } from '@/lib/automation-session-reuse' import { observeExistingAutomationSession } from '@/lib/automation-session-observer' import { launchWorktreeBackgroundTerminals } from '@/lib/launch-worktree-background-terminals' import { useAppStore } from '@/store' -import type { - AutomationDispatchResult, - AutomationPrecheckResult +import { + AUTOMATION_MISSING_AGENT_MESSAGE, + type AutomationDispatchResult, + type AutomationPrecheckResult } from '../../../shared/automations-types' import { getAutomationRunRepoId } from '../../../shared/automation-run-identity' import { @@ -125,6 +126,20 @@ export function useAutomationDispatchEvents(): void { return ownership?.finalize() ?? false } + const agentId = automation.agentId + if (!agentId) { + // Why: the host already skips agentless runs; this covers an older host + // dispatching an automation a retired-agent cleanup has since cleared. + await markDispatchResult({ + runId: run.id, + status: 'skipped_unavailable', + workspaceId: run.workspaceId, + workspaceDisplayName: run.workspaceDisplayName ?? null, + error: AUTOMATION_MISSING_AGENT_MESSAGE + }) + return + } + if (!repo) { await markDispatchResult({ runId: run.id, @@ -499,7 +514,7 @@ export function useAutomationDispatchEvents(): void { if (automation.reuseSession) { const reusableSession = findReusableAutomationSession({ automationId: automation.id, - agentId: automation.agentId, + agentId, worktreeId: worktree.id, currentRunId: run.id, runs: await window.api.automations.listRuns({ automationId: automation.id }), @@ -582,7 +597,7 @@ export function useAutomationDispatchEvents(): void { } } const result = await launchAgentBackgroundSession({ - agent: automation.agentId, + agent: agentId, worktreeId: worktree.id, prompt: automation.prompt, launchSource: 'unknown', diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 5923c6df197..d52d2823f59 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -14587,6 +14587,11 @@ "failed": "Failed" } } + }, + "draft": { + "model": { + "a0f61bd7c9": "No agent" + } } }, "external": { diff --git a/src/renderer/src/lib/launch-agent-background-session.ts b/src/renderer/src/lib/launch-agent-background-session.ts index f65204d6558..71c229e17e8 100644 --- a/src/renderer/src/lib/launch-agent-background-session.ts +++ b/src/renderer/src/lib/launch-agent-background-session.ts @@ -11,7 +11,7 @@ import { resolveTuiAgentLaunchArgs, resolveTuiAgentLaunchEnv } from '../../../shared/tui-agent-launch-defaults' -import { TUI_AGENT_CONFIG } from '../../../shared/tui-agent-config' +import { assertTuiAgent, TUI_AGENT_CONFIG } from '../../../shared/tui-agent-config' import { resolveAgentBackgroundLaunchHost } from '@/lib/agent-background-session-launch-host' import { makePaneKey } from '../../../shared/stable-pane-id' import { @@ -45,6 +45,7 @@ export async function launchAgentBackgroundSession( args: LaunchAgentBackgroundSessionArgs ): Promise { const { agent, worktreeId, prompt, launchSource, title, onData, onExit, onAgentStatus } = args + assertTuiAgent(agent) const store = useAppStore.getState() // Folder workspaces exist only in getKnownWorktreeById (#2989). const worktree = store.getKnownWorktreeById(worktreeId) diff --git a/src/shared/agent-icons/gemini.png b/src/shared/agent-icons/gemini.png deleted file mode 100644 index 84976ebc7fd21e352d8ed963823b66e1b1bdf5a6..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1445 zcmcIj`#aMM82?7HT#g<`2su(plQ2r6aAue>_mJDlCAVD5B|4{#VKin5$qMB#xeVn# z*D}|ljbfNZO>(rETenqY9iSc`h&Ck!gr60~;3 znpbgx>y6}lSuWeVZBZ4NV|qQ~(CTRNioab?aL#&>+GBSi^rp&2J6_P{(1y5ky#Ng7ay6h8aIEr|%fyH!c#mR=6#bBDxc9)g<@p zvZI?xF>Rwm5m8$t*K*@QQddao0=%-)xL|q}fsCVtK+9T!d!tj(Bd)Kqg(oIZvDZ2h z4W75@SG=gcRc4$wKjRtpYSgkPBx^Vzxir4+{1S7u~ZB7V21ODWpnr= z;;7eZgGyY=r1O(ui`GPNEs@>~L3D*@v;|gB?fsuqOkZZ|RgtLOru08U8nM}h=@i?1 z_Wiu-sKTDHsQ59wXKr=OXR1bLLKfye?OhRKaO`4#6*LO}fkO7FB4#!`jv_qGjctyN zt;r0jEClM6gc%tQsa+B#hri{7bO!ch_x?kN%PwPAXIC4pZHb;`D}dvN%~ zfF$N}ucvwZGlYWR)~KfL;}K_@j&?6) z!QvSf%a@FeIH!zfGQl04n~kX)WxdYM>`fgWl|YnM;3LZ$UY@S%!(#Dx{H}j=Ow1?L z!HH9RWKCU4Ly)Ihk}O|b+)CCyvdH)YPY2DKoP>XgPB8_q7AbCzXEqB&MUpP(Bg&k* z(irVWIjgd+-UX&6Cc~T;C*SN}?;c5Ln4CI?Fdv?gl@0=u6t^2bP;Ru6*6!UHXng6Dc!y^4d1)MZ0Kf zNo82!q$xP3vvU1(*!6-n`80x*hVSr9`Qbph1JWO&n@TbMk!=QCU98tJ;4r0oeqznf zynmth@Tb{(VW^k9ii2+Huc_bAN5+cRIT<18Y8_{f*jx}aByh4}>b%eE!1?ykx5pQ< z-xOnlirJ3RAoYNuk7jy6mWI4u?zLY1i>hEnshE!rcBjqc58XpI0xn&cmU^X-eGsk* zXfc7&lUxEzn>zrCAJM6U$UkXXWb>r%oHVN_ -> +> & { + /** Non-null: an update may replace a retired agent but never clear one. */ + agentId?: TuiAgent +} export type AutomationDispatchRequest = { automation: Automation diff --git a/src/shared/tui-agent-config.ts b/src/shared/tui-agent-config.ts index 72ddb83925b..389291eca96 100644 --- a/src/shared/tui-agent-config.ts +++ b/src/shared/tui-agent-config.ts @@ -328,6 +328,15 @@ export function isTuiAgent(value: unknown): value is TuiAgent { return typeof value === 'string' && Object.hasOwn(TUI_AGENT_CONFIG, value) } +/** Guards launch paths against ids that outlive a retired agent in saved state. */ +export function assertTuiAgent(value: unknown): asserts value is TuiAgent { + if (!isTuiAgent(value)) { + throw new Error( + `Unknown or retired TUI agent: ${typeof value === 'string' ? value : String(value)}` + ) + } +} + export function getTuiAgentDetectCommands(config: TuiAgentConfig): string[] { return [config.detectCmd, ...(config.detectCmdAliases ?? [])] } diff --git a/src/shared/tui-agent-draft-launch.ts b/src/shared/tui-agent-draft-launch.ts new file mode 100644 index 00000000000..236e2712767 --- /dev/null +++ b/src/shared/tui-agent-draft-launch.ts @@ -0,0 +1,97 @@ +// Draft launches: seed an agent with unsent composer text instead of a prompt. +import { + clearEnvCommand, + commandSeparator, + quoteStartupArg, + resolveStartupShell, + type AgentStartupShell +} from './tui-agent-startup-shell' +import { assertTuiAgent, TUI_AGENT_CONFIG } from './tui-agent-config' +import type { SleepingAgentLaunchConfig } from './agent-session-resume' +import type { StartupCommandDelivery } from './codex-startup-delivery' +import { buildSleepingAgentLaunchConfig } from './sleeping-agent-launch-config' +import { inlineAgentDraftFitsPlatform } from './agent-draft-platform-limit' +import type { TuiAgent } from './tui-agent' +import type { SessionOptionValue } from './native-chat-session-options' +import { resolveAgentLaunchCommand } from './tui-agent-launch-command' +import { appliedSessionOptionProps } from './tui-agent-session-option-props' + +export type AgentDraftLaunchPlan = { + agent: TuiAgent + launchCommand: string + expectedProcess: string + launchConfig: SleepingAgentLaunchConfig + env?: Record + startupCommandDelivery?: StartupCommandDelivery + sessionOptions?: Record +} + +export function buildAgentDraftLaunchPlan(args: { + agent: TuiAgent + draft: string + cmdOverrides: Partial> + platform: NodeJS.Platform + shell?: AgentStartupShell + agentArgs?: string | null + agentEnv?: Record | null + sessionOptions?: Record + /** Why: see buildAgentStartupPlan — remote launches use the plain `orca` shim. */ + isRemote?: boolean +}): AgentDraftLaunchPlan | null { + const { agent, draft, cmdOverrides, platform } = args + assertTuiAgent(agent) + const shell = resolveStartupShell(platform, args.shell) + const config = TUI_AGENT_CONFIG[agent] + const trimmed = draft.trim() + if (!trimmed) { + return null + } + const baseCommand = resolveAgentLaunchCommand({ + agent, + cmdOverrides, + platform, + shell, + agentArgs: args.agentArgs, + sessionOptions: args.sessionOptions, + isRemote: args.isRemote + }) + if (!baseCommand.ok) { + return null + } + const launchConfig = buildSleepingAgentLaunchConfig({ + ...args, + // Why: see the new-session path above — resume must not replay picker flags. + agentCommand: baseCommand.commandWithoutSessionOptions + }) + let plan: AgentDraftLaunchPlan | null = null + if (config.draftPromptFlag) { + const quoted = quoteStartupArg(trimmed, shell) + plan = { + agent, + launchCommand: `${baseCommand.command} ${config.draftPromptFlag} ${quoted}`, + expectedProcess: config.expectedProcess, + launchConfig, + ...appliedSessionOptionProps(baseCommand.appliedSessionOptions), + // Why: native draft flags carry user text on argv and must survive rc-file startup. + ...(agent === 'codex' ? { startupCommandDelivery: 'shell-ready' as const } : {}), + ...(args.agentEnv ? { env: { ...args.agentEnv } } : {}) + } + } else if (config.draftPromptEnvVar) { + const clearVar = clearEnvCommand(config.draftPromptEnvVar, shell) + plan = { + agent, + launchCommand: `${baseCommand.command}${commandSeparator(shell)}${clearVar}`, + expectedProcess: config.expectedProcess, + launchConfig, + ...appliedSessionOptionProps(baseCommand.appliedSessionOptions), + env: { ...args.agentEnv, [config.draftPromptEnvVar]: trimmed } + } + } + if ( + !plan || + !inlineAgentDraftFitsPlatform({ command: plan.launchCommand, env: plan.env, platform }) + ) { + return null + } + return plan +} diff --git a/src/shared/tui-agent-selection.test.ts b/src/shared/tui-agent-selection.test.ts index e7b87db0af4..0177494d5f2 100644 --- a/src/shared/tui-agent-selection.test.ts +++ b/src/shared/tui-agent-selection.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { haveSameDisabledTuiAgents, + isTuiAgentEnabled, normalizeDisabledTuiAgents, pickTuiAgent } from './tui-agent-selection' @@ -42,3 +43,16 @@ describe('haveSameDisabledTuiAgents', () => { expect(haveSameDisabledTuiAgents(['codex'], ['claude'])).toBe(false) }) }) + +describe('isTuiAgentEnabled', () => { + it('returns false for retired or unknown agent ids', () => { + expect(isTuiAgentEnabled('gemini')).toBe(false) + expect(isTuiAgentEnabled('not-an-agent')).toBe(false) + expect(isTuiAgentEnabled(null)).toBe(false) + }) + + it('returns false when a supported agent is disabled', () => { + expect(isTuiAgentEnabled('claude', ['claude'])).toBe(false) + expect(isTuiAgentEnabled('claude', ['codex'])).toBe(true) + }) +}) diff --git a/src/shared/tui-agent-selection.ts b/src/shared/tui-agent-selection.ts index 96700b1d80f..dd6f973cda3 100644 --- a/src/shared/tui-agent-selection.ts +++ b/src/shared/tui-agent-selection.ts @@ -85,8 +85,10 @@ export function haveSameDisabledTuiAgents(left: unknown, right: unknown): boolea 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) +export function isTuiAgentEnabled(agent: unknown, disabled?: Iterable | null): boolean { + // Why: retired/unknown ids must never count as "enabled" — callers pass + // persisted strings that outlive the TuiAgent union. + return isTuiAgent(agent) && !normalizeDisabledTuiAgents(disabled).includes(agent) } export function filterEnabledTuiAgents( diff --git a/src/shared/tui-agent-session-option-props.ts b/src/shared/tui-agent-session-option-props.ts new file mode 100644 index 00000000000..5e03f5115cc --- /dev/null +++ b/src/shared/tui-agent-session-option-props.ts @@ -0,0 +1,8 @@ +import type { SessionOptionValue } from './native-chat-session-options' + +/** Omits the key entirely when no picker options applied, keeping plans byte-stable. */ +export function appliedSessionOptionProps(values: Record): { + sessionOptions?: Record +} { + return Object.keys(values).length > 0 ? { sessionOptions: { ...values } } : {} +} diff --git a/src/shared/tui-agent-startup.test.ts b/src/shared/tui-agent-startup.test.ts index b3220956677..7e37c43b837 100644 --- a/src/shared/tui-agent-startup.test.ts +++ b/src/shared/tui-agent-startup.test.ts @@ -39,6 +39,17 @@ describe('tui agent startup plans', () => { } ) + it('rejects retired agent ids before indexing TUI_AGENT_CONFIG', () => { + expect(() => + buildAgentStartupPlan({ + agent: 'gemini' as never, + prompt: 'hello', + cmdOverrides: {}, + platform: 'linux' + }) + ).toThrow(/Unknown or retired TUI agent: gemini/) + }) + it('uses POSIX quoting when the target shell is Linux', () => { const plan = buildAgentStartupPlan({ agent: 'claude', diff --git a/src/shared/tui-agent-startup.ts b/src/shared/tui-agent-startup.ts index 7b013c4ffd7..233610ed8ae 100644 --- a/src/shared/tui-agent-startup.ts +++ b/src/shared/tui-agent-startup.ts @@ -6,21 +6,19 @@ import { type SleepingAgentLaunchConfig } from './agent-session-resume' import { - clearEnvCommand, - commandSeparator, quoteStartupArg, resolveStartupShell, type AgentStartupShell } from './tui-agent-startup-shell' -import { TUI_AGENT_CONFIG } from './tui-agent-config' +import { assertTuiAgent, TUI_AGENT_CONFIG } from './tui-agent-config' import type { StartupCommandDelivery } from './codex-startup-delivery' import { buildSleepingAgentLaunchConfig } from './sleeping-agent-launch-config' import { planHermesStartupQuery } from './hermes-startup-query' -import { inlineAgentDraftFitsPlatform } from './agent-draft-platform-limit' import type { TuiAgent } from './tui-agent' import type { SessionOptionValue } from './native-chat-session-options' import { resolveAgentLaunchCommand } from './tui-agent-launch-command' import { buildAgentResumeLaunchCommand } from './agent-resume-launch-command' +import { appliedSessionOptionProps } from './tui-agent-session-option-props' export type AgentStartupPlan = { agent: TuiAgent @@ -37,10 +35,6 @@ export type AgentStartupPlan = { sessionOptions?: Record } -function appliedSessionOptionProps(values: Record) { - return Object.keys(values).length > 0 ? { sessionOptions: { ...values } } : {} -} - export function buildAgentStartupPlan(args: { agent: TuiAgent prompt: string @@ -57,6 +51,7 @@ export function buildAgentStartupPlan(args: { isRemote?: boolean }): AgentStartupPlan | null { const { agent, prompt, cmdOverrides, platform, allowEmptyPromptLaunch = false } = args + assertTuiAgent(agent) const shell = resolveStartupShell(platform, args.shell) const trimmedPrompt = prompt.trim() const config = TUI_AGENT_CONFIG[agent] @@ -199,6 +194,7 @@ export function buildAgentResumeStartupPlan(args: { /** Why: see buildAgentStartupPlan — remote launches use the plain `orca` shim. */ isRemote?: boolean }): AgentStartupPlan | null { + assertTuiAgent(args.agent) const argv = getAgentResumeArgv(args.agent, args.providerSession, args.ompResumeFilePath) if (!argv) { return null @@ -233,84 +229,7 @@ export function buildAgentResumeStartupPlan(args: { } } -export type AgentDraftLaunchPlan = { - agent: TuiAgent - launchCommand: string - expectedProcess: string - launchConfig: SleepingAgentLaunchConfig - env?: Record - startupCommandDelivery?: StartupCommandDelivery - sessionOptions?: Record -} - -export function buildAgentDraftLaunchPlan(args: { - agent: TuiAgent - draft: string - cmdOverrides: Partial> - platform: NodeJS.Platform - shell?: AgentStartupShell - agentArgs?: string | null - agentEnv?: Record | null - sessionOptions?: Record - /** Why: see buildAgentStartupPlan — remote launches use the plain `orca` shim. */ - isRemote?: boolean -}): AgentDraftLaunchPlan | null { - const { agent, draft, cmdOverrides, platform } = args - const shell = resolveStartupShell(platform, args.shell) - const config = TUI_AGENT_CONFIG[agent] - const trimmed = draft.trim() - if (!trimmed) { - return null - } - const baseCommand = resolveAgentLaunchCommand({ - agent, - cmdOverrides, - platform, - shell, - agentArgs: args.agentArgs, - sessionOptions: args.sessionOptions, - isRemote: args.isRemote - }) - if (!baseCommand.ok) { - return null - } - const launchConfig = buildSleepingAgentLaunchConfig({ - ...args, - // Why: see the new-session path above — resume must not replay picker flags. - agentCommand: baseCommand.commandWithoutSessionOptions - }) - let plan: AgentDraftLaunchPlan | null = null - if (config.draftPromptFlag) { - const quoted = quoteStartupArg(trimmed, shell) - plan = { - agent, - launchCommand: `${baseCommand.command} ${config.draftPromptFlag} ${quoted}`, - expectedProcess: config.expectedProcess, - launchConfig, - ...appliedSessionOptionProps(baseCommand.appliedSessionOptions), - // Why: native draft flags carry user text on argv and must survive rc-file startup. - ...(agent === 'codex' ? { startupCommandDelivery: 'shell-ready' as const } : {}), - ...(args.agentEnv ? { env: { ...args.agentEnv } } : {}) - } - } else if (config.draftPromptEnvVar) { - const clearVar = clearEnvCommand(config.draftPromptEnvVar, shell) - plan = { - agent, - launchCommand: `${baseCommand.command}${commandSeparator(shell)}${clearVar}`, - expectedProcess: config.expectedProcess, - launchConfig, - ...appliedSessionOptionProps(baseCommand.appliedSessionOptions), - env: { ...args.agentEnv, [config.draftPromptEnvVar]: trimmed } - } - } - if ( - !plan || - !inlineAgentDraftFitsPlatform({ command: plan.launchCommand, env: plan.env, platform }) - ) { - return null - } - return plan -} +export { buildAgentDraftLaunchPlan, type AgentDraftLaunchPlan } from './tui-agent-draft-launch' export { isShellProcess } export {