From 4ff07da23c1366cbe34d5813bb5e32c8276488ec Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:23:19 -0700 Subject: [PATCH] fix(agent-hooks): stop startup from deleting another instance's managed hooks (STA-5679) Startup reconciliation removed the managed agent hooks whenever THIS profile had the agent-status-hooks off switch set. The hook files it removes are user-global (~/.claude/settings.json, ~/.cursor/hooks.json), so a second Orca profile with the switch off deleted the hooks every other running instance depends on. Cursor is the only agent with no title-derived status fallback: its native title is deliberately parsed as status-less, so a hookless Cursor pane is floored at 'idle' rather than showing a spinner. A global hook wipe therefore surfaces as "Cursor loading status missing from the sidebar" while Claude and Codex still paint status from their own titles, which is why this reads as a Cursor-only bug. Codex is unaffected either way because its hooks live in an Orca-owned runtime home. Honoring the off switch only requires skipping the install; removal stays on the explicit Settings toggle, which is the user-initiated path that should own it. Regression from #2778, which restored the destructive startup branch. --- .../managed-agent-hook-controls.test.ts | 31 +++++++++++++ .../managed-agent-hook-controls.ts | 13 ++++++ src/main/index.ts | 43 +++++++++---------- 3 files changed, 65 insertions(+), 22 deletions(-) diff --git a/src/main/agent-hooks/managed-agent-hook-controls.test.ts b/src/main/agent-hooks/managed-agent-hook-controls.test.ts index 6ea28d66247..204a004ec92 100644 --- a/src/main/agent-hooks/managed-agent-hook-controls.test.ts +++ b/src/main/agent-hooks/managed-agent-hook-controls.test.ts @@ -45,6 +45,7 @@ import { applyAgentStatusHooksEnabled, installManagedAgentHooks, removeManagedAgentHooksAsync, + resolveStartupManagedHookAction, shouldContinueManagedHookStartup } from './managed-agent-hook-controls' @@ -273,3 +274,33 @@ describe('managed agent hook controls', () => { expect(results).toEqual([expect.objectContaining({ agent: 'codex', state: 'not_installed' })]) }) }) + +describe('startup managed hook reconciliation (STA-5679)', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('skips instead of removing when this profile has the off switch set', () => { + // Why this matters: the hook files are user-global. Startup removal here deleted the hooks that + // every other Orca instance depends on, and Cursor then reads as idle with no status at all. + expect(resolveStartupManagedHookAction({ agentStatusHooksEnabled: false })).toBe('skip') + }) + + it('installs when hooks are enabled or the setting is unset', () => { + expect(resolveStartupManagedHookAction({ agentStatusHooksEnabled: true })).toBe('install') + expect(resolveStartupManagedHookAction({})).toBe('install') + expect(resolveStartupManagedHookAction(null)).toBe('install') + }) + + it('still removes through the explicit Settings toggle', async () => { + // Anchors the assertion above: the removers really are wired, so 'skip' is a behavioral + // difference rather than a vacuous constant. + mocks.removeClaude.mockResolvedValue(status('claude', 'not_installed')) + mocks.removeCodex.mockResolvedValue(status('codex', 'not_installed')) + + await applyAgentStatusHooksEnabled(false, { agentStatusHooksEnabled: false }) + + 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 64d426ae072..2a02acbee90 100644 --- a/src/main/agent-hooks/managed-agent-hook-controls.ts +++ b/src/main/agent-hooks/managed-agent-hook-controls.ts @@ -41,6 +41,19 @@ export function isAgentStatusHooksEnabled( return settings?.agentStatusHooksEnabled !== false } +export type StartupManagedHookAction = 'install' | 'skip' + +// Why never 'remove': this reads THIS instance's settings, but the managed hook files are +// user-global (~/.claude/settings.json, ~/.cursor/hooks.json). A second Orca profile with the off +// switch set would delete the hooks every other instance depends on, and Cursor — the one agent +// with no title-derived status fallback — then goes silently idle (STA-5679). Honoring the off +// switch only requires skipping the install; explicit removal stays on the Settings toggle. +export function resolveStartupManagedHookAction( + settings: ManagedHookSettings +): StartupManagedHookAction { + return isAgentStatusHooksEnabled(settings) ? 'install' : 'skip' +} + export function shouldContinueManagedHookStartup( isQuitting: boolean, settings: ManagedHookSettings, diff --git a/src/main/index.ts b/src/main/index.ts index 79f3f53f3a5..8b181a4d2fc 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -91,8 +91,8 @@ import { import { applyAgentStatusHooksEnabled, isAgentStatusHooksEnabled, - removeManagedAgentHooks, removeManagedAgentHooksAsync, + resolveStartupManagedHookAction, shouldContinueManagedHookStartup } from './agent-hooks/managed-agent-hook-controls' import { initCohortClassifier } from './telemetry/cohort-classifier' @@ -3097,29 +3097,28 @@ void app.whenReady().then(async () => { console.warn('[codex-real-home-hooks] startup ensure failed:', error) }) : Promise.resolve() - if (shouldInstallManagedHooks(is.dev)) { - // Why: check the persisted off switch before any auto-install so removed hooks don't silently reappear on launch. - if (isAgentStatusHooksEnabled(store.getSettings())) { - const managedHookStore = store - void realHomeCodexHookState - .then(() => - applyAgentStatusHooksEnabled(true, managedHookStore.getSettings(), { - shouldHydrateShellPath: app.isPackaged, - onInstallError: recordManagedHookInstallFailure, - shouldContinue: (agent) => { - const settings = managedHookStore.getSettings() - return shouldContinueManagedHookStartup(isQuitting, settings, agent) - } - }) - ) - .catch((error: unknown) => { - console.warn('[agent-hooks] failed to reconcile managed hooks on startup:', error) + // Why skip rather than remove when the off switch is set: the hook files are user-global but this + // decision reads only THIS profile's settings, so removing here deletes the hooks every other Orca + // instance depends on (STA-5679). Skipping already keeps removed hooks from reappearing on launch. + if ( + shouldInstallManagedHooks(is.dev) && + resolveStartupManagedHookAction(store.getSettings()) === 'install' + ) { + const managedHookStore = store + void realHomeCodexHookState + .then(() => + applyAgentStatusHooksEnabled(true, managedHookStore.getSettings(), { + shouldHydrateShellPath: app.isPackaged, + onInstallError: recordManagedHookInstallFailure, + shouldContinue: (agent) => { + const settings = managedHookStore.getSettings() + return shouldContinueManagedHookStartup(isQuitting, settings, agent) + } }) - } else { - void removeManagedAgentHooks().catch((error: unknown) => { - console.warn('[agent-hooks] failed to remove managed hooks on startup:', error) + ) + .catch((error: unknown) => { + console.warn('[agent-hooks] failed to reconcile managed hooks on startup:', error) }) - } } // Why: process-gone metrics only see survivors; retain a recent whole-app // snapshot for comparison in crash reports.