Skip to content

t1425, t1426, t1427: Fix pulse-wrapper zombie accumulation and blocking #4020

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
alex-solovyev merged 3 commits into from
Mar 9, 2026
Merged

t1425, t1426, t1427: Fix pulse-wrapper zombie accumulation and blocking #4020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
726bd44
fix: prevent pulse-wrapper zombie accumulation and rate limit blockin…
alex-solovyev Mar 9, 2026
fbbcd70
fix(t1426): cache person-stats hourly instead of every pulse
alex-solovyev Mar 9, 2026
c044f23
fix(t1426): gate person-stats cache refresh on estimated Search API cost
marcusquinn Mar 9, 2026
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
5 changes: 5 additions & 0 deletions .agents/scripts/contributor-activity-helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,11 @@ else:
HAVING human_ms + machine_ms > 5000
") || query_result="[]"

# t1427: sqlite3 -json returns "" (not "[]") when no rows match.
if [[ -z "$query_result" || "${query_result:0:1}" != "[" ]]; then
query_result="[]"
fi
Comment on lines +636 to +638
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

medium

The condition to check if query_result is a valid JSON array can be simplified. The current check [[ -z "$query_result" || "${query_result:0:1}" != "[" ]] is a bit verbose. A simpler and more robust way to check if the string starts with [ is to use a glob pattern match [[ "$query_result" != "["* ]]. This single check correctly handles both an empty string and a string that doesn't start with [, making the code more concise. This also aligns with the practice that shell functions expected to return JSON should output a valid empty JSON array ([]) on no results, ensuring consumers like jq do not break.

Suggested change
if [[ -z "$query_result" || "${query_result:0:1}" != "[" ]]; then
query_result="[]"
fi
if [[ "$query_result" != "["* ]]; then
query_result="[]"
fi
References
  1. When a shell function is expected to return JSON, it should output a valid empty JSON array ([]) or object ({}) on no results, rather than returning a non-zero exit code or empty string, to ensure consumers of the output (like pipes to jq) do not break.

Copy link
Copy Markdown
Owner

@marcusquinn marcusquinn Mar 9, 2026

Choose a reason for hiding this comment

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

Applied in follow-up PR. Simplified to [[ "$query_result" != "["* ]] as suggested -- handles both empty string and non-array in a single glob match.


# Process JSON in Python for classification and aggregation
echo "$query_result" | python3 -c "
import sys
Expand Down
124 changes: 121 additions & 3 deletions .agents/scripts/pulse-wrapper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ RAM_PER_WORKER_MB="${RAM_PER_WORKER_MB:-1024}" # 1 GB per worker
RAM_RESERVE_MB="${RAM_RESERVE_MB:-8192}" # 8 GB reserved for OS + user apps
MAX_WORKERS_CAP="${MAX_WORKERS_CAP:-8}" # Hard ceiling regardless of RAM
QUALITY_SWEEP_INTERVAL="${QUALITY_SWEEP_INTERVAL:-86400}" # 24 hours between sweeps
PERSON_STATS_INTERVAL="${PERSON_STATS_INTERVAL:-3600}" # 1 hour between person-stats refreshes (t1426)
DAILY_PR_CAP="${DAILY_PR_CAP:-5}" # Max PRs created per repo per day (GH#3821)
PRODUCT_RESERVATION_PCT="${PRODUCT_RESERVATION_PCT:-60}" # % of worker slots reserved for product repos (t1423)

Expand All @@ -82,6 +83,7 @@ RAM_PER_WORKER_MB=$(_validate_int RAM_PER_WORKER_MB "$RAM_PER_WORKER_MB" 1024 1)
RAM_RESERVE_MB=$(_validate_int RAM_RESERVE_MB "$RAM_RESERVE_MB" 8192)
MAX_WORKERS_CAP=$(_validate_int MAX_WORKERS_CAP "$MAX_WORKERS_CAP" 8)
QUALITY_SWEEP_INTERVAL=$(_validate_int QUALITY_SWEEP_INTERVAL "$QUALITY_SWEEP_INTERVAL" 86400)
PERSON_STATS_INTERVAL=$(_validate_int PERSON_STATS_INTERVAL "$PERSON_STATS_INTERVAL" 3600)
DAILY_PR_CAP=$(_validate_int DAILY_PR_CAP "$DAILY_PR_CAP" 5 1)
PRODUCT_RESERVATION_PCT=$(_validate_int PRODUCT_RESERVATION_PCT "$PRODUCT_RESERVATION_PCT" 60 0)
CHILD_RSS_LIMIT_KB=$(_validate_int CHILD_RSS_LIMIT_KB "$CHILD_RSS_LIMIT_KB" 2097152 1)
Expand All @@ -102,6 +104,8 @@ PULSE_MODEL="${PULSE_MODEL:-anthropic/claude-sonnet-4-6}"
REPOS_JSON="${REPOS_JSON:-${HOME}/.config/aidevops/repos.json}"
STATE_FILE="${HOME}/.aidevops/logs/pulse-state.txt"
QUALITY_SWEEP_LAST_RUN="${HOME}/.aidevops/logs/quality-sweep-last-run"
PERSON_STATS_LAST_RUN="${HOME}/.aidevops/logs/person-stats-last-run"
PERSON_STATS_CACHE_DIR="${HOME}/.aidevops/logs"
QUALITY_SWEEP_STATE_DIR="${HOME}/.aidevops/logs/quality-sweep-state"
CODERABBIT_ISSUE_SPIKE="${CODERABBIT_ISSUE_SPIKE:-10}" # trigger active review when issues increase by this many

Expand Down Expand Up @@ -1953,11 +1957,17 @@ ${worker_table}"
if [[ -x "$activity_helper" ]]; then
activity_md=$(bash "$activity_helper" summary "$repo_path" --period month --format markdown || echo "_Activity data unavailable._")
session_time_md=$(bash "$activity_helper" session-time "$repo_path" --period all --format markdown || echo "_Session data unavailable._")
person_stats_md=$(bash "$activity_helper" person-stats "$repo_path" --period month --format markdown || echo "_Person stats unavailable._")
else
activity_md="_Activity helper not installed._"
session_time_md="_Activity helper not installed._"
person_stats_md="_Activity helper not installed._"
fi
# t1426: person-stats from hourly cache (see _refresh_person_stats_cache)
local slug_safe="${repo_slug//\//-}"
local ps_cache="${PERSON_STATS_CACHE_DIR}/person-stats-cache-${slug_safe}.md"
if [[ -f "$ps_cache" ]]; then
person_stats_md=$(cat "$ps_cache")
else
person_stats_md="_Person stats not yet cached._"
fi

# --- Assemble body ---
Expand Down Expand Up @@ -2136,6 +2146,103 @@ _unpin_health_issue() {
return 0
}

#######################################
# Refresh person-stats cache (t1426)
#
# Runs at most once per PERSON_STATS_INTERVAL (default 1h).
# Computes per-repo and cross-repo person-stats, writes markdown
# to cache files. Health issue updates read from cache.
#######################################
_refresh_person_stats_cache() {
if [[ -f "$PERSON_STATS_LAST_RUN" ]]; then
local last_run
last_run=$(cat "$PERSON_STATS_LAST_RUN" 2>/dev/null || echo "0")
last_run="${last_run//[^0-9]/}"
last_run="${last_run:-0}"
local now
now=$(date +%s)
if [[ $((now - last_run)) -lt "$PERSON_STATS_INTERVAL" ]]; then
return 0
fi
fi

local activity_helper="${HOME}/.aidevops/agents/scripts/contributor-activity-helper.sh"
[[ -x "$activity_helper" ]] || return 0

local repos_json="$REPOS_JSON"
[[ -f "$repos_json" ]] || return 0

mkdir -p "$PERSON_STATS_CACHE_DIR"

# t1426: Estimate Search API cost before calling person_stats().
# person_stats() burns ~4 Search API requests per contributor per repo.
# GitHub Search API limit is 30 req/min. Check remaining budget against
# estimated cost to avoid blocking the pulse with rate-limit sleeps.
local search_remaining
search_remaining=$(gh api rate_limit --jq '.resources.search.remaining' 2>/dev/null) || search_remaining=0

# Per-repo person-stats
local repo_entries
repo_entries=$(jq -r '.initialized_repos[] | select(.pulse == true and (.local_only // false) == false and .slug != "") | "\(.slug)|\(.path)"' "$repos_json" 2>/dev/null || echo "")

# Count repos to estimate minimum cost (at least 1 contributor × 4 queries per repo)
local repo_count=0
local search_api_cost_per_contributor=4
while IFS='|' read -r _slug _path; do
[[ -z "$_slug" ]] && continue
repo_count=$((repo_count + 1))
done <<<"$repo_entries"

# Minimum budget: repo_count × 1 contributor × 4 queries. In practice,
# repos have 2-3 contributors, so this is a conservative lower bound.
local min_budget_needed=$((repo_count * search_api_cost_per_contributor))
if [[ "$search_remaining" -lt "$min_budget_needed" ]]; then
echo "[pulse-wrapper] Person stats cache refresh skipped: Search API budget ${search_remaining} < estimated cost ${min_budget_needed} (${repo_count} repos × ${search_api_cost_per_contributor} queries/contributor)" >>"$LOGFILE"
return 0
fi

while IFS='|' read -r slug path; do
[[ -z "$slug" ]] && continue

# Re-check budget before each repo — bail early if exhausted mid-refresh
search_remaining=$(gh api rate_limit --jq '.resources.search.remaining' 2>/dev/null) || search_remaining=0
if [[ "$search_remaining" -lt "$search_api_cost_per_contributor" ]]; then
echo "[pulse-wrapper] Person stats cache refresh stopped mid-run: Search API budget exhausted (${search_remaining} remaining)" >>"$LOGFILE"
break
fi

local slug_safe="${slug//\//-}"
local cache_file="${PERSON_STATS_CACHE_DIR}/person-stats-cache-${slug_safe}.md"
local md
md=$(bash "$activity_helper" person-stats "$path" --period month --format markdown 2>/dev/null) || md=""
if [[ -n "$md" ]]; then
echo "$md" >"$cache_file"
fi
done <<<"$repo_entries"

# Cross-repo person-stats — also gated on remaining budget
search_remaining=$(gh api rate_limit --jq '.resources.search.remaining' 2>/dev/null) || search_remaining=0
local all_repo_paths
all_repo_paths=$(jq -r '.initialized_repos[] | select(.pulse == true and (.local_only // false) == false) | .path' "$repos_json" 2>/dev/null || echo "")
if [[ -n "$all_repo_paths" && "$search_remaining" -ge "$search_api_cost_per_contributor" ]]; then
local -a cross_args=()
while IFS= read -r rp; do
[[ -n "$rp" ]] && cross_args+=("$rp")
done <<<"$all_repo_paths"
if [[ ${#cross_args[@]} -gt 1 ]]; then
local cross_md
cross_md=$(bash "$activity_helper" cross-repo-person-stats "${cross_args[@]}" --period month --format markdown 2>/dev/null) || cross_md=""
if [[ -n "$cross_md" ]]; then
echo "$cross_md" >"${PERSON_STATS_CACHE_DIR}/person-stats-cache-cross-repo.md"
fi
fi
fi

date +%s >"$PERSON_STATS_LAST_RUN"
echo "[pulse-wrapper] Person stats cache refreshed" >>"$LOGFILE"
return 0
}

#######################################
# Update health issues for ALL pulse-enabled repos
#
Expand All @@ -2159,9 +2266,13 @@ update_health_issues() {
return 0
fi

# Refresh person-stats cache if stale (t1426: hourly, not every pulse)
_refresh_person_stats_cache || true

# Pre-compute cross-repo summaries ONCE for all health issues.
# This avoids N×N git log walks (one cross-repo scan per repo dashboard)
# and redundant DB queries for session time.
# Person stats read from cache (refreshed hourly by _refresh_person_stats_cache).
local cross_repo_md=""
local cross_repo_session_time_md=""
local cross_repo_person_stats_md=""
Expand All @@ -2177,10 +2288,13 @@ update_health_issues() {
if [[ ${#cross_args[@]} -gt 1 ]]; then
cross_repo_md=$(bash "$activity_helper" cross-repo-summary "${cross_args[@]}" --period month --format markdown || echo "_Cross-repo data unavailable._")
cross_repo_session_time_md=$(bash "$activity_helper" cross-repo-session-time "${cross_args[@]}" --period all --format markdown || echo "_Cross-repo session data unavailable._")
cross_repo_person_stats_md=$(bash "$activity_helper" cross-repo-person-stats "${cross_args[@]}" --period month --format markdown || echo "_Cross-repo person stats unavailable._")
fi
fi
fi
local cross_repo_cache="${PERSON_STATS_CACHE_DIR}/person-stats-cache-cross-repo.md"
if [[ -f "$cross_repo_cache" ]]; then
cross_repo_person_stats_md=$(cat "$cross_repo_cache")
fi

local updated=0
while IFS='|' read -r slug path; do
Expand Down Expand Up @@ -3169,6 +3283,10 @@ main() {
return 0
fi

# t1425: Write PID early to prevent parallel instances during setup.
# run_pulse() overwrites with the opencode PID for watchdog tracking.
echo "$$" >"$PIDFILE"

cleanup_orphans
cleanup_worktrees
cleanup_stashes
Expand Down
Loading