Add a testcontainers-backed live-Postgres harness; wire it into CI (#102) - #118
Merged
Merged
Conversation
) .github/workflows/ci.yml ran a bare `cargo test --workspace` with no `--ignored`, so all 14 `*_live_postgres.rs`/`*_live.rs` suites in this workspace were silently skipped in CI and had never once run there — the exact blind spot that let `claim_live_postgres.rs`'s own policy-gap bug (missing `hasRole('system')` on Message.list/detail) sit undetected for a full milestone, per AGENTS.md's own account of #29. This adds `crates/sms-test-support`, a shared harness that starts (or reuses) one Postgres 16 container and hands back a fully migrated connection URL, and wires a new `live` job into CI running `cargo test --workspace -- --ignored`. `just test-live` / `just test-live-clean` give the same thing locally. Every one of the 14 suites now goes through `sms_test_support::database_url()` instead of reading `DATABASE_URL` from the environment. Container lifecycle deliberately avoids the usual `testcontainers::ContainerAsync` cached in a process-level `static`: Rust never runs `Drop` for a `static`'s contents on any process-exit path, so that pattern leaks a container per test binary with no bound — confirmed the hard way when an earlier attempt at this same harness accumulated 56 orphaned containers and filled the machine's RAM. Instead this manages exactly one container by shelling out to `docker` directly, keyed by a fixed name (`vsms-test-harness-postgres`) and a distinctive label (`dev.vsms.test-harness=true`) that scopes every cleanup operation this crate ever performs — verified end to end: baseline 8 containers on this machine before any run, 9 after two full `just test-live` sweeps (one cold, one immediately after, warm), never more. Two real bugs found only by actually running the harness against a live database, not by reading the code: - `apply_migrations_if_needed`'s original migration check ran `SELECT to_regclass('public.messages')` decoded into `Option<String>`. `to_regclass` returns `regclass`, which sqlx cannot decode as a string — except when the value is NULL, which decodes without ever consulting the declared type. So the query worked exactly once, against a fresh database where the table doesn't exist yet, and failed for every caller afterward once `0001_init` had actually run — surfacing as flaky ordering across suites rather than a decode error. One test binary took 9704 seconds before the fix (`::text` cast) and 0.70s after. - All 14 binaries originally shared one `vsms` database inside the container, and that database persists across runs by design (so reruns don't pay Postgres's startup/migration cost again). Most suites tolerate the resulting cross-run row accumulation — they scope queries by a freshly generated id — but `app/sms-worker/tests/ kill9_reclaim_live.rs` spawns a real `sms-worker --roles dispatch` process whose claim loop deliberately selects *any* eligible candidate row, the same way production does. Run after the other 13 suites left claimable rows behind, it picked up their leftovers instead of its own seeded message and timed out — passing in isolation (2.6s) but failing under the full sweep. This is the cross-binary half of the #102 problem `ca653a1` could only ever fix *within* one binary (its per-binary `TEST_MUTEX`es are unchanged and still needed for that reason — tests within one binary still share that binary's own database). Fixed by giving every test binary its own database inside the one shared container (`crates/sms-test-support/src/lib.rs`, `ensure_binary_database`/`binary_database_name`), derived deterministically from the test binary's own executable name so reruns of an unchanged binary reuse rather than accumulate. Each database is dropped (`WITH (FORCE)`, Postgres 13+) and recreated from a shared, lazily-migrated template (`vsms_template`) via `CREATE DATABASE ... TEMPLATE` on every call — a filesystem-level copy, far cheaper than running both migration files 14 times — and the drop-then-recreate, not just create-if-absent, is what actually stops row accumulation across reruns. `kill9_reclaim_live.rs` needed no changes for this beyond already reading its URL from `database_url()` and passing it explicitly to the subprocess it spawns via `--database-url`. Naming this deterministically hit a second real collision, again found only by inspecting `psql -l` live: an initial version stripped Cargo's `-<hex-metadata-hash>` suffix from the binary's file name for cosmetic/rebuild-survival reasons, which silently reunited `crates/sms-auth/tests/live_postgres.rs` and `crates/sms-worker/tests/ live_postgres.rs` — two different packages' test files that happen to share a name — onto one shared database, since Cargo's own `target/debug/deps/` is flat and workspace-wide and the hash suffix is the only thing in the executable's path that tells them apart. Fixed by keeping the hash; a rebuild that changes it just mints a differently- named database rather than a collision. Verified: two full `just test-live` sweeps (cold via `test-live-clean`, then warm) both fully green — all 14 suites, 63 tests. Also green: `just check`, `just lint`, `just test`, `just parity`, `./ci/assert-no-raw-sqlx.sh` (this crate is a fourth, allowlisted R1 exception alongside `sms-worker/src/lease.rs`, `sms-worker/src/notify.rs`, and `sms-api/src/cache.rs`). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
stephane-segning
force-pushed
the
claude/testcontainers-harness
branch
from
August 6, 2026 23:05
6ad2332 to
5705944
Compare
3 tasks
stephane-segning
added a commit
that referenced
this pull request
Aug 7, 2026
CI reproduced a real, ~30%-flaky failure in this suite (3/10 runs), not a fluke: two different tests' wiremock .expect(1) mocks intermittently saw 2 requests instead of 1. Root cause: dispatch::tick's claim loop (claim.rs::candidates()) selects any eligible accepted/queued/routed message system-wide with an expired-or-absent lease -- correctly matching production, since the loop has no way to know which test seeded which row. This binary's database (sms-test-support's per-binary design, #118) is shared by every test in the file and never reset between runs. TEST_MUTEX already serializes execution, but does nothing about residual state: a message a previous test left non-terminal is exactly as claimable as the row the current test is about to seed, and claim_batch's budget (up to tps_ceiling rows per tick) means a single tick() can claim both in the same batch -- so a test's own tick() submits a foreign leftover message to its own wiremock server on top of its own legitimate one. That mismatch is what tripped the affected mocks' .expect(1), and it is order-dependent (hence intermittent), not a data race. Fixed by draining the claimable backlog to a terminal state (cancelled, reachable directly from all three claimable states per §2.10) before every test seeds its own message, through CrateStack delegates only (R1) -- an isolated_db() helper that every test now calls instead of db() directly, so isolation can't be forgotten at a new call site. Under the same TEST_MUTEX every test already holds, so by induction the candidate set at the start of any test contains only rows that test itself goes on to seed. Verified: 15 consecutive clean runs of `cargo test -p sms-worker --test dispatch_live_postgres -- --ignored` (0 failures across 120 individual test executions, well past the 10-run bar). Two full `just test-live` runs both green across all 14 suites/66 tests; one interleaved run hit the pre-existing, unrelated sms-auth::rbac_layer2_live_postgres pg_type-cache flake (#102) in a file this change never touches, confirmed by passing cleanly in isolation immediately after. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 tasks
stephane-segning
added a commit
that referenced
this pull request
Aug 7, 2026
Three real bugs in the live-suite gate, each found by actually running it repeatedly, not by reading the code: 1. rbac_layer2_live_postgres.rs (added in #112, the day after #102's own fix landed in ca653a1) never picked up ca653a1's per-binary TEST_MUTEX pattern. Its 7 tests all call setup(), which calls rotate_signing_key (an insert) plus several distinct seeding query shapes, racing on Postgres's own pg_type catalog exactly the way ca653a1 already diagnosed and fixed for six other files. Confirmed structurally via git history (this file postdates the fix) and by grep (every other multi-test live suite has TEST_MUTEX; this one didn't). Fixed by applying the identical, already-proven pattern. crates/sms-worker/tests/live_postgres.rs had the same gap from the other direction: it predates ca653a1 by months (#92) and was never updated, protected only by being #[ignore]d and never run in CI until #118. Its four tests use distinct RoleLease role keys so they don't race on lock *state*, but that doesn't protect against the unrelated pg_type first-use race, which is about query-shape preparation, not which lock is held. Same fix applied. Reproduction: could not reproduce the pg_type collision locally despite 9 attempts across escalating conditions (default, 32x oversubscribed test threads, container CPU throttled to 0.5 and 0.25 cores, full-workspace cold sweeps) — consistent with this being a genuine low-probability race that needs CI's slower/fewer- core runners to reliably manifest, as ca653a1 itself found. The fix is the existing, already-proven mitigation applied to the two files that were missing it, not a new mechanism. 2. sms-test-support's stale-schema detection keyed on whether public.messages merely exists in the template database, not on whether it reflects the current migration files. Because the container (and its template) is deliberately left running between local runs, a bootstrap-only schema edit (0002_bootstrap changed, 0001_init untouched) was silently invisible to every suite until something forced a full `just test-live-clean` — already bit one real PR, which only passed after a forced clean rebuild. Fixed with a content fingerprint (FNV-1a over every up.sql's path and bytes) stamped onto the template via `COMMENT ON DATABASE` (pg_shdescription — cluster metadata, never copied into per-binary scratch databases, cleaned up automatically when the template is dropped). Any mismatch triggers a full drop-and-remigrate. Verified empirically: built a baseline 14-row operator_prefix_rules template with the old code, edited 0002_bootstrap to add a 15th row without touching 0001_init, reran with the old code (bug reproduced: count silently stayed at 14, tests still reported ok), then reran the identical scenario with the fix (count correctly became 15, fingerprint changed, no manual clean needed). 3. Folded in a test-only fix to claim_live_postgres.rs's two_concurrent_claimers_never_both_win_the_same_row, per a parallel investigation's finding: it asserted the seeded row appears in at most one claimer's combined output, which wrongly flags the harmless accepted->queued routing hop (deliberately lease-free, per Claimable for Message's own take_lease doc) as a false double-claim. Now asserts the real invariant — at most one claimer may reach `routed`, the only state that triggers a submission. Sanity-checked by temporarily removing if_match from the queued->routed write locally (never committed) and confirming the suite still compiles and runs against the real path cleanly; full CAS soundness was independently verified by the parallel investigation (240 concurrent attempts, exactly one winner per round). Verification: just check/lint/parity/assert-no-raw-sqlx all green. Three consecutive full `cargo test --workspace -- --ignored` sweeps (two against a warm container, one against a freshly cleaned cold container) all green, 37/37 test groups, 0 failures. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
stephane-segning
added a commit
that referenced
this pull request
Aug 7, 2026
CI reproduced a real, ~30%-flaky failure in this suite (3/10 runs), not a fluke: two different tests' wiremock .expect(1) mocks intermittently saw 2 requests instead of 1. Root cause: dispatch::tick's claim loop (claim.rs::candidates()) selects any eligible accepted/queued/routed message system-wide with an expired-or-absent lease -- correctly matching production, since the loop has no way to know which test seeded which row. This binary's database (sms-test-support's per-binary design, #118) is shared by every test in the file and never reset between runs. TEST_MUTEX already serializes execution, but does nothing about residual state: a message a previous test left non-terminal is exactly as claimable as the row the current test is about to seed, and claim_batch's budget (up to tps_ceiling rows per tick) means a single tick() can claim both in the same batch -- so a test's own tick() submits a foreign leftover message to its own wiremock server on top of its own legitimate one. That mismatch is what tripped the affected mocks' .expect(1), and it is order-dependent (hence intermittent), not a data race. Fixed by draining the claimable backlog to a terminal state (cancelled, reachable directly from all three claimable states per §2.10) before every test seeds its own message, through CrateStack delegates only (R1) -- an isolated_db() helper that every test now calls instead of db() directly, so isolation can't be forgotten at a new call site. Under the same TEST_MUTEX every test already holds, so by induction the candidate set at the start of any test contains only rows that test itself goes on to seed. Verified: 15 consecutive clean runs of `cargo test -p sms-worker --test dispatch_live_postgres -- --ignored` (0 failures across 120 individual test executions, well past the 10-run bar). Two full `just test-live` runs both green across all 14 suites/66 tests; one interleaved run hit the pre-existing, unrelated sms-auth::rbac_layer2_live_postgres pg_type-cache flake (#102) in a file this change never touches, confirmed by passing cleanly in isolation immediately after. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
stephane-segning
added a commit
that referenced
this pull request
Aug 7, 2026
…nding (#119) * Add ProviderError::Indeterminate for a submit that timed out after sending (#36's double-send gap) A submit whose response/read timed out after the request was already written to the wire was indistinguishable from "provider down" and got resubmitted on retry -- a real duplicate-SMS risk with no idempotency key anywhere, already flagged as a known, accepted gap in app/sms-worker/tests/kill9_reclaim_live.rs. - ProviderError::Indeterminate (crates/sms-provider): the request reached the provider, or may have; retrying is unsafe. Distinct from Unavailable in exactly that respect. RoutingConsequence gains HoldIndeterminate so the routing() match stays compiler-checked. - OrangeCmProvider::classify_transport_error (sms-provider-orange-cm): reqwest::Error::is_connect() is checked first (never wrote a byte, safe to retry -> Unavailable); is_timeout() || is_body() once past that point is a post-connect failure -> Indeterminate. A 2xx whose body is unparseable or missing resourceURL is also Indeterminate, not Unavailable -- Orange already accepted the submission by then. OrangeCmConfig::connect_timeout/request_timeout are now configurable (defaulted in production()) so tests can force a deterministic timeout without waiting on the 10s/30s production values. - routed -> uncertain (docs/architecture.md §2.10 table + §7.4 diagram, schema/migrations/postgres/0002_bootstrap/up.sql regenerated via ci/gen-bootstrap-sql.py, not hand-edited). Verified with ci/assert-state-machine-parity.py. - dispatch.rs drives the new edge and, on Indeterminate only, stamps Message.providerMessageRefAlt = message.id (== the callbackData Orange was sent, known before the network call regardless of outcome) so a later DLR can still correlate via sms_api::dlr::ingest_one's providerMessageRef-OR-Alt match. Found live: without this the claim genuinely did not hold -- an uncertain message from a timed-out submit had neither ref column set and a matching DLR silently failed to correlate. Live tests (crates/sms-worker/tests/dispatch_live_postgres.rs) prove the message lands in uncertain, that no second submit request ever reaches the mock (wiremock's own received-request count, not just DB state), that a connect-level failure still backs off to queued as before, and that a DLR arriving afterward now genuinely resolves the message. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix cross-test claimable-message leakage in dispatch_live_postgres.rs CI reproduced a real, ~30%-flaky failure in this suite (3/10 runs), not a fluke: two different tests' wiremock .expect(1) mocks intermittently saw 2 requests instead of 1. Root cause: dispatch::tick's claim loop (claim.rs::candidates()) selects any eligible accepted/queued/routed message system-wide with an expired-or-absent lease -- correctly matching production, since the loop has no way to know which test seeded which row. This binary's database (sms-test-support's per-binary design, #118) is shared by every test in the file and never reset between runs. TEST_MUTEX already serializes execution, but does nothing about residual state: a message a previous test left non-terminal is exactly as claimable as the row the current test is about to seed, and claim_batch's budget (up to tps_ceiling rows per tick) means a single tick() can claim both in the same batch -- so a test's own tick() submits a foreign leftover message to its own wiremock server on top of its own legitimate one. That mismatch is what tripped the affected mocks' .expect(1), and it is order-dependent (hence intermittent), not a data race. Fixed by draining the claimable backlog to a terminal state (cancelled, reachable directly from all three claimable states per §2.10) before every test seeds its own message, through CrateStack delegates only (R1) -- an isolated_db() helper that every test now calls instead of db() directly, so isolation can't be forgotten at a new call site. Under the same TEST_MUTEX every test already holds, so by induction the candidate set at the start of any test contains only rows that test itself goes on to seed. Verified: 15 consecutive clean runs of `cargo test -p sms-worker --test dispatch_live_postgres -- --ignored` (0 failures across 120 individual test executions, well past the 10-run bar). Two full `just test-live` runs both green across all 14 suites/66 tests; one interleaved run hit the pre-existing, unrelated sms-auth::rbac_layer2_live_postgres pg_type-cache flake (#102) in a file this change never touches, confirmed by passing cleanly in isolation immediately after. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks
stephane-segning
added a commit
that referenced
this pull request
Aug 7, 2026
…NTS.md (#123) Four PRs landed in one session (#118 harness, #120 gate stabilisation, #119 ProviderError::Indeterminate, #121 fake Orange + chaos suite) and the context file only picked up what the last two agents wrote inline. The biggest omission was the most consequential change: `sms-test-support` and the fact that CI now runs the 14 live-Postgres suites at all — previously `cargo test --workspace` ran without `--ignored`, so they were skipped in CI and had only ever run by hand. Records, in each case why it exists rather than just that it does: - The harness's five load-bearing constraints, each learned by something breaking: no container handle in a `static` (Drop never runs for statics, which leaked 56 containers), label-scoped cleanup only (an image-scoped sweep would destroy a developer's own database), one database per test binary (a shared one let a real dispatch subprocess steal other suites' messages), migration-content fingerprinting (an existence check served a stale schema silently), and the global container name that makes concurrent test runs corrupt each other. - That `ca653a1`'s mutex was never applied to two suites, both of which flaked in CI within hours of #118 running them for the first time. - `Indeterminate`'s connect-vs-read predicate, the `routed -> uncertain` edge it needed, and the fact that it trades a possibly lost message for never sending a duplicate — a product decision, not a free win. - A CLI-skew warning placed at both sites that tell you to run `cratestack migrate diff`, since the installed 0.7.4 CLI emits DDL the pinned 0.6.7 library never does. The warning previously existed only in an unrelated paragraph 150 lines away from the commands themselves. - The missing-`hasRole('system')` gap as an explicitly *unguarded* failure mode, now found seven times. The three entries above it each turned a silent failure loud; this one has no equivalent, which is why it recurs. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
crates/sms-test-support/— a new shared harness that starts (or reuses) one Postgres 16 container and hands back a fully migrated, per-test-binary connection URL.*_live_postgres.rs/*_live.rssuites now callsms_test_support::database_url()instead of readingDATABASE_URLfrom the environment (a human runningdocker run postgres:16+./ci/apply-migrations.shby hand first)..github/workflows/ci.yml— newlivejob runningcargo test --workspace -- --ignored.justfile—just test-live/just test-live-clean.ci/assert-no-raw-sqlx.sh— allowlistssms-test-support/src/lib.rsas a fourth, documented R1 exception.Intent
Source of truth: #102 and
ca653a1's own investigation of it. The existing CI workflow ran a barecargo test --workspacewith no--ignored, so all 14 live-Postgres suites in this workspace were silently skipped in CI and had never once run there. AGENTS.md documents the concrete cost of that blind spot:claim_live_postgres.rs's own policy-gap bug (Message.list/detailmissinghasRole('system'), making the claim loop's query silently return zero rows) sat undetected for a full milestone because nothing had run that suite live since#29merged. This PR closes that gap by making every live suite runnable without manual setup, and by actually wiring them into CI.Scope
testcontainers::ContainerAsyncin a process-levelstatic— Rust never runsDropfor astatic's contents on any process-exit path, so that pattern leaks a container per test binary with no bound (confirmed the hard way: an earlier attempt at this exact harness orphaned 56 containers and filled the machine's RAM). Instead,crates/sms-test-support/src/lib.rsmanages exactly one container via thedockerCLI, keyed by a fixed name (vsms-test-harness-postgres) and a distinctive label (dev.vsms.test-harness=true) that scopes every cleanup operation — self-healing across a crashed prior run, never touching anything not carrying that label.vsms_test_<binary-name>), dropped and recreated from a shared, lazily-migrated template (vsms_template) viaCREATE DATABASE ... TEMPLATEon every call — see "Verification" below for why this was necessary, not just nice-to-have.#[ignore]kept: every live suite stays#[ignore]d, same as before.just test-live(and the new CIlivejob) run-- --ignoredexplicitly. Nothing changes for a plaincargo test/just test— this PR's entire point is making the live path actually reachable, not changing the default path.docs/runbooks/36-handset-gate.md's two physical acceptance tests are untouched.Verification
tenant-provisioner-*stack).just test-live-clean, then a coldjust test-live: 8 → 9 containers (exactly one harness container), all 14 suites green, 63 tests passing, ~67s.just test-live(same compiled binaries, no rebuild): still 9 containers (no growth), all 14 suites green, 63 tests, ~51s.just test-live-cleanagain at the end returns the machine to its pre-existing container count.just check,just lint(fmt + clippy-D warnings),just test,just parity,./ci/assert-no-raw-sqlx.sh.Two real bugs found only by actually running this against a live database, not by reading the code — this repo's own house rule (
AGENTS.md: "verify against live execution... check/build/a prior session's own diagnosis can all stay green while wrong"):The
to_regclass/regclassdecode trap. The original migration-check query (SELECT to_regclass('public.messages')) decoded fine intoOption<String>in sqlx — except thatto_regclassreturnsregclass, which sqlx cannot actually decode as a string, and a NULL result decodes without ever consulting the declared type. So the query worked exactly once, against a fresh database where the table doesn't exist yet, then failed for every later caller once0001_inithad actually run — presenting as flaky suite ordering rather than a decode error. One suite took 9704 seconds before the fix (::textcast); 0.70s after.Cross-binary interference via a shared database (the cross-binary half of claim_live_postgres tests are flaky under the full workspace live sweep (shared-database test isolation) #102). All 14 binaries originally pointed at one
vsmsdatabase inside the container, which persists across runs by design. Most suites tolerate the resulting row accumulation — they scope queries by a freshly generated id — butapp/sms-worker/tests/kill9_reclaim_live.rsspawns a realsms-worker --roles dispatchprocess, and that process's claim loop deliberately selects any eligible candidate row, matching production. Run after the other 13 suites had left claimable rows behind, it picked up their leftovers instead of its own seeded message and timed out inwait_for_state— while passing in isolation in 2.6s.ca653a1(claim_live_postgres tests are flaky under the full workspace live sweep (shared-database test isolation) #102) already serializes each binary's tests against itself via a per-binaryTEST_MUTEX; this is the same underlying problem one layer further out, across binaries rather than within one, whichca653a1explicitly could not address. Fixed by giving every binary its own database (ensure_binary_database/binary_database_name), derived deterministically from the binary's own executable name so reruns of an unchanged binary reuse rather than accumulate;kill9_reclaim_live.rsneeded no changes for this beyond what it already did (reading its URL once fromdatabase_url()and passing it explicitly to the subprocess it spawns via--database-url).Naming this deterministically hit a second real collision, again found only by inspecting
psql -llive rather than by code review: an initial version stripped Cargo's-<hex-metadata-hash>suffix from the binary's file name (reasoning: survive a rebuild). That silently reunitedcrates/sms-auth/tests/live_postgres.rsandcrates/sms-worker/tests/live_postgres.rs— two different packages' test files that happen to share a file name — onto one shared database, since Cargo's owntarget/debug/deps/is flat and workspace-wide and the hash suffix is the only thing in the executable's path that tells them apart (confirmed: they really do get different hashes, e.g.live_postgres-8d453e5151bd11d1vslive_postgres-e79e21cf68671d0b). Fixed by keeping the hash; the tradeoff is that a rebuild which changes it mints a differently-named database rather than reusing the old one (cleaned up like any other harness state byjust test-live-clean), which only matters across rebuilds, not across the reruns-of-the-same-binaries scenario this task's own verification and CI's singlecargo test --workspace -- --ignoredinvocation both are.ca653a1's mutexes: kept unchanged, and still load-bearing. They serialize tests within one binary; giving each binary its own database doesn't remove that need, since tests within one binary still share that one binary's database and can still race each other's candidate-row queries or a coldpg_typecache the same wayca653a1originally found. Not removed speculatively — proven still necessary by the fact that removing per-binary isolation (the bug this PR fixes) was itself only found by seeing a real failure, not by reasoning about it.Unverified: this PR does not touch (and cannot verify) the two genuinely physical parts of
docs/runbooks/36-handset-gate.md— a real Orange handset and a human-timedkill -9against a real Orange account. Also unverified: behavior underVSMS_TEST_DATABASE_URL(the escape hatch for a developer-supplied Postgres instead of the managed container) — the per-binary-database logic applies to it identically in code, but wasn't separately exercised against a non-container Postgres in this round of verification.Screenshots/Evidence
N/A — a backend test-infrastructure change, no UI surface. Full per-suite pass counts (identical across both the cold and warm runs):
dlr_ingestion_live_postgres5,errors_live_postgres3,policy_golden_list_live_postgres6,send_message_live_postgres9,sms-auth live_postgres3,oidc_flow_live1,provision_app_client_live_postgres2,rbac_layer2_live_postgres7,m1_acceptance_gate_live_postgres1,claim_live_postgres7,dispatch_live_postgres5,jobs_live_postgres9,sms-worker live_postgres4,kill9_reclaim_live1 — 63 tests, 14/14 suites green.Risk Assessment
Low for production code (zero production code paths changed — this is test infrastructure only). Moderate for CI reliability, in the sense that this is the first time these 14 suites have ever run in CI at all, so this PR is also the first real test of whether they hold up outside a developer's own machine; the two full sweeps above (cold + warm) are the best available proxy for that, but a CI runner's Docker environment can still differ from this dev machine's in ways not caught here.
AI Usage Declaration
This PR was implemented by Claude Code (Claude Sonnet 5) under my direction, continuing work an earlier agent had substantially completed but left uncommitted, then diagnosing and fixing the one remaining known bug (cross-binary database sharing) plus a second collision bug found live while fixing the first, and verifying the whole thing end to end before committing.
statichandle, why drop-and-recreate rather than create-if-absent, why the template-copy approach, why the advisory lock cannot be shared between the two lock-acquisition sites).cargo check/cargo buildalone, per this repo's own house rule that a compiling test proves nothing about runtime behavior.Reviewer Focus
AGENTS.md) or a single shared database with stronger per-test scoping (rejected becausekill9_reclaim_live.rs's claim loop is deliberately global and can't be scoped that way without diverging from production behavior).ensure_binary_database/ensure_template_ready(one lock held for the whole ensure-template-then-drop-then-recreate sequence, with the template-readiness check deliberately not taking its own lock to avoid a same-process cross-connection deadlock) is correct under genuinely concurrent invocations — the module doc frames the concurrent-sweep race as a known, accepted limitation, and it'd be worth a second pair of eyes on whether that's still true here.liveCI job itself hasn't run in real GitHub Actions yet as part of this PR (no way to trigger that from here before merge) — worth watching its first real run for anything this local Docker environment doesn't reproduce.