fix(datatables): clone a fork's pointer instead of failing after the copy

Forking a fork with cloning left an orphan database. The preflight resolves the
pointer and sees the governing entry, so both endpoints ran and filled the new
database; `apply_forked_datatable` then refused the inherited pointer and rolled
the fork back, stranding a registered `wm_fork_*` that no entry names and whose
name blocks the retry.

Refusing earlier would have been the smaller change, but forking a fork and
cloning worked before pointers existed, so it would trade an orphan for a
regression. Resolve what the pointer names and write the terminal entry the
clone needs: the whole `database` object rather than a patch of its
`resource_path`, since a pointer has none, and `reference` removed with it.

Also accepts `-- role=x` and `-- Role = x`, two more spellings that fell through
to the default role.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb

backend/.sqlx/query-ebaf3ed3097621da59dd201b5a4b9d1f440692f183c7c378f59e4b73f1c6e241.json

@@ -0,0 +1,17 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "UPDATE workspace_settings\n               SET datatable = jsonb_set(\n                   jsonb_set(\n                       datatable #- ARRAY['datatables', $2, 'reference'],\n                       ARRAY['datatables', $2, 'database'], $3::jsonb),\n                   ARRAY['datatables', $2, 'forked_from'], $4::jsonb\n               )\n               WHERE workspace_id = $1",
+  "describe": {
+    "columns": [],
+    "parameters": {
+      "Left": [
+        "Text",
+        "Text",
+        "Jsonb",
+        "Jsonb"
+      ]
+    },
+    "nullable": []
+  },
+  "hash": "ebaf3ed3097621da59dd201b5a4b9d1f440692f183c7c378f59e4b73f1c6e241"
+}

backend/windmill-api-workspaces/src/workspaces.rs

@@ -45,12 +45,12 @@ use windmill_common::workspaces::GitRepositorySettings;
 #[cfg(feature = "enterprise")]
 use windmill_common::workspaces::WorkspaceDeploymentUISettings;
 use windmill_common::workspaces::{
-    check_deploy_rules, check_user_against_rule, get_datatable_resource_from_db,
-    datatable_ref_name, get_datatable_resource_from_db_unchecked, resolve_governing_datatable,
-    validate_dev_workspace_id, validate_fork_workspace_id, validate_workspace_name, DataTable,
-    DataTableCatalogResourceType, DataTableForkBehavior, DatatableAccess, ProtectionRuleKind,
-    ProtectionRules, ProtectionRuleset, RuleCheckResult, WorkspaceGitSyncSettings,
-    DEV_WORKSPACE_LOCK_RULE_NAME,
+    check_deploy_rules, check_user_against_rule, datatable_ref_name,
+    get_datatable_resource_from_db, get_datatable_resource_from_db_unchecked,
+    resolve_governing_datatable, validate_dev_workspace_id, validate_fork_workspace_id,
+    validate_workspace_name, DataTable, DataTableCatalogResourceType, DataTableForkBehavior,
+    DatatableAccess, ProtectionRuleKind, ProtectionRules, ProtectionRuleset, RuleCheckResult,
+    WorkspaceGitSyncSettings, DEV_WORKSPACE_LOCK_RULE_NAME,
 };
 use windmill_common::workspaces::{Ducklake, DucklakeCatalogResourceType};
 use windmill_common::PgDatabase;
