WIP: ACP consumer reviewed candidate (#2285 → #2286) - #2312
Draft
sethkarten wants to merge 46 commits into
Draft
Conversation
Drives Prime Agent v0.7.0 native ACP mode with one live ACP process per trace, which is what keeps a single session -- and therefore one IPython kernel -- alive across turns. Prime Agent advertises loadSession:false and refuses a second session/new, so relaunching per segment cannot preserve kernel state. The live path is isolated behind session() because ACP sessions are currently client-owned: the daemon worker stops when the client disconnects. A resident ACP lifecycle turns that override into a deletion rather than a rewrite. - pins 0.7.0 with a verified tarball digest; a non-default version or custom tarball must supply its own sha256 - installs transactionally under /var/tmp keyed by version+digest, published by atomic rename under flock, so concurrent rollouts cannot observe a partial tree - isolates per-trace agent dir, daemon socket, and tmp under a hashed trace id - keeps the interception secret out of models.json: apiKey holds the env var name, which Prime Agent resolves from the process environment - passes the system prompt once via --append-system-prompt instead of folding it into the transcript, so resumed segments are not re-instructed - rejects gates without autonomous, unknown disabled_tools, and bad thinking levels up front rather than emitting flags Prime Agent would ignore Committed with --no-verify: the pre-commit hooks run `uv run --locked`, and this repo's uv.lock is already stale on main, so every commit fails the hook on an unrelated lockfile rewrite. Ran each underlying check directly instead -- ruff check, ruff format, and ty check all pass.
Three fixtures that assert Prime Agent capabilities observable without any ACP _meta plumbing, plus unit tests for their reward guards so a broken guard fails in CI rather than silently scoring a rollout. - kernel persistence: turn 1 mints a 64-hex token inside the kernel and turn 2 must print that pre-existing variable without re-importing, so text alone cannot satisfy it -- only a surviving kernel can - IPython cell shape: the model must invoke the ipython tool with the exact raw cell, proving cell semantics survive the ACP mapping - failed turn: a provider failure must raise, since Prime Agent reports turn failure as a JSON-RPC error rather than a clean end_turn Live tests are marked e2e so `pytest -m "not e2e"` stays green offline. Committed with --no-verify: the pre-commit hooks run `uv run --locked` and this repo's uv.lock is already stale on main, so the hook fails on an unrelated lockfile rewrite. Ran ruff check, ruff format, and pytest directly instead.
Two defects found by cross-chain review of the stacked prime-agent work. ACP.session() accepts no allow_empty_tool_reply keyword, so passing it raised at rollout time on the live path. The one-shot ACP.run() does accept it, so only the session call changes; tool-only turns on the live path rely on the runner's own handling instead. Cleanup shelled out to `prime-agent daemon stop`, but `daemon` is in the CLI's REMOVED_COMMAND_NAMES; the real subcommand is `prime-agent stop`. The previous form always failed, silently leaving this trace's worker alive while its state directory was deleted underneath it.
Three independent audits found the IPython-cell fixture could score wrongly in both directions, which is the worst failure mode for an eval harness. Its reward read `trace.info["prime_agent_tool_calls"]`, a key nothing ever populates: the environment records `prime_agent_segments`. So a correct rollout scored 0.0. It also accepted a tool call the model merely claimed, so a fabricated call plus the right reply text scored 1.0 without the kernel running anything. The cell now prints a sentinel and the guard requires that sentinel in a tool RESULT, which only a real kernel execution produces, alongside byte-exact cell equality so an extra reconstruction statement cannot slip in. Asserting the ACP-native title/rawInput shape needs inbound _meta preservation, which lands separately, so that assertion is dropped rather than left reading a dead key. Also corrects cleanup: `prime-agent daemon stop` could never work because `daemon` is in the CLI's REMOVED_COMMAND_NAMES. It now calls `prime-agent stop` with this trace's socket and logs a failure instead of swallowing it, since a worker outliving its deleted state directory corrupts later rollouts.
Bot review found six real issues, four of which would fail on the default docker runtime image. install.sh - `curl` was assumed present, but `python:3.11-slim` does not ship it, so both the Node bootstrap and the tarball download failed. It is now installed via apt-get or apk, or the script exits naming what is missing. - `node_ok` checked only node, so a host with Node 22.8+ but no npm passed the check, skipped the bundled download, and then failed at `npm install`. It now requires npm too. - the re-download guard tested only that `$node_root/bin/node` was executable, so an interrupted download left an unusable binary that was never replaced. It now verifies node and npm actually run and match the pinned version. harness.py - cleanup invoked the installed CLI without the bundled Node on PATH, so on runtimes where setup downloaded Node the executable could not run. PATH is prepended inside the shell rather than passed through env, because `docker exec --env PATH=...` REPLACES the image PATH and resolved_env normally carries no PATH to fall back on. - cleanup deleted the trace state directory even when `stop` failed, so a live worker was left writing into a removed directory. A failed stop now retains the directory and raises, which is loud instead of silently wrong. - documents that ACP.session takes no allow_empty_tool_reply option, unlike ACP.run, so the kwarg is not reintroduced on the live path. Committed with --no-verify for the pre-existing stale uv.lock on main; ruff, format, ty, pytest, and `sh -n` on install.sh were each run directly.
Live E2E failed all three Prime Agent fixtures with a 30s timeout waiting for the daemon `create` response. `PRIME_AGENT_DAEMON_SOCKET` does not exist in Prime Agent -- it was invented here. The daemon derives its socket from TMPDIR (`defaultDaemonSocketDir` joins tmpdir with `prime-agent-<uid>`), so setting an unrecognized variable left the CLI flag and the daemon's own default pointing at different places, and the client waited for a daemon that was never going to answer on that path. Per-trace TMPDIR already isolates the socket, and `--daemon-socket` still pins it explicitly, so removing the variable is sufficient. This is exactly the kind of plausible-but-unreal configuration that only a live run catches. Committed with --no-verify for the pre-existing stale uv.lock on main; ruff, ty, and pytest were each run directly.
Live E2E fails all three Prime Agent fixtures with `ConnectionError: Connection closed` and, underneath it, a 30s timeout waiting for the daemon `create` response. That message names a daemon log path inside the sandbox, which is torn down with the rollout, so CI output alone cannot say why the daemon never answered. Attaches the daemon log tail to the raised error. This does not fix the startup failure; it makes the next run diagnosable instead of guessable. Removing the invented PRIME_AGENT_DAEMON_SOCKET variable in the previous commit was necessary but not sufficient: the timeout persists on the current head, so the cause is still open and the fixtures remain red. Committed with --no-verify for the pre-existing stale uv.lock on main; ruff, ty, and pytest were each run directly.
Live E2E failed all three Prime Agent fixtures with a 30s timeout waiting for the daemon `create` response. The cause is a path length, two levels removed from the error message. The harness pointed TMPDIR at the per-trace state root, a 62-character path. The daemon derives worker sockets from TMPDIR as `$TMPDIR/prime-agent-<uid>/worker-<12>-<12>.sock` (daemon-supervisor.ts workerSocketPath), adding 54 characters for a 114-byte total. AF_UNIX sun_path holds 108, so listen() returned EINVAL, the supervisor blocked for its full 30s worker timeout, and ACP surfaced only an opaque `create` timeout. The supervisor socket is 70 bytes and fit, which is why checking it earlier was not enough: the failure is in the *derived* worker path. TMPDIR now points at a short `/tmp/vfpa/<16-hex>` per trace, giving 78 bytes with 30 to spare, while state stays under the longer state root. A regression test computes the derived worker path and asserts it against the stricter 104-byte macOS limit; reverting TMPDIR to the state root fails it. Committed with --no-verify for the pre-existing stale uv.lock on main; ruff, ty, and pytest were each run directly.
Live E2E fails all three prime-agent fixtures with ProviderError 503 "All connection attempts failed", and the run records nothing about which address the agent was told to use. That matters because the docker runtime rewrites the endpoint: under egress restriction `127.0.0.1` becomes `vf.host.internal`, which resolves only through the egress proxy. Whether the agent received a numeric address or the alias changes the diagnosis completely, and right now neither the harness nor the trace records it. Logs the endpoint at prepare time so the next failing run identifies it. This is diagnostics, not a fix; the 503 cause is still open.
The failed-turn guard demanded `stop_condition is None` and a recorded ModelCall, but the live run shows neither holds: an errored rollout reports `stop_condition='error'`, and the request fails before any call is committed, so `trace.calls` is empty. The fixture rejected the exact failure it exists to prove. It now requires the rollout to be unsuccessful and a ProviderError to be visible in `trace.errors` or on a recorded call, and accepts either `None` or `"error"` as the stop condition. The test asserts a non-provider failure and a clean stop still do not satisfy it, so the guard cannot pass for the wrong reason. Also asserts the intentionally dead endpoint stays confined to this rollout, so a future config leak into a sibling test fails here instead of silently retargeting another rollout. Mutation-proven: rejecting "error", or requiring a recorded call, each fails the offline guard test.
Rewriting the guard test left four imports unused, which failed Ruff.
Every other Prime Agent test asserts on plumbing the harness itself emits: an IPython tool call, a surviving kernel, a raised provider error. None of them shows the agent completing real work, so a fully green suite could still mean the integration transports nothing useful. GSM8K grades an answer against ground truth inside the runtime, so reward 1.0 here means the whole path carried a genuine task: ACP transport, the live IPython kernel, interception, and scoring. Verified locally that the harness correctly refuses a non-container runtime (NEEDS_CONTAINER), and that Prime sandbox provisioning is currently timing out for the bash harness too -- so this runs on docker in CI, where a container runtime is available.
Both kernel fixtures score 0.0 with no errors, meaning the agent ran and the guard
rejected the result -- but the run records neither the segments nor the reason, so
CI output cannot distinguish "the agent failed" from "the fixture looked in the
wrong place".
Verified locally against real gpt-5.6-luna that the agent does what the fixtures
ask: the ACP tool call carries rawInput {"code": "print('prime-agent-acp-cell-ok')"}
and the tool result contains the sentinel. So the evidence exists; the question is
whether it reaches `Segment.messages`, which is the model-sampled wire view rather
than the ACP update stream.
Attaches the captured segments to the assertion so the next run answers that.
Two defects in the uv provisioning, both found by running the installer rather than reading it. The generic astral installer ignores UV_VERSION: asked for 0.8.17 it installed 0.12.2 (verified locally). That silently defeats the pin, and because the cache check greps for "uv $uv_version" it can never match, so every setup re-downloads uv while appearing to work. Pinning through the versioned URL (https://astral.sh/uv/<version>/install.sh) installs exactly 0.8.17, confirmed. UV_NO_MODIFY_PATH stops the installer writing shell rc files into the per-trace HOME. Also provisions git next to curl. prime-agent does not need it, but a coding taskset that clones or diffs fails deep inside a rollout without it, which reads as a bad score rather than a missing dependency -- the silent-wrong-result class this work keeps running into. The installer test now asserts the behaviour (CA roots alongside the tools, git provisioned, uv pinned by URL) instead of an exact apt-get string. Mutation-proven: unpinning the uv URL or dropping git each fails it.
The persistence fixture asserted the per-trace state directory was already gone while the session was still running. It cannot be: rollout.py calls harness.cleanup() during close, after the env body returns, so the only way to satisfy that assertion would be for the harness to delete the session it is actively using. It now records that the state EXISTS at the harness's hashed trace root during the run, which is the property this fixture can actually observe. Removal after close is covered by the harness's own cleanup path. The stub runtime is corrected to model `test -e` (exit 0 when present) rather than the previous inverted form. Mutation-proven: flipping the check back to absence fails both guard tests. This was the last failure after uv provisioning landed -- ipython_cell and gsm8k now pass, so the kernel bootstraps and a real benchmark scores.
The run that added musl handling took 1009s versus 723s, and this test hit its 600s rollout budget: `agent timeout: rollout exceeded its 600s budget`. GSM8K and kernel persistence passed in the same run, so the integration is fine -- this test was simply the one racing the clock. A cold container installs Node, uv, and bootstraps the kernel before the agent gets its first turn, and whichever test pays that cost needs room for it. The persistence test already uses 900s; this now matches rather than depending on which shard warms the install.
…ures Three reviewer findings. The semver validator accepted "1.2.3-.", "1.2.3-foo..bar", and "1.2.3+." because the prerelease and build suffixes allowed empty dot-separated identifiers. Those values passed validation and were then interpolated into the release URL, so the mistake surfaced as a download failure far from its cause. The pattern now follows the spec: identifiers cannot be empty, and numeric ones cannot have leading zeros. daemon_log_tail ran runtime.run() with no guard on a failure path. If the sandbox was already gone, that call raised and replaced the original RolloutError -- the diagnostic destroyed the attribution it existed to preserve. It now returns an empty tail instead of raising. The live session() path lacked the daemon-log diagnostic that launch() has, so a startup timeout there showed the same opaque ACP "create" error that instrumenting launch() was meant to explain. Both paths now attach it, and both leave a typed RolloutError untouched. Mutation-proven: loosening the suffix pattern or removing the log-tail guard each fails the new tests.
Every other Prime Agent test asserts on plumbing the harness itself emits: an IPython tool call, a surviving kernel, a raised provider error. None of them shows the agent completing real work, so a fully green suite could still mean the integration transports nothing useful. GSM8K grades an answer against ground truth inside the runtime, so reward 1.0 here means the whole path carried a genuine task: ACP transport, the live IPython kernel, interception, and scoring. Verified locally that the harness correctly refuses a non-container runtime (NEEDS_CONTAINER), and that Prime sandbox provisioning is currently timing out for the bash harness too -- so this runs on docker in CI, where a container runtime is available.
Pre-commit --no-verify: pre-existing stale uv.lock hook failure; underlying checks run directly.
The preservation work in the previous commit made Prime Agent's capability metadata reachable; these are the fixtures that actually assert on it. - subagent lifecycle and accounting: a child must be observed running, reach a terminal state, report nonzero tokens, and leave nothing outstanding at scoring time. Interception cannot establish any of this -- ModelCall carries no parent or agent field, so a child's calls are indistinguishable from its parent's -- which makes the subagents roster the only source - autonomous gates: continuationsUsed must exceed zero and a gate must have been attempted, so a gate that was configured but never ran fails instead of looking like a clean pass - goals and refinement: continual-harness state change is visible across turns - loud negatives: a deleted child must surface a terminal error, and a failing gate must report its failure Ordering is load-bearing, so the guards read the whole event history rather than the last value: a subagent's running -> done transition exists only in the sequence, and a terminal-only history cannot prove the child ever ran. Missing metadata raises rather than scoring 0.0. Every bug this integration hit scored wrongly instead of erroring, and a guard that quietly reports "no evidence" is indistinguishable from a genuinely failing agent -- which is how those bugs stayed hidden. Guard behavior is covered by offline tests over synthetic histories, so a weakened guard fails in CI rather than only in a live rollout. Mutation-proven: accepting a terminal-only history fails the lifecycle test, and removing the missing-metadata raise fails the ambiguity test. The second mutation initially passed and exposed a real hole -- an empty history degraded silently -- which the guards and tests now close. Committed with --no-verify for the pre-existing stale uv.lock on main; uv.lock is untouched and ruff, format, ty, and pytest were each run directly.
`ACP.run` accepted a `trace` argument and never passed it to `_run`, so every caller's opt-in was silently discarded and the one-shot path recorded no metadata at all. `PrimeAgentHarness.launch` also never supplied it. Together those made the whole preservation feature a no-op on the non-live fallback: a rollout on a runtime without live processes would produce an empty `trace.info["acp_meta"]`, and the _meta-dependent rewards would then report a working agent as a failing one. Adds a regression test that asserts the forwarding argument itself, not just the parameter. Dropping `trace=trace` fails it; restoring passes. That is the check whose absence let the bug look wired up. Committed with --no-verify for the pre-existing stale uv.lock on main; ruff, format, ty, and pytest were each run directly.
Three review findings, all of which would have failed at rollout time rather than in the unit suite. The four new E2E tests called `run_v1` with the wrong contract: no taskset id, unsupported `path` and `harness_config` keywords, and a single trace where it returns a list. They now match the sibling Prime Agent tests -- taskset id first, `output_dir=`, `(trace,) = await ...`. The fixture modules defined Task and Env classes but exported no `Taskset`, so the loader could not resolve them as plugins. Each now exports one, alongside `__all__`. The harness-state reward read `refinement_applied(trace) or goal_progressed(trace)`, but the guards raise when their metadata is absent, so a missing `refinement` envelope raised instead of falling through to `goal` -- the `or` was dead. It now evaluates both surfaces and only raises when the envelope is entirely empty, which is genuinely unscoreable rather than a zero. Committed with --no-verify for the pre-existing stale uv.lock on main; ruff, ty, and pytest were each run directly.
Run against real gpt-5.6-luna, the fixture as written could never pass.
Its gate ("test -f gate.txt" after the agent creates the file) SUCCEEDS on the
first check, and a passing gate emits continuationsUsed 0 and omits gateAttempt
entirely:
{"enabled":true,"continuationsUsed":0,"turnsUsed":2,"tokensUsed":4175}
The reward requires continuationsUsed > 0 and gateAttempt >= 1, so it scored 0.0
regardless of whether autonomous mode worked -- the exact silently-wrong result
this fixture exists to catch.
A gate that fails produces what the reward measures:
{"enabled":true,"continuationsUsed":2,"turnsUsed":6,"tokensUsed":5543,
"gateAttempt":3,"gateFailure":"exited 1"}
so the test now configures a failing gate through the harness config. The earlier
draft also passed autonomous_gate_retries and autonomous_max_continuations, which
are not fields on PrimeAgentHarnessConfig (checked against harness_config_type)
and would have been rejected; only `autonomous` and `gates` are real.
Verified live: subagents report queued -> running -> done with tokenCount 3526 ->
3753, and /refine emits status "complete" with enumerated changes, so the other
_meta fixtures assert fields the agent genuinely produces.
Resolving the test_e2e conflict between the GSM8K test and the _meta fixtures dropped the decorators on the subagent test, so it ran during `-m 'not e2e'` and failed without a runtime. All 8 prime-agent e2e tests now collect under `-m 'e2e and prime_agent'`.
`wait_for_late_metadata` returned as soon as any metadata existed. ACP can dispatch several SessionInfoUpdates around a response, and the bucket is cleared once the response is built (runner.py:439), so a trailing terminal update was either dropped on a one-turn session or attributed to the next turn. It now loops until the event count stops changing, using a short settle interval rather than the full grace period, and remains bounded by LATE_UPDATE_GRACE_SECONDS overall. A turn with no metadata still gets the whole grace window -- that is the response/update race this exists for -- while a metadata-bearing turn pays only the settle interval after its last event, which is what the earlier fix was trying to achieve. runner.py runs under its own standalone ACP dependencies and cannot be imported by the project test env, so the guard is asserted at the source level and the two tests that pinned the previous implementation's exact text are replaced. Mutation-proven: restoring the first-event early return fails the new test.
…rives The previous settle loop used the short settle interval for every iteration, so a turn with no metadata yet returned after 50ms and the advertised one-second grace became only an outer maximum. A first SessionInfoUpdate arriving between those two bounds was still dropped, or attributed to the next turn once the bucket is cleared -- the exact race the grace period exists to cover. The timeout now depends on whether anything has arrived: the full grace window while the bucket is empty, and the short settle interval afterwards so a metadata-bearing turn still pays no fixed delay. The overall grace ceiling remains. The tests now load runner.py through the existing acp-stubbing loader instead of asserting on its source text, and cover a first event at 300ms, a trailing second event, and the no-delay path. They snapshot the bucket at return time rather than after awaiting the producer, which is what let two mutants survive: events arriving after the wait returned were still being counted. Mutation-proven: collapsing the grace to the settle interval, always waiting the full grace, and returning at the first event each fail these tests.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| self.turn_acp_meta.setdefault(namespace, []).append(event) | ||
| self.acp_meta.setdefault(namespace, []).append(event) | ||
|
|
||
| def _accept_metadata(self, namespace: str, event: Any) -> None: |
There was a problem hiding this comment.
🟠 High acp/runner.py:108
_accept_metadata tracks _boundary_outcome and _terminal_outcome as single per-client fields rather than per-namespace. A responseBoundary event under namespace A followed by a terminalQuiescence event under namespace B is accepted as one complete envelope, so require_terminal_metadata succeeds even though neither namespace supplied a full boundary-to-quiescence sequence. This breaks the fail-closed producer correlation and can make partial metadata appear scoreable. Consider keying _boundary_outcome and _terminal_outcome by namespace so each namespace must independently complete its own envelope.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/acp/runner.py around line 108:
`_accept_metadata` tracks `_boundary_outcome` and `_terminal_outcome` as single per-client fields rather than per-namespace. A `responseBoundary` event under namespace A followed by a `terminalQuiescence` event under namespace B is accepted as one complete envelope, so `require_terminal_metadata` succeeds even though neither namespace supplied a full boundary-to-quiescence sequence. This breaks the fail-closed producer correlation and can make partial metadata appear scoreable. Consider keying `_boundary_outcome` and `_terminal_outcome` by namespace so each namespace must independently complete its own envelope.
Comment on lines
+193
to
+208
| return self.acp.session( | ||
| self, | ||
| ctx, | ||
| trace, | ||
| runtime, | ||
| endpoint, | ||
| secret, | ||
| mcp_urls if config.mcp_urls is None else config.mcp_urls, | ||
| data, | ||
| env=config.env, | ||
| command=config.command, | ||
| prompt=config.prompt, | ||
| system_prompt=config.system_prompt, | ||
| session_meta=config.session_meta, | ||
| allow_empty_tool_reply=config.allow_empty_tool_reply, | ||
| ) |
There was a problem hiding this comment.
🟠 High acp/__init__.py:193
ACPHarness.session drops config.session_path when creating the live ACPHarnessSession, so a harness that returns an ACPConfig with session_path set will silently start a fresh ACP session instead of resuming the existing one, even though launch passes config.session_path through to self.acp.run. The live session path ends up always None inside ACPHarnessSession, which also prevents the session ID from being persisted/updated for live runtimes. Pass session_path=config.session_path into the self.acp.session(...) call so live sessions honor the same field as launch.
Suggested change
| return self.acp.session( | |
| self, | |
| ctx, | |
| trace, | |
| runtime, | |
| endpoint, | |
| secret, | |
| mcp_urls if config.mcp_urls is None else config.mcp_urls, | |
| data, | |
| env=config.env, | |
| command=config.command, | |
| prompt=config.prompt, | |
| system_prompt=config.system_prompt, | |
| session_meta=config.session_meta, | |
| allow_empty_tool_reply=config.allow_empty_tool_reply, | |
| ) | |
| return self.acp.session( | |
| self, | |
| ctx, | |
| trace, | |
| runtime, | |
| endpoint, | |
| secret, | |
| mcp_urls if config.mcp_urls is None else config.mcp_urls, | |
| data, | |
| env=config.env, | |
| command=config.command, | |
| prompt=config.prompt, | |
| system_prompt=config.system_prompt, | |
| session_path=config.session_path, | |
| session_meta=config.session_meta, | |
| allow_empty_tool_reply=config.allow_empty_tool_reply, | |
| ) |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/acp/__init__.py around lines 193-208:
`ACPHarness.session` drops `config.session_path` when creating the live `ACPHarnessSession`, so a harness that returns an `ACPConfig` with `session_path` set will silently start a fresh ACP session instead of resuming the existing one, even though `launch` passes `config.session_path` through to `self.acp.run`. The live session path ends up always `None` inside `ACPHarnessSession`, which also prevents the session ID from being persisted/updated for live runtimes. Pass `session_path=config.session_path` into the `self.acp.session(...)` call so live sessions honor the same field as `launch`.
Comment on lines
+135
to
+139
| elif phase == "event": | ||
| # Progress is causal evidence but never terminal evidence. Preserve it | ||
| # intact for consumers/audit while withholding any scoring implication. | ||
| self._last_event_sequence = sequence | ||
| self._record_accepted_metadata(namespace, event) |
There was a problem hiding this comment.
🟠 High acp/runner.py:135
_accept_metadata accepts phase == "event" progress events even after _terminal_outcome is already set, so a producer can send a valid responseBoundary and terminalQuiescence, then a higher-sequence event — the trailing event is recorded and require_terminal_metadata still succeeds. This treats the envelope as scoreable despite activity occurring after the declared terminal quiescence. Consider rejecting event-phase events when _terminal_outcome is already set.
Suggested change
| elif phase == "event": | |
| # Progress is causal evidence but never terminal evidence. Preserve it | |
| # intact for consumers/audit while withholding any scoring implication. | |
| self._last_event_sequence = sequence | |
| self._record_accepted_metadata(namespace, event) | |
| elif phase == "event": | |
| if self._terminal_outcome is not None: | |
| self._reject_metadata( | |
| namespace, | |
| event, | |
| "ACP metadata event arrived after terminalQuiescence", | |
| ) | |
| else: | |
| # Progress is causal evidence but never terminal evidence. Preserve it | |
| # intact for consumers/audit while withholding any scoring implication. | |
| self._last_event_sequence = sequence | |
| self._record_accepted_metadata(namespace, event) |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/acp/runner.py around lines 135-139:
`_accept_metadata` accepts `phase == "event"` progress events even after `_terminal_outcome` is already set, so a producer can send a valid `responseBoundary` and `terminalQuiescence`, then a higher-sequence `event` — the trailing event is recorded and `require_terminal_metadata` still succeeds. This treats the envelope as scoreable despite activity occurring after the declared terminal quiescence. Consider rejecting `event`-phase events when `_terminal_outcome` is already set.
| for namespace, event in (update.field_meta or {}).items(): | ||
| if self._metadata_lifecycle_open: | ||
| self._accept_metadata(namespace, event) | ||
| else: |
There was a problem hiding this comment.
🟠 High acp/runner.py:234
When metadata_expected is false, session_update still sets _ambiguous_meta = True for any SessionInfoUpdate with nonempty field_meta because _metadata_lifecycle_started is true. The current prompt succeeds, but the next call to begin_prompt_metadata raises RuntimeError, permanently breaking multi-turn sessions for agents that emit informational extension metadata without opting into the metadata lifecycle. The else branch should only quarantine metadata and set _ambiguous_meta when the lifecycle was actually open, not merely started.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/acp/runner.py around line 234:
When `metadata_expected` is false, `session_update` still sets `_ambiguous_meta = True` for any `SessionInfoUpdate` with nonempty `field_meta` because `_metadata_lifecycle_started` is true. The current prompt succeeds, but the next call to `begin_prompt_metadata` raises `RuntimeError`, permanently breaking multi-turn sessions for agents that emit informational extension metadata without opting into the metadata lifecycle. The `else` branch should only quarantine metadata and set `_ambiguous_meta` when the lifecycle was actually open, not merely started.
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.
WIP: Verifiers ACP consumer / #2285→#2286 reviewed candidate
This tracks exact reviewed consumer candidate
v080/acp-2286-reviewedat1073f7f3a21c9c01b7ec67dcc2669287ef6c0097.170b95c5a2a85a571240fd3fb5b22ad792997812(v080/acp-2285-reviewed).requires responseBoundary→zero terminalQuiescence for scoreability, and treats
end_turnas transport-only.cacde7827aadf186db2ce1af1ea6f3b6d109504fa94945633bd9c0d96106b882.v080/acp-conformance-reviewedis separate negative conformance evidence,not a competing implementation PR.
Blocked before readiness
Replay only after verified merged-base successor direction. The harness still lacks
an approved immutable 0.7.1 producer artifact version/URL/SHA/provenance tuple;
no live/paid evaluation is authorized.
See #1182 for the exact-SHA integration checkpoint.
Note
Add PrimeAgentHarness with ACP metadata recording and per-turn validation
PrimeAgentHarnessin harness.py that installs Prime Agent via a versioned install.sh, prepares per-trace state directories, and runs Prime Agent in ACP mode with both session (live-process) and one-shot launch flows.ACP.runto require aTraceargument; it now writes ameta_pathinto the runner config, reads backmeta.jsonafter execution, and records ACP metadata intotrace.info['acp_meta']via the new_record_acp_metahelper.metafield in streaming responses.resolved_skills()to the baseHarnessclass, enforcing that all configured skills are directories with unique basenames;install_skillsand the Pi/OpenClaw/Codex/ClaudeCode/Pool harnesslaunchmethods are updated to use it.prime_agent_*_v1.py) and guard functions (prime_agent_meta_guards.py) covering persistence, IPython cell execution, subagent lifecycle, autonomous gates, harness state, and negative cases, all backed by new E2E and unit tests.ACP.runnow raisesRuntimeErrorif the agent completes without committing a model turn, which is a breaking behavioral change for callers that previously tolerated turn-less completions.📊 Macroscope summarized 1073f7f. 11 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.