diff --git a/config/scripts/pnpm-cli-spawn.mjs b/config/scripts/pnpm-cli-spawn.mjs new file mode 100644 index 000000000000..f1e847a36982 --- /dev/null +++ b/config/scripts/pnpm-cli-spawn.mjs @@ -0,0 +1,16 @@ +export function resolvePnpmCliSpawn( + args, + { + platform = process.platform, + nodePath = process.execPath, + npmExecPath = process.env.npm_execpath + } = {} +) { + if (platform !== 'win32') { + return { command: 'pnpm', args } + } + if (!npmExecPath) { + throw new Error('Run this command through pnpm so its Windows CLI path is available.') + } + return { command: nodePath, args: [npmExecPath, ...args] } +} diff --git a/config/scripts/pnpm-cli-spawn.test.mjs b/config/scripts/pnpm-cli-spawn.test.mjs new file mode 100644 index 000000000000..18d1eace838c --- /dev/null +++ b/config/scripts/pnpm-cli-spawn.test.mjs @@ -0,0 +1,34 @@ +import { describe, expect, it } from 'vitest' +import { resolvePnpmCliSpawn } from './pnpm-cli-spawn.mjs' + +describe('resolvePnpmCliSpawn', () => { + it('uses the executable directly on POSIX', () => { + expect(resolvePnpmCliSpawn(['run', 'typecheck'], { platform: 'linux' })).toEqual({ + command: 'pnpm', + args: ['run', 'typecheck'] + }) + }) + + it('preserves whitespace arguments without a Windows command shell', () => { + expect( + resolvePnpmCliSpawn(['exec', 'playwright', '--grep', 'legacy SSH fork'], { + platform: 'win32', + nodePath: 'C:\\Program Files\\nodejs\\node.exe', + npmExecPath: 'C:\\pnpm\\pnpm.cjs' + }) + ).toEqual({ + command: 'C:\\Program Files\\nodejs\\node.exe', + args: ['C:\\pnpm\\pnpm.cjs', 'exec', 'playwright', '--grep', 'legacy SSH fork'] + }) + }) + + it('requires pnpm package-script context on Windows', () => { + expect(() => + resolvePnpmCliSpawn([], { + platform: 'win32', + nodePath: 'node.exe', + npmExecPath: undefined + }) + ).toThrow('Run this command through pnpm') + }) +}) diff --git a/config/scripts/run-ssh-docker-fork-upstream-backfill-e2e.mjs b/config/scripts/run-ssh-docker-fork-upstream-backfill-e2e.mjs new file mode 100644 index 000000000000..a174cf1b1b0f --- /dev/null +++ b/config/scripts/run-ssh-docker-fork-upstream-backfill-e2e.mjs @@ -0,0 +1,40 @@ +import { spawnSync } from 'node:child_process' +import { resolvePnpmCliSpawn } from './pnpm-cli-spawn.mjs' + +const rawExtraArgs = process.argv.slice(2) +const extraArgs = rawExtraArgs[0] === '--' ? rawExtraArgs.slice(1) : rawExtraArgs +const env = { + ...process.env, + ORCA_E2E_SSH_DOCKER: '1' +} + +const spawnOptions = { + stdio: 'inherit', + env +} + +function runPnpm(args) { + const invocation = resolvePnpmCliSpawn(args) + return spawnSync(invocation.command, invocation.args, spawnOptions) +} + +const runtime = runPnpm(['run', 'ensure:electron-runtime']) + +if (runtime.status !== 0) { + process.exit(runtime.status ?? 1) +} + +const result = runPnpm([ + 'exec', + 'playwright', + 'test', + 'tests/e2e/ssh-docker-fork-upstream-backfill.spec.ts', + '--config', + 'tests/playwright.config.ts', + '--project', + 'electron-headless', + '--workers=1', + ...extraArgs +]) + +process.exit(result.status ?? 1) diff --git a/package.json b/package.json index 06407e811446..d1ed1261d7a2 100644 --- a/package.json +++ b/package.json @@ -96,6 +96,7 @@ "test:e2e:terminal-perf:html-report": "node config/scripts/generate-terminal-perf-html-report.mjs", "test:e2e:ssh-docker-perf": "node config/scripts/run-ssh-docker-perf-e2e.mjs", "test:e2e:ssh-docker-watcher-isolation": "node config/scripts/run-ssh-docker-watcher-isolation-e2e.mjs", + "test:e2e:ssh-docker-fork-upstream-backfill": "node config/scripts/run-ssh-docker-fork-upstream-backfill-e2e.mjs", "test:e2e:ssh-docker-terminal-parking": "node config/scripts/run-ssh-docker-terminal-parking-e2e.mjs", "test:e2e:nested-runtime-ssh": "node config/scripts/run-nested-runtime-ssh-e2e.mjs", "test:e2e:source-control-scale": "pnpm run ensure:electron-runtime && npx playwright test tests/e2e/source-control-large-file-count.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1", diff --git a/src/main/runtime/orca-runtime-fork-upstream-ssh-backfill.test.ts b/src/main/runtime/orca-runtime-fork-upstream-ssh-backfill.test.ts new file mode 100644 index 000000000000..a93fa929d690 --- /dev/null +++ b/src/main/runtime/orca-runtime-fork-upstream-ssh-backfill.test.ts @@ -0,0 +1,357 @@ +// Legacy SSH forks backfill only after a ready provider publishes connected (#12967). +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../shared/types' +import type { SshConnectionState } from '../../shared/ssh-types' +import { OrcaRuntimeService } from './orca-runtime' +import { getRepoUpstream } from '../github/client' +import { + getSshGitProvider, + registerSshGitProvider, + unregisterSshGitProvider +} from '../providers/ssh-git-dispatch' + +const getRepoUpstreamMock = vi.hoisted(() => vi.fn()) + +vi.mock('../github/client', async (importOriginal) => ({ + ...((await importOriginal()) as Record), + getRepoUpstream: getRepoUpstreamMock +})) + +const UPSTREAM = { owner: 'stablyai', repo: 'orca' } +const registeredConnectionIds = new Set() + +function makeRepo(overrides: Partial): Repo { + return { + id: 'repo', + path: '/srv/orca', + displayName: 'orca', + badgeColor: '#000', + addedAt: 1, + kind: 'git', + ...overrides + } as Repo +} + +function createRuntime(repos: Repo[]) { + const updateRepo = vi.fn((id: string, updates: Partial) => { + const repo = repos.find((entry) => entry.id === id) + if (!repo) { + return null + } + Object.assign(repo, updates) + return repo + }) + const runtime = new OrcaRuntimeService({ + getRepos: () => [...repos], + getRepo: (id: string) => repos.find((repo) => repo.id === id) ?? null, + updateRepo, + getAllWorktreeMeta: () => ({}), + getWorktreeMeta: () => null, + setWorktreeMeta: vi.fn(), + removeWorktreeMeta: vi.fn(), + getGitHubCache: () => null, + getSettings: () => ({}) + } as never) + return { runtime, repos, updateRepo } +} + +function sshState(targetId: string, status: SshConnectionState['status']): SshConnectionState { + return { targetId, status, error: null, reconnectAttempt: 0 } +} + +function notifyConnected(runtime: OrcaRuntimeService, targetId: string): void { + registeredConnectionIds.add(targetId) + if (!getSshGitProvider(targetId)) { + registerSshGitProvider(targetId, {} as never) + } + runtime.notifySshStateChanged(targetId, sshState(targetId, 'connected')) +} + +function notifyDisconnected(runtime: OrcaRuntimeService, targetId: string): void { + unregisterSshGitProvider(targetId) + runtime.notifySshStateChanged(targetId, sshState(targetId, 'disconnected')) +} + +// Why: fire-and-forget production work exposes no handle, so tests flush its microtask queue. +async function drainBackfill(): Promise { + for (let tick = 0; tick < 20; tick += 1) { + await Promise.resolve() + } +} + +beforeEach(() => { + getRepoUpstreamMock.mockReset() + getRepoUpstreamMock.mockResolvedValue(null) +}) + +afterEach(() => { + for (const connectionId of registeredConnectionIds) { + unregisterSshGitProvider(connectionId) + } + registeredConnectionIds.clear() +}) + +describe('fork upstream backfill for SSH repos', () => { + it('resolves the upstream when the connection comes up', async () => { + getRepoUpstreamMock.mockResolvedValue(UPSTREAM) + const { runtime, repos } = createRuntime([makeRepo({ id: 'ssh-fork', connectionId: 'ssh-1' })]) + + notifyConnected(runtime, 'ssh-1') + await drainBackfill() + + expect(getRepoUpstream).toHaveBeenCalledWith('/srv/orca', 'ssh-1') + expect(repos[0].upstream).toEqual(UPSTREAM) + }) + + it('migrates the auto-detected origin avatar to the upstream, like the local pass', async () => { + getRepoUpstreamMock.mockResolvedValue(UPSTREAM) + const { runtime, repos } = createRuntime([ + makeRepo({ + id: 'ssh-fork', + connectionId: 'ssh-1', + repoIcon: { type: 'image', src: 'https://avatars/fork.png', source: 'github' } + }), + makeRepo({ + id: 'ssh-chosen', + path: '/srv/other', + connectionId: 'ssh-1', + repoIcon: { type: 'emoji', emoji: '🦈' } + }) + ]) + + notifyConnected(runtime, 'ssh-1') + await drainBackfill() + + expect(repos[0].repoIcon).toEqual({ + type: 'image', + src: 'https://github.com/stablyai.png?size=64', + source: 'github', + label: 'stablyai/orca' + }) + expect(repos[1].repoIcon).toEqual({ type: 'emoji', emoji: '🦈' }) + }) + + it('skips folder repos, other connections, and already-resolved repos', async () => { + const { runtime } = createRuntime([ + makeRepo({ id: 'folder', connectionId: 'ssh-1', kind: 'folder' }), + makeRepo({ id: 'other-host', path: '/srv/b', connectionId: 'ssh-2' }), + makeRepo({ id: 'local', path: '/home/a' }), + makeRepo({ id: 'resolved', path: '/srv/c', connectionId: 'ssh-1', upstream: null }) + ]) + + notifyConnected(runtime, 'ssh-1') + await drainBackfill() + + expect(getRepoUpstream).not.toHaveBeenCalled() + }) + + it('leaves best-effort null unresolved without retrying every reconnect', async () => { + const { runtime, repos, updateRepo } = createRuntime([ + makeRepo({ id: 'ssh-fork', connectionId: 'ssh-1' }) + ]) + + notifyConnected(runtime, 'ssh-1') + runtime.notifySshStateChanged('ssh-1', sshState('ssh-1', 'connected')) + await drainBackfill() + notifyDisconnected(runtime, 'ssh-1') + notifyConnected(runtime, 'ssh-1') + await drainBackfill() + + expect(getRepoUpstream).toHaveBeenCalledTimes(1) + expect(repos[0].upstream).toBeUndefined() + expect(updateRepo).not.toHaveBeenCalled() + }) + + it('retries on the next connect when the probe threw', async () => { + getRepoUpstreamMock.mockRejectedValueOnce(new Error('ssh channel closed')) + const { runtime, repos } = createRuntime([makeRepo({ id: 'ssh-fork', connectionId: 'ssh-1' })]) + + notifyConnected(runtime, 'ssh-1') + await drainBackfill() + expect(repos[0].upstream).toBeUndefined() + + getRepoUpstreamMock.mockResolvedValue(UPSTREAM) + notifyDisconnected(runtime, 'ssh-1') + notifyConnected(runtime, 'ssh-1') + await drainBackfill() + + expect(repos[0].upstream).toEqual(UPSTREAM) + }) + + it('retries a newer provider generation when reconnect wins the failure race', async () => { + let rejectProbe: ((error: Error) => void) | undefined + getRepoUpstreamMock + .mockImplementationOnce( + () => + new Promise((_, reject) => { + rejectProbe = reject + }) + ) + .mockResolvedValueOnce(UPSTREAM) + const { runtime, repos } = createRuntime([makeRepo({ id: 'ssh-fork', connectionId: 'ssh-1' })]) + + notifyConnected(runtime, 'ssh-1') + await drainBackfill() + notifyDisconnected(runtime, 'ssh-1') + notifyConnected(runtime, 'ssh-1') + rejectProbe?.(new Error('old provider disconnected')) + await drainBackfill() + + expect(getRepoUpstream).toHaveBeenCalledTimes(2) + expect(repos[0].upstream).toEqual(UPSTREAM) + }) + + it('discards a stale null returned after the provider changed', async () => { + let resolveProbe: ((value: null) => void) | undefined + getRepoUpstreamMock + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveProbe = resolve + }) + ) + .mockResolvedValueOnce(UPSTREAM) + const { runtime, repos } = createRuntime([makeRepo({ id: 'ssh-fork', connectionId: 'ssh-1' })]) + + notifyConnected(runtime, 'ssh-1') + await drainBackfill() + notifyDisconnected(runtime, 'ssh-1') + notifyConnected(runtime, 'ssh-1') + resolveProbe?.(null) + await drainBackfill() + + expect(getRepoUpstream).toHaveBeenCalledTimes(2) + expect(repos[0].upstream).toEqual(UPSTREAM) + }) + + it('lets a healthy host pass a stalled host while bounding concurrent probes', async () => { + let inFlight = 0 + let maxInFlight = 0 + let releaseA: (() => void) | undefined + let releaseB: (() => void) | undefined + getRepoUpstreamMock.mockImplementation(async (path) => { + inFlight += 1 + maxInFlight = Math.max(maxInFlight, inFlight) + try { + if (path === '/srv/a') { + await new Promise((resolve) => { + releaseA = resolve + }) + } + if (path === '/srv/b') { + await new Promise((resolve) => { + releaseB = resolve + }) + } + return UPSTREAM + } finally { + inFlight -= 1 + } + }) + const { runtime, repos } = createRuntime([ + makeRepo({ id: 'a', path: '/srv/a', connectionId: 'ssh-1' }), + makeRepo({ id: 'b', path: '/srv/b', connectionId: 'ssh-2' }), + makeRepo({ id: 'c', path: '/srv/c', connectionId: 'ssh-3' }) + ]) + + notifyConnected(runtime, 'ssh-1') + notifyConnected(runtime, 'ssh-2') + notifyConnected(runtime, 'ssh-3') + await drainBackfill() + + expect(getRepoUpstream).toHaveBeenCalledTimes(2) + expect(getRepoUpstream).not.toHaveBeenCalledWith('/srv/c', 'ssh-3') + expect(maxInFlight).toBe(2) + + releaseB?.() + await drainBackfill() + + expect(repos[1].upstream).toEqual(UPSTREAM) + expect(repos[2].upstream).toEqual(UPSTREAM) + expect(repos[0].upstream).toBeUndefined() + expect(maxInFlight).toBe(2) + + releaseA?.() + await drainBackfill() + + expect(repos[0].upstream).toEqual(UPSTREAM) + }) + + it('does not persist a stale null when a queued provider disconnects', async () => { + let releaseA: (() => void) | undefined + let releaseB: (() => void) | undefined + getRepoUpstreamMock.mockImplementation((path) => { + if (path === '/srv/a') { + return new Promise((resolve) => { + releaseA = () => resolve(null) + }) + } + if (path === '/srv/b') { + return new Promise((resolve) => { + releaseB = () => resolve(null) + }) + } + return Promise.resolve(UPSTREAM) + }) + const { runtime, repos } = createRuntime([ + makeRepo({ id: 'a', path: '/srv/a', connectionId: 'ssh-1' }), + makeRepo({ id: 'b', path: '/srv/b', connectionId: 'ssh-2' }), + makeRepo({ id: 'c', path: '/srv/c', connectionId: 'ssh-3' }) + ]) + + notifyConnected(runtime, 'ssh-1') + notifyConnected(runtime, 'ssh-2') + await drainBackfill() + expect(getRepoUpstream).toHaveBeenCalledTimes(2) + + notifyConnected(runtime, 'ssh-3') + notifyDisconnected(runtime, 'ssh-3') + releaseA?.() + await drainBackfill() + + expect(repos[2].upstream).toBeUndefined() + expect(getRepoUpstream).not.toHaveBeenCalledWith('/srv/c', 'ssh-3') + + notifyConnected(runtime, 'ssh-3') + await drainBackfill() + + expect(repos[2].upstream).toEqual(UPSTREAM) + + releaseB?.() + await drainBackfill() + }) + + it('returns from the connect notification without waiting on the probe', async () => { + let resolveProbe: ((value: unknown) => void) | undefined + getRepoUpstreamMock.mockImplementation( + () => + new Promise((resolve) => { + resolveProbe = resolve + }) + ) + const { runtime, repos } = createRuntime([makeRepo({ id: 'ssh-fork', connectionId: 'ssh-1' })]) + + notifyConnected(runtime, 'ssh-1') + + expect(getRepoUpstream).not.toHaveBeenCalled() + await drainBackfill() + expect(getRepoUpstream).toHaveBeenCalledOnce() + expect(repos[0].upstream).toBeUndefined() + + resolveProbe?.(UPSTREAM) + await drainBackfill() + expect(repos[0].upstream).toEqual(UPSTREAM) + }) + + it('leaves the startup pass local-only', async () => { + const { runtime } = createRuntime([ + makeRepo({ id: 'local', path: '/home/a' }), + makeRepo({ id: 'ssh-fork', connectionId: 'ssh-1' }) + ]) + + runtime.setNotifier({ reposChanged: vi.fn() } as never) + await drainBackfill() + + expect(getRepoUpstream).toHaveBeenCalledExactlyOnceWith('/home/a', null) + }) +}) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 526acea823bb..c1849dcab4e9 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -2787,6 +2787,13 @@ function getSetupRunnerCommandPlatformForLaunch( return getSetupRunnerCommandPlatformForPath(setup?.runnerScriptPath ?? '', fallbackPlatform) } +type ForkUpstreamSshScope = { + connectionId: string + providerGeneration: number +} + +const FORK_UPSTREAM_BACKFILL_CONCURRENCY = 2 + export class OrcaRuntimeService { private readonly runtimeId = randomUUID() private readonly startedAt = Date.now() @@ -2920,6 +2927,11 @@ export class OrcaRuntimeService { >() private worktreeLifecycleListeners = new Set<(event: RuntimeWorktreeLifecycleEvent) => void>() private forkBackfillStarted = false + private forkUpstreamBackfillActivePasses = 0 + private forkUpstreamBackfillWaiters: (() => void)[] = [] + private forkUpstreamBackfilledConnectionIds = new Set() + private forkUpstreamBackfillReadyGenerationByConnectionId = new Map() + private forkUpstreamBackfillRunningConnectionIds = new Set() private agentBrowserBridge: AgentBrowserBridge | null = null private offscreenBrowserBackend: BrowserBackend | null = null private emulatorBridge: EmulatorBridge | null = null @@ -5296,6 +5308,9 @@ export class OrcaRuntimeService { this.invalidateSshWorktreeScanCache(targetId) if (state.status !== 'connected') { this.cancelLegacyWorkerTerminalRecoveryRetry(`ssh:${targetId}`) + this.forkUpstreamBackfillReadyGenerationByConnectionId.delete(targetId) + } else { + this.backfillForkUpstreamsForConnection(targetId) } this.emitClientEvent({ type: 'sshStateChanged', targetId, state: getPublicSshState(state)! }) } @@ -19390,22 +19405,141 @@ export class OrcaRuntimeService { } // Why: repos added before fork detection existed have no stored `upstream`, so - // their avatar/badge would never self-correct. Resolve it once at startup for - // local git repos; SSH repos resolve lazily when their settings open (their - // connection may not be up yet). Sequential to respect the gh rate limit; - // failures leave `upstream` unset so the next launch retries. - private async backfillForkUpstreams(): Promise { + // their avatar/badge/project rows would never self-correct. Resolve it once at + // startup for repos whose host is already reachable; SSH repos wait for their + // connection instead (their host may not be up yet). + private backfillForkUpstreams(): Promise { + return this.queueForkUpstreamBackfill(() => + this.runForkUpstreamBackfill((repo) => !repo.connectionId) + ) + } + + // Why: an SSH fork used to stay `upstream === undefined` until someone opened + // its settings page, hiding it from project rows and from the fork badge with + // no visible cause (#12967). Connect is the first moment the probe can succeed. + // Fire-and-forget so nothing on the connection path waits on the network. + private backfillForkUpstreamsForConnection(connectionId: string): void { + if ( + this.forkUpstreamBackfilledConnectionIds.has(connectionId) || + !getSshGitProvider(connectionId) + ) { + return + } + this.forkUpstreamBackfillReadyGenerationByConnectionId.set( + connectionId, + getSshGitProviderGeneration(connectionId) + ) + if (this.forkUpstreamBackfillRunningConnectionIds.has(connectionId)) { + return + } + this.forkUpstreamBackfillRunningConnectionIds.add(connectionId) + void this.drainForkUpstreamBackfillForConnection(connectionId) + .catch(() => undefined) + .finally(() => { + this.forkUpstreamBackfillRunningConnectionIds.delete(connectionId) + if ( + !this.forkUpstreamBackfilledConnectionIds.has(connectionId) && + this.forkUpstreamBackfillReadyGenerationByConnectionId.has(connectionId) + ) { + this.backfillForkUpstreamsForConnection(connectionId) + } + }) + } + + // Why: bound migration subprocesses without letting one stalled host block every other host. + private async queueForkUpstreamBackfill(run: () => Promise): Promise { + await this.acquireForkUpstreamBackfillPermit() + try { + return await run() + } finally { + this.releaseForkUpstreamBackfillPermit() + } + } + + private acquireForkUpstreamBackfillPermit(): Promise { + if (this.forkUpstreamBackfillActivePasses < FORK_UPSTREAM_BACKFILL_CONCURRENCY) { + this.forkUpstreamBackfillActivePasses += 1 + return Promise.resolve() + } + return new Promise((resolve) => this.forkUpstreamBackfillWaiters.push(resolve)) + } + + private releaseForkUpstreamBackfillPermit(): void { + const next = this.forkUpstreamBackfillWaiters.shift() + if (next) { + next() + return + } + this.forkUpstreamBackfillActivePasses -= 1 + } + + private async drainForkUpstreamBackfillForConnection(connectionId: string): Promise { + while (!this.forkUpstreamBackfilledConnectionIds.has(connectionId)) { + const providerGeneration = + this.forkUpstreamBackfillReadyGenerationByConnectionId.get(connectionId) + if (providerGeneration === undefined) { + return + } + const scope = { connectionId, providerGeneration } + const complete = await this.queueForkUpstreamBackfill(() => + this.runForkUpstreamBackfill((repo) => repo.connectionId === connectionId, scope) + ) + if (complete) { + this.forkUpstreamBackfilledConnectionIds.add(connectionId) + this.forkUpstreamBackfillReadyGenerationByConnectionId.delete(connectionId) + return + } + if ( + this.forkUpstreamBackfillReadyGenerationByConnectionId.get(connectionId) === + providerGeneration + ) { + this.forkUpstreamBackfillReadyGenerationByConnectionId.delete(connectionId) + } + } + } + + private isForkUpstreamBackfillScopeCurrent(scope: ForkUpstreamSshScope): boolean { + return ( + this.forkUpstreamBackfillReadyGenerationByConnectionId.get(scope.connectionId) === + scope.providerGeneration && + getSshGitProviderGeneration(scope.connectionId) === scope.providerGeneration && + getSshGitProvider(scope.connectionId) !== undefined + ) + } + + /** Resolves false when any probe failed, so the caller can retry that scope later. */ + private async runForkUpstreamBackfill( + selects: (repo: Repo) => boolean, + sshScope?: ForkUpstreamSshScope + ): Promise { + let complete = true try { const store = this.requireStore() let changed = false for (const repo of store.getRepos()) { - if (repo.upstream !== undefined || repo.kind === 'folder' || repo.connectionId) { + if (repo.upstream !== undefined || repo.kind === 'folder' || !selects(repo)) { continue } + if (sshScope && !this.isForkUpstreamBackfillScopeCurrent(sshScope)) { + complete = false + break + } let upstream: GitHubOwnerRepo | null try { - upstream = await getRepoUpstream(repo.path, null) + upstream = await getRepoUpstream(repo.path, repo.connectionId ?? null) } catch { + complete = false + if (sshScope && !this.isForkUpstreamBackfillScopeCurrent(sshScope)) { + break + } + continue + } + if (sshScope && !this.isForkUpstreamBackfillScopeCurrent(sshScope)) { + complete = false + break + } + // Why: SSH null may hide a transient Git/gh failure, so only persist a positive result. + if (sshScope && !upstream) { continue } const updates: Partial = { upstream: upstream ?? null } @@ -19419,8 +19553,10 @@ export class OrcaRuntimeService { if (changed) { this.notifyReposChanged() } + return complete } catch { - // Best-effort startup backfill; never disrupt launch. + // Best-effort; never disrupt launch or a connection. + return false } } diff --git a/tests/e2e/ssh-docker-fork-upstream-backfill.spec.ts b/tests/e2e/ssh-docker-fork-upstream-backfill.spec.ts new file mode 100644 index 000000000000..996833e86de1 --- /dev/null +++ b/tests/e2e/ssh-docker-fork-upstream-backfill.spec.ts @@ -0,0 +1,230 @@ +// Offline proof: a distinct upstream remote resolves before gh, requiring no network or auth. +import { readFileSync, readdirSync, writeFileSync } from 'node:fs' +import path from 'node:path' + +import type { Page } from '@stablyai/playwright-test' + +import { connectDockerSshRelayTarget } from './helpers/docker-ssh-relay-connection' +import { + cleanupDockerSshRelayTarget, + execDockerSshRelayTargetCommand, + shellQuote, + startDockerSshRelayTarget, + type DockerSshRelayTarget +} from './helpers/docker-ssh-relay-target' +import { test, expect } from './helpers/orca-app' +import { createRestartSession, readRestartRendererState } from './helpers/orca-restart' + +const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1' + +const FORK_REPO_PATH = '/tmp/orca-docker-fork-upstream-repo' +const FORK_ORIGIN_URL = 'https://github.com/orca-e2e-contributor/orca.git' +const UPSTREAM_URL = 'https://github.com/stablyai/orca.git' +const EXPECTED_UPSTREAM = { owner: 'stablyai', repo: 'orca' } +const FORK_BADGE_LABEL = 'Fork of stablyai/orca' + +/** A fork clone: `origin` is the personal copy, `upstream` is the parent. */ +function seedForkRepoOnTarget(target: DockerSshRelayTarget): void { + execDockerSshRelayTargetCommand( + target, + [ + `rm -rf ${shellQuote(FORK_REPO_PATH)}`, + `mkdir -p ${shellQuote(FORK_REPO_PATH)}`, + `cd ${shellQuote(FORK_REPO_PATH)}`, + 'git init', + 'git config user.email e2e@test.local', + 'git config user.name "Orca Fork Upstream E2E"', + `git remote add origin ${shellQuote(FORK_ORIGIN_URL)}`, + `git remote add upstream ${shellQuote(UPSTREAM_URL)}`, + "printf '%s\\n' 'fork upstream backfill fixture' > README.md", + 'git add README.md', + 'git commit -m initial' + ].join(' && ') + ) +} + +type PersistedUpstream = { owner: string; repo: string; host?: string } | null | 'absent' + +/** + * Why `readRestartRendererState`: a relaunched window replaces its document + * during initial hydration, which destroys the execution context mid-evaluate. + * The wrapper turns that transition into `null` so the enclosing poll retries. + */ +async function readRepoUpstream( + page: Page, + repoId: string +): Promise { + return readRestartRendererState(() => + page.evaluate(async (repoId) => { + const store = window.__store + if (!store) { + throw new Error('Store unavailable') + } + await store.getState().fetchRepos() + const repo = store.getState().repos.find((candidate) => candidate.id === repoId) + if (!repo) { + return 'gone' as const + } + // Why: `undefined` (never probed) and `null` (probed, not a fork) are + // different states and both survive this boundary badly — name them. + return repo.upstream === undefined ? ('absent' as const) : repo.upstream + }, repoId) + ) +} + +/** + * Every file the live store could be writing. + * + * Why not just `/orca-data.json`: the Store is constructed with the + * *active profile's* data file (`index.ts` → `new Store({ dataFile })`), which + * lives under `profiles//`. The launch dir only holds the seeded profile. + */ +function candidateStoreFiles(userDataPath: string): string[] { + const profilesDir = path.join(userDataPath, 'profiles') + let profileFiles: string[] = [] + try { + profileFiles = readdirSync(profilesDir, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => path.join(profilesDir, entry.name, 'orca-data.json')) + } catch { + // No profiles directory yet. + } + return [...profileFiles, path.join(userDataPath, 'orca-data.json')] +} + +function findStoreFileWithRepo(userDataPath: string, repoId: string): string | null { + for (const storePath of candidateStoreFiles(userDataPath)) { + try { + const data = JSON.parse(readFileSync(storePath, 'utf8')) as { repos?: { id: string }[] } + if ((data.repos ?? []).some((repo) => repo.id === repoId)) { + return storePath + } + } catch { + // Why: the store rewrites via a temp file, so a read can land mid-swap. + } + } + return null +} + +/** Why: saves are debounced ~1s, so closing right after `addRemote` can beat the write. */ +async function waitForPersistedRepo(userDataPath: string, repoId: string): Promise { + await expect + .poll(() => findStoreFileWithRepo(userDataPath, repoId) !== null, { + timeout: 30_000, + message: + `repo ${repoId} never reached any store file under ${userDataPath}. ` + + `Checked: ${candidateStoreFiles(userDataPath).join(' | ')}` + }) + .toBe(true) + const storePath = findStoreFileWithRepo(userDataPath, repoId) + if (!storePath) { + throw new Error(`Store file for ${repoId} disappeared after polling`) + } + return storePath +} + +/** + * Rewrite the persisted repo to the pre-fork-detection shape. + * + * Why not `repos.update`: `sanitizeRepoUpdatesForPersistence` treats an + * `undefined` upstream as "no change" and drops it, so the field cannot be + * cleared through the app. Legacy state only exists on disk. + */ +function stripPersistedUpstream(storePath: string, repoId: string): void { + const data = JSON.parse(readFileSync(storePath, 'utf8')) as { + repos?: { id: string; upstream?: unknown }[] + } + const repo = data.repos?.find((candidate) => candidate.id === repoId) + if (!repo) { + throw new Error(`Repo ${repoId} not found in ${storePath}`) + } + if (!('upstream' in repo)) { + throw new Error(`Repo ${repoId} had no persisted upstream to strip — fixture assumption broke`) + } + delete repo.upstream + writeFileSync(storePath, `${JSON.stringify(data, null, 2)}\n`) +} + +test.describe('Docker SSH fork upstream backfill', () => { + test.skip(!RUN_DOCKER_SSH, 'Set ORCA_E2E_SSH_DOCKER=1 to run Docker-backed SSH tests.') + test.skip(process.platform === 'win32', 'Docker SSH fixtures use POSIX tooling.') + + test('backfills a legacy SSH fork and renders its badge once the connection comes up', async (// oxlint-disable-next-line no-empty-pattern -- Playwright's second fixture arg is testInfo; the first must be an object destructure to opt out of the default fixture set. + {}, testInfo) => { + test.slow() + let target: DockerSshRelayTarget | null = null + const session = createRestartSession(testInfo) + + try { + target = startDockerSshRelayTarget(testInfo) + seedForkRepoOnTarget(target) + + // ── Launch 1: add the remote fork the normal way ────────────────── + const first = await session.launch() + const remote = await connectDockerSshRelayTarget(first.page, target, { + remotePath: FORK_REPO_PATH + }) + // Add-time detection already handles SSH, so this proves the probe itself + // works over the relay — isolating the bug to the startup backfill. + await expect + .poll(() => readRepoUpstream(first.page, remote.repoId), { + timeout: 60_000, + message: 'add-time detection never resolved the fork upstream over SSH' + }) + .toMatchObject(EXPECTED_UPSTREAM) + // Why: ask the app where it actually persists rather than assuming the + // launch dir — profile redirection would silently strip nothing. + const userDataPath = await first.app.evaluate(({ app }) => app.getPath('userData')) + const storePath = await waitForPersistedRepo(userDataPath, remote.repoId) + await session.close(first.app) + + stripPersistedUpstream(storePath, remote.repoId) + + // ── Launch 2: the repo now looks like it predates fork detection ── + const second = await session.launch() + // Why poll: this doubles as the hydration barrier — the relaunched window + // swaps its document once before the store is readable. + await expect + .poll(() => readRepoUpstream(second.page, remote.repoId), { + timeout: 30_000, + message: 'relaunched app never surfaced the stripped repo as unresolved' + }) + .toBe('absent') + await expect(second.page.getByLabel(FORK_BADGE_LABEL)).toHaveCount(0) + + const connected = await second.page.evaluate(async (targetId) => { + const store = window.__store + if (!store) { + throw new Error('Store unavailable') + } + const state = await window.api.ssh.connect({ targetId }) + if (!state || state.status !== 'connected') { + throw new Error(`SSH target did not connect: ${JSON.stringify(state)}`) + } + store.getState().setSshConnectionState(targetId, state) + return state.status + }, remote.targetId) + expect(connected).toBe('connected') + + // ── The fix: connect alone backfills, with no settings visit ────── + await expect + .poll(() => readRepoUpstream(second.page, remote.repoId), { + timeout: 90_000, + message: 'connecting the SSH target did not backfill the fork upstream' + }) + .toMatchObject(EXPECTED_UPSTREAM) + await expect(second.page.getByLabel(FORK_BADGE_LABEL).first()).toBeVisible({ + timeout: 30_000 + }) + + testInfo.annotations.push({ + type: 'ssh-fork-upstream-backfill', + description: `target=${remote.targetId} repo=${remote.repoId} upstream=stablyai/orca` + }) + await session.close(second.app) + } finally { + await session.dispose() + cleanupDockerSshRelayTarget(target) + } + }) +})