Files
PaperClipAI/packages/db/src/test-process-tree.ts
T
DottaandPaperclip 33fc7fcddc fix(db): clean up owned PostgreSQL test servers (#18627)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Database, server, and CLI tests use real PostgreSQL fixtures.
> - Each fixture must own its server until PostgreSQL has stopped.
> - The old teardown returned after five seconds even when its server
remained alive.
> - A startup failure could then mark the host unsupported and skip
database coverage.
> - This pull request waits for owned shutdown and makes supported-host
startup failures fail collection.
> - A shared-server command reduces process creation while keeping each
fixture's database isolated.

## Linked Issues or Issue Description

Refs: #7756 for related development-process ownership. That
development-server issue remains outside this test-fixture fix.
Related PR: #16799 collects macOS startup diagnostics. This PR changes
test shutdown and failure behavior.

**What happened?**

Repeated test runs can exhaust macOS PostgreSQL semaphore resources. The
fixture abandoned slow shutdown and swallowed stop errors. The support
probe converted ordinary startup failures into skipped integration
coverage.

**Expected behavior**

Tests must stop their PostgreSQL servers, retain files if stop fails,
and report resource exhaustion as a failure. Fixtures on a shared server
must not close another database's clients.

**Steps to reproduce**

Start a fixture whose PostgreSQL stop exceeds five seconds, or make a
supported-host startup fail with semaphore exhaustion. The former helper
returned from teardown or reported unsupported coverage. The new
regression tests exercise both paths without exhausting host resources.

**Paperclip version or commit**

Source checkout based on current master.

**Deployment mode**

Local source development and CI tests.

## What Changed

- Add explicit ownership for test PostgreSQL children. Wait for fast
shutdown, then request immediate PostgreSQL shutdown if needed. Keep
cleanup idempotent and fail when exit cannot be confirmed.
- Publish reusable templates only after clean shutdown. Do not publish a
crashed or immediately stopped cluster.
- Stop owned children on POSIX worker signals and process exit. Preserve
test directories when shutdown fails. Remove conflicting dependency exit
hooks.
- Retry port collisions only. Fail collection for resource exhaustion,
unavailable configured servers, and other startup failures on supported
platforms. Preserve skips for explicitly unsupported platforms.
- Move administrative-server support into the common fixture used by
database, server, and CLI tests. Close clients at database scope and
migrate cloned templates before use.
- Add a directly invoked shared-server runner to own one test server and
run suites against separate disposable databases. Preserve child exit
status and support an existing disposable server.
- Unify cancellation: stop and wait for the test process group before
PostgreSQL teardown. Add nested-process coverage, real worker-exit
coverage, shutdown regressions, shared-database isolation checks, and
setup documentation. Replace ignored heartbeat completion polls with the
existing execution drain before asserting persisted outcomes.
- Finish owned cluster-directory cleanup after signal-driven
cancellation. Preserve prepared templates across shared-fixture tests.
Remove stopped directories asynchronously, using native removal on POSIX
with a Node fallback.
- Align database and CLI fixture hook budgets with the existing
30-second server budget. This lets the ten-second shutdown protocol
finish and report errors. The idle-sleep suite uses the existing
90-second fixture budget for its unusually large final directory
cleanup.

## Verification

- The semaphore-exhaustion regression failed against the old helper
because it resolved to `supported: false` instead of rejecting.
- All 37 focused tests pass on the final runtime code (`e4de00ab40`).
They include real worker exit, SIGTERM, nested-process SIGINT/SIGTERM
cancellation with PostgreSQL available during child cleanup, failed
template shutdown, unsupported platforms, stale SQL templates, and
directory removal. A child exiting with status 7 makes the shared-server
runner return status 7 after cleanup.
- Full local `pnpm -r typecheck` and `pnpm build` pass on the final
runtime code (`e4de00ab40`). Subsequent changes only adjust server-test
lifecycle waits, the idle-sleep cleanup budget, and documentation; the
server typecheck also passes with those final tests.
- A fresh shared-server run on `90b65fd8ad` passed all 68 database files
and 297 tests. Earlier preparation runs had two ten-second teardown
timeouts and a stale-template failure; the final helper migrates cloned
templates and permits the full shutdown budget. These earlier failures
remain accounted for here.
- The local full repository attempt stopped on a Discord receiver-lease
test after 338 passing chat tests. That exact case passed in isolation.
This full-suite attempt is not a pass.
- On `90b65fd8ad`, [server shard
6](https://github.com/paperclipai/paperclip/actions/runs/41496495253/job/125452949369)
and its one unchanged retry reported two 15-second cached-input
heartbeat timeouts and a 30-second idle-sleep teardown timeout. All ten
cached-input cases then passed locally with their original deadlines. A
local parallel run of the full files reported eight behavioral/hook
failures and two suite teardown timeouts (281 tests passed).
- Serial idle-sleep runs passed all 225 test assertions but overran the
30-second final hook. Tracing measured client closure in 4 ms,
PostgreSQL exit after 5 seconds, and synchronous directory removal
taking another 35 seconds. Asynchronous Node removal and native removal
also exceeded that hook under the full workload. Cleanup now remains
asynchronous and awaited, and only this heavy suite's final hook uses
the existing 90-second fixture budget. No PostgreSQL shutdown deadline
or test assertion was weakened. The serial run then passed the complete
idle-sleep file, including all 225 tests and teardown. Its heartbeat
file passed 63/64 cases, exposing an ignored three-second completion
poll. After replacing that poll with an execution drain, another timer
case exposed the same pattern (63/64 passed). All remaining ignored
completion polls now await execution drain without changing test
deadlines. The complete heartbeat file then passed all 64 tests. The
intentional asserted lock-contention poll is unchanged. The final server
typecheck passes, and a process audit found no PostgreSQL server left by
the completed local runs.
- On final head `2cf316d3f571e4516890cba743ecffeb9703cce3`, the
previously failing server shard 6, database lane, full typecheck, and
build passed. The [first CI
attempt](https://github.com/paperclipai/paperclip/actions/runs/41506358136)
then lost self-hosted runners in the canary and serialized-server lanes,
with additional jobs cancelled. The server-2 lane had one 30-second
timeout in the unchanged workspace-recovery HTTP Retry test (2,282 other
tests passed). That exact test passed locally with its original deadline
(28.78 seconds including imports). One unchanged retry of only failed
and cancelled jobs completed successfully on the exact final commit
(workflow attempt 2). The workspace-recovery timeout case and every
interrupted lane passed; successful first-attempt jobs were retained.
The additional Paperclip Review check remains queued, waiting for its
assigned agent; it has no unresolved GitHub review threads. Greptile
completed review on this exact final head at 5/5, with zero unresolved
review threads. All four original review findings and their threads are
resolved; additional Windows shutdown, cancellation-directory, and
template-reuse findings are fixed.
- `pnpm check:module-boundaries` and `git diff --check` passed. Post-run
process checks found no PostgreSQL servers left by the completed
verification runs.

## Risks

- Immediate PostgreSQL shutdown can require crash recovery. This
fallback applies to disposable test clusters and runs only after fast
shutdown times out.
- No process can handle `SIGKILL` or machine power loss. Host resource
cleanup remains a separate backstop.
- Directory cleanup on cancellation applies to the shared-server
runner's own cluster. Directly terminated test workers stop their
PostgreSQL process but retain disposable files for inspection. The
process-exit guard cannot await file removal.
- The two-second `pg_ctl -W` deadline bounds signal dispatch. It is
separate from the five-second and thirty-second server-exit waits. A
control-command failure fails cleanup and retains its directory; it does
not report successful cleanup or force-delete live files.
- Shared fixtures require a disposable PostgreSQL server and a role that
can create databases. The directly invoked shared command is opt-in; the
existing `pnpm test` command remains available.
- These changes do not stop unrelated development servers or delete
host-wide IPC resources. Clean shutdown uses the owned `pg_ctl` binary
on every supported platform. Its real control path was tested on macOS.
A Windows host was not available for live verification; POSIX signal
tests skip on Windows.

## Model Used

- OpenAI Codex, based on GPT-6. The runtime does not expose the exact
serving variant or context-window size. The agent used reasoning, shell
execution, code editing, and GitHub tools.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-11-02 14:24:48 -06:00

79 lines
2.5 KiB
TypeScript

import { execFile, spawn } from "node:child_process";
import { constants } from "node:os";
import { setTimeout } from "node:timers/promises";
import { promisify } from "node:util";
const execute = promisify(execFile);
export function startTestProcessTree(command: string, args: string[], env: NodeJS.ProcessEnv) {
const posix = process.platform !== "win32";
const child = spawn(command, args, { stdio: "inherit", env, detached: posix });
let exited = false;
let stopping: Promise<void> | null = null;
let retired = false;
const exit = new Promise<number>((resolve, reject) => {
child.once("error", (error) => { exited = true; reject(error); });
child.once("exit", (code, signal) => {
exited = true;
resolve(code ?? (signal ? 128 + constants.signals[signal] : 1));
});
});
async function alive(): Promise<boolean> {
if (!child.pid) return false;
if (!posix) return !exited;
const { stdout } = await execute("/bin/ps", ["-axo", "pgid=,stat="], { timeout: 2000, maxBuffer: 1024 * 1024 });
return stdout.split("\n").some((line) => {
const [group, state] = line.trim().split(/\s+/);
return Number(group) === child.pid && state && !state.startsWith("Z");
});
}
function signal(signal: NodeJS.Signals): void {
if (!child.pid || retired) return;
try {
if (posix) process.kill(-child.pid, signal);
else child.kill(signal);
} catch (error) {
if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error;
}
}
async function waitUntilStopped(milliseconds: number): Promise<boolean> {
const deadline = Date.now() + milliseconds;
do {
if (!await alive()) return true;
await setTimeout(50);
} while (Date.now() < deadline);
return false;
}
function stop(requested: NodeJS.Signals = "SIGTERM"): Promise<void> {
if (stopping) return stopping;
stopping = (async () => {
if (posix) {
signal(requested);
if (!await waitUntilStopped(15_000)) {
signal("SIGKILL");
if (!await waitUntilStopped(5_000)) throw new Error("Test command process group did not stop");
}
} else if (child.pid && !exited) {
await execute("taskkill", ["/PID", String(child.pid), "/T", "/F"], { timeout: 15_000 });
}
await exit;
retired = true;
})();
return stopping;
}
return {
stop,
interrupt: () => signal("SIGTERM"),
async wait(): Promise<number> {
const status = await exit;
await stop();
return status;
},
};
}