diff --git a/backend/.sqlx/query-aba0caddc216e218225d41cc3e14db0d8013ca448ca469f1c8c90965a514a8fb.json b/backend/.sqlx/query-aba0caddc216e218225d41cc3e14db0d8013ca448ca469f1c8c90965a514a8fb.json new file mode 100644 index 0000000000..f344bf3795 --- /dev/null +++ b/backend/.sqlx/query-aba0caddc216e218225d41cc3e14db0d8013ca448ca469f1c8c90965a514a8fb.json @@ -0,0 +1,12 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO workspace_diff\n (source_workspace_id, fork_workspace_id, path, kind, ahead, behind, has_changes, exists_in_source, exists_in_fork)\n VALUES ('test-workspace', 'wm-fork-test-workspace', 'f/rt/parent_only', 'http_trigger', 1, 0, true, true, false)", + "describe": { + "columns": [], + "parameters": { + "Left": [] + }, + "nullable": [] + }, + "hash": "aba0caddc216e218225d41cc3e14db0d8013ca448ca469f1c8c90965a514a8fb" +} diff --git a/backend/windmill-api-integration-tests/tests/workspace_comparison.rs b/backend/windmill-api-integration-tests/tests/workspace_comparison.rs index 392f43d410..f9ee6df838 100644 --- a/backend/windmill-api-integration-tests/tests/workspace_comparison.rs +++ b/backend/windmill-api-integration-tests/tests/workspace_comparison.rs @@ -1754,6 +1754,83 @@ async fn test_compare_workspaces_phantom_trigger_shortfuse( Ok(()) } +/// A source-only row is offered to the fork whatever its counters say, so it can +/// carry `behind = 0` — a `behind`-derived tally never sees it, and hiding one must +/// still be reported to the update direction. The merge direction does not carry +/// such a row at all, so hiding one withholds nothing from that side. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_compare_workspaces_hidden_source_only_no_behind( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base_url = format!("http://localhost:{port}/api"); + let non_admin = windmill_api_client::create_client( + &format!("http://localhost:{port}"), + "SECRET_TOKEN_2".to_string(), + ); + + sqlx::query!( + "INSERT INTO workspace (id, name, owner, parent_workspace_id) + VALUES ('wm-fork-test-workspace', 'Fork', 'test-user', 'test-workspace')" + ) + .execute(&db) + .await?; + sqlx::query!("INSERT INTO workspace_settings (workspace_id) VALUES ('wm-fork-test-workspace')") + .execute(&db) + .await?; + sqlx::query!( + "INSERT INTO workspace_key(workspace_id, kind, key) + VALUES ('wm-fork-test-workspace', 'cloud', 'test-key')" + ) + .execute(&db) + .await?; + + // Source-only row with no `behind`: the trigger has no backing row, so the + // visibility filter drops it exactly as it would an ACL-hidden one. + sqlx::query!( + "INSERT INTO workspace_diff + (source_workspace_id, fork_workspace_id, path, kind, ahead, behind, has_changes, exists_in_source, exists_in_fork) + VALUES ('test-workspace', 'wm-fork-test-workspace', 'f/rt/parent_only', 'http_trigger', 1, 0, true, true, false)" + ) + .execute(&db) + .await?; + + let comparison: serde_json::Value = non_admin + .client() + .get(&format!( + "{base_url}/w/test-workspace/workspaces/compare/wm-fork-test-workspace" + )) + .send() + .await? + .json() + .await?; + assert_eq!( + comparison["all_behind_items_visible"].as_bool(), + Some(false), + "a hidden source-only row must trip the warning even at behind = 0: {comparison}" + ); + assert_eq!( + comparison["hidden_behind"]["total"].as_i64(), + Some(1), + "a hidden source-only row must be counted as withheld from the update direction: {comparison}" + ); + assert_eq!( + comparison["all_ahead_items_visible"].as_bool(), + Some(true), + "the merge direction does not carry a source-only row, so hiding one withholds nothing from it: {comparison}" + ); + assert_eq!( + comparison["hidden_ahead"]["total"].as_i64(), + Some(0), + "a source-only row must not be reported as withheld from the merge direction: {comparison}" + ); + + Ok(()) +} + /// Regression: the "sees everything" guard must require admin of BOTH sides, not /// just the fork. `filter_visible_diffs` keeps a modified/conflict row (one that /// exists in the source AND the fork) only when the caller can see it on both diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 3a1dbc5a1b..29bd3dd604 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -9585,16 +9585,28 @@ async fn compare_workspaces( .count(), }; - let all_ahead_items_visible = summary.total_ahead - == confirmed_diffs - .iter() + // Each direction accounts for what it carries (see the frontend's + // `diffActionableInDirection`): a lineage merge leaves out a row the fork lacks, + // while the update takes it whatever the counters say — and since it can carry + // `behind = 0`, that side counts rows rather than sums. + let source_only = |d: &WorkspaceDiffRow| { + d.exists_in_source.unwrap_or(false) && !d.exists_in_fork.unwrap_or(false) + }; + let merge_carries = |d: &WorkspaceDiffRow| !is_lineage_pair || !source_only(d); + let ahead_sum = |rows: &[WorkspaceDiffRow]| { + rows.iter() + .filter(|d| merge_carries(d)) .map(|s| s.ahead) - .fold(0, |acc, s| acc + s.try_into().unwrap_or(0)); + .fold(0i64, |acc, s| acc + i64::from(s)) + }; + let all_ahead_items_visible = ahead_sum(&visible_diffs) == ahead_sum(&confirmed_diffs); let all_behind_items_visible = summary.total_behind == confirmed_diffs .iter() .map(|s| s.behind) - .fold(0, |acc, s| acc + s.try_into().unwrap_or(0)); + .fold(0, |acc, s| acc + s.try_into().unwrap_or(0)) + && visible_diffs.iter().filter(|d| source_only(d)).count() + == confirmed_diffs.iter().filter(|d| source_only(d)).count(); // Blast-radius guard for the "changes not visible to your user" warning // (which hides the deploy button). The flag is a pure visibility guarantee — @@ -9634,7 +9646,9 @@ async fn compare_workspaces( if visible_keys.contains(&(d.kind.as_str(), d.path.as_str())) { continue; } - if d.ahead > 0 { + // Both sides mirror the flags above: a row is only withheld from a direction + // that would have carried it. + if d.ahead > 0 && merge_carries(d) { hidden_ahead.total += 1; *hidden_ahead.by_kind.entry(d.kind.clone()).or_default() += 1; if sees_all_items { @@ -9643,7 +9657,7 @@ async fn compare_workspaces( .push(HiddenItem { kind: d.kind.clone(), path: d.path.clone() }); } } - if d.behind > 0 { + if d.behind > 0 || source_only(d) { hidden_behind.total += 1; *hidden_behind.by_kind.entry(d.kind.clone()).or_default() += 1; if sees_all_items { diff --git a/frontend/src/lib/components/CompareWorkspaces.svelte b/frontend/src/lib/components/CompareWorkspaces.svelte index bf7a1cf433..f8a5320910 100644 --- a/frontend/src/lib/components/CompareWorkspaces.svelte +++ b/frontend/src/lib/components/CompareWorkspaces.svelte @@ -43,6 +43,9 @@ checkDeployPermission, deployItem, deleteItemInWorkspace, + diffActionableInDirection, + diffCreatesInTarget, + diffRemovesInTarget, getItemValue, getOnBehalfOf, type DeployPermission, @@ -407,13 +410,9 @@ let canPreserveOnBehalfOf = $derived(mergeIntoParent ? canPreserveInParent : canPreserveInCurrent) let selectableDiffs = $derived( - comparison?.diffs.filter((diff) => { - if (mergeIntoParent) { - return diff.ahead > 0 - } else { - return diff.behind > 0 - } - }) ?? [] + comparison?.diffs.filter((diff) => + diffActionableInDirection(diff, mergeIntoParent, isArbitraryTarget) + ) ?? [] ) let selectedItems = $state([]) @@ -457,10 +456,17 @@ }) ?? [] ) + // Gates the "update before deploying" alert, so it counts what a merge would + // actually carry — not every row with an `ahead` counter, which includes the + // parent-only ones the merge direction leaves out. let itemsWithAheadChanges = $derived( comparison?.diffs.filter((diff) => { const status = deploymentStatus[getItemKey(diff)]?.status - return diff && diff.ahead > 0 && !(status && status == 'deployed') + return ( + diff && + diffActionableInDirection(diff, true, isArbitraryTarget) && + !(status && status == 'deployed') + ) }) ?? [] ) @@ -477,6 +483,7 @@ let onBehalfOfChoice = $state>({}) let customOnBehalfOf = $state>({}) let deployTargetWorkspace = $derived(mergeIntoParent ? parentWorkspaceId : currentWorkspaceId) + let deploySourceWorkspace = $derived(mergeIntoParent ? currentWorkspaceId : parentWorkspaceId) function getItemKey(diff: WorkspaceItemDiff): string { return `${diff.kind}:${diff.path}` @@ -641,12 +648,10 @@ // All *diff* items selected. Trigger items are opt-in and don't count // toward "all selected" — see item merge below in deployableItems. - // Deploying a row the current workspace lacks deletes it in the target. Against - // the parent that is a real deletion to propagate and is selected like anything - // else; against an arbitrary target the two workspaces were simply never in sync, - // so it takes an explicit act on that row. No bulk action may sweep one in. + // A row whose deploy deletes in the target takes an explicit tick (see + // `diffRemovesInTarget`); no bulk action may sweep one in. function removesInTarget(diff: WorkspaceItemDiff): boolean { - return isArbitraryTarget && diff.exists_in_fork === false + return diffRemovesInTarget(diff, mergeIntoParent) } let bulkSelectableDiffs = $derived(selectableDiffs.filter((d) => !removesInTarget(d))) @@ -691,17 +696,14 @@ ) { deploymentStatus[statusKey] = { status: 'loading' } - // Check if the item was deleted in the source workspace. - // If so, archive/delete it in the target workspace instead of copying. + // The workspace this deploy reads from doesn't have the item: archive/delete + // it in the target instead of copying. Same predicate as the row's badge and + // its exclusion from bulk selection, so what the row says is what it does. const diff = comparison?.diffs.find((d) => getItemKey(d) === statusKey) - const itemDeletedInSource = diff - ? mergeIntoParent - ? diff.exists_in_fork === false - : diff.exists_in_source === false - : false + const removes = diff ? removesInTarget(diff) : false let result: DeployResult - if (itemDeletedInSource) { + if (removes) { result = await deleteItemInWorkspace(kind, path, workspaceToDeployTo) } else { result = await deployItem({ @@ -899,9 +901,15 @@ const filtered = bulkSelectableDiffs.filter( (d) => !isTriggerOrScheduleKind(d.kind) && !hasDraft(d) ) + // The update direction leaves out two ambiguous shapes, both still one click + // away through "Select all": a conflict, and a parent-only row the fork has + // deploy events for — the fork may have dropped it on purpose, and a routine + // update must not silently bring it back. const conflictSafe = mergeIntoParent ? filtered - : filtered.filter((d) => !(d.ahead > 0 && d.behind > 0)) + : filtered.filter( + (d) => !(d.ahead > 0 && d.behind > 0) && !(d.ahead > 0 && diffCreatesInTarget(d, false)) + ) // When reached from a session's Review, narrow the default to this chat's items. const scoped = chatMask ? conflictSafe.filter((d) => diffInMask(d, chatMask)) : conflictSafe selectedItems = scoped @@ -987,6 +995,10 @@ let removalKeys = new Set() $effect(() => { const diffs = comparison?.diffs + // Removals are direction-dependent, so a flip must refresh the set: keeping the + // other direction's would make every row of the new one look freshly flipped + // and revoke the opt-in this guard protects. + ;[mergeIntoParent] if (!diffs) return untrack(() => { const current = new Set(diffs.filter(removesInTarget).map((d) => getItemKey(d))) @@ -1565,39 +1577,20 @@ +Draft {/if} - - {#if !diff.exists_in_fork && diff.exists_in_source && diff.ahead == 0 && diff.behind > 0} + + {#if diffCreatesInTarget(diff, mergeIntoParent)} New - {/if} - {#if !diff.exists_in_fork && diff.exists_in_source && diff.ahead > 0} - + {:else if removesInTarget(diff)} {isArbitraryTarget ? 'Removes in target' : 'Deleted'} - {/if} - {#if diff.exists_in_fork && !diff.exists_in_source && diff.behind > 0} - Deleted - {/if} - {#if diff.exists_in_fork && !diff.exists_in_source && diff.ahead > 0 && diff.behind == 0} - NewRemoves in {deployTargetWorkspace} {/if} {@const ciStatus = getCiTestStatus(diff)} diff --git a/frontend/src/lib/components/ForkWorkspaceBanner.svelte b/frontend/src/lib/components/ForkWorkspaceBanner.svelte index 4484ba088f..d033e881f4 100644 --- a/frontend/src/lib/components/ForkWorkspaceBanner.svelte +++ b/frontend/src/lib/components/ForkWorkspaceBanner.svelte @@ -9,6 +9,7 @@ import { onMount, untrack } from 'svelte' import { useWorkspaceDrafts } from '$lib/workspaceDrafts.svelte' import { devLabelWord } from '$lib/utils/devWorkspaceLabel' + import { diffActionableInDirection } from '$lib/utils_workspace_deploy' let loading = $state(false) let comparison: WorkspaceComparison | undefined = $state(undefined) @@ -82,9 +83,14 @@ } } + // 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) { - goto('/forks/compare?workspace_id=' + encodeURIComponent($workspaceStore), { + const dir = comparisonLoaded && changesAhead === 0 ? '&dir=update' : '' + goto('/forks/compare?workspace_id=' + encodeURIComponent($workspaceStore) + dir, { replaceState: true }) } @@ -148,6 +154,19 @@ return () => clearInterval(interval) }) + // Counted with the compare page's own predicate so the banner never advertises a + // direction whose list is empty: the `ahead`/`behind` sums in the summary include + // rows a direction does not carry, and miss a parent-only row that the update + // direction carries at `behind = 0`. + 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)) + function forkAheadBehindMessage(changesAhead: number, changesBehind: number) { let msg: string[] = [] if (changesAhead > 0 || changesBehind > 0) { @@ -195,10 +214,7 @@
{#if comparison.summary.total_diffs > 0} - {forkAheadBehindMessage( - comparison.summary.total_ahead, - comparison.summary.total_behind - )} + {forkAheadBehindMessage(changesAhead, changesBehind)} {parentWorkspaceId} over {comparison .summary.total_diffs} items @@ -343,7 +359,7 @@ > {#if showDraftsOnly} Review & deploy drafts - {:else if (comparison?.summary.total_ahead ?? 0) > 0} + {:else if !comparisonLoaded || changesAhead > 0} Review & Deploy Changes {:else} Review & Update fork diff --git a/frontend/src/lib/components/WorkspaceDeployLayout.svelte b/frontend/src/lib/components/WorkspaceDeployLayout.svelte index 52ce309e9e..66beb18b73 100644 --- a/frontend/src/lib/components/WorkspaceDeployLayout.svelte +++ b/frontend/src/lib/components/WorkspaceDeployLayout.svelte @@ -85,7 +85,10 @@ }: Props = $props() let selectableItems = $derived(items.filter(selectablePredicate)) - let hasSelectableItems = $derived(selectableItems.length > 0) + // "Select all" is a bulk action, so it lives or dies by the bulk-selectable set: + // a list of nothing but bulk-excluded rows would otherwise offer an enabled + // control that selects nothing. Each such row is still selectable on its own. + let hasSelectableItems = $derived(selectableItems.some((i) => !bulkExcluded(i))) // Plain row click and the checkbox both toggle this row in/out — multi-select // is the default, no modifier needed. @@ -195,6 +198,13 @@ {#if showGroupHeaders} {@const selectable = groupSelectable(group)} {@const selectedCount = selectable.filter((i) => selectedItems.includes(i.key)).length} + + {@const selectedInGroup = group.items.filter((i) => + selectedItems.includes(i.key) + ).length}
{group.label} - {group.items.length} item{group.items.length !== 1 ? 's' : ''}{selectedCount > 0 - ? ` · ${selectedCount} selected` + {group.items.length} item{group.items.length !== 1 ? 's' : ''}{selectedInGroup > 0 + ? ` · ${selectedInGroup} selected` : ''} {#if groupActions} diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 1034dcb62a..e626083ae1 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -6014,8 +6014,8 @@ function formatForkIndexEntry(e: ForkDiffEntryView): string { switch (e.status) { case 'only_in_fork': return `- ${name} — only in fork (${e.patchLineCount} lines)${draftFlag}` - case 'deleted_in_fork': - return `- ${name} — deleted in fork, still in parent${draftFlag}` + case 'only_in_parent': + return `- ${name} — only in parent, not in fork${draftFlag}` case 'modified': return `- ${name} — differs (${aheadBehind}; ${e.patchLineCount} diff lines)${draftFlag}` case 'unchanged': @@ -6193,8 +6193,8 @@ function renderForkEntrySection( const header = entry.status === 'only_in_fork' ? `${entry.kind} "${path}" exists only in the fork — not in parent "${parent}". Full content:\n\n` - : entry.status === 'deleted_in_fork' - ? `${entry.kind} "${path}" was deleted in the fork but still exists in parent "${parent}". Removed content:\n\n` + : entry.status === 'only_in_parent' + ? `${entry.kind} "${path}" exists only in parent "${parent}" — not in the fork. Parent content:\n\n` : `Fork changes vs parent "${parent}" for ${entry.kind} "${path}":\n\n` if (args.file !== undefined && !entry.files) { throw new Error( diff --git a/frontend/src/lib/components/copilot/chat/global/diffSnapshot.test.ts b/frontend/src/lib/components/copilot/chat/global/diffSnapshot.test.ts index 76aed58b7c..7c8253684d 100644 --- a/frontend/src/lib/components/copilot/chat/global/diffSnapshot.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/diffSnapshot.test.ts @@ -465,7 +465,7 @@ describe('fork mode', () => { expect(byPath['f/a/b'].status).toBe('modified') expect(byPath['f/a/new'].status).toBe('only_in_fork') expect(byPath['f/a/new'].patch).not.toContain('parent-ws') - expect(byPath['f/a/gone'].status).toBe('deleted_in_fork') + expect(byPath['f/a/gone'].status).toBe('only_in_parent') // One-sided entries fetch only the existing side: 2 + 1 + 1 calls. expect(getItemValue).toHaveBeenCalledTimes(4) }) diff --git a/frontend/src/lib/components/copilot/chat/global/diffSnapshot.ts b/frontend/src/lib/components/copilot/chat/global/diffSnapshot.ts index f8a685af60..7b7d9ec1fd 100644 --- a/frontend/src/lib/components/copilot/chat/global/diffSnapshot.ts +++ b/frontend/src/lib/components/copilot/chat/global/diffSnapshot.ts @@ -620,7 +620,7 @@ const FORK_COMPARISON_REUSE_MS = 30_000 export type ForkDiffStatus = | 'modified' | 'only_in_fork' - | 'deleted_in_fork' + | 'only_in_parent' | 'unchanged' | 'pending' | 'error' @@ -657,7 +657,7 @@ export interface ForkDiffIndexView { } interface ForkMaterialized { - status: 'modified' | 'only_in_fork' | 'deleted_in_fork' | 'unchanged' | 'error' + status: 'modified' | 'only_in_fork' | 'only_in_parent' | 'unchanged' | 'error' patch: string lineCount: number files?: Record @@ -1023,7 +1023,7 @@ async function materializeFork( const forkValue = forkSide?.value const valueMasked = parentSide?.valueMasked === true || forkSide?.valueMasked === true const oneSidedStatus = !entry.existsInFork - ? 'deleted_in_fork' + ? 'only_in_parent' : !entry.existsInParent ? 'only_in_fork' : undefined diff --git a/frontend/src/lib/utils_workspace_deploy.test.ts b/frontend/src/lib/utils_workspace_deploy.test.ts new file mode 100644 index 0000000000..3764957e0d --- /dev/null +++ b/frontend/src/lib/utils_workspace_deploy.test.ts @@ -0,0 +1,41 @@ +import { describe, it, expect } from 'vitest' +import { + diffActionableInDirection, + diffCreatesInTarget, + diffRemovesInTarget +} from './utils_workspace_deploy' + +/** The row shape the fork comparison returns for an item the parent has and the + * fork does not. `ahead = 1, behind = 0` is what the tally leaves after the fork + * pulled the parent's item in and then lost it (a delete, a git-sync revert): + * every deploy event in the fork counts as `ahead`, whatever wrote it. */ +const parentOnly = { ahead: 1, behind: 0, exists_in_source: true, exists_in_fork: false } +const forkOnly = { ahead: 1, behind: 0, exists_in_source: false, exists_in_fork: true } +const bothSides = { ahead: 1, behind: 1, exists_in_source: true, exists_in_fork: true } + +describe('deploy direction of a one-sided diff row', () => { + it('offers a parent-only item to the fork even with no behind count', () => { + expect(diffActionableInDirection(parentOnly, false)).toBe(true) + expect(diffCreatesInTarget(parentOnly, false)).toBe(true) + expect(diffRemovesInTarget(parentOnly, false)).toBe(false) + }) + + it('keeps a parent-only row out of a merge into the parent, whatever its ahead count', () => { + expect(diffActionableInDirection(parentOnly, true)).toBe(false) + // An arbitrary target has no tally behind it and does propagate the removal. + expect(diffActionableInDirection(parentOnly, true, true)).toBe(true) + expect(diffRemovesInTarget(parentOnly, true)).toBe(true) + }) + + it('does not resurrect a fork-only item into an update of the fork', () => { + expect(diffActionableInDirection(forkOnly, false)).toBe(false) + expect(diffCreatesInTarget(forkOnly, true)).toBe(true) + }) + + it('keeps a two-sided row on its counters', () => { + expect(diffActionableInDirection(bothSides, true)).toBe(true) + expect(diffActionableInDirection({ ...bothSides, behind: 0 }, false)).toBe(false) + expect(diffCreatesInTarget(bothSides, true)).toBe(false) + expect(diffRemovesInTarget(bothSides, false)).toBe(false) + }) +}) diff --git a/frontend/src/lib/utils_workspace_deploy.ts b/frontend/src/lib/utils_workspace_deploy.ts index ccd3d29434..95941b289c 100644 --- a/frontend/src/lib/utils_workspace_deploy.ts +++ b/frontend/src/lib/utils_workspace_deploy.ts @@ -354,6 +354,53 @@ export async function deployItem(params: DeployItemParams): Promise 0 + } + return diff.behind > 0 || diffCreatesInTarget(diff, mergeIntoParent) +} + /** * Delete/archive an item in a workspace. * Used when deploying a deletion from one workspace to another. diff --git a/frontend/src/routes/(root)/(logged)/forks/compare/+page.svelte b/frontend/src/routes/(root)/(logged)/forks/compare/+page.svelte index 5f9d86ca21..f542a79d99 100644 --- a/frontend/src/routes/(root)/(logged)/forks/compare/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/forks/compare/+page.svelte @@ -10,6 +10,7 @@ reconcileAfterWorkspaceChange } from '$lib/components/sessions/sessionState.svelte' import { useWorkspaceDrafts } from '$lib/workspaceDrafts.svelte' + import { diffActionableInDirection } from '$lib/utils_workspace_deploy' import { page } from '$app/state' import { userWorkspaces, workspaceStore } from '$lib/stores' import { onDestroy, untrack } from 'svelte' @@ -65,7 +66,11 @@ // Which fork direction to restore when switching back from draft mode. The // merged toggle (CompareModeToggle, rendered inside each card) reports its // selection here; the page only swaps which comparison component is shown. - let forkDirection = $state<'deploy_to' | 'update'>('deploy_to') + // `?dir=update` opens on the other one, for callers that already know which + // direction has something in it (the fork banner's CTA). + let forkDirection = $state<'deploy_to' | 'update'>( + page.url.searchParams.get('dir') === 'update' ? 'update' : 'deploy_to' + ) // Explicit preselection via `?items=` (built by the chat's // open_page tool). Parsed synchronously from the live URL so it can never race @@ -149,17 +154,21 @@ ) // Per-direction counts for the merged toggle badges. Deployable = items ahead - // (fork has changes the parent lacks); updateable = items behind. Computed - // here so they show on the toggle in draft mode too (where CompareDrafts has - // no comparison data of its own). Typed helpers avoid a $state `never` - // inference quirk on `comparison` inside $derived. A conflict (ahead AND - // behind) is intentionally counted in both directions — it's actionable either - // way. - function countDir(c: WorkspaceComparison | undefined, dir: 'ahead' | 'behind'): number { - return c?.diffs.filter((d) => d[dir] > 0).length ?? 0 + // (fork has changes the parent lacks); updateable = items behind, plus what the + // parent has and the fork does not. Same predicate as the deploy list, so the + // badge never counts rows the list won't show. Computed here so they show on the + // toggle in draft mode too (where CompareDrafts has no comparison data of its + // own). Typed helpers avoid a $state `never` inference quirk on `comparison` + // inside $derived. A conflict (ahead AND behind) is intentionally counted in both + // directions — it's actionable either way. + function countDir(c: WorkspaceComparison | undefined, mergeIntoParent: boolean): number { + return ( + c?.diffs.filter((d) => diffActionableInDirection(d, mergeIntoParent, isArbitraryTarget)) + .length ?? 0 + ) } - const deployCount = $derived(countDir(comparison, 'ahead')) - const updateCount = $derived(countDir(comparison, 'behind')) + const deployCount = $derived(countDir(comparison, true)) + const updateCount = $derived(countDir(comparison, false)) $effect(() => { if (modeResolved || !currentWorkspaceData) return