From c9563e2cf3b74c7b2bec1aa1acf99f8376f9f61d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 16 Aug 2026 22:09:13 -0700 Subject: [PATCH] revert: drop the worktree-create error-context wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Written to make an unexplained ENOENT self-identifying when creating a worktree over SSH. The cause is now known and fixed — the error came back from the RELAY and isENOENT could not recognise it, because the multiplexer rebuilds a remote error with the transport's numeric code — so the wrapper is scaffolding for a solved problem. Worse, its central claim is false. It reported 'the remote (SSH) path failed on a local filesystem call', and the trace log shows the lstat ran on the SSH host, not locally. Keeping a message that asserts the wrong thing about the one failure it was built for is worse than not having it. 183 lines and a rewritten error at the IPC boundary, removed. --- .../ipc/worktree-create-error-context.test.ts | 115 ------------------ src/main/ipc/worktree-create-error-context.ts | 68 ----------- src/main/ipc/worktrees.ts | 13 +- 3 files changed, 2 insertions(+), 194 deletions(-) delete mode 100644 src/main/ipc/worktree-create-error-context.test.ts delete mode 100644 src/main/ipc/worktree-create-error-context.ts diff --git a/src/main/ipc/worktree-create-error-context.test.ts b/src/main/ipc/worktree-create-error-context.test.ts deleted file mode 100644 index 3065eac83eb..00000000000 --- a/src/main/ipc/worktree-create-error-context.test.ts +++ /dev/null @@ -1,115 +0,0 @@ -import { describe, expect, it } from 'vitest' - -import { - describeWorktreeCreateFailure, - resolveWorktreeCreateRoute -} from './worktree-create-error-context' -import type { Repo } from '../../shared/repo-types' - -/** - * A worktree create over SSH failed with a bare - * `ENOENT: no such file or directory, lstat '/home/neil/projects/orca-test1234'`. - * - * That message names nothing useful. An lstat is Node's LOCAL filesystem, so hitting one against a - * path that lives on an SSH host means creation ran a local implementation for a remote repo — but - * the user could not know that, and neither could we without re-deriving the routing by hand. The - * route is knowable at the throw site, so it should be stated there. - */ -function repo(overrides: Partial = {}): Pick { - return { - kind: 'git', - connectionId: undefined, - path: '/home/neil/projects/orca', - ...overrides - } as Pick -} - -function fsError(code: string, syscall = 'lstat'): NodeJS.ErrnoException { - const error = new Error( - `${code}: no such file or directory, ${syscall} '/home/neil/projects/orca-test1234'` - ) as NodeJS.ErrnoException - error.code = code - error.syscall = syscall - return error -} - -describe('resolveWorktreeCreateRoute', () => { - it('sends a folder repo to the folder route even when it has a connection', () => { - // Load-bearing ordering: the handler checks isFolderRepo BEFORE connectionId, so a folder repo - // on an SSH host never reaches the remote implementation. - expect(resolveWorktreeCreateRoute({ kind: 'folder', connectionId: 'ssh-1' } as never)).toBe( - 'folder' - ) - }) - - it('sends a git repo with a connection to the remote route', () => { - expect(resolveWorktreeCreateRoute({ kind: 'git', connectionId: 'ssh-1' } as never)).toBe( - 'remote' - ) - }) - - it('sends a git repo without a connection to the local route', () => { - expect(resolveWorktreeCreateRoute({ kind: 'git', connectionId: undefined } as never)).toBe( - 'local' - ) - }) -}) - -describe('describeWorktreeCreateFailure', () => { - it('calls out a remote repo that failed on a local filesystem call', () => { - const described = describeWorktreeCreateFailure( - fsError('ENOENT'), - repo({ connectionId: 'ssh-1' }) - ) - - expect(described).toBeInstanceOf(Error) - const message = (described as Error).message - expect(message).toContain('remote (SSH) path failed on a local filesystem call') - expect(message).toContain('ENOENT') - expect(message).toContain('lstat') - expect(message).toContain('connection=ssh-1') - expect(message).toContain('/home/neil/projects/orca') - }) - - it('names the local route when a repo has no connection', () => { - const described = describeWorktreeCreateFailure(fsError('ENOENT'), repo()) - - expect((described as Error).message).toContain('local path ran a local filesystem call') - expect((described as Error).message).toContain('connection=none') - }) - - it('names the folder route, which ignores the connection entirely', () => { - const described = describeWorktreeCreateFailure( - fsError('EACCES'), - repo({ kind: 'folder', connectionId: 'ssh-1' }) - ) - - expect((described as Error).message).toContain('folder path ran a local filesystem call') - expect((described as Error).message).toContain('EACCES') - }) - - it('keeps the original error as the cause', () => { - // Anything matching on `code` or reading the stack must keep working; this only adds context. - const original = fsError('ENOENT') - const described = describeWorktreeCreateFailure(original, repo({ connectionId: 'ssh-1' })) - - expect((described as Error).cause).toBe(original) - expect((described as Error).message).toContain(original.message) - }) - - it('leaves a non-filesystem failure completely untouched', () => { - // Git failures, relay-not-ready, validation errors — all already say what went wrong, and - // rewriting them would bury a good message under a worse one. - const gitFailure = new Error('Remote connection dropped. Click Reconnect on the SSH target.') - - expect(describeWorktreeCreateFailure(gitFailure, repo({ connectionId: 'ssh-1' }))).toBe( - gitFailure - ) - }) - - it('leaves an fs error with an unrelated code untouched', () => { - const busy = fsError('EBUSY') - - expect(describeWorktreeCreateFailure(busy, repo({ connectionId: 'ssh-1' }))).toBe(busy) - }) -}) diff --git a/src/main/ipc/worktree-create-error-context.ts b/src/main/ipc/worktree-create-error-context.ts deleted file mode 100644 index f3d678a5454..00000000000 --- a/src/main/ipc/worktree-create-error-context.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { isFolderRepo } from '../../shared/repo-kind' -import type { Repo } from '../../shared/repo-types' - -/** Which of the three create implementations a repo routes to. */ -export type WorktreeCreateRoute = 'folder' | 'remote' | 'local' - -export function resolveWorktreeCreateRoute( - repo: Pick -): WorktreeCreateRoute { - if (isFolderRepo(repo)) { - return 'folder' - } - return repo.connectionId ? 'remote' : 'local' -} - -/** Node fs errors carry a syscall and the path it was attempted on. */ -function localFilesystemFailure(error: unknown): { code: string; syscall?: string } | null { - if (!(error instanceof Error) || !('code' in error)) { - return null - } - const candidate = error as NodeJS.ErrnoException - if (candidate.code !== 'ENOENT' && candidate.code !== 'EACCES' && candidate.code !== 'EPERM') { - return null - } - return { - code: candidate.code, - ...(candidate.syscall ? { syscall: candidate.syscall } : {}) - } -} - -/** - * Name the route when a raw filesystem error escapes worktree creation. - * - * A bare `ENOENT: no such file or directory, lstat '/home/user/...'` tells a user nothing and told - * us almost nothing: an lstat is Node's LOCAL filesystem, so seeing one against a path that lives on - * an SSH host means creation ran the local implementation for a remote repo. Which of the three - * routes was taken, and why, is the whole diagnosis — and it is knowable exactly here, where the - * routing decision was just made. - * - * Deliberately additive: the original error stays the cause, so nothing that matches on `code` or - * inspects the stack changes behaviour. This only replaces an opaque message with one that says - * which implementation ran and what the repo looked like when it was chosen. - */ -export function describeWorktreeCreateFailure( - error: unknown, - repo: Pick -): unknown { - const failure = localFilesystemFailure(error) - if (!failure) { - return error - } - const route = resolveWorktreeCreateRoute(repo) - // A remote route failing on local fs is the contradiction worth stating outright; the other routes - // still benefit from naming themselves, because "which one ran" is the first question either way. - const summary = - route === 'remote' - ? 'the remote (SSH) path failed on a local filesystem call' - : `the ${route} path ran a local filesystem call` - const described = new Error( - `Workspace creation failed: ${summary} (${failure.code}${ - failure.syscall ? ` on ${failure.syscall}` : '' - }). Repo kind=${repo.kind ?? 'git'}, connection=${repo.connectionId ?? 'none'}, path=${repo.path}. Original: ${ - error instanceof Error ? error.message : String(error) - }`, - { cause: error } - ) - return described -} diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 096446f1c01..beebaf70058 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -108,7 +108,6 @@ import { hasUnrecognizedOrcaYamlKeys } from '../hooks' import { createIssueCommandRunnerScript, resolveSetupRunnerShell } from '../worktree-runner-script' -import { describeWorktreeCreateFailure } from './worktree-create-error-context' import { getSetupRunnerEnvVars } from '../setup-hook-env-vars' import { getEffectiveHooksFromConfig } from '../effective-hook-config' import { readIssueCommand, writeIssueCommand } from '../issue-command-file' @@ -2278,11 +2277,7 @@ export function registerWorktreeHandlers( error_class: classifyWorkspaceCreateError(error), ...getCohortAtEmit() }) - // Why here: the routing decision was just made above, so this is the only place that can - // say which implementation ran. A raw ENOENT from an lstat reached a user naming neither - // the route nor the repo shape, which cost a full investigation and still did not identify - // it. Additive only — the original error stays the cause. - throw describeWorktreeCreateFailure(error, repo) + throw error } finishAutomationWorkspaceProvenanceRequest(args.automationProvenanceRequest) @@ -2345,11 +2340,7 @@ export function registerWorktreeHandlers( error_class: classifyWorkspaceCreateError(error), ...getCohortAtEmit() }) - // Why here: the routing decision was just made above, so this is the only place that can - // say which implementation ran. A raw ENOENT from an lstat reached a user naming neither - // the route nor the repo shape, which cost a full investigation and still did not identify - // it. Additive only — the original error stays the cause. - throw describeWorktreeCreateFailure(error, repo) + throw error } finishAutomationWorkspaceProvenanceRequest(args.automationProvenanceRequest) track('workspace_created', {