Skip to content

Redrive runs whose workflow applied an update before the failure - #1007

Merged
rmcdaniel merged 4 commits into
mainfrom
fix/864-redrive-applied-updates
Nov 3, 2026
Merged

rmcdaniel merged 4 commits into
mainfrom
fix/864-redrive-applied-updates

Conversation

@rmcdaniel

Copy link
Copy Markdown
Member

The update half of A2c from #864, targeted at Workflow 2.20.0. The signal half shipped in 2.19.0 (#1002).

Problem

A failed run whose history contained an update (UpdateAccepted, UpdateApplied, UpdateCompleted) was refused with unsupported_history. Updates are a common way to change workflow state while it waits, so these runs could only be started over.

How updates replay

An update doesn't take a step of its own. WorkflowExecutor::applyRecordedUpdates() reruns the handler from UpdateApplied.payload.arguments at the recorded payload.sequence position, before that step's own handling. A handler that throws records no UpdateApplied, so replay never re-applies it. The successor gets the same events, so it replays exactly as the source would.

Change

  • Plan. FailedRunRedrivePlan::updateBundles() groups each update by update_id.
    • Applied update: it needs its UpdateAccepted, UpdateApplied and UpdateCompleted (with an inline result and no failure), all bound to the source run, instance, workflow_command_id and update_name. It must be attached at a valid step at or before the failure boundary.
    • Skipped: a rejected update, and an update closed by the run's failure before it ran (UpdateCompleted.terminal_reason).
    • Refusals:
      • unsupported_pending_update: accepted, but never applied or closed. The successor would lose it.
      • external_update_payload_not_supported: arguments or result in external storage.
      • incomplete_update_step / mismatched_update_history: malformed or cross-bound history.
  • Successor. successorEvents() writes each bundle ahead of the first reused step at or after its position, with reused_from_run_id / reused_recorded_at. No update row or command is recreated, so nothing is delivered again.
  • Service fence. A run a service worker started that applied an update refuses with service_redrive_history_not_qualified, before any write (Refuse service redrive of reused steps its SDKs can't replay yet #1006's rule extended).
  • Payload contract and docs/api-stability.md. The three update events accept the reuse keys.
  • CHANGELOG Unreleased updated.

Tests

Local runs, PHP 8.4, SQLite + Redis:

  • testRedriveReappliesAnUpdateTheSourceApplied uses a new TestRedriveUpdateWorkflow: activity, wait on a go signal, activity. An update set-note is applied during the wait, then the signal is sent and the second activity fails. The successor:
    • completes with note: urgent (the handler's state change replays),
    • doesn't rerun the first activity,
    • has no successor workflow_updates row,
    • records the three update events adjacent and ahead of the signal step, at the same recorded position, with the reuse keys and source times.
  • testRedriveRefusesAnUpdateTheSourceNeverResolved: with only UpdateAccepted left, the redrive refuses with unsupported_pending_update and no mutation (runs, history, cursor, current run).
  • DefaultWorkflowControlPlaneRedriveRejectionTest service-fence provider has a new applied update case. It refuses with service_redrive_history_not_qualified, with byte-identical engine tables and projections.
  • All three fail on main (refused there with unsupported_history).
  • Suites: V2FailedRunRedriveTest 30/30, V2UpdateWorkflowTest 31/31, FailedRunRedrivePlanTest, HistoryEventPayloadContractTest, SurfaceStabilityContractTest and ReplayRegressionCorpusTest all pass. The rejection suite is 48/49; the one failure is the known local-only cache case, which also fails on main.
  • Replay corpus: new redrive-reused-update fixture and scenario. validate-regression-corpus.py --base-ref 28893f97 passes with replay counterfactual_fixture_paths: 1.
  • ecs (check-only after fix) and phpstan are clean.

🤖 Generated with Claude Code

…#864)

The update half of A2c. Any UpdateAccepted, UpdateApplied or
UpdateCompleted event in the completed prefix refused redrive with
unsupported_history.

- The plan groups each applied update's acceptance, application and
  completion, bound to the source run, instance, command and update name,
  at the step it ran before (at or before the failure boundary).
- The successor records them with reused_from_run_id and
  reused_recorded_at ahead of that step, from the shared
  successorEvents() builder, and replay runs the handler again from its
  recorded arguments. No update row or command is recreated.
- Rejected updates and updates the failure closed before they ran
  (terminal_reason) are not reused. An accepted, unresolved update refuses
  with unsupported_pending_update; an external update payload with
  external_update_payload_not_supported; malformed history with
  incomplete_update_step or mismatched_update_history.
- The service-mode fence now also covers applied updates.

Counterfactuals: an applied update redrives with its state change, a
pending update refuses with no mutation, and a service run with an applied
update refuses with no engine or projection change; all three are refused
differently on main. Replay corpus evidence: redrive-reused-update.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security full review of exact head b1739b0e2480cfee1db6e9728b2deae67fef5dae: the service fence, transaction placement and happy-path replay are sound, but this head is held. The new update planner accepts malformed bundles and is one-hop-only.

  1. Enforce the update state machine and history order. Today an UpdateAccepted plus UpdateRejected is silently skipped, a lone UpdateCompleted.terminal_reason is silently skipped, and accepted/applied/completed may appear in any history order. Accept only these exact shapes:
    • rejected: one UpdateRejected, and no accepted/applied/completed;
    • closed before application: UpdateAccepted followed by one terminal UpdateCompleted, with no rejected/applied;
    • reusable: UpdateAccepted < UpdateApplied < successful UpdateCompleted by history-event sequence.
      Any extra, missing, duplicate, reversed or cross-state event must refuse with a typed reason and no mutation.
  2. Bind every reusable field exactly. Require nonblank update_id, workflow_command_id and update_name; exact source run and instance; identical IDs/names across all three events; and byte/type-exact equality of UpdateAccepted.arguments and UpdateApplied.arguments. The current all-null command/name comparison passes, and changed applied arguments pass even though replay invokes the handler from those changed arguments. Refuse failure_id/terminal_reason keys on the successful completion even when null, not only truthy values.
  3. Validate payload availability before accepting. Treat the presence of an external_storage key as external even when its value is null. For inline accepted/applied arguments and successful result, prove they can be decoded with the run's exact payload codec before the transaction writes a successor, as the activity path already does. Missing, malformed or unavailable inline payloads must refuse without mutation rather than create a successor whose first replay fails.
  4. Support a redrive chain. On the first successor the copied update payload retains the original source workflow_run_id; a second redrive then fails the strict payload.workflow_run_id === $run->id check. Add an A→B→C test where B fails after reusing the update and C reuses it again exactly once, preserving the earliest reused_recorded_at, immediate-hop provenance, state change, update result history and no successor update/command row. Define and validate original versus immediate-source provenance explicitly instead of weakening run binding.
  5. Add mutation counterfactuals for every refusal above. Each must snapshot the engine tables, current_run_id, history count, cursor and the summary/timeline projections, as the service-fence test does. The existing pending-only counterfactual does not cover the newly accepted malformed states.

Keep the update events ahead of their recorded step, preserve the service fence, and do not recreate or deliver an update row/command. Return the corrected exact head and focused suite/corpus evidence.

)

Security full review of #1007 found the update planner too permissive and
one-hop only.

- Updates are accepted in exactly three shapes, by history order: a lone
  rejection; an acceptance closed by the run's failure (terminal_reason);
  or acceptance < application < successful completion. Any extra,
  missing, duplicate, reversed or cross-state event refuses.
- A reusable update needs nonblank update_id, workflow_command_id and
  update_name, identical across its three events, and byte-identical
  accepted and applied arguments. A successful completion may not carry
  failure_id or terminal_reason keys, even when null.
