diff --git a/docs/design/agent-workflows/projects/cold-turn-startup/README.md b/docs/design/agent-workflows/projects/cold-turn-startup/README.md new file mode 100644 index 0000000000..c34d6a50a7 --- /dev/null +++ b/docs/design/agent-workflows/projects/cold-turn-startup/README.md @@ -0,0 +1,64 @@ +# Cold-turn startup cost removal + +This workspace plans two changes that each remove fixed wall-clock delay from the start of +a "cold" agent turn. Profiling on the live local runner on 2026-07-11 measured both. Together +they add up to roughly two seconds that the user waits before the model even begins to answer. + +## Glossary + +Read this once. Later files use these terms without re-defining them. + +- **Runner**: the Node sidecar under `services/runner/`. It receives a `/run` request and + drives one agent turn. Written in TypeScript because the agent harnesses are Node libraries. +- **Harness**: the coding-agent program the runner drives (Pi, Claude Code, Codex). This plan + is about the Pi harness. +- **ACP**: Agent Client Protocol. The JSON-RPC protocol the runner speaks to a harness. +- **Adapter**: a small program that translates ACP on one side into a specific harness's own + interface on the other. For Pi the adapter is an npm package named `pi-acp`. It spawns the + real `pi` binary and bridges it to ACP. +- **sandbox-agent**: an npm package plus a compiled command-line binary (`@sandbox-agent/cli`). + The runner launches this binary; the binary chooses and launches the adapter. The runner + already ships one hand-written change to this package through pnpm's patch mechanism. +- **Probe**: a short child-process call the adapter runs at session start to gather version + information (`pi --version`, `npm view ...`). Each probe blocks the turn until it returns. +- **Cold turn**: the first turn of a conversation, or any turn where no warm harness session is + reused. A cold turn pays full session-startup cost. This plan only concerns cold turns. +- **Session**: one harness conversation. `createSession` opens it; this is where the Pi cold + start (and the probes) happen. +- **Mount**: attaching a remote object store as if it were a local directory, using a + user-space filesystem. The runner mounts the caller's durable working directory this way so + files the agent writes persist across turns. +- **geesefs / FUSE**: geesefs is the program that performs the mount. FUSE ("filesystem in + user space") is the Linux mechanism it uses. A FUSE mount placed over an existing directory + hides whatever that directory already contained. +- **cwd**: the working directory a harness session runs in. The runner sets it to the mounted + durable directory. +- **Daytona**: the remote sandbox provider. When a run is remote, the harness runs inside a + Daytona sandbox instead of on the runner host. +- **Patch (pnpm)**: a stored diff pnpm re-applies to a dependency's files on every install. The + runner keeps these under `services/runner/patches/`. A patch only affects the copy of the + package inside the runner's own `node_modules`. + +## The two changes + +- **Fix A, remove the Pi startup probes.** The Pi adapter runs three version probes at every + cold session start, serialized before the first model request. They cost about 1.6 seconds + and add nothing the user asked for. The change removes them. This is the higher-value and + better-understood change. +- **Fix B, stop paying for the durable mount on chat-only turns.** The runner mounts the + durable working directory before it opens the session, even for a turn that never touches a + file. The mount costs about 0.55 to 0.6 seconds. The change avoids that cost when no file is + used. Research below shows this is genuinely harder than it looks; the plan recommends + shipping Fix A first and treating Fix B as a follow-up. + +## Reading order + +1. `context.md` answers: what does the user experience today, and why does it happen. +2. `research.md` answers: exactly what the code does now, with file and line references, and + which facts changed the plan. Read this before proposing any edit. +3. `plan.md` answers: what to change, in what order, and how to test each step. +4. `open-questions.md` answers: what must be confirmed at runtime before or during the work, + because static reading of the code could not settle it. +5. `upstream-issue-draft.md` is the ready-to-file text for the upstream `pi-acp` issue asking + for a switch that disables the update-check probes (plan Stage 2b). +6. `status.md` tracks progress and decisions. It is the running source of truth. diff --git a/docs/design/agent-workflows/projects/cold-turn-startup/context.md b/docs/design/agent-workflows/projects/cold-turn-startup/context.md new file mode 100644 index 0000000000..6a96625581 --- /dev/null +++ b/docs/design/agent-workflows/projects/cold-turn-startup/context.md @@ -0,0 +1,51 @@ +# Context + +## What the user experiences today + +When a user sends the first message of a conversation, the agent takes about two seconds +longer to begin answering than the model itself needs. That delay is fixed setup work the +runner and the Pi adapter do before the first model request goes out. It repeats on every +cold turn. It is present both for local runs and for runs inside a Daytona sandbox, and it is +worse inside a sandbox because one of the probes reaches the public npm registry over the +network from a European sandbox. + +Profiling on the live local runner on 2026-07-11 attributed the delay to two independent +causes: + +- About 1.6 seconds comes from three version-check probes the Pi adapter runs at session + start (Fix A). +- About 0.55 to 0.6 seconds comes from the runner mounting the durable working directory + before it opens the session, even on a turn that never reads or writes a file (Fix B). + +The two causes are unrelated in the code. They can ship separately. + +## Why this work exists + +Neither cost buys the user anything on a normal turn. + +The Pi adapter probes exist to print a startup banner and an "upgrade available" notice. The +runner already throws that banner away: `services/runner/src/tracing/otel.ts` strips the +banner lines out of the reply after the fact. So the runner pays 1.6 seconds to produce text +it then deletes. + +The durable mount exists so files the agent writes survive to the next turn. A chat-only turn +writes no files, so on those turns the mount is pure setup cost with no payoff. The difficulty +is that the runner cannot know in advance whether a turn will stay chat-only. + +## Goals + +- Remove the Pi adapter startup probes from the cold-turn path, for both local and Daytona + runs. Target saving: about 1.6 seconds per cold turn. +- Once the probes are gone, retire or shrink the banner-stripping code in `otel.ts`, since it + exists only to clean up after those probes. +- Investigate removing the eager durable mount from chat-only cold turns. Target saving: about + 0.55 to 0.6 seconds per cold local chat turn. Ship this only if it can be made safe. + +## Non-goals + +- No change to what the agent can do, only to how fast a cold turn starts. +- No change to the durable-store contract or the wire protocol. +- No attempt to warm-pool Pi sessions or otherwise avoid cold starts entirely. That is a + separate, larger effort. +- No change to the Daytona snapshot build in this plan beyond what Fix A strictly needs to + reach the in-sandbox adapter copy. diff --git a/docs/design/agent-workflows/projects/cold-turn-startup/open-questions.md b/docs/design/agent-workflows/projects/cold-turn-startup/open-questions.md new file mode 100644 index 0000000000..f4adb25973 --- /dev/null +++ b/docs/design/agent-workflows/projects/cold-turn-startup/open-questions.md @@ -0,0 +1,57 @@ +# Open questions + +These could not be settled by reading the code. Each names how to answer it and what depends on +the answer. + +## 1. Which `pi-acp` copy actually runs at session start? + +Static reading found two candidate copies: the runner's dependency `pi-acp@0.0.29` under +`node_modules`, reachable through the CLI's "PATH binary hint", and the CLI's own +registry-installed copy `pi-acp@0.0.31` under `~/.local/share/sandbox-agent/bin/agent_processes`. +The CLI's resolution order could pick either. The pnpm patch mechanism only reaches the first. + +- How to answer: add a temporary marker to each `dist/index.js` and see which prints on a cold + local run, or enable the CLI's adapter-resolution logging, or trace the adapter child + process to see which `index.js` path it opens. +- Depends on it: Stage 2 (patch target) and Stage 4 (Daytona approach). This is the first task. + +## 2. Does turning on `quietStartup` have any unwanted side effect? + +`quietStartup` suppresses the whole startup banner, not only the version line. The runner +already strips that banner, so the user should see no change. Confirm nothing else in the +runner or the Pi extension reads the banner text or the startup-info metadata before it is +stripped. + +- How to answer: grep the runner and the Pi extension for reads of the startup-info field + (`_meta.piAcp.startupInfo`) and confirm none depend on its content; then a cold local run + with `quietStartup` on, checking the reply is unchanged apart from the missing banner. +- Depends on it: Stage 1 shipping cleanly. + +## 3. Is the harness cold-start window long enough to hide the 0.55 second mount? + +The "overlap" variant of Fix B can only overlap the mount with the part of the cold start that +runs before workspace materialization, because `prepareWorkspace` depends on the mount being +complete. Whether that pre-workspace window is large enough to hide 0.55 seconds is a runtime +timing question. + +- How to answer: instrument `acquireEnvironment` to log timestamps at mount begin, mount + complete, workspace-prep begin, and session-open begin on a cold local chat turn. +- Depends on it: whether Fix B's overlap variant is worth building, or whether only the full + lazy-mount design would pay off. This is the first task of the Fix B follow-up. + +## 4. What is the correct Daytona delivery for a fixed adapter? (largely answered) + +Upstream research answered the mechanism question: the CLI honors a pre-seeded +`agent_processes/pi/` directory ("already installed" short-circuit), so baking a fixed adapter +copy into the Daytona snapshot at that path is the preferred delivery, with a runner-hosted +registry JSON via `SANDBOX_AGENT_ACP_REGISTRY_URL` as the alternative. See `research.md`, +"How the installed adapter version is actually chosen". + +What remains open is only verification: confirm the in-sandbox CLI's data directory path (the +in-sandbox equivalent of `~/.local/share/sandbox-agent/bin/agent_processes/pi/`) and that the +snapshot build can write there. + +- How to answer: run a Daytona cold turn and locate the adapter install directory inside the + sandbox; then add the pre-seed to the snapshot build and confirm the CLI logs + "already installed" instead of a registry fetch. +- Depends on it: Stage 4. diff --git a/docs/design/agent-workflows/projects/cold-turn-startup/plan.md b/docs/design/agent-workflows/projects/cold-turn-startup/plan.md new file mode 100644 index 0000000000..0d0ee31058 --- /dev/null +++ b/docs/design/agent-workflows/projects/cold-turn-startup/plan.md @@ -0,0 +1,157 @@ +# Plan + +The plan ships Fix A in stages that each stand alone, then treats Fix B as a measured +follow-up. Each stage names the change, the files it touches, and how to prove it worked. + +Terms used here are defined in `README.md`. Read `research.md` first; several stages depend on +a runtime fact that static reading could not settle (which adapter copy runs), and that fact is +the first task. + +## Stage 0: confirm which adapter copy runs (blocks the patch stages) + +Before choosing where to remove `buildUpdateNotice`, confirm at runtime whether the running +`pi-acp` is the runner's `0.0.29` copy (reachable via the "PATH binary hint") or the CLI's +self-installed `0.0.31` copy under `~/.local/share/sandbox-agent`. + +- How: start a cold local run and observe which file executes. Options, in order of directness: + add a temporary marker line to each candidate `dist/index.js` and see which appears; or run + the sandbox-agent CLI with its adapter-resolution logging enabled + (`SANDBOX_AGENT_LOG_STDOUT` and related, seen in the binary strings) and read which branch it + reports; or `strace`/`lsof` the adapter child process to see which `index.js` path it opened. +- Output: a one-line answer recorded in `status.md`, plus the exact path of the running copy. +- Run the check with the Pi install-skip setting (`AGENTA_AGENT_SANDBOX_PI_INSTALLED=false`, + being shipped separately) in its final state: with the install skipped, `PI_ACP_PI_COMMAND` + is no longer set on Daytona, the adapter resolves the baked `/usr/local/bin/pi` from PATH, + and adapter resolution may differ from today's. See `research.md`, "Daytona measurement and + the Pi install-skip interaction". +- Why it blocks: Stage 2's patch target and Stage 4's Daytona approach both depend on this. + +## Stage 1: turn on `quietStartup` (no patch, removes one probe) + +Remove the `buildStartupInfo` probe (about 440 ms) by writing `{"quietStartup": true}` into the +Pi agent settings file the runner controls. + +- Change: ensure the settings file at `PI_CODING_AGENT_DIR/settings.json` carries + `quietStartup: true`. The runner already copies `settings.json` when it seeds a per-run agent + dir (`services/runner/src/engines/sandbox_agent/pi-assets.ts`, `prepareLocalAgentDir` copies + `auth.json` and `settings.json`; the Daytona path uploads `settings.json` in + `daytona.ts` `uploadPiAuthToSandbox`). The cleanest point is to write or merge the flag when + the runner prepares the agent dir, so it applies to local and Daytona without depending on the + developer's own `~/.pi/agent/settings.json`. +- Design note: this is a configuration value the runner owns, not user data. Merge it into the + settings object rather than overwriting the file, so a developer's other Pi settings survive. +- Test: a runner unit test that runs the agent-dir preparation and asserts the resulting + `settings.json` parses to an object with `quietStartup === true`. Add it beside the existing + `pi-assets` tests. +- Verify live: cold local run; confirm the reply no longer contains the `pi v...` / `Context` + banner and that one of the two `pi --version` child processes is gone (process trace or the + cold-turn timing dropping by roughly 440 ms). +- Independence: this stage is safe and useful even if Stage 2 is deferred. + +## Stage 2: remove the unconditional `buildUpdateNotice` probe + +Remove the remaining `pi --version` plus `npm view` (about 670 to 720 ms). This needs a code +change to the adapter, because `research.md` showed there is no environment gate for it. + +Choose the mechanism by Stage 0's answer: + +- If the running local copy is the runner's `0.0.29`: add a pnpm patch under + `services/runner/patches/` for `pi-acp@0.0.29`, alongside the existing + `sandbox-agent@0.4.2.patch`, and add the `patchedDependencies` entry. The patch's content + should be the smallest safe edit: make `buildUpdateNotice` return `null` without spawning any + child process, or read the installed version from the adapter's own `package.json` instead of + spawning `pi --version` and skip the `npm view` entirely. Returning `null` is simplest and + loses only an upgrade hint the runner already deletes. +- If the running local copy is the CLI's self-installed `0.0.31`: a runner pnpm patch will not + reach it. Upstream research (see `research.md`, "Upstream pi-acp") settled two facts that + narrow the choice: no published version removes or gates the probes (0.0.31 is the latest and + its source still runs both), so "pin to a fixed version" is not available today; and the CLI + honors a pre-seeded `agent_processes/pi/` directory ("already installed" short-circuit), so + the runner can install its own patched copy there before the CLI ever consults the registry. + Prefer, in order: pre-seed the patched adapter into `agent_processes/pi/` at runner startup + or image build (works identically on the host and, via the snapshot, in the sandbox); + or override `SANDBOX_AGENT_ACP_REGISTRY_URL` to a registry JSON the runner hosts that pins a + runner-published adapter build. Optionally set `SANDBOX_AGENT_REQUIRE_PREINSTALL` so a + missing pre-seed fails loudly instead of silently reinstalling the unpatched registry copy. + Record the chosen path in `status.md` with its reasoning. + +- Test: if a pnpm patch is used, a runner test that imports or executes the patched + `buildUpdateNotice` equivalent and asserts it performs no child-process spawn and returns + empty. If the fix ships as a pre-seeded copy, add a startup check (or test) that the seeded + `dist/index.js` carries the patch marker. Either way the durable guard is Stage 5. +- Design note: prefer removing the network `npm view` call over merely caching it. A cached + result still costs the first cold turn and still reaches npm from a sandbox. + +## Stage 2b: file the upstream issue (parallel, not blocking) + +Upstream is active and has precedent for a settings-gated startup switch (it replaced a +`PI_ACP_STARTUP_INFO` environment variable with the `quietStartup` setting). An adjacent open +issue, svkozak/pi-acp#70, already complains about the update-check notice on protocol grounds. +File the latency issue with the measured numbers and propose a `checkForUpdates: false` +setting (or folding the update check under `quietStartup`); the draft text is in +`upstream-issue-draft.md`. If the maintainer lands it and releases, and the ACP registry pins +the new version, Stage 2's local patch and Stage 4's pre-seed both collapse into "wait for the +registry to move, then delete the workaround". Do not block any stage on this; treat an +upstream release as the retirement path for the patch, not the delivery path for the fix. + +## Stage 3: retire or shrink the banner-stripping in `otel.ts` + +The stripping in `services/runner/src/tracing/otel.ts` exists only to clean up the probe +output. + +- After Stage 1 only (update notice still emitted): narrow `isBannerLine` / + `stripStartupBanner` to match just the "New version available" and "Run: npm i" lines, since + the `pi v...` / `Context` / `Skills` banner no longer appears. Keep the streaming variant in + step. +- After Stage 2 (no probes emit any banner): remove the stripping entirely, including its unit + tests, since there is nothing left to strip. Confirm by reading a cold-turn reply and seeing + no banner lines before the stripping is removed, so the removal is provably safe. +- Test: update the existing `otel` unit tests to match whichever state ships. Do not leave + dead matchers behind. + +## Stage 4: make Fix A reach Daytona + +Local removal does not fix the in-sandbox adapter copy (`research.md`, "How the adapter reaches +a Daytona sandbox"). A Daytona measurement on 2026-07-11 put the in-sandbox probes at about +2.0 seconds (two `pi --version` at about 750 ms each plus `npm view` at about 305 ms), worse +than the local 1.6 seconds, so this stage carries the largest single saving in the plan. + +- Preferred delivery: pre-seed the fixed adapter into the sandbox's + `agent_processes/pi/` directory in the Daytona snapshot, the same mechanism as the host + pre-seed in Stage 2. The CLI's "already installed" short-circuit then uses it and never + fetches the registry from inside the sandbox. +- Alternative: set `SANDBOX_AGENT_ACP_REGISTRY_URL` in the sandbox environment to a + runner-hosted registry JSON pinning a fixed adapter build. +- If an upstream release with the fix exists by then: confirm the public ACP registry has + pinned it and let the normal install path deliver it; then remove the pre-seed. +- Out of scope but adjacent: the redundant in-sandbox Pi npm install (about 5.2 s) is being + removed separately via `AGENTA_AGENT_SANDBOX_PI_INSTALLED=false`. Coordinate timing tests + with that change, since both move the Daytona cold-turn number. +- Test: a Daytona cold run with the timing captured, compared against the same run before the + change. Record both numbers in `status.md`. + +## Stage 5: cold-turn timing regression guard + +Add one test that captures cold-turn startup time (or the count of `pi --version` / `npm view` +child processes spawned during a cold session start) so a future dependency bump that +reintroduces the probes is caught. This is the durable guard, especially where Stage 2's fix is +upstream and has no unit-testable surface in this repository. + +## Fix B: follow-up, not part of the Fix A ship + +`research.md` concludes Fix B is genuinely complicated. Do not block Fix A on it. Carry it as a +separate investigation with these first steps: + +1. Measure the timing window between session cold-start begin and workspace materialization, to + learn whether the "overlap the mount with the harness cold start" variant can hide 0.55 + seconds. This answers `open-questions.md` question 3 with a number. +2. If overlap captures most of the saving with no shadowing risk, prefer it: start + `mountLocalDurableCwd` as a background promise, keep `prepareWorkspace` and `createSession` + waiting on it, and only overlap it with the pre-workspace part of the cold start. This never + defers past the point where files must persist, so it needs no copy-up machinery. +3. Only if overlap does not pay off, scope the full lazy-mount design (mount on first file + tool, plus copy-up of local writes and a working-directory view refresh). Treat that as its + own plan. + +Recommendation: ship Stages 0 through 5 (Fix A) and defer Fix B to the follow-up above. The +brief explicitly allows dropping Fix B. diff --git a/docs/design/agent-workflows/projects/cold-turn-startup/research.md b/docs/design/agent-workflows/projects/cold-turn-startup/research.md new file mode 100644 index 0000000000..0ab8ec85ff --- /dev/null +++ b/docs/design/agent-workflows/projects/cold-turn-startup/research.md @@ -0,0 +1,300 @@ +# Research + +Every claim here was read out of the code on 2026-07-11. File paths are absolute-from-repo-root. +Line numbers are from the working tree that day and may drift; the surrounding code is quoted +so the reader can re-find it. + +## Fix A: the Pi adapter startup probes + +### What the probes are and where they run + +The Pi adapter is the npm package `pi-acp`. At the moment a session opens, inside the adapter's +"new session" handler, it builds two pieces of startup text and both call child processes: + +`node_modules/.pnpm/pi-acp@0.0.29/node_modules/pi-acp/dist/index.js` around line 1761: + +``` +const quietStartup = getQuietStartup(params.cwd); +const updateNotice = buildUpdateNotice(); +const preludeText = quietStartup ? (updateNotice ? updateNotice + "\n" : "") + : buildStartupInfo({ cwd, fileCommands, updateNotice }); +``` + +`buildUpdateNotice` (same file, around line 2550) runs two child processes: + +``` +const piVersion = spawnSync("pi", ["--version"], { encoding: "utf-8" }); +... +const latestRes = spawnSync("npm", ["view", "@earendil-works/pi-coding-agent", "version"], + { encoding: "utf-8", timeout: 800 }); +``` + +`buildStartupInfo` (around line 2570) runs a third child process: + +``` +const piVersion = spawnSync("pi", ["--version"], { encoding: "utf-8" }); +``` + +So a cold session start runs `pi --version` twice and `npm view ... version` once, one after +another, before the session response is returned and before the first model request. The +profiler attributed roughly 440 ms to each `pi --version` (a full Node boot plus the Pi bundle +import) and 230 to 280 ms to `npm view` (a network round trip to the npm registry). The +aggregate measured on the cold path was about 1.6 seconds. + +### Only one of the three probes is optional today + +`buildUpdateNotice` runs unconditionally. Nothing gates it. `buildStartupInfo` runs only when +`quietStartup` is false. `quietStartup` comes from Pi settings: + +`pi-acp/dist/index.js`, `getQuietStartup` (around line 1547) reads it from `getMergedSettings`, +which (around line 1529) merges two files: + +``` +const globalSettingsPath = join(getAgentDir(), "settings.json"); +const projectSettingsPath = resolve(cwd, ".pi", "settings.json"); +``` + +`getAgentDir()` resolves to the environment variable `PI_CODING_AGENT_DIR`, which the runner +sets and controls (`services/runner/src/engines/sandbox_agent/pi-assets.ts` writes into that +dir; `run-plan.ts` line 455 sets `sourcePiAgentDir` to `PI_CODING_AGENT_DIR` or +`~/.pi/agent`). So the runner can turn `quietStartup` on with no patch by writing +`{"quietStartup": true}` into that settings file. That removes the `buildStartupInfo` probe, +one of the two `pi --version` calls, about 440 ms. + +Turning `quietStartup` on does NOT remove `buildUpdateNotice`. Its `pi --version` plus +`npm view` still run, about 670 to 720 ms, and that is also the part that reaches npm over the +network from inside a Daytona sandbox. Removing it needs a change to the adapter's own code. + +### The adapter copy that runs is not the runner's dependency (most important finding) + +The runner declares `pi-acp` as a direct dependency, version `0.0.29` +(`services/runner/package.json` line 34), and pnpm materializes it at +`node_modules/.pnpm/pi-acp@0.0.29/...`. It is natural to assume the runner's existing patch +mechanism (`services/runner/patches/`, wired through `patchedDependencies` in `package.json`) +could patch this copy. That assumption is probably wrong, and this is the single fact that most +shapes Fix A. + +The runner does not spawn `pi-acp` itself. It launches the compiled `@sandbox-agent/cli` +binary (`services/runner/src/engines/sandbox_agent/daemon.ts`, `resolveDaemonBinary`). That +binary contains its own "agent manager" that resolves and installs adapters. Reading printable +strings out of the binary +(`node_modules/.pnpm/@sandbox-agent+cli-linux-x64@0.4.2/.../bin/sandbox-agent`) shows an +adapter registry and an npm-install path: + +``` +SANDBOX_AGENT_ACP_REGISTRY_URL +https://cdn.agentclientprotocol.com/registry/v1/latest/registry.json +agent_manager.install_agent_process_from_registry: npm package installed +"npmPackage": "pi-acp" +``` + +On this machine the binary has already done that install. It keeps a separate, self-managed +copy of the adapter here: + +``` +~/.local/share/sandbox-agent/bin/agent_processes/pi/node_modules/pi-acp (version 0.0.31) +~/.local/share/sandbox-agent/bin/agent_processes/pi/package.json -> { "pi-acp": "^0.0.31" } +``` + +So the copy that actually runs the probes is version `0.0.31`, installed by the CLI from the +ACP registry into a directory outside the repository. (The `^0.0.31` range looks floating but +is effectively pinned; see "How the installed adapter version is actually chosen" below.) The +runner's patchable `0.0.29` copy is a different file tree. The CLI's resolution order (also +visible in the strings) tries a builtin, then a "PATH binary hint", then a local launcher, +then the registry install. The runner does put its own `pi-acp` on `PATH` +(`daemon.ts` sets `PATH` to include `node_modules/.bin`, which has a `pi-acp` launcher), so it +is possible the "PATH binary hint" wins at run time and the `0.0.29` copy is used after all. +Static reading cannot decide which branch wins. This must be confirmed at runtime before a +patch target is chosen. See `open-questions.md`, question 1. + +Consequences either way: + +- If PATH resolution wins, a pnpm patch of `pi-acp@0.0.29` fixes the LOCAL path. It still does + not fix Daytona, because inside the sandbox the CLI installs its own adapter copy from the + registry (see below), where there is no runner `node_modules` to patch. +- If the registry install wins, the pnpm patch is inert even locally, because the running copy + lives under `~/.local/share/sandbox-agent`, outside anything pnpm manages. In that case the + fix must be a pre-seeded copy, a registry override, or an upstream release (see the + version-pinning section below). + +### How the adapter reaches a Daytona sandbox + +Inside a Daytona sandbox the harness runs remotely. The runner installs the `pi` binary into +the sandbox itself (`services/runner/src/engines/sandbox_agent/daytona.ts`, `installPiInSandbox` +runs `npm install @earendil-works/pi-coding-agent@` into +`/home/sandbox/.agenta-pi`). The `pi-acp` ADAPTER, however, is resolved by the same +`@sandbox-agent/cli` agent manager running inside the sandbox, which installs it from the ACP +registry the same way it does on the host. So the in-sandbox adapter is also the registry +copy, and its `npm view` probe runs from the sandbox's network location. There is no +repository file to patch for the sandbox copy; reaching it means baking a fixed adapter into +the Daytona snapshot (pre-seeding the `agent_processes/pi` directory), overriding the registry +URL the in-sandbox CLI reads, or an upstream release the registry then pins. + +### Upstream pi-acp: versions, issues, and switches (checked 2026-07-11) + +The upstream source is `github.com/svkozak/pi-acp` (npm package `pi-acp`, MIT, 498 stars, +42 open issues, last code push 2026-06-17). Facts that bear on the plan: + +- **There is no newer version than 0.0.31.** npm's latest is `0.0.31`, published 2026-06-17, + the same version the sandbox-agent CLI installs. The repository HEAD equals that release, and + HEAD still contains both probes: `src/acp/agent.ts` line 352 calls `buildUpdateNotice()` + unconditionally, and `buildUpdateNotice` (line 1430) still spawns `pi --version` and + `npm view`. So "upgrade to a fixed version" is not available today; the fix must be a patch, + a pre-seeded copy, or an upstream contribution followed by a release. +- **The README documents `quietStartup` and confirms its limit.** It states that with + `quietStartup: true` the adapter "will still emit a 'New version available' message". This + confirms from upstream documentation what the code reading found: the settings flag removes + only the `buildStartupInfo` probe. +- **Upstream already has complaints adjacent to these probes.** Open issue #70 (filed + 2026-07-09) reports that the update-check notice arrives as an `agent_message_chunk` after + the turn has ended, an ACP protocol violation, and asks for it to be queued or moved out of + band. Closed issue #68 reported the startup banner being emitted outside a turn. Open PRs + #61 ("emit startup info in-turn") and #42 ("dedupe startup info emission") touch the same + emission path. Nobody has yet filed the latency complaint or asked for a switch that + disables the update check entirely. +- **Upstream has precedent for adding exactly this kind of switch.** Commit `bae31bce` + replaced a `PI_ACP_STARTUP_INFO` environment variable with the `quietStartup` Pi setting, so + the maintainer has already accepted a settings-gated startup-output control once. A small + issue or PR proposing a `checkForUpdates: false` setting (or folding the update check under + `quietStartup`) is plausible to land. A ready-to-file draft is in `upstream-issue-draft.md`. + +### How the installed adapter version is actually chosen (pinning levers) + +The floating `^0.0.31` range in `agent_processes/pi/package.json` looked risky but is tamer +than it appears. The ACP registry entry pins the version exactly: + +``` +{"id": "pi-acp", "version": "0.0.31", + "distribution": {"npx": {"package": "pi-acp@0.0.31"}}, ...} +``` + +The CLI installs that exact spec; npm then writes `^0.0.31` into the scratch `package.json`, +but the adjacent `package-lock.json` resolves `pi-acp` to exactly `0.0.31`. The effective +version therefore follows the registry JSON and moves only when the registry publishes a new +entry. Levers to control it, visible as strings in the CLI binary: + +- `SANDBOX_AGENT_ACP_REGISTRY_URL` overrides the registry URL (default + `https://cdn.agentclientprotocol.com/registry/v1/latest/registry.json`). Pointing it at a + registry JSON the runner hosts pins the adapter package and version outright, on the host + and inside the sandbox alike. +- Pre-seeding `.../sandbox-agent/bin/agent_processes/pi/` short-circuits the install: the CLI + logs `agent_manager.install_agent_process: already installed` and uses what is there. A + pre-seeded, patched copy is therefore honored. For Daytona, baking that directory into the + snapshot is the clean delivery for the in-sandbox copy. +- `SANDBOX_AGENT_REQUIRE_PREINSTALL` (also read by the CLI) can forbid on-the-fly installs, + making the pre-seeded copy the only possible resolution and turning drift into a loud + failure instead of a silent registry fetch. + +### Daytona measurement and the Pi install-skip interaction + +A Daytona measurement on 2026-07-11 put the probes at about 2.0 seconds inside the EU +sandbox: two `pi --version` at about 750 ms each plus `npm view` at about 305 ms. The sandbox +is worse than the local 1.6 seconds, as predicted, which makes reaching the in-sandbox copy +(Stage 4) worth more than the local fix alone. + +Separately, that measurement found the runner redundantly npm-installs Pi into every sandbox +(about 5.2 seconds) even though the snapshot bakes it. That is being fixed elsewhere as a +configuration change (`AGENTA_AGENT_SANDBOX_PI_INSTALLED=false`) and is out of scope here. It +interacts with this plan in one way: with the install skipped, `daytonaEnvVars` no longer sets +`PI_ACP_PI_COMMAND` (it only sets it when the install runs), so the adapter resolves `pi` from +PATH, the baked `/usr/local/bin/pi`. That changes which `pi` binary the probes spawn and may +change which adapter copy runs. Stage 0's runtime confirmation must be done with that setting +in its final state. + +### The runner already deletes the probe output + +`services/runner/src/tracing/otel.ts` (around lines 735 to 785, `isBannerLine` and +`stripStartupBanner`, plus a streaming variant near line 993) exists solely to remove the +banner and the "New version available" line that these probes produce, because Pi emits them +as the first assistant message chunk. Its own comment (line 753) notes that the "New version +available" notice survives even when `quietStartup` suppresses the rest. This confirms the +gating described above from the consumer side. Once both probes are gone, this stripping code +has nothing left to strip and can be retired. While only `quietStartup` is applied and +`buildUpdateNotice` still runs, the stripping can be narrowed to the two update-notice lines +but not removed. + +### `pi-acp` reads no environment gate for the update check + +Grepping the adapter for `process.env` shows it reads only `HOME`, +`PI_ACP_ENABLE_EMBEDDED_CONTEXT`, `PI_ACP_PI_COMMAND`, and `PI_CODING_AGENT_DIR`. There is no +`NO_UPDATE_NOTIFIER`-style switch and no environment variable that disables `buildUpdateNotice`. +So there is no environment-only way to remove the unconditional probe. Removing it requires +changing the adapter code (a patch to the running copy, or a fix upstream in the `pi-acp` +source followed by a version bump), or replacing the running adapter with one the runner +controls. + +## Fix B: the eager durable mount on chat-only turns + +### Where the mount happens and what it costs + +`services/runner/src/engines/sandbox_agent.ts`, inside `acquireEnvironment`, mounts the durable +working directory before it opens the session: + +``` +// line ~1030 +// Durable cwd: mount BEFORE createSession (so the session opens inside it) and BEFORE +// workspace materialization (so AGENTS.md, harness files, and skills land in the durable +// prefix instead of being hidden under the FUSE mount). +if (environment.mountCreds && !plan.isDaytona) { + await mountLocalDurableCwd("initial"); // line ~1034 +} +``` + +`mountLocalDurableCwd` (line ~888) calls `mountStorage` +(`services/runner/src/engines/sandbox_agent/mount.ts` line 288), which spawns `geesefs` in the +foreground and polls up to about 15 seconds for the mount to serve input/output. On the normal +path this resolves in the 0.55 to 0.6 seconds the profiler measured. This is on the critical +path: nothing else in `acquireEnvironment` proceeds until it returns. + +### Why the mount is placed before the session, not after + +The ordering is deliberate and the code comment states the two reasons. Both are real +dependencies, and both are why "just make it lazy" is not a one-line change. + +1. Workspace materialization writes into the mounted directory. Right after the mount, + `prepareWorkspace` (`services/runner/src/engines/sandbox_agent/workspace.ts` line 43) writes + the instructions file (`AGENTS.md` or `CLAUDE.md`), any harness files, and non-Pi skill + directories into `plan.cwd` with plain `writeFileSync` and `cpSync` (lines 111 to 124). + Because `plan.cwd` is the mount point, those writes land in the durable store and persist to + the next turn. If the mount is deferred, those writes land on plain local disk instead. + +2. A FUSE mount placed over a directory hides what the directory already held. So if the + workspace files were written to local disk first and the mount arrived later, the mount + would shadow them: the harness would stop seeing the `AGENTS.md` and skills it was started + with, and any files it had already written would vanish under the mount and never reach the + durable store. + +3. The session's `cwd` is `plan.cwd` (`createSession({ cwd: plan.cwd, ... })`, + sandbox_agent.ts line ~1218). The harness process opens with that directory as its working + directory. Mounting over a directory that a running process already holds as its working + directory is the same shadowing hazard applied to a live process: the process keeps the old, + pre-mount view. + +### What this means for the "lazy" versus "overlap" framing + +The honest scope is not "make the mount lazy" as a small change. Two framings exist and neither +is free: + +- Lazy mount on first file use. The runner would write workspace files to local disk, start the + session, and mount only when the harness first calls a file tool. To be correct it must then + solve the shadowing problem: after mounting, the workspace files and anything the harness + already wrote must be copied up into the durable store, and the harness's working-directory + view must be refreshed. That is real new machinery, not a reordering. + +- Overlap the mount with the harness cold start. Start the mount as a background promise and let + it run while the session cold-starts (the same cold start that runs the Fix A probes), then + await it before the session actually needs the directory. This is safer than lazy mounting + because it never defers past the point where files must persist. But `prepareWorkspace` + depends on the mount being complete (reason 1 above), so the overlap window is only the part + of the cold start that happens before workspace materialization, not the whole cold start. + Whether that window is large enough to hide 0.55 seconds depends on runtime timing that + static reading cannot supply. See `open-questions.md`, question 3. + +### Verdict on Fix B + +Fix B is genuinely complicated. The eager mount is required, not incidental: workspace files +must persist and must be visible to the harness at session start, and FUSE shadowing makes a +late mount actively wrong rather than merely late. The lazy variant needs new copy-up and +view-refresh machinery. The overlap variant is safer but its saving is bounded by a timing +window we have not measured. The recommendation in `plan.md` is to ship Fix A first and carry +Fix B as a measured follow-up, which the brief explicitly permits. diff --git a/docs/design/agent-workflows/projects/cold-turn-startup/status.md b/docs/design/agent-workflows/projects/cold-turn-startup/status.md new file mode 100644 index 0000000000..399f0b918b --- /dev/null +++ b/docs/design/agent-workflows/projects/cold-turn-startup/status.md @@ -0,0 +1,63 @@ +# Status + +Source of truth for progress and decisions. Update as work proceeds. + +## Current state: planned, upstream research done, not started + +The workspace is research and a plan. No code change has been made. The plan was authored +against the `big-agents` branch on 2026-07-11 from live profiling of the local runner, then +extended the same day with upstream `pi-acp` research and a Daytona in-sandbox measurement. + +## Decisions taken + +- Fix A and Fix B are independent and ship separately. +- Fix A ships first. Fix B is deferred to a measured follow-up (the brief permits dropping it). +- `quietStartup` (Stage 1) is a no-patch win worth taking on its own, even though it removes + only one of the two version-check probes. +- Waiting for an upstream `pi-acp` release is the retirement path for the workaround, not the + delivery path for the fix: no published version removes the probes (0.0.31 is the latest and + still runs both), so the fix ships as a patched or pre-seeded copy, and the upstream issue + (Stage 2b, draft in `upstream-issue-draft.md`) runs in parallel. + +## Key findings that shape the work + +- The `pi-acp` adapter copy that runs the probes is very likely NOT the runner's patchable + `pi-acp@0.0.29` dependency. The `@sandbox-agent` CLI installs its own copy (`0.0.31`) from + the ACP registry under `~/.local/share/sandbox-agent`, and inside Daytona it does the same + within the sandbox. So the pnpm patch mechanism may not reach the running copy at all, and + it definitely does not reach the sandbox copy. Confirming which copy runs + (`open-questions.md` question 1) is the first task and it gates the patch strategy. +- Upstream (github.com/svkozak/pi-acp) is active but has no fix: latest is 0.0.31 (2026-06-17) + and HEAD still runs both probes with no disable switch beyond `quietStartup`, which the + README confirms does not cover the update check. Adjacent open issue #70 complains about the + same update-check notice on protocol grounds. Upstream precedent (env var replaced by the + `quietStartup` setting) suggests a `checkForUpdates: false` proposal is plausible to land. +- The installed adapter version is effectively pinned by the ACP registry JSON (exact + `pi-acp@0.0.31` spec plus a lockfile), and the CLI honors a pre-seeded + `agent_processes/pi/` directory, which is the clean delivery for both the host and the + Daytona snapshot. `SANDBOX_AGENT_ACP_REGISTRY_URL` and `SANDBOX_AGENT_REQUIRE_PREINSTALL` + give a registry-override and a fail-loud lever respectively. + +## Open questions + +See `open-questions.md`. Question 1 (which adapter copy runs) blocks Stage 2 and Stage 4; +question 4 is largely answered (pre-seed the snapshot), with only in-sandbox path verification +left. + +## Next actions + +1. Stage 0: confirm which `pi-acp` copy runs on a cold local turn, with the Pi install-skip + setting in its final state. Record the path here. +2. Stage 1: write `quietStartup: true` into the runner-controlled Pi settings, with a unit test. +3. Stage 2b: file the upstream issue from `upstream-issue-draft.md`. +4. Then Stages 2 through 5 per `plan.md`, choosing the Stage 2 mechanism by the Stage 0 answer. + +## Measurements to record as they land + +- Local baseline: about 1.6 s probes + about 0.55 to 0.6 s mount, per 2026-07-11 profiling. +- Daytona baseline: about 2.0 s probes inside the EU sandbox (two `pi --version` at ~750 ms + each plus `npm view` ~305 ms), measured 2026-07-11. The separate ~5.2 s redundant Pi install + is handled outside this plan (`AGENTA_AGENT_SANDBOX_PI_INSTALLED=false`). +- After Stage 1: expected local drop of about 440 ms (more on Daytona, ~750 ms). +- After Stage 2: expected further local drop of about 670 to 720 ms. +- Daytona cold-turn before and after Stage 4. diff --git a/docs/design/agent-workflows/projects/cold-turn-startup/upstream-issue-draft.md b/docs/design/agent-workflows/projects/cold-turn-startup/upstream-issue-draft.md new file mode 100644 index 0000000000..1a503434ad --- /dev/null +++ b/docs/design/agent-workflows/projects/cold-turn-startup/upstream-issue-draft.md @@ -0,0 +1,47 @@ +# Draft upstream issue for svkozak/pi-acp + +File this at https://github.com/svkozak/pi-acp/issues as part of plan Stage 2b. Adjust the +version numbers if a release has happened since. Related existing issue to cross-reference: +#70 (the update-check notice arriving after the turn ends). + +--- + +Title: Startup version probes add ~1.2-1.6s to every session/new; no way to disable the update check + +## Summary + +`session/new` is blocked by three synchronous child processes before it returns: + +- `buildUpdateNotice()` runs `spawnSync("pi", ["--version"])` and then + `spawnSync("npm", ["view", "@earendil-works/pi-coding-agent", "version"], { timeout: 800 })`, + unconditionally. +- `buildStartupInfo()` runs `spawnSync("pi", ["--version"])` again, unless `quietStartup` is + set. + +Measured on our hosts (pi-acp 0.0.31, pi 0.80.6): each `pi --version` is ~440-750 ms (a full +Node boot plus the Pi bundle import), and `npm view` is ~230-305 ms over the network. Together +they add ~1.2 s (with `quietStartup: true`) to ~1.6 s (default) of latency to every new +session, serialized before the first prompt can be answered. In network-restricted or +high-latency environments (we run pi-acp inside sandboxes) the `npm view` call is also an +unexpected outbound network dependency at session start. + +`quietStartup` cannot remove this: as the README notes, the update notice is still emitted +when `quietStartup` is enabled, so `buildUpdateNotice` and its two spawns always run. + +## Proposal + +Any of these would solve it; happy to send a PR for whichever you prefer: + +1. A setting such as `checkForUpdates: false` (next to `quietStartup`) that skips + `buildUpdateNotice` entirely. +2. Fold the update check under `quietStartup`, so quiet means fully quiet. +3. Make the check non-blocking and cheap: read the installed version from the pi package's own + `package.json` instead of spawning `pi --version`, and run the `npm view` asynchronously + after `session/new` returns rather than blocking it. (Note #70 shows the async notice then + needs to be delivered in-turn, so options 1 or 2 are simpler.) + +## Environment + +- pi-acp: 0.0.31 +- pi (@earendil-works/pi-coding-agent): 0.80.6 +- Client: sandbox-agent (ACP)