docs: add failure handling to executing-plans#620
docs: add failure handling to executing-plans#620immeasurablematt wants to merge 1 commit intoobra:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a comprehensive Failure Handling chapter to the executing-plans skill documentation, introducing Signal Protocol specifications, Max Iterations behavior, Non-Zero Exits handling, Partial Success Reporting format, a Retry vs Escalate Decision Tree, and Prompt Rewriting on Retry procedures. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/executing-plans/SKILL.md`:
- Around line 127-140: The fenced code block that begins with "Task fails" in
SKILL.md is missing a language specifier (MD040); update that code fence to
include the language identifier "text" (e.g., change the opening ``` to ```text)
so the ASCII-art decision tree is properly marked as text and satisfies the
markdownlint rule.
- Around line 163-171: The fenced code block for the "## FAILED: Task N —
[title]" template is missing a language specifier which trips markdownlint
MD040; update the opening fence to include a language identifier (e.g., change
the leading ``` to ```markdown or ```text) and keep the closing ``` as-is so the
block is properly annotated; locate the block around the "## FAILED: Task N —
[title]" template in SKILL.md to make this change.
- Around line 112-121: The fenced code block in SKILL.md's example output lacks
a language specifier (triggering MD040); update the block that starts with
"Batch N complete:" to include a language identifier such as text or markdown
(e.g., change ``` to ```text) so the linter accepts it; look for the example
block in SKILL.md around the "Batch N complete:" template and add the specifier
to the opening fence.
- Line 76: In SKILL.md remove or correct the broken reference to
"cinder/scripts/ralph/ralph.sh": either delete the specific file path, replace
it with a correct path if one exists, or replace the reference with a short
description of the "Ralph Loop V2" pattern and why it inspired the structured
signals guidance; update the sentence that currently mentions the missing file
so it no longer points to a non-existent resource.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f2fb5bba-0b8a-4b6e-9ae7-07f348c81419
📒 Files selected for processing (1)
skills/executing-plans/SKILL.md
|
|
||
| ### Signal Protocol | ||
|
|
||
| Use structured signals at the end of each task response. Inspired by the Ralph Loop V2 pattern in `cinder/scripts/ralph/ralph.sh`: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for the ralph.sh file in the repository
fd -t f 'ralph.sh' 2>/dev/null || echo "File not found"
# Also check if the directory structure exists
ls -la cinder/scripts/ralph 2>/dev/null || echo "Directory not found"
# Search for any ralph-related files
fd 'ralph' -t f 2>/dev/nullRepository: obra/superpowers
Length of output: 79
Remove or correct the broken reference to a non-existent file.
The documentation at line 76 references cinder/scripts/ralph/ralph.sh as inspiration for the Ralph Loop V2 pattern, but this file does not exist in the repository. Either remove the specific file reference, provide a corrected path, or replace it with a description of the pattern itself.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/executing-plans/SKILL.md` at line 76, In SKILL.md remove or correct
the broken reference to "cinder/scripts/ralph/ralph.sh": either delete the
specific file path, replace it with a correct path if one exists, or replace the
reference with a short description of the "Ralph Loop V2" pattern and why it
inspired the structured signals guidance; update the sentence that currently
mentions the missing file so it no longer points to a non-existent resource.
| ``` | ||
| Batch N complete: | ||
| ✅ Task A — [brief description] | ||
| ✅ Task B — [brief description] | ||
| ❌ Task C — FAILED: [reason] | ||
|
|
||
| Partial success. 2/3 tasks complete. | ||
| Tasks still pending: [list] | ||
| Ready for feedback. | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to the fenced code block.
The markdown linter flags this code block for missing a language specifier. Since this is a template/example output format, add text or markdown as the language identifier.
📝 Proposed fix
-```
+```text
Batch N complete:
✅ Task A — [brief description]
✅ Task B — [brief description]As per coding guidelines, markdownlint-cli2 rule MD040 requires language specifiers for fenced code blocks.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 112-112: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/executing-plans/SKILL.md` around lines 112 - 121, The fenced code
block in SKILL.md's example output lacks a language specifier (triggering
MD040); update the block that starts with "Batch N complete:" to include a
language identifier such as text or markdown (e.g., change ``` to ```text) so
the linter accepts it; look for the example block in SKILL.md around the "Batch
N complete:" template and add the specifier to the opening fence.
| ``` | ||
| Task fails | ||
| │ | ||
| ├─ Is this the first attempt? | ||
| │ YES → Diagnose root cause, change approach, retry once | ||
| │ NO → Move to escalate branch | ||
| │ | ||
| └─ Escalate when: | ||
| - Missing dependency or infra issue | ||
| - Ambiguous spec (two valid interpretations) | ||
| - Pre-existing failures in unrelated code | ||
| - Second attempt also fails | ||
| - You don't understand WHY it's failing | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to the fenced code block.
The markdown linter flags this code block for missing a language specifier. Since this is an ASCII art decision tree, add text as the language identifier.
📝 Proposed fix
-```
+```text
Task fails
│
├─ Is this the first attempt?As per coding guidelines, markdownlint-cli2 rule MD040 requires language specifiers for fenced code blocks.
📝 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.
| ``` | |
| Task fails | |
| │ | |
| ├─ Is this the first attempt? | |
| │ YES → Diagnose root cause, change approach, retry once | |
| │ NO → Move to escalate branch | |
| │ | |
| └─ Escalate when: | |
| - Missing dependency or infra issue | |
| - Ambiguous spec (two valid interpretations) | |
| - Pre-existing failures in unrelated code | |
| - Second attempt also fails | |
| - You don't understand WHY it's failing | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 127-127: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/executing-plans/SKILL.md` around lines 127 - 140, The fenced code
block that begins with "Task fails" in SKILL.md is missing a language specifier
(MD040); update that code fence to include the language identifier "text" (e.g.,
change the opening ``` to ```text) so the ASCII-art decision tree is properly
marked as text and satisfies the markdownlint rule.
| ``` | ||
| ## FAILED: Task N — [title] | ||
| - Reason: [brief] | ||
| - What was tried: | ||
| - Attempt 1: [approach] → [outcome] | ||
| - Attempt 2: [approach] → [outcome] | ||
| - Blocker: [missing dep / ambiguous spec / infra / logic error] | ||
| - Recommended next step: [fix X / clarify Y / skip and continue] | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to the fenced code block.
The markdown linter flags this code block for missing a language specifier. Since this is a markdown template format, add markdown or text as the language identifier.
📝 Proposed fix
-```
+```markdown
## FAILED: Task N — [title]
- Reason: [brief]
- What was tried:As per coding guidelines, markdownlint-cli2 rule MD040 requires language specifiers for fenced code blocks.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 163-163: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/executing-plans/SKILL.md` around lines 163 - 171, The fenced code
block for the "## FAILED: Task N — [title]" template is missing a language
specifier which trips markdownlint MD040; update the opening fence to include a
language identifier (e.g., change the leading ``` to ```markdown or ```text) and
keep the closing ``` as-is so the block is properly annotated; locate the block
around the "## FAILED: Task N — [title]" template in SKILL.md to make this
change.
|
Thanks for this! v5.0.0 just shipped and substantially reworked executing-plans (which now uses continuous execution instead of batch-and-stop), so this PR is no longer a clean apply against main. Closing as stale — if you'd like to revisit the idea against the current codebase, we'd welcome a fresh PR. — Claude (claude-opus-4-6, Claude Code 2.1.71, session 64908a66-5c98-4c79-b0f7-1aa7eac2dcb0) |
2 participants
Adds a Failure Handling section to executing-plans: signal protocol (COMPLETE/PASSED/FAILED), max iterations, non-zero exits, partial success reporting, retry vs escalate decision tree, and prompt rewriting guidance referencing Ralph Loop V2 + Elvis Sun article.\n\nRefs: BAG-85