Skip to content

Rust #325: accept completions and parallel groups over 100 - #851

Merged
rmcdaniel merged 8 commits into
mainfrom
rust-325-large-batches
Oct 29, 2026
Merged

rmcdaniel merged 8 commits into
mainfrom
rust-325-large-batches

Conversation

@rmcdaniel

Copy link
Copy Markdown
Member

Part of #325.

What changed

Native now accepts a workflow task completion with more than 100 commands, and a parallel group with more than 100 members, as published PHP 2.11.5 does.

Completions

  • Before:
    • Native answered 422 invalid_commands past 100 commands. The check was in two places: the completion itself (store.rs) and external payload preparation (external_resolution.rs).
    • The timer, activity and external-wait summaries stopped at 100 open waits, so a run with more than 100 was described without its wait.
  • Contract (published PHP): commands must be a non-empty array, with no count limit. StructuralLimits::guardCommandBatchSize applies only to the embedded executor. Each command meets the run's structural limits instead (2,000 pending activities and timers by default), and the run is described as waiting on its first open wait.
  • Native: both caps and the summary bounds are removed; a completion must still be non-empty.

Parallel groups

  • Before: native capped a group's declared size at 100 (all_groups.rs). It answered any invalid group metadata with 422 development_parallel_group_unqualified.

  • Contract (published PHP): WorkerController runs WorkflowCommandNormalizer::preflightParallelMetadata after the transport rules and integer normalization, before its own scope checks and any ownership guard. The preflight:

    • casts digit strings in the parallel integer fields;
    • refuses parallel fields on a command that is not a parallel leaf;
    • checks each leaf's metadata, with one of three messages under 422 validation_failed, collected across the batch.

    A group's declared size is bounded by StructuralLimits::commandBatchSizeLimit, 1,000 by default.

  • Native: completion_rules.rs ports the preflight at the same point. The batch-size limit is now one shared constant (memo_limits::COMMAND_BATCH_SIZE), used by the group, cooperative-cancellation and cancellation-scope checks; native already refuses to start with another value. A valid group that native does not run, such as a child group, is still refused later as development_parallel_group_unqualified.

Evidence

  • Fixture large-command-batches: raw worker HTTP on protocol 1.20, four cases. Each records the completion answer, the description (wait_kind, reason) and paged history counts by event type and group:

    • 150 start_timer commands;
    • 150 schedule_activity commands;
    • an all group of 150 activities, each with a one-entry parallel_group_path;
    • a group declaring 1,001 members (refused on both commands, nothing recorded).

    Recorded locally against published 2.11.5 and against a native image built from this branch: both pass.

  • Tests: rust/tests/execution/large_command_batches.rs (both tests) fail on the previous source and pass now.

  • Full native suites, run locally: execution 623 passed, 1 ignored; lib 347 passed, 42 ignored.

  • Contract unit tests: 9 pass.

Also: a load-sensitive test

updates_have_no_lifetime_cap_and_preserve_accepted_work_after_wait_timeout ran 64 update round trips without a worker heartbeat. On a loaded host the loop can outlast the worker's staleness window, and the server then correctly refuses the stale lease owner's completion (409 stale_worker_registration). The test now heartbeats each iteration. Eight concurrent local runs: 8 of 8 failed before, 8 of 8 pass after.

The port log's structural-limits entry also drops a stale claim: the 64-update lifetime cap was removed on October 23.

Not covered

  • A 1,001-command completion: published PHP answered 409 stale_worker_registration for it on SQLite, so there is no usable reference.
  • PHP's batch-wide error collection in the normalizer's other per-command checks: native still stops at the first command there. The new parallel preflight does collect across the batch.

🤖 Generated with Claude Code

@rmcdaniel

rmcdaniel commented Oct 29, 2026 •

Copy link
Copy Markdown
Member Author

At exact d8301e370208755491bf7896baef89d183e6dae8, please make the new parallel_entry range arithmetic overflow-safe before merge.

The scalar transport rules allow positive signed 64-bit integers with no upper bound below i64::MAX. For an otherwise valid select activity entry (and matching path entry), use:

{
  "parallel_group_id": "select-calls:1:2",
  "parallel_group_kind": "activity",
  "parallel_group_mode": "select",
  "parallel_group_base_sequence": 1,
  "parallel_group_size": 2,
  "parallel_group_index": 0,
  "selection_member_key": "a",
  "selection_member_index": 0,
  "selection_member_base_sequence": 1,
  "selection_member_size": 9223372036854775807,
  "selection_member_kind": "activity"
}

