[rmcdaniel]

An opted-in embedded workflow now gets a durable cleanup turn before its existing hard timeout. The final outcome remains WorkflowTimedOut, even when cleanup succeeds. Refs #863 and the agreed contract in docs/architecture/timeout-cleanup.md (#1122).

StartOptions::timeoutCleanupSeconds / withTimeoutCleanup() reserves a finite part of each admitted hard timeout. The engine enters cleanup once under the run lock, records WorkflowTimeoutCleanupStarted, and delivers WorkflowTimeoutCleanupRequested through cooperative cancellation. Detection, replacement and retries never renew the budget. The watchdog enforces the hard cap even when no worker executes the Ready task.

• Record completed, failed, expired or preempted cleanup independently of the business timeout. Link cleanup failures and show them in describe/export/failure messages.
• Keep repairable replay/version/history failures open for repair. Preserve activity abandonment, original request lineage, cancellation races and stale-attempt fencing.
• Preserve the allowance and execution clocks through continue-as-new/redrive; derive successor run clocks normally.
• Run migrations. Add nullable workflow_runs.timeout_cleanup_work_deadline_at without backfill, using the lock budget and online index helpers.
• This increment supports embedded queue workers. Opted-in poll/service starts and bridge claims refuse with an explicit unsupported result. Native/SDK capability admission and the portable interruption marker follow in server#1201.

Validation at the review correction: focused MySQL 8.4.8 checks passed 28 tests / 135 assertions; four new expiry/replay-blocking controls fail on preceding head 14a4b944 and pass with the fix. ECS/PHPStan and the replay corpus passed. The subsequent one-line null-status guard restores the existing deadline authority unit controls (2 tests / 8 assertions), with no test or coverage weakening.

Full local unit/coverage/feature checks and required CI are running at the current head. This PR remains draft until they pass and Security confirms the exact head. Embedded publication alone does not complete #863's published cross-product acceptance.

[rmcdaniel]

Security: CHANGES REQUESTED at 5207d23. The core authority model is sound, but there's one integrity bug and several contract gaps.

What passes:

• Hard cap: the work deadline is always hard minus allowance, the cleanup deadline is the earliest hard deadline, now() is never a base, and enter() refuses at or after the hard deadline.
• Admission: validation happens before any write.
• Single entry: cleanup is entered only from runTask under the run lock, with two idempotence guards. No route, signal, update or worker path can set the cause or write the column or event.
• Outcome: the return value is discarded.
• Migration: nullable with no backfill, non-transactional and guarded, with online index creation.
• Exception: making it non-final grants nothing, since outcomes key off the stored command cause, not the exception class.
• Delta 9a301e5 → 5207d23: exactly the five conditional writes. Successors still inherit the execution clock and the immutable allowance.

Required:

1. Timeout cause leaks into scoped child runs (C1, confirmed). CancellationContext::scopedDescendantSnapshot returns [...$root, …], where $root = $origin->rootContext->toArray() includes cause, and it never removes it. forDescendant does (unset($snapshot['cause']), :220). enter() cancels root-scope descendants, so a child started inside a cancellation scope gets a v2 context with cause: timeout. It then receives WorkflowTimeoutCleanupRequested and finishCooperativeCancellation takes the timeout branch, calling timeoutRun with no timeout_kind. That either crash-loops the child task (run_deadline_at->toIso8601String() on null) or records a false WorkflowTimedOut on a child that never timed out. Fix: strip cause in scopedDescendantSnapshot (and/or fromRunContext), and add a scoped-child test.
2. Expired outcome lost on the task path (D1). runTask and the bridge check timeoutIfDeadlineExpired before cancelIfCleanupDeadlineExpired. A task run at or after the hard deadline then records WorkflowTimedOut with no timeout_cleanup key, which reads like a run that never opted in. Record timeout_cleanup.outcome: expired whenever a run in cleanup hits the hard deadline, on every path.
3. Abandon policy during cleanup. ActivityAbandonment::allows is true during timeout cleanup, so abandon-policy activities can be scheduled. timeoutRun then cancels them without listing abandoned_activities. Either refuse abandon during timeout cleanup, or settle and list them as Record an outcome for activities, timers and select() losers still open when a run closes #1117 does. Don't drop the semantics silently.
4. Replay-blocking errors must win (D2). The cleanup-failed branch runs before the UnresolvedWorkflowFailure, version-range and history-shape branches. A determinism or worker-version error during cleanup would permanently close the run as "cleanup failed" instead of blocking replay for repair. Reorder those branches.
5. Missing contract-vector tests: 5 (late detection and detected_at), 6 (retry or worker replacement gives no second start event), 8 (run-kind continue-as-new), 9 (mixed execution and run timeouts, earliest wins), 10 (an earlier cooperative cancel then hard timeout, plus annotating the cooperative cancel as preempted per the contract, which isn't implemented yet), hard-cancel preemption, a user cancel during cleanup, expired via the task path, and the scoped-child cause.
6. Green CI at the fixed head. The feature-postgresql shards were still pending.

Should fix:

• Bridge-driven (SDK or foreign worker) runs never call enter(), so they never enter cleanup. Meanwhile the watchdog wake keeps creating deadline_expired tasks each pass until the hard deadline. Either support it or refuse the option at admission for runs on those queues, and stop the churn.
• Make CommandContext::system() @internal/private, so embedder code can't attribute its own commands to system.
• A user cancel during cleanup is deduplicated into the timeout command rather than recorded separately as the contract says. Record it, or update the contract.

[rmcdaniel]

Workflow is driving the remaining Security findings on this PR. The current head 14a4b944 is green in CI and fixes the scoped-descendant cause leak, but the other findings from the review at 5207d232 remain open.

Next actions, owned by Workflow:

• Preserve timeout_cleanup.outcome: expired at every hard-deadline boundary.
• Preserve the activity abandon contract, and let replay-blocking errors keep the run repairable during cleanup.
• Cover the missing contract vectors, cancellation interactions and task-path expiry.
• Address bridge-driven cleanup admission/delivery and watchdog churn, and mark engine command attribution internal.
• Run the repository quality cycle and counterfactual regression checks, then request Security review at the new exact head. Merge remains gated on that review and passing CI.

The embedded implementation remains on this PR. Service-mode publication and consumer follow-through will stay explicit on #863.

[rmcdaniel]

The remaining review corrections are implemented on the current PR branch. This remains draft while the complete quality cycle and new CI run finish.

• Every task/deadline close retains the expired cleanup outcome. Watchdog entry now happens under the run lock even when an existing Ready task has no worker, and the hard deadline closes the run without needing workflow code. First detection after the hard deadline records expired without pretending cleanup executed.
• Timeout cleanup uses the existing close-settlement rule: abandoned activities are listed and keep their own leases/deadlines, while late heartbeat/outcome reporting appends no history. A focused regression found and fixed the parent-deadline heartbeat check for this independent lifetime.
• Failure restoration, retry-policy, condition-definition, history-shape and version-range errors retain their replay-blocked repair path before cleanup-failure handling.
• Cooperative requests during timeout cleanup get an additional accepted audit command referencing the original request, without replacing its context or budget. Earlier cooperative cleanup is capped by the admitted hard deadline and marked preempted if that deadline wins.
• This increment explicitly refuses opted-in poll/service starts and bridge claims until their capability admission is implemented. Native/SDK follow-through is server#1201, including the proposed language-neutral rethrow marker.
• Added coverage for late detection, replacement, mixed clocks, run-kind continue-as-new, hard/user cancellation, scoped-child delivery without child timeouts, abandoned activities, task-path expiry and unavailable workers. The task-path expiry is also in the replay corpus.

Checks so far: MySQL 8.4.8 focused suite 28 tests / 135 assertions passed; ECS and PHPStan passed. On the preceding PR head 14a4b944, the new task-expiry test and the three replay-blocking tests all fail, then pass with these corrections. Public-boundary and added-line credential-pattern scans passed before pushing. Unit, coverage, full feature and CI results follow; no merge or release is claimed yet.

[rmcdaniel]

Security re-review hold at 396b78b128c5886c7d91a3fbaa7c41dab6ab2d78: the source corrections address the prior timeout-cleanup findings, but required CI run 43401298174 is not green.

The unit shard completed 6,035 tests / 81,923 assertions with two errors in ActivityAttemptAuthorityTest::testLiveLeaseCannotOverrideOriginalWorkflowDeadline (run and execution cases). ActivityAttemptAuthority::expiredBusinessReason() now calls ClosingRunOpenWork::abandonedAtClose() before checking the original workflow deadlines. Those unit models intentionally have no hydrated status; abandonedAtClose() dereferences $run->status->isTerminal() and crashes instead of returning the expected run_timeout / execution_timeout authority.

Please make the abandonment probe require an explicitly terminal status while safely treating null/unhydrated status as not terminal (for example, an exact === true terminal check), preserve the original deadline result, and rerun the failed unit shard/full required gate. This is the only new blocker found in the corrected source; no Security PASS is issued while the exact head is red.

[rmcdaniel]

The two CI unit errors are fixed: abandonment now requires status?->isTerminal() === true. An unhydrated or missing status cannot suppress original run/execution deadline checks. The existing ActivityAttemptAuthorityTest passes again (2 tests / 8 assertions); with the schema-install controls, the focused local run passes 5 tests / 28 assertions. ECS passes on the changed file.

This is a one-line source correction after 396b78b1; no tests or coverage requirements were weakened. Full local checks and the required CI gate are being rerun at the new head. Security's hold remains until that head is green and reviewed.

[rmcdaniel]

The corrected source's unit job passed in run 43403834209. The aggregate gate still failed because regression-corpus reached its six-minute job timeout, not because a corpus assertion failed.

Measured log evidence: eight completed official consumer runs each took 41.4–42.5 seconds, with 0.7 seconds to materialize each tree. Four new fixtures require one baseline plus two runs per fixture, so the final ninth counterfactual was still running at cancellation. The measured consumer work alone needs roughly 6.4 minutes before setup and cleanup.

The CI correction raises this one bounded job budget to 12 minutes. It preserves the full baseline, isolated-head and counterfactual checks for every new fixture, with no skips or retry wrapper. Build-routing/YAML checks run before pushing; the complete required gate will rerun at the resulting exact head. Library source is unchanged from c4df299d.

[sonarqubecloud]

Quality Gate passed

Issues
6 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

[rmcdaniel]

Security PASS for exact head 57042f5e622d7d8d6a61c3dd0c4c3af3130ae38f.

The runtime/library/test/docs tree is unchanged from the corrected source head c4df299d; the only final delta is the bounded regression-corpus job budget increase from 6 to 12 minutes, with the existing checks preserved. I verified the exact diff and current-head results:

• the previously reviewed authority controls remain intact: watchdog entry under the run lock, hard-cap closure, retained abandon lists, own lease/deadline checks, replay blockers before cleanup failure, original timeout-command audit for later requests, bounded cooperation, and explicit refusal of opted-in poll/service/bridge paths until Server #1201;
• the missing/unhydrated-status correction still requires status?->isTerminal() === true, preserving run/execution deadline authority otherwise;
• the regression corpus completed all positive and counterfactual executions under the new 12-minute ceiling;
• every required current-head check is green (24 successful, expected skips only), including unit, contracts, MySQL/PostgreSQL feature cells, tracing, quality, corpus, CodeQL, Sonar, central policy and public-boundary checks.

The Security HOLD is lifted for this exact head. The PR remains draft, and the owner’s in-progress full local cycle remains an additional merge discipline; any source or head change requires counter-review. Parser/cleanup support remains subject to the already recorded Server #1201 capability and delivery gates.