Skip to content

fix(user-interface): treat an unexpanded account_tools_enabled as its default - #10691

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/ui-account-tools-default
Oct 28, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
fix/ui-account-tools-default

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Refs: #9886

Summary

/user-interface:design printed ${user_config.account_tools_enabled} as the current value but gave no rule for a token that arrives unexpanded. The measurement in #9886 on Claude Code 2.1.312 shows an unset key with a manifest default reaches skill content as the literal token, so a model could read the default as off. The gotcha is recorded in docs/upstream/claude-code.md (PR #10675).

Fix

The skill now says an unexpanded token means the key's declared default, true, so account-bound tools are enabled. Same approach as #10675 for /planning:interview. One changelog fragment under .changes/user-interface/.

Verification

Related

🤖 Generated with Claude Code

… default

Refs: #9886

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 28, 2026 16:57
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 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-28T16:58:54.366864Z 1928579 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.

@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 192857913cf2631677ca0c1a9e441077e183b0a8: 0 finding(s), 0 as inline comments.

No security findings.

The change adds one clause to plugins/user-interface/skills/design/SKILL.md (Step 4) and a changelog fragment. The clause says an unexpanded ${user_config.account_tools_enabled} token means the key's default, true.

  • Guard default: plugins/user-interface/.claude-plugin/plugin.json:26 already declares "default": true. The manifest is unchanged, so the default does not widen. The skill text now matches it.
  • What the key gates: it only decides whether the skill routes design concerns to account-bound tools (Claude Design, Figma, axe). It does not gate any trust control, token scope, merge hold or secret. Routing to a tool still requires the tool to be installed and reachable, and the skill already tells the user they may need to sign in.
  • Instruction surface: nothing is deleted or softened. The diff adds a rule and no control is removed.
  • PR context: it contains no embedded directives aimed at the lane.
  • Manifests: no plugin.json or marketplace.json field changed, so there are no manifest <field> lines.
  • Spend: nothing in this change spends prepaid credits or starts a real charge.

critical-findings: 0

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 192857913cf2631677ca0c1a9e441077e183b0a8: 0 finding(s), 0 as inline comments.

No findings. The change adds one sentence to plugins/user-interface/skills/design/SKILL.md Step 4, plus a patch changelog fragment (bump: patch).

  • Closing issue: The PR says Refs: #9886 and has no Closes. The context file has no closing-issue section, so there are no acceptance criteria to check.
  • Default claim: I checked the PR's claim against plugins/user-interface/.claude-plugin/plugin.json. account_tools_enabled is boolean with "default": true, which matches the new text.
  • Manifest: The diff doesn't touch plugin.json or marketplace.json, so no manifest <field> lines apply.
  • Version bump: patch fits a fix to skill instruction text.
  • Security and trust: The edited file is not on the trusted-paths list. An unexpanded token now reads as true, so account-bound tools stay enabled. That matches the declared default and widens no guard.
  • Verification: I did not run anything. The PR's scripts/check-affected.sh result is author-claimed and unverified. The PR also says it did no live 2.1.312 re-probe, so the unexpanded-token behavior rests on the measurement cited in test(planning): probe user_config substitution of an unset default and a saved boolean in a fresh session #9886, which I did not check.
  • Spend: The change spends nothing and has no cost hold.

@kyle-sexton
kyle-sexton enabled auto-merge October 28, 2026 17:00
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The pr-verify verifier failed head 192857913cf2631677ca0c1a9e441077e183b0a8. What it wrote, as data:

Summary: This PR uses `Refs: #9886`, so it has to deliver a named part of #9886 and name what remains. #9886 asks for three things: record the Claude Code version, record whether the unset-default and saved-boolean cases substitute in skill content, and file a follow-up if `planning/skills/interview/SKILL.md` contradicts the result. The PR does none of these. It changes the wording in `user-interface/skills/design/SKILL.md`, which is outside the issue's scope, and its body names no work left on #9886. The wording change itself looks correct and safe.
- PR body: a `Refs:` PR must name the remaining work on the issue. 'Left out: a live 2.1.312 re-probe' is not remaining work from #9886. Either name what is left on #9886, or re-link the PR to an issue whose scope covers this change.
- .changes/user-interface/fix-9886-account-tools-unexpanded-default-3b7d5e12.md: the changelog fragment is fine, but its file name ties the fix to #9886. Rename it to match the issue the PR is re-linked to.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The pr-verify verifier failed head 192857913cf2631677ca0c1a9e441077e183b0a8. What it wrote about this file, as data:

- plugins/user-interface/skills/design/SKILL.md:82: outside the scope of #9886. The issue's done-when items are a measurement comment (now posted by the host run) and, only if the interview skill is contradicted, a follow-up issue for that skill's wording. Changing `/user-interface:design` is not one of them. To fix, open or link an issue that asks for this change, such as a follow-up from the host run's caveat about default-`true` keys, and close or reference that issue instead of #9886.
- plugins/user-interface/skills/design/SKILL.md:82: no correctness or safety problem found. Treating an unexpanded `${user_config.account_tools_enabled}` as its manifest default `true` matches the #9886 measurement (an unset key with a default reaches skill content as the literal token). Per the PR body, `plugin.json` declares the default as `true`; this was not checked against the diff.

Merged via the queue into main with commit b550018 Oct 28, 2026
73 checks passed
@kyle-sexton
kyle-sexton deleted the fix/ui-account-tools-default branch October 28, 2026 17:10
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