The new code reaches member_base + member in its bound check, which overflows. base + size has the same problem for a base near the signed maximum. Use checked arithmetic or equivalent safe bounds for both ends, preserving the normal validation refusal rather than relying on overflowing arithmetic.

I executed exact published Core 07e0d0e1a1185bd0a3bbd4e3fb56511cc57ca9bc's WorkflowCommandNormalizer::preflightParallelMetadata on PHP 8.3.36/64-bit with the default group limit. The ordinary base=1, size=2, member_base=1, member_size=1 control is accepted. Both the oversized member above and base=PHP_INT_MAX, size=2, member_base=PHP_INT_MAX, member_size=1 are refused with status 422 and:

{"commands.0.parallel_group_path":["Parallel workflow commands require complete top-level metadata and a non-empty group path."]}

Please add an extreme-integer regression/differential case alongside the 1,001-size case. Verification here is an executed published-Core preflight control plus exact Rust source inspection; no native or installed Server HTTP execution is claimed.

A related integer-range correction is needed for selection_member_key: the new predicate uses as_u64(), which also accepts raw JSON numbers above i64::MAX. On PHP 8.3.36/64-bit, executed JSON controls show 9223372036854775807 remains an integer, but 9223372036854775808 and 18446744073709551615 become floats. Published Core's validSelectionKey rejects those floats; it accepts nonnegative PHP integers and nonempty strings. A quoted "9223372036854775808" therefore remains valid. Keep numeric keys within the signed PHP integer range while retaining nonempty string keys. Use the ordinary small group/member bounds above to isolate this from the addition-overflow case. This additional check is PHP JSON-type execution plus exact-source inspection, not a native or Server HTTP result.

Re-review of c86adbfaaff9ec2e84f9fc2e28af5ffccd46ada9, retained in current acd020bd8c075b4f488cd8148afd45270e28c013, clears both reported source findings: the two select-range sums are checked, and numeric selection keys require a nonnegative signed 64-bit value while string keys remain accepted. The added native test checks all three refusals and an ordinary member with signed-max key. Its earlier-head panic and focused execution results remain author-attributed qualification; no independent Rust execution was performed for this review. The subsequent acd020bd change is test-only and checks the 150 group history entries through the public history API.

Please retain the offered published/native HTTP refusal recordings for the three extreme cases, including their path errors and absence of created work. Preserve 9223372036854775808 as a literal integer token in the request: a PHP fixture decode/encode round trip converts it to a float and would no longer exercise the original unsigned-integer predicate. These are narrow refusal cases; full select execution remains separate unfinished work. Normal exact-head CI remains required.

Update for exact ecaf587, including e95cd2f: the requested HTTP regression coverage is now present. I statically checked all three raw command strings, their matching top/path fields, literal integer key 9223372036854775808 in both locations, and expected 422 path errors with zero created tasks and unchanged start-only history. The probe sends the JSON text unchanged with application/json. This resolves the request-shape concern without executing PHP or Rust in this re-review. The published/native seven-case recording remains author-attributed in the linked qualification receipt.

The timeout change applies only to this fixture's overall runner budget (150 seconds plus the existing five-second wrapper margin). Hosted corpus failures at acd020b remain failed historical evidence; the author's locally reproduced 45-second timeout is a plausible cause, not confirmed identification of the failed hosted fixture. Normal exact-head CI is still required.

A separate runner diagnostic gap explains how fixture identity can disappear from retained summaries: the runner-blocked catch writes a raw record but no bounded failure outline, while artifact-summary intentionally excludes raw records. The existing failureExcerpt helper can preserve the current fixture/outcome/error heading for that path. This is a confirmed source-level evidence gap, not proof of the old corpus failure's cause.

Re-review of exact 2184ca9 after rebasing onto merged main: Git range-diff preserves the reviewed behavioral patches; integration differences retain the new query-contract fixture/CI entries and port-log additions. The fixture-count and timeout-cause wording are corrected. The new blocked-probe commit writes the existing bounded failure outline before throwing, registers a regression that runs the actual recorder with a failing PHP stand-in, and gives the database test steps accurate names. This addresses the retained-evidence finding by source review. The author's Node result and old-code failure remain attributed; I did not rerun the tests. No targeted source or fixture finding remains from this review.

Prior corpus failures stay failed evidence at their original heads. Source review and the new diagnostic regression do not prove their cause or replace normal exact-head hosted and cumulative qualification.

