From 7350705cea87bee1b9f97cd14e550bb29a60864d Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Tue, 8 Sep 2026 11:20:45 +0200 Subject: [PATCH] fix: read the app move's author from the head version, not the draft's base Co-Authored-By: Claude Opus 5 (1M context) --- ...96c1ad11eab84d03dd8c435b824e2e1b03ff1.json | 35 ++++++++++++++++++ backend/windmill-api/src/drafts.rs | 37 +++++++++++++------ frontend/src/lib/userDraftDbMigration.ts | 11 ++++++ 3 files changed, 72 insertions(+), 11 deletions(-) create mode 100644 backend/.sqlx/query-323f975428149815f3a9bc5c47396c1ad11eab84d03dd8c435b824e2e1b03ff1.json diff --git a/backend/.sqlx/query-323f975428149815f3a9bc5c47396c1ad11eab84d03dd8c435b824e2e1b03ff1.json b/backend/.sqlx/query-323f975428149815f3a9bc5c47396c1ad11eab84d03dd8c435b824e2e1b03ff1.json new file mode 100644 index 0000000000..53f0ca1d34 --- /dev/null +++ b/backend/.sqlx/query-323f975428149815f3a9bc5c47396c1ad11eab84d03dd8c435b824e2e1b03ff1.json @@ -0,0 +1,35 @@ +{ + "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/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index a2fc46e781..93d5ea79bb 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -501,18 +501,26 @@ async fn resolve_moved_to( 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, av.created_by, - (SELECT id FROM app_version WHERE app_id = a.id - ORDER BY created_at DESC LIMIT 1) as "head!" - FROM app_version av JOIN app a ON a.id = av.app_id + 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.created_by), json!(r.head))) + .map(|r| (r.path, Some(r.head_by), json!(r.head))) } } _ => None, @@ -530,12 +538,19 @@ async fn resolve_moved_to( // naming anywhere else ⇒ omit, so a rename staged in this very editor // survives the relocation. Same rule as `move_drafts_for_path`. // - // The version is restamped only for whoever performed the move, matching the - // `restamp_email` scoping on the passive carry and for the same reason. The - // deploy that moved the item may have edited it in the same breath; handing a - // teammate the new head would tell them they are up to date with content they - // have never seen, and their next deploy would silently revert it. The mover - // knows what they just pushed, so only they are spared the prompt. + // The version is restamped only for the LAST DEPLOYER AT THE NEW PATH, which + // is the strongest "did I put the content there?" test available without + // comparing payloads — nothing records who performed a move as distinct from + // who deployed. It is deliberately weaker than "the mover": if Alice moves + // A→B and Bob then deploys at B, Bob is spared the prompt for a head that + // also carries Alice's move-time edits. That residual is a missing prompt for + // someone who did deploy the head, not for a bystander. + // + // The point of the scoping is the bystander: the deploy that moved the item + // may have edited it in the same breath, and handing a teammate the new head + // would tell them they are up to date with content they have never seen, so + // their next deploy would silently revert it. Mirrors `restamp_email` on the + // passive carry in `move_drafts_for_path`. let repoint_path = base.typed_path(kind) == Some(path); Ok(moved.map(|(new_path, new_by, head)| { let moved_by_me = new_by.as_deref() == Some(authed.username.as_str()); diff --git a/frontend/src/lib/userDraftDbMigration.ts b/frontend/src/lib/userDraftDbMigration.ts index 1496e595cb..c27ece99bf 100644 --- a/frontend/src/lib/userDraftDbMigration.ts +++ b/frontend/src/lib/userDraftDbMigration.ts @@ -262,6 +262,17 @@ export async function migrateUserDraftsToDb(): Promise { path, requestBody: { value, last_sync: writtenAt, created_at: writtenAt } }) + if (res.status === 'moved') { + // Nothing was written server-side: the item left this path. Unlike a + // conflict, where a fresher server row already holds the work, dropping + // the LS entry here would destroy the only copy that exists. Leave it + // and let a later mount retry — by then the user has usually followed + // the item, and this path resolves normally. + console.warn( + `UserDraft LS→DB migration: ${path} was moved to ${res.moved_to}, keeping the LS copy` + ) + continue + } if (res.status === 'conflict') { console.info( `UserDraft LS→DB migration: server draft for ${path} is fresher, dropping LS copy`