docs(workflows): teach repairable gate patterns#32
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughTwo workflow documentation files are updated to introduce and exemplify a "repair before fail" pattern. Verification gates now capture output with ChangesRepair-Before-Fail Workflow Pattern
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
skills/writing-agent-relay-workflows/SKILL.md (1)
931-937: ⚡ Quick winShow a final deterministic check after
repair-filesin this example.This example currently repairs from
{{steps.verify-files.output}}but does not show a post-repair deterministic rerun. Adding one keeps the guidance consistent with “repair before fail, then verify deterministically.”🤖 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 `@skills/writing-agent-relay-workflows/SKILL.md` around lines 931 - 937, Add a deterministic post-repair verification step after step('repair-files') that reruns the original verification logic and fails the workflow if issues remain; specifically, introduce a new step (e.g., step('verify-files-final') or reuse step('verify-files') by making it depend on 'repair-files') that invokes the same check used by verify-files and uses its output to deterministically confirm repair (and propagate a clear failure if the check still reports missing/incorrect files).
🤖 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 `@skills/relay-80-100-workflow/SKILL.md`:
- Around line 412-423: The .step('commit') uses failOnError: false which can let
the workflow continue without a real commit; add a deterministic follow-up step
(e.g., .step('verify-commit')) that dependsOn: ['commit'] and explicitly
verifies the commit (for example by checking git status/that HEAD changed or
reading the commit message via git rev-parse/ git log) and fails the workflow if
the commit is missing, or routes to a repair step; update workflow transitions
so successful completion requires the new verify-commit step and include a
repair path if verification fails.
In `@skills/writing-agent-relay-workflows/SKILL.md`:
- Around line 164-170: The workflow needs a deterministic final verification
step after the repair loop: add a new step named 'verify-final' that dependsOn
'repair-verify' and re-runs the original verification logic deterministically
(same checks as 'verify') so the workflow only completes when 'verify-final'
reports success; reference the existing step names 'repair-verify' and 'verify'
when wiring dependencies and ensure 'verify-final' produces the final pass/fail
evidence used for completion.
---
Nitpick comments:
In `@skills/writing-agent-relay-workflows/SKILL.md`:
- Around line 931-937: Add a deterministic post-repair verification step after
step('repair-files') that reruns the original verification logic and fails the
workflow if issues remain; specifically, introduce a new step (e.g.,
step('verify-files-final') or reuse step('verify-files') by making it depend on
'repair-files') that invokes the same check used by verify-files and uses its
output to deterministically confirm repair (and propagate a clear failure if the
check still reports missing/incorrect files).
🪄 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: d0b705e6-9604-43ac-bb15-031e9a19a40a
📒 Files selected for processing (2)
skills/relay-80-100-workflow/SKILL.mdskills/writing-agent-relay-workflows/SKILL.md
14d8f1d to
f9cee4f
Compare
| // Step 4: Repair again if the rerun is still red | ||
| .step('fix-tests-final', { | ||
| agent: 'tester', | ||
| dependsOn: ['run-tests-final'], | ||
| task: `If the final test rerun passed, record the green evidence. | ||
| If it failed, fix the remaining issue and rerun until green: | ||
| {{steps.run-tests-final.output}}`, | ||
| verification: { type: 'exit_code' }, | ||
| }) |
There was a problem hiding this comment.
🟡 "Three-step pattern" text not updated after adding a fourth step
The PR adds a new Step 4 (fix-tests-final) at lines 83-91, making the pattern a 4-step pattern. However, the introductory text at skills/relay-80-100-workflow/SKILL.md:44 still says "follow this three-step pattern" and the summary at line 94 still says "Why three steps instead of one?" This is a documentation inconsistency introduced by this PR — readers will be confused when the code shows 4 clearly-numbered steps but the surrounding text describes 3.
Prompt for agents
The PR adds a Step 4 (fix-tests-final) to the Test-Fix-Rerun Pattern, making it a 4-step pattern. But the text around the code block still says "three-step pattern" (line 44) and "Why three steps instead of one?" (line 94). Both of these text references need to be updated to say "four" instead of "three". Additionally, the summary section after the code block currently lists 3 bullet points explaining the 3 steps — a 4th bullet should be added explaining the role of the new repair step.
Was this helpful? React with 👍 or 👎 to provide feedback.
| dependsOn: [fix-service-verification] | ||
| command: git add src/types.ts src/service.ts && git commit -m "feat: add pending status" | ||
| failOnError: true | ||
| failOnError: false |
There was a problem hiding this comment.
🟡 YAML commit step contradicts new "rerun acceptance checks" guidance
The PR adds new guidance at skills/writing-agent-relay-workflows/SKILL.md:1220 stating "Always commit with a deterministic step, never an agent step; rerun acceptance checks in that step and skip commit unless green." However, the YAML Multi-File Edit Pattern's commit step at line 1212 just runs git add && git commit without re-running any acceptance checks. Combined with the change at line 1213 from failOnError: true to failOnError: false (and all upstream verification steps also changed to failOnError: false), this example teaches a pattern where broken code could be committed — the repair agent steps have no verification field, so they complete on any exit 0, and the commit proceeds without verifying the repairs worked.
Prompt for agents
In the Multi-File Edit Pattern YAML example (around line 1209-1213), the commit step needs to be updated to match the PR's own stated guidance at line 1220: rerun acceptance checks before committing. Currently the commit step just does `git add && git commit` with `failOnError: false`. It should follow the same pattern as the TypeScript Full Workflow Template in relay-80-100-workflow/SKILL.md, which chains test commands before `git add && git commit` using `&&`. For example, the command could become something like: `npm run typecheck && npm test && git add src/types.ts src/service.ts && git commit -m "feat: add pending status"`. Also consider adding `verification` fields (e.g. `type: exit_code`) to the YAML repair steps (`fix-types-verification`, `fix-service-verification`, `fix-missing-files`) to match the TypeScript examples which consistently include verification on repair steps.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Addressed the visible review comments in the repair-first spirit:\n\n- Added deterministic proof steps after repair steps, so gates produce evidence instead of silently passing.\n- Kept initial gates repairable/permissive, with agents owning failures before terminal proof.\n- Added commit repair + commit proof so a failed commit cannot be ignored.\n- Fixed the stale three-step wording in the 80-to-100 skill.\n\nI treated the feedback as a request for proof-after-repair, not a return to hard-stop gates before agents get a chance to fix. |
Summary
Test plan