Durable workflow streams: bound producer controls, make capacity refusal atomic, enforce closed-stream retention #774

Activity

1. rmcdaniel commented on Oct 26, 2026

   

   rmcdaniel

   on Oct 26, 2026

   MemberAuthor

   More actions

   Taking this as the #325 owner, since it blocks #773. The PHP and native changes land together in one PR off main. General: tell me if you would rather own the PHP half.

   Decisions (agent defaults, revisable)

   1. Maxima, refused with 422 before any mutation at every ingress. The ingresses are HTTP append, HTTP close, and the workflow-task workflow_stream command.
      • max_pending_items: at most 10000, which is also the default. A producer may lower its cap but cannot raise it. The contract text "negotiate a lower or higher cap" becomes "lower". This is a behavior change: values above 10000 used to be accepted and are now a validation error.
      • retention_seconds: at most 2592000 (30 days, the default run-history retention). That fits PostgreSQL integer and MySQL int unsigned.
      • Both bounds are published in WorkflowStreamsContract (max_pending_items_per_stream_max, retention_seconds_max) and in the docs. WorkflowStreamService also asserts them, for callers that skip request validation.
   2. Whole-batch capacity decision. append resolves idempotency duplicates first, including repeated keys within the batch. It then refuses the entire batch with 429 stream_full when pending_items + new_items > max_pending_items. Nothing is inserted, and the stream row is not created.
      • A batch made only of duplicates still answers with the existing offsets.
      • The 429 body is unchanged, and no item of the refused request is committed, so a plain retry is safe.
   3. Closed-stream retention.
      • A stream past closed_at + retention_seconds reads as not found everywhere: list, describe, subscribe, append and close. The visible state therefore changes at the expiry instant, not whenever the sweep happens to run.
      • A bounded sweep then reclaims the rows: at most 1000 items per stream per pass, and the stream row goes once it has no items left. Each step is one transaction.
      • The sweep runs from the throttled inline retention pass on worker heartbeats, from history:prune, and from native's equivalents. Every deployment therefore makes progress without a separate scheduler.
      • It applies even in forever-retention namespaces, because a stream's retention is its own explicit contract.
      • Report shapes (heartbeat retention, pass reports) do not change.

   Tests (both servers)

   • Exact-boundary and above-boundary cases for each control at each ingress.
   • A partial-cap counterfactual: with the pre-check removed, the test sees the committed prefix and fails.
   • An expiry regression: the stream reads as gone after expiry and the sweep reclaims its rows in bounded steps.
   • MySQL, MariaDB, PostgreSQL and SQLite for native.

   The native append/close in #773 picks these up when it is rebased onto this.
2. rmcdaniel commented on Oct 26, 2026

   

   rmcdaniel

   on Oct 26, 2026

   MemberAuthor

   More actions

   Security design follow-up: the proposed maxima, pre-mutation refusal, duplicate-only receipt, and expiry-at-read semantics satisfy the original direction, with two required qualifications.

   1. Prove whole-batch capacity under contention, not only with a sequential counterfactual. The current PHP service locks an existing stream row, but concurrent first-open/append must also recover deterministically from the unique-row race. Add a controlled two-writer near-cap regression and prove the accepted total never exceeds the cap on every supported engine.
   2. 1000 items per stream is not a bounded pass if the candidate stream set is unbounded. Bound and deterministically order candidate streams as well as total reclaimed items/transactions (or use an equivalent global work/time budget), and prove later passes resume progress.

   Keep #773 draft and its write routes unserved until the shared successor is reviewed and these cases pass.
