diff --git a/backend/.sqlx/query-6fb0cdaffcbda46868d5049057bc2898a523acb7e4ac13eb803c12553de34b62.json b/backend/.sqlx/query-6fb0cdaffcbda46868d5049057bc2898a523acb7e4ac13eb803c12553de34b62.json new file mode 100644 index 0000000000..64c1f3e943 --- /dev/null +++ b/backend/.sqlx/query-6fb0cdaffcbda46868d5049057bc2898a523acb7e4ac13eb803c12553de34b62.json @@ -0,0 +1,19 @@ +{ + "db_name": "PostgreSQL", + "query": "\n INSERT INTO script\n (workspace_id, hash, path, parent_hashes, summary, description, content, created_by, schema, is_template, extra_perms, lock, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels, lock_error_logs, created_at, apply_to_perpetual_runs)\n\n SELECT workspace_id, $1, path, array_prepend($2::bigint, COALESCE(parent_hashes, '{}'::bigint[])), summary, description, content, created_by, schema, is_template, extra_perms, $4::text, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, COALESCE($5::jsonb, modules), labels, $6::text, clock_timestamp(), apply_to_perpetual_runs\n\n FROM script WHERE hash = $2 AND workspace_id = $3;\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int8", + "Int8", + "Text", + "Text", + "Jsonb", + "Text" + ] + }, + "nullable": [] + }, + "hash": "6fb0cdaffcbda46868d5049057bc2898a523acb7e4ac13eb803c12553de34b62" +} diff --git a/backend/.sqlx/query-ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637.json b/backend/.sqlx/query-ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637.json deleted file mode 100644 index bea6be2a7f..0000000000 --- a/backend/.sqlx/query-ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n INSERT INTO script\n (workspace_id, hash, path, parent_hashes, summary, description, content, created_by, schema, is_template, extra_perms, lock, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels, lock_error_logs, created_at)\n\n SELECT workspace_id, $1, path, array_prepend($2::bigint, COALESCE(parent_hashes, '{}'::bigint[])), summary, description, content, created_by, schema, is_template, extra_perms, $4::text, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, COALESCE($5::jsonb, modules), labels, $6::text, clock_timestamp()\n\n FROM script WHERE hash = $2 AND workspace_id = $3;\n ", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Int8", - "Int8", - "Text", - "Text", - "Jsonb", - "Text" - ] - }, - "nullable": [] - }, - "hash": "ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637" -} diff --git a/backend/.sqlx/query-cf14e4d52fb9d35141a76e6d38973fb2000ad85d1a4e065c6ef4305a4b4e800e.json b/backend/.sqlx/query-fa5009b8f8582408969c47db184f2d5cb52ab4db7023f10ea4269c3ae4176a6d.json similarity index 55% rename from backend/.sqlx/query-cf14e4d52fb9d35141a76e6d38973fb2000ad85d1a4e065c6ef4305a4b4e800e.json rename to backend/.sqlx/query-fa5009b8f8582408969c47db184f2d5cb52ab4db7023f10ea4269c3ae4176a6d.json index c15abe4649..2dd85d3571 100644 --- a/backend/.sqlx/query-cf14e4d52fb9d35141a76e6d38973fb2000ad85d1a4e065c6ef4305a4b4e800e.json +++ b/backend/.sqlx/query-fa5009b8f8582408969c47db184f2d5cb52ab4db7023f10ea4269c3ae4176a6d.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT on_behalf_of IS NOT NULL AS \"ran_on_behalf_of!\" FROM script WHERE hash = $1 AND workspace_id = $2", + "query": "SELECT (on_behalf_of IS NOT NULL OR on_behalf_of_email IS NOT NULL) AS \"ran_on_behalf_of!\" FROM script WHERE hash = $1 AND workspace_id = $2", "describe": { "columns": [ { @@ -19,5 +19,5 @@ null ] }, - "hash": "cf14e4d52fb9d35141a76e6d38973fb2000ad85d1a4e065c6ef4305a4b4e800e" + "hash": "fa5009b8f8582408969c47db184f2d5cb52ab4db7023f10ea4269c3ae4176a6d" } diff --git a/backend/tests/perpetual_version_switch.rs b/backend/tests/perpetual_version_switch.rs index 716234ee03..e9b1dd1c0a 100644 --- a/backend/tests/perpetual_version_switch.rs +++ b/backend/tests/perpetual_version_switch.rs @@ -4,6 +4,7 @@ use serde_json::{json, Value}; use sqlx::{types::Json, Pool, Postgres}; use uuid::Uuid; +use windmill_common::scripts::{deploy_relocked_version, fetch_script_for_update}; use windmill_queue::{add_completed_job, get_mini_completed_job}; const W_ID: &str = "test-workspace"; @@ -113,6 +114,26 @@ async fn a_flagged_newer_version_takes_over_the_next_run(db: Pool) -> Ok(()) } +/// A dependency change relocks a version into a new one, which has to keep the flag or the loop +/// never moves. +#[sqlx::test(fixtures("base"))] +async fn a_flagged_version_relocked_before_the_run_ends_still_takes_over( + db: Pool, +) -> anyhow::Result<()> { + insert_version(&db, 4001, 60.0, None, None, None).await?; + insert_version(&db, 4002, 0.0, None, Some(true), None).await?; + let mut tx = db.begin().await?; + let head = fetch_script_for_update(PATH, W_ID, &mut *tx) + .await? + .unwrap(); + let relocked = deploy_relocked_version(&mut tx, head, None, Some(""), None, None).await?; + tx.commit().await?; + + let (hash, _, _) = next_run_after_run_of(&db, 4001).await?; + assert_eq!(hash, relocked); + Ok(()) +} + #[sqlx::test(fixtures("base"))] async fn a_newer_version_without_the_flag_leaves_the_run_on_its_version( db: Pool, diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index 871ae6e73c..514de3e1ab 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -648,8 +648,8 @@ struct CreateScriptQuery { #[serde(default)] skip_if_noop: bool, /// Perpetual runs of an older version at this path start their next run on the deployed - /// version once their current one finishes. A deploy skipped as a no-op creates no version, - /// so it moves nothing. + /// version once their current one finishes, or stop when it is not perpetual. A deploy skipped + /// as a no-op creates no version, so it moves nothing. #[serde(default)] apply_to_perpetual_runs: bool, } diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 7d5bc7d80f..f966d20290 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -10200,7 +10200,8 @@ paths: description: | Move perpetual runs of an older version at this path to the deployed version. Each run finishes on its own version, and the next one starts on this version with its settings - and the finished run's arguments. + and the finished run's arguments. If this version is not perpetual, the runs stop + after their current run instead. in: query schema: type: boolean diff --git a/backend/windmill-common/src/scripts.rs b/backend/windmill-common/src/scripts.rs index 628ae2a709..fc8eedc4aa 100644 --- a/backend/windmill-common/src/scripts.rs +++ b/backend/windmill-common/src/scripts.rs @@ -465,7 +465,7 @@ pub async fn deploy_relocked_version( dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, \ delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, \ codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels, \ - lock_error_logs, created_at) + lock_error_logs, created_at, apply_to_perpetual_runs) SELECT workspace_id, $1, path, array_prepend($2::bigint, COALESCE(parent_hashes, '{}'::bigint[])), summary, description, \ content, created_by, schema, is_template, extra_perms, $4::text, language, kind, tag, \ @@ -473,7 +473,7 @@ pub async fn deploy_relocked_version( dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, \ delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, \ codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, COALESCE($5::jsonb, modules), labels, \ - $6::text, clock_timestamp() + $6::text, clock_timestamp(), apply_to_perpetual_runs FROM script WHERE hash = $2 AND workspace_id = $3; ", new_hash, s.hash.0, w_id, lock, modules_json, lock_error_logs).execute(&mut **tx).await?; diff --git a/backend/windmill-queue/src/jobs.rs b/backend/windmill-queue/src/jobs.rs index cdfdb41910..c142789334 100644 --- a/backend/windmill-queue/src/jobs.rs +++ b/backend/windmill-queue/src/jobs.rs @@ -1788,10 +1788,11 @@ async fn perpetual_version_switch( None => { // A run of a version with an on-behalf-of identity is permissioned as that identity, // not as whoever started the loop. Carrying it over would run a version that no - // longer names it with its authority. + // longer names it with its authority. An address-only row counts: a server predating + // the principal column dispatched it as that address. let ran_on_behalf_of = sqlx::query_scalar!( - "SELECT on_behalf_of IS NOT NULL AS \"ran_on_behalf_of!\" FROM script \ - WHERE hash = $1 AND workspace_id = $2", + "SELECT (on_behalf_of IS NOT NULL OR on_behalf_of_email IS NOT NULL) \ + AS \"ran_on_behalf_of!\" FROM script WHERE hash = $1 AND workspace_id = $2", hash.0, w_id ) @@ -1811,6 +1812,27 @@ async fn perpetual_version_switch( ) } }; + // The loop's tag was checked when it started; a new version's own tag has not been. + if latest.dedicated_worker != Some(true) { + if let Some(tag) = latest.tag.as_deref().filter(|t| !t.is_empty()) { + let is_super_admin = windmill_common::auth::is_super_admin_email(db, &email).await?; + if let Err(e) = windmill_common::jobs::check_tag_available_for_workspace_internal( + db, + w_id, + tag, + is_super_admin, + None, + ) + .await + { + tracing::warn!( + "Perpetual script {path} stays on version {hash}: version {} has tag {tag}: {e}", + ScriptHash(latest.hash) + ); + return Ok(PerpetualVersionSwitch::Stay); + } + } + } Ok(PerpetualVersionSwitch::To(PerpetualNextRun { hash: ScriptHash(latest.hash), language: latest.language, diff --git a/cli/src/commands/script/script.ts b/cli/src/commands/script/script.ts index 46ddcd8bdd..2d5552b110 100644 --- a/cli/src/commands/script/script.ts +++ b/cli/src/commands/script/script.ts @@ -706,6 +706,13 @@ export async function handleFile( deepEqual(modules ?? null, remote.modules ?? null)) ) { log.info(colors.green(`Script ${remotePath} is up to date`)); + if (opts?.applyToPerpetualRuns) { + log.warn( + colors.yellow( + `No new version of ${remotePath} was deployed, so --apply-to-perpetual-runs moves no runs` + ) + ); + } // Even when the body is unchanged, perms may still drift — sync them // independently before returning. await applyExtraPermsDiff( @@ -2226,7 +2233,7 @@ const command = new Command() .option("--message ", "Deployment message") .option( "--apply-to-perpetual-runs", - "Move running perpetual runs of this script to the new version once their current run finishes", + "Move running perpetual runs of this script to the new version once their current run finishes, or stop them if the new version is not perpetual", ) .action(push as any) .command("get", "get a script's details") diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 14196e183d..bfb1e79b66 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -6873,7 +6873,7 @@ const command = new Command() ) .option( "--apply-to-perpetual-runs", - "Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes", + "Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes, or stop them if the new version is not perpetual", ) .action(push as any) // Internal: invoked only by the git-sync hub script. Hidden from help and diff --git a/cli/src/guidance/skills.gen.ts b/cli/src/guidance/skills.gen.ts index bdcfd5922e..1d2dd0a6b3 100644 --- a/cli/src/guidance/skills.gen.ts +++ b/cli/src/guidance/skills.gen.ts @@ -7697,7 +7697,7 @@ script related commands - \`--json\` - Output as JSON (for piping to jq) - \`script push \` - push a local script spec. This overrides any remote versions. Use the script file (.ts, .js, .py, .sh) - \`--message \` - Deployment message - - \`--apply-to-perpetual-runs\` - Move running perpetual runs of this script to the new version once their current run finishes + - \`--apply-to-perpetual-runs\` - Move running perpetual runs of this script to the new version once their current run finishes, or stop them if the new version is not perpetual - \`script get \` - get a script's details - \`--json\` - Output as JSON (for piping to jq) - \`script show \` - show a script's content (alias for get) @@ -7793,7 +7793,7 @@ sync local with a remote workspaces or the opposite (push or pull) - \`--locks-required\` - Fail if scripts or flow inline scripts that need locks have no locks - \`--auto-metadata\` - Automatically regenerate stale metadata (locks and schemas) before pushing - \`--accept-overriding-permissioned-as-with-self\` - Accept that items with a different permissioned_as will be updated with your own user - - \`--apply-to-perpetual-runs\` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes + - \`--apply-to-perpetual-runs\` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes, or stop them if the new version is not perpetual ### token diff --git a/frontend/src/lib/components/scripts/perpetualRuns.ts b/frontend/src/lib/components/scripts/perpetualRuns.ts index 5259aedf91..a2c6880f51 100644 --- a/frontend/src/lib/components/scripts/perpetualRuns.ts +++ b/frontend/src/lib/components/scripts/perpetualRuns.ts @@ -7,7 +7,7 @@ export type PerpetualRunsAtPath = { count: number /** More runs than one page holds are queued at the path, so `count` is a lower bound. */ truncated: boolean - /** Arguments the deployed schema adds as required, removes, or retypes, compared with the runs' versions. */ + /** Arguments the deployed schema removes, retypes or newly requires, compared with the runs' versions. */ mismatchedArgs: string[] } @@ -22,23 +22,35 @@ export async function loadPerpetualRunsAtPath( jobKinds: 'script', perPage: RUNS_PAGE_SIZE }) - // A flow step running this script never restarts, whatever the script's perpetual setting. - const runs = queued.filter((job) => !job.parent_job) + // Only what the backend restarts: never a flow step, and only a run of a perpetual version. + const candidates = queued.filter((job) => !job.is_flow_step && job.script_hash) + const hashes = [...new Set(candidates.map((job) => job.script_hash!))] + const versions = new Map( + await Promise.all( + hashes.map( + async (hash) => + [ + hash, + await ScriptService.getScriptByHash({ workspace, hash }).catch(() => undefined) + ] as const + ) + ) + ) + const runs = candidates.filter((job) => versions.get(job.script_hash!)?.restart_unless_cancelled) if (runs.length === 0) { return undefined } - const hashes = [...new Set(runs.flatMap((job) => (job.script_hash ? [job.script_hash] : [])))] - const versions = await Promise.all( - hashes.map((hash) => ScriptService.getScriptByHash({ workspace, hash }).catch(() => undefined)) - ) const mismatchedArgs = new Set() - for (const version of versions) { - if (!version) continue - for (const [arg, { diff }] of Object.entries(computeDiff(schema, version.schema))) { - // A new optional argument takes its default when the reused arguments lack it. - if (diff === 'same' || (diff === 'added' && !schema?.required?.includes(arg))) continue - mismatchedArgs.add(arg) + for (const hash of new Set(runs.map((job) => job.script_hash!))) { + const previous = versions.get(hash)?.schema + for (const [arg, { diff }] of Object.entries(computeDiff(schema, previous))) { + // An added argument only breaks a reused run when it is required, checked below. + if (diff !== 'same' && diff !== 'added') mismatchedArgs.add(arg) + } + const previouslyRequired: unknown[] = Array.isArray(previous?.required) ? previous.required : [] + for (const arg of schema?.required ?? []) { + if (!previouslyRequired.includes(arg)) mismatchedArgs.add(arg) } } diff --git a/system_prompts/auto-generated/cli/cli-commands.md b/system_prompts/auto-generated/cli/cli-commands.md index f3228c1ebd..8ac4dac17d 100644 --- a/system_prompts/auto-generated/cli/cli-commands.md +++ b/system_prompts/auto-generated/cli/cli-commands.md @@ -556,7 +556,7 @@ script related commands - `--json` - Output as JSON (for piping to jq) - `script push ` - push a local script spec. This overrides any remote versions. Use the script file (.ts, .js, .py, .sh) - `--message ` - Deployment message - - `--apply-to-perpetual-runs` - Move running perpetual runs of this script to the new version once their current run finishes + - `--apply-to-perpetual-runs` - Move running perpetual runs of this script to the new version once their current run finishes, or stop them if the new version is not perpetual - `script get ` - get a script's details - `--json` - Output as JSON (for piping to jq) - `script show ` - show a script's content (alias for get) @@ -652,7 +652,7 @@ sync local with a remote workspaces or the opposite (push or pull) - `--locks-required` - Fail if scripts or flow inline scripts that need locks have no locks - `--auto-metadata` - Automatically regenerate stale metadata (locks and schemas) before pushing - `--accept-overriding-permissioned-as-with-self` - Accept that items with a different permissioned_as will be updated with your own user - - `--apply-to-perpetual-runs` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes + - `--apply-to-perpetual-runs` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes, or stop them if the new version is not perpetual ### token diff --git a/system_prompts/auto-generated/prompts.ts b/system_prompts/auto-generated/prompts.ts index 82e9ecac37..e58276d23e 100644 --- a/system_prompts/auto-generated/prompts.ts +++ b/system_prompts/auto-generated/prompts.ts @@ -3841,7 +3841,7 @@ script related commands - \`--json\` - Output as JSON (for piping to jq) - \`script push \` - push a local script spec. This overrides any remote versions. Use the script file (.ts, .js, .py, .sh) - \`--message \` - Deployment message - - \`--apply-to-perpetual-runs\` - Move running perpetual runs of this script to the new version once their current run finishes + - \`--apply-to-perpetual-runs\` - Move running perpetual runs of this script to the new version once their current run finishes, or stop them if the new version is not perpetual - \`script get \` - get a script's details - \`--json\` - Output as JSON (for piping to jq) - \`script show \` - show a script's content (alias for get) @@ -3937,7 +3937,7 @@ sync local with a remote workspaces or the opposite (push or pull) - \`--locks-required\` - Fail if scripts or flow inline scripts that need locks have no locks - \`--auto-metadata\` - Automatically regenerate stale metadata (locks and schemas) before pushing - \`--accept-overriding-permissioned-as-with-self\` - Accept that items with a different permissioned_as will be updated with your own user - - \`--apply-to-perpetual-runs\` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes + - \`--apply-to-perpetual-runs\` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes, or stop them if the new version is not perpetual ### token diff --git a/system_prompts/auto-generated/skills/cli-commands/SKILL.md b/system_prompts/auto-generated/skills/cli-commands/SKILL.md index 8ef259bc10..b75d248dbe 100644 --- a/system_prompts/auto-generated/skills/cli-commands/SKILL.md +++ b/system_prompts/auto-generated/skills/cli-commands/SKILL.md @@ -561,7 +561,7 @@ script related commands - `--json` - Output as JSON (for piping to jq) - `script push ` - push a local script spec. This overrides any remote versions. Use the script file (.ts, .js, .py, .sh) - `--message ` - Deployment message - - `--apply-to-perpetual-runs` - Move running perpetual runs of this script to the new version once their current run finishes + - `--apply-to-perpetual-runs` - Move running perpetual runs of this script to the new version once their current run finishes, or stop them if the new version is not perpetual - `script get ` - get a script's details - `--json` - Output as JSON (for piping to jq) - `script show ` - show a script's content (alias for get) @@ -657,7 +657,7 @@ sync local with a remote workspaces or the opposite (push or pull) - `--locks-required` - Fail if scripts or flow inline scripts that need locks have no locks - `--auto-metadata` - Automatically regenerate stale metadata (locks and schemas) before pushing - `--accept-overriding-permissioned-as-with-self` - Accept that items with a different permissioned_as will be updated with your own user - - `--apply-to-perpetual-runs` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes + - `--apply-to-perpetual-runs` - Move running perpetual runs of the scripts this push deploys to their new version once their current run finishes, or stop them if the new version is not perpetual ### token