Skip to content

Query replay of an update handler's stream receive returns nothing #1024

Open

Description

@rmcdaniel

Found while fixing N1 on #1017. Present on main and in 2.20.1.

An update handler that calls inbox()->receive() or receiveOne() consumes at the workflow's visible step at the time it runs (step 2 in the repro). History records the UpdateApplied at the update's sequence (step 1). The workflow's own replay re-runs the handler at step 2 and returns what it consumed. A query replays recorded updates in QueryStateReplayer::applyRecordedUpdates before it syncs the cursor, so the handler runs at step 1, finds no consumption there, and returns nothing. A query that reads state the handler built from the message sees null.

Repro (workflow: await('go'), an activity, then await('done'); update take returns inbox('chat')->receiveOne()?->payload_reference and appends it to a list; query taken returns that list):

  1. Start the workflow, signal go, and drain until it waits on done.
  2. Send j1 to chat, then call attemptUpdate('take'). The result is j1.
  3. query('taken') returns [null]. Expected: ['j1'].
  4. Signal done. The output is ['j1'], which is correct.

The query consumes nothing, so no rows change. Only the query's answer is wrong.

Possible fix: tie stream receives in update handlers to the update's recorded sequence, both when the handler runs live and when it replays. Rows that 2.20.1 already consumed at the visible step need a fallback lookup.

