diff --git a/packages/agent-core-v2/src/os/backends/node-local/hostFsService.ts b/packages/agent-core-v2/src/os/backends/node-local/hostFsService.ts index c5531b2ae..c1799da7d 100644 --- a/packages/agent-core-v2/src/os/backends/node-local/hostFsService.ts +++ b/packages/agent-core-v2/src/os/backends/node-local/hostFsService.ts @@ -5,7 +5,7 @@ * Bound at App scope. */ -import { open, readFile, readdir, stat, mkdir, rm, writeFile } from 'node:fs/promises'; +import { appendFile, open, readFile, readdir, stat, mkdir, rm, writeFile } from 'node:fs/promises'; import { InstantiationType } from '#/_base/di/extensions'; import { LifecycleScope, registerScopedService } from '#/_base/di/scope'; @@ -52,6 +52,10 @@ export class HostFileSystem implements IHostFileSystem { await writeFile(path, data, 'utf8'); } + async appendText(path: string, data: string): Promise { + await appendFile(path, data, 'utf8'); + } + async readBytes(path: string, n?: number): Promise { if (n === undefined) { const buf = await readFile(path); diff --git a/packages/agent-core-v2/src/os/backends/node-local/tools/write.ts b/packages/agent-core-v2/src/os/backends/node-local/tools/write.ts index 7c2d8daac..2620f47db 100644 --- a/packages/agent-core-v2/src/os/backends/node-local/tools/write.ts +++ b/packages/agent-core-v2/src/os/backends/node-local/tools/write.ts @@ -6,9 +6,9 @@ * (mirroring `mkdir(parents=True, exist_ok=True)`). Path access policy is * resolved before any filesystem I/O. * - * `IHostFileSystem.writeText` has no mode flag: overwrite maps to a direct - * write, while append reads the existing content first (treating a missing - * file as empty) and writes the concatenation back. + * Append uses `IHostFileSystem.appendText` (a native `O_APPEND`-style append), + * so existing content is never read or rewritten — keeping appends atomic with + * respect to concurrent writers and safe against mid-write crashes. * * Write access flows through the os `hostFs` domain (`IHostFileSystem`); path * semantics (home expansion, path class) come from the `hostEnvironment` @@ -107,14 +107,7 @@ export class WriteTool implements BuiltinTool { try { const mode = args.mode ?? 'overwrite'; if (mode === 'append') { - let existing = ''; - try { - existing = await this.fs.readText(safePath); - } catch (error) { - const code = (error as { code?: unknown } | null)?.code; - if (code !== 'ENOENT') throw error; - } - await this.fs.writeText(safePath, existing + args.content); + await this.fs.appendText(safePath, args.content); } else { await this.fs.writeText(safePath, args.content); } diff --git a/packages/agent-core-v2/src/os/interface/hostFileSystem.ts b/packages/agent-core-v2/src/os/interface/hostFileSystem.ts index 17d1755bb..b4305b023 100644 --- a/packages/agent-core-v2/src/os/interface/hostFileSystem.ts +++ b/packages/agent-core-v2/src/os/interface/hostFileSystem.ts @@ -34,6 +34,15 @@ export interface IHostFileSystem { options?: { encoding?: BufferEncoding; errors?: TextDecodeErrors }, ): Promise; writeText(path: string, data: string): Promise; + /** + * Append UTF-8 `data` to the end of `path`, creating the file if it does not + * exist. Maps to a native append (POSIX `O_APPEND` / `fs.appendFile`): it + * never reads or truncates existing content, so concurrent readers never see + * a partially-rewritten file and a crash mid-write can lose only the new + * bytes, never the prior contents. Prefer this over a read-then-rewrite for + * log-style appends. + */ + appendText(path: string, data: string): Promise; /** * Read bytes from `path`. When `n` is given, reads at most the first `n` * bytes (a ranged/prefix read); otherwise reads the whole file. The ranged diff --git a/packages/agent-core-v2/test/fileTools/write.test.ts b/packages/agent-core-v2/test/fileTools/write.test.ts index efc497d8b..e574685ec 100644 --- a/packages/agent-core-v2/test/fileTools/write.test.ts +++ b/packages/agent-core-v2/test/fileTools/write.test.ts @@ -6,10 +6,9 @@ * minimal fake `IHostFileSystem` inline so the tool can be exercised without * the composition root. * - * The v1 append path used `kaos.writeText(path, data, { mode: 'a' })`. v2's - * `IHostFileSystem.writeText` has no mode flag, so append is implemented as - * `readText` (treating a missing file as empty) followed by `writeText` of the - * concatenation. The append-call assertions below reflect that mechanic. + * Append is routed through `IHostFileSystem.appendText` (a native append), so + * the tool no longer reads the existing file. The append-call assertions below + * reflect that single-call mechanic. */ import { describe, expect, it, vi } from 'vitest'; @@ -51,6 +50,8 @@ interface WriteFsOptions { readText?: (path: string) => Promise; /** Override writeText. Default no-op. */ writeText?: (path: string, data: string) => Promise; + /** Override appendText. Default no-op. */ + appendText?: (path: string, data: string) => Promise; /** Override stat. Default reports an existing directory. */ stat?: (path: string) => Promise; /** Override mkdir. Default no-op. */ @@ -72,12 +73,13 @@ function createWriteFs(options: WriteFsOptions = {}) { }), ); const writeText = vi.fn(options.writeText ?? (async () => {})); + const appendText = vi.fn(options.appendText ?? (async () => {})); const stat = vi.fn( options.stat ?? (async () => ({ isFile: false, isDirectory: true, size: 0 })), ); const mkdir = vi.fn(options.mkdir ?? (async () => {})); - const fs = { cwd: '/', readText, writeText, stat, mkdir } as unknown as IHostFileSystem; - return { fs, readText, writeText, stat, mkdir }; + const fs = { cwd: '/', readText, writeText, appendText, stat, mkdir } as unknown as IHostFileSystem; + return { fs, readText, writeText, appendText, stat, mkdir }; } function makeTool(options: WriteFsOptions = {}, workspace = PERMISSIVE_WORKSPACE) { @@ -218,8 +220,8 @@ describe('WriteTool', () => { expect(result.output).toContain('Wrote 5 bytes'); }); - it('appends content by reading existing bytes then writing the concatenation', async () => { - const { tool, readText, writeText } = makeTool({ readText: async () => 'old' }); + it('appends content through appendText without reading existing bytes', async () => { + const { tool, readText, writeText, appendText } = makeTool(); const result = await execute(tool, { path: '/tmp/existing.txt', @@ -227,8 +229,9 @@ describe('WriteTool', () => { mode: 'append', }); - expect(readText).toHaveBeenCalledWith('/tmp/existing.txt'); - expect(writeText).toHaveBeenCalledWith('/tmp/existing.txt', 'old\nhello'); + expect(appendText).toHaveBeenCalledWith('/tmp/existing.txt', '\nhello'); + expect(readText).not.toHaveBeenCalled(); + expect(writeText).not.toHaveBeenCalled(); expect(result.output).toContain('Appended 6 bytes'); }); @@ -272,11 +275,11 @@ describe('WriteTool', () => { const expectedBytes = Buffer.byteLength(content, 'utf8'); expect(expectedBytes).toBe(5); - const { tool, writeText } = makeTool({ readText: async () => 'prefix' }); + const { tool, appendText } = makeTool(); const result = await execute(tool, { path: '/tmp/menu.txt', content, mode: 'append' }); - expect(writeText).toHaveBeenCalledWith('/tmp/menu.txt', 'prefixcafé'); + expect(appendText).toHaveBeenCalledWith('/tmp/menu.txt', 'café'); expect(result.output).toContain('Appended 5 bytes'); }); @@ -414,9 +417,9 @@ describe('WriteTool', () => { }); it('appending to a nonexistent file creates it with just the appended bytes', async () => { - // Append mode on a missing path returns success and creates the file: - // readText rejects with ENOENT, so existing content is treated as empty. - const { tool, readText, writeText } = makeTool(); + // Native append (fs.appendFile) creates the file when it is missing, so + // append mode on a new path succeeds and writes exactly the appended bytes. + const { tool, readText, appendText } = makeTool(); const result = await execute(tool, { path: '/tmp/new-append.txt', @@ -426,8 +429,8 @@ describe('WriteTool', () => { expect(result.isError).toBeFalsy(); expect(toolContentString(result).toLowerCase()).toContain('appended'); - expect(readText).toHaveBeenCalledWith('/tmp/new-append.txt'); - expect(writeText).toHaveBeenCalledWith('/tmp/new-append.txt', 'New content'); + expect(appendText).toHaveBeenCalledWith('/tmp/new-append.txt', 'New content'); + expect(readText).not.toHaveBeenCalled(); }); it('allows absolute writes to a sibling dir that merely shares the work-dir prefix', async () => { diff --git a/packages/agent-core-v2/test/harness/agent.ts b/packages/agent-core-v2/test/harness/agent.ts index 195bf207b..d74471c59 100644 --- a/packages/agent-core-v2/test/harness/agent.ts +++ b/packages/agent-core-v2/test/harness/agent.ts @@ -432,6 +432,7 @@ function isFullHostFs(input: unknown): boolean { const keys: readonly (keyof IHostFileSystem)[] = [ 'readText', 'writeText', + 'appendText', 'readBytes', 'writeBytes', 'readLines', diff --git a/packages/agent-core-v2/test/sessionFs/fsService.test.ts b/packages/agent-core-v2/test/sessionFs/fsService.test.ts index 59960bbb4..0c86b8024 100644 --- a/packages/agent-core-v2/test/sessionFs/fsService.test.ts +++ b/packages/agent-core-v2/test/sessionFs/fsService.test.ts @@ -66,6 +66,7 @@ function fakeFs(files: Record): IHostFileSystem { return c; }, writeText: async () => {}, + appendText: async () => {}, readBytes: async (p, n) => { const c = fileMap.get(p); if (c === undefined) throw enoent(p); diff --git a/packages/agent-core-v2/test/tools/fixtures/fake-exec.ts b/packages/agent-core-v2/test/tools/fixtures/fake-exec.ts index c6c64b219..6a85e62f8 100644 --- a/packages/agent-core-v2/test/tools/fixtures/fake-exec.ts +++ b/packages/agent-core-v2/test/tools/fixtures/fake-exec.ts @@ -20,6 +20,7 @@ export function createFakeHostFs(overrides: Partial = {}): IHos _serviceBrand: undefined, readText: () => notImplemented('FakeHostFs.readText'), writeText: () => notImplemented('FakeHostFs.writeText'), + appendText: () => notImplemented('FakeHostFs.appendText'), readBytes: () => notImplemented('FakeHostFs.readBytes'), writeBytes: () => notImplemented('FakeHostFs.writeBytes'), readLines: () => notImplemented('FakeHostFs.readLines'), diff --git a/packages/agent-core-v2/test/workspaceCommand/workspace-command.test.ts b/packages/agent-core-v2/test/workspaceCommand/workspace-command.test.ts index 6bd9bc974..20a91602d 100644 --- a/packages/agent-core-v2/test/workspaceCommand/workspace-command.test.ts +++ b/packages/agent-core-v2/test/workspaceCommand/workspace-command.test.ts @@ -76,6 +76,10 @@ class MemoryHostFs implements IHostFileSystem { this.files.set(path, data); } + async appendText(path: string, data: string): Promise { + this.files.set(path, (this.files.get(path) ?? '') + data); + } + pauseNextWrite(): { readonly started: Promise; readonly release: () => void } { let started!: () => void; let release!: () => void;