mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 16:03:47 +00:00
revert: drop the worktree-create error-context wrapper
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.
This commit is contained in:
@@ -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<Repo> = {}): Pick<Repo, 'kind' | 'connectionId' | 'path'> {
|
||||
return {
|
||||
kind: 'git',
|
||||
connectionId: undefined,
|
||||
path: '/home/neil/projects/orca',
|
||||
...overrides
|
||||
} as Pick<Repo, 'kind' | 'connectionId' | 'path'>
|
||||
}
|
||||
|
||||
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)
|
||||
})
|
||||
})
|
||||
@@ -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<Repo, 'kind' | 'connectionId'>
|
||||
): 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<Repo, 'kind' | 'connectionId' | 'path'>
|
||||
): 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
|
||||
}
|
||||
@@ -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', {
|
||||
|
||||
Reference in New Issue
Block a user