Skip to content

Redrive runs whose completed steps include an applied signal wait - #1002

Merged
rmcdaniel merged 5 commits into
mainfrom
fix/864-redrive-signal-waits
Nov 3, 2026
Merged

rmcdaniel merged 5 commits into
mainfrom
fix/864-redrive-signal-waits

Conversation

@rmcdaniel

Copy link
Copy Markdown
Member

A2c, first slice, of #864. Stacked on #1001 (A2d). The base is that branch, so this diff shows only the signal work. GitHub retargets it to main when #1001 merges.

Problem

A failed run whose completed prefix included a signal wait (await('name') that received and applied a signal) was refused with unsupported_history. Waiting on a signal before a later activity is a common shape: approve, then process.

Change

  • Plan. SignalWaitOpened, SignalReceived and SignalApplied are grouped into one reused step at the wait's step sequence. The wait opened, the receipt (matched by signal_id) and the application (matched by signal_wait_id and name) must all be present.
  • Refusals.
    • unsupported_signal_wait: a wait with a timeout, cancellation scope or parallel group. Those carry a timer race or scope state that this slice doesn't reuse.
    • unsupported_pending_signal: a signal the source received but never applied. Redrive would otherwise drop it.
    • incomplete_signal_step: a wait without both its receipt and its application.
    • Step-order and duplicate checks are shared with the other reused steps.
  • Successor history. All three events are recorded in order with reused_from_run_id and reused_recorded_at. No signal row or command is recreated, so nothing is delivered again.
  • Replay time. WorkflowExecutor::resumeSignal, QueryStateReplayer and WorkflowFiberRunner signal outcomes use the reused time, so now() after the wait matches the source.
  • Message cursor. MessageCursorAdvanced is bookkeeping, not a step, so the plan skips it. Redrive now calls MessageStreamCursor::transferCursor(), as continue-as-new does, so a consumed message stays consumed for the successor.
  • Payload contract and docs/api-stability.md. The three signal events accept the reuse keys.
  • CHANGELOG Unreleased updated.

Not in this slice: updates in the prefix (still refused), timeout and scoped waits, and pending signals. The successor's run detail shows the reused signal from history only; the source keeps its workflow_signals row.

Tests

Local runs, PHP 8.4, SQLite + Redis:

  • V2FailedRunRedriveTest (16 tests), with three new tests:
    • Redrive across an applied signal: the successor completes with the signal value, the first activity doesn't rerun, the three events are reused in order with the source's times, there's no signal row on the successor, the message cursor is carried over, and workflow time after the wait matches the source in the live run, export replay and stored-history replay.
    • Refusal with unsupported_pending_signal (a buffered receipt the source never applied).
    • Refusal with unsupported_signal_wait (a wait with a timeout).
  • The redrive plan/rejection, payload-contract, surface-stability and replay-corpus unit suites pass. The only failure is the known local-only cache case, which also fails on main.
  • Replay corpus: new redrive-reused-signal-wait-clock fixture plus a scenario test. Against main, the validator reports replay counterfactual_fixture_paths: 2 (this fixture and Redrive runs whose workflow code upserted memos or search attributes #1001's).
  • ecs and phpstan are clean on the changed files.

🤖 Generated with Claude Code

#864)

A2c (first slice) of #864. A SignalWaitOpened, SignalReceived or
SignalApplied event in the completed prefix refused redrive with
unsupported_history.

- The plan groups a plain signal wait (no timeout, cancellation scope or
  parallel group) with its receipt and application into one reused step.
  The successor records all three with reused_from_run_id and
  reused_recorded_at, and the executor, query replayer and fiber runner
  resume the wait at the original time with the recorded value.
- MessageCursorAdvanced is bookkeeping, not a step. Redrive now transfers
  the source's message cursor, as continue-as-new does, so a consumed
  message is not delivered again.
- New refusals: unsupported_signal_wait, unsupported_pending_signal (a
  received signal the source never applied) and incomplete_signal_step.
  Updates in the prefix are still refused.

Adds feature coverage and replay corpus evidence
redrive-reused-signal-wait-clock.

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

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

Copy link
Copy Markdown
Member Author

Security pre-review of stacked draft head 6d8daf92edf8bdc88410f8dd7d21bf6d3c79412c: hold before ready-for-review on two integrity/availability gaps.

1. External signal payload references cross into the successor without ownership or a refusal

SignalReceived.arguments and SignalApplied.value are both produced through ExternalPayloads::historyValue(), so either can be an {codec, external_storage} envelope. signalSteps() currently copies both payloads unchanged, while the successor deliberately gets no WorkflowSignal row or signal command. That makes the successor depend on an external object/reference owned by the source path and lacks the explicit cross-run policy already enforced for external activity and side-effect results.

Fail closed with a specific refusal such as external_signal_payload_not_supported when either reused field contains an external-storage reference, or deliberately re-home the bytes under successor ownership and prove lifecycle/rollback/cleanup. Add mutation tests for externalized SignalReceived.arguments and SignalApplied.value; both must refuse without creating a successor or changing message-cursor state unless the re-home path is implemented and verified.

2. The reused receipt is not fully bound to the wait/application/source identity

The plan selects a receipt through the application's signal_id and compares only its signal_name. It does not compare the receipt's non-null signal_wait_id to the opened/application wait, does not require workflow_command_id equality between SignalReceived and SignalApplied, and does not bind the receipt's workflow_instance_id / workflow_run_id to the source being redriven. A malformed or imported history can therefore combine a receipt from another wait/command/run with an application and still be eligible.

Pass the source identity into the grouping validation and fail closed on those mismatches. If a null receipt signal_wait_id is a supported buffered-before-wait case, make that exception explicit and cover it; a non-null different id must refuse. Add deterministic mutation tests for at least wait id, command id, run id, and instance id, and prove no successor/cursor mutation on every refusal.

The positive path, pending-signal refusal, timeout refusal, replay clock, and cursor transfer otherwise read coherently. After these fixes and after #1001 lands, refresh onto main, preserve the new Unreleased entry, and return the full green exact head for review.

…payloads (#864)

Security pre-review of #1002 found two gaps.

- A reused SignalReceived must belong to the source run and instance,
  carry the same workflow_command_id as its SignalApplied, and name the
  same wait. A receipt recorded before its wait opened has no wait id,
  which is the only allowed difference. Mismatches refuse with
  mismatched_signal_receipt.
- An externally stored SignalReceived.arguments or SignalApplied.value
  refuses with external_signal_payload_not_supported, because the
  successor gets no signal row or command that owns the reference.

Mutation tests cover wait, command, run and instance mismatches and both
external payloads; each refuses with no successor, no history write and
no cursor change. A buffered receipt with a null wait id is accepted.

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

Copy link
Copy Markdown
Member Author

Addressed Security's pre-review hold (comment 6612174945). New head 73ce17d0b132572fd14a2f75fc10106594c8410a, still stacked on #1001.

  1. External payloads. SignalReceived.arguments or SignalApplied.value holding an external-storage envelope refuses with external_signal_payload_not_supported, using the same detection as external_activity_result_not_supported. Nothing is re-homed.

  2. Receipt binding. The plan now takes the source run. A reused receipt must have:

    • workflow_run_id equal to the source run,
    • workflow_instance_id equal to its instance,
    • workflow_command_id equal to the application's (non-empty),
    • the wait's name and signal_wait_id.

    Any mismatch refuses with mismatched_signal_receipt. The one explicit exception is a null receipt signal_wait_id: a signal that arrived before its wait opened.

Tests (V2FailedRunRedriveTest, 23 total):

  • testRedriveRefusesAMismatchedOrExternalSignalStepWithoutMutation has six cases: wait id, command id, run id, instance id, external receipt arguments, external applied value. Each asserts the typed reason, a single run on the instance, an unchanged current_run_id, an unchanged history-event count, and an unchanged source message cursor. All six fail on the previous source 6d8daf92.
  • testRedriveAcceptsASignalReceivedBeforeItsWaitOpened: a null receipt wait id is accepted and the successor gets the value.

ecs and phpstan are clean. The CHANGELOG lists the two new reasons. After #1001 lands I'll retarget this onto main, keep the entry under Unreleased, and return the exact green head.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security readback of the two pre-review holds: addressed in principle at 73ce17d0b132572fd14a2f75fc10106594c8410a.

  • Both external envelope locations are checked through the same stored-reference predicate used by activity/side-effect redrive and refuse with external_signal_payload_not_supported; no cross-run ownership is invented.
  • The selected receipt is now bound to the source run and instance, the application's non-empty command id, signal name, and wait id. A null receipt wait id is the explicit buffered-before-open case; a different non-null id refuses.
  • The six mutations cover wait, command, run, instance, receipt arguments, and applied value, and bind refusal to no successor/current-run/history/cursor mutation. The buffered null-id positive case is explicit.

No further finding from this incremental fix. This is not exact-head merge clearance: #1002 remains draft/stacked. After #1001 merges, refresh onto main, run the full hosted matrix and corpus counterfactual against the new base, and return that exact head for final review.

@rmcdaniel
rmcdaniel changed the base branch from fix/864-redrive-visibility-upserts to main November 3, 2026 18:35
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

# Conflicts:
#	CHANGELOG.md
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security full-review follow-up on 73ce17d0b132572fd14a2f75fc10106594c8410a: the two earlier holds are fixed, but two additional signal-integrity fail-opens remain. Keep this PR draft.

  1. One buffered receipt can satisfy more than one wait. signalSteps() indexes one receipt by signal_id, then each opened sequence may select that same receipt. For a receipt whose signal_wait_id is null (the deliberately supported buffered-before-open case), two distinct SignalApplied events can name the same signal_id/command/name but different wait ids and sequences. Both pass; $appliedSignalIds records only set membership and never refuses a second consumption. The successor then synthesizes one admitted signal as two workflow inputs. Bind each receipt/signal id to exactly one application (or refuse the duplicate). Counterfactual: add a second opened/applied wait that reuses the buffered receipt and assert refusal plus no successor, current-run, cursor, or history mutation.

  2. An application without its durable value is accepted and changes the embedded result. The planner requires a SignalApplied event but does not require its value. It copies that event into the successor. WorkflowExecutor::executeSignal() selects the applied event, and its signalValue() reads only payload['value']; it does not fall back to the copied SignalReceived.arguments. The successor therefore resumes with null, even though QueryStateReplayer contains an explicit committed-receipt fallback for this history shape. Fail closed unless the application carries the exact inline reusable value, or synthesize/verify one deterministically from the receipt. Counterfactual: remove value from the otherwise valid applied event while retaining receipt arguments; the present head accepts and can complete with a different workflow value. Require refusal/no mutation, or prove identical embedded and query results after a safe derivation.

These findings do not reopen #1001. After fixing them, resolve the current conflict by refreshing onto merged main 6185ea47, run the full hosted matrix and corpus fail-before evidence, and return the exact head.

@rmcdaniel
rmcdaniel marked this pull request as ready for review November 3, 2026 18:37
@rmcdaniel
rmcdaniel marked this pull request as draft November 3, 2026 18:38
…value (#864)

Security full review of #1002 found two more fail-opens.

- A buffered receipt (null wait id) could satisfy two distinct waits,
  turning one admitted signal into two workflow inputs. Each receipt now
  binds to exactly one application; a second refuses with
  mismatched_signal_receipt.
- A SignalApplied without its inline value was accepted, but the
  embedded executor resumes from that value only, so the successor would
  see null. A missing value now refuses with incomplete_signal_step.

Counterfactuals: a two-signal source whose second application reuses the
first buffered receipt, and an application without its value. Both are
accepted by the previous plan and now refuse with no successor, history
or cursor change.

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

Copy link
Copy Markdown
Member Author

Addressed Security's follow-up holds (comment 6612383151). New head 959512d61686cf03df8f78fc8a88cf56f5d61f7a, based on merged main 6185ea47 (#1001). The PR now targets main.

  1. One receipt per wait. Each signal_id binds to exactly one application. A second application of the same receipt refuses with mismatched_signal_receipt before it is recorded as used.
  2. Inline value required. A SignalApplied without value refuses with incomplete_signal_step. Nothing is derived from the receipt, so the embedded executor and query replayer can't diverge.

Counterfactuals, each accepted by the previous plan (73ce17d0) and refused now:

  • testRedriveRefusesOneBufferedReceiptAppliedToTwoWaits uses a new TestRedriveTwoSignalsWorkflow (two await('approve') waits). The unmodified source is asserted redrivable first. Then the first receipt is made buffered (null wait id), the second application claims its signal_id and command, and the second receipt is removed so nothing is pending.
  • testRedriveRefusesASignalApplicationWithoutItsValue: value is removed from an otherwise valid application.

Both assert the typed reason, a single run on the instance, an unchanged current_run_id, an unchanged history-event count and an unchanged message cursor.

Local on this head: V2FailedRunRedriveTest 25/25 and V2EmbeddedReplayRegressionCorpusTest 17/17. validate-regression-corpus.py --base-ref 6185ea47 passes with replay counterfactual_fixture_paths: 1. ecs and phpstan are clean. Kept as draft while hosted CI runs.

@sonarqubecloud

sonarqubecloud Bot commented Nov 3, 2026

Copy link
Copy Markdown

@rmcdaniel
rmcdaniel marked this pull request as ready for review November 3, 2026 18:56
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security exact-head merge clearance: 959512d61686cf03df8f78fc8a88cf56f5d61f7a, on base/main 6185ea473484c9677dee1ab08495a2d9f648ceb0.

I re-read the complete source and test delta after #1001 landed. All four Security holds are fail-closed:

  1. A stored/external envelope in either SignalReceived.arguments or SignalApplied.value is refused as external_signal_payload_not_supported; the redrive does not re-home an unowned reference.
  2. The receipt is bound to the source run, workflow instance, non-empty application command id, signal name, and wait id. Only a receipt buffered before a wait opened may have a null receipt wait id. Identity drift is refused as mismatched_signal_receipt.
  3. One admitted signal_id may satisfy exactly one application. Reusing a buffered receipt for a second wait is refused.
  4. SignalApplied must contain its inline value; replay never derives a missing application result from the receipt. An incomplete application is refused as incomplete_signal_step.

The normal path remains narrow: only completed plain waits are reusable; timeout/cancellation/parallel waits and pending receipts remain refused. The successor transfers the already-consumed message cursor, records the wait/receipt/application as reused history with source provenance and original time, and creates no signal row or command to redeliver. Executor, query replay, and fiber replay use the reused time consistently.

The mutation cases cover run/instance/command/name/wait identity, external storage, duplicate receipt consumption, and missing application value. Each asserts no successor, no current_run_id change, no history growth, and no cursor change, and each is a counterfactual against an earlier head. The valid two-wait source and buffered-null-wait-id exception are also exercised. Local evidence is 25/25 redrive, 17/17 corpus, plus the guarded replay counterfactual.

Hosted CI is fully terminal at this exact head: 16 pass, 7 conditional skips, including the regression corpus, bounded MySQL selection, unit contracts, tracing, quality, public-boundary, CodeQL, and Sonar. GitHub reports CLEAN/MERGEABLE against the exact current main.

This head is cleared to merge. Any source, test, base-conflict-resolution, or history-contract change requires a fresh readback.

@rmcdaniel
rmcdaniel merged commit 5025226 into main Nov 3, 2026
23 checks passed
@rmcdaniel
rmcdaniel deleted the fix/864-redrive-signal-waits branch November 3, 2026 18:57
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security merge readback complete: merge 502522663ca10a0d14362e325f496f0fb0d21426 has parents 6185ea473484c9677dee1ab08495a2d9f648ceb0 and accepted head 959512d61686cf03df8f78fc8a88cf56f5d61f7a; its tree 454084ca7e6c86d7e634f1306385a95b299636e2 exactly equals the accepted head's tree. origin/main resolves to the merge. The four Security holds are closed in the durable mainline outcome.

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