bind the fork banner's comparison to the workspace it describes (#10483)

* fix: bind the fork banner's comparison to the workspace it describes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: invalidate in-flight comparisons when leaving a fork Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: drop a CI summary fetched for a superseded comparison Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-11 00:06:06 +00:00 · 2026-08-03 19:09:42 +00:00
parent b718ea8495
commit 7103da6484
1 changed files with 63 additions and 15 deletions
@@ -13,6 +13,9 @@

 	let loading = $state(false)
 	let comparison: WorkspaceComparison | undefined = $state(undefined)
+	/** Workspace `comparison` describes; control flow only, never rendered. */
+	let comparisonFor: string | undefined = undefined
+	let requestSeq = 0
 	let error: string | undefined = $state(undefined)

 	let currentWorkspaceData = $derived($userWorkspaces.find((w) => w.id === $workspaceStore))
@@ -31,23 +34,42 @@
 	const drafts = useWorkspaceDrafts(() => (isFork ? ($workspaceStore ?? undefined) : undefined))
 	const draftCount = $derived(drafts.count)

+	// Every read of `comparison` that decides what the banner says or where its button
+	// goes must go through this: anything else — in flight, failed, or a tally skipped
+	// outright — counts zero of everything, which is indistinguishable from "nothing to
+	// deploy". Typed helper avoids the `never`-inference quirk on `$state` in `$derived`.
+	function isAnswerable(c: WorkspaceComparison | undefined, isLoading: boolean): boolean {
+		return !isLoading && !!c && !c.skipped_comparison
+	}
+	const hasAnswer = $derived(isAnswerable(comparison, loading))
+
 	// Fork is fully in sync with its parent (comparison ran, no ahead/behind diffs).
-	// Typed helper avoids the $state `never`-inference quirk on `comparison` in $derived.
 	function isUpToDate(c: WorkspaceComparison | undefined): boolean {
 		return !!c && !c.skipped_comparison && c.summary.total_diffs === 0
 	}
-	let upToDate = $derived(isUpToDate(comparison))
+	let upToDate = $derived(hasAnswer && isUpToDate(comparison))
 	// Up to date with the parent but local drafts are pending — show the draft
 	// state (same text + CTA as the draft banner) instead of "Everything is up to date".
 	let showDraftsOnly = $derived(upToDate && draftCount > 0)

+	// Leaving for a workspace with no comparison of its own has to invalidate whatever
+	// is in flight too, or that response lands as this one's answer — and its CI counts
+	// would be fetched for paths this workspace may not even have.
+	function dropComparison() {
+		requestSeq++
+		comparison = undefined
+		comparisonFor = undefined
+		loading = false
+		resetCiTestSummary()
+	}
+
 	$effect(() => {
 		;[$workspaceStore, parentWorkspaceId]
 		untrack(() => {
 			if (isFork && $workspaceStore) {
 				checkForChanges()
 			} else {
-				comparison = undefined
+				dropComparison()
 			}
 		})
 	})
@@ -56,40 +78,57 @@
 		if (isFork && $workspaceStore) {
 			checkForChanges()
 		} else {
-			comparison = undefined
+			dropComparison()
 		}
 	})

 	async function checkForChanges() {
-		if (!$workspaceStore || !parentWorkspaceId) {
+		const ws = $workspaceStore
+		const parent = parentWorkspaceId
+		if (!ws || !parent) {
 			return
 		}

+		// A comparison only ever describes the workspace it was requested for. The
+		// component survives a fork switch, so drop the previous fork's rows before
+		// fetching rather than let them answer for this one, and let only the newest
+		// request write — responses can land out of order, and a late one would
+		// otherwise paint another fork's counts over the current answer.
+		if (comparisonFor !== ws) {
+			comparison = undefined
+			comparisonFor = undefined
+			resetCiTestSummary()
+		}
+		const seq = ++requestSeq
 		loading = true
 		error = undefined

 		try {
 			// Compare with parent workspace (shared single-flight fetch — the chat
 			// diff tool reuses this result instead of recomputing the comparison)
-			const result = await fetchWorkspaceComparison(parentWorkspaceId, $workspaceStore)
+			const result = await fetchWorkspaceComparison(parent, ws)

+			if (seq !== requestSeq) return
 			comparison = result
+			comparisonFor = ws
 		} catch (e) {
+			if (seq !== requestSeq) return
 			console.error('Failed to compare workspaces:', e)
 			error = `Failed to check for changes: ${e}`
-			// Still show banner if there's an error, but with error message
+			// Show the banner with the error rather than the rows we failed to refresh:
+			// on a switch those belong to the fork we just left.
+			comparison = undefined
+			comparisonFor = undefined
 		} finally {
-			loading = false
+			if (seq === requestSeq) loading = false
 		}
 	}

 	// Opens the direction the button offers, so the label and the list agree: a fork
 	// with nothing to deploy lands on the update side, not on an empty deploy list.
-	// Both read `comparisonLoaded` — an unknown comparison counts zero of everything,
-	// which is indistinguishable from "nothing to deploy".
 	function openComparisonDrawer() {
 		if (parentWorkspaceId && $workspaceStore) {
-			const dir = comparisonLoaded && changesAhead === 0 ? '&dir=update' : ''
+			const dir = hasAnswer && changesAhead === 0 ? '&dir=update' : ''
 			goto('/forks/compare?workspace_id=' + encodeURIComponent($workspaceStore) + dir, {
 				replaceState: true
 			})
@@ -109,17 +148,29 @@
 	let ciTestRunning = $state(0)
 	let ciTestTotal = $state(0)

+	// These describe the rows of one comparison, so they are dropped with it.
+	function resetCiTestSummary() {
+		ciTestPassing = 0
+		ciTestFailing = 0
+		ciTestRunning = 0
+		ciTestTotal = 0
+	}
+
 	async function fetchCiTestSummary() {
 		if (!$workspaceStore || !comparison?.diffs) return
 		const items = comparison.diffs
 			.filter((d) => d.kind === 'script' || d.kind === 'flow' || d.kind === 'resource')
 			.map((d) => ({ path: d.path, kind: d.kind as 'script' | 'flow' | 'resource' }))
 		if (items.length === 0) return
+		// Counted for the comparison current at call time — the poll below outlives a
+		// fork switch, and a slow batch for the fork we left would repaint this one's.
+		const seq = requestSeq
 		try {
 			const batch = await ScriptService.getCiTestResultsBatch({
 				workspace: $workspaceStore,
 				requestBody: { items }
 			})
+			if (seq !== requestSeq) return
 			let passing = 0
 			let failing = 0
 			let running = 0
@@ -161,9 +212,6 @@
 	function countDir(c: WorkspaceComparison | undefined, mergeIntoParent: boolean): number {
 		return c?.diffs.filter((d) => diffActionableInDirection(d, mergeIntoParent)).length ?? 0
 	}
-	// A comparison in flight is not an answer: on a fork switch the component stays
-	// mounted and `comparison` still holds the previous fork's rows.
-	const comparisonLoaded = $derived(!loading && comparison !== undefined)
 	const changesAhead = $derived(countDir(comparison, true))
 	const changesBehind = $derived(countDir(comparison, false))

@@ -359,7 +407,7 @@
 					>
 						{#if showDraftsOnly}
 							Review & deploy drafts
-						{:else if !comparisonLoaded || changesAhead > 0}
+						{:else if !hasAnswer || changesAhead > 0}
 							Review & Deploy Changes
 						{:else}
 							Review & Update fork