Skip to content

feat: add pre-dispatch verification to prevent dispatching already-done tasks #806

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 9, 2026
Merged

feat: add pre-dispatch verification to prevent dispatching already-done tasks #806

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
65 changes: 65 additions & 0 deletions .agents/scripts/supervisor-helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2542,6 +2542,60 @@ cmd_unclaim() {
# Returns 0 if free or claimed by self, 1 if claimed by another.
# Outputs the assignee on stdout if claimed by another.
#######################################
# check_task_already_done() — pre-dispatch verification
# Checks git history for evidence that a task was already completed.
# Returns 0 (true) if task appears done, 1 (false) if not.
# Searches for: (1) commits with task ID in message, (2) TODO.md [x] marker,
# (3) merged PR references. Fast path: git log grep is O(log n) on packed refs.
check_task_already_done() {
Copy link

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

Choose a reason for hiding this comment

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

medium

The style guide (line 50) states that 2>/dev/null should only be used for redirecting to log files, not for blanket error suppression. This function uses it in several places (lines 2562, 2572, 2576, 2587, 2589) to suppress potential errors from grep, git, and gh. This can hide important issues like permission errors or configuration problems, making debugging harder. Consider removing these suppressions or handling errors more explicitly.

References
  1. 2>/dev/null is acceptable ONLY when redirecting to log files, not blanket suppression. (link)

local task_id="${1:-}"
local project_root="${2:-.}"

if [[ -z "$task_id" ]]; then
return 1
fi

# Check 1: Is the task already marked [x] in TODO.md?
local todo_file="$project_root/TODO.md"
if [[ -f "$todo_file" ]]; then
# Match: "- [x] tNNN " with the exact task ID (word boundary via space)
if grep -qE "^\s*- \[x\] ${task_id}[[:space:]]" "$todo_file" 2>/dev/null; then
log_info "Pre-dispatch check: $task_id is marked [x] in TODO.md" >&2
return 0
fi
fi

# Check 2: Are there merged commits referencing this task ID?
# Look for commit messages containing the task ID (e.g., "t135.4", "(t135.4)")
# Only search the last 500 commits for performance
local commit_count=0
commit_count=$(git -C "$project_root" log --oneline -500 --all --grep="$task_id" 2>/dev/null | wc -l | tr -d ' ') || true
if [[ "$commit_count" -gt 0 ]]; then
# Verify at least one commit looks like a completion (not just "claim" or "blocked")
local completion_evidence=""
completion_evidence=$(git -C "$project_root" log --oneline -500 --all --grep="$task_id" 2>/dev/null \
| grep -iE "(feat|fix|refactor|chore|perf|test).*${task_id}|${task_id}.*(\(#[0-9]+\)|PR #[0-9]+|merged)" \
| head -1) || true
if [[ -n "$completion_evidence" ]]; then
log_info "Pre-dispatch check: $task_id has completion evidence: $completion_evidence" >&2
return 0
fi
fi
Comment on lines +2571 to +2583
Copy link

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

Choose a reason for hiding this comment

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

high

This block has a potential bug and an inefficiency.

  1. Bug: If the git log pipe fails, commit_count can become an empty string. This would cause the numeric comparison [[ "$commit_count" -gt 0 ]] to fail with an "integer expression expected" error, which will terminate the script under set -e.
  2. Inefficiency: The git log command is executed twice, which can be slow on large repositories.

You can address both issues by running git log once, storing the output, and then checking if the output is non-empty before grepping for evidence. This avoids the risky numeric comparison and the redundant git log call.

Suggested change
local commit_count=0
commit_count=$(git -C "$project_root" log --oneline -500 --all --grep="$task_id" 2>/dev/null | wc -l | tr -d ' ') || true
if [[ "$commit_count" -gt 0 ]]; then
# Verify at least one commit looks like a completion (not just "claim" or "blocked")
local completion_evidence=""
completion_evidence=$(git -C "$project_root" log --oneline -500 --all --grep="$task_id" 2>/dev/null \
| grep -iE "(feat|fix|refactor|chore|perf|test).*${task_id}|${task_id}.*(\(#[0-9]+\)|PR #[0-9]+|merged)" \
| head -1) || true
if [[ -n "$completion_evidence" ]]; then
log_info "Pre-dispatch check: $task_id has completion evidence: $completion_evidence" >&2
return 0
fi
fi
local log_output
log_output=$(git -C "$project_root" log --oneline -500 --all --grep="$task_id" 2>/dev/null) || true
if [[ -n "$log_output" ]]; then
# Verify at least one commit looks like a completion (not just "claim" or "blocked")
local completion_evidence
completion_evidence=$(echo "$log_output" \
| grep -iE "(feat|fix|refactor|chore|perf|test).*${task_id}|${task_id}.*(\(#[0-9]+\)|PR #[0-9]+|merged)" \
| head -1) || true
if [[ -n "$completion_evidence" ]]; then
log_info "Pre-dispatch check: $task_id has completion evidence: $completion_evidence" >&2
return 0
fi
fi


# Check 3: Does a merged PR exist for this task?
# Only check if gh CLI is available and authenticated (cached check)
if command -v gh &>/dev/null && check_gh_auth 2>/dev/null; then
local pr_count=0
pr_count=$(gh pr list --repo "$project_root" --state merged --search "$task_id in:title" --limit 1 --json number --jq 'length' 2>/dev/null) || true
if [[ "$pr_count" -gt 0 ]]; then
log_info "Pre-dispatch check: $task_id has a merged PR on GitHub" >&2
return 0
fi
fi

return 1
}

check_task_claimed() {
local task_id="${1:-}"
local project_root="${2:-.}"
Expand Down Expand Up @@ -3965,6 +4019,17 @@ cmd_dispatch() {
return 1
fi

# Pre-dispatch verification: check if task was already completed in a prior batch.
# Searches git history for commits referencing this task ID. If a merged PR commit
# exists, the task is already done — cancel it instead of wasting an Opus session.
# This prevents the exact bug from backlog-10 where 6 t135 subtasks were dispatched
# despite being completed months earlier.
if check_task_already_done "$task_id" "${trepo:-.}"; then
log_warn "Task $task_id appears already completed in git history — cancelling"
db "$SUPERVISOR_DB" "UPDATE tasks SET status='cancelled', error='Pre-dispatch: already completed in git history' WHERE id='$(sql_escape "$task_id")';"
return 0
fi

# Check if task is claimed by someone else via TODO.md assignee: field (t165)
local claimed_by=""
claimed_by=$(check_task_claimed "$task_id" "${trepo:-.}" 2>/dev/null) || true
Expand Down
Loading