fix(runner): cap HuggingFace Retry-After backoff at 30s

[SebTardif]

What Problem This Solves

clawscan benchmark clawhub-security-signals fetches HuggingFace dataset rows and retries on 429/5xx. huggingFaceRowsBackoff honors Retry-After as integer seconds or an HTTP-date with no ceiling, while the quadratic fallback is already capped at 30s.

The public CLI builds HuggingFaceBenchmarkClient with a nil Context, so requestContext() is context.Background() and the retry select never expires. A 429 with Retry-After: 3600 blocks the process for one hour per attempt (up to five waits). This change caps Retry-After at the same 30s ceiling. It does not add a process-wide signal context.

The uncapped header path was introduced in #3 (2026-06-25). #46 made the wait cancelable when a context is set; it did not cap the delay.

Evidence

Red (unfixed huggingFaceRowsBackoff):

$ go test -count=1 -timeout 30s -v -run TestHuggingFaceRowsBackoffCapsRetryAfter ./internal/runner/
=== RUN   TestHuggingFaceRowsBackoffCapsRetryAfter
    submission_test.go:276: Retry-After 3600 = 1h0m0s, want 30s
--- FAIL: TestHuggingFaceRowsBackoffCapsRetryAfter (0.00s)
FAIL
FAIL    github.com/openclaw/clawscan/internal/runner    0.248s
FAIL

Green (after the 30s cap):

$ go test -count=1 -timeout 30s -v -run TestHuggingFaceRowsBackoffCapsRetryAfter ./internal/runner/
=== RUN   TestHuggingFaceRowsBackoffCapsRetryAfter
--- PASS: TestHuggingFaceRowsBackoffCapsRetryAfter (0.00s)
PASS
ok      github.com/openclaw/clawscan/internal/runner    0.263s

Retry-After: 3600 and an HTTP-date two hours ahead both return 30s. The quadratic fallback path is unchanged.

Real behavior proof

• Behavior or issue addressed: HuggingFace row-fetch Retry-After delays (integer seconds and HTTP-date) are capped at 30s, matching the existing fallback ceiling, so a 429 cannot stall clawscan benchmark for an hour per attempt.

• Real environment tested: macOS 26.6.2 (Darwin 25.6.0 arm64), go1.27.0 darwin/arm64, checkout /tmp/oc-pr-clawscan-F003 on fix/f003-huggingface-retry-after-cap.

• Exact steps or command run after this patch:

  go test -count=1 -timeout 30s -v -run TestHuggingFaceRowsBackoffCapsRetryAfter ./internal/runner/

• Evidence after fix: terminal output from the patched tree:

  $ go test -count=1 -timeout 30s -v -run TestHuggingFaceRowsBackoffCapsRetryAfter ./internal/runner/
  === RUN   TestHuggingFaceRowsBackoffCapsRetryAfter
  --- PASS: TestHuggingFaceRowsBackoffCapsRetryAfter (0.00s)
  PASS
  ok      github.com/openclaw/clawscan/internal/runner    0.263s

• Observed result after fix: Retry-After: 3600 is 30s (was 1h). An HTTP-date two hours ahead is also 30s. The helper no longer returns a multi-hour wait.

• What was not tested: A live HuggingFace 429 with a long Retry-After header. Network fetch of datasets-server.huggingface.co during this change.

Command: go test -count=1 -timeout 30s -v -run TestHuggingFaceRowsBackoffCapsRetryAfter ./internal/runner/

Observed: unfixed delay was 1h0m0s for Retry-After: 3600; patched delay is 30s for both 3600 and a future HTTP-date.

Expected: both header forms cap at 30s.

Time: 11:07:34 PDT (2026-08-29 18:07:34 UTC)

Date: 2026-08-29

Environment: macOS 26.6.2, Darwin 25.6.0 arm64, go1.27.0 darwin/arm64

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 1, 2026, 6:58 AM ET / 10:58 UTC.

ClawSweeper review

What this changes

The PR limits numeric and HTTP-date Hugging Face Retry-After values to 30 seconds and adds helper-level tests for both forms.

Merge readiness

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

Keep this PR open: current main does not already contain the cap, but the patch makes the client retry before a valid server-directed delay expires and still permits numeric-duration overflow.

