feat(prompts): weave Pike's rules into develop + research prompts

src/agents/codex-session-discovery.js

@@ -13,7 +13,7 @@ const RETRY_DELAY_MS = 200;
  *
  * @param {string} workspaceDir - Absolute path to workspace (must match cwd in session file)
  * @param {number} runStartTimeMs - Timestamp (Date.now()) recorded before execute(); only accept files with mtime >= runStartTimeMs (strict; no slack)
- * @returns {string|null} - sessionId or null if not found
+ * @returns {Promise<string|null>} - sessionId or null if not found
  */
 export async function discoverCodexSessionId(workspaceDir, runStartTimeMs) {
   const codexHome = process.env.CODEX_HOME || path.join(os.homedir(), ".codex");

src/machines/develop/implementation.machine.js

@@ -130,91 +130,73 @@ Build upon existing correct work. Do not duplicate or revert it.
     const implPrompt = `${recoveryContext}Read ${paths.plan} and ${paths.critique}.
 
 ## Step 1: Address Critique
-Update ${paths.plan} to address any Critical Issues or Over-Engineering Concerns from the critique.
-If critique says REJECT, revise the plan significantly before proceeding.
+Update ${paths.plan} to resolve every finding in the critique —
+Critical Issues, Over-Engineering Concerns, Data Structure Review,
+Concerns, and Questions. For each Question: answer it only when the
+repo or ISSUE.md gives you an explicit, verifiable answer. When you
+can't:
+- **Blocking question** (affects required behavior, acceptance
+  criteria, or API/data-shape the implementation must match): STOP.
+  Record the question under an \`## Open Questions (BLOCKING)\`
+  section in ${paths.plan} and do NOT produce an implementation diff.
+  Let the workflow surface the blocker.
+- **Non-blocking question** (style, naming, minor polish): record
+  the question and the working assumption under \`## Open Questions\`
+  in ${paths.plan}, then proceed. Make the assumption obvious so
+  reviewers can catch it.
+
+If the critique says REJECT, revise significantly before proceeding.
+
+## Step 2: Tests First
 ${
   useRedGreen
-    ? `
-## Step 2: RED — Write Failing Tests First
-This is a difficulty ${difficulty} issue. Use Red/Green TDD.
-
-Before writing ANY implementation code:
-1. Read the Testing Strategy from ${paths.issue} and the Testing section from ${paths.plan}
-2. Write test files/cases that capture the expected behavior described in the issue
-   - Each test should target one specific requirement
-   - Use the repo's existing test framework and conventions
-3. **Run the test suite and confirm the new tests FAIL**
-   - Verify they fail for the RIGHT reasons: missing functions, unimplemented behavior, wrong return values
-   - NOT for syntax errors, import failures in the test itself, or broken test setup
-   - If a test passes before implementation, it is not testing new behavior — rewrite it
-4. Do NOT proceed to Step 3 until you have confirmed RED (failing tests)
-
-## Step 3: GREEN — Implement to Pass Tests
-Implement the feature following the plan. Your goal: make every failing test from Step 2 pass.
-- Work incrementally — implement one piece, run tests, see progress
-- Do NOT weaken assertions, skip tests, or reduce coverage to get green
-- Do NOT modify the tests you wrote in Step 2 to make them pass (fix the implementation, not the tests)
-- When all tests pass, you are done with this step`
-    : `
-## Step 2: Write Tests
-Before writing implementation code:
-1. Read the Testing Strategy from ${paths.issue} and the Testing section from ${paths.plan}
-2. Write tests that capture the expected behavior described in the issue
-3. Run the test suite to confirm the new tests fail for the right reasons (missing implementation, not broken tests)
-4. Only then proceed to Step 3
-
-## Step 3: Implement
-Implement the feature following the plan. Make the failing tests pass without shortcuts — do not weaken assertions, skip tests, or reduce coverage to get green.`
+    ? `Difficulty ${difficulty} — use Red/Green TDD.
+
+Write tests from the Testing Strategy in ${paths.issue} and
+${paths.plan}, using the repo's existing framework and conventions.
+Each test targets one specific requirement. Run them and confirm
+they FAIL for the RIGHT reasons (missing functions, unimplemented
+behavior, wrong return values — NOT syntax errors or broken setup).
+If a test passes before implementation, rewrite it. Do not proceed
+to Step 3 until RED is confirmed.`
+    : `Write tests from the Testing Strategy in ${paths.issue} and
+${paths.plan}, using the repo's existing framework and conventions.
+Run them to confirm they fail for the right reasons (missing
+implementation, not broken tests), then move on.`
 }
 
-Skip Steps 2-3 test phases ONLY when the change is purely non-behavioral (config files, documentation, pure refactors with no new behavior). For refactors, verify existing tests still pass before and after.
-
-## STRICT Requirements
-
-### Match Existing Patterns
-- Study similar code in this repo BEFORE writing
-- Copy the EXACT style: naming, formatting, error handling, comments
-- If the codebase doesn't have docstrings, don't add them
-- If the codebase uses terse variable names, use terse names
+Skip Step 2's failing-test-first requirement only for pure
+config/docs/refactors with no new behavior. Step 3 still runs — on
+the skip path, run the existing suite before and after to confirm
+nothing regressed.
 
-### Minimize Changes
-- Only modify files listed in the plan
-- Only add code that directly implements the feature
-- Delete any code that becomes unused
-- Prefer fewer lines over "cleaner" abstractions
-
-### NO Tutorial Comments
-FORBIDDEN comment patterns:
-- "First, we..." / "Now we..." / "Next, we..."
-- "This function does X" (obvious from the code)
-- "Step 1:", "Step 2:", etc.
-- Comments explaining what the next line does
-- Comments that restate the function name
-
-ALLOWED comments:
-- Non-obvious business logic explanations
-- Workaround explanations with ticket/issue references
-- Performance optimization explanations
-- Regex explanations
-
-### NO Over-Engineering
-FORBIDDEN patterns:
-- Creating interfaces/base classes for single implementations
-- Adding configuration for single use cases
-- Factory functions for simple object creation
-- Wrapper functions that just call one other function
-- Error handling for impossible code paths
-- Logging for debugging that won't ship
-
-### Scope Discipline
-- If you notice something that "should" be fixed but isn't in the issue, DON'T fix it
-- If you think of a "nice to have" feature, DON'T add it
-- If code could be "cleaner" with a refactor, DON'T refactor unless required
-
-### Code Quality
-- Fix root causes, no hacks
-- Do not bypass tests
-- Use the repo's normal commands (lint, format, test)`;
+## Step 3: Implement
+Make the Step 2 failing tests pass (or, on the skip path, keep the
+existing suite green). Work incrementally — one piece at a time, run
+tests, see progress. Do NOT weaken assertions, skip tests, reduce
+coverage, or edit the Step 2 tests to make them green. Fix the
+implementation instead.
+
+## House Rules
+- **Match the repo's style exactly.** Study similar code before
+  writing. Copy naming, formatting, error handling, and comment
+  density. No docstrings if the codebase has none; terse names if
+  surrounding code is terse.
+- **Minimum diff.** Only touch files in the plan. Delete code that
+  becomes unused. Fewer lines beat "cleaner" abstractions.
+- **No over-engineering.** No interfaces for one implementation, no
+  config for one use case, no factories for simple objects, no
+  wrappers around one call, no error handling for impossible paths,
+  no debug logging that won't ship, no speculative optimizations
+  (caches, memoization, custom data structures) without a benchmark.
+  When in doubt, use brute force.
+- **No tutorial comments.** Skip "First we...", "Step N:", and
+  comments that restate code. Keep only non-obvious logic, workaround
+  refs with ticket links, perf notes, and regex explanations.
+- **Stay in scope.** If you notice something "should" be fixed or
+  could be "cleaner", don't — it's not in the ticket.
+- **Fix root causes.** Don't bypass tests. Use the repo's standard
+  lint/format/test commands.`;
 
     async function captureCodexSessionId(runStartTimeMs, resultObj) {
       let sid = null;

src/machines/develop/issue-draft.machine.js

@@ -55,7 +55,7 @@ function formatBodyWithComments(body, comments) {
  * @param {string} id - Issue ID (e.g. "#42", "!7", "GH-60")
  * @param {string} repoRoot - Repo directory for CLI calls
  * @param {string} localIssuesDir - Resolved path to local issues dir (for "local" source)
- * @returns {string | null}
+ * @returns {Promise<string | null>}
  */
 async function fetchIssueBody(
   source,
@@ -341,7 +341,7 @@ export default defineMachine({
         ? "\nFetch the full issue description via Linear MCP using the issue id above.\n"
         : "";
 
-    const issuePrompt = `Draft an ISSUE.md for the chosen issue. Use the local codebase in ${repoRoot} as ground truth.
+    const issuePrompt = `Draft an ISSUE.md for the chosen issue. Use the codebase in ${repoRoot} as ground truth.
 
 Chosen issue:
 - source: ${input.issue.source}
@@ -352,38 +352,44 @@ ${issueBodySection}
 Clarifications from user:
 <user-data field="clarifications">${sanitizeUserData(input.clarifications || "(none provided)")}</user-data>
 
-Scratchpad for iterative notes:
-- path: ${scratchpadPath}
-- append hypotheses, constraints, open questions, and feedback between drafting passes
-- keep temporary notes in this scratchpad (not in \`issues/\`)
+Scratchpad for notes between drafting passes: ${scratchpadPath}
+(**Append** hypotheses, constraints, open questions, and feedback —
+preserve existing notes, never overwrite. Keep temporary notes here,
+not in \`issues/\`.)
 
-Output ONLY markdown suitable for writing directly to ISSUE.md.
-If you wrote ISSUE.md to disk via a tool, also output its full contents to stdout.
+Output ONLY markdown suitable for writing directly to ISSUE.md (no
+preamble, no commentary). If you wrote it to disk via a tool, also
+output its contents to stdout.
 
 ## Required Sections (in order)
-1. **Metadata**: Source, Issue ID, Repo Root (relative path), Difficulty (1-5)
-2. **Problem**: What's wrong or missing — reference specific files/functions
-3. **Requirements**: Behavioral requirements using EARS Syntax Patterns:
-   - Ubiquitous: The <system> shall <behavior>.
-   - Event-driven: WHEN <trigger>, the <system> shall <behavior>.
-   - State-driven: WHILE <state>, the <system> shall <behavior>.
-   - Unwanted Behavior: IF <trigger>, THEN the <system> shall <behavior>.
-   - Optional Feature: WHERE <feature is present>, the <system> shall <behavior>.
-4. **Changes**: Exactly which files need to change and how
-5. **Testing Strategy**: Search the codebase for existing test files/patterns, then specify:
-   - **Existing tests**: Which test files cover related behavior (paths + what they test)
-   - **Test patterns**: The repo's test framework, conventions, assertion style
-   - **New test cases**: Concrete test cases to write — inputs, expected outputs, edge cases
-   ${
+1. **Metadata** — Source, Issue ID, Repo Root (relative), Difficulty (1-5).
+2. **Problem** — what's wrong or missing, with specific file/function refs.
+3. **Requirements** — behavioral requirements in EARS syntax. Follow
+   these sentence forms, substituting concrete project-specific text
+   for the \`<...>\` placeholders. **Do NOT emit the angle-bracket
+   placeholders or backticks verbatim** — every requirement in the
+   final output must be a concrete project-specific sentence:
+   - Ubiquitous: \`The <system> shall <behavior>.\`
+   - Event-driven: \`WHEN <trigger>, the <system> shall <behavior>.\`
+   - State-driven: \`WHILE <state>, the <system> shall <behavior>.\`
+   - Unwanted Behavior: \`IF <trigger>, THEN the <system> shall <behavior>.\`
+   - Optional Feature: \`WHERE <feature is present>, the <system> shall <behavior>.\`
+4. **Changes** — exactly which files change and how.
+5. **Testing Strategy** — search the codebase for existing test files and
+   patterns first, then list: existing tests that cover related behavior
+   (paths + what they test), the repo's test framework and conventions,
+   and concrete new test cases (inputs, outputs, edge cases).${
      (input.issue.difficulty ?? 3) >= 3
-       ? `- **Red/Green TDD**: This is a difficulty ${input.issue.difficulty ?? 3} issue — use Red/Green TDD.
-     List specific failing assertions the implementation agent should write BEFORE coding:
-     - Test name, assertion, and expected failure reason (missing function, wrong return value, etc.)
-     - These form the RED phase — they must fail for the right reasons before implementation begins`
-       : `- For this low-complexity issue, a lightweight test-after approach is acceptable if a failing-test-first approach isn't practical`
+       ? ` For this difficulty ${input.issue.difficulty ?? 3} issue, add
+   a Red/Green TDD subsection listing the specific failing assertions the
+   implementation agent should write BEFORE coding (test name, assertion,
+   expected failure reason — e.g. missing function, wrong return value).`
+       : ` For this low-complexity issue, test-after is acceptable if a failing-test-first approach isn't practical.`
    }