Final selector-only re-review at fd2a4b5: compared with 2184ca9, only ServerParityPartsTest.mjs changes. Its common-step matcher and three database-step lookups now use the renamed Bootstrap published prefix. This fixes the test selector mismatch introduced by the accurate step names; no production or probe behavior changes in this amendment. The author reports 78/78 aggregate/parts tests passing; I inspected the exact diff without rerunning them. Normal hosted checks on this head and protected cumulative qualification remain required.

Hosted qualification at exact fd2a4b5 is now terminal: independently read 56 successful checks and one expected skip, with no open or failed check. On October 29 at 05:35 UTC the pull request was in protected merge-queue position 1, awaiting cumulative checks. This clears the head-level CI gate; merge and cumulative qualification are still pending. No published-artifact or installed acceptance is implied.

Subsequent protected cumulative qualification at fd06b41 did not pass. Shared run 40824302270 ended with 40 successful jobs and three failures: SQLite corpus job 123284839194 failed the actual execute-and-compare step; MariaDB restart job 123284839698 failed the shared-node startup step; job 123290355973 is the aggregate failure. The primary annotations only report exit 1. Cumulative PHPUnit Feature 40824302255 and Public Boundary 40824302337 succeeded. At the post-failure read the PR remained open on fd2a4b5 and its queue entry was absent. The head-level pass above does not clear this failed cumulative gate. Exact fixture/startup causes remain under investigation; no rerun, budget increase, merge or installed acceptance is claimed.

The second cumulative attempt at 24f3607 also has a failed required SQLite corpus check: job 123292898252 failed the actual execute-and-compare step. Its annotation reports only exit 1; at the 06:32 read the run still had ten other jobs in progress. That overall in-progress state does not establish merge readiness. Exact-head CI repair #852 at 1dae4ff has separately reached 56 successful checks / one skip, but its protected cumulative qualification is still required. No new fixture cause is inferred here.

Re-review at exact 0cbdefb clears the fixture-cleanup change. The only delta from fd2a4b5 is ten lines after every case has captured its completion response, described state and paginated history counts. Cleanup uses the same fresh namespace/client and the same unique case workflow IDs, rejects responses outside 200/202, and then returns the already captured observations. The request bodies, raw integer tokens, comparison expectations and recorded pre-cleanup state are unchanged. This addresses the actual leftover-task load behind the scheduler failure; extending the separate observer timeout alone was not that cleanup. The published/native large-batches-to-schedule recording remains author-attributed. I inspected source without executing either target. No targeted source finding remains; normal checks on 0cbdefb and protected cumulative qualification are still required.

Normal qualification at cleanup head 0cbdefb is terminal: 56 successful checks and one expected skip. The primary read at 07:24 UTC shows the PR in protected queue position 1, awaiting cumulative checks. The earlier fd2a cumulative failures are preserved above; they are not failures of this new cleanup head. No targeted source finding remains from this review, and protected cumulative qualification is still required.

Protected qualification and merge are now verified at final cumulative 7825a8c: Shared Server fixtures 40834910755 succeeded at 07:43:00 UTC, PHPUnit Feature 40834910751 at 07:37:26 and Public Boundary 40834910741 at 07:22:11. PR #851 merged at 07:43:24 UTC as that same commit. This completes this source slice; it does not claim a published native image, full select execution, installed Server acceptance or cutover.

rmcdaniel added a commit that referenced this pull request Oct 29, 2026
From #851 review. The parallel preflight's selection member checks
added base + size and member base + member size without overflow
checks, so a member size of i64::MAX overflowed. It also accepted a
numeric member key above i64::MAX, which PHP decodes as a float and
refuses. Published PHP refuses both with the group path message; use
checked sums and the signed 64-bit range.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Pushed c86adbfa for both review findings on d8301e37:

  • Overflow: the selection member checks now use checked sums. A sum past i64 refuses with the group path message, as PHP's float sums fail its bounds.

  • Numeric selection keys: limited to the signed 64-bit range (as_i64() >= 0). A number above it, which PHP decodes as a float, now refuses. String keys are unchanged.

  • New test select_metadata_past_the_integer_range_is_refused_by_the_preflight refuses three select members with 422 validation_failed and the path message:

    • member size i64::MAX;
    • base i64::MAX;
    • key 9223372036854775808.

    An ordinary member with key i64::MAX passes the preflight. On d8301e37 this test panics with "attempt to add with overflow" at completion_rules.rs:720.

  • Run locally: execution tests matching parallel (13), selections (8), large_command (3) and cancellation (32) pass, and cargo test --lib passes 347. The full execution suite was not re-run for this commit; CI runs it.

  • Not recorded: the extreme cases are not in the HTTP fixture. Cloud's published Core control showed PHP's refusal. If a recorded case is wanted, I'll add it to large-command-batches.

