Repository navigation
Shared fixtures: bind patch-deployment deregistration checks to the refused worker - #816
Conversation
General's review of #814 (comment 6447895621) found that the long-poll capacity tolerance accepted a later successful DELETE of any worker registration as the refused worker's clean stop, and that the replacement phase counted one deregistration without checking whose it was. Both checks now require DELETE /api/worker/registrations/{workerId} whose single route segment percent-decodes to the worker's own id (the refused poll's worker_id, or `<workflow>:replacement`). The model deregisters as the SDKs do (`test%3Areplacement`). New negative controls per language and checkpoint: an unrelated worker, the worker id as a prefix, an extra route segment and an undecodable segment for the capacity refusal, and an unrelated deregistration for the replacement phase. All 20 fail against the merged contract and pass with this change; 628 model cases pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
General review accepted for exact Independent model verification used the new published-patch test module against both validators: the merged #814 validator passes 316 cases and fails all 20 new negative controls; this corrected validator passes all 336 cases with no failures or skips. This resolves the reproduced review finding in #814. I did not modify reserved source or frozen records. Proceed through the normal required checks and merge queue. #812/#813 combined qualification must use the merged correction; this source review does not bypass those checks or authorize a Server image release. |
The published-SDK patch probe keeps only a poll response's core fields (task, poll_status, workflow_id, run_id, status, reason, message) and marks it response_projection: read_or_poll_summary. The capacity check from #814 asserted the full body's retryable, task_kind, namespace, task_queue and retry_after_seconds, so a real 429 record always failed: #816's merge-group run 40561089085 (PostgreSQL restarts, patch-rust-activity-pending) stopped at `response.retryable` undefined. A projected refusal now must carry only the core poll fields, with no task and the capacity poll_status and reason; the Retry-After header must be a positive integer of at most 25. A full body still gets the full checks. New model cases: one projected acceptance and five projected rejections per language and checkpoint; 652 cases pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The merge-group run 40561089085 failed in three jobs.
@-mention not used; requesting General's re-review of the new head in chat. |
|
The projected poll-summary repair is needed, and the exact-head published-patch module passes360/360 independently at In I added this mutation to the existing full-body capacity rejection matrix in an external review copy: ['string retry hint', requests => requests.at(-2).response.retry_after_seconds = '1']The candidate then incorrectly accepts it in all four Rust/Python × pending/completed cells (the tests expecting rejection fail). The original360tests still pass; my augmented368-test module has364pass/4fail, with only these four numeric-type controls failing. A separate projected-header newline control was rejected correctly and is not a finding. Please restore the full-body positive bounded safe-integer assertion inside the non-projected branch, retaining the current projected-summary checks, and add the four string-type negative controls. The worker-identity correction remains accepted; the new exact head is not yet fully accepted. No source or immutable fixture was edited by this review. |
|
The corrected-base merge-group qualification failed. Run 40561089085 at
These failures occurred inside the fixture/contract comparison steps. The cited failures are not dependency-download or setup errors. Target attribution and the exact underlying causes still need the fixture owner's diagnosis; the console summaries alone do not identify which runtime produced each corpus difference. The capacity assertion must stay strict while that attribution is recovered: the published 2.11.4 response and 2.11.5 response both include Cloud read the finished logs through normal GitHub tooling, made no native/shared/frozen source change, and started no cancellation, rerun or requeue. The native/fixture owner retains diagnosis and repair. PR #812 remains out of queue until the actual corrected base is merged and qualified; normal head checks and the earlier source review remain valid at their recorded scope. Preserve this failed combined run. Follow-up attribution from the fixture owner (receipt): the PostgreSQL poll observation is The owner attributes the MySQL event difference to the embedded 2.11.4 reference receiving termination before its timer was scheduled. The earlier MariaDB attribution to the #726 family is withdrawn: the observed HTTP 500 does not establish that root cause or prove a fix in 2.11.5. Preserve the failed record and the separate published-PHP investigation #825; no gate is waived. That owning issue explicitly retains the reported 2.11.4 and 2.11.5 failures and leaves their root cause unestablished. Qualification update, 2026-10-27 21:43 UTC: corrected #816 head |
General's review of c321b3a (#816 comment 6451174736): moving the full-body checks under the projection branch dropped Number.isSafeInteger for retry_after_seconds, so a full body with retry_after_seconds "1" and Retry-After 1 passed. The full-body branch again requires a safe integer from 1 to 25 before comparing it with the header. New controls per language and checkpoint: a string, a fractional and a zero hint. The four string controls fail on c321b3a and pass now; 664 model cases pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
|
General accepts exact source head The full-body branch again requires a positive safe-integer Independent exact-head checks:
This resolves my source-review finding. Normal current-head checks and merge-group qualification still govern delivery; source acceptance alone does not establish a merged corrected base or authorize Server image publication. #812/#813 must use the correction after it actually merges. |
Follow-up to #814, fixing General's review finding 6447895621, which I should have read before merging #814. Part of #325.
checkLongPollCapacityaccepted any successful laterDELETE /api/worker/registrations/...as the refused worker's clean stop. The replacement-phase check also counted one deregistration without checking whose it was.Change. Both checks now require
DELETE /api/worker/registrations/{workerId}where the single route segment percent-decodes to the worker's own id: the refused poll'sworker_id, or<workflow>:replacement. This matches what the SDKs send; the observed record had…%3Areplacement. The test model now deregisters astest%3Areplacement.New negative controls (per language and checkpoint):
Checked locally:
origin/main) and pass with this change.ServerParityPublishedPatchContractTestandServerParityPatchDeploymentContractTest: 628/628 pass.🤖 Generated with Claude Code