diff --git a/AGENTS.md b/AGENTS.md index bbf3952325..e39d4ebede 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -23,6 +23,7 @@ Open-source platform for internal tools, workflows, API integrations, background - **Code review**: review the current PR or branch against the shared review policy in `REVIEW.md` (severity triage, public-surface checklist, AGENTS.md compliance, test-coverage assessment). The skill at `.agents/skills/local-review/SKILL.md` orchestrates it. All three CLIs auto-discover the same SKILL — Claude reads `.claude/skills/` (symlinked to the canonical `.agents/skills/` file), Codex and Pi read `.agents/skills/` directly. Invoke with `/local-review` in Claude Code, `$local-review` (or `/skills` selector) in Codex, or `pi --skill local-review` / `/skill:local-review` in Pi. For a Codex-driven pass that mirrors the `codex-pr-review` GitHub action against your unpushed work (committed + uncommitted) before you push, use `/local-review-codex` (`.agents/skills/local-review-codex/`) — same `REVIEW.md` policy, `gpt-5.6-sol`, `xhigh` reasoning; requires the `codex` CLI >= 0.144.1. - **Domain guides**: `.claude/skills/native-trigger/` and `frontend/tutorial-system-guide.mdc` - **Brand/UI guidelines**: `frontend/brand-guidelines.md` +- **Domain vocabulary**: `CONTEXT.md` — the words this codebase uses for its own concepts (step, step setting, trigger step, …). Name things the way it does. - **CLI commands**: when adding/modifying/removing a command, subcommand, option, or description in `cli/src/commands/`, run `python system_prompts/generate.py` to refresh `system_prompts/auto-generated/` and `cli/src/guidance/skills.gen.ts`. The CLI docs the agents use to operate `wmill` are derived from the source — stale generated files give agents the wrong flags. - **Session recorder**: `frontend/src/lib/components/recording/` is also the recorder `wmill app dev --recording` serves, vendored into the CLI as `cli/src/commands/app/devRecorderBundle.gen.ts`. After changing `rawAppSnapshot.ts` or `rawAppRecording.svelte.ts`, run `bun run gen:dev-recorder` from `cli/` (`cli/test/dev_recorder_bundle_unit.test.ts` fails otherwise). diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000000..6efb92b669 --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,38 @@ +# Windmill + +Open-source platform for internal tools, workflows, API integrations, background jobs and UIs. This file pins the vocabulary that is specific to Windmill's domain, so that code, docs and reviews name the same thing the same way. + +## Language + +### Flows + +**Step**: +One node of a flow — the unit a user selects in the graph and configures in the right-hand panel. Typed as `FlowModule` in code. +_Avoid_: module (ambiguous with the architectural sense), node, action + +**Step setting**: +A per-step runtime option stored on the step itself: retries, error handling, timeout, concurrency limit, priority, cache, debounce, early stop, skip, suspend, sleep, lifetime. Distinct from the step's inputs and its code. The panel that edits them is the **run settings** tab; a single setting is still a step setting. +_Avoid_: advanced setting, step config, flow option + +**Configured**: +Said of a step setting whose config object is present on the step. Deliberately not the same as "would change the runtime's behaviour" — a setting can be configured and still be a no-op (`sleep` of `0`). Every surface that answers "is this setting on?" answers it this way. +_Avoid_: enabled, active, effective + +**Trigger step**: +The first step of a polling flow. It runs on a schedule and returns the items found since its last run; an empty return means there is nothing to process and the flow stops early, marked skipped rather than failed. +_Avoid_: poll script, trigger node, schedule step + +**Default predicate**: +The `stop_after_if` expression seeded onto a trigger step at creation, encoding what "nothing new" looks like. One value, owned in one place, shared by every path that creates a trigger step. + +**Connect**: +Arming an input so that the next property picked fills it. A property can be picked from the prop picker or, when the panel is docked beside the graph, by clicking a step node's output. At most one input is armed per panel, so a pick always has exactly one destination. +_Avoid_: link, bind, plug (the icon is a plug; the action is connecting) + +**Step input**: +One argument of a step, edited in the step's input form. Its prop picker is a pane beside the form, always visible, so previous results can be browsed without connecting. +_Avoid_: argument field, param + +**Expression input**: +Any other place a property can be picked into: the loop iterator, skip and early-stop predicates, the retry condition, a branch predicate, timeout. Its prop picker opens in a popover from the connect button rather than taking a pane. +_Avoid_: JS field, code input diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 0752d0051b..204bafda83 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1671,6 +1671,12 @@ version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "base64" +version = "0.23.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac07cdecf99051d9a5238b80f35af32cdeba5b336e55d957b318b50137e18da5" + [[package]] name = "base64-simd" version = "0.8.0" @@ -2270,6 +2276,12 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "cast" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5" + [[package]] name = "castaway" version = "0.2.4" @@ -6662,9 +6674,9 @@ dependencies = [ [[package]] name = "js-sys" -version = "0.3.80" +version = "0.3.85" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "852f13bec5eba4ba9afbeb93fd7c13fe56147f055939ae21c43a29a0ecb2702e" +checksum = "8c942ebf8e95485ca0d52d97da7c5a2c387d0e7f0ba4c35e93bfcaee045955b3" dependencies = [ "once_cell", "wasm-bindgen", @@ -8286,6 +8298,12 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "oorandom" +version = "11.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d6790f58c7ff633d8771f42965289203411a5e5c68388703c06e14f24770b41e" + [[package]] name = "opaque-debug" version = "0.3.1" @@ -10081,16 +10099,16 @@ dependencies = [ "url", "wasm-bindgen", "wasm-bindgen-futures", - "wasm-streams", + "wasm-streams 0.4.2", "web-sys", "webpki-roots 1.0.9", ] [[package]] name = "reqwest" -version = "0.13.1" +version = "0.13.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "04e9018c9d814e5f30cc16a0f03271aeab3571e609612d9fe78c1aa8d11c2f62" +checksum = "219c5811de6525e5416c7d5d53bb656d3afdbc6c5af816e0802bcfa42dbdc1c3" dependencies = [ "base64 0.22.1", "bytes", @@ -10127,7 +10145,7 @@ dependencies = [ "url", "wasm-bindgen", "wasm-bindgen-futures", - "wasm-streams", + "wasm-streams 0.5.0", "web-sys", ] @@ -10140,7 +10158,7 @@ dependencies = [ "anyhow", "async-trait", "http 1.5.0", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "thiserror 2.0.19", "tower-service", @@ -10158,7 +10176,7 @@ dependencies = [ "getrandom 0.2.17", "http 1.5.0", "hyper 1.11.0", - "reqwest 0.13.1", + "reqwest 0.13.4", "reqwest-middleware", "retry-policies", "thiserror 2.0.19", @@ -10252,13 +10270,12 @@ dependencies = [ [[package]] name = "rmcp" -version = "0.15.0" +version = "3.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1bef41ebc9ebed2c1b1d90203e9d1756091e8a00bbc3107676151f39868ca0ee" +checksum = "ad26b216c966e987e80e86daf784a455c039c43d98575ceed57b8faa259e5695" dependencies = [ "async-trait", - "axum 0.8.9", - "base64 0.22.1", + "base64 0.23.1", "bytes", "chrono", "futures", @@ -10268,8 +10285,8 @@ dependencies = [ "oauth2", "pastey", "pin-project-lite", - "rand 0.9.0", - "reqwest 0.12.28", + "rand 0.10.2", + "reqwest 0.13.4", "rmcp-macros", "schemars 1.2.2", "serde", @@ -10287,9 +10304,9 @@ dependencies = [ [[package]] name = "rmcp-macros" -version = "0.15.0" +version = "3.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0e88ad84b8b6237a934534a62b379a5be6388915663c0cc598ceb9b3292bbbfe" +checksum = "41bc748630c2be2a71b614c2f40d27bc0df0060696d224e1692c72345b7e0b79" dependencies = [ "darling 0.23.0", "proc-macro2", @@ -14220,9 +14237,9 @@ checksum = "b8dad83b4f25e74f184f64c43b150b91efe7647395b42289f38e50566d82855b" [[package]] name = "wasm-bindgen" -version = "0.2.103" +version = "0.2.108" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ab10a69fbd0a177f5f649ad4d8d3305499c42bab9aef2f7ff592d0ec8f833819" +checksum = "64024a30ec1e37399cf85a7ffefebdb72205ca1c972291c51512360d90bd8566" dependencies = [ "cfg-if", "once_cell", @@ -14232,27 +14249,14 @@ dependencies = [ "wasm-bindgen-shared", ] -[[package]] -name = "wasm-bindgen-backend" -version = "0.2.103" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0bb702423545a6007bbc368fde243ba47ca275e549c8a28617f56f6ba53b1d1c" -dependencies = [ - "bumpalo", - "log", - "proc-macro2", - "quote", - "syn 2.0.119", - "wasm-bindgen-shared", -] - [[package]] name = "wasm-bindgen-futures" -version = "0.4.53" +version = "0.4.58" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a0b221ff421256839509adbb55998214a70d829d3a28c69b4a6672e9d2a42f67" +checksum = "70a6e77fd0ae8029c9ea0063f87c46fde723e7d887703d74ad2616d792e51e6f" dependencies = [ "cfg-if", + "futures-util", "js-sys", "once_cell", "wasm-bindgen", @@ -14261,9 +14265,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.103" +version = "0.2.108" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc65f4f411d91494355917b605e1480033152658d71f722a90647f56a70c88a0" +checksum = "008b239d9c740232e71bd39e8ef6429d27097518b6b30bdf9086833bd5b6d608" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -14271,50 +14275,65 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.103" +version = "0.2.108" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ffc003a991398a8ee604a401e194b6b3a39677b3173d6e74495eb51b82e99a32" +checksum = "5256bae2d58f54820e6490f9839c49780dff84c65aeab9e772f15d5f0e913a55" dependencies = [ + "bumpalo", "proc-macro2", "quote", "syn 2.0.119", - "wasm-bindgen-backend", "wasm-bindgen-shared", ] [[package]] name = "wasm-bindgen-shared" -version = "0.2.103" +version = "0.2.108" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "293c37f4efa430ca14db3721dfbe48d8c33308096bd44d80ebaa775ab71ba1cf" +checksum = "1f01b580c9ac74c8d8f0c0e4afb04eeef2acf145458e52c03845ee9cd23e3d12" dependencies = [ "unicode-ident", ] [[package]] name = "wasm-bindgen-test" -version = "0.3.53" +version = "0.3.58" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aee0a0f5343de9221a0d233b04520ed8dc2e6728dce180b1dcd9288ec9d9fa3c" +checksum = "45649196a53b0b7a15101d845d44d2dda7374fc1b5b5e2bbf58b7577ff4b346d" dependencies = [ + "async-trait", + "cast", "js-sys", + "libm", "minicov", + "nu-ansi-term", + "num-traits", + "oorandom", + "serde", + "serde_json", "wasm-bindgen", "wasm-bindgen-futures", "wasm-bindgen-test-macro", + "wasm-bindgen-test-shared", ] [[package]] name = "wasm-bindgen-test-macro" -version = "0.3.53" +version = "0.3.58" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a369369e4360c2884c3168d22bded735c43cccae97bbc147586d4b480edd138d" +checksum = "f579cdd0123ac74b94e1a4a72bd963cf30ebac343f2df347da0b8df24cdebed2" dependencies = [ "proc-macro2", "quote", "syn 2.0.119", ] +[[package]] +name = "wasm-bindgen-test-shared" +version = "0.2.108" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8145dd1593bf0fb137dbfa85b8be79ec560a447298955877804640e40c2d6ea" + [[package]] name = "wasm-streams" version = "0.4.2" @@ -14328,6 +14347,19 @@ dependencies = [ "web-sys", ] +[[package]] +name = "wasm-streams" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d1ec4f6517c9e11ae630e200b2b65d193279042e28edd4a2cda233e46670bbb" +dependencies = [ + "futures-util", + "js-sys", + "wasm-bindgen", + "wasm-bindgen-futures", + "web-sys", +] + [[package]] name = "wasm_dep_analyzer" version = "0.3.0" @@ -14354,9 +14386,9 @@ dependencies = [ [[package]] name = "web-sys" -version = "0.3.80" +version = "0.3.85" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fbe734895e869dc429d78c4b433f8d17d95f8d05317440b4fad5ab2d33e596dc" +checksum = "312e32e551d92129218ea9a2452120f4aabc03529ef03e4d0d82fb2780608598" dependencies = [ "js-sys", "wasm-bindgen", @@ -14522,7 +14554,7 @@ dependencies = [ "prometheus", "rand 0.9.0", "rdkafka", - "reqwest 0.13.1", + "reqwest 0.13.4", "rumqttc", "rustls 0.23.35", "serde", @@ -14599,7 +14631,7 @@ dependencies = [ "http 1.5.0", "lazy_static", "mime_guess", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "serde_json", "sqlx", @@ -14679,7 +14711,7 @@ dependencies = [ "quick_cache", "rand 0.9.0", "regex", - "reqwest 0.13.1", + "reqwest 0.13.4", "rsa", "rust-embed", "rustls 0.23.35", @@ -14819,7 +14851,7 @@ dependencies = [ "jsonwebtoken 8.3.0", "lazy_static", "quick_cache", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "serde_json", "sqlx", @@ -14893,7 +14925,7 @@ dependencies = [ "candle-transformers", "hf-hub", "lazy_static", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "serde_json", "sqlx", @@ -14994,7 +15026,7 @@ dependencies = [ "hmac", "rand 0.9.0", "rdkafka", - "reqwest 0.13.1", + "reqwest 0.13.4", "rmcp", "rumqttc", "serde", @@ -15044,7 +15076,7 @@ version = "1.780.0" dependencies = [ "axum 0.8.9", "flate2", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "serde_json", "sqlx", @@ -15111,7 +15143,7 @@ dependencies = [ "lazy_static", "prometheus", "quick_cache", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "serde_json", "sql-builder", @@ -15346,7 +15378,7 @@ dependencies = [ "quick_cache", "rand 0.9.0", "regex", - "reqwest 0.13.1", + "reqwest 0.13.4", "reqwest-middleware", "reqwest-retry", "rsa", @@ -15488,7 +15520,7 @@ dependencies = [ "futures", "http 1.5.0", "oauth2", - "reqwest 0.12.28", + "reqwest 0.13.4", "rmcp", "serde", "serde_json", @@ -15513,7 +15545,7 @@ dependencies = [ "http 1.5.0", "itertools 0.14.0", "lazy_static", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "serde_json", "sha2 0.10.9", @@ -15575,7 +15607,7 @@ dependencies = [ "lazy_static", "object_store", "quick_cache", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "serde_json", "sqlx", @@ -15898,7 +15930,7 @@ dependencies = [ "quick_cache", "rand 0.9.0", "regex", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "serde_json", "serde_urlencoded", @@ -15940,7 +15972,7 @@ dependencies = [ "lazy_static", "rcgen", "regex", - "reqwest 0.13.1", + "reqwest 0.13.4", "rustls 0.23.35", "serde", "serde_json", @@ -15981,7 +16013,7 @@ dependencies = [ "lazy_static", "magic-crypt", "quick_cache", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "serde_json", "sha2 0.10.9", @@ -16036,7 +16068,7 @@ dependencies = [ "itertools 0.14.0", "lazy_static", "rand 0.9.0", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "serde_json", "sql-builder", @@ -16100,7 +16132,7 @@ dependencies = [ "lazy_static", "quick_cache", "rand 0.9.0", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "serde_json", "sha2 0.10.9", @@ -16153,7 +16185,7 @@ dependencies = [ "jsonwebtoken 8.3.0", "lazy_static", "quick_cache", - "reqwest 0.13.1", + "reqwest 0.13.4", "serde", "serde_json", "sqlx", @@ -16440,7 +16472,7 @@ dependencies = [ "rand 0.9.0", "rcgen", "regex", - "reqwest 0.13.1", + "reqwest 0.13.4", "reqwest-middleware", "rsa", "rust_decimal", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 38e38f5bed..0096cff28b 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -568,7 +568,12 @@ dashmap = "6.1.0" gosyn = "0.2.6" bytes = "1.4.0" gethostname = "0.4.3" -wasm-bindgen = "=0.2.103" +# Not pinned exactly: the excluded `parsers/windmill-parser-wasm` workspace pins +# =0.2.103 to match its vendored `cli/wasm/*` artifacts, yet path-depends on +# sibling parser crates that inherit this requirement from here. Two exact pins +# on the same semver range cannot both resolve, so keep this a range and let each +# workspace's lockfile settle it (here, js-sys forces 0.2.108). +wasm-bindgen = "0.2" serde-wasm-bindgen = "^0" wasm-bindgen-test = "^0" convert_case = "0.6.0" @@ -612,7 +617,7 @@ nkeys = "0.4.4" nu-parser = { version = "0.101.0", default-features = false } globset = "0.4.16" croner = "2.2.0" -rmcp = { version = "=0.15.0", features = ["client", "transport-streamable-http-client", "transport-streamable-http-client-reqwest"] } +rmcp = { version = "=3.1.0", features = ["client", "transport-streamable-http-client", "transport-streamable-http-client-reqwest"] } rquickjs = { version = "0.11", features = ["futures", "parallel", "macro"] } process-wrap = { version = "8.2.1", features = ["tokio1"] } diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index bbe9d84ec8..9857d2eac9 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -e46e1febdc5d464d923f6e74a12d49281faf3f04 +7622c1df38a1f858bd3f893537da0e2cca6c2d54 diff --git a/backend/windmill-api-integration-tests/tests/mcp_protocol_versions.rs b/backend/windmill-api-integration-tests/tests/mcp_protocol_versions.rs new file mode 100644 index 0000000000..e684e3ee14 --- /dev/null +++ b/backend/windmill-api-integration-tests/tests/mcp_protocol_versions.rs @@ -0,0 +1,221 @@ +//! Protocol-version negotiation for the MCP endpoint. +//! +//! The endpoint is dual-era: legacy revisions keep the `initialize` handshake, +//! while `2026-07-28` carries its version as per-request metadata and is served +//! statelessly. Both are answered on the same URL, so a bump of the rmcp SDK +//! must not silently drop either side. +#![cfg(feature = "mcp")] + +use serde_json::{json, Value}; +use sqlx::{Pool, Postgres}; + +use windmill_test_utils::*; + +/// Every revision the server advertises, oldest first. +const SUPPORTED: [&str; 5] = [ + "2024-11-05", + "2025-03-26", + "2025-06-18", + "2025-11-25", + "2026-07-28", +]; + +const MODERN: &str = "2026-07-28"; + +async fn insert_mcp_token(db: &Pool) -> anyhow::Result<()> { + sqlx::query( + "INSERT INTO token (token_hash, token_prefix, token, email, label, super_admin, scopes) + VALUES (encode(sha256('MCP_TOKEN'::bytea), 'hex'), 'MCP_TOK', 'MCP_TOKEN', 'test@windmill.dev', 'mcp token', true, ARRAY['mcp:all'])", + ) + .execute(db) + .await?; + Ok(()) +} + +/// POST one JSON-RPC message and return the HTTP status plus the decoded body. +/// The endpoint answers either `application/json` or a single-event SSE stream, +/// so strip the `data: ` framing before parsing. +async fn post( + port: u16, + headers: &[(&str, &str)], + body: Value, +) -> anyhow::Result<(reqwest::StatusCode, Value)> { + let mut req = reqwest::Client::new() + .post(format!( + "http://localhost:{port}/api/mcp/w/test-workspace/mcp" + )) + .header("Authorization", "Bearer MCP_TOKEN") + .header("Accept", "application/json, text/event-stream") + .json(&body); + for (k, v) in headers { + req = req.header(*k, *v); + } + let resp = req.send().await?; + let status = resp.status(); + let text = resp.text().await?; + let payload = text + .lines() + .find_map(|l| l.strip_prefix("data: ")) + .unwrap_or(text.trim()); + let parsed = serde_json::from_str(payload) + .map_err(|e| anyhow::anyhow!("status {status}, unparseable body {text:?}: {e}"))?; + Ok((status, parsed)) +} + +fn modern_meta() -> Value { + json!({ + "io.modelcontextprotocol/protocolVersion": MODERN, + "io.modelcontextprotocol/clientInfo": { "name": "test-client", "version": "0.0.1" }, + "io.modelcontextprotocol/clientCapabilities": {}, + }) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_mcp_legacy_initialize_negotiates_requested_version( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().await; + insert_mcp_token(&db).await?; + let server = ApiServer::start_mcp(db.clone()).await?; + let port = server.addr.port(); + + // A legacy client must be answered with the revision it asked for, not with + // whatever the SDK happens to call `LATEST`. + for version in SUPPORTED.iter().filter(|v| **v != MODERN) { + let (status, body) = post( + port, + &[("MCP-Protocol-Version", version)], + json!({ + "jsonrpc": "2.0", "id": 1, "method": "initialize", + "params": { + "protocolVersion": version, + "capabilities": {}, + "clientInfo": { "name": "test-client", "version": "0.0.1" }, + } + }), + ) + .await?; + + assert_eq!(status, 200, "initialize {version} failed: {body}"); + assert_eq!( + body["result"]["protocolVersion"], *version, + "initialize {version} negotiated the wrong revision: {body}" + ); + // `Implementation::from_build_env()` expands its `env!` inside rmcp, so + // the obvious constructor makes the server introduce itself as the SDK. + assert_eq!( + body["result"]["serverInfo"]["name"], "windmill", + "server must identify itself, not the SDK: {body}" + ); + } + + Ok(()) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_mcp_modern_requests_are_served_without_initialize( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().await; + insert_mcp_token(&db).await?; + let server = ApiServer::start_mcp(db.clone()).await?; + let port = server.addr.port(); + + // `server/discover` is the modern replacement for the handshake: it must + // exist and advertise exactly the revisions the server implements. + let (status, body) = post( + port, + &[ + ("MCP-Protocol-Version", MODERN), + ("Mcp-Method", "server/discover"), + ], + json!({ + "jsonrpc": "2.0", "id": 1, "method": "server/discover", + "params": { "_meta": modern_meta() } + }), + ) + .await?; + assert_eq!(status, 200, "server/discover failed: {body}"); + assert_eq!( + body["result"]["supportedVersions"], + json!(SUPPORTED), + "server/discover advertised the wrong revisions: {body}" + ); + + // A modern call carries its version in `_meta` and needs no prior session. + let (status, body) = post( + port, + &[ + ("MCP-Protocol-Version", MODERN), + ("Mcp-Method", "tools/list"), + ], + json!({ + "jsonrpc": "2.0", "id": 2, "method": "tools/list", + "params": { "_meta": modern_meta() } + }), + ) + .await?; + assert_eq!(status, 200, "modern tools/list failed: {body}"); + assert!( + body["result"]["tools"] + .as_array() + .is_some_and(|t| !t.is_empty()), + "modern tools/list returned no tools: {body}" + ); + + // SEP-2549 cache hints are required at 2026-07-28 and rmcp omits them unless + // set, which makes strict clients (e.g. the Python SDK) reject the whole + // response rather than degrade. + assert!( + body["result"]["ttlMs"].is_number(), + "modern tools/list is missing ttlMs: {body}" + ); + assert_eq!( + body["result"]["cacheScope"], "private", + "tools/list must not be cached across callers: {body}" + ); + + Ok(()) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_mcp_unsupported_version_lists_supported_ones( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().await; + insert_mcp_token(&db).await?; + let server = ApiServer::start_mcp(db.clone()).await?; + let port = server.addr.port(); + + // The client's only way forward is the `supported` list, so an unknown + // version must fail with it rather than with a generic error. + let (status, body) = post( + port, + &[ + ("MCP-Protocol-Version", "1900-01-01"), + ("Mcp-Method", "tools/list"), + ], + json!({ + "jsonrpc": "2.0", "id": 1, "method": "tools/list", + "params": { "_meta": { + "io.modelcontextprotocol/protocolVersion": "1900-01-01", + "io.modelcontextprotocol/clientInfo": { "name": "test-client", "version": "0.0.1" }, + "io.modelcontextprotocol/clientCapabilities": {}, + }} + }), + ) + .await?; + + assert_eq!(status, 400, "expected 400 for unknown version: {body}"); + assert_eq!(body["error"]["code"], -32022, "wrong error code: {body}"); + assert_eq!( + body["error"]["data"]["supported"], + json!(SUPPORTED), + "error did not advertise the supported revisions: {body}" + ); + + Ok(()) +} diff --git a/backend/windmill-api-integration-tests/tests/resources.rs b/backend/windmill-api-integration-tests/tests/resources.rs index 3cc59be473..07afbb3442 100644 --- a/backend/windmill-api-integration-tests/tests/resources.rs +++ b/backend/windmill-api-integration-tests/tests/resources.rs @@ -756,37 +756,26 @@ async fn test_mcp_client_get_job_and_logs(db: Pool) -> anyhow::Result< .auth_header("MCP_TOKEN"); let transport = StreamableHttpClientTransport::from_config(config); - let client_info = ClientInfo { - protocol_version: Default::default(), - capabilities: ClientCapabilities::default(), - client_info: Implementation { - name: "test-client".to_string(), - title: None, - version: "0.0.1".to_string(), - description: None, - website_url: None, - icons: None, - }, - meta: None, - }; + let client_info = ClientInfo::new( + ClientCapabilities::default(), + Implementation::new("test-client", "0.0.1"), + ); let client: RunningService = client_info.serve(transport).await?; // --- Test getJob --- let result = client - .call_tool(CallToolRequestParams { - name: "getJob".into(), - arguments: Some(serde_json::from_value(json!({ "id": job_id.to_string() }))?), - task: None, - meta: None, - }) + .call_tool( + CallToolRequestParams::new("getJob") + .with_arguments(serde_json::from_value(json!({ "id": job_id.to_string() }))?), + ) .await?; let text = result .content .first() - .and_then(|c| c.raw.as_text()) + .and_then(|c| c.as_text()) .expect("getJob should return text content"); let job: serde_json::Value = serde_json::from_str(&text.text)?; assert_eq!(job["id"], job_id.to_string()); @@ -800,18 +789,16 @@ async fn test_mcp_client_get_job_and_logs(db: Pool) -> anyhow::Result< // --- Test getJobLogs --- let result = client - .call_tool(CallToolRequestParams { - name: "getJobLogs".into(), - arguments: Some(serde_json::from_value(json!({ "id": job_id.to_string() }))?), - task: None, - meta: None, - }) + .call_tool( + CallToolRequestParams::new("getJobLogs") + .with_arguments(serde_json::from_value(json!({ "id": job_id.to_string() }))?), + ) .await?; let text = result .content .first() - .and_then(|c| c.raw.as_text()) + .and_then(|c| c.as_text()) .expect("getJobLogs should return text content"); // The logs endpoint returns text/plain, which gets wrapped as a JSON string by call_endpoint let logs: String = serde_json::from_str(&text.text)?; diff --git a/backend/windmill-api/src/lib.rs b/backend/windmill-api/src/lib.rs index 5a3443b3b9..e227e282a3 100644 --- a/backend/windmill-api/src/lib.rs +++ b/backend/windmill-api/src/lib.rs @@ -425,6 +425,27 @@ pub async fn run_server( .allow_headers([http::header::CONTENT_TYPE, http::header::AUTHORIZATION]) .allow_origin(Any); + // MCP carries protocol state in its own headers: `MCP-Protocol-Version` from + // revision 2025-06-18 onward, plus `Mcp-Method` and `Mcp-Name` at 2026-07-28. + // None of them are CORS-simple, so a browser-based MCP client fails preflight + // unless they are allowed — hence a separate layer rather than widening the + // one every other route shares. (`Mcp-Param-*` is only sent for tool inputs + // annotated with `x-mcp-header`, which no tool here declares.) + let mcp_cors = CorsLayer::new() + .allow_methods([http::Method::GET, http::Method::POST, http::Method::DELETE]) + .allow_headers([ + http::header::CONTENT_TYPE, + http::header::AUTHORIZATION, + http::HeaderName::from_static("mcp-protocol-version"), + http::HeaderName::from_static("mcp-method"), + http::HeaderName::from_static("mcp-name"), + ]) + // The 401 challenge is how a client discovers where to authorize (RFC 9728), + // and it is not a safelisted response header, so without this a browser + // client sees an empty one and has no way to begin the OAuth flow. + .expose_headers([http::header::WWW_AUTHENTICATE]) + .allow_origin(Any); + let sp_extension = Arc::new(saml_oss::build_sp_extension().await?); if server_mode { @@ -821,13 +842,13 @@ pub async fn run_server( // Deprecated, here for backwards compatibility: user should use /mcp/w/{workspace_id}/mcp instead .nest( "/mcp/w/{workspace_id}/sse", - mcp_router.clone().layer(cors.clone()), + mcp_router.clone().layer(mcp_cors.clone()), ) .nest( "/mcp/w/{workspace_id}/mcp", - mcp_router.clone().layer(cors.clone()), + mcp_router.clone().layer(mcp_cors.clone()), ) - .nest("/mcp/gateway", gateway_mcp_router.layer(cors.clone())) + .nest("/mcp/gateway", gateway_mcp_router.layer(mcp_cors.clone())) .nest("/agent_workers", { #[cfg(feature = "agent_worker_server")] { diff --git a/backend/windmill-api/src/mcp/core.rs b/backend/windmill-api/src/mcp/core.rs index 5d78536d86..af7e22fcba 100644 --- a/backend/windmill-api/src/mcp/core.rs +++ b/backend/windmill-api/src/mcp/core.rs @@ -570,12 +570,24 @@ pub async fn setup_mcp_server( let backend = WindmillBackend::new(db, user_db, base_internal_url, auth_cache); let runner = Runner::new(backend); - let service_config = StreamableHttpServerConfig { - sse_keep_alive: Some(Duration::from_secs(15)), - stateful_mode: false, - cancellation_token: cancellation_token.clone(), - sse_retry: Some(Duration::from_secs(15)), - }; + let service_config = StreamableHttpServerConfig::default() + .with_sse_keep_alive(Some(Duration::from_secs(15))) + .with_sse_retry(Some(Duration::from_secs(15))) + .with_cancellation_token(cancellation_token.clone()) + // Sessionless: every request re-resolves auth from its own bearer token, so + // there is no session to bind. This also makes legacy `initialize` clients + // take the same stateless path as 2026-07-28 ones. + .with_legacy_session_mode(false) + // rmcp's Host allowlist defaults to localhost, which guards an unauthenticated + // locally-bound server against DNS rebinding. This endpoint instead sits behind + // Windmill's own authentication, and is reached under whatever hostname the + // instance is served on, so keeping that default would reject every remote MCP + // client while adding nothing. + .disable_allowed_hosts() + // MCP bodies are ordinary API payloads — `createApp`/`updateApp` carry whole app + // sources — so they follow the instance's request size limit rather than rmcp's + // much smaller default, which would 413 them with no way to raise it. + .with_max_request_body_bytes(*crate::REQUEST_SIZE_LIMIT.read().await); let service = StreamableHttpService::new(move || Ok(runner.clone()), session_manager, service_config); diff --git a/backend/windmill-mcp/Cargo.toml b/backend/windmill-mcp/Cargo.toml index 36e0d03d18..8ac23e885d 100644 --- a/backend/windmill-mcp/Cargo.toml +++ b/backend/windmill-mcp/Cargo.toml @@ -17,7 +17,7 @@ auth = ["rmcp/auth", "dep:oauth2", "dep:sqlx", "dep:chrono"] oauth2 = { version = "5.0", optional = true } windmill-common = { workspace = true, default-features = false } anyhow.workspace = true -reqwest = { version = "=0.12", features = ["json", "stream", "gzip"] } +reqwest.workspace = true serde.workspace = true serde_json.workspace = true tracing.workspace = true diff --git a/backend/windmill-mcp/src/client/mod.rs b/backend/windmill-mcp/src/client/mod.rs index f61a1c3f85..863d6b0355 100644 --- a/backend/windmill-mcp/src/client/mod.rs +++ b/backend/windmill-mcp/src/client/mod.rs @@ -85,10 +85,7 @@ impl McpClient { // and does not legitimately rely on redirects. .redirect(reqwest::redirect::Policy::none()); // Pin DNS to the address validated above so the connect cannot rebind to - // an internal IP between the check and the request. `apply_dns_pinning` - // lives on windmill-common's reqwest, but this crate resolves a - // different reqwest version (via rmcp), so pin directly with the - // std-typed host/addrs the validation surfaced. Empty addrs (IP literal + // an internal IP between the check and the request. Empty addrs (IP literal // or ALLOW_PRIVATE_MCP_SERVER_URLS) leave resolution untouched. if !validated.addrs.is_empty() { client_builder = client_builder.resolve_to_addrs(&validated.host, &validated.addrs); @@ -102,19 +99,11 @@ impl McpClient { let transport = StreamableHttpClientTransport::with_client(reqwest_client, config); // Set up client info - let client_info = ClientInfo { - protocol_version: Default::default(), - capabilities: ClientCapabilities::default(), - client_info: Implementation { - name: "windmill-ai-agent".to_string(), - title: Some("Windmill AI Agent".to_string()), - version: env!("CARGO_PKG_VERSION").to_string(), - description: None, - website_url: None, - icons: None, - }, - meta: None, - }; + let client_info = ClientInfo::new( + ClientCapabilities::default(), + Implementation::new("windmill-ai-agent", env!("CARGO_PKG_VERSION")) + .with_title("Windmill AI Agent"), + ); // Initialize the connection let client = client_info @@ -143,14 +132,14 @@ impl McpClient { let mcp_args = Self::openai_args_to_mcp_args(arguments).context("Failed to parse tool arguments")?; + let mut params = CallToolRequestParams::new(name.to_string()); + if let Some(args) = mcp_args { + params = params.with_arguments(args); + } + let result = self .client - .call_tool(CallToolRequestParams { - name: name.to_string().into(), - arguments: mcp_args, - task: None, - meta: None, - }) + .call_tool(params) .await .context(format!("Failed to call MCP tool: {}", name))?; diff --git a/backend/windmill-mcp/src/client_registration.rs b/backend/windmill-mcp/src/client_registration.rs index 637b76df80..3aa39e840b 100644 --- a/backend/windmill-mcp/src/client_registration.rs +++ b/backend/windmill-mcp/src/client_registration.rs @@ -170,7 +170,7 @@ pub async fn get_or_refresh_mcp_client( .map_err(|e| error::Error::BadRequest(format!("Failed to create auth manager: {e}")))?; // Discovery hits the well-known endpoint on the MCP server host validated // above; pin to that address so it cannot rebind between check and connect. - // Limitation: rmcp's discover_metadata may additionally follow server-supplied + // Limitation: rmcp's resolve_metadata may additionally follow server-supplied // metadata URLs (resource_metadata / authorization_servers) on other hosts, // which this per-host pin does not cover — a pre-existing gap in rmcp discovery // that a validating resolver would need to close, out of scope for this pin. @@ -181,8 +181,7 @@ pub async fn get_or_refresh_mcp_client( .with_client(discovery_client) .map_err(|e| error::Error::BadRequest(format!("Failed to configure auth manager: {e}")))?; - let metadata = manager - .discover_metadata() + let metadata = crate::oauth::discover_authorization_metadata(&manager) .await .map_err(|e| error::Error::BadRequest(format!("OAuth discovery failed: {e}")))?; diff --git a/backend/windmill-mcp/src/lib.rs b/backend/windmill-mcp/src/lib.rs index d5eb0b9597..33a20be2f0 100644 --- a/backend/windmill-mcp/src/lib.rs +++ b/backend/windmill-mcp/src/lib.rs @@ -40,10 +40,30 @@ pub mod oauth { use std::time::Duration; - pub use rmcp::transport::auth::AuthorizationManager; + use rmcp::transport::auth::AuthorizationMetadataSource; + pub use rmcp::transport::auth::{AuthorizationManager, AuthorizationMetadata}; const DEFAULT_OAUTH_HTTP_TIMEOUT: Duration = Duration::from_secs(30); + /// Discover the MCP server's OAuth metadata, refusing endpoints the server + /// never advertised. + /// + /// When a server publishes no metadata at all, rmcp's `resolve_metadata` + /// falls back to inventing `/authorize`, `/token` and `/register` on the + /// server's own host. Dynamic client registration and the token exchange + /// both carry secrets, so they must only ever reach endpoints the server + /// actually published — a guessed path would send them somewhere the + /// operator never designated as an authorization server. + pub async fn discover_authorization_metadata( + manager: &AuthorizationManager, + ) -> anyhow::Result { + let resolution = manager.resolve_metadata().await?; + if resolution.source == AuthorizationMetadataSource::LegacyEndpointFallback { + anyhow::bail!("MCP server does not publish OAuth authorization metadata"); + } + Ok(resolution.metadata) + } + pub fn no_redirect_http_client() -> Result { no_redirect_http_client_with_timeout(DEFAULT_OAUTH_HTTP_TIMEOUT) } @@ -61,11 +81,8 @@ pub mod oauth { /// guard validated for the request URL so the connect cannot rebind to an /// internal IP after the check (TOCTOU). The OAuth DCR/discovery/token /// requests target author-controlled URLs and carry secrets, so they must - /// go through this rather than the unpinned client. `apply_dns_pinning` - /// lives on windmill-common's reqwest, which this crate resolves at a - /// different version (via rmcp), so pin directly with the std-typed - /// host/addrs. Empty `addrs` (IP literal or ALLOW_PRIVATE_MCP_SERVER_URLS) - /// leaves resolution untouched. + /// go through this rather than the unpinned client. Empty `addrs` (IP literal + /// or ALLOW_PRIVATE_MCP_SERVER_URLS) leaves resolution untouched. pub fn no_redirect_http_client_pinned( target: &windmill_common::ssrf::ValidatedTarget, ) -> Result { @@ -124,5 +141,41 @@ pub mod oauth { handle.join().unwrap(); } + + /// A server publishing no OAuth metadata must be rejected, not have its + /// endpoints guessed: rmcp's own fallback would invent `/authorize`, + /// `/token` and `/register` on that host, and DCR and the token exchange + /// send secrets to whatever comes back. + #[tokio::test] + async fn discovery_refuses_endpoints_the_server_never_published() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + + let handle = thread::spawn(move || { + // Every discovery probe 404s, which is what a plain MCP server + // with no authorization server looks like. + while let Ok((mut stream, _)) = listener.accept() { + let mut buffer = [0u8; 2048]; + let _ = stream.read(&mut buffer); + let _ = std::io::Write::write_all( + &mut stream, + b"HTTP/1.1 404 Not Found\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", + ); + } + }); + + let manager = AuthorizationManager::new(format!("http://{addr}/mcp")) + .await + .expect("manager should construct"); + let err = discover_authorization_metadata(&manager) + .await + .expect_err("must not fall back to guessed endpoints"); + assert!( + err.to_string().contains("does not publish OAuth"), + "unexpected error: {err}" + ); + + drop(handle); + } } } diff --git a/backend/windmill-mcp/src/server/endpoints.rs b/backend/windmill-mcp/src/server/endpoints.rs index 9e3adffc0b..cea2896caa 100644 --- a/backend/windmill-mcp/src/server/endpoints.rs +++ b/backend/windmill-mcp/src/server/endpoints.rs @@ -59,17 +59,13 @@ pub fn endpoint_tool_to_mcp_tool(tool: &EndpointTool) -> Tool { // Create annotations based on HTTP method and endpoint characteristics let annotations = create_endpoint_annotations(tool); - Tool { - name: tool.name.clone(), - description: Some(description.into()), - input_schema: Arc::new(combined_schema.as_object().unwrap().clone()), - title: Some(tool.name.to_string()), - output_schema: None, - icons: None, - annotations: Some(annotations), - meta: None, - execution: None, - } + Tool::new( + tool.name.clone(), + description, + Arc::new(combined_schema.as_object().unwrap().clone()), + ) + .with_title(tool.name.to_string()) + .with_annotations(annotations) } /// Convert an endpoint tool to an MCP tool for multi-workspace mode. @@ -130,26 +126,19 @@ pub fn list_workspaces_tool() -> Tool { "required": [] }); - Tool { - name: Cow::Borrowed("list_workspaces"), - description: Some( - "List the Windmill workspaces this token can access. Use the returned workspace ids as the `workspace_id` argument of the other tools." - .into(), - ), - input_schema: Arc::new(schema.as_object().unwrap().clone()), - title: Some("List accessible workspaces".to_string()), - output_schema: None, - icons: None, - annotations: Some(ToolAnnotations { - title: Some("List accessible workspaces".to_string()), - read_only_hint: Some(true), - destructive_hint: Some(false), - idempotent_hint: Some(true), - open_world_hint: Some(false), - }), - meta: None, - execution: None, - } + Tool::new( + Cow::Borrowed("list_workspaces"), + "List the Windmill workspaces this token can access. Use the returned workspace ids as the `workspace_id` argument of the other tools.", + Arc::new(schema.as_object().unwrap().clone()), + ) + .with_title("List accessible workspaces") + .with_annotations( + ToolAnnotations::with_title("List accessible workspaces") + .read_only(true) + .destructive(false) + .idempotent(true) + .open_world(false), + ) } /// Create appropriate annotations for endpoint tools based on HTTP method @@ -166,13 +155,11 @@ fn create_endpoint_annotations(tool: &EndpointTool) -> ToolAnnotations { _ => (false, true, false, true), // Default: assume can modify and be destructive }; - ToolAnnotations { - title: Some(format!("{} {}", method, tool.path)), - read_only_hint: Some(read_only), - destructive_hint: Some(destructive), - idempotent_hint: Some(idempotent), - open_world_hint: Some(open_world), - } + ToolAnnotations::with_title(format!("{} {}", method, tool.path)) + .read_only(read_only) + .destructive(destructive) + .idempotent(idempotent) + .open_world(open_world) } /// Merge schema into combined properties and required fields diff --git a/backend/windmill-mcp/src/server/mod.rs b/backend/windmill-mcp/src/server/mod.rs index da7032418b..19c7e43df6 100644 --- a/backend/windmill-mcp/src/server/mod.rs +++ b/backend/windmill-mcp/src/server/mod.rs @@ -23,10 +23,10 @@ pub use tools::create_tool_from_item; // Re-export rmcp types for convenience pub use rmcp::handler::server::ServerHandler; pub use rmcp::model::{ - Annotated, CallToolRequestParams, CallToolResult, Content, Implementation, - InitializeRequestParams, InitializeResult, ListPromptsResult, ListResourceTemplatesResult, - ListResourcesResult, ListToolsResult, PaginatedRequestParams, ProtocolVersion, RawContent, - RawTextContent, ServerCapabilities, ServerInfo, Tool, ToolAnnotations, + CallToolRequestParams, CallToolResult, ContentBlock, Implementation, InitializeRequestParams, + InitializeResult, ListPromptsResult, ListResourceTemplatesResult, ListResourcesResult, + ListToolsResult, PaginatedRequestParams, ProtocolVersion, ServerCapabilities, ServerInfo, Tool, + ToolAnnotations, }; pub use rmcp::service::{RequestContext, RoleServer}; pub use rmcp::transport::streamable_http_server::{ diff --git a/backend/windmill-mcp/src/server/runner.rs b/backend/windmill-mcp/src/server/runner.rs index 0e7c01699b..e8248920ed 100644 --- a/backend/windmill-mcp/src/server/runner.rs +++ b/backend/windmill-mcp/src/server/runner.rs @@ -17,16 +17,42 @@ use crate::server::endpoints::{ use crate::server::tools::create_tool_from_item; use rmcp::handler::server::ServerHandler; use rmcp::model::{ - CallToolRequestParams, CallToolResult, Content, Implementation, InitializeRequestParams, - InitializeResult, ListPromptsResult, ListResourceTemplatesResult, ListResourcesResult, - ListToolsResult, PaginatedRequestParams, ProtocolVersion, ServerCapabilities, ServerInfo, + CacheScope, CallToolRequestParams, CallToolResponse, CallToolResult, ContentBlock, + Implementation, InitializeResult, ListPromptsResult, ListResourceTemplatesResult, + ListResourcesResult, ListToolsResult, PaginatedRequestParams, ProtocolVersion, + ServerCapabilities, ServerInfo, }; use rmcp::service::{RequestContext, RoleServer}; use rmcp::ErrorData; use serde_json::Value; +use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::sync::Arc; +/// Protocol revisions this server is willing to speak. `2026-07-28` is served +/// statelessly with per-request metadata; the older revisions keep the +/// `initialize` handshake, so both eras are answered on the same endpoint. +const SUPPORTED_PROTOCOL_VERSIONS: &[ProtocolVersion] = &[ + ProtocolVersion::V_2024_11_05, + ProtocolVersion::V_2025_03_26, + ProtocolVersion::V_2025_06_18, + ProtocolVersion::V_2025_11_25, + ProtocolVersion::V_2026_07_28, +]; + +/// SEP-2549 cache hints, required on every list result at `2026-07-28` — rmcp +/// leaves them unset, and a strict client rejects the response without them. +/// +/// Zero because nothing here is cacheable: the listing is rebuilt from the +/// workspace's scripts and flows, which change at any time, and this server +/// advertises no `listChanged` capability, so a client that cached a stale list +/// would have no way to learn it had gone stale. +const LIST_TTL_MS: u64 = 0; +/// Every listing is filtered by the caller's token scopes and workspace +/// membership, so no two callers necessarily see the same tools — a shared +/// cache entry would leak one token's view to another. +const LIST_CACHE_SCOPE: CacheScope = CacheScope::Private; + // Re-export from http crate for extracting request parts use http::request::Parts as HttpParts; @@ -307,24 +333,26 @@ fn find_matching_path(candidates: Vec, request_name: &str) - impl ServerHandler for Runner { fn get_info(&self) -> ServerInfo { - ServerInfo { - protocol_version: ProtocolVersion::default(), - capabilities: ServerCapabilities::builder().enable_tools().build(), - server_info: Implementation::from_build_env(), - instructions: Some( + // Not `Implementation::from_build_env()`: its `env!` expands inside rmcp, so it + // would name the SDK crate rather than this server. + let server_info = Implementation::new("windmill", env!("CARGO_PKG_VERSION")) + .with_title("Windmill") + .with_website_url("https://windmill.dev"); + + InitializeResult::new(ServerCapabilities::builder().enable_tools().build()) + .with_server_info(server_info) + .with_instructions( "This server provides a list of scripts and flows the user can run on Windmill. \ - Each flow and script is a tool callable with their respective arguments." - .to_string(), - ), - } + Each flow and script is a tool callable with their respective arguments.", + ) } - async fn initialize( - &self, - _request: InitializeRequestParams, - _context: RequestContext, - ) -> Result { - Ok(self.get_info()) + /// Pinned rather than left to rmcp's default (every version the SDK knows), so a + /// future SDK revision cannot start advertising a version this server has not been + /// exercised against. Bounds `initialize` negotiation, `server/discover`, and + /// per-request version validation alike. + fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> { + Cow::Borrowed(SUPPORTED_PROTOCOL_VERSIONS) } async fn list_tools( @@ -359,7 +387,7 @@ impl ServerHandler for Runner { &self, request: CallToolRequestParams, context: RequestContext, - ) -> Result { + ) -> Result { let (auth, mode) = Self::extract_context(&context)?; // Parse MCP scopes for authorization @@ -370,7 +398,9 @@ impl ServerHandler for Runner { let args = request.arguments.map(Value::Object).unwrap_or(Value::Null); - match mode { + // Every tool here runs to completion in one round trip: none of them ask the + // client for input, so the MRTR variants of `CallToolResponse` are never built. + let result = match mode { McpMode::Single(workspace_id) => { self.call_tool_single( &auth, @@ -386,7 +416,8 @@ impl ServerHandler for Runner { self.call_tool_multi(&auth, &token, &scope_config, read_only, request.name, args) .await } - } + }?; + Ok(result.into()) } async fn list_resources( @@ -394,7 +425,9 @@ impl ServerHandler for Runner { _request: Option, _context: RequestContext, ) -> Result { - Ok(ListResourcesResult { resources: vec![], next_cursor: None, meta: None }) + Ok(ListResourcesResult::with_all_items(vec![]) + .with_ttl_ms(LIST_TTL_MS) + .with_cache_scope(LIST_CACHE_SCOPE)) } async fn list_prompts( @@ -402,7 +435,9 @@ impl ServerHandler for Runner { _request: Option, _context: RequestContext, ) -> Result { - Ok(ListPromptsResult::default()) + Ok(ListPromptsResult::default() + .with_ttl_ms(LIST_TTL_MS) + .with_cache_scope(LIST_CACHE_SCOPE)) } async fn list_resource_templates( @@ -410,7 +445,9 @@ impl ServerHandler for Runner { _request: Option, _context: RequestContext, ) -> Result { - Ok(ListResourceTemplatesResult::default()) + Ok(ListResourceTemplatesResult::default() + .with_ttl_ms(LIST_TTL_MS) + .with_cache_scope(LIST_CACHE_SCOPE)) } } @@ -547,7 +584,9 @@ impl Runner { tools.push(endpoint_tool_to_mcp_tool(&endpoint_tool)); } - Ok(ListToolsResult { tools, next_cursor: None, meta: None }) + Ok(ListToolsResult::with_all_items(tools) + .with_ttl_ms(LIST_TTL_MS) + .with_cache_scope(LIST_CACHE_SCOPE)) } /// Handle a tool call for a single, bound workspace. @@ -575,7 +614,7 @@ impl Runner { .await .map_err(|e| ErrorData::internal_error(e.message, None))?; - return Ok(CallToolResult::success(vec![Content::text( + return Ok(CallToolResult::success(vec![ContentBlock::text( truncate_tool_result( serde_json::to_string_pretty(&result).unwrap_or_else(|_| "{}".to_string()), ), @@ -699,7 +738,7 @@ impl Runner { }; match result { - Ok(value) => Ok(CallToolResult::success(vec![Content::text( + Ok(value) => Ok(CallToolResult::success(vec![ContentBlock::text( truncate_tool_result( serde_json::to_string_pretty(&value).unwrap_or_else(|_| "{}".to_string()), ), @@ -733,7 +772,9 @@ impl Runner { tools.push(endpoint_tool_to_mcp_tool_multi(&endpoint_tool)); } - ListToolsResult { tools, next_cursor: None, meta: None } + ListToolsResult::with_all_items(tools) + .with_ttl_ms(LIST_TTL_MS) + .with_cache_scope(LIST_CACHE_SCOPE) } /// Handle a tool call for a multi-workspace session. `base_auth` is the @@ -754,7 +795,7 @@ impl Runner { .list_accessible_workspaces(base_auth) .await .map_err(|e| ErrorData::internal_error(e.message, None))?; - return Ok(CallToolResult::success(vec![Content::text( + return Ok(CallToolResult::success(vec![ContentBlock::text( serde_json::to_string_pretty(&workspaces).unwrap_or_else(|_| "[]".to_string()), )])); } @@ -827,7 +868,7 @@ impl Runner { .await .map_err(|e| ErrorData::internal_error(e.message, None))?; - Ok(CallToolResult::success(vec![Content::text( + Ok(CallToolResult::success(vec![ContentBlock::text( truncate_tool_result( serde_json::to_string_pretty(&result).unwrap_or_else(|_| "{}".to_string()), ), diff --git a/backend/windmill-mcp/src/server/tools.rs b/backend/windmill-mcp/src/server/tools.rs index 50ed03426b..efdf86ed22 100644 --- a/backend/windmill-mcp/src/server/tools.rs +++ b/backend/windmill-mcp/src/server/tools.rs @@ -190,21 +190,17 @@ pub fn create_tool_from_item( } }; - Tool { - name: Cow::Owned(path), - description: Some(Cow::Owned(description)), - input_schema: Arc::new(input_schema_map), - title: Some(title.clone()), - output_schema: None, - icons: None, - annotations: Some(ToolAnnotations { - title: Some(title), - read_only_hint: Some(false), // Can modify environment - destructive_hint: Some(true), // Can potentially be destructive - idempotent_hint: Some(false), // Are not guaranteed to be idempotent - open_world_hint: Some(true), // Can interact with external services - }), - meta: None, - execution: None, - } + Tool::new( + Cow::Owned(path), + Cow::Owned(description), + Arc::new(input_schema_map), + ) + .with_title(title.clone()) + .with_annotations( + ToolAnnotations::with_title(title) + .read_only(false) // Can modify environment + .destructive(true) // Can potentially be destructive + .idempotent(false) // Are not guaranteed to be idempotent + .open_world(true), // Can interact with external services + ) } diff --git a/backend/windmill-worker/src/prepare_deps.rs b/backend/windmill-worker/src/prepare_deps.rs index 70b5718cf7..2e16e565c2 100644 --- a/backend/windmill-worker/src/prepare_deps.rs +++ b/backend/windmill-worker/src/prepare_deps.rs @@ -12,7 +12,11 @@ use regex::Regex; use serde::{Deserialize, Serialize}; use tokio::process::Command; -use crate::{BUN_CACHE_DIR, BUN_PATH, HOME_ENV, PATH_ENV, PROXY_ENVS, UV_CACHE_DIR}; +use crate::worker::non_empty_env; +use crate::{ + BUN_CACHE_DIR, BUN_PATH, HOME_ENV, INDEX_CERT, NATIVE_CERT, PATH_ENV, PROXY_ENVS, TRUSTED_HOST, + UV_CACHE_DIR, UV_HTTP_TIMEOUT, +}; use windmill_common::worker::write_file; const LOADER_BUILDER_CONTENT: &str = include_str!("../loader_builder.bun.js"); @@ -87,6 +91,15 @@ lazy_static::lazy_static! { /// UV binary path static ref UV_PATH: String = std::env::var("UV_PATH").unwrap_or_else(|_| "/usr/local/bin/uv".to_string()); + + /// This process has no database, so the `pip_index_url` / `pip_extra_index_url` instance + /// settings the job path resolves are unreachable here: their env-var equivalents are the + /// only registry configuration the debugger can see. + static ref PY_INDEX_URL: Option = non_empty_env("PY_INDEX_URL").or_else(|| non_empty_env("PIP_INDEX_URL")); + static ref PY_EXTRA_INDEX_URL: Option = non_empty_env("PY_EXTRA_INDEX_URL").or_else(|| non_empty_env("PIP_EXTRA_INDEX_URL")); + /// uv defaults to `first-index`; the job path overrides it so a package missing from the + /// first index is still resolved from the others. Same default here. + static ref PY_INDEX_STRATEGY: String = non_empty_env("UV_INDEX_STRATEGY").unwrap_or_else(|| "unsafe-best-match".to_string()); } /// Simple loader that doesn't require Windmill API for relative imports @@ -114,6 +127,11 @@ pub struct PrepareResponse { pub job_dir: String, pub success: bool, pub error: Option, + /// Raw stderr of the dependency installer when it exited non-zero, so a caller can show the + /// registry/TLS failure verbatim instead of the bare ModuleNotFoundError that follows. + /// Omitted from the JSON when absent, so callers that only know `success`/`error` are unaffected. + #[serde(skip_serializing_if = "Option::is_none")] + pub install_stderr: Option, } /// Parse Python imports and return a list of package names that need to be installed. @@ -160,6 +178,28 @@ fn get_proc_envs(cache_env: Option<(&str, &str)>) -> HashMap { envs } +/// uv registry arguments, mirroring what the job path passes in `python_executor`. +fn uv_registry_args() -> Vec { + let mut args: Vec = vec![]; + if let Some(urls) = PY_EXTRA_INDEX_URL.as_ref() { + for url in urls.split(',') { + args.extend(["--extra-index-url".to_string(), url.to_string()]); + } + } + if let Some(url) = PY_INDEX_URL.as_ref() { + args.extend(["--index-url".to_string(), url.to_string()]); + } + if let Some(hosts) = TRUSTED_HOST.as_ref() { + for host in hosts.split_whitespace() { + args.extend(["--trusted-host".to_string(), host.to_string()]); + } + } + if *NATIVE_CERT { + args.push("--native-tls".to_string()); + } + args +} + /// Prepare Python dependencies using uv async fn prepare_python_deps_standalone(code: &str) -> PrepareResponse { // Parse imports from the code @@ -172,6 +212,7 @@ async fn prepare_python_deps_standalone(code: &str) -> PrepareResponse { job_dir: String::new(), success: true, error: None, + install_stderr: None, }; } @@ -189,17 +230,37 @@ async fn prepare_python_deps_standalone(code: &str) -> PrepareResponse { job_dir: job_dir.clone(), success: false, error: Some(format!("Failed to create job directory: {}", e)), + install_stderr: None, }; } - let common_uv_envs = get_proc_envs(Some(("UV_CACHE_DIR", &UV_CACHE_DIR))); + let mut common_uv_envs = get_proc_envs(Some(("UV_CACHE_DIR", &UV_CACHE_DIR))); + common_uv_envs.insert( + "UV_INDEX_STRATEGY".to_string(), + PY_INDEX_STRATEGY.to_string(), + ); + if let Some(timeout) = UV_HTTP_TIMEOUT.as_ref() { + common_uv_envs.insert("UV_HTTP_TIMEOUT".to_string(), timeout.to_string()); + } + if let Some(cert_path) = INDEX_CERT.as_ref() { + // uv has no `--cert` on `venv`/`pip install` (astral-sh/uv#6715), so a custom CA bundle + // reaches it through SSL_CERT_FILE, as in the job path. + common_uv_envs.insert("SSL_CERT_FILE".to_string(), cert_path.to_string()); + } + + let registry_args = uv_registry_args(); // Step 1: Create virtual environment using uv + // `--seed` resolves pip/setuptools from the index, so the venv also needs the registry + // arguments: on a network that only reaches a private mirror it fails without them. + let mut venv_args = vec!["venv".to_string(), venv_dir.clone(), "--seed".to_string()]; + venv_args.extend(registry_args.iter().cloned()); + let output = Command::new(UV_PATH.as_str()) .current_dir(&job_dir) .env_clear() .envs(common_uv_envs.clone()) - .args(["venv", &venv_dir, "--seed"]) + .args(&venv_args) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .output() @@ -212,26 +273,33 @@ async fn prepare_python_deps_standalone(code: &str) -> PrepareResponse { job_dir: job_dir.clone(), success: false, error: Some(format!("Failed to create venv: {}", e)), + install_stderr: None, }; } let out = output.unwrap(); if !out.status.success() { - let stderr = String::from_utf8_lossy(&out.stderr); + let stderr = String::from_utf8_lossy(&out.stderr).to_string(); return PrepareResponse { node_modules_path: None, venv_path: None, job_dir: job_dir.clone(), success: false, error: Some(format!("uv venv failed: {}", stderr)), + install_stderr: Some(stderr), }; } // Step 2: Install packages using uv pip install let python_path = format!("{}/bin/python", venv_dir); - let mut args = vec!["pip", "install", "--python", &python_path]; - let package_refs: Vec<&str> = packages.iter().map(|s| s.as_str()).collect(); - args.extend(package_refs.iter()); + let mut args = vec![ + "pip".to_string(), + "install".to_string(), + "--python".to_string(), + python_path, + ]; + args.extend(packages.iter().cloned()); + args.extend(registry_args); let output = Command::new(UV_PATH.as_str()) .current_dir(&job_dir) @@ -246,11 +314,19 @@ async fn prepare_python_deps_standalone(code: &str) -> PrepareResponse { match output { Ok(out) => { if !out.status.success() { - let stderr = String::from_utf8_lossy(&out.stderr); - // Installation might fail for some packages (e.g., wrong package name) - // Log the error but continue - the script might still work if the - // package is actually installed elsewhere or the import is optional - tracing::warn!("uv pip install warning: {}", stderr); + // uv installs the whole set atomically, so a failure here means an empty venv: + // returning success would leave the caller with a bare ModuleNotFoundError and + // no way to see the registry/TLS/package-name error that caused it. + let stderr = String::from_utf8_lossy(&out.stderr).to_string(); + tracing::warn!("uv pip install failed: {}", stderr); + return PrepareResponse { + node_modules_path: None, + venv_path: None, + job_dir: job_dir.clone(), + success: false, + error: Some(format!("uv pip install failed: {}", stderr)), + install_stderr: Some(stderr), + }; } } Err(e) => { @@ -260,6 +336,7 @@ async fn prepare_python_deps_standalone(code: &str) -> PrepareResponse { job_dir: job_dir.clone(), success: false, error: Some(format!("Failed to run uv pip install: {}", e)), + install_stderr: None, }; } } @@ -292,6 +369,7 @@ async fn prepare_python_deps_standalone(code: &str) -> PrepareResponse { job_dir, success: true, error: None, + install_stderr: None, } } @@ -321,6 +399,7 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo "Unsupported language for dependency preparation: {}", language )), + install_stderr: None, }; } } @@ -336,6 +415,7 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo job_dir: job_dir.clone(), success: false, error: Some(format!("Failed to create job directory: {}", e)), + install_stderr: None, }; } @@ -347,6 +427,7 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo job_dir: job_dir.clone(), success: false, error: Some(format!("Failed to write main.ts: {}", e)), + install_stderr: None, }; } @@ -366,6 +447,7 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo job_dir: job_dir.clone(), success: false, error: Some(format!("Failed to write build.js: {}", e)), + install_stderr: None, }; } @@ -398,6 +480,7 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo job_dir: job_dir.clone(), success: false, error: Some(format!("Failed to write empty package.json: {}", e)), + install_stderr: None, }; } } @@ -411,6 +494,7 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo job_dir: job_dir.clone(), success: false, error: Some(format!("Failed to run build.js: {}", e)), + install_stderr: None, }; } } @@ -427,6 +511,7 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo job_dir: job_dir.clone(), success: true, error: None, + install_stderr: None, }; } }; @@ -441,6 +526,7 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo job_dir: job_dir.clone(), success: false, error: Some(format!("Failed to parse package.json: {}", e)), + install_stderr: None, }; } }; @@ -454,6 +540,7 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo job_dir: job_dir.clone(), success: true, error: None, + install_stderr: None, }; } @@ -471,13 +558,14 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo match output { Ok(out) => { if !out.status.success() { - let stderr = String::from_utf8_lossy(&out.stderr); + let stderr = String::from_utf8_lossy(&out.stderr).to_string(); return PrepareResponse { node_modules_path: None, venv_path: None, job_dir: job_dir.clone(), success: false, error: Some(format!("bun install failed: {}", stderr)), + install_stderr: Some(stderr), }; } } @@ -488,6 +576,7 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo job_dir: job_dir.clone(), success: false, error: Some(format!("Failed to run bun install: {}", e)), + install_stderr: None, }; } } @@ -500,6 +589,7 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo job_dir, success: true, error: None, + install_stderr: None, } } else { PrepareResponse { @@ -508,6 +598,7 @@ pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareRespo job_dir, success: true, error: None, + install_stderr: None, } } } @@ -531,6 +622,7 @@ pub async fn run_prepare_deps_cli() -> anyhow::Result<()> { job_dir: String::new(), success: false, error: Some(format!("Failed to read stdin: {}", e)), + install_stderr: None, }; println!("{}", serde_json::to_string(&response)?); return Ok(()); @@ -550,6 +642,7 @@ pub async fn run_prepare_deps_cli() -> anyhow::Result<()> { "Failed to parse JSON input: {}. Expected {{\"code\": \"...\", \"language\": \"bun\" or \"python3\"}}", e )), + install_stderr: None, }; println!("{}", serde_json::to_string(&response)?); return Ok(()); @@ -561,3 +654,43 @@ pub async fn run_prepare_deps_cli() -> anyhow::Result<()> { Ok(()) } + +#[cfg(test)] +mod tests { + use super::PrepareResponse; + + /// The debugger (`debugger/dap_websocket_server.py`) parses this JSON out of the CLI's + /// stdout, so `install_stderr` has to stay additive: a response without an install failure + /// must serialize to the shape callers already know. + #[test] + fn test_install_stderr_is_additive() { + let ok = PrepareResponse { + node_modules_path: None, + venv_path: Some("/tmp/windmill-deps/x/venv".to_string()), + job_dir: "/tmp/windmill-deps/x".to_string(), + success: true, + error: None, + install_stderr: None, + }; + assert_eq!( + serde_json::to_value(&ok).unwrap(), + serde_json::json!({ + "node_modules_path": null, + "venv_path": "/tmp/windmill-deps/x/venv", + "job_dir": "/tmp/windmill-deps/x", + "success": true, + "error": null, + }) + ); + + let failed = PrepareResponse { + install_stderr: Some("error: no such package".to_string()), + success: false, + error: Some("uv pip install failed: error: no such package".to_string()), + ..ok + }; + let failed = serde_json::to_value(&failed).unwrap(); + assert_eq!(failed["install_stderr"], "error: no such package"); + assert_eq!(failed["success"], false); + } +} diff --git a/backend/windmill-worker/src/python_executor.rs b/backend/windmill-worker/src/python_executor.rs index 397367742a..883b035a32 100644 --- a/backend/windmill-worker/src/python_executor.rs +++ b/backend/windmill-worker/src/python_executor.rs @@ -62,20 +62,8 @@ lazy_static::lazy_static! { static ref PY_CONCURRENT_DOWNLOADS: usize = var("PY_CONCURRENT_DOWNLOADS").ok().map(|flag| flag.parse().unwrap_or(20)).unwrap_or(20); - // uv's HTTP request timeout (seconds). spawn_uv_install uses env_clear(), so a - // UV_HTTP_TIMEOUT set on the worker is dropped unless forwarded explicitly. - // Only forwarded when set; otherwise uv keeps its own default. Lets operators - // raise it for slow/contended private registries ("operation timed out"). - static ref UV_HTTP_TIMEOUT: Option = - var("UV_HTTP_TIMEOUT").ok().filter(|v| !v.is_empty()); - - static ref NON_ALPHANUM_CHAR: Regex = regex::Regex::new(r"[^0-9A-Za-z=.-]").unwrap(); - static ref TRUSTED_HOST: Option = var("PY_TRUSTED_HOST").ok().or(var("PIP_TRUSTED_HOST").ok()); - pub static ref INDEX_CERT: Option = var("PY_INDEX_CERT").ok().or(var("PIP_INDEX_CERT").ok()); - pub static ref NATIVE_CERT: bool = var("PY_NATIVE_CERT").ok().or(var("UV_NATIVE_TLS").ok()).map(|flag| flag == "true").unwrap_or(false); - static ref RELATIVE_IMPORT_REGEX: Regex = Regex::new(r#"(import|from)\s(((u|f)\.)|\.)"#).unwrap(); static ref EPHEMERAL_TOKEN_CMD: Option = var("EPHEMERAL_TOKEN_CMD").ok(); @@ -163,9 +151,10 @@ use crate::{ handle_child::handle_child, is_sandboxing_enabled, read_ee_registry_with_workspace_override, worker_utils::ping_job_status, - PyV, DISABLE_NUSER, HOME_ENV, NSJAIL_AVAILABLE, NSJAIL_PATH, NSJAIL_PY_RLIMIT_AS_MB, PATH_ENV, - PIP_EXTRA_INDEX_URL, PIP_INDEX_URL, PROXY_ENVS, PY_INSTALL_DIR, TRACING_PROXY_CA_CERT_PATH, - TZ_ENV, UV_CACHE_DIR, UV_EXCLUDE_NEWER, UV_INDEX_STRATEGY, UV_PYTHON_INSTALL_MIRROR, + PyV, DISABLE_NUSER, HOME_ENV, INDEX_CERT, NATIVE_CERT, NSJAIL_AVAILABLE, NSJAIL_PATH, + NSJAIL_PY_RLIMIT_AS_MB, PATH_ENV, PIP_EXTRA_INDEX_URL, PIP_INDEX_URL, PROXY_ENVS, + PY_INSTALL_DIR, TRACING_PROXY_CA_CERT_PATH, TRUSTED_HOST, TZ_ENV, UV_CACHE_DIR, + UV_EXCLUDE_NEWER, UV_HTTP_TIMEOUT, UV_INDEX_STRATEGY, UV_PYTHON_INSTALL_MIRROR, }; use windmill_common::client::AuthedClient; diff --git a/backend/windmill-worker/src/python_versions.rs b/backend/windmill-worker/src/python_versions.rs index 3a1f35a302..11f673e8f4 100644 --- a/backend/windmill-worker/src/python_versions.rs +++ b/backend/windmill-worker/src/python_versions.rs @@ -23,9 +23,9 @@ use crate::python_executor::UV_PATH; use crate::{ common::{start_child_process, OccupancyMetrics}, handle_child::handle_child, - python_executor::{INDEX_CERT, NATIVE_CERT, PYTHON_PATH}, - HOME_ENV, INSTANCE_PYTHON_VERSION, PATH_ENV, PROXY_ENVS, PY_INSTALL_DIR, UV_CACHE_DIR, - UV_PYTHON_INSTALL_MIRROR, WIN_ENVS, + python_executor::PYTHON_PATH, + HOME_ENV, INDEX_CERT, INSTANCE_PYTHON_VERSION, NATIVE_CERT, PATH_ENV, PROXY_ENVS, + PY_INSTALL_DIR, UV_CACHE_DIR, UV_PYTHON_INSTALL_MIRROR, WIN_ENVS, }; impl From for PyVAlias { diff --git a/backend/windmill-worker/src/worker.rs b/backend/windmill-worker/src/worker.rs index 08bb085059..10e436d326 100644 --- a/backend/windmill-worker/src/worker.rs +++ b/backend/windmill-worker/src/worker.rs @@ -704,6 +704,26 @@ lazy_static::lazy_static! { pub static ref FLOW_RUNNER_RUNNING: Mutex = Mutex::new(false); } +lazy_static::lazy_static! { + /// Registry TLS/timeout settings for uv. Env-only (they have no instance setting), and read + /// both by the job path and by the DB-less `prepare-deps` CLI, which has no other source of + /// registry configuration. + pub static ref TRUSTED_HOST: Option = non_empty_env("PY_TRUSTED_HOST").or_else(|| non_empty_env("PIP_TRUSTED_HOST")); + pub static ref INDEX_CERT: Option = non_empty_env("PY_INDEX_CERT").or_else(|| non_empty_env("PIP_INDEX_CERT")); + pub static ref NATIVE_CERT: bool = non_empty_env("PY_NATIVE_CERT").or_else(|| non_empty_env("UV_NATIVE_TLS")).map(|flag| flag == "true").unwrap_or(false); + /// uv's HTTP request timeout (seconds). The uv invocations use env_clear(), so a + /// UV_HTTP_TIMEOUT set on the worker is dropped unless forwarded explicitly. + /// Only forwarded when set; otherwise uv keeps its own default. Lets operators + /// raise it for slow/contended private registries ("operation timed out"). + pub static ref UV_HTTP_TIMEOUT: Option = non_empty_env("UV_HTTP_TIMEOUT"); +} + +/// A variable declared but left empty (a common shape in compose/k8s manifests) must not +/// shadow the fallback name it is checked against. +pub(crate) fn non_empty_env(key: &str) -> Option { + std::env::var(key).ok().filter(|v| !v.is_empty()) +} + lazy_static::lazy_static! { /// Optional override for the size of the `/tmp` tmpfs mount in nsjail sandboxes (in megabytes). /// When `None` (or non-positive), executors fall back to the unified diff --git a/debugger/README.md b/debugger/README.md index dbeb213351..51c2838f92 100644 --- a/debugger/README.md +++ b/debugger/README.md @@ -75,6 +75,48 @@ Options: | `DAP_NSJAIL_PATH` | nsjail binary path | nsjail | | `DAP_NSJAIL_CONFIG` | nsjail config file path | - | +### Python dependency preparation + +Before debugging a Python script, its imports are installed through `windmill prepare-deps`, which +runs `uv` without a database connection. It cannot read the instance settings, so it takes its +registry configuration from the environment of the debug service instead, and the Python server is +handed the resulting venv with `--venv-path`. The install runs in the service rather than in the +session because a private index URL usually embeds credentials and the Python server executes the +debugged script inside its own interpreter, where anything it holds is readable by that script. + +Set these on the debug service. Where two names are listed the first wins; a worker reads the +`PIP_*` / `PY_*` names in the same way, except for the index URLs, whose worker env fallbacks are +only `PIP_INDEX_URL` / `PIP_EXTRA_INDEX_URL` (the `PY_*` spellings are accepted here for symmetry +with the other settings): + +| Variable | Description | Default | +|----------|-------------|---------| +| `PY_INDEX_URL` / `PIP_INDEX_URL` | Package index (`--index-url`) | PyPI | +| `PY_EXTRA_INDEX_URL` / `PIP_EXTRA_INDEX_URL` | Extra indexes, comma-separated (`--extra-index-url`) | - | +| `PY_TRUSTED_HOST` / `PIP_TRUSTED_HOST` | Hosts to trust, whitespace-separated (`--trusted-host`) | - | +| `PY_INDEX_CERT` / `PIP_INDEX_CERT` | CA bundle for the index, passed to uv as `SSL_CERT_FILE` | - | +| `PY_NATIVE_CERT` / `UV_NATIVE_TLS` | `true` to also trust the platform certificate store (`--native-tls`) | false | +| `UV_INDEX_STRATEGY` | uv index strategy | unsafe-best-match | +| `UV_HTTP_TIMEOUT` | uv HTTP request timeout, in seconds | uv's own default | +| `DAP_PREPARE_DEPS_TIMEOUT_MS` | How long to wait for the install before starting the session without it | 120000 | + +When the install fails, the CLI answers `success: false` and carries the installer's stderr in both +`error` and `install_stderr`; the service reports it to the client as an `output` event, so the +reason (unreachable mirror, untrusted certificate, unknown package) reaches the user instead of a +bare `ModuleNotFoundError` at the first import. + +Proxy variables (`HTTP_PROXY` / `HTTPS_PROXY` / `NO_PROXY`, in either case) are forwarded from the +service into each session, since the debugged script needs them for its own outbound calls, exactly +as a job's script does on a worker. When a proxy is set without a bypass list, `NO_PROXY` defaults +to `localhost,127.0.0.1` so calls to `BASE_INTERNAL_URL` are not proxied. + +Keeping the settings out of the session's environment only bounds what the debugged script can read +from itself. An unsandboxed session runs under the same user as the service and can still read the +service's environment through `/proc`, the same way a job can read a worker's when the worker runs +unsandboxed. Isolating sessions from the service takes `--nsjail --nsjail-config +nsjail.debug.config.proto`: it is that config's PID namespace and `mount_proc` that put the service +out of reach, not the flag on its own. + ### Frontend Integration ```svelte diff --git a/debugger/dap_debug_service.ts b/debugger/dap_debug_service.ts index 2b9ce46272..405cf15fde 100644 --- a/debugger/dap_debug_service.ts +++ b/debugger/dap_debug_service.ts @@ -352,6 +352,45 @@ interface SpawnOptions { stderr?: 'pipe' | 'inherit' } +/** + * Proxy settings forwarded to a debug session, matching what a worker gives a job's script. + * spawnProcess intentionally does not inherit this process's environment, so an outbound proxy + * is unreachable from a session unless these are passed explicitly. Registry settings are + * deliberately absent: they carry credentials and are consumed by the service itself (see + * PythonDebugSession.prepareDependencies). + */ +const SESSION_PROXY_ENV_VARS = [ + 'HTTP_PROXY', + 'HTTPS_PROXY', + 'NO_PROXY', + // The lowercase spellings take precedence in the worker, so forward both. + 'http_proxy', + 'https_proxy', + 'no_proxy' +] + +/** + * How long `windmill prepare-deps` may take before the session gives up on it and starts without + * the dependencies. Raise it for slow private mirrors, where a large install can outlast the default. + */ +const PREPARE_DEPS_TIMEOUT_MS = Number(process.env.DAP_PREPARE_DEPS_TIMEOUT_MS) || 120_000 + +function sessionProxyEnv(): Record { + const env: Record = {} + for (const key of SESSION_PROXY_ENV_VARS) { + const value = process.env[key] + if (value) { + env[key] = value + } + } + // A proxy without a bypass list would send the script's calls to BASE_INTERNAL_URL through it; + // the worker defaults the same way (PROXY_ENVS in windmill-worker). + if (!env.NO_PROXY && !env.no_proxy && (env.HTTP_PROXY || env.http_proxy || env.HTTPS_PROXY || env.https_proxy)) { + env.NO_PROXY = 'localhost,127.0.0.1' + } + return env +} + /** * Spawn a process, optionally wrapped with nsjail. * This is the key function for sandboxed execution. @@ -489,6 +528,15 @@ abstract class BaseDebugSession { // Python Debug Session // ============================================================================ +const DEFAULT_DEBUGPY_TIMEOUT_MS = 10_000 + +// `launch` waits on dependency preparation in the Python server, which allows `windmill +// prepare-deps` up to 120s; anything shorter here reports a timeout while the install is +// still legitimately running. +const DEBUGPY_TIMEOUT_MS_BY_COMMAND: Record = { + launch: 180_000 +} + class PythonDebugSession extends BaseDebugSession { private debugpyWs: WebSocket | null = null private debugpySeq = 1 @@ -502,6 +550,7 @@ class PythonDebugSession extends BaseDebugSession { private scriptResult: unknown = undefined private envVars: Record = {} private windmillPath?: string + private venvPath?: string private debugMode: boolean constructor(ws: { send: (data: string) => void; close: () => void }, windmillPath?: string, debugMode = false) { @@ -531,11 +580,13 @@ class PythonDebugSession extends BaseDebugSession { arguments: args } + const timeoutMs = DEBUGPY_TIMEOUT_MS_BY_COMMAND[command] ?? DEFAULT_DEBUGPY_TIMEOUT_MS + return new Promise((resolve, reject) => { const timeout = setTimeout(() => { this.pendingDebugpyRequests.delete(seq) - reject(new Error(`Debugpy command timeout: ${command}`)) - }, 10000) + reject(new Error(`Debugpy command timeout: ${command} (after ${timeoutMs}ms)`)) + }, timeoutMs) this.pendingDebugpyRequests.set(seq, { resolve: (value) => { @@ -627,6 +678,89 @@ class PythonDebugSession extends BaseDebugSession { } } + /** + * Install the script's imports through `windmill prepare-deps` and return the venv to add to + * the debugged script's sys.path. + * + * This runs here rather than in the Python server because the registry settings the CLI reads + * (`PY_INDEX_URL` and friends) routinely embed private-registry credentials, and the Python + * server executes the submitted script inside its own interpreter: anything in that process is + * recoverable by the script. The service never executes user code, so the credentials stop here. + * + * The trade-off is that the install itself is not jailed, so a source distribution's build + * backend runs outside nsjail, as it already does for Bun sessions. + */ + private async prepareDependencies(code: string): Promise { + if (!this.windmillPath) { + logger.info('No windmill binary path configured, skipping dependency preparation') + return null + } + + const warn = (reason: string): null => { + logger.error(`prepare-deps failed: ${reason}`) + this.sendEvent('output', { + category: 'stderr', + output: `Failed to prepare dependencies: ${reason}\n` + }) + return null + } + + try { + const proc = spawn({ + cmd: [this.windmillPath, 'prepare-deps'], + stdin: new Blob([JSON.stringify({ code, language: 'python3' }) + '\n']), + stdout: 'pipe', + stderr: 'pipe' + }) + + // The launch response is already sent, so an install that never returns would leave the + // client waiting on a session that never starts, with nothing on screen. The deadline + // races the read rather than only killing the child: a grandchild holding the pipe open + // keeps the read pending long after the child itself is gone. + let timer: ReturnType | undefined + const read = (async () => ({ + output: await new Response(proc.stdout).text(), + stderr: await new Response(proc.stderr).text() + }))() + const result = await Promise.race([ + read, + new Promise((resolve) => { + timer = setTimeout(() => resolve(null), PREPARE_DEPS_TIMEOUT_MS) + }) + ]) + clearTimeout(timer) + + if (!result) { + proc.kill() + return warn( + `dependency installation timed out after ${PREPARE_DEPS_TIMEOUT_MS / 1000}s` + ) + } + const { output, stderr } = result + + const lastLine = output.trim().split('\n').pop() || '' + if (!lastLine.startsWith('{')) { + return warn(stderr.trim() || 'windmill binary produced no response') + } + + const response = JSON.parse(lastLine) + if (!response.success) { + // install_stderr is the installer's raw output; `error` already contains it, so + // prefer whichever the CLI version at hand provides. + return warn(response.install_stderr || response.error || 'unknown error') + } + + if (response.venv_path) { + logger.info(`Dependencies installed at: ${response.venv_path}`) + } else { + logger.info('No external dependencies to install') + } + return response.venv_path || null + } catch (error) { + return warn(String(error)) + } + } + private async startPythonProcess(cwd: string): Promise { if (!this.scriptPath) { throw new Error('No script path') @@ -648,10 +782,11 @@ class PythonDebugSession extends BaseDebugSession { '--host', '127.0.0.1' ] - // Pass windmill path for dependency auto-installation if configured - if (this.windmillPath) { - cmd.push('--windmill', this.windmillPath) - logger.info(`Python session: autoinstall enabled with windmill at ${this.windmillPath}`) + // Dependencies are installed by the service (see prepareDependencies), so the server is + // handed the resulting venv instead of the windmill binary it would install with. + if (this.venvPath) { + cmd.push('--venv-path', this.venvPath) + logger.info(`Python session: using dependencies at ${this.venvPath}`) } // Pass debug flag to Python subprocess @@ -662,7 +797,7 @@ class PythonDebugSession extends BaseDebugSession { this.process = spawnProcess({ cmd, cwd, - env: { PYTHONUNBUFFERED: '1', ...this.envVars } + env: { PYTHONUNBUFFERED: '1', ...sessionProxyEnv(), ...this.envVars } }) // Read stderr to capture startup messages @@ -794,6 +929,13 @@ class PythonDebugSession extends BaseDebugSession { this.debugpyWs.onclose = () => { logger.info('Debugpy WebSocket closed') this.debugpyWs = null + // A Python server that dies mid-request must fail it now; otherwise the caller + // waits out the command timeout, which for `launch` is minutes. + const aborted = Array.from(this.pendingDebugpyRequests.values()) + this.pendingDebugpyRequests.clear() + for (const pending of aborted) { + pending.reject(new Error('Debugpy connection closed')) + } } }) } @@ -987,6 +1129,10 @@ sys.stdout.flush() this.sendResponse(request) try { + if (code) { + this.venvPath = (await this.prepareDependencies(code)) ?? undefined + } + await this.startPythonProcess(cwd) // Re-apply breakpoints to the Python server using the actual script path @@ -1012,7 +1158,13 @@ sys.stdout.flush() }) } catch (error) { this.sendEvent('output', { category: 'stderr', output: `Failed to start Python: ${error}\n` }) + // Claim the terminated event before cleanup kills the process, otherwise the + // `exited` handler sends a second one whose empty body erases this error. + this.terminatedSent = true this.sendEvent('terminated', { error: String(error) }) + // A Python server that refused the launch stays in its connection loop, so + // nothing else ever reaps it, its websocket or the temp dir. + await this.cleanup() } } diff --git a/debugger/dap_websocket_server.py b/debugger/dap_websocket_server.py index 624b339894..33937d9617 100644 --- a/debugger/dap_websocket_server.py +++ b/debugger/dap_websocket_server.py @@ -277,12 +277,55 @@ class WindmillDebugger(bdb.Bdb): return {} +PREPARE_DEPS_TIMEOUT_SECONDS = 120 +PREPARE_DEPS_PROGRESS_INTERVAL_SECONDS = 5 + + +@dataclass +class PrepareResult: + """ + Outcome of dependency preparation. + + `error` holds anything worth telling the user, including a problem reported by an + otherwise successful preparation. Only `fatal` means the packages are known to be + missing: failing to reach the CLI at all says nothing about the script's imports and + must not block a session that would otherwise run. + """ + + venv_path: str | None = None + error: str | None = None + fatal: bool = False + + +def _prepare_error_detail(response: dict) -> str: + """ + Build the failure reason from a prepare-deps response. + + `error` is the installer's own output prefixed with the step that failed, and + `install_stderr` is that same output unprefixed, so take one rather than both: joining + them prints the installer's output twice, and the prefix is what tells a reader whether + the venv or the install was what went wrong. + """ + for key in ("error", "install_stderr"): + detail = str(response.get(key) or "").strip() + if detail: + return detail + return "unknown error" + + +def _first_line(detail: str, limit: int = 300) -> str: + """Condense a multi-line failure into the single line a DAP response message allows.""" + line = next((s.strip() for s in detail.splitlines() if s.strip()), detail.strip()) + return line[:limit] + + class DebugSession: """Manages a single debug session.""" - def __init__(self, websocket, windmill_path: str | None = None): + def __init__(self, websocket, windmill_path: str | None = None, prepared_venv_path: str | None = None): self.websocket = websocket self.windmill_path = windmill_path + self._prepared_venv_path = prepared_venv_path self.seq = 1 self.initialized = False self.configured = False @@ -304,14 +347,22 @@ class DebugSession: self.seq += 1 return seq - def prepare_dependencies(self, code: str) -> str | None: + def prepare_dependencies(self, code: str) -> PrepareResult: """ Prepare Python dependencies by calling the windmill CLI. - Returns the path to the venv's site-packages directory, or None if no dependencies needed. + + Blocks for as long as the install takes, so it must run off the event loop; use + `_prepare_dependencies_with_progress` instead of calling this directly. """ + if self._prepared_venv_path: + # The debug service installs dependencies itself so that the registry credentials + # the CLI needs never enter this interpreter, which executes the debugged script. + logger.info(f"Using dependencies prepared by the debug service: {self._prepared_venv_path}") + return PrepareResult(venv_path=self._prepared_venv_path) + if not self.windmill_path: logger.info("No windmill binary path configured, skipping dependency preparation") - return None + return PrepareResult() logger.info(f"Preparing dependencies using {self.windmill_path}") @@ -328,7 +379,7 @@ class DebugSession: input=input_data, capture_output=True, text=True, - timeout=120, # 2 minute timeout for dependency installation + timeout=PREPARE_DEPS_TIMEOUT_SECONDS, ) elapsed = time.time() - start_time @@ -337,7 +388,11 @@ class DebugSession: if result.returncode != 0: logger.error(f"prepare-deps failed (stderr): {result.stderr}") logger.error(f"prepare-deps failed (stdout): {result.stdout}") - return None + detail = (result.stderr or "").strip() or (result.stdout or "").strip() + return PrepareResult( + error=detail or f"windmill prepare-deps exited with code {result.returncode}", + fatal=True, + ) # Log raw output for debugging logger.debug(f"prepare-deps stdout: {result.stdout[:500] if result.stdout else '(empty)'}") @@ -350,15 +405,18 @@ class DebugSession: json_start = output.find('{') if json_start == -1: logger.error(f"No JSON in prepare-deps output: {output}") - return None + return PrepareResult( + error=f"No JSON in prepare-deps output: {output[:500] or '(empty)'}" + ) json_str = output[json_start:] response = json.loads(json_str) logger.debug(f"prepare-deps response: {response}") if not response.get("success"): - logger.error(f"prepare-deps error: {response.get('error')}") - return None + detail = _prepare_error_detail(response) + logger.error(f"prepare-deps error: {detail}") + return PrepareResult(error=detail, fatal=True) venv_path = response.get("venv_path") cached = response.get("cached", False) @@ -371,18 +429,50 @@ class DebugSession: else: logger.info("No external dependencies detected in code") - return venv_path + return PrepareResult(venv_path=venv_path) except subprocess.TimeoutExpired: - logger.error("prepare-deps timed out after 120s") - return None + message = f"prepare-deps timed out after {PREPARE_DEPS_TIMEOUT_SECONDS}s" + logger.error(message) + return PrepareResult(error=message, fatal=True) except json.JSONDecodeError as e: + raw = output[:500] if 'output' in dir() else '(not available)' logger.error(f"Failed to parse prepare-deps JSON output: {e}") - logger.error(f"Raw output was: {output[:500] if 'output' in dir() else '(not available)'}") - return None + logger.error(f"Raw output was: {raw}") + return PrepareResult(error=f"Failed to parse prepare-deps output: {e}\n{raw}") except Exception as e: logger.exception(f"Error preparing dependencies: {e}") - return None + return PrepareResult(error=f"Error preparing dependencies: {e}") + + async def _prepare_dependencies_with_progress(self, code: str) -> PrepareResult: + """ + Run dependency preparation on a worker thread, reporting progress while it runs. + + The install can take minutes on a cold cache; on the event loop it would stall + websocket keepalive until it returns and block the progress events below. + """ + await self.send_event( + "output", {"category": "stdout", "output": "Preparing dependencies...\n"} + ) + + task = asyncio.create_task(asyncio.to_thread(self.prepare_dependencies, code)) + waited = 0 + while True: + done, _ = await asyncio.wait( + {task}, timeout=PREPARE_DEPS_PROGRESS_INTERVAL_SECONDS + ) + if done: + break + waited += PREPARE_DEPS_PROGRESS_INTERVAL_SECONDS + await self.send_event( + "output", + { + "category": "stdout", + "output": f"Still preparing dependencies... ({waited}s)\n", + }, + ) + + return task.result() def _next_var_ref(self) -> int: ref = self._variables_ref_counter @@ -521,7 +611,25 @@ class DebugSession: # Prepare dependencies before modifying the code if code: - self._venv_path = self.prepare_dependencies(code) + prepared = await self._prepare_dependencies_with_progress(code) + if prepared.error: + prefix = ( + "Failed to prepare dependencies" + if prepared.fatal + else "Warning: dependency preparation reported a problem, running anyway" + ) + await self.send_event( + "output", + {"category": "stderr", "output": f"{prefix}:\n{prepared.error}\n"}, + ) + if prepared.fatal: + await self.send_response( + request, + success=False, + message=f"Failed to prepare dependencies: {_first_line(prepared.error)}", + ) + return + self._venv_path = prepared.venv_path # If callMain is True, append a call to main() with the provided args if self._call_main and code: @@ -894,13 +1002,17 @@ class DebugSession: ) -# Module-level variable to store windmill binary path +# Module-level variables to store the windmill binary path and, when the debug service +# already installed the script's dependencies, the venv to use instead of installing here. _windmill_path: str | None = None +_prepared_venv_path: str | None = None async def handle_connection(websocket) -> None: """Handle a WebSocket connection.""" - session = DebugSession(websocket, windmill_path=_windmill_path) + session = DebugSession( + websocket, windmill_path=_windmill_path, prepared_venv_path=_prepared_venv_path + ) logger.info(f"New connection from {websocket.remote_address}") try: @@ -924,13 +1036,21 @@ async def handle_connection(websocket) -> None: session._cleanup_temp_file() -async def main(host: str = "localhost", port: int = 5679, windmill_path: str | None = None) -> None: +async def main( + host: str = "localhost", + port: int = 5679, + windmill_path: str | None = None, + prepared_venv_path: str | None = None, +) -> None: """Start the DAP WebSocket server.""" - global _windmill_path + global _windmill_path, _prepared_venv_path _windmill_path = windmill_path + _prepared_venv_path = prepared_venv_path if windmill_path: logger.info(f"Windmill binary path: {windmill_path}") + if prepared_venv_path: + logger.info(f"Dependencies prepared by the debug service: {prepared_venv_path}") logger.info(f"Starting DAP WebSocket server on ws://{host}:{port}") async with serve(handle_connection, host, port): @@ -944,6 +1064,7 @@ if __name__ == "__main__": parser.add_argument("--host", default="localhost", help="Host to bind to") parser.add_argument("--port", type=int, default=5679, help="Port to listen on") parser.add_argument("--windmill", help="Path to windmill binary for dependency preparation (or set WINDMILL_PATH env var)") + parser.add_argument("--venv-path", help="Site-packages directory of a venv the caller already prepared; skips dependency installation") parser.add_argument("--debug", action="store_true", help="Enable debug logging") args = parser.parse_args() @@ -957,6 +1078,6 @@ if __name__ == "__main__": windmill_path = args.windmill or os.environ.get("WINDMILL_PATH") try: - asyncio.run(main(args.host, args.port, windmill_path)) + asyncio.run(main(args.host, args.port, windmill_path, args.venv_path)) except KeyboardInterrupt: logger.info("Server stopped") diff --git a/debugger/dap_websocket_server_bun.ts b/debugger/dap_websocket_server_bun.ts index d6ed8967ad..ff060f3da6 100644 --- a/debugger/dap_websocket_server_bun.ts +++ b/debugger/dap_websocket_server_bun.ts @@ -222,6 +222,8 @@ function generateMainCallArgs(code: string, args: Record): stri const WINDMILL_BASE_URL = process.env.WINDMILL_BASE_URL || process.env.BASE_INTERNAL_URL // e.g., http://localhost:8000 const REQUIRE_SIGNED_REQUESTS = process.env.REQUIRE_SIGNED_DEBUG_REQUESTS !== 'false' +const PREPARE_DEPS_TIMEOUT_MS = 120_000 + // Opt-in cross-origin protection (CSWSH defense-in-depth); see // dap_debug_service.ts for the rationale. Only enforced for this file's // standalone Bun.serve entrypoint (the windmill-extra runtime imports the @@ -1579,6 +1581,20 @@ export class DebugSession { logger.info(`Preparing dependencies using ${this.windmillPath}`) + // The launch response is only sent once this returns, so without progress a cold + // cache looks like a frozen debugger for as long as the install takes. + this.sendEvent('output', { category: 'console', output: 'Preparing dependencies...\n' }) + let waited = 0 + const progress = setInterval(() => { + waited += 5 + this.sendEvent('output', { + category: 'console', + output: `Still preparing dependencies... (${waited}s)\n` + }) + }, 5000) + let killTimer: ReturnType | undefined + let timedOut = false + try { const input = JSON.stringify({ code, language }) + '\n' logger.info(`prepare-deps input length: ${input.length}`) @@ -1591,9 +1607,27 @@ export class DebugSession { stderr: 'pipe' }) + // Bound the wait: the only other ceiling is the DAP client's launch timeout, + // which is minutes, so a wedged installer would hang the session that long. + killTimer = setTimeout(() => { + timedOut = true + logger.error(`prepare-deps timed out after ${PREPARE_DEPS_TIMEOUT_MS}ms`) + proc.kill() + }, PREPARE_DEPS_TIMEOUT_MS) + // Wait for completion const output = await new Response(proc.stdout).text() const stderr = await new Response(proc.stderr).text() + + if (timedOut) { + const errorMsg = `prepare-deps timed out after ${PREPARE_DEPS_TIMEOUT_MS / 1000}s` + this.sendEvent('output', { + category: 'console', + output: `Warning: Failed to prepare dependencies: ${errorMsg}\n` + }) + return null + } + logger.info(`prepare-deps output: ${output.substring(0, 200)}`) logger.info(`prepare-deps stderr: ${stderr.substring(0, 200)}`) @@ -1648,6 +1682,9 @@ export class DebugSession { output: `Warning: Failed to prepare dependencies: ${error}\n` }) return null + } finally { + clearInterval(progress) + clearTimeout(killTimer) } } diff --git a/debugger/test_dap_server.py b/debugger/test_dap_server.py index 45e138f2a3..17430ea25f 100644 --- a/debugger/test_dap_server.py +++ b/debugger/test_dap_server.py @@ -45,6 +45,10 @@ def main(x: str, count: int = 1): # Breakpoints for the main() test: lines 3 and 4 (inside main function) MAIN_BREAKPOINT_LINES = [3, 4] +# `launch` waits on dependency installation, so the import test below needs far more than +# the default budget on a cold cache. +REQUEST_TIMEOUTS = {"launch": 180.0} + class DAPTestClient: def __init__(self, url: str = "ws://localhost:5679"): @@ -103,7 +107,7 @@ class DAPTestClient: # Wait for response with timeout try: - response = await asyncio.wait_for(future, timeout=10.0) + response = await asyncio.wait_for(future, timeout=REQUEST_TIMEOUTS.get(command, 10.0)) return response except asyncio.TimeoutError: print(f"Timeout waiting for response to {command}") diff --git a/frontend/src/lib/components/ArgInput.svelte b/frontend/src/lib/components/ArgInput.svelte index de5517add3..5ee7a5f429 100644 --- a/frontend/src/lib/components/ArgInput.svelte +++ b/frontend/src/lib/components/ArgInput.svelte @@ -719,7 +719,8 @@ onkeydown: () => (ignoreValueUndefined = true), placeholder: placeholder ?? defaultValue ?? '', min: extra['min'], - max: extra['max'] + max: extra['max'], + step: extra['step'] }} {error} bind:value diff --git a/frontend/src/lib/components/CompareWorkspaces.svelte b/frontend/src/lib/components/CompareWorkspaces.svelte index c40e2585f3..2b644e900d 100644 --- a/frontend/src/lib/components/CompareWorkspaces.svelte +++ b/frontend/src/lib/components/CompareWorkspaces.svelte @@ -118,6 +118,10 @@ /** False while the (async) chatMask is still loading. The select-all default * waits for this so it doesn't race the mask. Defaults to true. */ chatMaskReady?: boolean + /** Whether the mask also scopes the update direction (parent→fork). True only + * for an explicit `?items=` deep link, which can legitimately name items to + * pull in — a session's mask never can (see the preselect rule below). */ + maskAppliesToUpdate?: boolean /** Selecting `draft` asks the page to swap us out for CompareDrafts; * deploy_to/update are handled internally but reported so the page can * remember the direction. */ @@ -146,6 +150,7 @@ draftKeys = new Set(), chatMask, chatMaskReady = true, + maskAppliesToUpdate = false, onModeSelected, onChanged }: Props = $props() @@ -901,8 +906,10 @@ // Items with a pending draft are also left out by default: the deployed // version (not the draft) is what moves, so we make the user opt in. // The update direction (parent→fork) is never something the chat caused, so - // when scoped to a chat's items (chatMask set) preselect nothing there. - if (chatMask && !mergeIntoParent) { + // when scoped to a chat's items (chatMask set) preselect nothing there — + // unless the mask came in as an explicit `?items=` deep link, which names + // what to pull. + if (chatMask && !mergeIntoParent && !maskAppliesToUpdate) { selectedItems = [] return } diff --git a/frontend/src/lib/components/EditableSchemaForm.svelte b/frontend/src/lib/components/EditableSchemaForm.svelte index 5813926abd..5ceef0fbf0 100644 --- a/frontend/src/lib/components/EditableSchemaForm.svelte +++ b/frontend/src/lib/components/EditableSchemaForm.svelte @@ -932,7 +932,10 @@ {/if} diff --git a/frontend/src/lib/components/copilot/chat/ContextTextarea.svelte b/frontend/src/lib/components/copilot/chat/ContextTextarea.svelte index f9d5dbf777..2502cd075b 100644 --- a/frontend/src/lib/components/copilot/chat/ContextTextarea.svelte +++ b/frontend/src/lib/components/copilot/chat/ContextTextarea.svelte @@ -889,7 +889,7 @@ white-space: pre-wrap; word-break: break-words; width: 100%; - min-height: 2.25rem; + min-height: 1.9rem; } /* Hide the textarea's own glyphs (the highlight overlay renders the text) diff --git a/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte b/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte index a1ee9984e5..754db1aac0 100644 --- a/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte @@ -1,12 +1,11 @@
@@ -94,9 +102,9 @@ {color} {size} disabled={!$enterpriseLicense} - checked={Boolean(debounce_delay_s)} + checked={debounce_delay_s !== undefined} on:change={() => { - if (debounce_delay_s) { + if (debounce_delay_s !== undefined) { debounce_delay_s = undefined debounce_key = undefined } else { @@ -113,10 +121,10 @@ />
- {#if debounce_delay_s} -
+ {#if !off} +
{/if} diff --git a/frontend/src/lib/components/flows/FlowEditor.svelte b/frontend/src/lib/components/flows/FlowEditor.svelte index a484675f83..e20f3b856b 100644 --- a/frontend/src/lib/components/flows/FlowEditor.svelte +++ b/frontend/src/lib/components/flows/FlowEditor.svelte @@ -1,12 +1,17 @@ +{#snippet panelBody()} + +{/snippet} +
panelController.measure(w)} id="flow-editor" - class={'h-full overflow-hidden transition-colors duration-[400ms] ease-linear border-t'} + class={'relative h-full overflow-hidden transition-colors duration-[400ms] ease-linear border-t'} use:triggerableByAI={{ id: 'flow-editor', description: 'Component to edit a flow' }} > - + +
{#if graphOverlay} @@ -226,36 +384,96 @@ {/if}
- - {#if loading} -
-
- + {#if panelMode === 'docked'} + + + {#if loading} +
+
+ +
-
- {:else} - - {/if} - + {:else if modalPanel} +
+ + {#if detachClaims === 0} +
+ +
+ {/if} +
+ {@render panelBody()} +
+
+ {:else} + {@render panelBody()} + {/if} + + {/if} {#if !disableAi} {/if} + + {#if showStepHint} +
+ + {stepHintText} +
+ {/if}
+ + + + + {#snippet children({ zIndex })} + {#if panelMode === 'modal' && panelModalOpen} + + + + + + {/if} + {/snippet} + diff --git a/frontend/src/lib/components/flows/branchOps.test.ts b/frontend/src/lib/components/flows/branchOps.test.ts new file mode 100644 index 0000000000..844477c968 --- /dev/null +++ b/frontend/src/lib/components/flows/branchOps.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect } from 'vitest' +import { writable, get } from 'svelte/store' +import type { FlowModule } from '$lib/gen' +import type { StateStore } from '$lib/utils' +import type { ExtendedOpenFlow } from './types' +import { reorderBranches } from './branchOps' + +function branch(expr: string) { + return { summary: expr, expr, modules: [] as FlowModule[] } +} + +function ctx(branches: ReturnType[]) { + const flow = { + summary: '', + value: { + modules: [{ id: 'a', value: { type: 'branchone', branches, default: [] } } as FlowModule] + } + } as ExtendedOpenFlow + const flowStore: StateStore = { val: flow } + const history = writable({ history: [] as ExtendedOpenFlow[], index: -1 }) + const read = () => + (flowStore.val.value.modules[0].value as { branches: ReturnType[] }).branches + return { flowStore, history, read } +} + +describe('reorderBranches', () => { + it('commits the new order', () => { + const [x, y, z] = [branch('x'), branch('y'), branch('z')] + const { flowStore, history, read } = ctx([x, y, z]) + + reorderBranches('a', [z, x, y], { flowStore, history }) + + expect(read().map((b) => b.expr)).toEqual(['z', 'x', 'y']) + }) + + it('records an undo entry, so a drag can be undone like an add or a delete', () => { + const [x, y] = [branch('x'), branch('y')] + const { flowStore, history } = ctx([x, y]) + + reorderBranches('a', [y, x], { flowStore, history }) + + expect(get(history).history).toHaveLength(1) + }) + + it('spends nothing when the drag lands where it started', () => { + // `finalize` fires on every drop, so a no-op drag must not consume an undo step. + const [x, y] = [branch('x'), branch('y')] + const { flowStore, history, read } = ctx([x, y]) + + reorderBranches('a', [x, y], { flowStore, history }) + + expect(get(history).history).toHaveLength(0) + expect(read().map((b) => b.expr)).toEqual(['x', 'y']) + }) +}) diff --git a/frontend/src/lib/components/flows/branchOps.ts b/frontend/src/lib/components/flows/branchOps.ts new file mode 100644 index 0000000000..f42026bccb --- /dev/null +++ b/frontend/src/lib/components/flows/branchOps.ts @@ -0,0 +1,82 @@ +import type { FlowModule } from '$lib/gen' +import type { ExtendedOpenFlow } from './types' +import type { FlowState } from './flowState' +import type { StateStore } from '$lib/utils' +import type { History } from '$lib/history.svelte' +import { push } from '$lib/history.svelte' +import { dfs } from './dfs' +import { findModuleInFlow } from './flowTree' + +type BranchList = Array<{ summary?: string; expr?: string; modules: FlowModule[] }> + +type Ctx = { + flowStore: StateStore + flowStateStore: StateStore + history: History +} + +/** Append an empty branch to a branchone/branchall step. */ +export function addBranch(moduleId: string, { flowStore, history }: Omit) { + push(history, flowStore.val) + const module = findModuleInFlow(flowStore.val.value, moduleId) + if (!module) throw new Error(`Node ${moduleId} not found`) + + if (module.value.type === 'branchone' || module.value.type === 'branchall') { + module.value.branches.push({ summary: '', expr: 'false', modules: [] }) + } +} + +/** + * Drop a branch and the flow state of every step inside it. + * + * `index` counts the way the graph lays the branches out, where a branchone's default + * occupies slot 0 — one ahead of the same branch's position in `value.branches`. Callers + * working from the array (the settings panel) must add that offset back. + */ +export function removeBranch( + moduleId: string, + index: number, + { flowStore, flowStateStore, history }: Ctx +) { + push(history, flowStore.val) + const module = findModuleInFlow(flowStore.val.value, moduleId) + if (!module) throw new Error(`Node ${moduleId} not found`) + + if (module.value.type === 'branchone' || module.value.type === 'branchall') { + const offset = module.value.type === 'branchone' ? 1 : 0 + const at = index - offset + + if (module.value.branches[at]?.modules) { + const leaves = dfs(module.value.branches[at].modules, (mod) => mod.id) + leaves.forEach((leafId: string) => delete flowStateStore.val[leafId]) + } + + module.value.branches.splice(at, 1) + } +} + +/** + * Commit a reordered branch list. Undoable like add/remove — a drag is a structural edit, + * and for a branchone it changes which predicate is evaluated first. + */ +export function reorderBranches( + moduleId: string, + ordered: BranchList, + { flowStore, history }: Omit +) { + const module = findModuleInFlow(flowStore.val.value, moduleId) + if (!module) throw new Error(`Node ${moduleId} not found`) + if (module.value.type !== 'branchone' && module.value.type !== 'branchall') return + + const current = module.value.branches + // A drag that lands where it started must not spend an undo entry. + if (ordered.length === current.length && ordered.every((b, i) => b === current[i])) return + + push(history, flowStore.val) + module.value.branches = ordered as typeof current +} + +/** Slot a branch occupies in the graph's numbering, from its index in `value.branches`. */ +export function graphBranchIndex(type: 'branchone' | 'branchall', arrayIndex: number): number { + return type === 'branchone' ? arrayIndex + 1 : arrayIndex +} diff --git a/frontend/src/lib/components/flows/common/FlowCard.svelte b/frontend/src/lib/components/flows/common/FlowCard.svelte index aadb9d459d..091d3a9feb 100644 --- a/frontend/src/lib/components/flows/common/FlowCard.svelte +++ b/frontend/src/lib/components/flows/common/FlowCard.svelte @@ -6,6 +6,8 @@ title?: string | undefined summary?: string | undefined description?: string | undefined + subtitle?: string | undefined + subtitleDocLink?: string | undefined noEditor: boolean noHeader?: boolean flowModuleValue?: FlowModuleValue | undefined @@ -20,6 +22,8 @@ title = undefined, summary = $bindable(undefined), description = $bindable(undefined), + subtitle = undefined, + subtitleDocLink = undefined, noEditor, noHeader = false, flowModuleValue = undefined, @@ -37,9 +41,12 @@ - let cachedValues: Record< - string, - { - latestHash: string | undefined - } - > = {} - - @@ -101,8 +186,8 @@ class="overflow-x-auto scrollbar-hidden flex items-center justify-between flex-nowrap w-full" > {#if flowModuleValue} - -
+ +
{#if flowModuleValue.type === 'identity'} Identity (input copied to output) {:else if flowModuleValue.type === 'rawscript'} @@ -121,75 +206,16 @@ {siblingToolNames} /> {:else if flowModuleValue.type === 'script' && 'path' in flowModuleValue && flowModuleValue.path} - - - {#if hubVersionId} - + + {#if scriptItems.length > 0} + {/if} - {#if flowModuleValue.hash} - {#if latestHash != flowModuleValue.hash} - - {/if} - - {:else if latestHash} -
- -
- {/if} -
+
{/if} {#if title} -
{title}
+ +
{title}
{/if} {@render children?.()} {@render action?.()} +
+ {#if subtitle} +

+ {subtitle} + {#if subtitleDocLink} + Docs + {/if} +

+ {/if} {#if isAgentTool} {#if toolNameError}

{toolNameError}

diff --git a/frontend/src/lib/components/flows/common/FlowPanelChrome.svelte b/frontend/src/lib/components/flows/common/FlowPanelChrome.svelte new file mode 100644 index 0000000000..0a3d822fd1 --- /dev/null +++ b/frontend/src/lib/components/flows/common/FlowPanelChrome.svelte @@ -0,0 +1,29 @@ + + +
+ +
+{#if panelDetach?.modalOpen()} + -
-
-{/if} + {/snippet} + + diff --git a/frontend/src/lib/components/flows/content/DynamicInputHelpBox.svelte b/frontend/src/lib/components/flows/content/DynamicInputHelpBox.svelte index 93bfd2d533..8ed18d73bf 100644 --- a/frontend/src/lib/components/flows/content/DynamicInputHelpBox.svelte +++ b/frontend/src/lib/components/flows/content/DynamicInputHelpBox.svelte @@ -3,11 +3,12 @@ import Button from '$lib/components/common/button/Button.svelte' import { ChevronDown } from 'lucide-svelte' - let opened = $state(false); - + let opened = $state(false) -
+ +
diff --git a/frontend/src/lib/components/flows/content/ExpandedSubflowStep.svelte b/frontend/src/lib/components/flows/content/ExpandedSubflowStep.svelte index 59c2e26297..4aede5e3d1 100644 --- a/frontend/src/lib/components/flows/content/ExpandedSubflowStep.svelte +++ b/frontend/src/lib/components/flows/content/ExpandedSubflowStep.svelte @@ -15,6 +15,7 @@ } from '../expandedSubflowStep' import { parseExpandedSubflowId } from '$lib/components/restartFromStepPath' import { base } from '$app/paths' + import FlowPanelChrome from '../common/FlowPanelChrome.svelte' interface Props { /** Graph node id of the selected step, of the form `subflow:[:...]:`. */ @@ -100,9 +101,9 @@ {/each} {/if}
- {#if resolved} - {@const { containingFlowPath, module } = resolved} -
+
+ {#if resolved} + {@const { containingFlowPath, module } = resolved} {#if $flowEditorDrawer}
- {/if} + {/if} + +
{#if loaded == undefined} diff --git a/frontend/src/lib/components/flows/content/FlowBranchAllWrapper.svelte b/frontend/src/lib/components/flows/content/FlowBranchAllWrapper.svelte index 17f5767c46..da06034a8d 100644 --- a/frontend/src/lib/components/flows/content/FlowBranchAllWrapper.svelte +++ b/frontend/src/lib/components/flows/content/FlowBranchAllWrapper.svelte @@ -12,7 +12,7 @@ } } - let { noEditor, branch = $bindable() }: Props = $props() + let { noEditor, branch }: Props = $props()
@@ -23,13 +23,8 @@
{/snippet}
-
Skip failures
- +
Skip failures
+
diff --git a/frontend/src/lib/components/flows/content/FlowBranchOneWrapper.svelte b/frontend/src/lib/components/flows/content/FlowBranchOneWrapper.svelte index fd7bbc82e8..f118e0859d 100644 --- a/frontend/src/lib/components/flows/content/FlowBranchOneWrapper.svelte +++ b/frontend/src/lib/components/flows/content/FlowBranchOneWrapper.svelte @@ -16,13 +16,7 @@ enableAi?: boolean } - let { - branch = $bindable(), - parentModule, - previousModule, - noEditor, - enableAi = false - }: Props = $props() + let { branch, parentModule, previousModule, noEditor, enableAi = false }: Props = $props()
@@ -32,19 +26,8 @@
{/snippet} -
-

Predicate expression

- { - if (!branch.summary) { - branch.summary = e.detail - } - }} - /> +
+
diff --git a/frontend/src/lib/components/flows/content/FlowBranchesAllWrapper.svelte b/frontend/src/lib/components/flows/content/FlowBranchesAllWrapper.svelte index 8fa98b33d1..8374962083 100644 --- a/frontend/src/lib/components/flows/content/FlowBranchesAllWrapper.svelte +++ b/frontend/src/lib/components/flows/content/FlowBranchesAllWrapper.svelte @@ -1,20 +1,26 @@
- - - - - {#if !noEditor} - - The result of this step is the list of the result of each branch. - - {/if} -
-

{value.branches.length} branch{value.branches.length > 1 ? 'es' : ''}

-
- {#each value.branches as branch, i} -
-
- Branch {i + 1} - -
-
- +
+ + + + {#snippet extra()} + + {/snippet} + + + +
+ {#if selectedTab === 'branches'} +
+
+
+ {#each items as item, i (item.id)} + +
+ +
+ +
+
+ Branch {i + 1} + item.branch.summary ?? '', (v) => (item.branch.summary = String(v)) + } + inputProps={{ placeholder: 'Summary' }} + /> +
+
-
- {/each} + {/each} +
+
-

Add branches and steps directly on the graph.

-
Run in parallel
- -
- - {#if flowModule} - - - - - - - - - {#snippet content()} -
- -
- -
- - -
- -
- - -
- -
- - -
- -
- - -
- -
- - -
- -
- -
- {/snippet} - - +
+ + +
+ + {:else} + {/if} - - +
+
diff --git a/frontend/src/lib/components/flows/content/FlowBranchesOneWrapper.svelte b/frontend/src/lib/components/flows/content/FlowBranchesOneWrapper.svelte index aa6d1af97c..25b843268a 100644 --- a/frontend/src/lib/components/flows/content/FlowBranchesOneWrapper.svelte +++ b/frontend/src/lib/components/flows/content/FlowBranchesOneWrapper.svelte @@ -1,24 +1,29 @@
- - - - - {#if !noEditor} - - The result of this step is the result of the branch. - - {/if} -
-

- {value.branches.length + 1} branch{value.branches.length + 1 > 1 ? 'es' : ''} -

-
-
- Default branch -

If none of the predicates' expressions evaluated in-order match, this branch is - chosen

-
- {#each value.branches as branch, i} -
-
- Branch {i + 1} - -
-
- { - if (!branch.summary) { - branch.summary = e.detail + +
+ + + + {#snippet extra()} + + {/snippet} + + + +
+ {#if selectedTab === 'branches'} +
+
+
+ {#each items as item, i (item.id)} + +
+ +
+ +
+
+ Branch {i + 1} + item.branch.summary ?? '', (v) => (item.branch.summary = String(v)) } - }} - parentModule={flowModule} - {previousModule} - {enableAi} + inputProps={{ placeholder: 'Summary' }} + /> +
+
-
- {/each} + {/each} +
+
+ Default +

Runs if none of the above match

+
-

Add branches and steps directly on the graph.

-
- - {#if flowModule} - - - - - - - - - {#snippet content()} -
- -
- -
- - -
- -
- - -
- -
- - -
- -
- - -
- -
- - -
- -
- -
- {/snippet} - - + + + {:else} + {/if} - - +
+
diff --git a/frontend/src/lib/components/flows/content/FlowEditorPanel.svelte b/frontend/src/lib/components/flows/content/FlowEditorPanel.svelte index 768de22ec6..81600e2dcc 100644 --- a/frontend/src/lib/components/flows/content/FlowEditorPanel.svelte +++ b/frontend/src/lib/components/flows/content/FlowEditorPanel.svelte @@ -2,6 +2,7 @@ import { getContext } from 'svelte' import type { FlowEditorContext } from '../types' import FlowModuleWrapper from './FlowModuleWrapper.svelte' + import { moduleSlot, savedModuleById } from '../moduleSlot' import FlowSettings from './FlowSettings.svelte' import FlowInput from './FlowInput.svelte' import FlowFailureModule from './FlowFailureModule.svelte' @@ -124,7 +125,7 @@ {noEditor} disabled={disabledFlowInputs} on:openTriggers={(ev) => { - selectionManager.selectId('Trigger') + selectionManager.selectId('Trigger', { openPanel: true }) handleSelectTriggerFromKind(triggersState, triggersCount, savedFlow?.path, ev.detail.kind) showCaptureHint.set(true) }} @@ -188,12 +189,13 @@ {:else} {#key selectedId} {#each flowStore.val.value.modules as flowModule, index (flowModule.id ?? index)} + {@const slot = moduleSlot(() => flowStore.val.value.modules, flowModule.id, flowModule)} diff --git a/frontend/src/lib/components/flows/content/FlowEnvironmentVariables.svelte b/frontend/src/lib/components/flows/content/FlowEnvironmentVariables.svelte index e0f9a7d319..44204dc786 100644 --- a/frontend/src/lib/components/flows/content/FlowEnvironmentVariables.svelte +++ b/frontend/src/lib/components/flows/content/FlowEnvironmentVariables.svelte @@ -234,6 +234,11 @@ connectProp: () => {}, propPickerConfig: writable(undefined), clearConnect: () => {}, + pickerMode: () => 'popover' as const, + pickableProperties: () => undefined, + result: () => undefined, + extraResults: () => undefined, + onPick: () => {}, exprBeingEdited: writable([]) }) diff --git a/frontend/src/lib/components/flows/content/FlowLoop.svelte b/frontend/src/lib/components/flows/content/FlowLoop.svelte index a7bbe35251..d78a3199b7 100644 --- a/frontend/src/lib/components/flows/content/FlowLoop.svelte +++ b/frontend/src/lib/components/flows/content/FlowLoop.svelte @@ -1,37 +1,28 @@ +{#snippet parallelismToggle()} + { + ;(mod.value as ForloopFlow).parallelism = detail + ? { type: 'static', value: DEFAULT_PARALLELISM } + : undefined + }} + options={{ + right: PARALLELISM_LABEL, + rightTooltip: PARALLELISM_TOOLTIP, + rightDocumentationLink: 'https://www.windmill.dev/docs/flows/flow_loops' + }} + /> +{/snippet} + {#snippet header()}
-
+
Add steps inside the loop and specify an iterator expression that defines the sequence @@ -157,7 +200,7 @@
@@ -165,332 +208,171 @@
{/snippet} - - - {#if mod.value.type === 'forloopflow'} -
-
-
Skip failures If disabled, the flow will fail as soon as one of the iteration fail. Otherwise, - the error will be collected as the result of the iteration. Regardless of this - setting, if a flow level error handler is defined, it will process the error. - (Workspace error handlers will NOT be used to process errors if enabled.)
+ {#if mod.value.type === 'forloopflow'} + + + + {#snippet extra()} + + {/snippet} + + + +
+ {#if selectedTab === 'loop'} +
+ { + editor?.insertAtCursor(detail) + editor?.focus() + }} > + (mod.value as ForloopFlow).iterator, + (v) => { + ;(mod.value as ForloopFlow).iterator = v + } + } + argName="iterator" + label={ITERATOR_LABEL} + headerTooltip={ITERATOR_TOOLTIP} + error={iteratorMissing ? ITERATOR_MISSING : undefined} + schema={iteratorSchema} + noDynamicToggle + extraLib={stepPropPicker.extraLib} + pickableProperties={stepPropPicker.pickableProperties} + previousModuleId={previousModule?.id} + bind:suggestion + bind:focused={iteratorFieldFocused} + aiOnKeyUp={iteratorGen?.onKeyUp} + bind:editor + > + {#snippet aiGen()} + {#if enableAi} + (suggestion = e.detail || undefined)} + on:setExpr={(e) => setExpr(e.detail)} + pickableProperties={stepPropPicker.pickableProperties} + /> + {/if} + {/snippet} + + +
+
-
-
-
Squash - - Squashing a for loop runs all iterations on the same worker, using a single runner - per step for the entire loop. This eliminates cold starts between iterations for - supported languages (Bun, Deno, and Python). - -
{ ;(mod.value as ForloopFlow).squash = detail }} - options={{ - right: 'Squash' - }} - class="whitespace-nowrap" disabled={mod.value.parallel} - /> -
-
-
Run in parallel
- { - if (detail === false) { - ;(mod.value as ForloopFlow).parallelism = undefined - } - }} options={{ - right: 'All iterations run in parallel' + title: mod.value.parallel ? SQUASH_PARALLEL_CONFLICT : undefined, + right: 'Squash', + rightTooltip: + 'Squashing a for loop runs all iterations on the same worker, using a single runner per step for the entire loop. This eliminates cold starts between iterations for supported languages (Bun, Deno, and Python).', + rightDocumentationLink: 'https://www.windmill.dev/docs/flows/flow_loops' }} - class="whitespace-nowrap" - disabled={mod.value.squash} /> -
-
-
Parallelism Assign a maximum number of branches run in parallel to control huge for-loops. -
-
- { - const parallelismExpr = (mod.value as ForloopFlow).parallelism - - return parallelismExpr && parallelismExpr.type === 'static' - ? parallelismExpr.value - : '' - }, - (value) => { - if (value === '' || value === null || value === undefined) { - ;(mod.value as ForloopFlow).parallelism = undefined - } else { - ;(mod.value as ForloopFlow).parallelism = { - type: 'static', - value - } - } - } - } - /> - { - const forLoopFlow = mod.value as ForloopFlow - if (e.detail == parallelismType) return - if (e.detail === 'javascript') { - if (!forLoopFlow.parallelism || forLoopFlow.parallelism.type !== 'javascript') { - ;(mod.value as ForloopFlow).parallelism = { - type: 'javascript', - expr: '' - } - } - } else { - if (!forLoopFlow.parallelism || forLoopFlow.parallelism.type !== 'static') { - ;(mod.value as ForloopFlow).parallelism = { - type: 'static', - value: 0 - } - } - } + +
+ { + // An absent `parallelism` means "no cap" to the worker, so switching + // parallelism on must not seed one — the cap below is opted into. + if (!detail) (mod.value as ForloopFlow).parallelism = undefined + }} + disabled={mod.value.squash} + options={{ + title: mod.value.squash ? SQUASH_PARALLEL_CONFLICT : undefined, + right: 'Run in parallel', + rightTooltip: 'Run the iterations concurrently instead of one after the other.', + rightDocumentationLink: 'https://www.windmill.dev/docs/flows/flow_loops' }} - > - {#snippet children({ item })} - - - - {/snippet} - -
-
-
- - {#if mod.value.type === 'forloopflow' && mod.value.parallel && mod.value.parallelism?.type == 'javascript'} -
-
- Parallelism expression - - JavaScript expression that defines the maximum number of parallel executions. - Example: flow_input.max_parallel || 3 - -
-
- -
- { - parallelismEditor?.insertAtCursor(detail) - parallelismEditor?.focus() - }} - > - { - const parallelismExpr = (mod.value as ForloopFlow).parallelism - - return parallelismExpr && parallelismExpr.type === 'javascript' - ? parallelismExpr.expr - : '' - }, - (expr) => { - ;(mod.value as ForloopFlow).parallelism = { - type: 'javascript', - expr - } - } - } - class="h-full" - shouldBindKey={false} - extraLib={stepPropPicker.extraLib} /> - -
- {/if} -
-
- Iterator expression - - The JavaScript expression that will be evaluated to get the list of items to iterate - over. Example : ["banana", "apple", flow_input.my_fruit]. - -
- { - const config = { - onSelect: (code) => { - setExpr(code) - return true - }, - clearFocus: () => { - flowPropPickerConfig.set(undefined) - } - } - flowPropPickerConfig.set({ - ...config, - clearFocus: () => { - flowPropPickerConfig.set(undefined) - } - }) - }} - /> - {#if enableAi} - (suggestion = e.detail || undefined)} - on:setExpr={(e) => { - setExpr(e.detail) - }} - pickableProperties={stepPropPicker.pickableProperties} - /> - {/if} -
+ {#if mod.value.parallel} +
+ { + parallelismEditor?.insertAtCursor(detail) + parallelismEditor?.focus() + }} + > + + {#key parallelismCapped} + (mod.value as ForloopFlow).parallelism, + (v) => { + ;(mod.value as ForloopFlow).parallelism = v + } + } + argName="parallelism" + collapsed={!parallelismCapped} + header={parallelismToggle} + schema={parallelismSchema} + argExtra={{ min: 1, step: 1 }} + animateAppear + previousModuleId={previousModule?.id} + bind:editor={parallelismEditor} + /> + {/key} + +
+ {/if} +
- {#if mod.value.iterator.type == 'javascript'} - -
- { - if ($flowPropPickerConfig) { - setExpr(detail) - flowPropPickerConfig.set(undefined) - return - } - editor?.insertAtCursor(detail) - editor?.focus() - }} - noPadding - > -
- { - iteratorFieldFocused = true - }} - on:blur={() => { - iteratorFieldFocused = false - }} - lang="javascript" - bind:code={mod.value.iterator.expr} - class="h-full" - shouldBindKey={false} - extraLib={stepPropPicker.extraLib} - {suggestion} - /> -
- -
+ + {:else} -
+ {/if} +
diff --git a/frontend/src/lib/components/flows/content/FlowModuleCache.svelte b/frontend/src/lib/components/flows/content/FlowModuleCache.svelte index 99fc4c7b3a..981e8a5aa9 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleCache.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleCache.svelte @@ -1,12 +1,12 @@ -
- {#snippet header()} - - If defined, the result of the step will be cached for the number of seconds defined such that - if this step were to be re-triggered with the same input it would retrieve and return its - cached value instead of recomputing it. - - {/snippet} - +
{#if flowModule.value.type == 'script'} {:else} { - if (isCacheEnabled && flowModule.cache_ttl != undefined) { + if (isCacheEnabled) { flowModule.cache_ttl = undefined } else { - flowModule.cache_ttl = 600 + flowModule.cache_ttl = stepSettingDefaults('cache') } }} options={{ - right: 'Cache the results for each possible inputs' + right: 'Cache results', + rightTooltip: + 'The result of the step is cached for the configured number of seconds; a re-trigger with the same input returns the cached value instead of recomputing it.', + rightDocumentationLink: 'https://www.windmill.dev/docs/flows/cache' }} /> - {#if flowModule.cache_ttl} - - flowModule.cache_ignore_s3_path, - (v) => (flowModule.cache_ignore_s3_path = v || undefined) - } - options={{ - right: 'Ignore S3 Object paths for caching purposes', - rightTooltip: - 'If two S3 objects passed as input have the same content, they will hit the same cache entry, regardless of their path.' - }} - /> + {#if isCacheEnabled} +
+ + flowModule.cache_ignore_s3_path, + (v) => (flowModule.cache_ignore_s3_path = v || undefined) + } + options={{ + right: 'Ignore S3 object paths', + rightTooltip: + 'If two S3 objects passed as input have the same content, they will hit the same cache entry, regardless of their path.' + }} + /> +
{/if} {/if} -
+
diff --git a/frontend/src/lib/components/flows/content/FlowModuleComponent.svelte b/frontend/src/lib/components/flows/content/FlowModuleComponent.svelte index 75c0d03d65..d1fdf8f412 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleComponent.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleComponent.svelte @@ -1,10 +1,8 @@ -
- {#snippet header()} - - The logs, arguments and results of this flow step will be completely deleted from Windmill - after the specified delay once the flow is complete. They might be temporarily visible in UI - while the flow is running. -
- This also applies to a flow step that has failed: the error will not be accessible. -
-
- The deletion is irreversible. Set to 0 for immediate deletion. - {#if disabled} -
-
- This option is only available on Windmill Enterprise Edition. - {/if} - - {/snippet} - +
{ if (enabled) { flowModule.delete_after_secs = undefined } else { - flowModule.delete_after_secs = 0 + flowModule.delete_after_secs = stepSettingDefaults('lifetime') } }} options={{ - right: 'Delete logs, arguments and results after the flow is complete' + right: 'Delete after use', + rightTooltip: tip }} /> {#if enabled} -
+
{/if} -
+
diff --git a/frontend/src/lib/components/flows/content/FlowModuleEarlyStop.svelte b/frontend/src/lib/components/flows/content/FlowModuleEarlyStop.svelte index fbe747cff3..8f0f091320 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleEarlyStop.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleEarlyStop.svelte @@ -1,26 +1,43 @@ - -
- {#if !isBranchAll && !isParallelLoop} -
- {#snippet header()} - - If defined, at the end of the step, the predicate expression will be evaluated to decide - if the flow should stop early, skip rest of steps in iteration/branch if inside a parallel - for loop or branch all, or break if inside a for/while loop or branch all. - - {/snippet} + ? { + label: 'Skip rest of iteration if', + tooltip: + 'Evaluated after this step. When it returns true the remaining steps of this iteration are skipped; the other iterations are unaffected.' + } + : { + label: 'Skip rest of branch if', + tooltip: + 'Evaluated after this step. When it returns true the remaining steps of this branch are skipped; the other branches are unaffected.' + } + : { + label: 'Break parent loop if', + tooltip: `Evaluated after this step. When it returns true the enclosing loop ${breakableParent.stepId} stops iterating and the flow carries on.` + } + : { + label: 'Stop flow if', + tooltip: + "Evaluated after this step. When it returns true the flow stops here and returns this step's result." + } + ) - { - if (isStopAfterIfEnabled && flowModule.stop_after_if) { - flowModule.stop_after_if = undefined - } else { - flowModule.stop_after_if = { - expr: 'result == undefined', - skip_if_stopped: false, - error_message: undefined, - error_include_result: false + // The all-iterations predicate runs once the loop or branch-all is done, over what + // every iteration returned, so it can decide on the whole rather than on one result. + let allItersPrefix = $derived( + `Evaluated once ${isBranchAll ? 'every branch' : 'every iteration'} has completed, over their collected results.` + ) + let stopAfterAllItersCopy = $derived( + breakableParent + ? breakableParent.isParallel + ? breakableParent.type === 'loop' + ? { + label: 'Skip rest of iteration if', + tooltip: `${allItersPrefix} When it returns true the remaining steps of the enclosing iteration are skipped.` + } + : { + label: 'Skip rest of branch if', + tooltip: `${allItersPrefix} When it returns true the remaining steps of the enclosing branch are skipped.` + } + : { + label: `Break parent loop ${breakableParent.stepId} if`, + tooltip: `${allItersPrefix} When it returns true the enclosing loop stops iterating and the flow carries on.` + } + : { + label: 'Stop flow if', + tooltip: `${allItersPrefix} When it returns true the flow stops here and returns them.` + } + ) + + let earlyStopResult = $derived( + isLoop + ? Array.isArray(result) && result.length > 0 + ? result[result.length - 1] + : result === NEVER_TESTED_THIS_FAR + ? result + : undefined + : result + ) + + +{#snippet stopStatusPicker(stop: StopAfterIf)} +
+ + {#if stop.error_message != undefined} +
+ stop.error_message ?? '', (v) => (stop.error_message = String(v))} + inputProps={{ placeholder: 'Enter custom error message (optional)' }} + /> + stop.error_include_result ?? false, (v) => (stop.error_include_result = v) + } + options={{ + right: 'Include result in error', + rightTooltip: + "When enabled, this step's output is embedded inside the raised error object (as error.result) instead of being discarded. The flow result stays { error }." + }} + /> +
+ {/if} +
+{/snippet} + +{#snippet stopAfterToggle()} + { + if (isStopAfterIfEnabled && flowModule.stop_after_if) { + flowModule.stop_after_if = undefined + } else { + flowModule.stop_after_if = stepSettingDefaults('early-stop') + } + }} + options={{ + title: isParallelLoop ? stopAfterCopy.tooltip : undefined, + right: stopAfterCopy.label, + rightTooltip: stopAfterCopy.tooltip, + rightDocumentationLink: 'https://www.windmill.dev/docs/flows/early_stop' + }} + /> +{/snippet} + +{#snippet stopAfterAllItersToggle()} + { + if (isStopAfterAllIterationsEnabled && flowModule.stop_after_all_iters_if) { + flowModule.stop_after_all_iters_if = undefined + } else { + flowModule.stop_after_all_iters_if = stepSettingDefaults('early-stop') + } + }} + options={{ + right: stopAfterAllItersCopy.label, + rightTooltip: stopAfterAllItersCopy.tooltip, + rightDocumentationLink: 'https://www.windmill.dev/docs/flows/early_stop' + }} + /> +{/snippet} + +
+ {#if blocks !== 'all-iters' && !isBranchAll} +
+ { + stopAfterEditor?.insertAtCursor(detail) + stopAfterEditor?.focus() + }} + > + flowModule.stop_after_if, + (v) => { + flowModule.stop_after_if = v } } - }} - options={{ - right: isLoop - ? 'Break loop' - : breakableParent - ? breakableParent.isParallel - ? breakableParent.type === 'loop' - ? 'Skip rest of steps in iteration' - : 'Skip rest of steps in branch' - : 'Break parent loop module' - : 'Stop flow if condition met' - }} - /> - -
- {#if flowModule.stop_after_if} - {@const earlyStopResult = isLoop - ? Array.isArray(result) && result.length > 0 - ? result[result.length - 1] - : result === NEVER_TESTED_THIS_FAR - ? result - : undefined - : result} - {#if !breakableParent && !isLoop} -
- { - if (flowModule.stop_after_if && event.detail) { - flowModule.stop_after_if.error_message = undefined - flowModule.stop_after_if.error_include_result = false - raise_error_message_stop_after_if = false - } - }} - options={{ - right: 'Label flow as "skipped" if stopped' - }} - /> - { - if (flowModule.stop_after_if) { - if (event.detail) { - flowModule.stop_after_if.error_message = '' - flowModule.stop_after_if.skip_if_stopped = false - } else { - flowModule.stop_after_if.error_message = undefined - flowModule.stop_after_if.error_include_result = false - } - } - }} - options={{ - right: 'Raise an error message if stopped', - rightTooltip: - 'If enabled and the stop condition is met, an error message will be raised. A custom message can be provided; otherwise, a default message will be used. Mutually exclusive with "Label flow as skipped".' - }} - /> -
- {/if} - {#if raise_error_message_stop_after_if} - - - {/if} - Stop condition expression -
- { - editor?.insertAtCursor(detail) - editor?.focus() - }} - > - - -
- {:else} - {#if !breakableParent && !isLoop} -
- - -
- {/if} - Stop condition expression - - {/if} -
-
+ argName="stop_after_if" + argType="javascript" + collapsed={!isStopAfterIfEnabled || isParallelLoop} + animateAppear + header={stopAfterToggle} + noDynamicToggle + schema={predicateSchema} + previousModuleId={undefined} + pickableProperties={stepPropPicker.pickableProperties} + extraLib={`declare const result = ${JSON.stringify(earlyStopResult)};\n` + + stepPropPicker.extraLib + + (isLoop ? `\ndeclare const all_iters = ${JSON.stringify(result)};` : '')} + bind:editor={stopAfterEditor} + /> + + {#if isStopAfterIfEnabled && !breakableParent && !isLoop && flowModule.stop_after_if} + {@render stopStatusPicker(flowModule.stop_after_if)} + {/if} +
{/if} - {#if isLoop || isBranchAll} -
- {#snippet header()} - - If defined, at the end of the step, the predicate expression will be evaluated to decide - if the flow should stop early, skip rest of steps in iteration/branch if inside a parallel - for loop or branch all, or break if inside a for/while loop or branch all. - - {/snippet} - - { - if (isStopAfterAllIterationsEnabled && flowModule.stop_after_all_iters_if) { - flowModule.stop_after_all_iters_if = undefined - } else { - flowModule.stop_after_all_iters_if = { - expr: 'result == undefined', - skip_if_stopped: false, - error_message: undefined, - error_include_result: false + {#if blocks !== 'stop-after' && (isLoop || isBranchAll)} +
+ { + stopAfterAllItersEditor?.insertAtCursor(detail) + stopAfterAllItersEditor?.focus() + }} + > + flowModule.stop_after_all_iters_if, + (v) => { + flowModule.stop_after_all_iters_if = v } } - }} - options={{ - right: - (breakableParent - ? breakableParent.isParallel - ? breakableParent.type === 'loop' - ? 'Skip rest of steps in iteration' - : 'Skip rest of steps in branch' - : 'Break parent loop module ' + breakableParent.stepId - : 'Stop flow') + ' if condition met' - }} - /> - -
- {#if flowModule.stop_after_all_iters_if} - {#if !breakableParent} -
- { - if (flowModule.stop_after_all_iters_if && event.detail) { - flowModule.stop_after_all_iters_if.error_message = undefined - flowModule.stop_after_all_iters_if.error_include_result = false - raise_error_message_stop_after_all_if = false - } - }} - options={{ - right: 'Label flow as "skipped" if stopped' - }} - /> - { - if (flowModule.stop_after_all_iters_if) { - if (event.detail) { - flowModule.stop_after_all_iters_if.error_message = '' - flowModule.stop_after_all_iters_if.skip_if_stopped = false - } else { - flowModule.stop_after_all_iters_if.error_message = undefined - flowModule.stop_after_all_iters_if.error_include_result = false - } - } - }} - options={{ - right: 'Raise an error message if stopped', - rightTooltip: - 'If enabled and the stop condition is met, an error message will be raised. A custom message can be provided; otherwise, a default message will be used. Mutually exclusive with "Label flow as skipped".' - }} - /> -
- {/if} - {#if raise_error_message_stop_after_all_if} - - - {/if} - Stop condition expression -
- { - editor?.insertAtCursor(detail) - editor?.focus() - }} - > - - -
- {:else} - {#if !breakableParent} -
- - -
- {/if} - Stop condition expression - - {/if} -
-
+ argName="stop_after_all_iters_if" + argType="javascript" + collapsed={!isStopAfterAllIterationsEnabled} + animateAppear + header={stopAfterAllItersToggle} + noDynamicToggle + schema={predicateSchema} + previousModuleId={undefined} + pickableProperties={stepPropPicker.pickableProperties} + extraLib={`declare const result = ${JSON.stringify(result)};\n` + stepPropPicker.extraLib} + bind:editor={stopAfterAllItersEditor} + /> + + {#if isStopAfterAllIterationsEnabled && !breakableParent && flowModule.stop_after_all_iters_if} + {@render stopStatusPicker(flowModule.stop_after_all_iters_if)} + {/if} +
{/if}
diff --git a/frontend/src/lib/components/flows/content/FlowModuleHeader.svelte b/frontend/src/lib/components/flows/content/FlowModuleHeader.svelte index b34a78e1db..63d053fe14 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleHeader.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleHeader.svelte @@ -2,24 +2,10 @@ import Button from '$lib/components/common/button/Button.svelte' import { type FlowModule } from '$lib/gen' import { createEventDispatcher, getContext } from 'svelte' - import { - Bed, - Database, - Gauge, - GitFork, - Pen, - PhoneIncoming, - RefreshCcw, - Repeat, - Square, - Pin, - Save, - Settings - } from 'lucide-svelte' - import Popover from '../../Popover.svelte' + import { Pen, RefreshCcw, Save } from 'lucide-svelte' + import DropdownV2 from '../../DropdownV2.svelte' import type { FlowEditorContext } from '../types' import { sendUserToast } from '$lib/utils' - import { getLatestHashForScript } from '$lib/scripts' import type { FlowBuilderWhitelabelCustomUi } from '$lib/components/custom_ui' import FlowModuleWorkerTagSelect from './FlowModuleWorkerTagSelect.svelte' @@ -29,134 +15,14 @@ } let { module, tag }: Props = $props() - const { scriptEditorDrawer, workspaceScriptSettingsDrawer, flowEditorDrawer, opWorkspace } = - getContext('FlowEditorContext') + const { flowEditorDrawer } = getContext('FlowEditorContext') const dispatch = createEventDispatcher() let customUi: undefined | FlowBuilderWhitelabelCustomUi = getContext('customUi') - - let popoverClasses = - 'center-center rounded p-2 bg-blue-100 text-blue-800 border border-blue-300 hover:bg-blue-200 dark:bg-frost-700 dark:text-frost-100 dark:border-frost-600' -
- {#if module.value.type === 'script' || module.value.type === 'rawscript' || module.value.type == 'flow'} - {#if module.retry?.constant || module.retry?.exponential} - dispatch('toggleRetry')}> - - {#snippet text()} - Retries - {/snippet} - - {/if} - {#if module?.value?.['concurrent_limit'] != undefined} - dispatch('toggleConcurrency')} - > - - {#snippet text()} - Concurrency Limits - {/snippet} - - {/if} - {#if module.cache_ttl != undefined} - dispatch('toggleCache')}> - - {#snippet text()} - Cache - {/snippet} - - {/if} - {#if module.stop_after_if || module.stop_after_all_iters_if} - dispatch('toggleStopAfterIf')} - > - - {#snippet text()} - Early stop/break - {/snippet} - - {/if} - {#if module.suspend} - dispatch('toggleSuspend')}> - - {#snippet text()} - Suspend - {/snippet} - - {/if} - {#if module.sleep} - dispatch('toggleSleep')}> - - {#snippet text()} - Sleep - {/snippet} - - {/if} - {#if module.mock?.enabled} - dispatch('togglePin')}> - - {#snippet text()} - This step is pinned - {/snippet} - - {/if} - {/if} +
{#if module.value.type === 'script'} - {#if !module.value.path.startsWith('hub/') && customUi?.scriptEdit != false} - - + dispatch('createScriptFromInlineScript') + } + ]} + /> {/if}
diff --git a/frontend/src/lib/components/flows/content/FlowModuleMock.svelte b/frontend/src/lib/components/flows/content/FlowModuleMock.svelte index e3d4cb3bea..e56fee962c 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleMock.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleMock.svelte @@ -2,11 +2,11 @@ import { run } from 'svelte/legacy' import Toggle from '$lib/components/Toggle.svelte' - import Tooltip from '$lib/components/Tooltip.svelte' import type { FlowModule } from '$lib/gen' - import { Section } from '$lib/components/common' + import Label from '$lib/components/Label.svelte' import JsonEditor from '$lib/components/JsonEditor.svelte' import { untrack } from 'svelte' + import { slideDynamic } from '$lib/transitions' interface Props { flowModule: FlowModule @@ -56,47 +56,38 @@ } -
- {#snippet header()} -
- - If defined and enabled, the step will immediately return the mock value instead of being - executed. - - { - if (isMockEnabled) { - flowModule.mock = { - enabled: false, - return_value: flowModule.mock?.return_value - } - } else { - flowModule.mock = { - enabled: true, - return_value: flowModule.mock?.return_value ?? { example: 'value' } - } - code = JSON.stringify(flowModule.mock?.return_value, null, 2) - } - }} - size="xs" - /> +
+ { + if (isMockEnabled) { + flowModule.mock = { + enabled: false, + return_value: flowModule.mock?.return_value + } + } else { + flowModule.mock = { + enabled: true, + return_value: flowModule.mock?.return_value ?? { example: 'value' } + } + code = JSON.stringify(flowModule.mock?.return_value, null, 2) + } + }} + options={{ + right: 'Pin output', + rightTooltip: + 'While pinned, the step returns this value immediately instead of executing. The same control lives on the step in the graph.' + }} + /> + {#if isMockEnabled} +
+
- {/snippet} - -
- Mocked Return value - - {#if isMockEnabled} - {#key renderCount} - - {/key} - {:else} - 
{flowModule.mock?.return_value
-					? JSON.stringify(flowModule.mock?.return_value, null, 2)
-					: ''}
- {/if} -
-
+ {/if} +
diff --git a/frontend/src/lib/components/flows/content/FlowModuleMockTransitionMessage.svelte b/frontend/src/lib/components/flows/content/FlowModuleMockTransitionMessage.svelte deleted file mode 100644 index 34384cc506..0000000000 --- a/frontend/src/lib/components/flows/content/FlowModuleMockTransitionMessage.svelte +++ /dev/null @@ -1,145 +0,0 @@ - - - - -
- -
-
-
NEW
- Mock has evolved into - - - PIN - -
-
- Find it in: - "Test this step" tab -
-
- - -
- - How to use the PIN feature: - - -
- -
-
1
-
-
Pick a result from history
-
- - - History picker -
-
-
- - -
-
2
-
-
Pin it as a fixed output
-
- - - Pin action -
-
-
- - -
-
-
-
The last pin can be recovered from history
-
- - - Recover pins -
-
-
- - -
-
- -
-
-
All of this can be done from the flow view
-
- - - Flow view pinning -
-
-
-
-
-
diff --git a/frontend/src/lib/components/flows/content/FlowModuleSkip.svelte b/frontend/src/lib/components/flows/content/FlowModuleSkip.svelte index ffb0a8d502..51d75b0ca6 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleSkip.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleSkip.svelte @@ -1,12 +1,13 @@ -
-
- {#snippet header()} - - If the condition is met, the step will behave as an identity step, passing the previous - step's result through unchanged. - - {/snippet} +{#snippet skipToggle()} + { + if (isSkipEnabled && flowModule.skip_if) { + flowModule.skip_if = undefined + } else { + flowModule.skip_if = stepSettingDefaults('skip') + } + }} + options={{ + right: 'Skip step if', + rightTooltip: + "If the condition is met, the step behaves as an identity step, passing the previous step's result through unchanged." + }} + /> +{/snippet} - { - if (isSkipEnabled && flowModule.skip_if) { - flowModule.skip_if = undefined - } else { - flowModule.skip_if = { - expr: 'false' - } +
+ { + editor?.insertAtCursor(detail) + editor?.focus() + }} + > + flowModule.skip_if, + (v) => { + flowModule.skip_if = v } - }} - options={{ - right: 'Skip step if condition is met' - }} + } + argName="skip_if" + argType="javascript" + collapsed={!isSkipEnabled} + animateAppear + header={skipToggle} + noDynamicToggle + {schema} + previousModuleId={previousModule?.id} + pickableProperties={stepPropPicker.pickableProperties} + extraLib={`declare const result = ${JSON.stringify(result)};\n` + stepPropPicker.extraLib} + bind:editor /> - -
- {#if flowModule.skip_if} - Skip condition expression -
- { - editor?.insertAtCursor(detail) - editor?.focus() - }} - > - - -
- {:else} - Skip condition expression - - {/if} -
-
+
diff --git a/frontend/src/lib/components/flows/content/FlowModuleSleep.svelte b/frontend/src/lib/components/flows/content/FlowModuleSleep.svelte index 92086ccd37..03dcf2b6f5 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleSleep.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleSleep.svelte @@ -2,16 +2,14 @@ import InputTransformForm from '$lib/components/InputTransformForm.svelte' import type SimpleEditor from '$lib/components/SimpleEditor.svelte' import Toggle from '$lib/components/Toggle.svelte' - import Tooltip from '$lib/components/Tooltip.svelte' import type { FlowModule } from '$lib/gen' + import { stepSettingDefaults } from '../flowStepSettings' import { emptySchema } from '$lib/utils' import { getContext } from 'svelte' import PropPickerWrapper from '../propPicker/PropPickerWrapper.svelte' import type { FlowEditorContext } from '../types' - import { SecondsInput } from '../../common' - import Section from '$lib/components/Section.svelte' - import Label from '$lib/components/Label.svelte' import { getStepPropPicker } from '../previousResults' + import { slideDynamic } from '$lib/transitions' import { SAME_WORKER_INCOMPATIBLE_MSG } from '../utils.svelte' import { Alert } from '$lib/components/common' @@ -51,66 +49,54 @@ let sameWorker = $derived(Boolean(!isAgentTool && flowStore.val.value.same_worker)) -
- {#snippet header()} - - If defined, at the end of the step, the flow will sleep for a number of seconds before - scheduling the next job (if any, no effect if the step is the last one). - - {/snippet} - +
{#if sameWorker} - + {SAME_WORKER_INCOMPATIBLE_MSG} Disable `Same Worker` in the flow settings to use a sleep. {/if} - { if (isSleepEnabled && flowModule.sleep != undefined) { flowModule.sleep = undefined } else { - flowModule.sleep = { - type: 'static', - value: 0 - } + flowModule.sleep = stepSettingDefaults('sleep') } }} options={{ - right: 'Sleep after module successful execution' + right: 'Sleep after step', + rightTooltip: + 'At the end of the step, the flow sleeps for a number of seconds before scheduling the next job (no effect if the step is the last one).', + rightDocumentationLink: 'https://www.windmill.dev/docs/flows/sleep' }} /> - -
+ {#if flowModule.sleep && schema.properties['sleep'] && !sameWorker} +
+ { + editor?.insertAtCursor(detail) + editor?.focus() + }} + > + + +
+ {/if} +
diff --git a/frontend/src/lib/components/flows/content/FlowModuleSuspend.svelte b/frontend/src/lib/components/flows/content/FlowModuleSuspend.svelte index 3a9e791d2d..a20bb8c671 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleSuspend.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleSuspend.svelte @@ -1,22 +1,23 @@ -
- {#snippet action()} - - {/snippet} - {#snippet header()} -
- - If defined, at the end of the step, the flow will be suspended until it receives external - requests to be resumed or canceled. This is most useful to implement approval steps but can - be used flexibly for other purposes. - - { - if (isSuspendEnabled && flowModule.suspend != undefined) { - flowModule.suspend = undefined - } else { - flowModule.suspend = { - required_events: 1, - timeout: 1800 - } - } - }} - options={{ - right: 'Suspend flow execution until events/approvals received' - }} - /> -
- {/snippet} +
+ { + if (isSuspendEnabled && flowModule.suspend != undefined) { + flowModule.suspend = undefined + } else { + flowModule.suspend = stepSettingDefaults('suspend') + } + }} + options={{ + right: 'Suspend until approval/resume', + rightTooltip: + 'At the end of the step, the flow is suspended until it receives external requests to resume or cancel it. Most useful for approval steps, but can be used flexibly for other purposes.', + rightDocumentationLink: 'https://www.windmill.dev/docs/flows/flow_approval' + }} + /> -
- - - - - -
+ {#if isSuspendEnabled} +
+
+ + + + + +
- {#if suspendTabSelected === 'core'} -
- - + {#if suspendTabSelected === 'core'} +
+ + - { - if (flowModule.suspend) { - flowModule.suspend.continue_on_disapprove_timeout = e.detail - } - }} - /> - {#if Boolean(flowModule.suspend?.continue_on_disapprove_timeout)} - - We recommend using the expr resume?.error to handle null payload values. -
- To filter timeout, use resume?.error?.name === "SuspendedTimedOut".
- To filter disapproval, use resume?.error?.name === "SuspendedDisapproved" - - {/if} -
- {:else if suspendTabSelected === 'permissions'} -
- {#if emptyString($enterpriseLicense)} - - {/if} - {#if flowModule.suspend} -
- { if (flowModule.suspend) { - flowModule.suspend.user_auth_required = e.detail - if (e.detail && flowModule.suspend?.user_groups_required === undefined) { - flowModule.suspend.user_groups_required = { - type: 'static', - value: [] + flowModule.suspend.continue_on_disapprove_timeout = e.detail + } + }} + /> + {#if Boolean(flowModule.suspend?.continue_on_disapprove_timeout)} + + We recommend using the expr resume?.error to handle null payload values. +
+ To filter timeout, use resume?.error?.name === "SuspendedTimedOut". +
+ To filter disapproval, use resume?.error?.name === "SuspendedDisapproved" + + {/if} +
+ {:else if suspendTabSelected === 'permissions'} +
+
+ { + if (flowModule.suspend) { + flowModule.suspend.user_auth_required = e.detail + if (e.detail && flowModule.suspend?.user_groups_required === undefined) { + flowModule.suspend.user_groups_required = { + type: 'static', + value: [] + } + } else if (!e.detail) { + flowModule.suspend.user_groups_required = undefined + flowModule.suspend.self_approval_disabled = false } - } else if (!e.detail) { - flowModule.suspend.user_groups_required = undefined - flowModule.suspend.self_approval_disabled = false } - } - }} - /> + }} + /> - { - if (flowModule.suspend) { - flowModule.suspend.self_approval_disabled = e.detail - } - }} - /> + { + if (flowModule.suspend) { + flowModule.suspend.self_approval_disabled = e.detail + } + }} + /> -
+
- {#if Boolean(flowModule.suspend.user_auth_required) && allUserGroups.length !== 0 && flowModule.suspend && schema.properties['groups']} - Require approvers to be members of one of the following user groups (leave empty for - any) - -
+ {#if Boolean(flowModule.suspend?.user_auth_required) && allUserGroups.length !== 0 && flowModule.suspend && schema.properties['groups']} + Require approvers to be members of one of the following user groups (leave empty + for any) + { @@ -231,75 +237,79 @@ bind:editor /> + {/if} +
+
+ {:else} +
+ {#if flowModule?.suspend?.resume_form} +
+
+ +
+ +
+ {:else if emptyString($enterpriseLicense)} + + {:else} + + {/if} + + flowModule.suspend?.resume_form?.schema ?? draftFormSchema, + (v) => { + if (flowModule.suspend) { + flowModule.suspend.resume_form = { schema: v } + } + } + } + drawerOnly + /> + + {#if flowModule.suspend?.resume_form} + { + if (flowModule.suspend) { + flowModule.suspend.hide_cancel = e.detail + } + }} + options={{ + right: 'Hide cancel button on approval page' + }} + /> {/if}
{/if} -
- {:else} -
-
- {#if flowModule?.suspend?.resume_form} - - {:else if emptyString($enterpriseLicense)} - - {:else} -
- { - if (flowModule.suspend) { - flowModule.suspend.resume_form = { - schema: emptySchema() - } - } - jsonView = true - }} - /> -
- { - jsonView = false - if (flowModule.suspend) { - flowModule.suspend.resume_form = { - schema: e.detail - } - } - }} - schema={{}} - /> - {/if} -
-
- {#if flowModule.suspend} - {#if emptyString($enterpriseLicense)} - - {/if} -
-
- -
-
- {/if} - {#if flowModule.suspend} - - {/if} +
+
{/if} -
+
diff --git a/frontend/src/lib/components/flows/content/FlowModuleTimeout.svelte b/frontend/src/lib/components/flows/content/FlowModuleTimeout.svelte index 62a3432995..9f6a1b32b2 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleTimeout.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleTimeout.svelte @@ -1,19 +1,17 @@ -
- {#snippet header()} - - If defined, the custom timeout will be used instead of the instance timeout for the step. The - step's timeout cannot be greater than the instance timeout. - - {/snippet} - +
{ if (istimeoutEnabled && flowModule.timeout != undefined) { @@ -77,39 +70,37 @@ } }} options={{ - right: 'Add a custom timeout for this step' + right: 'Custom timeout', + rightTooltip: + "The custom timeout is used instead of the instance timeout for the step. The step's timeout cannot be greater than the instance timeout." }} /> - + {#if flowModule.timeout && schema.properties['timeout']} +
+ { + editor?.insertAtCursor(detail) + editor?.focus() + }} + > + + +
+ {/if} {#if flowModule.timeout && flowModule.timeout.type !== 'static'} -
+

A dynamic timeout expression is evaluated when running the full flow. It is ignored when @@ -118,4 +109,4 @@

{/if} -
+
diff --git a/frontend/src/lib/components/flows/content/FlowModuleWrapper.svelte b/frontend/src/lib/components/flows/content/FlowModuleWrapper.svelte index 0cae21b630..6f832902d0 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleWrapper.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleWrapper.svelte @@ -1,8 +1,11 @@ @@ -44,7 +38,6 @@ {#if isOwner !== undefined && suspendStatus} { + const r = flowModuleRetry + if (!r) return + untrack(() => { + if (delayType === 'constant' && !r.constant && r.exponential) { + delayType = 'exponential' + } else if (delayType === 'exponential' && !r.exponential && r.constant) { + delayType = 'constant' + } else if (delayType === 'disabled' && (r.constant || r.exponential)) { + // Retries added from outside: without this the row keeps reading "off" while + // rendering the incoming values greyed out, and toggling on would overwrite them. + delayType = r.constant ? 'constant' : 'exponential' + } + }) + }) + let displayRetry = $derived( + flowModuleRetry ?? { + constant: { attempts: 1, seconds: 5 }, + exponential: { attempts: 1, multiplier: 1, seconds: 5, random_factor: 0 } + } + ) + // Always-defined sub-configs so the read-only off-state can bind without + // undefined checks; when the real config exists these are the same refs. + let displayConstant = $derived(displayRetry.constant ?? { attempts: 1, seconds: 5 }) + let displayExponential = $derived( + displayRetry.exponential ?? { attempts: 1, multiplier: 1, seconds: 5, random_factor: 0 } + ) + // Only feed the preview the branch that's actually shown, so the off-state + // defaults don't render a bogus combined schedule. + let previewRetry = $derived( + retriesOff + ? displayDelayType === 'constant' + ? { constant: displayRetry.constant } + : { exponential: displayRetry.exponential } + : flowModuleRetry + ) let result = $derived( flowModule && flowStateStore?.val ? (flowStateStore.val[flowModule.id]?.previewResult ?? NEVER_TESTED_THIS_FAR) @@ -99,10 +157,12 @@ } function initialLoad() { + // Presence, not attempts > 0: a retry block with zero attempts is still + // configured, and flowStepSettings describes it that way. delayType = - (flowModuleRetry?.constant?.attempts ?? 0) > 0 + flowModuleRetry?.constant != undefined ? 'constant' - : (flowModuleRetry?.exponential?.attempts ?? 0) > 0 + : flowModuleRetry?.exponential != undefined ? 'exponential' : 'disabled' loaded = true @@ -122,258 +182,289 @@ const u32Max = 4294967295 -
+
{#if sameWorker} {SAME_WORKER_INCOMPATIBLE_MSG} Disable `Same Worker` in the flow settings to use retries. {/if} - - { - flowModuleRetry = undefined - if (e.detail === 'constant') { + checked={delayType === 'constant' || delayType === 'exponential'} + on:change={() => { + if (delayType === 'constant' || delayType === 'exponential') { flowModuleRetry = undefined + delayType = 'disabled' + } else { setConstantRetries() - } else if (e.detail === 'exponential') { - flowModuleRetry = undefined - setExponentialRetries() + delayType = 'constant' } }} - > - {#snippet children({ item })} - - - - {/snippet} - + options={{ + right: 'Retry on failure', + rightTooltip: + 'Upon error this step is retried with a delay and a maximum number of attempts as defined below.', + rightDocumentationLink: 'https://www.windmill.dev/docs/flows/retries' + }} + /> - {#if (delayType === 'constant' || delayType === 'exponential') && !sameWorker} -
- {#snippet header()} - - Optional condition to determine when to retry. If not specified, will retry on any failure - within the configured attempt limits. - - {/snippet} - - { - if (!flowModuleRetry) { - return + {#if !retriesOff && !sameWorker} +
+ { + flowModuleRetry = undefined + if (e.detail === 'constant') { + setConstantRetries() + delayType = 'constant' + } else if (e.detail === 'exponential') { + setExponentialRetries() + delayType = 'exponential' } - if (isRetryConditionEnabled && flowModuleRetry.retry_if) { - const { retry_if, ...rest } = flowModuleRetry - flowModuleRetry = rest - } else { - flowModuleRetry = { - ...flowModuleRetry, - retry_if: { - expr: 'error && error.name !== "PERMANENT_FAILURE"' + }} + > + {#snippet children({ item })} + + + {/snippet} + + + {#snippet retryConditionToggle()} + { + if (!flowModuleRetry) { + return + } + if (isRetryConditionEnabled && flowModuleRetry.retry_if) { + const { retry_if, ...rest } = flowModuleRetry + flowModuleRetry = rest + } else { + flowModuleRetry = { + ...flowModuleRetry, + retry_if: { + expr: 'error && error.name !== "PERMANENT_FAILURE"' + } } } - } - }} - options={{ - right: 'Only retry if condition is met' - }} - /> + }} + options={{ + right: 'Conditional retry', + rightTooltip: + 'Optional condition to determine when to retry. Expression should return true to retry, false to skip retry. If not specified, retries on any failure within the configured attempt limits.' + }} + /> + {/snippet} -
- {#if flowModuleRetry?.retry_if} - Retry condition expression - Expression should return true to retry, false to skip retry -
- {#if stepPropPicker} - { - editor?.insertAtCursor(detail) - editor?.focus() - }} - > - - - {:else} + {#if stepPropPicker} + { + retryIfEditor?.insertAtCursor(detail) + retryIfEditor?.focus() + }} + > + flowModuleRetry?.retry_if, + (v) => { + if (flowModuleRetry) flowModuleRetry.retry_if = v + } + } + argName="retry_if" + argType="javascript" + collapsed={!isRetryConditionEnabled} + animateAppear + header={retryConditionToggle} + noDynamicToggle + schema={predicateSchema} + previousModuleId={undefined} + pickableProperties={stepPropPicker.pickableProperties} + extraLib={`declare const result = ${JSON.stringify(result)};` + + `\ndeclare const flow_input = ${JSON.stringify(stepPropPicker.pickableProperties.flow_input || {})};`} + bind:editor={retryIfEditor} + /> + + {:else} +
+ {@render retryConditionToggle()} + {#if flowModuleRetry?.retry_if} +
- {/if} -
- {:else} - Retry condition expression - Expression should return true to retry, false to skip retry - - {/if} -
-
- {/if} +
+ {/if} +
+ {/if} - {#if (delayType === 'constant' || delayType === 'exponential') && !sameWorker} -
-
- {#if delayType === 'constant'} - {#if flowModuleRetry?.constant} -
Attempts
-
+
+
+ {#if displayDelayType === 'constant'} +
Attempts
+
+ + +
+
Delay
+ + {:else if displayDelayType === 'exponential'} +
Attempts
+
+ + +
+
Multiplier
+ delay = multiplier * base ^ (number of attempt) - -
-
Delay
- - {/if} - {:else if delayType === 'exponential'} - {#if flowModuleRetry?.exponential} -
Attempts
-
- - -
-
Multiplier
- delay = multiplier * base ^ (number of attempt) - -
Base (in seconds)
- - {#if validationError} - {validationError} - {:else} - Must be ≥ 1. A base of 0 would cause immediate retries. - {/if} -
Randomization factor (percentage)
-
- {#if !$enterpriseLicense} - - {/if} +
Base (in seconds)
-
+ {#if validationError} + {validationError} + {:else} + Must be ≥ 1. A base of 0 would cause immediate retries. + {/if} +
Randomization factor (percentage)
+
+ {#if !$enterpriseLicense} + + {/if} +
+ +
-
- {/if} - {/if} -
-
- {#if true} - {@const { attempts: cAttempts, seconds: cSeconds } = flowModuleRetry?.constant || {}} - {@const { - attempts: eAttempts, - seconds: eSeconds, - multiplier, - random_factor - } = flowModuleRetry?.exponential || {}} - {@const cArray = Array.from({ length: Math.min(cAttempts || 0, 100) }, () => cSeconds)} - {@const eArray = Array.from( - { length: Math.min(eAttempts || 0, 100) }, - (_, i) => (multiplier || 0) * (eSeconds || 0) ** (i + cArray.length + 1) - )} - {@const array = [...cArray, ...eArray]} -
-
Retry attempts
- {#if array.length > 0} - - - - - - - - - {#each array.slice(1, 100) as delay, i} - {@const index = i + 2} - - - -
1:After {array[0]} second{array[0] === 1 ? '' : 's'} - {#if (random_factor ?? 0) > 0}(+/- {((array[0] ?? 0) * (random_factor ?? 0)) / - 100} - seconds){/if}
{index}: - {delay} second{delay === 1 ? '' : 's'} - {#if (random_factor ?? 0) > 0}(+/- {((delay ?? 0) * (random_factor ?? 0)) / - 100} - seconds){/if} - after attempt #{index - 1} - {#if i > cArray.length - 2} - - ({multiplier} * {eSeconds}{index}) - + {/if} + +
+ {#if true} + {@const { attempts: cAttempts, seconds: cSeconds } = previewRetry?.constant || {}} + {@const { + attempts: eAttempts, + seconds: eSeconds, + multiplier, + random_factor + } = previewRetry?.exponential || {}} + {@const cArray = Array.from({ length: Math.min(cAttempts || 0, 100) }, () => cSeconds)} + {@const eArray = Array.from( + { length: Math.min(eAttempts || 0, 100) }, + (_, i) => (multiplier || 0) * (eSeconds || 0) ** (i + cArray.length + 1) + )} + {@const array = [...cArray, ...eArray]} +
+
Retry attempts
+ {#if array.length > 0} + + + + + + + + + {#each array.slice(1, 100) as delay, i} + {@const index = i + 2} + + + + + {/each} + {#if (cAttempts ?? 0) > 100 || (eAttempts ?? 0) > 100} + + + + {/if} - - - {/each} - {#if (cAttempts ?? 0) > 100 || (eAttempts ?? 0) > 100} - - - - + +
1:After {array[0]} second{array[0] === 1 ? '' : 's'} + {#if (random_factor ?? 0) > 0}(+/- {((array[0] ?? 0) * + (random_factor ?? 0)) / + 100} + seconds){/if}
{index}: + {delay} second{delay === 1 ? '' : 's'} + {#if (random_factor ?? 0) > 0}(+/- {((delay ?? 0) * + (random_factor ?? 0)) / + 100} + seconds){/if} + after attempt #{index - 1} + {#if i > cArray.length - 2} + + ({multiplier} * {eSeconds}{index}) + + {/if} +
......
......
{/if} -
- {/if} +
+ {/if} +
- {/if}
-
{/if}
diff --git a/frontend/src/lib/components/flows/content/FlowRunSettings.svelte b/frontend/src/lib/components/flows/content/FlowRunSettings.svelte new file mode 100644 index 0000000000..9c0c6d7de5 --- /dev/null +++ b/frontend/src/lib/components/flows/content/FlowRunSettings.svelte @@ -0,0 +1,371 @@ + + +{#snippet sectionHeader(title: string)} +
+ {title} +
+{/snippet} + +
+ {#if !isFailure} +
+ {@render sectionHeader('Flow control')} + +
+
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+
+ {/if} + +
+ {@render sectionHeader('Execution policy')} + +
+ {#if !loopSubset} +
+ +
+ +
+ +
+ + + {#if !isFailure} +
+ +
+ + {#if isRawScript || isWorkspaceScript} +
+ {#if flowModule.value.type === 'script'} + + {:else if flowModule.value.type === 'rawscript'} +
+ { + if (flowModule.value.type !== 'rawscript') return + flowModule.value.concurrent_limit = concurrencyOn ? undefined : 1 + }} + options={{ + right: 'Concurrency limit', + rightTooltip: 'Allowed concurrency within a given timeframe.', + rightDocumentationLink: + 'https://www.windmill.dev/docs/flows/concurrency_limit' + }} + /> + {#if concurrencyOn} +
+ + + +
+ {/if} +
+ {/if} +
+ {/if} + +
+ { + if (flowModule.priority !== undefined) { + flowModule.priority = undefined + } else { + flowModule.priority = stepSettingDefaults('priority') + } + }} + options={{ + right: 'High priority', + rightTooltip: + 'Jobs scheduled from this step take precedence over other jobs in the queue when the flow runs.' + }} + /> + {#if flowModule.priority !== undefined} +
+ +
+ {/if} + + {#if isCloudHosted()} + + Setting priority is not available on the cloud. + + {/if} +
+ +
+ +
+ +
+ +
+ {/if} + {/if} + + {#if !isFailure} +
+ +
+ {/if} + + {#if loopSubset} +
+ +
+ {/if} +
+ + {#if s3Language && onApplyS3Snippet && !isFailure} +
+ {@render sectionHeader('S3 snippets')} +

+ Read and write S3 objects, and use Polars or DuckDB to run efficient ETL processes. +

+
+ + {#snippet children({ item })} + {#if s3Language === 'deno'} + + {:else} + + + + {/if} + {/snippet} + + +
+ {#if s3Snippet} +
+ +
+ {/if} +
+ {/if} +
diff --git a/frontend/src/lib/components/flows/content/FlowSettings.svelte b/frontend/src/lib/components/flows/content/FlowSettings.svelte index 51a8ab5253..8e78a04269 100644 --- a/frontend/src/lib/components/flows/content/FlowSettings.svelte +++ b/frontend/src/lib/components/flows/content/FlowSettings.svelte @@ -116,7 +116,7 @@
-
+
diff --git a/frontend/src/lib/components/flows/content/McpToolEditor.svelte b/frontend/src/lib/components/flows/content/McpToolEditor.svelte index b73cc93ef9..027766d5d4 100644 --- a/frontend/src/lib/components/flows/content/McpToolEditor.svelte +++ b/frontend/src/lib/components/flows/content/McpToolEditor.svelte @@ -18,6 +18,7 @@ -
- - {#snippet children()} -

- MCP clients allow AI agents to access and execute a list of tools made available by an MCP - server. -
- Choose an MCP resource to make its tools available to the agent. -
-
- Note: Only HTTP streamable MCP servers are supported. -

- {/snippet} - + +
+ + {#snippet children()} +

+ MCP clients allow AI agents to access and execute a list of tools made available by an MCP + server. +
+ Choose an MCP resource to make its tools available to the agent. +
+
+ Note: Only HTTP streamable MCP servers are supported. +

+ {/snippet} + -
- -
- - {#if !resourcePath} - {#if !showOAuthForm} - - {:else} - (showOAuthForm = false)} - /> - {/if} - {/if} - - {#if resourcePath?.length > 0}
-
-
- {#snippet action()} - - {/snippet} -
- {#if error} -
{`Failed to load tools from MCP server: ${error}`}
- {:else if tools.status === 'loading'} -
-
Loading tools...
-
- {:else if (tools.value ?? []).length === 0 && !error} -
-
- No tools loaded yet. Click "Refresh Tools" to fetch tools from the MCP server. -
-
- {:else if (tools.value ?? []).length > 0} -
-
- {#each tools.value ?? [] as mcpTool} -
- {mcpTool.name} - {#if mcpTool.description} - — {mcpTool.description} - {/if} -
- {/each} -
-
- {/if} -
-
+ {:else} + (showOAuthForm = false)} + /> + {/if} + {/if} - {#if tool.value.include_tools && tool.value.exclude_tools} -
-
-
- -
-
- -
+ {#if resourcePath?.length > 0} +
+ +
+ +
+ {#snippet action()} + + {/snippet} +
+ {#if error} +
{`Failed to load tools from MCP server: ${error}`}
+ {:else if tools.status === 'loading'} +
+
Loading tools...
+
+ {:else if (tools.value ?? []).length === 0 && !error} +
+
+ No tools loaded yet. Click "Refresh Tools" to fetch tools from the MCP server. +
+
+ {:else if (tools.value ?? []).length > 0} +
+
+ {#each tools.value ?? [] as mcpTool} +
+ {mcpTool.name} + {#if mcpTool.description} + — {mcpTool.description} + {/if} +
+ {/each} +
+
+ {/if}
+ + {#if tool.value.include_tools && tool.value.exclude_tools} +
+
+
+ +
+
+ +
+
+
+ {/if} {/if} - {/if} -
+
+ diff --git a/frontend/src/lib/components/flows/content/StepSettingsBadges.svelte b/frontend/src/lib/components/flows/content/StepSettingsBadges.svelte new file mode 100644 index 0000000000..b92a8a05a5 --- /dev/null +++ b/frontend/src/lib/components/flows/content/StepSettingsBadges.svelte @@ -0,0 +1,30 @@ + + +{#if configured.length > 0} +
+ {#each configured as s (s.key)} + {@const Icon = s.icon} + + + {#snippet text()} + {s.tooltip} + · {s.summary.text} + {/snippet} + + {/each} +
+{/if} diff --git a/frontend/src/lib/components/flows/content/SuspendDrawer.svelte b/frontend/src/lib/components/flows/content/SuspendDrawer.svelte index 3ac2afe001..e351c2812c 100644 --- a/frontend/src/lib/components/flows/content/SuspendDrawer.svelte +++ b/frontend/src/lib/components/flows/content/SuspendDrawer.svelte @@ -16,21 +16,26 @@
- To add a form, go to the Form tab, inside the Advanced {'->'} Suspend tab, and add a form. - You can then get back the payloads using `resume` (single approver), or `resumes` (multiple approvers) - in the next step. Forms are an EE feature only. The approver list itself is fetchable using `approvers` + To add a form, open the Form tab of this Suspend until approval/resume setting + and click + Add a form. You can then get back the payloads using `resume` (single approver), or + `resumes` (multiple approvers) in the next step. Forms are an EE feature only. The approver + list itself is fetchable using `approvers`
A prompt is simply an approval step that can be self-approved. To do this, include the diff --git a/frontend/src/lib/components/flows/content/WebsearchToolDisplay.svelte b/frontend/src/lib/components/flows/content/WebsearchToolDisplay.svelte index 77e4d07c61..2d679ff665 100644 --- a/frontend/src/lib/components/flows/content/WebsearchToolDisplay.svelte +++ b/frontend/src/lib/components/flows/content/WebsearchToolDisplay.svelte @@ -1,10 +1,15 @@ -
- - Gives the AI Agent the ability to search the web. Only works for openai, anthropic and google - models for now. - -
+ +
+ + Gives the AI Agent the ability to search the web. Only works for openai, anthropic and google + models for now. + +
+ diff --git a/frontend/src/lib/components/flows/flowDeleteController.test.ts b/frontend/src/lib/components/flows/flowDeleteController.test.ts index b029ce702f..0a37e1ec7c 100644 --- a/frontend/src/lib/components/flows/flowDeleteController.test.ts +++ b/frontend/src/lib/components/flows/flowDeleteController.test.ts @@ -134,7 +134,11 @@ describe('flowDeleteController', () => { expect(flowStore.val.value.modules.map((module) => module.id)).toEqual(['dependent_step']) expect(flowStore.val.value.groups ?? []).toEqual([]) expect(Object.keys(flowStateStore.val)).toEqual(['dependent_step']) - expect(selectionManager.selectId).toHaveBeenCalledWith('dependent_step') + // The surviving step is selected as a side effect of the delete, so it must be + // marked as such — otherwise the modal step panel pops open on its own. + expect(selectionManager.selectId).toHaveBeenCalledWith('dependent_step', { + openPanel: false + }) expect(onDelete).toHaveBeenCalledWith('agent_step') }) }) diff --git a/frontend/src/lib/components/flows/flowDeleteController.ts b/frontend/src/lib/components/flows/flowDeleteController.ts index 5fea6a4cc3..a1933ba080 100644 --- a/frontend/src/lib/components/flows/flowDeleteController.ts +++ b/frontend/src/lib/components/flows/flowDeleteController.ts @@ -56,7 +56,9 @@ export function executeDeletePlan( if (plan.selection.kind === 'clear') { args.selectionManager.clearSelection() } else { - args.selectionManager.selectId(plan.selection.id) + // Whatever remains selected after a delete was not asked for, so it must not + // pop the modal panel open. + args.selectionManager.selectId(plan.selection.id, { openPanel: false }) } if (plan.targets.some((target) => target.kind === 'preprocessor')) { diff --git a/frontend/src/lib/components/flows/flowPanelMode.svelte.ts b/frontend/src/lib/components/flows/flowPanelMode.svelte.ts new file mode 100644 index 0000000000..2d9b505d89 --- /dev/null +++ b/frontend/src/lib/components/flows/flowPanelMode.svelte.ts @@ -0,0 +1,28 @@ +import { resolvePanelMode, type FlowPanelMode, type FlowPanelPreference } from './panelPlacement' + +/** + * Holds the step panel's placement preference and the editor's measured width, and reads + * the resolution off `resolvePanelMode`. The preference is not persisted: it lasts as long + * as the editor is open, so every flow opens on `auto` and a pin is a deliberate act each + * time. + */ +export function useFlowPanelMode(opts: { enabled: () => boolean }) { + let preference = $state('auto') + let width = $state(0) + + return { + get preference(): FlowPanelPreference { + return preference + }, + set preference(next: FlowPanelPreference) { + preference = next + }, + get mode(): FlowPanelMode { + return resolvePanelMode({ enabled: opts.enabled(), preference, width }) + }, + /** Fed by the editor root's measured width; drives `auto` in both directions. */ + measure(measured: number | null | undefined) { + width = measured ?? 0 + } + } +} diff --git a/frontend/src/lib/components/flows/flowStepSettings.test.ts b/frontend/src/lib/components/flows/flowStepSettings.test.ts new file mode 100644 index 0000000000..7ecb138214 --- /dev/null +++ b/frontend/src/lib/components/flows/flowStepSettings.test.ts @@ -0,0 +1,140 @@ +import { describe, it, expect } from 'vitest' +import type { FlowModule } from '$lib/gen' +import { describeStepSettings, hasInlineConcurrency } from './flowStepSettings' + +const stepSettingsByKey = (...args: Parameters) => + Object.fromEntries(describeStepSettings(...args).map((v) => [v.key, v])) + +function step(overrides: Partial = {}): FlowModule { + return { + id: 'a', + value: { type: 'rawscript', language: 'bun', content: '', input_transforms: {} }, + ...overrides + } as FlowModule +} + +describe('describeStepSettings', () => { + it('reports an untouched step as configured nowhere', () => { + expect(describeStepSettings(step()).filter((s) => s.configured)).toEqual([]) + }) + + it('treats a sleep of 0 as configured and says so, rather than claiming it is off', () => { + // Switching Sleep on seeds `{ value: 0 }`; the row must not contradict the toggle. + const s = stepSettingsByKey(step({ sleep: { type: 'static', value: 0 } }))['sleep'] + expect(s?.configured).toBe(true) + expect(s?.summary.text).toBe('0s after') + expect(s?.summary.state).toBe('configured') + }) + + it('does not let an empty stop_after_if mask a configured stop_after_all_iters_if', () => { + // Both can be set on a sequential loop. + const s = stepSettingsByKey( + step({ + stop_after_if: { expr: '', skip_if_stopped: false }, + stop_after_all_iters_if: { expr: 'result.done', skip_if_stopped: false } + }) + )['early-stop'] + expect(s?.configured).toBe(true) + expect(s?.summary.text).toBe('result.done') + }) + + it('describes retries with zero attempts instead of reporting None', () => { + const s = stepSettingsByKey(step({ retry: { constant: { attempts: 0, seconds: 5 } } }))[ + 'retries' + ] + expect(s?.configured).toBe(true) + expect(s?.summary.text).toBe('0 attempts, constant') + }) + + it('reads concurrency and cache from the referenced script for workspace-script steps', () => { + const mod = step({ value: { type: 'script', path: 'u/me/s', input_transforms: {} } as any }) + const off = stepSettingsByKey(mod) + expect(off['concurrency']?.configured).toBe(false) + + const on = stepSettingsByKey(mod, { concurrent_limit: 3, cache_ttl: 60 }) + expect(on['concurrency']?.configured).toBe(true) + expect(on['concurrency']?.summary.text).toBe('Max 3') + expect(on['cache']?.configured).toBe(true) + }) + + it("prefers the module's own cache_ttl over the referenced script's, like the worker", () => { + const mod = step({ + cache_ttl: 3600, + value: { type: 'script', path: 'u/me/s', input_transforms: {} } as any + }) + // No referenced settings loaded yet (the graph badges never load them), so a + // module-level TTL has to stand on its own here. + expect(stepSettingsByKey(mod)['cache']?.configured).toBe(true) + expect(stepSettingsByKey(mod, { cache_ttl: 60 })['cache']?.summary.text).toBe('1 h') + }) + + it('reports a non-positive inline concurrency as invalid, not as unset', () => { + const s = stepSettingsByKey( + step({ + cache_ttl: -1, + value: { + type: 'rawscript', + language: 'bun', + content: '', + input_transforms: {}, + concurrent_limit: -1 + } + } as Partial) + ) + // `configured` means the step carries the config, so a value the user set counts + // even when the runtime ignores it — the summary is what says it is a no-op. + expect(s['concurrency']?.configured).toBe(true) + expect(s['concurrency']?.summary).toMatchObject({ text: 'Invalid limit', state: 'invalid' }) + expect(s['cache']?.configured).toBe(true) + expect(s['cache']?.summary).toMatchObject({ text: 'No TTL set', state: 'invalid' }) + }) + + it('treats a cleared concurrency input as present, not as unset', () => { + // Emptying a number input binds `null`, not `undefined`. Reading that as unset is + // what disabled the field the user was editing, so presence must be strict. + const mod = step({ + value: { + type: 'rawscript', + language: 'bun', + content: '', + input_transforms: {}, + concurrent_limit: null + } + } as unknown as Partial) + // Presence keeps the setting editor's controls live while the field is empty. + expect(hasInlineConcurrency(mod)).toBe(true) + expect(stepSettingsByKey(mod)['concurrency']?.summary).toMatchObject({ + text: 'Invalid limit', + state: 'invalid' + }) + }) + + it('omits settings that do not apply to the step type', () => { + const subflow = step({ value: { type: 'flow', path: 'u/me/f' } as any }) + expect(describeStepSettings(subflow).some((s) => s.key === 'concurrency')).toBe(false) + expect(describeStepSettings(step()).some((s) => s.key === 'concurrency')).toBe(true) + }) + + it('marks an invalid retry config as invalid rather than configured', () => { + const s = stepSettingsByKey( + step({ retry: { exponential: { attempts: 2, multiplier: 1, seconds: -1 } } }) + )['retries'] + expect(s?.summary.state).toBe('invalid') + }) + + it('labels early stop for trigger steps by what it means there', () => { + const trigger = step({ + value: { + type: 'rawscript', + language: 'bun', + content: '', + input_transforms: {}, + is_trigger: true + } as any + }) + expect(stepSettingsByKey(trigger)['early-stop']?.tooltip).toBe( + 'Stop early if there are no new events' + ) + expect(stepSettingsByKey(step())['early-stop']?.tooltip).toBe('Early stop / break') + }) +}) diff --git a/frontend/src/lib/components/flows/flowStepSettings.ts b/frontend/src/lib/components/flows/flowStepSettings.ts new file mode 100644 index 0000000000..9c995a93a9 --- /dev/null +++ b/frontend/src/lib/components/flows/flowStepSettings.ts @@ -0,0 +1,362 @@ +import { + ChevronsUp, + CircleStop, + Combine, + Database, + Gauge, + Hand, + Moon, + RefreshCw, + ShieldAlert, + SkipForward, + Timer, + Trash2 +} from 'lucide-svelte' +import type { FlowModule } from '$lib/gen' +import type { ScriptAdvancedSettingsFields } from '$lib/components/scriptSettings' +import { validateRetryConfig } from '$lib/utils' + +// Single source for the per-step runtime settings: which apply, which are configured, +// and how each reads back. Graph badges, the run-settings accordion and the setting +// editors all read it here so they cannot drift. Script-level twin: `scriptSettings.ts`. + +export type StepSettingKey = + | 'skip' + | 'early-stop' + | 'suspend' + | 'sleep' + | 'retries' + | 'error-handling' + | 'timeout' + | 'concurrency' + | 'priority' + | 'cache' + | 'debounce' + | 'lifetime' + +export type StepSettingSummary = { + text: string + state: 'configured' | 'default' | 'invalid' + /** Render the text as code (it is a user-written expression). */ + mono?: boolean +} + +export type StepSettingView = { + key: StepSettingKey + label: string + /** Longer wording for hover surfaces; falls back to `label`. */ + tooltip: string + icon: any + /** The setting's config is present on the step. Deliberately not "the runtime + * would behave differently" — a configured setting can still be a no-op + * (a sleep of 0), and `summary` says so rather than claiming it is off. */ + configured: boolean + summary: StepSettingSummary +} + +/** A step whose script polls an external system and returns the new items. */ +export function isTriggerStep(module: FlowModule | undefined): boolean { + return ( + module?.value != undefined && + (module.value.type === 'script' || module.value.type === 'rawscript') && + module.value.is_trigger === true + ) +} + +const def = (text: string): StepSettingSummary => ({ text, state: 'default' }) +const inv = (text: string): StepSettingSummary => ({ text, state: 'invalid' }) +const cfg = (text: string, mono = false): StepSettingSummary => ({ + text, + state: 'configured', + mono +}) + +function formatDur(s: number | undefined): string { + if (s == null) return '' + if (s < 60) return `${s}s` + if (s < 3600) return `${Math.round(s / 60)} min` + return `${Math.round(s / 3600)} h` +} + +/** Describe a user-written predicate. An empty expression is still configured — + * the setting is on, it just has nothing to evaluate yet. */ +function exprSummary(expr: string | undefined): StepSettingSummary { + const e = expr?.trim() + if (!e) return cfg('No expression') + return e.length <= 24 ? cfg(e, true) : cfg('Expression set') +} + +type Ctx = { referenced?: ScriptAdvancedSettingsFields } + +type SettingSpec = { + label: string + tooltip?: (mod: FlowModule) => string + icon: any + applies?: (mod: FlowModule) => boolean + configured: (mod: FlowModule, ctx: Ctx) => boolean + summarize: (mod: FlowModule, ctx: Ctx) => StepSettingSummary +} + +// A value the step itself carries counts as configured even when the runtime would +// ignore it — `summary` is what reports the no-op, and the setting editors keep their +// controls live on presence so clearing a field mid-edit can't disable it. Only values +// read off a referenced workspace script use the runtime's `> 0` test: nobody is typing +// into those, and 0 there simply means the script has no limit. +const isWorkspaceScript = (mod: FlowModule) => mod.value.type === 'script' +const inlineConcurrentLimit = (mod: FlowModule) => + mod.value.type === 'rawscript' ? mod.value.concurrent_limit : undefined +const effectiveCacheTtl = (mod: FlowModule, ctx: Ctx) => + mod.cache_ttl ?? (isWorkspaceScript(mod) ? ctx.referenced?.cache_ttl : undefined) + +/** Whether an inline step carries a concurrency limit at all. The setting editor keeps its + * controls live on presence, so clearing the field mid-edit can't disable the input. A + * present-but-non-positive limit is surfaced as invalid rather than silently as "None". + * Strict: an emptied number input binds to `null`, which is still a value being typed. */ +export function hasInlineConcurrency(mod: FlowModule): boolean { + return inlineConcurrentLimit(mod) !== undefined +} + +/** Canonical order — every surface lists settings in this sequence. */ +const SPECS: { key: StepSettingKey; spec: SettingSpec }[] = [ + { + key: 'skip', + spec: { + label: 'Skip if', + icon: SkipForward, + configured: (m) => Boolean(m.skip_if), + summarize: (m) => (m.skip_if ? exprSummary(m.skip_if.expr) : def('Off')) + } + }, + { + key: 'early-stop', + spec: { + label: 'Early stop / break', + tooltip: (m) => + isTriggerStep(m) ? 'Stop early if there are no new events' : 'Early stop / break', + icon: CircleStop, + configured: (m) => m.stop_after_if != undefined || m.stop_after_all_iters_if != undefined, + summarize: (m) => { + // Both can be set on a sequential loop, so pick the first that carries an + // expression instead of letting an empty one mask the other. + const exprs = [m.stop_after_if?.expr, m.stop_after_all_iters_if?.expr].filter( + (e) => e != undefined + ) + if (exprs.length === 0) return def('Off') + return exprSummary(exprs.find((e) => e?.trim()) ?? exprs[0]) + } + } + }, + { + key: 'suspend', + spec: { + label: 'Suspend until approval/resume', + icon: Hand, + configured: (m) => Boolean(m.suspend), + summarize: (m) => { + if (!m.suspend) return def('Off') + const n = m.suspend.required_events ?? 1 + return cfg(`${n} approval${n > 1 ? 's' : ''}`) + } + } + }, + { + key: 'sleep', + spec: { + label: 'Sleep', + icon: Moon, + configured: (m) => Boolean(m.sleep), + summarize: (m) => { + const s = m.sleep + if (!s) return def('Off') + if (s.type === 'static') { + const v = Number(s.value) + return Number.isFinite(v) ? cfg(`${formatDur(v)} after`) : cfg('Dynamic') + } + return cfg('Dynamic') + } + } + }, + { + key: 'retries', + spec: { + label: 'Retries', + icon: RefreshCw, + configured: (m) => m.retry?.constant != undefined || m.retry?.exponential != undefined, + summarize: (m) => { + const r = m.retry + if (r?.constant == undefined && r?.exponential == undefined) return def('None') + if (validateRetryConfig(r)) return { text: 'Invalid', state: 'invalid' } + const isConstant = r?.constant != undefined + const n = (isConstant ? r?.constant?.attempts : r?.exponential?.attempts) ?? 0 + const kind = isConstant ? 'constant' : 'exponential' + return cfg(`${n} attempt${n === 1 ? '' : 's'}, ${kind}`) + } + } + }, + { + key: 'error-handling', + spec: { + label: 'Error handling', + icon: ShieldAlert, + configured: (m) => Boolean(m.continue_on_error), + summarize: (m) => (m.continue_on_error ? cfg('Continue on error') : def('Off')) + } + }, + { + key: 'timeout', + spec: { + label: 'Timeout', + icon: Timer, + configured: (m) => m.timeout != null, + summarize: (m) => { + const t = m.timeout + if (t == null) return def('None') + if (typeof t === 'number') return cfg(formatDur(t)) + if (t.type === 'static') { + const v = Number(t.value) + return Number.isFinite(v) ? cfg(formatDur(v)) : cfg('Dynamic') + } + return cfg('Dynamic') + } + } + }, + { + key: 'concurrency', + spec: { + label: 'Concurrency limit', + icon: Gauge, + applies: (m) => m.value.type === 'rawscript' || m.value.type === 'script', + // Presence for the step's own limit (the user may be mid-edit, and `summary` says + // when it is a no-op), effectiveness for the referenced script's — a remote + // value nobody is typing into, where 0 just means the script has no limit. + configured: (m, ctx) => + isWorkspaceScript(m) + ? ctx.referenced?.concurrent_limit != undefined && ctx.referenced.concurrent_limit > 0 + : hasInlineConcurrency(m), + summarize: (m, ctx) => { + if (isWorkspaceScript(m)) { + const l = ctx.referenced?.concurrent_limit + return l != undefined && l > 0 ? cfg(`Max ${l}`) : def('None') + } + const l = inlineConcurrentLimit(m) + if (l === undefined) return def('None') + if (!(l > 0)) return inv('Invalid limit') + const key = m.value.type === 'rawscript' ? m.value.custom_concurrency_key : undefined + return cfg(`Max ${l}${key ? ' per key' : ''}`) + } + } + }, + { + key: 'priority', + spec: { + label: 'Priority', + icon: ChevronsUp, + configured: (m) => m.priority !== undefined, + summarize: (m) => { + if (m.priority === undefined) return def('Off') + // 0 is how the runtime spells "no priority". + return m.priority > 0 ? cfg('High priority') : inv('No priority set') + } + } + }, + { + key: 'cache', + spec: { + label: 'Cache results', + icon: Database, + // The worker takes the module's cache_ttl over the referenced script's, so a + // module-level TTL must show here even for a workspace-script step. + configured: (m, ctx) => + m.cache_ttl !== undefined || (isWorkspaceScript(m) && (ctx.referenced?.cache_ttl ?? 0) > 0), + summarize: (m, ctx) => { + const ttl = effectiveCacheTtl(m, ctx) + if (ttl === undefined) return def('Off') + return ttl > 0 ? cfg(formatDur(ttl)) : inv('No TTL set') + } + } + }, + { + key: 'debounce', + spec: { + label: 'Debounce', + icon: Combine, + configured: (m) => m.debouncing?.debounce_delay_s !== undefined, + summarize: (m) => { + const d = m.debouncing?.debounce_delay_s + if (d === undefined) return def('Off') + return d > 0 ? cfg(`${formatDur(d)} debounce`) : inv('No delay set') + } + } + }, + { + key: 'lifetime', + spec: { + label: 'Lifetime', + icon: Trash2, + configured: (m) => m.delete_after_secs != null, + summarize: (m) => { + const s = m.delete_after_secs + if (s == null) return def('Off') + return s === 0 ? cfg('Delete now') : cfg(`Delete after ${formatDur(s)}`) + } + } + } +] + +/** The settings that apply to this step, in canonical order. + * `referenced` supplies the workspace script's own settings for `script` steps, + * whose concurrency and cache live on the script rather than on the step. */ +export function describeStepSettings( + mod: FlowModule, + referenced?: ScriptAdvancedSettingsFields +): StepSettingView[] { + const ctx: Ctx = { referenced } + return SPECS.filter(({ spec }) => spec.applies?.(mod) ?? true).map(({ key, spec }) => ({ + key, + label: spec.label, + tooltip: spec.tooltip?.(mod) ?? spec.label, + icon: spec.icon, + configured: spec.configured(mod, ctx), + summary: spec.summarize(mod, ctx) + })) +} + +/** How a trigger step decides it has nothing to process. Stored on the step at + * creation, so changing it only affects newly created steps. */ +// Falsy-or-empty, not `result == undefined`: a trigger returning nothing new may say so +// with any empty value, and stopping is always the right response. +const TRIGGER_STOP_EXPR = '!result || (Array.isArray(result) && result.length == 0)' + +/** The config a setting is seeded with when it is switched on. Read by the setting + * editors and by every path that creates a step, so both agree. Settings absent from + * this map have no seeded config (the editor writes the value directly). */ +const DEFAULTS = { + skip: () => ({ expr: 'false' }), + 'early-stop': (kind?: 'trigger' | 'end') => + kind === 'trigger' + ? { expr: TRIGGER_STOP_EXPR, skip_if_stopped: true } + : kind === 'end' + ? { expr: 'true', skip_if_stopped: false } + : { + expr: 'result == undefined', + skip_if_stopped: false, + error_message: undefined, + error_include_result: false + }, + suspend: () => ({ required_events: 1, timeout: 1800 }), + sleep: () => ({ type: 'static' as const, value: 0 }), + cache: () => 600, + lifetime: () => 0, + priority: () => 100 +} satisfies Partial unknown>> + +export type SeededSettingKey = keyof typeof DEFAULTS + +/** Seeded config for a setting. Typed per key, so an unhandled key is a compile + * error rather than a silent `undefined`. */ +export function stepSettingDefaults( + key: K, + kind?: 'trigger' | 'end' +): ReturnType<(typeof DEFAULTS)[K]> { + return DEFAULTS[key](kind) as ReturnType<(typeof DEFAULTS)[K]> +} diff --git a/frontend/src/lib/components/flows/header/FlowPreviewButtons.svelte b/frontend/src/lib/components/flows/header/FlowPreviewButtons.svelte index dbebc82f70..0e92ee8a5e 100644 --- a/frontend/src/lib/components/flows/header/FlowPreviewButtons.svelte +++ b/frontend/src/lib/components/flows/header/FlowPreviewButtons.svelte @@ -169,7 +169,12 @@ {#if !loading} - + @@ -103,10 +110,7 @@ title="Delete failure script" type="button" class="ml-1" - onclick={() => { - flowStore.val.value.failure_module = undefined - selectionManager.selectId('settings-metadata') - }} + onclick={deleteFailureModule} > @@ -117,10 +121,7 @@ title="Delete failure script" type="button" class="absolute -top-1.5 -right-1.5 rounded-full bg-surface border border-border p-0.5 hover:bg-surface-hover" - onclick={() => { - flowStore.val.value.failure_module = undefined - selectionManager.selectId('settings-metadata') - }} + onclick={deleteFailureModule} > diff --git a/frontend/src/lib/components/flows/map/FlowModuleSchemaItem.svelte b/frontend/src/lib/components/flows/map/FlowModuleSchemaItem.svelte index de949a6dd9..6aeaa14f35 100644 --- a/frontend/src/lib/components/flows/map/FlowModuleSchemaItem.svelte +++ b/frontend/src/lib/components/flows/map/FlowModuleSchemaItem.svelte @@ -4,23 +4,8 @@ import Popover from '$lib/components/Popover.svelte' import DropdownV2 from '$lib/components/DropdownV2.svelte' import { classNames, type Item, type StateStore } from '$lib/utils' - import { - Bed, - Database, - Gauge, - EllipsisVertical, - PhoneIncoming, - Repeat, - Square, - SkipForward, - Pin, - X, - Play, - Loader2, - TriangleAlert, - Timer, - Maximize2 - } from 'lucide-svelte' + import { EllipsisVertical, Pin, X, Play, Loader2, TriangleAlert, Maximize2 } from 'lucide-svelte' + import type { StepSettingView } from '../flowStepSettings' import { createEventDispatcher, getContext, untrack } from 'svelte' import { fade } from 'svelte/transition' import type { FlowEditorContext } from '../types' @@ -55,12 +40,8 @@ selected?: boolean deletable?: boolean moduleAction: ModuleActionInfo | undefined - retry?: boolean - cache?: boolean - earlyStop?: boolean - skip?: boolean - suspend?: boolean - sleep?: boolean + /** Configured settings to badge, from flowStepSettings. */ + settings?: StepSettingView[] mock?: | { enabled?: boolean @@ -72,12 +53,8 @@ label: string path?: string nodeState?: FlowNodeState - concurrency?: boolean - // TODO: Implement for this one. See how concurrency is implemented. - debouncing?: boolean retries?: number | undefined warningMessage?: string | undefined - isTrigger?: boolean editMode?: boolean alwaysShowOutputPicker?: boolean loopStatus?: { type: 'inside' | 'self'; flow: 'forloopflow' | 'whileloopflow' } | undefined @@ -97,23 +74,15 @@ selected = false, deletable = false, moduleAction = undefined, - retry = false, - cache = false, - earlyStop = false, - skip = false, - suspend = false, - sleep = false, + settings = [], mock = { enabled: false }, bold = false, id = undefined, label, path = '', nodeState, - concurrency = false, - debouncing = false, retries = undefined, warningMessage = undefined, - isTrigger = false, editMode = false, alwaysShowOutputPicker = false, loopStatus = undefined, @@ -304,112 +273,23 @@ class="absolute text-sm right-2 flex flex-row gap-1 z-10 transition-all duration-100" style={`bottom: ${outputPickerBarOpen ? '-38px' : '-12px'}`} > - {#if retry} + {#each settings as s (s.key)} + {@const Icon = s.icon}
- {#if retries}{retries}{/if} - + {#if s.key === 'retries' && retries}{retries}{/if} +
{#snippet text()} - Retries + {s.tooltip} + · {s.summary.text} {/snippet} - {/if} - - {#if concurrency} - -
- -
- {#snippet text()} - Concurrency Limits - {/snippet} - - {/if} - {#if debouncing} - -
- -
- {#snippet text()} - Debouncing - {/snippet} - - {/if} - {#if cache} - -
- -
- {#snippet text()} - Cached - {/snippet} - - {/if} - {#if earlyStop} - -
- -
- {#snippet text()} - {isTrigger ? 'Stop early if there are no new events' : 'Early stop/break'} - {/snippet} - - {/if} - {#if skip} - -
- -
- {#snippet text()} - Skip - {/snippet} - - {/if} - {#if suspend} - -
- -
- {#snippet text()} - Suspend - {/snippet} - - {/if} - {#if sleep} - -
- -
- {#snippet text()} - Sleep - {/snippet} - - {/if} + {/each} {#if mock?.enabled} - {/snippet} - {#snippet content({ close })} - { - close() - }} - on:new={(e) => { - data.eventHandlers.insert({ - index: -1, // ignored when agentId is set - agentId: data.agentModuleId, - ...e.detail - }) - close() - }} - on:insert={(e) => { - data.eventHandlers.insert({ - index: -1, // ignored when agentId is set - agentId: data.agentModuleId, - ...e.detail - }) - close() - }} - on:pickScript={(e) => { - data.eventHandlers.insert({ - index: -1, // ignored when agentId is set - agentId: data.agentModuleId, - kind: e.detail.kind, - script: { - ...e.detail, - summary: e.detail.summary - ? e.detail.summary.replace(/\s/, '_').replace(/[^a-zA-Z0-9_]/g, '') - : e.detail.path.split('/').pop() - } - }) - close() - }} - on:pickMcpTool={(e) => { - data.eventHandlers.insert({ - index: -1, - agentId: data.agentModuleId, - kind: 'mcpTool' - }) - close() - }} - on:pickWebsearchTool={(e) => { - data.eventHandlers.insert({ - index: -1, - agentId: data.agentModuleId, - kind: 'websearchTool' - }) - close() - }} - on:pickAiAgentTool={(e) => { - data.eventHandlers.insert({ - index: -1, - agentId: data.agentModuleId, - kind: 'aiAgentTool' - }) - close() - }} - /> - {/snippet} - + + + {#snippet trigger()} + + {/snippet} + {#snippet content({ close })} + { + close() + }} + on:new={(e) => { + data.eventHandlers.insert({ + index: -1, // ignored when agentId is set + agentId: data.agentModuleId, + ...e.detail + }) + close() + }} + on:insert={(e) => { + data.eventHandlers.insert({ + index: -1, // ignored when agentId is set + agentId: data.agentModuleId, + ...e.detail + }) + close() + }} + on:pickScript={(e) => { + data.eventHandlers.insert({ + index: -1, // ignored when agentId is set + agentId: data.agentModuleId, + kind: e.detail.kind, + script: { + ...e.detail, + summary: e.detail.summary + ? e.detail.summary.replace(/\s/, '_').replace(/[^a-zA-Z0-9_]/g, '') + : e.detail.path.split('/').pop() + } + }) + close() + }} + on:pickMcpTool={(e) => { + data.eventHandlers.insert({ + index: -1, + agentId: data.agentModuleId, + kind: 'mcpTool' + }) + close() + }} + on:pickWebsearchTool={(e) => { + data.eventHandlers.insert({ + index: -1, + agentId: data.agentModuleId, + kind: 'websearchTool' + }) + close() + }} + on:pickAiAgentTool={(e) => { + data.eventHandlers.insert({ + index: -1, + agentId: data.agentModuleId, + kind: 'aiAgentTool' + }) + close() + }} + /> + {/snippet} + {/if} diff --git a/frontend/src/lib/components/graph/renderers/nodes/TriggersNode.svelte b/frontend/src/lib/components/graph/renderers/nodes/TriggersNode.svelte index 58f95d4b33..43af1fef48 100644 --- a/frontend/src/lib/components/graph/renderers/nodes/TriggersNode.svelte +++ b/frontend/src/lib/components/graph/renderers/nodes/TriggersNode.svelte @@ -84,13 +84,15 @@ }} on:select={() => data?.eventHandlers?.select('Trigger')} onSelect={async (triggerIndex: number) => { - data?.eventHandlers?.select('Trigger') + data?.eventHandlers?.select('Trigger', { openPanel: true }) await tick() triggersState.selectedTriggerIndex = triggerIndex }} onAddDraftTrigger={async (type: TriggerType) => { const newTrigger = triggersState.addDraftTrigger(triggersCount, type) - data?.eventHandlers?.select('Trigger') + // A scheduled poll continues in the trigger-script picker that opens + // alongside this, so revealing the panel would cover it. + data?.eventHandlers?.select('Trigger', { openPanel: type !== 'poll' }) await tick() triggersState.selectedTriggerIndex = newTrigger }} diff --git a/frontend/src/lib/components/graph/renderers/triggers/TriggersWrapper.svelte b/frontend/src/lib/components/graph/renderers/triggers/TriggersWrapper.svelte index 4857729539..3755b5cf77 100644 --- a/frontend/src/lib/components/graph/renderers/triggers/TriggersWrapper.svelte +++ b/frontend/src/lib/components/graph/renderers/triggers/TriggersWrapper.svelte @@ -1,6 +1,7 @@
@@ -101,25 +113,35 @@
-{#if showTriggerScriptPicker} - -
- { - showTriggerScriptPicker = false - dispatch('new', e.detail) - }} - on:pickScript={(e) => { - showTriggerScriptPicker = false - dispatch('pickScript', e.detail) - }} - kind="trigger" - /> -
- -{/if} + + + {#snippet children()} + {#if showTriggerScriptPicker} + + +
(showTriggerScriptPicker = false) }} + > + { + showTriggerScriptPicker = false + dispatch('new', e.detail) + }} + on:pickScript={(e) => { + showTriggerScriptPicker = false + dispatch('pickScript', e.detail) + }} + kind="trigger" + /> +
+ + {/if} + {/snippet} + diff --git a/frontend/src/lib/components/graph/selectionUtils.svelte.ts b/frontend/src/lib/components/graph/selectionUtils.svelte.ts index daa9de3ecf..eddbae4fca 100644 --- a/frontend/src/lib/components/graph/selectionUtils.svelte.ts +++ b/frontend/src/lib/components/graph/selectionUtils.svelte.ts @@ -1,9 +1,34 @@ import type { Node } from '@xyflow/svelte' +/** Intent attached to a `selectId` call. `true` opens the panel even for ids that + * would not normally trigger it; `false` marks an incidental selection (what remains + * after a delete) and keeps it shut; omitted uses the default rules. */ +export type SelectIntentOptions = { + openPanel?: boolean +} + +/** Panels reached from toolbar buttons or dedicated graph nodes rather than step + * modules. They open on a single selection; step modules deliberately do not. */ +const FLOW_LEVEL_PANEL_IDS = new Set([ + 'constants', + 'failure', + 'preprocessor', + 'Input', + 'Result', + 'Trigger' +]) + +export function isFlowLevelPanelTarget(id: string): boolean { + // 'settings-' prefixed, not 'settings' prefixed: step ids are user-editable, so a + // step renamed settings_v2 must not be mistaken for the flow's settings panel. + return id === 'settings' || id.startsWith('settings-') || FLOW_LEVEL_PANEL_IDS.has(id) +} + export class SelectionManager { #selectedNodes = $state([]) #selectionMode = $state<'normal' | 'rect-select'>('normal') #clearGraphSelection: () => void = () => {} + #onSelectIntent: ((id: string, opts?: SelectIntentOptions) => void) | undefined = undefined constructor() {} @@ -11,7 +36,15 @@ export class SelectionManager { this.#clearGraphSelection = clearGraphSelection } - selectId(id: string) { + /** Fires on every `selectId` call, BEFORE the same-id dedup early-return — so a + * consumer can react even when the id is re-selected (e.g. clicking the already + * selected "Settings" toolbar button to re-open a modal panel). */ + setOnSelectIntent(cb: ((id: string, opts?: SelectIntentOptions) => void) | undefined) { + this.#onSelectIntent = cb + } + + selectId(id: string, opts?: SelectIntentOptions) { + this.#onSelectIntent?.(id, opts) if (this.#selectedNodes.length === 1 && this.#selectedNodes[0].id === id) { return } @@ -77,6 +110,12 @@ export class SelectionManager { return } + // Before the same-id early return, like `selectId`: re-selecting an already + // selected node must still be able to reopen its panel. + if (nodes.length === 1) { + this.#onSelectIntent?.(nodes[0].id) + } + // If the new selection is the same as the current selection, do nothing const newIds = nodes.map((n) => n.id).join(',') const currentIds = this.#selectedNodes.map((n) => n.id).join(',') diff --git a/frontend/src/lib/components/home/ItemsList.svelte b/frontend/src/lib/components/home/ItemsList.svelte index 42f63054af..52d53242be 100644 --- a/frontend/src/lib/components/home/ItemsList.svelte +++ b/frontend/src/lib/components/home/ItemsList.svelte @@ -1727,6 +1727,7 @@ on:rawAppChanged={reloadItemsAndCounts} on:reload={reloadItemsAndCounts} {showCode} + showEditButton={showEditButtons} /> {/key} {:else} diff --git a/frontend/src/lib/components/home/TreeView.svelte b/frontend/src/lib/components/home/TreeView.svelte index 4b3d39d05d..cf34998bf2 100644 --- a/frontend/src/lib/components/home/TreeView.svelte +++ b/frontend/src/lib/components/home/TreeView.svelte @@ -33,6 +33,7 @@ // Position of this node among the rendered root nodes; "expand all" only // auto-loads the first EXPAND_ALL_LOAD_LIMIT of them (see the effect below). rootIndex?: number + showEditButton?: boolean // Path prefix of the parent node, so this one can name its own (`ownerLoad` and // the listing endpoint are both keyed by full prefix). Unset at the top level. parentPrefix?: string @@ -54,6 +55,7 @@ onExpandOwner, onCollapseOwner, rootIndex = 0, + showEditButton = true, parentPrefix, ancestorHasMore = false }: Props = $props() @@ -309,6 +311,7 @@ on:rawAppChanged on:reload {showCode} + {showEditButton} depth={depth + 1} /> {/each} @@ -373,6 +376,7 @@ onExpandOwner?: (owner: string, more?: boolean) => void onCollapseOwner?: (owner: string) => void + showEditButton?: boolean } let { @@ -55,7 +56,8 @@ selfUsername, ownerLoad, onExpandOwner, - onCollapseOwner + onCollapseOwner, + showEditButton = true }: Props = $props() // How many root nodes render at once. A root node is a collapsed owner row that @@ -205,6 +207,7 @@ on:rawAppChanged on:reload {showCode} + {showEditButton} /> {/if} {/each} diff --git a/frontend/src/lib/components/meltComponents/Popover.svelte b/frontend/src/lib/components/meltComponents/Popover.svelte index 629e86a658..f850e53f6c 100644 --- a/frontend/src/lib/components/meltComponents/Popover.svelte +++ b/frontend/src/lib/components/meltComponents/Popover.svelte @@ -280,8 +280,10 @@ fullScreen ? `${fullScreenHost ? 'absolute' : 'fixed'} !top-1/2 !left-1/2 !-translate-x-1/2 !-translate-y-1/2 !resize-none` : 'w-fit', - contentClasses, - `z-[5001]` + // Last so `contentClasses` can raise it: a popover inside a ConfirmationModal has to + // clear that modal's own z-index, which sits above this layer. + `z-[5001]`, + contentClasses )} data-popover {...extraProps} diff --git a/frontend/src/lib/components/prop_picker.ts b/frontend/src/lib/components/prop_picker.ts index ced6a5e890..729d60409c 100644 --- a/frontend/src/lib/components/prop_picker.ts +++ b/frontend/src/lib/components/prop_picker.ts @@ -9,4 +9,7 @@ export type FlowPropPickerConfig = { export type PropPickerContext = { flowPropPickerConfig: Writable pickablePropertiesFiltered: Writable + /** True when the panel is a modal, which covers the graph. Connecting there could never + * be completed by clicking a step node, so the graph stays out of it. */ + inModalPanel?: () => boolean } diff --git a/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte b/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte index 926328751c..58329fac6c 100644 --- a/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte +++ b/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte @@ -72,7 +72,7 @@ import { AIBtnClasses } from '../copilot/chat/AIButtonStyle' import { stripRawAppDiffNoise } from './utils' import type { RawAppData } from './dataTableRefUtils' - import { buildForkEditUrl, editInForkAllowed, editInForkLabel } from '$lib/utils/editInFork' + import { editInForkAllowed, editInForkLabel, openEditInFork } from '$lib/utils/editInFork' import { isCloudHosted } from '$lib/cloud' // async function hash(message) { @@ -892,7 +892,7 @@ { label: editInForkLabel(opWorkspace, $userWorkspaces), onClick: () => { - window.open(buildForkEditUrl('raw_app', appPath)) + openEditInFork('raw_app', appPath, opWorkspace) } } ] diff --git a/frontend/src/lib/components/recording/FlowRecordingReplay.svelte b/frontend/src/lib/components/recording/FlowRecordingReplay.svelte index 3d677d8f54..26b48788a5 100644 --- a/frontend/src/lib/components/recording/FlowRecordingReplay.svelte +++ b/frontend/src/lib/components/recording/FlowRecordingReplay.svelte @@ -255,7 +255,6 @@ {#if job} > } - let { job, workspaceId, isOwner, suspendStatus }: Props = $props() + let { job, isOwner, suspendStatus }: Props = $props() const isWaitingForEvents = $derived( job?.flow_status?.modules?.[job?.flow_status?.step]?.type === 'WaitingForEvents' @@ -28,7 +27,7 @@ transition:slide={{ duration: 150 }} > {#if isWaitingForEvents} - + {:else if isSuspended}
{#each Object.values(suspendStatus.val) as suspendCount (suspendCount.job.id)} @@ -36,7 +35,11 @@
Flow suspended, waiting for {suspendCount.nb} events
- +
{/each}
diff --git a/frontend/src/lib/components/runs/JobRunsPreview.svelte b/frontend/src/lib/components/runs/JobRunsPreview.svelte index 4e45b18f06..ae61ca2d8e 100644 --- a/frontend/src/lib/components/runs/JobRunsPreview.svelte +++ b/frontend/src/lib/components/runs/JobRunsPreview.svelte @@ -173,7 +173,6 @@ {#if isFlow} void + /** Render only the editing drawer: the caller shows its own view of the schema and + * opens the drawer through `openDrawer()`. */ + drawerOnly?: boolean } let { schema = $bindable(), jsonView = $bindable(false), - hiddenArgs = undefined + hiddenArgs = undefined, + drawerOnly = false }: Props = $props() + export function openDrawer() { + schemaFormDrawer?.openDrawer() + } + // let schema = $state(structuredClone($state.snapshot(schema))) let schemaString: string = $state(JSON.stringify(schema, null, '\t')) @@ -82,108 +91,7 @@ const rnd = generateRandomString() -
- { - if (jsonView) { - schemaString = JSON.stringify(schema, null, '\t') - editor?.setCode(schemaString) - } - }} - bind:schema - bind:this={addPropertyComponent} - /> - - { - schemaString = JSON.stringify(schema, null, '\t') - editor?.setCode(schemaString) - }} - /> -
- -{#if !jsonView} - {#key rnd} -
- {#if items?.length > 0} - {#each items as item (item.id)} - -
- {#if schema.properties?.[item.value]} -
- {`${item.value}${ - schema.properties?.[item.value]?.title - ? ` (title: ${schema.properties?.[item.value]?.title})` - : '' - } `} - - -
-
-
- - {#if schema.properties[item.value]?.type === 'object' && !(schema.properties[item.value].oneOf && schema.properties[item.value].oneOf.length >= 2)} -
- -
- {/if} - {:else} -
Value is undefined
- {/if} -
- {/each} - {/if} -
- {/key} +{#snippet editorDrawer()} {#snippet children()} schemaFormDrawer?.closeDrawer()}> @@ -192,11 +100,8 @@ bind:this={editableSchemaForm} bind:schema isAppInput - on:edit={(e) => { - addPropertyComponent?.openDrawer(e.detail) - }} on:delete={(e) => { - addPropertyComponent?.handleDeleteArgument([e.detail]) + ;(addPropertyComponent ?? drawerAddProperty)?.handleDeleteArgument([e.detail]) }} {hiddenArgs} editTab="inputEditor" @@ -204,6 +109,7 @@ {#snippet addProperty()} { editableSchemaForm?.openField(argName) }} @@ -221,29 +127,147 @@ {/snippet} +{/snippet} + +{#if drawerOnly} + {@render editorDrawer()} {:else} -
- { - try { - schema = JSON.parse(schemaString) - error = '' - } catch (err) { - error = err.message - } +
+ { + schemaString = JSON.stringify(schema, null, '\t') + editor?.setCode(schemaString) }} - bind:code={schemaString} - lang="json" - autoHeight - automaticLayout />
- {#if !emptyString(error)} -
{error}
+ + {#if !jsonView} + {#key rnd} +
+ {#if items?.length > 0} + {#each items as item (item.id)} + +
+ {#if schema.properties?.[item.value]} +
+ {`${item.value}${ + schema.properties?.[item.value]?.title + ? ` (title: ${schema.properties?.[item.value]?.title})` + : '' + } `} + + +
+
+
+ + {#if schema.properties[item.value]?.type === 'object' && !(schema.properties[item.value].oneOf && schema.properties[item.value].oneOf.length >= 2)} +
+ +
+ {/if} + {:else} +
Value is undefined
+ {/if} +
+ {/each} + {/if} +
+ {/key} + + { + if (jsonView) { + schemaString = JSON.stringify(schema, null, '\t') + editor?.setCode(schemaString) + } + }} + > + {#snippet trigger()} +
+ +
+ {/snippet} + + + {@render editorDrawer()} {:else} -

+
+ { + try { + schema = JSON.parse(schemaString) + error = '' + } catch (err) { + error = err.message + } + }} + bind:code={schemaString} + lang="json" + autoHeight + automaticLayout + /> +
+ {#if !emptyString(error)} +
{error}
+ {:else} +

+ {/if} {/if} {/if} diff --git a/frontend/src/lib/components/triggers/TriggersEditor.svelte b/frontend/src/lib/components/triggers/TriggersEditor.svelte index 44f565d086..4b7733ac33 100644 --- a/frontend/src/lib/components/triggers/TriggersEditor.svelte +++ b/frontend/src/lib/components/triggers/TriggersEditor.svelte @@ -364,7 +364,7 @@
{/if} - +
diff --git a/frontend/src/lib/components/vscode.ts b/frontend/src/lib/components/vscode.ts index f30ffcd780..7ec61be145 100644 --- a/frontend/src/lib/components/vscode.ts +++ b/frontend/src/lib/components/vscode.ts @@ -1,6 +1,6 @@ import '@codingame/monaco-vscode-standalone-typescript-language-features' -import { editor as meditor, Uri as mUri } from 'monaco-editor' +import { editor as meditor, KeyCode, KeyMod, Uri as mUri } from 'monaco-editor' import { getAppliedDarkModeVariant } from '$lib/darkModeVariant' export let isInitialized = false @@ -157,6 +157,15 @@ export async function initializeVscode(caller?: string, htmlContainer?: HTMLElem await apiWrapper.start() isInitialized = true + + // vscode-api ships VS Code's keybindings, including Ctrl/Cmd+Shift+S + // (Save As) which has no meaning here. Left bound, every Monaco + // instance swallows the shortcut and the browser/OS one never fires. + meditor.addKeybindingRule({ + keybinding: KeyMod.CtrlCmd | KeyMod.Shift | KeyCode.KeyS, + command: null + }) + meditor.defineTheme('nord', { base: 'vs-dark', inherit: true, diff --git a/frontend/src/lib/transitions.ts b/frontend/src/lib/transitions.ts new file mode 100644 index 0000000000..d1d5812951 --- /dev/null +++ b/frontend/src/lib/transitions.ts @@ -0,0 +1,44 @@ +import { cubicOut } from 'svelte/easing' +import type { EasingFunction, TransitionConfig } from 'svelte/transition' + +/** `slide`, but re-measuring the content on every frame. + * + * `slide` snapshots the height once when the transition starts, so content that settles + * after mount (a Monaco editor sizing itself to its lines) animates towards a stale + * target and snaps to its real height at the end. */ +export function slideDynamic( + node: HTMLElement, + { + delay = 0, + duration = 150, + easing = cubicOut + }: { delay?: number; duration?: number; easing?: EasingFunction } = {} +): TransitionConfig { + const style = getComputedStyle(node) + const paddingTop = parseFloat(style.paddingTop) + const paddingBottom = parseFloat(style.paddingBottom) + const initial = { + overflow: node.style.overflow, + height: node.style.height, + paddingTop: node.style.paddingTop, + paddingBottom: node.style.paddingBottom + } + return { + delay, + duration, + easing, + tick: (t: number) => { + if (t === 1) { + Object.assign(node.style, initial) + return + } + node.style.overflow = 'hidden' + // Padding shrinks with the box, or `border-box` would floor the height at it. + node.style.paddingTop = `${t * paddingTop}px` + node.style.paddingBottom = `${t * paddingBottom}px` + // scrollHeight ignores the height clamp but does count the padding just written. + const content = node.scrollHeight - t * (paddingTop + paddingBottom) + node.style.height = `${t * (content + paddingTop + paddingBottom)}px` + } + } +} diff --git a/frontend/src/lib/utils/editInFork.ts b/frontend/src/lib/utils/editInFork.ts index c446732b7d..4e0344471f 100644 --- a/frontend/src/lib/utils/editInFork.ts +++ b/frontend/src/lib/utils/editInFork.ts @@ -9,8 +9,12 @@ import { } from '$lib/stores' import { findCanonicalDevWorkspace } from '$lib/utils/workspaceHierarchy' import { isRuleActive, canUserBypassRuleKind } from '$lib/workspaceProtectionRules.svelte' +import { goto } from '$lib/navigation' +import { sendUserToast } from '$lib/toast' +import { checkItemExists } from '$lib/utils_workspace_deploy' +import { updateDevWorkspaceModal } from '$lib/utils/editInForkModal.svelte' -type ItemType = 'script' | 'flow' | 'app' | 'raw_app' +export type ItemType = 'script' | 'flow' | 'app' | 'raw_app' /** * Whether to show the "edit in fork / dev workspace" affordance. Allowed when forking isn't disabled, @@ -64,26 +68,164 @@ function editPathFor(itemType: ItemType, itemPath: string): string { } } -function viewPathFor(itemType: ItemType, itemPath: string): string { - switch (itemType) { - case 'script': - return `${base}/scripts/get/${itemPath}` - case 'flow': - return `${base}/flows/get/${itemPath}` - case 'app': - return `${base}/apps/get/${itemPath}` - case 'raw_app': - return `${base}/apps_raw/get/${itemPath}` +export function buildForkEditUrl(itemType: ItemType, itemPath: string): string { + // When the current ("prod") workspace has a canonical dev workspace, edits are funneled there. + const dev = findCanonicalDevWorkspace(get(workspaceStore), get(userWorkspaces)) + return dev + ? devWorkspaceEditUrl(itemType, itemPath, dev.id) + : forkWorkspaceUrl(itemType, itemPath) +} + +/** Fork-creation flow, coming back to the item's editor once the fork exists. */ +export function forkWorkspaceUrl(itemType: ItemType, itemPath: string): string { + return `${base}/user/fork_workspace?rd=${encodeURIComponent(editPathFor(itemType, itemPath))}` +} + +/** The item's editor in the dev workspace — the target `buildForkEditUrl` produces when a dev exists. */ +export function devWorkspaceEditUrl( + itemType: ItemType, + itemPath: string, + devWorkspaceId: string +): string { + // `?workspace=` switches the workspace store (handled in the logged layout), so the editor + // opens against the dev workspace rather than whichever one the tab was on. + return `${editPathFor(itemType, itemPath)}?workspace=${encodeURIComponent(devWorkspaceId)}` +} + +/** + * A dev workspace can be behind its prod, so the URL built at render time dead-ends on a not-found + * page for any item prod has and dev doesn't. Resolve the destination at click time instead: + * return it when the item is there, else raise the prompt offering to update the dev workspace with + * it and return undefined. Shared by the row buttons and the editors' "Edit in " dropdown + * entries. + */ +let latestResolve = 0 + +async function resolveEditInForkTarget( + itemType: ItemType, + itemPath: string, + prod: string, + dev: UserWorkspace, + openInNewTab = false +): Promise { + const seq = ++latestResolve + const from = { path: window.location.pathname, workspace: get(workspaceStore) } + let exists: boolean + try { + exists = await checkItemExists(itemType, itemPath, dev.id) + } catch { + // Inconclusive — go anyway and let the editor report whatever is actually wrong. + exists = true + } + // Only act if the user is still where they asked from. A later click supersedes this one, and + // navigating or switching workspace abandons it — the modal is layout-global and `goto` is + // unconditional, so a late answer would otherwise hijack whatever they moved on to. + if (seq !== latestResolve) return undefined + if (window.location.pathname !== from.path || get(workspaceStore) !== from.workspace) + return undefined + if (exists) return devWorkspaceEditUrl(itemType, itemPath, dev.id) + updateDevWorkspaceModal.val = { + itemType, + itemPath, + devWorkspaceId: dev.id, + devWorkspaceName: dev.name, + prodWorkspaceId: prod, + openInNewTab + } + return undefined +} + +function currentDevWorkspace( + prodWorkspace?: string +): { prod: string; dev: UserWorkspace } | undefined { + const prod = prodWorkspace ?? get(workspaceStore) + const dev = findCanonicalDevWorkspace(prod, get(userWorkspaces)) + if (!dev || !prod) return undefined + return { prod, dev } +} + +/** + * Click handler for the "Edit in " affordance. Menu entries carry no href — the + * destination is only known after an async probe — so this navigates itself by default. Link + * callers pass `hasHref` so modifier/middle clicks still open the raw href in a new tab, and so the + * no-dev-workspace case is left to the anchor rather than being navigated twice. + */ +export async function onEditInForkClick( + e: Event | undefined, + itemType: ItemType, + itemPath: string, + { hasHref = false }: { hasHref?: boolean } = {} +): Promise { + const click = e as MouseEvent | undefined + if ( + hasHref && + (click?.ctrlKey || click?.metaKey || click?.shiftKey || click?.altKey || click?.button) + ) + return + const target = currentDevWorkspace() + if (!target) { + // Nothing to probe: the destination is the fork-creation flow, which the anchor already points at. + if (!hasHref) await goto(forkWorkspaceUrl(itemType, itemPath)) + return + } + e?.preventDefault() + const url = await resolveEditInForkTarget(itemType, itemPath, target.prod, target.dev) + if (url) await goto(url) +} + +export type ClaimedTab = { show: (url: string) => void; discard: () => void } + +/** + * Take a tab now, to point somewhere once an async step resolves. Safari refuses `window.open` from + * any promise continuation however fast it resolves, so a tab opened after an `await` never appears + * there — it has to be claimed inside the click's own transient activation. `discard` releases it + * when the answer turns out to be "nowhere to go". Returns undefined if the popup was blocked, which + * leaves the caller to decide between a late `window.open` and saying so. + */ +export function claimTab(): ClaimedTab | undefined { + const tab = window.open('about:blank') + if (!tab) return undefined + return { + show: (url: string) => { + tab.location.href = url + }, + discard: () => tab.close() } } -export function buildForkEditUrl(itemType: ItemType, itemPath: string): string { - // When the current ("prod") workspace has a canonical dev workspace, edits are funneled there: - // land on the item's page in the dev workspace (not straight in the editor) so the workspace - // switch is legible and the user opens the editor deliberately from there. - const dev = findCanonicalDevWorkspace(get(workspaceStore), get(userWorkspaces)) - if (dev) { - return `${viewPathFor(itemType, itemPath)}?workspace=${encodeURIComponent(dev.id)}` +/** + * "Edit in " from an editor's dropdown, which opens a new tab rather than navigating + * away from work in progress. `prodWorkspace` is the workspace the editor is operating on, which in + * a session pane is not the one the navigation store holds — pass the same value the surrounding + * `editInForkAllowed` / `editInForkLabel` are given, so the action can't resolve against a different + * workspace than the label above it names. + */ +export async function openEditInFork( + itemType: ItemType, + itemPath: string, + prodWorkspace?: string +): Promise { + const target = currentDevWorkspace(prodWorkspace) + if (!target) { + // No dev workspace to probe for: the destination is the fork-creation flow. + if (!window.open(forkWorkspaceUrl(itemType, itemPath))) { + sendUserToast('Allow popups to fork this workspace', true) + } + return + } + // Navigating in place would throw away whatever this editor is holding — the whole reason this + // entry opens a tab. The cost is a blank tab that flashes and closes when the item turns out to + // be missing and the prompt takes over; the prompt then opens its own tab on confirm. + const tab = claimTab() + const url = await resolveEditInForkTarget(itemType, itemPath, target.prod, target.dev, true) + if (!url) { + // Superseded, abandoned, or answered by the prompt in the original tab — nothing to show. + tab?.discard() + return + } + if (tab) { + tab.show(url) + } else if (!window.open(url)) { + sendUserToast(`Allow popups to open ${itemPath} in ${target.dev.name}`, true) } - return `${base}/user/fork_workspace?rd=${encodeURIComponent(editPathFor(itemType, itemPath))}` } diff --git a/frontend/src/lib/utils/editInForkModal.svelte.ts b/frontend/src/lib/utils/editInForkModal.svelte.ts new file mode 100644 index 0000000000..a331ba23ab --- /dev/null +++ b/frontend/src/lib/utils/editInForkModal.svelte.ts @@ -0,0 +1,28 @@ +import { createState } from '$lib/svelte5Utils.svelte' +import type { StateStore } from '$lib/utils' +import type { ItemType } from './editInFork' + +/** + * An "Edit in " click that landed on an item the dev workspace + * doesn't have yet. Held globally so the confirmation renders once in the logged + * layout instead of per item row. + */ +export type UpdateDevWorkspaceModalState = { + itemType: ItemType + itemPath: string + devWorkspaceId: string + devWorkspaceName: string + prodWorkspaceId: string + /** + * The click that raised this came from an editor's dropdown, which opens a tab rather than + * navigating away from work in progress. Answering it has to keep that promise: without this + * the "item is present" branch opens a tab while the "item is missing" branch — this prompt — + * would leave the editor the user was told would be preserved. + */ + openInNewTab?: boolean +} + +export let updateDevWorkspaceModal: StateStore = + createState({ + val: undefined + }) diff --git a/frontend/src/lib/utils_workspace_deploy.test.ts b/frontend/src/lib/utils_workspace_deploy.test.ts index 41ea2d1af0..08a877d012 100644 --- a/frontend/src/lib/utils_workspace_deploy.test.ts +++ b/frontend/src/lib/utils_workspace_deploy.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest' import { + checkPathWritePermission, diffActionableInDirection, diffCreatesInTarget, diffRemovesInTarget @@ -68,3 +69,50 @@ describe('deploy direction of a one-sided diff row', () => { expect(diffRemovesInTarget(bothSides, false)).toBe(false) }) }) + +describe('per-item write permission in the deploy target', () => { + const member = { is_admin: false, username: 'alice', folders: ['shared'] } + const never = async () => { + throw new Error('folder probe should not run') + } + + it('lets a workspace admin write anywhere', async () => { + const admin = { is_admin: true, username: 'root', folders: [] } + expect(await checkPathWritePermission('dev', 'u/someone/x', admin, never)).toEqual({ ok: true }) + expect(await checkPathWritePermission('dev', 'f/locked/x', admin, never)).toEqual({ ok: true }) + }) + + it('allows a user their own path and refuses someone else’s', async () => { + expect(await checkPathWritePermission('dev', 'u/alice/x', member, never)).toEqual({ ok: true }) + const refused = await checkPathWritePermission('dev', 'u/bob/x', member, never) + expect(refused.ok).toBe(false) + expect(refused.reason).toContain('u/bob') + }) + + it('allows a folder in the write set without probing for it', async () => { + expect(await checkPathWritePermission('dev', 'f/shared/x', member, never)).toEqual({ ok: true }) + }) + + it('refuses a folder that exists in the target but is not writable', async () => { + const refused = await checkPathWritePermission('dev', 'f/locked/x', member, async () => true) + expect(refused.ok).toBe(false) + expect(refused.reason).toContain('locked') + }) + + // The two fail-open paths. Turning either into a refusal would block a deploy the server + // would have accepted, so they are asserted rather than left to the `catch` reading as dead. + it('allows a folder the target does not have yet, since the deploy creates it', async () => { + expect( + await checkPathWritePermission('dev', 'f/brand_new/x', member, async () => false) + ).toEqual({ ok: true }) + }) + + it('allows when the folder probe itself fails', async () => { + const probeFailed = async () => { + throw new Error('network') + } + expect(await checkPathWritePermission('dev', 'f/locked/x', member, probeFailed)).toEqual({ + ok: true + }) + }) +}) diff --git a/frontend/src/lib/utils_workspace_deploy.ts b/frontend/src/lib/utils_workspace_deploy.ts index b9a397a32f..c330a2210b 100644 --- a/frontend/src/lib/utils_workspace_deploy.ts +++ b/frontend/src/lib/utils_workspace_deploy.ts @@ -14,10 +14,12 @@ import { ScheduleService, ScriptService, SqsTriggerService, + GroupService, UserService, VariableService, WebsocketTriggerService, - WorkspaceService + WorkspaceService, + type User } from '$lib/gen' import { fetchProtectionRulesForWorkspace, @@ -167,6 +169,16 @@ function legacyTriggerKind(kind: TriggerDeployKind) { return map[kind] } +/** An identity in both formats the app policy stores it in. */ +export type AppIdentity = { email: string; permissionedAs: string } + +/** + * Set when a create-only deploy was refused because the target turned out to already have the + * item. Carried on an object rather than matched out of the error text: `deployItem` swallows + * every throw into `{ success: false, error }`, so the flag is the only reliable signal. + */ +export type DeployConflict = { hit: boolean } + /** * `deployItem` overrides only the email half of the identity, while the body it builds * spreads the *source* item — which carries the source workspace's permissioned_as, valid @@ -176,11 +188,26 @@ function legacyTriggerKind(kind: TriggerDeployKind) { * clears it too, but this app consumes the published package, so the clear has to exist * on both sides until that version ships. */ -function makeProvider(onBehalfOfPrincipal?: string): DeployProvider { +function makeProvider( + onBehalfOfPrincipal?: string, + appIdentity?: AppIdentity, + /** + * Refuse the writes the shared `deployItem` reaches for only when the item already exists in + * the target, turning its silent switch to an update into a failure the caller can act on. + * The three below are exactly its `alreadyExists` branches: a flow and an app are replaced + * outright, and a script is given the target's head as `parent_hash`, which is what makes an + * otherwise identical `createScript` an update. + */ + conflict?: DeployConflict +): DeployProvider { const withPermissionedAs = >(requestBody: T): T => ({ ...requestBody, on_behalf_of: onBehalfOfPrincipal }) + const refuseUpdate = (): never => { + if (conflict) conflict.hit = true + throw new Error('item already exists in the target workspace') + } return { existsFlowByPath: (p) => FlowService.existsFlowByPath(p), existsScriptByPath: (p) => ScriptService.existsScriptByPath(p), @@ -193,17 +220,36 @@ function makeProvider(onBehalfOfPrincipal?: string): DeployProvider { createFlow: (p) => FlowService.createFlow({ ...p, requestBody: withPermissionedAs(p.requestBody) }), updateFlow: (p) => - FlowService.updateFlow({ ...p, requestBody: withPermissionedAs(p.requestBody) }), + conflict + ? refuseUpdate() + : FlowService.updateFlow({ ...p, requestBody: withPermissionedAs(p.requestBody) }), archiveFlowByPath: (p) => FlowService.archiveFlowByPath(p), getScriptByPath: (p) => ScriptService.getScriptByPath(p), createScript: (p) => - ScriptService.createScript({ ...p, requestBody: withPermissionedAs(p.requestBody) }), + conflict && p.requestBody.parent_hash + ? refuseUpdate() + : ScriptService.createScript({ ...p, requestBody: withPermissionedAs(p.requestBody) }), archiveScriptByPath: (p) => ScriptService.archiveScriptByPath(p), - getAppByPath: (p) => AppService.getAppByPath(p), + // An app's identity lives in its policy, and the shared deploy forwards the source policy + // untouched — it only turns `onBehalfOf` into `preserve_on_behalf_of: true`. Rewriting the + // policy on the way out is therefore the only way a chosen identity reaches the target; the + // backend honours it (`should_preserve` requires `policy.on_behalf_of.is_some()`). + getAppByPath: async (p) => { + const app = await AppService.getAppByPath(p) + if (!appIdentity) return app + return { + ...app, + policy: { + ...app.policy, + on_behalf_of: appIdentity.permissionedAs, + on_behalf_of_email: appIdentity.email + } + } + }, createApp: (p) => AppService.createApp(p), - updateApp: (p) => AppService.updateApp(p), + updateApp: (p) => (conflict ? refuseUpdate() : AppService.updateApp(p)), createAppRaw: (p) => AppService.createAppRaw(p), - updateAppRaw: (p) => AppService.updateAppRaw(p), + updateAppRaw: (p) => (conflict ? refuseUpdate() : AppService.updateAppRaw(p)), getPublicSecretOfLatestVersionOfApp: (p) => AppService.getPublicSecretOfLatestVersionOfApp(p), getRawAppData: (p) => AppService.getRawAppData(p), deleteApp: (p) => AppService.deleteApp(p), @@ -284,12 +330,19 @@ export interface DeployItemParams { */ onBehalfOf?: string /** - * Authorization half of `onBehalfOf` for flows/scripts (u/username or g/group). - * Must name the same identity as `onBehalfOf`. Set it only when the user picked a - * specific user; undefined clears the key, leaving the backend to derive the target - * workspace's own principal from `onBehalfOf`. + * Authorization half of `onBehalfOf` (u/username or g/group). Must name the same identity as + * `onBehalfOf`. Set it only when the user picked a specific user; undefined clears the key, + * leaving the backend to derive the target workspace's own principal from `onBehalfOf`. Apps + * additionally need it in the policy, which holds both formats — see `makeProvider`. */ onBehalfOfPrincipal?: string + /** + * Fail instead of overwriting when the target turns out to already have the item. The shared + * deploy re-probes and silently switches to an update, so a caller that only means to create — + * one acting on the item being absent — has to say so or it will overwrite whoever got there + * between the two probes. The result then carries `conflict`. + */ + createOnly?: boolean } /** @@ -297,7 +350,9 @@ export interface DeployItemParams { * `DeployKind` union plus the legacy generic `'trigger'` from `DeployWorkspace.svelte`, * which carries its sub-kind in `additionalInformation`. */ -export async function deployItem(params: DeployItemParams): Promise { +export async function deployItem( + params: DeployItemParams +): Promise { const { kind, path, @@ -305,7 +360,8 @@ export async function deployItem(params: DeployItemParams): Promise { + const provider = makeProvider() + if (kind === 'flow') return (await provider.getFlowByPath({ workspace, path })).on_behalf_of_email + if (kind === 'script') + return (await provider.getScriptByPath({ workspace, path })).on_behalf_of_email + return (await provider.getAppByPath({ workspace, path })).policy?.on_behalf_of_email +} + +/** + * Every workspace group, not just the first page. + * + * `listGroupNames` would be the obvious call but unions in instance groups, which folder rules do + * not resolve against — a same-named instance group would let an unusable rule through. `listGroups` + * reads the workspace's own `group_` rows, which is what the server checks, but it paginates: a + * group missed here reads as "no account in the target", which now refuses a folder copy outright. + */ +async function workspaceGroupNames(workspace: string): Promise> { + const PER_PAGE = 1000 + const names = new Set() + // Stops on a short page; the size check is the backstop for a server that ignores `page`. + for (let page = 1; page <= 50; page++) { + const batch = await GroupService.listGroups({ workspace, page, perPage: PER_PAGE }) + const before = names.size + batch.forEach((g) => names.add(g.name)) + if (batch.length < PER_PAGE || names.size === before) break + } + return names +} + +/** + * Resolve a source-workspace principal into the same person or group as the target names them. + * + * A `u/` is workspace-local: the same username in the target can be a different account, + * so copying one verbatim can hand a folder — or an item's execution identity — to a namesake. Email + * is the only identifier stable across workspaces, so users go source username -> email -> target + * username, and anyone without an account there resolves to undefined for the caller to deal with. + */ +async function principalTranslator(workspaceFrom: string, workspaceTo: string) { + const [fromUsers, toUsers, targetGroups] = await Promise.all([ + // `list_users` is unpaginated, unlike the group listing below. + UserService.listUsers({ workspace: workspaceFrom }), + UserService.listUsers({ workspace: workspaceTo }), + workspaceGroupNames(workspaceTo) + ]) + const emailOfSourceUsername = new Map(fromUsers.map((u) => [u.username, u.email])) + const targetUsernameOfEmail = new Map(toUsers.map((u) => [u.email, u.username])) + + /** The same principal as `workspaceTo` names it, or undefined when it has no account there. */ + return (principal: string): string | undefined => { + if (principal.startsWith('u/')) { + const email = emailOfSourceUsername.get(principal.slice(2)) + const username = email ? targetUsernameOfEmail.get(email) : undefined + return username ? `u/${username}` : undefined + } + if (principal.startsWith('g/')) { + return targetGroups.has(principal.slice(2)) ? principal : undefined + } + // An email is already workspace-independent; it only has to name someone there. + return targetUsernameOfEmail.has(principal) ? principal : undefined + } +} + +export type CreateFolderResult = DeployResult & { + /** Access dropped because its principal has no account in the target, if any. */ + droppedAccess?: string[] +} + +/** + * Copy a folder into `workspaceTo`, creating it and never updating it. + * + * `deployItem` re-probes and switches to `updateFolder` when the folder turns out to exist, which + * would replace its owners and ACL with the source's. For a folder the user asked to deploy that is + * the point; for one created on their behalf to give an item somewhere to land it would silently + * rewrite the permissions of a folder someone else just created. Losing that race is success here — + * the folder exists, which is all the caller needed. + * + * Every principal is translated into the target's own naming (see `principalTranslator`), and the + * two kinds of unresolvable principal are treated differently because they fail differently: + * + * - an **owner or ACL entry** with no account in the target is dropped. The folder ends up more + * restrictive than its source, never less, and `create_folder` makes the caller an owner, so + * nobody is locked out of what they just created. + * - an **identity rule** with no account in the target refuses the whole copy. Dropping it would + * leave the folder applying no rule where the source applied one, so an item landing inside runs + * as whoever deployed it — the silent substitution this prompt exists to prevent — and carrying + * it verbatim is worse still: the server validates a rule's shape at folder-create time but its + * principal's existence at item-create time, so the folder would be created and then reject + * every deploy into it, including the retry. + * + * `default_permissioned_as` and `labels` are carried at all, which the shared folder deploy drops. + */ +export async function createFolderIfAbsent( + name: string, + workspaceFrom: string, + workspaceTo: string +): Promise { + try { + const folder = await FolderService.getFolder({ workspace: workspaceFrom, name }) + const rules = folder.default_permissioned_as ?? [] + const owners = folder.owners ?? [] + const acl = Object.entries((folder.extra_perms ?? {}) as Record) + const translate = await principalTranslator(workspaceFrom, workspaceTo) + + const unresolvableRule = rules.map((r) => r.permissioned_as).find((p) => !translate(p)) + if (unresolvableRule) { + return { + success: false, + error: + `f/${name} runs items on behalf of ${unresolvableRule}, which has no account in ` + + `the target workspace. Bring the folder across from the compare page first.` + } + } + + const droppedAccess = [...owners, ...acl.map(([p]) => p)].filter((p) => !translate(p)) + await FolderService.createFolder({ + workspace: workspaceTo, + requestBody: { + name, + owners: owners.map(translate).filter((p): p is string => !!p), + extra_perms: Object.fromEntries( + acl.flatMap(([p, write]) => { + const t = translate(p) + return t ? [[t, write] as const] : [] + }) + ), + summary: folder.summary ?? undefined, + default_permissioned_as: rules.map((r) => ({ + ...r, + permissioned_as: translate(r.permissioned_as)! + })), + labels: folder.labels + } + }) + return { success: true, droppedAccess: droppedAccess.length ? droppedAccess : undefined } + } catch (e) { + // The name conflict a concurrent create produces is not part of the API contract, so ask + // again rather than matching its message. + try { + if (await checkItemExists('folder', `f/${name}`, workspaceTo)) return { success: true } + } catch {} + return { success: false, error: `${e}` } + } +} + export type DeployPermission = { ok: boolean; reason?: string } /** @@ -501,9 +716,13 @@ export type DeployPermission = { ok: boolean; reason?: string } * Fails open on any error — the server still enforces on the actual deploy. * Shared by the session dock and the compare page so both gate identically. */ -export async function checkDeployPermission(workspace: string): Promise { +export async function checkDeployPermission( + workspace: string, + /** Pre-fetched `whoami` for `workspace`, to save a round trip when the caller already has one. */ + whoami?: User +): Promise { try { - const me = await UserService.whoami({ workspace }) + const me = whoami ?? (await UserService.whoami({ workspace })) if (me.operator) { return { ok: false, reason: "You're an operator in this workspace — operators can't deploy" } } @@ -526,3 +745,75 @@ export async function checkDeployPermission(workspace: string): Promise, + folderExists: (folderPath: string) => Promise = (folderPath) => + checkItemExists('folder', folderPath, workspace) +): Promise { + if (me.is_admin) return { ok: true } + const owner = path.match(/^u\/([^/]+)\//)?.[1] + if (owner) { + return owner === me.username + ? { ok: true } + : { + ok: false, + reason: `${path} is owned by u/${owner} — only they or a workspace admin can write there` + } + } + const folder = path.match(/^f\/([^/]+)\//)?.[1] + if (!folder || me.folders?.includes(folder)) return { ok: true } + try { + // A folder the target doesn't have yet is created by the deploy, with the deployer as its + // owner — lacking write access to something that doesn't exist isn't a refusal. + if (!(await folderExists(`f/${folder}`))) return { ok: true } + } catch { + // Inconclusive: let the deploy decide rather than refusing on a failed probe. + return { ok: true } + } + return { ok: false, reason: `You don't have write access to folder ${folder}` } +} + +export type DeployTargetAccess = { + permission: DeployPermission + /** Whether the user may hand the item an identity other than their own. */ + canPreserveOnBehalfOf: boolean + /** The caller as `workspace` knows them — usernames are per-workspace, emails are not. */ + me?: AppIdentity +} + +/** + * What the target workspace says about landing one item in it: the workspace-level gate, write + * access to the item's path, and whether another identity may be preserved. Bundled so one `whoami` + * answers all of it, and so a refusal is known before the deploy rather than as a 403 on confirm. + */ +export async function checkItemDeployAccess( + workspace: string, + path: string +): Promise { + let me: User + try { + me = await UserService.whoami({ workspace }) + } catch { + return { permission: { ok: true }, canPreserveOnBehalfOf: false } + } + const workspaceLevel = await checkDeployPermission(workspace, me) + return { + permission: workspaceLevel.ok + ? await checkPathWritePermission(workspace, path, me) + : workspaceLevel, + canPreserveOnBehalfOf: me.is_admin || (me.groups ?? []).includes('wm_deployers'), + me: { email: me.email, permissionedAs: `u/${me.username}` } + } +} diff --git a/frontend/src/routes/(root)/(logged)/+layout.svelte b/frontend/src/routes/(root)/(logged)/+layout.svelte index 2781d130ec..a88184ed62 100644 --- a/frontend/src/routes/(root)/(logged)/+layout.svelte +++ b/frontend/src/routes/(root)/(logged)/+layout.svelte @@ -20,6 +20,7 @@ import { SIDEBAR_BG, SIDEBAR_BG_DARK } from '$lib/components/sidebar/sidebarChrome' import CriticalAlertModal from '$lib/components/sidebar/CriticalAlertModal.svelte' import ForkConflictModal from '$lib/components/ForkConflictModal.svelte' + import UpdateDevWorkspaceModal from '$lib/components/UpdateDevWorkspaceModal.svelte' import { enterpriseLicense, isPremiumStore, @@ -1400,6 +1401,8 @@ + + onEditInForkClick(e, 'flow', flow.path, { hasHref: true }), unifiedSize: 'md', variant: !showEditButtons ? 'default' : 'subtle', - startIcon: GitFork + startIcon: Pen } }) } diff --git a/frontend/src/routes/(root)/(logged)/forks/compare/+page.svelte b/frontend/src/routes/(root)/(logged)/forks/compare/+page.svelte index f542a79d99..d5ff28ccca 100644 --- a/frontend/src/routes/(root)/(logged)/forks/compare/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/forks/compare/+page.svelte @@ -67,7 +67,8 @@ // merged toggle (CompareModeToggle, rendered inside each card) reports its // selection here; the page only swaps which comparison component is shown. // `?dir=update` opens on the other one, for callers that already know which - // direction has something in it (the fork banner's CTA). + // direction has something in it (the fork banner's CTA, the "not in the dev + // workspace yet" prompt). let forkDirection = $state<'deploy_to' | 'update'>( page.url.searchParams.get('dir') === 'update' ? 'update' : 'deploy_to' ) @@ -458,6 +459,7 @@ {draftKeys} {chatMask} {chatMaskReady} + maskAppliesToUpdate={urlItemsMask !== undefined} onChanged={refreshCounts} onModeSelected={selectMode} /> diff --git a/frontend/src/routes/(root)/(logged)/run/[...run]/+page.svelte b/frontend/src/routes/(root)/(logged)/run/[...run]/+page.svelte index 397394a14e..faa019253c 100644 --- a/frontend/src/routes/(root)/(logged)/run/[...run]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/run/[...run]/+page.svelte @@ -37,7 +37,6 @@ Code2, ClipboardCopy, GitBranch, - GitFork, EllipsisVertical, Share2 } from 'lucide-svelte' @@ -104,7 +103,12 @@ import { useNestedRestartState } from '$lib/components/useNestedRestartState.svelte' import JobOtelTraces from '$lib/components/JobOtelTraces.svelte' import { isRuleActive } from '$lib/workspaceProtectionRules.svelte' - import { buildForkEditUrl, editInForkAllowed, editInForkLabel } from '$lib/utils/editInFork' + import { + buildForkEditUrl, + editInForkAllowed, + editInForkLabel, + onEditInForkClick + } from '$lib/utils/editInFork' import { isCloudHosted } from '$lib/cloud' let job: (Job & { result?: any; result_stream?: string }) | undefined = $state() let jobUpdateLastFetch: Date | undefined = $state() @@ -892,11 +896,12 @@ {#if !showEditButton && !isCloudHosted() && editInForkAllowed($workspaceStore, $userWorkspaces)} {editInForkLabel($workspaceStore, $userWorkspaces)} {/if} {/if} @@ -961,7 +966,6 @@ {job} {isOwner} {suspendStatus} - workspaceId={job?.workspace_id} innerModules={job?.flow_status?.modules} /> {/if} diff --git a/frontend/src/routes/(root)/(logged)/scripts/get/[...hash]/+page.svelte b/frontend/src/routes/(root)/(logged)/scripts/get/[...hash]/+page.svelte index bb8a38dde6..04c16c0acb 100644 --- a/frontend/src/routes/(root)/(logged)/scripts/get/[...hash]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/scripts/get/[...hash]/+page.svelte @@ -91,7 +91,12 @@ import TriggersEditor from '$lib/components/triggers/TriggersEditor.svelte' import { Triggers } from '$lib/components/triggers/triggers.svelte' import { page } from '$app/state' - import { buildForkEditUrl, editInForkAllowed, editInForkLabel } from '$lib/utils/editInFork' + import { + buildForkEditUrl, + editInForkAllowed, + editInForkLabel, + onEditInForkClick + } from '$lib/utils/editInFork' import { isCloudHosted } from '$lib/cloud' import { isWorkflowAsCode } from '$lib/components/graph/wacToFlow' import WacDiagram from '$lib/components/graph/WacDiagram.svelte' @@ -465,9 +470,10 @@ label: editInForkLabel($workspaceStore, $userWorkspaces), buttonProps: { href: buildForkEditUrl('script', script.path), + onClick: (e: Event | undefined) => onEditInForkClick(e, 'script', script.path, { hasHref: true }), unifiedSize: 'md', variant: !showEditButtons ? 'default' : 'subtle', - startIcon: GitFork + startIcon: Pen } }) } diff --git a/frontend/src/routes/(root)/+layout.svelte b/frontend/src/routes/(root)/+layout.svelte index 3db5efcf60..6fbffda21d 100644 --- a/frontend/src/routes/(root)/+layout.svelte +++ b/frontend/src/routes/(root)/+layout.svelte @@ -40,10 +40,22 @@ "Client got disposed and can't be restarted." ] + // The only load of the workspace list for the whole session, and an empty `$userWorkspaces` + // degrades silently rather than erroring — the edit-in-dev affordance, the no-direct-deploy + // alert and the fork banner all quietly lose their dev workspace. Retry rather than strand + // the tab in that state. + const WORKSPACE_LIST_RETRY_DELAYS_MS = [1000, 3000, 8000] async function setUserWorkspaceStore() { - const list = await WorkspaceService.listUserWorkspaces() - $usersWorkspaceStore = list - return list + for (let attempt = 0; ; attempt++) { + try { + $usersWorkspaceStore = await WorkspaceService.listUserWorkspaces() + return + } catch (e) { + if (attempt >= WORKSPACE_LIST_RETRY_DELAYS_MS.length) throw e + console.error('could not load workspace list, retrying', e) + await new Promise((r) => setTimeout(r, WORKSPACE_LIST_RETRY_DELAYS_MS[attempt])) + } + } } // A fork deleted remotely while the tab was open leaves the client pointing at a