Skip to content

Prevent abandoned polls from claiming through explicit task offers - #758

Draft
rmcdaniel wants to merge 6 commits into
mainfrom
fix/526-task-delivery-offers
Draft

rmcdaniel wants to merge 6 commits into
mainfrom
fix/526-task-delivery-offers

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

A disconnected long poll must not start an activity attempt before a worker receives its task. This draft adds explicit /offers endpoints for workflow, activity, query and update-validation tasks. An offer leaves the task available to peers; positive acceptance invokes the existing claim with the original registration, task, attempt, queue and logical-poll fences. Accepted replies replay without another attempt, and late SQL acceptance rolls back the claim and business history together.

SQL receipts live in existing task rows with indexed poll keys. Multiplexed workflow/update-validation claims share the registration lock. Query receipts use the existing authoritative cache record and queue lock. Ordinary /poll behavior remains unchanged; opt-in headers on that old endpoint are explicitly refused. New endpoints let older nodes reject before claiming. The OpenAPI document advances to version 57; worker protocol remains 1.20.

Validated implementation 914b40fc1225a998bc4059c4e7075d3a43de761b passed 19 checks with 7 intentional skips. Hosted PHP: 3,146 tests / 72,061 assertions; local SQLite: 214 / 7,448; zero unresolved Sonar PR findings. Earlier MySQL and PostgreSQL SQL-claim qualification passed 43 / 1,343 each at 9048c5a. Current combined head be4ea6c8418d56294f0b9e968a4cede91c77fa3e includes main #757 and is requalifying; the PHP implementation and feature tests are unchanged. Evidence and CI corrections.

Still a draft: qualify current-head CI and additional adversarial concurrency; harden query/cache failure points and alternative offers; integrate all first-party SDKs; run actual proxy disconnect, Server restart and worker replacement separately with persistent/shared storage; then verify the fixed published Server artifact. Existing publication restrictions remain in force. This does not complete #526 or its original acceptance criteria.

Refs #526.

@rmcdaniel

Copy link
Copy Markdown
Member Author

General — current draft qualification at 9048c5a887cc2e963383c2dcd2fa49d1926d4cc7, tree e40b782b3bb6f26326261e4330703d5cf26f3b30.

  • SQLite: 207 tests / 7,371 assertions; MySQL 8.0.46: 43 / 1,343; PostgreSQL 17.11: 43 / 1,343. All passed with zero failures/errors/skips, using normal Core 2.13.1. Pint passed.
  • Two independently booted PHP HTTP-kernel processes race the same offer on each database. Both replies return the same task and the activity starts one attempt, within the unchanged two-second acceptance budget. Workflow/activity/update-validation tests also inject delay after real SQL claims and verify rollback of task state, activity attempts and history.
  • Routine-heartbeat replay exposed a real defect: the four focused lost-reply tests all failed before the correction. Exact registration credentials now distinguish incarnation from changing heartbeat timestamps; original routing/process fences remain enforced. The focused corrected group passed eight tests / 320 assertions, and the broader suites cover a heartbeat during acceptance plus changed build/process identity controls.
  • Query replay is read from its existing authoritative task record even after removing its leased index. The original query deadline remains unchanged. Competing multiplexed workflow/validation offers cannot both claim one logical SQL poll; abandoned offers remain immediately available to peers.

The first CI cycle at c07c05b8 ran 3,131 PHP tests with one error: rollback tried to drop an index after the existing database-error test deliberately removed the validation table. The new migration now handles that absent-table teardown; the original test still requires the actual operation to return an unclassified 500. The first process-race harness also needed readiness synchronization and the activity fixture's actual queue; those corrections did not increase the acceptance budget or relax assertions.

Current-head CI is pending. The six initial Sonar maintainability findings were addressed in source by separating packet handling, registration/replay checks and claim steps; latest-head analysis must verify that result. All task database containers, tmpfs data and the exact task network have been removed. No published Server image or full #526 acceptance is claimed. Remaining work includes alternative-offer/cache failure points, first-party SDK integration and the original real-proxy/restart/worker-replacement/published-image gates.

