feat(rivetkit-agent-os): phase 3 filesystem — mkdir/readdir/exists/move/deleteFile/readFiles/writeFiles/readdirRecursive arms + batch DTOs

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

[claude]

Code Review: Phase 3 Filesystem Actions

Overview

This PR adds eight new filesystem action handlers (mkdir, readdir, exists, move, deleteFile, writeFiles, readFiles, readdirRecursive) to rivetkit-agent-os, and refactors the WriteFileContent/JsonCompatUint8Array DTOs from mod.rs into filesystem.rs where they now live alongside the new batch DTOs. The overall shape follows the existing readFile/writeFile pattern cleanly.

---

Issues

1. Em dashes in doc comments (project convention violation)

Multiple new doc comments use em dashes. CLAUDE.md explicitly prohibits em dashes and says to use periods instead. Affects mkdir, readdir, exists, move_path, delete_file, write_files, read_files, and readdir_recursive doc comments.

2. Misleading readdir doc comment

The comment states "Returns the (unsorted) child names, including . and ..." If this is accurate, the JS shim silently needs to filter these entries on every call. If it is not accurate (the upstream AgentOs::readdir already omits them), the comment is wrong. Either way this needs clarification. If . and .. are actually included, the filtering should probably be done inside this helper rather than pushed to callers.

3. Unnecessary stat reordering

The stat arm was removed from before writeFile and re-inserted after it with no behavioral change. This adds diff noise and makes the history harder to bisect. Reordering without a clear reason should be avoided or done in a separate refactoring commit.

4. Overly wide visibility on moved types

When WriteFileContent and JsonCompatUint8Array lived in mod.rs they were module-private. After the move they are pub in filesystem.rs, and JsonCompatUint8Array::bytes went from a private field to a pub field. Since nothing outside this crate needs to construct these, pub(crate) (or pub(super) for the field) would be the right scope.

5. Forward-looking comment in delete_file

The comment says: "Non-recursive by default; matches JS deleteFile semantics (use delete for the recursive variant when added)."

"When added" references planned future work. CLAUDE.md cautions against documenting things that are not there yet. The non-recursive behaviour is self-explanatory; the note about a future recursive variant belongs in a todo file, not in the source.

6. String content forced to bytes in write_files

All content variants including WriteFileContent::String are converted to FileContent::Bytes. The single-file writeFile arm likely passes FileContent::Text when the variant is a string. This inconsistency means string content sent through writeFiles will be stored differently than through writeFile. Worth confirming this is intentional, or mirroring the single-file logic.

---

Minor Observations

• The batch functions (write_files, read_files) intentionally omit a Result wrapper in favour of per-entry error fields. This is a reasonable design, but a brief comment explaining why there is no top-level Result would prevent future readers from wondering if it was an oversight.
• No tests are added. Given the WriteFileContent untagged-enum deserialization and the new batch DTOs are non-trivial, unit tests for the deserialization paths (especially the ["$Uint8Array", base64] variant in a batch context) would be valuable.

---

Summary

The core logic is sound and follows the established dispatcher pattern. Main actionable items: fix the em dashes, confirm or correct the readdir dot-entry claim, narrow pub visibility on the moved types, drop the forward-looking comment in delete_file, and verify the string-to-bytes coercion in write_files.