Files
windmill/cli/test/pipeline_bounded_cascade_unit.test.ts
Ruben Fiszel b13113964a fix(pipelines): canonicalize S3 asset keys so SDK writes and DuckDB reads connect (#9939)
* fix(pipelines): canonicalize S3 asset keys so SDK writes and DuckDB reads connect

The SDK object forms — TS `writeS3File({s3:"exports/x"})` and Python
`write_s3_file(S3Object(s3="exports/x"))` — resolve to the URI `s3:///exports/x`
(empty default storage), whose parsed asset path was `/exports/x` (leading
slash). DuckDB `read_csv('s3://exports/x')` and the `// on s3://exports/x`
trigger form yielded the bare `exports/x`. The same object thus produced two
asset identities, so a DuckDB consumer never connected to a TS/Python producer
in the pipeline graph.

`parse_asset_syntax` (shared by the native backend parsers and the wasm parser
that drives `frontend/src/lib/infer.ts` and the CLI `localGraph`) now strips a
single leading slash from S3 paths, so `s3:///key`, `s3://storage/key`, DuckDB
`s3://…`, and `// on` all canonicalize to one key. Both deploy-time inference
and editor/CLI inference agree, and the producer's write edge and the
consumer's read/trigger edge share a node.

Only one leading slash is stripped, so `s3:///` triple-slash default-storage
keys collapse to the bare key while Hive-partition keys
(`s3://bucket/y=2024/f.parquet`) and explicit-storage `s3://storage/key` paths
are untouched. Non-S3 asset kinds (res://, ducklake://, …) keep their paths
verbatim.

Note: existing deployed pipelines that recorded `/key` paths need a redeploy to
pick up the canonical `key`; the fix is forward-consistent for anything parsed
after this change.

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

* docs(pipelines): mark S3 asset-path normalization (item 6) resolved

The open-issues list still flagged the SDK-form leading-slash vs bare-URI
no-slash mismatch as "Still open", contradicting the fix in this PR. Mark it
resolved to match the updated Language-coverage prose.

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

* docs+test(pipelines): disclose S3 explicit-storage vs default-storage-nested-key aliasing

Collapsing to one canonical key means `s3://storage/key` (explicit storage) and
`s3:///storage/key` (default-storage nested key) now alias to the same node
`storage/key`, though they name different objects. Low-probability (needs a
storage config named to match a default-storage prefix) and inherent to a
best-effort lineage graph that doesn't split the first segment as a storage
name, but previously undisclosed. Document the tradeoff and pin the intended
aliasing with a test so it's intentional, not a latent surprise.

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

* fix(pipelines): mirror S3 leading-slash strip in frontend live-preview parser

The pipeline graph live preview parses `// on` annotations client-side via the
hand-written `parsePipelineAnnotations.ts` (a TS mirror of the Rust annotation
scanner), NOT the wasm parser. Its `parseAssetSyntax` still returned the raw
suffix, so `// on s3:///exports/x` yielded `/exports/x` while the deploy-time
and wasm parsers now canonicalize to `exports/x`. `resolveGraph` synthesizes
trigger edges from that path, so the browser preview could still render
disconnected `/exports/x` and `exports/x` nodes for the exact triple-slash case
this PR fixes at deploy time.

Mirror the S3-only single-leading-slash strip in the TS parser and extend the
shared parity fixture corpus (run by both the Rust and TS parity suites) with
the triple-slash trigger case, so Rust/TS drift on this is now caught.

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

* fix(pipelines): seed slashless S3 template asset paths to match canonical key

`autoOutputAsset` seeded new S3 template outputs with a leading slash
(`/pipelines/…`), which the old parser required to match `s3:///key` writes.
This PR made `parse_asset_syntax` strip that slash, so the seeded draft asset
(stored as `outputAssets`, used by `resolveGraph` for inactive-draft node
identity) no longer matched the body-inferred identity `pipelines/…` — the live
preview could render a duplicate `/pipelines/…` node and a phantom post-deploy
drift warning.

Seed the canonical slashless key instead, and switch the DuckDB body's S3 URIs
from `s3://${path}` to `s3:///${path}` so the generated runtime URI stays the
triple-slash default-storage form byte-for-byte (the SDK sites already build
`s3:///` + bare key). Add a pure-logic parity test asserting, for every
language and S3 output kind, that the seeded asset path is slashless and that
every S3 URI the generated body emits is triple-slash and canonicalizes back to
that seeded path.

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

* fix(pipelines): canonicalize S3 keys in CLI + frontend bounded-cascade resolvers

Two more hand-written S3-URI sites returned the raw suffix, so `s3:///exports/x`
stayed `/exports/x` while native/wasm parsers now canonicalize to `exports/x`:

- `cli/src/commands/pipeline/localGraph.ts` — the no-wasm fallback `// on`
  scanner (go/bash/ruby). A fallback consumer's `// on s3:///x` would not
  connect to a wasm-inferred `x` producer in `wmill pipeline show/run --local`.
- `boundedCascade.ts` `assetUriToNodeId` (duplicated in the CLI and the frontend
  AssetGraph engines, kept in sync) — `--to s3:///exports/x` / a cascade bound
  token would not resolve against the canonical graph node `s3object:exports/x`.
  `resolveToken` delegates here, so it is covered too.

Mirror the S3-only single-leading-slash strip in all three, and add `s3:///`
tests to the CLI local-graph fallback suite and both bounded-cascade suites
(explicit-storage and Hive-partition keys asserted untouched).

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

* docs(pipelines): phrase S3 template test comment as a current invariant

Describe the slashless-seed requirement as the invariant it is, not as change
history, per the AGENTS.md "describe the code as it is" rule.

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

* fix(pipelines): strip all leading slashes from S3 keys so trigger refs round-trip

`parse_asset_syntax` stripped only one leading slash, so `S3Object(s3="/x")` —
which resolves to the quad-slash URI `s3:////x` — parsed to path `/x`. But
`trigger_spec_to_row` rebuilds a stored trigger ref as `s3://<path>` =
`s3:///x`, which `parse_asset_trigger_ref` then parses back to `x`. The
producer recorded `/x` while its consumer trigger resolved to `x` → a broken
edge. The same asymmetry affects every `s3://`+path reconstruction site
(backend refs, frontend `assetUri`, page refs) whenever a path starts with `/`.

Strip ALL leading slashes so a canonical S3 path never starts with `/`; naive
`prefix + path` reconstruction then round-trips everywhere. Applied uniformly
across all six S3-URI sites (Rust `parse_asset_syntax`, the TS live-preview
parser, template `s3Key`, and the frontend+CLI `assetUriToNodeId` and CLI
fallback scanner). The pathological leading-slash key collapses to the bare key
— acceptable for a best-effort lineage graph that never split storage anyway.

Tests: a windmill-common round-trip test (parse → trigger_spec_to_row →
parse_asset_trigger_ref) over every URI form incl. the quad-slash case; a
`s3:////x` shared parity fixture (Rust + TS); and quad-slash assertions in the
Rust parser test and both bounded-cascade suites.

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

* test(pipelines): align S3 template parity helper with strip-all canonicalization

The template seed/body parity test's `canonicalS3Key` helper (and its comment)
still stripped a single leading slash, so it no longer mirrored the parser it
claims to pin. Strip all leading slashes to match `parse_asset_syntax` and the
frontend/CLI mirrors.

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-06 01:18:29 +02:00

308 lines
12 KiB
TypeScript

import { expect, test } from "bun:test";
// Mirror of
// frontend/src/lib/components/assets/AssetGraph/boundedCascade.test.ts — keep
// the two engines in sync. (Bun project → bun:test, not Deno.)
import {
type BCGraph,
ancestors,
assetUriToNodeId,
boundedSet,
buildLineageDag,
descendants,
resolveToken,
scriptNodeId,
scriptsOf,
topoOrder,
validStarts,
validFromStarts,
nonAutorunTriggerScripts,
reachableCutting,
isScriptNode,
scriptPathOf,
} from "../src/commands/pipeline/boundedCascade.ts";
type W = [script: string, asset: string];
type S = [script: string, asset: string];
type R = [script: string, asset: string];
function graph(opts: {
scripts?: string[];
writes?: W[];
reads?: R[];
subs?: S[];
native?: Array<[kind: string, script: string]>;
}): BCGraph {
const { scripts = [], writes = [], reads = [], subs = [], native = [] } = opts;
return {
assets: [],
runnables: scripts.map((p) => ({ path: p, usage_kind: "script" as const })),
edges: [
...writes.map(([s, a]) => ({
runnable_kind: "script",
runnable_path: s,
asset_kind: "datatable",
asset_path: a,
access_type: "w" as const,
})),
...reads.map(([s, a]) => ({
runnable_kind: "script",
runnable_path: s,
asset_kind: "datatable",
asset_path: a,
access_type: "r" as const,
})),
],
triggers: [
...subs.map(([s, a]) => ({
trigger_kind: "asset" as const,
asset_kind: "datatable",
asset_path: a,
runnable_kind: "script",
runnable_path: s,
})),
...native.map(([kind, s]) => ({
trigger_kind: kind,
runnable_kind: "script",
runnable_path: s,
})),
],
};
}
const sn = scriptNodeId;
const sorted = (it: Iterable<string>) => [...it].sort();
// a → x → b → y → c → z → d (linear chain through assets)
const chain = () =>
graph({
scripts: ["a", "b", "c", "d"],
writes: [
["a", "x"],
["b", "y"],
["c", "z"],
],
subs: [
["b", "x"],
["c", "y"],
["d", "z"],
],
});
test("boundedSet stops at a single end node", () => {
const res = boundedSet(buildLineageDag(chain()), sn("a"), [sn("c")]);
expect(sorted(scriptsOf(res.nodes))).toEqual(["a", "b", "c"]);
expect(res.nodes.has("datatable:z")).toBe(false);
});
test("boundedSet supports an asset as the end bound", () => {
const res = boundedSet(buildLineageDag(chain()), sn("a"), ["datatable:y"]);
expect(sorted(scriptsOf(res.nodes))).toEqual(["a", "b"]);
});
test("boundedSet drops ends not downstream of start", () => {
const res = boundedSet(buildLineageDag(chain()), sn("c"), [sn("a")]);
expect(res.droppedEnds).toEqual([sn("a")]);
expect([...res.nodes]).toEqual([sn("c")]);
});
test("validStarts: schedule and manual roots, not events/uploads/webhooks or subscribers", () => {
const g = graph({
scripts: ["a", "sub", "sched", "kfk", "upl", "hook"],
writes: [["a", "x"]],
subs: [["sub", "x"], ["sched", "x"]],
native: [
["schedule", "sched"],
["kafka", "kfk"],
["data_upload", "upl"],
["webhook", "hook"],
],
});
const starts = validStarts(g);
expect(starts.has(sn("a"))).toBe(true); // manual root
expect(starts.has(sn("sched"))).toBe(true); // schedule overrides subscriber
expect(starts.has(sn("sub"))).toBe(false); // pure subscriber
expect(starts.has(sn("kfk"))).toBe(false); // event-only
// data_upload / webhook need caller-supplied input → not auto-run roots
expect(starts.has(sn("upl"))).toBe(false);
expect(starts.has(sn("hook"))).toBe(false);
});
test("a scheduled root with a secondary data_upload trigger stays a start and isn't cut", () => {
// Regression: the barrier set must exclude valid starts, else a script with
// both `// on schedule` and `// on data_upload` resolves as the start yet is
// also a barrier, so reachableCutting skips it → empty run plan.
const g = graph({
scripts: ["sched_upload", "consumer"],
writes: [["sched_upload", "x"]],
subs: [["consumer", "x"]],
native: [
["schedule", "sched_upload"],
["data_upload", "sched_upload"],
],
});
const starts = validStarts(g);
expect(starts.has(sn("sched_upload"))).toBe(true); // schedule wins over the upload trigger
// Mirror run()'s barrier set: nonAutorun handlers minus the valid starts.
const barriers = new Set([...nonAutorunTriggerScripts(g)].filter((id) => !starts.has(id)));
expect(barriers.has(sn("sched_upload"))).toBe(false); // the scheduled root is protected
const sel = new Set(
[...reachableCutting(buildLineageDag(g), starts, barriers)]
.filter(isScriptNode)
.map(scriptPathOf),
);
expect(sel.has("sched_upload")).toBe(true); // runs on its schedule path…
expect(sel.has("consumer")).toBe(true); // …and its downstream isn't cut off
});
test("validFromStarts: mid-DAG models are eligible starts, event/upload/webhook are not", () => {
// a(root) → x → sub → y → reader ; k=kafka, upl=data_upload, hook=webhook.
const g = graph({
scripts: ["a", "sub", "reader", "k", "upl", "hook"],
writes: [["a", "x"], ["sub", "y"]],
reads: [["reader", "y"]],
subs: [["sub", "x"]],
native: [["kafka", "k"], ["data_upload", "upl"], ["webhook", "hook"]],
});
const from = validFromStarts(g);
expect(from.has(sn("a"))).toBe(true); // root
expect(from.has(sn("sub"))).toBe(true); // mid-DAG subscriber — NOT a validStart
expect(from.has(sn("reader"))).toBe(true); // pure reader
expect(validStarts(g).has(sn("sub"))).toBe(false); // old root-only gate rejected it
// Non-autorun handlers stay out (they need caller input / fan out per event).
expect(from.has(sn("k"))).toBe(false);
expect(from.has(sn("upl"))).toBe(false);
expect(from.has(sn("hook"))).toBe(false);
});
test("validFromStarts keeps a scheduled root that also carries a non-autorun trigger", () => {
// Regression: `--from sched_upload` must be accepted just like the implicit
// start. `sched_upload` is a scheduled root AND a data_upload handler — schedule
// wins in validStarts, so it stays --from-eligible despite being a nonAutorun
// handler (else explicit --from throws where the implicit start succeeds).
const g = graph({
scripts: ["sched_upload", "consumer"],
writes: [["sched_upload", "x"]],
subs: [["consumer", "x"]],
native: [["schedule", "sched_upload"], ["data_upload", "sched_upload"]],
});
expect(validStarts(g).has(sn("sched_upload"))).toBe(true);
expect(nonAutorunTriggerScripts(g).has(sn("sched_upload"))).toBe(true);
expect(validFromStarts(g).has(sn("sched_upload"))).toBe(true); // union with roots
});
test("a mid-DAG start runs itself + downstream, never upstream", () => {
// Starting at `sub`, the unbounded downstream is {sub, reader}; `a`/`x` upstream
// are never pulled in (dbt `--select sub+`).
const g = graph({
scripts: ["a", "sub", "reader"],
writes: [["a", "x"], ["sub", "y"]],
reads: [["reader", "y"]],
subs: [["sub", "x"]],
});
const dag = buildLineageDag(g);
const downstream = new Set([sn("sub"), ...descendants(dag, sn("sub"))]);
expect(sorted(scriptsOf(downstream))).toEqual(["reader", "sub"]);
expect(downstream.has(sn("a"))).toBe(false);
expect(downstream.has("datatable:x")).toBe(false);
});
test("nonAutorunTriggerScripts: event + upload/webhook handlers, incl. subscribers (descendants)", () => {
const g = graph({
scripts: ["a", "evt", "upl"],
writes: [["a", "x"]],
subs: [["evt", "x"], ["upl", "x"]], // both are lineage descendants of a…
native: [["kafka", "evt"], ["data_upload", "upl"]], // …and input-requiring handlers
});
const ev = nonAutorunTriggerScripts(g);
// excluded from a whole-pipeline run despite being descendants
expect(ev.has(sn("evt"))).toBe(true);
expect(ev.has(sn("upl"))).toBe(true);
expect(ev.has(sn("a"))).toBe(false);
});
test("reachableCutting: drops barrier + its exclusive downstream, keeps alt-path nodes", () => {
// root_a → x → handler(kafka) → y → consumer ; root_b → z ; (variant: consumer also ← z)
const g = graph({
scripts: ["root_a", "root_b", "handler", "consumer"],
writes: [["root_a", "x"], ["root_b", "z"], ["handler", "y"]],
subs: [["handler", "x"], ["consumer", "y"]],
native: [["kafka", "handler"]],
});
const dag = buildLineageDag(g);
const scripts = (s: Set<string>) =>
new Set([...s].filter(isScriptNode).map(scriptPathOf));
// consumer only reachable via the event handler → both excluded
expect(scripts(reachableCutting(dag, validStarts(g), nonAutorunTriggerScripts(g)))).toEqual(
new Set(["root_a", "root_b"]),
);
// now consumer also reads z (a non-event path via root_b) → it stays
const g2 = graph({
scripts: ["root_a", "root_b", "handler", "consumer"],
writes: [["root_a", "x"], ["root_b", "z"], ["handler", "y"]],
subs: [["handler", "x"], ["consumer", "y"], ["consumer", "z"]],
native: [["kafka", "handler"]],
});
expect(
scripts(reachableCutting(buildLineageDag(g2), validStarts(g2), nonAutorunTriggerScripts(g2))),
).toEqual(new Set(["root_a", "root_b", "consumer"]));
});
test("assetUriToNodeId maps s3 → s3object, others verbatim", () => {
expect(assetUriToNodeId("s3://b/k")).toBe("s3object:b/k");
expect(assetUriToNodeId("datatable://main/users")).toBe("datatable:main/users");
expect(assetUriToNodeId("nope")).toBe(undefined);
});
test("assetUriToNodeId strips leading slashes from S3 keys (canonical node)", () => {
// Mirror of Rust `parse_asset_syntax`: `--to s3:///exports/x` must resolve to
// the same canonical node as the graph's `s3object:exports/x`.
expect(assetUriToNodeId("s3:///exports/x")).toBe("s3object:exports/x");
expect(assetUriToNodeId("s3:///exports/x")).toBe(assetUriToNodeId("s3://exports/x"));
// All leading slashes stripped so a canonical key never starts with `/`
// (the quad-slash `S3Object(s3="/x")` form collapses to `x`).
expect(assetUriToNodeId("s3:////x")).toBe("s3object:x");
// Hive-partition keys and non-S3 kinds are untouched.
expect(assetUriToNodeId("s3:///t/y=2024/f.parquet")).toBe("s3object:t/y=2024/f.parquet");
});
test("resolveToken: short name, full path, and asset URI", () => {
const g = graph({ scripts: ["f/p/stage"], writes: [["f/p/stage", "main/staged"]] });
g.assets = [{ kind: "datatable", path: "main/staged" }];
expect(resolveToken(g, "stage")).toBe(sn("f/p/stage"));
expect(resolveToken(g, "f/p/stage")).toBe(sn("f/p/stage"));
expect(resolveToken(g, "datatable://main/staged")).toBe("datatable:main/staged");
expect(resolveToken(g, "missing")).toBe(undefined);
});
test("topoOrder sorts the bounded scripts and flags cycles", () => {
const { order, cyclic } = topoOrder(chain(), new Set(["a", "b", "c"]));
expect(order).toEqual(["a", "b", "c"]);
expect(cyclic).toEqual([]);
});
test("descendants/ancestors exclude the start even on a cycle", () => {
// a → x → b → y → a (cycle): closures must not contain a.
const g = graph({
scripts: ["a", "b"],
writes: [["a", "x"], ["b", "y"]],
subs: [["b", "x"], ["a", "y"]],
});
const dag = buildLineageDag(g);
expect(descendants(dag, sn("a")).has(sn("a"))).toBe(false);
expect(ancestors(dag, sn("a")).has(sn("a"))).toBe(false);
});
test("topoOrder orders a pure reader after its producer", () => {
// a writes x; c only *reads* x (no `// on x`). c must run after a.
const g = graph({
scripts: ["a", "c"],
writes: [["a", "x"]],
reads: [["c", "x"]],
});
const { order } = topoOrder(g, new Set(["a", "c"]));
expect(order).toEqual(["a", "c"]);
});