fix(studio): fix the caption overrides save protocol and a false-armed autosave

[rajanpanth]

What

Fixes #3501. useCaptionSync's save path now follows the same conditional-write protocol useFileManager.ts already uses for the main composition file, and a false-armed autosave on edit-mode activation is fixed.

Why

caption-overrides.json was PUT without an If-Match/If-None-Match header at all, so the server's 428 (Precondition Required) response was never satisfied and every save failed, surfacing as a toast even when the user hadn't edited anything.

That second half traced to a bug in the autosave subscription's own change tracking, separate from the missing headers: prevModel was only updated inside the !state.isEditMode early-return's guarded branch. The real activation order (useCaptionDetection.ts: setModel(), then setEditMode(true)) calls setModel() before entering edit mode, so that call was invisible to prevModel. The next tick's setEditMode(true) then looked like a model change against the stale prevModel and armed a save nobody asked for.

How

• Added a versionRef to useCaptionSync (undefined = unknown, null = confirmed absent, string = known ETag), mirroring useFileManager.ts. loadOverrides() captures the version from the GET response.
• putOverrides() sends If-None-Match: "*" when no version is known yet, If-Match: <version> once one is, via the existing retryStudioSave/StudioFileConflictError machinery. On a 409, adopts the server's currentVersion and retries once.
• Fixed prevModel tracking: compute modelChanged/update prevModel unconditionally at the top of every subscription tick, before the isEditMode early return, so a setModel() that happens before setEditMode(true) is no longer invisible.
• Added hasPendingSave to the caption store, set whenever a real edit arms a save, cleared once it's sent. Gated useCaptionDetection.ts's composition-switch flush behind it, since it previously called retrySave?.() unconditionally on every switch, not just when there was something to save.

Test plan

• Unit tests added/updated
• Manual testing performed
• Documentation updated (if applicable)

useCaptionSync.test.tsx (5 cases): first save sends If-None-Match: "*"; a save after loadOverrides() sends If-Match with the captured version; a 409 retries once with the adopted version; edit-mode activation with no prior edit does not arm a save (regression test for the prevModel bug); a genuine edit still arms one. useCaptionDetection.flushGate.test.tsx (3 cases) covers the gated flush directly.

Ran the full studio suite before and after: same 56 pre-existing failures both times (401→403 files affected, 3984→3992 tests passing), so this branch doesn't touch anything outside its own scope. lefthook run pre-commit (lint, format, fallow, typecheck, filesize, largefiles, tracked-artifacts, commitlint) all green on the actual commit.

[somanshreddy]

Review — two independent passes (Codex unbiased + mine), reconciled at source at head ebe6704. The two target bugs are fixed correctly and I verified both: the conditional-write protocol (undefined/null → If-None-Match: "*", known ETag → If-Match, a second 409 escapes after exactly one retry — 409 isn't in isRetryableStudioSaveError's {408,425,429,≥500} set, so retryStudioSave doesn't double-retry it), and the prevModel reorder that kills the false-armed autosave. Nice, careful work.

Codex surfaced a set of save-path races; I verified each at source and re-graded several down — recording the reasoning so it's auditable:

Should-fix (the one I'd want addressed or consciously deferred)

Overlapping saves aren't serialized — an older snapshot can overwrite a newer edit (useCaptionSync.ts save path). Each save() captures an immutable body, but there's no in-flight guard: if a PUT is still awaiting when the 800 ms debounce fires again, two saves run concurrently against the shared versionRef. editSeqRef is only consulted to clear pending, never to abort a stale attempt. On the normal ordering it self-heals (older lands first → newer 409s → adopts → retries → newer wins), but if the two PUTs reorder, or the older one's adopt-and-retry lands last, the older body wins. Needs a slow save + a concurrent edit, so it's not the common path — but it is real data loss. Suggest serializing saves (single-flight, coalesce to latest) or tagging each attempt with editSeqRef and aborting stale ones before each retry. Reasonable as a fast-follow given this PR fixes a strictly worse bug (every save failing) — your call whether it blocks.

Re-graded down (verified, not blocking)

• Non-identical 409 overwrites the other writer — this is deliberate and documented: the code comment states caption-overrides.json has exactly one writer and intentionally adopts last-write-wins because "there is no conflict UI for captions." Acceptable for a derived autosave file; only flagging so it's a conscious call — a second tab / manual edit is the one genuine two-writer case and it gets clobbered silently.
• Switch-flush doesn't cancel the armed debounce — mechanically true, but not data loss: retrySave is registered as save itself (:241), and save() builds body from getState().model synchronously (:169) before the async PUT, so the flush sends the latest edit; the leftover timer fires after reset(), sees model===null, and no-ops. Worth cancelling the timer in the flush for cleanliness (avoids a redundant late save), but no correctness impact.
• Suppress token skips the next edit if the user exits edit mode mid-load — real but pre-existing: the suppressSaveRef check sits after the isEditMode early-return on main too; this diff only moved the prevModel update above the guard, it didn't change suppress consumption. Latent fast-follow, not introduced here. (A scoped generation token instead of a shared boolean would fix it.)

Non-blocking

• versionRef isn't project-scoped — persists for StudioApp's lifetime while projectIdRef changes on hash-nav; a late response from a prior project could apply a stale version. Narrow (loadOverrides re-runs on switch), but a {projectId, version} pair + ignoring mismatched responses would close it.
• Network failures throw a plain Error, so retryStudioSave's network-retry path (StudioSaveNetworkError) never triggers for captions (its tests assert plain errors aren't retried). Throw StudioSaveNetworkError to match useFileManager.

Praise

The version state machine and the exactly-once conflict retry are correct, the identical-content 409 shortcut is a nice touch, and the prevModel/hasPendingSave fixes are well-reasoned with genuine regression tests.

---

Verdict: Comment (not blocking). The delivered fix is correct; I'm withholding a stamp only on the overlapping-save race (real data loss, narrow) — happy to stamp once that's addressed or explicitly deferred to a fast-follow.

Provenance: Codex mapped the race surface (I'd caught only the network-error nit on my own pass); I verified each at source and re-graded B1/B3/B4 down with source-specific reasons. Credit to it for the overlapping-save race, which is the real one.

[rajanpanth]

Thanks for the two-pass review, and for re-grading the rest at source rather than passing them through. Pushed 100e9fe addressing the should-fix.

Overlapping saves are now serialized. You had the failure mode exactly right: each save() captured its own body, but nothing stopped two PUTs being in flight against the shared versionRef. Saves now run one at a time, and a save requested while one is in flight replaces any already-queued attempt rather than stacking, so the next PUT always carries the newest body. Coalescing matters here as much as serializing, since a naive queue would just serialize the same lost-update in slow motion.

I mutation-tested the regression rather than trusting it: with the new test in place and only the source reverted, it fails with expected [ …(2) ] to have a length of 1 but got 2 — two concurrent PUTs, which is the race itself. All five pre-existing tests pass either way, so the test isolates the new behaviour.

Also took the network-error nit: putOverrides now throws StudioSaveNetworkError instead of a plain Error, so retryStudioSave's network-retry path actually applies to captions the way it does in useFileManager.

On the three you re-graded down, I agree with your reading on each and have left them alone:

• Non-identical 409 last-write-wins is the documented single-writer tradeoff. Worth revisiting only if captions ever get a conflict UI.
• Switch-flush not cancelling the armed debounce is a redundant late save, not data loss, for the reason you gave: save() reads getState().model synchronously, and the leftover timer no-ops after reset(). Left out of this PR to keep the diff on the data-loss path.
• Suppress-token skipping an edit is pre-existing on main and untouched by this diff. The scoped generation token is the right fix, but it belongs in its own change.

The two non-blocking items (project-scoping versionRef) are worth doing; happy to take them as a fast-follow rather than growing this PR further, unless you'd rather they land here.

CI note: the filesize pre-commit hook still aborts locally with syntax error: unexpected end of file, so this commit needed --no-verify. #3540 fixes that hook. Everything it guards was run manually: lint, format, fallow, typecheck, and the caption suite (122 passing), and the file is 362 lines against its 600 limit.

[rajanpanth]

Rebase cleared the real failure: CI is green on the rebased head. The earlier red was useStudioAgentTools.test.tsx, fixed by the WebMCP work that landed on main after this branch was cut.

The one remaining red, Windows render verification, looks unrelated to this change:

• it is offCanvasIndicatorRefresh.test.tsx > tracks the indicator to the new position when it stays off-canvas (off->off), and this branch only touches captions/ and useCaptionDetection
• that file passes locally for me on Windows, 2/2
• it is timing-sensitive (313 ms and 556 ms locally against 147 ms in CI), and the other open PRs are green on the same job

I do not have re-run rights here. Happy to push a no-op commit to retrigger if you would rather see it green before stamping.