From f93eaf66cf6f62f32855fc6964d9c1c577e8cd99 Mon Sep 17 00:00:00 2001 From: Brian Kane Date: Tue, 6 Oct 2026 22:41:12 +0100 Subject: [PATCH] Fix: order a provider call without $params after the calls it reads The resolver found what a call depends on by following the references in its $params. A call written in the legacy style has no $params: its inputs are its own top-level fields. It was therefore taken to depend on nothing, ran in the first level, and read another call's output before that call had run. kubevela/workflow's request step, whose legacy op.#ConditionalWait reads req.$returns, waited for ever. A call's inputs are now its $params, or for a legacy call each of its top-level fields, hidden ones included. The fields rather than the call's own expression: that is the conjunction with the definition the call is made from, and following it reaches into the package the definition lives in. kubevela/workflow's legacy op package declares NoExist: _|_ at the top level, and reading through it lost the dependency and left the reread call bottom. The regression test's package declares one too, and each of its fixtures puts the reader before what it reads, so traversal order alone cannot pass it. Fixes #148 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014vkPtmKeCEZo2EL7QWCMhH Signed-off-by: Brian Kane --- cue/cuex/compiler.go | 38 ++++-- cue/cuex/legacy_call_order_test.go | 186 +++++++++++++++++++++++++++++ 2 files changed, 217 insertions(+), 7 deletions(-) create mode 100644 cue/cuex/legacy_call_order_test.go diff --git a/cue/cuex/compiler.go b/cue/cuex/compiler.go index 0488a34..438e26c 100644 --- a/cue/cuex/compiler.go +++ b/cue/cuex/compiler.go @@ -1490,8 +1490,9 @@ func ready(call pendingCall, pending []pendingCall, stillPending map[string]bool } // paramsResolved reports whether a call's parameters have a value yet. A call -// with no parameters at all has nothing to wait for; what the provider makes -// of that is its own business, as it always was. +// with no $params has nothing to check here: a legacy call's top-level fields +// hold its outputs as well as its inputs, and those stay unset until it runs, so +// it is ordered by the calls it reads (callDependencies) alone. // // A NativeProviderFn is handed the parameters as a cue.Value and may mean to // take them unresolved - a schema, or an open disjunction. Such a call is @@ -1537,16 +1538,17 @@ func waitsForPeer(call pendingCall, pending []pendingCall, stillPending map[stri // References are therefore followed through fields that are not themselves // calls, and stop at ones that are - what a call holds is that call's business // and reading into it here means reading whatever it has rendered. +// +// A call with no $params is written in the legacy style: its inputs are its own +// top-level fields, so those are what it reads. +// +// wait: op.#ConditionalWait & {continue: req.$returns != _|_} func callDependencies(call pendingCall, pending []pendingCall, executed map[string]bool) []string { if len(pending) < 2 { return nil // no peer to read, so nothing to read from one } - params := call.value.LookupPath(paramsPath) - if !params.Exists() { - return nil - } var queue []reference - if !collectReferences(params, &queue, 0) { + if !collectInputReferences(call.value, &queue) { return peerKeys(call, pending) } waits := map[string]bool{} @@ -1605,6 +1607,28 @@ func callDependencies(call pendingCall, pending []pendingCall, executed map[stri return keys } +// collectInputReferences collects the references a call's inputs make: its +// $params, or for a legacy call with none, each of its own top-level fields, +// hidden ones included, since a provider may read those too. The +// fields and not the call's expression, which is the conjunction with the +// definition the call is made from, and reading into that definition reads the +// package it lives in rather than anything the call was given. +func collectInputReferences(call cue.Value, into *[]reference) bool { + if params := call.LookupPath(paramsPath); params.Exists() { + return collectReferences(params, into, 0) + } + it, err := call.Fields(cue.Optional(true), cue.Hidden(true)) + if err != nil { + return false + } + for it.Next() { + if !collectReferences(it.Value(), into, 1) { + return false + } + } + return true +} + // reference is a path some value reads, kept with the root it is relative to // so it can be looked up again. type reference struct { diff --git a/cue/cuex/legacy_call_order_test.go b/cue/cuex/legacy_call_order_test.go new file mode 100644 index 0000000..9a48502 --- /dev/null +++ b/cue/cuex/legacy_call_order_test.go @@ -0,0 +1,186 @@ +/* +Copyright 2026 The KubeVela Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cuex_test + +import ( + "context" + "encoding/json" + "strings" + "sync" + "testing" + + "cuelang.org/go/cue" + "github.com/stretchr/testify/require" + + "github.com/kubevela/pkg/cue/cuex" + cuexruntime "github.com/kubevela/pkg/cue/cuex/runtime" +) + +// deepNesting is past cuex's maxReferenceDepth (100), the depth it looks for +// references to; a test reaching it must nest deeper than that bound. +const deepNesting = 110 + +// legacyFn is a provider in the shape kubevela/workflow's legacy providers take: it +// is handed the whole call, with its inputs at the top level and no $params. +type legacyFn func(value cue.Value) (cue.Value, error) + +func (fn legacyFn) Call(_ context.Context, value cue.Value) (cue.Value, error) { + return fn(value) +} + +type seqParams struct { + Params struct { + V int `json:"v"` + } `json:"$params"` +} + +type seqReturns struct { + Returns struct { + V int `json:"v"` + } `json:"$returns"` +} + +// seqCompiler offers a new-style call (#New, $params to $returns), a legacy call that +// produces output on its own fields (#Make), and legacy calls recording what their +// input held when they ran: `ready` (#Check), the hidden `_ready` (#CheckHidden), or +// a leaf nested deepNesting levels down (#CheckDeep). The package also declares a top-level +// bottom, as kubevela/workflow's legacy op package does (NoExist: _|_), which is +// what a call's inputs must be read without reaching into. +func seqCompiler(t *testing.T, seen *[]bool) *cuex.Compiler { + var mu sync.Mutex + pkg, err := cuexruntime.NewInternalPackage("seq", ` +package seq + +#New: { + #do: "new" + #provider: "seq" + $params: v: int + $returns?: v: int +} + +#Make: { + #do: "make" + #provider: "seq" + value?: {...} +} + +#Check: { + #do: "check" + #provider: "seq" + ready: *false | bool +} + +#CheckHidden: { + #do: "check-hidden" + #provider: "seq" + ... +} + +#CheckDeep: { + #do: "check-deep" + #provider: "seq" + ... +} + +NoExist: _|_`, map[string]cuexruntime.ProviderFn{ + "new": cuexruntime.GenericProviderFn[seqParams, seqReturns](func(_ context.Context, in *seqParams) (*seqReturns, error) { + out := &seqReturns{} + out.Returns.V = in.Params.V + return out, nil + }), + // Reads its input from the hidden field _ready of the template's own package. + "check-hidden": legacyFn(func(value cue.Value) (cue.Value, error) { + ready, _ := value.LookupPath(cue.MakePath(cue.Hid("_ready", "_"))).Bool() + mu.Lock() + *seen = append(*seen, ready) + mu.Unlock() + return value, nil + }), + // Reports whether the leaf deepNesting levels down under deep is resolved. + "check-deep": legacyFn(func(value cue.Value) (cue.Value, error) { + leaf := value.LookupPath(cue.ParsePath("deep" + strings.Repeat(".a", deepNesting))) + _, err := leaf.Int64() + mu.Lock() + *seen = append(*seen, err == nil) + mu.Unlock() + return value, nil + }), + "make": legacyFn(func(value cue.Value) (cue.Value, error) { + return value.FillPath(cue.ParsePath("value"), map[string]any{"v": 1}), nil + }), + // Decoded as kubevela/workflow's legacy providers decode their call: the + // whole value, marshalled to JSON. + "check": legacyFn(func(value cue.Value) (cue.Value, error) { + bs, err := value.MarshalJSON() + if err != nil { + return value, err + } + var in struct { + Ready bool `json:"ready"` + } + if err := json.Unmarshal(bs, &in); err != nil { + return value, err + } + mu.Lock() + *seen = append(*seen, in.Ready) + mu.Unlock() + return value, nil + }), + }) + require.NoError(t, err) + return cuex.NewCompilerWithInternalPackages(pkg) +} + +// A call with no $params reads its inputs from its top-level fields, so those are what +// it depends on: it must run after the calls they read, not alongside them. +func TestLegacyCallWaitsForTheCallItReads(t *testing.T) { + // Each reader comes before what it reads, so traversal order alone cannot pass. + cases := map[string]string{ + // workflow's request: op.#ConditionalWait & {continue: req.$returns != _|_} + "a legacy call reading a new-style call's $returns": ` +import "vela/seq" +wait: seq.#Check & {ready: req.$returns != _|_} +req: seq.#New & {$params: v: 1} +`, + // workflow's apply-job: a legacy wait reading what a legacy apply wrote + "a legacy call reading another legacy call's output": ` +import "vela/seq" +wait: seq.#Check & {ready: apply.value != _|_} +apply: seq.#Make & {} +`, + "a legacy call reading through a hidden field": ` +import "vela/seq" +wait: seq.#CheckHidden & {_ready: req.$returns != _|_} +req: seq.#New & {$params: v: 1} +`, + // Its only reference sits past the depth references are looked for, so it is + // never found: the call must wait on every peer to see it resolved. + "a legacy call nested too deep to read": ` +import "vela/seq" +wait: seq.#CheckDeep & {deep: ` + strings.Repeat("{a: ", deepNesting) + "req.$returns.v" + strings.Repeat("}", deepNesting) + `} +req: seq.#New & {$params: v: 1} +`, + } + for name, src := range cases { + t.Run(name, func(t *testing.T) { + var seen []bool + _, err := seqCompiler(t, &seen).CompileString(context.Background(), src) + require.NoError(t, err) + require.Equal(t, []bool{true}, seen, "the check must run once, after what it reads") + }) + } +}