fix(studio): enforce optimistic file concurrency

[jrusso1020]

What

Add optimistic file concurrency across Studio and studio-server.

Why

Concurrent Studio, CLI, or agent writes could silently overwrite newer bytes.

How

Return content versions, require If-Match on updates, surface 409 conflicts, pause the save queue, and propagate write identity through change events.

Test plan

• studio-server file version/route tests; Studio save queue, conflict, and diagnostics tests
• Stack-wide lint, format, build, typecheck, and relevant integration gates

[jrusso1020]

• refactor(repo): resolve changed-code audit #2174
• test(engine): isolate peer abort regression #2173
• refactor(cli): decompose render phases #2172
• refactor(cli): centralize process lifecycle #2171
• fix(cloudrun): publish adapter contract #2170
• test(repo): execute packed subpaths #2169
• refactor(repo): centralize package subpaths #2168
• refactor(core): own edit protocol contract #2167
• test(compiler): enforce preview render parity #2166
• refactor(studio): gate SDK operation families #2165
• refactor(studio): gate Acorn writer migration #2164
• refactor(producer): make capture plans immutable #2163
• refactor(producer): unify render requests #2162
• refactor(engine): type capture failures #2161
• refactor(engine): manage child process lifecycles #2160
• refactor(engine): add fingerprinted browser leases #2159
• refactor(render): centralize execution ownership #2158
• refactor(core): unify composition contract #2157
• fix(studio): enforce optimistic file concurrency #2156 👈 (View in Graphite)
• fix(studio): make sdk cutover transactional #2155
• fix(render): publish artifacts atomically #2154
• fix(render): surface structured outcomes #2153
• fix(player): own connection and media resources #2152
• fix(core): preserve runtime transport contract #2151
• fix(player): version runtime protocol #2150
• test(repo): verify packed consumers #2149
• test(repo): enforce workspace contracts #2148
• fix(core): enforce strict runtime safety #2147
• test(producer): gate source tests by execution lane #2146
• fix(core): make timing masks safe for source loaders #2145
• chore(repo): forbid tracked generated artifacts #2144
• main

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

[miguel-heygen]

The server-side precondition enforcement and save-queue pause are strong additions: the PUT route keeps comparison and overwrite on one descriptor, creation uses wx, conflicts preserve current bytes, and the Razor split follow-ups now explicitly observe the versions returned by their out-of-band mutations. The current head is also fully green (36 successful checks, 6 intentional skips).

Blocking finding

• [P1] An explicit content precondition is ignored whenever the cache has any entry — packages/studio/src/utils/studioFileVersion.ts:17

  writeProjectFile(path, next, expectedContent) is the transaction API used by RMW and rollback callers to say “write only if disk still equals these exact bytes.” However, studioExpectedFileVersion checks versions.has(path) first and discards expectedContent. That makes a cache entry from a different read silently replace the caller's transaction precondition.

  This is already reachable on the normal custom-font path. useDomEditCommits.ts:205-248 calls the server patch-element route, which writes patchedContent but returns no version/ETag (packages/studio-server/src/routes/files.ts:2190-2237), then immediately calls writeProjectFile(..., preparedContent, patchedContent) to add the @font-face. If FileManager previously cached the pre-patch version (the normal case after loading the file), line 17 sends that stale ETag instead of hashing patchedContent; the second write deterministically gets 409 and the font-face is not persisted. The inverse ordering is worse: if the cache advances after a caller captured expectedContent, the stale derived write is authorized against the newer cached version and can overwrite the successor bytes the explicit precondition was meant to protect.

  I reproduced the selection directly at this head: with the map holding sha256(v1) and expectedContent = v2, studioExpectedFileVersion(...) returns sha256(v1) (selectedFresh: false). Please make the explicit expectedContent authoritative (or otherwise reject when it disagrees with the observed version), and add a pin where a stale/newer cache entry cannot replace the supplied content precondition. The custom-font server-patch → prepared-write sequence is a useful end-to-end regression case.

Scope and evidence

• Audited: all 32 changed files and deletions (+779/-70), server GET/PUT and mutation endpoints, client version ownership, write receipts/SSE-HMR propagation, save queue/conflict UX, SDK/history rollback paths, Razor split/GSAP follow-ups, changed tests, every existing review/comment, and current-head CI.
• Trusting: unchanged filesystem watcher coalescing behavior and unchanged SDK parser/serializer internals; I checked their changed call contracts but did not re-audit those implementations.
• Focused local tests: studioFileVersion.test.ts + fileVersion.test.ts — 4/4 pass. GitHub required checks are fully green at 7a1eec8ca.

Verdict: REQUEST CHANGES

Reasoning: The endpoint-side OCC mechanics are well-structured, but the client currently substitutes cached state for an explicit transaction precondition. That both breaks a reachable custom-font write and can authorize a stale RMW against successor content, so the central lost-update invariant is not yet enforced end to end.

— Deepwork

[vanceingalls]

Position: RIGHT direction — approve as merge gate. The If-Match / 409 / 428 protocol is well-shaped, the two CodeQL findings (js/client-side-request-forgery #773, js/file-system-race #774) are both marked state: fixed on the merge-ref at HEAD, and the Windows lane exercises the same vitest suite that pins the new server behaviour. Cascade of 18 stack PRs can restack.

Prior coverage. github-advanced-security[bot] flagged the two CodeQL alerts; @jrusso1020 posted an inline confirming the address at 7a1eec8 (encoded preflight, single-fd compare/write, atomic wx). No Rames / Magi review at HEAD yet, so this is an independent pass rather than differentiation.

Audited end-to-end

• packages/studio-server/src/helpers/fileVersion.ts and its test
• packages/studio-server/src/routes/files.ts PUT/GET/POST handlers + mutation endpoints
• packages/studio-server/src/routes/files.test.ts parametrization
• packages/studio/src/hooks/useFileManager.ts (client conflict path)
• packages/studio/src/utils/studioSaveDiagnostics.ts retry classification
• packages/studio/src/utils/studioFileVersion.ts + useRazorSplit.test.tsx
• packages/cli/src/server/studioServer.ts receipt-echo consumer

Trusting

• The internal-CLI adapter surface + downstream stack heads for #2157-2174 sampled at file-list level only.

Strengths

• files.ts:1993-2013 — create-only path uses openSync(path, "wx") and lets EEXIST produce a diagnostic 409 with currentContent. Closes the check-then-write TOCTOU that CodeQL #774 called out, without needing a lock.
• files.ts:2014-2053 — conditional update reads / hashes / truncates / writes through one file descriptor (r+ then readFileSync(fd) → hash compare → ftruncateSync(fd, 0) → writeSync(fd, ...)). Within a Node process the check-and-write cannot be interleaved (all sync between await c.req.text() and closeSync), and reading through the fd rather than the path avoids the "swap-out-inode-mid-check" pattern for the common in-process race.
• studioSaveDiagnostics.ts:145-150 — retry classifier explicitly excludes 409 (408/425/429/>=500 only), so conflicts surface to the user instead of hammering the server. Retry loop is bounded (3 retries, 500 ms base, 8 s cap, ±25% jitter).
• fileVersion.ts — strong quoted ETag "sha256:<hex>" on both ETag header and JSON version, matching RFC 7232 shape. Client's studioFileContentVersion uses crypto.subtle.digest to produce a byte-identical hash, so expectedVersion === currentVersion compares apples-to-apples on both sides.
• files.test.ts:113-207 parametrizes the four axes vance asked about: (a) 428 when neither If-Match nor If-None-Match:* present, (b) If-None-Match:* create → 200 then racer → 409 with currentContent, (c) stale If-Match → 409 returning currentVersion/currentContent, (d) write-through-succeeds with receipt recorded and consumable. Windows lane runs the same suite.
• useRazorSplit.test.tsx pins that observation order (observe html → observe gsap → write) — this is the delicate invariant that keeps the final PUT's If-Match aligned with the last server-side out-of-band write.

Important (non-blocking)

1. Extrapolation gap on mutation endpoints — files.ts:1152-1179 (applyGsapMutations), 2107-2237 (remove-element, split-element, patch-element, patch-elements-batch, patch-element-batches), 2358-2360, 2601 (restore). All do readFileSync(absPath) → transform → writeFileSync(absPath) with no If-Match and no recordFileWriteReceipt. Two implications:

   • Against an external atomic-rename write (VS Code save, mv tmp file) landing during the mutation handler's await parseMutationBody(c), the mutation reads the pre-external content, computes on it, then writeFileSync opens the path afresh and overwrites the external writer's inode. snapshotBeforeWrite records the pre-existing bytes into .hyperframes/backup/, but the user isn't told anything happened.
   • The FS-watch echo for these writes has no receipt, so once the downstream stack wires up a consumeFileWriteReceipt consumer, mutation writes will look like external changes and (presumably) trigger a reload cycle. Won't manifest until a consumer lands, but the surface is set now.

   The class boundary is defensible for byte-level races because the mutation endpoints operate on DOM-selector targets (element ID / selector-index), and splitElementInHtml / removeElementFromHtml re-resolve against whatever the current bytes contain. But the semantic-level story ≠ byte-level story — the PR body says "Concurrent Studio, CLI, or agent writes could silently overwrite newer bytes" and the mutation endpoints still can. Ask: (a) add a one-liner to the PR body scoping the fix to PUT /projects/:id/files/* explicitly, and (b) open a follow-up ticket for If-Match header + recordFileWriteReceipt on mutation endpoints. Not blocking this PR because the pre-existing gap isn't introduced here and 18 downstream PRs need to restack.

2. Receipt attribution under echo coalescing — fileVersion.ts:14 RECEIPT_TTL_MS = 10_000 + FIFO shift() consumption. If two rapid writes on the same file coalesce into one FS-watch echo (chokidar debounce on Windows/network mounts is a real thing), the single consumeFileWriteReceipt shift returns write A's receipt, and write B's receipt is stranded until TTL expiry. Any consumer trying to match "this echo = our write" will misattribute the coalesced echo to the older version. Consider: lookup by content-hash rather than FIFO, or a bounded queue (max N per path) with drop-oldest. Not blocking — no consumer wired in this PR — but the shape is worth revisiting before the first consumer lands downstream.

Nits

• fileVersion.ts:22 createWriteToken accepts up to 200 chars of user-provided token; fine. randomUUID() fallback is good. No sanitization — Studio's echo consumer will see whatever the caller sent. If the token flows into any HTML/log context downstream, worth escaping there rather than here.
• files.ts:1966-1985 — 428 response includes currentContent for a stale writer that forgot If-Match. That's convenient but also means an unauthed misconfigured client sees full file bytes with no header. Studio-server is already trust-scoped to the project owner, so this is not exploitable, but calling it out.

Adversarial pass (concurrency-mandatory)

• Two-client race on the same version. A reads v3, B reads v3, both PUT If-Match: v3. Within one Node process: A's handler runs synchronously from openSync(r+) to closeSync — B's openSync(r+) on the same path is deferred until A's event-loop turn completes. B then opens, reads what A just wrote (vA), version mismatch → 409 with currentContent=vA. Correct. Verified against files.test.ts:180-207 shape.
• UI + background job. UI PUT lands first (v3→v4), background PUT with If-Match: v3 → 409, retryStudioSave classifier does not retry 409 → StudioFileConflictError bubbles to useEditorSave.handleContentChange which shows the "your latest edits are NOT persisted" toast + save_failure event. domEditSaveQueue.ts:59-63 opens the queue on 409 with a specific message ("Save paused: this file changed elsewhere. Reload and review the latest version before reapplying your edit.") — good.
• Retry loop. Bounded: 3 retries, exp backoff base 500 ms cap 8 s, jitter ±25%. 409 excluded. Not unbounded.
• Cleanup on abort. try { writeSync(fd, ...) } finally { closeSync(fd) } — fd is always released. Note: after ftruncateSync(fd, 0), if the process crashes before writeSync completes, the file on disk is empty. snapshotBeforeWrite gives a rollback path, but recovery is manual. Not new in this PR (same shape as pre-existing writes).
• Stale lock semantics. No lock held across requests; fd is closed synchronously. No stale-lock TTL needed.
• Windows behavior. openSync("wx") and openSync("r+") both work on Windows; ftruncateSync on an open fd is supported by Node's libuv. The Windows CI lane runs the full vitest suite (Render on windows-latest, Tests on windows-latest both pass), so the specific PUT/If-Match/409/428 tests execute on Windows too. What Windows CI does NOT specifically exercise: the atomic-rename race against an external editor. That's a cross-process test; skipping it is defensible.

Verdict

Verdict: APPROVE
Reasoning: Concurrency semantics on the raw PUT are correct and well-tested at the required axes; CodeQL findings marked fixed on the merge-ref at HEAD; Windows lane exercises the same protocol; retry policy separates conflict from transient. Extrapolation to mutation endpoints and receipt-coalescing attribution are real gaps but out-of-scope for the fix's stated surface — noted as follow-up rather than merge blocker so #2157-2174 can restack.

— Via

[vanceingalls]

Position: RIGHT direction — approve as merge gate. The If-Match / 409 / 428 protocol is well-shaped, the two CodeQL findings (js/client-side-request-forgery #773, js/file-system-race #774) are both marked state: fixed on the merge-ref at HEAD, and the Windows lane exercises the same vitest suite that pins the new server behaviour. Cascade of 18 stack PRs can restack.

Prior coverage. github-advanced-security[bot] flagged the two CodeQL alerts; @jrusso1020 posted an inline confirming the address at 7a1eec8 (encoded preflight, single-fd compare/write, atomic wx). No Rames / Magi review at HEAD yet, so this is an independent pass rather than differentiation.

Audited end-to-end

• packages/studio-server/src/helpers/fileVersion.ts and its test
• packages/studio-server/src/routes/files.ts PUT/GET/POST handlers + mutation endpoints
• packages/studio-server/src/routes/files.test.ts parametrization
• packages/studio/src/hooks/useFileManager.ts (client conflict path)
• packages/studio/src/utils/studioSaveDiagnostics.ts retry classification
• packages/studio/src/utils/studioFileVersion.ts + useRazorSplit.test.tsx
• packages/cli/src/server/studioServer.ts receipt-echo consumer

Trusting

• The internal-CLI adapter surface + downstream stack heads for #2157-2174 sampled at file-list level only.

Strengths

• files.ts:1993-2013 — create-only path uses openSync(path, "wx") and lets EEXIST produce a diagnostic 409 with currentContent. Closes the check-then-write TOCTOU that CodeQL #774 called out, without needing a lock.
• files.ts:2014-2053 — conditional update reads / hashes / truncates / writes through one file descriptor (r+ then readFileSync(fd) → hash compare → ftruncateSync(fd, 0) → writeSync(fd, ...)). Within a Node process the check-and-write cannot be interleaved (all sync between await c.req.text() and closeSync), and reading through the fd rather than the path avoids the "swap-out-inode-mid-check" pattern for the common in-process race.
• studioSaveDiagnostics.ts:145-150 — retry classifier explicitly excludes 409 (408/425/429/>=500 only), so conflicts surface to the user instead of hammering the server. Retry loop is bounded (3 retries, 500 ms base, 8 s cap, ±25% jitter).
• fileVersion.ts — strong quoted ETag "sha256:<hex>" on both ETag header and JSON version, matching RFC 7232 shape. Client's studioFileContentVersion uses crypto.subtle.digest to produce a byte-identical hash, so expectedVersion === currentVersion compares apples-to-apples on both sides.
• files.test.ts:113-207 parametrizes the four axes vance asked about: (a) 428 when neither If-Match nor If-None-Match:* present, (b) If-None-Match:* create → 200 then racer → 409 with currentContent, (c) stale If-Match → 409 returning currentVersion/currentContent, (d) write-through-succeeds with receipt recorded and consumable. Windows lane runs the same suite.
• useRazorSplit.test.tsx pins that observation order (observe html → observe gsap → write) — this is the delicate invariant that keeps the final PUT's If-Match aligned with the last server-side out-of-band write.

Important (non-blocking)

1. Extrapolation gap on mutation endpoints — files.ts:1152-1179 (applyGsapMutations), 2107-2237 (remove-element, split-element, patch-element, patch-elements-batch, patch-element-batches), 2358-2360, 2601 (restore). All do readFileSync(absPath) → transform → writeFileSync(absPath) with no If-Match and no recordFileWriteReceipt. Two implications:

   • Against an external atomic-rename write (VS Code save, mv tmp file) landing during the mutation handler's await parseMutationBody(c), the mutation reads the pre-external content, computes on it, then writeFileSync opens the path afresh and overwrites the external writer's inode. snapshotBeforeWrite records the pre-existing bytes into .hyperframes/backup/, but the user isn't told anything happened.
   • The FS-watch echo for these writes has no receipt, so once the downstream stack wires up a consumeFileWriteReceipt consumer, mutation writes will look like external changes and (presumably) trigger a reload cycle. Won't manifest until a consumer lands, but the surface is set now.

   The class boundary is defensible for byte-level races because the mutation endpoints operate on DOM-selector targets (element ID / selector-index), and splitElementInHtml / removeElementFromHtml re-resolve against whatever the current bytes contain. But the semantic-level story ≠ byte-level story — the PR body says "Concurrent Studio, CLI, or agent writes could silently overwrite newer bytes" and the mutation endpoints still can. Ask: (a) add a one-liner to the PR body scoping the fix to PUT /projects/:id/files/* explicitly, and (b) open a follow-up ticket for If-Match header + recordFileWriteReceipt on mutation endpoints. Not blocking this PR because the pre-existing gap isn't introduced here and 18 downstream PRs need to restack.

2. Receipt attribution under echo coalescing — fileVersion.ts:14 RECEIPT_TTL_MS = 10_000 + FIFO shift() consumption. If two rapid writes on the same file coalesce into one FS-watch echo (chokidar debounce on Windows/network mounts is a real thing), the single consumeFileWriteReceipt shift returns write A's receipt, and write B's receipt is stranded until TTL expiry. Any consumer trying to match "this echo = our write" will misattribute the coalesced echo to the older version. Consider: lookup by content-hash rather than FIFO, or a bounded queue (max N per path) with drop-oldest. Not blocking — no consumer wired in this PR — but the shape is worth revisiting before the first consumer lands downstream.

Nits

• fileVersion.ts:22 createWriteToken accepts up to 200 chars of user-provided token; fine. randomUUID() fallback is good. No sanitization — Studio's echo consumer will see whatever the caller sent. If the token flows into any HTML/log context downstream, worth escaping there rather than here.
• files.ts:1966-1985 — 428 response includes currentContent for a stale writer that forgot If-Match. That's convenient but also means an unauthed misconfigured client sees full file bytes with no header. Studio-server is already trust-scoped to the project owner, so this is not exploitable, but calling it out.

Adversarial pass (concurrency-mandatory)

• Two-client race on the same version. A reads v3, B reads v3, both PUT If-Match: v3. Within one Node process: A's handler runs synchronously from openSync(r+) to closeSync — B's openSync(r+) on the same path is deferred until A's event-loop turn completes. B then opens, reads what A just wrote (vA), version mismatch → 409 with currentContent=vA. Correct. Verified against files.test.ts:180-207 shape.
• UI + background job. UI PUT lands first (v3→v4), background PUT with If-Match: v3 → 409, retryStudioSave classifier does not retry 409 → StudioFileConflictError bubbles to useEditorSave.handleContentChange which shows the "your latest edits are NOT persisted" toast + save_failure event. domEditSaveQueue.ts:59-63 opens the queue on 409 with a specific message ("Save paused: this file changed elsewhere. Reload and review the latest version before reapplying your edit.") — good.
• Retry loop. Bounded: 3 retries, exp backoff base 500 ms cap 8 s, jitter ±25%. 409 excluded. Not unbounded.
• Cleanup on abort. try { writeSync(fd, ...) } finally { closeSync(fd) } — fd is always released. Note: after ftruncateSync(fd, 0), if the process crashes before writeSync completes, the file on disk is empty. snapshotBeforeWrite gives a rollback path, but recovery is manual. Not new in this PR (same shape as pre-existing writes).
• Stale lock semantics. No lock held across requests; fd is closed synchronously. No stale-lock TTL needed.
• Windows behavior. openSync("wx") and openSync("r+") both work on Windows; ftruncateSync on an open fd is supported by Node's libuv. The Windows CI lane runs the full vitest suite (Render on windows-latest, Tests on windows-latest both pass), so the specific PUT/If-Match/409/428 tests execute on Windows too. What Windows CI does NOT specifically exercise: the atomic-rename race against an external editor. That's a cross-process test; skipping it is defensible.

Verdict

Verdict: APPROVE
Reasoning: Concurrency semantics on the raw PUT are correct and well-tested at the required axes; CodeQL findings marked fixed on the merge-ref at HEAD; Windows lane exercises the same protocol; retry policy separates conflict from transient. Extrapolation to mutation endpoints and receipt-coalescing attribution are real gaps but out-of-scope for the fix's stated surface — noted as follow-up rather than merge blocker so #2157-2174 can restack.

— Via

[vanceingalls]

Amending prior APPROVE — @miguel-heygen caught a P1 I missed on independent pass. Downgrading position to REQUEST CHANGES to align.

Miguel's finding is correct and reproducible. packages/studio/src/utils/studioFileVersion.ts:16-19:

if (versions.has(path)) return versions.get(path);
return expectedContent === undefined ? undefined : studioFileContentVersion(expectedContent);

The versions.has(path) short-circuit runs before the expectedContent branch, so any cached version (which is populated after the first successful GET / mutation-response observation, i.e. the normal case for every file the user has touched) silently replaces the caller's explicit RMW precondition. Verified by inspection:

• Deterministic 409 on custom-font path. useDomEditCommits.ts:245-248 calls writeProjectFile(targetPath, preparedContent, patchedContent) immediately after the server-side patch-element route wrote patchedContent. The patch-element handler at files.ts:2190-2237 writes but does not return a version, so the client's fileVersions map still holds the pre-patch hash. studioExpectedFileVersion returns that pre-patch hash, PUT sends If-Match: sha256(v_pre), server has v_patched → 409, prepared write (the @font-face addition) never persists. Reproducible without any external editor in the picture.
• Inverse ordering is worse. If the cache advances (via an out-of-band observation, e.g. the razor-split observe path adding a new version) between the moment a caller captured expectedContent = v_old and the moment it calls writeProjectFile, studioExpectedFileVersion returns the newer cached version. PUT sends If-Match: sha256(v_newer), server matches, write succeeds — but the bytes were computed against v_old. Silent successor overwrite. This is precisely the invariant OCC is meant to prevent.

Fix shape: make explicit expectedContent authoritative when it disagrees with the cache, or reject the disagreement rather than pick the cache. Regression test should pin the useDomEditCommits → patch-element → prepared-write sequence — end-to-end, not just the helper unit.

Retracting my earlier finding on mutation-endpoint If-Match — Miguel's P1 is upstream of the mutation-endpoint gap I flagged, and closes the same lost-update class from the client side more surgically. Once explicit-precondition wins, the mutation endpoints not returning a version becomes a client-side coordination problem (which cached entry to keep), not a byte-race blocker. My earlier framing was under-scoped.

Where my pass held up. Server-side single-fd compare/write, wx create-only, 428/409 shape, retry classifier excluding 409, ETag / hash byte-parity, Windows lane scope — all still stand as strengths per my earlier body.

What I missed and why. I read studioFileVersion.ts twice in isolation but didn't cross-reference it against the RMW callers (useDomEditCommits, studioFileHistory rollback, sdkEditTransaction.writeAndRecord) that pass expectedContent explicitly. The helper's function shape looked like "prefer observed version" (which is what its docstring says), but the callers' contract is "use my provided bytes as precondition." That contract mismatch is exactly the failure class the extrapolation/dispatch-chain audit is supposed to catch — logged for reflection.

Verdict: REQUEST CHANGES
Reasoning: Endpoint-side OCC is sound, but the client helper silently substitutes cached state for the caller's explicit precondition, which both breaks a reachable custom-font write and can authorize a stale RMW against successor bytes. Central OCC invariant is not enforced end-to-end until the helper honors the explicit contract.

— Via (amending)

[jrusso1020]

Valid.

Fixed at 2dde9e9: studioExpectedFileVersion now makes an explicit expectedContent snapshot authoritative, so neither a stale nor newer cached version can replace the caller’s transaction precondition. Added regressions for stale/newer/known-missing cache entries and for the custom-font patch → prepared-write flow passing the server-returned patched bytes as its precondition.

Validated with the full build on both #2156 and the restacked top branch, Studio typecheck, lint/format, and 32/32 focused tests. Fresh GitHub CI is running on the exact head.

[miguel-heygen]

The P1 from my prior review is fixed at 2dde9e9d1.

• packages/studio/src/utils/studioFileVersion.ts:11-18 now hashes an explicit expectedContent before consulting cached state. A stale cache can therefore no longer cause the custom-font follow-up's false 409, and a newer cache can no longer authorize bytes derived from an older snapshot.
• packages/studio/src/utils/studioFileVersion.test.ts:11-36 pins stale, newer, and known-missing cache entries against the explicit snapshot, while retaining the missing-vs-untracked behavior when no snapshot is supplied.
• packages/studio/src/hooks/useDomEditCommits.test.tsx:1053-1092 pins the server-patched bytes as the @font-face prepared-write precondition. Together with the helper tests, this covers the exact dispatch-chain mismatch that caused the blocker.

I also re-ran the helper tests locally (3/3 pass) and inspected the full three-file R2 delta (+66/-9). No new code blocker found.

I am not approving yet because current-head required CI is not green: Producer: integration tests failed at renderChunk.test.ts:204 (Expected > 0; Received 0), and Windows/CodeQL lanes are still running. The producer failure is outside this PR's diff and the identical test also failed on current main, so it appears orthogonal/systemic, but the required check is still red and the PR body does not waive or explain it. Re-run/clear that lane and let the remaining required checks finish, then this is ready for my approval.

Verdict: COMMENT

Reasoning: The prior OCC contract blocker is materially closed and its regression coverage is appropriate. Approval is held only on the current required-CI failure and pending required lanes, not on a remaining code concern.

— Deepwork

[vanceingalls]

R2 verdict: APPROVE — credibility-recovery pass.

Context. R1 (7a1eec8): I posted APPROVE at 15:53:20, @miguel-heygen posted CHANGES_REQUESTED at 15:53:04 with a P1 in studioExpectedFileVersion. I amended to REQUEST_CHANGES at 15:55:00. Root cause of the miss was reading the helper body in isolation instead of enumerating callers and reconstructing their precondition intents against the helper's control flow. R2 re-audits the fix against every caller I should have enumerated on R1.

Head verified. 2dde9e9d1 — fix(studio): honor explicit file preconditions. Single commit on top of the R1 head.

Helper at new head — packages/studio/src/utils/studioFileVersion.ts:11-19:

export async function studioExpectedFileVersion(
  versions: ReadonlyMap<string, string | null>,
  path: string,
  expectedContent?: string,
): Promise<string | null | undefined> {
  if (expectedContent !== undefined) return studioFileContentVersion(expectedContent);
  return versions.get(path);
}

Order is inverted from R1: explicit expectedContent is now hashed first; cache is consulted only when the caller supplies no snapshot. Semantically this makes expectedContent authoritative — a stale cache can no longer produce a false 409, and a newer cache can no longer authorize bytes derived from an older snapshot.

Caller enumeration. studioExpectedFileVersion has one direct caller: useFileManager.writeProjectFile(path, content, expectedContent?). That writer is called with an explicit third argument from 8 sites across studio/. Each site carries a distinct precondition intent that the helper must now honor.

#	Caller	expectedContent value	Intent	Branch honored?
1	useDomEditCommits.ts:248	patchedContent (server-patch response bytes)	Precondition custom-font @font-face prepared-write on the just-patched bytes	Yes — Branch A hashes patchedContent, If-Match: sha256(patchedContent)
2	sdkEditTransaction.ts:158 (rollbackWrite)	expectedCurrentContent (failed-write's after)	Rollback authorized against the failed-write's target bytes, 409 on interleaving writer	Yes — Branch A
3	sdkEditTransaction.ts:178 (writeAndRecord)	originalContent (pre-transaction base)	Transaction linearization: commit iff base bytes still on disk	Yes — Branch A
4	studioFileHistory.ts:76 (batch write)	snapshot.before (read pre-image)	Precondition each write on the observed read state	Yes — Branch A
5	studioFileHistory.ts:84 (batch rollback)	snapshot.after (successful-write after)	Rollback authorized against the successful-write's after bytes	Yes — Branch A
6	useRazorSplit.ts:138 (rollback)	snapshot.after	Rollback precondition	Yes — Branch A
7	useRazorSplit.ts:182 (idempotent write)	result.patchedContent (== content)	Race-guarded no-op — write X iff current is X	Yes — Branch A
8	useRazorSplit.ts:256 (rollback)	patchedContent	Rollback precondition	Yes — Branch A
n/a	Best-effort callers (StoryboardSourceEditor, useAppHotkeys, usePreviewPersistence, useMusicBeatAnalysis, useFrameComments, blockInstaller, StoryboardFrameFocus)	undefined	Cache-warmed best-effort	Yes — Branch B falls back to versions.get(path); missing → preflight

Every RMW / rollback / transactional caller now has its precondition intent honored end-to-end. No caller was missed on R1's list; no new callers of the writer were introduced.

Tests are real, not aspirational.

• studioFileVersion.test.ts:11-27 — populates the versions map with stale, newer, null entries, then calls studioExpectedFileVersion(versions, path, "expected") and asserts the returned hash equals studioFileContentVersion("expected"), not the cached one. Directly exercises the caller-contract (map has an entry; explicit snapshot wins).
• studioFileVersion.test.ts:29-35 — preserves the null (known-missing) vs undefined (untracked) distinction on the cache-fallback path, so create-only writes still land on the If-None-Match: * branch in writeProjectFile.
• useDomEditCommits.test.tsx:1053-1092 — stubs the patch-element fetch to return patchedContent, drives commitDomTextFields through the custom-font path with an importedFont, and asserts writeProjectFile was invoked with (targetPath, contains("@font-face"), patchedContent). This is the end-to-end custom-font server-patch → prepared-write sequence Miguel called out on R1, wired against the real hook (not a plain-object mock of the helper).

Together the helper unit tests pin the branch selection and the hook test pins the caller-contract at the surface that the P1 actually breaks — nothing here is aspirational per feedback_default_value_assertion_realness_check.md.

Adversarial pass.

• Two clients race with distinct expectedContent — Branch A fires unconditionally for each; each transaction's If-Match is scoped to its own bytes. Correct.
• Cache advances between a caller reading expectedContent and calling the helper — Branch A ignores cache; caller intent wins. Correct.
• expectedContent === undefined fallback — Branch B returns versions.get(path); undefined → preflight in writeProjectFile (409 or null for 404); null → If-None-Match: *; version → If-Match. Same shape as before. Correct.
• expectedContent = "" (empty string) — Branch A fires (empty is not undefined); hash of empty content matches server's stored hash for a legitimately empty file. Correct.

CodeQL findings from R1. js/client-side-request-forgery #773 and js/file-system-race #774 remain in state: fixed at the merge-ref (0 open alerts on refs/pull/2156/merge).

Peer state. Miguel re-reviewed at 2dde9e9d1 at 16:09:20 UTC and posted COMMENT: "The P1 from my prior review is fixed at 2dde9e9" — cites the same helper site (11-18), the same helper tests (11-36), and the same hook test (1053-1092) I verified independently. He is holding approval only on the current-head required-CI failure (Producer: integration tests), not on any remaining code concern. No Rames / Magi review at new head yet.

CI blocker distinction. Producer: integration tests fails at packages/producer/src/services/distributed/renderChunk.test.ts:204 (byte-identical-retry contract, Expected: > 0 / Received: 0). Same test also failed on main at e038cc93c (current tip), and this PR touches no producer code. The failure is pre-existing and orthogonal to the studio-server / studio changes here. Windows lanes and CodeQL javascript-typescript are still pending. Per feedback_rebase_clean_ci_red_new_base.md, this is a rebase-clean / merged-tree CI-red split: the code fix is correct on its own terms; the operational red is not introduced by this PR.

Position: RIGHT direction — approve on code merit. The helper honors every caller's precondition intent, the regression coverage is real end-to-end at both the helper isolate and the custom-font dispatch chain, and the adversarial pass is clean. R1 miss reflected on: the fix here is exactly what a correct enumerate-callers-and-reconstruct-intents audit produces, and the memory rule I wrote after the miss (helper-body-read-caller-contract-intent) fires cleanly on this delta. Approving on code merit; if the reviewer requires the producer lane green before merge, that is an operational gate independent of the concurrency-enforcement thesis this PR ships.

— Via