Skip to content

feat(terminal-bench): selectable agent harness plus full control and tracking - #20

Open
abhinav-pola wants to merge 14 commits into
mainfrom
abhinav-pola/ahead-frown
Open

feat(terminal-bench): selectable agent harness plus full control and tracking#20
abhinav-pola wants to merge 14 commits into
mainfrom
abhinav-pola/ahead-frown

Conversation

@abhinav-pola

@abhinav-pola abhinav-pola commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Why

Terminal Bench hardcoded the pi coding agent, so a result measured pi + model with no way to compare a different agent on the same tasks. This PR makes the agent selectable across the Harbor benchmarks, converges Terminal Bench onto the Harbor sandbox it should always have used, and fills in the control knobs and result columns that were missing.

Twelve commits, each reviewable on its own.

Agent selection

Every agent is launched through an Ori Harness passthrough command (ori pi, ori claude). Ori is not a proxy: it injects OpenRouter auth and execs the real CLI, resolving credentials from the OPENROUTER_API_KEY the sandbox already receives, and installs as a standalone Linux binary.

benchmark agent values default
terminal_bench pi, claude pi
swe_atlas_{qa,tw,rf}, deep_swe mini_swe, pi, claude mini_swe

mini_swe is the pre-existing in-harness loop, named for what it is — a mini-swe-agent scaffold (MINI_SWE_SYSTEM_MESSAGE, one bash tool, COMPLETE_TASK_AND_SUBMIT_FINAL_OUTPUT sentinel). It was briefly called native in this branch, which described nothing and became actively misleading once the genuinely native pi path was retired.

wandr is excluded: it injects OpenRouter server tools into its own requests, which an external CLI replaces with its own, so it would stop measuring what it is for.

bun src/cli/index.ts --benchmark terminal_bench --model anthropic/claude-opus-5 \
  --solver-config '{"agent":"claude","taskSubset":["fix-git"]}'

bun src/cli/index.ts --benchmark deep_swe --model anthropic/claude-opus-5 \
  --solver-config '{"agent":"pi","taskSubset":["abs-stepped-slices"]}'

Control

One agentReasoningEffort (nonemax) goes to ori --reasoning-effort, which each harness translates natively (pi --thinking, claude --effort). Verified on stable ori: none yields 0 reasoning tokens, high yields 58.

Also available on every agent: systemPrompt, appendSystemPrompt, allowedTools, disallowedTools, isolateAgentConfig, agentPackage, oriChannel.

The pi-only thinking and claude-only effort fields are removed, not aliased. An alias took precedence over the new field, so a config carrying both — likely on replay, since thinking was defaulted on main and the whole validated config is stored in the parquet benchmark_config column — would silently run at the legacy level.

Removing them alone would only have relocated the hazard, because zod strips unknown keys: {"thinking":"high"} was silently discarded, and so was a typo like agentReasoningEfort while the real field quietly took its default. buildSchemaValidatedConfig now rejects any solver-config key absent from the benchmark's own options schema or the shared model base, naming the offenders. This gives BENCHMARK_OPTIONS_SCHEMAS its first runtime consumer and covers new benchmarks automatically.

Tracking

generation_ids resolves against the OpenRouter generations API for either agent, so spend is reconcilable from stored results.

field pi claude
generation_ids responseId message.id
cost usage.cost.total total_cost_usd
reasoning tokens usage.reasoning thinking_tokens
generation_time_ms wall-clock duration_ms
agentTurns / agentToolCalls event counts num_turns / tool_use blocks
response_items stream events stream events

ori-parquet.test.ts runs the solver through the real parquet writer, reads the file back, and asserts no unexpectedly null columns. request_body, extra_scores and primary_score stay null: there is no single request body for an external CLI, and terminal_bench is binary pass/fail so accuracy already is the score.

Session attribution goes out as X-Session-Id via ORI_OPENROUTER_SESSION_ID, confirmed through a local TLS-terminating proxy on ori 0.7.1. Ori replaces a value containing a control character with a fresh UUID silently, which would detach a run from its generations, so both the CLI and the solver reject such a value rather than letting attribution vanish.

