fix(auto-fix): repair lead plan marker gates#58
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a deterministic repair that recognizes lead-plan-gate failures for missing required markers, injects a self-healing block into the workflow artifact to append missing sections (Non-goals, Routing contract, Implementation contract), and resumes the workflow with retry metadata after rewriting the artifact. ChangesLead-Plan Marker Deterministic Repair
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bca810038b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'const appendLeadPlanSection = (heading, lines) => {', | ||
| " const section = ['', '## ' + heading, '', ...lines].join('\\n');", | ||
| " if (body.includes('GENERATION_LEAD_PLAN_READY')) {", | ||
| " body = body.replace(/\\n*GENERATION_LEAD_PLAN_READY\\s*$/, section + '\\n\\nGENERATION_LEAD_PLAN_READY');", |
There was a problem hiding this comment.
When a lead plan contains GENERATION_LEAD_PLAN_READY but has any trailing text after it, this branch is taken but the anchored replace does not match, so body is written back unchanged and the final Non-goals/Routing/Implementation checks still fail on the retry. The previous gate only required body.includes(...), so this malformed-but-possible plan reaches the new self-healing path; append to the end or detect the no-op instead of assuming the marker is final.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/local/auto-fix-loop.test.ts (1)
208-255: ⚡ Quick winAdd a test variant exercising the
plainOldBlockfallback path inrepairLeadPlanRequiredMarkerGate.The current fixture (
leadPlanMarkerWorkflowContent) embeds the gate command using\\nescape sequences, which matches theescapedOldBlockbranch (lines 741-748 inauto-fix-loop.ts). TheplainOldBlockfallback (lines 750-757) — intended for workflow files where the gate command uses actual newlines — is not covered by any test. A regression in that path would go undetected.🧪 Suggested additional test or repairWorkflowDeterministically assertion
+ it('deterministically repairs lead-plan marker gates with plain-newline command format', () => { + const plainContent = leadPlanMarkerWorkflowContent().replace(/\\n/g, '\n'); + const evidence = localResponseToWorkflowRunEvidence_equivalent({ + // Evidence text must match the trigger pattern: + failureText: 'lead plan missing required marker: Non-goals', + }); + const repair = repairWorkflowDeterministically({ + artifactPath: 'workflows/generated/lead-plan-marker.ts', + artifactContent: plainContent, + evidence: leadPlanMarkerEvidence(), + }); + + expect(repair).toMatchObject({ + applied: true, + mode: 'deterministic', + summary: expect.stringContaining('lead-plan-gate append missing required plan markers'), + }); + expect(repair?.content).toContain("appendLeadPlanSection('Non-goals'"); + expect(repair?.content).toContain('Local execution must run through Agent Relay using the generated workflow artifact.'); + });Alternatively, a simpler inline assertion in the existing test:
expect(result.ok).toBe(true); + // Verify the plain-newline fallback also produces a repaired artifact: + const plainRepair = repairWorkflowDeterministically({ + artifactPath: 'workflows/generated/lead-plan-marker.ts', + artifactContent: leadPlanMarkerWorkflowContent().replace(/\\n/g, '\n'), + evidence: localResponseToWorkflowRunEvidence(firstFailure, 1), + }); + expect(plainRepair).not.toBeNull(); + expect(plainRepair?.content).toContain("appendLeadPlanSection('Non-goals'");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/local/auto-fix-loop.test.ts` around lines 208 - 255, Add a test variant that exercises the plainOldBlock fallback in repairLeadPlanRequiredMarkerGate by providing a workflow fixture where the gate command uses real newlines (not "\\n" escapes) — create a copy of leadPlanMarkerWorkflowContent with the gate command written using actual newline characters, then call runWithAutoFix (using the same runSingleAttempt/artifactWriter pattern) and assert that the artifactWriter receives the repaired 'workflows/generated/lead-plan-marker.ts' containing the multiline gate block and that result.auto_fix?.attempts[0] shows the expected applied_fix summary and failed_step 'lead-plan-gate'; this will ensure the plainOldBlock branch in repairLeadPlanRequiredMarkerGate (and not only escapedOldBlock) is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/local/auto-fix-loop.test.ts`:
- Around line 208-255: Add a test variant that exercises the plainOldBlock
fallback in repairLeadPlanRequiredMarkerGate by providing a workflow fixture
where the gate command uses real newlines (not "\\n" escapes) — create a copy of
leadPlanMarkerWorkflowContent with the gate command written using actual newline
characters, then call runWithAutoFix (using the same
runSingleAttempt/artifactWriter pattern) and assert that the artifactWriter
receives the repaired 'workflows/generated/lead-plan-marker.ts' containing the
multiline gate block and that result.auto_fix?.attempts[0] shows the expected
applied_fix summary and failed_step 'lead-plan-gate'; this will ensure the
plainOldBlock branch in repairLeadPlanRequiredMarkerGate (and not only
escapedOldBlock) is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c5e7c298-f199-4f25-8b2b-39c149975fd8
📒 Files selected for processing (2)
src/local/auto-fix-loop.test.tssrc/local/auto-fix-loop.ts
1 participant
Summary
Validation