Skip to content

fix: register blocked tasks in DB during auto-pickup #2239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcusquinn merged 1 commit into from
Feb 24, 2026
Merged

fix: register blocked tasks in DB during auto-pickup #2239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 63 additions & 1 deletion .agents/scripts/supervisor/cron.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,52 @@ _check_and_skip_if_blocked() {
return 1
}

#######################################
# 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
}
Comment on lines +470 to +498
Copy link

@gemini-code-assist gemini-code-assist bot Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This new function has a couple of areas for improvement:

  1. Incorrect failure handling: The function documentation at line 468 states it returns 1 on failure, but the current implementation always returns 0. If cmd_add fails, the function silently continues and returns success, which can hide problems and makes the function's behavior inconsistent with its documentation.
  2. Error suppression: The db calls 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 || true is sufficient to prevent script exit with set -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
  1. 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 +454 to +498
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.


#######################################
# t1239: Cross-repo misregistration guard for auto-pickup.
# Returns 0 (skip) if the task_id is already registered in the DB
Expand Down Expand Up @@ -560,8 +606,15 @@ cmd_auto_pickup() {
continue
fi

# Register blocked tasks in DB instead of skipping entirely.
# This makes blocked tasks visible to the supervisor so they
# can be dispatched immediately when blockers resolve.

# Skip tasks with unresolved blocked-by dependencies (t1085.4)
if _check_and_skip_if_blocked "$line" "$task_id" "$todo_file"; then
local unresolved
unresolved=$(is_task_blocked "$line" "$todo_file" || true)
_register_blocked_task "$task_id" "$repo" "$unresolved"
continue
fi

Expand All @@ -570,6 +623,7 @@ cmd_auto_pickup() {
local blocker_tag
blocker_tag=$(echo "$line" | grep -oE '(account|hosting|login|api-key|clarification|resources|payment|approval|decision|design|content|dns|domain|testing)-needed' | head -1)
log_info " $task_id: blocked by $blocker_tag (human action required) — skipping auto-pickup"
_register_blocked_task "$task_id" "$repo" "$blocker_tag"
continue
fi

Expand Down Expand Up @@ -653,8 +707,12 @@ cmd_auto_pickup() {
continue
fi

# Register blocked tasks in DB instead of skipping entirely.
# Skip tasks with unresolved blocked-by dependencies (t1085.4)
if _check_and_skip_if_blocked "$line" "$task_id" "$todo_file"; then
local unresolved
unresolved=$(is_task_blocked "$line" "$todo_file" || true)
_register_blocked_task "$task_id" "$repo" "$unresolved"
continue
fi

Expand All @@ -663,6 +721,7 @@ cmd_auto_pickup() {
local blocker_tag
blocker_tag=$(echo "$line" | grep -oE '(account|hosting|login|api-key|clarification|resources|payment|approval|decision|design|content|dns|domain|testing)-needed' | head -1)
log_info " $task_id: blocked by $blocker_tag (human action required) — skipping auto-pickup"
_register_blocked_task "$task_id" "$repo" "$blocker_tag"
continue
fi

Expand Down Expand Up @@ -816,8 +875,11 @@ cmd_auto_pickup() {
continue
fi

# Skip tasks with unresolved blocked-by dependencies
# Register blocked subtasks in DB instead of skipping entirely
if _check_and_skip_if_blocked "$sub_line" "$sub_id" "$todo_file"; then
local unresolved
unresolved=$(is_task_blocked "$sub_line" "$todo_file" || true)
_register_blocked_task "$sub_id" "$repo" "$unresolved"
continue
fi

Expand Down
10 changes: 10 additions & 0 deletions .agents/scripts/supervisor/todo-sync.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1260,6 +1260,16 @@ auto_unblock_resolved_tasks() {
sed_inplace "${line_num}s/[[:space:]]*$//" "$todo_file"
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;" 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
Comment on lines +1263 to +1271
Copy link

@gemini-code-assist gemini-code-assist bot Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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
  1. 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.


unblocked_count=$((unblocked_count + 1))
if [[ -n "$unblocked_ids" ]]; then
unblocked_ids="${unblocked_ids}, ${task_id}"
Expand Down
Loading