Conversation
…ects a config change Maps each incompatible plan path (endpoint port, db major version) to the config.toml key or SUPABASE_*_PORT env var that set it, shows the saved and requested values, and suggests reverting it or running the stack's exact destroy command with a data-loss warning.
…mplete Always suggest the exact --stack-id destroy command instead of an unquoted --stack <name> that re-resolves against the caller's current workdir, carry structured per-change records (service, path, key, saved, requested) plus the recreate command on the JSON/stream-json error envelope via MachineErrorContext, and collect every incompatible member into one error instead of reporting only the first, deduplicating shared API port lines. Use the production planSupabaseComposition planner in the start handler's integration tests instead of a hand-rolled approximation, and derive the port-setting table and createCreations' env-override calls from one shared definition per port to prevent drift.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🤖 AI Review
Verified all 10 supplied findings against the checked-out code and trusted conventions. Three overlapping pairs merge into seven confirmed findings: two minor error-guidance issues and five nits. Full PostgreSQL pins can produce identical displayed major versions despite an incompatibility, and artifact-version mismatches receive advice to revert a setting the CLI does not expose. No critical or major issue was established. Locations below use the actual new-side line numbers.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | apps/cli/src/commands/experimental/stack/start/start.handler.ts:229 |
error-messages |
claude | When saved and requested PostgreSQL pins differ within the same major version, the error displays identical major versions and suggests reverting a major_version setting that already matches. |
| 🟡 MINOR | apps/cli/src/commands/experimental/stack/start/start.handler.ts:328 |
error-handling |
claude+codex | Artifact-version mismatches receive instructions to revert a setting that has no config.toml key or environment override in the CLI. |
| ⚪ NIT | apps/cli/src/commands/experimental/stack/start/start.handler.ts:328 |
wording |
claude+codex | Multiple changed settings produce the grammatically incorrect suggestion 'Revert the settings above to its saved value', although the changes are joined inline. |
| ⚪ NIT | apps/cli/src/command-internal/stack-config.ts:173 |
documentation |
claude+codex | The PortSetting comment promises automatic consistency between creation wiring and endpoint-setting lookup that the implementation does not enforce. |
| ⚪ NIT | packages/stack/src/effect.ts:71 |
api-surface |
claude | The PR adds a planner re-export to the main Effect entrypoint for a new consumer that exists only in a CLI integration-test fake. |
| ⚪ NIT | apps/cli/src/commands/experimental/stack/start/start.handler.ts:299 |
code-quality |
claude | destroyCommandFor accepts an unused name property, and the same rejection constructs the destroy command separately for prose and structured output. |
| ⚪ NIT | apps/cli/src/commands/experimental/stack/start/start.handler.ts:177 |
comment-style |
codex | The new internal-helper comments narrate implementation mechanics prohibited by the trusted repository comment policy. |
Stats
Claude findings: 6 · Codex findings: 4 · Confirmed: 7 · 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.
…name-the-changed-setting-and-the-fix-when-a-saved-stack
…in the stack error Same-major Postgres version differences and catalog-pinned artifact versions have no config.toml key or env var, so the incompatible-stack error no longer suggests reverting major_version or an editable setting for them: it names the full saved and requested builds, drops the revert sentence when nothing is editable, and explains the fix in plain language. Mixed rejections keep the revert advice for the editable keys only, with singular/plural grammar matching their count. destroyCommandFor now takes only the stack id (it never read the name) and is computed once per rejection, reused for both the error suggestion and the JSON/stream-json envelope's recreate_command. Move planSupabaseComposition's export from the main @supabase/stack/effect entrypoint to @supabase/stack/testing, since the CLI only needs it to build a realistic test double for its own composition plan.
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
1 participant
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.
When
supabase start(with[experimental] stack) refuses a change to a saved stack, the error now says which setting changed and what to do about it. It used to report internal paths such asendpoints.httpwith a generic suggestion.config.tomlkey, or theSUPABASE_*environment variable that set it, with the saved and requested values. For example:[db] major_version: saved 17, requested 15, or[api] port: saved 54321, requested automatic.[api] portappears once.supabase stack destroy --stack-id <id>to recreate it. It warns that recreating deletes the local database data. The ID is used so the command targets this stack regardless of the current directory.stack_changeswith service, path, key, saved and requested values, plusrecreate_command), so agents don't have to parse prose.Port settings are now defined once in
stack-config.ts, shared by config translation and error attribution, so a new port can't be added to one and missed in the other. The tests use the real composition planner.