feat(cli): port supabase seed buckets to native TypeScript#5651
feat(cli): port supabase seed buckets to native TypeScript#5651Coly010 wants to merge 23 commits into
supabase seed buckets to native TypeScript#5651Conversation
Replace the Phase-0 Go proxy for `supabase seed buckets` with a native Effect implementation that is output-compatible with the Go command. The command is local-only in practice: Go's `seed` command defines no `--project-ref` flag, so the project ref is always empty and the remote client factory, service-role-key resolution, and analytics-bucket upsert are unreachable. Only the reachable local behavior is ported: - a native Storage service-gateway HTTP client (list/create/update bucket, vector list/create/delete, object upload) under `seed/buckets/` - bucket upsert with overwrite prompt, vector upsert with the two graceful-skip WARNINGs, and object-tree upload (5-way concurrency) - a lightweight `legacySeedRuntimeLayer` exposing only HttpClient, LegacyCliConfig, LegacyTelemetryState, and CommandRuntime - json/stream-json structured summary + prompt suppression Hoist the docker go-units size helpers (`ramInBytes`/`bytesSize`/ `intToUint`) out of `config/push` into `legacy/shared/legacy-size-units.ts` since `seed buckets` now also parses `file_size_limit`. Rewrite the `seed buckets` SIDE_EFFECTS.md (it described the wrong route) and mark the command `ported` in the porting-status tracker. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01X56gEKaCJZ6SxEasUq7pXM
Reconciles the cloud port with the local branch: ports the
acceptance-criteria e2e compatibility test and reproduces cobra's
MarkFlagsMutuallyExclusive("local","linked") (seed.go:32) with the
verbatim conflict message.
- Honor --yes/SUPABASE_YES in prompts, echoing Go's `<label> [Y/n] y` line and returning true even on a TTY (console.go PromptYesNo). - Match Go's isUploadableEntry for the object walk: detect symlinks no-follow, skip dangling symlinks / symlinks-to-directories / other non-regular entries with `Skipping non-regular file:` instead of crashing, and don't descend into symlinked directories (batch.go:65). - Omit `public` from the bucket create/update body when absent from the TOML, matching Go's `Public *bool` omitempty (buckets.go:29). Adds gateway body unit tests and integration coverage for --yes echo, dangling-symlink skip, and symlinked-dir non-descent.
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@1ca442efcfdd4eab7be39118eb5090598d9c6e22Preview package for commit |
![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: b26a497879
ℹ️ 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".
| ): LegacyUpsertBucketProps { | ||
| return { | ||
| public: publicWasSet ? bucket.public : undefined, | ||
| fileSizeLimit: legacyParseFileSizeLimit(bucket.file_size_limit), |
There was a problem hiding this comment.
When a bucket omits file_size_limit, @supabase/config supplies the schema default (50MiB, see packages/config/src/storage.ts), so this line sends file_size_limit: 52428800 on every create/update. The Go command's per-bucket FileSizeLimit is the zero value unless the TOML key is present, and omitempty then omits it, so common configs like [storage.buckets.images]\npublic = true now unintentionally cap the bucket at 50MiB and update existing buckets to that limit. Recover key presence from the raw document like public and send 0 when absent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks — I dug into the Go path on this one and it actually argues for keeping the current behavior, so I'm leaving file_size_limit as-is.
The premise that Go's per-bucket FileSizeLimit stays at the zero value when the TOML key is absent isn't what happens. config.resolve() normalizes every bucket before the seed command sees it (apps/cli-go/pkg/config/config.go:753-756):
if bucket.FileSizeLimit == 0 {
bucket.FileSizeLimit = c.Storage.FileSizeLimit
}An omitting bucket inherits the storage-level file_size_limit, which is "50MiB" in every generated config.toml (pkg/config/templates/config.toml:118). So for [storage.buckets.images] + public = true, UpsertBuckets sends file_size_limit: 52428800 (pkg/storage/batch.go:38,49) — the omitempty only fires when both the bucket and storage limits are 0. That matches what the TS code emits today; presence-detecting and omitting file_size_limit would actually diverge from Go (we'd send no limit where Go sends 50MiB). public is genuinely different — it's a *bool with no resolve-time fallback, which is why only that field gets the raw-document presence check.
One real but separate gap I noticed while checking: TS sources the 50MiB from the per-bucket schema default (packages/config/src/storage.ts:43) rather than inheriting the storage-level file_size_limit the way Go's resolve() does. Identical for the default 50MiB case, but it would diverge if someone sets a custom [storage] file_size_limit and omits it on a bucket. I'll track that as its own follow-up. Leaving this thread open for visibility.
Address Codex review threads on the seed buckets port (PR #5651): - Resolve the local services host via legacyGetHostname (Go utils.GetHostname: SUPABASE_SERVICES_HOSTNAME -> TCP DOCKER_HOST -> 127.0.0.1) instead of the inline 127.0.0.1 fallback, which dropped the Docker-host tier and broke remote-Docker / dev-container setups (buckets.handler.ts resolveLocalBaseUrl). - Mirror Go generateAPIKeys (pkg/config/apikeys.go:43-63) when deriving the local service-role key: fall back on an empty jwt_secret/service_role_key (length, not just undefined) and reject a non-empty jwt_secret shorter than 16 characters; unresolved env(...) literals are passed through verbatim as Go does (decode_hooks.go:15-26). - Gate the Authorization: Bearer header on a non-sb_ key prefix to match Go withAuthToken (pkg/fetcher/gateway.go:22), which sends only the apikey header for opaque sb_... keys (buckets.gateway.ts). Adds integration coverage for the Docker-host fallback, the sb_/JWT auth gate, empty-string key regeneration, and the <16-char jwt_secret error; updates SIDE_EFFECTS.md accordingly.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3862ec8999
ℹ️ 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".
Re-align the seed buckets SIDE_EFFECTS.md env-vars table with oxfmt; the table edited in 3862ec8 was committed unformatted and failed the "Check code quality" job (fmt:check). Doc-only, no behavior change.
…supabase/ Address Codex review threads on PR #5651: - Resolve a relative bucket objects_path under the supabase/ directory before walking it, matching Go's config resolver (pkg/config/config.go:757-759); absolute paths are used as-is. @supabase/config does not reproduce this resolve step and workdir is the project root, so objects_path = "./images" was looking under <root>/images instead of <root>/supabase/images. - Stream each object into the upload request via HttpClientRequest.bodyFile instead of buffering the whole file into a Uint8Array, matching Go's open-and-stream upload (pkg/storage/objects.go:94-127) and avoiding multi-GB allocations for large seed assets. Updates the object-walk integration tests for the supabase/ prefix, adds an absolute-objects_path case, and updates SIDE_EFFECTS.md.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c044de7481
ℹ️ 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".
| export function legacyContentTypeForPath(filePath: string): string { | ||
| const ext = nodePath.extname(filePath).toLowerCase(); | ||
| return CONTENT_TYPES[ext] ?? "application/octet-stream"; |
There was a problem hiding this comment.
For uploaded seed files whose extension is missing or not in this table, this falls back to application/octet-stream even when Go would sniff the first 512 bytes with http.DetectContentType (for example, an extensionless PNG is uploaded as image/png). Storage persists the supplied Content-Type, so these objects get different metadata and client behavior after the port; match Go by sniffing the file contents and only using extension lookup for generic text/plain.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid parity observation, escalating as a follow-up rather than fixing here. Go derives content type via http.DetectContentType (sniffing the first 512 bytes) with a mime.TypeByExtension override only for generic text/plain (pkg/storage/objects.go). The TS port deliberately uses an extension-only lookup (documented in buckets.upload.ts as a parity approximation) — matching Go requires porting Go's DetectContentType sniffing algorithm, which is a non-trivial, self-contained piece. Worth doing for strict metadata parity, but tracking separately.
| return yield* output | ||
| .promptConfirm(label, { defaultValue }) | ||
| .pipe(Effect.catchTag("NonInteractiveError", () => Effect.succeed(defaultValue))); |
There was a problem hiding this comment.
When stdin is non-interactive but contains an answer, such as printf 'n\n' | supabase seed buckets for an existing bucket, output.promptConfirm raises NonInteractiveError and this catch immediately returns the default true, so the bucket is overwritten despite the piped n. Go's Console.PromptYesNo still reads non-TTY stdin with a short timeout and parses y/n before falling back to the default, so this path needs to read stdin rather than treating all non-TTY runs as silent defaults.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real parity gap, but it lives in the shared legacy prompt infrastructure, not this command. Go's Console.PromptYesNo reads non-TTY stdin with a short timeout and parses y/n before falling back to the default; the TS promptConfirm/NonInteractiveError path treats every non-TTY run as "use the default". Fixing it correctly means changing the shared prompt service (it affects every legacy confirmation prompt, not just seed), so escalating it as a cross-cutting follow-up rather than special-casing seed.
| const configuredKey = auth.service_role_key; | ||
| return configuredKey !== undefined && configuredKey.length > 0 | ||
| ? configuredKey | ||
| : generateJwt(jwtSecret, "service_role"); |
There was a problem hiding this comment.
When auth.signing_keys_path is set and auth.service_role_key is omitted or empty, Go validates/loads that file during config load and generateJWT signs the service-role token with the first JWK; if the file is missing it fails before seeding. This fallback always creates an HS256 token from jwt_secret/the default secret, so asymmetric local stacks can reject the Storage request, and missing signing-key files are ignored while buckets may already be mutated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid for asymmetric local stacks, escalating as a follow-up. When auth.signing_keys_path is set and service_role_key is empty, Go loads the JWK file at config-load and signs the service-role token with the first key (failing if the file is missing); the TS port always HS256-signs from jwt_secret/the default secret. Matching this needs JWK loading + asymmetric signing support in @supabase/stack (today generateJwt is HS256-only), so it's a larger cross-package change for an uncommon local setup — tracking separately.
| } | ||
|
|
||
| // 1. Load config.toml. A parse failure aborts before any network call. | ||
| const loaded = yield* loadProjectConfig(cliConfig.workdir).pipe( |
There was a problem hiding this comment.
Fresh evidence beyond the earlier env-key fix is that this goes through @supabase/config's environment loader, which only reads supabase/.env and .env.local, while Go's loadNestedEnv also loads .env.${SUPABASE_ENV}.local and .env.${SUPABASE_ENV} (defaulting to development) from supabase/ and parent dirs. If auth.service_role_key = "env(SERVICE_ROLE_KEY)" is supplied by supabase/.env.development, Go resolves it before building the Storage client, but this leaves the literal env(...) and sends an invalid apikey.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real gap, but it belongs in @supabase/config's environment loader, not this command. Go's loadNestedEnv also loads .env.${SUPABASE_ENV}.local and .env.${SUPABASE_ENV} (default development) from supabase/ and parent dirs, whereas @supabase/config currently reads only .env/.env.local. Since that loader is shared by every config-consuming command, escalating it as a cross-cutting @supabase/config follow-up rather than working around it in seed.
| } | ||
|
|
||
| // 1. Load config.toml. A parse failure aborts before any network call. | ||
| const loaded = yield* loadProjectConfig(cliConfig.workdir).pipe( |
There was a problem hiding this comment.
loadProjectConfig prefers supabase/config.json whenever it exists and may ignore supabase/config.toml, but Go's seed path always loads utils.ConfigPath (supabase/config.toml). In a repo that has a next/experimental JSON config alongside the legacy TOML, seed buckets will now create/update buckets from the JSON file instead of the TOML file that Go users expect; load supabase/config.toml explicitly for this legacy command.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real gap in @supabase/config, escalating. loadProjectConfig prefers supabase/config.json when present, but Go's legacy commands always load supabase/config.toml (utils.ConfigPath). The right fix is in the shared loader (or a legacy-specific TOML-only load path) since it affects every legacy command that reads config, not just seed — so tracking it as a cross-cutting follow-up rather than a seed-local workaround.
…s enabled Go parity for two @supabase/config storage-schema gaps surfaced on PR #5651: - file_size_limit (storage-level and per-bucket) now accepts both a bare byte count and a human-readable string, normalizing a number to its decimal string, matching Go's sizeInBytes decoder (config_test.go TestFileSizeLimitConfigParsing). Previously a numeric value was rejected at decode time, aborting commands like seed buckets before they ran. - storage.vector.enabled now defaults to true when omitted, matching Go's merged config template (templates/config.toml + config_test.go:40). A partial config declaring [storage.vector.buckets.*] without an explicit enabled no longer silently skips vector seeding. analytics.enabled is left defaulting to false (the Go template sets it false).
Go's `seed buckets --linked` resolves the project ref and targets the remote
Storage API; the TS port was local-only, a regression. Implements the remote
path while keeping the local path login-free:
- Resolve the project ref via LegacyProjectRefResolver.loadProjectRef when
--linked is set (env SUPABASE_PROJECT_ID -> .supabase project-ref, hard
not-linked error), matching Go's LoadProjectRef. The short-circuit now keeps
the ref term (a ref present never short-circuits).
- Build a remote storage gateway at https://<ref>.<projectHost> with the
service-role key from SUPABASE_AUTH_SERVICE_ROLE_KEY or, failing that,
GET /v1/projects/{ref}/api-keys?reveal=true (internal/storage/client/api.go).
- Upsert analytics buckets (GET/POST/DELETE /storage/v1/iceberg/bucket) gated on
[storage.analytics].enabled && --linked, ordered buckets -> analytics ->
vector -> objects, with Go's exact stderr strings (pkg/storage/analytics.go).
Lazy auth: seed uses a dedicated runtime (legacySeedRuntimeLayer, modeled on
gen types) built on LegacyPlatformApiFactory so the access token is resolved
only when the --linked branch makes an API call — the local path requires no
login, matching Go. legacyGetProjectApiKeys now resolves the client via the
factory too (its other callers run under a runtime that exposes it).
…tack
Go's local storage client trusts the Kong CA (status.NewKongClient appends
Config.Api.Tls.CertContent to the cert pool, internal/storage/client/api.go),
so `seed buckets` works against a `[api.tls] enabled = true` local stack. The TS
port built an https:// URL but used the plain fetch client with no CA trust, so
the HTTPS request failed verification.
- @supabase/config exports KONG_LOCAL_CA_CERT, the embedded
pkg/config/templates/certs/kong.local.crt PEM (inline constant so it survives
the Bun compiled binary).
- On the local TLS path, the handler resolves the CA (api.tls.cert_path read
from disk when set, else the embedded cert; mirrors config.go:845-851) and
injects it via FetchHttpClient.Fetch (the same per-request override mechanism
legacy-http-dns.ts uses for DoH) as Bun's fetch `tls: { ca }`. Scoped to the
local --tls path only; remote --linked uses public certs.
Note: real-stack CA trust is not exercised by the test suite (the integration
harness mocks HttpClient, bypassing fetch); it requires a running
`supabase start` with TLS enabled.
…-validates Two file_size_limit parity gaps from review: - A bucket that omits file_size_limit (or sets 0) now inherits the storage-level [storage].file_size_limit, matching Go's resolve() (config.go:753-756). The per-bucket schema default (50MiB) previously won, so a custom storage-level limit was ignored. Presence is recovered from the raw document (same approach as the public tri-state). - All bucket sizes are parsed and validated up front, before the gateway is built and before any list/create/update — Go parses sizes at config-load, before NewStorageAPI, so an invalid file_size_limit fails with no Storage side effects (previously the lazy per-bucket parse could die mid-upsert). Adds integration coverage: storage-level inheritance into an omitting bucket, and an invalid later-bucket size failing before any Storage call.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5254b3a234
ℹ️ 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".
| } | ||
|
|
||
| // 1. Load config.toml. A parse failure aborts before any network call. | ||
| const loaded = yield* loadProjectConfig(cliConfig.workdir).pipe( |
There was a problem hiding this comment.
Even without --linked, Go's config load scans every [remotes.*] block and rejects duplicate project_ids or malformed remote refs before the Storage client is created. Because this load omits a projectRef, @supabase/config skips the duplicate-remote guard and its remote schema only requires a string, so supabase seed buckets can mutate local buckets from a config that Go would reject up front.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate on both counts — but this is a @supabase/config gap, not something seed buckets should fix locally, so escalating it as a cross-cutting follow-up.
Go runs two remote validations on every config.Load, independent of any ref:
- duplicate
project_idacross all[remotes.*]—config.go:503-518 - remote ref-format
^[a-z]{20}$—config.go:832-836(Validate, fromLoadatconfig.go:717)
On the TS side, @supabase/config has the duplicate guard (io.ts:128-166) but only runs it when a projectRef is supplied (io.ts:355), so a local loadProjectConfig(workdir) skips it; and remoteProjectId is bare Schema.String (base.ts:22-26) with no ^[a-z]{20}$ enforcement. Since every local-config command (db, gen, functions, start, …) shares this divergence, the right fix is in @supabase/config — run the duplicate scan unconditionally and add the ref-format check with Go's exact message — not a seed-local wart. Filing it as a separate config-package parity item; leaving this thread open for visibility. Shout if you'd rather block this PR on it.
Four parity gaps vs Go's config.Load (all settled before any Storage side
effect), from review:
- --linked merges the matching [remotes.<name>] block (project_id == ref) over
the base config before seeding (Go config.go:505-518). The ref is now resolved
before loadProjectConfig and passed as { projectRef }; @supabase/config already
implements the merge — this threads the option through ProjectConfigStore.load,
its layer, and the Bun wrapper.
- Bucket names are validated up front against Go's ValidateBucketName regex
(config.go:899-903,1382-1388), storage buckets only, with the byte-exact error,
before the gateway is built.
- The local service-role key honors SUPABASE_AUTH_JWT_SECRET /
SUPABASE_AUTH_SERVICE_ROLE_KEY (Go Viper AutomaticEnv, config.go:492-497) with
env > non-empty TOML > default precedence (the <16-char jwt_secret check
applies to the resolved secret). Full AutomaticEnv emulation remains a
cross-cutting @supabase/config follow-up.
- [api.tls] cert_path/key_path pairing + readability is validated before seeding
(Go config.go:845-861) with Go's exact messages; a TODO points at the broader
@supabase/config gap.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f26e879014
ℹ️ 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".
| const config = loaded.config; | ||
| const bucketsConfig = config.storage.buckets ?? {}; | ||
| const bucketNames = Object.keys(bucketsConfig); | ||
| const vectorEnabled = config.storage.vector.enabled; | ||
| const vectorBucketNames = Object.keys(config.storage.vector.buckets); | ||
| const hasVectorBuckets = vectorBucketNames.length > 0; |
There was a problem hiding this comment.
When the config contains a Go-invalid database setting such as db.major_version = 12, @supabase/config only decodes the number and this handler proceeds to Storage mutations. The Go seed path runs ParseDatabaseConfig before buckets.Run, and Config.Validate rejects Postgres 12 before NewStorageAPI, so this port can create/update buckets from a config the legacy command would fail up front; run the same db major-version validation before building the gateway.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate parity observation, but escalating rather than fixing seed-locally — this is the same class as the remotes-validation thread above: Go's Config.Validate rejects an unsupported db.major_version (e.g. 12) at config-load (before NewStorageAPI), and @supabase/config only schema-decodes the number without that validation.
db.major_version is unrelated to storage seeding, so a seed-local check would be a wart — the fix belongs in @supabase/config (port Go's Config.Validate db-version check so every config-loading command inherits it). Filing it with the other cross-cutting @supabase/config validation gaps (remotes duplicate/format, TLS pairing, AutomaticEnv); leaving this thread open for visibility. Shout if you'd rather block this PR on it.
… node wrapper Three more parity/consistency fixes from review: - The storage-level [storage].file_size_limit is now parsed once up front, unconditionally — Go unmarshals storage.FileSizeLimit at config.Load, so an invalid value aborts before any Storage call even when no bucket inherits it or only vector buckets are configured (previously only parsed lazily on inheritance). - --linked now writes the linked-project cache + fires org/project group identify via LegacyLinkedProjectCache.cache(ref), mirroring Go's root ensureProjectGroupsCached (cmd/root.go), gated on a non-empty resolved ref so the local path never writes it. - @supabase/config: the Node wrapper (node.ts) now forwards the LoadProjectConfigOptions added to ProjectConfigStore.load, keeping it in sync with the Bun wrapper so Node-based tooling also gets [remotes.*] merges.
There was a problem hiding this comment.
💡 Codex Review
Prefix the hoisted legacy size-unit exports
By hoisting these helpers into src/legacy/shared, these bare exports now violate apps/cli/AGENTS.md, which requires every exported token from legacy/ to carry a Legacy/legacy prefix with no exceptions. Leaving ramInBytes/intToUint/bytesSize unprefixed in shared legacy infrastructure reintroduces the auto-complete/import ambiguity that rule is meant to prevent; rename the exports and update the new imports accordingly.
ℹ️ 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".
| // Mirrors Go's `(*api).Validate` + `newLocalClient` | ||
| // (`apps/cli-go/pkg/config/config.go:845-861`, | ||
| // `apps/cli-go/internal/storage/client/api.go:30-37`). | ||
| const localTls = projectRef === "" && config.api.tls.enabled; |
There was a problem hiding this comment.
Fresh evidence beyond the earlier TLS-validation thread is that this guard still restricts api.tls validation to local runs. Go reaches Config.Validate from ParseDatabaseConfig before NewStorageAPI for both local and --linked (apps/cli-go/pkg/config/config.go:845-861), so with supabase seed buckets --linked and a config such as [api.tls] enabled = true with a missing/unreadable cert/key, this path skips the config-load error and can create/update remote buckets from a config the Go command rejects up front.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate parity observation — escalating rather than fixing seed-locally, because this is whole-config validation that Go runs in config.Load/Config.Validate for every command (not a storage concern), and the right home is @supabase/config.
Go reaches Config.Validate (config.go:845-861) for both local and --linked before NewStorageAPI, so a bad [api.tls] cert/key pair should also fail on --linked. The seed handler only validates it on the local path today (where it also needs the CA). This belongs with the rest of Go's Config.Validate in @supabase/config so it runs target-independently for all commands. Tracking in the consolidated config-Validate backlog; leaving open.
| if (projectRef === "") { | ||
| baseUrl = resolveLocalBaseUrl(config); | ||
| apiKey = yield* resolveLocalServiceRoleKey(config.auth); |
There was a problem hiding this comment.
When --linked is used with an invalid local auth config such as auth.jwt_secret = "short", Go still runs Auth.generateAPIKeys during config.Load (apps/cli-go/pkg/config/apikeys.go:43-48) and aborts before constructing the remote Storage client. This branch only calls resolveLocalServiceRoleKey on local runs, so the linked path ignores that validation, fetches a remote service key, and can mutate buckets even though the legacy command would fail before any Storage call.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate parity observation — escalating rather than fixing seed-locally, because this is whole-config validation that Go runs in config.Load/Config.Validate for every command (not a storage concern), and the right home is @supabase/config.
Go runs Auth.generateAPIKeys (apikeys.go:43-48, incl. the < 16 jwt_secret check) during config.Load regardless of target, so --linked with jwt_secret = "short" should fail before any Storage call. The TS port only runs that validation inside the local key derivation. Like the TLS check, this is whole-config validation that should live in @supabase/config (target-independent), not be duplicated per-path in the seed handler. Tracking in the consolidated backlog; leaving open.
| const storageFileSizeLimitBytes = yield* parseFileSizeLimitOrFail( | ||
| config.storage.file_size_limit, | ||
| ); |
There was a problem hiding this comment.
This is separate from the auth override fix: Go's config loader uses Viper AutomaticEnv with the SUPABASE_ prefix, so SUPABASE_STORAGE_FILE_SIZE_LIMIT=5MiB overrides [storage].file_size_limit before omitted/zero bucket limits inherit it. Here the inherited value is read only from the decoded TOML/default config, so buckets that omit file_size_limit (or set 0) are created/updated with the wrong limit whenever that supported env override is used.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate parity observation — escalating rather than fixing seed-locally, because this is whole-config validation that Go runs in config.Load/Config.Validate for every command (not a storage concern), and the right home is @supabase/config.
Same root cause as the auth-env item: Go's loader uses Viper AutomaticEnv + SUPABASE_ prefix (config.go:492-497), so SUPABASE_STORAGE_FILE_SIZE_LIMIT overrides [storage].file_size_limit before the inherit step. @supabase/config only does env(...) interpolation, not AutomaticEnv. A complete fix emulates AutomaticEnv for every config key in @supabase/config (it affects all commands) rather than reading one more env var in the seed handler. Tracking with the AutomaticEnv item in the consolidated backlog; leaving open.
| if (loaded === null) { | ||
| return; | ||
| } | ||
| const config = loaded.config; |
There was a problem hiding this comment.
After decoding, this path does not run Go's full Config.Validate; with [api] port = 0 and a valid external_url plus configured buckets, Go fails with Missing required field in config: api.port before NewStorageAPI, but this handler later accepts the external_url and can create/update buckets from the invalid config. Validate api.port (while api.enabled is true) before the no-op check or any gateway construction.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate parity observation — escalating rather than fixing seed-locally, because this is whole-config validation that Go runs in config.Load/Config.Validate for every command (not a storage concern), and the right home is @supabase/config.
Go's Config.Validate rejects api.port = 0 (Missing required field in config: api.port) when api.enabled, before NewStorageAPI; @supabase/config only schema-decodes. This is the same class as the db.major_version and remotes-validation items — whole-config Validate that should be ported into @supabase/config so every command inherits it. Tracking in the consolidated backlog; leaving open.
| // merged for --linked. A parse failure aborts before any network call. | ||
| const loadOptions: LoadProjectConfigOptions | undefined = | ||
| projectRef !== "" ? { projectRef } : undefined; | ||
| const loaded = yield* loadProjectConfig(cliConfig.workdir, loadOptions).pipe( |
There was a problem hiding this comment.
Go's config decoder includes mapstructure.StringToSliceHookFunc(","), so a bucket config like allowed_mime_types = "image/png,image/jpeg" is valid and reaches UpsertBuckets as a slice. This strict loadProjectConfig path still requires allowed_mime_types to be an array, so the native command rejects a Go-supported seed config before doing any work; accept the string form and split it in the storage schema.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate, but escalating rather than fixing seed-locally — this is a @supabase/config decode-behavior gap, not a seed concern. Go's config decoder installs mapstructure.StringToSliceHookFunc(","), so allowed_mime_types = "image/png,image/jpeg" is split into a slice for every string-slice config field, across every command. @supabase/config models it as a strict array. The right fix is to add the comma-split decode hook in the config package (so all commands accept the Go-supported string form), not to special-case the seed schema. Tracking with the other @supabase/config parity items; leaving open.
…ParseJSON) Go decodes the bucket/vector/analytics list responses via fetcher.ParseJSON (json.Decode into the typed target), so a 200 body whose shape doesn't match — a non-array (misrouted gateway / HTML), a non-object element, or a present-but-wrong-typed field — aborts before any bucket mutation. The TS gateway treated a non-array as an empty list and coerced bad entries to "", silently proceeding to create every configured bucket. The three list decoders now strict-decode to match Go: fail on a non-matching top-level type, a non-object element, or a wrong-typed name/id/vectorBucketName, while tolerating missing fields / null / empty / extra keys (zero values). The former "tolerates malformed entries" test asserted the opposite of Go (success + POST on a body Go rejects); it's split into a tolerated missing-field case and failure cases (bad element, non-array) asserting no mutation occurs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cec81c19ea
ℹ️ 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".
| const certContent = yield* fs.readFileString(absCert).pipe( | ||
| Effect.catchTag( | ||
| "PlatformError", | ||
| (cause) => |
There was a problem hiding this comment.
When api.tls.cert_path or key_path is absolute on a local TLS seed, Go still rewrites it with path.Join(SupabaseDirPath, value) during config resolve (apps/cli-go/pkg/config/config.go:795-800), so /tmp/kong.crt is read as supabase/tmp/kong.crt and a missing file aborts before Storage calls. This branch reads the absolute path directly, so a config the legacy command rejects can proceed to create/update buckets; apply the same Supabase-dir join for TLS paths before validation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Escalating — this is Go config-resolve path behavior that belongs in @supabase/config. Go rewrites api.tls.cert_path/key_path with filepath.Join(SupabaseDirPath, value) during config resolve (config.go:795-800), so even an absolute /tmp/kong.crt is read relative to supabase/ and a missing file aborts at config-load. The seed handler reads cert_path directly (with an isAbsolute shortcut) because @supabase/config doesn't resolve/read TLS cert content at all yet — the same gap behind the existing TODO in validateLocalKongTls. The faithful fix is to port Go's cert-path resolve (and the cert/key validation) into @supabase/config's load path, so this isn't reimplemented per-command. Tracking with the config-Validate/resolve backlog; leaving open.
| const info = yield* fs.stat(absRoot); | ||
| if (info.type === "Directory") { | ||
| return yield* collectDir(fs, path, output, absRoot, displayRoot); |
There was a problem hiding this comment.
When a bucket's objects_path itself is a symlink to a directory, Go's fs.WalkDir visits that root as a symlink and isUploadableEntry skips it as non-regular, so none of the target directory's files are uploaded. This uses stat on the root and then descends when the target is a directory, which can upload an entire linked directory tree that the legacy command would skip.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I traced this against Go and the current behavior is actually correct — Go does follow a symlinked objects_path root and uploads the target directory's files, so I'm leaving the code as-is (and adding a test to lock it in, d4532c6b).
io/fs.WalkDir is documented to follow a symlinked root: "WalkDir does not follow symbolic links found in directories, but if root itself is a symbolic link, its target will be walked" (io/fs/walk.go). Mechanically it does info := Stat(fsys, root) and builds the root DirEntry from that; fs.Stat falls back to Open(name).Stat() for a non-StatFS (io/fs/stat.go:20-31), and the CLI's rootFS implements only Open (internal/utils/config.go:197-207) → afero.OsFs.Open → os.Open, which follows symlinks. So the root DirEntry is a directory, WalkDir descends, and the target's files upload; isUploadableEntry's symlink branch only ever runs for nested entries (from ReadDir/lstat).
The TS collectFiles mirrors this exactly — fs.stat(absRoot) follows the root symlink, sees a directory, and descends, while nested symlinked dirs are still skipped via the readLink no-follow detector. I added an integration test asserting a symlinked objects_path uploads its target's files. So this is parity-correct as written; leaving the thread open in case you'd like to weigh in.
| for (const entry of list) { | ||
| const obj = asObject(entry); | ||
| const name = obj === null ? null : decodeStringField(obj, "vectorBucketName"); | ||
| if (name === null) { | ||
| return yield* Effect.fail(failParse("invalid vector bucket entry")); | ||
| } | ||
| names.push(name); |
There was a problem hiding this comment.
When ListVectorBuckets returns an entry with a malformed creationTime (for example a string), Go's fetcher.ParseJSON[ListVectorBucketsResponse] fails to unmarshal the uint64 field before any vector create/prune calls. This decoder only checks vectorBucketName, so it can continue with creates or deletes after a list response the legacy command rejects; validate the full Go response shape even for fields not otherwise used.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving this as-is — it's a technically-real but behaviorally-irrelevant divergence I don't think is worth reproducing. Go's ParseJSON[ListVectorBucketsResponse] would indeed fail to unmarshal a malformed creationTime (string into uint64), but seed buckets never reads creationTime — only vectorBucketName drives any create/prune decision. The only way to hit this is the local Storage server returning a malformed creationTime in an otherwise-valid list (a server bug, not a user config), and the sole observable effect would be erroring slightly earlier on a response we'd otherwise process identically.
Reproducing full struct-shape validation for unused numeric fields is gold-plating with no behavioral parity benefit, so I'm intentionally scoping the strict decode to the fields that affect behavior (the same call I flagged when adding it). Happy to revisit if you have a concrete case where it matters; leaving open for visibility.
…(Go parity) Go's fetcher.NewServiceGateway installs WithExpectedStatus(http.StatusOK) (pkg/fetcher/gateway.go:17) and Send rejects any status not exactly 200, for every seed route (list/create/update buckets, vector, analytics, object upload). The TS `send` accepted the whole 2xx range, so a 201/204 from an incompatible route would pass where Go aborts. Tightened the gate to `status !== 200`, reusing the existing `Error status <d>: <body>` status error. Also adds an integration test confirming a symlinked objects_path root uploads its target's files — Go's io/fs.WalkDir follows a symlinked root (only nested symlinks are skipped), which the TS collectFiles already matches; locking it in.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4532c6bfc
ℹ️ 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 { legacySeedBuckets } from "./buckets.handler.ts"; | ||
|
|
||
| const config = { | ||
| linked: Flag.boolean("linked").pipe(Flag.withDescription("Seeds the linked project.")), |
There was a problem hiding this comment.
Go registers --linked and --local as persistent flags on seed (apps/cli-go/cmd/seed.go:29-32), so existing invocations like supabase seed --linked buckets are valid. Here the TS seed parent has no flag config and these flags are attached only to the buckets leaf, so the parent-position form has no flag to consume before the handler runs; only supabase seed buckets --linked is supported. Move or share these flags at the seed command level to preserve the Go command surface.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate — Go registers --linked/--local as persistent flags on the seed parent (cmd/seed.go:29-32), so supabase seed --linked buckets is valid, whereas the TS attaches them to the buckets leaf, supporting only supabase seed buckets --linked. Escalating rather than fixing inline, because the faithful fix is a command/flag-wiring change with parser-level behavior I can't cheaply verify in the loop: declare the two flags via Command.withGlobalFlags([...]) on the seed group (the Effect equivalent of Go's PersistentFlags — position-independent + scoped to seed), then refactor the buckets handler to read them as global-flag tokens instead of the leaf Command.make("buckets", config) arg, and rethread them into withLegacyCommandInstrumentation({ flags }) for telemetry. (The mutual-exclusivity check already scans argv via legacySeedChangedTargetFlags, so it's position-independent today.) Tracking as a CLI-surface follow-up; leaving open.
| if (loaded === null) { | ||
| return; |
There was a problem hiding this comment.
When --linked is used from a workdir without supabase/config.toml/config.json, loadProjectConfig returns null and this exits before fetching the service key or calling Storage. The Go path still loads embedded defaults when the config file is absent; because the project ref is non-empty, buckets.Run does not short-circuit and NewStorageAPI/ListBuckets runs, surfacing missing-token or Storage errors. This native path can therefore report success for a linked invocation the legacy command would fail or at least contact the remote project for.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate, and escalating — this is @supabase/config Load behavior, not seed-specific. Go's config.Load falls back to embedded defaults when no config.toml exists, so seed buckets --linked with a non-empty ref still builds the Storage client and runs ListBuckets (surfacing auth/Storage errors) rather than short-circuiting. @supabase/config's loadProjectConfig returns null when no config file is present, so the handler exits early. Reproducing Go's "always have a config (defaults)" belongs in the config package (it affects every command that loads config without a file), alongside the other config.Load/Validate parity items I've flagged. Tracking there; leaving open.
| const names: Array<string> = []; | ||
| for (const entry of parsed) { | ||
| const obj = asObject(entry); | ||
| const name = obj === null ? null : decodeStringField(obj, "name"); |
There was a problem hiding this comment.
Fresh evidence beyond the earlier list-decoder thread is that the analytics decoder still only checks name. If /storage/v1/iceberg/bucket returns an entry with malformed id, created_at, or updated_at (for example a numeric id), Go's fetcher.ParseJSON[[]AnalyticsBucketResponse] fails before any analytics create/prune calls, but this path accepts the response and can continue to delete or create analytics buckets after a response the legacy command rejects.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving this as-is for the same reason as the vector creationTime thread — it's a technically-real but behaviorally-irrelevant divergence on unused fields. seed buckets reads only name from the analytics list (to compute create/prune); id/created_at/updated_at are never used. Go's ParseJSON[[]AnalyticsBucketResponse] would indeed reject a malformed id, but the only trigger is the local Storage server returning a malformed-but-otherwise-valid list (a server bug, not user config), and the sole effect would be erroring slightly earlier on a response we'd otherwise process identically.
I strict-decode the fields that drive behavior (name here; name+id for buckets, where id is used as the update target; vectorBucketName for vector) and intentionally don't gold-plate unused fields. Happy to revisit with a concrete case where it matters; leaving open for visibility.
| const configuredKey = envKey !== undefined && envKey.length > 0 ? envKey : auth.service_role_key; | ||
| return configuredKey !== undefined && configuredKey.length > 0 | ||
| ? configuredKey | ||
| : generateJwt(jwtSecret, "service_role"); |
There was a problem hiding this comment.
When a local config omits auth.service_role_key or sets it to an empty string, Go generates the deterministic default service-role key with issuer supabase-demo and the fixed expiry asserted in apps/cli-go/pkg/config/config_test.go; the legacy start command is still wrapped, so local Kong is commonly configured with that exact key. This call uses @supabase/stack's generator instead, which includes the current time and issuer supabase, so the apikey sent by seed buckets can differ from the key the running Go-started local gateway accepts and fail with auth errors before any seeding.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Traced this against the running local stack and it's actually fine as-is — leaving it. Your facts are right (Go's default key is deterministic: iss=supabase-demo, fixed exp=1983812996, no iat, per pkg/config/apikeys.go:18,34,76; @supabase/stack's generateJwt uses iss=supabase + current-time exp), but byte-parity isn't required because the local stack validates the JWT cryptographically, not by string match.
start hands the Storage container the raw HS256 secret (AUTH_JWT_SECRET) and its JWKS (JWT_JWKS) (internal/start/start.go:947-948); Kong forwards seed's Authorization: Bearer <jwt> verbatim (start.go:459) and strips apikey. Storage/PostgREST/GoTrue verify the signature against that secret and read the role claim — SERVICE_KEY is only the storage-api's own internal admin client, not an inbound gate. Decisive: for unauthenticated requests Kong mints Bearer <AnonKey> (start.go:463), a different JWT than SERVICE_KEY; if it were string-compared, no anon request could ever authorize. So any validly-signed, non-expired service_role JWT (same secret) is accepted — iss/exp/iat differences are immaterial.
(The one genuinely different case is auth.signing_keys_path → asymmetric RS256/ES256 signing, where the HS256 generateJwt wouldn't verify — that's a real separate gap already tracked in the escalated @supabase/config/auth backlog, not this symmetric default.) Leaving open for visibility.
| // merged for --linked. A parse failure aborts before any network call. | ||
| const loadOptions: LoadProjectConfigOptions | undefined = | ||
| projectRef !== "" ? { projectRef } : undefined; | ||
| const loaded = yield* loadProjectConfig(cliConfig.workdir, loadOptions).pipe( |
There was a problem hiding this comment.
For --workdir pointing at a subdirectory of a project, Go changes into exactly that directory and LoadConfig reads only ./supabase/config.toml, so an absent config there falls back to defaults. loadProjectConfig walks upward from cliConfig.workdir, so the same invocation can find the parent project's config and create/update buckets in a project that Go would not seed from; load the workdir-local legacy config path instead of doing parent discovery for this command.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate, and escalating — this is @supabase/config's project-discovery behavior, shared by every command. Go's --workdir chdirs into exactly that directory and LoadConfig reads only ./supabase/config.toml (no upward search), whereas loadProjectConfig walks upward from the workdir and can find a parent project's config. So with --workdir pointing at a subdirectory, the TS path can seed from a parent project's config that Go would not. The fix is to make @supabase/config honor an explicit workdir as the exact config root (no parent discovery) — or expose a workdir-local load path — which is cross-cutting config-package work, not a seed-local change. Tracking with the config backlog; leaving open.
… port conflicts
Two Go-parity gaps from review:
- Go's CreateBucket/UpdateBucket decode the 200 body via fetcher.ParseJSON into
{name}/{message} (pkg/storage/buckets.go:46,65) and fail on a non-JSON/empty
body before later uploads; the TS discarded the body with Effect.asVoid. Now
the create/update responses are validated (fail with "failed to parse response
body" on a non-object body), matching Go. Scoped to create/update only —
vector/analytics/upload discard the body in Go too, so those stay asVoid.
- Go's localGatewayHint (pkg/fetcher/http.go:117-143) appends a port-conflict
recovery hint on a local transport failure. The gateway now appends the
byte-identical hint when the local api.port is set and the failure is an Effect
TransportError (the structural analogue of Go's net/http string match, which
Bun doesn't emit). Gated to the local stack only — never fires for --linked.
There was a problem hiding this comment.
💡 Codex Review
Use a full numeric parse for size limits
When a config contains a malformed numeric prefix like file_size_limit = "1.2.3MiB", Number.parseFloat accepts the leading 1.2 and ignores the second dot, so seed buckets can create/update buckets with the wrong byte limit instead of rejecting the config. Go's docker/go-units path parses the entire numeric portion and fails during config load for this input, so this parser should verify that the whole num string was consumed before accepting it.
ℹ️ 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".
…nked api-keys
Two Go-parity fixes from review:
- The strict vector-list decoder now treats `{"vectorBuckets": null}` as an empty
list (Go decodes it into a nil slice), instead of failing — a config with
vector buckets no longer aborts when the service returns a null list.
- On `--linked`, when the Management API returns no api keys,
`legacyExtractServiceKeys` yields an empty service-role key; the handler now
fails with "Anon key not found." before building the remote Storage client,
mirroring Go's tenant.GetApiKeys → errMissingKey (IsEmpty when both anon and
service_role are absent), rather than issuing unauthenticated Storage calls.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97a2417db6
ℹ️ 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".
| * which stores whatever is sent. Unknown extensions fall back to | ||
| * `application/octet-stream`. | ||
| */ | ||
| export function legacyContentTypeForPath(filePath: string): string { |
There was a problem hiding this comment.
When objects_path contains files with no extension, an unknown extension, or a misleading extension, this extension-only helper sends application/octet-stream or the extension's type instead of Go's content sniffed type. Go reads the first 512 bytes with http.DetectContentType and only uses mime.TypeByExtension to refine generic text/plain (apps/cli-go/pkg/storage/objects.go:66-109), so common seeded objects like extensionless PDFs/images or .md text files now get different Storage Content-Type metadata and can be served incorrectly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real but deliberately deferred parity gap — escalating as a follow-up rather than fixing inline. Go derives the object Content-Type via http.DetectContentType (sniffing the first 512 bytes), refined by mime.TypeByExtension only for generic text/plain (pkg/storage/objects.go:66-109); the TS port uses an extension-only lookup (documented as an approximation in buckets.upload.ts). Matching Go requires porting Go's DetectContentType sniffing algorithm (a specific 512-byte signature table) — a self-contained but non-trivial chunk that affects only stored Content-Type metadata for extensionless/misnamed files. Worth doing for strict metadata parity, but tracking it as its own follow-up. Leaving open.
| const absRoot = path.isAbsolute(objectsPath) | ||
| ? objectsPath | ||
| : path.join(workdir, "supabase", objectsPath); | ||
| const files = yield* collectFiles(fs, path, output, absRoot, displayRoot); |
There was a problem hiding this comment.
When a bucket's objects_path contains a large asset tree, this first builds an array for every uploadable file before starting any upload. Go's UpsertObjects streams fs.WalkDir directly into a bounded job queue (apps/cli-go/pkg/storage/batch.go:81-123), so memory stays bounded and uploads begin during traversal; the TypeScript path can spike memory or appear hung/OOM on large seed directories.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving this as-is — the file contents are already streamed, and the discovery array is not a meaningful memory risk, so this is a perf micro-optimization rather than a parity divergence.
The actual large-payload concern (uploading multi-GB files) was already fixed: uploadObject streams each file via HttpClientRequest.bodyFile rather than buffering it. What remains here is collectFiles building an array of file paths (strings) before uploading — bounded by file count, not size, so a normal seed tree is kilobytes of paths, not gigabytes. The uploaded result is identical to Go's; only the timing of the first upload (after traversal vs during) differs, and the objects are uploaded in the same lexical order with the same concurrency (5). Converting discovery to a streaming bounded queue would be a sizable collectFiles refactor for no behavioral parity gain at seed-data scale, so I'm not taking it on. Leaving open for visibility.
…ed size numerals Two Go-parity fixes from review: - resolveLocalBaseUrl now brackets an IPv6 host when building the local gateway URL (`http://[::1]:54321`), matching Go's net.JoinHostPort (config.go:636-638); a bare `http://::1:54321` is an invalid URL, so local seeding against an IPv6-resolved host previously failed. - ramInBytes now validates the numeric part strictly (single decimal, no extra dots/spaces) before parsing, matching Go's strconv.ParseFloat via docker/go-units.RAMInBytes. JS Number.parseFloat silently parsed prefixes like "1.2.3" (→1.2) or "1 2" (→1); Go rejects the whole config before NewStorageAPI. Shared helper — config push inherits the same stricter parsing.
There was a problem hiding this comment.
💡 Codex Review
Lines 355 to 356 in 91274c0
Match remotes before env interpolation
When a remote declares project_id = "env(PREVIEW_REF)" and PREVIEW_REF is set to the target ref, this applies that remote override because interpolation already ran. Go chooses the matching [remotes.*] block with v.GetString("remotes.<name>.project_id") before the LoadEnvHook decode step, so it sees the literal env(PREVIEW_REF) and does not merge the override; this can seed buckets using remote-only settings that the legacy command would not apply.
ℹ️ 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".
| return configuredKey !== undefined && configuredKey.length > 0 | ||
| ? configuredKey | ||
| : generateJwt(jwtSecret, "service_role"); |
There was a problem hiding this comment.
Fresh evidence beyond the prior JWT-shape thread is the asymmetric signing-key path: when local config sets auth.signing_keys_path and omits auth.service_role_key, Go loads the JWKs during config validation and generateAPIKeys signs the generated service-role token with the first signing key. This fallback always signs an HS256 token from jwt_secret/the default secret, so a stack started from that asymmetric config can reject the apikey/Bearer token before any bucket seeding succeeds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate, and escalating with the other asymmetric-auth item — this needs @supabase/config + @supabase/stack support, not a seed-local change. When auth.signing_keys_path is set and service_role_key is omitted, Go loads the JWKs at config validation and generateAPIKeys signs the service-role token with the first signing key (asymmetric RS256/ES256). The TS local key derivation always HS256-signs from jwt_secret/the default secret via @supabase/stack's generateJwt, which has no asymmetric path — so a stack started from an asymmetric config would reject the token. The fix requires @supabase/config to load/validate signing_keys_path and @supabase/stack to sign with a JWK, which is cross-package work beyond this port. Tracking with the config/auth backlog; leaving open. (The default symmetric path is verified-correct — see the JWT-shape thread.)
| // merged for --linked. A parse failure aborts before any network call. | ||
| const loadOptions: LoadProjectConfigOptions | undefined = | ||
| projectRef !== "" ? { projectRef } : undefined; | ||
| const loaded = yield* loadProjectConfig(cliConfig.workdir, loadOptions).pipe( |
There was a problem hiding this comment.
When supabase/config.json exists (especially alongside a valid supabase/config.toml), this call uses loadProjectConfig, whose loader prefers JSON and ignores TOML. The Go seed path loads only supabase/config.toml/embedded defaults, so local or linked seed buckets can create/update buckets from a JSON config that the legacy command would ignore, or fail on JSON while Go would use TOML.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate, and escalating — this is @supabase/config loader behavior shared by every command, not seed-specific. loadProjectConfig prefers supabase/config.json when present, whereas Go's legacy commands always load supabase/config.toml (utils.ConfigPath). The faithful fix is in the config package — a legacy/TOML-only load path (or making the loader prefer config.toml for the legacy shell) — so it applies uniformly to every ported command, rather than a seed-local workaround. Tracking with the config-loader backlog; leaving open.
| return yield* output | ||
| .promptConfirm(label, { defaultValue }) | ||
| .pipe(Effect.catchTag("NonInteractiveError", () => Effect.succeed(defaultValue))); |
There was a problem hiding this comment.
When an existing bucket is overwritten or a vector/analytics bucket is pruned in an interactive text run, this delegates to the shared Clack confirmation UI. Go's PromptYesNo writes the literal line-oriented prompt (<label> [Y/n] or [y/N]) to stderr and reads a plain line, so the native seed command now produces different prompt bytes and interaction behavior in exactly the confirmation paths that legacy parity is meant to preserve.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate, and escalating — this is shared legacy prompt infrastructure, not seed-specific. Go's console.PromptYesNo writes the literal line-oriented prompt (<label> [Y/n] / [y/N]) to stderr and reads a plain line, whereas the legacy shell routes confirmations through the shared Clack UI. Matching Go's exact prompt bytes/interaction belongs in the shared prompt service (it affects every legacy confirmation prompt, e.g. db reset, link, not just seed's overwrite/prune), so it should be one cross-cutting change rather than re-implemented per command. Tracking it as shared prompt-infra work; leaving open. (Non-interactive/--yes/json behaviour already matches Go — this is specifically the interactive-TTY byte rendering.)
…gateway hint Two follow-ups from review (one fixing a regression from 91274c0): - ramInBytes' strict numeric check `^\d+(\.\d+)?$` over-rejected Go-valid forms (.5, 1., 1e6, +5). docker/go-units v0.5.0 hands the numeric part to strconv.ParseFloat, which accepts a leading/trailing dot, exponent and sign; the regex now matches that grammar (`^[+-]?(\d+\.?\d*|\.\d+)([eE][+-]?\d+)?$`) while still rejecting the prefix hazards (1.2.3, 1 2, leading space, 0x10). Negatives stay rejected post-parse. Shared with config push. - The local-gateway port-conflict hint now derives the host/port from the actual base URL (Go's localGatewayHint parses the server URL): it only fires for a loopback host and reports THAT URL's port, so an `api.external_url` override with a different port or a non-loopback host no longer shows a wrong/spurious hint. Replaces the api.port-based gating.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b332a6232c
ℹ️ 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".
| } else { | ||
| baseUrl = `https://${projectRef}.${cliConfig.projectHost}`; | ||
| const envKey = process.env["SUPABASE_AUTH_SERVICE_ROLE_KEY"]; | ||
| if (envKey !== undefined && envKey.length > 0) { | ||
| apiKey = envKey; | ||
| } else { | ||
| const keys = legacyExtractServiceKeys(yield* legacyGetProjectApiKeys(projectRef, true)); |
There was a problem hiding this comment.
On a --linked run, Go's root pre-run takes the linked branch of ParseDatabaseConfig before buckets.Run, including LoadProjectRef, config validation, and NewDbConfigWithPassword; if the DB password/temp role cannot be resolved or direct IPv6/pooler probing fails, Go aborts before any Storage mutations. This remote branch skips that DB-resolution step and goes straight to Management API keys/Storage, so it can create or update remote buckets in environments where the legacy command would fail before entering the seed handler.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate parity observation, but escalating — this is Go's shared root/seed pre-run resolving the DB connection for every seed subcommand, not something seed buckets (which only touches Storage) should reimplement. On --linked, Go's ParseDatabaseConfig runs LoadProjectRef + NewDbConfigWithPassword (password/temp-role resolution, IPv6/pooler probing) before buckets.Run, so an environment where the DB connection can't be resolved aborts before any Storage mutation — even though buckets never uses the DB. Reproducing that means pulling the entire linked-DB-config-resolution stack into the seed runtime purely as a fail-early side effect; it belongs in a shared seed/root pre-run parity effort (alongside the broader ParseDatabaseConfig work), not bolted into the buckets handler. The project-ref resolution that seed buckets does need is already done. Tracking the DB-resolution side effect as a separate item; leaving open.
…ect overflow size Three Go-parity fixes from review: - Target selection now keys off the changed-flag set (same source as the mutual-exclusivity check), not the parsed boolean, matching Go's flag.Changed precedence in ParseDatabaseConfig (db_url.go:46-63): `--linked=false` is still the linked path in Go. Previously the value-check wrongly went local. - The no-op short-circuit now emits an empty summary via output.success in json/stream-json mode, so scripted callers get a result object instead of empty stdout (the json modes are additive; text mode still emits nothing). - ramInBytes rejects a non-finite parsed size (e.g. `1e309` → Infinity), matching Go's strconv.ParseFloat range error, instead of letting Infinity flow into the request body as null. Shared with config push.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5725ca81f6
ℹ️ 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".
| * numeric value is normalized to its decimal string so the decoded type stays a | ||
| * `string` for all consumers (`ramInBytes` parses either form identically). | ||
| */ | ||
| const fileSizeLimit = Schema.Union([Schema.String, Schema.Number]).pipe( |
There was a problem hiding this comment.
This change makes file_size_limit accept bare numeric byte counts, but the checked-in public schema is still stale: repo-wide search shows apps/docs/public/cli/config.schema.json still declares both storage-level and bucket-level file_size_limit as only "type": "string" (for example around lines 3086 and 3118). Users and editors consuming that schema will reject a Go/CLI-valid config such as file_size_limit = 5000000 until the generated schema is updated with this source schema change.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
You're right that the published schema doesn't yet reflect the numeric form — but I'm escalating the regeneration rather than bundling it here, because apps/docs/public/cli/config.schema.json is a generated artifact that has already drifted from source. Regenerating it (via apps/cli/scripts/generate-docs.ts) produces a 458-line diff that is mostly unrelated to this change: an effect-Schema version bump has changed how unions/literals serialize (many anyOf wrappers collapse, enum representations for db.major_version/pooler.mode/network_restrictions etc. change), and Schema.Number even emits a {"type":"string","enum":["NaN"]} branch. The source change here is correct (storage.ts now accepts number | string); the published JSON schema will pick up the numeric type the next time it's regenerated.
Bundling a 458-line generated-file regen (dominated by drift the team hasn't regenerated yet) into this seed-buckets PR would be noisy and conflate concerns, so I'm flagging it for a dedicated config.schema.json regeneration pass (or whatever CI/build step owns that artifact) instead. Leaving open for that.
…at parity) Go's strconv.ParseFloat (via docker/go-units RAMInBytes) accepts underscores between digits in numeric literals (Go 1.13+ rule), so `file_size_limit = "1_000MiB"` parses as 1000 MiB. The strict numeric regex rejected it. The grammar now allows single underscores between digits (`\d(?:_?\d)*`, no leading/trailing/ doubled), and the underscores are stripped before Number.parseFloat (which would otherwise stop at the first `_`). Verified empirically against go-units v0.5.0. Shared with config push.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d8ac8ae24
ℹ️ 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".
| // leading-space, `0x10`, `_1`, `1_`). A negative value is rejected post-parse | ||
| // below (matching Go's `size < 0` check); `1e309`→Infinity by the isFinite check. | ||
| if ( | ||
| !/^[+-]?(?:\d(?:_?\d)*(?:\.(?:\d(?:_?\d)*)?)?|\.\d(?:_?\d)*)([eE][+-]?\d(?:_?\d)*)?$/.test(num) |
There was a problem hiding this comment.
Fresh evidence beyond the earlier numeric-limit fixes is that strconv.ParseFloat also accepts Go hexadecimal float syntax, and docker/go-units.RAMInBytes passes the numeric part through to it. With this regex, a legacy-valid string such as file_size_limit = "0x1p10MiB" is rejected before seed buckets or config-push size diffing runs, while the Go CLI parses it as 1 GiB; include Go's hex-float grammar or avoid pre-filtering those forms before parsing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving this as-is — it's a technically-real but behaviorally-irrelevant divergence I don't think is worth chasing. Go's strconv.ParseFloat does accept hex-float syntax (0x1p10 → 1024), so file_size_limit = "0x1p10MiB" parses in Go. But:
- No real
file_size_limitconfig uses Go hex-float byte sizes — the realistic forms (decimal5000000, human-readable50MiB, and now underscores1_000) are all handled. - JS can't even value a hex float natively:
Number.parseFloat("0x1p10")→ 0 andNumber("0x1p10")→ NaN. So accepting the syntax isn't enough — matching Go's value would require portingstrconv.ParseFloat's full hex-float grammar (mantissa + binarypexponent) by hand, which is a disproportionate amount of code for a form nobody writes in a size config.
So I'm intentionally scoping the numeric validation to the decimal/exponent/underscore forms that real configs use, and rejecting hex floats (fail-fast) rather than mis-valuing them. If a concrete config ever needs hex-float sizes, the right fix is a full strconv.ParseFloat port in legacy-size-units.ts. Leaving open for visibility.
… https local URL Two Go-parity fixes from review: - The --local/--linked mutual-exclusivity check moved out of the handler to the command, BEFORE withLegacyCommandInstrumentation, so a flag-validation rejection no longer emits cli_command_executed — matching Go, which rejects it at cobra flag validation before RunE/PostRun. Extracted as legacyAssertSeedTargetsExclusive (unit-tested); removed the handler-level check. - Kong CA trust is now installed whenever the resolved LOCAL gateway URL is https (e.g. an explicit `api.external_url = "https://127.0.0.1:54321"` with tls.enabled false/omitted), not only when api.tls.enabled — mirroring Go's newLocalClient installing status.NewKongClient unconditionally. cert_path validation still gates on tls.enabled (config.go:845); the embedded CA is used otherwise.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e026b9a7dd
ℹ️ 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".
…circuit Go decodes the whole config (storage.FileSizeLimit, bucket sizes) and runs ValidateBucketName during config.Load — before buckets.Run can take its no-op path — so a config with no buckets but an invalid `[storage] file_size_limit` fails at load. The TS no-op short-circuit ran before the storage-level size parse, so it silently succeeded. Moved the config-load-time validations (bucket names, storage-level size, per-bucket props) ahead of the short-circuit.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cac899497
ℹ️ 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".
| // merged for --linked. A parse failure aborts before any network call. | ||
| const loadOptions: LoadProjectConfigOptions | undefined = | ||
| projectRef !== "" ? { projectRef } : undefined; | ||
| const loaded = yield* loadProjectConfig(cliConfig.workdir, loadOptions).pipe( |
There was a problem hiding this comment.
When supabase/config.toml contains a Go-invalid non-storage field such as [db] major_version = 12, this loadProjectConfig call still succeeds because @supabase/config only schema-decodes the number, and the later parity checks cover only bucket names and size limits. The Go root pre-run calls flags.LoadConfig before buckets.Run, and config.Validate rejects Postgres 12 before any Storage list/create/update calls; here the same config can proceed to mutate buckets (or no-op successfully if no buckets are configured). Add the missing legacy config validation before the Storage gateway is constructed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate parity observation, but escalating — this is the same class as the earlier db.major_version/remotes/api.port threads: Go's config.Validate (run during flags.LoadConfig before buckets.Run) rejects a Go-invalid non-storage field like db.major_version = 12, whereas @supabase/config only schema-decodes it. db.major_version is unrelated to storage seeding, so a seed-local check would be a wart — the fix is to port Go's config.Validate into @supabase/config so every config-loading command inherits it (db version, remotes duplicate/format, api.port, tls pairing, …). I've kept the storage-domain validations (bucket names, sizes) in the handler because they're what seed buckets owns; the whole-config Validate belongs in the config package. Tracking with that backlog; leaving open.
…n-refused Go's localGatewayHint (pkg/fetcher/http.go:117-143) only appends the port-conflict hint for a malformed HTTP response or a header/deadline timeout — NOT a plain ECONNREFUSED (the local stack is simply stopped). The TS gate fired for every loopback TransportError, so stopping the stack wrongly told users "another process may be listening". The hint is now suppressed when the transport error is a connection-refused (Bun doesn't emit Go's net/http strings, so this is a substring check on the error detail). Added a connection-refused test and switched the port-conflict tests to a malformed-response description.
There was a problem hiding this comment.
💡 Codex Review
Prefix shared legacy size-unit exports
The apps/cli/AGENTS.md guide says every exported token from legacy/ must be prefixed with Legacy or legacy, with no exceptions. This new shared legacy module exports bare names (ramInBytes, plus intToUint and bytesSize below), so hoisting these helpers reintroduces the autocomplete/import ambiguity the rule is meant to prevent; prefix the exports and update the new call sites.
ℹ️ 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".
| } | ||
|
|
||
| if (hasCert) { | ||
| const absCert = path.isAbsolute(certPath) ? certPath : path.join(workdir, "supabase", certPath); |
There was a problem hiding this comment.
When local TLS config uses an absolute-looking api.tls.cert_path (and the same pattern applies to key_path below), this reads /tmp/... directly and can proceed with a cert/key that the Go CLI would not use. Go resolves TLS paths with path.Join(builder.SupabaseDirPath, c.Api.Tls.CertPath) in apps/cli-go/pkg/config/config.go:795-800, without an IsAbs guard, so cert_path = "/tmp/kong.crt" is read as supabase/tmp/kong.crt; matching that avoids seeding with a different CA or succeeding where legacy config validation fails.
Useful? React with 👍 / 👎.
| label: string, | ||
| defaultValue: boolean, | ||
| ) { | ||
| if (yes) { |
There was a problem hiding this comment.
When SUPABASE_YES=1 is set without passing --yes, Go's console.PromptYesNo checks viper.GetBool("YES"), echoes the confirmation line, and returns true. This branch only sees the parsed CLI flag, so a stale vector/analytics bucket in a non-interactive run keeps the default false and is not pruned (or a TTY run prompts) even though the legacy command would auto-confirm.
Useful? React with 👍 / 👎.
| apiKey = yield* resolveLocalServiceRoleKey(config.auth); | ||
| } else { | ||
| baseUrl = `https://${projectRef}.${cliConfig.projectHost}`; | ||
| const envKey = process.env["SUPABASE_AUTH_SERVICE_ROLE_KEY"]; |
There was a problem hiding this comment.
When supabase/.env provides SUPABASE_AUTH_SERVICE_ROLE_KEY for a linked seed run, Go's config load calls loadNestedEnv before NewStorageAPI, so viper.IsSet("AUTH_SERVICE_ROLE_KEY") uses that key and avoids the Management API/PAT path. This direct process.env lookup never sees values that @supabase/config read from the project .env without mutating the process environment, so the same invocation can unexpectedly require a login or fail fetching API keys instead of seeding with the configured service-role key.
Useful? React with 👍 / 👎.
2 participants
Ports
supabase seed buckets(CLI-1322) from the Go binary proxy to a native TypeScript implementation in the legacy shell.What
seed bucketsseeds the local Storage stack fromsupabase/config.toml: it upserts[storage.buckets](create/update with an overwrite prompt) and[storage.vector]buckets (create/prune with graceful "feature unavailable" skips), then uploads each bucket'sobjects_pathfile tree.Why local-only
Go's
seedcommand is in thelocal-devgroup, so the root pre-run never resolves a project ref (cmd/root.go:108-116) andbuckets.Runalways receives an emptyprojectRef.--linked/--localare therefore accepted for surface parity (and their mutual exclusivity is enforced), but seeding always targets the local Storage service gateway. The remote/analytics code paths Go gates on a project ref are unreachable here and are omitted.Structure
seed/buckets/—handler,gateway(Storage service-gateway client: bucket/vector/object endpoints,apikey+Bearerauth),classify(vector graceful-skip detectors),upload(path/content-type helpers),flags(--local/--linkedmutual-exclusivity),errors.seed/seed.layers.ts— lean runtime (no Management API stack; local-only).@supabase/configdecodes the file but doesn't reproduce it): API URL fromapi.external_urlelse<scheme>://<SUPABASE_SERVICES_HOSTNAME|127.0.0.1>:<port>(config.go+misc.go:302); service-role key fromauth.service_role_keyelse a JWT signed withauth.jwt_secret(apikeys.go).legacy-size-units.tshoisted tolegacy/shared/(used byconfig pushandseed buckets).Parity notes for reviewers
[Y/n]/[y/N], overwrite default yes / prune default no),--yesecho, and the two yellow vectorWARNING:fall-throughs match Go.isUploadableEntry(batch.go:65): symlinks detected no-follow; dangling symlinks / symlinks-to-dirs / other non-regular entries are skipped withSkipping non-regular file:(not fatal); symlinked dirs are not descended.omitempty(public*bool,file_size_limit,allowed_mime_types).http.DetectContentType+mime.TypeByExtensionis OS-mime-table dependent, so byte-parity isn't achievable). SeeSIDE_EFFECTS.md.--output-format json/stream-jsonemit a structured run summary; text mode emits nothing extra (Go has no machine output).