feat(cli): detect and confirm branch targets in config push - #6446
Merged
Coly010 merged 6 commits intoSep 4, 2026
Merged
Conversation
…, CLI-2289) config push now always reports the project or branch it resolved (by name when known), and gates an inferred branch target behind a confirmation before any network mutation, skippable via the existing --yes/SUPABASE_YES. --project-ref also now accepts a branch name or UUID, matching config diff's existing vocabulary. Hoists the branch/project detection shared by link and status into legacy-branch-target.ts, and the --project-ref branch-name/UUID resolution shared by config diff and config push into config.branch-target.ts, refactoring both existing commands onto them.
Contributor
Author
|
/ai-review |
Coly010
marked this pull request as ready for review
September 3, 2026 14:39
Contributor
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@40c3107824ba90af6af29e1372852dcaabd44e75Preview package for commit |
A hostile-name test case embedded a literal ESC (0x1B) control byte in the source instead of the \x1b JS string escape, which broke tools that treat the raw diff as terminal output (gh pr diff, this repo's own AI review pipeline). Same runtime string value, safe source bytes.
…8-inform-users-of-the-branch-they-push-to
Contributor
Author
|
/ai-review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🤖 AI Review
Verified and deduplicated all 14 reported findings into 13 entries. Twelve are confirmed and one is refuted. The principal regression is that config push now decodes the base document before applying a matching remote overlay, rejecting configurations that become valid only after that overlay. Branch-target detection also introduces several confirmed reliability, diagnostics, and coverage gaps.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | apps/cli/src/legacy/commands/config/push/push.handler.ts:211 |
correctness |
claude+codex | The preliminary target-less decode rejects configurations whose matching remote overlay would make the effective configuration valid, and matching remotes also repeat load-time warnings. |
| 🟡 MINOR | apps/cli/src/legacy/commands/config/push/push.branch-target.ts:157 |
error-handling |
claude | Non-404 failures from the target-classification probe abort config push even though they could safely degrade to an uncertain branch classification. |
| 🟡 MINOR | apps/cli/src/legacy/commands/config/push/push.branch-target.ts:178 |
correctness |
claude | A project lookup exceeding five seconds is reported as a branch and can cause an unattended plain-project push to be cancelled. |
| 🟡 MINOR | apps/cli/src/legacy/commands/config/push/push.integration.test.ts:1503 |
test-coverage |
claude | The broken project-ref test does not reach the filesystem read whose failure it claims to verify. |
| 🟡 MINOR | apps/cli/src/legacy/commands/config/push/push.handler.ts:316 |
usability |
claude | Machine-output branch cancellation omits the prompt's --yes recovery hint and reports only a generic cancellation error. |
| 🟡 MINOR | apps/cli/docs/supabase/config/push.md:9 |
documentation |
claude | The user-facing documentation says preview-branch pushes ask for confirmation without documenting that explicit branch names and UUIDs bypass it. |
| 🟡 MINOR | apps/cli/src/legacy/commands/config/push/push.branch-target.ts:209 |
user-experience |
codex | The best-effort branch-list request can leave text-mode users without progress feedback for up to five seconds. |
| 🟡 MINOR | apps/cli/src/legacy/commands/config/push/push.integration.test.ts:1544 |
test-coverage |
codex | The new timeout-degradation branch is explicitly left untested despite the repository's command-handler branch-coverage requirement. |
| ⚪ NIT | apps/cli/src/legacy/commands/config/push/push.branch-target.ts:197 |
dead-code |
claude | The knownBranch.parentRef-only recovery path and its UUID commentary are unreachable from the sole production caller. |
| ⚪ NIT | apps/cli/src/legacy/commands/config/push/push.branch-target.ts:173 |
progress-reporting |
claude | A failed project probe clears its spinner instead of marking it failed. |
| ⚪ NIT | apps/cli/src/legacy/commands/config/push/push.handler.ts:316 |
behavior-change |
claude | The new branch gate consumes an additional piped stdin line before existing per-service prompts, shifting scripted answer sequences. |
| ⚪ NIT | apps/cli/src/legacy/commands/status/status.integration.test.ts:1759 |
documentation |
claude | A test title refers to the old timeout constant name, location, and visibility. |
Findings outside the diff
- ⚪ NIT
apps/cli/src/legacy/commands/status/status.integration.test.ts:1759— A test title refers to the old timeout constant name, location, and visibility.
Refuted findings (kept for transparency, not posted as review comments)
apps/cli/src/legacy/commands/config/push/push.handler.ts:315(behavior-change): An explicit ref-shaped --project-ref should bypass the branch confirmation gate under the PR's same-invocation-intent rule.
Refuted: The implementation and accompanying contract consistently distinguish raw 20-letter refs from branch names/UUIDs. config.branch-target.ts:121-122 intentionally leaves branchResolution absent for raw refs, and push.handler.ts:305-315 expressly gates such targets if the later probe identifies them as branches. The alleged contradiction is therefore absent; this is the documented safety behavior of the change.
Stats
Claude findings: 11 · Codex findings: 3 · Confirmed: 12 · 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.
- config.toml now loads AFTER the push target resolves, in one call with the resolved ref, instead of a load-resolve-maybe-reload split: a [remotes.*] overlay is merged inside loadCliConfig itself before its one full schema decode, so a base document that's only valid once its matching remote applies is no longer wrongly rejected, and load-time warnings no longer fire twice on a real remote match. - The live getProject probe that tells config push whether a target is the linked project or a branch is now fully best-effort: a timeout, transport failure, or any non-200/404 status degrades to a new "unknown" target (never silently "project", which would skip the confirmation gate for a real branch; never a hard failure, which would abort an otherwise-successful push over a diagnostic-only read or a scoped token that can't read the project record). "unknown" never gates a confirmation and omits is_branch from the machine payload rather than asserting it. Removes the two error classes this made dead. - LegacyConfigPushKnownBranch is now a discriminated union of the only two shapes a real caller produces (name: both fields; uuid: neither), removing an unreachable partial-state branch in the resolver. - The target-detection probe and the branch-name recovery lookup now show progress feedback (a spinner, marked failed on a genuine probe error) instead of running silently for up to 5 seconds. - The declined-branch-gate error now carries a --yes/SUPABASE_YES suggestion, so a machine-mode or non-TTY decline (which never renders the interactive prompt's own inline hint) still surfaces it. - Docs/SIDE_EFFECTS.md: document that an explicit --project-ref name/UUID bypasses the confirmation gate, and that the gate shifts piped stdin answers by one for a script targeting an inferred branch. - Test fixes: the malformed-config test now asserts branch resolution running before config load (the corrected, intentional order); the EISDIR regression test now actually reaches the read it claims to verify; the two former probe-hard-failure tests now assert the degrade-to-unknown behavior; a stale test title in status.integration.test.ts is updated for the timeout constant's current name/location. Also fixes the failing "Check code quality" CI job (oxfmt table padding in SIDE_EFFECTS.md/push.md, unformatted by the prior commit).
…lope The json-mode branch-decline test only checked for LegacyConfigPushCancelledError's tag, not that its suggestion field (the only place a machine-mode/non-TTY decline surfaces the --yes escape hatch) actually carries the hint.
…8-inform-users-of-the-branch-they-push-to # Conflicts: # apps/cli/src/legacy/commands/config/diff/diff.handler.ts
jgoux
approved these changes
Sep 4, 2026
Coly010
deleted the
columferry/cli-2168-inform-users-of-the-branch-they-push-to
branch
September 4, 2026 08:23
Coly010
added a commit
that referenced
this pull request
Sep 4, 2026
Carries #6446 (branch-target detection and confirmation in config push) onto the diff-first handler: target resolution and the inferred-branch confirmation gate run before the cost-matrix read, the resolved target line replaces the bare ref line, and the machine payload gains the target fields. Adopts #6447's public diffProjectConfig entrypoint and re-applies the family hoists on develop's diff handler/formatter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
supabase config pushnow always reports the project or branch it resolved before doing anything — by name when known, e.g.Pushing config to project: My App (abcdefghijklmnopqrst)orPushing config to branch: feat-x (bbbb…)plus its parent project. When the resolved target is a preview branch that was only inferred (from a priorsupabase link <branch>,SUPABASE_PROJECT_ID, or the linked-ref cache — not named on this invocation), it gates behind a confirmation before any network mutation, skippable via the existing global--yes/SUPABASE_YES(CLI-2168).--project-refalso now accepts a branch name or UUID, not just a raw ref, matchingconfig diff's existing CLI-2167 vocabulary — so aconfig pull --project-ref <branch>/config push --project-ref <branch>round-trip no longer requires manually looking up the branch's raw ref (CLI-2289).Why
config pushgave no indication of whether the resolved ref was the main project or one of its branches, so a user linked to a branch (or overriding via env/flag) had no on-screen signal before mutating it. The confirmation defaults to decline in every non-interactive context without--yes(--output-format json/stream-json, a non-TTY with no piped answer) — fails the command rather than silently proceeding or silently no-opping, matching the existingprojects delete/db resetconvention for this shape of gate. An explicit--project-ref <name-or-uuid>this invocation skips the prompt entirely (the echo still prints) — same-invocation intent is treated as confirmation.Notes for reviewers
The branch/project detection shared by
linkandstatusis hoisted intolegacy-branch-target.ts; the--project-refbranch-name/UUID resolution shared byconfig diffandconfig pushis hoisted intoconfig.branch-target.ts— both existing commands are refactored onto the shared code in this same change, per this repo's hoisting policy. Neither command's own established behavior changes; their existing test suites are unmodified and green.