diff --git a/src/lib/obsidian/vault-writer.ts b/src/lib/obsidian/vault-writer.ts index cd69d359..3f546c2c 100644 --- a/src/lib/obsidian/vault-writer.ts +++ b/src/lib/obsidian/vault-writer.ts @@ -59,14 +59,14 @@ export class FsVaultWriter implements VaultWriter { ); } - await this.writeFile(absPath, prefix + result.body, 'utf8'); + await this.writeRaw(absPath, prefix + result.body, input.path); } async replaceFullBody(input: ReplaceFullBodyInput): Promise { const absPath = path.join(this.vaultRoot, input.path); const raw = await this.readRaw(absPath, input.path); const { prefix } = splitRawFrontmatter(raw); - await this.writeFile(absPath, prefix + input.content, 'utf8'); + await this.writeRaw(absPath, prefix + input.content, input.path); } private async readRaw(absPath: string, vaultRelativePath: string): Promise { @@ -87,4 +87,16 @@ export class FsVaultWriter implements VaultWriter { ); } } + + private async writeRaw(absPath: string, data: string, vaultRelativePath: string): Promise { + try { + await this.writeFile(absPath, data, 'utf8'); + } catch (err) { + throw new ToolHandlerError( + 'WRITE_FAILED', + `Failed to write ${vaultRelativePath}: ${(err as Error).message}`, + { details: { path: vaultRelativePath }, cause: err }, + ); + } + } } diff --git a/src/modules/operations/fs-vault-provider.ts b/src/modules/operations/fs-vault-provider.ts index d9f670bc..8234ebfd 100644 --- a/src/modules/operations/fs-vault-provider.ts +++ b/src/modules/operations/fs-vault-provider.ts @@ -62,7 +62,7 @@ export class FsVaultProvider implements VaultProvider { async createNote(input: CreateNoteInput): Promise { const vaultRoot = this.vaultRoot; if (input.name === undefined && input.path === undefined) { - throw new Error('createNote requires name or path'); + throw invalidArgument('createNote requires name or path', 'name'); } let relPath: string; if (input.path !== undefined) { diff --git a/test/lib/obsidian/vault-writer.test.ts b/test/lib/obsidian/vault-writer.test.ts index 87ffdab2..9821532a 100644 --- a/test/lib/obsidian/vault-writer.test.ts +++ b/test/lib/obsidian/vault-writer.test.ts @@ -21,6 +21,14 @@ function fakeFs(initial: Record) { return { files, readFile, writeFile }; } +function failingWriteFile(message: string, code: string) { + return vi.fn(async () => { + const err = new Error(message) as Error & { code?: string }; + err.code = code; + throw err; + }); +} + describe('FsVaultWriter.replaceInNote', () => { it('replaces a single occurrence and writes back', async () => { const fs = fakeFs({ @@ -112,6 +120,26 @@ describe('FsVaultWriter.replaceInNote', () => { }); expect(fs.writeFile).not.toHaveBeenCalled(); }); + + it('maps a write failure to WRITE_FAILED', async () => { + const fs = fakeFs({ '/vault/n.md': '---\ntype: note\n---\nfind me\n' }); + const writer = new FsVaultWriter({ + vaultRoot: '/vault', + readFile: fs.readFile, + writeFile: failingWriteFile('ENOSPC: no space left on device', 'ENOSPC'), + }); + + await expect( + writer.replaceInNote({ path: 'n.md', find: 'find me', content: 'x' }), + ).rejects.toBeInstanceOf(ToolHandlerError); + await expect( + writer.replaceInNote({ path: 'n.md', find: 'find me', content: 'x' }), + ).rejects.toMatchObject({ + code: 'WRITE_FAILED', + details: { path: 'n.md' }, + message: expect.stringContaining('no space left on device'), + }); + }); }); describe('FsVaultWriter.replaceFullBody', () => { @@ -184,4 +212,22 @@ describe('FsVaultWriter.replaceFullBody', () => { code: 'NOT_FOUND', }); }); + + it('maps a write failure to WRITE_FAILED', async () => { + const fs = fakeFs({ '/vault/n.md': '---\nx: y\n---\nbody\n' }); + const writer = new FsVaultWriter({ + vaultRoot: '/vault', + readFile: fs.readFile, + writeFile: failingWriteFile('EACCES: permission denied', 'EACCES'), + }); + + await expect(writer.replaceFullBody({ path: 'n.md', content: 'x' })).rejects.toBeInstanceOf( + ToolHandlerError, + ); + await expect(writer.replaceFullBody({ path: 'n.md', content: 'x' })).rejects.toMatchObject({ + code: 'WRITE_FAILED', + details: { path: 'n.md' }, + message: expect.stringContaining('permission denied'), + }); + }); }); diff --git a/test/operations/fs-vault-provider/create-note.test.ts b/test/operations/fs-vault-provider/create-note.test.ts index c546a0f1..dba672f4 100644 --- a/test/operations/fs-vault-provider/create-note.test.ts +++ b/test/operations/fs-vault-provider/create-note.test.ts @@ -79,11 +79,17 @@ describe('FsVaultProvider.createNote (disk)', () => { ); }); - it('throws when neither name nor path is given', async () => { + it('fails INVALID_ARGUMENT when neither name nor path is given', async () => { const root = await makeVault({}); const provider = makeProvider(root); - await expect(provider.createNote({})).rejects.toThrow('createNote requires name or path'); + // The tool layer already enforces the name/path XOR, but the provider + // contract must not depend on that: every escape carries a code (ADR-0003). + await expect(provider.createNote({})).rejects.toMatchObject({ + code: 'INVALID_ARGUMENT', + details: { field: 'name' }, + message: 'createNote requires name or path', + }); }); it('writes an empty file when content is omitted', async () => { diff --git a/test/operations/tools/edit-note.test.ts b/test/operations/tools/edit-note.test.ts index 48705eda..e11da9c7 100644 --- a/test/operations/tools/edit-note.test.ts +++ b/test/operations/tools/edit-note.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it, vi } from 'vitest'; +import { FsVaultWriter } from '../../../src/lib/obsidian/vault-writer.js'; +import { registerTool } from '../../../src/lib/tool-registry.js'; import { buildEditNoteTool } from '../../../src/modules/operations/tools/edit-note.js'; import { makeReader, makeWriter } from './_helpers.js'; import { makeTestRegistry } from './_test-registry.js'; @@ -135,6 +137,51 @@ describe('edit_note: path auto-promotion', () => { }); }); +describe('edit_note: disk write failures reach the client with a code', () => { + // ADR-0003: every tool error the client sees carries `{ code, message, + // details }`. A failing fs write used to escape FsVaultWriter as a bare + // Error, which `toToolErrorResponse` renders through its code-less branch — + // nothing for an LLM client to branch on. Exercise the real writer through + // the registered tool so both the mapping and the rendering are asserted. + function buildWithFailingDisk() { + const writer = new FsVaultWriter({ + vaultRoot: '/vault', + readFile: vi.fn().mockResolvedValue('---\nx: y\n---\nold body\n'), + writeFile: vi.fn().mockRejectedValue( + Object.assign(new Error('ENOSPC: no space left on device'), { + code: 'ENOSPC', + }), + ), + }); + const registry = makeTestRegistry([{ name: 'v', reader: makeReader(), writer }]); + return registerTool(buildEditNoteTool({ registry })); + } + + it('surfaces WRITE_FAILED on a full-body rewrite', async () => { + const result = await buildWithFailingDisk().handler({ path: 'n.md', content: 'new' }); + + expect(result.isError).toBe(true); + expect(result.structuredContent).toMatchObject({ + code: 'WRITE_FAILED', + details: { path: 'n.md' }, + }); + }); + + it('surfaces WRITE_FAILED on a targeted replace', async () => { + const result = await buildWithFailingDisk().handler({ + path: 'n.md', + content: 'new', + replace: 'old body', + }); + + expect(result.isError).toBe(true); + expect(result.structuredContent).toMatchObject({ + code: 'WRITE_FAILED', + details: { path: 'n.md' }, + }); + }); +}); + describe('edit_note: identifier validation', () => { it('rejects when both name and path are provided', async () => { const { tool } = buildTool();