diff --git a/src/agents/codex-session-discovery.js b/src/agents/codex-session-discovery.js index 448cd72..3434a2d 100644 --- a/src/agents/codex-session-discovery.js +++ b/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} - sessionId or null if not found */ export async function discoverCodexSessionId(workspaceDir, runStartTimeMs) { const codexHome = process.env.CODEX_HOME || path.join(os.homedir(), ".codex"); diff --git a/src/machines/develop/implementation.machine.js b/src/machines/develop/implementation.machine.js index c4aaf29..d726a5f 100644 --- a/src/machines/develop/implementation.machine.js +++ b/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; diff --git a/src/machines/develop/issue-draft.machine.js b/src/machines/develop/issue-draft.machine.js index 41207b6..4cd28fe 100644 --- a/src/machines/develop/issue-draft.machine.js +++ b/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} */ 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: ${sanitizeUserData(input.clarifications || "(none provided)")} -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 shall . - - Event-driven: WHEN , the shall . - - State-driven: WHILE , the shall . - - Unwanted Behavior: IF , THEN the shall . - - Optional Feature: WHERE , the shall . -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 shall .\` + - Event-driven: \`WHEN , the shall .\` + - State-driven: \`WHILE , the shall .\` + - Unwanted Behavior: \`IF , THEN the shall .\` + - Optional Feature: \`WHERE , the shall .\` +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, { diff --git a/src/machines/develop/plan-review.machine.js b/src/machines/develop/plan-review.machine.js index f22dbc3..f1ad1b4 100644 --- a/src/machines/develop/plan-review.machine.js +++ b/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. diff --git a/src/machines/develop/planning.machine.js b/src/machines/develop/planning.machine.js index 5e21eb2..110b286 100644 --- a/src/machines/develop/planning.machine.js +++ b/src/machines/develop/planning.machine.js @@ -206,63 +206,61 @@ export default defineMachine({ } // Full prompt used only when creating a fresh session. - const planPrompt = `You are planning an implementation. Follow this structured approach: - -## Phase 1: Research (MANDATORY) -Before writing any plan: -1. Read ${paths.issue} completely -2. Search the codebase to understand existing patterns, conventions, and architecture -3. For any external dependencies mentioned: - - Verify they exist and are actively maintained - - Read their actual documentation (not your training data) - - Confirm the APIs you plan to use actually exist -4. Identify similar existing implementations in this codebase to use as templates + const planPrompt = `You are planning an implementation. Write ${paths.plan}. + +## Phase 1: Research +- Read ${paths.issue} completely. +- Search the codebase for existing patterns, conventions, and similar + implementations to use as templates. +- For external dependencies: verify they exist, are maintained, and the + APIs you'd call are real — read actual docs, not your training data. ## Phase 2: Evaluate Approaches -Consider at least 2 different approaches. For each: -- Pros/cons -- Complexity -- Alignment with existing patterns - -Select the simplest approach that solves the problem. - -## Phase 3: Write Plan to ${paths.plan} - -Structure: -1. **Summary**: One paragraph describing what will change -2. **Approach**: Which approach and why (reference existing patterns) -3. **Files to Modify**: List each file with specific changes -4. **Files to Create**: Only if absolutely necessary (prefer modifying existing files) -5. **Dependencies**: Any new dependencies with version and justification -6. **Testing Strategy**: - - Reference the testing strategy from ISSUE.md if present - - List existing test files that validate related behavior - - Describe specific test cases to write (inputs, expected outputs, edge cases) - - Specify the test command to run - - **Red/Green TDD** (when ISSUE.md difficulty >= 3 or change is non-trivial): - - RED phase: List the exact test files to create/modify and the failing assertions to write BEFORE implementation. Each test should target one requirement from ISSUE.md. Describe the expected failure (e.g. "ReferenceError: parseConfig is not defined", "Expected 3 but got undefined"). - - GREEN phase: Which files to implement and in what order to make tests pass incrementally. - - If ISSUE.md difficulty < 3 and the change is straightforward, note that test-after is acceptable. -7. **Out of Scope**: Explicitly list what this change does NOT include - -## Complexity Budget -- Prefer modifying 1-3 files over touching many files -- Prefer using existing utilities over creating new abstractions -- Prefer inline code over new helper functions for one-time operations -- Prefer direct solutions over configurable/extensible patterns - -## Anti-Patterns to AVOID -- Do NOT add abstractions "for future flexibility" -- Do NOT create wrapper classes/functions around simple operations -- Do NOT add configuration options that aren't requested -- Do NOT refactor unrelated code -- Do NOT add error handling for impossible scenarios - -Constraints: -- Do NOT implement code yet -- Do NOT modify any tracked files (only write ${paths.plan}) -- Do NOT invent APIs - verify they exist in actual documentation -- Do NOT ask questions; use repo conventions and ISSUE.md as ground truth`; + +Pin down the **data** first: input shape, output shape, intermediate +state. Reuse existing repo types before inventing new ones. When the +data structures are right, the algorithm is usually obvious. + +Compare at least 2 approaches on pros/cons, complexity, alignment with +existing patterns, and **expected n** (loop / collection / request +counts). For bounded or small n, prefer brute force and record the +estimate so the reviewer can challenge it. + +Select the simplest approach. When in doubt, use brute force — clever +code is buggier and n is usually small. + +## Phase 3: Write ${paths.plan} + +Required sections: +1. **Summary** — one paragraph. +2. **Approach** — which approach and why, referencing existing patterns. +3. **Files to Modify** — list each existing file with the specific + changes it needs. +4. **Files to Create** — only when absolutely necessary; prefer + modifying existing files. Justify each new file. +5. **Dependencies** — versions + justification. +6. **Testing Strategy** — reference ISSUE.md's strategy, list existing + related tests, describe new test cases (inputs, outputs, edges), and + specify the test command. For difficulty ≥ 3, add a Red/Green TDD + subsection listing the failing assertions to write BEFORE code (test + name + expected failure reason — e.g. "ReferenceError: + parseConfig is not defined"). For difficulty < 3, test-after is + acceptable only when a failing-test-first approach isn't practical. +7. **Out of Scope** — what this change does NOT include. + +## House Rules +- Small scope: 1-3 files, existing utilities over new abstractions, + inline code over helpers for one-time ops, direct solutions over + configurable ones. +- No over-engineering: no "future flexibility" abstractions, no + wrappers around single operations, no unrequested configuration, no + unrelated refactors, no error handling for impossible cases, no + speculative optimizations (caches, memoization, custom data + structures) without a benchmark — measure before tuning. +- Do not invent APIs; verify in actual documentation. +- Do not implement code yet. +- Do not modify any tracked files — only write ${paths.plan}. +- Do not ask questions — ISSUE.md is ground truth.`; // Append active-branch awareness so the planner can detect conflicts let activeBranchesSection = ""; diff --git a/src/machines/develop/pr-creation.machine.js b/src/machines/develop/pr-creation.machine.js index f713958..967ac2b 100644 --- a/src/machines/develop/pr-creation.machine.js +++ b/src/machines/develop/pr-creation.machine.js @@ -189,7 +189,7 @@ export default defineMachine({ const mrRaw = (mr.stdout || "").trim(); const mrLines = mrRaw.split("\n").filter((l) => l.trim()); prUrl = mrLines.find((l) => l.startsWith("http")) || mrLines.pop() || ""; - if (!prUrl || !prUrl.startsWith("http")) { + if (!prUrl?.startsWith("http")) { throw new Error( `glab mr create did not return an MR URL. Output:\n${mrRaw || "(empty)"}`, ); @@ -217,7 +217,7 @@ export default defineMachine({ const raw = (pr.stdout || "").trim(); const lines = raw.split("\n").filter((l) => l.trim()); prUrl = lines.find((l) => l.startsWith("http")) || lines.pop() || ""; - if (!prUrl || !prUrl.startsWith("http")) { + if (!prUrl?.startsWith("http")) { throw new Error( `gh pr create did not return a PR URL. Output:\n${raw || "(empty)"}`, ); diff --git a/src/machines/develop/quality-review.machine.js b/src/machines/develop/quality-review.machine.js index 9f1a79a..f1f3760 100644 --- a/src/machines/develop/quality-review.machine.js +++ b/src/machines/develop/quality-review.machine.js @@ -100,10 +100,9 @@ Verify whether each critical/major finding has been addressed. If the programmer ? `\n**WARNING: The plan in ${paths.plan} was NOT approved by the plan reviewer** (review rounds exhausted with unresolved concerns). Treat the plan as a tentative guide, not an agreed-upon spec. Scrutinize the implementation more carefully against the original issue requirements rather than trusting the plan as authoritative.\n` : ""; - return `You are a code reviewer. Your role is to CRITIQUE only — do NOT modify any source code files. + return `You are a code reviewer. Critique only — do NOT modify source files. -Read ${paths.issue} to understand what was originally requested. -Read ${paths.plan} for the technical approach and constraints.${planCaveat} +Read ${paths.issue} (what was requested) and ${paths.plan} (technical approach).${planCaveat} ${roundContext} @@ -111,65 +110,59 @@ ${deltaSection} ## Review Checklist ### 1. Scope Conformance -- Does the change implement what ${paths.issue} requested and conform to the approach in ${paths.plan}? -- Are there unrequested features? Flag them. -- Are there unrelated refactors? Flag them. +Does the change implement what ${paths.issue} requested and conform to +${paths.plan}? Flag unrequested features and unrelated refactors. ### 2. Completeness -- Is the implementation fully complete? No stubs, TODOs, placeholders? -- Are there test bypasses or skipped tests? +Any stubs, TODOs, placeholders, test bypasses, or skipped tests? ### 3. Code Quality -- Is this the SIMPLEST solution that works? -- Are there unnecessary abstractions, wrapper functions, or over-engineering? +- Is this the SIMPLEST solution that works? Flag unnecessary + abstractions and wrapper functions. +- Any optimizations (caches, memoization, indexes, custom data + structures, fancy algorithms) added without a benchmark? Flag as + over-engineering — measure before tuning. +- Do the data structures make the logic self-evident, or is the code + fighting the shape of the data? ### 4. Comment Hygiene -Flag these comment patterns: -- Tutorial-style: "First we...", "Now we...", "Step N:" -- Restating code: "// increment counter" above counter++ -- Narration: "Here we define...", "This function..." +Flag tutorial-style ("First we...", "Step N:"), restating code +("// increment counter" above \`counter++\`), and narration +("Here we define...", "This function..."). ### 5. Backwards-Compat Hacks -Flag these patterns: -- Variables renamed to start with \`_\` but not used -- Re-exports of removed items for compatibility -- Empty functions kept for interface compatibility +Flag unused \`_\`-prefixed vars, re-exports of removed items, and +empty functions kept for interface compatibility. ### 6. Correctness -- Edge cases handled appropriately? -- Off-by-one errors? -- Error handling only for errors that can actually occur? +Edge cases, off-by-one errors, error handling only for errors that +can actually occur. ${planAdherenceItem} ## ppcommit (commit hygiene) ${ppSection} ## Output Format - -Write your findings to ${paths.reviewFindings} with this structure: +Write findings to ${paths.reviewFindings} as markdown. Each finding is a +\`## Finding N\` heading followed by **Severity** (critical | major | +minor), **File**, **Lines**, **Issue**, **Suggestion** — like this: \`\`\`markdown # Review Findings — Round ${round} ## Finding 1 -- **Severity**: critical | major | minor -- **File**: path/to/file.js +- **Severity**: major +- **File**: src/foo.js - **Lines**: 42-58 -- **Issue**: Description of the problem -- **Suggestion**: How to fix it +- **Issue**: +- **Suggestion**: -## Finding 2 -... - -## VERDICT: APPROVED +## VERDICT: REVISE \`\`\` -Use \`## VERDICT: APPROVED\` if there are no critical or major findings remaining. -Use \`## VERDICT: REVISE\` if critical or major findings need to be addressed. - -IMPORTANT: -- Write findings to the file, do NOT modify source code -- The verdict line must be the LAST ## heading in the file -- Minor findings alone are NOT grounds for REVISE — only critical/major findings`; +End the file with \`## VERDICT: APPROVED\` or \`## VERDICT: REVISE\` as +the LAST \`##\` heading. Use REVISE only for critical/major findings — +minor findings alone are not grounds for REVISE. Do NOT modify source +files.`; } function buildProgrammerFixPrompt(paths, round) { diff --git a/src/machines/research/issue-synthesis.machine.js b/src/machines/research/issue-synthesis.machine.js index b5bdd05..379165b 100644 --- a/src/machines/research/issue-synthesis.machine.js +++ b/src/machines/research/issue-synthesis.machine.js @@ -115,7 +115,7 @@ export default defineMachine({ Repo root: ${repoRoot} -## Input Artifacts (read these files) +## Input artifacts (read these) - Pointer analysis: ${briefPath} - Web references: ${webRefPath} - Validation results: ${validationPath} @@ -126,23 +126,31 @@ Clarifications: Feedback to incorporate: ${sanitizeUserData(feedbackSection)} -## Phase 1: Codebase Exploration (MANDATORY) -Before drafting issues, explore the codebase at \`${repoRoot}\`: -- Search for existing test files and understand the test framework/conventions -- Identify project structure, key modules, and architecture patterns -- Find existing implementations related to the problem spaces in the analysis brief -- Note file paths that issues should reference +## Phase 1: Codebase exploration (mandatory) +Before drafting, explore the codebase at \`${repoRoot}\` — identify the +test framework and conventions, project structure and key modules, +existing implementations related to the problem spaces in the analysis +brief, and file paths the issues should reference. -## Phase 2: Issue Drafting -With codebase context, draft the issue backlog. Ground every issue in actual files and patterns you found. +## Phase 2: Issue drafting +Ground every issue in actual files and patterns you found. Rules: -- Return EXACTLY ${maxIssues} issues (or fewer only if the analysis genuinely warrants fewer). -- Keep issues small, independently verifiable, and dependency-light. +- Return EXACTLY ${maxIssues} issues (fewer only if the analysis warrants). +- Keep issues small, independently verifiable, dependency-light. - Include references and validation metadata per issue. -- Do not use issues/ as scratch storage; this workflow uses .coder/scratchpad. -- Do NOT re-add issues that prior feedback explicitly asked to drop. -- Each issue MUST include a "testing_strategy" field grounded in actual test files you found in the codebase. Include: existing tests to leverage (with real file paths), new tests to write with expected behavior, and the repo's test framework/conventions. +- Use .coder/scratchpad for scratch notes, not \`issues/\`. +- Do NOT re-add issues prior feedback asked to drop. +- For new features, prefer brute-force / simple-data-structure for the + first issue UNLESS the analysis brief, validation results, or user + clarifications name an explicit performance or scale requirement the + simple version wouldn't meet. Only split optimization into a + follow-up when the simple version is already correct for the stated + constraints; when you do split, the follow-up's objective must name + a measurement target (baseline + goal). +- Each issue MUST include "testing_strategy" grounded in real test files: + existing tests to leverage (paths + what they cover), new tests to + write with expected behavior, and the repo's test framework/conventions. Return ONLY valid JSON in this schema: { diff --git a/src/machines/research/spec-architect.machine.js b/src/machines/research/spec-architect.machine.js index c48bf1f..87a6b82 100644 --- a/src/machines/research/spec-architect.machine.js +++ b/src/machines/research/spec-architect.machine.js @@ -48,9 +48,13 @@ ${JSON.stringify(manifest, null, 2)} Tasks: 1. Explore the codebase structure at ${repoRoot} 2. Decompose the project into architectural domains -3. Create ADR-style decision records for key architectural choices -4. Define implementation phases with issue groupings -5. Sequence issues within the same domain by populating the "depends_on" field with the titles of prerequisite issues +3. For each domain, record at least one ADR-style decision covering the + **core data structures** — their shape, ownership, and lifetime. Get the + data right and the algorithms follow; get it wrong and no algorithm + will save you. +4. Create ADR-style decision records for other key architectural choices +5. Define implementation phases with issue groupings +6. Sequence issues within the same domain by populating the "depends_on" field with the titles of prerequisite issues Return ONLY valid JSON: { diff --git a/src/workflows/develop-git.js b/src/workflows/develop-git.js index 4f9c006..e72b5a6 100644 --- a/src/workflows/develop-git.js +++ b/src/workflows/develop-git.js @@ -111,7 +111,7 @@ async function fetchMergeRequestsViaApi(repoRoot, _log, opts = {}) { if (urlRes.status !== 0) return []; const url = (urlRes.stdout || "").trim(); const projectPathRaw = extractGitLabProjectPath(url); - if (!projectPathRaw || !projectPathRaw.includes("/")) return []; + if (!projectPathRaw?.includes("/")) return []; const projectPath = encodeURIComponent( projectPathRaw.replace(/\.git$/, ""), ); diff --git a/src/workflows/failure-monitor.js b/src/workflows/failure-monitor.js index 8d28201..f946c1e 100644 --- a/src/workflows/failure-monitor.js +++ b/src/workflows/failure-monitor.js @@ -364,7 +364,7 @@ export function fileRcaIssue({ repoRoot, title, body, labels, repo }) { const raw = (res.stdout || "").trim(); const lines = raw.split("\n").filter((l) => l.trim()); const issueUrl = lines.find((l) => l.startsWith("http")) || lines.pop() || ""; - if (!issueUrl || !issueUrl.startsWith("http")) { + if (!issueUrl?.startsWith("http")) { throw new Error( `gh issue create did not return an issue URL. Output:\n${raw || "(empty)"}`, );