aaronsb_obsidian-mcp-plugin/tests/vault-rename-extension.test.ts
Aaron Bockelie 8bb580cc56 fix: .mcpignore semantics, TLS 1.3 startup, rename extension (#250, #252, #253)
Bundles the three reported bugs plus an unreported under-blocking bug found
while writing the tests for #250, and adds the coverage tooling that would
have caught all four.

#250 — .mcpignore negation was inverted
The parser stripped the leading '!' before Minimatch saw it, so .negate was
always false and the branch reading it was dead code. '!folder/*' compiled
into a positive exclusion: negation did not merely fail, it inverted.

Under-blocking (found while testing the above, not reported)
Patterns were handed to Minimatch raw, so two gitignore rules were never
implemented: a slash-less pattern matches at any depth ('*.secret' must hide
'a/b/creds.secret'), and a directory match covers its contents ('private/'
must hide 'private/notes.md'). No consumer of isExcluded() checks ancestors —
every caller passes a full file path — so nothing compensated downstream. The
plugin's own shipped .mcpignore template promises both behaviours, so a user
following it believed private files were hidden while they were being served
to the model. Patterns are now translated into the globs that implement
gitignore semantics, and negation is interpreted here rather than by Minimatch.

#252 — min TLS 1.3 crashed the server
secureProtocol mapped 1.3 to 'TLSv1_3_method', which OpenSSL never shipped, so
context creation threw "Unknown method: TLSv1_3_method". Switched to minVersion,
which also fixes a second latent bug: secureProtocol PINS one version, so
"minimum TLS 1.2" was silently refusing TLS 1.3 clients. The setting now
behaves as the floor it is labelled.

#253 — rename dropped the extension
'newName' was concatenated onto the source directory verbatim, so renaming
'note.md' to 'renamed' produced an extension-less file that drops out of
markdown views. The source extension is now carried over when newName omits
one, before the overwrite guard so it checks the right path.

Coverage + test contract
All four bugs shipped through code paths with zero coverage. Adds `make
coverage` / `coverage-map` / `coverage-gate`, a ratcheting coverageThreshold
(security boundary held to a far higher floor than global, since a hole there
leaks vault content), and tests/test-contract.test.ts enforcing that a green
run means something: no .only, no .skip, every file asserts, no tautologies.
The contract immediately caught an `expect(true).toBe(true)` placeholder in the
path-validator suite, now replaced with real assertions.

BREAKING (.mcpignore, both intended):
- The double-'!' workaround for #250 now correctly parses as a double negation
  (net exclude) and will stop working.
- A bare '*' now excludes at every depth, per gitignore, rather than top-level
  only. This is what makes the '*' + '!keep/**' whitelist idiom work.
2026-07-13 17:46:50 -05:00

102 lines
3.3 KiB
TypeScript

/**
* vault rename — extension preservation (#253).
*
* Drives the real SemanticRouter -> executeVaultOperation path. Only the vault I/O
* boundary is stubbed (ObsidianAPI.getFile, app.fileManager.renameFile), so the path
* construction under test is the shipped one. The rename action had no behavioural
* test at all before this, which is why the dropped extension shipped.
*/
import { SemanticRouter } from '../src/semantic/router';
import { ObsidianAPI } from '../src/utils/obsidian-api';
import { App, TFile } from 'obsidian';
interface RenameResult {
success: boolean;
oldPath: string;
newPath: string;
}
class MockObsidianAPI extends ObsidianAPI {
constructor(private existing: Set<string>) {
super({} as App);
}
async getFile(path: string): Promise<never> {
if (!this.existing.has(path)) {
throw new Error(`File not found: ${path}`);
}
return { path, content: 'body' } as never;
}
}
/**
* Fake Obsidian app that records the destination handed to fileManager.renameFile —
* the actual side effect a rename produces on the vault.
*/
function fakeApp(existing: Set<string>, renamed: string[]): App {
return {
vault: {
getAbstractFileByPath: (path: string) =>
existing.has(path) ? ({ path, extension: 'md' } as unknown as TFile) : null
},
fileManager: {
renameFile: async (_file: TFile, newPath: string) => {
renamed.push(newPath);
}
}
} as unknown as App;
}
async function rename(source: string, newName: string): Promise<{ result: RenameResult; renamed: string[] }> {
const existing = new Set([source]);
const renamed: string[] = [];
const router = new SemanticRouter(new MockObsidianAPI(existing), fakeApp(existing, renamed));
const response = await router.route({
operation: 'vault',
action: 'rename',
params: { path: source, newName }
});
return { result: response.result as unknown as RenameResult, renamed };
}
describe('vault rename — extension handling (#253)', () => {
it('should preserve the source extension when newName omits one', async () => {
const { result, renamed } = await rename('work/my-note.md', 'my-renamed');
expect(result.newPath).toBe('work/my-renamed.md');
expect(renamed).toEqual(['work/my-renamed.md']);
});
it('should not double up the extension when newName already has one', async () => {
const { result, renamed } = await rename('work/my-note.md', 'my-renamed.md');
expect(result.newPath).toBe('work/my-renamed.md');
expect(renamed).toEqual(['work/my-renamed.md']);
});
it('should preserve a non-markdown source extension', async () => {
const { result } = await rename('assets/diagram.png', 'architecture');
expect(result.newPath).toBe('assets/architecture.png');
});
it('should honour an explicit different extension in newName', async () => {
const { result } = await rename('work/my-note.md', 'my-renamed.txt');
expect(result.newPath).toBe('work/my-renamed.txt');
});
it('should preserve the extension for a file at the vault root', async () => {
const { result } = await rename('note.md', 'renamed');
expect(result.newPath).toBe('renamed.md');
});
it('should leave an extension-less source extension-less', async () => {
const { result } = await rename('work/LICENSE', 'COPYING');
expect(result.newPath).toBe('work/COPYING');
});
});