[chrom]

Problem

Business cancellation may require compensation after the originating workflow or a child has completed. Run-local Saga callbacks alone do not provide separately addressable, retained compensation obligations.

Changes

• Add generation-bound compensation scopes and durable obligation records.
• Register obligations before forward effects using bounded, versioned handler descriptors.
• Recover closed runs through existing activities, native Saga and canonical start/adopt/redrive.
• Resolve child compensation before earlier parent obligations.
• Validate forward handlers and inputs before dispatch admission. Preserve original nonretryable refusals and earlier uncertain outcomes.
• Reuse existing cancellation shielding and execution ownership without introducing another executor or lease.
• Preserve existing Saga APIs and the four-method transition contract.

Verification

All completed local checks passed:

Suite	Tests	Assertions	Skipped
Targeted regressions	74	366	0
Unit	6,021	81,622	49
PostgreSQL feature	2,975	29,721	41
MySQL feature	2,975	30,418	6

Unit line coverage: 79.90%, above the 60.00% threshold.
ECS, PHPStan, feature-shard inventory and regression-corpus validation passed.
Generator–Critic review finished with PASS.

The unit run reported 2 PHPUnit deprecations and 15 notices.

Initial limits

Embedded Laravel and sequential compensation only. One nesting level, explicit release, and no forward ownership transfer across continue-as-new or originating-run redrive.

The host supplies authorization, compatible handlers, provider idempotency and authoritative effect verification. Upstream CI qualification and production-provider validation remain pending.

Refs #1063.

[rmcdaniel]

Thanks, @chrom, this is a careful piece of work.

#1069: yes, the Workflow team owns the rest of #1069: the state-bound nested-machine bindings and the two-integration example. We'll build it on top of this scope foundation once it lands, so please keep your focus on #1063.

First-increment boundaries: these are acceptable for a first, experimental increment:

• embedded Laravel only;
• sequential compensation;
• one nesting level;
• explicit release;
• no forward ownership transfer across continue-as-new or originating-run redrive.

There are three conditions:

