Repository navigation
feat(sites): list bunny sites in help as a public preview - #243
Conversation
🦋 Changeset detectedLatest commit: 212586d The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: d06088ba02
ℹ️ 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 (err instanceof UserError && !(err instanceof ApiError)) throw err; | ||
| throw new UserError( |
There was a problem hiding this comment.
Re-throw ApiError and unexpected exceptions instead of wrapping both as a plain UserError. When provisioning receives a 401, this catch strips its status so defineCommand() cannot produce the credential-source/login hint; it also turns programming or runtime failures into exit code 1 without the verbose stack trace. The new resume hint should only augment errors whose classification and context can be preserved.
AGENTS.md reference: AGENTS.md:L269-L282
Useful? React with 👍 / 👎.
| // CI deploys a nested project's output from the repo root, so fall back to the bunny.jsonc above the deploy directory. | ||
| const siteConfig = | ||
| loadSiteConfig() ?? (args.dir ? loadSiteConfig(args.dir) : null); |
There was a problem hiding this comment.
When a deploy is launched from a repository root with a positional nested output directory, this fallback finds that project's bunny.jsonc, but selectSite() later performs its own loadSiteConfig() from the current working directory and never sees siteConfig.config.name. Consequently, a non-interactive command such as bunny sites deploy packages/web/dist --output json still fails with “No site specified” even though packages/web/bunny.jsonc names the site; pass the already loaded name into site selection.
Useful? React with 👍 / 👎.
| const lockDir = [projectRoot, root].find((dir) => | ||
| LOCKFILES.some((name) => existsSync(join(dir, name))), | ||
| ); |
There was a problem hiding this comment.
For a nested project without its own lockfile, this unconditionally treats any repository-root lockfile as governing the project. If the nested package is not actually a workspace member—for example, an independent npm app under a repository whose root has package-lock.json—the generated job runs from the nested directory but emits npm ci; npm requires an existing lockfile for that project and fails there. Verify workspace membership before falling back to the root lockfile, otherwise generate the unlocked install path.
Useful? React with 👍 / 👎.
| "refs/remotes/origin/HEAD", | ||
| ]); | ||
| return ref?.replace(/^origin\//, "") || "main"; |
There was a problem hiding this comment.
When refs/remotes/origin/HEAD is absent locally, this emits main even if the GitHub repository's default branch is master, trunk, or another name. That ref is commonly missing after git init plus git remote add, so the generated workflow listens on the wrong branch and ordinary pushes never deploy despite the command reporting that pushes to main go live. Query the remote HEAD or otherwise obtain the repository default before using the fallback.
Useful? React with 👍 / 👎.
| // statSync follows symlinks, so linked files and dirs ship as their targets. | ||
| const stat = entry.isSymbolicLink() | ||
| ? statSync(entryAbs, { throwIfNoEntry: false }) | ||
| : entry; |
There was a problem hiding this comment.
Following symlinks without checking the resolved target allows a harmless-looking entry inside the deploy directory to upload files from anywhere on the machine. For example, public/config -> ../.env or public/vendor -> ../node_modules bypasses the documented dotfile and node_modules exclusions and can publish credentials or unrelated files to the CDN. Resolve each target and reject targets outside the deploy root, or require an explicit opt-in before dereferencing them.
Useful? React with 👍 / 👎.
| // An imported site keeps its original zone names, so the name-pattern scan above can't see it. | ||
| if ((await fetchSites(coreClient)).some((s) => s.state.name === name)) { |
There was a problem hiding this comment.
This uses fetchSites() as an authoritative uniqueness check even though that helper catches per-zone read failures and returns a partial list. If the existing imported site's storage read fails transiently, creation proceeds with the same logical name; subsequent name-based resolution then fails because multiple sites match. The create path should fail closed when any candidate was unreadable rather than merely printing the partial-list warning.
Useful? React with 👍 / 👎.
| if (elapsed < SETTLE_FLOOR_MS) { | ||
| await promoteVerification.wait(SETTLE_FLOOR_MS - elapsed); | ||
| } | ||
| return confirmed; |
There was a problem hiding this comment.
The new boolean result is ignored by the unchanged-content deploy path and by migrateSite() (deploy.ts:473-475 and api.ts:992-995). When the edge does not confirm within the deadline, those commands still report an already-live or successfully migrated site without the warning emitted by the normal deploy and publish paths. Propagate the result through those callers and emit the same warning before reporting success.
Useful? React with 👍 / 👎.
💡 Codex Reviewcli/packages/cli/src/commands/sites/provision.ts Lines 94 to 95 in d06088b Preserve API and unexpected error classifications
Re-throw AGENTS.md reference: AGENTS.md:L269-L282 cli/packages/cli/src/commands/sites/deploy.ts Lines 273 to 275 in d06088b Use the nested config name when selecting the site
When a deploy is launched from a repository root with a positional nested output directory, this fallback finds that project's cli/packages/cli/src/commands/sites/ci/scaffold.ts Lines 90 to 92 in d06088b Do not treat an unrelated root lockfile as the project's
For a nested project without its own lockfile, this unconditionally treats any repository-root lockfile as governing the project. If the nested package is not actually a workspace member—for example, an independent npm app under a repository whose root has cli/packages/cli/src/commands/sites/ci/scaffold.ts Lines 33 to 35 in d06088b Resolve the remote default branch before assuming main
When cli/packages/cli/src/commands/sites/uploader.ts Lines 46 to 49 in d06088b Keep symlink targets inside the deploy boundary
Following symlinks without checking the resolved target allows a harmless-looking entry inside the deploy directory to upload files from anywhere on the machine. For example, cli/packages/cli/src/commands/sites/api.ts Lines 530 to 531 in d06088b Abort duplicate-name checks when site discovery is incomplete
This uses cli/packages/cli/src/commands/sites/api.ts Lines 853 to 858 in d06088b Handle unconfirmed propagation at every promotion call site
The new boolean result is ignored by the unchanged-content deploy path and by ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const deployId = | ||
| alreadyUploaded?.id ?? (shaTaken ? identity.contentHash : identity.id); |
There was a problem hiding this comment.
If an older custom deploy ID equals the new content hash, holds different bytes, and is neither live nor previous, shaTaken selects that ID. The conflict check only asks before replacement when customId was passed on this run. The handler then deletes the older files without asking, so that ID can no longer restore its original deployment. Check fallback IDs for conflicts too, and choose another ID or require confirmation before replacing one.
| const lockDir = [projectRoot, root].find((dir) => | ||
| LOCKFILES.some((name) => existsSync(join(dir, name))), | ||
| ); | ||
| const packageManager = await detectPackageManager(lockDir ?? projectRoot); | ||
| return { | ||
| packageManager, | ||
| lockfile: lockDir !== undefined, |
There was a problem hiding this comment.
If a standalone project under apps/web has no lockfile, an unrelated root package-lock.json makes this return lockfile: true. The generated job still runs in apps/web, so it emits npm ci where no usable lockfile exists and fails before building. Only reuse the root lockfile when the nested project belongs to its workspace; otherwise generate a plain install.
8c57b08 to
212586d
Compare
3 participants
Lists
bunny sitesinbunny --helpas a public preview.