@@ -8046,29 +8046,44 @@ async fn apply_forked_datatable(
     let dt: DataTable = serde_json::from_value(config_val)
         .map_err(|e| Error::internal_err(format!("Failed to parse datatable config: {}", e)))?;
 
-    // A cloned data table owns its copy, so the fork's entry must be terminal. It arrived that way
-    // from the settings clone; a pointer here would mean the parent's own entry was one, and the
-    // clone has to name its new database rather than follow anything.
-    let database = dt.database.as_ref().ok_or_else(|| {
-        Error::BadRequest(format!(
-            "Data table '{}' points at another workspace's data table and cannot be cloned; \
-             fork it from the workspace that owns it.",
-            fdt.name
-        ))
-    })?;
+    // A clone owns its copy, so the fork's entry has to be terminal. When the parent was itself a
+    // fork the settings clone hands down a pointer instead, and the database this clone just
+    // created is already filled — so resolve what it points at and name the copy, rather than
+    // refusing after the fact. Pointers are only ever written for instance databases
+    // (`point_kept_datatables_at_parent`), so the resolved entry is one.
+    let database = match dt.database.clone() {
+        Some(database) => database,
+        None => resolve_governing_datatable(db, parent_w_id, &fdt.name)
+            .await?
+            .datatable
+            .database
+            .ok_or_else(|| {
+                Error::internal_err(format!(
+                    "Data table '{}' resolves to an entry that owns no database",
+                    fdt.name
+                ))
+            })?,
+    };
 
     if database.resource_type == DataTableCatalogResourceType::Instance {
-        // Instance: update resource_path to the new dbname
+        // The whole `database` object, not just its `resource_path`: a pointer entry has none to
+        // patch. `reference` goes with it — exactly one of the two may be set.
+        let new_database = serde_json::json!({
+            "resource_type": "instance",
+            "resource_path": &fdt.new_dbname,
+        });
         sqlx::query!(
             r#"UPDATE workspace_settings
                SET datatable = jsonb_set(
-                   jsonb_set(datatable, ARRAY['datatables', $2, 'database', 'resource_path'], to_jsonb($3::text)),
+                   jsonb_set(
+                       datatable #- ARRAY['datatables', $2, 'reference'],
+                       ARRAY['datatables', $2, 'database'], $3::jsonb),
                    ARRAY['datatables', $2, 'forked_from'], $4::jsonb
                )
                WHERE workspace_id = $1"#,
             forked_w_id,
             &fdt.name,
-            &fdt.new_dbname,
+            new_database,
             forked_from,
         )
         .execute(&mut **tx)

backend/windmill-common/src/worker.rs

@@ -1105,9 +1105,10 @@ impl SqlAnnotations {
             if !line.starts_with("--") {
                 break;
             }
-            // `role`, `Role`, `role:` and `role:name` all open an attempt; `rolexyz` does not.
-            // The colon is worth accepting rather than skipping past: `-- role: x` is the likelier
-            // spelling, and skipping it is exactly the silent fallback this refuses.
+            // The keyword may be followed by whitespace, `:` or `=` — `role x`, `role: x`,
+            // `role=x`, `Role = x` all open an attempt, while `rolexyz` does not. Each accepted
+            // separator is one spelling that would otherwise take the `continue` below and run the
+            // query as the data table's default role, which is the silence this exists to remove.
             let body = line[2..].trim_start();
             let Some(after) = body
                 .get(..4)
@@ -1116,15 +1117,18 @@ impl SqlAnnotations {
             else {
                 continue;
             };
-            let colon = after.starts_with(':');
-            let after = after.strip_prefix(':').unwrap_or(after);
-            if !after.is_empty() && !colon && !after.starts_with(char::is_whitespace) {
+            if !after.is_empty()
+                && !after.starts_with(char::is_whitespace)
+                && !after.starts_with([':', '='])
+            {
                 continue;
             }
 
             // Past this point the line is an attempt to name a role, so a malformed one is an
             // error rather than a miss. Falling through would run the query as the data table's
             // default role — quietly, and under a login the author did not choose.
+            let after = after.trim_start();
+            let after = after.strip_prefix([':', '=']).unwrap_or(after);
             let mut tokens = after.split_whitespace();
             let role = tokens
                 .next()
@@ -2750,6 +2754,8 @@ mod tests {
             "-- role operator;\nSELECT 1",
             "-- role: operator\nSELECT 1",
             "-- role:operator\nSELECT 1",
+            "-- role=operator\nSELECT 1",
+            "-- Role = operator\nSELECT 1",
         ] {
             assert_eq!(
                 role(accepted).unwrap(),