Skip to content

fix(planning): read an unexpanded user_config token as the key's default - #10675

Merged
kyle-sexton merged 3 commits into
mainfrom
host-run/9886-userconfig-default
Oct 28, 2026
Merged

kyle-sexton merged 3 commits into
mainfrom
host-run/9886-userconfig-default

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Closes #9886

Summary

The #9886 host run on Claude Code 2.1.312 showed that an unset userConfig key with a manifest default reaches skill content as its literal ${user_config.KEY} token. A saved boolean true substitutes as true. /planning:interview read an unexpanded token as unset or off, which inverts shared_session_open_browser, whose default is true. The owner decided not to report this upstream, so it is recorded as a gotcha only.

Fix

  • plugins/planning/skills/interview/SKILL.md: the lane-table line says a value still shown as its token is unset and takes that key's declared default (surface terminal, use_emoji_question_markers false, shared_session_open_browser true). The emoji line names off as the default.

  • plugins/planning/skills/interview/context/lane-shared.md: drop --open only on an explicit false; an unexpanded token keeps --open.

  • docs/upstream/claude-code.md: a dated gotcha citing plugins manifest reference, Reference a saved value and the 2.1.312 probe, with an as-of date and a recheck trigger. The file has no gotcha section, so the note sits beside the existing 280-030 probe note at the end of Declined.

  • plugins/planning/tests/interview-defenses.test.sh: two digests re-pinned with repin.sh:

    • SKILL.md Stance section: 32ca83a2 -> 2425287a
    • SKILL.md Action Router section: cf717454 -> 2d05ea86

    The changed lines name the default an unexpanded token takes. They change how a setting is read, not what a round resolves. The no-silent-resolve and lock defenses are byte-identical.

  • A changelog fragment for planning (patch).

Other planning keys need no change. use_ask_user_question, use_emoji_question_markers, scaffold_stubs and time_units default to false. The phase_order, plan_store and time_units reads treat an unexpanded token as an unset user layer, and the layered resolution in reference/config.md then applies the repository layer or the default.

Default-true keys in other plugins (not changed here):

  • Already apply the default, or act only on an explicit false: actionlint, bash-format, biome-format, code-tidying, context-budget, context-guard, desktop-notification, disk-hygiene, eol-normalizer, evals, github, go-format, harness-ops, markdown-format, playwright (build_edl.py removes a layer only on off/false), powershell-format, rate-limit-guard, repo-hygiene, review, ruff-format, session-flow, typos-format.
  • States no rule for an unexpanded token: plugins/user-interface/skills/design/SKILL.md (around line 81), "account-bound tools are enabled (now: ${user_config.account_tools_enabled})". The model could read an unexpanded token there as off, but no wording says so.

Verification

  • scripts/check-affected.sh --tests: 14 passed, 2 skipped. The skipped tests step left 2 selected suites to their CI lanes.
  • bash plugins/planning/tests/interview-defenses.test.sh: PASS after the re-pin. check-affected --uncommitted did not run this suite, so it was run directly.
  • Left out: a live interview session run; the change is skill wording, and the defenses suite plus CI's skill checks cover it.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 3 commits October 28, 2026 10:07
The interview's lane table and shared lane read an unexpanded
${user_config.KEY} token as unset or off. For shared_session_open_browser,
whose default is true, that inverted the default. Each read now applies
the key's own default, and docs/upstream/claude-code.md records the 2.1.312
probe from #9886 as a gotcha.

Re-pinned interview-defenses digests:
- SKILL.md Stance section: 32ca83a2 -> 2425287a
- SKILL.md Action Router section: cf717454 -> 2d05ea86
The changed lines name the default an unexpanded token takes; the
no-silent-resolve and lock defenses are byte-identical.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 28, 2026 15:25
@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-28T15:26:38.493112Z 4aff889 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 4aff88943569ccb12c76fbc2e39192fa42ec4464: 0 finding(s), 0 as inline comments.

No security issues found in this pull request.

I read the whole diff (.claude-lane/pr.diff) and the PR context. The change is wording in the planning interview skill: an unexpanded ${user_config.KEY} token now reads as that key's declared default. It also adds a docs gotcha, a changelog fragment and two re-pinned digests in interview-defenses.test.sh.

  • Trusted paths: none of the five changed files match a TRUSTED PATHS glob, so no trust control (trusted-trigger, token scope, kill switch, merge hold) is touched.
  • Manifests: no plugin.json or marketplace.json changes, so no manifest <field> lines apply. The userConfig defaults themselves are unchanged. The skill now reads an unexpanded shared_session_open_browser as its declared default true, so it keeps --open. That only opens the owner's own local page in a browser. It gates no access, secret or spend, and I did not treat it as a guard.
  • Instruction-surface deletions: no rule is removed or weakened. The emoji-markers line still reads an unexpanded or empty token as off, now named as the declared default. The lane-shared.md change only narrows when --open is dropped. The tunnel flags and the quoted ${user_config.tunnel_*} substitutions are unchanged, so no new injection surface appears. I did not read the instruction exception register, because no standing rule was removed.
  • Test pins: the two re-pinned digests cover only the changed lines, and the no-silent-resolve and lock defenses stay byte-identical according to the PR comments. Re-pinning does not weaken the check itself.
  • PR context: it contains no embedded instructions aimed at the lane.

critical-findings: 0

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 4aff88943569ccb12c76fbc2e39192fa42ec4464: 0 finding(s), 0 as inline comments.

No findings. The change is skill wording, a docs note, a changelog fragment and two test digest re-pins, and I found no defect in the wording.

What I checked

  • Defaults: The three defaults the new SKILL.md line names match plugins/planning/.claude-plugin/plugin.json: surface is terminal, use_emoji_question_markers is false, and shared_session_open_browser is true.
  • --open rule: The new context/lane-shared.md text, which drops --open only on an explicit false, agrees with the settings table in the same file (shared_session_open_browser defaults to true and "adds --open"). It also agrees with the key's description in plugins/planning/reference/config.md.
  • Emoji fallback: The new "unexpanded or empty means the default, off" wording agrees with emoji_flag in plugins/planning/surface/round.py:2883, where an unexpanded token means false.
  • Spokes: lane-page.md and lane-shared.md both defer to SKILL.md's lane table for the resolved values, so the lane-table line is the single place that needed the fix. lane-page.md passes --open unconditionally and has no inversion.
  • Closing issue test(planning): probe user_config substitution of an unset default and a saved boolean in a fresh session #9886: Its criteria (record the version and per-case text, leave the upstream report to the owner, file a follow-up only if the wording is contradicted) are met by the gotcha note. The issue comment's one caveat, that an unexpanded default-true key must not read as off, is what this PR fixes.
  • Manifest: No plugin.json or marketplace.json change, so no manifest <field> lines apply. The bump: patch fragment matches a wording-only fix.

Not verified

  • I ran no commands. The two re-pinned SHA-256 digests in plugins/planning/tests/interview-defenses.test.sh (2425287a…, 2d05ea86…) are unconfirmed here. The PR's claim that the suite passes after the re-pin is the author's and unverified by me.
  • No test fails if lane-shared.md goes back to dropping --open on an unexpanded token. The re-pinned digests cover only the SKILL.md sections, not lane-shared.md. This is a coverage observation rather than a blocking gap, since the closing issue states no test criterion.

No cost or spend implications.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit a2d003d Oct 28, 2026
75 checks passed
@kyle-sexton
kyle-sexton deleted the host-run/9886-userconfig-default branch October 28, 2026 16:05
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.

test(planning): probe user_config substitution of an unset default and a saved boolean in a fresh session

1 participant