feat(tb): Terminal-Bench container integration + infrastructure hardening - #90
Closed
Zlatanwic wants to merge 20 commits into
Closed
feat(tb): Terminal-Bench container integration + infrastructure hardening#90Zlatanwic wants to merge 20 commits into
Zlatanwic wants to merge 20 commits into
Conversation
The pass-1 compiler agent rewrites SKILL.md wholesale and, despite prompt instructions, sometimes drops or mutates original fenced code blocks. The guard (correctly) rejects this, marking compilations guard=FAIL. Make code-block preservation structural instead of advisory: mask every fenced block as an atomic [[SKVM_CODE_BLOCK_N]] placeholder before the agent sees the skill, then restore verbatim afterward. The agent edits prose only; original code survives byte-for-byte, so guard check SJTU-IPADS#2 passes by construction. A safety net re-attaches any block whose placeholder the agent deleted outright. Verified on 5 terminal-bench skills: pre-fix 2/5 guard=FAIL (sqlite, gcode dropped original code blocks); post-fix 5/5 guard=PASS with zero code blocks lost.
- tb2.1-skills/: 89 Claude skills distilled from Terminal-Bench 2.1 tasks, the skill dataset for the SkVM compilation benchmark. - plan/: experiment status report documenting the Harbor-based Docker evaluation (noskill/original/aot conditions, terminus-2 + deepseek-v4-flash). - .gitignore: exclude temp/ (harbor jobs, env files with API keys, compiled artifacts) and terminal-bench-2-1/ (independent 3rd-party git repo clone).
Document the execution-log JIT path validated on gcode-to-text: - Convert Harbor failure jobs (reward.txt + verifier stdout + agent pane) into SkVM execution-log evidence (Simple JSON report format). - skvm jit-optimize --task-source=log consumes the evidence without rerunning tasks; the optimizer diagnosed the real bottleneck (OCR fails on single-stroke fonts + no verification gate + trusting G-code comments) from two failure traces — vs AOT which flagged an irrelevant rendering gap. - Harbor re-validation (verifier ran clean) confirmed the JIT diagnosis: OCR output was gibberish on thin-stroke text exactly as diagnosed, and the JIT-added verification gate prevented the agent from writing garbage to /app/out.txt (the original condition wrote a single '}'). - reward stayed 0: gcode is a capability-ceiling task (geometric letter recognition exceeds deepseek-v4-flash), not an instruction problem. The JIT loop's behavior-guidance succeeded; the ceiling is model capability. Adds plan/harbor-to-evidence.py as the reusable C2 converter.
Adapters' tierGlobal resolvers feed `which <name>` output straight back as cmd[0]. Under Git Bash / MSYS that returns MSYS drive paths (`/d/...`) which Bun.spawn cannot resolve, producing ENOENT uv_spawn. Convert `/x/...` to `X:/...` (trying .exe for bun-compiled binaries like node_modules/.bin/pi) on win32; no-op elsewhere and for bare PATH names.
The subprocess pi adapter used renderPiBaseUrlOverride, which writes only
the baseUrl override. pi's openai provider then defaults custom models to
openai-responses (POST {baseUrl}/responses); non-OpenAI backends
(DeepSeek, vLLM, any OpenAI proxy) don't implement Responses and return
404. Switch managed mode to renderPiModelRegistration, which registers the
model with api: 'openai-completions' so pi uses /chat/completions —
matching the headless library driver that already did this correctly.
Discovered running the pi adapter against DeepSeek via skvm run.
Import Terminal-Bench 2.1 tasks into skvm-data/tasks via `skvm bench --import=terminalbench --path=<tb-repo>`. Converts task.toml + instruction.md into a skvm task.json: instruction -> prompt (with /app/ rewritten to ./ and a guardrail telling the agent not to run the container-only verifier), docker_image -> tbDockerImage + evaluator payload, tests/ copied with CRLF->LF normalisation. Extends BenchTaskFileSchema with optional tbDockerImage / tbTestsDir so TB metadata is visible at the task level; the tb-grade evaluator (next commit) reads the same data from the custom criterion payload since CustomEvalContext has no task object. Loader and skvm run path transparently pass the new fields through.
Custom evaluator that runs a TB task's tests/test.sh INSIDE the task's docker image and reads /logs/verifier/reward.txt (0/1) -> score 0.0/1.0. One container per run, created and torn down within a single run() call (finally block) so errored tasks don't leak containers. Architecture (validated by path-C pilot): the agent ran on the host against workDir; this evaluator mounts that same workDir back into the image at /app and the LF-normalised tests/ at /tests:ro, then docker exec bash /tests/test.sh. Reads dockerImage/testsDir/verifierTimeoutSec from criterion.payload (written by the terminalbench importer). Pre-flight docker daemon probe + MSYS_NO_PATHCONV for Git Bash path safety.
For Terminal-Bench tasks (tbDockerImage set), copy the image's /app contents into the host workDir before the agent runs, so the host-side agent can read the task's actual files (e.g. filter.py). The tb-grade evaluator later mounts this same workDir back into the image at /app, so the agent's edits land in the container faithfully. No-op for non-TB tasks. Without this, the agent runs in an empty workDir, can't find the task files referenced in the prompt, and spins until OOM. Uses a throwaway docker run --rm with MSYS_NO_PATHCONV for Git Bash path safety.
runSubprocess's timeout only called proc.kill() (SIGTERM to the direct child). Adapter wrappers (pi.exe, opencode) spawn grandchildren that survive and keep the stdout pipe open, so proc.exited / Response.text() never resolve and the timeout never actually fires — a stuck agent hangs the whole bench run (observed: pi on tb-adaptive-rejection-sampler ran 11min past a 600s timeout, bun at 4.65GB, zero tasks completed). Two fixes: - killProcessTree: taskkill /T /F on Windows (SIGKILL the group elsewhere) to take down the wrapper AND its descendants. - Replace Response(stream).text() with cancellable reader.read() loops; on timeout we cancel the readers so runSubprocess returns promptly with partial output instead of blocking on a pipe held open by a killed process's orphaned grandchild. Verified: sleep 30 with 2s timeout now returns in ~2.6s (was 30s).
Long TB tasks (tb-circuit-fibsqrt, tb-feal-*-cryptanalysis) crashed with
RangeError: Out of memory. Root cause: piEventsToRunRecord received the
full PiEvent[] from parsePiNDJSON, which materialized every one of pi's
~30k emitted events (message_update / thinking / *_delta streaming
deltas make up ~99.9% of a 0.3-1.7 GB transcript). The builder only
consumes agent_end + message_end (~25 events); the other 30k parsed
objects sat in a retained array alongside the buffered stdout string,
driving peak heap to 10-32 GB.
Add piBuildRunRecordFromNDJSON: scan the stdout string with charCodeAt
(no split() array), pre-filter each line with .includes('"type":"agent_end"')
or '"type":"message_end"' to skip JSON.parse on 99.9% of lines, and
retain only the two event types the builder actually reads. Behaviorally
equivalent to the old path — a dedicated equivalence test compares
parsePiNDJSON+piEventsToRunRecord vs the new function on clean and
message_end-fallback transcripts.
Extracts piMessagesToRunRecord as a shared helper so streaming and
full-events paths cannot drift. piEventsToRunRecord retained for the
headless driver which already holds events in memory.
Tests: 29/29 pass in test/adapters/pi.test.ts (adds equivalence,
noise-skipping, and message_end-fallback cases). Verified against the
tb-circuit-fibsqrt log that previously OOM'd.
Follow-up to f8603a9: parser-side fix bounded RunRecord memory, but the
raw stdout string itself (0.3-1.7 GB for a long agentic transcript) was
still fully buffered by runSubprocess before pi.ts even saw it. Bench
resumed on f8603a9 crashed at RSS=12.16 GB / peak=18.61 GB with a Bun
segfault (SIGSEGV at a 2.29 TB virtual address, characteristic of a
very large string allocation) after 7 tasks. The Layer-1 fix cut
retention of parsed events; it did nothing about the raw string.
Layer 2 fix: give runSubprocess an optional stdoutSink option that
streams stdout bytes verbatim to a file lazily (no empty file if the
child produces nothing) with the same reader-cancellation semantics as
the existing string path. Pi adapter wires task.convLog.filePath in as
the sink — pi never populates convLog via logRequest/logResponse, so
finalize() early-returns on an empty entries[] and the streamed file
survives. Then piBuildRunRecordFromFile streams the same file back
line-by-line into the shared PiEventCollector (extracted from the two
existing paths so they cannot drift), retaining O(longest-line) bytes
instead of O(transcript).
Wall of writes:
before: pi -> stdout pipe -> stdout string (1 GB) -> parser
-> convLog file
after: pi -> stdout pipe -> convLog file
-> parser (stream reads back)
Peak heap projection: from 18.61 GB observed -> a few MB per task.
Also collapses the previous dual-write (buffer + convLog) into one.
Test files (test/adapters/pi.test.ts, test/core/subprocess.test.ts) are
scoring-adjacent under the PUA integrity policy and are intentionally
left uncommitted here for separate verifier ownership.
Add a container branch to the pi adapter so TB tasks (identified by
tbDockerImage on BenchTask) run inside skvm-pi-runtime with workDir
bind-mounted at /app. Model-issued shell commands (find /, grep -r, ...)
now execute in a clean Ubuntu root instead of the Windows Git-Bash host
via MSYS, fixing the tb-db-wal-recovery host-traversal timeout.
Components:
- src/core/docker-run.ts startContainer/execInContainer primitive,
symmetric to runSubprocess. Handles
MSYS_NO_PATHCONV, unique names, timeout kill,
idempotent cleanup.
- src/adapters/pi.ts runInContainer() method + branch on
task.tbDockerImage in run(). Existing host
path unchanged for non-TB tasks. Opt-out via
SKVM_PI_HOST_MODE=1.
- src/core/types.ts AgentAdapter.run() param type gains optional
tbDockerImage. Non-container adapters ignore.
- src/framework/runner.ts Thread BenchTask.tbDockerImage into adapter.run.
- docker/skvm-pi-runtime.Dockerfile
Ubuntu 24.04 + Node 20 + pi 0.67.68 pinned
to host node_modules version. CN apt mirror.
Verified: bench --adapter=pi --model=deepseek/deepseek-v4-pro
--tasks=tb-break-filter-js-from-html --conditions=no-skill → reward=1
in 4m19s. Replicates prior host-mode result on this task, confirming
the container branch is a functional equivalent.
Plan doc: plan/2026-07-02-containerized-pi-agent-for-tb.md
…er mode
Mirror skvm-pi-runtime for the two other adapters skvm will make
container-aware. Each preinstalls its own CLI on Ubuntu 24.04 + Node 20
with the same CN apt/npm mirror configuration proven by the pi image.
- skvm-claude-code-runtime npm i -g @anthropic-ai/claude-code
(built: 2.1.197, 1.39 GB)
- skvm-opencode-runtime npm i -g opencode-ai
(built: 1.17.13, 1.81 GB)
Unlike pi (which pins the version because the adapter writes an internal
models.json that the CLI parses), these CLIs are consumed purely through
documented command-line flags so version drift between host and container
is safe for A/B bench comparisons.
Adapter code that consumes these images comes in a follow-up commit.
Verify locally with:
docker run --rm skvm-claude-code-runtime:latest claude --version
docker run --rm skvm-opencode-runtime:latest opencode --version
Add a container branch to the claude-code adapter, mirroring the pi adapter (a2a283f). When a task carries tbDockerImage (and SKVM_CC_HOST_MODE is not set), claude runs inside skvm-claude-code-runtime with workDir bind-mounted at /app so model-issued shell commands (find /, grep -r, ...) execute in a Linux root instead of the Windows Git-Bash host via MSYS. - Existing host path unchanged; extracted into runOnHost() to keep the container branch isolated. - Container mode ships the managed-mode settings.json into a workDir-side .cc-sandbox and points CLAUDE_CONFIG_DIR at it via docker -e. Symmetric to pi's .pi-sandbox + PI_CODING_AGENT_DIR pattern. - Native mode inside container is explicitly rejected with a clear error (bench always uses managed; native support is a follow-up). - Opt-out for debugging: SKVM_CC_HOST_MODE=1. Verification status: TypeScript diagnostics pass; end-to-end reward=1 smoke NOT run because the current provider config has no Anthropic- protocol route (DeepSeek/LongCat are openai-compatible; Claude CLI needs /v1/messages). Verification will land when an Anthropic-compatible endpoint is available. Structural parity with the pi container branch (committed reward=1 on tb-break-filter-js-from-html) is the acting correctness signal for this change.
Add a container branch to the opencode adapter, mirroring the pi (a2a283f) and claude-code (fb06e89) adapters. When a task carries tbDockerImage (and SKVM_OC_HOST_MODE is not set), opencode runs inside skvm-opencode-runtime with workDir bind-mounted at /app so model-issued shell commands (find /, grep -r, ...) execute in a Linux root instead of the Windows Git-Bash host via MSYS. - Existing host path extracted into runOnHost() unchanged. - Container mode passes the cached OPENCODE_CONFIG_CONTENT JSON through the container's env — no sandbox file needed, since opencode reads provider config directly from that env variable. Simpler than pi and claude-code, both of which need a sandbox dir for their config files. - Native mode rejected inside container with a clear error; managed + openai-compatible only. Anthropic routes rejected because current setup path doesn't build a config for them anyway. - Opt-out for debugging: SKVM_OC_HOST_MODE=1. Verification: bench --adapter=opencode --model=deepseek/deepseek-v4-pro --tasks=tb-break-filter-js-from-html --conditions=no-skill exercised the full container path end-to-end in 1m12s / 9.6K tokens. Container lifecycle (start, exec, cleanup), workDir mount, OPENCODE_CONFIG_CONTENT plumbing, NDJSON parsing, and tb-grade verification all clean. tb-grade returned reward=0 (verifier ran normally; DeepSeek's XSS attempt didn't beat filter.py this run), which is the expected per-task-instance variability, not a code-path failure.
Add a container branch to the hermes adapter, mirroring the pi (a2a283f), claude-code (fb06e89), and opencode (73c074a) adapters. When a task carries tbDockerImage (and SKVM_HERMES_HOST_MODE is not set), hermes runs inside skvm-hermes-runtime with workDir bind-mounted at /app. - Existing host path extracted into runOnHost() unchanged; container branch is entirely additive. - Host CLI resolution (resolveHermesCmd) is deferred from setup() to runOnHost() lazily. Container-only bench runs no longer require hermes on the host — the previous eager resolveHermesCmd() would fail-fast even when the container branch owned the actual execution. This is a latent architectural gap in pi/claude-code/opencode too, hidden there because those CLIs happened to be reachable via host tiers. - Container mode runs TWO execs on the same running container: 1. hermes chat -q <prompt> -m <model> ... 2. hermes sessions export <sid> ... Both must land on the same container because session state lives in a SQLite DB under HERMES_HOME. The docker-run.ts container-per-task model (sleep 36000 PID 1) already supports multi-exec cleanly. - Managed config (config.yaml + .env, produced from providers.routes) is written to workDir/.hermes-sandbox and HERMES_HOME env points at the container-side path — same pattern as pi's .pi-sandbox and claude-code's .cc-sandbox. - Native mode rejected inside container with a clear error. - New Dockerfile skvm-hermes-runtime.Dockerfile: Ubuntu 24.04 + Python 3.12 + pip install hermes-agent==0.18.0 (PyPI, entry point `hermes`). Uses official archive.ubuntu.com because Docker Desktop vpnkit was hijacking aliyun to fake IPs on the build machine; the aliyun mirror rewrite from the older runtime Dockerfiles can be reintroduced when the split-DNS routing settles. Verification: bench --adapter=hermes --model=deepseek/deepseek-v4-pro --tasks=tb-break-filter-js-from-html --conditions=no-skill exercised the full container path end-to-end in 8m25s. Container lifecycle (start, chat exec, sessions export exec, cleanup), workDir mount, and tb-grade verification all clean; workDir produced out.html and the verifier ran normally. tb-grade returned reward=0 (verifier ran fine; DeepSeek's XSS attempt didn't beat filter.py this run), which is the expected per-task-instance variability, not a code-path failure. Known limitation: telemetry fell through to reduced-record path (Avg Tokens / Avg Cost "n/a") — the session_id trailer parsing works on the host branch but may need adjustment for container stdout buffering. Follow-up work; does not affect correctness of the workDir scored by tb-grade.
Add path-specific ignore rules resolved during S641/S642 (2026-07-05):
- /terminal-bench-2-1/ external Terminal-Bench 2.1 checkout (its own
git repo, ~3 GB dataset). Existing untracked line
kept for compatibility; this one is redundant but
harmless and self-documents intent.
- /tb2.1-skills/ skill files imported from Terminal-Bench 2.1
into the local dataset area. SkVM's TB glue code
(src/bench/importers/terminalbench.ts,
src/bench/evaluators/tb-grade.ts, related tests
and docs) IS tracked; only the imported skill
content is not.
- /bench-results/ ephemeral bench run outputs (per-session logs,
report.md/json). SkVM writes these under
$SKVM_CACHE by default, but a stale symlink or
local run can leak them here.
- /benchmark-results/ same as above with the alternative name some
scripts use.
- .playwright-mcp/ per-session Playwright MCP snapshots and traces.
- plan/**/*.pdf PDF exports of plan docs (regenerated from md).
- plan/**/*_tmp.html intermediate render output from plan tooling.
Companion commit removes tb2.1-skills from the index so these rules take
effect (gitignore alone doesn't untrack already-tracked files).
Files remain in the working tree — the /tb2.1-skills/ gitignore rule (added in 8ddb9f2) now takes effect so future imports don't sneak 90 more skill files back into the index. Rationale from S641 (2026-07-05): the tb2.1-skills directory holds skill content imported from an external Terminal-Bench 2.1 dataset, NOT source code. SkVM's TB glue lives in src/bench/importers/, src/bench/evaluators/, and companion tests/docs — all of which stay tracked and reviewable. The imported skills themselves are ~200 KB of generated bench material; tracking them mixes vendored data with source and clutters diffs/PRs. If a downstream user needs the skills, they should re-import via `skvm bench --import=terminalbench --path=<tb-2.1-repo>`.
Hermes CLI 0.18.0 (and likely all versions since the initial adapter commit 64d1ea5) writes the `session_id: <id>` trailer to STDERR, not stdout — regardless of exit code. Empirically verified against skvm-hermes-runtime:latest with `hermes chat -Q --source=tool`: - STDOUT: tirith banner + agent's final reply - STDERR: blank line + `session_id: YYYYMMDD_HHMMSS_hexhash` The adapter was matching the regex against `stdout` in both the host branch (hermes.ts:438) and the container branch (hermes.ts:711), which silently never matched. Every hermes bench run degraded to the reduced-telemetry path: `hermes sessions export` was skipped (no sessionId to pass), Avg Tokens / Avg Cost rendered as "n/a" in the report, and per-token accounting was lost. This is a pre-existing bug from the initial commit, not a regression introduced by containerization. It surfaced during the container-mode work (commit e13b7a8) because the container branch's verification specifically tracked whether telemetry populated. Host branch has always been broken on machines that never ran hermes bench before. Fix: change both regex sources from `stdout.match(...)` to `stderr.match(...)`. Two-line change per branch; added explanatory comments with the empirical evidence so the next contributor doesn't "fix" it back to stdout. Verification: bench --adapter=hermes --model=deepseek/deepseek-v4-pro --tasks=tb-break-filter-js-from-html --conditions=no-skill Before: Avg Tokens n/a, Avg Cost n/a (session_id missing) After: Avg Tokens 28.1K, hermes sessions export ran successfully. The earlier "CRLF in Docker stdout" hypothesis (raised in the post-e13b7a8 review) was a false lead: hexdump of the failing run's convLog showed 0 CR bytes and 0 occurrences of `session_id` anywhere in the file. The trailer was never on stdout to begin with.
Replace the single-mirror sed rewrite (aliyun-only) in three runtime Dockerfiles with a dual-source approach: keep the official archive.ubuntu.com / security.ubuntu.com entries in ubuntu.sources untouched, and append aliyun as a second DEB822 stanza. apt aggregates both stanzas and pulls from whichever responds. When aliyun is reachable (the common case in CN), builds use the faster mirror. When aliyun is hijacked by Docker Desktop vpnkit to fake IPs (198.18.0.x — the failure that hit skvm-hermes-runtime.Dockerfile mid-build on 2026-07-05), apt transparently falls back to the official archive and the build still succeeds. Hardens: - docker/skvm-pi-runtime.Dockerfile - docker/skvm-claude-code-runtime.Dockerfile - docker/skvm-opencode-runtime.Dockerfile skvm-hermes-runtime.Dockerfile already uses official-only (it was the incident that surfaced the issue). It is intentionally left as-is — its comment documents the incident for future contributors. A follow-up could unify all four Dockerfiles on the dual-source pattern. Why DEB822 multi-stanza (not a separate .list file): apt treats the same suite appearing in two different source files as "configured multiple times" and emits a warning on every apt-get update. Putting aliyun as a second stanza in the same ubuntu.sources file is treated as "the same repository mirrored at two URIs" and produces no warning. Verification (per image): docker build -t skvm-<name>-runtime:latest -f docker/skvm-<name>-runtime.Dockerfile . docker run --rm skvm-<name>-runtime:latest <cli> --version docker run --rm skvm-<name>-runtime:latest grep -c mirrors.aliyun.com /etc/apt/sources.list.d/ubuntu.sources All three rebuilt successfully (pi 1.28GB, claude-code 1.39GB, opencode 1.81GB) with the aliyun stanza present and apt-get update observed pulling from both mirrors in parallel.
Contributor
Author
|
Closing this monolithic PR in favor of 5 focused PRs:
Plan files excluded: The split PRs exclude the 4 Review priority: #93, #94, #95, #96 are independent. #97 depends on #94 (so merge #94 first). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR integrates Terminal-Bench (TB2.1) evaluation support into SkVM with containerized runtime environments for 4 adapters, plus critical infrastructure improvements discovered during integration testing.
Terminal-Bench Integration
Container Runtime Support
Bench Infrastructure
Bug Fixes Discovered During TB Testing
Infrastructure Hardening
Docker Apt Source Resilience
Maintenance
Gitignore & Tracking Cleanup
Documentation
Compiler Fix
Code Block Preservation
Test Plan
Breaking Changes
None. All changes are additive (new container branches) or hardening (apt sources, process cleanup).
Related Issues
Commits: 20
Files changed: ~40 (adapters, bench, runtime, docs, gitignore)
Lines: +2000/-500 (estimated)
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com