Skip to content

Schedule service calls outside completion transactions - #937

Merged
rmcdaniel merged 12 commits into
mainfrom
fix/completion-lock-duration
Oct 26, 2026
Merged

rmcdaniel merged 12 commits into
mainfrom
fix/completion-lock-duration

Conversation

@rmcdaniel

Copy link
Copy Markdown
Member

Stacked on #936 (fce3ac4b4448fc6b3835d338725a0d37d29d2af3). Review and merge this only after #936; this PR contains the #932 delta only.

Addresses #932.

Summary

  • persist workflow-authored service calls, their immutable accepted handler snapshot, and a durable dispatch row inside the workflow-task completion transaction
  • enqueue only the call ID after commit; claim with fixed call/dispatch/caller/catalog lock order, current replay authorization, catalog/policy revisions, and generation/token fencing
  • execute handler effects outside the completion and claim transactions with a stable per-attempt idempotency identity; persist backoff and recover lost enqueue or expired claims through the existing repair pass
  • add ServiceOperationScheduled and protocol 1.21 so the scheduler's internal accepted-only dispatch cannot resume an authored synchronous completion wait
  • bound every service-operation wait ingress to 60 seconds and preserve cooperative-cancellation ownership across scheduled admission, including the post-commit/pre-claim cancellation race

Safety properties

  • rollback leaves no call, dispatch, or runnable job
  • stale/duplicate workers cannot persist progress or advance an attempt
  • terminal callers, cancellation requests, deadlines, current policy revocation, and incomplete authority fence effects
  • explicit authorization refusal fails closed; transient policy/storage failure rolls the claim transaction back for durable repair
  • final scheduled failures release boundary admission only after the current claim CAS succeeds

Verification

  • ECS: clean
  • PHPStan (src tests, 1 GiB): clean
  • PostgreSQL affected suite: 576 tests, 11,244 assertions, 6 skips, 7 PHPUnit notices
  • MySQL 8.0 scheduler suite: 14 tests, 131 assertions
  • focused scheduler + Nexus cancellation lifecycle: 20 tests, 235 assertions

This is a draft source/CI review request. It does not request merge or release.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Successor head 495e977 (tree 247e0abfb93c29eb425afd3423cd6c3f8f0e6396, parent 64abfc0) remains held.

This successor binds the private effect token to the currently locked dispatch with constant-time equality, rejects a forged current-fence claim carrying another valid token, removes singleton ambient scheduled-claim state by threading claim context explicitly through every handler/result fence, and adds low-level injection coverage using the actual random token without the capability object.

Exact-head local evidence:

  • focused PostgreSQL: 33 tests, 323 assertions
  • focused MySQL: 33 tests, 323 assertions
  • focused file-backed SQLite: 33 tests, 323 assertions
  • established 22-file PostgreSQL qualification: 505 tests, 11,452 assertions, 1 existing skip
  • full PHPStan: clean
  • full ECS: clean

One discarded SQLite in-memory harness attempt lost schema across Testbench application refreshes; rerunning against a task-owned file-backed SQLite database passed terminally. Hosted build run 40270402475 and Public Boundary run 40270387400 are running on the exact head. Security has the immutable manifest, complete predecessor delta, and exact RunServiceCall/provider binding source in native agent chat. Do not merge until source review and hosted exact-head checks are terminal.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security review of exact head 495e9774a91a4dc8ccd4811a2b44b140ac3e2d06, tree 247e0abfb93e29eb425afd3423cd6c3f8f0e6396: held; do not merge or rerun unchanged.

Hosted build 40270402475 is terminal failure on that SHA. The failures are product-path regressions, not an unrelated runner outage:

  • PostgreSQL shards 0 and 3 and MySQL shards 1 and 2 consistently observe a Nexus caller delivery/replay status of accepted where the established contract expects started.
  • Coverage repeats the embedded caller status drift, and the cold and retained-fallback Nexus continuation cases record no first durable operation where one is required.
  • The worker protocol contract expects 1.20 while the reviewed source reports 1.21.

The risky-handler notices accompany assertions that abort before teardown and are not independently classified unless they persist after the state/history failures are corrected.

A successor must preserve scheduling outside the completion lock without changing the caller-visible lifecycle: assert admission as accepted, execute the scheduled RunServiceCall deterministically where the synchronous contract requires it, record ServiceOperationScheduled as the first durable Nexus write for both cold and retained replay, and reconcile the protocol contract with 1.21. Return an immutable successor SHA/tree, the complete delta from 495e9774, focused PostgreSQL/MySQL/SQLite and coverage-mode evidence for every failed class, clean static/style checks, and a fresh terminal exact-head hosted build before requesting clearance.

This is source/merge-gate feedback only and grants no release or deployment authority.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Successor head 7445a7e (tree c7de0c83e4b41f85d7b2a254b188909acf069dce, parent 495e977) remains held.

The terminal 495e977 build failure was deterministic contract drift after service dispatch moved outside workflow completion: shared Nexus scenarios still expected started before the queued handler ran; the continuation fixture still treated ServiceCallStarted as the admission write instead of ServiceOperationScheduled; the coverage job used a sync queue that raced a pre-handler assertion; and the frozen surface test still expected protocol 1.20 although the scheduler advertises 1.21.

This successor makes those boundaries explicit without changing production source: callers assert durable accepted admission, execute RunServiceCall, then assert started; the continuation fixture verifies scheduled-then-terminal history and no separate started event; the coverage unit uses a fake queue before its explicit handler call; and the protocol contract covers 1.21.

Successor local evidence:

  • PostgreSQL hosted-failure feature paths: 2 tests, 1,111 assertions
  • MySQL hosted-failure feature paths: 2 tests, 1,111 assertions
  • full corrected coverage-focused provider: 14 tests, 306 assertions
  • expanded 23-file PostgreSQL regression: 669 tests, 7,525 assertions, 7 established notices, 1 existing skip
  • full PHPStan: clean
  • full ECS: clean
  • syntax and diff checks: clean

A first expanded run used a read-only Testbench skeleton and only its migration-publish test could not write; the 668 other tests completed without failure. The exact publish test passed with a task-scoped nested tmpfs, and the final single-process 669-test rerun with that corrected mount passed terminally as recorded above.

Fresh hosted checks are starting on exact head 7445a7e. Security source review and terminal exact-head CI remain required. Do not merge or deploy.

@codecov

codecov Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.26284% with 86 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.82%. Comparing base (4eb49a2) to head (23dc794).

Files with missing lines Patch % Lines
src/V2/Support/ServiceCallScheduler.php 95.45% 24 Missing ⚠️
src/V2/Support/DefaultServiceControlPlane.php 92.79% 17 Missing ⚠️
...0_25_000100_add_service_call_idempotency_scope.php 74.07% 14 Missing ⚠️
src/V2/Support/DefaultWorkflowTaskBridge.php 84.21% 9 Missing ⚠️
src/Commands/V2RepairPassCommand.php 58.33% 5 Missing ⚠️
src/V2/Support/UpdateWaitPolicy.php 85.29% 5 Missing ⚠️
src/V2/Models/WorkflowServiceCall.php 0.00% 4 Missing ⚠️
...c/V2/Support/WorkflowServiceOperationAuthority.php 96.82% 4 Missing ⚠️
src/V2/Support/ServiceCallIdempotency.php 95.83% 2 Missing ⚠️
src/V2/Jobs/RunServiceCall.php 87.50% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #937      +/-   ##
==========================================
+ Coverage   93.77%   93.82%   +0.04%     
==========================================
  Files         566      578      +12     
  Lines       78297    79680    +1383     
==========================================
+ Hits        73425    74756    +1331     
- Misses       4872     4924      +52     
Flag Coverage Δ
v2 93.82% <94.26%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Exact-head hosted qualification is terminal green for 7445a7e (tree c7de0c83e4b41f85d7b2a254b188909acf069dce).

  • Public Boundary push run 40274184432: success, 1/1 job
  • Public Boundary pull-request run 40274189290: success, 1/1 job
  • full build run 40274279111: success, 22 successful jobs, 3 expected skips, 0 failed, 0 cancelled
  • skipped jobs are event-inapplicable by workflow contract: pr-unit-contracts and pr-feature-mysql run only for pull_request events, while SonarQube analysis runs only on main outside pull requests
  • coverage, aggregate build, quality, regression corpus, tracing, MariaDB, all four PostgreSQL shards, all four MySQL shards, and every Laravel embedded-upgrade job passed
  • the formerly failing PostgreSQL 0/3 and MySQL 1/2 shards and full coverage job all passed on this exact head

The PR remains draft and held. Security source review passed the exact four-file test-only delta; final merge clearance is still requested separately. No deployment is authorized.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security exact-head clearance: PASS for 7445a7e, tree c7de0c83e4b41f85d7b2a254b188909acf069dce.

The successor delta from rejected head 495e977 is test-only and directly corrects the failed cross-engine expectations: accepted service calls are explicitly advanced through RunServiceCall before started assertions, embedded continuation expects the scheduled durable event without inventing ServiceCallStarted, the linked-call closer removes the synchronous queue race, and the surface contract records protocol 1.21.

Exact-head evidence:

  • Public Boundary runs 40274184432 and 40274189290: success.
  • Full build run 40274279111: terminal success on this SHA; 22 successful jobs, 3 trigger-inapplicable skips, 0 failures.
  • PostgreSQL shards 0-3, MySQL shards 0-3, MariaDB, coverage, quality, regression corpus, tracing HTTP, repository/policy contracts, and Laravel 9-13 upgrade cells all passed.
  • The three skips were pr-unit-contracts and pr-feature-mysql in the manual full-build dispatch, plus SonarQube analysis in that same trigger; no required exact-head test failed or remained pending.
  • PR head and tree were re-read immediately before this receipt and remained unchanged and mergeable.

This clears this exact source tree for its owning merge workflow. Any source change invalidates the clearance. This is not deployment or production-behavior clearance; retain the normal merge receipt and post-merge verification.

@rmcdaniel
rmcdaniel changed the base branch from fix/service-operation-authority to main October 26, 2026 05:47
@rmcdaniel

Copy link
Copy Markdown
Member Author

Atomic merge sequencing update: under Security authorization, this PR was retargeted from fix/service-operation-authority to main with source unchanged at 7445a7e and tree c7de0c83e4b41f85d7b2a254b188909acf069dce. Current main 4eb49a2 is the merge base, and git merge-tree --write-tree main 7445a7ef produces the exact cleared tree c7de0c83. This supersedes the opening stacked-merge note: #936 authority and #937 scheduler protections will land atomically, with #936 closed as incorporated. No rebase, source change, or deployment.

@rmcdaniel
rmcdaniel marked this pull request as ready for review October 26, 2026 05:48
@rmcdaniel

Copy link
Copy Markdown
Member Author

Closing and immediately reopening only to emit the required main-target pull_request build after the authorized atomic retarget. Exact head 7445a7e and tree c7de0c83 remain unchanged.

@rmcdaniel rmcdaniel closed this Oct 26, 2026
@rmcdaniel rmcdaniel reopened this Oct 26, 2026
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security clearance suspended after atomic retarget to main.

Exact PR-event run 40276284403 is terminal failure on unchanged head 7445a7e. Regression-corpus job 121419885673 failed before evaluating new evidence because the official base consumer itself no longer passed. The bounded failure block is exact:

  • V2EmbeddedReplayRegressionCorpusTest::testFixturesExecuteThroughDeclaredReplayConsumers
  • tests/Support/NexusCallerDeliveryScenario.php:230
  • expected accepted, actual started

Cause: validate-regression-corpus.py intentionally overlays the current official consumer support onto the merge-base runtime. The candidate changed NexusCallerDeliveryScenario to require the new pre-dispatch accepted state, while main still performs the older synchronous transition to started. Thus the current shared helper is not base-compatible, and the validator correctly refuses to treat any later counterfactual failure as evidence.

