Skip to content

fix(stack): keep Vector and Functions configs visible to fresh containers - #6945

Open
jgoux wants to merge 2 commits into
developfrom
fix/stack-vector-config-visibility
Open

jgoux wants to merge 2 commits into
developfrom
fix/stack-vector-config-visibility

Conversation

@jgoux

@jgoux jgoux commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

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 args varies. 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 in args, after any previous container has stopped, never in prepare, 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.

…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.
@jgoux
jgoux requested a review from a team as a code owner October 1, 2026 13:42
@jgoux jgoux self-assigned this Oct 1, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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.

Comment thread packages/stack/src/services/Functions.ts Outdated
Comment thread packages/stack/src/functions/FunctionsBootstrap.ts
Comment thread packages/stack/src/functions/FunctionsBootstrap.ts Outdated
Comment thread packages/stack/src/services/Vector.ts Outdated
Comment thread packages/stack/src/functions/FunctionsBootstrap.ts
Comment thread packages/stack/src/services/Vector.ts Outdated
Comment thread packages/stack/src/services/Vector.ts Outdated
…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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant