fix(setup): stop caching an unreadable orca.yaml as "no setup script" (#12469)

* fix(setup): stop caching an unreadable orca.yaml as "no setup script"

`checkRepoHooks` returned `{hasHooks:false, hooks:null, mayNeedUpdate:false}` with no `status` field when the SSH filesystem provider was unavailable, and inside a blanket catch for any read error. The renderer only bails on `status === 'error'`, so that status-less false negative was cached as an authoritative "no setup script" and the prompt stayed on screen.

Mirror the `hooks:check` IPC twin exactly: `status:'error'` for a missing provider, ENOENT-aware in the catch, `status:'ok'` on the folder-repo, binary, SSH-success and local branches.

Fixes #8752

Co-authored-by: Orca <help@stably.ai>

* test(e2e): add recordable proof for setup-script-prompt-false-negative

Fails on origin/main, passes on this branch.

Test: recovers from an unreadable orca.yaml instead of pinning the failed verdict

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
2026-08-04 17:06:50 -07:00
committed by GitHub
co-authored by Orca
parent aa7e76ba61
commit 7956335cea
5 changed files with 448 additions and 21 deletions
+55
View File
@@ -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,
+20 -6
View File
@@ -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)
@@ -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<void> {
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()
})
})
@@ -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
])
}
@@ -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<void> {
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<string, InvokeHandler> })
._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<void> {
await electronApp.evaluate(() => {
;(globalThis as typeof globalThis & { __orcaE2eOrcaYamlUnreadable?: boolean })
.__orcaE2eOrcaYamlUnreadable = false
})
}
async function addRepoAndActivateMainWorktree(
page: Page,
repoPath: string,
featureWorktreePath: string
): Promise<WorktreeIds> {
// 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)
})
})