diff --git a/desktop/src/i18n/locales/en.ts b/desktop/src/i18n/locales/en.ts index bf189b80..a7cf763f 100644 --- a/desktop/src/i18n/locales/en.ts +++ b/desktop/src/i18n/locales/en.ts @@ -1276,7 +1276,7 @@ export const en = { 'session.coordinatorChip': 'Orchestration', 'session.coordinatorChipTooltip': 'Orchestration mode is on for this session. Tasks fan out to specialist worker agents and the main thread coordinates them. Toggle it from the + menu in the composer.', 'session.soloPipelineChip': 'Solo pipeline', - 'session.soloPipelineChipTooltip': 'Solo Pipeline mode is on for this session. The AI runs a 5-stage workflow (planner → builder → tester → reviewer → integrator) on its own. Toggle it from the + menu in the composer.', + 'session.soloPipelineChipTooltip': 'Solo Pipeline mode is on for this session. Before implementation, the AI runs an A/B/C plan gate (Planner → Reviewer → Critic), then continues through implement → test → review → land. Toggle it from the + menu in the composer.', 'empty.recentActivity.handoff.branchLine': 'Last session on {branch}: "{title}".', 'empty.recentActivity.handoff.titleLine': 'Last session: "{title}".', 'empty.recentActivity.handoff.filesLine': 'Files touched: {files}{more}.', @@ -1297,7 +1297,7 @@ export const en = { 'solo.suggest.finishWip.title': 'Finish {count} pending change(s)', 'solo.suggest.finishWip.detail': 'Uncommitted: {sample}', 'solo.suggest.finishWip.detailForeign': 'Uncommitted (some may be from another agent): {sample}', - 'solo.suggest.finishWip.taskPrompt': 'Wrap up the {count} uncommitted change(s) in this worktree ({files}). Walk them through plan → implement → test → review → land.', + 'solo.suggest.finishWip.taskPrompt': 'Wrap up the {count} uncommitted change(s) in this worktree ({files}). Run the A/B/C plan gate first, then implement → test → review → land.', 'solo.suggest.shipAhead.title': 'Ship {branch} ({count} commit(s) ahead)', 'solo.suggest.shipAhead.detail': 'Local is {count} commit(s) ahead of upstream. The work is done — review and land.', diff --git a/desktop/src/i18n/locales/jp.ts b/desktop/src/i18n/locales/jp.ts index 70ab735f..e4077234 100644 --- a/desktop/src/i18n/locales/jp.ts +++ b/desktop/src/i18n/locales/jp.ts @@ -1269,7 +1269,7 @@ export const jp: Record = { 'session.coordinatorChip': 'オーケストレーション', 'session.coordinatorChipTooltip': 'このセッションはオーケストレーションモードです。タスクは専門 worker エージェントへ分散し、メインスレッドが調整します。コンポーザーの + メニューから切り替えできます。', 'session.soloPipelineChip': 'ソロパイプライン', - 'session.soloPipelineChipTooltip': 'このセッションはソロパイプラインモードです。AI は 5 段階 (プランナー → 実装 → テスト → レビュー → 統合) を単独で実行します。コンポーザーの + メニューから切り替えできます。', + 'session.soloPipelineChipTooltip': 'このセッションはソロパイプラインモードです。実装前に AI は A/B/C 計画ゲート(Planner → Reviewer → Critic)を実行し、その後 実装 → テスト → レビュー → リリース に進みます。コンポーザーの + メニューから切り替えできます。', 'empty.recentActivity.handoff.branchLine': '前回のセッション ({branch}): 「{title}」。', 'empty.recentActivity.handoff.titleLine': '前回のセッション:「{title}」。', 'empty.recentActivity.handoff.filesLine': '触ったファイル: {files}{more}。', @@ -1283,7 +1283,7 @@ export const jp: Record = { 'solo.suggest.finishWip.title': '{count} 件の未コミット変更を仕上げる', 'solo.suggest.finishWip.detail': '未コミット: {sample}', 'solo.suggest.finishWip.detailForeign': '未コミット(他のエージェントの作業を含む可能性): {sample}', - 'solo.suggest.finishWip.taskPrompt': '現在の worktree にある {count} 件の未コミット変更({files})を仕上げてください。plan → implement → test → review → land を一通り通します。', + 'solo.suggest.finishWip.taskPrompt': '現在の worktree にある {count} 件の未コミット変更({files})を仕上げてください。まず A/B/C 計画ゲートを通し、その後 実装 → テスト → レビュー → リリース に進みます。', 'solo.suggest.shipAhead.title': '{branch} をリリース({count} 件先行)', 'solo.suggest.shipAhead.detail': 'ローカルが上流より {count} 件先行しており、作業はすでに完了しています — レビュー + リリース。', diff --git a/desktop/src/i18n/locales/kr.ts b/desktop/src/i18n/locales/kr.ts index aa1ba14e..c60b32b7 100644 --- a/desktop/src/i18n/locales/kr.ts +++ b/desktop/src/i18n/locales/kr.ts @@ -1269,7 +1269,7 @@ export const kr: Record = { 'session.coordinatorChip': '오케스트레이션', 'session.coordinatorChipTooltip': '이 세션은 오케스트레이션 모드입니다. 작업은 전문 worker 에이전트로 분산되고 메인 스레드가 조정합니다. 컴포저의 + 메뉴에서 전환할 수 있습니다.', 'session.soloPipelineChip': '솔로 파이프라인', - 'session.soloPipelineChipTooltip': '이 세션은 솔로 파이프라인 모드입니다. AI가 5 단계 (계획 → 구현 → 테스트 → 리뷰 → 통합)를 단독으로 실행합니다. 컴포저의 + 메뉴에서 전환할 수 있습니다.', + 'session.soloPipelineChipTooltip': '이 세션은 솔로 파이프라인 모드입니다. 구현 전에 AI가 A/B/C 계획 게이트(Planner → Reviewer → Critic)를 먼저 실행한 뒤 구현 → 테스트 → 리뷰 → 통합으로 진행합니다. 컴포저의 + 메뉴에서 전환할 수 있습니다.', 'empty.recentActivity.handoff.branchLine': '지난 세션({branch}): "{title}".', 'empty.recentActivity.handoff.titleLine': '지난 세션: "{title}".', 'empty.recentActivity.handoff.filesLine': '수정한 파일: {files}{more}.', @@ -1283,7 +1283,7 @@ export const kr: Record = { 'solo.suggest.finishWip.title': '{count}개의 미커밋 변경 마무리', 'solo.suggest.finishWip.detail': '미커밋: {sample}', 'solo.suggest.finishWip.detailForeign': '미커밋 (다른 에이전트의 작업 포함 가능): {sample}', - 'solo.suggest.finishWip.taskPrompt': '현재 worktree의 {count}개 미커밋 변경({files})을 마무리해 주세요. plan → implement → test → review → land 한 사이클을 진행합니다.', + 'solo.suggest.finishWip.taskPrompt': '현재 worktree의 {count}개 미커밋 변경({files})을 마무리해 주세요. 먼저 A/B/C 계획 게이트를 거친 뒤 구현 → 테스트 → 리뷰 → 통합을 진행합니다.', 'solo.suggest.shipAhead.title': '{branch} 출시 ({count}개 커밋 앞섬)', 'solo.suggest.shipAhead.detail': '로컬이 업스트림보다 {count}개 앞서 있고 작업은 이미 완료됨 — 리뷰 + 출시.', diff --git a/desktop/src/i18n/locales/zh-TW.ts b/desktop/src/i18n/locales/zh-TW.ts index 7153690d..7e4d20bd 100644 --- a/desktop/src/i18n/locales/zh-TW.ts +++ b/desktop/src/i18n/locales/zh-TW.ts @@ -1269,7 +1269,7 @@ export const zh: Record = { 'session.coordinatorChip': '編排模式', 'session.coordinatorChipTooltip': '該會話已開啟編排模式。任務會扇出到專家 worker 代理執行,主執行緒負責協調。在輸入框 + 選單裡切換。', 'session.soloPipelineChip': '獨立流水線', - 'session.soloPipelineChipTooltip': '該會話已開啟獨立流水線模式。AI 會按 5 個階段(規劃 → 實作 → 測試 → 審查 → 落地)獨立執行。在輸入框 + 選單裡切換。', + 'session.soloPipelineChipTooltip': '該會話已開啟獨立流水線模式。實作前 AI 會先走 A/B/C 計畫閘門(Planner → Reviewer → Critic),再繼續實作 → 測試 → 審查 → 落地。在輸入框 + 選單裡切換。', 'empty.recentActivity.handoff.branchLine': '上次在 {branch} 上做了:「{title}」。', 'empty.recentActivity.handoff.titleLine': '上次會話:「{title}」。', 'empty.recentActivity.handoff.filesLine': '動了這些檔案:{files}{more}。', @@ -1283,7 +1283,7 @@ export const zh: Record = { 'solo.suggest.finishWip.title': '收尾 {count} 個未提交改動', 'solo.suggest.finishWip.detail': '未提交:{sample}', 'solo.suggest.finishWip.detailForeign': '未提交(可能含其他 agent 的改動):{sample}', - 'solo.suggest.finishWip.taskPrompt': '把當前 worktree 裡 {count} 個未提交改動({files})收尾。走完一輪 plan → implement → test → review → land。', + 'solo.suggest.finishWip.taskPrompt': '把當前 worktree 裡 {count} 個未提交改動({files})收尾。先走 A/B/C 計畫閘門,再實作 → 測試 → 審查 → 落地。', 'solo.suggest.shipAhead.title': '把 {branch} 落地({count} 個領先提交)', 'solo.suggest.shipAhead.detail': '本地領先上游 {count} 個提交,工作已經做完——評審 + 落地。', diff --git a/desktop/src/i18n/locales/zh.ts b/desktop/src/i18n/locales/zh.ts index d8ad4bf5..5c1bec60 100644 --- a/desktop/src/i18n/locales/zh.ts +++ b/desktop/src/i18n/locales/zh.ts @@ -1269,7 +1269,7 @@ export const zh: Record = { 'session.coordinatorChip': '编排模式', 'session.coordinatorChipTooltip': '该会话已开启编排模式。任务会扇出到专家 worker 代理执行,主线程负责协调。在输入框 + 菜单里切换。', 'session.soloPipelineChip': '独立流水线', - 'session.soloPipelineChipTooltip': '该会话已开启独立流水线模式。AI 会按 5 个阶段(规划 → 实现 → 测试 → 审查 → 落地)独立运行。在输入框 + 菜单里切换。', + 'session.soloPipelineChipTooltip': '该会话已开启独立流水线模式。实现前 AI 会先走 A/B/C 计划闸门(Planner → Reviewer → Critic),再继续实现 → 测试 → 审查 → 落地。在输入框 + 菜单里切换。', 'empty.recentActivity.handoff.branchLine': '上次在 {branch} 上做了:"{title}"。', 'empty.recentActivity.handoff.titleLine': '上次会话:"{title}"。', 'empty.recentActivity.handoff.filesLine': '动了这些文件:{files}{more}。', @@ -1283,7 +1283,7 @@ export const zh: Record = { 'solo.suggest.finishWip.title': '收尾 {count} 个未提交改动', 'solo.suggest.finishWip.detail': '未提交:{sample}', 'solo.suggest.finishWip.detailForeign': '未提交(可能含其他 agent 的改动):{sample}', - 'solo.suggest.finishWip.taskPrompt': '把当前 worktree 里 {count} 个未提交改动({files})收尾。走完一轮 plan → implement → test → review → land。', + 'solo.suggest.finishWip.taskPrompt': '把当前 worktree 里 {count} 个未提交改动({files})收尾。先走 A/B/C 计划闸门,再实现 → 测试 → 审查 → 落地。', 'solo.suggest.shipAhead.title': '把 {branch} 落地({count} 个领先提交)', 'solo.suggest.shipAhead.detail': '本地领先上游 {count} 个提交,工作已经做完——评审 + 落地。', diff --git a/src/coordinator/coordinatorMode.ts b/src/coordinator/coordinatorMode.ts index a3375ff9..09f66f88 100644 --- a/src/coordinator/coordinatorMode.ts +++ b/src/coordinator/coordinatorMode.ts @@ -213,6 +213,7 @@ When calling ${AGENT_TOOL_NAME}, always pass \`subagent_type\`. Two flavours: | \`commit-pr\` | Draft commit messages or PR descriptions from a real diff. | | \`Explore\` | Fast read-only codebase search. Specify thoroughness ("quick" / "medium" / "very thorough"). | | \`Plan\` | Read-only architect that returns an implementation plan and the critical files for it. | +| \`plan-critic\` | Read-only challenge pass for an implementation plan before code is written. Ends in \`PLAN_REVIEW: APPROVE\` / \`CHANGES_NEEDED\`. | | \`verification\` | Independent adversarial verification of completed work. Use after non-trivial implementation lands. | ${workerCapabilities} diff --git a/src/coordinator/soloPipelinePrompt.test.ts b/src/coordinator/soloPipelinePrompt.test.ts index 3d151b48..3422c48f 100644 --- a/src/coordinator/soloPipelinePrompt.test.ts +++ b/src/coordinator/soloPipelinePrompt.test.ts @@ -72,14 +72,27 @@ describe('getSoloPipelineSystemPrompt — invariants', () => { expect(prompt).toContain('Never run the pipeline for chat') }) - it('declares all five stages with the planned specialists', () => { - expect(prompt).toMatch(/1\.\s+\*\*PLAN\*\*/) + it('declares all five Solo pipeline stages', () => { + expect(prompt).toMatch(/1\.\s+\*\*PLAN GATE/) expect(prompt).toMatch(/2\.\s+\*\*IMPLEMENT\*\*/) expect(prompt).toMatch(/3\.\s+\*\*TEST\*\*/) expect(prompt).toMatch(/4\.\s+\*\*REVIEW\*\*/) expect(prompt).toMatch(/5\.\s+\*\*LAND\*\*/) }) + it('requires the prompt-level A/B/C Plan Gate before implementation', () => { + expect(prompt).toContain('A/B/C Plan Gate') + expect(prompt).toContain('Solo Council') + expect(prompt).toContain('A = Planner') + expect(prompt).toContain('B = Reviewer') + expect(prompt).toContain('C = Critic') + expect(prompt).toContain('final execution plan') + expect(prompt).toContain('use the `Plan` specialist when available') + expect(prompt).toContain('use the `plan-critic` specialist when available') + expect(prompt).toContain('otherwise run the same critique as a prompt-level role') + expect(prompt).toContain('Do not enter Stage 2') + }) + it('keeps the HUMAN GATE in Stage 4 (the safety contract)', () => { // Auto-merging review without explicit user approval would // defeat the whole point of Solo. Anyone who edits the prompt diff --git a/src/coordinator/soloPipelinePrompt.ts b/src/coordinator/soloPipelinePrompt.ts index 0df5ee35..33940678 100644 --- a/src/coordinator/soloPipelinePrompt.ts +++ b/src/coordinator/soloPipelinePrompt.ts @@ -4,8 +4,9 @@ * Sibling to `coordinatorMode.ts`. While coordinator mode does * dynamic fan-out (the lead AI decides what to delegate, when, and * to whom), Solo Pipeline mode runs a fixed five-stage script with - * gates between stages: plan → implement → test → review (HUMAN - * APPROVAL) → land. + * gates between stages: plan gate (A/B/C council) → implement → test + * → review (HUMAN APPROVAL) → land. Stage 1 is a prompt-level + * council pass, not a separate worker protocol. * * This module owns ONLY the prompt text + the boolean predicate * (`isSoloPipelineMode`). Wiring (CLI flag plumbing, @@ -86,10 +87,13 @@ const SOLO_PIPELINE_PROMPT = `# Solo Pipeline Mode You are operating in **Solo Pipeline mode**: a staged, gated workflow for turning one feature/change request into shipped, verified work. You drive -five stages in order, delegating each to a specialist subagent, and you -pause at human gates. You are the conductor — you do NOT write the code, -run the tests, or approve the work yourself; you route to the right -specialist and enforce the gate between stages. +five stages in order. Before any implementation, you must run an internal +**A/B/C Plan Gate — Solo Council**: A = Planner proposes, B = Reviewer +audits, C = Critic challenges, then you synthesize the final execution +plan. A/B/C are prompt-level roles, not separate workers or a +worker-to-worker protocol unless the runtime explicitly provides one. You +are the conductor — you do NOT write the code, run the tests, or approve +the work yourself; you enforce the gate between stages. ## STAGE 0 — Intent triage (ALWAYS run this first, silently) @@ -117,26 +121,42 @@ only justified for real build work. Open with a one-line, scannable plan, e.g.: - "Solo pipeline: ① plan → ② implement → ③ test → ④ review (your - approval) → ⑤ land. Starting Stage 1: planning." + "Solo pipeline: ① plan gate (A/B/C council) → ② implement → ③ test → + ④ review (your approval) → ⑤ land. Starting Stage 1: A/B/C planning." Keep the user oriented: at each stage transition, print a single status line "▸ Stage N/5: ". ## The five stages -1. **PLAN** (specialist: planner / context-gatherer) - - Inspect the real code first. Produce a short written plan: the - changed surface, the approach, the files to touch, the success - criteria, and the verification commands. - - Hand-off artifact: \`plan\` (the written plan). - - GATE → automatic. Advance when the plan names concrete files + - a verification command. If it can't, loop back with the user. +1. **PLAN GATE — Solo Council** (A/B/C; no implementation yet) + - Inspect the real code first. Read enough to ground the plan in actual + files, commands, constraints, and existing patterns. + - **A = Planner**: use the \`Plan\` specialist when available. Produce the + concrete implementation plan — changed surface, candidate files, + approach, acceptance criteria, verification commands, and known risks. + - **B = Reviewer**: audit A's plan for missing files, integration points, + test gaps, safety issues, persistence/security boundaries, and repository + conventions. + - **C = Critic**: use the \`plan-critic\` specialist when available; + otherwise run the same critique as a prompt-level role. Challenge + assumptions, look for a smaller / safer / more verifiable alternative, + identify overreach, and name what would make the plan fail. The Critic + must not merely agree. + - **Synthesis**: merge A/B/C into one **final execution plan**. Treat that + final execution plan as the source of truth for Stage 2. + - Hand-off artifact: \`final execution plan\` with concrete files, + implementation steps, success criteria, verification commands, and + residual risks. + - GATE → automatic but strict. Do not enter Stage 2 or modify files until + the final execution plan is complete. If B reports a blocking issue or C + reports a blocking objection, revise the plan or ask the user one short + clarifying question before implementation. 2. **IMPLEMENT** (specialist: implementer) - - Receives the plan verbatim. Makes the narrowest change that - satisfies it. Touches only files the plan named (or explains any - deviation). + - Receives the final execution plan verbatim. Makes the narrowest change + that satisfies it. Touches only files the final execution plan named (or + explains any deviation before proceeding). - Hand-off artifact: \`diff\` (files changed + summary). 3. **TEST** (specialist: verification) @@ -179,10 +199,15 @@ suggestion that prefilled the prompt), respect the entry stage: ## Rules +- The A/B/C Plan Gate is mandatory for TASK messages that enter at + \`entryStage = 'plan'\` or have no entry-stage shortcut. Planning may read + code, but it must not implement or modify files. - One stage at a time. Never skip the human gate at Stage 4 unless the entry-stage shortcut placed you past it. - If a stage's specialist reports it can't proceed, surface it to the user rather than forcing the next stage. +- The Critic must look for a smaller or safer alternative; the final + execution plan is the source of truth for implementation. - If the user interrupts mid-pipeline with a new message, re-run Stage 0 intent triage on it: a chat aside shouldn't derail the pipeline, a new task should ask whether to queue or restart. diff --git a/src/coordinator/workerAgent.test.ts b/src/coordinator/workerAgent.test.ts index f7ecbdab..bebedaf5 100644 --- a/src/coordinator/workerAgent.test.ts +++ b/src/coordinator/workerAgent.test.ts @@ -23,6 +23,7 @@ describe('coordinator agent registry', () => { 'commit-pr', 'Explore', 'Plan', + 'plan-critic', 'verification', ] for (const expected of required) { diff --git a/src/coordinator/workerAgent.ts b/src/coordinator/workerAgent.ts index 6c6b3166..7f7722e3 100644 --- a/src/coordinator/workerAgent.ts +++ b/src/coordinator/workerAgent.ts @@ -32,6 +32,7 @@ import { EXPLORE_AGENT } from '../tools/AgentTool/built-in/exploreAgent.js' import { MIGRATION_AGENT } from '../tools/AgentTool/built-in/migrationAgent.js' import { PERFORMANCE_AGENT } from '../tools/AgentTool/built-in/performanceAgent.js' import { PLAN_AGENT } from '../tools/AgentTool/built-in/planAgent.js' +import { PLAN_CRITIC_AGENT } from '../tools/AgentTool/built-in/planCriticAgent.js' import { REFACTOR_AGENT } from '../tools/AgentTool/built-in/refactorAgent.js' import { SECURITY_REVIEWER_AGENT } from '../tools/AgentTool/built-in/securityReviewerAgent.js' import { TEST_AUTHOR_AGENT } from '../tools/AgentTool/built-in/testAuthorAgent.js' @@ -95,6 +96,7 @@ export function getCoordinatorAgents(): AgentDefinition[] { // research / verify EXPLORE_AGENT, PLAN_AGENT, + PLAN_CRITIC_AGENT, VERIFICATION_AGENT, ] } diff --git a/src/server/__tests__/conversations.test.ts b/src/server/__tests__/conversations.test.ts index 6e9dc833..1b59467e 100644 --- a/src/server/__tests__/conversations.test.ts +++ b/src/server/__tests__/conversations.test.ts @@ -265,6 +265,9 @@ describe('ConversationService', () => { // The injected text must be the Solo prompt — sniff for the unique // Stage-0 intent-triage phrasing locked in soloPipelinePrompt.test.ts. expect(args[idx + 1]).toContain('Solo Pipeline mode') + expect(args[idx + 1]).toContain('A/B/C Plan Gate') + expect(args[idx + 1]).toContain('Critic') + expect(args[idx + 1]).toContain('final execution plan') }) it('should not append the Solo prompt when soloPipelineMode is off', () => { @@ -287,7 +290,7 @@ describe('ConversationService', () => { expect(flagIndices.length).toBe(2) const texts = flagIndices.map((i) => args[i + 1] ?? '') expect(texts.some((t) => t.includes(ORCHESTRATION_PROMPT_MARKER))).toBe(true) - expect(texts.some((t) => t.includes('Solo Pipeline mode'))).toBe(true) + expect(texts.some((t) => t.includes('A/B/C Plan Gate'))).toBe(true) }) // Locks the orchestrator's "propagate project tool rules into every dispatched diff --git a/src/tools/AgentTool/built-in/planCriticAgent.ts b/src/tools/AgentTool/built-in/planCriticAgent.ts new file mode 100644 index 00000000..4d5a1988 --- /dev/null +++ b/src/tools/AgentTool/built-in/planCriticAgent.ts @@ -0,0 +1,92 @@ +import { BASH_TOOL_NAME } from 'src/tools/BashTool/toolName.js' +import { EXIT_PLAN_MODE_TOOL_NAME } from 'src/tools/ExitPlanModeTool/constants.js' +import { FILE_EDIT_TOOL_NAME } from 'src/tools/FileEditTool/constants.js' +import { FILE_WRITE_TOOL_NAME } from 'src/tools/FileWriteTool/prompt.js' +import { NOTEBOOK_EDIT_TOOL_NAME } from 'src/tools/NotebookEditTool/constants.js' +import { AGENT_TOOL_NAME } from '../constants.js' +import type { BuiltInAgentDefinition } from '../loadAgentsDir.js' + +const PLAN_CRITIC_SYSTEM_PROMPT = `You are a plan critic specialist for Claude Code. Your job is to challenge an implementation plan before any code is written. You are not a code reviewer: code reviewers inspect diffs after implementation; you inspect the proposed plan for feasibility, scope, risk, and whether a smaller or safer path exists. + +=== CRITICAL: READ-ONLY MODE - NO FILE MODIFICATIONS === +This is a READ-ONLY plan critique task. You are STRICTLY PROHIBITED from: +- Creating, modifying, or deleting any files (no Write, Edit, touch, rm, mv, cp) +- Using redirect operators (>, >>) or heredocs to write files +- Running git write operations (add, commit, push) or installing dependencies +- Running ANY command that changes system state +You do NOT have file-editing tools. Use ${BASH_TOOL_NAME} only for read-only inspection (git diff, git log, git status, ls, cat, head, tail, find, grep). Your output is a critique report, not edits — the parent agent revises or applies the plan. + +=== WHAT YOU RECEIVE === +You receive the user's requested outcome, a proposed implementation plan, and optionally relevant files or constraints. If important context is missing, inspect the codebase read-only to verify whether the plan is grounded in real files and existing patterns. + +=== CRITIQUE DIMENSIONS === +Challenge the plan in this order: + +**1. Feasibility** +- Does the plan name the right files, APIs, data flow, and integration points? +- Does it depend on behavior that does not exist or contradict current code? +- Are sequencing and dependencies realistic? + +**2. Minimality and alternatives** +- Is there a smaller change that satisfies the same user outcome? +- Is the plan over-engineered, speculative, or adding abstractions before they are needed? +- Can a safer staged rollout or narrower scope reduce risk? + +**3. Risk and contracts** +- Does it touch auth, permissions, persistence, networking, shell execution, external providers, or public APIs? +- Does it require migration, compatibility, or user-visible copy updates? +- Are there hidden cross-process or cross-package effects? + +**4. Verification gaps** +- Are success criteria concrete and testable? +- Are the proposed checks the narrowest meaningful checks? +- Is there an E2E/manual smoke path when unit tests cannot prove the behavior? + +=== OUTPUT FORMAT (REQUIRED) === +Return a concise report with these sections: + +1. Blocking objections +- List only issues that should prevent implementation until the plan changes. +- Use "None" if there are no blocking objections. + +2. Non-blocking concerns +- List risks or improvements that can be handled during implementation. +- Use "None" if there are no non-blocking concerns. + +3. Smaller / safer alternative +- Name the best smaller or safer path if one exists. +- If the proposed plan is already minimal, say so explicitly. + +4. Verification notes +- State which checks or smoke paths should prove the final implementation. + +End with exactly one summary line the caller can parse: + +PLAN_REVIEW: APPROVE +or +PLAN_REVIEW: CHANGES_NEEDED + +Use CHANGES_NEEDED if any blocking objection remains. Use APPROVE only when the plan is feasible enough to implement.` + +const PLAN_CRITIC_WHEN_TO_USE = + 'Use this agent to critique an implementation plan before code is written. It is read-only and challenges feasibility, scope, risks, verification gaps, and whether a smaller or safer plan exists. Ends with PLAN_REVIEW: APPROVE / CHANGES_NEEDED.' + +export const PLAN_CRITIC_AGENT: BuiltInAgentDefinition = { + agentType: 'plan-critic', + whenToUse: PLAN_CRITIC_WHEN_TO_USE, + color: 'yellow', + disallowedTools: [ + AGENT_TOOL_NAME, + EXIT_PLAN_MODE_TOOL_NAME, + FILE_EDIT_TOOL_NAME, + FILE_WRITE_TOOL_NAME, + NOTEBOOK_EDIT_TOOL_NAME, + ], + source: 'built-in', + baseDir: 'built-in', + model: 'inherit', + omitClaudeMd: true, + getSystemPrompt: () => PLAN_CRITIC_SYSTEM_PROMPT, + criticalSystemReminder_EXPERIMENTAL: + 'CRITICAL: This is a READ-ONLY plan critique task. You CANNOT edit, write, or create files. You MUST end with PLAN_REVIEW: APPROVE or PLAN_REVIEW: CHANGES_NEEDED.', +} diff --git a/src/tools/AgentTool/builtInAgents.test.ts b/src/tools/AgentTool/builtInAgents.test.ts index 899e9c9f..289230c8 100644 --- a/src/tools/AgentTool/builtInAgents.test.ts +++ b/src/tools/AgentTool/builtInAgents.test.ts @@ -6,6 +6,7 @@ import { areExplorePlanAgentsEnabled, getBuiltInAgents, } from './builtInAgents.js' +import { PLAN_CRITIC_AGENT } from './built-in/planCriticAgent.js' const originalDisableBuiltIns = process.env.CLAUDE_AGENT_SDK_DISABLE_BUILTIN_AGENTS @@ -48,6 +49,7 @@ describe('built-in agents', () => { expect(agentTypes).toContain('docs-writer') expect(agentTypes).toContain('performance') expect(agentTypes).toContain('commit-pr') + expect(agentTypes).toContain('plan-critic') // game-developer is a project-level agent (.claude/agents), not a built-in. expect(agentTypes).not.toContain('game-developer') }) @@ -68,6 +70,25 @@ describe('built-in agents', () => { expect(agentTypes).toContain('docs-writer') expect(agentTypes).toContain('performance') expect(agentTypes).toContain('commit-pr') + expect(agentTypes).toContain('plan-critic') + }) + + test('plan-critic is read-only and returns a parseable verdict', () => { + expect(PLAN_CRITIC_AGENT.agentType).toBe('plan-critic') + expect(PLAN_CRITIC_AGENT.disallowedTools).toEqual(expect.arrayContaining([ + 'Agent', + 'ExitPlanMode', + 'Edit', + 'Write', + 'NotebookEdit', + ])) + + const prompt = PLAN_CRITIC_AGENT.getSystemPrompt({} as never) + expect(prompt).toContain('READ-ONLY plan critique task') + expect(prompt).toContain('PLAN_REVIEW: APPROVE') + expect(prompt).toContain('PLAN_REVIEW: CHANGES_NEEDED') + expect(prompt).toContain('smaller or safer path') + expect(PLAN_CRITIC_AGENT.criticalSystemReminder_EXPERIMENTAL).toContain('PLAN_REVIEW: APPROVE') }) test('preserves SDK opt-out in noninteractive sessions', () => { diff --git a/src/tools/AgentTool/builtInAgents.ts b/src/tools/AgentTool/builtInAgents.ts index 6fce33e3..2b6a1358 100644 --- a/src/tools/AgentTool/builtInAgents.ts +++ b/src/tools/AgentTool/builtInAgents.ts @@ -12,6 +12,7 @@ import { GENERAL_PURPOSE_AGENT } from './built-in/generalPurposeAgent.js' import { MIGRATION_AGENT } from './built-in/migrationAgent.js' import { PERFORMANCE_AGENT } from './built-in/performanceAgent.js' import { PLAN_AGENT } from './built-in/planAgent.js' +import { PLAN_CRITIC_AGENT } from './built-in/planCriticAgent.js' import { REFACTOR_AGENT } from './built-in/refactorAgent.js' import { SECURITY_REVIEWER_AGENT } from './built-in/securityReviewerAgent.js' import { STATUSLINE_SETUP_AGENT } from './built-in/statuslineSetup.js' @@ -76,6 +77,7 @@ export function getBuiltInAgents(): AgentDefinition[] { DOCS_WRITER_AGENT, PERFORMANCE_AGENT, COMMIT_PR_AGENT, + PLAN_CRITIC_AGENT, ] if (areExplorePlanAgentsEnabled()) {