fix(agent-challenge): unblock Terminal-Bench scoring and surface trial failures - #64
Conversation
The own_runner job container runs with a read-only rootfs, so pip installs the miner agent and its dependencies into /tmp/.local. Docker defaults --tmpfs mounts to noexec, so every native wheel (tiktoken, pydantic-core, numpy) failed to load with "failed to map segment from shared object" and the submission scored 0 for an infrastructure reason rather than on merit. Add exec to the /tmp tmpfs options while keeping nosuid. The analyzer container intentionally keeps noexec; only this runner needs to execute miner-installed native code.
The broker never bind-mounts the job directory: it materializes each mount from an archive into its own workspace and copies nothing back, so per-trial artifacts (result.json, agent logs) never reach the validator. Only the runner's stdout/stderr survives, and the crash cause lived solely in an error_text that was never printed. Production failures were opaque harbor_trial_failed shells. Emit the cause on the channel that survives: add an additive trial_diagnostics field to the result payload and one stderr line per failed trial. Redacted via LogRedactor at build and emit time. Omitted entirely when every trial succeeded, so a green job's result line stays byte-identical.
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe own-runner now reports bounded, redacted diagnostics for failed trials and agent-construction failures. Broker-mode ChangesFailure diagnostics
Broker tmpfs execution
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant OwnRunnerBackend
participant ResultSchema
participant ResultOutput
participant Stderr
OwnRunnerBackend->>ResultSchema: build_trial_diagnostics(trial_outcomes)
ResultSchema->>OwnRunnerBackend: redacted trial_diagnostics
OwnRunnerBackend->>ResultOutput: emit BASE_BENCHMARK_RESULT with diagnostics
OwnRunnerBackend->>Stderr: emit formatted diagnostic breadcrumbs
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner_backend.py`:
- Around line 1229-1241: Remove the gateway_token argument from
_emit_time_log_redactor() in
packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner_backend.py
(lines 1229-1241), leaving miner environment redaction unchanged. Also remove
the BASE_GATEWAY_TOKEN fixture/comment from
packages/challenges/agent-challenge/tests/test_own_runner_trial_diagnostics.py
(lines 183-189).
In `@packages/challenges/agent-challenge/tests/test_container_config.py`:
- Around line 255-261: Update the `/tmp` mount assertions in the test covering
`_terminal_bench_broker_limits()` to explicitly require the `rw` option in the
parsed `options` list, while preserving the existing `exec`, `noexec`, and
`nosuid` checks.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 70158b3b-c3e2-469d-bc4a-7c4ac290f3a3
📒 Files selected for processing (5)
packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner/result_schema.pypackages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner_backend.pypackages/challenges/agent-challenge/src/agent_challenge/evaluation/runner.pypackages/challenges/agent-challenge/tests/test_container_config.pypackages/challenges/agent-challenge/tests/test_own_runner_trial_diagnostics.py
| miner_values: list[str] = [] | ||
| for key in ( | ||
| "OPENROUTER_API_KEY", | ||
| "OPENAI_API_KEY", | ||
| "ANTHROPIC_API_KEY", | ||
| "HF_API_TOKEN", | ||
| ): | ||
| value = os.environ.get(key) | ||
| if value: | ||
| miner_values.append(value) | ||
| return LogRedactor( | ||
| gateway_token=os.environ.get(GATEWAY_TOKEN_ENV), | ||
| miner_env_values=miner_values, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 'GATEWAY_TOKEN_ENV\s*=' packages/challenges/agent-challenge
rg -n -C2 'BASE_GATEWAY_TOKEN|BASE_LLM_GATEWAY_URL|/llm/v1' packages/challenges/agent-challengeRepository: BaseIntelligence/base
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -a 'own_runner_backend\.py|own_runner_trial_diagnostics\.py|own_runner/redaction\.py|gateway\.py' packages/challenges/agent-challenge | sed 's#^\./##'
echo
echo "== redaction.py lines 1-80 =="
cat -n packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner/redaction.py | sed -n '1,80p'
echo
echo "== own_runner_backend import + redactor lines =="
cat -n packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner_backend.py | sed -n '48,54p;1208,1248p;1408,1442p'
echo
echo "== test lines around diagnostics and secret redaction =="
cat -n packages/challenges/agent-challenge/tests/test_own_runner_trial_diagnostics.py | sed -n '160,205p'
cat -n packages/challenges/agent-challenge/tests/test_own_runner_secret_redaction.py | sed -n '60,170p'Repository: BaseIntelligence/base
Length of output: 15637
Remove Base gateway redaction from the host-side emit path.
GATEWAY_TOKEN_ENV is BASE_GATEWAY_TOKEN, so _emit_time_log_redactor() now sources a Base LLM gateway secret into the redactor. Drop gateway_token=os.environ.get(GATEWAY_TOKEN_ENV) from packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner_backend.py#L1239, and remove the BASE_GATEWAY_TOKEN fixture/comment from packages/challenges/agent-challenge/tests/test_own_runner_trial_diagnostics.py#L187-L188.
📍 Affects 2 files
packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner_backend.py#L1229-L1241(this comment)packages/challenges/agent-challenge/tests/test_own_runner_trial_diagnostics.py#L183-L189
🤖 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
`@packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner_backend.py`
around lines 1229 - 1241, Remove the gateway_token argument from
_emit_time_log_redactor() in
packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner_backend.py
(lines 1229-1241), leaving miner environment redaction unchanged. Also remove
the BASE_GATEWAY_TOKEN fixture/comment from
packages/challenges/agent-challenge/tests/test_own_runner_trial_diagnostics.py
(lines 183-189).
Source: Coding guidelines
| limits = _terminal_bench_broker_limits() | ||
| tmp_mounts = [m for m in limits.tmpfs if m.startswith("/tmp:")] | ||
| assert tmp_mounts, "the runner must expose a writable /tmp tmpfs" | ||
| options = tmp_mounts[0].split(":", 1)[1].split(",") | ||
| assert "exec" in options, f"/tmp must be executable, got {tmp_mounts[0]}" | ||
| assert "noexec" not in options, f"/tmp must not be noexec, got {tmp_mounts[0]}" | ||
| assert "nosuid" in options, "nosuid hardening must be preserved" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that /tmp remains writable.
The test claims to require a writable /tmp, but it never checks for rw; a regression to /tmp:ro,exec,... would pass. Add an explicit rw assertion.
Suggested assertion
options = tmp_mounts[0].split(":", 1)[1].split(",")
+ assert "rw" in options, "the /tmp tmpfs must remain writable"
assert "exec" in options, f"/tmp must be executable, got {tmp_mounts[0]}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| limits = _terminal_bench_broker_limits() | |
| tmp_mounts = [m for m in limits.tmpfs if m.startswith("/tmp:")] | |
| assert tmp_mounts, "the runner must expose a writable /tmp tmpfs" | |
| options = tmp_mounts[0].split(":", 1)[1].split(",") | |
| assert "exec" in options, f"/tmp must be executable, got {tmp_mounts[0]}" | |
| assert "noexec" not in options, f"/tmp must not be noexec, got {tmp_mounts[0]}" | |
| assert "nosuid" in options, "nosuid hardening must be preserved" | |
| limits = _terminal_bench_broker_limits() | |
| tmp_mounts = [m for m in limits.tmpfs if m.startswith("/tmp:")] | |
| assert tmp_mounts, "the runner must expose a writable /tmp tmpfs" | |
| options = tmp_mounts[0].split(":", 1)[1].split(",") | |
| assert "rw" in options, "the /tmp tmpfs must remain writable" | |
| assert "exec" in options, f"/tmp must be executable, got {tmp_mounts[0]}" | |
| assert "noexec" not in options, f"/tmp must not be noexec, got {tmp_mounts[0]}" | |
| assert "nosuid" in options, "nosuid hardening must be preserved" |
🤖 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 `@packages/challenges/agent-challenge/tests/test_container_config.py` around
lines 255 - 261, Update the `/tmp` mount assertions in the test covering
`_terminal_bench_broker_limits()` to explicitly require the `rw` option in the
parsed `options` list, while preserving the existing `exec`, `noexec`, and
`nosuid` checks.
A miner constructor shells out (typically pip). The child writes to the inherited file descriptors, so Python-level redirection never sees it and CalledProcessError keeps only argv plus exit status. Production reported "returned non-zero exit status 1" while the actual reason -- "Directory '/workspace/agent' is not installable. Neither 'setup.py' nor 'pyproject.toml' found." -- reached the runner stderr only, forcing an operator to dig through task_log_events to explain a 30/30 trial wipeout. Capture fd 1 and 2 around load and construction, append a bounded one-line tail to the error, and replay both streams to their original descriptors so runner logs stay byte-complete. Capture setup failures degrade to running uncaptured rather than breaking execution.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner/driver.py`:
- Around line 358-371: Redact captured constructor output before appending it to
AgentRunResult.error in both the load_error and construction_error branches.
Reuse the existing diagnostics redactor, applying it to captured_output() before
_captured_detail interpolation, and add a regression test that supplies a secret
and verifies the returned error excludes the full value.
- Around line 106-110: Bound captured stdout and stderr in the output-capture
setup around the driver’s descriptor redirection and the later diagnostic
handling: retain only a fixed-size tail while output is written, rather than
allowing TemporaryFile storage and subsequent reads to grow without limit.
Preserve teeing output to the original descriptors and ensure the existing
600-character diagnostic limit remains effective without first buffering the
entire runner output.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a44ce7ee-d5ed-4d62-9720-e94b3cf6500e
📒 Files selected for processing (2)
packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner/driver.pypackages/challenges/agent-challenge/tests/test_own_runner_driver.py
| for fd in (1, 2): | ||
| handle = tempfile.TemporaryFile() | ||
| saved[fd] = os.dup(fd) | ||
| os.dup2(handle.fileno(), fd) | ||
| buffers[fd] = handle |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Bound captured output before it consumes runner resources.
Line 107 writes arbitrary constructor output to an unbounded temporary file, and Lines 133-145 read it all into memory. The 600-character limit at Line 159 is applied too late; a failing submission can fill temporary storage or exhaust memory before diagnostics are truncated. Retain only a bounded tail while teeing output back to the original descriptors.
🤖 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
`@packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner/driver.py`
around lines 106 - 110, Bound captured stdout and stderr in the output-capture
setup around the driver’s descriptor redirection and the later diagnostic
handling: retain only a fixed-size tail while output is written, rather than
allowing TemporaryFile storage and subsequent reads to grow without limit.
Preserve teeing output to the original descriptors and ensure the existing
600-character diagnostic limit remains effective without first buffering the
entire runner output.
| if load_error is not None: | ||
| return AgentRunResult( | ||
| status="failed", | ||
| reason_code=load_error.reason_code, | ||
| error=f"{load_error}{_captured_detail(captured_output())}", | ||
| ) | ||
| except AgentLoadError as exc: | ||
| return AgentRunResult(status="failed", reason_code=exc.reason_code, error=str(exc)) | ||
| except Exception as exc: | ||
| if construction_error is not None: | ||
| return AgentRunResult( | ||
| status="failed", | ||
| reason_code=AGENT_LOAD_FAILED_REASON_CODE, | ||
| error=f"agent construction failed: {exc}", | ||
| error=( | ||
| f"agent construction failed: {construction_error}" | ||
| f"{_captured_detail(captured_output())}" | ||
| ), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Redact captured constructor output before returning it.
Both failure paths append raw stdout/stderr to AgentRunResult.error. A constructor can print API tokens or other environment-derived secrets, which would then enter diagnostic results. Apply the diagnostics redactor before interpolation and add a regression test proving a supplied secret is absent.
As per coding guidelines, “Never log, document, or include evidence containing private keys, wallet mnemonics, API tokens, or full secret values; only names, digests, and SHAs may be exposed.”
🤖 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
`@packages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner/driver.py`
around lines 358 - 371, Redact captured constructor output before appending it
to AgentRunResult.error in both the load_error and construction_error branches.
Reuse the existing diagnostics redactor, applying it to captured_output() before
_captured_detail interpolation, and add a regression test that supplies a secret
and verifies the returned error excludes the full value.
Source: Coding guidelines
Agent construction -- unpack, install, import, instantiate -- happens per trial but does not depend on the task. A submission whose ZIP ships a bare agent.py with no installable project fails identically on every task, so a 30-task job spent hours of wall clock and real LLM budget re-deriving one packaging error, 30 times, before reporting 0. Count construction failures across trials and short-circuit the remaining ones once a small threshold confirms the package is broken. The threshold is greater than one so a single transient fault, such as a flaky package index during pip install, cannot abort an otherwise healthy job. Trials that never ran are still planned, still aggregated, and still score 0, so the totals stay honest; each carries the construction reason code and an error text naming the short-circuit and what to fix.
1 participant
Summary
terminal-bench/fix-git.Changes
1.
fix: allowexecon the runner/tmptmpfs (evaluation/runner.py)The own_runner container has a read-only rootfs, so pip installs the agent into
/tmp/.local. Docker defaults--tmpfstonoexec, so every native wheel (tiktoken, pydantic-core, numpy) failed withfailed to map segment from shared object. Addsexec, keepsnosuid. The analyzer container intentionally keepsnoexecand is untouched.2.
feat: per-trial failure diagnostics (own_runner/result_schema.py,own_runner_backend.py)The broker never bind-mounts the job directory —
_materialize_mountextracts each mount into its own workspace and copies nothing back, soresult.jsonand agent logs never reach the validator. Only stdout/stderr survives. Adds an additivetrial_diagnosticsfield plus one stderr line per failed trial, redacted throughLogRedactorat build and emit time. Omitted when every trial succeeds, so a green job's result line stays byte-identical. Legacy branch only; the attested path is untouched.Test plan
ruff check src tests→ cleanpytest packages/challenges/agent-challenge/tests→ 2305 passed, 4 skipped, 24 failed (all 24 pre-existing,submission status 'received' cannot enqueue evaluation; baseline was 2304/24 before this branch)test_terminal_bench_broker_tmpfs_allows_exec— RED before the fix (assert 'exec' in ['rw','nosuid','size=2g']), GREEN aftertest_own_runner_trial_diagnostics.py— 9 cases covering filtering, ordering, truncation, redaction, schema validation, stderr shape.sofrom/tmpfails with the exact miner error underrw,nosuid,size=2gand succeeds underrw,nosuid,exec,size=2gdna-insertran 792s and completed (legitimate 0),fix-gitscored 1.0 in 745sEvidence
.sounloadable — agent never importedfix-gitNotes
harbor_forward_env_varswas empty, soOPENROUTER_API_KEYnever entered the runner. Fixed on prod viaembed.env(CHALLENGE_HARBOR_FORWARD_ENV_VARS); nothing to merge here._tmpfs_mount_arginswarm_backend.pydrops every tmpfs option exceptsize=, so thenoexecbug would return if the Swarm path is ever enabled. Out of scope for this PR.v*tags or manual dispatch.Summary by CodeRabbit
/tmptmpfs now allows executable code (exec) while still blockingnoexecand preserving hardening (nosuid).