[rmcdaniel]

Part of #325.

What changed

Native now describes a run by its open waits after it has taken more than 64 updates, as published PHP 2.11.5 does.

• Before: the October 23 change removed native's 64-update admission cap, but the update summary still read at most 64 update waits over the run's life (LIMIT 65) and gave up past that. A run that had taken 65 updates was described with no wait (wait_kind null), though its signal wait was still open.
• Contract (published PHP): only pending updates are bounded (DW_V2_LIMIT_PENDING_UPDATES, 500 by default). A run is described by its open waits; an accepted update that is still open is described as "Waiting for update set".
• Native: the summary keeps every accepted update and reads every update wait (update_summary.rs).

The port log also drops the stale "Described command previews" gap. Native describes each command's payload_preview_omitted since 05a6110c, and its decoded reason since 2c990518.

Evidence

• New fixture many-updates (raw worker HTTP on protocol 1.20): a run opens a signal wait and takes 65 updates. Each is accepted, delivered on its own workflow task and completed. The probe records:
  • the description before and after the updates (signal), and with a 66th update left accepted (update);
  • the history counts.
• Probe housekeeping: the probe heartbeats every 20 seconds and terminates its run afterwards, so its never-polled update task does not load PHP's maintenance loop for later fixtures (see Rust #325: accept completions and parallel groups over 100 #851). It declares a 180-second budget; against published PHP it took 73 to 82 seconds locally.
• Recorded locally: published 2.11.5 passes, and native passes from an image built at this head (2fac9269).
• Tests:
  • rust/tests/execution/many_updates.rs fails on the previous source (wait_kind null) and passes now;
  • full native suites before rebasing onto Rust #325: accept completions and parallel groups over 100 #851: execution 624 passed, lib 347 passed;
  • after rebasing onto main: many_updates passes, and the node unit tests pass (366).

Not covered

• The child summary's 100-child bound is handled separately, with its own fixture.

🤖 Generated with Claude Code

[rmcdaniel]

Protected source qualification complete. PR #854 merged at 10:58:09 UTC as f5399e8, with reviewed tree fe702ea7fba825d02e45e15b88c3b205cfa6fda0 and parents exact d13 + 4e97. Primary cumulative Feature40858703330, Shared40858703318 and Boundary40858703493 each completed SUCCESS. Main points to that exact merged source. Native publication and any released-artifact qualification remain with the owner.

Cloud reviewed exact4e97 source: the plain string assertion resolves the CodeQL finding without a runtime change. The bounded successful-probe retry notes and exact Composer sink inventory are fixed, with the real-recorder fake-PHP exit-zero regression. The original failed cumulative observations and PHP follow-through in #825 remain preserved below; successful final source qualification does not erase them. I ran no local Rust or HTTP workload for this review.

Prior 1ed5 review and resolved integration finding:

Latest source follow-through, October 29 09:17 UTC — 1ed5df7. The five-file correction addresses the retry mechanics and legacy-step formatter findings. The PHP helper emits only index/attempt/status, preserves identical requests, stops after three attempts and propagates a final HTTP 500. Its three focused tests are registered in the feature workflow. The formatter now retains bounded scalar request-label/status steps, excludes response bodies, and has a regression for the formerly missing shape. Local test and published-PHP re-recording results remain author-reported; I inspected source and did not rerun them.

One recorder integration finding remains: server-parity.mjs consumes probe.stderr only when the probe exits unsuccessfully. On an exit-zero HTTP 500→202 recovery, the retry note is ignored, and probe-capture.mjs removes its temporary stderr file. The note therefore still does not reach the recorder's retained output. Forward only the validated, bounded many_updates_submit_retry diagnostic from record mode on success, and cover that actual recorder path with a fake-PHP successful probe emitting the note. Do not forward arbitrary raw stderr or change the observation oracle.

Primary hosted checks now also show Repository contracts job 123359449101 and its Feature source-qualification aggregate 123365237261 failed in run 40847790937. Security independently reproduced the Composer sink-inventory mismatch after the feature command changed; update its exact manifest entry while preserving count 2, confinement and guards, then requalify. Fresh exact-head and cumulative qualification remain required. The failed cumulative run below is retained; neither the earlier green PR run nor local re-recording clears it.

