Repository navigation
fix(studio): harden draft durability and recovery - #14
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (27)
📝 WalkthroughWalkthroughStudio gains durable draft recovery, atomic conflict-aware file writes, serialized mutations, secure writer leases, isolated MDX compilation, server recovery APIs, browser IndexedDB recovery, and preview updates that preserve the existing preview document. ChangesStudio recovery and mutation flow
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
aetosdios27
marked this pull request as ready for review
July 25, 2026 00:56
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
packages/cli/src/studio-ui.ts (2)
371-377: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
revisionin the body is unused by the server.The coordinator reads
baseRevisionfrom the envelope thatrequest()injects; this extrarevisionfield is silently ignored and invites confusion about which value gates staleness.🤖 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 `@packages/cli/src/studio-ui.ts` around lines 371 - 377, Remove the unused revision field from the JSON body built in the request flow around revisionRef.current, leaving baseSource and baseDiskVersion unchanged. Preserve the coordinator’s use of the injected baseRevision envelope field as the sole staleness-gating value.
209-217: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAdd
onabortso an aborted delete cannot leave the promise pending.
writeBrowserRecoveryhandlesonabort;clearBrowserRecoverydoes not, so a quota/abort event never settles this promise.♻️ Suggested tweak
transaction.oncomplete = () => resolve(); transaction.onerror = () => reject(transaction.error); + transaction.onabort = () => reject(transaction.error); });🤖 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 `@packages/cli/src/studio-ui.ts` around lines 209 - 217, Update clearBrowserRecovery’s delete transaction promise to register an onabort handler that rejects with the transaction error, matching writeBrowserRecovery and ensuring aborted deletes always settle the promise.packages/cli/src/studio.test.ts (1)
141-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis assertion cannot distinguish cross-origin rejection from missing-token rejection.
authorizeStudioMutationchecks the session token first, so the request is rejected before the origin is ever inspected. Removing the cross-site/origin checks entirely would keep this test green. Add a case that supplies a validsessionTokenwith a foreignorigin(and/orsec-fetch-site: cross-site).🧪 Suggested addition
expect(unauthenticated.status).toBe(403); + + const crossOrigin = await fetch(`${handle.origin}/__scribe/api/discard`, { + method: "POST", + headers: { + "content-type": "application/json", + "x-scribe-studio-session": handle.sessionToken, + origin: "https://attacker.invalid", + "sec-fetch-site": "cross-site" + }, + body: "{}" + }); + expect(crossOrigin.status).toBe(403);🤖 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 `@packages/cli/src/studio.test.ts` around lines 141 - 146, Add a separate test case in the studio mutation authorization tests that sends a valid sessionToken while using a foreign origin and/or sec-fetch-site: cross-site, then asserts rejection. Keep the existing unauthenticated assertion, and ensure the new case exercises cross-origin validation after token authentication rather than missing-token handling.
🤖 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.
Inline comments:
In `@packages/cli/src/studio-compiler.test.ts`:
- Around line 12-45: Replace all six hardcoded “/tmp/...” compile identifiers in
the StudioCompiler tests, including the calls in the validation, event-loop,
worker-restart, and timeout cases, with a portable relative-style placeholder
while keeping the test behavior unchanged.
In `@packages/cli/src/studio-files.ts`:
- Around line 129-139: Update packages/cli/src/studio-files.ts lines 129-139 by
moving the Windows EINVAL/ENOTSUP/EPERM tolerance to cover the open call itself,
then extract syncDirectory into a shared module. In
packages/cli/src/studio-recovery.ts lines 188-198, remove the duplicate
syncDirectory implementation and import the shared helper so both callers use
identical handling.
In `@packages/cli/src/studio-recovery.ts`:
- Around line 188-198: Remove the duplicate syncDirectory implementation from
the current module and reuse a single shared helper with the existing
Windows-specific error tolerance. Update both callers/modules, including the
syncDirectory definition in studio-files.ts, to import or otherwise reference
that shared helper so platform handling is maintained in one place.
- Around line 105-111: Update `#trimArchives` to exclude in-flight temporary
archive files ending in .tmp from the entries eligible for deletion, while
preserving the existing active-record and .corrupt exclusions; if practical,
restrict matching to the recognized archive reason suffixes so unrelated files
are not trimmed.
- Around line 35-43: Update loadDraft so `#quarantineCorruptRecord` is invoked
only when reading succeeds but JSON parsing or validateRecord fails; propagate
non-ENOENT read errors such as EACCES or EIO to the caller instead of returning
undefined. Preserve the existing undefined result for ENOENT and ensure
startStudio handles surfaced read failures as diagnostics.
In `@packages/cli/src/studio-transactions.ts`:
- Around line 31-53: Update StudioTransactions.mutate around the enqueued result
and `#operations` cache so failed mutations evict their deduplication entry,
allowing a later call with the same clientId and operationId to rerun
operation(). Preserve caching for successful, stale, and rejected decision
results, and ensure thrown operation failures remove only their corresponding
key.
In `@packages/cli/src/studio-ui.ts`:
- Around line 861-870: Update discard() to check the discard request response
status before calling apply(body, true). For unsuccessful 423/409 responses,
avoid updating editor state, preserve the current session, and surface the
returned error through the existing UI error handling; only apply the body and
reset author mode/session state for successful responses containing the expected
source data.
- Around line 497-498: Update the writer state in the studio UI component to use
a nullable value, initializing it as unknown rather than false. Treat writer ===
null as writable while the lease request is pending, keeping the textarea
editable and hiding read-only status/affordances; render those restrictions only
after the lease explicitly resolves to false.
In `@packages/cli/src/studio.ts`:
- Around line 318-360: Update the error handling in handleExternalChange so
transient ENOENT failures from readStudioFile are ignored, leaving the current
editor state and diagnostics unchanged while waiting for the follow-up
filesystem event. For other errors, preserve the existing conflict state,
watcher diagnostic, and event publication behavior in the catch handler.
---
Nitpick comments:
In `@packages/cli/src/studio-ui.ts`:
- Around line 371-377: Remove the unused revision field from the JSON body built
in the request flow around revisionRef.current, leaving baseSource and
baseDiskVersion unchanged. Preserve the coordinator’s use of the injected
baseRevision envelope field as the sole staleness-gating value.
- Around line 209-217: Update clearBrowserRecovery’s delete transaction promise
to register an onabort handler that rejects with the transaction error, matching
writeBrowserRecovery and ensuring aborted deletes always settle the promise.
In `@packages/cli/src/studio.test.ts`:
- Around line 141-146: Add a separate test case in the studio mutation
authorization tests that sends a valid sessionToken while using a foreign origin
and/or sec-fetch-site: cross-site, then asserts rejection. Keep the existing
unauthenticated assertion, and ensure the new case exercises cross-origin
validation after token authentication rather than missing-token handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c7a498e9-0be1-4726-bd90-d6c7963c10e5
📒 Files selected for processing (22)
.changeset/durable-studios-recover.mdpackages/cli/src/rich-preservation.tspackages/cli/src/studio-compiler.test.tspackages/cli/src/studio-compiler.tspackages/cli/src/studio-events.test.tspackages/cli/src/studio-events.tspackages/cli/src/studio-files.test.tspackages/cli/src/studio-files.tspackages/cli/src/studio-lease.test.tspackages/cli/src/studio-lease.tspackages/cli/src/studio-recovery.test.tspackages/cli/src/studio-recovery.tspackages/cli/src/studio-security.test.tspackages/cli/src/studio-security.tspackages/cli/src/studio-transactions.test.tspackages/cli/src/studio-transactions.tspackages/cli/src/studio-ui.test.tspackages/cli/src/studio-ui.tspackages/cli/src/studio.test.tspackages/cli/src/studio.tsplaywright.studio.config.tstests/studio-browser/studio.spec.ts
9 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linear
Refs AET-15
Summary
Verification
git diff --checkpassesVisual evidence
No intentional visual change. The focused Chromium Studio flow verifies that draft updates reach the production preview through HMR without replacing the iframe document.
Summary by CodeRabbit
New Features
Bug Fixes