From 73dc892f9c9c840a5f0fb12fcbb28bbe0f38795f Mon Sep 17 00:00:00 2001 From: hugocasa Date: Wed, 16 Sep 2026 10:45:34 +0200 Subject: [PATCH] fix: walk the whole fork ancestry for app installations and fork conflicts (#11151) * fix: walk the whole fork ancestry for app installations and fork conflicts Co-Authored-By: Claude Fable 5.1 * docs: describe the fork-conflict gate as ancestor-wide Co-Authored-By: Claude Fable 5.1 * chore: update ee-repo-ref to d252afcc80e77fcc4f9a2a346b80908c8605a6c0 This commit updates the EE repository reference after PR #803 was merged in windmill-ee-private. Previous ee-repo-ref: 5f68c8c351ffc92feccffe69a857b60be376464e New ee-repo-ref: d252afcc80e77fcc4f9a2a346b80908c8605a6c0 Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Fable 5.1 Co-authored-by: windmill-internal-app[bot] --- backend/ee-repo-ref.txt | 2 +- .../tests/fixtures/schedule_fork_conflict.sql | 46 ++++++++ backend/tests/git_sync_fork_credential.rs | 100 +++++++++++++++++- backend/tests/schedule_fork_conflict.rs | 59 +++++++++++ backend/windmill-api-schedule/src/lib.rs | 46 +++----- backend/windmill-common/src/workspaces.rs | 42 ++++++++ backend/windmill-trigger/src/handler.rs | 76 ++++--------- .../lib/components/ForkConflictModal.svelte | 16 +-- frontend/src/lib/stores.ts | 2 +- frontend/src/lib/utils/forkConflict.ts | 15 +-- 10 files changed, 301 insertions(+), 103 deletions(-) create mode 100644 backend/tests/fixtures/schedule_fork_conflict.sql create mode 100644 backend/tests/schedule_fork_conflict.rs diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 5d28f73b5a..317ca12c5e 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -93433d7c9dc34f2c0f56a5297d3453aabd2f9472 \ No newline at end of file +d252afcc80e77fcc4f9a2a346b80908c8605a6c0 diff --git a/backend/tests/fixtures/schedule_fork_conflict.sql b/backend/tests/fixtures/schedule_fork_conflict.sql new file mode 100644 index 0000000000..8e81d9ede6 --- /dev/null +++ b/backend/tests/fixtures/schedule_fork_conflict.sql @@ -0,0 +1,46 @@ +-- A three-deep fork chain whose schedule rows were cloned down at fork time. +-- The middle fork has since deleted its copy, so the leaf's schedule shares +-- its cron only with the root — the shape a direct-parent check misses. + +INSERT INTO workspace (id, name, owner, parent_workspace_id) VALUES + ('sfc-root', 'sfc-root', 'sfc-admin', NULL), + ('sfc-mid', 'sfc-mid', 'sfc-admin', 'sfc-root'), + ('sfc-leaf', 'sfc-leaf', 'sfc-admin', 'sfc-mid'); + +INSERT INTO workspace_key (workspace_id, kind, key) VALUES + ('sfc-root', 'cloud', 'sfc-root-key'), + ('sfc-mid', 'cloud', 'sfc-mid-key'), + ('sfc-leaf', 'cloud', 'sfc-leaf-key'); + +INSERT INTO workspace_settings (workspace_id) VALUES + ('sfc-root'), ('sfc-mid'), ('sfc-leaf'); + +INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES + ('sfc-root', 'all', 'All users', '{}'), + ('sfc-mid', 'all', 'All users', '{}'), + ('sfc-leaf', 'all', 'All users', '{}'); + +INSERT INTO password(email, password_hash, login_type, super_admin, verified, name, username) + VALUES ('sfc-admin@windmill.dev', 'x', 'password', true, true, 'SFC Admin', 'sfc-admin'); + +INSERT INTO usr(workspace_id, email, username, is_admin, role) VALUES + ('sfc-root', 'sfc-admin@windmill.dev', 'sfc-admin', true, 'Admin'), + ('sfc-mid', 'sfc-admin@windmill.dev', 'sfc-admin', true, 'Admin'), + ('sfc-leaf', 'sfc-admin@windmill.dev', 'sfc-admin', true, 'Admin'); + +INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin) + VALUES (encode(sha256('SFC_ADMIN_TOKEN'::bytea), 'hex'), 'SFC_ADMIN_', 'SFC_ADMIN_TOKEN', 'sfc-admin@windmill.dev', 't', true); + +-- Enabling pushes the next run, which needs the scheduled script to exist. +INSERT INTO script (workspace_id, created_by, content, schema, summary, description, path, hash, language, lock) VALUES + ('sfc-leaf', 'sfc-admin', 'export async function main() { return "ok" }', '{}', '', '', 'f/shared/job', 7788001, 'deno', ''); + +INSERT INTO schedule (workspace_id, path, edited_by, edited_at, schedule, enabled, script_path, args, is_flow, email, timezone, extra_perms, permissioned_as) +VALUES + ('sfc-root', 'f/shared/nightly', 'sfc-admin', NOW(), '0 0 0 * * *', true, 'f/shared/job', '{}', false, 'sfc-admin@windmill.dev', 'UTC', '{}', 'u/sfc-admin'), + ('sfc-leaf', 'f/shared/nightly', 'sfc-admin', NOW(), '0 0 0 * * *', false, 'f/shared/job', '{}', false, 'sfc-admin@windmill.dev', 'UTC', '{}', 'u/sfc-admin'), + -- A path only the leaf has: nothing above shares it. + ('sfc-leaf', 'f/shared/own', 'sfc-admin', NOW(), '0 0 0 * * *', false, 'f/shared/job', '{}', false, 'sfc-admin@windmill.dev', 'UTC', '{}', 'u/sfc-admin'); + +GRANT ALL PRIVILEGES ON TABLE workspace_key TO windmill_admin; +GRANT ALL PRIVILEGES ON TABLE workspace_key TO windmill_user; diff --git a/backend/tests/git_sync_fork_credential.rs b/backend/tests/git_sync_fork_credential.rs index 10cf1e3735..9d5eb62dfb 100644 --- a/backend/tests/git_sync_fork_credential.rs +++ b/backend/tests/git_sync_fork_credential.rs @@ -12,8 +12,8 @@ use sqlx::{Pool, Postgres}; use windmill_common::git_sync_ee::{ - create_repo_webhook, git_credential_for_url, repo_provider, repo_supports_managed_git_features, - set_git_credential, GitProvider, + create_repo_webhook, git_app_installations_for, git_credential_for_url, managed_pr_base_branch, + repo_provider, repo_supports_managed_git_features, set_git_credential, GitProvider, }; use windmill_common::workspaces::GitCredentialProvider; @@ -278,3 +278,99 @@ async fn an_unreachable_gitlab_host_is_the_reported_error( ); Ok(()) } + +/// GitHub App installations are normally copied into a fork, but a workspace +/// attached as a dev workspace, or forked before its parent connected the App, +/// holds none, and neither does anything forked from it. The lookup reaches the +/// nearest workspace up the chain that holds some, and the background App path +/// (PR base resolution here) authenticates with that installation's token. +#[sqlx::test(fixtures("git_sync_fork_credential"))] +async fn app_installations_come_from_the_nearest_ancestor_holding_some( + db: Pool, +) -> anyhow::Result<()> { + use axum::{routing::get, Router}; + use std::sync::{Arc, Mutex}; + + // A stand-in GitHub API: one repository, and a record of who asked for it. + let seen: Arc>> = Arc::new(Mutex::new(vec![])); + let app = Router::new().route( + "/api/v3/repos/acme/repo", + get({ + let seen = seen.clone(); + move |headers: axum::http::HeaderMap| { + let seen = seen.clone(); + async move { + let auth = headers + .get("authorization") + .and_then(|v| v.to_str().ok()) + .unwrap_or("") + .to_string(); + seen.lock().unwrap().push(auth); + axum::Json(serde_json::json!({ "default_branch": "trunk" })) + } + } + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await?; + let port = listener.local_addr()?.port(); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + let stub = format!("http://127.0.0.1:{port}"); + + // The root holds the installation, with a cached token so nothing is minted. + sqlx::query( + "UPDATE workspace_settings SET git_app_installations = $1::jsonb WHERE workspace_id = 'parent-ws'", + ) + .bind(serde_json::json!([{ + "installation_id": 42, "account_id": "acme", "jwt_token": "x", + "github_base_url": stub, + "installation_token": "root-token", "installation_token_expiration": 4102444800i64 + }])) + .execute(&db) + .await?; + // The fork's copy of the resource names the App-backed repository. + sqlx::query("UPDATE resource SET value = $1::jsonb WHERE workspace_id = 'deep-fork-ws' AND path = 'u/admin/repo'") + .bind(serde_json::json!({ "url": format!("{stub}/acme/repo.git"), "is_github_app": true })) + .execute(&db) + .await?; + + assert_eq!( + git_app_installations_for(&db, "deep-fork-ws").await?, + ("parent-ws".to_string(), vec![(42, Some(stub.clone()))]), + "two levels down, the root's installations are the ones to use" + ); + assert_eq!( + git_app_installations_for(&db, "orphan-ws").await?, + ("orphan-ws".to_string(), vec![]), + "a workspace with nothing above it resolves nothing" + ); + assert_eq!( + managed_pr_base_branch(&db, "deep-fork-ws", REPO) + .await? + .as_deref(), + Some("trunk"), + "the background App path reaches the repository through the root's installation" + ); + let seen = seen.lock().unwrap().clone(); + assert!( + !seen.is_empty() && seen.iter().all(|auth| auth == "Bearer root-token"), + "every call authenticated with the root's cached token: {seen:?}" + ); + + // A closer holder takes precedence over the root. + sqlx::query( + "UPDATE workspace_settings SET git_app_installations = $1::jsonb WHERE workspace_id = 'fork-ws'", + ) + .bind(serde_json::json!([{ + "installation_id": 7, "account_id": "acme", "jwt_token": "x", + "github_base_url": stub, + "installation_token": "mid-token", "installation_token_expiration": 4102444800i64 + }])) + .execute(&db) + .await?; + assert_eq!( + git_app_installations_for(&db, "deep-fork-ws").await?.0, + "fork-ws", + "the nearest holder wins over the root" + ); + Ok(()) +} diff --git a/backend/tests/schedule_fork_conflict.rs b/backend/tests/schedule_fork_conflict.rs new file mode 100644 index 0000000000..9a7ae8ff6d --- /dev/null +++ b/backend/tests/schedule_fork_conflict.rs @@ -0,0 +1,59 @@ +//! Enabling a schedule in a fork warns about every ancestor sharing the path, +//! not only the direct parent: the row was cloned down the whole chain, so the +//! cron is shared with whichever ancestors still hold a copy. + +use serde_json::json; +use sqlx::{Pool, Postgres}; + +use windmill_test_utils::*; + +async fn set_enabled( + base: &str, + path: &str, + enabled: bool, + force: bool, +) -> anyhow::Result<(u16, String)> { + let resp = reqwest::Client::new() + .post(format!("{base}/api/w/sfc-leaf/schedules/setenabled/{path}")) + .header("Authorization", "Bearer SFC_ADMIN_TOKEN") + .json(&json!({ "enabled": enabled, "force": force })) + .send() + .await?; + Ok((resp.status().as_u16(), resp.text().await?)) +} + +#[sqlx::test(fixtures("schedule_fork_conflict"))] +async fn enabling_in_a_fork_names_the_nearest_ancestor_sharing_the_path( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let base = format!("http://localhost:{}", server.addr.port()); + + let (status, body) = set_enabled(&base, "f/shared/nightly", true, false).await?; + assert_eq!(status, 400, "{body}"); + assert!( + body.contains("fork-conflict:schedule:sfc-root"), + "the middle fork deleted its copy, so the root is the one still sharing the cron: {body}" + ); + + let (status, body) = set_enabled(&base, "f/shared/own", true, false).await?; + assert_eq!( + status, 200, + "a path nothing upstream has enables freely: {body}" + ); + + sqlx::query( + "INSERT INTO schedule (workspace_id, path, edited_by, edited_at, schedule, enabled, script_path, args, is_flow, email, timezone, extra_perms, permissioned_as) + VALUES ('sfc-mid', 'f/shared/nightly', 'sfc-admin', NOW(), '0 0 0 * * *', false, 'f/shared/job', '{}', false, 'sfc-admin@windmill.dev', 'UTC', '{}', 'u/sfc-admin')", + ) + .execute(&db) + .await?; + let (status, body) = set_enabled(&base, "f/shared/nightly", true, false).await?; + assert_eq!(status, 400, "{body}"); + assert!( + body.contains("fork-conflict:schedule:sfc-mid"), + "with the parent holding a copy again, it is the nearest and gets named: {body}" + ); + Ok(()) +} diff --git a/backend/windmill-api-schedule/src/lib.rs b/backend/windmill-api-schedule/src/lib.rs index d581608c10..dfd4a6e3ba 100644 --- a/backend/windmill-api-schedule/src/lib.rs +++ b/backend/windmill-api-schedule/src/lib.rs @@ -1129,35 +1129,23 @@ pub async fn set_enabled( check_scopes(&authed, || format!("schedules:write:{}", path))?; reject_reserved_schedule_path(path)?; - // Block enabling a schedule in a fork when the parent has the same path - // (regardless of parent's enabled flag), unless force=true. Two enabled - // crons fire in lockstep; even when the parent is currently disabled the - // user is likely to re-enable it later, at which point both fire — better - // to surface that risk at every fork-side enable. There's no namespacing - // fix for schedules (Phase 3 doesn't help cron); the user has to confirm - // or point the script at fork-only side effects. + // Block enabling a schedule in a fork when an ancestor has the same path + // (regardless of its enabled flag), unless force=true. Two enabled crons + // fire in lockstep; even when the ancestor is currently disabled the user + // is likely to re-enable it later, at which point both fire — better to + // surface that risk at every fork-side enable. There's no namespacing fix + // for schedules (Phase 3 doesn't help cron); the user has to confirm or + // point the script at fork-only side effects. if payload.enabled && !payload.force { - let parent_id: Option = sqlx::query_scalar!( - "SELECT parent_workspace_id FROM workspace WHERE id = $1", - &w_id + if let Some(ancestor_id) = windmill_common::workspaces::nearest_fork_ancestor_having( + &mut *tx, "schedule", &w_id, path, ) - .fetch_optional(&mut *tx) .await? - .flatten(); - if let Some(parent_id) = parent_id { - let exists: Option = sqlx::query_scalar!( - "SELECT EXISTS(SELECT 1 FROM schedule WHERE workspace_id = $1 AND path = $2)", - &parent_id, - path, - ) - .fetch_one(&mut *tx) - .await?; - if exists == Some(true) { - return Err(Error::BadRequest(format!( - "fork-conflict:schedule:{}", - parent_id - ))); - } + { + return Err(Error::BadRequest(format!( + "fork-conflict:schedule:{}", + ancestor_id + ))); } } let before = trigger_history::snapshot_row(&mut *tx, "schedule", &w_id, path).await?; @@ -1699,9 +1687,9 @@ pub use windmill_queue::schedule::clear_schedule; #[derive(Deserialize)] pub struct SetEnabled { pub enabled: bool, - /// Bypass the parent-state warning when enabling a schedule in a fork - /// whose parent has the same path enabled. The frontend sets this after - /// the user confirms the duplicate-firing dialog. + /// Bypass the fork-conflict warning when enabling a schedule in a fork + /// while an ancestor workspace has the same path. The frontend sets this + /// after the user confirms the duplicate-firing dialog. #[serde(default)] pub force: bool, } diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 9b6a471b2b..7cf910ef84 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1788,6 +1788,48 @@ pub async fn workspace_with_fork_ancestors(db: &crate::DB, w_id: &str) -> Result Ok(chain) } +/// The nearest fork ancestor of `w_id` holding a row at `path` in `table`, or `None` when no +/// ancestor does (or `w_id` is not a fork). Fork creation clones trigger and schedule rows down +/// the whole chain, so a row shares its upstream identifier (Kafka group, PG slot, cron) with +/// every ancestor that still has one, not just the direct parent, which may have deleted its +/// copy since. +/// +/// Runs on the caller's connection so it sees the caller's transaction, and uncached: the +/// answer depends on the target table, not only on lineage. +/// +/// `table` is interpolated into SQL, hence `'static`: a trigger's `TABLE_NAME` or a literal, +/// never caller input. Reads lineage for any `w_id` with no authorization check, like +/// [`fork_ancestor_chain`], so the caller must already be authorized for `w_id`. +pub async fn nearest_fork_ancestor_having( + conn: &mut sqlx::PgConnection, + table: &'static str, + w_id: &str, + path: &str, +) -> Result> { + sqlx::query_scalar(&format!( + r#" + WITH RECURSIVE chain AS ( + SELECT id, parent_workspace_id, 0 AS depth + FROM workspace WHERE id = $1 + UNION ALL + SELECT w.id, w.parent_workspace_id, chain.depth + 1 + FROM workspace w + JOIN chain ON w.id = chain.parent_workspace_id + WHERE chain.depth < 20 + ) + SELECT chain.id FROM chain + JOIN {table} t ON t.workspace_id = chain.id AND t.path = $2 + WHERE chain.depth > 0 + ORDER BY chain.depth LIMIT 1 + "# + )) + .bind(w_id) + .bind(path) + .fetch_optional(&mut *conn) + .await + .map_err(|e| Error::internal_err(format!("resolving fork ancestors of {w_id}: {e:#}"))) +} + lazy_static::lazy_static! { /// workspace id -> (root workspace id, expiry ts). Read once per job start, so correctness /// rests on the invalidation rather than on the TTL: every mutation that can change the answer diff --git a/backend/windmill-trigger/src/handler.rs b/backend/windmill-trigger/src/handler.rs index 953f3a571b..66d6770354 100644 --- a/backend/windmill-trigger/src/handler.rs +++ b/backend/windmill-trigger/src/handler.rs @@ -97,14 +97,14 @@ pub trait TriggerCrud: Send + Sync + 'static { const DEPLOYMENT_NAME: &'static str; const ADDITIONAL_SELECT_FIELDS: &[&'static str] = &[]; const IS_ALLOWED_ON_CLOUD: bool; - /// Whether enabling this trigger in a fork while the parent has the same - /// path enabled is a real conflict (shared upstream resource). True for + /// Whether enabling this trigger in a fork while an ancestor workspace has + /// the same path is a real conflict (shared upstream resource). True for /// listener-based kinds where two consumers compete (Kafka group, PG slot, /// SQS queue, etc.) and for Websocket where both subscribers fire on every /// broadcast. False for kinds whose upstream identifier is implicitly /// workspace-scoped at runtime (HTTP routes, Email local_part — clones for /// the non-workspaced sub-case are filtered out, so any cloned row is - /// already collision-free vs. the parent). + /// already collision-free vs. its ancestors). const FORK_CONFLICT_ON_ENABLE: bool = true; fn get_deployed_object(path: String, parent_path: Option) -> DeployedObject; @@ -1095,54 +1095,14 @@ async fn exists_trigger( #[derive(serde::Deserialize)] struct SetTriggerModePayload { mode: TriggerMode, - /// When true, bypass the parent-state warning that would otherwise reject - /// enabling a trigger that's already enabled in the parent workspace. - /// The frontend sets this after the user confirms the duplicate-execution - /// dialog. See windmill-trigger/src/handler.rs::set_trigger_mode for the - /// full check. + /// When true, bypass the fork-conflict warning that would otherwise reject + /// enabling a trigger an ancestor workspace also has at this path. The + /// frontend sets this after the user confirms the duplicate-execution + /// dialog. See `set_trigger_mode` for the full check. #[serde(default)] force: bool, } -/// Returns the parent workspace id when this workspace is a fork *and* the -/// parent has a row at the same trigger path. Used to gate enabling a trigger -/// in a fork behind an explicit `force=true` confirmation: the fork's row was -/// cloned from the parent, so its upstream identifier (Kafka group, PG slot, -/// SQS queue URL, etc.) is shared by construction. The risk is independent of -/// the parent's current `mode`: if the parent is enabled, the two listeners -/// compete; if it's disabled, the fork can destructively take over shared -/// state (e.g. advance the PG WAL, claim an MQTT client_id) before the parent -/// re-enables. Either way, the user should be asked to confirm. -async fn parent_has_trigger( - tx: &mut PgConnection, - table_name: &str, - workspace_id: &str, - path: &str, -) -> Result> { - let parent: Option = - sqlx::query_scalar("SELECT parent_workspace_id FROM workspace WHERE id = $1") - .bind(workspace_id) - .fetch_optional(&mut *tx) - .await? - .flatten(); - let Some(parent_id) = parent else { - return Ok(None); - }; - let exists: Option = sqlx::query_scalar(&format!( - "SELECT EXISTS(SELECT 1 FROM {} WHERE workspace_id = $1 AND path = $2)", - table_name - )) - .bind(&parent_id) - .bind(path) - .fetch_one(&mut *tx) - .await?; - Ok(if exists == Some(true) { - Some(parent_id) - } else { - None - }) -} - async fn set_trigger_mode( Extension(handler): Extension>, authed: ApiAuthed, @@ -1157,22 +1117,28 @@ async fn set_trigger_mode( let mut tx = user_db.begin(&authed).await?; // Block transitioning a trigger in a fork to any mode that attaches a - // listener (Enabled or Suspended) when the parent has the same path, + // listener (Enabled or Suspended) when an ancestor 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). + // with the ancestor. The risk is independent of the ancestor's current + // mode: enabled, the two listeners compete; disabled, the fork can + // destructively take over shared state (advance the PG WAL, claim an + // MQTT client_id) before it re-enables. 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? + if let Some(ancestor_id) = windmill_common::workspaces::nearest_fork_ancestor_having( + &mut *tx, + T::TABLE_NAME, + &workspace_id, + path, + ) + .await? { return Err(Error::BadRequest(format!( "fork-conflict:{}:{}", T::TRIGGER_TYPE, - parent_id + ancestor_id ))); } } diff --git a/frontend/src/lib/components/ForkConflictModal.svelte b/frontend/src/lib/components/ForkConflictModal.svelte index e0c360320d..03afd71407 100644 --- a/frontend/src/lib/components/ForkConflictModal.svelte +++ b/frontend/src/lib/components/ForkConflictModal.svelte @@ -32,29 +32,29 @@ close(true)} onCanceled={() => close(false)} > {#if state}

- The parent workspace ({state.parentWorkspaceId}) has the same - {state.kindLabel} configured at this path. Because this fork's row was cloned from it, the upstream - identifier is shared. + The upstream workspace ({state.upstreamWorkspaceId}) this fork + descends from has the same {state.kindLabel} configured at this path. Because this fork's row was + cloned from it, the upstream identifier is shared.

{#if family === 'split'} If both are enabled, the two listeners will compete on the same upstream and each side will receive only a fraction of its events. {:else if family === 'duplicate'} - If both are enabled, every event will fire the script twice — once in the fork and once in - the parent. + If both are enabled, every event will fire the script twice: once in the fork and once in + the upstream workspace. {:else if family === 'slot'} The cloned replication_slot_name points at the same Postgres slot, which only allows one consumer at a time. Enabling here will either fail with "slot already active" - if the parent is enabled, or hijack the slot's WAL position if it isn't — causing the parent - to lose events when re-enabled. + if the upstream workspace is enabled, or hijack the slot's WAL position if it isn't, causing + it to lose events when re-enabled. {:else} Enabling it here may compete for the same upstream events or duplicate side effects. {/if} diff --git a/frontend/src/lib/stores.ts b/frontend/src/lib/stores.ts index 229ecff7ad..a3147b171c 100644 --- a/frontend/src/lib/stores.ts +++ b/frontend/src/lib/stores.ts @@ -242,7 +242,7 @@ export type GlobalForkModalState = { export type ForkConflictModalState = { kind: string kindLabel: string - parentWorkspaceId: string + upstreamWorkspaceId: string resolve: (proceed: boolean) => void } diff --git a/frontend/src/lib/utils/forkConflict.ts b/frontend/src/lib/utils/forkConflict.ts index d903790b44..5572e01f51 100644 --- a/frontend/src/lib/utils/forkConflict.ts +++ b/frontend/src/lib/utils/forkConflict.ts @@ -2,14 +2,15 @@ import { forkConflictModal } from '$lib/stores' /** * The backend rejects "enable" requests on triggers/schedules in a fork when - * the parent workspace has the same path enabled. The error body is shaped as - * `fork-conflict::` + * an upstream workspace (the parent, or an ancestor further up) has the same + * path. The error body is shaped as + * `fork-conflict::` * so the UI can show a tailored confirm-to-proceed dialog and re-issue the * call with `force: true` if the user agrees. */ export interface ForkConflict { kind: string - parentWorkspaceId: string + upstreamWorkspaceId: string } export function detectForkConflict(e: unknown): ForkConflict | null { @@ -20,7 +21,7 @@ export function detectForkConflict(e: unknown): ForkConflict | null { : ((body as any)?.error?.message ?? (body as any)?.message ?? (e as any)?.message ?? '') const m = String(raw).match(/fork-conflict:([^:]+):(.+)/) if (!m) return null - return { kind: m[1], parentWorkspaceId: m[2].trim() } + return { kind: m[1], upstreamWorkspaceId: m[2].trim() } } /** @@ -30,11 +31,11 @@ export function detectForkConflict(e: unknown): ForkConflict | null { * on two rows in quick succession), resolve the older promise to false so * the prior caller doesn't hang. */ -function askForkConflictConfirm(kind: string, kindLabel: string, parentWorkspaceId: string) { +function askForkConflictConfirm(kind: string, kindLabel: string, upstreamWorkspaceId: string) { return new Promise((resolve) => { const previous = forkConflictModal.val previous?.resolve(false) - forkConflictModal.val = { kind, kindLabel, parentWorkspaceId, resolve } + forkConflictModal.val = { kind, kindLabel, upstreamWorkspaceId, resolve } }) } @@ -64,7 +65,7 @@ export async function withForkConflictRetry( const proceed = await askForkConflictConfirm( conflict.kind, kindLabel, - conflict.parentWorkspaceId + conflict.upstreamWorkspaceId ) // User explicitly dismissed the modal — treat as a silent no-op so the // caller's catch block doesn't pop a redundant error toast.