From 733ef9c3b8e7a70dfc30bed3ee32aede1c84d185 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Mon, 14 Sep 2026 21:55:24 +0200 Subject: [PATCH] test: pin a teammate's carried draft; name the kind that refuses a draft move Co-Authored-By: Claude Opus 5 (1M context) --- ...23cf3a3b7cea9a676b1cedc2ae36290aab40.json} | 8 ++--- backend/tests/drafts_move_taken.rs | 20 +++++++++---- backend/tests/drafts_save_follows_move.rs | 30 +++++++++++++++++++ .../fixtures/drafts_save_follows_move.sql | 7 +++++ backend/windmill-api/src/drafts.rs | 10 ++++--- 5 files changed, 62 insertions(+), 13 deletions(-) rename backend/.sqlx/{query-9b574bf93759d822066ac1629744a8df58840c463d8aa412871bd9774d978bae.json => query-ba312a2cade7e7e3bca1788e151e23cf3a3b7cea9a676b1cedc2ae36290aab40.json} (69%) diff --git a/backend/.sqlx/query-9b574bf93759d822066ac1629744a8df58840c463d8aa412871bd9774d978bae.json b/backend/.sqlx/query-ba312a2cade7e7e3bca1788e151e23cf3a3b7cea9a676b1cedc2ae36290aab40.json similarity index 69% rename from backend/.sqlx/query-9b574bf93759d822066ac1629744a8df58840c463d8aa412871bd9774d978bae.json rename to backend/.sqlx/query-ba312a2cade7e7e3bca1788e151e23cf3a3b7cea9a676b1cedc2ae36290aab40.json index 52f1194dff..5f3b64db1a 100644 --- a/backend/.sqlx/query-9b574bf93759d822066ac1629744a8df58840c463d8aa412871bd9774d978bae.json +++ b/backend/.sqlx/query-ba312a2cade7e7e3bca1788e151e23cf3a3b7cea9a676b1cedc2ae36290aab40.json @@ -1,12 +1,12 @@ { "db_name": "PostgreSQL", - "query": "SELECT\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $3\n AND typ::text = ANY($6::text[]) AND email = $4) as \"at_target!\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email = $4\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0\n ) as \"poisoned!\" ", + "query": "SELECT\n (SELECT typ::text FROM draft WHERE workspace_id = $1 AND path = $3\n AND typ::text = ANY($6::text[]) AND email = $4 LIMIT 1) as \"at_target\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email = $4\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0\n ) as \"poisoned!\" ", "describe": { "columns": [ { "ordinal": 0, - "name": "at_target!", - "type_info": "Bool" + "name": "at_target", + "type_info": "Text" }, { "ordinal": 1, @@ -63,5 +63,5 @@ null ] }, - "hash": "9b574bf93759d822066ac1629744a8df58840c463d8aa412871bd9774d978bae" + "hash": "ba312a2cade7e7e3bca1788e151e23cf3a3b7cea9a676b1cedc2ae36290aab40" } diff --git a/backend/tests/drafts_move_taken.rs b/backend/tests/drafts_move_taken.rs index 68a219d4d7..876fcaf577 100644 --- a/backend/tests/drafts_move_taken.rs +++ b/backend/tests/drafts_move_taken.rs @@ -94,7 +94,10 @@ async fn test_draft_move_refuses_the_other_app_kind(db: Pool) -> anyho let status = resp.status(); let body = resp.text().await?; assert_eq!(status, 400, "move onto a classic app draft was allowed: {body}"); - assert!(body.contains("already have a draft"), "unexpected refusal: {body}"); + assert!( + body.contains("already have a app draft"), + "the refusal did not name the occupying kind: {body}" + ); // Both drafts are untouched. let list: Vec = reqwest::Client::new() @@ -106,12 +109,19 @@ async fn test_draft_move_refuses_the_other_app_kind(db: Pool) -> anyho .await? .json() .await?; - let mut kinds = list + let mut at = list .iter() .filter(|d| matches!(d["kind"].as_str(), Some("app") | Some("raw_app"))) - .filter_map(|d| d["kind"].as_str()) + .filter_map(|d| Some((d["kind"].as_str()?, d["path"].as_str()?))) .collect::>(); - kinds.sort(); - assert_eq!(kinds, vec!["app", "raw_app"], "{list:?}"); + at.sort(); + assert_eq!( + at, + vec![ + ("app", "u/test-user/mvtaken_app"), + ("raw_app", "u/test-user/mvtaken_raw") + ], + "{list:?}" + ); Ok(()) } diff --git a/backend/tests/drafts_save_follows_move.rs b/backend/tests/drafts_save_follows_move.rs index 79683b7d75..7cb8b725a3 100644 --- a/backend/tests/drafts_save_follows_move.rs +++ b/backend/tests/drafts_save_follows_move.rs @@ -239,3 +239,33 @@ async fn test_move_back_ends_the_record(db: Pool) -> anyhow::Result<() assert_eq!(own_draft_paths(port).await?, vec!["u/test-user/follow_a"]); Ok(()) } + +/// A rename carries a teammate's row too: both its path keys follow, and the version +/// it forked from does not move. A restamp there would clear their out-of-date prompt +/// and let them deploy over the mover's version believing they were current. +#[sqlx::test(fixtures("base", "drafts_save_follows_move"))] +async fn test_a_teammates_draft_follows_with_its_base(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + rename(port, HEAD_HASH, "u/test-user/follow_b").await?; + + // Read from the pool: the teammate's row is another user's, and this asserts on + // `base`, which no endpoint exposes for someone else's draft. + let row: (String, String, Option) = sqlx::query_as( + "SELECT value::jsonb ->> 'path', value::jsonb ->> 'draft_path', base + FROM draft WHERE workspace_id = 'test-workspace' AND typ = 'script' + AND email = 'test2@windmill.dev'", + ) + .fetch_one(&db) + .await?; + assert_eq!(row.0, "u/test-user/follow_b", "typed path did not follow"); + assert_eq!(row.1, "u/test-user/follow_b", "mirror did not follow"); + assert_eq!( + row.2.as_deref(), + Some(HEAD_HASH), + "the teammate's base was restamped by someone else's rename" + ); + Ok(()) +} diff --git a/backend/tests/fixtures/drafts_save_follows_move.sql b/backend/tests/fixtures/drafts_save_follows_move.sql index 9374914e97..16a41feb1a 100644 --- a/backend/tests/fixtures/drafts_save_follows_move.sql +++ b/backend/tests/fixtures/drafts_save_follows_move.sql @@ -20,3 +20,10 @@ INSERT INTO draft (workspace_id, path, typ, value, email) VALUES ('test-workspace', 'u/test-user/draft_store', 'script', '{"path": "u/test-user/friendly", "draft_path": "u/test-user/friendly", "summary": "D", "content": "draft"}', 'test@windmill.dev'); + +-- A teammate's draft on the same deployed script, forked from the same head. The +-- rename must carry it too, without touching the version it forked from. +INSERT INTO draft (workspace_id, path, typ, value, email, base) +VALUES ('test-workspace', 'u/test-user/follow_a', 'script', + '{"path": "u/test-user/follow_a", "draft_path": "u/test-user/follow_a", "parent_hash": "0000000000001b76", "summary": "A", "content": "teammate draft"}', + 'test2@windmill.dev', '0000000000001b76'); diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index d99e8bee22..ccee9ead3a 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -757,8 +757,8 @@ async fn move_draft( if moved.is_none() { let row = sqlx::query!( r#"SELECT - EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $3 - AND typ::text = ANY($6::text[]) AND email = $4) as "at_target!", + (SELECT typ::text FROM draft WHERE workspace_id = $1 AND path = $3 + AND typ::text = ANY($6::text[]) AND email = $4 LIMIT 1) as "at_target", EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5 AND typ = $2 AND email = $4 AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0 @@ -780,8 +780,10 @@ async fn move_draft( "'{path}' contains a NUL character and predates the sanitizer, so it cannot be \ {attempted}. Reopen it, re-save to rewrite it cleanly, then retry." ) - } else if row.at_target && new_path != path { - format!("You already have a draft at '{new_path}'") + } else if let Some(occupant) = row.at_target.filter(|_| new_path != path) { + // Naming the kind matters for the app pair: a classic-app draft refusing a + // raw-app move is invisible in the raw-app list the caller is looking at. + format!("You already have a {} draft at '{new_path}'", occupant.replace('_', " ")) } else { format!("You have no draft at '{path}'") }));