Skip to content

fix: retry a polling 429 only when Retry-After says when to come back (sable-96ex) #227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Rome-1 wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix: retry a polling 429 only when Retry-After says when to come back (sable-96ex) #227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6da25da
fix: retry a polling 429 only when Retry-After says when to come back…
Rome-1 Sep 2, 2026
5defad6
ci: a honored Retry-After must not outlive the action's deadline
Rome-1 Sep 2, 2026
82df35b
fix: a repeated Retry-After is not a delay, in all three surfaces
Rome-1 Sep 2, 2026
5ef4ca5
fix: three findings from the security review of the 429 path
Rome-1 Sep 2, 2026
622dac1
docs: state the worst-case Retry-After waiting bound in CLI_SPEC
Rome-1 Sep 2, 2026
535ef46
docs: CHANGELOG — name the four ways a Retry-After counts as absent
Rome-1 Sep 2, 2026
752b1f0
Merge remote-tracking branch 'origin/main' into fix/sable-96ex-retry-…
Rome-1 Sep 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
289 changes: 289 additions & 0 deletions .github/workflows/test-github-action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,295 @@ jobs:
[ "$FAIL" -eq 0 ] && echo "PASS: counts reached the gate and the gate failed the build."
exit $FAIL

# sable-96ex — the day a rate limiter lands in front of the poll endpoint,
# polling every 10s from every customer repo is exactly the traffic it
# targets. A 429 is retried on one condition only: the server sent a
# Retry-After saying when to come back. Without it a 429 on this API means
# quota exhausted, and sleeping through five retries before failing anyway is
# worse than failing now. These three jobs pin both halves.
test-poll-429-with-retry-after:
name: "Poll: rides out a 429 that carries Retry-After"
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Start mock backend (429 + Retry-After on poll #2, then healthy)
env:
PORT: '8801'
FAIL_ON: '2'
FAIL_STATUS: '429'
FAIL_RETRY_AFTER: '1'
run: |
nohup python3 github-action/tests/mock-rafter-api.py > mock.log 2>&1 &
for _ in $(seq 1 30); do
curl -sf -X POST -d '{}' http://127.0.0.1:8801/api/static/scan >/dev/null && break
sleep 1
done
curl -sf -X POST -d '{}' http://127.0.0.1:8801/api/static/scan >/dev/null || {
echo "FAIL: mock backend never started"; cat mock.log; exit 1; }

- name: Run the action against the mock
id: scan
continue-on-error: true
uses: ./github-action
with:
api-key: 'not-a-real-key'
rafter-url: 'http://127.0.0.1:8801'
timeout-minutes: '2'
upload-sarif: 'false'
comment-on-pr: 'false'

- name: Assert the scan survived the throttle
run: |
cat mock.log
if [ "${{ steps.scan.outputs.status }}" != "completed" ]; then
echo "FAIL: a 429 that told us when to come back killed the run (status='${{ steps.scan.outputs.status }}')."
exit 1
fi
echo "PASS: the action honored Retry-After and completed."

test-poll-429-without-retry-after:
name: "Poll: a 429 with no Retry-After fails fast"
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Start mock backend (bare 429 on poll #2)
env:
PORT: '8802'
FAIL_ON: '2'
FAIL_STATUS: '429'
run: |
nohup python3 github-action/tests/mock-rafter-api.py > mock.log 2>&1 &
for _ in $(seq 1 30); do
curl -sf -X POST -d '{}' http://127.0.0.1:8802/api/static/scan >/dev/null && break
sleep 1
done
curl -sf -X POST -d '{}' http://127.0.0.1:8802/api/static/scan >/dev/null || {
echo "FAIL: mock backend never started"; cat mock.log; exit 1; }

- name: Run the action against the mock
id: scan
continue-on-error: true
uses: ./github-action
with:
api-key: 'not-a-real-key'
rafter-url: 'http://127.0.0.1:8802'
timeout-minutes: '2'
upload-sarif: 'false'
comment-on-pr: 'false'

- name: Assert it failed immediately rather than retrying
run: |
cat mock.log
if [ "${{ steps.scan.outcome }}" != "failure" ]; then
echo "FAIL: a bare 429 (quota exhausted) did not fail the run."
exit 1
fi
# The startup probe POSTs; only GETs are polls. Two of them means the
# opening poll and the 429 — no retry. This is the half that catches a
# 'simplification' making every 429 transient: that mutant retries to
# a completed scan, six GETs and status=completed.
gets=$(grep -c '"GET ' mock.log || true)
if [ "$gets" -ne 2 ]; then
echo "FAIL: expected exactly 2 polls (no retry), saw ${gets}."
exit 1
fi
echo "PASS: a 429 with no Retry-After failed fast, without retrying."

test-poll-429-duplicate-retry-after:
name: "Poll: a repeated Retry-After is not a delay"
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Start mock backend (429 carrying TWO Retry-After headers)
env:
PORT: '8804'
FAIL_ON: '2'
FAIL_STATUS: '429'
FAIL_RETRY_AFTER: '5|900'
run: |
nohup python3 github-action/tests/mock-rafter-api.py > mock.log 2>&1 &
for _ in $(seq 1 30); do
curl -sf -X POST -d '{}' http://127.0.0.1:8804/api/static/scan >/dev/null && break
sleep 1
done
curl -sf -X POST -d '{}' http://127.0.0.1:8804/api/static/scan >/dev/null || {
echo "FAIL: mock backend never started"; cat mock.log; exit 1; }

- name: Run the action against the mock
id: scan
continue-on-error: true
uses: ./github-action
with:
api-key: 'not-a-real-key'
rafter-url: 'http://127.0.0.1:8804'
timeout-minutes: '2'
upload-sarif: 'false'
comment-on-pr: 'false'

- name: Assert it treated the ambiguous header as no header
run: |
cat mock.log
# An origin's Retry-After and a proxy's are not a delay anyone can act
# on. Both runtimes answer null here; the shell must not quietly pick
# one and sleep on it.
if [ "${{ steps.scan.outcome }}" != "failure" ]; then
echo "FAIL: a repeated Retry-After was acted on instead of refused."
exit 1
fi
gets=$(grep -c '"GET ' mock.log || true)
if [ "$gets" -ne 2 ]; then
echo "FAIL: expected exactly 2 polls (no retry), saw ${gets}."
exit 1
fi
echo "PASS: a repeated Retry-After counted as absent, and the run failed fast."

test-poll-429-early-hints-retry-after:
name: "Poll: a hinted Retry-After is not the 429's own"
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Start mock backend (103 Early Hints carries Retry-After, the 429 does not)
env:
PORT: '8805'
FAIL_ON: '2'
FAIL_STATUS: '429'
EARLY_HINTS_RETRY_AFTER: '7'
run: |
nohup python3 github-action/tests/mock-rafter-api.py > mock.log 2>&1 &
for _ in $(seq 1 30); do
curl -sf -X POST -d '{}' http://127.0.0.1:8805/api/static/scan >/dev/null && break
sleep 1
done
curl -sf -X POST -d '{}' http://127.0.0.1:8805/api/static/scan >/dev/null || {
echo "FAIL: mock backend never started"; cat mock.log; exit 1; }

- name: Run the action against the mock
id: scan
continue-on-error: true
uses: ./github-action
with:
api-key: 'not-a-real-key'
rafter-url: 'http://127.0.0.1:8805'
timeout-minutes: '2'
upload-sarif: 'false'
comment-on-pr: 'false'

- name: Assert the hinted header was not read as the 429's
run: |
cat mock.log
# `curl -D` dumps EVERY header block it received. Reading Retry-After
# from the file without scoping it to the last block turns a bare 429
# into a retried one — the single thing this branch exists to prevent.
# Verified as non-vacuous: with a `grep | tail -n1` extraction this
# mock config sleeps 7s and completes with 6 polls.
if [ "${{ steps.scan.outcome }}" != "failure" ]; then
echo "FAIL: an Early Hints Retry-After was honored for a bare 429."
exit 1
fi
gets=$(grep -c '"GET ' mock.log || true)
if [ "$gets" -ne 2 ]; then
echo "FAIL: expected exactly 2 polls (no retry), saw ${gets}."
exit 1
fi
echo "PASS: only the final response's Retry-After counted."

test-poll-429-absurd-retry-after:
name: "Poll: an absurd Retry-After cannot hang the job"
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- uses: actions/checkout@v4

- name: Start mock backend (429 forever, Retry-After of 23 digits)
env:
PORT: '8806'
FAIL_ON: '2'
FAIL_STATUS: '429'
FAIL_FOREVER: '1'
FAIL_RETRY_AFTER: '99999999999999999999999'
run: |
nohup python3 github-action/tests/mock-rafter-api.py > mock.log 2>&1 &
for _ in $(seq 1 30); do
curl -sf -X POST -d '{}' http://127.0.0.1:8806/api/static/scan >/dev/null && break
sleep 1
done
curl -sf -X POST -d '{}' http://127.0.0.1:8806/api/static/scan >/dev/null || {
echo "FAIL: mock backend never started"; cat mock.log; exit 1; }

