Skip to content

feat(cli): add supabase workers push - #6262

Merged
kanadgupta merged 17 commits into
developfrom
FUNC-753/workers-push
Aug 26, 2026
Merged

feat(cli): add supabase workers push#6262
kanadgupta merged 17 commits into
developfrom
FUNC-753/workers-push

Conversation

@johnstonmatt

@johnstonmattjohnstonmatt commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds supabase workers push (aliased deploy) and the machinery it needs:

  • workers-api.ts — the typed Workers Management API client.
  • tar.ts / worker-package.ts — packaging a worker directory into the build
    context that gets uploaded.
  • worker-classify.ts — best-effort runtime detection from marker files, so a
    directory with no [workers.<name>] runtime can still deploy. The guess is
    always reported with a nudge to pin it down, never applied silently.

Stack 3 of 4, on top of workers new (#6261).

Linked issue

FUNC-753 (Linear). Supabase maintainer, exempt from the open-for-contribution flow.

Checklist

@johnstonmatt
johnstonmattforce-pushed the FUNC-753/workers-push branch 2 times, most recently from aa0ea27 to fa9be15CompareAugust 20, 2026 10:19
@johnstonmatt
johnstonmatt marked this pull request as ready for review August 20, 2026 13:27
@johnstonmatt
johnstonmatt requested a review from a team as a code ownerAugust 20, 2026 13:27

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:959520b26b

ℹ️ 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".

Comment threadapps/cli/src/shared/workers/worker-package.ts Outdated
Comment threadapps/cli/src/shared/workers/worker-config.ts
Comment threadapps/cli/src/shared/workers/workers-api.ts
Comment threadapps/cli/src/legacy/commands/workers/push/push.command.ts
Comment threadapps/cli/src/shared/workers/workers-api.ts
Comment threadapps/cli/src/shared/workers/worker-package.ts
Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts Outdated
Comment threadapps/cli/src/shared/workers/workers.errors.ts
Comment threadapps/cli/src/legacy/commands/workers/push/SIDE_EFFECTS.md Outdated
Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts
Comment threadapps/cli/src/shared/workers/worker-package.ts Outdated
Comment threadapps/cli/src/shared/workers/workers-api.ts
Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts
@github-actions

github-actionsBot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Supabase CLI preview

npx --yes https://pkg.pr.new/supabase/cli/supabase@9619d2ec81dcfc91d6f6edbd1607a44376e6103b

Preview package for commit 9619d2e.

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:e7acc025f1

ℹ️ 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".

Comment threadapps/cli/src/shared/workers/worker-package.ts Outdated
Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts Outdated
Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts Outdated
Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts Outdated
johnstonmatt added a commit that referenced this pull request Aug 21, 2026
… the upload URL
Addresses the review findings on #6262.
Two failures happened only after the remote project had already changed. `-o env`
cannot encode the payload's `workers` array, but that was discovered at emit
time — after every upload, deploy and build poll — so the command exited
non-zero having deployed, inviting a retry that deployed again. And an absent
optional `image_version` was left in the payload as `undefined`, which
smol-toml cannot represent, so `-o toml` threw at the same point. Both are now
settled before the first request: `legacyRejectUnsupportedWorkersOutput` runs up
front, and the field is spread conditionally like `url` beside it.
Packaging silently tolerated a filesystem it could not read. An unreadable file
was archived as zero bytes and an unreadable directory dropped its whole subtree,
so `push` reported success for an image built from an application with a hole in
it. Both propagate now. The redundant `Number()` around `File.Info.mode` is gone
too — it is a plain number, and wrapping it invited the reading that it was an
`Option`.
The presigned upload URL was reaching the `--debug` log. Its query string is a
write-capable credential for the archive a deploy is about to build from, so it
does not belong in terminal scrollback or a CI log. Fixed at the logging
boundary rather than by giving the upload its own HTTP client: redaction in
`legacyHttpClientLayer` keeps the client injectable for tests and covers every
presigned URL the CLI might ever log, not just this one.
Also: the build-poll read retries on a wall-clock budget instead of three
back-to-back attempts, which a two-second blip exhausted while the surrounding
poll still had minutes left; `WorkersApiUnexpectedStatusError` classifies from
the status it carries, so a 401 reads as "log in" rather than as a service
failure across every Workers endpoint; a source of nothing but empty directories
is refused before an upload slot is minted, instead of deploying an image with no
handler; the runtime guess is only reported once the source is known to exist;
and `config.toml` loading moved inside the finalizers, so a malformed config
still flushes telemetry — matching `config/push`, which already had it right.
Two findings are deliberately left for follow-ups: streaming the build context
rather than buffering it, and an ignore mechanism so `.env` and `.git` can be
kept out of the uploaded archive. Both are new work rather than defect fixes.
The `SIDE_EFFECTS.md` also now records the telemetry and linked-project writes,
the cache-miss API route, and why there is no live suite yet.
Comment threadapps/cli/src/shared/workers/worker-package.ts

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:38fd3f44bf

ℹ️ 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".

Comment threadapps/cli/src/shared/workers/worker-package.ts Outdated
Comment threadapps/cli/src/legacy/commands/workers/push/push.command.ts
Comment threadapps/cli/src/shared/workers/worker-package.ts Outdated
johnstonmatt added a commit that referenced this pull request Aug 21, 2026
…on a clock
Four follow-ups on #6262, three of them corners the previous round left open.
`fs.stat` was the last swallowed filesystem error in `collectEntries`. An entry
that disappears between `readDirectory` and the stat, or whose metadata cannot be
read, was dropped from the archive — the same "deploy an application with a hole
in it" outcome as the reads that were fixed, just losing a whole entry instead of
its contents.
`createTar` throws `TarPathTooLongError` for a name USTAR cannot represent, and
calling it directly inside the generator made that a defect rather than a typed
failure. `withJsonErrorHandling` only catches the failure channel, so
`--output-format json` would have terminated with no structured error payload for
an error the class explicitly declares as user-actionable. Wrapped in
`Effect.try`, narrowing to that class and letting anything else stay a defect —
because anything else there really is a bug.
The transient-failure integration test was waiting on the real clock. The outer
poll schedule was injected but the per-read retry was not, and that one is spaced
in seconds, so a 500-then-200 sequence sat for two seconds and more transient
responses would have pushed it toward thirty. Both schedules are parameters now;
that test went from ~2s to 22ms.
`--instances` is bounded at the parser, the way `[workers.<name>] instances` is
bounded in the config schema. `--instances=-1` was accepted by the integer flag
and carried into the deploy spec, so an impossible scaling request reached the
remote endpoint only after the build context had been packaged and uploaded.
`Flag.filter` refuses it during parsing, consistent with how `--runtime` is
validated.

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:eec654d4ca

ℹ️ 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".

Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts
Comment threadapps/cli/src/legacy/commands/workers/push/SIDE_EFFECTS.md
Comment threadapps/cli/src/shared/workers/worker-package.ts Outdated
Comment threadapps/cli/src/shared/workers/workers-api.ts

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:715ed177bd

ℹ️ 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".

Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts Outdated

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:49ee4a9a73

ℹ️ 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".

Comment threadapps/cli/src/shared/workers/worker-classify.ts
Comment threadapps/cli/src/shared/workers/worker-package.ts Outdated
Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts
Comment threadapps/cli/src/shared/workers/worker-package.ts Outdated

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:86d0f6bd28

ℹ️ 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".

Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts Outdated
Comment threadapps/cli/src/shared/workers/workers-api.ts
Comment threadapps/cli/src/shared/workers/workers-api.ts
Comment threadapps/cli/src/shared/workers/workers-api.ts
johnstonmatt added a commit that referenced this pull request Aug 21, 2026
… the upload URL
Addresses the review findings on #6262.
Two failures happened only after the remote project had already changed. `-o env`
cannot encode the payload's `workers` array, but that was discovered at emit
time — after every upload, deploy and build poll — so the command exited
non-zero having deployed, inviting a retry that deployed again. And an absent
optional `image_version` was left in the payload as `undefined`, which
smol-toml cannot represent, so `-o toml` threw at the same point. Both are now
settled before the first request: `legacyRejectUnsupportedWorkersOutput` runs up
front, and the field is spread conditionally like `url` beside it.
Packaging silently tolerated a filesystem it could not read. An unreadable file
was archived as zero bytes and an unreadable directory dropped its whole subtree,
so `push` reported success for an image built from an application with a hole in
it. Both propagate now. The redundant `Number()` around `File.Info.mode` is gone
too — it is a plain number, and wrapping it invited the reading that it was an
`Option`.
The presigned upload URL was reaching the `--debug` log. Its query string is a
write-capable credential for the archive a deploy is about to build from, so it
does not belong in terminal scrollback or a CI log. Fixed at the logging
boundary rather than by giving the upload its own HTTP client: redaction in
`legacyHttpClientLayer` keeps the client injectable for tests and covers every
presigned URL the CLI might ever log, not just this one.
Also: the build-poll read retries on a wall-clock budget instead of three
back-to-back attempts, which a two-second blip exhausted while the surrounding
poll still had minutes left; `WorkersApiUnexpectedStatusError` classifies from
the status it carries, so a 401 reads as "log in" rather than as a service
failure across every Workers endpoint; a source of nothing but empty directories
is refused before an upload slot is minted, instead of deploying an image with no
handler; the runtime guess is only reported once the source is known to exist;
and `config.toml` loading moved inside the finalizers, so a malformed config
still flushes telemetry — matching `config/push`, which already had it right.
Two findings are deliberately left for follow-ups: streaming the build context
rather than buffering it, and an ignore mechanism so `.env` and `.git` can be
kept out of the uploaded archive. Both are new work rather than defect fixes.
The `SIDE_EFFECTS.md` also now records the telemetry and linked-project writes,
the cache-miss API route, and why there is no live suite yet.
johnstonmatt added a commit that referenced this pull request Aug 21, 2026
…on a clock
Four follow-ups on #6262, three of them corners the previous round left open.
`fs.stat` was the last swallowed filesystem error in `collectEntries`. An entry
that disappears between `readDirectory` and the stat, or whose metadata cannot be
read, was dropped from the archive — the same "deploy an application with a hole
in it" outcome as the reads that were fixed, just losing a whole entry instead of
its contents.
`createTar` throws `TarPathTooLongError` for a name USTAR cannot represent, and
calling it directly inside the generator made that a defect rather than a typed
failure. `withJsonErrorHandling` only catches the failure channel, so
`--output-format json` would have terminated with no structured error payload for
an error the class explicitly declares as user-actionable. Wrapped in
`Effect.try`, narrowing to that class and letting anything else stay a defect —
because anything else there really is a bug.
The transient-failure integration test was waiting on the real clock. The outer
poll schedule was injected but the per-read retry was not, and that one is spaced
in seconds, so a 500-then-200 sequence sat for two seconds and more transient
responses would have pushed it toward thirty. Both schedules are parameters now;
that test went from ~2s to 22ms.
`--instances` is bounded at the parser, the way `[workers.<name>] instances` is
bounded in the config schema. `--instances=-1` was accepted by the integer flag
and carried into the deploy spec, so an impossible scaling request reached the
remote endpoint only after the build context had been packaged and uploaded.
`Flag.filter` refuses it during parsing, consistent with how `--runtime` is
validated.

@kanadguptakanadgupta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits documented below that are worth calling out, but nothing blocking — once you're happy with it, ping me and I can merge it after #6261!

Well-built slice. The handler reads top-to-bottom as the command's own story, the USTAR writer is carefully documented and unit-tested against real header layout, the error taxonomy carries detail/suggestion pairs with actionability declarations throughout, and the integration suite (27 scenarios) is exactly the scenario-oriented shape the testing pyramid asks for — including regression pins from earlier review rounds (no-request-made assertions, telemetry flush on config failure). Redacting presigned URLs at the logging boundary, with tests in both directions, was the right call.

Ticket coverage (FUNC-753 slice): the core asks are delivered. Deploys run from any project directory via the resolved workdir (tested); the async contract is handled properly — deploy accepts 202, awaitWorkerBuild polls on a 2s/10-minute schedule with a wall-clock-bounded retry for transient reads, build failures surface with the server's state_reason, and a timeout points at follow-up. Runtime/size/instances come from config.toml with a reported best-effort runtime guess and an --instances override. One gap against the ticket text: there is no interactive prompting for stack/size/instances anywhere in this slice. If guess-and-report deliberately replaced prompting, worth recording that on the ticket.

Findings, ranked (none blocking):

  1. tar.tswriteOctal accepts negative values and writes an invalid octal field (reachable via pre-epoch file mtimes from worker-package.ts); the archive uploads and then fails remotely. Inline comment; one-line guard.
  2. Both "re-scaffold it with supabase workers new <name> --force" suggestions reference a flag new does not define, so the recovery command itself errors. Pending reply on the existing thread; a second occurrence sits at the fileCount === 0 guard.
  3. The handler's fs.stat/readDirectory recoveries squash permission and I/O errors into "missing"/"empty" misdiagnoses with a scaffold-over-it suggestion. Inline comment; overlaps an open bot thread on line 170.
  4. The Effect.try narrow in worker-package.ts leaves TarFieldTooLargeError a defect even though the class declares itself user-actionable, so --output json exits unstructured for it. Pending reply on the existing thread.
  5. Doc nit: tar.ts's why-not paragraph covers shelling out to tar but not Bun.Archive, which the repo already uses to build tars in pgdata-snapshot.ts. Inline comment.

Questions rather than demands:

  • The presigned upload has no deadline, so a stalled connection hangs push indefinitely. A timeout is cheap, but any fixed value is wrong for some archive-size/link-speed combination — was unbounded deliberate?
  • Symlinks whose targets resolve outside the packaged tree (e.g. pnpm workspace node_modules pointing at a repo-root .pnpm) are archived as dangling links, so the remote build sees a hole and fails cryptically. Fine for alpha-scale workers; a local warning when a link target escapes the tree may belong on the follow-up list alongside the streaming work.

Out-of-scope observations (other PRs in the stack own these):

  • awaitWorkerBuild's timeout suggestion names supabase workers status, which does not exist until the next PR — harmless if the stack merges together, misleading if push ships first (open bot thread).
  • legacyDiscoverWorkerNames (base PR) swallows stat failures, so a bare push can silently skip a worker and still exit 0; and -o table/-o csv fall through to TOML in the shared workers output helper. Both live in #6261's files.

Several threads from the latest Codex round remain open; most are the same papercut caliber as the above, and the fix cadence on earlier rounds has been thorough.

Recommendation: approve with nits — the golden path is correct, the async-deploy/polling contract the ticket demands is genuinely delivered and tested, and everything found is edge-case robustness or recovery-text polish rather than wrong deploys, data loss, or credential mishandling.

Comment threadapps/cli/src/shared/workers/tar.ts Outdated
Comment threadapps/cli/src/shared/workers/tar.ts
Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts Outdated
Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts Outdated
Comment threadapps/cli/src/shared/workers/worker-package.ts Outdated
Base automatically changed from FUNC-753/workers-new to developAugust 26, 2026 20:27
Builds and deploys workers into the linked project, and brings the
Management API seam with it. Registered under `deploy` as an alias, for
anyone reaching for the `supabase functions` verb out of habit.
Given no names it deploys every worker in the project, matching
`supabase functions deploy`, whose conventions this command set otherwise
mirrors. "Every worker" is the union of the directories under
`supabase/workers/` and the `[workers.<name>]` entries, so one with a
`source` pointing elsewhere is not missed, and the order is sorted rather
than whatever the filesystem returned. Deploys run one at a time: each is a
server-side container build, so interleaving them would both compete for the
alpha's per-project capacity and shred the progress output; the first
failure stops the run.
The flow is mint an upload slot, PUT the `.tar.gz` build context straight at
the presigned URL, deploy, then poll until `build_state` leaves `building`.
The upload carries no Supabase credentials: the signature in the URL is the
authorization, and the bytes never pass through the management API. That
signature is also a write-capable credential for the archive a deploy is
about to build from, so `legacyHttpClientLayer` redacts presigned URLs at
the logging boundary — `--debug` scrollback and CI logs are not where it
belongs, and redacting there covers every presigned URL the CLI might log
rather than only this one.
Polling is a `Schedule`, and the read inside it retries on a wall-clock
budget so a blip of a second or two does not throw away a deploy that still
has minutes of build ahead of it.
Which spec is sent depends on the runtime: a `dockerfile` worker sends a
context and no `spec.runtime`, a catalog runtime sends both, and a bare
`sandbox` sends the runtime alone and skips packaging, so it has no URL. A
directory with no `[workers.<name>] runtime` has one guessed from marker
files once the source is known to exist, reported on stderr with a nudge to
pin it down.
Everything that can fail deterministically fails before the remote project
changes. `-o env` and a `-o toml` payload carrying an absent optional are
settled up front rather than at emit time, where the command would exit
non-zero having already deployed and invite a retry that deployed again;
`--instances` is bounded at the parser the way the config schema bounds
`[workers.<name>] instances`, instead of carrying an impossible scaling
request through a packaged upload; and a source of nothing but empty
directories is refused before an upload slot is minted, rather than
deployed as an image with no handler.
The build context is packaged in-process rather than by shelling out to
`tar`, whose BSD, GNU and absent-on-Windows variants each produce a
different archive from the same tree. `tar.ts` writes USTAR directly: files,
directories and symlinks, refusing a value too large for an octal header
field instead of letting it spill into the next one and read back as a
plausible but wrong size. Symlinks are stored as links rather than followed
— anything pnpm installs is symlink-dense, so following them would inline
every dependency and walk into a link pointing at an ancestor. Every
filesystem error propagates: an unreadable file archived as zero bytes, a
dropped subtree or an entry lost between `readDirectory` and its stat all
mean a successful `push` reporting an image built from an application with a
hole in it.
The Workers routes answer 404 both for a project outside the alpha's
allow-list and for a ref that names nothing this account can see, so the
classification reads `error.code`: `not_found` raises
`WorkerProjectNotFoundError` naming the ref, `supabase link` and
`supabase login`, and anything unrecognized keeps the enrolment answer,
since that is what the allow-list has historically returned and guessing the
other way sends someone to check a ref that is fine.
This is the first command in this shell to call a v2 Management API route;
every other one here is a Go-parity port and uses v1 only.
Two findings are deliberate follow-ups rather than defects: streaming the
build context instead of buffering it, and an ignore mechanism so `.env` and
`.git` can be kept out of the uploaded archive.
The same finding as the `workers new` change one commit down the stack, applied
to the three occurrences this branch adds: the symlink probe and the mtime
fallback in `worker-package.ts`, and the source-directory check in
`push.handler.ts`. `Option.isSome`/`isNone` are type guards, so the narrowing
after each check is unchanged.
`worker-package.unit.test.ts` read `exit._tag` for the same reason; the repo
guidance names `Exit.isSuccess`/`Exit.isFailure` and applies to tests too.
`push.integration.test.ts`'s `tagOf` keeps its `_tag` access. It classifies
values that may be a `Data.TaggedError` or a plain `Error` subclass with no tag
at all, which is the dynamic boundary the guidance carves out.
Also corrects the `push.handler.ts` module docblock: the argument is variadic,
so it is `[name...]`, matching the SIDE_EFFECTS title.
develop renamed `LegacyCliConfig` to `LegacyCliSettings` (and its module),
which this branch's push handler still imported under the old path. The
unresolved import widened the handler's requirements to `unknown`, so the 27
knock-on errors in `push.integration.test.ts` all came from this one line.
@kanadgupta
kanadguptaforce-pushed the FUNC-753/workers-push branch from c51d62e to 0fef65eCompareAugust 26, 2026 20:27

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:0fef65eb73

ℹ️ 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".

Comment threadapps/cli/src/shared/workers/workers-api.ts
Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts
Comment threadapps/cli/tests/helpers/legacy-workers.ts
`writeOctal` only checked the rendered width, which a negative or
non-finite value passes: `(-1).toString(8)` is `"-1"` and
`NaN.toString(8)` is `"NaN"`, and both pad to exactly the field width. The
header went out unparseable, so GNU tar rejected the whole archive
server-side after the build context had already uploaded.
The reachable path is a file mtime: a pre-1970 timestamp is negative, and
a corrupt one decodes to an `Invalid Date` whose `getTime()` is `NaN`.
Neither is worth failing a deploy over, so `packageWorkerDirectory` now
collapses both to the epoch before they reach the writer, and the writer
checks the range as well as the width for anything that still gets there.
`TarFieldTooLargeError` is renamed `TarFieldOutOfRangeError`, since it no
longer only reports values that are too large.
`TarFieldOutOfRangeError` carries `actionability.invalidInput` and
documents itself as user-actionable, but `packageWorkerDirectory` narrowed
its catch to `TarPathTooLongError` and rethrew the other as a defect. The
classification could therefore never take effect, and because
`withJsonErrorHandling` catches failures and not defects, `-o json` exited
with no structured error at all. An 8 GiB file in the source directory is
the realistic way in, through the size field.
The sibling test on the path error asserted only `Exit.isFailure`, which a
defect also satisfies, so it never pinned the distinction it was named
for. Both tests now check the cause.
`Effect.option` on the source-directory stat swallowed every failure, so a
permission or I/O error was reported as "There is no worker source at
<path>" with a suggestion to scaffold one — a misdiagnosis whose
remediation points at a path that is already occupied. The `readDirectory`
a few lines down had the same shape through `orElseSucceed(() => [])`,
reading an unopenable directory as an empty one.
Only a `NotFound` reason now maps to `WorkerSourceMissingError`; every
other `PlatformError` propagates as itself. `PlatformError` was already in
this handler's error channel via `packageWorkerDirectory`, so nothing
downstream changes.
Both "nothing to deploy" guards suggested re-scaffolding with `supabase
workers new <name> --force`. `new` defines no `--force` flag, so following
the suggestion exits with an unknown-option error — and dropping the flag
would not save it: `new` refuses any name already present in
`config.toml`, which is where a pushed worker almost always comes from,
and refuses a directory that exists and is not empty, which covers the
empty-subdirectories guard.
The directory is already there and already wired up, so the only honest
instruction is to put the code in it. Both call sites now share one
suggestion, and the tests pin that neither names `new`.
The why-not paragraph only covered shelling out to `tar`, leaving the next
reader to wonder why this does not reuse `Bun.Archive`, which the repo
already builds tar bytes with. Verified against Bun 1.3.14: creation takes
path-to-contents pairs and nothing else, and every entry is emitted as a
regular file with mode 0644 and the current wall-clock time — no symlinks,
no executable bit, no reproducible output.

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:e831044de1

ℹ️ 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".

Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts
A file sitting where the source directory should be fell into the same
`WorkerSourceMissingError` as a missing path, so `push` said "There is no
worker source at <path>" about a path that is occupied, and suggested
`supabase workers new <name>` — which refuses a destination that exists
and is not a directory, and refuses any name already in `config.toml`. The
user got a false diagnosis followed by a command that errors out.
The missing-path branch keeps that message, where both halves are true: a
name is only validated as a DNS label before dispatch, so `push <name>`
for a worker that is in neither `config.toml` nor the workers directory
does reach it, and `workers new <name>` is the right answer there.
SIDE_EFFECTS.md picks up this condition and the unreadable-source one from
the preceding commit.
The missing-source suggestion always said `supabase workers new <name>`,
but `new` refuses any name already under `[workers.<name>]` — so for a
configured worker whose directory is gone, the one recovery offered exits
with "already configured". The suggestion now depends on how the worker
got here:
- no config entry — the name reached `push` from argv alone, `new` is the
answer, message unchanged;
- configured, default directory — the entry is fine and the directory is
not, so say to create it;
- configured with an explicit `source` — the path in config is as likely
to be the mistake as the absent directory, so name both.

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:f0d3477d64

ℹ️ 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".

Comment threadapps/cli/src/legacy/commands/workers/push/push.integration.test.ts Outdated
Comment threadapps/cli/src/legacy/commands/workers/push/push.integration.test.ts Outdated
Comment threadapps/cli/src/shared/workers/workers.errors.ts
The test chmod-ed the source directory shut, then ran the deploy through
`Effect.flip`. Root ignores the permission bits, so the deploy succeeds
there — and `Effect.flip` turns a success into a failure, which fails the
test before the branch written to handle exactly that case can run. The
root-safe branch was unreachable.
The permission probe now happens before the run and selects which effect
to run, so both permission models are asserted rather than one of them
crashing.
`tagOf` goes with it: every call site knows the variant it expects, so
they use `Predicate.isTagged` or the error class instead of reaching for
`_tag`, per the repo rule that tests follow the same narrowing rules as
production code.
Both fallbacks read `error.reason._tag` when a transport error carried no
description, which reinvents `HttpClientError.message` and does it worse —
the library renders `Transport error (POST https://...)` where this
rendered the bare class name.
The two sites cannot take the same fix. `mapRequestError` talks to the
Management API, so `error.message` is a straight upgrade. `uploadBuildContext`
cannot use it: the message appends the URL that failed, and there that URL
is the presigned signature — a write-capable credential, and the same leak
`legacyRedactHttpUrl` exists to prevent on the debug log. That site keeps
the reason's own description with a fixed fallback.
The workers HTTP harness gains a transport-failure stub, so the leak has a
test rather than only a comment. Swapping `error.message` back in turns it
red.
`push` reused `legacyLoadWorkersProject`, which pins the loader to
`tomlOnly`. That constraint belongs to `workers new`, whose entry writer is
a TOML text editor and would corrupt a `config.json` by appending a
`[workers.<name>]` table to it. `push` only reads, and inherited it by
sharing one function.
With only a `config.json` on disk the loader returns null, so the workers
section came back empty: a bare `push` skipped any worker whose `source`
sits outside `supabase/workers/`, and a named one deployed with a guessed
runtime and the default size and instance count instead of its configured
values.
The loader now takes the flag, with two named entry points so the call
site says which it wants — `legacyLoadWorkersProject` for readers,
`legacyLoadWorkersProjectForEntryWrite` for the scaffolder. The TOML-only
gap is now the writer's alone.
`collectEntries` stored every symlink as a link entry, which is right for a
link inside the packaged tree and wrong for one pointing out of it. The
archive is the whole of what the server gets — it runs no install step and
has no view of the surrounding repository — so an escaping link arrives
dangling: a catalog runtime boots without the dependency, a Dockerfile
build fails on the `COPY`, both minutes later with nothing naming the
cause.
A worker directory that is a pnpm workspace member is the common way in.
Its dependencies link to the repository-root store, so every one of them
escapes. A worker with `source` pointing at an existing monorepo package is
the same case, and that is the use `source` exists for.
An escaping link is now refused before the upload. An absolute target that
does land back inside the tree is rewritten relative to the link, since a
path on this machine resolves to nothing on the other end.
Not runtime-specific, so not gated on one: the walk never sees the runtime,
and a Dockerfile worker with a symlinked config has the same problem.
`legacyDiscoverWorkerNames` mapped every `readDirectory` failure to `[]`
and every per-entry `stat` failure to `None`. A bare `push` on a workers
root it cannot list therefore reported "no workers were named, and none
were found" — or, when config named some, deployed those and exited 0
having silently skipped every directory-only worker. Absence and
unreadable again, one level above the source-directory guards.
A missing workers root still reads as nothing: a project may never have
scaffolded one, and `[workers.<name>]` entries can name workers that live
elsewhere. Every other reason propagates. The per-entry stat keeps skipping
a name that vanished between the listing and the stat, and nothing else.
A worker recorded as `runtime = "dockerfile"` deploys its uploaded context
as-is, so with no top-level `Dockerfile` the server has nothing to build.
That only surfaced as a remote build failure, minutes after the archive had
uploaded and a deployment had started, when the CLI was already standing in
the directory that answers the question.
Only reachable from a recorded runtime. A guessed `dockerfile` always
passes, because the classifier picks it by finding this exact file.
Classified `invalidConfig` rather than the `provideFlags` its neighbours in
this file use: `push` has no runtime flag, so the fix is in `config.toml`
or the directory, and the suggestion names both.

@kanadguptakanadgupta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was able to reproduce this and it worked as expected 🎊 will merge shortly

This reverts commit 8bac2f2.
The guard bought a clearer message for one misconfiguration and cost a
runtime-specific branch in a handler that had none, plus a second copy of
the `"Dockerfile"` literal already held by the classifier's marker table.
The refactor that would have justified it does not exist: markers are
evidence for a guess, not requirements. `deno.json` and `package.json` are
both optional — `workers new` scaffolds neither — so there is no shared
"required source" contract to hoist the check into, and `dockerfile` would
stay the lone special case however it were written.
Deploying and letting the build report it is the honest cost of not
modelling this yet.

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:9619d2ec81

ℹ️ 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".

Comment threadapps/cli/src/shared/workers/worker-package.ts
Comment threadapps/cli/src/shared/workers/worker-package.ts
Comment threadapps/cli/src/legacy/commands/workers/push/push.handler.ts
Comment threadapps/cli/src/shared/workers/worker-package.ts
Comment threadapps/cli/src/shared/workers/workers.errors.ts
@kanadgupta
kanadgupta added this pull request to the merge queueAug 26, 2026
Merged via the queue into develop with commit 4214807Aug 26, 2026
26 checks passed
@kanadgupta
kanadgupta deleted the FUNC-753/workers-push branch August 26, 2026 22:43
pullBot pushed a commit to chizee/cli that referenced this pull request Aug 27, 2026
## Summary
Completes the command family with the three remaining subcommands:
- `workers list` — this project's workers.
- `workers status` — one worker in detail.
- `workers delete` — remove a worker, with confirmation.
All three reuse the API client and output helpers introduced in supabase#6262,
so this is
the smallest layer of the stack.
**Stack 4 of 4**, on top of `workers push` (supabase#6262).
## Linked issue
FUNC-753 (Linear). Supabase maintainer, exempt from the
`open-for-contribution` flow.
## Checklist
- [x] The PR title follows [Conventional
Commits](https://www.conventionalcommits.org/)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@johnstonmatt@kanadgupta