Skip to content

test(addie): stabilize PostgreSQL ledger race barriers - #7514

Merged
bokelley merged 1 commit into
mainfrom
conductor/stabilize-private-ledger-pg-races
Sep 24, 2026
Merged

bokelley merged 1 commit into
mainfrom
conductor/stabilize-private-ledger-pg-races

Conversation

@bokelley

@bokelley bokelley commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Two PostgreSQL race tests can return persistence_uncertain because the harness leaves their ordering to scheduling. This test-only change waits for external recovery's ordered complete-plan SELECT ... FOR UPDATE to return before releasing the precheck, and replaces the second race's two 75 ms sleeps with PID-specific pg_stat_activity/pg_blocking_pids barriers.

Both ordering tests use explicitly scoped 15-second test transaction budgets: acquiring the plan alone still reproduced uncertainty while recovery finished its deferred COMMIT checks on this VM. Barriers fail after five seconds; cleanup releases gates/locks and awaits outstanding work before closing sessions. The direct INSERT now also requires SQLSTATE P0001 and the exact terminal-assignment error, so a timeout cannot satisfy that assertion. External recovery recorded, pending intent unknown_exposure, final authorization unknown_exposure, and zero open intents remain exact requirements. The separate existing lock-failure regression is byte-identical and still requires persistence_uncertain. Its nominal 50 ms setting is subsequently overwritten by the unchanged 250 ms production transaction setup; the held lock still forces an actual lock failure.

Only server/tests/integration/addie/fixed-trace-component-smoke-private-ledger-migration.test.ts changes (+76/-13). All other tracked paths, including the production ledger, migrations, production 250 ms/1000 ms timeouts, private admission/deployment workflows, and fail-closed behavior, are byte-identical. No changeset.

Published candidate 046f5ab1af0bdf3a7146b8ae3a5efcb26467ef2b is based directly on deployed main cdcc8f996e8acd2417d5f7a547711cd21929289b; deploy run 34925749626 completed successfully for that exact main commit. Fresh exact-head PR CI is terminal green: 29 successful checks, zero pending or failing checks, and one expected Draft-only automated-review skip. Build Check attempt 1 completed successfully at the exact head; all four integration shards passed, and target shard 3/4 passed the target 36/36 and the shard 616 passed/7 skipped. Prior green CI belongs to the superseded head and is not used as approval evidence for this one. Green CI and automated audits do not satisfy the human approval gate.

Fresh local validation used Node 24.19.0, PostgreSQL 16 in Docker, an allowlisted environment, and fresh per-run databases:

  • Four candidate and four current-main exact-file runs at concurrency four: every run passed all 36 tests, including both changed races and the unchanged nominal-50 ms lock-failure regression. All eight runs ended with zero leaked client sessions. Passing baseline runs do not establish that the original timing race is absent.
  • The two fix(auth): allow trailing slashes on MCP CSRF exemptions #7533-affected CSRF/MCP unit files passed 75/75, covering root and per-tenant MCP trailing slashes, guarded near-misses, and bearer-auth routing. Ledger state/authorization units: 45 passed. Four neighboring PostgreSQL migration suites: 18 passed. Required precommit: 1,063 root tests plus guards passed; required typecheck, diff, PR title, changeset-scope, and immutable-artifact checks passed.
  • Exact local integration selection (npm run test:server-integration -- --shard=3/4): 39 files/616 tests passed, 1 file/7 tests skipped; target 36/36 passed. Local setup used ordinary migrations and explicitly skipped evaluator-only schema. Zero task client sessions remained after validation. This does not reproduce protected provisioning or validate production deployment.
  • Independent exact-head delta audit in a separate Conductor session verified the exact base/head/tree below: Ready to push; no blockers, issues, or nits, including deadlock safety, cleanup, strong assertions, and no interaction with intervening main changes.

Historical limitations remain documented: the original identical-patch campaign on 98fa207 ran 23 candidate and 23 baseline repetitions. Both modified races and the lock-failure regression passed all 23 candidate runs, while main's precheck race failed 11/23 times. Whole-file results were candidate 5/23 and main 10/23 passes; every candidate failure was in the unchanged neighboring independent-recovery or application-terminalization races, which also failed on main. This narrowly scoped patch does not claim to remove all file-level nondeterminism. Historical ad hoc compilation including the integration file also matched main's 17 existing transitive billing diagnostics; required typecheck passes.

Original public failure evidence was read before editing: main 98fa shard 3/4, #7507 attempt 1, and #7508.

Exact review/publication identity:

Item Value
Base (main) cdcc8f996e8acd2417d5f7a547711cd21929289b
Head 046f5ab1af0bdf3a7146b8ae3a5efcb26467ef2b
Head tree b1a249366cef5906b89658b7dd7f005f90156284
Target test blob 2f260ae61d6e44439db5c56cfc6bc5a6c44db507
Diff bytes (git -c core.abbrev=10 diff --binary BASE...HEAD) 10,830
Diff SHA-256 (same command) 61aba36cbc9b862f985dca0295d9b754d67cf7eeda73f128db65a086c911c12c
Range-diff against 2da35544..ef610b17e =

Object-ID abbreviation is pinned to 10 characters for reproducible diff bytes across clones. The rebased diff is byte-identical to the previously reviewed patch and was published with expected-head lease ef610b17e739e4746dbcbcf5fdf98039b90a5d29. Intervening main PR #7533 changes only MCP CSRF trailing-slash handling and its tests; independent review found no ledger, migration, database-session, timeout, harness, or fail-closed interaction. The separate lock-failure test block is byte-identical across the new base, prior head, and new head (SHA-256 f22d96eb4c91b53ae707d80e5c1562cb4cedb7c34473acf1b2f2d5af19cc93fe). No production/migration changes were introduced by this PR.

Unrelated PR retries before this patch lands remain probabilistic: a green retry does not incorporate the stabilization. After a reviewed landing, affected PRs need to rebase onto main containing this patch and rerun their exact-head checks for durable stabilization of these two races. The unchanged neighboring flakes remain outside this patch.

Actual non-author human review and approval of exact head 046f5ab1af0bdf3a7146b8ae3a5efcb26467ef2b is required and pending. The stale prior-head review request was removed before publication; after fresh exact-head CI became terminal green, the request to @bmilekic was renewed for this exact head. No actual human approval has been recorded yet. Any approval must be recorded by review ID, exact commit, and submission time; the controlled merge decision remains external. Please review ordering, cleanup, assertion strength, test-only timing allowances, residual flakes, and the exact evidence. Automated reviews and green checks do not satisfy the human gate. Keep this PR Draft, auto-merge off, and do not merge without the human gate. No merge is authorized by this qualification. Dependent stacks #7458, #7462, #7501, and #7513 remain held until this exact PR receives human approval and actually lands.

This does not close private security acceptance and does not validate production deployment of this PR. No deployment or protected-environment provisioning of this change was performed. Local validation explicitly skipped evaluator-only schema; fresh CI used its isolated PostgreSQL setup. Discussion is limited to the public test/implementation, CI logs, and local validation; no private unpublished work is included.


Open workspace in Conductor

Copy link
Copy Markdown
Contributor Author

Triage execution: this is the first PR priority because it removes the recurring nondeterministic PostgreSQL ledger-test failure that blocks multiple exact-credential security PRs. CI is green and the human maintainer review request is already present. Keep Draft until that human review is completed; then mark ready and merge before refreshing the dependent stack.

@bokelley
bokelley marked this pull request as ready for review September 23, 2026 15:03

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — test-only PR, no blocking findings.

This PR changes a single integration test file (+76/−13), replacing timing-based sleeps with deterministic PID-specific pg_blocking_pids barriers and a query-fragment signal to synchronize two PostgreSQL ledger ordering races. The reviewer verified barrier precision against the ledger source, deadlock-safe cleanup (gate/lock released before Promise.allSettled), 15s timeout overrides scoped to the test wrappers only (production bounds untouched), the lock-failure regression left intact, and the strengthened P0001 assertion. Changeset correctly omitted — the change is outside the published protocol surface.

No Critical/High/Medium findings. Not a schema/docs/dist/protocol change, no high-risk paths, no author team gate, gated_paths is false. Rows 1–8 do not fire; falls through to row 9.

@bokelley
bokelley merged commit 38c5d91 into main Sep 24, 2026
31 checks passed
@bokelley
bokelley deleted the conductor/stabilize-private-ledger-pg-races branch September 24, 2026 12:47
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