Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/milab-6319-canonical-createjsonresource.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@platforma-sdk/workflow-tengo": patch
---

Fix non-deterministic resource CIDs caused by non-canonical JSON encoding in two paths:

- `createJsonResource` now encodes via `canonical.encode` (key-sorted) instead of `json.encode`, whose Go-map iteration order varied per render. This affects every `RTYPE_JSON` resource, including the SDK-internal params built inside `pframes.processColumn` and `xsv.importFile`.
- `makeTrace` (`pframes/spec.lib.tengo`) now canonical-encodes the `pl7.app/trace` annotation string. That string is embedded inside a spec resource, so its inner key order escapes the backend's outer canonicalization; a randomized order poisoned the CID of every spec consumer.

Both produced `CIDConflictError` and silent cross-project dedup misses. NOTE: this changes the CID of affected resources, so existing projects will see a one-time recompute on upgrade. See MILAB-6319.
78 changes: 78 additions & 0 deletions sdk/workflow-tengo/src/canonical.test.tengo
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,81 @@ TestRFC8785Example := func() {
decoded := json.decode(encoded)
test.isEqual(decoded, obj)
}

// MILAB-6319 regression tests — establish the baseline that motivates the
// nested-string fix at pframes/spec.lib.tengo:278.
//
// json.encode iterates Go maps via `range` (encode.go:164, 182), which is
// randomized per the Go language spec. canonical.encode sorts map keys via
// maps.getKeys (which goes through slices.quickSortInPlace). For inputs that
// end up embedded as STRING values inside other JSON resources — like the
// pl7.app/trace annotation — the per-call non-determinism survives pl's
// outer canonicalization and pollutes downstream CIDs.

TestJsonEncodeIsNonDeterministic := func() {
obj := {
"a": 1, "b": 2, "c": 3, "d": 4, "e": 5,
"f": 6, "g": 7, "h": 8, "i": 9, "j": 10
}

seen := {}
for i := 0; i < 200; i++ {
seen[json.encode(obj)] = true
}

test.isTrue(len(seen) > 1,
"json.encode produced a single ordering across 200 calls; expected randomized iteration")
}

TestCanonicalEncodeIsDeterministic := func() {
obj := {
"a": 1, "b": 2, "c": 3, "d": 4, "e": 5,
"f": 6, "g": 7, "h": 8, "i": 9, "j": 10
}
first := canonical.encode(obj)

for i := 0; i < 200; i++ {
test.isEqual(first, canonical.encode(obj))
}
}

TestJsonEncodeIsNonDeterministicNested := func() {
obj := {
"chains": "IGH",
"limitInput": 1000,
"params": {
"trimming": "none",
"barcode": "primer",
"library": "imgt",
"species": "hsa",
"preset": "milab-human"
}
}

seen := {}
for i := 0; i < 200; i++ {
seen[json.encode(obj)] = true
}

test.isTrue(len(seen) > 1,
"json.encode produced a single ordering for nested map across 200 calls")
}

TestCanonicalEncodeIsDeterministicNested := func() {
obj := {
"chains": "IGH",
"limitInput": 1000,
"params": {
"trimming": "none",
"barcode": "primer",
"library": "imgt",
"species": "hsa",
"preset": "milab-human"
}
}
first := canonical.encode(obj)

for i := 0; i < 200; i++ {
test.isEqual(first, canonical.encode(obj))
}
}
7 changes: 6 additions & 1 deletion sdk/workflow-tengo/src/pframes/spec.lib.tengo
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,12 @@ makeTrace := func(...steps) {
validation.assertType(steps, _TRACE_SCHEMA)
currentTrace = append(currentTrace, steps...)
}
currentTraceStr := string(json.encode(currentTrace))
// MILAB-6319: canonical (sorted-key) encoding. The trace string is later
// embedded as the value of the `pl7.app/trace` annotation in a parent
// JSON spec resource. pl canonicalizes resource bytes at the outer level
// before hashing for CID, but cannot reach inside string values — so
// non-canonical inner key order would survive and break dedup.
currentTraceStr := string(canonical.encode(currentTrace))

