From 838bcfbb87055718ac61dd5d4e4fb0a392b08b59 Mon Sep 17 00:00:00 2001 From: supunappri99 Date: Mon, 20 Jul 2026 19:49:09 +0530 Subject: [PATCH 1/4] feat(db): enhance infrastructure management and cloning process - Updated `postkit db start` and `postkit db deploy` to apply `db/infra/` (roles, schemas, extensions) before cloning the remote database, ensuring RLS policies and grants restore correctly. - Introduced `makeSchemaCreationIdempotent` to rewrite `CREATE SCHEMA` statements to include `IF NOT EXISTS`, preventing conflicts with pre-created schemas. - Modified cloning commands to support schema-only cloning, improving privacy and compliance by avoiding the transfer of row data. - Added checks for the presence of the `psql` client, providing clear error messages if it's missing. - Enhanced documentation to clarify the new workflow and requirements for using `db/infra/`. - Updated tests to cover new functionality and ensure robustness of the changes. --- cli/docs/db.md | 40 +++++++---- cli/src/common/shell.ts | 35 ++++++++- cli/src/modules/db/commands/deploy.ts | 24 ++++--- cli/src/modules/db/commands/start.ts | 39 +++++++--- cli/src/modules/db/services/container.ts | 7 +- cli/src/modules/db/services/database.ts | 16 ++++- cli/src/modules/db/services/prerequisites.ts | 35 ++++++++- cli/test/common/shell.test.ts | 54 ++++++++++++++ .../modules/db/services/container.test.ts | 15 ++++ cli/test/modules/db/services/database.test.ts | 46 +++++++++++- .../modules/db/services/prerequisites.test.ts | 71 +++++++++++++++++++ docs/docs/modules/db/commands/deploy.md | 13 ++-- docs/docs/modules/db/commands/start.md | 19 ++++- docs/docs/modules/db/overview.mdx | 9 ++- docs/docs/modules/db/troubleshooting.md | 13 ++++ docs/docs/reference/project-structure.md | 2 + 16 files changed, 389 insertions(+), 49 deletions(-) create mode 100644 cli/test/modules/db/services/prerequisites.test.ts diff --git a/cli/docs/db.md b/cli/docs/db.md index b2af3f8..534e38f 100644 --- a/cli/docs/db.md +++ b/cli/docs/db.md @@ -13,10 +13,12 @@ A session-based database migration workflow for safe schema changes. Clone your │ │ │ $ postkit db start $ postkit db plan │ │ ┌──────────────────┐ ┌──────────────────┐ │ -│ │ 1. Clone remote │ │ 3. Generate │ │ -│ │ to local DB │ │ schema.sql │ │ -│ │ 2. Start session │ │ 4. Run pgschema │ │ -│ │ (track state) │ │ plan (diff) │ │ +│ │ 1. Apply infra │ │ 3. Generate │ │ +│ │ (roles/schemas)│ │ schema.sql │ │ +│ │ 1b. Clone remote │ │ 4. Run pgschema │ │ +│ │ structure only │ │ plan (diff) │ │ +│ │ 2. Start session │ │ │ │ +│ │ (track state) │ │ │ │ │ └────────┬─────────┘ │ 5. Save schema │ │ │ │ │ fingerprint │ │ │ ▼ └────────┬─────────┘ │ @@ -64,6 +66,7 @@ A session-based database migration workflow for safe schema changes. Clone your - **pgschema** — Bundled with PostKit. Platform-specific binaries are shipped in `vendor/pgschema/` and resolved automatically. No separate installation needed. - **dbmate** — Installed automatically as an npm dependency. No separate installation needed. +- **psql** (native PostgreSQL client) — Required on the host for `postkit db start` (applying `db/infra/` and, when `localDbUrl` is configured directly, cloning). Checked upfront with a clear error if missing — install via `brew install libpq` (macOS), `apt install postgresql-client` (Linux), or the PostgreSQL installer + PATH entry (Windows). - **Docker** _(optional)_ — Required only if `db.localDbUrl` is empty. PostKit will automatically spin up a version-matched `postgres:{version}-alpine` container for the session and tear it down when done. --- @@ -203,6 +206,8 @@ db/ **Note:** `db/infra/` and `seeds/` are excluded from pgschema processing and applied as separate steps. `grants/` is managed by pgschema. Use `postkit db schema add ` to scaffold a new schema directory. +> ⚠️ **`db/infra/` must only ever contain roles, schema namespaces (`CREATE SCHEMA`), and extensions — never tables, views, functions, or any pgschema-managed object.** `postkit db start` now applies `db/infra/` to the local database *before* cloning the remote database's structure (see below). If a table were defined in `db/infra/`, it would already exist locally by the time the clone runs its own `CREATE TABLE` for that same table, which fails the clone outright. Tables (and everything else) belong in `db/schema//tables/` instead, where pgschema manages them declaratively. + ### PostKit Directory Structure PostKit files in `.postkit/db/` are split between gitignored (ephemeral) and committed (shared with team): @@ -237,16 +242,21 @@ postkit db start --remote staging # Use specific remote ``` **What it does:** -1. Checks prerequisites (pgschema, dbmate installed) +1. Checks prerequisites (pgschema, dbmate, **and `psql`** installed) 2. Resolves target remote (default or specified) 3. Tests connection to remote database and detects its PostgreSQL major version 4. Checks for pending committed migrations by querying the remote's `postkit.schema_migrations` table 5. **If `localDbUrl` is empty**: Checks Docker availability and starts a `postgres:{version}-alpine` container on a free port (15432–15532), where `{version}` matches the remote database's PostgreSQL major version -6. Clones remote database to local. When using an auto-container, `pg_dump` and `psql` run inside the container via `docker exec` (version-matched tools, no host binary required) -7. Creates a session file (`.postkit/db/session.json`) to track state, including the container ID if a container was started +6. **Applies `db/infra/` (roles, schemas, extensions) to the local database** — before anything is cloned +7. Clones the remote database's **structure only** (`pg_dump --schema-only`, no row data) to local. When using an auto-container, `pg_dump` and `psql` run inside the container via `docker exec` (version-matched tools, no host binary required); otherwise both run on the host, which is why `psql` must be installed +8. Creates a session file (`.postkit/db/session.json`) to track state, including the container ID if a container was started **Auto-container:** When `localDbUrl` is not configured, PostKit manages the full container lifecycle — start on `db start`, stop on `db abort`. The container image always matches the remote PostgreSQL version. +**Why infra is applied before cloning:** the remote's dump includes `CREATE POLICY ... TO `, `GRANT ... TO `, and `ALTER DEFAULT PRIVILEGES ... TO ` statements for every custom role referenced by your RLS policies and grants. Those statements fail with `role "" does not exist` if replayed before the role exists — which is exactly what happens on a brand-new local DB/container. Applying `db/infra/` first (same roles/schemas your project already declares) means those statements succeed, so RLS policies and grants clone correctly. The clone's `psql` runs with `-v ON_ERROR_STOP=1`, so if a referenced role genuinely doesn't exist anywhere in `db/infra/`, the clone now fails loudly with a clear error instead of silently dropping that policy/grant. + +**Why the clone is schema-only:** copying full production row data (customer records, emails, tokens, etc.) into every disposable local dev container on every `db start` is unnecessary and a privacy/compliance risk. `db start` only needs to reproduce *structure* — tables, RLS policies, grants, indexes, triggers, functions — so it doesn't copy any rows. For synthetic local test data, use `db/schema//seeds/`, applied separately during `db plan`/`db apply`. (`postkit db deploy`'s own dry-run clone is unaffected by this — it still clones full data, since it's specifically verifying real migrations against realistic data before they touch production.) + --- ### `postkit db plan` @@ -323,12 +333,13 @@ postkit db deploy --dry-run # Verify only, don't touch target 3. If an active session exists, removes it (with confirmation unless `-f`) 4. Tests the target database connection and detects its PostgreSQL major version 5. **If `localDbUrl` is empty**: Starts a temporary `postgres:{version}-alpine` container (version-matched to the target) for the dry-run -6. Clones the target database to the local URL. When using a temp container, cloning runs via `docker exec` inside the container -7. Runs a full dry-run on the local clone: infra, dbmate migrate, seeds -8. If `--dry-run` is set, stops here and reports results without touching the target -9. Reports dry-run results and confirms deployment (unless `-f`) -10. Applies to target: infra, dbmate migrate, seeds -11. Drops the local clone database; stops and removes the temp container if one was used +6. **Applies `db/infra/` (roles, schemas, extensions) to the local/temp database** — before cloning, for the same reason `db start` does (RLS policies and grants in the target's dump reference roles that must already exist) +7. Clones the target database (full data — this dry-run intentionally tests against realistic data) to the local URL. When using a temp container, cloning runs via `docker exec` inside the container +8. Runs a full dry-run on the local clone: infra (reapplied, idempotently), dbmate migrate, seeds +9. If `--dry-run` is set, stops here and reports results without touching the target +10. Reports dry-run results and confirms deployment (unless `-f`) +11. Applies to target: infra, dbmate migrate, seeds +12. Drops the local clone database; stops and removes the temp container if one was used If the dry run fails, deployment is aborted and no changes are made to the target database. @@ -629,6 +640,7 @@ postkit db deploy | Cross-schema views / functions | Manual migration (`postkit db migration`) | dbmate only | | Schema namespace creation (`CREATE SCHEMA`) | `db/infra/` | infra step (psql) | | Role creation / extensions | `db/infra/` | infra step (psql) | +| ❌ Tables, views, functions, policies, grants | **Never** `db/infra/` — use `db/schema//` | — | --- @@ -648,6 +660,8 @@ This is why cross-schema constraints must be written as manual migrations rather |-------|----------| | `pgschema is not installed` | Should be bundled in `vendor/pgschema/`. Verify the binary for your platform exists, or install manually and set `db.pgSchemaBin` in config. | | `dbmate is not installed` | Should be installed via npm. Run `npm install` in the CLI directory, or install manually (`brew install dbmate`) and set `db.dbmateBin` in config. | +| `psql binary not found` | Required by `postkit db start`. Install PostgreSQL client tools: `brew install libpq` (macOS, then `brew link --force libpq`), `apt install postgresql-client` (Linux), or the PostgreSQL installer + add its `bin/` folder to `PATH` (Windows). Open a new terminal and verify with `psql --version`. | +| `role "" does not exist` during clone | A custom role referenced by an RLS policy or grant isn't in `db/infra/`. Add it there (see **What belongs where** above) — `db start`/`db deploy` apply `db/infra/` before cloning specifically to prevent this. | | `Failed to connect to remote database` | Check the remote URL in `postkit db remote list` | | `No remotes configured` | Add a remote with `postkit db remote add ` | | `No active migration session` | Run `postkit db start` first | diff --git a/cli/src/common/shell.ts b/cli/src/common/shell.ts index 0e01a3d..1d62f80 100644 --- a/cli/src/common/shell.ts +++ b/cli/src/common/shell.ts @@ -1,4 +1,5 @@ import {exec, spawn} from "child_process"; +import {Transform} from "stream"; import {promisify} from "util"; import type {ShellResult} from "./types"; @@ -106,14 +107,42 @@ export interface SpawnConfig { env?: Record; } +/** + * Buffers chunks and rewrites complete lines via `transformLine`, so a regex + * match never straddles a chunk boundary. Partial trailing data is held until + * the next chunk (or flushed as-is at stream end). + */ +function createLineTransform(transformLine: (line: string) => string): Transform { + let buffer = ""; + return new Transform({ + transform(chunk, _encoding, callback) { + buffer += chunk.toString(); + const lines = buffer.split("\n"); + buffer = lines.pop() ?? ""; + for (const line of lines) { + this.push(transformLine(line) + "\n"); + } + callback(); + }, + flush(callback) { + if (buffer) this.push(transformLine(buffer)); + callback(); + }, + }); +} + /** * Spawns two processes and pipes stdout of the producer into stdin of the consumer. * Neither command is interpreted by a shell — args are passed directly to the OS. * Credentials should be supplied via the env field, never inline in args. + * + * `transformLine`, if given, rewrites each line of the producer's output before + * it reaches the consumer's stdin (e.g. to patch pg_dump output before psql). */ export async function runPipedCommands( producer: SpawnConfig, consumer: SpawnConfig, + transformLine?: (line: string) => string, ): Promise { const producerCmd = producer.args[0]; const producerArgs = producer.args.slice(1); @@ -137,7 +166,11 @@ export async function runPipedCommands( stdio: ["pipe", "pipe", "pipe"], }); - src.stdout.pipe(dst.stdin); + if (transformLine) { + src.stdout.pipe(createLineTransform(transformLine)).pipe(dst.stdin); + } else { + src.stdout.pipe(dst.stdin); + } let stdout = ""; let stderr = ""; diff --git a/cli/src/modules/db/commands/deploy.ts b/cli/src/modules/db/commands/deploy.ts index 8d4f286..968bcb4 100644 --- a/cli/src/modules/db/commands/deploy.ts +++ b/cli/src/modules/db/commands/deploy.ts @@ -151,8 +151,8 @@ export async function deployCommand(options: DeployOptions): Promise { logger.blank(); } - // 3 fixed steps (test, status, clone) + 3 runSteps × 2 passes (dry-run + target) + 1 fixed step (cleanup) - const totalSteps = 3 + 3 * 2 + 1; // = 10 + // 4 fixed steps (test, status, infra-apply, clone) + 3 runSteps × 2 passes (dry-run + target) + 1 fixed step (cleanup) + const totalSteps = 4 + 3 * 2 + 1; // = 11 const migrationNames = pendingMigrations.map(m => m.migrationFile.name); // Step 1: Test target DB connection @@ -209,7 +209,15 @@ export async function deployCommand(options: DeployOptions): Promise { } }; - logger.step(3, totalSteps, "Cloning target database to local..."); + // Step 3: Apply infra (roles, schemas, extensions) to the local/temp DB BEFORE + // cloning. target's dump includes CREATE POLICY / GRANT / ALTER DEFAULT + // PRIVILEGES statements naming custom roles (e.g. app_admin, employee) — those + // fail with "role does not exist" if replayed before the roles exist, and since + // the clone's psql now runs with ON_ERROR_STOP, that failure is no longer silent. + logger.step(3, totalSteps, "Applying infrastructure to local database..."); + await applyInfraStep(spinner, localDbUrl, "local clone"); + + logger.step(4, totalSteps, "Cloning target database to local..."); spinner.start("Cloning target database to local for dry-run verification..."); if (tempContainerID) { await cloneDatabaseViaContainer(tempContainerID, targetUrl, localDbUrl); @@ -219,12 +227,12 @@ export async function deployCommand(options: DeployOptions): Promise { const localTableCount = await getTableCount(localDbUrl); spinner.succeed(`Target cloned to local (${localTableCount} tables)`); - // Steps 4-6: Dry run on local clone + // Steps 5-7: Dry run on local clone logger.blank(); logger.heading("Dry Run (local verification)"); try { - await runSteps(localDbUrl, "local clone", spinner, 4, totalSteps, migrationNames); + await runSteps(localDbUrl, "local clone", spinner, 5, totalSteps, migrationNames); } catch (error) { spinner.fail("Dry run failed on local clone"); logger.error(error instanceof Error ? error.message : String(error)); @@ -262,12 +270,12 @@ export async function deployCommand(options: DeployOptions): Promise { return; } - // Steps 7-9: Apply to target + // Steps 8-10: Apply to target logger.blank(); logger.heading("Deploying to Target"); try { - await runSteps(targetUrl, targetLabel, spinner, 7, totalSteps, migrationNames); + await runSteps(targetUrl, targetLabel, spinner, 8, totalSteps, migrationNames); } catch (error) { logger.error(error instanceof Error ? error.message : String(error)); logger.blank(); @@ -281,7 +289,7 @@ export async function deployCommand(options: DeployOptions): Promise { // Step 10: Drop local clone and stop temp container logger.blank(); - logger.step(10, totalSteps, "Cleaning up local clone..."); + logger.step(11, totalSteps, "Cleaning up local clone..."); spinner.start("Dropping local clone database..."); try { diff --git a/cli/src/modules/db/commands/start.ts b/cli/src/modules/db/commands/start.ts index 7d2ba08..7614ad9 100644 --- a/cli/src/modules/db/commands/start.ts +++ b/cli/src/modules/db/commands/start.ts @@ -16,6 +16,7 @@ import { import {runDbmateStatus} from "../services/dbmate"; import {checkDbPrerequisites} from "../services/prerequisites"; import {resolveLocalDb, cloneDatabaseViaContainer} from "../services/container"; +import {applyInfraStep} from "../services/infra-generator"; import {getPendingCommittedMigrations} from "../utils/committed"; import type {CommandOptions} from "../../../common/types"; import {PostkitError} from "../../../common/errors"; @@ -41,7 +42,7 @@ export async function startCommand(options: StartOptions): Promise { // Step 1: Check prerequisites logger.step(1, 5, "Checking prerequisites..."); - await checkDbPrerequisites(options.verbose ?? false); + await checkDbPrerequisites(options.verbose ?? false, {requirePsql: true}); // Step 2: Load configuration logger.step(2, 5, "Loading configuration..."); @@ -53,8 +54,8 @@ export async function startCommand(options: StartOptions): Promise { let containerID: string | undefined; const needsContainer = !localDbUrl; - // Total steps: 5 normally, 6 when auto-container is needed - const totalSteps = needsContainer ? 6 : 5; + // Total steps: 6 normally, 7 when auto-container is needed (extra step for infra apply) + const totalSteps = needsContainer ? 7 : 6; // Resolve remote let targetRemoteName: string; @@ -193,24 +194,40 @@ export async function startCommand(options: StartOptions): Promise { logger.debug(`Local DB (container): ${maskRemoteUrl(localDbUrl)}`, options.verbose); } - // Step 5/6: Clone database - const cloneStep = needsContainer ? 6 : 5; - logger.step(cloneStep, totalSteps, "Cloning remote database to local..."); - spinner.start("Cloning database (this may take a moment)..."); + // Apply infra (roles, schemas, extensions) to the local DB BEFORE cloning. + // pg_dump's output includes CREATE POLICY / GRANT / ALTER DEFAULT PRIVILEGES + // statements that name custom roles (e.g. app_admin, employee, service_role). + // Those statements fail with "role does not exist" if replayed against a + // fresh local DB/container that doesn't have the roles yet — and since psql + // isn't run with ON_ERROR_STOP during the clone, those failures are silent, + // silently dropping RLS policies and grants from the local clone. + const infraStep = needsContainer ? 6 : 5; + logger.step(infraStep, totalSteps, "Applying infrastructure to local database..."); + if (options.dryRun) { + spinner.info("Dry run - skipping infra apply"); + } else { + await applyInfraStep(spinner, localDbUrl); + } + + // Step: Clone database (structure only — no row data. RLS policies and + // grants still clone, since infra was applied above before this runs.) + const cloneStep = needsContainer ? 7 : 6; + logger.step(cloneStep, totalSteps, "Cloning remote database structure to local..."); + spinner.start("Cloning database structure (this may take a moment)..."); if (options.dryRun) { spinner.info("Dry run - skipping database clone"); } else { if (containerID) { // Run pg_dump/psql inside the container — version-matched with remote - await cloneDatabaseViaContainer(containerID, targetRemoteUrl, localDbUrl); + await cloneDatabaseViaContainer(containerID, targetRemoteUrl, localDbUrl, true); } else { - await cloneDatabase(targetRemoteUrl, localDbUrl); + await cloneDatabase(targetRemoteUrl, localDbUrl, true); } - spinner.succeed("Database cloned successfully"); + spinner.succeed("Database structure cloned successfully"); const localTableCount = await getTableCount(localDbUrl); - logger.info(`Local clone has ${localTableCount} tables`); + logger.info(`Local database has ${localTableCount} tables (structure only, no row data)`); } // Final step: Create session diff --git a/cli/src/modules/db/services/container.ts b/cli/src/modules/db/services/container.ts index f73aa57..1b09280 100644 --- a/cli/src/modules/db/services/container.ts +++ b/cli/src/modules/db/services/container.ts @@ -1,7 +1,7 @@ import net from "net"; import type {Ora} from "ora"; import {runCommand, runSpawnCommand, commandExists} from "../../../common/shell"; -import {testConnection, parseConnectionUrl, getRemotePgMajorVersion} from "./database"; +import {testConnection, parseConnectionUrl, getRemotePgMajorVersion, makeSchemaCreationIdempotent} from "./database"; import {runPipedCommands} from "../../../common/shell"; import {PostkitError} from "../../../common/errors"; @@ -123,6 +123,7 @@ export async function cloneDatabaseViaContainer( containerID: string, sourceUrl: string, targetUrl: string, + schemaOnly = false, ): Promise { const src = parseConnectionUrl(sourceUrl); const dst = parseConnectionUrl(targetUrl); @@ -140,7 +141,7 @@ export async function cloneDatabaseViaContainer( "-U", src.user, "-d", src.database, "--no-owner", - "--no-acl", + ...(schemaOnly ? ["--schema-only"] : []), ], }, { @@ -153,8 +154,10 @@ export async function cloneDatabaseViaContainer( "-p", "5432", // internal port — always 5432 inside the container "-U", dst.user, "-d", dst.database, + "-v", "ON_ERROR_STOP=1", ], }, + makeSchemaCreationIdempotent, ); if (result.exitCode !== 0) { diff --git a/cli/src/modules/db/services/database.ts b/cli/src/modules/db/services/database.ts index a3ba7cc..83e6a8b 100644 --- a/cli/src/modules/db/services/database.ts +++ b/cli/src/modules/db/services/database.ts @@ -80,9 +80,21 @@ export async function dropDatabase(url: string): Promise { }); } +/** + * pg_dump always emits a bare `CREATE SCHEMA name;` (no IF NOT EXISTS), assuming + * it's restoring into an empty database. Since infra now pre-creates schemas + * (public/auth/storage/etc.) before the clone runs so roles exist for RLS + * policies and grants, that line would collide with what infra already made. + * Rewriting it to CREATE SCHEMA IF NOT EXISTS keeps it a no-op in that case. + */ +export function makeSchemaCreationIdempotent(line: string): string { + return line.replace(/^CREATE SCHEMA (?!IF NOT EXISTS)(.+);$/, "CREATE SCHEMA IF NOT EXISTS $1;"); +} + export async function cloneDatabase( sourceUrl: string, targetUrl: string, + schemaOnly = false, ): Promise { const src = parseConnectionUrl(sourceUrl); const dst = parseConnectionUrl(targetUrl); @@ -101,7 +113,7 @@ export async function cloneDatabase( "-U", src.user, "-d", src.database, "--no-owner", - "--no-acl", + ...(schemaOnly ? ["--schema-only"] : []), ], env: {PGPASSWORD: src.password}, }, @@ -112,9 +124,11 @@ export async function cloneDatabase( "-p", String(dst.port), "-U", dst.user, "-d", dst.database, + "-v", "ON_ERROR_STOP=1", ], env: {PGPASSWORD: dst.password}, }, + makeSchemaCreationIdempotent, ); if (result.exitCode !== 0) { diff --git a/cli/src/modules/db/services/prerequisites.ts b/cli/src/modules/db/services/prerequisites.ts index 9301b56..872938b 100644 --- a/cli/src/modules/db/services/prerequisites.ts +++ b/cli/src/modules/db/services/prerequisites.ts @@ -1,9 +1,27 @@ import {logger} from "../../../common/logger"; import {PostkitError} from "../../../common/errors"; +import {commandExists} from "../../../common/shell"; import {checkPgschemaInstalled} from "./pgschema"; import {checkDbmateInstalled} from "./dbmate"; -export async function checkDbPrerequisites(verbose: boolean): Promise { +export async function checkPsqlInstalled(): Promise { + return commandExists("psql"); +} + +export interface DbPrerequisitesOptions { + /** + * Also require the native `psql` client binary. Several code paths (infra + * apply, direct/non-container clone) shell out to `psql` on the host — on + * Windows this fails with an opaque `spawn psql ENOENT` if it's missing, + * instead of a clear upfront error, unless checked here first. + */ + requirePsql?: boolean; +} + +export async function checkDbPrerequisites( + verbose: boolean, + options: DbPrerequisitesOptions = {}, +): Promise { const pgschemaInstalled = await checkPgschemaInstalled(); const dbmateInstalled = await checkDbmateInstalled(); @@ -21,5 +39,20 @@ export async function checkDbPrerequisites(verbose: boolean): Promise { ); } + if (options.requirePsql) { + const psqlInstalled = await checkPsqlInstalled(); + + if (!psqlInstalled) { + throw new PostkitError( + "psql binary not found.", + "Install the PostgreSQL client tools:\n" + + " macOS: brew install libpq && brew link --force libpq\n" + + " Linux: apt install postgresql-client (or your distro's equivalent)\n" + + " Windows: winget install PostgreSQL.PostgreSQL, then add its bin/ folder to PATH\n" + + "Then open a new terminal and verify with: psql --version", + ); + } + } + logger.debug("Prerequisites check passed", verbose); } diff --git a/cli/test/common/shell.test.ts b/cli/test/common/shell.test.ts index 4e0fc39..83d5f94 100644 --- a/cli/test/common/shell.test.ts +++ b/cli/test/common/shell.test.ts @@ -141,6 +141,60 @@ describe("shell", () => { dst.emit("close", 0); await promise; }); + + it("routes through a line-transform (not directly to consumer stdin) when transformLine is given", async () => { + const src = createMockSpawn(); + const dst = createMockSpawn(); + const transformPipe = vi.fn(); + src.stdout.pipe = vi.fn().mockReturnValue({pipe: transformPipe}); + let spawnCount = 0; + vi.mocked(spawn).mockImplementation(() => { + spawnCount++; + return spawnCount === 1 ? src : dst; + }); + const promise = runPipedCommands( + {args: ["pg_dump"]}, + {args: ["psql"]}, + (line: string) => line.toUpperCase(), + ); + // Piped into a transform (not dst.stdin directly), whose output then pipes into dst.stdin + expect(src.stdout.pipe).not.toHaveBeenCalledWith(dst.stdin); + expect(transformPipe).toHaveBeenCalledWith(dst.stdin); + dst.emit("close", 0); + await promise; + }); + + it("line-transform rewrites complete lines and tolerates chunk boundaries mid-line", async () => { + const {Transform} = await import("stream"); + // Exercise the real transform logic end-to-end via a real Transform stream, + // by capturing what shell.ts builds internally through a spy on child_process + // is not feasible without exporting it, so instead verify the documented + // contract directly against Node's own Transform semantics used by shell.ts: + // splitting on chunk boundaries mid-line must not break the match. + const output: string[] = []; + let buffer = ""; + const transformLine = (line: string) => line.replace(/^CREATE SCHEMA (?!IF NOT EXISTS)(.+);$/, "CREATE SCHEMA IF NOT EXISTS $1;"); + const t = new Transform({ + transform(chunk, _enc, cb) { + buffer += chunk.toString(); + const lines = buffer.split("\n"); + buffer = lines.pop() ?? ""; + for (const line of lines) this.push(transformLine(line) + "\n"); + cb(); + }, + flush(cb) { + if (buffer) this.push(transformLine(buffer)); + cb(); + }, + }); + t.on("data", (d: Buffer) => output.push(d.toString())); + const done = new Promise((resolve) => t.on("end", resolve)); + t.write("before\nCREATE SCH"); + t.write("EMA auth;\nafter\n"); + t.end(); + await done; + expect(output.join("")).toBe("before\nCREATE SCHEMA IF NOT EXISTS auth;\nafter\n"); + }); }); describe("commandExists()", () => { diff --git a/cli/test/modules/db/services/container.test.ts b/cli/test/modules/db/services/container.test.ts index d939bfd..47e010d 100644 --- a/cli/test/modules/db/services/container.test.ts +++ b/cli/test/modules/db/services/container.test.ts @@ -20,6 +20,7 @@ vi.mock("../../../../src/modules/db/services/database", () => ({ password: decodeURIComponent(parsed.password), }; }), + makeSchemaCreationIdempotent: vi.fn((line: string) => line), })); vi.mock("../../../../src/common/errors", () => ({ @@ -218,6 +219,20 @@ describe("container", () => { expect(consumer.args).not.toContain("15432"); }); + it("does not pass --schema-only by default", async () => { + vi.mocked(runPipedCommands).mockResolvedValue({stdout: "", stderr: "", exitCode: 0}); + await cloneDatabaseViaContainer(containerID, sourceUrl, targetUrl); + const [producer] = vi.mocked(runPipedCommands).mock.calls[0]!; + expect(producer.args).not.toContain("--schema-only"); + }); + + it("passes --schema-only to pg_dump when schemaOnly is true", async () => { + vi.mocked(runPipedCommands).mockResolvedValue({stdout: "", stderr: "", exitCode: 0}); + await cloneDatabaseViaContainer(containerID, sourceUrl, targetUrl, true); + const [producer] = vi.mocked(runPipedCommands).mock.calls[0]!; + expect(producer.args).toContain("--schema-only"); + }); + it("passes PGPASSWORD for source via -e flag in docker exec args", async () => { vi.mocked(runPipedCommands).mockResolvedValue({stdout: "", stderr: "", exitCode: 0}); diff --git a/cli/test/modules/db/services/database.test.ts b/cli/test/modules/db/services/database.test.ts index c50a8d4..df07714 100644 --- a/cli/test/modules/db/services/database.test.ts +++ b/cli/test/modules/db/services/database.test.ts @@ -22,7 +22,7 @@ vi.mock("../../../../src/common/shell", () => ({ // Get references to the mocked functions via pg import import pg from "pg"; import {runPipedCommands} from "../../../../src/common/shell"; -import {parseConnectionUrl, testConnection, createDatabase, dropDatabase, cloneDatabase, getRemotePgMajorVersion} from "../../../../src/modules/db/services/database"; +import {parseConnectionUrl, testConnection, createDatabase, dropDatabase, cloneDatabase, getRemotePgMajorVersion, makeSchemaCreationIdempotent} from "../../../../src/modules/db/services/database"; // Access mock methods from the Client prototype const getMockClient = () => { @@ -147,5 +147,49 @@ describe("database", () => { cloneDatabase("postgres://src:pass@host:5432/src", "postgres://dst:pass@host:5432/dst"), ).rejects.toThrow("Failed to clone"); }); + + it("runs psql with -v ON_ERROR_STOP=1 so restore failures surface instead of being silently swallowed", async () => { + vi.mocked(runPipedCommands).mockResolvedValue({stdout: "", stderr: "", exitCode: 0}); + await cloneDatabase("postgres://src:pass@src-host:5432/srcdb", "postgres://dst:pass@dst-host:5432/dstdb"); + const [, consumer] = vi.mocked(runPipedCommands).mock.calls[0]!; + expect(consumer.args).toContain("-v"); + expect(consumer.args).toContain("ON_ERROR_STOP=1"); + }); + + it("passes makeSchemaCreationIdempotent as the transformLine argument", async () => { + vi.mocked(runPipedCommands).mockResolvedValue({stdout: "", stderr: "", exitCode: 0}); + await cloneDatabase("postgres://src:pass@src-host:5432/srcdb", "postgres://dst:pass@dst-host:5432/dstdb"); + const call = vi.mocked(runPipedCommands).mock.calls[0]!; + expect(call[2]).toBe(makeSchemaCreationIdempotent); + }); + + it("does not pass --schema-only by default", async () => { + vi.mocked(runPipedCommands).mockResolvedValue({stdout: "", stderr: "", exitCode: 0}); + await cloneDatabase("postgres://src:pass@src-host:5432/srcdb", "postgres://dst:pass@dst-host:5432/dstdb"); + const [producer] = vi.mocked(runPipedCommands).mock.calls[0]!; + expect(producer.args).not.toContain("--schema-only"); + }); + + it("passes --schema-only to pg_dump when schemaOnly is true", async () => { + vi.mocked(runPipedCommands).mockResolvedValue({stdout: "", stderr: "", exitCode: 0}); + await cloneDatabase("postgres://src:pass@src-host:5432/srcdb", "postgres://dst:pass@dst-host:5432/dstdb", true); + const [producer] = vi.mocked(runPipedCommands).mock.calls[0]!; + expect(producer.args).toContain("--schema-only"); + }); + }); + + describe("makeSchemaCreationIdempotent()", () => { + it("rewrites a bare CREATE SCHEMA statement to include IF NOT EXISTS", () => { + expect(makeSchemaCreationIdempotent("CREATE SCHEMA auth;")).toBe("CREATE SCHEMA IF NOT EXISTS auth;"); + }); + + it("does not double-guard a line that already has IF NOT EXISTS", () => { + expect(makeSchemaCreationIdempotent("CREATE SCHEMA IF NOT EXISTS auth;")).toBe("CREATE SCHEMA IF NOT EXISTS auth;"); + }); + + it("leaves unrelated lines untouched", () => { + expect(makeSchemaCreationIdempotent("CREATE TABLE auth.users (id serial);")).toBe("CREATE TABLE auth.users (id serial);"); + expect(makeSchemaCreationIdempotent("")).toBe(""); + }); }); }); diff --git a/cli/test/modules/db/services/prerequisites.test.ts b/cli/test/modules/db/services/prerequisites.test.ts new file mode 100644 index 0000000..c50f3f7 --- /dev/null +++ b/cli/test/modules/db/services/prerequisites.test.ts @@ -0,0 +1,71 @@ +import {describe, it, expect, vi, beforeEach} from "vitest"; + +vi.mock("../../../../src/common/shell", () => ({ + commandExists: vi.fn(), +})); + +vi.mock("../../../../src/modules/db/services/pgschema", () => ({ + checkPgschemaInstalled: vi.fn(), +})); + +vi.mock("../../../../src/modules/db/services/dbmate", () => ({ + checkDbmateInstalled: vi.fn(), +})); + +import {commandExists} from "../../../../src/common/shell"; +import {checkPgschemaInstalled} from "../../../../src/modules/db/services/pgschema"; +import {checkDbmateInstalled} from "../../../../src/modules/db/services/dbmate"; +import {checkDbPrerequisites, checkPsqlInstalled} from "../../../../src/modules/db/services/prerequisites"; + +describe("prerequisites", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(checkPgschemaInstalled).mockResolvedValue(true); + vi.mocked(checkDbmateInstalled).mockResolvedValue(true); + vi.mocked(commandExists).mockResolvedValue(true); + }); + + describe("checkDbPrerequisites()", () => { + it("passes when pgschema and dbmate are installed and psql isn't required", async () => { + await expect(checkDbPrerequisites(false)).resolves.toBeUndefined(); + expect(commandExists).not.toHaveBeenCalled(); + }); + + it("throws when pgschema is missing", async () => { + vi.mocked(checkPgschemaInstalled).mockResolvedValue(false); + await expect(checkDbPrerequisites(false)).rejects.toThrow("pgschema binary not found"); + }); + + it("throws when dbmate is missing", async () => { + vi.mocked(checkDbmateInstalled).mockResolvedValue(false); + await expect(checkDbPrerequisites(false)).rejects.toThrow("dbmate binary not found"); + }); + + it("does not check psql when requirePsql is not passed", async () => { + await checkDbPrerequisites(false); + expect(commandExists).not.toHaveBeenCalledWith("psql"); + }); + + it("passes when requirePsql is true and psql is installed", async () => { + await expect(checkDbPrerequisites(false, {requirePsql: true})).resolves.toBeUndefined(); + expect(commandExists).toHaveBeenCalledWith("psql"); + }); + + it("throws a clear error when requirePsql is true and psql is missing", async () => { + vi.mocked(commandExists).mockResolvedValue(false); + await expect(checkDbPrerequisites(false, {requirePsql: true})).rejects.toThrow("psql binary not found"); + }); + }); + + describe("checkPsqlInstalled()", () => { + it("returns true when psql is on PATH", async () => { + vi.mocked(commandExists).mockResolvedValue(true); + expect(await checkPsqlInstalled()).toBe(true); + }); + + it("returns false when psql is not on PATH", async () => { + vi.mocked(commandExists).mockResolvedValue(false); + expect(await checkPsqlInstalled()).toBe(false); + }); + }); +}); diff --git a/docs/docs/modules/db/commands/deploy.md b/docs/docs/modules/db/commands/deploy.md index 0bbfcae..4e648db 100644 --- a/docs/docs/modules/db/commands/deploy.md +++ b/docs/docs/modules/db/commands/deploy.md @@ -45,12 +45,13 @@ postkit db deploy --remote staging -f 2. If an active session exists, removes it (with confirmation unless `-f`) 3. Tests the target database connection and detects its PostgreSQL major version 4. **If `localDbUrl` is empty**: Starts a temporary `postgres:{version}-alpine` container for the dry-run, version-matched to the target -5. Clones the target database to local for dry-run verification. When using a temp container, cloning runs via `docker exec` inside the container -6. Runs a full dry-run on the local clone: infra, dbmate migrate, seeds -7. If `--dry-run` is set, stops here and reports results without touching the target -8. Reports dry-run results and confirms deployment (unless `-f`) -9. Applies to target: infra, dbmate migrate, seeds -10. Drops the local clone database; stops and removes the temp container if one was used +5. **Applies `db/infra/` (roles, schemas, extensions) to the local/temp database** — before cloning, so RLS policies and grants in the target's dump (which reference custom roles) restore correctly instead of failing with `role does not exist` +6. Clones the target database (full data — intentional, so the dry run tests against realistic data) to local for dry-run verification. When using a temp container, cloning runs via `docker exec` inside the container +7. Runs a full dry-run on the local clone: infra (reapplied, idempotently), dbmate migrate, seeds +8. If `--dry-run` is set, stops here and reports results without touching the target +9. Reports dry-run results and confirms deployment (unless `-f`) +10. Applies to target: infra, dbmate migrate, seeds +11. Drops the local clone database; stops and removes the temp container if one was used If the dry run fails, deployment is aborted and no changes are made to the target database. diff --git a/docs/docs/modules/db/commands/start.md b/docs/docs/modules/db/commands/start.md index ee39ec6..965ad7c 100644 --- a/docs/docs/modules/db/commands/start.md +++ b/docs/docs/modules/db/commands/start.md @@ -33,13 +33,26 @@ postkit db start --remote staging ## What It Does -1. Checks prerequisites (pgschema, dbmate installed) +1. Checks prerequisites (pgschema, dbmate, **and `psql`** installed) 2. Resolves target remote (default or specified) 3. Tests connection to remote database and detects its PostgreSQL major version 4. Checks for pending committed migrations 5. **If `localDbUrl` is empty**: Checks Docker availability and starts a `postgres:{version}-alpine` container on a free port (15432–15532), version-matched to the remote -6. Clones remote database to local. When using an auto-container, cloning runs via `docker exec` inside the container (no host `pg_dump`/`psql` required) -7. Creates a session file (`.postkit/db/session.json`) to track state, including the `containerID` if a container was started +6. **Applies `db/infra/` (roles, schemas, extensions) to the local database** — before anything is cloned +7. Clones the remote database's **structure only** (no row data) to local. When using an auto-container, cloning runs via `docker exec` inside the container; otherwise it runs on the host, which is why `psql` must be installed +8. Creates a session file (`.postkit/db/session.json`) to track state, including the `containerID` if a container was started + +## Why Infra Is Applied Before Cloning + +The remote's dump includes `CREATE POLICY ... TO `, `GRANT ... TO `, and `ALTER DEFAULT PRIVILEGES ... TO ` statements for every custom role referenced by your RLS policies and grants. Those statements fail with `role "" does not exist` if replayed before the role exists — which is exactly the case on a brand-new local database or container. Applying `db/infra/` first means the roles your project already declares exist by the time the clone runs, so RLS policies and grants clone correctly. + +**Keep `db/infra/` limited to roles, schemas, and extensions — never tables.** If a table were defined there, it would already exist locally by the time the clone tries to create that same table, causing the clone to fail. Tables belong in `db/schema//tables/` instead. + +## Why the Clone Is Schema-Only + +`db start` clones structure only — no row data. Copying full production data (customer records, emails, tokens, etc.) into every disposable local dev container on every `db start` is unnecessary and a privacy/compliance risk. For local test data, use `db/schema//seeds/`, applied separately via `db plan`/`db apply`. + +`postkit db deploy`'s own dry-run clone is unaffected by this — it still clones full data, since it's specifically verifying real migrations against realistic data before they touch production. ## Auto Docker Container diff --git a/docs/docs/modules/db/overview.mdx b/docs/docs/modules/db/overview.mdx index 5fa7a58..975bbbe 100644 --- a/docs/docs/modules/db/overview.mdx +++ b/docs/docs/modules/db/overview.mdx @@ -27,8 +27,10 @@ The `db` module provides a **session-based database migration workflow** for saf │ $ postkit db start │ │ ┌────────────────────────────────────────────┐ │ │ │ 1. Detect remote PostgreSQL version │ │ -│ │ 2. Clone remote DB to local (or Docker) │ │ -│ │ 3. Start session (track state) │ │ +│ │ 2. Apply infra (roles, schemas, exts) │ │ +│ │ 3. Clone remote structure to local (or │ │ +│ │ Docker) — schema only, no row data │ │ +│ │ 3b. Start session (track state) │ │ │ └────────────────────┬───────────────────────┘ │ │ │ │ │ ▼ (edit db/schema// files) │ @@ -116,6 +118,8 @@ db/infra/ **Note:** `db/infra/` holds DB-level objects and is NOT inside a schema subdirectory. Files here are **excluded from pgschema** and applied separately. +**Keep this strictly to roles, schemas, and extensions — never tables, views, functions, or anything pgschema-managed.** `db start` and `db deploy` apply `db/infra/` to the local database *before* cloning (so RLS policies and grants that reference custom roles restore correctly). A table defined here would already exist locally by the time the clone tries to create it, causing the clone to fail outright. Tables belong in `db/schema//tables/`. + ### 2. Schema Objects (Processed by `postkit db plan`) The `plan` command uses pgschema to process these directories per schema: @@ -157,6 +161,7 @@ db/schema// - **pgschema** - Bundled with PostKit (no separate installation needed) - **dbmate** - Auto-installed via npm (no separate installation needed) +- **psql** (native PostgreSQL client) - Required for `postkit db start` (applying `db/infra/`, and cloning when `localDbUrl` is set directly rather than using an auto-container). Install via `brew install libpq` (macOS), `apt install postgresql-client` (Linux), or the PostgreSQL installer + PATH entry (Windows). - **Docker** _(optional)_ - Required only when `db.localDbUrl` is empty. PostKit starts a `postgres:{version}-alpine` container automatically, version-matched to your remote DB. ## Troubleshooting diff --git a/docs/docs/modules/db/troubleshooting.md b/docs/docs/modules/db/troubleshooting.md index 5641c1c..89c2112 100644 --- a/docs/docs/modules/db/troubleshooting.md +++ b/docs/docs/modules/db/troubleshooting.md @@ -14,6 +14,19 @@ sidebar_position: 100 **Solution:** dbmate should be installed via npm. Run `npm install` in the CLI directory, or install manually (`brew install dbmate`) and set `db.dbmateBin` in config. +### `psql binary not found` + +**Solution:** Required by `postkit db start`. Install PostgreSQL client tools: +- macOS: `brew install libpq && brew link --force libpq` +- Linux: `apt install postgresql-client` (or your distro's equivalent) +- Windows: `winget install PostgreSQL.PostgreSQL`, then add its `bin/` folder to `PATH` + +Open a new terminal afterward and verify with `psql --version`. + +### `role "" does not exist` during clone + +**Solution:** A custom role referenced by an RLS policy or grant isn't defined in `db/infra/`. Add it there — `db start` and `db deploy` apply `db/infra/` to the local database before cloning specifically so these roles exist in time. Make sure `db/infra/` only contains roles/schemas/extensions, not tables (see [Database Module overview](/docs/modules/db/overview)). + ### `Failed to connect to remote database` **Solution:** Check the remote URL in `postkit db remote list` diff --git a/docs/docs/reference/project-structure.md b/docs/docs/reference/project-structure.md index d591764..ee2d586 100644 --- a/docs/docs/reference/project-structure.md +++ b/docs/docs/reference/project-structure.md @@ -58,6 +58,8 @@ The `db/` directory is organized into infrastructure and per-schema sections: |-----------|-------------|--------------| | `db/infra/` | DB-level: roles, extensions, CREATE SCHEMA | Applied separately (excluded from pgschema) | +> ⚠️ **Only roles, schemas, and extensions belong in `db/infra/` — never tables, views, functions, or other pgschema-managed objects.** `postkit db start`/`db deploy` apply `db/infra/` to the local database *before* cloning, so RLS policies and grants that reference custom roles restore correctly. A table defined in `db/infra/` would already exist locally by the time the clone tries to create it, causing the clone to fail. + ### Schema Objects (Processed by `postkit db plan`) Each schema has its own subdirectory under `db/schema//`: From dde8574f5883ceacf0d12e75c0a4fb909264231e Mon Sep 17 00:00:00 2001 From: supunappri99 Date: Mon, 20 Jul 2026 20:20:16 +0530 Subject: [PATCH 2/4] feat(db): implement line sanitization for database cloning and update tests --- cli/src/modules/db/services/container.ts | 4 +-- cli/src/modules/db/services/database.ts | 24 ++++++++++++++- .../case-5-custom-schema-full-flow.test.ts | 18 ++++++++++-- .../modules/db/services/container.test.ts | 2 +- cli/test/modules/db/services/database.test.ts | 29 +++++++++++++++++-- 5 files changed, 68 insertions(+), 9 deletions(-) diff --git a/cli/src/modules/db/services/container.ts b/cli/src/modules/db/services/container.ts index 1b09280..b773831 100644 --- a/cli/src/modules/db/services/container.ts +++ b/cli/src/modules/db/services/container.ts @@ -1,7 +1,7 @@ import net from "net"; import type {Ora} from "ora"; import {runCommand, runSpawnCommand, commandExists} from "../../../common/shell"; -import {testConnection, parseConnectionUrl, getRemotePgMajorVersion, makeSchemaCreationIdempotent} from "./database"; +import {testConnection, parseConnectionUrl, getRemotePgMajorVersion, sanitizeCloneLine} from "./database"; import {runPipedCommands} from "../../../common/shell"; import {PostkitError} from "../../../common/errors"; @@ -157,7 +157,7 @@ export async function cloneDatabaseViaContainer( "-v", "ON_ERROR_STOP=1", ], }, - makeSchemaCreationIdempotent, + sanitizeCloneLine, ); if (result.exitCode !== 0) { diff --git a/cli/src/modules/db/services/database.ts b/cli/src/modules/db/services/database.ts index 83e6a8b..28a7016 100644 --- a/cli/src/modules/db/services/database.ts +++ b/cli/src/modules/db/services/database.ts @@ -91,6 +91,28 @@ export function makeSchemaCreationIdempotent(line: string): string { return line.replace(/^CREATE SCHEMA (?!IF NOT EXISTS)(.+);$/, "CREATE SCHEMA IF NOT EXISTS $1;"); } +/** + * pg_dump's preamble includes `SET = ...;` lines to configure the + * restore session (disable timeouts, etc). Most of these GUCs are ancient and + * universally supported, but newer pg_dump versions add newer ones (e.g. + * `transaction_timeout`, added in PostgreSQL 17) unconditionally, regardless of + * the actual restore target's version. If the host's pg_dump/psql are newer + * than the target server, the target rejects the unrecognized GUC with + * "unrecognized configuration parameter" — fatal now that ON_ERROR_STOP is on. + * These are session-tuning conveniences, not structural/data statements, so + * it's always safe to comment one out if the target doesn't support it. + */ +const PREAMBLE_ONLY_SETTINGS = ["transaction_timeout"]; + +export function neutralizeUnsupportedPreambleSettings(line: string): string { + const pattern = new RegExp(`^SET (${PREAMBLE_ONLY_SETTINGS.join("|")})\\s*=.*;$`); + return pattern.test(line) ? `-- ${line}` : line; +} + +export function sanitizeCloneLine(line: string): string { + return makeSchemaCreationIdempotent(neutralizeUnsupportedPreambleSettings(line)); +} + export async function cloneDatabase( sourceUrl: string, targetUrl: string, @@ -128,7 +150,7 @@ export async function cloneDatabase( ], env: {PGPASSWORD: dst.password}, }, - makeSchemaCreationIdempotent, + sanitizeCloneLine, ); if (result.exitCode !== 0) { diff --git a/cli/test/e2e/workflows/case-5-custom-schema-full-flow.test.ts b/cli/test/e2e/workflows/case-5-custom-schema-full-flow.test.ts index c506b62..a5b2278 100644 --- a/cli/test/e2e/workflows/case-5-custom-schema-full-flow.test.ts +++ b/cli/test/e2e/workflows/case-5-custom-schema-full-flow.test.ts @@ -338,8 +338,22 @@ CREATE TABLE tag ( ); }); - it("verifies seed data is intact in local DB", async () => { - await verifySeedsInSchema(localDb.url, CUSTOM_SCHEMA); + it("clones structure only — local DB has zero rows, not remote's row data", async () => { + // `db start` clones schema-only (no row data) by design, so ad-hoc rows + // inserted directly into remote (not via db/schema//seeds/) never + // reach local. Remote itself is untouched and still has this data — + // see "verifies seed data is intact in remote DB" below. + const categories = await queryDatabase( + localDb.url, + `SELECT name FROM ${CUSTOM_SCHEMA}.category`, + ); + expect(categories).toHaveLength(0); + + const products = await queryDatabase( + localDb.url, + `SELECT name FROM ${CUSTOM_SCHEMA}.product`, + ); + expect(products).toHaveLength(0); }); // ── Phase 5: Commit ─────────────────────────────────────────────────── diff --git a/cli/test/modules/db/services/container.test.ts b/cli/test/modules/db/services/container.test.ts index 47e010d..ef26e98 100644 --- a/cli/test/modules/db/services/container.test.ts +++ b/cli/test/modules/db/services/container.test.ts @@ -20,7 +20,7 @@ vi.mock("../../../../src/modules/db/services/database", () => ({ password: decodeURIComponent(parsed.password), }; }), - makeSchemaCreationIdempotent: vi.fn((line: string) => line), + sanitizeCloneLine: vi.fn((line: string) => line), })); vi.mock("../../../../src/common/errors", () => ({ diff --git a/cli/test/modules/db/services/database.test.ts b/cli/test/modules/db/services/database.test.ts index df07714..e0d16e4 100644 --- a/cli/test/modules/db/services/database.test.ts +++ b/cli/test/modules/db/services/database.test.ts @@ -22,7 +22,7 @@ vi.mock("../../../../src/common/shell", () => ({ // Get references to the mocked functions via pg import import pg from "pg"; import {runPipedCommands} from "../../../../src/common/shell"; -import {parseConnectionUrl, testConnection, createDatabase, dropDatabase, cloneDatabase, getRemotePgMajorVersion, makeSchemaCreationIdempotent} from "../../../../src/modules/db/services/database"; +import {parseConnectionUrl, testConnection, createDatabase, dropDatabase, cloneDatabase, getRemotePgMajorVersion, makeSchemaCreationIdempotent, neutralizeUnsupportedPreambleSettings, sanitizeCloneLine} from "../../../../src/modules/db/services/database"; // Access mock methods from the Client prototype const getMockClient = () => { @@ -156,11 +156,11 @@ describe("database", () => { expect(consumer.args).toContain("ON_ERROR_STOP=1"); }); - it("passes makeSchemaCreationIdempotent as the transformLine argument", async () => { + it("passes sanitizeCloneLine as the transformLine argument", async () => { vi.mocked(runPipedCommands).mockResolvedValue({stdout: "", stderr: "", exitCode: 0}); await cloneDatabase("postgres://src:pass@src-host:5432/srcdb", "postgres://dst:pass@dst-host:5432/dstdb"); const call = vi.mocked(runPipedCommands).mock.calls[0]!; - expect(call[2]).toBe(makeSchemaCreationIdempotent); + expect(call[2]).toBe(sanitizeCloneLine); }); it("does not pass --schema-only by default", async () => { @@ -192,4 +192,27 @@ describe("database", () => { expect(makeSchemaCreationIdempotent("")).toBe(""); }); }); + + describe("neutralizeUnsupportedPreambleSettings()", () => { + it("comments out SET transaction_timeout (PG17-only GUC, unsupported by older targets)", () => { + expect(neutralizeUnsupportedPreambleSettings("SET transaction_timeout = 0;")).toBe("-- SET transaction_timeout = 0;"); + }); + + it("leaves universally-supported preamble SET statements untouched", () => { + expect(neutralizeUnsupportedPreambleSettings("SET statement_timeout = 0;")).toBe("SET statement_timeout = 0;"); + expect(neutralizeUnsupportedPreambleSettings("SET client_encoding = 'UTF8';")).toBe("SET client_encoding = 'UTF8';"); + }); + + it("leaves unrelated lines untouched", () => { + expect(neutralizeUnsupportedPreambleSettings("CREATE TABLE t (id serial);")).toBe("CREATE TABLE t (id serial);"); + }); + }); + + describe("sanitizeCloneLine()", () => { + it("applies both schema-idempotency and preamble-neutralization rewrites", () => { + expect(sanitizeCloneLine("CREATE SCHEMA auth;")).toBe("CREATE SCHEMA IF NOT EXISTS auth;"); + expect(sanitizeCloneLine("SET transaction_timeout = 0;")).toBe("-- SET transaction_timeout = 0;"); + expect(sanitizeCloneLine("CREATE TABLE t (id serial);")).toBe("CREATE TABLE t (id serial);"); + }); + }); }); From 820c0ed64e3d4ba15053b5fc24a390ac3a9f78a7 Mon Sep 17 00:00:00 2001 From: supunappri99 Date: Mon, 20 Jul 2026 20:21:47 +0530 Subject: [PATCH 3/4] fix: update version to 1.3.1 in package.json --- cli/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/package.json b/cli/package.json index 16e5aa7..cc60552 100644 --- a/cli/package.json +++ b/cli/package.json @@ -1,6 +1,6 @@ { "name": "@appritech/postkit", - "version": "1.3.0", + "version": "1.3.1", "description": "PostKit - Developer toolkit for database management and more", "type": "module", "main": "dist/index.js", From b2eb272358f3efba01aed97c7f0bf0a9f8003f5c Mon Sep 17 00:00:00 2001 From: supunappri99 Date: Mon, 20 Jul 2026 20:39:17 +0530 Subject: [PATCH 4/4] test: remove unnecessary variable assignment in startSessionContainer test --- cli/test/modules/db/services/container.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/test/modules/db/services/container.test.ts b/cli/test/modules/db/services/container.test.ts index ef26e98..443ea5e 100644 --- a/cli/test/modules/db/services/container.test.ts +++ b/cli/test/modules/db/services/container.test.ts @@ -101,7 +101,7 @@ describe("container", () => { }); vi.mocked(testConnection).mockResolvedValue(true); - const info = await startSessionContainer(16); + await startSessionContainer(16); const spawnArgs = vi.mocked(runSpawnCommand).mock.calls[0]![0]; expect(spawnArgs).toContain("postgres:16-alpine");