feat: add per-task build check after code-writer to catch type errors early (#66) (#66)#73
Merged
feat: add per-task build check after code-writer to catch type errors early (#66) (#66)#73
Conversation
…loop to implementation executor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds a per-task build/compile verification step that runs after each code-writer completes within the implementation phase. When the build fails, a fix-surgeon retry loop is triggered (up to a configurable maximum) before the task proceeds to test-writer. This catches TypeScript type errors and compilation failures in isolation, preventing them from compounding across subsequent tasks and reaching the integration checker with multiple accumulated failures.
Closes #66
Changes
src/config/schema.ts: Added two new options fields —perTaskBuildCheck(boolean, defaulttrue) andmaxBuildFixRounds(integer 1–5, default2) — placed alongside the existingbuildVerificationandtestVerificationoptions.src/executors/implementation-phase-executor.ts: Added per-task build check between code-writer and test-writer. ImportsexecShellfrom../util/process.js. When bothcommands.buildandoptions.perTaskBuildCheckare truthy, runs the build and enters a fix-surgeon retry loop on failure. Throws an error after exhaustingmaxBuildFixRoundsretries.src/agents/context-builder.ts: Extended theissueTypeunion inbuildForFixSurgeonto include'build', enabling fix-surgeon to receive build-specific failure context.tests/config-schema.test.ts: Added test suites forperTaskBuildCheckandmaxBuildFixRoundscovering defaults, valid values, boundary conditions, and invalid inputs.tests/implementation-phase-executor.test.ts: Added aper-task build checkdescribe block with 11 test cases covering all paths: skip when disabled, run when enabled, fix-surgeon invocation on failure, token recording, file writing, and error propagation after exhausting retries.Implementation Details
The build check follows the same patterns already established in
integration-phase-executor.ts:execShellfor running the build command,writeFileto persist failure output, andbuildForFixSurgeonto create the fix-surgeon context. The retry loop runs at mostmaxBuildFixRoundstimes; each iteration writes the combined stderr+stdout to a per-round file (build-failure-{taskId}-{round}.txt), launches fix-surgeon scoped to the task's files, and re-runs the build. If all rounds are exhausted with a non-zero exit code, anErroris thrown, which is caught by the outer task retry/block logic. The feature is gated by theperTaskBuildCheckconfig flag so teams with slow builds can opt out.Testing
npx vitest run— 0 failures)'build'issueType passed to context builder, failure output written to disk, token recording, re-run after each fix round, error thrown after exhausting retries, and fix-surgeon invoked exactlymaxBuildFixRoundstimesIntegration Verification
npm run build, exit code 0)npx vitest run, exit code 0)npm install, exit code 0)Notes
perTaskBuildCheckdefaults totrue, meaning existing projects that configure abuildcommand will automatically get this behavior. Projects that lack abuildcommand are unaffected (the check is skipped whencommands.buildis falsy).Cadre Process Challenges
N, whether to check after test-writer, how to scope files to fix-surgeon) required reasonable assumptions. A sentence or two in the issue specifying N and the exact scope model would have eliminated all three.buildForFixSurgeonsignature incontext-builder.tsneeded to be extended to accept'build'as anissueType, which was not mentioned in the issue. The code-writer had to discover this by reading adjacent code; it is not surfaced in the issue's "Affected Files" section.execShell,buildForFixSurgeon, and thewriteFilepattern already existed in the integration executor — but this required the planner to read a sibling executor file that was not listed in "Affected Areas." Making the pattern-reference explicit in the plan would help the code-writer agent without requiring extra exploration.context-builder.ts— a file not listed in the issue's affected areas — which required an unplanned edit to extend theissueTypeunion. Listing all transitively affected files (even for small type changes) in the issue or analysis would eliminate this class of surprise mid-implementation edit.Closes #66