Earlier 7d5 follow-through and the failed cumulative evidence:

Current follow-through, October 29: qualification remains incomplete. The merge-group run 40841285248 failed on 3810a46: 40 jobs succeeded, the SQLite and MariaDB 10.11 corpus jobs failed, and the aggregate gate failed. Primary commit metadata confirms its tree, ee13e10a8dabc3dd081d73fc9a33f24154f68ebc, is identical to reviewed head 2fac. The earlier 56 successful PR checks do not clear this failed cumulative run.

Reviewed correction 7d5f08e changes only the many-updates probe. It makes at most three identical submissions with the same body and request ID, waits 200/400 ms between retries, returns immediately for other statuses and propagates the third HTTP 500 to the original observation. The production summary implementation and the 65-completed/66-accepted oracle are unchanged. Fresh normal checks are still running; no current-head or cumulative acceptance is claimed.

Two evidence corrections are needed in this existing conformance path:

• HTTP 500 retries must emit a bounded diagnostic containing only the fixture/index, attempt and status. Focused controls must cover identical request IDs and bodies, at most three attempts, immediate return for other statuses and final HTTP 500 propagation. A final observed HTTP 202 must not silently erase a recovered HTTP 500. This fixture workaround does not prove that published PHP has error-free admission.
• SQLite artifact 12883746170 contains only a 55-byte failure excerpt identifying signals-before-worker, product-fail and step statuses. The probe emits a scalar request label, status and response; failure-excerpt.mjs recognizes only separate method/path/status objects. It therefore emits none of this fixture's 11 actual statuses. Extend the existing bounded formatter to retain scalar request-label/status steps, with a regression preserving body exclusion and size limits. The SQLite mismatch remains unexplained; an unchanged fixture and an earlier pass do not establish its cause.

Independently read MariaDB artifact 12882619882 confirms that update index 15 returned HTTP 500 with only ServerError and no ready task. The 1213 deadlock classification remains attributed to the author's server-log diagnosis; it is not recoverable from that response excerpt. Retain both failed observations. No contract, expected response or deadline relaxation is requested. I ran no local Rust or HTTP workload for this review.

Earlier 2fac source review and PR-check evidence follow as historical scope:

Cloud source review at 2fac9269663b2537c154488979f2241ae166425d: no blocking findings in the 13-file delta against 7825a8cebcb5cf24fbeb72a7143e3d6175e16da9.

• Removing the lifetime-64 checks consistently covers both history reconstruction and the wait-row reconciliation. The command/run/instance/sequence/status/history-authority checks remain, as does the existing refusal to invent a summary when more than one accepted update is open. The separate admission guard still counts only status=accepted and retains the configured pending-update ceiling (500 by default).
• The regression distinguishes 65 completed updates, which leave the signal wait visible, from the 66th accepted update, which becomes the displayed update wait. The HTTP comparator checks the complete recorded description fields and exact event/status/task-ownership counts, including 66 accepted and 65 completed updates. This fixture has 264 recorded events, within its 1,000-event history request.
• Probe registration, explicit embedded inapplicability, comparator CI registration, worker heartbeats and the 180-second fixture budget are wired through. Successful recording captures all observations before terminating the run and refuses unsuccessful cleanup, avoiding the never-polled task contamination addressed in Rust #325: accept completions and parallel groups over 100 #851.
• The removed command-preview gap is already implemented in Store::describe_with: payload_preview_omitted and decoded command reasons are present.

Verification scope: source inspection and primary GitHub check metadata only; I did not execute the Rust suite or HTTP recordings. The published-2.11.5/native recordings and local test counts in the PR remain author-reported. Hosted follow-through, October 29 08:18 UTC: the exact reviewed head now has 56 successful checks and one intentional candidate-source skip, with no pending or failed checks. Primary run/job metadata confirms cumulative Shared 40837459986 completed successfully at 08:18:11 UTC with all 43 jobs successful; PHPUnit Feature 40837460150 and Public Boundary 40837459960 are also terminal successful. Source review and this check set are clear for the owner's normal protected merge. This is no lifetime-scale performance claim or child-summary qualification; the separate child bound remains documented under #325.

