From 1e4df4ba6e56bde6a4665ba0bb4a0111eea7a53a Mon Sep 17 00:00:00 2001 From: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Date: Thu, 30 Jul 2026 10:20:07 -0700 Subject: [PATCH] fix(queue): absorb item-lifecycle and throttle stalls without spending review-failure budget --- .github/workflows/sweep.yml | 6 +++ CHANGELOG.md | 2 + dashboard/wrangler.toml | 11 ++++-- src/clawsweeper.ts | 9 ++++- test/command.test.ts | 74 +++++++++++++++++++++++++++++++++++ test/dashboard-worker.test.ts | 2 +- test/sweep-workflow.test.ts | 6 +++ 7 files changed, 103 insertions(+), 7 deletions(-) diff --git a/.github/workflows/sweep.yml b/.github/workflows/sweep.yml index d7d320976d..2713203f87 100644 --- a/.github/workflows/sweep.yml +++ b/.github/workflows/sweep.yml @@ -963,6 +963,12 @@ jobs: else reserve_exit=$? fi + if grep -Eqi 'rate limit exceeded|secondary rate limit|HTTP 429' "$reservation_error"; then + retry_at="$(date -u -d "@$(( $(date -u +%s) + 1200 ))" +%Y-%m-%dT%H:%M:%SZ)" + echo "::notice::GitHub throttled the exact-review reservation; deferring until $retry_at so the durable queue retries without spending failure budget." + reservation="{\"status\":\"held\",\"retryAt\":\"$retry_at\"}" + break + fi if ! grep -Eqi '(exact-review queue authority changed|review revision changed|could not (confirm|identify).+review lease).+retry required' "$reservation_error"; then cat "$reservation_error" >&2 exit "$reserve_exit" diff --git a/CHANGELOG.md b/CHANGELOG.md index e4a9595342..06ad1859e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ checkpoint, and status-only commits are intentionally omitted. ### Changed +- Exact reviews of items that closed after enqueue now complete as superseded no-ops, and GitHub-throttled reservations defer as held retries — neither spends the item's review-failure budget. +- Trimmed the exact-review feed rate from 600/h to 450/h after the resumed apply and comment-sync lanes pushed the shared GitHub App installation token into rate-limit 403s. - Exact reviews whose pull request head moved past the queued authority now complete as superseded no-ops (the newer push owns its own review) instead of failing and burning the item's review-failure budget. - Close-coverage proof runs now remove their model scratch files (`N-M.model.json`, `N-M.prompt.md`) from the proof artifact tree, which the apply-lane validator was correctly rejecting and failing every apply run. - Repaired legacy exact-review decisions whose empty branch was shifted to the string `0`, resolved invalid queued branches from the target repository default, and requeued temporary branch-resolution failures without spending the eight-attempt review-failure budget. diff --git a/dashboard/wrangler.toml b/dashboard/wrangler.toml index 30c991e254..497319ac37 100644 --- a/dashboard/wrangler.toml +++ b/dashboard/wrangler.toml @@ -66,10 +66,13 @@ EXACT_REVIEW_DISPATCH_DEBOUNCE_MAX_MS = "180000" EXACT_REVIEW_PENDING_SOFT_LIMIT = "600" # 200/h only sustained ~14 concurrent reviews against a 128-slot pool; with the # review/publication lane split (#953) the queue drains to zero pending while -# 3,306 items have never been reviewed at all (42.9% weekly coverage). 600/h at -# the measured 4.1-minute service time targets ~41 concurrent, clearing the -# first-pass backlog in hours instead of days. Model spend scales with this dial. -EXACT_REVIEW_TARGET_RATE_PER_HOUR = "600" +# 3,306 items have never been reviewed at all (42.9% weekly coverage). Model +# spend scales with this dial. 600/h saturated the GitHub App installation +# token budget once the apply and comment-sync lanes resumed alongside reviews +# (installation 122230863 rate-limit 403s, 2026-07-30 16:56Z): at ~30 API calls +# per review, 450/h leaves the write lanes headroom inside the shared allowance +# while still clearing the first-pass backlog in about a day. +EXACT_REVIEW_TARGET_RATE_PER_HOUR = "450" EXACT_REVIEW_TARGET_BURST = "120" # Keep each mutation lease at 8 while allowing four isolated workflows to prepare # concurrently. Final commits still cross the single state-writer coordinator. diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index ba65cace96..582139c5a5 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -22924,9 +22924,14 @@ function reserveReviewLeaseCommand(args: Args): void { } const { item, state } = fetchItem(itemNumber); if (state !== "open") { - throw new UserFacingCommandError( - `Cannot reserve a review lease for #${itemNumber}: state is ${state}.`, + // The item was closed between enqueue and review (typically by the apply + // lane or its author). The stale entry completes as a superseded no-op + // rather than burning the item's review-failure budget. + console.error( + `Item #${itemNumber} is ${state}; completing the reservation as a superseded no-op.`, ); + console.log(JSON.stringify({ status: "superseded", reason: "item_not_open", state })); + return; } const queueAuthority = exactReviewQueueAuthorityFromEnv(); const expectedItemKey = `${targetRepo()}#${itemNumber}`.toLowerCase(); diff --git a/test/command.test.ts b/test/command.test.ts index 94889d332c..4e10db6fa8 100644 --- a/test/command.test.ts +++ b/test/command.test.ts @@ -276,6 +276,80 @@ process.stdout.write("200"); } }); +test("reserve-review-lease completes as superseded when the item closed after enqueue", () => { + const root = mkdtempSync(join(tmpdir(), "cmd-reserve-lease-closed-")); + const binDir = join(root, "bin"); + const ghPath = join(binDir, "gh.js"); + const leasePath = join(root, "lease.json"); + try { + mkdirSync(binDir, { recursive: true }); + writeFileSync( + ghPath, + ` +const { readFileSync, writeFileSync } = require("node:fs"); +const leasePath = ${JSON.stringify(leasePath)}; +const args = process.argv.slice(2); +const path = args[1] || ""; +if (args[0] === "api" && path === "repos/openclaw/openclaw/issues/357") { + console.log(JSON.stringify({ + number: 357, + title: "Closed before review", + html_url: "https://github.com/openclaw/openclaw/pull/357", + created_at: "2026-07-15T00:00:00Z", + updated_at: "2026-07-15T00:00:00Z", + closed_at: "2026-07-16T00:00:00Z", + state: "closed", + locked: false, + active_lock_reason: null, + author_association: "CONTRIBUTOR", + user: { login: "reporter" }, + labels: [], + pull_request: {} + })); +} else if (args[0] === "api" && path === "repos/openclaw/openclaw/issues/357/comments" && args.includes("--method")) { + writeFileSync(leasePath, readFileSync(args[args.indexOf("--input") + 1], "utf8")); + console.log(JSON.stringify({ id: 9993 })); +} else { + console.error("unexpected gh args", JSON.stringify(args)); + process.exit(1); +} +`, + "utf8", + ); + const result = spawnSync( + process.execPath, + [ + CLI, + "reserve-review-lease", + "--target-repo", + "openclaw/openclaw", + "--item-number", + "357", + "--review-timeout-ms", + "600000", + ], + { + encoding: "utf8", + env: { + ...process.env, + ...mockGhBinEnv(ghPath, binDir), + GITHUB_RUN_ID: "999", + GITHUB_RUN_ATTEMPT: "1", + }, + }, + ); + + assert.equal(result.status, 0, result.stderr); + const reservation = JSON.parse(result.stdout); + assert.equal(reservation.status, "superseded"); + assert.equal(reservation.reason, "item_not_open"); + assert.equal(reservation.state, "closed"); + assert.equal(existsSync(leasePath), false, "no lease comment may be posted for closed items"); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + test("reserve-review-lease completes as superseded when the PR head drifted past the queue authority", () => { const root = mkdtempSync(join(tmpdir(), "cmd-reserve-lease-drift-")); const binDir = join(root, "bin"); diff --git a/test/dashboard-worker.test.ts b/test/dashboard-worker.test.ts index d34876ef26..651e8c0977 100644 --- a/test/dashboard-worker.test.ts +++ b/test/dashboard-worker.test.ts @@ -54,7 +54,7 @@ test("production doubles exact review claims and canonical publication batches", assert.match(wrangler, /EXACT_REVIEW_ACTIONS_BUDGET = "194"/); assert.match(wrangler, /EXACT_REVIEW_PUBLICATION_BATCH_SIZE = "8"/); assert.match(wrangler, /EXACT_REVIEW_PUBLICATION_BATCH_MAX_CONCURRENT = "8"/); - assert.match(wrangler, /EXACT_REVIEW_TARGET_RATE_PER_HOUR = "600"/); + assert.match(wrangler, /EXACT_REVIEW_TARGET_RATE_PER_HOUR = "450"/); assert.match(wrangler, /EXACT_REVIEW_TARGET_BURST = "120"/); assert.match(wrangler, /EXACT_REVIEW_PENDING_SOFT_LIMIT = "600"/); }); diff --git a/test/sweep-workflow.test.ts b/test/sweep-workflow.test.ts index 2fff53c00a..3dce0e759e 100644 --- a/test/sweep-workflow.test.ts +++ b/test/sweep-workflow.test.ts @@ -532,6 +532,12 @@ test("exact event review publishes directly with a queue-bounded canonical fallb assert.match(reserveLease.run ?? "", /RANDOM % 4/); assert.match(reserveLease.run ?? "", /status.*superseded/); assert.match(reserveLease.run ?? "", /successful no-op/); + assert.match( + reserveLease.run ?? "", + /rate limit exceeded\|secondary rate limit\|HTTP 429/, + "throttled reservations must defer as held instead of failing", + ); + assert.match(reserveLease.run ?? "", /\\"status\\":\\"held\\",\\"retryAt\\":\\"\$retry_at\\"/); assert.match(source, /Review exact item \{0\} rev \{1\} head \{2\}/); assert.equal( reserveLease.env?.EXACT_REVIEW_ITEM_KEY,