Repository navigation
fix(cli): recover start and db start from a stopped database container - #7369
Merged
Coly010 merged 4 commits intoOct 29, 2026
Conversation
…LI-2876) A start that died between creating and starting the database container left it in Docker's `created` state, and every later start reported "already running" before failing with "container is not running". - recover a `created` database container like an exited one, keeping volumes - fail a non-running, unrecoverable database container with an honest not-running error and a `supabase stop` suggestion - probe a reused volume for `PG_VERSION` before the container starts, and run the first-run setup when Postgres never initialised it - serialise `start` and `db start` per project with a lock under `$SUPABASE_HOME/locks`, so a failing start cannot prune a running one - log a missing profile file as a plain debug line Fixes #7344
![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 six findings against the checked-out code and trusted conventions, merging the duplicate data-safety reports into one entry. Five findings remain: a major data-loss risk, three minor cleanup/convention/test-quality concerns, and one design nit. The design nit does not currently misclassify either caller.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | apps/cli/src/command-internal/db-bootstrap/container-lifecycle.ts:651 |
data-safety |
claude+codex | The unrestricted missing-file pattern can misclassify a populated database volume as uninitialized after an infrastructure error, enabling fresh-volume setup and deletion during subsequent rollback. |
| 🟡 MINOR | apps/cli/src/command-internal/db-bootstrap/start-database.ts:201 |
error-handling |
codex | A reused volume found uninitialized remains marked non-fresh when docker start fails, so rollback leaves it behind despite the documented cleanup behavior. |
| 🟡 MINOR | apps/cli/src/command-internal/local-stack-start-lock.ts:20 |
telemetry |
codex | LocalStackStartBusyError is not exported, contrary to the trusted CLI error-classification convention. |
| 🟡 MINOR | apps/cli/src/commands/db/start/start.integration.test.ts:904 |
test-quality |
codex | The new rollback cases incur real readiness retry delays although their assertions concern failure and volume cleanup, not elapsed time. |
| ⚪ NIT | apps/cli/src/command-internal/status-errors.ts:57 |
design |
claude | StatusDbNotRunningError uses the presence of suggestion text as an implicit discriminator between startStack and stopStack actionability, coupling presentation to telemetry semantics. |
Stats
Claude findings: 2 · Codex findings: 4 · Confirmed: 5 · 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.
`db start` and the declarative generate/sync auto-start only checked that the database container existed, so a `created` or `exited` container was reported as already running while Postgres was down. The probe now reads the container state; a stopped database container is removed (volume kept) and brought up again, or fails with the not-running error in Bitbucket Pipelines. `db reset` and `db diff` keep the existence check.
Coly010
changed the title
jgoux
requested changes
Oct 29, 2026
jgoux
left a comment
jgoux
left a comment
Contributor
There was a problem hiding this comment.
Recovering stopped database containers fixes the failure from the issue, and the start lock is a welcome addition. Two changes requested: narrow the PG_VERSION-missing match so a connection error can't be read as an uninitialised volume, and keep db start's failure rollback from removing the project's other running containers.
…tart Only Docker's, Podman's and older Docker clients' diagnostics for a missing PG_VERSION count as an uninitialised data directory, so an unrelated missing-file error such as a socket dial failure never runs setup over a populated volume. Freshness is published from the pre-start hook, so a docker start failure after an uninitialised probe still prunes the volume. Also exports LocalStackStartBusyError and gives the db start rollback tests a zero health budget.
db start only creates the db container, its volume and the project network, so a failed run now removes just the db container, the db volume when it was fresh, and unused project networks. A stopped stack's other services and volumes are no longer stopped or pruned when bring-up fails after recovering the database. A stopped db container with --from-backup is refused before the container is removed, since its volume always exists.
jgoux
approved these changes
Oct 29, 2026
jgoux
left a comment
jgoux
left a comment
Contributor
There was a problem hiding this comment.
Both earlier findings are resolved: the db start rollback is now scoped to the database container and its fresh volume, and backup restore no longer removes a stopped container. Looks good.
Coly010
deleted the
columferry/cli-2876-windows-supabase-cli-start-fails-when-no-supabase-containers
branch
October 29, 2026 12:35
AmitKulkarni23
pushed a commit
to AmitKulkarni23/supabase_cli
that referenced
this pull request
Nov 1, 2026
…base#7372) Addresses supabase#7339 ## Problem Between `start` creating a project's Docker network and attaching its first container, the network has no endpoints. Any project-label-filtered `docker network prune` that lands in that window removes it, and `start` then fails with `network supabase_network_<id> not found`. Within one invocation every prune runs before bring-up (stopped-stack recovery) or after it stops (rollback), so the prune in the report most likely came from an overlapping lifecycle command on the same project: an earlier `start` still rolling back, a `stop`, `stop --all`, or a `db start` rollback. ## Change - `start`, `db start` (including the declarative auto-start), and `stop` hold a cross-process lifecycle lock under `$SUPABASE_HOME/locks`: - a per-project lock keyed by the project's Docker label, so checkouts sharing a `project_id` serialize and different projects still run in parallel; - a gate every per-project command holds shared and `stop --all` holds exclusively. - The lock is taken before the first Docker probe and released after any rollback, outside the rollback region, so a waiting command never rolls back another command's resources. - A blocked command prints one waiting line to stderr and fails with an actionable error after 10 minutes; waiting is interruptible. - The lock replaces the per-project start lock from supabase#7369, which it covers. - `@supabase/stack` gains `holdSharedLock` / `holdExclusiveLock` next to `holdLock`. - The `ensureNetwork` doc comment no longer claims the network cannot be removed mid-run. ## Limitations Older CLI binaries, raw `docker` commands, a different `SUPABASE_HOME`, and projects sharing a custom `--network-id` do not participate, and `stop --all` is not queued ahead of a stream of single-project commands (documented in `start`'s `SIDE_EFFECTS.md`). `db reset` and the shadow database still create their network without the lock; that is a separate follow-up. When `$SUPABASE_HOME/locks` cannot be created or a lock file there cannot be opened, these commands continue without the lock and log a debug line. --------- Co-authored-by: 7ttp <117663341+7ttp@users.noreply.github.com>
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
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.
Summary
If a
supabase startdies after Docker creates the database container but before it starts it, the container is left in Docker'screatedstate. That can happen if the process is killed, a port is already taken, or a secondstartrolls back the first one's containers. Everysupabase startafter that printedsupabase start is already running.and then failed withcontainer is not running: created. The way out was to remove the container by hand, and nothing pointed you there.Looking into it turned up three more problems along the same path:
startdecided whether to run the first-run setup (schema, roles, migrations, seed) purely on whether the database volume already existed. A leftover volume that Postgres never initialised still exists, so aftersupabase stopandsupabase startthe setup was skipped and the stack came up withoutsupabase_migrationsor any of the project's migrations.createdcontainer above.db startonly checked that the database container existed. For a stopped or never-started one it printedPostgres database is already running.and exited 0 with Postgres down. The local database auto-start indb schema declarative generateandsyncused the same check, skipped the start and then failed to connect.This fixes all of them.
What changes
startrecovers acreateddatabase container the same way it already recovers anexitedone. It removes the project's containers and network, keeps the volumes, and brings the stack up again.<container> container is not running: <state>and suggestssupabase stop. It no longer printssupabase start is already running.first.startanddb startcheck whether a reused volume was ever initialised. Betweendocker createanddocker startthey rundocker cp <container>:/var/lib/postgresql/data/PG_VERSION -, which reads through the volume mount while the container is still stopped. The Postgres image runsinitdbwhenPG_VERSIONis missing, so a missing file means the volume is uninitialised. In that case the first-run setup runs and bucket seeding happens as on a fresh volume, and a failed bring-up prunes the volume like it does for a fresh one. Only Docker's or Podman's own message for a missingPG_VERSIONcounts as uninitialised. Any other outcome, including a connection error, is treated as initialised, so an ambiguous probe never runs setup over real data.startanddb starttake a per-project lock at$SUPABASE_HOME/locks/start-<project_id>.lock. It's held through bring-up and rollback. A second start on the same project printsWaiting for another supabase start for this project to finish...and waits for the first, up to 10 minutes, before failing withLocalStackStartBusyError. If the lock file can't be created, the start carries on without it rather than failing.stopdoesn't take the lock.db startand the declarative auto-start now read the container's state as well as whether it exists. A database container that isn't running is removed on its own withdocker container rm -f, keeping the volume, and brought up again. ThePG_VERSIONcheck above decides whether setup runs. The project's other containers are left alone, sincedb startonly manages the database. In Bitbucket Pipelines they fail with the same not-running error asstartinstead.db resetanddb diffstill only check that the container exists, sincedb resetrecreates it anyway.db startfails, it now rolls back only what it creates: the database container, the database volume when this run created it or found it uninitialised, and any project network nothing else is using. It used to remove every container in the project and, on a fresh volume, prune every unused project volume, so a failed recovery could take down the rest of a running stack. A stopped database with--from-backupis refused before anything is removed.--debug, a missing profile file now logsNo profile file at <path>; using the default profile.instead of the raw file-not-found error. That line was the only thing in the original report that looked like an error, and it wasn't one.Not covered
startis killed after Postgres has started runninginitdbbut before the CLI's setup finishes, the volume hasPG_VERSION, so the next start treats it as initialised and skips setup. That's the same as before this change. Catching it would need a marker written once setup completes. I'd rather not guess at that from schema contents, since a project with no migrations looks the same as an unfinished one.Linked issue
Closes #7344 (CLI-2876)