Repository navigation
fix(stack): stop Postgres containers with a fast shutdown - #6834
Merged
Merged
Conversation
Docker stops the database container with SIGTERM by default, which is PostgreSQL's smart shutdown and waits for every client to disconnect. The container spec gains a stop signal, and the database recipe stops PostgreSQL with SIGINT (fast shutdown). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Sep 25, 2026
jgoux
marked this pull request as ready for review
September 25, 2026 21:39
![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
The stop-signal implementation appears sound. The database-specific SIGINT setting lacks a regression test. The reported container leak on ordinary test failures is refuted by the runtime’s scope finalizer.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| ⚪ NIT | packages/stack/src/services/Database.ts:816 |
test-coverage |
claude | No test checks that database containers are configured to stop with SIGINT, so removing this setting could go unnoticed. |
Refuted findings (kept for transparency, not posted as review comments)
packages/stack/src/runtime/Container.integration.test.ts:198(test-cleanup): The test leaves its container behind if readiness, stopping, or the exit-code assertion fails.
Refuted: Explicit removal is not the only cleanup path. The runtime registers stop-and-remove cleanup with the launch scope, so an earlier test failure does not by itself leave the container behind. The log observer is forked as a child fiber at Container.integration.test.ts:1066-1074.
Stats
Claude findings: 1 · Codex findings: 1 · Confirmed: 1 · Refuted: 1 · Uncertain: 0
Models: claude-opus-5-5 + gpt-6-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
- Test that a Docker database stop sends SIGINT and PostgreSQL exits cleanly while a client connection stays open. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
AI review triage Fixed in 421c289:
Every review thread on this PR has been answered and resolved. 🤖 Addressed by Claude Code |
avallete
approved these changes
Sep 28, 2026
pull Bot
pushed a commit
to chizee/cli
that referenced
this pull request
Sep 28, 2026
…abase#6835) ## Problem Several failure modes in the local runtime left stacks stuck or unreachable: - Health was checked once per launch and the failure was cached. One slow first boot left a running service that returned 502 until it was restarted. - Native services reserved a port by binding and releasing it, then launched on it. Only Pooler recovered from the resulting collision. - Every saved stack's port claims blocked other stacks, running or not. Switching git branch, or opening a second worktree, failed on the classic fixed ports until the other stack was destroyed. - One unreadable `state.json` in the state root blocked port allocation and owner startup for every stack. - `docker create` had no timeout and held the service's lifecycle gate while it ran. ## Change - Readiness is re-checked on demand. Callers share one bounded probe per launch, each session declares its own probe, and a stale probe can never mark a newer launch healthy. - A native launch that exits before becoming ready is relaunched on fresh ports, but only when its port is actually taken by another process. - Saved claims only steer automatic allocation; for fixed ports, the bind decides. A pre-bind probe runs where the OS lets loopback and wildcard binds overlap. - State listings skip unreadable entries and report them. `stop --all` warns about skipped entries. - A stack member counts as started only when it is running and healthy. - `docker create` is bounded, and the named container is removed on timeout or interrupt. - Unmarked database data is rejected at startup, and destroy still removes it. ## Stack Part 2 of 8 of the stack package simplification, based on supabase#6834. Review and merge in order: 1. supabase#6834 fix(stack): stop Postgres containers with a fast shutdown 2. supabase#6835 fix(stack): harden readiness, port claims, and container storage ← this PR 3. supabase#6836 refactor(stack): flatten the owner process layers 4. supabase#6837 refactor(stack): unify the database snapshot protocol across engines 5. supabase#6838 feat(stack): lease owners with a lock and bind session stacks to creators 6. supabase#6839 feat(stack): ship the functions bootstrap with the package 7. supabase#6840 refactor(stack): own composition policy and stack lookup in the package 8. supabase#6841 feat(stack): add a testing entrypoint and derive the Promise API --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
pull Bot
pushed a commit
to chizee/cli
that referenced
this pull request
Sep 28, 2026
## Problem The owner process had accumulated parallel structures: - RPC handlers that only forwarded to `Owner`, with errors rewrapped through three types. - Seven maps keyed by instance id, and the composition held in two places. - Service-specific routes and credential rules inside `Owner.ts`. Definition changes ran on the caller's fiber. A client disconnecting mid-`createService` could leave an instance in `state.json` that the live owner didn't know about, or leak a network namespace so that the next destroy failed. ## Change - `Owner` builds the RPC handlers directly, with one error mapping. `OwnerError`, the mirrored interface and the pass-through handlers are gone. - One instance registry and one composition, both owned by the orchestrator. - Credential rules move to `host/Credentials` and shared routes to `host/Endpoints`. - Definition changes run in the owner scope: - A disconnected caller ends only its own wait. - Shutdown waits for changes in flight and rejects new ones. - Config-changing restarts go through the same gate. - A failed creation rolls back credentials it introduced. - One container sweep path, one snapshot/reset helper, and typed saved state with unique instance ids. - RPC error messages are always strings. - Unused orchestrator operations and RPCs are deleted. ## Stack Part 3 of 8 of the stack package simplification, based on supabase#6835. Review and merge in order: 1. supabase#6834 fix(stack): stop Postgres containers with a fast shutdown 2. supabase#6835 fix(stack): harden readiness, port claims, and container storage 3. supabase#6836 refactor(stack): flatten the owner process layers ← this PR 4. supabase#6837 refactor(stack): unify the database snapshot protocol across engines 5. supabase#6838 feat(stack): lease owners with a lock and bind session stacks to creators 6. supabase#6839 feat(stack): ship the functions bootstrap with the package 7. supabase#6840 refactor(stack): own composition policy and stack lookup in the package 8. supabase#6841 feat(stack): add a testing entrypoint and derive the Promise API --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Sep 28, 2026
…ge (#6840) ## Problem The CLI re-derived policy the package owns: - It hand-copied the composition's binding table and the eager/lazy rules. - It passed placeholder URLs for inputs the composition overwrites. - It found stacks by listing every stack and matching identity fields. - It ran database initialisation in four places. Starting a stopped stack with changed configuration was rejected with a suggestion to destroy it. ## Change - `find` reads one stack by identity or id. CLI target resolution uses it, so an unreadable state file surfaces as an error instead of "not found". - Composition-bound inputs are optional. Required inputs are validated before a service stops, so an invalid restart leaves it running. Placeholder URLs are gone. - `composition.plan` reports, per member, whether a requested configuration is unchanged, changed or incompatible. An `eager` option replaces the CLI's copy of the activation policy, and `stack status` reports drift from the plan. - Starting a stack whose owner is not running applies changed configuration. Only incompatible changes are rejected: endpoints, top-level versions and the database major version. - One stack database initialisation routine is shared by `stack start`, `db start`, `db reset` and schema shadows. - The internal artifact entrypoints are merged, the auth mapper's pass-throughs are type-checked, and the composition `identity` option is renamed to `keys`. ## Stack Part 7 of 8 of the stack package simplification, based on #6839. Review and merge in order: 1. #6834 fix(stack): stop Postgres containers with a fast shutdown 2. #6835 fix(stack): harden readiness, port claims, and container storage 3. #6836 refactor(stack): flatten the owner process layers 4. #6837 refactor(stack): unify the database snapshot protocol across engines 5. #6838 feat(stack): lease owners with a lock and bind session stacks to creators 6. #6839 feat(stack): ship the functions bootstrap with the package 7. #6840 refactor(stack): own composition policy and stack lookup in the package ← this PR 8. #6841 feat(stack): add a testing entrypoint and derive the Promise API --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
pull Bot
pushed a commit
to oogalieboogalie/cli
that referenced
this pull request
Sep 28, 2026
…upabase#6837) ## Problem The snapshot protocol existed twice: in TypeScript for native data, and in an embedded shell script for Docker volumes. The protocol covers staging, retiring the old copy, publishing, rollback, retention and the descriptor check. Every fix had to land in both copies. The shell copy also depended, unchecked, on binaries from an externally built image. ## Change - One protocol module owns naming, staging, rollback, retention, descriptor checks and ready-marker publication. - The native engine runs its steps on the host. - The Docker engine compiles each operation into one helper script, so each operation is still a single exec. - Both engines hold a host-side lock and an in-script lock. Retired generations are recovered, and data and its ready marker are published atomically. - A restore creates the data directory's parent, so a fresh Docker instance can restore before its first start (the warm `db diff` shadow path). - Uses of one Docker helper run concurrently, and closes are guarded by a per-open generation. - The unreachable recursive fallback copy is deleted. - One contract suite runs the same scenarios against both engines. - Runtime session wrapping is shared between process recipes and the database, and old artifact-cache temp entries are reaped. ## Stack Part 4 of 8 of the stack package simplification, based on supabase#6836. Review and merge in order: 1. supabase#6834 fix(stack): stop Postgres containers with a fast shutdown 2. supabase#6835 fix(stack): harden readiness, port claims, and container storage 3. supabase#6836 refactor(stack): flatten the owner process layers 4. supabase#6837 refactor(stack): unify the database snapshot protocol across engines ← this PR 5. supabase#6838 feat(stack): lease owners with a lock and bind session stacks to creators 6. supabase#6839 feat(stack): ship the functions bootstrap with the package 7. supabase#6840 refactor(stack): own composition policy and stack lookup in the package 8. supabase#6841 feat(stack): add a testing entrypoint and derive the Promise API --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
pull Bot
pushed a commit
to oogalieboogalie/cli
that referenced
this pull request
Sep 28, 2026
…tors (supabase#6838) ## Problem The owner's exclusivity came from binding a sticky control port. - A foreign listener on that port wedged the stack, including `destroy`. - The owner never noticed its creator dying, so throwaway stacks (CLI schema shadows, test stacks) leaked the owner, containers and port claims after a SIGKILL, crash or OOM. - Nothing checked that a client and a running owner came from compatible releases. - Owner output was discarded. ## Change - Each owner holds a per-stack SQLite lease for its lifetime. It publishes an ephemeral loopback endpoint and a per-owner bearer secret in `owner.json`, with mode 0600. - Liveness is a no-wait lease probe, so dead stacks cost no network timeouts. - The lease is re-validated against the path it locked. - The handshake compares a release derived from the stack sources and the Effect version. Builds and source runs compute the same value. `stop` and `destroy` go through a release-stable endpoint. - `lifetime: "session"` binds a stack to its creator through stdin; the owner destroys the stack when the creator exits. CLI schema shadows use it. - The owner sweeps dead owners' containers and session stacks in the same state root, holding each lease while it does. - Owner output goes to `owner.log`, and start failures include its tail. - Clients cache the owner connection per handle and drop it only when the owner is gone. Calls release their borrows in their own scope, and waiting for an owner can be interrupted. - Stack errors carry `owner-unavailable` or `release-mismatch`, so the CLI only treats a missing owner as "not running". - The owner exits explicitly once shutdown has flushed its output. - supabase#6825's guarantees are kept in the new model: - The owner registers detached stacks under its lease, as it already did session stacks, and removes the registration if its startup fails. A failed first launch therefore leaves nothing behind. - When no owner can start because the container engine is unavailable, `destroy` removes the stack offline while holding its lease, and reports the skipped runtime cleanup. Stacks registered by develop builds from before this PR don't decode with it, because persisted stack state has no migrations and the package is unreleased. Destroy them with the build that created them, or remove `~/.supabase/stacks/<id>` and the containers labelled `com.supabase.stack=<id>`. Known gap: if an owner is SIGKILLed right after registering a stack, its registration is left behind. ## Stack Part 5 of 8 of the stack package simplification, based on supabase#6837. Review and merge in order: 1. supabase#6834 fix(stack): stop Postgres containers with a fast shutdown 2. supabase#6835 fix(stack): harden readiness, port claims, and container storage 3. supabase#6836 refactor(stack): flatten the owner process layers 4. supabase#6837 refactor(stack): unify the database snapshot protocol across engines 5. supabase#6838 feat(stack): lease owners with a lock and bind session stacks to creators ← this PR 6. supabase#6839 feat(stack): ship the functions bootstrap with the package 7. supabase#6840 refactor(stack): own composition policy and stack lookup in the package 8. supabase#6841 feat(stack): add a testing entrypoint and derive the Promise API --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
pull Bot
pushed a commit
to oogalieboogalie/cli
that referenced
this pull request
Sep 28, 2026
…6839) ## Problem The Functions recipe required every caller to pass the bundled edge-runtime main-service source. The CLI's build, the CLI's stack runtime and the package's tests each bundled the package's own `serve.main.ts` with esbuild. The full bundle was also persisted in every saved Functions creation. ## Change - Generate the bundle into a committed module with a drift test, wired into `pnpm generate`. The bundle is self-contained, so it works offline. - Make the Functions `bootstrap` input optional; the package default is never persisted. - Delete the CLI's stack bootstrap bundler, its build define, and the internal serve-main export. The legacy backend's bootstrap is untouched. ## Stack Part 6 of 8 of the stack package simplification, based on supabase#6838. Review and merge in order: 1. supabase#6834 fix(stack): stop Postgres containers with a fast shutdown 2. supabase#6835 fix(stack): harden readiness, port claims, and container storage 3. supabase#6836 refactor(stack): flatten the owner process layers 4. supabase#6837 refactor(stack): unify the database snapshot protocol across engines 5. supabase#6838 feat(stack): lease owners with a lock and bind session stacks to creators 6. supabase#6839 feat(stack): ship the functions bootstrap with the package ← this PR 7. supabase#6840 refactor(stack): own composition policy and stack lookup in the package 8. supabase#6841 feat(stack): add a testing entrypoint and derive the Promise API --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
pull Bot
pushed a commit
to chizee/cli
that referenced
this pull request
Sep 29, 2026
…abase#6841) ## Problem The package had no supported way to use a stack from tests. Test authors picked state, cache and project roots themselves, wrote service creations by hand and nested try/finally teardown, and CLI tests imported package test helpers by relative path. The Promise API was also a hand-written copy of the Effect API, and it had already drifted from it. ## Change - `@supabase/stack/testing`: - `createTestStack` works with `await using`, returns typed service handles, and provides `checkpoint` and `reset`. - `makeTestStack` is the scoped Effect variant. - Test stacks use the session lifetime, eager composition, automatic ports, and per-user shared roots. - Checkpoints are instance-scoped snapshots. Parallel stacks cannot evict them, and destroy removes them. `reset` never wipes data for a missing checkpoint, and it always brings the composition back. - The Promise client is derived from the Effect handles, with type-level parity tests. Only operations that return handles are adapted; observations and other data pass through unchanged. - Cleanup: - The unused `./tools` export and the duplicate default re-exports are removed. - `StackIdentityInput` is now `StackKeysInput`. - The default runtime is chosen in the package. - CLI tests use a CLI-local cleanup helper. ## Stack Part 8 of 8 of the stack package simplification, based on supabase#6840. Review and merge in order: 1. supabase#6834 fix(stack): stop Postgres containers with a fast shutdown 2. supabase#6835 fix(stack): harden readiness, port claims, and container storage 3. supabase#6836 refactor(stack): flatten the owner process layers 4. supabase#6837 refactor(stack): unify the database snapshot protocol across engines 5. supabase#6838 feat(stack): lease owners with a lock and bind session stacks to creators 6. supabase#6839 feat(stack): ship the functions bootstrap with the package 7. supabase#6840 refactor(stack): own composition policy and stack lookup in the package 8. supabase#6841 feat(stack): add a testing entrypoint and derive the Promise API ← this PR --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.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
2 participants
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.
Problem
The PostgreSQL image sets no stop signal, so
docker stopsends SIGTERM. PostgreSQL treats SIGTERM as a "smart" shutdown and waits for every client to disconnect. With stack services still connected,docker stopwaits out Docker's grace period, the container is killed, and the next start runs crash recovery.Change
--stop-signal SIGINT, PostgreSQL's fast shutdown. The official postgres image uses the same default.The native runtime already sent SIGINT.
Stack
Part 1 of 8 of the stack package simplification, based on develop. Review and merge in order:
🤖 Generated with Claude Code