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(),
	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: @rivet-dev/pi

Overall this is a well-structured, well-documented integration. The persistence model (one row per entry, flushed in append order with a retry-safe counter), the model allowlist, the host-env isolation and the sandbox lifecycle handling are thoughtful, and the tests cover the important behaviors (sleep/wake continuity, allowlist, credential refresh, sandbox lifecycle, tracing). Findings are roughly ordered by importance.

Potential issues

  1. All session actions are exposed to any connected client, including executeBash. executeBash, setActiveToolsByName, navigateTree, compact, getMessages and others are ordinary actions, so any client that can reach the actor can run commands in the sandbox and read the full transcript. That may be intended (the description only discusses setModel), but document it explicitly, including that access should be gated with onBeforeConnect/createConnState. Consider an option to disable the host-level actions (executeBash, setActiveToolsByName).
  2. Symlink escape in resolveSandboxPath. The check is lexical (posix.resolve plus prefix). A symlink inside the sandbox cwd pointing outside is followed by read/write/edit. If the sandbox is the real isolation boundary this is fine, but the error text implies a stronger guarantee. Soften the wording or document that the sandbox is the boundary.
  3. find and grep bound output only after collecting it. find . -type f ... lists every file, then filters and slices in JS, and MAX_GREP_OUTPUT_BYTES is applied after full stdout is buffered. On large trees this can exhaust memory or the exec buffer. Push limits into the command (-name/-path, grep -m, | head -n).
  4. Shutdown ordering and a silent swallow in closePiSession. try { handle = await ready; } catch {} discards the error with no log or comment (repo guidance is fail-by-default). Also session.abort() is followed directly by persistPiState and dispose() without waiting for the in-flight run to settle, so entries from a run still unwinding could be missed. Consider a bounded waitForIdle() between abort and persist.
  5. Possible sandbox leak in connectSandbox. If provider.create succeeds and savePiSandbox throws, the new sandbox is never recorded or destroyed. A try/catch that calls provider.destroy (when present) on save failure would close the gap.
  6. Unbounded payloads. getMessages and getSessionTree return whole structures, which can exceed message limits for long sessions. Worth a doc note or pagination later. Not blocking.

Style / conventions

  • actor.ts uses as any and Record<string, any> for the config split. Understandable given the generics, but add a short comment on why the cast is unavoidable.
  • package.json hard-codes "version": "2.3.20". Confirm the publish script rewrites it and that it matches the current release line.
  • The long generic parameter list on pi()/PiActorConfigInput duplicates actor(). Reuse a helper from rivetkit if one exists.

Test coverage

Good coverage of the actor, credentials, models and tracing. Gaps:

  • No direct unit tests for the pure, security-relevant helpers in sandbox.ts: resolveSandboxPath traversal (../, absolute paths outside root, root /), matchesToolGlob, and shellQuote with quotes and newlines.
  • No test for the connectSandbox failure path (create succeeds, save fails) or for closePiSession with several failures (AggregateError).
  • No test for restoring after a crash between createPiSession and the first entry flush.

Security notes

The stated properties hold from what I read: the host environment is not forwarded to the sandbox (options.env is dropped), refresh tokens never reach Pi (refresh: ""), the model object comes from the actor catalog rather than the client, and setModel is limited to scopedModels. The main residual concern is item 1.

Items 1 and 4 are the ones I would address before merging.

@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.

@eersnington
eersnington force-pushed the feat/headless-pi-actor branch from 1e7248d to 1448347 Compare September 25, 2026 23:40
@eersnington
eersnington force-pushed the feat/headless-pi-actor branch 2 times, most recently from 5719ca6 to ff2b289 Compare September 29, 2026 18:22

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