Skip to content

fix(ci): release throttled live-item checks for delayed retry - #1002

Merged
steipete merged 1 commit into
mainfrom
fix/throttle-live-item-check
Aug 1, 2026
Merged

fix(ci): release throttled live-item checks for delayed retry#1002
steipete merged 1 commit into
mainfrom
fix/throttle-live-item-check

Conversation

@steipete

@steipete steipete commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Problem

An 11-run failure burst at 13:12Z (e.g. run 30701241184) failed at "Check live target item state" with installation-token rate-limit 403s. #968 covered throttles at the reservation step, but the live-item check runs earlier and still fails the run — burning each item's 8-attempt review-failure budget on end-of-window API exhaustion.

Fix

The live-item step now recognizes rate-limit stderr (rate limit exceeded|secondary rate limit|HTTP 429) on the item fetch and releases the claim via the existing admission_retry path (#962 machinery: requeue with retry_at +20 min, failure counters untouched). All other fetch errors still fail the step; the 404-missing-item completion is unchanged.

Proof

  • Workflow-shape assertions for the throttle branch; sweep-workflow suite 101/101 (after build:all — one pre-existing test appeared red locally purely from stale dist/repair).
  • Autoreview (codex, xhigh): clean, "patch is correct (0.99)".

@steipete
steipete requested a review from a team as a code owner August 1, 2026 13:19
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Aug 1, 2026
@clawsweeper

clawsweeper Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed August 1, 2026, 9:23 AM ET / 13:23 UTC.

ClawSweeper review

What this changes

This PR makes the exact-review workflow release a claimed item for a 20-minute delayed retry when GitHub throttles the live-item lookup, rather than failing the run and spending the item’s review-failure budget.

Merge readiness

Blocked until real behavior proof from a real setup is added - 5 items remain

Keep this PR open. The narrow workflow change fits the existing delayed-retry protocol and addresses a live automation failure that current main still treats as a hard error, but the PR body provides only workflow-shape tests and autoreview rather than after-fix real workflow evidence; it also edits the release-owned changelog contrary to repository policy.

Priority: P1
Reviewed head: 7c3fddc4491822fc685bce9e53093a71e3b7bc58

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The implementation is focused and consistent with the existing retry protocol, but merge-gating real behavior proof is absent.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: The PR body reports workflow-shape assertions and automated review, but neither demonstrates an after-fix workflow run through the throttled live-item branch; add a redacted controlled replay or live execution transcript before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR body reports workflow-shape assertions and automated review, but neither demonstrates an after-fix workflow run through the throttled live-item branch; add a redacted controlled replay or live execution transcript before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 6 items Current-main gap: Current main sends any non-404 failure from the live GitHub item lookup to stderr and exits nonzero, so installation-token throttling still consumes the workflow failure path rather than the admission-retry path.
Patch follows the existing retry contract: The proposed live-item branch writes admission_retry, retry_at, and non-proceeding terminal fields; the existing generation-result and completion stages turn admission retry into a successful requeue_latest completion and pass retry_at to the queue lease completion.
Merged related provenance: The earlier merged throttle hardening in #968 added the same rate-limit recognition and delayed-retry model for reservation failures; this PR covers the distinct earlier live-item fetch stage.
Findings 1 actionable finding [P3] Remove the release-owned changelog entry
Security None None.

How this fits together

The exact-review workflow claims a queued GitHub item, verifies that the item is still live, then performs and publishes a Codex review. The live-item gate determines whether the claim proceeds, completes as a no-op, or is returned to the durable queue for a later retry.

flowchart TD
  A[Durable exact-review queue] --> B[Claim item lease]
  B --> C[Live GitHub item check]
  C --> D{Item lookup result}
  D -->|Open item| E[Review and publish]
  D -->|Missing or closed| F[Complete as no-op]
  D -->|Rate limited| G[Release claim with delayed retry]
  G --> A
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR body reports workflow-shape assertions and automated review, but neither demonstrates an after-fix workflow run through the throttled live-item branch; add a redacted controlled replay or live execution transcript before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Remove the release-owned changelog entry (P3) - Keep release-note context in the PR body or commit message instead. Repository policy reserves CHANGELOG.md for release ownership, so this workflow-only fix should not add the entry at this line.
  • Resolve merge risk (P1) - Because this changes queue-admission behavior in the live workflow, an unproven output or completion-path mismatch could still fail runs or spend review-failure budget during GitHub throttling.
  • Resolve merge risk (P1) - The release-owned CHANGELOG.md edit creates avoidable release-note churn and conflicts with the repository’s normal PR policy.
  • Complete next step (P2) - No repair lane is appropriate: real workflow proof must come from the contributor’s or maintainer’s controlled setup, and repository policy keeps release-note ownership outside routine repair work.

Findings

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:21
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 23 added, 0 removed across 3 files The functional change is tightly scoped to one workflow gate, one workflow-shape assertion, and one release-note entry.
Retry delay 20 minutes This matches the existing reservation-throttle retry window and affects when claimed items return to the durable queue.

Merge-risk options

Maintainer options:

  1. Prove the delayed-retry path before merge (recommended)
    Remove the release-owned changelog line and provide a redacted controlled Actions replay or equivalent live workflow transcript showing a throttled item lookup completes the claim as a delayed retry.
  2. Pause for a narrower workflow test harness
    If a safe controlled replay cannot be produced, pause this PR until the workflow test harness can execute the live Bash branch and expose the completion payload rather than only matching YAML text.

Technical review

Best possible solution:

Keep the live-item throttle handling narrow, remove the release-owned changelog entry, and add redacted after-fix evidence from a controlled workflow replay that shows the claim completing with requeue_latest and a delayed retry timestamp without increasing failure budget.

Do we have a high-confidence way to reproduce the issue?

Yes, source inspection gives a high-confidence current-main path: a rate-limited gh api repos/$TARGET_REPO/issues/$ITEM_NUMBER call falls through the non-404 error branch and exits the live-item step unsuccessfully. The PR body also identifies affected production runs, but no after-fix execution evidence is currently attached.

Is this the best way to solve the issue?

Yes for the workflow behavior: reusing the established admission_retry plus delayed queue-retry protocol is narrower and safer than adding a new throttle mechanism. No for the submitted release-note change, which should remain release-owned rather than land with this normal workflow fix.

Full review comments:

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:21
    Keep release-note context in the PR body or commit message instead. Repository policy reserves CHANGELOG.md for release ownership, so this workflow-only fix should not add the entry at this line.
    Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 83fa433a0537.

Labels

Label changes:

  • add P1: The report describes repeated exact-review workflow failures that spend a finite retry budget during shared GitHub installation-token throttling.
  • add merge-risk: 🚨 automation: The PR changes exact-review workflow control flow and queue completion behavior during GitHub API throttling.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports workflow-shape assertions and automated review, but neither demonstrates an after-fix workflow run through the throttled live-item branch; add a redacted controlled replay or live execution transcript before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P1: The report describes repeated exact-review workflow failures that spend a finite retry budget during shared GitHub installation-token throttling.
  • merge-risk: 🚨 automation: The PR changes exact-review workflow control flow and queue completion behavior during GitHub API throttling.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports workflow-shape assertions and automated review, but neither demonstrates an after-fix workflow run through the throttled live-item branch; add a redacted controlled replay or live execution transcript before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Current-main gap: Current main sends any non-404 failure from the live GitHub item lookup to stderr and exits nonzero, so installation-token throttling still consumes the workflow failure path rather than the admission-retry path. (.github/workflows/sweep.yml:750, 83fa433a0537)
  • Patch follows the existing retry contract: The proposed live-item branch writes admission_retry, retry_at, and non-proceeding terminal fields; the existing generation-result and completion stages turn admission retry into a successful requeue_latest completion and pass retry_at to the queue lease completion. (.github/workflows/sweep.yml:1636, 7c3fddc44918)
  • Merged related provenance: The earlier merged throttle hardening in fix(queue): absorb item-lifecycle and throttle stalls without spending review-failure budget #968 added the same rate-limit recognition and delayed-retry model for reservation failures; this PR covers the distinct earlier live-item fetch stage. (.github/workflows/sweep.yml:750, b4d1edea05cc)
  • Proof remains shape-only: The added test asserts that the workflow text contains the rate-limit expression and notice, but the submitted proof does not show an after-fix workflow execution that reaches the rate-limit branch and completes the lease with the delayed retry. (test/sweep-workflow.test.ts:649, 7c3fddc44918)
  • Release-note policy: Repository policy states that CHANGELOG.md is release-owned and normal contributor, repair, and automerge work should preserve release context in the PR body and commit messages instead. (README.md:264, 83fa433a0537)
  • Feature history: Recent workflow history shows Peter Steinberger authored the related retry and exact-review reliability changes, including the merged reservation-throttle fix and the current mainline workflow reliability work. (.github/workflows/sweep.yml:750, b4d1edea05cc)

Likely related people:

  • steipete: Peter Steinberger authored the merged reservation-throttle retry implementation and several recent exact-review workflow reliability changes on this path. (role: recent area contributor; confidence: high; commits: b4d1edea05cc, 197d807562e5, 56d32bee74e4; files: .github/workflows/sweep.yml, test/sweep-workflow.test.ts)
  • Martin Cleary: Martin Cleary authored the current-main refactor that recently touched the surrounding review orchestration, making him a useful secondary routing contact for integration context. (role: recent adjacent contributor; confidence: medium; commits: 922ebc673e12; files: .github/workflows/sweep.yml, test/sweep-workflow.test.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Remove the release-owned CHANGELOG.md entry.
  • Add redacted after-fix workflow evidence showing a throttled live-item fetch returns the lease for delayed retry without spending failure budget.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit 86cc49d into main Aug 1, 2026
17 checks passed
@steipete
steipete deleted the fix/throttle-live-item-check branch August 1, 2026 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant