fix(cli): edge and cosmetic parity sweep from the 2026-07-24 audit (CLI-1990) - #5978
Conversation
…LI-1990) Batch of small confirmed Go-parity divergences, each verified against apps/cli-go before fixing: Output/colour - functions delete/deploy: Aqua slug/ref and Bold bundling slug + No-Functions dir, with stdout-gated colour where the line is stdout-bound (delete.go:20, deploy.go:35,70, bundle.go:30) - encryption update-root-key: Aqua'd Finished line; removed the comment inventing a render-Aqua-as-plain convention (update.go:26) - start rollback: print "Stopping containers..." to stderr (docker.go:97 via start.go:77) - --debug prune reports: Pruned containers/volumes/network lines on stderr from legacyDockerRemoveAll, threading LegacyDebugFlag through stop/start (docker.go:123-143; "network" singular) - per-retry image-pull banner: Retrying after 4s/8s: <image> (docker.go:312-314) - inspect db blocking: backtick-wrap blocking_statement (col 2) like Go's row format (blocking.go:56) - seed buckets: mutex group bracket keeps Go's registration order [local linked] (cmd/seed.go:32, cobra flag_groups.go:73) Input handling - snippets download: accept google/uuid Parse's 4 forms incl. the braced-form s[1:] quirk, canonical lowercase into the URL, Go's three error branches verbatim (download.go:15-19, uuid.go:68-117) - storage cp: reject negative --jobs with pflag's uint error template; keep the documented 0->1 clamp (cmd/storage.go:107) - postgres-config: strconv.ParseBool's exact case-sensitive set, int64-overflow falls back to the verbatim string, and the pretty table renders numbers with Go's %+v float64 formatting via the hoisted legacyGoFormatFloat (update.go:41-49, get.go:32-35) Filesystem modes (Go pins them; Node defaulted) - init templates 0644/0755, test new 0644/0755, inspect report 0755/0644, login fallback dir 0700 -> Go's 0755 (token stays 0600) Misc edges - migration new: print the Created line even when the stdin copy fails (Go's deferred Println, new.go:24-28), stdout-gated Bold - telemetry state: all-or-nothing decode like Go's decodeState -- a partially-corrupt file recreates everything fresh (state.go:87-115) - services: warn failed to load project ref on a ref-file read error and continue unlinked (services.go:18-20) - db dump: --file "" means stdout, no Dumped-schema line (dump.go:20-32, cmd/db.go:152-159) Fixes CLI-1990
|
@codex review |
![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: e2fbd7309a
ℹ️ 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".
| import { operationDefinitions, type ApiClient } from "@supabase/api/effect"; | ||
| import { Effect, type Option } from "effect"; | ||
| import * as HttpClientError from "effect/unstable/http/HttpClientError"; | ||
| import { legacyAqua } from "../../legacy/shared/legacy-colors.ts"; |
There was a problem hiding this comment.
When a v3/next user runs functions delete with a color-capable stdout, next/commands/functions/delete/delete.handler.ts calls this shared function and now receives legacy Go-specific ANSI styling; this import also makes the next command transitively depend on the legacy tree. Apply the coloring in the legacy handler or inject shell-specific formatting so the next shell remains isolated.
AGENTS.md reference: apps/cli/AGENTS.md:L18-L21
Useful? React with 👍 / 👎.
| if (Option.isSome(flags.jobs) && flags.jobs.value < 0) { | ||
| return yield* new LegacyStorageInvalidJobsError(flags.jobs.value); | ||
| } |
There was a problem hiding this comment.
For storage cp --jobs=-1 without --experimental, this check is never reached because legacyRequireExperimental fails first; positional parsing can similarly preempt it. The Go command declares jobs with UintVarP in cmd/storage.go, so the negative value fails during flag parsing before PersistentPreRunE and must report the ParseUint error regardless of those later conditions. Put the unsigned validation in the flag parser/refinement rather than inside the handler.
AGENTS.md reference: apps/cli/AGENTS.md:L246-L256
Useful? React with 👍 / 👎.
| if (typeof rawLastActive === "string") { | ||
| if (!RFC3339_RE.test(rawLastActive) || !Number.isFinite(Date.parse(rawLastActive))) { | ||
| return undefined; |
There was a problem hiding this comment.
When telemetry.json contains a timestamp such as 2025-02-29T00:00:00Z, the regex matches and JavaScript Date.parse normalizes it to March 1, while Go's time.Parse(time.RFC3339Nano, ...) rejects it. The file is therefore treated as valid and its enabled state and identities are preserved instead of triggering the all-or-nothing regeneration this change is intended to match; use a strict RFC3339 calendar validation rather than Date.parse validity alone.
AGENTS.md reference: apps/cli/AGENTS.md:L246-L256
Useful? React with 👍 / 👎.
1 participant
Batch sweep of the small confirmed edge/cosmetic Go-parity divergences from the 2026-07-24 audit. Every item was re-verified against
apps/cli-goat current develop before changing anything (audit line numbers were stale); several items turned out to be already fixed or in-flight in other PRs and are listed as such.Fixes CLI-1990 — https://linear.app/supabase/issue/CLI-1990/edgecosmetic-parity-sweep-from-the-2026-07-24-audit-batchable-tail
Item-by-item
functions deleteline colourdelete.go:20)functions deploysuccess ref Aqua,Bundling Function:bold,No Functions specified…bolddeploy.go:70,35,bundle.go:30; stdout-gated where stdout-bound• <bold>c4b45874supabase start is not running.AquaassertLocalDbRunninghunk inshared/functions/serve.ts; one-liner to do after it mergesencryption update-root-keyFinished line + bogus commentStopping containers...DockerRemoveAllwriter on the start-failure path (start.go:77)--debugPruned …reportsPruned containers:/volumes:/network:(singular network)[a b c]on stderr; prune stdout now collected (also removes a latent unread-pipe hazard);LegacyDebugFlagthreaded through stop/start/rollbackRetrying after Ns: <image>banner4s/8sper Go's2<<(i+1)schedule (docker.go:314); the failed attempt's error line is played by the already-teeddocker pullstderrinspect db blockingbacktickblocking_statementblocked_statement) stays bare per Go's format string (blocking.go:56)seed bucketsmutex bracket[local linked]legacy-db-target-flags.ts(storage's[linked local]stays correct)snippets download4 UUID forms + lowercase URLuuid.Parseport incl. the braced-forms[1:]quirk; canonical lowercase interpolated into the URL; Go's three error branches verbatimstorage cp --jobsnegative rejectioninvalid argument "-1" for "-j, --jobs" flag: strconv.ParseUint: …), before mutex validation and without telemetry; the documented0 → 1clamp stays (Go's 0 deadlocks)postgres-configvalue coercion +%+vfloatsParseBoolcase set; int64-overflow → verbatim string; pretty table renders numbers with Go's float64%g(1000000→1e+06) via hoistedlegacyGoFormatFloat(also reused bydb query)access_token.go:91); the token file stays 0600, so no secret exposuretest new0644utils.WriteFileinspect report0755/0644resolveLegacyAccessTokendeliberately collapses invalid→Nonefor many callers (sso, snippets, postgres-config, …); distinguishing invalid-vs-missing needs a shared-semantics changemigration newCreated line on stdin-copy failurePrintln(new.go:24-28); also stdout-gates the Bold path (CLI-1546 class)decodeState; a corrupt file resetsenabledto true and rotates identity, exactly like Gofailed to load project ref: …on a read error and continues unlinked (TOCTOU NotFound stays silent, like Go'sErrNotLinkedbranch)domainsCNAME dump byte codes%+vis a runtime pointer address forValidationErrors, not byte codes; TS's deterministic rendering is already documented indomains.format.ts--domains=explicit-emptysso updatealready matches Go (len-check drops it); theaddfix touchessso/add/add.handler.ts, in-flight in open PR #5974db dump --file ""len > 0like Go: empty means stdout, no file open, noDumped schema to …linePersistentPreRunE; aligning overlaps open PR #5975 (incl. its integration test file). TheSIDE_EFFECTS.mdnote frames this as intended based on an incomplete Go reading — worth revisiting after #5975Known residuals (deliberate, documented in code)
postgres-configdigits in(2^53, 2^63)still lose precision on the way in (JSON.stringifycannot emit exact int64 tokens); Go sends exact integers there. Values beyond int64 now match Go (string fallback).legacy-colors.ts/CLI-1546 convention), whereas Go's lipgloss gates everything on stdout. Deliberate, pre-existing convention; only observable when exactly one of stdout/stderr is a TTY.util.styleTextcurrently ignoresvalidateStream/NO_COLOR(verified on Bun 1.3.x), so under Bun piped output still carries ANSI for all legacy colour sites — a pre-existing runtime gap that predates this PR and deserves its own issue.*PathErrorbytes — thefailed to load project ref:prefix is the parity-bearing part.--jobs abc/3.5still surface Effect CLI's parser error rather than pflag's; this PR scopes to negatives (the only caseFlag.integeraccepts that Go rejects).Review notes
Four-perspective review (architect / engineer / security / DX) run pre-PR; all approve. Engineer fuzz-verified
legacyGoFormatFloat(23k values) andlegacyParseSnippetUuid(~8k inputs) byte-identical to Go/google-uuid. Security signed off on the 0700→0755 fallback-dir change (token file unchanged at 0600, matches Go exactly). Remaining findings were the documented residuals above.