fix(documents): hydrate session before RPC reads and edits - #698
Conversation
The AI reads and edits collaborative documents by calling DocumentSession over native Durable Object RPC. Native RPC bypasses partyserver's onStart/onLoad, so on a hibernated session this.document was a fresh, empty Yjs doc: readMarkdownChunk returned empty content until the user opened the doc in a browser (which connects over fetch and hydrates), and applyMarkdownEdits could edit an empty document and checkpoint it over the real content. Introduce CollaborativeContentSession, a base for session DOs whose content lives in a hydrated in-memory Yjs document, exposing ensureLoaded() around the framework's initialization escape hatch. DocumentSession now awaits ensureLoaded() before any RPC method that touches the document. purgeForDeletion deliberately does not, since it only wipes storage. Future editable item types (flashcards, etc.) extend the same base so the guarantee and its rationale live in one place instead of being rediscovered per type. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
📝 WalkthroughWalkthroughAdds a shared ChangesCollaborative content hydration
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/features/workspaces/documents/document-session.ts (1)
134-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd cold native-RPC regression coverage.
These hydration calls fix the specific path where
onStartis bypassed, but the PR notes that the current suite does not reproduce it. Please add focused coverage for a coldreadMarkdownChunkandapplyMarkdownEditscall to ensure persisted content is loaded before either operation.Also applies to: 180-180
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/documents/document-session.ts` at line 134, Add focused regression tests for cold readMarkdownChunk and applyMarkdownEdits calls, starting with a fresh document-session instance whose onStart path is bypassed and persisted content is not yet hydrated. Verify each operation loads the persisted content before proceeding, covering the ensureLoaded calls in the corresponding methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/features/workspaces/documents/document-session.ts`:
- Line 134: Add focused regression tests for cold readMarkdownChunk and
applyMarkdownEdits calls, starting with a fresh document-session instance whose
onStart path is bypassed and persisted content is not yet hydrated. Verify each
operation loads the persisted content before proceeding, covering the
ensureLoaded calls in the corresponding methods.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 780133bc-84c3-4d7d-931b-037cc0b2f889
📒 Files selected for processing (2)
src/features/workspaces/collaborative-content-session.tssrc/features/workspaces/documents/document-session.ts
There was a problem hiding this comment.
No issues found across 2 files
Auto-approved: Fixes a data-loss bug where native DO RPC to a hibernated session would read/write an empty document. The fix is a small, bounded hydration call before each RPC method, with deliberate exclusion for deletion.
Re-trigger cubic
Problem
The AI reads and edits collaborative documents by calling
DocumentSessionover native Durable Object RPC (getByName(...).readMarkdownChunk(...)/.applyMarkdownEdits(...)).Native DO RPC bypasses partyserver's
onStart/onLoad.DocumentSessionsetshibernate: trueand keeps its canonical content as an in-memory Yjs document that onlyonLoadhydrates. So against a hibernated session:readMarkdownChunkserialized a fresh, empty document → the AI saw the document as empty until the user opened it in a browser (the WebSocket connects overfetch, which runsonLoadand hydrates).applyMarkdownEditscould edit that empty document and checkpoint it back to the kernel — overwriting real content. (data-loss risk)Only documents are affected. PDFs/images/folders read from durable SQL + R2 directly, with no in-memory hydration step, so a cold DO returns real data.
Fix
CollaborativeContentSessionbase for session DOs whose content is a hydrated in-memory Yjs document. It exposesensureLoaded()around partyserver's sanctioned init escape hatch, with the reasoning documented once.DocumentSessionawaitsensureLoaded()before every RPC method that touches the document.purgeForDeletiondeliberately does not (it only wipes storage).Testing
tsc, lint, full unit suite (195) green.onLoadis coupled to the kernel checkpoint and reproducing "cold RPC skips onStart" in the workers test pool has no existing precedent. Flagging as a follow-up rather than shipping a fragile/falsely-green test.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit