feat(cli): generate types natively with postgrest-typegen - #6404
feat(cli): generate types natively with postgrest-typegen#6404avallete wants to merge 19 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>
## 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.
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@174ebfd243cd707a2d4e012e5cde4790e3d6a7b1Preview package for commit |
|
/ai-review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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.
…#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>
|
/ai-review |
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 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.
Native generation cannot join a Docker network. Surface a docker-run workaround and classify native IPv6 dial failures so the pooler retry still runs. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/ai-review |
There was a problem hiding this comment.
🤖 AI Review
Adjudication confirmed 10 of 11 findings. The most serious issues are a TLS probe failure that falls back to unverified TLS, removal of functional --network-id support, ineffective cancellation of timed-out introspection, and connection-wide reuse of one host's TLS probe result. The Supavisor startup-options concern remains uncertain because the repository proves the payload changed but contains no evidence establishing whether Supavisor rejects it.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🔴 CRITICAL | apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:51 |
security |
codex | A failed TLS capability probe is swallowed and the subsequent connection uses the driver's unverified default TLS mode. |
| 🔴 CRITICAL | apps/cli/src/legacy/commands/gen/types/types.shared.ts:98 |
backward-compatibility |
codex | The PR removes functional --network-id support, breaking generation for databases reachable only through the selected Docker network. |
| 🟠 MAJOR | apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:72 |
cancellation |
codex | The client-side introspection timeout does not cancel its Promise or active database query, so stalled network I/O can outlive the timeout and retain the socket. |
| 🟠 MAJOR | apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:51 |
connection-handling |
codex | For multi-host connections, the primary host's TLS probe result is applied as one sslmode to every fallback host, preventing failover when hosts have different TLS capabilities. |
| 🟡 MINOR | apps/cli/src/legacy/commands/gen/types/types.handler.ts:449 |
correctness |
claude | The --network-id warning is omitted when the persistent flag appears before the command path. |
| 🟡 MINOR | apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:38 |
test-coverage |
claude | The production generator's TLS probing, CA lifecycle, timeout behavior, and error mapping lack focused unit or integration coverage. |
| 🟡 MINOR | apps/cli/src/legacy/commands/gen/types/types.shared.ts:58 |
correctness |
claude | The default query timeout is appended to a Supavisor tenant's options string as reference=<ref> -c statement_timeout=15000, which may be incompatible with pooler parsing. |
| 🟡 MINOR | apps/cli/tsconfig.types.json:5 |
build |
claude | CI type-checking validates postgrest-typegen's published declarations while Bun runtime resolution loads its source export, leaving the actual runtime-resolved API outside the TypeScript check. |
| ⚪ NIT | apps/cli/src/legacy/commands/gen/types/types.oxfmt.ts:53 |
error-handling |
claude | loadOxfmtBinding treats every ReferenceError as proof that require is unavailable, potentially masking a ReferenceError raised during addon initialization. |
| ⚪ NIT | apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:1 |
performance |
claude | postgrest-typegen and its transitive modules are evaluated on every CLI invocation because they are imported statically from the root command graph. |
| ⚪ NIT | apps/cli/src/legacy/shared/legacy-temp-paths.ts:41 |
dead-code |
claude | LegacyTempPaths.pgmetaVersion has no production consumer after the pg-meta image override was removed. |
Findings outside the diff
- ⚪ NIT
apps/cli/src/legacy/shared/legacy-temp-paths.ts:41— LegacyTempPaths.pgmetaVersion has no production consumer after the pg-meta image override was removed.
Stats
Claude findings: 7 · Codex findings: 4 · Confirmed: 10 · Refuted: 0 · Uncertain: 1
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.
| const networkIdOverride = legacyPflagStringValue(occurrences, "network-id"); | ||
| if (Option.isSome(networkIdOverride)) { | ||
| yield* output.warn(legacyGenTypesNetworkIdUnusedWarning(networkIdOverride.value)); | ||
| } |
There was a problem hiding this comment.
🟡 MINOR · correctness · source: claude
The --network-id warning is omitted when the persistent flag appears before the command path.
Evidence: apps/cli/src/legacy/commands/gen/types/types.handler.ts:449 reads only scan.occurrences. apps/cli/src/shared/cli/cobra-flag-groups.ts:175-184 stores pre-command persistent flags separately in prePathOccurrences, and network-id is registered as persistent at lines 113-121.
Suggested fix: Consult scan.prePathOccurrences for network-id, as the workdir and profile reconcilers do, and test supabase --network-id mynet gen types ....
| const generate = ( | ||
| sslProbe: LegacyPgDeltaSslProbe["Service"], | ||
| fs: FileSystem.FileSystem, | ||
| path: Path.Path, | ||
| input: LegacyGenTypesGenerateInput, | ||
| ) => | ||
| 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; | ||
|
|
||
| // Canonical sort before generation so output is deterministic regardless | ||
| // of the introspection queries' heap order. | ||
| const sorted = sortGeneratorMetadata(metadata); | ||
|
|
||
| const generateError = (cause: unknown) => | ||
| new LegacyGenTypesGenerateError({ | ||
| message: `failed to generate ${input.lang} types: ${describeCause(cause)}`, | ||
| }); | ||
|
|
||
| switch (input.lang) { | ||
| case "typescript": | ||
| return yield* Effect.tryPromise({ | ||
| try: () => | ||
| generateTypescript(sorted, { | ||
| detectOneToOneRelationships: !input.postgrestV9Compat, | ||
| // The statically-embedded oxfmt binding (see types.oxfmt.ts); | ||
| // the package's own default formatter cannot load its native | ||
| // addon inside the compiled binary. | ||
| format: legacyOxfmtTypegenFormat, | ||
| }), | ||
| catch: generateError, | ||
| }); | ||
| case "go": | ||
| return yield* Effect.try({ try: () => generateGo(sorted), catch: generateError }); | ||
| case "python": | ||
| return yield* Effect.try({ try: () => generatePython(sorted), catch: generateError }); | ||
| case "swift": | ||
| return yield* Effect.try({ | ||
| try: () => generateSwift(sorted, { accessControl: input.swiftAccessControl }), | ||
| catch: generateError, | ||
| }); | ||
| } | ||
| }), | ||
| ); | ||
|
|
||
| /** | ||
| * Production `LegacyGenTypesGenerator`: a scoped `pg.Pool` with the shared | ||
| * driver-layer connection parity (TLS mode, DoH resolver, fallback hosts), | ||
| * introspected and rendered by `@supabase/postgrest-typegen`. | ||
| */ | ||
| export const legacyGenTypesGeneratorLayer = Layer.effect( | ||
| LegacyGenTypesGenerator, | ||
| Effect.gen(function* () { | ||
| const sslProbe = yield* LegacyPgDeltaSslProbe; | ||
| const fs = yield* FileSystem.FileSystem; | ||
| const path = yield* Path.Path; | ||
| return { | ||
| generate: (input: LegacyGenTypesGenerateInput) => generate(sslProbe, fs, path, input), | ||
| }; | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🟡 MINOR · test-coverage · source: claude
The production generator's TLS probing, CA lifecycle, timeout behavior, and error mapping lack focused unit or integration coverage.
Evidence: apps/cli/src/legacy/commands/gen/types/types.integration.test.ts:150-165 replaces LegacyGenTypesGenerator with a fake. No test imports types.generator.layer.ts. The Docker e2e at types.e2e.test.ts:321-407 supplies only broad happy-path coverage, with its remote case explicitly opt-in.
Suggested fix: Add focused tests around the production layer or extract injectable pool and typegen boundaries to cover both probe outcomes, CA cleanup, timeout cancellation, and error mapping.
| export function applyQueryTimeouts( | ||
| conn: LegacyPgConnInput, | ||
| queryTimeoutSeconds: number, | ||
| ): LegacyPgConnInput { | ||
| const runtimeParams = { | ||
| ...conn.runtimeParams, | ||
| statement_timeout: `${queryTimeoutSeconds * 1000}`, | ||
| }; | ||
| if (queryTimeoutSeconds > 0 && conn.connectTimeoutSeconds === undefined) { | ||
| return { ...conn, connectTimeoutSeconds: queryTimeoutSeconds, runtimeParams }; | ||
| } |
There was a problem hiding this comment.
🟡 MINOR · correctness · source: claude
The default query timeout is appended to a Supavisor tenant's options string as reference=<ref> -c statement_timeout=15000, which may be incompatible with pooler parsing.
Evidence: apps/cli/src/legacy/commands/gen/types/types.shared.ts:58-68 always adds statement_timeout. apps/cli/src/legacy/shared/legacy-db-connection.sql-pg.layer.ts:405-413 appends runtime parameters to existing options, and its unit test at lines 105-113 confirms the resulting reference=abc -c ... form.
Suggested fix: Verify this form against a live Supavisor connection. If unsupported, apply statement_timeout after checkout or otherwise keep the tenant-routing options payload intact.
Adjudication (uncertain): The changed startup payload is verified, but the checked-out repository contains no Supavisor implementation or live test showing whether that payload is accepted or rejected.
| // noUncheckedIndexedAccess. Keep this pin off the bun-visible | ||
| // tsconfig — bun cannot resolve the `.d.ts` `./go.ts` re-exports. | ||
| "paths": { | ||
| "@supabase/pg-topo": ["./node_modules/@supabase/pg-topo/dist/index.d.ts"], | ||
| "@supabase/postgrest-typegen/generation": [ | ||
| "./node_modules/@supabase/postgrest-typegen/dist/generation/index.d.ts" | ||
| ], | ||
| "@supabase/postgrest-typegen/introspection": [ | ||
| "./node_modules/@supabase/postgrest-typegen/dist/introspection/index.d.ts" | ||
| ] |
There was a problem hiding this comment.
🟡 MINOR · build · source: claude
CI type-checking validates postgrest-typegen's published declarations while Bun runtime resolution loads its source export, leaving the actual runtime-resolved API outside the TypeScript check.
Evidence: apps/cli/tsconfig.types.json:7-14 pins the imports to dist declarations, while apps/cli/tsconfig.json:20-22 deliberately excludes those pins. apps/cli/vitest.config.ts:29-30 adds the bun export condition used by runtime tests.
Suggested fix: Document the split explicitly and add a runtime API smoke test or another check that validates the Bun-resolved exports used by the generator.
| function loadOxfmtBinding( | ||
| loadCompiled: () => LegacyOxfmtBinding, | ||
| specifier: string, | ||
| ): LegacyOxfmtBinding { | ||
| try { | ||
| return loadCompiled(); | ||
| } catch (error) { | ||
| if (error instanceof ReferenceError) { | ||
| return sourceRequire(specifier); | ||
| } | ||
| throw error; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
⚪ NIT · error-handling · source: claude
loadOxfmtBinding treats every ReferenceError as proof that require is unavailable, potentially masking a ReferenceError raised during addon initialization.
Evidence: apps/cli/src/legacy/commands/gen/types/types.oxfmt.ts:57-63 catches solely by error instanceof ReferenceError and then retries through sourceRequire, although lines 14-18 describe only a missing require binding as the intended fallback condition.
Suggested fix: Fallback only when require is unavailable or when the error specifically identifies an undefined require binding.
| import { | ||
| generateGo, | ||
| generatePython, | ||
| generateSwift, | ||
| generateTypescript, | ||
| sortGeneratorMetadata, | ||
| } from "@supabase/postgrest-typegen/generation"; | ||
| import { introspect } from "@supabase/postgrest-typegen/introspection"; |
There was a problem hiding this comment.
⚪ NIT · performance · source: claude
postgrest-typegen and its transitive modules are evaluated on every CLI invocation because they are imported statically from the root command graph.
Evidence: apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:1-8 imports typegen at module scope; types.layers.ts:26, types.command.ts:7, gen.command.ts:2, and legacy/cli/root.ts:13 connect it to the eagerly imported root graph.
Suggested fix: Load postgrest-typegen inside generate with dynamic imports and measure startup time before and after.
| 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); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 CRITICAL · security · source: codex
A failed TLS capability probe is swallowed and the subsequent connection uses the driver's unverified default TLS mode.
Evidence: apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:51 converts probe failure into a Result and only modifies the connection on success. apps/cli/src/legacy/shared/legacy-db-connection.sql-pg.layer.ts:507-508 maps an unset sslmode to rejectUnauthorized:false. The removed handler propagated requireSslForHost failures.
Suggested fix: Propagate probe failures, or use a fail-closed verified TLS configuration when probing cannot determine the endpoint's capability.
| "--network-id is unused: gen types no longer runs inside a container and cannot join a Docker network.\n" + | ||
| "To reach a hostname that exists only on that network:\n" + | ||
| ` docker run --rm --network ${network} node:lts npx --yes supabase gen types --db-url <url>` |
There was a problem hiding this comment.
🔴 CRITICAL · backward-compatibility · source: codex
The PR removes functional --network-id support, breaking generation for databases reachable only through the selected Docker network.
Evidence: apps/cli/src/legacy/commands/gen/types/types.shared.ts:98-100 says the in-process generator cannot join the network, and types.handler.ts:449-452 only warns. The trusted default-branch types.handler.ts previously passed the selected network to docker run --network, with integration coverage asserting the override.
Suggested fix: Retain a container-backed path when --network-id is supplied or provide an equivalent network-aware execution path before removing container generation.
| 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`, | ||
| }), | ||
| ), | ||
| }), | ||
| ) |
There was a problem hiding this comment.
🟠 MAJOR · cancellation · source: codex
The client-side introspection timeout does not cancel its Promise or active database query, so stalled network I/O can outlive the timeout and retain the socket.
Evidence: apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:72-82 wraps introspect without a cancellation signal or interruption finalizer; lines 85-95 only time out the Effect. The pool finalizer at legacy-db-connection.sql-pg.layer.ts:708-710 abandons pool.end after one second rather than destroying the active query socket.
Suggested fix: Run introspection through an interruption-aware checked-out client and destroy or cancel that client on interruption; test that timeout closes the socket and lets the process exit.
| 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, { |
There was a problem hiding this comment.
🟠 MAJOR · connection-handling · source: codex
For multi-host connections, the primary host's TLS probe result is applied as one sslmode to every fallback host, preventing failover when hosts have different TLS capabilities.
Evidence: apps/cli/src/legacy/commands/gen/types/types.generator.layer.ts:51 probes only conn.host/conn.port and lines 54-57 mutate the shared connection. apps/cli/src/legacy/shared/legacy-db-connection.sql-pg.layer.ts:777-790 then expands that same connection across cfg.fallbacks, and lines 890-899 derive every attempt from the single cfg.sslmode.
Suggested fix: Probe and configure each dial target independently, or avoid replacing the connection-wide mode when fallbacks are present.
…ork-id A failed SSL probe no longer falls through to unverified TLS, and the unused --network-id warning now covers the cobra-normal persistent-flag position. Co-authored-by: Cursor <cursoragent@cursor.com>
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
| if (!input.isLocal && conn.sslmode === undefined && !legacyIsUnixSocketHost(conn.host)) { | ||
| const useTls = yield* sslProbe.requireSslForHost(conn.host, conn.port); |
There was a problem hiding this comment.
🟠 Severity: HIGH
With --dns-resolver=https, the actual pool resolves this hostname through Cloudflare DoH, but this TLS decision uses native DNS. An attacker or split-horizon resolver can make the probe return N while DoH reaches TLS Postgres; the resulting forced plaintext connection exposes the database password and queried data.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: When --dns-resolver=https is active (input.dnsResolver === 'https'), the SSL probe (sslProbe.requireSslForHost) uses Node.js net.connect() which resolves via native OS DNS (getaddrinfo), NOT via Cloudflare DoH. Since legacyAcquirePgPool later resolves via DoH, these two resolution paths can diverge, allowing a split-horizon attacker to steer the native-DNS probe to a non-TLS host (returning N/false), causing applyProbedSslMode to set sslmode=disable while the actual pool connects (via DoH) to the real TLS-capable Postgres server in plaintext.
The fix is to skip the unreliable native-DNS-backed probe when dnsResolver === 'https' and instead fail closed by unconditionally requiring TLS. Wrap the existing probe path in an else branch guarded by input.dnsResolver !== 'https', and add a short-circuit branch for the DoH case that goes straight to applyProbedSslMode(conn, true, sslrootcert) without calling the probe.
⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.
| if (!input.isLocal && conn.sslmode === undefined && !legacyIsUnixSocketHost(conn.host)) { | |
| const useTls = yield* sslProbe.requireSslForHost(conn.host, conn.port); | |
| if (!input.isLocal && conn.sslmode === undefined && !legacyIsUnixSocketHost(conn.host)) { | |
| if (input.dnsResolver === "https") { | |
| // The SSL probe uses native OS DNS (net.connect / getaddrinfo) and does not | |
| // honour --dns-resolver=https. When native DNS and DoH diverge an adversary | |
| // controlling the native answer can steer the probe to a non-TLS endpoint, | |
| // forcing sslmode=disable on the DoH-resolved pool connection. Fail closed: | |
| // require TLS without probing when DoH is active. | |
| const sslrootcert = yield* pinProbedCaBundle(fs, path); | |
| conn = applyProbedSslMode(conn, true, sslrootcert); | |
| } else { | |
| const useTls = yield* sslProbe.requireSslForHost(conn.host, conn.port); | |
| if (!useTls) { | |
| conn = applyProbedSslMode(conn, false); | |
| } else { | |
| const sslrootcert = yield* pinProbedCaBundle(fs, path); | |
| conn = applyProbedSslMode(conn, true, sslrootcert); | |
| } | |
| } | |
| } |
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