diff --git a/CHANGELOG.md b/CHANGELOG.md index f1177286..a0ab0e8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,128 @@ 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 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`. + +### 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 eight lint types (`LintDiagnostic`, `LintFileOptions`, `LintFileReport`, + `LintOptions`, `LintResult`, `LintRuleName`, `LintSpan`, `RuleSeverity`) and the + `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 + 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** — 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 **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 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) + +`FileOptions` (used by `compileFile`) was previously declared as +`interface FileOptions extends CompileOptions`. This inheritance was an error: +`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 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) + +`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 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) + +`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` 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..5877bcd5 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`) @@ -102,35 +104,78 @@ 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. +// 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) +// basePath: required when the source contains @import or @extends. +// 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 { 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() // 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 @@ -139,10 +184,13 @@ 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 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`) **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/__test__/browser.spec.mjs b/packages/mds/__test__/browser.spec.mjs index 2600d133..94904283 100644 --- a/packages/mds/__test__/browser.spec.mjs +++ b/packages/mds/__test__/browser.spec.mjs @@ -19,10 +19,13 @@ import { check, getBackend, isMdsError, + lint, + lintVirtual, _resetForTesting as browserReset, _initWithModuleForTesting, } from '../dist/browser.js'; import { initWasmNode, _resetForTesting as wasmReset } from '../dist/backend/wasm.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. @@ -30,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(); }); // --------------------------------------------------------------------------- @@ -43,6 +48,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 +108,31 @@ 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')); + // 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, + `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 +166,171 @@ 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); + // 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.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-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 + // 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); + assert.deepStrictEqual( + browserResult, + nodeResult, + 'browser (WASM) and node lint must return byte-identical results for the same source', + ); + }); + + 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( + () => 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__/compile.spec.mjs b/packages/mds/__test__/compile.spec.mjs index 0cd45c90..9305bdea 100644 --- a/packages/mds/__test__/compile.spec.mjs +++ b/packages/mds/__test__/compile.spec.mjs @@ -56,8 +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 — varsOpt uses != null so both null - // and undefined are omitted from the options passed 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); diff --git a/packages/mds/__test__/options-validation.spec.mjs b/packages/mds/__test__/options-validation.spec.mjs index ad3741ff..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 @@ -15,6 +21,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, @@ -25,8 +32,9 @@ import { lintVirtual, isMdsError, init, + getBackend, } 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'; @@ -129,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', () => { @@ -288,68 +345,510 @@ 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) ── + // + // 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-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 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) ───── + + // 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'); + } + + // 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)', () => { + 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 () => { + // 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. + 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); + + // 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 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 }, + 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 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 = {}; + 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', + call: () => be.compile('', fullOpts('compile')), + expected: fullOpts('compile'), + }, + { + name: 'check', + call: () => be.check('', fullOpts('check')), + expected: fullOpts('check'), + }, + { + name: 'compileFile', + call: () => be.compileFile('/any.mds', fullOpts('compileFile')), + expected: fullOpts('compileFile'), + }, + { + name: 'checkFile', + call: () => be.checkFile('/any.mds', fullOpts('checkFile')), + expected: fullOpts('checkFile'), + }, + { + name: 'lint', + call: () => be.lint('', fullOpts('lint')), + expected: fullOpts('lint'), + }, + { + name: 'lintFile', + call: () => be.lintFile('/any.mds', fullOpts('lintFile')), + expected: fullOpts('lintFile'), + }, + { + name: 'lintVirtual', + call: () => be.lintVirtual({ 'a.mds': '' }, 'a.mds', fullOpts('lintVirtual')), + expected: fullOpts('lintVirtual'), + }, + ]; + + 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`, + ); + } }); - // ── basePath passthrough on file methods (issue #74) ────────────────────── + // ── cross-backend message equality for file basePath (AC-P3-06 / U-OV-27) ─ - 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. - const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-test-')); + 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-')); 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 ''; + } + + const { execFileSync } = await import('node:child_process'); + try { - let errorMsg = ''; - try { - await compileFile(file, { basePath: '.' }); - } catch (e) { - errorMsg = e instanceof Error ? e.message : String(e); + for (const method of ['compileFile', 'checkFile']) { + // 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: '.' }), + ); + const napiMsg = await captureMsg( + () => (method === 'compileFile' ? addon.compileFile : addon.checkFile)(file, { basePath: '.' }), + ); + + 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}"`, + ); } - assert.ok( - !errorMsg.startsWith('unknown option key "basePath"'), - `wrapper must not intercept basePath for compileFile with a generic message; got: "${errorMsg}"`, - ); } 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. - const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mds-test-')); + // ── {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/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, 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', 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) }, + ]; + try { - let errorMsg = ''; - try { - await checkFile(file, { basePath: '.' }); - } catch (e) { - errorMsg = e instanceof Error ? e.message : String(e); + 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 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 { + wasmBackendStr = execFileSync(process.execPath, ['--input-type=module'], { + input: script, + cwd: PKG_DIR, + env: { ...process.env, MDS_BACKEND: 'wasm' }, + timeout: 15000, + encoding: 'utf8', + }); + } 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, + `${name}: native (threw=${nativeThrew}) and WASM (threw=${wasmThrew}) must agree on {basePath: undefined}`, + ); } - assert.ok( - !errorMsg.startsWith('unknown option key "basePath"'), - `wrapper must not intercept basePath for checkFile with a generic message; got: "${errorMsg}"`, - ); } finally { fs.rmSync(tmp, { recursive: true, force: true }); } }); + // ── 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(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}"`, + ); + } + }); + // ── prototype-chain safety (issue #18 regression prevention) ───────────── test('U-OV-19: prototype-chain method names are handled without TypeError (issue #18)', () => { @@ -398,4 +897,148 @@ describe('options-validation', () => { '__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 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 + // 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', + ); + + // 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: {} }', + ); + }); + + // ── 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', + ); + }); }); 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 }); diff --git a/packages/mds/__test__/types/consumer-browser.ts b/packages/mds/__test__/types/consumer-browser.ts new file mode 100644 index 00000000..03140006 --- /dev/null +++ b/packages/mds/__test__/types/consumer-browser.ts @@ -0,0 +1,67 @@ +/** + * 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 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, + CompileOptions, + LintDiagnostic, + LintFileOptions, + LintFileReport, + LintOptions, + LintResult, + LintRuleName, + LintSpan, + RuleSeverity, + SourceMapV3, +} from '../../dist/browser.js'; + +// ── 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: {} }; + +// ── Negative: basePath NOT accepted on LintFileOptions ──────────────────────── +// @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 }; +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'; + +// 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 _lintFileFromVar; +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 new file mode 100644 index 00000000..b71bec8e --- /dev/null +++ b/packages/mds/__test__/types/consumer-node.ts @@ -0,0 +1,117 @@ +/** + * 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, + MarkdownResult, + RuleSeverity, + SourceMapV3, +} 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' }; + +// ── 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 +// 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' } }; +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. +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.) +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 _fileFromCompileVar; void _checkFileFromVar; void _lintFileFromVar; +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/__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..caf9e503 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 { forwardOpts } 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,48 +22,47 @@ 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 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. */ +type NapiLintFileOpts = { vars?: Record; rules?: Record }; + /** * Shape of the napi addon exports. * compile/check accept { basePath?, vars?, sourceMap?, sourcesContent? } for string sources. - * compileFile/checkFile accept { vars?, sourceMap?, sourcesContent? } for file paths. - * lint/lintFile/lintVirtual accept { basePath?, vars?, rules? }. + * compileFile accepts { vars?, sourceMap?, sourcesContent? }; checkFile accepts { vars? } only. + * lint accepts { basePath?, vars?, rules? }; lintFile/lintVirtual accept only + * { vars?, rules? } — napi rejects `basePath` on both (parse_lint_file_opts / + * parse_lint_virtual_opts in crates/mds-napi/src/lib.rs). */ 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; + 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; } -/** 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. * @@ -85,43 +80,43 @@ 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, forwardOpts(options, 'compile')); 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, forwardOpts(options, 'check')); 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, forwardOpts(options, 'compileFile')); 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, forwardOpts(options, 'checkFile')); 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')); 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')); 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')); assertResultShape(result, 'lint'); return result as LintResult; }, diff --git a/packages/mds/src/backend/wasm.ts b/packages/mds/src/backend/wasm.ts index ed6e00b7..892b7fee 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 { forwardOpts } 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. + * 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,15 @@ function compileOpts( sourceMap?: boolean; sourcesContent?: boolean; } { - const extra = compileOpt(options); + // 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) { @@ -370,9 +380,12 @@ 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 } + // 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 } : DEFAULT_COMPILE_OPTS; } @@ -388,12 +401,36 @@ export function fileOpts( sourceMap?: boolean; sourcesContent?: boolean; } { - const extra = compileOpt(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 }; } +/** + * Throw `mds::invalid_options` when a caller passes `basePath` to a WASM-backend + * string-surface method (compile, check, lint). + * + * 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). + * + * 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 { + 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,29 +443,29 @@ export function fileOpts( export function createWasmBackend(wasmModule: WasmModule): MdsBaseBackend { return { compile(source: string, options?: CompileOptions): CompileResult { + // 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 { + // 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'); 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. - const opts: { - vars?: Record; - rules?: Record; - } = {}; - if (options?.vars != null) opts.vars = options.vars; - if (options?.rules != null) opts.rules = options.rules; + // 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). const result: unknown = wasmModule.lint( source, - Object.keys(opts).length > 0 ? opts : undefined, + forwardOpts(options, 'lint'), ); assertResultShape(result, 'lint'); return result as LintResult; @@ -439,16 +476,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'), ); assertResultShape(result, 'lint'); return result as LintResult; diff --git a/packages/mds/src/browser.ts b/packages/mds/src/browser.ts index e7dd1443..55277be7 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,20 @@ export type { CompileOptions, CompileResult, InitOptions, + LintDiagnostic, + LintFileOptions, + LintFileReport, + LintOptions, + LintResult, + LintRuleName, + LintSpan, MarkdownResult, - Message, - MessagesResult, MdsError, MdsErrorSpan, + Message, + MessagesResult, + RuleSeverity, + SourceMapV3, } from './types.js'; let resolvedBackend: MdsBaseBackend | undefined; @@ -52,7 +72,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 +95,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 +112,35 @@ 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. + * + * `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. Returns a LintResult with per-rule findings. + * 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 deleted file mode 100644 index 579d7a3f..00000000 --- a/packages/mds/src/index.ts +++ /dev/null @@ -1,27 +0,0 @@ -export type { - CompileResult, - MarkdownResult, - MessagesResult, - Message, - CheckResult, - CompileOptions, - FileOptions, - LintDiagnostic, - LintFileOptions, - LintFileReport, - LintOptions, - LintResult, - LintRuleName, - LintSpan, - RuleSeverity, - MdsErrorSpan, - MdsError, - BackendType, - InitOptions, - MdsBackend, - MdsBaseBackend, - MdsNodeBackend, -} 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 388f521f..c470023e 100644 --- a/packages/mds/src/node.ts +++ b/packages/mds/src/node.ts @@ -1,22 +1,23 @@ 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'; 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']; @@ -90,14 +91,31 @@ 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'); 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 { + // 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 + // 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'); return result as CheckResult; @@ -123,15 +141,19 @@ function wrapWithFileOps( // Build a copy of modules without the entry (lint() inserts it separately). const extraModules: Record = { ...modules }; delete extraModules[entryFilename]; - const lintOpts: { - filename: string; - modules?: Record; + // 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 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; - } = { 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; + } | undefined; + const lintOpts = { + ...forwarded, + filename: entryFilename, + ...(Object.keys(extraModules).length > 0 ? { modules: extraModules } : undefined), + }; const result: unknown = wasmModule.lint(entrySource, lintOpts); assertResultShape(result, 'lint'); return result as LintResult; @@ -252,19 +274,43 @@ 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: 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'); + if (options != null) { + assertKnownKeys(options, 'compileFile'); + // 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; + } 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 sync-throw contract as compileFile: basePath guard throws synchronously. */ -export function checkFile(path: string, options?: CheckOptions): Promise { - if (options != null) assertKnownKeys(options, 'checkFile'); +export function checkFile(path: string, options?: CheckFileOptions): Promise { + if (options != null) { + assertKnownKeys(options, 'checkFile'); + // 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; + } return assertReady().checkFile(path, options); } @@ -295,14 +341,13 @@ 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. +// `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, + CheckFileOptions, CheckOptions, CheckResult, CompileOptions, @@ -316,10 +361,11 @@ export type { LintResult, LintRuleName, LintSpan, - RuleSeverity, MarkdownResult, - Message, - MessagesResult, MdsError, MdsErrorSpan, + Message, + MessagesResult, + RuleSeverity, + SourceMapV3, } from './types.js'; diff --git a/packages/mds/src/types.ts b/packages/mds/src/types.ts index 9536ed98..fd72e970 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}. + * + * `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. + */ + 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,49 @@ 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. + * `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; + /** + * 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; +} + +/** + * Options for file-based check-only operations. + * + * `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. */ + 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; +} // --------------------------------------------------------------------------- // Lint types @@ -262,7 +315,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). + * + * 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; } @@ -280,6 +338,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. */ @@ -321,6 +385,11 @@ export interface InitOptions { /** * Browser-safe backend interface — compile/check/lint/lintVirtual/getBackend. * Does not include file operations (which require node:fs). + * + * 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. */ export interface MdsBaseBackend { compile(source: string, options?: CompileOptions): CompileResult; @@ -346,9 +415,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..b9633e66 100644 --- a/packages/mds/src/util/options.ts +++ b/packages/mds/src/util/options.ts @@ -1,14 +1,38 @@ -import type { CompileOptions, CheckOptions, FileOptions, LintOptions, LintFileOptions } from '../types.js'; +import type { + CheckFileOptions, + CheckOptions, + CompileOptions, + FileOptions, + LintFileOptions, + LintOptions, +} from '../types.js'; // ── 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 @@ -16,7 +40,7 @@ import type { CompileOptions, CheckOptions, FileOptions, LintOptions, LintFileOp * // → readonly ['basePath', 'vars', 'rules'] * ``` */ -function keysOf(witness: Record): readonly string[] { +function keysOf(witness: Record, true>): readonly string[] { return Object.keys(witness); } @@ -39,63 +63,130 @@ export type MethodName = | 'lintFile' | 'lintVirtual'; -// ── Internal backend option shapes ──────────────────────────────────────────── +// ── Options-for-method map ───────────────────────────────────────────────────── -// 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; -} +/** + * 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 ─────────────────────────────────────────────────────── /** * 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. 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` 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. When a caller + * passes `basePath` on these methods the wrapper emits a purpose-built rejection via + * {@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. */ -const METHOD_KEYS: Readonly> = { - compile: keysOf<_CompileBackendOpts>({ basePath: true, vars: true, sourceMap: true, sourcesContent: true }), - check: keysOf<_CheckBackendOpts>({ basePath: true, vars: true }), +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 }), - 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 }), }; +// ── basePath error factory for file-surface methods ──────────────────────────── + +/** + * 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. 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( + '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` 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. + * 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: 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: + * 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: ReadonlySet = new Set([ - 'compileFile', - 'checkFile', -]); +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_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 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). + * @param method - Public method name. + */ +export function getBasePathError( + options: object, + method: MethodName, +): (Error & { code: string }) | undefined { + const factory = BASEPATH_REJECTORS.get(method); + if (!factory) return undefined; + const basePath = (options as Record)['basePath']; + return basePath != null ? factory() : undefined; +} // ── Main validator ───────────────────────────────────────────────────────────── @@ -105,9 +196,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; open issue #180). + * 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. @@ -123,9 +214,10 @@ 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). - if (BASEPATH_PASSTHROUGH.has(method) && k === 'basePath') return false; + // 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_REJECTORS.has(method) && k === 'basePath') return false; return !known.includes(k); }); if (unknowns.length === 0) return; @@ -142,33 +234,53 @@ export function assertKnownKeys(options: object, method: MethodName): void { throw err; } -/** - * Build the `{ vars }` sub-object only when `options.vars` is defined and non-null. - * - * 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. - */ -export function varsOpt( - options?: { vars?: Record }, -): { vars: Record } | undefined { - return options?.vars != null ? { vars: options.vars } : undefined; -} +// ── Option forwarding ────────────────────────────────────────────────────────── /** - * Build the options object for compile/compileFile, forwarding vars, - * sourceMap, and sourcesContent when present and non-null. + * Forward `options` to the backend, keeping only the keys listed in + * {@link METHOD_KEYS} for `method`. * - * Returns `undefined` when no options are set so the backend receives no - * options argument (avoids allocating a needless empty object on the hot path). + * 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. 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 `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). + * + * @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 compileOpt( - options?: CompileOptions | FileOptions, -): { vars?: Record; sourceMap?: boolean; sourcesContent?: boolean } | undefined { +export function forwardOpts( + options: OptionsFor[M] | null | undefined, + method: M, +): Partial | undefined { if (options == null) return undefined; - const out: { vars?: Record; sourceMap?: boolean; sourcesContent?: boolean } = {}; - 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; - return Object.keys(out).length > 0 ? out : 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) { + // The accumulation into Record loses per-key types + // 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; } 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"] +}