diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts index 6826f496f4..0a0c9ba4dd 100644 --- a/frontend/src/lib/userDraft.svelte.ts +++ b/frontend/src/lib/userDraft.svelte.ts @@ -237,6 +237,9 @@ const DRAFT_COMPARE_IGNORED_FIELDS = [ // Computed at read time from the parent folder; edited on the folder, not // here. `labels` itself IS editable and stays compared. 'inherited_labels', + // Fixed at creation and absent from the resource editor's draft shape, so + // it only ever shows up on the deployed side of a comparison. + 'resource_type', // A resource's OAuth/linked-secret state: owned by the OAuth flow and the // variable it points at, never by the resource form. 'is_oauth', diff --git a/frontend/src/lib/userDraftPrune.test.ts b/frontend/src/lib/userDraftPrune.test.ts new file mode 100644 index 0000000000..1fd260c9cc --- /dev/null +++ b/frontend/src/lib/userDraftPrune.test.ts @@ -0,0 +1,111 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' + +// The sweep DELETES drafts, so the guard that matters is which rows it picks. +// Stub the two collaborators it decides from — the draft listing and the +// per-kind diff — and assert on what it discards. +const listDrafts = vi.fn() +const getDraftDiffValues = vi.fn() +const discardDraft = vi.fn(async () => ({ success: true })) + +vi.mock('./gen', () => ({ + DraftService: { listDrafts: (...a: unknown[]) => listDrafts(...(a as [])) } +})) +vi.mock('./utils_draft_deploy', () => ({ + getDraftDiffValues: (...a: unknown[]) => getDraftDiffValues(...(a as [])), + discardDraft: (...a: unknown[]) => discardDraft(...(a as [])) +})) +vi.mock('./workspaceDrafts.svelte', () => ({ invalidateWorkspaceDrafts: vi.fn() })) +vi.mock('./toast', () => ({ sendUserToast: vi.fn() })) +vi.mock('./userDraftDbSyncer.svelte', () => ({ UserDraftDbSyncer: { save: vi.fn() } })) + +import { pruneMeaninglessDrafts } from './userDraftPrune' + +const row = (over: Record = {}) => ({ + kind: 'resource', + path: 'u/me/r', + draft_only: false, + legacy_draft: false, + mine: true, + can_write: true, + created_at: '', + ...over +}) +const diff = (over: Record = {}) => ({ + deployed: { value: { host: 'h' } }, + draft: { value: { host: 'h' } }, + hasDraft: true, + noDeployed: false, + ...over +}) +const discardedPaths = () => discardDraft.mock.calls.map((c: any[]) => c[1]) + +beforeEach(() => { + localStorage.clear() + vi.clearAllMocks() + discardDraft.mockResolvedValue({ success: true }) +}) + +describe('pruneMeaninglessDrafts', () => { + it('discards a draft whose diff against the deployed value is empty', async () => { + listDrafts.mockResolvedValue([row()]) + getDraftDiffValues.mockResolvedValue(diff()) + await pruneMeaninglessDrafts('main', 'me@x.dev') + expect(discardedPaths()).toEqual(['u/me/r']) + }) + + it('keeps a draft that carries a real change', async () => { + listDrafts.mockResolvedValue([row()]) + getDraftDiffValues.mockResolvedValue(diff({ draft: { value: { host: 'other' } } })) + await pruneMeaninglessDrafts('main', 'me@x.dev') + expect(discardDraft).not.toHaveBeenCalled() + }) + + it('ignores the empty fields a moved-on schema added', async () => { + listDrafts.mockResolvedValue([row()]) + getDraftDiffValues.mockResolvedValue( + diff({ draft: { value: { host: 'h', port: '', ssl: false, tags: [] } } }) + ) + await pruneMeaninglessDrafts('main', 'me@x.dev') + expect(discardedPaths()).toEqual(['u/me/r']) + }) + + it('never touches a draft-only item — the draft is the whole item', async () => { + listDrafts.mockResolvedValue([row({ draft_only: true })]) + getDraftDiffValues.mockResolvedValue(diff()) + await pruneMeaninglessDrafts('main', 'me@x.dev') + expect(discardDraft).not.toHaveBeenCalled() + }) + + it('never touches another user’s row, or one it cannot write', async () => { + listDrafts.mockResolvedValue([row({ mine: false }), row({ path: 'u/me/b', can_write: false })]) + getDraftDiffValues.mockResolvedValue(diff()) + await pruneMeaninglessDrafts('main', 'me@x.dev') + expect(discardDraft).not.toHaveBeenCalled() + }) + + it('leaves a draft alone when its diff cannot be fetched', async () => { + listDrafts.mockResolvedValue([row()]) + getDraftDiffValues.mockRejectedValue(new Error('boom')) + await pruneMeaninglessDrafts('main', 'me@x.dev') + expect(discardDraft).not.toHaveBeenCalled() + }) + + it('runs once per workspace and user', async () => { + listDrafts.mockResolvedValue([row()]) + getDraftDiffValues.mockResolvedValue(diff()) + await pruneMeaninglessDrafts('main', 'me@x.dev') + await pruneMeaninglessDrafts('main', 'me@x.dev') + expect(discardDraft).toHaveBeenCalledTimes(1) + await pruneMeaninglessDrafts('other', 'me@x.dev') + expect(discardDraft).toHaveBeenCalledTimes(2) + }) + + it('retries next mount when the listing failed', async () => { + listDrafts.mockRejectedValueOnce(new Error('offline')) + await pruneMeaninglessDrafts('main', 'me@x.dev') + listDrafts.mockResolvedValue([row()]) + getDraftDiffValues.mockResolvedValue(diff()) + await pruneMeaninglessDrafts('main', 'me@x.dev') + expect(discardedPaths()).toEqual(['u/me/r']) + }) +}) diff --git a/frontend/src/lib/userDraftPrune.ts b/frontend/src/lib/userDraftPrune.ts new file mode 100644 index 0000000000..fa2fec3edd --- /dev/null +++ b/frontend/src/lib/userDraftPrune.ts @@ -0,0 +1,122 @@ +/** + * One-off sweep that drops drafts carrying no changes. + * + * Before the draft comparison learned to ignore empty schema-added fields and + * server-managed metadata (see `normalizeDraftForCompare`), merely opening an + * item whose schema had moved on saved a draft — workspaces accumulated dozens + * that nobody wrote. New ones no longer appear; the ones already stored need + * this pass to clear. Runs once per (workspace, user) per browser, after the + * localStorage→DB migration so anything it just uploaded is swept too. + * + * A draft is dropped only when the diff the user would be shown is empty: both + * sides come from `getDraftDiffValues`, the same canonicalization the diff + * drawer renders, compared with the same `draftValuesEqual` the editors use. + * Anything that can't be established is left alone — a `draft_only` item (no + * deployed counterpart, so discarding would destroy the item itself), a kind + * with no diff support, a failed fetch. + */ + +import { DraftService } from './gen' +import type { UserDraftItemKind } from './gen' +import { sendUserToast } from './toast' +import { draftValuesEqual } from './userDraft.svelte' +import { discardDraft, getDraftDiffValues } from './utils_draft_deploy' +import { invalidateWorkspaceDrafts } from './workspaceDrafts.svelte' + +const SENTINEL_PREFIX = 'userdraft/pruned/v1/' + +/** Overlay GETs are one round trip each and a cluttered workspace has dozens; + * a small window keeps the sweep off the critical path of a fresh login. */ +const CONCURRENCY = 4 + +/** Guards against the layout effect firing again before the sentinel lands. */ +const inFlight = new Set() + +type Candidate = { + kind: UserDraftItemKind + path: string + legacy: boolean +} + +async function carriesNoChanges(workspace: string, { kind, path }: Candidate): Promise { + try { + const { deployed, draft, hasDraft, noDeployed } = await getDraftDiffValues( + kind, + path, + workspace + ) + // `hasDraft` false means the overlay had no draft row and the item's own + // value stood in for the draft side — there is nothing to discard, and the + // two sides would compare equal by construction. + if (!hasDraft || noDeployed) return false + return draftValuesEqual(draft, deployed) + } catch { + return false + } +} + +async function mapWithLimit( + items: T[], + limit: number, + fn: (item: T) => Promise +): Promise { + const out = new Array(items.length) + let next = 0 + await Promise.all( + Array.from({ length: Math.min(limit, items.length) }, async () => { + while (next < items.length) { + const i = next++ + out[i] = await fn(items[i]) + } + }) + ) + return out +} + +export async function pruneMeaninglessDrafts(workspace: string, userKey: string): Promise { + if (typeof localStorage === 'undefined') return + const sentinel = `${SENTINEL_PREFIX}${workspace}/${userKey}` + if (inFlight.has(sentinel)) return + try { + if (localStorage.getItem(sentinel)) return + } catch { + // Storage unavailable (private mode): the sweep can't record that it ran, + // and re-running it on every mount would cost an overlay GET per draft. + return + } + inFlight.add(sentinel) + try { + const rows = await DraftService.listDrafts({ workspace }) + const candidates: Candidate[] = rows + // `draft_only` rows ARE the item; `mine` / `can_write` are the same + // gate the discard endpoint enforces, so anything else would 403. + .filter((r) => !r.draft_only && r.mine && r.can_write) + .map((r) => ({ kind: r.kind, path: r.path, legacy: r.legacy_draft })) + + const empty: Candidate[] = [] + await mapWithLimit(candidates, CONCURRENCY, async (c) => { + if (await carriesNoChanges(workspace, c)) empty.push(c) + }) + + let discarded = 0 + for (const c of empty) { + const res = await discardDraft(c.kind, c.path, workspace, false, c.legacy, false) + if (res.success) discarded++ + } + if (discarded > 0) { + invalidateWorkspaceDrafts(workspace) + sendUserToast(`Cleared ${discarded} draft${discarded > 1 ? 's' : ''} that carried no changes`) + } + // Only after a completed pass: a run that threw retries on the next mount. + try { + localStorage.setItem(sentinel, new Date().toISOString()) + } catch { + // Nothing to do — the pass is idempotent, it just runs again. + } + } catch { + // Fire-and-forget from the layout: a workspace whose draft list can't be + // read is left exactly as it was. + } finally { + inFlight.delete(sentinel) + } +} diff --git a/frontend/src/routes/(root)/(logged)/+layout.svelte b/frontend/src/routes/(root)/(logged)/+layout.svelte index 257af0fbbd..04e21c41f2 100644 --- a/frontend/src/routes/(root)/(logged)/+layout.svelte +++ b/frontend/src/routes/(root)/(logged)/+layout.svelte @@ -74,6 +74,7 @@ import { createUsageResources, registerUsageResources } from '$lib/usage.svelte' import { purgeLegacyUserDrafts } from '$lib/userDraftLegacyMigration' import { migrateUserDraftsToDb } from '$lib/userDraftDbMigration' + import { pruneMeaninglessDrafts } from '$lib/userDraftPrune' import DraftMigrationErrorModal from '$lib/components/DraftMigrationErrorModal.svelte' import { onDestroy, setContext, untrack } from 'svelte' import { base } from '$app/paths' @@ -787,12 +788,16 @@ // drafts). `migrateUserDraftsToDb` then pushes the workspace-scoped // `userdraft/w/{ws}/{kind}/{path}` keys — written by the editor with the // correct workspace — onto the server-side draft table, clearing LS on - // success. + // success. `pruneMeaninglessDrafts` then clears the drafts an older, stricter + // comparison saved for changes nobody made; it runs after the upload so the + // entries that just landed are swept in the same pass. $effect(() => { - if ($workspaceStore && $userStore) { + const ws = $workspaceStore + const email = $userStore?.email + if (ws && email) { untrack(() => { purgeLegacyUserDrafts() - void migrateUserDraftsToDb() + void migrateUserDraftsToDb().then(() => pruneMeaninglessDrafts(ws, email)) }) } })