feat(config): trim the public surface and add a compiled build (CLI-2234, CLI-2232) - #6366
Conversation
….json artifact (CLI-2234, CLI-2232)
…nd check in an API report (CLI-2232)
…onditions (CLI-2234)
Superseded by a newer AI review🤖 AI ReviewThis PR is too large for a full AI review (+10567/-520 lines across 118 files). A maintainer can request a review anyway with a |
1 similar comment
Superseded by a newer AI review🤖 AI ReviewThis PR is too large for a full AI review (+10567/-520 lines across 118 files). A maintainer can request a review anyway with a |
…d declaration compare (CLI-2234)
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Adjudicated 11 raw findings into 10 deduplicated findings: nine confirmed and one refuted. The most consequential issue is that unbuilt workspace tests can resolve @supabase/config to nonexistent dist files; the shallow-checkout API comparison and ProjectConfig primitive acceptance are also confirmed.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | packages/config/package.json:34 |
build-resolution |
claude | Workspace tests can resolve ., ./effect, and ./internal to nonexistent dist files because only the bun condition points to source and Vitest is not configured to request that condition. |
| 🟡 MINOR | tools/config-api-compare.ts:100 |
ci-correctness |
claude+codex | The API comparison fallback cannot reliably find a merge base from the workflow's depth-one checkout because it fetches only the base tip without deepening HEAD's history. |
| 🟡 MINOR | packages/config/src/internal.ts:21 |
api-contract |
claude | InternalResolveCliConfigOptions is repeatedly documented as exported from @<!---->supabase/config/internal, but that entrypoint does not export it. |
| 🟡 MINOR | packages/config/src/schema-metadata.ts:3 |
schema-publishing |
claude | PROJECT_CONFIG_SCHEMA_URL identifies a document that the docs generator never publishes, while the published CLI schema bypasses the post-processing applied to the packaged schema. |
| 🟡 MINOR | packages/config/src/project-config/project-schema.ts:151 |
validation |
codex | Object-shaped fields that become empty AST object nodes accept primitive values, so ProjectConfigSchema can validate values such as db.vault: 42. |
| ⚪ NIT | packages/config/src/errors.ts:135 |
documentation |
claude | Removing MissingCliConfigValueError leaves stale references to that symbol in the repository's root guidance documents. |
| ⚪ NIT | packages/config/scripts/json-schema-postprocess.ts:234 |
correctness |
claude | withSchemaMetadata allows pre-existing root title and description values to overwrite the metadata supplied by its caller. |
| ⚪ NIT | packages/config/scripts/build.ts:86 |
path-handling |
claude | JSON schema output paths are resolved from the process working directory rather than the package root used by the rest of the build. |
| ⚪ NIT | packages/config/AGENTS.md:120 |
documentation |
claude | The testing documentation says three contract tests exist but lists four bullets containing five test modules. |
Refuted findings (kept for transparency, not posted as review comments)
apps/cli/tsconfig.json:12(documentation): ThecustomConditions: ["bun"]change silently affects @supabase/stack and @supabase/api in addition to the specifically named pg-topo package.
Refuted: The comment is deliberately general and explicitly warns that every other dependency declaringbunis affected;e.g.makes pg-topo non-exhaustive. Selecting the Bun variants is also consistent with this Bun-run CLI.
Stats
Claude findings: 9 · Codex findings: 2 · Confirmed: 9 · Refuted: 1 · 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.
…e on >=1 review (supabase#6380) Follow-up to the AI-review pipeline. Two changes. ## 1. Downgrade `codex-action` v1.12 → v1.11 On the large PR (supabase#6366, ~130k-token diff) the `codex-review` step ran to completion — Codex finished the turn, wrote its output file, printed its final message and token count — then **sat idle until the 45-minute job timeout**, discarding a completed review. That is a confirmed v1.12 regression: **openai/codex-action#150** ("v1.12: Linux run never returns after the turn completes; job dies on timeout with the output file already written"). The reporter confirms **v1.11 handles the same heavy workload cleanly**, and there is no released fix above v1.12. v1.11 (`52fe01ec…`) supports every input we use (`safety-strategy`, `sandbox`, `output-schema-file`, `output-file`, `codex-version`, `working-directory`, `effort`), so this is a drop-in pin change in both Codex jobs. ## 2. Adjudicate on ≥1 independent review (graceful degradation) Previously `adjudicate` required BOTH `claude-review` and `codex-review` to succeed, so one flaky model job sank the whole review. Now it runs when **at least one** independent pass succeeded: each findings download is guarded by its job's result, and the stage step substitutes an empty findings set for any review that didn't complete, so the adjudicator reconciles one or two. The prompt notes the one-review case and records it in its summary. Together: a Codex hiccup no longer wastes a 45-minute run or blocks Claude's (working) review from being posted.
…R (CLI-2234, CLI-2232)
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Two documentation findings are confirmed. The packaging finding is refuted because the README explicitly says publication is deferred, while Claude's placeholder finding contains no verifiable claim and remains uncertain.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | apps/cli/vitest.config.ts:25 |
testing |
claude | The submitted finding only claims "test" and does not identify a concrete defect. |
| 🟡 MINOR | packages/config/README.md:284 |
documentation |
codex | The documented ./io error-handling contract gives contradictory and unusable type/message guidance. |
| ⚪ NIT | packages/config/README.md:265 |
documentation |
codex | The README incorrectly calls the two resolver functions the only non-Promise exports from ./io. |
Refuted findings (kept for transparency, not posted as review comments)
packages/config/package.json:4(packaging): The package remains marked private, so npm will refuse to publish the newly prepared public package.
Refuted: The private flag is intentional for this PR. The newly added README explicitly documents deferred publication, and trusted/CLAUDE.md says internal TypeScript workspaces such as packages/config generally follow the private workspace template.
Stats
Claude findings: 1 · Codex findings: 3 · Confirmed: 2 · Refuted: 1 · 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.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@98728256a07c47f5fba0710803d20106163759a0Preview package for commit |
Closes CLI-2234
Closes CLI-2232
What changed
Prepares
@supabase/configfor its first npm publish: the export surface is audited and trimmed (CLI-2234) and the package gains a real compiled build (CLI-2232). One PR because the audit decides what the entrypoints contain and the build compiles exactly that, and the audit's type-surface enforcement (API report) only works once the build emits.d.ts.Export-surface audit (CLI-2234)
Every export got an explicit keep / move / trim decision:
apps/cliKONG_LOCAL_CA_CERT(+ its test) — a local-stack TLS asset, not config schema./internalsubpath (explicitly not semver-covered; enforced apps/cli-only)ENV_CAPTURE_REGEX,AUTH_HOOK_NAMES,unmappedSecretApiPaths,projectConfigMappingRows,ProjectConfigMappingRow,ProjectConfigApiAttributes,InternalLoadCliConfigOptions+ goViperCompat-capable typings ofloadCliConfig/resolveCliConfigValue/resolveCliConfigSubtreegoViperCompat(offLoadCliConfigOptions; resolvers lost their options param entirely — re-adding later is non-breaking)MissingCliConfigValueErrordeleted entirely (never constructed anywhere in the package; apps/cli only carried a telemetry mapping entry + a test that fabricated one — both removed),loadCliConfigFile/InternalResolveCliConfigOptionsoff the internal barrel (zero consumers)./io, zero consumers existed)findCliProjectRootFor→findCliProjectRoot,findCliProjectPathsFor→findCliProjectPaths,loadCliProjectEnvironmentFor→loadCliProjectEnvironment,loadFunctionsManifest→inferFunctionsManifest—./ionow mirrors./effect1:1; the subpath conveys Promise-vs-EffectresolveCliConfigValue/resolveCliConfigSubtreeon.(Effect-typed variants deliberately shadow them on./effect);ProjectConfigSchema(runtime, Standard Schema v1 viaSchema.toStandardSchemaV1— one symbol serves Effect-native and~standardconsumers);toProjectConfigJsonSchema;PROJECT_CONFIG_SCHEMA_URLtoProjectConfig,fromConfigDocument,fromApiProjectConfig,attachApiResponse,comparableProjectConfigPaths,unmappedApiFields), errors, functions-manifest modelProjectConfigSchemais derived fromCliConfigSchemaat the AST level (hosted sections → type-side → deep-optional →x-secretleaves dropped → cross-field checks stripped, leaf checks kept) with a two-way compile-time assignability pin against theProjectConfigtype, so the runtime schema and the type cannot drift../io's error channel is narrowed fromunknownto the exact five-member union (verified exact by review: no wider member, none unused).Compiled build (CLI-2232)
tsc(tsgo 7.0.2,nodenext+rewriteRelativeImportExtensions) emits ESM.js+.d.ts+ maps todist/; no bundler.exports: in-repo Bun resolvessrc/*.ts(withcustomConditions: ["bun"]so tsc typechecks against source, not stale dist); external consumers getdistjs + types.files+.npmignore(npm-packlist otherwise applies the root.gitignoreand ships nodist—npm packandpnpm packnow agree); publish metadata (license, repository, publishConfig, engines) added; peers widened to>=4.0.0-rc.111 <5.sideEffects: false, proven by a tree-shake probe with positive + negative controls against the built artifact.dist/project-schema.jsonjoinsdist/schema.json(both draft-2020-12, now with$id/title, and with Effect's non-finite-numberanyOfencoding collapsed so numeric fields keepdescription/default).generate-docs.tscopiesdist/*.jsontoapps/docs/public/cli/{config,project-config}.schema.json; docs generatedependsOnthe package build) — one post-processed source of truth, and both$idURLs resolve once deployed.bunexport condition (resolve.conditions/ssr.resolve.conditionsin the vitest configs), so vitest exercisessrc— never a stale or absentdist(caught by the AI review: previously tests resolved thedefault→distbranch).npm pack→ temp install → realnodeimports every subpath.Enforcement (surface changes stay deliberate)
./internal), purity walker overindex.tsandio-browser.ts.pnpm run check:config-api(root task; advisorycontinue-on-errorCI step) emits declarations for the PR base and head — base source extracted viagit archiveinto the package dir so the current install resolves deps, no second install — and reports the.d.tsdiff in the job summary. Per-PR signal, zero committed artifacts. The hard gate moves to release time (CLI-2233): diff the newdist/*.d.tsagainst the previously published tarball's in the human-approval step.@supabase/config/internalimports enforced apps/cli-only.Review rounds
Three internal reviews (engineer, architect, DX-as-consumer incl. a clean-Node tarball install exercising 23 checks) ran before this PR; all accepted findings are in the final commit. Explicitly rejected, for the record:
@deprecatedmarkers on./internalexports (strikethrough noise across apps/cli's own legitimate call sites; the no-semver contract is documented at the barrel, README, and AGENTS.md).messagegetters on the tagged error classes (would change CLI-visible error output pinned by normalize-error tests; README documents the structured-fields contract instead — candidate follow-up).api-report/.d.tsmirror (53 files + freshness test) existed in earlier commits of this branch and was removed by owner decision — per-PR accept semantics weren't worth 580 KB of generated diff noise. Replaced by the advisory base-vs-head compare above; an api-extractor-style rollup was also considered and skipped (TS7/tsgo compatibility unproven).saveCliConfig's atomic-write rename failure stays a defect (documented); re-channeling it as a typed failure is a behavioral follow-up.KONG_LOCAL_CA_CERTtopackages/stack(single legacy consumer today; speculative second move).Known collateral:
@supabase/pg-topoundercustomConditionscustomConditions: ["bun"]inapps/cli/tsconfig.json(needed so tsc typechecks@supabase/configagainst source instead of gitignoreddist/) also changes resolution for@supabase/pg-topo, whose ownbunexports condition points at unbuiltsrc/*.tscarrying 3 type errors at1.0.0-alpha.5. Worked around with a commentedpathspin to its shippeddist/index.d.ts.1.0.0-alpha.6is published but currently blocked by pnpm'sminimumReleaseAge; once it ages in, bump it inapps/cliand drop the pin (and thebun-condition source errors deserve an upstream fix insupabase/pg-toolbelteither way).Notes for CLI-2233 / CLI-2169 (publish)
dist/*.d.tsagainst the previously published tarball's and surface that diff in the human-approval step — that is the hard semver gate (the PR-time compare above is advisory only; first publish trivially has no compare target).pnpm publish(only pnpm rewrites any residualcatalog:; peers are now literal ranges regardless).effect@latestis still 3.x — README instructseffect@rc; revisit ranges when Effect 4 goes stable.main/types(deliberate ESM + exports-only; node10 resolution unsupported).@supabase/cli-go#lint:checkgosec findings fail localcheck:allon clean develop too — untouched by this PR.