Skip to content

test(bench): trial harness -- pinned container, run-trial.sh, parse-rollout.py - #389

Merged
thewrz merged 5 commits into
mainfrom
feat/issue-327
Aug 25, 2026
Merged

test(bench): trial harness -- pinned container, run-trial.sh, parse-rollout.py#389
thewrz merged 5 commits into
mainfrom
feat/issue-327

Conversation

@thewrz

@thewrz thewrz commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

This was written agentically; verify its assertions:

Why

Slice 5 of the token-benchmark epic (#152). Tier 0 (#343), the fixtures and gold tree (#349), the oracle (#326) and the pre-registration (#324) are merged; nothing yet runs a Tier-1 trial. The design's validity lives in its controls — one fresh container per trial, a baked-empty home, an identical concurrency cap on both arms, a GraphQL budget that pauses rather than fails — and those controls have to be mechanical, not remembered, before the pilot (#329) can spend anything.

What

  • bench/container/Dockerfile (+ README): debian:13 pinned by digest, every apt package pinned by exact version, gh from GitHub's keyed apt repo at a pinned version, the Codex CLI musl binary at rust-v0.148.0 verified against the sha256 GitHub's release-asset API reports. Non-root user, no secrets in any layer, no ENTRYPOINT. The header comment records the exact command used to resolve each pin.
  • bench/lib/: concurrency-cap-gate.sh (reuses the shipped concurrency-cap.sh parser; both arms present and identical or abort before spend), home-empty.sh (build + verify the baked-empty home against a named stateful-path list), container-lifecycle.sh (build/run/destroy plus an independent destruction proof; dry-run mode runs the same state machine on marker files), rate-limit-gate.sh (decides proceed/pause from gh api rate_limit, never sleeps itself), ledger-append.sh (append-only, symlink-refusing, extracted so tier1.jsonl does not carry a divergent copy of tier0.sh's inline logic).
  • bench/run-trial.sh: the nine-step trial sequence — cap gate → rate gate → empty home → container build/run → repo reset → invocation → oracle scoring via run-accept.shbench_trial_meta append + parse-rollout.py → ledger append → destroy and prove destroyed. --dry-run (default) exercises every step with no docker, no Codex, no spend; --live validates secrets then stops at the real invocation.
  • bench/parse-rollout.py: one trial's session logs → one ledger row: the grouping key, assigned vs realised (model, effort) with the pre-registered void rule made mechanical, the four token classes per actor, blended USD from an explicit placeholder rate table (--pricing override), per-path reference-read counts (the primary instrument), and the orchestration facts carried by a harness-appended bench_trial_meta record.
  • Tests: tests/test-bench-harness.sh (every gate, the lifecycle state machine, and two full dry-run trials — gold tree scores 10/10, untouched skeleton 0/10, a cap mismatch aborts before the home or ledger exist) and tests/test-bench-parse-rollout.sh (every expected number recomputed independently from the fixture bytes with jq/grep/awk; void paths; duplicate-meta and unpriced-model failures). Synthetic session fixtures under tests/fixtures/bench/.

Decisions

  • Two acceptance items are explicitly left to the operator, not quietly claimed: a real image build (never run in CI; ~150 MB pull) and one real, paid end-to-end trial. --live therefore stops with not implemented at the invocation step rather than shipping an unexercised docker exec path; wiring it is the first task of the pilot (test(bench): stage 1 pilot — two trials, go/no-go #329).
  • The rollout schema is modelled, not observed. Codex's real ~/.codex/sessions/*.jsonl shape was not readable within this slice's scope and a real trial was out of scope, so parse-rollout.py is written against a documented model of it and pinned by the synthetic fixture. The first real trial is the first chance to reconcile field names; parse-rollout.py's header says so.
  • bench_trial_meta is a harness-emitted record, not parsed prose. Selected set, chain plan, serialization/retry events, wall clock, worker count and exit condition are facts about how the harness drove the trial; recovering them by pattern-matching an LLM transcript would be unreliable, so run-trial.sh appends one record and the parser treats only those fields as ground truth — every token and reference figure still comes from the per-turn records.
  • Pricing is a placeholder table, deliberately. DEFAULT_PRICING exists so blended_usd is deterministic and spot-checkable; it is not live pricing and the parser never fetches any. Replace via --pricing before a real trial.
  • bench/results/tier1.jsonl is not committed: every test passes its own --ledger, so tests/test-bench-accept.sh's "no tier1.jsonl exists yet" assertion stays true until a real trial.
  • Well over the ~500-operational-line guideline. Left whole because the five gates, the trial script, and the parser are the single "harness" deliverable the issue names, and splitting them would ship gates with nothing calling them.

base=origin/main
files=22
total.insertions=1761
total.deletions=0
total.lines=1761
operational.files=12
operational.insertions=1702
operational.deletions=0
operational.lines=1702
generated.files=0
generated.insertions=0
generated.deletions=0
generated.lines=0
lockfile.files=0
lockfile.insertions=0
lockfile.deletions=0
lockfile.lines=0
fixture.files=10
fixture.insertions=59
fixture.deletions=0
fixture.lines=59
non_operational.files=10
non_operational.insertions=59
non_operational.deletions=0
non_operational.lines=59

Testing

  • Unit tests pass (full suite, fresh run against the final tree: ALL GREEN, rc=0)
  • Two full dry-run trials through run-trial.sh: gold tree 10/10, untouched skeleton 0/10, ledger row appended, container destruction independently proven
  • Concurrency-cap mismatch and absence abort before any spend-adjacent step
  • parse-rollout.py arithmetic spot-checked against independently computed fixture sums; void rule exercised both ways
  • Operator: docker build -t agent-kit-bench-tally:local bench/container succeeds
  • Operator: one real paid trial end-to-end; reconcile parse-rollout.py's schema against the real rollout
  • CI green

🤖 Co-authored by Claude Sonnet 5 (worker) / Claude Fable 5 (root).

Closes #327

…ollout.py

Adds the Tier-1 harness that actually runs a trial: a pinned debian:13 container image (digest + per-package version pins), bench/run-trial.sh (concurrency-cap gate, GraphQL rate-limit pause, baked-empty home, container lifecycle with destruction proof, repo reset, oracle scoring, ledger append), and bench/parse-rollout.py (turns Codex session logs into one schema-valid Tier-1 ledger record: token classes per actor, reference hit counts, blended USD, and the void-trial rule from PREREGISTRATION.md). Everything runs in a dry-run/stub mode with a synthetic session fixture -- no docker build, no real Codex spend -- which is the path tests/test-bench-harness.sh and tests/test-bench-parse-rollout.sh exercise end to end.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 05f7f3e8-23ea-4cb0-a9ca-bcc1953cf9e5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

…ard gaps in the trial harness

PR #389 adversarial review confirmed four findings against the Tier-1 harness (commit 0aaa0f4): (F1, P1) a fully-configured --live run built and started a real, credential-bearing container before dying "not implemented" at the invocation step, leaving it running -- --live is now rejected during argument validation, before any container/home/ledger allocation, with an EXIT trap as defense-in-depth for whenever a live container does start; (F2, P2) container-lifecycle functions only honored the --dry-run flag even though their own header documented BENCH_CONTAINER_DRY_RUN=1 as equivalent -- now both spellings route through one helper; (F3, P2) container_destroy/container_is_destroyed discarded docker rm -f/docker ps exit status, so a failed query could read as "destroyed" -- both now fail loudly and keep the running-marker on a genuine docker failure; (F4, P2) the ledger symlink guard used -e && -L, which a dangling symlink defeats (-e is false for it) -- now refuses on -L alone. Reproduced F1 and F2 live during RED verification (git-stashed the fixes and re-ran the new tests against the pre-fix code): the old code genuinely built an image and started two real containers with dummy secrets, and one BENCH_CONTAINER_DRY_RUN=1 test unexpectedly triggered a real docker build -- both cleaned up (docker rm -f / docker rmi) before restoring the fixes.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>
@thewrz

thewrz commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator Author

This was written agentically; verify its assertions:

Adversarial review receipt

  • Reviewer: provider=openai; model=gpt-5.6-terra; effort=xhigh; mode=cross-provider (reason: peer-cli=codex present; running harness=claude)
  • Counts: P1=1; P2=3; total=4
  • Confirmed finding: --live starts a credential-bearing container before dying unimplemented; never torn down — verdict=fixed; fix commit SHA(s)=8528c0a2464ff42497ffd891336dedf6461e3f4f
  • Confirmed finding: BENCH_CONTAINER_DRY_RUN=1 documented but not honored by lifecycle functions — verdict=fixed; fix commit SHA(s)=8528c0a2464ff42497ffd891336dedf6461e3f4f
  • Confirmed finding: container_destroy swallows docker rm failure; container_is_destroyed misreads a failed docker ps as gone — verdict=fixed; fix commit SHA(s)=8528c0a2464ff42497ffd891336dedf6461e3f4f
  • Confirmed finding: ledger_append symlink guard bypassed by a dangling symlink — verdict=fixed; fix commit SHA(s)=8528c0a2464ff42497ffd891336dedf6461e3f4f

🤖 Co-authored by Claude Fable 5 (root orchestrator).

thewrz pushed a commit that referenced this pull request Aug 23, 2026
…s from staleness

record-tier0.yml pushes a post-merge [skip ci] commit to main after every merge; merge-gate.sh blocked the next queued PR on base: stale=yes, forcing a second merge-down and CI run per PR (agent-kit#394, 2026-08-22 queue #387-#389). gh-pr-state.sh now exempts a base advance whose new commits touch only the repository-declared AGENT_GENERATED_PATHS prefixes from staleness; any other base advance still stales. Documented in onboard-repo and the dispatch-plan generated-path handling, with a fixture test covering a base advance confined to bench/results/tier0.jsonl.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>
thewrz added a commit that referenced this pull request Aug 23, 2026
…taleness (#416)

* fix(pr-to-green): exempt declared generated/results-path base advances from staleness

record-tier0.yml pushes a post-merge [skip ci] commit to main after every merge; merge-gate.sh blocked the next queued PR on base: stale=yes, forcing a second merge-down and CI run per PR (agent-kit#394, 2026-08-22 queue #387-#389). gh-pr-state.sh now exempts a base advance whose new commits touch only the repository-declared AGENT_GENERATED_PATHS prefixes from staleness; any other base advance still stales. Documented in onboard-repo and the dispatch-plan generated-path handling, with a fixture test covering a base advance confined to bench/results/tier0.jsonl.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(pr-to-green): declare bench/results/ in AGENT_GENERATED_PATHS

The base-staleness exemption added in f4c8728 was correct but inert for agent-kit's own record-tier0.yml commits: this repository's declared AGENT_GENERATED_PATHS (.agent/board.json only) did not cover bench/results/tier0.jsonl, so the issue's own Evidence-section defect stayed live on merge. Adds bench/results/ to the declaration, a regression-pin test asserting this repo's own config covers bench/results/tier0.jsonl, and a note on the dual meaning of AGENT_GENERATED_PATHS in its schema template.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(pr-to-green): fail closed on truncated compares and undeclared renames

Adversarial review of PR #416 (Codex gpt-5.6-terra, xhigh) confirmed two fail-open gaps in base_advance_is_automation_only: (1) GitHub compare API caps .files at 300 entries per page with no pagination follow-through here, so a base advance touching 301+ files could report stale=no if only the first 300 (all declared) were read; (2) a rename only checked the new filename, so a rename FROM an undeclared path INTO a declared one (or vice versa) reported stale=no despite moving application code. Both now fail closed: a files count at or above the named COMPARE_FILES_PAGE_CAP (300) is treated as unreadable evidence, and both filename and previous_filename must match a declared prefix for a renamed entry. Adds regression-pin tests for each.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

---------

Co-authored-by: mergetest <merge@test.local>
Co-authored-by: Claude claude-sonnet-5 <noreply@anthropic.com>
thewrz added a commit that referenced this pull request Aug 23, 2026
…igest (#418)

* fix(pr-to-green): exempt declared generated/results-path base advances from staleness

record-tier0.yml pushes a post-merge [skip ci] commit to main after every merge; merge-gate.sh blocked the next queued PR on base: stale=yes, forcing a second merge-down and CI run per PR (agent-kit#394, 2026-08-22 queue #387-#389). gh-pr-state.sh now exempts a base advance whose new commits touch only the repository-declared AGENT_GENERATED_PATHS prefixes from staleness; any other base advance still stales. Documented in onboard-repo and the dispatch-plan generated-path handling, with a fixture test covering a base advance confined to bench/results/tier0.jsonl.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(pr-to-green): declare bench/results/ in AGENT_GENERATED_PATHS

The base-staleness exemption added in f4c8728 was correct but inert for agent-kit's own record-tier0.yml commits: this repository's declared AGENT_GENERATED_PATHS (.agent/board.json only) did not cover bench/results/tier0.jsonl, so the issue's own Evidence-section defect stayed live on merge. Adds bench/results/ to the declaration, a regression-pin test asserting this repo's own config covers bench/results/tier0.jsonl, and a note on the dual meaning of AGENT_GENERATED_PATHS in its schema template.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(pr-to-green): fail closed on truncated compares and undeclared renames

Adversarial review of PR #416 (Codex gpt-5.6-terra, xhigh) confirmed two fail-open gaps in base_advance_is_automation_only: (1) GitHub compare API caps .files at 300 entries per page with no pagination follow-through here, so a base advance touching 301+ files could report stale=no if only the first 300 (all declared) were read; (2) a rename only checked the new filename, so a rename FROM an undeclared path INTO a declared one (or vice versa) reported stale=no despite moving application code. Both now fail closed: a files count at or above the named COMPARE_FILES_PAGE_CAP (300) is treated as unreadable evidence, and both filename and previous_filename must match a declared prefix for a renamed entry. Adds regression-pin tests for each.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(review-remote-pr): report landed CodeRabbit review state instead of coderabbit=none

gh-pr-state.sh derived provider: coderabbit=... from an issue-comment phrase scan, which an
APPROVED review with zero actionable threads (or a CHANGES_REQUESTED review with inline
threads only) never matches -- PR #386 read coderabbit=none for 15 wasted one-minute
rounds after CodeRabbit had already approved it. The real signal is the reviews endpoint
itself: an acknowledgement is a plain issue comment, never a review submission, so it
can never be mistaken for one. provider_state() now reports the most recent terminal
review directly (state=APPROVED|CHANGES_REQUESTED|COMMENTED threads=N since=TIMESTAMP),
falling back to the rate-limit/none scan only when no review has landed yet.

review-transition.sh gains a lightweight --observe --since TIMESTAMP mode that answers
"has the review landed since I triggered it" without repeating the full ready-transition
and provider-spend flow, returning LANDED (or PENDING); its TRIGGERED/ALREADY_SPENT
results now carry that since= boundary. merge-gate.sh accepts LANDED as evidence-green,
exactly like AUTO_REVIEW/ALREADY_SPENT.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(pr-to-green): pin CodeRabbit review evidence to the current head SHA

Adversarial review of PR #418 found that neither gh-pr-state.sh's provider_state()
nor review-transition.sh's new --observe path checked a review's own commit_id: a
CodeRabbit review submitted against an OLD head can finish AFTER the PR advances to a
new one, with a submitted_at that looks perfectly current. That let a stale review
(of code nobody has reviewed) be reported/accepted as landed evidence for the current
head -- the exact failure this issue set out to fix, arriving from the other direction.

Both selectors now require the review's commit_id to match the current head (fetched
fresh for --observe, read from the already-fetched PR metadata for the digest) before
calling it landed/reviewed. A terminal review that exists but targets a different head
is reported distinctly (stale-head / STALE_HEAD) -- never reviewed/LANDED (misrepresents
stale evidence as current) and never none/PENDING (hides that a review exists at all).
merge-gate.sh blocks STALE_HEAD exactly like an in-flight TRIGGERED review.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

---------

Co-authored-by: mergetest <merge@test.local>
Co-authored-by: Claude claude-sonnet-5 <noreply@anthropic.com>
thewrz added a commit that referenced this pull request Aug 23, 2026
…nly (#419)

* fix(pr-to-green): exempt declared generated/results-path base advances from staleness

record-tier0.yml pushes a post-merge [skip ci] commit to main after every merge; merge-gate.sh blocked the next queued PR on base: stale=yes, forcing a second merge-down and CI run per PR (agent-kit#394, 2026-08-22 queue #387-#389). gh-pr-state.sh now exempts a base advance whose new commits touch only the repository-declared AGENT_GENERATED_PATHS prefixes from staleness; any other base advance still stales. Documented in onboard-repo and the dispatch-plan generated-path handling, with a fixture test covering a base advance confined to bench/results/tier0.jsonl.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(pr-to-green): declare bench/results/ in AGENT_GENERATED_PATHS

The base-staleness exemption added in f4c8728 was correct but inert for agent-kit's own record-tier0.yml commits: this repository's declared AGENT_GENERATED_PATHS (.agent/board.json only) did not cover bench/results/tier0.jsonl, so the issue's own Evidence-section defect stayed live on merge. Adds bench/results/ to the declaration, a regression-pin test asserting this repo's own config covers bench/results/tier0.jsonl, and a note on the dual meaning of AGENT_GENERATED_PATHS in its schema template.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(pr-to-green): fail closed on truncated compares and undeclared renames

Adversarial review of PR #416 (Codex gpt-5.6-terra, xhigh) confirmed two fail-open gaps in base_advance_is_automation_only: (1) GitHub compare API caps .files at 300 entries per page with no pagination follow-through here, so a base advance touching 301+ files could report stale=no if only the first 300 (all declared) were read; (2) a rename only checked the new filename, so a rename FROM an undeclared path INTO a declared one (or vice versa) reported stale=no despite moving application code. Both now fail closed: a files count at or above the named COMPARE_FILES_PAGE_CAP (300) is treated as unreadable evidence, and both filename and previous_filename must match a declared prefix for a renamed entry. Adds regression-pin tests for each.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(review-remote-pr): report landed CodeRabbit review state instead of coderabbit=none

gh-pr-state.sh derived provider: coderabbit=... from an issue-comment phrase scan, which an
APPROVED review with zero actionable threads (or a CHANGES_REQUESTED review with inline
threads only) never matches -- PR #386 read coderabbit=none for 15 wasted one-minute
rounds after CodeRabbit had already approved it. The real signal is the reviews endpoint
itself: an acknowledgement is a plain issue comment, never a review submission, so it
can never be mistaken for one. provider_state() now reports the most recent terminal
review directly (state=APPROVED|CHANGES_REQUESTED|COMMENTED threads=N since=TIMESTAMP),
falling back to the rate-limit/none scan only when no review has landed yet.

review-transition.sh gains a lightweight --observe --since TIMESTAMP mode that answers
"has the review landed since I triggered it" without repeating the full ready-transition
and provider-spend flow, returning LANDED (or PENDING); its TRIGGERED/ALREADY_SPENT
results now carry that since= boundary. merge-gate.sh accepts LANDED as evidence-green,
exactly like AUTO_REVIEW/ALREADY_SPENT.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(review-remote-pr): detect Code Quality disabled (403) and skip cleanly

AGENT_REVIEW_PROVIDERS=github-code-quality was accepted at plan time even
when the repository had GitHub Code Quality disabled, and code-quality-state.sh
then died mid-gate on a raw 403. code-quality-state.sh gains --probe: a single
lightweight request that decides state=enabled/not-enabled/unknown, treating
only a confirmed "not enabled" 403 message as proof of disablement -- every
other failure (network, auth/scope, 5xx) reports unknown and fails closed.
review-provider-config.sh --probe uses it once to downgrade the declared
github-code-quality plan line on a confirmed not-enabled, leaving every other
provider and the default (no --probe) call path -- including review-transition.sh -- byte-for-byte
unchanged. The review-remote-pr Step 5 recipe probes before fetching findings
instead of dying on the 403, and merge-gate.sh accepts
--code-quality-scan-state not-enabled as a pass, since a disabled repository
has no scan to wait for.

Closes #403.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(pr-to-green): pin CodeRabbit review evidence to the current head SHA

Adversarial review of PR #418 found that neither gh-pr-state.sh's provider_state()
nor review-transition.sh's new --observe path checked a review's own commit_id: a
CodeRabbit review submitted against an OLD head can finish AFTER the PR advances to a
new one, with a submitted_at that looks perfectly current. That let a stale review
(of code nobody has reviewed) be reported/accepted as landed evidence for the current
head -- the exact failure this issue set out to fix, arriving from the other direction.

Both selectors now require the review's commit_id to match the current head (fetched
fresh for --observe, read from the already-fetched PR metadata for the digest) before
calling it landed/reviewed. A terminal review that exists but targets a different head
is reported distinctly (stale-head / STALE_HEAD) -- never reviewed/LANDED (misrepresents
stale evidence as current) and never none/PENDING (hides that a review exists at all).
merge-gate.sh blocks STALE_HEAD exactly like an in-flight TRIGGERED review.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* docs(pr-to-green): document --code-quality-scan-state not-enabled

auto-merge.md still described --code-quality-scan-state as complete|pending
after issue #403 added a third value; the STALE_HEAD provider-result direction
was already current (issue #395 documented it in the same file). Updates the
bullet to name not-enabled and explain it gates exactly like complete, while
an unreadable probe (network failure, auth/scope 403, a 5xx) still blocks.
Prose only, no behavior change.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

---------

Co-authored-by: mergetest <merge@test.local>
Co-authored-by: Claude claude-sonnet-5 <noreply@anthropic.com>
thewrz added a commit that referenced this pull request Aug 23, 2026
…dence (#420)

* fix(pr-to-green): exempt declared generated/results-path base advances from staleness

record-tier0.yml pushes a post-merge [skip ci] commit to main after every merge; merge-gate.sh blocked the next queued PR on base: stale=yes, forcing a second merge-down and CI run per PR (agent-kit#394, 2026-08-22 queue #387-#389). gh-pr-state.sh now exempts a base advance whose new commits touch only the repository-declared AGENT_GENERATED_PATHS prefixes from staleness; any other base advance still stales. Documented in onboard-repo and the dispatch-plan generated-path handling, with a fixture test covering a base advance confined to bench/results/tier0.jsonl.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(pr-to-green): declare bench/results/ in AGENT_GENERATED_PATHS

The base-staleness exemption added in f4c8728 was correct but inert for agent-kit's own record-tier0.yml commits: this repository's declared AGENT_GENERATED_PATHS (.agent/board.json only) did not cover bench/results/tier0.jsonl, so the issue's own Evidence-section defect stayed live on merge. Adds bench/results/ to the declaration, a regression-pin test asserting this repo's own config covers bench/results/tier0.jsonl, and a note on the dual meaning of AGENT_GENERATED_PATHS in its schema template.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(pr-to-green): fail closed on truncated compares and undeclared renames

Adversarial review of PR #416 (Codex gpt-5.6-terra, xhigh) confirmed two fail-open gaps in base_advance_is_automation_only: (1) GitHub compare API caps .files at 300 entries per page with no pagination follow-through here, so a base advance touching 301+ files could report stale=no if only the first 300 (all declared) were read; (2) a rename only checked the new filename, so a rename FROM an undeclared path INTO a declared one (or vice versa) reported stale=no despite moving application code. Both now fail closed: a files count at or above the named COMPARE_FILES_PAGE_CAP (300) is treated as unreadable evidence, and both filename and previous_filename must match a declared prefix for a renamed entry. Adds regression-pin tests for each.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(review-remote-pr): report landed CodeRabbit review state instead of coderabbit=none

gh-pr-state.sh derived provider: coderabbit=... from an issue-comment phrase scan, which an
APPROVED review with zero actionable threads (or a CHANGES_REQUESTED review with inline
threads only) never matches -- PR #386 read coderabbit=none for 15 wasted one-minute
rounds after CodeRabbit had already approved it. The real signal is the reviews endpoint
itself: an acknowledgement is a plain issue comment, never a review submission, so it
can never be mistaken for one. provider_state() now reports the most recent terminal
review directly (state=APPROVED|CHANGES_REQUESTED|COMMENTED threads=N since=TIMESTAMP),
falling back to the rate-limit/none scan only when no review has landed yet.

review-transition.sh gains a lightweight --observe --since TIMESTAMP mode that answers
"has the review landed since I triggered it" without repeating the full ready-transition
and provider-spend flow, returning LANDED (or PENDING); its TRIGGERED/ALREADY_SPENT
results now carry that since= boundary. merge-gate.sh accepts LANDED as evidence-green,
exactly like AUTO_REVIEW/ALREADY_SPENT.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(review-remote-pr): detect Code Quality disabled (403) and skip cleanly

AGENT_REVIEW_PROVIDERS=github-code-quality was accepted at plan time even
when the repository had GitHub Code Quality disabled, and code-quality-state.sh
then died mid-gate on a raw 403. code-quality-state.sh gains --probe: a single
lightweight request that decides state=enabled/not-enabled/unknown, treating
only a confirmed "not enabled" 403 message as proof of disablement -- every
other failure (network, auth/scope, 5xx) reports unknown and fails closed.
review-provider-config.sh --probe uses it once to downgrade the declared
github-code-quality plan line on a confirmed not-enabled, leaving every other
provider and the default (no --probe) call path -- including review-transition.sh -- byte-for-byte
unchanged. The review-remote-pr Step 5 recipe probes before fetching findings
instead of dying on the 403, and merge-gate.sh accepts
--code-quality-scan-state not-enabled as a pass, since a disabled repository
has no scan to wait for.

Closes #403.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(pr-to-green): pin CodeRabbit review evidence to the current head SHA

Adversarial review of PR #418 found that neither gh-pr-state.sh's provider_state()
nor review-transition.sh's new --observe path checked a review's own commit_id: a
CodeRabbit review submitted against an OLD head can finish AFTER the PR advances to a
new one, with a submitted_at that looks perfectly current. That let a stale review
(of code nobody has reviewed) be reported/accepted as landed evidence for the current
head -- the exact failure this issue set out to fix, arriving from the other direction.

Both selectors now require the review's commit_id to match the current head (fetched
fresh for --observe, read from the already-fetched PR metadata for the digest) before
calling it landed/reviewed. A terminal review that exists but targets a different head
is reported distinctly (stale-head / STALE_HEAD) -- never reviewed/LANDED (misrepresents
stale evidence as current) and never none/PENDING (hides that a review exists at all).
merge-gate.sh blocks STALE_HEAD exactly like an in-flight TRIGGERED review.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* feat(review-remote-pr): keep per-PR RUN_DIR pointers under .agent/evidence

Step 0c minted a randomly named RUN_DIR under ${TMPDIR:-/tmp} every run, with no durable pointer beyond the harness scratchpad. A resumed session lost the mapping even though the directory still existed on disk, orphaning digests, consent records, and receipts (issue #405).

The new run-dir.sh helper owns the PR -> RUN_DIR mapping: the same PR always resolves to the same private (0700) `.agent/evidence/pr-<N>` directory, already excluded from git. A pre-existing target that is a symlink, not a directory, not owned by this user, or not mode 0700 is refused rather than reused or silently widened. ${TMPDIR:-/tmp} is used only as a real fallback, keyed by repository and PR number, on hosts where `.agent/` genuinely cannot be created or secured.

Step 0c in review-remote-pr/SKILL.md now calls the helper instead of `mktemp -d`; pr-to-green and parallel-issues Phase 3 already delegate to that same step by reference, so no separate recipe exists there to replace.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(parallel-issues): derive RUN_DIR from run-dir.sh in Phase 3 instead of recalling it

Phase 3 still told its dispatched review-remote-pr loop to "re-set RUN_DIR to the Step 0c output" -- the exact unrecoverable-pointer pattern issue #405 fixes elsewhere: after a resume, nothing tells the reader what that prior output was.

Both Phase 3 blocks (the pre-launch adversarial-review precheck and the post-fix receipt publish) now call run-dir.sh --pr "$PR" directly, the same way review-remote-pr's own Step 0c does, so RUN_DIR is actively re-derived rather than remembered. The guard reordering keeps the fail-loud intent: the PR guard and the agentkit cache-rehydration guard both still run before the derivation, and a run-dir.sh failure still exits 1.

Net effect on parallel-issues/SKILL.md: 1133/1133 lines (unchanged) and 19554 -> 19529 estimated tokens (a small decrease), so no size-gate ceiling raise is needed.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* docs(pr-to-green): document --code-quality-scan-state not-enabled

auto-merge.md still described --code-quality-scan-state as complete|pending
after issue #403 added a third value; the STALE_HEAD provider-result direction
was already current (issue #395 documented it in the same file). Updates the
bullet to name not-enabled and explain it gates exactly like complete, while
an unreadable probe (network failure, auth/scope 403, a 5xx) still blocks.
Prose only, no behavior change.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

* fix(review-remote-pr): refuse a dangling symlink in run-dir.sh instead of falling back

Adversarial review (Codex, xhigh) on PR #420 found: `[[ -e $dir ]]` follows a symlink, so it is FALSE for a dangling symlink (target does not exist). Both `ensure_private_root` and try_primary's `.agent` check gated their `-L` refusal inside an `-e` branch, so a dangling symlink at `.agent` or `.agent/evidence` skipped the refusal entirely, fell through to `mkdir` (which fails EEXIST against the link itself), and that failure was read as "not writable" -- silently routing evidence to the /tmp fallback instead of refusing the hostile path.

Both sites now check `-L` unconditionally, before any `-e`-gated branch, matching the shape private_dir_ensure (private-dir.sh) already uses -- confirmed that shared helper does not have this bug. `finding-ledger.sh` records this as a confirmed P2, fixed, from the adversarial-review receipt.

Added regression coverage for the dangling case at every level a symlink can occupy: .agent/evidence, .agent itself, the per-PR leaf (via the shared private_dir_ensure, confirming it was already correct), and the /tmp fallback root -- each asserting the actual refusal (RC=1, symlink named, "not writable" NOT reported), not merely that the primary path was not returned.

Co-Authored-By: Claude claude-sonnet-5 <noreply@anthropic.com>

---------

Co-authored-by: mergetest <merge@test.local>
Co-authored-by: Claude claude-sonnet-5 <noreply@anthropic.com>
Refresh the trial harness branch onto the current repository state for final verification.

Co-Authored-By: Codex <noreply@openai.com>
@thewrz
thewrz marked this pull request as ready for review August 25, 2026 02:39
@thewrz
thewrz merged commit 02cc833 into main Aug 25, 2026
4 checks passed
@thewrz
thewrz deleted the feat/issue-327 branch August 26, 2026 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(bench): trial harness — pinned container, run-trial.sh, parse-rollout.py

1 participant