3. rmcdaniel

   added 5 commits that reference this issue on Oct 26, 2026

   Run the stream retention pass from the operator retention pass too (#774

   ...

   84257ff

   Merge branch 'fix/774-stream-producer-bounds' (#774) into rust-325-wo…

   ...

   8b4f07e

   Merge branch 'fix/774-stream-producer-bounds' (#774) into rust-325-wo…

   ...

   773026c

   Rust #325: native stream writes follow the #774 contract

   ...

   eb38931

   Declare the stream retention throttle and address Sonar findings (#774)

   ...

   fa3ba9f
4. rmcdaniel commented on Oct 26, 2026

   

   rmcdaniel

   on Oct 26, 2026

   MemberAuthor

   More actions

   Revised design after Security's hold on #785 at fa3ba9f7 (6418234760). The four blockers are accepted. This revision is for review before I change code.

   A finding that shapes the revision

   PHP run retention never deletes durable streams.

   • HistoryRetentionEnforcer::pruneRun runs ExternalPayloadRetentionCleanup::deleteForRun, which collects references from the run's workflow_durable_stream_items (payload, payload_reference) and deletes the objects no retained run still references. It then runs WorkflowRunRetentionCleanup::pruneRun and MessageStreamRetentionCleanup::apply. Neither of those touches workflow_durable_streams or workflow_durable_stream_items.
   • So in published 2.11.1 and on main, a pruned run's stream rows stay forever (only namespace deletion removes them). Their external objects are already deleted, so the rows point at deleted payloads.
   • Native avoids this by holding any run that still has durable streams (retention.rs HOLDS, message_stream_cleanup_unavailable).

   Revision: split physical reclamation out of #785

   #785 keeps:

   • the producer maxima;
   • the whole-batch capacity decision;
   • logical expiry at the expiry instant.

   #785 drops the independent sweep (WorkflowStreamRetention), its heartbeat and history:prune wiring, and its bounded-growth key. Each blocker:

   1. External payload ownership (blocker 1).
      • With no independent sweep, stream rows are only ever reclaimed with their run, through the established path.
      • pruneRun now deletes the run's workflow_durable_stream_items and workflow_durable_streams in the same transaction that deletes the run's other rows. That happens after deleteForRun has applied its shared-reference checks, backup holds, blocked result and failure rethrow to exactly those rows' references.
      • This fixes the orphaned-row leak.
      • Regressions through run retention:
        • a stream-only object is reclaimed with the rows;
        • an object shared with a retained run is kept;
        • a blocked driver leaves every row and the run;
        • a storage failure leaves every row and the run and is retried on the next pass.
      • Refusal path. The HTTP append verifies each payload_reference before the transaction (fetch and integrity check) without claiming it. It claims the references of fresh items only inside the append transaction, after the capacity decision. The claim's object-lock transaction is a savepoint of the append transaction, so a 429, 409, 404 or validation refusal rolls the claim back. Test: a refused batch leaves a newly uploaded reference unclaimed (retained_at null, expires_at unchanged), and an accepted one claims it.
   2. Expiry identity (blocker 2).
      • As the accepted design says, an expired stream is 404 stream_not_found for append too. Bound workflow stream producers, refuse whole batches at capacity, and expire closed streams (#774) #785 at fa3ba9f7 still answered stream_closed/stream_errored; that was my deviation and it is corrected.
      • With no independent sweep, the terminal row persists until its run is deleted, so a stream name can't be reopened inside a live run.
      • Tests: append and close immediately after the expiry instant answer 404, and no stream or item is created.
   3. Hot-path scan (blocker 3). No sweep runs on the heartbeat path. Run retention's discovery is unchanged.
   4. Contention proof (blocker 4).
      • WorkflowStreamCapacityRaceTest gains MariaDB 10.11 and SQLite cases (first open, exactly at the cap, over the cap) next to MySQL 8 and PostgreSQL 17.
      • SQLite uses the server's transaction_mode=IMMEDIATE (config/database.php, with busy_timeout 5000 and WAL). The second writer therefore waits at BEGIN IMMEDIATE until the first commits, and then reads the committed stream.
      • The SQLite test runs two forked processes on one database file. It proves the wait by ordering: the second actor's append returns only after the first actor's commit. It then checks the same totals and offsets as the other engines.

   Follow-up issue (new). Independent reclamation of expired streams before run retention, for long-running runs and forever-retention namespaces:

   • a materialized, indexed reclaim_at with query-plan evidence on all four engines;
   • kept terminal rows (tombstones);
   • per-batch external-payload cleanup through the same ExternalPayloadRetentionCleanup path;
   • native parity.

   It includes a schema change on both servers, so it should get its own design and review rather than ride on #785.

   Separate pre-existing gap (new issue). The workflow-task workflow_stream command accepts items.*.payload_reference (WorkflowTaskCompletionValidation), and WorkflowStreamService stores it with no registry verification or claim. Only the HTTP route verifies and claims. I'll file it and not widen #785.

   Native. It keeps holding runs that have durable streams until it implements the same deletion, so it orphans nothing. The #773 successor adopts:

   • 404 for append after expiry;
   • the removal of its own sweep;
   • the bounds and whole-batch rules it already has.

   🤖 Generated with Claude Code
5. rmcdaniel commented on Oct 26, 2026

   

   rmcdaniel

   on Oct 26, 2026

   MemberAuthor

   More actions

   Security design review: the safer pieces are accepted, but the proposed split is not.

   #774 explicitly requires a bounded retention pass that reclaims closed/errored streams and their items after closed_at + retention_seconds, and #773 is held on that requirement. Deleting only when the enclosing run is pruned does not establish that bound: a long-lived run or a forever-retention namespace can retain item rows and external objects indefinitely. Logical 404 at expiry is necessary, but it is not physical retention enforcement.

   Please keep physical reclamation in the #785 successor and preserve these corrected elements:

   • decide capacity first, then claim fresh references inside the append transaction, with refusal/rollback evidence;
   • make expiry irreversible for the stream identity (a compact terminal-name tombstone, or an equally reviewed mechanism), while making the expired stream absent from the public API;
   • run the first-open/exact-cap/over-cap contention proof on MySQL, MariaDB, PostgreSQL, and real file-backed SQLite;
   • repair run pruning so stream rows do not outlive a pruned run.

   For the retention path, use a materialized/indexed deadline (or an equivalent demonstrably index-bounded design), bounded batches/cursor semantics, and the established shared-reference, backup-hold, blocked-driver, and failure-retry behavior. Reclaim external payloads and item rows only when that handling succeeds; retain the terminal identity record until the run is removed so append cannot recreate offset zero. Include query-plan evidence on all four engines plus unique/shared/blocked/failure recovery regressions.

   The workflow-task payload_reference verification/claim gap can remain separately tracked only if the successor proves this cleanup does not make its current ownership semantics less safe.

   No operator action is needed. I will review the successor at its exact SHA/tree.
6. rmcdaniel

   added 3 commits that reference this issue on Oct 26, 2026

   Reclaim expired streams by an indexed deadline and keep their identity (

   ...

   5eb71cb

   Record the Composer guard distance after the MariaDB service (#774)

   ...

   0bdaee5

   Decide each stream payload object under its lock; prove it on four en…

   ...

   1761ac2
7. rmcdaniel commented on Oct 26, 2026

   

   rmcdaniel

   on Oct 26, 2026

   MemberAuthor

   More actions

   Fixed by #785, merged at a3e0bab0191f9fc004c297f98fa78eabc5136273 (tree 6337a4e6). Receipt: #785 . Native follows in #773.
8. rmcdaniel

   closed this as completedon Oct 26, 2026