Priority: P1
Reviewed head: b7be605e23df7db4203400710d007f0a71c2cd2e
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The patch is small and focused, but its retry-policy change, overflow defect, and helper-only proof prevent merge readiness.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: The changed production owner, fetchHuggingFaceRowsPage, waits on the computed delay before another HTTP request, but the supplied terminal trace calls only huggingFaceRowsBackoff directly. It does not show a real transport client receiving a long header and the resulting retry behavior. 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	🦪 silver shellfish (2/6)	2 actionable review findings remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The changed production owner, fetchHuggingFaceRowsPage, waits on the computed delay before another HTTP request, but the supplied terminal trace calls only huggingFaceRowsBackoff directly. It does not show a real transport client receiving a long header and the resulting retry behavior. 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	5 items	Introduced retry behavior: The introduced min calls cap both valid Retry-After forms; the caller waits for this result and then sends the next request. Public CLI remains uncancelable during this path: The benchmark CLI deliberately constructs a RunContext without Context, so the default Hugging Face client falls back to context.Background for its retry wait. Current main has not implemented this proposal: Current main removes neither uncapped header branch; the PR-to-main comparison shows the cap and its test are absent from main.
Findings	2 actionable findings	[P1] Honor valid server Retry-After values [P2] Clamp seconds before duration conversion
Security	None	None.

How this fits together

ClawScan’s benchmark runner fetches Hugging Face dataset rows before running benchmark cases. Transient HTTP responses feed a retry-delay calculation that controls when the next row request is sent.

flowchart LR
A[Benchmark command] --> B[Hugging Face row request]
B --> C[Transient HTTP response]
C --> D[Retry delay calculation]
D --> E[Wait before retry]
E --> B
B --> F[Benchmark rows]

Loading

Decision needed

Question	Recommendation
Should benchmark row fetching honor the full valid server Retry-After delay, or intentionally cap it at a documented operator-facing maximum?	Honor server-directed delays: Retain the existing valid-header behavior and address interruption through a separately defined cancellation or timeout policy.

Why: The proposed cap changes established rate-limit behavior rather than repairing a purely local calculation, and the source does not establish that retrying early is acceptable to Hugging Face.

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The changed production owner, fetchHuggingFaceRowsPage, waits on the computed delay before another HTTP request, but the supplied terminal trace calls only huggingFaceRowsBackoff directly. It does not show a real transport client receiving a long header and the resulting retry behavior. 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.
• Honor valid server Retry-After values (P1) - Both introduced caps make the fetch loop issue another request after 30 seconds even when the rate-limited server requests a longer valid delay. This reverses the established backoff behavior from ci: back off Hugging Face rate limits #3 and can sustain the 429 condition. This blocker was also visible in the prior review cycle.
• Clamp seconds before duration conversion (P2) - time.Duration(seconds) * time.Second can overflow for a valid large parsed integer before min runs, yielding a negative delay and an immediate retry. Compare seconds with the 30-second bound before conversion. This blocker was also visible in the prior review cycle.
• Resolve merge risk (P1) - A 30-second cap causes another request while the upstream server has explicitly requested a longer cooldown, risking repeated rate-limit failures.
• Resolve merge risk (P1) - Large valid numeric Retry-After values overflow during duration conversion before min can cap them, producing an immediate retry.
• Resolve merge risk (P1) - The supplied terminal proof covers only a direct helper call, not the production HTTP retry loop.
• Complete next step (P2) - A maintainer must choose the server-delay policy before a mechanical repair can proceed.
• Improve patch quality - Resolve the server-directed delay policy with the relevant area owners.
• Improve patch quality - Clamp numeric seconds before converting them to time.Duration.
• Improve patch quality - Post a redacted production-loop trace using a real local HTTP client and long Retry-After response; updating the PR body triggers re-review, or a maintainer can comment @clawsweeper re-review.

Findings

• [P1] Honor valid server Retry-After values — internal/runner/benchmark.go:932-936
• [P2] Clamp seconds before duration conversion — internal/runner/benchmark.go:932

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch surface	2 production lines changed, 13 test lines added	The small patch changes the timing contract for every retried Hugging Face row request.

Merge-risk options

Maintainer options:

1. Preserve server-directed backoff (recommended)
   Remove the unconditional cap and resolve the long-wait concern through an explicitly approved interruption policy.
2. Approve a bounded-wait contract
   If early retry is intentional, document that policy and add overflow-safe parsing plus production-path evidence before merging.

Technical review

Best possible solution:

Define the benchmark retry policy explicitly, preserve valid server-directed delays unless maintainers intentionally accept a capped policy, and guard numeric conversion before constructing a duration.

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

Yes, source-reproducible: a retriable response flows through the changed helper into time.After before the next real HTTP request. The supplied run confirms only the helper result, not that production request sequence.

Is this the best way to solve the issue?

