feat(config): expose config defaults and provide sparse mapping functions - #6205
feat(config): expose config defaults and provide sparse mapping functions#6205kanadgupta wants to merge 9 commits into
Conversation
Adds ADR 0017 pinning the design for the config defaults mapping function ahead of implementation: a pure, parameterized subtract core in @supabase/config, defaults derived from schema annotations, strict deep equality, and the remote-block baseline rule. Also seeds a root CONTEXT.md glossary with the project's config vocabulary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ons (CLI-2155)
Adds getDefaultProjectConfig() (memoized decode of {} through the
schema, keeping annotations as the single source of truth for
defaults), subtractProjectConfig(config, baseline), and
omitDefaultValues(config), per ADR 0017. The generic subtraction walk
is extracted from io.ts's private stripDefaults so the save-time
minimal encoding and the new sparse API share one implementation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…y (CLI-2155) Introduces [remotes.*] blocks as config overrides for a specific persistent Supabase branch (project_id binds the block; the label is a user-chosen alias) in ADR 0017, CONTEXT.md, and the sparse.ts doc comments. Replaces the flat "not a valid complete config" framing with a split by baseline: subtracting the default config yields a valid config document that re-decodes to the same effective config under the current schema's defaults, while any other baseline yields an overlay meaningful only relative to that baseline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…I-2155) CONTEXT.md is not a convention this codebase follows. Its only content not already defined in the ADR (base config, drift) is woven into ADR 0018's problem statement; the stale post-renumbering references (the ADR's own 0017 heading, sparse.ts and sparse.unit.test.ts pointing at ADR 0017) now say 0018. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kanadgupta
changed the title
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@24fab701f36cf5b98d53e962671a28d85640b1fdPreview package for commit |
![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: 635c7dd81a
ℹ️ 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".
| const baselineObject = isObject(baseline) ? baseline : {}; | ||
| const result: Record<string, unknown> = {}; | ||
|
|
||
| for (const [key, child] of Object.entries(value)) { |
There was a problem hiding this comment.
When config omits an optional field that exists in baseline—for example, config = decodeProjectConfig({}) and baseline = decodeProjectConfig({ api: { external_url: "https://example.com" } })—this loop never visits the baseline-only key and returns an empty overlay. Consumers therefore cannot distinguish this removal from equality, and applying the advertised overlay to the baseline leaves the old value intact, potentially hiding configuration drift. The subtraction representation needs to preserve baseline-only removals or explicitly reject such inputs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining as out of scope for this API. The scenario (baseline has a key the config lacks) cannot arise in the designed call patterns: with the defaults baseline, decoding fills every defaulted field, and undefaulted optional fields are absent from the default config too; with the CLI-2156 baseline, the config is merge(base, remote) and merging only adds keys, so the config is always a key-superset of its baseline. Symmetric removed-key reporting is the diff core's concern (CLI-2156), not this mapping function's — CLI-2155 only asks to omit values matching defaults. See ADR 0018 (docs/adr/0018-sparse-config-subtraction.md) for the scope split.
There was a problem hiding this comment.
One correction to my reasoning above: in the CLI-2156 remote-block call the config is not always a key-superset of its baseline — a base-only undefaulted field like api.external_url is baseline-only. The decline still holds, for a better reason: in that flow absence means inherit, so an empty overlay for a baseline-only key is the correct answer — reporting it as a removal would fabricate drift. Documented the asymmetric contract in subtractValue's docstring in 24fab70; symmetric removed-key reporting remains the diff core's concern (CLI-2156).
| const subtracted = subtractValue(child, baselineObject[key]); | ||
|
|
||
| if (subtracted !== undefined) { | ||
| result[key] = subtracted; |
There was a problem hiding this comment.
For a valid record key named __proto__—accepted for remote labels and function names—this assignment invokes the legacy prototype setter because result is a normal object rather than creating an own property. The entry consequently disappears from Object.keys(result) and from the sparse output; if it is the only entry, the entire section is dropped. Construct the result with a null prototype or define keys as own data properties.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining. Reaching this line with an own __proto__ key requires the TOML/JSON parse and schema decode to have produced one through the same setter trap first, and record entries (remotes, functions) pass through wholesale without a rebuild when absent from the baseline. Plain-object construction with direct assignment is also the established idiom in this package (e.g. mergeRemoteSubtree in io.ts) — hardening one function against a config author's own file while its siblings share the pattern buys nothing. If we ever harden, it should be package-wide in one pass.
There was a problem hiding this comment.
Reversing my decline — I tested the premise and it's wrong. smol-toml parses [functions.__proto__] into an own key on a normal-prototype object, JSON.parse does the same, and the schema decode preserves both, so the key reaches this walk intact: result[key] = … silently dropped it, and baseline[key] read Object.prototype off the prototype chain. Fixed in 24fab70 by hardening the package's record walks in one pass (subtractValue, mergeRemoteSubtree, stripFunctionRecordDefaults): Object.hasOwn lookups and own-data-property construction via a shared setOwnProperty, with a unit test pinning the __proto__ round-trip.
| */ | ||
| export function getDefaultProjectConfig(): ProjectConfig { | ||
| defaultProjectConfig ??= decodeProjectConfig({}); | ||
| return defaultProjectConfig; |
There was a problem hiding this comment.
When a consumer mutates the object returned here—possible from JavaScript or even type-checked code via APIs such as Object.assign—it permanently changes the module-wide subtraction baseline. Subsequent omitDefaultValues calls can then silently remove real overrides or retain actual defaults, and a later dynamic import of io.ts can derive its write baseline from the poisoned object. Return a clone or deeply freeze the cached value before exposing it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a717495: the memo is now deeply frozen before it is shared, so mutation attempts throw at runtime instead of poisoning the module-wide subtraction baseline. A unit test pins the frozen invariant.
| config: ProjectConfig, | ||
| baseline: ProjectConfig, |
There was a problem hiding this comment.
When the documented CLI-2156/CLI-2064 flow passes config.remotes[name] as config, TypeScript rejects the call because a decoded remote block has the root sections but no required remotes property, while this parameter requires a complete ProjectConfig. The API therefore cannot be used for its advertised remote-block subtraction without a cast or an adapter that fabricates a remotes field; model the accepted input around the shared root/remote shape instead.
AGENTS.md reference: AGENTS.md:L75-L75
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a717495: subtractProjectConfig/omitDefaultValues now accept BaseProjectConfig (Omit<ProjectConfig, "remotes">), which both full configs and decoded remote blocks satisfy structurally — the ADR 0018 remote-block call now type-checks without a cast, and a unit test pins that exact call shape.
…defaults memo (CLI-2155) Review follow-ups on #6205: subtractProjectConfig/omitDefaultValues now take BaseProjectConfig (ProjectConfig without the nested remotes), so the ADR 0018 remote-block call type-checks without a cast; the memoized default config is deeply frozen so a caller mutation cannot poison the shared subtraction baseline. Both pinned by unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
||
| - Fields declared `optionalKey` without a `default` annotation can never be pruned; if a platform default exists for such a field, it must be added to the schema before subtraction can see it. | ||
| - A user's explicitly-written default value (`max_rows = 1000` typed by hand) is indistinguishable from an omitted one and will be pruned; intent is not preserved. | ||
| - Callers must know the remote-block baseline rule; misusing the default config as a remote block's baseline reintroduces the override-erasure bug this ADR exists to prevent. |
There was a problem hiding this comment.
One more Negative worth recording: omitDefaultValues output is not sparse inside record-keyed entries. Decoding fills per-entry defaults, so functions.hello = { verify_jwt = false } comes back with enabled: true, import_map: "", entrypoint: "", static_files: [], env: {} and a decoded [remotes.x] block is a complete effective config (every section carries withDecodingDefault). The default config has functions: {} / remotes: {}, so subtraction keeps those entries whole, materialized defaults and all.
This cancels out in the diff (both sides subtract against each other), but a consumer rendering omitDefaultValues(localConfig) directly gets very non-sparse output, io.ts grew stripFunctionRecordDefaults for this on the encoded write path previously, and the typed API has no counterpart.
Fine to defer entry-level subtraction to the consumer that needs it, just might need to document the trap here so CLI-2156/2064 don't rediscover it.
There was a problem hiding this comment.
Good catch — reproduced exactly as described (functions.hello = { verify_jwt: false } comes back with all five materialized defaults, and a [remotes.*] entry survives as a 12-section effective config). Documented in 24fab70: the omitDefaultValues docstring and ADR 0018's Negatives now state that record-keyed entries survive whole with per-entry decoding defaults materialized, why (the default config's empty functions/remotes records offer no per-entry baseline), and that entry-level subtraction is deliberately the consumer's job — for remotes necessarily so, since the correct baseline is the merged base config. Left a pointer to stripFunctionRecordDefaults as the encoded-path precedent so CLI-2156/2064 find it instead of rediscovering the trap; if a consumer ends up rendering sparse output directly, that's the moment to unify the two into one typed entry-level strip.
| * Memoized (and the memo shared with callers) rather than computed at module | ||
| * load, so importing the package doesn't pay for a full schema decode. The | ||
| * memo is deeply frozen before it is shared: it doubles as the module-wide |
There was a problem hiding this comment.
Not sure of the claim in practice: io.ts:151 computes defaultEncodedProjectConfig at module load, and index.ts re-exports io.ts, so importing @supabase/config still pays the decode+encode at import time (pre-existing, unchanged by this PR).
Maybe soften to "so this module doesn't pay for a decode at load" or make io's constant lazy too, the memo itself is still worth having either way.
There was a problem hiding this comment.
Right — io.ts:151 was paying the decode+encode at import time anyway (and eagerly populating this memo through getDefaultProjectConfig()), so the claim didn't hold for anyone importing the package root; measured at ~14ms per import. Went with your second option in 24fab70: io's defaultEncodedProjectConfig is now a memoized function too (only the save path consumes it), so the comment here is accurate rather than softened — importing @supabase/config pays for no schema decode until something actually needs the defaults.
…e encoded defaults (CLI-2155) Review follow-ups on #6205: - A record entry named `__proto__` (a valid function name or remote label — both smol-toml and JSON.parse produce it as an own key, and the schema decode preserves it) was silently dropped by `result[key] = value` hitting the legacy prototype setter, and `baseline[key]` read `Object.prototype` off the prototype chain. All three record walks (`subtractValue`, `mergeRemoteSubtree`, `stripFunctionRecordDefaults`) now use `Object.hasOwn` lookups and define own data properties via a shared `setOwnProperty`; a unit test pins the `__proto__` round-trip. - `io.ts`'s encoded-defaults constant is now memoized like `getDefaultProjectConfig`, so importing `@supabase/config` no longer pays a schema decode+encode at module load — making sparse.ts's lazy-memo comment actually true. - Documented that `omitDefaultValues` output is sparse at the root scope only (record-keyed entries carry materialized per-entry decoding defaults), in the docstring and ADR 0018's Negatives, and that baseline-only keys are ignored by design (absence means inherit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24fab701f3
ℹ️ 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".
| export function subtractProjectConfig( | ||
| config: BaseProjectConfig, | ||
| baseline: BaseProjectConfig, | ||
| ): SparseProjectConfig; |
There was a problem hiding this comment.
When the base overrides any field that the remote block omits, accepting a decoded remote block here produces a semantically incorrect overlay. remoteProjectConfig in base.ts applies decoding defaults to every section, so with a base db.port = 54399 and a remote that only overrides api.max_rows, the decoded remote contains the global default db.port = 54322; subtraction retains that value and a subsequent pull/write changes the branch from inheriting 54399 to explicitly using 54322. Fresh evidence after the earlier type-shape fix is the new test at sparse.unit.test.ts:139-153, which only covers a base override also explicitly present in the remote and therefore misses this case. Require the merged effective remote configuration here (or preserve the raw sparse block) rather than advertising decoded config.remotes[name] as a valid operand.
Useful? React with 👍 / 👎.
2 participants
Implements CLI-2155: a stored reference of config default values and a mapping function that omits values matching them, so
config diff(CLI-2156) andconfig pull(CLI-2064) can compare sparse configs instead of full effective ones.What changed
New API in
@supabase/config(packages/config/src/sparse.ts, all pure and synchronous, operating on decodedProjectConfigvalues):getDefaultProjectConfig()— the default config, derived by decoding{}throughProjectConfigSchema(memoized). The schema's existingdefaultannotations and decoding defaults remain the single source of truth; there is no hand-maintained defaults table to drift.subtractProjectConfig(config, baseline)— returns the sparse configconfig − baseline: strict deep equality (order-sensitive arrays), sections emptied by subtraction dropped recursively. Directional, so CLI-2156 can subtract a remote block against the merged base config.omitDefaultValues(config)—subtractProjectConfigwith the default config as baseline. The result is itself a valid config document: re-decoding refills the removed defaults, yielding the same effective config under the current schema's defaults.[remotes.*]blocks (config overrides for a specific persistent Supabase branch, bound byproject_id) pass through untouched — pruning them against global defaults would silently change what a branch resolves to.Refactor:
io.tshad a privatestripDefaults/isEqualValuewalk used for writing minimal config files; that walk is now the sharedsubtractValuecore insparse.ts, consumed by both the new API and the save path, so there is one subtraction implementation in the package.Design docs: ADR 0018 (
docs/adr/0018-sparse-config-subtraction.md) records the decision, the remote-block baseline rule, what the output is depending on the baseline (defaults → a valid, effectively-equivalent config document whose meaning leans on the current defaults version; any other baseline → an overlay meaningful only relative to that baseline), and defines the working vocabulary inline (default config, sparse config, subtract, base config, remote block, drift).Reviewer notes
packages/apitypes. See the ticket comment and ADR 0018's Alternatives.subtractProjectConfiguses an overload with an untyped implementation signature rather than anascast — TypeScript can't verify that a structural walk overunknownreconstructs aDeepPartialof its input; the unit tests pin the contract.🤖 Generated with Claude Code