feat(cli): generate types natively with postgrest-typegen - #6404
Open
avallete wants to merge 16 commits into
Open
feat(cli): generate types natively with postgrest-typegen#6404avallete wants to merge 16 commits into
avallete wants to merge 16 commits into
Conversation
Replace the pg-meta Docker container behind `gen types` with the new @supabase/postgrest-typegen package, running introspection and language generation in-process over a direct Postgres connection. - New LegacyGenTypesGenerator service seam: the production layer acquires a scoped pg.Pool via legacyAcquirePgPool (full driver parity: TLS mode, DoH resolver, connect-error mapping), feeds it to introspect(), sorts with sortGeneratorMetadata, and renders typescript/go/python/swift. - `--local` connects to the host-mapped db port instead of spawning the pg-meta image inside the stack network; the `container inspect` stack-running check and rest-version v9 forcing are unchanged. The obsolete `.temp/pgmeta-version` image override is removed. - `--db-url` now resolves through the shared LegacyDbConfigResolver (libpq keywords, options=reference pooler tenants, sslmode, PG* env fallbacks), matching every other --db-url command. - Project-ref non-TypeScript paths and the preview-branch fallback keep their Management API flow and IPv4 pooler retry, now classifying the native connect error instead of container stderr. - `--query-timeout` maps to statement_timeout plus the connect timeout, mirroring the PG_QUERY_TIMEOUT_SECS/PG_CONN_TIMEOUT_SECS envs pg-meta received; `--postgrest-v9-compat` disables one-to-one detection in the TypeScript generator; output keeps the trailing newline console.log added in pg-meta. - The pg-meta SSL probe, CA bundle templates, and --network-id container override are gone with the container; the linked TypeScript path still uses the Management API unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RpnbmgTzwdTMkWYF7uFR1V
Validated the native typegen end-to-end against a real Postgres 16 and against postgres-meta 0.98.0 on the same schema, which surfaced two issues: - The driver requires TLS for remote-looking targets, so `gen types --db-url` against a plain-TCP server (common when self-hosting) failed where the pg-meta path adapted via its SSL probe. Restore that adaptivity in the generator layer: when the DSN carries no explicit sslmode, the shared SSLRequest probe decides whether to connect with sslmode=disable; probe failures keep the TLS default so the real connect error still surfaces. - prettier 3.5.3 (postgrest-typegen's pin) trips a Bun bundler renaming bug under `bun build --compile`, breaking TypeScript generation in the compiled binary only. Override it to the repo's prettier 3.9.6, which bundles cleanly; pg-meta itself floated ^3.3.3, so there is no output-parity concern. Parity results against postgres-meta on the same database: TypeScript and Swift byte-identical; Go and Python identical content with canonical sorted entity ordering (sortGeneratorMetadata) instead of pg-meta's environment-dependent row order. Documented in SIDE_EFFECTS.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RpnbmgTzwdTMkWYF7uFR1V
develop's SUPABASE_USE_SLIM_IMAGES change (de133cf) touched gen types only through resolvePgmetaImage and its tests, all of which this branch deletes with the pg-meta container path, so the branch side wins in all four files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RpnbmgTzwdTMkWYF7uFR1V
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d8c60d5cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
0.2.0 drops prettier for oxfmt, so the prettier bundling override goes away. oxfmt's ESM dist resolves its napi binding through createRequire(import.meta.url) and lazily imports optional prettier plugins — neither survives `bun build --compile` — so the CLI: - embeds the platform binding statically (the @parcel/watcher pattern: one @oxfmt/binding-* devDependency per shipped target, dispatched on platform/arch/SUPABASE_LIBC in types.oxfmt.ts) and injects it through the generator's new `format` option, verified byte-equivalent to the package's own default formatter; - marks the never-installed optional prettier plugins external in both the dev and release build scripts (shared bundle-externals.ts). Revalidated against a real Postgres 16 from the compiled binary: Go/Swift/Python output is byte-identical to the 0.1.0 integration; TypeScript content is identical with oxfmt's union-wrapping style (three lines differ from the prettier-era output), and source-run vs compiled-binary output is identical. SIDE_EFFECTS.md parity note updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RpnbmgTzwdTMkWYF7uFR1V
avallete
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebb449be92
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keep native typegen SIDE_EFFECTS; drop the slim-image pg-meta notes develop added, since this command no longer runs that container. Co-authored-by: Cursor <cursoragent@cursor.com>
Pin the embedded Supabase CA when the SSL probe reports TLS, treat --query-timeout 0 as disabled rather than an immediate connect timeout, let the flag override a DSN statement_timeout, bound introspect() on the client, and classify generator/formatter failures as internal instead of database findings. Co-authored-by: Cursor <cursoragent@cursor.com>
A probe error still leaves sslmode unset so the driver default and IPv6 pooler classification stay intact. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the duplicate TLS-probe comment, unused network-id test wiring, and a leftover localNetworkId assertion. Clarify that a TLS probe replaces sslrootcert when sslmode is omitted. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39cb1577fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The glibc and local compile paths already pass oxfmtExternalArgs; musl release binaries were still resolving those never-installed prettier plugins and would fail bun build --compile. Co-authored-by: Cursor <cursoragent@cursor.com>
pull Bot
pushed a commit
to oogalieboogalie/cli
that referenced
this pull request
Aug 31, 2026
## Summary Bumps the pinned pg-meta image from `v0.98.0` to `v0.99.0` in the shared service-image manifest (`apps/cli-go/pkg/config/templates/Dockerfile`, imported by the TypeScript CLI as its image source). postgres-meta v0.99.0 replaces the embedded type-generation templates with the shared [`@supabase/postgrest-typegen`](https://github.com/supabase/sdk/tree/main/packages/postgrest-typegen) package (supabase/postgres-meta#1084). This is part of a coordinated rollout with the hosted path (supabase/platform#37764) so `gen types` produces the same output locally and via `--project-id`. ## Relationship to supabase#6404 supabase#6404 makes `gen types` run postgrest-typegen in-process, removing the pg-meta container from that command entirely. This pin still matters independently of it: the same manifest entry provides the `pgmeta` service that `supabase start` runs for Studio's local API, and it covers `gen types` for any release cut before supabase#6404 lands. The two do not conflict (different files), and output is consistent either way since v0.99.0 serves the same generator package that supabase#6404 embeds. ## What changes for users Generated TypeScript output changes in two deliberate ways: deterministic metadata ordering (a one-time reordering diff when regenerating existing types) and oxfmt formatting instead of prettier (style-only). Content is otherwise unchanged. ## Validation - `go build ./...` passes in both modules. - The pre-existing `gen types` e2e tests pull this image tag directly, so CI exercises the new release; the image is published on Docker Hub and ECR Public.
Contributor
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@8ce5cfa7d6e81bcaef44893236b45b0b18de8bc5Preview package for commit |
This was referenced Sep 1, 2026
feat: add supabase_typegen package generating typed table definitions
supabase/supabase-flutter#1635
Merged
Member
Author
|
/ai-review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Merged 13 reviewer reports into 12 deduplicated findings. Nine are confirmed, including the IPv4 fallback regression and aggregate introspection timeout; three are refuted by the current implementation and trusted repository conventions.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | apps/cli/src/legacy/commands/gen/types/types.handler.ts:389 |
error-handling |
claude | IPv4 pooler fallback misses common Node IPv6-connectivity failures because the structured driver cause is discarded before classification. |
| 🟠 MAJOR | apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:91 |
timeout-semantics |
claude+codex | --query-timeout now bounds the complete multi-query introspection operation in addition to each SQL statement, causing cumulatively slow schemas to fail at the default 15 seconds. |
| 🟠 MAJOR | apps/cli/src/legacy/commands/gen/types/types.handler.ts:207 |
behavioral-compatibility |
claude | --network-id and SUPABASE_NETWORK_ID are silently ignored, breaking generation for database hostnames reachable only from the selected Docker network. |
| 🟡 MINOR | apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:30 |
error-classification |
claude | Operational temporary-directory or disk failures while writing the CA bundle are misclassified as internal CLI bugs. |
| 🟡 MINOR | apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:38 |
test-coverage |
claude | The production generator lacks focused integration coverage for its remote probe-failure, CA-pinning, and introspection-timeout branches. |
| 🟡 MINOR | apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:78 |
cancellation |
codex | Interrupting or timing out introspection does not cancel its currently running PostgreSQL query. |
| 🟡 MINOR | apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:106 |
error-handling |
codex | An exception from the foreign metadata sorter escapes as an Effect defect rather than the declared generation error. |
| ⚪ NIT | apps/cli/src/legacy/commands/gen/types/types.integration.test.ts:1937 |
test-isolation |
claude | The test mutates SUPABASE_DB_PASSWORD before constructing its Effect but restores it only inside that Effect, allowing setup failures to leak the variable. |
| ⚪ NIT | apps/cli/src/legacy/commands/gen/types/types.handler.ts:395 |
output-formatting |
codex | The promised single trailing newline is not enforced; generator output already ending in a newline receives another blank line. |
Refuted findings (kept for transparency, not posted as review comments)
apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:57(behavioral-compatibility): Removing theSUPABASE_CA_SKIP_VERIFY=truewarning is an unsanctioned behavior regression.
Refuted: legacy-pgdelta-ssl-probe.layer.ts:68-74 documents that this wire-level probe never validates certificates, and the native connection now pins the CA at types.generator.layer.ts:62-63. The removed variable therefore no longer controls any operation being performed. Trusted apps/cli/CLAUDE.md also directs intentional behavior changes to update tests and SIDE_EFFECTS rather than add new Go-divergence records, which this PR does.apps/cli/src/legacy/commands/gen/types/types.oxfmt.ts:90(maintainability): The oxfmt binding and formatter settings can silently drift from postgrest-typegen, and the native binding is repeatedly loaded.
Refuted: apps/cli/package.json pins postgrest-typegen exactly to 0.2.0 and every binding exactly to 0.65.0; pnpm-lock.yaml confirms that typegen resolves oxfmt 0.65.0. A transitive bump cannot occur without changing the direct typegen pin and lockfile. Repeated require calls also use the module cache rather than reloading the addon.apps/cli/src/legacy/commands/gen/types/types.handler.ts:365(behavioral-compatibility): The changed--localconnection diagnostic requires a separate Go-parity divergence note.
Refuted: The diagnostic accurately describes the new connection target, types.integration.test.ts:1870 asserts it, and SIDE_EFFECTS.md:60-61 documents the host-mapped connection. Trusted apps/cli/CLAUDE.md says the retired Go implementation is not authoritative and new divergence records must not be added.
Stats
Claude findings: 9 · Codex findings: 4 · Confirmed: 9 · Refuted: 3 · Uncertain: 0
Models: claude-opus-5 + gpt-5.6-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
spydon
added a commit
to supabase/supabase-flutter
that referenced
this pull request
Sep 1, 2026
…#1635) ## What kind of change does this PR introduce? Feature (draft, layer 2 of the typed table access work, stacked on #1634). Adds a new `supabase_typegen` package: a standalone code generator that turns a database schema into the typed table definitions introduced in #1634, so users get the fully typed surface without writing any of it by hand. Linear: SDK-1362 ## What is the new behavior? ```sh supabase gen types --lang dart --local > lib/supabase_schema.g.dart ``` The CLI runs postgrest-typegen's introspection in-process against the database and hands the language-neutral `GeneratorMetadata` document, the intermediate representation of [`@supabase/postgrest-typegen`](https://github.com/supabase/sdk/tree/main/packages/postgrest-typegen) that its TypeScript, Go, Swift, and Python generators also consume, to this tool over stdin. The tool emits one Dart file containing, per table: - a zero-cost row extension type over the decoded JSON map with typed getters (`DateTime` parsing, `double`/`num` coercion, `List` casts, Postgres enum mapping), - `Insert` and `Update` value extension types that implement `Map<String, dynamic>`, with required parameters derived from `NOT NULL`-without-default columns and null-aware omission for everything else; explicit SQL NULL writes go through generated `set…ToNull` copy methods that only exist for nullable, writable columns, - a `PostgrestTable` definition plus `TableColumn` tokens for compile-time checked filters, - Dart enums for Postgres enums with wire-name mapping (`toString` returns the wire name so enum values work directly in filters). See `packages/supabase_typegen/test/goldens/supabase_schema.dart` for what the output looks like for the fixture schema. Design choices worth reviewing: - **Introspection source**: the `GeneratorMetadata` contract of `@supabase/postgrest-typegen` (version 1, as shipped in the released 0.2.0 and embedded in postgres-meta v0.99.0). The CLI produces the document by running the package's `introspect()` in-process against the local database; there is no postgres-meta dependency. The document comes straight from the database catalog, so the output is exact where API-derived descriptions are lossy: `NOT NULL` columns with a database default read as non-nullable but stay optional on insert, identity columns are recognized, and `GENERATED ALWAYS` columns appear in the row type but are excluded from the insert and update types. Structural validation rejects non-matching documents. - **Relation and column writability**: tables and foreign tables emit the full surface; views gate `Insert` and `Update` independently on `is_insert_enabled` and `is_update_enabled` (falling back to `is_updatable` for documents predating the flags), so a view writable only through an INSTEAD OF INSERT trigger gets exactly an insert type; materialized views are read-only; non-updatable view columns read but are excluded from writes. This mirrors the TypeScript generator's semantics. - **Exact enum resolution**: a column's enum type resolves by its `type_schema` plus type name, so same-named enums in different schemas cannot be confused. - **Canonical ordering**: columns are emitted in the order `sortGeneratorMetadata` produces (name order within a table), matching every other postgrest-typegen generator and keeping output insensitive to column declaration order. - **Naming**: `books` emits `BooksRow`/`BooksInsert`/`BooksUpdate` plus a `Books` namespace class (no English singularization, so names stay predictable). Identifiers are sanitized against Dart reserved words and `Map` member names with a `$` suffix, and collisions are deduplicated. - **Lint-clean output**: the emitted code (checked in as a golden) passes `supabase_lints` and DCM with zero issues, including the strict extension type rules. ## Additional context - The metadata fixture is regenerated from a real introspection and stays reproducible: `test/fixtures/seed.sql` applied to a disposable Postgres container, introspected with the released `@supabase/postgrest-typegen@0.2.0` via `tool/regenerate_fixture.ts`. - CLI exposure as `supabase gen types --lang dart` is a small follow-up on supabase/cli#6404, which already runs postgrest-typegen's `introspect()` in-process: the CLI serializes the sorted document and pipes it to `dart run supabase_typegen` over stdin, the tool's only input channel. No pg-meta container, no metadata file on disk, and no user-facing json output language are involved (the earlier container-based supabase/cli#6230 is closed as superseded). - The package is excluded from the SDK compliance scan via `.sdk-parse-ignore` since it is a development-time tool, not SDK client surface; the symbol, drift and schema checks pass locally against the base branch. - `supabase_typegen` is added to the CI dart test matrix; tests are fully mocked/fixture-based (introspection unit tests over the checked-in metadata fixture, a whitespace-insensitive golden comparison with a `tool/regenerate_goldens.dart` refresh script, and behavior tests that run the generated golden code against a mock HTTP client to verify wire formats end to end). - `publish_to: none` until the API settles. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a Dart generator for strongly typed Supabase tables, rows, inserts, updates, columns, relationships, views, and Postgres enums. * Added the `supabase_typegen` command-line tool, accepting metadata through standard input and writing generated code to the terminal or a file. * Added safe handling for dates, timestamps, enums, arrays, comments, and reserved identifiers. * **Documentation** * Updated usage guidance, schema-target behavior, generated-code examples, options, and limitations. * **Tests** * Added comprehensive coverage for generation, parsing, serialization, views, relationships, enums, and typed data access. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
The native driver drops errno fields before classification, so IPv4-only hosts that fail as hostname resolving error (getaddrinfo ENOTFOUND) never retried through the pooler. Co-authored-by: Cursor <cursoragent@cursor.com>
CI tsc follows the package `bun` export into postgrest-typegen source, which fails under our noUncheckedIndexedAccess and has no pg-format types. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep develop's bun customConditions for @supabase/config and the pg-topo paths pin; add the same pin for postgrest-typegen published types. Co-authored-by: Cursor <cursoragent@cursor.com>
Path-mapping postgrest-typegen to its .d.ts made tsc pass but bun followed those declaration re-exports and broke compile plus the docs-spec unit test. Pin typegen only in tsconfig.types.json for types:check. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the pg-format compile workaround now that typegen inlines SQL literal escaping. Keep source-run oxfmt loading and a single trailing newline on generated output. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the native typegen dependency and oxfmt bindings. Take develop's other dep bumps and raise @supabase/pg-delta to 1.0.0-alpha.48 so db workflows pick up broader managed-schema RLS policy coverage and the ALTER ROLE search_path render fix. Co-authored-by: Cursor <cursoragent@cursor.com>
`--query-timeout 1ms` rounded to 0 and silently dropped both timeout guards. Parse through the shared Go duration helper, refuse rounded-to-0 except explicit disable, and document native TS/Python shape diffs. Co-authored-by: Cursor <cursoragent@cursor.com>
Member
Author
|
/ai-review |
Contributor
There was a problem hiding this comment.
🤖 AI Review
The reviews overlap on one major regression: gen types still accepts the Docker network override but no longer honors it, breaking Docker-network-only database hosts. Confirmed additional issues include incorrect filesystem-error telemetry, lost IPv6 fallback classification, an undocumented role change, and missing hermetic coverage for the new generator layer. Three findings were refuted because the behavior is intentional and documented or already supported by tested shared infrastructure.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | apps/cli/src/legacy/commands/gen/types/types.handler.ts:321 |
backward-compatibility |
claude+codex | gen types continues accepting --network-id and SUPABASE_NETWORK_ID but silently ignores them, breaking database hosts reachable only inside the requested Docker network. |
| 🟡 MINOR | apps/cli/src/legacy/commands/gen/types/types.errors.ts:58 |
error-handling |
claude | Failure to write the temporary TLS CA bundle is incorrectly classified as an internal CLI panic rather than an environmental filesystem error. |
| 🟡 MINOR | apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:62 |
behavior-documentation |
claude | Native remote generation now executes introspection after SET SESSION ROLE postgres for supabase_admin and cli_login_* users, but the command's database-side-effect documentation omits that role change. |
| 🟡 MINOR | apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:44 |
test-coverage |
claude | The new production generator layer lacks hermetic direct tests for its probe, CA-file, pool-acquisition, and introspection-timeout branches. |
| 🟡 MINOR | apps/cli/src/legacy/shared/legacy-connect-errors.ts:477 |
correctness |
claude | Native EHOSTUNREACH and EADDRNOTAVAIL IPv6 failures no longer trigger the pooler retry because their structured fields are discarded before classification and their rendered messages are not recognized. |
| ⚪ NIT | apps/cli/src/legacy/commands/gen/types/types.oxfmt.ts:132 |
maintainability |
claude | The duplicated oxfmt defaults are not guarded against drift when @<!---->supabase/postgrest-typegen is upgraded. |
| ⚪ NIT | apps/cli/src/legacy/commands/gen/types/types.integration.test.ts:428 |
test-quality |
claude | Most rewritten pooler-fallback tests use a legacy pgconn error string that the new native generator cannot emit. |
| ⚪ NIT | AGENTS.md:219 |
scope |
claude | The PR includes unrelated and partly duplicative agent workflow guidance in the root AGENTS.md. |
Findings outside the diff
- 🟡 MINOR
apps/cli/src/legacy/shared/legacy-connect-errors.ts:477— NativeEHOSTUNREACHandEADDRNOTAVAILIPv6 failures no longer trigger the pooler retry because their structured fields are discarded before classification and their rendered messages are not recognized.
Refuted findings (kept for transparency, not posted as review comments)
apps/cli/src/legacy/commands/gen/types/types.shared.ts:35(parity): Rejecting positive--query-timeoutvalues that round below one second is an unsanctioned compatibility regression.
Refuted: Concrete code comments, tests, and SIDE_EFFECTS documentation all identify this as an intentional validation rule with an actionable error. Trusted repository guidance requires intentional behavior changes to update tests and SIDE_EFFECTS.md, which this change does; it does not require a special “sanctioned divergence” label.apps/cli/src/legacy/commands/gen/types/types.shared.ts:62(correctness): Unconditionally settingstatement_timeout, including zero, creates an unsupported connection-string and Supavisor-options path.
Refuted: Zero must overwrite a DSN-provided timeout to implement the documented “flag wins” disable behavior. The shared default-branch connection layer already deliberately supports and testsoptions=reference=…combined with runtime-cflags, so this is neither a new unsupported form nor evidence of a connection failure.apps/cli/src/legacy/commands/gen/types/SIDE_EFFECTS.md:91(behavior-change): Removing theSUPABASE_CA_SKIP_VERIFY=truewarning leaves a meaningful TLS-verification behavior undocumented.
Refuted: The environment variable never controls the new probe or connection. The probe only reads the SSLRequest capability byte, and the actual generated TLS connection uses the pinned CA; emitting a warning that the variable “disabled” verification would therefore be inaccurate. The current SIDE_EFFECTS documentation correctly describes the replacement behavior.
Stats
Claude findings: 11 · Codex findings: 1 · Confirmed: 8 · Refuted: 3 · Uncertain: 0
Models: claude-opus-5 + gpt-5.6-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
| * Language generation or formatting failed after introspection succeeded — | ||
| * a CLI packaging / formatter / template defect, not a user schema finding. | ||
| */ | ||
| export class LegacyGenTypesGenerateError extends Data.TaggedError("LegacyGenTypesGenerateError")<{ |
Contributor
There was a problem hiding this comment.
🟡 MINOR · error-handling · source: claude
Failure to write the temporary TLS CA bundle is incorrectly classified as an internal CLI panic rather than an environmental filesystem error.
Evidence: types.generator.layer.ts:23-36 maps temporary-directory and file-write failures to LegacyGenTypesGenerateError, while types.errors.ts:54-63 documents that error as a post-introspection generation defect and assigns actionability.internalPanic.
Suggested fix: Use a distinct user-actionable filesystem error classification for CA-bundle materialization failures.
Comment on lines
+62
to
+65
| const pool = yield* legacyAcquirePgPool(conn, { | ||
| isLocal: input.isLocal, | ||
| dnsResolver: input.dnsResolver, | ||
| }); |
Contributor
There was a problem hiding this comment.
🟡 MINOR · behavior-documentation · source: claude
Native remote generation now executes introspection after SET SESSION ROLE postgres for supabase_admin and cli_login_* users, but the command's database-side-effect documentation omits that role change.
Evidence: types.generator.layer.ts:62-65 uses legacyAcquirePgPool; legacy-db-connection.sql-pg.layer.ts:796-799 and 948-960 apply SET SESSION ROLE postgres remotely. The removed container path only passed PG_META_DB_URL, while SIDE_EFFECTS.md:48-81 describes database access without listing the statement.
Suggested fix: Document the role change in SIDE_EFFECTS.md and confirm that introspection is intended to run with the postgres role's visibility.
Comment on lines
+44
to
+96
| Effect.scoped( | ||
| Effect.gen(function* () { | ||
| let conn = applyQueryTimeouts(input.conn, input.queryTimeoutSeconds); | ||
| // Remote DSNs without sslmode probe first (pg-meta did): no TLS → | ||
| // disable; TLS → require + the CA pin pg-meta got via | ||
| // PG_META_DB_SSL_ROOT_CERT. Probe failure keeps the driver default. | ||
| if (!input.isLocal && conn.sslmode === undefined) { | ||
| const probed = yield* sslProbe.requireSslForHost(conn.host, conn.port).pipe(Effect.result); | ||
| if (Result.isSuccess(probed)) { | ||
| if (!probed.success) { | ||
| conn = applyProbedSslMode(conn, false); | ||
| } else { | ||
| const sslrootcert = yield* pinProbedCaBundle(fs, path); | ||
| conn = applyProbedSslMode(conn, true, sslrootcert); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const pool = yield* legacyAcquirePgPool(conn, { | ||
| isLocal: input.isLocal, | ||
| dnsResolver: input.dnsResolver, | ||
| }); | ||
|
|
||
| // `introspect` drives the injected queryable itself, so the foreign | ||
| // Promise boundary is wrapped exactly once here; a live `pg.Pool` | ||
| // satisfies its `Queryable` contract directly. `statement_timeout` | ||
| // only bounds server-side execution — also cap the client wait so a | ||
| // stalled network cannot hang past `--query-timeout`. | ||
| const introspectEffect = Effect.tryPromise({ | ||
| try: () => | ||
| introspect( | ||
| pool, | ||
| input.includedSchemas.length > 0 ? { includedSchemas: [...input.includedSchemas] } : {}, | ||
| ), | ||
| catch: (cause) => | ||
| new LegacyGenTypesMetadataError({ | ||
| message: `failed to introspect database: ${describeCause(cause)}`, | ||
| }), | ||
| }); | ||
| const metadata = | ||
| input.queryTimeoutSeconds > 0 | ||
| ? yield* introspectEffect.pipe( | ||
| Effect.timeoutOrElse({ | ||
| duration: Duration.seconds(input.queryTimeoutSeconds), | ||
| orElse: () => | ||
| Effect.fail( | ||
| new LegacyGenTypesMetadataError({ | ||
| message: `introspection exceeded --query-timeout ${input.queryTimeoutSeconds}s`, | ||
| }), | ||
| ), | ||
| }), | ||
| ) | ||
| : yield* introspectEffect; |
Contributor
There was a problem hiding this comment.
🟡 MINOR · test-coverage · source: claude
The new production generator layer lacks hermetic direct tests for its probe, CA-file, pool-acquisition, and introspection-timeout branches.
Evidence: The gen/types directory contains no generator-layer test. types.integration.test.ts:145-166 replaces LegacyGenTypesGenerator with a recording fake, while types.unit.test.ts only covers the pure applyProbedSslMode/applyQueryTimeouts helpers. The optional remote e2e at types.e2e.test.ts:369-407 exercises only a live happy path when explicitly enabled.
Suggested fix: Add a hermetic generator-layer test covering TLS accepted/refused/probe-error outcomes, CA-write failure, and client-side introspection timeout.
Comment on lines
+132
to
+146
| /** | ||
| * Drop-in for `GenerateTypescriptOptions.format`, byte-equivalent to the | ||
| * typegen package's own oxfmt default (same virtual file name, same | ||
| * `semi`/`printWidth` options, same error surfacing). | ||
| */ | ||
| export async function legacyOxfmtTypegenFormat(code: string): Promise<string> { | ||
| const binding = legacyRequireOxfmtBinding(); | ||
| const { code: formatted, errors } = await binding.format( | ||
| "output.ts", | ||
| code, | ||
| { semi: false, printWidth: 80 }, | ||
| rejectEmbedded, | ||
| rejectEmbedded, | ||
| rejectEmbedded, | ||
| ); |
Contributor
There was a problem hiding this comment.
⚪ NIT · maintainability · source: claude
The duplicated oxfmt defaults are not guarded against drift when @<!---->supabase/postgrest-typegen is upgraded.
Evidence: types.oxfmt.ts:132-146 hard-codes output.ts, semi=false, and printWidth=80. types.unit.test.ts:165-184 only verifies binding dependency versions and never compares formatting with the package's default formatter.
Suggested fix: Add a fixture test comparing the injected formatter with postgrest-typegen's default output, or share the default options.
Comment on lines
+321
to
345
| const runTypegen = (input: { | ||
| readonly conn: LegacyPgConnInput; | ||
| readonly isLocal: boolean; | ||
| readonly includedSchemas: ReadonlyArray<string>; | ||
| readonly postgrestV9Compat: boolean; | ||
| readonly pgmetaVersionOverride?: string; | ||
| readonly poolerFallback?: { | ||
| readonly directHost: string; | ||
| readonly eligible: boolean; | ||
| readonly resolve: Effect.Effect<Option.Option<LegacyPgConnInput>, unknown>; | ||
| }; | ||
| }) => | ||
| Effect.scoped( | ||
| Effect.gen(function* () { | ||
| const buildRun = (target: { | ||
| readonly url: string; | ||
| readonly host: string; | ||
| readonly port: number; | ||
| readonly probeHost: string; | ||
| readonly probePort: number; | ||
| }) => | ||
| Effect.gen(function* () { | ||
| yield* output.raw(`Connecting to ${target.host} ${target.port}\n`, "stderr"); | ||
|
|
||
| // Each entry is a "KEY=VALUE" string, passed as a `--env | ||
| // KEY=VALUE` argument rather than a `--env-file`: env-files | ||
| // split on newlines, so they cannot carry the multi-line PEM CA | ||
| // bundle, and a value containing a newline could inject an extra | ||
| // variable. Passing argv elements keeps each entry as exactly | ||
| // one variable regardless of its contents. | ||
| const env = [ | ||
| `PG_META_DB_URL=${target.url}`, | ||
| `PG_CONN_TIMEOUT_SECS=${queryTimeoutSeconds}`, | ||
| `PG_QUERY_TIMEOUT_SECS=${queryTimeoutSeconds}`, | ||
| `PG_META_GENERATE_TYPES=${lang}`, | ||
| `PG_META_GENERATE_TYPES_INCLUDED_SCHEMAS=${input.includedSchemas}`, | ||
| `PG_META_GENERATE_TYPES_SWIFT_ACCESS_CONTROL=${swiftAccessControl}`, | ||
| `PG_META_GENERATE_TYPES_DETECT_ONE_TO_ONE_RELATIONSHIPS=${String(!input.postgrestV9Compat)}`, | ||
| ]; | ||
|
|
||
| // Emitted to stderr when the probe runs with certificate | ||
| // verification disabled. Our wire-level SSLRequest probe never | ||
| // verifies certificates, so honour the same env var here too. | ||
| if (process.env["SUPABASE_CA_SKIP_VERIFY"] === "true") { | ||
| yield* output.raw( | ||
| "WARNING: TLS certificate verification disabled for SSL probe (SUPABASE_CA_SKIP_VERIFY=true)\n", | ||
| "stderr", | ||
| ); | ||
| } | ||
|
|
||
| const useTls = yield* sslProbe.requireSslForHost(target.probeHost, target.probePort); | ||
| if (useTls) { | ||
| env.push(`PG_META_DB_SSL_ROOT_CERT=${legacyRootCaBundle()}`); | ||
| } | ||
|
|
||
| // `--network-id` overrides any base network mode (even the | ||
| // "host" mode used for --db-url), so honour the override here too. | ||
| const networkMode = Option.isSome(networkId) ? networkId.value : input.networkMode; | ||
| const pgmetaImage = resolvePgmetaImage(input.pgmetaVersionOverride); | ||
| const args = [ | ||
| "run", | ||
| "--rm", | ||
| "--network", | ||
| networkMode, | ||
| ...env.flatMap((entry) => ["--env", entry]), | ||
| pgmetaImage, | ||
| "node", | ||
| "dist/server/server.js", | ||
| ]; | ||
| const child = yield* spawnContainerCli(spawner, args, { | ||
| stdin: "ignore", | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| let stderrText = ""; | ||
| const [exitCode] = yield* Effect.all( | ||
| [ | ||
| child.exitCode.pipe(Effect.map(Number)), | ||
| forwardByteStream(child.stdout, (text) => output.raw(text, "stdout")), | ||
| forwardByteStream(child.stderr, (text) => | ||
| Effect.sync(() => { | ||
| stderrText += text; | ||
| }).pipe(Effect.andThen(output.raw(text, "stderr"))), | ||
| ), | ||
| ], | ||
| { concurrency: "unbounded" }, | ||
| ); | ||
| return { exitCode, stderrText }; | ||
| Effect.gen(function* () { | ||
| const generateTarget = (conn: LegacyPgConnInput, isLocal: boolean) => | ||
| Effect.gen(function* () { | ||
| yield* output.raw(`Connecting to ${conn.host} ${conn.port}\n`, "stderr"); | ||
| return yield* generator.generate({ | ||
| conn, | ||
| isLocal, | ||
| dnsResolver, | ||
| lang, | ||
| includedSchemas: input.includedSchemas, | ||
| postgrestV9Compat: input.postgrestV9Compat, | ||
| swiftAccessControl, | ||
| queryTimeoutSeconds, | ||
| }); |
Contributor
There was a problem hiding this comment.
🟠 MAJOR · backward-compatibility · source: claude+codex
gen types continues accepting --network-id and SUPABASE_NETWORK_ID but silently ignores them, breaking database hosts reachable only inside the requested Docker network.
Evidence: types.handler.ts:321-345 passes no network setting to native generation, while shared/legacy/global-flags.ts:73-77 still exposes --network-id. The diff removes the handler's LegacyNetworkIdFlag read and previous Docker --network override; SIDE_EFFECTS.md:78-81 acknowledges that such hostnames no longer resolve.
Suggested fix: Use a container-backed path when a network override is supplied, or reject the override with a clear actionable error.
Comment on lines
+428
to
435
| const IPV6_CONNECT_FAILURE = new LegacyDbConnectError({ | ||
| message: `failed to connect to postgres: could not translate host name "db.${LEGACY_VALID_REF}.supabase.co" to address: No address associated with hostname`, | ||
| }); | ||
|
|
||
| try { | ||
| return await run(address.port); | ||
| } finally { | ||
| await new Promise<void>((resolve, reject) => | ||
| server.close((error) => (error ? reject(error) : resolve())), | ||
| ); | ||
| } | ||
| } | ||
| const NATIVE_ENOTFOUND_CONNECT_FAILURE = new LegacyDbConnectError({ | ||
| message: `failed to connect to postgres: failed to connect to \`host=db.${LEGACY_VALID_REF}.supabase.co user=postgres database=postgres\`: hostname resolving error (getaddrinfo ENOTFOUND)`, | ||
| }); | ||
|
|
Contributor
There was a problem hiding this comment.
⚪ NIT · test-quality · source: claude
Most rewritten pooler-fallback tests use a legacy pgconn error string that the new native generator cannot emit.
Evidence: types.integration.test.ts:428-430 defines IPV6_CONNECT_FAILURE using could not translate host name; it is used at lines 1139, 1312, 1351, 1390, 1560, 1619, and 1838. The native-shaped fixture at lines 432-434 is used only at line 1235, while legacyConnectCauseDetail renders native DNS failures as hostname resolving error (...).
Suggested fix: Make the shared fixture use a native driver error shape and retain a separate legacy-text case only where backward-tolerant classification is intentionally tested.
Comment on lines
+219
to
+220
| Never `git commit` or `git push` until lint and `types:check` have been run and passed for the change. Targeted unit/integration tests are not a substitute — CI Check code quality runs `pnpm check:all` (`types:check`, oxlint, oxfmt, knip). Before commit or push, from each changed TypeScript workspace run `pnpm types:check`, and from the repo root run `pnpm exec oxlint` (or `pnpm check:all`). If those fail, fix them before committing. | ||
| After every `git push` to a branch that has a PR, check GitHub CI for that PR (`gh pr checks` / `gh run list`) and report whether it is green. If it is not, diagnose and fix; do not leave a red PR as done. |
Contributor
There was a problem hiding this comment.
⚪ NIT · scope · source: claude
The PR includes unrelated and partly duplicative agent workflow guidance in the root AGENTS.md.
Evidence: AGENTS.md:219-220 adds commit/push and GitHub-CI instructions immediately before the existing lines 222-224 requiring relevant checks to pass and unresolved failures to be fixed. The gen-types implementation does not depend on this guidance.
Suggested fix: Move the workflow-policy change to a separate PR and consolidate it with the existing quality-check section.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the pg-meta Docker container behind
gen typeswith the new@supabase/postgrest-typegenpackage (0.2.0), running introspection and language generation in-process over a direct Postgres connection.LegacyGenTypesGeneratorservice seam: the production layer acquires a scopedpg.PoolvialegacyAcquirePgPool(full driver parity: TLS mode, DoH resolver, connect-error mapping), feeds it tointrospect(), sorts withsortGeneratorMetadata, and renders typescript/go/python/swift.--localconnects to the host-mapped db port instead of spawning the pg-meta image inside the stack network; thecontainer inspectstack-running check and rest-version v9 forcing are unchanged. The obsolete.temp/pgmeta-versionimage override is removed.--db-urlnow resolves through the sharedLegacyDbConfigResolver(libpq keywords,options=referencepooler tenants,sslmode,PG*env fallbacks), matching every other--db-urlcommand. When the DSN carries no explicitsslmode, the existing SSLRequest probe decides whether to connect withsslmode=disable, so plain-TCP servers (common when self-hosting) keep working as they did with pg-meta.--linked/--project-idTypeScript path still uses the Management API unchanged.--query-timeoutmaps tostatement_timeoutplus the connect timeout;--postgrest-v9-compatdisables one-to-one detection in the TypeScript generator; output keeps the trailing newline pg-meta'sconsole.logadded.createRequire(import.meta.url), whichbun build --compilecannot follow, so the CLI embeds the platform binding statically (the@parcel/watcherpattern) and injects it through the generator'sformatoption — verified byte-equivalent to the package default. The never-installed optional prettier plugins oxfmt lazily imports are marked external in both build scripts.Output parity against postgres-meta 0.98.0 on the same database: Swift byte-identical; Go and Python identical content in canonical sorted order (pg-meta emitted environment-dependent SQL row order); TypeScript identical content with oxfmt's union-wrapping style. Details in the command's
SIDE_EFFECTS.md.Linked issue
Resolves CLI-2279 (no GitHub issue).
open-for-contributionlabel (or I'm a Supabase maintainer).Checklist
fix(cli): …).pnpm check:allpasses; relevant package tests pass for every touched workspace, andpnpm types:checkpasses for each touched TypeScript workspace (or workspace declaring it).https://claude.ai/code/session_01RpnbmgTzwdTMkWYF7uFR1V