test(rivetkit-agent-os): config parsing tests + documented JS↔Rust software shape mismatch (phase 3 prep)

[abcxff]

No description provided.

[abcxff]

Stack for rivet-dev/rivet

• 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

Check out this stack: forklift get https://github.com/rivet-dev/rivet/pull/5194
Pull/update this stack: forklift sync
Publish local edits: forklift submit
Merge when ready: forklift merge

[claude]

Code Review

Overview

This PR adds pure unit tests for the parse_agent_os_options Rust bridge function and documents why software is not yet threaded through the JS→Rust config envelope. The tests are sidecar-free (no AGENT_OS_SIDECAR_BIN needed) and live under the existing #[path] shim pattern.

---

What's Working Well

• Test structure is correct. The parsing module is placed inside tests/agent_os_factory.rs under the existing #[cfg(test)] #[path] shim in agent_os.rs, giving the moved tests private-module access per the repo convention.
• parse_builder_produces_fresh_config_each_call is a meaningful test. It verifies the stored closure is callable multiple times (i.e., not a FnOnce), which is load-bearing since each actor VM bring-up calls the builder again.
• deny_unknown_fields failure path is implicitly exercised by the fact that all four tests pass valid JSON — the full field mapping in AgentOsConfigJson is exercised end-to-end through the Arc<AgentOsConfigJson> builder closure.

---

Issues

Em dashes in TypeScript comment block (style violation)

CLAUDE.md is explicit: "Do not use em dashes (—). Use periods to separate sentences instead." The new JSDoc block added to buildConfigJson contains multiple em dashes, e.g.:

// makes the Rust sidecar look for `./node_modules/<name>` — but the real
// npm package is `@rivet-dev/agent-os-<name>`.

Replace each — with a period and start a new sentence.

Phase-numbered references in comments will rot

CLAUDE.md says: "Do not reference the current task, fix, or callers... since those belong in the PR description and rot as the codebase evolves." Several comments reference "Phase 3", "Phase 1c minimum", and "Phase 3 prep":

• JSDoc on buildConfigJson: "Phase 3 will resolve this when exec/spawn actually need installed software."
• Rust module doc: "Phase 3 prep: verifies that the JSON envelope..."

Once Phase 3 ships, these references become noise. The reason software is not threaded (the package-name mismatch) is the part worth keeping; "Phase 3 will resolve this" is not.

No negative-path tests for deny_unknown_fields

AgentOsConfigJson uses #[serde(deny_unknown_fields)] as the primary guard against silently-ignored fields. There is no test verifying that an unknown field produces an error. Consider adding:

#[test]
fn unknown_field_is_rejected() {
    let options = NapiAgentOsOptions {
        config_json: Some(r#"{"unknownField": true}"#.to_owned()),
    };
    assert!(parse_agent_os_options(options).is_err());
}

Exporting buildConfigJson widens the public surface prematurely

buildConfigJson is now exported so the Rust-side test comment can cross-reference it by name, but the Rust tests do not actually call this TypeScript function. The export widens the module's public API for a function that returns a hard-coded "{}". If this is genuinely needed by external consumers in Phase 3, the export is justified — but that should be stated explicitly. Otherwise, keep it unexported until it is actually needed.

---

Minor Notes

• The test name parse_threads_software_through_to_agent_os_config uses "threads" in the "pass through" sense, which reads momentarily as OS threads. A name like parse_forwards_software_to_agent_os_config avoids the ambiguity.
• parse_preserves_all_supported_fields checks software.len() == 1 but never asserts software[0].package == "coreutils", leaving the software entry partially unverified.