- Any external_storage key, even null, refuses with
  external_update_payload_not_supported. Inline arguments and result must
  decode with the run's codec, or redrive refuses with
  unavailable_update_payload before writing.
- Provenance is explicit: a payload recorded by this run names this run;
  a payload this run reused names its immediate redrive source in
  reused_from_run_id and an earlier run of the instance as its original.
  Signal receipts use the same rule, so a redrive successor that reused
  signal waits or updates can be redriven again.

Counterfactuals: an A->B->C chain reusing an update and a signal wait at
each hop, and eight malformed update histories refused with byte-identical
engine tables and projections. All nine fail on b1739b0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Addressed Security's hold (comment 6615544362). New head 1c17ed449225848e828e2094c4a65be487572829.

  1. State machine and order. Events are grouped by update_id and sorted by history sequence. Only three shapes are accepted:

    • [UpdateRejected],
    • [UpdateAccepted, UpdateCompleted+terminal_reason],
    • [UpdateAccepted, UpdateApplied, UpdateCompleted].

    A lone acceptance is unsupported_pending_update. Everything else is mismatched_update_history.

  2. Exact binding.

    • update_id, workflow_command_id and update_name must be nonblank strings, identical across all three events.
    • Accepted and applied arguments must be present and ===.
    • The completion needs result and must have no failure_id or terminal_reason key, even when null (incomplete_update_step).
  3. Payload availability.

    • array_key_exists('external_storage') or a stored-reference string refuses with external_update_payload_not_supported.
    • Inline arguments and result must pass ExternalPayloads::payloadBlob and Serializer::unserializeWithCodec with the run's codec. Otherwise the redrive refuses with unavailable_update_payload before any write.
  4. Chains. New explicit provenance shared by updates and signal receipts:

    • A payload without reused_from_run_id must name this run.
    • A reused payload must name this run's immediate redrive source in reused_from_run_id, and an earlier run of the same instance as its original workflow_run_id.
    • The instance must match in both cases.

    This also fixes released 2.19.0 signal waits. A successor that reused a signal wait was refused with mismatched_signal_receipt on a second redrive. That was safe, but it was the same gap. The CHANGELOG says so.

Counterfactuals (all fail on b1739b0e):

  • testAnUpdateAndSignalWaitRedriveAcrossAChain: A → B (fails again) → C completes. In C, each of UpdateAccepted, UpdateApplied, UpdateCompleted, SignalReceived and SignalApplied appears exactly once, with reused_from_run_id = B, the original workflow_run_id = A and A's reused_recorded_at. The note state change survives, the first activity ran once, there's one workflow_updates row for the instance, and B and C have no update or signal commands.
  • testRedriveRefusesMalformedUpdateHistoryWithoutMutation, eight cases: acceptance then rejection, lone terminal completion, application before acceptance, null command and update name, changed applied arguments, a null failure_id key, a null external_storage key, and an undecodable inline result. Each asserts its typed reason and a byte-identical snapshot of every engine table (instance, run including cursor, commands, history, tasks, links, memos, search attributes, signals, updates, activities, attempts, failures) and of WorkflowRunSummary and WorkflowTimelineEntry, plus current_run_id.

Local: V2FailedRunRedriveTest 39/39, corpus 20/20, plan unit 11/11, rejection 48/49 (the known local cache case only). ecs check-only and phpstan are clean. Hosted CI is rerunning.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security re-review of terminal-green exact head 1c17ed449225848e828e2094c4a65be487572829.

The original holds are substantially closed: the successful update triple is ordered and exactly bound, inline payloads are decoded, successful completions exclude failure/terminal keys, the service fence covers applied updates, and the A→B→C positive path preserves immediate/original provenance.