@rmcdaniel

rmcdaniel commented Oct 25, 2026 •

Copy link
Copy Markdown
Member Author

Latest verified qualification — 2026-10-25 08:50 UTC. Server head ed302326f0f73c383df88a918c8378755e0a56fe passes current SQLite 220 / 7,642 and current MySQL 8.0.46 plus PostgreSQL 17.11: 49 tests / 1,537 assertions each, zero failures/errors/skips. Cross-database coverage includes all six expiry regressions and concurrent positive acceptance. The first SQL fixture attempt failed before assertions because its readiness gate only checked the local socket; the corrected fixture proves TCP SQL authentication before tests. No product check was weakened. All fixture containers, networks and volumes are removed.

Hosted current-head PHP: 3,152 tests / 72,251 assertions, 37 existing fixture-dependent skips and six deprecations; additional 9/27 and 1/12 suites pass. All Actions jobs are terminal successful; GitHub rollup has 18 SUCCESS / 7 intended SKIP / 1 NEUTRAL (CodeQL aggregate), while all four individual CodeQL analyses passed. Sonar has zero unresolved PR findings.

PHP integration is visible in SDK draft #219, latest 13700088131c74a00fb3b656eb5d2c1c8c543d5a: 41 checks SUCCESS / 1 intended SKIP; PHP 8.3 2,858 tests / 14,911 assertions, 56 existing skips; PHP 8.1/8.2/8.4 also pass, local 106/488 and PHPStan. SDK Sonar has zero unresolved PR findings. Actual two-node PHP/MySQL/Redis/nginx tests at SDK 086b383 passed all three 4s offer polls behind 3s idle proxy; discarded positive replies replayed the same workflow/activity/query attempt after 3.1s and normal heartbeat on the other replica (0.279/0.075/0.085s); an abandoned offer left a peer able to claim in 0.138s. The query's public consumer used a direct control route and received its completed result. Later SDK terminal-stop/classification deltas are covered by focused and hosted checks. These are source/candidate results, not fixed published-image acceptance. Both PRs remain draft; Python/Rust integration and full original proxy/restart/replacement/published acceptance remain open.

Latest source qualification — 2026-10-25 08:19 UTC: current draft head ed302326f0f73c383df88a918c8378755e0a56fe fixes an expiry-reconciliation gap found during PHP SDK integration. An acceptance for a task claimed by a peer or removed could return ambiguous empty/unavailable indefinitely. The Server now serializes against in-flight positive acceptance under the registration lock and reads the actual task record. A live matching receipt remains replayable after packet expiry; otherwise it returns task_offer_expired without mutating another worker's authority. The original short-deadline rollback checks remain.

Current local SQLite qualification: 220 tests / 7,642 assertions, zero failures/errors/skips, plus Pint. This includes concurrent positive acceptance, normal heartbeats, SQL deadline rollback, workflow/activity/query/update-validation paths, diagnostics and transport. All six new expiry regressions fail against the preceding class, including two ambiguous200empty cases. Earlier MySQL/PostgreSQL43/1,343 results still apply only to the earlier documented SQL candidate; the new expiry guard is not yet claimed qualified on both engines. Current-head hosted checks are running. Previous combined head be4ea6c8418d56294f0b9e968a4cede91c77fa3e is now verified all19SUCCESS/7intendedSKIP.

PHP SDK offer/accept integration is locally committed and under actual HTTP/proxy qualification. Static analysis passes.105focused tests/485assertions pass; the first full2,853-test run found one old registration fixture, now corrected and passing in that focused suite. Nothing here claims a published SDK integration, fixed published Server image, full original #526 acceptance, or native implementation qualification.

General — query authority hardening at 914b40fc1225a998bc4059c4e7075d3a43de761b, tree b9e909cc2ec49d15d385de848e66d49ee1a90855.

The current SQLite suite passed 214 tests / 7,448 assertions, zero failures/errors/skips; Pint passed. Earlier MySQL and PostgreSQL SQL-claim qualification remains 43 tests / 1,343 assertions each at 9048c5a; the new query-cache changes are additionally qualified by the current SQLite/cache tests, not claimed as a new real-Redis/restart run.

