[migheorghe]

Problem

docker/Dockerfile installs the Pi CLI at PI_VERSION, and PI.md documents --driver docker --type pi as supported. docker/Dockerfile.runtime does not install it. A task that brings its own base image (runtime-mode: inject) gets only node and claude from the kit, so every Pi attempt there fails with "The 'pi' CLI was not found on PATH".

Change

• docker/Dockerfile.runtime installs @earendil-works/pi-coding-agent@${PI_VERSION} into /opt/coder-eval/node, the prefix Claude Code already uses. The entrypoint already puts that bin directory on PATH. pi --version runs in the builder, so a bin that did not land fails the build instead of every task.
• PI_VERSION is 0.87.1, the same as docker/Dockerfile. test_pi_version_pin_matches_framework keeps the two pins equal, the way test_claude_code_version_pin_matches_framework does for Claude Code.
• PI.md and DOCKER_ISOLATION.md say the kit carries Pi.

No other files change. OPENROUTER_API_KEY is already in the docker driver's env_passthrough.

Evidence

devtools-coding-agent-bench injects the kit into every task image. It carried this same hunk as a local patch on 0.12.12, and build 13944412 built the patched kit. In an injected task image, Pi then started and called OpenRouter for three models. Those attempts ended on our account's 401, not on the image. Once a release carries this change, we drop the patch.

Tests

ruff check and ruff format --check pass. The runtime-kit tests in tests/test_image_from_dockerfiles.py pass, both the new one and the existing Claude Code pin and install-dir tests. I did not build the image locally.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RarvxSsr4K2KkBisTbXxQY

[github-actions]

Claude finished @migheorghe's task in 1m 27s —— View job

---

Code Review in Progress

Review Checklist

• Read .github/code_review.md for review guidelines
• Read CLAUDE.md for project conventions
• Get full PR diff
• Review docker/Dockerfile.runtime changes
• Review docs/DOCKER_ISOLATION.md changes
• Review docs/agents/PI.md changes
• Review tests/test_image_from_dockerfiles.py changes
• Perform cross-file consistency checks
• Check for missing components
• Provide structured review feedback

[uipreliga]

Review: feat(docker): bake the Pi CLI into the runtime kit

PR #236 by @migheorghe · feat/pi-in-runtime-kit → main · OPEN · reviewed against b1fc1bd

Change class: simple — adds one pinned npm install step to the runtime kit, mirrored from docker/Dockerfile, plus a pin-parity test and doc lines; no Python runtime code path changes

coder_eval is in very good condition (9.9/10): no finding can change a task's score or final_status for the same agent output, and the type safety, security, architecture and evaluation-harness axes are clean; the only runtime risk is a low-severity Node-engine mismatch in the Pi runtime kit, and the other findings are test duplication and doc cleanup in the Pi changes, so the codebase is ready to ship after the small fixes below.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	9.3 / 10	0	0	1	2	Pi pin-parity test copies the Claude pin test and its Dockerfile ARG-pin parser again
2. Type Safety	10 / 10	0	0	0	0	—
3. Test Health	10 / 10	0	0	0	0	—
4. Security	10 / 10	0	0	0	0	—
5. Architecture & Design	10 / 10	0	0	0	0	—
6. Error Handling & Resilience	9.9 / 10	0	0	0	1	The Pi install accepts an unsupported Node engine with only a warning, and the pi --version gate cannot catch it
7. API Surface & Maintainability	10 / 10	0	0	0	0	—
8. Evaluation Harness Quality	10 / 10	0	0	0	0	—

Overall Score: 9.9 / 10 · Weakest Axis: Code Quality & Style at 9.3 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 1 · 🔵 3 across 8 axes.

Blockers

None.

Non-blocking, but please consider before merge

