From cbf54d4eb432638e27f67c4c8b879cbcc0291da3 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Tue, 23 Jun 2026 22:54:07 +0200 Subject: [PATCH] fix: preserve fork parent linkage on workspace id change (#9716) Co-authored-by: Claude Opus 4.8 (1M context) --- ...f0f1075b878f9916145929b3cd3b1a53b777e.json | 15 ++++++++++ ...22ba3d670cc36bdbc6451f29b8f22f8cff688.json | 28 +++++++++++++++++++ ...b775225dcb430026cebe15ba4994ac636514d.json | 17 +++++++++++ .../tests/workspaces.rs | 14 ++++++++++ .../src/deployment_requests.rs | 27 ++++++++++++------ .../src/workspaces_extra.rs | 27 ++++++++++++++++-- .../lib/components/ForkWorkspaceBanner.svelte | 6 +++- 7 files changed, 122 insertions(+), 12 deletions(-) create mode 100644 backend/.sqlx/query-40a8cf5e87bb489fd172689e9a6f0f1075b878f9916145929b3cd3b1a53b777e.json create mode 100644 backend/.sqlx/query-42322020ff9cc7dd7ebafc1cb4122ba3d670cc36bdbc6451f29b8f22f8cff688.json create mode 100644 backend/.sqlx/query-a54efa4a7466e61fd54d8fe293cb775225dcb430026cebe15ba4994ac636514d.json diff --git a/backend/.sqlx/query-40a8cf5e87bb489fd172689e9a6f0f1075b878f9916145929b3cd3b1a53b777e.json b/backend/.sqlx/query-40a8cf5e87bb489fd172689e9a6f0f1075b878f9916145929b3cd3b1a53b777e.json new file mode 100644 index 0000000000..6fd7d38f69 --- /dev/null +++ b/backend/.sqlx/query-40a8cf5e87bb489fd172689e9a6f0f1075b878f9916145929b3cd3b1a53b777e.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE workspace SET parent_workspace_id = $1 WHERE parent_workspace_id = $2", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Text" + ] + }, + "nullable": [] + }, + "hash": "40a8cf5e87bb489fd172689e9a6f0f1075b878f9916145929b3cd3b1a53b777e" +} diff --git a/backend/.sqlx/query-42322020ff9cc7dd7ebafc1cb4122ba3d670cc36bdbc6451f29b8f22f8cff688.json b/backend/.sqlx/query-42322020ff9cc7dd7ebafc1cb4122ba3d670cc36bdbc6451f29b8f22f8cff688.json new file mode 100644 index 0000000000..a6ab79cd7c --- /dev/null +++ b/backend/.sqlx/query-42322020ff9cc7dd7ebafc1cb4122ba3d670cc36bdbc6451f29b8f22f8cff688.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT p.id AS \"id!\", p.deleted AS \"deleted!\"\n FROM workspace f\n JOIN workspace p ON p.id = f.parent_workspace_id\n WHERE f.id = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id!", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "deleted!", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + false + ] + }, + "hash": "42322020ff9cc7dd7ebafc1cb4122ba3d670cc36bdbc6451f29b8f22f8cff688" +} diff --git a/backend/.sqlx/query-a54efa4a7466e61fd54d8fe293cb775225dcb430026cebe15ba4994ac636514d.json b/backend/.sqlx/query-a54efa4a7466e61fd54d8fe293cb775225dcb430026cebe15ba4994ac636514d.json new file mode 100644 index 0000000000..819928ddc1 --- /dev/null +++ b/backend/.sqlx/query-a54efa4a7466e61fd54d8fe293cb775225dcb430026cebe15ba4994ac636514d.json @@ -0,0 +1,17 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO workspace (id, name, owner, deleted, premium, parent_workspace_id)\n SELECT $1, $2, owner, false, premium,\n CASE WHEN $4 THEN parent_workspace_id ELSE NULL END\n FROM workspace WHERE id = $3", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Varchar", + "Text", + "Bool" + ] + }, + "nullable": [] + }, + "hash": "a54efa4a7466e61fd54d8fe293cb775225dcb430026cebe15ba4994ac636514d" +} diff --git a/backend/windmill-api-integration-tests/tests/workspaces.rs b/backend/windmill-api-integration-tests/tests/workspaces.rs index fcbdeea8ba..92f179c39b 100644 --- a/backend/windmill-api-integration-tests/tests/workspaces.rs +++ b/backend/windmill-api-integration-tests/tests/workspaces.rs @@ -646,6 +646,20 @@ async fn test_workspace_endpoints(db: Pool) -> anyhow::Result<()> { .unwrap(); assert_eq!(resp.json::().await?, true); + // Regression: changing a fork's workspace id must preserve its parent + // linkage. Dropping it leaves a wm-fork- workspace with no parent — a + // "fork of nothing" that can no longer be compared or merged. + let parent: Option = + sqlx::query_scalar("SELECT parent_workspace_id FROM workspace WHERE id = $1") + .bind("wm-fork-renamed") + .fetch_one(&db) + .await?; + assert_eq!( + parent.as_deref(), + Some("new-test-ws"), + "renamed fork must keep its parent_workspace_id" + ); + // --- create_fork over an existing (active) workspace id: clear 400, not a raw SQL 500 --- let resp = authed(client().post(format!("{new_ws_base}/create_fork"))) .json(&json!({ diff --git a/backend/windmill-api-workspaces/src/deployment_requests.rs b/backend/windmill-api-workspaces/src/deployment_requests.rs index 7782382073..a279f82d6c 100644 --- a/backend/windmill-api-workspaces/src/deployment_requests.rs +++ b/backend/windmill-api-workspaces/src/deployment_requests.rs @@ -717,16 +717,27 @@ async fn create_deployment_request_comment( // ---- helpers ------------------------------------------------------------ async fn parent_of_fork(db: &DB, w_id: &str) -> Result { - sqlx::query_scalar!( - "SELECT parent_workspace_id FROM workspace WHERE id = $1", + // Resolve the fork's parent and require it to still exist and be active. A + // parent that is archived (soft-deleted) can no longer be accessed, so a + // diff or deployment request against it targets an unreachable workspace. + let parent = sqlx::query!( + "SELECT p.id AS \"id!\", p.deleted AS \"deleted!\" + FROM workspace f + JOIN workspace p ON p.id = f.parent_workspace_id + WHERE f.id = $1", w_id, ) .fetch_optional(db) - .await? - .flatten() - .ok_or_else(|| { - Error::BadRequest(format!( + .await?; + + match parent { + None => Err(Error::BadRequest(format!( "workspace {w_id} is not a fork (no parent_workspace_id)" - )) - }) + ))), + Some(p) if p.deleted => Err(Error::BadRequest(format!( + "parent workspace {} of fork {w_id} is archived", + p.id + ))), + Some(p) => Ok(p.id), + } } diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index aa64514b4c..f1dfc4677c 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -65,13 +65,22 @@ pub(crate) async fn change_workspace_id( old_id, rw.new_id ); - // Create new workspace with new id and name + // Create new workspace with new id and name. A fork that keeps a wm-fork- + // id must carry its parent_workspace_id over, otherwise it becomes a + // parentless "fork of nothing" with no source to compare or merge against. + // A non-fork target id means the workspace is being promoted out of a fork, + // so the parent pointer is intentionally cleared. info!("Creating new workspace row"); + let new_is_fork = rw.new_id.starts_with(WM_FORK_PREFIX); sqlx::query!( - "INSERT INTO workspace SELECT $1, $2, owner, false, premium FROM workspace WHERE id = $3", + "INSERT INTO workspace (id, name, owner, deleted, premium, parent_workspace_id) + SELECT $1, $2, owner, false, premium, + CASE WHEN $4 THEN parent_workspace_id ELSE NULL END + FROM workspace WHERE id = $3", &rw.new_id, &rw.new_name, - &old_id + &old_id, + new_is_fork ) .execute(&mut *tx) .await?; @@ -347,6 +356,18 @@ pub(crate) async fn change_workspace_id( .execute(&mut *tx) .await?; + // Re-parent child forks: any fork whose parent_workspace_id was the old id + // must follow the renamed parent to the new id, otherwise it is left + // pointing at the soft-deleted old shell (whose data has moved here). + info!("Re-parenting child forks to the new workspace id"); + sqlx::query!( + "UPDATE workspace SET parent_workspace_id = $1 WHERE parent_workspace_id = $2", + &rw.new_id, + &old_id + ) + .execute(&mut *tx) + .await?; + info!("Updating workspace_protection_rule table"); sqlx::query!( "UPDATE workspace_protection_rule SET workspace_id = $1 WHERE workspace_id = $2", diff --git a/frontend/src/lib/components/ForkWorkspaceBanner.svelte b/frontend/src/lib/components/ForkWorkspaceBanner.svelte index 56418c1988..76740163ed 100644 --- a/frontend/src/lib/components/ForkWorkspaceBanner.svelte +++ b/frontend/src/lib/components/ForkWorkspaceBanner.svelte @@ -12,10 +12,14 @@ let comparison: WorkspaceComparison | undefined = $state(undefined) let error: string | undefined = $state(undefined) - let isFork = $derived($workspaceStore?.startsWith('wm-fork-') ?? false) let currentWorkspaceData = $derived($userWorkspaces.find((w) => w.id === $workspaceStore)) let parentWorkspaceId = $derived(currentWorkspaceData?.parent_workspace_id) let parentWorkspaceData = $derived($userWorkspaces.find((w) => w.id === parentWorkspaceId)) + // A fork must have a parent to compare/merge against. Treating the wm-fork- + // prefix alone as "is a fork" renders a parentless "Fork of ()" banner when + // the parent linkage was dropped (e.g. by a workspace id change), so require + // both, matching the forks/compare page. + let isFork = $derived(($workspaceStore?.startsWith('wm-fork-') ?? false) && !!parentWorkspaceId) // Drafts in this fork. When the fork is otherwise in sync with its parent, a // user with only pending drafts should still get the draft CTA (mirrors the