From 03cfab1cbc6e255188308d0c48139581b0ec96c5 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Tue, 28 Jul 2026 18:08:48 -0700 Subject: [PATCH 01/13] fix(skills): make /fixer loop until the backlog is drained and park on stall, not fixed counts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes: - /fixer previously stopped after one batch of --count issues even when more qualifying open issues remained. It now loops across batches by default (Phase 3), re-fetching a fresh queue and continuing until no qualifying open issues remain or a 15-batch safety cap is hit. --once restores the old single-batch behavior. An empty queue is now reported as a clean stop (exit 0), with an explicit rule against wandering to other repos or inventing unrelated work to fill a batch. - Convergence rounds (2f) and drain passes (Phase 4) were capped at a fixed 3, parking/reporting a PR regardless of whether it was still making real progress. Both now track a per-round/per-pass progress signature and park only after 3 consecutive rounds/passes with zero measurable change (genuinely blocked), backed by a 15-round/15-pass safety cap for runaway cases. Fixes 2 bugs Greptile found on the first attempt at this change (#2165, reverted in #2167, Confidence Score 2/5): - The drain phase never actually removed a merged PR's number from parked.txt, so its length never shrank and a pass that successfully merged a PR was still recorded as making no progress — the drain could wrongly report the remaining PRs as blocked after 3 passes even though each pass was merging something. Step 3 (merge the lowest-ready PR) now explicitly removes it from parked.txt as part of the same step. - Phase 0's resume handling only checked whether state.json existed, so a fresh invocation after a fully-completed prior run inherited that run's stale batches-done count and empty queue.json, causing new issues to be silently skipped or the 15-batch cap to trigger prematurely. Resume now requires queue.json to be non-empty (real unfinished work); otherwise every run-scoped artifact is reset before starting. --- .claude/skills/fixer/SKILL.md | 266 +++++++++++++++++++++++++++------- 1 file changed, 213 insertions(+), 53 deletions(-) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index 4d4ee6c7..70913380 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -1,13 +1,17 @@ --- name: fixer -description: Solve the N lowest-numbered open issues end-to-end — one fresh branch per issue, PR, review convergence, and merge — then drain any stragglers in a final sweep phase -argument-hint: "[count] [--author ] [--start-from ] [--dry-run]" +description: Solve open issues end-to-end in batches of N — one fresh branch per issue, PR, review convergence, and merge — looping across batches until the whole backlog for --author is drained (or a safety cap is hit), then draining any stragglers in a final sweep phase. Never operates outside the current repo; if there is nothing to do, does nothing. +argument-hint: "[count] [--author ] [--start-from ] [--once] [--dry-run]" allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Skill, Monitor, Agent --- -# /fixer — Solve, Ship, and Merge a Batch of Issues +# /fixer — Solve, Ship, and Merge Until the Backlog Is Actually Done -Work the lowest-numbered open issues in this repository from open issue to merged PR, one at a time, until `count` issues (default 10) are done. Each issue gets its own branch cut from a freshly fetched `origin/main`, its own PR, its own review-convergence loop, and its own merge. Any PR that cannot be brought to a mergeable state inline is **parked** so the loop keeps moving, and Phase 4 drains all parked PRs with `/sweep` and `/resolve` until every one is merged or provably blocked. +Work the lowest-numbered open issues in **this repository only** from open issue to merged PR, one at a time, in batches of `count` (default 10). Each issue gets its own branch cut from a freshly fetched `origin/main`, its own PR, its own review-convergence loop, and its own merge. Any PR that cannot be brought to a mergeable state inline is **parked** so the batch keeps moving, and Phase: Drain Parked PRs drains all parked PRs with `/sweep` and `/resolve` until every one is merged or provably blocked. + +**The objective is the whole qualifying backlog, not one arbitrary batch.** Once a batch's issues are all merged/parked/abandoned and Phase: Drain Parked PRs has run, Phase: Continue the Batch Loop, or Proceed to Drain checks whether more open issues by `AUTHOR` still qualify. If they do, the next batch starts automatically. Pass `--once` to process a single batch and stop instead, regardless of what remains. + +**Repo scope.** /fixer only ever acts on the repo it is invoked in (detected dynamically in Phase 0 — on this checkout, `optave/ops-codegraph-tool`). It never searches, opens issues on, or opens PRs against any other repository, even when this repo currently has nothing to do. **An empty queue is a successful outcome, not a failure**: if there is nothing to do, Phase 1 reports that and stops — it does not look elsewhere, does not lower the `--author`/`blocked`-label bar to manufacture a queue, and does not invent unrelated work (stale files, refactors, drive-by fixes) to fill a batch. --- @@ -35,9 +39,10 @@ Parse `$ARGUMENTS` into these state variables, persisted to `.codegraph/fixer/` | Token | Variable | Default | Meaning | |-------|----------|---------|---------| -| first bare integer | `COUNT` | `10` | How many issues to take to merged state | +| first bare integer | `COUNT` | `10` | How many issues to process per batch. The run loops across batches (Phase: Continue the Batch Loop, or Proceed to Drain) until the backlog is drained, unless `--once` is set | | `--author ` | `AUTHOR` | `carlos-alm` | Only consider issues opened by this login | -| `--start-from ` | `START_FROM` | none | Skip queue entries below this issue number (resume) | +| `--start-from ` | `START_FROM` | none | Skip queue entries below this issue number (resume, or manual batch offset) | +| `--once` | `ONCE` | `false` | Process a single batch of `COUNT` issues and stop, even if more qualifying open issues remain | | `--dry-run` | `DRY_RUN` | `false` | Plan and analyse only — create no branch, commit, PR, or merge | ```bash @@ -61,12 +66,14 @@ START_FROM=$(printf '%s\n' "$ARGS" | grep -oE '\-\-start-from[= ]+[0-9]+' | head [ -z "$START_FROM" ] && START_FROM=0 case "$ARGS" in *--dry-run*) DRY_RUN=true ;; *) DRY_RUN=false ;; esac +case "$ARGS" in *--once*) ONCE=true ;; *) ONCE=false ;; esac printf '%s\n' "$COUNT" > .codegraph/fixer/count printf '%s\n' "$AUTHOR" > .codegraph/fixer/author printf '%s\n' "$START_FROM" > .codegraph/fixer/start-from printf '%s\n' "$DRY_RUN" > .codegraph/fixer/dry-run -echo "fixer: count=$COUNT author=$AUTHOR start-from=$START_FROM dry-run=$DRY_RUN" +printf '%s\n' "$ONCE" > .codegraph/fixer/once +echo "fixer: count=$COUNT author=$AUTHOR start-from=$START_FROM once=$ONCE dry-run=$DRY_RUN" ``` --- @@ -102,6 +109,8 @@ gh repo view --json nameWithOwner --jq '.nameWithOwner' > .codegraph/fixer/repo echo "fixer: operating on $(cat .codegraph/fixer/repo)" ``` +**This is the only repo /fixer touches for the rest of this run.** Every `gh` command in every phase below targets `$(cat .codegraph/fixer/repo)` exclusively. Do not `cd` into another repo, do not pass `--repo` with a different slug, and do not fall back to a different repository's issue queue for any reason — including an empty queue in Phase 1. Hardcoding the slug instead of reading it from this file is exactly the bug this skill avoids (see issue #2164 — the `sweep` skill hardcoded `optave/codegraph` while the real repo is `optave/ops-codegraph-tool` — for what that looks like when it goes wrong). + Detect the package manager once and persist the commands (Pattern 6 — never assume `npm`): ```bash @@ -132,16 +141,27 @@ case "$(git rev-parse --show-toplevel)" in esac ``` -**Resume handling.** If `.codegraph/fixer/state.json` already exists, this is a resumed run: report the previous outcome and continue from the first issue not marked `merged`. Otherwise initialise it. +**Resume handling.** A non-empty `queue.json` means a previous invocation stopped mid-batch (crash, interruption) with unresolved work — that is the ONLY case this treats as a resume. An absent or empty `queue.json` means the last invocation's last batch already ran to completion (Phase: Continue the Batch Loop, or Proceed to Drain always leaves `queue.json` empty when it decides to stop), so this is a genuinely new run and every run-scoped artifact — including `batches-done` and `queue.json` itself — must be reset. Conflating these two cases is exactly how a stale `batches-done` count or a stale empty `queue.json` leaks into a brand-new invocation, silently skipping newly opened issues or hitting the batch safety cap prematurely. ```bash mkdir -p .codegraph/fixer -if [ -f .codegraph/fixer/state.json ]; then - echo "fixer: resuming — previous state:" - jq -r '.issues[] | " #\(.issue) \(.status) \(if .pr then "PR #\(.pr)" else "no PR" end)"' .codegraph/fixer/state.json +# 2>/dev/null || echo 0: a missing or corrupt queue.json is treated as empty, which +# correctly routes to the "fresh run" branch rather than erroring +QUEUE_LEN=$(jq 'length' .codegraph/fixer/queue.json 2>/dev/null || echo 0) +if [ -f .codegraph/fixer/queue.json ] && [ "$QUEUE_LEN" -gt 0 ]; then + echo "fixer: resuming an interrupted batch — previous state:" + jq -r '.issues[] | " #\(.issue) \(.status) \(if .pr then "PR #\(.pr)" else "no PR" end)"' .codegraph/fixer/state.json 2>/dev/null else + if [ -f .codegraph/fixer/state.json ]; then + echo "fixer: previous run's artifacts are still on disk but its last batch already completed — that run is done. Starting a genuinely fresh run, not resuming it." + else + echo "fixer: fresh run — state initialised" + fi printf '%s\n' '{"issues":[]}' > .codegraph/fixer/state.json - echo "fixer: fresh run — state initialised" + rm -f .codegraph/fixer/queue.json .codegraph/fixer/parked.txt .codegraph/fixer/batches-done .codegraph/fixer/loop-decision \ + .codegraph/fixer/outcome .codegraph/fixer/round .codegraph/fixer/current-pr .codegraph/fixer/gate-fail \ + .codegraph/fixer/stall-count .codegraph/fixer/gate-signature .codegraph/fixer/prev-gate-signature \ + .codegraph/fixer/drain-pass .codegraph/fixer/drain-stall .codegraph/fixer/drain-parked-count fi ``` @@ -177,8 +197,9 @@ fi QUEUED=$(jq 'length' .codegraph/fixer/queue.json) if [ "$QUEUED" -eq 0 ]; then - echo "ERROR: no open issues by '$AUTHOR' at or above #$START_FROM — nothing to do" - exit 1 + echo "fixer: no open issues by '$AUTHOR' at or above #$START_FROM in $(cat .codegraph/fixer/repo) — nothing to do." + echo "fixer: this is the correct terminal state, not a failure. Do NOT search other repositories, do NOT lower the '$AUTHOR'/blocked-label bar to manufacture a queue, and do NOT invent unrelated work (stale files, refactors, drive-by fixes) to fill a batch. Stop here." + exit 0 fi if [ "$QUEUED" -lt "$COUNT" ]; then echo "WARN: only $QUEUED open issues available (requested $COUNT) — the batch will be short" @@ -256,13 +277,15 @@ DRY_RUN=$(cat .codegraph/fixer/dry-run) ISSUE=$(jq -r '.[0].issue' .codegraph/fixer/queue.json) BRANCH="fix/issue-$ISSUE" -# A fresh branch means a fresh PR lifecycle. Clear any outcome/round/current-pr/gate-fail -# left behind by an interrupted earlier attempt on this same issue — e.g. a crash after -# convergence hit the round cap and wrote outcome=parked, but before 2g recorded it in -# state.json and cleared these files (the dedup loop above only filters on state.json, so -# that crash window still lands here). Without this, the new attempt would inherit a -# stale "parked" verdict or an inflated round count before it has even opened its own PR. -rm -f .codegraph/fixer/outcome .codegraph/fixer/round .codegraph/fixer/current-pr .codegraph/fixer/gate-fail +# A fresh branch means a fresh PR lifecycle. Clear any outcome/round/current-pr/gate-fail/ +# stall state left behind by an interrupted earlier attempt on this same issue — e.g. a +# crash after convergence hit the park threshold and wrote outcome=parked, but before 2g +# recorded it in state.json and cleared these files (the dedup loop above only filters on +# state.json, so that crash window still lands here). Without this, the new attempt would +# inherit a stale "parked" verdict, an inflated round count, or a stall counter/signature +# from the previous issue's PR before it has even opened its own. +rm -f .codegraph/fixer/outcome .codegraph/fixer/round .codegraph/fixer/current-pr .codegraph/fixer/gate-fail \ + .codegraph/fixer/stall-count .codegraph/fixer/gate-signature .codegraph/fixer/prev-gate-signature if ! printf '%s\n' "$BRANCH" | grep -qE '^(feat|fix|docs|refactor|test|chore|ci|perf|build|release|dependabot|revert)/'; then echo "ERROR: branch '$BRANCH' fails the repo's Validate branch name check"; exit 1 @@ -432,7 +455,7 @@ fi ### 2f. Converge the PR and merge it (invariant I2) -A PR merges only when **all five** gate conditions hold. Evaluate them together; if any is false, fix that specific condition and re-evaluate. Cap at **3 convergence rounds** — beyond that, park the PR and move on (I6). +A PR merges only when **all five** gate conditions hold. Evaluate them together; if any is false, fix that specific condition and re-evaluate. **Do not cap this on a fixed round count** — some PRs genuinely need many rounds of real back-and-forth with reviewers. Instead, park the PR only when it is **blocked** (I6): 3 consecutive rounds where the gate signature (below) did not change at all, meaning nothing about the PR's state moved between rounds despite your fixes. As long as each round changes something — a comment answered, a check turning green, the Greptile score moving — keep going. A 15-round absolute cap exists purely as a backstop against a genuinely runaway loop (e.g. a reviewer bot stuck in a reply loop); hitting it is itself worth flagging as unusual in the final report, not a normal outcome. | # | Condition | Check | |---|-----------|-------| @@ -552,9 +575,17 @@ fi printf '%s\n' "$GATE_FAIL" > .codegraph/fixer/gate-fail [ "$GATE_FAIL" -eq 0 ] && echo "fixer: PR #$PR passes all five gate conditions" || echo "fixer: PR #$PR is not mergeable yet" + +# A compact fingerprint of everything this round measured. Two consecutive rounds with an +# IDENTICAL signature mean nothing about the PR's state moved despite whatever was fixed +# in between — that is the actual definition of "blocked" this skill parks on, not an +# arbitrary round count. Order the fields consistently so an unrelated field re-ordering +# never masquerades as a change. +printf '%s\n' "score=${SCORE:-none};unanswered=$UNANSWERED;g3=$(git merge-base --is-ancestor origin/main HEAD && echo ok || echo behind);mergeable=$MERGEABLE;failed=$FAILED_CHECKS" \ + > .codegraph/fixer/gate-signature ``` -Track convergence rounds and park once the cap is hit (I6). This is the only place `outcome` is set to `parked`: +Track convergence rounds and park only when the PR is genuinely **blocked** (I6) — never on a fixed round count. This is the only place `outcome` is set to `parked`: ```bash mkdir -p .codegraph/fixer @@ -562,14 +593,36 @@ GATE_FAIL=$(cat .codegraph/fixer/gate-fail) # 2>/dev/null: the round file is expected to be absent on the first convergence round — || supplies the starting count of 0 ROUND=$(( $(cat .codegraph/fixer/round 2>/dev/null || echo 0) + 1 )) printf '%s\n' "$ROUND" > .codegraph/fixer/round + +CURRENT_SIG=$(cat .codegraph/fixer/gate-signature) +# 2>/dev/null: prev-gate-signature is expected to be absent on round 1 — an empty PREV_SIG +# just means "no prior round to compare against", never treated as a false match below +PREV_SIG=$(cat .codegraph/fixer/prev-gate-signature 2>/dev/null || echo "") +printf '%s\n' "$CURRENT_SIG" > .codegraph/fixer/prev-gate-signature + if [ "$GATE_FAIL" -eq 0 ]; then - rm -f .codegraph/fixer/round -elif [ "$ROUND" -ge 3 ]; then - printf '%s\n' "parked" > .codegraph/fixer/outcome - rm -f .codegraph/fixer/round - echo "fixer: convergence round $ROUND/3 exhausted — PR #$(cat .codegraph/fixer/current-pr) will be parked (I6)" + rm -f .codegraph/fixer/round .codegraph/fixer/stall-count .codegraph/fixer/prev-gate-signature .codegraph/fixer/gate-signature +elif [ -n "$PREV_SIG" ] && [ "$CURRENT_SIG" = "$PREV_SIG" ]; then + # 2>/dev/null: stall-count is expected to be absent before the first stalled round + STALL=$(( $(cat .codegraph/fixer/stall-count 2>/dev/null || echo 0) + 1 )) + printf '%s\n' "$STALL" > .codegraph/fixer/stall-count + echo "fixer: round $ROUND made no measurable progress vs. the previous round (stall $STALL/3)" else - echo "fixer: convergence round $ROUND/3 — re-evaluate after fixing the failing condition(s)" + printf '%s\n' 0 > .codegraph/fixer/stall-count + echo "fixer: round $ROUND changed the PR's state vs. the previous round — real progress, resetting stall counter" +fi + +STALL=$(cat .codegraph/fixer/stall-count 2>/dev/null || echo 0) +if [ "$GATE_FAIL" -ne 0 ] && { [ "$STALL" -ge 3 ] || [ "$ROUND" -ge 15 ]; }; then + printf '%s\n' "parked" > .codegraph/fixer/outcome + rm -f .codegraph/fixer/round .codegraph/fixer/stall-count .codegraph/fixer/prev-gate-signature .codegraph/fixer/gate-signature + if [ "$STALL" -ge 3 ]; then + echo "fixer: parking PR #$(cat .codegraph/fixer/current-pr) — blocked (3 consecutive rounds with zero measurable progress)" + else + echo "fixer: parking PR #$(cat .codegraph/fixer/current-pr) — 15-round safety cap reached despite ongoing progress; flag this as unusual in the final report" + fi +elif [ "$GATE_FAIL" -ne 0 ]; then + echo "fixer: round $ROUND — re-evaluate after fixing the failing condition(s)" fi ``` @@ -594,7 +647,7 @@ GATE_FAIL=$(cat .codegraph/fixer/gate-fail) # 2>/dev/null: outcome is expected to be absent before a PR's first convergence round — an empty OUTCOME just skips the "already parked" branch below OUTCOME=$(cat .codegraph/fixer/outcome 2>/dev/null) if [ "$OUTCOME" = "parked" ]; then - echo "fixer: PR #$PR already parked after 3 convergence rounds — not merging" + echo "fixer: PR #$PR already parked (blocked or safety cap reached) — not merging" elif [ "$GATE_FAIL" -ne 0 ]; then echo "fixer: gate not satisfied — fix the failing condition or park PR #$PR. Not merging." elif [ "$DRY_RUN" = "true" ]; then @@ -656,7 +709,8 @@ TMP_QUEUE=$(mktemp "${TMPDIR:-/tmp}/fixer-queue.XXXXXXXXXX") trap 'rm -f "$TMP_QUEUE"' EXIT jq '.[1:]' .codegraph/fixer/queue.json > "$TMP_QUEUE" && mv "$TMP_QUEUE" .codegraph/fixer/queue.json trap - EXIT -rm -f .codegraph/fixer/current-pr .codegraph/fixer/gate-fail .codegraph/fixer/outcome .codegraph/fixer/round +rm -f .codegraph/fixer/current-pr .codegraph/fixer/gate-fail .codegraph/fixer/outcome .codegraph/fixer/round \ + .codegraph/fixer/stall-count .codegraph/fixer/gate-signature .codegraph/fixer/prev-gate-signature echo "fixer: issue #$ISSUE recorded as $STATUS; $(jq 'length' .codegraph/fixer/queue.json) remaining" ``` @@ -667,11 +721,66 @@ Loop back to step 2a for the next queue entry. **Only after the current PR is me --- -## Phase 3 — Drain Parked PRs +## Phase 3 — Continue the Batch Loop, or Proceed to Drain + +Phase 1 and Phase 2 together process exactly one batch of up to `COUNT` issues. **The run's objective is the whole qualifying backlog, not one batch** — this phase decides whether more work remains before moving on to drain and report. + +Stop looping and go straight to Phase: Drain Parked PRs if `ONCE` or `DRY_RUN` is `true` — a single batch is the entire run in either case: + +```bash +mkdir -p .codegraph/fixer +# 2>/dev/null: once is expected to be absent on a fixer version predating this flag — || supplies the safe default +ONCE=$(cat .codegraph/fixer/once 2>/dev/null || echo false) +DRY_RUN=$(cat .codegraph/fixer/dry-run) +if [ "$ONCE" = "true" ] || [ "$DRY_RUN" = "true" ]; then + echo "fixer: --once or --dry-run set — this batch is the whole run, proceeding to drain" + printf '%s\n' "stop" > .codegraph/fixer/loop-decision +else + REPO=$(cat .codegraph/fixer/repo) + AUTHOR=$(cat .codegraph/fixer/author) + # One past the highest issue number this run has recorded, so the next batch's queue + # never re-examines an issue this run already marked merged/parked/abandoned. + NEXT_START=$(( $(jq '[.issues[].issue] | max // 0' .codegraph/fixer/state.json) + 1 )) + REMAINING=$(gh issue list --repo "$REPO" --state open --limit 400 \ + --json number,labels,author \ + --jq "[.[] | select(.author.login==\"$AUTHOR\") | select([.labels[].name]|index(\"blocked\")|not) | select(.number >= $NEXT_START)] | length") + + # 2>/dev/null: batches-done is expected to be absent before the first batch completes + BATCHES_DONE=$(( $(cat .codegraph/fixer/batches-done 2>/dev/null || echo 0) + 1 )) + printf '%s\n' "$BATCHES_DONE" > .codegraph/fixer/batches-done + + if [ "$REMAINING" -eq 0 ]; then + echo "fixer: no more qualifying open issues at or above #$NEXT_START — backlog drained after $BATCHES_DONE batch(es)" + printf '%s\n' "stop" > .codegraph/fixer/loop-decision + elif [ "$BATCHES_DONE" -ge 15 ]; then + echo "fixer: safety cap of 15 batches reached with $REMAINING qualifying issue(s) still open at or above #$NEXT_START" + echo "fixer: resume with: /fixer --author $AUTHOR --start-from $NEXT_START" + printf '%s\n' "stop" > .codegraph/fixer/loop-decision + else + echo "fixer: $REMAINING more qualifying issue(s) open at or above #$NEXT_START — starting batch $((BATCHES_DONE + 1))" + # Clear per-batch scratch state so Phase 1 builds a genuinely fresh queue. state.json + # and parked.txt are deliberately NOT cleared here — they accumulate across every + # batch in this run so Phase: Drain Parked PRs and the final report cover the whole + # run, not just the most recent batch. + rm -f .codegraph/fixer/queue.json .codegraph/fixer/current-pr .codegraph/fixer/gate-fail \ + .codegraph/fixer/outcome .codegraph/fixer/round + printf '%s\n' "$NEXT_START" > .codegraph/fixer/start-from + printf '%s\n' "loop" > .codegraph/fixer/loop-decision + fi +fi +``` + +If `.codegraph/fixer/loop-decision` is `loop`, go back to Phase 1 and process another batch — do **not** re-run Phase 0 (the repo slug, test/lint commands, and `state.json` are already established for this run, and re-running Phase 0's resume-detection would misread the mid-run `state.json` as a crash to resume from). If it is `stop`, proceed to Phase: Drain Parked PRs. + +**Exit condition:** `.codegraph/fixer/loop-decision` is `loop` (with a fresh `start-from` persisted and per-batch scratch state cleared) or `stop`. Every batch processed this run is recorded cumulatively in `state.json` — nothing from an earlier batch is lost or overwritten by a later one. + +--- + +## Phase 4 — Drain Parked PRs Skip this phase entirely if `.codegraph/fixer/parked.txt` is absent or empty — on a clean run, Phase: Solve and Merge Loop merges everything inline and there is nothing to drain. -This is the mode switch: stop solving new issues, and work the parked PRs as a set until each is merged or provably blocked. Run at most **3 passes**; report anything still unmerged afterwards as needing human review rather than looping forever. +This is the mode switch: stop solving new issues, and work the parked PRs as a set until each is merged or provably blocked. **Do not cap this at a fixed pass count** — keep running passes as long as each one makes progress. Stop only after 3 consecutive passes that merge nothing (blocked), or a 15-pass absolute safety cap; report anything still unmerged afterwards as needing human review. ```bash mkdir -p .codegraph/fixer @@ -687,7 +796,46 @@ Each pass does three things, in order: 1. **Sweep.** Invoke `/sweep` once. It processes every open PR in parallel: resolves conflicts, fixes CI, mines the Greptile summary as well as inline comments, addresses and replies to all reviewer feedback, and re-triggers reviewers. Do not re-derive that procedure here. 2. **Resolve.** For any parked PR still reporting `mergeable != MERGEABLE`, invoke `/resolve ` on it individually. `/resolve` reads both sides' intent before choosing, and stops rather than guessing on genuinely ambiguous conflicts. -3. **Merge the lowest-numbered ready PR first**, then re-evaluate. Merging lowest-first matches the requested order and keeps the catch-up cost predictable. +3. **Merge the lowest-numbered ready PR first**, then re-evaluate. Merging lowest-first matches the requested order and keeps the catch-up cost predictable. **Immediately remove that PR's number from `parked.txt`** — this is what makes the progress check below meaningful; without it, `parked.txt` never shrinks and every pass looks stalled even when merges are happening: + ```bash + mkdir -p .codegraph/fixer + # MERGED_PR is the PR number just merged in this step + grep -vxF "$MERGED_PR" .codegraph/fixer/parked.txt > .codegraph/fixer/parked.txt.tmp \ + && mv .codegraph/fixer/parked.txt.tmp .codegraph/fixer/parked.txt + echo "fixer: removed merged PR #$MERGED_PR from parked.txt — $(wc -l < .codegraph/fixer/parked.txt) remaining" + ``` + +**After each pass**, record whether it made progress and decide whether to run another one: + +```bash +mkdir -p .codegraph/fixer +# 2>/dev/null: expected to be absent before drain's first pass +PARKED_BEFORE=$(cat .codegraph/fixer/drain-parked-count 2>/dev/null || wc -l < .codegraph/fixer/parked.txt) +PARKED_AFTER=$(wc -l < .codegraph/fixer/parked.txt) +DRAIN_PASS=$(( $(cat .codegraph/fixer/drain-pass 2>/dev/null || echo 0) + 1 )) +printf '%s\n' "$DRAIN_PASS" > .codegraph/fixer/drain-pass + +if [ "$PARKED_AFTER" -eq 0 ]; then + echo "fixer: all parked PRs merged after $DRAIN_PASS pass(es)" + rm -f .codegraph/fixer/drain-pass .codegraph/fixer/drain-stall .codegraph/fixer/drain-parked-count +elif [ "$PARKED_AFTER" -lt "$PARKED_BEFORE" ]; then + echo "fixer: pass $DRAIN_PASS merged $((PARKED_BEFORE - PARKED_AFTER)) PR(s) — progress, resetting drain-stall" + printf '%s\n' 0 > .codegraph/fixer/drain-stall +else + # 2>/dev/null: expected to be absent before the first stalled pass + DRAIN_STALL=$(( $(cat .codegraph/fixer/drain-stall 2>/dev/null || echo 0) + 1 )) + printf '%s\n' "$DRAIN_STALL" > .codegraph/fixer/drain-stall + echo "fixer: pass $DRAIN_PASS merged nothing (drain-stall $DRAIN_STALL/3)" +fi +printf '%s\n' "$PARKED_AFTER" > .codegraph/fixer/drain-parked-count + +DRAIN_STALL=$(cat .codegraph/fixer/drain-stall 2>/dev/null || echo 0) +if [ "$PARKED_AFTER" -gt 0 ] && { [ "$DRAIN_STALL" -ge 3 ] || [ "$DRAIN_PASS" -ge 15 ]; }; then + echo "fixer: stopping drain — $([ "$DRAIN_STALL" -ge 3 ] && echo "3 consecutive passes with no merges (blocked)" || echo "15-pass safety cap reached"). $PARKED_AFTER PR(s) remain: report each as needing human review." +elif [ "$PARKED_AFTER" -gt 0 ]; then + echo "fixer: running another pass ($((DRAIN_PASS + 1)))" +fi +``` ### The catch-up merge, and the check that protects solved work (invariant I4) @@ -777,13 +925,13 @@ A reported line is not automatically a bug — `main` may have legitimately supe After the integrity check passes, re-run local verification and push, then re-evaluate the five gate conditions from Phase: Solve and Merge Loop before merging. -**Exit condition:** Every PR listed in `parked.txt` is merged, or is reported as needing human review with a specific reason. No catch-up merge was pushed without a passing I4 integrity check. At most 3 drain passes were run. +**Exit condition:** Every PR listed in `parked.txt` is merged, or is reported as needing human review with a specific reason. No catch-up merge was pushed without a passing I4 integrity check. Drain stopped only once a pass merged nothing 3 times in a row, or the 15-pass safety cap was hit — never on a fixed pass count while merges were still happening. --- -## Phase 4 — Final Report +## Phase 5 — Final Report -Report honestly. If something was skipped, left unmerged, bypassed, or split into a follow-up, say so explicitly. +Report honestly, across **every batch this run processed** (Phase 3 loops back to Phase 1 as long as qualifying issues remain, so `state.json` may cover several batches, not just the last one). If something was skipped, left unmerged, bypassed, or split into a follow-up, say so explicitly. ```bash mkdir -p .codegraph/fixer @@ -794,7 +942,8 @@ if [ "$TOTAL" -gt 0 ]; then else PCT=0 # no issues attempted — avoid dividing by zero fi -echo "fixer: $MERGED/$TOTAL issues merged (${PCT}%)" +BATCHES_DONE=$(cat .codegraph/fixer/batches-done 2>/dev/null || echo 1) +echo "fixer: $MERGED/$TOTAL issues merged (${PCT}%) across $BATCHES_DONE batch(es)" jq -r '.issues[] | " #\(.issue) \(.status) \(if .pr then "PR #\(.pr)" else "no PR" end)"' .codegraph/fixer/state.json ``` @@ -810,9 +959,9 @@ Also list, explicitly: - every issue that was split, with the follow-up covering the remainder - anything left for a human, with the specific blocker -**Cleanup.** State lives in `.codegraph/fixer/`. It is deliberately kept after a successful run so a later `/fixer` can report on it and so a crashed run can resume. Remove it with `rm -rf .codegraph/fixer` to force a completely fresh batch. +**Cleanup.** State lives in `.codegraph/fixer/`. It is left on disk after this run finishes, but Phase 0's resume handling already treats a completed run's artifacts as stale on the *next* invocation — an empty `queue.json` means the next `/fixer` call starts genuinely fresh, not "resuming" this run's results. The only thing left on disk that matters afterward is `state.json` as a historical record of what this run did; `rm -rf .codegraph/fixer` removes it entirely if you don't want even that. -**Exit condition:** The user has a per-issue table, the merged count, every follow-up issue number, every bypass with its justification, and an explicit list of anything unfinished. +**Exit condition:** The user has a per-issue table covering every batch this run processed, the merged count, the batch count, every follow-up issue number, every bypass with its justification, and an explicit list of anything unfinished (including a resume command if the 15-batch safety cap was hit). --- @@ -822,26 +971,33 @@ All state is under `.codegraph/fixer/`: | File | Format | Purpose | |------|--------|---------| -| `repo` | text | `owner/name` slug | -| `count`, `author`, `start-from`, `dry-run` | text | parsed arguments | +| `repo` | text | `owner/name` slug — the only repo this run ever touches | +| `count`, `author`, `start-from`, `once`, `dry-run` | text | parsed arguments | | `test-cmd`, `lint-cmd` | text | detected package-manager commands | -| `queue.json` | JSON array of `{issue, title}` | remaining work, ascending; entries are shifted off as they complete | -| `state.json` | `{"issues":[{issue, status, pr}]}` | per-issue outcome; drives resume | -| `parked.txt` | newline-separated PR numbers | input to Phase: Drain Parked PRs | +| `queue.json` | JSON array of `{issue, title}` | current batch's remaining work, ascending; entries are shifted off as they complete. **A present-but-empty `queue.json` means the last batch fully completed** — Phase 0 uses exactly this signal to decide fresh-run vs. resume | +| `state.json` | `{"issues":[{issue, status, pr}]}` | per-issue outcome, accumulated across every batch this run has processed; reset to empty whenever Phase 0 detects a fresh run | +| `parked.txt` | newline-separated PR numbers | accumulated across every batch; input to Phase: Drain Parked PRs; a merged PR's number is removed from this file the moment it merges | +| `batches-done` | text | count of batches completed this run; read by Phase 3 and the final report; reset on a fresh run | +| `loop-decision` | text | `loop`/`stop` — Phase 3's verdict on whether to start another batch | | `current-pr`, `last-pr-url`, `gate-fail` | text | current iteration's scratch state | | `outcome` | text | `abandoned`/`merged`/`parked` for the issue in progress; read by 2g, cleared after recording | | `round` | text | convergence-round counter for the current PR; cleared once it merges or parks | +| `gate-signature`, `prev-gate-signature` | text | fingerprint of this round's / the previous round's gate state, used to detect a stalled (blocked) PR rather than counting rounds | +| `stall-count` | text | consecutive convergence rounds with an unchanged gate signature; park threshold is 3 | +| `drain-pass`, `drain-stall`, `drain-parked-count` | text | drain-phase pass counter, consecutive no-merge passes, and `parked.txt` length as of the last pass — the same stall-detection pattern applied to draining | | `authored-lines.tsv`, `authored-files.txt` | text | I4 integrity-check baseline (tab-separated `filecountline`) | --- ## Examples -- `/fixer` — take the 10 lowest-numbered open issues by `carlos-alm` to merged, one at a time. -- `/fixer 3` — same, but only the 3 lowest-numbered issues. Good for a first run to confirm the loop behaves. -- `/fixer --dry-run` — print the queue and the plan for all 10 without creating a branch, commit, PR, or merge. -- `/fixer 5 --start-from 1900` — the 5 lowest-numbered open issues at or above #1900. -- `/fixer --author someone-else` — run the batch against another author's issues. +- `/fixer` — work through every open issue by `carlos-alm`, 10 at a time, looping across batches until none remain (or the 15-batch safety cap is hit). +- `/fixer 3` — same, but 3 issues per batch. Good for a first run to confirm the loop behaves before letting it run unattended. +- `/fixer 3 --once` — process exactly 3 issues and stop, even if more are open. Use this to sanity-check a single batch. +- `/fixer --dry-run` — print the queue and the plan for one batch of 10 without creating a branch, commit, PR, or merge (dry runs never loop past one batch). +- `/fixer 5 --start-from 1900` — starting at #1900, work 5 issues per batch until the backlog above that point is drained. +- `/fixer --author someone-else` — run against another author's issues instead of the default. +- If there are no open issues matching `--author` (and not `blocked`), /fixer reports that plainly and exits — it does not search other repos or invent work. --- @@ -863,9 +1019,13 @@ All state is under `.codegraph/fixer/`: - **Branch names must match** `^(feat|fix|docs|refactor|test|chore|ci|perf|build|release|dependabot|revert)/`; commits must be Conventional Commits with a header ≤ 100 characters. - **Never silently skip verification.** If lint, tests, or a build cannot run or fail for any reason, stop and report to the user. Do not decide on their behalf to proceed. - **Never document a bug as expected behaviour.** Engine divergence is a bug in the less accurate engine — fix the root cause. -- **Park, don't stall (I6).** After 3 convergence rounds, park the PR and move to the next issue. Phase: Drain Parked PRs handles it. -- **Bounded loops only.** 3 convergence rounds per PR, 3 drain passes. Then report for human review. +- **Park when blocked, not on a fixed round count (I6).** Keep converging as long as each round changes the gate signature — a comment answered, a check going green, the Greptile score moving. Park only after 3 consecutive rounds with zero measurable change (genuinely blocked), or the 15-round absolute safety cap. Then move to the next issue; Phase: Drain Parked PRs handles it. +- **Bounded loops only, but bounded by progress, not by an arbitrary count where it matters.** Convergence rounds and drain passes are both capped by stall detection (3 consecutive rounds/passes with zero measurable progress) with a 15-round / 15-pass safety backstop; the whole run is capped at 15 batches. Then report for human review or resume. +- **The run's objective is the whole backlog, not one batch.** By default, keep starting new batches (Phase 3) until no qualifying open issues remain or the 15-batch cap is hit. Pass `--once` to process exactly one batch and stop. +- **Only ever act on the repo detected in Phase 0.** Never search, open issues on, or open PRs against any other repository — including when this repo's queue comes up empty. Read the slug from `.codegraph/fixer/repo`; never hardcode it (see issue #2164). +- **An empty queue is success, not failure.** If Phase 1 finds no qualifying open issues, report that plainly and exit 0 — never search other repos, never lower the `--author`/`blocked`-label bar to manufacture a queue, and never invent unrelated work (stale files, refactors, drive-by fixes) to fill a batch. +- **A completed run is not a resumable run.** Phase 0 only resumes when `queue.json` is non-empty (real unfinished work). An empty or absent `queue.json` means the previous batch already finished, so every run-scoped artifact (`batches-done`, `queue.json`, `parked.txt`, `state.json`, per-issue scratch state) is reset before starting — never let a finished run's counters leak into a new invocation and skip issues or hit the batch cap early. - **No co-author lines** in commit messages or PR bodies. - **No Claude Code or Anthropic references** in commits, PR bodies, comments, or issues. - **Never trigger `@greptileai` until every Greptile comment has a reply.** Do not trigger Greptile for feedback that came from Claude or the user. -- **Report faithfully.** State what merged, what parked, what was bypassed and why, what was split, and what needs a human. +- **Report faithfully.** State what merged, what parked, what was bypassed and why, what was split, what needs a human, and how many batches the run took. From c06a69f2a630c4d1f3da8c2cb47ac6c0ef76abe6 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Tue, 28 Jul 2026 18:54:56 -0700 Subject: [PATCH 02/13] fix: reconcile state.json when Phase 4 drain merges a parked PR (#2169) Merging a parked PR during the drain phase only removed its number from parked.txt; the corresponding state.json entry stayed status="parked", so Phase 5's final report undercounted merged issues and reported completed work as unfinished. Flip the entry to "merged" (matched by PR number) in the same step that removes it from parked.txt. --- .claude/skills/fixer/SKILL.md | 58 +++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index 70913380..6bd45f25 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -184,10 +184,14 @@ START_FROM=$(cat .codegraph/fixer/start-from) if [ -f .codegraph/fixer/queue.json ]; then echo "fixer: reusing existing queue (Pattern 12 — artifact reuse). Delete .codegraph/fixer/queue.json to rebuild." else - gh issue list --state open --limit 400 \ + # --author filters server-side so the --limit ceiling bounds this author's own backlog, + # not the repo's entire open-issue count (gh issue list defaults to --limit 30 with no + # server-side author filter, which previously meant issues by other authors could crowd + # this author's own low-numbered issues out of the fetched window). 1000 is a deliberately + # generous safety bound on one author's queue, not a realistic ceiling to ever hit. + gh issue list --state open --author "$AUTHOR" --limit 1000 \ --json number,title,labels,author \ - --jq "[.[] | select(.author.login==\"$AUTHOR\") - | select([.labels[].name] | index(\"blocked\") | not) + --jq "[.[] | select([.labels[].name] | index(\"blocked\") | not) | {issue: .number, title: .title}] | sort_by(.issue) | map(select(.issue >= ($START_FROM|tonumber))) @@ -508,11 +512,18 @@ fi # unanswered Greptile inline comments. --- ALL_COMMENTS=$(gh api "repos/$REPO/pulls/$PR/comments" --paginate) UNANSWERED=0 +# Identities of the currently-unanswered comments, not just their count — two rounds can +# both show "2 unanswered" while the actual pair differs (one answered, a new one landed), +# which a bare count would wrongly read as zero movement. Folded into the gate signature below. +UNANSWERED_IDS="" for CID in $(printf '%s\n' "$ALL_COMMENTS" | jq -r '[.[] | select(.in_reply_to_id == null)] | .[].id'); do ORIGIN_USER=$(printf '%s\n' "$ALL_COMMENTS" | jq -r --argjson cid "$CID" '[.[] | select(.id == $cid)][0].user.login') REPLIES=$(printf '%s\n' "$ALL_COMMENTS" | jq -s --argjson cid "$CID" --arg origin "$ORIGIN_USER" \ '[.[][] | select(.in_reply_to_id == $cid) | select(.user.login != $origin)] | length') - [ "$REPLIES" -eq 0 ] && UNANSWERED=$((UNANSWERED + 1)) + if [ "$REPLIES" -eq 0 ]; then + UNANSWERED=$((UNANSWERED + 1)) + UNANSWERED_IDS="$UNANSWERED_IDS,$CID" + fi done # Top-level review bodies (Claude's CHANGES_REQUESTED/COMMENT summary, Greptile's @@ -581,7 +592,20 @@ printf '%s\n' "$GATE_FAIL" > .codegraph/fixer/gate-fail # in between — that is the actual definition of "blocked" this skill parks on, not an # arbitrary round count. Order the fields consistently so an unrelated field re-ordering # never masquerades as a change. -printf '%s\n' "score=${SCORE:-none};unanswered=$UNANSWERED;g3=$(git merge-base --is-ancestor origin/main HEAD && echo ok || echo behind);mergeable=$MERGEABLE;failed=$FAILED_CHECKS" \ +# +# The aggregate score, unanswered count, mergeability, and failing-check names alone are +# too coarse: a new commit, a fresh Greptile review, or a new CI run can all leave every +# one of those aggregates reading identical to the previous round (score lags a push, +# a re-review restates the same score for different reasons, a rerun fails the same named +# check for a different cause) — which would misread real activity as a stall. `commit` +# (the pushed SHA) and `greptile_hash` (a hash of every Greptile comment/review body, not +# just the extracted digit) both change the instant that activity happens, even before the +# aggregates catch up, so genuine progress is never mistaken for zero measurable change. +# `unanswered_ids` likewise tracks which comments are outstanding, not just how many, so a +# reply that resolves one comment while a new one lands does not cancel out to "no change". +GREPTILE_HASH=$(printf '%s' "$GREP_BODY" | cksum | awk '{print $1}') +COMMIT=$(git rev-parse HEAD) +printf '%s\n' "score=${SCORE:-none};greptile_hash=$GREPTILE_HASH;unanswered=$UNANSWERED;unanswered_ids=${UNANSWERED_IDS:-none};g3=$(git merge-base --is-ancestor origin/main HEAD && echo ok || echo behind);mergeable=$MERGEABLE;failed=$FAILED_CHECKS;commit=$COMMIT" \ > .codegraph/fixer/gate-signature ``` @@ -741,9 +765,13 @@ else # One past the highest issue number this run has recorded, so the next batch's queue # never re-examines an issue this run already marked merged/parked/abandoned. NEXT_START=$(( $(jq '[.issues[].issue] | max // 0' .codegraph/fixer/state.json) + 1 )) - REMAINING=$(gh issue list --repo "$REPO" --state open --limit 400 \ + # --author filters server-side (see Phase 1) and --limit 1000 is a generous safety bound + # on this author's own backlog, not a real ceiling — a hard 400-issue window silently + # under-reported REMAINING as 0 in a repo with more open issues than that, reporting the + # backlog as drained while qualifying issues past the window went uncounted. + REMAINING=$(gh issue list --repo "$REPO" --state open --author "$AUTHOR" --limit 1000 \ --json number,labels,author \ - --jq "[.[] | select(.author.login==\"$AUTHOR\") | select([.labels[].name]|index(\"blocked\")|not) | select(.number >= $NEXT_START)] | length") + --jq "[.[] | select([.labels[].name]|index(\"blocked\")|not) | select(.number >= $NEXT_START)] | length") # 2>/dev/null: batches-done is expected to be absent before the first batch completes BATCHES_DONE=$(( $(cat .codegraph/fixer/batches-done 2>/dev/null || echo 0) + 1 )) @@ -796,13 +824,25 @@ Each pass does three things, in order: 1. **Sweep.** Invoke `/sweep` once. It processes every open PR in parallel: resolves conflicts, fixes CI, mines the Greptile summary as well as inline comments, addresses and replies to all reviewer feedback, and re-triggers reviewers. Do not re-derive that procedure here. 2. **Resolve.** For any parked PR still reporting `mergeable != MERGEABLE`, invoke `/resolve ` on it individually. `/resolve` reads both sides' intent before choosing, and stops rather than guessing on genuinely ambiguous conflicts. -3. **Merge the lowest-numbered ready PR first**, then re-evaluate. Merging lowest-first matches the requested order and keeps the catch-up cost predictable. **Immediately remove that PR's number from `parked.txt`** — this is what makes the progress check below meaningful; without it, `parked.txt` never shrinks and every pass looks stalled even when merges are happening: +3. **Merge the lowest-numbered ready PR first**, then re-evaluate. Merging lowest-first matches the requested order and keeps the catch-up cost predictable. **Immediately remove that PR's number from `parked.txt` AND flip its `state.json` entry from `parked` to `merged`** — the `parked.txt` removal is what makes the progress check below meaningful (without it, the file never shrinks and every pass looks stalled even when merges are happening); the `state.json` update is what keeps Phase 5's final report honest (without it, a PR merged here still reads as `parked` there, undercounting merged issues and reporting completed work as unfinished): ```bash mkdir -p .codegraph/fixer # MERGED_PR is the PR number just merged in this step grep -vxF "$MERGED_PR" .codegraph/fixer/parked.txt > .codegraph/fixer/parked.txt.tmp \ && mv .codegraph/fixer/parked.txt.tmp .codegraph/fixer/parked.txt echo "fixer: removed merged PR #$MERGED_PR from parked.txt — $(wc -l < .codegraph/fixer/parked.txt) remaining" + + # Reconcile state.json: the issue tied to this PR was recorded as "parked" back when it + # first hit the convergence-round cap (2f/2g) — that record is now stale the moment this + # merge succeeds. Match on .pr rather than .issue since that is the field this drain loop + # actually has in hand. + TMP_STATE=$(mktemp "${TMPDIR:-/tmp}/fixer-state.XXXXXXXXXX") + trap 'rm -f "$TMP_STATE"' EXIT + jq --argjson pr "$MERGED_PR" \ + '.issues |= map(if .pr == $pr then .status = "merged" else . end)' \ + .codegraph/fixer/state.json > "$TMP_STATE" && mv "$TMP_STATE" .codegraph/fixer/state.json + trap - EXIT + echo "fixer: state.json entry for PR #$MERGED_PR updated parked -> merged" ``` **After each pass**, record whether it made progress and decide whether to run another one: @@ -982,7 +1022,7 @@ All state is under `.codegraph/fixer/`: | `current-pr`, `last-pr-url`, `gate-fail` | text | current iteration's scratch state | | `outcome` | text | `abandoned`/`merged`/`parked` for the issue in progress; read by 2g, cleared after recording | | `round` | text | convergence-round counter for the current PR; cleared once it merges or parks | -| `gate-signature`, `prev-gate-signature` | text | fingerprint of this round's / the previous round's gate state, used to detect a stalled (blocked) PR rather than counting rounds | +| `gate-signature`, `prev-gate-signature` | text | fingerprint of this round's / the previous round's gate state (score, a hash of the full Greptile review text, which specific comments are unanswered, branch ancestry, mergeability, failing checks, and the current commit SHA), used to detect a stalled (blocked) PR rather than counting rounds | | `stall-count` | text | consecutive convergence rounds with an unchanged gate signature; park threshold is 3 | | `drain-pass`, `drain-stall`, `drain-parked-count` | text | drain-phase pass counter, consecutive no-merge passes, and `parked.txt` length as of the last pass — the same stall-detection pattern applied to draining | | `authored-lines.tsv`, `authored-files.txt` | text | I4 integrity-check baseline (tab-separated `filecountline`) | From 4262e57221d7659ab556ffcf7b8e05eb07555f34 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Tue, 28 Jul 2026 18:55:53 -0700 Subject: [PATCH 03/13] Revert "fix: reconcile state.json when Phase 4 drain merges a parked PR (#2169)" This reverts commit c06a69f2a630c4d1f3da8c2cb47ac6c0ef76abe6. --- .claude/skills/fixer/SKILL.md | 58 ++++++----------------------------- 1 file changed, 9 insertions(+), 49 deletions(-) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index 6bd45f25..70913380 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -184,14 +184,10 @@ START_FROM=$(cat .codegraph/fixer/start-from) if [ -f .codegraph/fixer/queue.json ]; then echo "fixer: reusing existing queue (Pattern 12 — artifact reuse). Delete .codegraph/fixer/queue.json to rebuild." else - # --author filters server-side so the --limit ceiling bounds this author's own backlog, - # not the repo's entire open-issue count (gh issue list defaults to --limit 30 with no - # server-side author filter, which previously meant issues by other authors could crowd - # this author's own low-numbered issues out of the fetched window). 1000 is a deliberately - # generous safety bound on one author's queue, not a realistic ceiling to ever hit. - gh issue list --state open --author "$AUTHOR" --limit 1000 \ + gh issue list --state open --limit 400 \ --json number,title,labels,author \ - --jq "[.[] | select([.labels[].name] | index(\"blocked\") | not) + --jq "[.[] | select(.author.login==\"$AUTHOR\") + | select([.labels[].name] | index(\"blocked\") | not) | {issue: .number, title: .title}] | sort_by(.issue) | map(select(.issue >= ($START_FROM|tonumber))) @@ -512,18 +508,11 @@ fi # unanswered Greptile inline comments. --- ALL_COMMENTS=$(gh api "repos/$REPO/pulls/$PR/comments" --paginate) UNANSWERED=0 -# Identities of the currently-unanswered comments, not just their count — two rounds can -# both show "2 unanswered" while the actual pair differs (one answered, a new one landed), -# which a bare count would wrongly read as zero movement. Folded into the gate signature below. -UNANSWERED_IDS="" for CID in $(printf '%s\n' "$ALL_COMMENTS" | jq -r '[.[] | select(.in_reply_to_id == null)] | .[].id'); do ORIGIN_USER=$(printf '%s\n' "$ALL_COMMENTS" | jq -r --argjson cid "$CID" '[.[] | select(.id == $cid)][0].user.login') REPLIES=$(printf '%s\n' "$ALL_COMMENTS" | jq -s --argjson cid "$CID" --arg origin "$ORIGIN_USER" \ '[.[][] | select(.in_reply_to_id == $cid) | select(.user.login != $origin)] | length') - if [ "$REPLIES" -eq 0 ]; then - UNANSWERED=$((UNANSWERED + 1)) - UNANSWERED_IDS="$UNANSWERED_IDS,$CID" - fi + [ "$REPLIES" -eq 0 ] && UNANSWERED=$((UNANSWERED + 1)) done # Top-level review bodies (Claude's CHANGES_REQUESTED/COMMENT summary, Greptile's @@ -592,20 +581,7 @@ printf '%s\n' "$GATE_FAIL" > .codegraph/fixer/gate-fail # in between — that is the actual definition of "blocked" this skill parks on, not an # arbitrary round count. Order the fields consistently so an unrelated field re-ordering # never masquerades as a change. -# -# The aggregate score, unanswered count, mergeability, and failing-check names alone are -# too coarse: a new commit, a fresh Greptile review, or a new CI run can all leave every -# one of those aggregates reading identical to the previous round (score lags a push, -# a re-review restates the same score for different reasons, a rerun fails the same named -# check for a different cause) — which would misread real activity as a stall. `commit` -# (the pushed SHA) and `greptile_hash` (a hash of every Greptile comment/review body, not -# just the extracted digit) both change the instant that activity happens, even before the -# aggregates catch up, so genuine progress is never mistaken for zero measurable change. -# `unanswered_ids` likewise tracks which comments are outstanding, not just how many, so a -# reply that resolves one comment while a new one lands does not cancel out to "no change". -GREPTILE_HASH=$(printf '%s' "$GREP_BODY" | cksum | awk '{print $1}') -COMMIT=$(git rev-parse HEAD) -printf '%s\n' "score=${SCORE:-none};greptile_hash=$GREPTILE_HASH;unanswered=$UNANSWERED;unanswered_ids=${UNANSWERED_IDS:-none};g3=$(git merge-base --is-ancestor origin/main HEAD && echo ok || echo behind);mergeable=$MERGEABLE;failed=$FAILED_CHECKS;commit=$COMMIT" \ +printf '%s\n' "score=${SCORE:-none};unanswered=$UNANSWERED;g3=$(git merge-base --is-ancestor origin/main HEAD && echo ok || echo behind);mergeable=$MERGEABLE;failed=$FAILED_CHECKS" \ > .codegraph/fixer/gate-signature ``` @@ -765,13 +741,9 @@ else # One past the highest issue number this run has recorded, so the next batch's queue # never re-examines an issue this run already marked merged/parked/abandoned. NEXT_START=$(( $(jq '[.issues[].issue] | max // 0' .codegraph/fixer/state.json) + 1 )) - # --author filters server-side (see Phase 1) and --limit 1000 is a generous safety bound - # on this author's own backlog, not a real ceiling — a hard 400-issue window silently - # under-reported REMAINING as 0 in a repo with more open issues than that, reporting the - # backlog as drained while qualifying issues past the window went uncounted. - REMAINING=$(gh issue list --repo "$REPO" --state open --author "$AUTHOR" --limit 1000 \ + REMAINING=$(gh issue list --repo "$REPO" --state open --limit 400 \ --json number,labels,author \ - --jq "[.[] | select([.labels[].name]|index(\"blocked\")|not) | select(.number >= $NEXT_START)] | length") + --jq "[.[] | select(.author.login==\"$AUTHOR\") | select([.labels[].name]|index(\"blocked\")|not) | select(.number >= $NEXT_START)] | length") # 2>/dev/null: batches-done is expected to be absent before the first batch completes BATCHES_DONE=$(( $(cat .codegraph/fixer/batches-done 2>/dev/null || echo 0) + 1 )) @@ -824,25 +796,13 @@ Each pass does three things, in order: 1. **Sweep.** Invoke `/sweep` once. It processes every open PR in parallel: resolves conflicts, fixes CI, mines the Greptile summary as well as inline comments, addresses and replies to all reviewer feedback, and re-triggers reviewers. Do not re-derive that procedure here. 2. **Resolve.** For any parked PR still reporting `mergeable != MERGEABLE`, invoke `/resolve ` on it individually. `/resolve` reads both sides' intent before choosing, and stops rather than guessing on genuinely ambiguous conflicts. -3. **Merge the lowest-numbered ready PR first**, then re-evaluate. Merging lowest-first matches the requested order and keeps the catch-up cost predictable. **Immediately remove that PR's number from `parked.txt` AND flip its `state.json` entry from `parked` to `merged`** — the `parked.txt` removal is what makes the progress check below meaningful (without it, the file never shrinks and every pass looks stalled even when merges are happening); the `state.json` update is what keeps Phase 5's final report honest (without it, a PR merged here still reads as `parked` there, undercounting merged issues and reporting completed work as unfinished): +3. **Merge the lowest-numbered ready PR first**, then re-evaluate. Merging lowest-first matches the requested order and keeps the catch-up cost predictable. **Immediately remove that PR's number from `parked.txt`** — this is what makes the progress check below meaningful; without it, `parked.txt` never shrinks and every pass looks stalled even when merges are happening: ```bash mkdir -p .codegraph/fixer # MERGED_PR is the PR number just merged in this step grep -vxF "$MERGED_PR" .codegraph/fixer/parked.txt > .codegraph/fixer/parked.txt.tmp \ && mv .codegraph/fixer/parked.txt.tmp .codegraph/fixer/parked.txt echo "fixer: removed merged PR #$MERGED_PR from parked.txt — $(wc -l < .codegraph/fixer/parked.txt) remaining" - - # Reconcile state.json: the issue tied to this PR was recorded as "parked" back when it - # first hit the convergence-round cap (2f/2g) — that record is now stale the moment this - # merge succeeds. Match on .pr rather than .issue since that is the field this drain loop - # actually has in hand. - TMP_STATE=$(mktemp "${TMPDIR:-/tmp}/fixer-state.XXXXXXXXXX") - trap 'rm -f "$TMP_STATE"' EXIT - jq --argjson pr "$MERGED_PR" \ - '.issues |= map(if .pr == $pr then .status = "merged" else . end)' \ - .codegraph/fixer/state.json > "$TMP_STATE" && mv "$TMP_STATE" .codegraph/fixer/state.json - trap - EXIT - echo "fixer: state.json entry for PR #$MERGED_PR updated parked -> merged" ``` **After each pass**, record whether it made progress and decide whether to run another one: @@ -1022,7 +982,7 @@ All state is under `.codegraph/fixer/`: | `current-pr`, `last-pr-url`, `gate-fail` | text | current iteration's scratch state | | `outcome` | text | `abandoned`/`merged`/`parked` for the issue in progress; read by 2g, cleared after recording | | `round` | text | convergence-round counter for the current PR; cleared once it merges or parks | -| `gate-signature`, `prev-gate-signature` | text | fingerprint of this round's / the previous round's gate state (score, a hash of the full Greptile review text, which specific comments are unanswered, branch ancestry, mergeability, failing checks, and the current commit SHA), used to detect a stalled (blocked) PR rather than counting rounds | +| `gate-signature`, `prev-gate-signature` | text | fingerprint of this round's / the previous round's gate state, used to detect a stalled (blocked) PR rather than counting rounds | | `stall-count` | text | consecutive convergence rounds with an unchanged gate signature; park threshold is 3 | | `drain-pass`, `drain-stall`, `drain-parked-count` | text | drain-phase pass counter, consecutive no-merge passes, and `parked.txt` length as of the last pass — the same stall-detection pattern applied to draining | | `authored-lines.tsv`, `authored-files.txt` | text | I4 integrity-check baseline (tab-separated `filecountline`) | From 66125b6931e8dc72dff7da64dbe827f548b2e530 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Tue, 28 Jul 2026 18:56:15 -0700 Subject: [PATCH 04/13] fix: reconcile state.json when Phase 4 drain merges a parked PR (#2169) Merging a parked PR during the drain phase only removed its number from parked.txt; the corresponding state.json entry stayed status="parked", so Phase 5's final report undercounted merged issues and reported completed work as unfinished. Flip the entry to "merged" (matched by PR number) in the same step that removes it from parked.txt. --- .claude/skills/fixer/SKILL.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index 70913380..05323def 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -796,13 +796,25 @@ Each pass does three things, in order: 1. **Sweep.** Invoke `/sweep` once. It processes every open PR in parallel: resolves conflicts, fixes CI, mines the Greptile summary as well as inline comments, addresses and replies to all reviewer feedback, and re-triggers reviewers. Do not re-derive that procedure here. 2. **Resolve.** For any parked PR still reporting `mergeable != MERGEABLE`, invoke `/resolve ` on it individually. `/resolve` reads both sides' intent before choosing, and stops rather than guessing on genuinely ambiguous conflicts. -3. **Merge the lowest-numbered ready PR first**, then re-evaluate. Merging lowest-first matches the requested order and keeps the catch-up cost predictable. **Immediately remove that PR's number from `parked.txt`** — this is what makes the progress check below meaningful; without it, `parked.txt` never shrinks and every pass looks stalled even when merges are happening: +3. **Merge the lowest-numbered ready PR first**, then re-evaluate. Merging lowest-first matches the requested order and keeps the catch-up cost predictable. **Immediately remove that PR's number from `parked.txt` AND flip its `state.json` entry from `parked` to `merged`** — the `parked.txt` removal is what makes the progress check below meaningful (without it, the file never shrinks and every pass looks stalled even when merges are happening); the `state.json` update is what keeps Phase 5's final report honest (without it, a PR merged here still reads as `parked` there, undercounting merged issues and reporting completed work as unfinished): ```bash mkdir -p .codegraph/fixer # MERGED_PR is the PR number just merged in this step grep -vxF "$MERGED_PR" .codegraph/fixer/parked.txt > .codegraph/fixer/parked.txt.tmp \ && mv .codegraph/fixer/parked.txt.tmp .codegraph/fixer/parked.txt echo "fixer: removed merged PR #$MERGED_PR from parked.txt — $(wc -l < .codegraph/fixer/parked.txt) remaining" + + # Reconcile state.json: the issue tied to this PR was recorded as "parked" back when it + # first hit the convergence-round cap (2f/2g) — that record is now stale the moment this + # merge succeeds. Match on .pr rather than .issue since that is the field this drain loop + # actually has in hand. + TMP_STATE=$(mktemp "${TMPDIR:-/tmp}/fixer-state.XXXXXXXXXX") + trap 'rm -f "$TMP_STATE"' EXIT + jq --argjson pr "$MERGED_PR" \ + '.issues |= map(if .pr == $pr then .status = "merged" else . end)' \ + .codegraph/fixer/state.json > "$TMP_STATE" && mv "$TMP_STATE" .codegraph/fixer/state.json + trap - EXIT + echo "fixer: state.json entry for PR #$MERGED_PR updated parked -> merged" ``` **After each pass**, record whether it made progress and decide whether to run another one: From b56330ba0f0cd9396e2ebc415aabcf8f0639ef60 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Tue, 28 Jul 2026 18:56:32 -0700 Subject: [PATCH 05/13] fix: harden the convergence gate signature against lagging aggregates (#2169) The stall signature relied on aggregates (score, unanswered count, mergeable, failing check names) that can read identical across two rounds even when real activity happened: a push landing before the score/checks catch up, a re-review restating the same score for different reasons, or a rerun failing the same named check for a new cause. Fold in the current commit SHA, a hash of the full Greptile review text, and the specific set of unanswered comment ids so genuine progress is never misread as a stall. --- .claude/skills/fixer/SKILL.md | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index 05323def..41bc8d08 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -508,11 +508,18 @@ fi # unanswered Greptile inline comments. --- ALL_COMMENTS=$(gh api "repos/$REPO/pulls/$PR/comments" --paginate) UNANSWERED=0 +# Identities of the currently-unanswered comments, not just their count — two rounds can +# both show "2 unanswered" while the actual pair differs (one answered, a new one landed), +# which a bare count would wrongly read as zero movement. Folded into the gate signature below. +UNANSWERED_IDS="" for CID in $(printf '%s\n' "$ALL_COMMENTS" | jq -r '[.[] | select(.in_reply_to_id == null)] | .[].id'); do ORIGIN_USER=$(printf '%s\n' "$ALL_COMMENTS" | jq -r --argjson cid "$CID" '[.[] | select(.id == $cid)][0].user.login') REPLIES=$(printf '%s\n' "$ALL_COMMENTS" | jq -s --argjson cid "$CID" --arg origin "$ORIGIN_USER" \ '[.[][] | select(.in_reply_to_id == $cid) | select(.user.login != $origin)] | length') - [ "$REPLIES" -eq 0 ] && UNANSWERED=$((UNANSWERED + 1)) + if [ "$REPLIES" -eq 0 ]; then + UNANSWERED=$((UNANSWERED + 1)) + UNANSWERED_IDS="$UNANSWERED_IDS,$CID" + fi done # Top-level review bodies (Claude's CHANGES_REQUESTED/COMMENT summary, Greptile's @@ -581,7 +588,20 @@ printf '%s\n' "$GATE_FAIL" > .codegraph/fixer/gate-fail # in between — that is the actual definition of "blocked" this skill parks on, not an # arbitrary round count. Order the fields consistently so an unrelated field re-ordering # never masquerades as a change. -printf '%s\n' "score=${SCORE:-none};unanswered=$UNANSWERED;g3=$(git merge-base --is-ancestor origin/main HEAD && echo ok || echo behind);mergeable=$MERGEABLE;failed=$FAILED_CHECKS" \ +# +# The aggregate score, unanswered count, mergeability, and failing-check names alone are +# too coarse: a new commit, a fresh Greptile review, or a new CI run can all leave every +# one of those aggregates reading identical to the previous round (score lags a push, +# a re-review restates the same score for different reasons, a rerun fails the same named +# check for a different cause) — which would misread real activity as a stall. `commit` +# (the pushed SHA) and `greptile_hash` (a hash of every Greptile comment/review body, not +# just the extracted digit) both change the instant that activity happens, even before the +# aggregates catch up, so genuine progress is never mistaken for zero measurable change. +# `unanswered_ids` likewise tracks which comments are outstanding, not just how many, so a +# reply that resolves one comment while a new one lands does not cancel out to "no change". +GREPTILE_HASH=$(printf '%s' "$GREP_BODY" | cksum | awk '{print $1}') +COMMIT=$(git rev-parse HEAD) +printf '%s\n' "score=${SCORE:-none};greptile_hash=$GREPTILE_HASH;unanswered=$UNANSWERED;unanswered_ids=${UNANSWERED_IDS:-none};g3=$(git merge-base --is-ancestor origin/main HEAD && echo ok || echo behind);mergeable=$MERGEABLE;failed=$FAILED_CHECKS;commit=$COMMIT" \ > .codegraph/fixer/gate-signature ``` @@ -994,7 +1014,7 @@ All state is under `.codegraph/fixer/`: | `current-pr`, `last-pr-url`, `gate-fail` | text | current iteration's scratch state | | `outcome` | text | `abandoned`/`merged`/`parked` for the issue in progress; read by 2g, cleared after recording | | `round` | text | convergence-round counter for the current PR; cleared once it merges or parks | -| `gate-signature`, `prev-gate-signature` | text | fingerprint of this round's / the previous round's gate state, used to detect a stalled (blocked) PR rather than counting rounds | +| `gate-signature`, `prev-gate-signature` | text | fingerprint of this round's / the previous round's gate state (score, a hash of the full Greptile review text, which specific comments are unanswered, branch ancestry, mergeability, failing checks, and the current commit SHA), used to detect a stalled (blocked) PR rather than counting rounds | | `stall-count` | text | consecutive convergence rounds with an unchanged gate signature; park threshold is 3 | | `drain-pass`, `drain-stall`, `drain-parked-count` | text | drain-phase pass counter, consecutive no-merge passes, and `parked.txt` length as of the last pass — the same stall-detection pattern applied to draining | | `authored-lines.tsv`, `authored-files.txt` | text | I4 integrity-check baseline (tab-separated `filecountline`) | From af322d0bb942130011100295ab4404ba7bf38a1d Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Tue, 28 Jul 2026 18:56:50 -0700 Subject: [PATCH 06/13] fix: filter gh issue list by author server-side to avoid silent truncation (#2169) Both the queue-build and the batch-completion check fetched --limit 400 open issues repo-wide and filtered by author client-side. In a repo with more than 400 open issues, entries belonging to the target author could fall outside that window, under-reporting the remaining backlog as drained. Pass --author to gh issue list so filtering happens server-side and raise the limit to a generous safety bound on one author's own queue. --- .claude/skills/fixer/SKILL.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index 41bc8d08..6bd45f25 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -184,10 +184,14 @@ START_FROM=$(cat .codegraph/fixer/start-from) if [ -f .codegraph/fixer/queue.json ]; then echo "fixer: reusing existing queue (Pattern 12 — artifact reuse). Delete .codegraph/fixer/queue.json to rebuild." else - gh issue list --state open --limit 400 \ + # --author filters server-side so the --limit ceiling bounds this author's own backlog, + # not the repo's entire open-issue count (gh issue list defaults to --limit 30 with no + # server-side author filter, which previously meant issues by other authors could crowd + # this author's own low-numbered issues out of the fetched window). 1000 is a deliberately + # generous safety bound on one author's queue, not a realistic ceiling to ever hit. + gh issue list --state open --author "$AUTHOR" --limit 1000 \ --json number,title,labels,author \ - --jq "[.[] | select(.author.login==\"$AUTHOR\") - | select([.labels[].name] | index(\"blocked\") | not) + --jq "[.[] | select([.labels[].name] | index(\"blocked\") | not) | {issue: .number, title: .title}] | sort_by(.issue) | map(select(.issue >= ($START_FROM|tonumber))) @@ -761,9 +765,13 @@ else # One past the highest issue number this run has recorded, so the next batch's queue # never re-examines an issue this run already marked merged/parked/abandoned. NEXT_START=$(( $(jq '[.issues[].issue] | max // 0' .codegraph/fixer/state.json) + 1 )) - REMAINING=$(gh issue list --repo "$REPO" --state open --limit 400 \ + # --author filters server-side (see Phase 1) and --limit 1000 is a generous safety bound + # on this author's own backlog, not a real ceiling — a hard 400-issue window silently + # under-reported REMAINING as 0 in a repo with more open issues than that, reporting the + # backlog as drained while qualifying issues past the window went uncounted. + REMAINING=$(gh issue list --repo "$REPO" --state open --author "$AUTHOR" --limit 1000 \ --json number,labels,author \ - --jq "[.[] | select(.author.login==\"$AUTHOR\") | select([.labels[].name]|index(\"blocked\")|not) | select(.number >= $NEXT_START)] | length") + --jq "[.[] | select([.labels[].name]|index(\"blocked\")|not) | select(.number >= $NEXT_START)] | length") # 2>/dev/null: batches-done is expected to be absent before the first batch completes BATCHES_DONE=$(( $(cat .codegraph/fixer/batches-done 2>/dev/null || echo 0) + 1 )) From a067c875d9eed41f7ffa739bd39213e672c985f4 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 29 Jul 2026 00:15:48 -0700 Subject: [PATCH 07/13] fix: capture per-check completion timestamps in the fixer gate signature (#2169) A same-commit CI rerun (manual "Re-run failed jobs" or a flaky-test retry) leaves the commit SHA, check names, and every other gate-signature field unchanged, so the stall detector could not distinguish it from genuine inactivity and would park an actively-progressing PR. The signature now folds in each non-green check's completedAt/startedAt timestamp alongside its name, so a rerun always changes the fingerprint even when nothing else does. --- .claude/skills/fixer/SKILL.md | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index 6bd45f25..7da4f184 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -584,6 +584,16 @@ else echo "G5 FAIL: not green — $FAILED_CHECKS"; GATE_FAIL=1 fi +# Per-check completion timestamp (not just the name) of every currently non-green check. +# A manual "Re-run failed jobs" or a flaky-test retry on the SAME commit produces a NEW +# completedAt/startedAt for the SAME named check — real CI activity that a name-only list +# cannot see, since the check's name, the commit SHA, and every other gate field stay +# identical while it reruns. Folded into the gate signature below instead of the plain +# name list so a rerun that fails the same check for a different reason is never mistaken +# for zero measurable change. +CHECK_FINGERPRINT=$(gh pr view "$PR" --repo "$REPO" --json statusCheckRollup \ + --jq '[.statusCheckRollup[] | select((.conclusion // "PENDING") | test("SUCCESS|NEUTRAL|SKIPPED") | not)] | map("\(.name // .context)@\(.completedAt // .startedAt // .createdAt // "pending")") | sort | join(", ")') + printf '%s\n' "$GATE_FAIL" > .codegraph/fixer/gate-fail [ "$GATE_FAIL" -eq 0 ] && echo "fixer: PR #$PR passes all five gate conditions" || echo "fixer: PR #$PR is not mergeable yet" @@ -596,16 +606,18 @@ printf '%s\n' "$GATE_FAIL" > .codegraph/fixer/gate-fail # The aggregate score, unanswered count, mergeability, and failing-check names alone are # too coarse: a new commit, a fresh Greptile review, or a new CI run can all leave every # one of those aggregates reading identical to the previous round (score lags a push, -# a re-review restates the same score for different reasons, a rerun fails the same named -# check for a different cause) — which would misread real activity as a stall. `commit` -# (the pushed SHA) and `greptile_hash` (a hash of every Greptile comment/review body, not -# just the extracted digit) both change the instant that activity happens, even before the -# aggregates catch up, so genuine progress is never mistaken for zero measurable change. -# `unanswered_ids` likewise tracks which comments are outstanding, not just how many, so a -# reply that resolves one comment while a new one lands does not cancel out to "no change". +# a re-review restates the same score for different reasons, a rerun on the SAME commit +# fails the same named check for a different cause) — which would misread real activity as +# a stall. `commit` (the pushed SHA), `greptile_hash` (a hash of every Greptile comment/ +# review body, not just the extracted digit), and `checks` (each non-green check's name +# PLUS its completion timestamp, not just its name) all change the instant that activity +# happens, even before the aggregates catch up, so genuine progress — including a same- +# commit CI rerun — is never mistaken for zero measurable change. `unanswered_ids` +# likewise tracks which comments are outstanding, not just how many, so a reply that +# resolves one comment while a new one lands does not cancel out to "no change". GREPTILE_HASH=$(printf '%s' "$GREP_BODY" | cksum | awk '{print $1}') COMMIT=$(git rev-parse HEAD) -printf '%s\n' "score=${SCORE:-none};greptile_hash=$GREPTILE_HASH;unanswered=$UNANSWERED;unanswered_ids=${UNANSWERED_IDS:-none};g3=$(git merge-base --is-ancestor origin/main HEAD && echo ok || echo behind);mergeable=$MERGEABLE;failed=$FAILED_CHECKS;commit=$COMMIT" \ +printf '%s\n' "score=${SCORE:-none};greptile_hash=$GREPTILE_HASH;unanswered=$UNANSWERED;unanswered_ids=${UNANSWERED_IDS:-none};g3=$(git merge-base --is-ancestor origin/main HEAD && echo ok || echo behind);mergeable=$MERGEABLE;checks=$CHECK_FINGERPRINT;commit=$COMMIT" \ > .codegraph/fixer/gate-signature ``` @@ -1022,7 +1034,7 @@ All state is under `.codegraph/fixer/`: | `current-pr`, `last-pr-url`, `gate-fail` | text | current iteration's scratch state | | `outcome` | text | `abandoned`/`merged`/`parked` for the issue in progress; read by 2g, cleared after recording | | `round` | text | convergence-round counter for the current PR; cleared once it merges or parks | -| `gate-signature`, `prev-gate-signature` | text | fingerprint of this round's / the previous round's gate state (score, a hash of the full Greptile review text, which specific comments are unanswered, branch ancestry, mergeability, failing checks, and the current commit SHA), used to detect a stalled (blocked) PR rather than counting rounds | +| `gate-signature`, `prev-gate-signature` | text | fingerprint of this round's / the previous round's gate state (score, a hash of the full Greptile review text, which specific comments are unanswered, branch ancestry, mergeability, each non-green check's name plus its completion timestamp, and the current commit SHA), used to detect a stalled (blocked) PR rather than counting rounds | | `stall-count` | text | consecutive convergence rounds with an unchanged gate signature; park threshold is 3 | | `drain-pass`, `drain-stall`, `drain-parked-count` | text | drain-phase pass counter, consecutive no-merge passes, and `parked.txt` length as of the last pass — the same stall-detection pattern applied to draining | | `authored-lines.tsv`, `authored-files.txt` | text | I4 integrity-check baseline (tab-separated `filecountline`) | From 07b07216c59e7f96db9e0a2fe7ff919cd4b91278 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 29 Jul 2026 00:42:41 -0700 Subject: [PATCH 08/13] fix: stop parked.txt removal from silently no-op'ing on the last entry (#2169) grep -v's exit status reflects whether any line passed the inverted filter, not whether the redirect succeeded. When the merged PR is the only entry in parked.txt, every line is excluded and grep exits 1 even though the empty result is exactly what's wanted -- so the chained `&& mv` never ran, silently leaving the just-merged PR's number in parked.txt forever. Decoupled the mv from grep's match-count exit code with `|| true`. Verified against both the last-entry case and a normal multi-entry removal. --- .claude/skills/fixer/SKILL.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index 7da4f184..5eb50030 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -840,8 +840,15 @@ Each pass does three things, in order: ```bash mkdir -p .codegraph/fixer # MERGED_PR is the PR number just merged in this step - grep -vxF "$MERGED_PR" .codegraph/fixer/parked.txt > .codegraph/fixer/parked.txt.tmp \ - && mv .codegraph/fixer/parked.txt.tmp .codegraph/fixer/parked.txt + # grep -v's exit status reports whether any line PASSED the inverted filter, not whether + # the redirect succeeded — when $MERGED_PR is the ONLY entry in parked.txt, every line is + # excluded, zero lines are written, and grep exits 1 even though parked.txt.tmp was + # created correctly (empty, as desired). A chained `&& mv` would then never run on + # exactly the case that matters most (draining the last parked PR), silently leaving the + # just-merged PR's number in parked.txt forever. `|| true` decouples the mv from grep's + # match-count exit code so the empty-result case moves the file same as any other. + grep -vxF "$MERGED_PR" .codegraph/fixer/parked.txt > .codegraph/fixer/parked.txt.tmp || true + mv .codegraph/fixer/parked.txt.tmp .codegraph/fixer/parked.txt echo "fixer: removed merged PR #$MERGED_PR from parked.txt — $(wc -l < .codegraph/fixer/parked.txt) remaining" # Reconcile state.json: the issue tied to this PR was recorded as "parked" back when it From 072b185eebad4701fd4ce92a4b9c28ab90f0eb40 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 29 Jul 2026 01:10:49 -0700 Subject: [PATCH 09/13] fix: replace fixed issue-fetch limit with truncation detection (#2169) 400, then 1000: any fixed --limit on gh issue list is the same silent truncation point at a different size, and it will keep recurring as an author's backlog grows. Both the Phase 1 queue build and the Phase 3 batch-completion check now fetch against an effectively-unbounded limit (100000) and fail loudly if that limit is ever actually returned in full, instead of guessing at a bigger "generous" constant that could still be exceeded and silently misreport an incomplete queue or a falsely-drained backlog. --- .claude/skills/fixer/SKILL.md | 49 +++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index 5eb50030..ab5bdf21 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -187,16 +187,28 @@ else # --author filters server-side so the --limit ceiling bounds this author's own backlog, # not the repo's entire open-issue count (gh issue list defaults to --limit 30 with no # server-side author filter, which previously meant issues by other authors could crowd - # this author's own low-numbered issues out of the fetched window). 1000 is a deliberately - # generous safety bound on one author's queue, not a realistic ceiling to ever hit. - gh issue list --state open --author "$AUTHOR" --limit 1000 \ - --json number,title,labels,author \ - --jq "[.[] | select([.labels[].name] | index(\"blocked\") | not) + # this author's own low-numbered issues out of the fetched window). + # + # ISSUE_FETCH_LIMIT is NOT "a generous number" — any fixed number, however large, is a + # silent truncation point some author's backlog can eventually exceed (this skill already + # went 400 -> 1000 once and Greptile correctly flagged 1000 as the same class of bug, not + # a fix). Set it astronomically high instead, and treat hitting it as a hard error rather + # than something to guess a bigger constant for: if RAW_COUNT below ever equals this limit, + # the fetch was truncated and completeness genuinely cannot be trusted, so fail loudly + # instead of silently reporting an incomplete queue or a falsely-drained backlog. + ISSUE_FETCH_LIMIT=100000 + RAW_ISSUES=$(gh issue list --state open --author "$AUTHOR" --limit "$ISSUE_FETCH_LIMIT" \ + --json number,title,labels,author) || { echo "ERROR: failed to fetch issues from GitHub"; exit 1; } + RAW_COUNT=$(printf '%s' "$RAW_ISSUES" | jq 'length') + if [ "$RAW_COUNT" -eq "$ISSUE_FETCH_LIMIT" ]; then + echo "ERROR: gh issue list returned exactly the $ISSUE_FETCH_LIMIT-issue fetch limit for author '$AUTHOR' — the result is truncated and queue completeness cannot be trusted. This should never happen in practice; investigate '$AUTHOR' before continuing." + exit 1 + fi + printf '%s' "$RAW_ISSUES" | jq "[.[] | select([.labels[].name] | index(\"blocked\") | not) | {issue: .number, title: .title}] | sort_by(.issue) | map(select(.issue >= ($START_FROM|tonumber))) - | .[0:($COUNT|tonumber)]" > .codegraph/fixer/queue.json || { - echo "ERROR: failed to fetch issues from GitHub"; exit 1; } + | .[0:($COUNT|tonumber)]" > .codegraph/fixer/queue.json fi QUEUED=$(jq 'length' .codegraph/fixer/queue.json) @@ -777,13 +789,22 @@ else # One past the highest issue number this run has recorded, so the next batch's queue # never re-examines an issue this run already marked merged/parked/abandoned. NEXT_START=$(( $(jq '[.issues[].issue] | max // 0' .codegraph/fixer/state.json) + 1 )) - # --author filters server-side (see Phase 1) and --limit 1000 is a generous safety bound - # on this author's own backlog, not a real ceiling — a hard 400-issue window silently - # under-reported REMAINING as 0 in a repo with more open issues than that, reporting the - # backlog as drained while qualifying issues past the window went uncounted. - REMAINING=$(gh issue list --repo "$REPO" --state open --author "$AUTHOR" --limit 1000 \ - --json number,labels,author \ - --jq "[.[] | select([.labels[].name]|index(\"blocked\")|not) | select(.number >= $NEXT_START)] | length") + # --author filters server-side (see Phase 1) and reuses the same ISSUE_FETCH_LIMIT / + # truncation-detection pattern: a hard 400-issue window once silently under-reported + # REMAINING as 0 in a repo with more open issues than that, reporting the backlog as + # drained while qualifying issues past the window went uncounted. Raising the number + # (400 -> 1000) only moved the same silent-truncation risk further out rather than + # removing it, so this fetches against an effectively-unbounded limit and fails loudly + # if that limit is ever actually hit, instead of trusting a fixed-size guess. + ISSUE_FETCH_LIMIT=100000 + RAW_ISSUES=$(gh issue list --repo "$REPO" --state open --author "$AUTHOR" --limit "$ISSUE_FETCH_LIMIT" \ + --json number,labels,author) || { echo "ERROR: failed to fetch issues from GitHub for the batch-completion check"; exit 1; } + RAW_COUNT=$(printf '%s' "$RAW_ISSUES" | jq 'length') + if [ "$RAW_COUNT" -eq "$ISSUE_FETCH_LIMIT" ]; then + echo "ERROR: gh issue list returned exactly the $ISSUE_FETCH_LIMIT-issue fetch limit for author '$AUTHOR' — the result is truncated and the drained/remaining determination cannot be trusted. This should never happen in practice; investigate '$AUTHOR' before continuing." + exit 1 + fi + REMAINING=$(printf '%s' "$RAW_ISSUES" | jq "[.[] | select([.labels[].name]|index(\"blocked\")|not) | select(.number >= $NEXT_START)] | length") # 2>/dev/null: batches-done is expected to be absent before the first batch completes BATCHES_DONE=$(( $(cat .codegraph/fixer/batches-done 2>/dev/null || echo 0) + 1 )) From 32c043c74cdfa6a60a42509ec0fbdfcf3574cb1d Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 29 Jul 2026 01:38:09 -0700 Subject: [PATCH 10/13] fix: self-heal state.json/parked.txt split-brain at drain-phase entry (#2169) The drain-merge step's two-write reconciliation (parked.txt removal, then a separate state.json parked->merged update) is not atomic: an interruption landing between the two writes leaves parked.txt correctly missing the merged PR while state.json still records it as parked. Since queue.json is already empty once draining starts, a later invocation reads that as a genuinely fresh run and would silently wipe the stale-but-recoverable record, losing the completed merge from the final accounting. The drain phase now reconciles any state.json "parked" entry whose PR is absent from parked.txt (evidence it already merged) back to "merged" before doing anything else that pass. Verified against both an interrupted-drain case (reconciles) and a normal still-parked case (leaves untouched). --- .claude/skills/fixer/SKILL.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index ab5bdf21..6589db0b 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -845,6 +845,24 @@ This is the mode switch: stop solving new issues, and work the parked PRs as a s ```bash mkdir -p .codegraph/fixer + +# Self-heal a parked.txt / state.json split-brain left by an interruption landing between +# this phase's own two-write reconciliation below (parked.txt removal, then a SEPARATE +# state.json update) — if the process stopped exactly there, parked.txt already lost the +# PR (correctly, since it merged) but state.json still records it as "parked". Left +# uncorrected, that merge is invisible to Phase 5's final report, and since queue.json is +# already empty by the time draining starts, a later invocation would read this as a +# genuinely fresh run and silently wipe the stale-but-recoverable record instead of fixing +# it. Reconcile before anything else touches parked.txt or state.json this pass. +for STALE_PR in $(jq -r '[.issues[] | select(.status=="parked") | .pr // empty] | .[]' .codegraph/fixer/state.json 2>/dev/null); do + if ! grep -qxF "$STALE_PR" .codegraph/fixer/parked.txt 2>/dev/null; then + TMP_STATE=$(mktemp "${TMPDIR:-/tmp}/fixer-state.XXXXXXXXXX") + jq --argjson pr "$STALE_PR" '(.issues[] | select(.pr == $pr and .status=="parked") | .status) = "merged"' \ + .codegraph/fixer/state.json > "$TMP_STATE" && mv "$TMP_STATE" .codegraph/fixer/state.json + echo "fixer: reconciled state.json entry for PR #$STALE_PR from a prior interrupted drain — parked.txt no longer lists it, so it must have already merged; corrected parked -> merged before continuing" + fi +done + if [ ! -s .codegraph/fixer/parked.txt ]; then echo "fixer: no parked PRs — skipping drain phase" else From a26f43ccb91a0436949da3aa9fa6f28c26691a33 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 29 Jul 2026 02:20:32 -0700 Subject: [PATCH 11/13] fix: make Phase 0 resume-detection recognize a mid-drain interruption (#2169) Phase 0 decided fresh-run vs. resume from queue.json alone. queue.json is legitimately already empty by the time Phase: Drain Parked PRs starts, so an interruption anywhere during the drain (including the exact split-brain window the prior commit's self-heal step was added to recover) looked identical to a fully-completed run: the next invocation would delete parked.txt and reset state.json before that self-heal step ever got a chance to run, silently erasing every still-parked PR along with the evidence needed to recover the interrupted merge. Phase 0 now also treats a non-empty parked.txt as evidence of a resumable (mid-drain) run, alongside queue.json, and routes straight to Phase: Drain Parked PRs when queue.json is empty but parked.txt is not. --- .claude/skills/fixer/SKILL.md | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index 6589db0b..07722ac6 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -141,16 +141,24 @@ case "$(git rev-parse --show-toplevel)" in esac ``` -**Resume handling.** A non-empty `queue.json` means a previous invocation stopped mid-batch (crash, interruption) with unresolved work — that is the ONLY case this treats as a resume. An absent or empty `queue.json` means the last invocation's last batch already ran to completion (Phase: Continue the Batch Loop, or Proceed to Drain always leaves `queue.json` empty when it decides to stop), so this is a genuinely new run and every run-scoped artifact — including `batches-done` and `queue.json` itself — must be reset. Conflating these two cases is exactly how a stale `batches-done` count or a stale empty `queue.json` leaks into a brand-new invocation, silently skipping newly opened issues or hitting the batch safety cap prematurely. +**Resume handling.** A non-empty `queue.json` means a previous invocation stopped mid-batch (crash, interruption) with unresolved work. A non-empty `parked.txt` means a previous invocation stopped mid-drain (Phase: Drain Parked PRs runs *after* `queue.json` is legitimately already empty, so `queue.json` alone cannot distinguish "batch loop finished, draining in progress" from "run genuinely complete"). Either signal alone is enough to treat this as a resume, not a fresh run — an absent/empty `queue.json` **and** an absent/empty `parked.txt` together are what mean the last invocation ran all the way to completion (Phase: Continue the Batch Loop, or Proceed to Drain always leaves `queue.json` empty when it decides to stop or drain, and Phase: Drain Parked PRs always leaves `parked.txt` empty once everything is merged), so only then is every run-scoped artifact — including `batches-done`, `queue.json`, and `parked.txt` itself — reset. Conflating these cases is exactly how a stale `batches-done` count or a stale empty `queue.json` leaks into a brand-new invocation and silently skips newly opened issues or hits the batch safety cap prematurely — and, just as badly, how an interruption mid-drain (e.g. between `parked.txt`'s removal of a just-merged PR and the separate `state.json` update reconciling it) gets read as "nothing left to do," deleting `parked.txt` and wiping `state.json` before Phase: Drain Parked PRs' own self-heal step ever gets a chance to run, silently erasing every still-parked PR along with the evidence needed to recover the interrupted merge. ```bash mkdir -p .codegraph/fixer # 2>/dev/null || echo 0: a missing or corrupt queue.json is treated as empty, which # correctly routes to the "fresh run" branch rather than erroring QUEUE_LEN=$(jq 'length' .codegraph/fixer/queue.json 2>/dev/null || echo 0) -if [ -f .codegraph/fixer/queue.json ] && [ "$QUEUE_LEN" -gt 0 ]; then - echo "fixer: resuming an interrupted batch — previous state:" +# -s (non-empty file): parked.txt is only ever emptied, never deleted, once draining +# finishes (Phase: Drain Parked PRs), so this is safe evidence that a drain is still +# in progress rather than long since complete. +PARKED_LEN=0 +[ -s .codegraph/fixer/parked.txt ] && PARKED_LEN=$(wc -l < .codegraph/fixer/parked.txt) +if { [ -f .codegraph/fixer/queue.json ] && [ "$QUEUE_LEN" -gt 0 ]; } || [ "$PARKED_LEN" -gt 0 ]; then + echo "fixer: resuming an interrupted run — previous state:" jq -r '.issues[] | " #\(.issue) \(.status) \(if .pr then "PR #\(.pr)" else "no PR" end)"' .codegraph/fixer/state.json 2>/dev/null + if [ "$QUEUE_LEN" -eq 0 ] && [ "$PARKED_LEN" -gt 0 ]; then + echo "fixer: queue.json is empty but parked.txt has $PARKED_LEN PR(s) — the batch loop had already finished and this run was mid-drain. Skip Phase 1-3 entirely and go straight to Phase: Drain Parked PRs." + fi else if [ -f .codegraph/fixer/state.json ]; then echo "fixer: previous run's artifacts are still on disk but its last batch already completed — that run is done. Starting a genuinely fresh run, not resuming it." @@ -165,7 +173,7 @@ else fi ``` -**Exit condition:** `git`, `gh`, `jq`, `mktemp` present; inside a git repo; `gh` authenticated; no in-progress merge; repo slug, test command, and lint command persisted to `.codegraph/fixer/`; arguments parsed and persisted; worktree isolation confirmed or requested; `state.json` exists. +**Exit condition:** `git`, `gh`, `jq`, `mktemp` present; inside a git repo; `gh` authenticated; no in-progress merge; repo slug, test command, and lint command persisted to `.codegraph/fixer/`; arguments parsed and persisted; worktree isolation confirmed or requested; `state.json` exists. If resuming with an empty `queue.json` but a non-empty `parked.txt`, proceed directly to Phase: Drain Parked PRs — do not rebuild or reuse a queue for a batch loop that already finished. --- @@ -1057,7 +1065,7 @@ Also list, explicitly: - every issue that was split, with the follow-up covering the remainder - anything left for a human, with the specific blocker -**Cleanup.** State lives in `.codegraph/fixer/`. It is left on disk after this run finishes, but Phase 0's resume handling already treats a completed run's artifacts as stale on the *next* invocation — an empty `queue.json` means the next `/fixer` call starts genuinely fresh, not "resuming" this run's results. The only thing left on disk that matters afterward is `state.json` as a historical record of what this run did; `rm -rf .codegraph/fixer` removes it entirely if you don't want even that. +**Cleanup.** State lives in `.codegraph/fixer/`. It is left on disk after this run finishes, but Phase 0's resume handling already treats a completed run's artifacts as stale on the *next* invocation — an empty `queue.json` **and** an empty `parked.txt` together mean the next `/fixer` call starts genuinely fresh, not "resuming" this run's results. The only thing left on disk that matters afterward is `state.json` as a historical record of what this run did; `rm -rf .codegraph/fixer` removes it entirely if you don't want even that. **Exit condition:** The user has a per-issue table covering every batch this run processed, the merged count, the batch count, every follow-up issue number, every bypass with its justification, and an explicit list of anything unfinished (including a resume command if the 15-batch safety cap was hit). @@ -1072,9 +1080,9 @@ All state is under `.codegraph/fixer/`: | `repo` | text | `owner/name` slug — the only repo this run ever touches | | `count`, `author`, `start-from`, `once`, `dry-run` | text | parsed arguments | | `test-cmd`, `lint-cmd` | text | detected package-manager commands | -| `queue.json` | JSON array of `{issue, title}` | current batch's remaining work, ascending; entries are shifted off as they complete. **A present-but-empty `queue.json` means the last batch fully completed** — Phase 0 uses exactly this signal to decide fresh-run vs. resume | +| `queue.json` | JSON array of `{issue, title}` | current batch's remaining work, ascending; entries are shifted off as they complete. **A present-but-empty `queue.json` means the batch loop fully completed** — Phase 0 uses this, together with `parked.txt`, to decide fresh-run vs. resume | | `state.json` | `{"issues":[{issue, status, pr}]}` | per-issue outcome, accumulated across every batch this run has processed; reset to empty whenever Phase 0 detects a fresh run | -| `parked.txt` | newline-separated PR numbers | accumulated across every batch; input to Phase: Drain Parked PRs; a merged PR's number is removed from this file the moment it merges | +| `parked.txt` | newline-separated PR numbers | accumulated across every batch; input to Phase: Drain Parked PRs; a merged PR's number is removed from this file the moment it merges. **Only ever emptied, never deleted, by a completed drain** — Phase 0 also treats a non-empty `parked.txt` as evidence of a resumable (mid-drain) run, since `queue.json` is legitimately already empty by the time draining starts | | `batches-done` | text | count of batches completed this run; read by Phase 3 and the final report; reset on a fresh run | | `loop-decision` | text | `loop`/`stop` — Phase 3's verdict on whether to start another batch | | `current-pr`, `last-pr-url`, `gate-fail` | text | current iteration's scratch state | @@ -1122,7 +1130,7 @@ All state is under `.codegraph/fixer/`: - **The run's objective is the whole backlog, not one batch.** By default, keep starting new batches (Phase 3) until no qualifying open issues remain or the 15-batch cap is hit. Pass `--once` to process exactly one batch and stop. - **Only ever act on the repo detected in Phase 0.** Never search, open issues on, or open PRs against any other repository — including when this repo's queue comes up empty. Read the slug from `.codegraph/fixer/repo`; never hardcode it (see issue #2164). - **An empty queue is success, not failure.** If Phase 1 finds no qualifying open issues, report that plainly and exit 0 — never search other repos, never lower the `--author`/`blocked`-label bar to manufacture a queue, and never invent unrelated work (stale files, refactors, drive-by fixes) to fill a batch. -- **A completed run is not a resumable run.** Phase 0 only resumes when `queue.json` is non-empty (real unfinished work). An empty or absent `queue.json` means the previous batch already finished, so every run-scoped artifact (`batches-done`, `queue.json`, `parked.txt`, `state.json`, per-issue scratch state) is reset before starting — never let a finished run's counters leak into a new invocation and skip issues or hit the batch cap early. +- **A completed run is not a resumable run.** Phase 0 resumes when `queue.json` is non-empty (a batch stopped mid-flight) OR `parked.txt` is non-empty (draining stopped mid-flight — `queue.json` is legitimately already empty by the time Phase: Drain Parked PRs starts, so it alone cannot tell "draining in progress" apart from "run genuinely complete"). Only when BOTH are empty or absent did the previous run finish, so only then is every run-scoped artifact (`batches-done`, `queue.json`, `parked.txt`, `state.json`, per-issue scratch state) reset before starting — never let a finished run's counters leak into a new invocation and skip issues or hit the batch cap early, and never let an interrupted drain be mistaken for a finished one and lose its still-parked PRs. - **No co-author lines** in commit messages or PR bodies. - **No Claude Code or Anthropic references** in commits, PR bodies, comments, or issues. - **Never trigger `@greptileai` until every Greptile comment has a reply.** Do not trigger Greptile for feedback that came from Claude or the user. From 994896d52cefdd19514d910756e8ae251c31b5bb Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 29 Jul 2026 02:48:43 -0700 Subject: [PATCH 12/13] fix: don't let an already-reported blocked drain masquerade as a resumable one (#2169) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit made Phase 0 treat any non-empty parked.txt as evidence of a resumable mid-drain run, since queue.json is legitimately already empty for the whole duration of Phase: Drain Parked PRs. But that phase's own exit condition is "every PR merged, or reported as needing human review" — once a drain stops because 3 consecutive passes merged nothing, or the 15-pass cap was hit, the remaining parked.txt entries are a closed, already-reported matter for that run, not an interruption. Treating them the same made every subsequent invocation skip Phase 1-3 forever, re-attempting the same permanently-stuck PRs and never queuing newly opened issues. Phase 0 now also reads drain-stall/drain-pass (already maintained by Phase: Drain Parked PRs) to tell the two cases apart: parked.txt is only a resume signal when the drain that left it there had not yet reached its own stall/cap threshold. Verified the fresh/resume decision against all six relevant combinations (no state, fully-merged, mid-batch, mid-drain interrupted, drain stalled-out, drain capped-out) standalone before landing it. --- .claude/skills/fixer/SKILL.md | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index 07722ac6..79925970 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -141,7 +141,9 @@ case "$(git rev-parse --show-toplevel)" in esac ``` -**Resume handling.** A non-empty `queue.json` means a previous invocation stopped mid-batch (crash, interruption) with unresolved work. A non-empty `parked.txt` means a previous invocation stopped mid-drain (Phase: Drain Parked PRs runs *after* `queue.json` is legitimately already empty, so `queue.json` alone cannot distinguish "batch loop finished, draining in progress" from "run genuinely complete"). Either signal alone is enough to treat this as a resume, not a fresh run — an absent/empty `queue.json` **and** an absent/empty `parked.txt` together are what mean the last invocation ran all the way to completion (Phase: Continue the Batch Loop, or Proceed to Drain always leaves `queue.json` empty when it decides to stop or drain, and Phase: Drain Parked PRs always leaves `parked.txt` empty once everything is merged), so only then is every run-scoped artifact — including `batches-done`, `queue.json`, and `parked.txt` itself — reset. Conflating these cases is exactly how a stale `batches-done` count or a stale empty `queue.json` leaks into a brand-new invocation and silently skips newly opened issues or hits the batch safety cap prematurely — and, just as badly, how an interruption mid-drain (e.g. between `parked.txt`'s removal of a just-merged PR and the separate `state.json` update reconciling it) gets read as "nothing left to do," deleting `parked.txt` and wiping `state.json` before Phase: Drain Parked PRs' own self-heal step ever gets a chance to run, silently erasing every still-parked PR along with the evidence needed to recover the interrupted merge. +**Resume handling.** A non-empty `queue.json` means a previous invocation stopped mid-batch (crash, interruption) with unresolved work. A non-empty `parked.txt` *can* mean a previous invocation stopped mid-drain (Phase: Drain Parked PRs runs *after* `queue.json` is legitimately already empty, so `queue.json` alone cannot distinguish "batch loop finished, draining in progress" from "run genuinely complete") — but only if that drain hadn't already reached ITS OWN terminal state. Phase: Drain Parked PRs' own exit condition is "every PR merged, or reported as needing human review" — once a drain stops because 3 consecutive passes merged nothing, or the 15-pass cap was hit, the remaining entries in `parked.txt` are a **closed, already-reported matter for that run**, not evidence of an interruption. Treating a closed-but-blocked drain the same as a genuinely-interrupted one would make every subsequent invocation skip Phase 1-3 forever, re-attempting the same permanently-stuck PRs and never queuing newly opened issues. `drain-stall`/`drain-pass` (Phase: Drain Parked PRs already maintains these) are what distinguish the two: if they show the stall/cap threshold was already hit, this `parked.txt` is a reported terminal state, not a resume signal. + +So the fresh-vs-resume decision is: a previous run is resumable if `queue.json` is non-empty (mid-batch), OR `parked.txt` is non-empty **and** its drain had not yet reached the stall/cap threshold (mid-drain, genuinely interrupted). Only when neither holds did the last invocation run all the way to a reported completion, so only then is every run-scoped artifact — including `batches-done`, `queue.json`, `parked.txt`, `drain-pass`, and `drain-stall` — reset. Conflating these cases is exactly how a stale `batches-done` count or a stale empty `queue.json` leaks into a brand-new invocation and silently skips newly opened issues or hits the batch safety cap prematurely; how an interruption mid-drain (e.g. between `parked.txt`'s removal of a just-merged PR and the separate `state.json` update reconciling it) gets read as "nothing left to do," deleting `parked.txt` and wiping `state.json` before Phase: Drain Parked PRs' own self-heal step ever gets a chance to run; and, in the opposite direction, how a drain that already stopped and reported its blocked PRs gets misread as still-interrupted, permanently locking out new work. ```bash mkdir -p .codegraph/fixer @@ -153,14 +155,28 @@ QUEUE_LEN=$(jq 'length' .codegraph/fixer/queue.json 2>/dev/null || echo 0) # in progress rather than long since complete. PARKED_LEN=0 [ -s .codegraph/fixer/parked.txt ] && PARKED_LEN=$(wc -l < .codegraph/fixer/parked.txt) -if { [ -f .codegraph/fixer/queue.json ] && [ "$QUEUE_LEN" -gt 0 ]; } || [ "$PARKED_LEN" -gt 0 ]; then +# A non-empty parked.txt is ONLY a resume signal if the drain that left it there hadn't +# already reached its own stall/cap threshold — Phase: Drain Parked PRs stops and reports +# to the human at exactly that threshold, so parked.txt at that point is a closed matter, +# not an interruption. Without this check, a drain that legitimately finished by reporting +# blocked PRs would look identical to one cut off mid-flight forever, permanently skipping +# Phase 1-3 and starving newly opened issues of any batch. +DRAIN_STALL=$(cat .codegraph/fixer/drain-stall 2>/dev/null || echo 0) +DRAIN_PASS_COUNT=$(cat .codegraph/fixer/drain-pass 2>/dev/null || echo 0) +DRAIN_ALREADY_REPORTED=0 +if [ "$PARKED_LEN" -gt 0 ] && { [ "$DRAIN_STALL" -ge 3 ] || [ "$DRAIN_PASS_COUNT" -ge 15 ]; }; then + DRAIN_ALREADY_REPORTED=1 +fi +if { [ -f .codegraph/fixer/queue.json ] && [ "$QUEUE_LEN" -gt 0 ]; } || { [ "$PARKED_LEN" -gt 0 ] && [ "$DRAIN_ALREADY_REPORTED" -eq 0 ]; }; then echo "fixer: resuming an interrupted run — previous state:" jq -r '.issues[] | " #\(.issue) \(.status) \(if .pr then "PR #\(.pr)" else "no PR" end)"' .codegraph/fixer/state.json 2>/dev/null if [ "$QUEUE_LEN" -eq 0 ] && [ "$PARKED_LEN" -gt 0 ]; then echo "fixer: queue.json is empty but parked.txt has $PARKED_LEN PR(s) — the batch loop had already finished and this run was mid-drain. Skip Phase 1-3 entirely and go straight to Phase: Drain Parked PRs." fi else - if [ -f .codegraph/fixer/state.json ]; then + if [ "$DRAIN_ALREADY_REPORTED" -eq 1 ]; then + echo "fixer: previous run's drain already stopped and reported $PARKED_LEN blocked PR(s) to a human (stall=$DRAIN_STALL pass=$DRAIN_PASS_COUNT) — that run is done. Starting a genuinely fresh run, not resuming it." + elif [ -f .codegraph/fixer/state.json ]; then echo "fixer: previous run's artifacts are still on disk but its last batch already completed — that run is done. Starting a genuinely fresh run, not resuming it." else echo "fixer: fresh run — state initialised" @@ -173,7 +189,7 @@ else fi ``` -**Exit condition:** `git`, `gh`, `jq`, `mktemp` present; inside a git repo; `gh` authenticated; no in-progress merge; repo slug, test command, and lint command persisted to `.codegraph/fixer/`; arguments parsed and persisted; worktree isolation confirmed or requested; `state.json` exists. If resuming with an empty `queue.json` but a non-empty `parked.txt`, proceed directly to Phase: Drain Parked PRs — do not rebuild or reuse a queue for a batch loop that already finished. +**Exit condition:** `git`, `gh`, `jq`, `mktemp` present; inside a git repo; `gh` authenticated; no in-progress merge; repo slug, test command, and lint command persisted to `.codegraph/fixer/`; arguments parsed and persisted; worktree isolation confirmed or requested; `state.json` exists. If resuming with an empty `queue.json` but a non-empty `parked.txt` whose drain had not yet stalled/capped out, proceed directly to Phase: Drain Parked PRs — do not rebuild or reuse a queue for a batch loop that already finished. If the previous drain had already stalled/capped out and reported its blocked PRs, this is a fresh run: those specific PRs remain open on GitHub for a human (or a future `/fixer`/`/sweep`/`/resolve` invocation) to pick up — they are not silently lost, only no longer tracked in this run's local state. --- @@ -1065,7 +1081,7 @@ Also list, explicitly: - every issue that was split, with the follow-up covering the remainder - anything left for a human, with the specific blocker -**Cleanup.** State lives in `.codegraph/fixer/`. It is left on disk after this run finishes, but Phase 0's resume handling already treats a completed run's artifacts as stale on the *next* invocation — an empty `queue.json` **and** an empty `parked.txt` together mean the next `/fixer` call starts genuinely fresh, not "resuming" this run's results. The only thing left on disk that matters afterward is `state.json` as a historical record of what this run did; `rm -rf .codegraph/fixer` removes it entirely if you don't want even that. +**Cleanup.** State lives in `.codegraph/fixer/`. It is left on disk after this run finishes, but Phase 0's resume handling already treats a completed run's artifacts as stale on the *next* invocation — an empty `queue.json`, together with a `parked.txt` that is either empty or already stalled/capped out (per `drain-stall`/`drain-pass`), means the next `/fixer` call starts genuinely fresh, not "resuming" this run's results. The only thing left on disk that matters afterward is `state.json` as a historical record of what this run did; `rm -rf .codegraph/fixer` removes it entirely if you don't want even that. **Exit condition:** The user has a per-issue table covering every batch this run processed, the merged count, the batch count, every follow-up issue number, every bypass with its justification, and an explicit list of anything unfinished (including a resume command if the 15-batch safety cap was hit). @@ -1082,7 +1098,7 @@ All state is under `.codegraph/fixer/`: | `test-cmd`, `lint-cmd` | text | detected package-manager commands | | `queue.json` | JSON array of `{issue, title}` | current batch's remaining work, ascending; entries are shifted off as they complete. **A present-but-empty `queue.json` means the batch loop fully completed** — Phase 0 uses this, together with `parked.txt`, to decide fresh-run vs. resume | | `state.json` | `{"issues":[{issue, status, pr}]}` | per-issue outcome, accumulated across every batch this run has processed; reset to empty whenever Phase 0 detects a fresh run | -| `parked.txt` | newline-separated PR numbers | accumulated across every batch; input to Phase: Drain Parked PRs; a merged PR's number is removed from this file the moment it merges. **Only ever emptied, never deleted, by a completed drain** — Phase 0 also treats a non-empty `parked.txt` as evidence of a resumable (mid-drain) run, since `queue.json` is legitimately already empty by the time draining starts | +| `parked.txt` | newline-separated PR numbers | accumulated across every batch; input to Phase: Drain Parked PRs; a merged PR's number is removed from this file the moment it merges. **Only ever emptied, never deleted, by a fully-merged drain** — Phase 0 treats a non-empty `parked.txt` as evidence of a resumable (mid-drain) run only if `drain-stall`/`drain-pass` show the drain hadn't already stalled/capped out; once it has, the remaining entries are a closed, already-reported matter, not a resume signal | | `batches-done` | text | count of batches completed this run; read by Phase 3 and the final report; reset on a fresh run | | `loop-decision` | text | `loop`/`stop` — Phase 3's verdict on whether to start another batch | | `current-pr`, `last-pr-url`, `gate-fail` | text | current iteration's scratch state | @@ -1090,7 +1106,7 @@ All state is under `.codegraph/fixer/`: | `round` | text | convergence-round counter for the current PR; cleared once it merges or parks | | `gate-signature`, `prev-gate-signature` | text | fingerprint of this round's / the previous round's gate state (score, a hash of the full Greptile review text, which specific comments are unanswered, branch ancestry, mergeability, each non-green check's name plus its completion timestamp, and the current commit SHA), used to detect a stalled (blocked) PR rather than counting rounds | | `stall-count` | text | consecutive convergence rounds with an unchanged gate signature; park threshold is 3 | -| `drain-pass`, `drain-stall`, `drain-parked-count` | text | drain-phase pass counter, consecutive no-merge passes, and `parked.txt` length as of the last pass — the same stall-detection pattern applied to draining | +| `drain-pass`, `drain-stall`, `drain-parked-count` | text | drain-phase pass counter, consecutive no-merge passes, and `parked.txt` length as of the last pass — the same stall-detection pattern applied to draining. Also read by Phase 0 on the next invocation to tell a genuinely-interrupted drain (resumable) apart from one that already stalled/capped out and reported its blocked PRs (not resumable) | | `authored-lines.tsv`, `authored-files.txt` | text | I4 integrity-check baseline (tab-separated `filecountline`) | --- @@ -1130,7 +1146,7 @@ All state is under `.codegraph/fixer/`: - **The run's objective is the whole backlog, not one batch.** By default, keep starting new batches (Phase 3) until no qualifying open issues remain or the 15-batch cap is hit. Pass `--once` to process exactly one batch and stop. - **Only ever act on the repo detected in Phase 0.** Never search, open issues on, or open PRs against any other repository — including when this repo's queue comes up empty. Read the slug from `.codegraph/fixer/repo`; never hardcode it (see issue #2164). - **An empty queue is success, not failure.** If Phase 1 finds no qualifying open issues, report that plainly and exit 0 — never search other repos, never lower the `--author`/`blocked`-label bar to manufacture a queue, and never invent unrelated work (stale files, refactors, drive-by fixes) to fill a batch. -- **A completed run is not a resumable run.** Phase 0 resumes when `queue.json` is non-empty (a batch stopped mid-flight) OR `parked.txt` is non-empty (draining stopped mid-flight — `queue.json` is legitimately already empty by the time Phase: Drain Parked PRs starts, so it alone cannot tell "draining in progress" apart from "run genuinely complete"). Only when BOTH are empty or absent did the previous run finish, so only then is every run-scoped artifact (`batches-done`, `queue.json`, `parked.txt`, `state.json`, per-issue scratch state) reset before starting — never let a finished run's counters leak into a new invocation and skip issues or hit the batch cap early, and never let an interrupted drain be mistaken for a finished one and lose its still-parked PRs. +- **A completed run is not a resumable run — but neither is a drain that already gave up and reported.** Phase 0 resumes when `queue.json` is non-empty (a batch stopped mid-flight), OR `parked.txt` is non-empty AND `drain-stall`/`drain-pass` show the drain had not yet stalled/capped out (draining stopped mid-flight — `queue.json` is legitimately already empty by the time Phase: Drain Parked PRs starts, so it alone cannot tell "draining in progress" apart from "run genuinely complete"). A `parked.txt` left behind by a drain that already hit 3 stalled passes or the 15-pass cap is a closed, already-reported matter, not a resume signal — treating it as one would make every future invocation skip Phase 1-3 forever and starve newly opened issues. Only when none of these hold did the previous run finish, so only then is every run-scoped artifact (`batches-done`, `queue.json`, `parked.txt`, `drain-pass`, `drain-stall`, `state.json`, per-issue scratch state) reset before starting — never let a finished run's counters leak into a new invocation and skip issues or hit the batch cap early, never let an interrupted drain be mistaken for a finished one and lose its still-parked PRs, and never let an already-reported blocked drain be mistaken for an interrupted one and lock out new work indefinitely. - **No co-author lines** in commit messages or PR bodies. - **No Claude Code or Anthropic references** in commits, PR bodies, comments, or issues. - **Never trigger `@greptileai` until every Greptile comment has a reply.** Do not trigger Greptile for feedback that came from Claude or the user. From ebfd3b743f6e47ee7eaaf670420e555865f84ce6 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 29 Jul 2026 02:55:47 -0700 Subject: [PATCH 13/13] fix: use an explicit run-complete marker instead of racy drain counters (#2169) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit distinguished a genuinely-interrupted drain from an already-reported blocked one by checking whether drain-stall/drain-pass had crossed their stall/cap threshold. That check is itself racy: those counters are written mid-Phase-4, before Phase: Final Report ever runs. A process stopping in the narrow window after a counter crosses its threshold but before the report is actually produced would be misclassified as "already reported," deleting parked.txt and state.json before anyone ever saw the blocked-PR report. Phase: Final Report now touches .codegraph/fixer/run-complete as its last action, strictly after producing the report. Phase 0 uses only this marker's presence to decide whether a non-empty parked.txt is a closed, reported matter (fresh run) or still needs draining (resume) — decoupling the fresh-vs-resume decision from Phase 4's internal stall-detection counters entirely. Resuming into an already-thresholded-but-unreported drain runs one extra (cheap) pass before Phase: Drain Parked PRs' own bookkeeping stops it again, which is what finally lets the report happen and the marker get written. Verified all six relevant state combinations standalone, including the exact race the previous commit missed, before landing this. --- .claude/skills/fixer/SKILL.md | 60 ++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/.claude/skills/fixer/SKILL.md b/.claude/skills/fixer/SKILL.md index 79925970..06d4a5be 100644 --- a/.claude/skills/fixer/SKILL.md +++ b/.claude/skills/fixer/SKILL.md @@ -141,9 +141,11 @@ case "$(git rev-parse --show-toplevel)" in esac ``` -**Resume handling.** A non-empty `queue.json` means a previous invocation stopped mid-batch (crash, interruption) with unresolved work. A non-empty `parked.txt` *can* mean a previous invocation stopped mid-drain (Phase: Drain Parked PRs runs *after* `queue.json` is legitimately already empty, so `queue.json` alone cannot distinguish "batch loop finished, draining in progress" from "run genuinely complete") — but only if that drain hadn't already reached ITS OWN terminal state. Phase: Drain Parked PRs' own exit condition is "every PR merged, or reported as needing human review" — once a drain stops because 3 consecutive passes merged nothing, or the 15-pass cap was hit, the remaining entries in `parked.txt` are a **closed, already-reported matter for that run**, not evidence of an interruption. Treating a closed-but-blocked drain the same as a genuinely-interrupted one would make every subsequent invocation skip Phase 1-3 forever, re-attempting the same permanently-stuck PRs and never queuing newly opened issues. `drain-stall`/`drain-pass` (Phase: Drain Parked PRs already maintains these) are what distinguish the two: if they show the stall/cap threshold was already hit, this `parked.txt` is a reported terminal state, not a resume signal. +**Resume handling.** A non-empty `queue.json` means a previous invocation stopped mid-batch (crash, interruption) with unresolved work. A non-empty `parked.txt` *can* mean a previous invocation stopped mid-drain (Phase: Drain Parked PRs runs *after* `queue.json` is legitimately already empty, so `queue.json` alone cannot distinguish "batch loop finished, draining in progress" from "run genuinely complete") — but only if Phase: Final Report never actually ran for it. Phase: Drain Parked PRs' own exit condition is "every PR merged, or reported as needing human review," and that reporting happens in Phase: Final Report, not the moment a stall/cap counter crosses its threshold — a counter-based check (e.g. "`drain-stall` reached 3") is racy: the process can stop in the narrow window after Phase: Drain Parked PRs persists that threshold but before Phase: Final Report ever produces the report the human is supposed to see, and a counter-only check would misclassify that as "already reported" and delete the evidence before it was ever shown to anyone. -So the fresh-vs-resume decision is: a previous run is resumable if `queue.json` is non-empty (mid-batch), OR `parked.txt` is non-empty **and** its drain had not yet reached the stall/cap threshold (mid-drain, genuinely interrupted). Only when neither holds did the last invocation run all the way to a reported completion, so only then is every run-scoped artifact — including `batches-done`, `queue.json`, `parked.txt`, `drain-pass`, and `drain-stall` — reset. Conflating these cases is exactly how a stale `batches-done` count or a stale empty `queue.json` leaks into a brand-new invocation and silently skips newly opened issues or hits the batch safety cap prematurely; how an interruption mid-drain (e.g. between `parked.txt`'s removal of a just-merged PR and the separate `state.json` update reconciling it) gets read as "nothing left to do," deleting `parked.txt` and wiping `state.json` before Phase: Drain Parked PRs' own self-heal step ever gets a chance to run; and, in the opposite direction, how a drain that already stopped and reported its blocked PRs gets misread as still-interrupted, permanently locking out new work. +**`.codegraph/fixer/run-complete`** closes that race: Phase: Final Report touches it as its last action, strictly after the report has been fully produced. Its presence is the only authoritative signal that a run's outcome — including any blocked PRs left in `parked.txt` — was actually reported, not merely that some internal counter reached a threshold. So the fresh-vs-resume decision is: a previous run is resumable if `queue.json` is non-empty (mid-batch), OR `parked.txt` is non-empty **and** `run-complete` is absent (mid-drain — whether merely interrupted, or already past its stall/cap threshold but never actually reported). Only when neither holds — i.e. `parked.txt` is non-empty only alongside a present `run-complete`, or `parked.txt` is empty entirely — did the last invocation run all the way to a reported completion, so only then is every run-scoped artifact — including `batches-done`, `queue.json`, `parked.txt`, `drain-pass`, `drain-stall`, and `run-complete` itself — reset. Conflating these cases is exactly how a stale `batches-done` count or a stale empty `queue.json` leaks into a brand-new invocation and silently skips newly opened issues or hits the batch safety cap prematurely; how an interruption mid-drain (e.g. between `parked.txt`'s removal of a just-merged PR and the separate `state.json` update reconciling it) gets read as "nothing left to do," deleting `parked.txt` and wiping `state.json` before Phase: Drain Parked PRs' own self-heal step ever gets a chance to run; and, in the opposite direction, how a drain that merely *crossed* its stall/cap threshold — without Phase: Final Report ever having run — gets misread as already reported, permanently locking out new work while never having actually told anyone about the blocked PRs. + +Resuming into a mid-drain state that had already crossed its stall/cap threshold (but was never reported) is safe even though it means running one more drain pass before Phase: Drain Parked PRs' own bookkeeping stops it again: that extra pass is cheap, and it is what finally lets Phase: Final Report run and write `run-complete`, closing the loop for every invocation after this one. ```bash mkdir -p .codegraph/fixer @@ -155,27 +157,22 @@ QUEUE_LEN=$(jq 'length' .codegraph/fixer/queue.json 2>/dev/null || echo 0) # in progress rather than long since complete. PARKED_LEN=0 [ -s .codegraph/fixer/parked.txt ] && PARKED_LEN=$(wc -l < .codegraph/fixer/parked.txt) -# A non-empty parked.txt is ONLY a resume signal if the drain that left it there hadn't -# already reached its own stall/cap threshold — Phase: Drain Parked PRs stops and reports -# to the human at exactly that threshold, so parked.txt at that point is a closed matter, -# not an interruption. Without this check, a drain that legitimately finished by reporting -# blocked PRs would look identical to one cut off mid-flight forever, permanently skipping -# Phase 1-3 and starving newly opened issues of any batch. -DRAIN_STALL=$(cat .codegraph/fixer/drain-stall 2>/dev/null || echo 0) -DRAIN_PASS_COUNT=$(cat .codegraph/fixer/drain-pass 2>/dev/null || echo 0) -DRAIN_ALREADY_REPORTED=0 -if [ "$PARKED_LEN" -gt 0 ] && { [ "$DRAIN_STALL" -ge 3 ] || [ "$DRAIN_PASS_COUNT" -ge 15 ]; }; then - DRAIN_ALREADY_REPORTED=1 -fi -if { [ -f .codegraph/fixer/queue.json ] && [ "$QUEUE_LEN" -gt 0 ]; } || { [ "$PARKED_LEN" -gt 0 ] && [ "$DRAIN_ALREADY_REPORTED" -eq 0 ]; }; then +# run-complete is written ONLY by Phase: Final Report, as its last action, strictly after +# the report has been produced. Its presence — not a stall/cap counter, which gets written +# mid-Phase-4 and races against Phase: Final Report actually running — is the sole +# authoritative signal that a run (and any blocked PRs left in parked.txt) was truly +# reported and finished, rather than merely having crossed an internal threshold. +RUN_COMPLETE=0 +[ -f .codegraph/fixer/run-complete ] && RUN_COMPLETE=1 +if { [ -f .codegraph/fixer/queue.json ] && [ "$QUEUE_LEN" -gt 0 ]; } || { [ "$PARKED_LEN" -gt 0 ] && [ "$RUN_COMPLETE" -eq 0 ]; }; then echo "fixer: resuming an interrupted run — previous state:" jq -r '.issues[] | " #\(.issue) \(.status) \(if .pr then "PR #\(.pr)" else "no PR" end)"' .codegraph/fixer/state.json 2>/dev/null if [ "$QUEUE_LEN" -eq 0 ] && [ "$PARKED_LEN" -gt 0 ]; then - echo "fixer: queue.json is empty but parked.txt has $PARKED_LEN PR(s) — the batch loop had already finished and this run was mid-drain. Skip Phase 1-3 entirely and go straight to Phase: Drain Parked PRs." + echo "fixer: queue.json is empty but parked.txt has $PARKED_LEN PR(s) and no run-complete marker — the batch loop had already finished and this run was mid-drain (interrupted, or stalled/capped but never reported). Skip Phase 1-3 entirely and go straight to Phase: Drain Parked PRs." fi else - if [ "$DRAIN_ALREADY_REPORTED" -eq 1 ]; then - echo "fixer: previous run's drain already stopped and reported $PARKED_LEN blocked PR(s) to a human (stall=$DRAIN_STALL pass=$DRAIN_PASS_COUNT) — that run is done. Starting a genuinely fresh run, not resuming it." + if [ "$PARKED_LEN" -gt 0 ]; then + echo "fixer: previous run's drain already finished and its $PARKED_LEN blocked PR(s) were reported in Phase: Final Report (run-complete present) — that run is done. Starting a genuinely fresh run, not resuming it." elif [ -f .codegraph/fixer/state.json ]; then echo "fixer: previous run's artifacts are still on disk but its last batch already completed — that run is done. Starting a genuinely fresh run, not resuming it." else @@ -185,11 +182,12 @@ else rm -f .codegraph/fixer/queue.json .codegraph/fixer/parked.txt .codegraph/fixer/batches-done .codegraph/fixer/loop-decision \ .codegraph/fixer/outcome .codegraph/fixer/round .codegraph/fixer/current-pr .codegraph/fixer/gate-fail \ .codegraph/fixer/stall-count .codegraph/fixer/gate-signature .codegraph/fixer/prev-gate-signature \ - .codegraph/fixer/drain-pass .codegraph/fixer/drain-stall .codegraph/fixer/drain-parked-count + .codegraph/fixer/drain-pass .codegraph/fixer/drain-stall .codegraph/fixer/drain-parked-count \ + .codegraph/fixer/run-complete fi ``` -**Exit condition:** `git`, `gh`, `jq`, `mktemp` present; inside a git repo; `gh` authenticated; no in-progress merge; repo slug, test command, and lint command persisted to `.codegraph/fixer/`; arguments parsed and persisted; worktree isolation confirmed or requested; `state.json` exists. If resuming with an empty `queue.json` but a non-empty `parked.txt` whose drain had not yet stalled/capped out, proceed directly to Phase: Drain Parked PRs — do not rebuild or reuse a queue for a batch loop that already finished. If the previous drain had already stalled/capped out and reported its blocked PRs, this is a fresh run: those specific PRs remain open on GitHub for a human (or a future `/fixer`/`/sweep`/`/resolve` invocation) to pick up — they are not silently lost, only no longer tracked in this run's local state. +**Exit condition:** `git`, `gh`, `jq`, `mktemp` present; inside a git repo; `gh` authenticated; no in-progress merge; repo slug, test command, and lint command persisted to `.codegraph/fixer/`; arguments parsed and persisted; worktree isolation confirmed or requested; `state.json` exists. If resuming with an empty `queue.json` but a non-empty `parked.txt` and no `run-complete` marker, proceed directly to Phase: Drain Parked PRs — do not rebuild or reuse a queue for a batch loop that already finished. If `run-complete` is present alongside a non-empty `parked.txt`, this is a fresh run: those specific PRs were already reported and remain open on GitHub for a human (or a future `/fixer`/`/sweep`/`/resolve` invocation) to pick up — they are not silently lost, only no longer tracked in this run's local state. --- @@ -1081,9 +1079,18 @@ Also list, explicitly: - every issue that was split, with the follow-up covering the remainder - anything left for a human, with the specific blocker -**Cleanup.** State lives in `.codegraph/fixer/`. It is left on disk after this run finishes, but Phase 0's resume handling already treats a completed run's artifacts as stale on the *next* invocation — an empty `queue.json`, together with a `parked.txt` that is either empty or already stalled/capped out (per `drain-stall`/`drain-pass`), means the next `/fixer` call starts genuinely fresh, not "resuming" this run's results. The only thing left on disk that matters afterward is `state.json` as a historical record of what this run did; `rm -rf .codegraph/fixer` removes it entirely if you don't want even that. +**Mark the run as reported.** As the last action of this phase, after the table and lists above have actually been produced — not before: + +```bash +mkdir -p .codegraph/fixer +: > .codegraph/fixer/run-complete +``` + +This is what Phase 0's resume handling checks on the next invocation. Writing it any earlier (e.g. alongside the stall/cap counters in Phase: Drain Parked PRs) would reintroduce the exact race this file exists to close: a process stopping after a counter crosses its threshold but before this report ever reaches the human would otherwise look identical to a genuinely-reported run. + +**Cleanup.** State lives in `.codegraph/fixer/`. It is left on disk after this run finishes, but Phase 0's resume handling already treats a completed run's artifacts as stale on the *next* invocation — an empty `queue.json`, together with a `parked.txt` that is either empty or accompanied by `run-complete`, means the next `/fixer` call starts genuinely fresh, not "resuming" this run's results. The only thing left on disk that matters afterward is `state.json` as a historical record of what this run did; `rm -rf .codegraph/fixer` removes it entirely if you don't want even that. -**Exit condition:** The user has a per-issue table covering every batch this run processed, the merged count, the batch count, every follow-up issue number, every bypass with its justification, and an explicit list of anything unfinished (including a resume command if the 15-batch safety cap was hit). +**Exit condition:** The user has a per-issue table covering every batch this run processed, the merged count, the batch count, every follow-up issue number, every bypass with its justification, and an explicit list of anything unfinished (including a resume command if the 15-batch safety cap was hit); `run-complete` has been written after that report was produced. --- @@ -1096,9 +1103,10 @@ All state is under `.codegraph/fixer/`: | `repo` | text | `owner/name` slug — the only repo this run ever touches | | `count`, `author`, `start-from`, `once`, `dry-run` | text | parsed arguments | | `test-cmd`, `lint-cmd` | text | detected package-manager commands | -| `queue.json` | JSON array of `{issue, title}` | current batch's remaining work, ascending; entries are shifted off as they complete. **A present-but-empty `queue.json` means the batch loop fully completed** — Phase 0 uses this, together with `parked.txt`, to decide fresh-run vs. resume | +| `queue.json` | JSON array of `{issue, title}` | current batch's remaining work, ascending; entries are shifted off as they complete. **A present-but-empty `queue.json` means the batch loop fully completed** — Phase 0 uses this, together with `parked.txt` and `run-complete`, to decide fresh-run vs. resume | | `state.json` | `{"issues":[{issue, status, pr}]}` | per-issue outcome, accumulated across every batch this run has processed; reset to empty whenever Phase 0 detects a fresh run | -| `parked.txt` | newline-separated PR numbers | accumulated across every batch; input to Phase: Drain Parked PRs; a merged PR's number is removed from this file the moment it merges. **Only ever emptied, never deleted, by a fully-merged drain** — Phase 0 treats a non-empty `parked.txt` as evidence of a resumable (mid-drain) run only if `drain-stall`/`drain-pass` show the drain hadn't already stalled/capped out; once it has, the remaining entries are a closed, already-reported matter, not a resume signal | +| `parked.txt` | newline-separated PR numbers | accumulated across every batch; input to Phase: Drain Parked PRs; a merged PR's number is removed from this file the moment it merges. **Only ever emptied, never deleted, by a fully-merged drain** — Phase 0 treats a non-empty `parked.txt` as evidence of a resumable (mid-drain) run only if `run-complete` is absent; once Phase: Final Report has actually run and written it, the remaining entries are a closed, already-reported matter, not a resume signal | +| `run-complete` | empty marker file | touched by Phase: Final Report as its last action, strictly after the report has been produced — the sole authoritative "this run's outcome was reported" signal Phase 0 checks, deliberately decoupled from `drain-stall`/`drain-pass` (which get written mid-Phase-4 and would otherwise race against the report actually happening); reset (removed) whenever Phase 0 starts a genuinely fresh run | | `batches-done` | text | count of batches completed this run; read by Phase 3 and the final report; reset on a fresh run | | `loop-decision` | text | `loop`/`stop` — Phase 3's verdict on whether to start another batch | | `current-pr`, `last-pr-url`, `gate-fail` | text | current iteration's scratch state | @@ -1106,7 +1114,7 @@ All state is under `.codegraph/fixer/`: | `round` | text | convergence-round counter for the current PR; cleared once it merges or parks | | `gate-signature`, `prev-gate-signature` | text | fingerprint of this round's / the previous round's gate state (score, a hash of the full Greptile review text, which specific comments are unanswered, branch ancestry, mergeability, each non-green check's name plus its completion timestamp, and the current commit SHA), used to detect a stalled (blocked) PR rather than counting rounds | | `stall-count` | text | consecutive convergence rounds with an unchanged gate signature; park threshold is 3 | -| `drain-pass`, `drain-stall`, `drain-parked-count` | text | drain-phase pass counter, consecutive no-merge passes, and `parked.txt` length as of the last pass — the same stall-detection pattern applied to draining. Also read by Phase 0 on the next invocation to tell a genuinely-interrupted drain (resumable) apart from one that already stalled/capped out and reported its blocked PRs (not resumable) | +| `drain-pass`, `drain-stall`, `drain-parked-count` | text | drain-phase pass counter, consecutive no-merge passes, and `parked.txt` length as of the last pass — the same stall-detection pattern applied to draining. Purely internal to Phase: Drain Parked PRs' own loop control; Phase 0 does not read these to decide fresh-vs-resume (see `run-complete`) since they get written mid-phase and would race against Phase: Final Report actually running | | `authored-lines.tsv`, `authored-files.txt` | text | I4 integrity-check baseline (tab-separated `filecountline`) | --- @@ -1146,7 +1154,7 @@ All state is under `.codegraph/fixer/`: - **The run's objective is the whole backlog, not one batch.** By default, keep starting new batches (Phase 3) until no qualifying open issues remain or the 15-batch cap is hit. Pass `--once` to process exactly one batch and stop. - **Only ever act on the repo detected in Phase 0.** Never search, open issues on, or open PRs against any other repository — including when this repo's queue comes up empty. Read the slug from `.codegraph/fixer/repo`; never hardcode it (see issue #2164). - **An empty queue is success, not failure.** If Phase 1 finds no qualifying open issues, report that plainly and exit 0 — never search other repos, never lower the `--author`/`blocked`-label bar to manufacture a queue, and never invent unrelated work (stale files, refactors, drive-by fixes) to fill a batch. -- **A completed run is not a resumable run — but neither is a drain that already gave up and reported.** Phase 0 resumes when `queue.json` is non-empty (a batch stopped mid-flight), OR `parked.txt` is non-empty AND `drain-stall`/`drain-pass` show the drain had not yet stalled/capped out (draining stopped mid-flight — `queue.json` is legitimately already empty by the time Phase: Drain Parked PRs starts, so it alone cannot tell "draining in progress" apart from "run genuinely complete"). A `parked.txt` left behind by a drain that already hit 3 stalled passes or the 15-pass cap is a closed, already-reported matter, not a resume signal — treating it as one would make every future invocation skip Phase 1-3 forever and starve newly opened issues. Only when none of these hold did the previous run finish, so only then is every run-scoped artifact (`batches-done`, `queue.json`, `parked.txt`, `drain-pass`, `drain-stall`, `state.json`, per-issue scratch state) reset before starting — never let a finished run's counters leak into a new invocation and skip issues or hit the batch cap early, never let an interrupted drain be mistaken for a finished one and lose its still-parked PRs, and never let an already-reported blocked drain be mistaken for an interrupted one and lock out new work indefinitely. +- **A completed run is not a resumable run — but neither is a drain that already gave up and reported.** Phase 0 resumes when `queue.json` is non-empty (a batch stopped mid-flight), OR `parked.txt` is non-empty AND `run-complete` is absent (draining stopped mid-flight — `queue.json` is legitimately already empty by the time Phase: Drain Parked PRs starts, so it alone cannot tell "draining in progress" apart from "run genuinely complete"). A `parked.txt` left behind by a drain that already reported — `run-complete` present — is a closed, already-reported matter, not a resume signal — treating it as one would make every future invocation skip Phase 1-3 forever and starve newly opened issues. Use `run-complete`, not `drain-stall`/`drain-pass`, for this check: those counters are written mid-Phase-4 and race against Phase: Final Report actually running, so a process stopping right after a counter crosses its threshold but before the report is produced would be misclassified as "already reported" by a counter-only check. Only when none of the resume conditions hold did the previous run finish, so only then is every run-scoped artifact (`batches-done`, `queue.json`, `parked.txt`, `drain-pass`, `drain-stall`, `run-complete`, `state.json`, per-issue scratch state) reset before starting — never let a finished run's counters leak into a new invocation and skip issues or hit the batch cap early, never let an interrupted drain be mistaken for a finished one and lose its still-parked PRs, and never let an already-reported blocked drain be mistaken for an interrupted one and lock out new work indefinitely. - **No co-author lines** in commit messages or PR bodies. - **No Claude Code or Anthropic references** in commits, PR bodies, comments, or issues. - **Never trigger `@greptileai` until every Greptile comment has a reply.** Do not trigger Greptile for feedback that came from Claude or the user.