Repository navigation
fix(executor): bound condition loops and reject unnamed variable assignments - #9463
waleedlatif1 wants to merge 3 commits into
Conversation
…t unnamed variable assignments
A While loop, or a Do While loop given a condition, had no iteration bound: a body
that never changed what the condition checks repeated until the run timed out, and
billing-disabled deployments run untimed, so it repeated until the process stopped.
Such loops now fail with an error on the loop after 10,000 iterations if the
condition is still true. For, For Each and count-driven Do While loops are unchanged.
A Variables assignment with a value but no variable name was skipped without an
error. It now fails the block; blank editor rows are still ignored, and a row that
selects its variable by id alone now updates that variable. The block's published
input description named the wrong field ("name"), which is corrected to variableName.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
 [Medium impact] The PR appears safe to merge; no blocking issues remain. SummaryThe PR caps condition-driven loops at 10,000 iterations and rejects variable assignments that supply a value without a target.
Reviews (3) · Last reviewed commit: "chore(executor): shrink the explicit-any..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
Summary
Two executor gaps let a workflow loop forever without any error.
Condition-driven loops had no bound. A While loop, or a Do While loop given a condition, stopped only when its condition turned false. If the body never changed what the condition checks, the loop repeated until the run's timeout. The original While design relied on that timeout, but billing-disabled (self-hosted) deployments now run untimed, so there the loop repeated until the process was stopped. Hosted runs still used their full timeout.
These loops now stop after
DEFAULTS.MAX_CONDITION_LOOP_ITERATIONS(10,000) iterations if the condition is still true. The run then fails with an error log on the loop container ("While loop stopped after 10,000 iterations with its condition still true…"). Exiting quietly would pass a truncatedresultsdownstream as if the condition had ended the loop. A condition that turns false on the last allowed iteration still exits normally. For, For Each and count-driven Do While loops keep their configured count, and the ceiling does not apply to them.The ceiling is per loop run: nested loops reset their scope on each outer iteration, and
scope.iterationalready carries over through pause/resume snapshots, so no snapshot shape changes. 10,000 leaves room for realistic polling (a 1 s poll runs about 2.8 h, longer than the default hosted async timeout). A tight loop with a stuck condition now fails in seconds to minutes instead of running until the timeout.Variables assignments without a name were dropped.
parseAssignmentsskipped any row with a blankvariableName, so an update meant to advance loop state did nothing and raised no error. This is how a stuck While condition usually happens. The new rule:variableIdselects. A row with an id but no name previously did nothing, even though the variable existed.Assignment N sets a value but names no variable….typetakes its target variable's type, so{ variableName: 'count', value: 1 }stays a number. Before, it was stored as the string '1', which breaks<variable.count> + 1in a loop counter.The block's published input description said "objects with name and value properties".
nameis not the field the handler reads, so authoring tools that read the catalog were steered to the wrong key. It now namesvariableName.Type of Change
Testing
executor/execution/condition-loop-ceiling.test.tsruns real workflows throughDAGExecutor:handler.test.ts:namekey, or through an unknown id with no name now fails. On the previous code all three passed silently.vitest run executor lib/workflows lib/catalog blocks serializer: 335 files, 4607 tests pass.tsc --noEmitis clean for apps/sim, as are biome,check:test-types,check:comment-hygiene,check:utilsandcheck:source-conventions.Reviewers: does the 10,000 ceiling size fit how condition loops are used?
Checklist