Skip to content

feat(user-interface): add layered config and /user-interface:setup - #7039

Merged
kyle-sexton merged 26 commits into
mainfrom
feat/user-interface-config
Oct 11, 2026
Merged

kyle-sexton merged 26 commits into
mainfrom
feat/user-interface-config

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Refs: #6594

Summary

user-interface adopts the five-layer config cascade from ADR 0061, read through the shared resolver lib/config-cascade.mjs (#6984). It also gains /user-interface:setup to show and change that config. The css keys configure the CSS authoring skill that lands in the next PR. Until then they change no behaviour.

Fix

  • Vendored copies (scripts/shared-copies.txt, ADR 0019): lib/config-cascade.mjs, lib/yaml-subset.mjs and lib/resolve-convention-home.sh.

  • Config layers, later wins:

    1. reference/defaults.yaml
    2. userConfig css_browser_target, css_important, css_layer, css_token_fallback
    3. ~/docs/conventions/user-interface.{yaml,md}
    4. <home>/user-interface.{yaml,md}
    5. <home>/user-interface.local.{yaml,md}

    The schema is reference/team.schema.json, with additionalProperties: false. routing is accepted only in the team layer.

  • scripts/detect.mjs:

    • --config adds a top-level config holding the resolver output with per-key provenance, plus home, and home_error when the pointer line is broken.
    • --team FILE is a test seam. --user-config FILE passes userConfig values as data, so they never reach a shell.
    • The project object gains browserslist {query, source}, lint {stylelint, eslint_css, config_paths} and style_files [kinds]. All existing fields are unchanged.
    • When the convention pointer line is broken, the team and local layers are not read. This matches user-experience, and a stale docs/conventions file cannot load routing.
    • Team routing is validated entry by entry with scripts/lib/routing-validate.mjs, moved out of tests/routing.test.mjs unchanged. An unknown key drops only that key; a bad row, disable or deny entry drops alone. A re-rank of a bundled row (same concern and id) gets per-field checks; a new or complete row gets the whole row schema. /user-interface:design does not apply team routing yet (feat(user-interface): apply team routing in /user-interface:design #7041); the README says so.
    • A .browserslistrc or browserslist file is read only when its real path stays inside the project and it is at most 64 KiB.
  • /user-interface:setup check|apply [--user|--local] (disable-model-invocation: true):

    • check shows each value with its provenance, plus errors, legacy files, the .local ignore state and home_error. It warns when this worktree lacks a .local file that the main checkout has.
    • apply edits one key in one layer. With --user it edits the chezmoi source when chezmoi manages the file.
  • Convention docs: a user-interface Implementers row in docs/conventions/config-cascade/README.md and an updated Adopters row in docs/conventions/routing-as-data/README.md. Both record the layer-location deviation that ADR 0061 decides.

  • README: a config section and a fixed-defaults list. Changelog fragment under .changes/user-interface/.

Verification

  • tests/detect.test.sh 57/57, tests/defaults.test.sh 3/3, tests/routing.test.sh 16/16.
  • Codex review threads (check-ignore wording, team-row style, per-session userConfig handoff file) fixed in 7b16433; CI typos and argument-hint findings fixed in ea362a4.
  • Pre-ready code and security reviews of the merged head: findings fixed in f6a5a3e (symlinked .browserslistrc read outside the project; one bad routing entry dropping all of routing; new team rows and disable entries passing without required fields; setup's home_error wording and apply symlink stop). Team routing not applied by design: documented, follow-up feat(user-interface): apply team routing in /user-interface:design #7041.
  • scripts/sync-shared-copies.sh --check: 411 copies match. --check-bump origin/main passes.
  • check-skill.sh --require-evals plugins/user-interface/skills exits 0. sync-config-cascade-semantics.py --check exits 0. Fragment check, markdownlint and typos on the changed docs are clean.
  • Fresh-context phase verifiers:
    • Phase 4: 10/10, then 7/7 on the fix round. Its findings are fixed in 59232e8, 4c26c03 and 953aa2c.
    • A shell-metacharacter userConfig value is stored literally and nothing runs.
    • Per-key later-wins and *.disable union across all five layers are checked against a scratch project.
  • Known, documented:
    • css_important and css_token_fallback declare manifest defaults equal to defaults.yaml, because an options field needs one. Manifest defaults are not substituted (docs/extensibility-contract-smoke-tests.md), so an unset key shows defaults provenance. A test keeps the two default values equal.
    • Setup's offer to save write-css stack-checklist answers lands with write-css in the next PR.

Related

🤖 Generated with Claude Code

PLAN.md

Brief

Restatement: CONFIRMED (user, 2026-10-10, "Confirmed.")

TLDR

  • New skill /user-interface:write-css: CSS techniques absorbed from good-css (MIT, pinned b645ab15b9fc), restructured by topic as the first source of a long-lived master CSS skill, with a read-only review mode
  • The project's design system and conventions override every shipped default; any stack is handled by a fixed four-step discovery process, never per-framework files
  • New /user-interface:setup skill and a five-layer config cascade (plugin defaults, userConfig scalar mirror, user-global, project team, project .local) read through one shared resolver; everything sensible is configurable
  • docs/upstream/good-css.md attribution record, design detector extension and routing row, presence-gated cross-references from prototype and visualization
  • Evals written before skill text; work split across separate worktrees and feature branches

Goal

Agents writing or reviewing CSS in any consuming project apply modern, self-adapting CSS techniques without fighting that project's own design system, stack, browser targets or conventions. The skill discovers each project's stack and styling setup, fills only the gaps the project leaves, says which of its own defaults it used, and lets teams and individuals reconfigure every behavior that a consumer could reasonably need to change.

Constraints

  • Absorb, do not depend: restructure freely (take verbatim, move, reword, drop) to fit this repo's philosophy and Anthropic skill best practices (C1, C9)
  • Long-lived master skill: content split by topic, not by source, so later CSS sources merge in; the name carries no source (C10, Q14, Q18)
  • Lives in the user-interface plugin; no new plugin (C7, Q2)
  • Project first: the project's design system, conventions, lint config and layer system beat every good-css default, including always-on rules, linter findings and CSS placement (C2, Q3, Q23-Q26)
  • Never enumerate frameworks or libraries; stacks are discovered per project through a fixed process (C11, C15, Q13, Q22)
  • Skills cross-reference only through public /plugin:skill names, presence-gated (C12, Q6, Q20)
  • Credits and provenance live in docs/upstream/good-css.md and the CHANGELOG, never in skill files; MIT notice travels with copies, and the bundled linter keeps a vendor/LICENSE (C4, C8, C13, Q5, Q21)
  • Volatile specifics are pointers with an as-of date and recheck trigger, not copies (C5, Q27)
  • An always-on instruction needs eval evidence; one without measurable improvement moves to the on-demand references (C6, Q19)
  • Config cascade: plugin defaults, userConfig scalar mirror, user-global, project team (checked in), project .local personal overlay; locations resolved through one shared resolver, docs/conventions only a default (C16, Q34, Q35, Q39, D14, R5)
  • Everything sensible is configurable; each fixed default is a stated assumption (C18)
  • All work on separate worktrees and feature branches (C14)
  • Mechanism decisions belong to /planning:plan (Q8)

Acceptance criteria

  • /user-interface:write-css exists in plugins/user-interface, passes /skill-quality:check, and its description triggers on CSS authoring and review requests without overlapping /user-interface:design
  • All 47 upstream techniques are present in topic-organized reference files, each with a one-line rationale and a Baseline status pointer; no upstream browser-version numbers are copied; the six techniques without upstream support lines have a pointer
  • Upstream "Left out on purpose" content appears as the skill's Gotchas; upstream "Next" is not carried
  • No skill file contains upstream credits, author names or source links; docs/upstream/good-css.md holds the pinned commit, recheck trigger (new commit to PRACTICES.md or a Baseline change for a used technique), per-technique credits and the MIT notice, and the CHANGELOG entry carries the notice
  • Before writing CSS, the skill runs the four-step process: detect stack-agnostic facts (browser targets, token sources, lint config, style file kinds), identify the stack and research unknowns (presence-gated /context7:lookup, /discovery:research, /discovery:explore, else official docs), answer the six-question checklist, and verify compiled output when scoping or a build step rewrites CSS
  • When the project states a convention, the skill follows it over any good-css rule; always-on rules apply only where the project is silent
  • The skill never invents a token value; when no token exists it follows the configured token_fallback (default ask)
  • Browser target resolves from skill argument, then project browserslist, then configured target, defaulting to Baseline widely available; a technique newer than the target gets an @supports fallback or is skipped with a stated reason
  • New CSS goes in a low-priority @layer only when the project has no layer system; otherwise it uses the project's mechanism; !important appears only with a stated reason
  • Review mode edits nothing, reports file:line findings, runs the bundled linter first, points at the project's Stylelint or @eslint/css rules when present, and lists findings that contradict project conventions under a suppressed heading rather than dropping them
  • Every setting in the Design's settings schema (## Design > Contracts, reference/team.schema.json) is configurable at the documented layers, read through one shared resolver with no hard-coded path in skill text; a written list names every fixed default and why it is fixed
  • /user-interface:setup supports check and apply, writes the team layer at the resolved convention home and the user-global layer with --user
  • design's detector reports browserslist, token sources and lint config; design's routing data has a css-authoring row pointing at the new skill
  • /prototype:explore-directions and /visualization:visualize reference the new skill presence-gated
  • Eval suite exists before the skill body and covers each always-on rule (with vs without), a scoped-styles case, an existing-token-set case and review mode; always-on rules lacking a measurable improvement are in references, not the body
  • IF a research tool is missing or unreachable, THEN the skill marks which checklist answers it could not verify instead of guessing
  • WHILE the project has no token sources, the skill uses its own defaults and names each default it used

Captured assumptions

Out-of-scope

  • Any check, record or dependency for is-agentic.com (Q7)
  • Per-framework or per-library reference files (Q13, Q16)
  • Repo-wide decision on config format and folder layout (Q38; tracked on feat(conventions): one setup lifecycle for where plugins store team conventions and settings #6594)
  • A review-criteria document for the review plugin to consume (Q17)
  • A copy of upstream files committed to the repo for drift diffing (Q29)
  • userConfig options beyond the four scalar css_* keys (Q37, revised by R5)

Deferred questions

  • Q31: How the work splits into pull requests and how each plugin's version or changelog fragment is bumped, defer until planning; arbiter: /planning:plan
  • Q32: Eval case count, runs per case, cost ceiling and pass thresholds, defer until planning; arbiter: /planning:plan
  • Q33: Bundle check-css.mjs unmodified behind a wrapper or modified, defer until planning; arbiter: /planning:plan

Design

Threads: design/design-threads.md (D1-D14, plan-review fixes R1-R9). Full contracts: design/contracts.md.

Module layout

lib/                                       (repo root, canonical, ADR 0019)
├── config-cascade.mjs                     five-layer resolver, provenance, legacy-file report; takes `home` as input
├── config-cascade.test.{mjs,sh}
├── yaml-subset.mjs                        promoted from plugins/user-experience/scripts/lib/yaml-subset.mjs
└── yaml-subset.test.{mjs,sh}              the only yaml-subset suite
docs/adr/NNNN-five-layer-config-cascade.md recorded before PR A (R8)
plugins/user-interface/
├── .claude-plugin/plugin.json             userConfig: css_browser_target, css_important, css_layer, css_token_fallback
├── lib/resolve-convention-home.sh         generated copy; spawned by detect.mjs (R1)
├── reference/
│   ├── defaults.yaml                      layer 1
│   ├── team.schema.json                   one schema; `routing` accepted in layer 4 only
│   ├── routing.json                       + css-authoring row
│   └── types/web.md                       concern list and fallback section point to write-css
├── scripts/
│   ├── detect.mjs                         + --config, --team FILE (seam), --user-config FILE; project.{browserslist,lint,style_files}
│   ├── check-good-css-drift.sh            --check, row in the weekly drift workflow
│   └── lib/config-cascade.mjs, lib/yaml-subset.mjs   generated copies
├── skills/
│   ├── design/                            routes css-authoring to write-css
│   ├── write-css/
│   │   ├── SKILL.md
│   │   ├── reference/{foundations,layout,spacing-and-shape,text-and-media,interaction,motion,show-and-hide,scroll-and-viewport,stack-discovery,review,configuration}.md
│   │   ├── scripts/review.mjs
│   │   ├── vendor/{check-css.mjs,LICENSE}  unmodified upstream (R7)
│   │   └── evals/
│   └── setup/{SKILL.md,evals/evals.json}  disable-model-invocation: true
├── tests/                                 *.test.sh -> node --test
└── README.md                              fixed-defaults list
docs/upstream/good-css.md                  pin, blob SHAs, recheck trigger, credits, MIT notice
plugins/prototype/skills/explore-directions, plugins/visualization/skills/visualize   presence-gated pointers

Dependency direction: skills -> scripts/detect.mjs -> {lib/resolve-convention-home.sh, scripts/lib/config-cascade.mjs -> scripts/lib/yaml-subset.mjs}; write-css review mode -> scripts/review.mjs -> vendor/check-css.mjs, with the resolved config passed in as JSON (R4). The shared lib spawns nothing (R1). No plugin depends on another plugin's files; cross-plugin references are public /plugin:skill names only.

Contracts

Config cascade, later wins per key:

1 plugin defaults            plugins/user-interface/reference/defaults.yaml            shipped
2 userConfig                 settings.json pluginConfigs, scalar keys only (css_*)      user, not committed
3 user-global file           ~/docs/conventions/user-interface.{yaml,md}              user, not committed
4 project team               <home>/user-interface.{yaml,md}  (<home> default docs/conventions)   committed
5 project personal           <home>/user-interface.local.{yaml,md}                     gitignored

Merge: scalars later-wins; *.disable lists union; invalid value rejected with a message, lower layer keeps the key; routing accepted only in layer 4, rejected in 2, 3 and 5 (R6); every value carries its source layer; prose layers 3-5 concatenate; the detected project design system outranks every configured default. Layers 3 and 5 deviate from docs/conventions/config-cascade/README.md "What does not move" (lines 30-36, 84-86); the deviation is recorded by the ADR in Phase 1 and the Implementers row (R8).

Settings schema (reference/team.schema.json, additionalProperties: false, block-style YAML; yaml-subset rejects {):

version: 1
css:
  browser_target: "baseline widely available"   # browserslist query
  techniques:
    disable: []                                 # upstream technique slugs
  rules:
    disable: []                                 # hover|logical|outline|ease-in|color|transition-all|motion|duration
  important: avoid                              # avoid | utilities-only | allow
  layer: null                                   # null = project's own layer order
  token_fallback: ask                           # ask | propose-token | literal
routing:                                        # layer 4 only (R6)
  version: 1
  rows: []
  disable: []
  deny: []

userConfig (layer 2, R5): css_browser_target (string), css_important (string, options avoid, utilities-only, allow), css_layer (string), css_token_fallback (string, options ask, propose-token, literal). Lists live only in files. The skill writes the ${user_config.css_*} values to a temp JSON file with the Write tool and passes --user-config FILE to detect.mjs; substituted text never reaches a shell parser.

Resolver:

resolve({ plugin, projectRoot, home, schema, defaults, userConfig, teamOnly }) ->
  { values, provenance: { "<key>": "<layer>" }, prose: [paths],
    layers: [{ name, path, state: loaded|absent|invalid, errors }],
    legacy: [{ path, kind }] }

home is resolved by the caller (R1); teamOnly lists keys accepted only in layer 4, so the lib holds no plugin key names.

Detector (R2, R3): --config adds top-level config (resolver output); --team FILE replaces the resolved team file (test seam, user-experience meaning); --user-config FILE supplies layer 2; project gains browserslist {query, source}, lint {stylelint, eslint_css, config_paths}, style_files [kinds].

Review output (scripts/review.mjs --config FILE|JSON <paths>; exit 0 clean, 1 findings, 2 usage or crash; never spawns detect.mjs):

## Findings     path:line [rule-id] message
## Suppressed   path:line [rule-id] message — reason (config: rules.disable | project convention: <evidence>)
## Judgment     items no linter checks

Setup: /user-interface:setup check|apply [--user|--local]; check reports values with provenance, invalid keys, duplicate layer-2/3 keys, legacy files with a migrate offer, .local ignore state, and warns when this worktree lacks <home>/user-interface.local.* while the main checkout has one; apply edits one key per layer, adds the gitignore entry, adds the user-global pointer after a yes (chezmoi source when managed), offers to save stack-checklist answers.

Vendored upstream (pin b645ab15b9fcd059c0227c21b6a144bb70639f1a, git hash-object blob SHAs): scripts/check-css.mjs 6fa72714ba18cf02f698fa8015f63df043b3af85 -> skills/write-css/vendor/check-css.mjs; LICENSE 66a325d82e56f2f41ebd14812517d6c50f5b66da -> skills/write-css/vendor/LICENSE; PRACTICES.md 7d746e65d56ee4910572b75738051482f4735f38 (drift source only). Local copies: .work/good-css/raw/.

Routing row (JSON, in routing.json): {"concern":"css-authoring","rank":1,"id":"/user-interface:write-css","kind":"skill","detect":"user-interface","account":"none","status":"confirmed","platforms":["windows","macos","linux"],"as_of":"2026-10-10"}.

Variation verdicts

  • Every css: setting: varies, through the five-layer cascade (D3, D5, D14); lists only through files (R5)
  • Config locations and format: varies, through the shared resolver only (D4); repo-wide shape decided on feat(conventions): one setup lifecycle for where plugins store team conventions and settings #6594
  • Stack and framework handling: varies, through the four-step discovery process, never per-framework files (C15)
  • Guidance sources: varies, through routing rows under concern: css-authoring, team layer only (D11, R6)
  • Browser target: varies, skill argument > project browserslist > configured target > default (Q4)
  • Linter run and suppressed-findings display: varies, per-run skill arguments
  • Held fixed, each listed with its reason in reference/configuration.md and the README: topic split of reference files (merge target for later sources), review output sections and exit codes (contract for callers), the four-step process order (step 3 depends on 1-2), project-first precedence (C2)

Conventions followed and deviations

  • docs/conventions/routing-as-data/README.md (team file <home>/<plugin>.yaml, routing rows)
  • docs/conventions/config-cascade/README.md: merge rules followed; layer locations deviate (user-global ~/docs/conventions/, overlay <home>/*.local.*, userConfig scalar mirror), recorded in the Phase 1 ADR and the Implementers row (R8)
  • docs/adr/0019-share-code-across-plugins-by-vendoring-with-a-sync-gate.md, scripts/shared-copies.txt, scripts/sync-shared-copies.sh
  • docs/adr/0044 (structured team config): deviation recorded in the Phase 1 ADR, pending feat(conventions): one setup lifecycle for where plugins store team conventions and settings #6594
  • plugins/user-experience/scripts/detect.mjs (home resolution at line 150, --team seam), plugins/user-experience/skills/setup/SKILL.md (team read and setup flow)
  • plugins/multi-agent/scripts/resolve-roles.sh (layer merge model)
  • plugins/playwright/skills/playwright/vendor/ (vendored third-party with LICENSE; typos exclusion plugins/*/skills/*/vendor/**)
  • .github/workflows/maintenance-check-upstream-drift.yml (one matrix row per vendored upstream)
  • plugins/source-control/skills/worktree/context/create.md "Create the worktree" (substituted text written with the Write tool, never parsed by a shell)
  • docs/upstream/humanlayer-skills.md, docs/conventions/upstream-drift/README.md (absorption record)
  • scripts/fragment-plugins.txt (changelog fragments for user-interface)
  • plugins/playbooks/skills/skill-authoring/reference/skill-criteria.md (skill standard)

Plan

Approval: approved by the user in session on 2026-10-10 ("sounds good, approved, go with recommendations"), covering review changes R1-R9 and the Q37 reconfirmation

Scale: Large (an ADR, a new shared repo-level library vendored into plugins, a new config surface, two new skills, five pull requests).

Goal

What: Record the five-layer cascade decision, then ship /user-interface:write-css (good-css techniques absorbed by topic, project-first, four-step stack discovery, read-only review mode), the /user-interface:setup skill and the layered config it manages, read through one shared resolver lib/config-cascade.mjs.
Why: Agents writing or reviewing CSS should apply modern techniques without overriding a project's design system, stack, browser targets or conventions, and teams and individuals must be able to reconfigure every behavior a consumer could reasonably change (Brief: Goal, C2, C18).
Done when: the ADR PR and PRs A-D are merged with ci-status green; every Brief acceptance criterion maps to a phase Sanity Check below that passed; the three-arm eval run is recorded on PR C; the follow-up issue exists.

Standards grounding

Surface Sections cited Layer provenance
Repo agent rules AGENTS.md: open as draft, Conventional Commits title, stop list, fragment changelog, scope test team
PR body contract .claude/rules/pr-body-contract.md: Closes/Refs:/No related issue: first line, Summary/Fix/Verification/Related team
Skill bodies .claude/rules/skill-bodies-state-current-rules.md: pointer + as-of date + recheck trigger for volatile specifics (Baseline status), ## Next section team
Eval cases .claude/rules/eval-case-transcripts.md: no raw transcripts in evals/** team
Cost claims .claude/rules/cost-claims.md: no prices; eval cost points at the evals plugin ceiling team
Skill standard plugins/playbooks/skills/skill-authoring/reference/skill-criteria.md: Frontmatter, Descriptions, Body, Reference files, History and provenance, Measurement team
Config cascade docs/conventions/config-cascade/README.md: The layers (30-36), What does not move (84-86), Merge semantics, Deviations, Implementers (+ generated "Semantics at a glance" via scripts/sync-config-cascade-semantics.py) team
ADRs /architecture:record-decision into docs/adr/ team
Routing as data docs/conventions/routing-as-data/README.md: team file <home>/<plugin>.yaml, Adopters table team
Shared code ADR 0019, scripts/shared-copies.txt, scripts/sync-shared-copies.sh (--check, --check-bump) team
Changelog scripts/fragment-plugins.txt: user-interface (line 86), prototype (70), visualization (88) are fragment mode; user-experience is not team
Upstream record and drift docs/upstream/humanlayer-skills.md shape; .github/workflows/maintenance-check-upstream-drift.yml matrix rows (skill, script, args, drift, unverifiable) team
Vendored code .github/workflows/pr-require-checks.yml:809-816 typos exclusion plugins/*/skills/*/vendor/** team
Setup precedent plugins/user-experience/skills/setup/SKILL.md (check/apply, per-key block YAML), plugins/developer-experience/skills/setup/SKILL.md (--user pointer flow, chezmoi check) team

Approach

The work ships as five pull requests, each on its own worktree and feature branch (C14), in dependency order. Q31 is resolved here. [EXEC-SHAPE]

PR Branch (own worktree) Title Phases Depends on Changelog
ADR docs/adr-five-layer-config-cascade docs(adr): record the five-layer config cascade 1 none none (no plugin files)
A feat/config-cascade-lib feat(lib): add shared config-cascade resolver and canonical yaml-subset 2-3 ADR merged (A may be drafted while the ADR is in review) user-experience: plugin.json version bump + CHANGELOG.md entry (not fragment mode; its yaml-subset.mjs becomes a generated copy and its yaml-subset suite moves to lib/)
B feat/user-interface-config feat(user-interface): add layered config and /user-interface:setup 4-5 A merged .changes/user-interface/ fragment
C feat/good-css-skill (this worktree) feat(user-interface): add /user-interface:write-css 6-8 B merged .changes/user-interface/ fragment carrying the MIT notice
D feat/write-css-pointers feat: point prototype and visualization at /user-interface:write-css 9 C merged one fragment each under .changes/prototype/ and .changes/visualization/
close-out none n/a 10 A merged none

Every PR opens as a draft and its body follows the PR body contract: Refs: #6594 for the ADR PR, A and B, whose config shape that issue may change; C and D open with a Refs: line to a tracking issue if a search finds one, else No related issue: <reason>; then ## Summary, ## Fix, ## Verification, ## Related. Ready-flip and merge follow AGENTS.md's stop list (ask the user). B and C each start from main after the previous PR merges, because sync-shared-copies.sh --check-bump <base> compares canonicals against the base [EXEC-SHAPE]; stacking on the previous branch is the fallback if a PR stalls in review [FALLBACK].

Q33 (bundle check-css.mjs unmodified or modified) is resolved by Design D6: unmodified vendored copy behind scripts/review.mjs, at skills/write-css/vendor/ (R7). Not re-decided.

Dependencies

  • Upstream: lib/resolve-convention-home.sh (existing canonical), user-experience scripts/lib/yaml-subset.mjs (promoted), upstream scripts/check-css.mjs and LICENSE at b645ab15b9fc (.work/good-css/raw/, data only).
  • Downstream: skills/design/SKILL.md, tests/detect.test.mjs (line 55 asserts the exact project key set), README.md, prerequisites.json mention detect.mjs; tests/routing.test.mjs and routing.schema.json read routing.json; user-experience scripts/detect.mjs and tests/yaml-subset.test.mjs import yaml-subset.mjs; CI runs sync-shared-copies.sh --check, run-plugin-tests.sh, sync-config-cascade-semantics.py --check, typos (vendor excluded) and the changelog-fragment checks; the weekly drift workflow gains one row.

Test strategy

Test-first (Red-Green-Refactor) for every script; evals before skill text (Brief AC). Test types per /testing:plan classification: script logic gets deterministic node --test suites behind *.test.sh wrappers (Design D10); skill behavior gets claude plugin eval cases. Every detector run in tests and sanity checks passes the fixture seams (--home <scratch>, --plugin-list-json plugins/user-interface/tests/fixtures/plugin-list.json, --mcp-list plugins/user-interface/tests/fixtures/mcp-list.txt), so no live claude call.

Test boundaries (approval settles them):

Boundary Status First failing test
lib/yaml-subset.mjs exported parse API existing (user-experience), relocated lib/yaml-subset.test.mjs moved from plugins/user-experience/tests/yaml-subset.test.mjs, red until the file exists; the user-experience suite is removed
lib/config-cascade.mjs resolve({plugin, projectRoot, home, schema, defaults, userConfig, teamOnly}) new five-layer later-wins, *.disable union, invalid value keeps lower layer with an error, teamOnly key rejected in layers 2, 3, 5, provenance per key, prose order 3-4-5, legacy[] report, absent layers state: absent, no spawn of any script
plugins/user-interface/reference/defaults.yaml new tests/defaults.test.mjs parses the shipped file with yaml-subset and validates it against team.schema.json
plugins/user-interface/scripts/detect.mjs CLI JSON existing, additive fields tests/detect.test.mjs: project.browserslist, project.lint, project.style_files; --config top-level config with per-key provenance across layers 1-5; --team FILE seam; --user-config FILE; line 55 key-set assertion updated
reference/team.schema.json new unknown key, bad enum and layer-5 routing rejected through the resolver tests
reference/routing.json existing tests/routing.test.mjs asserts a css-authoring rank-1 own-plugin row
skills/write-css/scripts/review.mjs CLI new stdout sections ## Findings / ## Suppressed / ## Judgment, rule ids, rules.disable suppression from --config JSON, exit 0 clean / 1 findings / 2 usage or crash (a missing path must exit 2, not 1)
scripts/check-good-css-drift.sh --check new tests/check-good-css-drift.test.sh with a stubbed fetch: same SHAs exit 0, changed SHA exit 1, fetch failure exit 2
claude plugin eval cases plugins/user-interface/evals/css-* new baseline run without the plugin before SKILL.md exists

Edge cases: empty and comment-only YAML layers; a key valid in layer 4 and invalid in layer 5; a symlinked or .claude/ convention home (FAIL, as in user-experience setup); a file with no <style> block; an uppercase .CSS suffix; browserslist in package.json versus .browserslistrc; a userConfig value containing shell metacharacters (reaches detect.mjs only as file content).

Eval sizing (Q32, resolved here) [EXEC-SHAPE]: 17 cases. Cases 1-8 run in three arms (body: the skill with the rule in SKILL.md; reference: a variant plugin copy under .work/good-css/arm-reference/ with that rule moved to its topic reference; without: no plugin) at runs: 5; cases 9-17 run with versus without at runs: 3, matching the existing ten cases (for example plugins/user-interface/evals/conflicting-request-project-wins/case.yaml).

# Case Checks
1-8 css-rule-hover, -logical, -outline, -ease-in, -color, -transition-all, -motion, -duration one per "In all CSS" rule, three arms; deterministic regex grader where the rule is lexical, model-graded otherwise
9 css-scoped-styles scoped or compiled styles: stack discovered, compiled output checked
10 css-existing-tokens uses the project's tokens, invents none
11 css-no-tokens uses its own defaults and names each
12 css-token-fallback-ask no matching token: asks (default token_fallback: ask)
13 css-project-convention-wins a stated convention beats a good-css rule
14 css-browser-target a technique newer than the browserslist target gets @supports or a stated skip
15 css-research-tool-missing marks unverified checklist answers
16 css-review-mode edits nothing, path:line findings, linter first, Suppressed heading
17 css-control-no-trigger non-CSS and pure design requests do not fire write-css

Keep-in-body threshold (R9): an always-on rule stays in SKILL.md only when the body arm passes at least 4 of 5 runs and beats the reference arm by at least 2 runs; otherwise it moves to its topic reference (C6). The with-versus-without delta is still recorded. Basis: judgment inside the deferred Q32. Cost: /evals:plugin-eval prints the estimate per arm and passes the evals plugin's configured max_cost_usd ceiling; if the estimate exceeds it, run by tag in batches instead of raising the ceiling unasked. If the CLI cannot point a run at the variant copy, run the reference arm from a temporary branch holding the variant. [FALLBACK]

Phase 1: Record the five-layer cascade decision (ADR PR, before PR A) [DONE] (ADR 0061, draft PR #6976; merge pending)

Review: architecture

  • Run /architecture:record-decision for the target five-layer cascade: user-global ~/docs/conventions/<plugin>.{yaml,md}, project personal <home>/<plugin>.local.{yaml,md}, userConfig scalar mirror, one shared resolver lib/config-cascade.mjs that takes the resolved home as input; user-interface as first adopter; links feat(conventions): one setup lifecycle for where plugins store team conventions and settings #6594 (which may supersede it); states the deviation from docs/conventions/config-cascade/README.md "What does not move" (lines 30-36, 84-86) and from ADR 0044's structured team config. [EXEC-SHAPE]
  • Open the ADR PR as a draft (Refs: #6594).
  • Sanity Check: ls docs/adr | grep -ci "config-cascade" returns at least 1; grep -c "6594" docs/adr/*config-cascade*.md returns at least 1; grep -ci "what does not move" docs/adr/*config-cascade*.md returns at least 1.

Phase 2: Canonical yaml-subset (PR A, structural) [DONE] (2bd8e40)

Review: architecture

  • Pre-flight (FIRST): grep -rn "yaml-subset" plugins lib scripts and record every importer (known: user-experience scripts/detect.mjs, tests/yaml-subset.test.mjs); confirm none depends on a path other than scripts/lib/yaml-subset.mjs.
  • Red: move plugins/user-experience/tests/yaml-subset.test.{mjs,sh} to lib/yaml-subset.test.{mjs,sh} pointing at lib/yaml-subset.mjs; the tests live once.
  • Green: create lib/yaml-subset.mjs with the user-experience file's body unchanged; register lib/yaml-subset.mjs plugins/user-experience/scripts/lib/yaml-subset.mjs in scripts/shared-copies.txt; run scripts/sync-shared-copies.sh so the user-experience file becomes a generated copy (header only). Migrating user-experience onto the resolver stays a follow-up (Design D4). [EXEC-SHAPE]
  • Bump the user-experience plugin.json patch version and add a CHANGELOG.md entry.
  • Sanity Check: bash lib/yaml-subset.test.sh exits 0; test ! -e plugins/user-experience/tests/yaml-subset.test.mjs exits 0; bash scripts/sync-shared-copies.sh --check exits 0; diff <(git show main:plugins/user-experience/scripts/lib/yaml-subset.mjs) lib/yaml-subset.mjs exits 0.

Phase 3: Shared resolver lib/config-cascade.mjs (PR A) [DONE] (b0c6e17, 7858090; verifier 14/14 PASS)

Review: architecture

  • Red: lib/config-cascade.test.mjs + .test.sh with fixtures under lib/fixtures/config-cascade/ covering every resolver edge case in the Test strategy table.
  • Green: implement resolve() per Design Contracts (five layers, merge rules, teamOnly, provenance, layers[] state, legacy[]); home is an input, never resolved inside the lib (R1); no plugin-specific key names.
  • Refactor; open PR A as a draft with the PR body contract, linking the ADR PR.
  • Sanity Check: bash lib/config-cascade.test.sh exits 0; grep -cwE "user-interface|css" lib/config-cascade.mjs returns 0; grep -cE "spawn|execFile|resolve-convention-home" lib/config-cascade.mjs returns 0; bash scripts/run-plugin-tests.sh --list | grep -c "lib/config-cascade.test.sh" returns 1.

Phase 4: user-interface config surface and detector (PR B) [DONE] (811dba8..953aa2c; verifiers 10/10 and 7/7 PASS; detect 51/51)

Review: code-design

  • Pre-flight (FIRST): grep -rn "detect.mjs\|routing.json" plugins/user-interface .github scripts and list each parser of the detector JSON (known: skills/design/SKILL.md, tests/detect.test.mjs line 55 exact key set, README.md, prerequisites.json).
  • Register copies in scripts/shared-copies.txt: lib/config-cascade.mjs and lib/yaml-subset.mjs into plugins/user-interface/scripts/lib/, lib/resolve-convention-home.sh into plugins/user-interface/lib/; run the sync script.
  • Red then green: reference/defaults.yaml (layer 1, block-style values from Design Contracts) with tests/defaults.test.mjs loading and validating it; reference/team.schema.json (additionalProperties: false, css plus the existing routing shape, routing team-only); detect.mjs resolves the home through lib/resolve-convention-home.sh (as user-experience detect.mjs:150) and passes it to resolve(), adds --config, --team FILE seam, --user-config FILE, and project.browserslist, project.lint, project.style_files; tests in tests/detect.test.mjs plus fixtures, line 55 updated.
  • .claude-plugin/plugin.json userConfig: css_browser_target, css_important (options), css_layer, css_token_fallback (options), per R5.
File Action Rationale
[ ] scripts/shared-copies.txt MODIFY three registrations
[ ] plugins/user-interface/scripts/lib/config-cascade.mjs CREATE generated copy
[ ] plugins/user-interface/scripts/lib/yaml-subset.mjs CREATE generated copy
[ ] plugins/user-interface/lib/resolve-convention-home.sh CREATE generated copy, spawned by detect.mjs
[ ] plugins/user-interface/reference/defaults.yaml CREATE layer 1
[ ] plugins/user-interface/reference/team.schema.json CREATE schema, routing team-only
[ ] plugins/user-interface/scripts/detect.mjs MODIFY --config, --team, --user-config, project fields
[ ] plugins/user-interface/tests/detect.test.mjs MODIFY new cases, line 55 key set
[ ] plugins/user-interface/tests/defaults.test.{mjs,sh} CREATE shipped defaults load and validate
[ ] plugins/user-interface/tests/fixtures/<new> CREATE browserslist, lint, five-layer config fixtures
[ ] plugins/user-interface/.claude-plugin/plugin.json MODIFY four scalar css_* userConfig keys
[ ] plugins/user-interface/skills/design/SKILL.md KEEP additive fields; audited
  • Sanity Check: bash plugins/user-interface/tests/detect.test.sh exits 0; bash plugins/user-interface/tests/defaults.test.sh exits 0; bash scripts/sync-shared-copies.sh --check exits 0; node plugins/user-interface/scripts/detect.mjs --project plugins/user-interface/tests/fixtures/with-ds --home "$(mktemp -d)" --plugin-list-json plugins/user-interface/tests/fixtures/plugin-list.json --mcp-list plugins/user-interface/tests/fixtures/mcp-list.txt --config | node -e 'const j=JSON.parse(require("fs").readFileSync(0));process.exit(["browserslist","lint","style_files"].every(k=>k in j.project)&&j.config&&"provenance" in j.config?0:1)' exits 0; node -e 'const u=require("./plugins/user-interface/.claude-plugin/plugin.json").userConfig;process.exit(Object.keys(u).filter(k=>k.startsWith("css_")).sort().join()==="css_browser_target,css_important,css_layer,css_token_fallback"?0:1)' exits 0.

Phase 5: /user-interface:setup and convention rows (PR B) [TODO]

  • plugins/user-interface/skills/setup/SKILL.md, disable-model-invocation: true, check|apply [--user|--local] per Design Contracts, modeled on /user-experience:setup and the /developer-experience:setup --user flow (edit the chezmoi source when the file is managed); writes userConfig values to a temp file with the Write tool, never into a shell command; check warns when this worktree has no <home>/user-interface.local.* while the main checkout (first entry of git worktree list --porcelain) has one.
  • plugins/user-interface/skills/setup/evals/evals.json: cases for check output showing provenance per key, the missing-.local-in-worktree warning, and apply writing one key.
  • docs/conventions/config-cascade/README.md Implementers row for user-interface recording the deviation from "What does not move" and citing the Phase 1 ADR; run scripts/sync-config-cascade-semantics.py.
  • docs/conventions/routing-as-data/README.md Adopters row for user-interface: team-layer implementer with css keys, routing team-only, and the structured personal-layer deviation.
  • README config section; a user-interface fragment via scripts/new-changelog-fragment.sh; open PR B as a draft.
  • Sanity Check: bash plugins/skill-quality/scripts/check-skill.sh --require-evals plugins/user-interface/skills exits 0; python3 scripts/sync-config-cascade-semantics.py --check exits 0; grep -c "plugins/user-interface" docs/conventions/config-cascade/README.md returns at least 1; grep -c "provenance" plugins/user-interface/skills/setup/evals/evals.json returns at least 1; ls .changes/user-interface/ | wc -l returns at least 1.

Phase 6: Eval suite before skill text (PR C) [TODO]

  • Create the 17 cases in the Test strategy table under plugins/user-interface/evals/css-*/ (case.yaml, scaffold.sh, graders/*.md, samples/*.json) and plugins/user-interface/skills/write-css/evals/evals.json; cases 1-8 at runs: 5, cases 9-17 at runs: 3; no raw transcripts (eval-case rule). [EXEC-SHAPE]
  • Baseline run without the plugin via /evals:plugin-eval; record per-case pass counts in the PR C body.
File Action Rationale
[ ] plugins/user-interface/evals/css-rule-{hover,logical,outline,ease-in,color,transition-all,motion,duration}/ CREATE one per always-on rule, three arms
[ ] plugins/user-interface/evals/css-scoped-styles/ CREATE scoped styles
[ ] plugins/user-interface/evals/css-existing-tokens/ CREATE token set
[ ] plugins/user-interface/evals/css-no-tokens/ CREATE defaults named
[ ] plugins/user-interface/evals/css-token-fallback-ask/ CREATE token_fallback
[ ] plugins/user-interface/evals/css-project-convention-wins/ CREATE C2
[ ] plugins/user-interface/evals/css-browser-target/ CREATE Q4 order
[ ] plugins/user-interface/evals/css-research-tool-missing/ CREATE IF/THEN AC
[ ] plugins/user-interface/evals/css-review-mode/ CREATE review AC
[ ] plugins/user-interface/evals/css-control-no-trigger/ CREATE trigger boundary
[ ] plugins/user-interface/skills/write-css/evals/evals.json CREATE skill-creator format
  • Sanity Check: python3 -I plugins/evals/skills/validate/scripts/validate-cases.py plugins/user-interface/evals exits 0; ls -d plugins/user-interface/evals/css-* | wc -l returns 17; grep -L "runs: 5" plugins/user-interface/evals/css-rule-*/prompt.md prints nothing (prompt-only cases carry runs: in prompt.md frontmatter); every other css-* case sets runs: 3 in its case.yaml or prompt.md.

Phase 7: Review script and vendored linter (PR C) [DONE] (0645a5d, 57a3f5f, 7a50241; verifier 10/10 PASS; review.test.sh 17/17)

Review: security

  • Copy upstream scripts/check-css.mjs and LICENSE unmodified from .work/good-css/raw/ to plugins/user-interface/skills/write-css/vendor/ (Design D6, R7, Q33).
  • Red: plugins/user-interface/tests/review.test.mjs + .test.sh with CSS and HTML fixtures and a config JSON fixture (each rule id, a rules.disable suppression, a missing path, a crash path).
  • Green: skills/write-css/scripts/review.mjs imports check()/checkFile() from ../vendor/check-css.mjs, expands globs, reads the resolved config from --config FILE|JSON (never spawns detect.mjs, R4), prints the three sections with rule ids, exits 0/1/2.
  • Sanity Check: bash plugins/user-interface/tests/review.test.sh exits 0; git hash-object plugins/user-interface/skills/write-css/vendor/check-css.mjs prints 6fa72714ba18cf02f698fa8015f63df043b3af85; git hash-object plugins/user-interface/skills/write-css/vendor/LICENSE prints 66a325d82e56f2f41ebd14812517d6c50f5b66da; node plugins/user-interface/skills/write-css/scripts/review.mjs --config '{}' does-not-exist.css; test $? -eq 2 exits 0; grep -cE "detect\.mjs|spawn" plugins/user-interface/skills/write-css/scripts/review.mjs returns 0.

Phase 8: write-css skill, references, routing, attribution and drift row (PR C) [TODO]

Review: code-design

  • Pre-flight (FIRST): grep -rn "concern" plugins/user-interface/tests/routing.test.mjs plugins/user-interface/reference/routing.schema.json to confirm the row shape the schema and tests accept.
  • SKILL.md: precedence, four-step process (config read through detect.mjs --config, userConfig values written to a temp file with the Write tool), six-question checklist, always-on rules (provisional until the eval result), Gotchas from upstream "Left out on purpose", review summary (detect.mjs --config then review.mjs --config), arguments (--no-lint, --show-suppressed, browser target), ## Next; no credits or source links.
  • Eleven reference files per the Design module layout; all 47 techniques, each marked with its upstream slug, a one-line rationale and a Baseline status pointer (as-of date + recheck trigger), including the six techniques without upstream support lines (oklch-color, fluid-sizes-with-clamp, content-grid-with-breakouts, stack-layers-with-grid, carousel-on-native-scroll, no-rubber-band-bounce-on-desktop); no browser-version numbers; upstream "Next" items not carried.
  • routing.json css-authoring row (Design Contracts) + a tests/routing.test.mjs case; reference/types/web.md concern list and "When nothing covers the concern" point at write-css; skills/design/SKILL.md routes css-authoring.
  • README fixed-defaults list (mirrors reference/configuration.md: topic split, review output sections and exit codes, four-step process order, project-first precedence); docs/upstream/good-css.md (pin, blob SHAs from Design Contracts, recheck trigger, per-technique credits, MIT notice, in the humanlayer record shape); a fragment carrying the MIT notice.
  • plugins/user-interface/scripts/check-good-css-drift.sh (--check; exit 0 none, 1 drift, 2 unverifiable; modeled on plugins/playwright/skills/playwright/scripts/update.sh) + tests/check-good-css-drift.test.sh; add its row to .github/workflows/maintenance-check-upstream-drift.yml (skill: user-interface:write-css, args: --check, drift: "exit:1", unverifiable: "exit:2") covering scripts/check-css.mjs, LICENSE and PRACTICES.md.
  • Run evals in all arms; apply the keep-in-body threshold; record the per-arm result on PR C; open PR C as a draft.
File Action Rationale
[ ] plugins/user-interface/skills/write-css/SKILL.md CREATE skill body
[ ] plugins/user-interface/skills/write-css/reference/{foundations,layout,spacing-and-shape,text-and-media,interaction,motion,show-and-hide,scroll-and-viewport}.md CREATE 47 techniques by topic
[ ] plugins/user-interface/skills/write-css/reference/{stack-discovery,review,configuration}.md CREATE process, review, fixed defaults
[ ] plugins/user-interface/reference/routing.json MODIFY css-authoring row
[ ] plugins/user-interface/tests/routing.test.mjs MODIFY row assertion
[ ] plugins/user-interface/reference/types/web.md MODIFY pointer
[ ] plugins/user-interface/skills/design/SKILL.md MODIFY route css-authoring
[ ] plugins/user-interface/README.md MODIFY fixed-defaults list, skill entry
[ ] plugins/user-interface/scripts/check-good-css-drift.sh CREATE weekly drift check
[ ] plugins/user-interface/tests/check-good-css-drift.test.sh CREATE exit-code contract
[ ] .github/workflows/maintenance-check-upstream-drift.yml MODIFY one matrix row
[ ] docs/upstream/good-css.md CREATE attribution record, blob SHAs
[ ] .changes/user-interface/<fragment>.md CREATE changelog + MIT notice
  • Sanity Check: with R=plugins/user-interface/skills/write-css/reference and U=plugins/user-interface:
    • bash plugins/skill-quality/scripts/check-skill.sh --require-evals plugins/user-interface/skills exits 0; bash $U/tests/routing.test.sh exits 0; bash $U/tests/check-good-css-drift.test.sh exits 0.
    • Slugs: awk -F'|' '/^\| [0-9]+ \|/{gsub(/ /,"",$3);print $3}' .work/good-css/upstream-map.md | wc -l returns 47, and awk -F'|' '/^\| [0-9]+ \|/{gsub(/ /,"",$3);print $3}' .work/good-css/upstream-map.md | while read -r s; do grep -rqwF -- "$s" $R || echo "MISSING $s"; done prints nothing.
    • Support pointers: for s in oklch-color fluid-sizes-with-clamp content-grid-with-breakouts stack-layers-with-grid carousel-on-native-scroll no-rubber-band-bounce-on-desktop; do grep -rhA40 -wF -- "$s" $R | grep -qi "baseline" || echo "NO POINTER $s"; done prints nothing.
    • No upstream Next: grep -rnE "^#+ Next\b" $R prints nothing; grep -rlE "animation-timeline: *view\(|base-select" plugins/user-interface/skills/write-css prints nothing.
    • Held-fixed list: for k in "topic split" "exit code" "four-step" "project-first"; do grep -qi -- "$k" $U/README.md && grep -qi -- "$k" $R/configuration.md || echo "MISSING $k"; done prints nothing.
    • Attribution: grep -rciE "vojta|holik|good-css\.com|github\.com/vojtaholik" plugins/user-interface/skills/write-css | grep -v ':0$' prints nothing; grep -rcE "(Chrome|Firefox|Safari) [0-9]+" $R | grep -v ':0$' prints nothing; grep -c "b645ab15b9fc" docs/upstream/good-css.md returns at least 1; grep -cE "6fa72714ba18cf02f698fa8015f63df043b3af85|66a325d82e56f2f41ebd14812517d6c50f5b66da" docs/upstream/good-css.md returns at least 2.
    • Listing budget (the script always exits 0, so assert on the report): bash plugins/skill-quality/scripts/check-listing-budget.sh plugins/user-interface/skills | grep -c "WARN" returns 0.
    • Drift row: grep -c "user-interface:write-css" .github/workflows/maintenance-check-upstream-drift.yml returns 1; bash $U/scripts/check-good-css-drift.sh --check exits 0 or 1 (2 means it could not verify).

Phase 9: Presence-gated pointers (PR D) [TODO]

  • One presence-gated /user-interface:write-css reference each in plugins/prototype/skills/explore-directions/SKILL.md and plugins/visualization/skills/visualize/SKILL.md; one fragment per plugin. One PR for both: the same one-line change, and each plugin keeps its own fragment. [EXEC-SHAPE]
  • Sanity Check: grep -c "/user-interface:write-css" plugins/prototype/skills/explore-directions/SKILL.md plugins/visualization/skills/visualize/SKILL.md returns at least 1 for each; bash plugins/skill-quality/scripts/check-skill.sh plugins/prototype/skills plugins/visualization/skills exits 0.

Phase 10: Follow-up issue (close-out after PR A merges) [DONE] (#7027 filed after an empty search; linked in #6984 ## Related)

  • Phase-entry check (first): gh issue list --state all --search 'user-experience config-cascade resolver in:title,body' --json number,title,state.
  • If a match exists → pivot: record its number in PR A's ## Related and skip creation (comment on it only if this session opened it).
  • If empty → gh issue create --title 'refactor(user-experience): read team config through lib/config-cascade.mjs' --body '<scope, Refs: #6594, PR A link>'; the body also names the twin parsers plugins/code-metrics/scripts/yaml_subset.py and plugins/multi-agent/scripts/yaml-subset.awk as candidates to retire onto lib/yaml-subset.mjs or keep with a reason. [FALLBACK]
  • Other out-of-scope items already have an owner: the repo-wide format and migration of other plugins sit on feat(conventions): one setup lifecycle for where plugins store team conventions and settings #6594; no further issues.
  • Sanity Check: the issue number (created or found) is in PR A's body: gh pr view <A> --json body -q .body | grep -cE "#[0-9]+" returns at least 1; gh issue view <N> --json body -q .body | grep -cE "yaml_subset.py|yaml-subset.awk" returns at least 2.

Files affected

Listed per phase above. Totals: 1-2 files in the ADR PR; about 12 in PR A (lib, moved tests, registry, the user-experience copy, version, changelog); about 18 in PR B (adds defaults test and setup evals); about 85 in PR C (17 eval case directories of roughly four files each, 11 references, the skill, scripts, vendor, drift script and row, routing, docs); 4 in PR D.

Alternatives considered

Alternative Why rejected Switch condition
One PR for all work Unreviewable size; contradicts C14 Never for this scope
Merge PR A into PR B A touches user-experience and a repo-level gate; a separate PR keeps the structural change revertable alone (Tidy First) Reviewers ask for one PR and A stays under about 300 lines
Write the ADR at close-out after an admission test Code would ship ahead of the recorded deviation from config-cascade "What does not move"; reviewers of A and B would lack the rationale (R8) Never
Fold the ADR into PR A Mixes a decision review with a code review; the ADR stands without code Reviewers ask for one PR
Shared lib spawns resolve-convention-home.sh itself Generated copies sit at different relative paths per plugin, so a sibling-relative spawn breaks (R1) Never; the caller owns its plugin root
Reuse --team as the resolved-config output switch Breaks user-experience's meaning of --team FILE (R3) Never
review.mjs spawns detect.mjs Adds a second dependency direction and makes review.mjs untestable without a project (R4) Never
Mirror list keys (*.disable) in userConfig multiple options are hidden from /config, so the value would be invisible to the user (R5) Claude Code shows multiple options in /config
Accept routing in every layer Personal layers could reroute a team's agents unreviewed (R6) A routing-as-data update allows personal routing
Vendor under skills/write-css/scripts/vendor/ Outside the typos exclusion plugins/*/skills/*/vendor/**; diverges from playwright (R7) Never
Leave user-experience's yaml-subset.mjs unregistered (two hand copies) Two copies of one parser that drift apart is a silent second way; ADR 0019's Rule of Three is met The --check-bump cost on user-experience proves blocking; then move it to the follow-up issue
Keep the yaml-subset suite in both places Two suites for one file; the canonical owns its tests Never
Migrate user-experience onto the resolver in PR A Design D4 keeps it a follow-up; turns PR A into a behavior change The user asks to include it
One PR per plugin for PR D Two one-line changes; separate fragments already keep per-plugin release notes A plugin owner asks for separate review
Modified check-css.mjs Breaks clean drift diffs and the blob-SHA check (Design D6) Upstream adds rule ids and distinct exit codes, making the wrapper unnecessary
Two-arm (with versus without) evals for always-on rules Shows a rule helps, not that it must sit in the body (R9) Never for the keep-in-body decision
runs: 3 for the rule cases Cannot separate a 2-run margin between body and reference arms The ceiling estimate forces batches and the user prefers fewer runs

Risks and mitigations

Risk Likelihood Impact Mitigation
A registry or header mistake fails sync-shared-copies.sh --check for every PR Low High Phase 2 and 4 Sanity Checks run the gate locally before push
ADR review delays PR A's merge Med Low PR A is drafted while the ADR is in review; only its merge waits
Resolver semantics diverge from #6594's outcome Med Med The resolver hides paths (D4); legacy[] gives a migrate path; the ADR states #6594 may supersede it
Eval estimate exceeds the configured ceiling (three arms, runs: 5) Med Low Run by tag and arm in batches; never raise the ceiling unasked
Always-on rules show no body-over-reference margin and all move to references Med Low That is the Brief's intended outcome (C6); record it on PR C
B or C waits on the previous merge Med Med Stack on the previous branch as the fallback, rebase after merge
A detect.mjs consumer asserts the full key set Known Low tests/detect.test.mjs:55 updated in Phase 4; pre-flight lists any other
A userConfig value with shell metacharacters reaches a shell Low High Values travel only through a file written with the Write tool; edge-case test in Phase 4

Blast radius

Blast radius: MEDIUM. One ADR, a new repo-level canonical lib and four shared-copies.txt registrations under a repo-wide CI gate, one new row in the informational weekly drift workflow, and one other plugin (user-experience) gets a header-only change, a moved test suite and a version bump; every behavior change is additive inside user-interface, the resolver has no existing consumer, and each PR reverts on its own.

Execution shape

Dependency graph: 1 (ADR PR) → 2 → 3 (PR A) → 4 → 5 (PR B) → {6, 7} → 8 (PR C) → 9 (PR D); 10 after PR A merges. Phases 6 and 7 are file-disjoint (evals versus scripts, vendor and tests) and independent, so they run as Wave A in parallel; everything else is sequential. Cost note: two parallel workers roughly double token use for that wave versus sequential. [EXEC-SHAPE]

Agent Phase ALLOWED files LOC
W6 6 plugins/user-interface/evals/css-*/**, plugins/user-interface/skills/write-css/evals/evals.json ~950
W7 7 plugins/user-interface/skills/write-css/scripts/**, plugins/user-interface/skills/write-css/vendor/**, plugins/user-interface/tests/review.test.*, plugins/user-interface/tests/fixtures/review/** ~350

Each agent is FORBIDDEN any file outside its ALLOWED list, PLAN.md, and commit or push (the main session commits). Each brief carries the plan template's divergence-escalation clause verbatim. Sequential fallback: on a scope-fence violation or a cannot-complete report, abort that worker and run 6 → 7 in the main session.

Phase Surface Model Basis
1 main-session opus ADR wording, user interaction in record-decision
2 sub-agent worker sonnet Closed file list, byte-identical move, binary gate checks
3 sub-agent worker opus New shared contract; merge semantics need judgment
4 sub-agent worker opus Cross-module contract change (detector output, schema, userConfig)
5 main-session opus Setup skill text and convention-doc deviation wording
6 sub-agent worker opus Per-case grader design judgment, three-arm setup
7 sub-agent worker opus Wrapper handles untrusted file paths; crash versus finding semantics
8 main-session opus Skill authoring, eval-result decisions, attribution record
9 sub-agent worker sonnet Two one-line edits, grep checks
10 sub-agent worker sonnet Search-before-create, one issue

Decisions made (gate-passed)

Decision What it changes in the plan Basis (evidence) Source
Five PRs (ADR, A, B, C, D) in dependency order, each on its own worktree and branch Approach PR table; phase 1 → ADR, 2-3 → A, 4-5 → B, 6-8 → C, 9 → D C14; Design module layout dependency direction; R8 plan
ADR for the five-layer cascade recorded before PR A; late admission-test phase removed Phase 1; Conventions; Alternatives R8; docs/conventions/config-cascade/README.md:30-36, 84-86 design (R8)
B and C start from merged main; stacking only as fallback Approach; Risks row scripts/sync-shared-copies.sh:7-10: --check-bump <ref> compares canonicals against a base ref plan
resolve() takes home; detect.mjs resolves it through the plugin's lib/resolve-convention-home.sh Phases 3-4; Contracts R1; plugins/user-experience/scripts/detect.mjs:150; scripts/sync-shared-copies.sh:13-15 design (R1)
New detector fields under project, resolved config under top-level config Phase 4; Test strategy R2; plugins/user-interface/tests/detect.test.mjs:55 design (R2)
--team FILE stays a test seam; output switch is --config Phase 4; Contracts R3; plugins/user-experience/scripts/detect.mjs:150 design (R3)
review.mjs takes config JSON, never spawns detect.mjs Phase 7; Contracts R4 design (R4)
userConfig carries four scalar css_* keys; values travel through a Write-tool temp file; OQ1 and OQ2 closed, Brief Out-of-scope and AC lines revised Phase 4; Brief R5; manifest reference (design-threads facts, fetched 2026-10-10); plugins/source-control/skills/worktree/context/create.md "Create the worktree" design (R5)
routing accepted only in layer 4 Phase 4 schema and tests R6; docs/conventions/routing-as-data/README.md:105-108 design (R6)
Vendored linter at skills/write-css/vendor/ Phase 7 R7; .github/workflows/pr-require-checks.yml:809-816; playwright precedent design (R7)
Three-arm evals for the 8 rule cases at runs: 5; keep-in-body when body beats reference by ≥2 of 5 Test strategy; Phases 6, 8 R9; threshold numbers are judgment inside the deferred Q32 design (R9)
user-experience's yaml-subset.mjs becomes a generated copy and its suite moves to lib/ in PR A Phase 2; PR A changelog Design D4; explore.md §4 fact 4 (Rule of Three met) plan
user-experience gets a version bump + CHANGELOG, not a fragment PR A changelog cell scripts/fragment-plugins.txt has no user-experience line; user-interface (86), prototype (70), visualization (88) are listed plan
Canonical lib tests live at lib/<name>.test.{mjs,sh} Phases 2-3 files lib/installed.test.mjs precedent; scripts/run-plugin-tests.sh:5 runs lib/**/*.test.sh plan
Vendored files verified by upstream blob SHA Phase 7 Sanity Check; good-css.md gh api repos/vojtaholik/good-css/contents/<path>?ref=b645ab15... and git hash-object .work/good-css/raw/* agree (this session) plan
Weekly drift row via a plugin-level check-good-css-drift.sh --check Phase 8 .github/workflows/maintenance-check-upstream-drift.yml:11-15, 36-65 (rows name a script with an exit-code contract); skill files carry no source links (Brief AC) plan
Listing-budget check asserts on report text Phase 8 Sanity Check plugins/skill-quality/scripts/check-listing-budget.sh:41 (always exits 0 on a report) plan
Eval cost bounded by the evals plugin's max_cost_usd ceiling; batch by tag if exceeded Test strategy cost line plugins/evals/.claude-plugin/plugin.json max_cost_usd; plugins/evals/skills/plugin-eval/SKILL.md:186-194 plan
PR D covers both prototype and visualization Phase 9; PR D row Identical one-line pointer; per-plugin fragments keep release notes separate plan
Phases 6 and 7 run in parallel; all else sequential Execution shape File-overlap check: disjoint ALLOWED lists; no data dependency plan
Follow-up issue for the user-experience resolver migration names the twin parsers, search before create Phase 10 Design D4; plugins/code-metrics/scripts/yaml_subset.py, plugins/multi-agent/scripts/yaml-subset.awk plan

Open questions

None. OQ1 (userConfig keys) and OQ2 (acceptance-criterion wording) are resolved by R5.

Handoff to implementation

Surface for confirmation before executing: the [FALLBACK] items (stacking a PR on the previous branch, running the reference arm from a temporary branch, the follow-up issue create path), and any change to an acceptance criterion. Every ready-flip and merge asks the user.

kyle-sexton and others added 21 commits October 10, 2026 23:56
user-experience's parser becomes a generated copy of lib/yaml-subset.mjs
(ADR 0019); its tests move to lib/. Parser body unchanged.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
resolve() merges plugin defaults, userConfig, the user-global file, the
project team file and the project personal file, later layers winning per
key. It validates each key against the caller's JSON Schema subset, unions
*.disable lists, accepts team-only keys in the team layer alone, reports
per-key provenance, prose paths, per-layer state and legacy files, and
reads files only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A literal key containing "." collided with the nested path of the same
name in the merge, so it is now rejected with an error naming layer, file
and key, and the lower layer keeps its value. A legacy file is reported
once when the project root is the user home. Tests now cover later file
layers winning on valid scalars and a __proto__ key under a schema that
allows unknown keys, and legacy fixtures hold values no layer resolves.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main released user-experience 0.1.1 for the test-selection declarations
(#6457) while this branch also bumped to 0.1.1. Keep main's 0.1.1 entry
and move this branch's yaml-subset entry to 0.1.2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t scope

Three fixtures mapped to no suite, so a change to them alone ran nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The quote-opening test re-ran a regex over the growing line prefix for
every quote that did not open, so a line holding a long run of quotes
cost quadratic time. Track the last non-blank character instead; the
opening rule is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The team and local layers read only regular files, never a symlink, and
are invalid when a relative convention home resolves outside the project;
every layer refuses a device or FIFO. __proto__, constructor and
prototype are rejected as keys. Also: a rejected value is quoted once, an
empty disable list does not credit its layer, and a tilde fence hides a
config block from the legacy report.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uite

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… and convention-home helper

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…style files from detect.mjs

detect.mjs gains --config (the five-layer cascade with provenance, the
convention home from the project's pointer line), --team FILE and
--user-config FILE seams, and project.browserslist, project.lint and
project.style_files.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…and refuse an escaping --team home

An unusable convention-home pointer now leaves the team and local layers
unread (home null, home_error kept), and --team refuses the local layer
when the real convention home resolves outside the project. A test pins
each css_* userConfig default to reference/defaults.yaml.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ma.json

The routing validator moves from tests/routing.test.mjs to
scripts/lib/routing-validate.mjs; detect.mjs uses it to drop team rows
that break the bundled row schema and names each on the team layer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nd keep scratch paths out of --team output

A team routing row with every required field is validated against the
whole row schema, allOf included; the --team seam reports an absent
local file at its real path; and a convention home named like "..conf"
no longer counts as leaving the project.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…onfig

# Conflicts:
#	lib/config-cascade.mjs
#	lib/config-cascade.test.mjs
#	lib/yaml-subset.mjs
#	lib/yaml-subset.test.mjs
#	plugins/user-experience/CHANGELOG.md
#	scripts/shared-copies.txt
…ig layers

check reports each setting with its source layer, layer errors, a broken
convention home, duplicate personal keys, legacy files, the .local ignore
state and a personal file missing from this worktree; apply writes one key
in one layer. The README gains the five-layer config section, the fixed
defaults, and the regenerated options table.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ter and routing team-layer adopter

The config-cascade Implementers row states the deviation from "What does
not move" and ADR 0044, citing ADR 0061; the glance table is regenerated.
The routing-as-data Adopters row records the team-only routing key and the
structured personal-layer deviation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…point at upstream behavior

A manifest userConfig default is not substituted, so an unset css_* option
reads from a lower layer; setup now leaves empty or literal placeholder
values out of the userConfig file. Volatile /config and chezmoi behavior
becomes pointer records. --user runs the chezmoi managed check, by exit
status, before writing the user-global file too, and check reports a null
local path or a git check-ignore error without stopping.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…without -v

-v exits 0 on a negation match, so the status now comes from the plain
command. --user takes the user-global path from config.layers, and eval 5
expects the README Options pointer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…g per entry

- detect.mjs reads .browserslistrc or browserslist only when its real path
  stays inside the project and it is at most 64 KiB, so a committed symlink
  cannot print a file outside the checkout into the agent's context.
- Team routing is checked entry by entry: an unknown key drops only that key,
  a bad row, disable or deny entry drops alone, and an unknown routing version
  drops routing. A row that matches no bundled concern plus id needs the full
  row schema; disable entries need concern and id.
- The plugin README and the routing-as-data Adopters row say team routing is
  validated but not yet applied by /user-interface:design (#7041).
- /user-interface:setup quotes home_error instead of always blaming the
  pointer line, and apply stops when the target file is a symlink or resolves
  outside the project.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 11, 2026 05:42
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-11T05:46:57.665849Z f6a5a3e Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 50s —— View job


Security review complete

  • Gather context (PR diff, changed files)
  • Review config-cascade resolver and yaml-subset parser for injection/traversal risks
  • Review detect.mjs new flags (--config, --team FILE, --user-config FILE) for shell/path injection
  • Review resolve-convention-home.sh for unsafe shell handling
  • Review /user-interface:setup skill for unsafe write/apply flow
  • Review routing-validate.mjs
  • Check instruction-surface deletions (AGENTS.md/CLAUDE.md/rules changes)
  • Post findings

No security issues found in this PR's diff (origin/main...f6a5a3e).

What I looked at and why it holds up:

  • userConfig → shell/injection path (scripts/detect.mjs): --user-config is read as a file (readFileSync + JSON.parse), never interpolated into a shell string or spawned command. conventionHome() and the (pre-existing) claude() helper both use spawnSync with an argv array, not shell: true with interpolated strings, so no command-injection surface. tests/detect.test.mjs:195-200 explicitly asserts a css.layer value containing "; touch pwned; $(touch pwned2) \touch pwned3` | cat '` stays literal data and creates no files — this is the exact threat a reviewer would otherwise flag, already covered.
  • Symlink / path-escape reads: browserslist() in detect.mjs only reads .browserslistrc/browserslist when escapesProject() says the real path stays inside the project (L87-89), and scratchHome() re-creates a copied symlink rather than following it so the shared resolver's own repo-layer symlink refusal still applies (L213-241). This matches the fix already landed in the HEAD commit (f6a5a3e, "confine browserslist reads...").
  • Prototype pollution: both the vendored yaml-subset.mjs parser and config-cascade.mjs resolver define object keys with Object.defineProperty (own, non-prototype properties) and explicitly reject __proto__/constructor/prototype at every depth (config-cascade.mjs:52,117,133). checkRouting() in detect.mjs operates on values that already passed that filter, so no bypass there.
  • /user-interface:setup (skills/setup/SKILL.md): userConfig values are written via the Write tool to a JSON file, never through a shell command (L30-38); config file content, comments and prose are explicitly framed as data, not instructions, per the untrusted-content convention (L23-26); apply refuses to write when the target resolves outside the project or is a symlink (L81-82).
  • Instruction-surface check: the two convention-doc diffs (config-cascade/README.md, routing-as-data/README.md) are additive rows only — no existing rule is removed, narrowed, or softened.
  • routing-validate.mjs: new RegExp(s.pattern) only ever receives patterns from the repo's own routing.schema.json, not from team/user-controlled config, so no ReDoS-via-untrusted-input path.

I didn't re-flag the symlink/browserslist confinement or shell-metacharacter handling since both are already fixed and test-covered in this branch's head commit.

@claude

claude Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 6m 55s —— View job


Review of PR #7039

  • Gather context (CLAUDE.md, REVIEW.md, diff)
  • Review config-cascade and resolver logic
  • Review detect.mjs changes
  • Review setup skill
  • Review tests and fixtures
  • Adversarial validation of candidate findings
  • Post findings

Scope: Code-review lane only (a pr-review-security-hosted.yml workflow exists in this repo, so security findings are left to that lane per REVIEW.md). Reviewed all 34 changed files against origin/main...HEAD (f6a5a3e).

No blocking or high-confidence findings.

I read scripts/detect.mjs's new config()/scratchHome()/checkRouting()/browserslist()/lint()/styleFiles() logic closely, since it's the riskiest new surface (home resolution, the --team test seam, symlink-escape handling, per-entry routing validation). Specific things I traced through by hand:

  • The escapesProject() guard in scratchHome() only runs when --team triggers the scratch-dir path, but the normal (non-seam) path already gets the same protection from lib/config-cascade.mjs's own escapes() check (plugins/user-interface/scripts/lib/config-cascade.mjs:76-78, :274) before any file in home is read — not a gap.
  • The relabel() path-substitution after reading from the scratch dir, and the "refused" (home-escapes-project) short-circuit, both look correct for the orderings I checked (team file set vs. not, home null vs. resolved vs. escaping).
  • checkRouting()'s per-entry strip/validate against reference/routing.schema.json's row schema (via the new scripts/lib/routing-validate.mjs, moved unchanged out of tests/routing.test.mjs) matches the "full row vs. partial re-rank row" rule described in the PR body.
  • browserslist()/lint()/styleFiles() detection matches the new fixtures (tests/fixtures/css-project, tests/fixtures/browserslistrc-project) exactly, including the uppercase .CSS and case-insensitive <STYLE> block cases.

I checked these against the existing test suite (tests/detect.test.mjs) rather than just reading the implementation, and the tricky cases I was suspicious of (broken pointer line, --team with an escaping home, unknown routing version, partial vs. full row schema) are each covered by a dedicated test whose expectations match what the code actually does.

reference/team.schema.json, reference/defaults.yaml, .claude-plugin/plugin.json's new css_* options, and the doc updates (plugins/user-interface/README.md, docs/conventions/config-cascade/README.md, docs/conventions/routing-as-data/README.md) are consistent with each other and with the PR's documented known limitation (manifest defaults for css_important/css_token_fallback mirroring defaults.yaml without substitution).

Not independently verified: I was not able to execute the test suite or any shell commands in this sandbox (node --test, the *.test.sh wrappers, and even plain bash/node invocations were denied with no prompt handler to approve them). The PR body's 51/51, 3/3, 15/15, and sync-shared-copies.sh --check results are the author's claims, not something I ran myself — the test-by-reading-source-and-tests-together approach above is the closest substitute I had available.

@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review has reviewed this pull request through 42f55cd; a later push is reviewed from there.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6a5a3ed8a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

claude-review has reviewed this pull request through 42f55cd; a later push is reviewed from there.

kyle-sexton and others added 2 commits October 11, 2026 01:51
- detect.mjs marks the `.styl` (Stylus) extension line for the spell checker.
- The user-global fixture denies `figma` instead of a name typos flags.
- setup's Arguments line leads with its argument-hint, per the
  argument-hint convention.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- setup states the check-ignore rule the right way round: without -v a
  negation match exits 1; with -v it exits 0. Adds the pointer record.
- setup's userConfig handoff file carries the session id, so concurrent
  sessions sharing CLAUDE_PLUGIN_DATA cannot overwrite it, and is deleted
  when the skill finishes.
- team.schema.json lets a team row set `style`, as a bundled row may; a test
  keeps the team row fields equal to the row schema's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 35s —— View job


Code review complete

  • Gather context (incremental diff, CLAUDE.md/AGENTS.md, REVIEW.md)
  • Invoke /review:code-review per custom instructions
  • Review plugins/user-interface/reference/team.schema.json changes
  • Review plugins/user-interface/scripts/detect.mjs changes
  • Review plugins/user-interface/skills/setup/SKILL.md changes
  • Review plugins/user-interface/tests/fixtures/config-user-home/docs/conventions/user-interface.yaml
  • Review plugins/user-interface/tests/routing.test.mjs changes
  • Post findings as inline comments; summarize

Scope: Incremental review of the diff since f6a5a3ed8a (.claude-lane/incremental.diff), covering only the 5 files this lane re-reviews. This round is a fix pass for Codex's two prior findings.

1 finding posted inline on plugins/user-interface/skills/setup/SKILL.md (lines 70–76): the fix for Codex's P1 ("Replace the incorrect check-ignore behavior claim") swaps one incorrect claim for another. Per git-check-ignore(1)'s EXIT STATUS section, exit status (0 = ignored, 1 = not ignored) does not change with -v — a negation-rule match always exits 1, with or without -v. The new text claims -v "also exits 0 when a negation rule matches," which is wrong. It's also a second instance of the same underlying problem the P1 finding was about: this is a volatile upstream specific being restated in skill body text rather than only pointed at, which .claude/rules/skill-bodies-state-current-rules.md disallows even when dressed up with a pointer/as-of/recheck-trigger block ("a record never cures a restatement"). Fix this →

No issues found with the rest of the incremental diff:

  • reference/team.schema.json: adding the style ref for team routing rows is correct — routing.schema.json does define style under rows/items/properties (line 50), so the $ref resolves, and this matches the new routing.test.mjs assertion that the team schema's row properties mirror the bundled schema's exactly.
  • scripts/detect.mjs: the // spellchecker:disable-line addition on STYLE_EXT is a no-op comment; this directive is already used elsewhere in the codebase and matches this repo's typos tool convention (_typos.toml).
  • skills/setup/SKILL.md per-session userConfig handoff file (setup-user-config-${CLAUDE_SESSION_ID}.json, deleted on finish): correctly fixes Codex's P2 concurrency finding, and matches the established CLAUDE_SESSION_ID-scoped-temp-file pattern already used in other plugins' setup skills (e.g. context-guard, plugin-quality).
  • tests/fixtures/config-user-home/...user-interface.yaml (canva → figma) and the new routing.test.mjs case: consistent with how other fixtures in this plugin already use figma for deny-list cases.

@claude

claude Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 59s —— View job


Security review complete

  • Gather context (custom instructions, incremental diff scope)
  • Load .claude-lane/incremental.diff and run /review:security-review
  • Review changed files since f6a5a3e: team.schema.json, detect.mjs, setup/SKILL.md, config-user-home fixture, routing.test.mjs
  • Post findings

No security issues found in the incremental diff (f6a5a3ed8a...7b164335bb), reviewed per /review:security-review.

Scope: per the incremental-review header, only the five files changed since the last security-review pass at f6a5a3e were in scope; everything else in this PR was already reviewed at that commit.

What changed and why it's clean:

  • reference/team.schema.json: adds a "style" $ref to the routing row schema (addressing the earlier Codex P2 finding). The referenced property in routing.schema.json constrains it to "enum": ["pixel-art"], so this is a closed value set — no injection or schema-widening risk.
  • scripts/detect.mjs: adds only a // spellchecker:disable-line comment; no functional change.
  • skills/setup/SKILL.md: the userConfig handoff file is now named setup-user-config-${CLAUDE_SESSION_ID}.json instead of a fixed name, with an instruction to delete it when done. This is a genuine hardening over the prior draft — ${CLAUDE_PLUGIN_DATA} is shared machine-wide across sessions (per lib/state-key.sh), so the previous fixed filename let one concurrent session's project/userConfig values leak into or get clobbered by another's; the session-scoped name closes that cross-session data-isolation gap (the P2 finding from the prior Codex pass). The "never through a shell command" / Write-tool-only guardrail for this file is preserved, not weakened. The check-ignore wording edit nearby is a correctness/doc-accuracy fix (addressing the earlier Codex P1 finding), not a security control, so it's out of scope for this lane.
  • The config-user-home fixture (canva → figma in a routing deny list) and the new routing.test.mjs assertion (team schema's routing-row properties match the bundled row schema by $ref) are test-only changes with no runtime security surface.

Instruction-surface check: no standing instruction (guardrail, allowlist/denylist, shell-avoidance rule) is deleted, narrowed, or softened by this diff — the session-scoped temp file change strictly adds isolation, and the Write-tool-only / no-shell-interpolation rule for userConfig values is unchanged.

The user-interface copy makes scripts/lib/yaml-subset.mjs byte-identical in
two plugins, which check-cross-plugin-source-drift.sh rejects unless it is
registered. Its dedicated check is sync-shared-copies.sh --check, so it is
registered with the cluster line the duplication audit reads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 39s —— View job


Security review complete

  • Gather context (incremental diff, custom instructions, repo conventions)
  • Invoke /review:security-review for the incremental scope (scripts/cross-plugin-source-registry.txt)
  • Review the change for security issues
  • Post findings

Scope: incremental lane, reviewing only what changed since 7b164335bb — the single file scripts/cross-plugin-source-registry.txt (per .claude-lane/incremental.diff). Every other file in this PR was already reviewed at that commit.

No security issues found.

The diff adds two lines registering the newly-canonical lib/yaml-subset.mjs as a tracked shared-source cluster:

scripts/lib/yaml-subset.mjs
lib/yaml-subset.mjs -> plugins/*/scripts/lib/yaml-subset.mjs

Permalink

This is a static, plain-text registry with no executable content — it's consumed read-only by check-cross-plugin-source-drift.sh --check (byte-identity check across copies) and by the code-metrics duplication audit's registry-filter.py (excludes the canonical + its listed copies from duplication findings). The added lines:

  • Follow the exact same two-line shape (<bare path> then <canonical> -> <glob>) as every pre-existing cluster in the file (e.g. hook-utils.sh, rewrite-guard.sh), so no new parsing path is introduced.
  • Reference only paths already tracked elsewhere in this PR (lib/yaml-subset.mjs is the new canonical created in scripts/shared-copies.txt; plugins/*/scripts/lib/yaml-subset.mjs is the glob for its generated copies, consistent with scripts/shared-copies.txt's own registration).
  • Contain no shell metacharacters, no interpolation, and no path-escaping concern — the consuming scripts treat each line as a literal path/glob string, and this registry doesn't feed anything into a shell command (confirmed in the earlier full-PR security pass over scripts/lib/config-cascade.mjs/detect.mjs, which is the only part of this PR that spawns processes or builds shell-adjacent values).

No instruction-surface change, no new file I/O behavior, no injection or traversal surface — this is bookkeeping data only.

@claude

claude Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 2s —— View job


Code review complete

  • Gather context (incremental diff, custom instructions, CLAUDE.md/AGENTS.md)
  • Invoke /review:code-review per custom instructions
  • Review scripts/cross-plugin-source-registry.txt change (incremental scope)
  • Post findings / summary

Scope: Incremental review of the diff since 7b164335bb (.claude-lane/incremental.diff), which touches only scripts/cross-plugin-source-registry.txt (+8/-0). Per REVIEW.md/this lane's skill, security findings are out of scope here (a dedicated security-review lane covers this PR); this registry file has no security surface in any case.

No findings.

The change (permalink) registers the new plugins/user-interface/scripts/lib/yaml-subset.mjs generated copy as a cross-plugin cluster:

scripts/lib/yaml-subset.mjs
lib/yaml-subset.mjs -> plugins/*/scripts/lib/yaml-subset.mjs

I verified this by reading scripts/check-cross-plugin-source-drift.sh's logic directly (I could not execute it — shell commands are denied with no prompt handler in this sandbox) and tracing it by hand:

  • scripts/lib/yaml-subset.mjs now exists at that path-within-plugin in two plugins (plugins/user-experience/... and plugins/user-interface/...), both generated copies of the canonical lib/yaml-subset.mjs (identical // GENERATED from lib/yaml-subset.mjs by scripts/sync-shared-copies.sh header, identical body) — so the drift checker's clustering (check-cross-plugin-source-drift.sh:51-68) would see this as a 2-plugin, byte-identical cluster.
  • Without this registration, --check mode (check-cross-plugin-source-drift.sh:199-206) would fail with UNREGISTERED: scripts/lib/yaml-subset.mjs is byte-identical across 2 plugins..., since an identical-but-unregistered cluster is exactly what that check flags.
  • The registration's two-line shape (bare path, then the canonical -> plugins/*/glob cluster line) matches every other sync-shared-copies.sh-backed entry in this file (e.g. lib/config-root.sh at L365-371, lib/context-zone.sh at L134-143), and the glob plugins/*/scripts/lib/yaml-subset.mjs correctly matches both existing copies.
  • The commit message's own stated reason ("makes scripts/lib/yaml-subset.mjs byte-identical in two plugins, which check-cross-plugin-source-drift.sh rejects unless it is registered") matches what the script would actually do — not just restating the author's claim, but independently confirmed by reading the check's code.

I also checked for adjacent gaps the same commit could have had: lib/config-cascade.mjs (vendored only into user-interface so far, one plugin — no cluster, correctly unregistered) and lib/resolve-convention-home.sh (already covered by the pre-existing glob registration at L128/132, which already spans plugins/*/lib/resolve-convention-home.sh) — neither needed a change here.

Every other file in this PR was reviewed at 7b164335bb and is unchanged since.

setup states only the decision (non-verbose exit status; -v to show the
rule) and the pointer record, no restated check-ignore behavior, per the
skill-bodies-state-current-rules rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 56s —— View job


Security review complete

  • Gather context (custom instructions, incremental diff scope: 0e09f194df2f68d20bfead6c2b27ffe02c6a0f5c..42f55cde313364d247a2e95ce416cfb0149f5951)
  • Load .claude-lane/incremental.diff (touches only plugins/user-interface/skills/setup/SKILL.md)
  • Invoke /review:security-review for the incremental scope
  • Post findings

Scope: incremental lane, reviewing only what changed since 0e09f194df2f68d20bfead6c2b27ffe02c6a0f5c — a 4-line change in plugins/user-interface/skills/setup/SKILL.md. Every other file in this PR was already reviewed at that commit.

No security issues found.

The diff (permalink) removes a previously-incorrect restated claim about git check-ignore -v's interaction with negation rules (flagged by the prior code-review pass) and keeps only the operative decision rule plus the existing pointer/as-of/recheck-trigger block:

-  `git -C "<project root>" check-ignore <path>` without `-v`: with `-v` it also exits 0 when a
-  negation (`!`) rule matches, which means the path is not ignored. Run it with `-v` only to show
+  `git -C "<project root>" check-ignore <path>` run without `-v`; run it with `-v` only to show
   the matching rule.

This is a text-only documentation-accuracy fix with no code change:

  • No guardrail weakened. The actual decision mechanism — "decide PASS/FAIL on the exit status of git check-ignore <path> run without -v" — is byte-for-byte unchanged before and after this diff. Only the extra (incorrect) clause describing how -v behaves with negation matches was deleted. The personal-file-must-never-be-committed check this section backs (L69-75) still fires identically.
  • No injection or trust-boundary surface. No shell interpolation, no new file I/O, no new argument handling — the git -C "<project root>" check-ignore <path> invocation form is unchanged from the prior reviewed commit.
  • Instruction-surface check: this is a restatement removal, not a deletion/narrowing of a standing instruction — it brings the file into compliance with .claude/rules/skill-bodies-state-current-rules.md ("a record never cures a restatement") rather than weakening any guardrail, allowlist, or validator. No protected-class rule from the instruction exception register is affected.

@claude

claude Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 21s —— View job


Code review complete

  • Gather context (incremental diff, /review:code-review instructions, prior findings on this file)
  • Review the change to plugins/user-interface/skills/setup/SKILL.md
  • Post findings

Scope: incremental lane, reviewing only what changed since 0e09f194df2f68d20bfead6c2b27ffe02c6a0f5c — the single file plugins/user-interface/skills/setup/SKILL.md (per .claude-lane/incremental.diff). Every other file in this PR was already reviewed at that commit.

No findings.

This commit (42f55cd, "keep the check-ignore rule in setup to a pointer") is the fix for the finding this same lane raised last round on 7b164335bb — that the "Personal file ignored" bullet restated git check-ignore's -v/negation exit-status interaction, and restated it incorrectly. The diff:

-  `git -C "<project root>" check-ignore <path>` without `-v`: with `-v` it also exits 0 when a
-  negation (`!`) rule matches, which means the path is not ignored. Run it with `-v` only to show
+  `git -C "<project root>" check-ignore <path>` run without `-v`; run it with `-v` only to show
   the matching rule. Pointer: <https://git-scm.com/docs/git-check-ignore>. As of: 2026-10-11.

deletes the restated behavior entirely and keeps only the decision ("decide on the exit status of git check-ignore <path> run without -v; use -v only to show which rule matched") plus the existing pointer/as-of/recheck-trigger. This is exactly the remedy .claude/rules/skill-bodies-state-current-rules.md prescribes ("fixed by deleting the restated value and keeping the pointer"), and it no longer asserts anything about how -v and negation interact — so there's nothing left to be factually wrong. The surrounding decision table (Exit 0: PASS, Exit 1: FAIL, other: WARN) is unchanged and was already correct.

The unchanged Pointer:/As of:/Recheck: line itself is outside this diff (context only) and was already in scope in the prior review round at 0e09f194, so it isn't re-litigated here.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 11, 2026
Merged via the queue into main with commit 7b4c40b Oct 11, 2026
29 checks passed
@kyle-sexton
kyle-sexton deleted the feat/user-interface-config branch October 11, 2026 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant