From 98ceb7d647d73a84aa12b3be53bb6bc0644ad629 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Wed, 28 Oct 2026 02:51:39 -0700 Subject: [PATCH] feat: move a session to any workspace of its family A session could only move into a direct fork of its workspace. The picker now lists the whole family and lets a new fork be based on any of its workspaces, and /drafts/transfer accepts any target with the same root. A fork the move creates from elsewhere holds that base's drafts, so its drafts are never replaced. Co-Authored-By: Claude Opus 5.5 --- ...a35f985f57fc444a5a88c2e9a2b95f990b9a4.json | 23 ------- ...1d73d17533f1887021374451e3b6e4803c67b.json | 23 +++++++ .../tests/drafts.rs | 18 ++++- backend/windmill-api/openapi.yaml | 4 +- backend/windmill-api/src/drafts.rs | 45 +++++++----- backend/windmill-common/src/user_drafts.rs | 2 +- .../sessions/SessionMoveModal.svelte | 2 +- .../components/sessions/SessionWrapper.svelte | 6 +- .../sessions/WorkspaceFamilyPicker.svelte | 68 ++++++++----------- .../components/sessions/moveSessionToFork.ts | 9 ++- 10 files changed, 114 insertions(+), 86 deletions(-) delete mode 100644 backend/.sqlx/query-4a9430cb7d2a497f3aa648b020aa35f985f57fc444a5a88c2e9a2b95f990b9a4.json create mode 100644 backend/.sqlx/query-ac1007f6bdd93ebe9cebd9f1c541d73d17533f1887021374451e3b6e4803c67b.json diff --git a/backend/.sqlx/query-4a9430cb7d2a497f3aa648b020aa35f985f57fc444a5a88c2e9a2b95f990b9a4.json b/backend/.sqlx/query-4a9430cb7d2a497f3aa648b020aa35f985f57fc444a5a88c2e9a2b95f990b9a4.json deleted file mode 100644 index 9ecd1a96fb..0000000000 --- a/backend/.sqlx/query-4a9430cb7d2a497f3aa648b020aa35f985f57fc444a5a88c2e9a2b95f990b9a4.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT EXISTS(SELECT 1 FROM workspace WHERE id = $1 AND parent_workspace_id = $2 AND deleted = false)", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "exists", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "4a9430cb7d2a497f3aa648b020aa35f985f57fc444a5a88c2e9a2b95f990b9a4" -} diff --git a/backend/.sqlx/query-ac1007f6bdd93ebe9cebd9f1c541d73d17533f1887021374451e3b6e4803c67b.json b/backend/.sqlx/query-ac1007f6bdd93ebe9cebd9f1c541d73d17533f1887021374451e3b6e4803c67b.json new file mode 100644 index 0000000000..aa4b1eb9b8 --- /dev/null +++ b/backend/.sqlx/query-ac1007f6bdd93ebe9cebd9f1c541d73d17533f1887021374451e3b6e4803c67b.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "WITH RECURSIVE up(id, parent, start, depth) AS (\n SELECT id, parent_workspace_id, id, 0 FROM workspace\n WHERE id IN ($1, $2) AND deleted = false\n UNION ALL\n SELECT w.id, w.parent_workspace_id, up.start, up.depth + 1\n FROM workspace w JOIN up ON w.id = up.parent\n WHERE up.depth < 64\n )\n SELECT (SELECT id FROM up WHERE start = $1 AND parent IS NULL)\n = (SELECT id FROM up WHERE start = $2 AND parent IS NULL) AS \"same\"", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "same", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Varchar", + "Varchar" + ] + }, + "nullable": [ + null + ] + }, + "hash": "ac1007f6bdd93ebe9cebd9f1c541d73d17533f1887021374451e3b6e4803c67b" +} diff --git a/backend/windmill-api-integration-tests/tests/drafts.rs b/backend/windmill-api-integration-tests/tests/drafts.rs index e3aa882d95..85c2cba82e 100644 --- a/backend/windmill-api-integration-tests/tests/drafts.rs +++ b/backend/windmill-api-integration-tests/tests/drafts.rs @@ -530,12 +530,26 @@ async fn test_transfer_drafts_to_fork(db: Pool) -> anyhow::Result<()> (Some("ffffffffffffffff"), Some("ffffffffffffffff")) ); - // Only a fork of the source is a valid target. + // Any workspace of the family takes them: here back from the fork to its parent. let r = c .post(format!( "http://localhost:{port}/api/w/{fork}/drafts/transfer" )) - .json(&json!({ "target_workspace": WS, "items": items })) + .json(&json!({ + "target_workspace": WS, + "items": [{ "kind": "script", "path": "u/test-user/a" }], + "remove_from_source": true + })) + .send() + .await?; + assert_eq!(r.status(), 200, "{}", r.text().await?); + assert_eq!(draft_count(&db, "u/test-user/a", "script", me).await, 1); + assert_eq!(fork_count("u/test-user/a").await?, 0); + + // A workspace outside the family is refused. + let r = c + .post(&transfer_url) + .json(&json!({ "target_workspace": "admins", "items": items })) .send() .await?; assert_eq!(r.status(), 400); diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 14a09a4878..1619cdba55 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -11313,8 +11313,8 @@ paths: /w/{workspace}/drafts/transfer: post: - summary: copy the current user's drafts into a fork and/or remove them here - description: In one transaction, copies the authed user's own drafts of `items` into `target_workspace` (a direct fork of this workspace) and, with `remove_from_source`, deletes them here. Without `target_workspace` it only removes. If the user already has a draft of any item in the target, nothing is written and the items are returned in `conflicts`, unless `replace_existing` is set. A user who can only compose flows or apps in the target is held to the same rules as a direct draft save there. + summary: copy the current user's drafts into another workspace of the family and/or remove them here + description: In one transaction, copies the authed user's own drafts of `items` into `target_workspace` (another workspace of this one's family, with the same root through `parent_workspace_id`) and, with `remove_from_source`, deletes them here. Without `target_workspace` it only removes. If the user already has a draft of any item in the target, nothing is written and the items are returned in `conflicts`, unless `replace_existing` is set. A user who can only compose flows or apps in the target is held to the same rules as a direct draft save there. operationId: transferDrafts tags: - draft diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index b19ec1ef5e..f5a8cccdd2 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -1011,8 +1011,8 @@ pub struct DraftItemRef { #[derive(Deserialize)] pub struct TransferDraftsRequest { - /// A direct fork of this workspace to copy the drafts into. Omitted, the call only removes - /// them here (`remove_from_source`). + /// A workspace of this one's family (the same root: its forks, its parent, siblings) to copy + /// the drafts into. Omitted, the call only removes them here (`remove_from_source`). #[serde(default)] pub target_workspace: Option, pub items: Vec, @@ -1035,9 +1035,9 @@ pub struct TransferDraftsResponse { pub conflicts: Vec, } -/// Copy the caller's own drafts of `items` into a fork of this workspace and/or remove them -/// here, in one transaction, so a session moving to a fork never leaves its work in both -/// places or in neither. +/// Copy the caller's own drafts of `items` into another workspace of this one's family and/or +/// remove them here, in one transaction, so a session moving to a fork never leaves its work in +/// both places or in neither. /// /// Only the caller's rows are touched (never a teammate's or the legacy NULL-email one), so /// removing needs no write right here, as with discarding one's own draft. Copying needs it @@ -1062,21 +1062,34 @@ async fn transfer_drafts( let mut target_authed = None; if let Some(target) = req.target_workspace.as_deref() { - let is_fork = sqlx::query_scalar!( - "SELECT EXISTS(SELECT 1 FROM workspace WHERE id = $1 AND parent_workspace_id = $2 AND deleted = false)", - target, - &w_id, - ) - .fetch_one(&db) - .await? - .unwrap_or(false); - if !is_fork { + // Same root, reached through `parent_workspace_id`. The copy keeps what the target can + // resolve (script lineage it has, members it has) and repairs the rest, so any workspace + // of the family can take the drafts, not only a direct fork. + let same_family = target != w_id + && sqlx::query_scalar!( + r#"WITH RECURSIVE up(id, parent, start, depth) AS ( + SELECT id, parent_workspace_id, id, 0 FROM workspace + WHERE id IN ($1, $2) AND deleted = false + UNION ALL + SELECT w.id, w.parent_workspace_id, up.start, up.depth + 1 + FROM workspace w JOIN up ON w.id = up.parent + WHERE up.depth < 64 + ) + SELECT (SELECT id FROM up WHERE start = $1 AND parent IS NULL) + = (SELECT id FROM up WHERE start = $2 AND parent IS NULL) AS "same""#, + target, + &w_id, + ) + .fetch_one(&db) + .await? + .unwrap_or(false); + if !same_family { return Err(Error::BadRequest(format!( - "{target} is not a fork of {w_id}" + "{target} is not in the same workspace family as {w_id}" ))); } // Authentication checked the token against `w_id` only. A token bound to that - // workspace, or a job token (not in `token`), must not reach into the fork. + // workspace, or a job token (not in `token`), must not reach into the target. let token_valid_for_target = sqlx::query_scalar!( "SELECT EXISTS(SELECT 1 FROM token WHERE token_hash = $1 AND (expiration > NOW() OR expiration IS NULL) diff --git a/backend/windmill-common/src/user_drafts.rs b/backend/windmill-common/src/user_drafts.rs index b99c0a26d4..8859441ef1 100644 --- a/backend/windmill-common/src/user_drafts.rs +++ b/backend/windmill-common/src/user_drafts.rs @@ -665,7 +665,7 @@ pub async fn delete_own_draft_for_path( /// copied. With `items = None` this is the fork-time clone: every row of `email` plus the legacy /// NULL-email row. With `items`, only `email`'s own rows at those `(kind, path)` pairs. /// -/// No authorization here. The caller MUST have checked that `target` is a fork of `source`, that +/// No authorization here. The caller MUST have checked that `target` is in `source`'s family, that /// the credential behind `email` may act on `target`, and that it may write each copied path /// there. Secret values are copied as encrypted, so the two workspaces must share a key. /// diff --git a/frontend/src/lib/components/sessions/SessionMoveModal.svelte b/frontend/src/lib/components/sessions/SessionMoveModal.svelte index c8122342f8..bf51bfab36 100644 --- a/frontend/src/lib/components/sessions/SessionMoveModal.svelte +++ b/frontend/src/lib/components/sessions/SessionMoveModal.svelte @@ -72,7 +72,7 @@ diff --git a/frontend/src/lib/components/sessions/SessionWrapper.svelte b/frontend/src/lib/components/sessions/SessionWrapper.svelte index cb8db6721b..6f751471ff 100644 --- a/frontend/src/lib/components/sessions/SessionWrapper.svelte +++ b/frontend/src/lib/components/sessions/SessionWrapper.svelte @@ -803,7 +803,7 @@ startMove({ kind: 'existing', id })} onCreateFork={(fork) => startMove({ kind: 'new', fork })} createForkCaption="Created now, and the session moves into it." @@ -966,7 +966,9 @@ open={moveRequest.open} parentName={moveRequest.parentName} targetName={moveRequest.targetName} - newFork={moveRequest.target.kind === 'new' || !!moveRequest.target.replace} + newFork={moveRequest.target.kind === 'new' + ? moveRequest.target.fork.parent_workspace_id === moveRequest.parent + : !!moveRequest.target.replace} items={moveRequest.items} conflicts={moveRequest.conflicts} error={moveRequest.error} diff --git a/frontend/src/lib/components/sessions/WorkspaceFamilyPicker.svelte b/frontend/src/lib/components/sessions/WorkspaceFamilyPicker.svelte index 8d175e57ea..4510ffd7ea 100644 --- a/frontend/src/lib/components/sessions/WorkspaceFamilyPicker.svelte +++ b/frontend/src/lib/components/sessions/WorkspaceFamilyPicker.svelte @@ -78,9 +78,9 @@ // WorkspaceScopeHeader), so the superadmin lookup isn't duplicated. Omitted // by standalone consumers, which then resolve it themselves. forkableWorkspaces: forkableWorkspacesProp, - // Narrow the list to this workspace and its direct forks, and fork only from it — - // for moving something into a fork of where it is, rather than anywhere in the family. - forksOf, + // The workspace something moves out of: its family is listed, and a new fork is based on it + // unless another base is picked. Its own forking rule and the user's role there gate creating. + moveFrom, open: dropdownOpen = $bindable(), trigger }: { @@ -96,7 +96,7 @@ settingsHref?: string settingsLabel?: string forkableWorkspaces?: UserWorkspace[] - forksOf?: string + moveFrom?: string open?: boolean trigger: Snippet<[{ open: boolean }]> } = $props() @@ -112,21 +112,11 @@ enabled: () => forkableWorkspacesProp === undefined }) const forkableWorkspaces = $derived(forkableWorkspacesProp ?? ownForkable.current) - const root = $derived( - forksOf - ? forkableWorkspaces.find((w) => w.id === forksOf) - : findWorkspaceRoot(effectiveId, forkableWorkspaces) - ) + const root = $derived(findWorkspaceRoot(moveFrom ?? effectiveId, forkableWorkspaces)) // The same hierarchy the sidebar workspace menu builds: forks of forks follow their parent, and the // user's own forks lead each group of siblings. const hierarchy = $derived(buildWorkspaceHierarchy(forkableWorkspaces, $userStore?.email)) - const forkRows = $derived( - root - ? workspaceSubtreeRows(root.id, hierarchy).filter( - (h) => !forksOf || h.workspace.parent_workspace_id === forksOf - ) - : [] - ) + const forkRows = $derived(root ? workspaceSubtreeRows(root.id, hierarchy) : []) const forks = $derived(forkRows.map((h) => h.workspace)) // The family's canonical dev workspace, if any — still used for gating (a forking-locked root can be @@ -137,12 +127,14 @@ // Base a new fork gets by default: the dev workspace when the selection sits in its subtree, the // family root otherwise. const defaultForkBase = $derived( - forksOf ? root : findDefaultForkBase(effectiveId, forkableWorkspaces) + moveFrom + ? forkableWorkspaces.find((w) => w.id === moveFrom) + : findDefaultForkBase(effectiveId, forkableWorkspaces) ) const createForkLabel = 'Create new fork…' // Candidate bases ("targets") for a new fork: the root plus every fork/dev in the family, so a fork // can itself be the base — i.e. a fork of a fork. Root first, matching the list order below. - const baseOptions = $derived(root ? (forksOf ? [root] : [root, ...forks]) : []) + const baseOptions = $derived(root ? [root, ...forks] : []) // Options for the base-branch