perf(stack): replace snapshot archives with managed directory snapshots - #6700
Conversation
There was a problem hiding this comment.
🤖 AI Review
Nine deduplicated findings were adjudicated from eight Claude findings and two Codex findings. Seven are confirmed and two are refuted. The most significant confirmed issues are Docker storage markers accepting mismatched state/cache identities and stacks becoming undestroyable after a Docker daemon identity change. Additional confirmed concerns cover unreclaimed volumes, a Docker Hub dependency, root-owned fallback cache data, one non-hermetic test, and inaccurate copy error context.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | packages/stack/src/storage/DockerDatabaseStorage.ts:655 |
error-handling |
claude | A volume-backed database cannot be started or destroyed after its recorded Docker daemon identity changes, leaving no supported way to remove the stack state. |
| 🟠 MAJOR | packages/stack/src/storage/DockerDatabaseStorage.ts:229 |
data-isolation |
codex | An existing Docker storage marker is reused without verifying that its volume and cache namespace belong to the current stateRoot and cacheRoot, allowing copied or moved registries to share live PGDATA and stale cache unexpectedly. |
| 🟡 MINOR | packages/stack/src/storage/DockerDatabaseStorage.ts:19 |
reliability |
claude | The volume-backed database path introduces an unconditional Docker Hub dependency for its helper image, unlike the stack service images hosted on GHCR. |
| 🟡 MINOR | packages/stack/src/storage/DockerDatabaseStorage.ts:671 |
resource-leak |
claude | Managed database volumes are never reclaimed, so ephemeral state roots—including the new Docker cache E2E—accumulate orphaned volumes and retained snapshots. |
| 🟡 MINOR | packages/stack/src/storage/DockerDatabaseStorage.ts:693 |
reliability |
claude | The host-backed Docker fallback creates snapshot cache contents as root, leaving parts of the user's cache tree inaccessible or undeletable without a container or elevated privileges. |
| 🟡 MINOR | apps/cli/src/commands/db/shared/pgdelta-next-shadow.stack.integration.test.ts:139 |
test-coverage |
claude | The stack shadow integration test is non-hermetic because ambient SUPABASE_SHADOW_CACHE=0 disables caching and makes its identity assertion fail. |
| ⚪ NIT | packages/stack/src/storage/DirectoryCopy.ts:141 |
error-handling |
claude+codex | Nested lstat failures report the parent destination rather than the affected child destination. |
Refuted findings (kept for transparency, not posted as review comments)
packages/stack/src/storage/DockerDatabaseStorage.ts:418(resource-leak): An abrupt StackHost exit leaking the detached helper container violates the repository's expected crash-cleanup behavior.
Refuted: The trusted default-branch architecture explicitly states that startup does not scan for orphaned resources, unexpected host death has no orphan reconciliation, and leftovers may require manual cleanup (trusted/packages/stack/ARCHITECTURE.md:459,478,527,599). The helper follows that documented convention and remains manually discoverable through its management labels.packages/stack/src/runtime/Container.ts:148(error-handling): The image-inspection regex omits supported Docker or Podman missing-image diagnostics and can prevent pulling an absent image.
Refuted: Both supported engines' established missing-image messages are covered. The trusted default-branch resolver documents the same two classifications and intentionally fails other inspect errors instead of treating daemon, authorization, or plugin failures as cache misses (trusted/apps/cli/src/command-internal/docker-image-resolve.ts:37-43).
Stats
Claude findings: 8 · Codex findings: 2 · Confirmed: 7 · Refuted: 2 · Uncertain: 0
Models: claude-opus-5 + gpt-5.6-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
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>
Local and database-URL TypeScript generation now emits the generator’s unformatted output, removing the oxfmt implementation and native addon from the compiled CLI. The CLI explains on stderr that users can format the output after saving it. TypeScript returned by the Management API and Go, Python, and Swift generation are unchanged. Builds replace the unused typegen formatter import with a stub and retain the shared minification, ESM, and bytecode settings from #6708. --------- Co-authored-by: Julien Goux <hi@jgoux.dev>
The stack shadow cache currently exports and restores tar archives, adding archive creation, extraction, and host/container transfers to cache reuse. Replace that boundary with managed, keyed database snapshots:
Remove
exportSnapshotand the path-based restore contract, and migrate the CLI baseline cache to the new API. Docker keeps database data and snapshots in separate namespaces within a managed volume, using a reusable helper and filesystem cloning when available. Native snapshots use directory cloning with ordinary-copy fallback; Windows uses ordinary copies. Podman, older Docker engines, and existing host-backed data retain the host-backed path.Snapshots require stopped instances with wake disabled; restore rejects nonempty targets. Managed storage owns compatibility checks, atomic publication, replacement, locking, and retention of three entries by last use. The CLI rebuilds unusable cached baselines. Snapshots survive source-instance destruction; Docker reuse requires the same daemon, state root, and cache root. Cached helper images are resolved with image inspection to avoid unnecessary pulls for digest references.
Cache rotation and lifetime
Each cache bucket retains at most three published snapshots after a successful save: the newly saved entry and the two most recently used other entries. Successful saves and restores update the entry's last-used timestamp; saving evicts older entries under the same lock used by save and restore. Interrupted cache publication files are cleaned up under that lock.
For a typical developer setup with one Supabase home, one Docker daemon, and the default cache location, this means three Docker snapshots total, shared across projects—not three per stack. The Docker volume is scoped to the state root and daemon; its cache namespace is derived from the canonical cache root. Native and host-backed caches have their own buckets, so the limit is per bucket rather than host-wide, and limits entry count rather than bytes.
Destroying a stack removes its database instance data but retains the shared snapshot cache for subsequent reuse. Docker also retains the managed volume. Repeated stack creation/destruction does not add an unbounded number of snapshots to the same bucket; reclaiming abandoned buckets or whole volumes is outside this change.
Public API benchmarks: cloning and copying
Milliseconds, median of three measured repetitions after one warmup. Combined is the median of each repetition's save + restore, rather than the sum of the two independent medians.
These use real PostgreSQL 17 data: the small dataset has 96 rows; the large dataset has 32,768 rows containing 256 MiB of payload. The physical native trees are approximately 46.8 MiB and 428.8 MiB. Every measured restore starts PostgreSQL and verifies row counts and a content checksum, outside the save/restore timing.
Artifacts and images are already present. The source is stopped with its helper warm; each restore uses a fresh target component, including helper creation on Docker. Startup/readiness is shown separately; verification queries, stop, destroy, and initial source setup are excluded from the combined result. Filesystem caches are warm. The macOS large-data run was repeated unchanged after one SQL transport failure; the table uses the complete, verified rerun.
The macOS measurements are local; Linux measurements use CI runners. XFS and the local Docker Btrfs volume support forced reflinks; ext4 rejects them and exercises copying. Different machines and filesystems prevent interpreting these as controlled operating-system comparisons.
Linux Docker benchmark run and artifacts
Ordinary-copy fallback by platform
Milliseconds, median of five measured repetitions after one warmup. These call the unchanged native snapshot storage backend directly, including compatibility validation, locking, staging, publication, and retention. They exclude public API/RPC overhead and PostgreSQL startup, so they are a separate benchmark from the table above.
The fixtures reproduce the real PostgreSQL file layouts with deterministic synthetic bytes: 46.8 MiB / 1,317 files and 428.8 MiB / 1,326 files, with 28 directories each. No original database contents are copied. Full file hashes and source/cache/restore write isolation are checked outside timing; no PostgreSQL process runs in this benchmark.
macOS uses HFS+ because Bun can automatically clone large files on APFS even without a clone flag. Forced-clone probes return
ENOTSUPon HFS+ and Linux ext4. Windows uses the production ordinary-copy path on NTFS. These are warm-cache, non-fsync measurements on Bun 1.4.1. The macOS runner uses a virtual Apple M1, Windows an AMD EPYC 7763, and both Linux variants Intel Xeon Platinum 8573C runners; differences are not attributable to the OS or libc alone. Windows results establish storage-backend behavior, not availability of a native Windows PostgreSQL artifact.Cross-platform ordinary-copy benchmark run and artifacts