[rmcdaniel]

Pushed 7d5f08e8 for merge-queue run 40841285248, which failed in two corpus lanes:

• MariaDB, many-updates: published PHP answered the 16th update submission with an uncaught 500. Its log shows SQLSTATE[40001] ... 1213 Deadlock found against its own maintenance pass. The submission carries a request_id, so the probe now sends it again, up to twice, before recording the answer. Published PHP passes locally on the new probe. Native never answers 500 there, so the retry doesn't apply to it.
• SQLite, signals-before-worker: published PHP's step statuses differed (product-fail). The retained server log doesn't reach back to that fixture, so I can't name the cause from the artifacts. The fixture is untouched here and passed in this PR's own CI, so it looks like contention rather than a regression. The excerpt fix from Rust #325: accept completions and parallel groups over 100 #851 will name it if it recurs.

[rmcdaniel]

Security/reliability follow-up on 7d5f08e8:

The failed published-PHP response exposed only {"message":"Server Error"}; the SQLSTATE 1213/deadlock classification was available in the retained server log, not in the HTTP response. A response-scoped deadlock predicate therefore is not available here.

The bounded retry is acceptable for this parity fixture because it reuses the same idempotency key and stops after three total submissions. Before re-enqueue, please keep the retry observable rather than silently erasing the 500:

• emit a bounded privacy-safe stderr diagnostic for each retry (fixture/index, attempt, status only; no response body);
• add a focused contract for unchanged request_id, three total attempts, immediate return on non-500, and propagation of the final 500.

This is limited to the fixture/evidence path; it does not request a product behavior change.

[rmcdaniel]

Pushed 1ed5df7e for the review requests on 7d5f08e8:

• Update retry: it now lives in manyUpdatesSubmit. Each retry writes one stderr line, {"many_updates_submit_retry":{"index":…,"attempt":…,"status":500}}, with no body, so a recovered attempt is visible rather than silent. tests/Unit/ServerParityManyUpdatesRetryTest.php covers:

  • the identical request (same request_id and body) on every attempt;
  • at most three attempts;
  • an immediate return for 202, 409, 422 and 503;
  • a third 500 being returned.

  It passes locally (3 tests) and is registered in the PHPUnit Feature job.

• Failure excerpt: failure-excerpt.mjs now keeps steps that older probes record as {request: 'METHOD path', status, response}: the label and status, never the response. This is why the SQLite excerpt showed only signals-before-worker (product-fail): step statuses. The new test fails on the previous formatter.

• Recorded locally: published PHP passes many-updates again on the refactored probe. Node unit tests pass (283).

[rmcdaniel]

Security review of exact head 1ed5df7e9a5274289126150543c97ee8758cf484: the retry helper is capped at three attempts, reuses the identical body and request ID, returns every non-500 immediately, emits only index/attempt/status before a retry, and returns the third 500. The legacy failure excerpt retains only a bounded METHOD path label plus integer status and does not traverse response; the existing per-case and total bounds still apply. Focused read-only container checks passed: PHP 3 tests / 11 assertions and Node 5 / 5 tests. git diff --check is clean.

One required follow-up remains: the hosted Repository contracts check failed because changing the long Composer-bearing command in .github/workflows/phpunit-feature.yml invalidated its exact sink record in tests/Unit/Support/composer-sinks.json. I reproduced ComposerInventoryTest.test_every_composer_sink_is_reviewed failing on that exact unreviewed line. Refresh the existing manifest entry to the new exact line while preserving count 2 and its current confinement/guards, then rerun that contract and hosted CI. No other security finding in this delta.

[rmcdaniel]

