fix(forks): include Suspended in conflict gate, use parent_workspace_id for fork detection

Three fixes from the Claude review on PR #8976:

- Suspended mode still attaches the listener (it just pauses auto-run of
  queued jobs); two suspended fork+parent listeners would still split
  Kafka events / share a PG slot. Gate set_trigger_mode on
  `mode != Disabled` instead of `mode == Enabled` so Suspended also
  surfaces the warning.
- workspaces_export.rs::fork_*_ignore_keys keyed off the wm-fork-* prefix
  while set_trigger_mode and set_schedule_enabled key off
  parent_workspace_id. Switch the export filter to query
  parent_workspace_id once at the top of tarball_workspace and pass
  is_fork through. The column is the contract; the prefix is a
  creation-time naming convention that could in principle drift.
- TriggerModeToggle's suspend-dropdown action reassigned the non-bindable
  `triggerMode` prop instead of the local `innerTriggerMode` mirror,
  leaking inconsistent state if the dispatch was cancelled. Now writes
  to innerTriggerMode like the Toggle's on:change handler does.
This commit is contained in:
hugocasa
2026-05-01 17:46:35 +02:00
parent 09343805f5
commit 4dd38feefb
5 changed files with 67 additions and 20 deletions
@@ -0,0 +1,22 @@
{
"db_name": "PostgreSQL",
"query": "SELECT parent_workspace_id IS NOT NULL FROM workspace WHERE id = $1",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "?column?",
"type_info": "Bool"
}
],
"parameters": {
"Left": [
"Text"
]
},
"nullable": [
null
]
},
"hash": "f57ff370f6775602d2c200d78650d65ffa5bfc5f10e8bd2a3162894c93283259"
}
+19 -7
View File
@@ -45,7 +45,6 @@ use windmill_common::scripts::ScriptRunnableSettingsHandle;
use windmill_common::utils::require_admin;
use windmill_common::variables::decrypt;
use windmill_common::worker::WINDMILL_DIR;
use windmill_common::workspaces::WM_FORK_PREFIX;
use windmill_common::{
db::UserDB,
error::{to_anyhow, Error, Result},
@@ -124,16 +123,16 @@ pub fn is_none_or_false(val: &Option<bool>) -> bool {
/// source workspace is a fork. Stripping these keys avoids propagating
/// fork-local operational state (enabled flag, runtime listener identifiers)
/// back to the parent workspace through the git-sync round-trip.
fn fork_trigger_ignore_keys(w_id: &str) -> Option<Vec<&'static str>> {
if w_id.starts_with(WM_FORK_PREFIX) {
fn fork_trigger_ignore_keys(is_fork: bool) -> Option<Vec<&'static str>> {
if is_fork {
Some(vec!["mode", "enabled"])
} else {
None
}
}
fn fork_schedule_ignore_keys(w_id: &str) -> Option<Vec<&'static str>> {
if w_id.starts_with(WM_FORK_PREFIX) {
fn fork_schedule_ignore_keys(is_fork: bool) -> Option<Vec<&'static str>> {
if is_fork {
Some(vec!["enabled"])
} else {
None
@@ -436,6 +435,19 @@ pub(crate) async fn tarball_workspace(
let mut tx = user_db.begin(&authed).await?;
// Source-of-truth check for fork-ness: the workspace's parent_workspace_id
// column. The wm-fork-* prefix is a creation-time naming convention that
// could in principle drift (rename, manual SQL); the column is the
// contract that matches what the conflict-warning gates read.
let is_fork: bool = sqlx::query_scalar!(
"SELECT parent_workspace_id IS NOT NULL FROM workspace WHERE id = $1",
&w_id
)
.fetch_optional(&mut *tx)
.await?
.flatten()
.unwrap_or(false);
let tmp_dir = TempDir::new_in(&*WINDMILL_DIR)?;
let name = match archive_type.as_deref() {
@@ -703,7 +715,7 @@ pub(crate) async fn tarball_workspace(
.fetch_all(&mut *tx)
.await?;
let schedule_ignore_keys = fork_schedule_ignore_keys(&w_id);
let schedule_ignore_keys = fork_schedule_ignore_keys(is_fork);
for schedule in schedules {
let app_str =
&to_string_without_metadata(&schedule, false, schedule_ignore_keys.clone())
@@ -734,7 +746,7 @@ pub(crate) async fn tarball_workspace(
feature = "private"
)
))]
let trigger_ignore_keys = fork_trigger_ignore_keys(&w_id);
let trigger_ignore_keys = fork_trigger_ignore_keys(is_fork);
#[cfg(feature = "http_trigger")]
{
+10 -8
View File
@@ -820,14 +820,16 @@ async fn set_trigger_mode<T: TriggerCrud>(
let mut tx = user_db.begin(&authed).await?;
// Block enabling a trigger in a fork when the parent has the same path
// (regardless of parent's mode), unless the caller passes force=true.
// Until Phase 3 ships namespacing of listener identifiers, the cloned
// upstream identifier is shared with the parent — two enabled listeners
// would split or steal events; one enabled vs. one disabled can still
// destructively claim shared state. Skipped for kinds where the upstream
// identifier is already workspace-scoped at runtime (HTTP, Email).
if T::FORK_CONFLICT_ON_ENABLE && payload.mode == TriggerMode::Enabled && !payload.force {
// Block transitioning a trigger in a fork to any mode that attaches a
// listener (Enabled or Suspended) when the parent has the same path,
// unless the caller passes force=true. Suspended still keeps the
// listener attached — it just stops auto-running queued jobs — so a
// suspended fork would still split Kafka events / share a PG slot
// with the parent. The cloned upstream identifier is shared by
// construction; the risk is independent of the parent's current mode.
// Skipped for kinds where the upstream identifier is already
// workspace-scoped at runtime (HTTP, Email).
if T::FORK_CONFLICT_ON_ENABLE && payload.mode != TriggerMode::Disabled && !payload.force {
if let Some(parent_id) =
parent_has_trigger(&mut *tx, T::TABLE_NAME, &workspace_id, path).await?
{
+10 -4
View File
@@ -70,10 +70,16 @@ update path is naturally safe.
## Conflict warning on enable
The `set_*_trigger_mode` and `set_schedule_enabled` endpoints check whether
the parent workspace has a row at the same trigger path — **regardless of
the parent's current `mode`/`enabled`**. If so, they reject the request with
an error string of the shape:
The `set_*_trigger_mode` endpoint fires the warning whenever a fork transitions
to a mode that *attaches a listener*`Enabled` or `Suspended`. Suspended is
not "off": the listener still attaches and consumes events; only the auto-run
of queued jobs is paused. Two suspended forks would still split Kafka events
or share a Postgres slot with the parent. `Disabled` is the only mode that
fully detaches.
The check fires whenever the parent workspace has a row at the same trigger
path — **regardless of the parent's current `mode`/`enabled`**. If so, the
endpoint rejects the request with an error string of the shape:
```
fork-conflict:<kind>:<parent_workspace_id>
@@ -113,7 +113,12 @@
displayName: 'Suspend job execution',
icon: Pause,
action: () => {
triggerMode = 'suspended'
// Optimistically flip the local mirror, not the
// non-bindable `triggerMode` prop. The parent will
// echo the new mode back via $effect on success;
// on cancel, our on:change reset path snaps it
// back to the prop value.
innerTriggerMode = 'suspended'
onToggleMode?.('suspended')
},
tooltip: