From ab296e59c16fed3442528d828e3d58d44e94f330 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Fri, 11 Sep 2026 12:05:00 +0200 Subject: [PATCH] feat: save drafts by row id, so an open editor follows its draft through a move A rename carries every draft on the item to the new path. An editor left open across it was still saving by the path it opened on, which the server had to refuse and answer with where the item went (the "moved" handshake and its modal). The draft row has an id: the get-by-path overlay now returns it as draft_id, every later save sends it, and the server writes the row wherever it is and answers with that path. The editor then follows: it flushes what it holds, tells the user, and navigates to the item's new path, where the stale prompt says what changed. The lineage-based move resolvers, the moved status and the moved modal are gone. Co-Authored-By: Claude Fable 5.1 --- ...7a011dba4472741df120c2a556c87fe2825f.json} | 16 +- ...d927f3832b0de8b7a1b45223d09406c39a16.json} | 14 +- ...e7b48417a376412d7ab7ecd2a324a307897b7.json | 59 +++ ...08120b04670bf7261b2f5f5b61b1bb7fde5c8.json | 35 -- ...96c1ad11eab84d03dd8c435b824e2e1b03ff1.json | 35 -- ...af5d00d2fbcc50cb3615dacc85831a6fdf813.json | 35 -- ...ab5de58f58d3d7bb9f4cc94a98fd138c28114.json | 23 - ...8943c2b738ead4b870ef75c0d64a20bd83ecc.json | 23 - ...ebef6725238cea5ffbdc7a21b157b27d4c4dd.json | 23 - backend/tests/drafts_moved_rls.rs | 61 --- backend/tests/drafts_save_by_id.rs | 107 +++++ backend/tests/fixtures/drafts_moved_rls.sql | 30 -- backend/tests/fixtures/drafts_save_by_id.sql | 16 + backend/windmill-api/openapi.yaml | 26 +- backend/windmill-api/src/drafts.rs | 437 +++--------------- backend/windmill-common/src/user_drafts.rs | 12 +- .../src/lib/components/ScriptBuilder.svelte | 2 +- .../DraftEditorModals.svelte | 37 +- .../confirmationModal/DraftMovedModal.svelte | 149 ------ .../lib/components/usePageDraftSync.svelte.ts | 11 +- frontend/src/lib/userDraftDbMigration.ts | 30 -- frontend/src/lib/userDraftDbSyncer.svelte.ts | 104 ++--- .../(logged)/apps/edit/[...path]/+page.svelte | 3 +- .../apps_raw/edit/[...path]/+page.svelte | 5 +- .../flows/edit/[...path]/+page.svelte | 6 +- .../scripts/edit/[...path]/+page.svelte | 6 +- 26 files changed, 413 insertions(+), 892 deletions(-) rename backend/.sqlx/{query-d3686c73788b866090f25383da507e9a7d9f90d5f86fa4a1b89026cb088d0a58.json => query-0a8e315e6a808c9a8704d5cbdc607a011dba4472741df120c2a556c87fe2825f.json} (83%) rename backend/.sqlx/{query-0cc6770a81ecaecafe0d9b7100f94b329c4ec7978b016af4c964415e0c38396c.json => query-10e85bd008777fe980bba2515ce7d927f3832b0de8b7a1b45223d09406c39a16.json} (74%) create mode 100644 backend/.sqlx/query-14a475e8d090d3caca0b76ab3b2e7b48417a376412d7ab7ecd2a324a307897b7.json delete mode 100644 backend/.sqlx/query-189a8c5a3ac2d9ece750aa1e01c08120b04670bf7261b2f5f5b61b1bb7fde5c8.json delete mode 100644 backend/.sqlx/query-323f975428149815f3a9bc5c47396c1ad11eab84d03dd8c435b824e2e1b03ff1.json delete mode 100644 backend/.sqlx/query-394fea4bb407ef3f800df87032aaf5d00d2fbcc50cb3615dacc85831a6fdf813.json delete mode 100644 backend/.sqlx/query-9ab0ebbe787d71d8a36618e8fbcab5de58f58d3d7bb9f4cc94a98fd138c28114.json delete mode 100644 backend/.sqlx/query-c451a6d05b4aeb21ecbe29a355b8943c2b738ead4b870ef75c0d64a20bd83ecc.json delete mode 100644 backend/.sqlx/query-d5f06d5bca13ae5a22ef1290de4ebef6725238cea5ffbdc7a21b157b27d4c4dd.json delete mode 100644 backend/tests/drafts_moved_rls.rs create mode 100644 backend/tests/drafts_save_by_id.rs delete mode 100644 backend/tests/fixtures/drafts_moved_rls.sql create mode 100644 backend/tests/fixtures/drafts_save_by_id.sql delete mode 100644 frontend/src/lib/components/common/confirmationModal/DraftMovedModal.svelte diff --git a/backend/.sqlx/query-d3686c73788b866090f25383da507e9a7d9f90d5f86fa4a1b89026cb088d0a58.json b/backend/.sqlx/query-0a8e315e6a808c9a8704d5cbdc607a011dba4472741df120c2a556c87fe2825f.json similarity index 83% rename from backend/.sqlx/query-d3686c73788b866090f25383da507e9a7d9f90d5f86fa4a1b89026cb088d0a58.json rename to backend/.sqlx/query-0a8e315e6a808c9a8704d5cbdc607a011dba4472741df120c2a556c87fe2825f.json index f4edb8a807..3cef79cd00 100644 --- a/backend/.sqlx/query-d3686c73788b866090f25383da507e9a7d9f90d5f86fa4a1b89026cb088d0a58.json +++ b/backend/.sqlx/query-0a8e315e6a808c9a8704d5cbdc607a011dba4472741df120c2a556c87fe2825f.json @@ -1,10 +1,20 @@ { "db_name": "PostgreSQL", - "query": "INSERT INTO draft (workspace_id, email, path, typ, value, created_at, base)\n VALUES ($1, $2, $3, $4, $5::text::json, COALESCE($8::timestamptz, now()), $9)\n ON CONFLICT (workspace_id, path, typ, email) WHERE email IS NOT NULL\n DO UPDATE SET value = EXCLUDED.value, created_at = EXCLUDED.created_at,\n base = EXCLUDED.base\n WHERE $7::bool = true\n OR $6::timestamptz IS NULL\n OR draft.created_at <= $6::timestamptz\n RETURNING created_at", + "query": "INSERT INTO draft (workspace_id, email, path, typ, value, created_at, base)\n VALUES ($1, $2, $3, $4, $5::text::json, COALESCE($8::timestamptz, now()), $9)\n ON CONFLICT (workspace_id, path, typ, email) WHERE email IS NOT NULL\n DO UPDATE SET value = EXCLUDED.value, created_at = EXCLUDED.created_at,\n base = EXCLUDED.base\n WHERE $7::bool = true\n OR $6::timestamptz IS NULL\n OR draft.created_at <= $6::timestamptz\n RETURNING id, path, created_at", "describe": { "columns": [ { "ordinal": 0, + "name": "id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "path", + "type_info": "Varchar" + }, + { + "ordinal": 2, "name": "created_at", "type_info": "Timestamptz" } @@ -57,8 +67,10 @@ ] }, "nullable": [ + false, + false, false ] }, - "hash": "d3686c73788b866090f25383da507e9a7d9f90d5f86fa4a1b89026cb088d0a58" + "hash": "0a8e315e6a808c9a8704d5cbdc607a011dba4472741df120c2a556c87fe2825f" } diff --git a/backend/.sqlx/query-0cc6770a81ecaecafe0d9b7100f94b329c4ec7978b016af4c964415e0c38396c.json b/backend/.sqlx/query-10e85bd008777fe980bba2515ce7d927f3832b0de8b7a1b45223d09406c39a16.json similarity index 74% rename from backend/.sqlx/query-0cc6770a81ecaecafe0d9b7100f94b329c4ec7978b016af4c964415e0c38396c.json rename to backend/.sqlx/query-10e85bd008777fe980bba2515ce7d927f3832b0de8b7a1b45223d09406c39a16.json index 23d3b51c76..8c35c5eaa3 100644 --- a/backend/.sqlx/query-0cc6770a81ecaecafe0d9b7100f94b329c4ec7978b016af4c964415e0c38396c.json +++ b/backend/.sqlx/query-10e85bd008777fe980bba2515ce7d927f3832b0de8b7a1b45223d09406c39a16.json @@ -1,20 +1,25 @@ { "db_name": "PostgreSQL", - "query": "SELECT value as \"value!: sqlx::types::Json>\",\n created_at, base\n FROM draft\n WHERE workspace_id = $1\n AND (email = $2 OR email IS NULL)\n AND path = $3\n AND typ = $4\n ORDER BY email NULLS LAST\n LIMIT 1", + "query": "SELECT id, value as \"value!: sqlx::types::Json>\",\n created_at, base\n FROM draft\n WHERE workspace_id = $1\n AND (email = $2 OR email IS NULL)\n AND path = $3\n AND typ = $4\n ORDER BY email NULLS LAST\n LIMIT 1", "describe": { "columns": [ { "ordinal": 0, + "name": "id", + "type_info": "Int8" + }, + { + "ordinal": 1, "name": "value!: sqlx::types::Json>", "type_info": "Json" }, { - "ordinal": 1, + "ordinal": 2, "name": "created_at", "type_info": "Timestamptz" }, { - "ordinal": 2, + "ordinal": 3, "name": "base", "type_info": "Text" } @@ -62,10 +67,11 @@ ] }, "nullable": [ + false, false, false, true ] }, - "hash": "0cc6770a81ecaecafe0d9b7100f94b329c4ec7978b016af4c964415e0c38396c" + "hash": "10e85bd008777fe980bba2515ce7d927f3832b0de8b7a1b45223d09406c39a16" } diff --git a/backend/.sqlx/query-14a475e8d090d3caca0b76ab3b2e7b48417a376412d7ab7ecd2a324a307897b7.json b/backend/.sqlx/query-14a475e8d090d3caca0b76ab3b2e7b48417a376412d7ab7ecd2a324a307897b7.json new file mode 100644 index 0000000000..bc3b154d84 --- /dev/null +++ b/backend/.sqlx/query-14a475e8d090d3caca0b76ab3b2e7b48417a376412d7ab7ecd2a324a307897b7.json @@ -0,0 +1,59 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT path FROM draft WHERE id = $1 AND workspace_id = $2 AND typ = $3 AND email = $4", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "path", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Int8", + "Text", + { + "Custom": { + "name": "draft_kind", + "kind": { + "Enum": [ + "script", + "flow", + "app", + "raw_app", + "resource", + "variable", + "trigger_schedule", + "trigger_webhook", + "trigger_default_email", + "trigger_email", + "trigger_http", + "trigger_websocket", + "trigger_postgres", + "trigger_kafka", + "trigger_nats", + "trigger_mqtt", + "trigger_sqs", + "trigger_gcp", + "trigger_azure", + "trigger_poll", + "trigger_cli", + "trigger_nextcloud", + "trigger_google", + "trigger_github", + "data_pipeline", + "trigger_amqp" + ] + } + } + }, + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "14a475e8d090d3caca0b76ab3b2e7b48417a376412d7ab7ecd2a324a307897b7" +} diff --git a/backend/.sqlx/query-189a8c5a3ac2d9ece750aa1e01c08120b04670bf7261b2f5f5b61b1bb7fde5c8.json b/backend/.sqlx/query-189a8c5a3ac2d9ece750aa1e01c08120b04670bf7261b2f5f5b61b1bb7fde5c8.json deleted file mode 100644 index 9fa0e33c2d..0000000000 --- a/backend/.sqlx/query-189a8c5a3ac2d9ece750aa1e01c08120b04670bf7261b2f5f5b61b1bb7fde5c8.json +++ /dev/null @@ -1,35 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT path, created_by, hash FROM script\n WHERE workspace_id = $1 AND $2 = ANY(parent_hashes)\n AND NOT archived AND NOT deleted\n ORDER BY created_at DESC LIMIT 1", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "path", - "type_info": "Varchar" - }, - { - "ordinal": 1, - "name": "created_by", - "type_info": "Varchar" - }, - { - "ordinal": 2, - "name": "hash", - "type_info": "Int8" - } - ], - "parameters": { - "Left": [ - "Text", - "Int8" - ] - }, - "nullable": [ - false, - false, - false - ] - }, - "hash": "189a8c5a3ac2d9ece750aa1e01c08120b04670bf7261b2f5f5b61b1bb7fde5c8" -} diff --git a/backend/.sqlx/query-323f975428149815f3a9bc5c47396c1ad11eab84d03dd8c435b824e2e1b03ff1.json b/backend/.sqlx/query-323f975428149815f3a9bc5c47396c1ad11eab84d03dd8c435b824e2e1b03ff1.json deleted file mode 100644 index 53f0ca1d34..0000000000 --- a/backend/.sqlx/query-323f975428149815f3a9bc5c47396c1ad11eab84d03dd8c435b824e2e1b03ff1.json +++ /dev/null @@ -1,35 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT a.path, head.id as \"head!\", head.created_by as \"head_by!\"\n FROM app_version av\n JOIN app a ON a.id = av.app_id\n JOIN LATERAL (\n SELECT id, created_by FROM app_version\n WHERE app_id = a.id ORDER BY created_at DESC LIMIT 1\n ) head ON true\n WHERE av.id = $2 AND a.workspace_id = $1", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "path", - "type_info": "Varchar" - }, - { - "ordinal": 1, - "name": "head!", - "type_info": "Int8" - }, - { - "ordinal": 2, - "name": "head_by!", - "type_info": "Varchar" - } - ], - "parameters": { - "Left": [ - "Text", - "Int8" - ] - }, - "nullable": [ - false, - false, - false - ] - }, - "hash": "323f975428149815f3a9bc5c47396c1ad11eab84d03dd8c435b824e2e1b03ff1" -} diff --git a/backend/.sqlx/query-394fea4bb407ef3f800df87032aaf5d00d2fbcc50cb3615dacc85831a6fdf813.json b/backend/.sqlx/query-394fea4bb407ef3f800df87032aaf5d00d2fbcc50cb3615dacc85831a6fdf813.json deleted file mode 100644 index 55d9a4458a..0000000000 --- a/backend/.sqlx/query-394fea4bb407ef3f800df87032aaf5d00d2fbcc50cb3615dacc85831a6fdf813.json +++ /dev/null @@ -1,35 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT fv.path, f.edited_by, f.versions[array_upper(f.versions, 1)] as \"head!\"\n FROM flow_version fv\n JOIN flow f ON f.workspace_id = fv.workspace_id AND f.path = fv.path\n WHERE fv.id = $2 AND fv.workspace_id = $1", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "path", - "type_info": "Varchar" - }, - { - "ordinal": 1, - "name": "edited_by", - "type_info": "Varchar" - }, - { - "ordinal": 2, - "name": "head!", - "type_info": "Int8" - } - ], - "parameters": { - "Left": [ - "Text", - "Int8" - ] - }, - "nullable": [ - false, - false, - null - ] - }, - "hash": "394fea4bb407ef3f800df87032aaf5d00d2fbcc50cb3615dacc85831a6fdf813" -} diff --git a/backend/.sqlx/query-9ab0ebbe787d71d8a36618e8fbcab5de58f58d3d7bb9f4cc94a98fd138c28114.json b/backend/.sqlx/query-9ab0ebbe787d71d8a36618e8fbcab5de58f58d3d7bb9f4cc94a98fd138c28114.json deleted file mode 100644 index eead73a429..0000000000 --- a/backend/.sqlx/query-9ab0ebbe787d71d8a36618e8fbcab5de58f58d3d7bb9f4cc94a98fd138c28114.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT EXISTS(SELECT 1 FROM app WHERE workspace_id = $1 AND path = $2) as \"e!\"", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "e!", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "9ab0ebbe787d71d8a36618e8fbcab5de58f58d3d7bb9f4cc94a98fd138c28114" -} diff --git a/backend/.sqlx/query-c451a6d05b4aeb21ecbe29a355b8943c2b738ead4b870ef75c0d64a20bd83ecc.json b/backend/.sqlx/query-c451a6d05b4aeb21ecbe29a355b8943c2b738ead4b870ef75c0d64a20bd83ecc.json deleted file mode 100644 index ab97a26990..0000000000 --- a/backend/.sqlx/query-c451a6d05b4aeb21ecbe29a355b8943c2b738ead4b870ef75c0d64a20bd83ecc.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT EXISTS(SELECT 1 FROM flow WHERE workspace_id = $1 AND path = $2) as \"e!\"", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "e!", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "c451a6d05b4aeb21ecbe29a355b8943c2b738ead4b870ef75c0d64a20bd83ecc" -} diff --git a/backend/.sqlx/query-d5f06d5bca13ae5a22ef1290de4ebef6725238cea5ffbdc7a21b157b27d4c4dd.json b/backend/.sqlx/query-d5f06d5bca13ae5a22ef1290de4ebef6725238cea5ffbdc7a21b157b27d4c4dd.json deleted file mode 100644 index 9afa90e619..0000000000 --- a/backend/.sqlx/query-d5f06d5bca13ae5a22ef1290de4ebef6725238cea5ffbdc7a21b157b27d4c4dd.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT EXISTS(SELECT 1 FROM script WHERE workspace_id = $1 AND path = $2\n AND NOT archived AND NOT deleted) as \"e!\"", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "e!", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "d5f06d5bca13ae5a22ef1290de4ebef6725238cea5ffbdc7a21b157b27d4c4dd" -} diff --git a/backend/tests/drafts_moved_rls.rs b/backend/tests/drafts_moved_rls.rs deleted file mode 100644 index 1af90e0840..0000000000 --- a/backend/tests/drafts_moved_rls.rs +++ /dev/null @@ -1,61 +0,0 @@ -//! `POST /drafts/update` must never name a destination the caller cannot see. -//! -//! When an item moves, the saver's editor is still bound to the old path and its -//! next save is answered with `status: "moved"` plus the new path and the mover's -//! username. That answer is resolved twice — once before the write, once after it -//! on the write's own connection — and both must read under RLS. A post-write -//! re-assert on a raw pool connection reads rows the pre-check cannot see, so it -//! discloses the destination and permanently refuses a save that should land. - -use serde_json::{json, Value}; -use sqlx::{Pool, Postgres}; - -use windmill_test_utils::*; - -/// Hex form of script hash 7001, the way a script draft stores `parent_hash`. -const OLD_HASH: &str = "0000000000001b59"; - -async fn save_draft(port: u16, token: &str) -> anyhow::Result { - Ok(reqwest::Client::new() - .post(format!( - "http://localhost:{port}/api/w/test-workspace/drafts/update/script/f/mvrls_visible/s1" - )) - .header("Authorization", format!("Bearer {token}")) - .json(&json!({ - "value": { - "path": "f/mvrls_visible/s1", - "parent_hash": OLD_HASH, - "content": "export function main() { return 2 }", - "language": "deno", - "summary": "S1", - "description": "", - "schema": {} - } - })) - .send() - .await? - .json() - .await?) -} - -#[sqlx::test(fixtures("base", "drafts_moved_rls"))] -async fn test_moved_answer_is_rls_scoped(db: Pool) -> anyhow::Result<()> { - initialize_tracing().await; - let server = ApiServer::start(db.clone()).await?; - let port = server.addr.port(); - - // test-user-2 writes in `mvrls_visible` but has nothing on `mvrls_secret`. - // They must be told the save landed, not where the item went. - let res = save_draft(port, "SECRET_TOKEN_2").await?; - assert_eq!(res["status"], "saved", "non-admin save was refused: {res}"); - assert_eq!(res["moved_to"], Value::Null, "destination disclosed: {res}"); - assert_eq!(res["moved_by"], Value::Null, "mover disclosed: {res}"); - - // The admin sees the destination, so they get the real answer. - let res = save_draft(port, "SECRET_TOKEN").await?; - assert_eq!(res["status"], "moved", "admin was not told it moved: {res}"); - assert_eq!(res["moved_to"], "f/mvrls_secret/s1"); - assert_eq!(res["moved_by"], "test-user"); - - Ok(()) -} diff --git a/backend/tests/drafts_save_by_id.rs b/backend/tests/drafts_save_by_id.rs new file mode 100644 index 0000000000..c1484fc89f --- /dev/null +++ b/backend/tests/drafts_save_by_id.rs @@ -0,0 +1,107 @@ +//! A draft saved by row id lands where the row is, not where the editor was. +//! +//! A rename carries every draft on the item to the new path. An editor left open +//! across it is still bound to the old path; saving by the row's id writes at the +//! item's current path and the response names it, so the editor can follow. A +//! save by path would instead have planted a phantom draft at the old location. + +use serde_json::{json, Value}; +use sqlx::{Pool, Postgres}; + +use windmill_test_utils::*; + +/// Hex form of script hash 7030, the way the API takes a parent hash. +const HEAD_HASH: &str = "0000000000001b76"; +const DRAFT_ID: i64 = 9001; + +async fn own_draft_paths(port: u16) -> anyhow::Result> { + let list: Vec = reqwest::Client::new() + .get(format!( + "http://localhost:{port}/api/w/test-workspace/drafts/list" + )) + .header("Authorization", "Bearer SECRET_TOKEN") + .send() + .await? + .json() + .await?; + Ok(list + .iter() + .filter(|d| d["kind"] == "script") + .filter_map(|d| d["path"].as_str().map(String::from)) + .filter(|p| p.starts_with("u/test-user/byid_")) + .collect()) +} + +#[sqlx::test(fixtures("base", "drafts_save_by_id"))] +async fn test_save_by_id_follows_a_rename(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let client = reqwest::Client::new(); + + // Rename from Home: a redeploy of the deployed content at the new path that + // keeps the deployer's own draft, so the draft is carried rather than consumed. + let resp = client + .post(format!( + "http://localhost:{port}/api/w/test-workspace/scripts/create" + )) + .header("Authorization", "Bearer SECRET_TOKEN") + .json(&json!({ + "path": "u/test-user/byid_b", + "parent_hash": HEAD_HASH, + "summary": "A", + "description": "", + "content": "export function main() { return 1 }", + "language": "deno", + "schema": {}, + "skip_draft_deletion": true + })) + .send() + .await?; + assert_eq!(resp.status(), 201, "rename failed: {}", resp.text().await?); + assert_eq!(own_draft_paths(port).await?, vec!["u/test-user/byid_b"]); + + // The editor is still on the old path but saves by id. + let saved: Value = client + .post(format!( + "http://localhost:{port}/api/w/test-workspace/drafts/update/script/u/test-user/byid_a" + )) + .header("Authorization", "Bearer SECRET_TOKEN") + .json(&json!({ + "id": DRAFT_ID, + "value": { + "path": "u/test-user/byid_a", + "parent_hash": HEAD_HASH, + "summary": "A", + "content": "edited after the move" + } + })) + .send() + .await? + .json() + .await?; + assert_eq!(saved["status"], "saved", "save refused: {saved}"); + assert_eq!(saved["id"], DRAFT_ID); + assert_eq!( + saved["path"], "u/test-user/byid_b", + "save did not follow the row: {saved}" + ); + + // The write landed on the carried row; nothing reappeared at the old path. + assert_eq!(own_draft_paths(port).await?, vec!["u/test-user/byid_b"]); + let draft: Value = client + .get(format!( + "http://localhost:{port}/api/w/test-workspace/drafts/get_own/script/u/test-user/byid_b" + )) + .header("Authorization", "Bearer SECRET_TOKEN") + .send() + .await? + .json() + .await?; + assert_eq!( + draft["value"]["content"], "edited after the move", + "{draft}" + ); + + Ok(()) +} diff --git a/backend/tests/fixtures/drafts_moved_rls.sql b/backend/tests/fixtures/drafts_moved_rls.sql deleted file mode 100644 index 3dea7a8d64..0000000000 --- a/backend/tests/fixtures/drafts_moved_rls.sql +++ /dev/null @@ -1,30 +0,0 @@ --- Fixture for the "moved" draft answer under RLS. --- --- Models a script that has already been moved out of a folder the saver can --- reach (`mvrls_visible`, where test-user-2 is a writer) and into one they have --- no permission on (`mvrls_secret`). A script move goes through `create_script`, --- which archives the row in place and inserts a successor carrying the old hash --- in `parent_hashes` — the state reproduced here. - -INSERT INTO folder (workspace_id, name, display_name, owners, extra_perms, created_by) -VALUES ('test-workspace', 'mvrls_visible', 'Visible', '{"u/test-user"}', - '{"u/test-user": true, "u/test-user-2": true}', 'test-user'); - --- No entry for test-user-2: the destination is invisible to them. -INSERT INTO folder (workspace_id, name, display_name, owners, extra_perms, created_by) -VALUES ('test-workspace', 'mvrls_secret', 'Secret', '{"u/test-user"}', - '{"u/test-user": true}', 'test-user'); - --- The pre-move row, archived in place at the old path. Hash 7001 = 0x1b59. -INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by, - schema, summary, description, lock, extra_perms, archived) -VALUES ('test-workspace', 7001, 'f/mvrls_visible/s1', - 'export function main() { return 1 }', - 'deno', 'script', 'test-user', '{}', 'S1', '', '', '{}', true); - --- The post-move row, at the destination, pointing back at 7001. -INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by, - schema, summary, description, lock, extra_perms, parent_hashes) -VALUES ('test-workspace', 7002, 'f/mvrls_secret/s1', - 'export function main() { return 1 }', - 'deno', 'script', 'test-user', '{}', 'S1', '', '', '{}', '{7001}'); diff --git a/backend/tests/fixtures/drafts_save_by_id.sql b/backend/tests/fixtures/drafts_save_by_id.sql new file mode 100644 index 0000000000..10759cb45a --- /dev/null +++ b/backend/tests/fixtures/drafts_save_by_id.sql @@ -0,0 +1,16 @@ +-- Fixture for saving a draft by row id across a rename. +-- +-- A deployed script at `u/test-user/byid_a` (hash 7030 = 0x1b76) with the +-- deployer's own draft on it. The test renames the script and then saves the +-- draft by id from an editor still bound to the old path. + +INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by, + schema, summary, description, lock, extra_perms) +VALUES ('test-workspace', 7030, 'u/test-user/byid_a', + 'export function main() { return 1 }', + 'deno', 'script', 'test-user', '{}', 'A', '', '', '{}'); + +INSERT INTO draft (id, workspace_id, path, typ, value, email, base) +VALUES (9001, 'test-workspace', 'u/test-user/byid_a', 'script', + '{"path": "u/test-user/byid_a", "parent_hash": "0000000000001b76", "summary": "A", "content": "draft"}', + 'test@windmill.dev', '0000000000001b76'); diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index f62abc708e..436fee424f 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -9430,6 +9430,10 @@ paths: type: string format: date-time description: Upsert-only override for the stored creation timestamp. Normal saves omit it (stamped server-side); the localStorage→DB migration passes the draft's original write time so migrated drafts keep their age. + id: + type: integer + format: int64 + description: The draft row to write, from a previous save's `id` or the get-by-path `draft_id`. Addresses the row wherever a move took it; the URL path only applies when the id names no row of the caller's any more. responses: "200": description: save result @@ -9440,21 +9444,17 @@ paths: properties: status: type: string - enum: [saved, conflict, moved] - description: "`moved`: the item left this path, so nothing was written — see `moved_to`." + enum: [saved, conflict] current_timestamp: type: string format: date-time - moved_to: + id: + type: integer + format: int64 + description: "`saved` upserts only: the row's id, to save by from now on." + path: type: string - description: "`moved` only: the path the item lives at now." - moved_by: - type: string - description: "`moved` only: who last deployed it there. Best-effort." - moved_patch: - type: object - additionalProperties: true - description: "`moved` only: fields to merge into the refused draft before re-saving it at `moved_to` — the typed target path and the version the item now sits at. Sent as a patch so the client does not reproduce the per-kind key names." + description: "`saved` upserts only: where the row is. Differs from the URL path once a move has carried the row elsewhere; the editor follows it there." required: [status, current_timestamp] /w/{workspace}/drafts/move/{kind}/{path}: @@ -26105,6 +26105,10 @@ components: draft_saved_at: type: string format: date-time + draft_id: + type: integer + format: int64 + description: The draft row's id. Pass it as `id` on updateDraft so saves follow the row through a move. draft_base: type: string description: | diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index 5fe98559a5..cd6d964416 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -14,11 +14,9 @@ use axum::{ Json, Router, }; use serde::{Deserialize, Serialize}; -use serde_json::json; use windmill_common::{ db::UserDB, error::{Error, Result}, - scripts::ScriptHash, user_drafts::{DraftUserRef, UserDraftItemKind, ENCRYPTED_DRAFT_PREFIX}, users::resolve_username_to_email, utils::{check_proper_path, strip_json_nul}, @@ -314,6 +312,11 @@ pub struct SaveDraftRequest { /// keep their age instead of all resurfacing to the top as freshly created. #[serde(default)] pub created_at: Option>, + /// The draft row to write, when the client has saved or loaded it before. + /// Addresses the row wherever a move took it; the URL path is only the + /// fallback when the id names no row of the caller's any more. + #[serde(default)] + pub id: Option, } #[derive(Serialize, Debug)] @@ -321,10 +324,6 @@ pub struct SaveDraftRequest { pub enum SaveDraftStatus { Saved, Conflict, - /// The item this draft belongs to was moved away from this path. Nothing - /// was written — writing would plant a phantom draft-only item at a path - /// the item has left. - Moved, } #[derive(Serialize, Debug)] @@ -332,20 +331,14 @@ pub struct SaveDraftResponse { pub status: SaveDraftStatus, /// On `saved`: when the change was applied (client remembers it as the /// next `last_sync`). On `conflict`: the existing row's `created_at`. - /// On `moved`: the server's now(). pub current_timestamp: chrono::DateTime, - /// `moved` only: where the item lives now. + /// `saved` upserts only: the row's id, to save by from now on. #[serde(skip_serializing_if = "Option::is_none")] - pub moved_to: Option, - /// `moved` only: who last deployed it at its new path. Best-effort. + pub id: Option, + /// `saved` upserts only: where the row is. Differs from the URL path once a + /// move has carried the row elsewhere; the editor follows it there. #[serde(skip_serializing_if = "Option::is_none")] - pub moved_by: Option, - /// `moved` only: the path keys to merge into the refused draft before - /// re-saving it at `moved_to`. Sent as a patch so the client never has to - /// reproduce the per-kind key names (`UserDraftItemKind::typed_path_field` - /// and `mirror_path_field`). - #[serde(skip_serializing_if = "Option::is_none")] - pub moved_patch: Option, + pub path: Option, } /// The version a draft forked from, as the editors write it into `draft.value`. @@ -377,11 +370,9 @@ impl DraftBaseVersion { } } -/// The version this draft forked from, or `None` when it has no lineage to -/// follow — a kind that keeps none, a malformed payload, or a draft that was -/// never forked from a deploy. Pure: no queries. This is the escape hatch the -/// autosave path is built around, so every caller that is about to spend a -/// round-trip on move detection should consult it first. +/// The version this draft forked from, or `None` when it has none — a kind +/// that keeps no lineage, a malformed payload, or a draft that was never forked +/// from a deploy. Pure: no queries. fn draft_lineage(kind: UserDraftItemKind, value: &str) -> Option { use UserDraftItemKind::*; if !matches!(kind, Script | Flow | App | RawApp) { @@ -400,228 +391,15 @@ fn draft_lineage(kind: UserDraftItemKind, value: &str) -> Option Result, serde_json::Value)>> { - // `UserDB::begin` is not a bare BEGIN — it also issues `set_session_context` - // and, under `PG_SCHEMA`, `SET LOCAL search_path`, so this wraps one indexed - // existence check in 3-4 round-trips. Callers keep that off a draft with - // nothing to follow by having no `DraftBaseVersion` to pass. - let mut tx = user_db.clone().begin(authed).await?; - let moved = resolve_moved_to_in(&mut tx, w_id, kind, path, base).await; - tx.commit().await?; - moved -} - -/// The body of `resolve_moved_to`, on a caller-supplied transaction. Split out -/// so the post-write re-assert can reuse the connection it already holds rather -/// than acquiring a second one from the same pool while holding an open -/// transaction — that pattern stalls under pool pressure. The transaction it is -/// handed must be RLS-scoped: it reports a path and a username the caller may -/// have no access to, and passing the write gate at the old path says nothing -/// about what the caller may see at the new one. -async fn resolve_moved_to_in( - tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, - w_id: &str, - kind: UserDraftItemKind, - path: &str, - base: &DraftBaseVersion, -) -> Result, serde_json::Value)>> { - use UserDraftItemKind::*; - let script_hash = base - .parent_hash - .as_deref() - .and_then(|h| windmill_common::scripts::to_i64(h).ok()); - - let moved = match kind { - Script => { - // An archived row keeps sitting at the old path, so a plain - // existence check would miss every script move. - let still_here = sqlx::query_scalar!( - r#"SELECT EXISTS(SELECT 1 FROM script WHERE workspace_id = $1 AND path = $2 - AND NOT archived AND NOT deleted) as "e!""#, - w_id, - path, - ) - .fetch_one(&mut **tx) - .await?; - if still_here { - None - } else { - // Only reached once the item is gone. `parent_hashes` carries no - // index, so this is a workspace-wide scan — keeping it behind the - // cheap check above is what leaves the autosave hot path at one - // indexed lookup. - sqlx::query!( - r#"SELECT path, created_by, hash FROM script - WHERE workspace_id = $1 AND $2 = ANY(parent_hashes) - AND NOT archived AND NOT deleted - ORDER BY created_at DESC LIMIT 1"#, - w_id, - script_hash, - ) - .fetch_optional(&mut **tx) - .await? - // Hex text, the way the API serializes a hash and the way a - // script draft stores `parent_hash`. - .map(|r| { - ( - r.path, - Some(r.created_by), - json!(ScriptHash(r.hash).to_string()), - ) - }) - } - } - Flow => { - let still_here = sqlx::query_scalar!( - r#"SELECT EXISTS(SELECT 1 FROM flow WHERE workspace_id = $1 AND path = $2) as "e!""#, - w_id, - path, - ) - .fetch_one(&mut **tx) - .await?; - if still_here { - None - } else { - sqlx::query!( - r#"SELECT fv.path, f.edited_by, f.versions[array_upper(f.versions, 1)] as "head!" - FROM flow_version fv - JOIN flow f ON f.workspace_id = fv.workspace_id AND f.path = fv.path - WHERE fv.id = $2 AND fv.workspace_id = $1"#, - w_id, - base.version_id, - ) - .fetch_optional(&mut **tx) - .await? - .map(|r| (r.path, Some(r.edited_by), json!(r.head))) - } - } - App | RawApp => { - let still_here = sqlx::query_scalar!( - r#"SELECT EXISTS(SELECT 1 FROM app WHERE workspace_id = $1 AND path = $2) as "e!""#, - w_id, - path, - ) - .fetch_one(&mut **tx) - .await?; - if still_here { - None - } else { - // `created_by` must come from the HEAD row, not from `av` — `av` - // is the version the draft forked from, whose author is usually - // the person now reading this. Naming them would make - // `moved_by_me` true for the wrong user and restamp a draft that - // has never seen the head's content. - sqlx::query!( - r#"SELECT a.path, head.id as "head!", head.created_by as "head_by!" - FROM app_version av - JOIN app a ON a.id = av.app_id - JOIN LATERAL ( - SELECT id, created_by FROM app_version - WHERE app_id = a.id ORDER BY created_at DESC LIMIT 1 - ) head ON true - WHERE av.id = $2 AND a.workspace_id = $1"#, - w_id, - base.parent_version, - ) - .fetch_optional(&mut **tx) - .await? - .map(|r| (r.path, Some(r.head_by), json!(r.head))) - } - } - _ => None, - }; - - // Same path back ⇒ nothing moved (a stale read, or a path reused). - let moved = moved.filter(|(new_path, _, _)| new_path != path); - - // "Continue at the new path" is the user explicitly relocating their edits, so - // both path keys are set to the destination — unlike the passive carry, which - // leaves the value alone because it is not the user's action. The version is - // NOT restamped: the draft really is behind the version the move created, and - // the editor's stale prompt shows the diff that says whether that matters. - // `create_missing` semantics are the client's: it merges this over the value, - // so a key the draft never had is added, which is what relocating means here. - Ok(moved.map(|(new_path, new_by, _head)| { - let mut patch = serde_json::Map::new(); - for field in [kind.typed_path_field(), kind.mirror_path_field()] - .into_iter() - .flatten() - { - patch.insert(field.to_string(), json!(&new_path)); - } - (new_path, new_by, serde_json::Value::Object(patch)) - })) -} - -/// Is the item still deployed at `path`? One indexed existence check, used to -/// NARROW — not close — the window between the `moved` pre-check and the write -/// that follows it. A residual remains: the deploy that moves an item calls -/// `move_drafts_for_path` inside its own transaction and then does more work -/// before committing, so a first save with no prior row at the old path can read -/// the pre-move snapshot under READ COMMITTED, take no lock, and commit a stray -/// row. That row is bounded — the editor's next autosave hits the pre-check and -/// is told the item moved. A save that DOES have a row there serialises behind -/// the mover's own UPDATE on that tuple and detects the move correctly. -/// Runs on the write's own RLS-scoped connection. An item hidden by RLS reads as -/// `false` here, which asks `resolve_moved_to_in`, which answers `None`, so the -/// save lands — the same outcome as the `true` this would return if the row were -/// visible and unmoved. -/// -/// `false` covers three different situations — moved, deleted, and a genuinely -/// draft-only item that never had a deployed row — so it is only ever a cue to -/// ask `resolve_moved_to`, never a verdict on its own. -async fn deployed_still_at( - tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, - w_id: &str, - kind: UserDraftItemKind, - path: &str, -) -> Result { - let Some(table) = kind.deployed_table() else { - return Ok(false); - }; - // A script move archives its parent in place, so the row stays at the old - // path and a bare existence check would report "still here" for every script - // move. `table` comes from the closed `deployed_table()` enum, never input. - let archived_filter = if table == "script" { - " AND NOT archived AND NOT deleted" - } else { - "" - }; - let q = format!( - "SELECT EXISTS(SELECT 1 FROM {table} WHERE workspace_id = $1 AND path = $2{archived_filter})" - ); - Ok(sqlx::query_scalar::<_, bool>(&q) - .bind(w_id) - .bind(path) - .fetch_one(&mut **tx) - .await?) -} - -/// Apply the current user's draft at (workspace, kind, path): non-null `value` -/// upserts, `null` (or omitted) deletes. Either way, when the existing row is -/// newer than `last_sync` (and `force` is false) the op is skipped and the -/// response is `status = conflict` + the server's current timestamp. +/// The row is addressed by `id` when the client has one, and by the URL path +/// otherwise. An id follows the row wherever a move took it, so an editor left +/// open across a rename writes at the item's current path instead of planting +/// a phantom draft at the one it left; the response names that path. async fn update_draft( authed: ApiAuthed, Extension(db): Extension, @@ -630,7 +408,7 @@ async fn update_draft( Json(req): Json, ) -> Result> { let email = &authed.email; - let path = path.to_path(); + let url_path = path.to_path(); // Saving a draft requires write permission on the underlying path. Deleting // (discarding) one's OWN draft does not: the email-scoped row belongs to the // authed user, so they can always discard it even after losing write access @@ -640,57 +418,47 @@ async fn update_draft( // — they keep the write gate. let is_own_discard = req.value.is_none() && !req.legacy; - // Rejected before the moved branch below, not with the rest of the write gate - // after it. An operator can never save a draft, so a `moved` answer is no use - // to them — and letting them past would hand a read-only role the unindexed - // `parent_hashes` lineage scan, repeatable with any fabricated base version. if !is_own_discard && authed.is_operator { return Err(Error::NotAuthorized( "operators cannot save drafts".to_string(), )); } - // An editor left open across someone else's move is still bound to the old - // path and would re-plant its draft there. Refuse and answer with where the - // item went; the carried draft is already waiting at the new path. - // - // Answered BEFORE the write gate, and deliberately: that gate resolves against - // the OLD path, where a move has left no deployed row, so a collaborator whose - // write access came from the item's own `extra_perms` would be told - // "unauthorized" for an item they still have permission on — and never learn - // where it went. Nothing is written on this branch, and `resolve_moved_to` - // reads under RLS, so it can only name an item the caller can already see. - // - // Parsed once here and threaded to all three sites that need it: an app draft - // runs to hundreds of KB, and serde tokenizes the whole document even to skip - // the keys it does not want. - let lineage = req - .value - .as_ref() - .and_then(|value| draft_lineage(kind, value.0.get())); - if let Some(base) = &lineage { - if let Some((moved_to, moved_by, moved_patch)) = - resolve_moved_to(&authed, &user_db, &w_id, kind, path, base).await? - { - let now = sqlx::query_scalar!(r#"SELECT now() as "now!""#) - .fetch_one(&db) - .await?; - return Ok(Json(SaveDraftResponse { - status: SaveDraftStatus::Moved, - current_timestamp: now, - moved_to: Some(moved_to), - moved_by, - moved_patch: Some(moved_patch), - })); + // Where the row is now. `None` when the id names no row of ours any more + // (discarded elsewhere, or deleted with its item): the URL path then applies, + // as it does for a first save. + let row_path = match req.id { + Some(id) => { + sqlx::query_scalar!( + "SELECT path FROM draft WHERE id = $1 AND workspace_id = $2 AND typ = $3 AND email = $4", + id, + &w_id, + kind as UserDraftItemKind, + email, + ) + .fetch_optional(&db) + .await? + } + None => None, + }; + let followed = row_path.is_some(); + let path: &str = row_path.as_deref().unwrap_or(url_path); + + // Everything past here writes, so the gate applies from here on. Answered + // without the path when the row was followed: the move may have taken it + // somewhere the caller cannot see. + if !is_own_discard { + match require_can_write_path(&authed, &db, &user_db, &w_id, kind, path).await { + Err(Error::NotAuthorized(_)) if followed => { + return Err(Error::NotAuthorized( + "this draft's item was moved to a path you cannot write".to_string(), + )); + } + other => other?, } } - // Everything past here writes, so the gate applies from here on. - if !is_own_discard { - require_can_write_path(&authed, &db, &user_db, &w_id, kind, path).await?; - } - - let applied_at = if let Some(value) = &req.value { + let applied = if let Some(value) = &req.value { // Secret variable values must never sit in `draft.value` in plaintext // (see `encrypt_secret_variable_value`). let serialized = if kind == UserDraftItemKind::Variable { @@ -702,33 +470,14 @@ async fn update_draft( // escape and later make any `->>`/`to_jsonb` extraction raise `22P05`. // Strip it here so a NUL never reaches the column. let serialized = strip_json_nul(&serialized); + // `base` is derived here from the value's per-kind field rather than sent + // by the client, so every writer (editors, chat, CLI) fills it the same way. + let base = draft_lineage(kind, value.0.get()).and_then(|l| l.as_text(kind)); // Upsert. The conflict check rides on the DO UPDATE WHERE clause — // when the row is newer than `last_sync`, RETURNING yields nothing. // `created_at` defaults to `now()` but the migration overrides it ($8) // so a migrated draft keeps its original age instead of jumping to top. - // The re-assert below needs a transaction to be able to undo the write, - // but only a draft with lineage can ever be told it moved — so a draft - // with none (a draft-only item, a variable, a trigger) keeps the plain - // single-statement write instead of paying BEGIN/COMMIT for a check that - // cannot fire. - // RLS-scoped, because the re-assert below reads the item tables through - // this same connection and must see exactly what the pre-check saw. `draft` - // itself carries no policies and grants `windmill_user` full access, so the - // upsert is unaffected by the role. - let mut tx = match &lineage { - Some(_) => Some(user_db.clone().begin(&authed).await?), - None => None, - }; - // One owned connection for the no-transaction case, so the executor below - // borrows from a binding that outlives the call. - let mut plain = match tx { - Some(_) => None, - None => Some(db.acquire().await?), - }; - // `base` is derived here from the value's per-kind field rather than sent - // by the client, so every writer (editors, chat, CLI) fills it the same way. - let base = lineage.as_ref().and_then(|l| l.as_text(kind)); - let applied = sqlx::query_scalar!( + sqlx::query!( r#"INSERT INTO draft (workspace_id, email, path, typ, value, created_at, base) VALUES ($1, $2, $3, $4, $5::text::json, COALESCE($8::timestamptz, now()), $9) ON CONFLICT (workspace_id, path, typ, email) WHERE email IS NOT NULL @@ -737,7 +486,7 @@ async fn update_draft( WHERE $7::bool = true OR $6::timestamptz IS NULL OR draft.created_at <= $6::timestamptz - RETURNING created_at"#, + RETURNING id, path, created_at"#, &w_id, email, path, @@ -748,45 +497,9 @@ async fn update_draft( req.created_at, base.as_deref(), ) - .fetch_optional(match (tx.as_mut(), plain.as_mut()) { - (Some(tx), _) => &mut **tx as &mut sqlx::PgConnection, - (None, Some(conn)) => &mut **conn, - (None, None) => unreachable!("exactly one of tx/plain is set"), - }) - .await?; - if let Some(mut tx) = tx { - // Cheap on the path that matters: one indexed existence check when the - // write landed. Only when it says the item is gone do we pay for the - // lineage lookup — and that answer is what distinguishes a move (roll - // back, report it) from a delete or a never-deployed draft-only item - // (both legitimate saves, which resolve to `None`). - // - // Run on the connection we already hold: acquiring a second from the - // same pool while this transaction is open is the two-connection stall. - // `tx` exists only because `lineage` did, so this is the same draft the - // pre-check consulted. - if let Some(base) = lineage.as_ref().filter(|_| applied.is_some()) { - if !deployed_still_at(&mut tx, &w_id, kind, path).await? { - if let Some((moved_to, moved_by, moved_patch)) = - resolve_moved_to_in(&mut tx, &w_id, kind, path, base).await? - { - tx.rollback().await?; - let now = sqlx::query_scalar!(r#"SELECT now() as "now!""#) - .fetch_one(&db) - .await?; - return Ok(Json(SaveDraftResponse { - status: SaveDraftStatus::Moved, - current_timestamp: now, - moved_to: Some(moved_to), - moved_by, - moved_patch: Some(moved_patch), - })); - } - } - } - tx.commit().await?; - } - applied + .fetch_optional(&db) + .await? + .map(|r| (r.created_at, Some(r.id), Some(r.path))) } else { // Delete, same conflict rule in the WHERE clause. Returns NULL when // the row was too new (conflict) OR already absent (idempotent) — @@ -811,15 +524,15 @@ async fn update_draft( ) .fetch_optional(&db) .await? + .map(|ts| (ts, None, None)) }; - if let Some(ts) = applied_at { + if let Some((ts, id, path)) = applied { return Ok(Json(SaveDraftResponse { status: SaveDraftStatus::Saved, current_timestamp: ts, - moved_to: None, - moved_by: None, - moved_patch: None, + id, + path, })); } @@ -844,9 +557,8 @@ async fn update_draft( Some(ts) => Ok(Json(SaveDraftResponse { status: SaveDraftStatus::Conflict, current_timestamp: ts, - moved_to: None, - moved_by: None, - moved_patch: None, + id: None, + path: None, })), // Delete + nothing-was-there ⇒ report success with server's NOW(). None => { @@ -856,9 +568,8 @@ async fn update_draft( Ok(Json(SaveDraftResponse { status: SaveDraftStatus::Saved, current_timestamp: now, - moved_to: None, - moved_by: None, - moved_patch: None, + id: None, + path: None, })) } } @@ -878,12 +589,8 @@ pub struct MoveDraftRequest { /// keys inside its value — there is no deployed row, schedule or trigger to /// cascade to. /// -/// The owner's OWN open editor is not notified, and cannot be: the moved-item -/// handshake in `update_draft` resolves through a deployed version chain, and a -/// draft-only item has none by definition. An editor still open on the old path -/// re-plants a row there on its next autosave, leaving two items. Closing that -/// needs a stable identity for a draft-only item — a tombstone or a surrogate id -/// — which is a larger change than this endpoint. +/// The owner's own open editor follows: it saves by the row's id, so its next +/// autosave lands at the new path and it is told where that is. /// /// Scoped to the caller's own row on purpose: two users can each have a draft /// at the same never-deployed path, and those are two separate items. diff --git a/backend/windmill-common/src/user_drafts.rs b/backend/windmill-common/src/user_drafts.rs index 3c24f85b4a..9489e20806 100644 --- a/backend/windmill-common/src/user_drafts.rs +++ b/backend/windmill-common/src/user_drafts.rs @@ -240,6 +240,10 @@ pub struct WithDraftOverlay { pub is_draft: bool, #[serde(skip_serializing_if = "Option::is_none")] pub draft_saved_at: Option>, + /// The draft row's id. The editor saves by it from then on, so its writes + /// follow the row through a move. + #[serde(skip_serializing_if = "Option::is_none")] + pub draft_id: Option, /// The deployed version the draft forked from (`draft.base`), as text /// whatever the kind. The editor compares it to the head it loaded to tell /// a draft that is behind. Absent when there is no draft or it was never @@ -331,6 +335,7 @@ where inner: Box::new(deployed), is_draft: false, draft_saved_at: None, + draft_id: None, draft_base: None, no_deployed: false, draft: None, @@ -351,7 +356,7 @@ where // NULL-email workspace draft. `NULLS LAST` + `LIMIT 1` drops the legacy // row when an owned one exists. let row = sqlx::query!( - r#"SELECT value as "value!: sqlx::types::Json>", + r#"SELECT id, value as "value!: sqlx::types::Json>", created_at, base FROM draft WHERE workspace_id = $1 @@ -373,6 +378,7 @@ where inner: Box::new(deployed), is_draft: false, draft_saved_at: None, + draft_id: None, draft_base: None, no_deployed: false, draft: None, @@ -386,6 +392,7 @@ where inner: Box::new(deployed), is_draft: true, draft_saved_at: Some(row.created_at), + draft_id: Some(row.id), draft_base: row.base, no_deployed: false, draft: Some(draft_json), @@ -665,7 +672,7 @@ pub async fn fetch_draft_only( ) -> Result> { // Own draft first, legacy NULL-email row as fallback (see `maybe_overlay_draft`). let row = sqlx::query!( - r#"SELECT value as "value!: sqlx::types::Json>", + r#"SELECT id, value as "value!: sqlx::types::Json>", created_at, base FROM draft WHERE workspace_id = $1 @@ -697,6 +704,7 @@ pub async fn fetch_draft_only( inner: Box::new(draft_json.clone()), is_draft: true, draft_saved_at: Some(row.created_at), + draft_id: Some(row.id), draft_base: row.base, no_deployed: true, draft: Some(draft_json), diff --git a/frontend/src/lib/components/ScriptBuilder.svelte b/frontend/src/lib/components/ScriptBuilder.svelte index 8f81a36d71..7be89beafd 100644 --- a/frontend/src/lib/components/ScriptBuilder.svelte +++ b/frontend/src/lib/components/ScriptBuilder.svelte @@ -786,7 +786,7 @@ // + `tick()` first so the last keystrokes reach the bindable before the // syncer flushes. No toast — the AutosaveIndicator narrates the result, and // `flush` never rejects (postSave routes errors to the failures map). - async function saveDraft(): Promise { + export async function saveDraft(): Promise { if (!opWorkspace || !userDraftPath) return editor?.flushPendingChanges() await tick() diff --git a/frontend/src/lib/components/common/confirmationModal/DraftEditorModals.svelte b/frontend/src/lib/components/common/confirmationModal/DraftEditorModals.svelte index 816f4e8634..5b3fcff72f 100644 --- a/frontend/src/lib/components/common/confirmationModal/DraftEditorModals.svelte +++ b/frontend/src/lib/components/common/confirmationModal/DraftEditorModals.svelte @@ -2,7 +2,6 @@ /** * The draft modals every editor route mounts at its trailer: * - DraftSyncConflictModal: surfaces a 409 from the autosave pipeline. - * - DraftMovedModal: the item was moved away from this path mid-edit. * - OtherUsersDraftsModal: other users' drafts at this path, for forking. * - StaleDraftModal: prompts when the user's draft predates the latest * deploy; open-state is computed here from the route's timestamps. @@ -13,8 +12,11 @@ */ import type { UserDraftItemKind } from '$lib/gen' import DraftSyncConflictModal from './DraftSyncConflictModal.svelte' - import DraftMovedModal from './DraftMovedModal.svelte' import OtherUsersDraftsModal, { type OtherDraftUser } from './OtherUsersDraftsModal.svelte' + import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte' + import { sendUserToast } from '$lib/toast' + import { base } from '$app/paths' + import { goto } from '$app/navigation' import StaleDraftModal from './StaleDraftModal.svelte' import ConfirmationModal from './ConfirmationModal.svelte' import { OtherUserDraftLoad } from '$lib/components/otherUserDraftLoad.svelte' @@ -54,6 +56,11 @@ * choice between keeping and discarding is informed. Omit where the editor * has no diff drawer; the action is then not rendered. */ onViewDiff?: () => void | Promise + /** Runs before this editor follows its draft to the item's new path: the + * editor's own draft save, which materializes text the code editor still + * holds. Without it, keystrokes typed since the relocating save are lost + * to the navigation. */ + onBeforeRelocate?: () => void | Promise /** Defaults to true; set to false to suppress all modals. */ enabled?: boolean } @@ -74,6 +81,7 @@ deployedHeadVersion = undefined, onLoadLatestDeploy, onViewDiff, + onBeforeRelocate, enabled = true }: Props = $props() @@ -113,6 +121,30 @@ } }) }) + + const EDITOR_SEGMENT: Partial> = { + script: 'scripts/edit', + flow: 'flows/edit', + app: 'apps/edit', + raw_app: 'apps_raw/edit' + } + + // The item was moved while this editor was open: the draft row followed it + // and the save just landed there. Follow it too — the route reloads the item + // at its new path, and the stale prompt above then says what changed. Edits + // typed since that save are flushed first, so leaving this path drops none. + $effect(() => { + if (!enabled || !workspace || !path) return + const seg = EDITOR_SEGMENT[itemKind] + if (!seg) return + const query = { workspace, itemKind, path } + return UserDraftDbSyncer.onRelocated(query, async (newPath) => { + await onBeforeRelocate?.() + await UserDraftDbSyncer.flush(query) + sendUserToast(`This item was moved to ${newPath}. You are now editing it there.`) + await goto(`${base}/${seg}/${newPath}`) + }) + }) {#if enabled && workspace && path} @@ -121,7 +153,6 @@ {onLoadFromServer} {getLocalDraft} /> - {#if otherDraftsUsers.length > 0} {#key path} - /** - * Surfaces the "moved" verdict left by `UserDraftDbSyncer.postSave`: someone - * moved this item while the editor was open, so the server refused the - * autosave rather than plant a phantom draft-only item at the path the item - * has left. - * - * Continuing pushes the current in-memory draft to the new path (force, since - * the draft carried over by the move is older) and follows it there, so edits - * made after the move aren't lost to the relocation. - */ - import { base } from '$app/paths' - import { goto } from '$app/navigation' - import { UserDraftDbSyncer, type UserDraftLastSyncQuery } from '$lib/userDraftDbSyncer.svelte' - import Modal2 from '$lib/components/common/modal/Modal2.svelte' - import Button from '$lib/components/common/button/Button.svelte' - import Alert from '$lib/components/common/alert/Alert.svelte' - import { FolderInput } from 'lucide-svelte' - - type Props = { - query: UserDraftLastSyncQuery - /** Current local draft value, re-pointed at the new path before it is - * pushed there. */ - getLocalDraft: () => unknown - } - - let { query, getLocalDraft }: Props = $props() - - const moveHandle = $derived(UserDraftDbSyncer.getMove(query)) - let isOpen = $derived(moveHandle.move !== undefined) - let busy = $state(false) - - const EDITOR_SEGMENT: Partial> = { - script: 'scripts/edit', - flow: 'flows/edit', - app: 'apps/edit', - raw_app: 'apps_raw/edit' - } - - /** Applies the server's `moved_patch` — the typed target path plus the version - * the item now sits at. Both come from the server precisely so this file - * doesn't reproduce `typed_path_field` / `base_version_field`; re-pointing the - * path without the version restamp would land the draft at the new path still - * claiming the pre-move version, and greet the user with a stale-draft prompt - * offering to discard the edits they just chose to carry. */ - function repointed(value: unknown, patch: Record | undefined): unknown { - if (value == undefined || typeof value !== 'object' || patch == undefined) return value - return { ...(value as Record), ...patch } - } - - // Tagged with the destination it was raised for, because this component is - // mounted for the editor's lifetime rather than per prompt: an untagged error - // would still be rendered when the next move verdict opens the modal. Tagging - // also survives the A→B→C case, where we deliberately re-point the verdict and - // then raise an error about the new destination. - let carryError = $state<{ title: string; detail: string; forMovedTo: string } | undefined>( - undefined - ) - const shownError = $derived( - carryError && carryError.forMovedTo === moveHandle.move?.movedTo ? carryError : undefined - ) - - async function continueThere() { - const move = moveHandle.move - if (!move) return - busy = true - carryError = undefined - try { - const local = getLocalDraft() - if (local != undefined) { - const target = { workspace: query.workspace, itemKind: query.itemKind, path: move.movedTo } - await UserDraftDbSyncer.overwrite({ ...target, value: repointed(local, move.patch) }) - // `overwrite` resolves whether or not the write landed — a network - // failure and a second move both park state instead of throwing. Leaving - // here regardless would drop the editor's edits on the floor and, for an - // A→B→C move, land on a B that no longer holds the item. Stay put and say - // so; the draft is still in this editor, so the user can retry. - const failed = UserDraftDbSyncer.getState(target).failureMessage - const movedAgain = UserDraftDbSyncer.getMove(target).move - if (movedAgain) { - // It moved again while we were carrying (A→B→C). Re-point this - // editor's own move record at C so the modal now offers C and a - // retry makes progress — without this the retry would keep - // overwriting at B, be refused again, and loop with no way out. - UserDraftDbSyncer.recordMove(query, movedAgain) - carryError = { - title: 'It moved again while saving', - detail: `It is now at ${movedAgain.movedTo}. Your edits are still in this editor — continue to follow it there.`, - forMovedTo: movedAgain.movedTo - } - return - } - if (failed) { - carryError = { - title: 'Could not save at the new path', - detail: `${failed.replace(/\.?$/, '.')} Your edits are still in this editor.`, - forMovedTo: move.movedTo - } - return - } - } - UserDraftDbSyncer.clearMove(query) - const seg = EDITOR_SEGMENT[query.itemKind] - if (seg) await goto(`${base}/${seg}/${move.movedTo}`) - } finally { - busy = false - } - } - - - -
-
- -
-

- {#if moveHandle.move?.movedBy} - {moveHandle.move.movedBy} moved this to - {:else} - This was moved to - {/if} - {moveHandle.move?.movedTo}. Nothing was saved here - — this path no longer holds the item. -

-

- Continuing takes your current edits to the new path, replacing any draft already there. - Staying here leaves them unsaved. -

- {#if shownError} - {shownError.detail} - {/if} -
-
- -
- - -
-
- diff --git a/frontend/src/lib/components/usePageDraftSync.svelte.ts b/frontend/src/lib/components/usePageDraftSync.svelte.ts index 8ca2b0beee..cbf3ea7702 100644 --- a/frontend/src/lib/components/usePageDraftSync.svelte.ts +++ b/frontend/src/lib/components/usePageDraftSync.svelte.ts @@ -47,8 +47,10 @@ export interface PageDraftSync { seedBaseline(value: V): void /** After a backend load, record the server's `draft_saved_at` so the * next autosave attaches a matching `last_sync` and the server can - * reject stale writes. `undefined` clears it (no draft existed). */ - recordRemoteSync(draftSavedAt: string | undefined): void + * reject stale writes. `undefined` clears it (no draft existed). The + * row's `draft_id` makes later saves address the row, so they follow a + * move. */ + recordRemoteSync(draftSavedAt: string | undefined, draftId?: number): void /** Drop the draft (server row + local cell) — restore-to-deployed and * post-deploy cleanup. */ remove(): void @@ -99,14 +101,15 @@ export function usePageDraftSync(opts: PageDraftSyncOptions): Pa if (!ws || !p) return UserDraft.seed(opts.itemKind, p, value, { workspace: ws }) }, - recordRemoteSync(draftSavedAt: string | undefined) { + recordRemoteSync(draftSavedAt: string | undefined, draftId?: number) { const ws = opts.workspace() const p = opts.path() if (!ws || !p) return untrack(() => UserDraftDbSyncer.recordRemoteSync( { workspace: ws, itemKind: opts.itemKind, path: p }, - draftSavedAt + draftSavedAt, + draftId ) ) }, diff --git a/frontend/src/lib/userDraftDbMigration.ts b/frontend/src/lib/userDraftDbMigration.ts index 0868876e06..1496e595cb 100644 --- a/frontend/src/lib/userDraftDbMigration.ts +++ b/frontend/src/lib/userDraftDbMigration.ts @@ -262,36 +262,6 @@ export async function migrateUserDraftsToDb(): Promise { path, requestBody: { value, last_sync: writtenAt, created_at: writtenAt } }) - if (res.status === 'moved' && res.moved_to) { - // Nothing was written: the item left this path. Retrying here would - // never resolve — `resolve_moved_to` answers from the item's lineage, - // so the old path reports `moved` for as long as the item exists — and - // dropping the entry would destroy the only copy there is. Follow the - // item instead, applying the patch the server just handed us so the - // draft lands pointing at where it now lives. - const moved = await DraftService.updateDraft({ - workspace: parsed.workspace, - kind: parsed.itemKind, - path: res.moved_to, - requestBody: { - value: { ...(value as object), ...((res.moved_patch as object) ?? {}) }, - last_sync: writtenAt, - created_at: writtenAt - } - }) - if (moved.status !== 'saved' && moved.status !== 'conflict') { - // Still not landed. Surface it rather than looping: the modal is the - // user's only way to see or discard an un-migratable draft. - reportDraftMigrationError({ - key, - path, - workspace: parsed.workspace, - itemKind: parsed.itemKind, - value - }) - continue - } - } if (res.status === 'conflict') { console.info( `UserDraft LS→DB migration: server draft for ${path} is fresher, dropping LS copy` diff --git a/frontend/src/lib/userDraftDbSyncer.svelte.ts b/frontend/src/lib/userDraftDbSyncer.svelte.ts index 558afc3b82..a7e3bf6297 100644 --- a/frontend/src/lib/userDraftDbSyncer.svelte.ts +++ b/frontend/src/lib/userDraftDbSyncer.svelte.ts @@ -129,17 +129,6 @@ export type DraftConflictInfo = { localLastSync: string | null } -/** Where an item went after someone moved it, as reported by a refused save. */ -export type DraftMovedInfo = { - movedTo: string - movedBy: string | undefined - /** Fields the server says to merge into the draft before re-saving it at - * `movedTo` — the typed target path and the version the item now sits at. - * The server owns these key names (`UserDraftItemKind::typed_path_field` / - * `base_version_field`), so nothing here reproduces them. */ - patch: Record | undefined -} - export type UserDraftLastSyncQuery = { workspace: string itemKind: UserDraftItemKind @@ -226,12 +215,18 @@ const syncLocked = new Map void) | undefined>() const conflicts = new SvelteMap() /** - * Keys whose item was MOVED out from under an editor still bound to the old - * path. The server refuses the write (saving would plant a phantom draft-only - * item where the item no longer is) and answers with where it went; read via - * `getMove(query)` to prompt the user over there. + * The server row id per key, learned from the load (`draft_id`) or the first + * save. Sent with every later save so the write addresses the row wherever a + * move took it, rather than the path this editor was opened on. */ -const moves = new SvelteMap() +const draftIds = new Map() + +/** + * Per-key listeners fired when a save lands at a path other than the key's: + * the item was moved while the editor was open, and the row followed it. The + * editor follows too (see `onRelocated`). + */ +const relocationListeners = new Map void>>() /** * Draft keys whose last save threw (network / 5xx) → extracted error @@ -310,21 +305,10 @@ async function postSave(opts: UserDraftDbSyncerSaveOpts): Promise { // `last_sync` (first-ever save) hits the backend's "treat as // fresh" branch. last_sync: opts.force ? undefined : lastSync, - force: opts.force ?? false + force: opts.force ?? false, + id: draftIds.get(key) } }) - if (resp.status === 'moved') { - // Nothing was written. Like a conflict, `lastSync` stays put so the - // state survives every retry until the user acts on it. - if (resp.moved_to) { - moves.set(key, { - movedTo: resp.moved_to, - movedBy: resp.moved_by, - patch: resp.moved_patch as Record | undefined - }) - } - return - } if (resp.status === 'conflict') { // Someone advanced the row past our `last_sync`. Park the // snapshot for the UI; do NOT touch `lastSync` — the next save @@ -339,8 +323,10 @@ async function postSave(opts: UserDraftDbSyncerSaveOpts): Promise { // resp.status === 'saved' — advance lastSync (or drop on delete). if (opts.value === null) { clearLastSync(opts.workspace, opts.itemKind, opts.path) + draftIds.delete(key) } else { setLastSync(opts.workspace, opts.itemKind, opts.path, resp.current_timestamp) + if (resp.id != null) draftIds.set(key, resp.id) } // postSave is the only place a draft's server-side existence changes, // so it's the single source for the list pages' `*` hint @@ -348,7 +334,6 @@ async function postSave(opts: UserDraftDbSyncerSaveOpts): Promise { // free instead of maintaining a separate source of truth. setLocalDraftHint(opts.workspace, opts.itemKind, opts.path, opts.value !== null) conflicts.delete(key) - moves.delete(key) failures.delete(key) // Clear pending only if it's still the opts we just saved — a // newer `save()` that arrived during the POST replaces the entry @@ -365,6 +350,12 @@ async function postSave(opts: UserDraftDbSyncerSaveOpts): Promise { // cached state the same way an upsert does. Listener errors must never // make a committed save read as failed. notifyAnySaved({ workspace: opts.workspace, itemKind: opts.itemKind, path: opts.path }) + // The row was carried elsewhere by a move and the save followed it. Last, + // so the editor that reacts (by leaving this path) sees a settled key. + if (opts.value !== null && resp.path && resp.path !== opts.path) { + const listeners = relocationListeners.get(key) + if (listeners) for (const l of [...listeners]) l(resp.path) + } } catch (e) { console.error('UserDraftDbSyncer.save failed', e) // Leave pending opts in place so the next attempt retries the same @@ -558,18 +549,28 @@ export const UserDraftDbSyncer = { * Seed the per-tab `last_sync` after an editor reads a draft from the * server. Pass the response's `draft_saved_at` so the next save sends a * matching `last_sync`; pass `undefined` when no draft existed (next - * save omits `last_sync`, the backend's first-push branch). + * save omits `last_sync`, the backend's first-push branch). `draftId` is + * the row's id from the same response; with it the next save addresses + * the row rather than the path, so it follows a move. */ - recordRemoteSync(query: UserDraftLastSyncQuery, draftSavedAt: string | undefined): void { + recordRemoteSync( + query: UserDraftLastSyncQuery, + draftSavedAt: string | undefined, + draftId?: number + ): void { const key = draftKey(query.workspace, query.itemKind, query.path) if (draftSavedAt) { setLastSync(query.workspace, query.itemKind, query.path, draftSavedAt) } else { clearLastSync(query.workspace, query.itemKind, query.path) } + if (draftId != null) { + draftIds.set(key, draftId) + } else { + draftIds.delete(key) + } // Back in sync with the server: clear any conflict / failure. conflicts.delete(key) - moves.delete(key) failures.delete(key) }, @@ -647,27 +648,26 @@ export const UserDraftDbSyncer = { conflicts.delete(draftKey(query.workspace, query.itemKind, query.path)) }, - /** Reactive "the item moved away from this path" snapshot, if any. */ - getMove(query: UserDraftLastSyncQuery): { - readonly move: DraftMovedInfo | undefined - } { + /** + * Subscribe to saves for a draft key that landed at another path: the item + * was moved while this editor was open and the row followed it. Fired with + * the row's current path, after the save is fully accounted for. Returns an + * unsubscribe. + */ + onRelocated(query: UserDraftLastSyncQuery, listener: (newPath: string) => void): () => void { const key = draftKey(query.workspace, query.itemKind, query.path) - return { - get move() { - return moves.get(key) - } + let set = relocationListeners.get(key) + if (!set) { + set = new Set() + relocationListeners.set(key, set) + } + set.add(listener) + return () => { + const s = relocationListeners.get(key) + if (!s) return + s.delete(listener) + if (s.size === 0) relocationListeners.delete(key) } - }, - - clearMove(query: UserDraftLastSyncQuery): void { - moves.delete(draftKey(query.workspace, query.itemKind, query.path)) - }, - - /** Re-point a key's move verdict, for an item that moved again while its - * carry was in flight (A→B→C). Without this the prompt keeps naming B, and - * every retry is refused for the same reason. */ - recordMove(query: UserDraftLastSyncQuery, move: DraftMovedInfo): void { - moves.set(draftKey(query.workspace, query.itemKind, query.path), move) }, /** diff --git a/frontend/src/routes/(root)/(logged)/apps/edit/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/apps/edit/[...path]/+page.svelte index d3c8155a3c..998de1f657 100644 --- a/frontend/src/routes/(root)/(logged)/apps/edit/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/apps/edit/[...path]/+page.svelte @@ -240,7 +240,8 @@ if ($workspaceStore && path) { UserDraftDbSyncer.recordRemoteSync( { workspace: $workspaceStore, itemKind: 'app', path }, - backendApp.draft_saved_at + backendApp.draft_saved_at, + backendApp.draft_id ) } // The app autosave stores a raw `App`, but this loader (and AppEditor's diff --git a/frontend/src/routes/(root)/(logged)/apps_raw/edit/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/apps_raw/edit/[...path]/+page.svelte index ed1e86c508..fe9018d955 100644 --- a/frontend/src/routes/(root)/(logged)/apps_raw/edit/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/apps_raw/edit/[...path]/+page.svelte @@ -302,7 +302,10 @@ if (getDraft) { otherDraftsUsers = (backendApp.other_drafts_users ?? []) as OtherDraftUser[] } - draftSync.recordRemoteSync(backendApp.draft_saved_at as string | undefined) + draftSync.recordRemoteSync( + backendApp.draft_saved_at as string | undefined, + backendApp.draft_id as number | undefined + ) isNewApp = !!backendApp.no_deployed // Per-response, NOT sticky: a later no-own-draft load in the same editor // must reset this so it can't wrongly force overlay mode. diff --git a/frontend/src/routes/(root)/(logged)/flows/edit/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/flows/edit/[...path]/+page.svelte index 5027f5fd52..de0b831453 100644 --- a/frontend/src/routes/(root)/(logged)/flows/edit/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/flows/edit/[...path]/+page.svelte @@ -358,7 +358,10 @@ if (getDraft) { otherDraftsUsers = (backendFlow.other_drafts_users ?? []) as OtherDraftUser[] } - draftSync.recordRemoteSync(backendFlow.draft_saved_at as string | undefined) + draftSync.recordRemoteSync( + backendFlow.draft_saved_at as string | undefined, + backendFlow.draft_id + ) // Re-evaluate per load: true for draft-only paths, false once deployed. isNewFlow = !!backendFlow.no_deployed // Per-response, NOT sticky: a later no-own-draft load in the same editor @@ -522,6 +525,7 @@ {draftBaseVersion} deployedHeadVersion={version != null ? String(version) : undefined} onViewDiff={() => flowBuilder?.openDiffDrawer()} + onBeforeRelocate={() => flowBuilder?.saveDraft()} onLoadLatestDeploy={async () => { // stopSync-bracketed; see /scripts/edit's restoreDeployed for the race. if (!$workspaceStore) return diff --git a/frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte index a404dbe9e8..98fc9227c2 100644 --- a/frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte @@ -317,7 +317,10 @@ // Seed the per-tab `last_sync` so the next autosave attaches a matching // timestamp the backend can stale-check. `undefined` (no draft) clears // it, making the next save take the "first push" branch. - draftSync.recordRemoteSync(backendScript.draft_saved_at as string | undefined) + draftSync.recordRemoteSync( + backendScript.draft_saved_at as string | undefined, + backendScript.draft_id + ) // Per-response, NOT sticky: navigating to another path in the same editor // must reset this, else a later no-own-draft load wrongly enters overlay. const hasOwnDraft = !!backendScript.is_draft @@ -483,6 +486,7 @@ draftBaseVersion={draftBaseHash} deployedHeadVersion={deployedHeadHash} onViewDiff={() => scriptBuilder?.openDiffDrawer()} + onBeforeRelocate={() => scriptBuilder?.saveDraft()} onLoadLatestDeploy={async () => { // stopSync-bracketed; see restoreDeployed for the race. if (!$workspaceStore) return