server/tests leaks database connections: 55 files open a pool and never close it
Maintainers usually reply within 1 day
Assessment
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Newbie friendliness
- 70/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- postgresql, typescript
- Domain
- testing
Research direction
Start with server/tests: grep for files that open a database pool but never call $client.close(), and copy the afterAll pattern from identity-store.integration.test.ts, placing the close last. Then read the two sentences in scripts/test-server-sharded.ts that the issue quotes. Done when all 55 files close their pool, a guard test fails on a new file that opens a database without closing it, and the script's comments are corrected.
Written by the indexing model from the issue text.
Description
What
55 test files in server/tests open a database pool and never give it back. bun test runs
every file in one process, so the pools accumulate for the whole run and main currently sits
at 98 files opening a database, 43 closing one.
PostgreSQL's ceiling is 100 (97, less the superuser reserve), and a file costs two
connections from its first query onward. main is close enough to that ceiling that the next
few integration tests added will cross it, and the failure lands on whoever adds them rather
than on anything they did wrong.
That already happened once: #770 added 19 such files, and its tests job reported 114
failures and 402 sorry, too many clients already — none of which were about the change
under review. It was fixed there by closing the 15 files that branch had added, which returns
the count to main's own, but main's 55 are untouched and the margin is unchanged.
Measured, because the prose in the tree says otherwise
scripts/test-server-sharded.ts states the slots are exhausted "during the import phase", and
concludes the ceiling is "a harness ceiling, not a regression, and it is never a reason to
change source". Both halves are worth revisiting — measurements from a Bun 1.4.2 / PostgreSQL
17 run:
| Probe | Result |
|---|---|
| Construct 5 pools, run no query | 0 connections — construction opens nothing |
| Run one query on each | 10 — a single query opens the pool's full max, not one |
| Leave 20 pools idle 20s | flat at 40 — nothing is ever reaped |
await db.$client.close() |
releases fully |
So exhaustion is a function of how many files have run, not of how many have been imported.
A sharded sweep never sees it because each shard is a fresh process; test:ci walks straight
into it. That difference is why a branch can be green on every local gate and red on CI for a
reason that has nothing to do with it.
Proposed
-
Close the remaining 55.
await database.$client.close()as the last statement of the
file's ownafterAll, after that file's cleanup — the idiom 43 files already use (see
identity-store.integration.test.ts). It must go last: Bun runsafterAllhooks
first-registered-first, so a pool closed before the deletes takes the deletes with it. -
Enforce the pairing, so file 118 can't quietly reintroduce it. A guard test is the right
tool rather than a lint rule: "a file that opens a database closes it" is a pairing rule, not
an import restriction, andserver/testsis outsidetscso types will not catch it either. -
Correct the two sentences in
scripts/test-server-sharded.tsquoted above, and keep the
runner — sharding is still worth having for wall-clock, just not as the only thing standing
between the suite and the ceiling.
Considered and rejected
- One shared pool for the suite. Bounds connections by construction (40 databases over one
max: 4pool measured 4, flat). But 43 files already close their own client, and the first
one to do so kills a shared client for every file after it — measured, 466 failures, all
PostgresError: Connection closed. It would mean removing teardown from 43 files and
accepting that one strayclose()anywhere detonates the run. - A helper that registers its own
afterAll. Removes the convention entirely, and is the
nicest design on paper. Bun runs hooks FIFO, so the helper's close (registered at import)
always runs before the file's own cleanup — measured, 28 failures. - Raising
max_connectionsin CI. GitHub Actions service containers can't override the
container command, so it means running Postgres as an explicit step. It also masks the leak
and leavesbun test server/testsbroken locally.
Not included
While fixing this on #770 we found an unrelated latent bug of the same family: a test read the
audit trail with no ORDER BY and asserted .at(-1) was the most recent row. It passed locally
and failed on CI, reporting an audit misattribution the product had not committed. Fixed in
#770 (arcade-journey); the suite's other 37 audit reads use set semantics and are not at risk.
Worth a look if positional access on unordered reads shows up elsewhere.
- Dominant language
- TypeScript
- Stars
- 6.6k
- Forks
- 914
- Avg merge
- 17h 52m
- Merged PRs (30d)
- 155
Getting set up
- Ships a Dockerfile or Docker Compose file
- Has a pull request template
- No contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from CopilotKit/OpenBot
-
Wrong key location at startupPossibly taken @charan-rathore claimed this today. Open
Difficulty 1/5 Under an hour Newbie friendliness 68/100
CopilotKit/OpenBot#800 ·
Maintainers usually reply within 1 day
-
setup-learning.ts fails on Windows with "spawn npx.cmd EINVAL"Possibly taken @Caldas claimed this 3 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 80/100
CopilotKit/OpenBot#792 ·
Maintainers usually reply within 1 day
-
Difficulty 3/5 1-2 days Newbie friendliness 68/100
CopilotKit/OpenBot#804 ·
Maintainers usually reply within 1 day
-
Five out-of-subject findings from PR #770's tier-3 review (credential-revoking tests, single-user boot gate, OAuth client rotation, shared-use atomicity, untyped server/tests)Possibly taken A pull request linked to this issue is open or already merged. Open
Difficulty 4/5 Over a week Newbie friendliness 22/100
CopilotKit/OpenBot#786 ·
Maintainers usually reply within 1 day
-
Difficulty 3/5 1-2 days Newbie friendliness 35/100
CopilotKit/OpenBot#783 ·
Maintainers usually reply within 1 day
All issues in CopilotKit/OpenBot
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 90/100
chrisparsons83/flexspotff#215 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
vitest-dev/vitest#11633 · 1 comment ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
Maintainers usually reply within 1 day