Skip to content

test(harness): bound the LazySavepoint deadlock partner via PartnerProcess (Closes #76, Ref #37) - #77

Merged
megyptm merged 2 commits into
mainfrom
claude/scoped-ordering-test-harness-fix-750536
Oct 23, 2026
Merged

megyptm merged 2 commits into
mainfrom
claude/scoped-ordering-test-harness-fix-750536

Conversation

@megyptm

@megyptm megyptm commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #76. Ref #37 (the legacy release remains NO-GO; no tag/release authorized by this PR).

Exact Git identity / scope

  • Base: main@9b06ec5d0847fb84287b14d000192b78e2db12d8, Tree d3446e86fd60fa62dbe407bd7db84445d39551a0.
  • HEAD: 7ea5499325bba8396634d93318ee9a4a1a7b4f84, Tree c1e25e7653a3173c3339a269b1ad23b26f335b9d.
  • Two regular commits: 43f5450 (reuse bounded partner) → 7ea5499 (F77-01 fail-closed rollback cleanup); no amend/force-push.
  • Cumulative diff: exactly two tests-only files, +166/−54: tests/Integration/Pdo/Ordering/ScopedOrderingManagerLazySavepointTest.php, tests/Integration/Support/PartnerProcessIntegrationTest.php. No changes to src/, published PDO/retry/transaction/savepoint APIs, existing PartnerProcess, Composer, CI, standards or security policy.

Full cumulative change

  • Reuses the merged PartnerProcess::start/readLine(30.0)/writeLine('go')/stop() to replace the original private proc_open + blocking fgets that disregarded the declared stream timeout. Preserves the 15-second blocked-locking-read wait, genuine MySQL InnoDB deadlocks, C10/C15 SQLSTATE 40001 / driver 1213 assertions, and original transaction/savepoint counters.
  • New testFrozenLazySavepointPartnerFailsWithinTheDeadlineAndIsReaped freezes before actual partner exec, verifies a bounded read timeout and process cleanup.
  • tearDown() attempts all partner stops, every inTransaction()/rollback, and schema drop even when an earlier step fails. It records thrown exceptions and a false rollback return. Exactly one cleanup failure is rethrown unchanged; multiple failures are reported with the earliest reason first and original Throwable at getPrevious(). This is not a guarantee that an OS-level child surviving SIGKILL is released.
  • Two committed real-MySQL regression tests: (1) kill an active MySQL session server-side so rollback really fails; verify failure surfaces, other transaction rolls back and schema drops; (2) force stop signaling failure on a real frozen process alongside a rollback failure, verify stop failure stays primary, later cleanup attempts continue, and the test process is killed/reaped in its finally.

RED/GREEN evidence (executor reports; Lead review evaluates independently)

  • Old code's stream_set_timeout(30) + fgets did not return under a real pre-exec frozen partner for 45 seconds; external watchdog freed it at 47.0 seconds (macOS; MySQL 8.4.10 in Docker).
  • New frozen-path negative: 1-second deadline (<5s) then verified child kill/reap.
  • F77-01 rollback-only RED: prior tearDown swallowed rollback exception and returned success; GREEN new path produces original PDOException.
  • F77-01 multi-failure RED: prior cleanup dropped rollback cause; GREEN now reports partner-stop as primary and additional failure in aggregate message.
  • ScopedOrderingManagerLazySavepointTest: original 23 tests unchanged, now 25 tests including the two F77-01 regression tests, three repeated full class runs GREEN. All local gates on PHP 8.4.4 with real MySQL 8.4.10: composer validate --strict, static analysis, syntax, whitespace, unit 383, regression 97, consumer, integration 231 tests twice, DB residue check and php-cs-fixer dry run all passed.

Exact HEAD GitHub CI

  • Run #39978250208, 7ea5499325bba8396634d93318ee9a4a1a7b4f84, 10/10 completed SUCCESS: Workflow lint, Quality PHP 8.2, Unit/Regression PHP 8.2–8.5, Lowest Dependencies PHP 8.2/MySQL 8.4.10, Integration PHP 8.2/8.5 on real MySQL 8.4.10, final CI Gate. Clean dependency advisory audit and PHPStan; Lowest 383 Unit + 97 Regression + 231 Integration twice, residue clean; both Integration jobs Consumer Verification Harness twice + 231 Integration twice, residue clean.
  • The local RED/GREEN evidence above is from the executor and was not independently fully re-executed by Lead. GitHub-hosted CI confirms the final suite across the supported matrix.

Limits and governance

🤖 Generated with Claude Code

…Process and clean up fail-safe (Ref #76, #37)

Replaces the hand-rolled proc_open/fgets partner handling (nominal 30 s
stream_set_timeout that does not bound a blocking read, teardown that
terminated and closed without checking) with the bounded PartnerProcess
from #75. tearDown now attempts every cleanup (stop partners, roll back
connections, drop schema) and rethrows the first failure afterwards.
Adds a frozen-LazySavepoint-partner negative to PartnerProcessIntegrationTest.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>

megyptm commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Technical Lead — Independent Fresh Full Re-Review: MERGE NO-GO (one bounded fail-closed cleanup finding)

Identity/scope: PR #77 OPEN, not merged, mergeable, single commit, Base main@9b06ec5d0847fb84287b14d000192b78e2db12d8, Tree d3446e86fd60fa62dbe407bd7db84445d39551a0, HEAD 43f5450d87a2e89f0fba6c35c10b5d1f60697b97, Tree 665ba817e7cb444caadc94377963ae02aca7494a, direct parent = Base, ahead 1/behind 0. Reviewed complete cumulative diff, two tests/ files +60/−53: ScopedOrderingManagerLazySavepointTest.php and PartnerProcessIntegrationTest.php. No Runtime src/, PDO/savepoint/ordering public contract, Composer, CI, pinned standards or release changes. Review threads: none.

Accepted code: The LazySavepoint tests reuse existing PartnerProcess::start/readLine(30.0)/writeLine('go')/stop() rather than ineffective stream_set_timeout + blocking fgets, while preserving the 15-second real-lock synchronization and original 23 tests' SQLSTATE 40001 / driver 1213 / transaction and savepoint strictness. New test reliably freezes the actual named LazySavepoint partner script before exec, checks bounded timeout and child reaping. The modified tearDown() attempts all partner stops, connection rollbacks and schema drop, preserving the first partner-stop or schema-drop failure.

CI is GREEN, not the blocker: exact HEAD run #39975565444, 10/10 SUCCESS. Actual logs independently read: Lowest job 120489107485 security audit clean, PHPStan zero errors, 383 Unit, 97 Regression, 229 Integration twice, MySQL residue clean; PHP 8.2/8.5 Integration jobs 120489108006 and 120489107977: each Consumer Harness two successes, 229 Integration tests twice and residue clean. Final CI Gate 120490863319 confirms all jobs success. Old 47s watchdog RED/new frozen-child GREEN and cleanup fault probe are executor-reported, not independently rerun by Lead.

F77-01 — BLOCKER: rollback failure silently discarded in claimed fail-closed cleanup

Location: tests/Integration/Pdo/Ordering/ScopedOrderingManagerLazySavepointTest.php:77–85. The new tearDown() accumulates partner-stop and schema-drop failures in $failure, but the transaction loop still has:

try {
    if ($connection->inTransaction()) {
        $connection->rollBack();
    }
} catch (Throwable) {
}

A thrown inTransaction()/rollBack() failure is thus silently suppressed. If it is the only cleanup failure, teardown can incorrectly report success; if a partner stop already failed, the additional rollback failure loses diagnostic trace. This empty catch was present in the legacy method but contradicts the new advertised first-failure policy and explicit #76 fail-closed cleanup requirement. A successful normal MySQL/CI test is not evidence for this fault branch.

Required smallest remediation within the SAME PR: capture/propagate the first rollback or transaction-state failure using the same first-failure accumulator without skipping remaining connections or schema drop; handle rollBack() === false appropriately to PDO/test contract. Preserve an earlier partner-stop error as primary when appropriate; disclose supplemental cleanup errors rather than silently presenting complete success. Supply deterministic, bounded fault-injection RED/GREEN proving (a) rollback-only failure is surfaced, (b) failed stop still permits rollback and schema-drop attempts, and residue is checked. A small committed regression for the fail-closed path is preferable where practical; if scratch-only proof, state why. No changes to production/runtime, deps, shared PartnerProcess or test assertions. In the new docblock, change "Every owned resource is released" to an honest "Every cleanup step is attempted", since the already-documented SIGKILL-resistant child case cannot guarantee full release.

Decision: MERGE NO-GO on exact HEAD 43f5450d87a2e89f0fba6c35c10b5d1f60697b97 despite 10/10 CI. One normal follow-up commit (no amend/force-push), updated PR identity/proof, applicable local tests and exact-head full CI, then Fresh Full Re-Review. #76 remains OPEN; release tracker #37 remains RELEASE NO-GO; unrelated #66 and new-package migration are untouched. No auto-merge or Owner merge authorized.

…ailure cleanup honest (Ref #76, #37, F77-01)

tearDown no longer swallows inTransaction()/rollBack() failures (a false
rollBack() counts too). All failures are collected after every cleanup step
was attempted: one is rethrown as is, several are reported with the first as
primary (a failed PartnerProcess::stop() stays first). Docblock now promises
attempts, not guaranteed release. Adds two regression tests: a real
rollback failure (killed server session) and a failed partner stop with the
database cleanup still performed.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@megyptm

megyptm commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

F77-01 addressed in a new commit (no amend/force-push).

  • Base 9b06ec5d0847fb84287b14d000192b78e2db12d8; previous HEAD 43f5450; new HEAD 7ea5499325bba8396634d93318ee9a4a1a7b4f84, Tree c1e25e7653a3173c3339a269b1ad23b26f335b9d. Only ScopedOrderingManagerLazySavepointTest.php changed (no src/, PartnerProcess, Composer, CI).
  • tearDown() now collects every failure (partner stops, inTransaction()/rollBack() incl. a false return, schema drop) after attempting all steps; one failure is rethrown as is, several are reported with the first (a failed stop stays first) as primary and the rest disclosed. Docblock now says "Every cleanup step is attempted", not guaranteed release.
  • Two committed regression tests (real MySQL, deterministic):
    1. rollback-only failure: caller connection killed server-side → real rollBack failure; asserts a PDOException surfaces, the other connection is still rolled back and the schema dropped.
    2. failed stop (frozen child, signals undeliverable) + also a rollback failure: asserts the stop failure is primary, additional failure disclosed, rollback and schema drop still performed; the child is reaped in finally.
  • RED (new tests vs old tearDown): test 1 — nothing thrown ("null is not PDOException"); test 2 — stop message not wrapped, rollback failure lost.
  • GREEN: LazySavepoint class 25 tests OK ×3. Local gates on MySQL 8.4.10: validate, analyse, syntax, whitespace, unit 383, regression 97, consumer, integration ×2 (231 tests), residue, cs-fixer 0 — all OK.
  • Local run is macOS/Docker; exact-head CI result follows when it completes. Not merging.

megyptm commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Technical Lead — FINAL Fresh Full Acceptance Re-Review #2: MERGE GO — exact cumulative Tree

Independently verified identity & scope

  • PR #77 OPEN, not merged, not Draft, mergeable, no unresolved review threads.
  • Base main@9b06ec5d0847fb84287b14d000192b78e2db12d8 / Tree d3446e86fd60fa62dbe407bd7db84445d39551a0 still current.
  • HEAD 7ea5499325bba8396634d93318ee9a4a1a7b4f84 / Tree c1e25e7653a3173c3339a269b1ad23b26f335b9d; 2 chronological ordinary commits: 43f5450d87a2e89f0fba6c35c10b5d1f60697b97 → 7ea5499325bba8396634d93318ee9a4a1a7b4f84. Cumulative two tests/ files, +166/−54, no changes to src/, public contracts, SQL/runtime, transaction/ordering algorithms, Composer PHP platform, CI, pinned standards or release policy. PR metadata updated by Lead to match final HEAD/Tree, 25 LazySavepoint class tests and 231 Integration tests.
  • Read AGENTS.md, Pinned Adoption/Manifest and applicable Testing/AI Collaboration/CI requirements; checked complete accumulated patch, original deadlock assertions and cleanup paths, helper and MySQL schema fixtures, not only the last commit.

Prior finding F77-01 — CLOSED / PASS

  • ScopedOrderingManagerLazySavepointTest::tearDown():64–112: attempts all partner stops, inTransaction()/rollbacks and schema drop; records all thrown cleanup failures and a rollBack() === false result. Single failure is rethrown as its original Throwable; multiple are reported in order via RuntimeException with the first Throwable in getPrevious(). A failed partner stop stays primary; later rollback/schema steps are still attempted.
  • The docblock now correctly promises all cleanup steps attempted, not absolute release of an unkillable child. Previously suppressed rollback exceptions no longer disappear.
  • Two committed real-MySQL negatives: testTearDownSurfacesARollbackFailureAndStillCleansTheRest kills an open server session and asserts original PDOException + other rollback + clean schema; testTearDownKeepsAFailedPartnerStopPrimaryAndStillCleansTheDatabase combines an undeliverable signal to a frozen child and failed rollback, checks primary error + additional diagnostic + further cleanup and finally reaps the child. These tests materially cover the exact rejected failure branch. All original 23 LazySavepoint tests retain their strict deadlock (40001/1213), caller-owned transaction/savepoint and ordering assertions.
  • Original test(reliability): bound LazySavepoint deadlock partner reads and teardown #76 PartnerProcess integration remains correct: start/readLine(30.0)/writeLine('go')/stop() replacing ineffective stream_set_timeout+blocking fgets; the new frozen-before-exec test validates bounded read and reaping. No changes in the already reviewed shared PartnerProcess.
  • Executor's local RED/GREEN probes (47s watchdog on old read, old rollback failure suppressed, aggregate failure lost) and latest PHP 8.4.4/MySQL 8.4.10 tests remain executor-attributed; Lead inspected the actual new test code and independently checked final GitHub CI, not independently reran local RED.

Exact-head GitHub CI — 10/10 COMPLETED SUCCESS

Run #39978250208, attempt 1, exact HEAD 7ea5499325bba8396634d93318ee9a4a1a7b4f84; all Workflow lint, Quality PHP 8.2, Unit/Regression PHP 8.2–8.5, Lowest PHP 8.2/MySQL 8.4.10, Integration PHP 8.2/8.5/MySQL 8.4.10 and final CI Gate succeeded. Actual GitHub logs read independently:

  • Lowest job 120498442664: Security Audit 0 advisories, PHPStan no errors, 383 Unit / 2690 assertions, 97 Regression / 1085 assertions, 231 Integration twice (1747/1743 assertions), MySQL residue clean; lowest resolution phpunit 11.5.50 + symfony/process 5.4.51.
  • Integration jobs PHP 8.2 120498442544 and PHP 8.5 120498442802: Consumer Verification Harness two clean runs each, 231 Integration tests twice each, MySQL residue clean.
  • Final CI Gate 120500290467 explicitly confirms required jobs succeeded.

Decision & remaining governance

FINAL MERGE GO for the exact HEAD 7ea5499325bba8396634d93318ee9a4a1a7b4f84 and Tree c1e25e7653a3173c3339a269b1ad23b26f335b9d, no remaining code-blocking findings in the cumulative accepted #76 test-harness scope. The isolated #68 2 vs 4 retry failure cause remains UNVERIFIED and is not claimed cured. Published old maatify/persistence public API/PHP >=8.2 preserved; legacy #37 remains OPEN/RELEASE NO-GO and #66 F4-04 non-compliance has not been waived. This is not a release or extraction authorization.

Only Owner may authorize Squash Merge into main; Lead has not merged, enabled auto-merge or tagged anything. After Owner merges the exact reviewed HEAD, reconcile squash commit and merged Git Tree with c1e25e7653a3173c3339a269b1ad23b26f335b9d, verify the main-push 10/10 CI including MySQL/Consumer Harness/residue, then close #76 as completed and update #37. Next separate boundary: frozen extraction baseline + architecture for new maatify/php-persistence PHP ^8.4 package and consumer migration.

@megyptm
megyptm merged commit 1f6a7aa into main Oct 23, 2026
10 checks passed
@megyptm
megyptm deleted the claude/scoped-ordering-test-harness-fix-750536 branch October 23, 2026 16:17
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.

test(reliability): bound LazySavepoint deadlock partner reads and teardown

1 participant