feat(stack): prepare slim resources on demand - #6250
Conversation
|
Automated review triage is complete for this batch. Fixed:
Deliberately deferred:
This is the final automated-fix pass for this PR. The remaining items need human product and architecture review rather than another mechanical review loop. |
|
Final automated-review disposition:
This is the final automated-review pass. The remaining recorded deferrals are unchanged; the PR is ready for human review. |
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@05b8067fb17b61a99cf8d6c0f93f48b227f08d29Preview package for commit |
There was a problem hiding this comment.
💡 Codex Review
cli/packages/stack/src/BinaryResolver.ts
Lines 726 to 731 in 05b8067
When two processes repair the same incomplete cache, both can fail the initial rename and observe the destination as incomplete; if one publishes after the other's check, this unconditional removal deletes the newly complete directory that the winner may already have returned to a starting service. That produces intermittent missing executables despite successful resolution. Use a cross-process lock or a replacement protocol that cannot remove a destination after another contender has published it.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const managedStackLaunchSchema = Schema.Union([ | ||
| Schema.Struct({ | ||
| mode: Schema.Literal("native"), | ||
| ...managedStackLaunchFields, |
There was a problem hiding this comment.
Continue decoding legacy managed launch records
Existing managed stack documents remain formatVersion: 1, but stacks created by the previous default contain launch.mode: "auto"; this narrowed union rejects those documents before the supervisor can select and persist a concrete runtime. After upgrading, normal start, status, stop, and delete operations therefore classify the user's existing stack as corrupt. Decode and migrate the prior auto representation before requiring the new concrete launch shape.
Useful? React with 👍 / 👎.
| if (yield* isCompleteCache(cacheDir, release, info)) { | ||
| return { | ||
| path: cacheDir, | ||
| downloaded: false, | ||
| } satisfies ResolveBinaryResult; |
There was a problem hiding this comment.
Revalidate host compatibility on cache hits
When cacheRoot is shared or copied between Linux hosts, this fast path returns an artifact validated against the downloading host's glibc rather than the current host's version. Because the completion marker stores no manifest floor and validateManifest only runs during extraction, a cache produced on newer glibc can bypass the new compatibility gate and fail only when the service is spawned. Persist the compatibility metadata and validate it for every reuse.
Useful? React with 👍 / 👎.
| /** Normalizes a version string to the catalog's canonical stored form. */ | ||
| export function normalizeServiceVersion(service: ServiceName, version: string): string { | ||
| const trimmed = version.trim(); | ||
| const prefix = IMAGE_TAG_PREFIX[service]; | ||
|
|
||
| if (prefix === "v") { | ||
| return trimmed.replace(/^v/i, ""); | ||
| } | ||
|
|
||
| if (prefix === undefined && DEFAULT_VERSIONS[service].startsWith("v")) { | ||
| return /^v/i.test(trimmed) ? `v${trimmed.slice(1)}` : `v${trimmed}`; | ||
| } | ||
|
|
||
| return trimmed; | ||
| const normalized = version.trim(); | ||
| const tagPrefix = serviceMetadata(service).artifact.docker.tagPrefix; | ||
| return tagPrefix !== undefined && normalized.startsWith(tagPrefix) | ||
| ? normalized.slice(tagPrefix.length) | ||
| : normalized; |
There was a problem hiding this comment.
Migrate legacy version strings before exact-tag resolution
Existing project-link snapshots, local override files, and pinned manifests were written using the previous canonical form, which removed the leading v from services such as PostgREST, Auth, and Edge Runtime. This normalizer now preserves those legacy values, while the new release and GHCR resolvers treat the stored value as the exact tag, so an existing postgrest: "16.1" becomes a request for postgrest-16.1 or :16.1 instead of the published v16.1 artifact. Migrate persisted legacy values before applying the exact-tag convention.
Useful? React with 👍 / 👎.
| const containerRuntime = | ||
| document.launch?.mode === "docker" ? (document.launch.containerRuntime ?? "docker") : null; | ||
| const acquisition = yield* manager.acquireControl(stackId); |
There was a problem hiding this comment.
Clean up legacy Docker stacks without launch metadata
For an existing managed Docker stack whose optional launch field is absent, this now infers containerRuntime as null, so every dead-owner and failed-stack stop path skips dockerForceRemove and then records the stack as stopped. Those documents were valid previously, when stop always attempted exact-name Docker cleanup, so upgrading after a daemon crash can leave containers running and ports occupied. Preserve best-effort Docker cleanup or migrate the runtime when legacy launch metadata is missing.
Useful? React with 👍 / 👎.
| const attempt = runPullCommand(spawner, runtime, image).pipe( | ||
| Effect.retry({ | ||
| while: (error) => shouldRetryPull(error.detail), | ||
| schedule: Schedule.recurs(1).pipe( | ||
| Schedule.addDelay(() => Effect.succeed(Duration.millis(500))), | ||
| ), |
There was a problem hiding this comment.
Bound image-pull retries by elapsed time
For transient registry failures, this retry policy allows exactly one retry regardless of how little wall-clock time has elapsed, so a brief pair of rate-limit, timeout, or connection-reset responses can fail stack preparation after roughly 500 ms. Use a wall-clock deadline with spaced retries rather than an attempt-count budget so loaded CI and slow user networks receive a stable recovery window.
AGENTS.md reference: AGENTS.md:L170-L172
Useful? React with 👍 / 👎.
Summary
Context
This keeps resource preparation, runtime selection, and activation inside @supabase/stack, with the CLI remaining a thin consumer. An omitted mode selects Docker when Docker or Podman is usable and otherwise selects native mode. Explicit native or Docker choices are strict, preparation never falls back to the other mode, and managed stacks pin their persisted selection once claimed, including after a failed startup. Changing modes requires deleting and recreating the stack and its managed data.
Stack creation stays side-effect free beyond runtime detection and port reservation. Eager resources are prepared at startup, while lazy resources are prepared through the same activation path used by proxy and programmatic callers. Concurrent work is coalesced, disposal completes waiters with typed failures before cancellation, and cached paths cannot start services after disposal begins. Download completion restores prior public state atomically without overwriting a newer lifecycle transition. Concurrent Functions and Edge Runtime reloads preserve each committed state.
Docker and Podman remain exclusive runtime selections. Vector consumes the selected runtime socket when one is usable (readable and writable); a socket-less host uses internal Vector logs instead of crash-looping the service, and Podman never implicitly adopts the Docker default socket.
The catalog uses the frozen service versions, including Postgres 17.6.1.163, and preserves exact published container tags. The canonical Postgres image starts through its published non-root entrypoint and database bootstrap is modeled as an observable one-shot dependency before consumers become ready. Docker-only services remain container-backed until they have a concrete native runtime consumer.