Six cache failure cases cover exceptions/false writes before authority commit, exceptions after commit, lease-index exceptions/false writes, and pending-index failure. The original four controls failed against the prior implementation. Positive query acceptance now prepares and verifies its discovery index before committing its actual task record, then confirms the stored receipt before returning a task. It creates no provisional lease hint. A failed write leaves a pending task immediately available; an uncertain response after commit preserves the actual receipt for identical-token replay. A peer cannot claim the committed task.

A separate legacy failure control reproduced a stale lease hint blocking a genuinely pending query. It failed before the correction. New acceptance uses the queue lock and actual task record without deleting the old owner's hint, so that hint cannot reserve the task. Existing confirmed leases remain authoritative.

The second CI cycle at 9048c5a ran 3,139 PHP tests and failed only the two process-race assertions that demanded immediate200 despite typed SQLite backend_lock_pressure responses. The process harness now uses a 1 ms SQLite busy timeout and honors the normal typed backoff while reusing identical request bytes, token and poll ID. Both callers must ultimately return the same task and produce one activity attempt. The original two-second offer deadline is unchanged. This tests recovery through normal pressure rather than accepting pressure as task delivery.

All checks on the validated implementation head passed: 19 successful checks and 7 intentional skips, with no failures. The hosted PHP suite passed 3,146 tests / 72,061 assertions, plus the additional 9-test / 27-assertion and 1-test / 12-assertion suites. Its 37 existing fixture-dependent skips and 6 deprecations remain visible. Feature source qualification, ordinary MySQL replay/query HTTP topology, bounded-growth smoke and all CodeQL languages passed. Sonar finished with zero unresolved PR findings.

Current combined head be4ea6c8418d56294f0b9e968a4cede91c77fa3e incorporates the Rust owner's merged #757 CI changes (main 7ede756). The PHP implementation, configuration, migrations, routes, feature tests and process helper are unchanged from the validated implementation; only the accepted workflow and its Node parts test changed in this merge. Normal checks are re-running on that combined head: PHP qualification, bounded growth, CodeQL, and Sonar. Those jobs were confirmed in progress at 07:23 UTC; they are not yet counted as passed. No Server image was published, and this draft still does not complete #526. First-party SDK integration, further adversarial/mixed-node cases and real proxy/restart/worker-replacement/published-artifact acceptance remain required.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Source review of exact ed302326f0f73c383df88a918c8378755e0a56fe: query acceptance has a check/write window after queue-lock ownership validation.

commitQueryClaim() checks the queue lock before putTask(), but putTask() performs an unconditional cache replacement. A process pause or delayed cache write can cross the 10-second lock lifetime:

  1. A validates its offer, creates the receipt and passes the lock check, then pauses before the authority write reaches the cache.
  2. At 11 seconds, A's lock and original two-second offer have expired, while the original 30-second query deadline and lease are still live. B uses a different worker registration, obtains a fresh offer and accepts the still-pending task. B receives its execution-authorizing response.
  3. A resumes its unconditional write and replaces B's accepted record. The readback and assertQueryDelivery() / assertReceipt() now validate A's own replacement. They deliberately allow an existing receipt to replay after offer expiry, so no later offer-expiry check prevents A from also receiving execution authority for attempt 1.

The invariant needs an atomic authority transition that validates the lock/fence and expected task state together. A preceding ownership read, or a check after overwriting the peer, cannot preserve B's authority. Different worker-registration row locks do not serialize this shared query-cache record.

This is a source interleaving, not an executed concurrency result. General retains fixture/source ownership and has acknowledged that the inside-write pause is unqualified. The next proof should pause A at that boundary, let B accept through the actual shared cache, resume A and verify B's authority is unchanged and A receives no task. Exercise a database backend where the different registrations can proceed independently; a global SQLite writer lock must not accidentally mask the case. No native/shared/frozen fixture work is included in this review.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant