ci(review): cap review spend with --max-budget-usd - #673
Conversation
This repo's review pipeline invoked the CLI with no spend ceiling, so one
runaway exploration could bill without bound. It is also the org's one PUBLIC
repo, which is where that matters most. Default is $10.00 per review.
The org-membership gate already stops a fork pull request from triggering a
review at all. What it does not do is bound what a member's large pull request
costs, and sibling repos on the shared pipeline have billed $6.11 on a two-file
diff — the ceiling is the missing half of that control, not a replacement.
Treat it as a runaway guard, not a budget target. The CLI checks it BETWEEN
turns, so a run can overshoot by roughly one turn, and on a single hung turn it
never fires at all — the job timeout is the only bound there. Sized above
observed spend deliberately: the expensive reviews are the ones that find real
defects, so capping near the average would truncate exactly the runs worth
paying for.
The failure branch had to change too, or the ceiling would have been the least
diagnosable outcome in the script. It discarded the CLI's stdout and posted
"check workflow logs" above a log that held nothing, and budget exhaustion exits
1 exactly like a crash — so a ceiling hit would have looked like a broken
pipeline. It now logs the first 2000 chars of stdout (`VAR=$(cmd)` keeps it even
when cmd fails, and claude reports auth, quota and budget failures there rather
than on stderr) and branches on the machine-readable marker to post a comment
naming the ceiling and the spend.
Verified rather than assumed:
- --max-budget-usd is present in `claude --help` on 2.1.159, the version this
workflow pins in both install steps.
- The validator, driven through the real script under bash 5.2.21 (what
ubuntu-latest runs): 10.00, 10, .5 and 2.50 pass; 0, 00, 0.0, .0, 0.00,
.00, `.`, abc, 10,00, $10, 1., -1 and 1e3 are rejected. An empty value
takes the default, since bash `:-` treats unset and empty alike.
- All three runtime paths, with a stubbed CLI and gh: exhaustion posts
"reached the $7.50 spend ceiling after $7.61 without finishing"; an ordinary
crash posts the exit code and logs the payload; success posts the review
unchanged, so the happy path is untouched.
Shape and value are checked separately. Shape allows `.50` the way the CLI does;
value is checked arithmetically because a zero ceiling is accepted by the CLI
and fails every review on its first turn, and spelling "zero" as a regex means
enumerating 0, 00, 0.0, .0, 0.00 and .00 — where the no-leading-digit forms are
the ones easily missed.
`jq -e` rather than `grep -q` for the marker: grep stops reading on a match, the
upstream printf takes SIGPIPE, and pipefail then turns a MATCH into a non-zero
pipeline. jq drains stdin.
No RESULT="" declaration, unlike caura-ai/.github: that guard exists there
because a CLI smoke test calls the failure helper before RESULT is assigned.
Every $RESULT reference here follows the assignment, so there is nothing for
nounset to trip on. Checked rather than copied across.
No workflow change needed — both invocation sites pass env explicitly, so the
script-side default applies without touching them.
Signed-off-by: eldad-caura <eldad@caura.ai>
|
Claude Code Review — skipped: PR author 'Eldad-Caura' is not a public member of the 'caura-ai' org |
The ceiling is in, but this PR surfaced something bigger
The membership gate is working exactly as written. The precondition isn't met: Both maintainers' org membership is private. Rule 1 calls The consequence is that reviews on this repo are being skipped for every human PR, not just this The workflow's own comment predicted it: "Members must still make their org membership public." Fix, in order of preference
Only (1) is yours to do, and it also matters beyond this repo: the shared pipeline's On this PRThe ceiling itself is unaffected and the change stands on its own — |
|
The subagent found one theoretical concern — if Claude Code Review ✅ No issues found. Reviewed by |
|
@erni-a please approve |
2 participants
This repo's review pipeline ran the CLI with no spend ceiling, so one runaway exploration could
bill without bound — and this is the org's one public repo, which is where that matters most.
Default is
$10.00per review.Why the membership gate wasn't already enough
It stops a fork pull request from triggering a review at all, which is the right control for who
can spend. It does nothing about how much a legitimate member's large pull request costs. Sibling
repos on the org's shared pipeline have billed $6.11 on a two-file diff. The ceiling is the
missing half of that control, not a replacement for it.
It's a runaway guard, not a budget target: the CLI checks it between turns, so a run can overshoot
by about one turn, and on a single hung turn it never fires at all — the job timeout is the only bound
there. Sized above observed spend on purpose, because the expensive reviews are the ones that find
real defects; capping near the average would truncate exactly the runs worth paying for.
The failure branch had to change with it
Otherwise the ceiling would have been the least diagnosable outcome in the script. It discarded the
CLI's stdout and posted "check workflow logs" above a log that held nothing — and budget exhaustion
exits
1exactly like a crash, so hitting the ceiling would have looked like a broken pipeline.It now logs the first 2000 chars of stdout (
VAR=$(cmd)keeps it even when cmd fails, and claudereports auth, quota and budget failures there rather than on stderr) and branches on the
machine-readable marker to name the ceiling and the spend.
Verified, not assumed
Flag support —
--max-budget-usdis inclaude --helpon2.1.159, the version this workflowpins in both install steps.
Validator, driven through the real script under bash 5.2.21 (what
ubuntu-latestruns):An empty value takes the
10.00default — bash:-treats unset and empty alike, so that's intended,not a hole.
All three runtime paths, with a stubbed CLI and
gh:(The $7.61-against-$7.50 in that first row is the documented between-turns overshoot, not a bug.)
Two deliberate non-changes
jq -erather thangrep -qfor the marker: grep stops reading on a match, the upstreamprintftakes SIGPIPE, and
pipefailthen turns a match into a non-zero pipeline.No
RESULT=""declaration, unlikecaura-ai/.github. That guard exists there because a CLI smoketest calls the failure helper before
RESULTis assigned, and under bash 5.2${RESULT:0:2000}on anunset variable aborts. Every
$RESULTreference in this script follows the assignment, so there isnothing for
nounsetto trip on — I checked rather than copying the fix across.No workflow change: both invocation sites pass env explicitly, so the script-side default applies.
What this does not do
It doesn't bring over the rest of the shared pipeline's hardening —
--bare, the--toolsallow-list, the zero-exit
is_errorguard, or the docs-only skip. Those are a separate change, andthis repo can't consume the shared pipeline at all while it is public and
caura-ai/.githubisprivate (see #668). This is the one gap worth closing independently of that.