Skip to content

feat(planning): interview recommends downstream model, effort, and advisor - #750

Merged
kyle-sexton merged 3 commits into
mainfrom
feat/231-planning-model-effort-recommend
Jul 20, 2026
Merged

feat(planning): interview recommends downstream model, effort, and advisor#750
kyle-sexton merged 3 commits into
mainfrom
feat/231-planning-model-effort-recommend

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

Implements #231. The interview skill already reads task complexity and ambiguity to drive its rounds — this turns that read into a recommendation, at the stop/handoff boundary, for how the downstream execution session should be configured: model tier, effort level, and advisor pairing. The current model names and accepted pairings are researched dynamically at runtime from the official docs and never pinned in the skill, so the guidance does not rot as models and recommendations change.

Fix

Root cause: the natural place to recommend a session's model/effort was doing complexity assessment but never surfacing a configuration recommendation. Added the behavior where the assessment already happens, following the sibling draft-goal-condition skill's established live-doc discipline (read the current official docs each run; hardcode nothing).

  • skills/interview/SKILL.md — new ## Session-config recommendation (model, effort, advisor) section (lean hub, points to the context spoke). Frontmatter/description unchanged (no new invocation trigger — this is an internal output of the existing flow).
  • skills/interview/context/session-config.md (new spoke) — the detail:
    • Two orthogonal knobs per the official distinction: raise the model when the assistant would be confidently wrong despite full context (capability ceiling); raise effort when it would under-explore/under-verify (thoroughness).
    • Advisor pairing — a faster main model without a stronger advisor is not the recommended config for non-trivial work; names the current documented pairing (Sonnet main + Opus advisor) explicitly as "as of the contract this targets," deferring the live values to the fetch.
    • Read live, never pin — primary sources listed (model-config, advisor docs, the two blogs); fetched once when the recommendation is formed.
    • Graceful degrade — a doc-fetch failure falls back to the durable distinction with a visible note (cite the URL), never halts the interview or guesses a model name.
    • Advisory framing — the skill knows its own main model but cannot read the current effort/advisor state, so it recommends a delta applied via /model, /advisor, the effort setting.
    • Both domains (engineering + general) and the inverse mid-task direction ("too complex for the current model/effort").
  • skills/interview/templates/checklist.md — Step 5 now includes the recommendation.
  • skills/interview/evals/evals.json — new eval (id 9) exercising the behavior, including the graceful-degrade and never-pin expectations.
  • Version bump 0.22.2 → 0.23.0 (minor — new capability) + CHANGELOG entry.

Verification

$ bash scripts/check-changed-skills.sh origin/main
CHECK-SKILL interview: PASS — 0 errors, 2 warning(s)
1 skill(s) checked, 0 failed.