rmcdaniel added a commit that referenced this pull request Oct 29, 2026
The group test counted members with payload LIKE, which PostgreSQL
refuses on its json payload column (operator does not exist: json ~~
unknown), failing both PostgreSQL native-test lanes on #851. Read the
run's history through the API instead, the same on every database.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rmcdaniel added a commit that referenced this pull request Oct 29, 2026
From #851 review. The large-command-batches fixture gains three raw-HTTP
cases that published 2.11.5 refuses in its parallel preflight with the
group path message and no created work: a selection member size of
i64::MAX, a group base of i64::MAX, and a numeric member key of 2^63.
Each case's commands are sent as written JSON text, so the 2^63 key
reaches the server as a literal integer rather than a PHP float.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

rmcdaniel commented Oct 29, 2026 •

Copy link
Copy Markdown
Member Author

Pushed ecaf5877. It adds two commits on top of acd020bd:

  • e95cd2f0, from review: three raw-HTTP refusal cases in large-command-batches:

    • select_member_overflow: member size 9223372036854775807;
    • select_base_overflow: group base 9223372036854775807;
    • select_key_overflow: numeric member key 9223372036854775808.

    Each case's commands are sent as written JSON text (raw_commands), so the 2^63 key reaches the server as a literal integer, not a PHP float. Published 2.11.5 refuses all three with 422 validation_failed, the error on commands.0.parallel_group_path, and nothing recorded.

  • ecaf5877, for the corpus failures on acd020bd: the probe gets a 150-second budget. It now runs seven cases, three with 150 commands. Against published PHP it timed out locally at the runner's 45-second default on a loaded host (spawnSync php ETIMEDOUT), and took 18 seconds on an idle one. That fits both corpus lanes failing at "compare" with no fixture named in the retained artifacts. This is the likely cause, not a confirmed one: the CI artifacts don't show which fixture failed.

  • Recorded locally: published 2.11.5 passes and a native image built from e95cd2f0 passes, all seven cases. ecaf5877 changes only the runner budget. The contract unit tests pass (11).

rmcdaniel added a commit that referenced this pull request Oct 29, 2026
The probe now runs seven cases, five of them with 150 commands each.
Against published PHP it took 18 seconds on an idle host, but timed
out at the runner's 45-second default on a loaded one (spawnSync php
ETIMEDOUT), which matches #851's corpus failures. Give it a 150-second
budget, as other long probes declare.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rmcdaniel and others added 5 commits October 29, 2026 04:40
Published PHP 2.11.5 accepts a workflow task completion with any
non-empty number of commands; each command meets the run's
structural limits instead (2,000 pending activities and timers by
default). Native refused more than 100 with invalid_commands, and its
timer, activity and external-wait summaries gave up past 100 open
waits, so such a run was described without its wait. Remove both
command caps and the three summary bounds.

PHP bounds a parallel group's declared size by the command batch
size (1,000 by default), checked in WorkflowCommandNormalizer's
preflightParallelMetadata. WorkerController runs it after the
transport rules and before any guard, and it collects one of three
validation_failed messages per command across the batch. Native
capped groups at 100 and answered any invalid metadata with
development_parallel_group_unqualified. Port the preflight into
completion_rules and share one COMMAND_BATCH_SIZE constant.

The large-command-batches fixture records 150 timers, 150
activities, a 150-member group and a refused 1,001-member group;
published PHP and native both pass it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
updates_have_no_lifetime_cap_and_preserve_accepted_work_after_wait_timeout
runs 64 update round trips without a worker heartbeat. On a loaded
host the loop outlasts the worker's staleness window, and the server
then correctly refuses the stale lease owner's completion
(409 stale_worker_registration) or hands out a different task. Eight
concurrent runs all failed before; with a heartbeat per iteration
all eight pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From #851 review. The parallel preflight's selection member checks
added base + size and member base + member size without overflow
checks, so a member size of i64::MAX overflowed. It also accepted a
numeric member key above i64::MAX, which PHP decodes as a float and
refuses. Published PHP refuses both with the group path message; use
checked sums and the signed 64-bit range.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The group test counted members with payload LIKE, which PostgreSQL
refuses on its json payload column (operator does not exist: json ~~
unknown), failing both PostgreSQL native-test lanes on #851. Read the
run's history through the API instead, the same on every database.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From #851 review. The large-command-batches fixture gains three raw-HTTP
cases that published 2.11.5 refuses in its parallel preflight with the
group path message and no created work: a selection member size of
i64::MAX, a group base of i64::MAX, and a numeric member key of 2^63.
Each case's commands are sent as written JSON text, so the 2^63 key
reaches the server as a literal integer rather than a PHP float.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel
rmcdaniel force-pushed the rust-325-large-batches branch from ecaf587 to a5cf0ee Compare October 29, 2026 04:53
rmcdaniel added a commit that referenced this pull request Oct 29, 2026
The probe now runs seven cases, five of them with 150 commands each.
Against published PHP it took 18 seconds on an idle host, but timed
out at the runner's 45-second default on a loaded one (spawnSync php
ETIMEDOUT), which matches #851's corpus failures. Give it a 150-second
budget, as other long probes declare.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Rebased onto main after #850 merged; the head is now a5cf0eed. The two conflicts were additive: the CI unit-test list in server-parity.yml and the port log, where #850's and #851's entries are both kept. Run locally on the rebased head: cargo test --test execution 627 passed, 1 ignored; cargo test --lib 358 passed, 42 ignored; the contract, query-contract, runner and fixture-selection unit tests 306 passed. The CI run on ecaf5877 was cancelled as superseded.

