diff --git a/packages/code/src/linked-worktrees.test.ts b/packages/code/src/linked-worktrees.test.ts index 53ec156e..30c5b7e6 100644 --- a/packages/code/src/linked-worktrees.test.ts +++ b/packages/code/src/linked-worktrees.test.ts @@ -225,6 +225,59 @@ test('lane file tools are confined to the worktree and report the public workspa ); }); +test('lane writes create missing parents inside the worktree and report missing files', async (t) => { + const { parent, root } = await checkout(); + t.after(() => rm(parent, { recursive: true, force: true })); + const tools = await laneTools(root); + const lane = join(root, '.worktrees', 'task-a'); + + await tools.execute({ + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'repo', + worktree: 'task-a', + path: 'packages/new-package/package.json', + content: '{}\n', + }); + assert.equal(await readFile(join(lane, 'packages', 'new-package', 'package.json'), 'utf8'), '{}\n'); + await assert.rejects(stat(join(root, 'packages'))); + + await rejects( + tools.execute({ + protocolVersion: 1, + operation: 'read_file', + workspaceId: 'repo', + worktree: 'task-a', + path: '.checks/tests.log', + }), + 'NOT_FOUND', + ); + + await symlink(root, join(lane, 'checkout-link')); + await rejects( + tools.execute({ + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'repo', + worktree: 'task-a', + path: 'checkout-link/escaped/notes.txt', + content: 'blocked', + }), + 'INVALID_PATH', + ); + await rejects( + tools.execute({ + protocolVersion: 1, + operation: 'read_file', + workspaceId: 'repo', + worktree: 'task-a', + path: 'checkout-link/missing.txt', + }), + 'INVALID_PATH', + ); + await assert.rejects(stat(join(root, 'escaped'))); +}); + test('lane commands register a confined root once, whatever siblings come and go', async (t) => { const { parent, root } = await checkout(); t.after(() => rm(parent, { recursive: true, force: true })); diff --git a/packages/code/src/protocol.test.ts b/packages/code/src/protocol.test.ts index 340ddfdf..25c00cf3 100644 --- a/packages/code/src/protocol.test.ts +++ b/packages/code/src/protocol.test.ts @@ -8,8 +8,10 @@ import { isSupportedBridgeArtifactName, isValidBridgeWorkerCapabilities, isValidBridgeWorkerId, + isWorkspaceToolErrorCode, isWorkspaceToolRequest, isWorkspaceToolResult, + WORKSPACE_TOOL_ERROR_CODE_FALLBACKS, workspaceIsolationKey, workspaceIsolationKeysConflict, workspaceIsolationParent, @@ -19,6 +21,17 @@ import type { WorkspacePreviewEditRequest, } from './protocol.js'; +test('every added workspace error code falls back to a legacy code', () => { + assert.equal(isWorkspaceToolErrorCode('NOT_FOUND'), true); + assert.equal(isWorkspaceToolErrorCode('MISSING'), false); + for (const [code, legacy] of Object.entries(WORKSPACE_TOOL_ERROR_CODE_FALLBACKS)) { + assert.equal(isWorkspaceToolErrorCode(code), true); + assert.equal(isWorkspaceToolErrorCode(legacy), true); + assert.equal(legacy in WORKSPACE_TOOL_ERROR_CODE_FALLBACKS, false); + } + assert.equal(WORKSPACE_TOOL_ERROR_CODE_FALLBACKS.NOT_FOUND, 'INVALID_PATH'); +}); + test('accepts gateway directory markers as artifacts', () => { assert.equal(isSupportedBridgeArtifactName('.dirkeep'), true); assert.equal(isSupportedBridgeArtifactName('nested/.dirkeep'), true); diff --git a/packages/code/src/protocol.ts b/packages/code/src/protocol.ts index 26111273..216784a4 100644 --- a/packages/code/src/protocol.ts +++ b/packages/code/src/protocol.ts @@ -836,6 +836,8 @@ export interface BridgeWorkerRegistrationResponse { supportedWorkspaceInstanceTypes?: ['git_worktree']; /** Scheduling scopes this Code API can admit as independent lanes. */ supportedWorkspaceScopes?: ['git_linked_worktree']; + /** Added workspace tool error codes this Code API accepts in settlements. */ + supportedWorkspaceToolErrorCodes?: WorkspaceToolErrorCode[]; } /** Administrator-visible liveness for a configured worker. Credentials, @@ -976,6 +978,7 @@ export interface BridgeRejectedSettlement { export type WorkspaceToolErrorCode = | 'INVALID_PATH' + | 'NOT_FOUND' | 'INVALID_REQUEST' | 'READ_LIMIT_EXCEEDED' | 'WRITE_LIMIT_EXCEEDED' @@ -994,6 +997,7 @@ export type WorkspaceToolErrorCode = const WORKSPACE_TOOL_ERROR_CODES = new Set([ 'INVALID_PATH', + 'NOT_FOUND', 'INVALID_REQUEST', 'READ_LIMIT_EXCEEDED', 'WRITE_LIMIT_EXCEEDED', @@ -1011,6 +1015,15 @@ const WORKSPACE_TOOL_ERROR_CODES = new Set([ 'COMMAND_DISABLED', ]); +/** + * Codes added after the original settlement contract, each with the legacy code + * an older Code API accepts in its place. A worker reports an added code only + * after registration advertises it, and falls back if a settlement is refused. + */ +export const WORKSPACE_TOOL_ERROR_CODE_FALLBACKS: Readonly< + Partial> +> = { NOT_FOUND: 'INVALID_PATH' }; + export function isWorkspaceToolErrorCode( value: unknown, ): value is WorkspaceToolErrorCode { diff --git a/packages/code/src/root-access.ts b/packages/code/src/root-access.ts index b06de8d0..5bdddca9 100644 --- a/packages/code/src/root-access.ts +++ b/packages/code/src/root-access.ts @@ -350,7 +350,7 @@ export class WorkspaceRootAccess { } } - entry(path: string, operation: 'mkdir' | 'symlink' | 'readlink', target?: string): string | void { + entry(path: string, operation: 'mkdir' | 'symlink' | 'readlink', target?: string, mode = 0o700): string | void { const parent = this.parent(path); try { if (operation === 'readlink') { @@ -361,7 +361,7 @@ export class WorkspaceRootAccess { return buffer.subarray(0, length).toString(); } const result = operation === 'mkdir' - ? mkdirAt!(parent.fd, parent.name, 0o700) + ? mkdirAt!(parent.fd, parent.name, mode) : symlinkAt!(target!, parent.fd, parent.name); if (result !== 0) throw nativeError(); } finally { closeSync(parent.fd); } @@ -378,6 +378,8 @@ export class WorkspaceRootAccess { } const context = new AsyncLocalStorage(); +/** Whether filesystem adapters in this call are anchored to a held root descriptor. */ +export const holdsWorkspaceRoot = (): boolean => context.getStore() != null; export async function withWorkspaceRoot( root: string, identity: WorkspaceRootIdentity | undefined, @@ -504,10 +506,10 @@ export const readdir = async (path: string, maxEntries = 200_000): Promise => context.getStore()?.entry(path, 'readlink') as string ?? fs.readlink(path); -export const mkdir = async (path: string): Promise => { +export const mkdir = async (path: string, mode = 0o700): Promise => { const access = context.getStore(); - if (access) access.entry(path, 'mkdir'); - else await fs.mkdir(path, { mode: 0o700 }); + if (access) access.entry(path, 'mkdir', undefined, mode); + else await fs.mkdir(path, { mode }); }; export const symlink = async (target: string, path: string): Promise => { const access = context.getStore(); diff --git a/packages/code/src/worker.ts b/packages/code/src/worker.ts index 217ca2c7..17a9a6e1 100644 --- a/packages/code/src/worker.ts +++ b/packages/code/src/worker.ts @@ -11,6 +11,7 @@ import { workspaceIsolationKey, workspaceIsolationKeysConflict, workspaceIsolationParent, + WORKSPACE_TOOL_ERROR_CODE_FALLBACKS, } from './protocol.js'; import { EndpointRuntimeSupervisor } from './runtime.js'; import { signBridgeRequest } from './identity.js'; @@ -28,6 +29,7 @@ import type { BridgeWorkspaceToolOperation, BridgeWorkspaceProgrammaticRequest, RepositoryInstructionDescriptor, + WorkspaceToolErrorCode, } from './protocol.js'; import type { RuntimeLease, RuntimeSupervisor } from './runtime.js'; import type { WorkspaceToolExecutor } from './workspace.js'; @@ -464,6 +466,8 @@ export class BridgeWorker { private registrationCapabilities: BridgeWorkerCapabilities; private activeCapabilities: BridgeWorkerCapabilities; private instructionMetadataSupported = true; + /** Added error codes the current Code API registration accepts in settlements. */ + private settlementErrorCodes: ReadonlySet = new Set(); private registrationTtlMs = DEFAULT_REGISTRATION_TTL_MS; private lastRegisteredAtMs = 0; private maintenanceOnly = false; @@ -803,6 +807,11 @@ export class BridgeWorker { } this.registrationTtlMs = registration.leaseTtlMs; this.activeCapabilities = this.registrationCapabilities; + this.settlementErrorCodes = new Set( + Array.isArray(registration.supportedWorkspaceToolErrorCodes) + ? registration.supportedWorkspaceToolErrorCodes + : [], + ); await this.options.onRegistered?.(registration); if ( !this.maintenanceOnly && @@ -2085,7 +2094,7 @@ export class BridgeWorker { ...((assignment.executionKind === 'workspace_tool' || assignment.executionKind === 'workspace_programmatic') && error instanceof WorkspaceToolError - ? { errorCode: error.code } + ? { errorCode: this.settlementErrorCode(error.code) } : {}), error: (assignment.executionKind === 'workspace_tool' && isWorkspaceToolRequest(assignment.request) && @@ -2474,6 +2483,12 @@ export class BridgeWorker { ); } + /** An older Code API refuses a settlement carrying a code it does not know. */ + private settlementErrorCode(code: WorkspaceToolErrorCode): WorkspaceToolErrorCode { + if (this.settlementErrorCodes.has(code)) return code; + return WORKSPACE_TOOL_ERROR_CODE_FALLBACKS[code] ?? code; + } + private async settleWithRetry( assignment: BridgeAssignment, settlement: BridgeSettlement, @@ -2525,6 +2540,20 @@ export class BridgeWorker { } catch (error) { lastError = error; if (signal?.aborted) break; + const legacyErrorCode = + settlement.status === 'rejected' && settlement.errorCode != null + ? WORKSPACE_TOOL_ERROR_CODE_FALLBACKS[settlement.errorCode] + : undefined; + if ( + settlement.status === 'rejected' && + legacyErrorCode != null && + error instanceof BridgeProtocolError && + error.status === 400 + ) { + /** A replica that predates the added code can still serve this settlement. */ + settlement = { ...settlement, errorCode: legacyErrorCode }; + continue; + } if ( error instanceof BridgeProtocolError && error.status != null && diff --git a/packages/code/src/workspace-worker.test.ts b/packages/code/src/workspace-worker.test.ts index 71ef2cd3..99fcdd02 100644 --- a/packages/code/src/workspace-worker.test.ts +++ b/packages/code/src/workspace-worker.test.ts @@ -237,6 +237,112 @@ test('worker omits pagination fields until Code API negotiates them', async () = }); }); +async function settleMissingFile( + registeredCodes: string[] | undefined, + rejectSettlement: (body: Record) => boolean = () => false, +): Promise<{ settlements: Array>; failure?: unknown }> { + const settlements: Array> = []; + const workspaceCapabilities = { + protocolVersion: 1 as const, + operations: ['read_file' as const], + workspaces: [{ id: 'primary' }], + }; + const worker = new BridgeWorker({ + codeApiUrl: 'https://code.example/v1', + token: 'worker-secret', + workerId: 'vm-1', + incarnationId, + sandboxEndpoint: 'http://127.0.0.1:2000/api/v2', + capabilities: { + statefulWorkspace: false, + sandboxProfile: 'nsjail', + runtimes: [], + workspaceTools: workspaceCapabilities, + }, + workspaceTools: { + capabilities: workspaceCapabilities, + async execute() { + throw new WorkspaceToolError('Workspace path does not exist', 'NOT_FOUND'); + }, + }, + fetchImpl: async (input, init) => { + if (!String(input).endsWith('/settle')) { + return Response.json({ + protocolVersion: 1, + workerId: 'vm-1', + incarnationId, + registeredAt: new Date().toISOString(), + leaseTtlMs: 60_000, + supportedWorkspaceToolOperations: ['read_file'], + ...(registeredCodes ? { supportedWorkspaceToolErrorCodes: registeredCodes } : {}), + }); + } + const body = JSON.parse(String(init?.body)) as Record; + settlements.push(body); + if (rejectSettlement(body)) { + return Response.json({ error: 'Invalid bridge settlement' }, { status: 400 }); + } + return Response.json({ protocolVersion: 1, accepted: true }); + }, + }); + + await worker.register(); + const failure = await worker.executeAndSettle({ + protocolVersion: 1, + assignmentId: 'assignment-missing-file', + workerId: 'vm-1', + incarnationId, + generation: 1, + leaseToken: 'lease-token-that-is-long-enough-for-testing', + expiresAt: new Date(Date.now() + 5_000).toISOString(), + executionKind: 'workspace_tool', + request: { + protocolVersion: 1, + operation: 'read_file', + workspaceId: 'primary', + path: 'logs/pending.log', + }, + }).then(() => undefined, (error: unknown) => error); + return { settlements, failure }; +} + +test('worker reports NOT_FOUND only to a Code API that advertised it', async () => { + const { settlements: supported } = await settleMissingFile(['NOT_FOUND']); + assert.deepEqual( + supported.map(({ status, errorCode, error }) => ({ status, errorCode, error })), + [{ status: 'rejected', errorCode: 'NOT_FOUND', error: 'Workspace path does not exist' }], + ); + + for (const registeredCodes of [undefined, [], ['SOMETHING_ELSE']]) { + const { settlements: legacy } = await settleMissingFile(registeredCodes); + assert.deepEqual( + legacy.map(({ status, errorCode, error }) => ({ status, errorCode, error })), + [{ status: 'rejected', errorCode: 'INVALID_PATH', error: 'Workspace path does not exist' }], + ); + } +}); + +test('worker resends a refused NOT_FOUND settlement with its legacy code', async () => { + const { settlements, failure } = await settleMissingFile( + ['NOT_FOUND'], + (body) => body.errorCode === 'NOT_FOUND', + ); + + assert.equal(failure, undefined); + assert.deepEqual( + settlements.map(({ errorCode }) => errorCode), + ['NOT_FOUND', 'INVALID_PATH'], + ); +}); + +test('worker does not resend a refused legacy settlement', async () => { + const { settlements, failure } = await settleMissingFile(undefined, () => true); + + assert.deepEqual(settlements.map(({ errorCode }) => errorCode), ['INVALID_PATH']); + assert.ok(failure instanceof BridgeProtocolError); + assert.equal(failure.status, 400); +}); + test('worker omits restricted workspaces that legacy registration would widen', async () => { const registrations: Array<{ operations: string[]; diff --git a/packages/code/src/workspace.test.ts b/packages/code/src/workspace.test.ts index 4a0ecb3e..787fbe7d 100644 --- a/packages/code/src/workspace.test.ts +++ b/packages/code/src/workspace.test.ts @@ -15,6 +15,7 @@ import { unlink, writeFile, } from 'node:fs/promises'; +import { existsSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, sep } from 'node:path'; import test from 'node:test'; @@ -28,6 +29,7 @@ import { SandboxWorkspaceTools, WorkspaceToolError, } from './workspace.js'; +import { captureWorkspaceRootIdentity } from './root-identity.js'; import { BRIDGE_WORKSPACE_COMMAND_MAX_BYTES, @@ -1737,7 +1739,7 @@ test('oversized replaceAll previews and edits fail before writing the source fil } }); -test('writes reject symlink targets and missing parent directories', async (t) => { +test('writes reject symlink targets and symlinked parent directories', async (t) => { const parent = await mkdtemp(join(tmpdir(), 'librechat-code-workspace-')); t.after(() => rm(parent, { recursive: true, force: true })); const root = join(parent, 'root'); @@ -1752,8 +1754,8 @@ test('writes reject symlink targets and missing parent directories', async (t) = for (const path of [ 'link.txt', - 'missing/notes.txt', 'directory-link/notes.txt', + 'directory-link/missing/notes.txt', ]) { await assert.rejects( tools.execute({ @@ -1768,6 +1770,231 @@ test('writes reject symlink targets and missing parent directories', async (t) = ); } assert.equal(await readFile(join(parent, 'outside.txt'), 'utf8'), 'outside'); + assert.deepEqual(await readdir(join(root, 'real-directory')), []); +}); + +async function heldRootTools(root: string): Promise { + return LocalWorkspaceTools.create({ + workspaces: [ + { id: 'primary', root, identity: await captureWorkspaceRootIdentity(root), writable: true }, + ], + }); +} + +test('writes create missing parent directories inside a held workspace root', async (t) => { + const root = await realpath(await mkdtemp(join(tmpdir(), 'librechat-code-workspace-'))); + t.after(() => rm(root, { recursive: true, force: true })); + await mkdir(join(root, 'src')); + const tools = await heldRootTools(root); + + for (const [path, overwrite] of [ + ['src/feature/deep/new.ts', undefined], + ['fresh/created.ts', false], + ] as const) { + const result = await tools.execute({ + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'primary', + path, + content: 'export {};\n', + ...(overwrite === undefined ? {} : { overwrite }), + }); + assert.deepEqual(result, { + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'primary', + path, + created: true, + bytesWritten: 11, + }); + assert.equal(await readFile(join(root, ...path.split('/')), 'utf8'), 'export {};\n'); + } + assert.equal((await stat(join(root, 'src', 'feature', 'deep'))).isDirectory(), true); +}); + +test('parent creation never follows symlinks, crosses files, or leaves the workspace', async (t) => { + const parent = await realpath( + await mkdtemp(join(tmpdir(), 'librechat-code-workspace-parent-')), + ); + t.after(() => rm(parent, { recursive: true, force: true })); + const root = join(parent, 'root'); + const outside = join(parent, 'outside'); + await mkdir(join(root, 'src'), { recursive: true }); + await mkdir(outside); + await writeFile(join(root, 'file.txt'), 'file'); + await symlink(outside, join(root, 'linked-outside')); + await symlink(join(root, 'src'), join(root, 'alias')); + await symlink(join(parent, 'missing-target'), join(root, 'dangling')); + const tools = await heldRootTools(root); + + for (const [path, code] of [ + ['linked-outside/new/notes.txt', 'INVALID_PATH'], + ['alias/new/notes.txt', 'INVALID_PATH'], + ['dangling/new/notes.txt', 'INVALID_PATH'], + ['file.txt/new/notes.txt', 'INVALID_PATH'], + ['../outside/new/notes.txt', 'INVALID_REQUEST'], + ['new/../../outside/notes.txt', 'INVALID_REQUEST'], + ] as const) { + await assert.rejects( + tools.execute({ + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'primary', + path, + content: 'blocked', + }), + (error: unknown) => + error instanceof WorkspaceToolError && + error.code === code && + !error.mutationMayHaveCommitted, + path, + ); + } + assert.deepEqual(await readdir(outside), []); + assert.deepEqual(await readdir(join(root, 'src')), []); + assert.deepEqual((await readdir(root)).sort(), [ + 'alias', + 'dangling', + 'file.txt', + 'linked-outside', + 'src', + ]); + await assert.rejects(stat(join(parent, 'missing-target'))); +}); + +test('a write finishes once it has created parents, even if cancelled', async (t) => { + const root = await realpath(await mkdtemp(join(tmpdir(), 'librechat-code-workspace-'))); + t.after(() => rm(root, { recursive: true, force: true })); + const tools = await heldRootTools(root); + /** Reports cancellation as soon as the parents exist. */ + const signal = { + get aborted() { + return existsSync(join(root, 'fresh', 'nested')); + }, + } as AbortSignal; + + const result = await tools.execute( + { + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'primary', + path: 'fresh/nested/file.txt', + content: 'installed', + }, + signal, + ); + + assert.equal(result.operation === 'write_file' && result.created, true); + assert.equal(await readFile(join(root, 'fresh', 'nested', 'file.txt'), 'utf8'), 'installed'); +}); + +test('a write cancelled before creating parents leaves the workspace unchanged', async (t) => { + const root = await realpath(await mkdtemp(join(tmpdir(), 'librechat-code-workspace-'))); + t.after(() => rm(root, { recursive: true, force: true })); + const tools = await heldRootTools(root); + const controller = new AbortController(); + controller.abort(); + + await assert.rejects( + tools.execute( + { + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'primary', + path: 'fresh/nested/file.txt', + content: 'never installed', + }, + controller.signal, + ), + (error: unknown) => + error instanceof WorkspaceToolError && + error.code === 'EXECUTION_ABORTED' && + !error.mutationMayHaveCommitted, + ); + assert.deepEqual(await readdir(root), []); +}); + +test('a pathname-only workspace root reports a missing parent instead of creating it', async (t) => { + const root = await mkdtemp(join(tmpdir(), 'librechat-code-workspace-')); + t.after(() => rm(root, { recursive: true, force: true })); + const tools = await LocalWorkspaceTools.create({ + workspaces: [{ id: 'primary', root, writable: true }], + }); + + await assert.rejects( + tools.execute({ + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'primary', + path: 'missing/notes.txt', + content: 'blocked', + }), + (error: unknown) => + error instanceof WorkspaceToolError && + error.code === 'NOT_FOUND' && + error.message === 'Workspace path does not exist', + ); + assert.deepEqual(await readdir(root), []); +}); + +test('a missing workspace path is NOT_FOUND only when reached through the workspace', async (t) => { + const parent = await mkdtemp(join(tmpdir(), 'librechat-code-workspace-parent-')); + t.after(() => rm(parent, { recursive: true, force: true })); + const root = join(parent, 'root'); + const outside = join(parent, 'outside'); + await mkdir(join(root, 'src'), { recursive: true }); + await mkdir(outside); + await writeFile(join(root, 'file.txt'), 'file'); + await symlink(join(root, 'src'), join(root, 'alias')); + await symlink(outside, join(root, 'linked-outside')); + await symlink(join(parent, 'missing-target'), join(root, 'dangling')); + const tools = await LocalWorkspaceTools.create({ + workspaces: [{ id: 'primary', root, writable: true }], + }); + const edits = [{ oldText: 'a', newText: 'b' }]; + const requests = (path: string): WorkspaceToolRequest[] => [ + { protocolVersion: 1, operation: 'read_file', workspaceId: 'primary', path }, + { protocolVersion: 1, operation: 'edit_file', workspaceId: 'primary', path, edits }, + { protocolVersion: 1, operation: 'preview_edit', workspaceId: 'primary', path, edits }, + { protocolVersion: 1, operation: 'search_text', workspaceId: 'primary', query: 'x', path }, + { protocolVersion: 1, operation: 'list_files', workspaceId: 'primary', path }, + ]; + + for (const [path, code] of [ + ['missing.log', 'NOT_FOUND'], + ['src/missing.ts', 'NOT_FOUND'], + ['not-yet/created/output.log', 'NOT_FOUND'], + ['alias/missing.ts', 'INVALID_PATH'], + ['linked-outside/missing.txt', 'INVALID_PATH'], + ['dangling', 'INVALID_PATH'], + ['file.txt/missing.txt', 'INVALID_PATH'], + ] as const) { + for (const request of requests(path)) { + await assert.rejects( + tools.execute(request), + (error: unknown) => { + assert.ok(error instanceof WorkspaceToolError); + assert.equal(error.code, code, `${request.operation} ${path}`); + assert.equal(error.message.includes(parent), false); + if (code === 'NOT_FOUND') { + assert.equal(error.message, 'Workspace path does not exist'); + } + return true; + }, + ); + } + } + for (const path of ['../outside/missing.txt', 'src/../../outside/missing.txt']) { + for (const request of requests(path)) { + await assert.rejects( + tools.execute(request), + (error: unknown) => + error instanceof WorkspaceToolError && + (error.code === 'INVALID_REQUEST' || error.code === 'INVALID_PATH'), + `${request.operation} ${path}`, + ); + } + } }); test('workspace mutations preserve existing file permissions', async (t) => { diff --git a/packages/code/src/workspace.ts b/packages/code/src/workspace.ts index 0b9328d0..e48365c8 100644 --- a/packages/code/src/workspace.ts +++ b/packages/code/src/workspace.ts @@ -1,6 +1,6 @@ import { createHash, randomBytes } from 'node:crypto'; import { constants } from 'node:fs'; -import { link, lstat, open, realpath, rename, stat, unlink, spawn, withWorkspaceRoot, WorkspaceRootAccessError } from './root-access.js'; +import { holdsWorkspaceRoot, link, lstat, mkdir, open, realpath, rename, stat, unlink, spawn, withWorkspaceRoot, WorkspaceRootAccessError } from './root-access.js'; import { basename, dirname, isAbsolute, relative, resolve, sep } from 'node:path'; import type { FileHandle } from 'node:fs/promises'; @@ -324,6 +324,9 @@ async function readConfinedFileBuffer( return buffer.subarray(0, bytesRead); } catch (error) { if (error instanceof WorkspaceToolError) throw error; + if (isMissingEntry(error)) { + throw await classifyMissingWorkspacePath(root, candidate); + } throw new WorkspaceToolError('Invalid workspace path', 'INVALID_PATH'); } finally { await handle?.close(); @@ -369,6 +372,147 @@ async function verifyDirectoryPathHasNoSymlinks( return currentIdentity; } +function isMissingEntry(error: unknown): boolean { + return (error as NodeJS.ErrnoException | undefined)?.code === 'ENOENT'; +} + +interface DirectoryIdentity { + path: string; + dev: bigint | number; + ino: bigint | number; +} + +/** Whether each ancestor observed before a miss is still the same real directory. */ +async function ancestorsUnchanged(ancestors: readonly DirectoryIdentity[]): Promise { + for (const ancestor of ancestors) { + try { + const current = await lstat(ancestor.path); + if ( + current.isSymbolicLink() || + !current.isDirectory() || + current.dev !== ancestor.dev || + current.ino !== ancestor.ino + ) { + return false; + } + } catch { + return false; + } + } + return true; +} + +/** + * Reports an absent target as `NOT_FOUND` only when the walk from the root + * reaches the missing entry through real directories, the root included, that + * are unchanged after the miss. A symlink, a file used as a directory, or an ancestor replaced + * during the walk stays a path-safety rejection, so the distinction never + * describes a path beyond the root. + */ +async function classifyMissingWorkspacePath( + root: string, + candidate: string, +): Promise { + const invalid = new WorkspaceToolError('Invalid workspace path', 'INVALID_PATH'); + const segments = relative(root, candidate).split(sep).filter(Boolean); + const ancestors: DirectoryIdentity[] = []; + try { + const rootEntry = await lstat(root); + if (rootEntry.isSymbolicLink() || !rootEntry.isDirectory()) return invalid; + ancestors.push({ path: root, dev: rootEntry.dev, ino: rootEntry.ino }); + } catch { + return invalid; + } + let current = root; + for (const [index, segment] of segments.entries()) { + current = resolve(current, segment); + let entry: Awaited>; + try { + entry = await lstat(current); + } catch (error) { + return isMissingEntry(error) && (await ancestorsUnchanged(ancestors)) + ? new WorkspaceToolError('Workspace path does not exist', 'NOT_FOUND') + : invalid; + } + if (index === segments.length - 1) return invalid; + if (entry.isSymbolicLink() || !entry.isDirectory()) return invalid; + ancestors.push({ path: current, dev: entry.dev, ino: entry.ino }); + } + return invalid; +} + +/** + * Creates each missing ancestor of a write target as a real directory beneath + * the root, one verified level at a time, and syncs each new entry into its + * parent. Creation needs a held root descriptor, so `mkdirat` is anchored to + * the root; a pathname-only root creates nothing and its write reports the + * missing parent. An existing symlink or file stops the walk. + * + * Created directories are a committed side effect, never rolled back: once one + * exists, a later failure of this write is reported as a possible mutation, so + * the worker quarantines rather than claiming an atomic rejection. + */ +async function createMissingParentDirectories( + root: string, + candidate: string, +): Promise { + const created: string[] = []; + if (!holdsWorkspaceRoot()) return created; + const segments = relative(root, dirname(candidate)).split(sep).filter(Boolean); + let current = root; + try { + for (const segment of segments) { + current = resolve(current, segment); + let entry: Awaited> | undefined; + try { + entry = await lstat(current); + } catch (error) { + if (!isMissingEntry(error)) throw error; + } + let createdHere = false; + if (entry == null) { + try { + await mkdir(current, 0o777); + created.push(current); + createdHere = true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error; + } + entry = await lstat(current); + /** Also after a raced `EEXIST`: the write relies on that entry too. */ + await syncWorkspaceDirectory(dirname(current)); + } + if ( + entry.isSymbolicLink() || + !entry.isDirectory() || + (createdHere && !isWithinRoot(root, await realpath(current))) + ) { + throw new WorkspaceToolError('Invalid workspace path', 'INVALID_PATH'); + } + } + return created; + } catch (error) { + throw afterCreatedDirectories(created, classifyWritePathValidationError(error)); + } +} + +function afterCreatedDirectories( + created: readonly string[], + error: WorkspaceToolError, +): WorkspaceToolError { + if (created.length === 0 || error.mutationMayHaveCommitted) return error; + return new WorkspaceToolError(error.message, error.code, true); +} + +function assertWithinWriteLimit(content: Buffer): void { + if (content.byteLength > BRIDGE_WORKSPACE_WRITE_MAX_BYTES) { + throw new WorkspaceToolError( + 'Workspace file exceeds write limit', + 'WRITE_LIMIT_EXCEEDED', + ); + } +} + function classifyWritePathValidationError(error: unknown): WorkspaceToolError { if (error instanceof WorkspaceToolError) return error; const code = (error as NodeJS.ErrnoException).code; @@ -481,12 +625,7 @@ async function atomicWriteConfinedFile( allowOverwrite = true, ): Promise<{ created: boolean }> { throwIfAborted(signal); - if (content.byteLength > BRIDGE_WORKSPACE_WRITE_MAX_BYTES) { - throw new WorkspaceToolError( - 'Workspace file exceeds write limit', - 'WRITE_LIMIT_EXCEEDED', - ); - } + assertWithinWriteLimit(content); const candidate = resolveWorkspacePath(root, requestedPath); const parent = dirname(candidate); let canonicalParent: string; @@ -498,6 +637,9 @@ async function atomicWriteConfinedFile( throw new WorkspaceToolError('Invalid workspace path', 'INVALID_PATH'); } } catch (error) { + if (isMissingEntry(error)) { + throw await classifyMissingWorkspacePath(root, parent); + } throw classifyWritePathValidationError(error); } @@ -671,14 +813,26 @@ async function writeWorkspaceFile( signal?: AbortSignal, ): Promise { const content = Buffer.from(request.content, 'utf8'); - const { created } = await atomicWriteConfinedFile( + throwIfAborted(signal); + assertWithinWriteLimit(content); + const directories = await createMissingParentDirectories( root, - request.path, - content, - signal, - undefined, - request.overwrite !== false, + resolveWorkspacePath(root, request.path), ); + let created: boolean; + try { + /** Once parents exist, finish the write rather than abandon them to a cancellation. */ + ({ created } = await atomicWriteConfinedFile( + root, + request.path, + content, + directories.length === 0 ? signal : undefined, + undefined, + request.overwrite !== false, + )); + } catch (error) { + throw afterCreatedDirectories(directories, classifyWritePathValidationError(error)); + } return { protocolVersion: BRIDGE_PROTOCOL_VERSION, operation: 'write_file', @@ -757,6 +911,9 @@ async function editWorkspaceFile( }; } catch (error) { if (error instanceof WorkspaceToolError) throw error; + if (isMissingEntry(error)) { + throw await classifyMissingWorkspacePath(root, candidate); + } throw classifyWritePathValidationError(error); } finally { await opened?.close().catch(() => undefined); @@ -1009,7 +1166,10 @@ async function searchWorkspace( let canonicalTarget: string; try { canonicalTarget = await realpath(target); - } catch { + } catch (error) { + if (isMissingEntry(error)) { + throw await classifyMissingWorkspacePath(root, target); + } throw new WorkspaceToolError('Invalid workspace path', 'INVALID_PATH'); } if (!isWithinRoot(root, canonicalTarget)) @@ -1060,7 +1220,9 @@ async function searchWorkspace( } catch (error) { if ( error instanceof WorkspaceToolError && - (error.code === 'INVALID_PATH' || error.code === 'READ_LIMIT_EXCEEDED') + (error.code === 'INVALID_PATH' || + error.code === 'NOT_FOUND' || + error.code === 'READ_LIMIT_EXCEEDED') ) { continue; } @@ -1167,6 +1329,13 @@ async function listWorkspaceFiles( ); } catch (error) { if (error instanceof WorkspaceToolError) throw error; + if (isMissingEntry(error)) { + throw await withinListDeadline( + classifyMissingWorkspacePath(root, target), + signal, + deadline, + ); + } throw new WorkspaceToolError('Invalid workspace path', 'INVALID_PATH'); } if (!isWithinRoot(root, canonicalTarget)) { @@ -1181,6 +1350,13 @@ async function listWorkspaceFiles( ).isDirectory(); } catch (error) { if (error instanceof WorkspaceToolError) throw error; + if (isMissingEntry(error)) { + throw await withinListDeadline( + classifyMissingWorkspacePath(root, target), + signal, + deadline, + ); + } throw new WorkspaceToolError('Invalid workspace path', 'INVALID_PATH'); } const normalizedRequestedResultPath = request.path diff --git a/service/src/bridge/router.test.ts b/service/src/bridge/router.test.ts index 28035847..360414a0 100644 --- a/service/src/bridge/router.test.ts +++ b/service/src/bridge/router.test.ts @@ -438,6 +438,7 @@ describe('paired bridge HTTP API', () => { 'replace_all', ], supportedWorkspaceListFileFeatures: ['after_path'], + supportedWorkspaceToolErrorCodes: ['NOT_FOUND'], }); const crossDeploymentRevoke = await fetch( diff --git a/service/src/bridge/router.ts b/service/src/bridge/router.ts index 8fdc700e..0bc20a1e 100644 --- a/service/src/bridge/router.ts +++ b/service/src/bridge/router.ts @@ -541,6 +541,7 @@ router.post( supportedWorkspaceProgrammaticLanguages: ['bash'], supportedWorkspaceInstanceTypes: ['git_worktree'], supportedWorkspaceScopes: ['git_linked_worktree'], + supportedWorkspaceToolErrorCodes: ['NOT_FOUND'], }); } catch (error) { if (error instanceof BridgeStoreError) { diff --git a/service/src/workspace-tools/router.test.ts b/service/src/workspace-tools/router.test.ts index d3cb032e..8a753c08 100644 --- a/service/src/workspace-tools/router.test.ts +++ b/service/src/workspace-tools/router.test.ts @@ -223,6 +223,8 @@ test.each([ ['COMMAND_TIMEOUT', 504], ['COMMAND_UNAVAILABLE', 503], ['COMMAND_DISABLED', 403], + ['INVALID_PATH', 422], + ['NOT_FOUND', 422], ] as const)('maps worker %s rejections to HTTP %i', async (errorCode, expectedStatus) => { const app = express(); app.use(json());