chore(core): migrate actor persist & file system to bare

[NathanFlurry]

Fixes KIT-247

[claude]

Claude finished @NathanFlurry's task —— View job

---

PR Review: Migrate Actor Persist & File System to BARE

Completed comprehensive review of the migration from custom serialization to BARE encoding for actor persistence and file system functionality.

Review Progress

• Read project guidelines and understand BARE migration context
• Analyze new BARE schema definitions
• Review core actor implementation changes
• Examine driver implementation updates
• Check for potential bugs and security issues
• Assess performance implications
• Provide comprehensive feedback

---

✅ Code Quality & Best Practices

Strengths:

• Clean Architecture: Well-structured migration with clear separation between internal types and BARE serialization format
• Type Safety: Strong TypeScript typing maintained throughout conversion functions
• Versioning Strategy: Implements proper versioned data handlers for forward compatibility (packages/core/src/schemas/actor-persist/versioned.ts:19-25)
• Error Handling: Proper error handling in serialization/deserialization paths
• Code Organization: Logical separation of BARE schema definitions from implementation logic

Minor Improvements:

• Consider adding JSDoc comments to the conversion methods #convertToBarePersisted/#convertFromBarePersisted (packages/core/src/actor/instance.ts:1709-1786) to document the mapping logic

---

⚠️ Potential Issues

1. BigInt/Number Conversion Precision Loss Risk

• Location: packages/core/src/actor/instance.ts:1735,1739,1773,1777
• Issue: Timestamp conversions between BigInt and Number could lose precision for very large timestamps
• Current Code:

  lastSeen: BigInt(conn.lastSeen),     // Number -> BigInt
  lastSeen: Number(conn.lastSeen),     // BigInt -> Number

• Recommendation: Consider validating timestamp ranges or documenting precision limits

2. CBOR Encoding Error Handling

• Location: packages/core/src/actor/instance.ts:1715,1718,1724,1727
• Issue: CBOR encoding operations could throw but aren't wrapped in try-catch
• Recommendation: Add error handling for CBOR operations or document expected behavior

---

🚀 Performance Considerations

Positive Changes:

• Binary Format: BARE provides more efficient binary serialization compared to JSON
• Schema Evolution: Built-in versioning reduces migration overhead
• Reduced Parse Time: Binary format should improve deserialization performance

Considerations:

• Memory Usage: Multiple data transformations (Internal → BARE → Bytes → Storage) create temporary objects
• CBOR Overhead: Still uses CBOR for nested data structures which adds an encoding layer

---

🔒 Security Assessment

Good Practices:

• Data Validation: BARE schema provides structure validation
• Type Safety: Strong typing prevents serialization errors
• No Direct Buffer Access: Proper use of bufferToArrayBuffer utility

No Security Concerns Identified

---

📊 Test Coverage

Current State:

• Found existing test files: driver-file-system.test.ts, driver-memory.test.ts, etc.
• Recommendation: Ensure migration includes tests for:
  • BARE serialization/deserialization round-trips
  • Version compatibility (current schema can read v1 data)
  • Edge cases (empty data, null values, large timestamps)

---

🛠️ Technical Implementation Review

Schema Design (packages/core/schemas/actor-persist/v1.bare):

• Well-structured type definitions
• Proper use of optional types and unions
• Clear field naming and documentation

Conversion Logic (packages/core/src/actor/instance.ts:1709-1786):

• Correct mapping between internal and BARE types
• Proper handling of optional fields
• Consistent use of CBOR for complex nested data

Driver Integration (packages/core/src/drivers/file-system/):

• Clean integration with existing file system driver
• Atomic write operations preserved
• Proper error propagation maintained

---

✨ Overall Assessment

This is a well-executed migration that significantly improves RivetKit's persistence layer. The use of BARE encoding provides better performance, type safety, and forward compatibility. The implementation follows project guidelines and maintains code quality standards.

Verdict: ✅ Approved with minor considerations noted above.

The changes are production-ready, though adding tests for the new serialization paths would strengthen confidence in the migration.

---

[NathanFlurry]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• chore: simplify package directories #1205
• chore(core): migrate actor persist & file system to bare #1204 👈 (View in Graphite)
• chore(core): migrate client protocol to BARE #1203
• chore(core): move actor liveness check to setInterval #1202
• fix(core): fix disconnecting sse connections #1201
• feat(core): add abort controller & implement waitUntil #1200
• feat(core): actor sleeping #1178
• fix(core): revert deleted fixtures #1199
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• merge queue - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[graphite-app]

There appears to be a property name inconsistency between the source object and target schema. The source object uses hasInitiated while the BARE schema defines hasInitialized. For consistency with the schema definition, consider changing this line to:

hasInitialized: persist.hasInitialized

This will ensure the property names match between your data model and serialization format, preventing potential issues with data mapping.

Suggested change

	hasInitialized: persist.hasInitiated,
	hasInitialized: persist.hasInitialized,

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

Merge activity

• Sep 11, 10:34 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Sep 11, 10:35 PM UTC: CI is running for this pull request on a draft pull request (#1206) due to your merge queue CI optimization settings.
• Sep 11, 10:35 PM UTC: Merged by the Graphite merge queue via draft PR: #1206.

[linear]

KIT-247 Switch to VBARE