diff --git a/config/main-sync-spawn-baseline.txt b/config/main-sync-spawn-baseline.txt index e6b1135e6b7..43e9437cb77 100644 --- a/config/main-sync-spawn-baseline.txt +++ b/config/main-sync-spawn-baseline.txt @@ -41,7 +41,6 @@ 1 src/main/pty/posix-pty-process-groups.ts 1 src/main/pwsh.ts 1 src/main/runtime/runtime-skill-install-authority.ts -1 src/main/skills/skill-discovery-target.ts 2 src/main/startup/cli-launch-redirect.ts 1 src/main/startup/linux-secret-service-probe.ts 1 src/main/worktree-runner-script.ts diff --git a/src/main/ipc/filesystem-allowed-roots.test.ts b/src/main/ipc/filesystem-allowed-roots.test.ts index 0613ca97de5..9ae8aeee3d3 100644 --- a/src/main/ipc/filesystem-allowed-roots.test.ts +++ b/src/main/ipc/filesystem-allowed-roots.test.ts @@ -261,7 +261,7 @@ beforeEach(() => { }) describe('getAllowedRoots', () => { - it('stops scanning repositories when a local candidate settles each folder scope', () => { + it('stops scanning repositories when a local candidate settles each folder scope', async () => { const fixture: StoreFixture = { repos: Array.from({ length: 1_000 }, (_, index) => makeRepo({ id: `repo-${index}`, path: `/folders/root/repo-${index}` }) @@ -274,18 +274,18 @@ describe('getAllowedRoots', () => { } const { store } = makeCountingStore(fixture) vi.mocked(isPathInsideOrEqual).mockClear() - const actual = getAllowedRoots(store) + const actual = await getAllowedRoots(store) expect(isPathInsideOrEqual).toHaveBeenCalledTimes(100) vi.mocked(isPathInsideOrEqual).mockClear() expect(actual).toEqual(referenceAllowedRoots(store)) expect(isPathInsideOrEqual).toHaveBeenCalledTimes(100_000) }) - it('reads the store once and indexes project groups once per build', () => { + it('reads the store once and indexes project groups once per build', async () => { const fixture = makeMixedFixture() const { store, counts } = makeCountingStore(fixture) - getAllowedRoots(store) + await getAllowedRoots(store) expect.soft(counts.getRepos).toBe(1) expect.soft(counts.getProjectGroups).toBe(1) diff --git a/src/main/ipc/filesystem-allowed-roots.ts b/src/main/ipc/filesystem-allowed-roots.ts index 65cc69e503d..ddbd6f18d58 100644 --- a/src/main/ipc/filesystem-allowed-roots.ts +++ b/src/main/ipc/filesystem-allowed-roots.ts @@ -1,6 +1,6 @@ import { isAbsolute, resolve } from 'node:path' import type { RuntimeStore } from '../runtime/runtime-store-contract' -import { computeWorkspaceRoot, getWorktreePathSettings } from './worktree-logic' +import { computeWorkspaceRootWithin, getWorktreePathSettings } from './worktree-logic' import { getWorktreeMirrorDistroForRuntime, resolveLocalProjectRuntimesForRepos @@ -116,7 +116,11 @@ export function resolveUnanchoredWorkspaceRoot( return hostPath.isAbsolute(workspaceDir) ? hostPath.resolve(workspaceDir) : null } -export function getAllowedRoots(store: RuntimeStore): string[] { +// A broken distro must not hold every file check; the probe keeps running past this and caches. +const ALLOWED_ROOT_WSL_HOME_DEADLINE_MS = 5_000 + +/** Async because a root mirrored into WSL needs the distro's home, and a synchronous probe blocks main. */ +export async function getAllowedRoots(store: RuntimeStore): Promise { // Rehydrate repos only once per authorization snapshot. const repos = store.getRepos() const localRepos = filterLocalRepos(repos) @@ -133,21 +137,21 @@ export function getAllowedRoots(store: RuntimeStore): string[] { } } else { const projectRuntimeByRepoId = resolveLocalProjectRuntimesForRepos(store, localRepos) - for (const repo of localRepos) { - roots.push( - resolve( - computeWorkspaceRoot( - repo.path, - // Match creation placement, including WSL, so newly created worktrees remain authorized. - getWorktreePathSettings( - repo, - settings, - getWorktreeMirrorDistroForRuntime(projectRuntimeByRepoId.get(repo.id)) - ) - ) + const workspaceRoots = await Promise.all( + localRepos.map((repo) => + computeWorkspaceRootWithin( + repo.path, + // Match creation placement, including WSL, so newly created worktrees remain authorized. + getWorktreePathSettings( + repo, + settings, + getWorktreeMirrorDistroForRuntime(projectRuntimeByRepoId.get(repo.id)) + ), + ALLOWED_ROOT_WSL_HOME_DEADLINE_MS ) ) - } + ) + roots.push(...workspaceRoots.map((root) => resolve(root))) } } return roots diff --git a/src/main/ipc/filesystem-auth.ts b/src/main/ipc/filesystem-auth.ts index cd58d6ce20d..67d72f95aef 100644 --- a/src/main/ipc/filesystem-auth.ts +++ b/src/main/ipc/filesystem-auth.ts @@ -12,23 +12,25 @@ import { export const PATH_ACCESS_DENIED_MESSAGE = `${PATH_OUTSIDE_ALLOWED_DIRECTORIES}. If this blocks a legitimate workflow, please file a GitHub issue.` /** Lazily share allowed roots across every check in one authorization. */ -type AllowedRootsSnapshot = { get: () => readonly string[] } +type AllowedRootsSnapshot = { get: () => Promise } function createAllowedRootsSnapshot( store: RuntimeStore, extraRoots: readonly string[] = [] ): AllowedRootsSnapshot { - let roots: readonly string[] | undefined - return { get: () => (roots ??= [...getAllowedRoots(store), ...extraRoots]) } + let roots: Promise | undefined + return { + get: () => (roots ??= getAllowedRoots(store).then((found) => [...found, ...extraRoots])) + } } -export function isPathAllowed( +export async function isPathAllowed( targetPath: string, store: RuntimeStore, allowedRoots?: AllowedRootsSnapshot -): boolean { +): Promise { const resolvedTarget = resolve(targetPath) - return (allowedRoots?.get() ?? getAllowedRoots(store)).some((root) => + return (await (allowedRoots?.get() ?? getAllowedRoots(store))).some((root) => isDescendantOrEqual(resolvedTarget, root) ) } @@ -151,7 +153,7 @@ async function isPathAllowedIncludingRegisteredWorktrees( store: RuntimeStore, options: { canonicalSourcePath?: string; allowedRoots?: AllowedRootsSnapshot } = {} ): Promise { - if (isPathAllowed(targetPath, store, options.allowedRoots)) { + if (await isPathAllowed(targetPath, store, options.allowedRoots)) { return true } @@ -194,7 +196,7 @@ async function isPathAllowedByCanonicalAllowedRoot( if (!sourcePath) { return false } - for (const root of allowedRoots?.get() ?? getAllowedRoots(store)) { + for (const root of await (allowedRoots?.get() ?? getAllowedRoots(store))) { const resolvedRoot = resolve(root) if (!isDescendantOrEqual(sourcePath, resolvedRoot)) { continue diff --git a/src/main/ipc/filesystem-canonical-owner.test.ts b/src/main/ipc/filesystem-canonical-owner.test.ts index cb7a02a9cdb..0657abcc883 100644 --- a/src/main/ipc/filesystem-canonical-owner.test.ts +++ b/src/main/ipc/filesystem-canonical-owner.test.ts @@ -31,7 +31,7 @@ const mocks = vi.hoisted(() => ({ vi.mock('node:fs/promises', () => ({ stat: mocks.stat, realpath: mocks.realpath })) vi.mock('../repo-worktrees', () => ({ listRepoWorktreeGraph: mocks.graph, isRepoRoot: vi.fn() })) vi.mock('./worktree-logic', () => ({ - computeWorkspaceRoot: mocks.workspaceRoot, + computeWorkspaceRootWithin: async (...args: unknown[]) => mocks.workspaceRoot(...args), getWorktreePathSettings: mocks.pathSettings })) vi.mock('../project-runtime-git-options', () => ({ @@ -157,8 +157,8 @@ describe.each(deniedOwners)('$name fixture matrix', ({ owner }) => { 'local-looking group and folder over a remote child repo', () => storeFor([repo(owner, { path: join(root, 'child') })], [group()], [folder()]) ] - ])('denies the %s', (_case, build) => { - expect(isPathAllowed(join(root, 'file'), build())).toBe(false) + ])('denies the %s', async (_case, build) => { + expect(await isPathAllowed(join(root, 'file'), build())).toBe(false) }) }) @@ -166,7 +166,7 @@ describe.each(deniedOwners)('$name filesystem ownership', ({ owner }) => { it('does not grant a repository root or register linked roots', async () => { const store = storeFor([repo(owner)]) expect(getLocalRepos(store)).toEqual([]) - expect(isPathAllowed(join(root, 'file'), store)).toBe(false) + expect(await isPathAllowed(join(root, 'file'), store)).toBe(false) registerWorktreeRootsForRepo(store, 'repo', [linked]) registerCreatedWorktreeRoot(store, 'repo', linked) expect(isRegisteredWorktreePath(linked, store)).toBe(false) @@ -178,37 +178,37 @@ describe.each(deniedOwners)('$name filesystem ownership', ({ owner }) => { it.each(['group', 'folder', 'inherited group'] as const)( 'does not grant an empty %s scope', - (kind) => { + async (kind) => { const store = storeFor( [], kind !== 'folder' ? [group(owner)] : [], kind === 'folder' ? [folder(owner)] : kind === 'inherited group' ? [folder()] : [] ) - expect(getAllowedRoots(store)).toEqual([]) - expect(isPathAllowed(join(root, 'file'), store)).toBe(false) + expect(await getAllowedRoots(store)).toEqual([]) + expect(await isPathAllowed(join(root, 'file'), store)).toBe(false) } ) - it('does not infer a local group or folder from a remote child repo', () => { + it('does not infer a local group or folder from a remote child repo', async () => { const store = storeFor( [repo(owner, { path: join(root, 'child'), projectGroupId: 'nested' })], [group(), group({}, { id: 'nested', parentGroupId: 'group', parentPath: null })], [folder()] ) - expect(getAllowedRoots(store)).toEqual([]) - expect(isPathAllowed(join(root, 'file'), store)).toBe(false) + expect(await getAllowedRoots(store)).toEqual([]) + expect(await isPathAllowed(join(root, 'file'), store)).toBe(false) }) }) describe.each(allowedOwners)('$name filesystem ownership', ({ owner }) => { - it('preserves repo, group, folder and recovered worktree roots', () => { + it('preserves repo, group, folder and recovered worktree roots', async () => { for (const store of [ storeFor([repo(owner)]), storeFor([], [group(owner)]), storeFor([], [], [folder(owner)]) ]) { - expect(getAllowedRoots(store)).toEqual([root]) - expect(isPathAllowed(join(root, 'file'), store)).toBe(true) + expect(await getAllowedRoots(store)).toEqual([root]) + expect(await isPathAllowed(join(root, 'file'), store)).toBe(true) } const store = storeFor([repo(owner)]) registerWorktreeRootsForRepo(store, 'repo', [root]) @@ -220,7 +220,7 @@ describe.each(allowedOwners)('$name filesystem ownership', ({ owner }) => { }) }) -it('preserves an unpinned mixed local and SSH folder scope', () => { +it('preserves an unpinned mixed local and SSH folder scope', async () => { const store = storeFor( [ repo({ executionHostId: 'ssh:host-a' }, { path: join(root, 'remote') }), @@ -229,22 +229,22 @@ it('preserves an unpinned mixed local and SSH folder scope', () => { [group()], [folder()] ) - expect(getAllowedRoots(store)).toEqual([join(root, 'local'), root, root]) + expect(await getAllowedRoots(store)).toEqual([join(root, 'local'), root, root]) }) -it('keeps an explicit SSH folder scope remote even with a local candidate', () => { +it('keeps an explicit SSH folder scope remote even with a local candidate', async () => { const store = storeFor( [repo({}, { path: join(root, 'local') })], [group({ executionHostId: 'ssh:host-a' })], [folder()] ) - expect(getAllowedRoots(store)).toEqual([join(root, 'local')]) - expect(isPathAllowed(join(root, 'file'), store)).toBe(false) + expect(await getAllowedRoots(store)).toEqual([join(root, 'local')]) + expect(await isPathAllowed(join(root, 'file'), store)).toBe(false) }) -it('preserves explicit local folder overrides and the legacy empty connection override', () => { +it('preserves explicit local folder overrides and the legacy empty connection override', async () => { expect( - getAllowedRoots( + await getAllowedRoots( storeFor( [], [group({ executionHostId: 'ssh:host-a' })], @@ -253,18 +253,18 @@ it('preserves explicit local folder overrides and the legacy empty connection ov ) ).toEqual([root]) expect( - getAllowedRoots( + await getAllowedRoots( storeFor([], [group({ connectionId: 'host-a' })], [folder({ connectionId: '' })]) ) ).toEqual([root]) }) -it('denies a workspace pinned local under a group carrying only a legacy connection', () => { +it('denies a workspace pinned local under a group carrying only a legacy connection', async () => { const pinnedFolder = folder({ executionHostId: 'local' }) const legacyGroup = group({ connectionId: 'host-a' }) const store = storeFor([], [legacyGroup], [pinnedFolder]) - expect(getAllowedRoots(store)).toEqual([]) - expect(isPathAllowed(join(root, 'file'), store)).toBe(false) + expect(await getAllowedRoots(store)).toEqual([]) + expect(await isPathAllowed(join(root, 'file'), store)).toBe(false) // Dispatch accepts the local pin, but authorization preserves the stricter legacy ownership check. expect( resolveFolderWorkspaceHost( @@ -277,39 +277,39 @@ it('denies a workspace pinned local under a group carrying only a legacy connect [group({ executionHostId: 'local' }, { parentPath: null })], [folder({ connectionId: 'host-a' })] ) - expect(getAllowedRoots(mirrored)).toEqual([]) - expect(isPathAllowed(join(root, 'file'), mirrored)).toBe(false) + expect(await getAllowedRoots(mirrored)).toEqual([]) + expect(await isPathAllowed(join(root, 'file'), mirrored)).toBe(false) }) -it('still grants a local directory that happens to share an SSH repo path', () => { +it('still grants a local directory that happens to share an SSH repo path', async () => { const store = storeFor([ repo({ executionHostId: 'ssh:host-a' }), repo({}, { id: 'mine', path: root }) ]) - expect(getAllowedRoots(store)).toEqual([root]) - expect(isPathAllowed(join(root, 'file'), store)).toBe(true) + expect(await getAllowedRoots(store)).toEqual([root]) + expect(await isPathAllowed(join(root, 'file'), store)).toBe(true) }) describe('workspace-directory fallback', () => { const workspaceDir = resolve('/ws-fixture') - it('does not widen past the roots a local repo of its own would grant', () => { - const local = getAllowedRoots(storeFor([repo()], [], [], { workspaceDir })) - const sshOnly = getAllowedRoots( + it('does not widen past the roots a local repo of its own would grant', async () => { + const local = await getAllowedRoots(storeFor([repo()], [], [], { workspaceDir })) + const sshOnly = await getAllowedRoots( storeFor([repo({ executionHostId: 'ssh:host-a' })], [], [], { workspaceDir }) ) expect(local).toEqual([root, workspaceDir]) expect(sshOnly).toEqual([workspaceDir]) expect( - isPathAllowed( + await isPathAllowed( join(root, 'file'), storeFor([repo({ connectionId: 'host-a' })], [], [], { workspaceDir }) ) ).toBe(false) }) - it('grants a workspace directory this host reads as absolute, with no local repo', () => { - expect(getAllowedRoots(storeFor([], [], [], { workspaceDir: '/ws-fixture' }))).toEqual([ + it('grants a workspace directory this host reads as absolute, with no local repo', async () => { + expect(await getAllowedRoots(storeFor([], [], [], { workspaceDir: '/ws-fixture' }))).toEqual([ resolve('/ws-fixture') ]) }) @@ -318,11 +318,11 @@ describe('workspace-directory fallback', () => { ['bare name', 'orca-ws'], ['parent traversal', '..'], ['relative traversal', '../orca-ws'] - ])('grants nothing for a repo-relative %s with no repo to anchor it', (_case, dir) => { + ])('grants nothing for a repo-relative %s with no repo to anchor it', async (_case, dir) => { // `resolve` would anchor these to the main-process cwd, granting an unrelated tree. - expect(getAllowedRoots(storeFor([], [], [], { workspaceDir: dir }))).toEqual([]) + expect(await getAllowedRoots(storeFor([], [], [], { workspaceDir: dir }))).toEqual([]) expect( - getAllowedRoots( + await getAllowedRoots( storeFor([repo({ executionHostId: 'ssh:host-a' })], [], [], { workspaceDir: dir }) @@ -333,10 +333,10 @@ describe('workspace-directory fallback', () => { // Use the real host module here; the injected flavor matrix below covers both platforms. it.skipIf(process.platform === 'win32')( 'does not anchor a Windows-style workspace directory under the POSIX main-process cwd', - () => { + async () => { const store = storeFor([], [], [], { workspaceDir: 'C:\\workspaces' }) - expect(getAllowedRoots(store)).toEqual([]) - expect(isPathAllowed(join(resolve('C:\\workspaces'), 'file'), store)).toBe(false) + expect(await getAllowedRoots(store)).toEqual([]) + expect(await isPathAllowed(join(resolve('C:\\workspaces'), 'file'), store)).toBe(false) } ) diff --git a/src/main/ipc/skill-delete/handlers.ts b/src/main/ipc/skill-delete/handlers.ts index 949b52d074d..91cb6642548 100644 --- a/src/main/ipc/skill-delete/handlers.ts +++ b/src/main/ipc/skill-delete/handlers.ts @@ -27,10 +27,10 @@ export function registerSkillDeleteIpcHandlers(store: Store, runtime?: OrcaRunti : {}), userDataPath: app.getPath('userData') } - const resolve = (value: unknown) => { + const resolve = async (value: unknown) => { const request = SkillDeleteRequestSchema.parse(value) // Preserve discovery's nested stripping schema inside the strict deletion request. - const target = resolveSkillDiscoveryTarget( + const target = await resolveSkillDiscoveryTarget( request.target ? SkillDiscoveryTargetSchema.parse(request.target) : undefined ) return { request, target } @@ -39,7 +39,7 @@ export function registerSkillDeleteIpcHandlers(store: Store, runtime?: OrcaRunti handleMainWindowSkillIpc( 'skills:previewDelete', async (_event, value: unknown): Promise => { - const { request, target } = resolve(value) + const { request, target } = await resolve(value) return previewSkillDeleteRequest(request, target, dependencies) } ) @@ -47,7 +47,7 @@ export function registerSkillDeleteIpcHandlers(store: Store, runtime?: OrcaRunti handleMainWindowSkillIpc( 'skills:delete', async (_event, value: unknown): Promise => { - const { request, target } = resolve(value) + const { request, target } = await resolve(value) return runSkillDeleteRequest(request, target, dependencies) } ) diff --git a/src/main/ipc/skills.test.ts b/src/main/ipc/skills.test.ts index 78b10bb7c4d..7d6d56b4c02 100644 --- a/src/main/ipc/skills.test.ts +++ b/src/main/ipc/skills.test.ts @@ -45,6 +45,7 @@ vi.mock('./skill-cloud-ipc-handlers', () => ({ vi.mock('../wsl', () => ({ getDefaultWslDistro: getDefaultWslDistroMock, getWslHome: getWslHomeMock, + getWslHomeAsync: async (distro: string) => getWslHomeMock(distro), parseWslPath: parseWslPathMock, toLinuxPath: (pathValue: string) => { if (pathValue === '\\\\wsl.localhost\\Ubuntu\\home\\alice') { diff --git a/src/main/ipc/skills.ts b/src/main/ipc/skills.ts index dce8fe675eb..34539eb9dfe 100644 --- a/src/main/ipc/skills.ts +++ b/src/main/ipc/skills.ts @@ -21,7 +21,7 @@ import { handleMainWindowSkillIpc } from './skill-ipc-main-window' export function registerSkillsHandlers(store: Store, runtime?: OrcaRuntimeService): void { const discover = async (target?: SkillDiscoveryTarget): Promise => { const parsedTarget = target ? SkillDiscoveryTargetSchema.parse(target) : undefined - const resolvedTarget = resolveSkillDiscoveryTarget(parsedTarget) + const resolvedTarget = await resolveSkillDiscoveryTarget(parsedTarget) return discoverSkillsOnTarget(resolvedTarget, store.getRepos(), { providerRootOverrides: await runtime?.resolveSkillDiscoveryProviderRoots(resolvedTarget), refresh: parsedTarget?.refresh === true diff --git a/src/main/ipc/worktree-logic-wsl.test.ts b/src/main/ipc/worktree-logic-wsl.test.ts index 2aa46a42ee8..a4c583db426 100644 --- a/src/main/ipc/worktree-logic-wsl.test.ts +++ b/src/main/ipc/worktree-logic-wsl.test.ts @@ -17,6 +17,7 @@ import { computeWorktreePath, computeWorktreePathAsync, computeWorkspaceRootAsync, + computeWorkspaceRootWithin, getWorktreePathSettings } from './worktree-logic' import { @@ -53,6 +54,28 @@ describe('computeWorktreePath WSL layout', () => { expect(getWslHomeMock).not.toHaveBeenCalled() }) + it('stops waiting for a WSL home at the deadline and falls back, leaving the probe running', async () => { + vi.useFakeTimers() + try { + parseWslPathMock.mockReturnValue({ distro: 'Ubuntu', linuxPath: '/home/jin/repo' }) + const repoPath = String.raw`\\wsl.localhost\Ubuntu\home\jin\repo` + const settings = { workspaceDir: 'C:\\workspaces', nestWorkspaces: true } + getWslHomeAsyncMock.mockReturnValue(new Promise(() => {})) + const pending = computeWorkspaceRootWithin(repoPath, settings, 5_000) + await vi.advanceTimersByTimeAsync(5_000) + const fallback = await pending + expect(fallback).not.toContain('orca') + expect(getWslHomeMock).not.toHaveBeenCalled() + + getWslHomeAsyncMock.mockResolvedValue(String.raw`\\wsl.localhost\Ubuntu\home\jin`) + expect(await computeWorkspaceRootWithin(repoPath, settings, 5_000)).toBe( + win32.join(String.raw`\\wsl.localhost\Ubuntu\home\jin`, 'orca', 'workspaces') + ) + } finally { + vi.useRealTimers() + } + }) + it('places WSL repo worktrees under the distro home workspace root', () => { parseWslPathMock.mockReturnValue({ distro: 'Ubuntu', diff --git a/src/main/ipc/worktree-logic.ts b/src/main/ipc/worktree-logic.ts index fb25a162b49..820c3a8ba40 100644 --- a/src/main/ipc/worktree-logic.ts +++ b/src/main/ipc/worktree-logic.ts @@ -114,6 +114,31 @@ export async function computeWorkspaceRootAsync( ) } +/** + * For authorization: never waits more than `deadlineMs` for a WSL home. Past it the root falls back + * as if the home were unknown, while the probe keeps running and fills the cache for later checks. + */ +export async function computeWorkspaceRootWithin( + repoPath: string, + settings: { workspaceDir: string; wslMirrorDistro?: string }, + deadlineMs: number +): Promise { + const distro = mirrorDistroForWorkspaceRoot(repoPath, settings) + if (!distro) { + return workspaceRootForMirrorHome(repoPath, settings.workspaceDir, null) + } + let timer: ReturnType | undefined + const deadline = new Promise((resolve) => { + timer = setTimeout(() => resolve(null), deadlineMs) + }) + try { + const home = await Promise.race([getWslHomeAsync(distro), deadline]) + return workspaceRootForMirrorHome(repoPath, settings.workspaceDir, home) + } finally { + clearTimeout(timer) + } +} + export function computeWorkspaceRoot( repoPath: string, settings: { workspaceDir: string; wslMirrorDistro?: string } diff --git a/src/main/runtime/rpc/methods/skills.test.ts b/src/main/runtime/rpc/methods/skills.test.ts index 097a948febf..dbd056b8011 100644 --- a/src/main/runtime/rpc/methods/skills.test.ts +++ b/src/main/runtime/rpc/methods/skills.test.ts @@ -259,7 +259,7 @@ describe('skills.share RPC', () => { beforeEach(() => { vi.mocked(discoverSkillsOnTarget).mockClear() - vi.mocked(resolveSkillDiscoveryTarget).mockImplementation((target) => ({ + vi.mocked(resolveSkillDiscoveryTarget).mockImplementation(async (target) => ({ kind: 'native-host', cwd: target?.cwd ?? undefined })) @@ -307,7 +307,7 @@ describe('skills.share RPC', () => { }) it('rejects WSL before attempting to read Linux paths with native fs', async () => { - vi.mocked(resolveSkillDiscoveryTarget).mockReturnValueOnce({ + vi.mocked(resolveSkillDiscoveryTarget).mockResolvedValueOnce({ kind: 'wsl', distro: 'Ubuntu', homeDir: '/home/alice', diff --git a/src/main/runtime/rpc/methods/skills.ts b/src/main/runtime/rpc/methods/skills.ts index b245cd84113..601e064ef7e 100644 --- a/src/main/runtime/rpc/methods/skills.ts +++ b/src/main/runtime/rpc/methods/skills.ts @@ -77,7 +77,7 @@ export const SKILL_METHODS = [ // Why: the executing runtime owns WSL project preferences. Remote callers // send worktree identity only; trusting their projectRuntime absence // would scan this host's native filesystem for a WSL-configured project. - const resolvedTarget = resolveDiscoveryTarget(params, runtime) + const resolvedTarget = await resolveDiscoveryTarget(params, runtime) return discoverSkillsOnTarget(resolvedTarget, runtime.listRepos(), { providerRootOverrides: await runtime.resolveSkillDiscoveryProviderRoots(resolvedTarget), refresh: params.refresh === true @@ -91,7 +91,7 @@ export const SKILL_METHODS = [ handler: async (params, { runtime }) => previewSkillDeleteRequest( params, - resolveDiscoveryTarget(params.target ?? {}, runtime), + await resolveDiscoveryTarget(params.target ?? {}, runtime), skillDeleteDependencies(runtime) ) }), @@ -102,7 +102,7 @@ export const SKILL_METHODS = [ handler: async (params, { runtime }) => runSkillDeleteRequest( params, - resolveDiscoveryTarget(params.target ?? {}, runtime), + await resolveDiscoveryTarget(params.target ?? {}, runtime), skillDeleteDependencies(runtime) ) }), @@ -131,7 +131,7 @@ export const SKILL_METHODS = [ 'Publishing skills through a paired client is not supported. Run the command from Orca on the machine that stores the skills.' ) } - const resolvedTarget = resolveDiscoveryTarget(params.target ?? {}, runtime) + const resolvedTarget = await resolveDiscoveryTarget(params.target ?? {}, runtime) if (resolvedTarget.kind !== 'native-host') { throw new AgentSkillSharingError( AGENT_SKILL_SHARING_UNSUPPORTED_ENVIRONMENT_CODE, diff --git a/src/main/skills/skill-discovery-target.ts b/src/main/skills/skill-discovery-target.ts index 55c83cd94e5..7026ddf810c 100644 --- a/src/main/skills/skill-discovery-target.ts +++ b/src/main/skills/skill-discovery-target.ts @@ -4,7 +4,7 @@ import { } from './skill-discovery-wsl-observation' import type { Repo } from '../../shared/repo-types' import type { SkillDiscoveryResult, SkillDiscoveryTarget } from '../../shared/skills' -import { getDefaultWslDistro, getWslHome, parseWslPath, toLinuxPath } from '../wsl' +import { getDefaultWslDistro, getWslHomeAsync, parseWslPath, toLinuxPath } from '../wsl' import { clearSkillRootScanCache, discoverSkills } from './discovery' import { discoverSkillObservationInWsl } from './skill-discovery-wsl' import type { SkillProviderRootOverrides } from './skill-provider-destinations' @@ -46,9 +46,9 @@ export type ResolvedSkillDiscoveryTarget = sourceKinds?: SkillDiscoveryTarget['sourceKinds'] } -export function resolveSkillDiscoveryTarget( +export async function resolveSkillDiscoveryTarget( target: SkillDiscoveryTarget | undefined -): ResolvedSkillDiscoveryTarget { +): Promise { const projectRuntime = target?.projectRuntime if (projectRuntime?.status === 'repair-required') { throw new Error( @@ -79,7 +79,8 @@ export function resolveSkillDiscoveryTarget( if (process.platform !== 'win32') { throw new Error('WSL skill discovery is only available on Windows.') } - const homeDir = getWslHome(wslDistro) + // Why async: a synchronous probe blocks main and, on a booting distro, kills its first boot. + const homeDir = await getWslHomeAsync(wslDistro) if (!homeDir) { throw new Error(`Could not resolve the WSL home directory for ${wslDistro}.`) } diff --git a/src/main/wsl.test.ts b/src/main/wsl.test.ts index 803be2a6692..3cc8fd723f0 100644 --- a/src/main/wsl.test.ts +++ b/src/main/wsl.test.ts @@ -36,6 +36,7 @@ import { getCachedWslAvailability, getCachedWslDistros, getWslHome, + getWslHomeAsync, hasCachedWslAvailability, hasCachedWslDistros, isWslAvailableAsync, @@ -309,6 +310,34 @@ describe('WSL home cache', () => { }) }) +describe('WSL home probe on a booting distro', () => { + afterEach(() => { + runProcessMock.mockReset() + runProcessSyncMock.mockReset() + _resetWslCachesForTests() + }) + + // Real host (win-lowspec): a 5 s kill mid-boot left the distro failing every wsl.exe call. + it('gives the async home probe time for a cold boot instead of killing it at 5 s', async () => { + runProcessMock.mockResolvedValueOnce(exited('/home/alice\n')) + await withPlatformAsync('win32', () => getWslHomeAsync('Ubuntu')) + expect(runProcessMock.mock.calls[0]?.[0]).toMatchObject({ program: 'wsl.exe' }) + expect(runProcessMock.mock.calls[0]?.[0]?.timeoutMs).toBeGreaterThanOrEqual(30_000) + }) + + it('never starts a 5 s synchronous probe while the async one waits out a boot', async () => { + let finish!: (result: ProcessResult) => void + runProcessMock.mockReturnValueOnce(new Promise((resolve) => (finish = resolve))) + await withPlatformAsync('win32', async () => { + const pending = getWslHomeAsync('Ubuntu') + expect(getWslHome('Ubuntu')).toBeNull() + expect(runProcessSyncMock).not.toHaveBeenCalled() + finish(exited('/home/alice\n')) + expect(await pending).toContain('alice') + }) + }) +}) + describe('WSL availability cache', () => { afterEach(() => { runProcessMock.mockReset() diff --git a/src/main/wsl.ts b/src/main/wsl.ts index a8c357d46eb..87efad75e09 100644 --- a/src/main/wsl.ts +++ b/src/main/wsl.ts @@ -32,6 +32,10 @@ export type WslPathInfo = { } const WSL_PROBE_TIMEOUT_MS = 5000 +// Measured on a slow PC: a cold systemd distro takes 4-11 s to answer its first exec, and killing +// wsl.exe mid-boot leaves the distro failing every call with Wsl/Service/E_UNEXPECTED for 14+ s. +// An async probe blocks nothing, so it waits out the boot. +const WSL_HOME_PROBE_ASYNC_TIMEOUT_MS = 60_000 function wslHomeProbeArgs(distro: string): string[] { return buildWslExecArgs(distro, ['bash', '-c', 'echo $HOME']) @@ -214,6 +218,10 @@ export function getWslHome(distro: string): string | null { if (cachedHome !== undefined) { return cachedHome } + // A second, 5 s-bounded probe would kill a boot the in-flight one is waiting out. + if (wslHomeProbeCache.has(distro)) { + return null + } try { const home = wslExeStdout( @@ -242,7 +250,7 @@ export async function getWslHomeAsync(distro: string): Promise { return inflight } - const probe = runWslExe(wslHomeProbeArgs(distro)) + const probe = runWslExe(wslHomeProbeArgs(distro), undefined, WSL_HOME_PROBE_ASYNC_TIMEOUT_MS) .then((output) => { const home = output.trim() if (!home || !home.startsWith('/')) { @@ -301,6 +309,10 @@ export function _setWslCachesForTests(args: { } } -async function runWslExe(args: string[], env?: NodeJS.ProcessEnv): Promise { - return wslExeStdout(await runProcess(wslExeProcessSpec(args, WSL_PROBE_TIMEOUT_MS, env))) +async function runWslExe( + args: string[], + env?: NodeJS.ProcessEnv, + timeoutMs = WSL_PROBE_TIMEOUT_MS +): Promise { + return wslExeStdout(await runProcess(wslExeProcessSpec(args, timeoutMs, env))) }