Skip to content

config: reconcile ADR 0061 and ADR 0064 on the invalid-value rule and the two YAML-subset parsers #7104

Open

Description

@kyle-sexton

Summary

The repository has two YAML-subset parsers with different grammars: lib/yaml-subset.mjs (strict) and lib/yaml-subset.awk (accepts flow mappings and document markers). The owner chose yaml-subset.mjs's grammar as the only grammar on 2026-10-12 and confirmed it on 2026-10-20 (decision comment). This issue makes the awk parser match it, in this order:

  1. Rewrite every flow mapping in docs/conventions/*.yaml in block style (all of them are in docs/conventions/pr-pipeline.yaml: the models block and the issue-dimensions block).
  2. Make lib/yaml-subset.awk reject flow mappings and document markers (---, ...), as lib/yaml-subset.mjs does.
  3. Switch lib/setup-apply.mjs to import lib/yaml-subset.mjs instead of spawning awk.

Step 1 goes first because scripts/check-convention-yaml.sh parses every convention file with the awk parser, so step 2 fails that lint while any flow mapping remains.

Current behavior

Verified against main (419f77d).

  • lib/yaml-subset.mjs:87 throws flow mapping ({...}) is outside the subset; use block style. lib/yaml-subset.mjs:171 throws document markers are outside the subset. lib/yaml-subset.mjs:209 throws on a duplicate key. Flow sequences of scalars are accepted (:91, :99).
  • lib/yaml-subset.awk documents and implements flow mappings (header at :6, flow() at :121, mapping branch at :168-192) and skips ---, ... and % lines (:20, :261).
  • Duplicate keys: lib/yaml-subset.awk:43 fails them (fix(lib): strict parse-concern-value reports a duplicate key as a parse error #8175), but only under -v STRICT=1. lib/parse-concern-value.sh:108 passes STRICT. lib/setup-apply.mjs:432 and scripts/check-convention-yaml.sh:86 run awk without it.
  • lib/setup-apply.mjs:431-432 (flatten) spawns awk -f <parser> and reads its tab-separated records. Its callers are at :485, :771, :778 and :821.
  • lib/yaml-subset-awk.test.sh:35 asserts that a flow mapping parses (a duplicate in it fails under STRICT), and :38 asserts that a --- marker is accepted.
  • Flow mappings in docs/conventions/*.yaml (found with grep -nE ': \{'; no file has a line starting --- or ...):
    • docs/conventions/pr-pipeline.yaml:343-347, the five models.profiles entries (standard, review, pr-review, triage, summary).
    • docs/conventions/pr-pipeline.yaml:366, :369, :372, the options: maps under issue-dimensions.fields (size, complexity, uncertainty), with quoted numeric keys.
    • docs/conventions/pr-pipeline.yaml:376, :378, the small and medium entries under issue-dimensions.sizes.
    • code-metrics.yaml, source-control.yaml and testing.yaml have none.
  • Run through lib/yaml-subset.mjs, pr-pipeline.yaml fails today at line 343 with the flow-mapping error. The block-style form of a quoted numeric key ("27053568": small) parses in mjs.
  • The decision comment of 2026-10-20 names only :343-347. Lines :366-378 are also flow mappings, and the block-style rewrite has to cover them too or the lint fails.
  • The 2026-10-12 decision's first part (the invalid-value rule) is done by ADR 0075 and is out of scope here.

Desired behavior

  • docs/conventions/pr-pipeline.yaml holds the same values in block style. Every mapping that was written { k: v, ... } becomes one key per line.
  • lib/yaml-subset.awk fails with an error record, as the mjs parser does, on a flow mapping (a value starting {) and on a --- or ... document marker line. Its header comment and the plugin copies generated by scripts/sync-shared-copies.sh match.
  • lib/yaml-subset-awk.test.sh asserts the rejections in place of the flow-mapping and marker acceptances at :35 and :38.
  • lib/setup-apply.mjs imports lib/yaml-subset.mjs and builds its records from the parsed object. It does not spawn awk. The records keep the shape flatten returns today (path, top, nested, value), with scalar values as strings, so the validation and read-back callers need no change.
  • lib/parse-concern-value.sh and the other bash readers keep using the awk parser.

Acceptance criteria

  • No file under docs/conventions uses a flow mapping or a document marker.
  • docs/conventions/pr-pipeline.yaml parses with lib/yaml-subset.mjs, and every value it held before is unchanged.
  • lib/yaml-subset.awk reports an error record for m: {k: 1} and for a --- line, and still reads flow sequences of scalars.
  • lib/yaml-subset-awk.test.sh has cases for the flow-mapping rejection and the document-marker rejection.
  • lib/setup-apply.mjs does not call spawnSync("awk", ...) and imports yaml-subset.mjs.
  • The PR's CI runs scripts/check-convention-yaml.sh and it passes.
  • The PR's CI runs the lib/yaml-subset tests (mjs and awk) and lib/setup-apply.test.sh, and they pass.
  • The generated plugin copies of yaml-subset.awk equal lib/yaml-subset.awk (scripts/sync-shared-copies.sh leaves no diff).
  • The PR carries a changelog fragment or version bump for each plugin whose copy changed, as the repository requires (scripts/check-changelog-fragments.sh --check passes).

Out of scope

  • The invalid-value rule (ADR 0075 did it).
  • Making scripts/check-convention-yaml.sh and lib/setup-apply.mjs fail duplicate keys through awk (the mjs parser rejects them without STRICT, so setup-apply.mjs gains the check from the switch; the lint keeps its awk call).
  • Retiring yaml_subset.py (refactor(user-experience): read team config through lib/config-cascade.mjs #7027).
  • A shared fixture corpus for both suites (the 2026-10-12 plan listed it; the 2026-10-20 scope does not).
  • Reading consumer repos' docs/conventions/*.yaml; a consumer file with flow mappings fails after step 2 and needs the same rewrite.

Merging: a human merges this PR (lib/** path).

Related

Activity

  1. added
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 11, 2026
  2. github-actions commented on Oct 11, 2026

    @github-actions
    Contributor

    Thanks for the report. Automated triage handed this issue to a maintainer.

    Proposed work class: work-class: structural
    Basis: automated intake triage; a maintainer confirms or changes it.

    Automated triage. A maintainer reviews it.

  3. added
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    on Oct 11, 2026
  4. added theissue type on Oct 12, 2026
  5. removed
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 12, 2026
  6. kyle-sexton commented on Oct 13, 2026

    @kyle-sexton
    MemberAuthor

    Decision recorded. The owner accepted the adjudicated recommendation on 2026-10-12.

    Q1: adopt ADR 0064 D7 in both ADRs, stated per key: an invalid value is named by file, key and value and that key is dropped from that layer; a valid higher layer wins, otherwise the key's default; a lower layer's value is never used. Change lib/config-cascade.mjs to match. Q2: the grammars differ (verified this run), so pick lib/yaml-subset.mjs's stricter grammar as the one grammar: make lib/yaml-subset.awk reject flow mappings, document markers and duplicate keys, add one shared fixture corpus both suites run, and switch lib/setup-apply.mjs to import yaml-subset.mjs in the same change.

    Why: D7 is the only invalid-value rule labeled an owner decision (ADR 0064:64-68, amended 2026-10-04 'user decisions'); ADR 0061's lower-layer line carries no ruling, and the owner's newest merged change on the subject (#7009, 2026-10-11T23:51Z, after #6976 at 04:38Z) brought the README and AGENTS.md 'in line with ADR 0064 D7' and filed #7104 instead of conceding, so precedence rule 2 decides for D7, and on main eleven plugin reference docs plus README:67-69 already implement it while only lib/config-cascade.mjs:16-18 and README:583 implement 0061. Claude Code's own settings handling (settings.md:607-608, managed-settings.md:350-364) skips the single bad entry and lets the rest of the stack merge for ordinary keys, which is a design precedent in a different system, not a fact this repo depends on; it does support per-key granularity and fail-closed handling of a constraining layer, which the recommendation adopts. The parser cross-run was re-executed on origin/main: awk accepts 'a: {b: 1}' (emits a.b=1), a '---' marker and a duplicate key (emits both records) where mjs errors on each by documented design, and mjs types scalars where awk emits strings, so the issue's 'if grammars match' branch is closed and one grammar must be chosen.

    Precedence applied: Rule 2 (owner's newest explicit decision): D7 is the only dated owner ruling on invalid values and #7009, the owner's later merge, names it; #6976 merging ADR 0061 without naming D7 is flagged as possible course change, not proof. Rule 3 does not reverse it: 0061 is newer-dated but defers to #6594, and rule 2 outranks it. Rule 4 (upstream Claude Code settings docs) is cited, not a fact conflict: it describes a different system and informs only the per-key granularity. Researcher B's hybrid (fail closed only for declared restrictive keys) is an agent default (rule 5) no owner has ruled on and is not adopted.

    Sources:

    • origin/main docs/adr/0064-home-plugin-customization-in-docs-conventions-yaml.md:11,64-68,93-94 (git show this run)
    • origin/main docs/adr/0061-adopt-a-five-layer-plugin-config-cascade-read-through-one-shared-resolver.md:1-6,54,68,111-112
    • gh pr view 6976 (mergedAt 2026-10-11T04:38:40Z, owner-merged) and 7009 (mergedAt 2026-10-11T23:51:34Z, owner-merged; body lines 189, 221-222, 242 name D7 and 'Refs: config: reconcile ADR 0061 and ADR 0064 on the invalid-value rule and the two YAML-subset parsers #7104')
    • gh issue view 6594 comments: owner comments 2026-10-09, 2026-10-10, 2026-10-11 contain no invalid, lower layer or fail closed wording; state OPEN
    • gh issue view 7104 body (cites setup-apply.mjs:186-191, stale; actual lines 237-243); 7027 OPEN; 7331 OPEN
    • origin/main lib/config-cascade.mjs:16-18 (per-leaf drop, 'the lower layer keeps it')

    Next: One draft PR, Conventional Commits title, 'Closes #7104', Refs #6594, #7027, #7331. (1) Amend ADR 0061:54 to D7 wording and add a per-key clarification to ADR 0064 D7 ('drops that key from that layer'). (2) lib/config-cascade.mjs:16-18: an invalid leaf no longer lets the lower layer keep the key; resolve from a valid higher layer, else the default; update lib/config-cascade.test.mjs and .test.sh; regenerate plugins/user-interface/scripts/lib/config-cascade.mjs via scripts/sync-shared-copies.sh; note the user-interface behavior change in the PR body. (3) README: fix the user-interface row at :583 and reword rule 5 at :310-312 so 'as if that layer were absent' no longer implies lower-layer fallback. (4) lib/yaml-subset.awk: reject flow mappings, '---'/'...' markers and duplicate keys (folds #7331), regenerate the 42 plugin copies; add one shared fixture corpus consumed by lib/yaml-subset.test.mjs and lib/yaml-subset-awk.test.sh. (5) lib/setup-apply.mjs:237-243: import yaml-subset.mjs instead of spawning awk. Before merge, grep consumer repos' docs/conventions/*.yaml for '{' and '---' (none in this repo's four committed files). Open items for the PR, not the owner: the disable-list gap (an invalid *.disable list drops that layer's entries under either rule) is outside #7104 and worth a follow-up issue only if a reader acts on it; #7027 keeps ownership of retiring yaml_subset.py.

    Researched by two independent agents (local evidence and upstream docs) and adjudicated by a third.

  7. kyle-sexton commented on Oct 20, 2026

    @kyle-sexton
    MemberAuthor

    Decision recorded. The owner answered in the Issue Triage session on 2026-10-20.

    So: (b). The 2026-10-12 Q2 decision stands: one grammar, yaml-subset.mjs's. The change first rewrites the flow mappings in docs/conventions/pr-pipeline.yaml's models block (:343-347) in block style, then makes lib/yaml-subset.awk reject flow mappings and document markers as the mjs parser does, and switches setup-apply.mjs (:432) to import the mjs parser. Q1 is already done by ADR 0075. Paths are under lib/**, so a human merges the PR.

  8. added
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    and removed
    status: needs-decisionAwaiting a human or maintainer judgment call.
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    on Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageNot yet classified. Floor until a type and one priority tier are set.

    Type

    Fields

    No fields configured for Task.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions