t1139: Add supervisor DB→TODO.md cancelled state consistency check (Phase 0.5c)#1735
t1139: Add supervisor DB→TODO.md cancelled state consistency check (Phase 0.5c)#1735marcusquinn merged 2 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ 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. WalkthroughThis PR introduces Phase 0.5c for cancelled task synchronization from the supervisor DB to TODO.md, adds the Changes
Sequence Diagram(s)sequenceDiagram
participant Pulse as Phase 0.5c<br/>(pulse.sh)
participant DB as Supervisor DB
participant FS as Filesystem<br/>(TODO.md)
participant Sync as todo-sync.sh<br/>(update_todo_on_cancelled)
participant Git as Git Repo
Pulse->>DB: Query repos with cancelled tasks
DB-->>Pulse: Return cancelled task list
Pulse->>FS: Check if TODO.md exists for repo
alt TODO.md Found
Pulse->>FS: Scan for open task entry
FS-->>Pulse: Return task line + indentation
Pulse->>Sync: Call update_todo_on_cancelled(task_id, reason)
Sync->>FS: Locate/append Notes line with CANCELLED annotation
Sync->>Sync: Sanitize reason, handle duplicates
FS-->>Sync: Updated TODO.md
Sync->>Git: Commit and push changes
Git-->>Sync: Success confirmation
Sync-->>Pulse: Return status (synced)
else TODO.md Not Found
Pulse-->>Pulse: Skip repo, log missing file
end
Pulse->>Pulse: Track sync count, report drift or success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
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)
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 |
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical inconsistency where tasks cancelled in the database were not accurately reflected in their corresponding TODO.md entries, leading to inefficient supervisor processing. By introducing a new supervisor phase (0.5c) and enhancing existing task management commands, the system now proactively synchronizes the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable consistency check for cancelled tasks between the database and TODO.md, which will help prevent the supervisor from wasting cycles. The new update_todo_on_cancelled function is well-implemented and adheres to the repository's style guide. I've identified a couple of minor areas for improvement related to code duplication and a small bug in a generated string, both of which should be straightforward to address. Overall, this is a solid addition that enhances the supervisor's robustness.
| sed_inplace "${next_line_num}s/$/${append_text}/" "$todo_file" | ||
| else | ||
| # Insert a new Notes line after the task | ||
| local notes_line="${indent} - Notes: CANCELLED by supervisor: ${safe_reason}" |
There was a problem hiding this comment.
The hardcoded string by supervisor: in this line is redundant. The reason variable, which is interpolated as ${safe_reason}, already defaults to "cancelled by supervisor". This leads to a note like Notes: CANCELLED by supervisor: cancelled by supervisor. The string should be ${indent} - Notes: CANCELLED: ${safe_reason} to be consistent with the if branch on line 832 and the similar update_todo_on_blocked function.
| local notes_line="${indent} - Notes: CANCELLED by supervisor: ${safe_reason}" | |
| local notes_line="${indent} - Notes: CANCELLED: ${safe_reason}" |
There was a problem hiding this comment.
Fixed in a509c82. Removed the redundant 'by supervisor:' literal — the else branch now uses CANCELLED: ${safe_reason} consistent with the if branch append path and the update_todo_on_blocked pattern.
| cancelled) | ||
| local terror | ||
| terror=$(db "$SUPERVISOR_DB" "SELECT error FROM tasks WHERE id = '$escaped_id';") | ||
| update_todo_on_cancelled "$task_id" "${terror:-cancelled by supervisor}" | ||
| ;; |
There was a problem hiding this comment.
This new cancelled case duplicates the database query from the failed case. To improve maintainability and avoid redundancy, consider refactoring to combine the logic for both failed and cancelled statuses. You could use a single case branch failed | cancelled) and then use an if statement inside to call the appropriate update function. This would avoid calling the database twice for similar logic.
References
- In shell scripts, extract repeated logic into an internal helper function to improve maintainability. This applies even for standalone scripts where external
sourcedependencies are avoided.
There was a problem hiding this comment.
Fixed in a509c82. Combined failed and cancelled into a single failed | cancelled) branch with one DB query, using an if inside to dispatch to the appropriate update function. Eliminates the duplicate query and keeps the logic co-located.
marcusquinn
force-pushed
the
feature/t1139
branch
from
58283ba to
62408a3
Compare
…139) - Add update_todo_on_cancelled() to todo-sync.sh: annotates open TODO.md tasks with a Notes line when DB status is cancelled (does not mark [x]) - Add Phase 0.5c to pulse cycle: runs every pulse, finds DB-cancelled tasks still open in TODO.md, calls update_todo_on_cancelled for each - Extend cmd_reconcile_db_todo Gap 1 to include 'cancelled' state alongside failed/blocked — Phase 7b now annotates cancelled tasks too - Extend cmd_update_todo to handle 'cancelled' state via update_todo_on_cancelled - Zero ShellCheck violations Eliminates the persistent inconsistency where supervisor-cancelled tasks (Phase 0.5 dedup, Phase 3b2 obsolete PR, etc.) remain open in TODO.md, causing the supervisor to waste reasoning cycles on dead tasks.
marcusquinn
force-pushed
the
feature/t1139
branch
from
62408a3 to
3e87f2d
Compare
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Wed Feb 18 18:36:20 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Wed Feb 18 18:50:00 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
…ith dead workers (t1145) Phase 0.7 (pulse.sh): when a stale 'evaluating' task has a pr_url, route to 'pr_review' instead of re-queuing — the work is done, only the evaluation process died. Previously, tasks with completed PRs were wastefully re-run. supervisor-helper.sh: add 'evaluating:pr_review' to VALID_TRANSITIONS to support the new Phase 0.7 routing path. cleanup.sh: include 'evaluating' in stale PID file detection alongside 'running' and 'dispatched' for consistent cleanup. Manual recovery applied to stale tasks found at time of fix: - t1138: evaluating (dead PID 63748) + PR #1736 → pr_review - t1139: running (dead PID 19930) + PR #1735 → pr_review (via evaluating) - t1146: running (dead PID 12745) no PR → queued (retry) - t1148: running (dead PID 14125) no PR → queued (retry) - t1149: running (dead PID 15457) no PR → queued (retry) DB now consistent: 1 running (t1145/self), 0 evaluating, 0 stale entries. Supersedes t1140 and t1132.
… with dead workers (#1771) * chore: regenerate MODELS.md leaderboard (t1012, t1129) * fix: resolve supervisor DB inconsistency — stale running/evaluating with dead workers (t1145) Phase 0.7 (pulse.sh): when a stale 'evaluating' task has a pr_url, route to 'pr_review' instead of re-queuing — the work is done, only the evaluation process died. Previously, tasks with completed PRs were wastefully re-run. supervisor-helper.sh: add 'evaluating:pr_review' to VALID_TRANSITIONS to support the new Phase 0.7 routing path. cleanup.sh: include 'evaluating' in stale PID file detection alongside 'running' and 'dispatched' for consistent cleanup. Manual recovery applied to stale tasks found at time of fix: - t1138: evaluating (dead PID 63748) + PR #1736 → pr_review - t1139: running (dead PID 19930) + PR #1735 → pr_review (via evaluating) - t1146: running (dead PID 12745) no PR → queued (retry) - t1148: running (dead PID 14125) no PR → queued (retry) - t1149: running (dead PID 15457) no PR → queued (retry) DB now consistent: 1 running (t1145/self), 0 evaluating, 0 stale entries. Supersedes t1140 and t1132.
1 participant
Add Phase 0.5c to the supervisor pulse cycle that proactively syncs DB-cancelled task states to TODO.md.
Changes
supervisor/todo-sync.sh: Newupdate_todo_on_cancelled()function — annotates open TODO.md tasks with a Notes line when DB status iscancelled(does NOT mark[x]— cancelled ≠ done)supervisor/pulse.sh: New Phase 0.5c — runs every pulse, queries DB for cancelled tasks still open in TODO.md, callsupdate_todo_on_cancelledfor eachcmd_reconcile_db_todoGap 1: Extended to includecancelledalongsidefailed/blocked— Phase 7b now annotates cancelled tasks toocmd_update_todo: Extended to handlecancelledstateProblem solved
When the supervisor cancels tasks (Phase 0.5 dedup, Phase 3b2 obsolete PR, etc.), the DB is updated but TODO.md still shows
[ ]. This creates a persistent inconsistency where cancelled tasks appear dispatchable, causing the supervisor to waste reasoning cycles on dead tasks.Design decisions
[x]— cancelled tasks are not completed deliverablesCANCELLED:Ref #1710
Summary by CodeRabbit
New Features
Chores
Documentation