Skip to content
Merged
Changes from all commits
Commits
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
183 changes: 183 additions & 0 deletions .agents/scripts/supervisor/pulse.sh
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,19 @@ cmd_pulse() {
fi
fi

# Phase 3a: Adopt untracked PRs into the supervisor pipeline
# Scans open PRs for each tracked repo and adopts any that:
# 1. Have a task ID in the title (tNNN: description)
# 2. Are not already tracked in the supervisor DB
# 3. Have a matching open task in TODO.md
# Adopted PRs get a DB entry with status=complete so Phase 3 processes them
# through the normal review → merge → verify lifecycle.
# This closes the gap where interactive sessions create PRs that the
# supervisor can't manage (review, merge, verify, clean up).
if command -v gh &>/dev/null; then
adopt_untracked_prs 2>>"$SUPERVISOR_LOG" || true
fi

# Phase 3: Post-PR lifecycle (t128.8)
# Process tasks that workers completed (PR created) but still need merge/deploy
# t265: Redirect stderr to log and capture errors before || true suppresses them
Expand Down Expand Up @@ -2170,6 +2183,176 @@ RULES:
return 0
}

#######################################
# Phase 3a: Adopt untracked PRs into the supervisor pipeline
# Scans open PRs for each tracked repo and creates DB entries for any
# that have a task ID in the title but aren't tracked in the supervisor DB.
# This allows PRs created in interactive sessions to be managed by the
# supervisor (review, merge, verify, clean up) without manual registration.
#
# Adoption criteria:
# 1. PR title matches pattern: tNNN: description (or tNNN.N: description)
# 2. No task in the DB already has this PR URL
# 3. The task ID exists as an open task in TODO.md
# 4. The task is not already in the DB (avoids duplicating worker tasks)
#
# Adopted tasks enter the DB with status=complete and the PR URL, so
# Phase 3 picks them up through the normal lifecycle.
#######################################
adopt_untracked_prs() {
ensure_db

# Collect all unique repos from the DB
local repos
repos=$(db "$SUPERVISOR_DB" "SELECT DISTINCT repo FROM tasks WHERE repo IS NOT NULL AND repo != '';" 2>/dev/null || echo "")

if [[ -z "$repos" ]]; then
return 0
fi

local adopted_count=0

while IFS= read -r repo_path; do
[[ -z "$repo_path" || ! -d "$repo_path" ]] && continue

# Get repo slug for gh CLI
local repo_slug
repo_slug=$(detect_repo_slug "$repo_path" 2>/dev/null || echo "")
if [[ -z "$repo_slug" ]]; then
continue
fi

# List open PRs (limit to 20 to avoid API rate limits)
local open_prs
open_prs=$(gh pr list --repo "$repo_slug" --state open --limit 20 \
--json number,title,url 2>/dev/null || echo "[]")
Comment on lines +2227 to +2228
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The hardcoded limit 20 for the number of pull requests to fetch should be extracted into a constant for better readability and maintainability. This makes it easier to find and change in the future.

You can define it at the top of the adopt_untracked_prs function:

local -r MAX_PRS_TO_ADOPT=20
Suggested change
open_prs=$(gh pr list --repo "$repo_slug" --state open --limit 20 \
--json number,title,url 2>/dev/null || echo "[]")
open_prs=$(gh pr list --repo "$repo_slug" --state open --limit "$MAX_PRS_TO_ADOPT" \
--json number,title,url 2>/dev/null || echo "[]")


local pr_count
pr_count=$(printf '%s' "$open_prs" | jq 'length' 2>/dev/null || echo 0)

local i=0
while [[ "$i" -lt "$pr_count" ]]; do
local pr_number pr_title pr_url
pr_number=$(printf '%s' "$open_prs" | jq -r ".[$i].number" 2>/dev/null || echo "")
pr_title=$(printf '%s' "$open_prs" | jq -r ".[$i].title" 2>/dev/null || echo "")
pr_url=$(printf '%s' "$open_prs" | jq -r ".[$i].url" 2>/dev/null || echo "")
i=$((i + 1))

# Extract task ID from PR title (pattern: tNNN: or tNNN.N:)
local task_id=""
if [[ "$pr_title" =~ ^(t[0-9]+(\.[0-9]+)?):\ .* ]]; then
task_id="${BASH_REMATCH[1]}"
fi

if [[ -z "$task_id" ]]; then
continue
fi

# Check if this PR is already tracked in the DB
local existing_pr
existing_pr=$(db "$SUPERVISOR_DB" "
SELECT id FROM tasks
WHERE pr_url = '$(sql_escape "$pr_url")'
LIMIT 1;
" 2>/dev/null || echo "")
Comment on lines +2253 to +2257
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

This method of constructing SQL queries by embedding escaped variables is vulnerable to SQL injection if the sql_escape function is flawed. It also violates the repository style guide, which states: 'Use parameterized queries where possible' (line 25).

References
  1. The code constructs SQL queries using string interpolation with an escape function, which is not as secure as using parameterized queries. The style guide recommends using parameterized queries where possible to prevent SQL injection vulnerabilities. (link)
  2. To prevent SQL injection in shell scripts using sqlite3, create a helper function that uses .param set for safe parameterized bindings instead of direct string interpolation.


if [[ -n "$existing_pr" ]]; then
continue
fi

# Check if this task ID is already in the DB (worker-dispatched)
local existing_task
existing_task=$(db "$SUPERVISOR_DB" "
SELECT id, status FROM tasks
WHERE id = '$(sql_escape "$task_id")'
LIMIT 1;
" 2>/dev/null || echo "")

if [[ -n "$existing_task" ]]; then
# Task exists but doesn't have this PR URL — link it
local existing_status
existing_status=$(echo "$existing_task" | cut -d'|' -f2)
# Only link if the task is in a state where a PR makes sense
if [[ "$existing_status" =~ ^(queued|running|evaluating|retrying|complete)$ ]]; then
db "$SUPERVISOR_DB" "
UPDATE tasks
SET pr_url = '$(sql_escape "$pr_url")',
status = 'complete',
updated_at = strftime('%Y-%m-%dT%H:%M:%SZ', 'now')
WHERE id = '$(sql_escape "$task_id")';
" 2>/dev/null || true
log_info "Phase 3a: Linked PR #$pr_number to existing task $task_id (was: $existing_status)"
adopted_count=$((adopted_count + 1))
fi
continue
fi
Comment on lines +2271 to +2288
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Force-completing running/evaluating tasks can orphan active worker processes.

When existing_status is running, the UPDATE at line 2277 sets status='complete' while the worker process is still alive. Phase 4's health check skips tasks whose DB status is not running/dispatched, so the worker is no longer supervised. It continues executing — making commits, API calls — until it either exits naturally or Phase 4e's PPID=1 sweep eventually catches it.

evaluating is less severe (no live worker) but still bypasses the normal evaluation/outcome flow.

Remove running and evaluating from the allowlist; these states indicate in-flight work that shouldn't be hijacked.

🐛 Proposed fix
-			if [[ "$existing_status" =~ ^(queued|running|evaluating|retrying|complete)$ ]]; then
+			if [[ "$existing_status" =~ ^(queued|retrying|complete)$ ]]; then
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/scripts/supervisor/pulse.sh around lines 2271 - 2288, The code
currently forces tasks with existing_status matching
(queued|running|evaluating|retrying|complete) to be marked complete and linked
to the PR, which can orphan live workers; update the conditional in the pulse.sh
block that checks existing_status (the regex match used around the UPDATE for
task_id/pr_url and the log_info line "Phase 3a: Linked PR") to remove "running"
and "evaluating" from the allowlist so only tasks in queued, retrying, or
complete are auto-linked; keep the rest of the update and adopted_count
increment logic unchanged and ensure the conditional uses the updated regex to
prevent hijacking in-flight work.


# Task not in DB — check if it exists in TODO.md
local todo_file="$repo_path/TODO.md"
if [[ ! -f "$todo_file" ]]; then
continue
fi

local todo_line
todo_line=$(grep -E "^[[:space:]]*- \[( |x|-)\] $task_id " "$todo_file" 2>/dev/null | head -1 || true)

if [[ -z "$todo_line" ]]; then
continue
Comment on lines +2296 to +2300
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Unescaped . in extended regex: subtask IDs like t123.4 match any character in that position.

grep -E treats . in $task_id as a wildcard, so t123.4 would also match a line containing t123X4. Escape the dot before interpolation.

🛡️ Proposed fix
+		local escaped_task_id
+		escaped_task_id=$(printf '%s' "$task_id" | sed 's/\./\\./g')
 		todo_line=$(grep -E "^[[:space:]]*- \[( |x|-)\] $task_id " "$todo_file" 2>/dev/null | head -1 || true)
+		todo_line=$(grep -E "^[[:space:]]*- \[( |x|-)\] $escaped_task_id " "$todo_file" 2>/dev/null | head -1 || true)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/scripts/supervisor/pulse.sh around lines 2296 - 2300, The grep regex
is treating dots in $task_id as wildcards, so update the code that sets
todo_line to escape dots in the task id before interpolation; e.g. create an
escaped variable like escaped_task_id=${task_id//./\\.} and use grep -E
"^[[:space:]]*- \\[( |x|-)\\] $escaped_task_id " "$todo_file" to ensure literal
dots are matched (refer to todo_line, $task_id, grep -E and $todo_file in the
diff).

fi

# Extract description from TODO.md
local description
description=$(echo "$todo_line" | sed -E 's/^[[:space:]]*- \[( |x|-)\] [^ ]* //' || true)

# Adopt: create a DB entry with status=complete and the PR URL
# Phase 3 will then process it through review → merge → verify
local batch_id_for_adopt=""
# Find the active batch for this repo to associate the task
batch_id_for_adopt=$(db "$SUPERVISOR_DB" "
SELECT b.id FROM batches b
WHERE b.status IN ('active', 'running')
ORDER BY b.created_at DESC
LIMIT 1;
" 2>/dev/null || echo "")
Comment on lines +2309 to +2316
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Step 1: Locate and read the pulse.sh file around lines 2309-2316
wc -l .agents/scripts/supervisor/pulse.sh

Repository: marcusquinn/aidevops

Length of output: 104


🏁 Script executed:

# Step 2: Read the specific section in question
sed -n '2300,2325p' .agents/scripts/supervisor/pulse.sh

Repository: marcusquinn/aidevops

Length of output: 955


🏁 Script executed:

# Step 3: Search for CREATE TABLE statements to understand schema
rg "CREATE TABLE" --type sh -A 10

Repository: marcusquinn/aidevops

Length of output: 50376


🏁 Script executed:

# Step 4: Search for how other batch queries are structured in the codebase
rg "SELECT.*FROM.*batches" --type sh -A 3 -B 2

Repository: marcusquinn/aidevops

Length of output: 17112


🏁 Script executed:

# Step 5: Look for how repo filtering is typically done
rg "batch_tasks" --type sh -A 3

Repository: marcusquinn/aidevops

Length of output: 15281


🏁 Script executed:

# Check if there are other places in pulse.sh where batch_id_for_adopt is used
grep -n "batch_id_for_adopt" .agents/scripts/supervisor/pulse.sh | head -20

Repository: marcusquinn/aidevops

Length of output: 284


🏁 Script executed:

# Verify the batches table definition in the actual database schema
grep -A 15 "CREATE TABLE IF NOT EXISTS batches" .agents/scripts/supervisor/database.sh

Repository: marcusquinn/aidevops

Length of output: 838


🏁 Script executed:

# Check the context around lines 2309-2316 to see what repo_path is
sed -n '2295,2345p' .agents/scripts/supervisor/pulse.sh

Repository: marcusquinn/aidevops

Length of output: 1761


Batch lookup is not scoped to the current repo — cross-repo contamination in multi-repo setups.

The query at lines 2311–2318 picks the most-recently-created active batch across ALL repos. In a multi-repo deployment, a task adopted from repo A will be inserted into repo B's active batch, skewing batch progress counters and potentially triggering that batch's completion actions (retrospective, distillation, auto-release) prematurely.

Filter by repo by joining through batch_tasks and tasks. Since batches has no repo column, use EXISTS (SELECT 1 FROM batch_tasks bt JOIN tasks t ON bt.task_id = t.id WHERE bt.batch_id = b.id AND t.repo = '$(sql_escape "$repo_path")') to match the pattern used throughout the codebase (cron.sh, state.sh).

♻️ Proposed fix
-			batch_id_for_adopt=$(db "$SUPERVISOR_DB" "
-				SELECT b.id FROM batches b
-				WHERE b.status IN ('active', 'running')
-				ORDER BY b.created_at DESC
-				LIMIT 1;
-			" 2>/dev/null || echo "")
+			batch_id_for_adopt=$(db "$SUPERVISOR_DB" "
+				SELECT b.id FROM batches b
+				WHERE b.status IN ('active', 'running')
+				  AND EXISTS (
+				    SELECT 1 FROM batch_tasks bt
+				    JOIN tasks t ON bt.task_id = t.id
+				    WHERE bt.batch_id = b.id
+				      AND t.repo = '$(sql_escape "$repo_path")'
+				  )
+				ORDER BY b.created_at DESC
+				LIMIT 1;
+			" 2>/dev/null || echo "")
📝 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
local batch_id_for_adopt=""
# Find the active batch for this repo to associate the task
batch_id_for_adopt=$(db "$SUPERVISOR_DB" "
SELECT b.id FROM batches b
WHERE b.status IN ('active', 'running')
ORDER BY b.created_at DESC
LIMIT 1;
" 2>/dev/null || echo "")
local batch_id_for_adopt=""
# Find the active batch for this repo to associate the task
batch_id_for_adopt=$(db "$SUPERVISOR_DB" "
SELECT b.id FROM batches b
WHERE b.status IN ('active', 'running')
AND EXISTS (
SELECT 1 FROM batch_tasks bt
JOIN tasks t ON bt.task_id = t.id
WHERE bt.batch_id = b.id
AND t.repo = '$(sql_escape "$repo_path")'
)
ORDER BY b.created_at DESC
LIMIT 1;
" 2>/dev/null || echo "")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/scripts/supervisor/pulse.sh around lines 2309 - 2316, The batch
lookup currently selects the most-recent active batch across all repos; update
the SQL in the db call that sets batch_id_for_adopt to restrict batches to the
current repo by adding an EXISTS subquery that joins batch_tasks bt to tasks t
and matches t.repo = '$(sql_escape "$repo_path")' (same pattern used in
cron.sh/state.sh). Keep the surrounding variable (batch_id_for_adopt), db
invocation and SUPERVISOR_DB usage unchanged; only modify the WHERE clause to
add the EXISTS(...) repo filter so an adopted task is associated only with a
batch containing tasks from the same repo.


db "$SUPERVISOR_DB" "
INSERT INTO tasks (id, status, description, repo, pr_url, model, max_retries, created_at, updated_at)
VALUES (
'$(sql_escape "$task_id")',
'complete',
'$(sql_escape "$description")',
'$(sql_escape "$repo_path")',
'$(sql_escape "$pr_url")',
'interactive',
0,
strftime('%Y-%m-%dT%H:%M:%SZ', 'now'),
strftime('%Y-%m-%dT%H:%M:%SZ', 'now')
);
" 2>/dev/null || {
log_warn "Phase 3a: Failed to insert task $task_id (may already exist)"
continue
}

# Associate with active batch if one exists
if [[ -n "$batch_id_for_adopt" ]]; then
db "$SUPERVISOR_DB" "
INSERT OR IGNORE INTO batch_tasks (batch_id, task_id)
VALUES ('$(sql_escape "$batch_id_for_adopt")', '$(sql_escape "$task_id")');
" 2>/dev/null || true
fi

log_success "Phase 3a: Adopted PR #$pr_number ($pr_url) as task $task_id"
adopted_count=$((adopted_count + 1))
done
Comment on lines +2234 to +2346
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This loop is inefficient as it calls jq multiple times for every pull request. A more efficient and idiomatic approach is to use a single jq command to format all the data and pipe it to a while read loop. Using process substitution (< <(...)) also avoids running the loop in a subshell, ensuring that variables like adopted_count are correctly modified in the current shell context.

while IFS=$'\t' read -r pr_number pr_title pr_url; do
			# Extract task ID from PR title (pattern: tNNN: or tNNN.N:)
			local task_id=""
			if [[ "$pr_title" =~ ^(t[0-9]+(\.[0-9]+)?):\ .* ]]; then
				task_id="${BASH_REMATCH[1]}"
			fi

			if [[ -z "$task_id" ]]; then
				continue
			fi

			# Check if this PR is already tracked in the DB
			local existing_pr
			existing_pr=$(db "$SUPERVISOR_DB" "
				SELECT id FROM tasks
				WHERE pr_url = '$(sql_escape "$pr_url")'
				LIMIT 1;
			" 2>/dev/null || echo "")

			if [[ -n "$existing_pr" ]]; then
				continue
			fi

			# Check if this task ID is already in the DB (worker-dispatched)
			local existing_task
			existing_task=$(db "$SUPERVISOR_DB" "
				SELECT id, status FROM tasks
				WHERE id = '$(sql_escape "$task_id")'
				LIMIT 1;
			" 2>/dev/null || echo "")

			if [[ -n "$existing_task" ]]; then
				# Task exists but doesn't have this PR URL — link it
				local existing_status
				existing_status=$(echo "$existing_task" | cut -d'|' -f2)
				# Only link if the task is in a state where a PR makes sense
				if [[ "$existing_status" =~ ^(queued|running|evaluating|retrying|complete)$ ]]; then
					db "$SUPERVISOR_DB" "
						UPDATE tasks
						SET pr_url = '$(sql_escape "$pr_url")',
						    status = 'complete',
						    updated_at = strftime('%Y-%m-%dT%H:%M:%SZ', 'now')
						WHERE id = '$(sql_escape "$task_id")';
					" 2>/dev/null || true
					log_info "Phase 3a: Linked PR #$pr_number to existing task $task_id (was: $existing_status)"
					adopted_count=$((adopted_count + 1))
				fi
				continue
			fi

			# Task not in DB — check if it exists in TODO.md
			local todo_file="$repo_path/TODO.md"
			if [[ ! -f "$todo_file" ]]; then
				continue
			fi

			local todo_line
			todo_line=$(grep -E "^[[:space:]]*- \[ |x|-\] $task_id " "$todo_file" 2>/dev/null | head -1 || true)

			if [[ -z "$todo_line" ]]; then
				continue
			fi

			# Extract description from TODO.md
			local description
			description=$(echo "$todo_line" | sed -E 's/^[[:space:]]*- \[ |x|-\] [^ ]* //' || true)

			# Adopt: create a DB entry with status=complete and the PR URL
			# Phase 3 will then process it through review → merge → verify
			local batch_id_for_adopt=""
			# Find the active batch for this repo to associate the task
			batch_id_for_adopt=$(db "$SUPERVISOR_DB" "
				SELECT b.id FROM batches b
				WHERE b.status IN ('active', 'running')
				ORDER BY b.created_at DESC
				LIMIT 1;
			" 2>/dev/null || echo "")

			db "$SUPERVISOR_DB" "
				INSERT INTO tasks (id, status, description, repo, pr_url, model, max_retries, created_at, updated_at)
				VALUES (
					'$(sql_escape "$task_id")',
					'complete',
					'$(sql_escape "$description")',
					'$(sql_escape "$repo_path")',
					'$(sql_escape "$pr_url")',
					'interactive',
					0,
					strftime('%Y-%m-%dT%H:%M:%SZ', 'now'),
					strftime('%Y-%m-%dT%H:%M:%SZ', 'now')
				);
			" 2>/dev/null || {
				log_warn "Phase 3a: Failed to insert task $task_id (may already exist)"
				continue
			}

			# Associate with active batch if one exists
			if [[ -n "$batch_id_for_adopt" ]]; then
				db "$SUPERVISOR_DB" "
					INSERT OR IGNORE INTO batch_tasks (batch_id, task_id)
					VALUES ('$(sql_escape "$batch_id_for_adopt")', '$(sql_escape "$task_id")');
				" 2>/dev/null || true
			fi

			log_success "Phase 3a: Adopted PR #$pr_number ($pr_url) as task $task_id"
			adopted_count=$((adopted_count + 1))
		 done < <(printf '%s' "$open_prs" | jq -r '.[] | [.number, .title, .url] | @tsv')

done <<<"$repos"

if [[ "$adopted_count" -gt 0 ]]; then
log_info "Phase 3a: Adopted $adopted_count untracked PR(s)"
fi

return 0
}

#######################################
# Process post-PR lifecycle for all eligible tasks
# Called as Phase 3 of the pulse cycle
Expand Down
Loading