Harbor convergence, and a dropped-settings bug

Terminal Bench is a Harbor benchmark — its tasks come from harbor-framework/terminal-bench-2-1 and its task.toml is the format deep-swe parses — but it carried a parallel reimplementation of the harbor sandbox that silently dropped per-task settings.

TaskTomlSchema requires cpus, memory_mb and gpus, but the dataset never read them and the local Modal layer passed only a timeout, workdir and command. Every task ran at Modal's defaults: 22 of 90 request more than 2048 MB and 6 request more than 1 CPU. An under-provisioned task OOMs or thrashes and scores 0, which reads as model failure.

terminal-bench/sandbox.ts and terminal-bench/modal-sandbox.ts are deleted. session.ts builds harbor's CreateSessionInput with the declared resources and the task files as uploads, and runTerminalBenchVerifier replaces the sandbox-owned runTests(). Reward parsing is unchanged in practice: all 90 tasks write echo 1 or echo 0, so harbor's parseReward and the old strict === "1" agree on this dataset.

Running as root is correct here

Claude Code refuses --dangerously-skip-permissions as root, and Modal sandboxes run as root, so the first real run of the claude path failed every task before the agent did any work — zero tokens, zero cost, reward 0. The run script sets IS_SANDBOX=1, after which the same task completed cleanly.

Anthropic documents running Claude Code as a non-root user in containers. That is the wrong answer for this benchmark: across the 90 tasks, 122 shell files run root-requiring commands (114 apt-get install, 27 pip install, 2 useradd, 1 systemctl restart). A non-root agent could not install packages and would fail those tasks for environmental reasons; restoring the ability with passwordless sudo trips the identical refusal. The check exists to stop an agent wrecking a real system, and this is a disposable single-task container where root is the task.

