Fix: keep provider calls in template order by briankane · Pull Request #153 · kubevela/pkg

briankane · 2026-10-08T17:41:43Z

Description of your changes

Follows #149. Since #141, provider calls can run out of the order a template writes them. That breaks 9 of kubevela's shipped workflow steps (check-metrics, vela-cli, request, the cloud-resource steps and others): typically a step's wait runs before a #Fail or #Apply written ahead of it, and ends the step first.

Calls now run in template order, unless one reads another:

A round ends after a call whose result can make a new call appear, such as a guard on the result of a #Fail. Which calls can do that is read from the template's syntax and the packages it imports. Every other call still runs in batches.
A call only moves later for a call it actually reads. A call waiting on a result holds back the calls written after it, except the ones it is waiting for.
Working out what a call reads uses a separate copy of the template, in its own CUE context. Doing it on the live value left a later result coming back bottom (share-cloud-resource).

How has this code been tested

New tests for each ordering case, the reveal analysis, batching, and a fixture reduced from workflow's legacy op package. Each ordering case fails on main, and the fixture fails without the separate copy.
TestResolveMatchesTheOneAtATimeOrder generates templates (guards, loops, computed labels, lets, nesting, lists, a package definition, native and Go results, a call that ends the resolve) and requires the same calls, order and output as the resolver before cue/cuex: resolve a template's provider calls in batches, not one walk each #141. It runs 400 in CI, and has passed 5,000 locally. It found one more bug: a read through a let, which CUE renders as concrete before it has a value.
All pkg packages pass, on Go 1.27.1 and on Go 1.23.8.
kubevela's ./pkg/... passes against this, including every shipped definition test. kubevela/workflow's unit tests show no new failures.
BenchmarkRealWorld against main: time is within noise, except the largest pure-provider fanout (17 to 27 ms), and memory is 8 to 16% higher, both from the separate copy.

Special notes for your reviewer

TestCallsRunBeforeAFailureIsReported pinned cue/cuex: resolve a template's provider calls in batches, not one walk each #141's reordering past a call with unresolved params. TestAFailureStopsTheCallsWrittenAfterIt replaces it and asserts the pre-cue/cuex: resolve a template's provider calls in batches, not one walk each #141 behaviour.
kubevela pins kubevela/pkg from before Go 1.27, so the same change also goes to a maintenance branch cut at fcae6f4.

…ad away from where they run Since kubevela#141 a resolve runs every ready call it has found before searching the value again. That broke workflow steps three ways, all visible in kubevela's shipped workflow step definitions (check-metrics, request, restart-workflow, vela-cli, build-push-image, apply-terraform-config/-provider, deploy-cloud-resource, share-cloud-resource): - A call a comprehension or a computed label writes once an earlier call has answered ran after the calls that were there all along. A step writes a #Fail guarded on a check ahead of a wait, and the wait ends the step, so the fail never ran. - A call reading a default waited for every other call, and one whose parameters were not concrete waited for every call still to run, so a call written after it, such as a wait, ran first. - Working out what a call reads evaluates its inputs. Done in the value the providers are given, it left a result filled in afterwards coming back bottom (kubevela/workflow's legacy op package), and the calls after it were never found. Calls now run in the order the template writes them unless one reads another: - A round ends after a call that can reveal another, read from the syntax of the template and the packages it imports (a comprehension or computed label that can produce a call, reading the call's result directly or through the fields that do), and after a result a function built itself. Calls that reveal nothing still share a round. - A call moves later only for a call it reads, or for every call where the reference search gives up. A call reading a default, or with parameters that are not concrete, waits only for the calls written ahead of it. - What a call reads is worked out against a second build of the template in a CUE context of its own, built only when a call has a peer to read. The test reduced from the legacy op package fails without the separate build; the order tests fail on kubevela#141. kubevela/pkg's suites, kubevela's ./pkg/... and kubevela/workflow's unit suites pass against this. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aMhPL2ehfzoVXwT7rSX2m Signed-off-by: Brian Kane <briankane1@gmail.com>

cubic-dev-ai · 2026-10-08T17:42:04Z

We detected this is a high-risk PR and are running a free ultrareview. An ultrareview is a deeper, multi-pass review that catches hard-to-find bugs a standard review can miss. We'll post the findings when it completes.

This PR appears to move logic across many files, where a missed bug can hide in interactions a single-pass review does not trace, so a deeper multi-pass review is worth running.

Want an ultrareview on every high-risk PR? Set up automated ultrareviews.

codecov · 2026-10-08T17:46:42Z

Codecov Report

❌ Patch coverage is 96.86411% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.56%. Comparing base (567282f) to head (3f0ab3a).

Files with missing lines	Patch %	Lines
cue/cuex/compiler.go	94.64%	6 Missing ⚠️
cue/cuex/reveal.go	98.28%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
+ Coverage   92.40%   92.56%   +0.16%     
==========================================
  Files         114      115       +1     
  Lines        6659     6930     +271     
==========================================
+ Hits         6153     6415     +262     
- Misses        506      515       +9

Flag	Coverage Δ
unit-test	`92.56% <96.86%> (+0.16%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

cubic-dev-ai

We detected this is a high-risk PR and ran a free ultrareview. An ultrareview is a deeper, multi-pass review that catches hard-to-find bugs a standard review can miss.

This PR appears to move logic across many files, where a missed bug can hide in interactions a single-pass review does not trace, so a deeper multi-pass review is worth running.

Want an ultrareview on every high-risk PR? Set up automated ultrareviews.

All reported issues were addressed across 10 files

_{Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic}

A comprehension or computed label whose body copies a call from a let or another field, rather than writing it, can still produce a call, so the call its clauses read reveals one. Whether a body can hold a call is now also answered by name, through the fields and lets that hold one. The analysis fixture is reduced again, at every offset: every line of CUE in it is needed to reproduce. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aMhPL2ehfzoVXwT7rSX2m Signed-off-by: Brian Kane <briankane1@gmail.com>

cubic-dev-ai

All reported issues were addressed across 4 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic}

…d keep a held call ahead of the calls after it Which calls can reveal another counted field labels and selector names as reads. Every call has a $params and a $returns, so those names tied every call to every other, and most templates searched the value after every call. Only the references an expression reads count now, and the closure stops at a definition, which is a schema. That made the next case visible: a call waiting for a result was overtaken by a later call that read nothing, such as a wait, which ends the step first. A call held back now holds back the calls written after it, except those it is waiting for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aMhPL2ehfzoVXwT7rSX2m Signed-off-by: Brian Kane <briankane1@gmail.com>

cubic-dev-ai

All reported issues were addressed across 4 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic}

…definitions that name a result What may run ahead of a held call is now worked out from the first held call alone, and transitively: a chain written out of order (a waits for c, which waits for b) runs b, c, a rather than falling back to running a first, and a producer for a later held call no longer overtakes an earlier one. Which calls reveal another no longer stops at a definition: a definition can name a result (#result: check.$returns) that a guard reads, and stopping there under-approximated, which is the one direction this must not err. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aMhPL2ehfzoVXwT7rSX2m Signed-off-by: Brian Kane <briankane1@gmail.com>

…st the one-at-a-time resolver A call reading a result through a let ran alongside the call it read, with the result missing: the reference search cannot follow a let, and CUE renders an interpolation of a let with no value yet as concrete. A value written as a name with no path and no value yet now counts as a read the search cannot name, and the call waits for the calls written ahead of it. TestResolveMatchesTheOneAtATimeOrder generates templates whose reads are all of results written earlier, mixing guards, loops, computed labels, lets, named results, nesting, lists of calls, a package definition with its own guard, native and Go results and a call that ends the resolve, and requires the same calls in the same order, and the same result, as the resolver this replaced. It runs 400 by default; CUEX_DIFFERENTIAL_CASES runs more. It found the let. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aMhPL2ehfzoVXwT7rSX2m Signed-off-by: Brian Kane <briankane1@gmail.com>

cubic-dev-ai

All reported issues were addressed across 3 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic}

…il it merges The CEL engine's pkg (fcae6f4) carries kubevela/pkg#141's provider call ordering bugs, which fail kubevela's shipped workflow step definitions. kubevela/pkg#153 fixes them; this points at the same fix on fcae6f4, kept off Go 1.27, until it lands upstream. Not to be merged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aMhPL2ehfzoVXwT7rSX2m Signed-off-by: Brian Kane <briankane1@gmail.com>

…rors exactly A read through a let the reference search cannot follow is followed in the syntax instead: the let's expression, and what the fields it names read in turn, by name, to the calls it reaches, whether written before the reader or after it. Only where that names no call does the reader wait for the calls written ahead of it. The generated test compares errors exactly, except for the let an unreferenced-let error names, which CUE picks from several in no fixed order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aMhPL2ehfzoVXwT7rSX2m Signed-off-by: Brian Kane <briankane1@gmail.com>

cubic-dev-ai

1 issue found across 4 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic}

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="cue/cuex/compiler.go">

<violation number="1" location="cue/cuex/compiler.go:1759">
P1: The name closure is file-wide, so an unrelated nested field with the same name can make this call wait for its provider. `runLevel` can then run that later call before the call's actual producer, including a call that ends resolution; use scope- or path-aware edges here.</violation>
</file>

…il it merges The CEL engine's pkg (fcae6f4) carries kubevela/pkg#141's provider call ordering bugs, which fail kubevela's shipped workflow step definitions. kubevela/pkg#153 fixes them; this points at the same fix on fcae6f4, kept off Go 1.27, until it lands upstream. Not to be merged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aMhPL2ehfzoVXwT7rSX2m Signed-off-by: Brian Kane <briankane1@gmail.com>

Following a let by name tied it to any field sharing the name, and a call under such a field could then run ahead of the let's real producer, ending the resolve first. The syntax resolves each identifier to what it declares, so a let's reads are now followed to those fields' paths in the template and on through the reference search. Where a read resolves to something the template's syntax does not place, the reader waits for the calls written ahead of it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aMhPL2ehfzoVXwT7rSX2m Signed-off-by: Brian Kane <briankane1@gmail.com>

cubic-dev-ai

All reported issues were addressed across 3 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic}

…il it merges The CEL engine's pkg (fcae6f4) carries kubevela/pkg#141's provider call ordering bugs, which fail kubevela's shipped workflow step definitions. kubevela/pkg#153 fixes them; this points at the same fix on fcae6f4, kept off Go 1.27, until it lands upstream. Not to be merged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aMhPL2ehfzoVXwT7rSX2m Signed-off-by: Brian Kane <briankane1@gmail.com>

fieldPaths only went into a field whose value was a struct literal, so the fields of x: #D & {...}, the shape a definition's fields usually take, were not placed, and a let read among them fell back to waiting for the calls written ahead of it while its producer came later. It now goes into each struct literal a unification, a disjunction or brackets contribute. The generated templates add a let inside a struct unified with a definition. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aMhPL2ehfzoVXwT7rSX2m Signed-off-by: Brian Kane <briankane1@gmail.com>

…il it merges The CEL engine's pkg (fcae6f4) carries kubevela/pkg#141's provider call ordering bugs, which fail kubevela's shipped workflow step definitions. kubevela/pkg#153 fixes them; this points at the same fix on fcae6f4, kept off Go 1.27, until it lands upstream. Not to be merged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aMhPL2ehfzoVXwT7rSX2m Signed-off-by: Brian Kane <briankane1@gmail.com>

briankane requested review from FogDong, Somefive, anoop2811 and jguionnet as code owners October 8, 2026 17:41