feat(cli): add experimental stack status - #6508
Conversation
e120ca7 to
1764f96
Compare
fa05922 to
34ca600
Compare
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@34ca600fdad2a3cbe8f166998f05d4b6fdbc4d6fPreview package for commit |
|
/ai-review |
34ca600 to
9ad64f6
Compare
|
On this one, can we integrate :
It was done here but I'm closing this PR as well and porting what was done for start/stop/destroy directly from develop. |
Database-only stacks previously failed credentials() with an InvalidStackConfigError. The api credentials are now optional and are omitted when the Auth capability is disabled.
Suppress the JSON serialization lint in the redaction assertions of the drift test and add ExperimentalStackStatusError to the error-tag fixture.
status --env exports connection variables as dotenv or a JSON variable map, with --override-name for application-specific names. Ordinary status does not reveal credentials. Database-only stacks export database credentials with API credentials omitted when Auth is disabled. An absent config.toml is compared against default settings, matching stack start; invalid configuration still reports drift as unavailable. The legacy -o env form is rejected with a pointer to --env.
supabase status follows [experimental] stack and SUPABASE_EXPERIMENTAL_STACK like start and stop, so a project on the new backend gets a status command that understands its stack.
output.success merges message into the JSON payload, so --env produced
{ DB_URL, message: "" }. Use output.result for the bare variable map and
assert the serialized shape in the integration test.
|
/ai-review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 AI Review
Verified all 11 reported findings against the PR code and trusted repository conventions. One duplicate was merged, yielding 10 findings: five minor and five nit-level. All are confirmed; no critical or major issues were found.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | apps/cli/src/commands/experimental/stack/status/status.errors.ts:16 |
telemetry |
claude | The runtime reason is incorrectly classified as an external-service network failure even when it represents user-actionable lifecycle or local encoding errors. |
| 🟡 MINOR | apps/cli/src/commands/experimental/stack/status/status.handler.ts:229 |
error-handling |
claude | Configuration comparison errors discard their precise typed diagnostic and inaccurately report that the project configuration could not be loaded. |
| 🟡 MINOR | apps/cli/docs/stack-commands.md:31 |
documentation |
claude | The migration guide mentions rejection of legacy -o env but omits that every explicit legacy -o/--output value, including json and pretty, is rejected by the stack backend. |
| 🟡 MINOR | apps/cli/src/commands/experimental/stack/status/status.env.ts:84 |
correctness |
codex | Text export rejects values containing both a single quote and a backtick even when they can be represented losslessly with dotenv double quotes. |
| 🟡 MINOR | apps/cli/src/commands/experimental/stack/status/status.env.ts:39 |
correctness |
codex | Override collision validation includes optional variables that will not be exported, rejecting non-colliding renames on capability-limited stacks. |
| ⚪ NIT | packages/config/src/experimental.ts:44 |
documentation |
claude | The generated public configuration schemas retain the old description that says the stack backend affects only start and stop. |
| ⚪ NIT | apps/cli/src/commands/experimental/stack/status/status.env.ts:37 |
validation |
claude | Conflicting repeated overrides for the same source variable are silently resolved last-wins. |
| ⚪ NIT | apps/cli/src/commands/experimental/stack/status/status.handler.ts:213 |
code-quality |
claude | The environment-export branch returns an unrelated optional inspection whose presence depends solely on whether --stack-id was used. |
| ⚪ NIT | apps/cli/src/commands/experimental/stack/status/status.handler.ts:226 |
performance |
claude | Status with --stack-id and a loadable configuration inspects the stack twice, repeating the owner status RPC. |
| ⚪ NIT | apps/cli/src/commands/experimental/stack/status/SIDE_EFFECTS.md:49 |
documentation |
claude+codex | The side-effect contract omits the command's exit-code behavior and telemetry-state write. |
Stats
Claude findings: 8 · Codex findings: 3 · Confirmed: 10 · Refuted: 0 · Uncertain: 0
Models: claude-opus-5 + gpt-5.6-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
| }> { | ||
| get [ErrorActionabilityId](): CliErrorActionabilityDeclaration { | ||
| if (this.reason === "flags" || this.reason === "not-found") return actionability.provideFlags; | ||
| return this.reason === "runtime" ? actionability.externalNetwork : actionability.invalidConfig; |
There was a problem hiding this comment.
🟡 MINOR · telemetry · source: claude
The runtime reason is incorrectly classified as an external-service network failure even when it represents user-actionable lifecycle or local encoding errors.
Evidence: status.errors.ts:16 maps every runtime error to actionability.externalNetwork. status.handler.ts:203-208 uses that reason when the stack is stopped, and status.env.ts:85-90 uses it for dotenv encoding failures. error-actionability.ts:355-360 classifies the preset as ExternalService/Network.
Suggested fix: Split lifecycle, encoding, and genuine runtime failures into distinct reasons and map the user-actionable cases to appropriate non-network actionability presets.
| InvalidStackConfigError: () => | ||
| Effect.succeed({ inspection: undefined, warning: configUnavailableWarning }), | ||
| StackVersionUnsupportedError: () => | ||
| Effect.succeed({ inspection: undefined, warning: configUnavailableWarning }), |
There was a problem hiding this comment.
🟡 MINOR · error-handling · source: claude
Configuration comparison errors discard their precise typed diagnostic and inaccurately report that the project configuration could not be loaded.
Evidence: status.handler.ts:215-219 uses configUnavailableWarning for actual loading failures, while lines 228-233 reuse the same warning for InvalidStackConfigError and StackVersionUnsupportedError. Compiler.ts:468-472 supplies useful diagnostics such as Unsupported database version: ....
Suggested fix: Use a distinct, safely sanitized warning for comparison rejection that preserves the typed error's actionable diagnostic.
| accepts repeated or comma-separated `EXPORTED_VARIABLE=NAME` entries, requires `--env`, and rejects | ||
| unknown variables, invalid names, and collisions. API credentials are omitted when Auth is disabled. | ||
|
|
||
| The legacy `supabase status -o env` form is rejected on the stack backend; use `--env` instead. |
There was a problem hiding this comment.
🟡 MINOR · documentation · source: claude
The migration guide mentions rejection of legacy -o env but omits that every explicit legacy -o/--output value, including json and pretty, is rejected by the stack backend.
Evidence: stack-commands.md:31 discusses only -o env, whereas stack.shared.ts:109-121 rejects any populated legacy output flag and status.integration.test.ts:428-436 explicitly verifies rejection of json.
Suggested fix: Document that the entire legacy -o/--output flag is unsupported and list the corresponding --output-format replacements.
| Schema.Boolean.annotate({ | ||
| description: "Use the new local stack backend for top-level start and stop commands.", | ||
| description: | ||
| "Use the new local stack backend for top-level start, stop, and status commands.", |
There was a problem hiding this comment.
⚪ NIT · documentation · source: claude
The generated public configuration schemas retain the old description that says the stack backend affects only start and stop.
Evidence: experimental.ts:43-44 includes status, but apps/docs/public/cli/config.schema.json:2409 and :4916 and project-config.schema.json:1947 still say top-level start and stop commands. Trusted CLI conventions explicitly require generated schemas to be updated with experimental registration changes.
Suggested fix: Regenerate and commit the public CLI configuration schemas.
| message: | ||
| "--override-name must be EXPORTED_VARIABLE=VALID_ENV_NAME; for example API_URL=NEXT_PUBLIC_SUPABASE_URL.", | ||
| }); | ||
| names.set(source, target); |
There was a problem hiding this comment.
⚪ NIT · validation · source: claude
Conflicting repeated overrides for the same source variable are silently resolved last-wins.
Evidence: status.env.ts:23-38 unconditionally replaces the map entry for each source. The later uniqueness check only checks final destination values, so API_URL=FIRST followed by API_URL=SECOND succeeds and discards FIRST.
Suggested fix: Reject an override when its source variable has already appeared in the supplied entries.
| const values = stackEnvValues(status, credentials, envNames); | ||
| if (output.format === "text") yield* output.raw(yield* encodeStackEnv(values)); | ||
| else yield* output.result(values); | ||
| return target.inspection; |
There was a problem hiding this comment.
⚪ NIT · code-quality · source: claude
The environment-export branch returns an unrelated optional inspection whose presence depends solely on whether --stack-id was used.
Evidence: status.handler.ts:213 returns target.inspection; findDescriptor populates that field only in the explicit-id branch at lines 141-149. The command wrapper at status.command.ts:43-45 does not consume the success value.
Suggested fix: Return void or the exported values from the environment branch and narrow the handler's success type accordingly.
| ? target.inspection === undefined | ||
| ? yield* catchStackError(api.inspectStack(target.id)).pipe(Effect.map(comparedInspection)) | ||
| : { inspection: target.inspection } | ||
| : yield* api.inspectStack(target.id, { config: loaded.config }).pipe( |
There was a problem hiding this comment.
⚪ NIT · performance · source: claude
Status with --stack-id and a loadable configuration inspects the stack twice, repeating the owner status RPC.
Evidence: findDescriptor calls inspectStack at status.handler.ts:143, then the comparison calls it again at line 226. status.integration.test.ts:230-239 asserts two inspections. EffectStack.ts:1254-1262 performs an owner rpc.status during each live-owner inspection.
Suggested fix: Expose a lightweight persisted-descriptor lookup or otherwise arrange for target-root resolution and config comparison to require only one full inspection.
|
|
||
| Ordinary status (without `--env`) never opens a stack handle and never emits | ||
| credentials, regardless of the stack's lifecycle. A stopped stack or a | ||
| credentials failure with `--env` fails the command without emitting output. |
There was a problem hiding this comment.
⚪ NIT · documentation · source: claude+codex
The side-effect contract omits the command's exit-code behavior and telemetry-state write.
Evidence: SIDE_EFFECTS.md ends at line 49 without exit-code or telemetry persistence sections. status.handler.ts:245 ensures telemetryState.flush on success and failure. The trusted SIDE_EFFECTS template requires exit paths and telemetry events, and sibling stack contracts document the telemetry file.
Suggested fix: Document all exit paths, command telemetry, and the telemetry-state flush to <SUPABASE_HOME or ~/.supabase>/telemetry.json.
| const quote = ["'", "`"].find((candidate) => !value.includes(candidate)); | ||
| if (quote === undefined || value.includes("\r")) | ||
| return Effect.fail( | ||
| new StackCommandStatusError({ | ||
| reason: "runtime", | ||
| message: | ||
| "A credential cannot be represented losslessly as dotenv. Use --env --output-format json.", |
There was a problem hiding this comment.
🟡 MINOR · correctness · source: codex
Text export rejects values containing both a single quote and a backtick even when they can be represented losslessly with dotenv double quotes.
Evidence: status.env.ts:84 considers only single-quote and backtick delimiters, then lines 85-90 fail when both occur. The test's example both'andquotes` contains neither a double quote nor a double-quote escape sequence and is valid as a double-quoted dotenv value.
Suggested fix: Add a double-quoted encoding path when it round-trips losslessly through dotenv, retaining rejection only for genuinely unrepresentable values.
| if (new Set(names.values()).size !== names.size) | ||
| return yield* new StackCommandStatusError({ | ||
| reason: "flags", | ||
| message: "--override-name produces duplicate environment variable names.", |
There was a problem hiding this comment.
🟡 MINOR · correctness · source: codex
Override collision validation includes optional variables that will not be exported, rejecting non-colliding renames on capability-limited stacks.
Evidence: status.env.ts:22 initializes mappings for all canonical variables and lines 39-43 validate uniqueness before status or credentials are known. Lines 54-72 later omit API, endpoint, and storage variables when unavailable, so a target collision with one of those absent variables cannot affect the actual output.
Suggested fix: Validate destination collisions against the source variables actually present after status and credentials have been loaded.
3 participants
supabase stack statusreports the selected stack's identity, runtime, owner, readiness, endpoints, and configuration drift. It loads configuration from the selected stack's project, including when inspecting an ID from another working directory.Adds a read-only configuration comparison to the stack package that reports changed paths without exposing secret values. Invalid or unreadable project configuration leaves inspection available and marks drift as unavailable. An absent
config.tomlis compared against default settings, matching whatstack startdoes (develop's project loader treats an absent file as defaults, not an error).status --envPorted from #6517 (closed):
status --envexports connection variables as dotenv (text) or a JSON variable map (--output-format json/stream-json). Ordinary status never reveals credentials.--override-name API_URL=NEXT_PUBLIC_SUPABASE_URLrenames exported variables (CSV or repeated; requires--env; rejects unknown sources, invalid names, and duplicate destinations).stack.credentials()in@supabase/stacknow returns an optionalapiinstead of failing when Auth is disabled.supabase status -o envform is rejected with a pointer to--env.Feature-flag routing
Top-level
supabase statusnow follows[experimental] stack = true/SUPABASE_EXPERIMENTAL_STACKexactly likestartandstop, so a project on the new backend gets a status command that understands its stack.supabase stack statusalways uses the new backend.Also in this PR
develop(resolvedEffectStack.tsconflict from refactor(stack): simplify stack identity #6531'sdescriptor(state, id)change).lint:effect:checkwarnings in the drift test, the error-tag telemetry fixture, and the routing test's hardcodedstacksubcommand list.