fix: keep an unsent row on its own baseline, and let one editor own one delete

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

frontend/src/lib/components/copilot/chat/global/core.test.ts

@@ -342,7 +342,11 @@ import {
 	setListAppRunsHandler,
 	setOpenPreviewHandler
 } from './core'
-import { UserDraft, __resetUserDraftForTesting } from '$lib/userDraft.svelte'
+import {
+	UserDraft,
+	__resetUserDraftForTesting,
+	registerLiveItemBridge
+} from '$lib/userDraft.svelte'
 import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte'
 import {
 	clearGlobalDrafts,
@@ -2306,6 +2310,41 @@ describe('global AI tools', () => {
 		expect(res.status).toBe('conflict')
 	})
 
+	// An open editor owns its own row: the chat must defer to that editor's discard rather than
+	// send a delete of its own, which would be a second unconditional request.
+	it('leaves the delete to a live editor that owns the draft', async () => {
+		const path = 'f/scripts/livedel'
+		seedBackendDraft('script', path, {
+			path,
+			summary: 's',
+			content: 'export function main() {}',
+			language: 'bun'
+		})
+		let discards = 0
+		registerLiveItemBridge({
+			seed: () => false,
+			read: () => undefined,
+			// Claims the key and removes nothing, so anything still gone was deleted by the chat.
+			discard: () => {
+				discards++
+				return Promise.resolve({ removed: true })
+			},
+			list: () => []
+		})
+		try {
+			await deleteGlobalDraft(WORKSPACE, 'script', path)
+			expect(discards).toBe(1)
+			expect(getBackendDraft('script', path)).toBeDefined()
+		} finally {
+			registerLiveItemBridge({
+				seed: () => false,
+				read: () => undefined,
+				discard: () => undefined,
+				list: () => []
+			})
+		}
+	})
+
 	// A failed server delete must surface (throw), not silently report removed —
 	// the same guard the write path got, applied to the delete path.
 	it('deleteGlobalDraft throws when the server delete fails', async () => {

frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts

@@ -587,20 +587,26 @@ export async function deleteGlobalDraft(
 	if (!itemKind) return
 	const storagePath = resolveDraftStoragePath(workspace, itemKind, path)
 	const liveDraft = UserDraft.getLiveEditorDraft(itemKind, { workspace })
-	if (options.preserveLiveDraft && liveDraft?.storagePath === storagePath) {
-		UserDraft.remove(itemKind, storagePath, { workspace })
+	const live =
+		options.preserveLiveDraft && liveDraft?.storagePath === storagePath
+			? UserDraft.remove(itemKind, storagePath, { workspace })
+			: UserDraft.clear(itemKind, storagePath, { workspace })
+	if (live) {
+		// An open editor owns this row and its own discard deletes it, resolving once that has
+		// landed. A delete of ours alongside it would be a second request with nothing to check
+		// against, free to remove a draft saved from elsewhere in between.
+		await live
 	} else {
-		UserDraft.clear(itemKind, storagePath, { workspace })
+		// `remove`/`clear` only debounce the delete; persist it now so a deploy/discard
+		// that the caller awaits has actually cleared the server draft on return.
+		await UserDraftDbSyncer.save({
+			workspace,
+			itemKind,
+			path: storagePath,
+			value: null,
+			immediate: true
+		})
 	}
-	// `remove`/`clear` only debounce the delete; persist it now so a deploy/discard
-	// that the caller awaits has actually cleared the server draft on return.
-	await UserDraftDbSyncer.save({
-		workspace,
-		itemKind,
-		path: storagePath,
-		value: null,
-		immediate: true
-	})
 	// A failed (network/5xx) or conflicted delete is recorded in the syncer state,
 	// not thrown — surface it so callers don't report the draft as removed while
 	// the DB-backed source of truth still has it (same guard as the write path).

frontend/src/lib/itemStore.svelte.ts

@@ -286,7 +286,13 @@ class Entry<V> {
 	 *  `last_sync`: one fact, so taken together and against one `asOf`, the rows sent when the
 	 *  read was issued. Seeding behind a row sent since puts `last_sync` before what the syncer
 	 *  has already sent, which the server refuses from then on. */
-	private adoptRow(key: ItemKey, row: unknown, draftSavedAt: string | undefined, asOf: number) {
+	private adoptRow(
+		key: ItemKey,
+		row: unknown,
+		draftSavedAt: string | undefined,
+		asOf: number,
+		waiting: boolean
+	) {
 		if (this.ports.rowMark(key) !== asOf) return
 		// A payload the server *refused* predates this read and the baseline taken below would
 		// make it acceptable, so it goes; the reconcile after the read re-queues whatever the
@@ -294,7 +300,10 @@ class Entry<V> {
 		// only record of an edit that never reached anyone, so it stays.
 		if (this.ports.conflicted(key)) this.ports.dropPending(key)
 		this.row = row === null || row === undefined ? null : (serialize(row) ?? null)
-		this.ports.seedSync(key, draftSavedAt, asOf)
+		// A payload still waiting to be sent was written against the baseline it had then. Taking
+		// this read's would make it acceptable over whatever has been written since, so it would
+		// overwrite that without ever conflicting. Left alone, its own baseline decides.
+		if (!waiting) this.ports.seedSync(key, draftSavedAt, asOf)
 	}
 
 	private replaceValue(value: V | undefined): void {
@@ -414,7 +423,7 @@ class Entry<V> {
 			const draft = (unsent === null ? undefined : row) as V | undefined
 			// The deployed side above always advances: the item did change, and a discard after
 			// the user keeps theirs has to land on what the server holds now.
-			if (!standOff) this.adoptRow(key, row, res.draftSavedAt, rowsAtRead)
+			if (!standOff) this.adoptRow(key, row, res.draftSavedAt, rowsAtRead, unsent !== undefined)
 			this.loaded = true
 			if (!keepValue) {
 				this.pristine = this.settles && this.origin === 'deployed' && draft === undefined
@@ -1080,11 +1089,16 @@ export function createItemStore(ports: ItemRowPort) {
 			if (!entry || !entry.loaded) return undefined
 			return { value: entry.dirty && entry.origin !== 'new' ? snapshot(entry.value) : undefined }
 		},
-		discard(workspace: string, kind: UserDraftItemKind, path: string): boolean {
+		discard(
+			workspace: string,
+			kind: UserDraftItemKind,
+			path: string
+		): Promise<unknown> | undefined {
 			const entry = find(workspace, kind, path)
-			if (!entry) return false
-			void entry.discard()
-			return true
+			if (!entry) return undefined
+			// Returned rather than left running: the caller would otherwise send its own delete
+			// alongside this one, and the second goes out with no baseline to check.
+			return entry.discard()
 		},
 		list(workspace: string, kinds: readonly UserDraftItemKind[]): UserDraftEntry[] {
 			const out: UserDraftEntry[] = []

frontend/src/lib/itemStore.test.ts

@@ -1251,6 +1251,30 @@ describe('item store: conflicts', () => {
 		})
 	})
 
+	it('does not rebase an unsent edit onto a draft written since', async () => {
+		const rows = fakeRows()
+		const store = createItemStore(rows.port)
+		const key: ItemKey = { workspace: 'w', kind: 'resource', path: 'u/me/r' }
+		// Another tab saved B while our A sat parked, so the server's row is theirs now.
+		const theirs = { ...deployedRes, description: 'saved by another tab' }
+		const a = adapter(async () => ({ deployed: deployedRes, draft: theirs, draftSavedAt: 'T9' }))
+		const first = store.acquire(key, { workspace: 'w', path: 'u/me/r' }, a)
+		await settle()
+		rows.failing.add('u/me/r')
+		first.handle.value = { ...deployedRes, description: 'mine, never sent' }
+		await rows.port.flush(key)
+		first.release()
+
+		const seedsBefore = rows.seeds.length
+		const second = store.acquire(key, { workspace: 'w', path: 'u/me/r' }, a)
+		await settle()
+
+		// Ours is shown, because it is the only copy of it. But taking their timestamp would let
+		// it be accepted over their draft with no conflict ever raised, so it is not taken.
+		expect(second.handle.value?.description).toBe('mine, never sent')
+		expect(rows.seeds.slice(seedsBefore)).toEqual([])
+	})
+
 	it('re-issues a draft delete the server never received, on reopening', async () => {
 		const rows = fakeRows()
 		const store = createItemStore(rows.port)

frontend/src/lib/userDraft.svelte.ts

@@ -141,8 +141,13 @@ export type LiveItemBridge = {
 		itemKind: UserDraftItemKind,
 		path: string
 	): { value: unknown | undefined } | undefined
-	/** Discard the live item's draft. False when no live item holds the key. */
-	discard(workspace: string, itemKind: UserDraftItemKind, path: string): boolean
+	/** Discard the live item's draft, resolving once its delete has landed. `undefined` when no
+	 *  live item holds the key, so the caller issues the delete itself. */
+	discard(
+		workspace: string,
+		itemKind: UserDraftItemKind,
+		path: string
+	): Promise<unknown> | undefined
 	list(workspace: string, itemKinds: readonly UserDraftItemKind[]): UserDraftEntry[]
 }
 
@@ -370,15 +375,20 @@ export const UserDraft = {
 		return writtenCache.get(mk)?.val !== undefined
 	},
 
-	remove(itemKind: UserDraftItemKind, path: string, opts?: UserDraftOptions): void {
+	remove(
+		itemKind: UserDraftItemKind,
+		path: string,
+		opts?: UserDraftOptions
+	): Promise<unknown> | undefined {
 		const ws = resolveWorkspace(opts)
-		if (liveItems?.discard(ws, itemKind, path)) {
+		const live = liveItems?.discard(ws, itemKind, path)
+		if (live) {
 			// The live item owns the row, and its own discard deletes it. Falling through to the
 			// POST below would race that delete with an unconditional one (see `forgetLocal`), so
 			// only this caller's mirror is dropped — otherwise its reads keep answering with a
 			// draft that is gone.
 			UserDraft.forgetLocal(itemKind, path, opts)
-			return
+			return live
 		}
 		const mk = mapKey(ws, itemKind, path)
 		const entry = entries.get(mk)
@@ -418,8 +428,12 @@ export const UserDraft = {
 		writtenCache.delete(mk)
 	},
 
-	clear(itemKind: UserDraftItemKind, path: string, opts?: UserDraftOptions): void {
-		UserDraft.discard(itemKind, path, undefined, opts)
+	clear(
+		itemKind: UserDraftItemKind,
+		path: string,
+		opts?: UserDraftOptions
+	): Promise<unknown> | undefined {
+		return UserDraft.discard(itemKind, path, undefined, opts)
 	},
 
 	/**
@@ -568,12 +582,13 @@ export const UserDraft = {
 			 * leave it unset. */
 			auto?: boolean
 		}
-	): void {
+	): Promise<unknown> | undefined {
 		const ws = resolveWorkspace(opts)
 		const mk = mapKey(ws, itemKind, path)
 		const entry = entries.get(mk)
 		const safeFallback = snapshotDraftValue(fallback)
-		if (liveItems?.discard(ws, itemKind, path)) {
+		const live = liveItems?.discard(ws, itemKind, path)
+		if (live) {
 			// No POST, for the reason `remove` gives. The cell still has to reach `fallback`:
 			// a caller holding one resets it to what it just saved, and its apply-effect would
 			// otherwise copy the stale draft straight back over the form.
@@ -582,7 +597,7 @@ export const UserDraft = {
 				entry.state.val = safeFallback
 			}
 			writtenCache.delete(mk)
-			return
+			return live
 		}
 		if (entry) {
 			entry.skipNextSync = true