From 9f6a89b36a319ec7a2fc257a485be4a17533e424 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 29 Jun 2026 09:57:34 +0000 Subject: [PATCH] fix(cli): treat existing migrations dir as success in declarative sync `db schema declarative sync` (and `db diff`/`db pull`) aborted with `AlreadyExists: FileSystem.makeDirectory (...\supabase\migrations)` when the migrations directory already existed. Go's `os.MkdirAll` returns nil for an existing directory, but Effect's Bun `FileSystem.makeDirectory` can surface an `AlreadyExists` SystemError even with `recursive: true` on some platforms (notably Windows / OneDrive reparse-point dirs), which regressed the command when it was ported to native TypeScript. Add a shared `legacyMakeDir` helper that recovers from the `AlreadyExists` reason so re-creating an existing directory is a no-op, matching `os.MkdirAll`, and route the migration/output writers in sync, diff, and pull through it. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AWa8Hr8BS2pZucac9r3wb7 --- .../legacy/commands/db/diff/diff.handler.ts | 13 +-- .../legacy/commands/db/pull/pull.handler.ts | 7 +- .../schema/declarative/sync/sync.handler.ts | 3 +- apps/cli/src/legacy/shared/legacy-make-dir.ts | 26 ++++++ .../shared/legacy-make-dir.unit.test.ts | 93 +++++++++++++++++++ 5 files changed, 132 insertions(+), 10 deletions(-) create mode 100644 apps/cli/src/legacy/shared/legacy-make-dir.ts create mode 100644 apps/cli/src/legacy/shared/legacy-make-dir.unit.test.ts 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 577ff3f6c1..73c0f7b6d5 100644 --- a/apps/cli/src/legacy/commands/db/diff/diff.handler.ts +++ b/apps/cli/src/legacy/commands/db/diff/diff.handler.ts @@ -11,6 +11,7 @@ import { LegacyDbConfigResolver } from "../../../shared/legacy-db-config.service import type { LegacyPgConnInput } from "../../../shared/legacy-db-connection.service.ts"; import type { LegacyDbConnType } from "../../../shared/legacy-db-target-flags.ts"; import { legacyGetHostname } from "../../../shared/legacy-hostname.ts"; +import { legacyMakeDir } from "../../../shared/legacy-make-dir.ts"; import { legacyToPostgresURL } from "../../../shared/legacy-postgres-url.ts"; import { legacySchemaToCsvField } from "../../../shared/legacy-schema-flags.ts"; import { legacyFindDropStatements } from "../../../shared/legacy-sql-split.ts"; @@ -277,9 +278,9 @@ export const legacyDbDiff = Effect.fn("legacy.db.diff")(function* (flags: Legacy // Create parent dirs first, matching Go's `writeOutput` → `utils.WriteFile` // (`internal/db/diff/explicit.go`, `internal/utils/misc.go`), so a nested // `--output tmp/diff.sql` doesn't fail when `tmp/` doesn't exist yet. - yield* fs - .makeDirectory(path.dirname(target), { recursive: true }) - .pipe(Effect.mapError((cause) => new LegacyDbDiffWriteError({ message: cause.message }))); + yield* legacyMakeDir(fs, path.dirname(target)).pipe( + Effect.mapError((cause) => new LegacyDbDiffWriteError({ message: cause.message })), + ); yield* fs .writeFileString(target, result.sql) .pipe(Effect.mapError((cause) => new LegacyDbDiffWriteError({ message: cause.message }))); @@ -457,9 +458,9 @@ export const legacyDbDiff = Effect.fn("legacy.db.diff")(function* (flags: Legacy timestamp, flags.file.value, ); - yield* fs - .makeDirectory(path.dirname(migrationPath), { recursive: true }) - .pipe(Effect.mapError((cause) => new LegacyDbDiffWriteError({ message: cause.message }))); + yield* legacyMakeDir(fs, path.dirname(migrationPath)).pipe( + Effect.mapError((cause) => new LegacyDbDiffWriteError({ message: cause.message })), + ); yield* fs .writeFileString(migrationPath, out) .pipe(Effect.mapError((cause) => new LegacyDbDiffWriteError({ message: cause.message }))); 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 6936a83c26..05721ac867 100644 --- a/apps/cli/src/legacy/commands/db/pull/pull.handler.ts +++ b/apps/cli/src/legacy/commands/db/pull/pull.handler.ts @@ -21,6 +21,7 @@ import { legacyResolveDeclarativeDir, } from "../../../shared/legacy-db-config.toml-read.ts"; import type { LegacyDbConnType } from "../../../shared/legacy-db-target-flags.ts"; +import { legacyMakeDir } from "../../../shared/legacy-make-dir.ts"; import { legacyToPostgresURL } from "../../../shared/legacy-postgres-url.ts"; import { legacySchemaToCsvField } from "../../../shared/legacy-schema-flags.ts"; import { LegacyLinkedProjectCache } from "../../../telemetry/legacy-linked-project-cache.service.ts"; @@ -540,9 +541,9 @@ export const legacyDbPull = Effect.fn("legacy.db.pull")(function* (flags: Legacy new LegacyDbPullInSyncError({ message: "No schema changes found" }), ); } - yield* fs - .makeDirectory(path.dirname(migrationPath), { recursive: true }) - .pipe(Effect.mapError((cause) => new LegacyDbPullWriteError({ message: cause.message }))); + yield* legacyMakeDir(fs, path.dirname(migrationPath)).pipe( + Effect.mapError((cause) => new LegacyDbPullWriteError({ message: cause.message })), + ); yield* fs.writeFileString(migrationPath, out).pipe( Effect.mapError( (cause) => diff --git a/apps/cli/src/legacy/commands/db/schema/declarative/sync/sync.handler.ts b/apps/cli/src/legacy/commands/db/schema/declarative/sync/sync.handler.ts index d70db8625c..61f276e4c0 100644 --- a/apps/cli/src/legacy/commands/db/schema/declarative/sync/sync.handler.ts +++ b/apps/cli/src/legacy/commands/db/schema/declarative/sync/sync.handler.ts @@ -16,6 +16,7 @@ import { legacyReadDbToml, legacyResolveDeclarativeDir, } from "../../../../../shared/legacy-db-config.toml-read.ts"; +import { legacyMakeDir } from "../../../../../shared/legacy-make-dir.ts"; import { legacyApplyMigrationFile } from "../../../../../shared/legacy-migration-apply.ts"; import { legacyReadProjectRefFile } from "../../../../../shared/legacy-temp-paths.ts"; import { LegacyLinkedProjectCache } from "../../../../../telemetry/legacy-linked-project-cache.service.ts"; @@ -276,7 +277,7 @@ export const legacyDbSchemaDeclarativeSync = Effect.fn("legacy.db.schema.declara // Step 5: write the timestamped migration file. const timestamp = formatTimestamp(yield* Clock.currentTimeMillis); const migrationPath = path.join(migrationsDir, `${timestamp}_${migrationName}.sql`); - yield* fs.makeDirectory(migrationsDir, { recursive: true }); + yield* legacyMakeDir(fs, migrationsDir); yield* fs.writeFileString(migrationPath, result.diffSQL); yield* output.raw(`Created new migration at ${legacyBold(migrationPath)}\n`, "stderr"); diff --git a/apps/cli/src/legacy/shared/legacy-make-dir.ts b/apps/cli/src/legacy/shared/legacy-make-dir.ts new file mode 100644 index 0000000000..864c363150 --- /dev/null +++ b/apps/cli/src/legacy/shared/legacy-make-dir.ts @@ -0,0 +1,26 @@ +import { Effect, FileSystem } from "effect"; +import type { PlatformError } from "effect/PlatformError"; + +/** + * `os.MkdirAll`-equivalent: create `dir` and any missing parents, treating an + * already-existing directory as success. + * + * Go's `os.MkdirAll` returns nil when the target is already a directory, so the + * Go CLI's migration writers never failed on a pre-existing `supabase/migrations`. + * Effect's Bun `FileSystem.makeDirectory` does not always match that: even with + * `recursive: true` it can surface an `AlreadyExists` `SystemError` for an + * existing directory on some platforms (notably Windows / OneDrive reparse + * points — see CLI-1849). Recover from that single reason so re-creating an + * existing directory is a no-op, and let every other failure propagate. + */ +export const legacyMakeDir = ( + fs: FileSystem.FileSystem, + dir: string, +): Effect.Effect => + fs + .makeDirectory(dir, { recursive: true }) + .pipe( + Effect.catchTag("PlatformError", (error) => + error.reason._tag === "AlreadyExists" ? Effect.void : Effect.fail(error), + ), + ); diff --git a/apps/cli/src/legacy/shared/legacy-make-dir.unit.test.ts b/apps/cli/src/legacy/shared/legacy-make-dir.unit.test.ts new file mode 100644 index 0000000000..62fab1a4bd --- /dev/null +++ b/apps/cli/src/legacy/shared/legacy-make-dir.unit.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Exit, FileSystem, Layer, PlatformError } from "effect"; + +import { legacyMakeDir } from "./legacy-make-dir.ts"; + +const DIR = "/home/user/project/supabase/migrations"; + +type SystemReason = Parameters[0]["_tag"]; + +/** A FileSystem whose `makeDirectory` always fails with the given system reason. */ +function failingFs(reason: SystemReason) { + const calls: Array<{ readonly path: string; readonly recursive?: boolean }> = []; + return { + calls, + layer: Layer.succeed( + FileSystem.FileSystem, + FileSystem.makeNoop({ + makeDirectory: (path, opts) => + Effect.suspend(() => { + calls.push({ path, recursive: opts?.recursive }); + return Effect.fail( + PlatformError.systemError({ + _tag: reason, + module: "FileSystem", + method: "makeDirectory", + description: reason, + pathOrDescriptor: path, + }), + ); + }), + }), + ), + }; +} + +const run = (dir: string) => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + return yield* legacyMakeDir(fs, dir); + }); + +describe("legacyMakeDir", () => { + it.effect("creates the directory recursively, matching os.MkdirAll", () => { + const calls: Array<{ readonly path: string; readonly recursive?: boolean }> = []; + const layer = Layer.succeed( + FileSystem.FileSystem, + FileSystem.makeNoop({ + makeDirectory: (path, opts) => + Effect.sync(() => { + calls.push({ path, recursive: opts?.recursive }); + }), + }), + ); + return run(DIR).pipe( + Effect.tap(() => + Effect.sync(() => { + expect(calls).toEqual([{ path: DIR, recursive: true }]); + }), + ), + Effect.provide(layer), + ); + }); + + it.effect("treats an already-existing directory as success (CLI-1849)", () => { + const fs = failingFs("AlreadyExists"); + return run(DIR).pipe( + Effect.exit, + Effect.tap((exit) => + Effect.sync(() => { + expect(Exit.isSuccess(exit)).toBe(true); + expect(fs.calls).toEqual([{ path: DIR, recursive: true }]); + }), + ), + Effect.provide(fs.layer), + ); + }); + + it.effect("propagates every other filesystem error", () => { + const fs = failingFs("PermissionDenied"); + return run(DIR).pipe( + Effect.exit, + Effect.tap((exit) => + Effect.sync(() => { + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + expect(JSON.stringify(exit.cause)).toContain("PermissionDenied"); + } + }), + ), + Effect.provide(fs.layer), + ); + }); +});