1. [Axis 1] Pi pin-parity test copies the Claude pin test and its Dockerfile ARG-pin parser again (tests/test_image_from_dockerfiles.py:497) — The new test adds a nested def _pin(df: Path) -> str | None: (line 497) that differs from the one in test_claude_code_version_pin_matches_framework (line 473) only in its regex: re.match(r"\s*ARG PI_VERSION=(\S+)", ln) at line 499 versus re.match(r"\s*ARG CLAUDE_CODE_VERSION=(\S+)", ln) at line 475. The same for ln in ...splitlines(): m = re.match(r"\s*ARG <NAME>=(\S+)", ln) loop is also at line 526 (UV_VERSION) and line 554 (PI_VERSION again, in test_pi_cli_baked_and_pinned). So the file now has 4 copies, and PI_VERSION is read from docker/Dockerfile by two separate parsers. Fix: add one module-level helper, _arg_pin(df_text: str, name: str) -> str | None, and use it at all four sites. Then replace the two parity tests with one @pytest.mark.parametrize("arg", ["CLAUDE_CODE_VERSION", "PI_VERSION"]) test. Keep the Pi-only package-string assert (line 510), or move it into test_pi_cli_baked_and_pinned so that test checks both Dockerfiles. The anchor table puts duplication across 4+ sites at High. I filed this as Medium because the code is test-only, the loop is short, and the PR added only one of the four copies.

Nits

1. [Axis 1] test_pi_cli_baked_and_pinned docstring wrongly says the runtime kit does not bake Pi (tests/test_image_from_dockerfiles.py:548) — Lines 548-549 at PR head still say: Scope: main docker/Dockerfile only — Dockerfile.runtime deliberately does NOT bake Pi (inject-mode is out of scope), so it is not asserted here. The PR now bakes Pi into docker/Dockerfile.runtime (lines 92-95). The new test 40 lines above (line 488, test_pi_version_pin_matches_framework) asserts that the runtime kit installs Pi. So the old docstring contradicts the PR in the same file. This is a stale claim the rename/removal grep (Technique 1) should have caught. Fix: delete or reword the sentence, for example: Scope: main docker/Dockerfile; the runtime kit's Pi pin is guarded by test_pi_version_pin_matches_framework. Better: merge the two Pi tests, as in finding 1.
2. [Axis 1] A reflowed paragraph in PI.md leaves one 114-character source line in an otherwise ~80-column paragraph (docs/agents/PI.md:221) — Line 221 at PR head is brings its own base image runs Pi too. OPENROUTER_API_KEY is forwarded into the container by the docker driver's (114 characters). Lines 218-223 around it wrap at about 75-81 columns. The PR reflowed lines 219-220 but did not reflow the rest of the sentence. Re-wrap lines 221-223 to the same width as the rest of the paragraph. This is cosmetic only and does not change the rendered output.
3. [Axis 6] The Pi install accepts an unsupported Node engine with only a warning, and the pi --version gate cannot catch it (docker/Dockerfile.runtime:93) — The kit pins ARG NODE_VERSION=22.14.0 (line 68). The new step at lines 93-95 runs RUN PATH="/opt/coder-eval/node/bin:${PATH}" npm install -g --prefix /opt/coder-eval/node \ @earendil-works/pi-coding-agent@${PI_VERSION} \ && PATH="/opt/coder-eval/node/bin:${PATH}" pi --version. @earendil-works/pi-coding-agent@0.87.1 declares engines = { node: '>=22.19.0' } (checked with npm view). npm 10.9.2, which ships with Node 22.14.0, only prints npm warn EBADENGINE ... required: { node: '>=22.19.0' }, current: { node: 'v22.14.0' } and installs anyway. I reproduced this in node:22.14.0-slim: pi --version printed 0.87.1 with exit 0, and a pi -p --mode json run reached the provider and got a 401. So the build gate stays green when the package runs on a Node version it does not support. The framework image does not have this problem: it installs the newest Node 22.x through deb.nodesource.com/setup_22.x. As a result, inject mode runs Pi on a different and unsupported Node than rebase mode, and nothing reports it. I saw no breakage today, which is why this is Low. Fix: make the mismatch fail the build by adding --engine-strict to this npm install -g (or set npm_config_engine_strict=true for the step), or raise NODE_VERSION to at least 22.19.0. A static parity check could also compare the kit's NODE_VERSION with the engine floor of each npm CLI it installs.

What's Missing

Tests:

• 🟡 No CI job or test builds docker/Dockerfile.runtime. The kit is built only by make coder-eval-runtime, and .github/ does not reference it. So the new pi --version build gate (Dockerfile.runtime:93-95) never runs in this repo. The author says they did not build the image. The only build evidence is the downstream bench build 13944412, which built a local patch and not this commit. The new test is static text matching only. Add a CI smoke build of the kit, or a docker-marked test, that runs /opt/coder-eval/node/bin/pi --version in the final stage. Otherwise a broken install or a missing bin is found only by consumers who inject the kit. (trigger: docker/Dockerfile.runtime)
• 🔵 The Pi image guards are now split across two tests that disagree. test_pi_cli_baked_and_pinned still scopes itself to docker/Dockerfile only and says the kit does not bake Pi. test_pi_version_pin_matches_framework covers the kit's pin and package string, but not the latest guard. Merge them so one test asserts pin, non-latest and ${PI_VERSION} install on both Dockerfiles. (trigger: tests/test_image_from_dockerfiles.py) (restates: Axis 1: test_pi_cli_baked_and_pinned docstring wrongly says the runtime kit does not bake Pi)

Parallel paths:

• 🔵 docker/coder_eval_runtime_entrypoint.sh was not updated. Its header comment (lines 6-9) still says only coder-eval, claude, and node resolve from the kit. The PR updated the matching comments in Dockerfile.runtime (contract line 13 and the entrypoint note near line 113) to add pi, but missed the entrypoint script that those comments describe. (trigger: docker/Dockerfile.runtime)
• 🔵 The 'how to bake another agent CLI' guidance still names only the framework image. The docker/Dockerfile comment (lines 81-91, 'Adding it means a pinned version ... as CLAUDE_CODE_VERSION / PI_VERSION do') and docs/agents/OPENCODE.md 'Running in Docker' (line ~292) both name docker/Dockerfile only. After this PR, a baked Node CLI must also go into Dockerfile.runtime with a pin-parity test, or inject-mode tasks break as Pi did. Neither surface says this. (trigger: docker/Dockerfile.runtime)
• 🔵 This PR fixes agent coverage between the two images for Pi only. The kit still installs only --extra codex (Dockerfile.runtime:63). The framework image adds antigravity, litellm and harbor (Dockerfile:107). So --type antigravity and route: litellm judges still fail in inject mode, the same way Pi did before this PR. DOCKER_ISOLATION.md (~line 158) mentions this only as 'core + codex' and has no per-agent rebase-vs-inject support list. Add that list, or a static check that diffs the agent sets of the two images. (trigger: docs/DOCKER_ISOLATION.md)
• 🔵 The new Pi pin-parity test leaves Node itself unchecked. The kit pins NODE_VERSION=22.14.0 (line 68). The framework image installs the newest Node 22.x from nodesource. So inject-mode Pi runs on a Node version below the package's >=22.19.0 engine floor, and rebase-mode Pi does not. No parity test or note covers the runtime under the pinned CLI. (trigger: docker/Dockerfile.runtime) (restates: Axis 6: The Pi install accepts an unsupported Node engine with only a warning, and the pi --version gate cannot catch it)

Harness & Lint Improvements

Static checks (lint / type):

• [ce-lint] CE069 Dockerfile ARG-pin parity. This is a whole-tree @pytest.mark.lint class in tests/test_custom_lint.py, because it reads Dockerfiles and not Python ASTs. Parse every ARG <NAME>_VERSION=<value> line in docker/Dockerfile and docker/Dockerfile.runtime with ONE shared module-level helper, _arg_pin(text, name). Fail when a *_VERSION ARG is declared in both files with different values. Also fail when an npm install -g <pkg>@${<NAME>_VERSION} appears in one file but not the other, unless the name is in an EXEMPT dict that gives a reason. This one generic rule replaces the hand-written per-tool tests test_claude_code_version_pin_matches_framework and test_pi_version_pin_matches_framework (tests/test_image_from_dockerfiles.py:463/488). Delete those tests and their copied _pin parsers in the same change, so the next CLI baked into both images gets parity checking without a third copy. Use the next free number after CE068. Prevents: Finding 1: the copied Pi pin-parity test and the 4 copies of the ARG regex loop at tests/test_image_from_dockerfiles.py:473/497/526/554. Finding 2: the stale docstring at :548 is deleted together with the duplicate Pi test, and nothing replaces it with a hand-written scope claim.
• [ce-lint] CE070 npm global installs must be engine-strict. This is a whole-tree @pytest.mark.lint class over docker/Dockerfile*. Every RUN that contains npm install -g must pass --engine-strict, or it must run after an ENV npm_config_engine_strict=true in the same stage. Then an npm package whose engines.node floor is above the installed Node fails the image build, and does not just print npm warn EBADENGINE and continue. The rule also flags the existing unflagged installs at docker/Dockerfile:37 (claude-code) and :43 (pi-coding-agent). Fix those in the same change and do not exempt them. Prevents: Finding 4: docker/Dockerfile.runtime:93 installs @earendil-works/pi-coding-agent@0.87.1 (engines node>=22.19.0) on the pinned NODE_VERSION=22.14.0. The build passes, and the pi --version gate cannot detect the problem.

Harness improvements (not statically reachable):

• Add a network-gated check of the Node engine floor, as a make target run in the image-build CI job or marked live. For each npm install -g <pkg>@${X_VERSION} in docker/Dockerfile.runtime, run npm view <pkg>@<pin> engines.node and assert that the kit's ARG NODE_VERSION satisfies the range. Run it when docker/Dockerfile.runtime or a *_VERSION pin changes. Why not static: The engine floor is in the npm registry metadata of the pinned version, not in the repo. A static rule can require --engine-strict (CE070), but it cannot compute the floor without a network call. This check reports the mismatch at pin-bump time, before a docker build. Prevents: Finding 4: inject mode runs Pi on an unsupported Node (22.14.0 < 22.19.0), but rebase mode uses the newest nodesource 22.x, and nothing reports the difference.
• Make no static check for stale scope claims in docstrings. Instead, the code-review Technique 1 (rename/removal grep) must also grep tests/ for the negated form of the change, e.g. does NOT bake <tool> / not installed in <file>, when a PR adds a tool to an image or file that it did not have before. Why not static: To detect that a prose sentence ('Dockerfile.runtime deliberately does NOT bake Pi') contradicts the new state of another file, a tool must understand the sentence. No grep or AST pattern separates a stale claim from a true one. CE069 removes this instance, because it deletes the test that has the docstring. Prevents: Finding 2: the stale docstring at tests/test_image_from_dockerfiles.py:548.
• Decision recorded: add no line-width rule for docs/*.md. The repo has about 272 non-table, non-URL source lines in docs/ longer than 100 columns, so a width cap would either fail a large part of the tree or need a high limit that would not catch a 114-column line. The rendered output does not change, so leave this cosmetic issue to review. Why not static: A width rule is possible, but it is not worth the noise. Markdown source wrapping does not change the rendered HTML, and the existing docs have no shared wrap width that a rule could enforce without a large reflow. Prevents: Finding 3: the 114-character line in a ~80-column paragraph at docs/agents/PI.md:221 (deliberately not prevented).

Top 5 Priority Actions

1. No finding changes a task's score or final_status for the same agent output; the highest-impact item is docker/Dockerfile.runtime:93: add --engine-strict to the Pi npm install -g (or raise ARG NODE_VERSION at line 68 from 22.14.0 to at least 22.19.0) so that the build fails when Pi 0.87.1 (engines node >=22.19.0) would otherwise run on an unsupported Node in inject mode.
2. Add a static parity test that compares the runtime kit's NODE_VERSION (docker/Dockerfile.runtime:68) with the engines.node floor of each npm CLI it installs, so that a later CLI bump cannot quietly make inject mode run on a different Node than rebase mode.
3. Remove the duplicated parsing in tests/test_image_from_dockerfiles.py: add one module-level _arg_pin(df_text, name) helper, use it at the four copied ARG-pin parsing sites (lines 475, 499, 526, 554), and merge the Claude and Pi pin-parity tests (lines 473 and 497) into one @pytest.mark.parametrize("arg", ["CLAUDE_CODE_VERSION", "PI_VERSION"]) test.
4. Fix the stale docstring at tests/test_image_from_dockerfiles.py:548, which says that Dockerfile.runtime does not bake Pi; that is now wrong because the PR adds Pi at docker/Dockerfile.runtime:92-95. The simplest fix is to move the Pi package-string assert (line 510) into test_pi_cli_baked_and_pinned so that this test checks both Dockerfiles.
5. Re-wrap docs/agents/PI.md:221-223 to the ~80-column width of the paragraph around it (line 221 is 114 characters); this is a cosmetic change only and does not change the rendered output.

---

Stats: 0 🔴 · 0 🟠 · 1 🟡 · 3 🔵 across 8 axes reviewed.