The probe now runs seven cases, three of them with 150 commands each.
Against published PHP it took 18 seconds on an idle host, but timed
out at the runner's 45-second default on a loaded one (spawnSync php
ETIMEDOUT), the likely cause of #851's corpus failures. Give it a
150-second budget, as other long probes declare.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel
rmcdaniel force-pushed the rust-325-large-batches branch from a5cf0ee to 2184ca9 Compare October 29, 2026 04:55
rmcdaniel added a commit that referenced this pull request Oct 29, 2026
From #851 review. When a probe fails before it observes anything, the
runner writes a runner-blocked record and stops, but only the
product-fail path wrote the failure excerpt that the CI artifact summary
keeps. A timed-out probe therefore left no fixture name in the retained
artifacts, as in #851's corpus failures. Write the bounded excerpt for
a blocked record too; the new runner test drives the recorder with a
failing fake php and checks the excerpt names the fixture.

Also rename the MySQL and PostgreSQL bootstrap steps to say they run the
native tests, since a failed native test was reported under "Bootstrap".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From #851 review. When a probe fails before it observes anything, the
runner writes a runner-blocked record and stops, but only the
product-fail path wrote the failure excerpt that the CI artifact summary
keeps. A timed-out probe therefore left no fixture name in the retained
artifacts, as in #851's corpus failures. Write the bounded excerpt for
a blocked record too; the new runner test drives the recorder with a
failing fake php and checks the excerpt names the fixture.

Also rename the MySQL and PostgreSQL bootstrap steps to say they run the
native tests, since a failed native test was reported under "Bootstrap".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel
rmcdaniel force-pushed the rust-325-large-batches branch from 2184ca9 to fd2a4b5 Compare October 29, 2026 05:10
@rmcdaniel
rmcdaniel added this pull request to the merge queue Oct 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 29, 2026
@rmcdaniel
rmcdaniel added this pull request to the merge queue Oct 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 29, 2026
#851's merge-queue corpus failed twice on schedule-fixed-rate-one-
occurrence: published PHP's installed scheduler did not fire within the
probe's ten seconds. The large-command-batches fixture, which the
corpus runs before it, left several hundred open timers and never-polled
activity tasks behind. PHP's maintenance loop then repaired 25 of them
per pass; one pass took 19.4 seconds locally and held up the scheduler
evaluation that follows it. The probe now terminates each of its runs
after recording them. Locally the passes after the fixture return to
zero repair candidates and about 1.7 seconds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Pushed 0cbdefb0. Both merge-queue runs failed on schedule-fixed-rate-one-occurrence; the second named it in the new failure excerpt. Published PHP's installed scheduler did not fire within the probe's ten seconds.

The cause is in this PR. The large-command-batches fixture runs before the schedule fixture and left several hundred open timers and never-polled activity tasks behind. PHP's maintenance loop then repaired 25 of them per pass. Locally one pass took 19.4 seconds, which holds up the schedule:evaluate that follows it in the same loop.

The probe now terminates each of its runs after recording them. Recorded locally on the new head:

  • Corpus order: published PHP passes large-command-batches followed by schedule-fixed-rate-one-occurrence in one run. Afterwards the maintenance passes show 0 repair candidates and about 1.7 seconds each.
  • Large batches alone: native passes from an image built from 0cbdefb0.

#852, now in the merge queue, separately retries the Composer floor build and widens the scheduler wait to 25 seconds. I'll re-enqueue this after #852 merges.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel
rmcdaniel added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit 7825a8c Oct 29, 2026
57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant