fix(forks): respect caller-chosen state on create into a fork (only update preserves)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

backend/windmill-api-schedule/src/lib.rs

@@ -242,23 +242,6 @@ async fn create_schedule(
 
     let mut tx: Transaction<'_, Postgres> = user_db.begin(&authed).await?;
 
-    // A git-sync/merge/create write into a fork never sets operational state:
-    // force `enabled = false` so a cloned / synced / merged / UI-created schedule
-    // can't fire alongside the parent's. The fork owner re-enables locally via
-    // `setenabled`. Schedule analog of the trigger rule in
-    // `windmill-trigger::handler::workspace_is_fork`; the read half (parent-value
-    // substitution on fork export) lives in `workspaces_export.rs`. Read fork-ness
-    // on the non-RLS `db` pool (like the other two sites) so the determination is
-    // complete regardless of the caller's folder perms.
-    let target_is_fork: bool = sqlx::query_scalar!(
-        "SELECT parent_workspace_id IS NOT NULL FROM workspace WHERE id = $1",
-        w_id
-    )
-    .fetch_optional(&db)
-    .await?
-    .flatten()
-    .unwrap_or(false);
-
     // Check schedule for error
     ScheduleType::from_str(&ns.schedule, ns.cron_version.as_deref(), true)?;
 
@@ -358,12 +341,7 @@ async fn create_schedule(
         // flows (CLI merge, UI merge, `wmill push` of a fork tarball) — which
         // either send the source's actual flag (create case) or omit `enabled`
         // entirely (update case, where `EditSchedule` lacks the field).
-        // A write into a fork always lands disabled regardless of the request.
-        if target_is_fork {
-            false
-        } else {
-            ns.enabled.unwrap_or(true)
-        },
+        ns.enabled.unwrap_or(true),
         resolved_email,
         resolved_permissioned_as,
         ns.on_failure,
@@ -435,7 +413,7 @@ async fn create_schedule(
         .await?;
     }
 
-    if !target_is_fork && ns.enabled.unwrap_or(true) {
+    if ns.enabled.unwrap_or(true) {
         tx = push_scheduled_job(&db, tx, &schedule, Some(&authed.clone().into()), None).await?
     }
     tx.commit().await?;

backend/windmill-trigger/src/handler.rs

@@ -34,16 +34,15 @@ use windmill_git_sync::handle_deployment_metadata;
 
 /// True when the workspace is a fork (`parent_workspace_id IS NOT NULL`).
 ///
-/// Operational state (`mode`) belongs to the parent workspace: a git-sync /
-/// merge / clone / UI-create write into a fork must never set it. On create we
-/// force `disabled` so a fork trigger can't compete with the parent's listener;
-/// on update we preserve the fork's existing value. `setmode` is the intended
-/// explicit mutator of a fork's mode (and carries its own conflict warning) —
-/// runtime error handling may still auto-disable an errored trigger, which is
-/// orthogonal to this rule. This is the write half whose read half lives in
-/// `workspaces_export.rs` (parent-value substitution on fork export), and it is
-/// the single authority shared by both the git-sync round-trip and the in-app
-/// compare-workspaces merge.
+/// On *update* into a fork, preserve the fork's existing `mode` instead of
+/// writing the incoming value. This is the necessary complement to the export
+/// read-half (`workspaces_export.rs`): that substitutes the *parent's* `mode`
+/// into a fork's synced file, so a write-back of that file into the fork
+/// (`wmill sync push`, compare-workspaces "Update current") would otherwise
+/// flip the fork's trigger to the parent's (often enabled) state and re-create
+/// the listener conflict. `setmode` remains the explicit mutator of a fork's
+/// mode (with its own conflict warning); create deliberately does *not* force a
+/// state, so a fork-only trigger lands in whatever state its creator chose.
 async fn workspace_is_fork(db: &DB, workspace_id: &str) -> Result<bool> {
     let is_fork: Option<bool> =
         sqlx::query_scalar("SELECT parent_workspace_id IS NOT NULL FROM workspace WHERE id = $1")
@@ -461,13 +460,6 @@ async fn create_trigger<T: TriggerCrud>(
 
     let mut tx = user_db.begin(&authed).await?;
 
-    // Writing into a fork never sets operational state: force `disabled` so a
-    // cloned / synced / merged / UI-created trigger can't compete with the
-    // parent's listener. The fork owner re-enables locally via `setmode`.
-    if workspace_is_fork(&db, &workspace_id).await? {
-        new_trigger.base.set_mode(TriggerMode::Disabled);
-    }
-
     let new_path = new_trigger.base.path.clone();
     let labels = new_trigger.base.labels.clone();
 

docs/fork-triggers.md

@@ -84,16 +84,24 @@ parent had also edited it. Substituting the parent's value makes the fork's
 file byte-identical to the parent on that field, so the merge has nothing to
 resolve.
 
-**Write half — fork writes never set operational state.** A write into a fork
-(git-sync push, merge deploy, clone, or a plain UI create) must not set the
-state, otherwise pulling the substituted parent value straight back into the
-fork would re-enable it. So `create_trigger`/`create_schedule` force `disabled`
-for a fork target, and `update_trigger` preserves the fork's existing `mode`
-(`workspace_is_fork` in `windmill-trigger/src/handler.rs`; schedule `enabled` is
-naturally preserved because `EditSchedule` has no `enabled` field). The same
-handlers serve both merge paths, so the two can't diverge. The fork owner
-re-enables locally via `setmode`/`setenabled` (which carry the conflict
-warning below).
+**Write half — an *update* into a fork preserves its existing state.** The
+read-half writes the *parent's* (often enabled) value into a fork's synced
+file. If that file is then written back into the fork — `wmill sync push`,
+compare-workspaces "Update current", or any bidirectional sync — a naive update
+would flip the fork's trigger to the parent's state and re-create the listener
+conflict. So `update_trigger` preserves the fork's existing `mode` for a fork
+target (`workspace_is_fork` in `windmill-trigger/src/handler.rs`); schedule
+`enabled` is naturally preserved because `EditSchedule` has no `enabled` field.
+
+**Create is *not* forced.** A create into a fork keeps the caller's chosen
+state, so creating a trigger/schedule in the fork UI works normally and a
+fork-only row lands enabled if the user wants it. A fork-only row has no parent
+counterpart, so it can't share an upstream identifier and can't conflict. (Rows
+*cloned* from the parent at fork creation still start disabled — that's forced
+in the SQL clone, `clone_triggers_and_schedules`, not in the create handler.)
+The fork owner toggles state via `setmode`/`setenabled` (which carry the
+conflict warning below) — those endpoints are unchanged, so the UI's enable/
+disable controls behave the same in a fork as anywhere else.
 
 For a non-fork target the incoming value is applied as given — so a fork→parent
 merge of an existing trigger writes the parent's own (substituted) value (a
@@ -199,11 +207,13 @@ is owned by the parent* above) — the two paths share the backend `create`/
   For a fork target the backend preserves it regardless (`workspace_is_fork`);
   for a parent target the `is_mode_unspecified()` safeguard does. Schedules also
   rely on `EditSchedule` lacking the `enabled` field.
-- **Create**: the source's `mode`/`enabled` is passed through. Into a **parent**
-  there's no row to preserve, so a fork-only trigger/schedule lands with the
-  state the fork creator chose (omitting the flag defaults to `enabled`:
-  `BaseTriggerData::mode()` → `Enabled`, schedule insert → `true`). Into a
-  **fork** the backend forces `disabled` — a fork write never enables anything.
+- **Create**: the source's `mode`/`enabled` is passed through (into a fork or a
+  parent alike) — there's no row to preserve, so the trigger/schedule lands with
+  the state the source chose (omitting the flag defaults to `enabled`:
+  `BaseTriggerData::mode()` → `Enabled`, schedule insert → `true`). Create is not
+  force-disabled for forks: a fork-only row has no parent counterpart and can't
+  conflict, and rows cloned from the parent already start disabled via the SQL
+  clone at fork creation.
 
 ## Future work — runtime listener suffix