ci(bench): validate /benchmark suite against benchmarks main, not a stale pin - #4947
Conversation
…tale pin The /benchmark pre-dispatch suite check looked up `suites/<name>.toml` at a hardcoded BENCH_PIN SHA, but the dispatched `bench` job runs `bench-pr-reusable.yml@main` (and is documented to intentionally track benchmarks main). The pin had drifted, so suites that exist on benchmarks main were rejected as "Unknown suite" even though the run would have found them — e.g. `pinchbench26`, `officeqa`, `terminal-bench-2`. Point BENCH_PIN at `main` so the validation ref matches the ref we actually run against; they now stay in lock-step and new suites work the moment they land on benchmarks main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesBENCH_PIN Workflow Ref Update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/nearai-bench.yml:
- Around line 96-103: The BENCH_PIN environment variable set to main creates a
mutable reference that causes TOCTOU drift between pre-dispatch validation and
actual workflow execution in this privileged issue_comment dispatcher. Replace
BENCH_PIN from main with a full immutable commit SHA. Additionally, update the
reusable-workflow reference (the uses directive that currently specifies `@main`)
to use the same immutable commit SHA so that both the pre-dispatch authorization
check and the dispatched workflow execution are pinned to the identical
revision, eliminating the drift vulnerability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bcdb36b7-fea2-40cf-8b46-43e193dd4cd8
📒 Files selected for processing (1)
.github/workflows/nearai-bench.yml
| # Ref on nearai/benchmarks used to validate the requested suite | ||
| # exists. Must match the ref the `bench` job actually runs against | ||
| # (its `uses:` below) so the pre-dispatch check gives the same | ||
| # answer as the dispatched workflow. That `uses:` is `@main`, so a | ||
| # pinned SHA here drifts stale and rejects suites that already exist | ||
| # on main (e.g. pinchbench26, officeqa, terminal-bench-2). Track | ||
| # `main` so the two stay in lock-step. | ||
| BENCH_PIN: main |
There was a problem hiding this comment.
Use an immutable benchmarks ref in this privileged dispatcher path (Line 103).
BENCH_PIN: main makes pre-dispatch authorization/validation depend on a moving target, and it compounds with the mutable reusable-workflow ref at Line 219 (@main). In a privileged issue_comment workflow, this breaks immutability/provenance and reintroduces TOCTOU drift. Prefer a full commit SHA (or a bot-managed SHA bump process) and keep both validation and execution pinned to the same immutable revision.
As per coding guidelines, “.github/workflows/**: GitHub Actions hygiene… flag privileged workflows… unpinned third-party actions (pin full SHAs) …”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/nearai-bench.yml around lines 96 - 103, The BENCH_PIN
environment variable set to main creates a mutable reference that causes TOCTOU
drift between pre-dispatch validation and actual workflow execution in this
privileged issue_comment dispatcher. Replace BENCH_PIN from main with a full
immutable commit SHA. Additionally, update the reusable-workflow reference (the
uses directive that currently specifies `@main`) to use the same immutable commit
SHA so that both the pre-dispatch authorization check and the dispatched
workflow execution are pinned to the identical revision, eliminating the drift
vulnerability.
Source: Coding guidelines
…tale pin (nearai#4947) The /benchmark pre-dispatch suite check looked up `suites/<name>.toml` at a hardcoded BENCH_PIN SHA, but the dispatched `bench` job runs `bench-pr-reusable.yml@main` (and is documented to intentionally track benchmarks main). The pin had drifted, so suites that exist on benchmarks main were rejected as "Unknown suite" even though the run would have found them — e.g. `pinchbench26`, `officeqa`, `terminal-bench-2`. Point BENCH_PIN at `main` so the validation ref matches the ref we actually run against; they now stay in lock-step and new suites work the moment they land on benchmarks main. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problem
/benchmark pinchbench26 --framework ironclaw-rebornis rejected with:…even though
suites/pinchbench26.tomlis onnearai/benchmarksmainand the run would find it.Root cause
The pre-dispatch suite validation in
.github/workflows/nearai-bench.ymllooks up the suite TOML at a hardcodedBENCH_PINSHA:But the
benchjob that actually runs usesnearai/benchmarks/.github/workflows/bench-pr-reusable.yml@main— and its own comment says it intentionally tracks benchmarksmain. The pin had drifted behindmain, so the validation list is stale and rejects suites that already exist there:pinchbench26,officeqa,terminal-bench-2, etc. The check is supposed to "give the same answer the dispatched workflow would" — but it wasn't.Fix
Point
BENCH_PINatmainso the validation ref matches the ref we run against. They stay in lock-step, and new suites work the moment they land on benchmarksmain— no manual pin bump per suite. Same trust boundary the run already accepts (uses: …@main), so no new supply-chain delta.Verification
pinchbench26,officeqa,terminal-bench-2(all on benchmarksmain) now pass the existence check.suites/contents.🤖 Generated with Claude Code