Three fail-closed holds remain:

  1. Validate the other two accepted lifecycle shapes, not only their event-type list. [UpdateAccepted, UpdateCompleted] currently skips the update whenever the terminal_reason key merely exists. Require the actual terminal-closure shape: matching nonblank update/command/name identities, both events bound to the source provenance, a nonblank terminal reason and failure id, and no success result/step sequence. A null terminal reason, a result plus terminal key, a foreign run/command/name, or a missing failure id must refuse without mutation. Apply the same run/identity binding to a lone UpdateRejected before treating it as safely skipped, or narrow the public exact-shape claim and prove why malformed rejection data is irrelevant.
  2. Make chained provenance a pair, with no timestamp fallback. For every reused update event and each reused signal wait/receipt/application event, reused_from_run_id and reused_recorded_at must be both absent (original in this run) or both present/nonblank. When present, the source must be the immediate redrive parent and the original run must be an allowed earlier run; preserve the original timestamp. Today boundToRun() ignores a lone reused_recorded_at, does not require the timestamp when reused_from_run_id exists, and wait/application are not checked. successorEvents() then silently replaces a missing original time with the current event time. Add deletion/null/mismatch counterfactuals for every event in the update and signal bundles, with no mutation.
  3. Restore the already accepted external-signal boundary. isExternalResult() still uses isset($result['external_storage']), so an envelope containing external_storage: null is accepted. Use key presence (array_key_exists) as holdsExternalPayload() does, and cover both SignalReceived.arguments and SignalApplied.value with null-key counterfactuals. Any external-storage key is external.

Return the exact head with the full hosted matrix. The successful-triple argument equality and the new unavailable-update-payload reason are otherwise source-clean.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security emergency stop on exact head f44aee9: do not merge, mark ready, or treat this CI run as qualifying.

The single new commit accidentally tracks the validator consumer worktree .regression-corpus-consumer-zqh_l0l2/: 2,303 files under that directory, including a complete nested repository tree and generated dependencies/artifacts. GitHub reports 2,313 changed files and 552,584 additions; Sonar is already red. Against the last held head 1c17ed4, the intended delta is only CHANGELOG.md, src/V2/Support/FailedRunRedrivePlan.php, and tests/Feature/V2/V2FailedRunRedriveTest.php.

Remove the entire tracked .regression-corpus-consumer-zqh_l0l2 tree from the branch, ensure validator scratch paths cannot enter the index, and return an exact head whose diff contains only the intended product/test/changelog files. Re-run the complete hosted matrix. The prior functional Security holds remain in force until that clean head is reviewed; no evidence from f44aee9 carries.

Security's hold on 1c17ed4:
- A rejected update, or an acceptance closed by the failure, is skipped
  only when every event names the same update, command and handler and
  belongs to the source run. A closed acceptance needs a terminal reason
  and a failure id, with no result and no step.
- reused_from_run_id and reused_recorded_at come as a pair on every update
  and signal event (wait, receipt and application). A reused event keeps
  its original time with no fallback to the current event time.
- Any external_storage key on signal arguments or an applied value is
  external, even a null one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel
rmcdaniel force-pushed the fix/864-redrive-applied-updates branch from f44aee9 to c330f98 Compare November 3, 2026 22:04
@rmcdaniel

Copy link
Copy Markdown
Member Author

Acknowledged stop 6616550708. f44aee99 is replaced. Do not use it or its CI.

The cause was a validator scratch checkout that git add -A staged. It contained only copies of tracked public source; all 2,303 files exist in the base or main tree, and there were no credentials or env files.

New head c330f984b90d1a1995a5e281233fcde8cf712992:

  • Its parent is the held 1c17ed449225848e828e2094c4a65be487572829.
  • git diff --stat 1c17ed44 c330f984 shows exactly 3 files (+360 −31): CHANGELOG.md, src/V2/Support/FailedRunRedrivePlan.php and tests/Feature/V2/V2FailedRunRedriveTest.php.
  • Their content is byte-identical to those three files in f44aee99.
  • The PR now shows 10 files (+1088 −19) against main.

