[freeqaz]

Problem

PostgreSQL Application started failing intermittently after #3536, with a revision test dying on terminating connection due to administrator command (SQLSTATE 57P01):

• fix(console): show a failed deployment's error once on the agent page #3539 (console-only): run 40141237356, job 121013288227, postgres-worker-agent-revision-stop.test.mjs, "Agent deletion finalization rejects an expired lease without removing state".
• main 5d3f324 (docs(openshell): document startup rules for agent resources, protocols and unknown keys #3541, docs-only): run 40142666070, job 121017513104, postgres-worker-agent-revision-yielded-redeploy.test.mjs, "a yielded redeploy overtaken by an Agent stop finishes its predecessor stop before it ends stopped".

The revision fixture's teardown ends its pools and then drops the test's database WITH (FORCE). pg-pool's end() resolves once its idle clients are removed, before their sessions close: client.end() only sends Terminate. Since #3536 the drop goes over a direct connection, which takes milliseconds rather than the 0.2-0.4 s a docker compose exec psql took. So the drop can now reach the server before a closing backend has read its Terminate. That backend then sends FATAL 57P01 to its client. pg-pool's idle listener, which _remove() leaves attached, re-emits the error on the pool. The fixture pools have no error listener, so the emit throws and the test fails in its teardown. The failing test's duration matches its passing neighbours on #3536 heads, because test durations include the per-test copy and drop.

Change

• tests/helpers/postgres-worker-revision-fixture.mjs: teardown adds a no-op error listener to each fixture pool before ending it. That covers pools a worker's stop or the test already ended. Errors from a pool are still raised while its test runs; only an ended pool's session closing during the drop is ignored.
• postgres-worker-agent-revision-health.test.mjs: a new case releases an idle client, disposes the fixture, and then delivers the 57P01 that client would receive from the drop. It asserts nothing throws. The leaked-client case is unchanged: a checked-out client has no idle listener.

Verification

• Repro outside the suite against postgres:18 at 0.5 CPU: 4 processes x 80 rounds of create database, fill an 8-client pool, pool.end(), direct DROP DATABASE ... WITH (FORCE). Without the listener, 5 of 320 rounds raised an uncaught 57P01. With it, 0 of 320.
• On a remote test VM, the whole postgres-application lane (oce-ci-lane) passes with the change.
• With the listener line removed, the new case fails with Got unwanted exception ... terminating connection due to administrator command.
• Lane repeat runs at 2 CPUs: the whole postgres-application lane passed 20 of 20 times on the remote test VM (about 330-345 s each).
• Prettier and eslint are clean on both files.

Risks

• Low. The change is test-only.
• An ended fixture pool no longer reports idle-session errors. Those sessions are being closed and dropped anyway.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

This PR handles late pool errors during revision-fixture disposal and adds a regression case for SQLSTATE 57P01.

Example: A forced test-database drop terminates an idle session after pool.end() resolves.

• Before: The pool re-emits 57P01 without an error listener, failing teardown.
• After: The disposal listener absorbs the late error while existing teardown failures remain reportable.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	A focused, source-supported repair with useful regression coverage and passing integration CI; independently inspectable race-reproduction output is limited.
Proof confidence	🦐 gold shrimp (3/6)	This collaborator-authored fixture repair is outside the ordinary production proof gate; the exact-head PostgreSQL lane passed, and the body reports real PostgreSQL reproduction and a failing-base regression comparison.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Contributors encounter intermittent PostgreSQL integration failures caused by successful fixture cleanup terminating a closing connection.
Reason: The maintainer-authored change repairs a concrete shared-fixture failure without expanding product scope.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: it provides a focused repair for intermittent PostgreSQL fixture teardown failures, and no actionable patch defect was found.

Priority: P2
Reviewed head: 91ccae5082b95e83217f83c37cd17543d9cc921b

Before merge

None.

Findings

None.

Tests

• Missing end-to-end proof: The added case injects 57P01 through a real pg client's event path after disposal; it does not itself reproduce the server timing race.

Agent review details

How this fits together

Revision-worker integration tests use this shared fixture to create isolated PostgreSQL databases, stop workers, close connection pools, and drop their databases.

flowchart TD
  A[Revision worker test] --> B[Shared database fixture]
  B --> C[Stop owned workers]
  C --> D[Attach disposal listener]
  D --> E[End fixture pools]
  E --> F[Force drop database]
  F --> G[Late session error]
  G --> D

Loading

Technical review

Best possible solution:

Keep the repair in the shared revision fixture, preserving active-test error visibility and bounded teardown failures.

Do we have a high-confidence way to reproduce the issue?

Source inspection confirms that pg-pool can forward an idle-client error after pool closure begins, while main's fixture has no listener; the author reports a real PostgreSQL before-and-after reproduction, which was not rerun.

Is this the best way to solve the issue?

The listener is a small repair in the existing shared owner, installed only during disposal without changing production pool behavior or weakening timeout assertions.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 4c945e93b112.

Provenance checked

• Revision fixture teardown keeps the original intent (test(postgres): limit each revision fixture teardown step and name a stuck one #2312: Bound each teardown step, retain failure diagnostics, and continue to forced database cleanup when an earlier step fails.)
• Direct database cleanup transport keeps the original intent (perf(ci): copy and drop lane test databases over a direct postgresql connection #3536: Remove container CLI overhead while preserving administrative SQL and cleanup boundaries.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Focused introduced change: The pinned introduction adds only a disposal-time pool listener and one regression case; the verified test merge contains the same two-file change. (tests/helpers/postgres-worker-revision-fixture.mjs:205, 91ccae5082b9)
• Current main still has the failure path: Main closes raw fixture pools and then force-drops the database without installing this listener; production pool handling from fix(state): log idle PostgreSQL client errors instead of crashing #618 does not cover these directly constructed fixture pools. (tests/helpers/postgres-worker-revision-fixture.mjs:200, 4c945e93b112)
• Dependency contract confirms the mechanism: The fixture imports pg, and the lockfile selects pg-pool 3.14.0. That dependency removes idle clients before their close callbacks finish and forwards their error events to the pool, supporting the reported teardown race. (packages/pg-pool/index.js:51, 2a5850fb0db1)
• Existing teardown guarantees preserved: The repair retains worker-stop ordering, bounded pool closure, forced database cleanup, and aggregate failure reporting established by test(postgres): limit each revision fixture teardown step and name a stuck one #2312 and test(fixtures): end a file whose later root after hook fails after an abandoned step #2349. (tests/helpers/postgres-worker-revision-fixture.mjs:195, 4c945e93b112)
• Exact-head integration check passed: PostgreSQL Application passed for the reviewed head: https://github.com/openclaw/openclaw-enterprise/actions/runs/40144898370/job/121024069844. CI Required also passed. Detailed failure-log retrieval redirected to a blocked Azure storage host, so no claim relies on reading those logs. (scripts/ci/test-suites/postgres-application.json:16, 91ccae5082b9)
• Policy and discussion inspected: Read the complete root policy; no nested test policies, maintainer notes, or pr-simplify skill were present. Discussion contains only review-status bot comments. The supplied prepared images depict the separate Console change and provide no evidence for pool teardown. No helper, second reviewer, or target tests ran, and the checkout remained clean. (AGENTS.md:106, 91ccae5082b9)

Likely related people:

• Free Wortley: Raw commit bdbd7a2 adds tests/helpers/postgres-worker-revision-fixture.mjs:141 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: bdbd7a20d50d; files: tests/helpers/postgres-worker-revision-fixture.mjs)

Labels

Label changes:

• add P2: Intermittent teardown failures undermine the PostgreSQL application lane but do not change deployed product behavior.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

• P2: Intermittent teardown failures undermine the PostgreSQL application lane but do not change deployed product behavior.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 24, 2026, 8:12 PM ET / October 25, 2026, 00:12 UTC.