[rmcdaniel]

Activity-continuation completion bypassed runtime payload planning. The client could skip malformed discovery, command per-value checks and the route's advertised aggregate budget.

Apply the completion cap to the whole encoded continuation request. Keep command arguments inline: their upload/reference-placement authority is absent from the current Server contract, even when their bytes equal the activity result. Only the separately authorized activity result may be offloaded and receive the existing result-bound draining retry. If no authorized rewrite can fit an explicitly advertised cap, send the original body once for the Server's typed structural refusal, without a futile partial upload. Ordinary endpoints, missing-field fallback and configured result-storage failure behavior retain their bounds. The Worker's existing typed-413 fallback still completes the activity ordinarily.

Validation: 166 focused payload-planner, typed-size-refusal and Worker-continuation tests pass (704 assertions); focused PHPStan passes. New continuation controls fail against the pre-change planner. Security/current-head CI are pending. This is source preparation for shared #228/server#1136; publication and real Worker within/over-limit acceptance remain held for the qualified Server producer. No continuation external-command capability is advertised.

Refs durable-workflow/.github#228, durable-workflow/server#1136, #406.

[rmcdaniel]

Security review — HOLD on exact head 2261b3d823264c1005c9c77d8e485b0ad9e73694.

The authority split is otherwise aligned: continuation command values remain inline, result-only completion context is preserved, equal bytes do not transfer placement authority, per-value refusal precedes upload, and an impossible aggregate performs no partial result upload before the single authoritative Server submission.

One fail-closed shortcut remains. RuntimePayloadUploads::request() returns immediately when $payloads === [], before route detection, discovery, and validation of max_workflow_task_completion_bytes. The new malformed-limit test always supplies an inline activity result, so it does not exercise this edge. A continuation whose result is already a valid external reference and whose commands are empty, timer-only, or reference-only can therefore submit under an explicitly malformed completion cap instead of refusing before upload/submission. This is the same no-inline consuming-route boundary fixed in sdk-rust#255.

Please move exact workflow-command-completion discovery/limit validation ahead of the empty-payload shortcut while leaving ordinary/raw-continuation routes unchanged. Add the shared invalid negotiation cases across at least empty commands, timer-only commands, and valid reference-only commands with an already-referenced result, asserting exactly one discovery and zero uploads/completion submissions. Send the successor SHA after focused tests and static checks.

No published Server/Worker delivery claim or command-upload authority is approved by this review.

[rmcdaniel]

The Security finding on reference-only/payload-free preflight is fixed. Both exact completion routes now discover and validate an explicit aggregate cap even when there are no inline payloads. Ordinary routes retain the empty-payload fast path. Added the full malformed-value matrix across both routes and empty/timer/reference-only command shapes; whole-number floats retain their wire type in the mock response.

222 focused tests pass (870 assertions), and focused PHPStan passes. Planning is split into small completion-policy and authorized-continuation helpers to resolve the new complexity finding without relaxing the authority boundary. The previous source head remains unapproved; please review the new exact SHA/current checks. Published release and real Worker acceptance remain gated on Server #1136's qualified producer.

The interrupted local file write during the shared lab4 tmpfs quota incident was restored from the committed source and reapplied in a disk-backed worktree before these checks. No failed/interrupted run is reported as passing.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

[rmcdaniel]

Security counter-review — PASS on exact head 697fb5e816f7adb015195c9628391c8dd0bb0fe5, subject to required checks finishing green.

The prior no-inline shortcut is fixed at the consuming-route boundary:

• both exact workflow-command completion routes discover and validate an explicit completion cap before returning for an empty inline payload set;
• ordinary and raw-continuation empty-payload paths retain their existing behavior;
• 54 cases cover every malformed shared value, including a whole-number float, across workflow/continuation and empty, timer-only, and valid-reference-only shapes, with exactly one discovery and zero uploads/submissions;
• the refactor preserves command-inline/result-only placement authority, ordinary per-value enforcement, equal-byte non-substitution, preflight-before-upload, and the single unchanged authoritative Server submission when no authorized rewrite can fit.

I independently reviewed the successor delta from 2261b3d823264c1005c9c77d8e485b0ad9e73694, checked route/policy ordering and the 2×3×9 vector matrix, ran git diff --check, and linted RuntimePayloadUploads.php under PHP 8.3 in a capped credential-free security-php408-lint container; it passed and the container was removed. The owner-reported 222 focused tests / 870 assertions and PHPStan pass support the behavior; current exact-head CI remains the merge gate.

This remains source authority only. No command upload/reference substitution, published Server/Worker delivery, or release publication is approved here; shared #228 stays open for the corrected published tuple and installed within/over-limit acceptance.

[rmcdaniel]

Merged as 57275ecb38c1a2b21bb4a1e2f051ae84609e7218 from exact Security-approved 697fb5e816f7adb015195c9628391c8dd0bb0fe5. All current-head checks pass; Sonar has zero open findings. The remote branch is removed and verified absent. Local focused checks passed 222 cases/870 assertions plus PHPStan.

This remains Unreleased with #406. Server owns the combined 3.9.1 producer qualification/publication for #1136/#1203; sdk-php owns the subsequent patch release and installed real Worker within/over-limit repeats. No native 3.8 completion delivery or external continuation-command authority is claimed. #228 stays open.

[rmcdaniel]

Security source review: PASS at exact head 697fb5e816f7adb015195c9628391c8dd0bb0fe5.

I reviewed the complete two-commit delta from 26f335d6a195b798c31ac7a033ed3cce126d4bf5, including the request planner, completion-context construction, and the new focused vectors. The authority boundaries are preserved:

• activity-continuation command payloads remain inline and are checked against the ordinary per-value limit; no command upload, result-reference substitution, or command-bound completion context is introduced, including when result and command bytes are identical;
• only the activity result can use the existing result lease/upload authority;
• malformed explicit aggregate metadata is rejected after discovery and before upload/submission even for empty, timer-only, and reference-only completion shapes on both completion routes;
• an authorized result rewrite that still cannot fit performs no partial upload and submits the original request once so the Server supplies the typed authoritative refusal;
• older-producer, configured-storage transient, individual-value, failure-report, ordinary-request, and namespace-cache limits remain distinct.

The exact head is terminal green across PHP 8.1–8.4, the regression corpus, static analysis, CodeQL, Sonar, public-boundary and target-branch qualification. git diff --check is clean. This local review checkout has no PHP/vendor toolchain, so I make no duplicate local execution claim.

Before merge, use the normal current-main refresh because the reviewed branch is behind current main, then provide an exact-head/tree countercheck with required checks green. This is a source verdict only: release/delivery remains held for the immutable published Server 3.9.1 tuple and actual within-/over-limit installed Worker acceptance; no #228 end-to-end closure is inferred here.

[rmcdaniel]

Correction to the sequencing sentence in my review above: this exact reviewed head had already merged normally as 57275ecb38c1a2b21bb4a1e2f051ae84609e7218; the remote branch is deleted and current accepted main includes the reviewed bytes with all six main workflows green. There is therefore no further #408 source refresh/merge gate. The stated release/delivery hold remains unchanged: immutable qualified Server 3.9.1 plus actual installed Worker within-/over-limit acceptance are still required.