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

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:

a record-after-update flow whose start condition or edge predicate reads previous (the create-vs-update discriminator record_change flow start node binds to a single lifecycle event — no create-OR-update in one flow #3427 bound deliberately, "always, even to null") aborts — the retry fails for a reason attempt 1 never hit, which reads as a flaky flow rather than a defect;
a bare status / budget reference to a triggering-record field aborts for the same reason (the flattening is execute()-only);
$runId is undefined for a pausing node on a retry attempt. Its documented purpose is ADR-0019 mapping: "a pausing node (e.g. Approval) reads $runId to map its external state back to this run for resume". An approval request row minted on a retry attempt therefore carries no run id — the pause is stored (since bug(service-automation): a retry attempt that PAUSES is recorded as failed and its suspension is never persisted — executeWithoutRetry has no isSuspendSignal arm #9510) but the external record cannot point back at it.

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

bug(service-automation): a retry attempt that PAUSES is recorded as failed and its suspension is never persisted — executeWithoutRetry has no isSuspendSignal arm #9510 — the suspend-arm restoration on the same method; its parity pin (packages/services/service-automation/src/retry-attempt-pause.test.ts) asserts this divergence explicitly so it cannot be repaired silently
A flow variable cannot be declared with a default, so "always bound" is not expressible in metadata #4697 / record_change flow start node binds to a single lifecycle event — no create-OR-update in one flow #3427 — the unbound-name CEL abort and the previous binding this breaks
ADR-0019 — the $runId contract for pausing nodes

First-touch grading: promoted to pm:queue, type Bug (domain:services kept). The environment divergence is measured, user-reachable exactly where retry is most used (record-change flows with errorHandling.strategy: 'retry'), and abort-on-unbound CEL turns it into false flow failures plus an ADR-0019 $runId hole for pausing nodes on retry attempts. The "not decided here" question (hoist shared seeding into one helper vs pass the seeded map) is an implementation-route call within an unchanged contract — the dispatching seat/dev can decide it, with the buildRunTrigger chokepoint precedent and the four prior drift cards (#9378 #9415 #9414 #9510) arguing for the shared helper. Serial note: same file/method as #9510's landed work and #9705's open decision — check in-flight claims on service-automation/src/engine.ts before dispatch, and read #9705's outcome before asserting resume-path behavior.

Size/model suggestion: M, mode:subagent.

Generated by Claude Code

fix(service-automation): a retry attempt that PAUSES is a durable pau…

Claim: PM loop round 1
Session: session_01PnJHU45vPJj5UQrxe946Bx
Branch: claude/issue-9704-retry-variable-env
Worktree: objectstack-issue-9704
Domain: domain:services
File surface: packages/services/service-automation/src/** (engine.ts + its tests, incl. retry-attempt-pause.test.ts parity pin); stop on breach; explain in the report.
Container & model: M, mode:subagent, model: opus — tier per this round's node scripts/pm/dispatch-gates.mjs --tier packages/services/service-automation/src/engine.ts output: "no path-derived mandate — the tier stays the PM's per-card judgment call"; PM sets default judgment tier (opus): the repair-shape choice (shared seeding helper vs caller-passed map) is a design judgment. Triage suggested M/subagent — adopted; sonnet not taken because the card's open implementation-route question is judgment, not gate-mechanical.
Clause-②: no — restores the invariant that a retry attempt runs in the same variable environment as attempt 1; no contract accept/reject change, no public-surface widening.
Serial constraints cleared: service-automation/src/engine.ts — #9510's work LANDED (PR #9785 merged, same-day churn confirmed by git log origin/main), no open claim on the file. Sibling card #9705 (resume-path retry loop) is open, unassigned, same file — serialized behind this card, not dispatched this batch; this card's report must answer the mandatory repricing question for #9705.

Generated by Claude Code

{
  "issue": 9704,
  "status": "done",
  "branch": "claude/issue-9704-retry-variable-env",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/9888",
  "premise_still_valid": true,
  "summary": "Re-derived the divergence set fresh against merged main (55d29935b, post-PR #9785): intact and complete — executeWithoutRetry() seeded only seedDeclaredVariables + $record, while execute() also binds record, the trigger record's flattened fields, previous, $runId, $flowName and $flowLabel. Repaired via the ruled route (shared helper hoist): one private seedRunVariables(flow, flowName, context, runId) holding execute()'s block verbatim, called by both methods, with nextRunId() moved ahead of seeding in both (order-safe — it is a stateless random id, no counter). The #9510 parity pin flipped substantively: the two snapshots are now compared to each other with $runId normalized, plus by-value assertions on the retry route for $runId/$flowName/$flowLabel/previous/record/flattened fields, because the parity comparison alone is satisfied if BOTH routes lose them. No contract, vocabulary or public-surface change; patch changeset included (published package). File surface stayed within the declared packages/services/service-automation/src/**.",
  "tests": "All results at final commit 0dc1f8406 (clean porcelain); the gate union was re-run after that commit, including the three PM-named gates first run at ef8ac2dda. RED-BEFORE-GREEN: with only the test flipped and engine.ts unrepaired, `AssertionError: expected undefined to be 'run_c27e4d29-1f2e-4df3-8fab-2fac4b85e...'` (EXIT=1, 1 failed | 5 passed); after the engine fix, EXIT=0, 6 passed. FULL PACKAGE SUITE: `pnpm --filter @objectstack/service-automation test` EXIT=0 — `Test Files 82 passed (82) / Tests 988 passed (988)`, identical counts to the pre-change baseline on origin/main, so first-attempt behaviour is unchanged. ABLATION (standing lane clause, both legs): NO REBUILD IS INVOLVED OR NEEDED — the pin imports engine.ts as an intra-package relative specifier ('./engine.js'), not through a dependency's exports to dist/, so vitest reads source; this is proven rather than assumed by the red/green pair above (test-only edit went red, engine-only edit went green, no build between). Leg A, deleting variables.set('previous', ...): EXIT=1, `AssertionError: expected { '$record': ..., ...(7) } to have property \"previous\" with value null`. Leg B, deleting the record alias + flattening loop: EXIT=1, `AssertionError: expected undefined to deeply equal { id: 'ord_9510', amount: 500 }` — failing on the by-value assertion, not the parity comparison, which is the guard those assertions exist for. Both legs restored with `git checkout HEAD -- src/engine.ts` and proven byte-identical: `git hash-object` = 2d61de4d9ceccd18b687490206f19da5bac9ac36 = `git rev-parse HEAD:packages/services/service-automation/src/engine.ts`, porcelain empty. TYPECHECK: `npx tsc --noEmit -p tsconfig.json` = 3 errors, all pre-existing (nested-region-parity.test.ts TS2341 private 'flows'), matching the DEBT ledger's recorded errors:3 — none in either changed file. `pnpm --filter ... typecheck` was deliberately NOT used: the package declares no typecheck script, so it matches zero scripts and exits 0 having run nothing. GATES — five PM-named, all EXIT=0: check:cross-package-test-inputs (`OK: 12 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.`), check:test-source-alias (`OK — 72 packages with tests scanned`), check:type-source-resolution (`OK — 76 packages with a tsconfig.json scanned`), scripts/check-cross-package-test-inputs.mjs (same OK line), scripts/docs-audit/check-affected-docs.mjs (`affected-docs self-test: 242 cases pass.`). RE-DERIVED with `node scripts/pm/dispatch-gates.mjs` (no paths passed; it derived 3 committed paths vs merge base 55d29935b) — it added six path-matched and five convention-triggered families, ALL RUN, all EXIT=0: check:changeset-gate-self-tests, check:objectui-changeset, scripts/check-adr-0087-registration.mjs (`adds no declared-breaking changeset`), scripts/check-changeset-no-major.mjs (`introduces no major bump`), scripts/check-empty-changeset.mjs (`No empty-frontmatter changeset introduced`), check:query-options-erasure (`ratchet holds ... none new`), check:engine-double-contract (`OK — 321 pinned, 133 in the DEBT ledger, 2 exempt`), check:where-matcher (`258 matcher(s) ... none new`), check:type-check-coverage (`OK — 64/77 workspace packages type-checked`), check:type-check-debt (`--re-measure: OK — 33 ledger entr(ies) re-measured in 351.9s, 1926 raw tsc error(s) total, none above its recorded number.`), plus check:nul-bytes (`no raw ASCII control bytes`). check:type-check-debt refuses to measure on an unbuilt closure, so the full 70-package workspace build was run first (turbo: 70 successful) — its result is MEASURED, not skipped. Every exit code was captured before any pipe (redirect-then-tail), and each gate is quoted by its own verdict line, never a bare $?.",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #9889: node input-schema validation guards only the FIRST attempt — validateNodeInputSchemas is called in execute() only, and since the retry handoff sits inside execute()'s catch, a flow whose node config violates its own declared inputSchema throws before any node runs on attempt 1, then executes those very nodes for real on attempts 2..N through executeWithoutRetry. Different defect class (a skipped guard, not a divergent variable environment), so not repaired here; the guard's verdict is a pure function of the flow definition (its variables parameter is unused), which is what makes the sequence deterministic."
  ],
  "repricing_9705": "UNAFFECTED. #9705 is the resume path: resumeInternal neither reads flow.errorHandling nor enters retryExecution, while this change touches only how the two execution entry points seed their variable map — the file's own pin for that question ('answers the retry-budget question: a resumed run does not retry, on either route') is untouched and still green in the 988/988 run, so #9705's premise and difficulty are both unchanged."
}

Generated by Claude Code

ACCEPT — reviewer of record: PM session session_01PnJHU45vPJj5UQrxe946Bx, round 1. PR: #9888 (draft, CI converging — will flip ready + enqueue on green gates; readings verified against GitHub, not the report's self-description).

Checklist conclusions:

PR form: draft ✓, base main ✓, first line Fixes #9704 ✓; 3 changed files, all inside the declared packages/services/service-automation/src/** surface + one patch changeset (published package) ✓.
Route ruling compliance: shared seedRunVariables chokepoint, execute()'s seeding block moved verbatim with ordering preserved (declared vars first so flattening cannot shadow flow inputs); run id minted before seeding in both callers with the order-safety argument stated (stateless id, caller keeps ownership). Spot-checked the diff — executeWithoutRetry now seeds through the same helper. ✓
Premise re-derivation: divergence set re-measured at merged main (post-fix(service-automation): a retry attempt that PAUSES is a durable pause, not a failed attempt — executeWithoutRetry gets the ADR-0019 suspend arm (#9510) #9785), confirmed intact and complete — the card's five seedings are exactly the delta. ✓
Pin flip substance: the bug(service-automation): a retry attempt that PAUSES is recorded as failed and its suspension is never persisted — executeWithoutRetry has no isSuspendSignal arm #9510 divergence pins are replaced by (a) snapshot-to-snapshot parity with $runId normalized AND (b) by-value assertions on the retry route ($runId against the returned run id, $flowName/$flowLabel/previous via toHaveProperty/record/flattened fields) — the by-value layer guards exactly the both-routes-lose-them failure the parity comparison cannot see. Genuinely-illegal-shape assertions (retry budget accounting, resumed-run-does-not-retry) untouched. ✓
Evidence: red-before-green captured; two-leg ablation with predicted distinct signatures, restores proven blob-identical (git hash-object = HEAD blob); 988/988 package suite identical to baseline (first-attempt behavior unchanged); gate union re-derived and all green, check:type-check-debt measured on a built closure rather than skipped. ✓
Repricing bug(service-automation): a flow's declared errorHandling.strategy: 'retry' silently stops applying once the run pauses — the resume path has no retry loop and the continuation carries no attempt state #9705 (mandatory item): UNAFFECTED — resume path never enters retryExecution; its pin is untouched and green. Accepted; bug(service-automation): a flow's declared errorHandling.strategy: 'retry' silently stops applying once the run pauses — the resume path has no retry loop and the continuation carries no attempt state #9705 stays hard-serialized behind this PR reaching MERGED (same file), then dispatches at its original pricing.
Out-of-scope finding: bug(service-automation): node input-schema validation guards only the FIRST attempt — a retry attempt runs the nodes attempt 1 refused to run #9889 filed unassigned (input-schema guard skipped on retry attempts — refusal that holds only until retried). Verified it exists and is self-contained; it is fully bare (no labels) so the triage sweep will pick it up; its "not decided here" fork (guard parity vs non-retryable classification, which changes retry accounting) genuinely needs a ruling — correctly not folded into this PR.

Landing plan: verify gate-family jobs read completed: success by name on head 0dc1f8406, then flip ready and enqueue via the merge queue.

Generated by Claude Code

fix(service-automation): route every retry attempt through the node i…

fix(service-automation): a retry attempt that PAUSES is a durable pau…

fix(service-automation): route every retry attempt through the node i…

Navigation Menu

Repository navigation

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

Measured

Why it matters

Not decided here

Refs

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Measured

Why it matters

Not decided here

Refs

Activity

os-support-ai commented on Aug 18, 2026

os-warren commented on Aug 19, 2026

os-warren commented on Aug 19, 2026

os-warren commented on Aug 19, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

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