Conversation
…ners A freshly started container can fail to see a config file a host just replaced via temp-dir-then-rename, because some Docker file-sharing engines (confirmed with OrbStack) cache the replaced path's previous, now-deleted inode. Vector api/pipeline config and the Functions edge runtime bootstrap both rewrote a fixed, bind-mounted path on every start, so a reopened stack could intermittently log "Config file not found" for Vector or fail to bind-mount the Functions bootstrap. Make both recipes content-addressed instead: each write lands at a path derived from a digest of its content (still via temp-dir-then- rename for atomicity). The container mounts a static directory (Vector configRoot, the Functions bootstrap root); only the generation name inside it, computed in args, varies. A fresh container therefore only ever reads a name that has never been deleted before. Unchanged content resolves to the same name and is skipped. Stale generations are pruned once any previous container is confirmed stopped (after args, never in prepare, which can still run while a restart's old container is live). Every other service recipe that bind-mounts a file was checked (Database, Storage, Studio, Imgproxy, and the lazy-service mounts); only Vector and the Functions bootstrap write a stack-owned file onto a path they also bind-mount, so only those two needed this fix.
Contributor
There was a problem hiding this comment.
🤖 AI Review
Both independent reviews were available. Code inspection confirms publication and pruning races, the missing-bootstrap fallback, legacy Vector cleanup omissions, coverage gaps, incorrect error labels, and a duplication nit. Duplicate claims were merged; Claude's compound polish finding was split, yielding eight confirmed entries. Existing runtime tests indirectly cover valid mount paths. Tests were not run because dependencies are not installed.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | packages/stack/src/services/Functions.ts:205 |
error-handling |
claude | A missing published generation silently switches Edge Runtime's main service from the stack bootstrap to the user's functions directory. |
| 🟠 MAJOR | packages/stack/src/functions/FunctionsBootstrap.ts:166 |
concurrency |
claude+codex | Overlapping writes of identical bootstrap content can fail when the second writer renames its staged directory onto an already-published nonempty generation. |
| 🟠 MAJOR | packages/stack/src/functions/FunctionsBootstrap.ts:196 |
concurrency |
codex | Launch-time pruning can delete staging directories belonging to overlapping preparation writers in both Functions and Vector. |
| 🟡 MINOR | packages/stack/src/services/Vector.ts:283 |
data-cleanup |
claude | Vector destroy leaves previously generated fixed-name configuration files behind, including rendered configurations that may contain an API token. |
| 🟡 MINOR | packages/stack/src/functions/FunctionsBootstrap.ts:186 |
test-coverage |
claude | The new generation-pruning, missing-generation lookup, and unchanged-content write behavior lack focused regression coverage. |
| 🟡 MINOR | packages/stack/src/services/Vector.ts:110 |
error-handling |
claude+codex | Vector config failures receive inconsistent operation labels: digest or existence failures during prepare are labeled launch, while write failures during launch are labeled prepare. |
| ⚪ NIT | packages/stack/src/services/Vector.ts:63 |
duplication |
claude | Vector and FunctionsBootstrap duplicate the hex helper and truncated SHA-256 naming calculation. |
| ⚪ NIT | packages/stack/src/services/Vector.integration.test.ts:86 |
test-coverage |
claude | The caller-pipeline destroy test checks an obsolete filename that is never created, so its generated-config cleanup assertion passes even if cleanup is broken. |
Findings outside the diff
- ⚪ NIT
packages/stack/src/services/Vector.integration.test.ts:86— The caller-pipeline destroy test checks an obsolete filename that is never created, so its generated-config cleanup assertion passes even if cleanup is broken.
Stats
Claude findings: 6 · Codex findings: 3 · Confirmed: 8 · Refuted: 0 · Uncertain: 0
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
…g launches Overlapping writers for identical content could each stage separately and race their publish step: Vector rename replaced an already-live file, and the Functions bootstrap rename onto an already-published, non-empty generation failed outright. Vector now publishes with a hard link and treats an existing link as success; the Functions bootstrap treats a failed rename as success once the existing destination is a complete generation. Both remove only their own stage afterward. Pruning no longer removes staging entries (.vector-write-*, .generation-*.tmp) that an in-flight writer may still own; only completed, published generations are eligible, and only once a previous container is confirmed stopped. Abandoned stages are still removed in full by removeData/cleanupAll on destroy. Functions args() now always publishes the bootstrap itself instead of falling back to the caller's functions directory when nothing had been published yet, so the recipe never silently serves a degraded main service. The locate-only read path is gone since nothing else needed it. The Windows-path unit test now supplies a fake bootstrap owner instead of exercising a real one, since a real owner would join the Win32 stack root with real fs calls and write a garbled path on a POSIX test host. Vector removeData now also deletes the fixed-name files earlier releases wrote (vector-api.yaml, vector.yaml, vector.rendered.yaml), except one that resolves to the caller's own configured pipeline; the rendered file can carry an API token. The Functions bootstrap already removed its entire root on cleanup, so its own legacy loose-file layout needed no separate handling. Error operations (prepare vs. launch) are now threaded into Vector's write/prune helpers instead of being fixed at definition time, and Vector and the Functions bootstrap share one content-digest helper.
This branch has not been deployed
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.
After a stack stop and reopen on the Docker runtime, a lazily woken Vector container intermittently exited with
Config file not found in path. path="/etc/supabase/vector-api.yaml". Health checks then failed with ECONNRESET.Vector's API and pipeline configs, and the Functions edge-runtime bootstrap, were rewritten on every start at a fixed bind-mounted path, by writing to a temporary directory and renaming over the target. On Docker engines with a shared-filesystem cache (reproduced with OrbStack), a container created right after can resolve that path to the replaced, now-deleted file. Mounting the directory instead of the file doesn't help: a busybox reproduction of the exact sequence, with a fixed name inside a directory mount, failed 12 times in 500.
Both recipes now write content-addressed files: each write lands at a name derived from a digest of its content, still via a temporary file and rename. The container mounts a static directory read-only, and only the name passed in
argsvaries. A fresh container therefore only ever opens a name that has never been replaced, and the same reproduction with content-addressed names failed 0 times in 500. Unchanged content resolves to the same name and isn't rewritten. Stale generations are pruned inargs, after any previous container has stopped, never inprepare, which can run while a restart's old container is still live. Container paths are always joined as POSIX.The other service recipes were checked; only Vector and the Functions bootstrap wrote a stack-owned file onto a path they also bind-mount.