From a08347f4befdfc7512c8fe00c4845238f90c0bcd Mon Sep 17 00:00:00 2001 From: Aaron Bockelie Date: Mon, 25 May 2026 08:31:37 -0500 Subject: [PATCH] fix(dispatch): address PR #212 review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - vault.update content-guard hint pointed at vault.patch, which does not exist — patch lives at edit.patch. Bad recovery hint is worse than no hint. - edit.window guard test claimed "before searching the file" but only asserted on mutations; the mock's 'original' content doesn't contain 'undefined' so even a leaked guard wouldn't show a write. Add a reads recorder and assert getFile is never called, making the test honest about what it proves. - Strip a stray space from the lock-key guard comment. --- src/semantic/operations/vault.ts | 2 +- src/semantic/router.ts | 2 +- tests/dispatch-param-guards.test.ts | 8 +++++++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/semantic/operations/vault.ts b/src/semantic/operations/vault.ts index aa9d4f6..b0a0052 100644 --- a/src/semantic/operations/vault.ts +++ b/src/semantic/operations/vault.ts @@ -131,7 +131,7 @@ export async function executeVaultOperation(ctx: RouterContext, action: string, params, 'content', 'vault.update', - "For partial replacement, use vault.patch with operation='replace', oldText, newText — or edit.window for fuzzy in-place edits.", + "For partial replacement, use edit.patch with operation='replace', oldText, newText — or edit.window for fuzzy in-place edits.", ); return await ctx.api.updateFile(path, content); } diff --git a/src/semantic/router.ts b/src/semantic/router.ts index 2cbb007..cfb317d 100644 --- a/src/semantic/router.ts +++ b/src/semantic/router.ts @@ -137,7 +137,7 @@ export class SemanticRouter implements RouterContext { // client can no longer silently clobber each other (#139). Different // files remain fully concurrent. // Guard the lock key up-front so a missing path can't take a lock on - // the literal string "undefined" and serialize unrelated bad calls. + // the literal string "undefined" and serialize unrelated bad calls. const lockPath = requireParamStr(params, 'path', `edit.${action}`); return FileLockManager.getInstance().withLock(lockPath, async () => { switch (action) { diff --git a/tests/dispatch-param-guards.test.ts b/tests/dispatch-param-guards.test.ts index 549a98f..50fe501 100644 --- a/tests/dispatch-param-guards.test.ts +++ b/tests/dispatch-param-guards.test.ts @@ -21,6 +21,7 @@ type Mutation = class RecordingAPI extends ObsidianAPI { readonly mutations: Mutation[] = []; + readonly reads: string[] = []; private files = new Map([['existing.md', 'original']]); constructor() { @@ -28,6 +29,7 @@ class RecordingAPI extends ObsidianAPI { } async getFile(path: string): Promise { + this.reads.push(path); const content = this.files.get(path); if (content === undefined) throw new Error(`File not found: ${path}`); return { content, path, type: 'text' }; @@ -142,7 +144,7 @@ describe('dispatch-level param guards (#210)', () => { expect((await api.getFile('existing.md')).content).toBe('original'); }); - test('edit.window without oldText/newText rejects before searching the file', async () => { + test('edit.window without oldText/newText rejects before reading or writing the file', async () => { const response = await router.route({ operation: 'edit', action: 'window', @@ -150,6 +152,10 @@ describe('dispatch-level param guards (#210)', () => { }); expect((response as any).error).toBeDefined(); expect(api.mutations).toEqual([]); + // The guard must run before performWindowEdit fetches the file — + // otherwise a missing oldText could still trigger a search for the + // literal string "undefined" with confusing diagnostics. + expect(api.reads).toEqual([]); }); test('edit.* without path rejects before taking a file lock', async () => {