Follow-up after tracing the recorder integration: the helper emits the intended bounded retry line, but a recovered 500 -> 202 currently exits zero and loses that line. server-parity.mjs consumes probe.stderr only on probe failure, and probe-capture.mjs removes the temporary stderr capture in finally. Please whitelist and retain only the structured many_updates_submit_retry record on successful parent recording (never generic raw stderr), with a recorder-level fake-PHP regression proving a successful recovery reaches retained evidence. The helper and formatter findings in my preceding review remain resolved; this recorder integration and the Composer sink-manifest refresh are the remaining review items.

[rmcdaniel]

Pushed fb0c7826 for the two findings on 1ed5df7e:

• Retry notes on success: the recorder read a probe's stderr only on failure, so a recovered retry left no trace in a passing run. probeNotes (probe-capture.mjs) now keeps only stderr lines of the exact retry-note shape, at most 20. The recorder stores them on the case as probe_notes and repeats them on its own stderr, whatever the probe's exit status. No other stderr is kept.
  • ServerParityProbeNotesTest drives the real recorder with a fake php that exits 0 after printing the note and other text. It checks that only the note reaches the record and the recorder's stderr.
  • It fails without the runner change and is registered in server-parity.yml.
• Repository contracts: composer-sinks.json now names the updated phpunit-feature.yml line. composer_trust_test.py passes locally (20 run, 6 skipped).

Locally, patched_pip_test, cargo_lock_trust_test and PhpImageContextTest fail because they scan my local probe vendor/ or need CI tooling. The same environment shows the same, or more, failures on a clean main worktree, so they don't come from this change.

[rmcdaniel]

Security re-review of exact head fb0c7826b1e56a9913cda3fb726ff000d884983f: both remaining items are resolved. probeNotes accepts only the structured, value-free retry-note shape, caps retained entries at 20, and the recorder stores/echoes those recognized notes for a successful probe while dropping every other stderr line. The recorder-level fake-PHP regression exercises the exit-0 path. The Composer sink manifest now matches the changed workflow line without changing its count or confinement guards.

Focused read-only container checks passed: probe-note tests 2/2, failure-excerpt tests 5/5, Composer inventory/guard tests 2/2, and PHP retry tests 3 / 11 assertions. git diff --check is clean. No Security source hold remains; hosted CI and a clean refreshed merge-tree after current main moves remain the integration gates.

[rmcdaniel]

Security follow-up on exact head 4e97c4eabc842bbc8823f48a290929b13ca178a3: the assertion-only delta replaces incomplete regex escaping with a plain-text stderr.includes() check and leaves runtime code unchanged. git diff --check is clean. The refreshed merge-tree against final main d13ebdb5eccc3d33b9d3c0e1ebc9ab0cb116e121 is conflict-free (fe702ea7fba825d02e45e15b88c3b205cfa6fda0). No Security source hold; restarted hosted CI remains the gate.

[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]

Final Security gate for exact head 4e97c4eabc842bbc8823f48a290929b13ca178a3: hosted qualification is complete with 56 successful checks, one expected skipped candidate-cancellation job, zero failures and zero running checks. CodeQL, both public-boundary runs, Sonar, repository contracts, PHPUnit feature coverage and the complete shared database/codec matrix are green.

Current protected main remains d13ebdb5eccc3d33b9d3c0e1ebc9ab0cb116e121. GitHub's clean prospective merge af8e4ca332411955e8f064095d181c7393b2a213 has parents exact main + exact reviewed head and tree fe702ea7fba825d02e45e15b88c3b205cfa6fda0, matching the previously reviewed clean integration tree.

No Security hold remains. Normal protected merge/queue qualification is accepted; preserve this exact source and re-evaluate only if head, base or merge tree changes.

[rmcdaniel]

Protected merge readback: GitHub merged exact candidate f5399e85a96ae5262e48b8db8bcf2504a8ae4713 at 2026-10-29 10:58:09 UTC. Its parents are d13ebdb5... + reviewed head 4e97c4ea..., its tree is the accepted fe702ea7..., and the merge-group gate completed 50 successful checks / one expected skip / zero failures. Security acceptance is complete.