feat(loop): GitHub issue lifecycle (#73) + optional fix_plan sections (#239)#294
Conversation
…#239) Issue #73 — GitHub issue lifecycle management (lib/github_lifecycle.sh): - New `ralph --github-issue <ref>` tracking with opt-in lifecycle actions: progress comments (--comment-progress/--comment-interval), completion summary (--close-summary), PR creation linked with "Closes #N" (--create-pr/--link-issue/--draft-pr), grouped follow-up issue from TODO/FIXME markers (--create-followups/--followup-label), and issue close with labels (--auto-close/--add-label). - Uses the `gh` CLI exclusively (not raw REST/GITHUB_TOKEN); state at .ralph/.github_lifecycle_state (atomic temp+mv). Every gh op degrades gracefully — a permission failure is logged and the loop continues. - Wired into ralph_loop.sh: init at startup, progress hook in the loop body, completion workflow on graceful exit. Config via flags or .ralphrc. Issue #239 — Optional/Future sections in fix_plan.md: - `_count_blocking_unchecked()` (awk, section-aware) excludes unchecked items under OPTIONAL_SECTIONS (default "Optional,Future,Future Enhancements,Nice to Have", case-insensitive, configurable) from the plan-complete exit check. Resolves the deadlock where Claude skips low-priority items while Ralph waits. Backward compatible when no optional sections are present. Tests: +29 unit (test_github_lifecycle.bats), +9 (test_exit_detection.bats), +3 (test_cli_modern.bats). Full suite green: 769 unit + 250 integration + 13 e2e. Docs: CLAUDE.md, README.md, templates/fix_plan.md.
|
Warning Review limit reached
More reviews will be available in 8 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR implements GitHub issue lifecycle automation (Issue ChangesGitHub Issue Lifecycle & Optional Sections Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
Code Review — PR 294: GitHub issue lifecycle + optional fix_plan sectionsThis is a well-executed pair of features. Both follow established Ralph conventions (atomic state via temp+mv, graceful degradation, Issue 239 —
|
- P1: never push the default/protected branch when --create-pr is set. Detect the remote default branch (and main/master) and skip PR creation with a warning if Ralph is on it — pushing it would land work directly on the base instead of in a PR. - P2: scan follow-up TODOs across the whole run, not just the last commit. Capture the HEAD SHA at lifecycle init (lifecycle.start_sha) and diff the working tree from it so TODO/FIXME markers in earlier development commits are included. Tests: +2 (default-branch PR skip; multi-commit TODO scan). 771 unit pass.
- Log an INFO line before the best-effort git push so the push is visible in loop output. - Drop the undocumented GITHUB_PR_TITLE override; PR title comes from the tracked issue title (falling back to a generated title). Updates the PR-title test accordingly. - Add a clarifying comment on the intentional double-close when --auto-close and --link-issue are combined (GitHub treats the post-merge close as a no-op).
Follow-up Review — PR 294 (commit 238c044)All three actionable items from my previous review have been addressed. Here's the rundown: Items resolved1. git push logging — Fixed. 2. 3. Double-close comment — Added. The comment near step 4 of One remaining minor item (non-blocking)The issue raised about The test duplication for SummaryThe implementation is clean, all critical review items are resolved, the test suite is comprehensive (29 lifecycle unit tests, 9 exit-detection tests for optional sections), and the graceful-degradation contract is solid throughout. This is ready to merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ralph_loop.sh (1)
461-505:⚠️ Potential issue | 🟠 Major | ⚡ Quick winForward GitHub lifecycle flags into the tmux child command.
In
--monitormode, the script launches a freshralphprocess viaralph_cmd, but the new lifecycle CLI flags parsed at Line 2799+ are never appended there. So commands likeralph --monitor --github-issue 69 --auto-closesilently lose lifecycle behavior.Suggested patch
@@ # Forward --backup if enabled (Issue `#23`) if [[ "$ENABLE_BACKUP" == "true" ]]; then ralph_cmd="$ralph_cmd --backup" fi + + # Forward GitHub issue lifecycle flags (Issue `#73`) + if [[ -n "${GITHUB_ISSUE:-}" ]]; then + ralph_cmd="$ralph_cmd --github-issue '$GITHUB_ISSUE'" + fi + if [[ "${COMMENT_PROGRESS:-false}" == "true" ]]; then + ralph_cmd="$ralph_cmd --comment-progress" + fi + if [[ -n "${COMMENT_INTERVAL:-}" && "${COMMENT_INTERVAL}" != "5" ]]; then + ralph_cmd="$ralph_cmd --comment-interval $COMMENT_INTERVAL" + fi + [[ "${AUTO_CLOSE:-false}" == "true" ]] && ralph_cmd="$ralph_cmd --auto-close" + [[ "${CLOSE_SUMMARY:-false}" == "true" ]] && ralph_cmd="$ralph_cmd --close-summary" + [[ "${CREATE_PR:-false}" == "true" ]] && ralph_cmd="$ralph_cmd --create-pr" + [[ "${LINK_ISSUE:-false}" == "true" ]] && ralph_cmd="$ralph_cmd --link-issue" + [[ "${DRAFT_PR:-false}" == "true" ]] && ralph_cmd="$ralph_cmd --draft-pr" + [[ "${CREATE_FOLLOWUPS:-false}" == "true" ]] && ralph_cmd="$ralph_cmd --create-followups" + if [[ -n "${FOLLOWUP_LABEL:-}" && "${FOLLOWUP_LABEL}" != "tech-debt" ]]; then + ralph_cmd="$ralph_cmd --followup-label '$FOLLOWUP_LABEL'" + fi + if [[ -n "${ADD_COMPLETION_LABELS:-}" ]]; then + local _lbl + IFS=',' read -ra _labels <<< "$ADD_COMPLETION_LABELS" + for _lbl in "${_labels[@]}"; do + _lbl="$(echo "$_lbl" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')" + [[ -n "$_lbl" ]] && ralph_cmd="$ralph_cmd --add-label '$_lbl'" + done + fiAlso applies to: 2799-2864
🤖 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 `@ralph_loop.sh` around lines 461 - 505, The ralph_cmd builder ignores the GitHub lifecycle CLI flags, so when launching the tmux child the lifecycle options (e.g., --github-issue, --auto-close and related GitHub flags) are dropped; update the ralph_cmd construction (the block that appends flags to ralph_cmd) to forward the lifecycle variables parsed earlier by adding conditional appends for the GitHub lifecycle variables (check the parsed variables such as GITHUB_ISSUE, GITHUB_AUTO_CLOSE (or AUTO_CLOSE), GITHUB_REPO, GITHUB_OWNER, GITHUB_TOKEN, GITHUB_LABELS, etc.) in the same style as the existing flag forwards (use --github-issue "$GITHUB_ISSUE", --auto-close when true, and the corresponding --github-repo/--github-owner/--github-token/--github-labels when non-default) so the tmux-launched ralph inherits lifecycle behavior.
🧹 Nitpick comments (1)
lib/github_lifecycle.sh (1)
288-300: ⚡ Quick winInclude file paths in
scan_for_todosoutput for actionable follow-ups.Current extraction emits marker text only, so identical TODO strings from different files collapse in
sort -u, and generated follow-up issues lose location context.Refactor sketch
scan_for_todos() { local diff diff=$(git diff -U0 HEAD 2>/dev/null; git diff -U0 --cached 2>/dev/null; git diff -U0 HEAD~1 HEAD 2>/dev/null) - printf '%s\n' "$diff" \ - | grep -E "^\+" \ - | grep -vE "^\+\+\+" \ - | grep -oiE "(TODO|FIXME|HACK|XXX):?.*" \ - | sed 's/[[:space:]]*$//' \ - | sort -u + printf '%s\n' "$diff" | awk ' + /^diff --git / { file=$4; sub("^b/","",file); next } + /^\+\+\+ / { file=$2; sub("^b/","",file); next } + /^\+/ && $0 !~ /^\+\+\+/ { + line = substr($0, 2) + upper = toupper(line) + if (match(upper, /(TODO|FIXME|HACK|XXX):?.*/)) { + marker = substr(line, RSTART, RLENGTH) + sub(/[[:space:]]+$/, "", marker) + key = file ": " marker + if (!seen[key]++) print key + } + } + ' | sort -u }🤖 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 `@lib/github_lifecycle.sh` around lines 288 - 300, The scan_for_todos function currently extracts marker text from the unified git diff stored in the local diff variable, but loses file context so identical TODOs across files collapse; update scan_for_todos to parse the unified-diff headers (e.g., lines starting with "+++ " or "diff --git") to capture the current filename while processing added lines, and emit entries that prefix each match with the file path (and optional line number) such as "path: marker" or "path:line: marker" instead of just the marker text; implement this by replacing the grep/sed pipeline with an awk (or similar) script that tracks filename changes, matches "^\+" added lines (excluding "^\+\+\+"), extracts the TODO/FIXME/HACK/XXX token and prints the filename plus matched text, then sort -u so deduplication is per file.
🤖 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 `@lib/github_lifecycle.sh`:
- Around line 187-211: The current initialization writes directly to
$GITHUB_LIFECYCLE_STATE_FILE which can leave a truncated/corrupt file if
interrupted; change the write to generate the jq output into a temporary file in
the same directory (e.g., tmp="${GITHUB_LIFECYCLE_STATE_FILE}.tmp.$$" or mktemp
in that directory), check jq succeeded, then atomically mv the temp file to
$GITHUB_LIFECYCLE_STATE_FILE; ensure you still call _lifecycle_log on failure
and return 1 on error — update the block around get_iso_timestamp, the jq
invocation that writes the state file, and the error handling that currently
calls _lifecycle_log so the new flow writes to temp then mv (and cleans up temp
on failure) to make initialization atomic.
---
Outside diff comments:
In `@ralph_loop.sh`:
- Around line 461-505: The ralph_cmd builder ignores the GitHub lifecycle CLI
flags, so when launching the tmux child the lifecycle options (e.g.,
--github-issue, --auto-close and related GitHub flags) are dropped; update the
ralph_cmd construction (the block that appends flags to ralph_cmd) to forward
the lifecycle variables parsed earlier by adding conditional appends for the
GitHub lifecycle variables (check the parsed variables such as GITHUB_ISSUE,
GITHUB_AUTO_CLOSE (or AUTO_CLOSE), GITHUB_REPO, GITHUB_OWNER, GITHUB_TOKEN,
GITHUB_LABELS, etc.) in the same style as the existing flag forwards (use
--github-issue "$GITHUB_ISSUE", --auto-close when true, and the corresponding
--github-repo/--github-owner/--github-token/--github-labels when non-default) so
the tmux-launched ralph inherits lifecycle behavior.
---
Nitpick comments:
In `@lib/github_lifecycle.sh`:
- Around line 288-300: The scan_for_todos function currently extracts marker
text from the unified git diff stored in the local diff variable, but loses file
context so identical TODOs across files collapse; update scan_for_todos to parse
the unified-diff headers (e.g., lines starting with "+++ " or "diff --git") to
capture the current filename while processing added lines, and emit entries that
prefix each match with the file path (and optional line number) such as "path:
marker" or "path:line: marker" instead of just the marker text; implement this
by replacing the grep/sed pipeline with an awk (or similar) script that tracks
filename changes, matches "^\+" added lines (excluding "^\+\+\+"), extracts the
TODO/FIXME/HACK/XXX token and prints the filename plus matched text, then sort
-u so deduplication is per file.
🪄 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
Run ID: 3e324231-e935-4bc2-936b-bc3afb1959cc
📒 Files selected for processing (8)
CLAUDE.mdREADME.mdlib/github_lifecycle.shralph_loop.shtemplates/fix_plan.mdtests/unit/test_cli_modern.batstests/unit/test_exit_detection.batstests/unit/test_github_lifecycle.bats
- Major: forward GitHub lifecycle flags into the tmux child command in --monitor mode. Previously `ralph --monitor --github-issue 69 --auto-close` silently dropped all lifecycle behavior because the relaunched child command omitted the new flags. The inlined tmux builder in test_tmux_integration.bats is kept in sync, with two new forwarding tests. - Major: make init_github_lifecycle's state write atomic (temp file + mv) so an interrupted init can't leave a truncated/corrupt state file. - Nitpick: prefix scan_for_todos output with the file each marker lives in (POSIX awk, case-insensitive), matching the issue's follow-up template intent. Tests: +2 tmux forwarding, scan_for_todos file-path assertion. 771 unit + 252 integration pass.
Review feedback addressedcodex (cross-family, primary):
CodeRabbit:
claude-review (advisory):
Full local suite green after all fixes: 771 unit + 252 integration + 13 e2e = 1036, 0 failures. |
Code Review Update — PR 294Following up on my earlier review. After re-reading the diff more carefully, I need to correct two points I raised: Previously raised — already present in the original submission:
Still open from the previous review: 1. Duplicate The awk function is copied verbatim into 2.
Overall: The implementation is solid — atomic state management, graceful degradation on every |
frankbria
deleted the
feature/issue-73-lifecycle-and-239-optional-sections
branch
1 participant
Implements #73 (GitHub issue lifecycle management) and #239 (Optional/Future section support in
fix_plan.md) in one PR.#73 — GitHub issue lifecycle management
New
ralph --github-issue <ref>tracking, backed bylib/github_lifecycle.sh. All actions are opt-in and require--github-issue(a number,#N,owner/repo#N, or issue URL):--comment-progress/--comment-interval N--close-summary--create-pr/--link-issue/--draft-prCloses #N, optionally draft--create-followups/--followup-label--auto-close/--add-labelAdapted from the Traycer plan on the issue, which assumed a greenfield repo using raw REST +
GITHUB_TOKEN. This repo uses theghCLI everywhere and the.ralph/subfolder, so the implementation usesgh issue comment/close/edit,gh pr create,gh issue create, with state at.ralph/.github_lifecycle_state(atomic temp+mv). Dropped as YAGNI: multi-repo/cross-repo, custom rate-limit backoff (gh handles its own), GITHUB_TOKEN scope validation.Graceful degradation: every gh operation logs and returns non-zero on failure (e.g. missing permission) but the orchestration helpers always return 0 — a lifecycle hiccup never crashes the development loop.
#239 — Optional/Future sections in fix_plan.md
_count_blocking_unchecked()(awk, section-aware) makes the "all checkboxes complete" exit ignore unchecked- [ ]items under sections whose heading matchesOPTIONAL_SECTIONS(defaultOptional,Future,Future Enhancements,Nice to Have; case-insensitive; configurable in.ralphrc). Optional context persists into deeper subsections and closes at the next same-or-higher-level heading. Resolves the deadlock where Claude treats low-priority items as skippable while Ralph keeps looping for them. Backward compatible — with no optional sections present, behavior is identical to the prior full-file count.Tests
tests/unit/test_github_lifecycle.bats(29) — reference parsing, gh wrappers (mocked), generators, state, interval gating, completion-workflow ordering, graceful degradationtests/unit/test_exit_detection.bats(+9) — optional-section exclusion, nesting, case-insensitivity, configurability, backward compat (real + inlined copy kept in sync)tests/unit/test_cli_modern.bats(+3) — flag acceptance,--comment-intervalvalidation, help textFull suite green: 769 unit + 250 integration + 13 e2e = 1032, 0 failures.
Demo evidence
Both features were demoed against the real functions (gh mocked to capture exact commands). #239:
should_exit_gracefullyreturnsplan_completewith optional items present (counted 1/1, not 3/3) and""when a required section has unchecked items. #73: full lifecycle issued, in order,gh issue comment→gh pr create … --draft(body containsCloses #73) →gh issue create … --label tech-debt→gh issue edit … --add-label→gh issue close; a 403 from gh left the loop running (returned 0).Known limitations
--create-prbest-effort pushes the current branch; PR creation needs a pushed branch and push permission.Closes #73
Closes #239
Summary by CodeRabbit
Release Notes
New Features
Documentation