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.
This commit is contained in:
Brennan Benson
2026-08-27 23:23:19 -07:00
parent 6c0de76ced
commit 4ff07da23c
3 changed files with 65 additions and 22 deletions
@@ -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)
})
})
@@ -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,
+21 -22
View File
@@ -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.