feat(pi): add Pi harness (--type pi) with docker + skill injection - #159
feat(pi): add Pi harness (--type pi) with docker + skill injection#159CarlesUIPath wants to merge 24 commits into
--type pi) with docker + skill injection#159Conversation
Add the Pi harness's config surface: AgentKind.PI enum member, a 7-value PiThinkingLevel literal (strict superset of ThinkingLevel), PiAgentConfig in the AgentConfig discriminated union, and the models/__init__ export. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drive the pi Node CLI in JSON print mode over a subprocess, reducing its nd-JSON stream into the standardized event protocol + TurnRecord via EventCollector (mirrors OpenCodeAgent). Native turn cap off turn_start, cooperative should_stop, turn_timeout, crash/timeout pending_turn capture, per-agent --session-dir/--session-id continuity for simulation, and enforced allowed_tools/disallowed_tools/system_prompt. Registered via register_builtins; pi = [] extra added. Offline replay tests seeded from a byte-real 0.84.4 capture. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirror tasks/opencode_smoke_test.yaml for the Pi harness: type pi, the spike-verified kimi-k3 model, three criteria (file_exists / file_contains / run_command), and no smoke-pass tag (run locally — CI E2E lacks the pi CLI + OpenRouter creds). A loader test asserts it resolves to PiAgentConfig. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
New docs/agents/PI.md (mirrors OPENCODE.md), a filled pi column + divergence prose in HARNESS_PARITY.md, mkdocs nav + blurb (regenerated index tables), and every prose enumeration of the harness set (README, index, USER_GUIDE, EXTENDING, llms.txt, CLAUDE.md) extended to include Pi. Fix the stale agent.type comment in experiments/default.yaml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Harden PiAgent.start() to drop any prior session tempdir before creating a new one, so re-starting the same agent instance cannot leak (defensive; agents are single-use in-tree). Add a regression test. - Correct the PI.md Overview snippet to use --model (matching the code) instead of -m. - Defer a CExxx harness candidate: dead _*TurnState accumulator detection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The shared config default (experiments/default.yaml) sets Claude-namespaced tool
names (Bash/Read/Write/Edit/Glob/Grep/Skill). Pi's built-in tools are lowercase
and differently named (bash/read/write/edit/grep/find/ls), so forwarding the
PascalCase names to `--tools` allowlisted nonexistent tools and left the agent
with ZERO tools ("I don't have tool access") — the end-to-end smoke created no
files and scored 0/3.
Treat allowed_tools/disallowed_tools as unenforced (add to
_UNSUPPORTED_CONFIG_FIELDS, warn at start()), matching OpenCode/Codex/Antigravity,
so Pi runs with its full native toolset. system_prompt stays enforced
(--append-system-prompt). Updates the two encoding tests and the PI.md /
HARNESS_PARITY.md enforcement claims.
E2E smoke now passes 3/3 (score 1.000).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code review (multi-model) found the tool-forwarding removal (c79f5cb) left three doc surfaces claiming allowed_tools/disallowed_tools are enforced: - H1: PiAgentConfig docstring (models/agent_config.py) - H2: CLAUDE.md harness-parity bullet ("ENFORCES all three") - M1: pi_agent.py module-docstring command synopsis (--tools/--exclude-tools) All corrected: only system_prompt is enforced; the tool allowlists are ignored (Claude-namespaced default can't map to Pi's lowercase toolset), matching the shipped code + PI.md + HARNESS_PARITY.md. Also: - M2: fix _resolve_cost docstring to match the code (rate-card fallback on a streamed $0 for a priced model is a deliberate conservative choice — never understate max_usd budget gates); demote the expected warning to debug. - L1: correct the session-dir comment (removed in stop(), NOT kill()). - L2: tighten test_should_stop_ends_turn_cleanly to pin the cut at turn 0. No harness correctness bugs were found by the review. Tests 75/75, make lint 389/389, e2e smoke 3/3 (score 1.000). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a HARNESS_LOGO entry for agent.type "pi" (AgentKind.PI): the Pi mark (public/harness/pi.png, 128x128, rasterized from pi.dev's favicon.svg) plus short label "Pi" and tooltip "Pi · pi.dev". The badge + selector are already data-driven, so a Pi run now renders the logo and name instead of the raw id. Adds harness-badge test cases for the "pi" short label and alt text. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-message Cost column priced each row purely from the rate card
(messageCostUsd = per-message tokens x list rate). A harness that owns its
provider and reports a REAL per-turn total_cost_usd on a model absent from the
rate card (Pi on open-weight models like kimi-k3 / deepseek) priced every row
null, so the column dashed out ("—") even though the turn had a real bill —
while codex (gpt-5.6-terra, in the card) showed numbers.
parseMessages now apportions a turn's real token_usage.total_cost_usd across its
rows by token share whenever the rate card priced nothing for that turn. The
rows then show real numbers that sum EXACTLY to the turn total (last row absorbs
the rounding residual), and so to the task total. Priced harnesses
(Claude/Codex/Bedrock) already have per-row figures and are untouched — the pass
only fills the gap, never overrides. Display-only: the authoritative task total
still reads the backend aggregate, so no double-count.
Verified on a real Pi run: 3 rows that were "—" now read $0.002915/$0.003074/
$0.003325, summing to exactly the $0.0093141 turn total. Adds 3 parseMessages
tests and updates the per-message cost tooltip.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add ARG PI_VERSION=0.84.4 + `npm install -g @earendil-works/pi-coding-agent` after the claude-code layer, mirroring the CLAUDE_CODE_VERSION pinned-CLI pattern (Node 22 + npm already present). Correct the lines 61-66 comment so Pi is named as baked and OpenCode remains the sole not-baked example. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add OPENROUTER_API_KEY to DockerDriverConfig.env_passthrough default so a `--driver docker --type pi` run reaches OpenRouter without a per-task env_passthrough_extra, mirroring the CODEX_API_KEY / GEMINI_API_KEY precedent for the other baked agents. Add a membership test that reads the model default rather than a hardcoded list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rewrite PI.md "Running in Docker" (Pi is supported under --driver docker; baked CLI + OPENROUTER_API_KEY passthrough; docker recommended for untrusted/adversarial runs since permission_mode is unenforced). Drop the now-false "docker sandbox driver is unsupported" limitation bullet. Update the pyproject.toml `pi` extra comment to note the CLI is baked into the docker image. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add test_pi_cli_baked_and_pinned: assert docker/Dockerfile carries an
`ARG PI_VERSION=<exact>` (never `latest`) and an
`npm install -g @earendil-works/pi-coding-agent@${PI_VERSION}` line that
references the ARG, so the pin and the install cannot drift. Mirrors the
existing claude-code pinned-CLI guard; scoped to the main Dockerfile only
(Dockerfile.runtime deliberately does not bake Pi).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…hain docs Code-review follow-ups (both Low, no correctness bugs found): - The Dockerfile comment (and OPENCODE.md) said the OPENROUTER_API_KEY allowlist is on `SandboxConfig.env_passthrough`; the field is actually on `DockerDriverConfig.env_passthrough` (SandboxConfig has no such field). A maintainer grepping SandboxConfig would find nothing. Corrected both. - docs/DOCKER_ISOLATION.md and docs/tutorials/06 listed the baked toolchain but omitted the now-baked Pi CLI (and the codex/antigravity/litellm SDKs). Made both lists accurate. Comment/docs-only; the built image and the pinned Pi layer are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…andling - Resolve agent.plugins -> skills dirs via the shared _plugin_skill_dirs resolver (now harness-labeled) and forward each as `pi --skill <dir>`; drop `plugins` from the unsupported set. Records pi_skill_paths in environment_info so a report can confirm the skill under test reached the agent. - Close a dangling TurnStartEvent when a new turn_start arrives without a prior turn_end (the willRetry mid-turn abort), preserving the one-pair-per-inner-turn contract renderers depend on. - Surface a terminal provider error (stopReason=error) in the result, and reset it on a recovered turn so an intermediate retry error never leaks. - Warn once per turn on token-accounting drift (bad bucket type / missing usage object) instead of silently zeroing the turn's tokens and cost. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
--type pi) with docker + skill injection--type pi) with docker + skill injection
The captured session event embedded the author's machine-specific cwd (UID + scratchpad path). The harness never reads session.cwd, so this is inert, but it should not ship in a public fixture — replace with /work. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # README.md # docs/index.md # docs/llms.txt
…d the real CLI test_skill_paths_recorded_in_environment_info declared the patch_exec fixture but never invoked it, so start() hit the real `pi`-on-PATH check and failed on CI runners (Quality Gate + Windows Smoke) that have no pi binary. Call patch_exec() like the sibling skill-injection tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
uipreliga
left a comment
There was a problem hiding this comment.
Review: coder_eval — pr:159
Scope: pr:159 · branch feat/pi-harness (PR #159) · 24a006a · 2026-09-08T16:27Z · workflow variant
Change class: complex — introduces a whole new agent harness (--type pi): a 1182-line streaming/turn state machine, new Pydantic config surface, docker image plumbing and evalboard wiring; correctness requires reasoning about the event protocol, turn lifecycle and parity contracts
Architecture (10/10) and security (9.8/10) are solid and the new Pi harness is structurally faithful to its OpenCode sibling, but risk is concentrated in that new agent: an unsanitized task_id in --session-id makes every dataset-row Pi task die before doing work, a terminal provider error is booked as a clean turn (FAILURE instead of ERROR), a missing find→Glob mapping zeroes canonical tool criteria, and Test Health (6.4/10) is too thin to catch any of them — so this is mergeable only after the score-changing Pi defects and the ported teardown/cost tests land.
Summary
| Axis | Score | 🔴 | 🟠 | 🟡 | 🔵 | Top Issue |
|---|---|---|---|---|---|---|
| 1. Code Quality & Style | 8.4 / 10 | 0 | 1 | 1 | 1 | The nd-JSON-CLI agent scaffold is duplicated into pi_agent.py instead of hoisted into a shared base (24-26 co-named near-identical methods, plus a cross-agent import of OpenCode's private _plugin_skill_dirs) |
| 2. Type Safety | 8.8 / 10 | 0 | 1 | 0 | 2 | CE047's substring roster guard is vacuous for the 2-char name "Pi", so multiple onboarding surfaces (comparison.md, pages-stub, pyproject description/keywords, README Quick Start table + non-goals, docs/index.md) ship a stale four-harness roster undetected |
| 3. Test Health | 6.4 / 10 | 0 | 2 | 3 | 1 | No live-CLI process double: the fake process exits the instant it is awaited, so orphan reap/kill, CancelledError handling, SIGKILL escalation, post-EOF reap, leaked-pipe drain and the loop-top deadline cut are all untested |
| 4. Security | 9.8 / 10 | 0 | 0 | 0 | 2 | New third-party npm CLI baked into the shared docker image is outside every dependency-audit gate |
| 5. Architecture & Design | 10 / 10 | 0 | 0 | 0 | 0 | — |
| 6. Error Handling & Resilience | 8 / 10 | 0 | 2 | 0 | 0 | A terminal Pi provider error (stopReason: "error") is scored as a clean COMPLETED turn instead of crashing, so infrastructure failures are booked as agent failures |
| 7. API Surface & Maintainability | 8.4 / 10 | 0 | 1 | 1 | 1 | Pi's plugins -> --skill injection is live in code but documented as absent in five places across four surfaces (PI.md, HARNESS_PARITY.md, CLAUDE.md, PiAgentConfig docstring) |
| 8. Evaluation Harness Quality | 8.4 / 10 | 0 | 1 | 1 | 1 | Unsanitized task_id is interpolated into pi's path-shaped --session-id, so every dataset-row task (id contains '/') fails before any work |
Overall Score: 8.5 / 10 · Weakest Axis: Test Health at 6.4 / 10
Totals: 🔴 0 · 🟠 8 · 🟡 6 · 🔵 8 across 8 axes.
Blockers
- [Axis 1] The nd-JSON-CLI agent scaffold is duplicated into pi_agent.py instead of hoisted into a shared base (24-26 co-named near-identical methods, plus a cross-agent import of OpenCode's private _plugin_skill_dirs) (
src/coder_eval/agents/pi_agent.py:888) — Hoist the shared nd-JSON-CLI driver into one place (e.g.agents/_cli_stream.pyor anNdJsonCliAgentbase holdingcommunicate's spawn/read/settle loop,kill/kill_sync/_sweep_process_groups/_reap_orphaned_cli,_crash_turn/_timeout_turn,_warn_token_shape,_rate_card_cost,_close_tool,_RESULT_STATUS,_TERM_GRACE_SECONDS,_DRAIN_SECONDS,_MAX_UNRECOGNIZED_TYPES), leaving each agent only its event grammar and token semantics. Measured: 420 of 780 non-comment code lines in pi_agent.py sit in 23 same-named methods that are >=60% line-similar to opencode_agent.py's, 11 of them byte-identical after stripping comments —communicate(pi:888 vs opencode:1133) is 0.98 similar over 117 code lines, andkill/kill_sync/_sweep_process_groups(pi:772-805) are a verbatim copy of opencode_agent.py:994-1023. No other agent pair in the roster comes close (largest existing overlap in >=8-line identical blocks: codex<->antigravity at 84 lines; pi<->opencode is 457). The PR itself records the decision: pi_agent.py:14-15 "The design mirrors :mod:coder_eval.agents.opencode_agentalmost verbatim" and pi_agent.py:119-121 "Genuinely OpenCode-module-private (not exported), so re-declared here at the same value rather than imported (a shared-module hoist is out of scope)" — while pi_agent.py:78 nevertheless doesfrom coder_eval.agents.opencode_agent import _plugin_skill_dirs, so the module already reaches into OpenCode's privates; the hoist is the consistent choice, not an extra one. - [Axis 2] CE047's substring roster guard is vacuous for the 2-char name "Pi", so multiple onboarding surfaces (comparison.md, pages-stub, pyproject description/keywords, README Quick Start table + non-goals, docs/index.md) ship a stale four-harness roster undetected (
tests/lint/agent_roster_parity.py:57) —missing_agents_inmatches a display name by case-insensitive SUBSTRING (line 102:if not any(name.lower() in haystack for name in AGENT_DISPLAY_NAMES.get(kind, (kind,)))), and the new entry at line 57 is the two-character token"Pi". Every English/marketing surface contains that bigram, so the pi row can never fail. I replayed the exact rule over all sevenROSTER_SURFACESat PR HEAD: CE047 reportsmissing: []for every one, yetre.search(r"\bPi\b", region)is False for three of them —pyproject.toml(satisfied by "anthropic" inkeywords; itsdescriptionstill reads "(Claude Code, Codex, Gemini/Antigravity, OpenCode)" with no Pi and there is nopikeyword),docs/comparison.md(satisfied by "ci-pipeline.md"; lines 12-13, 28 and 126 list four harnesses), and.github/pages-stub/index.html(satisfied by "copies"; lines 15 and 230 list four harnesses). That is verbatim the defect the rule's own docstring says it exists to prevent — "exactly how OpenCode shipped while being absent from most of them" (lines 6-11) — reproduced for Pi with the gate green. Fix in two parts: (1) anchor the match on word boundaries, e.g.re.search(rf"\b{re.escape(name)}\b", haystack, re.IGNORECASE)inmissing_agents_in, and add a guard test that a short spelling like"Pi"does NOT match"anthropic"/"ci-pipeline"/"copies"; when flipping to\b, re-run all seven surfaces and add"claude-code"as an accepted spelling for theclaude-coderow (\bClaude Code\bwill not match the hyphenatedclaude-codeform used inpyproject.toml's keywords). (2) actually name Pi inpyproject.toml'sdescription+keywords,docs/comparison.md, and.github/pages-stub/index.html. - [Axis 3] No live-CLI process double: the fake process exits the instant it is awaited, so orphan reap/kill, CancelledError handling, SIGKILL escalation, post-EOF reap, leaked-pipe drain and the loop-top deadline cut are all untested (
tests/test_pi_agent.py:596) —_FakeProcess.wait()(tests/test_pi_agent.py:111-113) isasync def wait(self): self.returncode = self._final_returncode; return self.returncode— it reports an exit the first time it is awaited, andcommunicate()awaits it asexit_waiteron every turn._ExplodingProcess(line 596) inherits it, so by the time thefinallyrunsproc.returncodeis 0 and_reap_orphaned_clireturns atif proc is None or proc.returncode is not None: return(pi_agent.py:1049-1050). I proved this empirically: re-runningtest_stream_error_becomes_a_crash_with_partial_parkedwith an addedassert proc.killed is Truefails —AssertionError: orphan reap never ran / assert False is True. Consequently these stay uncovered:proc.kill()+self._sweep_process_groups()(pi_agent.py:1051-1053), the wholeexcept asyncio.CancelledError: self._finalize_external_cancel(...); self._capture_partial_turn(...); raisehandler (pi_agent.py:1024-1026, no test ever cancels thecommunicate()task), andkill()'s escalationif proc.returncode is None: ... proc.kill()(pi_agent.py:780-781). The consequence is written in the method's own docstring: "AgentCrashErroris retried, so attempt 2 would spawn a SECONDpiediting the very files the criteria are about to score." The OpenCode twin this module says it mirrors already solved this and documents exactly this trap —tests/test_opencode_agent.py:1542class _ExplodingRunningProcess(_HangingProcess): "_ExplodingProcessinherits the plain fake'swait(), which reports an exit code the instant it is awaited — so it can never model the case that matters for teardown." Port_ExplodingRunningProcess,TestTurnAlwaysReapsTheCli(test_opencode_agent.py:1733-1795, incl.test_external_cancel_kills_the_cliandtest_a_clean_turn_kills_nothing) andTestExternalCancel(1621-1646) to the Pi suite, assertingproc.killed is True, oneAgentEndEventwithcrash_reason == "turn cancelled", and a parkedcrashed=Truepending_turn. - [Axis 3]
_resolve_cost's two rate-card fallbacks are never asserted, so nothing pins the value that feedsmax_usdand the cost report (tests/test_pi_agent.py:204) —grep -n "total_cost_usd\|rate card\|calculate_cost" tests/test_pi_agent.py tests/test_pi_agent_config.py tests/test_pi_smoke_task.pyreturns exactly one hit — line 204,assert usage.total_cost_usd == pytest.approx(EXPECTED_COST)— which only covers the "stream cost wins" leg._resolve_cost(pi_agent.py:590-600) has two more legs, both load-bearing per its own docstring ("understating cost would silently defeatmax_usdbudget gates"):if not self.saw_cost: return rate(591-592) andif self.cost_usd == 0.0 and rate: ... return rate(593-599). Both merely execute incidentally — the shared_turn_end(...)helper defaultscost: float = 0.0and the test model IS priced (calculate_cost('openrouter/moonshotai/kimi-k3', uncached_input_tokens=10, output_tokens=5, ...)returns0.000105), so the $0-fallback runs in most tests with no assertion on the result. A regression that returnsself.cost_usdunconditionally would keep all 84 tests green while silently booking $0 for every Pi run and blindingrun_limits.max_usd(FinalStatus.COST_BUDGET_EXCEEDED). The OpenCode twin has the full matrix —tests/test_opencode_agent.py:386-450class TestCostFallsBackToTheRateCardwith five cases (stream cost wins / missing cost priced from the rate card / unpriced model reportsNonenot 0.0 / zero reported cost on a priced model uses the rate card and logs "not understated" / zero on an unpriced model stays 0.0). Port those five cases against_turn_endstreams with thecostkey omitted, withcost.total = 0, and withmodel="nowhere/not-a-real-model". - [Axis 6] A terminal Pi provider error (
stopReason: "error") is scored as a clean COMPLETED turn instead of crashing, so infrastructure failures are booked as agent failures (src/coder_eval/agents/pi_agent.py:1112) —_PiTurnState.on_turn_endcaptures the terminal provider error at lines 519-523 (self.error_message = err if isinstance(err, str) and err else "pi reported stopReason=error") — its own comment at 515-516 says this exists because "api -pthat exits 0 after exhausting retries still surfaces WHY" — but_settle_turnnever consults it: after the returncode guard (1090) and the vocabulary guard (1098) it falls straight through toreturn AgentEndStatus.COMPLETED(1112).finalizethen emitscrashed=Falseand only parks the text inResultSummary(result=...). The sibling harness this module says it "mirrors almost verbatim" does the opposite —opencode_agent.py:1378-1379:if state.error_message is not None: self._crash_turn(state, collector, f"OpenCode error: {state.error_message}"). FAILURE SCENARIO: OPENROUTER_API_KEY is expired; pi runs its internal retries, emitsturn_endwithmessage.stopReason == "error"/errorMessage: "401 unauthorized", thenagent_settled, and exits 0._settle_turnreturns COMPLETED,crashed=False, so_communicate_with_retrynever retries the transient error and the task is graded on an empty sandbox — recorded asFinalStatus.FAILURE, whosecategoryis"failed"(models/enums.py:36), instead ofFinalStatus.ERROR->"error"(line 31), which reports exclude from task outcomes. Provider outages therefore silently depress the measured pass rate. FIX: crash (or at minimum classify) on a non-Nonestate.error_messagein_settle_turn, mirroring OpenCode, and updatetests/test_pi_agent.py::test_terminal_error_is_surfaced_in_result(which currently assertsrecord.crashed is False, line 878) plusdocs/agents/HARNESS_PARITY.mdif a deliberate divergence is really wanted. - [Axis 6] Pi's turn_end usage telemetry is never validated: all-zero or renamed token buckets finalize as a clean COMPLETED turn, the PI.md-promised warning never fires, and max_usd/max_total_tokens go blind (
src/coder_eval/agents/pi_agent.py:1098) —_settle_turn's only telemetry guard isif not stopped_early and not state.max_turns_exhausted and state.recognized_events == 0:(line 1098). OpenCode deliberately added a second arm for exactly this gap —opencode_agent.py:1409-1411:nothing_recognized = state.recognized_events == 0;finished_without_tokens = state.steps_finished > 0 and state.usage.is_empty();if ... and (nothing_recognized or finished_without_tokens)— with the comment that keying onrecognized_eventsalone "left the identical outcome reachable one layer down". Pi dropped that arm (the PR's own.claude/harness-candidates.mdentry notesturns_finishedwas removed as dead because "Pi deliberately dropped that guard"), and the compensating warn does not cover the drift shape:_as_intreturns 0 silently for a missing key (lines 461-462,if value is None: return 0) and_warn_token_shapefires only when the wholeusageobject is missing/empty (line 483) or a bucket has a bad type (line 463). So ausagedict whose keys were renamed — or one that legitimately reports zeros — logs nothing at all, contradictingdocs/agents/PI.md:212("the turn is scored and a warning is logged"); the new testtests/test_pi_agent.py:823-834shows it, feeding a full zero-valued usage dict via_turn_end(inp=0, out=0)and asserting onlyrecord.crashed is False, with no warning assertion because none is emitted. FAILURE SCENARIO: a Pi upgrade renamesusage.input/output/cacheRead/cacheWriteand movescost; every event type is still recognized so line 1098 does not fire, each bucket resolves to 0 with no warning,saw_coststays False so_resolve_costreturns None, andEventCollector.build_turn_recordmaps the all-zero usage totoken_usage=None(streaming/collector.py:183-185) — every turn reports 0 tokens and $0,run_limits.max_usd/max_total_tokenscan never trip, and nothing names the drift. FIX (keeps the documented score-don't-crash policy): warn once when aturn_endthat carried a usage object yields all-zero buckets — restore an equivalent ofsteps_finished > 0 and state.usage.is_empty()as a warn condition in_settle_turn, with a test asserting the log line. - [Axis 7] Pi's plugins -> --skill injection is live in code but documented as absent in five places across four surfaces (PI.md, HARNESS_PARITY.md, CLAUDE.md, PiAgentConfig docstring) (
docs/agents/PI.md:238) — The PR's own commit91d67230 feat(pi): load agent.plugins skills via --skilllanded AFTER the docs commit1e4b61a8 docs(agents): 4/4 …, so every doc surface still describes the pre-feature state. Code (authoritative):src/coder_eval/agents/pi_agent.py:740self._skill_dirs = _plugin_skill_dirs(self.config.plugins, log=logger, harness="pi"),:850-854for skill_dir in self._skill_dirs: … argv += ["--skill", skill_dir],:179-180# NOTE … andpluginsIS supported (each resolved skills dir is\n# mapped to a--skillargument), so neither is here., andtests/test_pi_agent.py:436"""plugins IS supported now (-> --skill) …"""/:938assert "--skill" in argv. The four stale surfaces: (a)docs/agents/PI.md:238-244— "plugins/ skills are not injected. Pi has a native--skillflag, but v1 does not wireagent.plugins → --skill… Consequence: Pi cannot run activation suites in v1. Follow-up: map each resolved skill dir to a--skill <dir>arg and add a Pi branch toskill_triggered." (theskill_triggeredbranch is also unnecessary —criteria/skill_triggered.py:41,73-75matchesskills/<name>/in ANY string tool parameter, agent-agnostically); (b)docs/agents/PI.md:134-135— "permission_mode,plugins, andsystem_prompt_fileare unenforced too"; (c)docs/agents/HARNESS_PARITY.md:176-181— "Pi does not readpluginsat all in v1: it warns and ignores them, so it cannot run activation suites yet"; (d)src/coder_eval/models/agent_config.py:419-420(the publicPiAgentConfigdocstring) — "pluginsare NOT injected (no activation suites in v1)"; (e)CLAUDE.md:147— "but does NOT readplugins(warned+ignored; no activation suites in v1, though it has a native--skillflag)". Fix: rewrite all five to state thatpluginsIS honored for its skills half (each resolved skills dir → one--skill <dir>, recorded aspi_skill_pathsinenvironment_info), that Pi CAN therefore run activation suites, and add apicolumn to theagent.plugins[].pathdepth table atdocs/agents/HARNESS_PARITY.md:127-130(Pi reuses OpenCode's_plugin_skill_dirs, so it requires the plugin-root shape, not the bare skills dir). - [Axis 8] Unsanitized task_id is interpolated into pi's path-shaped --session-id, so every dataset-row task (id contains '/') fails before any work (
src/coder_eval/agents/pi_agent.py:758) —start()builds the CLI identifier asself._session_id = f"coder-eval-{self.task_id}-{uuid4().hex[:8]}"(pi_agent.py:758) and hands it to the CLI verbatim as--session-id(pi_agent.py:843-844), alongside--session-dir <tempdir>.task_idis path-special on every dataset-backed task:orchestration/task_loader.py:464setsdata["task_id"] = f"{task.task_id}/{row_id}", so an activation/suite row yields--session-id coder-eval-my_suite/row_003-a1b2c3d4. This repo already treats that exact hazard as a known one at the one other place a task_id reaches a filesystem name —sandbox.py:288:safe_task_id = self.task_id.replace("/", "_").replace("\\", "_"), commented "Dataset row tasks have IDs like "parent/row" -- flatten path separators so they don't become subdirectories". Pi is the only agent that putstask_idinto a value the CLI consumes (grep -rn "self.task_id" src/coder_eval/agents/*.pyshows every other agent only stores it for event labelling), so this is new surface. Ifpiderives its session file from the session id under--session-dir— the usual implementation for a--session-dir+--session-idpair — the segment resolves to a non-existent subdirectory and the session either errors (every row of the suite finalizes ERROR) or silently fails to persist (multi-turn/simulation continuity is lost while the run still reports SUCCESS). No test covers it:tests/test_pi_agent.py:172constructs every agent withtask_id="t1". Fix: sanitize once (reuse thesandbox.py:288replacement) before interpolating, and add a test asserting the emitted--session-idcontains no path separator fortask_id="suite/row_1".
Non-blocking, but please consider before merge
- [Axis 1] Four above-A blocks in the new agent: communicate D (26), on_turn_end C (19), _settle_turn C (17), _handle_line C (13) (
src/coder_eval/agents/pi_agent.py:472) — Split the two Pi-specific reducers that carry the new complexity:_PiTurnState.on_turn_end(line 472, radon C (19), ~90 lines) does five unrelated jobs in one body — bucket coercion (lines 490-497), turn-total accumulation (499-504), cost accumulation (505-510), stop-reason/error latching (512-523), and assistant-message +TurnEndEventassembly (525-564); extracting_accumulate_usage(usage)and_append_assistant_message(...)makes each independently testable.PiAgent.communicate(line 888) is radon D (26), above the axis's 10-20 band, and its stdout loop (lines 964-997: deadline arithmetic,asyncio.waitrace, post-exit drain, max_turns cut, should_stop cut) is the natural extraction. Calibration note:OpenCodeAgent.communicatemeasures exactly D (26) too and its_settle_turnis D (24) vs Pi's C (17), so the new module is not worse than the sibling it was copied from — resolving finding #1 (one shared driver) would retire most of this number at the same time. Verified withuv run radon cc -s -n Bon pi_agent.py at PR HEAD:M 888:4 PiAgent.communicate - D (26),M 472:4 _PiTurnState.on_turn_end - C (19),M 1055:4 PiAgent._settle_turn - C (17),M 1143:4 PiAgent._handle_line - C (13). - [Axis 3] The reconciliation invariant test asserts only 3 of the 4 token buckets, and
cacheWriteis zero in every Pi fixture and helper (tests/test_pi_agent.py:206) —test_reconciliation_invariantis docstringed "Summing the four buckets across messages must equal token_usage exactly" but sums three: lines 213-215 aresum(m.input_tokens ...),sum(m.output_tokens ...),sum(m.cache_read_tokens ...)— there is nosum(m.cache_creation_tokens for m in record.messages) == usage.cache_creation_input_tokens. Worse, no test ever supplies a nonzero cache-creation value: everycacheWriteintests/fixtures/pi_happy_stream.jsonlis0, and the_turn_end(...)helper'scache_writeparameter (tests/test_pi_agent.py:59) is never passed by any caller, so the only assertion on that bucket isassert usage.cache_creation_input_tokens == 0(line 203). A drift in theusage.get("cacheWrite")key or in the fold at pi_agent.py:502 / the per-message field at 543 / theTurnEndEventtokens at 560 would leave the whole suite green while zeroing cache-creation tokens for every Pi run. The OpenCode twin does both: its happy fixture carries a nonzero value (assert usage.cache_creation_input_tokens == 5, tests/test_opencode_agent.py:245) and its invariant test asserts the fourth bucket (assert sum(m.cache_creation_tokens for m in record.messages) == usage.cache_creation_input_tokens, tests/test_opencode_agent.py:258). Add the fourth assertion and give at least one_turn_end(..., cache_write=N)stream. - [Axis 3]
_build_env'sPLUGIN_TOOLS_DIRexport (pi_agent.py:883) is uncovered — the UiPath plugin-discovery pin half of theAgent.startcontract has no Pi test (src/coder_eval/agents/pi_agent.py:883) —_build_env'sif self._plugin_tools_dir and "PLUGIN_TOOLS_DIR" not in env: env["PLUGIN_TOOLS_DIR"] = self._plugin_tools_dir(pi_agent.py:882-883) is uncovered — line 883 is in the--cov-report=term-missingmiss list, andgrep -n "plugin_tools_dir\|PLUGIN_TOOLS_DIR" tests/test_pi_agent.pyreturns nothing.class TestSandboxEnvironment(tests/test_pi_agent.py:410-424) tests only the PATH prepend and whole-environment inheritance, even thoughstart()acceptsplugin_tools_dir(pi_agent.py:723) and the docstring calls it part of the mock-shadowing contract. This is the variable the sandbox's mock CLIs read to write their invocation log, so a break silently zeroescli_called-style criteria on every Pi task rather than erroring. OpenCode covers all three legs of the same two lines — tests/test_opencode_agent.py:695test_plugin_tools_dir_is_exported(assert captured["kwargs"]["env"]["PLUGIN_TOOLS_DIR"] == "/sandbox/tools"), :704test_inherited_plugin_tools_dir_wins, and :717 (absent → key not present). Port those three. - [Axis 3] The new per-turn slicing in the evalboard cost apportionment is only ever exercised with a single turn (
evalboard/lib/__tests__/parseMessages.test.ts:892) — All three tests indescribe("parseMessages — open-weight cost apportionment")pass a one-elementturnsarray (const turns: TurnEntry[] = [ { ... } ]at lines 893-902, 913-918 and 923-929), so the change's new index variable —const turnStart = out.length;(evalboard/lib/runs.ts:1614) andconst turnRows = out.slice(turnStart)— is only ever evaluated as 0. Nothing pins the property the variable exists for: that turn N's apportionment addresses turn N's rows only. If it regressed toout.slice(0), turn 2'sanyPricedcheck would see turn 1's already-priced rows and skip apportionment (or overwrite turn 1's costs), and every current test would still pass. Add a two-turn case — turn 1 priced from the rate card, turn 2 unpriced with a realtoken_usage.total_cost_usd— and assert turn 1's rows keep their rate-card values while turn 2's rows sum exactly to turn 2's total. - [Axis 7] docs/agents/OPENCODE.md's container-credential and Docker-blocker claims are left stale by this PR's own env_passthrough + image changes (
docs/agents/OPENCODE.md:301) —src/coder_eval/models/sandbox.pyadds"OPENROUTER_API_KEY",toDockerDriverConfig.env_passthrough's default allowlist. That directly contradicts two statements the same PR touched: (a)docs/agents/OPENCODE.md:298-303— "- No credentials would reach it. The docker driver forwards host environment\n variables through an explicit allowlist (DockerDriverConfig.env_passthrough),\n which carries per-harness blocks for Codex and Antigravity but none for\n OpenCode — soOPENROUTER_API_KEYand friends are not passed through" (the PR edited this exact paragraph, changing onlySandboxConfig→DockerDriverConfig); and (b)docker/Dockerfile:71-72— "npm install -g opencode-ai), absent from this image, and no OpenCode\n# credentials are in DockerDriverConfig.env_passthrough". Consequence:docs/agents/OPENCODE.md:305-308still tells users to "pass the credentials viasandbox.env_passthrough_extra" when building a custom OpenCode image, which is now unnecessary —OPENROUTER_API_KEYreaches the container by default. Fix: state in both places thatOPENROUTER_API_KEYIS now in the default allowlist (added for Pi) so only the missing CLI blocks OpenCode-in-docker, and drop the obsoleteenv_passthrough_extrainstruction. - [Axis 8] _TOOL_NAME_MAP omits Pi's real
findtool while carrying a deadglobentry, so a canonicalGlobcriterion scores 0 on Pi runs (src/coder_eval/agents/pi_agent.py:149) —_TOOL_NAME_MAP(pi_agent.py:142-157) maps"glob": "Glob"(line 149),"grep": "Grep"(150) and"list"/"ls": "LS"(151-152) but has nofindentry, even though this module's own comments enumeratefindas a Pi built-in twice: pi_agent.py:184-185 ("Pi's built-in tool names are lowercase (bash/read/write/edit/grep/find/ls)") and pi_agent.py:857-858 (same list); PI.md:130 repeats it. Unmapped names "pass through unchanged" (pi_agent.py:141), so a Pifindcall is persisted astool_name: "find"and anycommand_executed/commands_efficiencycriterion written against the canonicalGlobscores 0 on a Pi run that did the equivalent search — breaking the cross-harness comparability the map exists to provide. This is the same defect class the sibling documents atopencode_agent.py:144-152forapply_patch("Unmapped, everytool_name: Write/tool_name: Editcriterion scores 0 on a GPT-family model that edited the file correctly"). Fix: add"find": "Glob"(or"Grep", whichever matches Pi's tool semantics — confirm against a captured run) and extendtests/test_pi_agent.py::TestToolNormalizationwith a case for it.
Nits
- [Axis 1] Tautological Pi tests that restate the Literal/class definition or duplicate the preceding test instead of asserting behaviour (
tests/test_pi_agent_config.py:45) — Drop both tautological tests.test_thinking_level_literal_has_exactly_seven_values(lines 45-55) re-states the type definition it is checking —assert set(PiThinkingLevel.__value__.__args__) == {"off", "minimal", "low", "medium", "high", "xhigh", "max"}— and is fully covered behaviourally by the parametrizedtest_all_seven_thinking_levels_validate(lines 40-42) plustest_invalid_thinking_level_rejected(lines 58-60); it can only fail in lockstep with the edit that would change it, so it guards nothing.test_importable_from_models(lines 21-23) assertsPiAgentConfig.__name__ == "PiAgentConfig", which is vacuous — the real assertion is thefrom coder_eval.models import ... PiAgentConfigon line 13, already exercised bytest_member_of_discriminated_union. No precedent for either shape exists in the suite (grep -rn "__args__" tests/*.pyreturns nothing), so this is new noise rather than house style. - [Axis 2]
if self.config.thinking_level:is a statically always-true guard — the Literal admits no falsy value (src/coder_eval/agents/pi_agent.py:848) —_build_argvreadsif self.config.thinking_level:/argv += ["--thinking", self.config.thinking_level](lines 848-849), but the field isthinking_level: PiThinkingLevel = Field(default="medium", …)(models/agent_config.py:426-429) overLiteral["off", "minimal", "low", "medium", "high", "xhigh", "max"](line 388) — non-Optional, with no empty-string member, on a model declaringvalidate_assignment=True(agent_config.py:137). The condition can therefore never be False, but it reads as ifthinking_levelmight be unset and invites a future reader to add a| None. The sibling that owns the same field name reads it unconditionally —antigravity_agent.py:438:level = types.ThinkingLevel(self.config.thinking_level). Drop the guard (argv += ["--thinking", self.config.thinking_level]), matching Antigravity. - [Axis 2]
Anyand a missing return type on the test helper every Pi test funnels through (tests/test_pi_agent.py:165) —async def _run(agent: PiAgent, tmp_path: Any, prompt: str = "do the thing", **kwargs: Any):—tmp_pathis pytest'spathlib.Pathfixture (used asawait agent.start(str(tmp_path))on line 166), and the helper returns theTurnRecordfromagent.communicate(line 167), so both are precisely typeable. As written, the ~40 tests that consume its result get an implicitAnyback, so an assertion against a fieldTurnRecorddoes not have type-checks clean instead of failing. Annotate asasync def _run(agent: PiAgent, tmp_path: Path, prompt: str = "do the thing", **kwargs: Any) -> TurnRecord:. - [Axis 3] The Pi tests leak one
pi-session-*tempdir per test becausestart()is called without a matchingstop()(tests/test_pi_agent.py:166) —_rundoesawait agent.start(str(tmp_path))(line 166) andstart()creates a real host tempdir (self._session_dir = tempfile.mkdtemp(prefix="pi-session-"), pi_agent.py:759) that onlystop()removes (_cleanup_session_dir, 767-770).grep -c "await agent.stop()" tests/test_pi_agent.pyreturns 3 against 56 start/_runcall sites, so nearly every test leaks. Measured:ls -d $TMPDIR/pi-session-* | wc -lwent from 553 to 603 across onepytest tests/test_pi_agent.pyrun — 50 new directories per invocation. Wrap the agent in a fixture that callsawait agent.stop()in teardown (or pointtempfile.mkdtempattmp_pathvia monkeypatch) so the suite is self-cleaning like the rest of the repo's sandbox tests. - [Axis 4] New third-party npm CLI baked into the shared docker image is outside every dependency-audit gate (
docker/Dockerfile:43) — Line 43 addsRUN npm install -g @earendil-works/pi-coding-agent@${PI_VERSION}(pinARG PI_VERSION=0.84.4, line 42) to the single image every--driver dockertask runs, so a new third-party package — its transitive tree resolved by semver at build time, with npm lifecycle scripts enabled, executed as root — is now part of the trusted base image for all harnesses, not just Pi. The PR's Python-side delta adds nothing (pyproject.toml's new extra is literallypi = [], anduv.lock's only change isprovides-extras = [..., "pi"]), so the routedpip-audit"No known vulnerabilities found" does not cover this dependency at all, and no npm-side audit exists in CI. The version pin is guarded (tests/test_image_from_dockerfiles.py::test_pi_cli_baked_and_pinnedrejectslatest), which is the good half; complete it by adding an npm-side supply-chain gate (npm audit --audit-level=highor an SBOM/osv-scannerstep over the image) to the same workflow that runspip-audit, and considernpm install -g --ignore-scriptsplus installing Pi in a separate layer/stage so a non-Pi image can be built without it. CVSS:3.1/AV:L/AC:H/PR:H/UI:N/S:U/C:L/I:L/A:L - [Axis 4] OPENROUTER_API_KEY added to the blanket docker env allowlist, so it reaches containers of every agent type (
src/coder_eval/models/sandbox.py:263) — Line 263 adds"OPENROUTER_API_KEY",toDockerDriverConfig.env_passthrough'sdefault_factory, and that allowlist is agent-agnostic:isolation/docker_runner.py:1374-1381doesmerged_allowlist = set(cfg.env_passthrough) | set(cfg.env_passthrough_extra)thenargv += ["--env", env_var]for every name present in the host env, with no reference toagent.type. A claude-code or codex task run under--driver dockertherefore receives Pi's OpenRouter credential in its container environment even though nothing in that run needs it, and the container defaults to--network bridge(docker_runner.py:1353-1357) with an agent whosepermission_modemay auto-runbash— so the key is readable and exfiltratable by a prompt-injected or adversarial agent that has no legitimate use for it. The forwarding mechanism itself is fine (name-only--env VAR, so the value never lands in the logged argv). The least-privilege fix is to gate provider credentials on the resolved agent kind — forwardOPENROUTER_API_KEYonly whenagent.type == pi, asCODEX_*/GEMINI_API_KEY/ANTHROPIC_API_KEYshould likewise be — rather than growing one blanket set; at minimum record the deliberate trade-off next to the entry (the adjacentHOMEentry already sets that precedent with "Remove this entry if you don't want host HOME leakage"). CVSS:3.1/AV:L/AC:H/PR:L/UI:N/S:U/C:L/I:N/A:N - [Axis 7] PI.md documents an unpinned
npm installfor a CLI whose event-vocabulary drift is a hard crash (docs/agents/PI.md:44) —docs/agents/PI.md:43-46gives the host install as unpinned: "bash\nnpm install -g @earendil-works/pi-coding-agent\npi --version\n", and the page names no verified version. But the harness hard-fails on vocabulary drift —docs/agents/PI.md:203-208"### Drift is crashed, not scored" — and both the reducer's_RECOGNIZED_EVENTSand the whole event grammar are captured from one release (src/coder_eval/agents/pi_agent.py:17"Event grammar (captured frompi0.84.4)",:192-194"The full recognized Pi vocabulary (frompi0.84.4)"), while docker pins it (docker/Dockerfile:39-41"ARG PI_VERSION=0.84.4" with the rationale "@latest would freeze\n# nondeterministically under layer caching"). A host install of@latestcan therefore land on an untested vocabulary. Fix: document the pinned version in the install snippet (npm install -g @earendil-works/pi-coding-agent@0.84.4) and note that it must matchdocker/Dockerfile'sPI_VERSIONfor host↔docker result parity. - [Axis 8] Pi wired into the evalboard badge map but not into KNOWN_HARNESSES / HARNESS_COLORS (
evalboard/app/_components/harness-badge.tsx:20) — The PR addspi: { src: "/harness/pi.png", label: "Pi · pi.dev", short: "Pi" },toHARNESS_LOGO(harness-badge.tsx:20) but leaves the two sibling harness registries inevalboard/lib/harness.tsuntouched:KNOWN_HARNESSES(harness.ts:11-16) still lists onlyclaude-code,codex,antigravity,delegate-sdk, andHARNESS_COLORS(harness.ts:82-87) the same four.KNOWN_HARNESSESis passed as a hardcoded selector list byevalboard/app/trends/page.tsx:142andevalboard/app/watchlist/page.tsx:61, so those two pages cannot select a Pi run even once Pi runs exist, andharnessColor(harness.ts:94-95) folds Pi into the grayHARNESS_COLOR_FALLBACKin every multi-harness chart. Membership is documented as display-order/color only, so nothing breaks — but the PR half-wired the surface. Fix: add"pi"toKNOWN_HARNESSESand a validated hue toHARNESS_COLORS(harness.ts:80 notes "Re-run the palette validator before adding a fifth entry"), or leave the badge entry out too for consistency with OpenCode, which is absent from all three maps.
What's Missing
Downstream consumers:
- 🟠 The new apportionment (runs.ts:2014-2045) is gated on "nothing on this turn was priced by the rate card + the turn reports a real total_cost_usd" — NOT on the harness — so it also fires on existing LiteLLM/OpenRouter runs (litellm backend, OpenCode), whose OpenRouter models are deliberately absent from evalboard/lib/pricing.ts while
litellm_cost.apply_actual_costwrites a real turn-leveltotal_cost_usd. That silently reverses the documented decision those runs rely on (evalboard/lib/tests/messageActualCost.test.ts:4-7: "actual per-call cost is no longer distributed onto transcript messages — it lives in the separate providerCalls table"), retroactively for every historical run, so the same money now appears both apportioned across message rows and itemised in the ProviderCall table. The existing test only passes because its fixture turn carries nototal_cost_usd; nothing covers a turn with bothprovider_call_costsand a real total. Either skip apportionment whenturn.provider_call_costs?.length > 0(or key it on the absence of per-call actuals) and revisit that test's stated policy, or state the change explicitly. (trigger: evalboard/lib/runs.ts)
Parallel paths:
- 🟡 docker/Dockerfile bakes the pinned
piCLI (line 42-43), but the sibling image built from the same directory — docker/Dockerfile.runtime, the relocatable kit that inject-mode task imagesCOPY --from(its contract block only promises/opt/coder-eval/node/bin/{node,claude}, and it installs only thecodexextra) — was not extended. A--type pitask whose image uses the runtime kit therefore fails at spawn with "pi CLI not found", and neither the kit's contract comment nor docs/DOCKER_ISOLATION.md § "Tasks that bring their own base image" (which enumerates "standalone CPython + Node + the coder-eval CLI + Claude Code" and already records the kit's other exclusions, e.g. the[uipath]extra) says Pi is unsupported there. (trigger: docker/Dockerfile) - 🟡
OPENROUTER_API_KEY— Pi's only credential, and now a default docker passthrough (models/sandbox.py:263) — was not added to either credential-onboarding surface: docs/USER_GUIDE.md's Environment Variables table (which has rows forCODEX_API_KEY/CODEX_BASE_URL/CODEX_MODEL,GEMINI_API_KEY/ANTIGRAVITY_MODEL,LITELLM_*) and.env.example(which has a commented block per harness). The same PR did update USER_GUIDE's--typerow to listpi, so a reader is told the harness exists but not what it needs to authenticate; only docs/agents/PI.md names the variable. (trigger: docs/USER_GUIDE.md) - 🔵 Roster/
--typeenumerations outside CE047's seven tracked surfaces were left at four harnesses: docs/tutorials/01-first-evaluation.md:19 (the "or" list of agent guides), docs/CI_GATE.md:180-183 (per-harness credential prerequisites — namesGEMINI_API_KEYfor Antigravity, nothing for Pi) and theextrasexamples in docs/CI_GATE.md:58 + action.yml:31 (no mention of the newpiextra). CE047 cannot catch any of these because they are not inROSTER_SURFACESat all. (trigger: README.md) (restates: Axis 2: CE047's substring roster guard is vacuous for the 2-char name "Pi") - 🔵 Provenance/help surfaces for the baked agent binaries were not extended to the second one: the image stamps
LABEL org.coder-eval.claude-code-version(docker/Dockerfile:103-105, rationale "so the host can assert it viadocker image inspectbefore a (billed) run") but noorg.coder-eval.pi-version, and the Makefile'sdocker-imagetarget still advertises "core + both agents baked in" / echoes "(core + claude + codex)" (Makefile:114-116). Combined withutils.py::get_version_inforecording onlyclaude_code_cliandPiAgent.get_environment_inforecording no CLI version, a docker Pi run's harness version is unrecoverable from the run record. (trigger: docker/Dockerfile) - 🔵 Pi introduces a third
run_limits.max_turnsunit (nativeturn_startsteps), documented in the parity table, but the executable cross-harness fixture whose whole purpose is comparing that cap — tasks/run_limits/max_turns_cap.yaml:1-7, "Run it with --type claude-code / codex / antigravity and compare" — was not extended (OpenCode is missing from it too), so the new unit has unit-test coverage only. (trigger: docs/agents/HARNESS_PARITY.md)
Tests:
- 🟡 The
--skillinjection shipped to unblock activation suites has no test of the signal those suites actually score: tests/test_pi_agent.py:933-950 asserts only that--skill <dir>reaches argv and lands inenvironment_info, while nothing exercisesskill_triggeredagainst Pi's real engagement shape (per the pinned CLI's docs/skills.md, the agentreads the full SKILL.md, i.e. areadtool call normalized toReadwithpath→file_pathvia_PI_ARG_RENAME, path<root>/skills/<name>/SKILL.md). tests/test_skill_triggered.py carries Claude and Codex cases only, and src/coder_eval/criteria/skill_triggered.py's module docstring still enumerates just those two mechanisms — so a rename in_PI_ARG_RENAME/_TOOL_NAME_MAPwould report recall 0.0 for every positive row with no failing test. (trigger: tests/test_pi_agent.py) (restates: Axis 7: Pi's plugins -> --skill injection is live in code but documented as absent in five places across four surfaces) - 🟡 Docker support for Pi (baked CLI +
OPENROUTER_API_KEYpassthrough) ships with no task that exercises it: there is notasks/agents/pi_hello_world_docker.yamlcounterpart to the existingclaude_hello_world_docker.yaml/antigravity_hello_world_docker.yamlpair, and the only Pi task deliberately opts out of the CI bucket (nosmoke-pass). The whole docker claim rests on a Dockerfile-text assertion (tests/test_image_from_dockerfiles.py::test_pi_cli_baked_and_pinned) plus"OPENROUTER_API_KEY" in DockerDriverConfig().env_passthrough, so nothing verifies that an in-containerpiactually starts, authenticates, and produces a scored turn. (trigger: tasks/pi_smoke_test.yaml) - 🟡 The three apportionment tests (parseMessages.test.ts:892-931) use a hand-built 1-2 assistant-message turn, never the shape every real Pi turn has: the
EventCollectorbooks one syntheticreconciliationentry per turn, and parseMessages appends it LAST (runs.ts:1954-2000), so on a Pi turn it both draws a token-weighted share of the real cost and absorbs the rounding residual — i.e. the RECONCILE row shows money while the assistant rows show less. tests/fixtures/pi_happy_stream.jsonl already provides a realistic stream to derive that fixture from. (trigger: evalboard/lib/tests/parseMessages.test.ts) (restates: Axis 3: The new per-turn slicing in the evalboard cost apportionment is only ever exercised with a single turn) - 🔵 tests/test_pi_smoke_task.py:17 resolves the task as a bare cwd-relative
Path("tasks/pi_smoke_test.yaml"), so all three tests fail outside the repo root (reproduced: runningpytest test_pi_smoke_task.pyfromtests/gives 3 ×FileNotFoundError: Task file not found: tasks/pi_smoke_test.yaml). The house pattern is a file-anchored path (tests/test_custom_lint.py:2512 usesPath(__file__).parent.parent / "tasks"). (trigger: tests/test_pi_smoke_task.py)
Display & mapping dicts:
- 🟡
_TOOL_NAME_MAP(pi_agent.py:142-157) was not derived from Pi's actual tool set:find(Pi's glob-by-pattern tool) has no entry so it passes through asfindand every canonicalGlobcriterion scores 0, the"glob": "Glob"entry is dead (Pi has noglobtool), the real Windows built-inpowershellis unmapped, and eight entries (patch,multiedit,list,webfetch,todowrite,todoread,task,glob) name tools Pi never emits. Downstream this also skews analysis.py/report command rollups, which key on the normalized tool name. (trigger: src/coder_eval/agents/pi_agent.py) _(restates: Axis 8: TOOL_NAME_MAP omits Pi's realfindtool while carrying a deadglobentry) - 🔵 Only one of the three evalboard harness maps was extended for the new kind:
HARNESS_LOGOgot apirow, whileKNOWN_HARNESSESandHARNESS_COLORS(evalboard/lib/harness.ts:11-16, 82-87) did not — soharness-selector.tsx,trends/page.tsx:142andwatchlist/page.tsx:61cannot select a Pi run,orderHarnessessorts Pi into the unknown tail, andharnessColorfolds it into the gray fallback in every multi-harness chart. _(trigger: evalboard/app/components/harness-badge.tsx) (restates: Axis 8: Pi wired into the evalboard badge map but not into KNOWN_HARNESSES / HARNESS_COLORS)
Daily/nightly:
- 🟡 The PR states no blast radius for the shared production image: docker/Dockerfile is what
.github/workflows/docker-publish.ymlbuilds and pushes asghcr.io/<org>/coder-eval-agent:lateston every push to main, i.e. the image EVERY--driver dockertask of EVERY harness pulls, and it now carries a new third-party global npm install (~23 MB unpacked plus its dependency tree, lifecycle scripts enabled, installed as root). The repo has a live precedent for exactly this risk — commit 48a4d53 bumpedgoogle-antigravity"to clear a Defender false positive on the bundled harness" — and there is still no npm-side audit in the workflow that runspip-audit. (trigger: docker/Dockerfile) (restates: Axis 4: New third-party npm CLI baked into the shared docker image is outside every dependency-audit gate) - 🟡 Nothing scheduled or automated will ever run Pi: the only Pi task excludes itself from the CI E2E bucket, no experiment in experiments/ carries a
pivariant, and no nightly surface adds--type pi. That matters more for this harness than the others because vocabulary drift is a hard crash by design (docs/agents/PI.md § "Drift is crashed, not scored",_RECOGNIZED_EVENTScaptured from pi 0.84.4) while the documented host install is unpinned — so the first signal of a breakingpirelease is a human's ad-hoc run, and the PR does not say who validates Pi after a CLI bump. (trigger: tasks/pi_smoke_test.yaml) (restates: Axis 7: PI.md documents an unpinnednpm installfor a CLI whose event-vocabulary drift is a hard crash) - 🔵 Adding
OPENROUTER_API_KEYto the agent-agnostic default allowlist changes every scheduled docker run, not just Pi's: any nightly/CI container (claude-code, codex, antigravity) now receives the OpenRouter credential whenever it is present in the runner environment, on the default--network bridge. The PR does not state that consequence for the production run path or record the trade-off next to the entry, as the adjacentHOMEentry does. (trigger: src/coder_eval/models/sandbox.py) (restates: Axis 4: OPENROUTER_API_KEY added to the blanket docker env allowlist, so it reaches containers of every agent type)
Harness & Lint Improvements
Elided from this comment to fit GitHub's 65,536-character body limit.
15 static-check proposals (CE048–CE058 + a ruff ANN201 ratchet, a pyright tests-side widening, and an npm supply-chain gate) and 7 harness improvements are in the full report:
tmp/code-review-260908-0927/00-summary.md § Harness & Lint Improvements (and results.json).
They are durable-infrastructure work, separate from this PR's fixes.
Top 5 Priority Actions
- Sanitize
task_idbefore it enters the CLI--session-idat src/coder_eval/agents/pi_agent.py:758 (reuse the/→_flatten from sandbox.py:288, ideally the full[A-Za-z0-9._-]safe set), because pi 0.84.4 validates the id and exits 1, so today every dataset-row task burns its retry budget and finalizes ERROR with an empty event stream. - Make
_settle_turnconsultstate.error_messageand crash the turn at src/coder_eval/agents/pi_agent.py:1112 (mirroring opencode_agent.py:1378-1379), and restore the second telemetry arm at pi_agent.py:1098 as a warn when aturn_endthat carried a usage object yields all-zero buckets, so a provider outage reports ERROR instead of silently depressing the measured pass rate and token-shape drift stops blindingmax_usd/max_total_tokens. - Add
"find": "Glob"to_TOOL_NAME_MAPat src/coder_eval/agents/pi_agent.py:149 and drop the dead"glob"entry (Pi has noglobtool;findis glob-by-pattern), socommand_executed/commands_efficiencycriteria written against the canonicalGlobno longer score 0 on a Pi run that searched correctly. - Close the Test Health gap on the new code by porting the OpenCode doubles and matrices into tests/test_pi_agent.py:
_ExplodingRunningProcess+TestTurnAlwaysReapsTheCli/TestExternalCancel(the current_FakeProcess.wait()at line 111 exits on first await, leaving orphan reap, SIGKILL escalation andCancelledErrorunreachable), the fiveTestCostFallsBackToTheRateCardcases (both_resolve_costfallbacks stay green even when stubbed toreturn self.cost_usd), the fourth cache-creation bucket intest_reconciliation_invariant(line 206, with one nonzerocache_writestream), the threePLUGIN_TOOLS_DIRcases for pi_agent.py:883, and a two-turn case in evalboard/lib/tests/parseMessages.test.ts:892 that pins per-turn apportionment (mutating runs.ts:1614 to0leaves all 642 tests passing). - Restore truth in the guards and docs: anchor CE047's matcher on word boundaries in tests/lint/agent_roster_parity.py:102 (the 2-char
"Pi"added at line 57 matches "anthropic"/"ci-pipeline"/"copies", so pyproject.toml:4, docs/comparison.md and .github/pages-stub/index.html ship a stale four-harness roster with the gate green), then rewrite the five surfaces claiming Pi ignoresplugins(docs/agents/PI.md:238 and :134, docs/agents/HARNESS_PARITY.md:176 plus apicolumn in its depth table, src/coder_eval/models/agent_config.py:419, CLAUDE.md:147) and the three now-false OPENROUTER_API_KEY claims in docs/agents/OPENCODE.md:301 and :327 and docker/Dockerfile:71.
Stats: 0 🔴 · 8 🟠 · 6 🟡 · 8 🔵 across 8 axes reviewed.
bai-uipath
left a comment
There was a problem hiding this comment.
Approve — no risk to the existing harnesses; the OpenCode duplication is the one thing I'd fix.
-
The port is 2× with OpenCode, and only OpenCode. 16 of the 26 same-named methods are ≥0.75 similar, several byte-identical, with zero overlap against claude-code, codex and antigravity. Worth hoisting the shared subprocess scaffold in a follow-up, before a third CLI harness makes it three copies.
-
Nothing here can break an existing harness — the touched shared surfaces are all additive or defaulted. One caveat: the evalboard cost apportionment fires for any turn the rate card can't price, so it changes LiteLLM and OpenCode runs too, and two comments in that file still assert the opposite.
Minor: four doc surfaces still say plugins is warned-and-ignored after the last commit wired skill injection; CE047 passes vacuously for Pi because a two-letter name substring-matches anything (the comparison page, the Pages stub and the PyPI metadata never actually name it); reasoning tokens are folded into output while the stream's own totalTokens excludes them and is never read — worth a guard like OpenCode's; the reap and post-EOF teardown paths are uncovered (90.7% vs OpenCode's 95.8%); TestReviewFixes should be named for the behavior it covers.
Nice work on the byte-real capture driving offline replay and on baking a pinned CLI with a drift guard — both are better than what OpenCode shipped with.
…y warn, tool map, CE047 Addresses uipreliga review blockers on PR #159: - B8: sanitize task_id before it enters pi's `--session-id` (dataset-row ids are path-shaped "suite/row"; a raw '/' resolved to a non-existent session subdir and failed the row). Keep only [A-Za-z0-9._-]. - B5: crash `_settle_turn` on a terminal provider error (stopReason=error that survived pi's internal retries) so it books as ERROR (retryable / excluded from outcomes) instead of a clean COMPLETED FAILURE. Mirrors opencode_agent. - B6: warn once when a turn_end carried a usage object whose every bucket is 0 (the rename-each-key-to-0 drift shape) — otherwise tokens/cost silently vanish and max_usd / max_total_tokens go blind. - Tool map: `find`→`Glob` (Pi's glob-by-pattern search tool), drop the dead `glob` entry — so command_executed/commands_efficiency criteria written against the canonical `Glob` score on a Pi run that searched. - B2: anchor CE047's roster matcher on word boundaries (the 2-char "Pi" matched "anthropic"/"ci-pipeline"/"copies", hiding a stale roster) + name Pi in pyproject.toml description/keywords, docs/comparison.md, and the pages stub; add a guard test that a short spelling is not satisfied by a substring. Tests: +find→Glob, +all-zero-usage warn, +session-id sanitize, terminal-error now asserts crash; 465 pi + custom-lint tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…laims Blocker 7: the plugins->--skill feature landed after the docs commit, so five surfaces still claimed plugins is ignored. Rewrite PI.md (both spots), HARNESS_PARITY.md (prose + add a pi column to the plugin-path depth table), PiAgentConfig docstring, and CLAUDE.md to state plugins IS honored for its skills half (each resolved skills dir -> --skill <dir>, recorded as pi_skill_paths) and that Pi can therefore run activation suites. Also correct the now-false OPENROUTER_API_KEY claims this PR made stale in docs/agents/OPENCODE.md and docker/Dockerfile: OPENROUTER_API_KEY IS now in the default docker allowlist (added for Pi), so the OpenCode CLI (still not baked) is the only remaining docker blocker for env-auth runs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…, 4) The plain _FakeProcess exits the instant its stream is awaited, so orphan reap, SIGKILL escalation and CancelledError handling were all unreachable, and _resolve_cost's two rate-card fallbacks were never asserted. - Add _ExplodingRunningProcess (raises from readline, stays alive) and _HangingProcess (blocks in readline, returncode stays None) doubles. - TestTurnAlwaysReapsTheCli: read-loop crash and external cancel both kill the CLI; a clean turn kills nothing (else attempt 2 spawns a second pi editing the graded files). - TestExternalCancel: a cancelled turn parks a crashed partial, emits one AgentEndEvent (CRASHED, crash_reason='turn cancelled'), and reaps the child. - TestCostFallsBackToTheRateCard: five cases — stream cost wins / missing cost priced from the rate card / unpriced model reports None / reported-/usr/local/bin/bash on a priced model uses the rate card / reported-/usr/local/bin/bash on an unpriced model stays 0. 78 pi tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…skills.py Blocker 1 (concrete slice): pi_agent imported opencode_agent's private _plugin_skill_dirs, coupling the two harnesses through an implementation-private symbol while a comment claimed a shared-module hoist was "out of scope". Move the resolver (+ _manifest_skill_dirs and its constants) into a new shared agents/_skills.py; opencode_agent and pi_agent both import it from there, so the cross-agent private import is gone. opencode_agent re-exports it, so its existing test import is unchanged. The FULL nd-JSON-CLI driver hoist (a shared NdJsonCliAgent base for communicate / kill / reap / crash / cost across pi + opencode) is a larger refactor of two harnesses including the production OpenCode one; it is left as a tracked follow-up rather than bolted onto this review-response, and the deferral is now documented at the _TERM_GRACE_SECONDS comment instead of the stale "out of scope" note. 214 opencode+pi tests, custom lint (396), check and typecheck all pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… token/telemetry minors Caveat: the open-weight cost apportionment (evalboard/lib/runs.ts) fired for ANY unpriced turn with a real total_cost_usd — including LiteLLM-route and OpenCode runs, which book the real per-call cost in the separate ProviderCall table by design. Apportioning the turn total onto message rows too surfaced the same money twice and reversed the documented "actual cost is not distributed onto transcript messages" decision for every historical such run. Skip apportionment when the turn carries a `provider_call_costs` audit (Pi has none, so it is unaffected); add a parseMessages test pinning it. Minors: - Pi now cross-checks the stream's own `totalTokens` against input+output+cacheRead+cacheWrite (reasoning is billed but excluded from that field) and warns once on a mismatch — a renamed/moved bucket under a CLI upgrade would otherwise be absorbed silently by _as_int and blind max_total_tokens/max_usd. Mirrors OpenCode's `tokens.total` guard. +test. - Rename TestReviewFixes -> TestTurnLifecycleAndTokenTelemetry (named for behavior). (The plugins-docs and CE047 minors bai-uipath listed were already fixed in the blocker pass; the shared-subprocess-scaffold hoist remains a tracked follow-up.) 79 pi tests, 643 evalboard tests, lint/check/typecheck all pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dead dup + doc/guard nits Fable + Sonnet both independently caught a MAJOR edge in the blocker-round B5 fix: _settle_turn's terminal-error crash ran UNGATED, unlike the two crash arms below it. error_message is set at an error turn_end and cleared only by a LATER non-error turn_end — but a max_turns / should_stop cut fires at the next turn_start, before that clearing turn_end arrives, on an error pi was still retrying internally. The clean, budget-exhausted cut then crashed + burned retries, contradicting the documented "finalizes cleanly as max_turns_exhausted, no crash" contract. Gate the error arm on `not stopped_early and not state.max_turns_exhausted`, mirroring the sibling arms; +test (max_turns cut right after an error turn finalizes cleanly). Also: - Remove a dead duplicate `_HangingProcess` (the blocker-round port added a second definition that Python silently shadowed with the pre-existing one). - totalTokens reconciliation guard now accepts int OR float (a `123.0`-shaped total is itself plausible drift; consistent with _as_int). - Correct the HARNESS_PARITY plugin-path depth table + PI.md: Pi's shared resolver LOADS a bare skills dir (fallback → `--skill <dir>`), it does not "load nothing". The real divergence is skill_triggered detection needing a `skills/<name>/` path segment, so a bare dir scores recall 0 despite the skill running. 216 pi+opencode tests, lint 396, check, typecheck, evalboard tsc all pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pi harness (
--type pi)Adds the Pi coding agent (
pi, from pi.dev) as a first-class coder-eval harness, on par with claude-code / codex / antigravity / opencode. Registers via the agent SPI (agent.type: pi), streams the standardized event protocol, and runs under both the tempdir and docker drivers.What's included
agents/pi_agent.py):pi -p --mode jsonnd-JSON reducer →TurnRecord; native multi-turn loop percommunicate()(turn_start/turn_end),max_turnson the visible-turn boundary, internal-retry folding (agent_end.willRetry→ finalize atagent_settled/EOF),--append-system-prompt,--session-dircontinuity.agent.plugins→pi --skill <dir>via the shared_plugin_skill_dirsresolver (also used by OpenCode);pi_skill_pathsrecorded inenvironment_info.PI_VERSIONbaked intodocker/Dockerfile;OPENROUTER_API_KEYforwarded throughDockerDriverConfig.env_passthrough→--driver docker --type piworks.turn_startclosed on the next turn; terminal provider error (stopReason=error) surfaced in the result and reset on recovery; warn-once on token-accounting drift instead of silently zeroing tokens/cost.docs/agents/PI.md+ harness-parity table updates; tests for the reducer, skill injection, and the docker pin.Harness-parity notes
system_promptis enforced (--append-system-prompt). Not enforced (warned + ignored, documented):permission_mode(sandbox driver is the boundary),allowed_tools/disallowed_tools(Pi's built-in tool names are lowercase and can't map to the Claude-namespaced config default — forwarding would strip all tools),system_prompt_file,pluginsnon-skill assets.Benchmark: Pi (Kimi-K3, DeepSeek-V4-Pro) vs Terra & Sonnet-5 nightlies
30 tasks that ran in both nightlies and use deterministic graders (no judge / no live-tenant), 20 categories, 2 seeds per column (Pi = 2 run seeds; Terra/Sonnet = 2 most-recent nightly observations per task). Full write-up: Confluence.
Takeaways: quality is a tight ~2-point band (Sonnet 0.973 ≥ Terra 0.965 ≥ DeepSeek 0.954 ≈ Kimi 0.952); DeepSeek-V4-Pro is the value winner at $0.078/task (~14.6× cheaper than Sonnet); Sonnet's high token/cost is mostly the multi-turn harness accounting (39.7 turns, cache reads) vs Terra's single visible turn — not raw waste.
Test plan
make format/make check/make typecheck— clean.pytest tests/test_pi_agent.py tests/test_opencode_agent.py— 202 passed.make lint(CE rules) — 389 passed._shared/harness errors, skills loaded via--skill.🤖 Generated with Claude Code