No; the cap changes the server-directed retry contract and does not prevent overflow before duration conversion. A maintainer-approved delay policy is needed before selecting the narrow repair.

Full review comments:

• [P1] Honor valid server Retry-After values — internal/runner/benchmark.go:932-936
  Both introduced caps make the fetch loop issue another request after 30 seconds even when the rate-limited server requests a longer valid delay. This reverses the established backoff behavior from ci: back off Hugging Face rate limits #3 and can sustain the 429 condition. This blocker was also visible in the prior review cycle.
  Confidence: 0.96
  Late finding: first raised on code an earlier review cycle already covered.
• [P2] Clamp seconds before duration conversion — internal/runner/benchmark.go:932
time.Duration(seconds) * time.Second can overflow for a valid large parsed integer before min runs, yielding a negative delay and an immediate retry. Compare seconds with the 30-second bound before conversion. This blocker was also visible in the prior review cycle.
  Confidence: 0.98
  Late finding: first raised on code an earlier review cycle already covered.

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6432c40f2340.

Labels

Label justifications:

• P1: The patch can make benchmark clients repeatedly retry a rate-limited upstream service before its requested cooldown ends.
• merge-risk: 🚨 compatibility: It changes the established handling of valid Retry-After responses for existing benchmark workflows.
• merge-risk: 🚨 availability: Early retries can prolong or worsen rate limiting instead of allowing the benchmark fetch to recover.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The changed production owner, fetchHuggingFaceRowsPage, waits on the computed delay before another HTTP request, but the supplied terminal trace calls only huggingFaceRowsBackoff directly. It does not show a real transport client receiving a long header and the resulting retry behavior. 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:

• Introduced retry behavior: The introduced min calls cap both valid Retry-After forms; the caller waits for this result and then sends the next request. (internal/runner/benchmark.go:900, b7be605e23df)
• Public CLI remains uncancelable during this path: The benchmark CLI deliberately constructs a RunContext without Context, so the default Hugging Face client falls back to context.Background for its retry wait. (cmd/clawscan/main.go:286, 6432c40f2340)
• Current main has not implemented this proposal: Current main removes neither uncapped header branch; the PR-to-main comparison shows the cap and its test are absent from main. (internal/runner/benchmark.go:929, 6432c40f2340)
• Existing retry policy provenance: History identifies the merged Hugging Face rate-limit backoff change as commit 7a97f1c, authored by Vincent Koc; the current uncapped behavior is therefore not an accidental base-branch omission. (internal/runner/benchmark.go:930, 7a97f1cf114e)
• Supplied proof is helper-only: The added test invokes huggingFaceRowsBackoff directly; it does not exercise the production fetch loop and its HTTP client. (internal/runner/submission_test.go:273, b7be605e23df)

Likely related people:

• vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• SebTardif: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

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.

History

Review history (13 earlier review cycles; latest 8 shown)

• reviewed 2026-08-30T13:03:03.320Z sha b7be605 :: needs real behavior proof before merge. :: [P2] Clamp seconds before duration conversion
• reviewed 2026-08-30T17:00:40.359Z sha b7be605 :: needs real behavior proof before merge. :: [P1] Do not retry before a server-directed Retry-After expires | [P2] Clamp parsed seconds before converting to a duration
• reviewed 2026-08-30T21:59:50.397Z sha b7be605 :: needs real behavior proof before merge. :: [P1] Honor server-directed Retry-After before retrying | [P2] Clamp seconds before converting to time.Duration
• reviewed 2026-08-31T02:04:54.189Z sha b7be605 :: needs real behavior proof before merge. :: [P1] Do not retry before a server-directed Retry-After expires | [P2] Clamp Retry-After seconds before duration conversion
• reviewed 2026-08-31T07:13:47.659Z sha b7be605 :: needs real behavior proof before merge. :: [P1] Honor server-provided Retry-After values | [P2] Clamp numeric seconds before converting duration
• reviewed 2026-08-31T11:07:23.795Z sha b7be605 :: needs real behavior proof before merge. :: [P1] Honor valid server Retry-After delays | [P2] Clamp numeric Retry-After before duration conversion
• reviewed 2026-09-01T04:22:34.187Z sha b7be605 :: needs real behavior proof before merge. :: [P1] Honor valid server Retry-After delays | [P2] Clamp numeric Retry-After before duration conversion
• reviewed 2026-09-01T07:14:41.612Z sha b7be605 :: needs real behavior proof before merge. :: [P1] Honor valid server-directed Retry-After delays | [P2] Clamp numeric Retry-After before duration conversion