hotfix: KEEP-468 exclude condition fields from action-level post-scan#1192
Merged
Conversation
… post-scan
The KEEP-468 strict gate ran `assertResolved` over `processedConfig` after
re-attaching `condition` and `conditionConfig`. Those fields intentionally
keep their `{{...}}` tokens at the action level (they're substituted later
by `evaluateConditionExpression`, which has its own leftover-token gate),
so the post-scan saw resolvable tokens like `{{@fe:Each Item.currentItem}}`
and false-tripped with "Reference left in rendered config" — aborting
every iteration of any For Each that contained a Condition node.
Confirmed in prod execution data on workflow `befqn0fu66t7un1mqm8r2`
(KEEP-520-C cascading-conditions test fixture): every iteration of the
For Each body returned `cond_outer` failure with the unresolved-leftover
message, the `done` Collect aggregated 6 failed-result rows, no iteration
logs were written.
Fix: reorder `processActionConfig` so `assertResolved` scans the rendered
config BEFORE `condition`/`conditionConfig` are re-attached. The function
still returns the same shape, downstream consumers (executeActionStep,
condition step handler, true/false handle routing) see no change.
Condition templates remain protected by `evaluateConditionExpression`'s
own leftover-scan added in commit b3d5d9e.
Regression test: `tests/unit/template-fail-closed-condition-bypass.test.ts`
captures the contract directly against `assertResolved`:
- scanning a config with a Condition's templated expression trips the
post-scan (regression baseline)
- scanning the same config without `condition`/`conditionConfig` is
clean (post-fix order)
- leftover scans still catch genuinely unresolved action fields
Full suite: 4413 unit + integration tests pass, 0 failed.
🧹 PR Environment Cleaned UpThe PR environment has been successfully deleted. Deleted Resources:
All resources have been cleaned up and will no longer incur costs. |
ℹ️ No PR Environment to Clean UpNo PR environment was found for this PR. This is expected if:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hotfix for a regression introduced by #1187 (KEEP-468 strict-mode template fail-closed) that landed in prod via #1191. Every workflow with a Condition node referencing upstream data — including the For Each + Condition cascades shipped for KEEP-520 — has been aborting every iteration with
Reference left in rendered configsince the release.Root cause
processActionConfigran the strict-mode post-scan (assertResolved) overprocessedConfigAFTER re-attachingconditionandconditionConfig. Those fields intentionally keep their{{...}}tokens at the action level (they're substituted later byevaluateConditionExpression, which has its own leftover-token gate from commitb3d5d9eb), so the post-scan saw resolvable tokens like{{@fe:Each Item.currentItem}}and flagged them as unresolved leftover literals.Fix
Reorder
processActionConfigsoassertResolvedscans the rendered config beforecondition/conditionConfigare re-attached. The function still returns the same shape; downstream consumers (executeActionStep, condition step handler, true/false handle routing) see no change. Condition templates remain protected byevaluateConditionExpression's own leftover-scan.Confirmed against prod data
Workflow
befqn0fu66t7un1mqm8r2(KEEP-520-C cascading-conditions test fixture):gen_itemscorrectly produced[1, 5, 8, 12, 18, 25]fe(For Each) iterated 6 timescond_outeraborted with the false-positivedone_collectaggregated 6 error rowsVerification
tests/unit/template-fail-closed-condition-bypass.test.tscaptures the design contract: the action-level scan must not see condition fields.Notes
evaluateConditionExpressionwas already correct — this hotfix only fixes the action-level scan.