mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-13 04:02:28 +02:00
85f4205de3
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Pull requests to this repository run a large CI matrix. The slowest
check sets the time a contributor waits for a green run.
> - After #15853, the twelve `server (n/12)` shards are the slowest
check in every fully green PR run created since its merge (518s to 548s;
`Build` is 482s to 504s). Each shard is about 60s of setup and 454s to
490s of Vitest, with one worker, so its wall time is the sum of its
suites.
> - From two shard logs, 22 to 24 of the 71 to 74 suites in a shard
start an embedded Postgres cluster. Each one runs `initdb`, starts the
cluster, and applies all 331 migrations before its first test. The
smallest such suite takes 6s with one or two trivial tests. On top of
that, the `getEmbeddedPostgresTestSupport` probe starts and stops a
second throwaway cluster at import time in 19 of those suites, which is
why their per-file import overhead is 3.5s to 4.1s against 0.9s to 1.0s
for suites without Postgres.
> - A cluster data directory that was shut down cleanly can be copied
and started again as an independent cluster, and the migrated schema is
the same for every suite.
> - This pull request builds one migrated cluster per host, publishes
its data directory as a template under the OS temp directory, and starts
every later suite from a copy of it. The probe reuses the template
instead of starting a cluster.
> - The benefit is about 7s to 8s per Postgres suite on the CI runners
(measured 3.2s to 0.35s per start and 0.75s to 0.03s per probe locally),
about 120s to 170s per server shard, and a faster local test loop for
every suite that uses the fixture.
## Linked Issues or Issue Description
**What existing behavior does this improve?**
The `server (n/12)` PR checks are the slowest check in every fully green
PR run since 2026-10-11 05:27Z. Their Vitest step runs suites one at a
time, and about a third of each shard's time is embedded Postgres setup
that produces the same migrated cluster every time.
**Subsystem affected**
Test infrastructure: the embedded Postgres test fixture in
`@paperclipai/db` (`packages/db/src/test-embedded-postgres.ts`), used by
349 server suites, the db package suites, and the CLI suites.
**Current behavior**
`startEmbeddedPostgresTestDatabase` runs `initdb`, starts the cluster,
creates the database, and applies all 331 migrations for every suite.
`getEmbeddedPostgresTestSupport` starts and stops a second throwaway
cluster once per test file, because the module-level cache does not
survive Vitest's per-file isolation. Measured locally: 3.2s per suite
start plus 0.4s cleanup, and 0.75s per probe. On the CI runners the
smallest Postgres suite takes 6s and the probe adds 2.5s to 3s of import
time per file.
**Proposed behavior**
The first caller on a host builds one migrated cluster, stops it, and
renames its data directory to
`$TMPDIR/paperclip-embedded-postgres-templates-<uid>/<key>`. Every later
caller copies the template (about 90 MB) and starts Postgres on the
copy, then still runs `ensurePostgresDatabase` and
`applyPendingMigrations`, which are no-ops on a current template. The
probe treats a published template as proof that the host can run the
cluster. The key hashes the embedded-postgres version, the initdb flags,
the user, and the name and content of every migration file plus the
journal, so a changed migration builds a new template.
`PAPERCLIP_TEST_POSTGRES_TEMPLATE=0` bypasses the cache. Any template
failure falls back to the fresh path.
A template is a Postgres data directory, and starting it executes its
configuration, so the cache is private to the current OS user: the root
name carries the uid, the root is created with mode 0700, and both the
root and each template are used only when `lstat` shows a real directory
(not a symlink) owned by the current user with no group or world write
bit. A root that fails those checks (for example one pre-created by
another user in a shared `/tmp`) is left alone and the process uses
fresh clusters.
**Reason and benefit**
The slowest check decides the wall-clock time of every PR run. Measured
locally, a template start takes 0.3s to 0.4s instead of 3.2s, cleanup
0.15s instead of 0.43s, and the probe 0.03s instead of 0.75s. Four
server suites that passed in both modes dropped by 3.4s to 6.0s each;
the db `backup-lib` suite dropped from 42.7s to 20.9s. The CI runners
are about twice as slow, so the expected saving per server shard is 120s
to 170s.
**Breaking changes**
None. The returned database is fully migrated in both paths, each suite
still gets its own cluster and data directory, and a failure on the
template path falls back to the existing fresh path.
## What Changed
- `packages/db/src/test-embedded-postgres.ts`: adds the template cache.
`computeEmbeddedPostgresTemplateKey` hashes the inputs that determine a
migrated cluster. `buildEmbeddedPostgresTemplate` starts a fresh cluster
through the existing bounded retry, creates the database, applies
migrations, waits for the real `stop()` (30s bound), and publishes the
data directory with an atomic rename; a concurrent builder that loses
the rename discards its copy.
`startEmbeddedPostgresFromTemplateWithRetry` copies the template into a
fresh temp directory, sets mode 0700, and starts the cluster with the
same per-attempt port and directory rotation as the fresh path.
`startEmbeddedPostgresTestDatabase` tries the template first and falls
back to the fresh path with one warning per process.
`probeEmbeddedPostgresSupport` returns supported when a template exists
or can be built, and keeps its throwaway-cluster path for the case where
the template could not be published. Templates for other keys are pruned
after seven days. The connection strings and initdb flags move into
shared constants; no other behavior changes.
- `packages/db/src/test-embedded-postgres.test.ts`: two unit tests for
the key (stable for identical inputs; changes with migration content, a
new file, flags, version, or user; hashes this package's real migrations
directory) and two real-cluster tests (the published template is
complete and private, a suite starts from it in `template` mode with
migrations up to date, a table written to one copy is not visible in the
next copy, and `PAPERCLIP_TEST_POSTGRES_TEMPLATE=0` runs the `fresh`
path). The real-cluster tests skip on hosts without embedded Postgres,
like every other fixture suite.
- `doc/DEVELOPING.md`: a short section under Test Commands describing
the template, its key, and the bypass variable.
**Review fixes (second commit), one per finding:**
- Private cache root (Greptile P1, Superagent P2). The root is
`$TMPDIR/paperclip-embedded-postgres-templates-<uid>` (user name on
Windows), created with mode 0700. `prepareTemplateRootDir` and
`isUsableTemplate` both reject, via `lstat`, anything that is a symlink,
not a directory, owned by another uid, or writable by group or others; a
rejected root logs one warning and the process uses fresh clusters. A
root created by the first commit with a wider mode is narrowed to 0700
if this user owns it.
- Template failures never skip suites (Greptile P1).
`getEmbeddedPostgresTemplateDir` no longer rejects. A build or root
failure resolves `null` with one warning, and
`probeEmbeddedPostgresSupport` then runs the throwaway-cluster probe, so
only a cluster that cannot start reports `supported: false`. A migration
that cannot apply fails the suite on the fresh path instead of being
hidden by `describe.skip`.
- Idle-based pruning (Greptile P2). Each reuse refreshes the template's
timestamp with `utimes`, so the seven-day prune measures idle time. A
template that disappears after a process resolved its path is not
retried five times: `startEmbeddedPostgresFromTemplateWithRetry` checks
the template before each copy and throws at once, the suite falls back
to a fresh cluster, and the fallback forgets the path so the next caller
in that process rebuilds the template.
- The template-path test enables the cache for itself and restores the
original `PAPERCLIP_TEST_POSTGRES_TEMPLATE` value afterwards (Greptile
P1), so the suite passes under `PAPERCLIP_TEST_POSTGRES_TEMPLATE=0` as
well.
- Eight new tests: `isUsableTemplate` accepts a private cluster
directory and rejects a missing `PG_VERSION`, a leftover
`postmaster.pid`, a symlink, and (POSIX) group- or world-writable modes;
a world-writable pre-created root and a symlinked root both fall back to
fresh clusters, publish nothing, log one warning, and still report
`supported: true` from the probe; a ten-day-old template is refreshed on
reuse; a pruned template is rebuilt on the next start; and a template
build that fails after the cluster started falls through to the fresh
probe. The tests that need an empty cache point `os.tmpdir()` at a
scratch directory (`TMPDIR`, `TEMP` and `TMP` together, so the redirect
holds on Windows as well) and restore it.
**Review fixes (third commit, e8f8e9c8), one per finding:**
- Fallback test on Windows (Greptile P1). `useScratchTmpdir` set only
`TMPDIR`, which Windows `os.tmpdir()` ignores, so the one scratch-root
test that runs on every platform would assert on paths outside the
scratch root there. The helper now sets and restores `TMPDIR`, `TEMP`
and `TMP` together, and that test asserts `os.tmpdir() === scratch`
before it checks any path under it.
- Timestamp test runs on its own (Greptile P2). The test assumed an
earlier test had published a template; under
`PAPERCLIP_TEST_POSTGRES_TEMPLATE=0` the probe does not, so a
name-filtered run threw `ENOENT` from `utimes`. It now starts and cleans
up one suite first, which builds the template when nothing has, and
asserts `isUsableTemplate` before ageing it.
## Verification
- `pnpm --filter @paperclipai/db exec tsc --noEmit`: clean (both
commits).
- `pnpm --filter @paperclipai/db exec vitest run
src/test-embedded-postgres.test.ts`: 15 pass (12 new across both
commits) with the template root absent (first run builds it), with it
present, and with `PAPERCLIP_TEST_POSTGRES_TEMPLATE=0`.
- After the review fixes: `client.test.ts` and `backup-lib.test.ts` (29
tests) pass on the template path, and
`agent-action-audit-routes.test.ts` and `status-cards.test.ts` (36
tests) pass through the server route harness. The second commit's CI run
([38121021162](https://github.com/paperclipai/paperclip/actions/runs/38121021162))
passed every check: `server (n/12)` jobs 277s to 425s, Vitest `Duration`
219s to 365s, no fallback warning in any shard.
- Third commit: reproduced the Greptile P2 on the old file first
(`PAPERCLIP_TEST_POSTGRES_TEMPLATE=0 vitest run
src/test-embedded-postgres.test.ts -t "refreshes the template"` on an
empty cache root fails with `ENOENT … utime`), then with the fix the
same command passes, and the full file passes 15/15 on a cold cache, a
warm cache, and with the bypass set. `tsc --noEmit` clean.
**Measured on this PR's first CI run
([38117892083](https://github.com/paperclipai/paperclip/actions/runs/38117892083)),
which ran the branch's fixture in every server shard:**
| | Before (runs 38114982393, 38115279131, 38115459196) | This PR's run
|
| --- | --- | --- |
| Vitest `Duration` per `server (n/12)` shard | 454s to 490s | 224s to
337s |
| `server (n/12)` job | 518s to 548s | 286s to 401s |
| Slowest check in the run | a server shard | `Build`, 453s |
No shard logged a fixture fallback warning, so every Postgres suite in
the run started from the template.
- `pnpm --filter @paperclipai/db exec vitest run
src/status-card-migrations.test.ts
src/nested-skill-folders-migration.test.ts
src/chat-github-migration.test.ts
src/question-response-delivery-migration.test.ts src/client.test.ts`: 5
files, 24 tests pass on the template path. These suites drop indexes,
re-run migration files, and create extra databases on the cluster, so
they cover the semantics the template must preserve.
- Four server suites run with `PAPERCLIP_TEST_POSTGRES_TEMPLATE=0` and
then with the template, all passing in both modes:
| Suite | Fresh path | Template path |
| --- | --- | --- |
| `server/src/__tests__/agent-action-audit-routes.test.ts` (11 tests) |
17.9s | 14.6s |
| `server/src/__tests__/status-cards.test.ts` (25 tests) | 22.2s | 18.6s
|
| `server/src/__tests__/plugin-managed-routines.test.ts` (3 tests) |
18.2s | 14.3s |
| `server/src/__tests__/issue-thread-interactions-service.test.ts` (106
tests) | 19.0s | 12.9s |
| `packages/db/src/backup-lib.test.ts` (9 tests) | 42.7s | 20.9s |
- Per-operation timings on the same host: fresh `initdb` + start +
create database + 331 migrations 3.2s, cleanup 0.43s; template copy
0.20s to 0.23s, start from copy 0.03s to 0.04s, `ensurePostgresDatabase`
0.013s, `applyPendingMigrations` fast path 0.03s to 0.04s, stop 0.03s.
Probe with the template on disk 0.03s (was 0.75s). Template size 87 MB,
3215 files.
- This PR's own server shards run the branch's fixture, so their Vitest
`Duration` lines show the saving directly. Expected on this PR's run:
`server (n/12)` about 330s to 430s (was 440s to 548s on runs
38114982393, 38115279131 and 38115459196), and the slowest check moves
to `Build` (482s to 504s).
**Measured on the three fully green `master` PR runs created after
#15853 merged** (slowest server shard logs):
| Run | Server shards | Vitest `Duration` (slowest shard) | Postgres
suites in it | Their reported duration | Their import overhead |
| --- | --- | --- | --- | --- | --- |
| 38115459196 | 476s to 548s | 483.8s (tests 72%, import 24%) | 22 of 71
| 197s | 83s |
| 38114982393 | 409s to 518s | 452.8s (tests 65%, import 30%) | 24 of 74
| 264s | 98s |
## Risks
- Low risk to correctness. The template is a cleanly stopped cluster
produced by the same `initdb` flags and the same migration runner as the
fresh path, and each suite still runs `ensurePostgresDatabase` and
`applyPendingMigrations` on its copy. Each suite gets its own copy, its
own port, and its own data directory, so isolation between suites is
unchanged.
- A stale template is prevented by the key. If a migration file, the
journal, the embedded-postgres version, the initdb flags, or the user
changes, the key changes and a new template is built. Even if a stale
template were used, `applyPendingMigrations` on the copy would bring it
up to date.
- Concurrent first use (for example the file-parallel db project) can
build the same template twice. The rename is atomic, so one copy wins
and the other is discarded; a template that exists is always complete.
The loser pays one extra fresh start, which is what every suite paid
before.
- The template root lives under the OS temp directory and holds about 90
MB per key. Keys that have not been used for seven days are pruned when
a new template is built; each reuse refreshes the timestamp.
- A template is executable configuration, so the cache is only used when
the root and the template are private directories of the current user
(ownership, no symlink, no group or world write bit). On a host where
another user pre-created the path, the process logs one warning and uses
fresh clusters. Windows has no uid, so there the check is
directory-only; its temp directory is already per user.
- Any failure on the template path (root checks, build, copy, start,
publish) falls back to the fresh path and logs one warning per process,
so a broken cache can make a suite slower but never fail it or skip it.
The guard tests assert the mode, so a silent fallback on CI would fail
`test-embedded-postgres.test.ts` rather than hide.
- Hosts that cannot run embedded Postgres still get `supported: false`
with the real Postgres output, now from the throwaway probe: a failed
template build is one warning, then the probe decides. On such a host
the probe costs one extra failed start attempt cycle per file compared
with the first commit; every suite on that host skips anyway.
## Model Used
- Claude (Anthropic), model ID `claude-fable-5-1`, running as a
Paperclip agent through the Claude local adapter with tool use (shell,
file edits, GitHub CLI). The agent collected the CI data, made the
change, ran the tests, and wrote this description.
## 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 (53 checks green on both run
38117892083 and run 38121021162; the third commit re-runs them)
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
(1/5 on the first commit, 4/5 on the second; every finding so far is
fixed and answered, and Superagent confirmed its fix; re-review of the
third commit pending)
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Bender (Fable) <bender-fable@users.noreply.github.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com>