fix(v1): persist prime-agent ACP sessions - #2260
Conversation
Drives prime-agent's own ACP mode through the existing ACP helper rather than the pi harness's third-party pi-acp adapter. That adapter spawns `pi --mode rpc` and hard-codes pi's RPC command and event union, so prime-agent's IPython-only tool model, subagents, autonomous gates, goals, and heartbeats either degrade to a generic tool call or disappear. prime-agent speaks ACP natively as of its ACP mode, and carries the concepts ACP has no field for in a namespaced `ai.primeintellect.prime-agent` `_meta` envelope, so a rollout can observe them without the harness parsing a private protocol. Targets the current launch() contract. HOME is pinned per trace because prime-agent writes session and kernel state beneath it and concurrent rollouts must not share either.
0.6.0 is the first Prime Agent release that ships native ACP mode, so the harness can now install a published tarball that actually has --mode acp. Verified the derived tarball URL returns 200.
… key Four fixes found by running an actual eval in the docker runtime, each verified in a container rather than inferred: - Containers ship no Node and prime-agent requires >=22.8, so install failed with "npm: not found". Bootstraps Node the way the pi harness does, and exports it on PATH for the launch wrapper too, since the bundled Node is not on the container PATH. - The published release derives its env prefix from its own package piConfig, so it reads PRIME_AGENT_CODING_AGENT_DIR, not the upstream PI_ prefix. With the wrong name models.json was ignored and the run failed with "Unknown provider intercept". - prime-agent does not expand "$VAR" in models.json: it sends the literal string as the bearer token, which produced "401 unauthorized". Confirmed against a capturing HTTP server, which received `Bearer $PRIME_AGENT_INTERCEPT_KEY`. Inlines the secret instead of the pi-style indirection. With these, an eval completes end to end: 4 model calls, err 0.00, and a scored alphabet_sort reward.
…he prompt Review follow-up on two real problems. The bearer token was written in plaintext to the per-trace models.json. Concurrent rollouts share a runtime, so model-executed code in one rollout could read another rollout's credential and issue authenticated requests against its interception endpoint. This was a consequence of inlining the secret to work around prime-agent not expanding "$VAR": the pi harness's indirection had kept it out of the file. models.json now carries a placeholder, and the launch wrapper substitutes the real value from the environment at exec time under a 0700 dir and a 0600 file. The system prompt was applied twice: once via --append-system-prompt and again by the ACP runner, which seeds it into the conversation for a new session. Dropped the flag and left a note so it does not come back. Verified by rerunning the eval: ok=true, 3 model calls, 0 errors, and a scored alphabet_sort reward.
…Alpine The install guard short-circuited on the binary alone, so a runtime that already held one build reused it after `version` or `tarball_url` changed. Key the guard on the requested tarball like the pi harness keys on its versions, and give the Alpine branch the same repo-bump retry, since the official Node build is glibc-only and an older Alpine's own nodejs-current is below the 22.8 prime-agent needs.
…have prime-agent's ACP mode ignores the `mcpServers` of `session/new` entirely, and its own MCP integrations are authored Python skills the model imports in its kernel, so tool servers handed to the harness never reached the model: the repo's own echo-acp-resume-v1 fixture ran to completion with the agent reporting the tool did not exist and the reward at 0. Declare SUPPORTS_MCP false so `validate_pairing` rejects that pairing instead of degrading it.
…t segment A resumed segment replays the accreted conversation, and the ACP runner renders that transcript into the prompt with the `[system]` block the first segment already rendered into it, so passing `system_prompt` again delivered the task instructions twice. Observed on echo-user-sim-v1: the second segment's prompt carried two copies of the system prompt, one after this change.
…arly
Three install/launch edges from review, none reachable on the images this runs
on today but all of them failing obscurely when they are hit:
- the wrapper substituted the bearer token with `sed`, so a token containing
`|`, `&`, or a backslash would corrupt the key or abort the wrapper before
exec; node now rewrites the parsed models.json instead. Today's secret is
`secrets.token_urlsafe(16)`, which cannot contain those, so this is about not
depending on the generator's alphabet.
- a curl-less non-Debian image ran `apt-get` regardless and failed three steps
later inside `tar` ("tar: invalid magic"); it now says what it needs.
- an unrecognized machine fell back to the x64 Node archive and failed as
"prime-agent requires Node.js 22.8 or newer"; it is now rejected by name,
like the OS check already does.
The bucket URL reads like a stale internal endpoint next to the user-facing installer at app.primeintellect.ai/prime-agent/install.sh, so note why the harness uses it. That script is a thin front end: it defaults its own prime_agent_base_url to this bucket and downloads $base/releases/v$version/$package-$version.tgz, the same shape _tarball() builds. The value is also the prime-agent repo's R2_PUBLIC_BASE_URL variable, which its release workflow publishes to. This harness installs the npm tarball directly instead of running the script, so it needs the artifact base rather than the installer URL. Comment only; no behavior change. Verified the derived URL for the pinned version returns 200, and the docker eval still reports ok=true with 4 model calls, no errors, and a scored reward.
Drives prime-agent's own ACP mode through the existing ACP helper rather than the pi harness's third-party pi-acp adapter. That adapter spawns `pi --mode rpc` and hard-codes pi's RPC command and event union, so prime-agent's IPython-only tool model, subagents, autonomous gates, goals, and heartbeats either degrade to a generic tool call or disappear. prime-agent speaks ACP natively as of its ACP mode, and carries the concepts ACP has no field for in a namespaced `ai.primeintellect.prime-agent` `_meta` envelope, so a rollout can observe them without the harness parsing a private protocol. Targets the current launch() contract. HOME is pinned per trace because prime-agent writes session and kernel state beneath it and concurrent rollouts must not share either.
0.6.0 is the first Prime Agent release that ships native ACP mode, so the harness can now install a published tarball that actually has --mode acp. Verified the derived tarball URL returns 200.
… key Four fixes found by running an actual eval in the docker runtime, each verified in a container rather than inferred: - Containers ship no Node and prime-agent requires >=22.8, so install failed with "npm: not found". Bootstraps Node the way the pi harness does, and exports it on PATH for the launch wrapper too, since the bundled Node is not on the container PATH. - The published release derives its env prefix from its own package piConfig, so it reads PRIME_AGENT_CODING_AGENT_DIR, not the upstream PI_ prefix. With the wrong name models.json was ignored and the run failed with "Unknown provider intercept". - prime-agent does not expand "$VAR" in models.json: it sends the literal string as the bearer token, which produced "401 unauthorized". Confirmed against a capturing HTTP server, which received `Bearer $PRIME_AGENT_INTERCEPT_KEY`. Inlines the secret instead of the pi-style indirection. With these, an eval completes end to end: 4 model calls, err 0.00, and a scored alphabet_sort reward.
…he prompt Review follow-up on two real problems. The bearer token was written in plaintext to the per-trace models.json. Concurrent rollouts share a runtime, so model-executed code in one rollout could read another rollout's credential and issue authenticated requests against its interception endpoint. This was a consequence of inlining the secret to work around prime-agent not expanding "$VAR": the pi harness's indirection had kept it out of the file. models.json now carries a placeholder, and the launch wrapper substitutes the real value from the environment at exec time under a 0700 dir and a 0600 file. The system prompt was applied twice: once via --append-system-prompt and again by the ACP runner, which seeds it into the conversation for a new session. Dropped the flag and left a note so it does not come back. Verified by rerunning the eval: ok=true, 3 model calls, 0 errors, and a scored alphabet_sort reward.
…Alpine The install guard short-circuited on the binary alone, so a runtime that already held one build reused it after `version` or `tarball_url` changed. Key the guard on the requested tarball like the pi harness keys on its versions, and give the Alpine branch the same repo-bump retry, since the official Node build is glibc-only and an older Alpine's own nodejs-current is below the 22.8 prime-agent needs.
…have prime-agent's ACP mode ignores the `mcpServers` of `session/new` entirely, and its own MCP integrations are authored Python skills the model imports in its kernel, so tool servers handed to the harness never reached the model: the repo's own echo-acp-resume-v1 fixture ran to completion with the agent reporting the tool did not exist and the reward at 0. Declare SUPPORTS_MCP false so `validate_pairing` rejects that pairing instead of degrading it.
…t segment A resumed segment replays the accreted conversation, and the ACP runner renders that transcript into the prompt with the `[system]` block the first segment already rendered into it, so passing `system_prompt` again delivered the task instructions twice. Observed on echo-user-sim-v1: the second segment's prompt carried two copies of the system prompt, one after this change.
…arly
Three install/launch edges from review, none reachable on the images this runs
on today but all of them failing obscurely when they are hit:
- the wrapper substituted the bearer token with `sed`, so a token containing
`|`, `&`, or a backslash would corrupt the key or abort the wrapper before
exec; node now rewrites the parsed models.json instead. Today's secret is
`secrets.token_urlsafe(16)`, which cannot contain those, so this is about not
depending on the generator's alphabet.
- a curl-less non-Debian image ran `apt-get` regardless and failed three steps
later inside `tar` ("tar: invalid magic"); it now says what it needs.
- an unrecognized machine fell back to the x64 Node archive and failed as
"prime-agent requires Node.js 22.8 or newer"; it is now rejected by name,
like the OS check already does.
The bucket URL reads like a stale internal endpoint next to the user-facing installer at app.primeintellect.ai/prime-agent/install.sh, so note why the harness uses it. That script is a thin front end: it defaults its own prime_agent_base_url to this bucket and downloads $base/releases/v$version/$package-$version.tgz, the same shape _tarball() builds. The value is also the prime-agent repo's R2_PUBLIC_BASE_URL variable, which its release workflow publishes to. This harness installs the npm tarball directly instead of running the script, so it needs the artifact base rather than the installer URL. Comment only; no behavior change. Verified the derived URL for the pinned version returns 200, and the docker eval still reports ok=true with 4 model calls, no errors, and a scored reward.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
a0681ae to
f9513af
Compare
6276544 to
1db6552
Compare
1db6552 to
aeec853
Compare
This reverts commit f14b41c. Run identity returns to Trace.run / Trace.record_run. The episode-level home turned out to push consumers that keep per-trace records into episode reconstruction for no gain. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Fix colocated MCP services in Prime runtimes * Address review: drop run_background guard and comment Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: root <root@light-bright-tody.datacrunch.io> Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(v1): make creation limiters user-global Store the cross-process creation-limiter buckets under ~/.cache/verifiers/limiter instead of the shared temp dir, so cluster users do not collide on /tmp/vf-rate-limiters. * fix(v1): harden limiter cache path Define a shared CACHE_DIR with a temp-dir fallback when Path.home() cannot resolve, rename the limiter dir constant to LIMITER_DIR, and persist wall-clock cursors so cached reservations survive reboots without huge stale waits. * refactor: export home_dir helper
* feat(v1): persist and resume task validation * refactor(v1): simplify validate persistence layout * fix(v1): match eval resume identity * refactor(v1): share CLI resume primitives
Every command-session RPC rode pyqwest's shared default transport, which
multiplexes one HTTP/2 connection per host. The Prime gateway caps a
connection at 100 concurrent streams and each live process pins one
stream for its whole life, so accumulated ACP seats starved stdin/signal
RPCs into their 30s deadline ("process stdin RPC failed
(deadline_exceeded)") and queued new process streams for minutes under
the 24h stream timeout. Reproduced against a scratch sandbox: writes
stall at exactly the 100th live process while a fresh transport answers
in 0.1s.
- PrimeRuntime.open_process now rebuilds the SDK wiring with one
dedicated HTTPTransport per process; the process's stream and its
stdin/signal RPCs share that transport and compete only with each
other. Error strings match the SDK's.
- RuntimeProcess gains aclose(); PrimeProcess.aclose releases the SDK
handle and its transport, and ACPHarnessSession._stop always calls it,
so a seat abandoned without an exit event no longer pins a gateway
stream slot until sandbox deletion.
Canary on a live sandbox: 110 processes, one connection each, writes
steady at 0.06-0.08s through the old cliff; closing 55 released exactly
55 connections.
| closer = getattr(process, "aclose", None) | ||
| if closer is not None: | ||
| with contextlib.suppress(BaseException): | ||
| await closer() |
There was a problem hiding this comment.
🟠 High acp/__init__.py:340
_stop awaits process.aclose() without a timeout, unlike every other teardown operation in the method which is bounded. When the remote process handle was abandoned because its transport stalled, aclose() can block on that same stalled transport, so _stop never returns. This hangs close() and stalls rollout teardown indefinitely. Wrap the aclose() call in a bounded wait, the same way process.wait() and stderr_task are handled.
| closer = getattr(process, "aclose", None) | |
| if closer is not None: | |
| with contextlib.suppress(BaseException): | |
| await closer() | |
| closer = getattr(process, "aclose", None) | |
| if closer is not None: | |
| with contextlib.suppress(BaseException): | |
| await asyncio.wait_for(closer(), timeout=5) |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/acp/__init__.py around lines 340-343:
`_stop` awaits `process.aclose()` without a timeout, unlike every other teardown operation in the method which is bounded. When the remote process handle was abandoned because its transport stalled, `aclose()` can block on that same stalled transport, so `_stop` never returns. This hangs `close()` and stalls rollout teardown indefinitely. Wrap the `aclose()` call in a bounded wait, the same way `process.wait()` and `stderr_task` are handled.
* fix(v1): de-flake the live E2E suite
- openclaw: make reasoning replay opt-in (sampling.reasoning_effort), never
a model-name guess. OpenClaw redacts Prime's dotted encrypted_content in
its persisted transcript (fails its opaque-token allowlist), so resumed
sessions deterministically 400 replaying the mangled token.
- chat wire: replay an empty assistant completion as content "" instead of
null - strict providers reject null content without tool calls (422).
- echo-tool fixture: score the stamped TOOL result (the tool really ran)
instead of the assistant's verbatim relay (model obedience).
- e2e conftest: retry HarnessError'd rollouts too (empty agent turns,
agent-timeout stalls); deterministic failures still fail all attempts.
* fix(v1): keep the user-sim persona from doing the task itself
A strong instruction-follower reads the scenario's imperative text
("Call the `echo_back` tool ...") as its own instructions and opens the
conversation by emitting the task's answer format directly - the
assistant never hears the request, no tool ever runs, and the episode
scores 0. Frame the scenario as what the USER wants the ASSISTANT to do
and forbid the user seat from producing the task's output itself.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore: trim comments to one-liners
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
pyqwest 0.7 made system CA trust opt-in for explicitly constructed transports, so a bare HTTPTransport() fails TLS against the gateway with "invalid peer certificate: UnknownIssuer". Pass tls_include_system_certs=True and fall back for pyqwest 0.6.x, where the parameter does not exist and system CAs are on by default.
…rface Unions the conflicting additions: session_meta (main) rides alongside allow_empty_tool_reply (branch) through the ACP session surface, and the runner keeps both the keepalive interval and main's late-update grace. The late-update wait now runs before the tool-only-turn acceptance check.
| f.seek(0) | ||
| data = f.read().strip() | ||
| now = time.monotonic() | ||
| now = time.time() |
There was a problem hiding this comment.
🟠 High runtimes/limiters.py:42
Switching _reserve from time.monotonic() to time.time() makes the limiter vulnerable to wall-clock regressions. If the clock is moved backward after a reservation (NTP correction, manual change), the persisted cursor stays ahead of the new now, so slot - now returns the full backward offset and every subsequent creation sleeps for that entire duration — potentially minutes or hours, effectively hanging all sandbox/tunnel creation until wall time catches up. The previous monotonic clock was immune to this because it never goes backward. If the monotonic-vs-wall-clock tradeoff was intentional (the comment mentions surviving reboots), consider documenting why wall-clock backward jumps are acceptable, or guard against regressions (e.g., clamp slot to now when data is in the past).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/runtimes/limiters.py around line 42:
Switching `_reserve` from `time.monotonic()` to `time.time()` makes the limiter vulnerable to wall-clock regressions. If the clock is moved backward after a reservation (NTP correction, manual change), the persisted cursor stays ahead of the new `now`, so `slot - now` returns the full backward offset and every subsequent creation sleeps for that entire duration — potentially minutes or hours, effectively hanging all sandbox/tunnel creation until wall time catches up. The previous monotonic clock was immune to this because it never goes backward. If the monotonic-vs-wall-clock tradeoff was intentional (the comment mentions surviving reboots), consider documenting why wall-clock backward jumps are acceptable, or guard against regressions (e.g., clamp `slot` to `now` when `data` is in the past).
| def home_dir() -> Path: | ||
| """Best-effort home directory; fall back to the temp dir so import never fails.""" | ||
| try: | ||
| return Path.home() | ||
| except RuntimeError: | ||
| return Path(tempfile.gettempdir()) |
There was a problem hiding this comment.
🟠 High utils/path_utils.py:12
home_dir() falls back to the shared directory from tempfile.gettempdir() (commonly /tmp), so CACHE_DIR resolves to /tmp/.cache/verifiers for every affected user. Because that path is predictable and shared, another local user can pre-create or populate the tree, causing permission failures or allowing cache contents to be read or influenced across users. Use a per-user private directory (e.g. append getpass.getuser() or os.getuid() under the temp dir) rather than a fixed shared path.
| def home_dir() -> Path: | |
| """Best-effort home directory; fall back to the temp dir so import never fails.""" | |
| try: | |
| return Path.home() | |
| except RuntimeError: | |
| return Path(tempfile.gettempdir()) | |
| def home_dir() -> Path: | |
| """Best-effort home directory; fall back to a user-scoped temp dir so import never fails.""" | |
| try: | |
| return Path.home() | |
| except RuntimeError: | |
| return Path(tempfile.gettempdir()) / f"verifiers-{os.getuid()}" |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/utils/path_utils.py around lines 12-17:
`home_dir()` falls back to the shared directory from `tempfile.gettempdir()` (commonly `/tmp`), so `CACHE_DIR` resolves to `/tmp/.cache/verifiers` for every affected user. Because that path is predictable and shared, another local user can pre-create or populate the tree, causing permission failures or allowing cache contents to be read or influenced across users. Use a per-user private directory (e.g. append `getpass.getuser()` or `os.getuid()` under the temp dir) rather than a fixed shared path.
| ACP_BIN = f"{PACKAGES_DIR}/node_modules/.bin/claude-agent-acp" | ||
| ACP_COMMAND = [f"{NODE_BIN_DIR}/node", ACP_BIN] | ||
| CLAUDE_CONFIG_ROOT = ".vf-claude" | ||
| SKILLS_DIR = ".claude/skills" |
There was a problem hiding this comment.
🟠 High claude_code/harness.py:20
SKILLS_DIR points at .claude/skills, the project's own skills directory, so install_skills() overwrites any existing files at .claude/skills/<skill.name>/... in the checked-out repository. cleanup() only removes .vf-claude/<trace-id>, so the injected skill files persist after the run instead of being cleaned up. Consider using a sandbox-owned directory under CLAUDE_CONFIG_ROOT (e.g. .vf-claude/<trace-id>/skills) so task repository content is not modified.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/harnesses/claude_code/harness.py around line 20:
`SKILLS_DIR` points at `.claude/skills`, the project's own skills directory, so `install_skills()` overwrites any existing files at `.claude/skills/<skill.name>/...` in the checked-out repository. `cleanup()` only removes `.vf-claude/<trace-id>`, so the injected skill files persist after the run instead of being cleaned up. Consider using a sandbox-owned directory under `CLAUDE_CONFIG_ROOT` (e.g. `.vf-claude/<trace-id>/skills`) so task repository content is not modified.
The gateway reaps connections that carry no data for about 30 minutes. Keepalives previously flowed only while a turn was running, so a panel seat idle between turns sat on a silent dedicated connection and lost it: 47 "process stream RPC failed (unavailable): connection reset" failures in one overnight campaign, median 31 idle minutes before the reset surfaced. Emit keepalives for the whole serve_stream lifetime instead of per turn; the packet reader on the host side already discards them.
| await asyncio.get_running_loop().connect_read_pipe( | ||
| lambda: protocol, sys.stdin.buffer | ||
| ) | ||
| keepalive = asyncio.create_task(emit_keepalives(sys.stdout.buffer)) |
There was a problem hiding this comment.
🟠 High acp/runner.py:408
If emit_keepalives raises an exception (e.g., write_packet fails because the stdout consumer has disconnected while stdin stays open), the keepalive task silently dies but serve_stream never notices — it keeps blocking on read_packet indefinitely, holding the live ACP process and session open with no one reading the output. The keepalive task is created as a fire-and-forget background task with no supervision, so its termination is never observed. Consider racing the keepalive task against read_packet so that a write-side failure breaks the read loop and triggers session cleanup.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/acp/runner.py around line 408:
If `emit_keepalives` raises an exception (e.g., `write_packet` fails because the stdout consumer has disconnected while stdin stays open), the keepalive task silently dies but `serve_stream` never notices — it keeps blocking on `read_packet` indefinitely, holding the live ACP process and session open with no one reading the output. The keepalive task is created as a fire-and-forget background task with no supervision, so its termination is never observed. Consider racing the keepalive task against `read_packet` so that a write-side failure breaks the read loop and triggers session cleanup.
8271911 to
2007388
Compare
| PRIME_AGENT_ACP = ACP() | ||
|
|
||
|
|
||
| def _guarded_install(root: str, lock: str, command: str) -> str: |
There was a problem hiding this comment.
🟠 High prime_agent/harness.py:165
The no-flock fallback in _guarded_install can leave a stale ${lock}.d directory permanently after an untrappable termination such as SIGKILL or VM cancellation. Because the fallback loop only removes the directory via an EXIT trap, a signal that bypasses traps leaves the lock directory in place. Every subsequent setup call then waits 300 seconds and fails, even though no installer is active, so Prime Agent cannot be reinstalled without manually deleting the stale directory. Consider adding owner/liveness metadata to the lock directory or another stale-lock recovery mechanism so a dead owner does not block future installs.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/harnesses/prime_agent/harness.py around line 165:
The no-`flock` fallback in `_guarded_install` can leave a stale `${lock}.d` directory permanently after an untrappable termination such as `SIGKILL` or VM cancellation. Because the fallback loop only removes the directory via an `EXIT` trap, a signal that bypasses traps leaves the lock directory in place. Every subsequent `setup` call then waits 300 seconds and fails, even though no installer is active, so Prime Agent cannot be reinstalled without manually deleting the stale directory. Consider adding owner/liveness metadata to the lock directory or another stale-lock recovery mechanism so a dead owner does not block future installs.
| with tarfile.open( | ||
| fileobj=output, mode="w:gz", format=tarfile.PAX_FORMAT | ||
| ) as tar: | ||
| for configured in self.config.skills: |
There was a problem hiding this comment.
🟠 High prime_agent/harness.py:419
When two configured skill directories share the same basename (e.g. /repo/a/review and /repo/b/review), _skills_archive emits both under the same archive path (review/), so their files overwrite each other during extraction. prepare_run then passes the identical --skill .../review argument twice, so Prime Agent never receives the two skills independently. Consider rejecting duplicate basenames in config.skills or archiving each skill under a unique key.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/harnesses/prime_agent/harness.py around line 419:
When two configured skill directories share the same basename (e.g. `/repo/a/review` and `/repo/b/review`), `_skills_archive` emits both under the same archive path (`review/`), so their files overwrite each other during extraction. `prepare_run` then passes the identical `--skill .../review` argument twice, so Prime Agent never receives the two skills independently. Consider rejecting duplicate basenames in `config.skills` or archiving each skill under a unique key.
| || { apt-get update -qq && apt-get install -y -qq curl ca-certificates >/dev/null; } | ||
| case "$(uname -s)" in Linux) node_os=linux ;; Darwin) node_os=darwin ;; *) echo "unsupported os: $(uname -s)" >&2; exit 1 ;; esac | ||
| if [ ! -x "$node/bin/node" ] || [ "$("$node/bin/node" --version 2>/dev/null)" != "v$VF_NODE_VERSION" ]; then | ||
| case "$(uname -m)" in aarch64|arm64) node_arch=arm64 ;; *) node_arch=x64 ;; esac |
There was a problem hiding this comment.
🟠 High harnesses/node.py:29
The architecture fallback in the else branch maps every machine other than aarch64/arm64 to x64, so on hosts like ppc64le, s390x, riscv64, or 32-bit ARM, ensure_node downloads the x86-64 Node archive. The extracted node binary fails to execute, causing setup to fail with an opaque error instead of reporting the architecture as unsupported. Consider matching only known-supported architectures and exiting with a clear message otherwise.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/harnesses/node.py around line 29:
The architecture fallback in the `else` branch maps every machine other than `aarch64`/`arm64` to `x64`, so on hosts like `ppc64le`, `s390x`, `riscv64`, or 32-bit ARM, `ensure_node` downloads the x86-64 Node archive. The extracted `node` binary fails to execute, causing setup to fail with an opaque error instead of reporting the architecture as unsupported. Consider matching only known-supported architectures and exiting with a clear message otherwise.
| "disk_size_gb": self.config.disk, | ||
| "gpu_count": gpu_count, | ||
| "timeout_minutes": MAX_LIFETIME // 60, | ||
| "timeout_minutes": max(1, math.ceil(self.config.lifetime_timeout / 60)), |
There was a problem hiding this comment.
🟡 Medium runtimes/prime.py:180
lifetime_timeout is documented as the maximum sandbox lifetime in seconds, but max(1, math.ceil(self.config.lifetime_timeout / 60)) rounds it up to whole minutes when provisioning, so the sandbox lives longer than the configured cap. For example, lifetime_timeout=61 provisions a 120-second sandbox and lifetime_timeout=1 provisions a 60-second sandbox — background and live processes can stay active and billable beyond the configured maximum. Consider either constraining lifetime_timeout to minute-aligned values (or a 60-second minimum) so the conversion is lossless, or documenting that the effective maximum is rounded up to the next whole minute.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/runtimes/prime.py around line 180:
`lifetime_timeout` is documented as the maximum sandbox lifetime in seconds, but `max(1, math.ceil(self.config.lifetime_timeout / 60))` rounds it up to whole minutes when provisioning, so the sandbox lives longer than the configured cap. For example, `lifetime_timeout=61` provisions a 120-second sandbox and `lifetime_timeout=1` provisions a 60-second sandbox — background and live processes can stay active and billable beyond the configured maximum. Consider either constraining `lifetime_timeout` to minute-aligned values (or a 60-second minimum) so the conversion is lossless, or documenting that the effective maximum is rounded up to the next whole minute.
| stream.write(len(data).to_bytes(8, "big") + data) | ||
| stream.flush() |
There was a problem hiding this comment.
🟠 High acp/runner.py:381
write_packet now concatenates the 8-byte header with the entire encoded payload via len(data).to_bytes(8, "big") + data, allocating a new bytes object nearly the size of the payload while data is still live. For a packet close to the 128 MiB limit, this creates an avoidable ~128 MiB memory spike that can OOM the runner. The previous separate stream.write calls for the header and payload avoided this extra allocation. Consider reverting to two write calls.
| stream.write(len(data).to_bytes(8, "big") + data) | |
| stream.flush() | |
| stream.write(len(data).to_bytes(8, "big")) | |
| stream.write(data) | |
| stream.flush() |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/acp/runner.py around lines 381-382:
`write_packet` now concatenates the 8-byte header with the entire encoded payload via `len(data).to_bytes(8, "big") + data`, allocating a new `bytes` object nearly the size of the payload while `data` is still live. For a packet close to the 128 MiB limit, this creates an avoidable ~128 MiB memory spike that can OOM the runner. The previous separate `stream.write` calls for the header and payload avoided this extra allocation. Consider reverting to two `write` calls.
| async def cleanup(self, trace: Trace, runtime: Runtime) -> None: | ||
| root = self.trace_root(trace) | ||
| wrapper = f"{root}/prime-agent" | ||
| shutdown = await runtime.run( |
There was a problem hiding this comment.
🟡 Medium prime_agent/harness.py:738
cleanup raises immediately when prime-agent shutdown --force fails, so the subsequent rm -rf of the trace root never runs. When rollout cleanup suppresses that exception without retry, the trace's agent state and temp files are left behind for the lifetime of a reused runtime, and a live daemon may persist. The rm -rf should run even when shutdown reports an error — e.g. by moving the state removal into a finally block.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/harnesses/prime_agent/harness.py around line 738:
`cleanup` raises immediately when `prime-agent shutdown --force` fails, so the subsequent `rm -rf` of the trace root never runs. When rollout cleanup suppresses that exception without retry, the trace's agent state and temp files are left behind for the lifetime of a reused runtime, and a live daemon may persist. The `rm -rf` should run even when shutdown reports an error — e.g. by moving the state removal into a `finally` block.
| if [ -f /etc/alpine-release ]; then | ||
| apk add --no-cache curl ca-certificates nodejs-current npm >/dev/null | ||
| if ! node -e 'const [a,b]=process.versions.node.split(".").map(Number); process.exit(a>22 || a===22 && b>=19 ? 0 : 1)'; then | ||
| sed -E -i 's/v[0-9]+\.[0-9]+/v3.22/g' /etc/apk/repositories |
There was a problem hiding this comment.
🟠 High harnesses/node.py:17
When an Alpine image's packaged Node is below 22.19, the fallback rewrites every configured APK repository to Alpine v3.22 and runs apk upgrade --available, cross-upgrading the entire OS distribution just to install Node. This replaces unrelated task packages and can make the task environment incompatible with what was requested. Consider adding a targeted repository for the compatible Node version or falling back to a standalone Node tarball build instead of rewriting all OS repositories.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/harnesses/node.py around line 17:
When an Alpine image's packaged Node is below 22.19, the fallback rewrites *every* configured APK repository to Alpine `v3.22` and runs `apk upgrade --available`, cross-upgrading the entire OS distribution just to install Node. This replaces unrelated task packages and can make the task environment incompatible with what was requested. Consider adding a targeted repository for the compatible Node version or falling back to a standalone Node tarball build instead of rewriting all OS repositories.
| selected_keys = [ | ||
| task_key(task.data.model_dump(mode="json", exclude_none=True)) for task in tasks | ||
| ] | ||
| if config.resume is None: |
There was a problem hiding this comment.
🟠 High cli/validate.py:386
A non-resume run unconditionally truncates results.jsonl in save_run at the supplied --output-dir, so reusing an existing validation directory silently destroys all persisted results before any validation starts. Consider refusing a non-empty out (or requiring an explicit --force) before calling save_run so prior results are not wiped without opt-in.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/cli/validate.py around line 386:
A non-resume run unconditionally truncates `results.jsonl` in `save_run` at the supplied `--output-dir`, so reusing an existing validation directory silently destroys all persisted results before any validation starts. Consider refusing a non-empty `out` (or requiring an explicit `--force`) before calling `save_run` so prior results are not wiped without opt-in.
| records.append(record) | ||
| trace.record_run(EvalRunInfo(id=config.uuid)) | ||
| await append_trace(out, trace, write_lock, env=config.env_id) | ||
| records.append(Episode.of(trace)) |
There was a problem hiding this comment.
🟡 Medium eval/runner.py:263
In run_group_unit, each trace is persisted with env=config.env_id, but the returned Episode.of(trace) omits that argument, so the in-memory episode's env.id is empty. Group-scored legacy runs thus return episodes with incorrect environment metadata — the on-disk trace has the right env_id while the in-memory result does not. Pass env=config.env_id to Episode.of(trace) so the returned episode matches what was persisted.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/cli/eval/runner.py around line 263:
In `run_group_unit`, each trace is persisted with `env=config.env_id`, but the returned `Episode.of(trace)` omits that argument, so the in-memory episode's `env.id` is empty. Group-scored legacy runs thus return episodes with incorrect environment metadata — the on-disk trace has the right `env_id` while the in-memory result does not. Pass `env=config.env_id` to `Episode.of(trace)` so the returned episode matches what was persisted.
| @@ -76,7 +77,8 @@ async def run_eval(env: Env, config: EvalConfig) -> list[Episode]: | |||
| write_lock = asyncio.Lock() | |||
|
|
|||
| async def on_complete(episode: Episode) -> None: | |||
There was a problem hiding this comment.
🟡 Medium eval/runner.py:79
on_complete now stamps EvalRunInfo only onto existing traces via trace.record_run(...) for each trace in episode.traces. When an evaluation fails before minting a trace, episode.traces is empty, so the persisted failure episode has no EvalRunInfo. The previous episode.record_run(...) call stamped the run identity at the episode level and covered traceless failures. run_unit has the same problem — it iterates episode.traces to record run info, so a server-side failure episode produced before any trace exists gets persisted and returned without any run identity. In both cases, failure artifacts can no longer be attributed to the eval run. Consider keeping an episode-level record_run call (or otherwise stamping the episode directly) so traceless failures retain their run identity.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/cli/eval/runner.py around line 79:
`on_complete` now stamps `EvalRunInfo` only onto existing traces via `trace.record_run(...)` for each trace in `episode.traces`. When an evaluation fails before minting a trace, `episode.traces` is empty, so the persisted failure episode has no `EvalRunInfo`. The previous `episode.record_run(...)` call stamped the run identity at the episode level and covered traceless failures. `run_unit` has the same problem — it iterates `episode.traces` to record run info, so a server-side failure episode produced before any trace exists gets persisted and returned without any run identity. In both cases, failure artifacts can no longer be attributed to the eval run. Consider keeping an episode-level `record_run` call (or otherwise stamping the episode directly) so traceless failures retain their run identity.
The luna endpoint surfaces upstream failures as finish_reason "error", outside the OpenAI SDK's Literal, so the interception stream parser's ModdedChatCompletion.model_validate rejected the assembled completion and 500'd the agent's inference call. The non-stream path already maps out-of-enum values to None in response_from_wire; widen the choice's finish_reason (like service_tier before it) so both paths agree.
7 participants
Problem
The initial Prime Agent harness is not sufficient for long-lived concurrent rollout seats. It needs stronger trace isolation, safe shared installation, better process diagnostics, and Prime runtime support for persistent ACP streams.
Change
Verification
pytest tests/v1 -m "not e2e": passes.This PR is stacked on #2254. The transport implementation is temporary and should be replaced by a prime-sandboxes release containing prime#823.