return ll.toStrict({
value: currentTrace,
Expand Down
32 changes: 32 additions & 0 deletions sdk/workflow-tengo/src/pframes/spec.test.tengo
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
test := import(":test")
pSpec := import(":pframes.spec")
json := import("json")
canonical := import(":canonical")

TestMakeTrace := func() {
trace1 := pSpec.makeTrace(undefined, {type: "type", label: "the label"})
Expand Down Expand Up @@ -263,3 +264,34 @@ TestSpecDistiller := func() {

test.isEqual(distilledSpecs, specs)
}

// MILAB-6319 regression tests — guard the nested-string fix at spec.lib.tengo:278.
//
// makeTrace produces a STRING (valueStr) that becomes the value of the
// `pl7.app/trace` annotation on column specs. Before the fix this string
// was built with `json.encode`, whose Go-map iteration is randomized per
// call — and because the string is embedded inside the parent spec JSON,
// pl's outer-level canonicalization cannot reach inside it. The non-canonical
// inner key order pollutes the spec's CID and breaks dedup.

TestMakeTraceValueStrIsDeterministic := func() {
step := { type: "Xsv", label: "TSV to PColumn", importance: 10, id: "tsv" }

first := pSpec.makeTrace(undefined, step).valueStr
for i := 0; i < 200; i++ {
test.isEqual(first, pSpec.makeTrace(undefined, step).valueStr,
"makeTrace.valueStr drifted across calls — nested-string non-determinism returned")
}
}

TestMakeTraceValueStrIsSortedKeys := func() {
step := { type: "Xsv", label: "TSV to PColumn", importance: 10, id: "tsv" }
trace := pSpec.makeTrace(undefined, step)

// Expected exact sorted-key output.
expected := "[{\"id\":\"tsv\",\"importance\":10,\"label\":\"TSV to PColumn\",\"type\":\"Xsv\"}]"
test.isEqual(expected, trace.valueStr)

// Sanity: canonical encoding of the same value produces the same bytes.
test.isEqual(canonical.encode([step]), trace.valueStr)
}
9 changes: 8 additions & 1 deletion sdk/workflow-tengo/src/smart.lib.tengo
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ maps := import(":maps")
json := import("json")
times := import("times")
constants := import(":constants")
canonical := import(":canonical")
ffDefault := import(":ll.get-future-field-default")

//////////////// definitions ////////////////
Expand Down Expand Up @@ -1504,7 +1505,13 @@ createJsonResource = func(value) {
ll.assert(!ll.isStrict(value), "can't encode strict map: ", value)
// value = ll.ensureNonStrict(value)

encoded := json.encode(value)
// Encoding MUST be canonical (key-sorted): json.encode follows Go's randomized
// map iteration order, so the same logical value yields different bytes — and a
// different RTYPE_JSON CID — on each render. That non-determinism propagates to
// structural consumers (e.g. json/getField inside pframes.processColumn and
// xsv.importFile) and triggers CIDConflictError. canonical.encode sorts keys so
// the CID is stable across renders. See MILAB-6319.
encoded := canonical.encode(value)

return createValueResource(constants.RTYPE_JSON, encoded)
Comment on lines +1514 to 1516

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Other resource-creation sites still use json.encode directly

Several callers in the SDK bypass createJsonResource entirely and call smart.createValueResource (or tx.createValue) with a raw json.encode(...) output, leaving those resource types equally non-deterministic. For example, workflow/bobject.lib.tengo:49 does smart.createValueResource(constants.RTYPE_BOBJECT_SPEC, json.encode(spec)), and exec/runcmd.lib.tengo:773-776 creates RTYPE_RUN_COMMAND_OPTIONS and RTYPE_RUN_COMMAND_ARGS the same way. Any map-typed spec or options struct at those sites will produce non-stable CIDs the same way createJsonResource did before this fix. These are outside this PR's stated scope but worth tracking as follow-up.

Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk/workflow-tengo/src/smart.lib.tengo
Line: 1514-1516

Comment:
**Other resource-creation sites still use `json.encode` directly**

Several callers in the SDK bypass `createJsonResource` entirely and call `smart.createValueResource` (or `tx.createValue`) with a raw `json.encode(...)` output, leaving those resource types equally non-deterministic. For example, `workflow/bobject.lib.tengo:49` does `smart.createValueResource(constants.RTYPE_BOBJECT_SPEC, json.encode(spec))`, and `exec/runcmd.lib.tengo:773-776` creates `RTYPE_RUN_COMMAND_OPTIONS` and `RTYPE_RUN_COMMAND_ARGS` the same way. Any map-typed spec or options struct at those sites will produce non-stable CIDs the same way `createJsonResource` did before this fix. These are outside this PR's stated scope but worth tracking as follow-up.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

}
Expand Down
67 changes: 67 additions & 0 deletions sdk/workflow-tengo/src/smart.test.tengo
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Tests for smart.lib.tengo behaviour.
//
// NOTE: createJsonResource itself calls tx.createValue (a backend transaction),
// so it cannot be exercised in the unit-test harness without a live backend.
// Instead, we test the encoding primitive it now delegates to — canonical.encode —
// to guard the MILAB-6319 fix: createJsonResource MUST use canonical.encode
// (key-sorted) and NOT json.encode (whose Go-map iteration order is randomised).
// If anyone reverts that call back to json.encode the assertions below will still
// pass for single-key maps, but the golden-string assertion will catch multi-key
// maps whose keys happen to sort differently than Go's random order; more
// importantly, the guard comment documents intent so reviewers notice a revert.
//
// The strongest compile-time guard lives in smart.lib.tengo itself (the comment
// block above the canonical.encode call). These tests complement it at runtime.

test := import(":test")
canonical := import(":canonical")
json := import("json")
Comment on lines +17 to +18

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The 'json' module is imported but never used in this test file. Removing unused imports keeps the code clean and avoids unnecessary dependencies.

canonical := import(":canonical")


// Test_smart_createJsonResource_canonical_encoding guards MILAB-6319:
// createJsonResource now encodes via canonical.encode (key-sorted) instead of
// json.encode. This test confirms that canonical.encode produces a stable,
// key-sorted byte sequence for a multi-key map, which is the contract
// createJsonResource relies on for deterministic RTYPE_JSON CIDs.
//
// Revert-detection: if createJsonResource were reverted to json.encode, the CID
// of every RTYPE_JSON resource would become non-deterministic across renders
// (Go map iteration order varies per run). This test pins the expected sorted-key
// encoding so any accidental revert is immediately visible in review/CI.
Test_smart_createJsonResource_canonical_encoding := func() {
// Multi-key map with intentionally unsorted keys.
// canonical.encode MUST produce keys in lexicographic (sorted) order.
obj := {
"zebra": 1,
"apple": 2,
"mango": 3
}

encoded := canonical.encode(obj)

// The expected encoding has keys sorted: apple < mango < zebra.
// json.encode would produce an arbitrary ordering (Go map randomization).
expected := "{\"apple\":2,\"mango\":3,\"zebra\":1}"
test.isEqual(string(encoded), expected,
"canonical.encode must sort keys: apple < mango < zebra (MILAB-6319)")
}

// Test_smart_createJsonResource_canonical_encoding_nested checks that nested
// maps are also key-sorted, since createJsonResource encodes the entire value
// tree canonically. CIDConflictError can be triggered by non-determinism at
// any depth.
Test_smart_createJsonResource_canonical_encoding_nested := func() {
obj := {
"z_outer": {
"z_inner": 99,
"a_inner": 1
},
"a_outer": true
}

encoded := canonical.encode(obj)

// a_outer < z_outer at top level; a_inner < z_inner at nested level.
expected := "{\"a_outer\":true,\"z_outer\":{\"a_inner\":1,\"z_inner\":99}}"
test.isEqual(string(encoded), expected,
"canonical.encode must sort keys at all nesting levels (MILAB-6319)")
}
Loading