[feat] Bake pinned Pi into the Daytona snapshot - #5046
Conversation
dosubot
Bot
added
the
size:L
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the Daytona sandbox snapshot build and runner behavior so Pi no longer installs at session start by default (assuming the snapshot already bakes Pi), and adjusts related tests/docs. It also updates the session heartbeat unit test to reflect a contract change where status is no longer sent by the runner.
Changes:
- Flip Pi runtime install behavior to opt-in (
AGENTA_AGENT_SANDBOX_PI_INSTALLED === "true") and add unit tests for the new default. - Update Daytona snapshot docs/build script to reflect which harness components are baked and add base-image verification steps.
- Update the alive-heartbeat unit test to assert
statusis no longer included in the final heartbeat payload.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| services/runner/tests/unit/session-alive.test.ts | Updates heartbeat test expectations to match the runner → server heartbeat contract (no status in payload). |
| services/runner/tests/unit/sandbox-agent-daytona.test.ts | Adds coverage for the new default/compat behavior of DAYTONA_PI_INSTALL. |
| services/runner/src/engines/sandbox_agent/daytona.ts | Flips Pi runtime install from opt-out to opt-in and updates inline documentation. |
| services/runner/sandbox-images/daytona/README.md | Documents what the base -full image bakes and how the Pi install lever now behaves. |
| services/runner/sandbox-images/daytona/build_snapshot.py | Verifies baked harness binaries in the base image and installs the pi CLI into the snapshot. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| out to -- so this script's only real job is adding that. Everything else below is a | ||
| verify-and-mark step, not a re-install: we set the `_INSTALLED` marker for | ||
| claude/codex/opencode too so `daytona.ts` never re-runs their daemon auto-install | ||
| (currently a no-op safety net at runtime, since the daemon's own install is idempotent, | ||
| but skipping it avoids the "is it already there" round-trip on every run) and never pays | ||
| their (far larger) per-run cost were that ever to regress. Set the runner service to use |
| DAYTONA_SNAPSHOT=agenta-sandbox-pi | ||
| AGENTA_AGENT_SANDBOX_PI_INSTALLED=false | ||
| AGENTA_AGENT_SANDBOX_PI_INSTALLED=true | ||
| AGENTA_AGENT_SANDBOX_CODEX_INSTALLED=true | ||
| AGENTA_AGENT_SANDBOX_OPENCODE_INSTALLED=true | ||
| AGENTA_AGENT_SANDBOX_CLAUDE_INSTALLED=true |
|
@jp-agenta setting this to ready to review, since I think this one should make it into the initial release (in contrast to the other addition codex/opencode etc.. ). Did not test though, but the code is sensible |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ef5c855-b5e8-4203-8fc1-c1ac586d0328
📒 Files selected for processing (5)
services/runner/sandbox-images/daytona/README.mdservices/runner/sandbox-images/daytona/build_snapshot.pyservices/runner/src/engines/sandbox_agent/daytona.tsservices/runner/tests/unit/sandbox-agent-daytona.test.tsservices/runner/tests/unit/session-alive.test.ts
| The `-full` sandbox-agent base image already bakes ALL FOUR harnesses: its own build | ||
| runs `sandbox-agent install-agent --all` as its last layer, which installs the claude, | ||
| codex, and opencode native binaries + ACP adapters (verified by pulling the image | ||
| manifest and inspecting the layer: `bin/claude`, `bin/codex` + `agent_processes/codex-acp` | ||
| (`@zed-industries/codex-acp`), `bin/opencode` + `agent_processes/opencode-acp`), plus the | ||
| `pi-acp` adapter for Pi. The ONE thing `install-agent --all` does NOT bake is the | ||
| standalone `pi` CLI binary itself (`@earendil-works/pi-coding-agent`) that `pi-acp` shells | ||
| out to -- so this script's only real job is adding that. Everything else below is a | ||
| verify-and-mark step, not a re-install: we set the `_INSTALLED` marker for | ||
| claude/codex/opencode too so `daytona.ts` never re-runs their daemon auto-install | ||
| (currently a no-op safety net at runtime, since the daemon's own install is idempotent, | ||
| but skipping it avoids the "is it already there" round-trip on every run) and never pays | ||
| their (far larger) per-run cost were that ever to regress. Set the runner service to use | ||
| this snapshot: | ||
|
|
||
| DAYTONA_SNAPSHOT=agenta-sandbox-pi | ||
| AGENTA_AGENT_SANDBOX_PI_INSTALLED=false | ||
| AGENTA_AGENT_SANDBOX_PI_INSTALLED=true | ||
| AGENTA_AGENT_SANDBOX_CODEX_INSTALLED=true | ||
| AGENTA_AGENT_SANDBOX_OPENCODE_INSTALLED=true | ||
| AGENTA_AGENT_SANDBOX_CLAUDE_INSTALLED=true |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fix the configuration example: AGENTA_AGENT_SANDBOX_PI_INSTALLED=true should NOT be set when using this snapshot.
The docstring's configuration example (lines 23–26) recommends setting AGENTA_AGENT_SANDBOX_PI_INSTALLED=true alongside DAYTONA_SNAPSHOT=agenta-sandbox-pi. However, daytona.ts (line 31) interprets === "true" as opt in to runtime installation, and the README (lines 43–47) explicitly says to use true only for bare/non-snapshot images. Following this example with the snapshot would trigger a redundant ~150s npm install of Pi on every session, defeating the snapshot's purpose.
Additionally, the docstring (lines 15–19) claims the _INSTALLED markers for claude/codex/opencode prevent daytona.ts from re-running their daemon auto-install. But the README (lines 48–51) states these markers are "reserved for symmetry only" and "nothing reads them yet" — daytona.ts has no session-time install path for those three harnesses.
🔧 Proposed fix for the configuration example and docstring
-their (far larger) per-run cost were that ever to regress. Set the runner service to use
-this snapshot:
+their (far larger) per-run cost were that ever to regress.
+
+Set the runner service to use this snapshot (all four harnesses are baked, so leave
+the AGENTA_AGENT_SANDBOX_*_INSTALLED flags unset — the runner defaults to install-OFF):
DAYTONA_SNAPSHOT=agenta-sandbox-pi
- AGENTA_AGENT_SANDBOX_PI_INSTALLED=true
- AGENTA_AGENT_SANDBOX_CODEX_INSTALLED=true
- AGENTA_AGENT_SANDBOX_OPENCODE_INSTALLED=true
- AGENTA_AGENT_SANDBOX_CLAUDE_INSTALLED=true
+ # Only set AGENTA_AGENT_SANDBOX_PI_INSTALLED=true when running against a bare
+ # image that lacks pi (see sandbox-images/daytona/README.md).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The `-full` sandbox-agent base image already bakes ALL FOUR harnesses: its own build | |
| runs `sandbox-agent install-agent --all` as its last layer, which installs the claude, | |
| codex, and opencode native binaries + ACP adapters (verified by pulling the image | |
| manifest and inspecting the layer: `bin/claude`, `bin/codex` + `agent_processes/codex-acp` | |
| (`@zed-industries/codex-acp`), `bin/opencode` + `agent_processes/opencode-acp`), plus the | |
| `pi-acp` adapter for Pi. The ONE thing `install-agent --all` does NOT bake is the | |
| standalone `pi` CLI binary itself (`@earendil-works/pi-coding-agent`) that `pi-acp` shells | |
| out to -- so this script's only real job is adding that. Everything else below is a | |
| verify-and-mark step, not a re-install: we set the `_INSTALLED` marker for | |
| claude/codex/opencode too so `daytona.ts` never re-runs their daemon auto-install | |
| (currently a no-op safety net at runtime, since the daemon's own install is idempotent, | |
| but skipping it avoids the "is it already there" round-trip on every run) and never pays | |
| their (far larger) per-run cost were that ever to regress. Set the runner service to use | |
| this snapshot: | |
| DAYTONA_SNAPSHOT=agenta-sandbox-pi | |
| AGENTA_AGENT_SANDBOX_PI_INSTALLED=false | |
| AGENTA_AGENT_SANDBOX_PI_INSTALLED=true | |
| AGENTA_AGENT_SANDBOX_CODEX_INSTALLED=true | |
| AGENTA_AGENT_SANDBOX_OPENCODE_INSTALLED=true | |
| AGENTA_AGENT_SANDBOX_CLAUDE_INSTALLED=true | |
| The `-full` sandbox-agent base image already bakes ALL FOUR harnesses: its own build | |
| runs `sandbox-agent install-agent --all` as its last layer, which installs the claude, | |
| codex, and opencode native binaries + ACP adapters (verified by pulling the image | |
| manifest and inspecting the layer: `bin/claude`, `bin/codex` + `agent_processes/codex-acp` | |
| (`@zed-industries/codex-acp`), `bin/opencode` + `agent_processes/opencode-acp`), plus the | |
| `pi-acp` adapter for Pi. The ONE thing `install-agent --all` does NOT bake is the | |
| standalone `pi` CLI binary itself (`@earendil-works/pi-coding-agent`) that `pi-acp` shells | |
| out to -- so this script's only real job is adding that. Everything else below is a | |
| verify-and-mark step, not a re-install: we set the `_INSTALLED` marker for | |
| claude/codex/opencode too so `daytona.ts` never re-runs their daemon auto-install | |
| (currently a no-op safety net at runtime, since the daemon's own install is idempotent, | |
| but skipping it avoids the "is it already there" round-trip on every run) and never pays | |
| their (far larger) per-run cost were that ever to regress. | |
| Set the runner service to use this snapshot (all four harnesses are baked, so leave | |
| the AGENTA_AGENT_SANDBOX_*_INSTALLED flags unset — the runner defaults to install-OFF): | |
| DAYTONA_SNAPSHOT=agenta-sandbox-pi | |
| # Only set AGENTA_AGENT_SANDBOX_PI_INSTALLED=true when running against a bare | |
| # image that lacks pi (see sandbox-images/daytona/README.md). |
| image = Image.base(SANDBOX_AGENT_IMAGE).dockerfile_commands( | ||
| [ | ||
| "USER root", | ||
| f"RUN npm install -g --ignore-scripts {PI_PACKAGE}", | ||
| "RUN pi --version || true", |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
RUN pi --version || true silently masks a failed Pi installation.
Pi is the one binary this script actually installs, yet its verification uses || true while the pre-baked harnesses (claude/codex/opencode, lines 104–109) use fatal test -x checks. If the npm install -g --ignore-scripts on line 99 fails (e.g., due to --ignore-scripts skipping a required post-install step), the build silently produces a snapshot without Pi. Since DAYTONA_PI_INSTALL defaults to false, the runtime will assume Pi is present and never attempt a fallback install — resulting in broken Pi sessions.
🔒 Proposed fix: make Pi verification fatal
- "RUN pi --version || true",
+ "RUN pi --version",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| image = Image.base(SANDBOX_AGENT_IMAGE).dockerfile_commands( | |
| [ | |
| "USER root", | |
| f"RUN npm install -g --ignore-scripts {PI_PACKAGE}", | |
| "RUN pi --version || true", | |
| image = Image.base(SANDBOX_AGENT_IMAGE).dockerfile_commands( | |
| [ | |
| "USER root", | |
| f"RUN npm install -g --ignore-scripts {PI_PACKAGE}", | |
| "RUN pi --version", |
62da5d8 to
f48b249
Compare
dosubot
Bot
added
size:M
mmabrouk
changed the title
Context
The Daytona full sandbox-agent base already contains Claude, Codex, and OpenCode plus the Pi ACP adapter, but it does not contain the standalone Pi CLI that pi-acp launches.
Fresh bare images remain reliable by default: with AGENTA_AGENT_SANDBOX_PI_INSTALLED unset or true, the runner installs our pinned Pi version into the new sandbox session. Operators set it to false only when DAYTONA_SNAPSHOT points at a snapshot that already bakes Pi.
Changes
Scope
This PR changes the Daytona snapshot recipe and Pi installation behavior only. It does not add model-catalog entries; those remain in #5198.
Validation
The API, SDK, and web jobs also fail on the big-agents integration PR at the same base SHA with the same unrelated test failures. This PR touches none of those areas.