From cd49efa00a01c18a937e71e5cab7662b7017466e Mon Sep 17 00:00:00 2001 From: Mike Olson Date: Wed, 23 Sep 2026 20:20:53 -0400 Subject: [PATCH] fix(mobile): Import legacy model options without replacing choices --- .../src/state/use-composer-drafts.test.ts | 172 +++++++++++++++++- apps/mobile/src/state/use-composer-drafts.ts | 99 ++++++++-- 2 files changed, 249 insertions(+), 22 deletions(-) diff --git a/apps/mobile/src/state/use-composer-drafts.test.ts b/apps/mobile/src/state/use-composer-drafts.test.ts index ec79cf043191..2f855b118dc8 100644 --- a/apps/mobile/src/state/use-composer-drafts.test.ts +++ b/apps/mobile/src/state/use-composer-drafts.test.ts @@ -9,9 +9,13 @@ import { ThreadId, } from "@t3tools/contracts"; import { onTestFinished, vi } from "vite-plus/test"; +import { resolveNewTaskModelSelection } from "../lib/modelOptions"; const composerDraftFileMocks = vi.hoisted(() => { let document = JSON.stringify({ schemaVersion: 1, drafts: {} }); + let legacyDocument: string | null = null; + let legacyReadError: Error | null = null; + const legacyReads = vi.fn(); let readError: Error | null = null; let writeError: Error | null = null; let releaseRead: (() => void) | null = null; @@ -23,6 +27,13 @@ const composerDraftFileMocks = vi.hoisted(() => { return { readImage, + legacyReads, + setLegacyDocument(value: string | null) { + legacyDocument = value; + }, + setLegacyReadError(error: Error | null) { + legacyReadError = error; + }, blockRead() { readBarrier = new Promise((resolve) => { releaseRead = resolve; @@ -64,15 +75,32 @@ const composerDraftFileMocks = vi.hoisted(() => { } }, File: class { - exists = true; + readonly legacy: boolean; + constructor(_directory: unknown, fileName: string) { + this.legacy = fileName === "model-option-memory.json"; + } + get exists() { + return !this.legacy || legacyDocument !== null; + } parentDirectory = null; create() {} - moveSync() {} + moveSync() { + if (this.legacy) throw new Error("Legacy memory must remain read-only"); + } + + delete() { + if (this.legacy) throw new Error("Legacy memory must remain read-only"); + } async text() { await readBarrier; + if (this.legacy) { + legacyReads(); + if (legacyReadError) throw legacyReadError; + return legacyDocument!; + } if (readError) throw readError; return document; } @@ -82,6 +110,7 @@ const composerDraftFileMocks = vi.hoisted(() => { } write(value: string) { + if (this.legacy) throw new Error("Legacy memory must remain read-only"); if (writeError) { throw writeError; } @@ -206,6 +235,9 @@ afterEach(() => { resetComposerDraftsLoadState(); composerDraftFileMocks.setDocument({ schemaVersion: 1, drafts: {} }); composerDraftFileMocks.setReadError(null); + composerDraftFileMocks.setLegacyDocument(null); + composerDraftFileMocks.setLegacyReadError(null); + composerDraftFileMocks.legacyReads.mockClear(); composerDraftFileMocks.setWriteError(null); composerDraftFileMocks.setNextWriteBarrier(null); composerDraftFileMocks.setOnWrite(null); @@ -1956,6 +1988,142 @@ describe("mobile composer drafts", () => { }); }); + it("imports trial memory once, with per-model disk and in-flight choices winning", async () => { + vi.useFakeTimers(); + const options = (value: string) => [{ id: "thinking", value }]; + composerDraftFileMocks.setDocument({ + schemaVersion: 1, + drafts: { "environment-1:saved": DRAFT }, + modelOptionMemory: { pi: { a: options("high"), b: options("medium") } }, + }); + composerDraftFileMocks.setLegacyDocument( + JSON.stringify({ + schemaVersion: 1, + byInstance: { + pi: { a: options("low"), b: options("low"), c: options("off"), empty: [] }, + other: { a: options("max") }, + }, + }), + ); + composerDraftFileMocks.blockRead(); + ensureComposerDraftsLoaded(); + appAtomRegistry.set(modelOptionMemoryAtom, { pi: { a: options("xhigh") } }); + composerDraftFileMocks.releaseRead(); + await waitForComposerDraftsLoaded(); + const expected = { + pi: { a: options("xhigh"), b: options("medium"), c: options("off") }, + other: { a: options("max") }, + }; + expect(appAtomRegistry.get(modelOptionMemoryAtom)).toEqual(expected); + // A failed migration write leaves saved drafts retryable. + composerDraftFileMocks.setWriteError(new Error("disk unavailable")); + await expect(flushComposerDrafts()).rejects.toMatchObject({ operation: "write" }); + expect( + JSON.parse(composerDraftFileMocks.getDocument()).legacyModelOptionMemoryImported, + ).toBeUndefined(); + composerDraftFileMocks.setWriteError(null); + await flushComposerDrafts(); + expect(JSON.parse(composerDraftFileMocks.getDocument())).toMatchObject({ + drafts: { "environment-1:saved": DRAFT }, + modelOptionMemory: expected, + legacyModelOptionMemoryImported: true, + }); + resetComposerDraftsLoadState(); + appAtomRegistry.set(modelOptionMemoryAtom, {}); + composerDraftFileMocks.legacyReads.mockClear(); + await waitForComposerDraftsLoaded(); + expect(appAtomRegistry.get(modelOptionMemoryAtom)).toEqual(expected); + expect(composerDraftFileMocks.legacyReads).not.toHaveBeenCalled(); + }); + + it.each(["fresh", "existing", "explicit"] as const)( + "imports legacy options without replacing the %s draft selection or sticky priority", + async (kind) => { + vi.useFakeTimers(); + const selection = { + instanceId: ProviderInstanceId.make("codex"), + model: "chosen", + options: [{ id: "reasoning", value: "high" }], + }; + const draft = { + text: "Keep this prompt", + attachments: [], + modelSelection: selection, + interactionMode: "plan" as const, + }; + const key = kind === "existing" ? "environment-1:thread" : "new-task:chosen"; + composerDraftFileMocks.setDocument({ + schemaVersion: 1, + drafts: kind === "fresh" ? {} : { [key]: draft }, + stickyModelSelection: selection, + }); + composerDraftFileMocks.setLegacyDocument( + JSON.stringify({ + schemaVersion: 1, + byInstance: { codex: { chosen: [{ id: "reasoning", value: "low" }] } }, + }), + ); + await waitForComposerDraftsLoaded(); + const restored = appAtomRegistry.get(composerDraftsAtom)[key]; + expect(restored).toEqual(kind === "fresh" ? undefined : draft); + expect(appAtomRegistry.get(stickyComposerModelSelectionAtom)).toEqual(selection); + expect(appAtomRegistry.get(modelOptionMemoryAtom)).toEqual({ + codex: { chosen: [{ id: "reasoning", value: "low" }] }, + }); + expect( + resolveNewTaskModelSelection({ + draftSelection: restored?.modelSelection ?? null, + stickySelection: appAtomRegistry.get(stickyComposerModelSelectionAtom), + projectDefaultSelection: null, + modelOptions: [], + }), + ).toEqual(selection); + await flushComposerDrafts(); + expect(JSON.parse(composerDraftFileMocks.getDocument()).stickyModelSelection).toEqual( + selection, + ); + }, + ); + + it.each(["json", "schema", "read"] as const)( + "preserves drafts and retries the import after a legacy %s failure", + async (failure) => { + vi.useFakeTimers(); + composerDraftFileMocks.setDocument({ + schemaVersion: 1, + drafts: { "environment-1:saved": DRAFT }, + }); + composerDraftFileMocks.setLegacyDocument( + failure === "json" ? "{" : JSON.stringify({ schemaVersion: 999, byInstance: {} }), + ); + if (failure === "read") composerDraftFileMocks.setLegacyReadError(new Error("unavailable")); + setComposerDraftText("environment-1:new", "New edits"); + await flushComposerDrafts(); + expect(JSON.parse(composerDraftFileMocks.getDocument())).toMatchObject({ + drafts: { "environment-1:saved": DRAFT, "environment-1:new": { text: "New edits" } }, + }); + expect( + JSON.parse(composerDraftFileMocks.getDocument()).legacyModelOptionMemoryImported, + ).toBeUndefined(); + resetComposerDraftsLoadState(); + composerDraftFileMocks.setLegacyReadError(null); + composerDraftFileMocks.setLegacyDocument( + JSON.stringify({ + schemaVersion: 1, + byInstance: { pi: { a: [{ id: "thinking", value: "max" }] } }, + }), + ); + await flushComposerDrafts(); + expect(JSON.parse(composerDraftFileMocks.getDocument()).legacyModelOptionMemoryImported).toBe( + true, + ); + expect(appAtomRegistry.get(composerDraftsAtom)["environment-1:saved"]).toEqual(DRAFT); + expect(JSON.parse(composerDraftFileMocks.getDocument()).modelOptionMemory).toEqual({ + pi: { a: [{ id: "thinking", value: "max" }] }, + }); + }, + ); + it("waits for hydration before persisting the latest composer state", async () => { vi.useFakeTimers(); composerDraftFileMocks.setDocument({ diff --git a/apps/mobile/src/state/use-composer-drafts.ts b/apps/mobile/src/state/use-composer-drafts.ts index 6e33452605b0..bd2aa88f32e9 100644 --- a/apps/mobile/src/state/use-composer-drafts.ts +++ b/apps/mobile/src/state/use-composer-drafts.ts @@ -398,16 +398,21 @@ const ComposerDraftSchema = Schema.Struct({ project: Schema.optional(ComposerDraftProjectSchema), }); +const ModelOptionMemorySchema = Schema.Record( + Schema.String, + Schema.Record(Schema.String, Schema.Array(ProviderOptionSelectionSchema)), +); +const LEGACY_MODEL_OPTION_MEMORY_FILE = "model-option-memory.json"; +const decodeLegacyModelOptionMemory = Schema.decodeUnknownSync( + Schema.Struct({ schemaVersion: Schema.Literal(1), byInstance: ModelOptionMemorySchema }), +); + const PersistedComposerDraftsSchema = Schema.Struct({ schemaVersion: Schema.Literal(COMPOSER_DRAFTS_SCHEMA_VERSION), drafts: Schema.Record(Schema.String, ComposerDraftSchema), stickyModelSelection: Schema.optional(ModelSelectionSchema), - modelOptionMemory: Schema.optional( - Schema.Record( - Schema.String, - Schema.Record(Schema.String, Schema.Array(ProviderOptionSelectionSchema)), - ), - ), + modelOptionMemory: Schema.optional(ModelOptionMemorySchema), + legacyModelOptionMemoryImported: Schema.optional(Schema.Literal(true)), cloudAccountId: Schema.optional(Schema.String), signedOutDrafts: Schema.optional( Schema.Record( @@ -466,12 +471,14 @@ export const composerCloudDraftsAtom = Atom.make({ let loadPromise: Promise | null = null; let persistTimer: ReturnType | null = null; let persistRetryNeeded = false; +let legacyModelOptionMemoryImported = false; const persistenceQueue = new SerializedAsyncQueue(); /** Resets module-level state between test runs. */ export function resetComposerDraftsLoadState(): void { loadPromise = null; persistRetryNeeded = false; + legacyModelOptionMemoryImported = false; } function attachmentContextRecord( @@ -627,6 +634,7 @@ export function decodePersistedComposerState(value: unknown): { readonly drafts: Record; readonly stickyModelSelection: ModelSelection | null; readonly modelOptionMemory: ModelOptionMemoryState; + readonly legacyModelOptionMemoryImported?: true; readonly cloudDrafts: ComposerCloudDraftState; } { const parsed = decodePersistedComposerDraftsDocument(value); @@ -666,6 +674,7 @@ export function decodePersistedComposerState(value: unknown): { ), stickyModelSelection: parsed.stickyModelSelection ?? null, modelOptionMemory: parsed.modelOptionMemory ?? {}, + ...(parsed.legacyModelOptionMemoryImported ? { legacyModelOptionMemoryImported: true } : {}), cloudDrafts: { accountId: parsed.cloudAccountId ?? null, signedOut: Object.fromEntries( @@ -687,11 +696,57 @@ export function decodePersistedComposerState(value: unknown): { }; } -async function getComposerDraftsFile() { +async function getComposerDraftsFile(fileName = COMPOSER_DRAFTS_FILE) { const { Directory, File, Paths } = await import("expo-file-system"); const directory = new Directory(Paths.document, COMPOSER_DRAFTS_DIRECTORY); directory.create({ idempotent: true, intermediates: true }); - return new File(directory, COMPOSER_DRAFTS_FILE); + return new File(directory, fileName); +} + +function mergeModelOptionMemory( + persisted: ModelOptionMemoryState, + current: ModelOptionMemoryState, +): ModelOptionMemoryState { + return { + ...persisted, + ...Object.fromEntries( + Object.entries(current).map(([instanceId, models]) => [ + instanceId, + { ...persisted[instanceId], ...models }, + ]), + ), + }; +} + +async function loadLegacyModelOptionMemory(): Promise { + let operation: ComposerDraftPersistenceError["operation"] = "open"; + try { + const file = await getComposerDraftsFile(LEGACY_MODEL_OPTION_MEMORY_FILE); + if (!file.exists) return null; + operation = "read"; + const raw = await file.text(); + operation = "decode"; + const { byInstance } = decodeLegacyModelOptionMemory(JSON.parse(raw) as unknown); + return Object.fromEntries( + Object.entries(byInstance).map(([instanceId, models]) => [ + instanceId, + Object.fromEntries(Object.entries(models).filter(([, options]) => options.length > 0)), + ]), + ); + } catch (cause) { + // Legacy option memory must never prevent saved drafts from hydrating. Leave + // the import unmarked on failure so a later launch can retry the old file. + console.warn( + "[composer-drafts] could not import legacy model options", + new ComposerDraftPersistenceError({ + operation, + directory: COMPOSER_DRAFTS_DIRECTORY, + fileName: LEGACY_MODEL_OPTION_MEMORY_FILE, + cause, + }), + ); + return null; + } } async function loadPersistedComposerState(): Promise< @@ -737,6 +792,7 @@ async function writePersistedComposerState( const document = { schemaVersion: COMPOSER_DRAFTS_SCHEMA_VERSION, drafts: nonEmptyDrafts, + ...(legacyModelOptionMemoryImported ? { legacyModelOptionMemoryImported: true } : {}), ...(stickyModelSelection ? { stickyModelSelection } : {}), ...(Object.keys(appAtomRegistry.get(modelOptionMemoryAtom)).length > 0 ? { modelOptionMemory: appAtomRegistry.get(modelOptionMemoryAtom) } @@ -1028,7 +1084,13 @@ export function ensureComposerDraftsLoaded(): void { if (loadPromise !== null) { return; } - const loading = loadPersistedComposerState().then((persisted) => { + const loading = loadPersistedComposerState().then(async (persisted) => { + const legacyMemory = persisted.legacyModelOptionMemoryImported + ? null + : await loadLegacyModelOptionMemory(); + const persistedMemory = mergeModelOptionMemory(legacyMemory ?? {}, persisted.modelOptionMemory); + legacyModelOptionMemoryImported = + persisted.legacyModelOptionMemoryImported === true || legacyMemory !== null; appAtomRegistry.set(composerCloudDraftsAtom, persisted.cloudDrafts); if (Object.keys(persisted.drafts).length > 0) { const current = appAtomRegistry.get(composerDraftsAtom); @@ -1043,18 +1105,15 @@ export function ensureComposerDraftsLoaded(): void { ) { appAtomRegistry.set(stickyComposerModelSelectionAtom, persisted.stickyModelSelection); } - if (Object.keys(persisted.modelOptionMemory).length > 0) { - const current = appAtomRegistry.get(modelOptionMemoryAtom); - appAtomRegistry.set(modelOptionMemoryAtom, { - ...persisted.modelOptionMemory, - ...Object.fromEntries( - Object.entries(current).map(([instanceId, models]) => [ - instanceId, - { ...(persisted.modelOptionMemory[instanceId] ?? {}), ...models }, - ]), - ), - }); + if (Object.keys(persistedMemory).length > 0) { + appAtomRegistry.set( + modelOptionMemoryAtom, + mergeModelOptionMemory(persistedMemory, appAtomRegistry.get(modelOptionMemoryAtom)), + ); } + // The marker and imported choices land atomically with drafts through the + // existing retryable writer. Never delete or write the legacy file. + if (legacyMemory !== null) schedulePersistComposerState(); }); loadPromise = loading; // Handle fire-and-forget hook loads without swallowing failures from the