diff --git a/docs/spec/todos/TODO-0196.md b/docs/spec/todos/TODO-0196.md index ed408e8..c54a950 100644 --- a/docs/spec/todos/TODO-0196.md +++ b/docs/spec/todos/TODO-0196.md @@ -1,12 +1,15 @@ --- id: 196 title: Duplicate `[[fields.field]]` names are silently accepted — last one wins -status: todo +status: done priority: high created: 2026-08-20 +completed: 2026-08-20 depends_on: [] blocks: [] related: [194, 195] +files_updated: + - crates/mdvs/src/schema/config.rs --- # TODO-0196: Duplicate `[[fields.field]]` names are silently accepted — last one wins @@ -15,6 +18,36 @@ related: [194, 195] Two `[[fields.field]]` entries with the same `name` are accepted by config load without warning. Downstream lookup is keyed by the bare name, so the later entry silently overwrites the earlier one and the first declaration is discarded. Reject duplicates at config load. +## Resolution + +Shipped as **invariant 10** in `MdvsToml::validate()` (`crates/mdvs/src/schema/config.rs`), merged in PR #73. Config load now rejects any two `[[fields.field]]` entries sharing a `name`: + +``` +Error: mdvs.toml is invalid: field 'status' is declared more than once — each +[[fields.field]] name must be unique. Merge the entries into one, or rename one +of them. — fix the file or run 'mdvs init --force' +``` + +Exit 2, and it fires on every command that loads the config — `check`, `update`, `build`, `info` — because the rejection happens at load rather than in any one pipeline. + +The check runs **before** the per-field loop, so a structural problem is reported ahead of any per-field complaint. Names are matched exactly: invariant 7 already canonicalises dotted names, and YAML keys are case-sensitive, so `status` and `Status` remain independently declarable. Case-insensitive rejection would have forbidden a legal vault — that settles the "Open question" recorded below. + +### Decision: rejected on the name alone, type ignored + +Duplicates are rejected **regardless of whether the two entries agree on type** (decision 2026-08-20). The alternative considered was accepting repeated entries when their types match and merging them, which would have made the config someone writes by instinct into the eventual per-directory-constraint syntax. + +That was ruled out. Scoping a field per directory is a deliberate feature that belongs to [TODO-0194](TODO-0194.md)'s `[[scope]]` design, not something a repeated entry should back into — and repeating `type` on every entry makes it look as though it could legitimately differ, when it cannot. The likely future syntax is therefore **constraint blocks nested inside a single `[[fields.field]]`**, keeping "one entry per name" true permanently. + +A test (`validate_rejects_duplicate_names_even_with_matching_type`) pins this so the rule is not relaxed by accident. Relaxing it later would remain backward-compatible in any case — configs that error would start working, none that work would start failing. + +### Also fixed + +The `validate()` docstring listed eight invariants while the body enforced nine; invariant 9 (`Array(Object)` not representable on disk, from TODO-0155) had been added without a docstring entry. Both now describe ten. + +### Verification + +Four tests added, covering: duplicates with differing types, duplicates with matching types, `status` vs `Status` staying legal, and distinct names passing. Suite went 979 → 983 passing, with the 12 ignored real-model tests also green. Clippy clean under `-D warnings`, fmt clean, `ast-grep scan` clean. No false positives on real vaults — `example_kb` (46 files) and Refractions (688 files) both load. + ## Details ### Reproduction @@ -86,9 +119,11 @@ That covers the common real case, where the *values* diverge and the type does n Implementing invariant 10 now does not foreclose that: a future scoped-constraint syntax would attach constraint blocks to a single `[[fields.field]]` entry rather than repeating the entry, so "one entry per name" remains true either way. -### Open question +### Open question — resolved + +Should invariant 10 fire for names differing only by case, or by dotted-path normalisation? -Should invariant 10 fire for names differing only by case, or by dotted-path normalisation? Field names are matched exactly elsewhere, so exact-match rejection is the consistent choice, but worth confirming no vault relies on near-duplicate names. +**Resolved: exact match.** Case-insensitive rejection would be wrong rather than merely stricter — YAML keys are case-sensitive, so `status` and `Status` are genuinely different frontmatter fields and a vault using both is legal. Dotted names are already canonical by the time invariant 10 runs, since invariant 7 rejects leading/trailing dots and empty segments, leaving no normalisation ambiguity. ## Files diff --git a/docs/spec/todos/index.md b/docs/spec/todos/index.md index db310c1..4254ae7 100644 --- a/docs/spec/todos/index.md +++ b/docs/spec/todos/index.md @@ -197,4 +197,4 @@ | [0193](TODO-0193.md) | Support .mdx files — free validation, gated search-body stripping | todo | medium | 2026-07-06 | | [0194](TODO-0194.md) | Directory-scoped schema policy — freeze undeclared fields per path | todo | high | 2026-08-17 | | [0195](TODO-0195.md) | Cross-field rules — conditional requiredness, and the variant-type question | todo | medium | 2026-08-17 | -| [0196](TODO-0196.md) | Duplicate `[[fields.field]]` names are silently accepted — last one wins | todo | high | 2026-08-20 | +| [0196](TODO-0196.md) | Duplicate `[[fields.field]]` names are silently accepted — last one wins | done | high | 2026-08-20 |