From d888bd568b4872c88869d292e1cc3faa3572449a Mon Sep 17 00:00:00 2001 From: santisoutoo Date: Tue, 25 Aug 2026 10:46:27 +0200 Subject: [PATCH] fix: replace agent-loop's flat call timeout with an idle-timeout, add infra-failure escalation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flat 15min wall-clock timeout added yesterday (PR #102) killed the Implement step mid-exploration on a wide-scope issue (#52) that was actively producing output, not hung. It couldn't tell "genuinely stuck" (the 84min zero-output hang on issue #85 that motivated PR #102) from "slow but working". run-engine.sh now kills a call only after N seconds with NO NEW output, with a much larger hard ceiling as backstop; Implement gets a generous idle/ceiling budget, Review (where the real hang happened) stays tight, and the one-round fix-call gets its own inline override. Also, while auditing the loop for other robustness gaps: - Infra failures (engine crash, timeout, no result marker) never had a counter or escalation path — an issue that reliably infra-fails would retry forever, 3x/day, with no human ever finding out. They now get their own counter and escalate to a new `agent:infra-stuck` label after 3, excluded from future picks like `agent:blocked` already is. - A DONE-but-empty-diff guard failure (known opencode silent-exit bug, anomalyco/opencode#28605) was writing `result=DONE` before the guard could invalidate it, so Cleanup miscounted it as a real attempt instead of the infra failure it actually is. - Every failure path now writes a short reason (implement-idle-timeout, verify-backend, review-fix-exhausted, ...) into the issue comment instead of a generic "attempt N failed" — no more opening the Actions log just to see which stage broke. - Verify — landing now installs Chromium before building, matching ci.yml's landing-build job (agent-loop's Verify could previously diverge from what real CI would say). - Added a Verify — pip-audit (CVE gate) step: confirmed via branch protection that pip-audit isn't a required context and agent-loop never checked it either, so a CVE-flagged dependency bump could previously auto-merge unblocked. Verified main is currently clean before wiring this in as a blocking step. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KpuHkuMadFm8SGgw7nuZki --- .github/agent/README.md | 14 ++++- .github/agent/pick-issue.sh | 2 +- .github/agent/run-engine.sh | 73 +++++++++++++++++++--- .github/workflows/agent-loop.yml | 101 +++++++++++++++++++++++++++---- scripts/setup-github.sh | 1 + 5 files changed, 166 insertions(+), 25 deletions(-) diff --git a/.github/agent/README.md b/.github/agent/README.md index 681a4f8..3279e73 100644 --- a/.github/agent/README.md +++ b/.github/agent/README.md @@ -86,9 +86,17 @@ step (no failures, no noise). - `agent:wip` — claimed by a running job. Orphaned `wip` (no run in progress, no open PR) means a cancelled run; the Orca watchdog clears it. -- `agent:failed` — one failed attempt; the picker will retry it. -- `agent:blocked` — two failed attempts; the picker skips it until a human - removes the label or closes the issue. +- `agent:failed` — one failed *real* attempt (BLOCKED verdict, verify red, + review cascade exhausted, push/PR failure); the picker will retry it. +- `agent:blocked` — two failed real attempts; the picker skips it until a + human removes the label or closes the issue. +- `agent:infra-stuck` — three consecutive infra failures (engine crash, + idle/hard-ceiling timeout, no `AGENT_RESULT` marker, DONE-but-empty-diff). + Infra failures never count toward `agent:failed`/`agent:blocked` — they + are logged as `` issue comments and escalate on their + own counter, so an issue that reliably times out doesn't retry forever, + 3x/day, with no human ever finding out. The picker skips it until a human + removes the label (usually after splitting the issue into a narrower one). - `agent-pr` — on every loop PR. Only one may be open at a time (branch protection runs `strict:false`); a red agent PR therefore PAUSES the loop until it is closed or fixed — that is intentional fail-safe behaviour. diff --git a/.github/agent/pick-issue.sh b/.github/agent/pick-issue.sh index 61311db..51920ab 100755 --- a/.github/agent/pick-issue.sh +++ b/.github/agent/pick-issue.sh @@ -27,7 +27,7 @@ set -euo pipefail ALLOWED_AUTHORS='["VforVitorio", "Santisoutoo"]' -EXCLUDED_LABELS='["epic", "agent:wip", "agent:blocked", "area: ci-cd", "question", "wontfix", "duplicate", "invalid"]' +EXCLUDED_LABELS='["epic", "agent:wip", "agent:blocked", "agent:infra-stuck", "area: ci-cd", "question", "wontfix", "duplicate", "invalid"]' # Implementation (worker), general issues — Chinese OSS models via OpenCode # Go, cheapest-first among flagship-class options. Fallback chain if kimi-k3's diff --git a/.github/agent/run-engine.sh b/.github/agent/run-engine.sh index f7e309a..07a5a0a 100755 --- a/.github/agent/run-engine.sh +++ b/.github/agent/run-engine.sh @@ -9,32 +9,85 @@ # output identically. Auth: opencode reads auth.json from XDG_DATA_HOME; # cursor-agent reads CURSOR_API_KEY from the environment. # -# Every invocation is bounded by ENGINE_TIMEOUT_SECONDS (default 900s): on -# 2026-08-24 a cursor-agent review call (run 32749623046, issue #85) hung +# Every invocation is bounded by an IDLE timeout, not a flat wall-clock one. +# On 2026-08-24 a cursor-agent review call (run 32749623046, issue #85) hung # with zero output for 84 minutes and was only reaped when the job's -# 90-minute cap killed the whole run. Neither engine had a call-level -# timeout, so one hung call silently burned the entire job budget instead -# of failing fast into the caller's infra-failure / model-fallback path. +# 90-minute cap killed the whole run — neither engine had a call-level +# timeout at all. A first fix (PR #102) added a flat `timeout`, but on +# 2026-08-25 that killed a call still actively producing output on a +# wide-scope issue (#52): a flat wall-clock cap can't tell "hung" from +# "slow but working". So a call is only killed after IDLE_TIMEOUT_SECONDS +# with NO NEW bytes on stdout/stderr (catches a real hang fast, regardless +# of total duration), with HARD_CEILING_SECONDS as an absolute backstop for +# a call that dribbles output forever without ever finishing. set -euo pipefail ENGINE="$1" MODEL="$2" PROMPT_FILE="$3" -TIMEOUT_SECONDS="${ENGINE_TIMEOUT_SECONDS:-900}" +IDLE_TIMEOUT_SECONDS="${ENGINE_IDLE_TIMEOUT_SECONDS:-300}" +HARD_CEILING_SECONDS="${ENGINE_HARD_CEILING_SECONDS:-900}" case "$ENGINE" in opencode) - # -k 30: if TERM doesn't stop it within 30s, send KILL — mirrors the - # orphan-process reap GitHub Actions had to do at the job-level timeout. - timeout -k 30 "$TIMEOUT_SECONDS" opencode run --model "$MODEL" "$(cat "$PROMPT_FILE")" + cmd=(opencode run --model "$MODEL" "$(cat "$PROMPT_FILE")") ;; cursor) # -p: non-interactive print mode; --force: skip the workspace-trust and # command-approval prompts (required headless). - timeout -k 30 "$TIMEOUT_SECONDS" cursor-agent -p --force --model "$MODEL" "$(cat "$PROMPT_FILE")" + cmd=(cursor-agent -p --force --model "$MODEL" "$(cat "$PROMPT_FILE")") ;; *) echo "::error::Unknown engine '$ENGINE'" >&2 exit 1 ;; esac + +out_file="$(mktemp)" +trap 'rm -f "$out_file"' EXIT + +# -k 30: if TERM doesn't stop it within the hard ceiling, send KILL 30s +# later — mirrors the orphan-process reap GitHub Actions does at its own +# job-level timeout. +timeout -k 30 "$HARD_CEILING_SECONDS" "${cmd[@]}" > "$out_file" 2>&1 & +runner_pid=$! + +# Relay output live so the caller's `tee` still sees it as it happens; +# stops on its own once the runner exits. +tail -n +1 -f "$out_file" --pid="$runner_pid" & +tail_pid=$! + +last_size=0 +last_change=$(date +%s) +idle_killed=0 +while kill -0 "$runner_pid" 2>/dev/null; do + sleep 5 + cur_size=$(stat -c %s "$out_file" 2>/dev/null || echo 0) + now=$(date +%s) + if [ "$cur_size" -ne "$last_size" ]; then + last_size=$cur_size + last_change=$now + elif [ $(( now - last_change )) -ge "$IDLE_TIMEOUT_SECONDS" ]; then + echo "::error::Engine idle ${IDLE_TIMEOUT_SECONDS}s with no new output — killing" >&2 + kill -TERM "$runner_pid" 2>/dev/null || true + pkill -TERM -P "$runner_pid" 2>/dev/null || true + sleep 10 + kill -KILL "$runner_pid" 2>/dev/null || true + pkill -KILL -P "$runner_pid" 2>/dev/null || true + idle_killed=1 + break + fi +done + +set +e +wait "$runner_pid" +rc=$? +set -e +wait "$tail_pid" 2>/dev/null || true + +if [ "$idle_killed" -eq 1 ]; then + # Distinct from `timeout`'s native 124 (hard-ceiling exit) so the caller + # can tell an idle-kill apart from a hard-ceiling kill in its own logs. + exit 125 +fi +exit "$rc" diff --git a/.github/workflows/agent-loop.yml b/.github/workflows/agent-loop.yml index 162bc0a..9e01fe1 100644 --- a/.github/workflows/agent-loop.yml +++ b/.github/workflows/agent-loop.yml @@ -189,6 +189,11 @@ jobs: # Model credential for the cursor engine (same exposure class as # opencode's auth.json on disk); GitHub-write creds stay excluded. CURSOR_API_KEY: ${{ steps.pick.outputs.engine == 'cursor' && secrets.CURSOR_API_KEY || '' }} + # Wide-scope issues genuinely need exploration room (2026-08-25, + # issue #52: killed mid-exploration by a flat 15min cap) — give + # Implement a generous idle window and hard ceiling. + ENGINE_IDLE_TIMEOUT_SECONDS: "360" + ENGINE_HARD_CEILING_SECONDS: "2400" run: | { cat .github/agent/worker-prompt.md @@ -212,26 +217,47 @@ jobs: result=$(grep -oE 'AGENT_RESULT: (ALREADY_DONE|DONE|BLOCKED)' "$RUNNER_TEMP/worker-output.txt" | tail -1 | cut -d' ' -f2 || true) commits=$(git rev-list --count origin/main..HEAD) echo "exit_code=$rc" >> "$GITHUB_OUTPUT" - echo "result=${result:-NONE}" >> "$GITHUB_OUTPUT" echo "commits=$commits" >> "$GITHUB_OUTPUT" + # `result` is written ONLY once we know its final, true value — it + # used to be written before the DONE-but-empty-diff guard below + # could still invalidate it, so Cleanup's infra-vs-attempt split + # (which keys off `result != DONE`) misclassified that guard + # failure as a real attempt instead of the infra failure it is. if [ "$rc" != "0" ]; then + case "$rc" in + 124) reason="implement-hard-ceiling" ;; + 125) reason="implement-idle-timeout" ;; + *) reason="implement-infra-other" ;; + esac + echo "result=NONE" >> "$GITHUB_OUTPUT" + echo "$reason" > "$RUNNER_TEMP/failure-reason.txt" echo "::error::opencode exited $rc (infra failure — not an attempt)" exit 1 fi case "$result" in DONE) # Empty-diff guard: headless opencode is known to exit silently - # with no work (anomalyco/opencode#28605). + # with no real work (anomalyco/opencode#28605) — this is the + # SAME infra-class failure the guard above catches, so it must + # never count as one of the 2 real attempts before agent:blocked. if [ "$commits" = "0" ] || [ -n "$(git status --porcelain)" ]; then - echo "::error::Worker said DONE but the tree is not a clean set of new commits." + echo "result=EMPTY_DIFF" >> "$GITHUB_OUTPUT" + echo "implement-infra-empty-diff" > "$RUNNER_TEMP/failure-reason.txt" + echo "::error::Worker said DONE but the tree is not a clean set of new commits (opencode#28605) — infra failure." exit 1 - fi ;; + fi + echo "result=DONE" >> "$GITHUB_OUTPUT" ;; ALREADY_DONE) + echo "result=ALREADY_DONE" >> "$GITHUB_OUTPUT" echo "Issue already implemented on main; will close it." ;; BLOCKED) + echo "result=BLOCKED" >> "$GITHUB_OUTPUT" + echo "implement-blocked" > "$RUNNER_TEMP/failure-reason.txt" echo "::error::Worker reported BLOCKED." exit 1 ;; *) + echo "result=NONE" >> "$GITHUB_OUTPUT" + echo "implement-infra-no-marker" > "$RUNNER_TEMP/failure-reason.txt" echo "::error::No AGENT_RESULT marker in worker output (infra failure — not an attempt)." exit 1 ;; esac @@ -261,6 +287,11 @@ jobs: OPENCODE_CONFIG: ${{ github.workspace }}/.github/agent/opencode.json ISSUE_TITLE: ${{ steps.pick.outputs.title }} CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} + # Tighter than Implement's: this is where the real 84min hang + # happened (run 32749623046, issue #85) — a review call reads a + # diff and judges it, it doesn't need Implement's exploration room. + ENGINE_IDLE_TIMEOUT_SECONDS: "180" + ENGINE_HARD_CEILING_SECONDS: "900" run: | compose_review_prompt() { { @@ -313,6 +344,7 @@ jobs: verdict=$(run_review) if [ "$verdict" = "APPROVE" ]; then exit 0; fi if [ "$verdict" != "FIX" ]; then + echo "review-no-verdict" > "$RUNNER_TEMP/failure-reason.txt" echo "::error::Reviewer emitted no parseable verdict from any model in the cascade (infra)." exit 1 fi @@ -325,9 +357,16 @@ jobs: echo sed -n '/VERDICT: FIX/,$p' "$RUNNER_TEMP/review-output.txt" } > "$RUNNER_TEMP/fix-prompt.txt" - bash .github/agent/run-engine.sh "${{ steps.pick.outputs.engine }}" "${{ steps.pick.outputs.model }}" "$RUNNER_TEMP/fix-prompt.txt" + # Real code work scoped to a numbered fix list, not open + # exploration — a bit more room than a plain review verdict, but + # not Implement's full 40min budget. Inline override: this call + # sits inside a step whose `env:` otherwise sets the tighter + # review-cascade values above. + ENGINE_IDLE_TIMEOUT_SECONDS=300 ENGINE_HARD_CEILING_SECONDS=1200 \ + bash .github/agent/run-engine.sh "${{ steps.pick.outputs.engine }}" "${{ steps.pick.outputs.model }}" "$RUNNER_TEMP/fix-prompt.txt" verdict=$(run_review) if [ "$verdict" != "APPROVE" ]; then + echo "review-fix-exhausted" > "$RUNNER_TEMP/failure-reason.txt" echo "::error::Reviewer still not satisfied after one fix round." exit 1 fi @@ -336,14 +375,27 @@ jobs: - name: Verify — backend if: steps.implement.outputs.result == 'DONE' run: | + trap 'echo verify-backend > "$RUNNER_TEMP/failure-reason.txt"' ERR uv run pytest -q -n auto --dist=loadfile uvx ruff check . uvx ruff format --check . uv run mypy src/lexflow/ + # pip-audit isn't a required branch-protection context, but the loop + # itself arms auto-merge — without this, an agent PR that introduces + # a CVE-flagged dependency could sail through Verify and land on main + # unblocked. Always-on like ci.yml's own pip-audit job (a docs-only + # PR must still catch a fresh upstream advisory). + - name: Verify — pip-audit (CVE gate) + if: steps.implement.outputs.result == 'DONE' + run: | + trap 'echo verify-pip-audit > "$RUNNER_TEMP/failure-reason.txt"' ERR + uv run --with pip-audit pip-audit --skip-editable + - name: Verify — frontend (only if touched) if: steps.implement.outputs.result == 'DONE' run: | + trap 'echo verify-frontend > "$RUNNER_TEMP/failure-reason.txt"' ERR if git diff --name-only origin/main..HEAD | grep -q '^frontend/'; then cd frontend && npm run lint && npm run test && npm run build else @@ -353,8 +405,11 @@ jobs: - name: Verify — landing (only if touched) if: steps.implement.outputs.result == 'DONE' run: | + trap 'echo verify-landing > "$RUNNER_TEMP/failure-reason.txt"' ERR if git diff --name-only origin/main..HEAD | grep -q '^landing/'; then - cd landing && npm install --no-audit --no-fund && npm run typecheck && npm run build + cd landing && npm install --no-audit --no-fund + npx playwright install --with-deps chromium + npm run typecheck && npm run build else echo "Diff does not touch landing/; skipping." fi @@ -367,6 +422,7 @@ jobs: GH_TOKEN: ${{ secrets.AGENT_GH_PAT }} ISSUE_TITLE: ${{ steps.pick.outputs.title }} run: | + trap 'echo push-pr > "$RUNNER_TEMP/failure-reason.txt"' ERR branch=$(git branch --show-current) git push "https://x-access-token:${GH_TOKEN}@github.com/${{ github.repository }}.git" "HEAD:refs/heads/$branch" { @@ -380,6 +436,7 @@ jobs: echo "- [x] \`uv run pytest\` verde en el runner del loop" echo "- [x] \`uvx ruff check .\` y \`uvx ruff format --check .\` repo-wide" echo "- [x] \`uv run mypy src/lexflow/\`" + echo "- [x] \`pip-audit\` (CVE gate)" echo "- [x] Frontend lint+test+build si el diff toca \`frontend/\`" echo echo "_Run: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}_" @@ -410,22 +467,44 @@ jobs: result="${{ steps.implement.outputs.result }}" implement_outcome="${{ steps.implement.outcome }}" + reason_file="$RUNNER_TEMP/failure-reason.txt" + if [ -f "$reason_file" ]; then + reason=$(cat "$reason_file") + elif [ "${{ job.status }}" = "cancelled" ]; then + # timeout-minutes cancels the job before a later step can write + # the file — Cleanup itself still runs (if: always()). + reason="job-timeout" + else + reason="unknown" + fi + # Success paths: PR armed, or issue closed as already done. if [ "$result" = "ALREADY_DONE" ]; then exit 0; fi if [ "$result" = "DONE" ] && [ "${{ job.status }}" = "success" ]; then exit 0; fi - # Infra failures don't count as attempts: opencode crashed, no - # marker, quota/gateway errors. + # Infra failures don't count as real attempts: opencode crashed, no + # marker, quota/gateway errors, DONE-but-empty-diff. They DO get + # their own counter, though — an issue that reliably infra-fails + # (e.g. genuinely too large for a single call) would otherwise + # retry forever, 3x/day, with no escalation path (found + # 2026-08-25 investigating issue #52's timeout). if [ "$implement_outcome" = "failure" ] && [ "$result" != "BLOCKED" ] && [ "$result" != "DONE" ]; then - gh issue comment "$n" --body " Agent loop infra failure (not an attempt): $RUN_URL" + infra_attempts=$(gh issue view "$n" --json comments \ + --jq '[.comments[].body | select(contains(""))] | length') + infra_attempts=$((infra_attempts + 1)) + gh issue comment "$n" --body " Agent loop infra failure $infra_attempts ($reason, not a real attempt): $RUN_URL" + if [ "$infra_attempts" -ge 3 ]; then + gh issue edit "$n" --add-label "agent:infra-stuck" || true + fi exit 0 fi - # Attempt failure: BLOCKED, verify red, publish red, or timeout. + # Attempt failure: BLOCKED, verify red, publish red, or review + # cascade exhausted. attempts=$(gh issue view "$n" --json comments \ --jq '[.comments[].body | select(contains(""))] | length') attempts=$((attempts + 1)) - gh issue comment "$n" --body " Agent loop attempt $attempts failed: $RUN_URL" + gh issue comment "$n" --body " Agent loop attempt $attempts failed ($reason): $RUN_URL" if [ "$attempts" -ge 2 ]; then gh issue edit "$n" --add-label "agent:blocked" --remove-label "agent:failed" || true else diff --git a/scripts/setup-github.sh b/scripts/setup-github.sh index f15d833..6ba19c4 100644 --- a/scripts/setup-github.sh +++ b/scripts/setup-github.sh @@ -77,6 +77,7 @@ LABELS=( "agent:wip|8250df|Issue currently claimed by an agent-loop run" "agent:failed|d4c5f9|Last agent-loop attempt failed; retry allowed" "agent:blocked|6f42c1|Two agent-loop attempts failed; needs a human" + "agent:infra-stuck|e99695|3+ infra failures in a row; probably too large for a single call" ) echo "==> Ensuring labels exist on ${REPO}"