From 1d66603ef2418ca8fb544afc2653530a73055543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Tue, 4 Aug 2026 03:10:42 +0000 Subject: [PATCH 1/3] fix(studio): drain pending edits before reload --- .../studio/src/hooks/useEditorSave.test.tsx | 99 ++++++++++++ packages/studio/src/hooks/useEditorSave.ts | 149 +++++++++++++----- .../studio/src/utils/domEditSaveQueue.test.ts | 21 ++- packages/studio/src/utils/domEditSaveQueue.ts | 23 ++- .../src/utils/studioFileVersion.test.ts | 19 ++- .../studio/src/utils/studioFileVersion.ts | 25 +++ .../src/utils/studioPendingEdits.test.ts | 56 ++++++- .../studio/src/utils/studioPendingEdits.ts | 30 +++- 8 files changed, 375 insertions(+), 47 deletions(-) create mode 100644 packages/studio/src/hooks/useEditorSave.test.tsx diff --git a/packages/studio/src/hooks/useEditorSave.test.tsx b/packages/studio/src/hooks/useEditorSave.test.tsx new file mode 100644 index 00000000000..572872a582d --- /dev/null +++ b/packages/studio/src/hooks/useEditorSave.test.tsx @@ -0,0 +1,99 @@ +// @vitest-environment happy-dom +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { useEditorSave, type EditorSaveHandle } from "./useEditorSave"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +describe("useEditorSave pending work", () => { + beforeEach(() => { + vi.stubGlobal( + "requestAnimationFrame", + vi.fn(() => 41), + ); + vi.stubGlobal("cancelAnimationFrame", vi.fn()); + }); + + afterEach(() => vi.unstubAllGlobals()); + + it("exposes and flushes the latest rAF-buffered source candidate", async () => { + const writeProjectFile = vi.fn(async () => undefined); + const captured: { handle: EditorSaveHandle | null } = { handle: null }; + + function Probe() { + captured.handle = useEditorSave({ + editingPathRef: { current: "index.html" }, + projectIdRef: { current: "project-a" }, + readProjectFile: vi.fn(async () => "before"), + writeProjectFile, + recordEdit: vi.fn(async () => undefined), + domEditSaveTimestampRef: { current: 0 }, + setRefreshKey: vi.fn(), + showToast: vi.fn(), + }); + return null; + } + + const root = createRoot(document.createElement("div")); + await act(async () => root.render()); + act(() => captured.handle?.handleContentChange("studio candidate")); + + expect(captured.handle?.getPendingCandidate()).toEqual({ + projectId: "project-a", + path: "index.html", + content: "studio candidate", + }); + await expect(captured.handle?.flushPendingSave()).resolves.toEqual({ status: "clean" }); + expect(writeProjectFile).toHaveBeenCalledWith("index.html", "studio candidate", "before"); + + await act(async () => root.unmount()); + }); + + it("joins an in-flight source save instead of writing the frozen candidate twice", async () => { + let frame: FrameRequestCallback | null = null; + vi.stubGlobal( + "requestAnimationFrame", + vi.fn((callback: FrameRequestCallback) => { + frame = callback; + return 42; + }), + ); + let finishWrite!: () => void; + const writeProjectFile = vi + .fn() + .mockImplementationOnce( + () => + new Promise((resolve) => { + finishWrite = resolve; + }), + ) + .mockResolvedValue(undefined); + const captured: { handle: EditorSaveHandle | null } = { handle: null }; + function Probe() { + captured.handle = useEditorSave({ + editingPathRef: { current: "index.html" }, + projectIdRef: { current: "project-a" }, + readProjectFile: vi.fn(async () => "before"), + writeProjectFile, + recordEdit: vi.fn(async () => undefined), + domEditSaveTimestampRef: { current: 0 }, + setRefreshKey: vi.fn(), + showToast: vi.fn(), + }); + return null; + } + const root = createRoot(document.createElement("div")); + await act(async () => root.render()); + act(() => captured.handle?.handleContentChange("candidate")); + act(() => frame?.(0)); + await vi.waitFor(() => expect(writeProjectFile).toHaveBeenCalledOnce()); + + const drained = captured.handle?.flushPendingSave(); + expect(writeProjectFile).toHaveBeenCalledOnce(); + finishWrite(); + await expect(drained).resolves.toEqual({ status: "clean" }); + expect(writeProjectFile).toHaveBeenCalledOnce(); + await act(async () => root.unmount()); + }); +}); diff --git a/packages/studio/src/hooks/useEditorSave.ts b/packages/studio/src/hooks/useEditorSave.ts index f9e04a97748..b464651198e 100644 --- a/packages/studio/src/hooks/useEditorSave.ts +++ b/packages/studio/src/hooks/useEditorSave.ts @@ -2,6 +2,7 @@ import { useCallback, useRef } from "react"; import { saveProjectFilesWithHistory } from "../utils/studioFileHistory"; import type { EditHistoryKind } from "../utils/editHistory"; import { trackStudioEvent } from "../utils/studioTelemetry"; +import { StudioFileConflictError } from "../utils/studioSaveDiagnostics"; interface RecordEditInput { label: string; @@ -21,6 +22,25 @@ interface UseEditorSaveOptions { showToast: (message: string, tone?: "error" | "info") => void; } +export interface EditorSaveCandidate { + projectId: string; + path: string; + content: string; +} + +export type EditorSaveDrainResult = + | { status: "clean" } + | { status: "conflict"; error: StudioFileConflictError } + | { status: "failed"; error: unknown }; + +export interface EditorSaveHandle { + saveRafRef: React.MutableRefObject; + handleContentChange: (content: string) => void; + getPendingCandidate: () => EditorSaveCandidate | null; + flushPendingSave: () => Promise; + discardPendingSave: () => void; +} + export function useEditorSave({ editingPathRef, projectIdRef, @@ -30,12 +50,70 @@ export function useEditorSave({ domEditSaveTimestampRef, setRefreshKey, showToast, -}: UseEditorSaveOptions) { +}: UseEditorSaveOptions): EditorSaveHandle { const saveRafRef = useRef(null); const refreshRafRef = useRef(null); // One error toast per burst of failures — every keystroke retries the save, // and error toasts persist until dismissed, so don't stack duplicates. const lastFailureToastAtRef = useRef(0); + const pendingCandidateRef = useRef(null); + const inFlightRef = useRef | null>(null); + const inFlightCandidateRef = useRef(null); + + const reportFailure = useCallback( + (path: string, error: unknown) => { + trackStudioEvent("save_failure", { + source: "code_editor", + error_message: error instanceof Error ? error.message : "unknown", + }); + const now = Date.now(); + if (now - lastFailureToastAtRef.current > 5000) { + lastFailureToastAtRef.current = now; + showToast( + `Couldn't save ${path} — your latest edits are NOT persisted. Check the preview server; editing again retries the save.`, + "error", + ); + } + }, + [showToast], + ); + + const persistCandidate = useCallback( + (candidate: EditorSaveCandidate): Promise => { + const task = saveProjectFilesWithHistory({ + projectId: candidate.projectId, + label: "Edit source", + kind: "source", + coalesceKey: `source:${candidate.path}`, + files: { [candidate.path]: candidate.content }, + readFile: readProjectFile, + writeFile: writeProjectFile, + recordEdit, + }) + .then(() => { + if (pendingCandidateRef.current === candidate) pendingCandidateRef.current = null; + if (refreshRafRef.current != null) cancelAnimationFrame(refreshRafRef.current); + refreshRafRef.current = requestAnimationFrame(() => setRefreshKey((k) => k + 1)); + return { status: "clean" }; + }) + .catch((error: unknown) => { + reportFailure(candidate.path, error); + return error instanceof StudioFileConflictError + ? { status: "conflict", error } + : { status: "failed", error }; + }) + .finally(() => { + if (inFlightRef.current === task) { + inFlightRef.current = null; + inFlightCandidateRef.current = null; + } + }); + inFlightRef.current = task; + inFlightCandidateRef.current = candidate; + return task; + }, + [readProjectFile, recordEdit, reportFailure, setRefreshKey, writeProjectFile], + ); const handleContentChange = useCallback( (content: string) => { @@ -44,53 +122,46 @@ export function useEditorSave({ const path = editingPathRef.current; if (!path) return; + const candidate = { projectId: pid, path, content }; + pendingCandidateRef.current = candidate; + if (saveRafRef.current != null) cancelAnimationFrame(saveRafRef.current); saveRafRef.current = requestAnimationFrame(() => { + saveRafRef.current = null; domEditSaveTimestampRef.current = Date.now(); - saveProjectFilesWithHistory({ - projectId: pid, - label: "Edit source", - kind: "source", - coalesceKey: `source:${path}`, - files: { [path]: content }, - readFile: readProjectFile, - writeFile: writeProjectFile, - recordEdit, - }) - .then(() => { - if (refreshRafRef.current != null) cancelAnimationFrame(refreshRafRef.current); - refreshRafRef.current = requestAnimationFrame(() => setRefreshKey((k) => k + 1)); - }) - .catch((error) => { - trackStudioEvent("save_failure", { - source: "code_editor", - error_message: error instanceof Error ? error.message : "unknown", - }); - const now = Date.now(); - if (now - lastFailureToastAtRef.current > 5000) { - lastFailureToastAtRef.current = now; - showToast( - `Couldn't save ${path} — your latest edits are NOT persisted. Check the preview server; editing again retries the save.`, - "error", - ); - } - }); + void persistCandidate(candidate); }); }, - [ - domEditSaveTimestampRef, - editingPathRef, - projectIdRef, - readProjectFile, - recordEdit, - setRefreshKey, - showToast, - writeProjectFile, - ], + [domEditSaveTimestampRef, editingPathRef, projectIdRef, persistCandidate], ); + const flushPendingSave = useCallback(async (): Promise => { + if (saveRafRef.current != null) { + cancelAnimationFrame(saveRafRef.current); + saveRafRef.current = null; + } + const candidate = pendingCandidateRef.current; + if (candidate && candidate === inFlightCandidateRef.current && inFlightRef.current) { + return inFlightRef.current; + } + if (candidate) { + domEditSaveTimestampRef.current = Date.now(); + return persistCandidate(candidate); + } + return (await inFlightRef.current) ?? { status: "clean" }; + }, [domEditSaveTimestampRef, persistCandidate]); + + const discardPendingSave = useCallback(() => { + if (saveRafRef.current != null) cancelAnimationFrame(saveRafRef.current); + saveRafRef.current = null; + pendingCandidateRef.current = null; + }, []); + return { saveRafRef, handleContentChange, + getPendingCandidate: () => pendingCandidateRef.current, + flushPendingSave, + discardPendingSave, }; } diff --git a/packages/studio/src/utils/domEditSaveQueue.test.ts b/packages/studio/src/utils/domEditSaveQueue.test.ts index 33b82444e8d..5e82353fa2e 100644 --- a/packages/studio/src/utils/domEditSaveQueue.test.ts +++ b/packages/studio/src/utils/domEditSaveQueue.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { createDomEditSaveQueue } from "./domEditSaveQueue"; -import { StudioSaveHttpError } from "./studioSaveDiagnostics"; +import { StudioFileConflictError, StudioSaveHttpError } from "./studioSaveDiagnostics"; describe("dom edit save queue", () => { afterEach(() => { @@ -133,4 +133,23 @@ describe("dom edit save queue", () => { await expect(queue.enqueue(async () => {})).rejects.toThrow("Auto-save is paused"); queue.destroy(); }); + + it("returns the original conflict from a drain instead of erasing it", async () => { + const conflict = new StudioFileConflictError({ + filePath: "index.html", + currentVersion: "external-v2", + currentContent: "external", + attemptedContent: "studio", + }); + const queue = createDomEditSaveQueue(); + + await expect( + queue.enqueue(async () => { + throw conflict; + }), + ).rejects.toBe(conflict); + + await expect(queue.waitForIdle()).resolves.toEqual({ status: "conflict", error: conflict }); + queue.destroy(); + }); }); diff --git a/packages/studio/src/utils/domEditSaveQueue.ts b/packages/studio/src/utils/domEditSaveQueue.ts index fc722868bdd..f5f9ffa43b4 100644 --- a/packages/studio/src/utils/domEditSaveQueue.ts +++ b/packages/studio/src/utils/domEditSaveQueue.ts @@ -1,4 +1,8 @@ -import { getStudioSaveErrorMessage, getStudioSaveStatusCode } from "./studioSaveDiagnostics"; +import { + getStudioSaveErrorMessage, + getStudioSaveStatusCode, + StudioFileConflictError, +} from "./studioSaveDiagnostics"; interface DomEditSaveQueueOpenEvent { consecutiveFailures: number; @@ -14,11 +18,16 @@ interface DomEditSaveQueueOptions { export interface DomEditSaveQueue { enqueue: (save: () => Promise) => Promise; - waitForIdle: () => Promise; + waitForIdle: () => Promise; reset: () => void; destroy: () => void; } +export type DomEditSaveDrainResult = + | { status: "clean" } + | { status: "conflict"; error: StudioFileConflictError } + | { status: "failed"; error: unknown }; + const DEFAULT_FAILURE_THRESHOLD = 5; export class DomEditSaveQueueOpenError extends Error { @@ -34,11 +43,13 @@ export function createDomEditSaveQueue(options: DomEditSaveQueueOptions = {}): D let tail = Promise.resolve(); let consecutiveFailures = 0; let breakerOpen = false; + let drainError: unknown = null; const reset = (notify = true) => { const wasOpen = breakerOpen; consecutiveFailures = 0; breakerOpen = false; + drainError = null; if (notify && wasOpen) options.onReset?.(); }; @@ -58,6 +69,7 @@ export function createDomEditSaveQueue(options: DomEditSaveQueueOptions = {}): D if (!breakerOpen) consecutiveFailures = 0; return result; } catch (error) { + drainError = error; consecutiveFailures += 1; if (getStudioSaveStatusCode(error) === 409 || consecutiveFailures >= failureThreshold) open(error); @@ -76,8 +88,13 @@ export function createDomEditSaveQueue(options: DomEditSaveQueueOptions = {}): D return queued; }, - async waitForIdle() { + async waitForIdle(): Promise { await tail.catch(() => undefined); + if (drainError instanceof StudioFileConflictError) { + return { status: "conflict", error: drainError }; + } + if (drainError != null) return { status: "failed", error: drainError }; + return { status: "clean" }; }, reset, diff --git a/packages/studio/src/utils/studioFileVersion.test.ts b/packages/studio/src/utils/studioFileVersion.test.ts index ee9980a9a56..2047248b981 100644 --- a/packages/studio/src/utils/studioFileVersion.test.ts +++ b/packages/studio/src/utils/studioFileVersion.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it } from "vitest"; -import { studioExpectedFileVersion, studioFileContentVersion } from "./studioFileVersion"; +import { + consumeStudioWriteToken, + markStudioWriteToken, + resetStudioWriteTokens, + studioExpectedFileVersion, + studioFileContentVersion, +} from "./studioFileVersion"; describe("studioFileContentVersion", () => { it("matches the strong SHA-256 ETag format used by studio-server", async () => { @@ -34,3 +40,14 @@ describe("studioFileContentVersion", () => { expect(await studioExpectedFileVersion(versions, "untracked.html")).toBeUndefined(); }); }); + +describe("studio write-token echo identity", () => { + it("suppresses exactly one matching API write receipt without hiding path-only external writes", () => { + resetStudioWriteTokens(); + markStudioWriteToken("studio-write-1"); + + expect(consumeStudioWriteToken("studio-write-1")).toBe(true); + expect(consumeStudioWriteToken("studio-write-1")).toBe(false); + expect(consumeStudioWriteToken(null)).toBe(false); + }); +}); diff --git a/packages/studio/src/utils/studioFileVersion.ts b/packages/studio/src/utils/studioFileVersion.ts index 99da9df1231..7c6c5683ab3 100644 --- a/packages/studio/src/utils/studioFileVersion.ts +++ b/packages/studio/src/utils/studioFileVersion.ts @@ -1,3 +1,28 @@ +const WRITE_TOKEN_TTL_MS = 10_000; +const studioWriteTokens = new Map(); + +function pruneStudioWriteTokens(now: number): void { + for (const [token, createdAt] of studioWriteTokens) { + if (now - createdAt >= WRITE_TOKEN_TTL_MS) studioWriteTokens.delete(token); + } +} + +export function markStudioWriteToken(token: string, now: number = Date.now()): void { + pruneStudioWriteTokens(now); + studioWriteTokens.set(token, now); +} + +export function consumeStudioWriteToken(token: string | null, now: number = Date.now()): boolean { + pruneStudioWriteTokens(now); + if (!token || !studioWriteTokens.has(token)) return false; + studioWriteTokens.delete(token); + return true; +} + +export function resetStudioWriteTokens(): void { + studioWriteTokens.clear(); +} + /** Browser-safe SHA-256 version matching studio-server's strong ETag format. */ export async function studioFileContentVersion(content: string): Promise { const bytes = new TextEncoder().encode(content); diff --git a/packages/studio/src/utils/studioPendingEdits.test.ts b/packages/studio/src/utils/studioPendingEdits.test.ts index e9230a1a210..8421e7cfa5d 100644 --- a/packages/studio/src/utils/studioPendingEdits.test.ts +++ b/packages/studio/src/utils/studioPendingEdits.test.ts @@ -5,6 +5,7 @@ import { flushStudioPendingEdits, trackStudioPendingEdit, } from "./studioPendingEdits"; +import { StudioFileConflictError } from "./studioSaveDiagnostics"; describe("studio pending edit flush", () => { it("waits for mounted panels to persist pending local edits", async () => { @@ -12,13 +13,66 @@ describe("studio pending edit flush", () => { const remove = addStudioPendingEditFlushListener(persist); try { - await flushStudioPendingEdits(); + await expect(flushStudioPendingEdits()).resolves.toEqual({ status: "clean" }); expect(persist).toHaveBeenCalledTimes(1); } finally { remove(); } }); + it("commits the focused debounced field before draining pending work", async () => { + const input = document.createElement("textarea"); + document.body.append(input); + const persist = vi.fn(async () => undefined); + input.addEventListener("blur", () => { + trackStudioPendingEdit(persist()); + }); + input.focus(); + + await expect(flushStudioPendingEdits()).resolves.toEqual({ status: "clean" }); + + expect(document.activeElement).not.toBe(input); + expect(persist).toHaveBeenCalledOnce(); + input.remove(); + }); + + it("preserves a pending edit failure instead of reporting a clean drain", async () => { + const failure = new Error("field save failed"); + const remove = addStudioPendingEditFlushListener(async () => { + throw failure; + }); + + try { + await expect(flushStudioPendingEdits()).resolves.toEqual({ + status: "failed", + error: failure, + }); + } finally { + remove(); + } + }); + + it("keeps the full typed conflict payload for the external-change decision", async () => { + const conflict = new StudioFileConflictError({ + filePath: "index.html", + currentVersion: "v2", + currentContent: "external", + attemptedContent: "studio", + }); + const remove = addStudioPendingEditFlushListener(async () => { + throw conflict; + }); + + try { + await expect(flushStudioPendingEdits()).resolves.toEqual({ + status: "conflict", + error: conflict, + }); + } finally { + remove(); + } + }); + it("waits for edits already started by unmounted panels", async () => { const steps: string[] = []; let resolvePersist!: () => void; diff --git a/packages/studio/src/utils/studioPendingEdits.ts b/packages/studio/src/utils/studioPendingEdits.ts index 911907f96ac..f8ef9881583 100644 --- a/packages/studio/src/utils/studioPendingEdits.ts +++ b/packages/studio/src/utils/studioPendingEdits.ts @@ -1,9 +1,16 @@ +import { StudioFileConflictError } from "./studioSaveDiagnostics"; + const STUDIO_FLUSH_PENDING_EDITS_EVENT = "hf-studio-flush-pending-edits"; interface StudioFlushPendingEditsDetail { promises: Array>; } +export type StudioPendingEditsDrainResult = + | { status: "clean" } + | { status: "conflict"; error: StudioFileConflictError } + | { status: "failed"; error: unknown }; + const pendingEditPromises = new Set>(); export function trackStudioPendingEdit( @@ -19,7 +26,17 @@ export function trackStudioPendingEdit( return promise; } -export async function flushStudioPendingEdits(): Promise { +export async function flushStudioPendingEdits(): Promise { + const active = document.activeElement; + if ( + active instanceof HTMLElement && + active.matches('input, textarea, select, [contenteditable="true"], [role="textbox"]') + ) { + active.blur(); + // React blur handlers enqueue their save synchronously, while state-driven + // registrations can land in the next microtask. + await Promise.resolve(); + } const detail: StudioFlushPendingEditsDetail = { promises: [] }; window.dispatchEvent( new CustomEvent(STUDIO_FLUSH_PENDING_EDITS_EVENT, { detail }), @@ -27,8 +44,17 @@ export async function flushStudioPendingEdits(): Promise { while (detail.promises.length > 0 || pendingEditPromises.size > 0) { const promises = [...detail.promises, ...pendingEditPromises]; detail.promises = []; - await Promise.allSettled(promises); + const results = await Promise.allSettled(promises); + const rejected = results.find( + (result): result is PromiseRejectedResult => result.status === "rejected", + ); + if (rejected) { + return rejected.reason instanceof StudioFileConflictError + ? { status: "conflict", error: rejected.reason } + : { status: "failed", error: rejected.reason }; + } } + return { status: "clean" }; } export function addStudioPendingEditFlushListener( From ac89e2d1a2964031a4e065bb2896baac4070f7be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Tue, 4 Aug 2026 20:34:14 +0000 Subject: [PATCH 2/3] fix(studio): address drain review feedback (#2989) - prioritize conflicts and clear recovered DOM queue errors - cover delayed blur effects and missing drain branches - document stacked consumers and extend write-token retention --- .../studio/src/hooks/useEditorSave.test.tsx | 69 +++++++++++++++++++ packages/studio/src/hooks/useEditorSave.ts | 13 ++-- .../studio/src/utils/domEditSaveQueue.test.ts | 17 +++++ packages/studio/src/utils/domEditSaveQueue.ts | 11 +-- .../src/utils/studioFileVersion.test.ts | 10 +++ .../studio/src/utils/studioFileVersion.ts | 7 +- .../src/utils/studioPendingEdits.test.ts | 53 ++++++++++++++ .../studio/src/utils/studioPendingEdits.ts | 36 ++++++---- .../studio/src/utils/studioSaveDiagnostics.ts | 5 ++ 9 files changed, 195 insertions(+), 26 deletions(-) diff --git a/packages/studio/src/hooks/useEditorSave.test.tsx b/packages/studio/src/hooks/useEditorSave.test.tsx index 572872a582d..d646516bd10 100644 --- a/packages/studio/src/hooks/useEditorSave.test.tsx +++ b/packages/studio/src/hooks/useEditorSave.test.tsx @@ -3,6 +3,7 @@ import { act } from "react"; import { createRoot } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { useEditorSave, type EditorSaveHandle } from "./useEditorSave"; +import { StudioFileConflictError } from "../utils/studioSaveDiagnostics"; (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; @@ -96,4 +97,72 @@ describe("useEditorSave pending work", () => { expect(writeProjectFile).toHaveBeenCalledOnce(); await act(async () => root.unmount()); }); + + it("preserves conflict details when flushing a buffered source candidate", async () => { + const conflict = new StudioFileConflictError({ + filePath: "index.html", + currentVersion: "external-v2", + currentContent: "external", + attemptedContent: "studio candidate", + }); + const captured: { handle: EditorSaveHandle | null } = { handle: null }; + + function Probe() { + captured.handle = useEditorSave({ + editingPathRef: { current: "index.html" }, + projectIdRef: { current: "project-a" }, + readProjectFile: vi.fn(async () => "before"), + writeProjectFile: vi.fn(async () => { + throw conflict; + }), + recordEdit: vi.fn(async () => undefined), + domEditSaveTimestampRef: { current: 0 }, + setRefreshKey: vi.fn(), + showToast: vi.fn(), + }); + return null; + } + + const root = createRoot(document.createElement("div")); + await act(async () => root.render()); + act(() => captured.handle?.handleContentChange("studio candidate")); + + await expect(captured.handle?.flushPendingSave()).resolves.toEqual({ + status: "conflict", + error: conflict, + }); + + await act(async () => root.unmount()); + }); + + it("discards an rAF-buffered candidate without persisting it", async () => { + const writeProjectFile = vi.fn(async () => undefined); + const captured: { handle: EditorSaveHandle | null } = { handle: null }; + + function Probe() { + captured.handle = useEditorSave({ + editingPathRef: { current: "index.html" }, + projectIdRef: { current: "project-a" }, + readProjectFile: vi.fn(async () => "before"), + writeProjectFile, + recordEdit: vi.fn(async () => undefined), + domEditSaveTimestampRef: { current: 0 }, + setRefreshKey: vi.fn(), + showToast: vi.fn(), + }); + return null; + } + + const root = createRoot(document.createElement("div")); + await act(async () => root.render()); + act(() => captured.handle?.handleContentChange("discard me")); + act(() => captured.handle?.discardPendingSave()); + + expect(captured.handle?.getPendingCandidate()).toBeNull(); + await expect(captured.handle?.flushPendingSave()).resolves.toEqual({ status: "clean" }); + expect(writeProjectFile).not.toHaveBeenCalled(); + expect(cancelAnimationFrame).toHaveBeenCalledWith(41); + + await act(async () => root.unmount()); + }); }); diff --git a/packages/studio/src/hooks/useEditorSave.ts b/packages/studio/src/hooks/useEditorSave.ts index b464651198e..f00fd924655 100644 --- a/packages/studio/src/hooks/useEditorSave.ts +++ b/packages/studio/src/hooks/useEditorSave.ts @@ -2,7 +2,10 @@ import { useCallback, useRef } from "react"; import { saveProjectFilesWithHistory } from "../utils/studioFileHistory"; import type { EditHistoryKind } from "../utils/editHistory"; import { trackStudioEvent } from "../utils/studioTelemetry"; -import { StudioFileConflictError } from "../utils/studioSaveDiagnostics"; +import { + StudioFileConflictError, + type StudioSaveDrainResult, +} from "../utils/studioSaveDiagnostics"; interface RecordEditInput { label: string; @@ -28,16 +31,16 @@ export interface EditorSaveCandidate { content: string; } -export type EditorSaveDrainResult = - | { status: "clean" } - | { status: "conflict"; error: StudioFileConflictError } - | { status: "failed"; error: unknown }; +export type EditorSaveDrainResult = StudioSaveDrainResult; export interface EditorSaveHandle { saveRafRef: React.MutableRefObject; handleContentChange: (content: string) => void; + /** Read by the external-reload reconciliation introduced in stack PR #2993. */ getPendingCandidate: () => EditorSaveCandidate | null; + /** Wired into the external-reload drain by stack PR #2993. */ flushPendingSave: () => Promise; + /** Used by PR #2993 when the external version wins. */ discardPendingSave: () => void; } diff --git a/packages/studio/src/utils/domEditSaveQueue.test.ts b/packages/studio/src/utils/domEditSaveQueue.test.ts index 5e82353fa2e..f1166f7ee0e 100644 --- a/packages/studio/src/utils/domEditSaveQueue.test.ts +++ b/packages/studio/src/utils/domEditSaveQueue.test.ts @@ -115,6 +115,23 @@ describe("dom edit save queue", () => { queue.destroy(); }); + it("clears a stale drain failure after a successful save", async () => { + const failure = new Error("temporary failure"); + const queue = createDomEditSaveQueue(); + + await expect( + queue.enqueue(async () => { + throw failure; + }), + ).rejects.toBe(failure); + await expect(queue.waitForIdle()).resolves.toEqual({ status: "failed", error: failure }); + + await queue.enqueue(async () => undefined); + + await expect(queue.waitForIdle()).resolves.toEqual({ status: "clean" }); + queue.destroy(); + }); + it("pauses immediately on a file conflict instead of retrying stale work", async () => { const onOpen = vi.fn(); const queue = createDomEditSaveQueue({ failureThreshold: 5, onOpen }); diff --git a/packages/studio/src/utils/domEditSaveQueue.ts b/packages/studio/src/utils/domEditSaveQueue.ts index f5f9ffa43b4..280c8dadd02 100644 --- a/packages/studio/src/utils/domEditSaveQueue.ts +++ b/packages/studio/src/utils/domEditSaveQueue.ts @@ -2,6 +2,7 @@ import { getStudioSaveErrorMessage, getStudioSaveStatusCode, StudioFileConflictError, + type StudioSaveDrainResult, } from "./studioSaveDiagnostics"; interface DomEditSaveQueueOpenEvent { @@ -23,10 +24,7 @@ export interface DomEditSaveQueue { destroy: () => void; } -export type DomEditSaveDrainResult = - | { status: "clean" } - | { status: "conflict"; error: StudioFileConflictError } - | { status: "failed"; error: unknown }; +export type DomEditSaveDrainResult = StudioSaveDrainResult; const DEFAULT_FAILURE_THRESHOLD = 5; @@ -66,7 +64,10 @@ export function createDomEditSaveQueue(options: DomEditSaveQueueOptions = {}): D const run = async (save: () => Promise): Promise => { try { const result = await save(); - if (!breakerOpen) consecutiveFailures = 0; + if (!breakerOpen) { + consecutiveFailures = 0; + drainError = null; + } return result; } catch (error) { drainError = error; diff --git a/packages/studio/src/utils/studioFileVersion.test.ts b/packages/studio/src/utils/studioFileVersion.test.ts index 2047248b981..f17b3950139 100644 --- a/packages/studio/src/utils/studioFileVersion.test.ts +++ b/packages/studio/src/utils/studioFileVersion.test.ts @@ -50,4 +50,14 @@ describe("studio write-token echo identity", () => { expect(consumeStudioWriteToken("studio-write-1")).toBe(false); expect(consumeStudioWriteToken(null)).toBe(false); }); + + it("keeps a token through a slow write and expires abandoned identity state", () => { + resetStudioWriteTokens(); + markStudioWriteToken("slow-studio-write", 1_000); + + expect(consumeStudioWriteToken("slow-studio-write", 61_000)).toBe(true); + + markStudioWriteToken("abandoned-studio-write", 1_000); + expect(consumeStudioWriteToken("abandoned-studio-write", 301_000)).toBe(false); + }); }); diff --git a/packages/studio/src/utils/studioFileVersion.ts b/packages/studio/src/utils/studioFileVersion.ts index 7c6c5683ab3..2737c069b0f 100644 --- a/packages/studio/src/utils/studioFileVersion.ts +++ b/packages/studio/src/utils/studioFileVersion.ts @@ -1,4 +1,6 @@ -const WRITE_TOKEN_TTL_MS = 10_000; +// A token is marked before the request starts, so its lifetime must cover slow writes, +// retries, and the subsequent file-change echo without retaining abandoned tokens forever. +const WRITE_TOKEN_TTL_MS = 5 * 60_000; const studioWriteTokens = new Map(); function pruneStudioWriteTokens(now: number): void { @@ -7,11 +9,13 @@ function pruneStudioWriteTokens(now: number): void { } } +/** Marked by the Studio file writer introduced in external-change stack PR #2990. */ export function markStudioWriteToken(token: string, now: number = Date.now()): void { pruneStudioWriteTokens(now); studioWriteTokens.set(token, now); } +/** Consumed by the external-change coordinator introduced in stack PR #2991. */ export function consumeStudioWriteToken(token: string | null, now: number = Date.now()): boolean { pruneStudioWriteTokens(now); if (!token || !studioWriteTokens.has(token)) return false; @@ -19,6 +23,7 @@ export function consumeStudioWriteToken(token: string | null, now: number = Date return true; } +/** Resets module-level echo identity between PR #2991 coordinator tests. */ export function resetStudioWriteTokens(): void { studioWriteTokens.clear(); } diff --git a/packages/studio/src/utils/studioPendingEdits.test.ts b/packages/studio/src/utils/studioPendingEdits.test.ts index 8421e7cfa5d..d9837a32515 100644 --- a/packages/studio/src/utils/studioPendingEdits.test.ts +++ b/packages/studio/src/utils/studioPendingEdits.test.ts @@ -36,6 +36,33 @@ describe("studio pending edit flush", () => { input.remove(); }); + it("waits for a post-blur effect to register its pending edit listener", async () => { + const input = document.createElement("textarea"); + document.body.append(input); + const persist = vi.fn(async () => undefined); + let removeListener: (() => void) | undefined; + let registrationDone: Promise | undefined; + input.addEventListener("blur", () => { + registrationDone = new Promise((resolve) => { + setTimeout(() => { + removeListener = addStudioPendingEditFlushListener(persist); + resolve(); + }, 0); + }); + }); + input.focus(); + + try { + await expect(flushStudioPendingEdits()).resolves.toEqual({ status: "clean" }); + + expect(persist).toHaveBeenCalledOnce(); + } finally { + await registrationDone; + removeListener?.(); + input.remove(); + } + }); + it("preserves a pending edit failure instead of reporting a clean drain", async () => { const failure = new Error("field save failed"); const remove = addStudioPendingEditFlushListener(async () => { @@ -73,6 +100,32 @@ describe("studio pending edit flush", () => { } }); + it("prioritizes a conflict when pending edits fail with mixed errors", async () => { + const failure = new Error("field save failed"); + const conflict = new StudioFileConflictError({ + filePath: "index.html", + currentVersion: "v2", + currentContent: "external", + attemptedContent: "studio", + }); + const removeFailure = addStudioPendingEditFlushListener(async () => { + throw failure; + }); + const removeConflict = addStudioPendingEditFlushListener(async () => { + throw conflict; + }); + + try { + await expect(flushStudioPendingEdits()).resolves.toEqual({ + status: "conflict", + error: conflict, + }); + } finally { + removeFailure(); + removeConflict(); + } + }); + it("waits for edits already started by unmounted panels", async () => { const steps: string[] = []; let resolvePersist!: () => void; diff --git a/packages/studio/src/utils/studioPendingEdits.ts b/packages/studio/src/utils/studioPendingEdits.ts index f8ef9881583..7eddcebe817 100644 --- a/packages/studio/src/utils/studioPendingEdits.ts +++ b/packages/studio/src/utils/studioPendingEdits.ts @@ -1,4 +1,4 @@ -import { StudioFileConflictError } from "./studioSaveDiagnostics"; +import { StudioFileConflictError, type StudioSaveDrainResult } from "./studioSaveDiagnostics"; const STUDIO_FLUSH_PENDING_EDITS_EVENT = "hf-studio-flush-pending-edits"; @@ -6,13 +6,14 @@ interface StudioFlushPendingEditsDetail { promises: Array>; } -export type StudioPendingEditsDrainResult = - | { status: "clean" } - | { status: "conflict"; error: StudioFileConflictError } - | { status: "failed"; error: unknown }; +export type StudioPendingEditsDrainResult = StudioSaveDrainResult; const pendingEditPromises = new Set>(); +function waitForPostBlurEffects(): Promise { + return new Promise((resolve) => setTimeout(resolve, 0)); +} + export function trackStudioPendingEdit( result: Promise | unknown, ): Promise | undefined { @@ -33,28 +34,33 @@ export async function flushStudioPendingEdits(): Promise(STUDIO_FLUSH_PENDING_EDITS_EVENT, { detail }), ); + let firstFailure: unknown; + let hasFailure = false; while (detail.promises.length > 0 || pendingEditPromises.size > 0) { const promises = [...detail.promises, ...pendingEditPromises]; detail.promises = []; const results = await Promise.allSettled(promises); - const rejected = results.find( - (result): result is PromiseRejectedResult => result.status === "rejected", - ); - if (rejected) { - return rejected.reason instanceof StudioFileConflictError - ? { status: "conflict", error: rejected.reason } - : { status: "failed", error: rejected.reason }; + for (const result of results) { + if (result.status !== "rejected") continue; + if (result.reason instanceof StudioFileConflictError) { + return { status: "conflict", error: result.reason }; + } + if (!hasFailure) { + firstFailure = result.reason; + hasFailure = true; + } } } - return { status: "clean" }; + return hasFailure ? { status: "failed", error: firstFailure } : { status: "clean" }; } export function addStudioPendingEditFlushListener( diff --git a/packages/studio/src/utils/studioSaveDiagnostics.ts b/packages/studio/src/utils/studioSaveDiagnostics.ts index 837af11c4d4..f635d02c9b8 100644 --- a/packages/studio/src/utils/studioSaveDiagnostics.ts +++ b/packages/studio/src/utils/studioSaveDiagnostics.ts @@ -56,6 +56,11 @@ export class StudioFileConflictError extends StudioSaveHttpError { } } +export type StudioSaveDrainResult = + | { status: "clean" } + | { status: "conflict"; error: StudioFileConflictError } + | { status: "failed"; error: Failure }; + function readNumericProperty(value: object, key: string): number | undefined { const record = value as Record; const property = record[key]; From d066cbe729c3758634459ac44186263a08610957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Tue, 4 Aug 2026 20:40:26 +0000 Subject: [PATCH 3/3] test(studio): satisfy drain audit gate (#2989) - share the editor-save hook harness across drain regressions - extract settled failure inspection from the drain loop --- .../studio/src/hooks/useEditorSave.test.tsx | 143 +++++++----------- .../studio/src/utils/studioPendingEdits.ts | 31 ++-- 2 files changed, 71 insertions(+), 103 deletions(-) diff --git a/packages/studio/src/hooks/useEditorSave.test.tsx b/packages/studio/src/hooks/useEditorSave.test.tsx index d646516bd10..ee5b7bf82e1 100644 --- a/packages/studio/src/hooks/useEditorSave.test.tsx +++ b/packages/studio/src/hooks/useEditorSave.test.tsx @@ -7,6 +7,35 @@ import { StudioFileConflictError } from "../utils/studioSaveDiagnostics"; (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; +type WriteProjectFile = (path: string, content: string, expectedContent?: string) => Promise; + +async function mountEditorSave(writeProjectFile: WriteProjectFile) { + const captured: { handle: EditorSaveHandle | null } = { handle: null }; + + function Probe() { + captured.handle = useEditorSave({ + editingPathRef: { current: "index.html" }, + projectIdRef: { current: "project-a" }, + readProjectFile: vi.fn(async () => "before"), + writeProjectFile, + recordEdit: vi.fn(async () => undefined), + domEditSaveTimestampRef: { current: 0 }, + setRefreshKey: vi.fn(), + showToast: vi.fn(), + }); + return null; + } + + const root = createRoot(document.createElement("div")); + await act(async () => root.render()); + if (!captured.handle) throw new Error("Editor save handle was not mounted"); + + return { + handle: captured.handle, + unmount: () => act(async () => root.unmount()), + }; +} + describe("useEditorSave pending work", () => { beforeEach(() => { vi.stubGlobal( @@ -20,35 +49,18 @@ describe("useEditorSave pending work", () => { it("exposes and flushes the latest rAF-buffered source candidate", async () => { const writeProjectFile = vi.fn(async () => undefined); - const captured: { handle: EditorSaveHandle | null } = { handle: null }; - - function Probe() { - captured.handle = useEditorSave({ - editingPathRef: { current: "index.html" }, - projectIdRef: { current: "project-a" }, - readProjectFile: vi.fn(async () => "before"), - writeProjectFile, - recordEdit: vi.fn(async () => undefined), - domEditSaveTimestampRef: { current: 0 }, - setRefreshKey: vi.fn(), - showToast: vi.fn(), - }); - return null; - } - - const root = createRoot(document.createElement("div")); - await act(async () => root.render()); - act(() => captured.handle?.handleContentChange("studio candidate")); - - expect(captured.handle?.getPendingCandidate()).toEqual({ + const mounted = await mountEditorSave(writeProjectFile); + act(() => mounted.handle.handleContentChange("studio candidate")); + + expect(mounted.handle.getPendingCandidate()).toEqual({ projectId: "project-a", path: "index.html", content: "studio candidate", }); - await expect(captured.handle?.flushPendingSave()).resolves.toEqual({ status: "clean" }); + await expect(mounted.handle.flushPendingSave()).resolves.toEqual({ status: "clean" }); expect(writeProjectFile).toHaveBeenCalledWith("index.html", "studio candidate", "before"); - await act(async () => root.unmount()); + await mounted.unmount(); }); it("joins an in-flight source save instead of writing the frozen candidate twice", async () => { @@ -70,32 +82,17 @@ describe("useEditorSave pending work", () => { }), ) .mockResolvedValue(undefined); - const captured: { handle: EditorSaveHandle | null } = { handle: null }; - function Probe() { - captured.handle = useEditorSave({ - editingPathRef: { current: "index.html" }, - projectIdRef: { current: "project-a" }, - readProjectFile: vi.fn(async () => "before"), - writeProjectFile, - recordEdit: vi.fn(async () => undefined), - domEditSaveTimestampRef: { current: 0 }, - setRefreshKey: vi.fn(), - showToast: vi.fn(), - }); - return null; - } - const root = createRoot(document.createElement("div")); - await act(async () => root.render()); - act(() => captured.handle?.handleContentChange("candidate")); + const mounted = await mountEditorSave(writeProjectFile); + act(() => mounted.handle.handleContentChange("candidate")); act(() => frame?.(0)); await vi.waitFor(() => expect(writeProjectFile).toHaveBeenCalledOnce()); - const drained = captured.handle?.flushPendingSave(); + const drained = mounted.handle.flushPendingSave(); expect(writeProjectFile).toHaveBeenCalledOnce(); finishWrite(); await expect(drained).resolves.toEqual({ status: "clean" }); expect(writeProjectFile).toHaveBeenCalledOnce(); - await act(async () => root.unmount()); + await mounted.unmount(); }); it("preserves conflict details when flushing a buffered source candidate", async () => { @@ -105,64 +102,30 @@ describe("useEditorSave pending work", () => { currentContent: "external", attemptedContent: "studio candidate", }); - const captured: { handle: EditorSaveHandle | null } = { handle: null }; - - function Probe() { - captured.handle = useEditorSave({ - editingPathRef: { current: "index.html" }, - projectIdRef: { current: "project-a" }, - readProjectFile: vi.fn(async () => "before"), - writeProjectFile: vi.fn(async () => { - throw conflict; - }), - recordEdit: vi.fn(async () => undefined), - domEditSaveTimestampRef: { current: 0 }, - setRefreshKey: vi.fn(), - showToast: vi.fn(), - }); - return null; - } - - const root = createRoot(document.createElement("div")); - await act(async () => root.render()); - act(() => captured.handle?.handleContentChange("studio candidate")); - - await expect(captured.handle?.flushPendingSave()).resolves.toEqual({ + const mounted = await mountEditorSave(async () => { + throw conflict; + }); + act(() => mounted.handle.handleContentChange("studio candidate")); + + await expect(mounted.handle.flushPendingSave()).resolves.toEqual({ status: "conflict", error: conflict, }); - await act(async () => root.unmount()); + await mounted.unmount(); }); it("discards an rAF-buffered candidate without persisting it", async () => { const writeProjectFile = vi.fn(async () => undefined); - const captured: { handle: EditorSaveHandle | null } = { handle: null }; - - function Probe() { - captured.handle = useEditorSave({ - editingPathRef: { current: "index.html" }, - projectIdRef: { current: "project-a" }, - readProjectFile: vi.fn(async () => "before"), - writeProjectFile, - recordEdit: vi.fn(async () => undefined), - domEditSaveTimestampRef: { current: 0 }, - setRefreshKey: vi.fn(), - showToast: vi.fn(), - }); - return null; - } - - const root = createRoot(document.createElement("div")); - await act(async () => root.render()); - act(() => captured.handle?.handleContentChange("discard me")); - act(() => captured.handle?.discardPendingSave()); - - expect(captured.handle?.getPendingCandidate()).toBeNull(); - await expect(captured.handle?.flushPendingSave()).resolves.toEqual({ status: "clean" }); + const mounted = await mountEditorSave(writeProjectFile); + act(() => mounted.handle.handleContentChange("discard me")); + act(() => mounted.handle.discardPendingSave()); + + expect(mounted.handle.getPendingCandidate()).toBeNull(); + await expect(mounted.handle.flushPendingSave()).resolves.toEqual({ status: "clean" }); expect(writeProjectFile).not.toHaveBeenCalled(); expect(cancelAnimationFrame).toHaveBeenCalledWith(41); - await act(async () => root.unmount()); + await mounted.unmount(); }); }); diff --git a/packages/studio/src/utils/studioPendingEdits.ts b/packages/studio/src/utils/studioPendingEdits.ts index 7eddcebe817..876ce7e4323 100644 --- a/packages/studio/src/utils/studioPendingEdits.ts +++ b/packages/studio/src/utils/studioPendingEdits.ts @@ -14,6 +14,19 @@ function waitForPostBlurEffects(): Promise { return new Promise((resolve) => setTimeout(resolve, 0)); } +function inspectDrainFailures(results: PromiseSettledResult[]): { + conflict?: StudioFileConflictError; + firstFailure?: PromiseRejectedResult; +} { + let firstFailure: PromiseRejectedResult | undefined; + for (const result of results) { + if (result.status !== "rejected") continue; + if (result.reason instanceof StudioFileConflictError) return { conflict: result.reason }; + firstFailure ??= result; + } + return { firstFailure }; +} + export function trackStudioPendingEdit( result: Promise | unknown, ): Promise | undefined { @@ -43,24 +56,16 @@ export async function flushStudioPendingEdits(): Promise(STUDIO_FLUSH_PENDING_EDITS_EVENT, { detail }), ); - let firstFailure: unknown; - let hasFailure = false; + let firstFailure: PromiseRejectedResult | undefined; while (detail.promises.length > 0 || pendingEditPromises.size > 0) { const promises = [...detail.promises, ...pendingEditPromises]; detail.promises = []; const results = await Promise.allSettled(promises); - for (const result of results) { - if (result.status !== "rejected") continue; - if (result.reason instanceof StudioFileConflictError) { - return { status: "conflict", error: result.reason }; - } - if (!hasFailure) { - firstFailure = result.reason; - hasFailure = true; - } - } + const batchFailures = inspectDrainFailures(results); + if (batchFailures.conflict) return { status: "conflict", error: batchFailures.conflict }; + firstFailure ??= batchFailures.firstFailure; } - return hasFailure ? { status: "failed", error: firstFailure } : { status: "clean" }; + return firstFailure ? { status: "failed", error: firstFailure.reason } : { status: "clean" }; } export function addStudioPendingEditFlushListener(