fix(prepare-db): address REV review feedback

- Fix ALTER USER filtering to preserve comments and handle edge cases
- Add provider validation with warning for unknown providers
- Fix help text to only mention implemented providers (supabase)
- Add comprehensive tests:
  - verifyInitSetup skips search_path check for supabase
  - buildInitPlan preserves comments when filtering ALTER USER
  - validateProvider returns correct results
  - CLI --provider supabase skips role step
  - CLI warns about unknown provider

Author: claude

cli/bin/postgres-ai.ts

@@ -12,7 +12,7 @@ import { Client } from "pg";
 import { startMcpServer } from "../lib/mcp-server";
 import { fetchIssues, fetchIssueComments, createIssueComment, fetchIssue, createIssue, updateIssue, updateIssueComment } from "../lib/issues";
 import { resolveBaseUrls } from "../lib/util";
-import { applyInitPlan, buildInitPlan, connectWithSslFallback, DEFAULT_MONITORING_USER, redactPasswordsInSql, resolveAdminConnection, resolveMonitoringPassword, verifyInitSetup } from "../lib/init";
+import { applyInitPlan, buildInitPlan, connectWithSslFallback, DEFAULT_MONITORING_USER, KNOWN_PROVIDERS, redactPasswordsInSql, resolveAdminConnection, resolveMonitoringPassword, validateProvider, verifyInitSetup } from "../lib/init";
 import * as pkce from "../lib/pkce";
 import * as authServer from "../lib/auth-server";
 import { maskSecret } from "../lib/util";
@@ -564,7 +564,7 @@ program
   .option("--monitoring-user <name>", "Monitoring role name to create/update", DEFAULT_MONITORING_USER)
   .option("--password <password>", "Monitoring role password (overrides PGAI_MON_PASSWORD)")
   .option("--skip-optional-permissions", "Skip optional permissions (RDS/self-managed extras)", false)
-  .option("--provider <provider>", "Database provider (e.g., supabase, rds, aurora). Affects which steps are executed.")
+  .option("--provider <provider>", "Database provider (e.g., supabase). Affects which steps are executed.")
   .option("--verify", "Verify that monitoring role/permissions are in place (no changes)", false)
   .option("--reset-password", "Reset monitoring role password only (no other changes)", false)
   .option("--print-sql", "Print SQL plan and exit (no changes applied)", false)
@@ -644,6 +644,12 @@ program
     const shouldPrintSql = !!opts.printSql;
     const redactPasswords = (sql: string): string => redactPasswordsInSql(sql);
 
+    // Validate provider and warn if unknown
+    const providerWarning = validateProvider(opts.provider);
+    if (providerWarning) {
+      console.warn(`⚠ ${providerWarning}`);
+    }
+
     // Offline mode: allow printing SQL without providing/using an admin connection.
     // Useful for audits/reviews; caller can provide -d/PGDATABASE.
     if (!conn && !opts.dbUrl && !opts.host && !opts.port && !opts.username && !opts.adminPassword) {

cli/lib/init.ts

@@ -15,6 +15,9 @@ export const DEFAULT_MONITORING_USER = "postgres_ai_mon";
  */
 export type DbProvider = string;
 
+/** Known providers with special handling. Unknown providers are treated as self-managed. */
+export const KNOWN_PROVIDERS = ["self-managed", "supabase"] as const;
+
 /** Providers where we skip role creation (users managed externally). */
 const SKIP_ROLE_CREATION_PROVIDERS = ["supabase"];
 
@@ -24,6 +27,12 @@ const SKIP_ALTER_USER_PROVIDERS = ["supabase"];
 /** Providers where we skip search_path verification (not set via ALTER USER). */
 const SKIP_SEARCH_PATH_CHECK_PROVIDERS = ["supabase"];
 
+/** Check if a provider is known and return a warning message if not. */
+export function validateProvider(provider: string | undefined): string | null {
+  if (!provider || KNOWN_PROVIDERS.includes(provider as any)) return null;
+  return `Unknown provider "${provider}". Known providers: ${KNOWN_PROVIDERS.join(", ")}. Treating as self-managed.`;
+}
+
 export type PgClientConfig = {
   connectionString?: string;
   host?: string;
@@ -525,7 +534,13 @@ end $$;`;
   if (SKIP_ALTER_USER_PROVIDERS.includes(provider)) {
     permissionsSql = permissionsSql
       .split("\n")
-      .filter((line) => !line.toLowerCase().includes("alter user"))
+      .filter((line) => {
+        const trimmed = line.trim();
+        // Keep comments and empty lines
+        if (trimmed.startsWith("--") || trimmed === "") return true;
+        // Filter out ALTER USER statements (case-insensitive, flexible whitespace)
+        return !/^\s*alter\s+user\s+/i.test(line);
+      })
       .join("\n");
   }
 

cli/test/init.test.ts

@@ -326,6 +326,91 @@ describe("init module", () => {
     expect(calls[calls.length - 1].toLowerCase()).toBe("rollback;");
   });
 
+  test("verifyInitSetup skips search_path check for supabase provider", async () => {
+    const calls: string[] = [];
+    const client = {
+      query: async (sql: string, params?: any) => {
+        calls.push(String(sql));
+
+        if (String(sql).toLowerCase().startsWith("begin isolation level repeatable read")) {
+          return { rowCount: 1, rows: [] };
+        }
+        if (String(sql).toLowerCase() === "rollback;") {
+          return { rowCount: 1, rows: [] };
+        }
+        // Return empty rolconfig - would fail without provider=supabase
+        if (String(sql).includes("select rolconfig")) {
+          return { rowCount: 1, rows: [{ rolconfig: null }] };
+        }
+        if (String(sql).includes("from pg_catalog.pg_roles")) {
+          return { rowCount: 1, rows: [{ rolname: DEFAULT_MONITORING_USER }] };
+        }
+        if (String(sql).includes("has_database_privilege")) {
+          return { rowCount: 1, rows: [{ ok: true }] };
+        }
+        if (String(sql).includes("pg_has_role")) {
+          return { rowCount: 1, rows: [{ ok: true }] };
+        }
+        if (String(sql).includes("has_table_privilege")) {
+          return { rowCount: 1, rows: [{ ok: true }] };
+        }
+        if (String(sql).includes("to_regclass")) {
+          return { rowCount: 1, rows: [{ ok: true }] };
+        }
+        if (String(sql).includes("has_function_privilege")) {
+          return { rowCount: 1, rows: [{ ok: true }] };
+        }
+        if (String(sql).includes("has_schema_privilege")) {
+          return { rowCount: 1, rows: [{ ok: true }] };
+        }
+
+        throw new Error(`unexpected sql: ${sql} params=${JSON.stringify(params)}`);
+      },
+    };
+
+    // With provider=supabase, should pass even without search_path
+    const r = await init.verifyInitSetup({
+      client: client as any,
+      database: "mydb",
+      monitoringUser: DEFAULT_MONITORING_USER,
+      includeOptionalPermissions: false,
+      provider: "supabase",
+    });
+    expect(r.ok).toBe(true);
+    expect(r.missingRequired.length).toBe(0);
+    // Should not have queried for rolconfig since we skip search_path check
+    expect(calls.some((c) => c.includes("select rolconfig"))).toBe(false);
+  });
+
+  test("buildInitPlan preserves comments when filtering ALTER USER", async () => {
+    const plan = await init.buildInitPlan({
+      database: "mydb",
+      monitoringUser: DEFAULT_MONITORING_USER,
+      monitoringPassword: "pw",
+      includeOptionalPermissions: false,
+      provider: "supabase",
+    });
+    const permStep = plan.steps.find((s) => s.name === "02.permissions");
+    expect(permStep).toBeDefined();
+    // Should have removed ALTER USER but kept comments
+    expect(permStep!.sql.toLowerCase()).not.toMatch(/^\s*alter\s+user/m);
+    // Should still have comment lines
+    expect(permStep!.sql).toMatch(/^--/m);
+  });
+
+  test("validateProvider returns null for known providers", () => {
+    expect(init.validateProvider(undefined)).toBe(null);
+    expect(init.validateProvider("self-managed")).toBe(null);
+    expect(init.validateProvider("supabase")).toBe(null);
+  });
+
+  test("validateProvider returns warning for unknown providers", () => {
+    const warning = init.validateProvider("unknown-provider");
+    expect(warning).not.toBe(null);
+    expect(warning).toMatch(/Unknown provider/);
+    expect(warning).toMatch(/unknown-provider/);
+  });
+
   test("redactPasswordsInSql redacts password literals with embedded quotes", async () => {
     const plan = await init.buildInitPlan({
       database: "mydb",
@@ -355,6 +440,23 @@ describe("CLI commands", () => {
     expect(r.stdout).toMatch(new RegExp(`grant connect on database "mydb" to "${DEFAULT_MONITORING_USER}"`, "i"));
   });
 
+  test("cli: prepare-db --print-sql with --provider supabase skips role step", () => {
+    const r = runCli(["prepare-db", "--print-sql", "-d", "mydb", "--password", "monpw", "--provider", "supabase"]);
+    expect(r.status).toBe(0);
+    expect(r.stdout).toMatch(/provider: supabase/);
+    // Should not have 01.role step
+    expect(r.stdout).not.toMatch(/-- 01\.role/);
+    // Should have 02.permissions step
+    expect(r.stdout).toMatch(/-- 02\.permissions/);
+  });
+
+  test("cli: prepare-db warns about unknown provider", () => {
+    const r = runCli(["prepare-db", "--print-sql", "-d", "mydb", "--password", "monpw", "--provider", "unknown-cloud"]);
+    expect(r.status).toBe(0);
+    // Should warn about unknown provider
+    expect(r.stderr).toMatch(/Unknown provider.*unknown-cloud/);
+  });
+
   test("pgai wrapper forwards to postgresai CLI", () => {
     const r = runPgai(["--help"]);
     expect(r.status).toBe(0);