-6. **Verification**: A concrete shell command or test to prove the fix works (e.g. \`npm test\`, \`node -e "..."\`, \`curl ...\`). This is critical — downstream agents use this to close the feedback loop.
-7. **Out of Scope**: What this does NOT include
+6. **Verification** — a concrete shell command or test that proves the
+   fix works (e.g. \`npm test\`, \`node -e "..."\`, \`curl ...\`).
+   Downstream agents use this to close the feedback loop.
+7. **Out of Scope** — what this does NOT include.
 `;
 
     const res = await agent.execute(issuePrompt, {

src/machines/develop/plan-review.machine.js

@@ -90,10 +90,15 @@ function buildCritiqueRetryPrompt(planPath, critiquePath, round) {
     `2. Write a critical plan critique as markdown to **${critiquePath}**.${roundNote}\n\n` +
     `Required sections (in order):\n` +
     `1. Critical Issues (Must Fix)\n` +
-    `2. Over-Engineering Concerns\n` +
-    `3. Concerns (Should Address)\n` +
-    `4. Questions (Need Clarification)\n` +
-    `5. Verdict (REJECT | REVISE | PROCEED WITH CAUTION | APPROVED)\n\n` +
+    `2. Over-Engineering Concerns — flag speculative optimizations (caches,\n` +
+    `   memoization, fancy algorithms) that lack a measurement, and flag fancy\n` +
+    `   data structures where plain arrays/objects would work for the expected n\n` +
+    `3. Data Structure Review — is the plan carrying the right data shapes?\n` +
+    `   Would different data make the algorithm self-evident? Are core types\n` +
+    `   reused from elsewhere in the repo?\n` +
+    `4. Concerns (Should Address)\n` +
+    `5. Questions (Need Clarification)\n` +
+    `6. Verdict (REJECT | REVISE | PROCEED WITH CAUTION | APPROVED)\n\n` +
     `Constraints:\n` +
     `- Do not modify tracked files.\n` +
     `- Keep critique concrete with file-level references when possible.\n` +
@@ -249,10 +254,15 @@ export default defineMachine({
 
 Required sections (in order):
 1. Critical Issues (Must Fix)
-2. Over-Engineering Concerns
-3. Concerns (Should Address)
-4. Questions (Need Clarification)
-5. Verdict (REJECT | REVISE | PROCEED WITH CAUTION | APPROVED)
+2. Over-Engineering Concerns — flag speculative optimizations (caches,
+   memoization, fancy algorithms) that lack a measurement, and flag fancy
+   data structures where plain arrays/objects would work for the expected n
+3. Data Structure Review — is the plan carrying the right data shapes?
+   Would different data make the algorithm self-evident? Are core types
+   reused from elsewhere in the repo?
+4. Concerns (Should Address)
+5. Questions (Need Clarification)
+6. Verdict (REJECT | REVISE | PROCEED WITH CAUTION | APPROVED)
 
 Constraints:
 - Do not modify tracked files.