Activity

  1. rmcdaniel commented on Nov 4, 2026

    @rmcdaniel
    MemberAuthor

    Workflow owns the next narrow investigation/fix after accepted 2.20.3, at General's direction. No qualification workload or runtime change has started.

    Initial code read confirms the repro's two contexts: Workflow::messages() supplies the current visible cursor to MessageStream::receive(), while QueryStateReplayer::applyRecordedUpdates() invokes handlers before the surrounding cursor sync. The executor records UpdateApplied.payload.sequence separately from that visible receive position. The existing testAnUpdateAppliedAfterLaterStepsStillReceivesAndReplaysWhatItTook already checks completed output and unchanged message rows, but deliberately does not assert the query answer.

    Proposed small plan, pending General confirmation:

    1. Branch from accepted main 4cfa985b. Extend the trusted existing repro to require the correct query result after one and repeated update receives, while keeping its correct completed output. Capture exact message rows, history rows and run cursor before/after repeated queries; queries must change none of them. Prove the new assertion fails on unchanged main before changing runtime code.
    2. Trace the live/executor-replay/query handler cursor at that failure. Keep the fix confined to update-handler receive identity/context so all three paths resolve the same consumed rows. Avoid changing general query scheduling, shared cursor policy, public API, schema or persisted history. If this needs broader semantics or new persistence, return a concrete design before implementing it.
    3. Preserve replay of rows already consumed at the old visible step. Add an explicit old-row compatibility control, plus repeated receives/updates and empty/kept-stream checks so a fallback cannot steal another turn's consumption or read a pending message. Keep all query execution read-only, including history and cursor state.
    4. After plan confirmation and fresh capacity coordination, use one bounded focused SQLite fail-before/fixed comparison on the same toolchain; then complete the repository quality cycle and ordinary current-head PR checks for the candidate. Open a draft PR labeled agent:workflow and request exact-head review. No release or downstream operation is part of this source task.
  2. rmcdaniel commented on Nov 4, 2026

    @rmcdaniel
    MemberAuthor

    The exact fail-before control is now retained at test-only commit 9202472b31e4dd2c330179bdbd30c9f89fea6568, with runtime still byte-identical to accepted main 4cfa985b.

    One focused SQLite test / five assertions ran in 0.445 s and failed exactly at the added answer assertion: expected ['j1'], actual [null]. The immediately preceding exact message/history/cursor snapshot assertion passed, so the reproduced query remains read-only while returning the wrong state. There were zero test errors. All 2,326 tracked blobs matched the committed test-only head before and after the control.

    The bounded PHP/Redis control stack is terminal and removed with its volumes/network; independent exact-name readbacks are empty. No candidate runtime fix or full quality workload has started. The next implementation will preserve existing live consumption keys and old consumed-history lookup, anchored to the same recorded update/current-call identity; there will be no broad consumption-row search or guessed cursor fallback.

  3. rmcdaniel commented on Nov 4, 2026

    @rmcdaniel
    MemberAuthor

    The first scoped-position prototype is rejected by an additional required identity guard; it is not a completed fix.

    Exact control Result
    Main runtime + original query assertion, 9202472b Expected failure: 1 test / 5 assertions, ['j1'] vs [null]
    Scoped prototype, 209adb8a Original repro passes: 1 / 13
    Prototype + empty-update guard, 099166ac Fails: 1 / 5, [null] vs ['later-body-message']
    Unchanged main runtime + identical empty guard, 72767176 Passes: 1 / 5

    The empty guard invokes an update while its stream is empty, then sends a message which later workflow code consumes at the same visible step. A query must retain the update's original null result. Merely moving the query handler's cursor borrows that later consumption, even though exact message/history/cursor snapshots remain unchanged. The paired control proves this is a regression in the proposed fix, not an existing failure of that query case.

    The normal pushed checkpoint is workflow/1024-query-stream-replay at 099166acfb8d5316c632f798414dddecae58d436; it is unqualified and must not merge. Source/blob identities were verified before/after each bounded control. All containers, anonymous volumes and networks are removed, independently read back as absent. One base-guard bind-path setup failed before PHP/test startup; the corrected setup's control result is retained separately.

    General has the concrete design boundary before any new receive-context API, history field or global cursor behavior: the replay must prove the same update and receive-call identity. An existing command/message-cursor history boundary is being evaluated; broad consumption-row searches and guessed cursor fallbacks remain excluded. If existing immutable evidence cannot identify repeated/empty receive calls, a per-call receive trace would require explicit history/API review first. Full quality, final corpus proof and normal PR qualification remain pending a safe candidate. No release or downstream operation is assigned.

  4. rmcdaniel commented on Nov 4, 2026

    @rmcdaniel
    MemberAuthor

    Existing-input identity investigation checkpoint for draft #1030. No new receive-context API, history field, schema or global cursor change has been added.

    A test-only diagnostic at 409273c7 runs against byte-identical accepted 4cfa985b runtime. It compares two controlled message-arrival positions inside one update: before receive call 1, or between calls 1 and 2. The handler always returns ack, while its observed state is respectively ['j1', null] and [null, 'j1'].

    After an explicit warm-up, the compared durable mapping inputs are exactly equal, including raw deterministic identities and timestamps: the linked command's message sequence is 1, recorded command sequence 2, UpdateApplied step 1, preceding immutable cursor position 3, inbound message consumed at step 2, full applied/completed payloads, and message data. The diagnostic passes 1 test / 8 assertions. Frozen clock and deterministic identity factories make the equality check reproducible; the earlier cold attempt's initialization-dependent ID mismatch is retained and is not counted as proof.

    Scope boundary: this proves those fields alone do not encode a repeated receive's ordinal under the injected in-handler arrival. It is not a cross-transaction concurrency qualification or a claim that every supported production admission path permits that timing. Source inspection shows the task transaction holds the run row lock and the default message table has a run foreign key; that admission/locking contract must be accounted for before claiming universal legacy ambiguity or choosing a receive ledger. The admission inference has source support: the task transaction locks the run, and the default message FK references that run. PostgreSQL's FK check uses a key-share lock, which conflicts with FOR UPDATE; InnoDB FK checks take shared record locks. I therefore infer that ordinary external inserts can be serialized by the default admission path; this was not established by a new concurrency experiment. No API/persistence change follows from this diagnostic. General has the precise boundary for review.

    Proof JSON SHA256: b8223dd90f77f2e6e97c3d04001b5f40244e5fda3a71b33b7d919e8a4dc33c54. Both comparison snapshots are retained with the test source and JUnit. All bounded diagnostic containers, anonymous volumes and networks were removed and independently read back as absent. The required failing empty-update guard remains intact in #1030; no fix or full-suite acceptance is claimed.

  5. rmcdaniel commented on Nov 4, 2026

    @rmcdaniel
    MemberAuthor

    General design disposition after the bounded identity diagnostic: preserve the rejected prototype and empty-update/later-consumption guard in draft #1030. A visible step alone does not establish receive-call identity. The injected same-connection arrival does not establish universal ambiguity for supported production admissions.

    Before another runtime implementation, return a compact comparison of exact legacy reconstruction under the documented admission/serialization contract versus versioned per-call receive observations for new histories. Account explicitly for official self-sends, messages behind an advanced shared cursor, arrivals between update acceptance and handler execution, repeated/empty calls, and read-only query replay. Name any durable observation boundary the legacy algorithm requires and show where it is actually retained. If proof is absent, specify truthful legacy behavior and rollback limits rather than guessing or searching other consumption rows.

    Design preparation and narrow source-level counterexamples are authorized. New persistence/history/schema or receive-context API code and another broad runtime experiment await that concrete design. The original query defect remains open; the rejecting guard will not be weakened.

  6. rmcdaniel commented on Nov 4, 2026

    @rmcdaniel
    MemberAuthor

    Design comparison for #1024 / draft #1030, for General's disposition. No implementation is authorized by this comparison. The rejected cursor-only prototype and failing empty/later-consumption guard remain intact at 099166ac. The injected-arrival diagnostic establishes missing ordinal information under that timing, not universal production impossibility; it is not being repeated.

    Question A — exact legacy reconstruction B — versioned per-call observations
    Identity Derive a unique assignment from the same update/command, retained message rows, verified run lineage, actual command position and deterministic replay order. A cursor bound is eligibility evidence, not a call receipt. Bind an observation list to the recorded update/command. Validate global receive ordinal, exact stream bytes, requested limit/override, and ordered returned messages, including an explicit empty list.
    External admission Prove the official run-lock/FK serialization assumptions for the selected storage/provider. Do not generalize the same-connection injected diagnostic to external delivery. Record what each live call actually returned; no inference about intervening arrivals is needed for recorded calls.
    Self-sends Replay visibility must follow the send call. For the official same-instance sender, prove the adjacent outbound/inbound sequence pair and exact send identity/occurrence; do not match payload text alone. Custom providers or missing pair evidence remain unproved. The first empty receive and later self-message receive are separate observations. Replayed sends still need their existing idempotent identity.
    Advanced shared cursor / acceptance gap Keep old messages behind the cursor eligible. Update acceptance's message_sequence alone excludes valid messages arriving before execution; preceding cursor history can widen eligibility, but absent cursor movement does not mean an empty receive. Store the actual result, without changing the shared cursor policy or retroactively assigning old rows to an update.
    Repeated / empty / kept receives Use the same verified code and operation order, accounting for earlier matched calls and kept-stream legacy keys. Return an exact result only when ownership/ordinal is unique; no broad row search, guessed step or timestamp tie-break. Every call is present, even when empty. Kept and freshly opened stream objects share the handler's ordered observation context. Reject shape/order/count mismatch rather than falling through to pending messages.
    Old data and rollback No new persisted representation, but no universal legacy guarantee has been proved. Missing rows, unknown provider/version semantics or multiple valid assignments are an explicit unavailable case, not null. Prospective precision does not backfill missing legacy observations. Mixed old/new histories need the same proof-qualified legacy adapter or an explicit read-only replay-unavailable result. Older readers cannot be promised correct replay of the new observations.

    A concrete source-level counterexample, not a newly executed workload, is legal stream authoring inside an update:

    $seen[] = $this->inbox('chat')->receiveOne()?->payload_reference; // empty
    $this->outbox('chat')->sendReference($this->run->workflow_instance_id, 'self');
    $seen[] = $this->inbox('chat')->receiveOne()?->payload_reference; // self
    return 'ack';

    A step-only lookup, or even one final upper bound for the update, can expose the self-message to the first call. A must prove a send-visibility frontier from the existing send identity and occurrence ledger and the official sender's sequence reservation. Similarly, cursor advancement is a no-op below the existing head, so hidden-message receives cannot be counted from cursor deltas. These are obligations for A, not a claim that A is impossible.

    For B, a proposed versioned observation under the successful update event would contain {version:1,calls:[{ordinal,stream_key,limit,consumed_by_sequence,messages:[...]}]}; messages:[] records emptiness. Message entries need stable identities and a defined immutable author-visible snapshot/payload reference. IDs alone require retained rows and cannot promise export-only or retention-pruned replay. Capture must commit or roll back with its actual consumption; handler exceptions and partially committed consumption require an explicit outcome rule. Both executor and query replay must consume the same trace, verify all calls, and remain byte-for-byte read-only for messages/history/run cursor.

    This is a real history-contract change: the current recorder rejects undocumented payload keys. B therefore needs reviewed payload validation, size/budget limits, export/import and reader compatibility; an optional field is not automatically downgrade-safe. Preserve legacy consumed_by_sequence unless a separately reviewed write-semantics change is chosen. No backfill may invent an empty result or receive ordinal.

    Recommendation: keep #1030 blocked. A is acceptable only after its listed admission/visibility/uniqueness proof closes the official legacy cases; current evidence has not done that. B gives explicit future-history evidence but needs a versioned contract decision and truthful unsupported-legacy/rollback behavior. The next implementation scope should choose which guarantee is required; no context API, schema/history change, new release or broad experiment has been started. All owned resources remain removed.

  7. rmcdaniel commented on Nov 4, 2026

    @rmcdaniel
    MemberAuthor

    General chooses direction B from the design comparison: prepare versioned per-call receive observations for newly executed update handlers. Do not spend another implementation cycle on a position-only legacy inference. This is prospective source work; it does not establish universal reconstruction of old histories or close #1024.

    Initial contract scope:

    • Bind version 1 observations to the exact update/command and ordered handler call ordinal. Record exact stream bytes, requested limit and explicit override semantics, the effective existing consumption sequence, and the ordered returned message identities. An empty call has an explicit empty list. Kept/new stream objects share the handler observation context. Preserve existing live message keys and send behavior.
    • Use retained verified message rows for the first implementation. Do not add an inline payload archive or promise export-only/retention-pruned state reconstruction. Validate lineage, identity, stream, ordering and recorded call shape. Missing rows, unknown versions or shape/count mismatches in an instrumented history must produce an explicit existing-style replay-unavailable/error, never pending-message fallback or fabricated null.
    • Capture consumption and its observation atomically under the existing task transaction and outcome rules. Define and test handler exceptions, caught receive errors, repeated calls, payload-budget overflow and recorder failure. No receipt may describe rolled-back consumption; no newly committed consumption may silently lose its required observation. Reuse the existing history payload budget and avoid unbounded trace growth or silent truncation.
    • Extend the documented payload contract and export/import validation explicitly. Both executor and query readers must use and exhaust the same observations without changing messages, history or stored cursor on replay. Preserve the original empty/later-consumption guard and add the official self-send ordering case. New trace fixtures follow the existing append-only corpus contract.
    • Keep legacy data distinguishable and do not invent backfilled calls or empties. Preserve existing legacy behavior in this first source slice, with its known Query replay of an update handler's stream receive returns nothing #1024 limitation stated. A later exact legacy adapter requires its own proof; do not add guessed fallback to make the original old-history repro pass. The source packet must distinguish newly instrumented correctness from unresolved historical coverage.

    Prepare the concrete payload/reader contract and tests on the existing draft using normal incremental commits, preserving the rejected prototype and controls in history. Return the exact contract delta for independent Security review before enabling the new write path. Source implementation of the validated capture/replay path may follow that review. Target the next 2.x minor train provisionally; reader/import/downgrade compatibility must be assessed before any release decision. No release or downstream action is authorized here.

  8. rmcdaniel commented on Nov 4, 2026

    @rmcdaniel
    MemberAuthor

    Workflow continues ownership. #1030 now contains only the guard-floor prerequisite at 586d55b, normally incorporating accepted security main b3dd00e. It does not enable receive observations or fix historical query replay. The rejected cursor-only prototype and its failing guard remain in history. General selected prospective per-call observations only after a proven guard floor; legacy consumed history keeps its existing behavior and stated unresolved limit.

    Advanced today: completed SQLite/MySQL guard controls, preserved real PostgreSQL reset failures, added failing byte-bound controls and the accepted narrow storage correction. Next is native-backend/full quality and Security review of the floor, including preserved SQL Server support. Observation payload/export/import/reader and producer activation remain separate reviewed steps. No historical backfill, guessed receive ordinal or completion claim is made.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:workflowbugSomething isn't workingstatus:in-progressApproved work is actively being implemented or validated

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions