fix: resolve a draft's key once per operation and pass it down

Review round 7:

- Deleting a deployed item removes the reason its path outranks a draft staged
  under that name, so a cleanup that resolved afterwards deleted that
  unrelated draft. The key is resolved while the deployed item is still there,
  and both cleanups now pass it rather than let the delete resolve again.
- rebase_draft probed the deployed item at the name it was addressed by, so a
  renamed draft rebased against a path where nothing is deployed and the tool
  reported no deployed version. All three kinds read the storage path.
- A write resolved its merge base and its destination separately, and only the
  destination failed closed, so a listing that failed in between based the
  merge on the deployed item and wrote that over the draft. Writes resolve
  once and thread that key through base, merge and persist.

`readGlobalDraftValue` has no callers left and is replaced by
`readGlobalDraftValueAt`, which takes the key its caller resolved.

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

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

@@ -395,7 +395,7 @@ import {
 	deleteGlobalDraft,
 	listGlobalDrafts,
 	persistGlobalDraft,
-	readGlobalDraftValue,
+	readGlobalDraftValueAt,
 	saveGlobalAppDraft
 } from './userDraftAdapter'
 import { bundleRawAppDraft } from './rawAppBundlerBridge'
@@ -2590,6 +2590,37 @@ describe('global AI tools', () => {
 		}
 	})
 
