chore(cli-go): delete internal/start outright (unreachable) (CLI-1966) - #6075
Merged
Coly010 merged 5 commits intoAug 6, 2026
Conversation
…1966) internal/start (Go's `supabase start`, 1509 LOC) is unreachable from the TS shell -- native TS start talks to Docker directly, and the only remaining Go delegation is `db __db-bootstrap` -> internal/db/start, a separate, small package. Yet it alone drags in an exclusive dependency tree (docker-compose/v2, buildx, moby/buildkit, k8s client-go, aws-sdk-go-v2) that measures out to roughly half the shipped supabase-go binary's size across every release target (darwin/linux/windows, arm64/amd64): 97.6 MB -> 48.7 MB on darwin/arm64 (-48.8 MB, 50.0%); 51-52 MB saved on every other target. cmd/start.go keeps the "start" command's registration, flags, and validation tag-neutral so the shipped command/flag/completion surface can never drift between builds; only its RunE body differs, split into cmd/start_full.go (default build, backed by internal/start -- this is apps/cli-go's own CI and the migration's parity oracle) and cmd/start_bundled.go (-tags bundled, a stub error instead of importing internal/start). apps/cli/scripts/build.ts and tools/release/local-release.ts, the two places that produce the actually-shipped binary, now pass -tags bundled. cli-go-ci.yml's Start job builds and verifies both variants (dependency-tree exclusion, command/flag/completion parity, and the stub's error behavior) before running the existing full e2e against the untagged tree. internal/start's source is untouched and keeps building/testing normally without the tag.
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! 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". |
…from bundle (CLI-1966) internal/start (Go's own `supabase start`) is genuinely unreachable, not just unneeded in the shipped binary: native TS `start` never invokes it, and no other still-live TS->Go delegation seam (db test, db branch/remote, db diff --use-pgadmin/--use-pg-schema, db pull --experimental, the hidden db __db-bootstrap/__shadow/__catalog seams) imports or calls into it either -- a repo-wide grep confirms the only reference anywhere in apps/cli-go was its own cobra registration file. Keeping it "as a parity oracle" only makes sense if it could ever again be read as a porting reference; since nothing can, the build-tag exclusion from the previous commit is replaced with outright deletion. - Delete apps/cli-go/internal/start/ (source, tests, embedded templates) and the now-pointless cmd/start_full.go / cmd/start_bundled.go split. cmd/start.go's RunE is a single, permanent stub (same behavior as the old bundled stub); cmd/start_test.go pins its exact error text and flag/cobra surface now that the CI job that used to verify this against a real binary is gone. - go mod tidy drops the whole exclusive dependency tree (docker-compose/v2, buildx, moby/buildkit, k8s client-go, aws-sdk-go-v2, notary, secret-detector), confirmed exclusive via `go mod why` before removal. - Remove cli-go-ci.yml's Start job entirely (no more "full" build to run ./main start against) and the e2e-test.sh/tests/ fixtures it alone used; add back a lightweight dependency-tree guard so a future import can't silently reintroduce the removed weight. Update the two mirror workflows' comments that referenced the now-gone Start job as their CI consumer. - Remove the now-meaningless `-tags bundled` from the two release build scripts -- there is no remaining `//go:build bundled` constraint anywhere. - Fix a real functional break the deletion surfaced: an e2e test read internal/start's kong.yml template straight off disk; repointed to the existing TS transcription. - Repoint stale `apps/cli-go/internal/start/...` provenance comments across the TS tree to the last commit where the file existed, and remove a test.todo that could never be fulfilled once no Go implementation was left to diff against. Linear: CLI-1966
Coly010
changed the title
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! 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@3a552fa8d072a291c63477d4ea75dfff025bf850Preview package for commit |
avallete
approved these changes
Aug 5, 2026
…6-cut-internalstart-from-the-bundled-go-build-42-mb-per # Conflicts: # apps/cli/docs/go-cli-porting-status.md # apps/cli/src/legacy/commands/db/shared/legacy-pgdelta.seam.service.ts
oxfmt reflows markdown table column widths; the merge from develop left the table using stale widths.
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! 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". |
…6-cut-internalstart-from-the-bundled-go-build-42-mb-per # Conflicts: # apps/cli/src/shared/functions/serve.ts
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. 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". |
Coly010
deleted the
columferry/cli-1966-cut-internalstart-from-the-bundled-go-build-42-mb-per
branch
August 6, 2026 10:57
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
Deletes
internal/start(Go'ssupabase startimplementation, 1,509 LOC) outright, rather than only excluding it from the bundled/releasesupabase-gobinary via a build tag (the approach this PR originally took).Why the revised approach
The original build-tag version kept
apps/cli-go/internal/start/intact in-tree as "the migration's parity oracle," reasoning that the source might still be useful as a reference even though it doesn't ship. Re-examined under a stricter lens: keeping dead code around as a parity oracle only makes sense if it could ever again be read as a reference for porting something. It can't here — nothing in the TypeScript CLI will ever delegate to it, and nothing else Go-side that's still reachable calls into it either. That's not a parity oracle, it's dead weight, and this milestone's charter is removing Go, not archiving it.Verified both reachability paths, not just "does the bundled binary need it":
startcommand. Confirmed (per the issue's own claim) that native TSstarttalks to Docker directly and never invokes Go'sstart. The one place a stale comment suggested otherwise (legacy-pgdelta.seam.service.ts'sensureLocalDatabaseStarteddoc, which said it "starts the stack via the bundledsupabase-go start") was checked against the actual implementation and against Go's owncmd/db_schema_declarative.go— both actually usesupabase-go db start→internal/db/start, a separate, still-live package. The comment was simply wrong; fixed in this PR.docs/go-cli-porting-status.mdand Linear CLI-1970's own "expected alive set":db diff --use-pgadmin/--use-pg-schema,db pull --experimental, thedb branch/db remote/gen keysproxies,db test, the hiddendb __db-bootstrap/__shadow/__catalogseams) against a repo-widegrep -rn "github.com/supabase/cli/internal/start" apps/cli-go --include="*.go". The only hit anywhere in the module wascmd/start_full.go's own import — now deleted along with it.cmd/start.go's tag-neutral cobra registration never importedinternal/start— it only duplicated theexcludableContainers()helper for the flag's help text, confirming the registration is genuinely just flags/cobra wiring.internal/start's own tests (start_test.go) only import broadly-shared test infra (apitest,pgtest,pkg/config, etc.) that's used elsewhere too, so deleting them orphans nothing.What changed
apps/cli-go/internal/start/(source, tests, embedded templates) and the now-pointlesscmd/start_full.go/cmd/start_bundled.gosplit.cmd/start.go'sRunEis now a single, permanent stub with the same behavior the bundled build's stub already had (same error text, same--help/__completesurface).cmd/start_test.gopins the stub's exact error text and the command/flag surface, since the CI job that used to verify this against a real binary is gone.go mod tidydrops the entire exclusive dependency tree (docker-compose/v2, buildx, moby/buildkit, k8s client-go + friends, aws-sdk-go-v2 + friends, notary, secret-detector) — each confirmed exclusive tointernal/startviago mod whybefore removal.docker/cli(and its own transitive deps) stay, correctly, becauseinternal/utilshas its own independent, direct import ofdocker/cli/cli/command.cli-go-ci.yml'sStartjob entirely rather than rescoping it — rescoping only made sense when a "full" build with a realinternal/startimplementation still existed to run./main startagainst; once it's deleted, nothing in that job can function. Deleted the now-orphanedapps/cli-go/e2e-test.sh+apps/cli-go/tests/fixtures it alone used. Added back a lightweightgo list -depsguard so a future accidental import of the heavy orchestration tree gets caught immediately rather than silently regrowing the binary.-tags bundledflag fromapps/cli/scripts/build.tsandtools/release/local-release.ts— there is no remaining//go:build bundledconstraint anywhere in the module, so both files are now byte-identical to pre-milestonedevelop.shared/functions/serve-main-offline.e2e.test.tsreadinternal/start/templates/kong.ymlstraight off disk at test runtime; repointed to the existing TS transcription (LEGACY_START_KONG_YML_TEMPLATE), verified byte-identical to the deleted Go template and passing.apps/cli-go/internal/start/...provenance comments across the TS tree to the last commit where the file existed (a253ccba25c21356ccd33044c4474aecb77d1ae4), and removed atest.todo("parity: start")inapps/cli-e2ethat could never be fulfilled once no Go implementation was left to diff TS's behavior against.docs/binary-distribution.md,docs/go-cli-porting-status.md, andapps/cli/AGENTS.mdto describeinternal/startas removed rather than excluded from the bundle.Measured size impact
Rebuilt the simplified (single-implementation, no tags) binary with the exact release flags for every target and compared against the numbers this PR previously measured for the tagged/bundled build:
Byte-for-byte identical (within noise from source-layout changes on windows/arm64). This is expected: deleting the package outright and excluding it via a build tag produce the same compiled bytes — the size win was already fully realized by the previous commit; this change is about deleting genuinely dead source, not further shrinking the binary. The full before/after-this-milestone comparison (97–103 MB → ~47–52 MB per platform, ~50% reduction) is unchanged from the original PR description.
Review
Independently reviewed by go-parity-auditor, engineer-reviewer, and architect-reviewer against the full revised diff. All three confirmed the reachability analysis holds (re-derived independently, including rebuilding and diffing the binary's
--help/__complete/completionoutput against the pre-deletion build). Convergent and individual findings were fixed:cmd/start_test.gonow isolates the sharedutils.CmdSuggestionglobal and asserts the cobra/flag surface; two stale mirror-workflow comments that referenced the now-removedStartjob were updated; a stale "validation is kept" claim incmd/start.gowas corrected (validation moved to the TS port); write-only package globals incmd/start.gowere cleaned up; a docs claim aboutapps/cli-gobeing unconditionally authoritative was qualified for this one exception; and a new fast-tier unit test (cli-go-path-references.unit.test.ts) now guards everynew URL(".../cli-go/...")literal in the TS tree against exactly the kind of silent breakage this PR found and fixed.Linear: CLI-1966