feat(planning): add incumbent mode to devils-advocate - #823
Conversation
Extends /planning:devils-advocate with an `incumbent` mode that turns the same adversarial discipline on the status quo — stress-testing an incumbent tool or approach already in place against alternatives, as pre-implementation decision support (not a post-hoc runtime audit). Adds an Alternatives Sweep that runs in place of Rounds 1-4 in this mode: explore the incumbent first-hand via a fresh sub-agent (digest-bias guard — carries what to investigate, never the parent's conclusions), name the problem it solves, walk the native > official > vetted-third-party ladder with coupling priced, and reach a KEEP / MIGRATE / RESEARCH verdict. Inherits the skill's evidence mandate (no training-data-only findings) and routes load-bearing evaluations to /discovery:research, pointing to /re-anchor:pick-for-the-problem for the full selection discipline. Research depth is a per-invocation deep/shallow token, not a userConfig knob. Reconciles the new always-dispatch rule with the skill's existing Fresh-context requirement into one policy. Adds 4 eval cases; planning 0.23.1 -> 0.24.0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Claude finished @kyle-sexton's task in 1m 55s —— View job Security Review —
|
|
Claude finished @kyle-sexton's task in 2m 9s —— View job PR Review: feat(planning): add incumbent mode to devils-advocate
SummaryThis is a clean, additive feature. The Overall: approve with minor observations, none blocking. SKILL.mdWhat changed: frontmatter description, argument-hint, Purpose, Fresh-context requirement, When to Use, Input Resolution (new), Alternatives Sweep (new), Output Format tweak, What This Skill Does NOT Do. Strengths:
Minor observations (none blocking):
evals.jsonFour new evals added (IDs 6–9). Coverage is well-chosen:
The expectations in each eval are specific and testable (not "does it respond" but what the response must and must not contain). Eval 7 in particular is the right adversarial check for the digest-bias constraint. Eval 8 is important — it confirms KEEP is a legitimate outcome, not a fallback. Good breadth for a mode with this many failure surfaces. No issues. plugin.json
CHANGELOG.mdWell-structured Keep-a-Changelog entry. The description accurately reflects the implementation: the sweep steps, the sub-agent dispatch, the coupling-price requirement for MIGRATE, and the scope guard are all mentioned. The "Additive; plan-review mode is unchanged" closing line is the right thing to call out explicitly for consumers reading the changelog. Security / CLAUDE.md gateSkill-only change — no hooks, scripts, MCP servers, userConfig additions, or external egress introduced. No new trust surface. Plugin-acceptance review criteria satisfied. |
Independent review of #823 (clean, no critical). Folds: - README catalog row now documents the incumbent mode / Alternatives Sweep inline, matching sibling rows that describe their sub-behaviors. - Output Format notes how the verdict (KEEP/MIGRATE/RESEARCH) and the Assumption/Failure-scenario field labels re-read in incumbent mode. - Input Resolution adds an escape hatch: an inline plan beginning with incumbent/deep/shallow is passed as a file path so the leading word is not consumed as a mode/depth token. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 55s —— View job Security Review —
|
|
Claude finished @kyle-sexton's task in 2m 16s —— View job PR Review: feat(planning): add incumbent mode to devils-advocate
Two prior Claude reviews already covered this diff thoroughly (security audit + comprehensive review). The second commit ( VerdictApprove with minor observations — none are blocking. The feature is well-scoped, the constraints are correctly implemented in the diff, and plan-review mode is provably unchanged. SKILL.mdWhat's solid:
Remaining minor observations:
evals.jsonThe four new evals (IDs 6–9) are well-targeted — happy path, anti-digest-bias, legitimate KEEP, scope guard. Eval 7 (first-hand not parent digest) and eval 8 (KEEP on a native incumbent) are particularly important for preventing the most likely failure modes. Acknowledged gaps (carried forward from prior review, not blockers):
These are low priority for an additive per-invocation modifier and a naming-collision edge case that the Input Resolution rule already handles. plugin.json / CHANGELOG.md / README.md
SecuritySkill-only change — no new hooks, scripts, MCP servers, |
|
Tower merge-sit: merge-drive lane flagged CYCLE-BUDGET-REACHED (restart pending at next operator ritual), so the tower is sitting merges per the flag protocol. Gate-verified live: CLEAN, checks green (latest do-not-merge run SUCCESS), 0 unresolved threads, head pinned at merge time. This was generated by AI (control tower, session 6). |
Automated-review triage — HEAD
|
| # | Source | Finding | Severity / confidence | Disposition |
|---|---|---|---|---|
| 1 | security-review | Prompt injection via user-controlled [target] forwarded to the dispatched sub-agent |
SUGGESTION / LOW | Deferred with trigger. Not exploitable interactively (attacker is the invoking user; no privilege escalation). Real only if incumbent mode is ever wired into an automated pipeline that builds [target] from untrusted external input — at that point bracket the target as data in the dispatch prompt. No action for interactive use. |
| 2 | pr-review | RESEARCH verdict output underspecified in Output Format (no coupling price / next-step pointer) |
SUGGESTION / LOW | Decline. The routing already exists at the verdict definition — Alternatives Sweep Step 5 defines RESEARCH as "route it (step 4), never a verdict from recall," and Step 4 names /discovery:research. Restating it in Output Format would duplicate a single source of truth. |
| 3 | pr-review | Depth-token parse order not visible in the argument-hint bracket notation |
SUGGESTION / LOW | Decline. The stated failure (incumbent deep react → plan-review mode) misreads Input Resolution: the depth token is consumed only as the leading token, so incumbent is read as the mode and deep react as the target — incumbent mode, not plan-review. The hint already reads "an optional leading deep/shallow," which documents the order. |
| — | codex-connector | "Codex usage limits reached" | informational | No finding; bot quota notice. |
All 22 checks pass. Nothing here blocks.
## What Folds two post-merge review findings from the independent replay review of #823 (merged before the review pass ran): - **Seam disambiguation (IMPORTANT, low confidence):** `devils-advocate` incumbent mode and `/re-anchor:pick-for-the-problem` share semantic trigger territory; neither said when to reach for which. One sentence now names the seam: the corrector is the light in-session nudge when selection drift surfaces mid-conversation; the Alternatives Sweep is the formal, dispatched, verdict-producing review before a plan commits. - **Depth-token footgun (SUGGESTION):** no usage example demonstrated the leading `deep`/`shallow` token, and `incumbent deep <target>` silently folds "deep" into the target. Added the `deep incumbent <target>` example with the ordering caveat. Patch bump 0.24.0 → 0.24.1 (version is the update-delivery vehicle) with CHANGELOG entry. ## Gates - `markdownlint-cli2` clean on changed files - `skill-quality check devils-advocate` PASS (0 errors; 3 pre-existing advisory warnings, unchanged from main) ## Related - No linked issue — post-merge follow-up to #823 (review findings only; closes nothing). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…bent mode (#878) ## 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 Closes #866 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What
Extends
/planning:devils-advocatewith anincumbentmode: the same adversarial discipline, turned on the status quo. Instead of poking holes in a plan you hand it, the mode stress-tests an incumbent tool, library, or approach already in place — asking "is there a better way now" — as pre-implementation decision support, before a plan commits to keeping or replacing it.Invocation:
/planning:devils-advocate [deep|shallow] incumbent [target](target empty ⇒ taken from conversation context). The existing plan-review path is unchanged.A new Alternatives Sweep runs in place of Rounds 1–4 in this mode:
/discovery:explore) — never trusting a parent digest./discovery:research(-deep).Locked design constraints (all satisfied)
incumbentmode ALWAYS dispatches a fresh sub-agent that builds its own context by exploring the incumbent first-hand; the dispatch prompt carries only what to investigate, never the parent's conclusions ("go look yourself," not "confirm my finding"). Reconciled with the skill's pre-existing "Fresh-context requirement" into one coherent two-case policy.[deep|shallow] incumbent [target]unambiguously: depth token stripped first, mode keyword recognized only as the leading token (plan text merely containing the word is not a mode switch), else the existing file/inline/context path./re-anchor:pick-for-the-problem; only a concise self-sufficient baseline is inline. Cross-skill refs degrade gracefully ("if installed").Mode/argument naming result
incumbent(the mode keyword). Distinctive leading token unlikely to lead ordinary plan text; honest (targets the incumbent/status-quo); kebab-case single word; matches the locked design vocabulary ("incumbent-target mode") and the skill's "advocate against the status quo" framing.status-quo(accurate but two-word and less precise — names the situation, not the specific thing in place);reconsider(action-framed but can lead ordinary plan text, making it ambiguous as a leading mode token).challenge— collides with lane-2's new/challengere-anchor skill from this same batch (auto-invocation ambiguity);rethink— same lead-token ambiguity asreconsider.deep/shallowreuse the established ecosystem-deepconvention (e.g.discovery:research-deep); no separate naming pass needed.Depth-configurability decision + token-cost estimate
Decision: per-invocation argument (
deep/shallowtoken), NOT auserConfigknob. Default is the skill's existing risk-scaled research (Round 2's high/medium/low). Depth is a property of this stress-test, not a persistent personal/admin preference.Token-cost estimate (two axes):
userConfigwould instead add aplugin.jsonentry, an enable-time prompt, and a standing${user_config.KEY}surface — a persistent knob for a per-invocation concern — plus the same in-body reference. The two are a wash on tokens; fit is the tiebreaker, not cost.deep=/discovery:research-deepdispatch: tens-of-thousands of tokens burned inside the isolated tier, but only a bounded ~1–3k-token findings summary returns.shallow= codebase read/grep only (cheapest).userConfigis rejected even for the admin/enterprise case: depth of a single stress-test is per-invocation judgment, not a stable per-user/per-repo preference. The skill already auto-scales research to risk, so high-risk items already go deep org-wide; an "always deep" standing knob would mostly forceresearch-deepwhere a grep suffices — waste on low-risk runs. Per the migration playbook's "no speculative knobs / Rule of Three," a standing config surface isn't warranted; the per-invocation override covers the real need at zero standing config cost.Gates
skill-quality:check devils-advocate— PASS, 0 errors. All 4 base-ref description trigger phrases preserved (trigger continuity); description 722/1536 chars. Residual advisory WARNs: SKILL.md 216 lines vs 200 soft target (accepted — a full second mode was added; the Alternatives Sweep is a ~20-line core procedure, and the playbook says not to split merely to shorten a file — 216/500 is well under the hard cap); "no Gotchas surface" and "Use for vs Use when:" WARNs are pre-existing, not introduced here.skill-quality validate-evals— evals.json valid against the bundled 2020-12 schema (ajv--spec=draft2020).claude plugin validate --strict .andclaude plugin validate ./plugins/planning— both pass.userConfig/bin, no egress, no secrets, no new trust surface (surfaces 2/5/6/7 untouched). Clean — no re-trigger beyond noting an additive feature.review:code-reviewer, rationale withheld) audited the diff — clean, no CRITICAL. Findings folded in commit22a03e4e: (IMPORTANT) the per-plugin README catalog row now documents the incumbent mode inline like sibling rows; (SUGGESTION) Output Format notes how the KEEP/MIGRATE/RESEARCH verdict and the Assumption/Failure-scenario labels re-read in incumbent mode; (SUGGESTION) Input Resolution adds an escape hatch for an inline plan beginning withincumbent/deep/shallow. marketplace.json tags reviewed — no action.Mechanics
planningplugin.jsonversion 0.23.1 → 0.24.0 (minor, additive). Keywords addincumbent,alternatives,status-quo.[0.24.0]entry added (Keep-a-Changelog).marketplace.jsonuntouched (no version embedded; schema does not require a tag change).Open items for the user
incumbentas its top recommendation (runners-upunseat,status-quo,challenge-incumbent,reconsider; rejected--incumbent— no flag parser). No reconciliation needed.incumbentmode when a plan-review run surfaces an incumbency-driven assumption. Currentlyincumbentis an explicit mode only.No linked issue — this lands ledger item 4 of the re-anchor round-2 batch-3 plan (tracked in-repo under
.work/re-anchor-round-2/), which has no separate GitHub issue.Related
/challengecorrector). Thechallengekeyword was deliberately avoided for this mode to prevent auto-invocation collision with Lane 2's new/challengeskill./re-anchor:pick-for-the-problem— supplies the full selection discipline this mode's Alternatives Sweep points to (native > official > vetted ladder, coupling price, mandatory research routing) rather than restating.docs/MIGRATION-PLAYBOOK.md(plugin-acceptance security review, extensibility seams) anddocs/PLUGIN-PHILOSOPHY.md— the acceptance gates this change was reviewed against.Process note (SDLC dogfooding)
Ran the re-anchor posture stack inline and the SDLC spine (discovery/explore + fresh-docs research → design/plan → implement → quality-gate/independent review). Fresh official docs (skills + sub-agents) were WebFetched this session and cited in design. Final PR opened via
ghrather than the/source-control:pull-requestmerge-capable flow, to guarantee open-not-merged; the equivalent prep (independent review + verify + validate) was run explicitly.