I've added the scratch path to the local git exclude and now stage only explicit paths. I'll post the full reply to the functional holds after the hosted matrix for c330f984 is terminal.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security readback of rebuilt exact head c330f984b90d1a1995a5e281233fcde8cf712992.

The exposure/hygiene incident is contained: its sole parent is the held 1c17ed44; the diff is exactly CHANGELOG, FailedRunRedrivePlan, and V2FailedRunRedriveTest; the scratch tree is absent. The discarded public-repo tree introduced no credential or non-public content, so no rotation is required and none is authorized.

The functional fixes are source-clean: terminal-closed and rejected update shapes are exactly bound, reuse provenance keys are paired and immediate-source-bound across update/signal chains, and any external_storage key is external even when null.

One hygiene prevention remains a merge hold: the validator deliberately creates .regression-corpus-consumer-* under the repository root, but .gitignore does not exclude it. Add .regression-corpus-consumer-*/ (or move the temporary directory outside the repository) with a regression check that a consumer run cannot appear as untracked/committable source. Then rerun full hosted CI on the new exact head. No CI evidence from f44aee99 carries.

The validator builds its official consumer tree under the repository
root so it can hard-link vendor. A run killed before cleanup left that
tree as untracked, committable source. .gitignore now excludes
/.regression-corpus-consumer-*/, the prefix is one named constant, and
the policy suite runs a real consumer and checks that git lists the tree
as ignored, never untracked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Nov 3, 2026

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security source review of exact 173424d7449df6b11670d99e2275da3c7c114e2e.

The hygiene fix closes the prior contamination path:

  • the root-only ignore pattern matches the validator's shared CONSUMER_TREE_PREFIX;
  • the policy test exercises the real consumer materialization under the checkout, proves the scratch tree is ignored rather than untracked, and proves cleanup;
  • bytecode writes are disabled during dynamic validator loading, closing the additional __pycache__ artifact;
  • removing the ignore rule is a meaningful counterfactual.

This head is source-clean. Merge clearance waits only for full hosted CI bound to this exact SHA; no evidence from discarded f44aee99 carries.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Addressed Security's holds 6615950950 and 6616718073. New head 173424d7449df6b11670d99e2275da3c7c114e2e. Its parent is the clean c330f984, which itself sits on the held 1c17ed44.

  1. Closed and rejected updates are validated as full shapes. A new helper, sameUpdate(), is shared by all three lifecycle shapes. It requires a nonblank update_id, workflow_command_id and update_name that are identical on every event. It also requires each event to be bound to the source provenance with the same workflow_run_id and reused_from_run_id.
    • [UpdateAccepted, UpdateCompleted] additionally needs a nonblank string terminal_reason and a nonblank failure_id, and no result or sequence key. That is the shape PendingUpdateCloser writes.
    • A lone UpdateRejected gets the same identity and run binding before it is skipped.
    • Anything else refuses with mismatched_update_history.
  2. Reuse provenance is a pair, with no timestamp fallback. hasReuseProvenance() requires reused_from_run_id and reused_recorded_at to be either both absent (key presence), or both present. When present, the source must be the immediate redrive source and the time a nonblank, parseable string.
    • boundToRun() calls it for every update event and the signal receipt.
    • The signal wait and application, which carry no run id, are now checked with it too. All three signal events must share the receipt's reused_from_run_id.
    • An event recorded in this run must have a recorded_at.
    • successorEvents() now takes reused_recorded_at only by key presence, through originalRecordedAt(). A missing original time can no longer be replaced by the current event time.
  3. External-signal boundary. Signal arguments and applied values now use holdsExternalPayload() (array_key_exists), so external_storage: null is external. isExternalResult() remains only for activity and side-effect results, which this PR doesn't change.

Counterfactuals. All of these refuse without mutation (an engineSnapshot of 15 engine tables plus current_run_id, compared before and after):

  • testRedriveSkipsOnlyAnExactlyClosedOrRejectedUpdate: 17 cases, including positive controls for an exact closed update and an exact rejection. Covers:
    • a null or blank terminal reason, a result alongside the terminal reason, a step sequence, and a missing failure id;
    • a foreign run, instance, command or handler, and a null or blank command or handler;
    • a lone reuse time or a lone reuse source.
  • testRedriveRefusesABrokenReuseProvenancePairWithoutMutation: 48 cases over every update and signal event (Accepted, Applied, Completed, WaitOpened, Received, Applied):
    • on the source run, a lone reuse time and a lone reuse source;
    • on a redriven run (A→B, with a positive control that unmodified B is eligible), a missing, null or blank time, and a missing, null or foreign source.
  • Two new signalStepMutations cases: external_storage: null on SignalReceived.arguments and on SignalApplied.value.

49 of these fail against 1c17ed44's plan. The other 24 were already refused through the run-id binding and now guard regressions.

  1. Validator scratch hygiene (173424d7, 3 files):
    • .gitignore now excludes /.regression-corpus-consumer-*/.
    • The validator's prefix is one constant, CONSUMER_TREE_PREFIX. The tree stays under the root because it hard-links vendor.
    • The new ConsumerScratchTreeTest runs the real _consumer_result() with a probe file. From inside the tree, it lists git's untracked and ignored files and asserts that the tree appears only as ignored. A positive control proves it was under the root, and a final check confirms it was removed afterwards. A second test runs git check-ignore --no-index on a nested source path under the prefix.
    • Loading the validator sets sys.dont_write_bytecode, because the test exposed scripts/ci/__pycache__ as another untracked artifact.
    • Counterfactual: with the ignore rule removed (and my local exclude removed too), both tests fail, and the untracked listing shows .regression-corpus-consumer-…/src/ConsumerScratchProbe.php. The full policy suite passes 70/70, and git status --untracked-files=all is clean afterwards.

Local results: V2FailedRunRedriveTest 106/106 OK, redrive corpus scenarios OK, ECS and PHPStan clean on the changed files. Hosted matrix on this exact head: 16 pass, 7 skipped, 0 failing, merge state CLEAN.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security final exact-head clearance: merge 173424d7449df6b11670d99e2275da3c7c114e2e only.

The functional review remains satisfied from the clean replacement lineage:

  • update history is accepted only through the exact three-shape state machine;
  • identity, arguments, inline-codec availability, external-storage refusal and success-shape constraints fail closed before durable mutation;
  • immediate-source and original provenance are explicit across update and signal redrive chains;
  • the service fence includes applied updates;
  • the malformed-shape and A→B→C counterfactuals assert unchanged engine tables and projections.

The accidental consumer copy is not part of this head. The hygiene fix is also sufficient:

  • /.regression-corpus-consumer-*/ is root-ignored;
  • the validator uses one named scratch prefix;
  • the policy test runs the real consumer-tree path and proves it is ignored rather than untracked, with a control that fails without the ignore rule;
  • bytecode writes are disabled for the loader that exposed the secondary artifact.

The exact head is mergeable and terminal with 16 successful checks, 7 intended conditional skips, and no failure or pending check. The discarded contaminated head contributes no CI authority; no secret rotation was required because its copied blobs were already-public repository content.

Return for review if the SHA changes. After merge, read back the parents/tree and delete the branch.

@rmcdaniel
rmcdaniel merged commit 32e90d2 into main Nov 3, 2026
23 checks passed
@rmcdaniel

Copy link
Copy Markdown
Member Author

Merged on Security's clearance of exact head 173424d7449df6b11670d99e2275da3c7c114e2e (6617107139).

  • Merge commit: 32e90d2376e055efee4f3fd34e68042700aaa178. Its parents are prior main 28893f97 and 173424d7.
  • The merge tree equals the cleared head's tree (b978ef40), so there's no file delta.
  • origin/main is 32e90d23.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant