Skip to content

feat(pi): add pi actor integration - #5767

Open
eersnington wants to merge 9 commits into
feat/sandbox-adapterfrom
feat/headless-pi-actor
Open

eersnington wants to merge 9 commits into
feat/sandbox-adapterfrom
feat/headless-pi-actor

Conversation

@eersnington

@eersnington eersnington commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Adds @rivet-dev/pi, which runs one Pi coding-agent session per Rivet Actor.

import { pi } from "@rivet-dev/pi";
import { agentOSProvider } from "@rivet-dev/sandbox-adapter/agentos";

const agent = pi({
	model: "anthropic/claude-opus-5-5",
	scopedModels: ["anthropic/claude-opus-5-5", "openai-codex/gpt-6-astra"],
	sandbox: agentOSProvider({ actor: "vm" }),
	credentials: (c) => c.client().credentials.getOrCreate([c.key[0]]), // optional: the user's own subscription
});

const conn = client.agent.getOrCreate(["alice", "chat-1"]).connect();
conn.on("event", (e) => { /* every Pi event */ });
await conn.prompt("fix the failing test");
await conn.setModel("openai-codex", "gpt-6-astra");
  • pi() wraps actor(). Pi's session methods are actions, and every Pi event is broadcast on event.
  • The session is stored one entry per row in actor SQLite and restored on wake.
  • Pi's file and shell tools run in a sandbox from feat(sandbox-adapter): add sandbox providers for agentOS, E2B, and Daytona #5799. Without one they are disabled.
  • setModel switches only within scopedModels.
  • credentials supplies provider logins, such as a user's own subscription.
  • Each run is an invoke_agent pi span.

Security:

  • The host environment never reaches the sandbox, Pi never receives a refresh token, and setModel accepts only scopedModels, so a client cannot send a key to its own URL.

This is part 2 of 3 in a stack:

@railway-app

railway-app Bot commented Sep 22, 2026

Copy link
Copy Markdown

This PR was not deployed automatically as @eersnington does not have access to the Railway project.

In order to get automatic PR deploys, please add @eersnington to your workspace on Railway.

@claude

claude Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Review: feat(pi): add pi actor integration

Reviewed the new @rivet-dev/pi package (integrations/pi/). Overall this is a well-structured, carefully-designed integration — the security-sensitive paths (credential handling, sandbox path resolution, host-env isolation) are deliberately reasoned about and backed by tests that actually exercise them (e.g. a sandbox command cannot read the actor host's environment, a client cannot switch to a model outside the allowlist). A few notes below, nothing blocking.

Code quality / consistency

  • actions.ts:246 — executeBash throws a plain new Error("executeBash needs a sandbox...") instead of a UserError with a code, unlike every other user-facing validation in this file (setModel → user.model_not_allowed/user.model_unavailable, etc.). A client catching structured errors from other actions won't get a matching .code here and will see a generic internal error instead. Worth using UserError for consistency, e.g. code: "sandbox_required".

Potential bugs

  • Nothing that looks incorrect functionally. I traced through the trickier paths (credential invalidate/refresh caching in credentials.ts, the sandbox path-escape guard in sandbox.ts:1550 via resolveSandboxPath, the entry-flush serialization in runtime.ts via runtime.writes, and the sleep/destroy sandbox lifecycle in closePiSession) and they hold up, including the edge cases (failed first start before a sandbox row is written, provider mismatch on reconnect, action-timeout aborting an in-flight Pi run).
  • Minor: in sandbox.ts, the grep/find tools check result.exitCode !== 0 (&& !== 1) and fall back to a generic message like `grep exited with ${result.exitCode}`. Since SandboxExecResult.exitCode can be null on timeout/signal kill, that message renders as "grep exited with null" — cosmetic only, not a correctness issue.

Performance

  • No concerns. Entry writes are correctly serialized through runtime.writes so SQLite inserts keep append order without over-synchronizing across actions. MAX_GREP_OUTPUT_BYTES/MAX_GREP_MATCHES/MAX_FIND_RESULTS caps are sensible for bounding sandbox tool output.

Security

  • The host-key isolation claim ("Pi passes the actor host's environment as options.env... it is not forwarded") is correctly implemented in createSandboxBashOperations and is actually tested (a sandbox command cannot read the actor host's environment).
  • resolveSandboxPath correctly rejects paths that posix.resolve would otherwise place outside the sandbox root (including absolute-path override behavior), so tool file access can't escape cwd.
  • The credential design matches the PR description: SourceCredentialStore never persists a write (modify/delete throw), and toPiCredential never forwards a real refresh token to Pi (refresh: ""), keeping refresh logic and secrets in the application's hands. setModel/switchModel resolve strictly against the allowlisted scopedModels and the runtime's own credentialed snapshot, so a client can't redirect a request by supplying an arbitrary provider/model pair.
  • Shell commands built for the grep/find sandbox tools quote all dynamic segments via shellQuote (standard POSIX single-quote escaping), so params.pattern and resolved paths can't break out of the command string.

Test coverage

  • Good breadth: session persistence across sleep, abort semantics (mid-stream and mid-tool), action-timeout interaction, model allowlisting/credential unavailability, sandbox lifecycle (suspend/resume, replace-on-delete, destroy-on-actor-destroy), and tracing (span parenting, model-error vs. retried-success status). The vi.waitFor calls all have the required adjacent justification comments per this repo's testing convention.
  • Nothing missing stands out for this scope, though it might be worth a follow-up test for the executeBash "no sandbox configured" error path once/if it's converted to a structured UserError.

🤖 Generated with Claude Code

@eersnington
eersnington force-pushed the feat/headless-pi-actor branch 2 times, most recently from 4221a4d to 4f4fcc4 Compare September 25, 2026 15:42
@eersnington
eersnington force-pushed the feat/headless-pi-actor branch from 4f4fcc4 to b004815 Compare September 25, 2026 15:47
@eersnington
eersnington marked this pull request as ready for review September 25, 2026 15:52

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 3 medium-severity findings

Reviewed commit b004815.

Comment on lines +35 to +56
const stdout: Uint8Array[] = [];
const stderr: Uint8Array[] = [];
const result = (exitCode: number | null, timedOut: boolean): SandboxExecResult => ({
exitCode,
timedOut,
stdout: Buffer.concat(stdout).toString(),
stderr: Buffer.concat(stderr).toString(),
});
const deadline =
options.timeoutMs === undefined ? undefined : Date.now() + options.timeoutMs;
let after: number | undefined;

while (true) {
if (options.signal?.aborted) {
await killQuietly(process);
throw new Error("aborted");
}
const { chunks, exit } = await process.poll(after);
for (const chunk of [...chunks].sort((a, b) => a.sequence - b.sequence)) {
if (after !== undefined && chunk.sequence <= after) continue;
after = chunk.sequence;
(chunk.stream === "stdout" ? stdout : stderr).push(chunk.data);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Medium · Bound output retained by remote processes

Every chunk is retained until exit and then concatenated, even when the caller only needs streamed onData plus the exit code (as Pi's bash tool does). An authorized prompt or executeBash call can emit data continuously and exhaust the actor's memory before its timeout; the remote backend already retains the same logs, so this is also duplicate buffering. Add a bounded/spooled collection policy, and let streaming-only callers disable full stdout/stderr accumulation.

Comment on lines +47 to +52
while (true) {
if (options.signal?.aborted) {
await killQuietly(process);
throw new Error("aborted");
}
const { chunks, exit } = await process.poll(after);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Medium · Enforce cancellation while a poll is in flight

The timeout and abort checks cannot run while process.poll() is pending. A stalled agentOS action or sandbox-agent HTTP request therefore defeats both controls: the helper never reaches the deadline check and never kills the remote process, despite its contract. Race each poll against the abort signal and remaining deadline, then kill the process when either wins.

Comment on lines +68 to +76
return { isDirectory: () => stat.isDirectory };
},
},
});
const find = createFindToolDefinition(root, {
operations: {
exists: (path) => sandbox.exists(resolvePath(path)),
glob: async (pattern, searchDirectory, options) => {
const searchRoot = resolvePath(searchDirectory);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Medium · Preserve ignore semantics in sandbox search tools

This replacement for Pi's built-in find discards options.ignore and enumerates every file except two hard-coded directories; the custom grep below has the same limitation. Pi advertises these tools as respecting .gitignore, so sandboxed projects now return ignored build artifacts and secret files that the normal tools omit, and large ignored trees can dominate/truncate results. Implement the operation's ignore list plus repository ignore rules (for example with fd/rg or an equivalent fallback) for both tools.

@eersnington
eersnington force-pushed the feat/headless-pi-actor branch from b004815 to 1e7248d Compare September 25, 2026 22:24
@eersnington eersnington changed the title feat(pi): add pi actor integration and sandbox adapter feat(pi): add pi actor integration Sep 25, 2026
@eersnington
eersnington changed the base branch from main to feat/sandbox-adapter September 25, 2026 22:25

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 5 medium-severity findings

Reviewed commit 1e7248d.


🟠 Medium · Bound output retained by remote processes

Every chunk is still retained until exit and then concatenated, even when the caller only needs streamed onData plus the exit code (as Pi's bash tool does). A high-output command can exhaust the actor's memory before its timeout; agentOS already retains the same logs, so this is duplicate buffering. Add a bounded/spooled collection policy, and let streaming-only callers disable full stdout/stderr accumulation.

Original location: "shared/typescript/sandbox-adapter/src/remote-process.ts":56 (new side, not submitted inline).


🟠 Medium · Enforce cancellation while a poll is in flight

The timeout and abort checks still cannot run while process.poll() is pending. A stalled agentOS action therefore defeats both controls: the helper never reaches the deadline check and never kills the remote process, despite its contract. Race each poll against the abort signal and remaining deadline, then kill the process when either wins.

Original location: "shared/typescript/sandbox-adapter/src/remote-process.ts":52 (new side, not submitted inline).


🟠 Medium · Honor abort signals in the Daytona adapter

This provider never observes options.signal, although Sandbox.exec promises that aborting kills the process and rejects with Error("aborted"). Consequently abort(), actor action timeouts, and shutdown cannot stop a Daytona command; executeCommand continues until its own optional timeout (or indefinitely when none was supplied). Use a cancellable/background Daytona process API and terminate it when the signal fires, including the already-aborted case.

Original location: "shared/typescript/sandbox-adapter/src/daytona.ts":54 (new side, not submitted inline).


🟠 Medium · Report provider command timeouts as timeouts

Both new providers hard-code timedOut: false (also e2b.ts:74), so a provider-side command deadline can never satisfy SandboxExecResult's timeout contract. createSandboxBashOperations only converts timedOut: true into Pi's expected timeout:<seconds> error; with these adapters a timed-out command is instead surfaced as a normal nonzero exit or an SDK exception. Detect each SDK's timeout result/error and return { exitCode: null, timedOut: true, ... }.

Original location: "shared/typescript/sandbox-adapter/src/daytona.ts":62 (new side, not submitted inline).

Comment on lines +68 to +76
return { isDirectory: () => stat.isDirectory };
},
},
});
const find = createFindToolDefinition(root, {
operations: {
exists: (path) => sandbox.exists(resolvePath(path)),
glob: async (pattern, searchDirectory, options) => {
const searchRoot = resolvePath(searchDirectory);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Medium · Preserve ignore semantics in sandbox search tools

This replacement for Pi's built-in find still discards options.ignore and enumerates every file except two hard-coded directories; the custom grep below has the same limitation. Pi advertises these tools as respecting .gitignore, so ignored build artifacts and secret files enter results and large ignored trees can dominate/truncate them. Implement the operation's ignore list plus repository ignore rules for both tools.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant