fix: stop auto-fix retries for setup blockers#47
Conversation
📝 WalkthroughWalkthroughThe PR adds external setup blocker detection to the auto-fix loop. When ChangesExternal Setup Blocker Escalation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/local/auto-fix-loop.test.ts (1)
378-410: ⚡ Quick winConsider table-driving this test across all external setup blocker codes.
Right now only
MISSING_ENV_VARis asserted. AddingCREDENTIALS_REJECTEDandWORKDIR_DIRTYhere would better lock down the new gate behavior.Example refactor
- it('does not auto-repair missing environment prerequisites', async () => { - const runSingleAttempt = vi - .fn() - .mockResolvedValue(blockerResponse('MISSING_ENV_VAR', 'run-1', 'runtime-launch')); + it.each(['MISSING_ENV_VAR', 'CREDENTIALS_REJECTED', 'WORKDIR_DIRTY'] as const)( + 'does not auto-repair external setup blocker %s', + async (code) => { + const runSingleAttempt = vi + .fn() + .mockResolvedValue(blockerResponse(code, 'run-1', 'runtime-launch')); const workflowRepairer = vi.fn().mockResolvedValue(workflowRepair('should not run')); const result = await runWithAutoFix(baseRequest, { maxAttempts: 7, runSingleAttempt, @@ expect(runSingleAttempt).toHaveBeenCalledTimes(1); expect(workflowRepairer).not.toHaveBeenCalled(); @@ attempts: [ expect.objectContaining({ attempt: 1, - blocker_code: 'MISSING_ENV_VAR', + blocker_code: code, fix_error: 'external setup blocker; no safe automatic workflow repair', }), ], }); expect(result.auto_fix?.escalation?.summary).toContain('environment or credentials prerequisite'); expect(result.nextActions.join('\n')).toContain('Set TEST_TOKEN before retrying.'); - }); + }, + );🤖 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 378 - 410, The test currently only asserts behavior for the 'MISSING_ENV_VAR' blocker; change it to table-drive the same expectations for each external setup blocker code (e.g., 'MISSING_ENV_VAR', 'CREDENTIALS_REJECTED', 'WORKDIR_DIRTY') by iterating over an array of codes and running the same setup/assertions per code using the existing runWithAutoFix, runSingleAttempt, fakeClassification and workflowRepairer mocks; ensure you assert runSingleAttempt called once, workflowRepairer not called, result.ok false, result.auto_fix.attempts[0].blocker_code equals the current code and fix_error equals 'external setup blocker; no safe automatic workflow repair', plus the escalation summary and nextActions contain the same prerequisite message — keep test name descriptive or use test.each/forEach to create separate it() cases.
🤖 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.
Inline comments:
In `@src/local/auto-fix-loop.ts`:
- Line 145: Update the escalation reason string that currently reads "The
blocker is an environment or credentials prerequisite outside Ricky's safe
auto-fix scope." to explicitly include WORKDIR_DIRTY so users aren’t misled;
locate the mapping or constant where WORKDIR_DIRTY is assigned this reason
(symbol WORKDIR_DIRTY and the surrounding object/variable in auto-fix-loop.ts)
and change the message to mention "environment/credentials prerequisites or a
dirty working directory (WORKDIR_DIRTY) outside Ricky's safe auto-fix scope,"
keeping proper escaping for the apostrophe.
---
Nitpick comments:
In `@src/local/auto-fix-loop.test.ts`:
- Around line 378-410: The test currently only asserts behavior for the
'MISSING_ENV_VAR' blocker; change it to table-drive the same expectations for
each external setup blocker code (e.g., 'MISSING_ENV_VAR',
'CREDENTIALS_REJECTED', 'WORKDIR_DIRTY') by iterating over an array of codes and
running the same setup/assertions per code using the existing runWithAutoFix,
runSingleAttempt, fakeClassification and workflowRepairer mocks; ensure you
assert runSingleAttempt called once, workflowRepairer not called, result.ok
false, result.auto_fix.attempts[0].blocker_code equals the current code and
fix_error equals 'external setup blocker; no safe automatic workflow repair',
plus the escalation summary and nextActions contain the same prerequisite
message — keep test name descriptive or use test.each/forEach to create separate
it() cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aa99b475-fc7a-4044-88d0-18d818059268
📒 Files selected for processing (2)
src/local/auto-fix-loop.test.tssrc/local/auto-fix-loop.ts
| request: currentRequest, | ||
| response, | ||
| debuggerResult, | ||
| reason: 'The blocker is an environment or credentials prerequisite outside Ricky\'s safe auto-fix scope.', |
There was a problem hiding this comment.
Clarify the escalation reason text for WORKDIR_DIRTY.
The current reason mentions only environment/credentials prerequisites, but WORKDIR_DIRTY is also classified here and can make the message misleading for users.
Suggested wording update
- reason: 'The blocker is an environment or credentials prerequisite outside Ricky\'s safe auto-fix scope.',
+ reason: 'The blocker is an external setup prerequisite (environment, credentials, or dirty workdir) outside Ricky\'s safe auto-fix scope.',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| reason: 'The blocker is an environment or credentials prerequisite outside Ricky\'s safe auto-fix scope.', | |
| reason: 'The blocker is an external setup prerequisite (environment, credentials, or dirty workdir) outside Ricky\'s safe auto-fix scope.', |
🤖 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.ts` at line 145, Update the escalation reason string
that currently reads "The blocker is an environment or credentials prerequisite
outside Ricky's safe auto-fix scope." to explicitly include WORKDIR_DIRTY so
users aren’t misled; locate the mapping or constant where WORKDIR_DIRTY is
assigned this reason (symbol WORKDIR_DIRTY and the surrounding object/variable
in auto-fix-loop.ts) and change the message to mention "environment/credentials
prerequisites or a dirty working directory (WORKDIR_DIRTY) outside Ricky's safe
auto-fix scope," keeping proper escaping for the apostrophe.
Summary
MISSING_ENV_VAR,CREDENTIALS_REJECTED, andWORKDIR_DIRTY.Why
A Ricky local run exhausted 7/7 auto-fix attempts on
MISSING_ENV_VARatruntime-launch. Missing env/config is not safely patchable by rewriting the generated workflow, so Ricky should surface the setup blocker directly.Verification