-
Notifications
You must be signed in to change notification settings - Fork 7
t296: Auto-create batches on auto-pickup #1162
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13052,6 +13052,68 @@ cmd_auto_pickup() { | |
| log_info "No new tasks to pick up" | ||
| else | ||
| log_success "Picked up $picked_up new tasks" | ||
|
|
||
| # Auto-batch: assign picked-up tasks to a batch (t296) | ||
| # Find unbatched queued tasks (just added by auto-pickup) | ||
| local unbatched_queued | ||
| unbatched_queued=$(db "$SUPERVISOR_DB" " | ||
| SELECT t.id FROM tasks t | ||
| WHERE t.status = 'queued' | ||
| AND t.id NOT IN (SELECT task_id FROM batch_tasks) | ||
| ORDER BY t.created_at; | ||
| " 2>/dev/null || true) | ||
|
|
||
| if [[ -n "$unbatched_queued" ]]; then | ||
| # Check for an active batch (has non-terminal tasks) | ||
| local active_batch_id | ||
| active_batch_id=$(db "$SUPERVISOR_DB" " | ||
| SELECT b.id FROM batches b | ||
| WHERE EXISTS ( | ||
| SELECT 1 FROM batch_tasks bt | ||
| JOIN tasks t ON bt.task_id = t.id | ||
| WHERE bt.batch_id = b.id | ||
| AND t.status NOT IN ('complete','deployed','verified','verify_failed','merged','cancelled','failed','blocked') | ||
| ) | ||
| ORDER BY b.created_at DESC | ||
| LIMIT 1; | ||
| " 2>/dev/null || true) | ||
|
|
||
| if [[ -n "$active_batch_id" ]]; then | ||
| # Add to existing active batch | ||
| local added_count=0 | ||
| local max_pos | ||
| max_pos=$(db "$SUPERVISOR_DB" " | ||
| SELECT COALESCE(MAX(position), -1) FROM batch_tasks | ||
| WHERE batch_id = '$(sql_escape "$active_batch_id")'; | ||
| " 2>/dev/null || echo "-1") | ||
| local pos=$((max_pos + 1)) | ||
|
|
||
| while IFS= read -r tid; do | ||
| [[ -z "$tid" ]] && continue | ||
| db "$SUPERVISOR_DB" " | ||
| INSERT OR IGNORE INTO batch_tasks (batch_id, task_id, position) | ||
| VALUES ('$(sql_escape "$active_batch_id")', '$(sql_escape "$tid")', $pos); | ||
| " | ||
|
Comment on lines
+13085
to
+13096
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This SQL query is vulnerable to SQL Injection. Directly embedding shell variables into the query string, even with |
||
| pos=$((pos + 1)) | ||
| added_count=$((added_count + 1)) | ||
| done <<< "$unbatched_queued" | ||
|
Comment on lines
+13091
to
+13099
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This To improve performance, these inserts should be wrapped in a single database transaction. This significantly reduces the overhead of database writes. Example: db "$SUPERVISOR_DB" "BEGIN TRANSACTION;"
while IFS= read -r tid; do
[[ -z "$tid" ]] && continue
db "$SUPERVISOR_DB" "
INSERT OR IGNORE INTO batch_tasks (batch_id, task_id, position)
VALUES ('$(sql_escape "$active_batch_id")', '$(sql_escape "$tid")', $pos);
"
pos=$((pos + 1))
added_count=$((added_count + 1))
done <<< "$unbatched_queued"
db "$SUPERVISOR_DB" "COMMIT;" |
||
|
|
||
| if [[ "$added_count" -gt 0 ]]; then | ||
| log_success "Auto-batch: added $added_count tasks to active batch $active_batch_id" | ||
| fi | ||
| else | ||
| # Create a new auto-batch | ||
| local auto_batch_name | ||
| auto_batch_name="auto-$(date +%Y%m%d-%H%M%S)" | ||
| local task_csv | ||
| task_csv=$(echo "$unbatched_queued" | tr '\n' ',' | sed 's/,$//') | ||
| local auto_batch_id | ||
| auto_batch_id=$(cmd_batch "$auto_batch_name" --concurrency 3 --tasks "$task_csv" 2>/dev/null) | ||
| if [[ -n "$auto_batch_id" ]]; then | ||
| log_success "Auto-batch: created '$auto_batch_name' ($auto_batch_id) with $picked_up tasks" | ||
| fi | ||
| fi | ||
| fi | ||
| fi | ||
|
|
||
| return 0 | ||
|
|
@@ -14154,12 +14216,18 @@ GitHub Issue Auto-Creation (t149): | |
| (dedup by title search). Requires: gh CLI authenticated. | ||
| Disable: --no-issue flag on add, or SUPERVISOR_AUTO_ISSUE=false globally. | ||
|
|
||
| Cron Integration & Auto-Pickup (t128.5): | ||
| Cron Integration & Auto-Pickup (t128.5, t296): | ||
| Auto-pickup scans TODO.md for tasks to automatically queue: | ||
| 1. Tasks tagged with #auto-dispatch anywhere in the line | ||
| 2. Tasks listed under a "## Dispatch Queue" section header | ||
| Both strategies skip tasks already tracked by the supervisor. | ||
|
|
||
| Auto-batching (t296): When new tasks are picked up, they are automatically | ||
| assigned to a batch. If an active batch exists, tasks are added to it. | ||
| Otherwise, a new batch named 'auto-YYYYMMDD-HHMMSS' is created with | ||
| concurrency 3. This ensures all auto-dispatched tasks get batch-level | ||
| concurrency control, completion tracking, and lifecycle management. | ||
|
|
||
| Cron scheduling runs pulse (which includes auto-pickup) every N minutes: | ||
| supervisor-helper.sh cron install # Every 5 minutes (default) | ||
| supervisor-helper.sh cron install --interval 2 # Every 2 minutes | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of
2>/dev/nullhere and in other parts of this function (lines 13079, 13088, and 13111) suppresses potentially useful error messages from database operations. This can make debugging difficult if a query fails.According to the repository style guide, blanket error suppression is discouraged. It would be better to redirect stderr to a log file, for example
2>> "$SUPERVISOR_LOG".References
2>/dev/nullis acceptable only when redirecting to log files, not for blanket suppression of errors. (link)