feat(config): add the [workers] section to the project config schema - #6260
feat(config): add the [workers] section to the project config schema#6260johnstonmatt wants to merge 1 commit into
Conversation
johnstonmatt
changed the title
1f30b54 to
0058885
Compare
![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: 005888549e
ℹ️ 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".
0058885 to
ce69f5b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce69f5b12b
ℹ️ 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".
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@17a9058f3e956f1064eba88fa61e1809e4d79eaaPreview package for commit |
ce69f5b to
49964b7
Compare
Two schema defects found in review of #6260. `StructWithRest` applies its rest record to declared struct fields too, so excluding `root` from the worker-name pattern was the only thing keeping the record from claiming it — and `io.ts` decodes every unselected `[remotes.*]` block with `disableChecks: true`, which drops that pattern. A remote that merely names a workers root therefore failed the entire config load. The rest value now admits either a worker table or the `root` string, and a business-rule check rejects a bare value where a table belongs. That check relaxes under `disableChecks` along with every other business rule, which is what the flag is for: an unselected remote is never the config being acted on. `instances` was an unconstrained `Schema.Number`, so `1.5` and `-1` loaded happily. Neither ever reached the API — `readWorkersSection` drops a non-integer, so `push` silently deployed 1 instead, rescaling a worker against the recorded config. Bounded here as a non-negative integer, matching `spec.instances` in the Management API's own input schema, so the mistake is named at load time instead.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a0dfa965b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7759542fa
ℹ️ 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".
| Schema.Number.check( | ||
| Schema.isInt().annotate({ expected: "a whole number of instances" }), | ||
| Schema.isGreaterThanOrEqualTo(0).annotate({ expected: "zero or more instances" }), | ||
| ).annotate({ |
There was a problem hiding this comment.
The updated checks reject fractions and negative values, but they still accept instances = 9007199254740992. The generated V2DeployAWorkerInput schema bounds this field at 9007199254740991 (packages/api/src/generated/contracts.ts:10659-10668), so the intended deploy path will reject a configuration that this decoder and the published JSON schema both advertise as valid; add the corresponding upper bound here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76a45a82f7
ℹ️ 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".
| realtime, | ||
| storage, | ||
| studio, | ||
| workers, |
There was a problem hiding this comment.
When a project adds the newly supported [workers] section and invokes a path still delegated to supabase-go, the child rejects the entire config before doing any work. For example, apps/cli/src/legacy/commands/functions/download/download.handler.ts:67-74 delegates --legacy-bundle without sanitizing the config, while apps/cli-go/internal/functions/download/download.go:131-134 calls flags.LoadConfig; that loader uses UnmarshalExact, but its baseConfig has no workers field (apps/cli-go/pkg/config/config.go:228-247,749). Add an ignored/compatible workers field to the Go config model or strip the TS-only section for delegated invocations so adopting this advertised section does not break remaining legacy workflows.
Useful? React with 👍 / 👎.
Two schema defects found in review of #6260. `StructWithRest` applies its rest record to declared struct fields too, so excluding `root` from the worker-name pattern was the only thing keeping the record from claiming it — and `io.ts` decodes every unselected `[remotes.*]` block with `disableChecks: true`, which drops that pattern. A remote that merely names a workers root therefore failed the entire config load. The rest value now admits either a worker table or the `root` string, and a business-rule check rejects a bare value where a table belongs. That check relaxes under `disableChecks` along with every other business rule, which is what the flag is for: an unselected remote is never the config being acted on. `instances` was an unconstrained `Schema.Number`, so `1.5` and `-1` loaded happily. Neither ever reached the API — `readWorkersSection` drops a non-integer, so `push` silently deployed 1 instead, rescaling a worker against the recorded config. Bounded here as a non-negative integer, matching `spec.instances` in the Management API's own input schema, so the mistake is named at load time instead.
cf7adf6 to
0e8b578
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e8b5783e5
ℹ️ 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".
| "patternProperties": { | ||
| "^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$": { |
There was a problem hiding this comment.
When an editor validates [workers.Not_A_Label], this schema considers it valid because patternProperties only constrains matching keys and the surrounding object omits "additionalProperties": false. The runtime decoder instead drops that entry (as the new unit test confirms), so users can receive successful schema validation while their worker configuration silently disappears; close the object to unmatched properties so the published schema enforces the advertised worker-name rule.
Useful? React with 👍 / 👎.
Workers record their runtime, instance size, instance count and source directory in `supabase/config.toml`, keyed `[workers.<name>]`, next to the `[functions.<slug>]` entries already in the same file. The section is a plain `Schema.Record`: one sub-table per worker and no project-wide scalar sitting beside them, so there is nothing for the index signature to collide with. Worker names are DNS labels, matching what the Management API validates its `:name` path parameter against, since they end up in hostnames. `instances` is bounded as a non-negative integer to match `spec.instances` in the API's own input schema — a value that gets past the schema is dropped rather than sent, so leaving it unbounded silently deploys a different count than the config asked for. The section flows into the published `schema.json`, so editors offer completion for it in `config.toml`. That asset is served at PROJECT_CONFIG_SCHEMA_URL and stamped into every `config.toml` that `saveProjectConfig` writes, so a stale copy makes editors flag valid config as invalid. Most of that asset's diff is not workers. `toJsonSchemaDocument` changed how it emits unions between effect beta.107 and rc.108, and the bump landed on develop without the asset being regenerated, so inline `Infinity`/`NaN` unions collapse into `$defs` refs throughout — regenerating on the parent commit alone produces ~549 of those deletions. Nothing wires the generator into a script or CI job, so the drift is silent. Worth fixing separately.
0e8b578 to
17a9058
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17a9058f3e
ℹ️ 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".
| realtime, | ||
| storage, | ||
| studio, | ||
| workers, |
There was a problem hiding this comment.
When an unselected [remotes.<name>.workers] block contains the reserved scalar root, reusing the normal workers schema here still makes loadProjectConfig fail: decodeRemotesWithoutChecks only disables .check() refinements, while the record continues to decode every value—including root—as a worker struct. Fresh evidence in the final squashed tree is that the separate remote-only schema described in the earlier thread is absent and this field again references workers directly; restore a remote-specific shape that permits the scalar root without permitting scalar worker entries.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds the
[workers]section to the project config schema: aSchema.Recordofworker name to
{ runtime, size, instances, source }, mirroring the[functions.<slug>]convention in the same file. The same schema is used for theproject config and for
[remotes.*], so a remote can carry its own workeroverrides.
Two constraints live at the schema level:
:namepath parameter against, since they end up in hostnames.instancesis a non-negative integer, matchingspec.instancesin the API'sown input schema. A value that gets past the schema is dropped rather than sent,
so leaving it unbounded means a
pushsilently deploys a different count thanthe config asked for.
There is no project-wide scalar in the table — an earlier revision had a
[workers] rootfor relocating the grouping directory, and it was dropped because[workers.<name>] sourcealready puts a worker anywhere in the repo. That keepsworkersa plain record with nothing for the index signature to collide with,rather than a
StructWithRestneeding a key-pattern exclusion that vanished underthe
disableChecks: trueio.tsuses for unselected remotes.No CLI surface consumes this yet — it lands first so the schema and its generated
types are reviewable on their own.
Stack 1 of 4. Followed by
workers new(#6261),workers push(#6262), andworkers list/status/delete(#6263).Linked issue
FUNC-753 (Linear). Supabase maintainer, exempt from the
open-for-contributionflow.Checklist