(Both WARNs are pre-existing: SKILL.md soft-line-target 255>200, and the description's non-'Use when' phrasing — neither introduced by this change; all 5 base-ref trigger phrases preserved.)

$ bash scripts/check-changelog-parity.sh --check-bump origin/main
Every plugin whose version changed vs origin/main has a '## [<version>]' CHANGELOG.md entry.

$ node scripts/validate-plugin-contracts.mjs
Plugin contracts validated: 33 setup skills and 1829 plugin files checked.

$ npx --no-install markdownlint-cli2 <changed .md files>
Summary: 0 error(s)

$ node -e "JSON.parse(fs.readFileSync('.../interview/evals/evals.json'))"
evals.json valid JSON

Both live doc sources were confirmed reachable and current during authoring: code.claude.com/docs/en/model-config.md and code.claude.com/docs/en/advisor.md (the latter documents the Sonnet main + Opus advisor pairing).

Closes #231

Related

  • Mirrors the live-doc / never-pin discipline of the sibling draft-goal-condition skill (planning v0.22.0).
  • Sources verified in the issue (2026-07-16): model-config.md · advisor.md · the-advisor-strategy · claude-model-and-effort-level-in-claude-code.

…visor

The interview already reads task complexity and ambiguity to drive its rounds.
At the stop/handoff boundary it now turns that read into a recommendation for
the downstream execution session: a model tier (capability), an effort level
(thoroughness), and the advisor pairing when the main model is a faster tier —
picked per the official capability-vs-thoroughness distinction. Current model
names and accepted pairings are read live from the official docs each run and
never pinned (durable distinction stable, names drift), mirroring
draft-goal-condition's live-doc discipline. A doc-fetch failure degrades to the
durable distinction with a visible note rather than halting or guessing a name.
Advisory only, fires for engineering and general sessions, and carries an
inverse mid-task direction.

Closes #231

Co-Authored-By: Claude Opus <noreply@anthropic.com>
@kyle-sexton kyle-sexton added the automated Opened by automation. label Jul 20, 2026
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

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


Security Review

  • Gather context and read PR changes
  • Review changed files for security vulnerabilities
  • Post findings

No security issues found.

All six changed files are markdown skill instructions, a JSON eval, and a version bump. There is no executable code in this diff.

Files reviewed:

  • plugin.json — version bump only
  • CHANGELOG.md — documentation only
  • SKILL.md — markdown skill instructions (no executable code)
  • context/session-config.md — new markdown context file
  • evals/evals.json — eval test data (JSON, not executed)
  • templates/checklist.md — one-line text change

External URL fetching (prompt injection surface — assessed as acceptable):
The new session-config.md instructs the AI to fetch four URLs at runtime to source current model names and advisor pairings. All four are on Anthropic-controlled domains (code.claude.com and claude.com). Fetched content informs advisory text shown to the user — it does not flow into commands, file writes, or tool calls.

The trust boundary here is Anthropic's own infrastructure, which is the same level already accepted by the sibling draft-goal-condition skill's live-doc pattern. An attacker who could inject content into code.claude.com would need to have already compromised Anthropic's own documentation servers — outside the threat model for a plugin PR review.

No other surfaces: no shell templating, no path construction, no secrets handling, no workflow files, no dependency changes, no access-control modifications.

@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — feat(planning): interview recommends downstream model, effort, and advisor

  • Gather context and read changed files
  • Review plugin.json version bump
  • Review CHANGELOG.md entry
  • Review SKILL.md additions
  • Review new context/session-config.md spoke
  • Review evals/evals.json additions
  • Review templates/checklist.md change
  • Synthesize findings and post review

Summary

The implementation is sound and follows established patterns in this plugin. The design correctly separates two orthogonal knobs (capability vs. thoroughness), mirrors the draft-goal-condition live-doc discipline, degrades gracefully on fetch failure, and wires the recommendation into the right lifecycle point (stop/handoff boundary). Version bump (minor, new capability) and CHANGELOG entry are both correct.

Three findings below, ordered by severity.


Findings

1. URL format inconsistency with CLAUDE.md canonical table (low)

context/session-config.md lines 54–55 lists two primary sources with .md suffixes:

https://code.claude.com/docs/en/model-config.md
https://code.claude.com/docs/en/advisor.md

The CLAUDE.md canonical URL table uses the suffix-free form throughout (e.g. https://code.claude.com/docs/en/skills, https://code.claude.com/docs/en/settings). The PR description confirms both .md-suffixed URLs were verified reachable, so this may be a docs server behaviour where both resolve, but the inconsistency with the repo's own URL convention is worth noting. If the docs server later drops the .md aliases this becomes a latent break. Consider using the suffix-free form to match CLAUDE.md:

https://code.claude.com/docs/en/model-config
https://code.claude.com/docs/en/advisor

Fix this →


2. Naming Sonnet+Opus inline creates a training-time anchor despite the warning (low)

context/session-config.md lines 34–36:

As of the contract this skill targets, that pairing is **Sonnet main + Opus
advisor** — but the current model names and which pairings are accepted are exactly
the values that drift, so source them live (below), never from this sentence.

The "never from this sentence" guard is clever but fragile: a model reading this at recommendation-time will have both the concrete names and the instruction to fetch live in context. Under normal operation (successful fetch) the live names win. Under a fetch failure the graceful-degrade path explicitly falls back to the durable distinction — not to the inline names — which is correct. The risk is a partially-degraded path where the fetch returns a network error but the model still emits "Sonnet + Opus" from this sentence rather than the described durable distinction. Since the inline names are already in the PR description's "sources verified" note and the PR author accepted the tradeoff, this is a watch item rather than a blocker — but tightening the degrade wording in SKILL.md and session-config.md to say "do not use names from this file" rather than "do not use names from this sentence" would be slightly stronger.


3. Graceful-degrade path is asserted but not isolated in evals (observation)

evals/evals.json eval id 9 covers the happy path and asserts the degrade expectation as a property of the expected output. In practice the degrade path fires only when WebFetch fails, which the eval runner would need to simulate (a mocked 4xx/5xx or network unavailability). Without a separate eval that forces the failure condition, the degrade branch is expressed as an assertion the eval runner likely cannot exercise in isolation. This is a limitation of the eval format rather than a bug in the skill — noting it as context for whoever extends evals coverage later.


Positive notes

  • The two-knob framing (capability vs. thoroughness) is the correct abstraction. The SKILL.md summary is a good lean hub and session-config.md is the right depth for the detail.
  • Advisory framing is properly scoped: the skill correctly acknowledges it cannot read current effort/advisor state and recommends a delta, not a state assertion.
  • The "inverse mid-task direction" paragraph in session-config.md (surface "too complex for current config" during execution) is a valuable addition that closes the feedback loop, not just the planning boundary.
  • Checklist Step 5 update is minimal and correctly scoped — "Both:" applies the recommendation orthogonally to the engineering/general split.
  • Eval id 9 prompt is a good complexity signal (re-arch of cross-service authorization with invariant uncertainty) — the kind of task that should trigger a model-tier recommendation.

@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — feat(planning): interview recommends downstream model, effort, and advisor

  • Gather context and understand the request
  • Read PR diff and changed files
  • Analyze for security vulnerabilities
  • Post findings

No security issues found.

All six changed files are markdown skill instructions, a JSON eval, and a version bump. There is no executable code in this diff.

Files reviewed

File Change Notes
plugin.json version 0.22.2 → 0.23.0 Version bump only; no new permissions, scopes, or config keys
CHANGELOG.md +22 lines Documentation only
skills/interview/SKILL.md +30 lines New ## Session-config recommendation section; no new shell commands
skills/interview/context/session-config.md New file, +88 lines Markdown guidance with four external URL fetches
skills/interview/evals/evals.json +14 lines Eval test data (JSON, not executed)
skills/interview/templates/checklist.md +1/-1 One-line text change to Step 5

Prompt-injection surface — doc fetching (assessed: acceptable)

The new session-config.md lines 54–57 instructs the AI to fetch four URLs at recommendation time:

https://code.claude.com/docs/en/model-config.md
https://claude.com/blog/claude-model-and-effort-level-in-claude-code
https://code.claude.com/docs/en/advisor.md
https://claude.com/blog/the-advisor-strategy

All four are Anthropic-controlled domains. The fetched content informs advisory text shown to the user — it does not flow into shell commands, file writes, file paths, or tool arguments. An attacker exploiting this surface would need to have already compromised Anthropic's documentation infrastructure, which is outside the threat model for a plugin PR review.

This is identical in character to the already-accepted draft-goal-condition skill's live-doc discipline. The graceful-degrade path (fetch failure → durable distinction + visible note, never a model-name guess) is the correct posture and does not introduce a secondary injection surface.

Other surfaces — all clear

  • Shell commands: The pre-computed context block in SKILL.md (three git commands) is pre-existing and unchanged by this diff.
  • Secrets / credentials: None referenced in any changed file.
  • Dependency / supply-chain changes: None.
  • GitHub Actions / workflow changes: None.
  • Access-control or permission changes: None — plugin.json has no new userConfig keys, scopes, or permission fields.
  • Data egress: The only egress is the four Anthropic-domain doc fetches described above.

@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — feat(planning): interview recommends downstream model, effort, and advisor

  • Gather context and understand the request
  • Read changed files
  • Review plugin.json version bump
  • Review CHANGELOG.md entry
  • Review SKILL.md additions
  • Review new context/session-config.md spoke
  • Review evals/evals.json additions
  • Review templates/checklist.md change
  • Synthesize findings and post review

Summary

The implementation is correct and well-structured. The two-knob framing (capability vs. thoroughness), live-doc discipline, graceful degrade, and advisory framing are all handled properly. The hub/spoke split between SKILL.md and the new context/session-config.md is the right shape. One actionable fix below; two observations to track.


Findings

1. URL suffix inconsistency — fixed locally, push blocked (low)

context/session-config.md lines 54 and 56 uses .md-suffixed URLs:

https://code.claude.com/docs/en/model-config.md
https://code.claude.com/docs/en/advisor.md

The CLAUDE.md canonical URL table uses the suffix-free form throughout (e.g. https://code.claude.com/docs/en/skills). This is inconsistent with the repo's own convention and could become a latent break if the docs server drops .md aliases. I committed the fix locally (a1f60b9) but a push permission error prevented it landing on the branch. The fix is two-line:

- `https://code.claude.com/docs/en/model-config.md` — model aliases and the effort setting
+ `https://code.claude.com/docs/en/model-config` — model aliases and the effort setting
  `https://claude.com/blog/claude-model-and-effort-level-in-claude-code` — which model and effort fit which work
- `https://code.claude.com/docs/en/advisor.md` — advisor enablement and accepted main+advisor pairings
+ `https://code.claude.com/docs/en/advisor` — advisor enablement and accepted main+advisor pairings

Fix this →


2. Inline model name creates a training-time anchor despite the guard (watch item)

context/session-config.md lines 34–36:

As of the contract this skill targets, that pairing is **Sonnet main + Opus
advisor** — but the current model names and which pairings are accepted are exactly
the values that drift, so source them live (below), never from this sentence.

The "never from this sentence" guard is present and clever, but a partially-degraded fetch (error caught, fallback path entered) leaves both the inline names and the fallback instruction in context at the same time. The graceful-degrade path says to fall back to the durable distinction (not to these names), which is correct — but the instruction could be tightened to say "do not use model names from this file" rather than "from this sentence," which is fractionally stronger under a partial-context failure. Not a blocker given the degrade path is explicit, but worth a follow-up tightening.


3. Graceful-degrade eval assertion is not isolatable (observation)

evals/evals.json id 9 correctly asserts the degrade behavior as an expectation, but the degrade path fires only when WebFetch fails — a condition the eval runner would need to simulate (mocked network failure). Without a separate eval that forces the failure, this expectation is expressed but cannot be exercised in isolation. This is a known limitation of the eval format, not a bug in the skill. Note it as a gap for whoever extends eval coverage later.


Positive notes

  • The capability-vs-thoroughness distinction is the correct abstraction and is cleanly explained in both the SKILL.md hub and the spoke.
  • The context/session-config.md "Advisory framing" section correctly scopes what the skill can and cannot know (it knows its own model, cannot read current effort/advisor state) and phrases the recommendation as a delta, not an assertion.
  • The inverse mid-task direction paragraph closes the feedback loop in both directions, not just at the planning boundary.
  • The eval id 9 prompt (re-architecting cross-service authorization with uncertain invariants) is the right kind of complexity signal for triggering a model-tier recommendation.
  • Version bump 0.22.2 → 0.23.0 is correct for a new capability. CHANGELOG entry is thorough and accurately summarizes the behavior.
  • CLAUDE.md alignment: no hardcoded paths, plugin-form-safe file references, no PII/secrets, URL egress limited to Anthropic-controlled domains.

@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: 2203a9e549

ℹ️ 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".

Comment thread plugins/planning/skills/interview/context/session-config.md Outdated
Comment thread plugins/planning/skills/interview/context/session-config.md
Two review findings on the session-config recommendation spoke:

- Remove the inline "Sonnet main + Opus advisor" pairing. Eval id 9 asserts
  names are "not pinned in the skill" and the degrade path must "not guess a
  model name"; the pinned illustration contradicted both and left a stale
  fallback anchor under a partial-degrade fetch failure. The durable
  faster-main + stronger-advisor *shape* stays; the drifting names are sourced
  live only.
- Drop the `.md` suffix from the two code.claude.com/docs URLs to match the
  suffix-free canonical URL convention in CLAUDE.md (L15-29) and the sibling
  draft-goal-condition skill.

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

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — feat(planning): interview recommends downstream model, effort, and advisor

  • Gather context and understand the request
  • Read PR diff and changed files
  • Analyze for security vulnerabilities
  • Post findings

No security issues found.

All six changed files are markdown skill instructions, a JSON eval, and a version bump. There is no executable code introduced by this diff.

Files reviewed

File Change Security-relevant content
plugin.json version 0.22.2 → 0.23.0 Version bump only; no new permissions, userConfig keys, scopes, or egress
CHANGELOG.md +22 lines Documentation only
skills/interview/SKILL.md +30 lines New ## Session-config recommendation section (markdown only); three !git pre-computed context commands are pre-existing — not introduced by this PR
skills/interview/context/session-config.md New file, +89 lines Markdown guidance with four external URL fetches (see below)
skills/interview/evals/evals.json +14 lines Eval test data (JSON, not executed)
skills/interview/templates/checklist.md +1/-1 One-line text change in Step 5

Prompt-injection surface — doc fetching (assessed: acceptable)

session-config.md lines 55–58 instructs the AI to fetch four URLs when forming a recommendation:

https://code.claude.com/docs/en/model-config
https://claude.com/blog/claude-model-and-effort-level-in-claude-code
https://code.claude.com/docs/en/advisor
https://claude.com/blog/the-advisor-strategy

All four are Anthropic-controlled domains. Fetched content informs advisory text shown to the user — it does not flow into shell commands, file writes, file paths, or tool arguments. An attacker exploiting this surface would need to have already compromised Anthropic's documentation infrastructure, which is outside the threat model for a plugin PR review.

Note: the latest commit (be3590c) already corrected the URL format — the .md-suffixed variants from the original PR description are not present in the merged HEAD.

This surface is identical in character to the already-accepted draft-goal-condition skill's live-doc discipline. The graceful-degrade path (fetch failure → durable distinction + visible note, never a guessed model name) is the correct posture and does not introduce a secondary injection surface.

Other surfaces — all clear

  • Shell commands: The three !git pre-computed context commands in SKILL.md are pre-existing and unchanged by this diff. All are read-only (git branch --show-current, git log --oneline -5, git status --porcelain). git log output could theoretically carry adversarial commit messages, but this is a pre-existing surface already accepted by the plugin's design, and the output lands in the AI's advisory context, not in a command or tool call.
  • Secrets / credentials: None referenced in any changed file.
  • Dependency / supply-chain changes: None.
  • GitHub Actions / workflow changes: None.
  • Access-control or permission changes: None — plugin.json has no new userConfig keys, scopes, or permission fields.
  • Data egress: Limited to the four Anthropic-domain doc fetches described above.

@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — feat(planning): interview recommends downstream model, effort, and advisor

  • Gather context and understand the request
  • Read changed files
  • Review plugin.json version bump
  • Review CHANGELOG.md entry
  • Review SKILL.md additions
  • Review new context/session-config.md
  • Review evals/evals.json additions
  • Review templates/checklist.md change
  • Synthesize findings and post review

Summary

The implementation is correct and the fixup commit (be3590c) proactively addressed both major findings raised in prior reviews before this review ran. The PR is clean at HEAD. Version bump, CHANGELOG, and all six changed files are consistent. Two minor observations below; neither is a blocker.


Fixup commit verification

The prior reviews flagged two issues; both are resolved at HEAD:

1. Pinned advisor names removedcontext/session-config.md lines 34–37 now reads:

The concrete tier names that fill this faster-main + stronger-advisor shape are
exactly the values that drift between versions — and which specific pairings are
accepted drifts with them. Source them live (below), never pin them here.

The "Sonnet main + Opus advisor" anchor is gone. Only the durable shape remains. Eval id 9's "not pinned in the skill" expectation is no longer contradicted by the file's own text.

2. URL suffix fixed — both code.claude.com doc URLs now use the suffix-free form that matches CLAUDE.md's canonical table (lines 55 and 57).


Observations

1. Mid-task inverse direction is advisory text, not an automated signal (watch item)

context/session-config.md lines 84–89 says:

If execution starts showing confidently-wrong-despite-context (raise the model) or
under-exploration / under-verification (raise effort), surface "this may be too
complex for the current model/effort" ...

The Codex review flagged that no mechanism automatically loads this context during an execution session — the inverse-direction guidance is exercised only if the user explicitly invokes /planning:interview (or a skill that pulls this context) mid-execution. This is true. The section is framed as though it fires automatically, but it's advisory text for the model running under this context.

No fix required — it's useful guidance and doesn't create broken behavior. But if the framing gets a future tightening pass, adding "if this skill is invoked mid-execution" would remove the ambiguity about when the guidance applies.

2. Graceful-degrade eval branch is asserted but not isolatable (observation, pre-existing)

evals/evals.json id 9 line 113:

"On a doc-fetch failure the skill degrades to the durable distinction with a visible note and does not halt the interview or guess a model name"

This expectation can only be exercised if WebFetch is mocked to fail. Without a dedicated degrade eval that forces the failure condition, the assertion is expressed but cannot be run in isolation. Known limitation of the eval format — noted as a gap for future coverage.


Positive notes

  • The two-knob framing (capability vs. thoroughness) is the right durable abstraction: it survives model renames because it describes why you raise a tier, not which tier to name.
  • The hub/spoke split — lean summary in SKILL.md, full detail in context/session-config.md — is well-executed and follows the pattern established by context/loop.md.
  • Advisory framing is correctly scoped: the skill knows its own main model and explicitly states it cannot read current effort/advisor state, recommending a delta rather than asserting current config.
  • Checklist Step 5 update ("Both: recommend … never a pinned model name") is minimal, accurate, and orthogonal to the engineering/general routing split already in that line.
  • Version bump 0.22.2 → 0.23.0 is correct for a new capability. CHANGELOG entry is accurate and sufficiently detailed.
  • No hardcoded paths, no secrets, no workflow changes, no new userConfig keys — clean plugin-form footprint per CLAUDE.md design rules.

@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: be3590c95b

ℹ️ 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".

Comment thread plugins/planning/skills/interview/SKILL.md
@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Addressing the claude[bot] PR review findings (commit be3590c95b70739586ffad5b43a601081b6b2558):

# Finding Classification Evidence
1 URL suffix inconsistency — .md-suffixed code.claude.com/docs URLs vs CLAUDE.md's suffix-free convention (low) VALID — fixed Verified CLAUDE.md canonical table (L15-29) is uniformly suffix-free, and the cited sibling draft-goal-condition uses /en/goal for the same live-fetch purpose (so suffix-free serves fetchable content — no regression). Dropped .md from both URLs.
2 Inline model name creates a training-time anchor despite the guard; tighten "this sentence" → "this file" (watch item) VALID — fixed (stronger) Rather than tighten the guard, removed the pinned Sonnet main + Opus advisor names entirely — the root cause. This also aligns to eval id 9's "not pinned in the skill" / "does not guess a model name" expectations. Durable faster-main + stronger-advisor shape retained. Same fix covers the codex L36 inline thread.
3 Graceful-degrade eval assertion not isolatable without a mocked fetch failure (observation) VALID (observation) — no fix Confirmed: eval id 9 asserts the degrade property but the runner cannot force a WebFetch failure in isolation. This is an eval-format limitation, not a skill bug — left as a noted gap for whoever extends eval coverage (a separate forced-failure eval), per the observation's own framing.

CI is green; PR is BLOCKED pending required human review approval.

…el-effort-recommend

# Conflicts:
#	plugins/planning/.claude-plugin/plugin.json
#	plugins/planning/CHANGELOG.md
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

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


Security Review — feat(planning): interview recommends downstream model, effort, and advisor

  • Gather context and understand the request
  • Get PR diff and read changed files
  • Analyze for security vulnerabilities
  • Post findings

No security issues found.

All six changed files are markdown skill instructions, a JSON eval, and a version bump. No executable code is introduced by this diff.

Files reviewed

File Change Security-relevant content
plugin.json 0.22.3 → 0.23.0 Version bump only; no new userConfig keys, scopes, or permissions
CHANGELOG.md +22 lines Documentation only
skills/interview/SKILL.md +30 lines New ## Session-config recommendation section (markdown only); three !git pre-computed context commands are pre-existing and unchanged
skills/interview/context/session-config.md New file, +89 lines Markdown guidance with four external URL fetches (see below)
skills/interview/evals/evals.json +14 lines Eval test data (JSON, not executed)
skills/interview/templates/checklist.md +1/-1 One-line text addition to Step 5

Prompt-injection surface — doc fetching (assessed: acceptable)

session-config.md lines 55–58 instructs the AI to fetch four URLs when forming a recommendation:

https://code.claude.com/docs/en/model-config
https://claude.com/blog/claude-model-and-effort-level-in-claude-code
https://code.claude.com/docs/en/advisor
https://claude.com/blog/the-advisor-strategy

All four are Anthropic-controlled domains (code.claude.com and claude.com). The fetched content informs advisory text shown to the user — it does not flow into shell commands, file writes, file paths, or tool arguments. An attacker exploiting this surface would need to have already compromised Anthropic's documentation or blog infrastructure, which is outside the threat model for a plugin PR review.

Two of the four URLs are blog posts (claude.com/blog/...) rather than versioned docs pages. Blog content is more mutable than reference docs, but the trust boundary is the same (Anthropic-controlled), and the content lands only in advisory text, not in any executed path. This matches the risk level of the docs-page fetches.

The graceful-degrade path — fetch failure falls back to the durable capability/thoroughness distinction with a visible note, never a guessed model name — is the correct posture and does not introduce a secondary injection surface.

This surface is identical in character to the already-accepted draft-goal-condition skill's live-doc discipline.

Other surfaces — all clear

  • Shell commands: The three !git pre-computed context commands in SKILL.md (git branch --show-current, git log --oneline -5, git status --porcelain) are pre-existing and unchanged by this diff. All are read-only. git log output could theoretically carry adversarial commit messages into advisory context, but this is a pre-existing surface already accepted by the plugin's design, and the output lands in the AI's advisory context, not in a command or tool call.
  • Secrets / credentials: None referenced in any changed file.
  • Dependency / supply-chain changes: None.
  • GitHub Actions / workflow changes: None.
  • Access-control or permission changes: None — plugin.json has no new userConfig keys, scopes, or permission fields beyond the pre-existing use_ask_user_question.
  • Data egress: Limited to the four Anthropic-domain fetches described above.
  • Evals JSON: evals.json eval id 9 is test data — the prompt and expected-output strings are not executed as code.

@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — feat(planning): interview recommends downstream model, effort, and advisor

  • Gather context and understand the request
  • Read changed files at HEAD
  • Review plugin.json version bump
  • Review CHANGELOG.md entry
  • Review SKILL.md additions
  • Review context/session-config.md
  • Review evals/evals.json
  • Review templates/checklist.md
  • Evaluate open Codex threads (mid-task trigger, general-interview scope)
  • Synthesize and post findings

Summary

The PR is clean at HEAD. The two fixup-commit resolutions from the prior review cycle (be3590c) are confirmed: pinned model names are gone (only the durable faster-main + stronger-advisor shape remains), and both code.claude.com doc URLs use the suffix-free form. The two-knob framing, live-doc discipline, graceful degrade, advisory framing, and hub/spoke split are all correct.

Two open Codex threads remain, which the author correctly classified as design-intent questions rather than clear bugs. My read of the current file state confirms they are real tensions — minor, non-blocking — and adds precision about what would resolve each.


Findings

1. "Downstream execution session" framing doesn't apply to terminal general sessions (low)

context/session-config.md lines 5–8 frames the whole recommendation as:

this turns that read into a recommendation for how the downstream execution session should be configured.

SKILL.md line 190 says a general session is terminal and has no downstream consumer. Yet session-config.md lines 76–81 and SKILL.md line 225 both say "Both: recommend … for engineering and general sessions alike."

The design intent is defensible — a hard general decision can warrant a better-configured session for whatever comes next — but the preamble's "downstream execution session" is engineering-specific language that doesn't fit a terminal general session. The ## Both domains section explicitly extends the recommendation to general contexts, so the conflict is between the preamble language and the body, not a missing design decision.

A one-line clarification to the preamble (e.g., "downstream execution session (or, for general decisions, the next session in which the decision is actioned)") would close this without changing any behavior.

Fix this →


2. Mid-task inverse direction reads as instruction to an executing agent (low)

context/session-config.md lines 83–89:

If execution starts showing confidently-wrong-despite-context (raise the model) or under-exploration / under-verification (raise effort), surface "this may be too complex for the current model/effort" and recommend the upgrade…

session-config.md is only loaded at the interview's stop/handoff boundary, so this text is only ever read by the interview at handoff. The author's UNCERTAIN response correctly describes it as advisory the interview surfaces to the user as signals to watch for during execution — not runtime behavior the skill claims to auto-fire.

However, the imperative phrasing ("surface", "recommend the upgrade") reads as instruction to an agent operating this context during execution. A model reading this in a future execution context (if this file were ever loaded in a different skill) would take it as an active instruction rather than relay-to-user text. Tightening the framing — e.g., "When handing off, advise the user to watch for these mid-task signals: if execution shows…" — removes the ambiguity without changing what the interview surfaces.

Not a blocker given the file is only loaded at the interview boundary, but worth a follow-up tightening pass.


3. Graceful-degrade eval assertion not isolatable (observation, pre-existing)

evals/evals.json id 9, line 113 asserts the degrade behavior but the path fires only when WebFetch fails — a condition the eval runner would need to simulate. This is a known eval-format limitation, noted across all prior reviews. No action needed here; flagging as a gap for whoever extends eval coverage with a dedicated forced-failure eval.


Fixup-commit confirmation

Both major findings from earlier review rounds are resolved at HEAD (be3590c):

  • Pinned advisor names — removed. Lines 34–37 now carry only the durable faster-main + stronger-advisor shape, not "Sonnet main + Opus advisor." Eval id 9's "not pinned in the skill" expectation is no longer contradicted by the file itself.
  • URL suffix — both code.claude.com doc URLs now use the suffix-free form (/model-config, /advisor), consistent with CLAUDE.md's canonical URL table.

Positive notes

  • The capability-vs-thoroughness two-knob framing is the right durable abstraction — it survives model renames because it describes why to raise a tier.
  • The hub/spoke split (lean summary in SKILL.md:200-228, full detail in context/session-config.md) is well-executed.
  • Advisory framing is correctly scoped: the skill knows its own main model and explicitly states it cannot read current effort/advisor state, recommending a delta rather than asserting current config.
  • Eval id 9's prompt (re-architecting cross-service authorization with invariant uncertainty) is a well-chosen complexity signal that should trigger a model-tier recommendation.
  • Version bump 0.22.2 → 0.23.0 is correct for a new capability. CHANGELOG entry is accurate and detailed.
  • No hardcoded paths, no secrets, URL egress limited to Anthropic-controlled domains — clean plugin-form footprint per CLAUDE.md.

@kyle-sexton
kyle-sexton merged commit 58ee08e into main Jul 20, 2026
21 checks passed
@kyle-sexton
kyle-sexton deleted the feat/231-planning-model-effort-recommend branch July 20, 2026 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated Opened by automation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

planning/interview: recommend model + effort per task, researched dynamically

1 participant