[rmcdaniel]

Addresses #774 and #786 (both closed by hand after the merge receipt).

Review tier: A. It deletes stored data (payload reclamation, run pruning) and changes write-path locking. Policy: #325 comment 6423175739.

Problem

Published Server 2.11.1 had three gaps in the durable workflow stream producer contract, found in Security's review of #773:

• max_pending_items and retention_seconds were caller-unbounded.
• A capacity refusal committed the accepted prefix of a batch and then answered 429.
• The documented closed-stream retention was never enforced.

Qualifying this PR turned up more gaps in published code, fixed here as well:

• Run retention never deleted a pruned run's durable streams. It deleted the external objects their items referenced but left the rows.
• SQLite transactions begin DEFERRED on the server's PHP 8.3, whatever DB_SQLITE_TRANSACTION_MODE says. Laravel applies the configured mode only on PHP ≥ 8.4.
• A refused HTTP append could leave a just-claimed reference retained with no item.
• Worker workflow_stream append items were stored without claiming their references (Workflow-task workflow_stream items store payload_reference without verification or claim #786).
• Payload cleanup's retained-reference check and its delete were separate decisions, so a writer could reference an object between them.
• On SQLite, the payload object lock read its row before writing it, so a concurrent payload operation failed at once (503) instead of waiting.

Design: #774 decisions, revised after Security's holds (6418234760, 6418469554, 6420503497).

Change

1. Maxima, refused before any mutation at every ingress.
   • max_pending_items is 1 to 10000 (also the default); a producer may lower its cap but not raise it.
   • retention_seconds is 1 to 2592000 (30 days).
   • Covered ingresses: HTTP append, HTTP close, and the workflow-task workflow_stream command (PHP and native completion rules).
   • WorkflowStreamService asserts both bounds for callers that skip request validation.
   • The streams contract publishes max_pending_items_per_stream_max and retention_seconds_max.
2. Whole-batch capacity. Idempotency duplicates take no capacity, including keys repeated in the batch. A batch that would pass the cap is refused whole with 429 stream_full inside the transaction, so nothing is written and no stream is opened.
3. Run-row serialization (WorkflowRunStreamLock).
   • Appends and closes take the run row's lock before reading any stream row, so concurrent first appends wait instead of racing.
   • On SQLite a no-op write takes the database write lock first, because a deferred transaction that has read cannot wait for a writer.
   • Streams are created through their model again, so the namespace stream quota (max_workflow_streams) applies.
   • An append to a run that is gone, or whose details retention has pruned, answers 404 stream_not_found.
4. One decision per external payload object, under its lock.
   • Writers claim inside their transaction. The HTTP append fetches and verifies each payload_reference before its transaction, without claiming it. The worker command's append (Workflow-task workflow_stream items store payload_reference without verification or claim #786) relies on the runtime transport, which already resolves each reference to a registration in the namespace. Both claim the references of the items they will write inside the append's transaction, after the capacity decision. The HTTP path uses claimVerified; the command path uses claimRegistered, which checks the registration again. Each claim holds the object's lock until the append commits, and a refusal rolls it back.
   • Cleanup asks again under the lock. Stream reclamation and run retention first scan for deletable objects. Then, for each object, deleteUnlessReferenced takes its lock and asks again whether any retained row references it before deleting it. Either the writer's reference commits first and the object stays, or the object goes first and the claim fails.
   • Fresh reads (ReadCommittedTransaction). That second check must see a reference committed while cleanup waited for the lock. MySQL and MariaDB default to REPEATABLE READ, whose snapshot predates the wait, so reclamation batches and run pruning run READ COMMITTED. PostgreSQL is set to the same, so a server default can't change it. SQLite runs one writer at a time. Inside a caller's transaction the level can't change, and the callback runs as it is.
   • One canonical order. RuntimeExternalPayloadObjectLock::inLockOrder sorts by bucket, then URI. A multi-reference claim and cleanup's deletes both lock in that order, so two transactions never wait on each other in opposite orders.
   • SQLite. The object lock takes the write lock with its no-op write before reading its row, so a concurrent payload operation waits its turn instead of failing.
5. Expiry is visible at the instant and irreversible.
   • Past closed_at + retention_seconds, a stream reads as not found for list, describe, subscribe, close and append (404 stream_not_found).
   • Its row stays until its run is pruned, so the name is never opened again at offset 0.
6. Indexed, bounded reclamation (WorkflowStreamRetention).
   • Deadline. A new reclaim_at is set at close (backfilled for terminal streams) and indexed (namespace, reclaim_at, id).
   • Discovery. Each pass reads one namespace's due streams in deadline order from that index and stops at its stream limit.
   • Batches. It deletes at most its item budget, in batches of 100. Each batch is one transaction under the run and stream locks. It first settles its items' external payloads through ExternalPayloadRetentionCleanup::deleteForDurableStreamItems. That treats the items as released and applies run retention's shared-reference, backup-hold, blocked-driver and failure handling. The items are deleted only after it succeeds.
   • Completion. A stream with no items left keeps its row with reclaim_at cleared.
   • Where it runs. The throttled heartbeat inline pass (10 streams, 500 items, once a minute per namespace; failures are logged), history:prune (--limit streams and --limit × 100 items per namespace), and the operator retention pass. It runs in every retention mode.
7. Run pruning removes streams, fenced first.
   • HistoryRetentionEnforcer::pruneRun is one transaction that takes the run lock before anything else. Payload cleanup, the run's row deletes and its stream items and streams follow. No append to the run can commit in between, and a later one finds the details pruned.
   • This replaces the earlier skip of a run whose stream grew after cleanup (durable_stream_appended_during_prune).
   • A failure marks the run's message-stream plan blocked and propagates.
   • The deleted report gains durable_stream_items_deleted and durable_streams_deleted.

Native. Native has no stream write routes on main (501), and its run retention holds any run with durable streams, so it neither deletes nor orphans stream rows. Native expiry, kept terminal rows and indexed reclamation land with the #773 successor, through a native-owned deadline relation; native's catalog follows the frozen published layout (the #740 precedent). In native, this PR changes only the completion-rule bounds.

Not changed here. Two transactions opening new streams on different runs can deadlock on MySQL and MariaDB. That comes from the gap lock of the stream row's locking read, and main has the same code. It is tracked in #787.

Tests

• WorkflowStreamsTest.
  • Whole-batch decisions and maxima at every ingress.
  • Expiry at the instant: 409 one second before, 404 at it.
  • Append after expiry: no write.
  • The stream quota on an append that opens a stream.
  • Bounded, resumable reclamation: item budget, stream limit, namespace isolation, kept rows.
  • A reclaimed stream keeps its identity.
  • The heartbeat pass in a forever-retention namespace, history:prune, and the operator pass.
  • An SQLite plan check (SEARCH … USING INDEX wf_durable_streams_ns_reclaim_idx (namespace=? AND reclaim_at<?), no temp sort).
• HistoryRetentionTest.
  • Run retention: deletes streams and their stream-only object; keeps an object shared with a retained run; keeps everything when the driver is blocked; keeps everything when a deletion fails, then prunes on the next pass.
  • Reclamation pass: reclaims a stream-only object; keeps an object another retained row references; keeps items when blocked; keeps items when a deletion fails, then reclaims on retry.
• RuntimeExternalPayloadTransportTest. A reference stays unclaimed after a 429 refusal and a 409 refusal, and is claimed by an admitted append.
• Real-engine interleavings on MySQL 8, MariaDB 10.11, PostgreSQL 17 and a file-backed SQLite (Concerns\ForkedDatabaseActors).
  • How they work. Each actor is a forked process with its own connection. It pauses right after a chosen statement or top-level commit, so the test decides which side holds a lock.
  • Proving the wait. The other side's wait is shown on the expected table:
    • MySQL: performance_schema.data_lock_waits joined to data_locks.OBJECT_NAME.
    • PostgreSQL: pg_stat_activity lock wait in the locking read.
    • MariaDB: the connection held in that locking read.
    • SQLite: the other side doesn't finish while the write lock is held.
  • Shared database. One migrated database per engine per process, reset to its migrated rows before each case.
  • WorkflowStreamCapacityRaceTest.
    • Concurrent first open, near-cap over the limit (refused whole), and exactly at the limit (admitted).
    • Plan checks: MySQL and MariaDB use a range scan on the deadline index with no filesort, and read exactly the limit (Handler_read_key 1, Handler_read_next 9 for 10 rows, among 2,400 rows per namespace that are mostly not due). PostgreSQL uses an index scan on it with actual rows=10 and no sort.
  • WorkflowStreamPayloadRaceTest.
    • HTTP append vs reclamation, both orders. With cleanup first, the append has verified its object outside its transaction, then waits on the object lock; it is refused with nothing written, and the object and registration are gone. With the writer first, it holds the lock with its item written; cleanup waits, then keeps the object.
    • Command append vs reclamation, both orders.
    • HTTP append vs run pruning of another run, both orders.
    • A late append to a run being pruned, which comes after its object is deleted: it waits on the run lock, gets 404 stream_not_found and claims nothing.
    • Two appends listing two objects in different lock buckets in opposite orders: the second waits on the first's first lock, and both commit.
• CI's feature job gains a MariaDB 10.11 container for these.

Qualification

At 1761ac29 (parent 0bdaee54, main 0ef7c932), on the pinned server image:

• Interleavings on four engines (MySQL 8.0, MariaDB 10.11, PostgreSQL 17, file-backed SQLite in WAL mode): WorkflowStreamPayloadRaceTest 32/32 and WorkflowStreamCapacityRaceTest 15/15, in 1:33 at the exact head.

• Counterfactuals. Each one undoes a single fix; the tests that should catch it fail.

  Undone fix	Failing cases
  READ COMMITTED for cleanup	6: writer-first on MySQL and MariaDB
  Re-check under the object lock	9: writer-first on MySQL, MariaDB and PostgreSQL
  Canonical claim order	3: opposite-order on MySQL, MariaDB and PostgreSQL, which deadlock
  HTTP claim	14
  Command claim	7
  Refusing appends to a pruned run	4
  Run lock first in pruning	3; SQLite serializes writers, so it can't separate this one
  SQLite object lock writes first	1

• Full tests/Feature with the four engines configured: 2,973 tests. The 8 errors are all in ActivityWorkerDeregistrationTest, caused by the local runner leaving the image's on-disk SQLite default in place. With DB_DATABASE=:memory:, as CI uses, that file passes 10/10 with 2 skipped.

• MySQL 8 and PostgreSQL 17 as the application database, as CI's prepared-local job runs them: HistoryRetentionTest and WorkflowStreamsTest. Each engine passed 70 of 71. The 71st was the SQLite-only plan test, which errored on EXPLAIN QUERY PLAN. It is now scoped to SQLite and passes on both engines when run alone.

• Pint: clean.

• Native: unchanged by this successor.

Notes for operators:

• MySQL refuses InnoDB writes under READ COMMITTED when binlog_format=STATEMENT. The defaults, ROW (MySQL 8) and MIXED (MariaDB), are fine.
• A producer whose HTTP append is refused because cleanup removed the object first should upload the payload again.

🤖 Generated with Claude Code

[rmcdaniel]

Security review: hold exact head fa3ba9f7fed52950388b8564c45016405b6a45a0 / tree 5b2f772979c55516e4604a48b1372c3b897118f0.

The follow-up correctly declares the new cache prefix and replaces the Sonar-reported SHA-1 use. Four substantive gaps remain:

1. The sweep loses retained external-payload ownership before cleanup. WorkflowStreamRetention::pruneBatch() deletes workflow_durable_stream_items directly, including their payload_reference fields, then deletes the stream row. Incoming opaque references have already been claimed by RuntimeExternalPayloadTransport::resolveIncoming() / RuntimeExternalPayloadRegistry::resolveAndClaim(), which makes the registry row retained. ExternalPayloadRetentionCleanup later discovers durable-stream references from these rows; once this sweep removes them, the backing object and retained registry record can no longer be reclaimed through run retention. Route this through the established external-payload cleanup, shared-reference, backup-hold, failure/retry semantics before removing the authoritative rows. Add regressions proving a stream-only object is reclaimed, a shared reference is preserved, and a cleanup/storage failure leaves recoverable state. Also cover the related refusal path: external references are claimed before the capacity transaction, so an all-or-nothing 429 must not strand newly retained objects with no accepted item.

2. Expired stream identity still changes when the sweep happens. Before physical pruning, append() sees the terminal row and returns 409 stream_closed/stream_errored. After pruneExpired() deletes that row, lockStream(..., openIfMissing: true) silently creates a new open stream with offset 0. That both reuses a terminal stream identity and makes behavior depend on sweep timing. The accepted design explicitly says expired append is 404 stream_not_found and visible state changes at the expiry instant, not at sweep time. Preserve an irreversible terminal/tombstone decision (or return with an explicitly reviewed replacement design), and test append immediately after expiry and again after physical reclamation.

3. The inline pass bounds returned candidates, not database work. Candidate discovery applies a per-row computed expiry predicate, orders by closed_at, id, and has no matching expiry/order index; the existing stream indexes stop at namespace,status. LIMIT N therefore does not bound rows scanned or sorted. Because this runs from ordinary worker heartbeats every minute, terminal-stream growth can turn the bounded-looking path into an unbounded hot-path scan. Materialize/index the expiry/order key or provide equivalent engine-specific bounded discovery with query-plan evidence on SQLite, MySQL, MariaDB, and PostgreSQL.

4. The required contention proof is incomplete. WorkflowStreamCapacityRaceTest::drivers() and contended() exercise only MySQL and PostgreSQL. The accepted Security condition was controlled two-writer first-open and near-cap evidence on every supported engine. MariaDB and especially SQLite are not interchangeable here: SQLite ignores FOR UPDATE and has different writer-lock/busy behavior, so insertOrIgnore does not establish the claimed deterministic wait/recovery by inspection. Add real MariaDB and SQLite coverage (or an equally strong engine-specific implementation and proof) for first-open, exact-cap, and over-cap races.

No operator action is needed. Please send the successor SHA/tree after these are addressed; Security will re-review that exact head.

[rmcdaniel]

Security re-review: hold exact head 0bdaee54d7b0102140897cf7ed7659867b6b7062 / tree 95555a90b102011f73b56ba55bdf72acd02bfb28.

The final 0bdaee54 delta is only the corrected Composer guard-distance fixture; git diff --check is clean. The substantive source remains 5eb71cb0.

The successor materially addresses the earlier expiry-identity, indexed discovery, simple refusal/claim ordering, run-row cleanup, and four-engine contention points. Two payload-lifecycle concurrency blocks remain:

1. The retained-reference decision and destructive delete are not one serialized decision. ExternalPayloadRetentionCleanup::deleteReferences() classifies a URI as deletable at lines 236-239, then calls the driver at lines 263-266. The driver eventually takes the URI bucket lock, but RuntimeExternalPayloadRegistry::deleteRegisteredUri() checks registry ownership only at lines 351-388; it does not re-check retained database references under that lock. A concurrent HTTP append can therefore claim and commit the same URI after the retained-row scan. Cleanup then acquires the released object lock and deletes the now-live object and registry row. Stream reclamation makes this possible because payload cleanup occurs before the run/stream lock (WorkflowStreamRetention lines 120-142). Run pruning has the same destructive-before-fence ordering: payloads are deleted at HistoryRetentionEnforcer lines 238-243, while the run lock and high-water check occur only at lines 266-286. A late same-run append that reuses an already-collected URI can make pruning skip while its backing bytes are already gone; a cross-run append can be orphaned while pruning succeeds.

2. The accepted command-path exception is not safe under this new earlier deletion path, and multi-reference claims have no canonical lock order. WorkflowStreamCommandProcessor still calls append() without a claim callback at lines 131-139, so it does not participate in the URI object lock at all. Stream-expiry cleanup can now race that writer long before run pruning, which makes current ownership semantics less safe and does not satisfy the explicit condition for leaving Workflow-task workflow_stream items store payload_reference without verification or claim #786 separate. Separately, an HTTP append claims distinct references in request order (WorkflowStreamController lines 221-224; WorkflowStreamService lines 162-175). Each claim holds its bucket lock until the outer append transaction commits. Two appends on different runs with references A/B and B/A can therefore deadlock.

Required before clearance:

• make the retained-reference check and deletion one object-serialized decision, or add an equivalent deletion fence;
• fence run writes before destructive run cleanup rather than detecting growth after bytes were deleted;
• make the workflow-command path participate in that protocol, or conservatively prevent cleanup from deleting objects it can race;
• acquire multiple payload locks in a canonical bucket order;
• add controlled interleaving regressions for HTTP append versus reclamation, workflow-command append versus reclamation, run-prune late reuse of the same URI, and opposite-order multi-reference claims. Exercise the relevant real engines, not SQLite alone.

Hosted checks are also still active on this head. Merge/release remains held. No operator action is needed.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.3% Duplication on New Code

See analysis details on SonarQube Cloud

[rmcdaniel]

Security Tier-A review of exact 1761ac29f6402cec0026c5af62776c67069289fd / tree 6337a4e6ffd2824d72c899b0e85f41f06ce1ee42: changes requested; do not merge this head.

The new object-lock decision closes the two durable-stream append paths, but the common cleanup still races with existing non-stream writers. RuntimeTrackedExternalPayloadStorage::retainEnvelope() and get() call verifyFetchedBytesAndClaim() in their own object-lock transaction, then return to callers that persist the reference later. A concrete production path is MessageStreamController: it resolves/claims the envelope at line 65, then calls the durable message append at lines 85–103. Nothing holds the object lock across that gap.

Interleaving: an existing run references content-addressed URI U; a new non-stream writer claims U and releases the lock before its database insert; retention then takes U's lock, performs the new under-lock $isReferenced() check, sees no new durable row, and deletes U plus its registration; the writer then commits its already-resolved reference. The result is a durable reference to missing bytes. Setting retained_at does not prevent this path because retention decides from durable reference rows, not that registry flag. This contradicts the new deleteUriUnlessReferenced() guarantee that a writer committing first keeps the object. The handoff itself notes these non-stream claim paths were not audited.

Required successor:

• inventory every durable external-payload writer reached by retainEnvelope() / get() / verifyFetchedBytesAndClaim() and make the claim occur under the same object lock and database transaction that commits its reference, using the common canonical lock order where more than one URI can be claimed;
• add an executable interleaving regression at the actual claim-before-reference boundary using at least one concrete non-stream path (the Message Stream append above is suitable), proving writer-first preserves bytes and cleanup-first makes the writer fail without a dangling row on every supported engine;
• retain the current durable-stream, run/stream-lock, READ COMMITTED, counterfactual, and clean-failure coverage.

This is source-only review. No production state or operator action is involved.

[rmcdaniel]

Risk-tier correction to my preceding review: the non-stream claim-before-reference gap is pre-existing and outside PR #785's changed durable-stream scope. Under the operator-approved risk-tiered review policy, it is a separate scoped follow-up, not a merge blocker for #785. I withdraw the do not merge this head conclusion as to that finding.

#785 remains under focused Tier-A review only for regressions or incomplete guarantees in its changed production delta. The non-stream path should be handed to General with a reproducible case, but it does not need to be repaired before this Rust successor can merge.

This correction narrows the gate; it does not erase the audit finding or imply that the pre-existing path was tested.

[rmcdaniel]

Operator scope correction: #785 is pre-RC Rust development and is not waiting on Security review. The Rust Server owner may merge this exact successor after its own qualification and required CI.

Security will focus pre-merge blocking review on changes that can affect existing released/deployed surfaces, credentials/auth boundaries, production data, or CI/supply-chain trust. Findings confined to an unreleased Rust surface are advisory or follow-up work unless the owner explicitly asks for a gate. The earlier non-stream finding concerns an existing PHP surface and remains a separate handoff; it does not gate #785.

No further Security acknowledgment is required for this PR.

[rmcdaniel]

Merge receipt

Merged through the merge queue at 2026-10-26T18:25:24Z.

• Merge commit: a3e0bab0191f9fc004c297f98fa78eabc5136273
• Tree: 6337a4e6ffd2824d72c899b0e85f41f06ce1ee42
• Parents: 0ef7c9321c33963db20df146d18c383189952b6d (main) and 1761ac29f6402cec0026c5af62776c67069289fd (head)

Identity. The merged tree equals the qualified head's tree (1761ac29 → 6337a4e6). The head was enqueued pinned with expectedHeadOid 1761ac29, and main did not move between enqueue and merge.

Evidence.

• PR checks at the head: 60 success, 1 skipped. The Sonar PR gate is OK.
• The merge group at a3e0bab0 passed the required gates.
• Locally, four-engine interleavings pass 47/47 at the exact head, and the counterfactuals fail as listed in the body.

Review. Tier A. Security dropped its pre-merge hold under the operator's pre-RC correction (6423216033, #325 6423232503).

Closes, by hand: #774 and #786.

Follow-ups, owned by General:

• Concurrent stream opens on different runs can deadlock on MySQL/MariaDB #787: stream-open gap-lock deadlock on MySQL and MariaDB. It starts from this merge.
• A run can reference an external payload that retention deleted after the start's claim #789: non-stream claim-before-reference.

🤖 Generated with Claude Code