Skip to content

feat(planning): route incumbency-driven plan-review findings to incumbent mode - #878

Merged
kyle-sexton merged 1 commit into
mainfrom
feat/866-da-autosuggest
Jul 21, 2026
Merged

feat(planning): route incumbency-driven plan-review findings to incumbent mode#878
kyle-sexton merged 1 commit into
mainfrom
feat/866-da-autosuggest

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

What

In devils-advocate plan-review mode (Rounds 1–4), when Round 2's evidence check finds an assumption whose only support is incumbency ("we already use X"), the finding now names the follow-up — /planning:devils-advocate incumbent <target>, the Alternatives Sweep on that incumbent — instead of leaving it as prose. Suggestion only: it is never auto-run, so scope stays one mode per invocation.

Deferred enhancement recorded in #823 (the incumbent mode itself).

Why this placement

Incumbency is not evidence (the skill's Purpose already says "we already use it is evidence of what is, never proof it still fits"). So an incumbency-only assumption fails the Round 2 evidence check → becomes an unverified assumption → flows to a Round 3 finding whose Mitigation names the sweep. That ties the suggestion to finding output, per the acceptance. The authoritative rule (command + "suggest, never auto-run, one mode per invocation" + trigger precision) lives once in Round 2; Suggested Next Steps carries a bare pointer.

Trigger precision: an assumption also backed by a requirement, benchmark, or doc is verified on that evidence and does not trigger the routing.

Eval

Adds case 10 (plan-review-incumbency-assumption-suggests-incumbent-mode): a plain plan-review prompt whose plan leans on an in-house encoder justified only by "we already use it." Expectations verify it (a) stays in plan-review mode and does NOT run the sweep, (b) flags the choice as incumbency-only/unverified, (c) names the exact /planning:devils-advocate incumbent <target> follow-up as a suggestion. Distinct from case 6 (direct incumbent-mode invocation).

Version

Bumps planning to 0.24.3 with a CHANGELOG entry (same diff, changelog-parity gate).

Gates run (all green locally)

  • skill-quality check-skill: PASS (0 errors; pre-existing WARNs only)
  • markdownlint-cli2: 0 errors
  • eval JSON valid + validates against evals.schema.json
  • check-changelog-parity.sh --check and --check-bump origin/main: pass
  • validate-plugins.sh: all manifests + catalog pass

Related

Closes #866

…bent mode

When devils-advocate plan-review mode (Rounds 1-4) surfaces an assumption whose
only support is incumbency ("we already use X"), Round 2's evidence check now
treats it as unverified and the resulting finding's Mitigation names the
follow-up: `/planning:devils-advocate incumbent <target>` — the Alternatives
Sweep on that incumbent. Suggestion only; never auto-run, so scope stays one
mode per invocation. An assumption also backed by a requirement, benchmark, or
doc is verified on that evidence and does not trigger the routing.

Adds eval case 10 covering plan-review (default) mode detecting incumbency-only
support and suggesting — not running — the sweep, distinct from case 6's direct
incumbent-mode invocation. Bumps planning to 0.24.3 with a CHANGELOG entry.

Closes #866

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton enabled auto-merge (squash) July 21, 2026 17:34
@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown

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


PR Review — feat(planning): route incumbency-driven plan-review findings to incumbent mode

Tasks:

  • Gather context and understand the request
  • Review changed files
  • Assess eval correctness
  • Check CHANGELOG and version bump
  • Post findings

Summary

Clean, tightly scoped PR. The logic is sound, placement is correct, the eval exercises the right behavior, and the version bump is appropriately patch-level. No blocking issues.


SKILL.md — Round 2 addition

SKILL.md:109

The new paragraph is placed correctly — it extends the definition of "unverified" at the conclusion of Round 2, right after the evidence-check table format, before Round 3 is invoked. Key aspects check out:

  • Anchor to existing Purpose paragraph: "per Purpose: incumbency is evidence of what is, never proof it still fits" — the rule doesn't introduce a new principle; it operationalizes one that already exists.
  • Routing to Mitigation field specifically: "It flows to a Round 3 finding whose Mitigation names the follow-up" — this correctly uses the existing Round 3 structure rather than inventing a new output slot. The Round 3 template (Mitigation: How to fix) already handles this.
  • Precision guard: The closing sentence ("An assumption also backed by a requirement, benchmark, or doc is verified on that evidence and does not trigger this") is the right edge-case gate. Without it, any plan that incidentally mentions an existing tool would falsely flag.
  • "Suggest it; never auto-run it": Appropriately restated inline, even though the PR body covers it, because this is the authoritative rule location.

SKILL.md:210

The new Suggested Next Steps bullet is a clean pointer-back with no duplication of the rule:

Running the Alternatives Sweep on any incumbent a Round 2 finding flagged as supported only by incumbency — the finding's Mitigation already names the invocation

"The finding's Mitigation already names the invocation" correctly defers authority to Round 2/3 rather than restating the invocation syntax here.


evals.json — Case 10

evals.json:112–123

The three expectations map cleanly to the three behaviors stated in the PR body:

Expectation Behavior
Stays in plan-review mode, does NOT run the sweep Scope guard — one mode per invocation
Flags the encoder choice as incumbency-only/unverified Round 2 routing
Names /planning:devils-advocate incumbent <target> as a suggestion Mitigation field content

The prompt is well-chosen: "we already use it across the other services, so we'll wire it in here too" is a canonical incumbency-only justification with no requirement, benchmark, or doc attached. It correctly excludes the precision-guard edge case (no mixed evidence to complicate grading).

One observation worth noting: the third expectation says "names the exact follow-up /planning:devils-advocate incumbent <target>" — a human evaluator will need to understand <target> is a metavariable to be substituted with the actual target (the in-house YAML encoder), not a literal string. This is consistent with how other eval expectations work in this file (narrative-level, human-graded) and isn't a defect, but it's worth being aware of when grading.

Case 10 is correctly distinct from case 6: case 6 tests that incumbent mode runs the Alternatives Sweep; case 10 tests that plan-review mode does not run the sweep — only suggests it.


CHANGELOG and version bump

The [0.24.3] entry accurately describes the change. Patch-level bump is appropriate: the behavior being added is an enhancement within an existing mode, not a new mode. Phrasing matches the SKILL.md wording.


No blocking issues. Ready to merge.

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown

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


Security Review — PR #878

  • Gather context (PR metadata, changed files)
  • Read changed files for security analysis
  • Report findings

No security issues found.

This PR touches four files: a version bump in plugin.json, a changelog entry, three lines added to SKILL.md, and twelve lines added to evals.json. Here is the surface-level breakdown of each for security:

plugins/planning/.claude-plugin/plugin.json — single-line version bump 0.24.20.24.3. No permissions, no new dependencies, no trust delegation. Clean.

plugins/planning/CHANGELOG.md — documentation only. No execution surface.

plugins/planning/skills/devils-advocate/evals/evals.json — adds eval case 10, a new entry in a JSON test-case array. The added prompt is plain natural language fed to the skill for evaluation; it introduces no code-execution path and no new trust boundary.

plugins/planning/skills/devils-advocate/SKILL.md — the substantive change. The new paragraph at line 109 adds the incumbency routing rule. Specific checks:

  • Shell injection via $ARGUMENTS: $ARGUMENTS appears at line 17 and is processed as natural language read by Claude, not interpolated into a shell command. The only shell execution surfaces in this file are the two !-prefixed backtick expressions at lines 12–13 (git branch --show-current, git log --oneline -5), and both are hardcoded — they do not incorporate $ARGUMENTS or any derived value. No injection risk.
  • Auto-execution of suggested command: The rule explicitly states "Suggest it; never auto-run it — scope stays one mode per invocation." The suggested /planning:devils-advocate incumbent <target> text is emitted as advisory output, not dispatched as a command. No unintended invocation surface.
  • Target propagation: The <target> in the suggestion is filled in by Claude from the plan under review, not by directly echoing unsanitized user input into an executed command. Even if it were, $ARGUMENTS in a subsequent invocation would again be processed as natural language, not as a shell string.
  • No new permissions, secrets, external egress, or trust delegation introduced anywhere in the diff.

@kyle-sexton
kyle-sexton merged commit 12c512b into main Jul 21, 2026
22 of 23 checks passed
@kyle-sexton
kyle-sexton deleted the feat/866-da-autosuggest branch July 21, 2026 17:36
kyle-sexton added a commit that referenced this pull request Jul 21, 2026
Recompose against advanced main: #878 took planning 0.24.3 for the
devils-advocate incumbent-routing change. Kept that entry at 0.24.3 and
moved the interview session-config reframe (Fixed) up to 0.24.4; bumped
plugin.json to match.
kyle-sexton pushed a commit that referenced this pull request Aug 17, 2026
recheck-against-upstream vs mattpocock/skills HEAD 068b6e0: inventory
map verified intact (35 skills, mappings confirmed), no new release;
unreleased drift noted (invocation-reach invariant hardened via #878/
#880, diagnosing-bugs post-mortem step removed) for lane-6 bookkeeping.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LiiQ2eJ6mHqTPsNfTGJzCG
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: auto-suggest devils-advocate incumbent mode when plan review hits an incumbency-driven assumption

1 participant