- name: Run the action against the mock
id: scan
continue-on-error: true
timeout-minutes: 3
uses: ./github-action
with:
api-key: 'not-a-real-key'
rafter-url: 'http://127.0.0.1:8806'
timeout-minutes: '1'
upload-sarif: 'false'
comment-on-pr: 'false'

- name: Assert it ended at the deadline instead of sleeping forever
run: |
cat mock.log
# The value passes the digit guard, so only the LENGTH cap stands
# between it and `sleep 99999999999999999999999`, which does not
# return: `[ 1e23 -gt 60 ]` is a shell error that evaluates false, so
# an uncapped value reaches sleep intact. Asserting the status output
# is what makes this an artifact-level test rather than a grep over
# the file — a hung step writes no status at all.
if [ "${{ steps.scan.outputs.status }}" != "timeout" ]; then
echo "FAIL: expected status=timeout at the deadline, got '${{ steps.scan.outputs.status }}'."
exit 1
fi
echo "PASS: the honored delay stayed capped and the deadline held."

test-results-fetch-429-with-retry-after:
name: "Results fetch: rides out a 429 that carries Retry-After"
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Start mock backend (poll succeeds, first results fetch 429s)
env:
PORT: '8803'
FAIL_ON: '2'
FAIL_COUNT: '1'
FAIL_STATUS: '429'
FAIL_RETRY_AFTER: '1'
COMPLETE_AFTER: '1'
run: |
nohup python3 github-action/tests/mock-rafter-api.py > mock.log 2>&1 &
for _ in $(seq 1 30); do
curl -sf -X POST -d '{}' http://127.0.0.1:8803/api/static/scan >/dev/null && break
sleep 1
done
curl -sf -X POST -d '{}' http://127.0.0.1:8803/api/static/scan >/dev/null || {
echo "FAIL: mock backend never started"; cat mock.log; exit 1; }

- name: Run the action against the mock
id: scan
continue-on-error: true
uses: ./github-action
with:
api-key: 'not-a-real-key'
rafter-url: 'http://127.0.0.1:8803'
timeout-minutes: '2'
upload-sarif: 'false'
comment-on-pr: 'false'

- name: Assert the results fetch honored Retry-After
run: |
cat mock.log
if [ "${{ steps.scan.outputs.status }}" != "completed" ]; then
echo "FAIL: a throttled results fetch killed the run (status='${{ steps.scan.outputs.status }}')."
exit 1
fi
echo "PASS: the results fetch honored Retry-After and completed."

test-yaml-validity:
name: action.yml is valid YAML
runs-on: ubuntu-latest
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- **A 429 during scan polling is retried when it carries `Retry-After`** (sable-96ex). Polling every 10 seconds from every customer repo is exactly the traffic shape a rate limiter targets, and all three surfaces treated a 429 as fatal — so the first limiter in front of `GET /api/static/scan` would have failed every customer build instantly, on a condition one sleep would have resolved. A 429 cannot simply join 5xx either: on scan *submit* it means the account is out of credits. `Retry-After` is the disambiguator — a limiter sends one, a quota rejection does not — so the poll and results paths now retry a 429 that carries one, sleeping `min(Retry-After, 60s)` against the same failure budget as any other transient failure, and still fail fast on a bare 429. Submit is unchanged: exit `3`, even with the header. Giving up on repeated 429s now says the API rate limited us rather than blaming the report. Delay-seconds only, ASCII digits only, and only when the final response names the header exactly once — the HTTP-date form, a value `int()` cannot parse, a header repeated by a proxy, and one carried on a `103 Early Hints` block all count as absent, which means fail fast. Full contract in `shared-docs/CLI_SPEC.md`.
- **`timeout-minutes` on the GitHub Action is now a wall-clock deadline**, not a poll count. Previously the action ran `timeout-minutes * 6` polls, each costing 10s *plus* API latency, so a slow API pushed real elapsed time past the documented budget. It is now enforced as a real deadline. **This can fail workflows that were relying on the overrun** — if a scan sits near the boundary, raise `timeout-minutes`.
- `rafter get <scan_id>` (without `--interactive`) now retries transient failures too. It is the command the poll loop's give-up message recommends, so a remedy defeated by the same transient failure it is recommended for was not a remedy.
- HTTP requests on the poll and results paths now carry connect/read timeouts (`--connect-timeout 10 --max-time 60` for curl, 30s for axios), so a hung server cannot stall inside a request that the retry loop only checks between attempts.
Expand Down
Loading
Loading