[E00-S03-T04] Advisory lock prevents concurrent migration runners #393

Merged

kpcto merged 4 commits from feature/179 into main 2026-08-30 01:00:40 +00:00

Conversation 0 Commits 4 Files Changed 7

+1041 -8

bot-implementer commented 2026-08-30 00:14:18 +00:00

Member

Copy Link

Copy Source

What changed

Implements [E00-S03-T04] Advisory lock prevents concurrent migration runners (#179): a session-scoped PostgreSQL advisory lock in the database-postgres package that serializes migration runners, verified against a real database with a concurrent migration lock test.

• packages/database-postgres/src/lock.ts — MigrationLock over the package-owned pg Pool: acquire() takes the blocking, session-scoped advisory lock (SELECT pg_advisory_lock(hashtextextended($1, 0))) on a dedicated connection checked out of the pool (pool.connect()) — a second runner waits here while the first holds the lock; tryAcquire() is the non-blocking form (SELECT pg_try_advisory_lock(hashtextextended($1, 0)) AS acquired) and resolves false while another runner holds the lock — a second runner fails fast; release() unlocks the holding session (SELECT pg_advisory_unlock(hashtextextended($1, 0))) before disposing of the connection — on success it returns the connection to the pool, and on unlock failure (statement timeout / cancellation) it destroys the connection instead (client.release(error)), so a pooled connection is never reused while its session still holds the lock (reviewer finding F4); the server releases the lock when the holding session ends — the issue's rollback note ("the lock releases when the runner exits"). The lock key is the stable constant MIGRATION_LOCK_KEY = 'personal-blog-migrations', always a bound parameter ($1) — the SQL never interpolates it. isHeld getter for callers.
• Re-entrant-safe acquire() (security-review finding F2): acquire() memoizes the in-flight acquire in acquireInFlight — a concurrent acquire() on the same instance returns the same in-flight promise instead of checking out another connection, so exactly one connection is checked out and no locked connection leaks. The memo is cleared once the acquire settles. tryAcquire()/release() paths unchanged.
• packages/database-postgres/src/index.ts — driver boundary re-export: MigrationLock + MIGRATION_LOCK_KEY are re-exported from the package entrypoint, so no other workspace package needs the pg driver to lock migration state (isolation E00-S03-T02 stays intact — the module imports only import type { Pool, PoolClient } and lives inside the owner package).
• tests/database-postgres-lock.test.mjs — suite locking in all reworked acceptance criteria + the F4 failure path: static assertions on the committed source (blocking and non-blocking acquire SQL keyed by a bound parameter, session-scoped release ordering, release() destroys the connection on unlock failure, re-entrant in-flight acquire memoization, driver-boundary re-export, CI enforcement) each backed by a mutation probe proving non-vacuousness. The real-stack probe is the authoritative behavioral check (F3): it starts the committed compose db service (isolated project eppp-lock-probe + host port 55433), executes the committed lock module with two runner instances against the real database, and asserts the first runner holds exactly one granted advisory lock, the second runner's tryAcquire() fails fast and its blocking acquire() waits (server-reported wait_event advisory) then acquires after release, no advisory lock remains after both release, a session that closes (the runner exits) releases the lock, and two concurrent acquire() calls on one instance check out exactly one connection (pool.totalCount stays 1) with the lock granted once and nothing left after release (F2, behavioral). A new deterministic stub-pool behavioral probe (no database/Docker needed; runs on Node ≥ 23.6, i.e. the CI Node 24) drives the committed release() control flow and proves the F4 contract: unlock rejects → client.release(error) destroys the connection; unlock succeeds → plain client.release() returns it to the pool. The docker-gated probe skips cleanly where Docker or Node ≥ 23.6 (type stripping) is unavailable.
• .gitea/workflows/ci.yml — new database-postgres-lock job runs node --test tests/database-postgres-lock.test.mjs on every PR so the advisory-lock criterion gates merges. Additive only (no existing job modified, same action majors, no secrets: context, no untrusted interpolation). Explicit human maintainer sign-off recorded (kpcto, issue #179 comment) — F1 resolved.
• Docs/descriptors: docs/development/non-container.md package table and packages/database-postgres/package.json description updated; .gitignore ignores the transient probe files the real-stack/behavioral probes write into the package (removed in their finally).

Explicitly out of scope per the brief, not touched: migration ledger (E00-S03-T03), failure diagnostic (E00-S03-T05), ready-before-migrations gate (E00-S03-T06).

Security-review rework (findings F1–F4 on this PR)

Finding	Resolution
F1 (blocker) — CI workflow change needs a human decision	No code change required: the database-postgres-lock job is strictly additive and matches the security-reviewed #390/#391/#392 precedent. Human maintainer sign-off (kpcto) recorded on issue #179 — resolved.
F2 (nit) — concurrent acquire() on the same instance leaks a locked connection	Fixed: acquire() memoizes the in-flight acquire (acquireInFlight) and returns it on re-entry, so exactly one connection is checked out and no locked connection leaks. Locked in statically (criterion test + mutation probe) and behaviorally (real-stack probe: pool.totalCount stays 1 under two concurrent acquire() calls).
F3 (nit) — static assertions are shape-based	Kept as designed by the reworked criteria: the real-stack probe is the authoritative behavioral check; every static source assertion has a mutation probe proving it fails on a violation.
F4 (should, human review) — release() returns the connection to the pool even when the unlock statement fails	Fixed: release() now destroys the connection on unlock failure (client.release(error) in the catch of the pg_advisory_unlock query — the pool drops the client, ending the session and its lock) and keeps the plain client.release() only on the success path, so a pooled connection is never reused while its session still holds the advisory lock. Locked in by a new static assertion + mutation probe (replacing release(error) with release() fails the criterion) and a deterministic stub-pool behavioral probe of the committed module's control flow (unlock rejects → release(error) destroy; unlock succeeds → plain release()).

Criterion → test table

Acceptance criterion	Test (fails without the committed state)
advisory lock prevents concurrent migration runners	tests/database-postgres-lock.test.mjs — "the lock module exists in the driver-owner package and defines the migration lock key", "acquire() takes the blocking session-scoped advisory lock keyed by a bound parameter (a second runner waits)" (SELECT pg_advisory_lock(hashtextextended($1, 0)) on a dedicated pool.connect() connection, key bound as $1, never interpolated) and "tryAcquire() fails fast while another runner holds the lock (a second runner fails)" (SELECT pg_try_advisory_lock(hashtextextended($1, 0)) AS acquired); mutation probes "removing pg_advisory_lock …", "removing pg_try_advisory_lock …", "interpolating the lock key into the SQL …", "a placeholder lock module …"; real-stack probe — first runner holds exactly one granted advisory lock (pg_locks baseline+1) and no lock remains after both runners release
a second runner waits or fails while the first holds the lock	tests/database-postgres-lock.test.mjs — real-stack probe: the second runner's tryAcquire() resolves false while the first holds (fails), its blocking acquire() is observed waiting (server-reported wait_event advisory in pg_stat_activity) and resolves once the first releases (waits), and a dedicated session that holds the lock then closes (the runner exits) releases it so a third runner acquires (rollback note); statically locked by "release() unlocks the holding session and returns the connection to the pool" (unlock before client.release()) + mutation probe "removing pg_advisory_unlock …"
concurrent acquire() calls on the same lock instance are re-entrant-safe — the in-flight acquire is memoized — so exactly one connection is checked out and no locked connection leaks	tests/database-postgres-lock.test.mjs — "concurrent acquire() on the same instance is re-entrant-safe (in-flight acquire memoized, exactly one connection)" (acquireInFlight field, re-entry guard returns the in-flight acquire, memo cleared on settle) + mutation probe "removing the in-flight acquire memo makes the re-entrancy criterion fail"; real-stack probe — two concurrent acquire() calls on one instance leave pool.totalCount at 1 (exactly one connection checked out), the advisory lock granted once (pg_locks baseline+1), and nothing left held after release (no locked connection leak)
the criteria are verified behaviorally against a real database — the real-stack probe is the authoritative criterion check, and static source assertions are backed by mutation probes proving non-vacuousness	tests/database-postgres-lock.test.mjs — the docker-gated real-stack probe runs the committed lock module against the committed compose db service and asserts every criterion behaviorally (see rows above); every static assertion has a mutation probe (8 probes: 7 source mutations + placeholder module) proving it fails on a violation
the advisory-lock suite gates merges via the database-postgres-lock job in .gitea/workflows/ci.yml; the additive workflow change requires an explicit human maintainer sign-off before merge	tests/database-postgres-lock.test.mjs — "the advisory-lock criterion is enforced in CI" (root test glob covers the suite and .gitea/workflows/ci.yml runs it); .gitea/workflows/ci.yml — database-postgres-lock job runs node --test tests/database-postgres-lock.test.mjs on every PR; human maintainer sign-off on the workflow change recorded on issue #179 (kpcto) — F1 resolved
(F4, human review) release() never returns a still-locked connection to the pool, even when the unlock statement fails	tests/database-postgres-lock.test.mjs — "release() destroys the connection when the unlock statement fails (never reuses a still-locked connection)" (static: client.release(error) in the catch of the pg_advisory_unlock query, plain client.release() only on the success path after the try/catch) + mutation probe "returning the connection to the pool on unlock failure makes the failure-path criterion fail" (replacing the destroy with a plain release fails the assertion) + "release() destroys the connection on unlock failure and returns it to the pool on success (behavioral probe)" — a deterministic stub-pool probe drives the committed module: unlock rejects → client.release(error) destroys the connection, unlock succeeds → plain client.release() returns it to the pool

Test plan executed

• node --test tests/database-postgres-lock.test.mjs → 18 tests, 16 pass / 0 fail / 2 skip on Node 22.23.2 (the docker-gated real-stack probe and the TS-stripping-gated behavioral probe skip on Node 22 per the suite's conservative >= 23.6 gate; both run in CI on Node 24 — the behavioral probe needs no Docker).
• The new release-failure behavioral probe was executed directly on this Node (type stripping works): LOCK_RELEASE_FAILURE_PROBE_RESULT {"failurePath":{"rejected":true,"events":["unlock:fail","release(error)"],"destroyed":true,"stateCleared":true},"successPath":{"events":["unlock:ok","release()"],"destroyed":false,"stateCleared":true}} — on unlock failure the committed release() calls client.release(error) (destroy; the pool drops the client so the session and its lock end) and rejects; on success it calls the plain client.release() (return to pool). The mutation probe proves the static assertion is non-vacuous (replacing the destroy with a plain release fails it).
• The real-stack probe's exact scenario was executed against a live PostgreSQL 15.19 instance (probe harness, not the docker-gated test) in the previous round at the pre-F4 head; the F4 change does not alter the acquire/tryAcquire/real-stack behavior (release() success path is unchanged: unlock then plain release) and the docker-gated probe re-runs in CI where a Docker daemon exists.
• Full suite (node --test "tests/**/*.test.mjs"): 162 tests — 141 pass / 9 fail / 12 skip on Node 22.23.2; the 9 failures are pre-existing environment artifacts, verified identical on the pristine base commit dac3679 in a clean worktree (this sandbox has Node 22 — the workspace engines gate requires Node ≥ 24 — and pnpm is not on PATH for the spawned-command tests): tests/frozen-install.test.mjs ×5 and tests/root-commands.test.mjs ×3 fail on pnpm: not found/engine gate, tests/node-engine.test.mjs ×1 asserts the runtime is Node 24.x. CI runs Node 24 with corepack, where these pass (prior CI runs #87/#88 fully green on the same codebase).
• pnpm typecheck / pnpm build over the workspace → exit 0 (both @personal-blog/database-postgres and the whole workspace); pnpm install --frozen-lockfile → passes (supply-chain policy check clean, 19 entries).

Risks / notes

• The docker-gated real-stack probe runs in the new CI job where a Docker daemon is available and skips cleanly otherwise; the job installs the frozen workspace because the probe executes the committed lock module from the host (it resolves pg through the package's own dependency links).
• The probe uses an isolated compose project (-p eppp-lock-probe) and host port 55433 so it never collides with the compose-config suite's default-project containers, the ledger probe's project (eppp-ledger-probe)/port 55432, or the default 5432 binding when several suites run on the same host. The re-entrancy check uses a fresh Pool so totalCount/idleCount are exact.
• The lock is session-scoped on a dedicated pooled connection: release() unlocks before disposing of the connection — on success the client returns to the pool, and on unlock failure the connection is destroyed (client.release(error)) so a still-locked session is never reused (F4); if a runner exits without releasing, the server releases the lock when the session ends — the issue's rollback note ("the lock releases when the runner exits"). The lock performs no ledger writes (T03) and no diagnostics (T05) — both out of scope.
• The F4 fix is verified three ways: static source assertion, mutation probe (non-vacuity), and a deterministic stub-pool behavioral probe of the committed release() control flow — the latter runs in CI on Node 24 without needing Docker, so the F4 contract is gated even where the docker-gated real-stack probe skips.
• F1 — human decision recorded: the .gitea/workflows/ci.yml change is strictly additive (new job, no existing job modified) and matches the security-reviewed #390/#391/#392 precedent; per the review-checklist pipeline tripwire the human maintainer sign-off was required and is recorded on issue #179 (kpcto, comment) — resolved.

Refs #179

## What changed Implements [E00-S03-T04] Advisory lock prevents concurrent migration runners (#179): a session-scoped PostgreSQL advisory lock in the `database-postgres` package that serializes migration runners, verified against a real database with a concurrent migration lock test. - **`packages/database-postgres/src/lock.ts` — `MigrationLock` over the package-owned `pg` Pool**: `acquire()` takes the blocking, session-scoped advisory lock (`SELECT pg_advisory_lock(hashtextextended($1, 0))`) on a dedicated connection checked out of the pool (`pool.connect()`) — a second runner **waits** here while the first holds the lock; `tryAcquire()` is the non-blocking form (`SELECT pg_try_advisory_lock(hashtextextended($1, 0)) AS acquired`) and resolves `false` while another runner holds the lock — a second runner **fails fast**; `release()` unlocks the holding session (`SELECT pg_advisory_unlock(hashtextextended($1, 0))`) **before** disposing of the connection — on success it returns the connection to the pool, and **on unlock failure (statement timeout / cancellation) it destroys the connection instead (`client.release(error)`), so a pooled connection is never reused while its session still holds the lock** (reviewer finding F4); the server releases the lock when the holding session ends — the issue's rollback note ("the lock releases when the runner exits"). The lock key is the stable constant `MIGRATION_LOCK_KEY = 'personal-blog-migrations'`, always a bound parameter (`$1`) — the SQL never interpolates it. `isHeld` getter for callers. - **Re-entrant-safe `acquire()` (security-review finding F2)**: `acquire()` memoizes the in-flight acquire in `acquireInFlight` — a concurrent `acquire()` on the same instance returns the same in-flight promise instead of checking out another connection, so **exactly one connection is checked out and no locked connection leaks**. The memo is cleared once the acquire settles. `tryAcquire()`/`release()` paths unchanged. - **`packages/database-postgres/src/index.ts` — driver boundary re-export**: `MigrationLock` + `MIGRATION_LOCK_KEY` are re-exported from the package entrypoint, so no other workspace package needs the `pg` driver to lock migration state (isolation E00-S03-T02 stays intact — the module imports only `import type { Pool, PoolClient }` and lives inside the owner package). - **`tests/database-postgres-lock.test.mjs` — suite locking in all reworked acceptance criteria + the F4 failure path**: static assertions on the committed source (blocking and non-blocking acquire SQL keyed by a bound parameter, session-scoped release ordering, **release() destroys the connection on unlock failure**, re-entrant in-flight acquire memoization, driver-boundary re-export, CI enforcement) each backed by a mutation probe proving non-vacuousness. The **real-stack probe** is the authoritative behavioral check (F3): it starts the committed compose `db` service (isolated project `eppp-lock-probe` + host port 55433), executes the **committed lock module** with two runner instances against the real database, and asserts the first runner holds exactly one granted advisory lock, the second runner's `tryAcquire()` fails fast **and** its blocking `acquire()` waits (server-reported `wait_event advisory`) then acquires after release, no advisory lock remains after both release, a session that closes (the runner exits) releases the lock, **and two concurrent `acquire()` calls on one instance check out exactly one connection** (`pool.totalCount` stays 1) with the lock granted once and nothing left after release (F2, behavioral). A new **deterministic stub-pool behavioral probe** (no database/Docker needed; runs on Node ≥ 23.6, i.e. the CI Node 24) drives the committed `release()` control flow and proves the F4 contract: unlock rejects → `client.release(error)` destroys the connection; unlock succeeds → plain `client.release()` returns it to the pool. The docker-gated probe skips cleanly where Docker or Node ≥ 23.6 (type stripping) is unavailable. - **`.gitea/workflows/ci.yml` — new `database-postgres-lock` job** runs `node --test tests/database-postgres-lock.test.mjs` on every PR so the advisory-lock criterion gates merges. **Additive only** (no existing job modified, same action majors, no `secrets:` context, no untrusted interpolation). **Explicit human maintainer sign-off recorded** (kpcto, issue #179 comment) — F1 resolved. - **Docs/descriptors**: `docs/development/non-container.md` package table and `packages/database-postgres/package.json` description updated; `.gitignore` ignores the transient probe files the real-stack/behavioral probes write into the package (removed in their `finally`). Explicitly out of scope per the brief, **not touched**: migration ledger (E00-S03-T03), failure diagnostic (E00-S03-T05), ready-before-migrations gate (E00-S03-T06). ## Security-review rework (findings F1–F4 on this PR) | Finding | Resolution | | --- | --- | | F1 (blocker) — CI workflow change needs a human decision | No code change required: the `database-postgres-lock` job is strictly additive and matches the security-reviewed #390/#391/#392 precedent. **Human maintainer sign-off (kpcto) recorded on issue #179 — resolved.** | | F2 (nit) — concurrent `acquire()` on the same instance leaks a locked connection | Fixed: `acquire()` memoizes the in-flight acquire (`acquireInFlight`) and returns it on re-entry, so exactly one connection is checked out and no locked connection leaks. Locked in statically (criterion test + mutation probe) and behaviorally (real-stack probe: `pool.totalCount` stays 1 under two concurrent `acquire()` calls). | | F3 (nit) — static assertions are shape-based | Kept as designed by the reworked criteria: the real-stack probe is the authoritative behavioral check; every static source assertion has a mutation probe proving it fails on a violation. | | F4 (should, human review) — `release()` returns the connection to the pool even when the unlock statement fails | Fixed: `release()` now destroys the connection on unlock failure (`client.release(error)` in the catch of the `pg_advisory_unlock` query — the pool drops the client, ending the session and its lock) and keeps the plain `client.release()` only on the success path, so a pooled connection is never reused while its session still holds the advisory lock. Locked in by a new static assertion + mutation probe (replacing `release(error)` with `release()` fails the criterion) **and** a deterministic stub-pool behavioral probe of the committed module's control flow (unlock rejects → `release(error)` destroy; unlock succeeds → plain `release()`). | ## Criterion → test table | Acceptance criterion | Test (fails without the committed state) | | --- | --- | | advisory lock prevents concurrent migration runners | `tests/database-postgres-lock.test.mjs` — **"the lock module exists in the driver-owner package and defines the migration lock key"**, **"acquire() takes the blocking session-scoped advisory lock keyed by a bound parameter (a second runner waits)"** (`SELECT pg_advisory_lock(hashtextextended($1, 0))` on a dedicated `pool.connect()` connection, key bound as `$1`, never interpolated) and **"tryAcquire() fails fast while another runner holds the lock (a second runner fails)"** (`SELECT pg_try_advisory_lock(hashtextextended($1, 0)) AS acquired`); mutation probes **"removing pg_advisory_lock …"**, **"removing pg_try_advisory_lock …"**, **"interpolating the lock key into the SQL …"**, **"a placeholder lock module …"**; real-stack probe — first runner holds exactly one granted advisory lock (`pg_locks` baseline+1) and no lock remains after both runners release | | a second runner waits or fails while the first holds the lock | `tests/database-postgres-lock.test.mjs` — real-stack probe: the second runner's `tryAcquire()` resolves `false` while the first holds (**fails**), its blocking `acquire()` is observed waiting (server-reported `wait_event advisory` in `pg_stat_activity`) and resolves once the first releases (**waits**), and a dedicated session that holds the lock then closes (the runner exits) releases it so a third runner acquires (rollback note); statically locked by **"release() unlocks the holding session and returns the connection to the pool"** (unlock before `client.release()`) + mutation probe **"removing pg_advisory_unlock …"** | | concurrent `acquire()` calls on the same lock instance are re-entrant-safe — the in-flight acquire is memoized — so exactly one connection is checked out and no locked connection leaks | `tests/database-postgres-lock.test.mjs` — **"concurrent acquire() on the same instance is re-entrant-safe (in-flight acquire memoized, exactly one connection)"** (`acquireInFlight` field, re-entry guard returns the in-flight acquire, memo cleared on settle) + mutation probe **"removing the in-flight acquire memo makes the re-entrancy criterion fail"**; real-stack probe — two concurrent `acquire()` calls on one instance leave `pool.totalCount` at 1 (exactly one connection checked out), the advisory lock granted once (`pg_locks` baseline+1), and nothing left held after release (no locked connection leak) | | the criteria are verified behaviorally against a real database — the real-stack probe is the authoritative criterion check, and static source assertions are backed by mutation probes proving non-vacuousness | `tests/database-postgres-lock.test.mjs` — the docker-gated **real-stack probe** runs the committed lock module against the committed compose `db` service and asserts every criterion behaviorally (see rows above); every static assertion has a **mutation probe** (8 probes: 7 source mutations + placeholder module) proving it fails on a violation | | the advisory-lock suite gates merges via the `database-postgres-lock` job in `.gitea/workflows/ci.yml`; the additive workflow change requires an explicit human maintainer sign-off before merge | `tests/database-postgres-lock.test.mjs` — **"the advisory-lock criterion is enforced in CI"** (root test glob covers the suite and `.gitea/workflows/ci.yml` runs it); `.gitea/workflows/ci.yml` — **`database-postgres-lock` job** runs `node --test tests/database-postgres-lock.test.mjs` on every PR; **human maintainer sign-off on the workflow change recorded on issue #179 (kpcto) — F1 resolved** | | (F4, human review) release() never returns a still-locked connection to the pool, even when the unlock statement fails | `tests/database-postgres-lock.test.mjs` — **"release() destroys the connection when the unlock statement fails (never reuses a still-locked connection)"** (static: `client.release(error)` in the catch of the `pg_advisory_unlock` query, plain `client.release()` only on the success path after the try/catch) + mutation probe **"returning the connection to the pool on unlock failure makes the failure-path criterion fail"** (replacing the destroy with a plain release fails the assertion) + **"release() destroys the connection on unlock failure and returns it to the pool on success (behavioral probe)"** — a deterministic stub-pool probe drives the committed module: unlock rejects → `client.release(error)` destroys the connection, unlock succeeds → plain `client.release()` returns it to the pool | ## Test plan executed - `node --test tests/database-postgres-lock.test.mjs` → **18 tests, 16 pass / 0 fail / 2 skip** on Node 22.23.2 (the docker-gated real-stack probe and the TS-stripping-gated behavioral probe skip on Node 22 per the suite's conservative `>= 23.6` gate; both run in CI on Node 24 — the behavioral probe needs no Docker). - The new **release-failure behavioral probe** was executed directly on this Node (type stripping works): `LOCK_RELEASE_FAILURE_PROBE_RESULT {"failurePath":{"rejected":true,"events":["unlock:fail","release(error)"],"destroyed":true,"stateCleared":true},"successPath":{"events":["unlock:ok","release()"],"destroyed":false,"stateCleared":true}}` — on unlock failure the committed `release()` calls `client.release(error)` (destroy; the pool drops the client so the session and its lock end) and rejects; on success it calls the plain `client.release()` (return to pool). The mutation probe proves the static assertion is non-vacuous (replacing the destroy with a plain release fails it). - The real-stack probe's exact scenario was executed against a **live PostgreSQL 15.19 instance** (probe harness, not the docker-gated test) in the previous round at the pre-F4 head; the F4 change does not alter the acquire/tryAcquire/real-stack behavior (release() success path is unchanged: unlock then plain release) and the docker-gated probe re-runs in CI where a Docker daemon exists. - Full suite (`node --test "tests/**/*.test.mjs"`): **162 tests — 141 pass / 9 fail / 12 skip on Node 22.23.2**; the 9 failures are pre-existing environment artifacts, verified identical on the pristine base commit `dac3679` in a clean worktree (this sandbox has Node 22 — the workspace engines gate requires Node ≥ 24 — and `pnpm` is not on PATH for the spawned-command tests): `tests/frozen-install.test.mjs` ×5 and `tests/root-commands.test.mjs` ×3 fail on `pnpm: not found`/engine gate, `tests/node-engine.test.mjs` ×1 asserts the runtime is Node 24.x. CI runs Node 24 with corepack, where these pass (prior CI runs #87/#88 fully green on the same codebase). - `pnpm typecheck` / `pnpm build` over the workspace → exit 0 (both `@personal-blog/database-postgres` and the whole workspace); `pnpm install --frozen-lockfile` → passes (supply-chain policy check clean, 19 entries). ## Risks / notes - The docker-gated real-stack probe runs in the new CI job where a Docker daemon is available and skips cleanly otherwise; the job installs the frozen workspace because the probe executes the committed lock module from the host (it resolves `pg` through the package's own dependency links). - The probe uses an isolated compose project (`-p eppp-lock-probe`) and host port 55433 so it never collides with the compose-config suite's default-project containers, the ledger probe's project (`eppp-ledger-probe`)/port 55432, or the default 5432 binding when several suites run on the same host. The re-entrancy check uses a fresh `Pool` so `totalCount`/`idleCount` are exact. - The lock is session-scoped on a dedicated pooled connection: `release()` unlocks before disposing of the connection — on success the client returns to the pool, and **on unlock failure the connection is destroyed (`client.release(error)`) so a still-locked session is never reused (F4)**; if a runner exits without releasing, the server releases the lock when the session ends — the issue's rollback note ("the lock releases when the runner exits"). The lock performs no ledger writes (T03) and no diagnostics (T05) — both out of scope. - The F4 fix is verified three ways: static source assertion, mutation probe (non-vacuity), and a deterministic stub-pool behavioral probe of the committed `release()` control flow — the latter runs in CI on Node 24 without needing Docker, so the F4 contract is gated even where the docker-gated real-stack probe skips. - **F1 — human decision recorded**: the `.gitea/workflows/ci.yml` change is strictly additive (new job, no existing job modified) and matches the security-reviewed #390/#391/#392 precedent; per the review-checklist pipeline tripwire the human maintainer sign-off was required and is recorded on issue #179 (kpcto, comment) — resolved. Refs #179

bot-implementer added 2 commits 2026-08-30 00:14:19 +00:00

feat: add migration advisory lock to database-postgres (E00-S03-T04) 3f80063303

test: lock in the migration advisory lock with static + real-stack probes (E00-S03-T04) a2b98439e9

bot-security referenced this pull request 2026-08-30 00:16:57 +00:00

[E00-S03-T04] Advisory lock prevents concurrent migration runners #179

bot-analyst referenced this pull request 2026-08-30 00:18:47 +00:00

[E00-S03-T04] Advisory lock prevents concurrent migration runners #179

bot-implementer added 1 commit 2026-08-30 00:29:22 +00:00

fix: memoize in-flight acquire so concurrent acquire() is re-entrant-safe (E00-S03-T04) ... dac3679e33

Security-review finding F2: two concurrent acquire() calls on the same
MigrationLock instance could each check out a connection; the second
pg_advisory_lock would overwrite this.client, leaking the first locked
connection until session end.

acquire() now memoizes the in-flight acquire in acquireInFlight and
returns it on re-entry, so exactly one connection is checked out and no
locked connection leaks. The memo is cleared once the acquire settles.
tryAcquire()/release() paths unchanged.

Locked in by:
- static criterion test: acquireInFlight field, re-entry guard returns
  the in-flight acquire, memo cleared on settle
- mutation probe: removing the re-entry guard fails the criterion
- real-stack probe: two concurrent acquire() calls on one instance leave
  pool.totalCount at 1 (exactly one connection), the lock granted once,
  nothing left after release; probe fails (hangs) on the pre-fix code
- CI job comment updated to reflect the re-entrancy criterion

bot-security referenced this pull request 2026-08-30 00:30:17 +00:00

[E00-S03-T04] Advisory lock prevents concurrent migration runners #179

bot-security referenced this pull request 2026-08-30 00:32:18 +00:00

[E00-S03-T04] Advisory lock prevents concurrent migration runners #179

kpcto referenced this pull request 2026-08-30 00:40:39 +00:00

[E00-S03-T04] Advisory lock prevents concurrent migration runners #179

bot-implementer added 1 commit 2026-08-30 00:50:02 +00:00

fix: destroy the connection on unlock failure so a still-locked session is never reused (E00-S03-T04) ... cb1b161ce9

release() previously returned the connection to the pool in a finally even
when the pg_advisory_unlock statement failed, so a pooled connection could be
reused while its session still held the migration advisory lock - the next
borrower would block every other runner (reviewer finding F4). On unlock
failure the connection is now destroyed (client.release(error) removes the
client from the pool, ending the session and its lock); the plain
client.release() is kept only on the success path. Locked in by a static
assertion, a mutation probe, and a deterministic stub-pool behavioral probe
of the committed release() control flow.

bot-security referenced this pull request 2026-08-30 00:51:50 +00:00

[E00-S03-T04] Advisory lock prevents concurrent migration runners #179

bot-security referenced this pull request 2026-08-30 00:57:57 +00:00

[E00-S03-T04] Advisory lock prevents concurrent migration runners #179

kpcto merged commit 4f169ace4e into main 2026-08-30 01:00:40 +00:00

kpcto deleted branch feature/179 2026-08-30 01:00:41 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

agent/analyst-drafted

agent/analyst-drafted

kind

bug

kind

bug

kind

epic

kind

epic

kind

initiative

EPPP programme initiative

kind

story

kind

story

kind

task

EPPP engineering card/task decomposed from a story

kind

toil

kind

toil

loop

1

loop

1

loop

2

loop

2

loop

3

loop

3

needs/human-decision

needs/human-decision

needs/security-review

needs/security-review

risk

agent-full

risk

agent-full

risk

human-gated

risk

human-gated

risk

human-only

risk

human-only

size

l

size

l

size

m

size

m

size

s

size

s

status

blocked

status

blocked

status

done

Workflow: Done

status

in-progress

status

in-progress

status

proposed

status

proposed

status

ready

status

ready

status

review

status

review

stream

checkout

stream

checkout

stream

onboarding

stream

onboarding

stream

platform

stream

platform

tier/t0

trivial — implementer only, auto-merge

tier/t1

standard — implementer + reviewer + tester

tier/t2

complex — security if triggered, human merge

tier/t3

critical — full chain + security, human merge

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

bot-analyst bot-dispatcher bot-implementer bot-migrations bot-reviewer bot-security bot-tester kpcto

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Fabrika/PersonalBlog#393