test(rivetkit-agent-os): partial-failure driver test for readFiles batch DTO (Option<content>, Option<error>)

[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/5196
Pull/update this stack: forklift sync
Publish local edits: forklift submit
Merge when ready: forklift merge

[claude]

Code Review

This PR adds a test that covers partial-failure behavior in the readFiles batch DTO, specifically targeting the BatchReadResultDto serialization where Option<content> and Option<error> use #[serde(skip_serializing_if)]. This is a genuinely useful regression guard — the failure mode (fields silently elided on the wire) would otherwise be invisible.

Strengths

• Meaningful coverage: Targets a concrete serialization invariant that none of the existing batch tests covered. Without this, wrong None-handling in BatchReadResultDto could silently return empty results.
• Proper isolation: Fresh actor per test via crypto.randomUUID() key, no cross-test contamination.
• Thorough assertions: Checks path, content, and error for both the success and failure entries, in the correct order.
• Consistent structure: Timeout, useRealTimers, and setup follow the same pattern as all sibling tests.

Minor issues

1. Inline comment fragments (CLAUDE.md: comments must be complete sentences)

// Successful entry: content present, no error field.
// Failed entry: no content, error string surfaced.

Both are noun-phrase fragments. Per repo style these should be sentences:

// The successful entry has content and no error field.
// The failed entry has no content but surfaces a non-empty error string.

2. Redundant optional chaining after a type assertion

expect(typeof results[1].error).toBe("string");
expect(results[1].error?.length).toBeGreaterThan(0);

The typeof check already proves error is a string, so ?. is unnecessary. results[1].error.length is clearer.

3. Content not asserted as defined for the success case

expect(new TextDecoder().decode(results[0].content)).toBe("present");

TextDecoder.decode(undefined) returns "", so this would silently pass if content were undefined — the string mismatch would catch it eventually, but adding expect(results[0].content).toBeDefined() before the decode makes the failure message immediate and unambiguous, consistent with how results[1].content is handled.

---

None of these are blockers. The test adds real value and the logic is correct. Addressing the comment style and the optional-chaining nit before merge would keep the file consistent with the rest of the suite.