docs(cli): fix porting-status and SIDE_EFFECTS drift from 2026-07-24 audit (CLI-1967) - #6074
Merged
Coly010 merged 3 commits intoAug 5, 2026
Conversation
…audit (CLI-1967) Independently re-verified every claim from the audit against current Go/TS source rather than trusting it as-is (it's 12 days old and several items had already drifted further, in both directions).
Contributor
Author
|
@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: 628efa88a5
ℹ️ 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".
…view: PR #6074) Two Codex findings on the porting-status doc: - The `## Functions` (next/) table linked to real command implementations (list/delete/deploy/download/new/dev) while still marking every row `missing`, which the Legend defines as "no TS command/subcommand exists yet" - directly contradicted by the links added in the same pass. Reclassify to `partial`: the next/ root already diverges from Go's global flag surface, so none can be `ported` yet, but `missing` misreports that no TS surface exists. Update Family/Overall Summary counts to match. - `functions download` was marked `wrapped` in the Legacy Shell Command Status table, but the new legend defines `wrapped` as "the handler's own body forwards the whole invocation ... with no native business logic of its own." `download.handler.ts` always runs through the shared `downloadFunctions()`, which is fully native for `--use-api` and only delegates wholesale to Go for the default `--use-docker`/`--legacy-bundle` path - the same "flag-gated seam that shells out for one specific sub-path" the `ported` definition explicitly carves out (mirrors the `db diff --use-pgadmin` / `db pull --experimental` precedent). Reclassify to `ported` with a hybrid-path annotation matching that precedent's style.
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Contributor
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@02717dccde3a5a8d6f33fa1a4ba052db2136d97bPreview package for commit |
… quality) The `functions download` row's trailing padding was 3 spaces short of the table's fixed column width after the previous commit appended the "native for --use-api" annotation, tripping oxfmt --check. Re-run oxfmt to restore alignment; no content change.
jgoux
approved these changes
Aug 5, 2026
Coly010
deleted the
columferry/cli-1967-fix-porting-status-side_effects-and-docstring-drift-from-the
branch
August 5, 2026 14:12
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
Doc/comment-accuracy sweep from
apps/cli/docs/go-parity-audit-2026-07-24.md§6 (untracked locally, not part of this repo). No runtime behavior changes — every item below was independently re-verified against current Go (apps/cli-go/) and TS source before fixing, since the audit is 12 days old and several items had drifted further in that time (in both directions).Fixed
docs/go-cli-porting-status.md—functions listlegacy-shell statuswrapped→ported(verified: zeroLegacyGoProxyrefs).functions downloadstatus corrected the other way,ported→wrapped(its handler still delegates wholesale viaLegacyGoProxyfor the default--use-dockerpath; only--use-apiis native — noted as a hybrid in the row). Added aporteddefinition to the legacy-status legend (it only definedwrapped/missing, despiteportedbeing ~90% of rows) and retitled that section from "Legacy Shell Wrapping Status" to "Legacy Shell Command Status" to match. Fixed the adjacentfunctions delete/deploy/list/new/servelegacy-shell notes, which all still said "Wrapped in legacy shell" despite being natively ported.legacy-pgdelta.seam.service.ts— fixed two stale doc comments (execInherit,ensureLocalDatabaseStarted) that referenced things as "not yet ported" when they now are (db reset,start/db start). Did not touchexportCatalog's doc comment /LegacyCatalogMode, which the audit also flagged — that exact hunk is already being rewritten more completely by the in-flight CLI-1959 PR (fix(cli): remove db schema declarative __catalog seam for migrations mode (CLI-1959) #6061, open); fixing it here would guarantee a conflict with a strictly better version.legacy-db-bootstrap.seam.service.ts(the audit's other named file) was checked and found already accurate — CLI-1954/1955 (nativedb start/db reset --local) are still unmerged, so its "not yet ported" claim is currently true.network-restrictions/{get,update}/SIDE_EFFECTS.md— the-o {json,yaml,toml,env}sections previously implied Go itself produces distinct byte-identical output per format. Verified against Go source:restrictions/get/updatenever readOutputFormatat all — they always print the same 3-linefmt.Printftemplate regardless of-o. Corrected both docs to state this plainly, documented that TS's format-specific output here is a deliberate TS-only enhancement with no real Go behavior to match (including no Go casing convention, since TS uses the map-shaped encoders rather than CLI-1975's struct-spec ones), and trimmed the resulting repetition.inspect/report/SIDE_EFFECTS.md— added the empty/no-file divergence onCOPYfailure: Go'scopyToCSVopens the output file withO_TRUNCbefore running the query, so a failing query still leaves a file (empty or partial); TS buffers in memory and only writes on success, leaving no file on a fresh run — and leaving the previous run's stale CSV in place on a same-day re-run (the more consequential case). Cross-referenced fromlegacy-db-connection.errors.ts'sLegacyDbCopyErrordoc comment, which already covered the message-text angle of the same divergence.domains.cname.ts— the comment describing Go's CNAME "failed to locate" error dump wrongly implied Go embeds readable JSON. Verified against Go source and empirically (compiled the equivalent locally): Go JSON-marshals the answers to a[]byte, then formats that[]bytewith%+v, which Go'sfmtrenders as an uncapped decimal byte-value array, not the JSON text — a%+v-on-[]bytefootgun, not an intended format. Fixed both the function's JSDoc and the inline comment (they'd contradicted each other after an earlier pass), and cross-referenced the divergence fromdomains/SIDE_EFFECTS.md.branches/orgs/projects/secretsSIDE_EFFECTS-o toml/-o yamlclaims,functions deploy'sNPM_AUTH_TOKENenv table entry, andupdate-root-key.handler.ts's color comment — all already fixed by CLI-1975 (fix(cli): match Go machine-format encoder output for -o toml/yaml/json (CLI-1975) #6002), CLI-1985 (fix(cli): match Go bundler env and deploy path anchoring (CLI-1985) #6005), and CLI-1990 (fix(cli): edge and cosmetic parity sweep from the 2026-07-24 audit (CLI-1990) #5978) respectively, which merged after the audit ran. Verified current state matches; no changes needed.start/SIDE_EFFECTS.md's--ignore-health-checkruling — already fully handled by CLI-1987 (docs(cli): record intentional start --ignore-health-check divergence from Go (CLI-1987) #6007, merged), whose own description explicitly says CLI-1967 should not re-document it. Left untouched.Left as noted, not fixed (out of scope for a docs-only pass)
network-restrictions get/updateTS-only-osupport is a real, pre-existing behavioral divergence from Go (Go has no such behavior at all for these two commands) — documented accurately here, but whether it should be removed to enforce strict parity is a ruling this PR doesn't make.docs/go-cli-porting-status.md's "Functions" section (next/-shell table) has a larger, pre-existing inaccuracy discovered while fixing the adjacent legacy-shell notes: it claims there's "still no dedicatedfunctionsCLI surface" innext/, butnext/commands/functions/already exists (list/delete/deploy/download/new/dev, registered innext/cli/root.ts). Added command-path links and flagged the section as needing its own flag-by-flag parity audit rather than silently reclassifying rows without one.Fixes CLI-1967