fix: register blocked tasks in DB during auto-pickup#2239
Merged
marcusquinn merged 1 commit intomainfrom Feb 24, 2026
Merged
Conversation
Previously, cmd_auto_pickup() skipped blocked tasks entirely (blocked-by: dependencies and -needed tags), making them invisible to the supervisor. Tasks with unresolved blockers were never registered in the DB, so the supervisor couldn't dispatch them when blockers resolved — they had to wait for a subsequent auto-pickup run. Changes: - Add _register_blocked_task() helper in cron.sh that creates a task via cmd_add and immediately sets status='blocked' with blocker reason - At all 3 strategy locations in cmd_auto_pickup(), register blocked tasks in DB instead of skipping (strategies 1, 2, and 4) - Also register -needed blocker tag tasks as blocked (strategies 1, 2) - Update auto_unblock_resolved_tasks() in todo-sync.sh to transition DB status from 'blocked' to 'queued' when all blockers resolve This creates a complete lifecycle: blocked tasks are registered on first scan, then auto-dispatched the instant their blockers complete.
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 significantly improves the handling of blocked tasks within the supervisor system. By ensuring that tasks with dependencies or specific blocking tags are registered in the database immediately, even when blocked, the system gains better visibility and responsiveness. Once blockers are resolved, these tasks are automatically transitioned to a 'queued' state, allowing for their prompt dispatch and eliminating delays previously caused by waiting for subsequent auto-pickup runs. Highlights
Changelog
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
|
Contributor
WalkthroughThese changes add database-backed visibility for blocked tasks in the supervisor system. Instead of implicitly skipping tasks with unresolved blockers, they are now explicitly registered in the supervisor database as blocked with detailed blocker reasons. When blockers resolve, tasks automatically transition to queued state in the database, enabling immediate dispatch. Changes
Sequence DiagramsequenceDiagram
participant Cron as Cron Script
participant DB as Supervisor DB
participant TODO as TODO.md
rect rgba(100, 150, 200, 0.5)
Note over Cron,DB: Task Blocking Phase
Cron->>Cron: Detect unresolved blocker
Cron->>DB: Check if task exists in DB
alt Task exists & non-terminal
DB-->>Cron: Task found
Cron->>DB: UPDATE status=blocked, error=blocker_reason
else Task not in DB
DB-->>Cron: Task not found
Cron->>DB: INSERT task with status=blocked
end
end
rect rgba(200, 150, 100, 0.5)
Note over Cron,TODO: Task Unblocking Phase
Cron->>TODO: Detect blocker resolved
Cron->>TODO: Remove blocked-by marker
Cron->>DB: Query task status
alt Status is blocked
DB-->>Cron: blocked
Cron->>DB: UPDATE status=queued, clear error, update timestamp
DB-->>Cron: Transition logged
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
🔍 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: Tue Feb 24 21:16:57 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![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 fix for handling blocked tasks in cmd_auto_pickup(). Instead of skipping them, they are now registered in the supervisor database with a 'blocked' status. This ensures they are visible and can be dispatched immediately once their blockers are resolved. The changes in cron.sh and todo-sync.sh are logical and well-implemented to achieve this. My review focuses on improving error handling and consistency. Specifically, I've suggested removing stderr suppression (2>/dev/null) in several places to align with repository guidelines, which will make debugging easier. I also pointed out an inconsistency in a new function's return logic compared to its documentation and suggested a fix.
Comment on lines
+470
to
+498
| _register_blocked_task() { | ||
| local task_id="$1" | ||
| local repo="$2" | ||
| local blocker_reason="$3" | ||
|
|
||
| # Check if already in supervisor DB | ||
| local existing | ||
| existing=$(db "$SUPERVISOR_DB" "SELECT status FROM tasks WHERE id = '$(sql_escape "$task_id")';" 2>/dev/null || true) | ||
|
|
||
| if [[ -n "$existing" ]]; then | ||
| # Already tracked — update to blocked if not in a terminal state | ||
| if [[ "$existing" == "complete" || "$existing" == "cancelled" || "$existing" == "verified" ]]; then | ||
| return 0 | ||
| fi | ||
| if [[ "$existing" != "blocked" ]]; then | ||
| db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='Blocked by: ${blocker_reason}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true | ||
| log_info " $task_id: updated to blocked (was: $existing, blocked by: $blocker_reason)" | ||
| fi | ||
| return 0 | ||
| fi | ||
|
|
||
| # Not in DB — add it first, then mark blocked | ||
| if cmd_add "$task_id" --repo "$repo"; then | ||
| db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='Blocked by: ${blocker_reason}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true | ||
| log_info " $task_id: registered as blocked (blocked by: $blocker_reason)" | ||
| fi | ||
|
|
||
| return 0 | ||
| } |
There was a problem hiding this comment.
This new function has a couple of areas for improvement:
- Incorrect failure handling: The function documentation at line 468 states it returns 1 on failure, but the current implementation always returns 0. If
cmd_addfails, the function silently continues and returns success, which can hide problems and makes the function's behavior inconsistent with its documentation. - Error suppression: The
dbcalls use2>/dev/nullto suppress stderr. According to the repository's general rules, this should be avoided to ensure that underlying issues like database corruption or syntax errors are visible for debugging. The|| trueis sufficient to prevent script exit withset -e.
The suggested change below addresses both points by correctly handling the failure of cmd_add and removing the stderr suppression.
_register_blocked_task() {
local task_id="$1"
local repo="$2"
local blocker_reason="$3"
# Check if already in supervisor DB
local existing
existing=$(db "$SUPERVISOR_DB" "SELECT status FROM tasks WHERE id = '$(sql_escape "$task_id")';" || true)
if [[ -n "$existing" ]]; then
# Already tracked — update to blocked if not in a terminal state
if [[ "$existing" == "complete" || "$existing" == "cancelled" || "$existing" == "verified" ]]; then
return 0
fi
if [[ "$existing" != "blocked" ]]; then
db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='Blocked by: ${blocker_reason}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" || true
log_info " $task_id: updated to blocked (was: $existing, blocked by: $blocker_reason)"
fi
return 0
fi
# Not in DB — add it first, then mark blocked
if ! cmd_add "$task_id" --repo "$repo"; then
log_error " $task_id: failed to add to supervisor DB."
return 1
fi
db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='Blocked by: ${blocker_reason}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" || true
log_info " $task_id: registered as blocked (blocked by: $blocker_reason)"
return 0
}References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
Comment on lines
+1263
to
+1271
| # Transition DB status from blocked to queued so dispatch picks it up | ||
| if [[ -n "${SUPERVISOR_DB:-}" && -f "${SUPERVISOR_DB}" ]]; then | ||
| local db_status | ||
| db_status=$(db "$SUPERVISOR_DB" "SELECT status FROM tasks WHERE id = '$(sql_escape "$task_id")' LIMIT 1;" 2>/dev/null || echo "") | ||
| if [[ "$db_status" == "blocked" ]]; then | ||
| db "$SUPERVISOR_DB" "UPDATE tasks SET status='queued', error=NULL, updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true | ||
| log_info " auto-unblock: $task_id — DB status transitioned from blocked to queued" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
The db calls on lines 1266 and 1268 use 2>/dev/null to suppress stderr. According to the repository's general rules, this should be avoided to ensure that underlying issues like database corruption or syntax errors are visible for debugging. The || echo "" and || true constructs are sufficient to handle expected failures without exiting the script.
Suggested change
| # Transition DB status from blocked to queued so dispatch picks it up | |
| if [[ -n "${SUPERVISOR_DB:-}" && -f "${SUPERVISOR_DB}" ]]; then | |
| local db_status | |
| db_status=$(db "$SUPERVISOR_DB" "SELECT status FROM tasks WHERE id = '$(sql_escape "$task_id")' LIMIT 1;" 2>/dev/null || echo "") | |
| if [[ "$db_status" == "blocked" ]]; then | |
| db "$SUPERVISOR_DB" "UPDATE tasks SET status='queued', error=NULL, updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true | |
| log_info " auto-unblock: $task_id — DB status transitioned from blocked to queued" | |
| fi | |
| fi | |
| # Transition DB status from blocked to queued so dispatch picks it up | |
| if [[ -n "${SUPERVISOR_DB:-}" && -f "${SUPERVISOR_DB}" ]]; then | |
| local db_status | |
| db_status=$(db "$SUPERVISOR_DB" "SELECT status FROM tasks WHERE id = '$(sql_escape "$task_id")' LIMIT 1;" || echo "") | |
| if [[ "$db_status" == "blocked" ]]; then | |
| db "$SUPERVISOR_DB" "UPDATE tasks SET status='queued', error=NULL, updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" || true | |
| log_info " auto-unblock: $task_id — DB status transitioned from blocked to queued" | |
| fi | |
| fi |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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)
.agents/scripts/supervisor/cron.sh (1)
710-725:⚠️ Potential issue | 🟠 MajorAddress ShellCheck warnings in cron.sh before merging.
ShellCheck found 4 info-level violations (SC2016) in
.agents/scripts/supervisor/cron.shat lines 1199–1203. These echo statements use single quotes where expressions should expand; use double quotes instead:Line 1199: echo " children=$(pgrep -P $$ 2>/dev/null || true)" Line 1200: echo " if [[ -n "$children" ]]; then" Line 1201: echo " kill -TERM $children 2>/dev/null || true" Line 1203: echo " kill -9 $children 2>/dev/null || true"
.agents/scripts/supervisor/todo-sync.shpasses ShellCheck cleanly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/supervisor/cron.sh around lines 710 - 725, ShellCheck flagged SC2016: several echo statements emit literal dollar-sign expressions because they use single quotes; locate the echo lines that print the shell snippet (the ones emitting children=$(pgrep -P $$ ...), the if [[ -n "$children" ]] line, and the kill lines) and change their quotes to double quotes so variables expand (e.g., echo " children=$(pgrep -P $$ 2>/dev/null || true)"), making sure to escape any inner double quotes or dollar signs as needed; re-run ShellCheck to confirm the SC2016 warnings are resolved.
🧹 Nitpick comments (1)
.agents/scripts/supervisor/todo-sync.sh (1)
1263-1271: Prefer a state-log-aware transition for blocked → queued.Direct SQL updates bypass
state_log/transition hooks used elsewhere. Consider usingcmd_transitionor at least inserting a state_log entry for audit consistency.📝 Suggested state_log addition
if [[ "$db_status" == "blocked" ]]; then - db "$SUPERVISOR_DB" "UPDATE tasks SET status='queued', error=NULL, updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true + local escaped_task_id + escaped_task_id=$(sql_escape "$task_id") + db "$SUPERVISOR_DB" "UPDATE tasks SET status='queued', error=NULL, updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='${escaped_task_id}';" 2>/dev/null || true + db "$SUPERVISOR_DB" "INSERT INTO state_log (task_id, from_state, to_state, timestamp, reason) + VALUES ('${escaped_task_id}', 'blocked', 'queued', + strftime('%Y-%m-%dT%H:%M:%SZ','now'), + 'auto_unblock_resolved_tasks: blockers resolved');" 2>/dev/null || true log_info " auto-unblock: $task_id — DB status transitioned from blocked to queued" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/supervisor/todo-sync.sh around lines 1263 - 1271, The current auto-unblock block uses a direct SQL UPDATE via db "$SUPERVISOR_DB" which bypasses the application's state transition hooks and state_log/audit; replace the raw UPDATE with the existing transition mechanism (call cmd_transition or the helper that records state_log entries) so transitions for task_id go through the same code path that appends to state_log and triggers hooks, or if cmd_transition is not available here, after the UPDATE insert a state_log row for task_id documenting the blocked→queued change and include error=NULL and updated_at timestamp; locate the block around db "$SUPERVISOR_DB" "SELECT status..." and the UPDATE invocation and switch to calling cmd_transition (or adding the state_log insert) and ensure sql_escape "$task_id" is used for the id and any command failures are handled similarly to the current 2>/dev/null || true behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/scripts/supervisor/cron.sh:
- Around line 454-498: In _register_blocked_task, blocker_reason is interpolated
directly into SQL UPDATE statements (the two db calls that set error='Blocked
by: ${blocker_reason}'), allowing quotes or injection; escape the full error
string via sql_escape before embedding it in the SQL (e.g., compute an
escaped_error or call sql_escape on "Blocked by: ${blocker_reason}" and use that
in both UPDATE queries) so both UPDATEs use the escaped value.
---
Outside diff comments:
In @.agents/scripts/supervisor/cron.sh:
- Around line 710-725: ShellCheck flagged SC2016: several echo statements emit
literal dollar-sign expressions because they use single quotes; locate the echo
lines that print the shell snippet (the ones emitting children=$(pgrep -P $$
...), the if [[ -n "$children" ]] line, and the kill lines) and change their
quotes to double quotes so variables expand (e.g., echo " children=$(pgrep -P
$$ 2>/dev/null || true)"), making sure to escape any inner double quotes or
dollar signs as needed; re-run ShellCheck to confirm the SC2016 warnings are
resolved.
---
Nitpick comments:
In @.agents/scripts/supervisor/todo-sync.sh:
- Around line 1263-1271: The current auto-unblock block uses a direct SQL UPDATE
via db "$SUPERVISOR_DB" which bypasses the application's state transition hooks
and state_log/audit; replace the raw UPDATE with the existing transition
mechanism (call cmd_transition or the helper that records state_log entries) so
transitions for task_id go through the same code path that appends to state_log
and triggers hooks, or if cmd_transition is not available here, after the UPDATE
insert a state_log row for task_id documenting the blocked→queued change and
include error=NULL and updated_at timestamp; locate the block around db
"$SUPERVISOR_DB" "SELECT status..." and the UPDATE invocation and switch to
calling cmd_transition (or adding the state_log insert) and ensure sql_escape
"$task_id" is used for the id and any command failures are handled similarly to
the current 2>/dev/null || true behavior.
Comment on lines
+454
to
+498
| ####################################### | ||
| # Register a task as blocked in the supervisor DB. | ||
| # Creates the task via cmd_add if not already tracked, then | ||
| # sets status='blocked' with the blocker reason in the error field. | ||
| # | ||
| # This ensures blocked tasks are visible to the supervisor and | ||
| # will be dispatched immediately when auto_unblock_resolved_tasks | ||
| # transitions them to 'queued'. | ||
| # | ||
| # Args: | ||
| # $1 - task_id (e.g. t003.2) | ||
| # $2 - repo path | ||
| # $3 - blocker reason (e.g. "t003.1" or "hosting-needed") | ||
| # | ||
| # Returns: 0 on success, 1 on failure | ||
| ####################################### | ||
| _register_blocked_task() { | ||
| local task_id="$1" | ||
| local repo="$2" | ||
| local blocker_reason="$3" | ||
|
|
||
| # Check if already in supervisor DB | ||
| local existing | ||
| existing=$(db "$SUPERVISOR_DB" "SELECT status FROM tasks WHERE id = '$(sql_escape "$task_id")';" 2>/dev/null || true) | ||
|
|
||
| if [[ -n "$existing" ]]; then | ||
| # Already tracked — update to blocked if not in a terminal state | ||
| if [[ "$existing" == "complete" || "$existing" == "cancelled" || "$existing" == "verified" ]]; then | ||
| return 0 | ||
| fi | ||
| if [[ "$existing" != "blocked" ]]; then | ||
| db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='Blocked by: ${blocker_reason}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true | ||
| log_info " $task_id: updated to blocked (was: $existing, blocked by: $blocker_reason)" | ||
| fi | ||
| return 0 | ||
| fi | ||
|
|
||
| # Not in DB — add it first, then mark blocked | ||
| if cmd_add "$task_id" --repo "$repo"; then | ||
| db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='Blocked by: ${blocker_reason}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true | ||
| log_info " $task_id: registered as blocked (blocked by: $blocker_reason)" | ||
| fi | ||
|
|
||
| return 0 | ||
| } |
Contributor
There was a problem hiding this comment.
Escape blocker_reason before SQL updates.
blocker_reason flows from TODO.md into SQL string literals. Since the regex allows any non‑space chars, a single quote can break the query or be abused. Please escape the full error string before persisting it.
🔧 Proposed fix
_register_blocked_task() {
local task_id="$1"
local repo="$2"
local blocker_reason="$3"
+ local escaped_error
+ escaped_error=$(sql_escape "Blocked by: ${blocker_reason}")
@@
- db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='Blocked by: ${blocker_reason}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true
+ db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='${escaped_error}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true
@@
- db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='Blocked by: ${blocker_reason}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true
+ db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='${escaped_error}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true📝 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.
Suggested change
| ####################################### | |
| # Register a task as blocked in the supervisor DB. | |
| # Creates the task via cmd_add if not already tracked, then | |
| # sets status='blocked' with the blocker reason in the error field. | |
| # | |
| # This ensures blocked tasks are visible to the supervisor and | |
| # will be dispatched immediately when auto_unblock_resolved_tasks | |
| # transitions them to 'queued'. | |
| # | |
| # Args: | |
| # $1 - task_id (e.g. t003.2) | |
| # $2 - repo path | |
| # $3 - blocker reason (e.g. "t003.1" or "hosting-needed") | |
| # | |
| # Returns: 0 on success, 1 on failure | |
| ####################################### | |
| _register_blocked_task() { | |
| local task_id="$1" | |
| local repo="$2" | |
| local blocker_reason="$3" | |
| # Check if already in supervisor DB | |
| local existing | |
| existing=$(db "$SUPERVISOR_DB" "SELECT status FROM tasks WHERE id = '$(sql_escape "$task_id")';" 2>/dev/null || true) | |
| if [[ -n "$existing" ]]; then | |
| # Already tracked — update to blocked if not in a terminal state | |
| if [[ "$existing" == "complete" || "$existing" == "cancelled" || "$existing" == "verified" ]]; then | |
| return 0 | |
| fi | |
| if [[ "$existing" != "blocked" ]]; then | |
| db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='Blocked by: ${blocker_reason}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true | |
| log_info " $task_id: updated to blocked (was: $existing, blocked by: $blocker_reason)" | |
| fi | |
| return 0 | |
| fi | |
| # Not in DB — add it first, then mark blocked | |
| if cmd_add "$task_id" --repo "$repo"; then | |
| db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='Blocked by: ${blocker_reason}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true | |
| log_info " $task_id: registered as blocked (blocked by: $blocker_reason)" | |
| fi | |
| return 0 | |
| } | |
| ####################################### | |
| # Register a task as blocked in the supervisor DB. | |
| # Creates the task via cmd_add if not already tracked, then | |
| # sets status='blocked' with the blocker reason in the error field. | |
| # | |
| # This ensures blocked tasks are visible to the supervisor and | |
| # will be dispatched immediately when auto_unblock_resolved_tasks | |
| # transitions them to 'queued'. | |
| # | |
| # Args: | |
| # $1 - task_id (e.g. t003.2) | |
| # $2 - repo path | |
| # $3 - blocker reason (e.g. "t003.1" or "hosting-needed") | |
| # | |
| # Returns: 0 on success, 1 on failure | |
| ####################################### | |
| _register_blocked_task() { | |
| local task_id="$1" | |
| local repo="$2" | |
| local blocker_reason="$3" | |
| local escaped_error | |
| escaped_error=$(sql_escape "Blocked by: ${blocker_reason}") | |
| # Check if already in supervisor DB | |
| local existing | |
| existing=$(db "$SUPERVISOR_DB" "SELECT status FROM tasks WHERE id = '$(sql_escape "$task_id")';" 2>/dev/null || true) | |
| if [[ -n "$existing" ]]; then | |
| # Already tracked — update to blocked if not in a terminal state | |
| if [[ "$existing" == "complete" || "$existing" == "cancelled" || "$existing" == "verified" ]]; then | |
| return 0 | |
| fi | |
| if [[ "$existing" != "blocked" ]]; then | |
| db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='${escaped_error}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true | |
| log_info " $task_id: updated to blocked (was: $existing, blocked by: $blocker_reason)" | |
| fi | |
| return 0 | |
| fi | |
| # Not in DB — add it first, then mark blocked | |
| if cmd_add "$task_id" --repo "$repo"; then | |
| db "$SUPERVISOR_DB" "UPDATE tasks SET status='blocked', error='${escaped_error}', updated_at=strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$(sql_escape "$task_id")';" 2>/dev/null || true | |
| log_info " $task_id: registered as blocked (blocked by: $blocker_reason)" | |
| fi | |
| return 0 | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/scripts/supervisor/cron.sh around lines 454 - 498, In
_register_blocked_task, blocker_reason is interpolated directly into SQL UPDATE
statements (the two db calls that set error='Blocked by: ${blocker_reason}'),
allowing quotes or injection; escape the full error string via sql_escape before
embedding it in the SQL (e.g., compute an escaped_error or call sql_escape on
"Blocked by: ${blocker_reason}" and use that in both UPDATE queries) so both
UPDATEs use the escaped value.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cmd_auto_pickup()skipped blocked tasks entirely — tasks withblocked-by:dependencies or-neededtags were never registered in the supervisor DB, making them invisible until a subsequent auto-pickup run after blockers resolved_register_blocked_task()helper that creates tasks viacmd_addand immediately setsstatus='blocked'with the blocker reason in the error fieldauto_unblock_resolved_tasks()in todo-sync.sh to transition DB status fromblocked→queuedwhen all blockers resolve, enabling immediate dispatchChanges
cron.sh
_register_blocked_task()function: registers task in DB as blocked (handles both new and existing tasks)continuecontinuecontinue-neededblocker tags also registered as blocked (Strategies 1 & 2)todo-sync.sh
auto_unblock_resolved_tasks(): when all blockers resolve, also transition DB status fromblockedtoqueued(previously only removedblocked-by:from TODO.md)Impact
Blocked tasks (e.g., a managed private repo t003.2-t003.5, t009.2-t009.4) will now be registered in the DB immediately on first scan. When their blockers complete,
auto_unblock_resolved_taskstransitions them toqueuedand dispatch picks them up in the same pulse — no waiting for the next auto-pickup cycle.Summary by CodeRabbit
Release Notes