mirror of
https://github.com/stablyai/orca.git
synced 2026-09-27 16:02:35 +00:00
test(onboarding): cover the first-run gate's wiring, not just its decision
A second review found the same defect class as the first, one layer up: bypassing the startup gate entirely left 526 test files green, and deleting the three props connecting the checkbox to any state left all 115 onboarding renderer tests green. The previous round deleted a vacuous grep test and replaced it with a pure-function table test, which made the decision well covered and the wiring not covered at all. Fixing the anchor was the answer; removing the layer was not. Add behavioural wiring tests at both seams, each verified by ablation: driving the real ready-phase with only the installer and store faked, and driving the checkbox through OnboardingFlow rather than AgentStep, since an AgentStep-level test cannot see the props go missing. Also cover the consent ref's freshness, which is what makes uncheck-then-Esc carry the right value. Drop the disclosure's "isn't uploaded anywhere". Agent-hook envelopes are published to paired devices, and hook ingestion fires telemetry, so both that claim and the narrower "not sent as analytics" are falsifiable by grep. On a consent surface an absolute a reviewer can disprove is worse than the silence it replaces, so the line now states only what was traced.
This commit is contained in:
@@ -125,7 +125,10 @@ async function readHookSettings(
|
||||
|
||||
function updateEnabledOnDisk(enabled: boolean): {
|
||||
settingsPath: string
|
||||
settings: Pick<GlobalSettings, 'agentCmdOverrides' | 'disabledTuiAgents'>
|
||||
settings: Pick<
|
||||
GlobalSettings,
|
||||
'agentCmdOverrides' | 'agentStatusHooksEnabled' | 'disabledTuiAgents'
|
||||
>
|
||||
} {
|
||||
const dataPath = getDataPath()
|
||||
const state = readPersistedState(dataPath)
|
||||
@@ -139,6 +142,9 @@ function updateEnabledOnDisk(enabled: boolean): {
|
||||
settingsPath: dataPath,
|
||||
settings: {
|
||||
agentCmdOverrides: state.settings.agentCmdOverrides ?? {},
|
||||
// Why echoed back: installManagedAgentHooks reads its own authorization off this object, so
|
||||
// omitting the field we just wrote would leave the offline install passing only by default.
|
||||
agentStatusHooksEnabled: enabled,
|
||||
disabledTuiAgents: state.settings.disabledTuiAgents ?? []
|
||||
}
|
||||
}
|
||||
|
||||
@@ -169,6 +169,9 @@ describe('RuntimeClient module-graph deferral', () => {
|
||||
`${argv.join(' ')} hook application`
|
||||
).toHaveBeenCalledExactlyOnceWith(false, {
|
||||
agentCmdOverrides: {},
|
||||
// Echoed back from the disk write so installManagedAgentHooks' own authorization guard
|
||||
// reads the value the CLI just set instead of falling through to the default.
|
||||
agentStatusHooksEnabled: false,
|
||||
disabledTuiAgents: []
|
||||
})
|
||||
} else {
|
||||
|
||||
@@ -98,7 +98,9 @@ async function runInstaller(
|
||||
// Why (#11549 aftermath): a CLI that falls off PATH keeps its user-wide config invoking
|
||||
// Orca's script, but the presence gate below then skips install() forever, freezing the
|
||||
// script at whatever Orca generated last. Existing scripts are Orca-owned, so bring them
|
||||
// current before any gating; creating new ones remains install()'s presence-gated job.
|
||||
// current before the presence gate; creating new ones remains install()'s presence-gated job.
|
||||
// The hooks-off guard above is the one gate that still wins: ~/.orca/agent-hooks/ is user-global,
|
||||
// so a profile that declined writes nothing there either, and a consenting profile refreshes it.
|
||||
async function refreshExistingManagedScripts(options: InstallOptions): Promise<void> {
|
||||
const allowed = options.agents ? new Set(options.agents) : null
|
||||
for (const [agent, refresh] of MANAGED_AGENT_HOOK_SCRIPT_REFRESHERS) {
|
||||
|
||||
@@ -0,0 +1,211 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { GlobalSettings } from '../../shared/global-settings-types'
|
||||
import type { OnboardingState } from '../../shared/onboarding-state-types'
|
||||
import type * as AgentStatusHooksEnablement from '../agent-hooks/agent-status-hooks-enablement'
|
||||
|
||||
/**
|
||||
* The behavioural half of the managed-hook first-run gate. `startup-managed-hook-plan.test.ts`
|
||||
* tables the decision; this drives the real ready phase so that dropping the plan call, the
|
||||
* `shouldReconcile` conjunct or the latch-retirement write turns red. Without it the whole startup
|
||||
* gate can be reverted to origin/main with every other suite still green.
|
||||
*/
|
||||
const {
|
||||
installManagedAgentHooksMock,
|
||||
ensureRealHomeCodexHookStateMock,
|
||||
mainProcessStateFake,
|
||||
runtimeFake
|
||||
} = vi.hoisted(() => ({
|
||||
installManagedAgentHooksMock: vi.fn(async () => []),
|
||||
ensureRealHomeCodexHookStateMock: vi.fn(async () => undefined),
|
||||
runtimeFake: {
|
||||
setAgentBrowserBridge: vi.fn(),
|
||||
setEmulatorBridge: vi.fn(),
|
||||
notifyMobileSessionTabsChanged: vi.fn()
|
||||
},
|
||||
mainProcessStateFake: {
|
||||
store: null as unknown,
|
||||
stats: {},
|
||||
isQuitting: false,
|
||||
isServeMode: false,
|
||||
codexRuntimeHome: null as { isHostSystemDefaultRealHomeSelected: () => boolean } | null,
|
||||
agentBrowserBridge: null as unknown,
|
||||
emulatorBridge: null as unknown,
|
||||
gpuCrashDiagnostics: null
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('electron', () => ({
|
||||
app: {
|
||||
isPackaged: false,
|
||||
getPath: vi.fn(() => '/tmp/orca-managed-hook-gate-test'),
|
||||
on: vi.fn()
|
||||
},
|
||||
nativeTheme: { themeSource: 'system' }
|
||||
}))
|
||||
vi.mock('@electron-toolkit/utils', () => ({ is: { dev: false } }))
|
||||
vi.mock('../star-nag/service', () => ({
|
||||
StarNagService: class {
|
||||
start = vi.fn()
|
||||
registerIpcHandlers = vi.fn()
|
||||
}
|
||||
}))
|
||||
vi.mock('../browser/agent-browser-bridge', () => ({
|
||||
AgentBrowserBridge: class {
|
||||
sweepOrphanedSessions = vi.fn(async () => undefined)
|
||||
}
|
||||
}))
|
||||
vi.mock('../emulator/emulator-bridge', () => ({ EmulatorBridge: class {} }))
|
||||
vi.mock('../runtime/rpc/dispatcher', () => ({
|
||||
RpcDispatcher: class {
|
||||
dispatch = vi.fn()
|
||||
}
|
||||
}))
|
||||
vi.mock('../browser/browser-manager', () => ({ browserManager: {} }))
|
||||
vi.mock('../browser/browser-client-page-automation-runtime', () => ({
|
||||
configureBrowserClientPageAutomationRuntime: vi.fn()
|
||||
}))
|
||||
vi.mock('../browser/browser-client-page-command-failure', () => ({
|
||||
BrowserClientPageCommandError: class extends Error {}
|
||||
}))
|
||||
vi.mock('../crash-reporting/process-gone-diagnostics', () => ({
|
||||
startPreGoneCrashSampling: vi.fn()
|
||||
}))
|
||||
vi.mock('./main-window-lifecycle-flags', () => ({ recordProcessGoneCrash: vi.fn() }))
|
||||
vi.mock('./gpu-lifecycle', () => ({ handleGpuChildCrash: vi.fn() }))
|
||||
vi.mock('../crash-reporting/gpu-crash-fallback-decision', () => ({
|
||||
isGpuFallbackCrashCandidate: vi.fn(() => false)
|
||||
}))
|
||||
vi.mock('../codex/codex-real-home-hook-install', () => ({
|
||||
ensureRealHomeCodexHookState: ensureRealHomeCodexHookStateMock
|
||||
}))
|
||||
// Only the installer is faked: the real module's registry drags in every per-agent hook service,
|
||||
// but the off-switch predicates live in a registry-free module, so those stay real.
|
||||
vi.mock('../agent-hooks/managed-agent-hook-controls', async () => {
|
||||
const enablement = await vi.importActual<typeof AgentStatusHooksEnablement>(
|
||||
'../agent-hooks/agent-status-hooks-enablement'
|
||||
)
|
||||
return {
|
||||
installManagedAgentHooks: installManagedAgentHooksMock,
|
||||
resolveStartupManagedHookAction: enablement.resolveStartupManagedHookAction,
|
||||
shouldContinueManagedHookStartup: enablement.shouldContinueManagedHookStartup,
|
||||
shouldInstallStartupManagedAgentHook: enablement.shouldInstallStartupManagedAgentHook
|
||||
}
|
||||
})
|
||||
vi.mock('./configure-process', () => ({ shouldInstallManagedHooks: vi.fn(() => true) }))
|
||||
vi.mock('../agent-hooks/install-telemetry', () => ({
|
||||
recordManagedHookInstallFailure: vi.fn()
|
||||
}))
|
||||
vi.mock('./main-process-state', () => ({ mainProcessState: mainProcessStateFake }))
|
||||
vi.mock('./main-process-observers', () => ({ initializeMainProcessObservers: vi.fn() }))
|
||||
vi.mock('./main-process-account-services', () => ({
|
||||
initializeMainProcessAccountServices: vi.fn()
|
||||
}))
|
||||
vi.mock('./main-process-runtime-service', () => ({
|
||||
initializeMainProcessRuntime: vi.fn(() => runtimeFake),
|
||||
configureRuntimeServices: vi.fn()
|
||||
}))
|
||||
vi.mock('./main-process-automations', () => ({ initializeMainProcessAutomations: vi.fn() }))
|
||||
vi.mock('./main-process-plugins', () => ({
|
||||
initializeMainProcessPlugins: vi.fn(async () => undefined)
|
||||
}))
|
||||
vi.mock('../worktree-trash', () => ({
|
||||
collectWorktreeTrashSweepRoots: vi.fn(() => []),
|
||||
sweepStaleWorktreeTrash: vi.fn(async () => undefined)
|
||||
}))
|
||||
vi.mock('./first-window-deferral', () => ({ runAfterFirstWindowShown: vi.fn() }))
|
||||
vi.mock('./startup-diagnostics', () => ({ logStartupMilestone: vi.fn() }))
|
||||
|
||||
import { initializeReadyRuntimeServices } from './main-process-ready-runtime'
|
||||
|
||||
function createStoreFake(initial: {
|
||||
onboarding?: Partial<OnboardingState>
|
||||
settings?: Partial<GlobalSettings>
|
||||
}) {
|
||||
let settings = {
|
||||
disabledTuiAgents: [],
|
||||
managedAgentHookFirstRunGate: 'pending',
|
||||
...initial.settings
|
||||
} as GlobalSettings
|
||||
const onboarding = {
|
||||
flowVersion: 1,
|
||||
closedAt: null,
|
||||
outcome: null,
|
||||
lastCompletedStep: -1,
|
||||
...initial.onboarding
|
||||
} as OnboardingState
|
||||
return {
|
||||
getSettings: vi.fn(() => settings),
|
||||
getOnboarding: vi.fn(() => onboarding),
|
||||
getRepos: vi.fn(() => []),
|
||||
updateSettings: vi.fn((updates: Partial<GlobalSettings>) => {
|
||||
settings = { ...settings, ...updates }
|
||||
return settings
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
function latchWrites(store: ReturnType<typeof createStoreFake>): Partial<GlobalSettings>[] {
|
||||
return store.updateSettings.mock.calls
|
||||
.map(([updates]) => updates)
|
||||
.filter((updates) => 'managedAgentHookFirstRunGate' in updates)
|
||||
}
|
||||
|
||||
async function runReadyPhase(store: ReturnType<typeof createStoreFake>): Promise<void> {
|
||||
mainProcessStateFake.store = store
|
||||
await initializeReadyRuntimeServices()
|
||||
// The reconcile hangs off the codex real-home chain, so drain its microtasks before asserting.
|
||||
for (let tick = 0; tick < 8; tick += 1) {
|
||||
await Promise.resolve()
|
||||
}
|
||||
}
|
||||
|
||||
describe('managed hook first-run gate in the ready phase', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mainProcessStateFake.isServeMode = false
|
||||
mainProcessStateFake.isQuitting = false
|
||||
mainProcessStateFake.codexRuntimeHome = null
|
||||
})
|
||||
|
||||
it('writes nothing user-global for a fresh profile that has not reached step 1', async () => {
|
||||
const store = createStoreFake({})
|
||||
|
||||
await runReadyPhase(store)
|
||||
|
||||
expect(installManagedAgentHooksMock).not.toHaveBeenCalled()
|
||||
expect(ensureRealHomeCodexHookStateMock).not.toHaveBeenCalled()
|
||||
// The latch must stay armed, or the next launch installs without ever having asked.
|
||||
expect(latchWrites(store)).toEqual([])
|
||||
})
|
||||
|
||||
it('reconciles once and retires the latch after the user passes step 1', async () => {
|
||||
const store = createStoreFake({ onboarding: { lastCompletedStep: 1 } })
|
||||
|
||||
await runReadyPhase(store)
|
||||
|
||||
expect(latchWrites(store)).toEqual([{ managedAgentHookFirstRunGate: 'done' }])
|
||||
expect(installManagedAgentHooksMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('never defers on a serve host, which never paints the wizard', async () => {
|
||||
mainProcessStateFake.isServeMode = true
|
||||
const store = createStoreFake({})
|
||||
|
||||
await runReadyPhase(store)
|
||||
|
||||
expect(latchWrites(store)).toEqual([{ managedAgentHookFirstRunGate: 'done' }])
|
||||
expect(installManagedAgentHooksMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('still honours the off switch on a profile whose latch has already retired', async () => {
|
||||
const store = createStoreFake({
|
||||
onboarding: { lastCompletedStep: 1 },
|
||||
settings: { managedAgentHookFirstRunGate: 'done', agentStatusHooksEnabled: false }
|
||||
})
|
||||
|
||||
await runReadyPhase(store)
|
||||
|
||||
expect(installManagedAgentHooksMock).not.toHaveBeenCalled()
|
||||
expect(latchWrites(store)).toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -75,7 +75,7 @@ export function AgentStatusHooksControl({
|
||||
<AffectedAgents affected={affected} isDetecting={isDetecting} />
|
||||
{translate(
|
||||
'auto.components.onboarding.AgentStatusHooksControl.affectedScriptNote',
|
||||
"Plus a small script in ~/.orca/agent-hooks/. Status is reported to Orca on your machine, it isn't uploaded anywhere."
|
||||
"Plus a small script in ~/.orca/agent-hooks/. Status goes to Orca on this machine and to the devices you've paired with it."
|
||||
)}{' '}
|
||||
{translate(
|
||||
'auto.components.onboarding.AgentStatusHooksControl.affectedApproximate',
|
||||
|
||||
+121
@@ -0,0 +1,121 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import '@testing-library/jest-dom/vitest'
|
||||
import { act, cleanup, render, renderHook, screen, within } from '@testing-library/react'
|
||||
import userEvent from '@testing-library/user-event'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { getDefaultOnboardingState, getDefaultSettings } from '../../../../shared/constants'
|
||||
import type { GlobalSettings } from '../../../../shared/global-settings-types'
|
||||
import type { OnboardingState } from '../../../../shared/onboarding-state-types'
|
||||
|
||||
/**
|
||||
* The wiring between the consent checkbox and the state that actually travels to main. The
|
||||
* control's own rendering is covered by AgentStatusHooksControl.test.tsx; what is covered here is
|
||||
* that OnboardingFlow hands AgentStep the props that make the box live, and that the ref closeWith
|
||||
* reads tracks the box rather than freezing at its first-render value.
|
||||
*/
|
||||
const { storeState, updateSettingsMock, trackMock } = vi.hoisted(() => {
|
||||
const updateSettingsMock = vi.fn(() => Promise.resolve())
|
||||
return {
|
||||
updateSettingsMock,
|
||||
trackMock: vi.fn(),
|
||||
storeState: {
|
||||
settings: null as GlobalSettings | null,
|
||||
updateSettings: updateSettingsMock,
|
||||
refreshDetectedAgents: vi.fn(() => Promise.resolve([])),
|
||||
refreshPreflightStatus: vi.fn(() => Promise.resolve()),
|
||||
recordFeatureInteraction: vi.fn(),
|
||||
openModal: vi.fn(),
|
||||
detectedAgentIds: [] as string[],
|
||||
isDetectingAgents: false,
|
||||
isRefreshingAgents: false,
|
||||
pathSource: null,
|
||||
pathFailureReason: null,
|
||||
preflightStatus: null,
|
||||
preflightStatusChecked: false,
|
||||
preflightStatusLoading: false,
|
||||
linearStatus: null,
|
||||
linearStatusChecked: false
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('@/store', () => {
|
||||
const useAppStore = (selector: (state: typeof storeState) => unknown): unknown =>
|
||||
selector(storeState)
|
||||
useAppStore.getState = (): typeof storeState => storeState
|
||||
return { useAppStore }
|
||||
})
|
||||
|
||||
vi.mock('@/lib/telemetry', () => ({ track: trackMock }))
|
||||
|
||||
import OnboardingFlow from './OnboardingFlow'
|
||||
import { useOnboardingFlow } from './use-onboarding-flow'
|
||||
|
||||
const CHECKBOX_LABEL = 'Enable agent status hooks'
|
||||
|
||||
function onboardingUpdateMock(): ReturnType<typeof vi.fn> {
|
||||
return (window as unknown as { api: { onboarding: { update: ReturnType<typeof vi.fn> } } }).api
|
||||
.onboarding.update
|
||||
}
|
||||
|
||||
function freshOnboarding(): OnboardingState {
|
||||
return getDefaultOnboardingState()
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
storeState.settings = {
|
||||
...getDefaultSettings('/tmp'),
|
||||
agentStatusHooksEnabled: true,
|
||||
disabledTuiAgents: ['cursor']
|
||||
} as GlobalSettings
|
||||
storeState.detectedAgentIds = ['claude', 'cursor']
|
||||
;(window as unknown as { api: unknown }).api = {
|
||||
onboarding: { update: vi.fn(() => Promise.resolve(freshOnboarding())) },
|
||||
starNag: { onboardingCompleted: vi.fn(() => Promise.resolve()) },
|
||||
shell: { openUrl: vi.fn(() => Promise.resolve()) }
|
||||
}
|
||||
})
|
||||
|
||||
afterEach(cleanup)
|
||||
|
||||
describe('OnboardingFlow hands the consent checkbox its state', () => {
|
||||
it('routes an uncheck through the flow into the settings write', async () => {
|
||||
render(<OnboardingFlow onboarding={freshOnboarding()} onOnboardingChange={vi.fn()} />)
|
||||
const checkbox = screen.getByRole('checkbox', { name: CHECKBOX_LABEL })
|
||||
expect(checkbox).toBeChecked()
|
||||
|
||||
await userEvent.click(checkbox)
|
||||
|
||||
// Without the value+handler props the box is a permanently checked decoration.
|
||||
expect(updateSettingsMock).toHaveBeenCalledWith({ agentStatusHooksEnabled: false })
|
||||
expect(screen.getByRole('checkbox', { name: CHECKBOX_LABEL })).not.toBeChecked()
|
||||
})
|
||||
|
||||
it('tells the disclosure which agents the user has already turned off', async () => {
|
||||
render(<OnboardingFlow onboarding={freshOnboarding()} onOnboardingChange={vi.fn()} />)
|
||||
|
||||
await userEvent.click(screen.getByText('What Orca changes, and when'))
|
||||
|
||||
// Cursor is detected but disabled: listing it would promise a write Orca will not make.
|
||||
const rows = within(screen.getByRole('list')).getAllByRole('listitem')
|
||||
expect(rows).toHaveLength(1)
|
||||
expect(rows[0]).toHaveTextContent('Claude')
|
||||
})
|
||||
})
|
||||
|
||||
describe('the consent carried by a dismiss', () => {
|
||||
it('sends the box state at dismiss time, not the one it was first rendered with', async () => {
|
||||
const { result } = renderHook(() => useOnboardingFlow(freshOnboarding(), vi.fn()))
|
||||
|
||||
act(() => result.current.setAgentStatusHooksEnabled(false))
|
||||
await act(async () => {
|
||||
await result.current.dismissOnboarding('keyboard')
|
||||
})
|
||||
|
||||
const update = onboardingUpdateMock()
|
||||
expect(update).toHaveBeenCalledTimes(1)
|
||||
expect(update.mock.calls[0][1]).toEqual({ agentStatusHooksEnabled: false })
|
||||
})
|
||||
})
|
||||
@@ -13572,7 +13572,7 @@
|
||||
"affectedLabel": "Affected",
|
||||
"affectedDetecting": "Still checking which agent CLIs are on your PATH…",
|
||||
"affectedNone": "No agent CLIs found on your PATH, so nothing would be written right now.",
|
||||
"affectedScriptNote": "Plus a small script in ~/.orca/agent-hooks/. Status is reported to Orca on your machine, it isn't uploaded anywhere.",
|
||||
"affectedScriptNote": "Plus a small script in ~/.orca/agent-hooks/. Status goes to Orca on this machine and to the devices you've paired with it.",
|
||||
"affectedApproximate": "This list is approximate — Orca checks your PATH again when it installs.",
|
||||
"yourHooksLabel": "Your hooks",
|
||||
"yourHooksBody": "Orca only adds or removes entries that point at its own script, hooks you wrote are left alone. These live in your agent's own config, so they run in every session, not just ones started from Orca. When Orca isn't running the hook exits immediately and can never block or deny a tool call.",
|
||||
|
||||
Reference in New Issue
Block a user