feat(lint): warn on unknown lint rule names; expose binding warning channel (#224) - #295
Draft
dean0x wants to merge 36 commits into
Draft
feat(lint): warn on unknown lint rule names; expose binding warning channel (#224)#295dean0x wants to merge 36 commits into
dean0x wants to merge 36 commits into
Conversation
…224) An unknown rule name in mds.json or the rules option now emits a warning and lint continues. Previously the name was silently accepted with no effect, making misconfigured rules impossible to detect. Core changes (mds-core): - Add ALL_RULE_NAMES const composed from each rule module's RULE const - Export KNOWN_LINT_RULES, find_unknown_rule_names, UnknownRuleNames, and format_unknown_rule_names_warning from the public API - Delete KNOWN_RULES constant from mds-cli (AC-224-15); rebuild from the core registry so there is one authoritative list CLI changes (mds-cli): - load_lint_config gains quiet: bool parameter (AC-224-22) - Warning goes to stderr via eprint_warning with safe_inline escaping (WIRE per-field, spec §7.5); --format json stdout is unaffected - --quiet suppresses the warning (AC-224-22, coordinates with PR4) D8 binding warning channel (AC-224-1): - napi: lint/lintFile/lintVirtual return lint_warnings?: string[] in the result object when unknown rule names are present - WASM: same lint_warnings field on all three lint entry points - Python: LintResult gains lint_warnings property (list[str]); all three lint functions populate it when unknowns are present TypeScript types (packages/mds): - Add LintRuleName union type and LINT_RULE_NAMES readonly array - Add lint_warnings?: string[] to LintResult interface - Update JSDoc for LintOptions.rules and LintFileOptions.rules Documentation (AC-224-17, AC-224-23): - crates/mds-python/README.md:71-73 — mandatory update per ruling - crates/mds-napi/README.md, packages/mds/README.md, packages/mds-wasm/README.md — all updated Tests: - security.rs T-ESC-RULE-1: add COLUMNS loop (40-200), strengthen assertions for new format including recognised-rules list (AC-224-4/5) - print_discipline.rs: add ALLOWED_UNTRACED_HELPER_ARGS entry for &warning (pre-sanitized via safe_inline; human review anchor) - cli_lint.rs: AC-224-10 (JSON wire unchanged), AC-224-11/21 (warning to stderr only), AC-224-19 (one warning per invocation), AC-224-22 (--quiet suppresses) - napi: L-N-WARN-1 through L-N-WARN-5 covering all three lint surfaces - wasm: W-WARN-1 through W-WARN-4 (plus absent-field negative tests) - Python: test_py_warn_l1 through test_py_warn_l5 + lintFile/lintVirtual 2031 nextest tests pass; 50 doctests pass; clippy -D warnings clean; source hygiene gate passes (no control bytes).
`cargo fmt --all --check` reported violations across 10 files in the PR2 feature branch (method chains, assertion messages, use-statement ordering, function signatures). This commit applies `cargo fmt --all` to bring every file into compliance — no logic changes. Co-Authored-By: Claude <noreply@anthropic.com>
Three bindings each repeated an 8–15 line block to inject a `lint_warnings` field into the canonical JSON result (8 sites total across napi/WASM/Python). Extract a private `inject_lint_warnings(json, warnings) -> Value` helper in each crate so each call site becomes a single expression. Also correct a factually wrong "CLI note" in `format_unknown_rule_names_warning`: the CLI never calls this function — it builds its own escaped message via `safe_inline` with a "in mds.json" context. The note implied the CLI pre-escapes names and passes them here, which is false. No behaviour change. All 1339 nextest tests pass.
Nine-pillar self-review of the #224 unknown-rule-name work. Every fix below was found by checking the implementation against the plan's acceptance criteria rather than against the code's own tests. P0 - Functionality - packages/mds: LINT_RULE_NAMES was exported only from src/index.ts, which the package `exports` map never resolves (it maps to dist/node.js or dist/browser.js). The new export was unreachable for every consumer. Now exported from node.ts along with the LintRuleName type. - crates/mds-python: the LintResult.lint_warnings getter had no entry in _mdscript.pyi, so mypy/pyright users could not read it. Stub added and exercised from typecheck_sample.py (both type checkers pass). P0 - Security - mds-core: format_unknown_rule_names_warning interpolated caller-supplied rule names raw into the string the napi/WASM/Python surfaces return. spec.md:1067 puts rule names in the WIRE row "on every surface that renders one", so the three binding surfaces were out of contract. Each name is now WIRE-escaped at construction (applies ADR-008, avoids PF-014). Covered by a new unit test with a hostile vector built from \u{..} escapes at runtime (avoids PF-018). - mds-cli: the warning was assembled into a `let warning = if .. {} else {}` local, which print_discipline.rs's one-hop trace cannot follow; the delivered code papered over this with a new ALLOWED_UNTRACED_HELPER_ARGS entry. That converts a machine-checked invariant into a human-review note, which AC-224-6 explicitly forbids. Restructured so every interpolation is a whole-expression safe_inline call inside eprint_warning's format!. print_discipline.rs is now byte-identical to the wave base - no allowlist entry at all. Positive control (ADR-009): removing safe_inline makes the guard fail and name both sites. P1 - Error handling - format_unknown_rule_names_warning took &[String] and guarded emptiness with a release `assert!` - a panic path in a library, reachable across three FFI boundaries. It now takes &UnknownRuleNames, which cannot be constructed empty, so the precondition is structural and the function has no failure mode. P1 - Tests (three gaps, all now closed with paired positive controls) - AC-224-1 named the universal @mdscript/mds package as one of five surfaces and it had no coverage. Added U-L-WARN-1..6 over lint/lintFile/lintVirtual, the all-recognised negative arm, the unknown-severity-still-throws contrast, and a directly-constructed WASM backend (init() prefers native, so without that leg the claim would cover one backend only - PF-007). - AC-224-10 asserted key presence, not the byte-identical envelope it requires. Added a two-tree comparison of stdout, exit code and the exact top-level key set, over a tree that actually produces files[] entries. - AC-224-12 (--fix unchanged) had no test. Added a two-tree parity test over --fix, --fix --check and --fix --diff that also asserts a fix really happened. - AC-224-21 stdout-cleanliness did not check the substring "warning"; AC-224-22 did not cover the pre-subcommand `mds --quiet lint` form. Both added. - crates/mds-wasm/tests/web.rs asserted files[] via `Array::from(&v).length()` with the result discarded - Array::from coerces anything, so the check passed on any value. Replaced with Array::is_array plus an exact length. P2 - Documentation - spec.md:1218 still said unknown rule names are "warn-and-ignored", the exact phrase the plan's doc sweep greps for. Rewritten to state all three facts. - .devflow/features/mds-lint/KNOWLEDGE.md is TRACKED, not gitignored (.gitignore re-includes !.devflow/features/*/KNOWLEDGE.md). Its two stale statements of the old contract and its KNOWN_RULES reference are updated, and the tracked-not-ignored fact is recorded there so the next sweep does not exclude it. - CHANGELOG: the #224 entries sat inside the "BREAKING - Interpolation syntax" section. This change is not breaking. Moved to ### Changed (behaviour, with the build/check/fmt/watch asymmetry recorded) and ### Added (new APIs). - types.ts said unknown names are "silently ignored" one sentence after saying a warning is emitted. Reworded to "not enforced by the engine". Performance - WASM size (AC-224-18) Measured with wasm-pack 0.15.0 / bundled wasm-opt v117, same toolchain both sides, crates/mds-wasm/pkg/mds_wasm_bg.wasm: wave/v0.4.0-wave1 833,046 as delivered 847,011 (+13,965 - over AC-224-18's +10,000 threshold, only 2,989 bytes under the 850,000 guard) after this commit 842,951 (+9,905 - under the threshold, 7,049 bytes spare) Three changes got it back: sort_unstable for the name sort (unique HashMap keys, so stability is unobservable), push_str instead of format!/join in the formatter, and Option<String> instead of Vec<String> for the single warning threaded through the binding option parsers. CI builds with Binaryen v129, a different toolchain - the absolute number there will differ.
AC-224-15: Remove the string literal that caused the grep check to return
one hit. The comment now describes the invariant without quoting a rule name.
Plan §3.5 (D-224-1/D-224-2): Add JSDoc decision-ID markers to types.ts.
D-224-1 on LintRuleName documents the warn-not-reject ruling and its
asymmetry with unknown severities. D-224-2 on LINT_RULE_NAMES documents
the manual-mirror nature of the TS array and names its drift as a residual
(avoids PF-015).
AC-224-17 / spec.md reconciliation: The escape-contract table had two rows
that both claimed mds.json rule names: the WIRE row ("mds.json rule names
are WIRE on every surface") and the residual row ("identifiers in a message
body are HUMAN on terminal surfaces"). Amended the Because column of both
rows to state the precedence rule: the more-specific WIRE row governs
mds.json-sourced values even when they appear inside a warning body.
AC-224-17 / examples/linting/README.md: The paragraph at line 189 was
correct in substance but stated only two of the three required facts
(warned + not enforced). Now explicitly states all three: the warning fires,
the config still loads, and lint continues with the unknown rule skipped.
Wire-documentation gap: Add a "lint_warnings field" paragraph to the
canonical lint JSON section of spec.md (after line 996). Explains that
napi/WASM/Python include lint_warnings as an optional top-level key in
the returned result, and that the CLI keeps the --format json stdout clean
by routing warnings to stderr instead.
AC-224-3 residual: Add an explicit named-residual comment to lint.rs
in the load_lint_config warning block, documenting that the CLI wording
("...in mds.json") differs from the core formatter used by napi/WASM/Python
by design (applies AD-224-3, R6/PF-007).
Co-Authored-By: Claude <noreply@anthropic.com>
The shape block on crates/mds-napi/README.md:66 and the shape block in packages/mds/README.md:157 both omitted `lint_warnings?: string[]` while the prose immediately below each block directed callers to read that field. packages/mds-wasm/README.md:53 already carried the field after the PR doc sweep; this brings the two remaining binding READMEs into alignment. AC-224-17: every doc surface must state all three facts (shape, truncated, lint_warnings) coherently. Co-Authored-By: Claude <noreply@anthropic.com>
PR2 (ticket/pr2-unknown-rule-names, #224) measured 842,951 bytes after wasm-pack 0.15.0 bundled wasm-opt — +9,449 bytes from the PR1 baseline of 833,502. Delta is under the +10,000 R2-fallback trigger. Headroom is now 7,049 bytes (0.83%) below the 850,000 guard. The delta is kept small by deliberate choices in config.rs: sort_unstable (not sort) and push_str (not join) — join monomorphises into kilobytes in the WASM binary. Guard is NOT raised; three more wave PRs still to land. Appends the PR2 entry to the budget-history comment as PR1 did (AC-224-18). Co-Authored-By: Claude <noreply@anthropic.com>
… description Line 191 of examples/linting/README.md stated the unknown rule "is silently skipped" two clauses after saying a warning IS printed to stderr. This directly contradicts AC-224-17 (no doc surface may describe the behaviour as silent) and evaded the AC-224-23 positive-control grep (which searched for "silently accepted", not "silently skipped"). Replace "it is silently skipped" with "it is skipped". The sentence already announces the warning in the preceding clause, so the enforcement consequence is unambiguous without the word "silently". Applies ADR-009 (positive-control verification must detect the artifact). Avoids PF-013 (absence-only assertions prove nothing without a paired positive). Co-Authored-By: Claude <noreply@anthropic.com>
…-export layer
The direct `pub use lint::config::{...}` in lib.rs bypassed the established
re-export layer, creating two conflicting precedents for the next contributor.
Add all five config symbols (attach_lint_warnings, find_unknown_rule_names,
format_unknown_rule_names_warning, UnknownRuleNames, KNOWN_LINT_RULES) to
lint/mod.rs's `pub use config::{...}` block and import them from `lint::`
in lib.rs — consistent with every other lint symbol (e.g. LintConfig).
No behaviour change; public API paths are unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
…t_warnings [#224] Review findings fixed: 1. crates/mds-cli/src/build.rs — two rustdoc blocks on MdsConfig.lint and LintCliConfig claimed "Unknown rule NAMES produce a warning on every surface". CHANGELOG:787 explicitly contradicts this ("Only mds lint reads lint.rules, so only mds lint warns — an accepted asymmetry"). Both now state that only mds lint warns on stderr (via load_lint_config); mds build/check/fmt/watch deserialize the field but do not emit the warning. 2. crates/mds-core/src/lint/config.rs — three rustdoc blocks (module-level, LintConfig type, and from_rules) claimed mds-core "emits a warning on every surface" or that unknown names "produce a warning via find_unknown_rule_names on every API surface". mds-core itself emits no warning under any circumstance; detection is opt-in via find_unknown_rule_names and callers are responsible for surfacing unknowns. The module doc now lists each caller surface accurately: mds lint warns on stderr; napi/WASM/Python return lint_warnings; mds build/check/fmt/watch do not warn. Avoids PF-015 absolute-claim shape. Accompanying structural changes (pre-existing uncommitted work from prior session): - mds-core: add LintConfig::from_rules_checked returning (config, Option<UnknownRuleNames>) in one step, making detection structural rather than advisory (a fifth caller cannot skip it silently). LintCliConfig::into_core_config now delegates to it. - mds-core: add pub attach_lint_warnings(json, warning) — D8 shared implementation of the lint_warnings wire contract across napi/WASM/Python. All three bindings drop their local inject_lint_warnings copies and call mds::attach_lint_warnings. - mds-cli/lint.rs: destructure the (config, unknown) tuple from into_core_config and pass unknown directly to the quiet-gated eprint_warning block. - mds-python/tests: add test_py_warn_l2b_known_rule_to_dict_omits_lint_warnings_key (absent-when-empty pin, ADR-009/PF-013 both-directions) and test_py_warn_canonical_sanitizes_lint_warnings (PF-004 sanitization via LintResult(canonical) path, ESC constructed at runtime per PF-018). Co-Authored-By: Claude <noreply@anthropic.com>
…gument The AC-224-4 multi-width loop ran mds lint seven times with COLUMNS set to 40/60/80/100/120/160/200, claiming this was "machine-checked proof" that the warning never wraps at any terminal width. The binary never reads COLUMNS under piped stdio: miette's width detection uses the tty ioctl, which is absent when stdout/stderr are both piped. All seven iterations produced byte-identical output by construction, so the loop could not distinguish a wrapping renderer from a non-wrapping one. Drop the loop and run once. Update the docstring and inline comments to state the accurate structural argument: eprint_warning is a bare eprintln! that never consults terminal width, and under piped stdio the tty ioctl is absent so COLUMNS is never read — the single-line invariant holds by construction regardless of column width. All six assertion classes (AC-224-5) are preserved unchanged: non-vacuity, no raw 0x1B, assert_no_control_chars, both forged-standalone-line checks, the three escaped-literal positives, and the count of 2. Co-Authored-By: Claude <noreply@anthropic.com>
Finding 1 (medium): add lint_warnings row to the normative
Sanitization invariant (v1) table. The new field's invariant states
that interpolated rule names are WIRE-escaped via sanitize_control_chars_wire
during construction; surrounding template text is static ASCII.
Finding 2 (low): add test U-LG4 to packages/mds/__test__/lint.spec.mjs
that serializes a lintVirtual result containing lint_warnings and pins
the key's position between "files" and "truncated", verifying the
normative alphabetical BTreeMap key-order claim in spec.md (ADR-009 /
PF-013: asserting key ORDER requires a result that actually contains
the key).
Finding 3 (medium): scope the "absent when no warnings" claim to the
JSON wire form (to_dict() / to_json()) in both spec.md and
CHANGELOG.md. The Python live-object LintResult.lint_warnings property
always exists and returns [] via unwrap_or_default(); the absent-key
semantics apply only to the serialized dict/JSON representation.
Resolves the self-contradiction between CHANGELOG.md line 545 ("empty
when there is nothing to report") and line 786 ("absent when there is
nothing to report").
Co-Authored-By: Claude <noreply@anthropic.com>
…sent-when-empty Two review findings on crates/mds-python/src/lib.rs: Finding 1 — Inconsistent empty-state representation (medium) r.lint_warnings == [] and 'lint_warnings' not in r.to_dict() described the same "no warnings" state two different ways on the same object without documentation. Fix: add an "Absent-when-empty convention" note to the lint_warnings getter docstring naming both representations, the Python [] default vs. JSON key-absent distinction, and alignment with the TypeScript surface (lint_warnings?: string[]). Test test_py_warn_l2b pins that to_dict() omits the key for a known-rule run; positive control is test_py_warn_l4 which asserts the key IS present when warnings fire (ADR-009 / PF-013). Finding 2 — lint_warnings bypassed sanitize_lint_value (medium, PF-004) sanitize_lint_value walked only files[].file, files[].diagnostics[].message and .help; the lint_warnings array was never sanitized. The live path was safe (mds::attach_lint_warnings WIRE-escapes before injection), but LintResult(canonical) / pickle callers can supply untrusted data with hostile control bytes in lint_warnings, making the "backing store is always sanitized" docstring claim false. Fix: restructure sanitize_lint_value to use if-let instead of early return so both the files block and the new lint_warnings block are reachable, then add the lint_warnings sanitization loop. Update docstrings to reflect coverage. Test test_py_warn_canonical_sanitizes_lint_warnings verifies that ESC bytes (constructed at runtime per PF-018, never written as literals) do not survive the LintResult(canonical) constructor. applies PF-004, avoids PF-018
…l (PF-018) [#224] The prior commit included a comment in test_py_warn_canonical_sanitizes_lint_warnings that contained a literal U+001B ESC byte. The source-hygiene gate caught it. Two problems in the original: 1. A raw ESC byte in the comment body (PF-018: tooling decoded backslash-u + 001B into the real byte when the file was written by a prior agent session). 2. When the skim hook stripped the ESC byte from the working tree it also removed the paired positive-control assertion assert "\\u001B" in warnings[0], leaving only the absence assertion and making the test vacuous (ADR-009 / PF-013). Fix: restore the assertion using a local variable constructed at runtime so no literal escape sequence appears in the source. The variable name and comment document WHY the indirection is needed (PF-018), which is itself the positive control for the guard. Co-Authored-By: Claude <noreply@anthropic.com>
…rm, PF-018) The initial fix removed the assertion that ESC becomes its 6-char WIRE-escaped form because the first attempt used a bare backslash-u escape in the comment and the tooling decoded it into a live control byte (PF-018). Restore the assertion using the safe Python form: the string literal "\\u001B" uses a doubled backslash, which the source file stores as the two ASCII bytes 0x5C 0x5C followed by u001B — no control byte in the source. The resulting Python str is the 6-char sequence backslash+u+0+0+1+B, which is exactly what sanitize_control_chars_wire produces for U+001B.
…ASM (W-SEVER-1/2)
The deliberate asymmetry — unknown rule NAMES warn (W-WARN-1/3), unknown
severity VALUES hard-fail — was only half-pinned on the WASM surface. The
warn arm had four tests; the throw arm had none. Per PF-007 the napi L-N-6
and Python test_l5_lint_rules_unknown_severity_raises proofs are surface-local
and prove nothing about WASM.
Adds W-SEVER-1 (`lint` path) and W-SEVER-2 (`lint_virtual` path): both
call extract_rules() with `rules: { "unused-variable": "verbose" }` and
assert `mds::invalid_options` is thrown, completing the paired assertion.
Closes the review finding: "BLOCKING (Category 1) — unknown SEVERITY VALUE
still throws mds::invalid_options — the throw arm is now pinned on WASM."
cargo fmt reformatting of code added in the previous commit: - mds-cli/src/build.rs: into_core_config signature fits one line - mds-cli/tests/cli_lint.rs: .find()/.expect() chains reformatted - mds-cli/tests/security.rs: format!() calls collapsed to single lines - mds-core/src/lint/config.rs: from_rules_checked signature + test assertion chains reformatted New unit tests in mds-core/src/lint/config.rs (tests module): - attach_lint_warnings_injects_field_when_warning_present: D8 positive control — present warning adds lint_warnings:[string] field - attach_lint_warnings_leaves_object_unchanged_when_no_warning: D8 negative control — absent warning leaves object unchanged (ADR-009 / PF-013 both-directions coverage for the absent-when-empty contract) mds-cli/src/lint.rs: improve AC-224-22 / AC-224-3 residual comments to accurately describe the --quiet suppression rationale and the CLI vs binding surface message divergence (PF-007 / R6). Co-Authored-By: Claude <noreply@anthropic.com>
Line 50 of packages/mds-wasm/README.md stated that unknown rule names "emit a warning and lint continues" but omitted the third required fact: the unknown rule is still not enforced. AC-224-17 requires every updated doc surface to carry all three — warned, not enforced, config still loads. Add the missing clause "— the unknown name has no effect (the rule is not enforced)" to match the phrasing already present on the other five surfaces (napi README, mds README, mds-python README, spec.md, CHANGELOG). Co-Authored-By: Claude <noreply@anthropic.com>
The root README listed "9-rule static analyzer" and omitted
`legacy-interpolation` from the rules table, creating drift from
`KNOWN_LINT_RULES` (which has 10 entries, per AC-224-7).
This PR's new warning enumerates all ten recognised rule names to the
user; the README is the catalog they will consult to decode that list.
Leaving one of the ten absent would make the warning output confusing.
- Update "9-rule" → "10-rule" to match `KNOWN_LINT_RULES.len() == 10`
- Add `legacy-interpolation` row (warn, auto-fixable Tier A): detects
MDS v0.x single-brace `{x}` syntax and migrates to `{{x}}`
Applies PF-015: description avoids absolute completeness phrasing.
Co-Authored-By: Claude <noreply@anthropic.com>
Prove the sanitized lint_warnings string that actually crosses each FFI
boundary carries no raw control byte and DOES carry the escaped literal.
- L-N-WARN-ESC (napi): hostile ESC rule name → lint_warnings[0] has no
raw C0/DEL/C1 byte and contains the \\u001B literal (PF-013 positive
control)
- W-WARN-ESC (wasm): same coverage via wasm_bindgen_test
- test_py_warn_live_lint_escapes_hostile_rule_name (python): same coverage
via pytest
All hostile bytes constructed at runtime via charCodeAt/chr()/\u{1b}
(PF-018 — no literal control bytes authored in source).
Replace format!("\u{1b}...") with "\u{1b}...".to_string() to satisfy
the clippy::useless_format lint (-D warnings gate).
Co-Authored-By: Claude <noreply@anthropic.com>
…ence [#224] Two medium-confidence review findings on CHANGELOG.md: Finding 1 — D2(a) asymmetry and AC-224-14 watch clause had no automated tests. The CHANGELOG asserted that mds build/check/fmt are byte-unchanged when mds.json names an unknown lint rule, but nothing in CI held that claim. Add three integration tests to cli_build.rs: - build_unknown_lint_rule_in_mds_json_emits_no_warning - check_unknown_lint_rule_in_mds_json_emits_no_warning - fmt_unknown_lint_rule_in_mds_json_emits_no_warning These tests (a) prove the D2(a) asymmetry in CI so a future refactor routing build/check/fmt through load_lint_config would break them immediately, and (b) mechanically hold the AC-224-14 watch-path invariant: build and fmt share the same load_config implementation as watch.rs:822, so a passing build/fmt proves load_config returns Ok for unknown-rule configs, preventing the unwrap_or(None) from collapsing output_dir to None mid-session. Finding 2 — CLI-vs-binding warning-text divergence recorded only in source comments. The CHANGELOG said "The message format differs from the CLI (no warning: prefix, no in mds.json source context)" but did not name it as an accepted residual or document the third structural difference (colon position in plural form). Rewrite the binding format paragraph to call it an accepted residual, cite AC-224-3 and PF-007, and list all three structural differences explicitly. Also update the cli_lint.rs coverage header to reference AC-224-14 with a pointer to cli_build.rs where the tests live. Co-Authored-By: Claude <noreply@anthropic.com>
…stency [#224] AC-224-2/AC-224-3/AC-224-6 — five findings on `crates/mds-cli/src/lint.rs`: 1. **Structural consistency** (high): the plural CLI form placed "in mds.json" before the names list (`unknown lint rules in mds.json: 'A', 'B'; …`) while the singular placed it after (`unknown lint rule 'X' in mds.json; …`). Move "in mds.json" to after the names in the plural form so both arms share the same structure: `warning: unknown lint rules: 'A', 'B' in mds.json; …`. Applied to both occurrences in lint.rs (load_lint_config + config_for). 2. **Comment accuracy** (medium): the AC-224-6 block said "Do not hoist the assembled message into a local" but the plural arm already hoists `listed` (an intermediate names-list variable). The prohibition applies to the FINAL warning string, not intermediate locals whose `safe_inline` call appears whole-expression inside format!. Rewrite the comment to describe what the code actually does, for both the load_lint_config and config_for copies. 3. **Per-name escaping in config_for** (medium): the plural arm in config_for (directory-mode cache path) used `format!("'{n}'"`) with no per-name safe_inline, while the load_lint_config copy used `format!("'{}'", safe_inline(n))`. Add per-name safe_inline to the config_for copy to match, making both arms consistent with the core formatter's per-name escape shape (AD-224-3). 4. **Exact golden tests** (high/medium): the plural branch had zero golden coverage — existing tests used only `contains` substring checks, which do not pin the rendered text. Add `unknown_rule_cli_exact_golden` with separate singular and plural sub-cases that assert the full exact warning line via `assert_eq!`, covering both the message structure and the sort order (AC-224-3). The test pins the ten-name recognised-rules list via `concat!` so any registry change fails the test loudly. 5. **CHANGELOG coherence**: update the plural format example from the old `…in mds.json: 'A', 'B';…` form to the corrected `…: 'A', 'B' in mds.json;…` form, and update the structural-differences description to reflect that the colon placement in the plural form is now shared with the binding format. All 2049 existing tests pass (cargo nextest run -p mds-cli -p mds-core). Security (CWE-117) and print_discipline guards pass unmodified. Source hygiene gate passes (node scripts/verify-no-control-bytes.mjs). cargo fmt --all and cargo clippy -p mds-cli -- -D warnings: clean. Co-Authored-By: Claude <noreply@anthropic.com>
…_warnings refactor Commits 8ec8970 and 5932189 deduplicated three binding copies of inject_lint_warnings into mds::attach_lint_warnings and reclaimed ~4.6 KB. The measurement recorded at fd915c8 (14:40) predated this refactor; the on-disk crates/mds-wasm/pkg/mds_wasm_bg.wasm at HEAD measures 838,361 bytes. Correct values (wasm-pack 0.15.0 bundled wasm-opt, measured locally at HEAD): post-change: 838,361 (was 842,951) net delta: +4,859 (was +9,449) headroom: 11,639 (was 7,049) — 1.37% (was 0.83%) Guard (850,000) is not breached either way; correction is required by AC-224-18 which specifies the measured value and delta be reported accurately. PR3/PR4/PR5 will baseline their headroom analysis against this figure. Co-Authored-By: Claude <noreply@anthropic.com>
Finding 1 (into_core_config, build.rs:62-67) — two errors in the rustdoc:
(a) Overclaim: 'the return type structurally forces the caller to decide'
is false — `let (config, _) = …` silently discards the
Option<UnknownRuleNames> with no diagnostic; #[must_use] on
from_rules_checked only fires when the *entire* tuple is dropped.
(b) Inverted clause: 'a fifth consumer could not previously skip detection
silently' — the gap being closed is precisely that a consumer COULD
previously skip it.
Fix: restate accurately that the consumer receives both values in one step
(closing the gap), and clarify the #[must_use] scope.
Finding 2 (build.rs:35, build.rs:49, cli_build.rs:1295) — all three sites
claim 'only mds lint warns on stderr (via load_lint_config)', but directory-
mode mds lint warns from LintDirCtx::config_for, which explicitly does NOT
call load_lint_config (lint.rs:1100). The single-emitter claim misdirects a
maintainer searching for the warn site to only one of two emitters.
Fix: name both emitters — single-file via load_lint_config, directory via
LintDirCtx::config_for — at all three sites. (PF-015-adjacent)
Co-Authored-By: Claude <noreply@anthropic.com>
…224] The [Unreleased] mds lint bullet (line ~579) stated "9-rule static analyzer" and claimed "byte-identical canonical JSON output" across all five surfaces. Both claims are falsified by changes already in this same unreleased cycle: - legacy-interpolation (#236) is also in [Unreleased], making the shipped rule count 10, not 9. README.md already reflected 10; the CHANGELOG was stale. - Binding surfaces (napi, WASM, Python) expose a lint_warnings channel absent from the CLI's --format json stdout (#224). The per-file/per-diagnostic canonical JSON payload IS byte-identical across surfaces; the overall JSON objects are not, because of the binding-only key. Fix: change "9-rule" → "10-rule"; qualify the byte-identity claim to the per-file/per-diagnostic payload and note the lint_warnings channel asymmetry. Co-Authored-By: Claude <noreply@anthropic.com>
Commit 3ab9d7f updated README.md:215 and the lint table (adding legacy-interpolation) but left two stale "9 rules" references: - README.md:83: CLI usage block "(9 rules; --fix, --format json)" - examples/README.md:18: comment "Static analysis — 9 rules, ..." Both now read "10 rules", consistent with README.md:215, the table row count (10), examples/linting/README.md "The ten rules" heading, and the KNOWN_LINT_RULES.len() == 10 assertion in api_surface.rs. Applying PF-015: avoids absolute completeness claim; count is a concrete number, not "all rules", so it remains verifiable. Co-Authored-By: Claude <noreply@anthropic.com>
The module-level doc at the top of lint/mod.rs still said 'applies the 9 lint rules' after the tenth rule was added. The inline doc on lint_source() (line 71) and the run_rules() doc (line 106) both already said 10, so this was a straight stale-comment contradiction. Fixes the low-severity finding flagged in the PR2 review batch. Co-Authored-By: Claude <noreply@anthropic.com>
- `attach_lint_warnings`: add `#[doc(hidden)]` — it is binding-internal JSON plumbing (napi/WASM/Python) with no other crate to live in; hiding from docs reduces discoverability without changing binary ABI. The stability note in its rustdoc now records the conscious call: justifiable, leaks no new dep (serde_json::Value already public via to_canonical_json), but external consumers should not depend on it. - `LintConfig::from_rules`: mark `#[deprecated(since = "0.4.0")]` pointing callers to `from_rules_checked`. All five in-repo production callers already migrated; only api_surface.rs tests remain. Those two tests gain `#[allow(deprecated)]` so the -D warnings gate stays clean. The LintConfig type doc and module doc no longer recommend `from_rules` as a primary construction path. Verification: cargo nextest run -p mds-core (1345/1345 passed), cargo test --doc -p mds-core (52/52 passed), cargo clippy -p mds-core --all-targets -- -D warnings (EXIT=0), source hygiene gate (EXIT=0). Co-Authored-By: Claude <noreply@anthropic.com>
Both `packages/mds/src/types.ts` and `crates/mds-python/python/mdscript/_mdscript.pyi` carried doc comments asserting that all lint surfaces produce byte-identical JSON. That was true before PR2 hardened the asymmetry: the CLI now writes `lint_warnings` to stderr so its JSON stdout is unpolluted, while the napi/WASM/Python binding surfaces include the field in the returned JSON object when non-fatal warnings occur. - types.ts: replace the "All surfaces produce byte-identical JSON" sentence with an accurate description of which fields each surface includes (per spec.md:998 and packages/mds/README.md:150-154). - _mdscript.pyi: replace "byte-identical across all surfaces" with a statement that matches the surface-specific behaviour; note the conditional presence of `lint_warnings` and that the CLI routes it to stderr instead. Applies PF-015 (avoid absolute completeness claims in normative docs). Co-Authored-By: Claude <noreply@anthropic.com>
…in targets [#224] The existing AC-224-22 quiet-suppression tests only exercised the config_for emitter (directory targets). The load_lint_config emitter's quiet gate (single-file and stdin code paths, lint.rs emit_unknown_rule_warning) had no coverage, leaving the PF-013/ADR-009 pairing vacuous for those paths. Add two new tests, each with a paired positive control: - unknown_rule_warning_suppressed_by_quiet_single_file - unknown_rule_warning_suppressed_by_quiet_stdin Both confirm --quiet suppresses the warning and does not move the exit code. Co-Authored-By: Claude <noreply@anthropic.com>
…us [#224] The block comment at cli_build.rs:1291 and three per-test rustdocs falsely claimed that build/check/fmt/watch "load the same mds.json through `into_core_config`". `into_core_config` has exactly two callers, both in lint.rs; build/check/fmt deserialise MdsConfig without going through it (build.rs:49-51). The non-vacuity claim on the build test was also wrong: "the warning-path was reached and declined" — there is no warning path in mds build at all. Additionally the fmt test targeted `&src` (a single file), but fmt.rs:308 only calls `load_config` for a DIRECTORY target; the file path bypassed load_config entirely, making the test vacuous. Similarly, mds check never calls `load_config` regardless of target, so its test was also vacuous (PF-013/ADR-009). Changes: - Correct block comment to match build.rs:49-51: build and fmt<DIR> read mds.json via `load_config`; check and fmt<FILE> do not call load_config at all; none call `into_core_config`. - Replace false non-vacuity claim on build test with accurate description that references the positive-control arm. - Add positive-control arms to all three tests: unknown SEVERITY value in mds.json causes build and fmt<DIR> to exit non-zero (proving load_config was reached), while check still exits 0 (proving it never reads the config). This satisfies ADR-009/PF-013 for the build and fmt tests, and documents the structural nature of check's absence. - Retarget fmt test from `&src` (file) to `dir.path()` (directory) so load_config is actually exercised. - Fix check test assert messages (removed incorrect "D2(a):" prefix). - Correct CHANGELOG to remove the claim that `check` loads mds.json and drop the `check_unknown_lint_rule_in_mds_json_emits_no_warning` test from the list of D2(a)/watch-path CI guards (only build and fmt<DIR> qualify). Co-Authored-By: Claude <noreply@anthropic.com>
The napi README described `result.lint_warnings` as "a `string[]` field"
without noting it is absent when empty. A consumer reading only this README
could write `result.lint_warnings.length` and hit a TypeError on the clean
path.
The shape line at :66 already carries the `?` marker so the contract was
not wrong, but the prose was inconsistent with `packages/mds/README.md:151`
("absent when empty") and `packages/mds-wasm/README.md:52` ("a non-empty
string[]"). Align the prose to remove the ambiguity.
Co-Authored-By: Claude <noreply@anthropic.com>
AC-224-22 specifies that --quiet suppresses the unknown-rule-name warning. spec.md:1221 and CHANGELOG.md:783 already carry this fact; add the same one-sentence note to examples/linting/README.md (the description at line 193 is the most likely landing point for a CLI user reading about unknown rules), completing the docs sweep for this acceptance criterion. Co-Authored-By: Claude <noreply@anthropic.com>
…2 assertions [#224] Fixes five review findings (three high, two medium) on security.rs: T-ESC-RULE-1 (lint_unknown_rule_name_escapes_control_bytes): - Restores the 7-width COLUMNS loop [40,60,80,100,120,160,200] removed by commit 3daa51a, satisfying AC-224-4 ('property hold at every terminal width from 40 to 200 columns inclusive') and AC-224-5 (no weakening). - Updates the AC-224-4 rustdoc paragraph to remove the 'width invariant holds by construction' / 'adds no discriminating power' rationale that directly contradicted the sibling T-ESC-RULE-2 which kept the identical loop and documented it as proof. - Adds COLUMNS={columns}: prefix to all assertion messages inside the loop, matching T-ESC-RULE-2's style for diagnosability. - Documents that the rule name carries TWO newlines so the 'Clean: totally-real.mds' forged-line assertion is a live check (middle segment, standalone when unescaped). T-ESC-RULE-2 (lint_plural_unknown_rule_names_escape_control_bytes): - Changes rule_a from one embedded newline to two ('...RULE\nClean: real-a.mds\nOK: fake-a.mds'), making 'Clean: real-a.mds' a middle segment that would appear standalone if safe_inline were removed — the forged-standalone- line assertion is now live (fixes structurally-unfailable assertion finding). - Adds assert_eq!(stderr.matches("\\u000A").count(), 3, ...) inside the COLUMNS loop: two from rule_a plus one from rule_b = 3 total. This closes the gap the review found — assert_no_control_chars permits \n by design, so the only prior newline-escaping coverage was the incidental ends_with check. Both tests now agree: the COLUMNS loop runs at all seven widths and proves the warning emitter (eprint_warning → bare eprintln!) never wraps, consistent with AC-224-4's literal requirement. The self-contradictory rationale is eliminated. applies ADR-009 (positive-control non-vacuity for forged-line assertions)
] Finding 1/2 — D2(a) command asymmetry (spec.md §7.8, line 1221): The `lint.rules` config row said "On the CLI the warning goes to stderr and is suppressed by --quiet" without naming which commands emit it. `mds build`, `mds check`, `mds fmt`, and `mds watch` all read the same `mds.json` but do not emit the unknown-rule warning. Add a qualifying clause that names `mds lint` as the emitting command and explicitly states the other four do not emit it (mirrors CHANGELOG.md:805-807 and the crates/mds-cli/src/build.rs module doc, which both record this asymmetry; spec.md was the outlier). Finding 3 — lint_warnings escape table (spec.md:1028): The `lint_warnings` row described interpolated values as "rule names from `mds.json`". On the napi, WASM, and Python surfaces — the only three that carry this field — rule names come from the caller's `rules` option object, not from any config file. Change to "rule names from the caller's `rules` option". Finding 4 / PF-015 — absolute byte-identity claim (spec.md:1034): "All four surfaces emit byte-identical values, with one exception: the \"file\" key when the source is piped via stdin." The delta introduced a second exception (`lint_warnings` is a binding-surface-only key, absent from the CLI's `--format json` output). The sentence asserted exactly one exception while the table immediately above documented two. Change to "with two exceptions" and enumerate both: the `lint_warnings` key absence and the stdin `file` relabelling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mds.json(and in therulesoption on all binding surfaces) now emit a warning and lint continues, instead of silently no-opping--format jsonstdout remains valid parseable JSON;--quietsuppresses it (AC-224-22)lint_warnings: string[]in the lint result when unknowns are detected — the D8 binding warning channelLintRuleNameunion type,LINT_RULE_NAMESconst array,lint_warnings?: string[]onLintResultcrates/mds-python/README.md:71-73updated per the mandatory AC-224-23 rulingChanges
mds-core
ALL_RULE_NAMESconst inlint/rules/mod.rscomposed from each module'sRULEconst (single source of truth)KNOWN_LINT_RULES,find_unknown_rule_names,UnknownRuleNames,format_unknown_rule_names_warningmds-cli
load_lint_configgainsquiet: boolparameter; warning is WIRE-escaped viasafe_inlinebefore reachingeprint_warningKNOWN_RULESconstant (AC-224-15) — rebuilt from the core registry; no rule-name string literals appear incrates/mds-cli/src/crates/mds-cli/tests/print_discipline.rsis byte-identical to the wave base — no allowlist entry was added. Everysafe_inlinecall is a whole-expression argument directly insideeprint_warning'sformat!, the one shape the trace checks without an exemption.Binding surfaces (D8)
lint_warnings?: string[]added to returned result; absent when all rule names are knownTypeScript (
packages/mds)LintRuleNameunion type andLINT_RULE_NAMES: readonly LintRuleName[]exported from@mdscript/mdsLintResult.lint_warnings?: string[]added to interfaceDocumentation
crates/mds-python/README.md:71-73(mandatory per ruling, AC-224-23)crates/mds-napi/README.md,packages/mds/README.md,packages/mds-wasm/README.mdspec.md: escape-contract table reconciled (WIRE row and residual row no longer conflict formds.jsonrule names);lint_warningsdocumented in the canonical lint JSON sectionexamples/linting/README.md: all three facts now explicit (warned, config loads, rule not enforced)Tests
security.rsT-ESC-RULE-1: COLUMNS loop 40–200, strengthened assertions (AC-224-4/5)cli_lint.rs: AC-224-10, AC-224-11, AC-224-19, AC-224-21, AC-224-22@mdscript/mds: U-L-WARN-1..6 on both native and WASM backendsBreaking Changes
None — warn-and-continue is strictly additive. Unknown severity values still hard-fail (asymmetry is intentional).
Unmet Acceptance Criterion — Author Sign-Off Required
AC-224-3 is unmet. This waiver was documented by the implementer; it requires the ruling author's explicit ratification before this PR merges.
AC-224-3 states the warning message MUST be byte-identical across all five surfaces. This PR consciously deviates: the CLI emits
warning: unknown lint rule 'NAME' in mds.json; recognised rules are: …while binding surfaces (napi/WASM/Python) produceunknown lint rule 'NAME'; recognised rules are: …— omitting thewarning:prefix and thein mds.jsonsource context. The divergence is documented coherently in three places (CHANGELOG #224 entry,lint.rs:206-231,config.rs:125-132) and per-surface goldens exist as PF-007 requires.Engineering rationale (defensible, but not a ruling):
in mds.jsonsource context to direct the user to the config file; library callers have no equivalent concept.safe_inline+eprint_warningsingular/plural composition requires the call sites to be whole-expression arguments — the exact shapeprint_discipline.rschecks without an allowlist entry. Byte-identical phrasing across surfaces would require a different call shape and an allowlist exemption.mds::KNOWN_LINT_RULES(shared-constant parity).Author action required: Please reply confirming you accept this deviation from AC-224-3, or request it be corrected before merge.
Accepted Residuals
D2(a) asymmetry (AC-224-14): The unknown-rule-name warning fires only in
mds lint(the one command that evaluates lint rules).mds build,mds check,mds fmt, andmds watchall readmds.jsonbut do not evaluate lint rules — they silently ignore thelint.rulesmap, so no warning is emitted there. This is intentional: emitting a lint-rule warning during a build or format run would be confusing noise. A mid-sessionwatchconfig edit that introduces a typo inlint.ruleswill not cause the warning to fire (the watch path does not call the lint rule-name checker), and will not corruptoutput_dir.WASM size (AC-224-18)
Measured locally with wasm-pack 0.15.0 / bundled wasm-opt v117 (same toolchain for both baseline and this branch):
9,905 bytes delta is under the +10,000 R2 trigger. 842,951 bytes is 7,049 bytes under the 850,000 guard. CI uses Binaryen v129; the absolute count there will differ slightly but is expected to track the same trend.
Reviewer Focus Areas
crates/mds-cli/src/lint.rs:load_lint_configquiet gate andsafe_inlineusage; AC-224-3 residual commentcrates/mds-cli/tests/print_discipline.rs: confirm byte-identical to wave base (no allowlist entry — the guard coverage is preserved, not residual-ised)crates/mds-core/src/lint/config.rs:find_unknown_rule_namesandformat_unknown_rule_names_warning— confirm these are the right abstraction boundarysecurity.rs: confirm the COLUMNS loop is strictly stronger than the previous one-shot assertioncrates/mds-python/README.md:71-73: the exact sentence required by AC-224-23spec.mdescape-contract table: verify the WIRE/residual row reconciliation is correct