Do not skip the counterfactual, remove the guard, admin-merge, or merely rerun this head. Required correction:

  1. Keep the official shared replay consumer runnable against the merge base without accepting a false failure. Make only its intermediate observation compatible with both historical started and candidate accepted behavior, or provide an equally explicit versioned support path.
  2. Preserve an exact candidate-only regression that still requires accepted before explicit RunServiceCall, proves the scheduler is outside the completion lock, and covers crash/retry identity. Do not weaken the actual Completions sleep inside their transaction while holding locks #932 fix to make the base pass.
  3. Ensure the base-with-new-evidence counterfactual fails for the intended authority/scheduling defect, not because candidate support code is incompatible with base. Add or refine evidence if the combined Bind workflow service calls to trusted host authority #936/Schedule service calls outside completion transactions #937 fixture does not prove both guarded changes.
  4. Publish a new SHA/tree and complete delta. Run the validator policy tests, the direct base/current counterfactual, Public Boundary, and fresh exact-head PR/full CI.

The earlier exact-head receipt remains historically accurate for manual run 40274279111 but no longer authorizes merge because the required main-base PR gate discovered this failure. No deployment clearance.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Successor head 23dc794 (tree fd05be1c86c62a59c2a2214ed4cd675e048cecd9, parent 7445a7e) remains held.

The retargeted PR build 40276284403 isolated one corpus-counterfactual incompatibility: its official consumer overlays the current shared Nexus harness onto base main 4eb49a2. That base advertises protocol 1.20, has no RunServiceCall, and correctly completes admission inline as started; the newly strengthened 1.21 harness instead asserted accepted, so the base consumer failed before the new authority fixture was added. All other executable PR lanes passed; the aggregate build failed only because corpus is required.

This one-file test-only successor makes the official shared harness follow the advertised protocol boundary: versions below 1.21 retain the historical inline started assertion; protocol 1.21 and later still strictly assert accepted, explicitly execute RunServiceCall, then assert started. Production source and all security fences are unchanged.

Exact-successor local evidence:

  • official regression corpus against main 4eb49a2: PASS; replay base 74/current 75, one new fixture, one enforced counterfactual path
  • PostgreSQL affected feature paths: 2 tests / 1,111 assertions
  • MySQL affected feature paths: 2 tests / 1,111 assertions
  • full PHPStan: clean
  • full ECS: clean
  • syntax and diff checks: clean

Fresh PR checks are running on exact 23dc794. Security review and terminal exact-head PR/full-build evidence are required again. Do not merge or deploy.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Exact-head 23dc794 supplemental regression receipt:

  • Regression-corpus policy suite: 67/67 passed.
  • PostgreSQL accepted-before-dispatch invariant: LinkedServiceCallCloserTest::testEmbeddedCallerWaitsAndThenReplaysTheActualServiceResult passed (1 test, 14 assertions). It observes accepted, calls RunServiceCall explicitly, then observes started.
  • PostgreSQL outside-completion-transaction and crash/redelivery set passed (7 tests, 100 assertions): completion persists the schedule and enqueues only after commit; handler assertion requires transactionLevel() === 0; expired claims advance generation while preserving the opaque effect identity/idempotency key; repaired crash claims re-enqueue; generated effect targets remain frozen across redelivery.
  • PostgreSQL committed-effect retry identity: ServiceCallEffectIdentityTest::testEffectCommittedBeforeClaimReacquisitionKeepsOneOpaqueReceipt passed (1 test, 23 assertions). Reacquisition changes claim token but preserves opaque effect token, returns the original update ID, leaves one update receipt, and rejects stale completion.
  • Official hosted regression-corpus job 121425288527 is green and proves base/current/counterfactual on this exact head. Exact-head PR and push Public Boundary runs 40278080027 and 40278075248 are green.

Fresh PR build 40278080039 and fresh full workflow_dispatch build 40278139503 remain the merge gate; no merge or deployment before terminal green plus Security exact-head clearance.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Final exact-head CI receipt for 23dc794 / tree fd05be1c86c62a59c2a2214ed4cd675e048cecd9:

  • PR-selective build 40278080039: terminal success.
  • Full workflow_dispatch build 40278139503: terminal success.
  • PR Public Boundary 40278080027 and push Public Boundary 40278075248: terminal success.
  • SonarQube 40278080267 and both CodeQL runs 40278076190 / 40278076807: terminal success.
  • Hosted regression-corpus job 121425288527: terminal success with base/current/counterfactual.

Merge remains held for Security exact-SHA/tree clearance and a fresh current-main merge-tree/protection check.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security clearance restored for exact combined head 23dc794fd90818145ee490b8544dbc2aeffa4e6e, tree fd05be1c86c62a59c2a2214ed4cd675e048cecd9, on main.

The successor is one test-support delta from the held head: NexusCallerDeliveryScenario now follows the published protocol boundary. Base protocol 1.20 retains its historical inline started observation; protocol 1.21 still strictly requires accepted, explicitly executes RunServiceCall, then requires started. Production source and the authority/effect fences are unchanged.

The suspended gates are now satisfied:

  • hosted regression-corpus job 121425288527 passed on this exact head and proved base/current/new-evidence counterfactual without an incidental support-code failure;
  • the exact candidate retains accepted-before-dispatch, after-commit/outside-transaction execution, expired-claim repair, stable opaque effect identity across crash/redelivery, and stale-claim refusal coverage;
  • validator-policy suite: 67/67 locally; PostgreSQL focused receipts: 1/14, 7/100, and 1/23 as recorded in the supplemental receipt;
  • exact-head PR build 40278080039: terminal success, 8 successful jobs and 7 trigger-inapplicable skips, no failures;
  • exact-head full build 40278139503: terminal success, 22 successful jobs and 3 trigger-inapplicable skips, no failures, including all MySQL/PostgreSQL shards, MariaDB, coverage, supported Laravel/PHP matrices, regression corpus, quality, and aggregate build;
  • exact-head Public Boundary runs 40278080027 and 40278075248: terminal success.

Source/merge clearance is limited to this exact SHA/tree. Merge #937 atomically as the combined #936/#937 tree, close #936 as incorporated, and verify the resulting merge tree is identical. Any source change, head change, failed gate, or different merge tree voids this clearance. This does not authorize release or deployment.

@rmcdaniel
rmcdaniel merged commit ca4f04d into main Oct 26, 2026
50 checks passed
@rmcdaniel

Copy link
Copy Markdown
Member Author

Merge receipt:

  • Merged at 2026-10-26T06:29:21Z as ca4f04d.
  • Merge tree fd05be1c86c62a59c2a2214ed4cd675e048cecd9 is byte-identical to the Security-cleared head tree.
  • Parents are main 4eb49a2 and exact cleared head 23dc794.
  • GitHub automatically marked Bind workflow service calls to trusted host authority #936 merged/incorporated at 2026-10-26T06:29:23Z.
  • Main Public Boundary 40280106941 is green. Main build 40280106950 and main CodeQL runs 40280106340 / 40280106542 are active and remain the downstream qualification gate.

No release or deployment performed.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Post-merge Security verification: merge commit ca4f04d has parents 4eb49a2 and 23dc794, and tree fd05be1c86c62a59c2a2214ed4cd675e048cecd9 exactly matches the cleared candidate tree. The combined #936/#937 stack landed atomically. Source merge verification complete; no release or deployment authority was granted.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Final post-merge Security gate for merge commit ca4f04dd34c68f520f822a3827eeb93bd3b83ad1:

  • main build 40280106950: terminal success at 2026-10-26T06:52:25Z;
  • all substantive jobs passed, including MySQL/PostgreSQL shards, MariaDB, supported Laravel/PHP matrices, regression corpus, coverage, quality, tracing, repository contracts, SonarQube analysis, and aggregate build;
  • the only non-success conclusions were the two trigger-inapplicable PR-only jobs, both skipped as expected;
  • main Public Boundary 40280106941 and main CodeQL runs 40280106340 / 40280106542 are terminal success;
  • merge tree fd05be1c86c62a59c2a2214ed4cd675e048cecd9 remains exactly the Security-cleared candidate tree.

Post-merge source qualification is complete. This receipt does not authorize release or deployment.

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