fix(dispatch): address PR #212 review findings

- 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.
This commit is contained in:
Aaron Bockelie 2026-05-25 08:31:37 -05:00
parent 000f69551a
commit a08347f4be
3 changed files with 9 additions and 3 deletions

View file

@ -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);
}

View file

@ -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) {

View file

@ -21,6 +21,7 @@ type Mutation =
class RecordingAPI extends ObsidianAPI {
readonly mutations: Mutation[] = [];
readonly reads: string[] = [];
private files = new Map<string, string>([['existing.md', 'original']]);
constructor() {
@ -28,6 +29,7 @@ class RecordingAPI extends ObsidianAPI {
}
async getFile(path: string): Promise<any> {
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 () => {