feat(cli): add experimental stack start - #6506
Conversation
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@162cc57f457e10b0c33a2377b1cb2d78e9639b0ePreview package for commit |
There was a problem hiding this comment.
🤖 AI Review
Only Claude's independent review was available; the Codex review did not complete. All 11 reported findings were verified against the checked-out code and confirmed. No critical or major issues were found. The confirmed concerns cover error classification and guidance, unexported error classes, function environment/path translation, database release selection, dead code, and potentially flaky fixed test ports.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | apps/cli/src/commands/experimental/stack/start/start.handler.ts:220 |
error-handling |
claude | StackStateInvalidError and StackStateFormatUnsupportedError are declared start errors but are not explicitly classified, so telemetry records them as unknown failures. |
| 🟡 MINOR | apps/cli/src/commands/experimental/stack/start/start.handler.ts:207 |
error-handling |
claude | StackRuntimeError and StackCleanupError receive misleading stop-the-stack guidance and are classified as invalid configuration through the lifecycle reason. |
| 🟡 MINOR | apps/cli/src/commands/experimental/stack/stack-config.ts:14 |
conventions |
claude | LegacyStackConfigError and LegacyExperimentalStackTargetError are file-private despite the trusted CLI convention requiring new error classes under apps/cli/src to be exported. |
| 🟡 MINOR | apps/cli/src/commands/experimental/stack/stack-config.ts:76 |
error-handling |
claude | Function dotenv keys are copied without validation into a schema that only accepts uppercase environment names, causing otherwise parseable dotenv files to abort stack configuration with a generic schema error. |
| 🟡 MINOR | apps/cli/src/commands/experimental/stack/stack-config.ts:158 |
correctness |
claude | Translating a legal function path located under supabase/ but outside supabase/functions/ produces .. segments that the stack resolver rejects, silently making the function unavailable. |
| 🟡 MINOR | packages/stack/src/model/Compiler.ts:461 |
correctness |
claude | The database major-version fallback selects the first matching catalog key by insertion order rather than applying a deterministic newest-release policy. |
| 🟡 MINOR | apps/cli/src/commands/experimental/stack/stack-config.ts:151 |
correctness |
claude | Paths beginning with supabase/ are incorrectly treated as project-root-relative even though legacy config paths are resolved relative to the existing supabase/ directory. |
| ⚪ NIT | apps/cli/src/commands/experimental/stack/stack-config.ts:513 |
dead-code |
claude | legacyConfigInput accepts a projectRoot parameter that its implementation never uses. |
| ⚪ NIT | apps/cli/src/commands/experimental/stack/stack-config.ts:531 |
dead-code |
claude | The translated config hardcodes preparation: "background", but the production consumer always overwrites it with the command flag. |
| ⚪ NIT | packages/stack/src/supervisor/ingress.integration.test.ts:56 |
test-flakiness |
claude | The hash-derived fixed test ports occupy the normal Linux ephemeral-port range and are not reserved before use, allowing sporadic collisions. |
| ⚪ NIT | apps/cli/src/commands/experimental/stack/stack.shared.ts:76 |
style |
claude | The target resolver layer wraps a constant service value in Effect.succeed and Layer.effect even though Layer.succeed directly expresses the same construction. |
Stats
Claude findings: 11 · Codex findings: 0 · Confirmed: 11 · Refuted: 0 · 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.
|
/ai-review |
avallete
left a comment
There was a problem hiding this comment.
Approving. Dispatch, targeting, and error classification look good for an experimental first command.
Inline comments are follow-ups on config translation (Studio default URL, functions .env parsing, encrypted: secrets, SUPABASE_* overlays, Windows signing-key paths, telemetry flush). None of those should block landing this.
On the Depthfirst thread: db.network_restrictions / ssl_enforcement / vault are accepted by the stack schema but nothing in the stack runtime reads them today, same as local supabase start. Optional to forward for definition fidelity; not a current local access-control hole.
|
I merged While resolving this, a new commit (d81788e, "address experimental stack start review feedback") landed on this branch mid-resolution, so I merged that in too rather than force-pushing over it — its content is unchanged, just the same rename applied consistently (e.g. Verified: |
…upabase#6535) Restore existing configuration behavior in `supabase experimental stack start`: apply supported automatic `SUPABASE_*` overrides using the shared configuration resolvers, and decrypt dotenvx `encrypted:` secrets before passing them to the stack. Overrides retain shell and project dotenv precedence, optional-section semantics, and dynamic listener allocation when no port is explicitly supplied. The CLI applies overrides to the complete configuration and calls `validateCliConfig` from `@supabase/config/effect` before projecting the validated values into stack settings. Loading and revalidation share the package's existing rules; this adds no new validation rules. Consumed secrets are decrypted at the stack boundary. Decryption supports base, suffixed, and comma-separated private keys and reports configuration errors without exposing secret values. Follow-up to supabase#6506, implementing the requests in [encrypted-secret handling](supabase#6506 (comment)) and [automatic environment overrides](supabase#6506 (comment)). Update command help and side-effect documentation to describe the restored behavior. --------- Co-authored-by: Andrew Valleteau <avallete@users.noreply.github.com>
## TL;DR stops every cli command on macos from waiting on the automounter before it prints anything. ## whats broken? since supabase#6506 every command loads the stack code at startup, and msgpackr inside it probes for its native addon at the linux build runner's store path baked into the binary on macos that `/home/runner` path goes through the automounter on every run, and when the lookup stalls every invocation blocks in `open(2)` ## now fixed by: setting msgpackr's own `MSGPACKR_NATIVE_ACCELERATION_DISABLED` switch at build time for the release and `build:binary` binaries. the addon never ships in the binary, so msgpackr keeps its js path like it already did wherever that path is missing, and it no longer loads an addon from that path when one exists.. ## ref: - closes: supabase#6771 - broken in: supabase#6506
supabase experimental stack startcreates or resumes a managed stack for the current project and branch, with named-stack and explicit-ID targeting, Docker/native selection, and eager activation or background artifact preparation.Translate project configuration at the CLI boundary and embed the supervisor and native-process dispatch in the compiled binary. The stack package owns lifecycle and persistent state, and resolves PostgreSQL major versions through its release catalog.