feat(agent-os): use agent-os-client from crates.io and pass sidecar binary path from npm package

[NathanFlurry]

No description provided.

[NathanFlurry]

Stack for rivet-dev/rivet

Current stack:

• feat(agent-os): use agent-os-client from crates.io and pass sidecar binary path from npm package #5234 xkllkonn · 2026-06-12 18:59:42 👈

Dependencies:

• feat(rivetkit-agent-os): phase 3 process — 11 arms (exec/spawn/wait/kill/stop/list/all/tree/get/writeStdin/closeStdin) + ExecResultDto #5197 unkrsszt · 2026-06-08 18:29:29
• test(rivetkit-agent-os): partial-failure driver test for readFiles batch DTO (Option<content>, Option<error>) #5196 pzzzpmtr · 2026-06-08 18:29:28
• feat(rivetkit-agent-os): phase 3 filesystem — mkdir/readdir/exists/move/deleteFile/readFiles/writeFiles/readdirRecursive arms + batch DTOs #5195 onyykwlk · 2026-06-08 18:29:27
• test(rivetkit-agent-os): config parsing tests + documented JS↔Rust software shape mismatch (phase 3 prep) #5194 ouyvrlxt · 2026-06-08 18:29:26
• feat(rivetkit-agent-os): phase 2 — stat action arm + structured-payload wrapper tests for cross-encoding parity #5193 slkvonmt · 2026-06-08 18:29:25
• feat(rivetkit): phase 1c — agentOs() native ladder + engine driver branch + driver-suite e2e green #5192 nxzrussy · 2026-06-08 18:29:24
• feat(rivetkit-napi): phase 1b — NapiActorFactory.fromAgentOs static constructor + writeFile arm + e2e factory round-trip #5191 mtlnunov · 2026-06-08 18:29:22
• feat(rivetkit-agent-os): phase 1a — rust crate skeleton + readFile arm + dispatcher e2e against real sidecar #5190 wrxkrssv · 2026-06-08 18:29:21
• feat(rivetkit): JSON_COMPAT_UINT8_ARRAY byte-payload encoding parity with TS (encode + decode + cross-language tests) #5189 ukssumln · 2026-06-08 18:29:20
• chore: phase 0 prereqs (rivetkit workspace member, serde_bare align, wasm skip.agentOs default) #5188 qvnkvkxw · 2026-06-08 18:29:19
• docs: PLAN2 + TODOLIST + RIVETKIT_RUST_FIX (clean slate, post-wipe) #5187 yqrwozvn · 2026-06-08 18:29:18
• main

Get stack: forklift get 5234
Push local edits: forklift submit
Merge when ready: forklift merge 5234

[claude]

PR Review: feat(agent-os): use agent-os-client from crates.io and pass sidecar binary path from npm package\n\nGood overall direction — moving from path deps to published crates and wiring up the sidecar binary path from npm are both the right calls. A few issues worth addressing before merge.\n\n---\n\n### Critical\n\n**unsafe { std::env::set_var } in rivetkit-typescript/packages/rivetkit-napi/src/agent_os.rs:68-78\n\nThe SAFETY comment claims this runs once before any VM is created, but this assumption can be violated: (1) if createAgentOsFactory is called more than once (tests, hot-reload), concurrent calls race on the env var; (2) Node.js libuv/thread-pool threads can read env vars concurrently with this write; (3) it is a process-global side effect affecting every dependency that reads AGENT_OS_SIDECAR_BIN. The safer fix is to pass the path through the structured config JSON alongside software and moduleAccessCwd, and have Rust config parsing forward it to the client directly, avoiding the global mutation entirely.\n\n---\n\n### Memory\n\nPreviewStore grows unbounded in actions/preview.rs\n\nExpired tokens are only evicted lazily on resolve lookup. An actor that creates many preview URLs and never explicitly calls expireSignedPreviewUrl will accumulate stale entries indefinitely. At minimum, create should sweep expired entries before inserting, or the run loop should run a periodic GC pass.\n\n---\n\n### Correctness\n\nMulti-value HTTP headers silently dropped**\n\nIn run.rs::proxy_preview and actions/network.rs::fetch, response headers are collected into a HashMap/BTreeMap keyed by name. insert silently overwrites duplicates, corrupting legitimately multi-valued headers like Set-Cookie. Consider Vec<(String, String)> to preserve all values, or document the limitation.\n\nInconsistent header UTF-8 handling between proxy and fetch\n\nproxy_preview uses String::from_utf8_lossy (replaces bad bytes with the replacement character) while network::fetch uses to_str().unwrap_or_default() (returns empty string). Pick one and use it consistently.\n\n---\n\n### Style (per CLAUDE.md)\n\n**map_err(|e| anyhow!(e)) pattern in cron.rs and session.rs\n\nCLAUDE.md: "Prefer .context() over the anyhow! macro." If the underlying error already implements std::error::Error, ? or .map_err(anyhow::Error::from) suffices. If context is useful, use .context(\"...\").\n\n---\n\n### Minor\n\nUnsafe cast in flattenSoftware (agent-os/actor/index.ts:42)\n\nout.push(input as SoftwareDescriptorLike) bypasses type checking. The downstream typeof d.commandDir === \"string\" guards handle bad shapes at runtime, but a plain-object check (Object.getPrototypeOf(input) === Object.prototype) would be more precise.\n\nTruthy check for kind classification may misfire (actor/index.ts:55)**\n\nd.hostTool || d.toolkit ? \"tool\" : \"agent\" misclassifies if either field is falsy but present (empty string, 0, false). Prefer d.hostTool != null || d.toolkit != null.\n\n---\n\n### Non-issues\n\n- _ => in mod.rs::dispatch matches on &str, not an enum — CLAUDE.md's explicit-variant rule does not apply.\n- f64 for epoch-millis timestamps is fine; current values (~1.7e12) are well within 2^53.\n- PreviewStore as a plain HashMap is correct — it is single-threaded local state owned by the run loop.