feat(cli): add config diff command

[kanadgupta]

Implements CLI-2156: a read-only supabase config diff that classifies drift between supabase/config.toml and the effective configuration GET /v2/projects/{ref}/config reports for a target project or branch. Never writes. Builds on CLI-2155's sparse subtraction/defaults (#6205) and consumes CLI-2230's ProjectConfig convergence normalizers (#6339) as its comparison operands.

What changed

packages/config — the comparison core (ADR 0022)

• config-diff.ts: a pure classifier producing a typed ConfigChangeSet (update / remote_only / local_only, plus masked and per-class counts), reusable by config pull without the command layer.
• Both operands are CLI-2230 convergence projections (ADR 0021): fromConfigDocument({config, document}) locally (raw-presence-masked, canonicalized, secret-omitting) and fromApiProjectConfig(response) remotely. All wire knowledge — renames, boolean inversions, duration/byte-size conversions, the GoTrue key table — lives in the shared projectConfigMappingRows registry, so the managed surface is isComparableProjectConfigPath by construction: a path with no registry row ([studio], ports, image pins, [realtime] locals, workers) can never be reported.
• Classification is driven by the raw document's declared-key set — the one signal a decoded config cannot recover — so "the file wrote the default" and "the file is silent" classify differently (update vs suppressed/remote_only).
• remote_only suppression baseline: the default config's own convergence projection, falling back to the raw schema default for push-gated containers (network restrictions' allow-all default is exactly the platform's unconfigured state), then to the type's zero value. An untouched project diffs clean.
• Secrets (the registry's isSecret rows) are "present, unknown": both normalizers omit them, they never classify or count, and locally-declared ones surface via masked so a clean diff is visibly a partial claim.
• Residual equality is meaning-based: multiset array comparison (additional_redirect_urls order is not drift) and string/number, string/boolean scalar tolerance.
• io.ts/lib/env.ts: value origins now record the resolving env-var name, so a change on an env()-fed property names the variable.

apps/cli — legacy-shell command

• legacy/commands/config/diff/: command + handler + errors + SIDE_EFFECTS.md. Target resolution: --target <branch-name|uuid|ref> (same acceptance as link; 404 → "run supabase branches list"), --project-ref, else the linked ref; --target + --project-ref together is a hard error. When the resolved ref matches a [remotes.*] block's project_id, the local operand is the branch's merged effective config (ADR 0018), otherwise the base config — the echoed line always says which.
• Output: text (unset renders (unset) / (not returned); (from env VAR) annotations; masked note) and --output-format json|stream-json (structured payload with schema_version, target, scope, changes[], masked[], counts). The comparison-scope line lists which response blocks were carried. The Go-compat -o/--output flag is rejected outright (every value, pretty included) with an error pointing at --output-format — per the ticket-thread decision that net-new commands carry no Go parity contract. --exit-code sets exit 1 on drift via ProcessControl.setExitCode after the payload is emitted.
• legacy/shared/legacy-branch-ref.resolver.ts: the branch name/UUID/ref resolver hoisted out of the branches family (cross-family use) with injected error mappers; the branches family keeps a thin binding so its call sites are unchanged.

Docs: ADR 0022 (classification + managed surface, incl. the registry consolidation and its relationship to ADR 0019/0020/0021), go-cli-divergences.md TS-only command entry (replacing the ticket's stale go-cli-porting-status.md criterion), per-command SIDE_EFFECTS.md.

History note for reviewers

The branch was first implemented with a self-contained translation table (a ~900-line port of the Go CLI's FromRemoteAuthConfig). After #6339 landed the registry-driven normalizers on develop — with this command as their named consumer — the merge commit (766182f) brought develop in and the follow-up (cae9c14) deleted the tables and rebuilt the classifier on the registry, per the repo's no-parallel-code-paths policy. ADR 0022's "Considered Alternatives" records both designs.

Decisions & assumptions worth reviewing

1. Managed surface = the shared registry (vs schema annotations or response-key walking): single source of truth shared with Studio and the future push mapper; a missing row fails safe (silently unmanaged, never misreported). Alternatives in ADR 0022.
2. Suppression baseline chain (default projection → raw schema default → zero value) is a judgment call: without it, every unconfigured OAuth provider and the platform's allow-all network restrictions read as drift on untouched projects. The cost is that a remote override that happens to equal a schema default on an undeclared path is not reported.
3. schema_version in the payload is the file's $schema ref (falling back to the current schema URL) — CLI-2155 shipped no separate version token.
4. Rendered "local" values are convergence projections (ADR 0021), i.e. what pushing the file would produce hosted — canonicalized durations/byte sizes, push-gated omissions — not necessarily the file's literal spelling. Documented in SIDE_EFFECTS.md.
5. Legacy -o support was implemented per the original acceptance criteria, then removed after Colum confirmed on the ticket that parity isn't a goal for net-new commands. The flag now fails fast with a bespoke invalid-input error; the JSON payload always carries explicit null for unset sides.
6. Masked credentials are transparent: listed in masked[] / a text note rather than silently skipped, and never affect --exit-code.
7. Partial responses degrade, never error: comparable paths the response doesn't carry are local_only when declared locally, silent otherwise; the scope line calls out missing blocks. (Today's v2 schema requires all six blocks, so this is belt-and-braces for API evolution and permission-trimmed keys.)
8. The classifier inherits ADR 0021's limits: unconditionally-mapped fields with no local-silence signal can surface as honest-but-push-unactionable remote_only entries (tracked on CLI-2266).

🤖 Generated with Claude Code

[kanadgupta]

Merged develop in (merge commit, no rebase) and consolidated the comparison core onto CLI-2230's registry:

• Merge commit (766182f): conflict resolution + mechanical adaptation — our ADR renumbered 0019→0022 (develop took 0019–0021), the env-var-name threading re-applied to the relocated CliConfigValueOrigin, CliConfig/entrypoint-split renames, the live test rewritten for the fixture-based live harness, and the v2 test fixture gaining the new required database.major_version.
• Consolidation (cae9c14): feat(config): add toProjectConfig and the ProjectConfig hosted subset (CLI-2230) #6339 built fromConfigDocument/fromApiProjectConfig explicitly as this diff's operands (ADR 0021), so the branch's own ~900-line translation tables (config-diff.{managed,auth,read}.ts) are deleted and diffProjectConfig now compares the two convergence projections over isComparableProjectConfigPath, with masked secrets from the registry's isSecret rows and remote_only suppression baselined on the default config's projection (falling back to raw schema defaults for push-gated containers, e.g. network restrictions' allow-all). Classification semantics (declared-set-driven update/remote_only/local_only, masked note, env-var naming, --exit-code) are unchanged. ADR 0022 records the consolidation and its relationship to ADR 0019/0020/0021.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cae9c14a97

ℹ️ 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".

[chatgpt-codex-connector]

Honor the legacy output flag instead of rejecting it

When any global -o/--output value is supplied—including pretty—the handler exits before performing the diff. Legacy handlers are required to honor both output mechanisms, with --output taking priority, so scripted invocations such as config diff -o json unexpectedly fail instead of producing the requested payload. Implement the legacy encodings rather than rejecting the flag.

AGENTS.md reference: apps/cli/AGENTS.md:L300-L300

Useful? React with 👍 / 👎.

[kanadgupta]

Done in 04a174a — -o json|yaml|toml|env now encode the structured payload via the shared encoders with --output taking priority, and pretty falls through to --output-format, per Invariant #6 and the backups/list pattern.

[chatgpt-codex-connector]

Flush telemetry for pre-resolution failures

The finalizers only wrap the inner effect after ref has been resolved, so unsupported-output errors, flag conflicts, missing-link failures, and branch-resolution failures all bypass telemetryState.flush. Those are normal command invocations and failures, and omitting the flush violates the legacy lifecycle invariant and leaves telemetry state incomplete; wrap the full invocation with the telemetry finalizer while keeping the ref-dependent cache finalizer appropriately scoped.

AGENTS.md reference: apps/cli/AGENTS.md:L290-L290

Useful? React with 👍 / 👎.

[kanadgupta]

Done in fba78b7 — Effect.ensuring(telemetryState.flush) wraps the full handler body (config load and branch resolution included), with the ref-dependent cache finalizer gated on a resolved ref. Failure-path assertions added in 977e300.

[chatgpt-codex-connector]

Resolve UUID targets without requiring a linked project

For a UUID --target, resolver.resolve(Option.none()) runs before legacyResolveBranchProjectRef, even though the UUID lookup endpoint does not use a parent ref. Consequently, in an unlinked non-interactive directory, config diff --target <uuid> fails with LegacyProjectNotLinkedError without making the documented /v1/branches/{uuid} request; only branch-name targets should require the parent project ref.

Useful? React with 👍 / 👎.

[kanadgupta]

Done in fba78b7 — the parent ref is passed to the branch resolver lazily and evaluated only for branch-NAME lookups, so a UUID --project-ref resolves through GET /v1/branches/{id} in an unlinked directory. Pinned by an integration test with projectId: Option.none().

[chatgpt-codex-connector]

Remove the entry from the frozen divergence record

This adds config diff to go-cli-divergences.md, but that file is explicitly frozen and new CLI behavior must instead be documented through help text, tests, and SIDE_EFFECTS.md. Remove this row so the historical record does not keep accumulating current feature documentation.

AGENTS.md reference: apps/cli/AGENTS.md:L539-L542

Useful? React with 👍 / 👎.

[kanadgupta]

Done in 04a174a — the file is restored to develop's version (row and reflow both gone).

[chatgpt-codex-connector]

Describe the output behavior without Go-parity framing

This new comment defines the command's behavior by saying it has no Go parity contract, and the same framing is repeated in the new error and side-effect documentation. New legacy work must describe behavior on its own terms rather than use the removed Go implementation as the compatibility baseline; rephrase this around the supported output flags themselves.

AGENTS.md reference: apps/cli/AGENTS.md:L56-L58

Useful? React with 👍 / 👎.

[kanadgupta]

Done — the framing disappeared with the code that carried it: the -o rejection comment/error (04a174a) and the SIDE_EFFECTS "no Go CLI equivalent" line. The command's behavior is now described in its own terms throughout.

[chatgpt-codex-connector]

Validate the local config before resolving remote branches

When --target is a branch name or UUID, target resolution can make a Management API request before this load runs. Therefore a missing or malformed local config does not abort before network activity as promised by LegacyConfigDiffLoadConfigError and SIDE_EFFECTS.md; for example, config diff --target staging can contact /v1/projects/.../branches/staging before reporting the TOML parse failure. Parse and validate the local document before performing branch lookup, then apply the target-specific remote overlay once the ref is known.

Useful? React with 👍 / 👎.

[kanadgupta]

Done in fba78b7 — the config is loaded and validated before any network call, with the [remotes.*] overlay applied by a re-load once the resolved ref is known (only configs that declare remotes reload). A malformed TOML with a branch target now pins zero API requests in the integration suite.

[chatgpt-codex-connector]

Show progress while resolving branch targets

For branch-name and UUID targets, legacyResolveBranchProjectRef performs a Management API request without an output.task, so text-mode users receive no progress indication while that network request is pending. Wrap this lookup in a task and fail or clear it on every exit, as is already done for the subsequent project-config fetch.

AGENTS.md reference: apps/cli/AGENTS.md:L425-L427

Useful? React with 👍 / 👎.

[kanadgupta]

Done in fba78b7 — branch resolution runs under output.task("Resolving branch...") with fail/clear on every exit, matching the config fetch.

[github-actions]

Supabase CLI preview

npx --yes https://pkg.pr.new/supabase/cli/supabase@b02b1948733b4ab69fdb90caf0d2646305837e3a

Preview package for commit b02b194.

[Coly010]

Ran an adversarial review of this PR: four independent passes (implementation, architecture, security, DX) plus live runs from source against staging (api.supabase.green) with a real linked project. Every finding was reproduced by running code, not read off the diff.

Verdict: request changes. The design underneath — the registry-driven managed surface, the secret triple-gating, convergence-projection operands — held up under genuinely hostile probing (16 secret-leak probes across all output modes failed; unknown future secret API fields fail safe; read-only is proven by mtime+content pinning; machine stdout stays payload-pure; the branch-resolver hoist is behaviour-identical with all 88 branches tests green). The blockers are concentrated in the classifier's equality/suppression edges, the flag wiring, and documented claims the code doesn't keep.

Blockers (all inline)

1. --exit-code is accidentally required → plain supabase config diff (the help's own first example) errors out. One-line fix; needs a parser-level test.
2. Order-insensitive array equality false-negatives on api.schemas / api.extra_search_path → --exit-code exits 0 on real drift.
3. The remote_only suppression baseline misses canonicalized zeros and platform-default subjects → untouched projects report drift. Live on staging: 15 of 18 remote-only entries on a near-default project were this noise ("0s" session values + 13 template/notification subjects + 3 storage defaults).
4. False clean: a declared auth.oauth_server.enabled disagreeing with the remote prints "No config differences found."

Confirmed live against staging

Beyond blocker 1, the command hard-fails on staging today: SchemaError(Missing key at ["data"]["attributes"]["storage"]["database_pool_mode"]) — the generated contract requires every block key, so the documented "partial responses degrade, never error" behaviour is unreachable (inline on SIDE_EFFECTS.md). I only obtained a successful run by locally patching the contract to make that key optional. With that patch, the happy path works well end-to-end: 29 classified changes, correct counts, (from env VAR) annotations, masked-credentials note, machine payload and exit codes all as designed.

Majors (all inline)

• remote_only erases the local value it just compared — the output can't answer "what would config push change?".
• --workdir is silently ignored (config push shares the bug).
• Telemetry flush + linked-project cache skipped on every pre-resolution failure path (Legacy Shell Invariant #1).
• ANSI/control-character injection via unsanitized path segments and names in text output (legacySanitizeInlineName exists for exactly this and is used 14 lines away).
• --exit-code conflates drift with failure (both exit 1).
• JSON schema_version is the user's $schema URL, not a payload contract version.
• Response-decode failures mislabeled as network errors, dropping the upstream suggestion and bypassing the purpose-built actionability adapter.
• Dotted-path round-tripping silently drops record keys containing ..

Smaller items not carried by an inline thread

• DiffProjectConfigOptions asks for local and declared separately — two params that must come from the same load, with nothing enforcing it. Consider accepting the loaded pair and deriving both.
• counts is derived state computed in three places (config-diff.ts, both formatters + changes.length in the handler); one will drift. Either drop it from the package type or make total part of it and use it everywhere.
• No docs-site overlay (config push has docs/supabase/config/push.md), so the published reference page for a semantically subtle command falls back to one sentence — nothing on --exit-code, (unset) vs (not returned), masking, or the fact that rendered local values are convergence projections (a user who writes "1m" and sees "1m0s" will grep their file and file an issue; worth a one-line note in the output or docs).
• One concept, three spellings: [remote only] / remote-only / remote_only across label, summary, and JSON. And N difference(s) where the count is known at render time.
• --target <uuid> is echoed as a quoted display name (Comparing against '1111…-…'); the branch's actual name is never shown.
• SIDE_EFFECTS.md "config.toml is read before any network call" is false for --target <branch> — branch resolution runs first, so a broken TOML burns an API round-trip, and in a fresh directory the "run supabase link" error wins over the friendlier "run supabase init" one.
• JSON scope lists only present blocks; consumers must re-derive the missing set from a hardcoded list — consider scope: {present, missing}.
• ADR 0022 ships as proposed (README row too) while its body says it "was first accepted", and it's silent on three shipped decisions: --target, the -o rejection, and the remote_only local-value nulling.

Test-suite structure (why the suite missed the blockers)

The parser is never exercised (blocker 1); the auth: {} fixture means the largest, most transform-heavy mapping surface never runs end-to-end (blocker 3); the live test asserts only exit 0 where its own comment says cleanliness is the point; and no test asserts a secret string is absent from output. Details inline on the fixture.

What's genuinely good here

The (from env VAR) annotation, the (unset)/(not returned)/null distinctions, byte sizes rendered in the user's units, masked secrets surfaced-but-never-counted, the read-only proof in tests, and the registry consolidation over a parallel translation table are all exactly right. This is close to a really good command — it just can't currently be invoked, and each of its two core promises (no false drift, no false clean) has a reproduced counterexample.

[Coly010]

Blocker: Flag.boolean("exit-code") without Flag.withDefault(false) makes this a required flag, so the command cannot be invoked as documented:

$ supabase config diff
Error: required flag(s) "exit-code" not set

That's the exact invocation in the EXAMPLES block below and in the generated docs spec (which even emits default_value: "false"). The integration suite can't catch this because every test hands a pre-built flags object to the handler and never goes through the parser — please ship a parser-level test with the one-line fix (e.g. an e2e assertion that config diff with no args doesn't emit required flag(s)), or the next boolean flag will regress the same way.

[kanadgupta]

Fixed in 04a174a: Flag.withDefault(false), and — since you're right that the integration suite structurally can't catch this — a new diff.e2e.test.ts pins the parser at the subprocess boundary (config diff with no args must not emit required flag(s)), so the next boolean flag can't regress the same way.

[Coly010]

Blocker: comparing every array as a multiset false-negatives on order-significant config. api.schemas (PostgREST's default schema is the first entry) and api.extra_search_path (a literal search_path, where order is resolution order) are sequences, not sets. Reproduced: local ["public","extensions"] vs remote db_extra_search_path: "extensions,public" → changes: [], so --exit-code exits 0 on a difference that changes runtime behaviour — in a drift detector.

"Is this array a set or a sequence?" is per-field wire semantics, i.e. registry knowledge: suggest a row property (arrayEquality: "set" | "sequence") on projectConfigMappingRows, defaulting to sequence (over-report rather than under-report), with additional_redirect_urls opting into set semantics.

[kanadgupta]

Adopted your design verbatim in 3f23ca8: arrayEquality: "set" | "sequence" on projectConfigMappingRows, defaulting to sequence (over-report), with additional_redirect_urls opting into set semantics. Your exact repro — local ["public","extensions"] vs db_extra_search_path: "extensions,public" — is now a pinned unit test (both api.schemas and api.extra_search_path), and isEqualConfigValue's exported default flipped to sequence with the mode as an explicit parameter.

[Coly010]

Blocker: this fallback tests JS zeros, but the operand is already canonicalized by the registry — GoTrue's sessions_timebox: 0 arrives here as the string "0s" and escapes the check. Confirmed live against staging: a near-default project reports auth.sessions.timebox / auth.sessions.inactivity_timeout as [remote only] remote: "0s", plus 13 more noise lines for auth.email.template.*.subject / notification.*.subject (platform-reported defaults with no baseline in the default CLI config; the three storage.* entries look like the same class). On the staging bench project, 15 of the 18 remote_only entries were this noise — an untouched project fails --exit-code, which is exactly the flooding ADR 0022 says the baseline prevents.

Suggest not inferring "unconfigured" from JS zeros at all: put the platform's unconfigured value on the registry row (e.g. unconfiguredValue), and add a registry-driven test enumerating every comparable path whose baseline is undefined, asserting its zero form suppresses.

[kanadgupta]

Adopted in 3f23ca8: zero inference is gone entirely — the suppression baseline is now default-projection → raw-default → the row's declared unconfiguredValue, and a path with no baseline at any tier reports. Rows declare: "0s" for both session bounds, the 13 provisioning-default subject strings, and false for the 7 notification toggles. For provenance I didn't guess the subjects: they're pinned byte-for-byte from the recorded real responses already in this repo (apps/cli-e2e/fixtures/recorded/GET_v1_projects___PROJECT_REF___config_auth/), and the notification default comes from supabase/auth's NotificationsConfiguration (default:"false"). The registry-driven test you asked for walks every comparable path whose config-side baseline is undefined: rows with unconfiguredValue must classify their unconfigured report clean, and absence-class rows must REPORT a zero-form value rather than swallow it.

[Coly010]

Blocker (false clean): applyPushUnmanagedOmissions unconditionally drops auth.oauth_server from the local projection, but auth.oauth_server.enabled is a comparable registry path the API reports. Reproduced end-to-end: file declares [auth.oauth_server] enabled = true, remote reports oauth_server_enabled: false → local is silent, remote equals the raw default, suppressed here — and the command prints No config differences found. A declared local value that genuinely disagrees with the remote vanishes with no change entry, no masked note, no signal.

Suggestion (additive): a third bucket alongside masked — unmanaged: ReadonlyArray<string> for declared paths the local projection dropped — surfaced like the masked note ("N declared property(ies) cannot be pushed and were not compared: …"). The classifier already has both inputs it needs (declared + local silence).

Related, same line: the ?? valueAtPath(getDefaultCliConfig(), path) fallback is hard-coded, so the defaults option (which no call site currently passes) only controls one of the three baseline tiers. Either make the option the complete baseline or delete it.

[kanadgupta]

Both adopted in 3f23ca8. ConfigChangeSet.unmanaged is the third bucket: declared comparable paths the local projection dropped (your auth.oauth_server.enabled repro, sentinel-pruned siblings, unselected SMS providers), surfaced like the masked note — text prints Note: 1 declared property cannot be pushed and was not compared: auth.oauth_server.enabled, and the machine message carries the caveat too. Your exact end-to-end repro is a pinned test. The defaults option is deleted — with no call site and only one of three tiers under its control it was dead optionality, and the tiering is now fully internal.

[Coly010]

Major: on an undeclared path the classifier had the local effective value (the materialized default it just compared) and throws it away. The user sees:

api.max_rows [remote only]
  local:  (unset)
  remote: 250

…when the local effective value is 1000 and a config push would overwrite the remote 250 with it. That's the "someone changed it in the dashboard" case — the primary reason this command exists — and [remote only] reads as "key exists only remotely", which is false for anything with a schema default. It also collapses two states the future config pull consumer needs distinguished (file-silent vs materialized-default-disagrees).

Suggestion: keep local populated and add readonly declared: boolean to ConfigChange (already computed above); render local: 1000 (schema default — not declared in config.toml).

[kanadgupta]

Adopted in 3f23ca8: local stays populated with the materialized default and ConfigChange carries declared: boolean; text renders local: 1000 (schema default — not declared in config.toml) (pinned integration test with your exact max_rows: 250 scenario), and the JSON payload carries declared per change so the future config pull consumer gets the file-silent vs default-disagrees distinction.

[Coly010]

safeFlags: ["project-ref"] logs the ref verbatim while config push (same family, same flag) redacts it, and the established safe list in apps/cli/CLAUDE.md doesn't include the config family. Telemetry drift is silent and breaks dashboards — either drop this or add it to config push and extend the documented list in the same change.

[kanadgupta]

Fixed in fba78b7, taking your "add it to config push and extend the documented list" option — with one wrinkle the flag reshape introduced: diff's --project-ref now accepts branch names, so it uses link's ref-shape-gated guard (verbatim only when PROJECT_REF_PATTERN matches; a user-created branch name never reaches PostHog). Push's ref-only flag logs verbatim unconditionally. The documented safe list in apps/cli/CLAUDE.md now names the config family and the branch-accepting guard rule.

[Coly010]

/v2/projects/{ref}/config names a user-selected resource, so a 404 means "wrong project ref" — user-actionable. Without { notFoundIsInvalidInput: true } (which LegacyConfigDiffBranchResolveStatusError above and all 11 push.errors.ts status errors on ref-addressed routes pass), this classifies a 404 as an external-service problem and skews the KPI split.

[kanadgupta]

Fixed in 27b62ef: LegacyConfigDiffReadStatusError passes { notFoundIsInvalidInput: true }, matching the branch-resolve error and the ref-addressed push.errors.ts convention, with a comment stating the rule.

[Coly010]

This claim (and ADR 0022's "partially-populated responses degrade … instead of an error") doesn't hold: V2GetProjectConfigOutput makes all six blocks — and their keys — required, so a missing block or key fails the typed decode inside the API client before any of this leniency runs. Confirmed live: staging doesn't return storage.database_pool_mode yet, and the command hard-fails on every invocation with

failed to read project config: SchemaError(Missing key
  at ["data"]["attributes"]["storage"]["database_pool_mode"])

— i.e. the command is currently broken against staging, and a permission-truncated response (the case the ADR names) surfaces as an opaque SchemaError. Consequences: the scope line's "(not returned: …)" branch is unreachable in production (it prints the constant six-block list on every run), and two diff.format.unit.test.ts cases exercise unreachable states.

Pick one: loosen the contract (make blocks/keys optional, matching auth's leniency — the stated intent) and keep the scope machinery, or delete the scope machinery and correct this doc + ADR 0022. Don't leave the doc asserting behaviour the contract forbids.

[kanadgupta]

Took the first option in a32ada7: the contract is loosened rather than the scope machinery deleted. All 13 object-level required arrays under data.attributes (the six blocks and their nested containers) are relaxed through the established openapi-overrides.json test+replace mechanism — the same pattern as the SAML attribute_mapping and custom-hostname entries — so your staging failure shape (missing storage.database_pool_mode) and permission-truncated blocks decode and degrade as ADR 0022 intended. @supabase/config's lenient mirror was already optional-everywhere, so only the generated side moved. A new packages/api client test pins the exact staging shape decoding, and the two formerly-unreachable scope-line states are now real (and empty blocks count as not-returned — see the scope thread). The envelope (data/type/id/attributes) and array-item shapes stay strict: a partial response omits fields, not halves of array elements.

[Coly010]

apps/cli/CLAUDE.md declares this file "a frozen historical record … Do not add new entries — new flags and features are simply new CLI behavior." This row was flagged independently by every review pass. Suggest dropping it (the -o rationale already lives in SIDE_EFFECTS.md and the error text; ADR 0022 is the right home for the decision), which also undoes the table reflow that turned a 1-line change into a 15-line diff. SIDE_EFFECTS.md:9 points here too and should stop.

[kanadgupta]

Dropped in 04a174a by restoring the file to develop's version outright, which also undoes the table reflow. The -o rationale is moot now that the flag is honored (see that thread); the surviving decisions live in SIDE_EFFECTS.md and ADR 0022 (now accepted, a4dc035).

[Coly010]

This auth: {} is the structural blind spot that let both classifier blockers through: v2 attributes.auth is an open record, so an empty object is schema-valid — meaning the entire GoTrue mapping surface (~200 keys; all the duration/inversion/sentinel logic) is never exercised end-to-end, and the fixture docstring's "an empty config.toml diffs clean" claim is only true because auth is empty.

Highest-value test additions:

1. A realistic all-defaults auth block in this fixture, asserting a fresh config diffs clean (this is where the "0s" suppression bug lives).
2. The same cleanliness assertion in diff.live.test.ts — its own comment names "the GoTrue-keyed auth record … classifying cleanly" as the one thing mocks can't prove, then asserts only exitCode === 0.
3. A not.toContain(<secret>) assertion on the masking scenario (it already seeds GITHUB_SECRET=shh and an HMAC-shaped remote value without asserting absence), so the "secrets never leak" claim is pinned against formatter changes.
4. Telemetry-flush assertions on the -o, flag-conflict, and branch-404 paths (currently they'd fail).

[kanadgupta]

All four adopted in 977e300 (with the fixture landing on the classifier fixes from 3f23ca8):

1. The shared fixture now carries a ~65-key realistic fresh-project auth record at platform defaults — sessions zeros, all 13 subjects, notification toggles, durations, inversions — and the existing clean-config test proves the whole surface classifies cleanly against an empty config.toml.
2. diff.live.test.ts asserts no auth. change lines on a fresh project, not just exit 0.
3. The masking scenario now asserts not.toContain for both the resolved plaintext and an HMAC-shaped remote digest, on both streams, in text and JSON modes (including the payload serialization).
4. Telemetry-flush assertions cover the failure paths that still exist after the -o rejection and flag conflict were retired: branch 404, missing config, and malformed TOML with a branch target (the last also pinning zero API requests, per the read-before-network fix).

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-31T20:30:46.996725Z	a4dc035	New commits

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a4dc035bac

ℹ️ 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".

[chatgpt-codex-connector]

Preserve drift exit semantics in telemetry

When config diff --exit-code finds drift, this sets ProcessControl to 2, but withLegacyCommandAnalyticsImplementation currently collapses every nonzero process-controlled status to exit_code: 1 and attaches the generic ProcessControlledFailure classification. Consequently, an expected drift result is reported as an unknown CLI failure, corrupting command-success and error-category dashboards; teach the instrumentation about config diff's intentional status 2 instead of routing it through the generic failure fallback.

AGENTS.md reference: apps/cli/AGENTS.md:L308-L312

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Deduplicate values before comparing sets

When a field using arrayEquality: "set" contains duplicate entries on only one side, the early length check reports drift even though the memberships are identical. For example, local auth.additional_redirect_urls = ["https://a", "https://a"] and remote ["https://a"] compare unequal despite this field being explicitly modeled as membership-only; deduplicate the canonicalized arrays before checking their sizes and elements.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Prevent config push from climbing above an explicit workdir

When --workdir or SUPABASE_WORKDIR names a config-less directory beneath another project, cliSettings.workdir is already the authoritative directory, but findCliProjectRoot climbs to the ancestor project and the later loadCliConfig call searches upward again. config push can therefore read the ancestor's .env and config and send those settings to the explicitly resolved remote instead of failing for a missing config; use the resolved workdir directly and load with ancestor searching disabled.

AGENTS.md reference: apps/cli/AGENTS.md:L272-L279

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Report parse failures using the selected config filename

When a project contains supabase/config.json—or both files, in which case the loader selects JSON—a malformed JSON document is caught here but reported as a failure to parse supabase/config.toml. This directs the user to edit a file that was not read and obscures which supported config document failed; construct the message from the CliConfigParseError path or format instead of hard-coding TOML.

AGENTS.md reference: AGENTS.md:L50-L53

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Redact invalid project-ref values from telemetry

When config push --project-ref <value> receives anything other than a valid 20-letter ref, the flag parser still accepts the string and LegacyProjectRefResolver rejects it only inside the wrapped handler. Because this unconditionally marks the flag safe, the post-run cli_command_executed event contains the arbitrary invalid value verbatim rather than "<redacted>", allowing a mistyped branch name, token, or other user data to be sent to PostHog; only add project-ref to safeFlags after validating it with PROJECT_REF_PATTERN, as config diff already does.

AGENTS.md reference: apps/cli/AGENTS.md:L313-L313

Useful? React with 👍 / 👎.

[kanadgupta]

Thank you for the adversarial review — every blocker reproduced against running code made this round very direct to act on. All feedback is addressed in seven follow-up commits (no force pushes):

Commit	What it carries
a32ada7 fix(api)	Relaxes all 13 object-level required arrays under V2ProjectConfigResponse.data.attributes via the established openapi-overrides.json mechanism, so the staging shape (missing storage.database_pool_mode) and permission-truncated responses decode instead of hard-failing. Pinned by a client test with the exact staging shape.
3f23ca8 fix(config)	The classifier cluster, expressed as registry row knowledge: arrayEquality: "set" | "sequence" (sequence default; additional_redirect_urls opts into set), unconfiguredValue replacing JS-zero inference (sessions "0s", the 13 provisioning-default subjects — values pinned from the recorded config_auth fixtures in this repo —, notification toggles false), the unmanaged bucket for declared-but-unpushable paths, segment-array paths end to end, remote_only keeping the materialized local + declared, counts computed once with total, the defaults option deleted, DiffProjectConfigOptions taking the loaded pair, envVariables as a list, and ProjectConfigParseError kept in the typed channel for both normalizer calls.
04a174a fix(cli)	--exit-code gets Flag.withDefault(false) (plus a parser-level e2e pin), and -o/--output is now honored per Legacy Shell Invariant #6 instead of rejected — which also retires the rejection error, its three papercuts, and the frozen-record row (file restored to develop's version, undoing the reflow).
fba78b7 refactor(cli)	--target folded into a branch-accepting --project-ref (link's vocabulary, with the 20-lowercase-letters sentence); config loaded and validated before any network call with the [remotes.*] overlay re-applied post-resolution; UUID targets resolve without a linked parent; branch resolution under an output.task; UUID echoed as an identifier, not a quoted name; telemetry flush outermost with the cache write gated on a resolved ref; safeFlags aligned across the config family (ref-shape-gated on diff, plain on push, documented list extended).
27b62ef fix(cli)	--workdir honored (diff and push), drift exits 2 with 1 reserved for errors, legacySanitizeInlineName on every non-constant string in text output, schema_version: 1 (integer payload contract) + config_schema (the $schema URL), scope: {present, missing} with the block set exported from @supabase/config and empty records counting as not-returned, the masked/unmanaged caveats carried in the machine message, 404 on the config route classified as invalid input, and one spelling per concept with real pluralization.
977e300 test(cli)	The structural blind spots: a realistic fresh-project auth record in the shared fixture (clean end to end), the live test asserting auth-record cleanliness rather than just exit 0, not.toContain(<secret>) on both streams and the machine payload, and telemetry-flush/no-cache assertions on the failure paths.
a4dc035 docs(cli)	The docs-site overlay (docs/supabase/config/diff.md) and ADR 0022 → accepted, with the review-driven decisions recorded (including the three it was silent on — all three changed in this round).

Verification: pnpm check:all green (12/12 tasks), full CLI unit (5433) + integration (3423) suites green, packages/api/packages/config suites green, and the targeted diff.e2e.test.ts green against the built binary. I couldn't run the live suite from this environment (no staging credentials here) — the staging repro is covered by the new contract test with your exact database_pool_mode shape, and the live test now asserts the auth-record cleanliness your review flagged.

One deliberate non-change, called out inline as well: the linked-project cache write for an explicit --project-ref in an unlinked directory is pre-existing family-wide behavior (backups/list, config push both Effect.ensuring(cache(ref)) with whatever ref resolved; the cache itself is fire-and-forget and only fills an absent file). Diverging in one command felt worse than the status quo — happy to open a separate issue on the family semantics if you think it should change.

Also worth noting: the three storage.* remote-only entries from your staging run are, as far as I can tell, genuine plan-provisioned state (image_transformation.enabled: true etc. on the platform vs the schema's false default) — a config push of a default file would genuinely try to change them, so they report by design (ADR 0022's "platform defaults that diverge from schema defaults surface as drift"). The row-level unconfiguredValue mechanism is there if a plan-independent unconfigured value emerges for them.

[Coly010]

/ai-review

[github-actions]

Superseded by a newer AI review

🤖 AI Review

Both independent reviews completed. After code and convention verification, all 13 deduplicated findings are confirmed: one critical telemetry privacy issue, one major branch-resolution bug, six minor concerns, and five nits.

Findings

Severity	Location	Category	Sources	Claim
🔴 CRITICAL	apps/cli/src/legacy/commands/config/push/push.command.ts:39	telemetry-privacy	claude+codex	config push sends any supplied --project-ref string to telemetry verbatim, including invalid values that may contain user data or secrets.
🟠 MAJOR	apps/cli/src/legacy/commands/config/diff/diff.handler.ts:137	correctness	claude	Branch-name resolution uses the linked branch ref as the parent, causing config diff --project-ref <branch-name> to fail after linking directly to a branch.
🟡 MINOR	apps/cli/src/legacy/commands/config/diff/diff.handler.ts:171	user-experience	claude	Configs containing [remotes.*] are loaded twice, duplicating load-time deprecation warnings.
🟡 MINOR	apps/cli/src/legacy/commands/config/diff/diff.handler.ts:186	forward-compatibility	codex	The command passes the response through strict generated decoding before its lenient config projection, so new API enum values can fail before fromApiProjectConfig runs.
🟡 MINOR	packages/config/src/config-diff.ts:239	correctness	codex	The set array mode actually implements multiset equality, producing drift when duplicate counts differ even though membership is identical.
🟡 MINOR	apps/cli/src/legacy/commands/config/diff/diff.format.ts:236	api-contract	codex	The versioned payload always emits target.branch: null, although its documented contract says the field is optional.
🟡 MINOR	packages/config/src/config-diff.unit.test.ts:438	code-quality	codex	The TypeScript test source contains a literal NUL byte.
🟡 MINOR	apps/cli/src/legacy/commands/config/push/push.integration.test.ts:426	test-coverage	claude	The subdirectory config-push test no longer exercises subdirectory discovery because it changes only the mocked runtime cwd while the handler reads cliSettings.workdir.
⚪ NIT	apps/cli/src/legacy/shared/legacy-db-target-flags.ts:149	dead-code	claude	target is a stale value-consuming flag registration even though no legacy command declares --target.
⚪ NIT	apps/cli/src/legacy/commands/config/diff/diff.e2e.test.ts:18	test-cleanup	claude+codex	The e2e test leaks its temporary working directory.
⚪ NIT	apps/cli/src/legacy/commands/config/diff/diff.format.ts:7	module-boundaries	claude	The pure formatter imports a regex from a service-bearing resolver module, unnecessarily pulling Effect and API-service code into its dependency graph.
⚪ NIT	apps/cli/src/legacy/commands/config/diff/diff.format.ts:81	dead-code	claude	LEGACY_CONFIG_DIFF_PAYLOAD_VERSION is exported despite having no external consumer.
⚪ NIT	packages/api/scripts/openapi-overrides.json:629	diff-noise	claude	Two unrelated pre-existing comments were changed from literal em dashes to JSON Unicode escapes.

Findings outside the diff

• 🟡 MINOR apps/cli/src/legacy/commands/config/push/push.integration.test.ts:426 — The subdirectory config-push test no longer exercises subdirectory discovery because it changes only the mocked runtime cwd while the handler reads cliSettings.workdir.

Stats

Claude findings: 9 · Codex findings: 6 · Confirmed: 13 · Refuted: 0 · 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.

[github-actions]

🟠 MAJOR · correctness · source: claude

Branch-name resolution uses the linked branch ref as the parent, causing config diff --project-ref <branch-name> to fail after linking directly to a branch.

Evidence: diff.handler.ts:137 passes resolver.resolve(Option.none()); legacy-project-ref.layer.ts:87-93 resolves that from .temp/project-ref, while legacy-parent-project-ref.ts:155-181 documents that this file contains the branch ref after branch linking and parent-scoped endpoints reject it. legacy-branch-ref.resolver.ts:67-70 then sends it to the parent-scoped branch lookup.

Suggested fix: Pass legacyResolveParentScopedProjectRef(Option.none()) as the lazy parent-ref effect and add a test where .temp/project-ref contains a branch ref.

[github-actions]

🟡 MINOR · user-experience · source: claude

Configs containing [remotes.*] are loaded twice, duplicating load-time deprecation warnings.

Evidence: diff.handler.ts:121 loads without a target and lines 171-172 reload after resolution. packages/config/src/io.ts:505-510 and 603-609 print warnings on every load without deduplication.

Suggested fix: Apply the remote overlay without rereading, or suppress/deduplicate diagnostics during the second load.

[github-actions]

⚪ NIT · dead-code · source: claude

target is a stale value-consuming flag registration even though no legacy command declares --target.

Evidence: legacy-db-target-flags.ts:149 contains target; the command surface instead declares --project-ref at diff.command.ts:14, and the repository has no Flag.*("target") declaration. The completeness test at legacy-db-target-flags.unit.test.ts:262-275 checks only for missing registrations.

Suggested fix: Remove target and optionally make the completeness test detect stale registrations.

[github-actions]

⚪ NIT · test-cleanup · source: claude+codex

The e2e test leaks its temporary working directory.

Evidence: diff.e2e.test.ts:18 creates the directory with mkdtemp; the test ends at line 26 without any removal or cleanup hook.

Suggested fix: Remove the directory in a finally block or use a scoped temporary-directory helper.

[github-actions]

⚪ NIT · module-boundaries · source: claude

The pure formatter imports a regex from a service-bearing resolver module, unnecessarily pulling Effect and API-service code into its dependency graph.

Evidence: diff.format.ts:7 imports LEGACY_BRANCH_UUID_PATTERN from legacy-branch-ref.resolver.ts, whose lines 1-4 import Effect and LegacyPlatformApi, despite diff.format.ts:10-12 describing the formatter as containing no Effect or services.

Suggested fix: Move the shared project-ref and UUID patterns into a small service-free module.

[github-actions]

⚪ NIT · diff-noise · source: claude

Two unrelated pre-existing comments were changed from literal em dashes to JSON Unicode escapes.

Evidence: pr.diff changes only — to \u2014 in the CLI-2157 comments at openapi-overrides.json:629 and 659; the substantive response-contract overrides are elsewhere.

Suggested fix: Restore the literal em dashes to keep the diff scoped to the intended overrides.

[github-actions]

🟡 MINOR · forward-compatibility · source: codex

The command passes the response through strict generated decoding before its lenient config projection, so new API enum values can fail before fromApiProjectConfig runs.

Evidence: diff.handler.ts:186 calls generated api.v2.getProjectConfig; effect-client.ts:2509-2521 uses decoded client.execute, and contracts.ts:11135 restricts pool_mode to three literals. Trusted ADR 0019 lines 51-67 and 120-128 explicitly require executeRaw before lenient decoding.

Suggested fix: Use executeRaw, handle non-2xx status explicitly, and pass the raw JSON to fromApiProjectConfig.

[github-actions]

🟡 MINOR · correctness · source: codex

The set array mode actually implements multiset equality, producing drift when duplicate counts differ even though membership is identical.

Evidence: config-diff.ts:239 rejects unequal lengths and lines 244-248 sort without deduplicating. registry-auth.ts:738-741 describes auth.additional_redirect_urls as membership-only, while config-diff.unit.test.ts:455-458 currently pins duplicate-sensitive behavior.

Suggested fix: Canonicalize set-mode arrays through a Set before comparing and add duplicate-membership coverage.

[github-actions]

🟡 MINOR · api-contract · source: codex

The versioned payload always emits target.branch: null, although its documented contract says the field is optional.

Evidence: diff.format.ts:224-226 converts undefined to null and line 236 always spreads the resulting branch property. SIDE_EFFECTS.md:89-95 documents target.branch as optional.

Suggested fix: Omit branch when no branch was selected, or document it as always present and nullable before publishing schema version 1.

[github-actions]

🟡 MINOR · code-quality · source: codex

The TypeScript test source contains a literal NUL byte.

Evidence: config-diff.unit.test.ts:438 contains byte 0x00 between the quotes passed to join; file inspection classifies the source as data rather than text.

Suggested fix: Replace the literal byte with the escaped source spelling "\0" or "\u0000".

[kanadgupta]

Develop is merged in (merge commit 7c63ce697, no force pushes) and all 13 AI-review findings are addressed across three follow-up commits.

The merge, and one deliberate reversal. Develop's CLI-2234 surface trim moved the diff engine's exports (diffProjectConfig, the ConfigChange* types, projectConfigApiBlockKeys) onto the apps/cli-only ./internal subpath — they stay out of the semver-covered contract until config pull settles the shape (isEqualConfigValue left the barrels entirely; only the package's own tests consume it). The upstream spec sync also renamed V2ProjectConfigResponse → V2ProjectConfigResponse_Output, stranding the 26 contract-relaxation override entries from the last review round — and rather than re-pointing them, the merge drops the relaxation entirely, because the AI review's forward-compatibility finding is right on the underlying architecture: ADR 0019 rule 2 says this fetch must go through executeRaw ("required, not incidental"), and 17254efdd now does exactly that, following the projects/list pattern. That supersedes the contract-loosening we settled on in the previous round — same goal, the ADR-designed mechanism: partial responses and unknown enum values (a new pool_mode) both degrade through @supabase/config's one lenient mirror, pinned by a new integration test, and the generated contract stays strict for typed-client consumers. The PR is ~800 lines smaller for it.

Finding → commit map:

Finding	Commit
🔴 CRITICAL: push logs any --project-ref verbatim	44572340c — the whitelist is gated on PROJECT_REF_PATTERN (link/diff's guard); the wiring is exported (legacyConfigPushHandler, link's precedent) and both directions are pinned (verbatim ref / "<redacted>" arbitrary string). The context-merging analytics mock those tests need existed as three identical local copies — hoisted to tests/helpers/mocks.ts per the hoist rule.
🟠 MAJOR: branch-name resolution uses the linked branch ref as parent	44572340c — the lazy parent is now legacyResolveParentScopedProjectRef (the branches family's resolver), with a regression test where .temp/project-ref holds a branch ref and the lookup must hit the parent from linked-project.json.
🟡 strict decode before lenient projection	17254efdd → executeRaw per ADR 0019 (see above).
🟡 remotes configs loaded twice (duplicate warnings)	b02b19487 — reload only when a remote's project_id actually matches the resolved ref, checked on the already-loaded document.
🟡 set mode is multiset	b02b19487 — true membership equality via Set, duplicate-membership coverage added.
🟡 target.branch: null vs documented-optional	b02b19487 — omitted when absent.
🟡 literal NUL byte in test source	b02b19487 — escaped "\u0000" spelling.
🟡 push subdirectory test no longer exercises discovery	b02b19487 — it varies cliSettings.workdir (what --workdir resolves to) instead of the now-unused mocked cwd.
⚪ stale target flag registration	b02b19487 — removed.
⚪ e2e temp-dir leak	b02b19487 — removed in a finally.
⚪ formatter imports a service-bearing module for a regex	b02b19487 — patterns hoisted to a service-free legacy-ref-patterns.ts.
⚪ unused LEGACY_CONFIG_DIFF_PAYLOAD_VERSION export	b02b19487 — consumed by the integration test asserting the payload's schema_version.
⚪ em-dash JSON escapes in the overrides	resolved by the merge itself — the overrides file reverted to develop's version with the relaxation drop.

Verification: pnpm check:all green (13/13), full CLI unit (5532) + integration (3439) suites green, packages/config (1119) and packages/api suites green, targeted diff.e2e.test.ts green against the built binary.