diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 37949d578ec..62c9f1987f7 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -6170,6 +6170,61 @@ describe('OrcaRuntimeService', () => { ) }) + describe('checkRepoHooks status', () => { + const remoteStore = { + ...store, + getRepos: () => [ + { + id: TEST_REPO_ID, + path: '/remote/repo', + displayName: 'repo', + badgeColor: 'blue', + addedAt: 1, + connectionId: 'ssh-1' + } + ] + } + + it('reports an error when the SSH filesystem provider is unavailable', async () => { + const runtime = new OrcaRuntimeService(remoteStore as never) + + await expect(runtime.checkRepoHooks('id:repo-1')).resolves.toEqual({ + status: 'error', + hasHooks: false, + hooks: null, + mayNeedUpdate: false + }) + }) + + it('reports ok for a missing remote orca.yaml and error for any other read failure', async () => { + const readFile = vi.fn() + registerSshFilesystemProvider('ssh-1', { readFile } as never) + const runtime = new OrcaRuntimeService(remoteStore as never) + + try { + readFile.mockRejectedValueOnce(Object.assign(new Error('missing'), { code: 'ENOENT' })) + await expect(runtime.checkRepoHooks('id:repo-1')).resolves.toMatchObject({ + status: 'ok', + hasHooks: false + }) + + readFile.mockRejectedValueOnce(Object.assign(new Error('down'), { code: 'ECONNRESET' })) + await expect(runtime.checkRepoHooks('id:repo-1')).resolves.toMatchObject({ + status: 'error', + hasHooks: false + }) + } finally { + unregisterSshFilesystemProvider('ssh-1') + } + }) + + it('reports ok for a local repo hook check', async () => { + const runtime = new OrcaRuntimeService(store as never) + + await expect(runtime.checkRepoHooks('id:repo-1')).resolves.toMatchObject({ status: 'ok' }) + }) + }) + it('resolves SSH issue commands from shared orca.yaml and deletes empty overrides', async () => { const remoteStore = { ...store, diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 731b997c44b..bc72387da21 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -19889,28 +19889,42 @@ export class OrcaRuntimeService { async checkRepoHooks(repoSelector: string) { const repo = await this.resolveRepoSelector(repoSelector) if (isFolderRepo(repo)) { - return { hasHooks: false, hooks: null, mayNeedUpdate: false } + return { status: 'ok' as const, hasHooks: false, hooks: null, mayNeedUpdate: false } } if (repo.connectionId) { const fsProvider = getSshFilesystemProvider(repo.connectionId) + // Why: callers cache "no hooks" as authoritative, so an unreadable repo must fail + // closed with an error status (mirrors the hooks:check IPC handler) instead of + // pinning a false "no setup script" verdict until the client remounts. if (!fsProvider) { - return { hasHooks: false, hooks: null, mayNeedUpdate: false } + return { status: 'error' as const, hasHooks: false, hooks: null, mayNeedUpdate: false } } try { const result = await fsProvider.readFile(joinWorktreeRelativePath(repo.path, 'orca.yaml')) if (result.isBinary) { - return { hasHooks: false, hooks: null, mayNeedUpdate: false } + return { status: 'ok' as const, hasHooks: false, hooks: null, mayNeedUpdate: false } + } + return { + status: 'ok' as const, + hasHooks: true, + hooks: parseOrcaYaml(result.content), + mayNeedUpdate: false + } + } catch (error) { + return { + status: isENOENT(error) ? ('ok' as const) : ('error' as const), + hasHooks: false, + hooks: null, + mayNeedUpdate: false } - return { hasHooks: true, hooks: parseOrcaYaml(result.content), mayNeedUpdate: false } - } catch { - return { hasHooks: false, hooks: null, mayNeedUpdate: false } } } const has = hasHooksFile(repo.path) const hooks = has ? loadHooks(repo.path) : null return { + status: 'ok' as const, hasHooks: has, hooks, mayNeedUpdate: has && !hooks && hasUnrecognizedOrcaYamlKeys(repo.path) diff --git a/src/renderer/src/components/sidebar/useSetupScriptPromptRevalidation.test.tsx b/src/renderer/src/components/sidebar/useSetupScriptPromptRevalidation.test.tsx index 6ea400b5bd8..cb4e364d1b2 100644 --- a/src/renderer/src/components/sidebar/useSetupScriptPromptRevalidation.test.tsx +++ b/src/renderer/src/components/sidebar/useSetupScriptPromptRevalidation.test.tsx @@ -27,6 +27,31 @@ function effectiveSetup(repoId: string): OkSetupScriptPromptState { return { ...missingSetup(repoId), hasEffectiveSetup: true } } +function inspectionError(repoId: string, hostId = 'local'): SetupScriptPromptState { + return { + status: 'error', + repoId, + repoHostIdentity: getRepoHostIdentityForParts(repoId, hostId) + } +} + +const RUNTIME_ENVIRONMENT_ID = 'env-1' +const RUNTIME_REPO = { + id: 'repo-1', + kind: 'git', + executionHostId: `runtime:${RUNTIME_ENVIRONMENT_ID}` +} as unknown as Repo + +async function setRuntimeConnectionGeneration(connectionGeneration: number): Promise { + await act(async () => { + useAppStore.setState({ + runtimeStatusByEnvironmentId: new Map([ + [RUNTIME_ENVIRONMENT_ID, { status: null, checkedAt: 0, connectionGeneration }] + ]) + }) + }) +} + type HarnessProps = { activeRepo: Repo | null isDismissed: boolean @@ -77,7 +102,7 @@ describe('useSetupScriptPromptRevalidation', () => { afterEach(() => { roots.splice(0).forEach((root) => act(() => root.unmount())) document.body.replaceChildren() - useAppStore.setState({ activeWorktreeId: null }) + useAppStore.setState({ activeWorktreeId: null, runtimeStatusByEnvironmentId: new Map() }) vi.clearAllMocks() }) @@ -188,4 +213,84 @@ describe('useSetupScriptPromptRevalidation', () => { expect(requestRevalidation).not.toHaveBeenCalled() }) + + it('re-inspects on window focus after a failed inspection', async () => { + const requestRevalidation = vi.fn() + await render({ + activeRepo: GIT_REPO, + isDismissed: false, + sidebarOpen: true, + promptState: inspectionError('repo-1'), + requestRevalidation + }) + + await dispatchWindowFocus() + + expect(requestRevalidation).toHaveBeenCalledTimes(1) + }) + + it('replays a worktree activation that landed while the prompt state was unsettled', async () => { + const requestRevalidation = vi.fn() + const rerender = await render({ + activeRepo: GIT_REPO, + isDismissed: false, + sidebarOpen: true, + promptState: missingSetup('repo-1'), + requestRevalidation + }) + // The card nulls its state for the whole inspection round trip, so the + // activation lands while nothing is revalidatable. + await rerender({ + activeRepo: GIT_REPO, + isDismissed: false, + sidebarOpen: true, + promptState: null, + requestRevalidation + }) + await setActiveWorktree('worktree-2') + expect(requestRevalidation).not.toHaveBeenCalled() + + await rerender({ + activeRepo: GIT_REPO, + isDismissed: false, + sidebarOpen: true, + promptState: missingSetup('repo-1'), + requestRevalidation + }) + + expect(requestRevalidation).toHaveBeenCalledTimes(1) + }) + + it('re-inspects when the repo runtime reconnects', async () => { + const requestRevalidation = vi.fn() + await setRuntimeConnectionGeneration(1) + await render({ + activeRepo: RUNTIME_REPO, + isDismissed: false, + sidebarOpen: true, + promptState: missingSetup('repo-1', `runtime:${RUNTIME_ENVIRONMENT_ID}`), + requestRevalidation + }) + expect(requestRevalidation).not.toHaveBeenCalled() + + await setRuntimeConnectionGeneration(2) + + expect(requestRevalidation).toHaveBeenCalledTimes(1) + }) + + it('ignores a reconnect of a runtime that does not own the repo', async () => { + const requestRevalidation = vi.fn() + await setRuntimeConnectionGeneration(1) + await render({ + activeRepo: GIT_REPO, + isDismissed: false, + sidebarOpen: true, + promptState: missingSetup('repo-1'), + requestRevalidation + }) + + await setRuntimeConnectionGeneration(2) + + expect(requestRevalidation).not.toHaveBeenCalled() + }) }) diff --git a/src/renderer/src/components/sidebar/useSetupScriptPromptRevalidation.ts b/src/renderer/src/components/sidebar/useSetupScriptPromptRevalidation.ts index 04f9dbcab90..ce6a5e78fce 100644 --- a/src/renderer/src/components/sidebar/useSetupScriptPromptRevalidation.ts +++ b/src/renderer/src/components/sidebar/useSetupScriptPromptRevalidation.ts @@ -3,6 +3,7 @@ import { useAppStore } from '@/store' import { isGitRepoKind } from '../../../../shared/repo-kind' import type { Repo } from '../../../../shared/types' import { getRepoHostIdentity } from '@/store/slices/repo-host-identity' +import { getRepoExecutionHostId, parseExecutionHostId } from '../../../../shared/execution-host' import type { SetupScriptPromptState } from './setup-script-prompt-render-state' /** @@ -19,15 +20,27 @@ export function useSetupScriptPromptRevalidation(input: { }): void { const { activeRepo, isDismissed, sidebarOpen, promptState, requestRevalidation } = input const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) + const activeRepoHostIdentity = activeRepo ? getRepoHostIdentity(activeRepo) : null + const repoHost = parseExecutionHostId(activeRepo ? getRepoExecutionHostId(activeRepo) : null) + const repoRuntimeEnvironmentId = repoHost?.kind === 'runtime' ? repoHost.environmentId : null + // Why: scope to the repo's own runtime so an unrelated host reconnect does not + // re-fire inspections for every repo. + const repoConnectionGeneration = useAppStore((s) => + repoRuntimeEnvironmentId + ? (s.runtimeStatusByEnvironmentId.get(repoRuntimeEnvironmentId)?.connectionGeneration ?? 0) + : 0 + ) - // Why: only revalidate while the prompt still shows no effective setup — there is - // nothing to clear (and no RPC worth spending, notably over SSH) once it is - // configured. - const showsMissingSetup = - promptState?.status === 'ok' && - promptState.repoId === activeRepo?.id && - Boolean(activeRepo && promptState.repoHostIdentity === getRepoHostIdentity(activeRepo)) && - !promptState.hasEffectiveSetup + // Why: revalidate while the prompt shows no effective setup or a failed inspection — + // both can be stale. `forbidden` is permanent and an effective setup has nothing to + // clear, so neither is worth an RPC (notably over SSH). + const promptBelongsToActiveRepo = + promptState?.repoId === activeRepo?.id && + Boolean(activeRepo && promptState?.repoHostIdentity === activeRepoHostIdentity) + const promptNeedsRevalidation = + promptBelongsToActiveRepo && + (promptState?.status === 'error' || + (promptState?.status === 'ok' && !promptState.hasEffectiveSetup)) // Why: orca.yaml is edited on disk or the hook runs in a terminal outside React // state. Re-inspect on window focus so returning to Orca detects it (mirrors @@ -38,7 +51,7 @@ export function useSetupScriptPromptRevalidation(input: { !activeRepo || !isGitRepoKind(activeRepo) || isDismissed || - !showsMissingSetup + !promptNeedsRevalidation ) { return } @@ -46,18 +59,56 @@ export function useSetupScriptPromptRevalidation(input: { return () => { window.removeEventListener('focus', requestRevalidation) } - }, [activeRepo, isDismissed, requestRevalidation, showsMissingSetup, sidebarOpen]) + }, [activeRepo, isDismissed, requestRevalidation, promptNeedsRevalidation, sidebarOpen]) // Why: the setup hook runs during worktree creation, so activating a worktree in // this repo can make the setup effective after a negative result was cached. Fire // only on an actual activation change, not on mount/remount with a seeded id — // the initial inspection already covers the mounted worktree. const previousWorktreeIdRef = useRef(activeWorktreeId) + const previousRepoHostIdentityRef = useRef(activeRepoHostIdentity) + const previousConnectionGenerationRef = useRef(repoConnectionGeneration) + // Why: these signals usually land while the prompt state is still unsettled (the + // card nulls it for the whole inspection round trip). Remember them instead of + // dropping them, then replay once a revalidatable result exists. + const pendingRevalidationRef = useRef(false) + useEffect(() => { - const changed = previousWorktreeIdRef.current !== activeWorktreeId + const worktreeChanged = previousWorktreeIdRef.current !== activeWorktreeId + const hostChanged = previousRepoHostIdentityRef.current !== activeRepoHostIdentity + // Why: a runtime reconnect can turn an unreadable orca.yaml into a readable one, + // and nothing else in the card's inputs changes when that happens. + const reconnected = repoConnectionGeneration > previousConnectionGenerationRef.current previousWorktreeIdRef.current = activeWorktreeId - if (changed && showsMissingSetup) { - requestRevalidation() + previousRepoHostIdentityRef.current = activeRepoHostIdentity + previousConnectionGenerationRef.current = repoConnectionGeneration + if (hostChanged) { + // A repo/host switch re-runs the card's own inspection, so no extra pass is owed. + pendingRevalidationRef.current = false + } else if (worktreeChanged || reconnected) { + pendingRevalidationRef.current = true } - }, [activeWorktreeId, requestRevalidation, showsMissingSetup]) + + if ( + !pendingRevalidationRef.current || + !sidebarOpen || + !activeRepo || + !isGitRepoKind(activeRepo) || + isDismissed || + !promptNeedsRevalidation + ) { + return + } + pendingRevalidationRef.current = false + requestRevalidation() + }, [ + activeRepo, + activeRepoHostIdentity, + activeWorktreeId, + isDismissed, + promptNeedsRevalidation, + repoConnectionGeneration, + requestRevalidation, + sidebarOpen + ]) } diff --git a/tests/e2e/setup-script-prompt-unreadable-orca-yaml.spec.ts b/tests/e2e/setup-script-prompt-unreadable-orca-yaml.spec.ts new file mode 100644 index 00000000000..5cc07ef0e5b --- /dev/null +++ b/tests/e2e/setup-script-prompt-unreadable-orca-yaml.spec.ts @@ -0,0 +1,202 @@ +import { execFileSync } from 'node:child_process' +import { mkdirSync, rmSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import type { ElectronApplication, Page } from '@stablyai/playwright-test' +import { test, expect } from './helpers/orca-app' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { worktreeRow, worktreeRowSurface } from './worktree-row-locators' + +const INSPECTION_ERROR_TEXT = "Couldn't verify this repo's setup script right now." +const SETUP_SCRIPT_COMMAND = 'echo orca-e2e-setup' +const RECORDING_DWELL_MS = 1200 + +type WorktreeIds = { + repoId: string + mainWorktreeId: string + featureWorktreeId: string +} + +function runGit(cwd: string, args: string[]): void { + execFileSync('git', args, { cwd, stdio: 'pipe' }) +} + +/** Repo whose shared orca.yaml carries a real setup script, plus a second worktree. */ +function createRepoWithSharedSetupScript(repoPath: string, featureWorktreePath: string): void { + rmSync(repoPath, { recursive: true, force: true }) + rmSync(featureWorktreePath, { recursive: true, force: true }) + mkdirSync(repoPath, { recursive: true }) + runGit(repoPath, ['init']) + runGit(repoPath, ['config', 'user.email', 'e2e@test.local']) + runGit(repoPath, ['config', 'user.name', 'E2E Test']) + writeFileSync(path.join(repoPath, 'README.md'), '# Unreadable orca.yaml E2E\n') + writeFileSync(path.join(repoPath, 'orca.yaml'), `scripts:\n setup: ${SETUP_SCRIPT_COMMAND}\n`) + runGit(repoPath, ['add', '-A']) + runGit(repoPath, ['commit', '-m', 'Initial commit']) + runGit(repoPath, ['worktree', 'add', '-b', 'setup-prompt-proof', featureWorktreePath]) +} + +/** + * Makes the main process report the failure the fix now surfaces: orca.yaml could + * not be read (SSH filesystem provider gone), so the hook check fails closed with + * `status: 'error'` instead of an authoritative "no setup script". + * The real handler stays captured so healing restores production behavior. + */ +async function installUnreadableOrcaYamlFault(electronApp: ElectronApplication): Promise { + await electronApp.evaluate(({ ipcMain }) => { + type InvokeHandler = (event: unknown, ...args: unknown[]) => unknown + const faultState = globalThis as typeof globalThis & { __orcaE2eOrcaYamlUnreadable?: boolean } + const registry = (ipcMain as unknown as { _invokeHandlers?: Map }) + ._invokeHandlers + const productionHandler = registry?.get('hooks:check') + if (!productionHandler) { + throw new Error('hooks:check handler was not registered in the main process') + } + faultState.__orcaE2eOrcaYamlUnreadable = true + ipcMain.removeHandler('hooks:check') + ipcMain.handle('hooks:check', async (event, ...args) => { + if (faultState.__orcaE2eOrcaYamlUnreadable) { + return { status: 'error', hasHooks: false, hooks: null, mayNeedUpdate: false } + } + return productionHandler(event, ...args) + }) + }) +} + +/** orca.yaml becomes readable again — every later check runs the production handler. */ +async function healOrcaYamlRead(electronApp: ElectronApplication): Promise { + await electronApp.evaluate(() => { + ;(globalThis as typeof globalThis & { __orcaE2eOrcaYamlUnreadable?: boolean }) + .__orcaE2eOrcaYamlUnreadable = false + }) +} + +async function addRepoAndActivateMainWorktree( + page: Page, + repoPath: string, + featureWorktreePath: string +): Promise { + // Why: repos hide externally created worktrees by default, so the second + // worktree only reaches the sidebar once the repo opts into showing them. + const repoId = await page.evaluate(async (targetRepoPath) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + const addedRepo = await store.getState().addRepoPath(targetRepoPath) + if (!addedRepo) { + throw new Error(`Failed to add repo at ${targetRepoPath}`) + } + await store.getState().updateRepo(addedRepo.id, { externalWorktreeVisibility: 'show' }) + return addedRepo.id + }, repoPath) + + await expect + .poll( + () => + page.evaluate(async (targetRepoId) => { + const store = window.__store + if (!store) { + return 0 + } + await store.getState().fetchWorktrees(targetRepoId) + return store.getState().worktreesByRepo[targetRepoId]?.length ?? 0 + }, repoId), + { timeout: 20_000, message: 'proof repo worktrees did not load' } + ) + .toBeGreaterThanOrEqual(2) + + return page.evaluate( + ({ targetRepoId, targetRepoPath, targetFeaturePath }) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + const normalize = (value: string): string => + value.startsWith('/private/var/') ? value.slice('/private'.length) : value + + const state = store.getState() + const worktrees = state.worktreesByRepo[targetRepoId] ?? [] + const mainWorktree = worktrees.find( + (entry) => normalize(entry.path) === normalize(targetRepoPath) + ) + const featureWorktree = worktrees.find( + (entry) => normalize(entry.path) === normalize(targetFeaturePath) + ) + if (!mainWorktree || !featureWorktree) { + throw new Error( + `Missing worktrees for ${targetRepoPath}: ${worktrees.map((entry) => entry.path).join(', ')}` + ) + } + + state.setSidebarOpen(true) + state.setGroupBy('none') + state.setSortBy('recent') + state.setShowActiveOnly(false) + state.setShowSleepingWorkspaces(true) + state.setHideDefaultBranchWorkspace(false) + state.setFilterRepoIds([]) + state.setActiveRepo(targetRepoId) + state.setActiveWorktree(mainWorktree.id) + state.revealWorktreeInSidebar(featureWorktree.id, { behavior: 'auto' }) + return { + repoId: targetRepoId, + mainWorktreeId: mainWorktree.id, + featureWorktreeId: featureWorktree.id + } + }, + { targetRepoId: repoId, targetRepoPath: repoPath, targetFeaturePath: featureWorktreePath } + ) +} + +test.describe('Setup script prompt', () => { + test.beforeEach(async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + }) + + test('recovers from an unreadable orca.yaml instead of pinning the failed verdict', async ({ + electronApp, + orcaPage + }, testInfo) => { + const repoPath = testInfo.outputPath('unreadable-orca-yaml-repo') + const featureWorktreePath = testInfo.outputPath('unreadable-orca-yaml-feature') + createRepoWithSharedSetupScript(repoPath, featureWorktreePath) + + await installUnreadableOrcaYamlFault(electronApp) + const { repoId, featureWorktreeId } = await addRepoAndActivateMainWorktree( + orcaPage, + repoPath, + featureWorktreePath + ) + + const promptCard = orcaPage.locator('[data-setup-script-prompt-layer]') + const inspectionError = promptCard.getByText(INSPECTION_ERROR_TEXT) + await expect(inspectionError).toBeVisible({ timeout: 20_000 }) + + // orca.yaml is readable again; the card is still pinned to the failed verdict. + await healOrcaYamlRead(electronApp) + const healthyCheck = await orcaPage.evaluate( + (targetRepoId) => window.api.hooks.check({ repoId: targetRepoId }), + repoId + ) + expect(healthyCheck.status).toBe('ok') + expect((healthyCheck.hooks as { scripts?: { setup?: string } } | null)?.scripts?.setup).toBe( + SETUP_SCRIPT_COMMAND + ) + await expect(inspectionError).toBeVisible() + await expect(promptCard.getByRole('button', { name: 'Retry' })).toBeVisible() + // Not a wait for state: holds the pinned card on screen for the proof recording. + await orcaPage.waitForTimeout(RECORDING_DWELL_MS) + + // Activating another worktree in the same repo must re-inspect. + const featureRow = worktreeRow(orcaPage, featureWorktreeId) + await expect(featureRow).toBeVisible() + await worktreeRowSurface(orcaPage, featureWorktreeId).click() + await expect(featureRow).toHaveAttribute('aria-current', 'page') + + // The repo has a valid orca.yaml scripts.setup, so no prompt may remain. + await expect(inspectionError).toBeHidden({ timeout: 20_000 }) + await expect(promptCard).toHaveCount(0) + await orcaPage.waitForTimeout(RECORDING_DWELL_MS) + }) +})