Residual risk is that IS_SANDBOX is undocumented (anthropics/claude-code#58150 asks for it to be specified). If it stops working the failure is loud rather than silent — exit 1 with the exact message captured in testOutput and agentExitCode in metadata.

Network policy on the CLI path

The agent runs inside the sandbox and must reach OpenRouter. deep_swe declares allow_internet = false by default, which the Modal layer maps to blockNetwork: true, so a CLI agent there would have failed every task with no route to the model. The agent session now gets egress when a CLI agent is selected; where the verifier has its own sandbox (deep-swe) it keeps the task's declared policy so scoring stays offline. swe-atlas and terminal-bench share one sandbox between agent and verifier, so egress covers both — moot in practice, since swe-atlas defaults to true and all 90 terminal-bench tasks declare true. When the policy is overridden, agentNetworkForced and taskAllowInternet land in sample metadata.

The grading contract reaches the agent

swe_atlas and deep_swe encode how work is handed in inside buildInstanceMessage, not in instruction.md: qa must write /logs/agent/answer.txt in <<FINAL_ANSWER>> tags, tw must write /logs/agent/manifest.txt in <<TEST_MANIFEST>> tags, rf starts at / and must locate the repo root without touching test files, and deep-swe is graded from git commits with uncommitted changes discarded. The CLI path uploaded only the raw instruction, so an agent could do the work correctly and score zero. Each benchmark now passes a CLI-appropriate variant of its contract as the appended system prompt, preserving any caller-supplied prompt alongside.

Real Modal runs

agent exit gen_ids turns cost reasoning time_ms
pi 137 224 223 $1.203 88 780,168
claude (before IS_SANDBOX) 1 0 $0.000 0 2,048
claude (after) 0 8 15 $0.376 757 38,404

pi's 0 is honest: it used its full 750s budget and was SIGKILLed, the timeout firing 168ms past the configured bound. claude's 0 is a clean completion that did not solve the task. Cost reconciliation checks out to the digit — pi's reported per-generation cost matched OpenRouter's billing on every sampled generation — and prompt caching is working (99.98% cache reads on the pi run).

Behavior changes

  • Honoring declared resources will move scores on the 28 tasks requesting more than default cpus or memory. Presumably upward, but not comparable to previous runs.
  • terminalBenchScorer emits a verifier_log trajectory, filling scorer_trajectory for both agents. Additive; score values unchanged.
  • pi parquet columns change value: generation_time_ms and reasoning_tokens from hardcoded 0 to real numbers, response_items from null to the full event stream, so files get larger.
  • CLI-path results measure a different system than mini_swe and are not comparable to existing harbor baselines. mini_swe remains the default everywhere it existed.
  • A pi request failing at the API used to exit 0 with zero usage, indistinguishable from a cheap successful run. It is now surfaced. Past pi runs showing reward 0 with zero cost are worth re-checking.

isolateAgentConfig defaults to false, preserving current pi behavior. With the default, pi reads AGENTS.md/CLAUDE.md from the task working directory, so task-shipped files can influence the agent; the knob exists to rule that out, but flipping the default would move scores.

Breaking change

./benchmarks/terminal-bench/sandbox and ./benchmarks/terminal-bench/modal-sandbox, added in #22, are removed — the convergence deletes those modules. ./benchmarks/harbor/sandbox and ./benchmarks/harbor/modal-sandbox are exported in their place, plus ./benchmarks/agent-cli/schema for the agent enums. A re-export shim was rejected: the interfaces genuinely differ (runTests() and a task-file triple versus uploads, resources and attach), so it could not be type-compatible.

Verification

All five gate commands exit 0: format:check, check, typecheck, bun test (1321 pass, 0 fail), build. Rebased on current main.

Flags and parsers were checked against the real CLIs, not just fixtures: --reasoning-effort verified to change reasoning-token counts, parsers run against captured success and failure streams from both agents, generation ids resolved against /api/v1/generation, generated run scripts pass bash -n in minimal and all-knobs-on forms, and a dataset test walks all 90 real task files asserting every declared cpus and memory survives into metadata.

Known gaps

  • Response caching is not applied on the CLI path. mini_swe gets it through responses-client.ts; agent CLIs make their own requests. fix(response-cache): send cache salt as header #36 moved the salt from a body field to x-openrouter-cache-salt, so all three pieces are now headers and injection through pi's models.json headers has become feasible — not done here.
  • swe_atlas and deep_swe have never run on Modal. Both the network fix and the grading-contract fix were written for them and are unit-tested only. Given that this branch's two worst bugs were findable only by running, those paths should get one real task each before their numbers are trusted.
  • stepLimit is inert on the CLI path. Neither CLI exposes a turn limit, so wall-clock is the only bound. One haiku task cost $1.20 over 223 turns; a 90-task sweep is $100+ on haiku alone.
  • oriInstallUrl and agentPackage are interpolated into Dockerfile RUN steps without validation, as flagged by Devin Review. Config reaches them only via --solver-config from a caller who already has a shell, so this is hardening rather than a live boundary; deliberately left out by decision.
  • Claude's X-Session-Id forwarding is unconfirmed. Claude Code ignores HTTP_PROXY, so it bypassed the observation method that confirmed pi.

🤖 Generated with Claude Code

@abhinav-pola
abhinav-pola requested a review from a team as a code owner August 12, 2026 01:18
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@abhinav-pola
abhinav-pola force-pushed the abhinav-pola/ahead-frown branch from a529816 to eb148b5 Compare August 14, 2026 19:33
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

Abhinav Pola and others added 12 commits August 15, 2026 18:44
…tracking

Terminal Bench hardcoded the `pi` coding agent, so a result measured
pi-plus-model with no way to compare a different agent on the same tasks.
Add a selectable agent and fill in the control knobs and result columns
both paths were missing.

Agent selection:

- `agent: "pi" | "claude"`, defaulting to `pi`. The pi path keeps its
  direct `pi --print --mode json` invocation, so existing runs are
  unchanged.
- `claude` runs Claude Code inside the sandbox through Ori Harness
  (`ori claude`), which injects OpenRouter auth from the
  `OPENROUTER_API_KEY` the sandbox already receives. ori installs as a
  standalone Linux binary; no extra runtime is needed.

Control, now available for both agents:

- reasoning level: `thinking` (pi `--thinking`, gains `max`) and
  `effort` (claude `--effort`). Separate enums because claude has no
  `off`/`minimal`.
- `systemPrompt` override, `allowedTools`, `disallowedTools`.
- `isolateAgentConfig` disables extension, skill, prompt-template and
  context-file discovery for reproducible runs. Defaults to `false` to
  preserve current pi behavior.

Tracking. `generation_ids` now resolves against the OpenRouter
generations API for either agent, so spend is reconcilable from stored
results:

- pi: `responseId` per assistant message, `usage.reasoning` for
  reasoning tokens, wall-clock `generationTimeMs` (was hardcoded 0),
  and stream events as `responseItems` (was null).
- claude: `message.id`, `total_cost_usd`, `thinking_tokens`,
  `duration_ms`, `num_turns`, reconstructed assistant turns.
- both: turn and tool-call counts, plus `agent`, `agentExitCode` and
  `agentIsError` in sample metadata.

A pi request that fails at the API (for example an invalid model id)
exits 0 with empty content and zero usage, which was indistinguishable
from a cheap successful run. `stopReason: "error"` and `errorMessage`
are now collected and surfaced in the verifier output.

`terminalBenchScorer` emits a `verifier_log` trajectory, filling
`scorer_trajectory` for both agents. Score values are unchanged; no
score movement is expected from any change here.

Prompts and tool lists pass through env vars rather than argv to avoid
shell quoting issues. Flags were verified against pi 0.84.1 and Claude
Code 2.1.228, and parsers against captured real streams from both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Terminal Bench is a Harbor benchmark - its tasks come from
harbor-framework/terminal-bench-2-1 and its task.toml is the same format
deep-swe parses - but it carried a parallel reimplementation of the
harbor sandbox. That divergence silently dropped per-task settings.

`TaskTomlSchema` requires `cpus`, `memory_mb` and `gpus` and defaults
`allow_internet`, but the dataset never read them, the local
`CreateSessionInput` had no fields for them, and the local Modal layer
passed only a timeout, workdir and command. Every task therefore ran at
Modal's defaults: 22 of 90 tasks request more than 2048 MB and 6 request
more than 1 CPU. An under-provisioned task does not fail cleanly, it
OOMs or thrashes and scores 0, which reads as model failure.

Delete `terminal-bench/sandbox.ts` and `terminal-bench/modal-sandbox.ts`
and move both solvers onto `harbor/sandbox.ts`:

- `session.ts` builds harbor's `CreateSessionInput`, passing the task's
  declared cpus, memory and network policy, with the tests directory and
  instruction as `uploads`.
- `runTerminalBenchVerifier` replaces the sandbox-owned `runTests()`,
  matching how swe-atlas verifies, and uses harbor's `parseReward`.
- The Modal layer is harbor's, with the terminal-bench app name.

Reward parsing is unchanged in practice: every one of the 90 tasks writes
`echo 1` or `echo 0` to reward.txt, so harbor's `parseReward` and the old
strict `=== "1"` agree on this dataset.

Terminal Bench also gains `attach()`, which checkpoint and resume would
need later.

Expected score change: tasks that declare more than the default cpus or
memory were under-provisioned and are now given what they ask for. Scores
on those 28 tasks may move, presumably upward, and are not comparable to
previous runs. Nothing else here alters scoring.

Test fakes live in `test/helpers/terminal-bench-sandbox.ts` over harbor's
callback fake, so no test-only code sits in src. A dataset test walks all
90 real task files and asserts every declared cpus and memory survives
into sample metadata.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With terminal-bench on the harbor sandbox, the agent-CLI machinery it grew
is no longer terminal-bench specific. Lift it into `agent-cli/` and let the
other harbor benchmarks select it.

- `agent-cli/schema.ts` owns the agent enums, effort levels and package
  defaults. `terminal-bench/schema` re-exports what it previously declared,
  so the published `benchmarks/terminal-bench/schema` entry point is
  unchanged.
- `agent-cli/harness.ts` is the former `terminal-bench/ori-harness.ts`, with
  the instruction and log paths now parameters rather than constants, since
  each benchmark mounts its instruction elsewhere.
- `agent-cli/runner.ts` holds the shared exec, stream parse, generation-id
  recording, failure summary and metadata shaping. `terminal-bench/ori-solver`
  drops to a thin wrapper over it.

swe_atlas (all three tracks) and deep_swe gain `agent`, defaulting to
`native`, which is the existing `runAgentLoop` path. Selecting `claude`
installs the agent into the task image and runs it in place of the loop;
each benchmark keeps its own verifier, so swe-atlas still verifies in the
agent sandbox and deep-swe still extracts a patch and verifies in a second
one. The shared control knobs (effort, system prompt, tool allow and deny
lists, config isolation) come along through `AgenticOptionsSchema`.

deep-swe additionally uploads its instruction to /instruction.md on the CLI
path, because the native path passes the prompt in memory and never needed a
file in the sandbox.

Two capability gaps on the CLI path, both inherent to driving an external
process rather than the loop:

- `stepLimit` has no equivalent. Neither pi 0.84.1 nor Claude Code 2.1.228
  exposes a turn limit, so the wall-clock agent timeout is the only bound.
- deep-swe checkpoint resume is skipped. A `claude -p` process cannot be
  resumed from step N, so the CLI path always starts a fresh sandbox rather
  than attaching to a checkpointed one.

wandr is deliberately excluded: it injects OpenRouter server tools into its
own requests, which an external CLI replaces with its own tools, so the
benchmark would no longer measure what it is for.

Results from the CLI path measure a different system than `runAgentLoop` and
are not comparable to existing harbor baselines. The native default keeps
current numbers reproducible; tests assert the native path still builds no
image steps and still drives the model loop.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ori 0.7.0 ships `ori pi` as a passthrough launcher alongside claude,
codex and opencode, plus a single `--reasoning-effort` flag that each
harness translates natively:

  ori pi     --reasoning-effort high  -> pi --thinking high (none -> off)
  ori claude --reasoning-effort high  -> claude --effort high

This removes the reason pi was invoked directly. The earlier blocker was
that pi could only be reached through `ori code`, which exposes no
reasoning control and strips the `model:thinking` suffix. A passthrough
has no such limit: everything after `--` reaches pi untouched, so pi's
native JSON stream, and therefore `responseId`, `usage.reasoning` and
`usage.cost.total`, all survive.

Verified against the stable 0.7.0 binary, not the alpha: `--reasoning-effort
none` yields 0 reasoning tokens and `high` yields 58, and the generated run
script produces a stream this parser reads end to end, including a
generation id that resolves against the generations API.

- Retire the native pi solver, `pi-custom-models`, and the `--provider` and
  models.json plumbing. ori provisions pi's config itself, confirmed from a
  clean HOME, so none of it is needed.
- `agent` is now `pi | claude` for terminal-bench and `native | pi | claude`
  for the harbor benchmarks, all ori-launched. deep_swe and swe_atlas can
  now run pi as well, which was not previously reachable.
- Replace the pi-only `thinking` and claude-only `effort` fields with one
  `agentReasoningEffort` over ori's enum (none through max). `thinking` and
  `piPackage` stay accepted as optional legacy aliases and are mapped, so
  existing --solver-config invocations keep working; `off` maps to `none`.
- Add `oriChannel` (stable by default) since install.sh honours
  ORI_CHANNEL, so a run can opt into a pre-release ori when a fix is needed
  before it reaches stable.

`agentReasoningEffort` is deliberately not named `reasoningEffort`: that
name already exists on `InferenceOverrideSchema` for the model-level
override used by the native harbor loop, and spreading both into one config
would have shadowed it.

Note that `bun run typecheck` accepted several of these type errors while
`bun run build` caught them, so the gate here was checked per-command exit
code rather than by reading output.

Score impact: terminal-bench's only pi path now goes through ori, which
provisions pi's config differently than the models.json this used to write.
Scores may move and are not comparable to previous pi runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The harbor agent selector shipped with `native` as the name for the
in-harness loop. That name was invented for the selector and describes
nothing: the loop is a mini-swe-agent scaffold, which the surrounding code
already says. `harbor/prompts.ts` exports `MINI_SWE_SYSTEM_MESSAGE`, passed
as `instructions` by both swe-atlas and deep-swe, and the loop matches
mini-swe-agent's contract - one `bash` tool, `COMPLETE_TASK_AND_SUBMIT_
FINAL_OUTPUT` as the submit sentinel, head and tail observation elision.

`native` was also actively misleading after the previous commit removed the
one genuinely native path, leaving `native` as the only agent that is not
native to anything.

`mini_swe` names the scaffold and reads correctly beside `pi` and `claude`
as one of three. No alias: `native` only ever existed on this branch, so
nothing depends on it.

`DEFAULT_HARBOR_AGENT` keeps the same meaning, so no behavior changes. The
unrelated `native` values elsewhere - draco's search backend, the search
lane engine literal, and `WebSearchEngine.Native` - are untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two review findings, both real.

The agent CLI runs inside the sandbox and must reach OpenRouter, but the
sandbox was created with the task's declared network policy. deep-swe
declares `allow_internet = false` by default, which the Modal layer maps to
`blockNetwork: true`, so selecting a CLI agent there would have failed every
task with no route to the model. The native loop is unaffected because it
calls the model from the host.

The agent session now gets egress whenever a CLI agent is selected. Where
the verifier has its own sandbox, as in deep-swe, it keeps the task's
declared policy so scoring stays offline. swe-atlas and terminal-bench run
the agent and verifier in one sandbox, so egress necessarily covers both
there; swe-atlas tasks declare `allow_internet = true` and all 90
terminal-bench tasks do as well, so nothing changes for them in practice.
When the policy is overridden, `agentNetworkForced` and `taskAllowInternet`
are recorded in sample metadata rather than the deviation being silent.

`runAgentCli` also never measured elapsed time, so pi runs reported
`generation_time_ms` as 0. The wall-clock timing existed in the native pi
solver and was lost when that solver was retired, which left the PR
description claiming a value the code no longer produced. The exec is now
timed and that value is used whenever a harness parser reports no duration
of its own; claude still prefers its reported `duration_ms`.

The remaining review finding, unvalidated `agentPackage` and `oriInstallUrl`
interpolated into image build steps, is intentionally not addressed here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main added `./benchmarks/terminal-bench/sandbox` and
`./benchmarks/terminal-bench/modal-sandbox` in #22, and this branch deletes
both modules when converging terminal-bench onto harbor, so those entry
points resolved to files that no longer exist.

Remove them and export `./benchmarks/harbor/sandbox` and
`./benchmarks/harbor/modal-sandbox` instead, which is what terminal-bench
now uses and therefore what an external consumer wants. Also export
`./benchmarks/agent-cli/schema` so the agent and reasoning-effort enums are
reachable when building a run config.

This is a breaking change for anything importing the two removed paths.
A re-export shim was considered and rejected: the two sandbox interfaces
differ, since terminal-bench's carried `runTests()` and a task-file triple
while harbor's carries `uploads`, resources and `attach`, so a shim could
not be type-compatible and would misrepresent what it wraps.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…del id

Two more review findings, both real.

The submission protocol was unreachable on the CLI path. swe-atlas and
deep-swe encode how work is handed in inside `buildInstanceMessage`, not in
`instruction.md`: qa must write /logs/agent/answer.txt wrapped in
<<FINAL_ANSWER>> tags, tw must write /logs/agent/manifest.txt with
<<TEST_MANIFEST>> tags, rf starts at / and must locate the repository root
without touching test files, and deep-swe is graded from git commits with
uncommitted changes discarded. The CLI branches uploaded the raw instruction
and nothing else, so an agent could do the work correctly and still score
zero because it never produced the artifact the verifier reads. That would
have made any comparison against the mini_swe loop meaningless.

Each benchmark now composes a CLI-appropriate variant of its contract and
passes it as the appended system prompt, which is where harness-level
guidance belongs, leaving `instruction.md` as the task text. The variants
drop the bash-tool ceremony and the submit sentinel, since a CLI agent has
its own tools and finishes on its own. A caller-supplied appendSystemPrompt
is preserved alongside rather than overwritten.

The model id also changed meaning silently. The retired solver's `parseModel`
required "provider/model" and stripped a leading `openrouter/` segment before
handing the id to pi, so callers passed `openrouter/anthropic/claude-...`.
The new path forwarded the config value verbatim to `ori <agent> --model`,
turning a legacy config into a request for a model that does not exist and a
zero-scoring run rather than a clear error. `normalizeAgentModel` restores
the old semantics exactly, including leaving router ids such as
`openrouter/auto` intact, which the previous code special-cased.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first real Modal run of the claude path failed every task before the
agent did any work:

  claude exited 1
  --dangerously-skip-permissions cannot be used with root/sudo privileges
  for security reasons

`--permission-mode bypassPermissions` resolves to
`--dangerously-skip-permissions`, which Claude Code refuses to honour as
root, and the sandbox runs as root. The run recorded zero tokens, zero cost
and reward 0. No unit test could have caught this: a fake sandbox has no uid.

Set `IS_SANDBOX=1` for the claude invocation, which is the escape the
container is entitled to, and re-ran to confirm: exit 0, 15 turns, 14 tool
calls, 8 generation ids, 757 reasoning tokens, $0.376, and every parquet
column populated. A stored generation id resolves against the generations
API with `origin: https://claude.ai/code`, so cost reconciliation works on
this path as it does for pi.

This is knowingly an interim fix. Anthropic's documented approach for
containers is to run Claude Code as a non-root user, and `IS_SANDBOX` is
undocumented, with anthropics/claude-code#58150 open asking for it to be
specified. Switching to a non-root user means adding a user to the task image
and granting it write access to the workdir, which risks tasks whose
verifiers assume root-owned files, so it wants its own change validated by
real runs rather than being bundled here. Until then the choice is between an
undocumented environment variable and a claude path that cannot start at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The CLI path set `OPENROUTER_SESSION_ID`, which nothing consumes. ori's
binary references `ORI_OPENROUTER_SESSION_ID` and substitutes a fresh UUID
when it is absent, so every run was being attributed to a random session
rather than the benchmark run.

This also lost ground relative to the retired native pi path, which injected
`x-session-id` explicitly through the `headers` field of the models.json it
wrote. That mechanism was removable because ori provisions pi's config
itself, but it was verifiable in a way the environment variable is not.

Rename to the variable ori actually reads. Not yet confirmed end to end:
`ori pi` logs no session mapping at the default log level, and the
generations API exposes no session field, so there is no external signal that
the id reaches OpenRouter. If attribution by run matters, that wants either
confirmation from the ori side or an explicit header injection for pi.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`thinking` was pi-specific, existing only because the retired native solver
passed it as `pi --thinking`. It was superseded by `agentReasoningEffort`,
which ori translates per harness, and kept only as an alias. Review pointed
out the alias took precedence over the new field, so a config carrying both -
likely on replay, since `thinking` was defaulted on main and the whole
validated config is stored in the parquet `benchmark_config` column - would
run at the legacy level and silently ignore the requested one.

There are no callers, so `thinking` and `piPackage` are removed outright
along with `PI_THINKING_LEVELS`, `DEFAULT_PI_THINKING` and
`DEFAULT_PI_PACKAGE`, rather than the precedence being reordered.

Removing them alone would only have relocated the hazard. zod strips unknown
keys, so `--solver-config '{"thinking":"high"}'` was silently discarded, and
so was a typo: `agentReasoningEfort` vanished while `agentReasoningEffort`
quietly took its default. Either way a run executes at a reasoning level the
caller did not ask for, and the results look valid.

`buildSchemaValidatedConfig` now rejects any solver-config key that is not in
the benchmark's own options schema or the shared model base, naming the
offenders. This finally gives `BENCHMARK_OPTIONS_SCHEMAS` a runtime consumer;
it previously existed only as a compile-time exhaustiveness table.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Confirmed against ori 0.7.1: exporting `ORI_OPENROUTER_SESSION_ID` sends the
value verbatim as `X-Session-Id` on the inference request. Captured through a
local TLS-terminating proxy on `POST /api/v1/responses`:

  x-session-id: bench-session-verify-99

An earlier check of this reported the header absent. That measurement was
taken against 0.7.0+f411e1a, where it genuinely was not sent; the forwarding
landed in 0.7.1. The env var this code already sets is therefore correct, and
nothing about the wiring needed to change. Note the capability requires ori
>= 0.7.1, which the sandbox gets since install.sh always fetches current
stable.

What did need changing is the failure mode. ori replaces a value containing a
newline or other control character with a fresh UUID, silently, so the run
would be recorded under one session id while its generations were attributed
to another - the kind of mismatch that is invisible until someone tries to
reconcile spend. `sessionId` originates from `BENCH_CHILD_WORKFLOW_ID`, which
is operator-supplied and exactly the sort of value that carries a stray
newline.

The CLI now refuses such a value up front, before any Modal spend, and
`runAgentCli` fails the sample for callers that reach it through the library
entry point instead. The check walks code points rather than using a regex,
since matching control characters trips `no-control-regex`.

Verified for pi only. Claude Code does not honour HTTP_PROXY, so its traffic
bypasses this method of observation and its header remains unconfirmed here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@abhinav-pola
abhinav-pola force-pushed the abhinav-pola/ahead-frown branch from 68cb5b6 to b944de4 Compare August 16, 2026 01:45
devin-ai-integration[bot]

This comment was marked as resolved.

Two review findings, both introduced by this branch.

deep-swe keeps the agent sandbox alive across an interrupt so the next
attempt can reattach from a checkpoint. On the CLI-agent path neither half of
that holds: attach is disabled by construction, and no checkpoint is ever
written because `onCheckpoint` is wired only into `runAgentLoop`. The retained
sandbox therefore had no id anyone could attach to and sat billing until
`maxAgentTimeoutSec + 300` elapsed. It is now retained only when it is
actually reattachable, so a cancelled CLI run tears down immediately.

`BENCH_CHILD_WORKFLOW_ID` set to an empty string reached the new session-id
guard as `""`, which is not a valid session id, so the command aborted
claiming a control character that was not there. Empty now means unset and
falls back to a generated id, which is also what the code did before the
guard existed. `runAgentCli` likewise omits an empty value rather than failing
the sample.

The remaining finding, that the description promised `thinking` and
`piPackage` as mapped aliases while the code removes them, was a stale claim
in the pull request body rather than in the code. The body has been rewritten
to state the removal, since there are no callers and a loud rejection beats a
silently ignored key.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

The grading tests were copied into the agent's sandbox before the agent
started, so an autonomous agent could rewrite or short-circuit them and be
scored correct without solving the task. Terminal Bench and swe-atlas were
both exposed.

Upstream Harbor does not do this. `SingleStepTrial._run` runs the agent, then
the verifier, and the tests are uploaded inside `Verifier.verify()`, so they
reach the environment only at verification time. That holds even in
`VerifierEnvironmentMode.SHARED`, where the verifier reuses the agent's
container; `skip_tests_upload` exists for images that bake tests in, as an
explicit opt-in. So this was a divergence in this repo, not upstream
behaviour.

It also predates this branch: the deleted `uploadTaskFiles` copied
`instruction.md`, `test.sh` and the whole tests directory at sandbox creation,
and the harbor convergence carried that faithfully. deep-swe was already
correct, since it verifies in a separate sandbox the agent never touches.

Both benchmarks now create the sandbox with only the instruction and upload
the tests from the verifier, which required exposing `uploadDir` on harbor's
`SandboxSessionInstance` alongside the existing `uploadFile`; the directory
walk already existed for create-time uploads.

Only 1 of the 90 terminal-bench task instructions references /tests at all,
so almost no task has reason for the agent to see them. swe-atlas's rf track
compensated in the prompt - "Do NOT modify any test files... The verifier will
reject your trial if test files are touched" - which was a prompt-level
mitigation for exactly this hole and is now backed by the tests being absent.

Expected score change: any task where an agent was previously reading or
editing the tests can now score differently, which is the point. That applies
to the pi path as much as the CLI agents.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment thread src/benchmarks/agent-cli/harness.ts
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.

1 participant