diff --git a/.github/workflows/sdk-tests.yml b/.github/workflows/sdk-tests.yml new file mode 100644 index 0000000000..efcbcd6ec1 --- /dev/null +++ b/.github/workflows/sdk-tests.yml @@ -0,0 +1,61 @@ +# The python and typescript SDK unit suites, on release tags only: they guard +# what gets published to npm / PyPI / JSR, and a tag is the moment that decides +# it. +# +# This runs alongside the publish workflows rather than ahead of them, so it +# reports a broken SDK rather than holding one back. Gating would mean putting +# the job inside each publish workflow, since Actions cannot express `needs` +# across workflows. +name: SDK Tests + +on: + workflow_dispatch: + push: + tags: + - "v*" + +jobs: + typescript-client: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version: latest + + # No build step: these suites are deliberately free of the generated API + # client, so they run against the sources as committed. + - name: Run tests + working-directory: ./typescript-client + run: bun test --timeout 120000 tests/ + + python-client: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup uv + uses: astral-sh/setup-uv@v5 + + # The interpreter is named explicitly: on a clean checkout uv picks the + # runner's system python and stops with "not compatible with the locked + # Python requirement" rather than fetching one. Keep in step with + # `requires-python` in uv.lock. + # + # Note this is not the version a worker runs the SDK on — those are 3.12. + # `uv.lock` asks for >=3.14, so pinning lower means regenerating it, which + # is worth doing separately. + - name: Install the interpreter the lockfile requires + run: uv python install 3.14 + + # `--frozen` so a drifted lockfile fails here rather than quietly + # resolving to something nobody has run. + - name: Run tests + working-directory: ./python-client/wmill + env: + PYTHONPATH: . + run: uv run --frozen --python 3.14 pytest tests/ -q diff --git a/AGENTS.md b/AGENTS.md index d7ebf2452d..90c0060ae2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,6 +12,7 @@ Open-source platform for internal tools, workflows, API integrations, background ## Documentation - **Validation**: `docs/validation.md` — what checks to run based on what you changed +- **Unreleased SDK changes**: `docs/wac-sdk-e2e.md` — exercising a client change on a real worker - **Enterprise**: `docs/enterprise.md` — EE file conventions and PR workflow - **Backend patterns**: use the `rust-backend` skill when writing Rust code - **Frontend patterns**: use the `svelte-frontend` skill when writing Svelte code. Do NOT edit svelte files unless you have read that skill. diff --git a/CHANGELOG.md b/CHANGELOG.md index 24b4b0cc25..ac51063de9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,58 @@ # Changelog +## [1.775.1](https://github.com/windmill-labs/windmill/compare/v1.775.0...v1.775.1) (2026-07-28) + + +### Bug Fixes + +* **ai:** render pending parallel tool calls as faded queued cards ([#10208](https://github.com/windmill-labs/windmill/issues/10208)) ([d842f76](https://github.com/windmill-labs/windmill/commit/d842f765a457f0c80a2cd1eb06bc703167512160)) +* **frontend:** add the preprocessor node before the error handler markers ([#10395](https://github.com/windmill-labs/windmill/issues/10395)) ([39dd411](https://github.com/windmill-labs/windmill/commit/39dd411481d450b1cdb92ddba3a8b613ddd2218a)) + +## [1.775.0](https://github.com/windmill-labs/windmill/compare/v1.774.0...v1.775.0) (2026-07-28) + + +### Features + +* add github dark mode variant switchable in user settings ([#10002](https://github.com/windmill-labs/windmill/issues/10002)) ([bd24664](https://github.com/windmill-labs/windmill/commit/bd246644c962f754a8477353d49ed39ba9b87ffd)) +* validate and make explicit the hub project's resource type export ([#10388](https://github.com/windmill-labs/windmill/issues/10388)) ([91b8ce5](https://github.com/windmill-labs/windmill/commit/91b8ce581a15baf7fc9ea4e983b50f233cf5e023)) + + +### Bug Fixes + +* render the flow editor's error handler node as an inert run marker ([#10391](https://github.com/windmill-labs/windmill/issues/10391)) ([bff6545](https://github.com/windmill-labs/windmill/commit/bff654596fb9cf8b086919367614ba0915cd7b87)) +* truncate strings on char boundaries to avoid panics on multibyte input ([#10390](https://github.com/windmill-labs/windmill/issues/10390)) ([faa2aaf](https://github.com/windmill-labs/windmill/commit/faa2aaf214444f4fbb129f10a46060d4a8ab3ae8)) + +## [1.774.0](https://github.com/windmill-labs/windmill/compare/v1.773.0...v1.774.0) (2026-07-28) + + +### Features + +* **ai-chat:** add get_flow_run_details tool for per-step flow run results ([#10374](https://github.com/windmill-labs/windmill/issues/10374)) ([c12e7c3](https://github.com/windmill-labs/windmill/commit/c12e7c34310b417e89f94efc3c412b3c7278344b)) +* **ai-chat:** recall queued/last message into the composer ([#10191](https://github.com/windmill-labs/windmill/issues/10191)) ([ad0fef4](https://github.com/windmill-labs/windmill/commit/ad0fef4a2e0ea63ae00562ff10d9ebbae66c42d0)) +* bulk discard selected drafts on the compare & deploy page ([#10372](https://github.com/windmill-labs/windmill/issues/10372)) ([ac970ef](https://github.com/windmill-labs/windmill/commit/ac970efa1698e42ad1e7b3cea5f06f18a567269d)) +* reusable AI agent steps with rigid linking and edit/fork ([#9825](https://github.com/windmill-labs/windmill/issues/9825)) ([3b95a2d](https://github.com/windmill-labs/windmill/commit/3b95a2d0967544640feb53fe962a8215e049f4d2)) + + +### Bug Fixes + +* **ai-agent:** keep tool description through flow deployment ([#10373](https://github.com/windmill-labs/windmill/issues/10373)) ([8d684c0](https://github.com/windmill-labs/windmill/commit/8d684c0b2305798a781685ca022b1b9dc0731cf0)) +* **ai-chat:** make hub script paths readable from the global chat ([#10381](https://github.com/windmill-labs/windmill/issues/10381)) ([5feec9b](https://github.com/windmill-labs/windmill/commit/5feec9b4cd3cdd4eed3edf5be0fc42dabfeb13d2)) +* **ai:** resolve deployment-pinned Azure base URLs to the v1 surface ([#10362](https://github.com/windmill-labs/windmill/issues/10362)) ([621718b](https://github.com/windmill-labs/windmill/commit/621718b32fe5322638039d4a560b845e11b38008)) +* **apps:** stop cross-origin isolating the raw app viewer ([#10370](https://github.com/windmill-labs/windmill/issues/10370)) ([3c2dab9](https://github.com/windmill-labs/windmill/commit/3c2dab9f8f302e2f80ef800a4271bfe5aa152767)) +* **datatable:** provision the replication user on managed postgres ([#10375](https://github.com/windmill-labs/windmill/issues/10375)) ([1b6b2aa](https://github.com/windmill-labs/windmill/commit/1b6b2aa8598100b5327ecc3e16f1541db95f9e47)) +* **frontend:** preserve top-level flow settings in AI flow tools ([#10369](https://github.com/windmill-labs/windmill/issues/10369)) ([a544dfd](https://github.com/windmill-labs/windmill/commit/a544dfde9a5f409247e78ef1c1aac2b9699d9ac8)) +* home kind filter no longer resets a fork or reloads the page ([#10384](https://github.com/windmill-labs/windmill/issues/10384)) ([2a2ef41](https://github.com/windmill-labs/windmill/commit/2a2ef411525f032743af93f143c10ad373c20fc8)) +* loop "Test an iteration" progress bar, while-loop modules and schema ([#10357](https://github.com/windmill-labs/windmill/issues/10357)) ([350eb66](https://github.com/windmill-labs/windmill/commit/350eb66560d2626c4100a21edb2101cdfa4a0a72)) +* make same worker mutually exclusive with retries and sleeps in the flow editor ([#10379](https://github.com/windmill-labs/windmill/issues/10379)) ([ecde945](https://github.com/windmill-labs/windmill/commit/ecde94567c5ec43002a06d2d765927e0d89640a4)) +* mark Setup URL as required in self-managed GitHub App instructions ([#10380](https://github.com/windmill-labs/windmill/issues/10380)) ([c4e7568](https://github.com/windmill-labs/windmill/commit/c4e75683a8a346f4cb39038f3045c94f3a9621b2)) +* raw apps with no stylesheet were permanently un-deployable ([#10364](https://github.com/windmill-labs/windmill/issues/10364)) ([8a96e3a](https://github.com/windmill-labs/windmill/commit/8a96e3a4ec47c0a28dd422b31388f7f0cced8e67)) +* show draft badge and disable the toggle for draft-only triggers ([#10155](https://github.com/windmill-labs/windmill/issues/10155)) ([b0c7e09](https://github.com/windmill-labs/windmill/commit/b0c7e0917376248cda0f894b138075d82f19f620)) +* surface postgres publication errors as 400 instead of 500 ([#10376](https://github.com/windmill-labs/windmill/issues/10376)) ([5e52346](https://github.com/windmill-labs/windmill/commit/5e523462423f73419df25e4d7aa1cd2e77c0228a)) +* surface the real postgres error when data table migrations fail ([#10371](https://github.com/windmill-labs/windmill/issues/10371)) ([fbf9f04](https://github.com/windmill-labs/windmill/commit/fbf9f04e107c674474f61c1559a2f1e56dfeeaba)) +* **wac:** one failure record for tasks and steps, in every round ([#10368](https://github.com/windmill-labs/windmill/issues/10368)) ([aeaea57](https://github.com/windmill-labs/windmill/commit/aeaea57ca1a0fac02157035530e84c3588a7aec1)) +* **wac:** report a task failure the child round's body catches ([#10366](https://github.com/windmill-labs/windmill/issues/10366)) ([727d22b](https://github.com/windmill-labs/windmill/commit/727d22b9a1f9520788afb9b519be8fcd8fd64319)) +* **wac:** return the checkpointed value from step(), not the live object ([#10367](https://github.com/windmill-labs/windmill/issues/10367)) ([044ce39](https://github.com/windmill-labs/windmill/commit/044ce39e5f9c8207287e19a0d2eefa22b79188f2)) + ## [1.773.0](https://github.com/windmill-labs/windmill/compare/v1.772.0...v1.773.0) (2026-07-27) diff --git a/ai_evals/adapters/frontend/mockBackend.ts b/ai_evals/adapters/frontend/mockBackend.ts index 64cacabd58..40a6dfa4ad 100644 --- a/ai_evals/adapters/frontend/mockBackend.ts +++ b/ai_evals/adapters/frontend/mockBackend.ts @@ -809,6 +809,142 @@ export function listBenchmarkMcpTools(): EndpointTool[] { return BENCHMARK_MCP_TOOLS } +/** A stand-in Windmill hub. `search_hub_scripts` and a `hub/` read go out over + * relative `/api/...` fetches, which have no origin here, so without these the + * hub tools throw and no case can exercise hub reuse. Serving fixtures rather + * than the live hub also keeps assertions on script content stable as the real + * hub republishes new versions. */ +const BENCHMARK_HUB_SCRIPTS = [ + { + version_id: 22235, + app: 'holded', + summary: 'Send Document', + terms: 'holded invoice document send email mail', + language: 'bun', + content: `//native +type Holded = { + apiKey: string; +}; +/** + * Send Document + * Send a specific document by email. + */ +export async function main( + auth: Holded, + docType: string, + documentId: string, + body: { + mailTemplateId?: string; + emails: string; + subject?: string; + message?: string; + docIds?: string; + }, +) { + const url = new URL( + \`https://api.holded.com/api/invoicing/v1/documents/\${docType}/\${documentId}/send\`, + ); + + const response = await fetch(url, { + method: "POST", + headers: { + "Content-Type": "application/json", + key: auth.apiKey, + }, + body: JSON.stringify(body), + }); + if (!response.ok) { + const text = await response.text(); + throw new Error(\`\${response.status} \${text}\`); + } + return await response.json(); +} +`, + schema: { + type: 'object', + required: ['auth', 'docType', 'documentId', 'body'], + properties: { + auth: { type: 'object', format: 'resource-holded' }, + docType: { type: 'string' }, + documentId: { type: 'string' }, + body: { type: 'object' } + } + } + }, + { + version_id: 28294, + app: 'discord', + summary: 'Send a message to Discord using Webhook', + terms: 'discord webhook message send chat channel', + language: 'bunnative', + content: `//native + +type DiscordWebhook = { + webhook_url: string; +}; +export async function main(discord_webhook: DiscordWebhook, message: string) { + const response = await fetch(\`\${discord_webhook.webhook_url}?wait=true\`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ content: message }), + }); + if (!response.ok) { + throw new Error(\`\${response.status} \${await response.text()}\`); + } + return await response.json(); +} +`, + schema: { + type: 'object', + required: ['discord_webhook', 'message'], + properties: { + discord_webhook: { type: 'object', format: 'resource-discord_webhook' }, + message: { type: 'string' } + } + } + } +] + +/** Naive whole-word overlap — enough to rank a handful of fixtures for a natural + * query without pulling an embedding model into the benchmark. Every frontend eval + * shares this handler, so the bar to match is deliberately high: naming the + * integration, or overlapping on three meaningful words. A looser bar answers + * "send a Slack message" with the Discord fixture, handing an unrelated case a + * plausible-looking wrong integration. */ +function searchBenchmarkHubScripts(text: string) { + const tokens = new Set( + text + .toLowerCase() + .split(/[^a-z0-9]+/) + .filter((token) => token.length > 2) + ) + return BENCHMARK_HUB_SCRIPTS.map((script) => { + const words = new Set( + `${script.app} ${script.summary} ${script.terms}`.toLowerCase().split(/[^a-z0-9]+/) + ) + const score = [...tokens].filter((token) => words.has(token)).length + return { script, score, namesApp: tokens.has(script.app) } + }) + .filter((entry) => entry.namesApp || entry.score >= 3) + .sort((a, b) => b.score - a.score) + .map(({ script }, index) => ({ + ask_id: script.version_id, + id: script.version_id, + version_id: script.version_id, + summary: script.summary, + app: script.app, + kind: 'script', + score: 1 - index * 0.01 + })) +} + +/** The hub keys a script by its version id; the app and slug segments that + * follow are descriptive, so match on the id exactly as the real hub does. */ +function getBenchmarkHubScript(path: string) { + const versionId = Number(path.replace(/^\/api\/scripts\/hub\/get_full\/hub\//, '').split('/')[0]) + return BENCHMARK_HUB_SCRIPTS.find((script) => script.version_id === versionId) +} + const BENCHMARK_WORKERS = [ { worker: 'wk-benchmark-1', @@ -837,10 +973,16 @@ const BENCHMARK_WORKERS = [ * intercepting it with a synthetic 404 sends the model into retry loops. */ export function hasBenchmarkApiHandler(url: string): boolean { const path = url.split('?')[0] - return path === '/api/workers/list' || /^\/api\/w\/[^/]+\/jobs\/queue\/list$/.test(path) + return ( + path === '/api/workers/list' || + /^\/api\/w\/[^/]+\/jobs\/queue\/list$/.test(path) || + path === '/api/embeddings/query_hub_scripts' || + path.startsWith('/api/scripts/hub/get_full/') + ) } -/** Answer a relative `/api/...` fetch issued by the API catalog executor. */ +/** Answer a relative `/api/...` fetch — from the API catalog executor, or from the + * chat's hub tools. */ export function handleBenchmarkApiFetch(url: string): Response { const path = url.split('?')[0] if (path === '/api/workers/list') { @@ -849,5 +991,21 @@ export function handleBenchmarkApiFetch(url: string): Response { if (/^\/api\/w\/[^/]+\/jobs\/queue\/list$/.test(path)) { return Response.json([]) } + if (path === '/api/embeddings/query_hub_scripts') { + const text = new URLSearchParams(url.split('?')[1] ?? '').get('text') ?? '' + return Response.json(searchBenchmarkHubScripts(text)) + } + if (path.startsWith('/api/scripts/hub/get_full/')) { + const script = getBenchmarkHubScript(path) + if (!script) { + return Response.json({ error: 'hub script not found' }, { status: 404 }) + } + return Response.json({ + content: script.content, + language: script.language, + schema: script.schema, + summary: script.summary + }) + } return Response.json({ error: `no benchmark handler for ${path}` }, { status: 404 }) } diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml index 72739d2778..005c677e79 100644 --- a/ai_evals/cases/global.yaml +++ b/ai_evals/cases/global.yaml @@ -1877,3 +1877,49 @@ stringIncludesAnyOf: - email skipJudge: true + +# --- Windmill Hub reuse (search_hub_scripts + read_workspace_item on a hub/ path) --- +# Holded's API is obscure enough that a model writing from memory cannot reproduce +# its endpoint and `key` auth header — so the draft's fidelity to the published +# script is what proves the hub content was actually fetched, not guessed. +- id: global-hub1-reuse-hub-script + prompt: |- + I want to email one of my Holded invoices to a customer from Windmill. + There is already a script for that on the Windmill hub — reuse it instead of writing + your own, and save it as a draft script at `f/evals/global/holded_send_document`. + Leave it as an AI draft; do not deploy it. + initial: ai_evals/fixtures/frontend/global/initial/user_admin_evals_folder.json + runtime: + maxTurns: 12 + validate: + draftCountExactly: 1 + requiredDrafts: + - type: script + path: f/evals/global/holded_send_document + language: bun + valueIncludes: + - api.holded.com/api/invoicing/v1/documents + # `mailTemplateId` is an optional field of the published script's body + # that a model writing from memory does not invent, so it is what + # separates reusing the hub script from re-deriving one that merely + # hits the same endpoint. + - mailTemplateId + toolExpect: + requiredToolsUsed: + - search_hub_scripts + - read_workspace_item + - write_script + forbiddenToolsUsed: + - deploy_workspace_item + - delete_workspace_item + toolCallArgs: + - tool: read_workspace_item + field: path + stringIncludesAnyOf: + - hub/ + judgeChecklist: + - the draft sends an existing Holded document by email rather than creating one + - the request targets Holded's document send endpoint, not an invented URL + - authentication uses Holded's own key header rather than a bearer token + - the document type, document id, and recipient emails are inputs to the script + - the result stays an AI draft and is not deployed diff --git a/backend/.sqlx/query-2ef832a6d8e9feb83de6706b0ffe8e849e56f8db795df2ad4d907cdb3c9fce60.json b/backend/.sqlx/query-2ef832a6d8e9feb83de6706b0ffe8e849e56f8db795df2ad4d907cdb3c9fce60.json new file mode 100644 index 0000000000..183c620240 --- /dev/null +++ b/backend/.sqlx/query-2ef832a6d8e9feb83de6706b0ffe8e849e56f8db795df2ad4d907cdb3c9fce60.json @@ -0,0 +1,24 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT DISTINCT flow_step_id as \"flow_step_id!\" FROM v2_job\n WHERE parent_job = $1 AND workspace_id = $2 AND flow_step_id IS NOT NULL\n AND ($3::text[] IS NULL OR tag = ANY($3))\n ORDER BY flow_step_id", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "flow_step_id!", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Uuid", + "Text", + "TextArray" + ] + }, + "nullable": [ + true + ] + }, + "hash": "2ef832a6d8e9feb83de6706b0ffe8e849e56f8db795df2ad4d907cdb3c9fce60" +} diff --git a/backend/.sqlx/query-48c2eeca50aedc1895256f615aecc8b68627c8ed9a6da969674734854ef2742a.json b/backend/.sqlx/query-48c2eeca50aedc1895256f615aecc8b68627c8ed9a6da969674734854ef2742a.json new file mode 100644 index 0000000000..93b908a567 --- /dev/null +++ b/backend/.sqlx/query-48c2eeca50aedc1895256f615aecc8b68627c8ed9a6da969674734854ef2742a.json @@ -0,0 +1,37 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT j.id,\n COALESCE(c.status::text,\n CASE WHEN q.running AND q.suspend > 0 THEN 'suspended'\n WHEN q.running THEN 'running'\n ELSE 'queued' END) as \"status!\",\n COALESCE((\n SELECT m->'value'->>'type'\n FROM v2_job parent_j\n LEFT JOIN flow_version fv ON fv.id = parent_j.runnable_id\n AND parent_j.kind::text = 'flow'\n LEFT JOIN flow f ON f.path = parent_j.runnable_path\n AND f.workspace_id = parent_j.workspace_id\n LEFT JOIN flow_node fn ON fn.id = parent_j.runnable_id\n CROSS JOIN LATERAL jsonb_array_elements(\n COALESCE(parent_j.raw_flow, fv.value, f.value, fn.flow)->'modules'\n ) m\n WHERE parent_j.id = $1\n AND m->>'id' = $3\n LIMIT 1\n ), '')::text as \"parent_module_type!\"\n FROM v2_job j\n LEFT JOIN v2_job_completed c ON c.id = j.id\n LEFT JOIN v2_job_queue q ON q.id = j.id\n LEFT JOIN (\n SELECT fj.jid, fj.ord\n FROM (SELECT COALESCE(\n (SELECT flow_status FROM v2_job_completed WHERE id = $1),\n (SELECT flow_status FROM v2_job_status WHERE id = $1)\n ) AS fs) pf\n CROSS JOIN LATERAL (\n SELECT m FROM jsonb_array_elements(pf.fs->'modules') m\n WHERE m->>'id' = $3\n LIMIT 1\n ) md\n CROSS JOIN LATERAL jsonb_array_elements_text(md.m->'flow_jobs')\n WITH ORDINALITY fj(jid, ord)\n ) pos ON pos.jid = j.id::text\n WHERE j.parent_job = $1 AND j.workspace_id = $2 AND j.flow_step_id = $3\n AND ($4::text[] IS NULL OR j.tag = ANY($4))\n ORDER BY pos.ord NULLS LAST, j.id", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Uuid" + }, + { + "ordinal": 1, + "name": "status!", + "type_info": "Text" + }, + { + "ordinal": 2, + "name": "parent_module_type!", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Uuid", + "Text", + "Text", + "TextArray" + ] + }, + "nullable": [ + false, + null, + null + ] + }, + "hash": "48c2eeca50aedc1895256f615aecc8b68627c8ed9a6da969674734854ef2742a" +} diff --git a/backend/.sqlx/query-5b95f464d492227ffe8a449844da00a6539788f9be647d6b52f01cbb2967473e.json b/backend/.sqlx/query-5b95f464d492227ffe8a449844da00a6539788f9be647d6b52f01cbb2967473e.json deleted file mode 100644 index 3b1d7fae8c..0000000000 --- a/backend/.sqlx/query-5b95f464d492227ffe8a449844da00a6539788f9be647d6b52f01cbb2967473e.json +++ /dev/null @@ -1,83 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "WITH RECURSIVE job_tree AS (\n SELECT j.id, j.kind::text, j.flow_step_id, j.parent_job,\n '' as path_label, 0 as depth,\n j.id::text as id_path,\n ''::text as parent_module_type\n FROM v2_job j\n WHERE j.id = $2 AND j.workspace_id = $1\n UNION ALL\n SELECT j.id, j.kind::text, j.flow_step_id, j.parent_job,\n CASE\n WHEN jt.path_label = '' THEN COALESCE(j.flow_step_id, '')\n ELSE jt.path_label || '/' || COALESCE(j.flow_step_id, '')\n END,\n jt.depth + 1,\n jt.id_path || '/' || j.id::text,\n COALESCE((\n SELECT m->'value'->>'type'\n FROM v2_job parent_j\n LEFT JOIN flow f ON f.path = parent_j.runnable_path\n AND f.workspace_id = parent_j.workspace_id\n LEFT JOIN flow_node fn ON fn.id = parent_j.runnable_id\n CROSS JOIN LATERAL jsonb_array_elements(\n COALESCE(parent_j.raw_flow, f.value, fn.flow)->'modules'\n ) m\n WHERE parent_j.id = jt.id\n AND m->>'id' = j.flow_step_id\n LIMIT 1\n ), '')::text\n FROM v2_job j\n JOIN job_tree jt ON j.parent_job = jt.id\n WHERE j.workspace_id = $1\n ),\n with_sibling_index AS (\n SELECT jt.*,\n ROW_NUMBER() OVER (\n PARTITION BY jt.parent_job, jt.flow_step_id\n ORDER BY jt.id\n ) as sibling_index,\n COUNT(*) OVER (\n PARTITION BY jt.parent_job, jt.flow_step_id\n ) as sibling_count\n FROM job_tree jt\n )\n SELECT w.id, w.kind, w.flow_step_id, w.path_label,\n w.sibling_index::int as sibling_index,\n w.sibling_count::int as sibling_count,\n w.depth::int as depth,\n w.parent_module_type,\n coalesce(job_logs.logs, '') as logs,\n COALESCE(job_logs.log_offset, 0) as log_offset,\n job_logs.log_file_index\n FROM with_sibling_index w\n LEFT JOIN job_logs ON job_logs.job_id = w.id\n ORDER BY w.id_path ASC", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "id", - "type_info": "Uuid" - }, - { - "ordinal": 1, - "name": "kind", - "type_info": "Text" - }, - { - "ordinal": 2, - "name": "flow_step_id", - "type_info": "Varchar" - }, - { - "ordinal": 3, - "name": "path_label", - "type_info": "Text" - }, - { - "ordinal": 4, - "name": "sibling_index", - "type_info": "Int4" - }, - { - "ordinal": 5, - "name": "sibling_count", - "type_info": "Int4" - }, - { - "ordinal": 6, - "name": "depth", - "type_info": "Int4" - }, - { - "ordinal": 7, - "name": "parent_module_type", - "type_info": "Text" - }, - { - "ordinal": 8, - "name": "logs", - "type_info": "Text" - }, - { - "ordinal": 9, - "name": "log_offset", - "type_info": "Int4" - }, - { - "ordinal": 10, - "name": "log_file_index", - "type_info": "TextArray" - } - ], - "parameters": { - "Left": [ - "Text", - "Uuid" - ] - }, - "nullable": [ - null, - null, - null, - null, - null, - null, - null, - null, - null, - null, - true - ] - }, - "hash": "5b95f464d492227ffe8a449844da00a6539788f9be647d6b52f01cbb2967473e" -} diff --git a/backend/.sqlx/query-6981553b904f9ae38d6eaeae05716ac71390028dfe8c54798b1701b01591fff7.json b/backend/.sqlx/query-6981553b904f9ae38d6eaeae05716ac71390028dfe8c54798b1701b01591fff7.json new file mode 100644 index 0000000000..1a965a6dc3 --- /dev/null +++ b/backend/.sqlx/query-6981553b904f9ae38d6eaeae05716ac71390028dfe8c54798b1701b01591fff7.json @@ -0,0 +1,66 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT j.kind::text as kind,\n c.status::text as completed_status,\n c.duration_ms as \"duration_ms?\",\n COALESCE(c.started_at, q.started_at) as started_at,\n LEFT(c.result::text, $3) as result_prefix,\n length(c.result::text) as result_length,\n q.running as \"q_running?\",\n q.suspend as \"q_suspend?\"\n FROM v2_job j\n LEFT JOIN v2_job_completed c ON c.id = j.id\n LEFT JOIN v2_job_queue q ON q.id = j.id\n WHERE j.id = $1 AND j.workspace_id = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "kind", + "type_info": "Text" + }, + { + "ordinal": 1, + "name": "completed_status", + "type_info": "Text" + }, + { + "ordinal": 2, + "name": "duration_ms?", + "type_info": "Int8" + }, + { + "ordinal": 3, + "name": "started_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 4, + "name": "result_prefix", + "type_info": "Text" + }, + { + "ordinal": 5, + "name": "result_length", + "type_info": "Int4" + }, + { + "ordinal": 6, + "name": "q_running?", + "type_info": "Bool" + }, + { + "ordinal": 7, + "name": "q_suspend?", + "type_info": "Int4" + } + ], + "parameters": { + "Left": [ + "Uuid", + "Text", + "Int4" + ] + }, + "nullable": [ + null, + null, + false, + null, + null, + null, + false, + false + ] + }, + "hash": "6981553b904f9ae38d6eaeae05716ac71390028dfe8c54798b1701b01591fff7" +} diff --git a/backend/.sqlx/query-94f0e271a67179c78de217ae1f4d7190324533119792b650c6db734a3b99575c.json b/backend/.sqlx/query-94f0e271a67179c78de217ae1f4d7190324533119792b650c6db734a3b99575c.json new file mode 100644 index 0000000000..c284efca75 --- /dev/null +++ b/backend/.sqlx/query-94f0e271a67179c78de217ae1f4d7190324533119792b650c6db734a3b99575c.json @@ -0,0 +1,84 @@ +{ + "db_name": "PostgreSQL", + "query": "WITH RECURSIVE job_tree AS (\n SELECT j.id, j.kind::text, j.flow_step_id, j.parent_job,\n '' as path_label, 0 as depth,\n j.id::text as id_path,\n ''::text as parent_module_type\n FROM v2_job j\n WHERE j.id = $2 AND j.workspace_id = $1\n UNION ALL\n SELECT j.id, j.kind::text, j.flow_step_id, j.parent_job,\n CASE\n WHEN jt.path_label = '' THEN COALESCE(j.flow_step_id, '')\n ELSE jt.path_label || '/' || COALESCE(j.flow_step_id, '')\n END,\n jt.depth + 1,\n jt.id_path || '/' || j.id::text,\n COALESCE((\n SELECT m->'value'->>'type'\n FROM v2_job parent_j\n LEFT JOIN flow_version fv ON fv.id = parent_j.runnable_id\n AND parent_j.kind::text = 'flow'\n LEFT JOIN flow f ON f.path = parent_j.runnable_path\n AND f.workspace_id = parent_j.workspace_id\n LEFT JOIN flow_node fn ON fn.id = parent_j.runnable_id\n CROSS JOIN LATERAL jsonb_array_elements(\n COALESCE(parent_j.raw_flow, fv.value, f.value, fn.flow)->'modules'\n ) m\n WHERE parent_j.id = jt.id\n AND m->>'id' = j.flow_step_id\n LIMIT 1\n ), '')::text\n FROM v2_job j\n JOIN job_tree jt ON j.parent_job = jt.id\n WHERE j.workspace_id = $1\n AND ($3::text[] IS NULL OR j.tag = ANY($3))\n ),\n positions AS (\n SELECT g.parent_job, g.flow_step_id, fj.jid, fj.ord\n FROM (SELECT DISTINCT parent_job, flow_step_id FROM job_tree\n WHERE parent_job IS NOT NULL AND flow_step_id IS NOT NULL) g\n CROSS JOIN LATERAL (\n SELECT COALESCE(\n (SELECT flow_status FROM v2_job_completed WHERE id = g.parent_job),\n (SELECT flow_status FROM v2_job_status WHERE id = g.parent_job)\n ) AS fs\n ) pf\n CROSS JOIN LATERAL (\n SELECT m FROM jsonb_array_elements(pf.fs->'modules') m\n WHERE m->>'id' = g.flow_step_id\n LIMIT 1\n ) md\n CROSS JOIN LATERAL jsonb_array_elements_text(md.m->'flow_jobs')\n WITH ORDINALITY fj(jid, ord)\n ),\n with_sibling_index AS (\n SELECT jt.*,\n ROW_NUMBER() OVER (\n PARTITION BY jt.parent_job, jt.flow_step_id\n ORDER BY pos.ord NULLS LAST, jt.id\n ) as sibling_index,\n COUNT(*) OVER (\n PARTITION BY jt.parent_job, jt.flow_step_id\n ) as sibling_count\n FROM job_tree jt\n LEFT JOIN positions pos ON pos.parent_job = jt.parent_job\n AND pos.flow_step_id = jt.flow_step_id\n AND pos.jid = jt.id::text\n )\n SELECT w.id, w.kind, w.flow_step_id, w.path_label,\n w.sibling_index::int as sibling_index,\n w.sibling_count::int as sibling_count,\n w.depth::int as depth,\n w.parent_module_type,\n coalesce(job_logs.logs, '') as logs,\n COALESCE(job_logs.log_offset, 0) as log_offset,\n job_logs.log_file_index\n FROM with_sibling_index w\n LEFT JOIN job_logs ON job_logs.job_id = w.id\n ORDER BY w.id_path ASC", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Uuid" + }, + { + "ordinal": 1, + "name": "kind", + "type_info": "Text" + }, + { + "ordinal": 2, + "name": "flow_step_id", + "type_info": "Varchar" + }, + { + "ordinal": 3, + "name": "path_label", + "type_info": "Text" + }, + { + "ordinal": 4, + "name": "sibling_index", + "type_info": "Int4" + }, + { + "ordinal": 5, + "name": "sibling_count", + "type_info": "Int4" + }, + { + "ordinal": 6, + "name": "depth", + "type_info": "Int4" + }, + { + "ordinal": 7, + "name": "parent_module_type", + "type_info": "Text" + }, + { + "ordinal": 8, + "name": "logs", + "type_info": "Text" + }, + { + "ordinal": 9, + "name": "log_offset", + "type_info": "Int4" + }, + { + "ordinal": 10, + "name": "log_file_index", + "type_info": "TextArray" + } + ], + "parameters": { + "Left": [ + "Text", + "Uuid", + "TextArray" + ] + }, + "nullable": [ + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + true + ] + }, + "hash": "94f0e271a67179c78de217ae1f4d7190324533119792b650c6db734a3b99575c" +} diff --git a/backend/.sqlx/query-b8e732a03969666444f73397ac153d23ec2af084b2f93da24c920532c1916384.json b/backend/.sqlx/query-b8e732a03969666444f73397ac153d23ec2af084b2f93da24c920532c1916384.json new file mode 100644 index 0000000000..b892061f56 --- /dev/null +++ b/backend/.sqlx/query-b8e732a03969666444f73397ac153d23ec2af084b2f93da24c920532c1916384.json @@ -0,0 +1,110 @@ +{ + "db_name": "PostgreSQL", + "query": "WITH RECURSIVE job_tree AS (\n SELECT j.id, j.kind::text, j.flow_step_id, j.parent_job,\n '' as path_label, 0 as depth,\n j.id::text as id_path,\n ''::text as parent_module_type\n FROM v2_job j\n WHERE j.id = $2 AND j.workspace_id = $1\n UNION ALL\n SELECT j.id, j.kind::text, j.flow_step_id, j.parent_job,\n CASE\n WHEN jt.path_label = '' THEN COALESCE(j.flow_step_id, '')\n ELSE jt.path_label || '/' || COALESCE(j.flow_step_id, '')\n END,\n jt.depth + 1,\n jt.id_path || '/' || j.id::text,\n COALESCE((\n SELECT m->'value'->>'type'\n FROM v2_job parent_j\n LEFT JOIN flow_version fv ON fv.id = parent_j.runnable_id\n AND parent_j.kind::text = 'flow'\n LEFT JOIN flow f ON f.path = parent_j.runnable_path\n AND f.workspace_id = parent_j.workspace_id\n LEFT JOIN flow_node fn ON fn.id = parent_j.runnable_id\n CROSS JOIN LATERAL jsonb_array_elements(\n COALESCE(parent_j.raw_flow, fv.value, f.value, fn.flow)->'modules'\n ) m\n WHERE parent_j.id = jt.id\n AND m->>'id' = j.flow_step_id\n LIMIT 1\n ), '')::text\n FROM v2_job j\n JOIN job_tree jt ON j.parent_job = jt.id\n WHERE j.workspace_id = $1\n AND ($4::text[] IS NULL OR j.tag = ANY($4))\n ),\n positions AS (\n SELECT g.parent_job, g.flow_step_id, fj.jid, fj.ord\n FROM (SELECT DISTINCT parent_job, flow_step_id FROM job_tree\n WHERE parent_job IS NOT NULL AND flow_step_id IS NOT NULL) g\n CROSS JOIN LATERAL (\n SELECT COALESCE(\n (SELECT flow_status FROM v2_job_completed WHERE id = g.parent_job),\n (SELECT flow_status FROM v2_job_status WHERE id = g.parent_job)\n ) AS fs\n ) pf\n CROSS JOIN LATERAL (\n SELECT m FROM jsonb_array_elements(pf.fs->'modules') m\n WHERE m->>'id' = g.flow_step_id\n LIMIT 1\n ) md\n CROSS JOIN LATERAL jsonb_array_elements_text(md.m->'flow_jobs')\n WITH ORDINALITY fj(jid, ord)\n ),\n with_sibling_index AS (\n SELECT jt.*,\n ROW_NUMBER() OVER (\n PARTITION BY jt.parent_job, jt.flow_step_id\n ORDER BY pos.ord NULLS LAST, jt.id\n ) as sibling_index,\n COUNT(*) OVER (\n PARTITION BY jt.parent_job, jt.flow_step_id\n ) as sibling_count\n FROM job_tree jt\n LEFT JOIN positions pos ON pos.parent_job = jt.parent_job\n AND pos.flow_step_id = jt.flow_step_id\n AND pos.jid = jt.id::text\n ),\n limited AS (\n SELECT * FROM with_sibling_index ORDER BY id_path ASC LIMIT $5\n )\n SELECT w.id, w.kind, w.flow_step_id, w.path_label,\n w.sibling_index::int as sibling_index,\n w.sibling_count::int as sibling_count,\n w.depth::int as depth,\n w.parent_module_type,\n c.status::text as completed_status,\n c.duration_ms as \"duration_ms?\",\n COALESCE(c.started_at, q.started_at) as started_at,\n LEFT(c.result::text, $3) as result_prefix,\n length(c.result::text) as result_length,\n q.running as \"q_running?\",\n q.suspend as \"q_suspend?\"\n FROM limited w\n LEFT JOIN v2_job_completed c ON c.id = w.id\n LEFT JOIN v2_job_queue q ON q.id = w.id\n ORDER BY w.id_path ASC", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Uuid" + }, + { + "ordinal": 1, + "name": "kind", + "type_info": "Text" + }, + { + "ordinal": 2, + "name": "flow_step_id", + "type_info": "Varchar" + }, + { + "ordinal": 3, + "name": "path_label", + "type_info": "Text" + }, + { + "ordinal": 4, + "name": "sibling_index", + "type_info": "Int4" + }, + { + "ordinal": 5, + "name": "sibling_count", + "type_info": "Int4" + }, + { + "ordinal": 6, + "name": "depth", + "type_info": "Int4" + }, + { + "ordinal": 7, + "name": "parent_module_type", + "type_info": "Text" + }, + { + "ordinal": 8, + "name": "completed_status", + "type_info": "Text" + }, + { + "ordinal": 9, + "name": "duration_ms?", + "type_info": "Int8" + }, + { + "ordinal": 10, + "name": "started_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 11, + "name": "result_prefix", + "type_info": "Text" + }, + { + "ordinal": 12, + "name": "result_length", + "type_info": "Int4" + }, + { + "ordinal": 13, + "name": "q_running?", + "type_info": "Bool" + }, + { + "ordinal": 14, + "name": "q_suspend?", + "type_info": "Int4" + } + ], + "parameters": { + "Left": [ + "Text", + "Uuid", + "Int4", + "TextArray", + "Int8" + ] + }, + "nullable": [ + null, + null, + null, + null, + null, + null, + null, + null, + null, + false, + null, + null, + null, + false, + false + ] + }, + "hash": "b8e732a03969666444f73397ac153d23ec2af084b2f93da24c920532c1916384" +} diff --git a/backend/.sqlx/query-bd594f06413dd21d7e6923e6754b58fbce8c09e6ec8172d9ed94716958a29b40.json b/backend/.sqlx/query-bd594f06413dd21d7e6923e6754b58fbce8c09e6ec8172d9ed94716958a29b40.json new file mode 100644 index 0000000000..5e9e4bfa6e --- /dev/null +++ b/backend/.sqlx/query-bd594f06413dd21d7e6923e6754b58fbce8c09e6ec8172d9ed94716958a29b40.json @@ -0,0 +1,30 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT created_by, COALESCE(flow_innermost_root_job, parent_job) as enclosing_job FROM v2_job WHERE id = $1 AND workspace_id = $2 AND ($3::text[] IS NULL OR tag = ANY($3))", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "created_by", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "enclosing_job", + "type_info": "Uuid" + } + ], + "parameters": { + "Left": [ + "Uuid", + "Text", + "TextArray" + ] + }, + "nullable": [ + false, + null + ] + }, + "hash": "bd594f06413dd21d7e6923e6754b58fbce8c09e6ec8172d9ed94716958a29b40" +} diff --git a/backend/Cargo.lock b/backend/Cargo.lock index d7f7f16891..11167f13cf 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -4482,13 +4482,13 @@ dependencies = [ [[package]] name = "displaydoc" -version = "0.2.6" +version = "0.2.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ac70aa55017e108007fbaf5aa0f54b021c98f92ff8af59d42eda9da96e3dd4f" +checksum = "c6232dd377dcc64799954cbd3a9bb882e9cdc1308ccd87b1c098f1fb2eaf82a8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.119", + "syn 3.0.3", ] [[package]] @@ -7073,7 +7073,7 @@ dependencies = [ "bitflags 2.13.1", "libc", "plain", - "redox_syscall 0.9.0", + "redox_syscall 0.9.1", ] [[package]] @@ -9920,9 +9920,9 @@ dependencies = [ [[package]] name = "redox_syscall" -version = "0.9.0" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c5102a6aaa05aa011a238e178e6bca86d2cb56fc9f586d37cb80f5bca6e07759" +checksum = "07507be7b4a5f9f26eeb41eeaebb1f5a7ff29dfb29739facc21d35bf8b11c21e" dependencies = [ "bitflags 2.13.1", ] @@ -14489,7 +14489,7 @@ dependencies = [ [[package]] name = "windmill" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-nats", @@ -14574,7 +14574,7 @@ dependencies = [ [[package]] name = "windmill-ai" -version = "1.773.0" +version = "1.775.1" dependencies = [ "async-stream", "async-trait", @@ -14607,7 +14607,7 @@ dependencies = [ [[package]] name = "windmill-alerting" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14620,7 +14620,7 @@ dependencies = [ [[package]] name = "windmill-api" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "argon2", @@ -14759,7 +14759,7 @@ dependencies = [ [[package]] name = "windmill-api-agent-workers" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14782,7 +14782,7 @@ dependencies = [ [[package]] name = "windmill-api-assets" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14797,7 +14797,7 @@ dependencies = [ [[package]] name = "windmill-api-auth" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "axum 0.8.9", @@ -14823,7 +14823,7 @@ dependencies = [ [[package]] name = "windmill-api-client" -version = "1.773.0" +version = "1.775.1" dependencies = [ "reqwest 0.12.28", "serde", @@ -14833,7 +14833,7 @@ dependencies = [ [[package]] name = "windmill-api-configs" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14850,7 +14850,7 @@ dependencies = [ [[package]] name = "windmill-api-debug" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "base64 0.22.1", @@ -14872,7 +14872,7 @@ dependencies = [ [[package]] name = "windmill-api-embeddings" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "axum 0.8.9", @@ -14895,7 +14895,7 @@ dependencies = [ [[package]] name = "windmill-api-flow-conversations" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14911,7 +14911,7 @@ dependencies = [ [[package]] name = "windmill-api-flows" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14932,7 +14932,7 @@ dependencies = [ [[package]] name = "windmill-api-groups" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14953,7 +14953,7 @@ dependencies = [ [[package]] name = "windmill-api-inputs" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14967,7 +14967,7 @@ dependencies = [ [[package]] name = "windmill-api-integration-tests" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-nats", @@ -15002,7 +15002,7 @@ dependencies = [ [[package]] name = "windmill-api-jobs" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "axum 0.8.9", @@ -15027,7 +15027,7 @@ dependencies = [ [[package]] name = "windmill-api-npm-proxy" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "flate2", @@ -15045,7 +15045,7 @@ dependencies = [ [[package]] name = "windmill-api-openapi" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "axum 0.8.9", @@ -15067,7 +15067,7 @@ dependencies = [ [[package]] name = "windmill-api-schedule" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "chrono", @@ -15087,7 +15087,7 @@ dependencies = [ [[package]] name = "windmill-api-scripts" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "chrono", @@ -15124,7 +15124,7 @@ dependencies = [ [[package]] name = "windmill-api-settings" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "axum 0.8.9", @@ -15152,7 +15152,7 @@ dependencies = [ [[package]] name = "windmill-api-sse" -version = "1.773.0" +version = "1.775.1" dependencies = [ "lazy_static", "serde", @@ -15164,7 +15164,7 @@ dependencies = [ [[package]] name = "windmill-api-users" -version = "1.773.0" +version = "1.775.1" dependencies = [ "argon2", "axum 0.8.9", @@ -15189,7 +15189,7 @@ dependencies = [ [[package]] name = "windmill-api-workers" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "chrono", @@ -15203,7 +15203,7 @@ dependencies = [ [[package]] name = "windmill-api-workspaces" -version = "1.773.0" +version = "1.775.1" dependencies = [ "axum 0.8.9", "chrono", @@ -15238,7 +15238,7 @@ dependencies = [ [[package]] name = "windmill-audit" -version = "1.773.0" +version = "1.775.1" dependencies = [ "chrono", "lazy_static", @@ -15252,7 +15252,7 @@ dependencies = [ [[package]] name = "windmill-autoscaling" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "axum 0.8.9", @@ -15271,7 +15271,7 @@ dependencies = [ [[package]] name = "windmill-common" -version = "1.773.0" +version = "1.775.1" dependencies = [ "aes-gcm", "aho-corasick", @@ -15375,7 +15375,7 @@ dependencies = [ [[package]] name = "windmill-dep-map" -version = "1.773.0" +version = "1.775.1" dependencies = [ "chrono", "itertools 0.14.0", @@ -15394,7 +15394,7 @@ dependencies = [ [[package]] name = "windmill-git-sync" -version = "1.773.0" +version = "1.775.1" dependencies = [ "regex", "serde", @@ -15409,7 +15409,7 @@ dependencies = [ [[package]] name = "windmill-indexer" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "astral-tokio-tar", @@ -15433,7 +15433,7 @@ dependencies = [ [[package]] name = "windmill-jseval" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "futures", @@ -15450,7 +15450,7 @@ dependencies = [ [[package]] name = "windmill-macros" -version = "1.773.0" +version = "1.775.1" dependencies = [ "itertools 0.14.0", "lazy_static", @@ -15466,7 +15466,7 @@ dependencies = [ [[package]] name = "windmill-mcp" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -15487,7 +15487,7 @@ dependencies = [ [[package]] name = "windmill-native-triggers" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -15518,7 +15518,7 @@ dependencies = [ [[package]] name = "windmill-oauth" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "arc-swap", @@ -15543,7 +15543,7 @@ dependencies = [ [[package]] name = "windmill-object-store" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-stream", @@ -15577,7 +15577,7 @@ dependencies = [ [[package]] name = "windmill-operator" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "futures", @@ -15595,7 +15595,7 @@ dependencies = [ [[package]] name = "windmill-parser" -version = "1.773.0" +version = "1.775.1" dependencies = [ "convert_case 0.6.0", "serde", @@ -15604,7 +15604,7 @@ dependencies = [ [[package]] name = "windmill-parser-bash" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "lazy_static", @@ -15616,7 +15616,7 @@ dependencies = [ [[package]] name = "windmill-parser-csharp" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "serde_json", @@ -15628,7 +15628,7 @@ dependencies = [ [[package]] name = "windmill-parser-go" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "gosyn", @@ -15640,7 +15640,7 @@ dependencies = [ [[package]] name = "windmill-parser-graphql" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "lazy_static", @@ -15652,7 +15652,7 @@ dependencies = [ [[package]] name = "windmill-parser-java" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "serde_json", @@ -15664,7 +15664,7 @@ dependencies = [ [[package]] name = "windmill-parser-nu" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "nu-parser", @@ -15675,7 +15675,7 @@ dependencies = [ [[package]] name = "windmill-parser-php" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "itertools 0.14.0", @@ -15686,7 +15686,7 @@ dependencies = [ [[package]] name = "windmill-parser-py" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "itertools 0.14.0", @@ -15698,7 +15698,7 @@ dependencies = [ [[package]] name = "windmill-parser-py-asset" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "rustpython-ast", @@ -15709,7 +15709,7 @@ dependencies = [ [[package]] name = "windmill-parser-py-imports" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-recursion", @@ -15731,7 +15731,7 @@ dependencies = [ [[package]] name = "windmill-parser-r" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "serde_json", @@ -15743,7 +15743,7 @@ dependencies = [ [[package]] name = "windmill-parser-ruby" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "lazy_static", @@ -15757,7 +15757,7 @@ dependencies = [ [[package]] name = "windmill-parser-rust" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "convert_case 0.6.0", @@ -15774,7 +15774,7 @@ dependencies = [ [[package]] name = "windmill-parser-sql" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "lazy_static", @@ -15787,7 +15787,7 @@ dependencies = [ [[package]] name = "windmill-parser-sql-asset" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "serde", @@ -15799,7 +15799,7 @@ dependencies = [ [[package]] name = "windmill-parser-ts" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "lazy_static", @@ -15817,7 +15817,7 @@ dependencies = [ [[package]] name = "windmill-parser-ts-asset" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "serde-wasm-bindgen", @@ -15833,7 +15833,7 @@ dependencies = [ [[package]] name = "windmill-parser-wac" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "rustpython-ast", @@ -15849,7 +15849,7 @@ dependencies = [ [[package]] name = "windmill-parser-yaml" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "serde", @@ -15860,7 +15860,7 @@ dependencies = [ [[package]] name = "windmill-queue" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-recursion", @@ -15899,7 +15899,7 @@ dependencies = [ [[package]] name = "windmill-runtime-nativets" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "const_format", @@ -15939,7 +15939,7 @@ dependencies = [ [[package]] name = "windmill-sql-datatype-parser-wasm" -version = "1.773.0" +version = "1.775.1" dependencies = [ "getrandom 0.3.4", "wasm-bindgen", @@ -15950,7 +15950,7 @@ dependencies = [ [[package]] name = "windmill-store" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-recursion", @@ -15984,7 +15984,7 @@ dependencies = [ [[package]] name = "windmill-test-utils" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -16008,7 +16008,7 @@ dependencies = [ [[package]] name = "windmill-trigger" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -16041,7 +16041,7 @@ dependencies = [ [[package]] name = "windmill-trigger-amqp" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -16068,7 +16068,7 @@ dependencies = [ [[package]] name = "windmill-trigger-azure" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -16101,7 +16101,7 @@ dependencies = [ [[package]] name = "windmill-trigger-email" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -16121,7 +16121,7 @@ dependencies = [ [[package]] name = "windmill-trigger-gcp" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -16155,7 +16155,7 @@ dependencies = [ [[package]] name = "windmill-trigger-http" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -16191,7 +16191,7 @@ dependencies = [ [[package]] name = "windmill-trigger-kafka" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -16214,7 +16214,7 @@ dependencies = [ [[package]] name = "windmill-trigger-mqtt" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -16238,7 +16238,7 @@ dependencies = [ [[package]] name = "windmill-trigger-nats" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-nats", @@ -16262,7 +16262,7 @@ dependencies = [ [[package]] name = "windmill-trigger-postgres" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -16297,7 +16297,7 @@ dependencies = [ [[package]] name = "windmill-trigger-sqs" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -16325,7 +16325,7 @@ dependencies = [ [[package]] name = "windmill-trigger-websocket" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-trait", @@ -16350,7 +16350,7 @@ dependencies = [ [[package]] name = "windmill-types" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "bitflags 2.13.1", @@ -16369,7 +16369,7 @@ dependencies = [ [[package]] name = "windmill-worker" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-once-cell", @@ -16484,7 +16484,7 @@ dependencies = [ [[package]] name = "windmill-worker-volumes" -version = "1.773.0" +version = "1.775.1" dependencies = [ "bytes", "futures", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 6ab31d6809..49beac0b8a 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "windmill" -version = "1.773.0" +version = "1.775.1" authors.workspace = true edition.workspace = true @@ -88,7 +88,7 @@ members = [ exclude = ["./windmill-duckdb-ffi-internal", "./parsers/windmill-parser-wasm"] [workspace.package] -version = "1.773.0" +version = "1.775.1" authors = ["Ruben Fiszel "] edition = "2021" diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index c8df5f828e..d2079a69f3 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -72b9d9fd2eece229631c6dc13bfb010c8ea664ac +4e0358ab7301a9cffe29508dd5cd75bcf5eee319 diff --git a/backend/parsers/windmill-parser-wasm/Cargo.lock b/backend/parsers/windmill-parser-wasm/Cargo.lock index 193a97ed0e..e3639e30c2 100644 --- a/backend/parsers/windmill-parser-wasm/Cargo.lock +++ b/backend/parsers/windmill-parser-wasm/Cargo.lock @@ -6191,7 +6191,7 @@ checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" [[package]] name = "windmill-common" -version = "1.773.0" +version = "1.775.1" dependencies = [ "aho-corasick", "anyhow", @@ -6272,7 +6272,7 @@ dependencies = [ [[package]] name = "windmill-macros" -version = "1.773.0" +version = "1.775.1" dependencies = [ "proc-macro2", "quote", @@ -6284,7 +6284,7 @@ dependencies = [ [[package]] name = "windmill-parser" -version = "1.773.0" +version = "1.775.1" dependencies = [ "convert_case", "serde", @@ -6293,7 +6293,7 @@ dependencies = [ [[package]] name = "windmill-parser-bash" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "lazy_static", @@ -6305,7 +6305,7 @@ dependencies = [ [[package]] name = "windmill-parser-csharp" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "serde_json", @@ -6317,7 +6317,7 @@ dependencies = [ [[package]] name = "windmill-parser-go" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "gosyn", @@ -6329,7 +6329,7 @@ dependencies = [ [[package]] name = "windmill-parser-graphql" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "lazy_static", @@ -6341,7 +6341,7 @@ dependencies = [ [[package]] name = "windmill-parser-java" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "serde_json", @@ -6353,7 +6353,7 @@ dependencies = [ [[package]] name = "windmill-parser-nu" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "nu-parser", @@ -6364,7 +6364,7 @@ dependencies = [ [[package]] name = "windmill-parser-php" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "itertools 0.14.0", @@ -6375,7 +6375,7 @@ dependencies = [ [[package]] name = "windmill-parser-py" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "itertools 0.14.0", @@ -6387,7 +6387,7 @@ dependencies = [ [[package]] name = "windmill-parser-py-asset" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "rustpython-ast", @@ -6398,7 +6398,7 @@ dependencies = [ [[package]] name = "windmill-parser-py-imports" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "async-recursion", @@ -6420,7 +6420,7 @@ dependencies = [ [[package]] name = "windmill-parser-r" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "serde_json", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "windmill-parser-ruby" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "lazy_static", @@ -6446,7 +6446,7 @@ dependencies = [ [[package]] name = "windmill-parser-rust" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "convert_case", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "windmill-parser-sql" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "lazy_static", @@ -6476,7 +6476,7 @@ dependencies = [ [[package]] name = "windmill-parser-sql-asset" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "serde", @@ -6488,7 +6488,7 @@ dependencies = [ [[package]] name = "windmill-parser-ts" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "lazy_static", @@ -6506,7 +6506,7 @@ dependencies = [ [[package]] name = "windmill-parser-ts-asset" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "serde-wasm-bindgen", @@ -6522,7 +6522,7 @@ dependencies = [ [[package]] name = "windmill-parser-wac" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "rustpython-ast", @@ -6538,7 +6538,7 @@ dependencies = [ [[package]] name = "windmill-parser-wasm" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "getrandom 0.2.17", @@ -6570,7 +6570,7 @@ dependencies = [ [[package]] name = "windmill-parser-yaml" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "serde", @@ -6581,7 +6581,7 @@ dependencies = [ [[package]] name = "windmill-types" -version = "1.773.0" +version = "1.775.1" dependencies = [ "anyhow", "bitflags", diff --git a/backend/parsers/windmill-parser-wasm/Cargo.toml b/backend/parsers/windmill-parser-wasm/Cargo.toml index 1866ea19d5..9ee10c7fbb 100644 --- a/backend/parsers/windmill-parser-wasm/Cargo.toml +++ b/backend/parsers/windmill-parser-wasm/Cargo.toml @@ -12,7 +12,7 @@ resolver = "2" members = ["."] [workspace.package] -version = "1.773.0" +version = "1.775.1" edition = "2021" authors = ["Ruben Fiszel "] diff --git a/backend/tests/datatable_migrations_grants.rs b/backend/tests/datatable_migrations_grants.rs new file mode 100644 index 0000000000..ebe9aeba4d --- /dev/null +++ b/backend/tests/datatable_migrations_grants.rs @@ -0,0 +1,300 @@ +//! Regression test for running data table migrations against a database whose +//! role only holds DML grants. +//! +//! Two failure modes are pinned here: +//! - the Postgres message must reach the caller. `tokio_postgres::Error`'s +//! `Display` renders only the error kind, so interpolating it with `{}` +//! produced a bare `Failed to ensure _wm_migrations table: db error`. +//! - `CREATE TABLE IF NOT EXISTS` checks CREATE on the schema *before* it +//! checks existence, so the run must probe for `_wm_migrations` first or an +//! unprivileged role can never migrate, even against a pre-created table. +//! +//! Plus the privilege report that surfaces the same state from workspace +//! settings before anyone reaches a migration. + +use serde_json::{json, Value}; +use sqlx::{Pool, Postgres}; + +use windmill_test_utils::*; + +const ROLE: &str = "wm_dtmig_test_role"; +const ROLE_PASSWORD: &str = "wm_dtmig_test_pwd"; +/// Deliberately hyphenated: it only parses inside double quotes, so it pins that +/// the emitted recovery statement quotes the role rather than interpolating it. +const NOSCHEMA_ROLE: &str = "wm-dtmig-noschema"; + +fn authed(b: reqwest::RequestBuilder) -> reqwest::RequestBuilder { + b.header("Authorization", "Bearer DTMIG_ADMIN_TOKEN") +} + +/// Point the fixture's data table at this test's own database, connecting as a +/// role that may read and write but not create: `GRANT USAGE` without `CREATE`, +/// and the schema's own CREATE revoked from PUBLIC so the outcome does not +/// depend on the server's default `public` grants (relaxed before Postgres 15). +async fn setup_unprivileged_datatable_role(db: &Pool) -> anyhow::Result<()> { + let opts = (*db.connect_options()).clone(); + let dbname = opts.get_database().expect("test database name").to_string(); + + sqlx::query(&format!( + // Roles are cluster objects, not per-test-database ones. A previous run + // leaving the role behind raises duplicate_object; the tests in this + // binary run in parallel, so two sessions can also clear that check + // together and collide on pg_authid's unique index instead. + "DO $$ BEGIN \ + CREATE ROLE {ROLE} LOGIN PASSWORD '{ROLE_PASSWORD}'; \ + EXCEPTION WHEN duplicate_object OR unique_violation THEN NULL; \ + END $$" + )) + .execute(db) + .await?; + sqlx::raw_sql(&format!( + "REVOKE CREATE ON SCHEMA public FROM PUBLIC; \ + GRANT CONNECT ON DATABASE \"{dbname}\" TO {ROLE}; \ + GRANT USAGE ON SCHEMA public TO {ROLE};" + )) + .execute(db) + .await?; + + sqlx::query( + "INSERT INTO resource (workspace_id, path, value, resource_type, created_by) \ + VALUES ('dtmig-ws', 'u/dtmig-admin/pg', $1, 'postgresql', 'dtmig-admin')", + ) + .bind(json!({ + "host": opts.get_host(), + "port": opts.get_port(), + "dbname": dbname, + "user": ROLE, + "password": ROLE_PASSWORD, + "sslmode": "disable", + })) + .execute(db) + .await?; + + Ok(()) +} + +#[sqlx::test(fixtures("datatable_migrations_grants"))] +async fn test_run_migrations_without_create_privilege(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + setup_unprivileged_datatable_role(&db).await?; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let url = + format!("http://localhost:{port}/api/w/dtmig-ws/workspaces/run_datatable_migrations/main"); + + // No `_wm_migrations` yet and no way to create one: the caller must be told + // what Postgres actually refused, not "db error". + let resp = authed(reqwest::Client::new().post(&url)).send().await?; + assert_eq!(resp.status(), 500); + let body = resp.text().await?; + assert!( + body.contains("permission denied for schema"), + "the Postgres message should reach the caller, got: {body}" + ); + // The suggested statement must be complete and quoted, not a placeholder. + assert!( + body.contains(&format!("GRANT CREATE ON SCHEMA \"public\" TO \"{ROLE}\"")), + "the hint should name the actual role and schema, got: {body}" + ); + + // Once an operator has created the bookkeeping table and granted DML on it, + // migrations run even though the role still cannot create tables. + sqlx::raw_sql(&format!( + "CREATE TABLE _wm_migrations ( \ + datatable TEXT NOT NULL, \ + version BIGINT NOT NULL, \ + installed_at TIMESTAMPTZ NOT NULL DEFAULT now(), \ + PRIMARY KEY (datatable, version)); \ + GRANT SELECT, INSERT, UPDATE, DELETE ON _wm_migrations TO {ROLE};" + )) + .execute(&db) + .await?; + + let resp = authed(reqwest::Client::new().post(&url)).send().await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!( + status, 200, + "run should succeed on a pre-created table: {body}" + ); + + Ok(()) +} + +#[sqlx::test(fixtures("datatable_migrations_grants"))] +async fn test_datatable_connection_report(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + setup_unprivileged_datatable_role(&db).await?; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let url = + format!("http://localhost:{port}/api/w/dtmig-ws/workspaces/test_datatable_connection/main"); + + // The report is a privilege disclosure about the data table's database, so + // it stays behind the same bar as editing the data table config. + let resp = reqwest::Client::new() + .get(&url) + .header("Authorization", "Bearer DTMIG_USER_TOKEN") + .send() + .await?; + assert_eq!(resp.status(), 403, "non-admins must not get the report"); + + let report: Value = authed(reqwest::Client::new().get(&url)) + .send() + .await? + .json() + .await?; + assert_eq!(report["user"], ROLE); + assert_eq!(report["schema"], "public"); + assert_eq!(report["can_create_table"], false); + assert_eq!(report["can_create_schema"], false); + let grants = report["suggested_grants"].as_array().unwrap(); + assert!( + grants + .iter() + .any(|g| g.as_str().unwrap() + == format!("GRANT CREATE ON SCHEMA \"public\" TO \"{ROLE}\"")), + "missing schema grant: {report}" + ); + // Pin the name, not just the shape: the endpoint reads it from + // `current_database()` rather than the resource, and a prefix assertion + // would pass either way. + let dbname = (*db.connect_options()) + .clone() + .get_database() + .expect("test database name") + .to_string(); + assert!( + grants.iter().any(|g| g.as_str().unwrap() + == format!("GRANT CREATE ON DATABASE \"{dbname}\" TO \"{ROLE}\"")), + "missing database grant for {dbname}: {report}" + ); + + // A pre-created bookkeeping table lets migration *tracking* work, but the + // role still cannot create anything: the report must keep saying so rather + // than falling silent because nothing needs creating right now. + sqlx::raw_sql(&format!( + "CREATE TABLE _wm_migrations ( \ + datatable TEXT NOT NULL, \ + version BIGINT NOT NULL, \ + installed_at TIMESTAMPTZ NOT NULL DEFAULT now(), \ + PRIMARY KEY (datatable, version)); \ + GRANT SELECT, INSERT, UPDATE, DELETE ON _wm_migrations TO {ROLE};" + )) + .execute(&db) + .await?; + + let report: Value = authed(reqwest::Client::new().get(&url)) + .send() + .await? + .json() + .await?; + assert_eq!(report["migrations_table_exists"], true); + assert_eq!(report["can_create_table"], false); + assert!( + report["suggested_grants"] + .as_array() + .unwrap() + .iter() + .any(|g| g.as_str().unwrap().contains("ON SCHEMA")), + "an existing bookkeeping table must not suppress the schema grant: {report}" + ); + + // Granting the privileges clears the suggestions. + sqlx::raw_sql(&format!( + "GRANT CREATE ON SCHEMA public TO {ROLE}; \ + GRANT CREATE ON DATABASE \"{dbname}\" TO {ROLE};" + )) + .execute(&db) + .await?; + + let report: Value = authed(reqwest::Client::new().get(&url)) + .send() + .await? + .json() + .await?; + assert_eq!(report["can_create_table"], true); + assert_eq!(report["can_create_schema"], true); + assert_eq!(report["suggested_grants"].as_array().unwrap().len(), 0); + + Ok(()) +} + +/// Point the fixture's second data table at a role whose `search_path` resolves +/// to nothing, the one state where no grant helps. +async fn setup_schemaless_datatable_role(db: &Pool) -> anyhow::Result<()> { + let opts = (*db.connect_options()).clone(); + let dbname = opts.get_database().expect("test database name").to_string(); + + sqlx::query(&format!( + "DO $$ BEGIN \ + CREATE ROLE \"{NOSCHEMA_ROLE}\" LOGIN PASSWORD '{ROLE_PASSWORD}'; \ + EXCEPTION WHEN duplicate_object OR unique_violation THEN NULL; \ + END $$" + )) + .execute(db) + .await?; + // Cluster-wide for this role, which is why it gets one of its own rather + // than sharing the role the other assertions connect with. + sqlx::raw_sql(&format!( + "ALTER ROLE \"{NOSCHEMA_ROLE}\" SET search_path = wm_dtmig_absent_schema; \ + GRANT CONNECT ON DATABASE \"{dbname}\" TO \"{NOSCHEMA_ROLE}\";" + )) + .execute(db) + .await?; + + sqlx::query( + "INSERT INTO resource (workspace_id, path, value, resource_type, created_by) \ + VALUES ('dtmig-ws', 'u/dtmig-admin/pg_noschema', $1, 'postgresql', 'dtmig-admin')", + ) + .bind(json!({ + "host": opts.get_host(), + "port": opts.get_port(), + "dbname": dbname, + "user": NOSCHEMA_ROLE, + "password": ROLE_PASSWORD, + "sslmode": "disable", + })) + .execute(db) + .await?; + + Ok(()) +} + +#[sqlx::test(fixtures("datatable_migrations_grants"))] +async fn test_datatable_connection_without_a_resolvable_schema( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + setup_schemaless_datatable_role(&db).await?; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let report: Value = authed(reqwest::Client::new().get(format!( + "http://localhost:{port}/api/w/dtmig-ws/workspaces/test_datatable_connection/noschema" + ))) + .send() + .await? + .json() + .await?; + + assert!(report["schema"].is_null(), "expected no schema: {report}"); + // No grant fixes an empty search_path, so suggesting one would send the + // reader after a statement that changes nothing. + assert!( + !report["suggested_grants"] + .as_array() + .unwrap() + .iter() + .any(|g| g.as_str().unwrap().contains("ON SCHEMA")), + "an empty search_path must not yield a schema grant: {report}" + ); + assert_eq!( + report["suggested_search_path"], + format!("ALTER ROLE \"{NOSCHEMA_ROLE}\" SET search_path = public") + ); + + Ok(()) +} diff --git a/backend/tests/fixtures/datatable_migrations_grants.sql b/backend/tests/fixtures/datatable_migrations_grants.sql new file mode 100644 index 0000000000..81feb9ce91 --- /dev/null +++ b/backend/tests/fixtures/datatable_migrations_grants.sql @@ -0,0 +1,35 @@ +-- Fixture for the data table migration bookkeeping-grants regression test. +-- Workspace + admin token + a data table pointing at a postgres resource; the +-- test fills that resource in with credentials for a deliberately unprivileged +-- role, since the database name is allocated per test run. + +INSERT INTO workspace (id, name, owner) VALUES + ('dtmig-ws', 'DTMIG WS', 'dtmig-admin'); + +INSERT INTO workspace_key (workspace_id, kind, key) VALUES + ('dtmig-ws', 'cloud', 'dtmig-key'); + +INSERT INTO workspace_settings (workspace_id, datatable) VALUES + ('dtmig-ws', '{"datatables": {"main": {"database": {"resource_type": "postgresql", "resource_path": "u/dtmig-admin/pg"}, "migrations_enabled": true}, "noschema": {"database": {"resource_type": "postgresql", "resource_path": "u/dtmig-admin/pg_noschema"}, "migrations_enabled": true}}}'); + +INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES + ('dtmig-ws', 'all', 'All users', '{}'); + +INSERT INTO password(email, password_hash, login_type, super_admin, verified, name, username) + VALUES ('dtmig-admin@windmill.dev', 'x', 'password', true, true, 'DTMIG Admin', 'dtmig-admin'); + +INSERT INTO usr(workspace_id, email, username, is_admin, role) VALUES + ('dtmig-ws', 'dtmig-admin@windmill.dev', 'dtmig-admin', true, 'Admin'); + +INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin) + VALUES (encode(sha256('DTMIG_ADMIN_TOKEN'::bytea), 'hex'), 'DTMIG_ADM', 'DTMIG_ADMIN_TOKEN', 'dtmig-admin@windmill.dev', 't', true); + +-- Non-admin member, to pin that the privilege report stays admin-only. +INSERT INTO password(email, password_hash, login_type, super_admin, verified, name, username) + VALUES ('dtmig-user@windmill.dev', 'x', 'password', false, true, 'DTMIG User', 'dtmig-user'); + +INSERT INTO usr(workspace_id, email, username, is_admin, role) VALUES + ('dtmig-ws', 'dtmig-user@windmill.dev', 'dtmig-user', false, 'User'); + +INSERT INTO token(token_hash, token_prefix, token, email, label) + VALUES (encode(sha256('DTMIG_USER_TOKEN'::bytea), 'hex'), 'DTMIG_USR', 'DTMIG_USER_TOKEN', 'dtmig-user@windmill.dev', 't'); diff --git a/backend/windmill-api-debug/src/lib.rs b/backend/windmill-api-debug/src/lib.rs index 61ec59be75..3b43a04ad1 100644 --- a/backend/windmill-api-debug/src/lib.rs +++ b/backend/windmill-api-debug/src/lib.rs @@ -422,11 +422,7 @@ async fn sign_expression( let mut tx = user_db.begin(&authed).await?; // Truncate expression for resource field if too long (max 255 chars) - let resource = if request.expression.len() > 200 { - format!("{}...", &request.expression[..200]) - } else { - request.expression.clone() - }; + let resource = windmill_common::utils::truncate_with_ellipsis(&request.expression, 200); audit_log( &mut *tx, diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index bd34ca35ef..eca0001eab 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -53,7 +53,7 @@ use windmill_common::{ auth::is_super_admin_email, ee_oss::{get_license_plan, LicensePlan}, email_oss::send_email_plain_text, - error::{self, JsonResult, Result}, + error::{self, pg_error_message, JsonResult, Result}, get_database_url, global_settings::{ AI_CONFIG_SETTING, APP_WORKSPACED_ROUTE_SETTING, AUTOMATE_USERNAME_CREATION_SETTING, @@ -1705,7 +1705,7 @@ async fn setup_custom_instance_pg_database_inner( .map_err(|e| { error::Error::ExecutionErr(format!( "Failed to grant permissions to custom_instance_user: {}", - e.to_string(), + pg_error_message(&e), )) })?; diff --git a/backend/windmill-api-workspaces/src/datatable_migrations.rs b/backend/windmill-api-workspaces/src/datatable_migrations.rs index fef9963933..73e2fb85cc 100644 --- a/backend/windmill-api-workspaces/src/datatable_migrations.rs +++ b/backend/windmill-api-workspaces/src/datatable_migrations.rs @@ -21,14 +21,16 @@ use chrono::Utc; use serde::{Deserialize, Serialize}; use sqlx::{Postgres, Transaction}; use std::collections::{HashMap, HashSet}; +use tokio_postgres::error::SqlState; use windmill_api_auth::{require_super_admin, ApiAuthed}; use windmill_api_jobs::run_wait_result_internal; use windmill_audit::audit_oss::audit_log; use windmill_audit::ActionKind; use windmill_common::db::UserDB; -use windmill_common::error::{Error, JsonResult, Result}; +use windmill_common::error::{pg_error_message, Error, JsonResult, Result}; use windmill_common::jobs::{JobPayload, RawCode}; +use windmill_common::query_builders::{render_db_quoted_identifier, DbType}; use windmill_common::runnable_settings::{ConcurrencySettingsWithCustom, DebouncingSettings}; use windmill_common::scripts::ScriptLang; use windmill_common::users::username_to_permissioned_as; @@ -222,7 +224,31 @@ async fn run_datatable_migration_job( /// key would let one data table's migration mark another's same-version /// migration as already applied (and rollback could touch the wrong row). async fn ensure_wm_migrations_schema(client: &tokio_postgres::Client) -> Result<()> { - client + // `CREATE TABLE IF NOT EXISTS` checks CREATE on the schema before it checks + // existence, so probing first is what lets a data table whose role only holds + // DML grants keep migrating against an already-created bookkeeping table. + // `to_regclass` resolves through search_path, like the unqualified statements + // the rest of this module runs against it. Takes no parameters, so it goes + // through the simple protocol: a named prepared statement is what stalls + // behind a transaction-pooling proxy (see `pg_get_full_schema`). + let rows = client + .simple_query("SELECT to_regclass('_wm_migrations') IS NOT NULL AS present") + .await + .map_err(|e| { + Error::internal_err(format!( + "Failed to look up _wm_migrations table: {}", + pg_error_message(&e) + )) + })?; + let exists = rows.iter().any(|msg| match msg { + tokio_postgres::SimpleQueryMessage::Row(row) => row.get("present") == Some("t"), + _ => false, + }); + if exists { + return Ok(()); + } + + let Err(e) = client .batch_execute( "CREATE TABLE IF NOT EXISTS _wm_migrations (\ datatable TEXT NOT NULL, \ @@ -231,10 +257,53 @@ async fn ensure_wm_migrations_schema(client: &tokio_postgres::Client) -> Result< PRIMARY KEY (datatable, version))", ) .await - .map_err(|e| { - Error::internal_err(format!("Failed to ensure _wm_migrations table: {}", e)) - })?; - Ok(()) + else { + return Ok(()); + }; + + let mut msg = format!( + "Failed to ensure _wm_migrations table: {}", + pg_error_message(&e) + ); + // A role with only table-level grants cannot create it: since Postgres 15 the + // `public` schema no longer grants CREATE to PUBLIC, so this is the usual + // failure on a bring-your-own database. + if e.code() == Some(&SqlState::INSUFFICIENT_PRIVILEGE) { + // Windmill connects as the role that lacks the privilege, so it cannot + // grant it: hand over the statement a schema owner has to run instead. + // Keep it ahead of the explanation below — the UI collapses everything + // past the first couple of lines behind a "Show more". + if let Some((user, schema)) = connection_identity(client).await { + // Both come back unquoted, so a mixed-case or hyphenated name would + // otherwise render a statement that targets a different schema. + msg.push_str(&format!( + ". Run: GRANT CREATE ON SCHEMA {} TO {}", + render_db_quoted_identifier(&schema, DbType::Postgresql), + render_db_quoted_identifier(&user, DbType::Postgresql), + )); + } + msg.push_str( + ". Applied migrations are recorded in a `_wm_migrations` table in the data \ + table's own database, so its user needs to be able to create it", + ); + } + Err(Error::internal_err(msg)) +} + +/// The role and default schema of a data table connection, for grant hints. +/// Both come from the server so the statement we suggest names what the +/// connection actually resolves to, not what the resource happens to say. +async fn connection_identity(client: &tokio_postgres::Client) -> Option<(String, String)> { + let rows = client + .simple_query("SELECT current_user AS usr, current_schema() AS sch") + .await + .ok()?; + rows.iter().find_map(|msg| match msg { + tokio_postgres::SimpleQueryMessage::Row(row) => { + Some((row.get("usr")?.to_string(), row.get("sch")?.to_string())) + } + _ => None, + }) } /// Open a connection to a data table's own database and hold the session-level @@ -265,7 +334,12 @@ async fn lock_datatable_migration_runs( client .batch_execute("SELECT pg_advisory_lock(hashtext('windmill_datatable_migrations')::int8)") .await - .map_err(|e| Error::internal_err(format!("Failed to acquire migration lock: {}", e)))?; + .map_err(|e| { + Error::internal_err(format!( + "Failed to acquire migration lock: {}", + pg_error_message(&e) + )) + })?; Ok(client) } @@ -287,7 +361,7 @@ async fn read_applied_versions_on_client( Err(e) if e.as_db_error().map(|d| d.code().code()) == Some("42P01") => Ok(HashSet::new()), Err(e) => Err(Error::internal_err(format!( "Failed to read _wm_migrations: {}", - e + pg_error_message(&e) ))), } } @@ -366,7 +440,12 @@ async fn run_datatable_migrations( &[&datatable_name, &m.timestamp], ) .await - .map_err(|e| Error::internal_err(format!("Failed to record migration: {}", e)))?; + .map_err(|e| { + Error::internal_err(format!( + "Failed to record migration: {}", + pg_error_message(&e) + )) + })?; applied.push(AppliedMigration { version: m.timestamp, name: m.name }); } @@ -433,7 +512,12 @@ async fn rollback_datatable_migrations( &[&datatable_name, &only], ) .await - .map_err(|e| Error::internal_err(format!("Failed to read _wm_migrations: {}", e)))?, + .map_err(|e| { + Error::internal_err(format!( + "Failed to read _wm_migrations: {}", + pg_error_message(&e) + )) + })?, None => client .query_opt( "SELECT version FROM _wm_migrations WHERE datatable = $1 \ @@ -441,7 +525,12 @@ async fn rollback_datatable_migrations( &[&datatable_name], ) .await - .map_err(|e| Error::internal_err(format!("Failed to read _wm_migrations: {}", e)))?, + .map_err(|e| { + Error::internal_err(format!( + "Failed to read _wm_migrations: {}", + pg_error_message(&e) + )) + })?, }; let version: i64 = match target { @@ -492,7 +581,12 @@ async fn rollback_datatable_migrations( &[&datatable_name, &version], ) .await - .map_err(|e| Error::internal_err(format!("Failed to drop migration record: {}", e)))?; + .map_err(|e| { + Error::internal_err(format!( + "Failed to drop migration record: {}", + pg_error_message(&e) + )) + })?; Ok(Json(RollbackDatatableMigrationsResult { rolled_back: vec![RolledBackMigration { version, name: definition.name }], @@ -940,7 +1034,10 @@ async fn mark_datatable_version_installed( ) .await .map_err(|e| { - Error::internal_err(format!("Failed to mark initial migration installed: {}", e)) + Error::internal_err(format!( + "Failed to mark initial migration installed: {}", + pg_error_message(&e) + )) })?; Ok(()) } @@ -1378,7 +1475,7 @@ fn ignore_missing_wm_migrations(e: tokio_postgres::Error) -> Result<()> { Some("42P01") => Ok(()), _ => Err(Error::internal_err(format!( "Failed to update _wm_migrations: {}", - e + pg_error_message(&e) ))), } } diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index c8f67c9d2e..9501ed96f9 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -34,6 +34,7 @@ use windmill_audit::audit_oss::{audit_log, AuditAuthorable}; use windmill_audit::ActionKind; use windmill_common::db::UserDB; use windmill_common::global_settings::HTTP_ROUTE_WORKSPACED_ROUTE; +use windmill_common::query_builders::{render_db_quoted_identifier, DbType}; use windmill_common::users::username_to_permissioned_as; use windmill_common::variables::{ build_crypt, decrypt, encrypt, SECRET_SALT, WORKSPACE_CRYPT_CACHE, @@ -51,7 +52,7 @@ use windmill_common::workspaces::{ use windmill_common::workspaces::{Ducklake, DucklakeCatalogResourceType}; use windmill_common::PgDatabase; use windmill_common::{ - error::{Error, JsonResult, Result}, + error::{pg_error_message, Error, JsonResult, Result}, global_settings::{ AUTOMATE_USERNAME_CREATION_SETTING, DISABLE_WORKSPACE_INVITE_EMAILS_SETTING, }, @@ -133,6 +134,10 @@ pub fn workspaced_service() -> Router { get(get_datatable_table_schema), ) .route("/edit_datatable_config", post(edit_datatable_config)) + .route( + "/test_datatable_connection/{datatable_name}", + get(test_datatable_connection), + ) .merge(crate::datatable_migrations::routes()) .route("/git_sync_enabled", get(get_git_sync_enabled)) .route("/git_sync_deploy_mode", get(get_git_sync_deploy_mode)) @@ -2023,6 +2028,127 @@ struct DataTableTableSchema { columns: ColumnMap, } +#[derive(Serialize, Debug)] +struct DataTableConnectionCheck { + /// The role the data table actually connects as, and the schema its + /// unqualified statements resolve to. Both are read from the server rather + /// than the resource, which need not spell either of them out. + user: String, + schema: Option, + /// Whether that role can create tables in `schema` / schemas in the database. + can_create_table: bool, + can_create_schema: bool, + /// Whether the migration bookkeeping table is already present. Informative + /// only: it explains why migration *tracking* can work without CREATE, and + /// grants nothing beyond that. + migrations_table_exists: bool, + /// Statements to run for the privileges that are missing, empty when there + /// are none. Windmill connects as the role that lacks them, so it can only + /// name them for a schema owner to run. + suggested_grants: Vec, + /// Statement that gives the session a schema to work in, when `search_path` + /// resolves to none. Rendered here rather than by the caller so identifier + /// quoting stays in one place. + #[serde(skip_serializing_if = "Option::is_none")] + suggested_search_path: Option, +} + +/// Report what the data table's own database lets its role do. Surfacing this +/// from the settings page is the difference between finding out here and finding +/// out on a first schema change, when the failure reads as a Postgres refusal +/// deep inside a migration. +async fn test_datatable_connection( + authed: ApiAuthed, + Extension(db): Extension, + Path((w_id, datatable_name)): Path<(String, String)>, +) -> JsonResult { + require_admin(authed.is_admin, &authed.username)?; + + let db_resource = get_datatable_resource_from_db_unchecked(&db, &w_id, &datatable_name).await?; + let pg_db: PgDatabase = serde_json::from_value(db_resource) + .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e)))?; + let (client, connection) = pg_db.connect(Some(&db)).await?; + let join_handle = tokio::spawn(async move { connection.await }); + + // One round trip, no side effects: `has_*_privilege` answers for the + // connected role without attempting the operation. + let rows = client + .simple_query( + "SELECT current_user AS usr, \ + current_schema() AS sch, \ + current_database() AS db, \ + has_schema_privilege(current_schema(), 'CREATE') AS can_create_table, \ + has_database_privilege(current_database(), 'CREATE') AS can_create_schema, \ + to_regclass('_wm_migrations') IS NOT NULL AS has_migrations_table", + ) + .await + .map_err(|e| { + Error::internal_err(format!( + "Failed to inspect data table privileges: {}", + pg_error_message(&e) + )) + }); + + drop(client); + let _ = windmill_common::shutdown_pg_connection(join_handle).await; + + let row = rows? + .into_iter() + .find_map(|msg| match msg { + tokio_postgres::SimpleQueryMessage::Row(row) => Some(row), + _ => None, + }) + .ok_or_else(|| Error::internal_err("Privilege query returned no row".to_string()))?; + + let user = row.get("usr").unwrap_or_default().to_string(); + let schema = row.get("sch").map(str::to_string); + let can_create_table = row.get("can_create_table") == Some("t"); + let can_create_schema = row.get("can_create_schema") == Some("t"); + let migrations_table_exists = row.get("has_migrations_table") == Some("t"); + + let quoted_user = render_db_quoted_identifier(&user, DbType::Postgresql); + let mut suggested_grants = Vec::new(); + // Suggest on the capability alone: an existing `_wm_migrations` spares only + // that one table, and says nothing about the tables a migration will create. + // A NULL `current_schema()` means search_path resolves to nothing, and no + // grant fixes that — an unqualified CREATE fails with `no schema has been + // selected to create in` whoever holds the privilege — so suggest nothing + // and let `schema: null` carry the diagnosis. + if let (false, Some(target)) = (can_create_table, schema.as_deref()) { + suggested_grants.push(format!( + "GRANT CREATE ON SCHEMA {} TO {}", + render_db_quoted_identifier(target, DbType::Postgresql), + quoted_user + )); + } + if !can_create_schema { + // Named from the server like every other identifier here: behind a + // pooler the resource's dbname can be an alias for another database. + let dbname = row.get("db").unwrap_or(pg_db.dbname.as_str()); + suggested_grants.push(format!( + "GRANT CREATE ON DATABASE {} TO {}", + render_db_quoted_identifier(dbname, DbType::Postgresql), + quoted_user + )); + } + + // An empty search_path is not a privilege problem, so it gets a statement of + // its own rather than a grant. + let suggested_search_path = schema + .is_none() + .then(|| format!("ALTER ROLE {quoted_user} SET search_path = public")); + + Ok(Json(DataTableConnectionCheck { + user, + schema, + can_create_table, + can_create_schema, + migrations_table_exists, + suggested_grants, + suggested_search_path, + })) +} + async fn list_datatable_schemas( _authed: ApiAuthed, Extension(db): Extension, @@ -2139,7 +2265,9 @@ async fn get_datatable_schema(db: &DB, w_id: &str, datatable_name: &str) -> Resu &[], ) .await - .map_err(|e| Error::internal_err(format!("Failed to query schemas: {}", e)))?; + .map_err(|e| { + Error::internal_err(format!("Failed to query schemas: {}", pg_error_message(&e))) + })?; // Build hierarchical structure: schema -> table -> column -> compact_type let mut schema_map: SchemaMap = HashMap::new(); @@ -2173,7 +2301,9 @@ async fn get_datatable_schema(db: &DB, w_id: &str, datatable_name: &str) -> Resu &[&schema_names], ) .await - .map_err(|e| Error::internal_err(format!("Failed to query columns: {}", e)))?; + .map_err(|e| { + Error::internal_err(format!("Failed to query columns: {}", pg_error_message(&e))) + })?; for row in rows { let table_schema: String = row.get(0); @@ -2221,7 +2351,9 @@ async fn get_datatable_tables(db: &DB, w_id: &str, datatable_name: &str) -> Resu &[], ) .await - .map_err(|e| Error::internal_err(format!("Failed to query schemas: {}", e)))?; + .map_err(|e| { + Error::internal_err(format!("Failed to query schemas: {}", pg_error_message(&e))) + })?; let mut table_map: TableListMap = HashMap::new(); let schema_names: Vec = schema_rows @@ -2247,7 +2379,9 @@ async fn get_datatable_tables(db: &DB, w_id: &str, datatable_name: &str) -> Resu &[&schema_names], ) .await - .map_err(|e| Error::internal_err(format!("Failed to query tables: {}", e)))?; + .map_err(|e| { + Error::internal_err(format!("Failed to query tables: {}", pg_error_message(&e))) + })?; for row in rows { let table_schema: String = row.get(0); @@ -2299,7 +2433,9 @@ async fn get_datatable_table_columns( &[&schema_name, &table_name], ) .await - .map_err(|e| Error::internal_err(format!("Failed to query columns: {}", e)))?; + .map_err(|e| { + Error::internal_err(format!("Failed to query columns: {}", pg_error_message(&e))) + })?; if rows.is_empty() { return Err(Error::NotFound(format!( @@ -2671,7 +2807,10 @@ async fn create_pg_database( ) .await .map_err(|e| { - Error::internal_err(format!("Failed to check database existence: {}", e)) + Error::internal_err(format!( + "Failed to check database existence: {}", + pg_error_message(&e) + )) })?; let db_exists: bool = row.get(0); @@ -2690,7 +2829,8 @@ async fn create_pg_database( .map_err(|e| { Error::internal_err(format!( "Failed to create database '{}': {}", - req.target_dbname, e + req.target_dbname, + pg_error_message(&e) )) })?; @@ -3209,20 +3349,60 @@ async fn check_dev_promotion_targets_parent_repo<'a>( repos: impl Iterator, ) -> Result<()> { for r in repos.filter(|r| r.use_individual_branch.unwrap_or(false)) { - if !windmill_common::git_sync_ee::dev_promotion_target_matches_parent( + let Some(m) = windmill_common::git_sync_ee::dev_promotion_target_mismatch( db, w_id, &r.git_repo_resource_path, ) .await? - { - return Err(Error::BadRequest( - "Promotion mode on a dev workspace must reuse the parent workspace's git repository \ - (same URL and branch), but this repository is not one the parent tracks — promotion \ - would target a repository the parent does not sync with." - .to_string(), - )); - } + else { + continue; + }; + // A resource with no `branch` field normalizes to an empty string. + let named = |b: &str| { + if b.is_empty() { + "".to_string() + } else { + b.to_string() + } + }; + let branch = named(&m.branch); + let hint = if m.parent_branches.is_empty() { + format!( + "the parent workspace '{}' does not track that repository. Point this repository at \ + the one the parent syncs with, or add it to the parent's git sync settings.", + m.parent_workspace + ) + } else { + // With no branch of its own there is nothing for the parent to add, + // so only offer the side of the advice that can be acted on. + let other_way = if m.branch.is_empty() { + String::new() + } else { + format!(", or add branch '{branch}' to the parent's git sync settings") + }; + format!( + "the parent workspace '{}' tracks it on {} '{}'. Set this repository's branch to \ + match{other_way}.", + m.parent_workspace, + if m.parent_branches.len() > 1 { + "branches" + } else { + "branch" + }, + m.parent_branches + .iter() + .map(|b| named(b)) + .collect::>() + .join("', '") + ) + }; + return Err(Error::BadRequest(format!( + "Promotion mode on a dev workspace must reuse the parent workspace's git repository \ + (same URL and branch). Repository '{}' targets '{}' on branch '{branch}', but {hint}", + r.git_repo_resource_path.trim_start_matches("$res:"), + m.repo, + ))); } Ok(()) } diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index ddb70e5ff4..f9143131f5 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -1,7 +1,7 @@ openapi: "3.0.3" info: - version: 1.773.0 + version: 1.775.1 title: Windmill API contact: @@ -4633,6 +4633,52 @@ paths: items: $ref: "#/components/schemas/DataTableSchema" + /w/{workspace}/workspaces/test_datatable_connection/{datatable_name}: + get: + summary: check what the data table's database lets its role do + operationId: testDataTableConnection + tags: + - workspace + parameters: + - $ref: "#/components/parameters/WorkspaceId" + - name: datatable_name + in: path + required: true + schema: + type: string + responses: + "200": + description: connection and privilege report + content: + application/json: + schema: + type: object + properties: + user: + type: string + schema: + type: string + nullable: true + can_create_table: + type: boolean + can_create_schema: + type: boolean + migrations_table_exists: + type: boolean + suggested_grants: + type: array + items: + type: string + suggested_search_path: + type: string + required: + - user + - schema + - can_create_table + - can_create_schema + - migrations_table_exists + - suggested_grants + /w/{workspace}/workspaces/list_datatable_tables: get: summary: list tables of all connected Datatables @@ -14215,6 +14261,116 @@ paths: - sibling_count - logs + /w/{workspace}/jobs_u/get_flow_all_results/{id}: + get: + summary: get statuses and truncated results for all jobs of a flow job's execution tree + operationId: getFlowAllResults + tags: + - job + parameters: + - $ref: "#/components/parameters/WorkspaceId" + - $ref: "#/components/parameters/JobId" + - name: max_result_len + in: query + description: per-entry cap (in characters of JSON text) on result_prefix (default 2000, max 30000) + schema: + type: integer + - name: step + in: query + description: 'step address to resolve to a single job instead of enumerating the tree: "b", "b/c", "b[12]" (1-based iteration/branch), composable as "b[12]/c"' + schema: + type: string + responses: + "200": + description: per-job statuses and truncated results of the flow's execution tree, or the single resolved entry when step is provided + content: + application/json: + schema: + type: object + properties: + enclosing_job: + type: string + format: uuid + description: set when the requested job is itself a step of a larger flow run; id of the flow run directly enclosing it + entries: + type: array + items: + type: object + properties: + job_id: + type: string + label: + type: string + description: human-readable label describing the job's position in the flow tree + kind: + type: string + description: job kind (script, flow, forloopflow, ...) + flow_step_id: + type: string + nullable: true + step_path: + type: string + nullable: true + description: materialized step path (e.g. "a/b") + depth: + type: integer + description: depth in the flow tree (0 for the root flow job) + parent_module_type: + type: string + nullable: true + description: parent module type (forloopflow, branchall, ...) + sibling_index: + type: integer + description: 1-based index of this job among siblings sharing the same step + sibling_count: + type: integer + description: total number of siblings sharing the same step + status: + type: string + enum: + [ + success, + failure, + canceled, + skipped, + suspended, + running, + queued, + ] + success: + type: boolean + duration_ms: + type: integer + format: int64 + started_at: + type: string + format: date-time + result_prefix: + type: string + description: result JSON text truncated to the per-entry budget; absent until the job has completed + result_length: + type: integer + description: full length in characters of the result JSON text (greater than the prefix length when truncated) + required: + - job_id + - label + - kind + - depth + - sibling_index + - sibling_count + - status + truncated: + type: boolean + description: true when the tree has more jobs than the entry cap; entries then hold the depth-first prefix + scope_filtered: + type: boolean + description: true when the caller's token is tag-scoped; steps running on other tags are omitted + step_error: + type: string + description: set when step was provided but could not be resolved; a diagnostic listing available step ids or iteration statuses + required: + - entries + /w/{workspace}/jobs_u/get_completed_logs_tail/{id}: get: summary: get completed job logs tail diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index f030aaa3d1..3621ea0564 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -406,6 +406,7 @@ pub fn workspace_unauthed_service() -> Router { "/get_flow_all_logs_structured/{id}", get(get_flow_all_logs_structured), ) + .route("/get_flow_all_results/{id}", get(get_flow_all_results)) .route( "/get_completed_logs_tail/{id}", get(get_completed_job_logs_tail), @@ -2269,48 +2270,35 @@ async fn resolve_logs_to_string( logs.to_string() } -/// A single job in a flow's execution tree, with its resolved logs and a -/// human-readable label describing its position (iteration, branch, subflow…). -#[derive(Serialize)] -struct FlowLogEntry { - job_id: String, - /// Human-readable label, e.g. "Step a (iteration 2/3)" or "Flow". - label: String, - /// Job kind (script, flow, forloopflow, …). - kind: String, - /// The flow step id this job corresponds to, if any. - flow_step_id: Option, - /// Materialized step path (e.g. "a/b") used to locate the step in the flow. - step_path: Option, - /// Depth in the flow tree (0 for the root flow job). - depth: i32, - /// The parent module type (forloopflow, branchall, …), if any. - parent_module_type: Option, - /// 1-based index of this job among its siblings sharing the same step. - sibling_index: i32, - /// Total number of siblings sharing the same step. - sibling_count: i32, - /// Resolved logs for this job (pulled from disk/object store as needed). - logs: String, +struct FlowTreeReadAuth { + /// Enclosing flow job id when the requested job is itself a step of a + /// larger flow run (flow_innermost_root_job is NULL on subflow jobs — + /// parent_job covers them). + enclosing_job: Option, + /// Scope tags of the caller's token, to re-apply on every descendant query + /// (child jobs of a `preserve_step_tags` flow can run on other tags). + scope_tags: Option>, } -async fn collect_flow_log_entries( +/// Shared preamble of the flow-tree endpoints (`get_flow_all_logs*`, +/// `get_flow_all_results`): verifies the job exists (scope-tag filtered), +/// checks read access, and records the view. +async fn authorize_flow_tree_read( view_token: Option, - opt_authed: Option, - opt_tokened: OptTokened, + opt_authed: &Option, + opt_tokened: &OptTokened, db: &DB, user_db: &UserDB, w_id: &str, id: Uuid, -) -> error::Result> { +) -> error::Result { let tags = opt_authed .as_ref() .map(|authed| get_scope_tags(authed).map(|v| v.iter().map(|s| s.to_string()).collect_vec())) .flatten(); - // Verify the root job exists and check auth let root_job = sqlx::query!( - "SELECT created_by FROM v2_job WHERE id = $1 AND workspace_id = $2 AND ($3::text[] IS NULL OR tag = ANY($3))", + "SELECT created_by, COALESCE(flow_innermost_root_job, parent_job) as enclosing_job FROM v2_job WHERE id = $1 AND workspace_id = $2 AND ($3::text[] IS NULL OR tag = ANY($3))", id, w_id, tags.as_ref().map(|v| v.as_slice()) @@ -2346,6 +2334,128 @@ async fn collect_flow_log_entries( ) .await?; + Ok(FlowTreeReadAuth { + enclosing_job: root_job.enclosing_job.filter(|r| *r != id), + scope_tags: tags, + }) +} + +/// Human-readable label for a job's position in a flow's execution tree, +/// e.g. "Flow", "Step a (iteration 2/3)", "Step b (subflow)". +fn flow_tree_entry_label( + kind: &str, + path: &str, + parent_module_type: &str, + sibling_index: i32, + sibling_count: i32, + depth: i32, +) -> String { + let kind_label = match parent_module_type { + "branchall" => " branchall", + "branchone" => " branchone", + "forloopflow" => " forloop", + "whileloopflow" => " whileloop", + "aiagent" => " ai-agent", + _ => "", + }; + + // Only a known non-fan-out parent module marks siblings as retry attempts; + // an unresolved type ("" — e.g. the flow was edited since the run) keeps + // the generic iteration wording, matching resolve_flow_step_job and the + // frontend's FAN_OUT_MODULE_TYPES check. + let attempt_like = !parent_module_type.is_empty() && !is_fan_out_module(parent_module_type); + + if depth == 0 { + "Flow".to_string() + } else if matches!( + kind, + "flow" | "flowpreview" | "flownode" | "singlestepflow" | "aiagent" + ) { + // Intermediate flow job (loop iteration or branch) + if parent_module_type == "branchone" { + // sibling_index is a row number among sibling jobs, not the chosen + // branch (branchone only enqueues the branch it selected) — don't + // pretend to know which branch ran. + format!("Step {}{} (selected branch)", path, kind_label) + } else if parent_module_type == "branchall" { + format!("Step {}{} (branch {})", path, kind_label, sibling_index) + } else if parent_module_type == "forloopflow" || parent_module_type == "whileloopflow" { + format!( + "Step {}{} (iteration {}/{})", + path, kind_label, sibling_index, sibling_count + ) + } else if sibling_count > 1 { + if attempt_like { + format!( + "Step {} (attempt {}/{})", + path, sibling_index, sibling_count + ) + } else { + format!( + "Step {}{} (iteration {}/{})", + path, kind_label, sibling_index, sibling_count + ) + } + } else { + format!("Step {} (subflow)", path) + } + } else if sibling_count > 1 { + if attempt_like { + format!( + "Step {} (attempt {}/{})", + path, sibling_index, sibling_count + ) + } else { + // Simple module optimization: forloop/whileloop with single step + // runs iterations as direct script jobs instead of subflows + format!( + "Step {}{} (iteration {}/{})", + path, kind_label, sibling_index, sibling_count + ) + } + } else { + format!("Step {}", path) + } +} + +/// A single job in a flow's execution tree, with its resolved logs and a +/// human-readable label describing its position (iteration, branch, subflow…). +#[derive(Serialize)] +struct FlowLogEntry { + job_id: String, + /// Human-readable label, e.g. "Step a (iteration 2/3)" or "Flow". + label: String, + /// Job kind (script, flow, forloopflow, …). + kind: String, + /// The flow step id this job corresponds to, if any. + flow_step_id: Option, + /// Materialized step path (e.g. "a/b") used to locate the step in the flow. + step_path: Option, + /// Depth in the flow tree (0 for the root flow job). + depth: i32, + /// The parent module type (forloopflow, branchall, …), if any. + parent_module_type: Option, + /// 1-based index of this job among its siblings sharing the same step. + sibling_index: i32, + /// Total number of siblings sharing the same step. + sibling_count: i32, + /// Resolved logs for this job (pulled from disk/object store as needed). + logs: String, +} + +async fn collect_flow_log_entries( + view_token: Option, + opt_authed: Option, + opt_tokened: OptTokened, + db: &DB, + user_db: &UserDB, + w_id: &str, + id: Uuid, +) -> error::Result> { + let auth = + authorize_flow_tree_read(view_token, &opt_authed, &opt_tokened, db, user_db, w_id, id) + .await?; + // Fetch all jobs in the flow tree using recursive CTE. // Uses a materialized id_path for depth-first ordering so children // appear right after their parent (e.g. iteration 1 → its steps → iteration 2 → ...). @@ -2369,11 +2479,13 @@ async fn collect_flow_log_entries( COALESCE(( SELECT m->'value'->>'type' FROM v2_job parent_j + LEFT JOIN flow_version fv ON fv.id = parent_j.runnable_id + AND parent_j.kind::text = 'flow' LEFT JOIN flow f ON f.path = parent_j.runnable_path AND f.workspace_id = parent_j.workspace_id LEFT JOIN flow_node fn ON fn.id = parent_j.runnable_id CROSS JOIN LATERAL jsonb_array_elements( - COALESCE(parent_j.raw_flow, f.value, fn.flow)->'modules' + COALESCE(parent_j.raw_flow, fv.value, f.value, fn.flow)->'modules' ) m WHERE parent_j.id = jt.id AND m->>'id' = j.flow_step_id @@ -2382,17 +2494,39 @@ async fn collect_flow_log_entries( FROM v2_job j JOIN job_tree jt ON j.parent_job = jt.id WHERE j.workspace_id = $1 + AND ($3::text[] IS NULL OR j.tag = ANY($3)) + ), + positions AS ( + SELECT g.parent_job, g.flow_step_id, fj.jid, fj.ord + FROM (SELECT DISTINCT parent_job, flow_step_id FROM job_tree + WHERE parent_job IS NOT NULL AND flow_step_id IS NOT NULL) g + CROSS JOIN LATERAL ( + SELECT COALESCE( + (SELECT flow_status FROM v2_job_completed WHERE id = g.parent_job), + (SELECT flow_status FROM v2_job_status WHERE id = g.parent_job) + ) AS fs + ) pf + CROSS JOIN LATERAL ( + SELECT m FROM jsonb_array_elements(pf.fs->'modules') m + WHERE m->>'id' = g.flow_step_id + LIMIT 1 + ) md + CROSS JOIN LATERAL jsonb_array_elements_text(md.m->'flow_jobs') + WITH ORDINALITY fj(jid, ord) ), with_sibling_index AS ( SELECT jt.*, ROW_NUMBER() OVER ( PARTITION BY jt.parent_job, jt.flow_step_id - ORDER BY jt.id + ORDER BY pos.ord NULLS LAST, jt.id ) as sibling_index, COUNT(*) OVER ( PARTITION BY jt.parent_job, jt.flow_step_id ) as sibling_count FROM job_tree jt + LEFT JOIN positions pos ON pos.parent_job = jt.parent_job + AND pos.flow_step_id = jt.flow_step_id + AND pos.jid = jt.id::text ) SELECT w.id, w.kind, w.flow_step_id, w.path_label, w.sibling_index::int as sibling_index, @@ -2407,6 +2541,7 @@ async fn collect_flow_log_entries( ORDER BY w.id_path ASC", w_id, id, + auth.scope_tags.as_deref(), ) .fetch_all(db) .await?; @@ -2421,57 +2556,15 @@ async fn collect_flow_log_entries( let sibling_index = record.sibling_index.unwrap_or(0); let parent_module_type = record.parent_module_type.as_deref().unwrap_or(""); - // Build a descriptive label let path = record.path_label.as_deref().unwrap_or(step_id); - - let kind_label = match parent_module_type { - "branchall" => " branchall", - "branchone" => " branchone", - "forloopflow" => " forloop", - "whileloopflow" => " whileloop", - "aiagent" => " ai-agent", - _ => "", - }; - - let label = if depth == 0 { - "Flow".to_string() - } else if matches!( + let label = flow_tree_entry_label( kind, - "flow" | "flowpreview" | "flownode" | "singlestepflow" | "aiagent" - ) { - // Intermediate flow job (loop iteration or branch) - if parent_module_type == "branchone" { - let branch_label = if sibling_index == 1 { - "default".to_string() - } else { - format!("{}", sibling_index - 1) - }; - format!("Step {}{} (branch {})", path, kind_label, branch_label) - } else if parent_module_type == "branchall" { - format!("Step {}{} (branch {})", path, kind_label, sibling_index) - } else if parent_module_type == "forloopflow" || parent_module_type == "whileloopflow" { - format!( - "Step {}{} (iteration {}/{})", - path, kind_label, sibling_index, sibling_count - ) - } else if sibling_count > 1 { - format!( - "Step {} (iteration {}/{})", - path, sibling_index, sibling_count - ) - } else { - format!("Step {} (subflow)", path) - } - } else if sibling_count > 1 { - // Simple module optimization: forloop/whileloop with single step - // runs iterations as direct script jobs instead of subflows - format!( - "Step {}{} (iteration {}/{})", - path, kind_label, sibling_index, sibling_count - ) - } else { - format!("Step {}", path) - }; + path, + parent_module_type, + sibling_index, + sibling_count, + depth, + ); let job_id = record.id.map(|u| u.to_string()).unwrap_or_default(); let logs = record.logs.as_deref().unwrap_or(""); @@ -2557,6 +2650,636 @@ async fn get_flow_all_logs_structured( Ok(Json(entries)) } +/// A single job in a flow's execution tree with its status and (truncated) +/// result — the results twin of `FlowLogEntry`. +#[derive(Serialize)] +struct FlowResultEntry { + job_id: String, + /// Human-readable label, e.g. "Step a (iteration 2/3)" or "Flow". + label: String, + /// Job kind (script, flow, forloopflow, …). + kind: String, + /// The flow step id this job corresponds to, if any. + flow_step_id: Option, + /// Materialized step path (e.g. "a/b") used to locate the step in the flow. + step_path: Option, + /// Depth in the flow tree (0 for the root flow job). + depth: i32, + /// The parent module type (forloopflow, branchall, …), if any. + parent_module_type: Option, + /// 1-based index of this job among its siblings sharing the same step. + sibling_index: i32, + /// Total number of siblings sharing the same step. + sibling_count: i32, + /// success | failure | canceled | skipped | suspended | running | queued + status: String, + #[serde(skip_serializing_if = "Option::is_none")] + success: Option, + #[serde(skip_serializing_if = "Option::is_none")] + duration_ms: Option, + #[serde(skip_serializing_if = "Option::is_none")] + started_at: Option>, + /// Result JSON text truncated to the per-entry budget; absent until the job + /// has completed. + #[serde(skip_serializing_if = "Option::is_none")] + result_prefix: Option, + /// Full length in characters of the result JSON text (greater than the + /// prefix length when truncated). + #[serde(skip_serializing_if = "Option::is_none")] + result_length: Option, +} + +#[derive(Serialize)] +struct FlowAllResultsResponse { + /// Set when the requested job is itself a step of a larger flow run: the id + /// of the flow run directly enclosing it. + #[serde(skip_serializing_if = "Option::is_none")] + enclosing_job: Option, + entries: Vec, + /// True when the tree has more jobs than the entry cap; entries then hold + /// the depth-first prefix. + #[serde(skip_serializing_if = "std::ops::Not::not")] + truncated: bool, + /// True when the caller's token is tag-scoped: steps running on other tags + /// are omitted and indistinguishable from steps that never ran. + #[serde(skip_serializing_if = "std::ops::Not::not")] + scope_filtered: bool, + /// Set when `step` was provided but could not be resolved; a diagnostic the + /// caller can act on (available step ids, iteration statuses). + #[serde(skip_serializing_if = "Option::is_none")] + step_error: Option, +} + +#[derive(Deserialize)] +struct FlowAllResultsQuery { + /// Per-entry cap (in characters of JSON text) on `result_prefix`. + max_result_len: Option, + /// Step address to resolve instead of enumerating the tree: 'b', 'b/c', + /// 'b[12]' (1-based iteration/branch), composable as 'b[12]/c'. + step: Option, +} + +const FLOW_ALL_RESULTS_DEFAULT_MAX_LEN: i32 = 2000; +const FLOW_ALL_RESULTS_MAX_MAX_LEN: i32 = 30_000; +/// Hard cap on enumerated tree entries — bounds both the response size and the +/// per-row result::text materialization the query performs. +const FLOW_ALL_RESULTS_MAX_ENTRIES: i64 = 2000; + +struct ResolvedStepJob { + job_id: Uuid, + flow_step_id: String, + path: String, + sibling_index: i32, + sibling_count: i32, + depth: i32, + parent_module_type: String, +} + +/// Fan-out module types whose sibling jobs are iterations/branches; sibling +/// jobs of any other step are retry attempts. +fn is_fan_out_module(parent_module_type: &str) -> bool { + matches!( + parent_module_type, + "forloopflow" | "whileloopflow" | "branchall" | "branchone" | "aiagent" + ) +} + +/// 'b[12]' -> ("b", Some(12)); 'b' -> ("b", None). +fn parse_step_segment(segment: &str) -> (&str, Option) { + if let Some(rest) = segment.strip_suffix(']') { + if let Some((step_id, idx)) = rest.rsplit_once('[') { + if let Ok(idx) = idx.parse::() { + return (step_id, Some(idx)); + } + } + } + (segment, None) +} + +#[cfg(test)] +mod flow_tree_tests { + use super::{flow_tree_entry_label, parse_step_segment}; + + #[test] + fn label_classifies_iterations_branches_and_retry_attempts() { + assert_eq!(flow_tree_entry_label("flow", "", "", 1, 1, 0), "Flow"); + assert_eq!( + flow_tree_entry_label("script", "l", "forloopflow", 2, 3, 1), + "Step l forloop (iteration 2/3)" + ); + assert_eq!( + flow_tree_entry_label("flow", "b", "branchall", 2, 2, 1), + "Step b branchall (branch 2)" + ); + // branchone only enqueues its chosen branch — the label must not claim + // which branch that was + assert_eq!( + flow_tree_entry_label("flow", "b", "branchone", 1, 1, 1), + "Step b branchone (selected branch)" + ); + // siblings of a non-fan-out step are retry attempts, for both direct + // and subflow steps + assert_eq!( + flow_tree_entry_label("script", "a", "rawscript", 2, 3, 1), + "Step a (attempt 2/3)" + ); + assert_eq!( + flow_tree_entry_label("flow", "b", "flow", 2, 2, 1), + "Step b (attempt 2/2)" + ); + // unresolved parent module type ("", e.g. the flow was edited since the + // run) must NOT claim retries — keep the generic iteration wording + assert_eq!( + flow_tree_entry_label("script", "a", "", 2, 3, 1), + "Step a (iteration 2/3)" + ); + assert_eq!( + flow_tree_entry_label("flow", "b", "flow", 1, 1, 1), + "Step b (subflow)" + ); + assert_eq!( + flow_tree_entry_label("script", "a", "rawscript", 1, 1, 1), + "Step a" + ); + } + + #[test] + fn parses_step_segments() { + assert_eq!(parse_step_segment("b"), ("b", None)); + assert_eq!(parse_step_segment("b[12]"), ("b", Some(12))); + assert_eq!(parse_step_segment("b[0]"), ("b", Some(0))); + assert_eq!(parse_step_segment("b[x]"), ("b[x]", None)); + assert_eq!(parse_step_segment("b]"), ("b]", None)); + } +} + +/// Resolve a step address ('b/c', 'b[12]/c', '.' separators accepted) to a +/// single job of the flow tree rooted at `root`, walking one indexed +/// parent_job + flow_step_id lookup per segment — no tree enumeration. +/// Siblings are numbered by their position in the parent flow_status's +/// flow_jobs array — the authoritative iteration order — matching the tree +/// view's numbering; job-id order alone would shuffle parallel iterations +/// created in the same millisecond (ULID randomness). Retries have no +/// flow_jobs and fall back to id order. Err carries a diagnostic listing what +/// exists instead. +async fn resolve_flow_step_job( + db: &DB, + w_id: &str, + root: Uuid, + address: &str, + scope_tags: Option<&[String]>, +) -> error::Result> { + let segments = address + .replace('.', "/") + .split('/') + .filter(|s| !s.is_empty()) + .map(|s| s.to_string()) + .collect_vec(); + if segments.is_empty() { + return Ok(Err("Empty step address.".to_string())); + } + + let mut current = root; + let mut resolved: Option = None; + let mut path_segments: Vec<&str> = vec![]; + for (depth, segment) in segments.iter().enumerate() { + let (step_id, index) = parse_step_segment(segment); + path_segments.push(step_id); + + // parent_module_type is resolved the same way as in the tree CTE (the + // module named `step_id` in the parent's flow definition); it is + // constant across the sibling rows. + let siblings = sqlx::query!( + "SELECT j.id, + COALESCE(c.status::text, + CASE WHEN q.running AND q.suspend > 0 THEN 'suspended' + WHEN q.running THEN 'running' + ELSE 'queued' END) as \"status!\", + COALESCE(( + SELECT m->'value'->>'type' + FROM v2_job parent_j + LEFT JOIN flow_version fv ON fv.id = parent_j.runnable_id + AND parent_j.kind::text = 'flow' + LEFT JOIN flow f ON f.path = parent_j.runnable_path + AND f.workspace_id = parent_j.workspace_id + LEFT JOIN flow_node fn ON fn.id = parent_j.runnable_id + CROSS JOIN LATERAL jsonb_array_elements( + COALESCE(parent_j.raw_flow, fv.value, f.value, fn.flow)->'modules' + ) m + WHERE parent_j.id = $1 + AND m->>'id' = $3 + LIMIT 1 + ), '')::text as \"parent_module_type!\" + FROM v2_job j + LEFT JOIN v2_job_completed c ON c.id = j.id + LEFT JOIN v2_job_queue q ON q.id = j.id + LEFT JOIN ( + SELECT fj.jid, fj.ord + FROM (SELECT COALESCE( + (SELECT flow_status FROM v2_job_completed WHERE id = $1), + (SELECT flow_status FROM v2_job_status WHERE id = $1) + ) AS fs) pf + CROSS JOIN LATERAL ( + SELECT m FROM jsonb_array_elements(pf.fs->'modules') m + WHERE m->>'id' = $3 + LIMIT 1 + ) md + CROSS JOIN LATERAL jsonb_array_elements_text(md.m->'flow_jobs') + WITH ORDINALITY fj(jid, ord) + ) pos ON pos.jid = j.id::text + WHERE j.parent_job = $1 AND j.workspace_id = $2 AND j.flow_step_id = $3 + AND ($4::text[] IS NULL OR j.tag = ANY($4)) + ORDER BY pos.ord NULLS LAST, j.id", + current, + w_id, + step_id, + scope_tags, + ) + .fetch_all(db) + .await?; + + if siblings.is_empty() { + let available = sqlx::query_scalar!( + "SELECT DISTINCT flow_step_id as \"flow_step_id!\" FROM v2_job + WHERE parent_job = $1 AND workspace_id = $2 AND flow_step_id IS NOT NULL + AND ($3::text[] IS NULL OR tag = ANY($3)) + ORDER BY flow_step_id", + current, + w_id, + scope_tags, + ) + .fetch_all(db) + .await?; + return Ok(Err(format!( + "No step \"{}\" at this level. Available steps: {}.", + step_id, + if available.is_empty() { + "(none)".to_string() + } else { + available.join(", ") + } + ))); + } + + let parent_module_type = siblings[0].parent_module_type.clone(); + let node = if let Some(index) = index { + match index.checked_sub(1).and_then(|i| siblings.get(i)) { + Some(node) => node, + None => { + return Ok(Err(format!( + "Step \"{}\" has no iteration [{}]. Existing: 1..{}.", + step_id, + index, + siblings.len() + ))); + } + } + } else if siblings.len() > 1 { + // These diagnostics go verbatim into the model's context — cap the + // per-sibling listing so huge loops can't flood it. + const MAX_STATUSES_LISTED: usize = 20; + let mut statuses = siblings + .iter() + .take(MAX_STATUSES_LISTED) + .enumerate() + .map(|(i, s)| format!("[{}] {}", i + 1, s.status)) + .join(", "); + if siblings.len() > MAX_STATUSES_LISTED { + statuses.push_str(&format!( + ", … (+{} more)", + siblings.len() - MAX_STATUSES_LISTED + )); + } + return Ok(Err( + if is_fan_out_module(&parent_module_type) || parent_module_type.is_empty() { + format!( + "Step \"{}\" ran {} times (loop/branches) — pick one with \"{}[i]\". Iterations: {}.", + step_id, + siblings.len(), + step_id, + statuses + ) + } else { + format!( + "Step \"{}\" was retried — {} attempts, the last one is the final outcome. Pick one with \"{}[i]\". Attempts: {}.", + step_id, + siblings.len(), + step_id, + statuses + ) + }, + )); + } else { + &siblings[0] + }; + + let index_in_siblings = siblings.iter().position(|s| s.id == node.id).unwrap_or(0); + current = node.id; + resolved = Some(ResolvedStepJob { + job_id: node.id, + flow_step_id: step_id.to_string(), + path: path_segments.join("/"), + sibling_index: (index_in_siblings + 1) as i32, + sibling_count: siblings.len() as i32, + depth: (depth + 1) as i32, + parent_module_type, + }); + } + + Ok(Ok(resolved.expect("segments is non-empty"))) +} + +/// Results twin of `get_flow_all_logs_structured`: one entry per job of the +/// flow's execution tree (same recursive enumeration), carrying per-job status +/// and truncated result instead of logs. +async fn get_flow_all_results( + OptViewToken(view_token): OptViewToken, + OptAuthed(opt_authed): OptAuthed, + opt_tokened: OptTokened, + Extension(db): Extension, + Extension(user_db): Extension, + Path((w_id, id)): Path<(String, Uuid)>, + Query(query): Query, +) -> JsonResult { + let auth = authorize_flow_tree_read( + view_token, + &opt_authed, + &opt_tokened, + &db, + &user_db, + &w_id, + id, + ) + .await?; + let enclosing_job = auth.enclosing_job; + let scope_tags = auth.scope_tags; + + let max_result_len = query + .max_result_len + .unwrap_or(FLOW_ALL_RESULTS_DEFAULT_MAX_LEN) + .clamp(1, FLOW_ALL_RESULTS_MAX_MAX_LEN); + + // Step mode: resolve the address to one job directly (a few indexed + // lookups) instead of enumerating the whole tree. + if let Some(step) = query.step.as_deref().filter(|s| !s.trim().is_empty()) { + let resolved = + match resolve_flow_step_job(&db, &w_id, id, step, scope_tags.as_deref()).await? { + Ok(resolved) => resolved, + Err(step_error) => { + return Ok(Json(FlowAllResultsResponse { + enclosing_job, + entries: vec![], + truncated: false, + scope_filtered: scope_tags.is_some(), + step_error: Some(step_error), + })); + } + }; + + let record = sqlx::query!( + "SELECT j.kind::text as kind, + c.status::text as completed_status, + c.duration_ms as \"duration_ms?\", + COALESCE(c.started_at, q.started_at) as started_at, + LEFT(c.result::text, $3) as result_prefix, + length(c.result::text) as result_length, + q.running as \"q_running?\", + q.suspend as \"q_suspend?\" + FROM v2_job j + LEFT JOIN v2_job_completed c ON c.id = j.id + LEFT JOIN v2_job_queue q ON q.id = j.id + WHERE j.id = $1 AND j.workspace_id = $2", + resolved.job_id, + w_id, + max_result_len, + ) + .fetch_one(&db) + .await?; + + let kind = record.kind.as_deref().unwrap_or(""); + let status = match record.completed_status.as_deref() { + Some(s) => s.to_string(), + None => { + if record.q_running.unwrap_or(false) && record.q_suspend.unwrap_or(0) > 0 { + "suspended".to_string() + } else if record.q_running.unwrap_or(false) { + "running".to_string() + } else { + "queued".to_string() + } + } + }; + let success = record.completed_status.as_deref().map(|s| s == "success"); + let label = flow_tree_entry_label( + kind, + &resolved.path, + &resolved.parent_module_type, + resolved.sibling_index, + resolved.sibling_count, + resolved.depth, + ); + + return Ok(Json(FlowAllResultsResponse { + enclosing_job, + entries: vec![FlowResultEntry { + job_id: resolved.job_id.to_string(), + label, + kind: kind.to_string(), + flow_step_id: Some(resolved.flow_step_id), + step_path: Some(resolved.path), + depth: resolved.depth, + parent_module_type: if resolved.parent_module_type.is_empty() { + None + } else { + Some(resolved.parent_module_type) + }, + sibling_index: resolved.sibling_index, + sibling_count: resolved.sibling_count, + status, + success, + duration_ms: record.duration_ms, + started_at: record.started_at, + result_prefix: record.result_prefix, + result_length: record.result_length, + }], + truncated: false, + scope_filtered: scope_tags.is_some(), + step_error: None, + })); + } + + // Same recursive CTE as `collect_flow_log_entries` (kept textually in sync — + // sqlx macros cannot share the SQL string), joined against the completed and + // queue tables instead of `job_logs`. + let records = sqlx::query!( + "WITH RECURSIVE job_tree AS ( + SELECT j.id, j.kind::text, j.flow_step_id, j.parent_job, + '' as path_label, 0 as depth, + j.id::text as id_path, + ''::text as parent_module_type + FROM v2_job j + WHERE j.id = $2 AND j.workspace_id = $1 + UNION ALL + SELECT j.id, j.kind::text, j.flow_step_id, j.parent_job, + CASE + WHEN jt.path_label = '' THEN COALESCE(j.flow_step_id, '') + ELSE jt.path_label || '/' || COALESCE(j.flow_step_id, '') + END, + jt.depth + 1, + jt.id_path || '/' || j.id::text, + COALESCE(( + SELECT m->'value'->>'type' + FROM v2_job parent_j + LEFT JOIN flow_version fv ON fv.id = parent_j.runnable_id + AND parent_j.kind::text = 'flow' + LEFT JOIN flow f ON f.path = parent_j.runnable_path + AND f.workspace_id = parent_j.workspace_id + LEFT JOIN flow_node fn ON fn.id = parent_j.runnable_id + CROSS JOIN LATERAL jsonb_array_elements( + COALESCE(parent_j.raw_flow, fv.value, f.value, fn.flow)->'modules' + ) m + WHERE parent_j.id = jt.id + AND m->>'id' = j.flow_step_id + LIMIT 1 + ), '')::text + FROM v2_job j + JOIN job_tree jt ON j.parent_job = jt.id + WHERE j.workspace_id = $1 + AND ($4::text[] IS NULL OR j.tag = ANY($4)) + ), + positions AS ( + SELECT g.parent_job, g.flow_step_id, fj.jid, fj.ord + FROM (SELECT DISTINCT parent_job, flow_step_id FROM job_tree + WHERE parent_job IS NOT NULL AND flow_step_id IS NOT NULL) g + CROSS JOIN LATERAL ( + SELECT COALESCE( + (SELECT flow_status FROM v2_job_completed WHERE id = g.parent_job), + (SELECT flow_status FROM v2_job_status WHERE id = g.parent_job) + ) AS fs + ) pf + CROSS JOIN LATERAL ( + SELECT m FROM jsonb_array_elements(pf.fs->'modules') m + WHERE m->>'id' = g.flow_step_id + LIMIT 1 + ) md + CROSS JOIN LATERAL jsonb_array_elements_text(md.m->'flow_jobs') + WITH ORDINALITY fj(jid, ord) + ), + with_sibling_index AS ( + SELECT jt.*, + ROW_NUMBER() OVER ( + PARTITION BY jt.parent_job, jt.flow_step_id + ORDER BY pos.ord NULLS LAST, jt.id + ) as sibling_index, + COUNT(*) OVER ( + PARTITION BY jt.parent_job, jt.flow_step_id + ) as sibling_count + FROM job_tree jt + LEFT JOIN positions pos ON pos.parent_job = jt.parent_job + AND pos.flow_step_id = jt.flow_step_id + AND pos.jid = jt.id::text + ), + limited AS ( + SELECT * FROM with_sibling_index ORDER BY id_path ASC LIMIT $5 + ) + SELECT w.id, w.kind, w.flow_step_id, w.path_label, + w.sibling_index::int as sibling_index, + w.sibling_count::int as sibling_count, + w.depth::int as depth, + w.parent_module_type, + c.status::text as completed_status, + c.duration_ms as \"duration_ms?\", + COALESCE(c.started_at, q.started_at) as started_at, + LEFT(c.result::text, $3) as result_prefix, + length(c.result::text) as result_length, + q.running as \"q_running?\", + q.suspend as \"q_suspend?\" + FROM limited w + LEFT JOIN v2_job_completed c ON c.id = w.id + LEFT JOIN v2_job_queue q ON q.id = w.id + ORDER BY w.id_path ASC", + w_id, + id, + max_result_len, + scope_tags.as_deref(), + FLOW_ALL_RESULTS_MAX_ENTRIES + 1, + ) + .fetch_all(&db) + .await?; + + // One row past the cap fetched only to detect truncation. + let mut records = records; + let truncated = records.len() as i64 > FLOW_ALL_RESULTS_MAX_ENTRIES; + if truncated { + records.truncate(FLOW_ALL_RESULTS_MAX_ENTRIES as usize); + } + + let mut entries = Vec::with_capacity(records.len()); + + for record in records { + let kind = record.kind.as_deref().unwrap_or(""); + let step_id = record.flow_step_id.as_deref().unwrap_or(""); + let depth = record.depth.unwrap_or(0); + let sibling_count = record.sibling_count.unwrap_or(1) as i32; + let sibling_index = record.sibling_index.unwrap_or(0) as i32; + let parent_module_type = record.parent_module_type.as_deref().unwrap_or(""); + + let path = record.path_label.as_deref().unwrap_or(step_id); + let label = flow_tree_entry_label( + kind, + path, + parent_module_type, + sibling_index, + sibling_count, + depth, + ); + + let status = match record.completed_status.as_deref() { + Some(s) => s.to_string(), + None => { + if record.q_running.unwrap_or(false) && record.q_suspend.unwrap_or(0) > 0 { + "suspended".to_string() + } else if record.q_running.unwrap_or(false) { + "running".to_string() + } else { + "queued".to_string() + } + } + }; + let success = record.completed_status.as_deref().map(|s| s == "success"); + + entries.push(FlowResultEntry { + job_id: record.id.map(|u| u.to_string()).unwrap_or_default(), + label, + kind: kind.to_string(), + flow_step_id: record.flow_step_id.clone(), + step_path: record.path_label.clone(), + depth, + parent_module_type: if parent_module_type.is_empty() { + None + } else { + Some(parent_module_type.to_string()) + }, + sibling_index, + sibling_count, + status, + success, + duration_ms: record.duration_ms, + started_at: record.started_at, + result_prefix: record.result_prefix, + result_length: record.result_length, + }); + } + + Ok(Json(FlowAllResultsResponse { + enclosing_job, + entries, + truncated, + scope_filtered: scope_tags.is_some(), + step_error: None, + })) +} + async fn get_args( OptViewToken(view_token): OptViewToken, OptAuthed(opt_authed): OptAuthed, @@ -5812,6 +6535,15 @@ pub struct WacInlineCheckpointPayload { pub duration_ms: Option, } +#[derive(Serialize)] +pub struct WacInlineCheckpointResponse { + /// The normalized failure record, when the posted step failed. The SDK + /// raises from this rather than from its own copy, so the round that ran + /// the failing body and every replay of it read the same object. + #[serde(skip_serializing_if = "Option::is_none")] + pub failure: Option, +} + /// Fast-path endpoint called by the WAC v2 SDKs to persist a single `step()` /// checkpoint delta without unwinding the parent workflow subprocess. /// @@ -5837,7 +6569,7 @@ pub async fn wac_inline_checkpoint( Extension(db): Extension, Path((w_id, job_id)): Path<(String, Uuid)>, Json(payload): Json, -) -> error::Result { +) -> error::Result> { // Enforce ephemeral-job-token binding: the presented token must be the // one issued to *this* specific job. Regular workspace API tokens don't // have `job_id` populated in their JWT claims, so `token_job_id` is None @@ -5869,7 +6601,7 @@ pub async fn wac_inline_checkpoint( let source_hash = runnable_id.map(|h| h.to_string()); let mut tx = db.begin().await?; - windmill_common::wac::persist_inline_checkpoint_delta( + let failure = windmill_common::wac::persist_inline_checkpoint_delta( &mut tx, &job_id, source_hash.as_deref(), @@ -5881,7 +6613,9 @@ pub async fn wac_inline_checkpoint( .await?; tx.commit().await?; - Ok(StatusCode::OK) + // Only failures come back: a successful step's result can be large, and the + // SDK already holds it. + Ok(Json(WacInlineCheckpointResponse { failure })) } lazy_static::lazy_static! { diff --git a/backend/windmill-common/src/cache.rs b/backend/windmill-common/src/cache.rs index 6c34db8169..c5af3bfc1a 100644 --- a/backend/windmill-common/src/cache.rs +++ b/backend/windmill-common/src/cache.rs @@ -297,20 +297,54 @@ impl std::fmt::Debug for FlowData { } } -#[derive(Debug, Clone, Deserialize, Serialize)] +/// Top-level fields of a stored flow value that [`FlowValue`] does not model, so parsing a +/// flow into a `FlowValue` drops them. Every write-back that round-trips a stored flow +/// through `FlowValue` must capture them first and re-attach them with +/// [`FlowExtras::reattach`], or they are destroyed on save. Adding a display-only flow +/// field means adding it here — this is the only list, and `FlowValue` must never model a +/// field named here: `reattach` flattens the two together, so a name in both would be +/// emitted twice and the value would no longer deserialize. +#[derive(Debug, Clone, Deserialize, Serialize, Default)] pub struct FlowExtras { pub notes: Option>, pub groups: Option>, } +impl FlowExtras { + /// Serialize `flow` with these extras folded back in. Fallible on purpose: the result is + /// written straight over a deployed flow value, so a serialization failure must abort the + /// write rather than persist a truncated value. + pub fn reattach(&self, flow: &FlowValue) -> error::Result> { + // `flatten` + `RawValue` is fine for serialization; only deserialization breaks. + #[derive(Serialize)] + struct FlowValueWithExtras<'a> { + #[serde(flatten)] + flow: &'a FlowValue, + #[serde(skip_serializing_if = "Option::is_none")] + notes: Option<&'a Box>, + #[serde(skip_serializing_if = "Option::is_none")] + groups: Option<&'a Box>, + } + + serde_json::value::to_raw_value(&FlowValueWithExtras { + flow, + notes: self.notes.as_ref(), + groups: self.groups.as_ref(), + }) + .map_err(|e| error::Error::internal_err(format!("Failed to serialize flow value: {e}"))) + } + + /// Capture the extras carried by a raw stored flow value. + pub fn capture(raw_flow: &RawValue) -> Self { + serde_json::from_str::(raw_flow.get()) + .map_err(|e| tracing::warn!("Failed to parse flow extras: {e}")) + .unwrap_or_default() + } +} + impl FlowData { - pub fn extras(&self) -> Option { - serde_json::from_str::(self.raw_flow.get()) - .map_err(|e| { - tracing::error!("Failed to parse flow extras: {}", e); - error::Error::internal_err(format!("Failed to parse flow extras: {}", e)) - }) - .ok() + pub fn extras(&self) -> FlowExtras { + FlowExtras::capture(&self.raw_flow) } } impl FlowData { @@ -1294,7 +1328,7 @@ mod tests { assert!(data.value().modules.is_empty()); // But extras() recovers them from the raw JSON - let extras = data.extras().expect("extras should parse"); + let extras = data.extras(); let notes: serde_json::Value = serde_json::from_str(extras.notes.expect("notes present").get()).unwrap(); assert_eq!(notes.as_array().unwrap().len(), 1); @@ -1312,9 +1346,7 @@ mod tests { let raw = serde_json::value::to_raw_value(&json!({"modules": []})).unwrap(); let data = FlowData::from_raw(raw).unwrap(); - let extras = data - .extras() - .expect("extras should parse even without notes/groups"); + let extras = data.extras(); assert!(extras.notes.is_none()); assert!(extras.groups.is_none()); } @@ -1337,10 +1369,51 @@ mod tests { let data2 = FlowData::from_raw(stripped_raw).unwrap(); // Notes are gone after the FlowValue round-trip - let extras = data2.extras().expect("extras should parse"); assert!( - extras.notes.is_none(), + data2.extras().notes.is_none(), "notes lost after FlowValue round-trip" ); } + + #[test] + fn flow_extras_reattach_restores_what_the_roundtrip_drops() { + // Every write-back that re-serializes a stored flow through FlowValue must go + // through reattach, or notes/groups are destroyed. + let raw = serde_json::value::to_raw_value(&json!({ + "modules": [{ + "id": "a", + "value": {"type": "rawscript", "content": "x", "language": "bun", + "input_transforms": {}} + }], + "same_worker": true, + "notes": [{"id": "n1", "text": "t", "color": "blue", "type": "free"}], + "groups": [{"start_id": "a", "end_id": "b", "summary": "grp"}] + })) + .unwrap(); + + let data = FlowData::from_raw(raw.clone()).unwrap(); + let reattached: serde_json::Value = + serde_json::from_str(data.extras().reattach(data.value()).unwrap().get()).unwrap(); + let original: serde_json::Value = serde_json::from_str(raw.get()).unwrap(); + + for key in original.as_object().unwrap().keys() { + assert_eq!( + reattached.get(key), + original.get(key), + "{key} did not survive the FlowValue round-trip" + ); + } + + // Absent extras must stay absent rather than serialize as null, which would show + // up as a spurious change in flow diffs. + let without = + FlowData::from_raw(serde_json::value::to_raw_value(&json!({ "modules": [] })).unwrap()) + .unwrap(); + let output = without + .extras() + .reattach(without.value()) + .unwrap() + .to_string(); + assert!(!output.contains("notes") && !output.contains("groups")); + } } diff --git a/backend/windmill-common/src/error.rs b/backend/windmill-common/src/error.rs index f202bf27f9..0d5ebdc2c3 100644 --- a/backend/windmill-common/src/error.rs +++ b/backend/windmill-common/src/error.rs @@ -247,6 +247,41 @@ pub fn to_anyhow(e: T) -> anyhow:: From::from(e) } +/// Render a `tokio_postgres` error for a user-facing message. +/// +/// The pinned rust-postgres build prints only the error *kind* in its `Display` +/// impl, so `format!("{e}")` on one of these yields the useless `db error` and +/// drops the Postgres message. Interpolate errors from a `tokio_postgres::Client` +/// through this instead. +pub fn pg_error_message(e: &tokio_postgres::Error) -> String { + match e.as_db_error() { + Some(db_err) => format_db_error(db_err.message(), db_err.detail(), db_err.hint()), + // Non-database failures (io, tls, protocol) keep their message in the cause. + None => error_source_chain(e), + } +} + +fn format_db_error(message: &str, detail: Option<&str>, hint: Option<&str>) -> String { + let mut msg = message.to_string(); + if let Some(detail) = detail { + msg.push_str(&format!(" ({detail})")); + } + if let Some(hint) = hint { + msg.push_str(&format!(". Hint: {hint}")); + } + msg +} + +fn error_source_chain(e: &dyn std::error::Error) -> String { + let mut msg = e.to_string(); + let mut source = e.source(); + while let Some(cause) = source { + msg.push_str(&format!(": {cause}")); + source = cause.source(); + } + msg +} + impl IntoResponse for Error { fn into_response(self) -> axum::response::Response { let status = match self { @@ -419,4 +454,56 @@ mod tests { let rendered = Error::JsonErr(v).to_string(); assert_eq!(rendered, "[\n 1,\n 2,\n 3\n]"); } + + #[test] + fn db_error_renders_message_with_detail_and_hint() { + assert_eq!( + super::format_db_error("permission denied for schema public", None, None), + "permission denied for schema public" + ); + assert_eq!( + super::format_db_error("insert violates foreign key", Some("Key (id)=(1)"), None), + "insert violates foreign key (Key (id)=(1))" + ); + assert_eq!( + super::format_db_error( + "column does not exist", + None, + Some("Perhaps you meant \"b\"") + ), + "column does not exist. Hint: Perhaps you meant \"b\"" + ); + } + + #[test] + fn non_db_error_walks_the_source_chain() { + #[derive(Debug)] + struct Layer(&'static str, Option>); + impl std::fmt::Display for Layer { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.0) + } + } + impl std::error::Error for Layer { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.1 + .as_ref() + .map(|c| c.as_ref() as &(dyn std::error::Error + 'static)) + } + } + + // The pinned rust-postgres build renders only the kind, so everything + // actionable is in the causes: they must all reach the message. + let err = Layer( + "error connecting to server", + Some(Box::new(Layer( + "tcp connect error", + Some(Box::new(Layer("timed out", None))), + ))), + ); + assert_eq!( + super::error_source_chain(&err), + "error connecting to server: tcp connect error: timed out" + ); + } } diff --git a/backend/windmill-common/src/flow_conversations.rs b/backend/windmill-common/src/flow_conversations.rs index 33056f59fa..21b1f56389 100644 --- a/backend/windmill-common/src/flow_conversations.rs +++ b/backend/windmill-common/src/flow_conversations.rs @@ -5,6 +5,7 @@ use uuid::Uuid; use crate::db::DB; use crate::error::Result; +use crate::utils::truncate_with_ellipsis; #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, sqlx::Type)] #[sqlx(type_name = "MESSAGE_TYPE", rename_all = "lowercase")] @@ -51,12 +52,8 @@ pub async fn get_or_create_conversation_with_id( return Ok(existing); } - // Truncate title to 25 char characters max - let title = if title.len() > 25 { - format!("{}...", &title[..25]) - } else { - title.to_string() - }; + // Truncate title to 25 characters max + let title = truncate_with_ellipsis(title, 25); // Create new conversation with provided ID let conversation = sqlx::query_as!( diff --git a/backend/windmill-common/src/flows.rs b/backend/windmill-common/src/flows.rs index 5903aa8ea6..11ffd5da76 100644 --- a/backend/windmill-common/src/flows.rs +++ b/backend/windmill-common/src/flows.rs @@ -10,7 +10,6 @@ pub use windmill_types::flows::*; use anyhow::Context; use serde::Deserialize; -use serde::Serialize; use sqlx::types::Json; use sqlx::types::JsonRawValue; @@ -99,18 +98,6 @@ pub async fn get_full_hub_flow_by_path( .flow) } -/// Serialize-only wrapper that combines resolved FlowValue with display-only extras. -/// flatten + RawValue is fine for serialization (only deserialization breaks). -#[derive(Serialize)] -struct FlowValueWithExtras<'a> { - #[serde(flatten)] - flow: &'a FlowValue, - #[serde(skip_serializing_if = "Option::is_none")] - notes: Option<&'a Box>, - #[serde(skip_serializing_if = "Option::is_none")] - groups: Option<&'a Box>, -} - /// Resolve the value of a flow if any. pub async fn resolve_maybe_value( e: &sqlx::PgPool, @@ -130,17 +117,13 @@ pub async fn resolve_maybe_value( } /// Resolve modules recursively. -/// Stashes display-only fields (notes, groups) before the FlowValue round-trip -/// and re-injects them after, since FlowValue doesn't carry them. async fn resolve_value_for_api( e: &sqlx::PgPool, workspace_id: &str, value: &mut Box, with_code: bool, ) -> Result<(), Error> { - let extras = serde_json::from_str::(value.get()) - .map_err(|e| tracing::warn!("Failed to parse flow extras: {e}")) - .ok(); + let extras = FlowExtras::capture(value); let mut val = serde_json::from_str::(value.get()).map_err(|err| { Error::internal_err(format!("resolve: Failed to parse flow value: {}", err)) @@ -149,12 +132,7 @@ async fn resolve_value_for_api( resolve_module(e, workspace_id, &mut module.value, with_code).await?; } - let extras = extras.unwrap_or(FlowExtras { notes: None, groups: None }); - *value = to_raw_value(&FlowValueWithExtras { - flow: &val, - notes: extras.notes.as_ref(), - groups: extras.groups.as_ref(), - }); + *value = extras.reattach(&val)?; Ok(()) } @@ -271,43 +249,6 @@ pub async fn resolve_modules( #[cfg(test)] mod tests { use super::*; - use serde_json::json; - - #[test] - fn flow_value_with_extras_serializes_notes_and_groups() { - let input = json!({ - "modules": [], - "notes": [{"id": "n1", "text": "hello", "color": "yellow", "type": "free"}], - "groups": [{"start_id": "a", "end_id": "b", "summary": "grp"}] - }); - let input_str = serde_json::to_string(&input).unwrap(); - - // Parse FlowValue (drops notes/groups) and FlowExtras (captures them) - let val: FlowValue = serde_json::from_str(&input_str).unwrap(); - let extras: FlowExtras = serde_json::from_str(&input_str).unwrap(); - - // Serialize via FlowValueWithExtras — should include both - let combined = FlowValueWithExtras { - flow: &val, - notes: extras.notes.as_ref(), - groups: extras.groups.as_ref(), - }; - let output: serde_json::Value = - serde_json::from_str(&serde_json::to_string(&combined).unwrap()).unwrap(); - - assert_eq!(output["notes"], input["notes"]); - assert_eq!(output["groups"], input["groups"]); - assert!(output["modules"].is_array()); - } - - #[test] - fn flow_value_with_extras_omits_none_extras() { - let val: FlowValue = serde_json::from_str(r#"{"modules":[]}"#).unwrap(); - let combined = FlowValueWithExtras { flow: &val, notes: None, groups: None }; - let output = serde_json::to_string(&combined).unwrap(); - assert!(!output.contains("notes")); - assert!(!output.contains("groups")); - } #[test] fn extract_hub_flow_id_accepts_id_only_paths() { diff --git a/backend/windmill-common/src/instance_config.rs b/backend/windmill-common/src/instance_config.rs index abf1c1b662..0c8f8e4f4a 100644 --- a/backend/windmill-common/src/instance_config.rs +++ b/backend/windmill-common/src/instance_config.rs @@ -1077,12 +1077,7 @@ pub fn format_setting_value(key: &str, value: &serde_json::Value) -> String { }; } let value = mask_nested_sensitive(key, value); - let s = value.to_string(); - if s.len() > 200 { - format!("{}...", &s[..197]) - } else { - s - } + crate::utils::truncate_with_ellipsis(&value.to_string(), 197) } /// Extract the expiry timestamp from a license key JSON value. diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs index 1eda7503af..30a5edad32 100644 --- a/backend/windmill-common/src/lib.rs +++ b/backend/windmill-common/src/lib.rs @@ -1257,7 +1257,7 @@ pub async fn create_custom_instance_database( tracing::warn!( "Failed to grant permissions on '{}': {}. Continuing.", dbname, - e + crate::error::pg_error_message(&e) ); } diff --git a/backend/windmill-common/src/query_builders.rs b/backend/windmill-common/src/query_builders.rs index c0eedadbc4..c315a564af 100644 --- a/backend/windmill-common/src/query_builders.rs +++ b/backend/windmill-common/src/query_builders.rs @@ -1,6 +1,8 @@ use serde::{Deserialize, Deserializer, Serialize}; use windmill_types::scripts::ScriptLang; +use crate::error::pg_error_message; + fn deserialize_bool_from_null<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, @@ -4848,7 +4850,7 @@ fn required_str<'a>( column: &str, ) -> Result<&'a str, String> { row.try_get(column) - .map_err(|e| format!("Failed to read column {}: {}", column, e))? + .map_err(|e| format!("Failed to read column {}: {}", column, pg_error_message(&e)))? .ok_or_else(|| format!("Unexpected NULL in column {}", column)) } @@ -4894,7 +4896,7 @@ pub async fn pg_get_full_schema( ) .await .map(simple_query_rows) - .map_err(|e| format!("Failed to query columns: {}", e))?; + .map_err(|e| format!("Failed to query columns: {}", pg_error_message(&e)))?; let fk_rows = client .simple_query( @@ -4922,7 +4924,7 @@ pub async fn pg_get_full_schema( ) .await .map(simple_query_rows) - .map_err(|e| format!("Failed to query foreign keys: {}", e))?; + .map_err(|e| format!("Failed to query foreign keys: {}", pg_error_message(&e)))?; let mut result: FullDatabaseSchema = std::collections::HashMap::new(); diff --git a/backend/windmill-common/src/utils.rs b/backend/windmill-common/src/utils.rs index 7b1f2a13f9..2bd6f1b204 100644 --- a/backend/windmill-common/src/utils.rs +++ b/backend/windmill-common/src/utils.rs @@ -1364,10 +1364,45 @@ pub fn strip_json_nul(serialized: &str) -> Cow<'_, str> { Cow::Owned(String::from_utf8(out).expect("removing a NUL escape preserves valid UTF-8")) } +/// Prefix of `s` holding at most `max_chars` characters. +/// Slicing by byte index (`&s[..n]`) panics when `n` lands inside a multibyte character. +pub fn truncate_chars(s: &str, max_chars: usize) -> &str { + match s.char_indices().nth(max_chars) { + Some((byte_idx, _)) => &s[..byte_idx], + None => s, + } +} + +/// Keep at most `max_chars` characters of `s`, appending `...` when anything was dropped — +/// so a truncated result is `max_chars + 3` characters long, not `max_chars`. +pub fn truncate_with_ellipsis(s: &str, max_chars: usize) -> String { + let truncated = truncate_chars(s, max_chars); + if truncated.len() < s.len() { + format!("{}...", truncated) + } else { + s.to_string() + } +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn truncate_handles_multibyte_at_boundary() { + // Byte 25 of this string falls inside a 2-byte 'а'; naive `&s[..25]` would panic. + let cyrillic = "а".repeat(30); + assert_eq!(truncate_chars(&cyrillic, 25), "а".repeat(25)); + assert_eq!( + truncate_with_ellipsis(&cyrillic, 25), + format!("{}...", "а".repeat(25)) + ); + assert_eq!(truncate_with_ellipsis("ааааааааааааа", 25), "ааааааааааааа"); + assert_eq!(truncate_chars("abcd", 3), "abc"); + assert_eq!(truncate_with_ellipsis("abc", 3), "abc"); + assert_eq!(truncate_with_ellipsis("abcd", 3), "abc..."); + } + // The 6-char JSON escape for U+0000: backslash + "u0000". Written via an // escaped backslash so no literal NUL byte ever appears in this source. const NUL_ESC: &str = "\\u0000"; diff --git a/backend/windmill-common/src/wac.rs b/backend/windmill-common/src/wac.rs index 51abc5d60f..d46c937e33 100644 --- a/backend/windmill-common/src/wac.rs +++ b/backend/windmill-common/src/wac.rs @@ -80,6 +80,172 @@ mod tests { assert_eq!(approval_resume_id("approval_2"), 0x50d1_eeca); assert_eq!(approval_resume_id("manager"), 0x6ee4_a469); } + + use super::wac_failure_record; + use serde_json::json; + + /// The point of the function: a task failure and a step failure describing + /// the same error must be indistinguishable to the handler that catches + /// them, apart from the child job only a task has. + #[test] + fn a_task_and_a_step_failure_read_the_same() { + let from_child = wac_failure_record( + "fetch", + Some("abc-123"), + &json!({"error": {"name": "ValueError", "message": "nope", "stack": "frames"}}), + ); + let from_step = wac_failure_record( + "fetch", + None, + &json!({"error": {"name": "ValueError", "message": "nope", "stack": "frames"}}), + ); + assert_eq!(from_child["result"], from_step["result"]); + assert_eq!(from_child["message"], json!("nope")); + assert_eq!(from_step["message"], json!("nope")); + assert_eq!(from_child["child_job_id"], json!("abc-123")); + assert_eq!(from_step.get("child_job_id"), None); + } + + /// A child job's result is whatever the failing job produced — a cancel, a + /// timeout, an executor that writes a bare string. The handler is still + /// promised `name` and `message`, so they cannot be conjured per-caller. + #[test] + fn an_unshaped_child_result_still_yields_name_and_message() { + for raw in [ + json!({"error": "boom"}), + json!({"error": {"message": "boom"}}), + json!("boom"), + json!(null), + ] { + let rec = wac_failure_record("s", None, &raw); + assert_eq!(rec["result"]["error"]["name"], json!("Error"), "{raw}"); + assert!( + rec["result"]["error"]["message"].is_string(), + "{raw} produced no message" + ); + assert_eq!(rec["result"]["error"].get("stack"), None, "{raw}"); + } + // Nothing to say beats saying `{}`: the fallback exists for these. + for empty in [json!(null), json!({}), json!([]), json!({"error": ""})] { + assert_eq!( + wac_failure_record("s", None, &empty)["message"], + json!("WAC step 's' failed"), + "{empty}" + ); + } + } + + /// Extra keys are the failing side's own, and dropping them would lose a + /// custom error's fields; the three normalized ones still win. + #[test] + fn extra_error_fields_survive_normalization() { + let rec = wac_failure_record( + "s", + None, + &json!({"error": {"name": "HttpError", "message": "429", "code": 429, "stack": 12}}), + ); + assert_eq!(rec["result"]["error"]["code"], json!(429)); + assert_eq!(rec["result"]["error"]["name"], json!("HttpError")); + // a non-string stack is not something a handler can be told to read + assert_eq!(rec["result"]["error"].get("stack"), None); + } + + use super::normalize_posted_step_result; + + /// An SDK that predates the echoed record raises from the copy it built, so + /// rewriting what it posted would make its live round and its replays + /// disagree — the very thing being fixed here. It keeps its own shape. + #[test] + fn a_legacy_sdk_marker_is_stored_untouched() { + let legacy = json!({ + "__wmill_error": true, + "message": "nope", + "step_key": "s", + "result": {"error": "nope", "type": "TypeError"}, + }); + assert_eq!(normalize_posted_step_result("s", legacy.clone()), legacy); + } + + #[test] + fn a_current_sdk_marker_is_normalized_and_a_success_is_not() { + let posted = json!({ + "__wmill_error": true, + "message": "nope", + "step_key": "s", + "result": {"error": {"name": "ValueError", "message": "nope"}}, + }); + let stored = normalize_posted_step_result("s", posted); + assert_eq!(stored["result"]["error"]["name"], json!("ValueError")); + assert_eq!(stored["step_key"], json!("s")); + + let success = json!({"rows": 3}); + assert_eq!(normalize_posted_step_result("s", success.clone()), success); + } + + #[test] + fn an_oversized_stack_is_truncated() { + let rec = wac_failure_record( + "s", + None, + &json!({"error": {"message": "m", "stack": "x".repeat(100_000)}}), + ); + let stack = rec["result"]["error"]["stack"].as_str().unwrap(); + assert!(stack.len() < 100_000, "stack was not truncated"); + assert!(stack.ends_with("... (truncated)")); + } + + /// The cap bounds what lands in the checkpoint, so it has to be bytes: a + /// multibyte traceback counted in characters would be up to 4x over. + #[test] + fn the_stack_cap_counts_bytes_not_characters() { + let rec = wac_failure_record( + "s", + None, + &json!({"error": {"message": "m", "stack": "é".repeat(50_000)}}), + ); + let stack = rec["result"]["error"]["stack"].as_str().unwrap(); + assert!( + stack.len() <= 8 * 1024 + "\n... (truncated)".len(), + "kept {} bytes", + stack.len() + ); + } + + /// `extra` is the failing side's own attributes, so it can carry a response + /// body straight past the cap that exists to bound the checkpoint. + #[test] + fn an_oversized_extra_is_dropped_rather_than_stored() { + let rec = wac_failure_record( + "s", + None, + &json!({"error": {"message": "m", "extra": {"body": "x".repeat(100_000)}}}), + ); + assert_eq!(rec["result"]["error"].get("extra"), None); + assert_eq!(rec["result"]["error"]["extra_omitted"], json!(true)); + + // one that fits is kept whole + let small = wac_failure_record( + "s", + None, + &json!({"error": {"message": "m", "extra": {"code": 429}}}), + ); + assert_eq!(small["result"]["error"]["extra"], json!({"code": 429})); + assert_eq!(small["result"]["error"].get("extra_omitted"), None); + } + + /// `message` is the failure's own message; `Value::to_string` on a string + /// would hand the handler `"boom"` with the JSON quotes still on it. + #[test] + fn a_bare_string_failure_keeps_its_message_unquoted() { + assert_eq!( + wac_failure_record("s", None, &json!({"error": "boom"}))["message"], + json!("boom") + ); + assert_eq!( + wac_failure_record("s", None, &json!("boom"))["message"], + json!("boom") + ); + } } /// Load the WAC checkpoint from `v2_job_status.workflow_as_code_status._checkpoint`. @@ -139,6 +305,164 @@ pub async fn save_checkpoint( Ok(()) } +/// Marks a `completed_steps` entry as a failure rather than a step result. +pub(crate) const WAC_ERROR_MARKER: &str = "__wmill_error"; + +/// Per-field budget for the two unbounded things a failure record carries, the +/// stack and `extra`. `persist_inline_checkpoint_delta` rewrites the whole +/// checkpoint on every step, so either one left unbounded is re-serialized once +/// per subsequent step for the rest of the workflow. The two are additive, so a +/// record costs at most twice this. +const MAX_CHECKPOINT_FIELD_BYTES: usize = 8 * 1024; + +fn truncate_stack(stack: &str) -> String { + if stack.len() <= MAX_CHECKPOINT_FIELD_BYTES { + return stack.to_string(); + } + // Byte budget, not characters: the cap exists to bound what goes into the + // checkpoint, and a multibyte traceback would otherwise be up to 4x it. + let mut cut = MAX_CHECKPOINT_FIELD_BYTES; + while cut > 0 && !stack.is_char_boundary(cut) { + cut -= 1; + } + format!("{}\n... (truncated)", &stack[..cut]) +} + +/// A failure's own message, without the JSON quoting `Value::to_string` puts +/// around a string. `None` when the value carries no message at all, so the +/// caller's fallback wins — a reader handed `{}` learns less than one handed +/// "WAC step 'x' failed". +fn value_message(v: &Value) -> Option { + match v { + Value::Null => None, + Value::String(s) if s.is_empty() => None, + Value::String(s) => Some(s.clone()), + Value::Object(o) if o.is_empty() => None, + Value::Array(a) if a.is_empty() => None, + other => Some(other.to_string()), + } +} + +fn message_only(message: Option) -> serde_json::Map { + let mut m = serde_json::Map::new(); + if let Some(message) = message { + m.insert("message".to_string(), Value::String(message)); + } + m +} + +/// Build the failure record a caught WAC failure reads, from whatever the +/// failing side produced. +/// +/// The single place this shape is decided, for both a task failure (arriving as +/// the child job's own result) and a `step()` failure (as the SDK posted it). +/// Assembling it per caller instead is how the two come to disagree on `name` +/// or on `stack` while both claim to be one shape. `name`, `message` and +/// `stack` are normalized here; any other key the failing side attached to its +/// error is passed through untouched, so a custom error's own fields survive. +pub fn wac_failure_record(step_key: &str, child_job_id: Option<&str>, raw_result: &Value) -> Value { + let mut error = match raw_result.get("error") { + Some(Value::Object(o)) => o.clone(), + // A bare-string error (some executors), or a result not shaped like a + // failure at all: keep whatever it says as the message rather than + // dropping it. + Some(other) => message_only(value_message(other)), + None => message_only(value_message(raw_result)), + }; + + let name = error + .get("name") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .unwrap_or("Error") + .to_string(); + let message = error + .get("message") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .map(|s| s.to_string()) + .unwrap_or_else(|| format!("WAC step '{step_key}' failed")); + error.insert("name".to_string(), Value::String(name)); + error.insert("message".to_string(), Value::String(message.clone())); + match error.get("stack").and_then(|v| v.as_str()) { + Some(stack) => { + error.insert("stack".to_string(), Value::String(truncate_stack(stack))); + } + // Never invent one, and never keep a non-string in the field a handler + // is told it can read. + None => { + error.remove("stack"); + } + } + + // `extra` is the failing side's own attributes, so it can hold a response + // body or a dataframe repr and route straight around the stack cap into the + // checkpoint this record is rewritten into on every later step. Dropped + // wholesale past the same budget rather than truncated, since half a + // structure is worse than a flag saying it was too big. + if let Some(extra) = error.get("extra") { + if serde_json::to_string(extra).map_or(true, |s| s.len() > MAX_CHECKPOINT_FIELD_BYTES) { + error.remove("extra"); + error.insert("extra_omitted".to_string(), Value::Bool(true)); + } + } + + let mut record = serde_json::Map::new(); + record.insert(WAC_ERROR_MARKER.to_string(), Value::Bool(true)); + // `str(e)` / `e.message` reads the failure's own message whether it came + // from a task or a step; which task, and which child job, are the fields + // below rather than prose baked into the message. + record.insert("message".to_string(), Value::String(message)); + record.insert("step_key".to_string(), Value::String(step_key.to_string())); + if let Some(child) = child_job_id { + record.insert("child_job_id".to_string(), Value::String(child.to_string())); + } + record.insert( + "result".to_string(), + Value::Object( + [("error".to_string(), Value::Object(error))] + .into_iter() + .collect(), + ), + ); + Value::Object(record) +} + +/// Decide what to store for a step result an SDK posted. +/// +/// A failure is normalized through `wac_failure_record`, the same function that +/// shapes task failures, so the two cannot drift apart. +/// +/// A marker an older SDK posted is stored untouched instead. Those are +/// recognizable by an `error` that is a message string rather than an object, +/// and the SDK that posted one raises from the copy it built and ignores the +/// record echoed back to it. Rewriting it here would leave the round that ran +/// the failing body reading one shape and every replay of it reading another — +/// the divergence this whole mechanism exists to remove. It keeps its own shape +/// until it upgrades. +pub(crate) fn normalize_posted_step_result(key: &str, posted: Value) -> Value { + if !is_wac_failure(&posted) { + return posted; + } + let normalizable = posted + .get("result") + .and_then(|r| r.get("error")) + .map(|e| e.is_object()) + .unwrap_or(false); + if !normalizable { + return posted; + } + wac_failure_record(key, None, posted.get("result").unwrap_or(&Value::Null)) +} + +/// Whether a `completed_steps` entry is a failure record. +pub(crate) fn is_wac_failure(value: &Value) -> bool { + value + .get(WAC_ERROR_MARKER) + .and_then(|v| v.as_bool()) + .unwrap_or(false) +} + /// Process a completed child job result: add to checkpoint's completed_steps. pub fn add_completed_step(checkpoint: &mut WacCheckpoint, step_key: &str, result: Value) { checkpoint @@ -225,6 +549,11 @@ pub fn add_completed_step(checkpoint: &mut WacCheckpoint, step_key: &str, result /// multiple times per call, and the `||` merges re-serialize the whole /// object. The two-statement Rust-side load-modify-save below is ~10× /// faster in practice, so we keep it and rely on the SDK-level lock. +/// +/// Returns the value actually stored: a failure posted by an SDK is normalized +/// through `wac_failure_record` first, so the round that ran the failing body +/// can raise from the same record every replay will read instead of building +/// its own copy of it. pub async fn persist_inline_checkpoint_delta( tx: &mut Transaction<'_, Postgres>, job_id: &Uuid, @@ -233,7 +562,7 @@ pub async fn persist_inline_checkpoint_delta( result: Value, started_at: Option<&str>, duration_ms: Option, -) -> error::Result<()> { +) -> error::Result> { // Row-lock the existing checkpoint row (if any) for the duration of the // transaction. NULL if the row doesn't exist yet — see the doc comment // above for why the first-write race is accepted. @@ -288,6 +617,12 @@ pub async fn persist_inline_checkpoint_delta( "WAC v2 inline checkpoint — persisting step result" ); + let result = normalize_posted_step_result(key, result); + + // Only a failure is ever read back, so only a failure is copied: a + // successful step's result can be large and moves straight into the + // checkpoint. + let failure = is_wac_failure(&result).then(|| result.clone()); add_completed_step(&mut checkpoint, key, result); let status_json = serde_json::to_value(&checkpoint) @@ -335,5 +670,5 @@ pub async fn persist_inline_checkpoint_delta( .await .map_err(|e| Error::InternalErr(format!("Failed to write step timeline: {e}")))?; - Ok(()) + Ok(failure) } diff --git a/backend/windmill-common/src/wac_failure_corpus.json b/backend/windmill-common/src/wac_failure_corpus.json new file mode 100644 index 0000000000..e4498159cd --- /dev/null +++ b/backend/windmill-common/src/wac_failure_corpus.json @@ -0,0 +1,108 @@ +{ + "_readme": [ + "Cases the python and the typescript SDK must both satisfy when they serialize", + "a failed step() body into a `__wmill_error` marker.", + "", + "The two drifted apart twice while #10368 was in review: `name` was taken from", + "the constructor in one client and from `e.name` in the other, and `stack` was", + "written in two different formats. Each was caught by a reviewer reading the", + "diff, not by a test, because each suite only ever checked its own language.", + "A shared corpus turns the next divergence into a failure in both suites.", + "", + "It sits next to `wac_failure_record`, the function that decides the record's", + "final shape, because that is the contract these markers are the raw material", + "for. The SDK test suites read it by relative path.", + "", + "`thrown` describes a value each language constructs natively:", + " name the name the record must report", + " message the message the record must report", + " props own properties/attributes that must reach `extra`", + " circular_prop an own property holding a cycle, which must be dropped", + " without taking the rest of `extra` with it", + "`expect.stack` is only \"present\" or \"absent\": its text is language-specific", + "(each client matches its own executor's format) and is asserted there.", + "", + "Known divergence, deliberately not covered here: a non-finite float reaches", + "`extra` as the string \"NaN\" in python and as null in typescript, because", + "JSON.stringify has no hook for it. The executors differ the same way.", + "", + "Second known divergence, also not covered: an attribute named like a field", + "the executors report separately \u2014 `name`, `message`, `stack` \u2014 reaches", + "`extra` in python and not in typescript. Each client mirrors its own", + "executor, which differ the same way: the python one copies `__dict__`", + "wholesale, the bun/deno one filters that skip-list out." + ], + "cases": [ + { + "case": "a named error keeps its name and message", + "thrown": { + "name": "HttpError", + "message": "429 too many requests" + }, + "expect": { + "name": "HttpError", + "message": "429 too many requests", + "stack": "present", + "absent": [ + "extra" + ] + } + }, + { + "case": "custom properties reach extra", + "thrown": { + "name": "HttpError", + "message": "429", + "props": { + "code": 429, + "retry_after": 5, + "endpoint": "/v1/jobs" + } + }, + "expect": { + "name": "HttpError", + "message": "429", + "stack": "present", + "extra": { + "code": 429, + "retry_after": 5, + "endpoint": "/v1/jobs" + } + } + }, + { + "case": "a property that cannot be serialized is dropped on its own", + "thrown": { + "name": "RequestError", + "message": "socket hang up", + "props": { + "code": "ECONNRESET" + }, + "circular_prop": "request" + }, + "expect": { + "name": "RequestError", + "message": "socket hang up", + "stack": "present", + "extra": { + "code": "ECONNRESET" + } + } + }, + { + "case": "an error carrying nothing of its own has no extra", + "thrown": { + "name": "ValueError", + "message": "nope" + }, + "expect": { + "name": "ValueError", + "message": "nope", + "stack": "present", + "absent": [ + "extra" + ] + } + } + ] +} diff --git a/backend/windmill-types/src/flows.rs b/backend/windmill-types/src/flows.rs index c1213b7c05..017484caeb 100644 --- a/backend/windmill-types/src/flows.rs +++ b/backend/windmill-types/src/flows.rs @@ -875,19 +875,17 @@ pub struct AgentTool { pub value: ToolValue, } -// Convert FlowModule -> AgentTool -impl From for AgentTool { - fn from(flow_module: FlowModule) -> Self { +impl AgentTool { + /// Fold a `FlowModule` that went through dependency locking back into the tool it came from. + /// `description` has no `FlowModule` counterpart, so it must be carried over from the + /// existing tool: rebuilding an `AgentTool` from the module alone drops it on every deploy. + pub fn update_from_module(&mut self, flow_module: FlowModule) { let module_value = serde_json::from_str::(flow_module.value.get()) .unwrap_or(FlowModuleValue::Identity); - AgentTool { - id: flow_module.id, - summary: flow_module.summary, - // FlowModule has no dedicated tool description; it is carried on AgentTool only. - description: None, - value: ToolValue::FlowModule(module_value), - } + self.id = flow_module.id; + self.summary = flow_module.summary; + self.value = ToolValue::FlowModule(module_value); } } @@ -1356,6 +1354,41 @@ mod tests { assert_eq!(val.modules.len(), 1); } + #[test] + fn agent_tool_keeps_description_through_locking() { + // #10244: the dependency job rebuilds each tool from its locked FlowModule; the + // tool description lives only on AgentTool and must survive that round-trip. + let mut tool: AgentTool = serde_json::from_value(json!({ + "id": "b", + "summary": "my_tool", + "description": "when to call me", + "value": { + "tool_type": "flowmodule", + "type": "rawscript", + "content": "def main(): return 1", + "language": "python3", + "input_transforms": {} + } + })) + .unwrap(); + + let mut locked: FlowModule = Option::::from(&tool).unwrap(); + locked.value = to_raw_value(&json!({ + "type": "rawscript", + "content": "def main(): return 1", + "language": "python3", + "lock": "# py: 3.11", + "input_transforms": {} + })); + tool.update_from_module(locked); + + assert_eq!(tool.description.as_deref(), Some("when to call me")); + assert_eq!(tool.summary.as_deref(), Some("my_tool")); + assert!(serde_json::to_string(&tool.value) + .unwrap() + .contains("# py: 3.11")); + } + #[test] fn flow_rejects_absolute_step_path() { // #9751: an absolute local path baked into a step must be rejected on deploy. diff --git a/backend/windmill-worker/src/result_processor.rs b/backend/windmill-worker/src/result_processor.rs index c95bd70899..1709e4a150 100644 --- a/backend/windmill-worker/src/result_processor.rs +++ b/backend/windmill-worker/src/result_processor.rs @@ -1791,13 +1791,11 @@ pub(crate) async fn handle_wac_child_completion( step_key = %step_key, "WAC v2 child job failed, storing error for workflow try/catch" ); - json!({ - "__wmill_error": true, - "message": format!("WAC task '{}' failed (child job {})", step_key, child_job_id), - "child_job_id": child_job_id.to_string(), - "step_key": step_key, - "result": child_err, - }) + windmill_common::wac::wac_failure_record( + &step_key, + Some(&child_job_id.to_string()), + &child_err, + ) }; tracing::info!( diff --git a/backend/windmill-worker/src/worker_lockfiles.rs b/backend/windmill-worker/src/worker_lockfiles.rs index ed75e8ec27..e882bd0c4e 100644 --- a/backend/windmill-worker/src/worker_lockfiles.rs +++ b/backend/windmill-worker/src/worker_lockfiles.rs @@ -6,7 +6,6 @@ use std::fs::{create_dir_all, remove_dir_all}; use crate::ansible_executor::{get_git_repos_lock, AnsibleDependencyLocks}; use async_recursion::async_recursion; use itertools::Itertools; -use serde::Serialize; use serde_json::value::RawValue; use serde_json::{from_value, json, Value}; use sha2::Digest; @@ -33,7 +32,7 @@ use windmill_parser_yaml::AnsibleRequirements; use windmill_common::{ apps::AppScriptId, cache::{self, RawData}, - error::{self, to_anyhow}, + error, flows::{add_virtual_items_if_necessary, FlowValue}, scripts::ScriptLang, DB, @@ -647,26 +646,7 @@ pub async fn handle_flow_dependency_job( .await?; } - #[derive(Debug, Clone, Serialize)] - struct FlowValueWithExtras<'a> { - #[serde(flatten)] - value: &'a FlowValue, - - #[serde(skip_serializing_if = "Option::is_none")] - notes: Option>, - - #[serde(skip_serializing_if = "Option::is_none")] - groups: Option>, - } - - let new_flow_value = Json( - serde_json::value::to_raw_value(&FlowValueWithExtras { - value: &flow, - notes: extras.as_ref().and_then(|e| e.notes.clone()), - groups: extras.as_ref().and_then(|e| e.groups.clone()), - }) - .map_err(to_anyhow)?, - ); + let new_flow_value = Json(extras.reattach(&flow)?); // Re-check cancellation to ensure we don't accidentally override a flow. if sqlx::query_scalar!( @@ -758,14 +738,7 @@ pub async fn handle_flow_dependency_job( ) .await?; - let value_lite_with_extras = Json( - serde_json::value::to_raw_value(&FlowValueWithExtras { - value: &value_lite, - notes: extras.as_ref().and_then(|e| e.notes.clone()), - groups: extras.as_ref().and_then(|e| e.groups.clone()), - }) - .map_err(to_anyhow)?, - ); + let value_lite_with_extras = Json(extras.reattach(&value_lite)?); sqlx::query!( "INSERT INTO flow_version_lite (id, value) VALUES ($1, $2) ON CONFLICT (id) DO UPDATE SET value = EXCLUDED.value", @@ -1296,7 +1269,7 @@ async fn lock_modules( let locked = locked_iter.next().ok_or_else(|| { Error::internal_err("locked tool module should exist".to_string()) })?; - tools[idx] = locked.into(); + tools[idx].update_from_module(locked); } e.value = FlowModuleValue::AIAgent { diff --git a/benchmarks/lib.ts b/benchmarks/lib.ts index be6fe5b529..e77bfb67ba 100644 --- a/benchmarks/lib.ts +++ b/benchmarks/lib.ts @@ -2,7 +2,7 @@ import { sleep } from "https://deno.land/x/sleep@v1.2.1/mod.ts"; import * as windmill from "https://deno.land/x/windmill@v1.174.0/mod.ts"; import * as api from "https://deno.land/x/windmill@v1.174.0/windmill-api/index.ts"; -export const VERSION = "v1.773.0"; +export const VERSION = "v1.775.1"; export async function login(email: string, password: string): Promise { return await windmill.UserService.login({ diff --git a/cli/src/core/constants.ts b/cli/src/core/constants.ts index 40aee3823e..bd5add7ecd 100644 --- a/cli/src/core/constants.ts +++ b/cli/src/core/constants.ts @@ -10,4 +10,4 @@ export const WM_FORK_PREFIX = "wm-fork"; // (e.g. utils.ts) can read it without importing main.ts and creating a circular // dependency (main → workspace → utils → main) that triggers a TDZ. // Re-exported from main.ts for backwards compatibility. -export const VERSION = "1.773.0"; +export const VERSION = "1.775.1"; diff --git a/cli/src/guidance/skills.gen.ts b/cli/src/guidance/skills.gen.ts index 5f1c5ab994..57a900f50e 100644 --- a/cli/src/guidance/skills.gen.ts +++ b/cli/src/guidance/skills.gen.ts @@ -6506,6 +6506,8 @@ Python: \`except Exception\` is safe around WAC calls because internal suspensio TypeScript: avoid broad \`try/catch\` around WAC SDK calls. The SDK uses an internal suspension error during initial dispatch; catching it can break workflow suspension. If a broad catch is unavoidable, rethrow internal suspension errors before handling business errors. +A caught failure reads the same whether it came from a task or from a \`step()\`, and the same in the round that ran the failing body as in every round replaying it. It carries \`step_key\`, \`child_job_id\` (absent for a \`step()\`, which runs in the workflow job and has no child job), a \`message\` that is the failure's own message, and \`result\` = \`{"error": {"name", "message", "stack"?, "extra"?}}\`. \`name\`, \`message\` and \`stack\` are the fields that read the same whichever side failed; \`name\` and \`message\` are always there, \`stack\` only when the failure had a traceback to give. \`extra\` carries the failure's own custom fields (an exception's attributes, an error's properties) and is best-effort: it is absent when there were none, and a task can report entries a step does not, so read it defensively and don't branch on its absence. \`extra\` is dropped when it is too large to keep in the checkpoint, and \`extra_omitted: true\` says so — absent \`extra\` with no \`extra_omitted\` means the failure simply had no custom fields. Branch on those, not on the original exception type: the workflow body re-runs from the top every round and a replay rebuilds the failure from the checkpoint, so nothing outside that record survives. Python raises \`TaskError\`; TypeScript throws an \`Error\` named \`TaskError\` carrying the same fields. Nothing is chained onto \`__cause__\` / \`cause\` — the traceback is in \`result.error.stack\`, and is also printed to the job log when the step fails. + ## TypeScript Workflow-as-Code API (windmill-client) @@ -6644,14 +6646,19 @@ export async function parallel(items: T[], fn: (item: T) => PromiseLike Import: \`from wmill import workflow, task, task_script, task_flow, step, sleep, wait_for_approval, get_approval_urls, get_resume_urls, parallel, TaskError\` \`\`\`python -# Raised when a WAC task step failed. +# Raised when a WAC \`\`task\`\` or \`\`step\`\` failed. # # Attributes: # step_key: The checkpoint key of the failed step. -# child_job_id: The UUID of the failed child job. -# result: The error result from the child job. +# child_job_id: The UUID of the failed child job, or \`\`None\`\` for a +# \`\`step()\`\`, which runs in the workflow job and has no child job. +# result: \`\`{"error": {"name", "message", "stack"?, "extra"?}}\`\` — the +# same shape whether a task or a step failed. \`\`name\`\` and \`\`message\`\` +# are always present; \`\`stack\`\` only when the failure had a traceback, +# and \`\`extra\`\` only when it carried custom fields of its own, dropped +# with \`\`extra_omitted: True\`\` beside it when too large to checkpoint. class TaskError(Exception): - def __init__(self, message: str, *, step_key: str = '', child_job_id: str = '', result = None) + def __init__(self, message: str, *, step_key: str = '', child_job_id: Optional[str] = None, result = None) # Get URLs needed for resuming a flow after suspension. # diff --git a/docs/wac-sdk-e2e.md b/docs/wac-sdk-e2e.md new file mode 100644 index 0000000000..5c6682d93d --- /dev/null +++ b/docs/wac-sdk-e2e.md @@ -0,0 +1,68 @@ +# Exercising an unreleased SDK change end to end + +A job installs the **published** `windmill-client` / `wmill`, so a change in this +repo is invisible to a real job until it is injected into the worker's dependency +cache. The SDK unit suites cover neither the worker nor the SDK the worker +installs, and that gap is where the Workflow-as-Code failure contract kept coming +apart: every finding behind #10366, #10367 and #10368 came from review or from a +run like the one below, never from a green suite. + +## Recipe + +Use a private `WINDMILL_DIR`. `/tmp/windmill/` is shared by every worktree's +backend, so patching it in place leaks a modified SDK into other people's jobs. + +```bash +# Run from the repository root. The backend holds a terminal of its own; every +# other command is a subshell, so nothing depends on where the last one left you. + +# 1. a backend of your own, with its own cache root — in its own terminal +(cd backend && DATABASE_URL=... PORT=8062 WINDMILL_DIR=/tmp/windmill-mytest \ + cargo run --features quickjs) # add ,python to run python jobs + +# 2. one job to populate the cache with the published SDK +# (any preview job importing the client will do) + +# 3. build the client and overwrite what the cache holds +C=$(echo /tmp/windmill-mytest/cache_nomount/bun/windmill-client@*@@@1) +(cd typescript-client && ./build.sh && npx tsdown --format esm --no-dts \ + && cp dist/index.mjs "$C/dist/index.mjs" \ + && cp dist/index.mjs "$C/dist/client.mjs") # code-split package: cover both + +# python instead: copy the source file straight over, then drop the bytecode +cp python-client/wmill/wmill/client.py \ + /tmp/windmill-mytest/cache/python_3_12/wmill==*/wmill/client.py +find /tmp/windmill-mytest -name __pycache__ -type d -exec rm -rf {} + + +# 4. RESTART the backend — see below +# 5. run your scenarios, and rm -rf /tmp/windmill-mytest when done +``` + +## Restart the workers after injecting + +A worker materializes the package once and keeps using its copy, so patching the +cache under a running backend leaves some workers on the old code. With more than +one worker the results then **alternate run to run** as jobs land on one worker or +the other, which reads like flakiness in the product rather than in the harness. +Restarting after the swap makes it deterministic. + +Symptom worth recognising: identical jobs returning two different answers in a +stable pattern, with each run internally consistent. + +## Run your scenarios twice + +Once against the published SDK, once against the injected one. A scenario that +behaves the same either way is not testing what you think it is, and it is easy +to write several of those without noticing. + +As a calibration: a spread of WAC scenarios written this way scored 10/17 (bun) +and 7/18 (python) against the published SDK and 17/17 and 18/18 against a client +carrying #10366, #10367 and #10368. The ones that did not move were covering +behaviour those PRs never touched — worth knowing before concluding that a green +run means anything. + +## Worth covering, and easy to miss + +The deno path, `taskScript` / `taskFlow`, `waitForApproval`, and failures +interleaved with parallelism. None of these were exercised while the failure +record was being unified. diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 1cbd409013..ad6838fa37 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -1,12 +1,12 @@ { "name": "@windmill-labs/components", - "version": "1.773.0", + "version": "1.775.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@windmill-labs/components", - "version": "1.773.0", + "version": "1.775.1", "hasInstallScript": true, "license": "AGPL-3.0", "dependencies": { diff --git a/frontend/package.json b/frontend/package.json index 9c300c94d1..4bf61cee55 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "@windmill-labs/components", - "version": "1.773.0", + "version": "1.775.1", "scripts": { "dev": "vite dev", "dev:ui-builder": "mv static/ui_builder static/ui_builder.dev-disabled 2>/dev/null || true ; trap 'mv static/ui_builder.dev-disabled static/ui_builder 2>/dev/null || true' EXIT ; vite dev", diff --git a/frontend/scripts/ui_builder_artifact.json b/frontend/scripts/ui_builder_artifact.json index 4360794dbe..01cfbb2888 100644 --- a/frontend/scripts/ui_builder_artifact.json +++ b/frontend/scripts/ui_builder_artifact.json @@ -1,5 +1,5 @@ { "baseUrl": "https://pub-06154ed168a24e73a86ab84db6bf15d8.r2.dev", - "version": "1f1fe4f", - "sha256": "4c20b2b51f324e93dda3b46d914ebf7bc7d0cb4dc7eb2d37068408191220d206" + "version": "76ee616", + "sha256": "1d696d38a152179ef6e1cada5df302d078cc677b6dbc244b5c42335e11feabac" } diff --git a/frontend/src/lib/assets/tokens/githubDark.json b/frontend/src/lib/assets/tokens/githubDark.json new file mode 100644 index 0000000000..ea5b36145e --- /dev/null +++ b/frontend/src/lib/assets/tokens/githubDark.json @@ -0,0 +1,35 @@ +{ + "surface-accent-primary": "#506ab8", + "surface-accent-hover": "#5873c8", + "surface-accent-clicked": "#425797", + "text-primary": "#d5dbe2", + "text-secondary": "#8b949e", + "text-primary-inverse": "#24292f", + "text-secondary-inverse": "#57606a", + "text-tertiary-inverse": "#6e7781", + "surface-selected": "#212830", + "surface-disabled": "#151b23", + "surface-secondary": "#010409", + "surface-hover": "#6e768119", + "surface-primary": "#0d1117", + "border-light": "#242a32", + "border-normal": "#6e7681", + "border-accent": "#4a6ed6", + "surface-accent-selected": "#141a2e", + "surface-accent-secondary": "#e3e8ee", + "surface-tertiary": "#151b23", + "text-emphasis": "#dfe5eb", + "text-hint": "#6e7681", + "text-disabled": "#484f58", + "surface-accent-secondary-hover": "#d0d7de", + "surface-accent-secondary-clicked": "#afb8c1", + "component-button-accent-secondary": "#f0f6fc", + "text-emphasis-inverse": "#1f2328", + "reserved-ai": "#d2a8ff", + "component-virtual-node": "#151b23", + "text-accent": "#bdc8ee", + "border-selected": "#5a72c4", + "surface-sunken": "#010409", + "text-tertiary": "#6e7681", + "surface-input": "#0c0f14" +} diff --git a/frontend/src/lib/assets/tokens/tokens.json b/frontend/src/lib/assets/tokens/tokens.json index f4f35c81d9..349b4376b8 100644 --- a/frontend/src/lib/assets/tokens/tokens.json +++ b/frontend/src/lib/assets/tokens/tokens.json @@ -239,7 +239,12 @@ "magenta-950": "#6e2ba1" } }, - "guidelines": { "mode-1": { "blue": "#5e81ac", "demo-background": "#ffffff00" } }, + "guidelines": { + "mode-1": { + "blue": "#5e81ac", + "demo-background": "#ffffff00" + } + }, "tailwind-c-s-s-v-3-3-2": { "mode-1": { "black": "#000000", diff --git a/frontend/src/lib/components/CompareDrafts.svelte b/frontend/src/lib/components/CompareDrafts.svelte index 32be6caf35..65fe205046 100644 --- a/frontend/src/lib/components/CompareDrafts.svelte +++ b/frontend/src/lib/components/CompareDrafts.svelte @@ -22,7 +22,11 @@ draftBaseIsStale } from '$lib/utils_draft_deploy' import { checkDeployPermission, type DeployPermission } from '$lib/utils_workspace_deploy' - import { type DraftItem, useWorkspaceDrafts } from '$lib/workspaceDrafts.svelte' + import { + type DraftItem, + invalidateWorkspaceDrafts, + useWorkspaceDrafts + } from '$lib/workspaceDrafts.svelte' import type { Kind as LayoutKind } from '$lib/utils_deployable' import { userStore } from '$lib/stores' @@ -44,8 +48,8 @@ draftCount?: number /** When set (reached via a session's Review button), preselect only the * rows this chat modified — `${UserDraftItemKind}:${path}` keys, matching - * Row.key. Undefined → preselect all deployable rows (the default). All rows - * are still shown either way. */ + * Row.key. Undefined → preselect all actionable rows (the default). All + * rows are still shown either way. */ chatMask?: Set /** False while the (async) chatMask is still loading. The select-all default * waits for this so it doesn't race the mask and select everything. Defaults @@ -180,35 +184,45 @@ isFork && hideUnchanged ? items.filter((i) => i.unchanged_from_parent !== true) : items ) - // A row is actionable when it isn't already deployed this session, the user has - // write permission, AND it's their own draft (you can't deploy someone else's - // draft — those show view-only in the "all drafts" view). The server enforces - // the same; this keeps the UI honest. A data-pipeline bundle is never deployable - // from this page — its scripts deploy individually inside the pipeline view — so - // it's excluded from every selection path. - function isSelectable(item: Row): boolean { + // Selection gate: a row is actionable when it isn't already deployed this + // session AND it's the user's own draft (someone else's shows view-only in the + // "all drafts" view). A data-pipeline bundle is excluded — its scripts deploy + // individually inside the pipeline view. Every selectable row can at least be + // discarded: discarding your own email-scoped draft never needs write + // permission on the path — only legacy (ownerless) drafts stay write-gated, + // mirroring the server's discard check. + function isDiscardable(item: Row): boolean { return ( deploymentStatus[item.key]?.status !== 'deployed' && - item.can_write && item.mine && - item.draftKind !== 'data_pipeline' + item.draftKind !== 'data_pipeline' && + (!item.legacy_draft || item.can_write) ) } - // Why a row can't be deployed (drives the disabled-checkbox tooltip). - // `undefined` ⇒ actionable. + // Deploying additionally requires write permission on the path, so the Deploy + // count can be lower than the selection when a drafted path lost writability. + function isDeployable(item: Row): boolean { + return isDiscardable(item) && item.can_write + } + + // Why a row can't be selected (drives the disabled-checkbox tooltip). + // `undefined` ⇒ selectable. function blockedReason(item: Row): string | undefined { if (!item.mine) return 'This draft belongs to another user' - if (!item.can_write) return "You don't have write permission on this path" + if (item.legacy_draft && !item.can_write) + return 'Discarding a legacy draft requires write permission on the path' return undefined } // Why a row can't be discarded (drives the Discard button's title). - // Discarding only removes the caller's own draft row, which they always own, - // so — unlike deploy — it never requires write permission on the path. The - // only block is someone else's draft (view-only in the "all drafts" view). + // Discarding only removes the caller's own draft row, so it doesn't require + // write permission on the path — except for legacy (ownerless) drafts, which + // the server write-gates like a deploy. function discardBlockedReason(item: Row): string | undefined { if (!item.mine) return 'This draft belongs to another user' + if (item.legacy_draft && !item.can_write) + return 'Discarding a legacy draft requires write permission on the path' return undefined } @@ -290,8 +304,9 @@ if (ws === currentWorkspaceId) deployPerm = p }) }) - // Select all on the first non-empty load (deploy-all is the common intent); - // only once, so a refetch after a deploy doesn't re-select the leftovers. + // Select all on the first non-empty load (acting on everything is the common + // intent); only once, so a refetch after a deploy doesn't re-select the + // leftovers. let hasAutoSelected = $state(false) const deploymentStatus: Record< @@ -314,9 +329,9 @@ $effect(() => { if (!hasAutoSelected && chatMaskReady && visibleItems.length > 0) { - // Default intent is deploy-all; when reached from a session's Review + // Default intent is act-on-all; when reached from a session's Review // (chatMask set), preselect only that chat's items instead. - const selectable = visibleItems.filter(isSelectable) + const selectable = visibleItems.filter(isDiscardable) selectedItems = ( chatMask ? selectable.filter((i) => @@ -332,17 +347,24 @@ } }) - // Selected items still in the visible list and deployable. Derived (not a - // pruning effect) so the "Deploy N drafts" button stays reactive to the - // Workspace Drafts resource: deploy/discard drop items, and stale keys left in - // selectedItems are simply ignored here (and by deploySelected). - let selectedCount = $derived( - visibleItems.filter((i) => selectedItems.includes(i.key) && isSelectable(i)).length + // Selected items still in the visible list, per action. Derived (not a + // pruning effect) so the footer buttons stay reactive to the Workspace + // Drafts resource: deploy/discard drop items, and stale keys left in + // selectedItems are simply ignored here (and by the action handlers). + let deployableCount = $derived( + visibleItems.filter((i) => selectedItems.includes(i.key) && isDeployable(i)).length ) + let discardableCount = $derived( + visibleItems.filter((i) => selectedItems.includes(i.key) && isDiscardable(i)).length + ) + // Selected rows the user can discard but not deploy (own draft on a path + // without write permission) — surfaced under the footer so the diverging + // button counts are explained. + let undeployableSelectedCount = $derived(discardableCount - deployableCount) let allSelected = $derived( - visibleItems.filter(isSelectable).length > 0 && - visibleItems.filter(isSelectable).every((i) => selectedItems.includes(i.key)) + visibleItems.filter(isDiscardable).length > 0 && + visibleItems.filter(isDiscardable).every((i) => selectedItems.includes(i.key)) ) function toggleItem(item: { key: string }) { @@ -354,7 +376,7 @@ } function selectAll() { - selectedItems = visibleItems.filter(isSelectable).map((i) => i.key) + selectedItems = visibleItems.filter(isDiscardable).map((i) => i.key) } function deselectAll() { @@ -394,8 +416,8 @@ deploying = true // Snapshot the items to deploy: deployDraft invalidates the Workspace Drafts // resource, so `items` can change mid-loop — iterate a stable copy. Guard on - // isSelectable so a non-writable row can never be deployed via a stale key. - const toDeploy = visibleItems.filter((i) => selectedItems.includes(i.key) && isSelectable(i)) + // isDeployable so a non-writable row can never be deployed via a stale key. + const toDeploy = visibleItems.filter((i) => selectedItems.includes(i.key) && isDeployable(i)) let deployedAny = false for (const item of toDeploy) { deploymentStatus[item.key] = { status: 'loading' } @@ -470,6 +492,64 @@ if (item) void doDiscard(item) } + // --- Bulk discard --- + // One click can drop many drafts at once, and some of them (draft_only with + // no other drafter) are permanent deletions — always confirm, listing the + // permanent ones explicitly. + let bulkDiscardItems = $state(undefined) + let discarding = $state(false) + const bulkPermanent = $derived((bulkDiscardItems ?? []).filter(isDestructiveDiscard)) + // Third outcome the modal must cover: a draft-only item someone else also + // drafted isn't deleted — only this user's draft goes; the item survives via + // the other drafts. + const bulkSharedCount = $derived( + (bulkDiscardItems ?? []).filter((i) => i.draft_only && !isDestructiveDiscard(i)).length + ) + + function onDiscardSelectedClick() { + const toDiscard = visibleItems.filter((i) => selectedItems.includes(i.key) && isDiscardable(i)) + if (toDiscard.length > 0) bulkDiscardItems = toDiscard + } + + async function discardSelected(toDiscard: Row[]) { + discarding = true + let changed = false + for (const item of toDiscard) { + deploymentStatus[item.key] = { status: 'loading' } + // invalidate: false — one refetch after the whole batch (below), not + // one per row. + const res = await discardDraft( + item.draftKind, + item.path, + currentWorkspaceId, + item.draft_only, + item.legacy_draft, + false + ) + if (res.success) { + changed = true + delete deploymentStatus[item.key] + } else { + deploymentStatus[item.key] = { status: 'failed', error: res.error } + sendUserToast(`Failed to discard ${item.path}: ${res.error}`, true) + } + } + discarding = false + selectedItems = [] + if (changed) { + // Refetch the Draft list once for the batch, then refresh the fork + // comparison. + invalidateWorkspaceDrafts(currentWorkspaceId) + onChanged?.() + } + } + + function confirmBulkDiscard() { + const toDiscard = bulkDiscardItems + bulkDiscardItems = undefined + if (toDiscard) void discardSelected(toDiscard) + } + // Editor URL for a draft item, scoped to the current workspace. Raw apps live // under a different editor route, so map their kind accordingly. Kinds whose // editor is a drawer on a list page (variables, resources, schedules, @@ -559,7 +639,7 @@ {selectedItems} {deploymentStatus} {allSelected} - selectablePredicate={(item) => isSelectable(item as unknown as Row)} + selectablePredicate={(item) => isDiscardable(item as unknown as Row)} selectBlockedReason={(item) => blockedReason(item as unknown as Row)} onToggleItem={toggleItem} onSelectAll={selectAll} @@ -752,17 +832,34 @@ {#snippet footer()}
- +
+ + +
{#if !deployPerm.ok} {deployPerm.reason} + {:else if undeployableSelectedCount > 0} + + {undeployableSelectedCount} selected draft{undeployableSelectedCount !== 1 ? 's' : ''} + can't be deployed (no write permission on the path) but can still be discarded + {/if}
{/snippet} @@ -772,6 +869,39 @@ + (bulkDiscardItems = undefined)} +> +

+ This will discard {bulkDiscardItems?.length} draft{(bulkDiscardItems?.length ?? 0) !== 1 + ? 's' + : ''}. Items with a deployed version revert to it. +

+ {#if bulkSharedCount > 0} +

+ {bulkSharedCount} draft-only {bulkSharedCount === 1 ? 'item is' : 'items are'} also drafted by + other users: only your draft is removed and the {bulkSharedCount === 1 ? 'item' : 'items'} will + remain through theirs. +

+ {/if} + {#if bulkPermanent.length > 0} +

+ {bulkPermanent.length} + {bulkPermanent.length === 1 ? 'item exists' : 'items exist'} only as a draft and will be + permanently deleted: +

+
    + {#each bulkPermanent as item (item.key)} +
  • {item.draft_path ?? item.path}
    • + {/each} +
+ {/if} + + - import { isInitialized } from './vscode' + import { getEditorTheme, isInitialized } from './vscode' import { editor as meditor } from 'monaco-editor' @@ -8,11 +8,7 @@ function onThemeChange() { if (isInitialized) { - if (document.documentElement.classList.contains('dark')) { - meditor.setTheme('nord') - } else { - meditor.setTheme('myTheme') - } + meditor.setTheme(getEditorTheme()) } } diff --git a/frontend/src/lib/components/ScriptEditor.svelte b/frontend/src/lib/components/ScriptEditor.svelte index aaca5752c1..9a65fc9d2b 100644 --- a/frontend/src/lib/components/ScriptEditor.svelte +++ b/frontend/src/lib/components/ScriptEditor.svelte @@ -2505,7 +2505,7 @@ {/snippet} {#snippet editorContent()} -
+
{#if supportsModules}
(getDarkModeVariant()) const dispatch = createEventDispatcher() @@ -51,15 +59,39 @@ } - + (darkVariant = getDarkModeVariant())} + on:close={removeHash} + {disableChatOffset} +>
{#if scopes == undefined}
-
- Theme +
+
+ Theme +
+
+ Dark variant + { + darkVariant = v + setDarkModeVariant(v) + }} + > + {#snippet children({ item })} + + + {/snippet} + +
Windmill diff --git a/frontend/src/lib/components/apps/editor/PublicApp.svelte b/frontend/src/lib/components/apps/editor/PublicApp.svelte index 8995684e28..c0cf693e0f 100644 --- a/frontend/src/lib/components/apps/editor/PublicApp.svelte +++ b/frontend/src/lib/components/apps/editor/PublicApp.svelte @@ -1,5 +1,6 @@ - - - - - - - {label ? label + ' · ' : ''}{formatElapsed(loadingElapsedMs)} - + + Waiting for your input + +{:else} + + + + + + + {label ? label + ' · ' : ''}{formatElapsed(elapsedMs)} + +{/if} diff --git a/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte b/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte index 8360ed962f..a1ee9984e5 100644 --- a/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/ToolExecutionDisplay.svelte @@ -26,6 +26,7 @@ const isSuccessful = $derived( !message.isLoading && + !message.isQueued && !message.error && !message.needsConfirmation && !message.isStreamingArguments @@ -64,7 +65,14 @@ {#if activeUserQuestion} {:else} -
+ +
{#snippet headerButton()}
{:else if advancedSelected === 'sleep'}
- +
{:else if advancedSelected === 'debounce'}
diff --git a/frontend/src/lib/components/flows/content/FlowModuleSleep.svelte b/frontend/src/lib/components/flows/content/FlowModuleSleep.svelte index dc45738338..92086ccd37 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleSleep.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleSleep.svelte @@ -12,13 +12,16 @@ import Section from '$lib/components/Section.svelte' import Label from '$lib/components/Label.svelte' import { getStepPropPicker } from '../previousResults' + import { SAME_WORKER_INCOMPATIBLE_MSG } from '../utils.svelte' + import { Alert } from '$lib/components/common' interface Props { flowModule: FlowModule previousModuleId: string | undefined + isAgentTool?: boolean } - let { flowModule = $bindable(), previousModuleId }: Props = $props() + let { flowModule = $bindable(), previousModuleId, isAgentTool = false }: Props = $props() const { selectionManager, flowStore, flowStateStore, previewArgs } = getContext('FlowEditorContext') @@ -44,6 +47,8 @@ const result = flowStateStore.val[selectionManager.getSelectedId()]?.previewResult ?? {} let isSleepEnabled = $derived(Boolean(flowModule.sleep)) + // Agent tools never go through the flow scheduler, so `same_worker` doesn't apply to them. + let sameWorker = $derived(Boolean(!isAgentTool && flowStore.val.value.same_worker))
@@ -54,8 +59,15 @@ {/snippet} + {#if sameWorker} + + {SAME_WORKER_INCOMPATIBLE_MSG} Disable `Same Worker` in the flow settings to use a sleep. + + {/if} + { if (isSleepEnabled && flowModule.sleep != undefined) { @@ -72,7 +84,7 @@ }} />