fix(bench,arenabench): refuse a SUT binary too far behind the code it is reported as measuring - #2052
Merged
Merged
Conversation
… is reported as measuring Pinning and freshness are different properties, and only the first was checked. `~/.arenabench/sut/stella` — the path every runbook exports as STELLA_BINARY — sat 291 commits and three days behind origin/main while carrying its own sut_commit.txt naming that ancient commit, so it looked pinned, passed everything #2016/#2020 added, and produced perfectly scoreable trials attributed to code rewritten underneath them. The new guard reads the commit out of the artifact's own compile-time bytes rather than the sidecar beside it: build.rs stamps STELLA_BUILD_GIT_SHA into a NUL-delimited literal precisely so the identity can be recovered without executing a cross-compiled binary. A repointed path or a hand-refreshed sidecar therefore cannot make an old build look new, and a sidecar disagreeing with the binary is itself a refusal. Also names the build_sut.sh fetch race reported on the issue: the script recorded origin/main only after its own fetch, so a branch that moved mid-run produced a drift list of files nobody touched, reading as local contamination. It now takes an optional commit argument, and when it fetches it compares against where origin/main stood beforehand so the refusal can say which of the two causes it is. Closes #2032 Refs #2049, #2050, #2051
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
Reviewer's GuideAdds a binary-freshness guard for the Stella SUT, wiring it into both the Terminal-Bench runbook and arenabench so unpinned runs refuse binaries that are measurably too far behind origin/main, and fixes a build race in build_sut.sh; includes a standalone Python checker, new arena-side helpers, and comprehensive tests/docs. Sequence diagram for unpinned arena match freshness checksequenceDiagram
actor Operator
participant ArenabenchRunner as arenabench.runner
participant SutModule as arenabench.sut
participant GitRepo as git_checkout
Operator->>ArenabenchRunner: create_match(spec with sut_ref="")
ArenabenchRunner->>SutModule: sut_problem_for(spec)
SutModule->>SutModule: unpinned_problem()
SutModule->>SutModule: ambient_sut()
SutModule->>SutModule: embedded_commit(path)
SutModule->>GitRepo: resolve_ref("main")
GitRepo-->>SutModule: target_commit
SutModule->>GitRepo: drift_between(ambient.commit, target_commit)
GitRepo-->>SutModule: Drift(behind, comparable)
alt drift.comparable and drift.behind > MAX_BEHIND_UNPINNED
SutModule-->>ArenabenchRunner: problem string (binary too far behind)
ArenabenchRunner-->>Operator: refuse launch
else not stale or not comparable
SutModule-->>ArenabenchRunner: None
ArenabenchRunner-->>Operator: launch proceeds (with warning logic)
end
Sequence diagram for Terminal-Bench SUT build and freshness verificationsequenceDiagram
actor Operator
participant BuildScript as build_sut.sh
participant EnvSh as env.sh
participant Freshness as freshness.py
participant GitRepo as git_checkout
participant Binary as STELLA_BINARY
Operator->>BuildScript: ./build_sut.sh [<commit>]
alt commit argument given
BuildScript->>GitRepo: git rev-parse --verify "<commit>^{commit}"
GitRepo-->>BuildScript: SUT_commit
BuildScript->>GitRepo: git diff --name-only SUT_commit -- RUST_INPUTS
else no commit argument
BuildScript->>GitRepo: git rev-parse --verify --quiet origin/main
GitRepo-->>BuildScript: BEFORE
BuildScript->>GitRepo: git fetch origin main
BuildScript->>GitRepo: git rev-parse origin/main
GitRepo-->>BuildScript: SUT_commit
BuildScript->>GitRepo: git diff --name-only SUT_commit -- RUST_INPUTS
end
BuildScript-->>Operator: build SUT binary at STELLA_BINARY
Operator->>EnvSh: preflight()
EnvSh->>EnvSh: assert_portable_binary()
EnvSh->>EnvSh: assert_fresh_sut()
EnvSh->>Freshness: python3 freshness.py "$STELLA_BINARY" --repo "$TB_REPO" --reference "$SUT" --max-behind 0 (from build_sut.sh) or DEFAULT_MAX_BEHIND
Freshness->>Freshness: read_identity(binary)
Freshness->>Freshness: embedded_source_commits(path)
Freshness->>Freshness: read_sidecar_commit(directory)
Freshness->>GitRepo: measure_distance(identity.commit, repo, reference)
GitRepo-->>Freshness: Distance
Freshness->>Freshness: check_freshness(identity, distance, max_behind)
alt violations found
Freshness-->>EnvSh: exit status 1 or 2
EnvSh-->>Operator: FATAL: binary not fresh - rebuild
else no violations
Freshness-->>EnvSh: exit status 0
EnvSh-->>Operator: preflight succeeds - run may proceed
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Pinning and freshness are different properties, and until now only the first was
checked.
~/.arenabench/sut/stella— the path every runbook and launch scriptexports as
STELLA_BINARY— sat 291 commits and three days behindorigin/mainwhile carrying its ownsut_commit.txtnaming that ancientcommit. So it looked pinned, passed everything #2016/#2020 added, and produced
perfectly scoreable trials attributed to code that had been rewritten underneath
them. A commit id answers which code; only a distance from a reference answers
whether it is the code anyone meant.
Closes #2032
The guard reads the artifact, not the paperwork
sut_commit.txtis extrinsic — a claim about a file, and precisely the thingthat went stale and that a symlink can silently repoint.
crates/stella-cli/build.rsstamps
STELLA_BUILD_GIT_SHAinto a single<version>-dev.<40-hex>literal, andbuild_info.rsdeliberately surrounds it with NUL bytes so LLVM's string poolingcannot adjoin identifier characters to it — its doc comment says outright that
this exists so the identity can be attested without executing the binary. That
is intrinsic: it travels inside the artifact and no rename, copy, or symlink
can separate the two.
New
bench/harbor_adapter/stella_harbor/freshness.pyreads it and measures thedistance. A repointed path or a hand-refreshed sidecar cannot make an old build
look new, and a sidecar that disagrees with the binary is itself a refusal.
Reading the ELF rather than running
stella --versionis what lets this run onthe host before any container exists, on a macOS machine that cannot exec a
linux/amd64 binary at all — the same reasoning that made
portability.pyparse.gnu.version_rin pure stdlib instead of shelling out toreadelf.Exemplar followed:
stella_harbor/portability.py, deliberately and closely —stdlib-only, standalone-runnable,
0fresh /1stale /2undeterminable, acheck_*returning a list of violations, blocking on the host inenv.shratherthan per-trial in the container.
Wired at both launch paths
preflight()inbench/evidence/run/env.sh2and fatal, because this path publishes numberssut.unpinned_problem()inarenabench/arenabench/sut.pycargo build --releasecarries no stamp, cannot be dated, and must not block the local development loopThe difference is deliberate and follows from who reads the result, not from a
disagreement about what is safe. Clearing
sut_refopts out of pinning, notout of every check: an unpinned match asks for whatever is current, so a binary
that is measurably not current contradicts the request rather than opting out of
it. Measuring older code on purpose is still available and is spelled the
supported way — pin
sut_refto that commit, which puts the answer in theresult.
The limit is a stated distance, measured not guessed
DEFAULT_MAX_BEHIND = 25.origin/mainmoved 654 commits in the seven days to2026-08-07 (~93/day), so 25 is about six hours of drift: it tolerates a binary
staged at the start of a long run whose
mainmoved underneath it, and refusesthe 291-commit artifact by a factor of twelve. It lives in the checker and is not
restated in shell, so the two copies cannot drift apart.
The
build_sut.shfetch race, from the issue commentThe script recorded
origin/mainonly after its owngit fetch, so a branchthat moved between your checkout and that fetch produced a drift list of files
nobody touched — reading as local contamination and sending you hunting for edits
that do not exist (observed on the issue:
44f4c63a→9b502f1c).It now takes an optional commit argument, so a wrapper that fetches and checks
out can build exactly what it prepared. Unargued, it records
origin/mainbeforethe fetch as well and separates the two causes: if the tree is byte-identical to
where the branch started, the refusal says so, names both SHAs and the distance,
and suggests
build_sut.sh <that-commit>. It also verifies its own output —--reference "$SUT" --max-behind 0is an equality assertion on the binary'scompile-time stamp, catching a build that silently stamped something else.
The witness
main, passes here)TestAnUnpinnedMatchStillRefusesStaleCode::test_the_stale_runbook_binary_now_blocks_the_launchis the witness the issue asks for verbatim: an unpinned Stella seat with
STELLA_BINARYpointing at a binary far behindorigin/mainmust refuse, namingthe commit and the distance.
Checked the artisanal way, and checked behaviourally rather than by deletion —
reverting only the
sut_problem_fordispatch line (leaving the new constants inplace, so the failure is not merely a collection error):
Nonemeans the launch proceeds — exactly the reported failure. With the change,green.
bench/harbor_adapter/tests/test_freshness.pyadds 33 more, includingTestCli::test_the_documented_witness(the issue'sreadlink -fscenario as aCLI assertion), the symlink case (the rig's own mitigation — a guard reading the
link's directory would report the stale claim), and a stamp straddling the
1 MiB read boundary, which is the one input a naive reader loses and which fails
open onto the sidecar this module exists to distrust.
The gate
make guards-fastgreen (no Rust touched — this is the rung the pre-pushhook picks for a diff that reaches no crate)
bench/harbor_adapter: 473 passed, 1 skippedarenabench: full suite greenbench/RUNBOOK.md,bench/evidence/run/README.md,arenabench/README.mdCloses #2032appears both here and as a commit trailerTwo mechanical notes for review, both flagged by guards rather than by me:
freshness.pyhad to be added to_FIXED_ADAPTER_SOURCE_PATHSinsecure_launcher.py. That tuple is enumerated by hand on purpose — the paidlauncher byte-compares the running adapter against the published commit, so a
globbed file would execute without ever being compared. Forgetting it failed
closed with "public adapter tree hash differs from runtime identity", which is
the guard doing its job.
secure_launcher.pyfrom 4293 to 4294, over its ceiling.Handled with
make file-size-update, and the baseline diff is exactly+1onthat one row — the documented irreducible case (a required registration entry
in an already-oversized file), not a ceiling raised to turn a gate green.
Nothing left behind
Filed: bench: make shellcheck skips bench/evidence/run/*.sh, so the paid-run preflight scripts are ungated #2049, bench: two copies of the embedded-commit reader inside stella_harbor can disagree about a binary's identity #2050, bench: a run driven through the adapter without env.sh still measures a binary of any age #2051
bench: make shellcheck skips bench/evidence/run/*.sh, so the paid-run preflight scripts are ungated #2049 —
make shellcheckcovers onlyinstall.sh,scripts/*.shand.githooks/*, sobench/evidence/run/*.shis ungated. My edits to two ofthose scripts passed
make gategreen; running shellcheck by hand surfacedreal findings, including a pre-existing SC2155 in
env.sh.bench: two copies of the embedded-commit reader inside stella_harbor can disagree about a binary's identity #2050 — there are now two copies of the embedded-commit reader inside
stella_harbor(secure_launcher._binary_version_source_commitsandfreshness.embedded_source_commits). They can disagree about an edge case, andthe failure mode is one surface calling a binary identifiable while the other
does not. The third copy in
arenabench/arenabench/sut.pyis deliberate anddocumented — arenabench is Apache-2.0 and slated for ejection, so it must not
import from the AGPL adapter tree.
bench: a run driven through the adapter without env.sh still measures a binary of any age #2051 — the guard blocks at two launch-time chokepoints. A caller who
drives Harbor directly and never sources
env.shis still unguarded; that isthe residual half of option 3 in the issue.
Anything reviewers should know?
The judgement call worth a second opinion is the arena's fail-open posture.
An unstamped
cargo build --releaseis not blocked, because refusing it wouldbreak the local development loop. That is a deliberate asymmetry with the
evidence runbook, argued in
unpinned_problem's docstring — but it does mean thearena still cannot catch a stale unstamped binary. If you would rather it fail
closed there too, say so; the change is one branch.
I did not implement the issue's option 1 ("delete
sut/stella, make the barepath an error").
sut_ref=""is an existing, documented affordance, and turningit into a hard error is a behaviour decision I did not think was mine to make
silently. Option 3 was the issue's own note as strictly more general, and it is
what shipped. Option 2 is already applied rig-side and this PR's symlink test
pins that it keeps working.
MAX_BEHIND_UNPINNEDandDEFAULT_MAX_BEHINDare two copies of25across thetwo trees, documented as matching but not enforced to be. Cross-tree enforcement
is the wrong fix given the pending ejection; #2050 asks for an explicit decision
on whether they should stay coupled at all.
Summary by Sourcery
Enforce freshness checks for the Stella SUT binary across arena matches and Terminal-Bench runbook, measuring how far the compiled artifact’s embedded commit is behind the reference branch and refusing stale or inconsistent binaries, while tightening build tooling and documentation around SUT provenance.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: