Skip to content

bug(service-automation): a RETRY attempt runs with a smaller variable environment than the first — executeWithoutRetry seeds none of the engine-owned variables #9704

Description

@os-project-manager

Found while implementing #9510 (the missing isSuspendSignal arm on the same method). Filed rather than fixed there: different defect class — a divergent run environment, not a lost durable pause — and repairing it changes what every retry attempt does, pausing or not, which #9510's ruling does not cover.

Measured

packages/services/service-automation/src/engine.ts, origin/main @ ed4ca5999.

execute() seeds the run's variable map with five things executeWithoutRetry() does not:

// execute() only:
variables.set('record', context.record);          // + every record field flattened,
                                                  //   so bare `status` / `budget` resolve
variables.set('previous', context?.previous ?? null);
variables.set('$runId', runId);
variables.set('$flowName', flowName);
variables.set('$flowLabel', flow.label ?? flowName);

executeWithoutRetry() — the method retryExecution re-runs the flow through on every retry attempt — seeds only seedDeclaredVariables(flow, context) and $record.

Observed directly: two runs of one flow, pausing on attempt 1 vs on attempt 2, produce continuations whose variable snapshots differ by exactly $runId, $flowName, $flowLabel, previous — present on the first-attempt run, absent on the retry.

Why it matters

Conditions are strict CEL, where reading an unbound name ABORTS the predicate (Unknown variable: X) rather than yielding false — the mechanism #4697 documents on seedDeclaredVariables. So on a retry attempt:

Every one of these is reachable exactly where retry is most used: errorHandling.strategy: 'retry' on a record-change flow.

Not decided here

Whether the repair is to hoist the shared seeding into one helper both methods call (the obvious reading — the two methods already duplicate the disabled / no-start-node / failure exits and have drifted once per card: #9378, #9415, #9414, #9510), or to make executeWithoutRetry take the already-seeded map from its caller. The first is more in keeping with buildRunTrigger's chokepoint precedent, but it touches the first-attempt path too, so it wants a ruling rather than a guess.

Refs

Activity

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

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions