From 91cf21d80433ef7de2573c4b78a6b7e0f8abba4e Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 19:44:33 +0300 Subject: [PATCH 01/26] feat(mds)!: basePath propagation (#180), browser lint exports (#215), option-type second pass (#213) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **#180 (live bug)** basePath was accepted by the unknown-option validator but silently discarded before reaching the backend: the old forwarding builders (compileOpt/varsOpt) never included it. Templates using @import/@extends with a string-source call and a basePath option would resolve against the wrong directory or fail silently. Fix: add basePath to CompileOptions and CheckOptions; propagate through four new typed builders (compileSrcOpt, checkSrcOpt, fileCompileOpt, fileCheckOpt). The WASM backend now rejects non-null basePath on compile/check/lint with mds::invalid_options (OD-1 — errors instead of silently ignoring). The file-surface guard (compileFile/checkFile) returns Promise.reject(fileBasePathError()) so callers using assert.rejects()/.catch() receive a proper rejection; unknown-key and pre-init errors still throw synchronously (existing contract preserved per U-OV-12/U-B11). **#215** lint() and lintVirtual() exported from the browser entry point. All seven lint types (LintDiagnostic, LintFileOptions, LintFileReport, LintOptions, LintResult, LintRuleName, LintSpan, RuleSeverity) and LINT_RULE_NAMES are now available from both the Node.js and browser entries. **#213** options.ts second pass: delete _CompileBackendOpts/_CheckBackendOpts stubs; replace compileOpt/varsOpt with four typed per-surface builders; update METHOD_KEYS witnesses to the real public types; add BASEPATH_PASSTHROUGH set for compileFile/ checkFile so assertKnownKeys lets basePath through to the purpose-built guard. BREAKING: FileOptions no longer extends CompileOptions (D-TS-02); checkFile parameter changed from CheckOptions to CheckFileOptions; WASM backend rejects non-null basePath on string-surface methods instead of silently ignoring. New: tsconfig.types.json + __test__/types/ type-level matrix test (consumer-node.ts, consumer-browser.ts) with @ts-expect-error guards as positive controls (ADR-010). Closes #180 #213 #215 --- CHANGELOG.md | 71 ++++ packages/mds/README.md | 58 ++- packages/mds/__test__/browser.spec.mjs | 95 +++++ .../mds/__test__/options-validation.spec.mjs | 375 ++++++++++++++++-- .../mds/__test__/types/consumer-browser.ts | 44 ++ packages/mds/__test__/types/consumer-node.ts | 79 ++++ packages/mds/__test__/wasm-backend.spec.mjs | 93 ++++- packages/mds/package.json | 2 +- packages/mds/src/backend/native.ts | 35 +- packages/mds/src/backend/wasm.ts | 56 ++- packages/mds/src/browser.ts | 52 ++- packages/mds/src/index.ts | 26 +- packages/mds/src/node.ts | 81 +++- packages/mds/src/types.ts | 69 +++- packages/mds/src/util/options.ts | 129 +++--- packages/mds/tsconfig.types.json | 8 + 16 files changed, 1130 insertions(+), 143 deletions(-) create mode 100644 packages/mds/__test__/types/consumer-browser.ts create mode 100644 packages/mds/__test__/types/consumer-node.ts create mode 100644 packages/mds/tsconfig.types.json diff --git a/CHANGELOG.md b/CHANGELOG.md index f1177286..e5285114 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,77 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **`basePath` option is now honored on `compile()`, `check()`, and `lint()` (#180).** + Previously `basePath` was accepted by the unknown-option validator (so no error was + thrown) but was silently discarded before reaching the backend: the forwarding builders + (`compileOpt`/`varsOpt`) never included it in what they passed through. Templates + containing `@import` or `@extends` directives compiled with a string-source call and a + `basePath` option would either fail to resolve their imports (native backend) or fail + silently (WASM backend). The fix adds `basePath` to both `CompileOptions` and + `CheckOptions` and propagates it through all four per-surface option builders + (`compileSrcOpt`, `checkSrcOpt`, `fileCompileOpt`, `fileCheckOpt`). + + The WASM backend has no filesystem access and cannot resolve file-relative imports; it + now **rejects** a non-null `basePath` immediately with `mds::invalid_options` instead + of silently ignoring it, so misconfigured callers receive an actionable error rather + than a silent wrong answer. `{basePath: undefined}` is treated as absent on both + backends (`!= null` check; value-is-intent). To use `basePath` with import resolution, + set `MDS_BACKEND=native` or use `lintVirtual` with a pre-resolved module map. + + **Note:** `compileFile` and `checkFile` reject a non-null `basePath` at the JS layer + because the base directory for file-based operations is derived from the file path + itself; passing `basePath` there is always a caller error. + +### Added + +- **`lint` and `lintVirtual` are now exported from the browser entry point (#215).** + Previously only `compile` and `check` were available from the WASM/browser surface; + the underlying WASM module already supported linting but the exports were missing from + `browser.ts`. Both functions are available after `init()` and follow the same + unknown-option guard used by `compile`/`check`. + + All seven lint types (`LintDiagnostic`, `LintFileOptions`, `LintFileReport`, + `LintOptions`, `LintResult`, `LintRuleName`, `LintSpan`, `RuleSeverity`) and the + `LINT_RULE_NAMES` constant are now exported from both the Node.js and browser entry + points. + +### **BREAKING** — TypeScript option types and WASM `basePath` rejection (#213, #180) + +#### `FileOptions` no longer extends `CompileOptions` (#213) + +`FileOptions` (used by `compileFile`) was previously declared as +`interface FileOptions extends CompileOptions`. This inheritance was an error: +`CompileOptions` now carries `basePath`, which is explicitly not valid for file-based +operations (the base directory is derived from the file path). `FileOptions` is now +a standalone interface with its own `vars`, `sourceMap`, and `sourcesContent` fields. + +**Migration:** code that assigned a `CompileOptions` to a `FileOptions` variable (or +vice versa) now needs an explicit mapping. Code that used `FileOptions` purely for +`vars`, `sourceMap`, and `sourcesContent` is unaffected. + +#### `checkFile` parameter type changed from `CheckOptions` to `CheckFileOptions` (#213) + +`checkFile(path, options?)` previously accepted `CheckOptions`, which includes a +`basePath` field that is invalid for file-based operations. The parameter is now typed +as `CheckFileOptions` — a new interface with only `vars?: Record`. + +**Migration:** if you passed `CheckOptions` to `checkFile`, remove the `basePath` field. +If you used a shared variable typed as `CheckOptions`, destructure or pick `vars` before +passing it. + +#### WASM backend rejects `basePath` on string-surface methods (#180) + +`compile(source, { basePath: '/dir' })` and `check(source, { basePath: '/dir' })` +previously silently ignored `basePath` on the WASM backend. They now throw +`Error { code: 'mds::invalid_options' }`. `lint(source, { basePath: '/dir' })` was +already documented as WASM-unsupported; it now enforces this at runtime too. + +**Migration:** switch to the native backend (`MDS_BACKEND=native`) when you need +import resolution with a `basePath`, or use `lintVirtual` with a pre-resolved module +map in WASM environments. + ### **BREAKING** — Interpolation syntax: `{x}` → `{{x}}` Interpolation now uses **double braces**: `{{variable}}`, `{{obj.field}}`, diff --git a/packages/mds/README.md b/packages/mds/README.md index 072e7e05..a1843c8a 100644 --- a/packages/mds/README.md +++ b/packages/mds/README.md @@ -102,27 +102,64 @@ try { ### Options +#### Option matrix + +Each cell names the accepted option type. `basePath` is a string-surface option only; +file-path methods derive the base directory from the file argument. + +| | String source | File path | +|-----------|------------------------------|--------------------| +| `compile` | `CompileOptions` | `FileOptions` | +| `check` | `CheckOptions` | `CheckFileOptions` | +| `lint` | `LintOptions` | `LintFileOptions` | + +#### Option types + ```ts -// CompileOptions — accepted by compile() and compileFile() -interface CompileOptions { +// CheckOptions — accepted by check() (string source) +// basePath: required when the source contains @import or @extends. +// WASM backend: basePath throws mds::invalid_options (no filesystem access); +// set MDS_BACKEND=native to use the native backend with import resolution. +// {basePath: undefined} is treated as absent on both backends. +interface CheckOptions { vars?: Record; + basePath?: string; +} + +// CompileOptions — accepted by compile() (string source) +// Extends CheckOptions: inherits vars and basePath. +// basePath behaves the same as for CheckOptions (see above). +interface CompileOptions extends CheckOptions { sourceMap?: boolean; // generate Source Map v3; result gains a `sourceMap` field sourcesContent?: boolean; // embed source text in map (requires sourceMap: true) // ⚠ Privacy: embeds the full template source } -// CheckOptions — accepted by check() and checkFile() only -// Source-map options are NOT accepted; passing them throws mds::invalid_options -interface CheckOptions { +// FileOptions — accepted by compileFile() +// basePath is NOT accepted: the base directory is derived from the file path. +interface FileOptions { + vars?: Record; + sourceMap?: boolean; + sourcesContent?: boolean; +} + +// CheckFileOptions — accepted by checkFile() +// basePath is NOT accepted (base directory from file path). +// Source-map options are NOT accepted; passing them throws mds::invalid_options. +interface CheckFileOptions { vars?: Record; } // LintOptions — accepted by lint() (string-source) +// basePath: required when the source contains @import or @extends. +// WASM backend: basePath throws mds::invalid_options (OD-1 — rejects instead of +// silently ignoring so misconfigured callers see an actionable error). +// Set MDS_BACKEND=native to use the native backend, or use lintVirtual with +// pre-resolved modules. interface LintOptions { vars?: Record; rules?: Record; - basePath?: string; // base directory for @import resolution; required when the source - // contains @import or @extends. Ignored by the WASM backend. + basePath?: string; } // LintFileOptions — accepted by lintFile() and lintVirtual() @@ -139,10 +176,9 @@ interface InitOptions { } ``` -**Strict unknown-option rejection:** passing any key not listed above throws -`Error { code: 'mds::invalid_options' }` immediately, before calling the backend. -This applies to `compile`, `compileFile`, `check`, `checkFile`, `lint`, `lintFile`, -and `lintVirtual`. +**Unknown-option rejection:** passing an unrecognised key to any of the seven +public methods throws `Error { code: 'mds::invalid_options' }` immediately, before +calling the backend. The error names the offending key(s) and lists the accepted keys. **Source maps:** for string-source compiles (`compile`) `sources[0]` in the generated map is `"input.mds"`. For stdin builds via the CLI it is `""`. diff --git a/packages/mds/__test__/browser.spec.mjs b/packages/mds/__test__/browser.spec.mjs index 2600d133..c64ccb77 100644 --- a/packages/mds/__test__/browser.spec.mjs +++ b/packages/mds/__test__/browser.spec.mjs @@ -19,6 +19,8 @@ import { check, getBackend, isMdsError, + lint, + lintVirtual, _resetForTesting as browserReset, _initWithModuleForTesting, } from '../dist/browser.js'; @@ -43,6 +45,34 @@ describe('browser entry — pre-init', () => { // Ensure we start in a clean state before each test in this block. before(() => browserReset()); + test('U-BR20: lint throws before init() with message mentioning init() (AC-P3-15)', () => { + assert.throws( + () => lint('Hello!\n'), + (err) => { + assert.ok(err instanceof Error); + assert.ok( + err.message.includes('init()'), + `expected init() in message, got: ${err.message}`, + ); + return true; + }, + ); + }); + + test('U-BR21: lintVirtual throws before init() with message mentioning init() (AC-P3-15)', () => { + assert.throws( + () => lintVirtual({ 'a.mds': 'Hello\n' }, 'a.mds'), + (err) => { + assert.ok(err instanceof Error); + assert.ok( + err.message.includes('init()'), + `expected init() in message, got: ${err.message}`, + ); + return true; + }, + ); + }); + test('U-BR1: compile throws before init()', () => { assert.throws( () => compile('Hello!\n'), @@ -75,6 +105,23 @@ describe('browser entry — pre-init', () => { assert.equal(getBackend(), 'wasm'); }); + test('U-BR14: lint and lintVirtual are exported from browser entry (AC-P3-12)', () => { + assert.equal(typeof lint, 'function', 'lint must be a function'); + assert.equal(typeof lintVirtual, 'function', 'lintVirtual must be a function'); + }); + + test('U-BR15: lintFile is NOT exported from browser entry (AC-P3-13)', async () => { + const moduleExports = Object.keys(await import('../dist/browser.js')); + assert.equal( + moduleExports.includes('lintFile'), + false, + `lintFile must not be exported from browser entry; found: ${moduleExports.join(', ')}`, + ); + // Also verify compileFile and checkFile remain absent. + assert.equal(moduleExports.includes('compileFile'), false, 'compileFile must not be exported'); + assert.equal(moduleExports.includes('checkFile'), false, 'checkFile must not be exported'); + }); + test('U-BR12: compileFile is NOT a property of browser module', async () => { // Browser entry no longer exports compileFile — it requires node:fs which is // not available in browser environments. @@ -108,6 +155,54 @@ describe('browser entry — post-init', () => { _initWithModuleForTesting(sharedWasmModule); }); + test('U-BR16: browser lint returns a LintResult with version/files/truncated (AC-P3-12)', () => { + // Use a source with an unused frontmatter variable so we get at least one finding. + const src = '---\ngreeting: Hello\nunused_key: this key is never referenced\n---\n\n{{greeting}}, world!\n'; + const result = lint(src); + assert.equal(result.version, 1, 'version must be 1'); + assert.ok(Array.isArray(result.files), 'files must be an array'); + assert.equal(result.truncated, false, 'truncated must be false'); + // At least one diagnostic (unused-variable for unused_key). + assert.ok(result.files.length > 0, 'expected at least one file report for unused frontmatter'); + }); + + test('U-BR17: browser lintVirtual returns findings keyed by entry (AC-P3-12)', () => { + // Use a self-contained module map so no cross-module import is needed. + // The entry has an unused frontmatter variable to produce at least one diagnostic. + const modules = { + 'main.mds': '---\ngreeting: Hello\nunused_key: never used\n---\n{{greeting}}, world!\n', + }; + const result = lintVirtual(modules, 'main.mds'); + assert.equal(result.version, 1); + assert.ok(Array.isArray(result.files)); + assert.equal(result.truncated, false); + // files contains reports keyed by entry name or dep name. + const fileNames = result.files.map((f) => f.file); + assert.ok(fileNames.length >= 0, 'lintVirtual must return a valid result'); + }); + + test('U-BR19: browser lint/lintVirtual reject unknown option keys (AC-P3-14)', () => { + // Proves the browser path runs assertKnownKeys. + assert.throws( + () => lint('Hello\n', { basePathh: '.' }), + (err) => { + assert.ok(isMdsError(err), `expected isMdsError, got: ${err}`); + assert.equal(err.code, 'mds::invalid_options'); + assert.ok(err.message.includes('"basePathh"'), `key in message: ${err.message}`); + return true; + }, + ); + assert.throws( + () => lintVirtual({ 'a.mds': '' }, 'a.mds', { ruless: {} }), + (err) => { + assert.ok(isMdsError(err), `expected isMdsError, got: ${err}`); + assert.equal(err.code, 'mds::invalid_options'); + assert.ok(err.message.includes('"ruless"'), `key in message: ${err.message}`); + return true; + }, + ); + }); + test('U-BR6: concurrent init() cannot double-init an already-initialized backend', () => { // Backend is already set by _initWithModuleForTesting; additional init() calls // resolve immediately (resolvedBackend guard). This verifies idempotency. diff --git a/packages/mds/__test__/options-validation.spec.mjs b/packages/mds/__test__/options-validation.spec.mjs index ad3741ff..767d050b 100644 --- a/packages/mds/__test__/options-validation.spec.mjs +++ b/packages/mds/__test__/options-validation.spec.mjs @@ -303,50 +303,375 @@ describe('options-validation', () => { ); }); - // ── basePath passthrough on file methods (issue #74) ────────────────────── + // ── basePath passthrough on file methods: purposeful rejection (issue #74) ── + // + // U-OV-25 / U-OV-26 replace the now-vacuous U-OV-17 / U-OV-18. The old tests + // asserted only that the message did NOT start with "unknown option key" — they + // passed because compileFile/checkFile with basePath succeeded silently (the bug + // was that basePath was accepted and then dropped). After the fix, the call MUST + // throw with code 'mds::invalid_options' and a purpose-built message. - test('U-OV-17: compileFile does not intercept basePath with a generic unknown-key message (issue #74)', async () => { - // Before the fix, the wrapper emitted "unknown option key 'basePath'; recognised - // keys are: vars, sourceMap, sourcesContent", masking the backend's purpose-built - // "not valid for compileFile/checkFile" message. After the fix, basePath is passed - // through without wrapper interception. + test('U-OV-25: compileFile rejects basePath with a purposeful error (AC-P3-06, AC-P3-07)', async () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-test-')); const file = path.join(tmp, 'ok.mds'); fs.writeFileSync(file, 'Hello\n', 'utf8'); try { - let errorMsg = ''; - try { - await compileFile(file, { basePath: '.' }); - } catch (e) { - errorMsg = e instanceof Error ? e.message : String(e); - } - assert.ok( - !errorMsg.startsWith('unknown option key "basePath"'), - `wrapper must not intercept basePath for compileFile with a generic message; got: "${errorMsg}"`, + await assert.rejects( + () => compileFile(file, { basePath: '.' }), + (err) => { + assert.ok(isMdsError(err), `expected isMdsError, got: ${err}`); + assert.equal(err.code, 'mds::invalid_options'); + // Must name basePath and state the base is derived from the file path. + assert.ok(err.message.includes('basePath'), `basePath in message: ${err.message}`); + assert.ok(err.message.includes('derived from the file path'), `remedy in message: ${err.message}`); + // Must NOT be the generic unknown-key message (AC-P3-07). + assert.ok( + !err.message.startsWith('unknown option key'), + `must not be generic rejection: "${err.message}"`, + ); + return true; + }, ); } finally { fs.rmSync(tmp, { recursive: true, force: true }); } }); - test('U-OV-18: checkFile does not intercept basePath with a generic unknown-key message (issue #74)', async () => { - // Same as U-OV-17 but for checkFile. + test('U-OV-26: checkFile rejects basePath with a purposeful error (AC-P3-06, AC-P3-07)', async () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-test-')); const file = path.join(tmp, 'ok.mds'); fs.writeFileSync(file, 'Hello\n', 'utf8'); try { - let errorMsg = ''; + await assert.rejects( + () => checkFile(file, { basePath: '.' }), + (err) => { + assert.ok(isMdsError(err), `expected isMdsError, got: ${err}`); + assert.equal(err.code, 'mds::invalid_options'); + assert.ok(err.message.includes('basePath'), `basePath in message: ${err.message}`); + assert.ok(err.message.includes('derived from the file path'), `remedy in message: ${err.message}`); + assert.ok( + !err.message.startsWith('unknown option key'), + `must not be generic rejection: "${err.message}"`, + ); + return true; + }, + ); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + // ── basePath honored on native backend (AC-P3-01 / AC-P3-02 / AC-P3-03) ───── + + // Hard-fail sentinel for tests that require the native addon — a silently-passing + // skip is exactly how behavioral regressions survive undetected (avoids PF-013). + function requireNativeAddon() { + return require('@mdscript/mds-napi'); + } + + const FIXTURES = path.join(new URL('.', import.meta.url).pathname, 'fixtures'); + const IMPORT_SRC = '@import { greet } from "./import_provider.mds"\n\n{{greet("World")}}\n'; + + test('U-OV-21: native compile honors basePath for import resolution (AC-P3-01)', () => { + requireNativeAddon(); // hard-fail without addon + const result = compile(IMPORT_SRC, { basePath: FIXTURES }); + assert.equal(result.kind, 'markdown'); + assert.ok( + result.output.includes('Hello World!'), + `expected "Hello World!" in output, got: "${result.output}"`, + ); + assert.ok( + result.dependencies.some((d) => d.endsWith('import_provider.mds')), + `expected import_provider.mds in dependencies: ${result.dependencies.join(', ')}`, + ); + }); + + test('U-OV-22: value-sensitive positive control — wrong basePath throws, right basePath succeeds (AC-P3-02)', () => { + requireNativeAddon(); // hard-fail without addon + // A fresh empty directory has no import_provider.mds; the import cannot resolve. + const empty = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-bp-')); + try { + assert.throws( + () => compile(IMPORT_SRC, { basePath: empty }), + (err) => { + assert.ok(isMdsError(err), `expected isMdsError, got: ${err}`); + return true; + }, + 'compiling with a wrong basePath must throw — proves the VALUE is honored', + ); + } finally { + fs.rmSync(empty, { recursive: true, force: true }); + } + // The correct basePath must succeed (paired with U-OV-21 above). + const good = compile(IMPORT_SRC, { basePath: FIXTURES }); + assert.equal(good.kind, 'markdown'); + }); + + test('U-OV-23: native check honors basePath (AC-P3-03)', () => { + requireNativeAddon(); // hard-fail without addon + const empty = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-bp-')); + try { + // Correct basePath: check must succeed and return warnings array. + const result = check(IMPORT_SRC, { basePath: FIXTURES }); + assert.ok(Array.isArray(result.warnings), 'check result must have warnings array'); + // check results carry no dependencies field (napi F-K11). + assert.equal( + Object.prototype.hasOwnProperty.call(result, 'dependencies'), + false, + 'check result must not have dependencies field', + ); + // Wrong basePath must throw. + assert.throws( + () => check(IMPORT_SRC, { basePath: empty }), + (err) => { + assert.ok(isMdsError(err), `expected isMdsError, got: ${err}`); + return true; + }, + 'check with wrong basePath must throw', + ); + } finally { + fs.rmSync(empty, { recursive: true, force: true }); + } + }); + + // ── forwarding drift guard: spy addon (AC-P3-04 / AC-P3-05) ────────────── + + test('U-OV-24: all 7 methods forward exactly the accepted keys to the backend (AC-P3-04, AC-P3-05)', async () => { + requireNativeAddon(); // hard-fail without addon + const { createNativeBackend } = await import('../dist/backend/native.js'); + + // Minimal valid result shapes for each assertResultShape kind. + const compileResult = { kind: 'markdown', output: '', warnings: [], dependencies: [] }; + const checkResult = { warnings: [] }; + const lintResult = { version: 1, files: [], truncated: false }; + + // Build a spy addon: each method records the last options argument received. + let lastOpts; + function makeResult(shape) { return shape; } + const spyAddon = { + compile: (_src, opts) => { lastOpts = opts; return makeResult(compileResult); }, + check: (_src, opts) => { lastOpts = opts; return makeResult(checkResult); }, + compileFile: (_p, opts) => { lastOpts = opts; return Promise.resolve(makeResult(compileResult)); }, + checkFile: (_p, opts) => { lastOpts = opts; return Promise.resolve(makeResult(checkResult)); }, + lint: (_src, opts) => { lastOpts = opts; return makeResult(lintResult); }, + lintFile: (_p, opts) => { lastOpts = opts; return Promise.resolve(makeResult(lintResult)); }, + lintVirtual: (_m, _e, opts) => { lastOpts = opts; return makeResult(lintResult); }, + }; + + const be = createNativeBackend(spyAddon); + + const VARS = { k: 1 }; + const RULES = { 'unused-variable': 'warn' }; + const BP = '/some/base/path'; + + const cases = [ + { + name: 'compile', + call: () => be.compile('', { basePath: BP, vars: VARS, sourceMap: true, sourcesContent: true }), + expected: { basePath: BP, vars: VARS, sourceMap: true, sourcesContent: true }, + }, + { + name: 'check', + call: () => be.check('', { basePath: BP, vars: VARS }), + expected: { basePath: BP, vars: VARS }, + }, + { + name: 'compileFile', + call: () => be.compileFile('/any.mds', { vars: VARS, sourceMap: true, sourcesContent: true }), + expected: { vars: VARS, sourceMap: true, sourcesContent: true }, + }, + { + name: 'checkFile', + call: () => be.checkFile('/any.mds', { vars: VARS }), + expected: { vars: VARS }, + }, + { + name: 'lint', + call: () => be.lint('', { basePath: BP, vars: VARS, rules: RULES }), + expected: { basePath: BP, vars: VARS, rules: RULES }, + }, + { + name: 'lintFile', + call: () => be.lintFile('/any.mds', { vars: VARS, rules: RULES }), + expected: { vars: VARS, rules: RULES }, + }, + { + name: 'lintVirtual', + call: () => be.lintVirtual({ 'a.mds': '' }, 'a.mds', { vars: VARS, rules: RULES }), + expected: { vars: VARS, rules: RULES }, + }, + ]; + + for (const { name, call, expected } of cases) { + lastOpts = undefined; + await call(); + assert.deepStrictEqual( + lastOpts, + expected, + `${name}: forwarded options must be deep-equal to the expected key subset`, + ); + } + }); + + // ── cross-backend message equality for file basePath (AC-P3-06 / U-OV-27) ─ + + test('U-OV-27: compileFile/checkFile basePath rejection message is byte-identical on native and WASM (AC-P3-06, avoids PF-007)', async () => { + requireNativeAddon(); // hard-fail without addon + + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-bp-')); + const file = path.join(tmp, 'ok.mds'); + fs.writeFileSync(file, 'Hello\n', 'utf8'); + + async function captureMsg(fn) { + try { await fn(); } catch (e) { return e instanceof Error ? e.message : String(e); } + return ''; + } + + try { + for (const method of ['compileFile', 'checkFile']) { + // Native path: comes through the napi addon. + const nativeMsg = await captureMsg( + () => (method === 'compileFile' ? compileFile : checkFile)(file, { basePath: '.' }), + ); + // WASM path: set MDS_BACKEND=wasm via env then run in subprocess — or + // drive the WASM path directly via wrapWithFileOps. We use a subprocess + // for full isolation (no shared module singleton). + const { execFileSync } = await import('node:child_process'); + const wasmMsg = await captureMsg(() => { + const script = ` +import { ${method} } from './dist/node.js'; +${method}(${JSON.stringify(file)}, { basePath: '.' }).catch(e => { + process.stdout.write(e.message); + process.exit(0); +}).then(r => { if (r !== undefined) process.exit(0); }); +`; + const out = execFileSync(process.execPath, ['--input-type=module'], { + input: script, + cwd: new URL('..', import.meta.url).pathname, + env: { ...process.env, MDS_BACKEND: 'wasm' }, + timeout: 15000, + }); + throw new Error(out.toString().trim() || 'WASM subprocess produced no output'); + }); + + assert.ok(nativeMsg.length > 0, `native ${method} must throw for basePath`); + assert.ok(wasmMsg.length > 0, `WASM ${method} must throw for basePath`); + assert.strictEqual( + nativeMsg, + wasmMsg, + `byte-identical messages required for ${method} — native: "${nativeMsg}" | wasm: "${wasmMsg}"`, + ); + } + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + // ── {basePath: undefined} cross-backend parity (AC-P3-08 / U-OV-29) ──────── + + test('U-OV-29: {basePath: undefined} has consistent throw/no-throw on both backends for compile/check (AC-P3-08)', async () => { + requireNativeAddon(); // hard-fail without addon + + // Semantic: basePath: undefined is treated as absent ("value is intent") on the + // wrapper side. The WASM guard checks != null so undefined passes through. + // On native, napi sees the property but its value is undefined/null and does not + // trigger basePath handling. Both backends must agree. + const methods = [ + { name: 'compile', fn: (opts) => compile('Hello\n', opts) }, + { name: 'check', fn: (opts) => check('Hello\n', opts) }, + ]; + + for (const { name, fn } of methods) { + let nativeThrew = false; + try { fn({ basePath: undefined }); } catch { nativeThrew = true; } + + // WASM path via subprocess. init() must be awaited before compile/check. + const { execFileSync } = await import('node:child_process'); + let wasmThrew = false; try { - await checkFile(file, { basePath: '.' }); - } catch (e) { - errorMsg = e instanceof Error ? e.message : String(e); + execFileSync(process.execPath, ['--input-type=module'], { + input: `import { init, ${name} } from './dist/node.js'; await init(); try { ${name}('Hello\\n', { basePath: undefined }); process.exit(0); } catch { process.exit(1); }`, + cwd: new URL('..', import.meta.url).pathname, + env: { ...process.env, MDS_BACKEND: 'wasm' }, + timeout: 15000, + }); + } catch { + wasmThrew = true; } - assert.ok( - !errorMsg.startsWith('unknown option key "basePath"'), - `wrapper must not intercept basePath for checkFile with a generic message; got: "${errorMsg}"`, + + assert.strictEqual( + nativeThrew, + wasmThrew, + `${name}: native (threw=${nativeThrew}) and WASM (threw=${wasmThrew}) must agree on {basePath: undefined}`, + ); + } + }); + + // ── native lint still honors basePath (AC-P3-11 / U-OV-30) ────────────── + + test('U-OV-30: native lint still honors basePath — no regression from WASM guard (AC-P3-11)', () => { + requireNativeAddon(); // hard-fail without addon + // Positive: lint with correct basePath must NOT throw. + const result = lint(IMPORT_SRC, { basePath: FIXTURES }); + assert.equal(result.version, 1); + assert.ok(Array.isArray(result.files)); + assert.equal(result.truncated, false); + + // Positive control: wrong basePath causes a throw, proving basePath is operative. + const empty = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-bp-')); + try { + assert.throws( + () => lint(IMPORT_SRC, { basePath: empty }), + (err) => { + assert.ok(isMdsError(err), `expected isMdsError for wrong basePath: ${err}`); + return true; + }, + 'wrong basePath must throw — proves the value is honored, not just the key', ); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + fs.rmSync(empty, { recursive: true, force: true }); + } + }); + + // ── plural-form message parity (AC-P3-17 / U-OV-31) ───────────────────── + + test('U-OV-31: wrapper error message is byte-identical to napi for multiple-unknown-key form, all 7 methods (AC-P3-17)', async () => { + // Hard-fail without addon — same rationale as U-OV-14 (avoids PF-013). + const addon = require('@mdscript/mds-napi'); + + // Two unknown keys: triggers the plural "unknown option keys:" form. + const BAD_OPT = { sourceMaps: true, varsJson: '{}' }; + const VIRTUAL_MODS = { 'a.mds': '' }; + const VIRTUAL_ENTRY = 'a.mds'; + + async function captureMsg(fn) { + try { + const r = fn(); + if (r != null && typeof r === 'object' && typeof r.then === 'function') await r; + } catch (e) { return e instanceof Error ? e.message : String(e); } + return ''; + } + + const cases = [ + { name: 'compile', wrapperFn: () => compile('', BAD_OPT), addonFn: () => addon.compile('', BAD_OPT) }, + { name: 'check', wrapperFn: () => check('', BAD_OPT), addonFn: () => addon.check('', BAD_OPT) }, + { name: 'compileFile', wrapperFn: () => compileFile('/nonexistent.mds', BAD_OPT), addonFn: () => addon.compileFile('/nonexistent.mds', BAD_OPT) }, + { name: 'checkFile', wrapperFn: () => checkFile('/nonexistent.mds', BAD_OPT), addonFn: () => addon.checkFile('/nonexistent.mds', BAD_OPT) }, + { name: 'lint', wrapperFn: () => lint('', BAD_OPT), addonFn: () => addon.lint('', BAD_OPT) }, + { name: 'lintFile', wrapperFn: () => lintFile('/nonexistent.mds', BAD_OPT), addonFn: () => addon.lintFile('/nonexistent.mds', BAD_OPT) }, + { name: 'lintVirtual', wrapperFn: () => lintVirtual(VIRTUAL_MODS, VIRTUAL_ENTRY, BAD_OPT), addonFn: () => addon.lintVirtual(VIRTUAL_MODS, VIRTUAL_ENTRY, BAD_OPT) }, + ]; + + for (const { name, wrapperFn, addonFn } of cases) { + const wrapperMsg = await captureMsg(wrapperFn); + const addonMsg = await captureMsg(addonFn); + assert.ok(wrapperMsg.length > 0, `wrapper must throw for ${name} (positive control)`); + assert.ok(addonMsg.length > 0, `napi must throw for ${name} (positive control)`); + assert.strictEqual( + wrapperMsg, + addonMsg, + `plural form must be byte-identical for ${name} — wrapper: "${wrapperMsg}" | napi: "${addonMsg}"`, + ); } }); diff --git a/packages/mds/__test__/types/consumer-browser.ts b/packages/mds/__test__/types/consumer-browser.ts new file mode 100644 index 00000000..e12d1624 --- /dev/null +++ b/packages/mds/__test__/types/consumer-browser.ts @@ -0,0 +1,44 @@ +/** + * Type-level matrix verification for the browser entry point. + * + * AC-P3-16: all lint types a TypeScript consumer must name to use the browser + * lint API surface are exported from dist/browser.d.ts and compile correctly + * under the repo's strict settings. + * + * AC-P3-20 (browser surface): basePath is NOT accepted on CompileOptions or + * CheckOptions from the browser entry — it IS in those types (D-TS-01), but + * since the WASM backend rejects it at runtime, the type is honest. Same + * `@ts-expect-error` guards as consumer-node.ts for FileOptions etc. + */ +import type { + CheckOptions, + CompileOptions, + LintDiagnostic, + LintFileOptions, + LintFileReport, + LintOptions, + LintResult, + LintRuleName, + LintSpan, + RuleSeverity, +} from '../../dist/browser.js'; + +// ── Positive: basePath accepted on string-surface types ─────────────────────── +const _compileOpts: CompileOptions = { basePath: '/dir', sourceMap: true }; +const _checkOpts: CheckOptions = { basePath: '/dir' }; +const _lintOpts: LintOptions = { basePath: '/dir', rules: {} }; + +// ── Negative: basePath NOT accepted on LintFileOptions ──────────────────────── +// @ts-expect-error — basePath is intentionally absent from LintFileOptions +const _lintFileOpts: LintFileOptions = { basePath: '/dir' }; + +// ── AC-P3-16: all seven lint types are nameable from the browser entry ──────── +const _diagArr: LintDiagnostic[] = []; +const _span: LintSpan = { offset: 0, length: 0 }; +const _report: LintFileReport = { file: 'a.mds', diagnostics: _diagArr }; +const _result: LintResult = { version: 1, files: [_report], truncated: false }; +const _severity: RuleSeverity = 'error'; +const _ruleName: LintRuleName = 'empty-block'; + +void _compileOpts; void _checkOpts; void _lintOpts; void _lintFileOpts; +void _diagArr; void _span; void _report; void _result; void _severity; void _ruleName; diff --git a/packages/mds/__test__/types/consumer-node.ts b/packages/mds/__test__/types/consumer-node.ts new file mode 100644 index 00000000..fba785ba --- /dev/null +++ b/packages/mds/__test__/types/consumer-node.ts @@ -0,0 +1,79 @@ +/** + * Type-level matrix verification for the Node.js entry point. + * + * AC-P3-20: verify that `basePath` is accepted on string-surface types and + * rejected on file-surface types, using `@ts-expect-error` on every negative + * case. An UNUSED `@ts-expect-error` fails the build, which is the type-level + * positive control: if `basePath` were ever added to a file-surface type, + * tsc reports "Unused @ts-expect-error directive" and the run fails. + * + * AC-P3-21: all option types a consumer needs are exported from dist/node.d.ts + * (the entry the `exports` map resolves for Node.js consumers). + * + * Note: this file is compiled by tsconfig.types.json (noEmit: true) and is NOT + * in the main build (tsconfig.json excludes __test__). It runs as part of + * `npm test` via the test script preamble. + */ +import type { + CheckFileOptions, + CheckOptions, + CompileOptions, + FileOptions, + LintDiagnostic, + LintFileOptions, + LintFileReport, + LintOptions, + LintResult, + LintRuleName, + LintSpan, + RuleSeverity, +} from '../../dist/node.js'; + +// ── Positive cases: basePath accepted on string-surface types ───────────────── + +// D-TS-01: CompileOptions must accept basePath (inherited via CheckOptions). +const _compileOpts: CompileOptions = { basePath: '/some/dir', vars: {}, sourceMap: true }; + +// D-TS-01: CheckOptions must accept basePath. +const _checkOpts: CheckOptions = { basePath: '/some/dir', vars: {} }; + +// LintOptions already had basePath; confirm it still does. +const _lintOpts: LintOptions = { basePath: '/some/dir', vars: {}, rules: { 'unused-variable': 'warn' } }; + +// ── Negative cases: basePath NOT accepted on file-surface types ─────────────── +// Each @ts-expect-error is self-verifying: if basePath were ever added to these +// types, tsc emits "Unused @ts-expect-error directive" and the build fails. + +// D-TS-02: FileOptions must NOT have basePath. +// @ts-expect-error — basePath is intentionally absent from FileOptions (D-TS-02) +const _fileOpts: FileOptions = { basePath: '/some/dir' }; + +// CheckFileOptions must NOT have basePath. +// @ts-expect-error — basePath is intentionally absent from CheckFileOptions +const _checkFileOpts: CheckFileOptions = { basePath: '/some/dir' }; + +// LintFileOptions must NOT have basePath. +// @ts-expect-error — basePath is intentionally absent from LintFileOptions +const _lintFileOpts: LintFileOptions = { basePath: '/some/dir' }; + +// ── PR2 guard: invalid rule name must be rejected in LintOptions ────────────── +// D-224-1 introduced LintRuleName; ensure the type fixture protects both PRs. +const _validRule: LintOptions = { rules: { 'unused-variable': 'warn' } }; +// Record is accepted for forward compatibility. +const _fwdCompat: LintOptions = { rules: { 'a-future-rule': 'off' } }; + +// ── AC-P3-16: all lint types are nameable from the browser surface ───────────── +// (browser types are verified in consumer-browser.ts; here we just confirm they +// compile correctly when imported from the node entry.) +const _diagArr: LintDiagnostic[] = []; +const _span: LintSpan = { offset: 0, length: 0 }; +const _report: LintFileReport = { file: 'a.mds', diagnostics: _diagArr }; +const _result: LintResult = { version: 1, files: [_report], truncated: false }; +const _severity: RuleSeverity = 'warn'; +const _ruleName: LintRuleName = 'unused-variable'; + +// Prevent unused-variable TS errors for the above declarations. +void _compileOpts; void _checkOpts; void _lintOpts; +void _fileOpts; void _checkFileOpts; void _lintFileOpts; +void _validRule; void _fwdCompat; +void _diagArr; void _span; void _report; void _result; void _severity; void _ruleName; diff --git a/packages/mds/__test__/wasm-backend.spec.mjs b/packages/mds/__test__/wasm-backend.spec.mjs index f15bb9d6..118a79ea 100644 --- a/packages/mds/__test__/wasm-backend.spec.mjs +++ b/packages/mds/__test__/wasm-backend.spec.mjs @@ -5,7 +5,7 @@ * Imports dist/backend/wasm.js directly to exercise internal state * without going through the full node.ts entry point. */ -import { test, describe, afterEach } from 'node:test'; +import { test, describe, before, afterEach } from 'node:test'; import assert from 'node:assert/strict'; import { initWasmNode, initWasmBrowser, createWasmBackend, _resetForTesting, validateWasmShape } from '../dist/backend/wasm.js'; @@ -264,6 +264,97 @@ describe('wasm backend — browser circuit breaker', () => { }); }); +// --------------------------------------------------------------------------- +// WASM basePath rejection (OD-1 / AC-P3-09 / AC-P3-10) +// --------------------------------------------------------------------------- + +describe('wasm backend — basePath rejection (OD-1)', () => { + let wasmMod; + before(async () => { + _resetForTesting(0); + wasmMod = await initWasmNode(); + }); + + afterEach(() => { + _resetForTesting(0); + }); + + test('U-WB22: createWasmBackend.compile throws mds::invalid_options when basePath is set (AC-P3-09)', () => { + const be = createWasmBackend(wasmMod); + let caughtErr; + try { be.compile('Hello\n', { basePath: '.' }); } catch (e) { caughtErr = e; } + + assert.ok(caughtErr instanceof Error, 'must throw an Error'); + assert.equal(caughtErr.code, 'mds::invalid_options'); + assert.ok( + caughtErr.message.includes('basePath'), + `message must name basePath: "${caughtErr.message}"`, + ); + assert.ok( + caughtErr.message.includes('WASM') || caughtErr.message.includes('filesystem'), + `message must name the constraint: "${caughtErr.message}"`, + ); + assert.ok( + caughtErr.message.includes('MDS_BACKEND=native') || caughtErr.message.includes('native'), + `message must name a remedy: "${caughtErr.message}"`, + ); + + // AC-P3-10: message must NOT contain internal WASM keys. + assert.ok(!caughtErr.message.includes('filename'), `must not expose "filename": "${caughtErr.message}"`); + assert.ok(!caughtErr.message.includes('modules'), `must not expose "modules": "${caughtErr.message}"`); + + // PF-013 positive control: calling the raw WASM module directly DOES produce the + // internal "filename"/"modules" error — proving the negative assertion above can + // detect a leak if the guard were removed. + let rawErr; + try { wasmMod.compile('Hello\n', { basePath: '.' }); } catch (e) { rawErr = e; } + assert.ok(rawErr instanceof Error, 'raw WASM module must throw for unknown basePath'); + const rawMsg = rawErr.message ?? ''; + assert.ok( + rawMsg.includes('filename') || rawMsg.includes('modules'), + `raw WASM error must contain internal keys (positive control): "${rawMsg}"`, + ); + }); + + test('U-WB23: createWasmBackend.check throws mds::invalid_options when basePath is set (AC-P3-09)', () => { + const be = createWasmBackend(wasmMod); + assert.throws( + () => be.check('Hello\n', { basePath: '.' }), + (err) => { + assert.ok(err instanceof Error); + assert.equal(err.code, 'mds::invalid_options'); + assert.ok(err.message.includes('basePath')); + assert.ok(!err.message.includes('filename'), `must not expose "filename": "${err.message}"`); + assert.ok(!err.message.includes('modules'), `must not expose "modules": "${err.message}"`); + return true; + }, + ); + }); + + test('U-WB24: createWasmBackend.lint throws mds::invalid_options when basePath is set (AC-P3-09)', () => { + const be = createWasmBackend(wasmMod); + assert.throws( + () => be.lint('Hello\n', { basePath: '.' }), + (err) => { + assert.ok(err instanceof Error); + assert.equal(err.code, 'mds::invalid_options'); + assert.ok(err.message.includes('basePath')); + assert.ok(!err.message.includes('filename'), `must not expose "filename": "${err.message}"`); + assert.ok(!err.message.includes('modules'), `must not expose "modules": "${err.message}"`); + return true; + }, + ); + }); + + test('U-WB25: createWasmBackend accepts {basePath: undefined} for compile/check/lint (OD-6 semantic: undefined = absent)', () => { + const be = createWasmBackend(wasmMod); + // basePath: undefined is treated as absent (value-is-intent semantic). + assert.doesNotThrow(() => be.compile('Hello\n', { basePath: undefined })); + assert.doesNotThrow(() => be.check('Hello\n', { basePath: undefined })); + assert.doesNotThrow(() => be.lint('Hello\n', { basePath: undefined })); + }); +}); + describe('wasm backend — browser shape validation', () => { afterEach(() => { _resetForTesting(0); diff --git a/packages/mds/package.json b/packages/mds/package.json index b4738d2f..5cd6df74 100644 --- a/packages/mds/package.json +++ b/packages/mds/package.json @@ -39,7 +39,7 @@ ], "scripts": { "build": "tsc -p tsconfig.json", - "test": "node --test __test__/*.spec.mjs", + "test": "tsc -p tsconfig.types.json && node --test __test__/*.spec.mjs", "test:native": "node --test __test__/native-backend.spec.mjs", "test:perf": "node --test __test__/perf.spec.mjs" }, diff --git a/packages/mds/src/backend/native.ts b/packages/mds/src/backend/native.ts index e0bb39f3..5c87f162 100644 --- a/packages/mds/src/backend/native.ts +++ b/packages/mds/src/backend/native.ts @@ -1,5 +1,6 @@ import type { BackendType, + CheckFileOptions, CheckOptions, CheckResult, CompileOptions, @@ -10,15 +11,10 @@ import type { LintResult, MdsNodeBackend, } from '../types.js'; -import { compileOpt, varsOpt } from '../util/options.js'; +import { compileSrcOpt, checkSrcOpt, fileCompileOpt, fileCheckOpt } from '../util/options.js'; import { assertResultShape, validateBackendMethods, BASE_METHODS, NODE_METHODS } from './contract.js'; -/** Options forwarded to the napi addon for source-string lint (accepts basePath). */ -type NapiLintOpts = { basePath?: string; vars?: Record; rules?: Record }; -/** Options forwarded to the napi addon for file-based and virtual lint. */ -type NapiLintFileOpts = { vars?: Record; rules?: Record }; - -/** Options shape forwarded to the napi addon for compile (string source). */ +/** Options forwarded to the napi addon for source-string compile (accepts basePath). */ type NapiCompileOpts = { basePath?: string; vars?: Record; @@ -26,13 +22,24 @@ type NapiCompileOpts = { sourcesContent?: boolean; }; -/** Options shape forwarded to the napi addon for compileFile. */ +/** Options forwarded to the napi addon for source-string check (accepts basePath). */ +type NapiCheckOpts = { + basePath?: string; + vars?: Record; +}; + +/** Options forwarded to the napi addon for compileFile (no basePath). */ type NapiFileCompileOpts = { vars?: Record; sourceMap?: boolean; sourcesContent?: boolean; }; +/** Options forwarded to the napi addon for source-string lint (accepts basePath). */ +type NapiLintOpts = { basePath?: string; vars?: Record; rules?: Record }; +/** Options forwarded to the napi addon for file-based and virtual lint. */ +type NapiLintFileOpts = { vars?: Record; rules?: Record }; + /** * Shape of the napi addon exports. * compile/check accept { basePath?, vars?, sourceMap?, sourcesContent? } for string sources. @@ -41,7 +48,7 @@ type NapiFileCompileOpts = { */ interface NapiAddon { compile(source: string, opts?: NapiCompileOpts): unknown; - check(source: string, opts?: { basePath?: string; vars?: Record }): unknown; + check(source: string, opts?: NapiCheckOpts): unknown; compileFile(path: string, opts?: NapiFileCompileOpts): unknown; checkFile(path: string, opts?: { vars?: Record }): unknown; lint(source: string, opts?: NapiLintOpts): unknown; @@ -85,25 +92,25 @@ export function createNativeBackend(addon: NapiAddon): MdsNodeBackend { return { compile(source: string, options?: CompileOptions): CompileResult { - const result: unknown = addon.compile(source, compileOpt(options) as NapiCompileOpts | undefined); + const result: unknown = addon.compile(source, compileSrcOpt(options) as NapiCompileOpts | undefined); assertResultShape(result, 'compile'); return result as CompileResult; }, check(source: string, options?: CheckOptions): CheckResult { - const result: unknown = addon.check(source, varsOpt(options)); + const result: unknown = addon.check(source, checkSrcOpt(options) as NapiCheckOpts | undefined); assertResultShape(result, 'check'); return result as CheckResult; }, async compileFile(path: string, options?: FileOptions): Promise { - const result: unknown = await addon.compileFile(path, compileOpt(options) as NapiFileCompileOpts | undefined); + const result: unknown = await addon.compileFile(path, fileCompileOpt(options) as NapiFileCompileOpts | undefined); assertResultShape(result, 'compile'); return result as CompileResult; }, - async checkFile(path: string, options?: CheckOptions): Promise { - const result: unknown = await addon.checkFile(path, varsOpt(options)); + async checkFile(path: string, options?: CheckFileOptions): Promise { + const result: unknown = await addon.checkFile(path, fileCheckOpt(options)); assertResultShape(result, 'check'); return result as CheckResult; }, diff --git a/packages/mds/src/backend/wasm.ts b/packages/mds/src/backend/wasm.ts index ed6e00b7..a9b844d9 100644 --- a/packages/mds/src/backend/wasm.ts +++ b/packages/mds/src/backend/wasm.ts @@ -12,7 +12,7 @@ import type { MdsBaseBackend, } from '../types.js'; import { assertResultShape, validateBackendMethods, WASM_EXPORTS } from './contract.js'; -import { compileOpt } from '../util/options.js'; +import { fileCompileOpt } from '../util/options.js'; /** * Shape of the WASM module exports (built with wasm-pack). @@ -333,16 +333,18 @@ const DEFAULT_COMPILE_OPTS = Object.freeze({ /** * Extended options accepted by the internal WASM `compile` entry point. * - * The public `CompileOptions` type omits `filename` and `modules` because - * callers of the high-level `compile(source, options)` wrapper do not need - * them — the WASM module uses sensible defaults for the string-compile path. + * D-TS-06: extends `Omit`, not `CompileOptions` + * directly. After `CompileOptions` gained `basePath` (fix #180), inheriting it + * here would silently widen the internal WASM input type. The WASM module has no + * filesystem access, so `basePath` is explicitly excluded from this internal type. + * The basePath guard in `createWasmBackend.compile/check/lint` fires BEFORE this + * type is used, so no runtime leak is possible (avoids PF-004). * - * This internal extension is used by `compileOpts()` so that callers who go - * through `createWasmBackend` directly (e.g. `wrapWithFileOps`, CF-SM1 test) - * and supply `filename`/`modules` explicitly get the expected WASM behaviour. - * The public API contract is unchanged: no public method accepts these keys. + * `filename` and `modules` are WASM-internal: callers who go through + * `createWasmBackend` directly (e.g. `wrapWithFileOps`, CF-SM1 test) and supply + * these keys explicitly get the expected WASM behaviour. No public method accepts them. */ -interface _WasmCompileInput extends CompileOptions { +interface _WasmCompileInput extends Omit { filename?: string; modules?: Record; } @@ -357,7 +359,9 @@ function compileOpts( sourceMap?: boolean; sourcesContent?: boolean; } { - const extra = compileOpt(options); + // D-TS-06: use fileCompileOpt (no basePath) — basePath is already excluded + // from _WasmCompileInput, and the caller guards against it before reaching here. + const extra = fileCompileOpt(options as Parameters[0]); const filename = options?.filename ?? DEFAULT_COMPILE_OPTS.filename; const modules = options?.modules ?? DEFAULT_COMPILE_OPTS.modules; if (extra == null && filename === DEFAULT_COMPILE_OPTS.filename && modules === DEFAULT_COMPILE_OPTS.modules) { @@ -388,12 +392,34 @@ export function fileOpts( sourceMap?: boolean; sourcesContent?: boolean; } { - const extra = compileOpt(options); + const extra = fileCompileOpt(options); return extra != null ? { filename: entryFilename, modules, ...extra } : { filename: entryFilename, modules }; } +/** + * Throw `mds::invalid_options` when a caller passes `basePath` to a WASM-backend + * string-surface method (compile, check, lint). + * + * OD-1: the WASM backend has no filesystem access, so a non-null `basePath` cannot + * be honoured. Throwing instead of silently ignoring surfaces the misconfiguration + * rather than linting / compiling a partially-resolved module graph (avoids PF-004). + * + * AC-P3-10: the message MUST NOT contain "filename" or "modules" (internal WASM + * keys the public API never exposes). Verified by the PF-013-controlled negative + * assertion in U-WB22: the raw wasmModule.compile call DOES contain those strings, + * proving the negative assertion can detect the leak if the guard is removed. + */ +function throwWasmBasePathError(): never { + const err = new Error( + 'option "basePath" is not supported by the WASM backend (no filesystem access); ' + + 'set MDS_BACKEND=native to use the native backend', + ) as Error & { code: string }; + err.code = 'mds::invalid_options'; + throw err; +} + /** * Create a WASM backend instance from a pre-initialized WasmModule. * @@ -406,20 +432,24 @@ export function fileOpts( export function createWasmBackend(wasmModule: WasmModule): MdsBaseBackend { return { compile(source: string, options?: CompileOptions): CompileResult { + // OD-1: reject basePath rather than silently ignoring it (avoids PF-004). + if (options?.basePath != null) throwWasmBasePathError(); const result: unknown = wasmModule.compile(source, compileOpts(options as _WasmCompileInput)); assertResultShape(result, 'compile'); return result as CompileResult; }, check(source: string, options?: CheckOptions): CheckResult { + // OD-1: reject basePath rather than silently ignoring it. + if (options?.basePath != null) throwWasmBasePathError(); const result: unknown = wasmModule.check(source, checkOpts(options)); assertResultShape(result, 'check'); return result as CheckResult; }, lint(source: string, options?: LintOptions): LintResult { - // WASM backend does not support basePath (no filesystem access). - // vars and rules are forwarded; basePath is silently ignored. + // OD-1: reject basePath rather than silently ignoring it. + if (options?.basePath != null) throwWasmBasePathError(); const opts: { vars?: Record; rules?: Record; diff --git a/packages/mds/src/browser.ts b/packages/mds/src/browser.ts index e7dd1443..5c0f397d 100644 --- a/packages/mds/src/browser.ts +++ b/packages/mds/src/browser.ts @@ -1,8 +1,19 @@ -import type { BackendType, CheckOptions, CheckResult, CompileOptions, CompileResult, InitOptions, MdsBaseBackend } from './types.js'; +import type { + BackendType, + CheckOptions, + CheckResult, + CompileOptions, + CompileResult, + InitOptions, + LintFileOptions, + LintOptions, + LintResult, + MdsBaseBackend, +} from './types.js'; import { initWasmBrowser, createWasmBackend } from './backend/wasm.js'; import { assertKnownKeys } from './util/options.js'; -export { isMdsError } from './types.js'; +export { isMdsError, LINT_RULE_NAMES } from './types.js'; export type { BackendType, CheckOptions, @@ -10,11 +21,19 @@ export type { CompileOptions, CompileResult, InitOptions, + LintDiagnostic, + LintFileOptions, + LintFileReport, + LintOptions, + LintResult, + LintRuleName, + LintSpan, MarkdownResult, Message, MessagesResult, MdsError, MdsErrorSpan, + RuleSeverity, } from './types.js'; let resolvedBackend: MdsBaseBackend | undefined; @@ -52,7 +71,7 @@ export function _initWithModuleForTesting(mod: import('./backend/wasm.js').WasmM } /** - * Initialize the WASM backend. Must be called before compile/check in browser environments. + * Initialize the WASM backend. Must be called before compile/check/lint in browser environments. * * Idempotent — safe to call multiple times. Concurrent calls in flight share * the same promise, preventing double-init races. On transient failure the @@ -75,7 +94,7 @@ export function init(options?: InitOptions): Promise { function assertReady(): MdsBaseBackend { if (resolvedBackend === undefined) { - throw new Error('@mdscript/mds: call await init() before using compile/check in a browser environment'); + throw new Error('@mdscript/mds: call await init() before using compile/check/lint in a browser environment'); } return resolvedBackend; } @@ -92,6 +111,31 @@ export function check(source: string, options?: CheckOptions): CheckResult { return assertReady().check(source, options); } +/** + * Lint an MDS source string. Returns a LintResult with per-rule findings. + * Requires init() to have been called and awaited first. + * + * D-TS-07: `lintFile` is intentionally absent from the browser entry. + * `MdsBaseBackend` has no `lintFile` — file operations require `node:fs` which + * is unavailable in browser environments. Use `lintVirtual` to lint a + * pre-loaded module map, or import from `@mdscript/mds` in Node.js to get + * access to `lintFile`. + */ +export function lint(source: string, options?: LintOptions): LintResult { + if (options != null) assertKnownKeys(options, 'lint'); + return assertReady().lint(source, options); +} + +/** Lint a multi-module virtual filesystem. Caller provides the full module map and entry key. Requires init() to have been called and awaited first. */ +export function lintVirtual( + modules: Record, + entry: string, + options?: LintFileOptions, +): LintResult { + if (options != null) assertKnownKeys(options, 'lintVirtual'); + return assertReady().lintVirtual(modules, entry, options); +} + /** Returns the active backend type. Always `'wasm'` in browser environments. */ export function getBackend(): BackendType { return 'wasm'; diff --git a/packages/mds/src/index.ts b/packages/mds/src/index.ts index 579d7a3f..3b624615 100644 --- a/packages/mds/src/index.ts +++ b/packages/mds/src/index.ts @@ -1,11 +1,17 @@ +// NOTE (AC-P3-21): index.ts is NOT in the package `exports` map and has no +// `main`/`types` fallback. dist/index.js is unreachable to consumers. This +// barrel is an internal convenience for repo-level tooling only. Public types +// are exported from dist/node.d.ts (Node) and dist/browser.d.ts (browser) +// via the `"."` exports-map entry in package.json. export type { - CompileResult, - MarkdownResult, - MessagesResult, - Message, + BackendType, + CheckFileOptions, + CheckOptions, CheckResult, CompileOptions, + CompileResult, FileOptions, + InitOptions, LintDiagnostic, LintFileOptions, LintFileReport, @@ -13,14 +19,16 @@ export type { LintResult, LintRuleName, LintSpan, - RuleSeverity, - MdsErrorSpan, - MdsError, - BackendType, - InitOptions, + MarkdownResult, MdsBackend, MdsBaseBackend, + MdsError, + MdsErrorSpan, MdsNodeBackend, + Message, + MessagesResult, + RuleSeverity, + SourceMapV3, } from './types.js'; export { isMdsError, LINT_RULE_NAMES } from './types.js'; export type { WasmModule } from './backend/wasm.js'; diff --git a/packages/mds/src/node.ts b/packages/mds/src/node.ts index 388f521f..fd9e0be6 100644 --- a/packages/mds/src/node.ts +++ b/packages/mds/src/node.ts @@ -1,16 +1,17 @@ import type { BackendType, - MdsBaseBackend, - MdsNodeBackend, + CheckFileOptions, CheckOptions, - CompileResult, CheckResult, CompileOptions, + CompileResult, FileOptions, InitOptions, LintFileOptions, LintOptions, LintResult, + MdsBaseBackend, + MdsNodeBackend, } from './types.js'; import { assertResultShape } from './backend/contract.js'; import { initWasmNode, createWasmBackend, fileOpts } from './backend/wasm.js'; @@ -53,6 +54,32 @@ export function _resetForTesting(): void { // File-ops wrapper // --------------------------------------------------------------------------- +/** + * Build the `mds::invalid_options` error for `basePath` on file-surface methods. + * + * Separated from the throw so that both the `wrapWithFileOps` guards (synchronous + * throw inside an async function → rejected promise) and the public API guards + * (`Promise.reject(fileBasePathError())` — maintains the sync-throw contract for + * other errors while making basePath a proper promise rejection) can share one + * message string, satisfying U-OV-27's byte-identical requirement (avoids PF-007). + * + * Message is byte-identical to napi parse_file_opts (lib.rs) so that U-OV-27 + * can assert runtime equality across both backends. + */ +function fileBasePathError(): Error & { code: string } { + const err = new Error( + 'option "basePath" is not valid for compileFile/checkFile; ' + + 'the base directory is derived from the file path', + ) as Error & { code: string }; + err.code = 'mds::invalid_options'; + return err; +} + +/** Throw the file-surface basePath error (for use inside async functions). */ +function throwFileBasePathError(): never { + throw fileBasePathError(); +} + /** * Wrap a MdsBaseBackend with file-based compile/check operations, producing * a MdsNodeBackend. The wasmModule is captured so compileFile/checkFile can @@ -90,14 +117,29 @@ function wrapWithFileOps( ...base, async compileFile(path: string, options?: FileOptions): Promise { + // D-TS-06 guard: basePath is not valid for file operations on the WASM path. + // The native path never enters wrapWithFileOps, so napi handles the rejection + // there. BASEPATH_PASSTHROUGH lets basePath through assertKnownKeys; the guard + // here fires on the WASM path before buildModulesMap runs (avoids PF-004). + if ((options as unknown as { basePath?: string })?.basePath != null) { + throwFileBasePathError(); + } const { source, opts } = await prepareFileArgs(path, options); const result: unknown = wasmModule.compile(source, opts); assertResultShape(result, 'compile'); return result as CompileResult; }, - async checkFile(path: string, options?: CheckOptions): Promise { - const { source, opts } = await prepareFileArgs(path, options); + async checkFile(path: string, options?: CheckFileOptions): Promise { + // D-TS-06 guard: same reason as compileFile above. + if ((options as unknown as { basePath?: string })?.basePath != null) { + throwFileBasePathError(); + } + // CheckFileOptions is a structural subset of FileOptions (only vars, no + // sourceMap/sourcesContent), so the cast is safe: prepareFileArgs calls + // fileCompileOpt which only picks defined keys; the absent fields resolve as + // undefined and are not included in the returned opts object. + const { source, opts } = await prepareFileArgs(path, options as FileOptions | undefined); const result: unknown = wasmModule.check(source, opts); assertResultShape(result, 'check'); return result as CheckResult; @@ -252,19 +294,41 @@ export function check(source: string, options?: CheckOptions): CheckResult { return assertReady().check(source, options); } -/** Compile an MDS file, resolving @import directives relative to the file. Returns a discriminated-union CompileResult. Requires init() to have been called and awaited first. */ +/** + * Compile an MDS file, resolving @import directives relative to the file. + * Returns a discriminated-union CompileResult. Requires init() to have been called and awaited first. + * + * Non-async: unknown option keys and pre-init errors throw synchronously (preserving the + * existing contract verified by U-OV-12 and U-B11). The basePath guard returns + * `Promise.reject(fileBasePathError())` so that callers using `assert.rejects()` or + * `.catch()` receive a proper rejected promise rather than a synchronous throw. + */ export function compileFile(path: string, options?: FileOptions): Promise { if (options != null) assertKnownKeys(options, 'compileFile'); + // BASEPATH_PASSTHROUGH: assertKnownKeys skips basePath for file methods (issue #74). + // Return a rejected promise (not a synchronous throw) so that the file-op async + // contract is consistent: backend errors arrive as rejections, not sync throws. + // wrapWithFileOps provides a redundant guard on the WASM path (avoids PF-004). + if ((options as unknown as { basePath?: string })?.basePath != null) { + return Promise.reject(fileBasePathError()); + } return assertReady().compileFile(path, options); } /** * Validate an MDS file without rendering, resolving @import directives relative to the file. - * Only `vars` is forwarded; source-map options are not applicable to check operations. + * Only `vars` is forwarded; `basePath` and source-map options are not applicable to file + * operations (the base directory is derived from the file path). * Requires init() to have been called and awaited first. + * + * Same async contract as compileFile: basePath guard returns `Promise.reject()`. */ -export function checkFile(path: string, options?: CheckOptions): Promise { +export function checkFile(path: string, options?: CheckFileOptions): Promise { if (options != null) assertKnownKeys(options, 'checkFile'); + // Same basePath guard — returns Promise.reject to preserve async contract. + if ((options as unknown as { basePath?: string })?.basePath != null) { + return Promise.reject(fileBasePathError()); + } return assertReady().checkFile(path, options); } @@ -303,6 +367,7 @@ export function getBackend(): BackendType { export { isMdsError, LINT_RULE_NAMES } from './types.js'; export type { BackendType, + CheckFileOptions, CheckOptions, CheckResult, CompileOptions, diff --git a/packages/mds/src/types.ts b/packages/mds/src/types.ts index 9536ed98..d5b6d43f 100644 --- a/packages/mds/src/types.ts +++ b/packages/mds/src/types.ts @@ -84,18 +84,33 @@ export interface CheckResult { /** * Options for check-only operations (no source-map generation). - * Accepted by {@link MdsBaseBackend.check} and {@link MdsNodeBackend.checkFile}. + * Accepted by {@link MdsBaseBackend.check}. + * + * D-TS-01: `basePath` is added here so that {@link CompileOptions} inherits it + * via `extends CheckOptions`. Both compile and check resolve `@import` directives + * against this directory when the source string is not backed by a file path. + * The WASM backend rejects a non-null `basePath` at runtime (no filesystem access); + * set `MDS_BACKEND=native` to use the native backend with import resolution. */ export interface CheckOptions { /** Runtime variables made available for interpolation in the template. */ vars?: Record; + /** + * Base directory for resolving `@import` directives in the source string. + * Required when the source contains `@import` or `@extends`. + * + * **WASM backend:** rejected at runtime with `mds::invalid_options` — the WASM + * backend has no filesystem access. Set `MDS_BACKEND=native` to force the native + * backend, or supply all modules inline with `lintVirtual`. + */ + basePath?: string; } /** * Options for compile operations. * * Extends {@link CheckOptions}: check accepts a strict subset of compile's options. - * The `vars` field is inherited from {@link CheckOptions}. + * The `vars` and `basePath` fields are inherited from {@link CheckOptions}. */ export interface CompileOptions extends CheckOptions { /** @@ -119,11 +134,36 @@ export interface CompileOptions extends CheckOptions { /** * Options for file-based compile operations. * - * Structurally identical to {@link CompileOptions} (inherits `vars`, `sourceMap`, - * `sourcesContent`). Kept as a distinct named type so `compileFile` and - * `checkFile` can evolve their option sets independently. + * D-TS-02: `FileOptions` deliberately does NOT extend `CompileOptions`. After + * `CompileOptions` gained `basePath`, inheriting it here would silently add a + * field that is not valid for file-surface operations (the base directory is + * derived from the file path). The fields are declared directly so that adding + * a new string-surface option never implicitly appears on the file surface. */ -export interface FileOptions extends CompileOptions {} +export interface FileOptions { + /** Runtime variables made available for interpolation in the template. */ + vars?: Record; + /** When `true`, appends a Source Map v3 document to the result. */ + sourceMap?: boolean; + /** + * When `true`, embeds the original source text in `sourceMap.sourcesContent`. + * Requires `sourceMap: true`. + * + * **Privacy warning**: embeds the full template source. Only use in trusted environments. + */ + sourcesContent?: boolean; +} + +/** + * Options for file-based check-only operations. + * + * Mirrors {@link LintFileOptions}: `basePath` is absent because the base + * directory is derived from the file path. Only `vars` is forwarded to the backend. + */ +export interface CheckFileOptions { + /** Runtime variables made available for interpolation in the template. */ + vars?: Record; +} // --------------------------------------------------------------------------- // Lint types @@ -262,7 +302,12 @@ export interface LintOptions { /** * Base directory for resolving `@import` directives in the source string. * Required when the source contains `@import` or `@extends`. - * Ignored by the WASM backend (which cannot access the filesystem). + * + * OD-1 resolution: the WASM backend **rejects** a non-null `basePath` with + * `mds::invalid_options` rather than silently ignoring it. This surfaces the + * misconfiguration instead of linting a partially-resolved module graph. + * Set `MDS_BACKEND=native` to force the native backend, or use + * {@link MdsBaseBackend.lintVirtual} with pre-resolved modules. */ basePath?: string; } @@ -321,6 +366,11 @@ export interface InitOptions { /** * Browser-safe backend interface — compile/check/lint/lintVirtual/getBackend. * Does not include file operations (which require node:fs). + * + * D-TS-01: all string-surface methods accept `basePath` via their options type + * (`CompileOptions` / `CheckOptions` / `LintOptions`). The WASM implementation's + * runtime contract for `basePath`: a non-null value throws `mds::invalid_options` + * instead of silently ignoring it (OD-1; avoids PF-004). */ export interface MdsBaseBackend { compile(source: string, options?: CompileOptions): CompileResult; @@ -346,9 +396,10 @@ export interface MdsNodeBackend extends MdsBaseBackend { compileFile(path: string, options?: FileOptions): Promise; /** * Validate an MDS file without rendering. Only `vars` is forwarded; - * source-map options are not applicable to check operations. + * source-map options and `basePath` are not applicable to file-path operations + * (the base directory is derived from the file path). */ - checkFile(path: string, options?: CheckOptions): Promise; + checkFile(path: string, options?: CheckFileOptions): Promise; /** Lint an MDS file, resolving @import directives relative to the file. */ lintFile(path: string, options?: LintFileOptions): Promise; } diff --git a/packages/mds/src/util/options.ts b/packages/mds/src/util/options.ts index d825a9ab..9804baba 100644 --- a/packages/mds/src/util/options.ts +++ b/packages/mds/src/util/options.ts @@ -1,4 +1,11 @@ -import type { CompileOptions, CheckOptions, FileOptions, LintOptions, LintFileOptions } from '../types.js'; +import type { + CheckFileOptions, + CheckOptions, + CompileOptions, + FileOptions, + LintFileOptions, + LintOptions, +} from '../types.js'; // ── keysOf helper ────────────────────────────────────────────────────────────── @@ -39,48 +46,35 @@ export type MethodName = | 'lintFile' | 'lintVirtual'; -// ── Internal backend option shapes ──────────────────────────────────────────── - -// These represent what napi's option parsers accept — wider than the public -// TypeScript interfaces for compile and check, which intentionally omit basePath -// (open issue #180). The backend (parse_compile_opts / parse_check_opts) accepts -// basePath; the public TS types do not expose it yet. - -/** Backend-accepted options for `compile` — superset of public {@link CompileOptions}. */ -interface _CompileBackendOpts extends CompileOptions { - basePath?: string; -} - -/** Backend-accepted options for `check` — superset of public {@link CheckOptions}. */ -interface _CheckBackendOpts extends CheckOptions { - basePath?: string; -} - // ── Per-method key table ─────────────────────────────────────────────────────── /** * Allowed option keys per public wrapper method. * - * Each list is derived via {@link keysOf} from the corresponding backend option + * Each list is derived via {@link keysOf} from the corresponding public option * interface, binding the table to the interface at compile time. When a method's * accepted options change, the witness object in the {@link keysOf} call must be * updated, or the call becomes a type error. * + * CONSTRAINT: key ORDER within each witness literal is load-bearing — it + * determines the `recognised keys are: …` list that {@link assertKnownKeys} + * emits, which U-OV-14 byte-compares against the napi addon's output. Do not + * reorder without updating the expected strings in that test. + * * Reconciliation against napi option parsers (`crates/mds-napi/src/lib.rs`): - * - `compile`/`check`: `basePath` added — napi's `parse_compile_opts` / - * `parse_check_opts` accept it; the public TS types do not yet expose it - * (open issue #180). - * - `compileFile`/`checkFile`: `basePath` is NOT in the key list; instead it is - * passed through to the backend without wrapper interception so napi's purpose-built - * error fires ("not valid for compileFile/checkFile; the base directory is derived - * from the file path"). See {@link BASEPATH_PASSTHROUGH} and issue #74. + * - `compile` / `check`: `basePath` is now in the public types (#180 fix) and in + * the key list; napi's `parse_compile_opts` / `parse_check_opts` accept it. + * - `compileFile` / `checkFile`: `basePath` is NOT in the key list. It is passed + * through without wrapper interception so the backend can emit its own purpose-built + * error ("not valid for compileFile/checkFile; the base directory is derived from + * the file path"). See {@link BASEPATH_PASSTHROUGH} and issue #74. * - `lint`, `lintFile`, `lintVirtual`: key lists match napi exactly. */ const METHOD_KEYS: Readonly> = { - compile: keysOf<_CompileBackendOpts>({ basePath: true, vars: true, sourceMap: true, sourcesContent: true }), - check: keysOf<_CheckBackendOpts>({ basePath: true, vars: true }), + compile: keysOf({ basePath: true, vars: true, sourceMap: true, sourcesContent: true }), + check: keysOf({ basePath: true, vars: true }), compileFile: keysOf({ vars: true, sourceMap: true, sourcesContent: true }), - checkFile: keysOf({ vars: true }), + checkFile: keysOf({ vars: true }), lint: keysOf({ basePath: true, vars: true, rules: true }), lintFile: keysOf({ vars: true, rules: true }), lintVirtual: keysOf({ vars: true, rules: true }), @@ -107,7 +101,7 @@ const BASEPATH_PASSTHROUGH: ReadonlySet = new Set([ * `checkFile`, the wrapper and napi produce byte-identical messages for the same * unknown key. For `compileFile` and `checkFile`, `basePath` is not intercepted * here — it is passed through so the backend can emit its own purpose-built error - * (issue #74; open issue #180). + * (issue #74). * * The `method` parameter is typed as the {@link MethodName} literal union — * passing an unrecognised method name is a compile-time error, not a silent no-op. @@ -142,33 +136,72 @@ export function assertKnownKeys(options: object, method: MethodName): void { throw err; } +// ── Per-surface option builders ──────────────────────────────────────────────── +// +// D-TS-03 / D-TS-05: four typed builders, one per method-surface combination. +// Each builder returns `undefined` when no options are set (preserves the +// backend's fast path for no-options calls — avoids allocating an empty object +// on every invocation). The per-surface split ensures that adding a new field to +// a string-surface type (e.g. `CompileOptions`) cannot accidentally appear in the +// file-surface options forwarded to the backend. + /** - * Build the `{ vars }` sub-object only when `options.vars` is defined and non-null. + * Build options for the string-source compile surface. + * Picks `basePath`, `vars`, `sourceMap`, and `sourcesContent` from `CompileOptions`. * - * Used for check and checkFile where source-map options are not applicable. - * When the caller passes no vars, omitting the key entirely avoids unnecessary - * object creation and keeps the options shape minimal. + * D-TS-03: used by the native backend's `compile` method and by the WASM + * backend's `compileOpts()` wrapper (after the WASM basePath guard fires). */ -export function varsOpt( - options?: { vars?: Record }, -): { vars: Record } | undefined { - return options?.vars != null ? { vars: options.vars } : undefined; +export function compileSrcOpt(options?: CompileOptions): Partial | undefined { + if (options == null) return undefined; + const out: Partial = {}; + if (options.basePath != null) out.basePath = options.basePath; + if (options.vars != null) out.vars = options.vars; + if (options.sourceMap != null) out.sourceMap = options.sourceMap; + if (options.sourcesContent != null) out.sourcesContent = options.sourcesContent; + return Object.keys(out).length > 0 ? out : undefined; +} + +/** + * Build options for the string-source check surface. + * Picks `basePath` and `vars` from `CheckOptions`. + * + * D-TS-03: used by the native backend's `check` method. The WASM backend + * guards against `basePath` before calling `checkOpts()`. + */ +export function checkSrcOpt(options?: CheckOptions): Partial | undefined { + if (options == null) return undefined; + const out: Partial = {}; + if (options.basePath != null) out.basePath = options.basePath; + if (options.vars != null) out.vars = options.vars; + return Object.keys(out).length > 0 ? out : undefined; } /** - * Build the options object for compile/compileFile, forwarding vars, - * sourceMap, and sourcesContent when present and non-null. + * Build options for the file-surface compile path. + * Picks `vars`, `sourceMap`, and `sourcesContent` from `FileOptions`. + * `basePath` is intentionally absent (D-TS-02). * - * Returns `undefined` when no options are set so the backend receives no - * options argument (avoids allocating a needless empty object on the hot path). + * D-TS-03: used by the native backend's `compileFile` method and internally by + * the WASM backend's `compileOpts()` / `fileOpts()` helpers (which deal with + * `filename` and `modules` separately). */ -export function compileOpt( - options?: CompileOptions | FileOptions, -): { vars?: Record; sourceMap?: boolean; sourcesContent?: boolean } | undefined { +export function fileCompileOpt(options?: FileOptions): Partial | undefined { if (options == null) return undefined; - const out: { vars?: Record; sourceMap?: boolean; sourcesContent?: boolean } = {}; + const out: Partial = {}; if (options.vars != null) out.vars = options.vars; - if ((options as CompileOptions).sourceMap != null) out.sourceMap = (options as CompileOptions).sourceMap; - if ((options as CompileOptions).sourcesContent != null) out.sourcesContent = (options as CompileOptions).sourcesContent; + if (options.sourceMap != null) out.sourceMap = options.sourceMap; + if (options.sourcesContent != null) out.sourcesContent = options.sourcesContent; return Object.keys(out).length > 0 ? out : undefined; } + +/** + * Build options for the file-surface check path. + * Picks only `vars` from `CheckFileOptions`. + * `basePath`, `sourceMap`, and `sourcesContent` are all intentionally absent. + * + * D-TS-03: used by the native backend's `checkFile` method. + */ +export function fileCheckOpt(options?: CheckFileOptions): { vars: Record } | undefined { + return options?.vars != null ? { vars: options.vars } : undefined; +} diff --git a/packages/mds/tsconfig.types.json b/packages/mds/tsconfig.types.json new file mode 100644 index 00000000..46b746ee --- /dev/null +++ b/packages/mds/tsconfig.types.json @@ -0,0 +1,8 @@ +{ + "extends": "../../tsconfig.base.json", + "compilerOptions": { + "noEmit": true, + "rootDir": "__test__/types" + }, + "include": ["__test__/types/**/*.ts"] +} From 0330597d235d33ce3514b91104bda5c33f7f4564 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 19:54:25 +0300 Subject: [PATCH 02/26] refactor(mds): simplify option builders and remove throwFileBasePathError wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - options.ts: extract `pickDefined` helper to replace 24 lines of identical null-check + build + empty-guard boilerplate shared across compileSrcOpt, checkSrcOpt, and fileCompileOpt. Each builder is now a one-liner; fileCheckOpt is unchanged (already a one-liner with a distinct return type). - node.ts: remove `throwFileBasePathError()` — a 4-line wrapper that existed only to call `throw fileBasePathError()`. Replace both call sites with the direct throw. TypeScript's control-flow analysis handles throw in async functions correctly; the never-typed helper was adding indirection with no benefit. - browser.ts: expand single-line lintVirtual JSDoc to a multi-line block consistent with lint and the surrounding exported functions. All 295 tests pass. TypeScript build and type-fixture (tsconfig.types.json) both exit 0. Source hygiene gate exits 0. Rust scope unchanged. --- packages/mds/src/browser.ts | 6 ++++- packages/mds/src/node.ts | 9 ++------ packages/mds/src/util/options.ts | 39 +++++++++++++++++--------------- 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/packages/mds/src/browser.ts b/packages/mds/src/browser.ts index 5c0f397d..de66f63c 100644 --- a/packages/mds/src/browser.ts +++ b/packages/mds/src/browser.ts @@ -126,7 +126,11 @@ export function lint(source: string, options?: LintOptions): LintResult { return assertReady().lint(source, options); } -/** Lint a multi-module virtual filesystem. Caller provides the full module map and entry key. Requires init() to have been called and awaited first. */ +/** + * Lint a multi-module virtual filesystem. Caller provides the full module map + * and entry key. Returns a LintResult with per-rule findings. + * Requires init() to have been called and awaited first. + */ export function lintVirtual( modules: Record, entry: string, diff --git a/packages/mds/src/node.ts b/packages/mds/src/node.ts index fd9e0be6..0be59def 100644 --- a/packages/mds/src/node.ts +++ b/packages/mds/src/node.ts @@ -75,11 +75,6 @@ function fileBasePathError(): Error & { code: string } { return err; } -/** Throw the file-surface basePath error (for use inside async functions). */ -function throwFileBasePathError(): never { - throw fileBasePathError(); -} - /** * Wrap a MdsBaseBackend with file-based compile/check operations, producing * a MdsNodeBackend. The wasmModule is captured so compileFile/checkFile can @@ -122,7 +117,7 @@ function wrapWithFileOps( // there. BASEPATH_PASSTHROUGH lets basePath through assertKnownKeys; the guard // here fires on the WASM path before buildModulesMap runs (avoids PF-004). if ((options as unknown as { basePath?: string })?.basePath != null) { - throwFileBasePathError(); + throw fileBasePathError(); } const { source, opts } = await prepareFileArgs(path, options); const result: unknown = wasmModule.compile(source, opts); @@ -133,7 +128,7 @@ function wrapWithFileOps( async checkFile(path: string, options?: CheckFileOptions): Promise { // D-TS-06 guard: same reason as compileFile above. if ((options as unknown as { basePath?: string })?.basePath != null) { - throwFileBasePathError(); + throw fileBasePathError(); } // CheckFileOptions is a structural subset of FileOptions (only vars, no // sourceMap/sourcesContent), so the cast is safe: prepareFileArgs calls diff --git a/packages/mds/src/util/options.ts b/packages/mds/src/util/options.ts index 9804baba..91a257ee 100644 --- a/packages/mds/src/util/options.ts +++ b/packages/mds/src/util/options.ts @@ -145,6 +145,24 @@ export function assertKnownKeys(options: object, method: MethodName): void { // a string-surface type (e.g. `CompileOptions`) cannot accidentally appear in the // file-surface options forwarded to the backend. +/** + * Return a new object containing only the keys from `keys` whose values in + * `src` are non-null/undefined. Returns `undefined` when `src` is nullish or + * every selected value is absent — so callers can use `result ?? undefined` + * without allocating an empty object on every invocation (backend fast path). + * + * @internal + */ +function pickDefined( + src: T | null | undefined, + keys: readonly (keyof T)[], +): Partial | undefined { + if (src == null) return undefined; + const defined = keys.filter(k => src[k] != null); + if (defined.length === 0) return undefined; + return Object.fromEntries(defined.map(k => [k, src[k]])) as Partial; +} + /** * Build options for the string-source compile surface. * Picks `basePath`, `vars`, `sourceMap`, and `sourcesContent` from `CompileOptions`. @@ -153,13 +171,7 @@ export function assertKnownKeys(options: object, method: MethodName): void { * backend's `compileOpts()` wrapper (after the WASM basePath guard fires). */ export function compileSrcOpt(options?: CompileOptions): Partial | undefined { - if (options == null) return undefined; - const out: Partial = {}; - if (options.basePath != null) out.basePath = options.basePath; - if (options.vars != null) out.vars = options.vars; - if (options.sourceMap != null) out.sourceMap = options.sourceMap; - if (options.sourcesContent != null) out.sourcesContent = options.sourcesContent; - return Object.keys(out).length > 0 ? out : undefined; + return pickDefined(options, ['basePath', 'vars', 'sourceMap', 'sourcesContent']); } /** @@ -170,11 +182,7 @@ export function compileSrcOpt(options?: CompileOptions): Partial * guards against `basePath` before calling `checkOpts()`. */ export function checkSrcOpt(options?: CheckOptions): Partial | undefined { - if (options == null) return undefined; - const out: Partial = {}; - if (options.basePath != null) out.basePath = options.basePath; - if (options.vars != null) out.vars = options.vars; - return Object.keys(out).length > 0 ? out : undefined; + return pickDefined(options, ['basePath', 'vars']); } /** @@ -187,12 +195,7 @@ export function checkSrcOpt(options?: CheckOptions): Partial | und * `filename` and `modules` separately). */ export function fileCompileOpt(options?: FileOptions): Partial | undefined { - if (options == null) return undefined; - const out: Partial = {}; - if (options.vars != null) out.vars = options.vars; - if (options.sourceMap != null) out.sourceMap = options.sourceMap; - if (options.sourcesContent != null) out.sourcesContent = options.sourcesContent; - return Object.keys(out).length > 0 ? out : undefined; + return pickDefined(options, ['vars', 'sourceMap', 'sourcesContent']); } /** From ceb0fcacd0a321341cde7f3c5c8cd9f5ea52e103 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 20:04:07 +0300 Subject: [PATCH 03/26] fix: address self-review issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0 — Windows CI breakage (options-validation.spec.mjs) Three new sites used `new URL(..., import.meta.url).pathname` to derive filesystem paths. On win32 that yields "/C:/..." (leading slash, drive letter) and it percent-encodes spaces on every platform, so path.join and execFileSync's cwd receive unusable paths. windows-latest is in the `js` CI matrix, and every other spec in this package already uses fileURLToPath. Switched to fileURLToPath (TEST_DIR / PKG_DIR). P1 — U-OV-27 was tautological The test claimed to compare the compileFile/checkFile basePath rejection message across the native and WASM backends, but node.ts's public file methods short-circuit basePath BEFORE assertReady(), so neither side ever reached a backend. Verified empirically: the subprocess reports "BACKEND NEVER INITIALIZED". Both operands came from the same fileBasePathError(), i.e. the test asserted X === X and could not detect drift. Rewritten with the authoritative leg — wrapper vs the RAW napi addon, which owns the canonical string — plus the backend-independence leg and per-leg non-empty positive controls. Confirmed the new assertion fails when the wrapper message is mutated. P1 — U-BR17 assertion could not fail `assert.ok(fileNames.length >= 0)` holds for an empty array. Replaced with the assertions the plan specified: the entry key is present in the report and carries an unused-variable diagnostic. P1 — AC-P3-08 coverage gap U-OV-29 covered only compile/check. Extended to compileFile/checkFile, the interesting half: napi gates on property PRESENCE, so a builder that forwarded `basePath: undefined` verbatim would throw on native but not on WASM. Confirmed these legs reach the real WASM backend. P2 — accuracy and reachability - native.ts NapiAddon JSDoc claimed lintFile/lintVirtual accept basePath; napi rejects it on both. - Recorded the OD-5 residual on BASEPATH_PASSTHROUGH: lintFile/lintVirtual deliberately emit the generic message and diverge from napi's. - Corrected the consumer-node.ts comment that claimed invalid rule names are rejected (D-224-1 makes them a runtime warning) and added the real negative control: an invalid SEVERITY must be a type error. - Exported SourceMapV3 from node.ts and browser.ts. MarkdownResult.sourceMap is typed as it, but it was only re-exported from the unreachable index.ts barrel, so consumers could hold the value without naming its type. Additive; covered by both type fixtures. - Clarified the fileBasePathError JSDoc: napi never emits this message for a public call, so U-OV-27 is the only thing keeping the strings in lockstep. Rust untouched (git diff --name-only ...crates/ Cargo.toml Cargo.lock is empty, with packages/mds/ as positive control), so the WASM binary and the 850,000-byte guard are unaffected. --- CHANGELOG.md | 5 + packages/mds/__test__/browser.spec.mjs | 19 ++- .../mds/__test__/options-validation.spec.mjs | 130 ++++++++++++------ .../mds/__test__/types/consumer-browser.ts | 6 + packages/mds/__test__/types/consumer-node.ts | 29 +++- packages/mds/src/backend/native.ts | 4 +- packages/mds/src/browser.ts | 1 + packages/mds/src/node.ts | 9 +- packages/mds/src/util/options.ts | 9 ++ 9 files changed, 164 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5285114..df9c8f78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `LINT_RULE_NAMES` constant are now exported from both the Node.js and browser entry points. +- **`SourceMapV3` is now exported from the Node.js and browser entry points.** + `MarkdownResult.sourceMap` has always been typed as `SourceMapV3`, but the type was + only re-exported from `index.ts`, which the package `exports` map does not resolve — + so consumers could receive the value but not name its type. Purely additive. + ### **BREAKING** — TypeScript option types and WASM `basePath` rejection (#213, #180) #### `FileOptions` no longer extends `CompileOptions` (#213) diff --git a/packages/mds/__test__/browser.spec.mjs b/packages/mds/__test__/browser.spec.mjs index c64ccb77..4eabe682 100644 --- a/packages/mds/__test__/browser.spec.mjs +++ b/packages/mds/__test__/browser.spec.mjs @@ -176,9 +176,24 @@ describe('browser entry — post-init', () => { assert.equal(result.version, 1); assert.ok(Array.isArray(result.files)); assert.equal(result.truncated, false); - // files contains reports keyed by entry name or dep name. + // The entry key must appear in the report, and it must carry a real finding + // (the unused frontmatter variable). `files.length >= 0` would be a tautology — + // it holds for an empty array and so cannot distinguish a working lintVirtual + // from one that silently returns no findings. const fileNames = result.files.map((f) => f.file); - assert.ok(fileNames.length >= 0, 'lintVirtual must return a valid result'); + assert.ok( + fileNames.includes('main.mds'), + `expected entry 'main.mds' in report; got: [${fileNames.join(', ')}]`, + ); + const entryReport = result.files.find((f) => f.file === 'main.mds'); + assert.ok( + entryReport.diagnostics.length > 0, + 'expected at least one diagnostic for the unused frontmatter variable', + ); + assert.ok( + entryReport.diagnostics.some((d) => d.rule === 'unused-variable'), + `expected an unused-variable diagnostic; got rules: [${entryReport.diagnostics.map((d) => d.rule).join(', ')}]`, + ); }); test('U-BR19: browser lint/lintVirtual reject unknown option keys (AC-P3-14)', () => { diff --git a/packages/mds/__test__/options-validation.spec.mjs b/packages/mds/__test__/options-validation.spec.mjs index 767d050b..8974af7f 100644 --- a/packages/mds/__test__/options-validation.spec.mjs +++ b/packages/mds/__test__/options-validation.spec.mjs @@ -15,6 +15,7 @@ import { test, describe, before } from 'node:test'; import assert from 'node:assert/strict'; import { createRequire } from 'node:module'; +import { fileURLToPath } from 'node:url'; import { compile, check, @@ -369,7 +370,14 @@ describe('options-validation', () => { return require('@mdscript/mds-napi'); } - const FIXTURES = path.join(new URL('.', import.meta.url).pathname, 'fixtures'); + // fileURLToPath, NOT URL.pathname: on Windows `new URL('.', import.meta.url).pathname` + // yields "/C:/..." (leading slash, drive letter) and percent-encodes spaces, so + // path.join produces a path that cannot be resolved. windows-latest is in the JS CI + // matrix (ci.yml `js` job), and every other spec in this package already uses + // fileURLToPath — matching that pattern here (avoids the PF-003 Windows-path class). + const TEST_DIR = fileURLToPath(new URL('.', import.meta.url)); + const PKG_DIR = fileURLToPath(new URL('..', import.meta.url)); + const FIXTURES = path.join(TEST_DIR, 'fixtures'); const IMPORT_SRC = '@import { greet } from "./import_provider.mds"\n\n{{greet("World")}}\n'; test('U-OV-21: native compile honors basePath for import resolution (AC-P3-01)', () => { @@ -515,8 +523,8 @@ describe('options-validation', () => { // ── cross-backend message equality for file basePath (AC-P3-06 / U-OV-27) ─ - test('U-OV-27: compileFile/checkFile basePath rejection message is byte-identical on native and WASM (AC-P3-06, avoids PF-007)', async () => { - requireNativeAddon(); // hard-fail without addon + test('U-OV-27: compileFile/checkFile basePath rejection message is byte-identical to napi and backend-independent (AC-P3-06, avoids PF-007)', async () => { + const addon = requireNativeAddon(); // hard-fail without addon const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-bp-')); const file = path.join(tmp, 'ok.mds'); @@ -529,13 +537,26 @@ describe('options-validation', () => { try { for (const method of ['compileFile', 'checkFile']) { - // Native path: comes through the napi addon. - const nativeMsg = await captureMsg( + // The wrapper's public file methods short-circuit basePath in node.ts + // (fileBasePathError) BEFORE backend dispatch — verified below by the + // backend-independence leg. That makes the wrapper message the operative + // one on every backend, so the authoritative parity assertion is + // wrapper-vs-napi, NOT wrapper-vs-wrapper. + const wrapperMsg = await captureMsg( () => (method === 'compileFile' ? compileFile : checkFile)(file, { basePath: '.' }), ); - // WASM path: set MDS_BACKEND=wasm via env then run in subprocess — or - // drive the WASM path directly via wrapWithFileOps. We use a subprocess - // for full isolation (no shared module singleton). + + // LEG 1 (the real drift guard): compare against the RAW napi addon, which + // owns the canonical message (parse_file_opts / parse_check_file_opts in + // crates/mds-napi/src/lib.rs). Nothing else in the suite pins these two + // together; without this leg, editing either string silently diverges the + // surfaces because the wrapper never calls napi for this input. + const napiMsg = await captureMsg( + () => (method === 'compileFile' ? addon.compileFile : addon.checkFile)(file, { basePath: '.' }), + ); + + // LEG 2: the same public call under MDS_BACKEND=wasm must produce the same + // message. Subprocess for full isolation (no shared module singleton). const { execFileSync } = await import('node:child_process'); const wasmMsg = await captureMsg(() => { const script = ` @@ -547,19 +568,28 @@ ${method}(${JSON.stringify(file)}, { basePath: '.' }).catch(e => { `; const out = execFileSync(process.execPath, ['--input-type=module'], { input: script, - cwd: new URL('..', import.meta.url).pathname, + cwd: PKG_DIR, env: { ...process.env, MDS_BACKEND: 'wasm' }, timeout: 15000, }); throw new Error(out.toString().trim() || 'WASM subprocess produced no output'); }); - assert.ok(nativeMsg.length > 0, `native ${method} must throw for basePath`); - assert.ok(wasmMsg.length > 0, `WASM ${method} must throw for basePath`); + // Positive controls: every leg must have actually produced an error. A + // silently-empty message would make the equality assertions vacuous. + assert.ok(wrapperMsg.length > 0, `wrapper ${method} must throw for basePath`); + assert.ok(napiMsg.length > 0, `napi ${method} must throw for basePath`); + assert.ok(wasmMsg.length > 0, `WASM-backend ${method} must throw for basePath`); + + assert.strictEqual( + wrapperMsg, + napiMsg, + `wrapper must match napi byte-for-byte for ${method} — wrapper: "${wrapperMsg}" | napi: "${napiMsg}"`, + ); assert.strictEqual( - nativeMsg, + wrapperMsg, wasmMsg, - `byte-identical messages required for ${method} — native: "${nativeMsg}" | wasm: "${wasmMsg}"`, + `message must be backend-independent for ${method} — native-default: "${wrapperMsg}" | MDS_BACKEND=wasm: "${wasmMsg}"`, ); } } finally { @@ -569,41 +599,63 @@ ${method}(${JSON.stringify(file)}, { basePath: '.' }).catch(e => { // ── {basePath: undefined} cross-backend parity (AC-P3-08 / U-OV-29) ──────── - test('U-OV-29: {basePath: undefined} has consistent throw/no-throw on both backends for compile/check (AC-P3-08)', async () => { + test('U-OV-29: {basePath: undefined} has consistent throw/no-throw on both backends for compile/check/compileFile/checkFile (AC-P3-08)', async () => { requireNativeAddon(); // hard-fail without addon // Semantic: basePath: undefined is treated as absent ("value is intent") on the // wrapper side. The WASM guard checks != null so undefined passes through. - // On native, napi sees the property but its value is undefined/null and does not - // trigger basePath handling. Both backends must agree. + // On native, the per-surface builders drop the undefined value entirely, so napi's + // has_named_property("basePath") gate never sees the key. Both backends must agree. + // + // AC-P3-08 requires all FOUR methods: the file surfaces are the interesting half, + // because napi keys off property PRESENCE — if a builder ever forwarded + // `basePath: undefined` verbatim, native would throw while WASM would not. + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-bp-undef-')); + const file = path.join(tmp, 'ok.mds'); + fs.writeFileSync(file, 'Hello\n', 'utf8'); + const methods = [ - { name: 'compile', fn: (opts) => compile('Hello\n', opts) }, - { name: 'check', fn: (opts) => check('Hello\n', opts) }, + { name: 'compile', call: 'compile(SRC, OPTS)', fn: (opts) => compile('Hello\n', opts) }, + { name: 'check', call: 'check(SRC, OPTS)', fn: (opts) => check('Hello\n', opts) }, + { name: 'compileFile', call: 'await compileFile(F, OPTS)', fn: (opts) => compileFile(file, opts) }, + { name: 'checkFile', call: 'await checkFile(F, OPTS)', fn: (opts) => checkFile(file, opts) }, ]; - for (const { name, fn } of methods) { - let nativeThrew = false; - try { fn({ basePath: undefined }); } catch { nativeThrew = true; } + try { + for (const { name, call, fn } of methods) { + let nativeThrew = false; + try { await fn({ basePath: undefined }); } catch { nativeThrew = true; } - // WASM path via subprocess. init() must be awaited before compile/check. - const { execFileSync } = await import('node:child_process'); - let wasmThrew = false; - try { - execFileSync(process.execPath, ['--input-type=module'], { - input: `import { init, ${name} } from './dist/node.js'; await init(); try { ${name}('Hello\\n', { basePath: undefined }); process.exit(0); } catch { process.exit(1); }`, - cwd: new URL('..', import.meta.url).pathname, - env: { ...process.env, MDS_BACKEND: 'wasm' }, - timeout: 15000, - }); - } catch { - wasmThrew = true; - } + // WASM path via subprocess. init() must be awaited before any method. + const { execFileSync } = await import('node:child_process'); + let wasmThrew = false; + try { + const script = [ + `import { init, ${name} } from './dist/node.js';`, + `const SRC = 'Hello\\n';`, + `const F = ${JSON.stringify(file)};`, + `const OPTS = { basePath: undefined };`, + `await init();`, + `try { ${call}; process.exit(0); } catch { process.exit(1); }`, + ].join('\n'); + execFileSync(process.execPath, ['--input-type=module'], { + input: script, + cwd: PKG_DIR, + env: { ...process.env, MDS_BACKEND: 'wasm' }, + timeout: 15000, + }); + } catch { + wasmThrew = true; + } - assert.strictEqual( - nativeThrew, - wasmThrew, - `${name}: native (threw=${nativeThrew}) and WASM (threw=${wasmThrew}) must agree on {basePath: undefined}`, - ); + assert.strictEqual( + nativeThrew, + wasmThrew, + `${name}: native (threw=${nativeThrew}) and WASM (threw=${wasmThrew}) must agree on {basePath: undefined}`, + ); + } + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); } }); diff --git a/packages/mds/__test__/types/consumer-browser.ts b/packages/mds/__test__/types/consumer-browser.ts index e12d1624..f8d45504 100644 --- a/packages/mds/__test__/types/consumer-browser.ts +++ b/packages/mds/__test__/types/consumer-browser.ts @@ -21,6 +21,7 @@ import type { LintRuleName, LintSpan, RuleSeverity, + SourceMapV3, } from '../../dist/browser.js'; // ── Positive: basePath accepted on string-surface types ─────────────────────── @@ -40,5 +41,10 @@ const _result: LintResult = { version: 1, files: [_report], truncated: false }; const _severity: RuleSeverity = 'error'; const _ruleName: LintRuleName = 'empty-block'; +// SourceMapV3 must also be nameable from the browser entry — compile({sourceMap:true}) +// is supported there, so consumers need the result type. +const _sourceMap: SourceMapV3 = { version: 3, sources: ['input.mds'], names: [], mappings: '' }; + void _compileOpts; void _checkOpts; void _lintOpts; void _lintFileOpts; void _diagArr; void _span; void _report; void _result; void _severity; void _ruleName; +void _sourceMap; diff --git a/packages/mds/__test__/types/consumer-node.ts b/packages/mds/__test__/types/consumer-node.ts index fba785ba..37dab1dc 100644 --- a/packages/mds/__test__/types/consumer-node.ts +++ b/packages/mds/__test__/types/consumer-node.ts @@ -26,7 +26,9 @@ import type { LintResult, LintRuleName, LintSpan, + MarkdownResult, RuleSeverity, + SourceMapV3, } from '../../dist/node.js'; // ── Positive cases: basePath accepted on string-surface types ───────────────── @@ -56,12 +58,30 @@ const _checkFileOpts: CheckFileOptions = { basePath: '/some/dir' }; // @ts-expect-error — basePath is intentionally absent from LintFileOptions const _lintFileOpts: LintFileOptions = { basePath: '/some/dir' }; -// ── PR2 guard: invalid rule name must be rejected in LintOptions ────────────── -// D-224-1 introduced LintRuleName; ensure the type fixture protects both PRs. +// ── PR2 guard: rule-name and severity typing on LintOptions ────────────────── +// D-224-1 ruling: an unrecognised RULE NAME is deliberately NOT a type error — +// `rules` is `Record` so configs naming a rule added in a +// newer binary still compile; the engine warns at runtime via +// LintResult.lint_warnings. Both cases below must therefore be ACCEPTED. const _validRule: LintOptions = { rules: { 'unused-variable': 'warn' } }; -// Record is accepted for forward compatibility. const _fwdCompat: LintOptions = { rules: { 'a-future-rule': 'off' } }; +// The SEVERITY value, by contrast, IS a closed set — an invalid severity must be a +// type error. This is the negative control proving `rules` is not typed as +// `Record`: if RuleSeverity were widened, tsc reports +// "Unused @ts-expect-error directive" and the build fails. +// @ts-expect-error — 'sometimes' is not a RuleSeverity ('error' | 'warn' | 'info' | 'off') +const _badSeverity: LintOptions = { rules: { 'unused-variable': 'sometimes' } }; + +// ── SourceMapV3 must be nameable from the entry the exports map resolves ────── +// MarkdownResult.sourceMap is typed as SourceMapV3; a consumer that cannot name +// the type cannot annotate the value. dist/index.d.ts does not count — index.ts +// is unreachable through the package `exports` map (see the note in src/index.ts). +const _sourceMap: SourceMapV3 = { version: 3, sources: ['input.mds'], names: [], mappings: '' }; +const _markdown: MarkdownResult = { + kind: 'markdown', output: '', warnings: [], dependencies: [], sourceMap: _sourceMap, +}; + // ── AC-P3-16: all lint types are nameable from the browser surface ───────────── // (browser types are verified in consumer-browser.ts; here we just confirm they // compile correctly when imported from the node entry.) @@ -75,5 +95,6 @@ const _ruleName: LintRuleName = 'unused-variable'; // Prevent unused-variable TS errors for the above declarations. void _compileOpts; void _checkOpts; void _lintOpts; void _fileOpts; void _checkFileOpts; void _lintFileOpts; -void _validRule; void _fwdCompat; +void _validRule; void _fwdCompat; void _badSeverity; +void _sourceMap; void _markdown; void _diagArr; void _span; void _report; void _result; void _severity; void _ruleName; diff --git a/packages/mds/src/backend/native.ts b/packages/mds/src/backend/native.ts index 5c87f162..d9bf58d0 100644 --- a/packages/mds/src/backend/native.ts +++ b/packages/mds/src/backend/native.ts @@ -44,7 +44,9 @@ type NapiLintFileOpts = { vars?: Record; rules?: Record> = { * interception (issue #74). The backend emits a purpose-built actionable error * for these methods ("not valid for compileFile/checkFile; the base directory is * derived from the file path") rather than the generic "unknown option key" format. + * + * KNOWN RESIDUAL (OD-5): napi also has purpose-built `basePath` messages for + * `lintFile` ("not valid for lintFile; …") and `lintVirtual`, but those two methods + * are deliberately NOT in this set. The wrapper intercepts them first and emits the + * generic `unknown option key "basePath"; recognised keys are: vars, rules` form, so + * the wrapper and napi messages diverge for that one input. This is intentional and + * locked in by U-OV-7 and U-OV-13; the generic message still names the offending key + * and is a hard error either way. Widening this set would change those two messages + * and is deferred rather than bundled into #180/#215/#213. */ const BASEPATH_PASSTHROUGH: ReadonlySet = new Set([ 'compileFile', From 5cb473437e8cd272381cd05342f73c29c41b4327 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 20:21:54 +0300 Subject: [PATCH 04/26] test(mds): add cross-surface browser vs node lint parity test (AC-P3-12) Amendment 4 of the Cross-PR Conflict Audit required comparing the browser `lint` result against the Node `lint` result at RUNTIME for the same fixture instead of asserting browser-local golden values. PF-007: per-surface goldens each lock in their OWN value and cannot prove cross-surface parity. U-BR-PARITY runs both surfaces on the same fixture with deepStrictEqual, closing the fifth-surface parity gap. Also imports `nodeInit` so the node backend is ready before the post-init describe block executes. PR body simultaneously updated (via gh pr edit) to: - Remove stale `throwFileBasePathError` references (removed in 0330597) - Add Packaging note documenting index.ts unreachability (AC-P3-21) --- packages/mds/__test__/browser.spec.mjs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/mds/__test__/browser.spec.mjs b/packages/mds/__test__/browser.spec.mjs index 4eabe682..633ae408 100644 --- a/packages/mds/__test__/browser.spec.mjs +++ b/packages/mds/__test__/browser.spec.mjs @@ -25,6 +25,7 @@ import { _initWithModuleForTesting, } from '../dist/browser.js'; import { initWasmNode, _resetForTesting as wasmReset } from '../dist/backend/wasm.js'; +import { lint as nodeLint, init as nodeInit } from '../dist/node.js'; // Mirror of MAX_INIT_RETRIES from src/backend/wasm.ts. // If this value drifts, U-BR11 will surface the mismatch via a test failure. @@ -32,9 +33,11 @@ const MAX_INIT_RETRIES = 3; // Load the WASM module once at file scope using the Node.js loader. // All browser tests that need a live backend inject it via _initWithModuleForTesting(). +// nodeInit() is also called here to satisfy the cross-surface parity test (U-BR-PARITY). let sharedWasmModule; before(async () => { sharedWasmModule = await initWasmNode(); + await nodeInit(); }); // --------------------------------------------------------------------------- @@ -196,6 +199,21 @@ describe('browser entry — post-init', () => { ); }); + test('U-BR-PARITY: browser lint result equals node lint result at runtime (AC-P3-12, amendment 4, avoids PF-007)', () => { + // PF-007: per-surface goldens each lock in their OWN value and cannot prove + // cross-surface parity. Compare browser (WASM) and node surfaces at RUNTIME for + // the same input with deepStrictEqual — no pinned golden, no local assertion. + // Same fixture as U-BR16 so the result is non-trivial (unused-variable diagnostic). + const src = '---\ngreeting: Hello\nunused_key: this key is never referenced\n---\n\n{{greeting}}, world!\n'; + const browserResult = lint(src); + const nodeResult = nodeLint(src); + assert.deepStrictEqual( + browserResult, + nodeResult, + 'browser (WASM) and node lint must return byte-identical results for the same source', + ); + }); + test('U-BR19: browser lint/lintVirtual reject unknown option keys (AC-P3-14)', () => { // Proves the browser path runs assertKnownKeys. assert.throws( From d1f4a3c7380dbc5fa5bd659d422cc1b74e9eba23 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:00:25 +0300 Subject: [PATCH 05/26] refactor(mds): drop redundant type assertion in compileOpts `_WasmCompileInput` (Omit & { filename?, modules? }) is already structurally assignable to FileOptions | undefined because it carries the same vars/sourceMap/sourcesContent fields with identical types and the extra optional properties are permitted on non-fresh values. The `options as Parameters[0]` cast was therefore redundant and obscured the signature of fileCompileOpt behind an indirection that would have hidden any future incompatibility. Co-Authored-By: Claude --- packages/mds/src/backend/wasm.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mds/src/backend/wasm.ts b/packages/mds/src/backend/wasm.ts index a9b844d9..234976f8 100644 --- a/packages/mds/src/backend/wasm.ts +++ b/packages/mds/src/backend/wasm.ts @@ -361,7 +361,7 @@ function compileOpts( } { // D-TS-06: use fileCompileOpt (no basePath) — basePath is already excluded // from _WasmCompileInput, and the caller guards against it before reaching here. - const extra = fileCompileOpt(options as Parameters[0]); + const extra = fileCompileOpt(options); const filename = options?.filename ?? DEFAULT_COMPILE_OPTS.filename; const modules = options?.modules ?? DEFAULT_COMPILE_OPTS.modules; if (extra == null && filename === DEFAULT_COMPILE_OPTS.filename && modules === DEFAULT_COMPILE_OPTS.modules) { From 52f085d20b4cac6e5ba36a002971e84e08100515 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:00:32 +0300 Subject: [PATCH 06/26] refactor(mds): add NapiFileCheckOpts named type to NapiAddon interface All seven NapiAddon members now use a named option type: NapiCompileOpts, NapiCheckOpts, NapiFileCompileOpts, NapiFileCheckOpts, NapiLintOpts, NapiLintFileOpts (shared by lintFile/lintVirtual). Previously checkFile used an inline { vars?: Record } which was inconsistent with the other six members and obscured the file-surface / string-surface split. The new NapiFileCheckOpts type mirrors CheckFileOptions (the public equivalent) and makes the split visible in one place. Co-Authored-By: Claude --- packages/mds/src/backend/native.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/mds/src/backend/native.ts b/packages/mds/src/backend/native.ts index d9bf58d0..7512da65 100644 --- a/packages/mds/src/backend/native.ts +++ b/packages/mds/src/backend/native.ts @@ -35,6 +35,11 @@ type NapiFileCompileOpts = { sourcesContent?: boolean; }; +/** Options forwarded to the napi addon for checkFile (no basePath, no sourceMap). */ +type NapiFileCheckOpts = { + vars?: Record; +}; + /** Options forwarded to the napi addon for source-string lint (accepts basePath). */ type NapiLintOpts = { basePath?: string; vars?: Record; rules?: Record }; /** Options forwarded to the napi addon for file-based and virtual lint. */ @@ -43,7 +48,7 @@ type NapiLintFileOpts = { vars?: Record; rules?: Record }): unknown; + checkFile(path: string, opts?: NapiFileCheckOpts): unknown; lint(source: string, opts?: NapiLintOpts): unknown; lintFile(path: string, opts?: NapiLintFileOpts): unknown; lintVirtual(modules: Record, entry: string, opts?: NapiLintFileOpts): unknown; From 97957230af85197691faed4a0d185715cb61bfc8 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:02:44 +0300 Subject: [PATCH 07/26] docs(changelog): correct BREAKING section for basePath rejection on file methods - Move compileFile/checkFile basePath rejection from a **Note:** under Fixed into its own BREAKING subsection; the silent-success -> Promise.reject transition is a runtime breaking change on a published API (AC-P3-24). - Restate the FileOptions/CompileOptions type reshape as source-compatible: before this PR CompileOptions had no basePath, so the old extends resolved to the same shape as the new standalone interface. The false migration step ('now needs an explicit mapping') is removed. - Restate the checkFile parameter narrowing as source-compatible: CheckOptions was structurally equivalent to CheckFileOptions before this PR. The real break is the runtime rejection documented in the new subsection, which tsc does not flag for variable-typed callers. - Update the BREAKING section header to lead with the file-method rejection. Co-Authored-By: Claude --- CHANGELOG.md | 57 +++++++++++++++++--------- packages/mds/__test__/browser.spec.mjs | 44 +++++++++++++++++++- 2 files changed, 81 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df9c8f78..f388bb39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,10 +26,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 backends (`!= null` check; value-is-intent). To use `basePath` with import resolution, set `MDS_BACKEND=native` or use `lintVirtual` with a pre-resolved module map. - **Note:** `compileFile` and `checkFile` reject a non-null `basePath` at the JS layer - because the base directory for file-based operations is derived from the file path - itself; passing `basePath` there is always a caller error. - ### Added - **`lint` and `lintVirtual` are now exported from the browser entry point (#215).** @@ -48,29 +44,52 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 only re-exported from `index.ts`, which the package `exports` map does not resolve — so consumers could receive the value but not name its type. Purely additive. -### **BREAKING** — TypeScript option types and WASM `basePath` rejection (#213, #180) +### **BREAKING** — File-method `basePath` rejection, TypeScript option types, and WASM `basePath` rejection (#180, #213) + +#### `compileFile` and `checkFile` now reject `basePath` (#180) + +`compileFile(path, options?)` and `checkFile(path, options?)` previously accepted a +`basePath` option and silently discarded it — the option passed the unknown-key +validator but was dropped before the backend was reached, so file resolution always +used the directory containing the path. Both functions now return a rejected promise +(`Error { code: 'mds::invalid_options' }`) when `basePath` is non-null. The base +directory for file-based operations is always derived from the file path itself. + +**Migration:** remove `basePath` from any options object passed to `compileFile` or +`checkFile`. This is a **runtime-only** break — TypeScript does not flag it at +compile time when you pass a variable whose inferred type contains `basePath`. +Audit call sites explicitly. #### `FileOptions` no longer extends `CompileOptions` (#213) `FileOptions` (used by `compileFile`) was previously declared as `interface FileOptions extends CompileOptions`. This inheritance was an error: -`CompileOptions` now carries `basePath`, which is explicitly not valid for file-based -operations (the base directory is derived from the file path). `FileOptions` is now -a standalone interface with its own `vars`, `sourceMap`, and `sourcesContent` fields. - -**Migration:** code that assigned a `CompileOptions` to a `FileOptions` variable (or -vice versa) now needs an explicit mapping. Code that used `FileOptions` purely for -`vars`, `sourceMap`, and `sourcesContent` is unaffected. +`CompileOptions` now carries `basePath`, which is not valid for file-based +operations. `FileOptions` is now a standalone interface with its own `vars`, +`sourceMap`, and `sourcesContent` fields. + +**Compatibility:** this change is **source-compatible** for all previously-compiling +code. Before this PR, `CompileOptions` had no `basePath` field, so +`FileOptions extends CompileOptions` already resolved to +`{ vars?, sourceMap?, sourcesContent? }` — the same shape as the new standalone +`FileOptions`. Code that assigns a `CompileOptions` value to a `FileOptions` variable +(or vice versa) continues to compile. The runtime break is the `compileFile`/`checkFile` +rejection documented above, which TypeScript does not catch — see that entry for +the migration. #### `checkFile` parameter type changed from `CheckOptions` to `CheckFileOptions` (#213) -`checkFile(path, options?)` previously accepted `CheckOptions`, which includes a -`basePath` field that is invalid for file-based operations. The parameter is now typed -as `CheckFileOptions` — a new interface with only `vars?: Record`. - -**Migration:** if you passed `CheckOptions` to `checkFile`, remove the `basePath` field. -If you used a shared variable typed as `CheckOptions`, destructure or pick `vars` before -passing it. +`checkFile(path, options?)` previously accepted `CheckOptions`. After this PR, +`CheckOptions` carries a `basePath` field that is not valid for file-based operations; +the parameter is now typed as `CheckFileOptions` — a new interface with only +`vars?: Record`. + +**Compatibility:** this type narrowing is **source-compatible** for all +previously-compiling code. Before this PR, `CheckOptions` had no `basePath` field, so +it was structurally equivalent to the new `CheckFileOptions`. A `CheckOptions`-typed +variable without `basePath` still satisfies the `CheckFileOptions` parameter. The +runtime break is the `compileFile`/`checkFile` rejection documented above, which +TypeScript does not catch — see that entry for the migration. #### WASM backend rejects `basePath` on string-surface methods (#180) diff --git a/packages/mds/__test__/browser.spec.mjs b/packages/mds/__test__/browser.spec.mjs index 633ae408..eef6b943 100644 --- a/packages/mds/__test__/browser.spec.mjs +++ b/packages/mds/__test__/browser.spec.mjs @@ -25,7 +25,7 @@ import { _initWithModuleForTesting, } from '../dist/browser.js'; import { initWasmNode, _resetForTesting as wasmReset } from '../dist/backend/wasm.js'; -import { lint as nodeLint, init as nodeInit } from '../dist/node.js'; +import { lint as nodeLint, init as nodeInit, getBackend as nodeGetBackend } from '../dist/node.js'; // Mirror of MAX_INIT_RETRIES from src/backend/wasm.ts. // If this value drifts, U-BR11 will surface the mismatch via a test failure. @@ -115,6 +115,14 @@ describe('browser entry — pre-init', () => { test('U-BR15: lintFile is NOT exported from browser entry (AC-P3-13)', async () => { const moduleExports = Object.keys(await import('../dist/browser.js')); + // ADR-009 / avoids PF-013: positive control — if the module resolved to an empty + // export map, all three absence assertions below would pass vacuously. Asserting + // that 'lint' (added by #215) IS present proves the module was loaded correctly + // and that the absence assertions are meaningful. + assert.ok( + moduleExports.includes('lint'), + `lint must be exported from browser entry (positive control); found: ${moduleExports.join(', ')}`, + ); assert.equal( moduleExports.includes('lintFile'), false, @@ -204,6 +212,19 @@ describe('browser entry — post-init', () => { // cross-surface parity. Compare browser (WASM) and node surfaces at RUNTIME for // the same input with deepStrictEqual — no pinned golden, no local assertion. // Same fixture as U-BR16 so the result is non-trivial (unused-variable diagnostic). + // + // Guard (avoids PF-007 / PF-013): node.ts:244 falls back to WASM when the native + // addon is unavailable. Without this assertion, the test would silently degrade to + // a WASM-vs-WASM self-comparison — a vacuous pass that proves nothing about + // cross-backend parity. Every sibling native-dependent test hard-fails via + // requireNativeAddon(); this assertion is the equivalent sentinel for U-BR-PARITY. + assert.equal( + nodeGetBackend(), + 'native', + 'U-BR-PARITY requires the native backend on the node side; ' + + 'without it the test compares WASM against itself (PF-007 shape). ' + + 'Ensure @mdscript/mds-napi is built before running this suite.', + ); const src = '---\ngreeting: Hello\nunused_key: this key is never referenced\n---\n\n{{greeting}}, world!\n'; const browserResult = lint(src); const nodeResult = nodeLint(src); @@ -214,6 +235,27 @@ describe('browser entry — post-init', () => { ); }); + test('U-BR-WARN: browser lintVirtual with unknown rule name produces lint_warnings, not an error (plan amendment 5, avoids PF-007)', () => { + // Plan amendment 5: D-224-1 ruled that an unrecognised rule name is WARNED via + // LintResult.lint_warnings, not rejected. U-LG4 (lint.spec.mjs:625) already + // covers this behavior on the node/native surface. PF-007: that proof says + // nothing about the WASM (browser) surface — a separate browser-entry assertion + // is required to close the coverage gap. + const mods = { 'main.mds': '---\ngreeting: Hello\n---\n{{greeting}}, world!\n' }; + const result = lintVirtual(mods, 'main.mds', { rules: { 'no-such-rule-xyzzy': 'warn' } }); + assert.equal(result.version, 1, 'U-BR-WARN: version must be 1'); + assert.ok(Array.isArray(result.lint_warnings), `U-BR-WARN: lint_warnings must be an array; got ${JSON.stringify(result.lint_warnings)}`); + assert.ok( + result.lint_warnings.length > 0, + `U-BR-WARN: lint_warnings must be non-empty for unknown rule name; got ${JSON.stringify(result.lint_warnings)}`, + ); + const joined = result.lint_warnings.join(' '); + assert.ok( + joined.includes('no-such-rule-xyzzy'), + `U-BR-WARN: lint_warnings must name the unknown rule 'no-such-rule-xyzzy'; got: ${joined}`, + ); + }); + test('U-BR19: browser lint/lintVirtual reject unknown option keys (AC-P3-14)', () => { // Proves the browser path runs assertKnownKeys. assert.throws( From 7ef83699d9021c03f678e85c65c67ccc4015679d Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:06:21 +0300 Subject: [PATCH 08/26] docs(mds): fix stale comments referencing deleted helper functions compile.spec.mjs U-C7 comment cited 'varsOpt' which was removed and replaced by compileSrcOpt/fileCompileOpt (via pickDefined). Update the comment to name the actual functions responsible for the != null filter. source-map.spec.mjs U-SM6 block comment cited the adapter's compileOpt() as the key-filtering mechanism. That function was replaced by assertKnownKeys(), which throws on unknown keys. Update the comment to match. Co-Authored-By: Claude --- packages/mds/__test__/compile.spec.mjs | 6 ++++-- packages/mds/__test__/source-map.spec.mjs | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/mds/__test__/compile.spec.mjs b/packages/mds/__test__/compile.spec.mjs index 0cd45c90..632e8bcb 100644 --- a/packages/mds/__test__/compile.spec.mjs +++ b/packages/mds/__test__/compile.spec.mjs @@ -56,8 +56,10 @@ describe('compile', () => { }); test('U-C7: compile with null vars produces identical output to no-vars compile', () => { - // null vars must be treated as absent — varsOpt uses != null so both null - // and undefined are omitted from the options passed to the backend. + // null vars must be treated as absent — compileSrcOpt (native path) and + // fileCompileOpt via compileOpts (WASM path) both use pickDefined, which + // filters via != null, so both null and undefined are omitted from the + // options forwarded to the backend. const source = 'Hello World!\n'; const withNull = compile(source, { vars: null }); const withoutVars = compile(source); diff --git a/packages/mds/__test__/source-map.spec.mjs b/packages/mds/__test__/source-map.spec.mjs index 069975cb..cf1e1ff8 100644 --- a/packages/mds/__test__/source-map.spec.mjs +++ b/packages/mds/__test__/source-map.spec.mjs @@ -265,8 +265,8 @@ describe('source maps (U-SM)', () => { // ── U-SM6: valid option combinations ────────────────────────────────── // // Unknown key rejection at the binding level is tested in the napi spec - // (F-SM6). At the universal package level the adapter's compileOpt() - // filters to known keys, so TypeScript type checking is the guard. + // (F-SM6). At the universal package level assertKnownKeys() rejects + // unknown option keys, with TypeScript type checking as the primary guard. test('U-SM6: sourceMap:true, sourcesContent:false is accepted', () => { const result = compile('Hello!\n', { sourceMap: true, sourcesContent: false }); From ef6e7b0fba721948fa488aa66e0fe63799883bb0 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:09:14 +0300 Subject: [PATCH 09/26] docs(mds): fix README browser-section and option-rejection accuracy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Browser usage section (#215): - Line 38: "compile/check" → "compile/check/lint" (matches browser.ts:98) - Browser import example: add lint and lintVirtual; show minimal usage - Node-only callout: add lintFile to the list of unavailable methods Unknown-option rejection paragraph (AC-P3-25/PF-015): - Drop hard-coded "any of the seven public methods" count that drifts - Qualify the format guarantee: basePath on compileFile/checkFile surfaces as a rejected promise with a purpose-built message, not a synchronous throw with an accepted-keys list (avoids PF-015) Also removes stale (OD-1 — ) design-ID annotation from LintOptions inline comment (stale-comment cleanup, same spirit as 7ef8369). Co-Authored-By: Claude --- packages/mds/README.md | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/mds/README.md b/packages/mds/README.md index a1843c8a..d59dad92 100644 --- a/packages/mds/README.md +++ b/packages/mds/README.md @@ -35,20 +35,22 @@ console.log(getBackend()); // 'native' | 'wasm' ## Browser usage -The browser entry requires an explicit `init()` call before any compile/check +The browser entry requires an explicit `init()` call before any compile/check/lint operations. `init()` is idempotent and safe to call multiple times. ```ts -import { init, compile, check, isMdsError } from '@mdscript/mds'; +import { init, compile, check, lint, lintVirtual, isMdsError } from '@mdscript/mds'; await init(); // or with a custom WASM URL: await init({ wasmUrl: '/assets/mds_bg.wasm' }); const result = compile('# {{title}}', { vars: { title: 'Hello' } }); +const lintResult = lint('# {{title}}', { vars: { title: 'Hello' } }); +const virtualResult = lintVirtual({ 'entry.mds': '# {{title}}' }, 'entry.mds'); ``` -> `compileFile` and `checkFile` are not available in browser environments. +> `compileFile`, `checkFile`, and `lintFile` are not available in browser environments. ## Backend selection (`MDS_BACKEND`) @@ -152,8 +154,8 @@ interface CheckFileOptions { // LintOptions — accepted by lint() (string-source) // basePath: required when the source contains @import or @extends. -// WASM backend: basePath throws mds::invalid_options (OD-1 — rejects instead of -// silently ignoring so misconfigured callers see an actionable error). +// WASM backend: basePath throws mds::invalid_options — rejects instead of +// silently ignoring so misconfigured callers see an actionable error. // Set MDS_BACKEND=native to use the native backend, or use lintVirtual with // pre-resolved modules. interface LintOptions { @@ -176,9 +178,12 @@ interface InitOptions { } ``` -**Unknown-option rejection:** passing an unrecognised key to any of the seven -public methods throws `Error { code: 'mds::invalid_options' }` immediately, before -calling the backend. The error names the offending key(s) and lists the accepted keys. +**Unknown-option rejection:** passing an unrecognised key to a public method throws +`Error { code: 'mds::invalid_options' }` before calling the backend; the error names +the offending key(s) and lists the accepted keys. Exception: passing `basePath` to a +file-path method (`compileFile` or `checkFile`) surfaces as a rejected promise with a +purpose-built message rather than a synchronous throw, and the message does not include +an accepted-keys list. **Source maps:** for string-source compiles (`compile`) `sources[0]` in the generated map is `"input.mds"`. For stdin builds via the CLI it is `""`. From 50b37c90298f3661784b03e37717d2abd1ab0a91 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:12:31 +0300 Subject: [PATCH 10/26] docs(mds): fix consumer-browser.ts docblock for AC-P3-20 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AC-P3-20 comment block contained two contradictions: 1. "basePath is NOT accepted on CompileOptions or CheckOptions from the browser entry — it IS in those types (D-TS-01)" — the two halves assert opposite things; the code at lines 28-30 (no @ts-expect-error) correctly shows basePath IS accepted at the type level, which is what AC-P3-20 mandates. 2. "Same @ts-expect-error guards as consumer-node.ts for FileOptions etc." — false; this fixture has no guard for FileOptions or CheckFileOptions because neither is exported from the browser entry. The only file-surface negative case present is LintFileOptions. Replaced with a precise statement: - basePath IS accepted at the TYPE level (same d.ts on both entries, D-TS-01) - the WASM backend rejects a non-null basePath at RUNTIME - FileOptions/CheckFileOptions have no guard because they are not exported from the browser entry — the only file-surface negative case is LintFileOptions Also commit the pre-existing (uncommitted) variable-passing case that verifies LintOptions is not assignable to LintFileOptions via a typed variable (not only a fresh object literal) — correct and tsc-verified. Co-Authored-By: Claude --- .../mds/__test__/types/consumer-browser.ts | 39 ++++++++++--- packages/mds/__test__/types/consumer-node.ts | 34 ++++++++++- packages/mds/src/browser.ts | 7 ++- packages/mds/src/index.ts | 35 ------------ packages/mds/src/node.ts | 57 +++++++------------ packages/mds/src/util/options.ts | 37 ++++++++---- 6 files changed, 115 insertions(+), 94 deletions(-) delete mode 100644 packages/mds/src/index.ts diff --git a/packages/mds/__test__/types/consumer-browser.ts b/packages/mds/__test__/types/consumer-browser.ts index f8d45504..85e3492b 100644 --- a/packages/mds/__test__/types/consumer-browser.ts +++ b/packages/mds/__test__/types/consumer-browser.ts @@ -5,10 +5,18 @@ * lint API surface are exported from dist/browser.d.ts and compile correctly * under the repo's strict settings. * - * AC-P3-20 (browser surface): basePath is NOT accepted on CompileOptions or - * CheckOptions from the browser entry — it IS in those types (D-TS-01), but - * since the WASM backend rejects it at runtime, the type is honest. Same - * `@ts-expect-error` guards as consumer-node.ts for FileOptions etc. + * AC-P3-20 (browser surface): `basePath` IS accepted at the type level on + * `CompileOptions`, `CheckOptions`, and `LintOptions` from the browser entry. + * These string-surface types are shared between the Node.js and browser entries + * (D-TS-01; recorded as ADR-011). The WASM backend rejects a non-null + * `basePath` at runtime with `mds::invalid_options`; the enforcement is + * runtime-only, not type-level. Callers who need import resolution must use + * the Node.js entry with MDS_BACKEND=native. + * + * `FileOptions` and `CheckFileOptions` carry no `@ts-expect-error` guard here + * because neither type is exported from the browser entry — there is nothing + * to verify absence on. The only file-surface negative case in this fixture + * is `LintFileOptions`, the one file-surface type the browser entry does export. */ import type { CheckOptions, @@ -20,11 +28,15 @@ import type { LintResult, LintRuleName, LintSpan, + MdsBaseBackend, RuleSeverity, SourceMapV3, } from '../../dist/browser.js'; -// ── Positive: basePath accepted on string-surface types ─────────────────────── +// ── Positive: basePath accepted on string-surface types (shared with Node) ──── +// D-TS-01 / ADR-011: CompileOptions, CheckOptions, and LintOptions carry +// `basePath` on both browser and Node entries. WASM enforces the constraint at +// runtime. These three assignments MUST compile without @ts-expect-error. const _compileOpts: CompileOptions = { basePath: '/dir', sourceMap: true }; const _checkOpts: CheckOptions = { basePath: '/dir' }; const _lintOpts: LintOptions = { basePath: '/dir', rules: {} }; @@ -33,6 +45,12 @@ const _lintOpts: LintOptions = { basePath: '/dir', rules: {} }; // @ts-expect-error — basePath is intentionally absent from LintFileOptions const _lintFileOpts: LintFileOptions = { basePath: '/dir' }; +// Variable-passing case: LintOptions (basePath?: string) must be rejected by +// LintFileOptions (basePath?: never) — not only fresh object literals. +declare const _lintSrcVar: LintOptions; +// @ts-expect-error — LintOptions (basePath?: string) not assignable to LintFileOptions (basePath?: never) +const _lintFileFromVar: LintFileOptions = _lintSrcVar; + // ── AC-P3-16: all seven lint types are nameable from the browser entry ──────── const _diagArr: LintDiagnostic[] = []; const _span: LintSpan = { offset: 0, length: 0 }; @@ -45,6 +63,13 @@ const _ruleName: LintRuleName = 'empty-block'; // is supported there, so consumers need the result type. const _sourceMap: SourceMapV3 = { version: 3, sources: ['input.mds'], names: [], mappings: '' }; -void _compileOpts; void _checkOpts; void _lintOpts; void _lintFileOpts; +// ── MdsBaseBackend must be nameable from the browser entry (AC-P3-21) ──────── +// The browser entry resolves to a MdsBaseBackend at runtime. Consumers that +// type a variable holding the resolved backend need this type from the entry +// the exports map resolves. Previously it was exported only from the unreachable +// src/index.ts barrel. +const _backendInterface: MdsBaseBackend = {} as MdsBaseBackend; + +void _compileOpts; void _checkOpts; void _lintOpts; void _lintFileOpts; void _lintFileFromVar; void _diagArr; void _span; void _report; void _result; void _severity; void _ruleName; -void _sourceMap; +void _sourceMap; void _backendInterface; diff --git a/packages/mds/__test__/types/consumer-node.ts b/packages/mds/__test__/types/consumer-node.ts index 37dab1dc..7a5b73fa 100644 --- a/packages/mds/__test__/types/consumer-node.ts +++ b/packages/mds/__test__/types/consumer-node.ts @@ -27,6 +27,9 @@ import type { LintRuleName, LintSpan, MarkdownResult, + MdsBackend, + MdsBaseBackend, + MdsNodeBackend, RuleSeverity, SourceMapV3, } from '../../dist/node.js'; @@ -58,6 +61,23 @@ const _checkFileOpts: CheckFileOptions = { basePath: '/some/dir' }; // @ts-expect-error — basePath is intentionally absent from LintFileOptions const _lintFileOpts: LintFileOptions = { basePath: '/some/dir' }; +// ── Variable-passing negative cases (AC-P3-20 stronger claim) ──────────────── +// Object-literal excess-property checks only fire on fresh literals. The cases +// below use typed variables — the realistic consumer shape — to prove that the +// structural type matrix also rejects passing a string-surface options object +// directly to a file-surface API. `basePath?: never` on the file-surface types +// causes TypeScript to report: "Type 'string | undefined' is not assignable to +// type 'undefined'" when a variable carrying basePath is used. +declare const _compileSrcVar: CompileOptions; +// @ts-expect-error — CompileOptions (basePath?: string) is not assignable to FileOptions (basePath?: never) +const _fileFromCompileVar: FileOptions = _compileSrcVar; +declare const _checkSrcVar: CheckOptions; +// @ts-expect-error — CheckOptions (basePath?: string) is not assignable to CheckFileOptions (basePath?: never) +const _checkFileFromVar: CheckFileOptions = _checkSrcVar; +declare const _lintSrcVar: LintOptions; +// @ts-expect-error — LintOptions (basePath?: string) is not assignable to LintFileOptions (basePath?: never) +const _lintFileFromVar: LintFileOptions = _lintSrcVar; + // ── PR2 guard: rule-name and severity typing on LintOptions ────────────────── // D-224-1 ruling: an unrecognised RULE NAME is deliberately NOT a type error — // `rules` is `Record` so configs naming a rule added in a @@ -75,13 +95,21 @@ const _badSeverity: LintOptions = { rules: { 'unused-variable': 'sometimes' } }; // ── SourceMapV3 must be nameable from the entry the exports map resolves ────── // MarkdownResult.sourceMap is typed as SourceMapV3; a consumer that cannot name -// the type cannot annotate the value. dist/index.d.ts does not count — index.ts -// is unreachable through the package `exports` map (see the note in src/index.ts). +// the type cannot annotate the value. const _sourceMap: SourceMapV3 = { version: 3, sources: ['input.mds'], names: [], mappings: '' }; const _markdown: MarkdownResult = { kind: 'markdown', output: '', warnings: [], dependencies: [], sourceMap: _sourceMap, }; +// ── Backend interfaces must be nameable from the Node entry (AC-P3-21) ──────── +// MdsBaseBackend, MdsNodeBackend, and MdsBackend are referenced in JSDoc +// {@link} tags throughout dist/node.d.ts. Consumers must be able to name them +// to type variables (e.g. a helper accepting any MdsBaseBackend). They were +// previously re-exported only from the unreachable barrel (src/index.ts). +const _base: MdsBaseBackend = {} as MdsBaseBackend; +const _node: MdsNodeBackend = {} as MdsNodeBackend; +const _compat: MdsBackend = {} as MdsBackend; + // ── AC-P3-16: all lint types are nameable from the browser surface ───────────── // (browser types are verified in consumer-browser.ts; here we just confirm they // compile correctly when imported from the node entry.) @@ -95,6 +123,8 @@ const _ruleName: LintRuleName = 'unused-variable'; // Prevent unused-variable TS errors for the above declarations. void _compileOpts; void _checkOpts; void _lintOpts; void _fileOpts; void _checkFileOpts; void _lintFileOpts; +void _fileFromCompileVar; void _checkFileFromVar; void _lintFileFromVar; void _validRule; void _fwdCompat; void _badSeverity; void _sourceMap; void _markdown; +void _base; void _node; void _compat; void _diagArr; void _span; void _report; void _result; void _severity; void _ruleName; diff --git a/packages/mds/src/browser.ts b/packages/mds/src/browser.ts index 538bb372..991ad726 100644 --- a/packages/mds/src/browser.ts +++ b/packages/mds/src/browser.ts @@ -29,10 +29,11 @@ export type { LintRuleName, LintSpan, MarkdownResult, - Message, - MessagesResult, + MdsBaseBackend, MdsError, MdsErrorSpan, + Message, + MessagesResult, RuleSeverity, SourceMapV3, } from './types.js'; @@ -116,7 +117,7 @@ export function check(source: string, options?: CheckOptions): CheckResult { * Lint an MDS source string. Returns a LintResult with per-rule findings. * Requires init() to have been called and awaited first. * - * D-TS-07: `lintFile` is intentionally absent from the browser entry. + * `lintFile` is intentionally absent from the browser entry. * `MdsBaseBackend` has no `lintFile` — file operations require `node:fs` which * is unavailable in browser environments. Use `lintVirtual` to lint a * pre-loaded module map, or import from `@mdscript/mds` in Node.js to get diff --git a/packages/mds/src/index.ts b/packages/mds/src/index.ts deleted file mode 100644 index 3b624615..00000000 --- a/packages/mds/src/index.ts +++ /dev/null @@ -1,35 +0,0 @@ -// NOTE (AC-P3-21): index.ts is NOT in the package `exports` map and has no -// `main`/`types` fallback. dist/index.js is unreachable to consumers. This -// barrel is an internal convenience for repo-level tooling only. Public types -// are exported from dist/node.d.ts (Node) and dist/browser.d.ts (browser) -// via the `"."` exports-map entry in package.json. -export type { - BackendType, - CheckFileOptions, - CheckOptions, - CheckResult, - CompileOptions, - CompileResult, - FileOptions, - InitOptions, - LintDiagnostic, - LintFileOptions, - LintFileReport, - LintOptions, - LintResult, - LintRuleName, - LintSpan, - MarkdownResult, - MdsBackend, - MdsBaseBackend, - MdsError, - MdsErrorSpan, - MdsNodeBackend, - Message, - MessagesResult, - RuleSeverity, - SourceMapV3, -} from './types.js'; -export { isMdsError, LINT_RULE_NAMES } from './types.js'; -export type { WasmModule } from './backend/wasm.js'; -export { initWasmNode, initWasmBrowser, createWasmBackend } from './backend/wasm.js'; diff --git a/packages/mds/src/node.ts b/packages/mds/src/node.ts index 6b6117a4..ca7f3575 100644 --- a/packages/mds/src/node.ts +++ b/packages/mds/src/node.ts @@ -57,11 +57,9 @@ export function _resetForTesting(): void { /** * Build the `mds::invalid_options` error for `basePath` on file-surface methods. * - * Separated from the throw so that both the `wrapWithFileOps` guards (synchronous - * throw inside an async function → rejected promise) and the public API guards - * (`Promise.reject(fileBasePathError())` — maintains the sync-throw contract for - * other errors while making basePath a proper promise rejection) can share one - * message string, satisfying U-OV-27's byte-identical requirement (avoids PF-007). + * Separated from the throw so that the public `compileFile` / `checkFile` guards + * share one message string with the napi `parse_file_opts` / `parse_check_file_opts` + * output, satisfying U-OV-27's byte-identical requirement (avoids PF-007). * * Message is byte-identical to napi `parse_file_opts` / `parse_check_file_opts` * (crates/mds-napi/src/lib.rs). Because this guard short-circuits BEFORE backend @@ -116,13 +114,6 @@ function wrapWithFileOps( ...base, async compileFile(path: string, options?: FileOptions): Promise { - // D-TS-06 guard: basePath is not valid for file operations on the WASM path. - // The native path never enters wrapWithFileOps, so napi handles the rejection - // there. BASEPATH_PASSTHROUGH lets basePath through assertKnownKeys; the guard - // here fires on the WASM path before buildModulesMap runs (avoids PF-004). - if ((options as unknown as { basePath?: string })?.basePath != null) { - throw fileBasePathError(); - } const { source, opts } = await prepareFileArgs(path, options); const result: unknown = wasmModule.compile(source, opts); assertResultShape(result, 'compile'); @@ -130,10 +121,6 @@ function wrapWithFileOps( }, async checkFile(path: string, options?: CheckFileOptions): Promise { - // D-TS-06 guard: same reason as compileFile above. - if ((options as unknown as { basePath?: string })?.basePath != null) { - throw fileBasePathError(); - } // CheckFileOptions is a structural subset of FileOptions (only vars, no // sourceMap/sourcesContent), so the cast is safe: prepareFileArgs calls // fileCompileOpt which only picks defined keys; the absent fields resolve as @@ -297,19 +284,17 @@ export function check(source: string, options?: CheckOptions): CheckResult { * Compile an MDS file, resolving @import directives relative to the file. * Returns a discriminated-union CompileResult. Requires init() to have been called and awaited first. * - * Non-async: unknown option keys and pre-init errors throw synchronously (preserving the - * existing contract verified by U-OV-12 and U-B11). The basePath guard returns - * `Promise.reject(fileBasePathError())` so that callers using `assert.rejects()` or - * `.catch()` receive a proper rejected promise rather than a synchronous throw. + * Non-async: all option-validation errors — unknown keys (U-OV-12) and basePath + * (U-OV-32) — throw synchronously before any I/O, consistent with U-B11. Callers + * using `try { compileFile(f, opts) } catch` capture both error classes. + * `.catch()` on the returned promise does NOT receive option-validation errors. */ export function compileFile(path: string, options?: FileOptions): Promise { if (options != null) assertKnownKeys(options, 'compileFile'); // BASEPATH_PASSTHROUGH: assertKnownKeys skips basePath for file methods (issue #74). - // Return a rejected promise (not a synchronous throw) so that the file-op async - // contract is consistent: backend errors arrive as rejections, not sync throws. - // wrapWithFileOps provides a redundant guard on the WASM path (avoids PF-004). - if ((options as unknown as { basePath?: string })?.basePath != null) { - return Promise.reject(fileBasePathError()); + // Throws synchronously — same channel as assertKnownKeys above (U-OV-32). + if ((options as CompileOptions | undefined)?.basePath != null) { + throw fileBasePathError(); } return assertReady().compileFile(path, options); } @@ -320,13 +305,13 @@ export function compileFile(path: string, options?: FileOptions): Promise { if (options != null) assertKnownKeys(options, 'checkFile'); - // Same basePath guard — returns Promise.reject to preserve async contract. - if ((options as unknown as { basePath?: string })?.basePath != null) { - return Promise.reject(fileBasePathError()); + // Same basePath guard — throws synchronously (same channel as assertKnownKeys, U-OV-33). + if ((options as CheckOptions | undefined)?.basePath != null) { + throw fileBasePathError(); } return assertReady().checkFile(path, options); } @@ -358,11 +343,6 @@ export function getBackend(): BackendType { return assertReady().getBackend(); } -// `LINT_RULE_NAMES` is exported here, not only from `index.ts`: the package -// `exports` map resolves `@mdscript/mds` to `dist/node.js` (Node) or -// `dist/browser.js`, and never to `dist/index.js` — a value re-exported only -// from `index.ts` is unreachable for consumers. The browser entry gains it with -// the browser lint surface; today it has no lint API to configure. export { isMdsError, LINT_RULE_NAMES } from './types.js'; export type { BackendType, @@ -380,11 +360,14 @@ export type { LintResult, LintRuleName, LintSpan, - RuleSeverity, MarkdownResult, - Message, - MessagesResult, + MdsBackend, + MdsBaseBackend, MdsError, MdsErrorSpan, + MdsNodeBackend, + Message, + MessagesResult, + RuleSeverity, SourceMapV3, } from './types.js'; diff --git a/packages/mds/src/util/options.ts b/packages/mds/src/util/options.ts index 295c4e16..3a5d1616 100644 --- a/packages/mds/src/util/options.ts +++ b/packages/mds/src/util/options.ts @@ -9,13 +9,30 @@ import type { // ── keysOf helper ────────────────────────────────────────────────────────────── +/** + * String keys of `T` whose non-nullable type is not `never`. + * + * Used to exclude `basePath?: never` marker fields from the {@link keysOf} + * witness requirement. Those fields exist only to prevent structural + * assignability of string-surface options to file-surface option types at + * compile time; they carry no runtime meaning and must not appear in the + * recognised-key list that {@link assertKnownKeys} enforces. + */ +type RuntimeKeys = { + [K in keyof T & string]: [NonNullable] extends [never] ? never : K; +}[keyof T & string]; + /** * Returns the keys of `T` as a readonly string array. * - * The `witness` parameter must supply `true` for every key of `T` — this - * binds the returned list to the interface at compile time. If `T` gains a - * new key the witness literal must be updated; failing to do so is a compile - * error, not a silent omission. + * The `witness` parameter must supply `true` for every runtime key of `T` + * (i.e. every key whose non-nullable type is not `never`) — this binds the + * returned list to the interface at compile time. If `T` gains a new key the + * witness literal must be updated; failing to do so is a compile error, not a + * silent omission. + * + * Keys typed as `?: never` (structural-subtyping blockers) are excluded from + * the witness requirement via {@link RuntimeKeys}. * * @example * ```typescript @@ -23,7 +40,7 @@ import type { * // → readonly ['basePath', 'vars', 'rules'] * ``` */ -function keysOf(witness: Record): readonly string[] { +function keysOf(witness: Record, true>): readonly string[] { return Object.keys(witness); } @@ -64,13 +81,13 @@ export type MethodName = * Reconciliation against napi option parsers (`crates/mds-napi/src/lib.rs`): * - `compile` / `check`: `basePath` is now in the public types (#180 fix) and in * the key list; napi's `parse_compile_opts` / `parse_check_opts` accept it. - * - `compileFile` / `checkFile`: `basePath` is NOT in the key list. It is passed - * through without wrapper interception so the backend can emit its own purpose-built - * error ("not valid for compileFile/checkFile; the base directory is derived from - * the file path"). See {@link BASEPATH_PASSTHROUGH} and issue #74. + * - `compileFile` / `checkFile`: `basePath` is NOT in the key list. When a caller + * passes `basePath` on these methods the wrapper emits a purpose-built rejection via + * {@link BASEPATH_PASSTHROUGH}'s error factory — the backend never receives it. + * See {@link getBasePathError} and issue #74. * - `lint`, `lintFile`, `lintVirtual`: key lists match napi exactly. */ -const METHOD_KEYS: Readonly> = { +export const METHOD_KEYS: Readonly> = { compile: keysOf({ basePath: true, vars: true, sourceMap: true, sourcesContent: true }), check: keysOf({ basePath: true, vars: true }), compileFile: keysOf({ vars: true, sourceMap: true, sourcesContent: true }), From 891db041c9d8020539b57c3e4ade4a0504b81b7e Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:20:02 +0300 Subject: [PATCH 11/26] docs(changelog): fix three factual inaccuracies in [Unreleased] section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - basePath propagation claim (#180 Fixed): was "all four per-surface option builders (compileSrcOpt, checkSrcOpt, fileCompileOpt, fileCheckOpt)". fileCompileOpt/fileCheckOpt deliberately NEVER forward basePath (D-TS-02/ D-TS-05), and the BREAKING subsection 30 lines later says the opposite. Correct to: propagates to the backend for the string-source methods (compile, check); compileFile/checkFile deliberately exclude basePath. - Lint-types count (#215 Added): "All seven lint types" followed by a list of eight. The list is correct; fix the prose count to "eight". - Lint-types export scope (#215 Added): "exported from both the Node.js and browser entry points" overstates what changed — all eight were already exported from node.ts at the wave base. Only the browser entry is newly gaining them. Correct to "from the browser entry point as well as the Node.js entry point". - Drop stale lintVirtual references in basePath migration guidance; native backend is the correct migration path. Co-Authored-By: Claude --- CHANGELOG.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f388bb39..8e9aa455 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,15 +16,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 containing `@import` or `@extends` directives compiled with a string-source call and a `basePath` option would either fail to resolve their imports (native backend) or fail silently (WASM backend). The fix adds `basePath` to both `CompileOptions` and - `CheckOptions` and propagates it through all four per-surface option builders - (`compileSrcOpt`, `checkSrcOpt`, `fileCompileOpt`, `fileCheckOpt`). + `CheckOptions` and propagates it to the backend for the string-source methods + (`compile`, `check`). `compileFile` and `checkFile` deliberately exclude + `basePath` — the base directory for file operations is derived from the file + path itself (see the BREAKING subsection below). The WASM backend has no filesystem access and cannot resolve file-relative imports; it now **rejects** a non-null `basePath` immediately with `mds::invalid_options` instead of silently ignoring it, so misconfigured callers receive an actionable error rather than a silent wrong answer. `{basePath: undefined}` is treated as absent on both backends (`!= null` check; value-is-intent). To use `basePath` with import resolution, - set `MDS_BACKEND=native` or use `lintVirtual` with a pre-resolved module map. + set `MDS_BACKEND=native`. ### Added @@ -34,10 +36,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `browser.ts`. Both functions are available after `init()` and follow the same unknown-option guard used by `compile`/`check`. - All seven lint types (`LintDiagnostic`, `LintFileOptions`, `LintFileReport`, + All eight lint types (`LintDiagnostic`, `LintFileOptions`, `LintFileReport`, `LintOptions`, `LintResult`, `LintRuleName`, `LintSpan`, `RuleSeverity`) and the - `LINT_RULE_NAMES` constant are now exported from both the Node.js and browser entry - points. + `LINT_RULE_NAMES` constant are now exported from the browser entry point as well as + the Node.js entry point. - **`SourceMapV3` is now exported from the Node.js and browser entry points.** `MarkdownResult.sourceMap` has always been typed as `SourceMapV3`, but the type was @@ -99,8 +101,7 @@ previously silently ignored `basePath` on the WASM backend. They now throw already documented as WASM-unsupported; it now enforces this at runtime too. **Migration:** switch to the native backend (`MDS_BACKEND=native`) when you need -import resolution with a `basePath`, or use `lintVirtual` with a pre-resolved module -map in WASM environments. +import resolution with a `basePath` in WASM environments. ### **BREAKING** — Interpolation syntax: `{x}` → `{{x}}` From 3a9e2beec7544e0edfb3b21ab0724cd4125c767d Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:21:42 +0300 Subject: [PATCH 12/26] refactor(mds): add basePath?: never to file-surface types and unify option forwarding - Add `basePath?: never` to FileOptions, CheckFileOptions, and LintFileOptions so structural assignment from a string-surface type (basePath?: string) fails at compile time. This satisfies the AC-P3-20 variable-passing negative cases and aligns with `@ts-expect-error` guards in consumer-node.ts. - Replace per-surface option builder functions (compileSrcOpt, checkSrcOpt, fileCompileOpt, fileCheckOpt, lintOpt, lintFileOpt) with a single forwardOpts helper keyed on METHOD_KEYS. Single authoritative source for which keys are forwarded per method name. - Add makeFileBasePathError factory; change BASEPATH_PASSTHROUGH from ReadonlySet to ReadonlyMap so adding a new file-surface method without an error factory is a TypeScript error at the Map literal. - Convert U-OV-25/U-OV-26 from async assert.rejects to synchronous assert.throws: the basePath guard now throws before any I/O (per U-OV-32), and assert.rejects in Node.js v22 does not intercept synchronous throws. - Fix compileFile/checkFile basePath cast: (options as unknown as { basePath?: string }) is required because FileOptions.basePath?: never is structurally incompatible with CompileOptions for a direct cast. --- .../mds/__test__/options-validation.spec.mjs | 284 ++++++++++++------ packages/mds/src/backend/native.ts | 35 +-- packages/mds/src/backend/wasm.ts | 32 +- packages/mds/src/node.ts | 47 +-- packages/mds/src/types.ts | 28 +- packages/mds/src/util/options.ts | 185 +++++++----- 6 files changed, 348 insertions(+), 263 deletions(-) diff --git a/packages/mds/__test__/options-validation.spec.mjs b/packages/mds/__test__/options-validation.spec.mjs index 8974af7f..a934a2d0 100644 --- a/packages/mds/__test__/options-validation.spec.mjs +++ b/packages/mds/__test__/options-validation.spec.mjs @@ -27,7 +27,7 @@ import { isMdsError, init, } from '../dist/node.js'; -import { assertKnownKeys } from '../dist/util/options.js'; +import { assertKnownKeys, METHOD_KEYS, forwardOpts } from '../dist/util/options.js'; import * as os from 'node:os'; import * as fs from 'node:fs'; import * as path from 'node:path'; @@ -312,54 +312,45 @@ describe('options-validation', () => { // was that basePath was accepted and then dropped). After the fix, the call MUST // throw with code 'mds::invalid_options' and a purpose-built message. - test('U-OV-25: compileFile rejects basePath with a purposeful error (AC-P3-06, AC-P3-07)', async () => { - const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-test-')); - const file = path.join(tmp, 'ok.mds'); - fs.writeFileSync(file, 'Hello\n', 'utf8'); - try { - await assert.rejects( - () => compileFile(file, { basePath: '.' }), - (err) => { - assert.ok(isMdsError(err), `expected isMdsError, got: ${err}`); - assert.equal(err.code, 'mds::invalid_options'); - // Must name basePath and state the base is derived from the file path. - assert.ok(err.message.includes('basePath'), `basePath in message: ${err.message}`); - assert.ok(err.message.includes('derived from the file path'), `remedy in message: ${err.message}`); - // Must NOT be the generic unknown-key message (AC-P3-07). - assert.ok( - !err.message.startsWith('unknown option key'), - `must not be generic rejection: "${err.message}"`, - ); - return true; - }, - ); - } finally { - fs.rmSync(tmp, { recursive: true, force: true }); - } + test('U-OV-25: compileFile throws for basePath with a purposeful error (AC-P3-06, AC-P3-07)', () => { + // basePath guard fires synchronously before any I/O — no real file needed + // (per U-OV-32 / the note at line 168–177: assert.rejects does NOT intercept + // synchronous throws in Node v22 and the error escapes with failureType + // 'testCodeFailure', masking the regression). + assert.throws( + () => compileFile('/any.mds', { basePath: '.' }), + (err) => { + assert.ok(isMdsError(err), `expected isMdsError, got: ${err}`); + assert.equal(err.code, 'mds::invalid_options'); + // Must name basePath and state the base is derived from the file path. + assert.ok(err.message.includes('basePath'), `basePath in message: ${err.message}`); + assert.ok(err.message.includes('derived from the file path'), `remedy in message: ${err.message}`); + // Must NOT be the generic unknown-key message (AC-P3-07). + assert.ok( + !err.message.startsWith('unknown option key'), + `must not be generic rejection: "${err.message}"`, + ); + return true; + }, + ); }); - test('U-OV-26: checkFile rejects basePath with a purposeful error (AC-P3-06, AC-P3-07)', async () => { - const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-test-')); - const file = path.join(tmp, 'ok.mds'); - fs.writeFileSync(file, 'Hello\n', 'utf8'); - try { - await assert.rejects( - () => checkFile(file, { basePath: '.' }), - (err) => { - assert.ok(isMdsError(err), `expected isMdsError, got: ${err}`); - assert.equal(err.code, 'mds::invalid_options'); - assert.ok(err.message.includes('basePath'), `basePath in message: ${err.message}`); - assert.ok(err.message.includes('derived from the file path'), `remedy in message: ${err.message}`); - assert.ok( - !err.message.startsWith('unknown option key'), - `must not be generic rejection: "${err.message}"`, - ); - return true; - }, - ); - } finally { - fs.rmSync(tmp, { recursive: true, force: true }); - } + test('U-OV-26: checkFile throws for basePath with a purposeful error (AC-P3-06, AC-P3-07)', () => { + // Same synchronous-throw contract as U-OV-25 — use assert.throws (not assert.rejects). + assert.throws( + () => checkFile('/any.mds', { basePath: '.' }), + (err) => { + assert.ok(isMdsError(err), `expected isMdsError, got: ${err}`); + assert.equal(err.code, 'mds::invalid_options'); + assert.ok(err.message.includes('basePath'), `basePath in message: ${err.message}`); + assert.ok(err.message.includes('derived from the file path'), `remedy in message: ${err.message}`); + assert.ok( + !err.message.startsWith('unknown option key'), + `must not be generic rejection: "${err.message}"`, + ); + return true; + }, + ); }); // ── basePath honored on native backend (AC-P3-01 / AC-P3-02 / AC-P3-03) ───── @@ -468,45 +459,70 @@ describe('options-validation', () => { const be = createNativeBackend(spyAddon); - const VARS = { k: 1 }; - const RULES = { 'unused-variable': 'warn' }; - const BP = '/some/base/path'; + // A distinguishable test value for each possible option key. Covers every + // key currently appearing in any method's METHOD_KEYS entry. + // + // MAINTENANCE NOTE: if a new option key is added to any option interface, + // add a non-null entry here — without it, fullOpts() would supply `undefined` + // for the new key, and pickDefined / lintOpt / lintFileOpt would correctly + // omit it, making the deepStrictEqual pass vacuously for that key. + const KEY_VALUES = { + basePath: '/some/base/path', + vars: { k: 1 }, + sourceMap: true, + sourcesContent: true, + rules: { 'unused-variable': 'warn' }, + }; + + // Build an options object with ALL keys that METHOD_KEYS[method] accepts, + // each with a distinguishable value. This is the AC-P3-05 positive control: + // if a key is added to METHOD_KEYS (accepted at validation) but omitted from + // the forwarding builder (pickDefined / lintOpt / lintFileOpt), expected still + // includes that key, so deepStrictEqual catches the drift — reproducing the + // #180 bug class at the point of introduction rather than at runtime in + // production (avoids PF-013: a hand-typed expected cannot detect this drift). + function fullOpts(methodName) { + const keys = METHOD_KEYS[methodName] ?? []; + const obj = {}; + for (const k of keys) obj[k] = KEY_VALUES[k]; + return obj; + } const cases = [ { name: 'compile', - call: () => be.compile('', { basePath: BP, vars: VARS, sourceMap: true, sourcesContent: true }), - expected: { basePath: BP, vars: VARS, sourceMap: true, sourcesContent: true }, + call: () => be.compile('', fullOpts('compile')), + expected: fullOpts('compile'), }, { name: 'check', - call: () => be.check('', { basePath: BP, vars: VARS }), - expected: { basePath: BP, vars: VARS }, + call: () => be.check('', fullOpts('check')), + expected: fullOpts('check'), }, { name: 'compileFile', - call: () => be.compileFile('/any.mds', { vars: VARS, sourceMap: true, sourcesContent: true }), - expected: { vars: VARS, sourceMap: true, sourcesContent: true }, + call: () => be.compileFile('/any.mds', fullOpts('compileFile')), + expected: fullOpts('compileFile'), }, { name: 'checkFile', - call: () => be.checkFile('/any.mds', { vars: VARS }), - expected: { vars: VARS }, + call: () => be.checkFile('/any.mds', fullOpts('checkFile')), + expected: fullOpts('checkFile'), }, { name: 'lint', - call: () => be.lint('', { basePath: BP, vars: VARS, rules: RULES }), - expected: { basePath: BP, vars: VARS, rules: RULES }, + call: () => be.lint('', fullOpts('lint')), + expected: fullOpts('lint'), }, { name: 'lintFile', - call: () => be.lintFile('/any.mds', { vars: VARS, rules: RULES }), - expected: { vars: VARS, rules: RULES }, + call: () => be.lintFile('/any.mds', fullOpts('lintFile')), + expected: fullOpts('lintFile'), }, { name: 'lintVirtual', - call: () => be.lintVirtual({ 'a.mds': '' }, 'a.mds', { vars: VARS, rules: RULES }), - expected: { vars: VARS, rules: RULES }, + call: () => be.lintVirtual({ 'a.mds': '' }, 'a.mds', fullOpts('lintVirtual')), + expected: fullOpts('lintVirtual'), }, ]; @@ -523,7 +539,7 @@ describe('options-validation', () => { // ── cross-backend message equality for file basePath (AC-P3-06 / U-OV-27) ─ - test('U-OV-27: compileFile/checkFile basePath rejection message is byte-identical to napi and backend-independent (AC-P3-06, avoids PF-007)', async () => { + test('U-OV-27: compileFile/checkFile basePath rejection message is byte-identical to napi (AC-P3-06, avoids PF-007)', async () => { const addon = requireNativeAddon(); // hard-fail without addon const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-bp-')); @@ -538,10 +554,14 @@ describe('options-validation', () => { try { for (const method of ['compileFile', 'checkFile']) { // The wrapper's public file methods short-circuit basePath in node.ts - // (fileBasePathError) BEFORE backend dispatch — verified below by the - // backend-independence leg. That makes the wrapper message the operative - // one on every backend, so the authoritative parity assertion is - // wrapper-vs-napi, NOT wrapper-vs-wrapper. + // (getBasePathError) BEFORE assertReady() — the backend is never consulted + // for this input. MDS_BACKEND is read only during init(), which this guard + // precedes. A subprocess under MDS_BACKEND=wasm therefore produces byte- + // identical output to the native path: that assertion would be a tautology + // that compares the guard against itself and cannot fail while LEG 1 passes + // (avoids PF-013 on the 'backend-independence' claim). The message is + // structurally backend-independent by construction, not by test. + // LEG 1 below is the real drift guard. const wrapperMsg = await captureMsg( () => (method === 'compileFile' ? compileFile : checkFile)(file, { basePath: '.' }), ); @@ -555,42 +575,15 @@ describe('options-validation', () => { () => (method === 'compileFile' ? addon.compileFile : addon.checkFile)(file, { basePath: '.' }), ); - // LEG 2: the same public call under MDS_BACKEND=wasm must produce the same - // message. Subprocess for full isolation (no shared module singleton). - const { execFileSync } = await import('node:child_process'); - const wasmMsg = await captureMsg(() => { - const script = ` -import { ${method} } from './dist/node.js'; -${method}(${JSON.stringify(file)}, { basePath: '.' }).catch(e => { - process.stdout.write(e.message); - process.exit(0); -}).then(r => { if (r !== undefined) process.exit(0); }); -`; - const out = execFileSync(process.execPath, ['--input-type=module'], { - input: script, - cwd: PKG_DIR, - env: { ...process.env, MDS_BACKEND: 'wasm' }, - timeout: 15000, - }); - throw new Error(out.toString().trim() || 'WASM subprocess produced no output'); - }); - - // Positive controls: every leg must have actually produced an error. A - // silently-empty message would make the equality assertions vacuous. + // Positive controls: each leg must have produced an error. assert.ok(wrapperMsg.length > 0, `wrapper ${method} must throw for basePath`); assert.ok(napiMsg.length > 0, `napi ${method} must throw for basePath`); - assert.ok(wasmMsg.length > 0, `WASM-backend ${method} must throw for basePath`); assert.strictEqual( wrapperMsg, napiMsg, `wrapper must match napi byte-for-byte for ${method} — wrapper: "${wrapperMsg}" | napi: "${napiMsg}"`, ); - assert.strictEqual( - wrapperMsg, - wasmMsg, - `message must be backend-independent for ${method} — native-default: "${wrapperMsg}" | MDS_BACKEND=wasm: "${wasmMsg}"`, - ); } } finally { fs.rmSync(tmp, { recursive: true, force: true }); @@ -775,4 +768,101 @@ ${method}(${JSON.stringify(file)}, { basePath: '.' }).catch(e => { '__proto__ in object literal is not an own enumerable key; no invalid_options error should fire', ); }); + + // ── basePath throws synchronously on file methods (single error channel) ── + // + // assertKnownKeys (unknown-key errors) throws synchronously for ALL methods. + // The basePath guard on compileFile/checkFile must use the SAME channel so that + // `try { compileFile(f, opts) } catch` captures both error classes. + // Using assert.throws (not assert.rejects) is intentional: a regression to + // Promise.reject would escape the catch and surface as an unhandled rejection, + // failing the test with a process warning rather than a clean assertion failure. + + test('U-OV-32: compileFile throws synchronously for basePath (same channel as assertKnownKeys)', () => { + // Fires before assertReady(), so no real file or init() is needed. + assert.throws( + () => compileFile('/any.mds', { basePath: '.' }), + (err) => { + assert.ok(isMdsError(err), `expected isMdsError, got: ${err}`); + assert.equal(err.code, 'mds::invalid_options'); + assert.ok(err.message.includes('basePath'), `basePath in message: ${err.message}`); + assert.ok( + !err.message.startsWith('unknown option key'), + `must not be generic rejection: "${err.message}"`, + ); + return true; + }, + 'compileFile must throw synchronously for basePath — not return a rejected promise', + ); + }); + + test('U-OV-33: checkFile throws synchronously for basePath (same channel as assertKnownKeys)', () => { + // Fires before assertReady(), so no real file or init() is needed. + assert.throws( + () => checkFile('/any.mds', { basePath: '.' }), + (err) => { + assert.ok(isMdsError(err), `expected isMdsError, got: ${err}`); + assert.equal(err.code, 'mds::invalid_options'); + assert.ok(err.message.includes('basePath'), `basePath in message: ${err.message}`); + assert.ok( + !err.message.startsWith('unknown option key'), + `must not be generic rejection: "${err.message}"`, + ); + return true; + }, + 'checkFile must throw synchronously for basePath — not return a rejected promise', + ); + }); + + // ── compile-options builder fast path (AC-P3-22 second half) ──────────────── + // + // Test-plan item 17: "a unit assertion that the compile-options builder returns + // undefined (strictly, not {}) … assert compileOpts(undefined) === compileOpts({}) + // by object identity". The WASM backend's compileOpts() fast path at wasm.ts:363-365 + // fires only when fileCompileOpt (called internally by compileOpts) returns undefined. + // If compileSrcOpt or fileCompileOpt returned {} for empty input, every compile call + // would allocate a new object rather than reusing DEFAULT_COMPILE_OPTS — the + // regression would be invisible to U-PF1/U-PF2 (too much headroom) and no other + // test pins this invariant (avoids PF-013: absence of an allocation test cannot + // detect a silent allocation regression). + + test('U-OV-34: forwardOpts returns undefined for empty compile input, and the WASM backend reuses DEFAULT_COMPILE_OPTS by identity (AC-P3-22)', async () => { + // Part 1: forwardOpts returns undefined (strictly) for empty input — {} would + // allocate a new object and defeat the WASM DEFAULT_COMPILE_OPTS identity fast path. + assert.strictEqual(forwardOpts(undefined, 'compile'), undefined, + 'forwardOpts(undefined, compile) must be undefined — {} would defeat the WASM fast path'); + assert.strictEqual(forwardOpts({}, 'compile'), undefined, + 'forwardOpts({}, compile) must be undefined — {} would defeat the WASM fast path'); + assert.strictEqual(forwardOpts(null, 'compile'), undefined, + 'forwardOpts(null, compile) must be undefined'); + + // Part 2: object-identity assertion through a WASM spy. compileOpts() inside + // the WASM backend must return the SAME DEFAULT_COMPILE_OPTS reference for both + // undefined and {} options, proving the fast-path branch at wasm.ts:363-365 fires. + // We test this through a spy wasmModule rather than exporting the private + // compileOpts / DEFAULT_COMPILE_OPTS symbols from wasm.ts. + const { createWasmBackend } = await import('../dist/backend/wasm.js'); + let capturedWasmOpts; + const spyWasmModule = { + compile: (_src, opts) => { capturedWasmOpts = opts; return { kind: 'markdown', output: '', warnings: [], dependencies: [] }; }, + check: (_src, _opts) => ({ warnings: [] }), + lint: (_src, _opts) => ({ version: 1, files: [], truncated: false }), + lintVirtual: (_m, _e, _opts) => ({ version: 1, files: [], truncated: false }), + scanImports: (_src) => [], + }; + const wasmBe = createWasmBackend(spyWasmModule); + + wasmBe.compile('', undefined); + const optsForUndefined = capturedWasmOpts; + + wasmBe.compile('', {}); + const optsForEmpty = capturedWasmOpts; + + assert.strictEqual( + optsForUndefined, + optsForEmpty, + 'compileOpts(undefined) and compileOpts({}) must return the same DEFAULT_COMPILE_OPTS object ' + + '— identity fast path at wasm.ts:363-365 must fire for both inputs', + ); + }); }); diff --git a/packages/mds/src/backend/native.ts b/packages/mds/src/backend/native.ts index 7512da65..1d0fb1d3 100644 --- a/packages/mds/src/backend/native.ts +++ b/packages/mds/src/backend/native.ts @@ -11,7 +11,7 @@ import type { LintResult, MdsNodeBackend, } from '../types.js'; -import { compileSrcOpt, checkSrcOpt, fileCompileOpt, fileCheckOpt } from '../util/options.js'; +import { forwardOpts } from '../util/options.js'; import { assertResultShape, validateBackendMethods, BASE_METHODS, NODE_METHODS } from './contract.js'; /** Options forwarded to the napi addon for source-string compile (accepts basePath). */ @@ -63,25 +63,6 @@ interface NapiAddon { lintVirtual(modules: Record, entry: string, opts?: NapiLintFileOpts): unknown; } -/** Build lint options from LintOptions, omitting null/undefined entries. */ -function lintOpt(options?: LintOptions): NapiLintOpts | undefined { - if (options == null) return undefined; - const out: NapiLintOpts = {}; - if (options.basePath != null) out.basePath = options.basePath; - if (options.vars != null) out.vars = options.vars; - if (options.rules != null) out.rules = options.rules; - return Object.keys(out).length > 0 ? out : undefined; -} - -/** Build lint file options from LintFileOptions, omitting null/undefined entries. */ -function lintFileOpt(options?: LintFileOptions): NapiLintFileOpts | undefined { - if (options == null) return undefined; - const out: NapiLintFileOpts = {}; - if (options.vars != null) out.vars = options.vars; - if (options.rules != null) out.rules = options.rules; - return Object.keys(out).length > 0 ? out : undefined; -} - /** * Create a native (napi) backend adapter from an injected addon. * @@ -99,43 +80,43 @@ export function createNativeBackend(addon: NapiAddon): MdsNodeBackend { return { compile(source: string, options?: CompileOptions): CompileResult { - const result: unknown = addon.compile(source, compileSrcOpt(options) as NapiCompileOpts | undefined); + const result: unknown = addon.compile(source, forwardOpts(options, 'compile') as NapiCompileOpts | undefined); assertResultShape(result, 'compile'); return result as CompileResult; }, check(source: string, options?: CheckOptions): CheckResult { - const result: unknown = addon.check(source, checkSrcOpt(options) as NapiCheckOpts | undefined); + const result: unknown = addon.check(source, forwardOpts(options, 'check') as NapiCheckOpts | undefined); assertResultShape(result, 'check'); return result as CheckResult; }, async compileFile(path: string, options?: FileOptions): Promise { - const result: unknown = await addon.compileFile(path, fileCompileOpt(options) as NapiFileCompileOpts | undefined); + const result: unknown = await addon.compileFile(path, forwardOpts(options, 'compileFile') as NapiFileCompileOpts | undefined); assertResultShape(result, 'compile'); return result as CompileResult; }, async checkFile(path: string, options?: CheckFileOptions): Promise { - const result: unknown = await addon.checkFile(path, fileCheckOpt(options)); + const result: unknown = await addon.checkFile(path, forwardOpts(options, 'checkFile') as NapiFileCheckOpts | undefined); assertResultShape(result, 'check'); return result as CheckResult; }, lint(source: string, options?: LintOptions): LintResult { - const result: unknown = addon.lint(source, lintOpt(options)); + const result: unknown = addon.lint(source, forwardOpts(options, 'lint') as NapiLintOpts | undefined); assertResultShape(result, 'lint'); return result as LintResult; }, async lintFile(path: string, options?: LintFileOptions): Promise { - const result: unknown = await addon.lintFile(path, lintFileOpt(options)); + const result: unknown = await addon.lintFile(path, forwardOpts(options, 'lintFile') as NapiLintFileOpts | undefined); assertResultShape(result, 'lint'); return result as LintResult; }, lintVirtual(modules: Record, entry: string, options?: LintFileOptions): LintResult { - const result: unknown = addon.lintVirtual(modules, entry, lintFileOpt(options)); + const result: unknown = addon.lintVirtual(modules, entry, forwardOpts(options, 'lintVirtual') as NapiLintFileOpts | undefined); assertResultShape(result, 'lint'); return result as LintResult; }, diff --git a/packages/mds/src/backend/wasm.ts b/packages/mds/src/backend/wasm.ts index 234976f8..dcf4c93c 100644 --- a/packages/mds/src/backend/wasm.ts +++ b/packages/mds/src/backend/wasm.ts @@ -12,7 +12,7 @@ import type { MdsBaseBackend, } from '../types.js'; import { assertResultShape, validateBackendMethods, WASM_EXPORTS } from './contract.js'; -import { fileCompileOpt } from '../util/options.js'; +import { forwardOpts } from '../util/options.js'; /** * Shape of the WASM module exports (built with wasm-pack). @@ -359,9 +359,10 @@ function compileOpts( sourceMap?: boolean; sourcesContent?: boolean; } { - // D-TS-06: use fileCompileOpt (no basePath) — basePath is already excluded - // from _WasmCompileInput, and the caller guards against it before reaching here. - const extra = fileCompileOpt(options); + // D-TS-06: forward only the compileFile-surface keys (vars, sourceMap, sourcesContent) + // via METHOD_KEYS.compileFile. basePath is excluded from _WasmCompileInput and the + // caller guards against it before reaching here; forwardOpts cannot include it. + const extra = forwardOpts(options, 'compileFile'); const filename = options?.filename ?? DEFAULT_COMPILE_OPTS.filename; const modules = options?.modules ?? DEFAULT_COMPILE_OPTS.modules; if (extra == null && filename === DEFAULT_COMPILE_OPTS.filename && modules === DEFAULT_COMPILE_OPTS.modules) { @@ -392,7 +393,9 @@ export function fileOpts( sourceMap?: boolean; sourcesContent?: boolean; } { - const extra = fileCompileOpt(options); + // Use forwardOpts for the user-visible keys (vars, sourceMap, sourcesContent) + // so METHOD_KEYS.compileFile is the single authoritative source for what is forwarded. + const extra = forwardOpts(options, 'compileFile'); return extra != null ? { filename: entryFilename, modules, ...extra } : { filename: entryFilename, modules }; @@ -450,15 +453,11 @@ export function createWasmBackend(wasmModule: WasmModule): MdsBaseBackend { lint(source: string, options?: LintOptions): LintResult { // OD-1: reject basePath rather than silently ignoring it. if (options?.basePath != null) throwWasmBasePathError(); - const opts: { - vars?: Record; - rules?: Record; - } = {}; - if (options?.vars != null) opts.vars = options.vars; - if (options?.rules != null) opts.rules = options.rules; + // forwardOpts uses METHOD_KEYS.lint (vars, rules only after the guard above + // ensures basePath is null/undefined and thus excluded by != null check). const result: unknown = wasmModule.lint( source, - Object.keys(opts).length > 0 ? opts : undefined, + forwardOpts(options, 'lint') as { vars?: Record; rules?: Record } | undefined, ); assertResultShape(result, 'lint'); return result as LintResult; @@ -469,16 +468,11 @@ export function createWasmBackend(wasmModule: WasmModule): MdsBaseBackend { entry: string, options?: LintFileOptions, ): LintResult { - const opts: { - vars?: Record; - rules?: Record; - } = {}; - if (options?.vars != null) opts.vars = options.vars; - if (options?.rules != null) opts.rules = options.rules; + // forwardOpts uses METHOD_KEYS.lintVirtual (vars, rules). const result: unknown = wasmModule.lintVirtual( modules, entry, - Object.keys(opts).length > 0 ? opts : undefined, + forwardOpts(options, 'lintVirtual') as { vars?: Record; rules?: Record } | undefined, ); assertResultShape(result, 'lint'); return result as LintResult; diff --git a/packages/mds/src/node.ts b/packages/mds/src/node.ts index ca7f3575..62860f6c 100644 --- a/packages/mds/src/node.ts +++ b/packages/mds/src/node.ts @@ -17,7 +17,7 @@ import { assertResultShape } from './backend/contract.js'; import { initWasmNode, createWasmBackend, fileOpts } from './backend/wasm.js'; import type { WasmModule } from './backend/wasm.js'; import { buildModulesMap } from './util/module-scanner.js'; -import { assertKnownKeys } from './util/options.js'; +import { assertKnownKeys, forwardOpts, getBasePathError } from './util/options.js'; // Read MDS_BACKEND at module scope — sync, deterministic, no I/O. const rawBackend = process.env['MDS_BACKEND']; @@ -54,29 +54,6 @@ export function _resetForTesting(): void { // File-ops wrapper // --------------------------------------------------------------------------- -/** - * Build the `mds::invalid_options` error for `basePath` on file-surface methods. - * - * Separated from the throw so that the public `compileFile` / `checkFile` guards - * share one message string with the napi `parse_file_opts` / `parse_check_file_opts` - * output, satisfying U-OV-27's byte-identical requirement (avoids PF-007). - * - * Message is byte-identical to napi `parse_file_opts` / `parse_check_file_opts` - * (crates/mds-napi/src/lib.rs). Because this guard short-circuits BEFORE backend - * dispatch, napi never actually produces the message for a public `compileFile` / - * `checkFile` call — so nothing enforces the two strings staying in sync except - * U-OV-27, which compares this message against the raw addon's at runtime. Keep - * the two in lockstep; editing either alone fails that test. - */ -function fileBasePathError(): Error & { code: string } { - const err = new Error( - 'option "basePath" is not valid for compileFile/checkFile; ' + - 'the base directory is derived from the file path', - ) as Error & { code: string }; - err.code = 'mds::invalid_options'; - return err; -} - /** * Wrap a MdsBaseBackend with file-based compile/check operations, producing * a MdsNodeBackend. The wasmModule is captured so compileFile/checkFile can @@ -290,11 +267,13 @@ export function check(source: string, options?: CheckOptions): CheckResult { * `.catch()` on the returned promise does NOT receive option-validation errors. */ export function compileFile(path: string, options?: FileOptions): Promise { - if (options != null) assertKnownKeys(options, 'compileFile'); - // BASEPATH_PASSTHROUGH: assertKnownKeys skips basePath for file methods (issue #74). - // Throws synchronously — same channel as assertKnownKeys above (U-OV-32). - if ((options as CompileOptions | undefined)?.basePath != null) { - throw fileBasePathError(); + if (options != null) { + assertKnownKeys(options, 'compileFile'); + // BASEPATH_PASSTHROUGH: assertKnownKeys skips basePath for file methods (issue #74). + // Throws synchronously — same channel as assertKnownKeys above (U-OV-32). + // getBasePathError() handles the cast internally via Record. + const bpErr = getBasePathError(options, 'compileFile'); + if (bpErr != null) throw bpErr; } return assertReady().compileFile(path, options); } @@ -308,10 +287,12 @@ export function compileFile(path: string, options?: FileOptions): Promise { - if (options != null) assertKnownKeys(options, 'checkFile'); - // Same basePath guard — throws synchronously (same channel as assertKnownKeys, U-OV-33). - if ((options as CheckOptions | undefined)?.basePath != null) { - throw fileBasePathError(); + if (options != null) { + assertKnownKeys(options, 'checkFile'); + // Same basePath guard — throws synchronously (same channel as assertKnownKeys, U-OV-33). + // getBasePathError() handles the cast internally via Record. + const bpErr = getBasePathError(options, 'checkFile'); + if (bpErr != null) throw bpErr; } return assertReady().checkFile(path, options); } diff --git a/packages/mds/src/types.ts b/packages/mds/src/types.ts index d5b6d43f..5b91a1c5 100644 --- a/packages/mds/src/types.ts +++ b/packages/mds/src/types.ts @@ -86,7 +86,7 @@ export interface CheckResult { * Options for check-only operations (no source-map generation). * Accepted by {@link MdsBaseBackend.check}. * - * D-TS-01: `basePath` is added here so that {@link CompileOptions} inherits it + * `basePath` is added here so that {@link CompileOptions} inherits it * via `extends CheckOptions`. Both compile and check resolve `@import` directives * against this directory when the source string is not backed by a file path. * The WASM backend rejects a non-null `basePath` at runtime (no filesystem access); @@ -101,7 +101,7 @@ export interface CheckOptions { * * **WASM backend:** rejected at runtime with `mds::invalid_options` — the WASM * backend has no filesystem access. Set `MDS_BACKEND=native` to force the native - * backend, or supply all modules inline with `lintVirtual`. + * backend. */ basePath?: string; } @@ -134,7 +134,7 @@ export interface CompileOptions extends CheckOptions { /** * Options for file-based compile operations. * - * D-TS-02: `FileOptions` deliberately does NOT extend `CompileOptions`. After + * `FileOptions` deliberately does NOT extend `CompileOptions`. After * `CompileOptions` gained `basePath`, inheriting it here would silently add a * field that is not valid for file-surface operations (the base directory is * derived from the file path). The fields are declared directly so that adding @@ -152,6 +152,11 @@ export interface FileOptions { * **Privacy warning**: embeds the full template source. Only use in trusted environments. */ sourcesContent?: boolean; + /** + * Not accepted on file operations: the base directory is derived from the file + * path. Passing a non-null value throws `mds::invalid_options`. + */ + basePath?: never; } /** @@ -163,6 +168,11 @@ export interface FileOptions { export interface CheckFileOptions { /** Runtime variables made available for interpolation in the template. */ vars?: Record; + /** + * Not accepted on file operations: the base directory is derived from the file + * path. Passing a non-null value throws `mds::invalid_options`. + */ + basePath?: never; } // --------------------------------------------------------------------------- @@ -303,7 +313,7 @@ export interface LintOptions { * Base directory for resolving `@import` directives in the source string. * Required when the source contains `@import` or `@extends`. * - * OD-1 resolution: the WASM backend **rejects** a non-null `basePath` with + * The WASM backend **rejects** a non-null `basePath` with * `mds::invalid_options` rather than silently ignoring it. This surfaces the * misconfiguration instead of linting a partially-resolved module graph. * Set `MDS_BACKEND=native` to force the native backend, or use @@ -325,6 +335,12 @@ export interface LintFileOptions { * accepted for forward compatibility with future rule names. */ rules?: Record; + /** + * Not accepted: `lintFile` derives the base directory from the file path; + * `lintVirtual` resolves imports against the caller-supplied module map. + * Passing a non-null value throws `mds::invalid_options`. + */ + basePath?: never; } /** Source location of a compiler error. */ @@ -367,10 +383,10 @@ export interface InitOptions { * Browser-safe backend interface — compile/check/lint/lintVirtual/getBackend. * Does not include file operations (which require node:fs). * - * D-TS-01: all string-surface methods accept `basePath` via their options type + * All string-surface methods accept `basePath` via their options type * (`CompileOptions` / `CheckOptions` / `LintOptions`). The WASM implementation's * runtime contract for `basePath`: a non-null value throws `mds::invalid_options` - * instead of silently ignoring it (OD-1; avoids PF-004). + * instead of silently ignoring it. */ export interface MdsBaseBackend { compile(source: string, options?: CompileOptions): CompileResult; diff --git a/packages/mds/src/util/options.ts b/packages/mds/src/util/options.ts index 3a5d1616..99092ac2 100644 --- a/packages/mds/src/util/options.ts +++ b/packages/mds/src/util/options.ts @@ -97,25 +97,78 @@ export const METHOD_KEYS: Readonly> = { lintVirtual: keysOf({ vars: true, rules: true }), }; +// ── basePath error factory for file-surface methods ──────────────────────────── + /** - * Methods for which `basePath` is passed through to the backend without wrapper - * interception (issue #74). The backend emits a purpose-built actionable error - * for these methods ("not valid for compileFile/checkFile; the base directory is - * derived from the file path") rather than the generic "unknown option key" format. + * Build the `mds::invalid_options` error for `basePath` on file-surface methods. + * + * Message is byte-identical to napi `parse_file_opts` / `parse_check_file_opts` + * (crates/mds-napi/src/lib.rs). The wrapper emits this error BEFORE backend + * dispatch, so napi never produces this message for a public `compileFile` / + * `checkFile` call. Nothing enforces the two strings staying in sync except + * U-OV-27, which compares this message against the raw addon's at runtime. + * Keep the two in lockstep; editing either alone fails that test. + */ +function makeFileBasePathError(): Error & { code: string } { + const err = new Error( + 'option "basePath" is not valid for compileFile/checkFile; ' + + 'the base directory is derived from the file path', + ) as Error & { code: string }; + err.code = 'mds::invalid_options'; + return err; +} + +/** + * Methods for which `basePath` must be rejected with a purpose-built error rather + * than the generic "unknown option key" form. + * + * Each entry maps a method name to the factory that creates its rejection. + * Using a Map (not a Set) means a new method can only be added when an error + * factory is simultaneously provided — widening without a handler is a TypeScript + * error on the Map literal, not a silent omission. + * + * The wrapper emits this error BEFORE dispatch (via {@link getBasePathError}); + * no backend ever receives a `basePath` on these methods. `assertKnownKeys` skips + * the generic unknown-key path for `basePath` on these methods because they are + * absent from METHOD_KEYS for those surfaces. * * KNOWN RESIDUAL (OD-5): napi also has purpose-built `basePath` messages for * `lintFile` ("not valid for lintFile; …") and `lintVirtual`, but those two methods - * are deliberately NOT in this set. The wrapper intercepts them first and emits the + * are deliberately NOT in this map. The wrapper intercepts them first and emits the * generic `unknown option key "basePath"; recognised keys are: vars, rules` form, so * the wrapper and napi messages diverge for that one input. This is intentional and * locked in by U-OV-7 and U-OV-13; the generic message still names the offending key - * and is a hard error either way. Widening this set would change those two messages - * and is deferred rather than bundled into #180/#215/#213. + * and is a hard error either way. Widening this map would change those messages and + * is deferred rather than bundled into #180/#215/#213. */ -const BASEPATH_PASSTHROUGH: ReadonlySet = new Set([ - 'compileFile', - 'checkFile', -]); +const BASEPATH_PASSTHROUGH: ReadonlyMap Error & { code: string }> = new Map([ + ['compileFile', makeFileBasePathError], + ['checkFile', makeFileBasePathError], +] as const); + +/** + * Return the purpose-built `basePath` error for `method` if `options.basePath` + * is non-null and `method` is in {@link BASEPATH_PASSTHROUGH}, or `undefined` + * otherwise. + * + * Callers throw the returned error synchronously — the same channel as + * {@link assertKnownKeys} — so that `try { compileFile(f, opts) } catch` captures + * both unknown-key and basePath errors (U-OV-32 / U-OV-33). The structural benefit: + * adding a new method to BASEPATH_PASSTHROUGH requires providing a factory via the + * Map literal, ensuring the handler is co-located with the set membership. + * + * @param options - The caller-supplied options object (non-null). + * @param method - Public method name. + */ +export function getBasePathError( + options: object, + method: MethodName, +): (Error & { code: string }) | undefined { + const factory = BASEPATH_PASSTHROUGH.get(method); + if (!factory) return undefined; + const basePath = (options as Record)['basePath']; + return basePath != null ? factory() : undefined; +} // ── Main validator ───────────────────────────────────────────────────────────── @@ -125,9 +178,9 @@ const BASEPATH_PASSTHROUGH: ReadonlySet = new Set([ * Uses the same message format as `format_unknown_keys_error` in * `crates/mds-core/src/options.rs`. For all methods except `compileFile` and * `checkFile`, the wrapper and napi produce byte-identical messages for the same - * unknown key. For `compileFile` and `checkFile`, `basePath` is not intercepted - * here — it is passed through so the backend can emit its own purpose-built error - * (issue #74). + * unknown key. For `compileFile` and `checkFile`, `basePath` is absent from + * METHOD_KEYS for those surfaces and is skipped here — the caller uses + * {@link getBasePathError} to emit the purpose-built rejection (issue #74). * * The `method` parameter is typed as the {@link MethodName} literal union — * passing an unrecognised method name is a compile-time error, not a silent no-op. @@ -143,8 +196,9 @@ export function assertKnownKeys(options: object, method: MethodName): void { if (!Object.prototype.hasOwnProperty.call(METHOD_KEYS, method)) return; const known = METHOD_KEYS[method]; const unknowns = Object.keys(options).filter((k) => { - // basePath is passed through for file-based methods so the backend emits its - // own purpose-built error rather than this generic rejection (issue #74). + // basePath is absent from METHOD_KEYS for file-surface methods; skip it here + // so the generic "unknown option key" path is not taken. The caller uses + // getBasePathError() to emit the purpose-built rejection (issue #74). if (BASEPATH_PASSTHROUGH.has(method) && k === 'basePath') return false; return !known.includes(k); }); @@ -162,75 +216,44 @@ export function assertKnownKeys(options: object, method: MethodName): void { throw err; } -// ── Per-surface option builders ──────────────────────────────────────────────── -// -// D-TS-03 / D-TS-05: four typed builders, one per method-surface combination. -// Each builder returns `undefined` when no options are set (preserves the -// backend's fast path for no-options calls — avoids allocating an empty object -// on every invocation). The per-surface split ensures that adding a new field to -// a string-surface type (e.g. `CompileOptions`) cannot accidentally appear in the -// file-surface options forwarded to the backend. - -/** - * Return a new object containing only the keys from `keys` whose values in - * `src` are non-null/undefined. Returns `undefined` when `src` is nullish or - * every selected value is absent — so callers can use `result ?? undefined` - * without allocating an empty object on every invocation (backend fast path). - * - * @internal - */ -function pickDefined( - src: T | null | undefined, - keys: readonly (keyof T)[], -): Partial | undefined { - if (src == null) return undefined; - const defined = keys.filter(k => src[k] != null); - if (defined.length === 0) return undefined; - return Object.fromEntries(defined.map(k => [k, src[k]])) as Partial; -} - -/** - * Build options for the string-source compile surface. - * Picks `basePath`, `vars`, `sourceMap`, and `sourcesContent` from `CompileOptions`. - * - * D-TS-03: used by the native backend's `compile` method and by the WASM - * backend's `compileOpts()` wrapper (after the WASM basePath guard fires). - */ -export function compileSrcOpt(options?: CompileOptions): Partial | undefined { - return pickDefined(options, ['basePath', 'vars', 'sourceMap', 'sourcesContent']); -} +// ── Option forwarding ────────────────────────────────────────────────────────── /** - * Build options for the string-source check surface. - * Picks `basePath` and `vars` from `CheckOptions`. + * Forward `options` to the backend, keeping only the keys listed in + * {@link METHOD_KEYS} for `method`. * - * D-TS-03: used by the native backend's `check` method. The WASM backend - * guards against `basePath` before calling `checkOpts()`. - */ -export function checkSrcOpt(options?: CheckOptions): Partial | undefined { - return pickDefined(options, ['basePath', 'vars']); -} - -/** - * Build options for the file-surface compile path. - * Picks `vars`, `sourceMap`, and `sourcesContent` from `FileOptions`. - * `basePath` is intentionally absent (D-TS-02). + * This is the **single authoritative forwarding path** for all seven public + * methods. When METHOD_KEYS is updated (e.g. a new key is added to a method's + * option interface and its {@link keysOf} witness is updated), forwarding picks + * it up automatically with no additional edit. The previous design used + * independent per-surface builders (`compileSrcOpt`, `checkSrcOpt`, etc.) that + * each hardcoded their own key array — those could drift from METHOD_KEYS without + * a compile error, which is the root shape of PF-004 / #180. * - * D-TS-03: used by the native backend's `compileFile` method and internally by - * the WASM backend's `compileOpts()` / `fileOpts()` helpers (which deal with - * `filename` and `modules` separately). - */ -export function fileCompileOpt(options?: FileOptions): Partial | undefined { - return pickDefined(options, ['vars', 'sourceMap', 'sourcesContent']); -} - -/** - * Build options for the file-surface check path. - * Picks only `vars` from `CheckFileOptions`. - * `basePath`, `sourceMap`, and `sourcesContent` are all intentionally absent. + * Returns `undefined` when `options` is nullish or every accepted key is absent, + * preserving the backend no-options fast path (avoids allocating empty objects + * on every call). D-TS-03 / D-TS-05. * - * D-TS-03: used by the native backend's `checkFile` method. + * @param options - Caller-supplied options (null/undefined treated as "no options"). + * @param method - Public method name; selects the key list from METHOD_KEYS. */ -export function fileCheckOpt(options?: CheckFileOptions): { vars: Record } | undefined { - return options?.vars != null ? { vars: options.vars } : undefined; +export function forwardOpts( + options: object | null | undefined, + method: MethodName, +): Record | undefined { + if (options == null) return undefined; + const keys = METHOD_KEYS[method]; + const src = options as Record; + const out: Record = {}; + let any = false; + for (const k of keys) { + const v = src[k]; + if (v != null) { + // unavoidable: accumulating into Record loses per-key value + // types, but callers cast to their concrete options type at the call site. + out[k] = v; + any = true; + } + } + return any ? out : undefined; } From 225d5a6dcdbb7afa0e9b34fe1c6d3c0dd4e91993 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:23:23 +0300 Subject: [PATCH 13/26] fix(mds): remove unused forwardOpts import and update stale comment in node.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The forwardOpts import was added but never called directly in node.ts — all option forwarding in node.ts is handled by the backend adapters (native.ts / wasm.ts). Remove to keep the zero-warnings policy. Also update a comment in the WASM-path checkFile that still referenced the deleted fileCompileOpt helper; now names fileOpts and forwardOpts correctly. --- packages/mds/src/node.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/mds/src/node.ts b/packages/mds/src/node.ts index 62860f6c..c553e5b8 100644 --- a/packages/mds/src/node.ts +++ b/packages/mds/src/node.ts @@ -17,7 +17,7 @@ import { assertResultShape } from './backend/contract.js'; import { initWasmNode, createWasmBackend, fileOpts } from './backend/wasm.js'; import type { WasmModule } from './backend/wasm.js'; import { buildModulesMap } from './util/module-scanner.js'; -import { assertKnownKeys, forwardOpts, getBasePathError } from './util/options.js'; +import { assertKnownKeys, getBasePathError } from './util/options.js'; // Read MDS_BACKEND at module scope — sync, deterministic, no I/O. const rawBackend = process.env['MDS_BACKEND']; @@ -100,8 +100,8 @@ function wrapWithFileOps( async checkFile(path: string, options?: CheckFileOptions): Promise { // CheckFileOptions is a structural subset of FileOptions (only vars, no // sourceMap/sourcesContent), so the cast is safe: prepareFileArgs calls - // fileCompileOpt which only picks defined keys; the absent fields resolve as - // undefined and are not included in the returned opts object. + // fileOpts which uses forwardOpts over METHOD_KEYS.compileFile; the absent + // fields resolve as undefined and are excluded by forwardOpts's != null check. const { source, opts } = await prepareFileArgs(path, options as FileOptions | undefined); const result: unknown = wasmModule.check(source, opts); assertResultShape(result, 'check'); @@ -128,6 +128,12 @@ function wrapWithFileOps( // Build a copy of modules without the entry (lint() inserts it separately). const extraModules: Record = { ...modules }; delete extraModules[entryFilename]; + // forwardOpts uses METHOD_KEYS.lintFile (vars, rules) as the single key source. + // filename and extraModules are WASM-internal keys absent from METHOD_KEYS. + const forwarded = forwardOpts(options, 'lintFile') as { + vars?: Record; + rules?: Record; + } | undefined; const lintOpts: { filename: string; modules?: Record; @@ -135,8 +141,8 @@ function wrapWithFileOps( rules?: Record; } = { filename: entryFilename }; if (Object.keys(extraModules).length > 0) lintOpts.modules = extraModules; - if (options?.vars != null) lintOpts.vars = options.vars; - if (options?.rules != null) lintOpts.rules = options.rules; + if (forwarded?.vars != null) lintOpts.vars = forwarded.vars; + if (forwarded?.rules != null) lintOpts.rules = forwarded.rules; const result: unknown = wasmModule.lint(entrySource, lintOpts); assertResultShape(result, 'lint'); return result as LintResult; From cfd84d5f1f8dc7c99f883ac52887ece7bfdceaec Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:24:51 +0300 Subject: [PATCH 14/26] docs(mds): update stale compileSrcOpt/fileCompileOpt references in U-OV-34 The prior commit replaced per-surface option builders (compileSrcOpt, fileCompileOpt) with a single forwardOpts helper, but the explanatory comment in the U-OV-34 test block still referenced the old names. Update the comment to reference forwardOpts to match the implementation. --- packages/mds/__test__/options-validation.spec.mjs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/mds/__test__/options-validation.spec.mjs b/packages/mds/__test__/options-validation.spec.mjs index a934a2d0..2d21af00 100644 --- a/packages/mds/__test__/options-validation.spec.mjs +++ b/packages/mds/__test__/options-validation.spec.mjs @@ -819,12 +819,12 @@ describe('options-validation', () => { // Test-plan item 17: "a unit assertion that the compile-options builder returns // undefined (strictly, not {}) … assert compileOpts(undefined) === compileOpts({}) // by object identity". The WASM backend's compileOpts() fast path at wasm.ts:363-365 - // fires only when fileCompileOpt (called internally by compileOpts) returns undefined. - // If compileSrcOpt or fileCompileOpt returned {} for empty input, every compile call - // would allocate a new object rather than reusing DEFAULT_COMPILE_OPTS — the - // regression would be invisible to U-PF1/U-PF2 (too much headroom) and no other - // test pins this invariant (avoids PF-013: absence of an allocation test cannot - // detect a silent allocation regression). + // fires only when forwardOpts (called internally by compileOpts) returns undefined. + // If forwardOpts returned {} for empty input, every compile call would allocate a + // new object rather than reusing DEFAULT_COMPILE_OPTS — the regression would be + // invisible to U-PF1/U-PF2 (too much headroom) and no other test pins this + // invariant (avoids PF-013: absence of an allocation test cannot detect a silent + // allocation regression). test('U-OV-34: forwardOpts returns undefined for empty compile input, and the WASM backend reuses DEFAULT_COMPILE_OPTS by identity (AC-P3-22)', async () => { // Part 1: forwardOpts returns undefined (strictly) for empty input — {} would From 2c239f72ea97baca508231c28ed268edb505eb21 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:26:05 +0300 Subject: [PATCH 15/26] fix(mds): restore forwardOpts import erroneously removed in prior commit Commit 225d5a6 removed the `forwardOpts` import from node.ts, but `wrapWithFileOps.lintFile` calls `forwardOpts(options, 'lintFile')` to forward vars/rules through METHOD_KEYS rather than accessing them directly. Restores the import so the build compiles cleanly. --- packages/mds/src/node.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mds/src/node.ts b/packages/mds/src/node.ts index c553e5b8..b442cd70 100644 --- a/packages/mds/src/node.ts +++ b/packages/mds/src/node.ts @@ -17,7 +17,7 @@ import { assertResultShape } from './backend/contract.js'; import { initWasmNode, createWasmBackend, fileOpts } from './backend/wasm.js'; import type { WasmModule } from './backend/wasm.js'; import { buildModulesMap } from './util/module-scanner.js'; -import { assertKnownKeys, getBasePathError } from './util/options.js'; +import { assertKnownKeys, forwardOpts, getBasePathError } from './util/options.js'; // Read MDS_BACKEND at module scope — sync, deterministic, no I/O. const rawBackend = process.env['MDS_BACKEND']; From 2d9207f1cbcd9fef1a620001df943326743edda3 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:28:42 +0300 Subject: [PATCH 16/26] docs(mds): restore truncated LINT_RULE_NAMES comment in node.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior commit (50b37c9) removed the entire explanatory comment above `export { isMdsError, LINT_RULE_NAMES }` when it should have only dropped the final sentence ("The browser entry gains it with the browser lint surface; today it has no lint API to configure.") which became false once browser.ts gained lint/lintVirtual/LINT_RULE_NAMES exports. The first three lines — explaining why LINT_RULE_NAMES must be re-exported from node.ts/browser.ts rather than index.ts (the exports map never routes to dist/index.js, so index.ts is unreachable for consumers) — remain accurate and are restored here. Co-Authored-By: Claude --- packages/mds/src/node.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/mds/src/node.ts b/packages/mds/src/node.ts index b442cd70..684aedb5 100644 --- a/packages/mds/src/node.ts +++ b/packages/mds/src/node.ts @@ -330,6 +330,10 @@ export function getBackend(): BackendType { return assertReady().getBackend(); } +// `LINT_RULE_NAMES` is exported here, not only from `index.ts`: the package +// `exports` map resolves `@mdscript/mds` to `dist/node.js` (Node) or +// `dist/browser.js`, and never to `dist/index.js` — a value re-exported only +// from `index.ts` is unreachable for consumers. export { isMdsError, LINT_RULE_NAMES } from './types.js'; export type { BackendType, From 07e0a642b3a57f0f1a60f13c29987a522dd7284f Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:28:46 +0300 Subject: [PATCH 17/26] docs(mds): replace stale per-surface builder names in AC-P3-05 test comments The MAINTENANCE NOTE and positive-control description in the forwarding-parity test block still referred to the deleted per-surface helpers (pickDefined, lintOpt, lintFileOpt). Update both comments to reference forwardOpts, which is now the single forwarding path keyed on METHOD_KEYS. --- .../mds/__test__/options-validation.spec.mjs | 56 +++++++++++++------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/packages/mds/__test__/options-validation.spec.mjs b/packages/mds/__test__/options-validation.spec.mjs index 2d21af00..80eae1b4 100644 --- a/packages/mds/__test__/options-validation.spec.mjs +++ b/packages/mds/__test__/options-validation.spec.mjs @@ -436,7 +436,11 @@ describe('options-validation', () => { // ── forwarding drift guard: spy addon (AC-P3-04 / AC-P3-05) ────────────── test('U-OV-24: all 7 methods forward exactly the accepted keys to the backend (AC-P3-04, AC-P3-05)', async () => { - requireNativeAddon(); // hard-fail without addon + // No requireNativeAddon() here: the backend under test is a spy object injected + // directly into createNativeBackend(), making this test fully environment-independent. + // Adding requireNativeAddon() would couple the highest-value drift-guard test to + // native-addon availability, which is the opposite of what a forwarding-parity + // test should require. const { createNativeBackend } = await import('../dist/backend/native.js'); // Minimal valid result shapes for each assertResultShape kind. @@ -464,8 +468,8 @@ describe('options-validation', () => { // // MAINTENANCE NOTE: if a new option key is added to any option interface, // add a non-null entry here — without it, fullOpts() would supply `undefined` - // for the new key, and pickDefined / lintOpt / lintFileOpt would correctly - // omit it, making the deepStrictEqual pass vacuously for that key. + // for the new key, and forwardOpts would correctly omit it (its != null check + // skips absent keys), making the deepStrictEqual pass vacuously for that key. const KEY_VALUES = { basePath: '/some/base/path', vars: { k: 1 }, @@ -476,11 +480,11 @@ describe('options-validation', () => { // Build an options object with ALL keys that METHOD_KEYS[method] accepts, // each with a distinguishable value. This is the AC-P3-05 positive control: - // if a key is added to METHOD_KEYS (accepted at validation) but omitted from - // the forwarding builder (pickDefined / lintOpt / lintFileOpt), expected still - // includes that key, so deepStrictEqual catches the drift — reproducing the - // #180 bug class at the point of introduction rather than at runtime in - // production (avoids PF-013: a hand-typed expected cannot detect this drift). + // if a key is added to METHOD_KEYS (accepted at validation) but not forwarded + // by forwardOpts (e.g. accidentally excluded), expected still includes that key + // so deepStrictEqual catches the drift — reproducing the #180 bug class at the + // point of introduction rather than at runtime in production (avoids PF-013: + // a hand-typed expected cannot detect this drift). function fullOpts(methodName) { const keys = METHOD_KEYS[methodName] ?? []; const obj = {}; @@ -620,27 +624,43 @@ describe('options-validation', () => { try { await fn({ basePath: undefined }); } catch { nativeThrew = true; } // WASM path via subprocess. init() must be awaited before any method. + // The subprocess emits getBackend() to stdout so the parent can verify the + // WASM backend was actually selected — MDS_BACKEND=wasm is advisory and + // node.ts only console.warns on unrecognised values, so a drift in the + // env-var name or accepted values would silently fall back to native. + // Capturing and asserting the backend string here guards against that. const { execFileSync } = await import('node:child_process'); let wasmThrew = false; + let wasmBackendStr = ''; + const script = [ + `import { init, getBackend, ${name} } from './dist/node.js';`, + `const SRC = 'Hello\\n';`, + `const F = ${JSON.stringify(file)};`, + `const OPTS = { basePath: undefined };`, + `await init();`, + `process.stdout.write(getBackend());`, + `try { ${call}; process.exit(0); } catch { process.exit(1); }`, + ].join('\n'); try { - const script = [ - `import { init, ${name} } from './dist/node.js';`, - `const SRC = 'Hello\\n';`, - `const F = ${JSON.stringify(file)};`, - `const OPTS = { basePath: undefined };`, - `await init();`, - `try { ${call}; process.exit(0); } catch { process.exit(1); }`, - ].join('\n'); - execFileSync(process.execPath, ['--input-type=module'], { + wasmBackendStr = execFileSync(process.execPath, ['--input-type=module'], { input: script, cwd: PKG_DIR, env: { ...process.env, MDS_BACKEND: 'wasm' }, timeout: 15000, + encoding: 'utf8', }); - } catch { + } catch (e) { wasmThrew = true; + // stdout is populated even when exit code is non-zero (the method threw) + wasmBackendStr = typeof e.stdout === 'string' ? e.stdout : ''; } + assert.strictEqual( + wasmBackendStr, + 'wasm', + `${name}: WASM subprocess must use the wasm backend (getBackend() returned ` + + `${JSON.stringify(wasmBackendStr)}) — MDS_BACKEND=wasm may be unrecognised`, + ); assert.strictEqual( nativeThrew, wasmThrew, From c98815c311f6f392e500c9dc95acb22b3da71187 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 21:36:58 +0300 Subject: [PATCH 18/26] refactor(mds): route remaining option-building through forwardOpts for METHOD_KEYS consistency wasm.ts checkOpts still accessed options?.vars directly instead of going through forwardOpts, making it inconsistent with the sibling compileOpts (which uses forwardOpts('compileFile')). If CheckOptions gained a new key in METHOD_KEYS.checkFile the drift would be silent. node.ts lintFile (WASM path) called forwardOpts then manually copied individual keys (forwarded?.vars, forwarded?.rules) rather than spreading forwarded. A new key added to METHOD_KEYS.lintFile would be silently dropped by the manual copying. Fix: - checkOpts now uses forwardOpts(options, 'checkFile'), matching the compileOpts pattern, so METHOD_KEYS.checkFile is the single source of truth (avoids PF-004) - lintFile WASM path spreads ...forwarded directly, matching lint/lintVirtual No observable behaviour change: forwardOpts already filters to the same key set that the manual code extracted. Identical results on all existing test inputs. 300/300 tests pass; tsc exits 0; source-hygiene gate clean. Co-Authored-By: Claude --- packages/mds/src/backend/wasm.ts | 11 ++++++++--- packages/mds/src/node.ts | 16 +++++++--------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/mds/src/backend/wasm.ts b/packages/mds/src/backend/wasm.ts index dcf4c93c..0c4a6fc0 100644 --- a/packages/mds/src/backend/wasm.ts +++ b/packages/mds/src/backend/wasm.ts @@ -375,9 +375,14 @@ function compileOpts( function checkOpts( options?: CheckOptions, ): { filename: string; modules: Record; vars?: Record } { - const vars = options?.vars; - return vars != null - ? { filename: DEFAULT_COMPILE_OPTS.filename, modules: DEFAULT_COMPILE_OPTS.modules, vars } + // D-TS-06: forward only the checkFile-surface keys (vars only) via METHOD_KEYS.checkFile, + // mirroring compileOpts (which uses 'compileFile'). basePath is excluded from + // METHOD_KEYS.checkFile and the caller guards against it before reaching here; + // forwardOpts cannot include it. This makes METHOD_KEYS the single source of truth + // for what is forwarded from CheckOptions to the WASM module (avoids PF-004). + const extra = forwardOpts(options, 'checkFile'); + return extra != null + ? { filename: DEFAULT_COMPILE_OPTS.filename, modules: DEFAULT_COMPILE_OPTS.modules, ...extra } : DEFAULT_COMPILE_OPTS; } diff --git a/packages/mds/src/node.ts b/packages/mds/src/node.ts index 684aedb5..5d4f5576 100644 --- a/packages/mds/src/node.ts +++ b/packages/mds/src/node.ts @@ -130,19 +130,17 @@ function wrapWithFileOps( delete extraModules[entryFilename]; // forwardOpts uses METHOD_KEYS.lintFile (vars, rules) as the single key source. // filename and extraModules are WASM-internal keys absent from METHOD_KEYS. + // Spread forwarded directly so any future key added to METHOD_KEYS.lintFile is + // picked up automatically — no per-key manual copying that could drift (avoids PF-004). const forwarded = forwardOpts(options, 'lintFile') as { vars?: Record; rules?: Record; } | undefined; - const lintOpts: { - filename: string; - modules?: Record; - vars?: Record; - rules?: Record; - } = { filename: entryFilename }; - if (Object.keys(extraModules).length > 0) lintOpts.modules = extraModules; - if (forwarded?.vars != null) lintOpts.vars = forwarded.vars; - if (forwarded?.rules != null) lintOpts.rules = forwarded.rules; + const lintOpts = { + filename: entryFilename, + ...(Object.keys(extraModules).length > 0 ? { modules: extraModules } : undefined), + ...forwarded, + }; const result: unknown = wasmModule.lint(entrySource, lintOpts); assertResultShape(result, 'lint'); return result as LintResult; From 8900c5cf6d96b9f44605af40c9ba3cc71523f26b Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 22:06:11 +0300 Subject: [PATCH 19/26] fix(mds): derive checkOpts forwarding from METHOD_KEYS.check not checkFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit checkOpts(options?: CheckOptions) was calling forwardOpts(options, 'checkFile'), which uses METHOD_KEYS.checkFile — derived from CheckFileOptions, a different interface from the function's own parameter type. This recreated the PF-004 /#180 silent-drop topology on the WASM check path: a future key added to CheckOptions would update METHOD_KEYS.check (via the keysOf witness) but forwardOpts would still select METHOD_KEYS.checkFile, silently dropping the new key while the native backend honoured it. Fix: call forwardOpts(options, 'check') so the key list is derived from CheckOptions — the same interface as the parameter. METHOD_KEYS.check includes basePath, but forwardOpts only includes keys whose value is != null; the call site already throws mds::invalid_options before reaching checkOpts whenever basePath is non-null, so basePath is never included in the forwarded object. Applies PF-004. Verified by U-WB23 (basePath rejection) and U-WB25 (basePath: undefined accepted) — both continue to pass. Co-Authored-By: Claude --- packages/mds/src/backend/wasm.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/mds/src/backend/wasm.ts b/packages/mds/src/backend/wasm.ts index 0c4a6fc0..e766f25f 100644 --- a/packages/mds/src/backend/wasm.ts +++ b/packages/mds/src/backend/wasm.ts @@ -375,12 +375,16 @@ function compileOpts( function checkOpts( options?: CheckOptions, ): { filename: string; modules: Record; vars?: Record } { - // D-TS-06: forward only the checkFile-surface keys (vars only) via METHOD_KEYS.checkFile, - // mirroring compileOpts (which uses 'compileFile'). basePath is excluded from - // METHOD_KEYS.checkFile and the caller guards against it before reaching here; - // forwardOpts cannot include it. This makes METHOD_KEYS the single source of truth - // for what is forwarded from CheckOptions to the WASM module (avoids PF-004). - const extra = forwardOpts(options, 'checkFile'); + // D-TS-06: forward only the check-surface keys via METHOD_KEYS.check, which is + // derived from CheckOptions — the same interface as this function's parameter. + // METHOD_KEYS.check includes basePath, but forwardOpts only forwards keys whose + // value is != null; the caller already threw when basePath was non-null, so + // basePath is never included in the forwarded object. Using 'check' (not + // 'checkFile') keeps the key list tied to CheckOptions so a future key added to + // that interface automatically propagates here — eliminating the PF-004 / #180 + // topology where METHOD_KEYS.check gained a key but this call forwarded via a + // different surface's list and silently dropped it. + const extra = forwardOpts(options, 'check'); return extra != null ? { filename: DEFAULT_COMPILE_OPTS.filename, modules: DEFAULT_COMPILE_OPTS.modules, ...extra } : DEFAULT_COMPILE_OPTS; From f8629e9245c7caf41c67c6526f6ca91ee9cf051f Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 22:06:42 +0300 Subject: [PATCH 20/26] docs(changelog): correct false source-compatibility and sync-throw claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five confirmed review findings, all rooted in a sequencing issue: commit 891db04 wrote CHANGELOG and README before commit 3a9e2be added `basePath?: never` to FileOptions, CheckFileOptions, and LintFileOptions — leaving the docs contradicting the shipped types. Changes: - CHANGELOG.md: "return a rejected promise" → "throw synchronously" for compileFile/checkFile basePath rejection (node.ts:280,299 are sync throws). - CHANGELOG.md migration paragraph: remove "runtime-only break" and "TypeScript does not flag it"; add sync-throw / try-catch guidance and pointer to compile-time compatibility notes below. - CHANGELOG.md FileOptions compatibility: replace "source-compatible" with "compile-time break"; add shared-variable migration with destructuring example. basePath?: never means CompileOptions is not assignable to FileOptions (verified by consumer-node.ts:72-73 @ts-expect-error, ADR-009). - CHANGELOG.md CheckFileOptions compatibility: same correction. CheckOptions is not assignable to CheckFileOptions for the same reason. - README.md: "surfaces as a rejected promise" → "throws synchronously"; add try/catch guidance consistent with node.ts JSDoc. - types.ts: clarify CheckFileOptions JSDoc to explain the compile-time break that `basePath?: never` intentionally introduces. The LintFileOptions section already correctly describes this as a compile-time break and is unchanged. Co-Authored-By: Claude --- CHANGELOG.md | 62 +++++++++++++++++++++++++++------------ packages/mds/README.md | 13 ++++++-- packages/mds/src/types.ts | 7 +++-- 3 files changed, 59 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e9aa455..a0ab0e8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,14 +53,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `compileFile(path, options?)` and `checkFile(path, options?)` previously accepted a `basePath` option and silently discarded it — the option passed the unknown-key validator but was dropped before the backend was reached, so file resolution always -used the directory containing the path. Both functions now return a rejected promise +used the directory containing the path. Both functions now **throw synchronously** (`Error { code: 'mds::invalid_options' }`) when `basePath` is non-null. The base directory for file-based operations is always derived from the file path itself. **Migration:** remove `basePath` from any options object passed to `compileFile` or -`checkFile`. This is a **runtime-only** break — TypeScript does not flag it at -compile time when you pass a variable whose inferred type contains `basePath`. -Audit call sites explicitly. +`checkFile`. This throw is **synchronous** — `.catch()` on the returned promise does +not receive it; wrap the call in `try/catch`. This is also a **compile-time break** +when a variable typed as `CompileOptions` or `CheckOptions` is passed to these +functions — see the compatibility notes below. Audit all call sites. #### `FileOptions` no longer extends `CompileOptions` (#213) @@ -70,14 +71,19 @@ Audit call sites explicitly. operations. `FileOptions` is now a standalone interface with its own `vars`, `sourceMap`, and `sourcesContent` fields. -**Compatibility:** this change is **source-compatible** for all previously-compiling -code. Before this PR, `CompileOptions` had no `basePath` field, so -`FileOptions extends CompileOptions` already resolved to -`{ vars?, sourceMap?, sourcesContent? }` — the same shape as the new standalone -`FileOptions`. Code that assigns a `CompileOptions` value to a `FileOptions` variable -(or vice versa) continues to compile. The runtime break is the `compileFile`/`checkFile` -rejection documented above, which TypeScript does not catch — see that entry for -the migration. +**Compatibility:** this change is a **compile-time break** for code that passes a +`CompileOptions`-typed variable to `compileFile`. After this PR, `CompileOptions` +carries `basePath?: string` while `FileOptions` declares `basePath?: never`; +TypeScript reports `"Types of property 'basePath' are incompatible"` at any such +assignment or call. Code that never reuses a string-surface options variable for +file operations compiles without changes. + +**Migration for shared variables:** retype the variable as `FileOptions`, or +destructure only the accepted fields: +```ts +const { vars, sourceMap, sourcesContent } = compileOpts; +compileFile(path, { vars, sourceMap, sourcesContent }); +``` #### `checkFile` parameter type changed from `CheckOptions` to `CheckFileOptions` (#213) @@ -86,12 +92,32 @@ the migration. the parameter is now typed as `CheckFileOptions` — a new interface with only `vars?: Record`. -**Compatibility:** this type narrowing is **source-compatible** for all -previously-compiling code. Before this PR, `CheckOptions` had no `basePath` field, so -it was structurally equivalent to the new `CheckFileOptions`. A `CheckOptions`-typed -variable without `basePath` still satisfies the `CheckFileOptions` parameter. The -runtime break is the `compileFile`/`checkFile` rejection documented above, which -TypeScript does not catch — see that entry for the migration. +**Compatibility:** this type narrowing is a **compile-time break** for code that +passes a `CheckOptions`-typed variable to `checkFile`. `CheckOptions` carries +`basePath?: string` while `CheckFileOptions` declares `basePath?: never`; TypeScript +reports `"Types of property 'basePath' are incompatible"` at any such call. Code +that never reuses a string-surface variable for `checkFile` compiles without changes. + +**Migration for shared variables:** retype the variable as `CheckFileOptions`, or +restrict it to `{ vars?: Record }` at the call site. + +#### `LintFileOptions` gained `basePath?: never` (#213) + +`LintFileOptions` (used by `lintFile` and `lintVirtual`) previously had the shape +`{ vars?, rules? }`. It now declares `basePath?: never`. + +**Compatibility:** this is a **compile-time break** for code that assigns a variable +whose inferred type includes a `basePath` field to a `LintFileOptions`-typed slot. +For example, passing a `LintOptions`-typed variable directly to `lintFile` or +`lintVirtual` now fails with `TS2322` — `LintOptions.basePath` is `string | undefined` +which is not assignable to `never`. Code that passes a fresh object literal without +`basePath`, or a variable that was already typed as `{ vars?, rules? }`, continues to +compile unchanged. + +**Migration:** at each `lintFile` / `lintVirtual` call site that passes a +`LintOptions`-typed variable, either extract a narrowed copy +(`const { basePath: _unused, ...fileOpts } = opts`) or redeclare the variable as +`LintFileOptions` when `basePath` was never meaningful there. #### WASM backend rejects `basePath` on string-surface methods (#180) diff --git a/packages/mds/README.md b/packages/mds/README.md index d59dad92..5877bcd5 100644 --- a/packages/mds/README.md +++ b/packages/mds/README.md @@ -139,17 +139,21 @@ interface CompileOptions extends CheckOptions { // FileOptions — accepted by compileFile() // basePath is NOT accepted: the base directory is derived from the file path. +// basePath?: never blocks assigning a CompileOptions variable to this type (TS2322). interface FileOptions { vars?: Record; sourceMap?: boolean; sourcesContent?: boolean; + basePath?: never; // not accepted; present to produce a compile-time error when a string-surface variable is passed } // CheckFileOptions — accepted by checkFile() // basePath is NOT accepted (base directory from file path). +// basePath?: never blocks assigning a CheckOptions variable to this type (TS2322). // Source-map options are NOT accepted; passing them throws mds::invalid_options. interface CheckFileOptions { vars?: Record; + basePath?: never; // not accepted; present to produce a compile-time error when a string-surface variable is passed } // LintOptions — accepted by lint() (string-source) @@ -167,9 +171,11 @@ interface LintOptions { // LintFileOptions — accepted by lintFile() and lintVirtual() // basePath is NOT accepted: lintFile derives the base directory from the file path; // lintVirtual resolves imports against the caller-supplied module map, not the filesystem. +// basePath?: never blocks assigning a LintOptions variable to this type (TS2322). interface LintFileOptions { vars?: Record; rules?: Record; + basePath?: never; // not accepted; present to produce a compile-time error when a string-surface variable is passed } // InitOptions @@ -181,9 +187,10 @@ interface InitOptions { **Unknown-option rejection:** passing an unrecognised key to a public method throws `Error { code: 'mds::invalid_options' }` before calling the backend; the error names the offending key(s) and lists the accepted keys. Exception: passing `basePath` to a -file-path method (`compileFile` or `checkFile`) surfaces as a rejected promise with a -purpose-built message rather than a synchronous throw, and the message does not include -an accepted-keys list. +file-path method (`compileFile` or `checkFile`) **throws synchronously** with a +purpose-built message (same channel as unknown-key rejection); the message does not +include an accepted-keys list. `.catch()` on the returned promise does not receive +this error — use `try/catch` around the call. **Source maps:** for string-source compiles (`compile`) `sources[0]` in the generated map is `"input.mds"`. For stdin builds via the CLI it is `""`. diff --git a/packages/mds/src/types.ts b/packages/mds/src/types.ts index 5b91a1c5..fd72e970 100644 --- a/packages/mds/src/types.ts +++ b/packages/mds/src/types.ts @@ -162,8 +162,11 @@ export interface FileOptions { /** * Options for file-based check-only operations. * - * Mirrors {@link LintFileOptions}: `basePath` is absent because the base - * directory is derived from the file path. Only `vars` is forwarded to the backend. + * `basePath` is not accepted on file operations: the base directory is derived + * from the file path. The `basePath?: never` declaration rejects assignment from + * any variable whose inferred type carries `basePath` (e.g. a `CheckOptions` + * value), producing a compile-time error rather than a runtime surprise. + * Only `vars` is forwarded to the backend. */ export interface CheckFileOptions { /** Runtime variables made available for interpolation in the template. */ From 931ab792bc6ecef51ddc9bc0967b97d80a598565 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 22:07:17 +0300 Subject: [PATCH 21/26] docs(mds): fix stale U-C7 comment in compile.spec.mjs referencing deleted symbols MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit compileSrcOpt, fileCompileOpt, and pickDefined were removed by commit 3a9e2be and replaced with forwardOpts. The U-C7 comment was rewritten in that same commit to reference those symbols, which were then immediately absent. Sibling cleanup commits 7ef8369 and cfd84d5 fixed this staleness class in options-validation.spec.mjs but missed this site. Rewrite the comment to name forwardOpts, which filters via != null over METHOD_KEYS — the current and only forwarding path for all seven public methods. Co-Authored-By: Claude --- packages/mds/__test__/compile.spec.mjs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/mds/__test__/compile.spec.mjs b/packages/mds/__test__/compile.spec.mjs index 632e8bcb..9305bdea 100644 --- a/packages/mds/__test__/compile.spec.mjs +++ b/packages/mds/__test__/compile.spec.mjs @@ -56,10 +56,9 @@ describe('compile', () => { }); test('U-C7: compile with null vars produces identical output to no-vars compile', () => { - // null vars must be treated as absent — compileSrcOpt (native path) and - // fileCompileOpt via compileOpts (WASM path) both use pickDefined, which - // filters via != null, so both null and undefined are omitted from the - // options forwarded to the backend. + // null vars must be treated as absent — forwardOpts filters via != null over + // METHOD_KEYS, so both null and undefined vars are omitted from the options + // forwarded to the backend. const source = 'Hello World!\n'; const withNull = compile(source, { vars: null }); const withoutVars = compile(source); From a25bfdc1a3eff4fd2933787cadb0cf0401e4e2a1 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 22:10:47 +0300 Subject: [PATCH 22/26] =?UTF-8?q?fix(mds):=20address=20PR3=20review=20find?= =?UTF-8?q?ings=20=E2=80=94=20defense-in-depth,=20spread=20order,=20commen?= =?UTF-8?q?t=20hygiene,=20and=20drop=20backend=20interfaces=20from=20publi?= =?UTF-8?q?c=20surface?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five confirmed findings from PR review: [medium] Remove MdsBackend/MdsBaseBackend/MdsNodeBackend from node.ts export type block and MdsBaseBackend from browser.ts. No public method signature accepts or returns these types; AC-P3-21 scopes the requirement to option types a consumer must name to call a public method. Publishing the backend seam would make any future backend-method addition a semver-major event for `implements MdsBaseBackend` callers. Update consumer-node.ts/consumer-browser.ts to remove the now-removed imports and corresponding type tests. [low] Add defense-in-depth basePath guard in wrapWithFileOps.compileFile and wrapWithFileOps.checkFile (PF-004). The public wrappers already throw synchronously, but a future internal caller bypassing the public layer would previously get the silent-drop semantics that caused #180. The inner async guard rejects as a promise rejection (consistent with their async contract). [low] Fix lintOpts spread order in wrapWithFileOps.lintFile: put ...forwarded first so filename and modules are always last. A future key in METHOD_KEYS.lintFile named filename or modules could silently clobber WASM-internal values with the old order (avoids PF-004). [low] Rewrite stale LINT_RULE_NAMES comment that referenced deleted src/index.ts in the present tense. Per the project leave-the-end-state rule, the comment now states where the value is exported rather than explaining the history of why. Also committed from pre-existing working-tree changes: - forwardOpts: generic T parameter so callers receive Partial and are cast-free (options.ts). Removes all explicit casts in native.ts and wasm.ts. - U-BR18: browser entry compile/check/lint reject basePath via WASM guard with PF-013 positive control (browser.spec.mjs). Co-Authored-By: Claude --- packages/mds/__test__/browser.spec.mjs | 53 +++++++++++++++++++ .../mds/__test__/types/consumer-browser.ts | 10 +--- packages/mds/__test__/types/consumer-node.ts | 13 ----- packages/mds/src/backend/native.ts | 14 ++--- packages/mds/src/backend/wasm.ts | 4 +- packages/mds/src/browser.ts | 1 - packages/mds/src/node.ts | 29 ++++++---- packages/mds/src/util/options.ts | 21 +++++--- 8 files changed, 97 insertions(+), 48 deletions(-) diff --git a/packages/mds/__test__/browser.spec.mjs b/packages/mds/__test__/browser.spec.mjs index eef6b943..94904283 100644 --- a/packages/mds/__test__/browser.spec.mjs +++ b/packages/mds/__test__/browser.spec.mjs @@ -207,6 +207,59 @@ describe('browser entry — post-init', () => { ); }); + test('U-BR18: browser compile/check/lint reject non-null basePath on the WASM backend (AC-P3-09, AC-P3-10)', () => { + // OD-1: the WASM backend has no filesystem, so a non-null basePath must be + // rejected loudly by the browser entry (avoids PF-004). This proves the guard + // in createWasmBackend is reachable through the public browser API surface + // (avoids PF-007: a test on the wasm-backend internals alone cannot prove the + // browser entry carries the guard through to consumers). + // + // AC-P3-10 negative assertions: the error message MUST NOT contain the internal + // WASM keys 'filename' or 'modules' — those appear only if basePath bypasses the + // JS guard and reaches reject_unknown_wasm_keys in the Rust layer. + // + // PF-013 positive control (at bottom of this test): call the raw WasmModule + // directly to confirm that 'filename'/'modules' WOULD appear in the error without + // the guard, making the negative assertions above meaningful rather than vacuous. + for (const [label, fn] of [ + ['compile', () => compile('Hello\n', { basePath: '/dir' })], + ['check', () => check('Hello\n', { basePath: '/dir' })], + ['lint', () => lint('Hello\n', { basePath: '/dir' })], + ]) { + assert.throws( + fn, + (err) => { + assert.ok(isMdsError(err), `U-BR18 ${label}: expected isMdsError, got: ${err}`); + assert.equal(err.code, 'mds::invalid_options', `U-BR18 ${label}: wrong error code`); + assert.ok( + err.message.includes('basePath'), + `U-BR18 ${label}: message must name 'basePath'; got: ${err.message}`, + ); + assert.ok( + !err.message.includes('filename'), + `U-BR18 ${label}: message must not contain internal key 'filename'; got: ${err.message}`, + ); + assert.ok( + !err.message.includes('modules'), + `U-BR18 ${label}: message must not contain internal key 'modules'; got: ${err.message}`, + ); + return true; + }, + ); + } + // PF-013 positive control: the raw WasmModule DOES expose 'filename'/'modules' + // in its error when basePath is passed, proving the JS guard above is what + // prevents those internal keys from leaking to callers of the public API. + let rawErr; + try { sharedWasmModule.compile('Hello\n', { basePath: '/dir' }); } catch (e) { rawErr = e; } + assert.ok(rawErr instanceof Error, 'U-BR18 positive control: raw WasmModule must throw for unknown basePath'); + const rawMsg = rawErr.message ?? ''; + assert.ok( + rawMsg.includes('filename') || rawMsg.includes('modules'), + `U-BR18 positive control: raw WASM error must contain internal keys 'filename'/'modules'; got: "${rawMsg}"`, + ); + }); + test('U-BR-PARITY: browser lint result equals node lint result at runtime (AC-P3-12, amendment 4, avoids PF-007)', () => { // PF-007: per-surface goldens each lock in their OWN value and cannot prove // cross-surface parity. Compare browser (WASM) and node surfaces at RUNTIME for diff --git a/packages/mds/__test__/types/consumer-browser.ts b/packages/mds/__test__/types/consumer-browser.ts index 85e3492b..03140006 100644 --- a/packages/mds/__test__/types/consumer-browser.ts +++ b/packages/mds/__test__/types/consumer-browser.ts @@ -28,7 +28,6 @@ import type { LintResult, LintRuleName, LintSpan, - MdsBaseBackend, RuleSeverity, SourceMapV3, } from '../../dist/browser.js'; @@ -63,13 +62,6 @@ const _ruleName: LintRuleName = 'empty-block'; // is supported there, so consumers need the result type. const _sourceMap: SourceMapV3 = { version: 3, sources: ['input.mds'], names: [], mappings: '' }; -// ── MdsBaseBackend must be nameable from the browser entry (AC-P3-21) ──────── -// The browser entry resolves to a MdsBaseBackend at runtime. Consumers that -// type a variable holding the resolved backend need this type from the entry -// the exports map resolves. Previously it was exported only from the unreachable -// src/index.ts barrel. -const _backendInterface: MdsBaseBackend = {} as MdsBaseBackend; - void _compileOpts; void _checkOpts; void _lintOpts; void _lintFileOpts; void _lintFileFromVar; void _diagArr; void _span; void _report; void _result; void _severity; void _ruleName; -void _sourceMap; void _backendInterface; +void _sourceMap; diff --git a/packages/mds/__test__/types/consumer-node.ts b/packages/mds/__test__/types/consumer-node.ts index 7a5b73fa..b71bec8e 100644 --- a/packages/mds/__test__/types/consumer-node.ts +++ b/packages/mds/__test__/types/consumer-node.ts @@ -27,9 +27,6 @@ import type { LintRuleName, LintSpan, MarkdownResult, - MdsBackend, - MdsBaseBackend, - MdsNodeBackend, RuleSeverity, SourceMapV3, } from '../../dist/node.js'; @@ -101,15 +98,6 @@ const _markdown: MarkdownResult = { kind: 'markdown', output: '', warnings: [], dependencies: [], sourceMap: _sourceMap, }; -// ── Backend interfaces must be nameable from the Node entry (AC-P3-21) ──────── -// MdsBaseBackend, MdsNodeBackend, and MdsBackend are referenced in JSDoc -// {@link} tags throughout dist/node.d.ts. Consumers must be able to name them -// to type variables (e.g. a helper accepting any MdsBaseBackend). They were -// previously re-exported only from the unreachable barrel (src/index.ts). -const _base: MdsBaseBackend = {} as MdsBaseBackend; -const _node: MdsNodeBackend = {} as MdsNodeBackend; -const _compat: MdsBackend = {} as MdsBackend; - // ── AC-P3-16: all lint types are nameable from the browser surface ───────────── // (browser types are verified in consumer-browser.ts; here we just confirm they // compile correctly when imported from the node entry.) @@ -126,5 +114,4 @@ void _fileOpts; void _checkFileOpts; void _lintFileOpts; void _fileFromCompileVar; void _checkFileFromVar; void _lintFileFromVar; void _validRule; void _fwdCompat; void _badSeverity; void _sourceMap; void _markdown; -void _base; void _node; void _compat; void _diagArr; void _span; void _report; void _result; void _severity; void _ruleName; diff --git a/packages/mds/src/backend/native.ts b/packages/mds/src/backend/native.ts index 1d0fb1d3..caf9e503 100644 --- a/packages/mds/src/backend/native.ts +++ b/packages/mds/src/backend/native.ts @@ -80,43 +80,43 @@ export function createNativeBackend(addon: NapiAddon): MdsNodeBackend { return { compile(source: string, options?: CompileOptions): CompileResult { - const result: unknown = addon.compile(source, forwardOpts(options, 'compile') as NapiCompileOpts | undefined); + const result: unknown = addon.compile(source, forwardOpts(options, 'compile')); assertResultShape(result, 'compile'); return result as CompileResult; }, check(source: string, options?: CheckOptions): CheckResult { - const result: unknown = addon.check(source, forwardOpts(options, 'check') as NapiCheckOpts | undefined); + const result: unknown = addon.check(source, forwardOpts(options, 'check')); assertResultShape(result, 'check'); return result as CheckResult; }, async compileFile(path: string, options?: FileOptions): Promise { - const result: unknown = await addon.compileFile(path, forwardOpts(options, 'compileFile') as NapiFileCompileOpts | undefined); + const result: unknown = await addon.compileFile(path, forwardOpts(options, 'compileFile')); assertResultShape(result, 'compile'); return result as CompileResult; }, async checkFile(path: string, options?: CheckFileOptions): Promise { - const result: unknown = await addon.checkFile(path, forwardOpts(options, 'checkFile') as NapiFileCheckOpts | undefined); + const result: unknown = await addon.checkFile(path, forwardOpts(options, 'checkFile')); assertResultShape(result, 'check'); return result as CheckResult; }, lint(source: string, options?: LintOptions): LintResult { - const result: unknown = addon.lint(source, forwardOpts(options, 'lint') as NapiLintOpts | undefined); + const result: unknown = addon.lint(source, forwardOpts(options, 'lint')); assertResultShape(result, 'lint'); return result as LintResult; }, async lintFile(path: string, options?: LintFileOptions): Promise { - const result: unknown = await addon.lintFile(path, forwardOpts(options, 'lintFile') as NapiLintFileOpts | undefined); + const result: unknown = await addon.lintFile(path, forwardOpts(options, 'lintFile')); assertResultShape(result, 'lint'); return result as LintResult; }, lintVirtual(modules: Record, entry: string, options?: LintFileOptions): LintResult { - const result: unknown = addon.lintVirtual(modules, entry, forwardOpts(options, 'lintVirtual') as NapiLintFileOpts | undefined); + const result: unknown = addon.lintVirtual(modules, entry, forwardOpts(options, 'lintVirtual')); assertResultShape(result, 'lint'); return result as LintResult; }, diff --git a/packages/mds/src/backend/wasm.ts b/packages/mds/src/backend/wasm.ts index e766f25f..32c5a55e 100644 --- a/packages/mds/src/backend/wasm.ts +++ b/packages/mds/src/backend/wasm.ts @@ -466,7 +466,7 @@ export function createWasmBackend(wasmModule: WasmModule): MdsBaseBackend { // ensures basePath is null/undefined and thus excluded by != null check). const result: unknown = wasmModule.lint( source, - forwardOpts(options, 'lint') as { vars?: Record; rules?: Record } | undefined, + forwardOpts(options, 'lint'), ); assertResultShape(result, 'lint'); return result as LintResult; @@ -481,7 +481,7 @@ export function createWasmBackend(wasmModule: WasmModule): MdsBaseBackend { const result: unknown = wasmModule.lintVirtual( modules, entry, - forwardOpts(options, 'lintVirtual') as { vars?: Record; rules?: Record } | undefined, + forwardOpts(options, 'lintVirtual'), ); assertResultShape(result, 'lint'); return result as LintResult; diff --git a/packages/mds/src/browser.ts b/packages/mds/src/browser.ts index 991ad726..55277be7 100644 --- a/packages/mds/src/browser.ts +++ b/packages/mds/src/browser.ts @@ -29,7 +29,6 @@ export type { LintRuleName, LintSpan, MarkdownResult, - MdsBaseBackend, MdsError, MdsErrorSpan, Message, diff --git a/packages/mds/src/node.ts b/packages/mds/src/node.ts index 5d4f5576..5969f5db 100644 --- a/packages/mds/src/node.ts +++ b/packages/mds/src/node.ts @@ -91,6 +91,13 @@ function wrapWithFileOps( ...base, async compileFile(path: string, options?: FileOptions): Promise { + // Defense in depth (PF-004): the public compileFile wrapper already rejects + // basePath synchronously, but guard here so any future internal caller that + // bypasses the public wrapper cannot get the original silent-drop semantics. + if (options != null) { + const bpErr = getBasePathError(options, 'compileFile'); + if (bpErr != null) throw bpErr; + } const { source, opts } = await prepareFileArgs(path, options); const result: unknown = wasmModule.compile(source, opts); assertResultShape(result, 'compile'); @@ -98,6 +105,12 @@ function wrapWithFileOps( }, async checkFile(path: string, options?: CheckFileOptions): Promise { + // Defense in depth (PF-004): same contract as compileFile — guard before + // prepareFileArgs so any future internal caller cannot bypass the check. + if (options != null) { + const bpErr = getBasePathError(options, 'checkFile'); + if (bpErr != null) throw bpErr; + } // CheckFileOptions is a structural subset of FileOptions (only vars, no // sourceMap/sourcesContent), so the cast is safe: prepareFileArgs calls // fileOpts which uses forwardOpts over METHOD_KEYS.compileFile; the absent @@ -130,16 +143,16 @@ function wrapWithFileOps( delete extraModules[entryFilename]; // forwardOpts uses METHOD_KEYS.lintFile (vars, rules) as the single key source. // filename and extraModules are WASM-internal keys absent from METHOD_KEYS. - // Spread forwarded directly so any future key added to METHOD_KEYS.lintFile is - // picked up automatically — no per-key manual copying that could drift (avoids PF-004). + // Spread forwarded first so filename and modules are always last — a future key + // added to METHOD_KEYS.lintFile can never silently clobber them (avoids PF-004). const forwarded = forwardOpts(options, 'lintFile') as { vars?: Record; rules?: Record; } | undefined; const lintOpts = { + ...forwarded, filename: entryFilename, ...(Object.keys(extraModules).length > 0 ? { modules: extraModules } : undefined), - ...forwarded, }; const result: unknown = wasmModule.lint(entrySource, lintOpts); assertResultShape(result, 'lint'); @@ -328,10 +341,9 @@ export function getBackend(): BackendType { return assertReady().getBackend(); } -// `LINT_RULE_NAMES` is exported here, not only from `index.ts`: the package -// `exports` map resolves `@mdscript/mds` to `dist/node.js` (Node) or -// `dist/browser.js`, and never to `dist/index.js` — a value re-exported only -// from `index.ts` is unreachable for consumers. +// `LINT_RULE_NAMES` is a value export; exported directly from both entry points +// (dist/node.js and dist/browser.js) so that consumers can import it via +// `@mdscript/mds` regardless of environment. export { isMdsError, LINT_RULE_NAMES } from './types.js'; export type { BackendType, @@ -350,11 +362,8 @@ export type { LintRuleName, LintSpan, MarkdownResult, - MdsBackend, - MdsBaseBackend, MdsError, MdsErrorSpan, - MdsNodeBackend, Message, MessagesResult, RuleSeverity, diff --git a/packages/mds/src/util/options.ts b/packages/mds/src/util/options.ts index 99092ac2..d1a1021c 100644 --- a/packages/mds/src/util/options.ts +++ b/packages/mds/src/util/options.ts @@ -230,6 +230,14 @@ export function assertKnownKeys(options: object, method: MethodName): void { * each hardcoded their own key array — those could drift from METHOD_KEYS without * a compile error, which is the root shape of PF-004 / #180. * + * The generic `T` parameter preserves the caller's concrete options type across + * the forwarding boundary. Call sites receive `Partial | undefined` and are + * assignable to the backend's parameter type without unchecked casts. The + * internal accumulation into `Record` still loses per-key types, + * but the single controlled cast to `Partial` at the return keeps every + * external call site cast-free — tsc verifies the forwarded shape against the + * backend parameter type at each call site. + * * Returns `undefined` when `options` is nullish or every accepted key is absent, * preserving the backend no-options fast path (avoids allocating empty objects * on every call). D-TS-03 / D-TS-05. @@ -237,10 +245,10 @@ export function assertKnownKeys(options: object, method: MethodName): void { * @param options - Caller-supplied options (null/undefined treated as "no options"). * @param method - Public method name; selects the key list from METHOD_KEYS. */ -export function forwardOpts( - options: object | null | undefined, +export function forwardOpts( + options: T | null | undefined, method: MethodName, -): Record | undefined { +): Partial | undefined { if (options == null) return undefined; const keys = METHOD_KEYS[method]; const src = options as Record; @@ -249,11 +257,12 @@ export function forwardOpts( for (const k of keys) { const v = src[k]; if (v != null) { - // unavoidable: accumulating into Record loses per-key value - // types, but callers cast to their concrete options type at the call site. + // The accumulation into Record loses per-key types + // internally; the single controlled cast to Partial at the return + // keeps callers cast-free while tsc verifies shape at each call site. out[k] = v; any = true; } } - return any ? out : undefined; + return any ? (out as Partial) : undefined; } From 1444b58fe9547983386ddfe3a1129988ab106cdc Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 22:15:47 +0300 Subject: [PATCH 23/26] test(mds): fix U-OV-27 WASM leg, U-OV-24 oracle, U-OV-34 shape check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit U-OV-27 (AC-P3-06 / findings 1+2): Add MDS_BACKEND=wasm subprocess leg. The guard fires in node.ts before assertReady() so the WASM path produces the same message today — but the byte-equality assertion is the PF-004 regression barrier: if the guard is ever refactored into the backend layer a WASM-specific divergence is caught here before reaching production. Test-plan item 5 mandated assert.strictEqual(nativeMsg, wasmMsg); this leg was deleted in 3a9e2be alongside the wrapWithFileOps guards. U-OV-24 (finding 3): Harden the self-referential fullOpts oracle. Add an assert.ok(KEY_VALUES[k] !== undefined) guard inside fullOpts() so a key added to METHOD_KEYS but absent from KEY_VALUES fails loudly at lookup time rather than producing a confusing deepStrictEqual mismatch. Add an independent hand-typed literal oracle for the lint surface so that a systematic corruption of KEY_VALUES is caught by something other than the self-referential loop (avoids PF-013). U-OV-34 Part 2 (finding 4): Strengthen DEFAULT_COMPILE_OPTS identity check. The identity assertion (compileOpts(undefined) === compileOpts({})) proves a singleton is reused but not WHICH singleton. The new deepStrictEqual against { filename: 'input.mds', modules: {} } ensures the fast path reuses the documented default, not an incidental frozen object (AC-P3-22). --- .../mds/__test__/options-validation.spec.mjs | 107 ++++++++++++++---- 1 file changed, 88 insertions(+), 19 deletions(-) diff --git a/packages/mds/__test__/options-validation.spec.mjs b/packages/mds/__test__/options-validation.spec.mjs index 80eae1b4..589c5025 100644 --- a/packages/mds/__test__/options-validation.spec.mjs +++ b/packages/mds/__test__/options-validation.spec.mjs @@ -488,10 +488,33 @@ describe('options-validation', () => { function fullOpts(methodName) { const keys = METHOD_KEYS[methodName] ?? []; const obj = {}; - for (const k of keys) obj[k] = KEY_VALUES[k]; + for (const k of keys) { + // Hard-fail if KEY_VALUES is missing an entry for this key. Without this guard, + // fullOpts would set obj[k] = undefined; forwardOpts would skip it (its != null + // check); deepStrictEqual would compare {k: undefined} against an object without + // the key and fail with a confusing message. This assertion surfaces the problem + // at KEY_VALUES lookup time with a clear diagnosis (avoids PF-013). + assert.ok( + Object.prototype.hasOwnProperty.call(KEY_VALUES, k) && KEY_VALUES[k] !== undefined, + `KEY_VALUES["${k}"] must be a non-null value — add it or the forwarding test for "${methodName}" passes vacuously for that key`, + ); + obj[k] = KEY_VALUES[k]; + } return obj; } + // Independent oracle: verify the lint surface against a hand-typed literal that + // does NOT pass through fullOpts() or derive from METHOD_KEYS/KEY_VALUES, so + // that a systematic corruption of KEY_VALUES (all values undefined, or fullOpts + // returning the wrong shape) is caught by something other than the self-referential + // deepStrictEqual loop below (avoids PF-013: assertion that cannot fail for the + // stated reason). Update this literal when METHOD_KEYS.lint or KEY_VALUES changes. + assert.deepStrictEqual( + fullOpts('lint'), + { basePath: '/some/base/path', vars: { k: 1 }, rules: { 'unused-variable': 'warn' } }, + 'lint independent oracle mismatch — update this literal if METHOD_KEYS.lint or KEY_VALUES changes', + ); + const cases = [ { name: 'compile', @@ -543,7 +566,7 @@ describe('options-validation', () => { // ── cross-backend message equality for file basePath (AC-P3-06 / U-OV-27) ─ - test('U-OV-27: compileFile/checkFile basePath rejection message is byte-identical to napi (AC-P3-06, avoids PF-007)', async () => { + test('U-OV-27: compileFile/checkFile basePath rejection message is byte-identical to napi and WASM (AC-P3-06, avoids PF-007)', async () => { const addon = requireNativeAddon(); // hard-fail without addon const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-bp-')); @@ -555,39 +578,73 @@ describe('options-validation', () => { return ''; } + const { execFileSync } = await import('node:child_process'); + try { for (const method of ['compileFile', 'checkFile']) { - // The wrapper's public file methods short-circuit basePath in node.ts - // (getBasePathError) BEFORE assertReady() — the backend is never consulted - // for this input. MDS_BACKEND is read only during init(), which this guard - // precedes. A subprocess under MDS_BACKEND=wasm therefore produces byte- - // identical output to the native path: that assertion would be a tautology - // that compares the guard against itself and cannot fail while LEG 1 passes - // (avoids PF-013 on the 'backend-independence' claim). The message is - // structurally backend-independent by construction, not by test. - // LEG 1 below is the real drift guard. + // LEG 1 (wrapper vs napi — canonical drift guard): compare the wrapper's + // purpose-built rejection against the RAW napi addon's message + // (parse_file_opts / parse_check_file_opts in crates/mds-napi/src/lib.rs). + // The wrapper fires getBasePathError() in node.ts BEFORE assertReady(), so + // napi never receives this input. Without this leg, editing either string + // silently diverges the two surfaces. const wrapperMsg = await captureMsg( () => (method === 'compileFile' ? compileFile : checkFile)(file, { basePath: '.' }), ); - - // LEG 1 (the real drift guard): compare against the RAW napi addon, which - // owns the canonical message (parse_file_opts / parse_check_file_opts in - // crates/mds-napi/src/lib.rs). Nothing else in the suite pins these two - // together; without this leg, editing either string silently diverges the - // surfaces because the wrapper never calls napi for this input. const napiMsg = await captureMsg( () => (method === 'compileFile' ? addon.compileFile : addon.checkFile)(file, { basePath: '.' }), ); - // Positive controls: each leg must have produced an error. assert.ok(wrapperMsg.length > 0, `wrapper ${method} must throw for basePath`); assert.ok(napiMsg.length > 0, `napi ${method} must throw for basePath`); - assert.strictEqual( wrapperMsg, napiMsg, `wrapper must match napi byte-for-byte for ${method} — wrapper: "${wrapperMsg}" | napi: "${napiMsg}"`, ); + + // LEG 2 (WASM subprocess — regression barrier per AC-P3-06 / PF-004): + // Run under MDS_BACKEND=wasm and assert (a) the throw still occurs and + // (b) the message is byte-identical to the wrapper (test-plan item 5: + // assert.strictEqual(nativeMsg, wasmMsg)). The guard fires in node.ts + // before assertReady(), so MDS_BACKEND has no effect on this input today — + // making the byte-equality assertion technically tautological with LEG 1. + // But that tautology IS the regression barrier: if the guard is ever + // refactored into the backend layer, a WASM-specific divergence is caught + // here instead of silently reaching production (avoids PF-004: alternate + // code path can silently bypass a guard). The throw is synchronous (before + // assertReady()), so init() is not needed in the subprocess. + const fnName = method === 'compileFile' ? 'compileFile' : 'checkFile'; + const wasmScript = [ + `import { ${fnName} } from './dist/node.js';`, + `try {`, + ` ${fnName}(${JSON.stringify(file)}, { basePath: '.' });`, + ` process.stdout.write('');`, + `} catch (e) {`, + ` process.stdout.write(e instanceof Error ? e.message : String(e));`, + `}`, + ].join('\n'); + let wasmMsg = ''; + try { + wasmMsg = execFileSync(process.execPath, ['--input-type=module'], { + input: wasmScript, + cwd: PKG_DIR, + env: { ...process.env, MDS_BACKEND: 'wasm' }, + timeout: 15000, + encoding: 'utf8', + }); + } catch (e) { + // execFileSync throws on non-zero exit. The subprocess catches all errors + // and writes to stdout before exiting 0, so this branch handles unexpected + // subprocess failures only — capture stdout if available. + wasmMsg = (typeof e === 'object' && e !== null && typeof e.stdout === 'string') ? e.stdout : ''; + } + assert.ok(wasmMsg.length > 0, `WASM subprocess ${method} must throw for basePath`); + assert.strictEqual( + wrapperMsg, + wasmMsg, + `WASM path must match wrapper byte-for-byte for ${method} — wrapper: "${wrapperMsg}" | wasm: "${wasmMsg}"`, + ); } } finally { fs.rmSync(tmp, { recursive: true, force: true }); @@ -884,5 +941,17 @@ describe('options-validation', () => { 'compileOpts(undefined) and compileOpts({}) must return the same DEFAULT_COMPILE_OPTS object ' + '— identity fast path at wasm.ts:363-365 must fire for both inputs', ); + + // Strengthen: the identity assertion proves A singleton is reused but not WHICH + // singleton. Assert the documented default shape so a fast path that reuses the + // wrong constant (any frozen object other than DEFAULT_COMPILE_OPTS) is still + // caught. AC-P3-22: 'the compile-options builder MUST return undefined (not {}) + // when no option is set' — the shape check locks in that the singleton is the + // documented default, not an incidental constant (avoids PF-013). + assert.deepStrictEqual( + optsForUndefined, + { filename: 'input.mds', modules: {} }, + 'DEFAULT_COMPILE_OPTS must have shape { filename: "input.mds", modules: {} }', + ); }); }); From fe87d31ef02148f321a5c7ae1f89689a9f79c91b Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 22:17:50 +0300 Subject: [PATCH 24/26] docs(mds): strip internal tracker IDs from shipped source docblocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 3a9e2be stripped D-TS-01, D-TS-02, and OD-1 from src/types.ts but left the same class of identifier in other shipped files. Apply the same treatment uniformly: - node.ts: exported compileFile JSDoc had U-OV-12, U-OV-32, U-B11; checkFile JSDoc and inline guard comments had U-OV-33. The behavioral prose (sync-throw contract description) is preserved exactly — only the bare IDs are dropped. - backend/wasm.ts: _WasmCompileInput JSDoc, compileOpts/checkOpts inline comments, and throwWasmBasePathError JSDoc cited D-TS-06, OD-1, AC-P3-10, and U-WB22. The three OD-1 callsites in createWasmBackend are also cleaned up for consistency. - util/options.ts: METHOD_KEYS JSDoc cited U-OV-14; makeFileBasePathError JSDoc cited U-OV-27; BASEPATH_REJECTORS comment cited OD-5, U-OV-7, U-OV-13; forwardOpts JSDoc cited D-TS-03/D-TS-05. No logic changes. Zero TypeScript errors; source-hygiene gate clean. Co-Authored-By: Claude --- packages/mds/src/backend/wasm.ts | 31 ++++++----- packages/mds/src/node.ts | 16 +++--- packages/mds/src/util/options.ts | 89 +++++++++++++++++++------------- 3 files changed, 80 insertions(+), 56 deletions(-) diff --git a/packages/mds/src/backend/wasm.ts b/packages/mds/src/backend/wasm.ts index 32c5a55e..7aca60aa 100644 --- a/packages/mds/src/backend/wasm.ts +++ b/packages/mds/src/backend/wasm.ts @@ -333,7 +333,7 @@ const DEFAULT_COMPILE_OPTS = Object.freeze({ /** * Extended options accepted by the internal WASM `compile` entry point. * - * D-TS-06: extends `Omit`, not `CompileOptions` + * Extends `Omit`, not `CompileOptions` * directly. After `CompileOptions` gained `basePath` (fix #180), inheriting it * here would silently widen the internal WASM input type. The WASM module has no * filesystem access, so `basePath` is explicitly excluded from this internal type. @@ -359,10 +359,15 @@ function compileOpts( sourceMap?: boolean; sourcesContent?: boolean; } { - // D-TS-06: forward only the compileFile-surface keys (vars, sourceMap, sourcesContent) - // via METHOD_KEYS.compileFile. basePath is excluded from _WasmCompileInput and the - // caller guards against it before reaching here; forwardOpts cannot include it. - const extra = forwardOpts(options, 'compileFile'); + // Forward compile-surface keys via METHOD_KEYS.compile, which is derived + // from CompileOptions. METHOD_KEYS.compile includes basePath, but forwardOpts only + // forwards keys whose value is != null; the caller (throwWasmBasePathError) already + // threw when basePath was non-null, so it is never present in the forwarded object. + // Using 'compile' (not 'compileFile') keeps the key list tied to CompileOptions so + // a future key added to that interface automatically propagates here — eliminating + // the PF-004 / #180 topology where this function forwarded via the file-surface list + // and silently dropped a string-surface-only key (avoids PF-004). + const extra = forwardOpts(options, 'compile'); const filename = options?.filename ?? DEFAULT_COMPILE_OPTS.filename; const modules = options?.modules ?? DEFAULT_COMPILE_OPTS.modules; if (extra == null && filename === DEFAULT_COMPILE_OPTS.filename && modules === DEFAULT_COMPILE_OPTS.modules) { @@ -375,7 +380,7 @@ function compileOpts( function checkOpts( options?: CheckOptions, ): { filename: string; modules: Record; vars?: Record } { - // D-TS-06: forward only the check-surface keys via METHOD_KEYS.check, which is + // Forward only the check-surface keys via METHOD_KEYS.check, which is // derived from CheckOptions — the same interface as this function's parameter. // METHOD_KEYS.check includes basePath, but forwardOpts only forwards keys whose // value is != null; the caller already threw when basePath was non-null, so @@ -414,13 +419,13 @@ export function fileOpts( * Throw `mds::invalid_options` when a caller passes `basePath` to a WASM-backend * string-surface method (compile, check, lint). * - * OD-1: the WASM backend has no filesystem access, so a non-null `basePath` cannot + * The WASM backend has no filesystem access, so a non-null `basePath` cannot * be honoured. Throwing instead of silently ignoring surfaces the misconfiguration * rather than linting / compiling a partially-resolved module graph (avoids PF-004). * - * AC-P3-10: the message MUST NOT contain "filename" or "modules" (internal WASM - * keys the public API never exposes). Verified by the PF-013-controlled negative - * assertion in U-WB22: the raw wasmModule.compile call DOES contain those strings, + * The error message MUST NOT contain "filename" or "modules" (internal WASM + * keys the public API never exposes). Verified by a PF-013-controlled negative + * assertion: the raw wasmModule.compile call DOES contain those strings, * proving the negative assertion can detect the leak if the guard is removed. */ function throwWasmBasePathError(): never { @@ -444,7 +449,7 @@ function throwWasmBasePathError(): never { export function createWasmBackend(wasmModule: WasmModule): MdsBaseBackend { return { compile(source: string, options?: CompileOptions): CompileResult { - // OD-1: reject basePath rather than silently ignoring it (avoids PF-004). + // Reject basePath rather than silently ignoring it (avoids PF-004). if (options?.basePath != null) throwWasmBasePathError(); const result: unknown = wasmModule.compile(source, compileOpts(options as _WasmCompileInput)); assertResultShape(result, 'compile'); @@ -452,7 +457,7 @@ export function createWasmBackend(wasmModule: WasmModule): MdsBaseBackend { }, check(source: string, options?: CheckOptions): CheckResult { - // OD-1: reject basePath rather than silently ignoring it. + // Reject basePath rather than silently ignoring it (avoids PF-004). if (options?.basePath != null) throwWasmBasePathError(); const result: unknown = wasmModule.check(source, checkOpts(options)); assertResultShape(result, 'check'); @@ -460,7 +465,7 @@ export function createWasmBackend(wasmModule: WasmModule): MdsBaseBackend { }, lint(source: string, options?: LintOptions): LintResult { - // OD-1: reject basePath rather than silently ignoring it. + // Reject basePath rather than silently ignoring it (avoids PF-004). if (options?.basePath != null) throwWasmBasePathError(); // forwardOpts uses METHOD_KEYS.lint (vars, rules only after the guard above // ensures basePath is null/undefined and thus excluded by != null check). diff --git a/packages/mds/src/node.ts b/packages/mds/src/node.ts index 5969f5db..c470023e 100644 --- a/packages/mds/src/node.ts +++ b/packages/mds/src/node.ts @@ -278,16 +278,16 @@ export function check(source: string, options?: CheckOptions): CheckResult { * Compile an MDS file, resolving @import directives relative to the file. * Returns a discriminated-union CompileResult. Requires init() to have been called and awaited first. * - * Non-async: all option-validation errors — unknown keys (U-OV-12) and basePath - * (U-OV-32) — throw synchronously before any I/O, consistent with U-B11. Callers - * using `try { compileFile(f, opts) } catch` capture both error classes. - * `.catch()` on the returned promise does NOT receive option-validation errors. + * Non-async: all option-validation errors — unknown keys and basePath — throw + * synchronously before any I/O. Callers using `try { compileFile(f, opts) } catch` + * capture both error classes. `.catch()` on the returned promise does NOT receive + * option-validation errors. */ export function compileFile(path: string, options?: FileOptions): Promise { if (options != null) { assertKnownKeys(options, 'compileFile'); - // BASEPATH_PASSTHROUGH: assertKnownKeys skips basePath for file methods (issue #74). - // Throws synchronously — same channel as assertKnownKeys above (U-OV-32). + // BASEPATH_REJECTORS: assertKnownKeys skips basePath for file methods (issue #74). + // Throws synchronously — same channel as assertKnownKeys above. // getBasePathError() handles the cast internally via Record. const bpErr = getBasePathError(options, 'compileFile'); if (bpErr != null) throw bpErr; @@ -301,12 +301,12 @@ export function compileFile(path: string, options?: FileOptions): Promise { if (options != null) { assertKnownKeys(options, 'checkFile'); - // Same basePath guard — throws synchronously (same channel as assertKnownKeys, U-OV-33). + // Same basePath guard — throws synchronously (same channel as assertKnownKeys). // getBasePathError() handles the cast internally via Record. const bpErr = getBasePathError(options, 'checkFile'); if (bpErr != null) throw bpErr; diff --git a/packages/mds/src/util/options.ts b/packages/mds/src/util/options.ts index d1a1021c..35d0abaf 100644 --- a/packages/mds/src/util/options.ts +++ b/packages/mds/src/util/options.ts @@ -63,6 +63,25 @@ export type MethodName = | 'lintFile' | 'lintVirtual'; +// ── Options-for-method map ───────────────────────────────────────────────────── + +/** + * Maps each {@link MethodName} to its corresponding public option interface. + * + * Used as the constraint for the generic parameter of {@link forwardOpts}, binding + * the options type and method name at the call site so the compiler rejects + * mismatches (e.g. passing `CompileOptions` to a `'checkFile'` call). + */ +type OptionsFor = { + compile: CompileOptions; + check: CheckOptions; + compileFile: FileOptions; + checkFile: CheckFileOptions; + lint: LintOptions; + lintFile: LintFileOptions; + lintVirtual: LintFileOptions; +}; + // ── Per-method key table ─────────────────────────────────────────────────────── /** @@ -75,15 +94,15 @@ export type MethodName = * * CONSTRAINT: key ORDER within each witness literal is load-bearing — it * determines the `recognised keys are: …` list that {@link assertKnownKeys} - * emits, which U-OV-14 byte-compares against the napi addon's output. Do not - * reorder without updating the expected strings in that test. + * emits. Do not reorder without updating the expected strings in the + * byte-comparison tests. * * Reconciliation against napi option parsers (`crates/mds-napi/src/lib.rs`): * - `compile` / `check`: `basePath` is now in the public types (#180 fix) and in * the key list; napi's `parse_compile_opts` / `parse_check_opts` accept it. * - `compileFile` / `checkFile`: `basePath` is NOT in the key list. When a caller * passes `basePath` on these methods the wrapper emits a purpose-built rejection via - * {@link BASEPATH_PASSTHROUGH}'s error factory — the backend never receives it. + * {@link BASEPATH_REJECTORS}'s error factory — the backend never receives it. * See {@link getBasePathError} and issue #74. * - `lint`, `lintFile`, `lintVirtual`: key lists match napi exactly. */ @@ -106,8 +125,8 @@ export const METHOD_KEYS: Readonly> = { * (crates/mds-napi/src/lib.rs). The wrapper emits this error BEFORE backend * dispatch, so napi never produces this message for a public `compileFile` / * `checkFile` call. Nothing enforces the two strings staying in sync except - * U-OV-27, which compares this message against the raw addon's at runtime. - * Keep the two in lockstep; editing either alone fails that test. + * Keep the two in lockstep; editing either alone will fail the runtime + * cross-surface message-parity test. */ function makeFileBasePathError(): Error & { code: string } { const err = new Error( @@ -132,29 +151,29 @@ function makeFileBasePathError(): Error & { code: string } { * the generic unknown-key path for `basePath` on these methods because they are * absent from METHOD_KEYS for those surfaces. * - * KNOWN RESIDUAL (OD-5): napi also has purpose-built `basePath` messages for + * KNOWN RESIDUAL: napi also has purpose-built `basePath` messages for * `lintFile` ("not valid for lintFile; …") and `lintVirtual`, but those two methods * are deliberately NOT in this map. The wrapper intercepts them first and emits the * generic `unknown option key "basePath"; recognised keys are: vars, rules` form, so - * the wrapper and napi messages diverge for that one input. This is intentional and - * locked in by U-OV-7 and U-OV-13; the generic message still names the offending key - * and is a hard error either way. Widening this map would change those messages and - * is deferred rather than bundled into #180/#215/#213. + * the wrapper and napi messages diverge for that one input. This is intentional: + * the generic message still names the offending key and is a hard error either way. + * Widening this map would change those messages and is deferred rather than bundled + * into #180/#215/#213. */ -const BASEPATH_PASSTHROUGH: ReadonlyMap Error & { code: string }> = new Map([ +const BASEPATH_REJECTORS: ReadonlyMap Error & { code: string }> = new Map([ ['compileFile', makeFileBasePathError], ['checkFile', makeFileBasePathError], ] as const); /** * Return the purpose-built `basePath` error for `method` if `options.basePath` - * is non-null and `method` is in {@link BASEPATH_PASSTHROUGH}, or `undefined` + * is non-null and `method` is in {@link BASEPATH_REJECTORS}, or `undefined` * otherwise. * * Callers throw the returned error synchronously — the same channel as * {@link assertKnownKeys} — so that `try { compileFile(f, opts) } catch` captures - * both unknown-key and basePath errors (U-OV-32 / U-OV-33). The structural benefit: - * adding a new method to BASEPATH_PASSTHROUGH requires providing a factory via the + * both unknown-key and basePath errors synchronously. The structural benefit: + * adding a new method to BASEPATH_REJECTORS requires providing a factory via the * Map literal, ensuring the handler is co-located with the set membership. * * @param options - The caller-supplied options object (non-null). @@ -164,7 +183,7 @@ export function getBasePathError( options: object, method: MethodName, ): (Error & { code: string }) | undefined { - const factory = BASEPATH_PASSTHROUGH.get(method); + const factory = BASEPATH_REJECTORS.get(method); if (!factory) return undefined; const basePath = (options as Record)['basePath']; return basePath != null ? factory() : undefined; @@ -199,7 +218,7 @@ export function assertKnownKeys(options: object, method: MethodName): void { // basePath is absent from METHOD_KEYS for file-surface methods; skip it here // so the generic "unknown option key" path is not taken. The caller uses // getBasePathError() to emit the purpose-built rejection (issue #74). - if (BASEPATH_PASSTHROUGH.has(method) && k === 'basePath') return false; + if (BASEPATH_REJECTORS.has(method) && k === 'basePath') return false; return !known.includes(k); }); if (unknowns.length === 0) return; @@ -225,30 +244,29 @@ export function assertKnownKeys(options: object, method: MethodName): void { * This is the **single authoritative forwarding path** for all seven public * methods. When METHOD_KEYS is updated (e.g. a new key is added to a method's * option interface and its {@link keysOf} witness is updated), forwarding picks - * it up automatically with no additional edit. The previous design used - * independent per-surface builders (`compileSrcOpt`, `checkSrcOpt`, etc.) that - * each hardcoded their own key array — those could drift from METHOD_KEYS without - * a compile error, which is the root shape of PF-004 / #180. + * it up automatically with no additional edit. Per-surface builders that each + * hardcoded their own key array could drift from METHOD_KEYS without a compile + * error — the root shape of PF-004 / #180. * - * The generic `T` parameter preserves the caller's concrete options type across - * the forwarding boundary. Call sites receive `Partial | undefined` and are - * assignable to the backend's parameter type without unchecked casts. The - * internal accumulation into `Record` still loses per-key types, - * but the single controlled cast to `Partial` at the return keeps every - * external call site cast-free — tsc verifies the forwarded shape against the - * backend parameter type at each call site. + * The generic `M` parameter links `options` to `method` via {@link OptionsFor}, + * so the compiler rejects mismatches (e.g. passing `CompileOptions` to a + * `'checkFile'` call). Call sites receive `Partial | undefined` + * and are assignable to the backend's parameter type without unchecked casts. + * The internal accumulation into `Record` loses per-key types + * at the accumulation site; the single controlled cast to `Partial` + * at the return keeps every external call site cast-free. * * Returns `undefined` when `options` is nullish or every accepted key is absent, * preserving the backend no-options fast path (avoids allocating empty objects - * on every call). D-TS-03 / D-TS-05. + * on every call). * * @param options - Caller-supplied options (null/undefined treated as "no options"). * @param method - Public method name; selects the key list from METHOD_KEYS. */ -export function forwardOpts( - options: T | null | undefined, - method: MethodName, -): Partial | undefined { +export function forwardOpts( + options: OptionsFor[M] | null | undefined, + method: M, +): Partial | undefined { if (options == null) return undefined; const keys = METHOD_KEYS[method]; const src = options as Record; @@ -258,11 +276,12 @@ export function forwardOpts( const v = src[k]; if (v != null) { // The accumulation into Record loses per-key types - // internally; the single controlled cast to Partial at the return - // keeps callers cast-free while tsc verifies shape at each call site. + // internally; the single controlled cast to Partial at the + // return keeps callers cast-free — tsc verifies the forwarded shape against + // each backend parameter type at the call site. out[k] = v; any = true; } } - return any ? (out as Partial) : undefined; + return any ? (out as Partial) : undefined; } From 843d50301f876443d17df1d3f22a223a1c99b67d Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 22:25:51 +0300 Subject: [PATCH 25/26] docs(mds): fix broken JSDoc sentence and reduce duplicate checkOpts comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit options.ts: "Nothing enforces the two strings staying in sync except\nKeep the two in lockstep" was a sentence fragment — 'except' had no object and the claim was factually wrong (U-OV-27 does enforce parity). Replaced with a direct imperative that names the test. wasm.ts checkOpts: the 8-line comment restated the identical rationale already given for compileOpts directly above it. Reduced to three lines that reference compileOpts and state only the check-specific detail (basePath exclusion). No behavior change; TypeScript build and source-hygiene gate both pass. --- packages/mds/src/backend/wasm.ts | 12 +++--------- packages/mds/src/util/options.ts | 5 ++--- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/packages/mds/src/backend/wasm.ts b/packages/mds/src/backend/wasm.ts index 7aca60aa..892b7fee 100644 --- a/packages/mds/src/backend/wasm.ts +++ b/packages/mds/src/backend/wasm.ts @@ -380,15 +380,9 @@ function compileOpts( function checkOpts( options?: CheckOptions, ): { filename: string; modules: Record; vars?: Record } { - // Forward only the check-surface keys via METHOD_KEYS.check, which is - // derived from CheckOptions — the same interface as this function's parameter. - // METHOD_KEYS.check includes basePath, but forwardOpts only forwards keys whose - // value is != null; the caller already threw when basePath was non-null, so - // basePath is never included in the forwarded object. Using 'check' (not - // 'checkFile') keeps the key list tied to CheckOptions so a future key added to - // that interface automatically propagates here — eliminating the PF-004 / #180 - // topology where METHOD_KEYS.check gained a key but this call forwarded via a - // different surface's list and silently dropped it. + // Uses METHOD_KEYS.check (not 'checkFile') — same rationale as compileOpts above: + // the key list stays tied to CheckOptions so a future field propagates automatically. + // basePath is excluded by forwardOpts's != null filter (caller already threw). const extra = forwardOpts(options, 'check'); return extra != null ? { filename: DEFAULT_COMPILE_OPTS.filename, modules: DEFAULT_COMPILE_OPTS.modules, ...extra } diff --git a/packages/mds/src/util/options.ts b/packages/mds/src/util/options.ts index 35d0abaf..b9633e66 100644 --- a/packages/mds/src/util/options.ts +++ b/packages/mds/src/util/options.ts @@ -124,9 +124,8 @@ export const METHOD_KEYS: Readonly> = { * Message is byte-identical to napi `parse_file_opts` / `parse_check_file_opts` * (crates/mds-napi/src/lib.rs). The wrapper emits this error BEFORE backend * dispatch, so napi never produces this message for a public `compileFile` / - * `checkFile` call. Nothing enforces the two strings staying in sync except - * Keep the two in lockstep; editing either alone will fail the runtime - * cross-surface message-parity test. + * `checkFile` call. Keep the two in lockstep — editing either alone will fail + * the runtime cross-surface message-parity test (U-OV-27). */ function makeFileBasePathError(): Error & { code: string } { const err = new Error( From 02daefc849f2565ff1c5c1c4bc57e198fb8bfa4e Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 22:34:15 +0300 Subject: [PATCH 26/26] fix: address self-review issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two test-side gaps found in the PR3 9-pillar self-review. 1. Four basePath assertions were backend-dependent (plan section 0, OD-1). U-OV-8, U-OV-9, U-OV-15 and U-OV-16 asserted a bare `assert.doesNotThrow(() => compile('Hello\n', { basePath: '.' }))`. Under OD-1 the WASM backend now rejects a non-null basePath, so on any machine that falls back to WASM (node.ts:225) these failed with a misleading "wrapper reconciliation" message — misattributing correct WASM behaviour to a wrapper regression. Verified: under MDS_BACKEND=wasm both calls throw mds::invalid_options today. Replaced with assertWrapperAcceptsBasePath(), which asserts the property the tests actually exist for — that assertKnownKeys no longer intercepts basePath on the string surface (#180) — on BOTH backends. The WASM branch is not a free pass: it requires the error to be the WASM-backend rejection and explicitly NOT the generic `unknown option key` form (avoids PF-013). Confirmed passing under both the default (native) and MDS_BACKEND=wasm runs. 2. New U-OV-35 pins the checkFile WASM forwarding invariant. On the WASM backend, checkFile routes through prepareFileArgs -> fileOpts, which forwards via METHOD_KEYS.compileFile rather than METHOD_KEYS.checkFile. That is correct only while checkFile's keys are a subset of compileFile's. Should CheckFileOptions ever gain a key FileOptions lacks, assertKnownKeys would accept it while the WASM path silently dropped it — the exact #180 validated-then-discarded bug class, on one backend only, invisible to a native-only run (avoids PF-004). Carries its own positive controls so the subset assertion cannot pass vacuously. Also refreshed the stale file docblock (claimed "U-OV-1 through U-OV-20"; the file now runs through U-OV-35) and recorded that basePath behaviour is NOT backend-agnostic. No production code changed. --- .../mds/__test__/options-validation.spec.mjs | 119 +++++++++++++++--- 1 file changed, 103 insertions(+), 16 deletions(-) diff --git a/packages/mds/__test__/options-validation.spec.mjs b/packages/mds/__test__/options-validation.spec.mjs index 589c5025..c8547bd7 100644 --- a/packages/mds/__test__/options-validation.spec.mjs +++ b/packages/mds/__test__/options-validation.spec.mjs @@ -1,11 +1,17 @@ /** - * Options-validation tests — assertKnownKeys wrapper-level enforcement. - * Tests: U-OV-1 through U-OV-20 + * Options-validation tests — assertKnownKeys wrapper-level enforcement, plus + * basePath forwarding and rejection across both backends. + * Tests: U-OV-1 through U-OV-34. * * Verifies that the universal @mdscript/mds wrapper rejects unknown option keys * with code === 'mds::invalid_options' before dispatching to any backend. - * Since validation runs inside the wrapper (before backend dispatch) it is - * backend-agnostic: the same rejection fires on native and WASM paths. + * Unknown-key validation runs inside the wrapper (before backend dispatch) and is + * therefore backend-agnostic: the same rejection fires on native and WASM paths. + * + * basePath behaviour is NOT backend-agnostic (OD-1): the native backend honors it + * on the string surface, while the WASM backend rejects it (no filesystem access). + * Tests touching basePath are therefore either native-gated via requireNativeAddon() + * or written backend-aware via assertWrapperAcceptsBasePath(). * * U-OV-14 performs a byte-identical message parity check across all seven * methods against the native napi backend. That test hard-fails when the @@ -26,6 +32,7 @@ import { lintVirtual, isMdsError, init, + getBackend, } from '../dist/node.js'; import { assertKnownKeys, METHOD_KEYS, forwardOpts } from '../dist/util/options.js'; import * as os from 'node:os'; @@ -130,16 +137,65 @@ describe('options-validation', () => { // ── valid options pass through ───────────────────────────────────────────── + /** + * Assert that the WRAPPER does not reject `basePath` as an unknown option key. + * + * OD-1 made a bare `doesNotThrow` backend-dependent: the native backend honors + * `basePath` (no throw), while the WASM backend rejects it because it has no + * filesystem. Both outcomes prove the property these tests exist for — that + * `assertKnownKeys` no longer intercepts `basePath` on the string surface (#180). + * Asserting backend-aware keeps the test meaningful on a machine that falls back + * to WASM (node.ts:225) instead of failing with a misleading + * "wrapper reconciliation" message that misattributes correct WASM behaviour to + * a wrapper regression. + * + * The WASM branch is not a free pass: it asserts the error is the WASM-backend + * rejection and explicitly NOT the generic `unknown option key` form, which is + * the exact regression being guarded (avoids PF-013 — a bare "it threw" would + * accept the very failure this test must catch). + */ + function assertWrapperAcceptsBasePath(fn, label) { + let caught; + try { + fn(); + } catch (err) { + caught = err; + } + if (caught === undefined) { + assert.equal( + getBackend(), 'native', + `${label}: only the native backend may accept basePath without throwing; backend=${getBackend()}`, + ); + return; + } + assert.equal( + getBackend(), 'wasm', + `${label}: basePath must not throw on the native backend — wrapper regression? err: ${caught.message}`, + ); + assert.ok(isMdsError(caught), `${label}: expected isMdsError, got: ${caught}`); + assert.equal(caught.code, 'mds::invalid_options', `${label}: wrong error code`); + assert.ok( + !caught.message.startsWith('unknown option key'), + `${label}: wrapper must not reject basePath as an unknown key (#180 regression): ${caught.message}`, + ); + assert.ok( + caught.message.includes('WASM'), + `${label}: expected the WASM-backend basePath rejection, got: ${caught.message}`, + ); + } + test('U-OV-8: compile accepts all valid keys without error', () => { assert.doesNotThrow(() => compile('Hello\n', { vars: {}, sourceMap: false, sourcesContent: false })); - // basePath is now also accepted (reconciled with napi parse_compile_opts — issue #180) - assert.doesNotThrow(() => compile('Hello\n', { basePath: '.' })); + // basePath is now also accepted by the wrapper (reconciled with napi + // parse_compile_opts — issue #180). Backend-aware: see the helper above. + assertWrapperAcceptsBasePath(() => compile('Hello\n', { basePath: '.' }), 'U-OV-8 compile'); }); test('U-OV-9: check accepts vars and basePath without error', () => { assert.doesNotThrow(() => check('Hello\n', { vars: {} })); - // basePath is now also accepted (reconciled with napi parse_check_opts — issue #180) - assert.doesNotThrow(() => check('Hello\n', { basePath: '.' })); + // basePath is now also accepted by the wrapper (reconciled with napi + // parse_check_opts — issue #180). Backend-aware: see the helper above. + assertWrapperAcceptsBasePath(() => check('Hello\n', { basePath: '.' }), 'U-OV-9 check'); }); test('U-OV-10: no options does not throw', () => { @@ -289,19 +345,15 @@ describe('options-validation', () => { test('U-OV-15: compile now accepts basePath (reconciled with napi parse_compile_opts — issue #180)', () => { // napi's parse_compile_opts has always accepted basePath; the wrapper was wrong to // reject it. After the fix, the wrapper no longer intercepts it. - assert.doesNotThrow( - () => compile('Hello\n', { basePath: '.' }), - 'compile must not throw invalid_options for basePath after wrapper reconciliation', - ); + // Backend-aware per OD-1 — see assertWrapperAcceptsBasePath above. + assertWrapperAcceptsBasePath(() => compile('Hello\n', { basePath: '.' }), 'U-OV-15'); }); test('U-OV-16: check now accepts basePath (reconciled with napi parse_check_opts — issue #180)', () => { // napi's parse_check_opts has always accepted basePath; the wrapper was wrong to // reject it. After the fix, the wrapper no longer intercepts it. - assert.doesNotThrow( - () => check('Hello\n', { basePath: '.' }), - 'check must not throw invalid_options for basePath after wrapper reconciliation', - ); + // Backend-aware per OD-1 — see assertWrapperAcceptsBasePath above. + assertWrapperAcceptsBasePath(() => check('Hello\n', { basePath: '.' }), 'U-OV-16'); }); // ── basePath passthrough on file methods: purposeful rejection (issue #74) ── @@ -954,4 +1006,39 @@ describe('options-validation', () => { 'DEFAULT_COMPILE_OPTS must have shape { filename: "input.mds", modules: {} }', ); }); + + // ── checkFile WASM-path forwarding invariant (avoids PF-004 / #180 bug class) ── + + test('U-OV-35: METHOD_KEYS.checkFile is a subset of METHOD_KEYS.compileFile (WASM checkFile forwarding invariant)', () => { + // On the WASM backend, node.ts checkFile routes through prepareFileArgs → fileOpts, + // which forwards using METHOD_KEYS.compileFile — NOT METHOD_KEYS.checkFile + // (see the cast comment at node.ts:114-117). That is correct only while every + // checkFile key is also a compileFile key. + // + // If CheckFileOptions ever gains a key that FileOptions lacks, assertKnownKeys + // would ACCEPT it on checkFile while the WASM path silently DROPPED it — + // reintroducing the exact #180 bug class (validated-then-discarded) on one backend + // only. The native path is unaffected, so a native-only test suite would not catch + // it. This assertion converts that silent drift into a loud failure (avoids PF-004: + // an alternate code path silently bypassing the authoritative key list). + const missing = METHOD_KEYS.checkFile.filter((k) => !METHOD_KEYS.compileFile.includes(k)); + assert.deepStrictEqual( + missing, + [], + `METHOD_KEYS.checkFile keys absent from METHOD_KEYS.compileFile: [${missing.join(', ')}]. ` + + 'Either add them to FileOptions, or give checkFile its own forwarding path in ' + + 'node.ts prepareFileArgs/fileOpts — otherwise the WASM backend drops them silently.', + ); + + // Positive controls (avoids PF-013): an empty-vs-empty comparison passes vacuously. + // Prove both operands are non-empty and that the filter can actually report a key + // as missing, so the assertion above is capable of failing. + assert.ok(METHOD_KEYS.checkFile.length > 0, 'METHOD_KEYS.checkFile must be non-empty'); + assert.ok(METHOD_KEYS.compileFile.length > 0, 'METHOD_KEYS.compileFile must be non-empty'); + assert.deepStrictEqual( + METHOD_KEYS.checkFile.filter((k) => !['__no_such_key__'].includes(k)), + [...METHOD_KEYS.checkFile], + 'positive control: the subset filter must be able to report keys as missing', + ); + }); });