mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 00:02:13 +00:00
3d2fa493584fbf62184256e7dc027ab792c40df2
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Diego Imbert
|
7619a7a51e |
fix(datatables): keep role passwords out of audit, and role names out of SQL
The audit parameter of a data table config save carried the whole settings blob, generated role passwords included. Redact it the way every other export of that blob already is. Both SDKs pasted the caller's role straight into the `-- role` annotation, where a newline ends the comment and leaves the rest running as whatever the first line named. Check the value against the role-name grammar the server enforces. |
||
|
Diego Imbert
|
23cd7737b6 | feat(datatables): opt-in role-based permissions | ||
  Ruben FiszelandClaude Opus 5
|
3b95c947d4 |
test(wac): pin the failure record with one corpus both SDKs read (#10385)
* test(wac): pin the failure record with one corpus both SDKs read Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(wac): add the behaviour matrix that verified the failure record Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(wac): record how to exercise an unreleased SDK change Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): guard the whole extra pair, not just its value Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): never rehash an untrusted extra key Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): walk only a real __dict__ when collecting extra Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(wac): name the divergence the corpus cannot pin Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(wac): state the extra-encoding constraints in four lines Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Ruben FiszelandClaude Opus 5
|
aeaea57ca1 |
fix(wac): one failure record for tasks and steps, in every round (#10368)
* fix(wac): hand a caught task and step failure the same shape in every round Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(wac): decide the failure record once, server-side Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): leave a legacy SDK's failure marker untouched, and ship wacError to jsr Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): carry a step's custom error fields, and bound the stack in bytes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): keep a step's extra fields serializable and bounded Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): record a non-Error throw the way a task records it Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): guard the last unguarded throw site in the step marker Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): make failure reporting non-throwing on both clients Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): take the step traceback the way the executor takes it Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): contain the reads that happen before a failure is checkpointed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): fall back to the checkpointed marker, not the live one Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): keep non-finite fields and hostile proxies out of the checkpoint path Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): keep the snapshot that passed the serialization probe Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(wac): keep the failure-record module's surface to what is used Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Ruben FiszelandClaude Opus 5
|
044ce39e5f |
fix(wac): return the checkpointed value from step(), not the live object (#10367)
* fix(wac): return the checkpointed value from step(), not the live object Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(wac): regenerate system prompts and narrow the round-trip claim Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * style(wac): condense the round-trip comments and fix the fallback note Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(sdk): type step() as the JSON round trip of its body's result Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(sdk): apply the JSON round trip to task() and the standalone paths Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(sdk): encode bigint, keep unknown as unknown, align dropped-key results Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): null out results whose key JSON.stringify would drop Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): normalize only the top-level result, keeping nested keys as they were Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wac): normalize a child task's result so a deployed job cannot fail to parse Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(sdk): pin non-finite number behavior in Jsonified and its tests Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(sdk): admit undefined for keys whose value JSON.stringify may omit Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(sdk): make a key JSON.stringify may omit optional, not just nullable Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(sdk): treat a class-valued property as dropped, like any other function Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Ruben Fiszel
|
727d22b9a1 |
fix(wac): report a task failure the child round's body catches (#10366)
* fix(wac): report a task failure the child round's body catches * chore(wac): state the child-round failure invariant once * test(wac): pin the catch-then-continue re-raise in the child round |
||
|
Ruben FiszelandClaude Opus 5
|
be5e3bbfc4 |
fix(wac): checkpoint step errors so a caught exception does not hang replay (#10348)
* fix(wac): checkpoint step errors so a caught exception does not hang replay Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(wac): honour a step suspend the workflow body caught and swallowed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(wac): park every suspend, not only those from a failing step Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(wac): keep the generated bun wrapper comment-free Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(wac): park the child task-completion suspend and align error identity Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(wac): pin the TaskError identity of replayed step and task failures Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Ruben FiszelandClaude Fable 5
|
5ad2de91a2 |
feat(sdk): enforce s3:// URIs for string S3 params + ingestion (EL) docs (#9912)
* feat(pipelines): ingestion (EL) templates + docs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(pipelines): review nits — draft collision guard, template-mode selection reset, invariant test Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(pipelines): lead the insert menu with ingestion templates Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(pipelines): ingestion story as docs-only — drop editor template UI The insert-menu template section mixed two selection grammars in one popover and confused more than it helped. The three E2E-verified example pipelines now live verbatim in docs/pipeline-ingestion.md; the Python bare-string S3 key fix in pipelineTemplates.ts stays. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(sdk): bare string S3 keys in py/ts clients + asset parsers A plain string passed where an S3Object is expected is now a bare key in the default storage — previously the py client silently degraded it to s3="" (auto-generated key) and both asset parsers canonicalized it without the leading slash, splitting lineage. parseS3Object moves to s3Types.ts so it is unit-testable without the generated services. The pipeline template fix from the earlier commit is superseded (bare strings are the supported spelling again); docs examples flipped to bare keys. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(sdk): enforce s3:// URIs for string S3Object params Bare strings now raise/throw with a hint pointing at the s3:///<key> spelling instead of being treated as keys (previous commit) or silently degrading to an empty key (original behavior). One string spelling everywhere: SDK calls, // on annotations, and DuckDB SQL all use s3:///<key>. TS regains the s3://-template-literal type; the asset parsers record no asset for a bare string (the call can only error); templates emit the URI form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(pipelines): move ingestion (EL) guide to windmilldocs, keep design constraints User-facing how-to (engine choice, cursor recipes, schema drift, worked examples) moves to windmilldocs core_concepts/63_pipelines (windmilldocs#1462); the repo keeps only the design constraints future feature work must not break, as a section of ducklake-materialization.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: regenerate system prompts after parse_s3_object docstring change Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(sdk): reject empty-key s3 URIs; align asset parsers with the runtime rule Addresses CI review: s3:/// and s3://bucket/ now raise (an empty key would fall back to the auto-generated-key path the strict contract exists to prevent); the asset parsers' string branch applies the same valid-URI-with-non-empty-key rule so no R/W edge is recorded for a call that can only error (the generic URI-literal scan still records ambiguous access-None assets, by design); comments rephrased as current constraints per AGENTS.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
 Diego ImbertandClaude Opus 4.8
|
3eeccaf968 |
feat: add ducklake schema support to the database manager (#9633)
* feat: add ducklake schema support to the database manager Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: support schema in wmill.ducklake("name:schema") template helper Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: preserve schema when parsing ducklake asset/favorite paths Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: regenerate system prompts for ducklake schema syntax doc Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Ruben FiszelandClaude Opus 4.7
|
aedf369174 |
fix: pair PG arg type with actual Rust binding to keep query_typed_raw safe (#8999)
* fix: pair PG arg type with actual Rust binding to keep query_typed_raw safe Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(pg): wrap encoder errors with arg context, add fallback test Followups on #8999 review: - Wrap rust-postgres "error serializing parameter N" failures with the arg name, JSON value kind, and asserted Postgres type plus a hint about an explicit cast — so users see actionable context instead of an opaque WrongType. - Drift-prevention meta-test: assert otyp_to_pg_type and convert_val agree on the Type for every recognised arg_t when the JSON value matches its natural Rust kind. Catches future drift if either side changes. - Integration test for the prepare + query_raw fallback path: confirms unrecognised arg_t (custom enum) is routed through prepare and the server-resolved type appears in the failure surface — flips into a test failure if a regression accidentally routes unrecognised types through query_typed_raw. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pg): add otyp_inferred flag + regex-based placeholder renumbering Two follow-ups from the review of #8999: 1. **Issue #1 (Number/Bool + explicit text decl in WHERE)** Add `Arg::otyp_inferred: bool` to the parser. The PG SQL parser sets it `true` only at the "no info → fall back to text" site (bare `$N`, no inline cast, no `-- $N (TYPE)` decl). All other arg sources keep it `false`. In `convert_val` this flag distinguishes: - explicit text-like target (`-- $1 (text)` or `$1::text`) — coerce `Bool`/`Number` → `Box<String>` so `WHERE text_col = $1` works (`text = text` operator). Pre-#8988 behaviour, restored. - parser-default text (bare `$N`) — bind the value's natural Rust type so the regression case (`Value::Bool` against a real `bool` column via `CAST AS bool`) keeps working. `Arg` is in `windmill-parser`; the new field has `#[serde(default)]` so persisted signatures stay backward-compatible. 2. **Issue #4 ($5/$50 substring rewrite collision)** Replace the per-index `String::replace` chain (which turned `$50` into `$10` when oidx=5 was processed first) with a single regex pass. `\d+` is greedy, so `$5` and `$50` match as distinct units; indices outside the mapping are left intact. 3. Tests: - parser: `test_parse_pgsql_otyp_inferred_flag` covers bare/inline- cast/decl/mixed shapes. - executor unit: `convert_val_bool_against_every_arg_t` and `convert_val_*_number_*` split each text-like target into explicit vs inferred expectations. - executor unit: `renumber_sparse_placeholders_no_collision`. - integration: `test_postgresql_arg_type_combinations` adds 4 cases covering decl(text)+Number/Bool in WHERE, bare $1+Bool, and sparse positional args ($5/$50). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pg+sdk): enum support, extended String arms, position-aware $N rewrite, SDK quality Backend: 1. **`AnyTextValue` ToSql/FromSql wrapper**: vanilla `tokio_postgres`'s `ToSql for String` / `FromSql for String` reject `Kind::Enum` and `Kind::Domain` even though the wire format is plain UTF-8. The wrapper accepts those kinds in both directions. End result: explicit `$1::my_enum` / `CAST($1 AS my_enum)` casts now round-trip without the ugly `CAST($1::text AS my_enum)` workaround, AND `SELECT enum_col` results come back as JSON strings instead of erroring at the FromSql layer. 2. **#10 — Value::String → numeric/real/double/oid/bool**. Without these arms, a string-encoded value (`"3.14"`, `"true"`) for a non-text / non-temporal arg_t fell through to `Box<String> + TEXT`, which then failed at the server (no implicit cast text→numeric in expression context). Now strings are parsed into the matching native type with clear error messages on parse failure. 3. **Position-aware `$N` rewrite**: replaces the regex-based renumbering (which fixed the `$5/$50` substring collision but still walked through string literals and comments, mangling `'price: $5'` etc.) with a walk over `parse_pg_statement_arg_positions` — the same string/comment/dollar-quote-aware tokenizer used for index discovery. Adds `parse_pg_statement_arg_positions` to the parser's public API. SDK: 4. **BigInt support**: `JSON.stringify(BigInt)` throws. The SDK now stringifies bigints before serialisation; the executor accepts numeric strings into BIGINT arg slots via the existing `Value::String → INT8` parsing arm. SDK-side `inferSqlType` is split so `BigInt` always resolves to `BIGINT` (was reaching `Number.isInteger(BigInt)` which returns false → wrong default). 5. **Homogeneous array auto-tag**: `${[1,2,3]}` against an `int[]` column now emits `$1::BIGINT[]` instead of `$1::JSON`. Detection covers primitive types only (number / bigint / string / boolean); mixed or nested arrays still fall back to JSON. Mixed int/float widens to `DOUBLE PRECISION[]`. 6. **`.query()` positional bug**: previously the `.query()` method abused the template-tag builder, which appended `$N::TYPE` after the user's literal SQL string instead of binding by position (`SELECT $1, $2` became `SELECT $1, $2$1::BIGINT`). Now `.query()` builds the executor-shaped content directly: a `-- $N argN (TYPE)` declaration block followed by the user's SQL verbatim. Tests: - Parser: `test_parse_pg_statement_arg_positions_skips_strings_and_comments` asserts string literals, comments, and dollar-quoted blocks don't produce positions (so renumbering doesn't mangle them). - Executor unit: `renumber_sparse_placeholders_no_collision_no_string_mangling` uses the new position-aware path and includes string-literal + comment + `$$…$$` cases. Existing convert_val tests grow to cover new String→numeric/real/double/oid/bool arms. - Integration: `test_postgresql_arg_type_combinations` adds 13 cases (enum round-trip both directions, string→numeric/real/double/bool/oid, string-literal `$N` non-mangling). The prepare-fallback test now asserts SUCCESS (not failure) for enum encoding via AnyTextValue. - SDK: new `typescript-client/tests/sqlUtils.test.ts` (42 tests) exhaustively covering inferSqlType primitives + arrays, parseTypeAnnotation, datatable() template tag (with all the new shapes — BigInt, homogeneous arrays, RawSql, schema preamble), datatable().query() positional, and ducklake() shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pg): replace DISCARD ALL with curated reset (preserves typeinfo cache) Found while exhaustively probing custom-type DX: every cached-connection reuse was running `DISCARD ALL`, whose included `DEALLOCATE ALL` deallocates *all* prepared statements server-side — including the typeinfo statements that tokio_postgres caches per-Client to resolve custom enum / domain Oids. tokio_postgres still held `Statement` objects whose names the server had forgotten, so the next custom-type query failed with intermittent "prepared statement \"sN\" does not exist" errors. The failure was easy to reproduce: any sequence that forced typeinfo lookup for two different custom-type kinds on the same cached connection (e.g. enum followed by domain) would hit it. Replace `DISCARD ALL` with a curated reset that explicitly targets the state we actually care about, *without* touching prepared statements: RESET ALL — GUC parameters (search_path, application _name, statement_timeout, …) RESET SESSION AUTHORIZATION — undoes both `SET SESSION AUTHORIZATION` and `SET ROLE` (RESET ALL does NOT — these aren't GUC parameters, so without this an elevated role from a previous job would silently leak) UNLISTEN * — drops LISTEN registrations CLOSE ALL — closes open cursors Trade-off: temp tables, advisory locks (session-scoped), and user-created PREPARE statements may persist across cached-connection reuse — rare in datatable / PG-script workloads. tokio_postgres's typeinfo cache survives intact, so custom enum / domain queries are fast on subsequent reuse. Tests: - `test_postgresql_custom_types_on_cached_connection` — runs 10× alternating enum + domain queries on a cached connection. Pre-fix this failed with `prepared statement "sN" does not exist` after the first reuse; post-fix passes. - `test_postgresql_set_role_does_not_leak_across_cached_connection` — switches `SET ROLE` and `SET SESSION AUTHORIZATION` to a non-postgres role, then runs a follow-up job and asserts current_user/session_user are restored. Specifically catches the case where someone might switch back to `RESET ALL` alone (which doesn't cover SET ROLE / SESSION AUTHORIZATION) and silently introduce a permission-leak vector. - All existing session-isolation tests (`test_postgresql_cached_connection_resets_session`, `test_postgresql_single_worker_session_isolation`, `test_postgresql_100_jobs_cached`) continue to pass. Found via end-to-end probing of datatable / PG-script DX, not previously covered: the existing isolation tests only did `SET ROLE postgres`, the connecting user, so the leak was invisible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pg): address PR #8999 review (cubic + claude) cubic (P1, real bug): - `convert_vec_val` for `timetz` array asserted `Type::TIMETZ_ARRAY`, but chrono `NaiveTime` only encodes for TIME (same caveat as the scalar arm). Switch to `Type::TIME_ARRAY`; rely on PG's implicit `time→timetz` assignment cast at the column site. Add an explicit unit test. claude (#1, silent failure → explicit error): - `Bool` + explicit `(char)` / `(character)` decl previously silently bound BOOL, hoping the server would cast at the use site — but PG has no implicit `bool→char` and the resulting error ("operator does not exist: bool = char") was opaque. Now error at bind time with an actionable hint to use `bool` decl or pass the value as a "t"/"f" string. claude (#2, asymmetry doc): - Object/Array still coerce to text on `matches!(typ, Typ::Str(_))` (covers both explicit AND inferred-default text), unlike Bool/Number which key on `explicit_text_target`. The asymmetry is intentional (no implicit `jsonb → text` cast in expression context vs PG having implicit `bool/int → text` casts) — added a body comment so future maintainers don't try to "align" them. claude (#3, perf): - `parse_pg_statement_arg_indices` and `parse_pg_statement_arg_positions` walked the SQL tokenizer twice. Fold into a single pass that derives the index set from the position list. claude (#4, fmt drift): - `cargo fmt` over the parser crates I touched with perl scripts in the earlier commit (windmill-parser-{sql,bash,ts,go,php,java,csharp,nu,py, rust,graphql,yaml,r}). Net cosmetic. claude (#5, parseTypeAnnotation): - One-line caveat in the SDK's `parseTypeAnnotation` that the returned string is presence-only (e.g. `${x}::DOUBLE PRECISION` returns `"DOUBLE"`, `CAST(${x} AS int)` returns `"int)"` — neither matches a real PG type, but the only consumer just checks `!== undefined`). While here — discovered + fixed independently while exhaustively probing DX: - **Replace `DISCARD ALL` with curated reset** (`RESET ALL; RESET SESSION AUTHORIZATION; UNLISTEN *; CLOSE ALL;`). DISCARD's `DEALLOCATE ALL` killed tokio_postgres' typeinfo cache, producing intermittent `prepared statement "sN" does not exist` errors on custom-type queries after cached-conn reuse. New regression tests: `test_postgresql_custom_types_on_cached_connection` and `test_postgresql_set_role_does_not_leak_across_cached_connection` (the latter catches the case where someone might switch back to `RESET ALL` alone and silently introduce a permission-leak vector — RESET ALL doesn't cover SET ROLE / SET SESSION AUTHORIZATION). - **ISO-8601 timestamp results** (`pg_cell_to_json_value`). Pre-fix `TIMESTAMP` was rendered with a space separator ("2024-01-15 10:30:00") and `TIMESTAMPTZ` with " UTC" suffix ("2024-01-15 10:30:00 UTC") — neither parseable by `date-fns parseISO`, JavaScript `new Date()` is lenient enough to handle them but several frontend `App*Input.svelte` components use parseISO and fail silently. Switched to ISO-8601 with `T` separator and `+00:00` offset; arg-parsing path still accepts the legacy " UTC" suffix for back-compat. Test coverage: - 17/17 unit (`pg_executor::tests`) - 9/9 integration (`backend/tests/worker.rs`, `test_postgresql_*`) - 27/27 parser (`windmill-parser-sql`) - 42/42 SDK (`typescript-client/tests/sqlUtils.test.ts`) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pg): bounded one-shot warning on numeric precision loss + ISO-8601 + NaN handling Found while probing PG-script DX with millions of numeric cells: 1. **Numeric precision-loss warning**: `numeric` results are still serialised as JSON Number (back-compat — switching to JSON String would silently break user code doing arithmetic on results), but we now detect `Decimal -> f64 -> Decimal` round-trip failure and emit a single job-log warning recommending a `::text` cast in the SQL. Bounded by `NUMERIC_PRECISION_CHECK_BUDGET = 256` cells per query (one atomic load + one fetch_sub on the hot path; first lossy value short-circuits to a single load thereafter). Worst-case overhead on a 1M-cell numeric-heavy query: ~25µs of checks + 5ns × N atomic loads (vs. ~100ms unbounded). 2. **ISO-8601 timestamps**: `pg_cell_to_json_value` previously returned `"2024-01-15 10:30:00"` (TIMESTAMP) and `"2024-01-15 10:30:00 UTC"` (TIMESTAMPTZ) — neither parseable by date-fns `parseISO`, which is what the apps `App*Input.svelte` components use, so timestamp values silently failed to round-trip into date pickers. Switch to ISO-8601 (`T` separator + `+00:00` offset) on the result side; arg-parser continues to accept the legacy `" UTC"`-suffixed format for back-compat. 3. **Float NaN / Infinity results**: `Number::from_f64` returns None for NaN / ±Inf, which `pg_cell_to_json_value` was raising as "invalid json-float" — failing the *entire* query if any cell held one of these special values. Now serialise them as JSON strings ("NaN", "Infinity", "-Infinity") and let the rest of the row come through. Arg-side: `s.parse::<f64>()` already accepts the same strings. Tests: - `decimal_fits_f64_losslessly_predicate` — covers fits / doesn't-fit cases for the precision-loss predicate. - `precision_check_budget_caps_per_query_overhead` — locks in the budget cap and the loss-flag short-circuit. - All 9 PG integration tests + 17 unit tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pg): add pg_advisory_unlock_all to reset; warn on missing args; honor decl defaults While probing PG-script DX further found three more frictions: 1. **Advisory lock leak** (cubic P2): switching from `DISCARD ALL` to `RESET ALL; RESET SESSION AUTHORIZATION; UNLISTEN *; CLOSE ALL;` meant session-scoped advisory locks (`pg_advisory_lock`) leaked across cached-connection reuse. Add `SELECT pg_advisory_unlock_all()` to the chain — `DISCARD ALL` covered this implicitly via `DISCARD PLANS / DEALLOCATE / pg_advisory_unlock_all` and we lost it in the switch. 2. **Missing-arg silent NULL**: an arg declared in the SQL (e.g. `-- $1 amount (numeric)`) but not provided in the args object was bound as NULL with no error / warning. Misspelling the key in the args object silently produced a row of NULLs — a notorious DX debugging trap. Now: collect the names of declared-but-missing args during dispatch and emit a single one-shot warning to the job logs at end-of-query naming each one. Bound NULL is preserved for back-compat. 3. **Declaration defaults ignored**: `-- $1 a (int) = 5` carries `arg.default = Some(Number(5))`, but the dispatch fell straight to NULL when the arg was missing. Now: respect the default — user-supplied value > declaration default > NULL. Also fixes the warning logic above (only warn for args that *don't* have a default). Tests: existing 19 unit + 9 integration pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pg): multi-word PG types with [] suffix lost the array-ness; array arms accept stringified values Two more frictions found while probing SDK end-to-end against a real datatable resource: 1. **Multi-word array types lose the [] suffix in the parser**. `transform_types_with_spaces` recognises aliases for "double precision", "character varying", "timestamp with time zone", etc. but its return type was `&'a str` — only the bare alias, never with a trailing `[]`. The `RE_CODE_PGSQL` regex's `\w+` captures stop at the first space, so the regex's own `(?:\[\])?` array-suffix branch sees only `"double"` (not `"double precision[]"`); the `[]` was silently lost. Result: `$1::double precision[]` (which the SDK now emits for homogeneous float arrays via the new auto-tag) routed through `Value::Array → Type::JSONB` and the server failed with "cannot cast type jsonb to double precision[]". Fix: switch `transform_types_with_spaces` to return `Cow<'a, str>` and re-check the trailing bytes after a multi-word match. If they start with `[]`, return `format!("{alias}[]")` — Owned. Single-word types and the no-match path keep returning Borrowed slices, so no allocation in the hot path. 2. **Array arms in `convert_vec_val` rejected stringified values for numeric / int* / bool / oid / real / double**. The scalar `convert_val` already parses strings into the matching native type for these arg_ts, but the array variant only accepted JSON-native counterparts. Sending `["1.5", "2.5", "3.5"]` against `$1::numeric[]` (e.g. via `unnest` for bulk loading, or `JSON.stringify(BigInt[])` round-trip) failed with "Mixed types in array". Now the array arms mirror the scalar ones — `as_<native>().or_else(|| as_str().and_then(parse))` — so both shapes round-trip cleanly. Tests: 19 unit + 9 integration pass; existing parser tests cover the multi-word array forms (the regex-cap behaviour didn't break for single-word types, and Cow plumbing is transparent to all callers). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(parsers): add otyp_inferred field to Arg literals in tests + 3 missed src files CI failures: the perl-driven sweep that added `otyp_inferred: false` to every `Arg { ... }` literal when I introduced the field in the parser schema covered `src/lib.rs` files but missed: - parsers/windmill-parser-bash/src/lib.rs (mass-edited but a later format pass un-applied a few sites) - parsers/windmill-parser-go/src/lib.rs (same) - parsers/windmill-parser-graphql/src/lib.rs (same) - parsers/windmill-parser-nu/tests/tests.rs (test file — not swept the first time) - parsers/windmill-parser-ts/tests/tests.rs (test file — same) Also tightened the regex to handle `oidx: None` without the trailing comma (some test files had the field as the last initialiser line). `cargo build --features <CI feature combo> --workspace --all-targets` is clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sdk): Date → TIMESTAMPTZ; NaN / ±Infinity → string Two more frictions found while running the actual SDK end-to-end against a live datatable resource: 1. **JS `Date`** fell into the typeof "object" branch and was tagged `::JSON`. It worked accidentally for `${date}::timestamptz` via PG's `json → text → timestamptz` implicit cast chain, but `${date}` against a `timestamptz` column without a user-supplied cast bound the value as a JSON string and the comparison `timestamptz = json` failed. Now: `inferSqlType` recognises `Date` and tags `::TIMESTAMPTZ`; `serializeArgValue` emits `Date.toISOString()` so the executor's `Value::String → TIMESTAMPTZ` arm parses it cleanly. 2. **JS `NaN` / `±Infinity`** silently became NULL. `JSON.stringify(NaN)` returns `"null"` per the JS spec, so the value reached the executor as JSON null — the SDK's `::DOUBLE PRECISION` tag then bound a NULL double. Fix: detect non-finite numbers in `serializeArgValue` and stringify them as `"NaN" / "Infinity" / "-Infinity"`. The executor's `Value::String → FLOAT8` arm (`f64::from_str`) accepts these literals directly, and the result-side already renders the values as JSON strings (matching round-trip). SDK unit tests grow from 42 → 44 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(pg): integration coverage for multi-word arrays + stringified array elements Locks in the two array fixes from the previous commit (`fix(pg): multi-word PG types with [] suffix lost the array-ness`) with end-to-end cases in `test_postgresql_arg_type_combinations`: - `double precision[]`, `character varying[]`, `timestamp without time zone[]` — verifies the parser keeps the `[]` suffix after multi-word alias resolution. - `numeric[]` / `int[]` / `bool[]` from stringified primitives — verifies the array arms of `convert_vec_val` apply the same string-coercion the scalar arms do. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: fix indentation drift on otyp_inferred lines cargo fmt cleanup of leftover indentation where the perl-driven sweep that introduced the otyp_inferred field landed at the wrong column. No behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> |
||
|
Ruben FiszelandClaude Opus 4.6
|
a6d4390790 |
feat: workflow-as-code (WAC) v2 (#8172)
* feat: workflow-as-code v2 with @task decorator API
Replace ctx.step("name", "script") API with @task decorators where
functions are called directly. Users no longer need to pass WorkflowCtx
or use string-based step names/script paths.
Python: @task decorator with contextvars-based implicit context
TypeScript: task() wrapper with module-level context variable
Parsers: detect @task function calls instead of ctx.step() calls
Worker: updated wrappers to set implicit context
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: WAC v2 checkpoint/replay with _executing_key child dispatch
- Rust-side orchestration: parent dispatches child jobs, suspends, resumes on completion
- _executing_key in checkpoint tells child which step to execute directly
- task() throws StepSuspend(mode="step_complete") after executing target step
- result_processor handles child completion and updates parent checkpoint
- WacGraph.svelte for runtime execution visualization
- Sequential and parallel workflows tested end-to-end
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: WAC v2 bundle cache, globalThis ctx sharing, description optional
- Disable bun bundle caching for WAC v2 scripts (wrapper needs
windmill-client from node_modules, not available in bundle mode)
- Use Reflect.set/get(globalThis, "__wmill_wf_ctx") to share workflow
context across dual module instances (wrapper vs user script)
- Never-resolving thenable for non-matching steps in child job mode
prevents Promise.all race conditions
- Make description field optional in NewScript API (defaults to "")
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: add step() primitive for inline checkpointed steps
step() executes a function inline (no child job) and persists the result
to the checkpoint. On replay, the cached value is returned — ensuring
deterministic behavior for non-deterministic operations like Date.now()
or Math.random().
- TypeScript: step(name, fn) — executes inline, throws StepSuspend with
mode "inline_checkpoint" to persist before continuing
- Rust: InlineCheckpoint variant in WacOutput, saves to checkpoint and
resets running=false for immediate re-pickup (no zombie wait)
- Shared step counter between task() and step() via _allocKey()
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: add Python WAC v2 support with task(), step(), workflow()
- Python SDK: WorkflowCtx with _executing_key child mode, _alloc_key
shared counter, _run_inline_step for step(), _execute_directly and
_never_resolve for child mode, step() async function
- Python executor: WAC v2 detection, checkpoint.json writing, WAC
wrapper.py generation calling _run_workflow(), post-execution hook
into shared handle_wac_v2_output()
- Make handle_wac_v2_output pub so both bun and python executors share
the same dispatch/suspend/inline-checkpoint logic
- 17 Python tests covering dispatch, replay, parallel, conditional,
inline checkpoint, and child mode
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* chore: update sqlx prepared queries
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: WacGraph Tooltip→Popover, simplify wacToFlow parsers
- Fix type error: Tooltip doesn't accept text snippet, use Popover
- Extract shared helpers for task matching and block collection
- Replace linear tasks.find() with Map lookups
- Remove mutable module-level counter
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: Box::pin WAC v2 output handler to prevent stack overflow
handle_python_job's async state machine was too large when combined
with handle_wac_v2_output. Box::pin heap-allocates the future.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: merge WAC v1 and v2 task decorators to preserve backward compat
The v2 @task decorator was shadowing the v1 one, breaking WAC v1
scripts that rely on HTTP-based dispatch via /workflow_as_code/ API.
The merged decorator handles three modes:
- v2: inside @workflow context → checkpoint/replay dispatch
- v1: WM_JOB_ID set, no @workflow → HTTP API dispatch + wait_job
- standalone: no Windmill env → execute function body directly
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: skip no_main_func detection for WAC v2 scripts in TS and Python parsers
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: prevent empty/noop dispatch causing infinite requeue loop
- Validate steps.len() > 0 in WAC dispatch handler (issue 3)
- Replace noop StepSuspend throw with never-resolving promise so it
can't reach the backend as an empty dispatch (issue 4)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: Python task wrapper now converts positional args to kwargs in v2 mode
Previously only **kwargs were passed to _next_step(), silently dropping
positional arguments. Extract shared _merge_args() helper used by both
v1 and v2 paths.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: replace unwrap() with proper error propagation in WAC arg serialization
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: add workspace_id filter to v2_job queries in WAC dispatch
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: prevent race condition in WAC child dispatch
Restructure dispatch to save checkpoint + suspend parent + seed child
checkpoints in a single transaction BEFORE pushing child jobs. This
ensures a fast child can't complete before the parent is suspended.
Also wrap InlineCheckpoint save + running reset in a transaction to
prevent corrupted state on crash.
Use ULID for pre-generated child job IDs (consistent with rest of API).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: include step key and child job ID in WAC error propagation
Move step_key lookup before the success check so failed child errors
include which task failed, the child job ID, and the original error.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* docs: document WAC determinism contract and step dispatch semantics
- Document that workflow functions must be deterministic across replays
- Document that WacStepDispatch.script/args are metadata, not dispatch targets
- Add comments on counter-based key allocation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: tighten WAC v2 detection to reduce false positives
Replace naive substring matching with line-aware checks that skip
comments and look for specific patterns:
- TS: import from "windmill-client" containing workflow/task
- Python: @workflow and @task decorators with wmill import
Extracted shared helpers in wac_executor.rs used by both executors.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: show failed steps in WacGraph when workflow completes with errors
When flowDone is true and a pending step isn't in completedSteps,
mark it as 'failed' instead of 'running'. The failed state CSS and
XCircle icon were already defined but never triggered.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: unsuspend and fail parent when WAC child push fails
Previously if a child push failed mid-batch, the parent remained
suspended with suspend = num_steps but fewer children, hanging until
the 14-day timeout. Now the push loop catches errors and unsuspends
the parent before returning the error.
Also adds source hash validation: if the script content changes between
replays, the job fails with a clear error instead of silently feeding
stale checkpoint data into wrong steps.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: clear suspend_until when unsuspending WAC parent
Set suspend_until = NULL alongside suspend = 0 in both the child
failure and all-children-complete paths, so the parent doesn't rely
on subtle pull query invariants to be re-picked-up.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* test: add exhaustive edge case tests for WAC v2 SDK
fix: make TS task wrapper non-async to fix unawaited task flush
The async wrapper caused microtask-based thenable auto-resolution that
fired .then() and threw StepSuspend before _flushPending() could capture
unawaited steps — making the flush mechanism completely broken. Now the
thenable is returned directly without async wrapping. Backward compatible
with v1 (all code paths still return awaitables).
Tests added (59 TS + 66 Python) covering: full sequential lifecycle,
step after parallel, parallel after parallel, conditional on step result,
empty/single-task workflows, 10+ steps, falsy value preservation, inline
steps, mixed step/task, unawaited flush, child mode with parallel,
key determinism, large parallel groups, and complex mixed patterns.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: atomic checkpoint updates to prevent parallel child race condition
Replace read-modify-write pattern in handle_wac_child_completion with
atomic SQL operations:
- completed_steps merged via jsonb_set(... || jsonb_build_object(...))
so concurrent children on different workers don't overwrite each other
- suspend counter decremented atomically with RETURNING to determine
"all done" condition (instead of checking completed_steps in memory)
- suspend_until cleared in the same atomic decrement statement
Before this fix, two parallel children completing simultaneously could
both load the same checkpoint, each add their step, and save — the
second write would overwrite the first, silently losing a child result
and leaving the parent suspended forever.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: cancel already-pushed children on partial WAC dispatch failure
When pushing child jobs sequentially, if pushing child N fails, children
1..N-1 are already running. Previously the error handler only unsuspended
the parent, leaving orphaned children that would complete and corrupt the
checkpoint state (decrementing suspend on an already-unsuspended parent,
potentially causing duplicate step execution on re-run).
Now on partial failure:
1. Cancel all already-pushed children (prevents them from completing
and corrupting checkpoint state)
2. Clear pending_steps from checkpoint (so parent doesn't think
children are outstanding on re-run)
3. Then unsuspend parent (so the error propagates)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: skip WAC duration write and child check for non-WAC parents
The duration write to workflow_as_code_status was running for every
non-flow child with a parent (error handlers, success handlers,
run_script children), even though it was only intended for WAC jobs.
Add WHERE workflow_as_code_status IS NOT NULL to skip non-WAC parents
entirely. Piggyback RETURNING pending_steps.job_ids on the same query
so WAC v2 child completion needs zero extra DB round-trips on the
success path.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: seed child checkpoint in same transaction as push
The child checkpoint insert was happening before the child job was
pushed, violating the FK constraint on v2_job_status. Move it into
the push transaction so the job row exists and the child can't be
picked up before its checkpoint is ready.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: set running=false when WAC parent suspends for child dispatch
The parent job kept running=true after suspending, so workers wouldn't
pick it up when children completed and suspend reached 0. The parent
only advanced when the zombie job detector reset it (~90s). Now the
dispatch suspend sets running=false so the parent is immediately
eligible for pickup.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: WAC parent suspend/unsuspend lifecycle
Keep running=true when suspending the parent so the normal pull query
(WHERE running=false) never picks it up. Keep suspend_until non-null
when decrementing suspend to 0 so the suspended pull query
(WHERE suspend_until IS NOT NULL AND suspend<=0) picks it up.
Previously: setting running=false caused infinite restart loops because
the normal pull query has no suspend check and would immediately re-pick
the parent. Clearing suspend_until on the last child prevented the
suspended pull from ever seeing it, requiring the 90s zombie detector.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: add approval primitive, flow child completion, timeline fixes for WAC v2
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: add error propagation, task options, sleep, and parallel for WAC v2
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* test: fix python SDK tests to use name-based keys and add new test coverage
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address WAC v2 review findings (sleep timing, error marker, atomicity)
- Fix sleep using suspend=1 instead of 0 to enforce actual delay
- Add approval/sleep resume injection to Python executor
- Fix TS SDK concurrency_limit mapping (was reading wrong property)
- Namespace error marker as __wmill_error to avoid user data collision
- Wrap child completion SQL in transaction for atomicity
- Decrement suspend even when step key is missing (prevents hang)
- Expand TASK_RE to handle export const, let, var, generics
- Validate step key uniqueness before dispatch
- Log warning on checkpoint deserialization failure
- Remove unimplemented delete_after_use from SDKs
- Add TaskError exception class to Python SDK with diagnostic context
- Fix extra positional args handling and add functools.wraps
- Improve getParamNames to handle typed/destructured params
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* sqlx
* sqlx
* test: add WAC v1 e2e integration tests for TS and Python
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: revert fake test versions in typescript-client
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: remove unused WacGraph component and strip wacToFlow to isWorkflowAsCode
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: extract shared approval/sleep resume logic into wac_executor
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
|