mirror of
https://github.com/aaronsb/obsidian-mcp-plugin.git
synced 2026-07-22 06:45:14 +00:00
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.
109 lines
3.9 KiB
TypeScript
109 lines
3.9 KiB
TypeScript
/**
|
|
* The test contract — rules the test suite itself must satisfy.
|
|
*
|
|
* A test suite is only worth its green checkmark if a passing run actually means the
|
|
* code works. Every rule below closes a way a suite can be green while proving nothing.
|
|
* This file is the enforcement; it lints the corpus the way ESLint lints src/.
|
|
*
|
|
* (Source-grep guards are an established pattern here — see tls-cert-verification.test.ts.
|
|
* This avoids adding eslint-plugin-jest, which the CLAUDE.md supply-chain hold would
|
|
* otherwise delay by 7 days.)
|
|
*
|
|
* Rules:
|
|
* 1. No focused tests — `.only` greens a run while silently skipping everything else.
|
|
* 2. No skipped tests — a skip is an assertion you have stopped making. Known bugs
|
|
* use `it.failing`, which fails if the bug is ever fixed and
|
|
* therefore self-corrects; a `.skip` rots silently forever.
|
|
* 3. Every file asserts — a test with no expect() passes by merely not throwing.
|
|
* 4. No lone expect(true).toBe(true) style tautologies.
|
|
* 5. Coverage floors exist — the ratchet in jest.config.js must not be deleted.
|
|
*/
|
|
import { readFileSync, readdirSync, statSync } from 'fs';
|
|
import { join } from 'path';
|
|
|
|
const TESTS_DIR = join(__dirname);
|
|
|
|
function testFiles(dir: string): string[] {
|
|
return readdirSync(dir).flatMap(entry => {
|
|
const full = join(dir, entry);
|
|
if (statSync(full).isDirectory()) return testFiles(full);
|
|
return full.endsWith('.test.ts') ? [full] : [];
|
|
});
|
|
}
|
|
|
|
const FILES = testFiles(TESTS_DIR).map(path => ({
|
|
path,
|
|
rel: path.replace(join(__dirname, '..') + '/', ''),
|
|
// Strip block and line comments so prose about `.only` (like this file's own header)
|
|
// is not mistaken for the real thing.
|
|
source: readFileSync(path, 'utf8')
|
|
.replace(/\/\*[\s\S]*?\*\//g, '')
|
|
.replace(/(^|[^:])\/\/.*$/gm, '$1')
|
|
}));
|
|
|
|
describe('test contract', () => {
|
|
it('should have found the test corpus', () => {
|
|
// Guards the guard: if the walker breaks, every rule below would vacuously pass.
|
|
expect(FILES.length).toBeGreaterThan(20);
|
|
});
|
|
|
|
it('should contain no focused tests', () => {
|
|
const offenders = FILES
|
|
.filter(f => /\b(?:it|test|describe)\.only\b|\bfit\(|\bfdescribe\(/.test(f.source))
|
|
.map(f => f.rel);
|
|
|
|
expect(offenders).toEqual([]);
|
|
});
|
|
|
|
it('should contain no skipped tests', () => {
|
|
const offenders = FILES
|
|
.filter(f => /\b(?:it|test|describe)\.skip\b|\bxit\(|\bxdescribe\(/.test(f.source))
|
|
.map(f => f.rel);
|
|
|
|
expect(offenders).toEqual([]);
|
|
});
|
|
|
|
it('should assert something in every test file', () => {
|
|
const offenders = FILES
|
|
.filter(f => !/\bexpect\(/.test(f.source))
|
|
.map(f => f.rel);
|
|
|
|
expect(offenders).toEqual([]);
|
|
});
|
|
|
|
it('should contain no tautological assertions', () => {
|
|
const offenders = FILES
|
|
.filter(f => /expect\(\s*(true|false|1)\s*\)\s*\.\s*toBe\(\s*(true|false|1)\s*\)/.test(f.source))
|
|
.map(f => f.rel);
|
|
|
|
expect(offenders).toEqual([]);
|
|
});
|
|
});
|
|
|
|
describe('coverage ratchet', () => {
|
|
// eslint-disable-next-line @typescript-eslint/no-require-imports -- jest.config.js is CJS
|
|
const config = require('../jest.config.js');
|
|
|
|
it('should define coverage thresholds', () => {
|
|
expect(config.coverageThreshold).toBeDefined();
|
|
});
|
|
|
|
it('should hold the security boundary to a higher floor than the global one', () => {
|
|
const security = config.coverageThreshold['./src/security/'];
|
|
const global = config.coverageThreshold.global;
|
|
|
|
expect(security.statements).toBeGreaterThan(global.statements);
|
|
expect(security.branches).toBeGreaterThan(global.branches);
|
|
});
|
|
|
|
it('should keep every floor above zero', () => {
|
|
const floors = Object.values(config.coverageThreshold) as Record<string, number>[];
|
|
|
|
expect(floors.length).toBeGreaterThan(0);
|
|
for (const floor of floors) {
|
|
for (const value of Object.values(floor)) {
|
|
expect(value).toBeGreaterThan(0);
|
|
}
|
|
}
|
|
});
|
|
});
|