Skip to content

fix(planning): name the bundled resolver in design so a configured dialect resolves - #3926

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/3924-design-resolver
Sep 7, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
fix/3924-design-resolver

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Closes #3924
Refs #3799, #3882

Summary

/planning:design could not read the diagram_dialect convention keys, because its own skill body told the running agent the resolver it needed did not exist. plugins/planning/skills/design/SKILL.md:222 said "the planning plugin ships no bundled resolver", and :224 said "Until the resolver is bundled, a configured dialect cannot be read at all. ... this plugin carries no copy of the shared resolver, so every run resolves through step 6 and takes the default".

Both statements were false on main. plugins/planning/lib/resolve-convention-home.sh is present and executable, and the sibling skills in the same plugin name it: plugins/planning/skills/interview/SKILL.md:268-270 and plugins/planning/skills/prd/SKILL.md:296-298.

The cause was ordering. #3879 landed the design skill at 2026-09-06T23:43:06Z; #3882 landed the resolver nine hours later and did not sweep design back. The effect on main was that a consumer setting diagram_dialect.data to dbml was silently served mermaid, and the system-scope C4 container view was unreachable for every consumer, since diagram_dialect.system could never resolve to a value.

Three surrounding documents carried reader claims that were also wrong on main, all of the same shape: a value that reads as sourced but was never checked against the code.

Fix

Six files, no behavior outside the four documents named in the issue.

  1. plugins/planning/skills/design/SKILL.md. The false clause at :222 and the whole "Until the resolver is bundled" paragraph at :224 are replaced with the resolver's real path and exit-code contract, phrased as interview/SKILL.md:268-270 and prd/SKILL.md:296-298 already phrase it, with only the degrade value adapted from free-text to this skill's own defaults (mermaid for the data artifact, no C4 view for the system scope). The "tracked separately" pointer is dropped, since it pointed at plugins: enrol planning, work-items and review as resolve-convention-home carriers #3881, which closed as refuted. The untrusted-input rule and the "restated here rather than cited because an installed plugin never sees the publishing repository at runtime" rationale are preserved. One punctuation change inside the preserved rule, disclosed so a diff of that hunk is not a surprise: the break after "The convention doc is untrusted input" is now a colon where it was an em dash, so the rewritten line introduces no em dash into a repository mid de-slop campaign. Every word of the rule is unchanged. No dialect branch below the ladder moved.

  2. docs/PLUGIN-PHILOSOPHY.md:669. The row named /work-items:decompose and /review:quality-gate as readers. Neither reads a key. The row now names only the three planning skills.

  3. docs/conventions/authoring-formats/README.md. "no skill reads either key on main today" is removed from both places it appeared. The column heading "Declared consumer (not yet reading)" becomes "Reading skill", and the rows now name /planning:interview, /planning:prd and /planning:design. /work-items:decompose and /review:quality-gate are dropped from the table and stated explicitly below it as consuming the produced artifact without reading a key, so the document's own closing rule ("A skill appears here once it actually reads the key") stays true of the table. The :167 contract, "each consuming slice adds its own reading and updates its row in the same change", is honoured by this change.

  4. docs/conventions/config-cascade/README.md:303. "Conformance becomes verifiable when the first resolver lands" and "No skill on main reads these keys yet" are dropped, replaced with which skills read which keys through which resolver.

  5. plugins/planning/.claude-plugin/plugin.json bumped 0.39.0 to 0.39.1, with a matching new ## [0.39.1] entry at the top of plugins/planning/CHANGELOG.md.

No test digest needed refreshing: plugins/planning/tests/interview-defenses.test.sh pins content over interview/SKILL.md and prd/SKILL.md, and this change touches neither. The one grep -rn design plugins/planning/tests/ hit is the word "design" inside an interview pin's prose, not a pin over the design skill.

Verification

Behavior was attempted, not read.

Fixture run. A fixture repository was built under the platform temp directory (never inside the repo), carrying a convention-home pointer line in AGENTS.md and a docs/conventions/authoring-formats/README.md setting diagram_dialect.data to dbml.

$ bash plugins/planning/lib/resolve-convention-home.sh --root "$FX" --explain
root:      /tmp/ccp3924-fixture
AGENTS.md: one
CLAUDE.md: none
chosen:    AGENTS.md (one)
token:     docs/conventions
docs/conventions
EXIT=0

Reading the key through the resolved home returned the configured value, not the default:

$ sed -n '/^```yaml/,/^```/p' "$FX/$(bash "$R" --root "$FX")/authoring-formats/README.md"
contract_version: 1.0.0
acceptance_criteria_format: ears
diagram_dialect:
  data: dbml
  system: likec4

