From 098bf0679cf6c1931c0d89822170e21feea00dc1 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 2 Oct 2026 21:42:50 -0400 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=A7=AD=20fix:=20Report=20Missing=20Wo?= =?UTF-8?q?rkspace=20Paths=20as=20NOT=5FFOUND=20and=20Create=20Missing=20P?= =?UTF-8?q?arents=20on=20Write?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A missing file, directory or search scope no longer reports INVALID_PATH. When every existing ancestor resolves to a directory inside the workspace, the worker returns NOT_FOUND ("Workspace path does not exist"); escapes, symlinks, files used as directories and other path-safety failures keep INVALID_PATH. write_file creates missing parent directories beneath the workspace or lane root, one verified level at a time, never through a symlink or an existing file, and removes the directories it created if the write then fails. Code API advertises the added code in registration and maps it to HTTP 422 like INVALID_PATH. A worker reports NOT_FOUND only to a Code API that advertised it, and resends a refused settlement with INVALID_PATH, so older Code API replicas keep accepting settlements. --- packages/code/src/linked-worktrees.test.ts | 53 +++++++ packages/code/src/protocol.test.ts | 13 ++ packages/code/src/protocol.ts | 13 ++ packages/code/src/root-access.ts | 10 +- packages/code/src/worker.ts | 31 +++- packages/code/src/workspace-worker.test.ts | 106 +++++++++++++ packages/code/src/workspace.test.ts | 139 ++++++++++++++++- packages/code/src/workspace.ts | 169 +++++++++++++++++++-- service/src/bridge/router.test.ts | 1 + service/src/bridge/router.ts | 1 + service/src/workspace-tools/router.test.ts | 2 + 11 files changed, 516 insertions(+), 22 deletions(-) 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..91c7162a 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); } @@ -504,10 +504,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..a6907ec8 100644 --- a/packages/code/src/workspace.test.ts +++ b/packages/code/src/workspace.test.ts @@ -1737,7 +1737,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 +1752,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 +1768,141 @@ 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')), []); +}); + +test('writes create missing parent directories inside the workspace', async (t) => { + const root = await mkdtemp(join(tmpdir(), 'librechat-code-workspace-')); + t.after(() => rm(root, { recursive: true, force: true })); + await mkdir(join(root, 'src')); + const tools = await LocalWorkspaceTools.create({ + workspaces: [{ id: 'primary', root, writable: true }], + }); + + 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 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(root); + await mkdir(outside); + await writeFile(join(root, 'file.txt'), 'file'); + 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 }], + }); + + for (const [path, code] of [ + ['linked-outside/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(root)).sort(), ['dangling', 'file.txt', 'linked-outside']); + await assert.rejects(stat(join(parent, 'missing-target'))); +}); + +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', 'NOT_FOUND'], + ['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..1c192bbf 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 { link, lstat, mkdir, open, realpath, rename, rmdir, 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,114 @@ async function verifyDirectoryPathHasNoSymlinks( return currentIdentity; } +function isMissingEntry(error: unknown): boolean { + return (error as NodeJS.ErrnoException | undefined)?.code === 'ENOENT'; +} + +/** + * Reports an absent target as `NOT_FOUND` only when every existing ancestor + * resolves to a directory inside the workspace. Anything else stays a + * path-safety rejection, so the distinction reveals nothing 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); + 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) + ? new WorkspaceToolError('Workspace path does not exist', 'NOT_FOUND') + : invalid; + } + if (index === segments.length - 1) return invalid; + if (!entry.isSymbolicLink()) { + if (!entry.isDirectory()) return invalid; + continue; + } + try { + current = await realpath(current); + if (!isWithinRoot(root, current) || !(await stat(current)).isDirectory()) { + return invalid; + } + } catch { + return invalid; + } + } + return invalid; +} + +/** + * Creates each missing ancestor of a write target as a real directory beneath + * the root, one verified level at a time. An existing symlink or file stops the + * walk, and a created directory that resolves outside the root is rejected. + */ +async function createMissingParentDirectories( + root: string, + candidate: string, + signal?: AbortSignal, +): Promise { + const created: string[] = []; + 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) { + throwIfAborted(signal); + 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); + } + if ( + entry.isSymbolicLink() || + !entry.isDirectory() || + (createdHere && !isWithinRoot(root, await realpath(current))) + ) { + throw new WorkspaceToolError('Invalid workspace path', 'INVALID_PATH'); + } + } + return created; + } catch (error) { + await removeCreatedDirectories(created); + throw classifyWritePathValidationError(error); + } +} + +/** Best effort: `rmdir` removes a directory only while it is still empty. */ +async function removeCreatedDirectories(created: readonly string[]): Promise { + for (let index = created.length - 1; index >= 0; index -= 1) { + await rmdir(created[index]).catch(() => undefined); + } +} + +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 +592,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; @@ -671,14 +777,27 @@ 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, + resolveWorkspacePath(root, request.path), signal, - undefined, - request.overwrite !== false, ); + let created: boolean; + try { + ({ created } = await atomicWriteConfinedFile( + root, + request.path, + content, + signal, + undefined, + request.overwrite !== false, + )); + } catch (error) { + await removeCreatedDirectories(directories); + throw error; + } return { protocolVersion: BRIDGE_PROTOCOL_VERSION, operation: 'write_file', @@ -757,6 +876,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 +1131,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 +1185,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 +1294,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 +1315,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()); From 4e82c9e83b422bfa80f48ec0fbf538da86449158 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 2 Oct 2026 21:54:15 -0400 Subject: [PATCH 2/4] fix: Anchor Parent Creation to a Held Root, Sync It, and Surface Cleanup Failure Parent directories are created only under a held root descriptor, where mkdirat cannot follow a swapped path out of the root. A pathname-only root creates nothing and reports the missing parent as NOT_FOUND. Each created directory is synced into its parent before the write is acknowledged, and a rejected write that cannot remove a directory it created now reports a possible mutation instead of an atomic failure. --- packages/code/src/root-access.ts | 2 + packages/code/src/workspace.test.ts | 92 +++++++++++++++++++++++++---- packages/code/src/workspace.ts | 54 +++++++++++++---- 3 files changed, 127 insertions(+), 21 deletions(-) diff --git a/packages/code/src/root-access.ts b/packages/code/src/root-access.ts index 91c7162a..5bdddca9 100644 --- a/packages/code/src/root-access.ts +++ b/packages/code/src/root-access.ts @@ -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, diff --git a/packages/code/src/workspace.test.ts b/packages/code/src/workspace.test.ts index a6907ec8..d8441ec9 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, @@ -1771,13 +1773,19 @@ test('writes reject symlink targets and symlinked parent directories', async (t) assert.deepEqual(await readdir(join(root, 'real-directory')), []); }); -test('writes create missing parent directories inside the workspace', async (t) => { - const root = await mkdtemp(join(tmpdir(), 'librechat-code-workspace-')); +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 LocalWorkspaceTools.create({ - workspaces: [{ id: 'primary', root, writable: true }], - }); + const tools = await heldRootTools(root); for (const [path, overwrite] of [ ['src/feature/deep/new.ts', undefined], @@ -1805,21 +1813,23 @@ test('writes create missing parent directories inside the workspace', async (t) }); test('parent creation never follows symlinks, crosses files, or leaves the workspace', async (t) => { - const parent = await mkdtemp(join(tmpdir(), 'librechat-code-workspace-parent-')); + 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(root); + 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 LocalWorkspaceTools.create({ - workspaces: [{ id: 'primary', root, writable: true }], - }); + 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'], @@ -1841,10 +1851,70 @@ test('parent creation never follows symlinks, crosses files, or leaves the works ); } assert.deepEqual(await readdir(outside), []); - assert.deepEqual((await readdir(root)).sort(), ['dangling', 'file.txt', 'linked-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 rejected write removes the parent directories it created', 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); + /** Aborts only once both parents exist, after creation and before the file is installed. */ + const signal = { + get aborted() { + return existsSync(join(root, 'fresh', 'nested')); + }, + } as AbortSignal; + + await assert.rejects( + tools.execute( + { + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'primary', + path: 'fresh/nested/file.txt', + content: 'never installed', + }, + 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 })); diff --git a/packages/code/src/workspace.ts b/packages/code/src/workspace.ts index 1c192bbf..2cae3701 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, mkdir, open, realpath, rename, rmdir, stat, unlink, spawn, withWorkspaceRoot, WorkspaceRootAccessError } from './root-access.js'; +import { holdsWorkspaceRoot, link, lstat, mkdir, open, realpath, rename, rmdir, 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'; @@ -380,10 +380,12 @@ function isMissingEntry(error: unknown): boolean { * Reports an absent target as `NOT_FOUND` only when every existing ancestor * resolves to a directory inside the workspace. Anything else stays a * path-safety rejection, so the distinction reveals nothing beyond the root. + * Writes never pass through a symlink, so they report one as `INVALID_PATH`. */ async function classifyMissingWorkspacePath( root: string, candidate: string, + followDirectorySymlinks = true, ): Promise { const invalid = new WorkspaceToolError('Invalid workspace path', 'INVALID_PATH'); const segments = relative(root, candidate).split(sep).filter(Boolean); @@ -403,6 +405,7 @@ async function classifyMissingWorkspacePath( if (!entry.isDirectory()) return invalid; continue; } + if (!followDirectorySymlinks) return invalid; try { current = await realpath(current); if (!isWithinRoot(root, current) || !(await stat(current)).isDirectory()) { @@ -417,8 +420,10 @@ async function classifyMissingWorkspacePath( /** * Creates each missing ancestor of a write target as a real directory beneath - * the root, one verified level at a time. An existing symlink or file stops the - * walk, and a created directory that resolves outside the root is rejected. + * the root, one verified level at a time, and syncs each new entry into its + * parent. Creation needs a held root descriptor, so `mkdirat` cannot follow a + * swapped path out of the root; a pathname-only root creates nothing and its + * write reports the missing parent. An existing symlink or file stops the walk. */ async function createMissingParentDirectories( root: string, @@ -426,6 +431,7 @@ async function createMissingParentDirectories( signal?: AbortSignal, ): Promise { const created: string[] = []; + if (!holdsWorkspaceRoot()) return created; const segments = relative(root, dirname(candidate)).split(sep).filter(Boolean); let current = root; try { @@ -448,6 +454,7 @@ async function createMissingParentDirectories( if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error; } entry = await lstat(current); + if (createdHere) await syncWorkspaceDirectory(dirname(current)); } if ( entry.isSymbolicLink() || @@ -459,16 +466,37 @@ async function createMissingParentDirectories( } return created; } catch (error) { - await removeCreatedDirectories(created); - throw classifyWritePathValidationError(error); + throw await withCreatedDirectoriesRemoved( + created, + classifyWritePathValidationError(error), + ); } } -/** Best effort: `rmdir` removes a directory only while it is still empty. */ -async function removeCreatedDirectories(created: readonly string[]): Promise { +/** + * Removes directories a rejected write created, deepest first. `rmdir` only + * removes an empty directory, so content is never touched. A directory that is + * already gone or that another writer has filled is no longer this write's to + * remove; any other failure leaves the rejection uncertain, so the worker + * quarantines the workspace instead of reporting an atomic failure. + */ +async function withCreatedDirectoriesRemoved( + created: readonly string[], + error: WorkspaceToolError, +): Promise { + let removed = true; for (let index = created.length - 1; index >= 0; index -= 1) { - await rmdir(created[index]).catch(() => undefined); + try { + await rmdir(created[index]); + } catch (cleanupError) { + const code = (cleanupError as NodeJS.ErrnoException).code; + if (code !== 'ENOENT' && code !== 'ENOTEMPTY' && code !== 'EEXIST') { + removed = false; + } + } } + if (removed || error.mutationMayHaveCommitted) return error; + return new WorkspaceToolError(error.message, error.code, true); } function assertWithinWriteLimit(content: Buffer): void { @@ -604,6 +632,9 @@ async function atomicWriteConfinedFile( throw new WorkspaceToolError('Invalid workspace path', 'INVALID_PATH'); } } catch (error) { + if (isMissingEntry(error)) { + throw await classifyMissingWorkspacePath(root, parent, false); + } throw classifyWritePathValidationError(error); } @@ -795,8 +826,11 @@ async function writeWorkspaceFile( request.overwrite !== false, )); } catch (error) { - await removeCreatedDirectories(directories); - throw error; + if (directories.length === 0) throw error; + throw await withCreatedDirectoriesRemoved( + directories, + classifyWritePathValidationError(error), + ); } return { protocolVersion: BRIDGE_PROTOCOL_VERSION, From d0f6d78e4655020bff48d98dd7885511030f6106 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 2 Oct 2026 22:03:49 -0400 Subject: [PATCH 3/4] fix: Recheck Ancestors Before NOT_FOUND and Sync Parent Cleanup A miss is reported as NOT_FOUND only when the walk reached it through real directories that still have the same identity afterwards; any symlink in the chain stays INVALID_PATH. Cleanup syncs each removal into its parent and treats any directory it could not durably remove as a possible mutation. --- packages/code/src/workspace.test.ts | 36 +++++++++++++- packages/code/src/workspace.ts | 73 +++++++++++++++++------------ 2 files changed, 78 insertions(+), 31 deletions(-) diff --git a/packages/code/src/workspace.test.ts b/packages/code/src/workspace.test.ts index d8441ec9..1f50f2c3 100644 --- a/packages/code/src/workspace.test.ts +++ b/packages/code/src/workspace.test.ts @@ -15,7 +15,7 @@ import { unlink, writeFile, } from 'node:fs/promises'; -import { existsSync } from 'node:fs'; +import { existsSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, sep } from 'node:path'; import test from 'node:test'; @@ -1892,6 +1892,38 @@ test('a rejected write removes the parent directories it created', async (t) => assert.deepEqual(await readdir(root), []); }); +test('a rejected write that cannot remove its parents reports a possible mutation', 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 nested = join(root, 'fresh', 'nested'); + /** Leaves a file behind in the created directory, as a leaked staging file would. */ + const signal = { + get aborted() { + if (!existsSync(nested)) return false; + writeFileSync(join(nested, '.librechat-code-leaked.tmp'), ''); + return true; + }, + } as AbortSignal; + + await assert.rejects( + tools.execute( + { + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'primary', + path: 'fresh/nested/file.txt', + content: 'never installed', + }, + signal, + ), + (error: unknown) => + error instanceof WorkspaceToolError && + error.code === 'EXECUTION_ABORTED' && + error.mutationMayHaveCommitted, + ); +}); + 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 })); @@ -1942,7 +1974,7 @@ test('a missing workspace path is NOT_FOUND only when reached through the worksp ['missing.log', 'NOT_FOUND'], ['src/missing.ts', 'NOT_FOUND'], ['not-yet/created/output.log', 'NOT_FOUND'], - ['alias/missing.ts', 'NOT_FOUND'], + ['alias/missing.ts', 'INVALID_PATH'], ['linked-outside/missing.txt', 'INVALID_PATH'], ['dangling', 'INVALID_PATH'], ['file.txt/missing.txt', 'INVALID_PATH'], diff --git a/packages/code/src/workspace.ts b/packages/code/src/workspace.ts index 2cae3701..ff195d19 100644 --- a/packages/code/src/workspace.ts +++ b/packages/code/src/workspace.ts @@ -376,19 +376,46 @@ 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 every existing ancestor - * resolves to a directory inside the workspace. Anything else stays a - * path-safety rejection, so the distinction reveals nothing beyond the root. - * Writes never pass through a symlink, so they report one as `INVALID_PATH`. + * Reports an absent target as `NOT_FOUND` only when the walk from the root + * reaches the missing entry through real directories 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, - followDirectorySymlinks = true, ): Promise { const invalid = new WorkspaceToolError('Invalid workspace path', 'INVALID_PATH'); const segments = relative(root, candidate).split(sep).filter(Boolean); + const ancestors: DirectoryIdentity[] = []; let current = root; for (const [index, segment] of segments.entries()) { current = resolve(current, segment); @@ -396,24 +423,13 @@ async function classifyMissingWorkspacePath( try { entry = await lstat(current); } catch (error) { - return isMissingEntry(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()) { - if (!entry.isDirectory()) return invalid; - continue; - } - if (!followDirectorySymlinks) return invalid; - try { - current = await realpath(current); - if (!isWithinRoot(root, current) || !(await stat(current)).isDirectory()) { - return invalid; - } - } catch { - return invalid; - } + if (entry.isSymbolicLink() || !entry.isDirectory()) return invalid; + ancestors.push({ path: current, dev: entry.dev, ino: entry.ino }); } return invalid; } @@ -474,11 +490,12 @@ async function createMissingParentDirectories( } /** - * Removes directories a rejected write created, deepest first. `rmdir` only - * removes an empty directory, so content is never touched. A directory that is - * already gone or that another writer has filled is no longer this write's to - * remove; any other failure leaves the rejection uncertain, so the worker - * quarantines the workspace instead of reporting an atomic failure. + * Removes directories a rejected write created, deepest first, syncing each + * removal into its parent. `rmdir` only removes an empty directory, so content + * is never touched. Unless every created directory is durably gone, the + * rejection is uncertain and the worker quarantines the workspace instead of + * reporting an atomic failure: a directory left non-empty may hold this + * write's own staged file. */ async function withCreatedDirectoriesRemoved( created: readonly string[], @@ -488,11 +505,9 @@ async function withCreatedDirectoriesRemoved( for (let index = created.length - 1; index >= 0; index -= 1) { try { await rmdir(created[index]); + await syncWorkspaceDirectory(dirname(created[index])); } catch (cleanupError) { - const code = (cleanupError as NodeJS.ErrnoException).code; - if (code !== 'ENOENT' && code !== 'ENOTEMPTY' && code !== 'EEXIST') { - removed = false; - } + if (!isMissingEntry(cleanupError)) removed = false; } } if (removed || error.mutationMayHaveCommitted) return error; @@ -633,7 +648,7 @@ async function atomicWriteConfinedFile( } } catch (error) { if (isMissingEntry(error)) { - throw await classifyMissingWorkspacePath(root, parent, false); + throw await classifyMissingWorkspacePath(root, parent); } throw classifyWritePathValidationError(error); } From 4321ba9318c83ec9e639f00b3ebaba698e666029 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 2 Oct 2026 22:12:25 -0400 Subject: [PATCH 4/4] fix: Keep Created Parents as a Committed Side Effect Directories a write creates are no longer rolled back. Once one exists the write finishes despite cancellation, and any later failure is reported as a possible mutation. A raced EEXIST parent is synced too, and the NOT_FOUND ancestry recheck now includes the workspace root. --- packages/code/src/workspace.test.ts | 52 +++++++++-------------- packages/code/src/workspace.ts | 66 ++++++++++++----------------- 2 files changed, 47 insertions(+), 71 deletions(-) diff --git a/packages/code/src/workspace.test.ts b/packages/code/src/workspace.test.ts index 1f50f2c3..787fbe7d 100644 --- a/packages/code/src/workspace.test.ts +++ b/packages/code/src/workspace.test.ts @@ -15,7 +15,7 @@ import { unlink, writeFile, } from 'node:fs/promises'; -import { existsSync, writeFileSync } from 'node:fs'; +import { existsSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, sep } from 'node:path'; import test from 'node:test'; @@ -1862,49 +1862,38 @@ test('parent creation never follows symlinks, crosses files, or leaves the works await assert.rejects(stat(join(parent, 'missing-target'))); }); -test('a rejected write removes the parent directories it created', async (t) => { +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); - /** Aborts only once both parents exist, after creation and before the file is installed. */ + /** Reports cancellation as soon as the parents exist. */ const signal = { get aborted() { return existsSync(join(root, 'fresh', 'nested')); }, } as AbortSignal; - await assert.rejects( - tools.execute( - { - protocolVersion: 1, - operation: 'write_file', - workspaceId: 'primary', - path: 'fresh/nested/file.txt', - content: 'never installed', - }, - signal, - ), - (error: unknown) => - error instanceof WorkspaceToolError && - error.code === 'EXECUTION_ABORTED' && - !error.mutationMayHaveCommitted, + const result = await tools.execute( + { + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'primary', + path: 'fresh/nested/file.txt', + content: 'installed', + }, + signal, ); - assert.deepEqual(await readdir(root), []); + + assert.equal(result.operation === 'write_file' && result.created, true); + assert.equal(await readFile(join(root, 'fresh', 'nested', 'file.txt'), 'utf8'), 'installed'); }); -test('a rejected write that cannot remove its parents reports a possible mutation', async (t) => { +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 nested = join(root, 'fresh', 'nested'); - /** Leaves a file behind in the created directory, as a leaked staging file would. */ - const signal = { - get aborted() { - if (!existsSync(nested)) return false; - writeFileSync(join(nested, '.librechat-code-leaked.tmp'), ''); - return true; - }, - } as AbortSignal; + const controller = new AbortController(); + controller.abort(); await assert.rejects( tools.execute( @@ -1915,13 +1904,14 @@ test('a rejected write that cannot remove its parents reports a possible mutatio path: 'fresh/nested/file.txt', content: 'never installed', }, - signal, + controller.signal, ), (error: unknown) => error instanceof WorkspaceToolError && error.code === 'EXECUTION_ABORTED' && - error.mutationMayHaveCommitted, + !error.mutationMayHaveCommitted, ); + assert.deepEqual(await readdir(root), []); }); test('a pathname-only workspace root reports a missing parent instead of creating it', async (t) => { diff --git a/packages/code/src/workspace.ts b/packages/code/src/workspace.ts index ff195d19..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 { holdsWorkspaceRoot, link, lstat, mkdir, open, realpath, rename, rmdir, 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'; @@ -404,8 +404,8 @@ async function ancestorsUnchanged(ancestors: readonly DirectoryIdentity[]): Prom /** * Reports an absent target as `NOT_FOUND` only when the walk from the root - * reaches the missing entry through real directories that are unchanged after - * the miss. A symlink, a file used as a directory, or an ancestor replaced + * 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. */ @@ -416,6 +416,13 @@ async function classifyMissingWorkspacePath( 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); @@ -437,14 +444,17 @@ async function classifyMissingWorkspacePath( /** * 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` cannot follow a - * swapped path out of the root; a pathname-only root creates nothing and its - * write reports the missing parent. An existing symlink or file stops the walk. + * 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, - signal?: AbortSignal, ): Promise { const created: string[] = []; if (!holdsWorkspaceRoot()) return created; @@ -461,7 +471,6 @@ async function createMissingParentDirectories( } let createdHere = false; if (entry == null) { - throwIfAborted(signal); try { await mkdir(current, 0o777); created.push(current); @@ -470,7 +479,8 @@ async function createMissingParentDirectories( if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error; } entry = await lstat(current); - if (createdHere) await syncWorkspaceDirectory(dirname(current)); + /** Also after a raced `EEXIST`: the write relies on that entry too. */ + await syncWorkspaceDirectory(dirname(current)); } if ( entry.isSymbolicLink() || @@ -482,35 +492,15 @@ async function createMissingParentDirectories( } return created; } catch (error) { - throw await withCreatedDirectoriesRemoved( - created, - classifyWritePathValidationError(error), - ); + throw afterCreatedDirectories(created, classifyWritePathValidationError(error)); } } -/** - * Removes directories a rejected write created, deepest first, syncing each - * removal into its parent. `rmdir` only removes an empty directory, so content - * is never touched. Unless every created directory is durably gone, the - * rejection is uncertain and the worker quarantines the workspace instead of - * reporting an atomic failure: a directory left non-empty may hold this - * write's own staged file. - */ -async function withCreatedDirectoriesRemoved( +function afterCreatedDirectories( created: readonly string[], error: WorkspaceToolError, -): Promise { - let removed = true; - for (let index = created.length - 1; index >= 0; index -= 1) { - try { - await rmdir(created[index]); - await syncWorkspaceDirectory(dirname(created[index])); - } catch (cleanupError) { - if (!isMissingEntry(cleanupError)) removed = false; - } - } - if (removed || error.mutationMayHaveCommitted) return error; +): WorkspaceToolError { + if (created.length === 0 || error.mutationMayHaveCommitted) return error; return new WorkspaceToolError(error.message, error.code, true); } @@ -828,24 +818,20 @@ async function writeWorkspaceFile( const directories = await createMissingParentDirectories( root, resolveWorkspacePath(root, request.path), - signal, ); let created: boolean; try { + /** Once parents exist, finish the write rather than abandon them to a cancellation. */ ({ created } = await atomicWriteConfinedFile( root, request.path, content, - signal, + directories.length === 0 ? signal : undefined, undefined, request.overwrite !== false, )); } catch (error) { - if (directories.length === 0) throw error; - throw await withCreatedDirectoriesRemoved( - directories, - classifyWritePathValidationError(error), - ); + throw afterCreatedDirectories(directories, classifyWritePathValidationError(error)); } return { protocolVersion: BRIDGE_PROTOCOL_VERSION,