From 7ebb08133cd4027bc00bacc4a0fc5865cd5709ec Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 13 May 2026 06:38:57 +0000 Subject: [PATCH 01/24] fix(operator): refresh IAM RDS / Entra ID tokens in operator process (#9141) * fix(operator): refresh IAM RDS / Entra ID tokens in operator process * fix: gate DEFAULT_MAX_CONNECTIONS_OPERATOR on operator feature --- backend/src/db_connect.rs | 135 +++++++++++++++++++++++--------------- backend/src/main.rs | 19 +++++- 2 files changed, 99 insertions(+), 55 deletions(-) diff --git a/backend/src/db_connect.rs b/backend/src/db_connect.rs index 3860c18909..deda032058 100644 --- a/backend/src/db_connect.rs +++ b/backend/src/db_connect.rs @@ -6,6 +6,8 @@ use windmill_common::{ pub const DEFAULT_MAX_CONNECTIONS_SERVER: u32 = 50; pub const DEFAULT_MAX_CONNECTIONS_WORKER: u32 = 5; pub const DEFAULT_MAX_CONNECTIONS_INDEXER: u32 = 5; +#[cfg(feature = "operator")] +pub const DEFAULT_MAX_CONNECTIONS_OPERATOR: u32 = 2; pub async fn initial_connection() -> Result, error::Error> { let connect_options = get_database_url().await?.connect_options().await?; @@ -16,12 +18,35 @@ pub async fn initial_connection() -> Result, error::E .map_err(|err| Error::ConnectingToDatabase(err.to_string())) } +/// Connect to the database for the Kubernetes operator process. +/// +/// Long-running operator pods need IAM RDS / Entra ID token refresh just like the server, +/// otherwise new pool connections start failing once the initial token expires (~15 min). +#[cfg(feature = "operator")] +pub async fn operator_connection( + #[cfg(all(feature = "enterprise", feature = "private"))] + killpill_rx: tokio::sync::broadcast::Receiver<()>, +) -> anyhow::Result> { + let database_url = get_database_url().await?; + let pool = connect( + database_url.clone(), + DEFAULT_MAX_CONNECTIONS_OPERATOR, + false, + ) + .await?; + + #[cfg(all(feature = "enterprise", feature = "private"))] + spawn_token_refresh_task(pool.clone(), database_url, killpill_rx); + + Ok(pool) +} + pub async fn connect_db( server_mode: bool, indexer_mode: bool, worker_mode: bool, num_workers: i32, - #[cfg(feature = "private")] mut killpill_rx: tokio::sync::broadcast::Receiver<()>, + #[cfg(feature = "private")] killpill_rx: tokio::sync::broadcast::Receiver<()>, ) -> anyhow::Result> { use anyhow::Context; @@ -43,70 +68,72 @@ pub async fn connect_db( let pool = connect(database_url.clone(), max_connections, worker_mode).await?; #[cfg(all(feature = "enterprise", feature = "private"))] - { - let needs_token_refresh = matches!( - database_url, - DatabaseUrl::IamRds(_) | DatabaseUrl::EntraId(_) - ); - let label = match &database_url { - DatabaseUrl::IamRds(_) => "IAM RDS", - DatabaseUrl::EntraId(_) => "Entra ID", - DatabaseUrl::Static(_) => "", - }; - if needs_token_refresh { - let pool2 = pool.clone(); - let database_url2 = database_url.clone(); - tokio::spawn(async move { - loop { - tokio::select! { - _ = killpill_rx.recv() => { - break; - } - _ = tokio::time::sleep(std::time::Duration::from_secs(10)) => { - if !database_url2.needs_refresh().await { - continue; - } - let new_url = tokio::time::timeout( - std::time::Duration::from_secs(10), - get_database_url(), - ) - .await; - match new_url { - Ok(Ok(new_url)) => { - match new_url.connect_options().await { - Ok(connect_options) => { - pool2.set_connect_options(connect_options); - tracing::info!("Refreshed {label} URL successfully"); - } - Err(e) => { - tracing::error!( - "Error getting {label} connect options, retrying in 10s: {e}" - ); - continue; - } - } - } - Ok(Err(e)) => { - tracing::error!( - "Error refreshing {label} URL, trying again in 10s: {e}" - ); - continue; + spawn_token_refresh_task(pool.clone(), database_url, killpill_rx); + + Ok(pool) +} + +/// Spawn a background task that refreshes IAM RDS / Entra ID tokens before they expire +/// and updates the pool's connect options so new connections use the fresh token. +/// No-op for static (password-based) database URLs. +#[cfg(all(feature = "enterprise", feature = "private"))] +pub fn spawn_token_refresh_task( + pool: sqlx::Pool, + database_url: DatabaseUrl, + mut killpill_rx: tokio::sync::broadcast::Receiver<()>, +) { + let label = match &database_url { + DatabaseUrl::IamRds(_) => "IAM RDS", + DatabaseUrl::EntraId(_) => "Entra ID", + DatabaseUrl::Static(_) => return, + }; + tokio::spawn(async move { + loop { + tokio::select! { + _ = killpill_rx.recv() => { + break; + } + _ = tokio::time::sleep(std::time::Duration::from_secs(10)) => { + if !database_url.needs_refresh().await { + continue; + } + let new_url = tokio::time::timeout( + std::time::Duration::from_secs(10), + get_database_url(), + ) + .await; + match new_url { + Ok(Ok(new_url)) => { + match new_url.connect_options().await { + Ok(connect_options) => { + pool.set_connect_options(connect_options); + tracing::info!("Refreshed {label} URL successfully"); } Err(e) => { tracing::error!( - "Timeout after 10s refreshing {label} URL, trying again in 10s: {e}" + "Error getting {label} connect options, retrying in 10s: {e}" ); continue; } } } + Ok(Err(e)) => { + tracing::error!( + "Error refreshing {label} URL, trying again in 10s: {e}" + ); + continue; + } + Err(e) => { + tracing::error!( + "Timeout after 10s refreshing {label} URL, trying again in 10s: {e}" + ); + continue; + } } } - }); + } } - } - - Ok(pool) + }); } pub async fn connect( diff --git a/backend/src/main.rs b/backend/src/main.rs index 9a2709e9a9..74e2c240ba 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -670,7 +670,24 @@ async fn windmill_main() -> anyhow::Result<()> { tracing_subscriber::fmt::init(); tracing::info!("Starting Windmill Kubernetes operator..."); tracing::info!("Connecting to database..."); - let db = crate::db_connect::initial_connection().await?; + + #[cfg(all(feature = "enterprise", feature = "private"))] + let (operator_killpill_tx, operator_killpill_rx) = + tokio::sync::broadcast::channel::<()>(2); + + let db = crate::db_connect::operator_connection( + #[cfg(all(feature = "enterprise", feature = "private"))] + operator_killpill_rx, + ) + .await?; + + #[cfg(all(feature = "enterprise", feature = "private"))] + tokio::spawn(async move { + if let Ok(()) = tokio::signal::ctrl_c().await { + let _ = operator_killpill_tx.send(()); + } + }); + tracing::info!("Database connected. Starting ConfigMap watcher..."); windmill_operator::run(db).await?; return Ok(()); From 79c5b7b8b7676b0a06fa6480dd04b7105d39d250 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 13 May 2026 07:09:45 +0000 Subject: [PATCH 02/24] fix(cli): prevent !inline-corruption in flow push/pull (#9142) * fix(cli): hard-fail flow push on missing inline files; guard extractor * fix(cli): gate !inline extractor guard with opt-in flag * test(cli): fix createFlowFixture !inline path to be relative to flow folder --- cli/src/commands/flow/flow.ts | 12 ++-- cli/src/commands/flow/flow_metadata.ts | 30 +++++++-- cli/src/commands/sync/sync.ts | 6 +- ..._scripts_failure_preprocessor_unit.test.ts | 65 +++++++++++++++++++ cli/test/sync_pull_push.test.ts | 8 ++- .../src/inline-scripts/extractor.ts | 47 +++++++++++--- 6 files changed, 145 insertions(+), 23 deletions(-) diff --git a/cli/src/commands/flow/flow.ts b/cli/src/commands/flow/flow.ts index efd05c376a..b76b890053 100644 --- a/cli/src/commands/flow/flow.ts +++ b/cli/src/commands/flow/flow.ts @@ -174,10 +174,14 @@ export async function pushFlow( await replaceInlineScripts([localFlow.value.preprocessor_module], fileReader, log, localPath, SEP, undefined, missingFiles); } if (missingFiles.length > 0) { - log.warn(colors.yellow( - `Warning: missing inline script file(s): ${missingFiles.join(", ")}. ` + - `The flow will be pushed with unresolved !inline references.` - )); + // Hard-fail rather than push the literal `!inline path` text as + // rawscript.content. That string would be persisted in flow_version.value + // and round-trip as the script body on the next pull, overwriting the + // user's local handler with the directive — see GIT-871 / #9140. + throw new Error( + `Cannot push flow ${remotePath}: missing inline script file(s): ${missingFiles.join(", ")}. ` + + `Either restore the file(s) or remove the !inline reference(s) from flow.yaml before pushing.` + ); } const hasOnBehalfOf = (localFlow as any).has_on_behalf_of ?? !!localFlow.on_behalf_of_email; diff --git a/cli/src/commands/flow/flow_metadata.ts b/cli/src/commands/flow/flow_metadata.ts index f33ef07bcc..5c3ca578c7 100644 --- a/cli/src/commands/flow/flow_metadata.ts +++ b/cli/src/commands/flow/flow_metadata.ts @@ -266,19 +266,32 @@ export async function generateFlowLockInternal( return tree.isStale(treePath); }) : changedScripts; + const missingFiles: string[] = []; await replaceInlineScripts( flowValue.value.modules, fileReader, log, folder + SEP!, SEP, - locksToRemove + locksToRemove, + missingFiles ); if (flowValue.value.failure_module) { - await replaceInlineScripts([flowValue.value.failure_module], fileReader, log, folder + SEP!, SEP, locksToRemove); + await replaceInlineScripts([flowValue.value.failure_module], fileReader, log, folder + SEP!, SEP, locksToRemove, missingFiles); } if (flowValue.value.preprocessor_module) { - await replaceInlineScripts([flowValue.value.preprocessor_module], fileReader, log, folder + SEP!, SEP, locksToRemove); + await replaceInlineScripts([flowValue.value.preprocessor_module], fileReader, log, folder + SEP!, SEP, locksToRemove, missingFiles); + } + if (missingFiles.length > 0) { + // Abort before updateFlow rather than push the literal `!inline path` + // string as rawscript.content (GIT-871 / #9140). Note: at this point + // replaceInlineScripts has already mutated `flowValue.value` in place + // for the modules that *did* resolve. All current callers re-throw on + // this error; do not catch and reuse `flowValue` without re-parsing. + throw new Error( + `Cannot regenerate lock for flow ${remote_path}: missing inline script file(s): ${missingFiles.join(", ")}. ` + + `Either restore the file(s) or remove the !inline reference(s) from flow.yaml before retrying.` + ); } //removeChangedLocks @@ -304,18 +317,23 @@ export async function generateFlowLockInternal( const lockAssigner = newPathAssigner(opts.defaultTs ?? "bun", { skipInlineScriptSuffix: getNonDottedPaths(), }); + // flowValue.value here is the backend's response from updateFlow, so a + // rawscript whose content is `!inline ...` is corruption (GIT-871) — fail + // fast rather than writing the literal directive back to a script file. + const extractOpts = { skipInlineScriptSuffix: getNonDottedPaths(), failOnInlineDirective: true }; const inlineScripts = extractInlineScriptsForFlows( flowValue.value.modules, currentMapping, SEP, opts.defaultTs, - lockAssigner + lockAssigner, + extractOpts ); if (flowValue.value.failure_module) { - inlineScripts.push(...extractInlineScriptsForFlows([flowValue.value.failure_module], currentMapping, SEP, opts.defaultTs, lockAssigner)); + inlineScripts.push(...extractInlineScriptsForFlows([flowValue.value.failure_module], currentMapping, SEP, opts.defaultTs, lockAssigner, extractOpts)); } if (flowValue.value.preprocessor_module) { - inlineScripts.push(...extractInlineScriptsForFlows([flowValue.value.preprocessor_module], currentMapping, SEP, opts.defaultTs, lockAssigner)); + inlineScripts.push(...extractInlineScriptsForFlows([flowValue.value.preprocessor_module], currentMapping, SEP, opts.defaultTs, lockAssigner, extractOpts)); } inlineScripts.forEach((s) => { writeIfChanged(process.cwd() + SEP + folder + SEP + s.path, s.content); diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 6cd3545c5d..215a0d3120 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -950,7 +950,7 @@ function ZipFSElement( SEP, defaultTs, assigner, - { skipInlineScriptSuffix: getNonDottedPaths() }, + { skipInlineScriptSuffix: getNonDottedPaths(), failOnInlineDirective: true }, ); if (flow.value.failure_module) { inlineScripts.push(...extractInlineScriptsForFlows( @@ -959,7 +959,7 @@ function ZipFSElement( SEP, defaultTs, assigner, - { skipInlineScriptSuffix: getNonDottedPaths() }, + { skipInlineScriptSuffix: getNonDottedPaths(), failOnInlineDirective: true }, )); } if (flow.value.preprocessor_module) { @@ -969,7 +969,7 @@ function ZipFSElement( SEP, defaultTs, assigner, - { skipInlineScriptSuffix: getNonDottedPaths() }, + { skipInlineScriptSuffix: getNonDottedPaths(), failOnInlineDirective: true }, )); } } catch (error) { diff --git a/cli/test/inline_scripts_failure_preprocessor_unit.test.ts b/cli/test/inline_scripts_failure_preprocessor_unit.test.ts index 78af37a02e..a5d1298f15 100644 --- a/cli/test/inline_scripts_failure_preprocessor_unit.test.ts +++ b/cli/test/inline_scripts_failure_preprocessor_unit.test.ts @@ -574,3 +574,68 @@ describe("extractInlineScripts with mapping preserves file paths", () => { expect(lockScript!.path).toBe("my.inline_script.lock"); }); }); + +// --------------------------------------------------------------------------- +// failOnInlineDirective option (GIT-871 / #9140) +// --------------------------------------------------------------------------- + +describe("failOnInlineDirective option", () => { + test("default behavior: yaml-parsed module with !inline content extracts without throwing", () => { + // Simulates flow_metadata / dev callers: yaml-parsed local flow whose + // rawscript.content is the literal `!inline foo.ts` directive (the + // legitimate on-disk shape after extraction). + const mod = makeRawscriptModule("a", "!inline a.inline_script.ts", "bun"); + expect(() => + extractInlineScripts([mod], {}, "/", "bun"), + ).not.toThrow(); + }); + + test("yaml-parsed !inline content round-trips as the script's body", () => { + const mod = makeRawscriptModule("a", "!inline a.inline_script.ts", "bun"); + const scripts = extractInlineScripts([mod], {}, "/", "bun"); + const script = scripts.find((s) => !s.is_lock); + expect(script).toBeDefined(); + expect(script!.content).toBe("!inline a.inline_script.ts"); + }); + + test("opt-in: failOnInlineDirective=true throws on !inline content", () => { + // Simulates the sync-pull call site: rawscript came from the backend's + // flow_version.value, so `!inline ...` content means the row is corrupt. + const mod = makeRawscriptModule("failure", "!inline Handle_error.ts", "bun"); + expect(() => + extractInlineScripts([mod], {}, "/", "bun", undefined, { + failOnInlineDirective: true, + }), + ).toThrow(/corrupted inline script/); + }); + + test("opt-in: real script content still extracts cleanly", () => { + const mod = makeRawscriptModule( + "failure", + 'export function main() { return 1; }', + "bun", + ); + expect(() => + extractInlineScripts([mod], {}, "/", "bun", undefined, { + failOnInlineDirective: true, + }), + ).not.toThrow(); + }); + + test("opt-in: throws for nested rawscript inside branchall", () => { + const inner = makeRawscriptModule("inner", "!inline poisoned.ts", "bun"); + const outer: FlowModule = { + id: "branch", + value: { + type: "branchall" as const, + branches: [{ summary: "b1", expr: "true", modules: [inner], skip_failure: false, parallel: false }], + parallel: false, + }, + }; + expect(() => + extractInlineScripts([outer], {}, "/", "bun", undefined, { + failOnInlineDirective: true, + }), + ).toThrow(/corrupted inline script/); + }); +}); diff --git a/cli/test/sync_pull_push.test.ts b/cli/test/sync_pull_push.test.ts index e31c4e1084..4b5b8e5389 100644 --- a/cli/test/sync_pull_push.test.ts +++ b/cli/test/sync_pull_push.test.ts @@ -111,6 +111,10 @@ kind: script function createFlowFixture(name: string): Record { const flowSuffix = getFolderSuffix("flow"); const metadataFile = getMetadataFileName("flow", "yaml"); + // !inline paths are resolved relative to the flow folder (see + // pushFlow's fileReader in cli/src/commands/flow/flow.ts), so the + // path inside the directive must NOT include the flow folder prefix. + const scriptFile = "a.ts"; return { metadata: { @@ -122,7 +126,7 @@ value: - id: a value: type: rawscript - content: "!inline ${name}${flowSuffix}/a.ts" + content: "!inline ${scriptFile}" language: bun input_transforms: {} schema: @@ -133,7 +137,7 @@ schema: `, }, inlineScript: { - path: `${name}${flowSuffix}/a.ts`, + path: `${name}${flowSuffix}/${scriptFile}`, content: `export async function main() {\n return "Hello from flow ${name}";\n}`, }, }; diff --git a/cli/windmill-utils-internal/src/inline-scripts/extractor.ts b/cli/windmill-utils-internal/src/inline-scripts/extractor.ts index e472372a99..f556b32c57 100644 --- a/cli/windmill-utils-internal/src/inline-scripts/extractor.ts +++ b/cli/windmill-utils-internal/src/inline-scripts/extractor.ts @@ -20,13 +20,28 @@ function extractRawscriptInline( rawscript: RawScript, mapping: Record, separator: string, - assigner: PathAssigner + assigner: PathAssigner, + failOnInlineDirective: boolean ): InlineScript[] { const [basePath, ext] = assigner.assignPath(summary ?? id, rawscript.language); const mappedPath = mapping[id]; const path = mappedPath ?? basePath + ext; const language = rawscript.language; const content = rawscript.content; + // Opt-in defensive guard: when extracting from backend-shaped data (i.e. + // sync pull), a rawscript whose content is itself an `!inline ...` directive + // means the backend was poisoned by a prior push that sent the unresolved + // directive as the script body (GIT-871 / #9140). Refuse to write it back + // to disk. Off by default because callers that operate on YAML-parsed local + // flows (flow_metadata, dev) legitimately see `!inline foo.ts` as content. + if (failOnInlineDirective && typeof content === "string" && content.startsWith("!inline ")) { + throw new Error( + `Refusing to extract corrupted inline script for module '${id}': ` + + `rawscript.content is the literal string \`${content.split("\n")[0]}\` ` + + `instead of script source. The backend's flow_version.value is corrupt — ` + + `re-push from a known-good local copy to repair it.` + ); + } const r = [{ path: path, content: content, language, is_lock: false}]; rawscript.content = "!inline " + path.replaceAll(separator, "/"); const lock = rawscript.lock; @@ -50,6 +65,15 @@ function extractRawscriptInline( export interface ExtractInlineScriptsOptions { /** When true, skip the .inline_script. suffix in file names */ skipInlineScriptSuffix?: boolean; + /** + * When true, throw if a `rawscript.content` is itself an `!inline ...` + * directive. Set this only at the sync-pull call site, where the input + * comes from the backend's `flow_version.value` and `!inline ...` content + * means the row is corrupt (GIT-871 / #9140). Leave off for callers that + * pass YAML-parsed local flows — the directive is the legitimate on-disk + * shape there. + */ + failOnInlineDirective?: boolean; } /** @@ -74,6 +98,7 @@ export function extractInlineScripts( ): InlineScript[] { // Create pathAssigner only if not provided (top-level call), but reuse it for nested calls const assigner = pathAssigner ?? newPathAssigner(defaultTs ?? "bun", { skipInlineScriptSuffix: options?.skipInlineScriptSuffix }); + const failOnInlineDirective = options?.failOnInlineDirective ?? false; return modules.flatMap((m) => { if (m.value.type == "rawscript") { @@ -83,7 +108,8 @@ export function extractInlineScripts( m.value, mapping, separator, - assigner + assigner, + failOnInlineDirective ); } else if (m.value.type == "forloopflow") { return extractInlineScripts( @@ -91,11 +117,12 @@ export function extractInlineScripts( mapping, separator, defaultTs, - assigner + assigner, + options ); } else if (m.value.type == "branchall") { return m.value.branches.flatMap((b) => - extractInlineScripts(b.modules, mapping, separator, defaultTs, assigner) + extractInlineScripts(b.modules, mapping, separator, defaultTs, assigner, options) ); } else if (m.value.type == "whileloopflow") { return extractInlineScripts( @@ -103,7 +130,8 @@ export function extractInlineScripts( mapping, separator, defaultTs, - assigner + assigner, + options ); } else if (m.value.type == "branchone") { return [ @@ -113,7 +141,8 @@ export function extractInlineScripts( mapping, separator, defaultTs, - assigner + assigner, + options ) ), ...extractInlineScripts( @@ -121,7 +150,8 @@ export function extractInlineScripts( mapping, separator, defaultTs, - assigner + assigner, + options ), ]; } else if (m.value.type == "aiagent") { @@ -138,7 +168,8 @@ export function extractInlineScripts( toolValue, mapping, separator, - assigner + assigner, + failOnInlineDirective ); }); } else { From b348119ab9c76e3d80102d1e62d4037bdf901594 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 13 May 2026 07:11:24 +0000 Subject: [PATCH 03/24] publish CLI skills + AGENTS.md to windmill-cli-docs for context7 (#9143) * feat: publish CLI skills + AGENTS.md to windmill-cli-docs for context7 Auto-generates a public docs snapshot (AGENTS.md, full CLI reference, all rendered skills) and pushes it to windmill-labs/windmill-cli-docs on every release tag, so context7 can index Windmill CLI docs. - generate.py: new --context7-dir flag rendering fully-resolved skills + AGENTS.md (extracted from cli/src/guidance/core.ts to avoid drift) + cli-commands.md + README.md + manifest.json into a docs-repo checkout. Preserves .git, .github, LICENSE, context7.json across regenerations. - publish-cli-docs.yml: GitHub Action on v* tag and workflow_dispatch that regenerates the docs repo and pushes via the CLI_DOCS_DEPLOY_KEY SSH deploy key. * fix: skip tag mirror on workflow_dispatch from non-tag ref * docs: turn windmill-cli-docs README into a CLI quickstart * fix: address PR review (target safety, regex anchor, concurrency, tag mirror) - Refuse to wipe --context7-dir unless empty, has a context7 marker, or points at the windmill-cli-docs remote (P1, prevents typo blast). - Anchor AGENTS.md template regex on `generateAgentsMdContent` so adding other template-returning functions to core.ts can't silently retarget it. - Decode TS escapes in one pass to avoid order-sensitive mangling. - Include Windmill version (from version.txt) in manifest.json so each snapshot is self-describing. - Add concurrency group on the publish workflow. - Always mirror version tag on tag pushes, even when content is unchanged, so the docs repo has a tag for every Windmill release. - Expand preserve list with .gitignore, .gitattributes, CODEOWNERS. * fix: validate manifest.json content, not just presence, before wipe --- .github/workflows/publish-cli-docs.yml | 84 +++++ system_prompts/README.md | 15 + system_prompts/generate.py | 404 +++++++++++++++++++++++++ 3 files changed, 503 insertions(+) create mode 100644 .github/workflows/publish-cli-docs.yml diff --git a/.github/workflows/publish-cli-docs.yml b/.github/workflows/publish-cli-docs.yml new file mode 100644 index 0000000000..9e76117eb5 --- /dev/null +++ b/.github/workflows/publish-cli-docs.yml @@ -0,0 +1,84 @@ +name: Publish CLI docs repo + +# Regenerates the windmill-cli-docs repo (consumed by context7) from the +# canonical sources in this repo on every Windmill release. +# +# Required secret: +# CLI_DOCS_DEPLOY_KEY — ed25519 private key whose public half is registered +# as a write-access deploy key on +# windmill-labs/windmill-cli-docs. + +on: + push: + tags: + - "v*" + workflow_dispatch: + +# Serialize pushes to windmill-cli-docs so two release tags landing close +# together (e.g. a release-please bump + a hotfix) can't race to force-push +# the docs repo. +concurrency: + group: publish-cli-docs + cancel-in-progress: false + +jobs: + publish: + runs-on: ubuntu-latest + steps: + - name: Checkout windmill (source of truth) + uses: actions/checkout@v4 + with: + path: windmill + + - name: Checkout windmill-cli-docs (publish target) + uses: actions/checkout@v4 + with: + repository: windmill-labs/windmill-cli-docs + path: windmill-cli-docs + ssh-key: ${{ secrets.CLI_DOCS_DEPLOY_KEY }} + fetch-depth: 0 + + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install dependencies + run: pip install pyyaml + + - name: Regenerate docs + run: | + python3 windmill/system_prompts/generate.py \ + --context7-dir "$GITHUB_WORKSPACE/windmill-cli-docs" + + - name: Commit and push if changed + working-directory: windmill-cli-docs + env: + REF_NAME: ${{ github.ref_name }} + REF_TYPE: ${{ github.ref_type }} + run: | + git config user.name "windmill-bot" + git config user.email "bot@windmill.dev" + git add -A + if git diff --cached --quiet; then + echo "No doc changes for ${REF_NAME}." + committed=false + else + committed=true + if [ "${REF_TYPE}" = "tag" ]; then + git commit -m "chore: sync from windmill ${REF_NAME}" + else + git commit -m "chore: sync from windmill (manual dispatch from ${REF_NAME})" + fi + git push origin HEAD + fi + # Always mirror the version tag on tag pushes, even when content + # didn't change — downstream consumers tie snapshots to releases by + # tag, and skipping it would leave the docs repo without a tag for + # the new Windmill release. + # workflow_dispatch from a non-tag ref skips this so we don't + # create a junk tag named after a branch. + if [ "${REF_TYPE}" = "tag" ]; then + git tag -f "${REF_NAME}" + git push origin "${REF_NAME}" --force + echo "Mirrored tag ${REF_NAME} to windmill-cli-docs (content changed: ${committed})." + fi diff --git a/system_prompts/README.md b/system_prompts/README.md index cd3a123b6a..cc9449f93e 100644 --- a/system_prompts/README.md +++ b/system_prompts/README.md @@ -38,6 +38,21 @@ python system_prompts/generate.py --plugin-dir ~/windmill-claude-plugin - a plugin root such as `plugins/windmill-code-plugin` - a direct `skills/` directory +To regenerate the public docs repo (consumed by context7): + +```bash +python system_prompts/generate.py --context7-dir ~/windmill-cli-docs +``` + +`--context7-dir` writes a fully-rendered snapshot (`AGENTS.md`, +`cli-commands.md`, `skills//SKILL.md`, `README.md`, `manifest.json` +with the Windmill `version`) with all template placeholders resolved — +suitable for ingestion by docs aggregators. In CI this runs from +`.github/workflows/publish-cli-docs.yml` on every release tag. The +generator refuses to wipe the target directory unless it's empty or has +a context7 marker (`context7.json`, `manifest.json`, or a +`windmill-cli-docs` git remote), so a typo can't delete unrelated files. + This will: 1. Parse TypeScript and Python SDK files to extract function signatures diff --git a/system_prompts/generate.py b/system_prompts/generate.py index 87f0b63576..92090d525e 100644 --- a/system_prompts/generate.py +++ b/system_prompts/generate.py @@ -12,6 +12,7 @@ This script: Usage: python generate.py python generate.py --plugin-dir /path/to/windmill-claude-plugin + python generate.py --context7-dir /path/to/windmill-cli-docs """ import argparse @@ -1778,6 +1779,395 @@ def generate_plugin_skills( return skills_dir +# ============================================================================= +# Context7 Docs Repo Generation +# ============================================================================= + +# Files in the context7 target directory that must survive a regeneration +# (everything else is wiped to keep the export deterministic). +CONTEXT7_PRESERVE = frozenset( + { + ".git", + ".github", + ".gitignore", + ".gitattributes", + "CODEOWNERS", + "LICENSE", + "LICENSE.md", + "context7.json", + } +) + +# Name written into manifest.json — also used to recognise the docs repo +# when re-generating into an existing checkout. +CONTEXT7_REPO_NAME = "windmill-cli-docs" + + +def extract_agents_md_template() -> str: + """Extract the AGENTS.md template string from cli/src/guidance/core.ts. + + Keeping a single source of truth in TypeScript avoids drift between what + `wmill init` writes locally and what we publish for context7 ingestion. + """ + core_ts_path = SCRIPT_DIR.parent / "cli" / "src" / "guidance" / "core.ts" + content = core_ts_path.read_text() + # Anchor on the function name so adding other template-literal-returning + # functions to core.ts can't silently re-target the regex. + match = re.search( + r"function\s+generateAgentsMdContent\b[\s\S]*?return\s+`([\s\S]*?)`;", + content, + ) + if not match: + raise RuntimeError( + f"Could not extract AGENTS.md template from {core_ts_path}" + ) + return _unescape_ts_template_literal(match.group(1)) + + +def _unescape_ts_template_literal(raw: str) -> str: + """Decode TS template-literal escapes in one pass. + + Multi-pass `.replace()` would mangle e.g. `\\\\` -> `\\` -> `` ` `` if the + template ever contained a literal backslash followed by a backtick. A + single-pass scan is order-independent. + """ + return re.sub( + r"\\(.)", + lambda m: {"`": "`", "$": "$", "\\": "\\"}.get(m.group(1), m.group(0)), + raw, + ) + + +def render_agents_md_for_docs( + skills: list[str], skill_desc_map: dict[str, str] +) -> str: + """Render AGENTS.md exactly as `wmill init` would, for the docs repo.""" + template = extract_agents_md_template() + skills_reference = "\n".join( + f"- `.claude/skills/{name}/SKILL.md` - {skill_desc_map[name]}" + for name in skills + if name in skill_desc_map + ) + return template.replace("${skillsReference}", skills_reference) + + +def build_skill_desc_map(skills: list[str]) -> dict[str, str]: + """Map each skill name to its user-facing description. + + Mirrors the logic in `generate_skills_ts_export`: language skills draw from + LANGUAGE_METADATA, everything else from SKILL_DEFINITIONS. + """ + desc_map = {s["name"]: s["description"] for s in SKILL_DEFINITIONS} + for skill in skills: + if skill.startswith("write-script-"): + lang_key = skill.replace("write-script-", "") + metadata = LANGUAGE_METADATA.get(lang_key) + if metadata: + desc_map[skill] = metadata["description"] + return desc_map + + +def _looks_like_windmill_manifest(path: Path) -> bool: + """Return True iff `path` is a JSON file whose top-level `name` is ours. + + Used to distinguish a previously-generated docs repo from an unrelated + project that happens to have a `manifest.json` (Chrome extensions, npm + packages, web app manifests, etc.). + """ + try: + data = json.loads(path.read_text()) + except (OSError, json.JSONDecodeError): + return False + return isinstance(data, dict) and data.get("name") == CONTEXT7_REPO_NAME + + +def _verify_context7_target(target_dir: Path) -> None: + """Refuse to wipe a non-empty dir that doesn't look like the docs repo. + + A typo such as `--context7-dir .`, `~`, or the wrong checkout could + otherwise nuke unrelated files. We accept the target if it's empty/new, + if it has our ownership file, if its `manifest.json` self-identifies as + the windmill-cli-docs repo, or if its git origin points at one. + """ + if not target_dir.exists() or not any(target_dir.iterdir()): + return + + if (target_dir / "context7.json").exists(): + return + + manifest_path = target_dir / "manifest.json" + if manifest_path.exists() and _looks_like_windmill_manifest(manifest_path): + return + + git_dir = target_dir / ".git" + if git_dir.exists(): + import subprocess + + try: + origin = subprocess.run( + ["git", "-C", str(target_dir), "config", "--get", "remote.origin.url"], + capture_output=True, + text=True, + check=True, + ).stdout.strip() + if CONTEXT7_REPO_NAME in origin: + return + except subprocess.CalledProcessError: + pass + + raise RuntimeError( + f"Refusing to overwrite {target_dir}: target does not look like the " + f"{CONTEXT7_REPO_NAME} docs repo.\n" + f"Expected one of:\n" + f" - a `context7.json` at the top level,\n" + f" - a `manifest.json` whose top-level `name` is {CONTEXT7_REPO_NAME!r},\n" + f" - a git remote `origin` containing '{CONTEXT7_REPO_NAME}'.\n" + f"If this is the right directory, add a `context7.json` and retry." + ) + + +def clear_context7_dir(target_dir: Path) -> None: + """Wipe the docs repo dir of previously generated content. + + Preserves a small allowlist (.git, .github, LICENSE, context7.json, etc.) + so this can run against a real checkout without nuking version control or + CI config. + """ + if not target_dir.exists(): + return + for entry in target_dir.iterdir(): + if entry.name in CONTEXT7_PRESERVE: + continue + if entry.is_dir(): + shutil.rmtree(entry) + else: + entry.unlink() + + +def _read_windmill_version() -> str | None: + """Return the Windmill release version (e.g. '1.700.2'), or None if absent. + + Sourced from `version.txt` at the repo root — the same file release-please + updates on every release. + """ + version_file = SCRIPT_DIR.parent / "version.txt" + if not version_file.exists(): + return None + return version_file.read_text().strip() or None + + +def generate_context7_repo( + target_dir: Path, + skills: list[str], + schema_yaml_content: dict[str, str], + cli_commands_md: str, +) -> Path: + """Generate a fully-rendered docs repo suitable for context7 ingestion. + + Layout written to `target_dir`: + AGENTS.md # the prompt agents see in their projects + README.md # stable intro for humans / context7 + manifest.json # version + skill list (for indexing) + cli-commands.md # full CLI flag reference + skills//SKILL.md # one rendered skill per file + """ + target_dir = target_dir.expanduser().resolve() + target_dir.mkdir(parents=True, exist_ok=True) + _verify_context7_target(target_dir) + clear_context7_dir(target_dir) + + skill_desc_map = build_skill_desc_map(skills) + + # AGENTS.md — the same content `wmill init` writes locally. + (target_dir / "AGENTS.md").write_text( + render_agents_md_for_docs(skills, skill_desc_map) + ) + + # Full CLI reference at top level. + (target_dir / "cli-commands.md").write_text(cli_commands_md) + + # One markdown per skill, with schemas inlined (no template placeholders). + skills_dir = target_dir / "skills" + skills_dir.mkdir(parents=True, exist_ok=True) + for skill_name in skills: + skill_dir = skills_dir / skill_name + skill_dir.mkdir(parents=True, exist_ok=True) + (skill_dir / "SKILL.md").write_text( + render_plugin_skill_content(skill_name, schema_yaml_content) + ) + + # Stable README so the GitHub repo landing page tells readers (and + # context7's crawler) what they're looking at. + (target_dir / "README.md").write_text(_context7_readme(skills)) + + # Machine-readable index for context7 / downstream consumers. + # Note: the `name` field is also the marker `_verify_context7_target` + # uses to distinguish our `manifest.json` from generic ones. + manifest = { + "name": CONTEXT7_REPO_NAME, + "description": ( + "Auto-generated Windmill CLI docs: agent prompt, skills, and " + "full CLI reference. Source: github.com/windmill-labs/windmill." + ), + "skills": [ + {"name": name, "description": skill_desc_map.get(name, "")} + for name in skills + ], + } + version = _read_windmill_version() + if version: + manifest["version"] = version + (target_dir / "manifest.json").write_text( + json.dumps(manifest, indent=2) + "\n" + ) + + print(f"\nGenerated for context7 docs repo:") + print(f" - {target_dir} ({len(skills)} skills + AGENTS.md + cli-commands.md)") + return target_dir + + +def _context7_readme(skills: list[str]) -> str: + """Render the README that ships at the root of the docs repo. + + Doubles as a CLI quickstart for humans landing on the GitHub page and as + the top-level entry point context7 indexes first — keep it actionable. + """ + skill_lines = "\n".join(f"- `skills/{name}/SKILL.md`" for name in skills) + return f"""# Windmill CLI Quickstart + +[`wmill`](https://www.windmill.dev/docs/advanced/cli) is the official command +line interface for [Windmill](https://www.windmill.dev) — an open-source +platform for internal tools, workflows, API integrations, background jobs, and +UIs. Use it to authenticate against a workspace, scaffold local projects, +sync scripts/flows/apps between your filesystem and a workspace, and run or +debug jobs from your terminal. + +## Install + +```sh +npm install -g windmill-cli +wmill --version +``` + +Upgrade later with `wmill upgrade`. + +## Connect to a workspace + +```sh +wmill workspace add +``` + +This walks you through adding a workspace profile — a `(name, remote URL, +workspace id, token)` tuple stored under `~/.config/windmill`. You can have +multiple profiles and switch between them with `wmill workspace switch `. + +A workspace token is created from the Windmill UI under +`User Settings → Tokens`. For self-hosted instances, point the remote at your +own URL (e.g. `https://windmill.example.com`). + +## Initialize a project directory + +```sh +wmill init +``` + +`wmill init` creates: + +- `wmill.yaml` — sync configuration (which folders/types to track). +- `AGENTS.md` + `CLAUDE.md` — the agent prompt published in this repo. +- `.claude/skills/` and `.agents/skills/` — per-task guides used by AI coding + assistants (Claude Code, Codex, Pi). These are the same `SKILL.md` files + you'll find under `skills/` in this repo. + +It also offers to bind a workspace profile to the current git branch and to +import git-sync settings from the backend if any are configured. + +## Sync between local files and a workspace + +```sh +wmill sync pull # workspace → local (writes flows, scripts, apps, etc.) +wmill sync push # local → workspace +``` + +Sync is idempotent and diff-aware: `wmill sync push --dry-run` previews the +changes without applying them. Use `--yaml` (recommended) to keep specs as +YAML rather than JSON. + +For individual entities you can also use the type-specific commands: + +```sh +wmill script push path/to/script.ts +wmill flow push path/to/flow.yaml +wmill app push path/to/app.yaml +wmill resource push path/to/resource.yaml +``` + +## Run, inspect, and debug jobs + +```sh +wmill script run u/me/my_script --data '{{"foo": "bar"}}' +wmill flow run u/me/my_flow --data @inputs.json +wmill job list --failed --limit 20 +wmill job get +wmill job logs +``` + +Logs and flow steps stream as the job runs. For flow failures, `wmill job get` +shows the step tree with each sub-job's id so you can drill in with +`wmill job logs `. + +## Scaffold new entities + +```sh +wmill script new u/me/path --language bun +wmill flow new u/me/path --summary "..." +wmill app new u/me/path --summary "..." --framework svelte +``` + +These create the correct folder layout and a minimal spec file, then print +next-step hints. Prefer them over hand-creating the folders — they pick the +right naming conventions for your workspace. + +## Triggers and schedules + +Triggers (HTTP routes, WebSocket, Kafka, NATS, MQTT, SQS, GCP Pub/Sub, Azure +Event Hubs, Email, Postgres CDC) and cron schedules are tracked as YAML files +synced alongside your scripts and flows. See `skills/triggers/SKILL.md` and +`skills/schedules/SKILL.md` for the full schemas. + +## Completion + +```sh +source <(wmill completions bash) # bash, zsh: source <(wmill completions zsh) +source (wmill completions fish | psub) # fish +``` + +## Reference + +- `cli-commands.md` — every `wmill` command and flag, generated from the + source. +- `AGENTS.md` — the top-level prompt the CLI installs into each project (and + the same instructions AI coding assistants follow when working in a + Windmill repo). +- `skills//SKILL.md` — one self-contained guide per common task. + +### Skills index + +{skill_lines} + +## About this repo + +Auto-generated mirror of the Windmill CLI's bundled AI-agent guidance and +command reference, published for ingestion by docs aggregators such as +[context7](https://context7.com). + +**Do not edit by hand.** This repo is regenerated from +[windmill-labs/windmill](https://github.com/windmill-labs/windmill) on every +release. Open issues and PRs in the source repo, not here. The generator is +`system_prompts/generate.py --context7-dir`. +""" + + # ============================================================================= # Main Entry Point # ============================================================================= @@ -1799,6 +2189,15 @@ def parse_args() -> argparse.Namespace: "a plugin root, or a skills directory, and refreshes standalone skills there." ), ) + parser.add_argument( + "--context7-dir", + type=Path, + help=( + "Optional path to a docs-repo checkout (e.g. windmill-cli-docs). " + "Writes AGENTS.md, cli-commands.md, skills/, README.md, and manifest.json " + "with all placeholders resolved, suitable for context7 ingestion." + ), + ) return parser.parse_args() @@ -2117,6 +2516,11 @@ export declare function getWorkflowAsCodePrompt(language?: string): string; if args.plugin_dir: generate_plugin_skills(args.plugin_dir, skills, schema_yaml_content) + if args.context7_dir: + generate_context7_repo( + args.context7_dir, skills, schema_yaml_content, cli_commands + ) + print("\nDone!") From 7a7d246a6e27aef6bc15c2e88a4163874f474b86 Mon Sep 17 00:00:00 2001 From: centdix <40307056+centdix@users.noreply.github.com> Date: Wed, 13 May 2026 10:20:30 +0200 Subject: [PATCH 04/24] test: add global ai eval mode (#9129) * feat: add global ai eval mode * fix: improve global eval validation feedback --- ai_evals/AGENTS.md | 11 + ai_evals/README.md | 16 +- ai_evals/adapters/frontend/benchmarkRunner.ts | 7 +- .../frontend/core/global/globalEvalRunner.ts | 127 ++++++ ai_evals/adapters/frontend/progress.ts | 2 +- ai_evals/adapters/frontend/runtime.ts | 2 +- .../adapters/frontend/vitestAdapter.test.ts | 174 ++++++++ ai_evals/cases/global.yaml | 55 +++ ai_evals/cli/index.ts | 7 +- ai_evals/core/cases.test.ts | 20 + ai_evals/core/models.ts | 2 +- ai_evals/core/types.ts | 26 +- ai_evals/core/validators.test.ts | 225 +++++++++++ ai_evals/core/validators.ts | 376 ++++++++++++++++++ .../initial/format_greeting_script.json | 23 ++ ai_evals/modes/global.ts | 81 ++++ 16 files changed, 1142 insertions(+), 12 deletions(-) create mode 100644 ai_evals/adapters/frontend/core/global/globalEvalRunner.ts create mode 100644 ai_evals/cases/global.yaml create mode 100644 ai_evals/fixtures/frontend/global/initial/format_greeting_script.json create mode 100644 ai_evals/modes/global.ts diff --git a/ai_evals/AGENTS.md b/ai_evals/AGENTS.md index 096baf5b58..d26e6d60ea 100644 --- a/ai_evals/AGENTS.md +++ b/ai_evals/AGENTS.md @@ -6,6 +6,7 @@ This folder contains black-box benchmark cases for: - `app` - `script` - `cli` +- `global` The goal is to test the current production prompts and guidance with realistic user requests, not to test one exact implementation shape. @@ -75,6 +76,16 @@ Still, avoid benchmark phrasing. The prompt should read like a repo task, not a When relevant, ask the assistant to tell the user which `wmill` commands to run next. That is part of the benchmarked behavior. +## Global-specific rules + +Global prompts should exercise workspace-level drafting behavior: + +- inspecting existing scripts, flows, apps, schedules, triggers, resources, and variables when relevant +- writing AI drafts rather than saving or deploying by default +- producing coherent multi-artifact changes when the request crosses artifact boundaries + +Keep deterministic validation focused on the draft contract: required draft type/path, required content snippets, forbidden draft paths, and forbidden mutating tools such as deploy/delete unless the case explicitly asks for them. + ## Deterministic validation Use deterministic validation only for hard failures such as: diff --git a/ai_evals/README.md b/ai_evals/README.md index 2e1f3210f8..6982d70da9 100644 --- a/ai_evals/README.md +++ b/ai_evals/README.md @@ -1,11 +1,12 @@ # AI Evals -Small benchmark runner for the four Windmill AI generation modes: +Small benchmark runner for the Windmill AI generation modes: - `cli` - `flow` - `script` - `app` +- `global` The benchmark always tests the current production prompts, tools, and guidance in this checkout. @@ -57,6 +58,7 @@ bun run cli -- run flow flow-test0-sum-two-numbers --runs 3 --verbose bun run cli -- run flow --record GEMINI_API_KEY=... bun run cli -- run app app-test1-counter-create --model gemini-pro WMILL_AI_EVAL_BACKEND_URL=http://127.0.0.1:8000 bun run cli -- run flow --backend-validation preview +bun run cli -- run global global-test1-script-create bun run cli -- run cli bun-hello-script ``` @@ -94,7 +96,7 @@ Today: Notes: - the command also prints accepted alias spellings such as `gpt-4o`, `claude-opus-4.6`, and `claude-haiku-4.5` -- frontend modes (`flow`, `script`, `app`) can use Anthropic, OpenAI, and Gemini-backed aliases +- frontend modes (`flow`, `script`, `app`, `global`) can use Anthropic, OpenAI, and Gemini-backed aliases - `cli` mode always uses the Anthropic agent SDK, so only Anthropic aliases are valid there - the judge model is separate and currently defaults to `claude-sonnet-4-6` @@ -133,6 +135,13 @@ For `app` mode, `validate` can express narrow hard requirements such as: - minimum datatable / datatable-table counts - specific required datatable tables +For `global` mode, `validate` can express draft-level requirements such as: + +- required draft type/path/language +- required or forbidden snippets in draft values +- required or forbidden draft counts +- forbidden draft paths + App fixtures can also include an optional `datatables.json` file at the fixture root. For `flow` mode, an `initial` fixture can also include a benchmark workspace catalog of @@ -174,6 +183,7 @@ If `--record` is used, the CLI also appends one compact JSON line to: - `ai_evals/history/flow.jsonl` - `ai_evals/history/script.jsonl` - `ai_evals/history/app.jsonl` +- `ai_evals/history/global.jsonl` - `ai_evals/history/cli.jsonl` Each recorded line contains: @@ -194,6 +204,7 @@ Typical artifacts by mode: - `flow`: `flow.json` - `script`: `script.json` plus the generated script file - `app`: `app.json` plus frontend/backend files +- `global`: `global-drafts.json` - `cli`: `assistant-output.txt`, `trace.json`, `wmill-invocations.jsonl`, plus generated workspace files - backend-validated attempts also include `backend-preview.json` @@ -209,6 +220,7 @@ Typical artifacts by mode: ## Notes - Frontend modes reuse the production frontend chat code through the Vitest bridge. +- Global mode evaluates the production global AI tools and validates the resulting AI draft store. - CLI mode creates an isolated workspace, writes the current checkout guidance into it, and benchmarks the real skills / `AGENTS.md` flow. - CLI mode now also records a structured trace of invoked skills, tool calls, proposed `wmill` commands, and any attempted `wmill` executions. - Frontend progress streams live while the benchmark is running. diff --git a/ai_evals/adapters/frontend/benchmarkRunner.ts b/ai_evals/adapters/frontend/benchmarkRunner.ts index 50a0d10c3c..14be108a10 100644 --- a/ai_evals/adapters/frontend/benchmarkRunner.ts +++ b/ai_evals/adapters/frontend/benchmarkRunner.ts @@ -12,10 +12,11 @@ import { resolveWindmillBackendSettings } from "../../core/windmillBackendSettin import { emitFrontendBenchmarkProgress } from "./progress"; import { createAppModeRunner } from "../../modes/app"; import { createFlowModeRunner } from "../../modes/flow"; +import { createGlobalModeRunner } from "../../modes/global"; import { createScriptModeRunner } from "../../modes/script"; import { DEFAULT_JUDGE_MODEL } from "../../core/judge"; -export type FrontendBenchmarkMode = "flow" | "app" | "script"; +export type FrontendBenchmarkMode = "flow" | "app" | "script" | "global"; export async function runFrontendBenchmarkFromEnv(): Promise { const mode = parseMode(process.env.WMILL_FRONTEND_AI_EVAL_MODE); @@ -85,11 +86,13 @@ function getModeRunner( backendValidation, backendSettings, ); + case "global": + return createGlobalModeRunner(model, backendSettings); } } function parseMode(value: string | undefined): FrontendBenchmarkMode { - if (value === "flow" || value === "app" || value === "script") { + if (value === "flow" || value === "app" || value === "script" || value === "global") { return value; } throw new Error(`Unsupported frontend benchmark mode: ${String(value)}`); diff --git a/ai_evals/adapters/frontend/core/global/globalEvalRunner.ts b/ai_evals/adapters/frontend/core/global/globalEvalRunner.ts new file mode 100644 index 0000000000..5e00dd6f34 --- /dev/null +++ b/ai_evals/adapters/frontend/core/global/globalEvalRunner.ts @@ -0,0 +1,127 @@ +import { mkdtemp, rm } from "fs/promises"; +import { tmpdir } from "os"; +import { join } from "path"; +import type { AIProvider } from "$lib/gen/types.gen"; +import { + globalTools, + prepareGlobalSystemMessage, + prepareGlobalUserMessage, +} from "../../../../../frontend/src/lib/components/copilot/chat/global/core"; +import { globalDraftStore } from "../../../../../frontend/src/lib/components/copilot/chat/global/draftStore.svelte"; +import type { Tool as ProductionTool } from "../../../../../frontend/src/lib/components/copilot/chat/shared"; +import type { ModeRunContext } from "../../../../core/types"; +import type { GlobalDraftState } from "../../../../core/validators"; +import type { WindmillBackendSettings } from "../../../../core/windmillBackendSettings"; +import { + registerBenchmarkWorkspaceRunnables, + unregisterBenchmarkWorkspaceRunnables, + type BenchmarkWorkspaceRunnables, +} from "../../mockBackend"; +import { runEval } from "../shared"; +import type { TokenUsage, ToolCallDetail } from "../shared/types"; + +const MUTATING_GLOBAL_TOOLS = new Set([ + "deploy_workspace_item", + "delete_workspace_item", +]); + +export interface GlobalEvalResult { + success: boolean; + state: GlobalDraftState; + error?: string; + assistantMessageCount: number; + toolCallCount: number; + toolsUsed: string[]; + toolCallDetails: ToolCallDetail[]; + tokenUsage: TokenUsage; +} + +export interface GlobalEvalOptions { + workspaceFixtures?: BenchmarkWorkspaceRunnables; + model?: string; + maxIterations?: number; + provider?: AIProvider; + backend: WindmillBackendSettings; + workspaceRoot?: string; + runContext?: ModeRunContext; +} + +export async function runGlobalEval( + userPrompt: string, + apiKey: string, + options: GlobalEvalOptions, +): Promise { + const workspaceRoot = + options.workspaceRoot ?? + (await mkdtemp(join(tmpdir(), "wmill-frontend-global-benchmark-"))); + + globalDraftStore.clearDrafts(workspaceRoot); + registerBenchmarkWorkspaceRunnables(workspaceRoot, options.workspaceFixtures ?? {}); + + try { + const model = options.model ?? "claude-haiku-4-5-20251001"; + const rawResult = await runEval({ + userPrompt, + systemMessage: prepareGlobalSystemMessage(), + userMessage: prepareGlobalUserMessage(userPrompt), + tools: getGlobalEvalTools(), + helpers: {}, + apiKey, + getOutput: () => ({ drafts: globalDraftStore.listDrafts(workspaceRoot) }), + onAssistantMessageStart: options.runContext?.onAssistantMessageStart, + onAssistantToken: options.runContext?.onAssistantChunk, + onAssistantMessageEnd: options.runContext?.onAssistantMessageEnd, + onToolCall: options.runContext?.onToolCall, + options: { + maxIterations: options.maxIterations, + model, + workspace: workspaceRoot, + provider: options.provider, + backend: options.backend, + caseId: options.runContext?.caseId, + attempt: options.runContext?.attempt, + }, + }); + + return { + state: rawResult.output, + success: rawResult.success, + error: rawResult.error, + assistantMessageCount: rawResult.iterations, + toolCallCount: rawResult.toolCallsCount, + toolsUsed: rawResult.toolsCalled, + toolCallDetails: rawResult.toolCallDetails, + tokenUsage: rawResult.tokenUsage, + }; + } finally { + globalDraftStore.clearDrafts(workspaceRoot); + unregisterBenchmarkWorkspaceRunnables(workspaceRoot); + if (!options.workspaceRoot) { + await rm(workspaceRoot, { recursive: true, force: true }); + } + } +} + +function getGlobalEvalTools(): ProductionTool<{}>[] { + return (globalTools as ProductionTool<{}>[]).map((tool) => { + if (!MUTATING_GLOBAL_TOOLS.has(tool.def.function.name)) { + return tool; + } + + return { + ...tool, + requiresConfirmation: false, + validateBeforeConfirmation: undefined, + fn: async () => + JSON.stringify( + { + success: false, + error: + "This mutating workspace tool is disabled during ai_evals global mode.", + }, + null, + 2, + ), + }; + }); +} diff --git a/ai_evals/adapters/frontend/progress.ts b/ai_evals/adapters/frontend/progress.ts index b5b8f12c83..3a4810c4a3 100644 --- a/ai_evals/adapters/frontend/progress.ts +++ b/ai_evals/adapters/frontend/progress.ts @@ -1,4 +1,4 @@ -export type FrontendBenchmarkProgressSurface = 'flow' | 'app' | 'script' +export type FrontendBenchmarkProgressSurface = 'flow' | 'app' | 'script' | 'global' export type FrontendBenchmarkProgressEvent = | { diff --git a/ai_evals/adapters/frontend/runtime.ts b/ai_evals/adapters/frontend/runtime.ts index 180c7a2993..347e15191c 100644 --- a/ai_evals/adapters/frontend/runtime.ts +++ b/ai_evals/adapters/frontend/runtime.ts @@ -16,7 +16,7 @@ const FRONTEND_BENCHMARK_TEST = const FRONTEND_BENCHMARK_CONFIG = "../ai_evals/adapters/frontend/vitest.config.ts"; -export type FrontendMode = "flow" | "app" | "script"; +export type FrontendMode = "flow" | "app" | "script" | "global"; export async function runFrontendBenchmarkAdapter(input: { mode: FrontendMode; diff --git a/ai_evals/adapters/frontend/vitestAdapter.test.ts b/ai_evals/adapters/frontend/vitestAdapter.test.ts index 542feaf89b..1275acf0b4 100644 --- a/ai_evals/adapters/frontend/vitestAdapter.test.ts +++ b/ai_evals/adapters/frontend/vitestAdapter.test.ts @@ -65,6 +65,10 @@ vi.mock('$lib/gen', async () => { hasBenchmarkWorkspace(data.workspace) ? (listBenchmarkScripts(data.workspace) ?? []) : actual.ScriptService.listScripts(data), + existsScriptByPath: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) + ? Boolean(getBenchmarkScriptByPath(data.workspace, data.path)) + : actual.ScriptService.existsScriptByPath(data), getScriptByPath: async (data: { workspace: string; path: string }) => { if (hasBenchmarkWorkspace(data.workspace)) { const script = getBenchmarkScriptByPath(data.workspace, data.path) @@ -91,6 +95,10 @@ vi.mock('$lib/gen', async () => { hasBenchmarkWorkspace(data.workspace) ? (listBenchmarkFlows(data.workspace) ?? []) : actual.FlowService.listFlows(data), + existsFlowByPath: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) + ? Boolean(getBenchmarkFlowByPath(data.workspace, data.path)) + : actual.FlowService.existsFlowByPath(data), getFlowByPath: async (data: { workspace: string; path: string }) => { if (hasBenchmarkWorkspace(data.workspace)) { const flow = getBenchmarkFlowByPath(data.workspace, data.path) @@ -142,6 +150,16 @@ vi.mock('$lib/gen', async () => { } }), ScheduleService: wrapService(actual.ScheduleService, { + existsSchedule: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.ScheduleService.existsSchedule(data), + listSchedules: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.ScheduleService.listSchedules(data), + getSchedule: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Schedule "${data.path}" not found in benchmark workspace`) + } + return actual.ScheduleService.getSchedule(data) + }, previewSchedule: async (data: { requestBody?: Record }) => previewBenchmarkSchedule(data), createSchedule: async (data: { workspace: string; requestBody: Record }) => @@ -149,11 +167,167 @@ vi.mock('$lib/gen', async () => { ? createBenchmarkSchedule(data) : actual.ScheduleService.createSchedule(data) }), + ResourceService: wrapService(actual.ResourceService, { + existsResource: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.ResourceService.existsResource(data), + listResource: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.ResourceService.listResource(data), + getResource: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Resource "${data.path}" not found in benchmark workspace`) + } + return actual.ResourceService.getResource(data) + }, + queryResourceTypes: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.ResourceService.queryResourceTypes(data) + }), + VariableService: wrapService(actual.VariableService, { + existsVariable: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.VariableService.existsVariable(data), + listVariable: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.VariableService.listVariable(data), + getVariable: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Variable "${data.path}" not found in benchmark workspace`) + } + return actual.VariableService.getVariable(data) + } + }), + AppService: wrapService(actual.AppService, { + existsApp: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.AppService.existsApp(data), + listApps: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.AppService.listApps(data), + getAppByPath: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`App "${data.path}" not found in benchmark workspace`) + } + return actual.AppService.getAppByPath(data) + } + }), HttpTriggerService: wrapService(actual.HttpTriggerService, { + existsHttpTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.HttpTriggerService.existsHttpTrigger(data), + listHttpTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.HttpTriggerService.listHttpTriggers(data), + getHttpTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`HTTP trigger "${data.path}" not found in benchmark workspace`) + } + return actual.HttpTriggerService.getHttpTrigger(data) + }, createHttpTrigger: async (data: { workspace: string; requestBody: Record }) => hasBenchmarkWorkspace(data.workspace) ? createBenchmarkHttpTrigger(data) : actual.HttpTriggerService.createHttpTrigger(data) + }), + WebsocketTriggerService: wrapService(actual.WebsocketTriggerService, { + existsWebsocketTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) + ? false + : actual.WebsocketTriggerService.existsWebsocketTrigger(data), + listWebsocketTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) + ? [] + : actual.WebsocketTriggerService.listWebsocketTriggers(data), + getWebsocketTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Websocket trigger "${data.path}" not found in benchmark workspace`) + } + return actual.WebsocketTriggerService.getWebsocketTrigger(data) + } + }), + KafkaTriggerService: wrapService(actual.KafkaTriggerService, { + existsKafkaTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) + ? false + : actual.KafkaTriggerService.existsKafkaTrigger(data), + listKafkaTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.KafkaTriggerService.listKafkaTriggers(data), + getKafkaTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Kafka trigger "${data.path}" not found in benchmark workspace`) + } + return actual.KafkaTriggerService.getKafkaTrigger(data) + } + }), + NatsTriggerService: wrapService(actual.NatsTriggerService, { + existsNatsTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.NatsTriggerService.existsNatsTrigger(data), + listNatsTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.NatsTriggerService.listNatsTriggers(data), + getNatsTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`NATS trigger "${data.path}" not found in benchmark workspace`) + } + return actual.NatsTriggerService.getNatsTrigger(data) + } + }), + PostgresTriggerService: wrapService(actual.PostgresTriggerService, { + existsPostgresTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) + ? false + : actual.PostgresTriggerService.existsPostgresTrigger(data), + listPostgresTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) + ? [] + : actual.PostgresTriggerService.listPostgresTriggers(data), + getPostgresTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Postgres trigger "${data.path}" not found in benchmark workspace`) + } + return actual.PostgresTriggerService.getPostgresTrigger(data) + } + }), + MqttTriggerService: wrapService(actual.MqttTriggerService, { + existsMqttTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.MqttTriggerService.existsMqttTrigger(data), + listMqttTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.MqttTriggerService.listMqttTriggers(data), + getMqttTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`MQTT trigger "${data.path}" not found in benchmark workspace`) + } + return actual.MqttTriggerService.getMqttTrigger(data) + } + }), + SqsTriggerService: wrapService(actual.SqsTriggerService, { + existsSqsTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.SqsTriggerService.existsSqsTrigger(data), + listSqsTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.SqsTriggerService.listSqsTriggers(data), + getSqsTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`SQS trigger "${data.path}" not found in benchmark workspace`) + } + return actual.SqsTriggerService.getSqsTrigger(data) + } + }), + GcpTriggerService: wrapService(actual.GcpTriggerService, { + existsGcpTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.GcpTriggerService.existsGcpTrigger(data), + listGcpTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.GcpTriggerService.listGcpTriggers(data), + getGcpTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`GCP trigger "${data.path}" not found in benchmark workspace`) + } + return actual.GcpTriggerService.getGcpTrigger(data) + } + }), + AzureTriggerService: wrapService(actual.AzureTriggerService, { + existsAzureTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) + ? false + : actual.AzureTriggerService.existsAzureTrigger(data), + listAzureTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.AzureTriggerService.listAzureTriggers(data), + getAzureTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Azure trigger "${data.path}" not found in benchmark workspace`) + } + return actual.AzureTriggerService.getAzureTrigger(data) + } }) } }) diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml new file mode 100644 index 0000000000..709081942b --- /dev/null +++ b/ai_evals/cases/global.yaml @@ -0,0 +1,55 @@ +- id: global-test1-script-create + prompt: |- + Create a draft Bun script at `f/evals/global/greet_user`. + It should take a string `name` input and return `Hello, ${name}!`. + Leave it as an AI draft only; do not deploy or save it. + runtime: + maxTurns: 8 + validate: + draftCountExactly: 1 + requiredDrafts: + - type: script + path: f/evals/global/greet_user + language: bun + valueIncludes: + - name + - Hello + toolExpect: + requiredToolsUsed: + - write_script + forbiddenToolsUsed: + - deploy_workspace_item + - delete_workspace_item + judgeChecklist: + - creates a Bun script draft at f/evals/global/greet_user + - the script accepts a name input + - the script returns a greeting containing Hello, the provided name, and an exclamation mark + - the result stays as an AI draft and is not deployed or saved to the workspace + +- id: global-test2-script-edit-existing + prompt: |- + Update the existing workspace script at `f/evals/global/format_greeting`. + Keep it as a Bun script, but change the greeting so the provided name is uppercased and the returned message ends with an exclamation mark. + Leave the result as an AI draft only; do not deploy or save it. + initial: ai_evals/fixtures/frontend/global/initial/format_greeting_script.json + runtime: + maxTurns: 8 + validate: + draftCountExactly: 1 + requiredDrafts: + - type: script + path: f/evals/global/format_greeting + language: bun + valueIncludes: + - toUpperCase + - "!" + toolExpect: + forbiddenToolsUsed: + - deploy_workspace_item + - delete_workspace_item + judgeChecklist: + - creates an AI draft for the existing f/evals/global/format_greeting script + - preserves the script as Bun + - uppercases the provided name in the greeting + - returns a message ending with an exclamation mark + - does not deploy or save the draft to the workspace diff --git a/ai_evals/cli/index.ts b/ai_evals/cli/index.ts index 259b055ff5..8ed61740c8 100644 --- a/ai_evals/cli/index.ts +++ b/ai_evals/cli/index.ts @@ -53,6 +53,7 @@ async function main() { " bun run cli -- run flow --record", " bun run cli -- run flow --backend-validation preview", " bun run cli -- run flow flow-test5-simple-modification --runs 3", + " bun run cli -- run global global-test1-script-create", " bun run cli -- run cli bun-hello-script", "", "Models:", @@ -70,7 +71,7 @@ async function main() { program .command("cases") .description("List available cases") - .argument("[mode]", "cli, flow, script, or app", parseOptionalMode) + .argument("[mode]", "cli, flow, script, app, or global", parseOptionalMode) .action(async (mode?: EvalMode) => { await handleCases(mode); }); @@ -78,7 +79,7 @@ async function main() { program .command("run") .description("Run one benchmark mode") - .argument("", "cli, flow, script, or app", parseMode) + .argument("", "cli, flow, script, app, or global", parseMode) .argument("[caseIds...]", "specific case ids to run") .option( "--runs ", @@ -152,7 +153,7 @@ function handleModels() { process.stdout.write("Available models\n"); for (const model of EVAL_MODELS) { const supports = [ - ...(model.frontend ? ["flow", "script", "app"] : []), + ...(model.frontend ? ["flow", "script", "app", "global"] : []), ...(model.cli ? ["cli"] : []), ]; const aliases = [ diff --git a/ai_evals/core/cases.test.ts b/ai_evals/core/cases.test.ts index 977bb71390..733d34ddd2 100644 --- a/ai_evals/core/cases.test.ts +++ b/ai_evals/core/cases.test.ts @@ -183,6 +183,26 @@ describe("loadCases", () => { }); }); + it("loads global draft validation and forbidden tool expectations", async () => { + const globalCases = await loadCases("global"); + const caseEntry = globalCases.find((entry) => entry.id === "global-test1-script-create"); + + expect(caseEntry?.validate).toMatchObject({ + draftCountExactly: 1, + requiredDrafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + }, + ], + }); + expect(caseEntry?.toolExpect).toMatchObject({ + requiredToolsUsed: ["write_script"], + forbiddenToolsUsed: ["deploy_workspace_item", "delete_workspace_item"], + }); + }); + it("loads tool expectations for workspace mutation cases", async () => { const scriptCases = await loadCases("script"); const caseEntry = scriptCases.find( diff --git a/ai_evals/core/models.ts b/ai_evals/core/models.ts index 9cc0ab0597..82f3b3f69b 100644 --- a/ai_evals/core/models.ts +++ b/ai_evals/core/models.ts @@ -145,7 +145,7 @@ export function resolveEvalModel(mode: EvalMode, alias?: string): EvalModelSpec export function getEvalModelHelpText(): string { return EVAL_MODELS.map((model) => { const modes = [ - ...(model.frontend ? ["flow", "script", "app"] : []), + ...(model.frontend ? ["flow", "script", "app", "global"] : []), ...(model.cli ? ["cli"] : []), ]; return ` ${model.id.padEnd(8)} ${model.label} (${modes.join(", ")})`; diff --git a/ai_evals/core/types.ts b/ai_evals/core/types.ts index dfc6882f84..2b42a0dfc5 100644 --- a/ai_evals/core/types.ts +++ b/ai_evals/core/types.ts @@ -1,4 +1,4 @@ -export const EVAL_MODES = ["cli", "flow", "script", "app"] as const; +export const EVAL_MODES = ["cli", "flow", "script", "app", "global"] as const; export type EvalMode = (typeof EVAL_MODES)[number]; @@ -108,6 +108,27 @@ export interface AppValidationSpec { forbiddenAppContent?: string[]; } +export interface GlobalDraftRequirement { + type: string; + path: string; + triggerKind?: string; + language?: string; + summaryIncludes?: string[]; + valueIncludes?: string[]; + valueExcludes?: string[]; +} + +export interface GlobalValidationSpec { + draftCountAtLeast?: number; + draftCountExactly?: number; + requiredDrafts?: GlobalDraftRequirement[]; + forbiddenDrafts?: Array<{ + type: string; + path: string; + triggerKind?: string; + }>; +} + export interface CliValidationSpec { requiredSkills?: string[]; forbiddenSkills?: string[]; @@ -136,10 +157,11 @@ export interface ToolCallArgumentRule { export interface ToolValidationSpec { requiredToolsUsed?: string[]; + forbiddenToolsUsed?: string[]; toolCallArgs?: ToolCallArgumentRule[]; } -export type EvalValidationSpec = FlowValidationSpec | AppValidationSpec; +export type EvalValidationSpec = FlowValidationSpec | AppValidationSpec | GlobalValidationSpec; export interface EvalCase { id: string; diff --git a/ai_evals/core/validators.test.ts b/ai_evals/core/validators.test.ts index 406172b955..d2a6e954bb 100644 --- a/ai_evals/core/validators.test.ts +++ b/ai_evals/core/validators.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "bun:test"; import { validateAppState, validateCliWorkspace, + validateGlobalState, validateScriptState, validateToolExpectations, } from "./validators"; @@ -117,6 +118,230 @@ describe("validateToolExpectations", () => { details: 'rejected prefixes: schedules/; values: "schedules/greet_user_daily"', }); }); + + it("rejects forbidden tool usage", () => { + const checks = validateToolExpectations({ + run: { + success: true, + actual: {}, + assistantMessageCount: 1, + toolCallCount: 1, + toolsUsed: ["write_script", "deploy_workspace_item"], + skillsInvoked: [], + }, + toolExpect: { + forbiddenToolsUsed: ["deploy_workspace_item"], + }, + }); + + expect(checks).toContainEqual({ + name: "does not use deploy_workspace_item", + passed: false, + details: "tools used: write_script, deploy_workspace_item", + }); + }); +}); + +describe("validateGlobalState", () => { + it("accepts a required script draft", () => { + const checks = validateGlobalState({ + actual: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + value: + "export async function main(name: string) {\n return `Hello, ${name}!`\n}\n", + isDraft: true, + }, + ], + }, + validate: { + draftCountExactly: 1, + requiredDrafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + valueIncludes: ["Hello"], + }, + ], + }, + }); + + expect(checks.every((check) => check.passed)).toBe(true); + }); + + it("fails when a required draft is missing", () => { + const checks = validateGlobalState({ + actual: { + drafts: [], + }, + validate: { + requiredDrafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + }, + ], + }, + }); + + expect(checks).toContainEqual({ + name: "global includes script draft f/evals/global/greet_user", + passed: false, + details: "drafts: none", + }); + }); + + it("does not require a TypeScript entrypoint for non-TypeScript script drafts", () => { + const checks = validateGlobalState({ + actual: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_python", + language: "python3", + value: "def main(name: str):\n return f'Hello, {name}!'\n", + isDraft: true, + }, + ], + }, + }); + + expect(checks.some((check) => check.name.includes("exports entrypoint"))).toBe( + false + ); + expect(checks.every((check) => check.passed)).toBe(true); + }); + + it("allows read-only global cases without draft expectations", () => { + const checks = validateGlobalState({ + actual: { + drafts: [], + }, + }); + + expect( + checks.some( + (check) => check.name === "global produced at least one draft" + ) + ).toBe(false); + expect(checks.every((check) => check.passed)).toBe(true); + }); + + it("matches expected global draft fixtures", () => { + const checks = validateGlobalState({ + actual: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + value: + "export async function main(name: string) {\r\n return `Hello, ${name}!`\r\n}\r\n", + isDraft: true, + }, + ], + }, + expected: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + value: + "export async function main(name: string) {\n return `Hello, ${name}!`\n}\n", + isDraft: true, + }, + ], + }, + }); + + expect(checks).toContainEqual({ + name: "global drafts match expected", + passed: true, + }); + }); + + it("fails when expected global draft fixtures differ", () => { + const checks = validateGlobalState({ + actual: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + value: + "export async function main(name: string) {\n return `Hello, ${name}!`\n}\n", + isDraft: true, + }, + ], + }, + expected: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + value: + "export async function main(name: string) {\n return `Bonjour, ${name}!`\n}\n", + isDraft: true, + }, + ], + }, + }); + + const expectedMatchCheck = checks.find( + (check) => check.name === "global drafts match expected" + ); + expect(expectedMatchCheck?.passed).toBe(false); + expect(expectedMatchCheck?.details).toContain( + "script:f/evals/global/greet_user value differs" + ); + expect(expectedMatchCheck?.details).toContain("Hello"); + expect(expectedMatchCheck?.details).toContain("Bonjour"); + }); + + it("explains expected global draft metadata mismatches", () => { + const checks = validateGlobalState({ + actual: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + value: + "export async function main(name: string) {\n return `Hello, ${name}!`\n}\n", + isDraft: true, + }, + ], + }, + expected: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "python3", + value: + "export async function main(name: string) {\n return `Hello, ${name}!`\n}\n", + isDraft: true, + }, + ], + }, + }); + + const expectedMatchCheck = checks.find( + (check) => check.name === "global drafts match expected" + ); + expect(expectedMatchCheck?.passed).toBe(false); + expect(expectedMatchCheck?.details).toContain( + "script:f/evals/global/greet_user language differs" + ); + expect(expectedMatchCheck?.details).toContain('actual="bun"'); + expect(expectedMatchCheck?.details).toContain('expected="python3"'); + }); }); describe("validateAppState", () => { diff --git a/ai_evals/core/validators.ts b/ai_evals/core/validators.ts index e690b3d7eb..4f59368113 100644 --- a/ai_evals/core/validators.ts +++ b/ai_evals/core/validators.ts @@ -6,6 +6,7 @@ import type { CliTrace, CliValidationSpec, FlowValidationSpec, + GlobalValidationSpec, ModeRunOutput, ToolValidationSpec, } from "./types"; @@ -51,6 +52,20 @@ export interface AppDatatableState { error?: string; } +export interface GlobalDraftState { + drafts: GlobalDraft[]; +} + +export interface GlobalDraft { + type: string; + path: string; + triggerKind?: string; + summary?: string; + language?: string; + value?: unknown; + isDraft?: boolean; +} + const TS_LIKE_LANGUAGES = new Set(["bun", "deno", "nativets", "bunnative", "ts", "typescript"]); const CONTROL_FLOW_MODULE_TYPES = new Set(["branchone", "branchall", "forloopflow", "whileloopflow"]); @@ -154,6 +169,16 @@ export function validateToolExpectations(input: { ); } + for (const toolName of expect.forbiddenToolsUsed ?? []) { + checks.push( + check( + `does not use ${toolName}`, + !input.run.toolsUsed.includes(toolName), + `tools used: ${input.run.toolsUsed.join(", ") || "none"}` + ) + ); + } + for (const rule of expect.toolCallArgs ?? []) { const calls = toolCallDetails.filter((call) => call.name === rule.tool); checks.push( @@ -202,6 +227,161 @@ export function validateToolExpectations(input: { return checks; } +export function validateGlobalState(input: { + actual: GlobalDraftState; + expected?: GlobalDraftState; + validate?: GlobalValidationSpec; +}): BenchmarkCheck[] { + const drafts = input.actual.drafts ?? []; + const checks: BenchmarkCheck[] = []; + + // Read-only global cases are valid; only enforce draft production when the + // case explicitly asks for draft output. + if (globalValidationExpectsDrafts(input)) { + checks.push( + check( + "global produced at least one draft", + drafts.length > 0, + `drafts=${drafts.length}` + ) + ); + } + + checks.push( + check( + "all global outputs are drafts", + drafts.every((draft) => draft.isDraft === true), + summarizeGlobalDrafts(drafts) + ) + ); + + for (const draft of drafts) { + if (draft.type !== "script" || typeof draft.value !== "string") { + continue; + } + + const language = (draft.language ?? "bun").toLowerCase(); + const syntaxErrors = getScriptSyntaxErrors(draft.value, language); + if (TS_LIKE_LANGUAGES.has(language)) { + checks.push( + check( + `script draft ${draft.path} exports entrypoint`, + hasSupportedEntrypoint(draft.value) + ) + ); + } + checks.push( + check( + `script draft ${draft.path} has no syntax errors`, + syntaxErrors.length === 0, + summarizeProblems(syntaxErrors) + ) + ); + } + + if (input.expected) { + checks.push( + check( + "global drafts match expected", + globalDraftStatesEqual(input.actual, input.expected), + describeGlobalDraftStateMismatch(input.actual, input.expected) + ) + ); + } + + const validate = input.validate; + if (!validate) { + return checks; + } + + if (validate.draftCountAtLeast !== undefined) { + checks.push( + check( + `global includes at least ${validate.draftCountAtLeast} draft(s)`, + drafts.length >= validate.draftCountAtLeast, + `drafts=${drafts.length}` + ) + ); + } + + if (validate.draftCountExactly !== undefined) { + checks.push( + check( + `global includes exactly ${validate.draftCountExactly} draft(s)`, + drafts.length === validate.draftCountExactly, + `drafts=${drafts.length}` + ) + ); + } + + for (const required of validate.requiredDrafts ?? []) { + const draft = findGlobalDraft(drafts, required.type, required.path, required.triggerKind); + checks.push( + check( + `global includes ${required.type} draft ${required.path}`, + Boolean(draft), + summarizeGlobalDrafts(drafts) + ) + ); + if (!draft) { + continue; + } + + if (required.language !== undefined) { + checks.push( + check( + `${required.type} draft ${required.path} uses ${required.language}`, + draft.language === required.language, + `language=${draft.language ?? "(none)"}` + ) + ); + } + + for (const snippet of required.summaryIncludes ?? []) { + checks.push( + check( + `${required.type} draft ${required.path} summary includes '${snippet}'`, + normalizeText(draft.summary ?? "").includes(normalizeText(snippet)), + `summary=${draft.summary ?? ""}` + ) + ); + } + + const valueText = stringifyGlobalDraftValue(draft.value); + for (const snippet of required.valueIncludes ?? []) { + checks.push( + check( + `${required.type} draft ${required.path} value includes '${snippet}'`, + normalizeText(valueText).includes(normalizeText(snippet)), + truncateForDetails(valueText) + ) + ); + } + + for (const snippet of required.valueExcludes ?? []) { + checks.push( + check( + `${required.type} draft ${required.path} value excludes '${snippet}'`, + !normalizeText(valueText).includes(normalizeText(snippet)), + truncateForDetails(valueText) + ) + ); + } + } + + for (const forbidden of validate.forbiddenDrafts ?? []) { + checks.push( + check( + `global does not include ${forbidden.type} draft ${forbidden.path}`, + !findGlobalDraft(drafts, forbidden.type, forbidden.path, forbidden.triggerKind), + summarizeGlobalDrafts(drafts) + ) + ); + } + + return checks; +} + export function validateAppState(input: { actual: AppFilesState; initial?: AppFilesState; @@ -433,6 +613,202 @@ function summarizeProblems(problems: string[], limit = 5): string | undefined { return `${problems.slice(0, limit).join("; ")}; ...and ${problems.length - limit} more`; } +function findGlobalDraft( + drafts: GlobalDraft[], + type: string, + path: string, + triggerKind?: string +): GlobalDraft | undefined { + return drafts.find( + (draft) => + draft.type === type && + draft.path === path && + (triggerKind === undefined || draft.triggerKind === triggerKind) + ); +} + +function summarizeGlobalDrafts(drafts: GlobalDraft[]): string { + const summary = drafts + .map((draft) => formatGlobalDraftKey(draft)) + .join(", "); + return `drafts: ${summary || "none"}`; +} + +function formatGlobalDraftKey(draft: GlobalDraft): string { + return `${draft.type}${draft.triggerKind ? `:${draft.triggerKind}` : ""}:${draft.path}`; +} + +function globalValidationExpectsDrafts(input: { + expected?: GlobalDraftState; + validate?: GlobalValidationSpec; +}): boolean { + const validate = input.validate; + return ( + (input.expected?.drafts?.length ?? 0) > 0 || + (validate?.requiredDrafts?.length ?? 0) > 0 || + (validate?.draftCountAtLeast ?? 0) > 0 || + (validate?.draftCountExactly ?? 0) > 0 + ); +} + +function globalDraftStatesEqual(left: GlobalDraftState, right: GlobalDraftState): boolean { + return ( + JSON.stringify(canonicalizeGlobalDrafts(left.drafts ?? [])) === + JSON.stringify(canonicalizeGlobalDrafts(right.drafts ?? [])) + ); +} + +function describeGlobalDraftStateMismatch( + actual: GlobalDraftState, + expected: GlobalDraftState +): string { + const actualDrafts = actual.drafts ?? []; + const expectedDrafts = expected.drafts ?? []; + const actualByKey = new Map( + actualDrafts.map((draft) => [globalDraftSortKey(draft), draft] as const) + ); + const expectedByKey = new Map( + expectedDrafts.map((draft) => [globalDraftSortKey(draft), draft] as const) + ); + + for (const key of Array.from(expectedByKey.keys()).sort()) { + const expectedDraft = expectedByKey.get(key); + if (expectedDraft && !actualByKey.has(key)) { + return `missing expected draft ${formatGlobalDraftKey(expectedDraft)}; actual=${summarizeGlobalDrafts(actualDrafts)}`; + } + } + + for (const key of Array.from(actualByKey.keys()).sort()) { + const actualDraft = actualByKey.get(key); + if (actualDraft && !expectedByKey.has(key)) { + return `unexpected draft ${formatGlobalDraftKey(actualDraft)}; expected=${summarizeGlobalDrafts(expectedDrafts)}`; + } + } + + for (const key of Array.from(expectedByKey.keys()).sort()) { + const actualDraft = actualByKey.get(key); + const expectedDraft = expectedByKey.get(key); + if (!actualDraft || !expectedDraft) { + continue; + } + + const fieldMismatch = describeGlobalDraftFieldMismatch( + formatGlobalDraftKey(expectedDraft), + actualDraft, + expectedDraft + ); + if (fieldMismatch) { + return fieldMismatch; + } + } + + return `actual=${summarizeGlobalDrafts(actualDrafts)}; expected=${summarizeGlobalDrafts(expectedDrafts)}`; +} + +function describeGlobalDraftFieldMismatch( + key: string, + actual: GlobalDraft, + expected: GlobalDraft +): string | undefined { + const fields: Array<"language" | "summary" | "value" | "isDraft"> = [ + "language", + "summary", + "value", + "isDraft", + ]; + + for (const field of fields) { + const actualValue = comparableGlobalDraftFieldValue(actual, field); + const expectedValue = comparableGlobalDraftFieldValue(expected, field); + if (JSON.stringify(actualValue) === JSON.stringify(expectedValue)) { + continue; + } + + return `${key} ${field} differs: actual=${formatGlobalDraftFieldValue( + actualValue + )}; expected=${formatGlobalDraftFieldValue(expectedValue)}`; + } + + return undefined; +} + +function comparableGlobalDraftFieldValue( + draft: GlobalDraft, + field: "language" | "summary" | "value" | "isDraft" +): unknown { + if (field === "summary" && typeof draft.summary === "string") { + return normalizeText(draft.summary); + } + if (field === "value" && typeof draft.value === "string") { + return normalizeText(draft.value); + } + if (field === "value") { + return canonicalizeJsonValue(draft.value); + } + return draft[field]; +} + +function formatGlobalDraftFieldValue(value: unknown): string { + if (value === undefined) { + return "(missing)"; + } + return truncateForDetails(JSON.stringify(value), 300); +} + +function canonicalizeGlobalDrafts(drafts: GlobalDraft[]): unknown[] { + return drafts + .slice() + .sort((left, right) => globalDraftSortKey(left).localeCompare(globalDraftSortKey(right))) + .map((draft) => + canonicalizeJsonValue({ + type: draft.type, + path: draft.path, + triggerKind: draft.triggerKind, + language: draft.language, + summary: + typeof draft.summary === "string" ? normalizeText(draft.summary) : draft.summary, + value: + typeof draft.value === "string" + ? normalizeText(draft.value) + : canonicalizeJsonValue(draft.value), + isDraft: draft.isDraft, + }) + ); +} + +function globalDraftSortKey(draft: GlobalDraft): string { + return `${draft.type}:${draft.triggerKind ?? ""}:${draft.path}`; +} + +function canonicalizeJsonValue(value: unknown): unknown { + if (Array.isArray(value)) { + return value.map(canonicalizeJsonValue); + } + if (value && typeof value === "object") { + return Object.fromEntries( + Object.entries(value as Record) + .sort(([left], [right]) => left.localeCompare(right)) + .map(([key, nested]) => [key, canonicalizeJsonValue(nested)]) + ); + } + return value; +} + +function stringifyGlobalDraftValue(value: unknown): string { + if (typeof value === "string") { + return value; + } + return JSON.stringify(value ?? null, null, 2); +} + +function truncateForDetails(value: string, maxLength = 500): string { + const normalized = value.replace(/\s+/g, " ").trim(); + if (normalized.length <= maxLength) { + return normalized; + } + return `${normalized.slice(0, Math.max(0, maxLength - 3))}...`; +} + function validateCliExpectations( assistantOutput: string, trace: CliTrace | undefined, diff --git a/ai_evals/fixtures/frontend/global/initial/format_greeting_script.json b/ai_evals/fixtures/frontend/global/initial/format_greeting_script.json new file mode 100644 index 0000000000..e66eee2ed2 --- /dev/null +++ b/ai_evals/fixtures/frontend/global/initial/format_greeting_script.json @@ -0,0 +1,23 @@ +{ + "workspace": { + "scripts": [ + { + "path": "f/evals/global/format_greeting", + "summary": "Format a greeting for a provided name", + "description": "Returns a plain greeting for the provided name.", + "language": "bun", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "name": { + "type": "string" + } + }, + "required": ["name"] + }, + "content": "export async function main(name: string) {\n return `Hello, ${name}`\n}\n" + } + ] + } +} diff --git a/ai_evals/modes/global.ts b/ai_evals/modes/global.ts new file mode 100644 index 0000000000..d68df9f5f8 --- /dev/null +++ b/ai_evals/modes/global.ts @@ -0,0 +1,81 @@ +import { readFile } from "node:fs/promises"; +import { runGlobalEval } from "../adapters/frontend/core/global/globalEvalRunner"; +import type { BenchmarkWorkspaceRunnables } from "../adapters/frontend/mockBackend"; +import type { FrontendEvalModelConfig } from "../core/models"; +import type { BenchmarkArtifactFile, GlobalValidationSpec, ModeRunner } from "../core/types"; +import { validateGlobalState, type GlobalDraftState } from "../core/validators"; +import type { WindmillBackendSettings } from "../core/windmillBackendSettings"; +import { getFrontendApiKey } from "./frontendCommon"; + +export interface GlobalInitialFixture { + workspace?: BenchmarkWorkspaceRunnables; +} + +export function createGlobalModeRunner( + modelConfig: FrontendEvalModelConfig, + backendSettings: WindmillBackendSettings, +): ModeRunner { + return { + mode: "global", + concurrency: 3, + judgeThreshold: 80, + async loadInitial(path) { + return path ? await loadGlobalInitialFixture(path) : undefined; + }, + async loadExpected(path) { + return path ? await loadGlobalExpectedFixture(path) : undefined; + }, + async run(prompt, initial, context) { + const result = await runGlobalEval( + prompt, + getFrontendApiKey(modelConfig.provider), + { + workspaceFixtures: initial?.workspace, + maxIterations: context.evalCase?.runtime?.maxTurns, + provider: modelConfig.provider, + model: modelConfig.model, + backend: backendSettings, + runContext: context, + }, + ); + + return { + success: result.success, + actual: result.state, + error: result.error, + assistantMessageCount: result.assistantMessageCount, + toolCallCount: result.toolCallCount, + toolsUsed: result.toolsUsed, + toolCallDetails: result.toolCallDetails, + skillsInvoked: [], + tokenUsage: result.tokenUsage, + }; + }, + validate({ evalCase, actual, expected }) { + return validateGlobalState({ + actual, + expected, + validate: evalCase.validate as GlobalValidationSpec | undefined, + }); + }, + buildArtifacts(actual): BenchmarkArtifactFile[] { + return [ + { + path: "global-drafts.json", + content: JSON.stringify(actual, null, 2) + "\n", + }, + ]; + }, + }; +} + +async function loadGlobalInitialFixture(path: string): Promise { + const parsed = JSON.parse(await readFile(path, "utf8")) as GlobalInitialFixture; + return { + workspace: parsed.workspace ?? {}, + }; +} + +async function loadGlobalExpectedFixture(path: string): Promise { + return JSON.parse(await readFile(path, "utf8")) as GlobalDraftState; +} From 2ec1863340e759bba3408dbc4f41b16912b959ea Mon Sep 17 00:00:00 2001 From: hugocasa Date: Wed, 13 May 2026 13:35:30 +0200 Subject: [PATCH 05/24] fix: scope promotion-mode debounce key per repo (#9145) * test(git-sync): regression tests for secondary promotion repos Adds two integration tests that reproduce the bug where a second promotion-mode repo's deployment callback was silently dropped via debounce-key collision, plus the EE ref bump that includes the fix. Updates the two existing promotion-mode debounce-key tests to expect the new repo-namespaced key shape. * test(git-sync): drop redundant distinct-debounce-keys test The behavior test (`test_two_promotion_repos_both_enqueue_callback`) already covers the same regression one layer up: if the debounce keys collide, one callback gets marked skipped, which the behavior test catches. * chore: update ee-repo-ref to 7a32388adaa37eb1dd1820b40e140ff1877110f2 This commit updates the EE repository reference after PR #572 was merged in windmill-ee-private. Previous ee-repo-ref: dbf26f5e4c01c0de536f606679be46eb316aaf31 New ee-repo-ref: 7a32388adaa37eb1dd1820b40e140ff1877110f2 Automated by sync-ee-ref workflow. --------- Co-authored-by: windmill-internal-app[bot] --- backend/ee-repo-ref.txt | 2 +- .../tests/workspace_dependencies_git_sync.rs | 171 +++++++++++++++++- 2 files changed, 167 insertions(+), 6 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 8a34b69a0e..cad5dee291 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -f9494c6320bb5fd07c1e9e09734b7fd5fbe7aa38 +7a32388adaa37eb1dd1820b40e140ff1877110f2 diff --git a/backend/windmill-api-integration-tests/tests/workspace_dependencies_git_sync.rs b/backend/windmill-api-integration-tests/tests/workspace_dependencies_git_sync.rs index 0049280aec..a5af874f8c 100644 --- a/backend/windmill-api-integration-tests/tests/workspace_dependencies_git_sync.rs +++ b/backend/windmill-api-integration-tests/tests/workspace_dependencies_git_sync.rs @@ -573,8 +573,10 @@ async fn test_promotion_individual_branch_debounces_per_path( // Wait for both deployment callbacks to resolve a debounce key. The // alpha key is polled first as a warm-up, then we also wait for beta // so the assertions don't race the second spawned callback. - let expected_alpha = "git_sync:script:f/target/alpha"; - let expected_beta = "git_sync:script:f/target/beta"; + // Keys are namespaced by the repo's resource path so multiple promotion + // repos don't collide on the same key. + let expected_alpha = "git_sync:$res:u/test-user/test_git_repo:script:f/target/alpha"; + let expected_beta = "git_sync:$res:u/test-user/test_git_repo:script:f/target/beta"; let _ = wait_for_debounce_key(&db, expected_alpha, Duration::from_secs(5)).await?; let keys = wait_for_debounce_key(&db, expected_beta, Duration::from_secs(5)).await?; assert!( @@ -589,6 +591,162 @@ async fn test_promotion_individual_branch_debounces_per_path( Ok(()) } +/// Create a second git repository resource for multi-repo tests. +#[allow(dead_code)] +async fn create_second_git_repo_resource(db: &Pool) -> anyhow::Result<()> { + sqlx::query( + r#" + INSERT INTO resource (workspace_id, path, value, resource_type, extra_perms, created_by) + VALUES ('test-workspace', 'u/test-user/test_git_repo_2', $1::jsonb, 'git_repository', '{}'::jsonb, 'test-user') + ON CONFLICT (workspace_id, path) DO NOTHING + "#, + ) + .bind(json!({ + "url": "https://github.com/test/test2.git", + "branch": "main", + "token": "test-token-2" + })) + .execute(db) + .await?; + Ok(()) +} + +/// Configure git sync with TWO promotion-mode repositories pointing at distinct +/// git repo resources. Both repos use the same sync script and the same item +/// filters — they only differ in the repo they target. +#[allow(dead_code)] +async fn setup_two_promotion_repos_config( + db: &Pool, + sync_script_path: &str, + group_by_folder: bool, +) -> anyhow::Result<()> { + let git_sync_config = json!({ + "include_type": ["script"], + "include_path": ["**"], + "repositories": [ + { + "script_path": sync_script_path, + "git_repo_resource_path": "$res:u/test-user/test_git_repo", + "use_individual_branch": true, + "group_by_folder": group_by_folder + }, + { + "script_path": sync_script_path, + "git_repo_resource_path": "$res:u/test-user/test_git_repo_2", + "use_individual_branch": true, + "group_by_folder": group_by_folder + } + ] + }); + + sqlx::query!( + "UPDATE workspace_settings SET git_sync = $1 WHERE workspace_id = $2", + git_sync_config, + "test-workspace" + ) + .execute(db) + .await?; + + Ok(()) +} + +/// Regression test for: when two promotion-mode repos are configured with the +/// same `use_individual_branch=true` settings (i.e. a primary and a secondary +/// promotion repo), deploying a single script must enqueue ONE deployment +/// callback per repo. Both callbacks must remain in the queue — neither may +/// be debounced into oblivion by the other. +/// +/// The bug this guards against: the debounce key for promotion mode was +/// derived only from (path_type, path) and omitted any per-repo identifier, +/// so the second repo's push hit ON CONFLICT in `upsert_debounce_key` and +/// `complete_debounced_job` flagged the first repo's job as `status='skipped'` +/// — silently dropping one of the two pushes. +#[cfg(all(feature = "enterprise", feature = "private"))] +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_two_promotion_repos_both_enqueue_callback(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + + create_folder(&db, "28103").await?; + create_folder(&db, "target").await?; + create_git_repo_resource(&db).await?; + create_second_git_repo_resource(&db).await?; + let sync_script_path = "f/28103/test_sync_two_promotion_repos"; + create_sync_script(&db, sync_script_path).await?; + setup_two_promotion_repos_config(&db, sync_script_path, false).await?; + + let (client, _port, _server) = init_client(db.clone()).await; + + // Deploy a single script — handle_deployment_metadata should iterate + // both repos and create one callback job per repo. + create_test_script(&client, "f/target/alpha").await?; + + // Both callbacks should reach the queue. With the bug, only one survives + // (the other is moved to v2_job_completed with status='skipped'). + let deadline = tokio::time::Instant::now() + Duration::from_secs(5); + let mut last_jobs: Vec = vec![]; + loop { + last_jobs = + get_deployment_callback_jobs(&db, sync_script_path, Duration::from_millis(200)).await?; + if last_jobs.len() >= 2 { + break; + } + if tokio::time::Instant::now() >= deadline { + break; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + + // Inspect what landed in v2_job_completed so failure messages explain why. + let skipped: Vec<(uuid::Uuid, String)> = sqlx::query_as( + r#" + SELECT c.id, c.status::text + FROM v2_job_completed c + JOIN v2_job j ON j.id = c.id + WHERE j.runnable_path = $1 AND j.kind = 'deploymentcallback' + "#, + ) + .bind(sync_script_path) + .fetch_all(&db) + .await?; + + assert_eq!( + last_jobs.len(), + 2, + "expected 2 deployment callback jobs in v2_job_queue (one per promotion repo), got {} queued + {:?} completed", + last_jobs.len(), + skipped, + ); + + // Per-repo args sanity check: the two jobs must target different repos. + let mut repo_paths: Vec = last_jobs + .iter() + .filter_map(|j| { + j.args + .as_ref() + .and_then(|a| a.get("repo_url_resource_path")) + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) + }) + .collect(); + repo_paths.sort(); + repo_paths.dedup(); + assert_eq!( + repo_paths.len(), + 2, + "expected callbacks to target two distinct repos, got: {:?}", + last_jobs.iter().map(|j| &j.args).collect::>() + ); + + // No callback should have been silently skipped via debouncing collision. + assert!( + skipped.iter().all(|(_, s)| s != "skipped"), + "no deployment callback should be marked skipped, got: {:?}", + skipped, + ); + + Ok(()) +} + /// Promotion mode with group_by_folder: items destined for the same per-folder /// branch must share one debounce key so they accumulate into a single sync /// job; scripts in different folders must get distinct keys. @@ -615,8 +773,9 @@ async fn test_promotion_group_by_folder_debounces_per_folder( // One in a different folder — should get its own key. create_test_script(&client, "f/other/gamma").await?; - let expected_grouped = "git_sync:folder:f/grouped"; - let expected_other = "git_sync:folder:f/other"; + // Keys are namespaced by the repo's resource path. + let expected_grouped = "git_sync:$res:u/test-user/test_git_repo:folder:f/grouped"; + let expected_other = "git_sync:$res:u/test-user/test_git_repo:folder:f/other"; // Wait for BOTH folder keys to appear, not just the first one. let keys = wait_for_debounce_key(&db, expected_other, Duration::from_secs(5)).await?; assert!( @@ -629,7 +788,9 @@ async fn test_promotion_group_by_folder_debounces_per_folder( ); // Paths within the same folder must NOT leak as their own keys. assert!( - !keys.iter().any(|k| k.starts_with("git_sync:script:")), + !keys + .iter() + .any(|k| k.contains(":script:f/grouped/") || k.contains(":script:f/other/")), "group_by_folder mode should not emit per-path keys, got: {keys:?}" ); From 818cb31fbc731fa5c70ddf5942bb37bc4bc56e4d Mon Sep 17 00:00:00 2001 From: "windmill-internal-app[bot]" <217088191+windmill-internal-app[bot]@users.noreply.github.com> Date: Wed, 13 May 2026 12:32:35 +0000 Subject: [PATCH 06/24] fix: send flow push-loop ping outside transaction so zombie monitor sees it (#9136) * fix: send flow push-loop ping outside transaction so zombie monitor sees it * fix: keep flow push-loop ping using now() with reusable sqlx cache --------- Co-authored-by: windmill-internal-app[bot] <1429786+windmill-internal-app[bot]@users.noreply.github.com> Co-authored-by: Ruben Fiszel --- backend/windmill-worker/src/worker_flow.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/backend/windmill-worker/src/worker_flow.rs b/backend/windmill-worker/src/worker_flow.rs index 53c94a7458..68ca83c270 100644 --- a/backend/windmill-worker/src/worker_flow.rs +++ b/backend/windmill-worker/src/worker_flow.rs @@ -3919,11 +3919,14 @@ async fn push_next_flow_job( for (i, payload_tag) in job_payloads.into_iter().enumerate() { if i % 100 == 0 && i != 0 { tracing::info!(id = %flow_job.id, root_id = %job_root, "pushed (non-commited yet) first {i} subflows of {len}"); + // Ping on the pool, outside `tx`, so the zombie flow monitor sees it before the + // push transaction commits — otherwise large parallel pushes can be flagged as + // zombie and trigger a cancel/push deadlock. sqlx::query!( - "UPDATE v2_job_runtime SET ping = now() WHERE id = $1 AND ping < now()", + "UPDATE v2_job_runtime SET ping = now() WHERE id = $1", flow_job.id, ) - .execute(&mut *tx) + .execute(db) .warn_after_seconds(3) .await?; } From d243e0cde899b6d34ee6d54be76bd46f08c4b64a Mon Sep 17 00:00:00 2001 From: centdix <40307056+centdix@users.noreply.github.com> Date: Wed, 13 May 2026 14:38:49 +0200 Subject: [PATCH 07/24] align global flow tool arguments (#9146) --- ai_evals/cases/global.yaml | 34 +++++ .../copilot/chat/global/core.test.ts | 77 ++++++++-- .../components/copilot/chat/global/core.ts | 144 +++++++++++------- 3 files changed, 191 insertions(+), 64 deletions(-) diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml index 709081942b..b4526f8a85 100644 --- a/ai_evals/cases/global.yaml +++ b/ai_evals/cases/global.yaml @@ -53,3 +53,37 @@ - uppercases the provided name in the greeting - returns a message ending with an exclamation mark - does not deploy or save the draft to the workspace + +- id: global-test3-flow-create + prompt: |- + Create a draft flow at `f/evals/global/sum_numbers`. + It should take two numeric inputs, `a` and `b`, and return their sum. + Leave it as an AI draft only; do not deploy or save it. + runtime: + maxTurns: 8 + validate: + draftCountExactly: 1 + requiredDrafts: + - type: flow + path: f/evals/global/sum_numbers + valueIncludes: + - modules + - rawscript + - flow_input.a + - flow_input.b + toolExpect: + requiredToolsUsed: + - write_flow + forbiddenToolsUsed: + - deploy_workspace_item + - delete_workspace_item + toolCallArgs: + - tool: write_flow + field: modules + stringStartsWithAnyOf: + - "[" + judgeChecklist: + - creates a flow draft at f/evals/global/sum_numbers + - the flow accepts numeric inputs a and b + - the flow returns the sum of a and b + - the result stays as an AI draft and is not deployed or saved to the workspace diff --git a/frontend/src/lib/components/copilot/chat/global/core.test.ts b/frontend/src/lib/components/copilot/chat/global/core.test.ts index 2664c1495d..2732db4dac 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -113,19 +113,17 @@ describe('global AI tools', () => { await callGlobalTool('write_flow', { path: 'f/flows/empty-module', summary: 'Flow with empty module', - value: JSON.stringify({ - modules: [ - { - id: 'empty_step', - value: { - type: 'rawscript', - language: 'bun', - content: '', - input_transforms: {} - } + modules: JSON.stringify([ + { + id: 'empty_step', + value: { + type: 'rawscript', + language: 'bun', + content: '', + input_transforms: {} } - ] - }) + } + ]) }) const code = 'export async function main() {\n\treturn 42\n}' @@ -145,4 +143,59 @@ describe('global AI tools', () => { }) ).resolves.toBe(code) }) + + it('writes flows with flow-mode arguments and reads compact flow value', async () => { + const writeResult = JSON.parse( + await callGlobalTool('write_flow', { + path: 'f/flows/with-schema-and-groups', + summary: 'Flow with schema and groups', + modules: JSON.stringify([ + { + id: 'start', + summary: 'Start', + value: { + type: 'identity' + } + } + ]), + schema: JSON.stringify({ + type: 'object', + properties: { + name: { type: 'string' } + }, + required: ['name'] + }), + groups: JSON.stringify([{ summary: 'Main', start_id: 'start', end_id: 'start' }]) + }) + ) + + expect(writeResult.item.value.value).toBeUndefined() + + const raw = await callGlobalTool('read_workspace_item', { + type: 'flow', + path: 'f/flows/with-schema-and-groups' + }) + const item = JSON.parse(raw) + + expect(item.value).toMatchObject({ + modules: [ + { + id: 'start', + summary: 'Start', + value: { type: 'identity' } + } + ], + schema: { + type: 'object', + properties: { + name: { type: 'string' } + }, + required: ['name'] + }, + preprocessor_module: null, + failure_module: null, + groups: [{ summary: 'Main', start_id: 'start', end_id: 'start' }] + }) + expect(item.value.value).toBeUndefined() + }) }) diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index de18426a3c..210636e43d 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -37,6 +37,7 @@ import { import { applyEditableFlowJsonToFlow, buildEditableFlowJson, + type EditableFlowJson, validateEditableFlowJson } from '../flow/editableFlowJson' import { createInlineScriptSession } from '../flow/inlineScriptsUtils' @@ -59,7 +60,6 @@ import { type ToolCallbacks, type ToolDisplayAction } from '../shared' -import { flowModuleSchema, flowModulesSchema } from '../flow/openFlowZod.gen' import { resourceRequestSchema, scheduleRequestSchema, @@ -149,22 +149,6 @@ const writeScriptSchema = z.object({ content: z.string().describe('Full script source code.') }) -const flowValueSchema = z - .looseObject({ - modules: flowModulesSchema.describe('Sequential flow modules.'), - preprocessor_module: flowModuleSchema - .nullable() - .optional() - .describe( - "Optional preprocessor module with id 'preprocessor'. Runs before normal modules; cannot reference results.*." - ), - failure_module: flowModuleSchema - .nullable() - .optional() - .describe("Optional failure handler module with id 'failure'.") - }) - .describe('OpenFlow value: modules plus optional preprocessor_module and failure_module.') - const readFlowModuleCodeSchema = z.object({ path: z.string().describe('Workspace path of the flow.'), module_id: z @@ -184,24 +168,81 @@ const setFlowModuleCodeSchema = z.object({ code: z.string().describe('New script source. Replaces the module\'s value.content entirely.') }) -// `value` is taken as a JSON string rather than a typed object because the -// underlying flowValueSchema is recursive (modules can contain modules), which -// makes z.toJSONSchema emit $defs/$ref. Gemini's tools API rejects those -// keywords ("Unknown name $ref/$defs"). The string is parsed and validated -// against flowValueSchema inside the handler. Same trick as set_flow_json in -// chat/flow/core.ts (see comment on its schema). +// Flow structure fields are taken as JSON strings rather than typed objects +// because the underlying flow module schema is recursive (modules can contain +// modules), which makes z.toJSONSchema emit $defs/$ref. Gemini's tools API +// rejects those keywords ("Unknown name $ref/$defs"). Same trick as +// set_flow_json in chat/flow/core.ts. const writeFlowSchema = z.object({ path: z .string() .describe('Workspace path of the flow, e.g. f/folder/name or u/user/name.'), summary: z.string().optional().describe('Short human-readable summary.'), - value: z + modules: z.string().describe('JSON string containing the complete flow modules array.'), + schema: z .string() + .optional() + .nullable() + .describe('JSON string containing the flow input schema.'), + preprocessor_module: z + .string() + .optional() + .nullable() + .describe('JSON string containing the optional preprocessor module.'), + failure_module: z + .string() + .optional() + .nullable() + .describe('JSON string containing the optional failure module.'), + groups: z + .string() + .optional() + .nullable() .describe( - 'JSON string of the OpenFlow value object: { modules, preprocessor_module?, failure_module? }. Pass it as a JSON-encoded string, not a nested object.' + 'JSON string containing the optional array of semantic flow groups. Pass null to clear groups.' ) }) +function parseOptionalJsonArg(value: unknown, field: string): unknown { + if (value === undefined || value === null) { + return value + } + + try { + return typeof value === 'string' ? JSON.parse(value) : value + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + throw new Error(`Invalid JSON for ${field}: ${message}`) + } +} + +function editableFlowToDraftValue(editable: EditableFlowJson): FlowDraftValue { + const value: FlowValue = { + modules: editable.modules, + preprocessor_module: editable.preprocessor_module ?? undefined, + failure_module: editable.failure_module ?? undefined, + groups: editable.groups ?? undefined + } + return { + value, + schema: editable.schema, + groups: editable.groups + } +} + +function flowDraftAsEditableInput(flowDraft: FlowDraftValue): { + value: FlowValue + schema?: Record | null | undefined +} { + return { + value: + flowDraft.groups === undefined + ? flowDraft.value + : { ...flowDraft.value, groups: flowDraft.groups ?? undefined }, + schema: flowDraft.schema + } +} + const writeScheduleSchema = scheduleRequestSchema const writeTriggerSchema = z.object({ @@ -428,7 +469,7 @@ Important rules: - Use search_resource_types before write_resource to discover the resource_type name and the JSON Schema its value must match. - Use get_instructions before writing a script, flow, resource, or app. For scripts, pass the target language; when modifying, use the language from the item you read. - Schedules, triggers, and variables do not need get_instructions — their tool schemas describe every field. -- A workspace item is { type, path, summary?, language?, triggerKind?, value, isDraft }. For scripts, value is the source code string. For flows, value is { value: , schema, groups } so the inputs schema and groups round-trip through deploy. For schedules/triggers/resources/variables, value is the full request body for that type. For apps, value is { files, runnables, data?, policy?, custom_path? } with frontend file contents and backend runnable definitions. +- A workspace item is { type, path, summary?, language?, triggerKind?, value, isDraft }. For scripts, value is the source code string. For flows, read_workspace_item returns value as the compact flow object { modules, schema, preprocessor_module, failure_module, groups }; write_flow takes the same flow fields as top-level tool arguments plus path/summary. For schedules/triggers/resources/variables, value is the full request body for that type. For apps, value is { files, runnables, data?, policy?, custom_path? } with frontend file contents and backend runnable definitions. - Apps (raw apps): use list_workspace_items with types: ['app'] to find them, read_workspace_item with type 'app' for a metadata summary (file paths + runnable list, no contents), then read_app_file to read individual files. Edit with write_app_file / patch_app_file / delete_app_file for frontend files and write_app_runnable / delete_app_runnable for backend runnables. Frontend file paths start with "/" (e.g. /index.tsx). Backend inline runnables are addressed as "backend//main.{ts|py}". /wmill.d.ts is generated and cannot be written. - To create a new raw app, use init_app. Before calling it, confirm framework (react19 / react18 / svelte5 / vue), path, and summary with the user — do not silently default to react19, even though it is the recommended choice. - Apps cannot be deployed from chat. The app editor bundles JS/CSS before save; tell the user to open the app editor to deploy app drafts. @@ -496,7 +537,7 @@ function serializeWorkspaceItemForRead(item: WorkspaceItem): unknown { if (item.type !== 'flow' || !item.value) return item const flowDraft = item.value as FlowDraftValue const session = createInlineScriptSession() - const editable = buildEditableFlowJson(flowDraft, session) + const editable = buildEditableFlowJson(flowDraftAsEditableInput(flowDraft), session) return { type: 'flow', path: item.path, @@ -1074,8 +1115,9 @@ function getFlowInstructions(): string { return `# Global draft flow instructions - Global mode writes complete draft payloads only; it does not save, deploy, run, scaffold local files, or generate metadata. -- A flow draft is a workspace item: \`{ type: 'flow', path, summary?, value, isDraft }\` where \`value\` is \`{ value: , schema, groups }\`. The inputs schema and groups are kept alongside the OpenFlow value so deploy round-trips them. -- \`value.modules\` contains normal sequential modules. Use top-level \`value.preprocessor_module\` and \`value.failure_module\` for special modules; do not put \`preprocessor\` or \`failure\` in \`value.modules\`. +- \`write_flow\` mirrors flow mode's \`set_flow_json\`: pass \`path\`, optional \`summary\`, required \`modules\`, and optional \`schema\`, \`preprocessor_module\`, \`failure_module\`, and \`groups\`. The flow-structure arguments are JSON strings, matching the tool schema descriptions. +- \`read_workspace_item\` returns a compact flow \`value\` object with \`modules\`, \`schema\`, \`preprocessor_module\`, \`failure_module\`, and \`groups\`. +- \`modules\` contains normal sequential modules. Use top-level \`preprocessor_module\` and \`failure_module\` for special modules; do not put \`preprocessor\` or \`failure\` in \`modules\`. - Every module needs a stable unique \`id\` and a useful \`summary\` when the schema supports it. - Prefer path/script/flow modules when composing existing workspace logic. Use rawscript modules only when new inline code is needed. - When writing rawscript module code, call \`get_instructions\` with \`subject: "script"\` and the rawscript language first. @@ -1087,7 +1129,7 @@ function getFlowInstructions(): string { - \`read_flow_module_code(path, module_id)\` — returns the raw inline script content for one module. - \`set_flow_module_code(path, module_id, code)\` — overwrites that module's inline script content; saves to the AI draft. - Use \`patch_flow_json\` for *structural* edits: module ids, paths, input_transforms, branch arrangement, summaries, preprocessor/failure swaps, schema/groups. Use \`set_flow_module_code\` for changes inside a specific rawscript body. -- \`write_flow\` is for full overwrites / create-from-scratch. Its \`value\` argument is the **non-compact** OpenFlow value (rawscript content is the actual code, not a placeholder). +- \`write_flow\` is for full overwrites / create-from-scratch. Its \`modules\`, \`preprocessor_module\`, and \`failure_module\` arguments use **non-compact** flow modules (rawscript content is the actual code, not a placeholder). # Windmill flow authoring reference @@ -1271,35 +1313,29 @@ export const globalTools: Tool<{}>[] = [ def: createToolDef( writeFlowSchema, 'write_flow', - 'Create or overwrite an AI draft flow. Does not save or deploy. Read the existing flow first when overwriting. value must be a JSON-encoded string of the OpenFlow value object.' + 'Create or overwrite an AI draft flow. Does not save or deploy. Read the existing flow first when overwriting. Uses the same flow-structure arguments as set_flow_json plus path and summary.' ), showDetails: true, streamArguments: true, showFade: true, fn: async (ctx) => { const parsed = writeFlowSchema.parse(ctx.args) - let value: unknown - try { - value = JSON.parse(parsed.value) - } catch (error) { - const message = error instanceof Error ? error.message : String(error) - throw new Error(`Invalid JSON for value: ${message}`) - } - const validated = flowValueSchema.safeParse(value) - if (!validated.success) { - throw new Error( - `Invalid flow value: ${validated.error.issues - .slice(0, 5) - .map((i) => `${i.path.join('.')}: ${i.message}`) - .join('; ')}` - ) - } + const editable = validateEditableFlowJson({ + modules: parseOptionalJsonArg(parsed.modules, 'modules'), + schema: parseOptionalJsonArg(parsed.schema, 'schema'), + preprocessor_module: parseOptionalJsonArg( + parsed.preprocessor_module, + 'preprocessor_module' + ), + failure_module: parseOptionalJsonArg(parsed.failure_module, 'failure_module'), + groups: parseOptionalJsonArg(parsed.groups, 'groups') + }) return writeDraft( { type: 'flow', path: parsed.path, summary: parsed.summary, - value: { value: validated.data as FlowValue, schema: null, groups: null }, + value: editableFlowToDraftValue(editable), isDraft: true }, ctx @@ -1683,7 +1719,7 @@ async function patchFlowJson( // model uses set_flow_module_code to change inline script bodies. const base = await loadFlowDraftValue(path, ctx.workspace) const session = createInlineScriptSession() - const editable = buildEditableFlowJson(base.flow, session) + const editable = buildEditableFlowJson(flowDraftAsEditableInput(base.flow), session) const currentJson = JSON.stringify(editable) const updatedJson = findAndReplace( currentJson, @@ -1730,7 +1766,7 @@ async function readFlowModuleCode( }) const base = await loadFlowDraftValue(args.path, workspace) const session = createInlineScriptSession() - buildEditableFlowJson(base.flow, session) + buildEditableFlowJson(flowDraftAsEditableInput(base.flow), session) const content = session.get(args.module_id) if (content === undefined) { throw new Error( @@ -1753,7 +1789,7 @@ async function setFlowModuleCode( }) const base = await loadFlowDraftValue(args.path, workspace) const session = createInlineScriptSession() - const editable = buildEditableFlowJson(base.flow, session) + const editable = buildEditableFlowJson(flowDraftAsEditableInput(base.flow), session) if (!session.has(args.module_id)) { throw new Error( `Module "${args.module_id}" is not an inline rawscript in flow "${args.path}". Use patch_flow_json or write_flow for structural changes.` @@ -2342,6 +2378,10 @@ async function writeDraft(item: WorkspaceItem, ctx: WriteDraftCtx): Promise Date: Wed, 13 May 2026 13:04:31 +0000 Subject: [PATCH 08/24] fix(bun): pass --preserve-symlinks on unbundled execution (#9147) * fix(bun): pass --preserve-symlinks on unbundled execution Bun 1.2/1.3 moved its global package cache to a content-addressed layout and the installer now creates a single directory symlink from node_modules/ to the cache entry. Without --preserve-symlinks, Bun resolves modules from each file's realpath, so any require/import inside an installed package walks up from cache_nomount/bun/... and never finds the sibling deps living under /node_modules/. This manifested as e.g. ENOENT while resolving package 'zod/v3' from '/tmp/windmill/cache_nomount/bun/@langchain/core@1.1.44@@@1/dist/...' on //nobundling scripts that pull @langchain/core, even though zod is correctly installed alongside it in node_modules. The bundled execution path already had --preserve-symlinks since #4132 (needed because we symlink the cached bundle file into the job dir). The unbundled path didn't, because at the time Bun installed via per- file hardlinks and the realpath of node_modules entries was the job dir itself. The Bun installer's layout change made the flag necessary on the unbundled path as well. Add the flag to all three unbundled `bun run` invocations: - nsjail unbundled path - non-nsjail unbundled path - dedicated worker (always unbundled) This also fixes a latent bug on the first run of any bun script that imports a package whose internals reference siblings (the build_cache path runs unbundled this round while it builds the bundle for next time). Co-Authored-By: Claude Opus 4.7 (1M context) * test(bun): regression test for nobundling + transitive require resolution Adds an integration test that mirrors the original failure: a //nobundling script importing @langchain/core, which (in its CJS internals) does require('zod/v3'). Before --preserve-symlinks was added to the unbundled bun run invocations, this failed with: ENOENT while resolving package 'zod/v3' from '.../cache_nomount/bun/@langchain/core@@@@1/dist/runnables/base.js' The test covers the non-nsjail unbundled path. Reproducibility of the pre-fix failure depends on Bun's installer choosing the directory-symlink layout for the node_modules entry (the default on Bun 1.2/1.3+ with the new content-addressed global cache that produced the user's error). Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- backend/tests/bun_jobs.rs | 57 +++++++++++++++++++++ backend/windmill-worker/src/bun_executor.rs | 3 ++ 2 files changed, 60 insertions(+) diff --git a/backend/tests/bun_jobs.rs b/backend/tests/bun_jobs.rs index fa15866bca..d51e51f58b 100644 --- a/backend/tests/bun_jobs.rs +++ b/backend/tests/bun_jobs.rs @@ -577,6 +577,63 @@ export function main() { Ok(()) } +/// Regression test: a `//nobundling` script that pulls a package whose CJS +/// internals do bare-specifier `require()` of a sibling dependency. +/// +/// Before the `--preserve-symlinks` fix, Bun 1.2/1.3+ would follow the +/// directory symlink in `node_modules/@langchain/core` to its global cache +/// entry, walk parent dirs from the cache realpath, and fail to find +/// `node_modules/zod` — producing: +/// ENOENT while resolving package 'zod/v3' from +/// '.../cache_nomount/bun/@langchain/core@@@@1/dist/runnables/base.js' +/// +/// The fix passes `--preserve-symlinks` so Bun resolves from the +/// symlink path under `/node_modules/`, where `zod` is a sibling. +#[sqlx::test(fixtures("base"))] +async fn test_bun_nobundling_transitive_require(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + let content = r#"//nobundling +import { ChatPromptTemplate } from "@langchain/core/prompts"; + +export async function main() { + const tpl = ChatPromptTemplate.fromMessages([ + ["system", "you are a {role}"], + ["human", "{input}"], + ]); + const out = await tpl.formatMessages({ role: "tester", input: "ping" }); + return out.length; +} +"# + .to_owned(); + + let job = JobPayload::Code(RawCode { + hash: None, + content, + path: None, + language: ScriptLang::Bun, + lock: None, + concurrency_settings: windmill_common::runnable_settings::ConcurrencySettings::default() + .into(), + debouncing_settings: windmill_common::runnable_settings::DebouncingSettings::default(), + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + modules: None, + tag: None, + }); + + let result = run_job_in_new_worker_until_complete(&db, false, job, port) + .await + .json_result() + .unwrap(); + + assert_eq!(result, serde_json::json!(2)); + Ok(()) +} + // ============================================================================ // Native Mode Tests (requires deno_core feature) // ============================================================================ diff --git a/backend/windmill-worker/src/bun_executor.rs b/backend/windmill-worker/src/bun_executor.rs index 5e640c1fde..97ff28cae1 100644 --- a/backend/windmill-worker/src/bun_executor.rs +++ b/backend/windmill-worker/src/bun_executor.rs @@ -2172,6 +2172,7 @@ try {{ "--", &BUN_PATH, "run", + "--preserve-symlinks", "-i", "--prefer-offline", "-r", @@ -2238,6 +2239,7 @@ try {{ } else { vec![ "run", + "--preserve-symlinks", "-i", "--prefer-offline", "-r", @@ -3895,6 +3897,7 @@ pub async fn start_worker( common_bun_proc_envs, vec![ "run", + "--preserve-symlinks", "-i", "--prefer-offline", "-r", From dd19e52a84fb9a9f48e3ad061b084841c2ee7464 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 13 May 2026 14:58:35 +0000 Subject: [PATCH 09/24] perf(dynselect): only retrigger when helper args actually change (#9148) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(dynselect): only retrigger when helper-script args actually change Parse the inline helper's signature with the existing WASM parser and restrict the form-arg diff to keys the helper actually consumes. Typing into unrelated fields no longer queues a dynselect job every second. Falls back to the previous full-args comparison when the helper is deployed or parsing fails. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(dynselect): avoid double helper-script fetch on mount usePromise defaults to loadInit=true, so refresh() ran before the JobLoader child was bound (firing a no-op pending promise) and the $effect then fired a second refresh once the bind:this resolved. Disable loadInit so the effect owns the single first call. Co-Authored-By: Claude Opus 4.7 (1M context) * refactor(dynselect): use parser directly instead of inferArgs inferArgs mutates a Schema object we never use and goes through a shared cache; when fed an empty schema for non-main entrypoints the caller cannot reliably read back the resulting properties. Add parseEntrypointArgs that just runs the parser and returns the parameter name Set (or undefined when unknown / unsupported / has rest args / function not found). DynamicInput uses that and keeps the previous params in flight while the next parse is computing. Co-Authored-By: Claude Opus 4.7 (1M context) * feat(dynselect): support deployed helpers in smart retrigger Add getHelperEntrypointArgs which dispatches on HelperScript.source: inline parses immediately; deployed fetches the script (or the flow's inline dyn-select code) once and caches per (workspace, kind, path, entrypoint). Without this the /scripts/get/* run view fell back to the full-args comparison and still retriggered on unrelated fields. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(dynselect): zero-arg helpers report empty deps, not unknown Codex review flagged that a valid zero-parameter entrypoint was being treated as "couldn't determine signature" and falling back to the full-args comparison. Distinguish "function found with no params" from "function not found" via the parser's auto_kind field — only the latter sets it, so empty args + auto_kind=null means a real zero-arg helper and we return an empty Set (no retrigger on unrelated fields). Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../src/lib/components/DynamicInput.svelte | 41 ++++++++- frontend/src/lib/infer.ts | 90 ++++++++++++++++++- 2 files changed, 128 insertions(+), 3 deletions(-) diff --git a/frontend/src/lib/components/DynamicInput.svelte b/frontend/src/lib/components/DynamicInput.svelte index 692272d50e..f4e2ba9c75 100644 --- a/frontend/src/lib/components/DynamicInput.svelte +++ b/frontend/src/lib/components/DynamicInput.svelte @@ -25,6 +25,7 @@ import { type DynamicInput } from '$lib/utils' import { deepEqual } from 'fast-equals' import { untrack } from 'svelte' + import { getHelperEntrypointArgs } from '$lib/infer' interface Props { value?: any @@ -48,7 +49,9 @@ }) let resultJobLoader: JobLoader | undefined = $state() - let _items = usePromise(getItemsFromOptions, { clearValueOnRefresh: false }) + // loadInit:false — the $effect below owns the first refresh once + // resultJobLoader is bound; without this the promise is kicked off twice. + let _items = usePromise(getItemsFromOptions, { clearValueOnRefresh: false, loadInit: false }) let items = $derived(_items.value) let filterText: string = $state('') @@ -125,9 +128,43 @@ }, 1000) }) + // Parameter names declared by the helper function. When known, we restrict + // the change-detection to only those keys so typing in unrelated form fields + // no longer retriggers the dynselect job. `undefined` means we couldn't + // determine the signature → fall back to a full-args comparison. + let helperParams = $state | undefined>(undefined) + + $effect(() => { + const script = helperScript + const ep = entrypoint + if (!script) { + helperParams = undefined + return + } + let cancelled = false + void getHelperEntrypointArgs(script, ep || undefined).then((params) => { + if (!cancelled) helperParams = params + }) + return () => { + cancelled = true + } + }) + + function filterArgs(args: Record | undefined) { + if (!args || !helperParams) return args + const filtered: Record = {} + for (const k of helperParams) { + if (k in args) filtered[k] = args[k] + } + return filtered + } + $effect(() => { ;[filterText, entrypoint, helperScript] - if (resultJobLoader && (open || neverLoaded || !deepEqual(lastArgs, nargs))) { + if ( + resultJobLoader && + (open || neverLoaded || !deepEqual(filterArgs(lastArgs), filterArgs(nargs))) + ) { neverLoaded = false lastArgs = $state.snapshot(otherArgs) _items.refresh() diff --git a/frontend/src/lib/infer.ts b/frontend/src/lib/infer.ts index 1c5ec7557e..7c8b83a64f 100644 --- a/frontend/src/lib/infer.ts +++ b/frontend/src/lib/infer.ts @@ -7,7 +7,13 @@ import { } from '$lib/gen' import { get, writable } from 'svelte/store' import type { Schema, SupportedLanguage } from './common.js' -import { emptySchema, getHubFlowIdFromPath, isHubFlowPath, sortObject } from './utils.js' +import { + type DynamicInput, + emptySchema, + getHubFlowIdFromPath, + isHubFlowPath, + sortObject +} from './utils.js' import { tick } from 'svelte' import initTsParser, { parse_deno, parse_outputs } from 'windmill-parser-wasm-ts' @@ -286,6 +292,88 @@ const SQL_LANGUAGES = [ 'duckdb' ] +/** + * Returns the parameter names of `entrypoint` in `code` (or `main` if not given), + * or `undefined` if the function can't be found, the code can't be parsed, the + * language isn't supported here, or the signature contains rest/keyword args + * (in which case the callee should fall back to a conservative full comparison). + * + * Lighter than {@link inferArgs} — does not touch any schema. + */ +export async function parseEntrypointArgs( + language: SupportedLanguage | 'bunnative' | undefined, + code: string, + entrypoint?: string +): Promise | undefined> { + if (!code) return undefined + try { + let sig: MainArgSignature + if (language === 'python3') { + await initWasmPython() + sig = JSON.parse(parse_python(code, entrypoint)) + } else if ( + language === 'deno' || + language === 'nativets' || + language === 'bun' || + language === 'bunnative' + ) { + await initWasmTs() + sig = JSON.parse(parse_deno(code, entrypoint)) + } else { + return undefined + } + if (sig.type === 'Invalid') return undefined + if (sig.star_args || sig.star_kwargs) return undefined + if (!Array.isArray(sig.args)) return undefined + // The parser sets auto_kind when no matching entrypoint function was + // found — empty args in that case means "unknown signature", not + // "function takes no params", so we fall back to a full comparison. + if (sig.args.length === 0 && sig.auto_kind != null) return undefined + return new Set(sig.args.map((a) => a.name)) + } catch { + return undefined + } +} + +const helperEntrypointCache = new Map | undefined>() + +/** + * Resolves a {@link DynamicInput.HelperScript} to its entrypoint parameter + * names. For deployed helpers it fetches the script (or the flow's inline + * dyn-select code) once and caches the result per workspace+path+entrypoint. + */ +export async function getHelperEntrypointArgs( + helper: DynamicInput.HelperScript, + entrypoint?: string +): Promise | undefined> { + if (helper.source === 'inline') { + return parseEntrypointArgs(helper.lang, helper.code, entrypoint) + } + const workspace = get(workspaceStore) + if (!workspace) return undefined + const cacheKey = `${workspace}::${helper.runnable_kind}::${helper.path}::${entrypoint ?? ''}` + if (helperEntrypointCache.has(cacheKey)) return helperEntrypointCache.get(cacheKey) + let result: Set | undefined + try { + if (helper.runnable_kind === 'script') { + const script = await ScriptService.getScriptByPath({ workspace, path: helper.path }) + result = await parseEntrypointArgs(script.language, script.content ?? '', entrypoint) + } else { + const flow = await FlowService.getFlowByPath({ workspace, path: helper.path }) + const schema = flow.schema as Record | undefined + const code = schema?.['x-windmill-dyn-select-code'] + const lang = schema?.['x-windmill-dyn-select-lang'] + if (typeof code === 'string' && typeof lang === 'string') { + result = await parseEntrypointArgs(lang as SupportedLanguage, code, entrypoint) + } + } + } catch { + result = undefined + } + helperEntrypointCache.set(cacheKey, result) + return result +} + export async function inferArgs( language: SupportedLanguage | 'bunnative' | undefined, code: string, From c5092069cbeda2c4c18bea80dd629c7c087b30bf Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 13 May 2026 15:05:25 +0000 Subject: [PATCH 10/24] fix: align script path existence check with deploy logic; hide Delete for non-admin (#9152) - exists_script_by_path now filters archived = false, matching the conflict check in create_script_internal. Previously the frontend blocked creating a new script at a path occupied only by archived scripts, even though renaming to that same path was allowed. - Hide the Delete entry in the script details "..." menu unless the user is admin. The backend delete_script_by_hash already requires admin, so non-admins would always see an error after clicking. --- ...a8a5f2e75d3b12ee4718452e82c7318b1bcf4.json | 23 ------------------- backend/windmill-api-scripts/src/scripts.rs | 22 ++++++++---------- .../scripts/get/[...hash]/+page.svelte | 18 ++++++++------- 3 files changed, 19 insertions(+), 44 deletions(-) delete mode 100644 backend/.sqlx/query-2a49e5b5486b650d96f3e9038cba8a5f2e75d3b12ee4718452e82c7318b1bcf4.json diff --git a/backend/.sqlx/query-2a49e5b5486b650d96f3e9038cba8a5f2e75d3b12ee4718452e82c7318b1bcf4.json b/backend/.sqlx/query-2a49e5b5486b650d96f3e9038cba8a5f2e75d3b12ee4718452e82c7318b1bcf4.json deleted file mode 100644 index 77f61ccc47..0000000000 --- a/backend/.sqlx/query-2a49e5b5486b650d96f3e9038cba8a5f2e75d3b12ee4718452e82c7318b1bcf4.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT EXISTS(SELECT 1 FROM script WHERE path = $1 AND workspace_id = $2 ORDER BY created_at DESC LIMIT 1)", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "exists", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "2a49e5b5486b650d96f3e9038cba8a5f2e75d3b12ee4718452e82c7318b1bcf4" -} diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index de0f38cd34..93c524f10a 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -978,12 +978,10 @@ async fn create_script_internal<'c>( .fetch_one(&mut *tx) .await?; } - let clashing_script = sqlx::query_as::<_, Script>( - &format!( - "SELECT {} FROM script WHERE path = $1 AND archived = false AND workspace_id = $2", - windmill_common::scripts::SCRIPT_COLUMNS, - ), - ) + let clashing_script = sqlx::query_as::<_, Script>(&format!( + "SELECT {} FROM script WHERE path = $1 AND archived = false AND workspace_id = $2", + windmill_common::scripts::SCRIPT_COLUMNS, + )) .bind(&ns.path) .bind(&w_id) .fetch_optional(&mut *tx) @@ -2248,7 +2246,7 @@ async fn exists_script_by_path( let path = path.to_path(); let exists = sqlx::query_scalar!( - "SELECT EXISTS(SELECT 1 FROM script WHERE path = $1 AND workspace_id = $2 ORDER BY created_at DESC LIMIT 1)", + "SELECT EXISTS(SELECT 1 FROM script WHERE path = $1 AND workspace_id = $2 AND archived = false ORDER BY created_at DESC LIMIT 1)", path, w_id ) @@ -2282,12 +2280,10 @@ async fn get_script_by_hash_internal<'c>( .fetch_optional(&mut **db) .await? } else { - sqlx::query_as::<_, ScriptWithStarred>( - &format!( - "SELECT {}, NULL as starred FROM script WHERE hash = $1 AND workspace_id = $2", - windmill_common::scripts::SCRIPT_COLUMNS, - ), - ) + sqlx::query_as::<_, ScriptWithStarred>(&format!( + "SELECT {}, NULL as starred FROM script WHERE hash = $1 AND workspace_id = $2", + windmill_common::scripts::SCRIPT_COLUMNS, + )) .bind(hash) .bind(workspace_id) .fetch_optional(&mut **db) 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 ff80e76177..12ca517490 100644 --- a/frontend/src/routes/(root)/(logged)/scripts/get/[...hash]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/scripts/get/[...hash]/+page.svelte @@ -565,14 +565,16 @@ }) } - menuItems.push({ - label: 'Delete', - Icon: Trash, - onclick: async () => { - deleteScript(script.hash) - }, - color: 'red' - }) + if ($userStore?.is_admin) { + menuItems.push({ + label: 'Delete', + Icon: Trash, + onclick: async () => { + deleteScript(script.hash) + }, + color: 'red' + }) + } } return menuItems From 110bef0a6e76615c7b371c5c0f5bc1f4e7a73a64 Mon Sep 17 00:00:00 2001 From: "windmill-internal-app[bot]" <217088191+windmill-internal-app[bot]@users.noreply.github.com> Date: Wed, 13 May 2026 15:12:11 +0000 Subject: [PATCH 11/24] fix: Allow devops role to use all_workspaces runs filter in admins workspace (#9153) Co-authored-by: windmill-internal-app[bot] <1429786+windmill-internal-app[bot]@users.noreply.github.com> --- frontend/src/lib/components/RunsPage.svelte | 6 +++--- frontend/src/lib/components/runs/runsFilter.ts | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/frontend/src/lib/components/RunsPage.svelte b/frontend/src/lib/components/RunsPage.svelte index 86b7588099..360eb9276d 100644 --- a/frontend/src/lib/components/RunsPage.svelte +++ b/frontend/src/lib/components/RunsPage.svelte @@ -10,7 +10,7 @@ } from '$lib/gen' import { sendUserToast } from '$lib/toast' - import { userStore, workspaceStore, userWorkspaces, superadmin } from '$lib/stores' + import { userStore, workspaceStore, userWorkspaces, superadmin, devopsRole } from '$lib/stores' import { Button, ButtonType, @@ -82,7 +82,7 @@ usernames, folders, jobTriggerKinds, - isSuperAdmin: !!$superadmin, + isSuperAdminOrDevops: !!$superadmin || !!$devopsRole, isAdminsWorkspace: $workspaceStore === 'admins' }) ) @@ -750,7 +750,7 @@ )} schema={runsFilterSearchbarSchema} presets={buildRunsFilterPresets({ - isSuperadmin: !!$superadmin, + isSuperAdminOrDevops: !!$superadmin || !!$devopsRole, isAdminsWorkspace: $workspaceStore === 'admins' })} bind:value={filters.val} diff --git a/frontend/src/lib/components/runs/runsFilter.ts b/frontend/src/lib/components/runs/runsFilter.ts index cbece7aefb..9d0b85dd75 100644 --- a/frontend/src/lib/components/runs/runsFilter.ts +++ b/frontend/src/lib/components/runs/runsFilter.ts @@ -23,14 +23,14 @@ export function buildRunsFilterSearchbarSchema({ usernames, folders, jobTriggerKinds, - isSuperAdmin, + isSuperAdminOrDevops, isAdminsWorkspace }: { paths: string[] usernames: string[] folders: string[] jobTriggerKinds: JobTriggerKind[] - isSuperAdmin: boolean + isSuperAdminOrDevops: boolean isAdminsWorkspace: boolean }) { return { @@ -206,12 +206,12 @@ export function buildRunsFilterSearchbarSchema({ label: 'Show future jobs (Default: true)', description: 'Include jobs that are planned later' }, - ...(isSuperAdmin && + ...(isSuperAdminOrDevops && isAdminsWorkspace && { all_workspaces: { type: 'boolean' as const, label: 'All workspaces', - description: 'Show jobs of all workspaces (superadmin only)' + description: 'Show jobs of all workspaces (superadmin or devops only)' } }) } satisfies FilterSchemaRec @@ -230,16 +230,16 @@ export function allowWildcards(filters: Partial | undefined) } export const buildRunsFilterPresets = ({ - isSuperadmin, + isSuperAdminOrDevops, isAdminsWorkspace }: { - isSuperadmin: boolean + isSuperAdminOrDevops: boolean isAdminsWorkspace: boolean }) => [ { name: 'Hide schedules', value: 'job_trigger_kind:\\ !schedule' }, { name: 'Hide future jobs', value: 'show_future_jobs:\\ false' }, { name: 'Show skipped', value: 'show_skipped:\\ true' }, - ...(isSuperadmin && isAdminsWorkspace + ...(isSuperAdminOrDevops && isAdminsWorkspace ? [{ name: 'All workspaces', value: 'all_workspaces:\\ true' }] : []) ] From d666e8431cdbf14d9373d9ef625b5aafc50ac50a Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 13 May 2026 15:26:21 +0000 Subject: [PATCH 12/24] feat: read-only flag on API tokens (#9144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: read-only flag on API tokens, orthogonal to scopes Add a per-token `read_only` boolean set at creation time. When true, the token can only call HTTP methods classified as Read (GET/HEAD/OPTIONS). Mutating methods and job-run actions are rejected with 403, regardless of which scopes are attached. Surfaced as a prominent toggle in the standard token-creation flow and a discreet `2xs` toggle in MCP mode (where users often want write access, so we don't bias them toward enabling it). MCP enforcement: read-only tokens hide all script/flow/hub tools from `list_tools` and only see endpoint tools whose method is GET, and the runner rejects `call_tool` on anything mutating. Co-Authored-By: Claude Opus 4.7 (1M context) * fix: review fixes for read-only token flag - Exempt /api/mcp/* and /mcp/* paths from the read-only middleware check. MCP transport runs over POST (streamable HTTP / SSE), so otherwise the middleware would 403 every MCP request before the runner could enforce read-only at the tool-call level. - Tighten is_endpoint_read_only to GET only, matching the read_only_hint that create_endpoint_annotations actually emits. - Add unit test for check_read_only_for_route covering GET/HEAD/OPTIONS, mutating methods, and run paths. Co-Authored-By: Claude Opus 4.7 (1M context) * chore: bump ee-repo-ref to read-only-trigger-toggle Co-Authored-By: Claude Opus 4.7 (1M context) * fix(frontend): make read-only toggle discreet in both modes Match the MCP-mode treatment in standard mode: text-tertiary, 2xs, shared "Read-only" label. The tooltip switches per mode so the explanation still fits the context. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(frontend): gate read-only toggle behind Limit token permissions The read-only toggle now only shows when the user has limited the token's scopes (standard mode) or in MCP mode (which always picks an MCP scope). Turning the limit off also resets read-only so it doesn't silently stick. Co-Authored-By: Claude Opus 4.7 (1M context) * feat(frontend): hide incompatible MCP tools when read-only is on When the read-only toggle is on in MCP mode: - Endpoint badges and the custom-mode endpoint MultiSelect filter to GET. - Already-selected non-GET endpoints are pruned from the scope. - The scripts/flows preview is replaced with a note explaining they're hidden (the runner already rejects script/flow runs for read-only). Co-Authored-By: Claude Opus 4.7 (1M context) * fix(frontend): place read-only toggle at top of limited scope area The previous gate required at least one scope to be picked before the read-only toggle appeared, which made it look missing while the user was still building their scope list. Move the toggle inside ScopesPicker: - Standard mode: sits directly under the "Limit token permissions" toggle whenever Limit is on, before the scope selector. - MCP mode: sits at the top of the MCP scope block. readOnly is now $bindable on ScopesPicker so CreateToken still owns the value. The auto-reset on un-limit moves into ScopesPicker too. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(frontend): nest read-only toggle inside the scope list card Place the read-only toggle at the top of the scope list (between the Selected Scopes summary and the bordered domain list) via a new optional topSlot snippet on ScopeSelector. Keeps ScopeSelector decoupled from read-only specifics; ScopesPicker fills the slot. Co-Authored-By: Claude Opus 4.7 (1M context) * chore: update ee-repo-ref to 9bc8160be50b3e57a60daf4e1b71c389a6e02b8a This commit updates the EE repository reference after PR #571 was merged in windmill-ee-private. Previous ee-repo-ref: f53d26e6685dfd60bfa67686fbd7358169cfd130 New ee-repo-ref: 9bc8160be50b3e57a60daf4e1b71c389a6e02b8a Automated by sync-ee-ref workflow. * fix: address CI review for read-only token flag - P1 (Codex): narrow the MCP middleware exemption from "any /api/mcp/*" to just the streamable HTTP transport endpoints (/api/mcp/gateway, /api/mcp/w/{ws}/{mcp,sse,list_tools}). Without this, a read-only token could POST /api/mcp/gateway/oauth/server/approve and mint a follow-on non-read-only MCP token via the OAuth code/token exchange. - P2 (Claude/cubic): fix test comment/assertion mismatch — the run-path assertion now exercises GET (which is what the RUN_PATH_ACTIONS elevation comment describes) in addition to POST. Add a regression assertion for /api/mcp/gateway/oauth/server/approve. - P2 (cubic): short-circuit script/flow/hub-script/resource fetches in MCP list_tools when read_only is on — they would only be discarded below, so skipping the DB and resource fan-out is pure win. - P2 (cubic): when scopes are pre-supplied via the CreateToken prop, the ScopesPicker isn't rendered, which previously hid the read-only toggle entirely. Render it next to the pre-supplied scopes display. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) Co-authored-by: windmill-internal-app[bot] --- ...104a5af2fd565706204b7d5d33594ff91e61e.json | 66 ++++++ ...153c43903f929ae5d62fbba12610f89c36d55.json | 2 +- ...ef468969b5452150ee6fffe31d59b52a4f1c2.json | 23 ++ ...c9fb0714c9c91a7ea3924811d345bee94c013.json | 66 ++++++ ...43e8e250cc9772cca4a7dae3442be86f4060c.json | 53 +++++ backend/ee-repo-ref.txt | 2 +- .../20260513095235_token_read_only.down.sql | 1 + .../20260513095235_token_read_only.up.sql | 4 + backend/tests/trigger_listener_queries.rs | 1 + backend/windmill-api-auth/src/auth.rs | 54 ++++- backend/windmill-api-auth/src/lib.rs | 17 +- backend/windmill-api-auth/src/scopes.rs | 41 ++++ .../tests/native_triggers.rs | 1 + backend/windmill-api-users/src/users.rs | 5 +- backend/windmill-api/openapi.yaml | 9 + backend/windmill-api/src/lib.rs | 1 + backend/windmill-mcp/src/server/backend.rs | 7 + backend/windmill-mcp/src/server/endpoints.rs | 6 + backend/windmill-mcp/src/server/mod.rs | 2 +- backend/windmill-mcp/src/server/runner.rs | 197 ++++++++++-------- .../windmill-native-triggers/src/handler.rs | 1 + .../components/mcp/McpScopeSelector.svelte | 70 +++++-- .../components/settings/CreateToken.svelte | 18 +- .../components/settings/ScopeSelector.svelte | 17 +- .../components/settings/ScopesPicker.svelte | 57 ++++- .../components/settings/TokensTable.svelte | 11 +- 26 files changed, 604 insertions(+), 128 deletions(-) create mode 100644 backend/.sqlx/query-52777947d60d5ddd6d28852a1cf104a5af2fd565706204b7d5d33594ff91e61e.json create mode 100644 backend/.sqlx/query-9a1bf7c424154172b56bbd29b51ef468969b5452150ee6fffe31d59b52a4f1c2.json create mode 100644 backend/.sqlx/query-e001fb68c60fa736cecce52be49c9fb0714c9c91a7ea3924811d345bee94c013.json create mode 100644 backend/.sqlx/query-ed82ffc4d806a19519701e39abc43e8e250cc9772cca4a7dae3442be86f4060c.json create mode 100644 backend/migrations/20260513095235_token_read_only.down.sql create mode 100644 backend/migrations/20260513095235_token_read_only.up.sql diff --git a/backend/.sqlx/query-52777947d60d5ddd6d28852a1cf104a5af2fd565706204b7d5d33594ff91e61e.json b/backend/.sqlx/query-52777947d60d5ddd6d28852a1cf104a5af2fd565706204b7d5d33594ff91e61e.json new file mode 100644 index 0000000000..3aba64d16c --- /dev/null +++ b/backend/.sqlx/query-52777947d60d5ddd6d28852a1cf104a5af2fd565706204b7d5d33594ff91e61e.json @@ -0,0 +1,66 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT label, token_prefix, expiration, created_at, last_used_at, scopes, workspace_id, read_only FROM token WHERE email = $1\n ORDER BY created_at DESC LIMIT $2 OFFSET $3", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "label", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "token_prefix", + "type_info": "Varchar" + }, + { + "ordinal": 2, + "name": "expiration", + "type_info": "Timestamptz" + }, + { + "ordinal": 3, + "name": "created_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 4, + "name": "last_used_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 5, + "name": "scopes", + "type_info": "TextArray" + }, + { + "ordinal": 6, + "name": "workspace_id", + "type_info": "Varchar" + }, + { + "ordinal": 7, + "name": "read_only", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text", + "Int8", + "Int8" + ] + }, + "nullable": [ + true, + false, + true, + false, + false, + true, + true, + false + ] + }, + "hash": "52777947d60d5ddd6d28852a1cf104a5af2fd565706204b7d5d33594ff91e61e" +} diff --git a/backend/.sqlx/query-5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55.json b/backend/.sqlx/query-5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55.json index 36ddb8ab9f..713ccb9dd3 100644 --- a/backend/.sqlx/query-5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55.json +++ b/backend/.sqlx/query-5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55.json @@ -15,7 +15,7 @@ ] }, "nullable": [ - true + null ] }, "hash": "5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55" diff --git a/backend/.sqlx/query-9a1bf7c424154172b56bbd29b51ef468969b5452150ee6fffe31d59b52a4f1c2.json b/backend/.sqlx/query-9a1bf7c424154172b56bbd29b51ef468969b5452150ee6fffe31d59b52a4f1c2.json new file mode 100644 index 0000000000..b4715fa36b --- /dev/null +++ b/backend/.sqlx/query-9a1bf7c424154172b56bbd29b51ef468969b5452150ee6fffe31d59b52a4f1c2.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO token\n (token_hash, token_prefix, token, email, label, expiration, super_admin, scopes, workspace_id, read_only)\n SELECT $1, $2, $3, $4, $5, $6, $7, $8, $9, $10\n WHERE $9::varchar IS NULL OR NOT EXISTS(\n SELECT 1 FROM workspace WHERE id = $9 AND deleted = true\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Varchar", + "Varchar", + "Varchar", + "Varchar", + "Timestamptz", + "Bool", + "TextArray", + "Varchar", + "Bool" + ] + }, + "nullable": [] + }, + "hash": "9a1bf7c424154172b56bbd29b51ef468969b5452150ee6fffe31d59b52a4f1c2" +} diff --git a/backend/.sqlx/query-e001fb68c60fa736cecce52be49c9fb0714c9c91a7ea3924811d345bee94c013.json b/backend/.sqlx/query-e001fb68c60fa736cecce52be49c9fb0714c9c91a7ea3924811d345bee94c013.json new file mode 100644 index 0000000000..d0d9bd8d95 --- /dev/null +++ b/backend/.sqlx/query-e001fb68c60fa736cecce52be49c9fb0714c9c91a7ea3924811d345bee94c013.json @@ -0,0 +1,66 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT label, token_prefix, expiration, created_at, last_used_at, scopes, workspace_id, read_only FROM token WHERE email = $1 AND (label != 'ephemeral-script' OR label IS NULL)\n ORDER BY created_at DESC LIMIT $2 OFFSET $3", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "label", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "token_prefix", + "type_info": "Varchar" + }, + { + "ordinal": 2, + "name": "expiration", + "type_info": "Timestamptz" + }, + { + "ordinal": 3, + "name": "created_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 4, + "name": "last_used_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 5, + "name": "scopes", + "type_info": "TextArray" + }, + { + "ordinal": 6, + "name": "workspace_id", + "type_info": "Varchar" + }, + { + "ordinal": 7, + "name": "read_only", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text", + "Int8", + "Int8" + ] + }, + "nullable": [ + true, + false, + true, + false, + false, + true, + true, + false + ] + }, + "hash": "e001fb68c60fa736cecce52be49c9fb0714c9c91a7ea3924811d345bee94c013" +} diff --git a/backend/.sqlx/query-ed82ffc4d806a19519701e39abc43e8e250cc9772cca4a7dae3442be86f4060c.json b/backend/.sqlx/query-ed82ffc4d806a19519701e39abc43e8e250cc9772cca4a7dae3442be86f4060c.json new file mode 100644 index 0000000000..b1f0fc0cff --- /dev/null +++ b/backend/.sqlx/query-ed82ffc4d806a19519701e39abc43e8e250cc9772cca4a7dae3442be86f4060c.json @@ -0,0 +1,53 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE token SET last_used_at = now() WHERE\n token_hash = $1\n AND (expiration > NOW() OR expiration IS NULL)\n AND (workspace_id IS NULL OR workspace_id = $2)\n RETURNING owner, email, super_admin, scopes, label, read_only", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "owner", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "email", + "type_info": "Varchar" + }, + { + "ordinal": 2, + "name": "super_admin", + "type_info": "Bool" + }, + { + "ordinal": 3, + "name": "scopes", + "type_info": "TextArray" + }, + { + "ordinal": 4, + "name": "label", + "type_info": "Varchar" + }, + { + "ordinal": 5, + "name": "read_only", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + true, + true, + false, + true, + true, + false + ] + }, + "hash": "ed82ffc4d806a19519701e39abc43e8e250cc9772cca4a7dae3442be86f4060c" +} diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index cad5dee291..92dd5ba787 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -7a32388adaa37eb1dd1820b40e140ff1877110f2 +9bc8160be50b3e57a60daf4e1b71c389a6e02b8a diff --git a/backend/migrations/20260513095235_token_read_only.down.sql b/backend/migrations/20260513095235_token_read_only.down.sql new file mode 100644 index 0000000000..e5380a3b02 --- /dev/null +++ b/backend/migrations/20260513095235_token_read_only.down.sql @@ -0,0 +1 @@ +ALTER TABLE token DROP COLUMN IF EXISTS read_only; diff --git a/backend/migrations/20260513095235_token_read_only.up.sql b/backend/migrations/20260513095235_token_read_only.up.sql new file mode 100644 index 0000000000..4fdeb9db3a --- /dev/null +++ b/backend/migrations/20260513095235_token_read_only.up.sql @@ -0,0 +1,4 @@ +-- Add a flag to restrict a token to read-only HTTP endpoints. +-- Orthogonal to `scopes`: even if scopes grant write/run, this flag denies +-- mutating methods (POST/PUT/PATCH/DELETE) and Run actions. +ALTER TABLE token ADD COLUMN read_only BOOLEAN NOT NULL DEFAULT false; diff --git a/backend/tests/trigger_listener_queries.rs b/backend/tests/trigger_listener_queries.rs index c169d5289a..d1c20da407 100644 --- a/backend/tests/trigger_listener_queries.rs +++ b/backend/tests/trigger_listener_queries.rs @@ -174,6 +174,7 @@ fn make_authed() -> windmill_api_auth::ApiAuthed { scopes: None, username_override: None, token_prefix: None, + read_only: false, } } diff --git a/backend/windmill-api-auth/src/auth.rs b/backend/windmill-api-auth/src/auth.rs index ac4017b81c..bda9f54bdd 100644 --- a/backend/windmill-api-auth/src/auth.rs +++ b/backend/windmill-api-auth/src/auth.rs @@ -194,6 +194,7 @@ impl AuthCache { scopes: None, username_override, token_prefix: claims.audit_span, + read_only: false, }; let job_id = claims.job_id.and_then(|j| uuid::Uuid::from_str(&j).ok()); AUTH_CACHE.insert( @@ -221,11 +222,20 @@ impl AuthCache { token_hash = $1 AND (expiration > NOW() OR expiration IS NULL) AND (workspace_id IS NULL OR workspace_id = $2) - RETURNING owner, email, super_admin, scopes, label", + RETURNING owner, email, super_admin, scopes, label, read_only", t_hash, w_id.as_ref(), ) - .map(|x| (x.owner, x.email, x.super_admin, x.scopes, x.label)) + .map(|x| { + ( + x.owner, + x.email, + x.super_admin, + x.scopes, + x.label, + x.read_only, + ) + }) .fetch_optional(&self.db) .await .ok() @@ -234,7 +244,9 @@ impl AuthCache { if let Some(user) = user_o { let authed_o = { match user { - (Some(owner), Some(email), super_admin, _, label) if w_id.is_some() => { + (Some(owner), Some(email), super_admin, _, label, read_only) + if w_id.is_some() => + { let username_override = username_override_from_label(label); if let Some((prefix, name)) = owner.split_once('/') { if prefix == "u" { @@ -280,6 +292,7 @@ impl AuthCache { scopes: None, username_override, token_prefix: Some(safe_token_prefix(token)), + read_only, }) } else { let groups = vec![name.to_string()]; @@ -305,6 +318,7 @@ impl AuthCache { scopes: None, username_override, token_prefix: Some(safe_token_prefix(token)), + read_only, }) } } else { @@ -320,10 +334,11 @@ impl AuthCache { scopes: None, username_override, token_prefix: Some(safe_token_prefix(token)), + read_only, }) } } - (_, Some(email), super_admin, scopes, label) => { + (_, Some(email), super_admin, scopes, label, read_only) => { let username_override = username_override_from_label(label); if w_id.is_some() { let row_o = sqlx::query!( @@ -368,6 +383,7 @@ impl AuthCache { scopes, username_override, token_prefix: Some(safe_token_prefix(token)), + read_only, }) } None if super_admin => Some(ApiAuthed { @@ -380,6 +396,7 @@ impl AuthCache { scopes, username_override, token_prefix: Some(safe_token_prefix(token)), + read_only, }), None => None, } @@ -394,6 +411,7 @@ impl AuthCache { scopes, username_override, token_prefix: Some(safe_token_prefix(token)), + read_only, }) } } @@ -428,6 +446,7 @@ impl AuthCache { scopes: None, username_override: None, token_prefix: Some(safe_token_prefix(token)), + read_only: false, }; Some(OptJobAuthed { authed, job_id: None }) } else { @@ -630,6 +649,7 @@ pub async fn resolve_opt_job_authed( scopes: None, username_override: None, token_prefix: None, + read_only: false, }; return Ok((OptJobAuthed { authed, job_id: None }, parts)); } @@ -667,12 +687,11 @@ pub async fn resolve_opt_job_authed( cache.get_opt_job_authed(workspace_id.clone(), &token).await { let authed = &mut opt_job_authed.authed; + let path = original_uri.path(); + let method = parts.method.as_str(); if authed.scopes.is_some() { transform_old_scope_to_new_scope(authed.scopes.as_mut()); - let path = original_uri.path(); - let method = parts.method.as_str(); - if let Err(err) = crate::scopes::check_scopes_for_route( authed.scopes.as_deref(), path, @@ -681,6 +700,27 @@ pub async fn resolve_opt_job_authed( return Err((err, parts)); } } + if authed.read_only { + // MCP transport runs over POST (streamable HTTP / SSE handshake), + // so the middleware can't safely reject mutating methods on it — + // the MCP runner itself filters out write tools and rejects + // mutating tool calls for read-only tokens. Narrow to the actual + // transport endpoints: anything else under `/api/mcp/*` (OAuth + // approve, token exchange, client registration) must still go + // through the read-only check, otherwise a read-only token + // could approve an OAuth flow that mints a new non-read-only + // token. + let is_mcp_transport = path == "/api/mcp/gateway" + || (path.starts_with("/api/mcp/w/") + && (path.ends_with("/mcp") + || path.ends_with("/sse") + || path.ends_with("/list_tools"))); + if !is_mcp_transport { + if let Err(err) = crate::scopes::check_read_only_for_route(path, method) { + return Err((err, parts)); + } + } + } parts.extensions.insert(authed.clone()); Span::current().record("username", &authed.username.as_str()); diff --git a/backend/windmill-api-auth/src/lib.rs b/backend/windmill-api-auth/src/lib.rs index c5b9b5adc7..b9bc2e2417 100644 --- a/backend/windmill-api-auth/src/lib.rs +++ b/backend/windmill-api-auth/src/lib.rs @@ -55,6 +55,7 @@ pub struct ApiAuthed { pub scopes: Option>, pub username_override: Option, pub token_prefix: Option, + pub read_only: bool, } impl ApiAuthed { @@ -103,6 +104,7 @@ impl From for ApiAuthed { scopes: value.scopes, username_override: None, token_prefix: value.token_prefix, + read_only: false, } } } @@ -183,6 +185,10 @@ impl windmill_mcp::server::McpAuth for ApiAuthed { fn scopes(&self) -> Option<&[String]> { self.scopes.as_deref() } + + fn read_only(&self) -> bool { + self.read_only + } } // ------------ Utility functions ------------ @@ -478,6 +484,7 @@ pub async fn fetch_api_authed_from_permissioned_as( scopes: authed.scopes, username_override: None, token_prefix: authed.token_prefix, + read_only: false, }; API_AUTHED_CACHE.insert( @@ -506,6 +513,8 @@ pub struct NewToken { pub impersonate_email: Option, pub scopes: Option>, pub workspace_id: Option, + #[serde(default)] + pub read_only: Option, } impl NewToken { @@ -515,8 +524,9 @@ impl NewToken { impersonate_email: Option, scopes: Option>, workspace_id: Option, + read_only: Option, ) -> Self { - Self { label, expiration, impersonate_email, scopes, workspace_id } + Self { label, expiration, impersonate_email, scopes, workspace_id, read_only } } } @@ -564,8 +574,8 @@ pub async fn create_token_internal( } let rows = sqlx::query!( "INSERT INTO token - (token_hash, token_prefix, token, email, label, expiration, super_admin, scopes, workspace_id) - SELECT $1, $2, $3, $4, $5, $6, $7, $8, $9 + (token_hash, token_prefix, token, email, label, expiration, super_admin, scopes, workspace_id, read_only) + SELECT $1, $2, $3, $4, $5, $6, $7, $8, $9, $10 WHERE $9::varchar IS NULL OR NOT EXISTS( SELECT 1 FROM workspace WHERE id = $9 AND deleted = true )", @@ -578,6 +588,7 @@ pub async fn create_token_internal( is_super_admin, token_config.scopes.as_ref().map(|x| x.as_slice()), token_config.workspace_id, + token_config.read_only.unwrap_or(false), ) .execute(&mut *tx) .await?; diff --git a/backend/windmill-api-auth/src/scopes.rs b/backend/windmill-api-auth/src/scopes.rs index 6df2f74ae8..87ca3a8862 100644 --- a/backend/windmill-api-auth/src/scopes.rs +++ b/backend/windmill-api-auth/src/scopes.rs @@ -686,6 +686,19 @@ fn scope_grants_access( Ok(true) } +/// Enforces a token's `read_only` flag: only methods classified as `Read` +/// (GET/HEAD/OPTIONS) are allowed. Run actions and mutating methods are +/// rejected. Independent of `scopes`. +pub fn check_read_only_for_route(route_path: &str, http_method: &str) -> Result<()> { + if map_http_method_to_action(http_method, route_path) == ScopeAction::Read { + Ok(()) + } else { + Err(Error::PermissionDenied( + "Token is read-only. Mutating endpoints are not allowed.".to_string(), + )) + } +} + /// Helper function to check if scopes allow access to a route pub fn check_scopes_for_route( token_scopes: Option<&[String]>, @@ -778,6 +791,34 @@ mod tests { assert_eq!(route_suffix, Some("flow_conversations/list".to_string())); } + #[test] + fn test_check_read_only_for_route() { + // Plain GETs pass. + assert!(check_read_only_for_route("/api/w/x/scripts/list", "GET").is_ok()); + assert!(check_read_only_for_route("/api/w/x/scripts/get/foo", "HEAD").is_ok()); + assert!(check_read_only_for_route("/api/w/x/anything", "OPTIONS").is_ok()); + + // Mutating methods are rejected. + assert!(check_read_only_for_route("/api/w/x/scripts/create", "POST").is_err()); + assert!(check_read_only_for_route("/api/w/x/scripts/update", "PUT").is_err()); + assert!(check_read_only_for_route("/api/w/x/scripts/delete", "DELETE").is_err()); + assert!(check_read_only_for_route("/api/w/x/scripts/patch", "PATCH").is_err()); + + // Run paths are rejected even on GET (map_http_method_to_action elevates + // them to Run via RUN_PATH_ACTIONS). + assert!(check_read_only_for_route("/api/w/x/jobs/run/p/f/foo", "GET").is_err()); + assert!(check_read_only_for_route("/api/w/x/jobs/run/p/f/foo", "POST").is_err()); + + // OAuth/registration endpoints under /api/mcp/* must NOT be exempted by + // the auth middleware — they go through this check on the gateway side + // because they can mint non-read-only tokens. The middleware decides + // which paths to exempt; this helper is method-only, so we just assert + // that mutating methods still fail. + assert!( + check_read_only_for_route("/api/mcp/gateway/oauth/server/approve", "POST").is_err() + ); + } + #[test] fn test_specific_scope_access() { let scopes = vec!["jobs:read".to_string()]; diff --git a/backend/windmill-api-integration-tests/tests/native_triggers.rs b/backend/windmill-api-integration-tests/tests/native_triggers.rs index 0d1530a7d9..159236ed4b 100644 --- a/backend/windmill-api-integration-tests/tests/native_triggers.rs +++ b/backend/windmill-api-integration-tests/tests/native_triggers.rs @@ -51,6 +51,7 @@ fn test_authed() -> ApiAuthed { scopes: None, username_override: None, token_prefix: None, + read_only: false, } } diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index e4540ca903..2ae6c838c1 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -297,6 +297,7 @@ pub struct TruncatedToken { pub last_used_at: chrono::DateTime, pub scopes: Option>, pub workspace_id: Option, + pub read_only: bool, } // NewToken is re-exported from windmill-api-auth above @@ -2249,7 +2250,7 @@ async fn list_tokens( sqlx::query_as!( TruncatedToken, "SELECT label, token_prefix, expiration, created_at, \ - last_used_at, scopes, workspace_id FROM token WHERE email = $1 AND (label != 'ephemeral-script' OR label IS NULL) + last_used_at, scopes, workspace_id, read_only FROM token WHERE email = $1 AND (label != 'ephemeral-script' OR label IS NULL) ORDER BY created_at DESC LIMIT $2 OFFSET $3", email, per_page as i64, @@ -2261,7 +2262,7 @@ async fn list_tokens( sqlx::query_as!( TruncatedToken, "SELECT label, token_prefix, expiration, created_at, \ - last_used_at, scopes, workspace_id FROM token WHERE email = $1 + last_used_at, scopes, workspace_id, read_only FROM token WHERE email = $1 ORDER BY created_at DESC LIMIT $2 OFFSET $3", email, per_page as i64, diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 258e268f88..953fe8e278 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -22631,10 +22631,13 @@ components: type: string workspace_id: type: string + read_only: + type: boolean required: - token_prefix - created_at - last_used_at + - read_only ExternalJwtToken: type: object @@ -22683,6 +22686,12 @@ components: type: string workspace_id: type: string + read_only: + type: boolean + description: | + If true, the token is restricted to read-only HTTP methods + (GET/HEAD/OPTIONS). Mutating endpoints and job-run actions are + rejected with 403, regardless of the scopes attached. NewTokenImpersonate: type: object diff --git a/backend/windmill-api/src/lib.rs b/backend/windmill-api/src/lib.rs index 0fde5cacd1..75b3469c41 100644 --- a/backend/windmill-api/src/lib.rs +++ b/backend/windmill-api/src/lib.rs @@ -330,6 +330,7 @@ async fn inject_agent_authed( scopes: None, username_override: None, token_prefix: None, + read_only: false, }, job_id: None, }); diff --git a/backend/windmill-mcp/src/server/backend.rs b/backend/windmill-mcp/src/server/backend.rs index 0b942353b3..7a6e007c4d 100644 --- a/backend/windmill-mcp/src/server/backend.rs +++ b/backend/windmill-mcp/src/server/backend.rs @@ -33,6 +33,13 @@ pub trait McpAuth: Send + Sync + Clone + 'static { /// Get token scopes fn scopes(&self) -> Option<&[String]>; + /// True if the token was created with the `read_only` flag. + /// When set, write-capable tools must be hidden from `list_tools` and + /// rejected by `call_tool`. Defaults to false so existing impls compile. + fn read_only(&self) -> bool { + false + } + /// Check if the user has an MCP scope fn has_mcp_scope(&self) -> bool { self.scopes() diff --git a/backend/windmill-mcp/src/server/endpoints.rs b/backend/windmill-mcp/src/server/endpoints.rs index 2401b10757..373db36eb1 100644 --- a/backend/windmill-mcp/src/server/endpoints.rs +++ b/backend/windmill-mcp/src/server/endpoints.rs @@ -26,6 +26,12 @@ pub struct EndpointTool { pub body_field_renames: Option, } +/// True if this endpoint is safe to expose to a read-only token. Mirrors the +/// `read_only_hint` computed by `create_endpoint_annotations`: only `GET`. +pub fn is_endpoint_read_only(tool: &EndpointTool) -> bool { + tool.method.as_ref() == "GET" +} + /// Convert a single endpoint tool to MCP tool pub fn endpoint_tool_to_mcp_tool(tool: &EndpointTool) -> Tool { let mut combined_properties = serde_json::Map::new(); diff --git a/backend/windmill-mcp/src/server/mod.rs b/backend/windmill-mcp/src/server/mod.rs index d489b3f429..688c12b827 100644 --- a/backend/windmill-mcp/src/server/mod.rs +++ b/backend/windmill-mcp/src/server/mod.rs @@ -12,7 +12,7 @@ pub mod tools; // Re-export main types pub use backend::{BackendResult, McpAuth, McpBackend}; -pub use endpoints::{endpoint_tool_to_mcp_tool, EndpointTool}; +pub use endpoints::{endpoint_tool_to_mcp_tool, is_endpoint_read_only, EndpointTool}; pub use runner::Runner; pub use tools::create_tool_from_item; diff --git a/backend/windmill-mcp/src/server/runner.rs b/backend/windmill-mcp/src/server/runner.rs index 0cb2962c55..6d33573d95 100644 --- a/backend/windmill-mcp/src/server/runner.rs +++ b/backend/windmill-mcp/src/server/runner.rs @@ -145,94 +145,100 @@ impl ServerHandler for Runner { parse_mcp_scopes(scopes).map_err(|e| ErrorData::internal_error(e, None))?; let favorites_only = scope_config.favorites; - - // Fetch all items concurrently - let (scripts, flows, resource_types, hub_scripts) = tokio::try_join!( - self.backend - .list_scripts(&auth, &workspace_id, favorites_only, None), - self.backend - .list_flows(&auth, &workspace_id, favorites_only, None), - self.backend.list_resource_types(&auth, &workspace_id), - async { - if let Some(ref apps) = scope_config.hub_apps { - self.backend.list_hub_scripts(Some(apps)).await - } else { - Ok(vec![]) - } - } - )?; - - // Filter items based on scope - let filtered_scripts: Vec<_> = scripts - .into_iter() - .filter(|s| !scope_config.granular || scope_config.is_allowed("script", &s.path)) - .collect(); - - let filtered_flows: Vec<_> = flows - .into_iter() - .filter(|f| !scope_config.granular || scope_config.is_allowed("flow", &f.path)) - .collect(); - - // Collect all needed resource types from all schemas - let mut needed_resource_types: HashSet = HashSet::new(); - for script in &filtered_scripts { - needed_resource_types.extend(extract_resource_types_from_schema(&script.get_schema())); - } - for flow in &filtered_flows { - needed_resource_types.extend(extract_resource_types_from_schema(&flow.get_schema())); - } - for hub_script in &hub_scripts { - needed_resource_types - .extend(extract_resource_types_from_schema(&hub_script.get_schema())); - } - - // Pre-fetch all resources - let resource_futures: Vec<_> = needed_resource_types - .into_iter() - .map(|rt| { - let backend = self.backend.clone(); - let auth = auth.clone(); - let workspace_id = workspace_id.clone(); - async move { - backend - .list_resources(&auth, &workspace_id, &rt) - .await - .map(|resources| (rt, resources)) - } - }) - .collect(); - - let resource_results = futures::future::try_join_all(resource_futures).await?; - let resources_cache: HashMap> = - resource_results.into_iter().collect(); + let read_only = auth.read_only(); let mut tools = Vec::new(); - for script in &filtered_scripts { - tools.push(create_tool_from_item( - script, - self.backend.as_ref(), - &resources_cache, - &resource_types, - )); - } + // Read-only tokens cannot run scripts/flows/hub-scripts (running is a + // mutating action), so skip the script/flow/hub/resource fetches + // entirely — they would only be discarded below. + if !read_only { + let (scripts, flows, resource_types, hub_scripts) = tokio::try_join!( + self.backend + .list_scripts(&auth, &workspace_id, favorites_only, None), + self.backend + .list_flows(&auth, &workspace_id, favorites_only, None), + self.backend.list_resource_types(&auth, &workspace_id), + async { + if let Some(ref apps) = scope_config.hub_apps { + self.backend.list_hub_scripts(Some(apps)).await + } else { + Ok(vec![]) + } + } + )?; - for flow in &filtered_flows { - tools.push(create_tool_from_item( - flow, - self.backend.as_ref(), - &resources_cache, - &resource_types, - )); - } + let filtered_scripts: Vec<_> = scripts + .into_iter() + .filter(|s| !scope_config.granular || scope_config.is_allowed("script", &s.path)) + .collect(); - for hub_script in &hub_scripts { - tools.push(create_tool_from_item( - hub_script, - self.backend.as_ref(), - &resources_cache, - &resource_types, - )); + let filtered_flows: Vec<_> = flows + .into_iter() + .filter(|f| !scope_config.granular || scope_config.is_allowed("flow", &f.path)) + .collect(); + + // Collect all needed resource types from all schemas + let mut needed_resource_types: HashSet = HashSet::new(); + for script in &filtered_scripts { + needed_resource_types + .extend(extract_resource_types_from_schema(&script.get_schema())); + } + for flow in &filtered_flows { + needed_resource_types + .extend(extract_resource_types_from_schema(&flow.get_schema())); + } + for hub_script in &hub_scripts { + needed_resource_types + .extend(extract_resource_types_from_schema(&hub_script.get_schema())); + } + + // Pre-fetch all resources + let resource_futures: Vec<_> = needed_resource_types + .into_iter() + .map(|rt| { + let backend = self.backend.clone(); + let auth = auth.clone(); + let workspace_id = workspace_id.clone(); + async move { + backend + .list_resources(&auth, &workspace_id, &rt) + .await + .map(|resources| (rt, resources)) + } + }) + .collect(); + + let resource_results = futures::future::try_join_all(resource_futures).await?; + let resources_cache: HashMap> = + resource_results.into_iter().collect(); + + for script in &filtered_scripts { + tools.push(create_tool_from_item( + script, + self.backend.as_ref(), + &resources_cache, + &resource_types, + )); + } + + for flow in &filtered_flows { + tools.push(create_tool_from_item( + flow, + self.backend.as_ref(), + &resources_cache, + &resource_types, + )); + } + + for hub_script in &hub_scripts { + tools.push(create_tool_from_item( + hub_script, + self.backend.as_ref(), + &resources_cache, + &resource_types, + )); + } } // Add endpoint tools from the generated MCP tools, filtered by scope @@ -241,6 +247,9 @@ impl ServerHandler for Runner { if scope_config.granular && !scope_config.is_allowed("endpoint", &endpoint_tool.name) { continue; } + if read_only && !crate::server::is_endpoint_read_only(&endpoint_tool) { + continue; + } tools.push(endpoint_tool_to_mcp_tool(&endpoint_tool)); } @@ -259,6 +268,7 @@ impl ServerHandler for Runner { let scopes = auth.scopes().unwrap_or(&[]); let scope_config = parse_mcp_scopes(scopes).map_err(|e| ErrorData::internal_error(e, None))?; + let read_only = auth.read_only(); let args = request.arguments.map(Value::Object).unwrap_or(Value::Null); @@ -278,6 +288,15 @@ impl ServerHandler for Runner { None, )); } + if read_only && !crate::server::is_endpoint_read_only(endpoint_tool) { + return Err(ErrorData::internal_error( + format!( + "Access denied: endpoint '{}' is not read-only and this token is restricted to read-only operations", + endpoint_tool.name + ), + None, + )); + } // This is an endpoint tool, call via backend let result = self @@ -294,6 +313,18 @@ impl ServerHandler for Runner { } } + // Anything below this point runs a script or flow, which is a mutating + // action and must be denied for read-only tokens. + if read_only { + return Err(ErrorData::internal_error( + format!( + "Access denied: tool '{}' runs a script/flow and this token is restricted to read-only operations", + request.name + ), + None, + )); + } + // Resolve the tool name to (type, path, is_hub) let (type_str, is_hub, is_hashed) = parse_tool_prefix(&request.name).map_err(|e| { ErrorData::internal_error(format!("Failed to parse tool name: {}", e), None) diff --git a/backend/windmill-native-triggers/src/handler.rs b/backend/windmill-native-triggers/src/handler.rs index 1b6aa83b92..fda28af407 100644 --- a/backend/windmill-native-triggers/src/handler.rs +++ b/backend/windmill-native-triggers/src/handler.rs @@ -94,6 +94,7 @@ async fn new_webhook_token( None, Some(scopes), Some(workspace_id.to_owned()), + None, ); let token = create_token_internal(&mut *tx, &db, &authed, token_config).await?; diff --git a/frontend/src/lib/components/mcp/McpScopeSelector.svelte b/frontend/src/lib/components/mcp/McpScopeSelector.svelte index 61ae086515..cf95617790 100644 --- a/frontend/src/lib/components/mcp/McpScopeSelector.svelte +++ b/frontend/src/lib/components/mcp/McpScopeSelector.svelte @@ -15,9 +15,27 @@ workspaceId: string scope: string initialScope?: string + readOnly?: boolean } - let { workspaceId, scope = $bindable(), initialScope }: Props = $props() + let { workspaceId, scope = $bindable(), initialScope, readOnly = false }: Props = $props() + + // Endpoints we can actually advertise to a read-only MCP token. Mirrors the + // runner's filter (only GET endpoints). + const visibleEndpointTools = $derived( + readOnly ? mcpEndpointTools.filter((e) => e.method === 'GET') : mcpEndpointTools + ) + + // When read-only flips on, prune already-selected non-GET endpoints so the + // scope string doesn't keep references to tools the server will reject. + $effect(() => { + if (!readOnly || selectedEndpoints.length === 0) return + const allowed = new Set(visibleEndpointTools.map((e) => e.name)) + const filtered = selectedEndpoints.filter((n) => allowed.has(n)) + if (filtered.length !== selectedEndpoints.length) { + selectedEndpoints = filtered + } + }) const parsedInitial = parseInitialScope(initialScope) @@ -410,7 +428,7 @@ selectedFlows = [] } function selectAllEndpoints() { - selectedEndpoints = [...mcpEndpointTools.map((e) => e.name)] + selectedEndpoints = [...visibleEndpointTools.map((e) => e.name)] } function clearAllEndpoints() { selectedEndpoints = [] @@ -529,7 +547,7 @@
{@render sectionHeader('API Endpoints', selectAllEndpoints, clearAllEndpoints)} e.name))} + items={safeSelectItems(visibleEndpointTools.map((e) => e.name))} placeholder="Select endpoints" bind:value={selectedEndpoints} /> @@ -594,29 +612,35 @@
{:else}
- Scripts & Flows that will be available via MCP -
- {#if includedRunnables.length > 0 && includedRunnables.length <= 5} - {#each includedRunnables as scriptOrFlow (scriptOrFlow)} - {scriptOrFlow} - {/each} - {:else if includedRunnables.length > 0} - {#each includedRunnables.slice(0, 3) as scriptOrFlow (scriptOrFlow)} - {scriptOrFlow} - {/each} - - +{includedRunnables.length - 3} more - - {:else} -

- {warning} -

- {/if} -
+ {#if !readOnly} + Scripts & Flows that will be available via MCP +
+ {#if includedRunnables.length > 0 && includedRunnables.length <= 5} + {#each includedRunnables as scriptOrFlow (scriptOrFlow)} + {scriptOrFlow} + {/each} + {:else if includedRunnables.length > 0} + {#each includedRunnables.slice(0, 3) as scriptOrFlow (scriptOrFlow)} + {scriptOrFlow} + {/each} + + +{includedRunnables.length - 3} more + + {:else} +

+ {warning} +

+ {/if} +
+ {:else} +

+ Scripts and flows are hidden because this token is read-only. +

+ {/if} API endpoint tools that will be available via MCP
- {#each mcpEndpointTools as endpoint (endpoint.name)} + {#each visibleEndpointTools as endpoint (endpoint.name)} {#snippet text()}
diff --git a/frontend/src/lib/components/settings/CreateToken.svelte b/frontend/src/lib/components/settings/CreateToken.svelte index 6ba2461725..80c6cb34ef 100644 --- a/frontend/src/lib/components/settings/CreateToken.svelte +++ b/frontend/src/lib/components/settings/CreateToken.svelte @@ -46,6 +46,7 @@ let mcpLabelAutofilled = $state(false) let pickedScopes = $state(null) + let readOnly = $state(false) function ensureCurrentWorkspaceIncluded( workspacesList: UserWorkspace[], @@ -67,6 +68,7 @@ newTokenWorkspace = defaultNewTokenWorkspace ?? $workspaceStore newToken = undefined newMcpToken = undefined + readOnly = false if (!newTokenLabel) { newTokenLabel = 'MCP token' mcpLabelAutofilled = true @@ -80,6 +82,7 @@ newTokenExpiration = undefined newTokenWorkspace = defaultNewTokenWorkspace newMcpToken = undefined + readOnly = false if (mcpLabelAutofilled) { newTokenLabel = undefined } @@ -100,7 +103,8 @@ label: newTokenLabel, expiration: date?.toISOString(), scopes: tokenScopes, - workspace_id: mcpMode ? newTokenWorkspace || $workspaceStore : newTokenWorkspace + workspace_id: mcpMode ? newTokenWorkspace || $workspaceStore : newTokenWorkspace, + read_only: readOnly } as NewToken }) @@ -184,6 +188,17 @@ {#each scopes as scope (scope)} {/each} +
+ +
{/if} @@ -192,6 +207,7 @@ mode={mcpCreationMode ? 'mcp' : 'standard'} workspaceId={newTokenWorkspace || $workspaceStore || ''} bind:value={pickedScopes} + bind:readOnly /> {/if} diff --git a/frontend/src/lib/components/settings/ScopeSelector.svelte b/frontend/src/lib/components/settings/ScopeSelector.svelte index da9d4949a6..91a62e65f7 100644 --- a/frontend/src/lib/components/settings/ScopeSelector.svelte +++ b/frontend/src/lib/components/settings/ScopeSelector.svelte @@ -7,10 +7,14 @@ import Tooltip from '../Tooltip.svelte' import { twMerge } from 'tailwind-merge' + import type { Snippet } from 'svelte' + interface Props { selectedScopes?: string[] disabled?: boolean class?: string + /** Renders above the scope-list card, below the Selected Scopes summary. */ + topSlot?: Snippet } interface ScopeState { @@ -30,7 +34,12 @@ domains: Record } - let { selectedScopes = $bindable([]), disabled = false, class: className = '' }: Props = $props() + let { + selectedScopes = $bindable([]), + disabled = false, + class: className = '', + topSlot + }: Props = $props() let scopeDomains = $state(null) let loading = $state(false) @@ -535,6 +544,12 @@ {/if}
+ {#if topSlot} +
+ {@render topSlot()} +
+ {/if} +
{#each scopeDomains as domain} {@const domainState = getDomainState(domain.name)} diff --git a/frontend/src/lib/components/settings/ScopesPicker.svelte b/frontend/src/lib/components/settings/ScopesPicker.svelte index 2e96269a00..e8cdccf40c 100644 --- a/frontend/src/lib/components/settings/ScopesPicker.svelte +++ b/frontend/src/lib/components/settings/ScopesPicker.svelte @@ -10,9 +10,28 @@ initialScopes?: string[] /** Final scope value: null = unrestricted/full access, array = explicit list */ value: string[] | null + /** Read-only flag; also forwarded to McpScopeSelector to filter incompatible + * endpoints/runnables. Two-way bound so the inline toggle below the + * "Limit token permissions" switch (and the MCP variant) writes back. */ + readOnly?: boolean } - let { mode, workspaceId = '', initialScopes, value = $bindable() }: Props = $props() + let { + mode, + workspaceId = '', + initialScopes, + value = $bindable(), + readOnly = $bindable(false) + }: Props = $props() + + // In standard mode, only meaningful when the user has turned "Limit token + // permissions" on. Reset when they un-limit so the flag doesn't quietly + // stick if they re-enable later. + $effect(() => { + if (mode === 'standard' && !limited && readOnly) { + readOnly = false + } + }) const initialMcpScope = $derived( (initialScopes ?? []).length > 0 ? (initialScopes ?? []).join(' ') : undefined @@ -51,9 +70,41 @@ size="xs" /> {#if limited} - + + {#snippet topSlot()} +
+ +
+ {/snippet} + {/if}
{:else} - +
+
+ +
+ +
{/if} diff --git a/frontend/src/lib/components/settings/TokensTable.svelte b/frontend/src/lib/components/settings/TokensTable.svelte index 816d710e3c..5907c808a6 100644 --- a/frontend/src/lib/components/settings/TokensTable.svelte +++ b/frontend/src/lib/components/settings/TokensTable.svelte @@ -170,7 +170,7 @@ {#snippet body()} {#if tokens && tokens.length > 0} - {#each tokens as { token_prefix, expiration, label, scopes, workspace_id } (token_prefix)} + {#each tokens as { token_prefix, expiration, label, scopes, workspace_id, read_only } (token_prefix)} {@const badge = expirationBadge(expiration, label)} {token_prefix}**** @@ -185,8 +185,15 @@ {scopes?.join(', ') ?? ''} +
+ {#if read_only} + Read-only + {/if} + {scopes?.join(', ') ?? ''} +
+
{/each}
+
+ + +

+ Comma-separated host patterns that job HTTP clients should bypass the tracing + proxy for — those hosts will not be traced. Use this for clients that pin their + own CA (kubectl, helm, terraform providers, aws cli for EKS, etc.) which would + otherwise fail with x509: certificate signed by unknown authority. + Independent of the worker's own NO_PROXY env, which governs the proxy's + upstream relay (e.g. through a corporate proxy). +

+
{/if}
{:else if setting.fieldType == 'object_store_config'}