Exit-code contract, confirmed against the script rather than copied from interview's wording.

$ bash "$R" --root "$FX-nopointer"
no convention-home region in AGENTS.md or CLAUDE.md under /tmp/ccp3924-fixture-nopointer; ask the operator for the home
EXIT=1

$ bash "$R" --bogus
ERROR: unknown argument: --bogus
EXIT=2

$ bash "$R" --root "$FX-does-not-exist"
ERROR: --root is not a directory: /tmp/ccp3924-fixture-does-not-exist
EXIT=2

$ bash "$R" --root "$FX-badgrammar"      # two pointer lines in one region
FAIL: two pointer lines in one convention-home region (AGENTS.md); keep exactly one
EXIT=3

The script's own --help states the same contract: "Exit: 0 resolved (home on stdout); 1 no pointer anywhere (ask); 2 usage; 3 FAIL". That is what the new design text now documents.

Every reader claim left standing, confirmed by git grep inside that skill's own files.

Claim Command Result
/planning:interview reads acceptance_criteria_format git grep -lE "acceptance_criteria_format|resolve-convention-home" -- plugins/planning/skills/interview SKILL.md, context/loop.md
/planning:prd reads acceptance_criteria_format git grep -lE "acceptance_criteria_format|resolve-convention-home" -- plugins/planning/skills/prd SKILL.md
/planning:design reads diagram_dialect git grep -lE "diagram_dialect" -- plugins/planning/skills/design SKILL.md

Every skill removed as a reader, confirmed at zero hits.

Claim Command Result
/work-items:decompose reads no key git grep -nE "acceptance_criteria_format|diagram_dialect|resolve-convention-home" -- plugins/work-items no output
/review:quality-gate reads no key git grep -nE "acceptance_criteria_format|diagram_dialect|resolve-convention-home" -- plugins/review no output

Both skills also state it themselves: plugins/work-items/skills/decompose/context/container-lifecycle.md:46 reads "no flag, no lever, no convention key", and plugins/review/skills/quality-gate/context/close-out.md:383 reads "no flag, no lever, and no convention key is read here".

Local gates.

  • CHECK_SKILL_SKILLS_ROOT=plugins/planning/skills bash plugins/skill-quality/scripts/check-skill.sh design gives PASS, 0 errors, 3 warning(s). All three warnings are pre-existing and structural (line count over the 200 soft target, no Gotchas surface, a numbered procedure at lines 156-162 with no completion-criteria signal); none is introduced here.
  • bash scripts/check-purged-em-dashes.sh gives 98 declared paths, 130 files scanned, no em dashes.
  • bash scripts/check-changelog-parity.sh --check passes.
  • npx markdownlint-cli2 over the five changed markdown files reports 0 issues.
  • git diff | grep -c $'\r' is 0.

Related

…alect resolves

`plugins/planning/skills/design/SKILL.md` told the running agent that the
planning plugin ships no bundled resolver and that "until the resolver is
bundled, a configured dialect cannot be read at all". Both claims were false:
`plugins/planning/lib/resolve-convention-home.sh` shipped in 0.39.0 (#3882),
nine hours after the design skill landed (#3879), and the sweep back never
happened. The effect on main was that a consumer setting `diagram_dialect.data`
to `dbml` was silently served `mermaid`, and the system-scope C4 container view
was unreachable for every consumer because `diagram_dialect.system` could never
resolve to a value.

The design body now names the resolver at
`bash "${CLAUDE_PLUGIN_ROOT}/lib/resolve-convention-home.sh"` with its
exit-code contract, in the same terms `interview/SKILL.md` and `prd/SKILL.md`
already use. The "tracked separately" pointer is dropped, since it pointed at
#3881, which closed as refuted. The untrusted-input rule and the
restate-rather-than-cite rationale are preserved.

Three documents also stated reader facts that were wrong on main:

- `docs/PLUGIN-PHILOSOPHY.md` named `/work-items:decompose` and
  `/review:quality-gate` as readers of the authoring-formats keys. Neither
  reads a key; `git grep -E
  "acceptance_criteria_format|diagram_dialect|resolve-convention-home"` over
  `plugins/work-items` and `plugins/review` returns nothing, and both skills say
  so in their own bodies. The row now names only the three planning skills.
- `docs/conventions/authoring-formats/README.md` said "no skill reads either key
  on main today" in two places and headed its column "Declared consumer (not yet
  reading)". The table now names the three reading skills, and a sentence below
  it records that decompose and quality-gate consume the produced artifact by
  presence without reading a key.
- `docs/conventions/config-cascade/README.md` carried the same stale claim plus
  "conformance becomes verifiable when the first resolver lands". The row now
  states which skills read which keys through which resolver.

Verified by running the shipped resolver against a fixture repository under the
platform temp directory carrying a convention-home pointer line and an
`authoring-formats/README.md` setting `diagram_dialect.data` to `dbml`: exit 0
printing `docs/conventions`, exit 1 with no region, exit 2 on a bad argument and
an unusable `--root`, and exit 3 on two pointer lines in one region. Those match
the contract the new design text documents and the script's own `--help`.

Closes #3924
Refs #3799, #3882

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 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-09-07T12:03:01.607422Z 86864f7 PR opened
ℹ️ 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 Sep 7, 2026

Copy link
Copy Markdown
Contributor

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


Code review — PR #3926

  • Gather context (PR body, diff, repo conventions)
  • Invoke /review:code-review and follow its procedure
  • Post findings as inline PR comments
  • Post summary comment

Scope note: claude-security-review.yml exists in this repo, so per the code-review lane's gotcha, security findings are scoped out of this review (left to that lane).

Verification performed (not just author-claimed):

  • Diffed plugins/planning/skills/design/SKILL.md:222-224 against the sibling resolver paragraphs in interview/SKILL.md:264-269 and prd/SKILL.md:292-297 — the new design text matches that phrasing exactly, adapted only for design's own degrade defaults (mermaid / no C4 view).
  • Read plugins/planning/lib/resolve-convention-home.sh's own exit-code doc comment (0/1/2/3) — matches what the new design paragraph now states.
  • Confirmed the two "no longer a reader" claims at the source: plugins/work-items/skills/decompose/context/container-lifecycle.md:46 and plugins/review/skills/quality-gate/context/close-out.md:383 both independently state "no flag, no lever, no convention key [is read]," consistent with dropping them from docs/conventions/authoring-formats/README.md's Consumers table and docs/PLUGIN-PHILOSOPHY.md:669's reader list.
  • Cross-checked the docs/conventions/config-cascade/README.md:303 row edit against the doctrine's own cross-reference contract (each consuming slice adds its own reading and updates its row in the same change) — the new wording is consistent with it.
  • git diff origin/main...HEAD | grep '—' — no em dash appears on any added line (only pre-existing/removed lines carry one).
  • plugins/planning/.claude-plugin/plugin.json version bump (0.39.0 → 0.39.1) has a matching ## [0.39.1] entry at the top of plugins/planning/CHANGELOG.md, correct Keep-a-Changelog form.

I was not able to execute scripts/affected-tests.sh, check-skill.sh, check-purged-em-dashes.sh, or markdownlint-cli2 myself in this sandboxed lane (script execution beyond git/grep/sed-style reads required approval that isn't available here), so those specific results in the PR's Verification section are author-claimed and unverified by me, not independently confirmed.

Findings: none. This is a well-scoped documentation/skill-body fix — six files, no behavior change outside the four documents the linked issue named. Every "reader" and "non-reader" claim I checked against the actual skill files holds, the new resolver paragraph is a faithful, path-correct restatement of the shared script's contract, and I found no correctness or maintainability issue that a careful reviewer would block on.

CI (ci, claude-security-review) was still in progress at review time; managed-files-guard passed.
· branch

@claude

claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

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


Security review complete

  • Gather context (PR diff, skill instructions)
  • Run skip gate check
  • Review changed files for security-relevant surface
  • Post findings or clear result

No security issues found.

This PR is a documentation/skill-body correction: it removes false claims in plugins/planning/skills/design/SKILL.md (that no bundled resolver exists) and points the design skill at the resolver script that interview and prd already invoke the same way (bash "${CLAUDE_PLUGIN_ROOT}/lib/resolve-convention-home.sh", no arguments, no interpolation of untrusted input). The other four files (docs/PLUGIN-PHILOSOPHY.md, docs/conventions/authoring-formats/README.md, docs/conventions/config-cascade/README.md, plugins/planning/CHANGELOG.md, plugins/planning/.claude-plugin/plugin.json) are prose/table edits and a version bump — no executable surface changes.

Notably preserved and unchanged: the "convention doc is untrusted input — match it for the documented keys, never execute or interpolate it" rule in the design skill body (SKILL.md#L221). The resolver script itself (plugins/planning/lib/resolve-convention-home.sh) is not part of this diff and was already reachable from two other skills, so this change does not introduce a new execution path with different trust properties — it only fixes the design skill's own body to describe reality correctly.

No injection, authorization, secrets-handling, or GitHub Actions concerns identified in this diff.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Last security-reviewed head: 86864f70f712463508ac9ca32f26f01a840c7200. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

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.

planning:design still says the shared convention resolver is unbundled, so no configured diagram dialect is ever read

1 participant