Skip to content

Commit 4b54880

Browse files
committed
fix(presets): handle persistence failures
1 parent 112bea0 commit 4b54880

10 files changed

Lines changed: 126 additions & 19 deletions

‎electron/ai-edition/style-preset-service.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,13 @@ describe("StylePresetService", () => {
129129
await expectCode(service.create("corrupt", APPEARANCE), "NAME_TAKEN");
130130
});
131131

132+
it("reserves the built-in preset id on create and rename", async () => {
133+
await expectCode(service.create("openscreen-factory", APPEARANCE), "NAME_TAKEN");
134+
const created = await service.create("Mine", APPEARANCE);
135+
await expectCode(service.rename(created.id, "OPENSCREEN-FACTORY"), "NAME_TAKEN");
136+
expect((await service.list()).map((preset) => preset.id)).toEqual(["Mine"]);
137+
});
138+
132139
it("rejects an invalid appearance before writing anything", async () => {
133140
await expect(
134141
service.create("Bad", { ...APPEARANCE, wallpaper: "file:///Users/alice/a.jpg" }),
@@ -156,6 +163,24 @@ describe("StylePresetService", () => {
156163
expect(await fs.readdir(dir)).toEqual(["Studio.openscreenpreset"]);
157164
});
158165

166+
it("removes the destination and preserves the source when rename cleanup fails", async () => {
167+
const created = await service.create("Source", APPEARANCE);
168+
const sourcePath = path.join(dir, "Source.openscreenpreset");
169+
const unlink = fs.unlink.bind(fs);
170+
vi.spyOn(fs, "unlink").mockImplementation(async (filePath) => {
171+
if (filePath === sourcePath) {
172+
throw Object.assign(new Error("busy"), { code: "EBUSY" });
173+
}
174+
return unlink(filePath);
175+
});
176+
177+
await expect(service.rename(created.id, "Destination")).rejects.toMatchObject({
178+
code: "EBUSY",
179+
});
180+
expect((await fs.readdir(dir)).sort()).toEqual(["Source.openscreenpreset"]);
181+
expect(await service.list()).toMatchObject([{ id: "Source", name: "Source" }]);
182+
});
183+
159184
it("updates the appearance and keeps the name", async () => {
160185
const created = await service.create("Look", APPEARANCE);
161186
const updated = await service.update(created.id, { ...APPEARANCE, padding: 10 });

‎electron/ai-edition/style-preset-service.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import fs, { type FileHandle } from "node:fs/promises";
1111
import path from "node:path";
1212
import {
1313
compareStylePresets,
14+
FACTORY_STYLE_PRESET_ID,
1415
parseStylePresetFile,
1516
STYLE_PRESET_FILE_EXTENSION,
1617
type StylePreset,
@@ -116,6 +117,7 @@ export class StylePresetService {
116117
// Validates before anything touches the disk.
117118
const json = serializeStylePresetFile({ name: cleanName, appearance });
118119
const id = stylePresetFileBaseName(cleanName);
120+
this.assertNotFactoryId(id, cleanName);
119121
await this.ensureDirectory();
120122
this.assertNameFree(await this.readEntries(), id, cleanName);
121123
await this.writeAtomic(this.pathFor(id), json);
@@ -128,6 +130,7 @@ export class StylePresetService {
128130
const current = await this.read(id);
129131
const cleanName = sanitizeStylePresetName(name);
130132
const nextId = stylePresetFileBaseName(cleanName);
133+
this.assertNotFactoryId(nextId, cleanName);
131134
this.assertNameFree(await this.readEntries(), nextId, cleanName, id);
132135
const json = serializeStylePresetFile({ name: cleanName, appearance: current.appearance });
133136
const from = this.pathFor(id);
@@ -142,9 +145,12 @@ export class StylePresetService {
142145
await renameWithRetry(from, to);
143146
} else {
144147
await this.writeAtomic(to, json);
145-
await fs.unlink(from).catch((error) => {
148+
try {
149+
await fs.unlink(from);
150+
} catch (error) {
151+
if (!isMissing(error)) await fs.unlink(to).catch(() => undefined);
146152
if (!isMissing(error)) throw error;
147-
});
153+
}
148154
}
149155
return this.read(nextId);
150156
});
@@ -245,6 +251,12 @@ export class StylePresetService {
245251
}
246252
}
247253

254+
private assertNotFactoryId(id: string, name: string): void {
255+
if (id.toLowerCase() === FACTORY_STYLE_PRESET_ID) {
256+
throw new StylePresetError("NAME_TAKEN", `A style preset named "${name}" already exists.`);
257+
}
258+
}
259+
248260
private serialise<T>(work: () => Promise<T>): Promise<T> {
249261
// Chained on both settlements: one failed mutation must not cancel the next.
250262
const run = this.writeTail.then(work, work);

‎src/components/ai-edition/StylePresetsMenu.tsx‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ import { getEditorSettings, patchEditorSettings } from "@/lib/ai-edition/store/e
1717
import { useProjectStore } from "@/lib/ai-edition/store/projectStore";
1818
import { useEditorSettings } from "@/lib/ai-edition/store/useEditorSettings";
1919
import {
20+
FACTORY_STYLE_PRESET_ID,
2021
type StylePreset,
2122
type StylePresetAppearance,
2223
sanitizeStylePresetName,
2324
} from "@/lib/ai-edition/stylePresets";
2425
import {
25-
FACTORY_STYLE_PRESET_ID,
2626
factoryStylePresetAppearance,
2727
stylePresetAppearanceFromSettings,
2828
stylePresetPatch,
@@ -96,7 +96,7 @@ function NameForm({
9696
}: NameFormProps) {
9797
const [name, setName] = useState(initialName);
9898
const onKeyDown = (event: KeyboardEvent<HTMLInputElement>) => {
99-
if (event.key === "Enter") {
99+
if (event.key === "Enter" && !busy) {
100100
event.preventDefault();
101101
onSubmit(name);
102102
}
@@ -106,6 +106,7 @@ function NameForm({
106106
<div className={styles.presetForm}>
107107
<input
108108
type="text"
109+
disabled={busy}
109110
autoFocus
110111
value={name}
111112
placeholder={placeholder}
@@ -188,16 +189,16 @@ export function StylePresetsMenu() {
188189
const current = stylePresetAppearanceFromSettings(settings);
189190
const factory = factoryStylePresetAppearance();
190191

191-
const apply = (appearance: StylePresetAppearance, name: string) => {
192+
const apply = async (appearance: StylePresetAppearance, name: string) => {
192193
setOpen(false);
193194
const patch = stylePresetPatch(appearance);
195+
if (!(await set(patch))) return;
194196
// The overlay only pushes native params on mount, and each control here pushes its own
195197
// diff; a preset touches all of them at once, so it pushes the whole resulting snapshot.
196198
const doc = useProjectStore.getState().document;
197199
if (doc && isNativeCompositorActive()) {
198200
pushAllNativeParams(getEditorSettings(patchEditorSettings(doc, patch)));
199201
}
200-
void set(patch);
201202
toast.success(ts("stylePresets.applied", { name }));
202203
};
203204

@@ -323,7 +324,7 @@ export function StylePresetsMenu() {
323324
role="menuitemradio"
324325
aria-checked={active}
325326
className={rowClass(active)}
326-
onClick={() => apply(preset.appearance, preset.name)}
327+
onClick={() => void apply(preset.appearance, preset.name)}
327328
>
328329
<span className={styles.presetCheck} aria-hidden="true">
329330
{active ? <Check size={12} /> : null}
@@ -433,7 +434,7 @@ export function StylePresetsMenu() {
433434
aria-checked={factoryActive}
434435
key={FACTORY_STYLE_PRESET_ID}
435436
className={rowClass(factoryActive)}
436-
onClick={() => apply(factory, ts("stylePresets.factoryName"))}
437+
onClick={() => void apply(factory, ts("stylePresets.factoryName"))}
437438
>
438439
<span className={styles.presetCheck} aria-hidden="true">
439440
{factoryActive ? <Check size={12} /> : null}

‎src/components/ai-edition/v4/EditorTopBar.stylePresets.test.tsx‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ beforeEach(() => {
131131
localStorage.clear();
132132
state.platform = "darwin";
133133
state.settings = DEFAULT_EDITOR_SETTINGS;
134-
state.set = vi.fn<Fn>(async () => undefined);
134+
state.set = vi.fn<Fn>(async () => true);
135135
state.toastSuccess = vi.fn<Fn>();
136136
state.presets.list = vi.fn<Fn>(async () => [WARM]);
137137
state.presets.create = vi.fn<Fn>(async (name) => ({ ...WARM, id: name, name }));
@@ -182,7 +182,16 @@ describe("Presets menu in the editor top bar", () => {
182182
expect(state.set).toHaveBeenCalledTimes(1);
183183
expect(state.set).toHaveBeenCalledWith(stylePresetPatch(WARM.appearance));
184184
expect(screen.queryByRole("menu")).not.toBeInTheDocument();
185-
expect(state.toastSuccess).toHaveBeenCalledWith("Applied “Warm”");
185+
await waitFor(() => expect(state.toastSuccess).toHaveBeenCalledWith("Applied “Warm”"));
186+
});
187+
188+
it("does not report an applied preset when the project save fails", async () => {
189+
state.set = vi.fn<Fn>(async () => false);
190+
renderPane();
191+
await openMenu();
192+
fireEvent.click(screen.getByRole("menuitemradio", { name: "Warm" }));
193+
await waitFor(() => expect(state.set).toHaveBeenCalledTimes(1));
194+
expect(state.toastSuccess).not.toHaveBeenCalled();
186195
});
187196

188197
it("applies the built-in preset from its row", async () => {
@@ -253,6 +262,23 @@ describe("Presets menu in the editor top bar", () => {
253262
expect(screen.queryByRole("textbox")).not.toBeInTheDocument();
254263
});
255264

265+
it("blocks repeated Enter submissions while create is busy", async () => {
266+
let finishCreate: (() => void) | undefined;
267+
state.presets.create = vi.fn<Fn>(
268+
() => new Promise<void>((resolve) => (finishCreate = resolve)),
269+
);
270+
renderPane();
271+
await openMenu();
272+
fireEvent.click(screen.getByRole("menuitem", { name: "Create new preset…" }));
273+
const input = screen.getByRole("textbox", { name: "Preset name" });
274+
fireEvent.change(input, { target: { value: "Mine" } });
275+
fireEvent.keyDown(input, { key: "Enter" });
276+
await waitFor(() => expect(input).toBeDisabled());
277+
fireEvent.keyDown(input, { key: "Enter" });
278+
expect(state.presets.create).toHaveBeenCalledTimes(1);
279+
finishCreate?.();
280+
});
281+
256282
it("asks for a name instead of saving a blank one", async () => {
257283
renderPane();
258284
await openMenu();

‎src/lib/ai-edition/store/useEditorSettings.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,13 @@ describe("useEditorSettings drag snapshots", () => {
111111
vi.clearAllMocks();
112112
});
113113

114+
it("returns whether a settings edit persisted", async () => {
115+
const { result } = renderHook(() => useEditorSettings());
116+
await expect(result.current.set({ borderRadius: 10 })).resolves.toBe(true);
117+
bridgeMocks.save.mockResolvedValueOnce({ success: false, error: "disk full" });
118+
await expect(result.current.set({ borderRadius: 20 })).resolves.toBe(false);
119+
});
120+
114121
it("does not record a snapshot of the project the user left", async () => {
115122
const { result, rerender } = renderHook(() => useEditorSettings());
116123

‎src/lib/ai-edition/store/useEditorSettings.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export interface UseEditorSettingsResult {
2727
/** True when there's a project loaded — `set`/`setLive` are no-ops otherwise. */
2828
hasDocument: boolean;
2929
/** Apply a patch, persist to disk. */
30-
set: (patch: EditorSettingsPatch) => Promise<void>;
30+
set: (patch: EditorSettingsPatch) => Promise<boolean>;
3131
/** Apply a patch, no persist. Pair with `commit` on slider release. */
3232
setLive: (patch: EditorSettingsPatch) => void;
3333
/** Force-flush the current document to disk. */
@@ -47,13 +47,13 @@ export function useEditorSettings(): UseEditorSettingsResult {
4747
const set = useCallback(
4848
async (patch: EditorSettingsPatch) => {
4949
const doc = useProjectStore.getState().document;
50-
if (!doc) return;
50+
if (!doc) return false;
5151
const next = patchEditorSettings(doc, patch);
5252
// The optimistic write is not the edit — the save is. Only the one that can
5353
// fail records, and it names `doc` as what Ctrl+Z returns to because by then
5454
// the store already holds `next`.
5555
setDocument(next, { history: false });
56-
await saveDocument(next, { history: true, historyBase: doc });
56+
return saveDocument(next, { history: true, historyBase: doc });
5757
},
5858
[setDocument, saveDocument],
5959
);

‎src/lib/ai-edition/stylePresets.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ export const STYLE_PRESET_FILE_EXTENSION = ".openscreenpreset";
2424
export const STYLE_PRESET_FORMAT = "openscreen-style-preset";
2525
export const STYLE_PRESET_FORMAT_VERSION = 1;
2626
export const STYLE_PRESET_NAME_MAX_LENGTH = 80;
27+
/** Reserved for the built-in row; a user preset must never resolve to this file id. */
28+
export const FACTORY_STYLE_PRESET_ID = "openscreen-factory";
2729
/** Characters, not bytes: a 15 MB base64 image is ~11 MB decoded, far past any wallpaper. */
2830
export const STYLE_PRESET_DATA_URL_MAX_LENGTH = 15 * 1024 * 1024;
2931
/** Colours and gradients are CSS text; anything this long is not one a user typed. */

‎src/lib/ai-edition/stylePresetsEditor.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@ import {
99
} from "@/lib/ai-edition/store/editorSettings";
1010
import type { StylePresetAppearance } from "@/lib/ai-edition/stylePresets";
1111

12-
/** The built-in look every new project starts from. Never a file, so never an id on disk
13-
* either: file ids are base names, and this one is reserved for the list's first entry. */
14-
export const FACTORY_STYLE_PRESET_ID = "openscreen-factory";
15-
1612
/** Copies the appearance fields out of a settings snapshot — nothing footage-dependent. */
1713
export function stylePresetAppearanceFromSettings(
1814
settings: EditorSettingsSnapshot,

‎src/native/browserShim.presets.test.ts‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// @vitest-environment jsdom
22
// The shim keeps presets in localStorage, so this needs a DOM.
3-
import { beforeAll, beforeEach, describe, expect, it } from "vitest";
3+
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
44
import type { StylePresetAppearance } from "@/lib/ai-edition/stylePresets";
55
import { installBrowserShims } from "./browserShim";
66
import {
@@ -78,6 +78,36 @@ describe("browserShim presets", () => {
7878
expect((await presets.list()).map((p) => p.id)).toEqual(["Three", "Two"]);
7979
});
8080

81+
it("reserves the built-in preset id on create and rename", async () => {
82+
await expect(presets.create("openscreen-factory", APPEARANCE)).rejects.toMatchObject({
83+
code: "NAME_TAKEN",
84+
});
85+
const created = await presets.create("Mine", APPEARANCE);
86+
await expect(presets.rename(created.id, "OPENSCREEN-FACTORY")).rejects.toMatchObject({
87+
code: "NAME_TAKEN",
88+
});
89+
});
90+
91+
it("rejects every mutation when localStorage persistence fails", async () => {
92+
const created = await presets.create("Mine", APPEARANCE);
93+
const setItem = vi.spyOn(Storage.prototype, "setItem").mockImplementation(() => {
94+
throw new DOMException("full", "QuotaExceededError");
95+
});
96+
try {
97+
for (const mutate of [
98+
() => presets.create("Other", APPEARANCE),
99+
() => presets.rename(created.id, "Renamed"),
100+
() => presets.update(created.id, { ...APPEARANCE, padding: 10 }),
101+
() => presets.delete(created.id),
102+
]) {
103+
await expect(mutate()).rejects.toMatchObject({ code: "INTERNAL_ERROR" });
104+
}
105+
} finally {
106+
setItem.mockRestore();
107+
}
108+
expect((await presets.list()).map((preset) => preset.id)).toEqual(["Mine"]);
109+
});
110+
81111
it("reports invalid input and missing presets with the bridge's codes", async () => {
82112
const invalid = await rejection(
83113
presets.create("Bad", { ...APPEARANCE, wallpaper: "file:///Users/alice/a.jpg" }),

‎src/native/browserShim.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { PROVIDER_DEFINITIONS } from "../../electron/ai-edition/provider-registr
77
import { axcutSchemaVersion, migrateRawDocumentToCurrent } from "../lib/ai-edition/schema";
88
import {
99
compareStylePresets,
10+
FACTORY_STYLE_PRESET_ID,
1011
parseStylePresetAppearance,
1112
type StylePreset,
1213
type StylePresetAppearance,
@@ -400,7 +401,11 @@ function createShimBridgeClient() {
400401
try {
401402
localStorage.setItem(presetsStorageKey, JSON.stringify(records));
402403
} catch {
403-
// ponytail: localStorage may be full or unavailable; silently skip
404+
throw new NativeBridgeRequestError({
405+
code: "INTERNAL_ERROR",
406+
message: "Failed to persist style presets.",
407+
retryable: false,
408+
});
404409
}
405410
};
406411
const presetError = (code: "NAME_TAKEN" | "NOT_FOUND" | "INVALID_REQUEST", message: string) =>
@@ -426,6 +431,9 @@ function createShimBridgeClient() {
426431
name: string,
427432
exceptId?: string,
428433
) => {
434+
if (id.toLowerCase() === FACTORY_STYLE_PRESET_ID) {
435+
throw presetError("NAME_TAKEN", `A style preset named "${name}" already exists.`);
436+
}
429437
const taken = Object.entries(records).some(
430438
([otherId, record]) =>
431439
otherId !== exceptId &&

0 commit comments

Comments
 (0)