+	// Deleting the deployed item removes the reason its path outranks a draft staged under
+	// that name, so a cleanup resolved afterwards would delete that unrelated draft.
+	it('leaves alone a draft staged under a deleted item\'s path', async () => {
+		seedBackendDraft(
+			'raw_app',
+			'u/admin/draft_namesake',
+			{ summary: 'Namesake', draft_path: 'f/sales/deleted_app', files: {}, runnables: {} },
+			{ workspace: WORKSPACE }
+		)
+		const deletedApps = new Set<string>()
+		vi.mocked(AppService.existsApp).mockImplementation(
+			async ({ path }) => path === 'f/sales/deleted_app' && !deletedApps.has(path)
+		)
+		const deleteApp = vi
+			.spyOn(AppService, 'deleteApp')
+			.mockImplementation(async ({ path }: any) => {
+				deletedApps.add(path)
+				return 'deleted' as any
+			})
+
+		try {
+			await callGlobalTool('delete_workspace_item', { type: 'app', path: 'f/sales/deleted_app' })
+			expect(
+				getBackendDraft('raw_app', 'u/admin/draft_namesake', { workspace: WORKSPACE })
+			).toBeDefined()
+		} finally {
+			vi.mocked(AppService.existsApp).mockImplementation(async () => false)
+			deleteApp.mockRestore()
+		}
+	})
+
 	it('refuses a draft_path that two drafts are staged under', async () => {
 		for (const storage of ['u/admin/draft_a', 'u/admin/draft_b']) {
 			seedBackendDraft(
@@ -2897,7 +2928,7 @@ describe('global AI tools', () => {
 	// directly: a non-force save whose recorded baseline is older than the
 	// server row is rejected with `status:'conflict'`, and `override` (force)
 	// pushes our version through. NB: this targets persistGlobalDraft, not the
-	// write_* tools — those re-read the backend first (readGlobalDraftValue ->
+	// write_* tools — those re-read the backend first (readGlobalDraftValueAt ->
 	// recordRemoteSync), which re-seeds the baseline and so can only surface a
 	// conflict when a live editor cell is mounted (not the case in unit tests).
 	it('persistGlobalDraft surfaces a conflict on a stale baseline and override forces it', async () => {
@@ -2962,7 +2993,7 @@ describe('global AI tools', () => {
 	it('a non-404 backend read failure propagates instead of returning undefined', async () => {
 		const path = 'f/scripts/readfail'
 		failingReads.add(`script:${path}`)
-		await expect(readGlobalDraftValue(WORKSPACE, 'script', path)).rejects.toThrow()
+		await expect(readGlobalDraftValueAt(WORKSPACE, 'script', path)).rejects.toThrow()
 	})
 
 	// Raw-app writes go through saveGlobalAppDraft, which must carry the conflict

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

@@ -228,9 +228,10 @@ import {
 	chosenDraftName,
 	listGlobalDrafts,
 	liveGlobalDraftStoragePath,
+	readGlobalDraftValueAt,
+	resolveGlobalDraft,
 	resolveGlobalDraftStoragePath,
 	persistGlobalDraft,
-	readGlobalDraftValue,
 	readLocalDraftCellByKind,
 	resolveGlobalDraftStoragePathByKind,
 	saveGlobalAppDraft,
@@ -5130,7 +5131,16 @@ async function writeDraft<T, A>(
 	const { workspace } = ctx
 	startDraftWrite(ctx, type, path)
 
-	const existingDraft = await readGlobalDraftValue<T>(workspace, type, path, opts.triggerKind)
+	// One resolution for the merge base and the write: resolving twice lets a listing that
+	// fails in between base the merge on the deployed item and write that over the draft.
+	const resolved = await resolveGlobalDraft(workspace, type, path, opts.triggerKind, {
+		forWrite: true
+	})
+	const existingDraft = (
+		resolved.fetched
+			? resolved.value
+			: await readGlobalDraftValueAt<T>(workspace, type, resolved.storagePath, opts.triggerKind)
+	) as T | undefined
 	let base = existingDraft
 	let existed = existingDraft !== undefined
 	if (base === undefined && (await spec.probe(workspace, path))) {
@@ -5142,7 +5152,8 @@ async function writeDraft<T, A>(
 
 	const result = await persistGlobalDraft(workspace, type, path, draft, {
 		triggerKind: opts.triggerKind,
-		force: opts.override
+		force: opts.override,
+		storagePath: resolved.storagePath
 	})
 	return finishDraftWrite(result, existed, ctx)
 }
@@ -6911,11 +6922,14 @@ async function rebaseScriptDraft(path: string, ctx: WriteDraftCtx): Promise<stri
 	if (!draft || typeof draft.value !== 'string' || !draft.language) {
 		throw new Error(`No script draft found for "${path}".`)
 	}
-	if (!(await ScriptService.existsScriptByPath({ workspace, path }))) {
+	// The deployed version a draft is based on sits at the path it is stored at: a rename
+	// is staged over the old path, and nothing is deployed yet at the new name.
+	const basePath = await resolveGlobalDraftStoragePath(workspace, 'script', path)
+	if (!(await ScriptService.existsScriptByPath({ workspace, path: basePath }))) {
 		throw new Error(`Script "${path}" is not deployed; there is no newer version to rebase onto.`)
 	}
 
-	const latest = await ScriptService.getScriptByPath({ workspace, path })
+	const latest = await ScriptService.getScriptByPath({ workspace, path: basePath })
 	const baseHash = draft.parentHash
 	if (baseHash && baseHash === latest.hash) {
 		const message = `Draft "${path}" is already based on the latest deployed version (${latest.hash}).`
@@ -6976,11 +6990,12 @@ async function rebaseFlowDraft(path: string, ctx: WriteDraftCtx): Promise<string
 	if (!draft || draft.value === undefined || typeof draft.value === 'string') {
 		throw new Error(`No flow draft found for "${path}".`)
 	}
-	if (!(await FlowService.existsFlowByPath({ workspace, path }))) {
+	const basePath = await resolveGlobalDraftStoragePath(workspace, 'flow', path)
+	if (!(await FlowService.existsFlowByPath({ workspace, path: basePath }))) {
 		throw new Error(`Flow "${path}" is not deployed; there is no newer version to rebase onto.`)
 	}
 
-	const latest = await FlowService.getFlowByPath({ workspace, path })
+	const latest = await FlowService.getFlowByPath({ workspace, path: basePath })
 	const baseVersion = draft.parentVersionId
 	if (baseVersion != null && baseVersion === latest.version_id) {
 		const message = `Draft "${path}" is already based on the latest deployed version (${latest.version_id}).`
@@ -7049,11 +7064,12 @@ async function rebaseAppDraft(path: string, ctx: WriteDraftCtx): Promise<string>
 	if (!draft || !draft.value || typeof draft.value === 'string' || !('files' in draft.value)) {
 		throw new Error(`No app draft found for "${path}".`)
 	}
-	if (!(await AppService.existsApp({ workspace, path }))) {
+	const basePath = await resolveGlobalDraftStoragePath(workspace, 'app', path)
+	if (!(await AppService.existsApp({ workspace, path: basePath }))) {
 		throw new Error(`App "${path}" is not deployed; there is no newer version to rebase onto.`)
 	}
 
-	const deployed = await AppService.getAppByPath({ workspace, path })
+	const deployed = await AppService.getAppByPath({ workspace, path: basePath })
 	const headVersion = deployed.versions?.[deployed.versions.length - 1]
 	const baseVersion = draft.parentVersionId
 	if (baseVersion != null && baseVersion === headVersion) {
@@ -8014,7 +8030,8 @@ async function deployDraft(
 		// back, so read it here — post-flush, since a rename may have been parked. This is
 		// where a draft-only or renamed item lands, and what the callbacks below must name.
 		deployedPath =
-			chosenDraftName(type, await readGlobalDraftValue(workspace, type, storagePath)) ?? storagePath
+			chosenDraftName(type, await readGlobalDraftValueAt(workspace, type, storagePath)) ??
+			storagePath
 		const result = await deployDraftToWorkspace(type, storagePath, workspace, {
 			draftOnly,
 			deploymentMessage
@@ -8276,8 +8293,9 @@ async function deployDraft(
 
 	// By the path resolved before the deploy: the app now deployed at the draft's chosen
 	// name would otherwise win the name back, and the draft would be left behind.
-	await deleteGlobalDraft(workspace, type, draftStoragePath, triggerKind, {
-		preserveLiveDraft: true
+	await deleteGlobalDraft(workspace, type, path, triggerKind, {
+		preserveLiveDraft: true,
+		storagePath: draftStoragePath
 	})
 
 	// Move the chat's mask entry to the deployed path: a draft-only item's
@@ -8427,6 +8445,11 @@ async function deleteWorkspaceItem(
 		throw new Error('trigger_kind is required when deleting a trigger.')
 	}
 
+	// Resolved while the deployed item is still there: once it is gone, this path could
+	// resolve to another item's draft staged under its name, and the cleanup below would
+	// delete that instead.
+	const draftStoragePath = await resolveGlobalDraftStoragePath(workspace, type, path, triggerKind)
+
 	toolCallbacks.setToolStatus(toolId, {
 		content: `Deleting ${type} "${path}"...`
 	})
@@ -8459,17 +8482,14 @@ async function deleteWorkspaceItem(
 	// are no longer trustworthy (same rule as deploy success). Before the
 	// draft cleanup: a cleanup failure must not leave stale comparisons.
 	invalidateWorkspaceComparison(workspace)
-	await deleteGlobalDraft(workspace, type, path, triggerKind)
+	await deleteGlobalDraft(workspace, type, path, triggerKind, { storagePath: draftStoragePath })
 
 	// Record the deletion in the chat's modified-items mask. In a fork this leaves a
 	// reviewable "removed" diff vs the parent that stays scoped to this chat. Keyed
 	// by the same (itemKind, storagePath) as writes so it joins the draft/fork lists.
 	const deletedKind = itemKindFor(type, triggerKind)
 	if (deletedKind) {
-		toolCallbacks.onItemModified?.(
-			deletedKind,
-			liveGlobalDraftStoragePath(workspace, type, path, triggerKind)
-		)
+		toolCallbacks.onItemModified?.(deletedKind, draftStoragePath)
 	}
 
 	toolCallbacks.setToolStatus(toolId, {

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

@@ -428,6 +428,29 @@ export function liveGlobalDraftStoragePath(
 	return itemKind ? liveEditorStoragePath(workspace, itemKind, path) : path
 }
 
+/** One resolution for a whole operation, so a tool reads its merge base, writes and
+ * cleans up at the same key. Resolving twice lets a listing that fails in between send
+ * the two halves to different drafts. `value` is the draft found while resolving, if any;
+ * `fetched` says the backend was already asked, so no caller asks again. */
+export async function resolveGlobalDraft(
+	workspace: string,
+	type: WorkspaceItemType,
+	path: string,
+	triggerKind?: TriggerKind,
+	opts: { forWrite?: boolean } = {}
+): Promise<{ storagePath: string; value?: unknown; fetched: boolean }> {
+	const itemKind = itemKindFor(type, triggerKind)
+	if (!itemKind) return { storagePath: path, fetched: false }
+	const resolved = await resolveDraft(workspace, itemKind, path, opts)
+	const cell = UserDraft.get(itemKind, resolved.storagePath, { workspace })
+	return {
+		storagePath: resolved.storagePath,
+		value: cell ?? resolved.value,
+		// Only the path it probed was fetched; a name resolved elsewhere was not.
+		fetched: cell !== undefined || resolved.storagePath === path
+	}
+}
+
 /** Where the draft a model-supplied path names is stored. */
 export async function resolveGlobalDraftStoragePath(
 	workspace: string,
@@ -490,23 +513,19 @@ async function fetchBackendDraftValue(
 	return resp.value ?? undefined
 }
 
-// Draft VALUE for a write merge: cell-if-present (the user's freshest in-tab
-// edits) else the current user's backend draft.
-export async function readGlobalDraftValue<V>(
+/** Draft VALUE at a key already resolved: cell-if-present (the user's freshest in-tab
+ * edits) else the current user's backend draft. */
+export async function readGlobalDraftValueAt<V>(
 	workspace: string,
 	type: WorkspaceItemType,
-	path: string,
+	storagePath: string,
 	triggerKind?: TriggerKind
 ): Promise<V | undefined> {
 	const itemKind = itemKindFor(type, triggerKind)
 	if (!itemKind) return undefined
-	const resolved = await resolveDraft(workspace, itemKind, path)
-	// The cell at the RESOLVED path: it is the freshest state when a save is parked, failed
-	// or conflicted, and a draft that exists only as a cell has no backend row behind it.
-	const cell = UserDraft.get<V>(itemKind, resolved.storagePath, { workspace })
+	const cell = UserDraft.get<V>(itemKind, storagePath, { workspace })
 	if (cell !== undefined) return cell
-	if (resolved.value !== undefined) return resolved.value as V
-	return (await fetchBackendDraftValue(workspace, itemKind, resolved.storagePath)) as V | undefined
+	return (await fetchBackendDraftValue(workspace, itemKind, storagePath)) as V | undefined
 }
 
 // `itemKind` + `storagePath` are the canonical identity of the persisted draft
@@ -538,14 +557,16 @@ export async function persistGlobalDraft(
 	type: WorkspaceItemType,
 	path: string,
 	value: unknown,
-	opts: { triggerKind?: TriggerKind; force?: boolean } = {}
+	opts: { triggerKind?: TriggerKind; force?: boolean; storagePath?: string } = {}
 ): Promise<DraftPersistResult> {
 	const itemKind = itemKindFor(type, opts.triggerKind)
 	if (!itemKind) throw new Error(`Unsupported draft type "${type}".`)
 	// Resolved rather than taken as given: a write under a chosen name that did not resolve
-	// would create a second draft beside the one it meant to edit.
-	const storagePath = (await resolveDraft(workspace, itemKind, path, { forWrite: true }))
-		.storagePath
+	// would create a second draft beside the one it meant to edit. A caller that resolved
+	// for the whole operation passes its key so both halves land on one draft.
+	const storagePath =
+		opts.storagePath ??
+		(await resolveDraft(workspace, itemKind, path, { forWrite: true })).storagePath
 	UserDraft.seed(itemKind, storagePath, value, { workspace })
 	await UserDraftDbSyncer.save({
 		workspace,
@@ -706,6 +727,10 @@ export async function saveGlobalAppDraft(
 
 type DeleteGlobalDraftOptions = {
 	preserveLiveDraft?: boolean
+	/** Key resolved by the caller, for a cleanup whose own resolution would now answer
+	 * differently — after a deploy created the item at the draft's chosen name, or after
+	 * the deployed item that outranked that name was deleted. */
+	storagePath?: string
 }
 
 export async function deleteGlobalDraft(
@@ -717,7 +742,8 @@ export async function deleteGlobalDraft(
 ): Promise<void> {
 	const itemKind = itemKindFor(type, triggerKind)
 	if (!itemKind) return
-	const storagePath = (await resolveDraft(workspace, itemKind, path)).storagePath
+	const storagePath =
+		options.storagePath ?? (await resolveDraft(workspace, itemKind, path)).storagePath
 	const liveDraft = UserDraft.getLiveEditorDraft(itemKind, { workspace })
 	if (options.preserveLiveDraft && liveDraft?.storagePath === storagePath) {
 		UserDraft.remove(itemKind, storagePath, { workspace })