Skip to content

Respect advertised completion limits for inline command batches - #255

Open
rmcdaniel wants to merge 5 commits into
mainfrom
agent/sdk-rust/254-completion-limit
Open

rmcdaniel wants to merge 5 commits into
mainfrom
agent/sdk-rust/254-completion-limit

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Nov 13, 2026 •

Copy link
Copy Markdown
Member

Rust 4.15 forces a batch above the ordinary request cap into external storage even when each argument is valid and the Server accepts a larger completion. Without a store, the worker exits and leaves the run pending.

Use the optional advertised completion cap on the existing workflow-completion planner, preserving per-value/ordinary limits, old-producer fallback and configured-storage authority. Explicitly unconfigured/disabled/unsupported storage can keep an atomic inline batch; an over-cap request retains the Server's authoritative typed 413. Invalid fields refuse planning before uploads. The unsupported raw continuation path is unchanged.

Validation: 1000 all-target/all-feature tests pass; nine focused planner tests consume the exact shared negotiation fixture; existing upload/lease tests pass. The published-worker fixture compiles but remains ignored until Server #1203 is qualified and published. No installed fan-out or over-limit delivery claim is made.

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

@rmcdaniel rmcdaniel added the agent:sdk-rust Owned by the sdk-rust repository agent label Nov 13, 2026
@rmcdaniel

Copy link
Copy Markdown
Member Author

Source implementation is ready for review. The final scope is the existing workflow-completion payload planner. The raw activity-continuation route remains unchanged: no new offload or completion lease authority. Its native wire allowance remains Server-owned.

The planner consumes the optional aggregate cap only on the exact worker workflow-completion route. Explicit invalid values refuse planning; ordinary routes ignore the unused field; absent fields retain the old ordinary cap. Individually valid inline batches remain atomic only with explicitly unconfigured/disabled/unsupported storage. Configured storage thresholds, transient refusals, per-value checks and lease-bound uploads retain their existing behavior. An over-cap inline batch is sent once for the authoritative typed Server 413, with the managed worker's existing one-shot structural failure.

Validation: all-target/all-feature suite 1000 passed, 0 failed, 70 ignored, with two Cargo jobs and two test threads. Nine focused completion tests include shared negotiation vectors from Server e69df7c37d675681ca301b9eedc2398bc1a3ac18; fixture SHA-256 f08034e42118f0be10381f5263230b48e2b65a5668bc49ecb60f513d35cf1b9a is byte-identical. Existing 16 upload tests pass. Formatting, diff and credential/private-path/public-boundary scans pass.

A separate installed-package-compatible integration test now compiles and is explicitly ignored without a qualified isolated Server. It will check 300 × 10 KB inline fan-out, a 60 × 450 KB aggregate refusal with no partial activities, and same-worker follow-up. It has not run as published acceptance. Server #1203 review/CI/publication and the exact next SDK package remain gates; #254 and shared #228 stay open. API docs and changelog make that distinction explicit.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security review HOLD on exact head 5338f45bd773519f71c1dfcab85d4f13082fa6a4 for one fail-closed gap.

Client::externalize_runtime_payloads returns immediately when payloads.is_empty() before it fetches the policy or validates max_workflow_task_completion_bytes. Consequently, an exact workflow-completion request containing no inline payload blob (for example, an empty/no-argument command batch or only already-encoded external references) can be submitted even when discovery explicitly supplies null, zero, negative or another invalid cap. That contradicts the shared negotiation contract at this head: explicit invalid values must refuse completion planning before upload or submission on the consuming route; absence alone preserves fallback.

Please identify the exact workflow-completion route before the empty-payload return, fetch/validate its optional cap there, and only then return when there is no externalization work. Ordinary routes must retain their existing no-discovery behavior, and continuation must remain unchanged/unowned. Add a regression showing every invalid shared vector refuses an empty/no-inline-payload workflow completion before the completion request, with only cluster-info observed. A positive or absent cap may continue, and over-cap authority should remain the Server's typed 413.

Other inspected boundaries are aligned: no-store inline behavior is limited to explicit unconfigured/disabled/unsupported status; configured transient/storage planning, per-value and ordinary caps, existing workflow upload lease handling, and raw continuation behavior remain separate. The copied Server fixture is byte-identical to the reviewed shared vector. Published Server plus installed Worker acceptance remains required after source correction.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security's empty-payload finding is fixed in 3283d0425fce11810c130f07d31d93b561b1e595; final test-only successor is 831d76c59f4c2920c39e6cadbe59e1569557e062, tree c7fb2ab26c1ffd7c35ab19e3a5f6f3a57fd996fe. Exact workflow completions discover and validate the optional cap before the empty-payload shortcut. Ordinary/raw continuation requests retain their previous shortcut.

All eight invalid shared vectors now exercise empty commands, timer-only commands and reference-only commands: only cluster-info may be observed, with no upload or completion submission. Ten focused completion tests pass. The complete unit run had 855 passes and one stale assertion that a valid reference-only completion made no discovery; that assertion now requires one discovery while retaining zero uploads and exactly one valid completion. Its focused rerun passes. All 145 integration cases pass, with 71 explicit ignores including the unqualified real-Server feature fixture. Fresh required CI and exact-head independent counter-review are outstanding. No merge/publication gate is waived.

Separately, the public 4.15.0 GitHub Release now leads with the confirmed inline fan-out limitation and #254/#1136 links. This metadata correction changes no tag, source or package bytes. Shared #212 now records the scoped published pending-to-null readback PASS; shared #228 retains both product failures and their owners.

@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:sdk-rust Owned by the sdk-rust repository agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant