Skip to content

fix(cli): propagate delegated Go child exit codes through finalizers #5841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Coly010 merged 1 commit into from
Jul 9, 2026
Merged

fix(cli): propagate delegated Go child exit codes through finalizers #5841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions apps/cli/src/legacy/commands/bootstrap/SIDE_EFFECTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,11 @@ Human banners are suppressed; a single structured result is emitted:
- **Interim Go-proxy delegation for migration push.** The push step shells out to the bundled
Go binary (`db push --include-roles --include-seed`) until `db push` gets its own native port
(separate Linear issue). The sub-step is **not** instrumentation-wrapped (the subprocess fires
its own push telemetry). Known divergence: `LegacyGoProxy.exec` propagates the exit code, so Go's
push backoff is **not** reproduced (single attempt) — to be restored when `db push` is natively
ported. (`LegacyGoProxy.exec` exits the process on a non-zero exit rather than returning a
failure, so the step cannot be wrapped in `Effect.retry`.)
its own push telemetry). Known divergence: Go's push backoff is **not** reproduced (single
attempt) — to be restored when `db push` is natively ported. (`LegacyGoProxy.exec` fails with a
typed `LegacyGoChildExitError` on a non-zero exit rather than exiting the process — CLI-1879 — so
the step COULD now be wrapped in `Effect.retry`; leaving that unimplemented here is a deliberate
scope decision for the native `db push` port, not a technical blocker.)
- **DB password is forwarded on the same channel the user supplied it (CLI-1617).** The proxy must
be called 1:1 with the user's input: a flag stays a flag, an env var stays an env var. So when the
user passed `-p/--password`, the push sub-step receives `--password <value>` (flag → flag); when
Expand Down
29 changes: 17 additions & 12 deletions apps/cli/src/legacy/commands/db/reset/SIDE_EFFECTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,18 +81,23 @@ seeded over the Storage gateway (reusing the `seed buckets` local path).

## Exit Codes

| Code | Condition |
| ---- | ---------------------------------------------------------------- |
| `0` | success |
| `1` | mutually exclusive target flags (`[db-url linked local]`) |
| `1` | `--version` + `--last` together (`[last version]`) |
| `1` | `--version` not an integer (`invalid version number`) |
| `1` | `--version` has no matching migration file |
| `1` | local: database not running (`supabase start is not running.`) |
| `1` | user declined the reset confirmation (`context canceled`) |
| `1` | `config.toml` parse failure |
| `1` | drop / migrate / seed / vault apply failure, or connection error |
| `1` | local: container recreate / storage health-gate failure (seam) |
| Code | Condition |
| -------------------- | -------------------------------------------------------------------------------------------------------------------------- |
| `0` | success |
| `1` | mutually exclusive target flags (`[db-url linked local]`) |
| `1` | `--version` + `--last` together (`[last version]`) |
| `1` | `--version` not an integer (`invalid version number`) |
| `1` | `--version` has no matching migration file |
| `1` | local: database not running (`supabase start is not running.`) |
| `1` | user declined the reset confirmation (`context canceled`) |
| `1` | `config.toml` parse failure |
| `1` | drop / migrate / seed / vault apply failure, or connection error |
| child's exact code\* | local: container recreate / storage health-gate failure (seam), or `--experimental`/`--linked` delegate (proxy) child exit |

\* The `db __db-bootstrap` seam and the `--experimental` remote delegate both
propagate the spawned `supabase-go` child's real exit code (e.g. `130` after a
Ctrl-C mid-recreate) instead of collapsing every failure to `1` — in every
`--output-format` (CLI-1879).

## Output

Expand Down
82 changes: 57 additions & 25 deletions apps/cli/src/legacy/commands/db/reset/reset.handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,36 @@ export const legacyDbReset = Effect.fn("legacy.db.reset")(function* (flags: Lega
}

const connType = target.connType ?? "local";
// Single source of truth for "does this reset delegate to the Go child?" —
// checked at both delegation sites below (before `resolve()` for a linked
// target, after it for a `--db-url` target) so the two call sites can never
// drift apart.
const shouldDelegateExperimental = experimental && resolvedVersion === "";

// Delegates the remaining `--experimental` schema-files apply path
// (`apply.MigrateAndSeed`, not ported) to the Go child. In text mode inherit
// its stdio. Under a machine-output mode (`--output-format json|stream-json`)
// the Go child emits no TS envelope, so suppress its stdout (capture + discard)
// and emit the same structured success the native local and remote paths do,
// keeping the JSON contract consistent across all reset paths.
const delegateExperimentalReset = () =>
Effect.gen(function* () {
const env = { SUPABASE_TELEMETRY_DISABLED: "1" };
if (output.format === "text") {
yield* proxy.exec(buildResetArgs(flags, connType, yes), { env });
} else {
// Machine-output mode is non-interactive: give the Go child a non-TTY stdin
// (`stdin: "ignore"`) so it can't block on (or be answered at) Go's
// destructive reset prompt — it takes the default `false`, matching the
// native reset path which suppresses prompts under json/stream-json.
yield* proxy.execCapture(buildResetArgs(flags, connType, yes), { env, stdin: "ignore" });
yield* output.success("Reset remote database.", {
target: "remote",
version: resolvedVersion,
});
}
});

// Go's ParseDatabaseConfig runs LoadProjectRef BEFORE the fallible linked
// resolution (db_url.go:87-95), and Execute() writes the linked-project cache
// even when a later step errors (root.go:171-181). Pre-load the ref so the
Expand All @@ -235,7 +265,23 @@ export const legacyDbReset = Effect.fn("legacy.db.reset")(function* (flags: Lega
if (connType === "linked") {
const refResolver = yield* LegacyProjectRefResolver;
linkedRefForCache = yield* refResolver.loadProjectRef(Option.none());

// A linked target is never local (`resolver.resolve()`'s "linked" branch
// always returns `isLocal: false`), so the delegated-experimental check can
// run BEFORE calling `resolve()`. This matters: for `connType === "linked"`,
// `resolve()` mints/verifies a temporary Postgres login role over the
// Management API — and the delegated Go child re-runs that exact same
// `ParseDatabaseConfig` work itself once delegation happens. Calling
// `resolve()` here would mint the temp role twice for zero downstream use on
// this branch (Go's own reset flow mints it exactly once, as part of the code
// path being delegated to — confirmed against `apps/cli-go/internal/utils/
// flags/db_url.go`'s `NewDbConfigWithPassword`/`initLoginRole`). CLI-1879.
if (shouldDelegateExperimental) {
yield* delegateExperimentalReset();
return;
}
}

const cfg = yield* resolver.resolve({ dbUrl: flags.dbUrl, connType, dnsResolver });

// Local target → native local reset. The container-recreate primitives live
Expand Down Expand Up @@ -308,34 +354,20 @@ export const legacyDbReset = Effect.fn("legacy.db.reset")(function* (flags: Lega
return;
}

// Resolve the linked ref before any return so the post-run cache (Go's
// `PersistentPostRun` `ensureProjectGroupsCached`) is written even on the
// delegated `--experimental` path below — the Go child runs with telemetry
// disabled and skips that cache, so the TS finalizer must own it.
// Re-confirm `linkedRefForCache` from the now-resolved `cfg.ref` for the native
// remote linked path below (a linked+experimental+versionless target already
// delegated and returned above, before `resolve()` was ever called — see the
// `connType === "linked"` block earlier in this function). A `connType ===
// "db-url"` target leaves `linkedRefForCache` as whatever the pre-load block
// set (nothing, for `db-url`), since this assignment only fires when linked.
const linkedRef = Option.getOrUndefined(cfg.ref ?? Option.none());
if (connType === "linked" && linkedRef !== undefined) linkedRefForCache = linkedRef;

// Remote path. The niche `--experimental` schema-files apply path
// (`apply.MigrateAndSeed`) is not ported; delegate it to the Go child. In text
// mode inherit its stdio. Under a machine-output mode (`--output-format
// json|stream-json`) the Go child emits no TS envelope, so suppress its stdout
// (capture + discard) and emit the same structured success the native local and
// remote paths do, keeping the JSON contract consistent across all reset paths.
if (experimental && resolvedVersion === "") {
const env = { SUPABASE_TELEMETRY_DISABLED: "1" };
if (output.format === "text") {
yield* proxy.exec(buildResetArgs(flags, connType, yes), { env });
} else {
// Machine-output mode is non-interactive: give the Go child a non-TTY stdin
// (`stdin: "ignore"`) so it can't block on (or be answered at) Go's
// destructive reset prompt — it takes the default `false`, matching the
// native reset path which suppresses prompts under json/stream-json.
yield* proxy.execCapture(buildResetArgs(flags, connType, yes), { env, stdin: "ignore" });
yield* output.success("Reset remote database.", {
target: "remote",
version: resolvedVersion,
});
}
// Remaining remote target: a `--db-url` pointing at a non-local host (the
// `connType === "linked"` case already delegated above, before `resolve()`,
// without resolving a connection at all).
if (shouldDelegateExperimental) {
yield* delegateExperimentalReset();
return;
}

Expand Down
Loading
Loading