diff --git a/apps/cli/docs/go-cli-divergences.md b/apps/cli/docs/go-cli-divergences.md index c27ea4ea82..fbe419f359 100644 --- a/apps/cli/docs/go-cli-divergences.md +++ b/apps/cli/docs/go-cli-divergences.md @@ -104,6 +104,12 @@ These commands exist in the TS CLI today but have no direct top-level equivalent `supabase start`, `db start`, `--from-backup`, and shadow containers (the last is why the shadow baseline cache's cold export can stop/start in ~1s). Timing is not part of the Go-parity surface (ADR 0016). +- `test db` (and its `db test` alias) exits `1` when `pg_prove` ran no tests (CLI-2194, #6206). + `pg_prove` prints `Result: NOTESTS` and still exits `0` for an empty run, and Go returns that + code verbatim (`internal/db/test/test.go` → `DockerRunOnceWithConfig`), so a typo'd path, an + empty tests directory, or a bind the daemon resolved against a different filesystem than the + CLI's (a sibling-container Docker socket) all reported a green build that ran zero tests. The + TAP stream on stdout is unchanged; the diagnostic goes to stderr like every other failure. - `functions serve` per-function env discovery (CLI-2184, #6179): without `--env-file`, each `supabase/functions//.env` overrides matching values from the shared `supabase/functions/.env` for that Function only; an explicit `--env-file` remains the diff --git a/apps/cli/src/legacy/commands/db/diff/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/db/diff/SIDE_EFFECTS.md index 8f3d5d76ad..4f171ff695 100644 --- a/apps/cli/src/legacy/commands/db/diff/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/db/diff/SIDE_EFFECTS.md @@ -36,17 +36,17 @@ it, and JSON `null` disables formatting without disabling safe compaction. ## Files Written -| Path | Format | When | -| --------------------------------------------------------------------------- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `/supabase/migrations/_.sql` | SQL | non-empty `--file` diff; bundled pg-delta may emit ordered transaction-aware files, while pgAdmin always emits one | -| `` (from `--output` / `-o`) | SQL | explicit `--from/--to` mode with `--output`; flattened review representation, not a portable apply script | -| `/supabase/.temp/pgdelta/*.json` | JSON | legacy opt-out's explicit migrations catalog | -| `/supabase/.temp/pgdelta/pgdelta-target-ca.crt` | PEM | legacy opt-out, for a Supabase TLS target | -| `/supabase/.temp/pgdelta/v2/debug//*.json` | JSON | bundled engine with `PGDELTA_DEBUG` | -| `~/.supabase/cache/shadow-baseline/shadow-baseline-.tar` | tar | cache-enabled (default) COLD shadow provision creates the current key's snapshot (native diff targets + the explicit `--from/--to migrations` catalog miss; never `--use-pgadmin`/`--use-pg-schema`); a warm hit `touch`es its mtime (LRU); every cache-eligible acquire may delete other keys under LRU keep-8 + 14-day mtime TTL — ~90MB (`SUPABASE_HOME` overrides the root) | -| `~/.supabase/cache/shadow-baseline/shadow-baseline-.tar..partial` | tar | during a cold export — the in-flight temp file, `rename`d into the tar above on success and removed on failure; only a crash/SIGKILL leaves it behind, and later cold exports / warm hits sweep leftovers older than an hour | -| `~/.supabase//linked-project.json` | JSON | `--linked` (post-run cache) | -| `~/.supabase/telemetry.json` | JSON | every invocation (post-run) | +| Path | Format | When | +| --------------------------------------------------------------------------- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `/supabase/migrations/_.sql` | SQL | non-empty `--file` diff; bundled pg-delta may emit ordered transaction-aware files, while pgAdmin always emits one | +| `` (from `--output` / `-o`) | SQL | explicit `--from/--to` mode with `--output`; flattened review representation, not a portable apply script | +| `/supabase/.temp/pgdelta/*.json` | JSON | legacy opt-out's explicit migrations catalog | +| `/supabase/.temp/pgdelta/pgdelta-target-ca.crt` | PEM | legacy opt-out, for a Supabase TLS target | +| `/supabase/.temp/pgdelta/v2/debug//*.json` | JSON | bundled engine with `PGDELTA_DEBUG` | +| `~/.supabase/cache/shadow-baseline/shadow-baseline-.tar` | tar | cache-enabled (default) COLD shadow provision creates the current key's snapshot (native diff targets, `--use-pgadmin`, and the explicit `--from/--to migrations` catalog miss; never `--use-pg-schema`, which delegates to the bundled Go binary); a warm hit `touch`es its mtime (LRU); every cache-eligible acquire may delete other keys under LRU keep-8 + 14-day mtime TTL — ~90MB (`SUPABASE_HOME` overrides the root) | +| `~/.supabase/cache/shadow-baseline/shadow-baseline-.tar..partial` | tar | during a cold export — the in-flight temp file, `rename`d into the tar above on success and removed on failure; only a crash/SIGKILL leaves it behind, and later cold exports / warm hits sweep leftovers older than an hour | +| `~/.supabase//linked-project.json` | JSON | `--linked` (post-run cache) | +| `~/.supabase/telemetry.json` | JSON | every invocation (post-run) | ## Docker @@ -62,9 +62,10 @@ it, and JSON `null` disables formatting without disabling safe compaction. called with `targetLocal: false`/`usePgDelta: false` to skip the declarative-schema-override branch — not a second, `__catalog`-specific shadow, and not a shared `mode: "diff"` parameter (that seam-era concept no longer exists). `--use-pgadmin` provisions its OWN shadow via a - narrower composition — `legacyCreateShadowDatabase` -> health-wait -> `legacyMigrateShadowDatabase` - directly (`diff.handler.ts`'s pgadmin branch) — with no declarative-schema-override branch and - no `targetUrlOverride`. + narrower composition — `legacyWithShadowDatabase` (the same cached acquire/release seam as the + native branch) -> `legacyWaitForShadowReady` (the same connect probe, not the Docker + HEALTHCHECK) -> `legacyMigrateShadowDatabase` directly (`diff.handler.ts`'s pgadmin branch) — + with no declarative-schema-override branch and no `targetUrlOverride`. - `supabase/migra` container — the migra OOM bash fallback only. - **Differ container** (`--use-pgadmin`, CLI-1968) — `supabase/pgadmin-schema-diff:cli-0.0.5` (`dockerfileServiceImage("differ")`). One `docker run --rm` when no `--schema` is given; one @@ -107,8 +108,9 @@ of this command's own target resolve, ahead of the differ container. | `SUPABASE_INTERNAL_IMAGE_REGISTRY` | overrides the differ's / shadow's image registry (shell **or** project `.env`, applied for the run via `legacyApplyProjectEnv`, matching `db push`/`db pull`/`db dump`) | no | `SUPABASE_DB_SHADOW_PORT`/`SUPABASE_NETWORK_ID`/`--network-id`/`SUPABASE_PROJECT_ID`/ -`SUPABASE_DB_HEALTH_TIMEOUT` all apply to `--use-pgadmin` too — its shadow is provisioned -through the same primitives. +`SUPABASE_DB_HEALTH_TIMEOUT`/`SUPABASE_HOME`/`SUPABASE_SHADOW_CACHE`/`SUPABASE_SHADOW_DEBUG` all +apply to `--use-pgadmin` too — its shadow is provisioned through the same primitives, including +the same cached acquire. `SUPABASE_EXPERIMENTAL_PG_DELTA` is **read, no effect** on the pgadmin path: the pg-delta engine-selection lookup (`legacyShouldUsePgDelta`) runs unconditionally, before the @@ -245,8 +247,9 @@ Container lifecycle is identical to the uncached path except a cold run drops `- on release). A cache anomaly never fails the command — a warm-path anomaly cold-provisions instead, a cold export failure only warns and leaves the run uncached (one exception: a shadow that fails to come back up after the snapshot fails the run rather than reporting a false success). See `shared/db-bootstrap/ -shadow-cache.ts`'s doc comment for the mechanics. `--use-pgadmin` is NOT cached — its shadow keeps -the plain create/remove lifecycle. +shadow-cache.ts`'s doc comment for the mechanics. `--use-pgadmin` shares this cache and these +snapshots: its shadow runs the same forced-on Webhooks/`pg_net` baseline +(`legacyMigrateShadowDatabase`), so it keys to the same tars as the native branch. ### `--use-pgadmin` parity quirks and deliberate divergence (CLI-1968) diff --git a/apps/cli/src/legacy/commands/db/diff/diff.handler.ts b/apps/cli/src/legacy/commands/db/diff/diff.handler.ts index 79f3279a1c..92cc2d9924 100644 --- a/apps/cli/src/legacy/commands/db/diff/diff.handler.ts +++ b/apps/cli/src/legacy/commands/db/diff/diff.handler.ts @@ -28,12 +28,10 @@ import { legacySchemaToCsvField } from "../../../shared/legacy-schema-flags.ts"; import { legacyFindDropStatements } from "../../../shared/legacy-sql-split.ts"; import { legacyBuildLocalDbContainerInputs } from "../../../shared/db-bootstrap/local-container-inputs.ts"; import { legacyIsLocalDbRunning } from "../../../shared/db-bootstrap/local-db-running.ts"; -import { legacyWaitForHealthyServices } from "../../../shared/db-bootstrap/health-check.ts"; +import { legacyWaitForShadowReady } from "../../../shared/db-bootstrap/health-check.ts"; import { legacyWithShadowDatabase } from "../../../shared/db-bootstrap/shadow-cache.ts"; import { - legacyCreateShadowDatabase, legacyMigrateShadowDatabase, - legacyRemoveShadowDatabase, legacyShadowRunInputFromLocalContainerInputs, } from "../../../shared/db-bootstrap/shadow-database.ts"; import { LegacyLinkedProjectCache } from "../../../telemetry/legacy-linked-project-cache.service.ts"; @@ -630,24 +628,39 @@ export const legacyDbDiff = Effect.fn("legacy.db.diff")(function* (flags: Legacy password: shadowBase.password, database: "postgres", }; - // Register cleanup atomically with shadow creation; preparation stays interruptible. - const sql = yield* Effect.acquireUseRelease( - legacyCreateShadowDatabase(spawner, shadowBase), + // Register cleanup atomically with shadow acquisition; preparation stays interruptible. + // Same cached provisioning seam as the native branch below (`legacyWithShadowDatabase`, + // `shadow-cache.ts`) — see that call site's comment for the full rationale. The migrate + // step is untouched: it still receives the whole local migration set through the SAME + // `legacyMigrateShadowDatabase`, only now told (via `handle`) whether the cluster already + // carries the platform baseline. `webhooks: "enabled"` matches that function's own forced + // `pg_net` baseline, so this shares the snapshots the native branch below keys for the SAME + // forced-on baseline (its legacy-engine runs) rather than a second, pgAdmin-only set. It + // deliberately does NOT share with next's config-following migrate — see that branch's own + // `migrationMode`-conditional `webhooks` opt. + const sql = yield* legacyWithShadowDatabase( + spawner, + shadowBase, (handle) => Effect.gen(function* () { - yield* legacyWaitForHealthyServices(spawner, [handle.containerId], { + yield* legacyWaitForShadowReady(spawner, handle.containerId, shadowConnConfig, { timeoutSeconds: shadowBase.healthTimeoutSeconds, + image: shadowBase.image, }); - yield* legacyMigrateShadowDatabase(spawner, { - fs, - path, - workdir: cliConfig.workdir, - projectId: shadowBase.projectId, - container: handle.containerId, - networkId: shadowBase.networkId, - connConfig: shadowConnConfig, - setup: shadowBase.setup, - }); + yield* legacyMigrateShadowDatabase( + spawner, + { + fs, + path, + workdir: cliConfig.workdir, + projectId: shadowBase.projectId, + container: handle.containerId, + networkId: shadowBase.networkId, + connConfig: shadowConnConfig, + setup: shadowBase.setup, + }, + handle, + ); yield* emitStatus("Diffing local database with current migrations..."); return yield* legacyDiffSchemaPgAdmin({ // `source`/`target` are INVERTED relative to the migra/pg-delta path below: @@ -663,7 +676,7 @@ export const legacyDbDiff = Effect.fn("legacy.db.diff")(function* (flags: Legacy emitStatus, }); }), - (handle) => legacyRemoveShadowDatabase(spawner, handle.containerId), + { webhooks: "enabled" }, ); diffResult = { sql, files: undefined }; } else { @@ -689,8 +702,14 @@ export const legacyDbDiff = Effect.fn("legacy.db.diff")(function* (flags: Legacy // why the cache seam sits here (with `SUPABASE_SHADOW_CACHE` unset it IS today's // create/remove pair; otherwise a key-matching PGDATA snapshot is restored into the fresh // container in a few seconds instead of cold-provisioning the baseline in ~15s). - // `webhooks: "enabled"` matches `legacyMigrateShadowDatabase`'s forced `pg_net` - // baseline — the cache key must not collide with next's config-following migrate. + // The `webhooks` policy MUST describe the baseline the `use` callback below actually + // provisions, because that is what the cache key hashes: `legacyPrepareShadowSource` + // dispatches on `migrationMode`, running `legacyMigrateShadowDatabase` (forced `pg_net`) for + // the legacy engine but `legacyMigrateNextShadowDatabase` (config-following) for pg-delta + // next. Hardcoding `"enabled"` for both would make a next-mode cold run on a + // webhooks-disabled project publish a `pg_net`-less cluster under the + // `webhooks_enabled=true` key that the pgAdmin branch above (whose baseline really is + // forced-on) could then warm-restore, and vice versa. diffResult = yield* legacyWithShadowDatabase( spawner, shadowInput, @@ -741,7 +760,7 @@ export const legacyDbDiff = Effect.fn("legacy.db.diff")(function* (flags: Legacy // single migration file. return { sql, files: undefined }; }), - { webhooks: "enabled" }, + migrationMode === "pgdelta-next" ? {} : { webhooks: "enabled" }, ); } const out = diffResult.sql; diff --git a/apps/cli/src/legacy/commands/db/diff/diff.integration.test.ts b/apps/cli/src/legacy/commands/db/diff/diff.integration.test.ts index 9f4b41aa16..5aed03f4c9 100644 --- a/apps/cli/src/legacy/commands/db/diff/diff.integration.test.ts +++ b/apps/cli/src/legacy/commands/db/diff/diff.integration.test.ts @@ -2,7 +2,7 @@ import { existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from import { basename, join } from "node:path"; import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; -import { Effect, Exit, Fiber, Layer, Option } from "effect"; +import { Effect, Exit, Fiber, Layer, Option, Path } from "effect"; import * as HttpClient from "effect/unstable/http/HttpClient"; import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse"; @@ -18,6 +18,7 @@ import { mockLegacyTelemetryStateTracked, useLegacyShadowCacheDisabled, useLegacyTempWorkdir, + withLegacyShadowCacheEnabled, } from "../../../../../tests/helpers/legacy-mocks.ts"; import { mockOutput, mockRuntimeInfo } from "../../../../../tests/helpers/mocks.ts"; import { dockerfileServiceImage } from "../../../../shared/services/dockerfile-images.ts"; @@ -54,6 +55,7 @@ import { LegacyEdgeRuntimeScript, } from "../../../shared/legacy-edge-runtime-script.service.ts"; import { LegacyPgDeltaSslProbe } from "../../../shared/legacy-pgdelta-ssl-probe.service.ts"; +import { legacyShadowBaselineCacheDir } from "../../../shared/legacy-pgdelta.paths.ts"; import { LegacyPgDeltaEngine, type LegacyPgDeltaDatabaseDiffInput, @@ -93,16 +95,11 @@ interface SetupOpts { // over `failWriteOnCall` when shadow setup writes extra SQL before the // command's `--file` migration. readonly failWriteMatching?: (path: string) => boolean; - // When set, the shadow container never reports healthy — for the interrupt-during- - // health-wait regression coverage (review: PRRT_kwDOErm0O86XMrID). See - // `mockLegacyShadowContainerCliSpawner`'s own doc comment for why this is required - // (not `Effect.never`) to observe a genuinely suspended retry loop. Only the - // `--use-pgadmin` branch still gates on the Docker healthcheck; the shadow-source - // branch gates on `neverConnectableShadow` below instead. - readonly neverHealthyShadow?: boolean; // When set, every connect to the shadow's own port is refused, so the readiness gate - // (`legacyWaitForShadowReady`) keeps polling — the shadow-source branch's equivalent of - // `neverHealthyShadow`, since that wait no longer consults the Docker healthcheck. + // (`legacyWaitForShadowReady`) keeps polling — for the interrupt-during-readiness-wait + // regression coverage (review: PRRT_kwDOErm0O86XMrID). A refused connect, not an unhealthy + // container, is what keeps a provisioning fiber genuinely suspended: NO branch (pgAdmin + // included) gates on the Docker healthcheck any more. readonly neverConnectableShadow?: boolean; // `LegacyCliConfig.projectId` (the `SUPABASE_PROJECT_ID` env-only reader). Defaults // to `Option.some("test")`; pass `Option.none()` to exercise the @@ -195,7 +192,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { // and a real (fake) Postgres session backs the shadow's own // platform-baseline/migration/declarative setup. const shadowSpawner = mockLegacyShadowContainerCliSpawner({ - neverHealthy: opts.neverHealthyShadow ?? false, dbNotRunning: opts.dbNotRunning ?? false, dbInspectFailsWith: opts.dbInspectFailsWith, }); @@ -1078,6 +1074,84 @@ describe("legacy db diff", () => { }, ); + // The shadow baseline cache (`shared/db-bootstrap/shadow-cache.ts`) is ON by default in + // production; the suite-wide `useLegacyShadowCacheDisabled` above turns it off everywhere else. + // `--use-pgadmin` used to be the one native branch that provisioned a bare, uncached shadow — + // this scenario turns the cache back on (under a per-test `SUPABASE_HOME`) and drives two + // pgAdmin diffs to prove it now shares the same seam as the migra/pg-delta branch. The cache's + // own mechanics are covered in `shared/db-bootstrap/shadow-cache.integration.test.ts`. + it.effect( + "--use-pgadmin reuses the cached platform baseline on a second run, without changing the diff it produces", + () => + withLegacyShadowCacheEnabled( + join(tmp.current, "_supabase_home"), + Effect.gen(function* () { + const run = Effect.fnUntraced(function* () { + const s = setup(tmp.current, { + pgadminStdout: [JSON.stringify([pgadminEntry()])], + }); + yield* legacyDbDiff(flags({ usePgAdmin: Option.some(true) })).pipe( + Effect.provide(s.layer), + ); + return s; + }); + + const cold = yield* run(); + expect(cold.shadowSetupJobCalls.length).toBeGreaterThan(0); + expect(cold.shadowSpawned.filter((c) => c.args[0] === "stop")).toHaveLength(1); + + const warm = yield* run(); + // The restored cluster already carries the platform baseline, so + // `legacyMigrateShadowDatabase` skips `SetupDatabase` and its one-shot jobs — but + // still creates `contrib_regression` and replays every local migration, and the + // differ run and its output are byte-identical to the cold run's. + expect(warm.shadowSetupJobCalls).toHaveLength(0); + expect(warm.shadowSpawned.filter((c) => c.args[0] === "stop")).toHaveLength(0); + expect(warm.shadowSpawned.filter((c) => c.args[0] === "rm")).toHaveLength(1); + expect(warm.differCalls).toHaveLength(1); + expect(stdout(warm.out)).toBe(stdout(cold.out)); + }), + ), + ); + + // The two native branches provision DIFFERENT clusters on a webhooks-disabled project: + // `--use-pgadmin` migrates through `legacyMigrateShadowDatabase`, whose baseline installs + // `pg_net` unconditionally, while pg-delta next migrates through + // `legacyMigrateNextShadowDatabase`, which follows `config.toml` (webhooks absent = off). If the + // cache key described the caller's literal `webhooks` opt rather than the baseline the run + // actually builds, one would warm-restore the other's snapshot and silently diff against a + // cluster with the wrong extension set. + it.effect( + "--use-pgadmin does not reuse the pg-delta next baseline when config leaves webhooks disabled", + () => + withLegacyShadowCacheEnabled( + join(tmp.current, "_supabase_home"), + Effect.gen(function* () { + const next = setup(tmp.current, { pgDeltaImplementation: "next" }); + yield* legacyDbDiff(flags({ usePgDelta: Option.some(true) })).pipe( + Effect.provide(next.layer), + ); + expect(next.shadowSpawned.filter((c) => c.args[0] === "stop")).toHaveLength(1); + + const pgadmin = setup(tmp.current, { + pgadminStdout: [JSON.stringify([pgadminEntry()])], + }); + yield* legacyDbDiff(flags({ usePgAdmin: Option.some(true) })).pipe( + Effect.provide(pgadmin.layer), + ); + // Cold, not warm: it ran its own `SetupDatabase` one-shot jobs and published its own + // snapshot instead of restoring the config-following one next just wrote. + expect(pgadmin.shadowSetupJobCalls.length).toBeGreaterThan(0); + expect(pgadmin.shadowSpawned.filter((c) => c.args[0] === "stop")).toHaveLength(1); + + // Two keys, two tars — the poisoning scenario cannot arise. + const cacheDir = legacyShadowBaselineCacheDir(yield* Path.Path); + const tars = readdirSync(cacheDir).filter((name) => name.endsWith(".tar")); + expect(tars).toHaveLength(2); + }).pipe(Effect.provide(BunServices.layer)), + ), + ); + it.effect("rejects --project-ref combined with --use-pg-schema before delegating", () => { // The bundled Go binary's own `db diff` never registered `--project-ref`, so // the flag can't be forwarded — fail up front instead of silently dropping it. @@ -2527,23 +2601,18 @@ describe("legacy db diff", () => { it.live( "removes the shadow container on interruption during the health wait for --use-pgadmin too", () => { - const s = setup(tmp.current, { neverHealthyShadow: true }); + const s = setup(tmp.current, { neverConnectableShadow: true }); return Effect.gen(function* () { const fiber = yield* legacyDbDiff(flags({ usePgAdmin: Option.some(true) })).pipe( Effect.provide(s.layer), Effect.forkChild({ startImmediately: true }), ); - // Wait for the SHADOW's own health probe specifically (its 64-hex id) — - // the pgadmin path's separate `supabase_db_test` "is running" probe fires - // first and would otherwise satisfy a looser check immediately. - while ( - !s.shadowSpawned.some( - (c) => - c.args[0] === "container" && - c.args[1] === "inspect" && - c.args[2] === LEGACY_FAKE_SHADOW_CONTAINER_ID, - ) - ) { + // Wait until the shadow's readiness gate has actually refused a connect at least + // once — proving the fiber is genuinely suspended inside `legacyWaitForShadowReady`'s + // retry loop, not merely past the `create` call. Same gate as the native branch's + // own interrupt test above: pgAdmin gates on the connect probe too now, not on the + // Docker healthcheck. + while (s.shadowConnectedDatabases.length === 0) { yield* Effect.sleep("5 millis"); } yield* Fiber.interrupt(fiber); diff --git a/apps/cli/src/legacy/commands/db/pull/pull.handler.ts b/apps/cli/src/legacy/commands/db/pull/pull.handler.ts index 168a0a9f5b..73fbc8b5ce 100644 --- a/apps/cli/src/legacy/commands/db/pull/pull.handler.ts +++ b/apps/cli/src/legacy/commands/db/pull/pull.handler.ts @@ -795,8 +795,14 @@ export const legacyDbPull = Effect.fn("legacy.db.pull")(function* (flags: Legacy // pooler-retry attempt still acquires and releases its own shadow — on the warm path // each attempt restores its own fresh container from the same cached snapshot, // sequentially. - // `webhooks: "enabled"` matches `legacyMigrateShadowDatabase`'s forced `pg_net` - // baseline — the cache key must not collide with next's config-following migrate. + // The `webhooks` policy MUST describe the baseline the `use` callback below actually + // provisions, because that is what the cache key hashes: `legacyPrepareShadowSource` + // dispatches on `migrationMode`, running `legacyMigrateShadowDatabase` (forced + // `pg_net`) for the legacy engine but `legacyMigrateNextShadowDatabase` + // (config-following) for pg-delta next. Hardcoding `"enabled"` for both would make a + // next-mode cold run on a webhooks-disabled project publish a `pg_net`-less cluster + // under the `webhooks_enabled=true` key, which `db diff --use-pgadmin` (whose baseline + // really is forced-on) could then warm-restore, and vice versa. return yield* legacyWithShadowDatabase( spawner, shadowInput, @@ -849,7 +855,7 @@ export const legacyDbPull = Effect.fn("legacy.db.pull")(function* (flags: Legacy }); return { sql, files: undefined, debug: undefined }; }), - { webhooks: "enabled" }, + migrationMode === "pgdelta-next" ? {} : { webhooks: "enabled" }, ); }); const diffOutcome = yield* withPoolerFallback(targetEndpoint, runShadowDiff); diff --git a/apps/cli/src/legacy/commands/db/test/test.integration.test.ts b/apps/cli/src/legacy/commands/db/test/test.integration.test.ts index 9f109c93fb..38fd0c105c 100644 --- a/apps/cli/src/legacy/commands/db/test/test.integration.test.ts +++ b/apps/cli/src/legacy/commands/db/test/test.integration.test.ts @@ -135,7 +135,7 @@ function mockDbConnection() { return { layer, execCalls }; } -function mockDockerRun(opts: { exitCode?: number } = {}) { +function mockDockerRun(opts: { exitCode?: number; stdout?: ReadonlyArray } = {}) { let lastOpts: LegacyDockerRunOpts | undefined; const layer = Layer.succeed(LegacyDockerRun, { run: (runOpts) => { @@ -150,9 +150,15 @@ function mockDockerRun(opts: { exitCode?: number } = {}) { stderr: "", }); }, - runStream: (runOpts) => { + runStream: (runOpts, streamOpts) => { lastOpts = runOpts; - return Effect.succeed({ exitCode: opts.exitCode ?? 0, stderr: "" }); + return Effect.gen(function* () { + const encoder = new TextEncoder(); + for (const chunk of opts.stdout ?? []) { + yield* streamOpts.onStdout(encoder.encode(chunk)); + } + return { exitCode: opts.exitCode ?? 0, stderr: "" }; + }); }, }); return { @@ -175,6 +181,7 @@ const runtimeInfoLayer = Layer.succeed(RuntimeInfo, { interface SetupOpts { format?: "text" | "json" | "stream-json"; exitCode?: number; + stdout?: ReadonlyArray; } function setup(opts: SetupOpts = {}) { @@ -183,7 +190,7 @@ function setup(opts: SetupOpts = {}) { const analytics = mockContextualAnalytics(); const telemetry = mockLegacyTelemetryStateTracked(); const connection = mockDbConnection(); - const docker = mockDockerRun({ exitCode: opts.exitCode }); + const docker = mockDockerRun({ exitCode: opts.exitCode, stdout: opts.stdout }); const args = ["db", "test"]; const layer = Layer.mergeAll( out.layer, @@ -317,4 +324,32 @@ describe("legacy db test (alias) integration", () => { }).pipe(Effect.provide(layer)); }, ); + + it.live("fails in text mode when the run found no tests", () => { + const { layer, processControl } = setup({ + exitCode: 0, + stdout: ["Files=0, Tests=0, 0 wallclock secs\nResult: NOTESTS\n"], + }); + return Effect.gen(function* () { + const exit = yield* Effect.exit(legacyRunTestDbCommand(flags())); + expect(exit._tag).toBe("Failure"); + // Text mode lets the failed Effect drive the exit code, as for a run failure. + expect(processControl.exitCode).toBeUndefined(); + }).pipe(Effect.provide(layer)); + }); + + it.live("in json mode, a run that found no tests takes the same stderr + exit 1 path", () => { + const { layer, out, processControl } = setup({ + format: "json", + exitCode: 0, + stdout: ["Files=0, Tests=0, 0 wallclock secs\nResult: NOTESTS\n"], + }); + return Effect.gen(function* () { + yield* legacyRunTestDbCommand(flags()); + expect(out.stderrText).toContain("no pgTAP tests found in /work/project/supabase/tests"); + expect(processControl.exitCode).toBe(1); + // The TAP stream reached stdout intact, with no JSON envelope appended. + expect(out.stdoutText).toBe("Files=0, Tests=0, 0 wallclock secs\nResult: NOTESTS\n"); + }).pipe(Effect.provide(layer)); + }); }); diff --git a/apps/cli/src/legacy/commands/gen/bearer-jwt/bearer-jwt.signing-key.ts b/apps/cli/src/legacy/commands/gen/bearer-jwt/bearer-jwt.signing-key.ts index 30580158d6..0fbc88c385 100644 --- a/apps/cli/src/legacy/commands/gen/bearer-jwt/bearer-jwt.signing-key.ts +++ b/apps/cli/src/legacy/commands/gen/bearer-jwt/bearer-jwt.signing-key.ts @@ -285,8 +285,8 @@ const resolveSigningKeyFromConfigured = Effect.fnUntraced(function* ( // what a real `text` run already uses) — only on `NonInteractiveError` (the // json/stream-json `Output` layers' `promptSelect`/`raw`, `output.layer.ts`) fall // back to a REAL, locally-provided `textOutputLayer` instance so the picker still - // renders on a genuine TTY. `textOutputLayer` only needs `Tty` (already in scope), - // so this fallback is a purely local override; the token itself is still written + // renders on a genuine TTY. `textOutputLayer` needs only `Tty` and `Stdio`, both + // ambient from the CLI root, so this fallback is a local override; the token is written // through the AMBIENT `Output` later, in `bearer-jwt.handler.ts`, completely // unaffected by it. See this function's doc comment above for why this // picker needs this at all. diff --git a/apps/cli/src/legacy/commands/init/init.integration.test.ts b/apps/cli/src/legacy/commands/init/init.integration.test.ts index 85f46baa7d..ce74c26f59 100644 --- a/apps/cli/src/legacy/commands/init/init.integration.test.ts +++ b/apps/cli/src/legacy/commands/init/init.integration.test.ts @@ -4,7 +4,7 @@ import { mkdtempSync } from "node:fs"; import { readFile, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { Cause, Effect, Exit, Layer, Option } from "effect"; +import { Cause, Effect, Exit, Layer, Option, Stdio } from "effect"; import { CliArgs } from "../../../shared/cli/cli-args.service.ts"; import { LegacyExperimentalFlag, @@ -95,7 +95,9 @@ function renderFailureToStderr(exit: Exit.Exit) { const out = yield* Output; yield* out.fail(normalizeCause(exit.cause)); }).pipe( - Effect.provide(textOutputLayer.pipe(Layer.provide(mockTty({})))), + Effect.provide( + textOutputLayer.pipe(Layer.provide(Layer.mergeAll(mockTty({}), Stdio.layerTest({})))), + ), Effect.ensuring( Effect.sync(() => { process.stderr.write = originalWrite; diff --git a/apps/cli/src/legacy/commands/migration/squash/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/migration/squash/SIDE_EFFECTS.md index cced4a7655..fdc63f87ac 100644 --- a/apps/cli/src/legacy/commands/migration/squash/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/migration/squash/SIDE_EFFECTS.md @@ -19,24 +19,29 @@ migration-history table to match. | `/supabase/.temp/{project-ref,postgres-version,pooler-url}` | plain text | `--linked` / linked path — skipped when `--project-ref` (or `SUPABASE_PROJECT_ID`) is set | | `~/.supabase/access-token` | plain text | `--linked` without `--password`/`SUPABASE_ACCESS_TOKEN` | | `~/.docker/config.json` + Docker context store | JSON | resolving the Docker hostname for shadow/pg_dump containers | +| `~/.supabase/cache/shadow-baseline/shadow-baseline-.tar` | tar | warm shadow-cache hit — the matching snapshot is streamed into the fresh shadow (see the shadow baseline cache section below) | ## Files Written -| Path | Format | When | -| -------------------------------------------------- | -------- | -------------------------------------------------------------------------------------------------------------------- | -| `/supabase/migrations/.sql` | SQL text | ≥2 migrations squash — **truncated** (0644) then rewritten as the full dump + separator + `auth`/`storage` line diff | -| `/supabase/migrations/.sql` (×N) | — | **deleted** — every earlier merged migration; a per-file failure is non-fatal (printed, not raised) | -| scoped temp dir | SQL | shadow's `initSchema`/`ApplyApiPrivileges` SQL (PG≤14) — removed when the scope closes | -| `/supabase/.temp/linked-project.json` | JSON | `--linked` (post-run cache, even when the command itself fails) | -| `~/.supabase/telemetry.json` | JSON | every invocation (post-run) | +| Path | Format | When | +| --------------------------------------------------------------------------- | -------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `/supabase/migrations/.sql` | SQL text | ≥2 migrations squash — **truncated** (0644) then rewritten as the full dump + separator + `auth`/`storage` line diff | +| `/supabase/migrations/.sql` (×N) | — | **deleted** — every earlier merged migration; a per-file failure is non-fatal (printed, not raised) | +| scoped temp dir | SQL | shadow's `initSchema`/`ApplyApiPrivileges` SQL (PG≤14) — removed when the scope closes | +| `~/.supabase/cache/shadow-baseline/shadow-baseline-.tar` | tar | cache-enabled (default) COLD shadow provision creates the current key's snapshot; a warm hit `touch`es its mtime (LRU); every cache-eligible acquire may delete other keys under LRU keep-8 + 14-day mtime TTL — ~90MB (`SUPABASE_HOME` overrides the root) | +| `~/.supabase/cache/shadow-baseline/shadow-baseline-.tar..partial` | tar | during a cold export — the in-flight temp file, `rename`d into the tar above on success and removed on failure; only a crash/SIGKILL leaves it behind, and later cold exports / warm hits sweep leftovers older than an hour | +| `/supabase/.temp/linked-project.json` | JSON | `--linked` (post-run cache, even when the command itself fails) | +| `~/.supabase/telemetry.json` | JSON | every invocation (post-run) | ## Docker - Network ensure (`legacyEnsureNetwork`, same as `db diff`/`db pull`). - Shadow Postgres container: no `--name`, no network alias, `--publish :5432`, `-c max_worker_processes=0`, `--rm`, PG≤14 tmpfs on `/docker-entrypoint-initdb.d` — created, - started, health-polled (`container inspect`), then removed (`rm -f -v`) once squash finishes, - success or failure. + started, readiness-polled, then removed (`rm -f -v`) once squash finishes, success or failure. + Readiness is `legacyWaitForShadowReady`: each round a `container inspect` still-running check + plus a direct Postgres connect probe — NOT the container's own Docker HEALTHCHECK, whose first + probe only fires at t+10s. - PG15+ one-shot realtime/storage/auth migrate jobs (`initSchema15`), dialed at the shadow container's own 12-char short id as `DB_HOST` (no name/alias needed — see `shared/db-bootstrap/shadow-database.ts`'s own header for why that host still resolves). @@ -51,6 +56,27 @@ migration-history table to match. Unlike `db diff`/`db pull`, the shadow only ever gets `legacySetupDatabase` (platform baseline + roles.sql) — **no** `CREATE DATABASE contrib_regression` template database. +### Shadow baseline cache (`SUPABASE_SHADOW_CACHE`, default ON) + +Squash acquires its shadow through the same `legacyWithShadowDatabase` seam as `db diff`/`db +pull` (`shared/db-bootstrap/shadow-cache.ts`), so everything documented in those commands' +`SIDE_EFFECTS.md` applies verbatim: ON by default, `SUPABASE_SHADOW_CACHE=false`/`=0` opts out +(honored from the ambient env AND the project's dotenv), the artifact is a ~90MB PGDATA snapshot +under `~/.supabase/cache/shadow-baseline/` keyed by a hash of every input baked into the cluster, +retention is LRU keep-8 + 14-day mtime TTL, a cold run drops `--rm` (still removed on release), +and a cache anomaly never fails the command. + +Two squash-specific points: + +- The snapshot covers the platform baseline ONLY. A warm hit skips `legacySetupDatabase` — so + neither `Initialising schema...` nor `Seeding globals from roles.sql...` prints, and the PG15+ + one-shot realtime/storage/auth migrate jobs do not run — and then resumes at exactly the same + seam as a cold run: the before-migration `auth`/`storage` dump, the migrations up to the target, + the after-migration dump, and the full dump are all unchanged. +- Squash's `SetupDatabase` follows `config.toml` for Webhooks/`pg_net`, unlike `db diff`/`db +pull`'s forced-on `legacyMigrateShadowDatabase` baseline. That effective policy is part of the + cache key, so squash keys to its own tars and can never warm-restore a `pg_net`-forced cluster. + ## API Routes | Method | Path | Auth | Purpose | @@ -66,7 +92,9 @@ migration-history table to match. `SUPABASE_YES`, `DB_PASSWORD`, `SUPABASE_ACCESS_TOKEN`, `SUPABASE_SERVICES_HOSTNAME`, `DOCKER_HOST`/`DOCKER_CONTEXT`/`DOCKER_CONFIG`, `SUPABASE_NETWORK_ID`, `SUPABASE_INTERNAL_IMAGE_REGISTRY`, `SUPABASE_PROJECT_ID`, `SUPABASE_DEBUG`, -`SUPABASE_EXPERIMENTAL`. +`SUPABASE_EXPERIMENTAL`, `SUPABASE_HOME` (root of the shadow baseline cache), +`SUPABASE_SHADOW_CACHE` (shadow baseline cache; ON by default, `false`/`0` opts out), +`SUPABASE_SHADOW_DEBUG` (opt-in shadow phase-timing diagnostics on stderr). ## Exit Codes @@ -85,8 +113,8 @@ stderr, in order (path-dependent): ``` Loading config override: [remotes.] (only when --linked resolves a [remotes.] block) -Initialising schema... -Seeding globals from roles.sql... (unconditional — printed even when roles.sql is absent) +Initialising schema... (cold shadow only — a warm baseline-cache hit skips it) +Seeding globals from roles.sql... (cold shadow only; then unconditional — printed even when roles.sql is absent) Applying migration ... (once per migration applied to the shadow) is already the earliest migration. (single-migration no-op) -- or -- @@ -155,4 +183,6 @@ code or the rest of the payload. already-open file descriptor. - `Initialising schema...` is printed by the shared setup prelude just before `legacySetupDatabase` runs rather than from inside it — inherited from CLI-1956, shared with - `db diff`/`db pull`'s identical shadow-provisioning prelude. + `db diff`/`db pull`'s identical shadow-provisioning prelude. A warm shadow-cache hit never + reaches that prelude, so the line (and `Seeding globals from roles.sql...`) is absent — + progress text reflects the work actually performed. diff --git a/apps/cli/src/legacy/commands/migration/squash/squash.handler.ts b/apps/cli/src/legacy/commands/migration/squash/squash.handler.ts index 7d3e85f4e7..9198a3ea96 100644 --- a/apps/cli/src/legacy/commands/migration/squash/squash.handler.ts +++ b/apps/cli/src/legacy/commands/migration/squash/squash.handler.ts @@ -19,16 +19,10 @@ import { legacyBuildLocalDbContainerInputs, type LegacyLocalDbContainerInputs, } from "../../../shared/db-bootstrap/local-container-inputs.ts"; +import { legacyWaitForShadowReady } from "../../../shared/db-bootstrap/health-check.ts"; +import { legacyWithShadowDatabase } from "../../../shared/db-bootstrap/shadow-cache.ts"; import { - legacyResolveDbSetupPrelude, - legacySetupDatabase, -} from "../../../shared/db-bootstrap/db-setup.ts"; -import { legacyWaitForHealthyServices } from "../../../shared/db-bootstrap/health-check.ts"; -import { - legacyBuildShadowSetupDatabaseInput, - legacyConnectShadowDatabase, - legacyCreateShadowDatabase, - legacyRemoveShadowDatabase, + legacyOpenShadowBaselineSession, legacyShadowRunInputFromLocalContainerInputs, } from "../../../shared/db-bootstrap/shadow-database.ts"; import { LegacyDbConfigResolver } from "../../../shared/legacy-db-config.service.ts"; @@ -85,14 +79,31 @@ type Spawner = ChildProcessSpawnerType["Service"]; /** * `squashMigrations`: - * shadow create -> health-wait -> connect -> `start.SetupDatabase` DIRECTLY + * shadow acquire -> health-wait -> connect -> `start.SetupDatabase` DIRECTLY * (NOT `setupShadowConn`, so NO `CREATE DATABASE contrib_regression` template) -> dump the - * auth/storage schema before migrating -> apply every migration -> dump auth/storage again -> - * write the target file as the FULL (unrestricted) dump + the separator + the auth/storage - * line diff. `acquire` is only shadow creation (brief, Docker-API-bound); the health-wait/ - * connect/setup/dump/apply sequence runs in the interruptible `use` phase, matching the CLI-1956 - * review ruling `shadow-database.ts`/`diff.handler.ts` already established (a SIGINT during the - * health-wait must land immediately, from a single cancellable scope). + * auth/storage schema before migrating -> apply the migrations up to the target -> dump + * auth/storage again -> write the target file as the FULL (unrestricted) dump + the separator + + * the auth/storage line diff. `acquire` is only shadow acquisition (brief, Docker-API-bound); the + * health-wait/connect/setup/dump/apply sequence runs in the interruptible `use` phase, matching + * the CLI-1956 review ruling `shadow-database.ts`/`diff.handler.ts` already established (a SIGINT + * during the health-wait must land immediately, from a single cancellable scope). + * + * `legacyWithShadowDatabase` (`shadow-cache.ts`) rather than a bare `legacyCreateShadowDatabase`/ + * `legacyRemoveShadowDatabase` pair — see its doc comment for both halves of the rationale: why + * the lifecycle is an `Effect.acquireUseRelease` (an interrupt must not be able to land between + * creation and the finalizer being attached) and why the cache seam sits here (with + * `SUPABASE_SHADOW_CACHE` unset it IS today's create/remove pair; otherwise a key-matching PGDATA + * snapshot is restored into the fresh container in a few seconds instead of cold-provisioning the + * baseline in ~15s). No `webhooks` override, unlike `db diff`/`db pull`'s forced-on + * `legacyMigrateShadowDatabase` baseline: squash's `SetupDatabase` call has always followed + * `config.toml`, so the cache key must hash the config-following policy or a squash run would + * warm-restore a `pg_net`-forced cluster. + * + * The baseline is the ONLY thing the cache covers, and + * {@link legacyOpenShadowBaselineSession} hands back the open session at exactly that seam — so + * squash's own `before` dump / apply-migrations / `after` dump sequence is unchanged, and a warm + * shadow merely reaches the `before` dump without having re-run `SetupDatabase` (and therefore + * without printing its `Initialising schema...`/`Seeding globals...` progress lines). */ const squashMigrations = Effect.fnUntraced(function* ( spawner: Spawner, @@ -122,107 +133,104 @@ const squashMigrations = Effect.fnUntraced(function* ( // `pg_dump` container below uses; `legacySquashDumpSchema` applies the registry mirror itself. const image = localInputs.bootstrapConfig.postgresImage; - yield* Effect.acquireUseRelease( - legacyCreateShadowDatabase(spawner, shadowInput), - (handle) => - Effect.scoped( - Effect.gen(function* () { - yield* legacyWaitForHealthyServices(spawner, [handle.containerId], { - timeoutSeconds: shadowInput.healthTimeoutSeconds, - }); - const session = yield* legacyConnectShadowDatabase(connConfig); - const resolved = yield* legacyResolveDbSetupPrelude(shadowInput.setup); - yield* legacySetupDatabase( - spawner, - legacyBuildShadowSetupDatabaseInput( - { - fs: shadowInput.fs, - path: shadowInput.path, - workdir: shadowInput.workdir, - projectId: shadowInput.projectId, - container: handle.containerId, - networkId: shadowInput.networkId, - connConfig, - setup: shadowInput.setup, - }, - session, - resolved, - ), - ); + yield* legacyWithShadowDatabase(spawner, shadowInput, (handle) => + Effect.scoped( + Effect.gen(function* () { + yield* legacyWaitForShadowReady(spawner, handle.containerId, connConfig, { + timeoutSeconds: shadowInput.healthTimeoutSeconds, + // The shadow container's OWN resolved image, not the (pin-resolved, unmapped) `image` + // the `pg_dump` containers below use — this one only names the shadow in the + // exec-format recovery hint. + image: shadowInput.image, + }); + const session = yield* legacyOpenShadowBaselineSession( + spawner, + { + fs: shadowInput.fs, + path: shadowInput.path, + workdir: shadowInput.workdir, + projectId: shadowInput.projectId, + container: handle.containerId, + networkId: shadowInput.networkId, + connConfig, + setup: shadowInput.setup, + }, + {}, + handle, + ); - const before = yield* legacySquashDumpSchemaToString({ - image, - conn: connConfig, - schema: ["auth", "storage"], - projectEnvValues: localInputs.context.projectEnvValues, - }); - yield* legacyApplyMigrations( - session, - fs, - path, - migrations, - (message) => new LegacyMigrationApplyError({ message }), - ); - const after = yield* legacySquashDumpSchemaToString({ - image, - conn: connConfig, - schema: ["auth", "storage"], - projectEnvValues: localInputs.context.projectEnvValues, - }); - - const targetPath = migrations[migrations.length - 1]!; - const targetRel = path.relative(workdir, targetPath); - yield* Effect.scoped( - Effect.gen(function* () { - // One open call that both truncates (or creates) the target file AND opens it for the - // writes below, matching `new.handler.ts:87`'s identical `{ flag: "w" }` precedent. - // There is no separate truncate-then-reopen step. - const file = yield* fs.open(targetPath, { flag: "w", mode: 0o644 }).pipe( - Effect.mapError( - (cause) => - new LegacyMigrationSquashWriteError({ - message: `failed to open migration file: ${legacyRelativizeErrorMessage(legacyErrorMessage(cause), targetPath, targetRel)}`, - }), - ), - ); - // The full dump — NO schema restriction — streamed straight into the - // already-truncated file at constant memory. The underlying failure here is - // the docker-log-stream write into the file handle, - // not the line-diff writer below, so it byte-matches "failed to copy - // docker logs:" rather than "failed to write line:". - yield* legacySquashDumpSchema({ - image, - conn: connConfig, - schema: [], - projectEnvValues: localInputs.context.projectEnvValues, - onStdout: (chunk) => - file.writeAll(chunk).pipe( - Effect.mapError( - (cause) => - new LegacyMigrationSquashWriteError({ - message: `failed to copy docker logs: ${legacyErrorMessage(cause)}`, - }), - ), + const before = yield* legacySquashDumpSchemaToString({ + image, + conn: connConfig, + schema: ["auth", "storage"], + projectEnvValues: localInputs.context.projectEnvValues, + }); + yield* legacyApplyMigrations( + session, + fs, + path, + migrations, + (message) => new LegacyMigrationApplyError({ message }), + ); + const after = yield* legacySquashDumpSchemaToString({ + image, + conn: connConfig, + schema: ["auth", "storage"], + projectEnvValues: localInputs.context.projectEnvValues, + }); + + const targetPath = migrations[migrations.length - 1]!; + const targetRel = path.relative(workdir, targetPath); + yield* Effect.scoped( + Effect.gen(function* () { + // One open call that both truncates (or creates) the target file AND opens it for the + // writes below, matching `new.handler.ts:87`'s identical `{ flag: "w" }` precedent. + // There is no separate truncate-then-reopen step. + const file = yield* fs.open(targetPath, { flag: "w", mode: 0o644 }).pipe( + Effect.mapError( + (cause) => + new LegacyMigrationSquashWriteError({ + message: `failed to open migration file: ${legacyRelativizeErrorMessage(legacyErrorMessage(cause), targetPath, targetRel)}`, + }), + ), + ); + // The full dump — NO schema restriction — streamed straight into the + // already-truncated file at constant memory. The underlying failure here is + // the docker-log-stream write into the file handle, + // not the line-diff writer below, so it byte-matches "failed to copy + // docker logs:" rather than "failed to write line:". + yield* legacySquashDumpSchema({ + image, + conn: connConfig, + schema: [], + projectEnvValues: localInputs.context.projectEnvValues, + onStdout: (chunk) => + file.writeAll(chunk).pipe( + Effect.mapError( + (cause) => + new LegacyMigrationSquashWriteError({ + message: `failed to copy docker logs: ${legacyErrorMessage(cause)}`, + }), ), - }); - // The separator and the auth/storage line diff write sequentially to the - // SAME handle, with nothing observable - // between the two writes — combined into one `writeAll` here. - const tail = - LEGACY_SQUASH_SEPARATOR_COMMENT + legacySquashLineByLineDiff(before, after); - yield* file.writeAll(new TextEncoder().encode(tail)).pipe( - Effect.mapError( - (cause) => - new LegacyMigrationSquashWriteError({ - message: `failed to write line: ${legacyRelativizeErrorMessage(legacyErrorMessage(cause), targetPath, targetRel)}`, - }), ), - ); - }), - ); - }), - ), - (handle) => legacyRemoveShadowDatabase(spawner, handle.containerId), + }); + // The separator and the auth/storage line diff write sequentially to the + // SAME handle, with nothing observable + // between the two writes — combined into one `writeAll` here. + const tail = + LEGACY_SQUASH_SEPARATOR_COMMENT + legacySquashLineByLineDiff(before, after); + yield* file.writeAll(new TextEncoder().encode(tail)).pipe( + Effect.mapError( + (cause) => + new LegacyMigrationSquashWriteError({ + message: `failed to write line: ${legacyRelativizeErrorMessage(legacyErrorMessage(cause), targetPath, targetRel)}`, + }), + ), + ); + }), + ); + }), + ), ); }); diff --git a/apps/cli/src/legacy/commands/migration/squash/squash.integration.test.ts b/apps/cli/src/legacy/commands/migration/squash/squash.integration.test.ts index 2d8eff9410..bc1e4a7b6d 100644 --- a/apps/cli/src/legacy/commands/migration/squash/squash.integration.test.ts +++ b/apps/cli/src/legacy/commands/migration/squash/squash.integration.test.ts @@ -1,8 +1,8 @@ -import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; -import { Cause, Effect, Exit, FileSystem, Layer, Option } from "effect"; +import { Cause, Effect, Exit, FileSystem, Layer, Option, Path } from "effect"; import { PlatformError, SystemError } from "effect/PlatformError"; import * as HttpClient from "effect/unstable/http/HttpClient"; import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse"; @@ -17,6 +17,7 @@ import { mockLegacyTelemetryStateTracked, useLegacyShadowCacheDisabled, useLegacyTempWorkdir, + withLegacyShadowCacheEnabled, } from "../../../../../tests/helpers/legacy-mocks.ts"; import { mockOutput, @@ -39,12 +40,16 @@ import { LegacyProjectNotLinkedError } from "../../../config/legacy-project-ref. import { LegacyProjectRefResolver } from "../../../config/legacy-project-ref.service.ts"; import { LEGACY_INTERNAL_SCHEMAS } from "../../../shared/legacy-pg-dump.env.ts"; import { legacyDumpSchemaScript } from "../../../shared/legacy-pg-dump.scripts.ts"; +import { legacyShadowBaselineCacheDir } from "../../../shared/legacy-pgdelta.paths.ts"; import { LegacyDbConfigResolver } from "../../../shared/legacy-db-config.service.ts"; import type { LegacyDbConfigFlags, LegacyResolvedDbConfig, } from "../../../shared/legacy-db-config.types.ts"; -import { LegacyDbExecError } from "../../../shared/legacy-db-connection.errors.ts"; +import { + LegacyDbConnectError, + LegacyDbExecError, +} from "../../../shared/legacy-db-connection.errors.ts"; import { LegacyDbConnection, type LegacyDbSession, @@ -208,6 +213,9 @@ function faultyFsLayer(opts: FsFaultOpts): Layer.Layer { ).pipe(Layer.provide(BunServices.layer)); } +/** The default `[db] shadow_port`, i.e. the port squash's own shadow listens on. */ +const LEGACY_SHADOW_PORT = 54320; + const alwaysReadyHttpClientLayer = Layer.succeed( HttpClient.HttpClient, HttpClient.make((request) => @@ -229,7 +237,13 @@ interface SetupOpts { readonly failResolve?: boolean; readonly failSql?: string; readonly networkId?: string; - readonly neverHealthyShadow?: boolean; + /** + * Every connect to the shadow's own port is refused, so its readiness gate + * (`legacyWaitForShadowReady`) keeps polling until the health budget runs out. That gate is a + * direct Postgres connect probe, not the Docker healthcheck, so an unconnectable shadow — not + * an unhealthy container — is what a squash readiness timeout actually looks like. + */ + readonly neverConnectableShadow?: boolean; readonly failCreateShadow?: boolean; readonly failRemoveShadow?: boolean; readonly failSetupJob?: boolean; @@ -246,7 +260,6 @@ function setup(workdir: string, opts: SetupOpts = {}) { const cache = mockLegacyLinkedProjectCacheTracked(); const spawner = mockLegacyShadowContainerCliSpawner({ - neverHealthy: opts.neverHealthyShadow ?? false, failCreate: opts.failCreateShadow ?? false, failRemove: opts.failRemoveShadow ?? false, }); @@ -268,8 +281,11 @@ function setup(workdir: string, opts: SetupOpts = {}) { const connectedDatabases: Array = []; const connection = Layer.succeed(LegacyDbConnection, { connect: (cfg: LegacyPgConnInput) => - Effect.sync(() => { + Effect.gen(function* () { connectedDatabases.push(cfg.database); + if (opts.neverConnectableShadow === true && cfg.port === LEGACY_SHADOW_PORT) { + return yield* Effect.fail(new LegacyDbConnectError({ message: "connection refused" })); + } const session: LegacyDbSession = { exec: (sql: string) => Effect.suspend(() => { @@ -883,6 +899,96 @@ describe("legacy migration squash", () => { }); }); + // The shadow baseline cache (`shared/db-bootstrap/shadow-cache.ts`) is ON by default in + // production; the suite-wide `useLegacyShadowCacheDisabled` above turns it off everywhere else + // so the other scenarios assert the plain shadow lifecycle. These scenarios turn it back on — + // under a per-test `SUPABASE_HOME`, so the ~90MB-in-production tar never lands in the + // developer's real `~/.supabase` — and drive squash twice to prove the seam is wired: the + // second run must reuse the first's baseline WITHOUT changing anything squash itself produces. + // The cache's own mechanics (key derivation, atomic publish, retention, degradation) are + // covered at their own level in `shared/db-bootstrap/shadow-cache.integration.test.ts`. + describe("shadow baseline cache", () => { + const BEFORE_SQL = "CREATE SCHEMA IF NOT EXISTS auth;\nold auth object;\n"; + const AFTER_SQL = "CREATE SCHEMA IF NOT EXISTS auth;\nnew auth object;\n"; + const FULL_SQL = "CREATE TABLE t (id int);\n"; + const EXPECTED_TARGET = + FULL_SQL + + "\n--\n-- Dumped schema changes for auth and storage\n--\n\n" + + "new auth object;\n"; + + /** Re-enables the cache the suite-wide gate turned off, rooted at a per-test `SUPABASE_HOME`. */ + const withCacheEnabled = (body: Effect.Effect): Effect.Effect => + withLegacyShadowCacheEnabled(join(tmp.current, "_supabase_home"), body); + + /** One full squash run over a freshly re-seeded two-migration project. */ + const runSquash = Effect.fnUntraced(function* () { + seedMigration(tmp.current, "0_init.sql", "create table a (id int);\n"); + seedMigration(tmp.current, "1_target.sql", "create table b (id int);\n"); + const s = setup(tmp.current, { + beforeDumpSql: BEFORE_SQL, + afterDumpSql: AFTER_SQL, + fullDumpSql: FULL_SQL, + }); + yield* legacyMigrationSquash(flags()).pipe(Effect.provide(s.layer)); + return s; + }); + + it.effect( + "reuses the first run's platform baseline on the second squash, without changing the dumps it produces", + () => + withCacheEnabled( + Effect.gen(function* () { + const cold = yield* runSquash(); + // Cold: the baseline really ran (progress lines + the PG15+ one-shot setup jobs), + // and the snapshot was taken at the baseline seam (`docker stop` -> `cp` -> `start`). + expect(stderr(cold.out)).toContain("Initialising schema..."); + expect(stderr(cold.out)).toContain("Seeding globals from roles.sql..."); + expect(cold.setupJobCalls.length).toBeGreaterThan(0); + expect(cold.shadowSpawned.filter((c) => c.args[0] === "stop")).toHaveLength(1); + expect(cold.shadowSpawned.filter((c) => c.args[0] === "start").length).toBeGreaterThan( + 0, + ); + + const warm = yield* runSquash(); + // Warm: the restored cluster already carries the baseline, so `SetupDatabase` — and + // therefore its progress text and its one-shot jobs — is skipped entirely, and + // nothing is re-snapshotted. + expect(stderr(warm.out)).not.toContain("Initialising schema..."); + expect(stderr(warm.out)).not.toContain("Seeding globals from roles.sql..."); + expect(warm.setupJobCalls).toHaveLength(0); + expect(warm.shadowSpawned.filter((c) => c.args[0] === "stop")).toHaveLength(0); + + // Everything downstream of the baseline seam is untouched: both dumps, the + // migrations, the rewritten target file, and the shadow's own lifecycle. + expect(warm.dumpCalls).toHaveLength(3); + expect(stderr(warm.out)).toContain("Applying migration 0_init.sql..."); + expect(stderr(warm.out)).toContain("Applying migration 1_target.sql..."); + expect(stderr(warm.out)).toContain( + "Squashed local migrations to supabase/migrations/1_target.sql", + ); + expect(warm.shadowSpawned.filter((c) => c.args[0] === "rm")).toHaveLength(1); + const target = join(tmp.current, "supabase", "migrations", "1_target.sql"); + expect(readFileSync(target, "utf8")).toBe(EXPECTED_TARGET); + }), + ), + ); + + it.effect("keys its snapshots under the cache root, not the project directory", () => + withCacheEnabled( + Effect.gen(function* () { + yield* runSquash(); + // The production path helper resolving the `SUPABASE_HOME` `withCacheEnabled` pinned, + // not a hand-built join — so this stays honest if the cache root ever moves. + const cacheDir = legacyShadowBaselineCacheDir(yield* Path.Path); + const tars = readdirSync(cacheDir).filter((name) => name.endsWith(".tar")); + expect(tars).toHaveLength(1); + expect(tars[0]).toMatch(/^shadow-baseline-[0-9a-f]+\.tar$/); + expect(existsSync(join(tmp.current, "supabase", ".temp", "shadow-baseline"))).toBe(false); + }).pipe(Effect.provide(BunServices.layer)), + ), + ); + }); + // Failure paths — every one leaves the shadow removed (unless creation // itself is what failed, matching the established leak-on-create-failure behavior). @@ -903,7 +1009,7 @@ describe("legacy migration squash", () => { }); it.effect( - "fails with a health-check timeout when the shadow never becomes healthy, and removes it", + "fails with a health-check timeout when the shadow never becomes connectable, and removes it", () => { seedMigration(tmp.current, "0_init.sql"); seedMigration(tmp.current, "1_target.sql"); @@ -914,7 +1020,7 @@ describe("legacy migration squash", () => { join(tmp.current, "supabase", "config.toml"), '[db]\nhealth_timeout = "0s"\n', ); - const s = setup(tmp.current, { neverHealthyShadow: true }); + const s = setup(tmp.current, { neverConnectableShadow: true }); return Effect.gen(function* () { const exit = yield* legacyMigrationSquash(flags()).pipe(Effect.exit); expect(failureTag(exit)).toBe("LegacyHealthCheckTimeoutError"); diff --git a/apps/cli/src/legacy/commands/test/db/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/test/db/SIDE_EFFECTS.md index 51d3041156..a81a3d0c2d 100644 --- a/apps/cli/src/legacy/commands/test/db/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/test/db/SIDE_EFFECTS.md @@ -61,6 +61,7 @@ One-shot `docker run --rm `, where the image is `supabase/pg_pro | ---- | ---------------------------------------------------------------------------------------------------- | | `0` | all pgTAP tests pass | | `1` | `pg_prove` exits non-zero (test failures) — `error running container: exit N` | +| `1` | `pg_prove` ran no tests (`Result: NOTESTS`) — `no pgTAP tests found in `; Go exits `0` here | | `1` | `--db-url` / `--linked` / `--local` set together (mutually exclusive) | | `1` | database connection failure / pgTAP enable failure / docker failure / `--linked` auth or IPv6 errors | | `1` | `--project-ref` set with a resolved target other than linked (see Notes) | @@ -82,8 +83,8 @@ invocation path. ## Output `pg_prove`'s TAP output streams to **stdout in every output format** (the docker -subprocess inherits stdout) — `test db` is a live test -stream with no structured equivalent. +subprocess's stdout is forwarded chunk-by-chunk, byte-exact and unframed) — +`test db` is a live test stream with no structured equivalent. ### `--output-format text` diff --git a/apps/cli/src/legacy/output/legacy-quiet-progress-text-output.layer.unit.test.ts b/apps/cli/src/legacy/output/legacy-quiet-progress-text-output.layer.unit.test.ts index cbab687f78..107fb61c48 100644 --- a/apps/cli/src/legacy/output/legacy-quiet-progress-text-output.layer.unit.test.ts +++ b/apps/cli/src/legacy/output/legacy-quiet-progress-text-output.layer.unit.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "@effect/vitest"; import { beforeEach, vi } from "vitest"; -import { Effect, Layer } from "effect"; +import { Effect, Layer, Stdio } from "effect"; import { mockTty } from "../../../tests/helpers/mocks.ts"; import { Output } from "../../shared/output/output.service.ts"; @@ -65,7 +65,7 @@ beforeEach(() => { describe("legacyQuietProgressTextOutputLayer", () => { const layer = legacyQuietProgressTextOutputLayer.pipe( - Layer.provide(mockTty({ stdoutIsTty: true })), + Layer.provide(Layer.mergeAll(mockTty({ stdoutIsTty: true }), Stdio.layerTest({}))), ); it.effect("never starts a spinner, even after the spinner delay elapses", () => diff --git a/apps/cli/src/legacy/shared/db-bootstrap/shadow-database.ts b/apps/cli/src/legacy/shared/db-bootstrap/shadow-database.ts index c0c640894d..db1e4702b5 100644 --- a/apps/cli/src/legacy/shared/db-bootstrap/shadow-database.ts +++ b/apps/cli/src/legacy/shared/db-bootstrap/shadow-database.ts @@ -723,22 +723,84 @@ export const legacyBuildShadowSetupDatabaseInput = ( }); /** - * Port of Go's `SetupShadowDatabase` (`apps/cli-go/internal/db/diff/diff.go:181-193`): - * connects to the shadow (Go's `ConnectShadowDatabase`, {@link legacyConnectShadowDatabase}) - * FIRST, THEN resolves the setup prelude (JWKS/pinned image names, {@link - * legacyResolveDbSetupPrelude}) and runs {@link legacySetupShadowConn} — the platform - * baseline plus the template database, no user migrations. Connect-then-setup, matching Go's - * own `SetupShadowDatabase` (which dials `ConnectShadowDatabase` before ever calling - * `start.SetupDatabase`, `diff.go:186-192`) and this same module's `legacyRunFreshDbSetup` - * (`db-setup.ts`) for the real local `db` container: an unconnectable shadow must surface a - * connect error immediately, not pay for JWKS work first. The connection is closed once this - * resolves (Go's `defer conn.Close(...)`), matching `Effect.scoped`'s finalizer running at the - * end of this function rather than leaking a `Scope.Scope` requirement to the caller. + * Go's `SetupShadowDatabase`/`MigrateShadowDatabase` shared prologue: connect to the shadow + * (Go's `ConnectShadowDatabase`, {@link legacyConnectShadowDatabase}), resolve the setup prelude + * (JWKS/pinned image names, {@link legacyResolveDbSetupPrelude}), run the platform baseline + * ({@link legacySetupDatabase}) — and hand the caller back the STILL-OPEN session everything + * after the baseline runs on. Connect-then-setup, matching Go's own `SetupShadowDatabase` + * (which dials `ConnectShadowDatabase` before ever calling `start.SetupDatabase`, + * `diff.go:186-192`) and this same module's `legacyRunFreshDbSetup` (`db-setup.ts`) for the real + * local `db` container: an unconnectable shadow must surface a connect error immediately, not + * pay for JWKS work first. + * + * The returned session's lifetime is the CALLER's enclosing `Scope.Scope` (Go's + * `defer conn.Close(...)`), which is why this function leaks that requirement instead of + * wrapping itself in `Effect.scoped`. * * `baseline` defaults to {@link LEGACY_SHADOW_BASELINE_COLD}, i.e. exactly the sequence above. - * A warm shadow-cache hit skips the prelude + `SetupDatabase` (the restored cluster already - * has them) and only recreates `contrib_regression`; a cache-enabled COLD provision snapshots - * between the baseline and the template, matching {@link migrateShadowDatabase}. + * A warm shadow-cache hit skips the prelude + `SetupDatabase` entirely (the restored cluster + * already carries them); a cache-enabled COLD provision runs the baseline in its OWN scope, so + * its session is closed before {@link LegacyShadowBaselineState.snapshotBaseline} stops the + * container, and returns a second session opened against the restarted one. + * + * Shared by all three baseline-running shadow compositions — {@link legacySetupShadowDatabase} + * and {@link migrateShadowDatabase} here, plus `migration squash`'s own dump/apply/dump sequence + * (`migration/squash/squash.handler.ts`), which needs the baseline WITHOUT the template database + * (see {@link legacySetupShadowConn}'s own doc comment) and keeps the session open across its + * mid-sequence `pg_dump`s. + */ +export const legacyOpenShadowBaselineSession = ( + spawner: Spawner, + input: LegacyShadowSetupRunInput, + options: LegacySetupDatabaseOptions = {}, + baseline: LegacyShadowBaselineState = LEGACY_SHADOW_BASELINE_COLD, +): Effect.Effect< + LegacyDbSession, + LegacyStartSetupLocalDatabaseError | LegacyShadowDbError | LegacyImagePrepullError | E, + Output | LegacyDockerRun | RuntimeInfo | LegacyDbConnection | Scope.Scope +> => + Effect.gen(function* () { + if (!baseline.baselinePresent && baseline.snapshotRequired) { + // Own scope: the baseline session must be closed before `snapshotBaseline` — see + // {@link LegacyShadowBaselineState.snapshotRequired}. + yield* Effect.scoped( + Effect.gen(function* () { + const setupSession = yield* legacyConnectShadowDatabase(input.connConfig); + const resolved = yield* legacyResolveDbSetupPrelude(input.setup); + yield* legacySetupDatabase( + spawner, + legacyBuildShadowSetupDatabaseInput(input, setupSession, resolved), + options, + ); + }), + ); + yield* baseline.snapshotBaseline; + } + const session = yield* legacyConnectShadowDatabase(input.connConfig); + if (!baseline.baselinePresent && !baseline.snapshotRequired) { + // Go's single-connection flow, verbatim: baseline and everything after it on this one + // session — see {@link LegacyShadowBaselineState.snapshotRequired}. + const resolved = yield* legacyResolveDbSetupPrelude(input.setup); + yield* legacySetupDatabase( + spawner, + legacyBuildShadowSetupDatabaseInput(input, session, resolved), + options, + ); + } + return session; + }); + +/** + * Port of Go's `SetupShadowDatabase` (`apps/cli-go/internal/db/diff/diff.go:181-193`): + * {@link legacyOpenShadowBaselineSession} (connect + platform baseline) followed by the template + * database — together Go's `setupShadowConn`, no user migrations. The connection is closed once + * this resolves (Go's `defer conn.Close(...)`), matching `Effect.scoped`'s finalizer running at + * the end of this function rather than leaking a `Scope.Scope` requirement to the caller. + * + * `baseline` defaults to {@link LEGACY_SHADOW_BASELINE_COLD}. A warm shadow-cache hit skips the + * prelude + `SetupDatabase` (the restored cluster already has them) and only recreates + * `contrib_regression`; a cache-enabled COLD provision snapshots between the baseline and the + * template, matching {@link migrateShadowDatabase}. */ export const legacySetupShadowDatabase = ( spawner: Spawner, @@ -752,29 +814,7 @@ export const legacySetupShadowDatabase = ( > => Effect.scoped( Effect.gen(function* () { - if (!baseline.baselinePresent && baseline.snapshotRequired) { - yield* Effect.scoped( - Effect.gen(function* () { - const setupSession = yield* legacyConnectShadowDatabase(input.connConfig); - const resolved = yield* legacyResolveDbSetupPrelude(input.setup); - yield* legacySetupDatabase( - spawner, - legacyBuildShadowSetupDatabaseInput(input, setupSession, resolved), - options, - ); - }), - ); - yield* baseline.snapshotBaseline; - } - const session = yield* legacyConnectShadowDatabase(input.connConfig); - if (!baseline.baselinePresent && !baseline.snapshotRequired) { - const resolved = yield* legacyResolveDbSetupPrelude(input.setup); - yield* legacySetupDatabase( - spawner, - legacyBuildShadowSetupDatabaseInput(input, session, resolved), - options, - ); - } + const session = yield* legacyOpenShadowBaselineSession(spawner, input, options, baseline); yield* legacyCreateShadowTemplateDatabase(session); }), ); @@ -853,14 +893,9 @@ export const LEGACY_SHADOW_BASELINE_COLD: LegacyShadowBaselineState = { * the template database and the user migrations; a COLD cache-enabled provision passes the same * cold sequence plus a `snapshotBaseline` step between the baseline and the template database. * - * The one structural divergence from Go is confined to the SNAPSHOTTING cold branch - * (`baseline.snapshotRequired`): there the baseline runs in its own scope, its session is CLOSED - * before {@link LegacyShadowBaselineState.snapshotBaseline} (the disk-level PGDATA snapshot stops - * the container, which severs any live backend), and the template database + migrations run on a - * second session. Every OTHER state — uncached (cache off / `--no-cache` / OrioleDB) and warm — - * uses exactly one session, matching Go's single connection: see - * {@link LegacyShadowBaselineState.snapshotRequired} for why the split must not leak into the - * uncached path. The SQL every path issues is unchanged. + * The one structural divergence from Go is confined to the SNAPSHOTTING cold branch and is owned + * by {@link legacyOpenShadowBaselineSession} — see its doc comment. The SQL every path issues is + * unchanged. */ const migrateShadowDatabase = ( spawner: Spawner, @@ -885,33 +920,12 @@ const migrateShadowDatabase = ( ), ); - if (!baseline.baselinePresent && baseline.snapshotRequired) { - // Own scope: the baseline session must be closed before `snapshotBaseline` — see this - // function's own doc comment. - yield* Effect.scoped( - Effect.gen(function* () { - const setupSession = yield* legacyConnectShadowDatabase(input.connConfig); - const resolved = yield* legacyResolveDbSetupPrelude(input.setup); - yield* legacySetupDatabase( - spawner, - legacyBuildShadowSetupDatabaseInput(input, setupSession, resolved), - setupOptions, - ); - }), - ); - yield* baseline.snapshotBaseline; - } - const session = yield* legacyConnectShadowDatabase(input.connConfig); - if (!baseline.baselinePresent && !baseline.snapshotRequired) { - // Go's single-connection flow, verbatim: baseline + template + migrations all on this - // one session — see this function's own doc comment. - const resolved = yield* legacyResolveDbSetupPrelude(input.setup); - yield* legacySetupDatabase( - spawner, - legacyBuildShadowSetupDatabaseInput(input, session, resolved), - setupOptions, - ); - } + const session = yield* legacyOpenShadowBaselineSession( + spawner, + input, + setupOptions, + baseline, + ); yield* legacyCreateShadowTemplateDatabase(session); yield* legacyApplyMigrations( session, diff --git a/apps/cli/src/legacy/shared/legacy-docker-run.layer.ts b/apps/cli/src/legacy/shared/legacy-docker-run.layer.ts index fd77fe9fe0..73002bf41b 100644 --- a/apps/cli/src/legacy/shared/legacy-docker-run.layer.ts +++ b/apps/cli/src/legacy/shared/legacy-docker-run.layer.ts @@ -113,6 +113,7 @@ export const legacyDockerRunLayer: Layer.Layer< Effect.scoped( Effect.gen(function* () { const teeStderr = streamOpts.teeStderr ?? false; + const captureStderr = streamOpts.captureStderr ?? true; yield* processControl.holdSignals(["SIGINT", "SIGTERM", "SIGHUP"]); const resolvedOpts = yield* withResolvedImage(opts); const args = buildLegacyDockerArgs( @@ -142,7 +143,9 @@ export const legacyDockerRunLayer: Layer.Layer< ), Stream.runForEach(handle.stderr, (chunk) => Effect.sync(() => { - stderrChunks.push(chunk); + // Retained only for the returned string — skipped when the caller + // opts out, so a tee-only consumer stays at constant memory. + if (captureStderr) stderrChunks.push(chunk); if (teeStderr) globalThis.process.stderr.write(chunk); }), ).pipe(Effect.mapError(spawnError)), diff --git a/apps/cli/src/legacy/shared/legacy-docker-run.service.ts b/apps/cli/src/legacy/shared/legacy-docker-run.service.ts index 80561b88d7..10950df25a 100644 --- a/apps/cli/src/legacy/shared/legacy-docker-run.service.ts +++ b/apps/cli/src/legacy/shared/legacy-docker-run.service.ts @@ -99,12 +99,18 @@ interface LegacyDockerRunShape { * propagates as `E`. `teeStderr` mirrors `runCapture` (Go's * `io.MultiWriter(os.Stderr, errBuf)`). Returns the exit code + captured stderr; the * stdout bytes are not retained. + * + * `captureStderr` (default `true`) buffers stderr for that returned string. Callers + * that only tee it and never read the result should pass `false` — retaining it + * grows with the container's total stderr, which the inherited-stdio `run` never did + * (`test db`, whose pgTAP suites can emit unbounded psql notices). */ readonly runStream: ( opts: LegacyDockerRunOpts, streamOpts: { readonly onStdout: (chunk: Uint8Array) => Effect.Effect; readonly teeStderr?: boolean; + readonly captureStderr?: boolean; }, ) => Effect.Effect< { readonly exitCode: number; readonly stderr: string }, diff --git a/apps/cli/src/legacy/shared/legacy-pgdelta.cache.ts b/apps/cli/src/legacy/shared/legacy-pgdelta.cache.ts index 4eb27a598a..3e4bf7b918 100644 --- a/apps/cli/src/legacy/shared/legacy-pgdelta.cache.ts +++ b/apps/cli/src/legacy/shared/legacy-pgdelta.cache.ts @@ -888,6 +888,13 @@ export const legacyResolveMigrationsCatalogRef = Effect.fnUntraced(function* ( timestamp, ); }), + // Same forced-on policy {@link legacyGetMigrationsCatalogRef} passes below, for the same + // reason: `legacyProvisionMigrationsShadow` migrates through `legacyMigrateShadowDatabase`, + // whose baseline installs `pg_net` regardless of `config.toml`. Leaving this at the + // config-following default would key the published tar as `webhooks_enabled=false` on a + // webhooks-disabled project even though the snapshotted cluster HAS `pg_net` — poisoning + // every other config-following consumer of that key. + { webhooks: "enabled" }, ); }); diff --git a/apps/cli/src/legacy/shared/legacy-test-db.command-handler.ts b/apps/cli/src/legacy/shared/legacy-test-db.command-handler.ts index 151144595f..a15e437f29 100644 --- a/apps/cli/src/legacy/shared/legacy-test-db.command-handler.ts +++ b/apps/cli/src/legacy/shared/legacy-test-db.command-handler.ts @@ -6,7 +6,7 @@ import { withJsonErrorHandling } from "../../shared/output/json-error-handling.t import { Output } from "../../shared/output/output.service.ts"; import { ProcessControl } from "../../shared/runtime/process-control.service.ts"; import { withLegacyCommandInstrumentation } from "../telemetry/legacy-command-instrumentation.ts"; -import { LegacyTestDbRunError } from "./legacy-test-db.errors.ts"; +import type { LegacyTestDbNoTestsError, LegacyTestDbRunError } from "./legacy-test-db.errors.ts"; import { legacyTestDb } from "./legacy-test-db.handler.ts"; /** @@ -21,13 +21,14 @@ export const LEGACY_TEST_DB_SHORT = "Tests local database with pgTAP"; /** * `test db` has no machine-format envelope: its entire output is the streamed * pg_prove TAP on stdout (Go has no `--output-format` for it). On a *run* failure - * (failing tests), the default `withJsonErrorHandling` would append a JSON error - * object to stdout — after the TAP already streamed — corrupting machine consumers. + * (failing tests, or a run that executed none), the default `withJsonErrorHandling` + * would append a JSON error object to stdout — after the TAP already streamed — + * corrupting machine consumers. * So in json/stream-json mode, send the diagnostic to stderr and exit 1 instead, * matching Go's `recoverAndExit` (stderr, exit 1). Text mode keeps the normal error * rendering; pre-stream errors still flow through `withJsonErrorHandling`. */ -const onRunFailure = (error: LegacyTestDbRunError) => +const onRunFailure = (error: LegacyTestDbRunError | LegacyTestDbNoTestsError) => Effect.gen(function* () { const output = yield* Output; if (output.format === "text") return yield* Effect.fail(error); @@ -114,9 +115,10 @@ export function legacyRunTestDbCommand( // --project-ref registrations (cmd/pgdelta_catalog.go:44 and most // others) are unmarked, so it stays redacted. }), - // Run failures (failing tests) must not corrupt the TAP stream on stdout in - // machine modes; other errors (pre-stream) still get the JSON envelope. - Effect.catchTag("LegacyTestDbRunError", onRunFailure), + // Run failures (failing tests, or a run that executed none) must not corrupt the + // TAP stream on stdout in machine modes; other errors (pre-stream) still get the + // JSON envelope. + Effect.catchTag(["LegacyTestDbRunError", "LegacyTestDbNoTestsError"], onRunFailure), withJsonErrorHandling, ); } diff --git a/apps/cli/src/legacy/shared/legacy-test-db.errors.ts b/apps/cli/src/legacy/shared/legacy-test-db.errors.ts index 42b19185bc..3d3b630e64 100644 --- a/apps/cli/src/legacy/shared/legacy-test-db.errors.ts +++ b/apps/cli/src/legacy/shared/legacy-test-db.errors.ts @@ -32,6 +32,19 @@ export class LegacyTestDbRunError extends Data.TaggedError("LegacyTestDbRunError } } +/** + * `pg_prove` ran but found nothing to execute. It reports that as `Result: NOTESTS` + * and still exits 0, so without this the command reports success for a run that + * executed zero tests (CLI-2194). + */ +export class LegacyTestDbNoTestsError extends Data.TaggedError("LegacyTestDbNoTestsError")<{ + readonly message: string; +}> { + get [ErrorActionabilityId](): CliErrorActionabilityDeclaration { + return actionability.invalidInput; + } +} + /** * More than one of `--db-url` / `--linked` / `--local` was set. Reproduces * cobra's `MarkFlagsMutuallyExclusive("db-url", "linked", "local")` error from diff --git a/apps/cli/src/legacy/shared/legacy-test-db.handler.ts b/apps/cli/src/legacy/shared/legacy-test-db.handler.ts index d59fff22be..5663823618 100644 --- a/apps/cli/src/legacy/shared/legacy-test-db.handler.ts +++ b/apps/cli/src/legacy/shared/legacy-test-db.handler.ts @@ -21,6 +21,7 @@ import type { LegacyTestDbFlags } from "./legacy-test-db.command-handler.ts"; import { LegacyTestDbEnablePgtapError, LegacyTestDbMutuallyExclusiveFlagsError, + LegacyTestDbNoTestsError, LegacyTestDbRunError, } from "./legacy-test-db.errors.ts"; import { buildLegacyPgProveArgs } from "./legacy-test-db.pg-prove-args.ts"; @@ -33,9 +34,30 @@ const DISABLE_PGTAP = "drop extension if exists pgtap"; // The TS config schema does not model an `[images]` override, so it is fixed here. // Go resolves it through `GetRegistryImageUrl` (`DockerStart`), honoring // `SUPABASE_INTERNAL_IMAGE_REGISTRY` / the default ECR mirror, so do the same -// before passing it to `docker run`. +// before passing it to `docker run`. Re-verify `NO_TESTS_VERDICT` still matches +// when bumping this tag. const LEGACY_PG_PROVE_IMAGE = "supabase/pg_prove:3.36"; const MAX_PROJECT_ID_LENGTH = 40; +/** + * `TAP::Harness` closes every run with exactly one `Result: ` line. + * `pg_prove` still exits 0 for the empty-run verdict, so "found nothing to run" is + * otherwise indistinguishable from "everything passed" — a typo'd path, an empty + * directory, or a bind the daemon resolved against a different filesystem than the + * CLI's (a sibling-container Docker socket) all report a green build that ran zero + * tests (CLI-2194). + * + * Only the harness's FINAL verdict decides: under `--debug` (`--verbose`) the + * harness replays each test's raw TAP, and a passing test may legally print its own + * `Result: …` line, which must not be mistaken for the run's outcome. Matching the + * harness's human summary is a heuristic pinned to the image tag above. + * + * The verdict alone is not enough: a suite that deliberately skips itself + * (`1..0 # SKIP …`) also ends `NOTESTS`, but reports `Files=1`. Only a run that + * aggregated ZERO files found nothing to run, so both signals must agree. + */ +const VERDICT_PREFIX = "Result: "; +const NO_TESTS_VERDICT = "Result: NOTESTS"; +const FILES_SUMMARY = /^Files=(\d+),/; /** Port of Go's `sanitizeProjectId` (`pkg/config/config.go:1037`). */ function sanitizeProjectId(src: string): string { @@ -138,10 +160,16 @@ export const legacyTestDb = Effect.fn("legacy.test.db")(function* (flags: Legacy }) : { _tag: "host" as const }; - const exitCode = yield* Effect.scoped( + const decoder = new TextDecoder(); + // The trailing partial line, plus the last complete summary/verdict lines so far. + let pendingLine = ""; + let lastVerdict = ""; + let lastSummary = ""; + + const { exitCode } = yield* Effect.scoped( Effect.gen(function* () { - // stdout is reserved for the pg_prove TAP stream (the docker subprocess - // writes it there directly), so connection diagnostics must go to stderr — + // stdout is reserved for the pg_prove TAP stream (forwarded byte-exact + // below), so connection diagnostics must go to stderr — // exactly as Go does (`ConnectByConfigStream` writes "Connecting to … // database…" to `os.Stderr`, `connect.go:205-228`). A `Output.task` // spinner would corrupt the TAP stream: clack writes spinner ANSI to @@ -187,16 +215,40 @@ export const legacyTestDb = Effect.fn("legacy.test.db")(function* (flags: Legacy // Windows Docker Desktop provide the mapping natively (empty there). const extraHosts = runtimeInfo.platform === "linux" ? ["host.docker.internal:host-gateway"] : []; - return yield* docker.run({ - image: legacyGetRegistryImageUrl(LEGACY_PG_PROVE_IMAGE), - cmd: args.cmd, - env: runEnv, - binds: args.binds, - workingDir: args.workingDir, - securityOpt: inBitbucket ? [] : ["label:disable"], - extraHosts, - network, - }); + // Stream (rather than inherit) stdout so the verdict can be read on the way + // past; every chunk is forwarded byte-exact and unframed, leaving the TAP + // stream identical to what the container wrote. stderr is teed live, as + // inheriting it did. + return yield* docker.runStream( + { + image: legacyGetRegistryImageUrl(LEGACY_PG_PROVE_IMAGE), + cmd: args.cmd, + env: runEnv, + binds: args.binds, + workingDir: args.workingDir, + securityOpt: inBitbucket ? [] : ["label:disable"], + extraHosts, + network, + }, + { + onStdout: (chunk) => + Effect.suspend(() => { + // Split on newlines, carrying the incomplete trailing line into the + // next chunk so a verdict straddling a chunk boundary is still seen. + const lines = (pendingLine + decoder.decode(chunk, { stream: true })).split("\n"); + pendingLine = lines.pop() ?? ""; + for (const line of lines) { + if (line.startsWith(VERDICT_PREFIX)) lastVerdict = line; + else if (FILES_SUMMARY.test(line)) lastSummary = line; + } + return output.rawBytes(chunk, "stdout"); + }), + // Teed straight to the terminal as inheriting it did; nothing here reads + // the buffered copy, and a pgTAP suite's psql notices are unbounded. + teeStderr: true, + captureStderr: false, + }, + ); }), ); @@ -212,5 +264,16 @@ export const legacyTestDb = Effect.fn("legacy.test.db")(function* (flags: Legacy new LegacyTestDbRunError({ message: `error running container: exit ${exitCode}` }), ); } + + // A stream that ends without a trailing newline leaves the verdict unterminated. + const finalVerdict = pendingLine.startsWith(VERDICT_PREFIX) ? pendingLine : lastVerdict; + const aggregatedFiles = FILES_SUMMARY.exec(lastSummary)?.[1]; + if (finalVerdict.trimEnd() === NO_TESTS_VERDICT && aggregatedFiles === "0") { + return yield* Effect.fail( + new LegacyTestDbNoTestsError({ + message: `no pgTAP tests found in ${args.hostPaths.join(", ")}`, + }), + ); + } }).pipe(Effect.ensuring(telemetryState.flush)); }); diff --git a/apps/cli/src/legacy/shared/legacy-test-db.integration.test.ts b/apps/cli/src/legacy/shared/legacy-test-db.integration.test.ts index d9a13b2782..359340ff9e 100644 --- a/apps/cli/src/legacy/shared/legacy-test-db.integration.test.ts +++ b/apps/cli/src/legacy/shared/legacy-test-db.integration.test.ts @@ -115,8 +115,16 @@ function mockDbConnection(opts: { }; } -function mockDockerRun(opts: { exitCode?: number; runFails?: boolean }) { +function mockDockerRun(opts: { + exitCode?: number; + runFails?: boolean; + /** pg_prove's stdout, delivered to `onStdout` one array entry per chunk. */ + stdout?: ReadonlyArray; +}) { let lastOpts: LegacyDockerRunOpts | undefined; + let lastStreamOpts: + | { readonly teeStderr?: boolean; readonly captureStderr?: boolean } + | undefined; const layer = Layer.succeed(LegacyDockerRun, { run: (runOpts) => { lastOpts = runOpts; @@ -142,8 +150,9 @@ function mockDockerRun(opts: { exitCode?: number; runFails?: boolean }) { ) : Effect.succeed({ exitCode: opts.exitCode ?? 0, stdout: new Uint8Array(0), stderr: "" }); }, - runStream: (runOpts) => { + runStream: (runOpts, streamOpts) => { lastOpts = runOpts; + lastStreamOpts = streamOpts; return opts.runFails === true ? Effect.fail( new LegacyDockerRunError({ @@ -152,7 +161,13 @@ function mockDockerRun(opts: { exitCode?: number; runFails?: boolean }) { daemonDown: false, }), ) - : Effect.succeed({ exitCode: opts.exitCode ?? 0, stderr: "" }); + : Effect.gen(function* () { + const encoder = new TextEncoder(); + for (const chunk of opts.stdout ?? []) { + yield* streamOpts.onStdout(encoder.encode(chunk)); + } + return { exitCode: opts.exitCode ?? 0, stderr: "" }; + }); }, }); return { @@ -160,6 +175,9 @@ function mockDockerRun(opts: { exitCode?: number; runFails?: boolean }) { get lastOpts() { return lastOpts; }, + get lastStreamOpts() { + return lastStreamOpts; + }, }; } @@ -184,6 +202,7 @@ interface SetupOpts { dropFails?: boolean; exitCode?: number; runFails?: boolean; + stdout?: ReadonlyArray; debug?: boolean; networkId?: string; workdir?: string; @@ -408,6 +427,115 @@ describe("legacy test db integration", () => { }).pipe(Effect.provide(layer)); }); + it.live("fails when pg_prove ran no tests, even though it exited 0 (CLI-2194)", () => { + const { layer } = setup({ + exitCode: 0, + stdout: ["Files=0, Tests=0, 0 wallclock secs\nResult: NOTESTS\n"], + }); + return Effect.gen(function* () { + const exit = yield* Effect.exit(legacyTestDb(flags({ paths: ["tests/db"] }))); + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + expect(JSON.stringify(exit.cause)).toContain( + "no pgTAP tests found in /work/project/tests/db", + ); + } + }).pipe(Effect.provide(layer)); + }); + + it.live("detects the NOTESTS verdict when it straddles a stdout chunk boundary", () => { + const { layer } = setup({ exitCode: 0, stdout: ["Files=0, Tests=0\nResult: NOTE", "STS\n"] }); + return Effect.gen(function* () { + const exit = yield* Effect.exit(legacyTestDb(flags())); + expect(Exit.isFailure(exit)).toBe(true); + }).pipe(Effect.provide(layer)); + }); + + it.live("detects the NOTESTS verdict arriving one byte per chunk", () => { + const { layer } = setup({ + exitCode: 0, + stdout: [..."Files=0, Tests=0\nResult: NOTESTS\n"], + }); + return Effect.gen(function* () { + const exit = yield* Effect.exit(legacyTestDb(flags())); + expect(Exit.isFailure(exit)).toBe(true); + }).pipe(Effect.provide(layer)); + }); + + it.live("detects the NOTESTS verdict when the stream ends without a trailing newline", () => { + const { layer } = setup({ exitCode: 0, stdout: ["Files=0, Tests=0\nResult: NOTESTS"] }); + return Effect.gen(function* () { + const exit = yield* Effect.exit(legacyTestDb(flags())); + expect(Exit.isFailure(exit)).toBe(true); + }).pipe(Effect.provide(layer)); + }); + + it.live("passes a suite that deliberately skips itself, which also ends NOTESTS", () => { + // `1..0 # SKIP …` reports `Files=1, Tests=0` + `Result: NOTESTS` and exits 0. A + // file WAS found, so this is a successful run, not an empty one (PR #6210 review). + const { layer } = setup({ + exitCode: 0, + stdout: [ + "skip.test.sql .. skipped: not applicable on this platform\n", + "Files=1, Tests=0, 0 wallclock secs\nResult: NOTESTS\n", + ], + }); + return Effect.gen(function* () { + const exit = yield* Effect.exit(legacyTestDb(flags())); + expect(Exit.isSuccess(exit)).toBe(true); + }).pipe(Effect.provide(layer)); + }); + + it.live("does not trip on the verdict text mid-line, as a --verbose replay can emit", () => { + const { layer } = setup({ + exitCode: 0, + stdout: ["# diag: Result: NOTESTS is what an empty run prints\n", "Result: PASS\n"], + }); + return Effect.gen(function* () { + const exit = yield* Effect.exit(legacyTestDb(flags())); + expect(Exit.isSuccess(exit)).toBe(true); + }).pipe(Effect.provide(layer)); + }); + + it.live("takes the harness's final verdict, not a passing test's own Result: line", () => { + // `--debug` replays each test's raw TAP, and a passing test may legally print a + // line of its own starting `Result: NOTESTS…`. Only the harness's last verdict + // decides the run (PR #6210 review). + const { layer } = setup({ + exitCode: 0, + stdout: [ + "ok 1 - passes\n", + "Result: NOTESTS is diagnostic text\n", + "ok 2 - passes\n", + "Files=1, Tests=2, 0 wallclock secs\nResult: PASS\n", + ], + }); + return Effect.gen(function* () { + const exit = yield* Effect.exit(legacyTestDb(flags())); + expect(Exit.isSuccess(exit)).toBe(true); + }).pipe(Effect.provide(layer)); + }); + + it.live("tees container stderr without retaining it, as inheriting stdio did", () => { + // A pgTAP suite's psql notices are unbounded; buffering them for a string nothing + // reads would grow with the whole run (PR #6210 review). + const { layer, docker } = setup(); + return Effect.gen(function* () { + yield* legacyTestDb(flags()); + expect(docker.lastStreamOpts?.teeStderr).toBe(true); + expect(docker.lastStreamOpts?.captureStderr).toBe(false); + }).pipe(Effect.provide(layer)); + }); + + it.live("forwards the TAP stream to stdout byte-exact and succeeds on a passing run", () => { + const chunks = ["a.test.sql .. ok\n", "All tests successful.\n", "Result: PASS\n"]; + const { layer, out } = setup({ exitCode: 0, stdout: chunks }); + return Effect.gen(function* () { + yield* legacyTestDb(flags()); + expect(out.stdoutText).toBe(chunks.join("")); + }).pipe(Effect.provide(layer)); + }); + it.live("fails when docker itself cannot run", () => { const { layer } = setup({ runFails: true }); return Effect.gen(function* () { diff --git a/apps/cli/src/legacy/shared/legacy-test-db.pg-prove-args.ts b/apps/cli/src/legacy/shared/legacy-test-db.pg-prove-args.ts index fd8ec0a2cf..c7b856f0e2 100644 --- a/apps/cli/src/legacy/shared/legacy-test-db.pg-prove-args.ts +++ b/apps/cli/src/legacy/shared/legacy-test-db.pg-prove-args.ts @@ -8,6 +8,12 @@ export interface LegacyPgProveArgs { readonly cmd: ReadonlyArray; /** Docker volume binds, each `hostpath:dockerpath:ro`. */ readonly binds: ReadonlyArray; + /** + * The searched paths as they exist on the *host*, for diagnostics. Deliberately + * not the `legacyToDockerPath` form used in `cmd`: on Windows that has the volume + * name stripped, so an error naming it would point at a path the user does not have. + */ + readonly hostPaths: ReadonlyArray; /** Container working directory (dir of the first test path). */ readonly workingDir: Option.Option; } @@ -39,6 +45,7 @@ export function buildLegacyPgProveArgs(opts: { const cmd: string[] = ["pg_prove", "--ext", ".pg", "--ext", ".sql", "-r"]; const binds: string[] = []; + const hostPaths: string[] = []; const seenTargets = new Set(); // `testFiles` is never empty (it defaults to supabase/tests), so the first // iteration always sets this; Go derives workingDir from the first path only. @@ -48,6 +55,7 @@ export function buildLegacyPgProveArgs(opts: { const fp = nodePath.isAbsolute(candidate) ? candidate : nodePath.join(opts.cwd, candidate); const dockerPath = legacyToDockerPath(fp); cmd.push(dockerPath); + hostPaths.push(fp); // Mount the *directory* containing a test file (not the lone file) so psql // `\ir ./sibling.sql` includes resolve: they look relative to the test file's @@ -70,5 +78,5 @@ export function buildLegacyPgProveArgs(opts: { if (opts.debug) cmd.push("--verbose"); - return { cmd, binds, workingDir: Option.some(workingDir) }; + return { cmd, binds, hostPaths, workingDir: Option.some(workingDir) }; } diff --git a/apps/cli/src/legacy/shared/legacy-test-db.pg-prove-args.unit.test.ts b/apps/cli/src/legacy/shared/legacy-test-db.pg-prove-args.unit.test.ts index 0d46f6ce9c..3d4e069e48 100644 --- a/apps/cli/src/legacy/shared/legacy-test-db.pg-prove-args.unit.test.ts +++ b/apps/cli/src/legacy/shared/legacy-test-db.pg-prove-args.unit.test.ts @@ -90,6 +90,8 @@ describe("buildLegacyPgProveArgs", () => { ]); // workingDir is derived from the first path only (a file → its parent). expect(Option.getOrNull(result.workingDir)).toBe("/abs"); + // `hostPaths` reports what pg_prove searches — the files/dirs, not their mounts. + expect(result.hostPaths).toEqual(["/abs/first_test.sql", "/abs/second/dir"]); }); test("appends --verbose when debug is enabled", () => { diff --git a/apps/cli/src/shared/output/output.layer.ts b/apps/cli/src/shared/output/output.layer.ts index cf21dd5189..bf2a987284 100644 --- a/apps/cli/src/shared/output/output.layer.ts +++ b/apps/cli/src/shared/output/output.layer.ts @@ -41,6 +41,18 @@ function formatTaskMessage(message: string | undefined): string | undefined { return [firstLine, ...rest.map((line) => `${guide}${line}`)].join("\n"); } +/** + * Shared by all three layers. The sink waits for `drain`; `process.stdout.write` + * does not, so a streamed payload piped to a slow consumer buffers in memory. + */ +const stdioWriter = + (stdio: typeof Stdio.Stdio.Service) => + (chunk: string | Uint8Array, stream: "stdout" | "stderr" = "stdout") => + Stream.make(chunk).pipe( + Stream.run(stream === "stderr" ? stdio.stderr() : stdio.stdout()), + Effect.orDie, + ); + /** * Output layers - Concrete output mode implementations for the CLI. * @@ -51,6 +63,8 @@ export const textOutputLayer = Layer.effect( Output, Effect.gen(function* () { const tty = yield* Tty; + const write = stdioWriter(yield* Stdio.Stdio); + const DEFAULT_AUTOCOMPLETE_THRESHOLD = 10; const buildSelectOptions = ( options: ReadonlyArray<{ @@ -379,22 +393,8 @@ export const textOutputLayer = Layer.effect( ); } }), - raw: (text: string, stream: "stdout" | "stderr" = "stdout") => - Effect.sync(() => { - if (stream === "stderr") { - process.stderr.write(text); - } else { - process.stdout.write(text); - } - }), - rawBytes: (bytes: Uint8Array, stream: "stdout" | "stderr" = "stdout") => - Effect.sync(() => { - if (stream === "stderr") { - process.stderr.write(bytes); - } else { - process.stdout.write(bytes); - } - }), + raw: (text: string, stream: "stdout" | "stderr" = "stdout") => write(text, stream), + rawBytes: (bytes: Uint8Array, stream: "stdout" | "stderr" = "stdout") => write(bytes, stream), }); }), ); @@ -403,12 +403,9 @@ export const textOutputLayer = Layer.effect( export const jsonOutputLayer = Layer.effect( Output, Effect.gen(function* () { - const stdio = yield* Stdio.Stdio; - - const writeStdout = (s: string) => - Stream.make(s).pipe(Stream.run(stdio.stdout()), Effect.orDie); - const writeStderr = (s: string) => - Stream.make(s).pipe(Stream.run(stdio.stderr()), Effect.orDie); + const write = stdioWriter(yield* Stdio.Stdio); + const writeStdout = (s: string) => write(s, "stdout"); + const writeStderr = (s: string) => write(s, "stderr"); const nonInteractive = (action: string) => Effect.fail( @@ -469,13 +466,8 @@ export const jsonOutputLayer = Layer.effect( // shape it's decorating always wins. yield* writeStdout(JSON.stringify({ ...extra, _tag: "Error", error: err }) + "\n"); }), - raw: (text: string, stream: "stdout" | "stderr" = "stdout") => - stream === "stderr" ? writeStderr(text) : writeStdout(text), - rawBytes: (bytes: Uint8Array, stream: "stdout" | "stderr" = "stdout") => - Stream.make(bytes).pipe( - Stream.run(stream === "stderr" ? stdio.stderr() : stdio.stdout()), - Effect.orDie, - ), + raw: (text: string, stream: "stdout" | "stderr" = "stdout") => write(text, stream), + rawBytes: (bytes: Uint8Array, stream: "stdout" | "stderr" = "stdout") => write(bytes, stream), }); }), ); @@ -484,12 +476,8 @@ export const jsonOutputLayer = Layer.effect( export const streamJsonOutputLayer = Layer.effect( Output, Effect.gen(function* () { - const stdio = yield* Stdio.Stdio; - - const writeStdout = (s: string) => - Stream.make(s).pipe(Stream.run(stdio.stdout()), Effect.orDie); - const writeStderr = (s: string) => - Stream.make(s).pipe(Stream.run(stdio.stderr()), Effect.orDie); + const write = stdioWriter(yield* Stdio.Stdio); + const writeStdout = (s: string) => write(s, "stdout"); const emitLog = (level: "info" | "warn" | "success" | "error", message: string) => { const event: StreamEvent = { type: "log", @@ -577,13 +565,8 @@ export const streamJsonOutputLayer = Layer.effect( // over an opt-in context field of the same name (PR #6168 review). yield* writeStdout(JSON.stringify({ ...extra, ...event }) + "\n"); }), - raw: (text: string, stream: "stdout" | "stderr" = "stdout") => - stream === "stderr" ? writeStderr(text) : writeStdout(text), - rawBytes: (bytes: Uint8Array, stream: "stdout" | "stderr" = "stdout") => - Stream.make(bytes).pipe( - Stream.run(stream === "stderr" ? stdio.stderr() : stdio.stdout()), - Effect.orDie, - ), + raw: (text: string, stream: "stdout" | "stderr" = "stdout") => write(text, stream), + rawBytes: (bytes: Uint8Array, stream: "stdout" | "stderr" = "stdout") => write(bytes, stream), }); }), ); diff --git a/apps/cli/src/shared/output/output.layer.unit.test.ts b/apps/cli/src/shared/output/output.layer.unit.test.ts index e911c74198..e8df6def3c 100644 --- a/apps/cli/src/shared/output/output.layer.unit.test.ts +++ b/apps/cli/src/shared/output/output.layer.unit.test.ts @@ -106,7 +106,9 @@ function getFailError(exit: Exit.Exit): unknown { describe("Output", () => { describe("text layer", () => { - const layer = textOutputLayer.pipe(Layer.provide(mockTty({ stdoutIsTty: true }))); + const layer = textOutputLayer.pipe( + Layer.provide(Layer.mergeAll(mockTty({ stdoutIsTty: true }), mockStdio().layer)), + ); it.effect("task uses clack spinner and can resolve into info", () => Effect.gen(function* () { @@ -329,6 +331,21 @@ describe("Output", () => { }).pipe(Effect.provide(layer)); }); + it.effect("raw and rawBytes write unframed through the stdio sink", () => { + const mock = mockStdio(); + const sunk = textOutputLayer.pipe( + Layer.provide(Layer.mergeAll(mockTty({ stdoutIsTty: true }), mock.layer)), + ); + return Effect.gen(function* () { + const out = yield* Output; + yield* out.raw("plain text\n"); + yield* out.rawBytes(new TextEncoder().encode("raw bytes\n")); + yield* out.raw("to stderr\n", "stderr"); + expect(mock.stdout).toEqual(["plain text\n", "raw bytes\n"]); + expect(mock.stderr).toEqual(["to stderr\n"]); + }).pipe(Effect.provide(sunk)); + }); + it.effect("promptText interrupts on cancel", () => { mockClack.text.mockResolvedValue(Symbol.for("clack:cancel")); mockClack.isCancel.mockReturnValue(true); diff --git a/apps/cli/tests/helpers/legacy-mocks.ts b/apps/cli/tests/helpers/legacy-mocks.ts index 11e5c66ffb..0646b1bc3e 100644 --- a/apps/cli/tests/helpers/legacy-mocks.ts +++ b/apps/cli/tests/helpers/legacy-mocks.ts @@ -725,6 +725,44 @@ export function useLegacyShadowCacheDisabled(): void { }); } +/** + * The opposite direction of {@link useLegacyShadowCacheDisabled}, scoped to ONE effect rather than + * a whole file: turns the shadow baseline cache back on for `body` and roots it at `homeDir`, + * restoring whatever the host had afterwards. Both variables have to move together — the cache + * reads `SUPABASE_SHADOW_CACHE` for the gate and `SUPABASE_HOME` for the tar directory + * (`legacyShadowBaselineCacheDir`), so pinning only the gate would write ~90MB-shaped tars into + * the developer's real `~/.supabase`. + * + * For a suite that has opted out file-wide, this is how a single cache-focused scenario opts back + * in. Point `homeDir` at a per-test temp dir (see {@link useLegacyTempWorkdir}). + */ +export const withLegacyShadowCacheEnabled = ( + homeDir: string, + body: Effect.Effect, +): Effect.Effect => + Effect.acquireUseRelease( + Effect.sync(() => { + const previous = { + cache: process.env["SUPABASE_SHADOW_CACHE"], + home: process.env["SUPABASE_HOME"], + }; + process.env["SUPABASE_SHADOW_CACHE"] = "1"; + process.env["SUPABASE_HOME"] = homeDir; + return previous; + }), + () => body, + (previous) => + Effect.sync(() => { + for (const [name, value] of [ + ["SUPABASE_SHADOW_CACHE", previous.cache], + ["SUPABASE_HOME", previous.home], + ] as const) { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; + } + }), + ); + /** * Ambient isolation for tests that construct the REAL `legacyCliConfigLayer` / * `legacyCredentialsLayer` (directly or inside a command runtime layer) against @@ -817,30 +855,19 @@ export const LEGACY_FAKE_SHADOW_CONTAINER_ID = "abc123456789shadow0".padEnd(64, const LEGACY_SHADOW_HEALTHY_STATE = '{"Running":true,"Status":"running","Health":{"Status":"healthy"}}'; -/** - * A real (Docker-valid) "still starting" state — NOT `Effect.never` — so - * {@link legacyWaitForHealthyServices}'s retry loop genuinely retries on its real 1-second - * `Schedule.spaced` backoff instead of hanging on a single probe forever. Mirrors - * `start.integration.test.ts`'s own "never healthy" containers (same rationale: a fiber - * interrupted mid-retry must be observed actually suspended inside the retry loop, not merely - * past the initial `create` call). - */ -const LEGACY_SHADOW_STARTING_STATE = - '{"Running":true,"Status":"running","Health":{"Status":"starting"}}'; - /** * Fakes every `docker`/`podman` subprocess call the native shadow-provisioning path issues * (`legacyBuildLocalDbContainerInputs`'s image-cache check, `legacyCreateShadowDatabase`'s - * network-create + container create/start, `legacyWaitForHealthyServices`'s container + * network-create + container create/start, `legacyWaitForShadowReady`'s container * inspect, and `legacyRemoveShadowDatabase`'s cleanup) — scoped-down port of * `start.integration.test.ts`'s own `mockContainerCliSpawner`, since both callers only ever * create one (shadow) container, never named. * - * `neverHealthy` (default `false`) makes every `container inspect` report `"starting"` instead - * of `"healthy"` — for the interrupt-during-health-wait regression coverage (review: - * PRRT_kwDOErm0O86XMrID): with the default healthy-immediately response, a forked fiber can run - * the ENTIRE shadow-provisioning sequence to completion synchronously before a test's own - * polling loop is even scheduled, making `Fiber.interrupt` a no-op on an already-finished fiber. + * `container inspect` on the shadow's own id always reports `"healthy"`: no shadow consumer + * gates on the Docker healthcheck any more — they all wait on `legacyWaitForShadowReady`'s direct + * connect probe instead, so a suite that needs a shadow to stay un-ready refuses connects on the + * shadow port through its own `LegacyDbConnection` fake (see `diff.integration.test.ts`'s + * `neverConnectableShadow`) rather than faking a `"starting"` container. * * `failCreate`/`failRemove` (both default `false`) make `docker create`/`docker rm` exit * non-zero instead — hoisted from `migration squash`'s own scoped-down copy of this mock @@ -853,7 +880,7 @@ const LEGACY_SHADOW_STARTING_STATE = * supabase_db_` probe `legacyIsLocalDbRunning` issues before `--use-pgadmin` * provisions anything — distinguished from the shadow's own `container inspect <64-hex-id>` * health probe by the target id's `supabase_db_` prefix, so both options leave the shadow's - * own health check on its normal (healthy/never-healthy) path. `dbNotRunning` reports the + * own health check on its normal path. `dbNotRunning` reports the * Go/Docker "container doesn't exist" shape (`legacyIsContainerNotFoundMessage`); mutually * exclusive with `dbInspectFailsWith`, which instead reports a daemon-unreachable failure * (`legacyIsDockerDaemonUnreachable`) with the given stderr text — enforced below (a test @@ -861,7 +888,6 @@ const LEGACY_SHADOW_STARTING_STATE = */ export function mockLegacyShadowContainerCliSpawner( opts: { - readonly neverHealthy?: boolean; readonly failCreate?: boolean; readonly failRemove?: boolean; readonly dbNotRunning?: boolean; @@ -876,7 +902,6 @@ export function mockLegacyShadowContainerCliSpawner( "mockLegacyShadowContainerCliSpawner: dbNotRunning and dbInspectFailsWith are mutually exclusive", ); } - const neverHealthy = opts.neverHealthy ?? false; const failCreate = opts.failCreate ?? false; const failRemove = opts.failRemove ?? false; const spawned: Array<{ readonly args: ReadonlyArray }> = []; @@ -935,7 +960,7 @@ export function mockLegacyShadowContainerCliSpawner( stdoutLines = [LEGACY_FAKE_SHADOW_CONTAINER_ID]; } } else if (args[0] === "container" && args[1] === "inspect") { - stdoutLines = [neverHealthy ? LEGACY_SHADOW_STARTING_STATE : LEGACY_SHADOW_HEALTHY_STATE]; + stdoutLines = [LEGACY_SHADOW_HEALTHY_STATE]; } else if (args[0] === "rm") { if (failRemove) { exitCode = 1; diff --git a/docs/superpowers/specs/2026-08-15-managed-identity-maintainability-design.md b/docs/superpowers/specs/2026-08-15-managed-identity-maintainability-design.md new file mode 100644 index 0000000000..9962769f25 --- /dev/null +++ b/docs/superpowers/specs/2026-08-15-managed-identity-maintainability-design.md @@ -0,0 +1,92 @@ +# Managed Identity Maintainability Design + +## Context + +Pull request #6214 fixes confirmed managed-identity recovery defects found after #6202. The same +review identified duplication and documentation drift in the code that owns workspace discovery, +identity settlement, Git configuration locking, and recovery tests. + +The managed surface has not shipped as a stable compatibility boundary. The repository's +refactoring policy therefore favors the simplest current model over preserving unused exports. + +## Goal + +Reduce verified maintenance hazards in the managed-identity implementation without broadening +#6214 into a package-wide decomposition or changing developer-visible behavior. + +## Scope + +### Shared workspace facts + +Create one small pure module that derives the managed workspace, context, and context descriptor +from a workspace inspection. Discovery and identity publication will consume this derivation +instead of rebuilding ordinary-folder and Git-checkout facts independently. + +The discovery types remain the public read-only report contract. Resolved-plan types remain the +mutation-settlement contract; structurally similar types will not be merged merely to remove lines. + +### Concurrent registration settlement + +Move reusable topology and monotonic-publication predicates out of the large identity +implementation into a focused pure settlement module. The module will express the benign +first-start outcomes accepted by the facade, while transition-specific ownership checks remain in +their existing recovery flows. + +The service facade will consume the named settlement policy rather than reassembling it from field +comparisons. Existing double discovery remains intact because it is the race-settlement guard. + +### Git lock retry policy + +Define the 10 millisecond exponential retry with a 400 millisecond bound once inside the Git module +and reuse it for ordinary Git config locking and explicit conditional-replacement lock acquisition. +The retrying Git config path and the non-retrying self-lock-aware path remain distinct. + +### Unused Promise canonicalizer + +Delete the Promise-based `canonicalizeManagedWorkspacePath`, its Node filesystem imports, its +managed entrypoint export, and the export assertion. Repository-wide search found no consumer; the +Effect filesystem implementation remains the only production path. + +### Documentation and test quality + +Correct the managed architecture documentation: + +- describe four public entrypoint levels rather than three; +- describe the actual testing entrypoint, including fixtures, validators, repository seams, and + transport test tags; and +- repair the malformed acquisition-failure sentence. + +Strengthen the managed discovery integration tests by requiring exactly one concurrent rebind +winner, tracking every opened SQLite handle for cleanup, and removing assertions that only restate +discovery's already-filtered projection. + +## Module size and ownership + +Line count is a diagnostic, not a rule. This change will not mechanically slice large files or +introduce pass-through modules. New shared policy belongs in focused pure modules measured in +hundreds of lines, and the large identity implementation must shrink rather than grow. + +The broader decomposition of `workspace-identity.ts`, `sqlite.ts`, `git.ts`, and `repository.ts` is +explicitly deferred until #6214 is complete. That follow-up will choose deep modules around domain +ownership rather than file size alone. + +## Error behavior + +No error tag, error code, recovery ordering, transition ownership rule, or fail-closed ambiguity +behavior changes. Settlement helpers only name behavior already exercised through the managed +public Interface. Removing the unused Promise canonicalizer is the only exported-surface change. + +## Verification + +Behavior remains protected through existing public-interface integration suites for managed +discovery, identity, resolution, and Git configuration. Focused tests will cover the strengthened +concurrent-rebind assertion and cleanup correction. Completion requires the full `packages/stack` +unit and integration suite plus type, lint, format, and unused-code checks. + +## Explicit exclusions + +- No generic identity matcher across exact, compatible, monotonic, and ownership semantics. +- No wholesale merge of branch-copy and adopt-context takeover implementations. +- No merge of resolved-plan and discovery contracts. +- No contract-fixture execution-policy change. +- No package-wide file decomposition. diff --git a/packages/stack/docs/architecture.md b/packages/stack/docs/architecture.md index f91d6810d4..7c0538c860 100644 --- a/packages/stack/docs/architecture.md +++ b/packages/stack/docs/architecture.md @@ -7,7 +7,7 @@ delegated to [`@supabase/process-compose`](../../process-compose/docs/architectu ## Public entrypoints -The package exposes three levels of Interface: +The package exposes four levels of Interface: - `@supabase/stack` selects `bun.ts` or `node.ts` through export conditions and exposes the Promise-oriented `createStack()` / `StackHandle` Interface plus prefetch helpers. @@ -15,8 +15,9 @@ The package exposes three levels of Interface: Effect Interfaces plus platform-bound layer factories used by the CLI and advanced callers. - `@supabase/stack/managed` selects the Node or Bun SQLite Adapter and exposes managed identity, discovery, persistence, and lifecycle coordination. Its repository can be replaced by a caller. -- `@supabase/stack/testing` exposes only the service tags needed to replace daemon transport in - consumer tests. Runtime implementation tags do not leak through the root or Effect barrels. +- `@supabase/stack/testing` exposes test-only tags plus contract fixtures, validators, the + in-memory repository seam, and transport helpers. Runtime implementation tags do not leak + through the root or Effect barrels. Internal runtime Adapters provide Effect filesystem, path, child-process, HTTP-server, and Unix socket HTTP implementations. `createStack.ts` and the layer factories remain platform-agnostic; @@ -590,9 +591,9 @@ than incidental: build the runtime's context through `runtime.context()`, because opening the registry is I/O: a file is created and hardened, and a cold start may have to wait out another process' WAL conversion. Everything that can refuse the acquisition arrives as a rejection — a - blank state root, an owner PID that could never be probed, and a registry written by an - option failures reject with typed error instances, so a caller has one failure channel instead of a - throw plus a rejection. + blank state root, an owner PID that could never be probed, an incompatible registry, or invalid + options reject with typed error instances, so a caller has one failure channel instead of a throw + plus a rejection. - **Reads are Promises too.** `inspectStack` and `listStacks` return Promises rather than answering inline. A handle that read synchronously would only be hiding the registry's I/O from its caller, and it is what forced the cold-start retry below to block. The `repository` accessor stays a plain diff --git a/packages/stack/src/entrypoints.unit.test.ts b/packages/stack/src/entrypoints.unit.test.ts index 504370f533..c005f28711 100644 --- a/packages/stack/src/entrypoints.unit.test.ts +++ b/packages/stack/src/entrypoints.unit.test.ts @@ -124,7 +124,6 @@ describe("@supabase/stack entrypoints", () => { "assertManagedStackRoot", "assertManagedUuid", "bunSqliteManagedStackRepositoryLayer", - "canonicalizeManagedWorkspacePath", "createManagedStackService", "createManagedUuid", "ensureBranchContextId", diff --git a/packages/stack/src/managed-discovery.integration.test.ts b/packages/stack/src/managed-discovery.integration.test.ts index a03d90ea48..b751a6d111 100644 --- a/packages/stack/src/managed-discovery.integration.test.ts +++ b/packages/stack/src/managed-discovery.integration.test.ts @@ -225,6 +225,7 @@ describe.each(adapters)("managed discovery with the %s adapter", (_name, open) = } else { serviceB = await open(root); const base = await open(root); + openHandles.push(base); const wrapped: ManagedStackRepositoryShape = { ...base.repository, listIdentityClaims: (projectId) => @@ -255,14 +256,6 @@ describe.each(adapters)("managed discovery with the %s adapter", (_name, open) = expect(injectedStart).toBeDefined(); await injectedStart; expect(startedA.identity.checkoutId).toBeDefined(); - - const reportA = await serviceA.discoverWorkspace(workspaceA); - expect(reportA.locations.map((location) => location.canonicalPath)).not.toContain(workspaceB); - const fresh = makeDirectory(root, "workspace-c"); - const reportFresh = await serviceA.discoverWorkspace(fresh); - expect(reportFresh.locations.map((location) => location.canonicalPath)).not.toContain( - workspaceB, - ); }); it("resolves healthy branch, detached, and ordinary identities", async () => { @@ -785,6 +778,48 @@ describe.each(adapters)("managed discovery with the %s adapter", (_name, open) = expect((await inspect(service.repository, repository)).activeTransition).toBeUndefined(); }); + it("does not expose a reserved branch transition on an unrelated branch", async () => { + const root = makeRoot(); + const repository = makeRepository(root); + const service = await open(root); + openHandles.push(service); + const started = await service.resolveStack({ workspacePath: repository, operation: "start" }); + git(repository, "branch", "-m", "renamed"); + const inspection = await inspect(service.repository, repository); + await Effect.runPromise( + service.repository.reserveIdentityTransition({ + id: "00000000-0000-7000-8000-000000000302", + kind: "adopt-context", + projectId: started.identity.projectId, + checkoutId: started.identity.checkoutId, + contextId: started.identity.contextId, + branch: "renamed", + path: repository, + projectIdentityLocation: inspection.workspace.projectIdentityLocation, + expectedGitValue: started.identity.contextId, + targetGitValue: started.identity.contextId, + expectedOwnerBranch: "main", + now: new Date().toISOString(), + }), + ); + + git(repository, "checkout", "-q", "-b", "other"); + expect((await inspect(service.repository, repository)).activeTransition).toBeUndefined(); + + git(repository, "checkout", "-q", "renamed"); + expect((await inspect(service.repository, repository)).activeTransition).toMatchObject({ + kind: "adopt-context", + branch: "renamed", + phase: "reserved", + }); + await expect( + service.abandonIdentityTransition({ + transitionId: "00000000-0000-7000-8000-000000000302", + workspacePath: repository, + }), + ).resolves.toEqual({ outcome: "abandoned" }); + }); + it("releases an untouched reservation when a project winner appears before publication", async () => { const root = makeRoot(); const repository = makeRepository(root); @@ -1459,50 +1494,75 @@ describe.each(adapters)("managed discovery with the %s adapter", (_name, open) = }); it.each([ - ["branch-copy", "00000000-0000-7000-8000-000000000126"], - ["adopt-context", "00000000-0000-7000-8000-000000000127"], - ] as const)("refuses %s abandonment when Git path is replaced", async (kind, transitionId) => { - const root = makeRoot(); - const original = makeRepository(root, `${kind}-original`); - const path = join(root, `${kind}-transition-path`); - renameSync(original, path); - const service = await open(root); - openHandles.push(service); - const started = - kind === "adopt-context" - ? await service.resolveStack({ workspacePath: path, operation: "start" }) - : undefined; - if (kind === "adopt-context") git(path, "branch", "-m", "renamed"); - const replacement = makeRepository(root, `${kind}-replacement`); - renameSync(replacement, join(root, `${kind}-replacement-moved`)); - renameSync(path, join(root, `${kind}-original-moved`)); - renameSync(join(root, `${kind}-replacement-moved`), path); - const reserved = await Effect.runPromise( - service.repository.reserveIdentityTransition({ - id: transitionId, - kind, - projectId: started?.identity.projectId ?? "00000000-0000-7000-8000-000000000128", - checkoutId: started?.identity.checkoutId ?? "00000000-0000-7000-8000-000000000129", - contextId: started?.identity.contextId ?? "00000000-0000-7000-8000-000000000130", - branch: kind === "adopt-context" ? "renamed" : "main", - path, - expectedGitValue: - kind === "branch-copy" - ? "00000000-0000-7000-8000-000000000130" - : started?.identity.contextId, - targetGitValue: kind === "branch-copy" ? "00000000-0000-7000-8000-000000000131" : undefined, - expectedOwnerBranch: kind === "adopt-context" ? "main" : undefined, - now: new Date().toISOString(), - }), - ); - const replaced = await inspect(service.repository, path); - expect(replaced.state).toBe("transitioning"); - expect(replaced.workspace.checkoutKind).not.toBe("ordinary"); - await expect( - service.abandonIdentityTransition({ transitionId: reserved.id, workspacePath: path }), - ).rejects.toMatchObject({ _tag: "ManagedIdentityTransitionOwnershipError" }); - expect((await inspect(service.repository, path)).activeTransition?.id).toBe(reserved.id); - }); + { + kind: "branch-copy" as const, + transitionId: "00000000-0000-7000-8000-000000000126", + expectedState: "transitioning" as const, + transitionVisible: true, + }, + { + kind: "adopt-context" as const, + transitionId: "00000000-0000-7000-8000-000000000127", + expectedState: "duplicate" as const, + transitionVisible: false, + }, + ])( + "refuses $kind abandonment when Git path is replaced", + async ({ kind, transitionId, expectedState, transitionVisible }) => { + const root = makeRoot(); + const original = makeRepository(root, `${kind}-original`); + const path = join(root, `${kind}-transition-path`); + renameSync(original, path); + const service = await open(root); + openHandles.push(service); + const started = + kind === "adopt-context" + ? await service.resolveStack({ workspacePath: path, operation: "start" }) + : undefined; + if (kind === "adopt-context") git(path, "branch", "-m", "renamed"); + const replacement = makeRepository(root, `${kind}-replacement`); + renameSync(replacement, join(root, `${kind}-replacement-moved`)); + renameSync(path, join(root, `${kind}-original-moved`)); + renameSync(join(root, `${kind}-replacement-moved`), path); + const reserved = await Effect.runPromise( + service.repository.reserveIdentityTransition({ + id: transitionId, + kind, + projectId: started?.identity.projectId ?? "00000000-0000-7000-8000-000000000128", + checkoutId: started?.identity.checkoutId ?? "00000000-0000-7000-8000-000000000129", + contextId: started?.identity.contextId ?? "00000000-0000-7000-8000-000000000130", + branch: kind === "adopt-context" ? "renamed" : "main", + path, + expectedGitValue: + kind === "branch-copy" + ? "00000000-0000-7000-8000-000000000130" + : started?.identity.contextId, + targetGitValue: + kind === "branch-copy" ? "00000000-0000-7000-8000-000000000131" : undefined, + expectedOwnerBranch: kind === "adopt-context" ? "main" : undefined, + now: new Date().toISOString(), + }), + ); + const replaced = await inspect(service.repository, path); + expect(replaced.state).toBe(expectedState); + expect(replaced.workspace.checkoutKind).not.toBe("ordinary"); + if (transitionVisible) { + expect(replaced.activeTransition?.id).toBe(reserved.id); + } else { + expect(replaced.activeTransition).toBeUndefined(); + } + await expect( + service.abandonIdentityTransition({ transitionId: reserved.id, workspacePath: path }), + ).rejects.toMatchObject({ _tag: "ManagedIdentityTransitionOwnershipError" }); + if (transitionVisible) { + expect((await inspect(service.repository, path)).activeTransition?.id).toBe(reserved.id); + } else { + expect( + (await Effect.runPromise(service.repository.listIdentityClaims())).transitions, + ).toContainEqual(expect.objectContaining({ id: reserved.id, phase: "reserved" })); + } + }, + ); it("advances a reserved rebind before publishing its registry location", async () => { const root = makeRoot(); @@ -1670,17 +1730,45 @@ describe.each(adapters)("managed discovery with the %s adapter", (_name, open) = openHandles.push(service); const first = await service.resolveStack({ workspacePath: previous, operation: "start" }); renameSync(previous, next); + + let arrivals = 0; + let releaseReservations: () => void = () => undefined; + const bothReservations = new Promise((resolve) => { + releaseReservations = resolve; + }); + const gateReservation = (repository: ManagedStackRepositoryShape) => ({ + ...repository, + reserveIdentityTransition: ( + input: Parameters[0], + ) => + Effect.gen(function* () { + arrivals += 1; + if (arrivals === 2) releaseReservations(); + yield* Effect.promise(() => bothReservations); + return yield* repository.reserveIdentityTransition(input); + }), + }); + const serviceA = await makeManagedStackService({ + repository: gateReservation(service.repository), + stateRoot: join(root, "concurrent-recovery-managed-a"), + publicationPollMs: 1, + }); + const serviceB = await makeManagedStackService({ + repository: gateReservation(service.repository), + stateRoot: join(root, "concurrent-recovery-managed-b"), + publicationPollMs: 1, + }); + openHandles.push(serviceA, serviceB); const outcomes = await Promise.allSettled([ - service.rebindCheckout({ workspacePath: next, checkoutId: first.identity.checkoutId }), - service.rebindCheckout({ workspacePath: next, checkoutId: first.identity.checkoutId }), + serviceA.rebindCheckout({ workspacePath: next, checkoutId: first.identity.checkoutId }), + serviceB.rebindCheckout({ workspacePath: next, checkoutId: first.identity.checkoutId }), ]); - expect(outcomes.some((outcome) => outcome.status === "fulfilled")).toBe(true); + expect(arrivals).toBe(2); const successful = outcomes.flatMap((outcome) => outcome.status === "fulfilled" ? [outcome.value] : [], ); - expect( - successful.every((result) => result.identity.checkoutId === first.identity.checkoutId), - ).toBe(true); + expect(successful).toHaveLength(1); + expect(successful[0]?.identity.checkoutId).toBe(first.identity.checkoutId); for (const outcome of outcomes) { if (outcome.status === "rejected") { expect(outcome.reason).toMatchObject({ @@ -2222,8 +2310,11 @@ describe.each(adapters)("managed discovery with the %s adapter", (_name, open) = const migrated = await service.resolveStack({ workspacePath: nested, operation: "start" }); expect(migrated.identity).toEqual(ordinary.identity); + expect(migrated.identityMarkerCreated).toBe(true); expect(migrated.stack.id).toBe(ordinary.stack.id); expect((await inspect(service.repository, nested)).state).toBe("healthy"); + const reused = await service.resolveStack({ workspacePath: nested, operation: "start" }); + expect(reused.identityMarkerCreated).toBe(false); }); it("resumes detached folder-to-Git migration with the original context and marker", async () => { diff --git a/packages/stack/src/managed-resolve-stack.integration.test.ts b/packages/stack/src/managed-resolve-stack.integration.test.ts index 5ff9103b19..bc9c54a07c 100644 --- a/packages/stack/src/managed-resolve-stack.integration.test.ts +++ b/packages/stack/src/managed-resolve-stack.integration.test.ts @@ -536,6 +536,23 @@ describe.each(adapters)("resolveStack over git workspaces with the %s adapter", expect(branchStacks.map((stack) => stack.id)).toEqual([feature.stack.id]); }); + it("settles a moved checkout and renamed branch in one subsequent start", async () => { + const root = makeRoot(); + const original = makeRepository(root, "original"); + const moved = join(root, "moved"); + const service = await openService(root); + const first = await service.resolveStack({ workspacePath: original, operation: "start" }); + + git(original, "branch", "-m", "main", "renamed"); + renameSync(original, moved); + + const recovered = await service.resolveStack({ workspacePath: moved, operation: "start" }); + expect(recovered.outcome).toBe("reuse"); + expect(recovered.stack.id).toBe(first.stack.id); + expect(recovered.identity).toEqual(first.identity); + expect(recovered.context).toEqual({ kind: "branch", branch: "renamed" }); + }); + it("repairs a copied branch on its first mutating start without changing the owner", async () => { const root = makeRoot(); const repository = makeRepository(root); diff --git a/packages/stack/src/managed-service.integration.test.ts b/packages/stack/src/managed-service.integration.test.ts index b37e36be5e..3b26535f15 100644 --- a/packages/stack/src/managed-service.integration.test.ts +++ b/packages/stack/src/managed-service.integration.test.ts @@ -646,6 +646,32 @@ describe("ordinary-folder managed stack contract", () => { await service.close(); }); + it("preserves an in-flight typed failure when close races before rejection", async () => { + const root = makeRoot(); + const base = createInMemoryManagedStackRepository(); + let signalStarted: () => void = () => undefined; + const started = new Promise((resolve) => { + signalStarted = resolve; + }); + const typedFailure = new InvalidManagedIdentityError({ message: "controlled failure" }); + const service = await makeManagedStackService({ + repository: { + ...base, + listStackProjections: () => + Effect.sync(() => { + signalStarted(); + throw typedFailure; + }), + }, + stateRoot: join(root, "close-race-managed"), + }); + const inFlight = service.listStacks(); + await started; + await service.close(); + + await expect(inFlight).rejects.toBe(typedFailure); + }); + it("rejects a copied ordinary-folder identity claim", async () => { const root = makeRoot(); const firstWorkspace = makeWorkspace(root, "first"); diff --git a/packages/stack/src/managed.ts b/packages/stack/src/managed.ts index 905de96ad5..2f82ab1ede 100644 --- a/packages/stack/src/managed.ts +++ b/packages/stack/src/managed.ts @@ -20,7 +20,6 @@ export type { EnsureGitCheckoutIdentityResult, } from "./managed/git.ts"; export { - canonicalizeManagedWorkspacePath, ensureOrdinaryWorkspaceIdentity, readOrdinaryWorkspaceIdentity, } from "./managed/identity.ts"; diff --git a/packages/stack/src/managed/create-service.ts b/packages/stack/src/managed/create-service.ts index bb6d49f1e9..4b387b98b2 100644 --- a/packages/stack/src/managed/create-service.ts +++ b/packages/stack/src/managed/create-service.ts @@ -156,12 +156,14 @@ const managedStackServiceHandle = async ( * message, or stack. While the handle is open, every failure is the failure * itself and passes through untouched. */ - const run = (effect: Effect.Effect): Promise => - runtime.runPromise(effect).catch((error: unknown) => { - throw closed + const run = (effect: Effect.Effect): Promise => { + const callWasClosed = closed; + return runtime.runPromise(effect).catch((error: unknown) => { + throw callWasClosed ? new Error(`The managed stack service handle is closed (${String(error)})`) : error; }); + }; /** * A function declaration rather than a property initializer, so the handle's diff --git a/packages/stack/src/managed/discovery.ts b/packages/stack/src/managed/discovery.ts index b0673db590..0bf7c99312 100644 --- a/packages/stack/src/managed/discovery.ts +++ b/packages/stack/src/managed/discovery.ts @@ -3,9 +3,7 @@ import { InvalidManagedIdentityError, UnsupportedGitWorkspaceError, type ManagedCheckoutLocation, - type ManagedCheckoutKind, type ManagedContextDescriptor, - type ManagedContextKind, type ManagedIdentityTransitionRecord, type ManagedIdentityClaims, type ManagedOperationRecord, @@ -20,7 +18,6 @@ import { readBranchContextId, readGitCheckoutIdentityWithFileSystem, type GitCheckoutInspection, - type WorkspaceInspection, } from "./git.ts"; import { canonicalizeManagedWorkspacePathWithFileSystem, @@ -32,7 +29,17 @@ import { protectedManagedCheckoutLocationIds, type ManagedStackRepositoryShape, } from "./repository.ts"; -import { checkoutKindOf, newCheckoutTopologyMatches } from "./topology.ts"; +import { newCheckoutTopologyMatches } from "./topology.ts"; +import { + workspaceMetadata, + type ManagedWorkspaceDiscoveryContext, + type ManagedWorkspaceDiscoveryWorkspace, +} from "./workspace-metadata.ts"; + +export type { + ManagedWorkspaceDiscoveryContext, + ManagedWorkspaceDiscoveryWorkspace, +} from "./workspace-metadata.ts"; export type ManagedWorkspaceDiscoveryState = | "adoptable" @@ -50,20 +57,6 @@ export type ManagedRecoveryOperation = | { readonly operation: "adoptContext"; readonly contextId: string; readonly branch: string } | { readonly operation: "prune"; readonly recordIds: ReadonlyArray }; -export interface ManagedWorkspaceDiscoveryWorkspace { - readonly checkoutKind: ManagedCheckoutKind; - readonly canonicalPath: string; - readonly workspaceRoot: string; - readonly projectIdentityLocation: string; - readonly checkoutIdentityLocation: string; -} - -export interface ManagedWorkspaceDiscoveryContext { - readonly kind: ManagedContextKind; - readonly branch?: string; - readonly commit?: string; -} - export interface ManagedWorkspaceDiscoveryIdentity { readonly projectId?: string; readonly checkoutId?: string; @@ -149,19 +142,26 @@ const transitionMatches = ( return transition.path === workspaceRoot && newCheckoutTopologyMatches(transition, context); } const branch = context.kind === "branch" ? context.branch : undefined; + if ( + (transition.kind === "folder-to-git" || + transition.kind === "adopt-context" || + transition.kind === "branch-copy") && + transition.branch !== branch + ) { + return false; + } return ( - (transition.kind !== "folder-to-git" || transition.branch === branch) && - ((transition.projectId === identity.projectId && + (transition.projectId === identity.projectId && transition.checkoutId === undefined && transition.contextId === undefined && transition.branch === undefined && transition.path === undefined) || - transition.path === workspaceRoot || - (transition.checkoutId !== undefined && transition.checkoutId === identity.checkoutId) || - (transition.contextId !== undefined && transition.contextId === identity.contextId) || - (transition.branch !== undefined && - transition.branch === branch && - transition.projectId === identity.projectId)) + transition.path === workspaceRoot || + (transition.checkoutId !== undefined && transition.checkoutId === identity.checkoutId) || + (transition.contextId !== undefined && transition.contextId === identity.contextId) || + (transition.branch !== undefined && + transition.branch === branch && + transition.projectId === identity.projectId) ); }; @@ -464,48 +464,6 @@ const inspectBranchOwners = ( return { contextId, claims: liveClaims }; }); -const workspaceMetadata = ( - inspection: WorkspaceInspection, -): { - readonly workspace: ManagedWorkspaceDiscoveryWorkspace; - readonly context: ManagedWorkspaceDiscoveryContext; - readonly contextDescriptor: ManagedContextDescriptor; -} => { - if (inspection.kind === "ordinary-folder") { - const markerPath = ordinaryWorkspaceIdentityPath(inspection.canonicalPath); - return { - workspace: { - checkoutKind: "ordinary", - canonicalPath: inspection.canonicalPath, - workspaceRoot: inspection.canonicalPath, - projectIdentityLocation: markerPath, - checkoutIdentityLocation: markerPath, - }, - context: { kind: "workspace" }, - contextDescriptor: { kind: "workspace" }, - }; - } - const context: ManagedWorkspaceDiscoveryContext = - inspection.head.kind === "detached" - ? { kind: "detached", commit: inspection.head.commit } - : { kind: "branch", branch: inspection.head.branch }; - const contextDescriptor: ManagedContextDescriptor = - inspection.head.kind === "detached" - ? { kind: "detached" } - : { kind: "branch", locator: inspection.head.branch }; - return { - workspace: { - checkoutKind: checkoutKindOf(inspection), - canonicalPath: inspection.canonicalPath, - workspaceRoot: inspection.workspaceRoot, - projectIdentityLocation: inspection.commonDirectory, - checkoutIdentityLocation: inspection.gitDirectory, - }, - context, - contextDescriptor, - }; -}; - /** Read-only managed identity discovery. No repository or identity writes occur. */ export const discoverWorkspace = ( workspacePath: string, @@ -543,17 +501,14 @@ export const discoverWorkspace = ( // becoming a repository. It is read as transition evidence only; its // values never populate the active Git identity below. const markerPath = ordinaryWorkspaceIdentityPath(inspection.workspaceRoot); - const fs = yield* FileSystem.FileSystem; - const markerExists = yield* fs - .exists(markerPath) - .pipe(Effect.catchTag("PlatformError", () => Effect.succeed(false))); const marker = yield* readOrdinaryWorkspaceIdentityWithFileSystem(inspection.workspaceRoot); - const markerTracked = markerExists - ? yield* isOrdinaryIdentityMarkerTracked(inspection.workspaceRoot) - : false; + const markerTracked = + marker !== undefined + ? yield* isOrdinaryIdentityMarkerTracked(inspection.workspaceRoot) + : false; ordinaryMarker = { path: markerPath, - present: markerExists, + present: marker !== undefined, tracked: markerTracked, identity: marker, }; diff --git a/packages/stack/src/managed/git.integration.test.ts b/packages/stack/src/managed/git.integration.test.ts index 3f12adbc9d..9414e3a89e 100644 --- a/packages/stack/src/managed/git.integration.test.ts +++ b/packages/stack/src/managed/git.integration.test.ts @@ -572,6 +572,26 @@ describe("git-stored identity", () => { }).pipe(Effect.provide(gitLayer)), ); + it.live("collapses duplicated equal branch context values during replacement", () => + Effect.gen(function* () { + const root = makeRoot(); + const repository = makeRepository(root); + const inspection = yield* inspectCheckout(repository); + const expected = "00000000-0000-7000-8000-000000000304"; + const target = "00000000-0000-7000-8000-000000000305"; + const config = gitConfigPath(inspection.commonDirectory); + git(repository, "config", gitBranchContextIdKey("main"), expected); + git(repository, "config", "--add", gitBranchContextIdKey("main"), expected); + + yield* replaceBranchContextId(inspection, "main", expected, target); + + expect(storedConfigValue(config, gitBranchContextIdKey("main"))).toBe(target); + expect(git(repository, "config", "--get-all", gitBranchContextIdKey("main"))).toBe( + `${target}\n`, + ); + }).pipe(Effect.provide(gitLayer)), + ); + it.live("gives sibling linked worktrees one project and separate checkouts", () => Effect.gen(function* () { const root = makeRoot(); diff --git a/packages/stack/src/managed/git.ts b/packages/stack/src/managed/git.ts index 6c33e397ff..5eb2184546 100644 --- a/packages/stack/src/managed/git.ts +++ b/packages/stack/src/managed/git.ts @@ -658,6 +658,9 @@ const runGitConfig = ( }); }); +const gitLockRetrySchedule = () => + Schedule.exponential(Duration.millis(10)).pipe(Schedule.upTo({ duration: Duration.millis(400) })); + /** * `git config` does not wait for another process' config lock — it refuses * immediately — so waiting is this store's job. Every claim in a repository with @@ -679,9 +682,7 @@ const gitConfig = ( ), { while: (error) => error.kind === "retryable", - schedule: Schedule.exponential(Duration.millis(10)).pipe( - Schedule.upTo({ duration: Duration.millis(400) }), - ), + schedule: gitLockRetrySchedule(), }, ), (error) => @@ -764,9 +765,7 @@ const gitConfigReplaceExpected = ( const acquireLock = fs.writeFileString(lockPath, "", { flag: "wx" }).pipe( Effect.retry({ while: (error) => error.reason._tag === "AlreadyExists", - schedule: Schedule.exponential(Duration.millis(10)).pipe( - Schedule.upTo({ duration: Duration.millis(400) }), - ), + schedule: gitLockRetrySchedule(), }), ); yield* Effect.acquireUseRelease( @@ -774,7 +773,13 @@ const gitConfigReplaceExpected = ( () => Effect.gen(function* () { const current = yield* gitConfigOnce(["--file", file, "--get-all", key], true, file); - if ((current ?? "").trim() !== expected) { + const settled = settledValue( + (current ?? "") + .split("\n") + .map((candidate) => candidate.trim()) + .filter((candidate) => candidate.length > 0), + ); + if (settled !== expected) { return yield* Effect.fail( new InvalidManagedIdentityError({ message: `${key} changed before conditional replacement`, diff --git a/packages/stack/src/managed/identity.ts b/packages/stack/src/managed/identity.ts index 335b1417cb..7dd4a43d05 100644 --- a/packages/stack/src/managed/identity.ts +++ b/packages/stack/src/managed/identity.ts @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { mkdir, readFile, realpath, stat } from "node:fs/promises"; +import { mkdir, readFile } from "node:fs/promises"; import { dirname } from "node:path"; import { Effect, FileSystem, type PlatformError } from "effect"; import { claimFileAtomically } from "./atomic-claim.ts"; @@ -64,30 +64,6 @@ const decodeIdentity = (content: string): OrdinaryWorkspaceIdentity => { }; }; -/** - * The canonical path of a workspace directory, whatever it turns out to be. - * - * Every resolve starts here, ordinary folders and git checkouts alike: a path - * that is not a directory is a caller mistake rather than a workspace to - * classify, and canonicalizing once keeps a symlinked alias from registering as - * a second location for the same checkout. - */ -export const canonicalizeManagedWorkspacePath = ( - workspacePath: string, -): Effect.Effect => - failsWithIdentity( - Effect.tryPromise({ - try: async () => { - const info = await stat(workspacePath); - if (!info.isDirectory()) { - throw new InvalidManagedIdentityError({ message: `${workspacePath} is not a directory` }); - } - return realpath(workspacePath); - }, - catch: asRaised, - }), - ); - /** Effect FileSystem variant used by managed discovery. */ export const canonicalizeManagedWorkspacePathWithFileSystem = ( workspacePath: string, diff --git a/packages/stack/src/managed/service.ts b/packages/stack/src/managed/service.ts index d4714c22c5..3f71d38396 100644 --- a/packages/stack/src/managed/service.ts +++ b/packages/stack/src/managed/service.ts @@ -37,6 +37,7 @@ import { import type { ManagedIdentityRecoveryError } from "./repository.ts"; import type { ManagedWorkspaceDiscovery } from "./discovery.ts"; import { discoveryObservation } from "./discovery-observation.ts"; +import { benignConcurrentRegistration } from "./workspace-settlement.ts"; import { makeStackLifecycle, type StackLifecycle, @@ -480,40 +481,10 @@ export class ManagedStackService extends Context.Service< resolveOptions.operation === "start" ? yield* workspaceIdentity.discover(resolveOptions.workspacePath) : report; - const sameWorkspaceTopology = workspaceIdentity.sameManagedWorkspaceTopology( - report, - settledReport, - ); - const settledIdentityIsMonotonic = workspaceIdentity.identityPublicationIsMonotonic( - report, - settledReport, - ); - const settledIdentityPublished = - settledReport.identity.projectId !== undefined && - settledReport.identity.checkoutId !== undefined && - settledReport.identity.contextId !== undefined; - const settledNewCheckoutReservation = - report.state === "unregistered" && - settledReport.state === "transitioning" && - settledReport.activeTransition?.kind === "new-checkout" && - settledReport.activeTransition.path === settledReport.workspace.workspaceRoot && - settledReport.activeTransition.projectIdentityLocation === - settledReport.workspace.projectIdentityLocation && - settledReport.conflicts.length === 0; - const benignConcurrentRegistration = - report.state === "unregistered" && - sameWorkspaceTopology && - settledIdentityIsMonotonic && - ((settledReport.activeTransition === undefined && - ((settledReport.state === "healthy" && - settledIdentityPublished && - settledReport.conflicts.length === 0) || - workspaceIdentity.concurrentIdentityPublication(report, settledReport))) || - settledNewCheckoutReservation); if ( resolveOptions.operation === "start" && discoveryObservation(report) !== discoveryObservation(settledReport) && - !benignConcurrentRegistration + !benignConcurrentRegistration(report, settledReport) ) { return yield* Effect.fail( new InvalidManagedIdentityError({ @@ -522,44 +493,86 @@ export class ManagedStackService extends Context.Service< ); } let recoveryReportForStart = settledReport; - if ( - resolveOptions.operation === "start" && - (settledReport.folderToGitClaims.length > 0 || - (settledReport.state === "transitioning" && - settledReport.activeTransition?.kind === "folder-to-git")) - ) { - recoveryReportForStart = yield* workspaceIdentity.migrateFolderToGit(settledReport); - } - if (resolveOptions.operation === "start" && settledReport.state === "moved") { - recoveryReportForStart = yield* workspaceIdentity.rebindCheckout({ - workspacePath: resolveOptions.workspacePath, - checkoutId: settledReport.identity.checkoutId, - observation: settledReport, - }); - } - if ( - resolveOptions.operation === "start" && - (((settledReport.state === "adoptable" || settledReport.state === "orphaned") && - settledReport.recoveryOperations.some( - (operation) => operation.operation === "adoptContext", - )) || - (settledReport.state === "transitioning" && - settledReport.activeTransition?.kind === "adopt-context")) - ) { - recoveryReportForStart = yield* workspaceIdentity.adoptContext({ - workspacePath: resolveOptions.workspacePath, - observation: settledReport, - }); - } - if ( - resolveOptions.operation === "start" && - ((recoveryReportForStart.state === "duplicate" && - workspaceIdentity.branchCopyIsUnambiguous(recoveryReportForStart)) || - (recoveryReportForStart.state === "transitioning" && - recoveryReportForStart.activeTransition?.kind === "branch-copy")) - ) { - recoveryReportForStart = - yield* workspaceIdentity.repairCopiedBranch(recoveryReportForStart); + let identityMarkerCreated = false; + if (resolveOptions.operation === "start") { + const automaticRecoveryKind = ( + candidate: ManagedWorkspaceDiscovery, + ): + | "folder-to-git" + | "rebind-checkout" + | "adopt-context" + | "branch-copy" + | undefined => { + if ( + candidate.folderToGitClaims.length > 0 || + (candidate.state === "transitioning" && + candidate.activeTransition?.kind === "folder-to-git") + ) { + return "folder-to-git"; + } + if (candidate.state === "moved") return "rebind-checkout"; + if ( + ((candidate.state === "adoptable" || candidate.state === "orphaned") && + candidate.recoveryOperations.some( + (operation) => operation.operation === "adoptContext", + )) || + (candidate.state === "transitioning" && + candidate.activeTransition?.kind === "adopt-context") + ) { + return "adopt-context"; + } + if ( + (candidate.state === "duplicate" && + workspaceIdentity.branchCopyIsUnambiguous(candidate)) || + (candidate.state === "transitioning" && + candidate.activeTransition?.kind === "branch-copy") + ) { + return "branch-copy"; + } + return undefined; + }; + const maxRecoveryIterations = 8; + let iteration = 0; + while (true) { + const kind = automaticRecoveryKind(recoveryReportForStart); + if (kind === undefined) break; + if (iteration >= maxRecoveryIterations) { + return yield* Effect.fail( + new InvalidManagedIdentityError({ + message: "Managed workspace recovery did not converge", + }), + ); + } + const before = discoveryObservation(recoveryReportForStart); + if (kind === "folder-to-git") { + const migrated = + yield* workspaceIdentity.migrateFolderToGit(recoveryReportForStart); + recoveryReportForStart = migrated.report; + identityMarkerCreated ||= migrated.identityMarkerCreated; + } else if (kind === "rebind-checkout") { + recoveryReportForStart = yield* workspaceIdentity.rebindCheckout({ + workspacePath: resolveOptions.workspacePath, + checkoutId: recoveryReportForStart.identity.checkoutId, + observation: recoveryReportForStart, + }); + } else if (kind === "adopt-context") { + recoveryReportForStart = yield* workspaceIdentity.adoptContext({ + workspacePath: resolveOptions.workspacePath, + observation: recoveryReportForStart, + }); + } else { + recoveryReportForStart = + yield* workspaceIdentity.repairCopiedBranch(recoveryReportForStart); + } + iteration += 1; + if (before === discoveryObservation(recoveryReportForStart)) { + return yield* Effect.fail( + new InvalidManagedIdentityError({ + message: "Managed workspace recovery made no progress", + }), + ); + } + } } const plan: ResolvedWorkspacePlan = { workspace: recoveryReportForStart.workspace, @@ -573,7 +586,7 @@ export class ManagedStackService extends Context.Service< contextId: recoveryReportForStart.registryContextId, } : recoveryReportForStart.identity, - identityMarkerCreated: false, + identityMarkerCreated, }; if ( resolveOptions.operation === "start" && diff --git a/packages/stack/src/managed/workspace-identity.ts b/packages/stack/src/managed/workspace-identity.ts index 1dce7c8b2a..4deb4e1bf4 100644 --- a/packages/stack/src/managed/workspace-identity.ts +++ b/packages/stack/src/managed/workspace-identity.ts @@ -33,7 +33,7 @@ import { type GitCheckoutInspection, } from "./git.ts"; import { assertManagedUuid } from "./ids.ts"; -import { ordinaryWorkspaceIdentityPath, gitConfigPath } from "./paths.ts"; +import { gitConfigPath } from "./paths.ts"; import { ManagedStackRepository, type AbandonManagedIdentityTransitionResult, @@ -48,6 +48,12 @@ import { newCheckoutTopologyMatches, } from "./topology.ts"; import { discoveryObservation } from "./discovery-observation.ts"; +import { workspaceMetadata } from "./workspace-metadata.ts"; +import { + concurrentIdentityPublication, + identityPublicationIsMonotonic, + sameManagedWorkspaceTopology, +} from "./workspace-settlement.ts"; export interface WorkspaceIdentityDependencies { readonly repository: ManagedStackRepositoryShape; @@ -174,50 +180,6 @@ const observationMatches = ( ); }; -const sameManagedWorkspaceTopology = ( - report: ManagedWorkspaceDiscovery, - freshReport: ManagedWorkspaceDiscovery, -): boolean => - report.workspace.checkoutKind === freshReport.workspace.checkoutKind && - report.workspace.workspaceRoot === freshReport.workspace.workspaceRoot && - report.workspace.projectIdentityLocation === freshReport.workspace.projectIdentityLocation && - report.workspace.checkoutIdentityLocation === freshReport.workspace.checkoutIdentityLocation && - report.context.kind === freshReport.context.kind && - report.context.branch === freshReport.context.branch && - report.context.commit === freshReport.context.commit; - -const identityPublicationIsMonotonic = ( - report: ManagedWorkspaceDiscovery, - freshReport: ManagedWorkspaceDiscovery, -): boolean => - (report.identity.projectId === undefined || - report.identity.projectId === freshReport.identity.projectId) && - (report.identity.checkoutId === undefined || - report.identity.checkoutId === freshReport.identity.checkoutId) && - (report.identity.contextId === undefined || - report.identity.contextId === freshReport.identity.contextId); - -const identityPublicationAdvanced = ( - report: ManagedWorkspaceDiscovery, - freshReport: ManagedWorkspaceDiscovery, -): boolean => - (report.identity.projectId === undefined && freshReport.identity.projectId !== undefined) || - (report.identity.checkoutId === undefined && freshReport.identity.checkoutId !== undefined) || - (report.identity.contextId === undefined && freshReport.identity.contextId !== undefined); - -/** A same-topology start may have published part of the Git identity meanwhile. */ -const concurrentIdentityPublication = ( - report: ManagedWorkspaceDiscovery, - freshReport: ManagedWorkspaceDiscovery, -): boolean => - report.state === "unregistered" && - freshReport.state === "unregistered" && - freshReport.conflicts.length === 0 && - freshReport.activeTransition === undefined && - sameManagedWorkspaceTopology(report, freshReport) && - identityPublicationIsMonotonic(report, freshReport) && - identityPublicationAdvanced(report, freshReport); - const newCheckoutTransitionMatches = ( transition: ManagedIdentityTransitionRecord | undefined, transitionId: string, @@ -363,9 +325,9 @@ export const makeWorkspaceIdentity = ({ }), ); } + const metadata = workspaceMetadata(inspection); if (inspection.kind === "ordinary-folder") { - const markerPath = ordinaryWorkspaceIdentityPath(canonicalPath); const marker = targetIdentity === undefined ? yield* ensureOrdinaryWorkspaceIdentity(canonicalPath, idFactory) @@ -382,15 +344,7 @@ export const makeWorkspaceIdentity = ({ return { // A folder keeps all three identities in one marker, so that // marker is both identity locations. - workspace: { - checkoutKind: "ordinary", - canonicalPath, - workspaceRoot: canonicalPath, - projectIdentityLocation: markerPath, - checkoutIdentityLocation: markerPath, - }, - context: { kind: "workspace" }, - contextDescriptor: { kind: "workspace" }, + ...metadata, identity: { projectId: identity?.projectId, checkoutId: identity?.checkoutId, @@ -433,23 +387,8 @@ export const makeWorkspaceIdentity = ({ targetIdentity.contextId, ); } - const head = inspection.head; return { - workspace: { - checkoutKind: checkoutKindOf(inspection), - canonicalPath: inspection.canonicalPath, - workspaceRoot: inspection.workspaceRoot, - projectIdentityLocation: inspection.commonDirectory, - checkoutIdentityLocation: inspection.gitDirectory, - }, - context: - head.kind === "detached" - ? { kind: "detached", commit: head.commit } - : { kind: "branch", branch: head.branch }, - contextDescriptor: - head.kind === "detached" - ? { kind: "detached" } - : { kind: "branch", locator: head.branch }, + ...metadata, identity: targetIdentity, identityMarkerCreated: checkoutIdentityCreated, }; @@ -460,24 +399,10 @@ export const makeWorkspaceIdentity = ({ const checkoutId = claimed.checkoutId; const head = inspection.head; return { - workspace: { - checkoutKind: checkoutKindOf(inspection), - canonicalPath: inspection.canonicalPath, - workspaceRoot: inspection.workspaceRoot, - projectIdentityLocation: inspection.commonDirectory, - checkoutIdentityLocation: inspection.gitDirectory, - }, + ...metadata, // An unborn branch names a context exactly as a born one does: it is // the state a fresh repository starts in, and a first start there // must not be treated as a detached `HEAD`. - context: - head.kind === "detached" - ? { kind: "detached", commit: head.commit } - : { kind: "branch", branch: head.branch }, - contextDescriptor: - head.kind === "detached" - ? { kind: "detached" } - : { kind: "branch", locator: head.branch }, identity: { projectId: claimed.projectId, checkoutId, @@ -613,10 +538,15 @@ export const makeWorkspaceIdentity = ({ * sequence, so an interrupted publication can only resume after every * expected value still matches. */ + interface ManagedFolderToGitRecoveryResult { + readonly report: ManagedWorkspaceDiscovery; + readonly identityMarkerCreated: boolean; + } + const migrateFolderToGit = ( report: ManagedWorkspaceDiscovery, ): Effect.Effect< - ManagedWorkspaceDiscovery, + ManagedFolderToGitRecoveryResult, | InvalidManagedIdentityError | DuplicateManagedIdentityError | UnsupportedGitWorkspaceError @@ -625,7 +555,9 @@ export const makeWorkspaceIdentity = ({ Effect.gen(function* () { const resuming = report.activeTransition?.kind === "folder-to-git"; if (!resuming) { - if (report.folderToGitClaims.length === 0) return report; + if (report.folderToGitClaims.length === 0) { + return { report, identityMarkerCreated: false }; + } if (report.folderToGitClaims.length > 1) { return yield* Effect.fail( new ManagedCheckoutConflictError({ @@ -641,7 +573,7 @@ export const makeWorkspaceIdentity = ({ report.identity.checkoutId !== undefined || report.identity.contextId !== undefined ) { - return report; + return { report, identityMarkerCreated: false }; } } const claim = @@ -657,7 +589,9 @@ export const makeWorkspaceIdentity = ({ canonicalPath: report.activeTransition.path, } : undefined); - if (claim === undefined || report.workspace.checkoutKind === "ordinary") return report; + if (claim === undefined || report.workspace.checkoutKind === "ordinary") { + return { report, identityMarkerCreated: false }; + } const path = report.workspace.workspaceRoot; const inspection = yield* withWorkspaceServices(inspectWorkspace(path)); if (inspection.kind !== "git-checkout") { @@ -715,6 +649,7 @@ export const makeWorkspaceIdentity = ({ ); } + let identityMarkerCreated = false; if (transition.phase === "reserved") { const ordinaryMarker = yield* withWorkspaceServices( readOrdinaryWorkspaceIdentityWithFileSystem(path), @@ -811,7 +746,7 @@ export const makeWorkspaceIdentity = ({ }), ); yield* publishConfig(GIT_PROJECT_ID_KEY, claim.projectId); - yield* withWorkspaceServices( + identityMarkerCreated = yield* withWorkspaceServices( publishGitCheckoutIdentity(inspection.gitDirectory, claim.checkoutId), ); if (inspection.head.kind !== "detached") { @@ -935,7 +870,7 @@ export const makeWorkspaceIdentity = ({ }); } const migrated = yield* discover(path); - return migrated; + return { report: migrated, identityMarkerCreated }; }); const requestedRecoveryPath = ( @@ -2277,8 +2212,5 @@ export const makeWorkspaceIdentity = ({ repairCopiedBranch, adoptContext, abandonIdentityTransition, - sameManagedWorkspaceTopology, - identityPublicationIsMonotonic, - concurrentIdentityPublication, }; }; diff --git a/packages/stack/src/managed/workspace-metadata.ts b/packages/stack/src/managed/workspace-metadata.ts new file mode 100644 index 0000000000..e2978ad190 --- /dev/null +++ b/packages/stack/src/managed/workspace-metadata.ts @@ -0,0 +1,58 @@ +import type { ManagedCheckoutKind, ManagedContextDescriptor, ManagedContextKind } from "./model.ts"; +import type { WorkspaceInspection } from "./git.ts"; +import { ordinaryWorkspaceIdentityPath } from "./paths.ts"; +import { checkoutKindOf } from "./topology.ts"; + +export interface ManagedWorkspaceDiscoveryWorkspace { + readonly checkoutKind: ManagedCheckoutKind; + readonly canonicalPath: string; + readonly workspaceRoot: string; + readonly projectIdentityLocation: string; + readonly checkoutIdentityLocation: string; +} + +export interface ManagedWorkspaceDiscoveryContext { + readonly kind: ManagedContextKind; + readonly branch?: string; + readonly commit?: string; +} + +export interface ManagedWorkspaceMetadata { + readonly workspace: ManagedWorkspaceDiscoveryWorkspace; + readonly context: ManagedWorkspaceDiscoveryContext; + readonly contextDescriptor: ManagedContextDescriptor; +} + +export const workspaceMetadata = (inspection: WorkspaceInspection): ManagedWorkspaceMetadata => { + if (inspection.kind === "ordinary-folder") { + const markerPath = ordinaryWorkspaceIdentityPath(inspection.canonicalPath); + return { + workspace: { + checkoutKind: "ordinary", + canonicalPath: inspection.canonicalPath, + workspaceRoot: inspection.canonicalPath, + projectIdentityLocation: markerPath, + checkoutIdentityLocation: markerPath, + }, + context: { kind: "workspace" }, + contextDescriptor: { kind: "workspace" }, + }; + } + return { + workspace: { + checkoutKind: checkoutKindOf(inspection), + canonicalPath: inspection.canonicalPath, + workspaceRoot: inspection.workspaceRoot, + projectIdentityLocation: inspection.commonDirectory, + checkoutIdentityLocation: inspection.gitDirectory, + }, + context: + inspection.head.kind === "detached" + ? { kind: "detached", commit: inspection.head.commit } + : { kind: "branch", branch: inspection.head.branch }, + contextDescriptor: + inspection.head.kind === "detached" + ? { kind: "detached" } + : { kind: "branch", locator: inspection.head.branch }, + }; +}; diff --git a/packages/stack/src/managed/workspace-settlement.ts b/packages/stack/src/managed/workspace-settlement.ts new file mode 100644 index 0000000000..033cae45ca --- /dev/null +++ b/packages/stack/src/managed/workspace-settlement.ts @@ -0,0 +1,72 @@ +import type { ManagedWorkspaceDiscovery } from "./discovery.ts"; + +export const sameManagedWorkspaceTopology = ( + before: ManagedWorkspaceDiscovery, + after: ManagedWorkspaceDiscovery, +): boolean => + before.workspace.checkoutKind === after.workspace.checkoutKind && + before.workspace.workspaceRoot === after.workspace.workspaceRoot && + before.workspace.projectIdentityLocation === after.workspace.projectIdentityLocation && + before.workspace.checkoutIdentityLocation === after.workspace.checkoutIdentityLocation && + before.context.kind === after.context.kind && + before.context.branch === after.context.branch && + before.context.commit === after.context.commit; + +export const identityPublicationIsMonotonic = ( + before: ManagedWorkspaceDiscovery, + after: ManagedWorkspaceDiscovery, +): boolean => + (before.identity.projectId === undefined || + before.identity.projectId === after.identity.projectId) && + (before.identity.checkoutId === undefined || + before.identity.checkoutId === after.identity.checkoutId) && + (before.identity.contextId === undefined || + before.identity.contextId === after.identity.contextId); + +const identityPublicationAdvanced = ( + before: ManagedWorkspaceDiscovery, + after: ManagedWorkspaceDiscovery, +): boolean => + (before.identity.projectId === undefined && after.identity.projectId !== undefined) || + (before.identity.checkoutId === undefined && after.identity.checkoutId !== undefined) || + (before.identity.contextId === undefined && after.identity.contextId !== undefined); + +/** A same-topology start may have published part of the Git identity meanwhile. */ +export const concurrentIdentityPublication = ( + before: ManagedWorkspaceDiscovery, + after: ManagedWorkspaceDiscovery, +): boolean => + before.state === "unregistered" && + after.state === "unregistered" && + after.conflicts.length === 0 && + after.activeTransition === undefined && + sameManagedWorkspaceTopology(before, after) && + identityPublicationIsMonotonic(before, after) && + identityPublicationAdvanced(before, after); + +export const benignConcurrentRegistration = ( + before: ManagedWorkspaceDiscovery, + after: ManagedWorkspaceDiscovery, +): boolean => { + const identityComplete = + after.identity.projectId !== undefined && + after.identity.checkoutId !== undefined && + after.identity.contextId !== undefined; + const newCheckoutReserved = + before.state === "unregistered" && + after.state === "transitioning" && + after.activeTransition?.kind === "new-checkout" && + after.activeTransition.path === after.workspace.workspaceRoot && + after.activeTransition.projectIdentityLocation === after.workspace.projectIdentityLocation && + after.conflicts.length === 0; + + return ( + before.state === "unregistered" && + sameManagedWorkspaceTopology(before, after) && + identityPublicationIsMonotonic(before, after) && + ((after.activeTransition === undefined && + ((after.state === "healthy" && identityComplete && after.conflicts.length === 0) || + concurrentIdentityPublication(before, after))) || + newCheckoutReserved) + ); +}; diff --git a/packages/stack/src/managed/workspace-settlement.unit.test.ts b/packages/stack/src/managed/workspace-settlement.unit.test.ts new file mode 100644 index 0000000000..7de355c810 --- /dev/null +++ b/packages/stack/src/managed/workspace-settlement.unit.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from "vitest"; +import type { ManagedWorkspaceDiscovery } from "./discovery.ts"; +import { benignConcurrentRegistration } from "./workspace-settlement.ts"; + +const report = (overrides: Partial = {}): ManagedWorkspaceDiscovery => ({ + state: "unregistered", + workspace: { + checkoutKind: "git", + canonicalPath: "/workspace", + workspaceRoot: "/workspace", + projectIdentityLocation: "/workspace/.git", + checkoutIdentityLocation: "/workspace/.git", + }, + context: { kind: "branch", branch: "main" }, + contextDescriptor: { kind: "branch", locator: "main" }, + identity: {}, + folderToGitClaims: [], + stacks: [], + locations: [], + activeOperations: [], + conflicts: [], + warnings: [], + recoveryOperations: [], + ...overrides, +}); + +describe("benignConcurrentRegistration", () => { + it("accepts a same-topology healthy winner", () => { + const before = report(); + const after = report({ + state: "healthy", + identity: { projectId: "project", checkoutId: "checkout", contextId: "context" }, + }); + expect(benignConcurrentRegistration(before, after)).toBe(true); + }); + + it("accepts monotonic partial identity publication", () => { + const before = report(); + const after = report({ identity: { projectId: "project" } }); + expect(benignConcurrentRegistration(before, after)).toBe(true); + }); + + it("accepts a same-workspace new-checkout reservation", () => { + const before = report(); + const after = report({ + state: "transitioning", + activeTransition: { + id: "transition", + kind: "new-checkout", + phase: "reserved", + path: "/workspace", + projectIdentityLocation: "/workspace/.git", + createdAt: "2026-08-15T00:00:00.000Z", + updatedAt: "2026-08-15T00:00:00.000Z", + }, + }); + expect(benignConcurrentRegistration(before, after)).toBe(true); + }); + + it("rejects topology changes and non-monotonic identities", () => { + expect( + benignConcurrentRegistration( + report(), + report({ workspace: { ...report().workspace, workspaceRoot: "/moved" } }), + ), + ).toBe(false); + expect( + benignConcurrentRegistration( + report({ identity: { projectId: "project" } }), + report({ identity: { projectId: "different-project" } }), + ), + ).toBe(false); + }); +});