Skip to content

The evaluator, audited against cel-go's own machinery: planner, activation, attributes, decorators, frame, cost and memory — the slate #1489

Open
10 of 26 issues completed

Description

@picatz

Written against 40cc365 (2026-09-02, merge of #1424) and against cel-go at the pin (github.com/google/cel-go v0.31.0), the v0.32.0 tag, and upstream master at f8478f1. Third audit beside #1459 (how a value's type travels) and #1421 (how a name resolves): this one reads how an expression is planned and executed — cel-go's interpreter package (the planner, Activation, Attribute/Qualifier, InterpretableDecorator, ExecutionFrame, the cost and memory trackers) against the one evaluator in pkg/flowstate/v1/celenv.go and the activation in eval.go — and what moved upstream in August 2026. Every claim in the sub-issues cites file:line at that revision, a go doc result against the pinned module, or a measurement reproduced in this pass.

What holds

One builder (buildEnv, celenv.go:748), one program cache, one cost estimator installed where both drivers build a program (Limits.programOptions, celenv.go:80), one custom interpreter.Activation both drivers fill through Scope.Activation (nodes.go:93), and a profile pinned per run that every evaluation site resolves from the spec. The ordered-map rewrite (celmap.go) is the one evaluator-side rule and it is applied as an AST rewrite before planning, which keeps both drivers and the stored specification honest. ExecutionFrame (interpreter/frame.go), ConcurrentEval and async functions are already in the pinned version and correctly unused: Flowstate's cancellation contract is ContextEval plus InterruptCheckFrequency, and an async function would be I/O inside workflow code.

What the audit found

The slate, in the order the pass recommends

Independent, landable now (no upstream change needed):

  1. eval: one steps.<id>.<output> read converts every prior step's every output into a CEL value — 21 ms and 24 MB for one field at 1,000 steps, linear in what the run has accumulated, on both drivers #1482 — lazy steps root: a bounded-work defect on both drivers, measured.
  2. cel: both drivers plan parsed ASTs against a variable-less environment — every steps.x.y is a three-candidate maybe-attribute, every call carries an empty overload id, and cel-go's overload-aware costing, plan-time regex compilation and static cost estimation are unreachable #1481 — check before plan with dyn declarations only; deletes the debugger's copy of envDeclaring.
  3. cel: Limits is two knobs (cost, interrupt frequency); the pinned cel-go exposes nine bounds and the five policy compilers set three of their own — decide each bound once, including the memory bound no site has #1483 — decide the eight bounds that exist at the pin; route the five policy sites through Limits.
  4. cel: every evaluation site discards EvalDetails — actual cost is measured on every expression and reported nowhere, and OptTrackState would give flow policy explain, DAP evaluate and check witnesses a real per-node trace instead of a re-evaluation #1485 — keep EvalDetails: cost into spans and metrics, one redacted per-node trace for the explain surfaces.
  5. cel: the profile is a Go string slice and a generated markdown table; cel-go's Env.ToConfig renders the same environment as a YAML env.Config — pin 2026.1 as data, feed it to the REPL and the editor, and make #1358's "cel.md unchanged" a structural diff #1486 — the profile as env.Config, pinned and round-tripped.

Upstream-gated:
6. #1358 (existing) — pin the eight open libraries, read from v0.31.0's gates.
7. #1480 — the bump, as a build-id change, byte-identical for 2026.1; brings the memory bound (#1483's last row) and the cost-observer fix (#1357). Note the comment there: three dependencies still import github.com/google/cel-go, so the rename links two copies of cel-go until protovalidate moves.
8. #1484 — partial evaluation for compaction; the comprehension case waits on cel-expr/cel-go#1341.
9. #1487 — the next profile's membership, decided from the screen after 6 and 7.

Decision record:
10. #1488 — cel-policy for the five surfaces, before #548's module system.

Second pass, same revision

Filed after the first slate, from the surfaces the first pass touched but did not open. Each was reproduced or counted before filing.

Bugs, reproduced with go run ./cmd/flow:

One mechanism:

What the checked AST could say at validate:

Tooling the existing data already supports:

Pinning semantics, not only names:

Checked and not filed: plugins evaluate no CEL in their own process (cel-go is an indirect dependency of the six plugin modules and no plugin or SDK file references it), so there is no second evaluator across the socket; the macroExamples hand table is correctly hand-written (cel-go's Macro.IsReceiverStyle cannot distinguish a namespace receiver from a value receiver, as catalog_functions.go:261-273 records); the default time zone is UTC unless DefaultUTCTimeZone(false) is set (cel/env.go:904 at v0.31.0), so timestamp accessors are worker-independent.

Third pass: the protobuf side

The second evaluator the second pass said did not exist does, one dependency over.

Checked and not filed: payload encoding is already decided (#911, binary protobuf since #1196); task claims as descriptor options is #1493; the *.test.yaml and policy-file formats as schema messages is #923's question (a comment on #1488 records that the policy documents are Go structs too); secret-typed inputs are #535's family.

Fourth pass: options, prose and the agent surface

Where the schema's own comments and #729's options package should be the one source, and are not yet.

Comments rather than issues: on #1493, that a plugin's example should be a flowstate.v1.Value literal validated against the field's own rules (as InputDeclaration.example already is) rather than a second literal vocabulary, and that a bounded CEL snippet option compiled at manifest load would make plugin documentation executable the way TestEveryDocumentedCELIdiomCompiles makes the reference docs; on #1452, that the TODO(kent) at value.proto:160-167 is the schema's only in-line statement of the value-kind question and should be retired by that decision.

Checked and not filed: diagnostic codes travel as strings by a recorded decision (diagnosticcodes.go:28-40); plugin comments already travel in the manifest (#723, closed by #874).

Fifth pass: hands-on, from the outside

A two-file workload (a caller with typed inputs, a for_each over call:s, a signal gate, a switch:, typed enum and struct outputs, and a test file) was written against the same build and pushed through every surface — validate, lint, audit, fmt --check, compile, test, run local in text and JSON, twenty-eight deliberate mutations, embed.Compile/RunLocal from a Go program, flow mcp over stdio, flow lsp over stdio, and flow debug replay. What held: twenty-two of the twenty-eight mutations were refused at validate with a position, the right name and usually a suggestion; the test verb's coverage report named the unreached arm by line; the debugger replay refused a bad script before running anything. The findings are at the joints between surfaces, and every one is a reproducer in its issue.

The evaluator's own voice (sub-issues here):

The surfaces around it (standalone):

Comments rather than issues: on #1439, the debugger's break place arms both the caller's and the callee's step of that id and the preflight lists callee ids flattened into the caller's universe; on #1452, the catalog's map[string, string] / info | warn | error prose and the MCP JSON Schema are two more spellings of the type vocabulary.

Checked and not filed: fmt reorders with: keys alphabetically and the declaration keys into a fixed order (R3, one spelling; a callee-declaration order for with: would be a style decision, not a defect); flow validate is quiet on int + "x" over steps.* (that is #1481, not a new finding); hover on ${inputs.plan} inside with: answers nothing and completion after r.place. inside a results.map offers nothing (#1463 and #1434 already carry both); embed.Compile refuses a call: with a diagnostic that names the fix (#1376, working as documented); the LSP advertises no signatureHelpProvider (#1506); the MCP tools carry no annotations (#1543) and take base64 for validate and plain YAML for run_local (#1290).

Recorded, not filed

Already tracked and cited from the finding it touches: #1357, #1358, #1360 (fusion inside one evaluation is the decorator/optimizer seam), #1383, #1432, #1436, #1452, #1454, #1459, #1421, #1437, #1438, #1449, #1468, #1491, #1493, #729, #1225, #1285, #558, #1469, #1288, #545, #548, #326, #332, #911, #923, #1012, #1465, #184, #1439, #1290, #1293, #1426, #1471, #1497, #230.

Not verified

Activity

  1. added
    dslFlowfile DSL, YAML front end, expressions
    engineExecution engine, schema, runtime internals
    kind/umbrellaTracks multiple independently executable slices
    on Sep 2, 2026
  2. added sub-issues on Sep 2, 2026
  3. 2 remaining items

  4. added sub-issues on Sep 2, 2026
  5. picatz commented on Sep 7, 2026

    @picatz
    OwnerAuthor

    Sixth pass: the interpreter package against the durable model, and the agent surface

    Read at 7530b29, against cel-go v0.31.0 (pin) and aa72cde (master, 2026-09-04): interpreter/planner.go (decorators and observers), async.go, prune.go, frame.go, evalstate.go, dispatcher.go, runtimecost.go, runtimememory.go; and, outside cel-go, cel-expr/skills (an MCP server and two agent skills over cel.AuthoringPrompt, OptTrackState coverage and env.Config) and cel-expr/cel-spec's doc/extensions/. Every behavioural claim below was reproduced with go run ./cmd/flow on a throwaway file; nothing was posted upstream.

    Reproduced, security-relevant:

    Decisions the slate did not have:

    Capability, with prior art in the CEL project's own tooling:

    Comments rather than issues: #1856 (policy subjects are ext.NativeTypes structs with 23 cel: tags; the proto-first version of the same finding), #1852 and #1486 (cel-expr/skills consumes cel.AuthoringPrompt and env.Config exactly as those issues propose; Flowstate should ship the two skills), #1850 (cel-spec's extension prose as the description source), #1862 (two ledger rows: ResolveName cannot see the frame, and the spec prose as the ext docs source; async/memory/dispatcher read and not needed).

    Checked and not filed: the workflow-side deadlock budget vs the cost budget is already decided and measured (size.go:481-493, engine/panicpolicy.go, #1769, #1357); evalContext() returning context.Background() on the workflow side is a recorded invariant with a guard test (engine/evalcontext.go); the nested stored-expression budget split is documented at eval.go:44-62 and bounded by count, with the path unused by every execution path today; string.format argument checking already runs at validate ("%d".format(["x"]) is refused at flow validate); a runtime profile fingerprint from Dispatcher.OverloadIds() is #1486's document hashed, not a new mechanism.

    Not verified: the durable driver's rendering of derived sensitive outputs (#1868) was inferred from the shared redactGetResponse path, not observed on a Temporal run; cel-expr/skills was read at main over raw.githubusercontent.com (the tarball and API are outside this session's scope), not at a tagged release.


    Generated by Claude Code

  6. picatz commented on Sep 7, 2026

    @picatz
    OwnerAuthor

    Seventh pass: the interpreter against the durable driver

    Read at 7530b29: engine/evalcontext.go, engine/activities.go, engine/execute.go's value/switch/loop paths, engine/progress.go's query handlers, server/list.go, size.go's deadlock budget, and the async/width bounds. One measurement, one design record, two comments.

    Checked and not filed: expression failures on the activity side are non-retryable (ErrorKind.Retryable() excludes expression); flow server dev --db gives single-player durability; typed signal payloads are a recorded refusal (outputs: on the wait shapes them, DSL.md:1742, :1822); the http task's expect: evaluates through DefaultEvaluator with a hierarchical activation (one evaluator); cel-go recovers evaluation panics in prog.Eval (cel/program.go:376), so a cost or bound panic never escapes into the workflow goroutine.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    designdslFlowfile DSL, YAML front end, expressionsengineExecution engine, schema, runtime internalskind/umbrellaTracks multiple independently executable slices

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions