fix: hold a waiting row across a read, guard learn, keep a kept draft masked

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

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

@@ -6608,8 +6608,10 @@ async function discardLocalDraft(
 	const discarded = await deleteGlobalDraft(workspace, type, path, triggerKind)
 
 	// The chat's touch on the item is undone — drop it from the mask so a
-	// pre-existing deployed item doesn't keep reading as this chat's edit.
-	const discardedKind = itemKindFor(type, triggerKind)
+	// pre-existing deployed item doesn't keep reading as this chat's edit. Only when the draft
+	// actually went: one an editor kept is still this chat's edit, and dropping it would leave it
+	// out of Compare & Deploy.
+	const discardedKind = discarded.removed ? itemKindFor(type, triggerKind) : undefined
 	if (discardedKind) {
 		toolCallbacks.onItemDiscarded?.(
 			discardedKind,

frontend/src/lib/itemStore.svelte.ts

@@ -412,6 +412,7 @@ class Entry<V> {
 			let rowsAtRead = 0
 			let valuesAtRead = 0
 			let unbasedPending = false
+			let waitingBefore: unknown | undefined
 			let res: ItemLoad<V>
 			try {
 				if (this.retired) return superseded
@@ -431,6 +432,11 @@ class Entry<V> {
 				// above already has its row on this side of the count, and crediting it again
 				// would pay for a row that arrives later and belongs to nobody here.
 				valuesAtRead = this.values
+				// Taken before the read, not after: a row waiting here may land while the read is
+				// out, and then the response predates it while `pending` has gone quiet. Read
+				// afterwards, this entry would answer with the older value and reseed over the
+				// baseline that row's own save had just set.
+				waitingBefore = this.ports.pending(key)
 				res = await adapter.load(key)
 			} catch (e) {
 				this.error = errorMessage(e)
@@ -466,7 +472,7 @@ class Entry<V> {
 			// A row the syncer took but could not send is newer than this response and is the only
 			// copy of that edit, so the item opens on it. One the server *refused* is the other
 			// way round — the response is what won — and `adoptRow` drops it below.
-			const unsent = this.ports.conflicted(key) ? undefined : this.ports.pending(key)
+			const unsent = this.ports.conflicted(key) ? undefined : waitingBefore
 			// A parked delete is what this tab wants gone, not what is there: the row is still the
 			// server's, so the reconcile at the end re-issues the delete instead of believing it
 			// landed and leaving the draft behind with nothing to remove it.
@@ -837,7 +843,7 @@ class Entry<V> {
 	/** Fetch something about the deployed item the load did not carry (a secret's plaintext),
 	 *  and write it into both sides. */
 	learn(fetch: (key: ItemKey) => Promise<(side: V) => void>): Promise<CommandOutcome> {
-		return this.run(async () => {
+		return this.run(async (at) => {
 			let apply: (side: V) => void
 			try {
 				apply = await fetch(this.key)
@@ -849,7 +855,12 @@ class Entry<V> {
 				apply(d)
 				this.serverDeployed = d
 			}
-			if (this.value !== undefined) {
+			// What it learned is about the deployed item, so that side takes it either way. The
+			// value is the user's, and anything written there while the fetch was out is newer
+			// than this: folding a decrypted secret from before into it would put the old one
+			// back and autosave it.
+			const moved = this.edits !== at.edits || this.externals !== at.externals
+			if (this.value !== undefined && !moved) {
 				const v = snapshot(this.value)
 				apply(v)
 				this.replaceValue(v)

frontend/src/lib/itemStore.test.ts

@@ -90,8 +90,16 @@ function fakeRows() {
 			handed(key.path)
 		},
 		rowMark: (key) => sends.get(key.path) ?? 0,
-		pending: (key) => (parked.has(key.path) ? parked.get(key.path) : undefined),
-		unbased: (key) => parked.has(key.path) && !baselines.has(key.path),
+		// `pendingSaveOpts` holds the latest payload from the moment it is handed over, so a row
+		// still in the debouncer counts as waiting just as much as one the server refused.
+		pending: (key) =>
+			queued.has(key.path)
+				? queued.get(key.path)
+				: parked.has(key.path)
+					? parked.get(key.path)
+					: undefined,
+		unbased: (key) =>
+			(queued.has(key.path) || parked.has(key.path)) && !baselines.has(key.path),
 		markConflict: (key) => void conflicts.add(key.path),
 		seedSync: (key, at, since) => {
 			// The syncer refuses a baseline from a read a row handed over since has passed.
@@ -109,7 +117,10 @@ function fakeRows() {
 		failure: (key) => failures.get(key.path),
 		dropPending: (key) => {
 			dropped.push(key.path)
+			// Cancels the debounce as well as forgetting the refused payload: both live in
+			// `pendingSaveOpts`, and leaving the queued one would resurrect what was dropped.
 			parked.delete(key.path)
+			queued.delete(key.path)
 		},
 		hint: (key, on) => void hints.set(key.path, on)
 	}
@@ -1829,6 +1840,58 @@ describe('item store: conflicts', () => {
 		expect(open.deployed).toEqual({ ...b, args: { a: 2 } })
 	})
 
+	it('does not fold a secret it fetched over a value written since', async () => {
+		type Var = { path: string; value: string; is_secret: boolean }
+		const rows = fakeRows()
+		const store = createItemStore(rows.port)
+		const decrypting = deferred<(side: Var) => void>()
+		const { handle: item } = store.acquire(
+			{ workspace: 'w', kind: 'variable', path: 'u/me/v' },
+			{ workspace: 'w', path: 'u/me/v' },
+			{
+				load: async () => ({
+					deployed: { path: 'u/me/v', value: '$encrypted:xx', is_secret: true }
+				})
+			} as ItemAdapter<Var>
+		)
+		await settle()
+
+		const learning = item.learn(() => decrypting.promise)
+		await settle()
+		// The chat writes a newer secret while the decryption request is out.
+		item.applyExternal({ path: 'u/me/v', value: 'the new secret', is_secret: true })
+		decrypting.resolve((side) => void (side.value = 'the old secret'))
+
+		expect(await learning).toEqual({ ok: true })
+		// What it decrypted is the deployed secret from before that write. Folding it into the
+		// value would put the old one back on screen and autosave it over the new one.
+		expect(item.value?.value).toBe('the new secret')
+		expect(item.deployed?.value).toBe('the old secret')
+	})
+
+	it('keeps a row that landed while its reopening read was in flight', async () => {
+		const rows = fakeRows()
+		const read = deferred<ItemLoad<Res>>()
+		const store = createItemStore(rows.port)
+		const key: ItemKey = { workspace: 'w', kind: 'resource', path: 'u/me/r' }
+		const mine = { ...deployedRes, description: 'queued, not yet sent' }
+		const a = adapter(() => read.promise)
+		// A first autosave is queued and unsent, so the key has no baseline yet.
+		rows.port.write(key, mine)
+		const acq = store.acquire(key, { workspace: 'w', path: 'u/me/r' }, a)
+		await settle()
+		// Its debounce fires and lands while the reopening GET is still out.
+		await rows.port.flush(key)
+		// The GET predates that, so it reports no draft at all.
+		read.resolve({ deployed: deployedRes })
+		await settle()
+
+		// Answering with the deployed value would hide the draft that just landed and reseed the
+		// baseline its own save had set. What was waiting when the read went out is what stands.
+		expect(acq.handle.value).toEqual(mine)
+		expect(rows.seeds).toEqual([])
+	})
+
 	it('does not replay a first failed autosave over a draft created since', async () => {
 		const rows = fakeRows()
 		const store = createItemStore(rows.port)