[AIWithShrey]

What was wrong

Two cancellation/correctness bugs in CEL evaluation:

• EvaluateExpression built the resource binding only when vars["resource"] was unset. A reused variables map therefore kept the first evaluation's context: after that context was cancelled, later resource.Get/resource.List calls on a live context failed with context canceled, and a later call's own deadline was ignored.
• A prometheusMetrics query cancelled by its context was returned as an error map instead of an error.

What changed

• cel.go: the resource binding is rebuilt on every evaluation, as the other bindings already are.
• resource_macros.go: when the query context has ended and the query error wraps that context error, prometheusMetrics returns prometheus query cancelled: %w. The check is errors.Is, so other query errors keep the error map.

Tests

cel_ctx_binding_test.go:

• TestCELReusedVarsHonourLaterDeadline: one vars map, a later evaluation's own deadline is honoured. Fails if the vars["resource"] skip is restored.
• TestCELPrometheusMetricsCancelledQueryReturnsError: a cancelled prometheusMetrics query (real HTTP client) returns an error satisfying errors.Is(err, context.DeadlineExceeded). Fails with the new check disabled.
• TestCELPrometheusMetricsUnrelatedErrorOnCancelledCtxKeepsErrorMap: an unrelated query error with the text upstream: context canceled on an already cancelled context keeps the error map. Passes with or without the fix; fails if the check is loosened to qctx.Err() != nil alone or to an error-text match.
• TestCELPrometheusMetricsDeadlineErrorOnLiveCtxKeepsErrorMap: a dial timeout wrapping context.DeadlineExceeded on a live context keeps the error map. Fails if the check ignores the query context and matches on the error alone.

go test -race, go vet and golangci-lint v2.11.4 pass on internal/workflow/executor and internal/metrics.

Note on #113

Until #113 merges, a cancelled prometheusMetrics inside an outputs: value evaluated through EvaluateOutputValue yields the literal expression string, because that method falls back to the literal on any evaluation error. The in-cluster runner's root context is context.Background() today, so there this needs #113. A local ottoflow run reaches it when Ctrl-C or --timeout ends the context during the query; before this change that path stored the error map. Either merge order works.

🤖 Generated with Claude Code

[Copilot]

🟢 Approved

No active review issues are recorded, although tests were not run during this review.

No open issues 🎉

What changed in this PR

This PR fixes context handling in OttoFlow’s CEL evaluator so reused variables honor the current evaluation’s context and cancelled Prometheus queries surface as errors.

Changes:

• Rebuilds the resource binding on every evaluation to avoid retaining an earlier context.
• Returns a CEL error when a Prometheus query error wraps the query context’s cancellation error, while preserving error maps for unrelated failures.
• Adds regression tests for reused-context deadlines and Prometheus cancellation handling.

Tests were not run during this review.

File	Description
internal/​workflow/​executor/​resource_macros.go	Propagates context-related Prometheus query failures as CEL errors.
internal/​workflow/​executor/​cel.go	Refreshes the resource binding for each evaluation.
internal/​workflow/​executor/​cel_ctx_binding_test.go	Tests deadline handling and cancellation error classification.

_{Review effort: Balanced}

[patelrit]

The code looks like a faithful port of ottoflow-enterprise#250. The resource binding is rebuilt every call (cel.go:471-478), and the prometheusMetrics cancel check uses errors.Is on the query ctx (resource_macros.go:550-554), so there's no string matching and the Prometheus URL isn't leaked. The tests cover T13/T14/T14b/T14c and fail on revert. I also checked that dropping the vars["resource"] skip doesn't break resourceQuery outputs, since BuildVariableMap never passes "resource" through.

Two things before I can approve:

1. CI never got past compile on this head. It fails on undefined: runIsTerminal, which is the main breakage #211 fixed, and the branch is 8 commits behind main. Can you update from main so CI actually runs?
2. There's no e2e (kind) run on this head. Please start the E2E workflow (workflow_dispatch is fine) on the updated SHA.

Not blocking: T12 (flat resource.List cancel attribution) doesn't apply here because OSS doesn't have ContextEval/cancelRec yet. Worth a follow-up issue to port the rest of #250's interruptibility work.

[patelrit]

Both things I asked for at 6d148a0 are done on 169eeab.

• CI is green on 169eeab (run 42056971970). lint, build, test, verify-codegen and validate-samples all pass, so the runIsTerminal compile break is gone after the merge from main. CodeQL passes too.
• E2E workflow_dispatch run 42056990993 on 169eeab passed, and the e2e job actually ran.
• 169eeab is only a merge from main. It doesn't touch cel.go, resource_macros.go or cel_ctx_binding_test.go, so what I said at 6d148a0 still holds. The resource binding is rebuilt on every call (cel.go:471-478), and the cancel check uses errors.Is on the query ctx (resource_macros.go:550-554). The new tests fail on revert.
• Source PR ottoflow-enterprise#250 is merged and approved.

No open threads, and Copilot is 🟢 on this head. Approving. I'd still like a follow-up issue to port the rest of #250's interruptibility work (T12 / ContextEval).