Repository navigation
perf(stack): reuse postgres image for storage helper - #6760
Merged
Merged
Conversation
Compiled CLI builds now enable minification, ESM, and bytecode at depth 2 to reduce startup work. Release, local, and E2E binaries share the same compilation options, including the local npm release workflow. Depth 2 limits the binary-size cost compared with compiling all nested functions ahead of time. --------- Co-authored-by: Julien Goux <hi@jgoux.dev>
A Postgres data directory is mostly small files, so one clone is cheaper than a clone per file. Unsupported entries still use the per-file copy.
Linux reflinks when the filesystem can, and Windows uses robocopy. A failed or interrupted host copy removes the partial tree first.
Shadow destroy issues docker stop and rm in the background. An explicit stop, including the one before a snapshot, still waits for a checkpoint.
Reuse one helper container per volume, remember the daemon identity, and turn crash safety off for a shadow that is deleted after the diff.
The phase file was only for benchmarks. A shared helper is removed only by the process that created it, so another diff cannot delete the container the first one is using.
docker exec cannot use a container left in created, and adopting it again left later diffs stuck. The helper handle type stays private.
A cancel during chmod left the failed host copy in place, and the interrupt test read the last argument, which is /NP on Windows.
Effect lint rejects node:fs and node:path in this package, and those warnings fail the quality check.
Keep the explicit copy destination from the base and write the partial fixture through Effect file APIs.
Keep develop's oxfmt stub and root-key default. Keep the host-command directory copy and the spawner it needs.
Parallel tests reused one stack id, so on Linux they shared a data directory and the pooler database exited during ready.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🤖 AI Review
All four Claude findings are supported by the code; Codex reported none. PostgreSQL 15 has database integration coverage, but its snapshot helper path is untested. The pinned image contents could not be inspected in this pass, so these findings do not establish that either image currently lacks a required tool.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | packages/stack/src/storage/DockerDatabaseStorage.ts:831 |
maintainability |
claude | Snapshot operations depend on specific binaries and applets in the pinned PostgreSQL images, but the artifact catalog does not record or check those requirements. |
| 🟡 MINOR | packages/stack/src/storage/DockerDatabaseStorage.integration.test.ts:81 |
test-coverage |
claude | The changed PostgreSQL 15 digest is not exercised by a snapshot helper test; every Docker storage integration case selects PostgreSQL 17. |
| 🟡 MINOR | packages/stack/src/storage/DockerDatabaseStorage.integration.test.ts:151 |
test-reliability |
claude | The new xattr preservation check fails the entire integration test on a Docker volume filesystem that cannot store user xattrs. |
| ⚪ NIT | packages/stack/src/storage/DockerDatabaseStorage.ts:630 |
consistency |
claude | Helper commands mix bare tools resolved through PATH with absolute BusyBox applet paths, making their command requirements harder to track. |
Stats
Claude findings: 4 · Codex findings: 0 · Confirmed: 4 · Refuted: 0 · Uncertain: 0
Models: claude-opus-5-5 + gpt-6-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
jgoux
force-pushed
the
perf/stack-postgres-storage-helper
branch
from
September 23, 2026 17:36
03e1e69 to
45c6f27
Compare
The node:fs imports fail the required quality check.
avallete
approved these changes
Sep 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use the pinned Postgres service image for database storage operations. The image includes GNU cp, while BusyBox supplies the remaining filesystem tools. This removes the separate Debian helper pull that could fail under registry rate limits.
Pin the two database versions in the stack catalog to their republished image digests. The storage integration suite exercises snapshot save, restore, retention, metadata preservation, and cleanup against the published image.
The shared volume helper introduced by #6710 keys its container by both mounts and Postgres image, so separate database versions use the correct image.