1. Experimental marking: the new Workflow\V2\Compensation surface is marked experimental in code and in docs/api-stability.md, so it isn't covered by the stability guarantees yet.
2. Fail safe at the boundaries: a workflow holding an open scope that continues as new, or whose origin run is redriven, must refuse or keep its obligations recoverable through ScopeRecovery. It must never silently orphan them. Note that main now hands still-open children to the continued run at continue-as-new (Hand open children to the continued run at continue-as-new (#868) #1104, Retry a handed-over child's failure under its recorded retry policy #1107), so please check that interaction.
3. Split the redrive change: the change to core redrive (FailedRunRedrivePlan / ActivityRetryHistory accepting ActivityRetryScheduled) affects every workflow, not only scoped ones. It probably needs its own regression-corpus evidence, and Redrive failed runs whose completed steps include a child workflow #1109 has since changed the same planner. We may ask to split it into its own PR.

Review status: a full review is in progress. It covers replay determinism, lock order, the migration, the authorization boundary and a rebase onto current main, plus the feature suites your fork's CI skipped. We'll post concrete findings here. Security will also review the authorization boundary before merge.

[rmcdaniel]

Review of head 861dc405 (General, Durable Workflow). The suites were run locally on MySQL 8.0.46 + Redis and on a trial merge onto current main; the results are at the end.

Verdict: request changes. The first-increment boundaries are acceptable, with conditions.

The design follows the #1063 contract:

• a closed handler registry and bounded portable descriptors;
• registration committed before the effect;
• separate forward and compensation states, and an uncertain effect never becomes verified;
• recovery reuses native activities, Saga, start/adopt and redrive.

ECS and PHPStan are clean on the changed files, and the suites pass on MySQL 8.

Boundaries:

• Embedded Laravel only: yes. But the PR also changes core failed-run redrive eligibility for every run, embedded and service-mode, so that change has to be split out (B1).
• Sequential compensation: yes. The ledger enforces reverse order even if the author enables setParallelCompensation or setContinueWithError, because ScopeProgress::beginCompensation throws. DurableScope should refuse those modes, or the docs should say so.
• One nesting level: yes. ChildScopes::register enforces it structurally, although ChildScopes::MAX_DEPTH is declared and never used.
• Explicit release: yes, as long as no scope can end up neither recoverable nor releasable (B4). It also needs a supported, authorized status read (N2).
• No forward ownership transfer across continue-as-new or originating-run redrive: yes as a boundary, but it must fail closed, be documented and be tested (B5). Add child-workflow retry (#1105/#1107) to the same list. Any new run that reuses the same ScopeIdentity hits the same refusal, so generation must be unique per originating run.

Blocking

B1. Split the core redrive change into its own PR: FailedRunRedrivePlan.php:88,179,330,550 and the new ActivityRetryHistory.php.

It merges cleanly with current main, including #1109. The logic looks safe: it accepts only a linear retry chain for one activity execution, matching what ActivityOutcomeRecorder writes (:382). The successor still copies only ActivityCompleted for reused steps, so I found no history that would now redrive and then replay differently.

It still changes behaviour for every run. A run with any retried activity used to be refused with unsupported_history and is now eligible. That adds the reason invalid_activity_retry_history and changes when Waterline and run detail offer redrive. The split PR should include:

• CHANGELOG: an entry.
• Feature tests for ordinary workflows in V2FailedRunRedriveTest:
  • a retried-then-completed prefix step is reused, not re-run;
  • a retried failed step re-runs once;
  • an explicit decision and test for runs started by a service worker (#1109 refuses unqualified steps for those).
• Unit cases: a retried completed prefix step, and one with heartbeats in between. Today only a retried failed boundary step is covered.
• Start adoption: the generic isActiveStartAdoption should say why only ReturnedExistingActive is accepted while StartRejected still blocks redrive.
• Nit: the new unit tests in FailedRunRedrivePlanTest.php:182-185 are inserted under linearFailureHistory()'s @return docblock.

The replay-corpus guard does not trigger here, and since successor histories keep their shape, feature tests are the right evidence.

B2. Deadlock on MySQL/MariaDB from INSERT IGNORE followed by lockForUpdate. Two places do this:

• ChildScopes.php:41-48, on the graph row per (application, namespace), so every participate() in a namespace contends;
• ScopeRepository.php:37-43, on the scope row.

InnoDB takes a shared lock on the existing duplicate row for INSERT IGNORE, then both transactions ask for the exclusive lock. This exact statement pair reproduced ERROR 1213 Deadlock found on MySQL 8.0.46.

Nothing retries it:

• ScopeBoundary uses transaction($op, 1);
• RegisterChildScopeActivity and RegisterObligationActivity keep the default $tries = 1.

So two unrelated workflows in one namespace can fail a business step, and with the documented catch-then-compensate pattern that also triggers compensation. CI doesn't catch it because V2CompensationScopeConcurrencyTest.php:29 only runs on pgsql.

Suggested fix:

• Lock order: lock first, and insert-or-ignore then lock only when the row is missing. Alternatively, create the graph row in its own short transaction.
• Retries: raise $tries on the idempotent register, request, prepare and verify activities.
• Test: add a concurrent participate() test on MySQL.

B3. Host authorization and input validation run inside the native lock transaction. ScopeBoundary.php:37-66 calls ScopeAuthorization::assertAllowed() while it holds the attempt, execution, run and task rows (via DefaultActivityTaskBridge::status()), plus the scope row. On SQLite that is the whole database writer lock.

The same applies to:

• HandlerRegistry::resolve() → ScopedOperation::assertInput() in admitForward (ScopeProgress.php:45 via ForwardOperationActivity.php:38-40);
• RegisterObligationActivity.php:24;
• the child authorization check at RegisterChildScopeActivity.php:30.

A typical host policy check calls a policy service, an identity provider or another database. That contradicts "No external call runs under a database lock" in docs/compensation-scopes.md, and it stalls the run's workflow tasks (on SQLite, every writer).

Suggested fix: run authorization and assertInput before opening the transaction. Inside it, re-check native authority and record the policy version already obtained. At minimum, document both interfaces as pure, local and with no I/O, and correct the docs.

B4. A terminated or cancelled recovery run strands the scope permanently (ScopeRecovery.php:85-102). If the latest canonical RecoveryWorkflow run is Cancelled or Terminated, every later request() throws The recovery is terminal without verified compensation. Only Failed runs are redriven, and release() requires verified.

Reproduced: recovery waiting on a retry → attemptTerminate() → the second request is refused and the scope stays requested. Waterline offers cancel and terminate on every run, so one operator click strands the obligations, which breaks the no-orphan contract.

Suggested fix: start a fresh run of the canonical instance after Cancelled/Terminated (with a test), or refuse cancel and terminate on the recovery workflow.

B5. The ownership boundaries have no tests. From reading the code and local probes:

• Continue-as-new or child retry reusing the identity: registration fails with a plain LogicException from ScopeRepository::open ("cannot change its original run"). That is fail-closed, but it is not a typed non-retryable reason. The old obligations stay open until someone calls ScopeRecovery::request.
• Origin redrive after the documented catch → compensate pattern: the history contains FailureHandled, so redrive is refused with unsupported_history. The explicit request then verifies through recovery.
• Origin redrive after an uncaught forward failure: redrive is accepted, and the successor's first non-reused scope activity is refused with scope_execution_owner_mismatch.

Please:

• add tests that pin each case;
• add a typed non-retryable "scope owned by another run" reason;
• document that generation must be unique per originating run;
• document that request() on an active origin cancels the whole run, which compensates every Saga callback in it, including other scopes and non-scope compensations.

B6. Mark the API experimental and fix the persisted formats before any data exists.

• Markers: no new class has @experimental or @api, and docs/api-stability.md is unchanged.
  • Mark the types apps use @experimental (precedent: StateMachine\TransitionRunner).
  • Mark the rest @internal: the 7 activities, ChildResolution, PortableDescriptor, EvidenceReferences and RecoveryWorkflow.
  • Add an entry to the api-stability doc.
• Identity hashes: ScopeIdentity::id() (:25,35), Obligation::fingerprint() (:29,34), ForwardResult and Resolution hash get_object_vars($this). Renaming a property would change every scope id, recovery instance id and fingerprint, and every retained obligation would then be refused as tampered. Hash explicit, versioned arrays instead.
• Activity type names: activities are stored in history by fully qualified class name. Add stable #[Type('dw.compensation.….v1')] attributes.
• History shape: the sequence of activities DurableScope runs (DurableScope.php:36-48,55-59) becomes part of every caller's history. Add a version marker (Workflow::getVersion) so it can change deterministically later.

Non-blocking

• N1 Naming. "Scope" collides with cancellation scopes (cancellation_scope_id, ScopedCancellationContext, CancellationScope*). Consider CompensationScope, CompensationScopeIdentity and CompensationHandler, and an enum for the authorization operation strings (register/forward/request/compensate/participate/release).
• N2 Hosts have no supported read path. They have to use the @internal ScopeRepository::inspect to see verified or unresolved work. Add an authorized status().
• N3 ChildResolution waits for a child by spending activity retries (tries=3, native backoff). The tests pass because the sync queue finishes the child inline. On a real queue the parent recovery will usually run out of retries and need a re-request. Test with Redis or the database queue.
• N4 A child scope that never opened blocks its parent forever (testUnavailableChildCannotAdvanceEarlierParentCompensation). Consider writing a closed tombstone with zero obligations under the graph lock, so a late child origin is refused.
• N5 The journal cap (ScopeStorage.php:56, 8192 rows) also counts the authorized row that every boundary writes. With 256 obligations plus retries and redrives, a scope can hit the cap, and then no outcome can be recorded. Reserve headroom, or stop journaling read-only boundaries.
• N6 ScopeBoundary.php:67 loads every obligation row and descriptor (up to 256 × 16 KiB) FOR UPDATE at each boundary, twice per activity.
• N7 CompensateObligationActivity.php:57-59 resolves the handler before checking forward_status === 'registered', so a never-dispatched obligation is blocked if its handler was removed.
• N8 Each obligation's compensation callback runs RequestScopeActivity again (DurableScope.php:39,57), which adds N redundant activities.
• N9 Security.
  • The principal is a plain string from workflow code, stored in activity arguments. Document that it must never come from webhook or HTTP start input.
  • The fingerprint is checked against a SHA-256 stored in the same row, so it catches corruption but not someone with database write access. Say so, or use an HMAC.
  • ChildResolution.php:38 discards the child policy version instead of validating and journaling it.
• N10 Tests.
  • The cold-replay test only checks an effect counter. Also assert zero workflow_compensation_* queries and no ScopeAuthorization calls during replay.
  • Add a test that an unbound ScopeAuthorization fails closed. Today it fails with BindingResolutionException.
• N11 Docs and housekeeping.
  • Trim the process notes from docs/compensation-scopes.md (probe counts, upstream SHA, the previous draft's lease tests, Generator–Critic).
  • Add a CHANGELOG entry.
  • The single-column scope_id indexes on obligations and requests duplicate the leading column of the unique (scope_id, position) indexes.
  • ScopeRepository duplicates ScopeStorage's table constants and hard-codes the requests table name.
• N12 RecoveryWorkflow starts with no queue or connection options, so hosts can't route it.

The migration is fine:

• Connection: it uses WorkflowMigration and the configured storage connection.
• MySQL 8: it creates cleanly, and identifier lengths are within the MySQL and Postgres limits.
• No foreign keys: retention cleanup can't cascade into the ledger, and pruneRun keeps the run row that recovery needs.
• Rollback: down() drops all six tables.
• SQL Server: it can't run insertOrIgnore, but core already depends on it, so this isn't a new regression.

Local results

Run	Tree	Result
ECS + PHPStan, 44 changed PHP files	PR head	clean
Unit: Compensation, FailedRunRedrivePlan, redrive-rejection tests	PR head	93/93 OK
Same, plus HistoryBudget	trial merge onto current main	141 OK, 2 skipped
Feature: compensation files + V2FailedRunRedriveTest, MySQL 8.0.46 + Redis	PR head	152 OK, 3 skipped (pgsql-only)
Feature: same, including #1109's tests, MySQL 8	trial merge onto current main	162/162 OK

Before merge (maintainers)

• Full CI: run it from a maintainer branch. The fork skipped feature-mysql, feature-mariadb, coverage, the Laravel embedded upgrade matrix (relevant for the new migration) and SonarQube. MariaDB matters most because of B2.
• Security review of the authorization boundary: the principal trust model, B3, and the descriptor limits.
• Order: land the split-out redrive PR (B1) first.

Once B1–B6 are addressed we'll re-review promptly. Thanks again.

[rmcdaniel]

Thanks @chrom. The follow-up in 1d2e412 (posted on #1069) addresses my first comment's conditions: the experimental markers and the api-stability entry, the merge of main, the three run-boundary regressions, and the planner corpus fixtures with guards. That's good progress.

The formal review above (B1–B6) is still the bar for merge, and four of its blocking items are unchanged at 1d2e412:

Item	Status at 1d2e412
B1: split the core redrive change	Still in this PR. Your isolated run (20 tests / 282 assertions against main) is exactly the evidence the split needs. Please open it as its own PR with the CHANGELOG entry and the ordinary-workflow feature tests listed in B1, and rebase this PR on it.
B2: MySQL/MariaDB deadlock	Unchanged. ChildScopes.php:46-73 and ScopeRepository still insertOrIgnore then lockForUpdate. Each existing-row path needs lock-first (or a separate short insert transaction), retries ($tries) on the idempotent activities, and a concurrent participate() test that runs on MySQL.
B3: host callbacks under native locks	Unchanged. ScopeBoundary.php:66 still calls assertAllowed() inside the transaction. The same applies to assertInput()/handler resolution and the child policy check. Move them before the transaction, or at minimum document both interfaces as pure/local/no-I/O and correct the docs claim.
B4: a terminated or cancelled recovery strands the scope	Unchanged. ScopeRecovery.php:105 still throws The recovery is terminal without verified compensation. Please start a fresh run of the canonical instance (or refuse cancel/terminate on RecoveryWorkflow), with a test.
B5: typed reason for "scope owned by another run"	The boundary tests are in. Please also replace the plain LogicException from ScopeRepository::open with a typed non-retryable reason, and add the generation / active-origin request() notes to the docs.
B6: persisted formats	The experimental marking is done. Still open: identities and fingerprints hash get_object_vars($this) (ScopeIdentity, Obligation, ForwardResult, Resolution); activities lack stable #[Type('dw.compensation.….v1')] names; and DurableScope has no version marker. These are cheap to fix now and expensive after any data exists.

Once B1–B6 are in, we'll re-review straight away and run the full CI matrix the fork skips (feature-mysql/mariadb, coverage, the Laravel upgrade matrix, Sonar) from a maintainer branch. Security will review the authorization boundary at that point.

Small note: please keep #1110's discussion on the PR. #1069 is now the Workflow team's nested-binding work.

[rmcdaniel]

Workflow has reconciled the current head 1d2e412 with the trusted maintainer review. The open review work remains B1–B6, and #1069 still depends on this foundation.

@chrom, please confirm whether you are taking the remaining corrections, starting with the independent redrive PR (B1). Workflow owns the review, the full maintainer CI matrix and Security coordination. If you no longer have capacity for a specific correction, identify it here so we can take that implementation rather than leave the dependent feature blocked.

Current intake state: PR head 1d2e41203005b8ac086e436c019d17c00a079454 has no intake:approved label. Approval of issue #1063 does not approve this PR prose. The earlier wording overstated progress on intake handling; that approval has not been completed. Trusted maintainer reviews and source inspection can continue. Merging still requires B1–B6, the full maintainer CI matrix and exact-head Security review.

[rmcdaniel]

Maintainer B1 follow-up is now draft #1131 at cebeab3b, based on current main and restricted to generic activity-retry redrive. It extracts the planner/retry-history source and four replay fixtures from reviewed contributor head 1d2e412, adds ordinary queue-job and existing service-fingerprint controls, and carries no Saga ledger/API changes. Workflow owns the remaining full local quality cycle, hosted checks and exact-head Security review; focused planner/replay controls plus ECS/PHPStan pass, and current-main planner substitution fails five new controls. After this prerequisite is qualified, Workflow owns B2–B6 follow-ups on separate maintainer branches. General's #996/#1069 slices remain separate; current external PR prose is still held from intake.

[rmcdaniel]

Maintainer continuation is now draft #1132 at d0920b12, stacked on B1 #1131. It imports only the compensation foundation from reviewed 1d2e412 and starts B2 with existing-row lock-first adoption, bounded idempotent activity retries and deterministic InnoDB contention controls. Those new controls are not yet qualified; all B2–B6 merge blockers remain explicitly open in the draft. Workflow owns the source and qualification; General counterchecks the B4 native restart boundary, since terminal attemptStart currently refuses duplicates and ordinary native redrive supports failed runs only. No existing contributor branch is rewritten, and no source/registry/native delivery is claimed.

[chrom]

Thanks, Richard. Understood: Workflow owns B1 in #1131 and the B2–B6 continuation in #1132. We are returning to the OMS work and have stopped our separate B1 split and further parallel DW implementation.

The follow-up we had already prepared is now available in 671cf026, on top of 1d2e412. This single commit contains 32 B2–B6 files and the scope CHANGELOG entry. The additional generic B1 changes are excluded from this commit. Existing PR history has not been rewritten.

• B2 — locking and retries: lock existing scope/graph rows before insertion, add bounded transaction and idempotent activity retries, and extend the concurrency regression to exercise InnoDB.
• B3 — host callbacks: move authorization, input validation and child-policy callbacks outside native SQL lock transactions. Recheck native authority and the prepared descriptor at admission. Added regressions for callbacks outside transactions and cancellation during authorization.
• B4 — interrupted recovery: use attemptSignalWithStart('recover', ...) to obtain a fresh run of the canonical recovery instance after cancellation or termination. Regressions assert a new run ID, preservation of the previous terminal history, and no repetition of verified compensation.
• B5 — ownership boundaries: add the non-retryable scope_owned_by_another_run reason and document generation-per-originating-run semantics and whole-origin cancellation by an active-origin recovery request.
• B6 — persisted contracts: use explicit versioned arrays, stable activity type names, workflow layout version markers, and identity/fingerprint regression vectors. Experimental marking was already included in 1d2e412.

Full local verification of the content-hashed working candidate:

Check	Tests	Assertions	Skipped
Unit	6,034	82,013	49
PostgreSQL feature	3,029	30,527	41
Unit coverage with PCOV	6,034	81,988	49

All three completed with zero failures/errors. ECS and PHPStan passed. V2 line coverage was 79.90%, above the 60.00% unit-only floor. Unit and coverage reported 2 PHPUnit deprecations and 15 notices.

These full runs included the separate B1-only regression additions in the working tree. The B2–B6 source and test blobs in 671cf026 match that tested snapshot. The table is evidence for that local candidate, not qualification of the maintainer branch heads.

The initial Xdebug coverage attempt hit two fan-out process timeouts and one delayed-retry timestamp assertion failure. The final run used PCOV only in PHPUnit and file-based SQLite in tmpfs. The full suite and coverage gate passed without changing source code, assertions or time limits.

For handoff, the commit is available for selective reuse on #1132. B2 already overlaps your maintainer work, so please take only the fixes or regression tests that remain useful. I have retained the content-hash manifest and JUnit/Clover results and can provide those as needed. This leaves implementation ownership with Workflow and avoids a second active workstream. Hosted CI, the full database matrix and exact-head Security review remain with your team.

[rmcdaniel]

Maintainer status, November 14 — General owns this reconciliation; Workflow owns the remaining compensation implementation and qualification.

This PR is still open at contributor head 671cf02644e7c7baf58a53f3c10f15ba445b78aa, but the implementation continuation is now on maintainer branches. The contributor's approved handoff in #1110 (comment) confirms that they stopped the separate DW workstream and returned to their application work. Contributor availability is not a dependency.

• B1 is delivered: #1131 merged as a7178856475e495714d1e5172d544b8535163019, with its normal main qualification. This is the generic validated activity-retry redrive prerequisite.
• B2–B6 continue in draft #1132, currently 9d0191c025fbd040ee62b103469125b39e0530c0. Its source incorporates the lock ordering, host-callback, recovery-authority, persisted-identity and replay corrections, with independent review and passing current hosted checks. That does not replace the final complete combined-source qualification.
• The remaining gate is ours: finish the combined Core timer/index corrections and full final database/quality qualification, then rebind the exact compensation successor and integrate it normally. The latest combined Core unit run passed 6,119 tests / 83,203 assertions, while its complete MariaDB run is NONPASS on a final service-delivery case; Workflow is diagnosing the actual dispatch timestamp metadata before selecting the narrow correction. That failure is retained, not relabeled as a pass or attributed to this contributor's branch.

The original contribution has not been merged wholesale, and durable retained scopes are not shipped by B1 or by passing partial checks. Issue #1063 remains open through actual foundation delivery. Workflow retains source, tests, independent review, normal integration and cleanup. General's following state-bound business bindings under #1069 remain a separate increment; their confirmed native child-start prerequisite is source-delivered in #1135.

Workflow will post the final #1132 landing evidence here and resolve this original PR as superseded once that handoff is complete. No additional correction is currently requested from the contributor.

[rmcdaniel]

Superseded by the maintainer-owned implementation in #1131 and #1132. The retained-scope source is accepted as main 80f2a916e677f220927bb6d60cd4e7328e59db13, whose tree exactly equals the reviewed/tested 4b3906bc; its actual-main build and the other triggered workflows pass. Final source/quality evidence and independent integration PASS are recorded on the replacement PR.

The parent issue #1063 remains open for supported published delivery, and the separate #1069 state-machine integration continues under its existing ownership. No further contributor corrections are requested here. This closes the duplicate implementation stream; the accepted foundation remains experimental and unreleased, with the #1130 stop and absence of 2.29 publication selection preserved.

Final accepted-main source receipt.