diff --git a/conductor/directives/clarify_unclear_review_before_partial_impl/meta.md b/conductor/directives/clarify_unclear_review_before_partial_impl/meta.md new file mode 100644 index 00000000..10d38666 --- /dev/null +++ b/conductor/directives/clarify_unclear_review_before_partial_impl/meta.md @@ -0,0 +1,8 @@ +# clarify_unclear_review_before_partial_impl + +## v1 + +**Why this iteration:** Lifted from the global OpenCode superpowers plugin (obra/superpowers) — `skills/receiving-code-review/SKILL.md`. The directive encodes the "Handling Unclear Feedback" block at lines 42-58 (STOP and ASK if any item is unclear; partial understanding = wrong implementation). Counterweight to the LLM default of implementing clear items "in parallel" while asking about unclear ones later — which creates partial working-tree commits that may need reverting when the unclear items are clarified. +**Source:** superpowers plugin `skills/receiving-code-review/SKILL.md:42-58` (Handling Unclear Feedback) + +**Lifted:** 2026-07-04 (scavenge sweep: superpowers plugin directives) diff --git a/conductor/directives/clarify_unclear_review_before_partial_impl/v1.md b/conductor/directives/clarify_unclear_review_before_partial_impl/v1.md new file mode 100644 index 00000000..3ff69c50 --- /dev/null +++ b/conductor/directives/clarify_unclear_review_before_partial_impl/v1.md @@ -0,0 +1,39 @@ +# When feedback has unclear items, clarify ALL of them BEFORE implementing anything — never implement partial feedback + +## The rule + +When the agent receives multi-item feedback (e.g., "fix items 1-6") and any item is unclear, the agent pauses and asks for clarification on the unclear items BEFORE implementing any of them. The agent does NOT implement the clear items in parallel and ask about the unclear items "in parallel" or "later." Items may be related; partial understanding → wrong implementation of the clear items. + +Per `skills/receiving-code-review/SKILL.md:42-58` (Handling Unclear Feedback): + +> ``` +> IF any item is unclear: +> STOP - do not implement anything yet +> ASK for clarification on unclear items +> +> WHY: Items may be related. Partial understanding = wrong implementation. +> ``` +> +> **Example:** +> ``` +> your human partner: "Fix 1-6" +> You understand 1,2,3,6. Unclear on 4,5. +> +> ❌ WRONG: Implement 1,2,3,6 now, ask about 4,5 later +> ✅ RIGHT: "I understand items 1,2,3,6. Need clarification on 4 and 5 before proceeding." +> ``` + +## Why + +Items in a code-review or a fix request are often related. The unclear items may be the most important (the reviewer flagged them but couldn't articulate why cleanly), or they may have dependencies on the clear items (changing the order changes what the clear items mean). Implementing the clear items while waiting creates a partial state: commits in the working tree that may need to be reverted when the unclear items are clarified. + +The "ask later" pattern is also a politeness-induced corruption — the agent wants to start work, the user is waiting, the agent feels rude pausing. The polite path is to ask; the productive path is to ask; both are the same path. + +## What this means in practice + +- Multi-item feedback is parsed first: "I understand X, unclear on Y." +- The clarification message is sent BEFORE any implementation begins. +- The clarification is specific: "Item 4 — do you mean [interpretation A] or [interpretation B]?" not "Could you clarify?" +- The agent does NOT implement the clear items in the meantime. The work-tree stays clean until all items are clarified. +- A single combined clarification message is preferred over multiple sequential ones (one round-trip, not three). +- If the user replies "interpret as A," the agent implements ALL items in the order the user specified (with the user-confirmed interpretation), then verifies per `verify_critique_before_implementing`. diff --git a/conductor/directives/no_performative_agreement_in_review/meta.md b/conductor/directives/no_performative_agreement_in_review/meta.md new file mode 100644 index 00000000..55b8ac0b --- /dev/null +++ b/conductor/directives/no_performative_agreement_in_review/meta.md @@ -0,0 +1,8 @@ +# no_performative_agreement_in_review + +## v1 + +**Why this iteration:** Lifted from the global OpenCode superpowers plugin (obra/superpowers) — `skills/receiving-code-review/SKILL.md`. The directive encodes the Acknowledging Correct Feedback block at lines 130-148 (NEVER "Thanks"/"Great point"/"You're absolutely right!") plus the explicit CLAUDE.md violation referenced at line 30. Counterweight to the LLM's idiomatic sycophancy patterns that waste user reading time and obscure whether the fix actually addressed the issue. +**Source:** superpowers plugin `skills/receiving-code-review/SKILL.md:130-148` (Acknowledging Correct Feedback) + +**Lifted:** 2026-07-04 (scavenge sweep: superpowers plugin directives) diff --git a/conductor/directives/no_performative_agreement_in_review/v1.md b/conductor/directives/no_performative_agreement_in_review/v1.md new file mode 100644 index 00000000..309af908 --- /dev/null +++ b/conductor/directives/no_performative_agreement_in_review/v1.md @@ -0,0 +1,48 @@ +# Code-review acknowledgments must NOT contain gratitude or performative agreement — state the fix or stay silent + +## The rule + +When responding to code-review feedback, the agent does NOT use: + +- "Thanks" / "Thanks for catching that" / "Thanks for..." +- "Great point!" / "Excellent feedback!" +- "You're absolutely right!" +- Any "I appreciate" / "That's helpful" + +Per `skills/receiving-code-review/SKILL.md:130-148` (Acknowledging Correct Feedback): + +> When feedback IS correct: +> ``` +> ✅ "Fixed. [Brief description of what changed]" +> ✅ "Good catch - [specific issue]. Fixed in [location]." +> ✅ [Just fix it and show in the code] +> +> ❌ "You're absolutely right!" +> ❌ "Great point!" +> ❌ "Thanks for catching that!" +> ❌ "Thanks for [anything]" +> ❌ ANY gratitude expression +> ``` +> +> **Why no thanks:** Actions speak. Just fix it. The code itself shows you heard the feedback. +> +> **If you catch yourself about to write "Thanks":** DELETE IT. State the fix instead. + +And the CLAUDE.md violation at line 30: "**NEVER:** 'You're absolutely right!' (explicit CLAUDE.md violation)". + +## Why + +Gratitude in this context is performance, not communication. The reviewer does not need thanks; they need their feedback acknowledged by an actual code change. "Thanks for catching that!" followed by half a fix obscures whether the fix actually addressed the issue. The user's reading time is wasted on pleasantries that say nothing about the work. + +The rule also preempts the LLM's idiomatic sycophancy (training-data politeness patterns). The agent default is to agree, thank, and partially comply. The directive forces the agent to commit to the fix or to nothing — gratitude is forbidden silence. + +## What this means in practice + +- After receiving correct review feedback, the agent's response is one of three: + 1. **`[Just fix it]`** — apply the change, show the diff in the next tool output, no acknowledgment prose. + 2. **"Fixed. [one-sentence description of what changed]."** — terse, factual. + 3. **"Good catch — [issue]. Fixed in [file:line]."** — terse, includes the location for auditability. +- The agent never starts a reply with "Thanks" or "You're absolutely right." +- The agent never apologizes performatively ("Sorry, I should have caught that"). +- If the agent catches itself writing "Thanks" — DELETE IT and rewrite as the fix statement. +- This applies to all reviews: user reviews, GitHub PR reviews, Tier 4 QA reports, internal team reviews. diff --git a/conductor/directives/presets/current_baseline.md b/conductor/directives/presets/current_baseline.md index 5a176b76..65d48290 100644 --- a/conductor/directives/presets/current_baseline.md +++ b/conductor/directives/presets/current_baseline.md @@ -21,6 +21,7 @@ Read each file below before any action. - ban_dict_get_on_known_fields: conductor/directives/ban_dict_get_on_known_fields/v1.md - ban_getattr_dispatch: conductor/directives/ban_getattr_dispatch/v1.md - ban_hasattr_dispatch: conductor/directives/ban_hasattr_dispatch/v1.md +- clarify_unclear_review_before_partial_impl: conductor/directives/clarify_unclear_review_before_partial_impl/v1.md - ban_local_imports: conductor/directives/ban_local_imports/v1.md - ban_optional_returns: conductor/directives/ban_optional_returns/v1.md - ban_prefix_aliasing: conductor/directives/ban_prefix_aliasing/v1.md @@ -42,6 +43,7 @@ Read each file below before any action. - decorator_orphan_pitfall: conductor/directives/decorator_orphan_pitfall/v1.md - delete_means_delete_no_reference: conductor/directives/delete_means_delete_no_reference/v1.md - defer_heavy_sdk_imports_to_subprocess: conductor/directives/defer_heavy_sdk_imports_to_subprocess/v1.md +- no_performative_agreement_in_review: conductor/directives/no_performative_agreement_in_review/v1.md - defer_not_catch_for_native_crashes: conductor/directives/defer_not_catch_for_native_crashes/v1.md - deduction_loop_limit: conductor/directives/deduction_loop_limit/v1.md - docs_philosophy_then_boundaries_then_logic_then_verify: conductor/directives/docs_philosophy_then_boundaries_then_logic_then_verify/v1.md @@ -117,6 +119,7 @@ Read each file below before any action. - quarantine_flag_the_engine_not_shared_types: conductor/directives/quarantine_flag_the_engine_not_shared_types/v1.md - rag_six_rules: conductor/directives/rag_six_rules/v1.md - report_instead_of_fix_ban: conductor/directives/report_instead_of_fix_ban/v1.md +- review_after_each_task_not_end: conductor/directives/review_after_each_task_not_end/v1.md - reset_session_preserves_project_path: conductor/directives/reset_session_preserves_project_path/v1.md - result_error_pattern: conductor/directives/result_error_pattern/v1.md - run_full_tier_after_phase_refactor: conductor/directives/run_full_tier_after_phase_refactor/v1.md @@ -165,6 +168,7 @@ Read each file below before any action. - view_composes_does_not_leak_into_theme_get_color: conductor/directives/view_composes_does_not_leak_into_theme_get_color/v1.md - verify_before_editing: conductor/directives/verify_before_editing/v1.md - verbose_commit_message_ban: conductor/directives/verbose_commit_message_ban/v1.md +- verify_critique_before_implementing: conductor/directives/verify_critique_before_implementing/v1.md - warm_md_duplicates_not_in_place: conductor/directives/warm_md_duplicates_not_in_place/v1.md - workspace_paths: conductor/directives/workspace_paths/v1.md @@ -205,7 +209,11 @@ debugging) added 6 more (3 from test-driven-development — test-must-fail-for- believed-reason + delete-means-delete-no-reference + test-passing-immediately- proves-nothing; 3 from systematic-debugging — three-fix-failures-question- architecture + single-hypothesis-minimal-test + reproduction-before-fix) for a -total of 148 — see HARVEST_SUMMARY.md for what was lifted in each pass). +total of 148, and the 2026-07-04 superpowers scavenge (batch 3 of 5: review +cluster) added 4 more (3 from receiving-code-review — no-performative-agreement ++ verify-critique-before-implementing + clarify-unclear-before-partial-impl; +1 from requesting-code-review — review-after-each-task-not-end) for a total of +152 — see HARVEST_SUMMARY.md for what was lifted in each pass). No alternative encodings tested yet. This preset is the control group for future experiments. diff --git a/conductor/directives/review_after_each_task_not_end/meta.md b/conductor/directives/review_after_each_task_not_end/meta.md new file mode 100644 index 00000000..89500838 --- /dev/null +++ b/conductor/directives/review_after_each_task_not_end/meta.md @@ -0,0 +1,8 @@ +# review_after_each_task_not_end + +## v1 + +**Why this iteration:** Lifted from the global OpenCode superpowers plugin (obra/superpowers) — `skills/requesting-code-review/SKILL.md`. The directive encodes the "When to Request Review" block at lines 14-22 (Mandatory: After each task in subagent-driven development). Counterweight to the LLM default of batching reviews until the end of an implementation, when issues have compounded across tasks and a reviewer flag triggers a costly walk-back. +**Source:** superpowers plugin `skills/requesting-code-review/SKILL.md:14-22` (When to Request Review) + `:92-95` (Red Flags) + +**Lifted:** 2026-07-04 (scavenge sweep: superpowers plugin directives) diff --git a/conductor/directives/review_after_each_task_not_end/v1.md b/conductor/directives/review_after_each_task_not_end/v1.md new file mode 100644 index 00000000..92a218bc --- /dev/null +++ b/conductor/directives/review_after_each_task_not_end/v1.md @@ -0,0 +1,33 @@ +# Code review is requested AFTER each task, not at the end — issues caught later cost more to fix + +## The rule + +In subagent-driven development and executing-plans flows, the agent requests code review after EACH completed task, not at the end of the implementation. Catching a critical issue after 4 tasks of compounded work is more expensive than catching it after task 1 by an order of magnitude. + +Per `skills/requesting-code-review/SKILL.md:14-22` (When to Request Review): + +> **Mandatory:** +> - After each task in subagent-driven development +> - After completing major feature +> - Before merge to main +> +> **Optional but valuable:** +> - When stuck (fresh perspective) +> - Before refactoring (baseline check) +> - After fixing complex bug + +## Why + +Issues compound across tasks. A wrong architecture introduced in task 1 is reproduced (unconsciously) in tasks 2-5 because each subsequent task fits the assumed architecture. By the time a reviewer sees the whole diff, the architecture is welded in place; the reviewer-flagged rewrites have to walk back 5 tasks of compounding. + +A review at the end is also a "summit" — the agent has lost the design rationale and now must defend the result rather than design correctly. Early reviews are design conversations; late reviews are debates about an entrenched choice. + +## What this means in practice + +- After each task commits, the agent dispatches the code reviewer (via the standard requesting-code-review template). +- The reviewer's verdict (Critical / Important / Minor / Approved) is acted on BEFORE the next task starts. +- If Critical: block on Critical, fix, re-review the fix. +- If Important: fix before next task. +- If Minor: log for later; proceed. +- A review skipped because "this task was simple" is a red flag (per `skills/requesting-code-review/SKILL.md:92-95` Red Flags: "Never: Skip review because 'it's simple'"). +- The final review at the end is in addition to per-task reviews, not a replacement. diff --git a/conductor/directives/verify_critique_before_implementing/meta.md b/conductor/directives/verify_critique_before_implementing/meta.md new file mode 100644 index 00000000..c6fc6cb9 --- /dev/null +++ b/conductor/directives/verify_critique_before_implementing/meta.md @@ -0,0 +1,8 @@ +# verify_critique_before_implementing + +## v1 + +**Why this iteration:** Lifted from the global OpenCode superpowers plugin (obra/superpowers) — `skills/receiving-code-review/SKILL.md`. The directive encodes the "From External Reviewers" block at lines 67-86 (5-point verification before implementing). Counterweight to the LLM default of blind compliance with reviewer suggestions, even when they may be wrong or may conflict with the user's prior decisions. +**Source:** superpowers plugin `skills/receiving-code-review/SKILL.md:67-86` (From External Reviewers) + +**Lifted:** 2026-07-04 (scavenge sweep: superpowers plugin directives) diff --git a/conductor/directives/verify_critique_before_implementing/v1.md b/conductor/directives/verify_critique_before_implementing/v1.md new file mode 100644 index 00000000..432cdba2 --- /dev/null +++ b/conductor/directives/verify_critique_before_implementing/v1.md @@ -0,0 +1,41 @@ +# Verify review feedback is technically correct BEFORE implementing — reviewer suggestions are evaluations, not orders + +## The rule + +When the agent receives code-review feedback (from the user, a teammate, or an external reviewer), the agent verifies the suggestion against the codebase BEFORE implementing. The agent does not assume the reviewer is correct; the agent does not assume the reviewer is wrong; the agent checks and decides. + +Per `skills/receiving-code-review/SKILL.md:67-86` (Source-Specific Handling: From External Reviewers): + +> ``` +> BEFORE implementing: +> 1. Check: Technically correct for THIS codebase? +> 2. Check: Breaks existing functionality? +> 3. Check: Reason for current implementation? +> 4. Check: Works on all platforms/versions? +> 5. Check: Does reviewer understand full context? +> +> IF suggestion seems wrong: +> Push back with technical reasoning +> +> IF can't easily verify: +> Say so: "I can't verify this without [X]. Should I [investigate/ask/proceed]?" +> +> IF conflicts with your human partner's prior decisions: +> Stop and discuss with your human partner first +> ``` + +## Why + +Reviewers have less context than the implementer. They see the diff or the discussion; they do not see the broader code, the design constraints, the prior decisions. Their suggestions are hypotheses, not commands. Implementing without verification is "blind compliance," which is its own failure mode — the agent ships a worse code path because the reviewer may have been wrong. + +The "principle of least astonishment" applies both ways: the user is surprised when the agent silently implements a wrong suggestion (trust loss) and is surprised when the agent silently overrides a correct one (because the trust was based on the agent's judgment). The transparent path is: verify, then either implement or push back with reasoning. + +## What this means in practice + +- The agent reads the suggestion, identifies what it claims is wrong/missing/improvable. +- The agent looks at the actual code at the location in question. +- The agent considers the 5 checks (correct for this codebase / breaks nothing / reason for current impl / platform coverage / reviewer context). +- The agent decides: IMPLEMENT (with technical note), PUSH BACK (with technical reasoning), or DEFER (need more info from user). +- "Defer" is the default for the "can't easily verify" case — the agent names what's needed and asks. +- The agent NEVER writes performative agreement ("You're absolutely right!") as a substitute for verification (see `no_performative_agreement_in_review`). +- A reviewer suggestion that conflicts with the user's prior decisions is escalated to the user, not silently implemented or silently overridden.