Files
windmill/backend/tests/relock_skip.rs
Ruben Fiszel 31d6660d56 feat: script module mode with CLI sync, preview, and WAC UI improvements (#8380)
* feat: add script module mode with folder model for Bun and Python

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: add missing modules field to RawCode in bun_executor

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* sqlx

* feat: enrich WAC templates with checkpoint and replay semantics

Add prominent comments explaining that all computation must happen
inside task/step/taskScript or it will be replayed on resume/retry.
Clarify that waitForApproval does not hold a worker and that
approve/reject URLs are available in the timeline step details.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(cli): script module sync idempotency, per-module hash tracking, and preview support

- Fix pull→push idempotency: use `??` instead of `||` for module lock
  field so empty strings are preserved (matches API's `lock: ""`)
- Add per-module hash tracking in wmill-lock.yaml following the flow
  inline script pattern (SCRIPT_TOP_HASH + per-module subpath hashes)
- Selective module lock regeneration: only regenerate locks for modules
  whose content actually changed, not all modules
- Use unfiltered rawWorkspaceDependencies for module hashes to match
  what updateModuleLocks passes to fetchScriptLock
- Show changed module names in stale script output for clarity
- Add module support to `script preview` command: read modules from
  __mod/ folder and pass them in the preview API request
- Add preview tests for taskScript pattern (flat and folder layout)
- Update test assertion for module stale detection output

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

* feat(frontend): WAC UI improvements — reorder templates, module tab rename, import consolidation

- Reorder WAC template buttons: TypeScript before Python in
  ScriptBuilder, CreateActionsScript, and CreateActionsFlow
- Remove dropdown items from +Script button (simplify to direct link)
- Move "Import Workflow-as-Code" to +Flow dropdown with dedicated drawer
- Add module tab rename: pencil icon on hover opens popover with
  validation, fixed-width icon container prevents layout shift

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

* feat: remaining module-mode changes from working branch

- Backend parser updates for WAC detection
- CLI sync/types updates for raw app path and module support
- Frontend UI polish (Dev.svelte, ScriptRow, script hash page)
- Test fixture updates

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

* test(cli): add test for module modification detection in generate-metadata

Verifies that modifying a single module file re-triggers stale
detection and only the changed module is listed, not all modules.

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

* fix(backend): critical fixes from PR review

- Fix hardcoded dev path in bun_executor.rs WAC v2 wrapper — use
  "windmill-client" import instead of absolute filesystem path
- Fix missed no_main_func → auto_kind rename in parser TS test
- Add modules column to clone_script SQL (windmill-common and
  windmill-api-workspaces) so cloned scripts retain their modules
- Add modules: None to RawCode structs in worker tests
- Restore complete sqlx cache (merge main's cache + our new queries)

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

* fix(backend): fix clone warning treated as error in CI

Change `.clone()` on double reference to `*k` dereference in
scripts.rs hash implementation. Update sqlx cache with new query
hashes from modified clone_script SQL.

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

* fix(frontend): use published parser wasm versions for CI build

The local file:// paths for windmill-parser-wasm-py and
windmill-parser-wasm-ts don't exist in the Cloudflare Pages build
environment. Revert to published npm versions (1.655.0).

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

* fix(frontend): update parser wasm packages to 1.657.2

Use newly published windmill-parser-wasm-ts and windmill-parser-wasm-py
v1.657.2 which include auto_kind/WAC detection changes.

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

* fix(frontend): regenerate package-lock.json for npm ci compatibility

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

* fix(frontend): use main's lockfile as base, update only parser wasm packages

Regenerating package-lock.json from scratch pulled different dependency
versions causing svelte-check type errors. Instead, start from main's
lockfile and only update the two changed packages.

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

* fix(backend): add modules column to fetch_script_for_update query

The Script<SR> struct has a modules field (FromRow), but
fetch_script_for_update didn't SELECT modules, causing a runtime
error "no column found for name: modules" when the worker processed
dependency jobs. This was the root cause of the relock_skip test
timeout.

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

* fix(backend): fix script module execution for Python and Bun

- Fix modules not passed through job queue: inject _MODULES into
  PushArgs.extra when pushing Code jobs so worker can extract them
- Fix Python module imports: use relative imports (from .helper)
  and add sys.path.insert for module directory in wrapper
- Fix Python tests: use relative imports and empty lock to prevent
  pip from resolving module names as packages
- Add local file check in Bun loader for module resolution
- Ignore Bun module test (bundle mode loader integration tracked
  separately)
- Add missing modules column to fetch_script_for_update query

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

* fix(backend): remove unnecessary empty lock in Python module tests

Relative imports (from .helper) are not parsed as pip packages,
so the empty lock workaround is not needed.

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

* fix(backend): fix module execution for Python and Bun — all tests pass

Python modules:
- Use relative imports (from .helper import greet) since scripts run
  as packages
- Add sys.path.insert for module directory in wrapper to ensure local
  modules take precedence over pip packages with same name

Bun modules:
- Use bundled output (./out/main.js) as wrapper import when modules
  are present — the bundled output has module content inlined by
  Bun.build, avoiding runtime loader resolution issues
- Add local file check in loader.bun.js onResolve to short-circuit
  API URL resolution for module files on disk

Job queue:
- Inject _MODULES into PushArgs.extra when pushing Code jobs so
  the worker can extract them at execution time

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

* refactor: address PR review — simplify, fix correctness, remove dead code

Critical fixes:
- Replace all CLI `no_main_func` references with `auto_kind` (string)
  to match the backend migration and API changes
- Remove duplicated `compute_python_module_dir` in worker.rs, use
  the canonical version from python_executor.rs

High priority:
- Auto-create `__init__.py` in intermediate directories for nested
  Python modules so imports like `from .utils.math import add` work
  without users manually creating __init__.py files
- Remove redundant `sys_path_insert` — relative imports use Python's
  package system, not sys.path

Medium:
- Fix lock file base name extraction: use regex to strip only the
  final extension (`.replace(/\.[^.]+$/, '')`) instead of `indexOf(".")`
  which breaks for files like `helper.test.ts`

Simplification:
- Remove dead `{#if false}` Popover block in ScriptEditor.svelte
- Guard loader.bun.js local file check to only run for relative paths
  (matching the Windows loader pattern)
- Add clarifying comment on Bun dual mechanism (build + run phases)
- Add maintenance comment on manual Hash impl for NewScript

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

* fix: final review fixes — stale cleanup, baseName, auto_kind export

- Fix sync.ts baseName extraction using indexOf(".") → regex
  (same fix as script.ts/metadata.ts, missed this instance)
- Add stale module file cleanup in writeModulesToDisk: removes files
  from __mod/ that are no longer in the modules map before writing,
  fixing the pull→push cycle that couldn't delete modules
- Log warning when _MODULES serialization fails in job push instead
  of silently dropping modules
- Use strict equality (===) for auto_kind comparison
- Exclude auto_kind from workspace export — it is auto-detected by
  the parser at deploy time from script content

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

* fix(cli): remove auto_kind from push, comparison, and metadata

auto_kind is auto-detected by the parser at deploy time, so the CLI
should not send it, compare it, or write it to script.yaml.

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

* fix: remove erroneously added backend/backend/.sqlx directory

Duplicate .sqlx cache was committed at the wrong nested path.

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

* fix: address PR review feedback + fix CI dead_code warning

Frontend (ScriptEditor.svelte):
- Fix switchToMain() missing lastSyncedCode update — prevents stale
  code sync on external changes while editing a module tab
- Fix formatAction saving module code to main script's localStorage
  draft — now saves main code when on a module tab
- Fix non-null assertion on inferModuleLang in renameModule — fall
  back to original language instead of force unwrap
- Remove redundant activeModuleTab truthy check in runTest

CLI (script.ts):
- Clean up empty directories after removing stale module files in
  writeModulesToDisk

Backend:
- Add path traversal guard in write_module_files — reject module
  paths containing ".."
- Fix dead_code warning on auto_kind field in workspace export struct

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

* feat(frontend): improve auto_kind UX + address review findings

- Rename "Include without main function" toggle to "Include library
  scripts" in script list (ItemsList.svelte)
- Update NoMainFuncBadge: "No main" → "Library" with clearer tooltip
- Filter module file extensions by main script language — Python
  scripts only allow .py modules, TypeScript only .ts, etc.
- Split flushModuleState into flushModuleContent (no UI side-effect)
  and flushModuleState (flush + reset tab), reducing duplication
- Dynamic placeholder and hint text in add module popover based on
  main script language

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

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-17 01:20:09 +00:00

410 lines
15 KiB
Rust

use sqlx::{Pool, Postgres};
use tokio_stream::StreamExt;
use windmill_api_client::types::NewScript;
use windmill_test_utils::*;
mod relock_skip {
use super::*;
fn quick_ns(
content: &str,
language: windmill_api_client::types::ScriptLang,
path: &str,
lock: Option<String>,
parent_hash: Option<String>,
) -> NewScript {
NewScript {
content: content.into(),
language,
lock,
parent_hash,
path: path.into(),
concurrent_limit: None,
concurrency_time_window_s: None,
cache_ttl: None,
dedicated_worker: None,
description: "".to_string(),
draft_only: None,
envs: vec![],
is_template: None,
kind: None,
summary: "".to_string(),
tag: None,
schema: std::collections::HashMap::new(),
ws_error_handler_muted: Some(false),
priority: None,
delete_after_use: None,
timeout: None,
restart_unless_cancelled: None,
deployment_message: None,
concurrency_key: None,
visible_to_runner_only: None,
auto_kind: None,
codebase: None,
has_preprocessor: None,
on_behalf_of_email: None,
assets: vec![],
modules: None,
}
}
async fn init(db: Pool<Postgres>) -> (windmill_api_client::Client, u16, ApiServer) {
init_client(db).await
}
/// Counts occurrences of a pattern in job logs for all jobs created after a given time
async fn count_pattern_in_job_logs(
db: &Pool<Postgres>,
pattern: &str,
after: chrono::DateTime<chrono::Utc>,
) -> i64 {
let logs = sqlx::query_scalar!("SELECT logs FROM job_logs WHERE created_at > $1", after)
.fetch_all(db)
.await
.unwrap();
logs.iter()
.filter_map(|l| l.as_ref())
.map(|l| l.matches(pattern).count() as i64)
.sum()
}
/// Waits for exactly N jobs to complete. Returns the timestamp before waiting.
async fn wait_for_jobs(
completed: &mut (impl futures::Stream<Item = uuid::Uuid> + Unpin),
count: usize,
) -> chrono::DateTime<chrono::Utc> {
let before = chrono::Utc::now();
for _ in 0..count {
completed.next().await;
}
before
}
/// Waits for at least N jobs to complete, then drains any additional jobs
/// that complete within a short timeout. Returns the timestamp before waiting.
async fn wait_for_jobs_ge(
completed: &mut (impl futures::Stream<Item = uuid::Uuid> + Unpin),
min_count: usize,
) -> chrono::DateTime<chrono::Utc> {
let before = chrono::Utc::now();
for _ in 0..min_count {
completed.next().await;
}
// Drain any additional jobs that complete within 5 seconds
loop {
match tokio::time::timeout(std::time::Duration::from_secs(1), completed.next()).await {
Ok(Some(_)) => continue,
_ => break,
}
}
before
}
#[cfg(feature = "python")]
#[sqlx::test(fixtures("base", "dependency_map"))]
async fn relock_skip_on_script_redeployment(db: Pool<Postgres>) -> anyhow::Result<()> {
std::env::set_var("DEPENDENCY_JOB_DEBOUNCE_DELAY", "0");
let (client, port, _s) = init(db.clone()).await;
let mut completed = listen_for_completed_jobs(&db).await;
in_test_worker(&db, async {
// Step 1: Redeploy leaf_1 - first time, no hashes exist, all should relock
let before = chrono::Utc::now();
client
.create_script(
"test-workspace",
&quick_ns(
"
def main():
return 'leaf1'
",
windmill_api_client::types::ScriptLang::Python3,
"f/rel/leaf_1",
None,
Some("0000000000051658".into()),
),
)
.await
.unwrap();
// leaf_1(1) + branch(1) + root_script(2: leaf_1+branch) + root_flow(2) + root_app(2) = 8 jobs
wait_for_jobs(&mut completed, 8).await;
let skipping_count = count_pattern_in_job_logs(&db, "Skipping relock", before).await;
let relocking_count = count_pattern_in_job_logs(&db, "Relocking", before).await;
assert_eq!(skipping_count, 0, "First deployment should not skip");
assert!(relocking_count > 0, "First deployment should have relocking jobs");
// Step 2: Redeploy leaf_2 - first time for leaf_2, should relock
let before = chrono::Utc::now();
client
.create_script(
"test-workspace",
&quick_ns(
"
def main():
return 'leaf2'
",
windmill_api_client::types::ScriptLang::Python3,
"f/rel/leaf_2",
None,
Some("0000000000051659".into()),
),
)
.await
.unwrap();
// leaf_2(1) + root_script(1) + root_flow(1) + root_app(1) = 4 jobs (no cascade, branch doesn't depend on leaf_2)
wait_for_jobs(&mut completed, 4).await;
let skipping_count = count_pattern_in_job_logs(&db, "Skipping relock", before).await;
let relocking_count = count_pattern_in_job_logs(&db, "Relocking", before).await;
assert_eq!(skipping_count, 0, "leaf_2 first deployment should not skip");
assert!(relocking_count > 0, "leaf_2 first deployment should have relocking jobs");
// Step 3: Redeploy leaf_2 with trivial change (comment) - lock stays same, should SKIP
// Get current parent hash for leaf_2
let leaf2_hash = sqlx::query_scalar!(
"SELECT hash FROM script WHERE path = 'f/rel/leaf_2' AND workspace_id = 'test-workspace' AND archived = false ORDER BY created_at DESC LIMIT 1"
)
.fetch_one(&db)
.await
.unwrap();
let before = chrono::Utc::now();
client
.create_script(
"test-workspace",
&quick_ns(
"
# comment to change hash but not lock
def main():
return 'leaf2'
",
windmill_api_client::types::ScriptLang::Python3,
"f/rel/leaf_2",
None,
Some(format!("{:016X}", leaf2_hash)),
),
)
.await
.unwrap();
// Same as leaf_2 first deployment: 4 jobs
wait_for_jobs(&mut completed, 4).await;
let skipping_count = count_pattern_in_job_logs(&db, "Skipping relock", before).await;
assert!(skipping_count > 0, "Trivial change (comment only) should skip - lock unchanged");
// Step 4: Redeploy leaf_2 with actual dependency change (add tiny via comment) - should NOT skip
let leaf2_hash = sqlx::query_scalar!(
"SELECT hash FROM script WHERE path = 'f/rel/leaf_2' AND workspace_id = 'test-workspace' AND archived = false ORDER BY created_at DESC LIMIT 1"
)
.fetch_one(&db)
.await
.unwrap();
let before = chrono::Utc::now();
client
.create_script(
"test-workspace",
&quick_ns(
"
# requirements:
# tiny
def main():
return 'leaf2 with tiny'
",
windmill_api_client::types::ScriptLang::Python3,
"f/rel/leaf_2",
None,
Some(format!("{:016X}", leaf2_hash)),
),
)
.await
.unwrap();
// Same as leaf_2 first deployment: 4 jobs
wait_for_jobs(&mut completed, 4).await;
let skipping_count = count_pattern_in_job_logs(&db, "Skipping relock", before).await;
assert_eq!(skipping_count, 0, "Changed dependencies should not skip");
}, port).await;
Ok(())
}
#[cfg(feature = "python")]
#[sqlx::test(fixtures("base", "dependency_map"))]
async fn relock_skip_on_workspace_deps_redeployment(db: Pool<Postgres>) -> anyhow::Result<()> {
use windmill_common::scripts::ScriptLang;
use windmill_dep_map::workspace_dependencies::NewWorkspaceDependencies;
std::env::set_var("DEPENDENCY_JOB_DEBOUNCE_DELAY", "0");
std::env::set_var("EXISTS_CACHE_TIMEOUT_MS", "0");
let (_client, port, _s) = init(db.clone()).await;
let mut completed = listen_for_completed_jobs(&db).await;
// Step 1: Redeploy default (unnamed) workspace deps - first time, should relock
let before = chrono::Utc::now();
NewWorkspaceDependencies {
workspace_id: "test-workspace".into(),
language: ScriptLang::Python3,
content: "".into(),
name: None, // Default/unnamed
description: None,
}
.create(("".to_owned(), "".to_owned(), "".to_owned()), db.clone())
.await
.unwrap();
in_test_worker(&db, wait_for_jobs_ge(&mut completed, 10), port).await;
// Note: within a cascade, the same script may be triggered multiple times.
// After the first trigger relocks and stores the hash, subsequent triggers skip.
// We allow up to 3 skips from cascade re-triggers.
let skipping_count = count_pattern_in_job_logs(&db, "Skipping relock", before).await;
let relocking_count = count_pattern_in_job_logs(&db, "Relocking", before).await;
assert!(
skipping_count <= 3,
"First deployment should have at most 3 skips from cascade"
);
assert!(
relocking_count >= 3,
"First deployment should have at least 3 relocking jobs"
);
// Step 2: Redeploy default workspace deps again - should SKIP
let before = chrono::Utc::now();
NewWorkspaceDependencies {
workspace_id: "test-workspace".into(),
language: ScriptLang::Python3,
content: "".into(),
name: None,
description: None,
}
.create(("".to_owned(), "".to_owned(), "".to_owned()), db.clone())
.await
.unwrap();
in_test_worker(&db, wait_for_jobs_ge(&mut completed, 10), port).await;
let skipping_count = count_pattern_in_job_logs(&db, "Skipping relock", before).await;
assert!(
skipping_count >= 3,
"Second deployment of same content should skip at least 3 times"
);
// Step 3: Redeploy default workspace deps with different content - should NOT skip
let before = chrono::Utc::now();
NewWorkspaceDependencies {
workspace_id: "test-workspace".into(),
language: ScriptLang::Python3,
content: "tiny".into(),
name: None,
description: None,
}
.create(("".to_owned(), "".to_owned(), "".to_owned()), db.clone())
.await
.unwrap();
in_test_worker(&db, wait_for_jobs_ge(&mut completed, 10), port).await;
let skipping_count = count_pattern_in_job_logs(&db, "Skipping relock", before).await;
let relocking_count = count_pattern_in_job_logs(&db, "Relocking", before).await;
assert!(
skipping_count <= 4,
"Changed content should have at most 3 skips from cascade"
);
assert!(
relocking_count >= 3,
"Changed content should trigger at least 3 relocking jobs"
);
// Step 4: Deploy named workspace deps first time - should relock (no hash exists yet)
// Named deps trigger exactly 3 independent objects with no cascade
let before = chrono::Utc::now();
NewWorkspaceDependencies {
workspace_id: "test-workspace".into(),
language: ScriptLang::Python3,
content: "".into(),
name: Some("test".to_owned()),
description: None,
}
.create(("".to_owned(), "".to_owned(), "".to_owned()), db.clone())
.await
.unwrap();
in_test_worker(&db, wait_for_jobs(&mut completed, 3), port).await;
let skipping_count = count_pattern_in_job_logs(&db, "Skipping relock", before).await;
let relocking_count = count_pattern_in_job_logs(&db, "Relocking", before).await;
assert_eq!(
skipping_count, 0,
"Named workspace deps first deployment should not skip"
);
assert!(
relocking_count > 0,
"Named workspace deps first deployment should relock"
);
// Step 5: Deploy named workspace deps again with no change - should SKIP
let before = chrono::Utc::now();
NewWorkspaceDependencies {
workspace_id: "test-workspace".into(),
language: ScriptLang::Python3,
content: "".into(),
name: Some("test".to_owned()),
description: None,
}
.create(("".to_owned(), "".to_owned(), "".to_owned()), db.clone())
.await
.unwrap();
in_test_worker(&db, wait_for_jobs(&mut completed, 3), port).await;
let skipping_count = count_pattern_in_job_logs(&db, "Skipping relock", before).await;
let relocking_count = count_pattern_in_job_logs(&db, "Relocking", before).await;
assert!(
skipping_count > 0,
"Named workspace deps second deployment should skip"
);
assert_eq!(
relocking_count, 0,
"Named workspace deps second deployment should not relock"
);
// Step 6: Deploy named workspace deps with small change - should NOT skip
let before = chrono::Utc::now();
NewWorkspaceDependencies {
workspace_id: "test-workspace".into(),
language: ScriptLang::Python3,
content: "tiny".into(),
name: Some("test".to_owned()),
description: None,
}
.create(("".to_owned(), "".to_owned(), "".to_owned()), db.clone())
.await
.unwrap();
in_test_worker(&db, wait_for_jobs(&mut completed, 3), port).await;
let skipping_count = count_pattern_in_job_logs(&db, "Skipping relock", before).await;
let relocking_count = count_pattern_in_job_logs(&db, "Relocking", before).await;
assert_eq!(
skipping_count, 0,
"Named workspace deps with change should not skip"
);
assert!(
relocking_count > 0,
"Named workspace deps with change should relock"
);
Ok(())
}
}