mirror of
https://github.com/harivansh-afk/sandbox-agent.git
synced 2026-04-15 07:04:48 +00:00
fix: normalize Pi ACP bootstrap payloads (#227)
* fix: normalize pi ACP bootstrap payloads * docs(cli): document custom pi binary override * docs(quickstart): list all supported agent IDs * docs(code): clarify Pi payload normalization rationale
This commit is contained in:
parent
e740d28e0a
commit
d850a3b77a
4 changed files with 190 additions and 1 deletions
33
docs/cli.mdx
33
docs/cli.mdx
|
|
@ -59,6 +59,39 @@ sandbox-agent install-agent claude --reinstall
|
|||
sandbox-agent install-agent --all
|
||||
```
|
||||
|
||||
### Custom Pi implementation path
|
||||
|
||||
If you use a forked/custom `pi` binary with `pi-acp`, you can override what executable gets launched.
|
||||
|
||||
#### Option 1: explicit command override (recommended)
|
||||
|
||||
Set `PI_ACP_PI_COMMAND` in the environment where `sandbox-agent` runs:
|
||||
|
||||
```bash
|
||||
PI_ACP_PI_COMMAND=/absolute/path/to/your/pi-fork sandbox-agent server
|
||||
```
|
||||
|
||||
This is forwarded to `pi-acp`, which uses it instead of looking up `pi` on `PATH`.
|
||||
|
||||
#### Option 2: PATH override
|
||||
|
||||
Put your custom `pi` first on `PATH` before starting `sandbox-agent`:
|
||||
|
||||
```bash
|
||||
export PATH="/path/to/custom-pi-dir:$PATH"
|
||||
sandbox-agent server
|
||||
```
|
||||
|
||||
#### Option 3: symlink override
|
||||
|
||||
Point `pi` to your custom binary via symlink in a directory that is early on `PATH`:
|
||||
|
||||
```bash
|
||||
ln -sf /absolute/path/to/your/pi-fork /usr/local/bin/pi
|
||||
```
|
||||
|
||||
Then start `sandbox-agent` normally.
|
||||
|
||||
## opencode (experimental)
|
||||
|
||||
Start/reuse daemon and run `opencode attach` against `/opencode`.
|
||||
|
|
|
|||
|
|
@ -217,6 +217,8 @@ icon: "rocket"
|
|||
</Step>
|
||||
|
||||
<Step title="Install agents (optional)">
|
||||
Supported agent IDs: `claude`, `codex`, `opencode`, `amp`, `pi`, `cursor`, `mock`.
|
||||
|
||||
To preinstall agents:
|
||||
|
||||
```bash
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ use futures::Stream;
|
|||
use sandbox_agent_agent_management::agents::{AgentId, AgentManager, InstallOptions};
|
||||
use sandbox_agent_error::SandboxError;
|
||||
use sandbox_agent_opencode_adapter::{AcpDispatch, AcpDispatchResult, AcpPayloadStream};
|
||||
use serde_json::Value;
|
||||
use serde_json::{Number, Value};
|
||||
use tokio::sync::{Mutex, RwLock};
|
||||
|
||||
const DEFAULT_REQUEST_TIMEOUT_MS: u64 = 120_000;
|
||||
|
|
@ -134,6 +134,8 @@ impl AcpProxyRuntime {
|
|||
"acp_proxy: instance resolved"
|
||||
);
|
||||
|
||||
let payload = normalize_payload_for_agent(instance.agent, payload);
|
||||
|
||||
match instance.runtime.post(payload).await {
|
||||
Ok(PostOutcome::Response(value)) => {
|
||||
let total_ms = start.elapsed().as_millis() as u64;
|
||||
|
|
@ -510,6 +512,64 @@ fn map_adapter_error(err: AdapterError, agent: Option<AgentId>) -> SandboxError
|
|||
}
|
||||
}
|
||||
|
||||
fn normalize_payload_for_agent(agent: AgentId, payload: Value) -> Value {
|
||||
if agent != AgentId::Pi {
|
||||
return payload;
|
||||
}
|
||||
|
||||
// Pi's ACP adapter is stricter than other adapters for a couple of bootstrap
|
||||
// fields. Normalize here so older/raw ACP clients still work against Pi.
|
||||
normalize_pi_payload(payload)
|
||||
}
|
||||
|
||||
fn normalize_pi_payload(mut payload: Value) -> Value {
|
||||
let method = payload
|
||||
.get("method")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or_default();
|
||||
|
||||
match method {
|
||||
"initialize" => {
|
||||
// Some clients send ACP protocolVersion as a string ("1.0"), but
|
||||
// pi-acp expects a numeric JSON value and rejects strings.
|
||||
if let Some(protocol) = payload.pointer_mut("/params/protocolVersion") {
|
||||
if let Some(raw) = protocol.as_str() {
|
||||
if let Some(number) = parse_json_number(raw) {
|
||||
*protocol = Value::Number(number);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
"session/new" => {
|
||||
// The TypeScript SDK and opencode adapter already send mcpServers: [],
|
||||
// but raw /v1/acp callers may omit it. pi-acp currently validates
|
||||
// mcpServers as required, so default it here for compatibility.
|
||||
if let Some(params) = payload.get_mut("params").and_then(Value::as_object_mut) {
|
||||
params
|
||||
.entry("mcpServers".to_string())
|
||||
.or_insert_with(|| Value::Array(Vec::new()));
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
||||
payload
|
||||
}
|
||||
|
||||
fn parse_json_number(raw: &str) -> Option<Number> {
|
||||
let trimmed = raw.trim();
|
||||
|
||||
if let Ok(unsigned) = trimmed.parse::<u64>() {
|
||||
return Some(Number::from(unsigned));
|
||||
}
|
||||
|
||||
if let Ok(signed) = trimmed.parse::<i64>() {
|
||||
return Some(Number::from(signed));
|
||||
}
|
||||
|
||||
trimmed.parse::<f64>().ok().and_then(Number::from_f64)
|
||||
}
|
||||
|
||||
/// Inspect JSON-RPC error responses from agent processes and add helpful hints
|
||||
/// when we can infer the root cause from a known error pattern.
|
||||
fn annotate_agent_error(agent: AgentId, mut value: Value) -> Value {
|
||||
|
|
|
|||
|
|
@ -33,6 +33,51 @@ done
|
|||
write_executable(path, &script);
|
||||
}
|
||||
|
||||
fn write_strict_pi_agent_process(path: &Path) {
|
||||
// This stub intentionally mirrors the strict bootstrap validation behavior
|
||||
// observed in pi-acp:
|
||||
// - initialize.params.protocolVersion must be numeric
|
||||
// - session/new.params.mcpServers must be present (array)
|
||||
//
|
||||
// The proxy normalization layer should adapt legacy/raw client payloads so
|
||||
// requests still succeed against this stricter contract.
|
||||
let script = r#"#!/usr/bin/env sh
|
||||
if [ "${1:-}" = "--help" ] || [ "${1:-}" = "--version" ] || [ "${1:-}" = "version" ] || [ "${1:-}" = "-V" ]; then
|
||||
echo "pi-agent-process 0.0.1"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
while IFS= read -r line; do
|
||||
method=$(printf '%s\n' "$line" | sed -n 's/.*"method"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p')
|
||||
id=$(printf '%s\n' "$line" | sed -n 's/.*"id"[[:space:]]*:[[:space:]]*\([^,}]*\).*/\1/p')
|
||||
|
||||
if [ "$method" = "initialize" ] && [ -n "$id" ]; then
|
||||
if printf '%s\n' "$line" | grep -Eq '"protocolVersion"[[:space:]]*:[[:space:]]*"'; then
|
||||
printf '{"jsonrpc":"2.0","id":%s,"error":{"code":-32603,"message":"Internal error","data":[{"expected":"number","code":"invalid_type","path":["protocolVersion"],"message":"Invalid input: expected number, received string"}]}}\n' "$id"
|
||||
else
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"ok":true,"echoedMethod":"initialize"}}\n' "$id"
|
||||
fi
|
||||
continue
|
||||
fi
|
||||
|
||||
if [ "$method" = "session/new" ] && [ -n "$id" ]; then
|
||||
if printf '%s\n' "$line" | grep -Eq '"mcpServers"[[:space:]]*:[[:space:]]*\['; then
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"sessionId":"pi-session","echoedMethod":"session/new"}}\n' "$id"
|
||||
else
|
||||
printf '{"jsonrpc":"2.0","id":%s,"error":{"code":-32603,"message":"Internal error","data":[{"expected":"array","code":"invalid_type","path":["mcpServers"],"message":"Invalid input: expected array, received undefined"}]}}\n' "$id"
|
||||
fi
|
||||
continue
|
||||
fi
|
||||
|
||||
if [ -n "$method" ] && [ -n "$id" ]; then
|
||||
printf '{"jsonrpc":"2.0","id":%s,"result":{"ok":true,"echoedMethod":"%s"}}\n' "$id" "$method"
|
||||
fi
|
||||
done
|
||||
"#;
|
||||
|
||||
write_executable(path, script);
|
||||
}
|
||||
|
||||
fn setup_stub_artifacts(install_dir: &Path, agent: &str) {
|
||||
let native = install_dir.join(agent);
|
||||
write_stub_native(&native, agent);
|
||||
|
|
@ -47,6 +92,17 @@ fn setup_stub_artifacts(install_dir: &Path, agent: &str) {
|
|||
write_stub_agent_process(&launcher, agent);
|
||||
}
|
||||
|
||||
fn setup_strict_pi_agent_process_only(install_dir: &Path) {
|
||||
let agent_processes = install_dir.join("agent_processes");
|
||||
fs::create_dir_all(&agent_processes).expect("create agent processes dir");
|
||||
let launcher = if cfg!(windows) {
|
||||
agent_processes.join("pi-acp.cmd")
|
||||
} else {
|
||||
agent_processes.join("pi-acp")
|
||||
};
|
||||
write_strict_pi_agent_process(&launcher);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn acp_bootstrap_requires_agent_query() {
|
||||
let test_app = TestApp::new(AuthConfig::disabled());
|
||||
|
|
@ -115,6 +171,44 @@ async fn acp_round_trip_and_replay() {
|
|||
assert!(second_event_id > first_event_id);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn pi_initialize_and_session_new_are_normalized() {
|
||||
let test_app = TestApp::with_setup(AuthConfig::disabled(), |install_dir| {
|
||||
setup_strict_pi_agent_process_only(install_dir);
|
||||
});
|
||||
|
||||
let (status, _, body) = send_request(
|
||||
&test_app.app,
|
||||
Method::POST,
|
||||
"/v1/acp/server-pi?agent=pi",
|
||||
Some(initialize_payload()),
|
||||
&[],
|
||||
)
|
||||
.await;
|
||||
assert_eq!(status, StatusCode::OK);
|
||||
assert_eq!(parse_json(&body)["result"]["echoedMethod"], "initialize");
|
||||
|
||||
let session_new = json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": 2,
|
||||
"method": "session/new",
|
||||
"params": {
|
||||
"cwd": "/tmp"
|
||||
}
|
||||
});
|
||||
let (status, _, body) = send_request(
|
||||
&test_app.app,
|
||||
Method::POST,
|
||||
"/v1/acp/server-pi",
|
||||
Some(session_new),
|
||||
&[],
|
||||
)
|
||||
.await;
|
||||
assert_eq!(status, StatusCode::OK);
|
||||
assert_eq!(parse_json(&body)["result"]["echoedMethod"], "session/new");
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn acp_agent_mismatch_returns_conflict() {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue