fix(studio): drain pending edits before reload (#2989)

* fix(studio): drain pending edits before reload

* 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

* test(studio): satisfy drain audit gate (#2989)

- share the editor-save hook harness across drain regressions
- extract settled failure inspection from the drain loop
This commit is contained in:
Miguel Ángel
2026-08-04 20:58:19 +00:00
committed by GitHub
parent 552419c52d
commit 4713138544
9 changed files with 513 additions and 48 deletions
@@ -0,0 +1,131 @@
// @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";
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<void>;
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(<Probe />));
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(
"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 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(mounted.handle.flushPendingSave()).resolves.toEqual({ status: "clean" });
expect(writeProjectFile).toHaveBeenCalledWith("index.html", "studio candidate", "before");
await mounted.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<void>((resolve) => {
finishWrite = resolve;
}),
)
.mockResolvedValue(undefined);
const mounted = await mountEditorSave(writeProjectFile);
act(() => mounted.handle.handleContentChange("candidate"));
act(() => frame?.(0));
await vi.waitFor(() => expect(writeProjectFile).toHaveBeenCalledOnce());
const drained = mounted.handle.flushPendingSave();
expect(writeProjectFile).toHaveBeenCalledOnce();
finishWrite();
await expect(drained).resolves.toEqual({ status: "clean" });
expect(writeProjectFile).toHaveBeenCalledOnce();
await mounted.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 mounted = await mountEditorSave(async () => {
throw conflict;
});
act(() => mounted.handle.handleContentChange("studio candidate"));
await expect(mounted.handle.flushPendingSave()).resolves.toEqual({
status: "conflict",
error: conflict,
});
await mounted.unmount();
});
it("discards an rAF-buffered candidate without persisting it", async () => {
const writeProjectFile = vi.fn(async () => undefined);
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 mounted.unmount();
});
});
+113 -39
View File
@@ -2,6 +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,
type StudioSaveDrainResult,
} from "../utils/studioSaveDiagnostics";
interface RecordEditInput {
label: string;
@@ -21,6 +25,25 @@ interface UseEditorSaveOptions {
showToast: (message: string, tone?: "error" | "info") => void;
}
export interface EditorSaveCandidate {
projectId: string;
path: string;
content: string;
}
export type EditorSaveDrainResult = StudioSaveDrainResult;
export interface EditorSaveHandle {
saveRafRef: React.MutableRefObject<number | null>;
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<EditorSaveDrainResult>;
/** Used by PR #2993 when the external version wins. */
discardPendingSave: () => void;
}
export function useEditorSave({
editingPathRef,
projectIdRef,
@@ -30,12 +53,70 @@ export function useEditorSave({
domEditSaveTimestampRef,
setRefreshKey,
showToast,
}: UseEditorSaveOptions) {
}: UseEditorSaveOptions): EditorSaveHandle {
const saveRafRef = useRef<number | null>(null);
const refreshRafRef = useRef<number | null>(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<EditorSaveCandidate | null>(null);
const inFlightRef = useRef<Promise<EditorSaveDrainResult> | null>(null);
const inFlightCandidateRef = useRef<EditorSaveCandidate | null>(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<EditorSaveDrainResult> => {
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<EditorSaveDrainResult>(() => {
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<EditorSaveDrainResult>((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 +125,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<EditorSaveDrainResult> => {
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,
};
}
@@ -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(() => {
@@ -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 });
@@ -133,4 +150,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();
});
});
+22 -4
View File
@@ -1,4 +1,9 @@
import { getStudioSaveErrorMessage, getStudioSaveStatusCode } from "./studioSaveDiagnostics";
import {
getStudioSaveErrorMessage,
getStudioSaveStatusCode,
StudioFileConflictError,
type StudioSaveDrainResult,
} from "./studioSaveDiagnostics";
interface DomEditSaveQueueOpenEvent {
consecutiveFailures: number;
@@ -14,11 +19,13 @@ interface DomEditSaveQueueOptions {
export interface DomEditSaveQueue {
enqueue: <T>(save: () => Promise<T>) => Promise<T>;
waitForIdle: () => Promise<void>;
waitForIdle: () => Promise<DomEditSaveDrainResult>;
reset: () => void;
destroy: () => void;
}
export type DomEditSaveDrainResult = StudioSaveDrainResult;
const DEFAULT_FAILURE_THRESHOLD = 5;
export class DomEditSaveQueueOpenError extends Error {
@@ -34,11 +41,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?.();
};
@@ -55,9 +64,13 @@ export function createDomEditSaveQueue(options: DomEditSaveQueueOptions = {}): D
const run = async <T>(save: () => Promise<T>): Promise<T> => {
try {
const result = await save();
if (!breakerOpen) consecutiveFailures = 0;
if (!breakerOpen) {
consecutiveFailures = 0;
drainError = null;
}
return result;
} catch (error) {
drainError = error;
consecutiveFailures += 1;
if (getStudioSaveStatusCode(error) === 409 || consecutiveFailures >= failureThreshold)
open(error);
@@ -76,8 +89,13 @@ export function createDomEditSaveQueue(options: DomEditSaveQueueOptions = {}): D
return queued;
},
async waitForIdle() {
async waitForIdle(): Promise<DomEditSaveDrainResult> {
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,
@@ -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,24 @@ 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);
});
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);
});
});
@@ -1,3 +1,33 @@
// 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<string, number>();
function pruneStudioWriteTokens(now: number): void {
for (const [token, createdAt] of studioWriteTokens) {
if (now - createdAt >= WRITE_TOKEN_TTL_MS) studioWriteTokens.delete(token);
}
}
/** 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;
studioWriteTokens.delete(token);
return true;
}
/** Resets module-level echo identity between PR #2991 coordinator tests. */
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<string> {
const bytes = new TextEncoder().encode(content);
@@ -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,119 @@ 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("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<void> | undefined;
input.addEventListener("blur", () => {
registrationDone = new Promise<void>((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 () => {
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("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;
@@ -1,11 +1,32 @@
import { StudioFileConflictError, type StudioSaveDrainResult } from "./studioSaveDiagnostics";
const STUDIO_FLUSH_PENDING_EDITS_EVENT = "hf-studio-flush-pending-edits";
interface StudioFlushPendingEditsDetail {
promises: Array<Promise<unknown>>;
}
export type StudioPendingEditsDrainResult = StudioSaveDrainResult;
const pendingEditPromises = new Set<Promise<unknown>>();
function waitForPostBlurEffects(): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, 0));
}
function inspectDrainFailures(results: PromiseSettledResult<unknown>[]): {
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> | unknown,
): Promise<unknown> | undefined {
@@ -19,16 +40,32 @@ export function trackStudioPendingEdit(
return promise;
}
export async function flushStudioPendingEdits(): Promise<void> {
export async function flushStudioPendingEdits(): Promise<StudioPendingEditsDrainResult> {
const active = document.activeElement;
if (
active instanceof HTMLElement &&
active.matches('input, textarea, select, [contenteditable="true"], [role="textbox"]')
) {
active.blur();
// ponytail: Preserve synchronous/microtask blur commits, then cross one task boundary
// so React effects triggered by the blur can register their flush listener.
await Promise.resolve();
await waitForPostBlurEffects();
}
const detail: StudioFlushPendingEditsDetail = { promises: [] };
window.dispatchEvent(
new CustomEvent<StudioFlushPendingEditsDetail>(STUDIO_FLUSH_PENDING_EDITS_EVENT, { detail }),
);
let firstFailure: PromiseRejectedResult | undefined;
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 batchFailures = inspectDrainFailures(results);
if (batchFailures.conflict) return { status: "conflict", error: batchFailures.conflict };
firstFailure ??= batchFailures.firstFailure;
}
return firstFailure ? { status: "failed", error: firstFailure.reason } : { status: "clean" };
}
export function addStudioPendingEditFlushListener(
@@ -56,6 +56,11 @@ export class StudioFileConflictError extends StudioSaveHttpError {
}
}
export type StudioSaveDrainResult<Failure = unknown> =
| { status: "clean" }
| { status: "conflict"; error: StudioFileConflictError }
| { status: "failed"; error: Failure };
function readNumericProperty(value: object, key: string): number | undefined {
const record = value as Record<string, unknown>;
const property = record[key];