From 3216b5f6e6573320841c12bdeccde0b758b9bddc Mon Sep 17 00:00:00 2001 From: jamie-at-bunny Date: Fri, 28 Aug 2026 10:07:57 +0100 Subject: [PATCH 1/7] feat(sites): support a caller-supplied deploy ID via --deploy-id --- .changeset/sites-custom-deploy-id.md | 5 + AGENTS.md | 14 +- README.md | 1 + .../cli/src/commands/sites/constants.test.ts | 114 +++++++++++- packages/cli/src/commands/sites/constants.ts | 36 +++- .../cli/src/commands/sites/deploy-id.test.ts | 45 +++++ packages/cli/src/commands/sites/deploy-id.ts | 24 ++- .../cli/src/commands/sites/deploy.test.ts | 166 +++++++++++++++++- packages/cli/src/commands/sites/deploy.ts | 122 ++++++++++++- .../src/commands/sites/deployments/delete.ts | 14 +- .../src/commands/sites/deployments/list.ts | 14 +- .../src/commands/sites/deployments/publish.ts | 8 +- skills/bunny-cli/references/sites.md | 7 +- 13 files changed, 533 insertions(+), 37 deletions(-) create mode 100644 .changeset/sites-custom-deploy-id.md diff --git a/.changeset/sites-custom-deploy-id.md b/.changeset/sites-custom-deploy-id.md new file mode 100644 index 00000000..3ca4446b --- /dev/null +++ b/.changeset/sites-custom-deploy-id.md @@ -0,0 +1,5 @@ +--- +"@bunny.net/cli": minor +--- + +Add `--deploy-id` to `bunny sites deploy` so a deploy can carry your own release identifier diff --git a/AGENTS.md b/AGENTS.md index 95a45888..a0dec702 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -425,7 +425,7 @@ bunny-cli/ │ │ │ └── remove.ts # Delete a file or directory (alias: rm; positional, --zone, trailing slash = recursive); `/` empties the zone and takes the same double confirmation as deleting the zone, and requireConfirmable means unattended runs need --force │ │ ├── sites/ # Experimental (hidden from help and landing page) — static-site hosting (storage zone + pull zone + middleware router) │ │ │ ├── index.ts # defineNamespace("sites", false, ...): create/list/show/deploy/deployments/domains/link/unlink/upgrade-router/delete; describe:false keeps it out of help while it stabilizes -│ │ │ ├── constants.ts # SITES_MANIFEST (.bunny/site.json), REMOTE_STATE_PATH (_bunny/site.json), RemoteSiteState/DeployRecord types (state carries routerVersion), parseRemoteState (shape-checked; null = not a site), deployPrefix, deploy-ID + site-name validators (3-47 chars), suffixedResourceName/siteResourcePattern (zone names are `sites-{name}-{random 6}`: the prefix marks them in the dashboard, the suffix dodges the global zone namespace) +│ │ │ ├── constants.ts # SITES_MANIFEST (.bunny/site.json), REMOTE_STATE_PATH (_bunny/site.json), RemoteSiteState/DeployRecord types (state carries routerVersion), parseRemoteState (shape-checked; null = not a site), deployPrefix, deployIdError/isValidDeployId (the ID is interpolated into storage paths and the router's URL pathname, so the charset is a security boundary: alnum plus `-`/`_`/`.`, 4-64 chars, alphanumeric at both ends, no `..`; case is preserved, not folded, since a caller-supplied ID exists to match whatever produced the deploy and the ID never reaches a client-facing URL; deployIdError returns the reason so `deploy` can explain a rejected --deploy-id, isValidDeployId is the boolean guard delete/prune use before building a path), findDeploy (exact match plus the case-variant deploy, so a miss can say "did you mean" instead of a bare not-found), site-name validator (3-47 chars), suffixedResourceName/siteResourcePattern (zone names are `sites-{name}-{random 6}`: the prefix marks them in the dashboard, the suffix dodges the global zone namespace) │ │ │ ├── constants.test.ts # parseRemoteState round-trip/rejection + helper tests │ │ │ ├── api.ts # siteFiles IO seam (connect/download/upload/remove; swap in tests instead of mock.module), remote state read/write (sha256 etag optimistic lock: concurrent deploy records merge on mismatch, ours win per id; current/previous follow promotedTo, so last promote wins and non-promoting writers adopt the concurrent pointers), siteContextFromZone, fetchSites (pull zone listing → middleware+storage candidates → per-zone state verification), createSite (idempotent provisioning: storage zone → router script code+publish+CURRENT_DEPLOY → pull zone + MiddlewareScriptId attach → state; both zones share a random name suffix so globally-taken names can't block the create, retrying fresh suffixes on collision; resume adopts a stateless name-pattern zone, and state.name keeps the clean site name), promoteDeploy (env var PUT + purgeCache POST), ensureRouterCurrent (republishes the router when state.routerVersion lags ROUTER_VERSION), fetchSystemHostname, deleteSiteResources (pull zone → script → storage zone, best-effort), deleteDeployFiles. fetchSites only accepts a candidate whose state names it as the site's own pullZoneId, so another zone sharing the storage origin is never mistaken for a site │ │ │ ├── api.test.ts # In-memory siteFiles store + path-branching fake clients: state round-trip, etag conflict, createSite fresh/resume/already-exists, promote, fetchSites filtering @@ -433,8 +433,8 @@ bunny-cli/ │ │ │ ├── provision.ts # promptSiteName (normalize/validate, directory-name suggestion) + createSiteWithProgress (createSite under a step-tracking spinner; shared with create.ts) + createLinkedSite (create + manifest link → SiteContext, skipping create's domain/CI prompts) for the deploy picker's new-site branch │ │ │ ├── config.ts # loadSiteConfig: reads bunny.jsonc via core/bunny-config.ts and validates ONLY the `sites` block (SiteConfigSchema from @bunny.net/config), so sites-only configs work without an `app` block or `version` │ │ │ ├── router/source.ts # routerSource + ROUTER_VERSION (recorded in site state; deploy republishes stale routers via ensureRouterCurrent): the middleware Edge Script, one per site, attached to its pull zone. CRITICAL platform gotcha: at the edge ctx.request.url is the ORIGIN-facing address (http://:9000/...), NOT the requested host; the client hostname comes from the CDN-Host/Host headers (clientHostname()), and the index-retry probe URL must be rebuilt on that host so it re-enters the CDN instead of hitting unrouted storage paths. Every request → CURRENT_DEPLOY's directory, root-served so client-side routers and root-absolute assets work as-is; /_bunny/* → 403 (the client-sent x-bunny-index-retry header is stripped; the flag is router-internal), trailing-slash → index.html, and a slashless GET/HEAD 404 probes its directory index (re-entrant HEAD of the URL + "/") and 301-redirects to the slash URL when it exists (so /blog resolves with the right relative-URL base, while exact extensionless objects and dotted directories stay reachable) -│ │ │ ├── deploy-id.ts # gitIdentity (short sha + dirty check via Bun.spawn), contentHashId (sorted path+sha256 merkle → 8 hex), resolveDeployIdentity (clean git → sha, else content hash) -│ │ │ ├── deploy-id.test.ts # Hash determinism + real temp git repos (clean → sha, dirty → content hash) +│ │ │ ├── deploy-id.ts # gitIdentity (short sha + dirty check via Bun.spawn), contentHashId (sorted path+sha256 merkle → 12 hex), resolveDeployIdentity (explicit --deploy-id wins, else clean git → sha, else content hash; a custom id still records the git sha for provenance, and contentHash is always computed so change detection never depends on the display id) +│ │ │ ├── deploy-id.test.ts # Hash determinism + real temp git repos (clean → sha, dirty → content hash) + custom-id precedence in and out of a repo │ │ │ ├── uploader.ts # collectFiles (recursive walk, skips dotfiles/node_modules, sorted), hashFiles (streaming sha256), uploadDeploy (8-way concurrency, per-file checksum, 3-attempt backoff retry) via siteFiles.upload │ │ │ ├── uploader.test.ts # Walk/skip/hash tests + upload paths/checksums/retry via siteFiles swap │ │ │ ├── build.ts # resolveAutoBuild (framework preset or package.json build script, via ci/frameworks detection) + runBuildCommand (Bun.spawn shell, caller env + overrides, throws on non-zero exit) @@ -444,13 +444,13 @@ bunny-cli/ │ │ │ ├── show.ts # Site details + hostname table (SSL cert + Force SSL columns); a failed hostname fetch hides the table, never the site │ │ │ ├── open.ts # bunny sites open [site]: open the live URL (recorded custom domain when the zone still serves it, else system host) in the browser; --print emits it, siteLiveUrl is the pure resolver │ │ │ ├── ssl.ts # bunny sites ssl [site]: toggle Force HTTPS on the site's b-cdn.net system host via setForceSsl (no cert issued; --no-force-ssl allows HTTP); custom domains use `sites domains ssl` -│ │ │ ├── deploy.ts # bunny sites deploy [dir]: resolve site (picker offers to create a new site when none is linked) → build (--build resolves flag command → `sites.build` → detected build, failing before any site is created; no --build offers a detected/configured build interactively; without a dir arg, --build deploys the detected framework's output dir) → router upgrade check (ensureRouterCurrent; a failed republish is a warning, since the old router still resolves CURRENT_DEPLOY) → hash → no-op if unchanged AND already live → upload deploys/{id}/ → state write → promote. Every deploy publishes; rollback to any earlier deploy is `deployments publish`. A domainless site's first-ever deploy also offers a custom production domain (setupSiteDomain, interactive text runs only); later domainless deploys print a dim `sites domains add` hint +│ │ │ ├── deploy.ts # bunny sites deploy [dir]: resolve site (picker offers to create a new site when none is linked) → build (--build resolves flag command → `sites.build` → detected build, failing before any site is created; no --build offers a detected/configured build interactively; without a dir arg, --build deploys the detected framework's output dir) → router upgrade check (ensureRouterCurrent; a failed republish is a warning, since the old router still resolves CURRENT_DEPLOY) → hash → resolveDeployTarget (pure, exported: picks the ID and whether the upload can be skipped; change detection keys on contentHash not the display id, an explicit --deploy-id never aliases onto an earlier deploy that merely shares content, and conflicts are returned rather than resolved: reason "content" (same id, different bytes) is refusable with --force, reason "case" (an id differing only in case exists) never is, because two storage paths differing only by case are indistinguishable to anything that folds case) → no-op if unchanged AND already live → upload deploys/{id}/ → state write → promote. Every deploy publishes; rollback to any earlier deploy is `deployments publish`. A domainless site's first-ever deploy also offers a custom production domain (setupSiteDomain, interactive text runs only); later domainless deploys print a dim `sites domains add` hint │ │ │ ├── link.ts # Link directory to a site (.bunny/site.json) │ │ │ ├── unlink.ts # Remove .bunny/site.json │ │ │ ├── upgrade-router.ts # Republish the site's router script with the CLI's current source and record ROUTER_VERSION in state (deploy also auto-republishes stale routers) │ │ │ ├── delete.ts # Delete a site (typed-name confirm; --keep-storage; drops .bunny/site.json if it pointed here) │ │ │ ├── ci/ # frameworks.ts (preset table of ~30 frameworks across js/ruby/hugo/python/zola/dotnet toolchains + detection: package.json deps/Gemfile/python+zola config files + lockfile pm), workflow.ts (renderSitesWorkflow -> .github/workflows/bunny-sites.yml using BunnyWay/actions/deploy-site: push-to-main + workflow_dispatch, contents:read + deployments:write; optional dir/build override the preset, workingDirectory/cacheDependencyPath place a project that sits below the workflow root (`defaults.run.working-directory` covers every run step, `uses` inputs take the prefix via workflowPath instead), installDeps adds the JS setup/install steps to a configured build the static preset wouldn't have installed for, and a configured build command is always a quoted scalar so YAML can't retype it), scaffold.ts (git helpers, projectPrefix (bunny.jsonc directory relative to the git root, realpath-resolved; undefined when it escapes the root, which drops its paths with a warning), framework/package-manager detection runs in that project directory, scaffoldSitesWorkflow -> ScaffoldResult.dir is the effective root-relative deploy dir, printWorkflowInstructions, offerGitHubSecret via gh), init.ts (bunny sites ci init) + tests -│ │ │ ├── deployments/ # list (● Live/○ Previous), publish [id]|--previous (alias promote; confirm + promote + current/previous swap), prune --keep N (resolveKeepCount validates the count first; pruneVictims never drops current/previous) + prune.test.ts, delete [id] (single-deploy cleanup: deleteBlocker refuses current/previous with --force only skipping the confirmation, revalidated on freshly re-read state inside the destructive phase since the confirmation window can race a concurrent deploy; an already-gone id is a no-op success so re-runs converge) + delete.test.ts +│ │ │ ├── deployments/ # list (● Live/○ Previous; deploySource renders git/content/custom, showing the recorded git sha next to a custom id), publish [id]|--previous (alias promote; confirm + promote + current/previous swap), prune --keep N (resolveKeepCount validates the count first; pruneVictims never drops current/previous) + prune.test.ts, delete [id] (single-deploy cleanup: deleteBlocker refuses current/previous with --force only skipping the confirmation, revalidated on freshly re-read state inside the destructive phase since the confirmation window can race a concurrent deploy; an already-gone id is a no-op success so re-runs converge) + delete.test.ts │ │ │ └── domains/index.ts # Mounts core/hostnames createHostnamesCommands as "sites domains" with onAdded/onRemoved hooks: the first added domain is recorded as state.domain (the production URL; recordSiteDomain rolls back the in-memory value if the state write fails), and an add on a site with nothing published hints at `sites deploy` (the domain serves the router's 404 until then); remove clears state.domain. setupSiteDomain (create --domain + deploy's first-run offer) records the domain only once the hostname is verifiably on the zone │ │ ├── registries/ │ │ │ ├── index.ts # Manual CommandModule (not defineNamespace); default handler runs list @@ -1199,8 +1199,8 @@ bunny │ ├── list (alias: ls) List sites (middleware+storage pull zones with matching remote state) │ ├── show [site] [--link] Show resources, domains (with SSL + Force SSL state), current deploy; warns when a newer router is available │ ├── open [site] [--print] Open the live URL (recorded custom domain when live, else system host) in the browser; --print emits it -│ ├── deploy [dir] [--site] [--link] [--build [cmd]] [--env K=V] [--env-file] [--force] -│ │ Deploy a directory: git short-sha ID when the tree is clean, content hash otherwise; identical IDs are no-ops (an already-uploaded ID skips the upload and just publishes). Every deploy is published as the live site. The target site resolves via selectSite (--site → linked → bunny.jsonc → picker); when nothing is linked, the interactive picker offers to create a new site (or, with no sites yet, goes straight to create) and links it. A [dir] arg is cwd-relative; without it the target is `sites.dir` (or the detected output dir), resolved against the bunny.jsonc directory where the build runs, else that directory (dotfiles + node_modules excluded). --build runs the command (or `sites.build`, else the detected build; resolved before any site is created so a missing command can't leave an orphan site) in the caller's environment plus --env/--env-file overrides. Without --build, an interactive run offers to run the configured `sites.build`, else a detected build (the CI framework preset's command, else a package.json `build` script); confirming builds first and, when no dir was given, deploys the framework's output dir. +│ ├── deploy [dir] [--site] [--link] [--build [cmd]] [--env K=V] [--env-file] [--force] [--deploy-id] +│ │ Deploy a directory: git short-sha ID when the tree is clean, content hash otherwise; identical IDs are no-ops (an already-uploaded ID skips the upload and just publishes). --deploy-id supplies the ID instead, so a deploy carries the identifier of whatever produced it (release tag, catalog build, timestamped artifact) and needs no cross-referencing in `deployments list`; it is used exactly as given (case included; IDs are case-sensitive, and publish/delete suggest a case variant on a miss), never aliases onto an earlier deploy sharing the same content, and reusing it for different content is refused without --force (rolling back to that ID would otherwise serve the new files); an ID differing from an existing deploy only in case is refused outright. Every deploy is published as the live site. The target site resolves via selectSite (--site → linked → bunny.jsonc → picker); when nothing is linked, the interactive picker offers to create a new site (or, with no sites yet, goes straight to create) and links it. A [dir] arg is cwd-relative; without it the target is `sites.dir` (or the detected output dir), resolved against the bunny.jsonc directory where the build runs, else that directory (dotfiles + node_modules excluded). --build runs the command (or `sites.build`, else the detected build; resolved before any site is created so a missing command can't leave an orphan site) in the caller's environment plus --env/--env-file overrides. Without --build, an interactive run offers to run the configured `sites.build`, else a detected build (the CI framework preset's command, else a package.json `build` script); confirming builds first and, when no dir was given, deploys the framework's output dir. │ ├── deployments │ │ ├── list [site] [--link] (alias: ls) List deploys (● Live / ○ Previous markers, created, source, files, size) │ │ ├── publish [id] [--previous] [--site] [--link] [--force] (alias: promote) diff --git a/README.md b/README.md index 8fd9383e..467b5f56 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,7 @@ bun ny sites create my-site # provision a static site (storage z bun ny sites deploy # no linked site? offers to create one or pick an existing; detects the framework, offers to build, then deploys (a site's first deploy also offers to attach a custom domain) bun ny sites deploy ./dist # deploy a directory and publish it as the live site bun ny sites deploy --build # run `sites.build` from bunny.jsonc (else the detected framework's build), then deploy `sites.dir` (or the detected output dir) +bun ny sites deploy ./catalog --deploy-id 20260827-1433-r42 # identify the deploy with your own release ID instead of the git sha / content hash bun ny sites deployments list # list deploys with the live one marked bun ny sites deployments publish --previous # instant rollback to the previous deploy bun ny sites deployments prune # delete old deploys (keeps the newest 5, never current/previous) diff --git a/packages/cli/src/commands/sites/constants.test.ts b/packages/cli/src/commands/sites/constants.test.ts index 101b3c98..8fd0940d 100644 --- a/packages/cli/src/commands/sites/constants.test.ts +++ b/packages/cli/src/commands/sites/constants.test.ts @@ -1,6 +1,9 @@ -import { expect, test } from "bun:test"; +import { describe, expect, test } from "bun:test"; +import type { DeployRecord } from "./constants.ts"; import { + deployIdError, deployPrefix, + findDeploy, isValidDeployId, isValidSiteName, parseRemoteState, @@ -48,11 +51,12 @@ test("deploy path helper", () => { expect(deployPrefix("a1b2c3d4")).toBe("deploys/a1b2c3d4"); }); -test("isValidDeployId accepts git shas and content hashes", () => { +test("isValidDeployId accepts git shas, content hashes and caller-supplied IDs", () => { expect(isValidDeployId("a1b2c3d4")).toBe(true); expect(isValidDeployId("0f9e8d7c6b5a4321")).toBe(true); + // Case is part of a caller-supplied ID, not something to normalize away. + expect(isValidDeployId("HAS-CAPS")).toBe(true); expect(isValidDeployId("ab")).toBe(false); // too short - expect(isValidDeployId("HAS-CAPS")).toBe(false); expect(isValidDeployId("has/slash")).toBe(false); expect(isValidDeployId("")).toBe(false); }); @@ -82,3 +86,107 @@ test("suffixed resource names round-trip through the site pattern", () => { }); // Cleanup and site discovery key on the name shape, and the router parses the same shape from the hostname, so the round-trip must be exact and everything else rejected. + +describe("deployIdError", () => { + test("accepts git shas and content hashes", () => { + expect(deployIdError("a1b2c3d4")).toBeNull(); + expect(deployIdError("0f1e2d3c4b5a")).toBeNull(); + }); + + test("accepts the release-style IDs a custom deploy needs", () => { + for (const id of [ + "20260827-1433-r42", + "catalog_v3", + "2026.08.27-r42", + "v1.2.3", + "release-2026-08-27t14.33.00z", + ]) { + expect(deployIdError(id)).toBeNull(); + } + }); + + // The ID is interpolated into a storage path and into the router's URL pathname. + test("rejects anything that could escape the deploy prefix", () => { + for (const id of [ + "../etc/passwd", + "..", + "a/../b", + "deploys/../../x", + "foo..bar", + "a/b", + "a\\b", + "a b", + "a?b", + "a#b", + "a%2fb", + "a:b", + ]) { + expect(deployIdError(id)).not.toBeNull(); + expect(isValidDeployId(id)).toBe(false); + } + }); + + test("rejects separators at the edges, so a path segment is never empty or hidden", () => { + for (const id of ["-abc", "abc-", ".abc", "abc.", "_abc", "abc_"]) { + expect(deployIdError(id)).not.toBeNull(); + } + }); + + // The ID exists to match whatever produced the deploy, so its case is data, not style. + test("accepts mixed case and preserves it", () => { + expect(deployIdError("Release-42")).toBeNull(); + expect(deployIdError("Catalog_V3")).toBeNull(); + expect(deployIdError("ABC1")).toBeNull(); + }); + + test("enforces the length bounds", () => { + expect(deployIdError("abc")).toBe("must be 4 to 64 characters"); + expect(deployIdError("a".repeat(64))).toBeNull(); + expect(deployIdError("a".repeat(65))).toBe("must be 4 to 64 characters"); + }); + + test("every accepted ID survives a round trip through a URL pathname", () => { + for (const id of ["20260827-1433-r42", "2026.08.27-r42", "catalog_v3"]) { + const url = new URL(`https://example.b-cdn.net/deploys/${id}/index.html`); + expect(url.pathname).toBe(`/deploys/${id}/index.html`); + } + }); +}); + +describe("findDeploy", () => { + const rec = (id: string): DeployRecord => ({ + id, + createdAt: "2026-08-27T00:00:00.000Z", + source: "custom", + contentHash: "hash1", + files: 1, + bytes: 10, + }); + + test("matches exactly, never by case", () => { + const deploys = [rec("Release-42")]; + expect(findDeploy(deploys, "Release-42").deploy?.id).toBe("Release-42"); + expect(findDeploy(deploys, "release-42").deploy).toBeUndefined(); + }); + + test("surfaces a case variant so a miss can say 'did you mean'", () => { + const deploys = [rec("Release-42")]; + expect(findDeploy(deploys, "release-42").caseVariant?.id).toBe( + "Release-42", + ); + expect(findDeploy(deploys, "RELEASE-42").caseVariant?.id).toBe( + "Release-42", + ); + }); + + test("an exact hit reports no variant", () => { + const found = findDeploy([rec("Release-42")], "Release-42"); + expect(found.caseVariant).toBeUndefined(); + }); + + test("an unrelated id reports neither", () => { + expect(findDeploy([rec("Release-42")], "r99")).toEqual({ + caseVariant: undefined, + }); + }); +}); diff --git a/packages/cli/src/commands/sites/constants.ts b/packages/cli/src/commands/sites/constants.ts index 0728e13e..c13f2fa6 100644 --- a/packages/cli/src/commands/sites/constants.ts +++ b/packages/cli/src/commands/sites/constants.ts @@ -23,7 +23,8 @@ export interface SiteManifest { export interface DeployRecord { id: string; createdAt: string; - source: "git" | "content"; + /** How the ID was chosen; "custom" means the caller supplied it with --deploy-id. */ + source: "git" | "content" | "custom"; gitSha?: string; dirty?: boolean; /** Hash of the deployed bytes; the no-op check keys on this. */ @@ -81,11 +82,38 @@ export function routerScriptName(siteName: string): string { return `${siteName}-router`; } -// Deploy IDs are git short-shas or content hashes (lowercase hex-ish); the router regex and storage paths rely on this. -const DEPLOY_ID_RE = /^[a-z0-9]{4,40}$/; +// A deploy ID becomes a storage path and the router's CURRENT_DEPLOY, so its charset is a boundary, not a style choice: alphanumerics plus `-`, `_` and `.`, bounded by an alphanumeric, and never a traversal sequence. Case is preserved rather than folded: a caller-supplied ID exists to match whatever produced the deploy, and the ID never reaches a client-facing URL (the router builds the origin path itself), so nothing downstream needs it normalized. +const DEPLOY_ID_RE = /^[A-Za-z0-9][A-Za-z0-9._-]{2,62}[A-Za-z0-9]$/; + +/** Why an ID is unusable, or null when it's fine. Phrased to complete "Deploy ID ...". */ +export function deployIdError(id: string): string | null { + if (id.length < 4 || id.length > 64) return "must be 4 to 64 characters"; + if (id.includes("..")) return 'must not contain ".."'; + if (!DEPLOY_ID_RE.test(id)) { + return "may use only letters, digits, and -, _ or ., and must start and end with a letter or digit"; + } + return null; +} export function isValidDeployId(id: string): boolean { - return DEPLOY_ID_RE.test(id); + return deployIdError(id) === null; +} + +/** + * Look up a deploy by ID, exactly. + * + * `caseVariant` is the deploy that differs only in case, so a caller can say + * "did you mean" instead of a bare not-found: IDs preserve the case they were + * given, and eyeballing `Release-42` against `release-42` in a list is no fun. + */ +export function findDeploy( + deploys: DeployRecord[], + id: string, +): { deploy?: DeployRecord; caseVariant?: DeployRecord } { + const deploy = deploys.find((d) => d.id === id); + if (deploy) return { deploy }; + const lower = id.toLowerCase(); + return { caseVariant: deploys.find((d) => d.id.toLowerCase() === lower) }; } // Site names become `sites-{name}-{suffix}` zone names; 3-47 chars keeps those within zone-name limits. diff --git a/packages/cli/src/commands/sites/deploy-id.test.ts b/packages/cli/src/commands/sites/deploy-id.test.ts index 76d53b2e..ae5977da 100644 --- a/packages/cli/src/commands/sites/deploy-id.test.ts +++ b/packages/cli/src/commands/sites/deploy-id.test.ts @@ -82,3 +82,48 @@ test("clean git repo uses the short sha; dirty tree falls back to content", asyn expect(dirty.id).toBe(contentHashId(FILES)); expect(dirty.gitSha).toBe(clean.id); }); + +test("a custom id wins over git and content, but still records both", async () => { + const dir = mkdtempSync(join(tmpdir(), "bunny-sites-custom-")); + await run(dir, ["init", "-q"]); + await Bun.write(join(dir, "index.html"), "

hi

"); + await run(dir, ["add", "."]); + await run(dir, [ + "-c", + "user.email=test@example.com", + "-c", + "user.name=test", + "commit", + "-q", + "-m", + "init", + ]); + + const identity = await resolveDeployIdentity(dir, FILES, "20260827-1433-r42"); + expect(identity.id).toBe("20260827-1433-r42"); + expect(identity.source).toBe("custom"); + // Provenance survives: the git sha is still recorded, and the content hash still drives the no-op check. + expect(identity.gitSha).toMatch(/^[0-9a-f]{8}$/); + expect(identity.contentHash).toBe(contentHashId(FILES)); +}); + +test("a custom id works outside a git repo", async () => { + const dir = mkdtempSync(join(tmpdir(), "bunny-sites-custom-nogit-")); + const identity = await resolveDeployIdentity(dir, FILES, "catalog_v3"); + expect(identity.id).toBe("catalog_v3"); + expect(identity.source).toBe("custom"); + expect(identity.gitSha).toBeUndefined(); + expect(identity.contentHash).toBe(contentHashId(FILES)); +}); + +test("the same custom id with different content yields a different content hash", async () => { + const dir = mkdtempSync(join(tmpdir(), "bunny-sites-custom-drift-")); + const a = await resolveDeployIdentity(dir, FILES, "r42"); + const b = await resolveDeployIdentity( + dir, + [{ path: "index.html", sha256: "ff99" }], + "r42", + ); + expect(a.id).toBe(b.id); + expect(a.contentHash).not.toBe(b.contentHash); +}); diff --git a/packages/cli/src/commands/sites/deploy-id.ts b/packages/cli/src/commands/sites/deploy-id.ts index b810348d..4f6b8d59 100644 --- a/packages/cli/src/commands/sites/deploy-id.ts +++ b/packages/cli/src/commands/sites/deploy-id.ts @@ -9,7 +9,7 @@ export interface HashedFile { export interface DeployIdentity { id: string; - source: "git" | "content"; + source: "git" | "content" | "custom"; gitSha?: string; dirty?: boolean; // Hash of the deployed bytes; the no-op check keys on this (not `id`), so a rebuilt `dist/` at the same git sha isn't wrongly skipped. @@ -40,13 +40,33 @@ export async function gitIdentity( }; } -// Resolve the deploy identity: display `id` is the git short-sha on a clean tree, else the content hash; `contentHash` always hashes what ships and drives the no-op check. +/** + * Resolve the deploy identity. + * + * `customId` wins when given, so a release can carry the same ID as whatever + * produced it. Otherwise the display `id` is the git short-sha on a clean tree + * and the content hash elsewhere. `contentHash` always hashes what ships and + * drives the no-op check, so an explicit ID never disturbs change detection; + * the git sha is still recorded when there is one, for provenance. + */ export async function resolveDeployIdentity( cwd: string, files: HashedFile[], + customId?: string, ): Promise { const contentHash = contentHashId(files); const gitInfo = await gitIdentity(cwd); + + if (customId) { + return { + id: customId, + source: "custom", + gitSha: gitInfo?.sha, + dirty: gitInfo?.dirty, + contentHash, + }; + } + if (gitInfo && !gitInfo.dirty) { return { id: gitInfo.sha, source: "git", gitSha: gitInfo.sha, contentHash }; } diff --git a/packages/cli/src/commands/sites/deploy.test.ts b/packages/cli/src/commands/sites/deploy.test.ts index ad2e303a..4c9e945d 100644 --- a/packages/cli/src/commands/sites/deploy.test.ts +++ b/packages/cli/src/commands/sites/deploy.test.ts @@ -1,7 +1,12 @@ -import { expect, test } from "bun:test"; +import { describe, expect, test } from "bun:test"; import { resolve } from "node:path"; -import type { RemoteSiteState } from "./constants.ts"; -import { productionUrl, resolveDeployDir } from "./deploy.ts"; +import type { DeployRecord, RemoteSiteState } from "./constants.ts"; +import { + productionUrl, + resolveDeployDir, + resolveDeployTarget, +} from "./deploy.ts"; +import type { DeployIdentity } from "./deploy-id.ts"; const ROOT = "/project/root"; @@ -39,3 +44,158 @@ test("productionUrl prefers the custom domain over the system host", () => { test("productionUrl is undefined when the site has neither a domain nor a system host", () => { expect(productionUrl(stateWithDomain(undefined), undefined)).toBeUndefined(); }); + +const deploy = ( + id: string, + contentHash: string, + source: DeployRecord["source"] = "content", +): DeployRecord => ({ + id, + createdAt: "2026-08-27T00:00:00.000Z", + source, + contentHash, + files: 1, + bytes: 10, +}); + +const identity = ( + id: string, + contentHash: string, + source: DeployIdentity["source"] = "content", +): DeployIdentity => ({ id, source, contentHash }); + +describe("resolveDeployTarget", () => { + test("unchanged content reuses the existing deploy and skips the upload", () => { + const target = resolveDeployTarget({ + deploys: [deploy("aaaa1111", "hash1")], + identity: identity("aaaa1111", "hash1"), + force: false, + }); + expect(target).toEqual({ deployId: "aaaa1111", skipUpload: true }); + }); + + test("without a custom id, matching content aliases onto the earlier deploy's id", () => { + const target = resolveDeployTarget({ + deploys: [deploy("aaaa1111", "hash1")], + identity: identity("bbbb2222", "hash1", "git"), + force: false, + }); + expect(target.deployId).toBe("aaaa1111"); + expect(target.skipUpload).toBe(true); + }); + + // A catalog release must keep its own ID even when the bytes happen to match the last one. + test("a custom id never aliases onto a different deploy that shares content", () => { + const target = resolveDeployTarget({ + deploys: [deploy("r41", "hash1")], + identity: identity("r42", "hash1", "custom"), + customId: "r42", + force: false, + }); + expect(target.deployId).toBe("r42"); + expect(target.skipUpload).toBe(false); + expect(target.conflict).toBeUndefined(); + }); + + test("redeploying a custom id with identical content is a no-op", () => { + const target = resolveDeployTarget({ + deploys: [deploy("r42", "hash1", "custom")], + identity: identity("r42", "hash1", "custom"), + customId: "r42", + force: false, + }); + expect(target).toEqual({ deployId: "r42", skipUpload: true }); + }); + + test("reusing a custom id for different content is a conflict, not a silent overwrite", () => { + const existing = deploy("r42", "hash1", "custom"); + const target = resolveDeployTarget({ + deploys: [existing], + identity: identity("r42", "hash2", "custom"), + customId: "r42", + force: false, + }); + expect(target.conflict).toEqual({ record: existing, reason: "content" }); + expect(target.deployId).toBe("r42"); + }); + + test("a custom id is used exactly as given, case and all", () => { + const target = resolveDeployTarget({ + deploys: [], + identity: identity("Release-42", "hash1", "custom"), + customId: "Release-42", + force: false, + }); + expect(target.deployId).toBe("Release-42"); + expect(target.conflict).toBeUndefined(); + }); + + // Two deploys whose storage paths differ only by case are indistinguishable to anything that folds case. + test("an id differing from an existing deploy only in case is refused", () => { + const existing = deploy("Release-42", "hash1", "custom"); + const target = resolveDeployTarget({ + deploys: [existing], + identity: identity("release-42", "hash2", "custom"), + customId: "release-42", + force: false, + }); + expect(target.conflict).toEqual({ record: existing, reason: "case" }); + }); + + test("--force does not override a case-variant conflict", () => { + const existing = deploy("Release-42", "hash1", "custom"); + const target = resolveDeployTarget({ + deploys: [existing], + identity: identity("release-42", "hash2", "custom"), + customId: "release-42", + force: true, + }); + expect(target.conflict).toEqual({ record: existing, reason: "case" }); + }); + + test("reusing the exact existing casing is a normal redeploy, not a case conflict", () => { + const target = resolveDeployTarget({ + deploys: [deploy("Release-42", "hash1", "custom")], + identity: identity("Release-42", "hash1", "custom"), + customId: "Release-42", + force: false, + }); + expect(target).toEqual({ deployId: "Release-42", skipUpload: true }); + }); + + test("--force overrides the conflict and forces a fresh upload", () => { + const target = resolveDeployTarget({ + deploys: [deploy("r42", "hash1", "custom")], + identity: identity("r42", "hash2", "custom"), + customId: "r42", + force: true, + }); + expect(target.conflict).toBeUndefined(); + expect(target.skipUpload).toBe(false); + expect(target.deployId).toBe("r42"); + }); + + test("--force redeploys unchanged content under the same id", () => { + const target = resolveDeployTarget({ + deploys: [deploy("r42", "hash1", "custom")], + identity: identity("r42", "hash1", "custom"), + customId: "r42", + force: true, + }); + expect(target.skipUpload).toBe(false); + expect(target.deployId).toBe("r42"); + }); + + test("a brand new custom id on an empty site just uploads", () => { + const target = resolveDeployTarget({ + deploys: [], + identity: identity("20260827-1433-r42", "hash1", "custom"), + customId: "20260827-1433-r42", + force: false, + }); + expect(target).toEqual({ + deployId: "20260827-1433-r42", + skipUpload: false, + }); + }); +}); diff --git a/packages/cli/src/commands/sites/deploy.ts b/packages/cli/src/commands/sites/deploy.ts index 2609516e..07d118ba 100644 --- a/packages/cli/src/commands/sites/deploy.ts +++ b/packages/cli/src/commands/sites/deploy.ts @@ -28,10 +28,12 @@ import { import { loadSiteConfig } from "./config.ts"; import { type DeployRecord, + deployIdError, + findDeploy, markCurrent, type RemoteSiteState, } from "./constants.ts"; -import { resolveDeployIdentity } from "./deploy-id.ts"; +import { type DeployIdentity, resolveDeployIdentity } from "./deploy-id.ts"; import { setupSiteDomain } from "./domains/index.ts"; import { type SiteSelectorArgs, @@ -48,11 +50,80 @@ interface DeployArgs extends SiteSelectorArgs { env?: string[]; "env-file"?: string; force?: boolean; + "deploy-id"?: string; +} + +export interface DeployTarget { + /** The ID this deploy will live under in storage. */ + deployId: string; + /** True when these exact bytes are already uploaded under `deployId`. */ + skipUpload: boolean; + /** + * An existing deploy that blocks this one. + * + * `content`: the same ID already holds different bytes; --force replaces it. + * `case`: an ID differing only in case exists. Not forceable, because two + * deploys whose paths differ only by case are indistinguishable to anything + * that folds case, and the loser's files would back the winner's rollback. + */ + conflict?: { record: DeployRecord; reason: "content" | "case" }; +} + +/** + * Decide which ID this deploy lands under and whether the upload can be skipped. + * + * Change detection keys on content, not the display ID, so a rebuilt `dist/` at + * the same git sha is never wrongly skipped. An explicit ID is an assertion + * about identity, so it never aliases onto an earlier deploy that merely shares + * content: a catalog release keeps its own ID even when the bytes are identical + * to the last one. Reusing an ID for different bytes rewrites what a rollback + * to it would serve, so that is reported as a conflict rather than done quietly. + */ +export function resolveDeployTarget(opts: { + deploys: DeployRecord[]; + identity: DeployIdentity; + customId?: string; + force: boolean; +}): DeployTarget { + const { deploys, identity, customId, force } = opts; + + const alreadyUploaded = force + ? undefined + : deploys.find((d) => + customId + ? d.id === customId && d.contentHash === identity.contentHash + : d.contentHash === identity.contentHash, + ); + // A skipped deploy reuses the already-uploaded deploy's id; that's where its files live. + const deployId = alreadyUploaded?.id ?? identity.id; + const skipUpload = alreadyUploaded !== undefined; + + if (customId && !skipUpload) { + const { deploy: exact, caseVariant } = findDeploy(deploys, customId); + if (caseVariant) { + return { + deployId, + skipUpload, + conflict: { record: caseVariant, reason: "case" }, + }; + } + if (exact && exact.contentHash !== identity.contentHash && !force) { + return { + deployId, + skipUpload, + conflict: { record: exact, reason: "content" }, + }; + } + } + return { deployId, skipUpload }; } const DOMAIN_HINT = " Add a custom production domain: bunny sites domains add "; +const DEPLOY_ID_HINT = + "IDs become storage paths, so they take letters, digits, and -, _ or . (e.g. 20260827-1433-r42)."; + // A site's live URL: the custom domain when it has one, else its b-cdn.net host. Always https (b-cdn.net hosts carry bunny's certificate). export function productionUrl( state: RemoteSiteState, @@ -85,6 +156,10 @@ export const sitesDeployCommand = defineCommand({ "Explicit build command", ], ["$0 sites deploy ./dist --site my-site", "Target a specific site"], + [ + "$0 sites deploy ./catalog --deploy-id 20260827-1433-r42", + "Identify the deploy with your own release ID", + ], ], builder: (yargs) => @@ -114,6 +189,11 @@ export const sitesDeployCommand = defineCommand({ type: "boolean", default: false, describe: "Deploy even when the content is unchanged", + }) + .option("deploy-id", { + type: "string", + describe: + "Identify this deploy yourself (e.g. a release or catalog ID) instead of using the git sha or content hash; used exactly as given", }), ), @@ -223,15 +303,39 @@ export const sitesDeployCommand = defineCommand({ } const totalBytes = files.reduce((sum, f) => sum + f.size, 0); - const identity = await resolveDeployIdentity(dir, files); + const customId = args["deploy-id"]?.trim(); + if (customId) { + const problem = deployIdError(customId); + if (problem) { + throw new UserError( + `Deploy ID "${customId}" ${problem}.`, + DEPLOY_ID_HINT, + ); + } + } - // The no-op check keys on content, not the display id, so a rebuilt `dist/` at the same git sha isn't wrongly skipped. - const alreadyUploaded = args.force - ? undefined - : state.deploys.find((d) => d.contentHash === identity.contentHash); - const skipUpload = alreadyUploaded !== undefined; - // A skipped deploy reuses the already-uploaded deploy's id; that's where its files live. - const deployId = alreadyUploaded?.id ?? identity.id; + const identity = await resolveDeployIdentity(dir, files, customId); + const target = resolveDeployTarget({ + deploys: state.deploys, + identity, + customId, + force: args.force ?? false, + }); + + if (target.conflict?.reason === "case") { + throw new UserError( + `Deploy ${target.conflict.record.id} already exists for ${state.name}, differing from "${customId}" only in case.`, + `Reuse that exact ID to redeploy it, or pick one that isn't a case variant.`, + ); + } + if (target.conflict?.reason === "content") { + throw new UserError( + `Deploy ${customId} already exists for ${state.name} with different content.`, + "Rolling back to that ID would serve these new files instead of the originals. Pick another ID, or pass --force to replace it.", + ); + } + + const { deployId, skipUpload } = target; const alreadyLive = state.current === deployId; // The production URL prefers the custom domain; only fetch the system host when there is none. diff --git a/packages/cli/src/commands/sites/deployments/delete.ts b/packages/cli/src/commands/sites/deployments/delete.ts index 14aab61c..6fc170a2 100644 --- a/packages/cli/src/commands/sites/deployments/delete.ts +++ b/packages/cli/src/commands/sites/deployments/delete.ts @@ -10,7 +10,11 @@ import { readRemoteState, writeRemoteState, } from "../api.ts"; -import { isValidDeployId, type RemoteSiteState } from "../constants.ts"; +import { + findDeploy, + isValidDeployId, + type RemoteSiteState, +} from "../constants.ts"; import { type SiteSelectorArgs, selectSite, @@ -86,7 +90,7 @@ export const sitesDeploymentsDeleteCommand = defineCommand({ // No etag here: the destructive phase re-reads state and writes with the fresh one. const { state, connection } = site; - const record = state.deploys.find((d) => d.id === id); + const { deploy: record, caseVariant } = findDeploy(state.deploys, id); if (!record) { // Idempotent for CI: a retry after a successful delete still exits 0. if (output === "json") { @@ -98,6 +102,12 @@ export const sitesDeploymentsDeleteCommand = defineCommand({ logger.info( `Deploy ${id} not found on ${state.name}; nothing to delete.`, ); + // A case typo would otherwise look like a successful no-op. + if (caseVariant) { + logger.dim( + ` Did you mean ${caseVariant.id}? Deploy IDs are case-sensitive.`, + ); + } return; } diff --git a/packages/cli/src/commands/sites/deployments/list.ts b/packages/cli/src/commands/sites/deployments/list.ts index e1dfc5b4..c5b7860f 100644 --- a/packages/cli/src/commands/sites/deployments/list.ts +++ b/packages/cli/src/commands/sites/deployments/list.ts @@ -8,6 +8,7 @@ import { formatTable, } from "../../../core/format.ts"; import { logger } from "../../../core/logger.ts"; +import type { DeployRecord } from "../constants.ts"; import { type SiteSelectorArgs, selectSite, @@ -17,6 +18,15 @@ import { type ListArgs = SiteSelectorArgs; +// How a deploy got its ID, for the Source column. A custom ID still shows the git sha when one was recorded, since that's the only provenance it carries. +function deploySource(d: DeployRecord): string { + if (d.source === "git") return `git ${d.gitSha ?? d.id}`; + if (d.source === "custom") { + return d.gitSha ? `custom (git ${d.gitSha})` : "custom"; + } + return `content${d.dirty ? " (dirty tree)" : ""}`; +} + export const sitesDeploymentsListCommand = defineCommand({ command: "list [site]", aliases: ["ls"], @@ -76,9 +86,7 @@ export const sitesDeploymentsListCommand = defineCommand({ ? "○ Previous" : "○", formatDateTime(d.createdAt), - d.source === "git" - ? `git ${d.gitSha ?? d.id}` - : `content${d.dirty ? " (dirty tree)" : ""}`, + deploySource(d), String(d.files), formatBytes(d.bytes), ]), diff --git a/packages/cli/src/commands/sites/deployments/publish.ts b/packages/cli/src/commands/sites/deployments/publish.ts index a15477a3..fe5f874f 100644 --- a/packages/cli/src/commands/sites/deployments/publish.ts +++ b/packages/cli/src/commands/sites/deployments/publish.ts @@ -9,7 +9,7 @@ import { UserError } from "../../../core/errors.ts"; import { logger } from "../../../core/logger.ts"; import { confirm, requireConfirmable, withSpinner } from "../../../core/ui.ts"; import { promoteDeploy, writeRemoteState } from "../api.ts"; -import { markCurrent } from "../constants.ts"; +import { findDeploy, markCurrent } from "../constants.ts"; import { type SiteSelectorArgs, selectSite, @@ -88,11 +88,13 @@ export const sitesDeploymentsPublishCommand = defineCommand({ ); } - const deploy = state.deploys.find((d) => d.id === targetId); + const { deploy, caseVariant } = findDeploy(state.deploys, targetId); if (!deploy) { throw new UserError( `Deploy ${targetId} not found for site ${state.name}.`, - "Run `bunny sites deployments list` to see available deploys.", + caseVariant + ? `Did you mean ${caseVariant.id}? Deploy IDs are case-sensitive.` + : "Run `bunny sites deployments list` to see available deploys.", ); } diff --git a/skills/bunny-cli/references/sites.md b/skills/bunny-cli/references/sites.md index 222b431d..9b6d0339 100644 --- a/skills/bunny-cli/references/sites.md +++ b/skills/bunny-cli/references/sites.md @@ -43,7 +43,12 @@ Content is root-served, so client-side routers (TanStack Router, React Router, V ## Deploy IDs -- The deploy ID is the **git short-sha** when the working tree is clean, otherwise an 8-char **content hash**. Re-deploying identical content is a no-op (`--force` overrides). +- The deploy ID is the **git short-sha** when the working tree is clean, otherwise a 12-char **content hash**. Re-deploying identical content is a no-op (`--force` overrides). +- `--deploy-id ` sets the ID yourself, so a deploy can carry the same identifier as whatever produced it (a release tag, a catalog build, a timestamped artifact) and `deployments list` needs no cross-referencing. The ID is used **exactly as given**, case included: it exists to match your identifier, and it never appears in a client-facing URL (the router builds the origin path from it server-side). IDs become storage paths, so they take letters, digits and `-`, `_` or `.`, 4 to 64 characters, starting and ending alphanumeric: `20260827-1433-r42`, `Catalog_V3`, `v1.2.3`. + - Deploy IDs are therefore **case-sensitive**. `publish`/`delete` match exactly and suggest a case variant when one exists, and deploying an ID that differs from an existing one only in case is refused (not even with `--force`), since two storage paths differing only by case are indistinguishable to anything that folds case. + - An explicit ID is an assertion about identity, so it is never aliased onto an earlier deploy that happens to share content: each release keeps its own ID and rollback target even when the bytes are unchanged. + - Reusing an ID for **different** content is refused, because rolling back to that ID would then serve the new files instead of the originals. Pass `--force` to replace it deliberately. + - The git sha is still recorded alongside a custom ID when the deploy came from a repo, so provenance is not lost; `deployments list` shows it as `custom (git abc12345)`. - Dotfiles and `node_modules` are never uploaded. --- From fdcfeeffe9761398d6c3c94bf5124ea516ad3a9c Mon Sep 17 00:00:00 2001 From: jamie-at-bunny Date: Fri, 28 Aug 2026 11:20:59 +0100 Subject: [PATCH 2/7] refactor old tests --- .../cli/src/commands/sites/constants.test.ts | 150 +++++---------- .../cli/src/commands/sites/deploy-id.test.ts | 12 -- .../cli/src/commands/sites/deploy.test.ts | 180 +++++++----------- 3 files changed, 120 insertions(+), 222 deletions(-) diff --git a/packages/cli/src/commands/sites/constants.test.ts b/packages/cli/src/commands/sites/constants.test.ts index 8fd0940d..4ff55a58 100644 --- a/packages/cli/src/commands/sites/constants.test.ts +++ b/packages/cli/src/commands/sites/constants.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from "bun:test"; +import { expect, test } from "bun:test"; import type { DeployRecord } from "./constants.ts"; import { deployIdError, @@ -87,106 +87,58 @@ test("suffixed resource names round-trip through the site pattern", () => { // Cleanup and site discovery key on the name shape, and the router parses the same shape from the hostname, so the round-trip must be exact and everything else rejected. -describe("deployIdError", () => { - test("accepts git shas and content hashes", () => { - expect(deployIdError("a1b2c3d4")).toBeNull(); - expect(deployIdError("0f1e2d3c4b5a")).toBeNull(); - }); - - test("accepts the release-style IDs a custom deploy needs", () => { - for (const id of [ - "20260827-1433-r42", - "catalog_v3", - "2026.08.27-r42", - "v1.2.3", - "release-2026-08-27t14.33.00z", - ]) { - expect(deployIdError(id)).toBeNull(); - } - }); - - // The ID is interpolated into a storage path and into the router's URL pathname. - test("rejects anything that could escape the deploy prefix", () => { - for (const id of [ - "../etc/passwd", - "..", - "a/../b", - "deploys/../../x", - "foo..bar", - "a/b", - "a\\b", - "a b", - "a?b", - "a#b", - "a%2fb", - "a:b", - ]) { - expect(deployIdError(id)).not.toBeNull(); - expect(isValidDeployId(id)).toBe(false); - } - }); - - test("rejects separators at the edges, so a path segment is never empty or hidden", () => { - for (const id of ["-abc", "abc-", ".abc", "abc.", "_abc", "abc_"]) { - expect(deployIdError(id)).not.toBeNull(); - } - }); - - // The ID exists to match whatever produced the deploy, so its case is data, not style. - test("accepts mixed case and preserves it", () => { - expect(deployIdError("Release-42")).toBeNull(); - expect(deployIdError("Catalog_V3")).toBeNull(); - expect(deployIdError("ABC1")).toBeNull(); - }); - - test("enforces the length bounds", () => { - expect(deployIdError("abc")).toBe("must be 4 to 64 characters"); - expect(deployIdError("a".repeat(64))).toBeNull(); - expect(deployIdError("a".repeat(65))).toBe("must be 4 to 64 characters"); - }); - - test("every accepted ID survives a round trip through a URL pathname", () => { - for (const id of ["20260827-1433-r42", "2026.08.27-r42", "catalog_v3"]) { - const url = new URL(`https://example.b-cdn.net/deploys/${id}/index.html`); - expect(url.pathname).toBe(`/deploys/${id}/index.html`); - } - }); +test("deployIdError accepts shas, hashes, and release-style IDs, case intact", () => { + for (const id of [ + "a1b2c3d4", + "0f1e2d3c4b5a", + "20260827-1433-r42", + "catalog_v3", + "2026.08.27-r42", + "v1.2.3", + "Release-42", + "a".repeat(64), + ]) { + expect(deployIdError(id)).toBeNull(); + } }); -describe("findDeploy", () => { - const rec = (id: string): DeployRecord => ({ - id, - createdAt: "2026-08-27T00:00:00.000Z", - source: "custom", - contentHash: "hash1", - files: 1, - bytes: 10, - }); - - test("matches exactly, never by case", () => { - const deploys = [rec("Release-42")]; - expect(findDeploy(deploys, "Release-42").deploy?.id).toBe("Release-42"); - expect(findDeploy(deploys, "release-42").deploy).toBeUndefined(); - }); - - test("surfaces a case variant so a miss can say 'did you mean'", () => { - const deploys = [rec("Release-42")]; - expect(findDeploy(deploys, "release-42").caseVariant?.id).toBe( - "Release-42", - ); - expect(findDeploy(deploys, "RELEASE-42").caseVariant?.id).toBe( - "Release-42", - ); - }); - - test("an exact hit reports no variant", () => { - const found = findDeploy([rec("Release-42")], "Release-42"); - expect(found.caseVariant).toBeUndefined(); - }); +// The ID is interpolated into a storage path and the router's URL pathname, so anything +// that could escape the deploy prefix or leave an empty/hidden segment has to be rejected. +test("deployIdError rejects path escapes and edge separators", () => { + for (const id of [ + "../etc/passwd", + "a/../b", + "foo..bar", + "a/b", + "a\\b", + "a b", + "a?b", + "a%2fb", + "-abc", + "abc.", + "_abc", + ]) { + expect(deployIdError(id)).not.toBeNull(); + } + expect(deployIdError("abc")).toBe("must be 4 to 64 characters"); + expect(deployIdError("a".repeat(65))).toBe("must be 4 to 64 characters"); +}); - test("an unrelated id reports neither", () => { - expect(findDeploy([rec("Release-42")], "r99")).toEqual({ - caseVariant: undefined, - }); +test("findDeploy matches exactly and surfaces a case variant for 'did you mean'", () => { + const deploys: DeployRecord[] = [ + { + id: "Release-42", + createdAt: "2026-08-27T00:00:00.000Z", + source: "custom", + contentHash: "hash1", + files: 1, + bytes: 10, + }, + ]; + + expect(findDeploy(deploys, "Release-42")).toEqual({ deploy: deploys[0] }); + expect(findDeploy(deploys, "release-42")).toEqual({ + caseVariant: deploys[0], }); + expect(findDeploy(deploys, "r99")).toEqual({ caseVariant: undefined }); }); diff --git a/packages/cli/src/commands/sites/deploy-id.test.ts b/packages/cli/src/commands/sites/deploy-id.test.ts index ae5977da..a9555896 100644 --- a/packages/cli/src/commands/sites/deploy-id.test.ts +++ b/packages/cli/src/commands/sites/deploy-id.test.ts @@ -115,15 +115,3 @@ test("a custom id works outside a git repo", async () => { expect(identity.gitSha).toBeUndefined(); expect(identity.contentHash).toBe(contentHashId(FILES)); }); - -test("the same custom id with different content yields a different content hash", async () => { - const dir = mkdtempSync(join(tmpdir(), "bunny-sites-custom-drift-")); - const a = await resolveDeployIdentity(dir, FILES, "r42"); - const b = await resolveDeployIdentity( - dir, - [{ path: "index.html", sha256: "ff99" }], - "r42", - ); - expect(a.id).toBe(b.id); - expect(a.contentHash).not.toBe(b.contentHash); -}); diff --git a/packages/cli/src/commands/sites/deploy.test.ts b/packages/cli/src/commands/sites/deploy.test.ts index 4c9e945d..f284829e 100644 --- a/packages/cli/src/commands/sites/deploy.test.ts +++ b/packages/cli/src/commands/sites/deploy.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from "bun:test"; +import { expect, test } from "bun:test"; import { resolve } from "node:path"; import type { DeployRecord, RemoteSiteState } from "./constants.ts"; import { @@ -64,138 +64,96 @@ const identity = ( source: DeployIdentity["source"] = "content", ): DeployIdentity => ({ id, source, contentHash }); -describe("resolveDeployTarget", () => { - test("unchanged content reuses the existing deploy and skips the upload", () => { - const target = resolveDeployTarget({ - deploys: [deploy("aaaa1111", "hash1")], +test("matching content skips the upload, aliasing onto the existing deploy's id", () => { + const deploys = [deploy("aaaa1111", "hash1")]; + + expect( + resolveDeployTarget({ + deploys, identity: identity("aaaa1111", "hash1"), force: false, - }); - expect(target).toEqual({ deployId: "aaaa1111", skipUpload: true }); - }); + }), + ).toEqual({ deployId: "aaaa1111", skipUpload: true }); - test("without a custom id, matching content aliases onto the earlier deploy's id", () => { - const target = resolveDeployTarget({ - deploys: [deploy("aaaa1111", "hash1")], + // A different git sha over identical bytes reuses the earlier deploy rather than duplicating it. + expect( + resolveDeployTarget({ + deploys, identity: identity("bbbb2222", "hash1", "git"), force: false, - }); - expect(target.deployId).toBe("aaaa1111"); - expect(target.skipUpload).toBe(true); - }); + }), + ).toEqual({ deployId: "aaaa1111", skipUpload: true }); +}); - // A catalog release must keep its own ID even when the bytes happen to match the last one. - test("a custom id never aliases onto a different deploy that shares content", () => { - const target = resolveDeployTarget({ +// A catalog release must keep its own ID even when the bytes match another deploy. +test("a custom id is used exactly as given and never aliases onto another deploy", () => { + expect( + resolveDeployTarget({ deploys: [deploy("r41", "hash1")], identity: identity("r42", "hash1", "custom"), customId: "r42", force: false, - }); - expect(target.deployId).toBe("r42"); - expect(target.skipUpload).toBe(false); - expect(target.conflict).toBeUndefined(); - }); - - test("redeploying a custom id with identical content is a no-op", () => { - const target = resolveDeployTarget({ - deploys: [deploy("r42", "hash1", "custom")], - identity: identity("r42", "hash1", "custom"), - customId: "r42", - force: false, - }); - expect(target).toEqual({ deployId: "r42", skipUpload: true }); - }); + }), + ).toEqual({ deployId: "r42", skipUpload: false }); - test("reusing a custom id for different content is a conflict, not a silent overwrite", () => { - const existing = deploy("r42", "hash1", "custom"); - const target = resolveDeployTarget({ - deploys: [existing], - identity: identity("r42", "hash2", "custom"), - customId: "r42", - force: false, - }); - expect(target.conflict).toEqual({ record: existing, reason: "content" }); - expect(target.deployId).toBe("r42"); - }); - - test("a custom id is used exactly as given, case and all", () => { - const target = resolveDeployTarget({ - deploys: [], - identity: identity("Release-42", "hash1", "custom"), - customId: "Release-42", - force: false, - }); - expect(target.deployId).toBe("Release-42"); - expect(target.conflict).toBeUndefined(); - }); - - // Two deploys whose storage paths differ only by case are indistinguishable to anything that folds case. - test("an id differing from an existing deploy only in case is refused", () => { - const existing = deploy("Release-42", "hash1", "custom"); - const target = resolveDeployTarget({ - deploys: [existing], - identity: identity("release-42", "hash2", "custom"), - customId: "release-42", - force: false, - }); - expect(target.conflict).toEqual({ record: existing, reason: "case" }); - }); - - test("--force does not override a case-variant conflict", () => { - const existing = deploy("Release-42", "hash1", "custom"); - const target = resolveDeployTarget({ - deploys: [existing], - identity: identity("release-42", "hash2", "custom"), - customId: "release-42", - force: true, - }); - expect(target.conflict).toEqual({ record: existing, reason: "case" }); - }); - - test("reusing the exact existing casing is a normal redeploy, not a case conflict", () => { - const target = resolveDeployTarget({ + // Same id, same content: a no-op redeploy. + expect( + resolveDeployTarget({ deploys: [deploy("Release-42", "hash1", "custom")], identity: identity("Release-42", "hash1", "custom"), customId: "Release-42", force: false, - }); - expect(target).toEqual({ deployId: "Release-42", skipUpload: true }); - }); + }), + ).toEqual({ deployId: "Release-42", skipUpload: true }); +}); - test("--force overrides the conflict and forces a fresh upload", () => { - const target = resolveDeployTarget({ - deploys: [deploy("r42", "hash1", "custom")], - identity: identity("r42", "hash2", "custom"), - customId: "r42", - force: true, - }); - expect(target.conflict).toBeUndefined(); - expect(target.skipUpload).toBe(false); - expect(target.deployId).toBe("r42"); +test("reusing a custom id for different content conflicts instead of overwriting", () => { + const existing = deploy("r42", "hash1", "custom"); + const target = resolveDeployTarget({ + deploys: [existing], + identity: identity("r42", "hash2", "custom"), + customId: "r42", + force: false, }); + expect(target.conflict).toEqual({ record: existing, reason: "content" }); +}); - test("--force redeploys unchanged content under the same id", () => { - const target = resolveDeployTarget({ - deploys: [deploy("r42", "hash1", "custom")], - identity: identity("r42", "hash1", "custom"), - customId: "r42", - force: true, - }); - expect(target.skipUpload).toBe(false); - expect(target.deployId).toBe("r42"); +// Two deploys whose storage paths differ only by case are indistinguishable to anything that folds case, +// so this one conflict stands even under --force. +test("an id differing only in case is refused, with or without --force", () => { + const existing = deploy("Release-42", "hash1", "custom"); + const args = { + deploys: [existing], + identity: identity("release-42", "hash2", "custom"), + customId: "release-42", + }; + const conflict = { record: existing, reason: "case" } as const; + + expect(resolveDeployTarget({ ...args, force: false }).conflict).toEqual( + conflict, + ); + expect(resolveDeployTarget({ ...args, force: true }).conflict).toEqual( + conflict, + ); +}); + +test("--force clears the conflict and re-uploads under the same id", () => { + const target = resolveDeployTarget({ + deploys: [deploy("r42", "hash1", "custom")], + identity: identity("r42", "hash2", "custom"), + customId: "r42", + force: true, }); + expect(target).toEqual({ deployId: "r42", skipUpload: false }); +}); - test("a brand new custom id on an empty site just uploads", () => { - const target = resolveDeployTarget({ +test("a brand new custom id on an empty site just uploads", () => { + expect( + resolveDeployTarget({ deploys: [], identity: identity("20260827-1433-r42", "hash1", "custom"), customId: "20260827-1433-r42", force: false, - }); - expect(target).toEqual({ - deployId: "20260827-1433-r42", - skipUpload: false, - }); - }); + }), + ).toEqual({ deployId: "20260827-1433-r42", skipUpload: false }); }); From 4b15458365b2c6744274a7c598d26927cb88fc08 Mon Sep 17 00:00:00 2001 From: jamie-at-bunny Date: Fri, 28 Aug 2026 11:41:22 +0100 Subject: [PATCH 3/7] fix(sites): clear files a replaced deploy no longer includes --- .changeset/sites-custom-deploy-id.md | 2 +- packages/cli/src/commands/sites/api.ts | 2 + packages/cli/src/commands/sites/deploy.ts | 24 ++++++- .../cli/src/commands/sites/uploader.test.ts | 67 +++++++++++++++++++ packages/cli/src/commands/sites/uploader.ts | 50 ++++++++++++++ 5 files changed, 142 insertions(+), 3 deletions(-) diff --git a/.changeset/sites-custom-deploy-id.md b/.changeset/sites-custom-deploy-id.md index 3ca4446b..4dd3e25b 100644 --- a/.changeset/sites-custom-deploy-id.md +++ b/.changeset/sites-custom-deploy-id.md @@ -2,4 +2,4 @@ "@bunny.net/cli": minor --- -Add `--deploy-id` to `bunny sites deploy` so a deploy can carry your own release identifier +Add `--deploy-id` to `bunny sites deploy` so a deploy can carry your own release identifier, and clear files a replaced deploy no longer includes diff --git a/packages/cli/src/commands/sites/api.ts b/packages/cli/src/commands/sites/api.ts index 032db73c..91506880 100644 --- a/packages/cli/src/commands/sites/api.ts +++ b/packages/cli/src/commands/sites/api.ts @@ -19,6 +19,7 @@ import { connectStorageZone, deleteFile, downloadFile, + listFiles, type StorageZone, uploadFile, } from "../storage/files-api.ts"; @@ -42,6 +43,7 @@ type PullZone = components["schemas"]["PullZoneModel"]; export const siteFiles = { connect: connectStorageZone, download: downloadFile, + list: listFiles, upload: uploadFile, remove: deleteFile, }; diff --git a/packages/cli/src/commands/sites/deploy.ts b/packages/cli/src/commands/sites/deploy.ts index 07d118ba..e6d70a65 100644 --- a/packages/cli/src/commands/sites/deploy.ts +++ b/packages/cli/src/commands/sites/deploy.ts @@ -42,7 +42,12 @@ import { siteOptionBuilder, } from "./interactive.ts"; import { createLinkedSite, promptSiteName } from "./provision.ts"; -import { collectFiles, hashFiles, uploadDeploy } from "./uploader.ts"; +import { + collectFiles, + hashFiles, + pruneDeployOrphans, + uploadDeploy, +} from "./uploader.ts"; interface DeployArgs extends SiteSelectorArgs { dir?: string; @@ -337,6 +342,10 @@ export const sitesDeployCommand = defineCommand({ const { deployId, skipUpload } = target; const alreadyLive = state.current === deployId; + // Re-uploading onto an existing ID (--force, or a rebuilt artifact under the same git sha) + // leaves any file the new build dropped behind in the prefix, still reachable via the router. + const replacing = + !skipUpload && state.deploys.some((d) => d.id === deployId); // The production URL prefers the custom domain; only fetch the system host when there is none. const systemHost = state.domain @@ -383,7 +392,18 @@ export const sitesDeployCommand = defineCommand({ }), ); - // Record the deploy. A re-deployed ID keeps its slot but gets fresh metadata; the promote below purges the zone, so its old bytes can't be served. + if (replacing) { + const orphans = await withSpinner("Removing replaced files...", () => + pruneDeployOrphans(connection, deployId, files), + ); + if (orphans.length > 0 && output !== "json") { + logger.dim( + `Removed ${orphans.length} file(s) the new build no longer includes.`, + ); + } + } + + // Record the deploy. A re-deployed ID keeps its slot but gets fresh metadata; the purge on promote drops the old bytes from cache. const record: DeployRecord = { id: deployId, createdAt: new Date().toISOString(), diff --git a/packages/cli/src/commands/sites/uploader.test.ts b/packages/cli/src/commands/sites/uploader.test.ts index 8d716896..8d61f86f 100644 --- a/packages/cli/src/commands/sites/uploader.test.ts +++ b/packages/cli/src/commands/sites/uploader.test.ts @@ -7,13 +7,18 @@ import { siteFiles } from "./api.ts"; import { collectFiles, hashFiles, + pruneDeployOrphans, shouldSkipEntry, uploadDeploy, } from "./uploader.ts"; const realUpload = siteFiles.upload; +const realList = siteFiles.list; +const realRemove = siteFiles.remove; afterEach(() => { siteFiles.upload = realUpload; + siteFiles.list = realList; + siteFiles.remove = realRemove; }); const fakeConnection = {} as StorageZone; @@ -113,3 +118,65 @@ test("uploadDeploy surfaces an error after retries are exhausted", async () => { "permanent", ); }); + +// Stand in for a storage prefix: maps a listed directory to its entries. +function fakeStorage(paths: string[]) { + siteFiles.list = (async (_zone, dir: string) => { + const under = paths.filter((p) => p.startsWith(dir)); + const seen = new Map(); + for (const path of under) { + const rest = path.slice(dir.length); + const slash = rest.indexOf("/"); + seen.set(slash === -1 ? rest : rest.slice(0, slash), slash !== -1); + } + return [...seen].map(([objectName, isDirectory]) => ({ + objectName, + isDirectory, + length: 1, + })); + }) as typeof siteFiles.list; + + const removed: string[] = []; + siteFiles.remove = (async (_zone, path: string) => { + removed.push(path); + }) as typeof siteFiles.remove; + return removed; +} + +const hashed = (path: string) => + ({ path, absPath: `/tmp/${path}`, size: 1, sha256: "ab" }) as const; + +test("pruneDeployOrphans deletes only files the new build dropped", async () => { + const removed = fakeStorage([ + "deploys/r42/index.html", + "deploys/r42/old-page.html", + "deploys/r42/assets/app.js", + "deploys/r42/assets/old.css", + ]); + + const orphans = await pruneDeployOrphans(fakeConnection, "r42", [ + hashed("index.html"), + hashed("assets/app.js"), + ]); + + expect(orphans.sort()).toEqual(["assets/old.css", "old-page.html"]); + expect(removed.sort()).toEqual([ + "deploys/r42/assets/old.css", + "deploys/r42/old-page.html", + ]); +}); + +test("pruneDeployOrphans removes nothing when the build still has every file", async () => { + const removed = fakeStorage([ + "deploys/r42/index.html", + "deploys/r42/assets/app.js", + ]); + + const orphans = await pruneDeployOrphans(fakeConnection, "r42", [ + hashed("index.html"), + hashed("assets/app.js"), + ]); + + expect(orphans).toEqual([]); + expect(removed).toEqual([]); +}); diff --git a/packages/cli/src/commands/sites/uploader.ts b/packages/cli/src/commands/sites/uploader.ts index 152f633f..eca22792 100644 --- a/packages/cli/src/commands/sites/uploader.ts +++ b/packages/cli/src/commands/sites/uploader.ts @@ -120,3 +120,53 @@ export async function uploadDeploy( }, ); } + +// Every object under a deploy's prefix, as paths relative to it. +async function listDeployObjects( + connection: StorageZone, + prefix: string, + dir = "", +): Promise { + const entries = await siteFiles.list(connection, `${prefix}/${dir}`); + const paths: string[] = []; + for (const entry of entries) { + const rel = `${dir}${entry.objectName}`; + if (entry.isDirectory) { + paths.push(...(await listDeployObjects(connection, prefix, `${rel}/`))); + } else { + paths.push(rel); + } + } + return paths; +} + +/** + * Delete objects an earlier upload of the same deploy ID left behind. + * + * Re-uploading writes the new files but never removes ones the artifact has + * dropped, so without this a replaced deploy serves a mix of both. Runs after + * the new files are in place, so a live deploy is never missing a file mid-replace. + * Returns the paths removed. + */ +export async function pruneDeployOrphans( + connection: StorageZone, + deployId: string, + files: HashedLocalFile[], +): Promise { + const prefix = deployPrefix(deployId); + const keep = new Set(files.map((file) => file.path)); + const orphans = (await listDeployObjects(connection, prefix)).filter( + (path) => !keep.has(path), + ); + + await mapWithConcurrency( + orphans, + DEFAULT_UPLOAD_CONCURRENCY, + async (path) => { + await withRetries(() => + siteFiles.remove(connection, `${prefix}/${path}`), + ); + }, + ); + return orphans; +} From 067d845525cecc9ebde69815d4e9f18bab3ec6f9 Mon Sep 17 00:00:00 2001 From: Jamie Barton Date: Sun, 30 Aug 2026 11:01:18 +0000 Subject: [PATCH 4/7] fix(sites): claim a deploy in state before uploading, finalize after --- .changeset/sites-custom-deploy-id.md | 2 +- packages/cli/README.md | 8 +- packages/cli/src/commands/sites/api.test.ts | 57 +++++++++++++ packages/cli/src/commands/sites/api.ts | 18 +++- packages/cli/src/commands/sites/constants.ts | 7 ++ .../cli/src/commands/sites/deploy.test.ts | 32 ++++++++ packages/cli/src/commands/sites/deploy.ts | 82 +++++++++++++------ .../src/commands/sites/deployments/list.ts | 13 +-- .../src/commands/sites/deployments/publish.ts | 7 ++ skills/bunny-cli/references/sites.md | 3 +- 10 files changed, 192 insertions(+), 37 deletions(-) diff --git a/.changeset/sites-custom-deploy-id.md b/.changeset/sites-custom-deploy-id.md index 4dd3e25b..4dd3059f 100644 --- a/.changeset/sites-custom-deploy-id.md +++ b/.changeset/sites-custom-deploy-id.md @@ -2,4 +2,4 @@ "@bunny.net/cli": minor --- -Add `--deploy-id` to `bunny sites deploy` so a deploy can carry your own release identifier, and clear files a replaced deploy no longer includes +Add `--deploy-id` to `bunny sites deploy` so a deploy can carry your own release identifier, and clear files a replaced deploy no longer includes. Deploys now claim their ID in site state before uploading and finalize it after, so an interrupted or concurrently raced upload is marked incomplete (shown in `deployments list`, refused by `deployments publish`, finished by re-running the deploy) instead of silently serving mixed files diff --git a/packages/cli/README.md b/packages/cli/README.md index 474ad0d9..b7c75303 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -927,9 +927,10 @@ bunny sites deploy ./dist # deploy a directory and p bunny sites deploy --build # run `sites.build` from bunny.jsonc (else the detected build), then deploy bunny sites deploy --build "npm run build" --env API_URL=https://api.example.com bunny sites deploy ./dist --site my-site --force # target a site explicitly; redeploy unchanged content +bunny sites deploy ./catalog --deploy-id 20260827-1433-r42 # your own release ID instead of the git sha / content hash # Deploys: list, publish (roll back), prune -bunny sites deployments list # ● Live / ○ Previous markers, created, source, files, size +bunny sites deployments list # ● Live / ○ Previous / ⚠ Incomplete markers, created, source, files, size bunny sites deployments publish a1b2c3d4 # promote a past deploy (alias: promote) bunny sites deployments publish --previous # instant rollback bunny sites deployments prune --keep 10 # delete old deploys (default keeps 5; never live/previous) @@ -959,14 +960,15 @@ bunny sites delete my-site --keep-storage # typed-name confirmation; Preconfigure the `sites` block in `bunny.jsonc` (`name`, `build`, `dir`) and a deploy needs no arguments: `bunny sites deploy --build`. `sites ci init` reads the same block, so the generated workflow builds and deploys exactly what the local command does; without it, the framework is detected from `package.json` deps, `Gemfile`, or a `hugo`/`python`/`zola` config file, with the lockfile picking the package manager. `sites create` offers to scaffold the workflow on GitHub repos. -Every deploy publishes: the files land in an immutable `deploys//` directory and the router is pointed at it, so `deployments publish` rolls back to any earlier deploy by moving that pointer, with no files moving and nothing re-uploaded. Content is root-served, so client-side routing and absolute asset paths work as-is. Site state lives at `_bunny/site.json` inside the storage zone (the router blocks it with a 403); `.bunny/site.json` is only a local pointer, so a fresh clone can `sites link` and pick up where the last machine left off. +Every deploy publishes: the files land in an immutable `deploys//` directory and the router is pointed at it, so `deployments publish` rolls back to any earlier deploy by moving that pointer, with no files moving and nothing re-uploaded. The ID is the git short-sha when the tree is clean, a content hash otherwise, or whatever `--deploy-id` supplies (letters, digits, `-`, `_`, `.`; 4-64 chars; case-sensitive) — a custom ID never aliases onto another deploy's content, and reusing one for different content is refused unless `--force` deliberately replaces it (files the new build no longer includes are removed). A deploy interrupted mid-upload is marked `⚠ Incomplete` in `deployments list`, can't be published, and is finished by re-running the deploy. Content is root-served, so client-side routing and absolute asset paths work as-is. Site state lives at `_bunny/site.json` inside the storage zone (the router blocks it with a 403); `.bunny/site.json` is only a local pointer, so a fresh clone can `sites link` and pick up where the last machine left off. | Flag | Commands | Description | | -------------------------------------- | ---------------------------------------------------------- | -------------------------------------------------------------------------------------------------- | | `--region`, `--domain` | `create` | Main storage region code (default `DE`); custom production domain to attach | | `--site` | `deploy`, `ci init`, `deployments publish` | Site name or storage zone ID (defaults to the linked site) | | `--build [cmd]`, `--env`, `--env-file` | `deploy` | Build before deploying (bare flag uses the configured or detected build); build-time env overrides | -| `--force` | `deploy` | Deploy even when the content is unchanged | +| `--force` | `deploy` | Deploy even when the content is unchanged, or replace an existing `--deploy-id`'s content | +| `--deploy-id` | `deploy` | Identify the deploy yourself (release tag, catalog ID); case-sensitive, used exactly as given | | `--previous` | `deployments publish` | Publish the previous deploy (instant rollback) | | `--keep` | `deployments prune` | Number of recent deploys to keep (default 5; live and previous are always kept) | | `--ssl`, `--wait`, `--force-ssl` | `domains add` | Issue SSL now; wait up to 10 minutes for DNS then issue it; `--no-force-ssl` keeps HTTP working | diff --git a/packages/cli/src/commands/sites/api.test.ts b/packages/cli/src/commands/sites/api.test.ts index 10e91f71..5e552bda 100644 --- a/packages/cli/src/commands/sites/api.test.ts +++ b/packages/cli/src/commands/sites/api.test.ts @@ -370,6 +370,63 @@ test("a non-promoting write adopts the concurrent writer's current/previous", as expect(read?.state.deploys.map((d) => d.id)).toEqual(["aaa", "zzz"]); }); +test("writeRemoteState aborts a claim when a concurrent writer holds the same ID with different content", async () => { + const connection = fakeConnection(); + const etag = await writeRemoteState(connection, fakeState()); + + // Another deploy claimed r42 (still uploading) between our read and our claim write. + const theirs = { + id: "r42", + createdAt: "2026-01-02T00:00:00.000Z", + source: "custom" as const, + contentHash: "hash-theirs", + files: 1, + bytes: 10, + pending: true, + }; + store.set( + REMOTE_STATE_PATH, + JSON.stringify(fakeState({ deploys: [theirs] })), + ); + + const ours = { ...theirs, contentHash: "hash-ours" }; + await expect( + writeRemoteState(connection, fakeState({ deploys: [ours] }), etag, { + claimedId: "r42", + }), + ).rejects.toThrow("different content"); + // The abort leaves their claim untouched. + const read = await readRemoteState(connection); + expect(read?.state.deploys).toEqual([theirs]); +}); + +test("a concurrent claim of the same ID and content merges instead of aborting", async () => { + const connection = fakeConnection(); + const etag = await writeRemoteState(connection, fakeState()); + + // Same bytes racing under the same ID: both writers upload identical objects, so ours-wins is safe. + const theirs = { + id: "r42", + createdAt: "2026-01-02T00:00:00.000Z", + source: "custom" as const, + contentHash: "hash1", + files: 1, + bytes: 10, + pending: true, + }; + store.set( + REMOTE_STATE_PATH, + JSON.stringify(fakeState({ deploys: [theirs] })), + ); + + const ours = { ...theirs, createdAt: "2026-01-03T00:00:00.000Z" }; + await writeRemoteState(connection, fakeState({ deploys: [ours] }), etag, { + claimedId: "r42", + }); + const read = await readRemoteState(connection); + expect(read?.state.deploys).toEqual([ours]); +}); + test("writeRemoteState does not resurrect intentionally removed deploys on a prune/deploy race", async () => { const connection = fakeConnection(); const kept = { diff --git a/packages/cli/src/commands/sites/api.ts b/packages/cli/src/commands/sites/api.ts index 27bc4cb5..15abe369 100644 --- a/packages/cli/src/commands/sites/api.ts +++ b/packages/cli/src/commands/sites/api.ts @@ -108,7 +108,7 @@ export async function readRemoteState( return { state, etag: sha256Hex(raw) }; } -// Write `_bunny/site.json` (returns the new etag). On an `expectedEtag` mismatch a parseable concurrent state is reconciled: deploy records merge (minus any `removedIds` this writer intentionally deleted, so a prune racing a deploy doesn't resurrect pruned records), and the current/previous pointers follow `promotedTo` (last promote wins; a non-promoting writer adopts the concurrent pointers rather than clobber them with its stale read). An unparseable conflict aborts rather than overwrite. +// Write `_bunny/site.json` (returns the new etag). On an `expectedEtag` mismatch a parseable concurrent state is reconciled: deploy records merge (minus any `removedIds` this writer intentionally deleted, so a prune racing a deploy doesn't resurrect pruned records), and the current/previous pointers follow `promotedTo` (last promote wins; a non-promoting writer adopts the concurrent pointers rather than clobber them with its stale read). A concurrent record under `claimedId` with different content aborts instead of merging, and an unparseable conflict aborts rather than overwrite. export async function writeRemoteState( connection: StorageZone, state: RemoteSiteState, @@ -118,6 +118,8 @@ export async function writeRemoteState( promotedTo?: string; /** Deploy IDs this writer intentionally removed (e.g. prune); the conflict merge must not resurrect them from concurrent state. */ removedIds?: readonly string[]; + /** Deploy ID whose files this writer owns (deploy's claim/finalize/promote writes). A concurrent record under it with a different contentHash aborts: `deploys/{id}/` can only hold one artifact, so an ours-win merge would vouch for bytes another writer is scribbling over. Storage has no compare-and-swap, so this is detection, not a lock — but it shrinks the blind window from the whole upload to this read-check-write. */ + claimedId?: string; }, ): Promise { if (expectedEtag) { @@ -130,6 +132,20 @@ export async function writeRemoteState( "Another process may be writing it. Re-run the command.", ); } + if (opts?.claimedId) { + const ourClaim = state.deploys.find((d) => d.id === opts.claimedId); + const theirClaim = remote.deploys.find((d) => d.id === opts.claimedId); + if ( + ourClaim && + theirClaim && + theirClaim.contentHash !== ourClaim.contentHash + ) { + throw new UserError( + `Another deploy is writing ${opts.claimedId} with different content.`, + `Two deploys raced the same ID, so deploys/${opts.claimedId}/ may hold a mix of both. Once the other finishes, re-run this deploy with --force to make this content the deploy, or leave the other writer's.`, + ); + } + } const ours = new Set(state.deploys.map((d) => d.id)); const removed = new Set(opts?.removedIds ?? []); state.deploys = [ diff --git a/packages/cli/src/commands/sites/constants.ts b/packages/cli/src/commands/sites/constants.ts index c13f2fa6..e298ac6f 100644 --- a/packages/cli/src/commands/sites/constants.ts +++ b/packages/cli/src/commands/sites/constants.ts @@ -31,6 +31,13 @@ export interface DeployRecord { contentHash: string; files: number; bytes: number; + /** + * Set while the deploy's files are being written, cleared once they all + * landed. A record still pending was interrupted (or is being written right + * now), so its prefix cannot be trusted to hold what `contentHash` says: + * deploy re-uploads it rather than no-op'ing, and publish refuses it. + */ + pending?: boolean; } // Source of truth (at `_bunny/site.json`) for a site's resource triple and deploys; `.bunny/site.json` is just a local pointer to it. diff --git a/packages/cli/src/commands/sites/deploy.test.ts b/packages/cli/src/commands/sites/deploy.test.ts index f284829e..2f33f4f0 100644 --- a/packages/cli/src/commands/sites/deploy.test.ts +++ b/packages/cli/src/commands/sites/deploy.test.ts @@ -147,6 +147,38 @@ test("--force clears the conflict and re-uploads under the same id", () => { expect(target).toEqual({ deployId: "r42", skipUpload: false }); }); +// An interrupted upload leaves a pending record; its prefix may hold only part of these bytes. +test("a pending record never satisfies the no-op check", () => { + expect( + resolveDeployTarget({ + deploys: [{ ...deploy("r42", "hash1", "custom"), pending: true }], + identity: identity("r42", "hash1", "custom"), + customId: "r42", + force: false, + }), + ).toEqual({ deployId: "r42", skipUpload: false }); + + // Content-addressed deploys re-upload rather than alias onto a half-written prefix. + expect( + resolveDeployTarget({ + deploys: [{ ...deploy("aaaa1111", "hash1"), pending: true }], + identity: identity("bbbb2222", "hash1"), + force: false, + }), + ).toEqual({ deployId: "bbbb2222", skipUpload: false }); +}); + +test("a pending record with different content still conflicts under its custom id", () => { + const stuck = { ...deploy("r42", "hash1", "custom"), pending: true }; + const target = resolveDeployTarget({ + deploys: [stuck], + identity: identity("r42", "hash2", "custom"), + customId: "r42", + force: false, + }); + expect(target.conflict).toEqual({ record: stuck, reason: "content" }); +}); + test("a brand new custom id on an empty site just uploads", () => { expect( resolveDeployTarget({ diff --git a/packages/cli/src/commands/sites/deploy.ts b/packages/cli/src/commands/sites/deploy.ts index e6d70a65..864cd6fc 100644 --- a/packages/cli/src/commands/sites/deploy.ts +++ b/packages/cli/src/commands/sites/deploy.ts @@ -94,10 +94,13 @@ export function resolveDeployTarget(opts: { const alreadyUploaded = force ? undefined - : deploys.find((d) => - customId - ? d.id === customId && d.contentHash === identity.contentHash - : d.contentHash === identity.contentHash, + : deploys.find( + (d) => + // A pending record marks an interrupted (or in-flight) write; its prefix can't be trusted to hold these bytes, so re-upload instead of skipping. + !d.pending && + (customId + ? d.id === customId && d.contentHash === identity.contentHash + : d.contentHash === identity.contentHash), ); // A skipped deploy reuses the already-uploaded deploy's id; that's where its files live. const deployId = alreadyUploaded?.id ?? identity.id; @@ -193,7 +196,8 @@ export const sitesDeployCommand = defineCommand({ .option("force", { type: "boolean", default: false, - describe: "Deploy even when the content is unchanged", + describe: + "Deploy even when the content is unchanged, or replace an existing --deploy-id's content", }) .option("deploy-id", { type: "string", @@ -334,6 +338,12 @@ export const sitesDeployCommand = defineCommand({ ); } if (target.conflict?.reason === "content") { + if (target.conflict.record.pending) { + throw new UserError( + `An earlier deploy of ${customId} to ${state.name} never finished, and this content differs from what it was uploading.`, + "Its files can't be trusted either way. Pass --force to replace it with this content, or delete it with `bunny sites deployments delete`.", + ); + } throw new UserError( `Deploy ${customId} already exists for ${state.name} with different content.`, "Rolling back to that ID would serve these new files instead of the originals. Pick another ID, or pass --force to replace it.", @@ -384,26 +394,7 @@ export const sitesDeployCommand = defineCommand({ } if (!skipUpload) { - await withSpinner(`Uploading ${files.length} files...`, (spin) => - uploadDeploy(connection, deployId, files, { - onFileUploaded: (done, total) => { - spin.text = `Uploading ${done}/${total} files (${formatBytes(totalBytes)} total)...`; - }, - }), - ); - - if (replacing) { - const orphans = await withSpinner("Removing replaced files...", () => - pruneDeployOrphans(connection, deployId, files), - ); - if (orphans.length > 0 && output !== "json") { - logger.dim( - `Removed ${orphans.length} file(s) the new build no longer includes.`, - ); - } - } - - // Record the deploy. A re-deployed ID keeps its slot but gets fresh metadata; the purge on promote drops the old bytes from cache. + // The deploy record. A re-deployed ID keeps its slot but gets fresh metadata; the purge on promote drops the old bytes from cache. const record: DeployRecord = { id: deployId, createdAt: new Date().toISOString(), @@ -414,11 +405,49 @@ export const sitesDeployCommand = defineCommand({ files: files.length, bytes: totalBytes, }; + + // Claim the ID in state before touching any object, and finalize only once every file landed: an interrupted or raced write leaves a record that says the prefix can't be trusted, never one vouching for bytes that aren't all there. The claim write also surfaces a concurrent deploy of the same ID (via `claimedId`) before this one starts scribbling over its files. + state.deploys = [ + { ...record, pending: true }, + ...state.deploys.filter((d) => d.id !== deployId), + ]; + etag = await writeRemoteState(connection, state, etag, { + claimedId: deployId, + }); + + try { + await withSpinner(`Uploading ${files.length} files...`, (spin) => + uploadDeploy(connection, deployId, files, { + onFileUploaded: (done, total) => { + spin.text = `Uploading ${done}/${total} files (${formatBytes(totalBytes)} total)...`; + }, + }), + ); + + if (replacing) { + const orphans = await withSpinner("Removing replaced files...", () => + pruneDeployOrphans(connection, deployId, files), + ); + if (orphans.length > 0 && output !== "json") { + logger.dim( + `Removed ${orphans.length} file(s) the new build no longer includes.`, + ); + } + } + } catch (err) { + logger.warn( + `Deploy ${deployId} is marked incomplete; re-run the deploy to finish it.`, + ); + throw err; + } + state.deploys = [ record, ...state.deploys.filter((d) => d.id !== deployId), ]; - etag = await writeRemoteState(connection, state, etag); + etag = await writeRemoteState(connection, state, etag, { + claimedId: deployId, + }); } await withSpinner("Publishing to production...", async () => { @@ -431,6 +460,7 @@ export const sitesDeployCommand = defineCommand({ markCurrent(state, deployId); etag = await writeRemoteState(connection, state, etag, { promotedTo: deployId, + claimedId: deployId, }); }); diff --git a/packages/cli/src/commands/sites/deployments/list.ts b/packages/cli/src/commands/sites/deployments/list.ts index c5b7860f..f3e506fd 100644 --- a/packages/cli/src/commands/sites/deployments/list.ts +++ b/packages/cli/src/commands/sites/deployments/list.ts @@ -80,11 +80,14 @@ export const sitesDeploymentsListCommand = defineCommand({ ["ID", "Status", "Created", "Source", "Files", "Size"], state.deploys.map((d) => [ d.id, - d.id === state.current - ? "● Live" - : d.id === state.previous - ? "○ Previous" - : "○", + // Incomplete trumps the pointer markers: an interrupted upload is the actionable state, whatever the pointers say. + d.pending + ? "⚠ Incomplete" + : d.id === state.current + ? "● Live" + : d.id === state.previous + ? "○ Previous" + : "○", formatDateTime(d.createdAt), deploySource(d), String(d.files), diff --git a/packages/cli/src/commands/sites/deployments/publish.ts b/packages/cli/src/commands/sites/deployments/publish.ts index fe5f874f..f7ef03a5 100644 --- a/packages/cli/src/commands/sites/deployments/publish.ts +++ b/packages/cli/src/commands/sites/deployments/publish.ts @@ -97,6 +97,13 @@ export const sitesDeploymentsPublishCommand = defineCommand({ : "Run `bunny sites deployments list` to see available deploys.", ); } + // A pending record's upload never finished, so its files may be missing or mixed with an earlier deploy's. + if (deploy.pending) { + throw new UserError( + `Deploy ${targetId} never finished uploading and can't be published.`, + "Re-run `bunny sites deploy` for that content to complete it, or remove it with `bunny sites deployments delete`.", + ); + } if (state.current === targetId) { if (output === "json") { diff --git a/skills/bunny-cli/references/sites.md b/skills/bunny-cli/references/sites.md index 7cf48fcd..fe8176d0 100644 --- a/skills/bunny-cli/references/sites.md +++ b/skills/bunny-cli/references/sites.md @@ -47,7 +47,8 @@ Content is root-served, so client-side routers (TanStack Router, React Router, V - `--deploy-id ` sets the ID yourself, so a deploy can carry the same identifier as whatever produced it (a release tag, a catalog build, a timestamped artifact) and `deployments list` needs no cross-referencing. The ID is used **exactly as given**, case included: it exists to match your identifier, and it never appears in a client-facing URL (the router builds the origin path from it server-side). IDs become storage paths, so they take letters, digits and `-`, `_` or `.`, 4 to 64 characters, starting and ending alphanumeric: `20260827-1433-r42`, `Catalog_V3`, `v1.2.3`. - Deploy IDs are therefore **case-sensitive**. `publish`/`delete` match exactly and suggest a case variant when one exists, and deploying an ID that differs from an existing one only in case is refused (not even with `--force`), since two storage paths differing only by case are indistinguishable to anything that folds case. - An explicit ID is an assertion about identity, so it is never aliased onto an earlier deploy that happens to share content: each release keeps its own ID and rollback target even when the bytes are unchanged. - - Reusing an ID for **different** content is refused, because rolling back to that ID would then serve the new files instead of the originals. Pass `--force` to replace it deliberately. + - Reusing an ID for **different** content is refused, because rolling back to that ID would then serve the new files instead of the originals. Pass `--force` to replace it deliberately; the replacement also removes files the new content no longer includes. + - A deploy claims its ID in site state before uploading and finalizes it after, so an upload that is interrupted (or raced by a concurrent deploy of the same ID) leaves the record marked **incomplete** (`⚠ Incomplete` in `deployments list`) instead of one vouching for half-written files. An incomplete deploy can't be published; re-run the deploy (with `--force` if its content differs) to finish it, or `deployments delete` it. Two deploys writing the same ID with different content abort with an error when they detect each other, rather than silently mixing files. - The git sha is still recorded alongside a custom ID when the deploy came from a repo, so provenance is not lost; `deployments list` shows it as `custom (git abc12345)`. - Dotfiles and `node_modules` are never uploaded. From cc96282eb492b7e7355e4ecd208b6afdd155af56 Mon Sep 17 00:00:00 2001 From: Jamie Barton Date: Sun, 30 Aug 2026 11:06:41 +0000 Subject: [PATCH 5/7] fix(sites): never replace the live or rollback deploy's content in place --- packages/cli/README.md | 2 +- .../cli/src/commands/sites/deploy.test.ts | 45 ++++++++++++++ packages/cli/src/commands/sites/deploy.ts | 60 ++++++++++++++++--- skills/bunny-cli/references/sites.md | 2 +- 4 files changed, 98 insertions(+), 11 deletions(-) diff --git a/packages/cli/README.md b/packages/cli/README.md index b7c75303..7e3c87d4 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -960,7 +960,7 @@ bunny sites delete my-site --keep-storage # typed-name confirmation; Preconfigure the `sites` block in `bunny.jsonc` (`name`, `build`, `dir`) and a deploy needs no arguments: `bunny sites deploy --build`. `sites ci init` reads the same block, so the generated workflow builds and deploys exactly what the local command does; without it, the framework is detected from `package.json` deps, `Gemfile`, or a `hugo`/`python`/`zola` config file, with the lockfile picking the package manager. `sites create` offers to scaffold the workflow on GitHub repos. -Every deploy publishes: the files land in an immutable `deploys//` directory and the router is pointed at it, so `deployments publish` rolls back to any earlier deploy by moving that pointer, with no files moving and nothing re-uploaded. The ID is the git short-sha when the tree is clean, a content hash otherwise, or whatever `--deploy-id` supplies (letters, digits, `-`, `_`, `.`; 4-64 chars; case-sensitive) — a custom ID never aliases onto another deploy's content, and reusing one for different content is refused unless `--force` deliberately replaces it (files the new build no longer includes are removed). A deploy interrupted mid-upload is marked `⚠ Incomplete` in `deployments list`, can't be published, and is finished by re-running the deploy. Content is root-served, so client-side routing and absolute asset paths work as-is. Site state lives at `_bunny/site.json` inside the storage zone (the router blocks it with a 403); `.bunny/site.json` is only a local pointer, so a fresh clone can `sites link` and pick up where the last machine left off. +Every deploy publishes: the files land in an immutable `deploys//` directory and the router is pointed at it, so `deployments publish` rolls back to any earlier deploy by moving that pointer, with no files moving and nothing re-uploaded. The ID is the git short-sha when the tree is clean, a content hash otherwise, or whatever `--deploy-id` supplies (letters, digits, `-`, `_`, `.`; 4-64 chars; case-sensitive) — a custom ID never aliases onto another deploy's content, and reusing one for different content is refused unless `--force` deliberately replaces it (files the new build no longer includes are removed); the live deploy and the rollback target are never replaced in place, so deploy those under a new ID. A deploy interrupted mid-upload is marked `⚠ Incomplete` in `deployments list`, can't be published, and is finished by re-running the deploy. Content is root-served, so client-side routing and absolute asset paths work as-is. Site state lives at `_bunny/site.json` inside the storage zone (the router blocks it with a 403); `.bunny/site.json` is only a local pointer, so a fresh clone can `sites link` and pick up where the last machine left off. | Flag | Commands | Description | | -------------------------------------- | ---------------------------------------------------------- | -------------------------------------------------------------------------------------------------- | diff --git a/packages/cli/src/commands/sites/deploy.test.ts b/packages/cli/src/commands/sites/deploy.test.ts index 2f33f4f0..4476539f 100644 --- a/packages/cli/src/commands/sites/deploy.test.ts +++ b/packages/cli/src/commands/sites/deploy.test.ts @@ -179,6 +179,51 @@ test("a pending record with different content still conflicts under its custom i expect(target.conflict).toEqual({ record: stuck, reason: "content" }); }); +// Replacing the deploy production serves (or the rollback target) rewrites its prefix while the router reads it, so it is never forceable. +test("replacing the live or rollback deploy's content is refused, even with --force", () => { + const live = deploy("r42", "hash1", "custom"); + for (const pointers of [{ current: "r42" }, { previous: "r42" }]) { + for (const force of [false, true]) { + const target = resolveDeployTarget({ + deploys: [live], + identity: identity("r42", "hash2", "custom"), + customId: "r42", + force, + ...pointers, + }); + expect(target.conflict).toEqual({ + record: live, + reason: "current" in pointers ? "live" : "rollback", + }); + } + } +}); + +// Same git sha over different bytes lands on the same ID without a custom flag; the live guard must cover it too. +test("a non-custom deploy replacing the live deploy's content is refused", () => { + const live = deploy("aaaa1111", "hash1", "git"); + const target = resolveDeployTarget({ + deploys: [live], + identity: identity("aaaa1111", "hash2", "git"), + force: false, + current: "aaaa1111", + }); + expect(target.conflict).toEqual({ record: live, reason: "live" }); +}); + +// --force's "redeploy unchanged content" path: every write is byte-identical, so in-place is safe. +test("a forced same-content redeploy of the live deploy is allowed", () => { + expect( + resolveDeployTarget({ + deploys: [deploy("r42", "hash1", "custom")], + identity: identity("r42", "hash1", "custom"), + customId: "r42", + force: true, + current: "r42", + }), + ).toEqual({ deployId: "r42", skipUpload: false }); +}); + test("a brand new custom id on an empty site just uploads", () => { expect( resolveDeployTarget({ diff --git a/packages/cli/src/commands/sites/deploy.ts b/packages/cli/src/commands/sites/deploy.ts index 864cd6fc..f8ffc63e 100644 --- a/packages/cli/src/commands/sites/deploy.ts +++ b/packages/cli/src/commands/sites/deploy.ts @@ -70,8 +70,15 @@ export interface DeployTarget { * `case`: an ID differing only in case exists. Not forceable, because two * deploys whose paths differ only by case are indistinguishable to anything * that folds case, and the loser's files would back the winner's rollback. + * `live`/`rollback`: the ID holds different bytes AND is the production + * deploy or the rollback target. Not forceable, because replacing it means + * rewriting the very prefix the router serves (or would roll back to) + * file-by-file, and a failure mid-replace strands it on a mix of both. */ - conflict?: { record: DeployRecord; reason: "content" | "case" }; + conflict?: { + record: DeployRecord; + reason: "content" | "case" | "live" | "rollback"; + }; } /** @@ -89,8 +96,11 @@ export function resolveDeployTarget(opts: { identity: DeployIdentity; customId?: string; force: boolean; + /** The production deploy and the rollback target; their content is never replaced in place. */ + current?: string; + previous?: string; }): DeployTarget { - const { deploys, identity, customId, force } = opts; + const { deploys, identity, customId, force, current, previous } = opts; const alreadyUploaded = force ? undefined @@ -107,7 +117,7 @@ export function resolveDeployTarget(opts: { const skipUpload = alreadyUploaded !== undefined; if (customId && !skipUpload) { - const { deploy: exact, caseVariant } = findDeploy(deploys, customId); + const { caseVariant } = findDeploy(deploys, customId); if (caseVariant) { return { deployId, @@ -115,12 +125,29 @@ export function resolveDeployTarget(opts: { conflict: { record: caseVariant, reason: "case" }, }; } - if (exact && exact.contentHash !== identity.contentHash && !force) { - return { - deployId, - skipUpload, - conflict: { record: exact, reason: "content" }, - }; + } + + if (!skipUpload) { + const existing = deploys.find((d) => d.id === deployId); + if (existing && existing.contentHash !== identity.contentHash) { + // Replacing the deploy production serves (or would roll back to) rewrites its prefix while the router reads it, so it is refused outright — before the forceable content conflict, which would otherwise send the caller down a --force dead end. A same-bytes re-upload stays fine: every write is byte-identical. + if (deployId === current || deployId === previous) { + return { + deployId, + skipUpload, + conflict: { + record: existing, + reason: deployId === current ? "live" : "rollback", + }, + }; + } + if (customId && !force) { + return { + deployId, + skipUpload, + conflict: { record: existing, reason: "content" }, + }; + } } } return { deployId, skipUpload }; @@ -329,8 +356,23 @@ export const sitesDeployCommand = defineCommand({ identity, customId, force: args.force ?? false, + current: state.current, + previous: state.previous, }); + if ( + target.conflict?.reason === "live" || + target.conflict?.reason === "rollback" + ) { + const role = + target.conflict.reason === "live" + ? "the live production deploy" + : "the rollback target"; + throw new UserError( + `Deploy ${target.deployId} is ${role} for ${state.name}, and this content differs from what it holds.`, + "Replacing it in place would rewrite files while the router serves them. Deploy under a new --deploy-id, or publish another deploy first and re-run.", + ); + } if (target.conflict?.reason === "case") { throw new UserError( `Deploy ${target.conflict.record.id} already exists for ${state.name}, differing from "${customId}" only in case.`, diff --git a/skills/bunny-cli/references/sites.md b/skills/bunny-cli/references/sites.md index fe8176d0..d841cc6d 100644 --- a/skills/bunny-cli/references/sites.md +++ b/skills/bunny-cli/references/sites.md @@ -47,7 +47,7 @@ Content is root-served, so client-side routers (TanStack Router, React Router, V - `--deploy-id ` sets the ID yourself, so a deploy can carry the same identifier as whatever produced it (a release tag, a catalog build, a timestamped artifact) and `deployments list` needs no cross-referencing. The ID is used **exactly as given**, case included: it exists to match your identifier, and it never appears in a client-facing URL (the router builds the origin path from it server-side). IDs become storage paths, so they take letters, digits and `-`, `_` or `.`, 4 to 64 characters, starting and ending alphanumeric: `20260827-1433-r42`, `Catalog_V3`, `v1.2.3`. - Deploy IDs are therefore **case-sensitive**. `publish`/`delete` match exactly and suggest a case variant when one exists, and deploying an ID that differs from an existing one only in case is refused (not even with `--force`), since two storage paths differing only by case are indistinguishable to anything that folds case. - An explicit ID is an assertion about identity, so it is never aliased onto an earlier deploy that happens to share content: each release keeps its own ID and rollback target even when the bytes are unchanged. - - Reusing an ID for **different** content is refused, because rolling back to that ID would then serve the new files instead of the originals. Pass `--force` to replace it deliberately; the replacement also removes files the new content no longer includes. + - Reusing an ID for **different** content is refused, because rolling back to that ID would then serve the new files instead of the originals. Pass `--force` to replace it deliberately; the replacement also removes files the new content no longer includes. The **live deploy and the rollback target are never replaceable in place** (not even with `--force`): that would rewrite the files the router is serving. Deploy under a new ID, or publish another deploy first. - A deploy claims its ID in site state before uploading and finalizes it after, so an upload that is interrupted (or raced by a concurrent deploy of the same ID) leaves the record marked **incomplete** (`⚠ Incomplete` in `deployments list`) instead of one vouching for half-written files. An incomplete deploy can't be published; re-run the deploy (with `--force` if its content differs) to finish it, or `deployments delete` it. Two deploys writing the same ID with different content abort with an error when they detect each other, rather than silently mixing files. - The git sha is still recorded alongside a custom ID when the deploy came from a repo, so provenance is not lost; `deployments list` shows it as `custom (git abc12345)`. - Dotfiles and `node_modules` are never uploaded. From 02da8e9f45e468ceaaa7bb1d61eaef9d9ddd8747 Mon Sep 17 00:00:00 2001 From: Jamie Barton Date: Sun, 30 Aug 2026 11:26:40 +0000 Subject: [PATCH 6/7] fix(sites): revalidate fresh state before publishing a deploy --- .../cli/src/commands/sites/deploy.test.ts | 72 +++++++------------ packages/cli/src/commands/sites/deploy.ts | 6 -- .../src/commands/sites/deployments/publish.ts | 27 +++++-- 3 files changed, 47 insertions(+), 58 deletions(-) diff --git a/packages/cli/src/commands/sites/deploy.test.ts b/packages/cli/src/commands/sites/deploy.test.ts index 4476539f..525192a0 100644 --- a/packages/cli/src/commands/sites/deploy.test.ts +++ b/packages/cli/src/commands/sites/deploy.test.ts @@ -147,7 +147,7 @@ test("--force clears the conflict and re-uploads under the same id", () => { expect(target).toEqual({ deployId: "r42", skipUpload: false }); }); -// An interrupted upload leaves a pending record; its prefix may hold only part of these bytes. +// An interrupted upload leaves a pending record; its prefix may hold only part of these bytes, so re-upload rather than no-op onto it. test("a pending record never satisfies the no-op check", () => { expect( resolveDeployTarget({ @@ -157,58 +157,36 @@ test("a pending record never satisfies the no-op check", () => { force: false, }), ).toEqual({ deployId: "r42", skipUpload: false }); - - // Content-addressed deploys re-upload rather than alias onto a half-written prefix. - expect( - resolveDeployTarget({ - deploys: [{ ...deploy("aaaa1111", "hash1"), pending: true }], - identity: identity("bbbb2222", "hash1"), - force: false, - }), - ).toEqual({ deployId: "bbbb2222", skipUpload: false }); -}); - -test("a pending record with different content still conflicts under its custom id", () => { - const stuck = { ...deploy("r42", "hash1", "custom"), pending: true }; - const target = resolveDeployTarget({ - deploys: [stuck], - identity: identity("r42", "hash2", "custom"), - customId: "r42", - force: false, - }); - expect(target.conflict).toEqual({ record: stuck, reason: "content" }); }); -// Replacing the deploy production serves (or the rollback target) rewrites its prefix while the router reads it, so it is never forceable. +// Replacing the deploy production serves (or the rollback target) rewrites its prefix while the router reads it, so it is never forceable — custom ID or not. test("replacing the live or rollback deploy's content is refused, even with --force", () => { const live = deploy("r42", "hash1", "custom"); - for (const pointers of [{ current: "r42" }, { previous: "r42" }]) { - for (const force of [false, true]) { - const target = resolveDeployTarget({ - deploys: [live], - identity: identity("r42", "hash2", "custom"), - customId: "r42", - force, - ...pointers, - }); - expect(target.conflict).toEqual({ - record: live, - reason: "current" in pointers ? "live" : "rollback", - }); - } - } -}); - -// Same git sha over different bytes lands on the same ID without a custom flag; the live guard must cover it too. -test("a non-custom deploy replacing the live deploy's content is refused", () => { - const live = deploy("aaaa1111", "hash1", "git"); - const target = resolveDeployTarget({ + const args = { deploys: [live], - identity: identity("aaaa1111", "hash2", "git"), - force: false, - current: "aaaa1111", + identity: identity("r42", "hash2", "custom"), + customId: "r42", + force: true, + }; + expect(resolveDeployTarget({ ...args, current: "r42" }).conflict).toEqual({ + record: live, + reason: "live", + }); + expect(resolveDeployTarget({ ...args, previous: "r42" }).conflict).toEqual({ + record: live, + reason: "rollback", }); - expect(target.conflict).toEqual({ record: live, reason: "live" }); + + // Same git sha over different bytes lands on the same ID without --deploy-id. + const gitLive = deploy("aaaa1111", "hash1", "git"); + expect( + resolveDeployTarget({ + deploys: [gitLive], + identity: identity("aaaa1111", "hash2", "git"), + force: false, + current: "aaaa1111", + }).conflict, + ).toEqual({ record: gitLive, reason: "live" }); }); // --force's "redeploy unchanged content" path: every write is byte-identical, so in-place is safe. diff --git a/packages/cli/src/commands/sites/deploy.ts b/packages/cli/src/commands/sites/deploy.ts index f8ffc63e..8585e95b 100644 --- a/packages/cli/src/commands/sites/deploy.ts +++ b/packages/cli/src/commands/sites/deploy.ts @@ -380,12 +380,6 @@ export const sitesDeployCommand = defineCommand({ ); } if (target.conflict?.reason === "content") { - if (target.conflict.record.pending) { - throw new UserError( - `An earlier deploy of ${customId} to ${state.name} never finished, and this content differs from what it was uploading.`, - "Its files can't be trusted either way. Pass --force to replace it with this content, or delete it with `bunny sites deployments delete`.", - ); - } throw new UserError( `Deploy ${customId} already exists for ${state.name} with different content.`, "Rolling back to that ID would serve these new files instead of the originals. Pick another ID, or pass --force to replace it.", diff --git a/packages/cli/src/commands/sites/deployments/publish.ts b/packages/cli/src/commands/sites/deployments/publish.ts index f7ef03a5..2c699187 100644 --- a/packages/cli/src/commands/sites/deployments/publish.ts +++ b/packages/cli/src/commands/sites/deployments/publish.ts @@ -8,7 +8,7 @@ import { defineCommand } from "../../../core/define-command.ts"; import { UserError } from "../../../core/errors.ts"; import { logger } from "../../../core/logger.ts"; import { confirm, requireConfirmable, withSpinner } from "../../../core/ui.ts"; -import { promoteDeploy, writeRemoteState } from "../api.ts"; +import { promoteDeploy, readRemoteState, writeRemoteState } from "../api.ts"; import { findDeploy, markCurrent } from "../constants.ts"; import { type SiteSelectorArgs, @@ -66,7 +66,8 @@ export const sitesDeploymentsPublishCommand = defineCommand({ output, force: args.force, }); - const { state, connection, etag } = site; + // No etag kept from this read: the destructive phase re-reads state and writes with the fresh one. + const { state, connection } = site; let targetId = args.id; if (args.previous) { @@ -140,14 +141,30 @@ export const sitesDeploymentsPublishCommand = defineCommand({ } await withSpinner("Publishing...", async () => { + // Revalidate on fresh state right before promoting: the confirmation window is long enough for a concurrent deploy to have claimed this ID and started rewriting its files. + const fresh = await readRemoteState(connection); + if (!fresh) { + throw new UserError( + "Couldn't re-read the site state.", + "Retry the publish; nothing was changed.", + ); + } + const { state: latest, etag: latestEtag } = fresh; + const record = latest.deploys.find((d) => d.id === targetId); + if (!record || record.pending) { + throw new UserError( + `Deploy ${targetId} ${record ? "is being rewritten by a concurrent deploy" : `is gone from ${latest.name}`} and can't be published.`, + "Run `bunny sites deployments list` and retry.", + ); + } await promoteDeploy({ computeClient, coreClient, - state, + state: latest, deployId: targetId, }); - markCurrent(state, targetId); - await writeRemoteState(connection, state, etag, { + markCurrent(latest, targetId); + await writeRemoteState(connection, latest, latestEtag, { promotedTo: targetId, }); }); From c6acd4be113cae09d11491f968f56f5f9f65415c Mon Sep 17 00:00:00 2001 From: Jamie Barton Date: Sun, 30 Aug 2026 11:44:32 +0000 Subject: [PATCH 7/7] refactor(sites): confirm and clear-first when replacing a deploy, drop the claim machinery --- .changeset/sites-custom-deploy-id.md | 2 +- packages/cli/README.md | 6 +- packages/cli/src/commands/sites/api.test.ts | 57 -------- packages/cli/src/commands/sites/api.ts | 25 +--- packages/cli/src/commands/sites/constants.ts | 7 - .../cli/src/commands/sites/deploy.test.ts | 39 ++---- packages/cli/src/commands/sites/deploy.ts | 128 ++++++++---------- .../src/commands/sites/deployments/list.ts | 13 +- .../src/commands/sites/deployments/publish.ts | 15 +- .../cli/src/commands/sites/uploader.test.ts | 67 --------- packages/cli/src/commands/sites/uploader.ts | 50 ------- skills/bunny-cli/references/sites.md | 3 +- 12 files changed, 88 insertions(+), 324 deletions(-) diff --git a/.changeset/sites-custom-deploy-id.md b/.changeset/sites-custom-deploy-id.md index 4dd3059f..ee2e7089 100644 --- a/.changeset/sites-custom-deploy-id.md +++ b/.changeset/sites-custom-deploy-id.md @@ -2,4 +2,4 @@ "@bunny.net/cli": minor --- -Add `--deploy-id` to `bunny sites deploy` so a deploy can carry your own release identifier, and clear files a replaced deploy no longer includes. Deploys now claim their ID in site state before uploading and finalize it after, so an interrupted or concurrently raced upload is marked incomplete (shown in `deployments list`, refused by `deployments publish`, finished by re-running the deploy) instead of silently serving mixed files +Add `--deploy-id` to `bunny sites deploy` so a deploy can carry your own release identifier. Reusing an ID for different content asks before replacing (`--force` skips the prompt) and clears the old files first; the live deploy and the rollback target are never replaced in place diff --git a/packages/cli/README.md b/packages/cli/README.md index 7e3c87d4..302daeab 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -930,7 +930,7 @@ bunny sites deploy ./dist --site my-site --force # target a site explicitly bunny sites deploy ./catalog --deploy-id 20260827-1433-r42 # your own release ID instead of the git sha / content hash # Deploys: list, publish (roll back), prune -bunny sites deployments list # ● Live / ○ Previous / ⚠ Incomplete markers, created, source, files, size +bunny sites deployments list # ● Live / ○ Previous markers, created, source, files, size bunny sites deployments publish a1b2c3d4 # promote a past deploy (alias: promote) bunny sites deployments publish --previous # instant rollback bunny sites deployments prune --keep 10 # delete old deploys (default keeps 5; never live/previous) @@ -960,14 +960,14 @@ bunny sites delete my-site --keep-storage # typed-name confirmation; Preconfigure the `sites` block in `bunny.jsonc` (`name`, `build`, `dir`) and a deploy needs no arguments: `bunny sites deploy --build`. `sites ci init` reads the same block, so the generated workflow builds and deploys exactly what the local command does; without it, the framework is detected from `package.json` deps, `Gemfile`, or a `hugo`/`python`/`zola` config file, with the lockfile picking the package manager. `sites create` offers to scaffold the workflow on GitHub repos. -Every deploy publishes: the files land in an immutable `deploys//` directory and the router is pointed at it, so `deployments publish` rolls back to any earlier deploy by moving that pointer, with no files moving and nothing re-uploaded. The ID is the git short-sha when the tree is clean, a content hash otherwise, or whatever `--deploy-id` supplies (letters, digits, `-`, `_`, `.`; 4-64 chars; case-sensitive) — a custom ID never aliases onto another deploy's content, and reusing one for different content is refused unless `--force` deliberately replaces it (files the new build no longer includes are removed); the live deploy and the rollback target are never replaced in place, so deploy those under a new ID. A deploy interrupted mid-upload is marked `⚠ Incomplete` in `deployments list`, can't be published, and is finished by re-running the deploy. Content is root-served, so client-side routing and absolute asset paths work as-is. Site state lives at `_bunny/site.json` inside the storage zone (the router blocks it with a 403); `.bunny/site.json` is only a local pointer, so a fresh clone can `sites link` and pick up where the last machine left off. +Every deploy publishes: the files land in an immutable `deploys//` directory and the router is pointed at it, so `deployments publish` rolls back to any earlier deploy by moving that pointer, with no files moving and nothing re-uploaded. The ID is the git short-sha when the tree is clean, a content hash otherwise, or whatever `--deploy-id` supplies (letters, digits, `-`, `_`, `.`; 4-64 chars; case-sensitive) — a custom ID never aliases onto another deploy's content, and reusing one for different content asks before replacing (`--force` skips the prompt); a replacement clears the old files first, so nothing stale survives. The live deploy and the rollback target are never replaced in place — deploy those under a new ID. Content is root-served, so client-side routing and absolute asset paths work as-is. Site state lives at `_bunny/site.json` inside the storage zone (the router blocks it with a 403); `.bunny/site.json` is only a local pointer, so a fresh clone can `sites link` and pick up where the last machine left off. | Flag | Commands | Description | | -------------------------------------- | ---------------------------------------------------------- | -------------------------------------------------------------------------------------------------- | | `--region`, `--domain` | `create` | Main storage region code (default `DE`); custom production domain to attach | | `--site` | `deploy`, `ci init`, `deployments publish` | Site name or storage zone ID (defaults to the linked site) | | `--build [cmd]`, `--env`, `--env-file` | `deploy` | Build before deploying (bare flag uses the configured or detected build); build-time env overrides | -| `--force` | `deploy` | Deploy even when the content is unchanged, or replace an existing `--deploy-id`'s content | +| `--force` | `deploy` | Deploy even when the content is unchanged, and replace an existing `--deploy-id` without asking | | `--deploy-id` | `deploy` | Identify the deploy yourself (release tag, catalog ID); case-sensitive, used exactly as given | | `--previous` | `deployments publish` | Publish the previous deploy (instant rollback) | | `--keep` | `deployments prune` | Number of recent deploys to keep (default 5; live and previous are always kept) | diff --git a/packages/cli/src/commands/sites/api.test.ts b/packages/cli/src/commands/sites/api.test.ts index 5e552bda..10e91f71 100644 --- a/packages/cli/src/commands/sites/api.test.ts +++ b/packages/cli/src/commands/sites/api.test.ts @@ -370,63 +370,6 @@ test("a non-promoting write adopts the concurrent writer's current/previous", as expect(read?.state.deploys.map((d) => d.id)).toEqual(["aaa", "zzz"]); }); -test("writeRemoteState aborts a claim when a concurrent writer holds the same ID with different content", async () => { - const connection = fakeConnection(); - const etag = await writeRemoteState(connection, fakeState()); - - // Another deploy claimed r42 (still uploading) between our read and our claim write. - const theirs = { - id: "r42", - createdAt: "2026-01-02T00:00:00.000Z", - source: "custom" as const, - contentHash: "hash-theirs", - files: 1, - bytes: 10, - pending: true, - }; - store.set( - REMOTE_STATE_PATH, - JSON.stringify(fakeState({ deploys: [theirs] })), - ); - - const ours = { ...theirs, contentHash: "hash-ours" }; - await expect( - writeRemoteState(connection, fakeState({ deploys: [ours] }), etag, { - claimedId: "r42", - }), - ).rejects.toThrow("different content"); - // The abort leaves their claim untouched. - const read = await readRemoteState(connection); - expect(read?.state.deploys).toEqual([theirs]); -}); - -test("a concurrent claim of the same ID and content merges instead of aborting", async () => { - const connection = fakeConnection(); - const etag = await writeRemoteState(connection, fakeState()); - - // Same bytes racing under the same ID: both writers upload identical objects, so ours-wins is safe. - const theirs = { - id: "r42", - createdAt: "2026-01-02T00:00:00.000Z", - source: "custom" as const, - contentHash: "hash1", - files: 1, - bytes: 10, - pending: true, - }; - store.set( - REMOTE_STATE_PATH, - JSON.stringify(fakeState({ deploys: [theirs] })), - ); - - const ours = { ...theirs, createdAt: "2026-01-03T00:00:00.000Z" }; - await writeRemoteState(connection, fakeState({ deploys: [ours] }), etag, { - claimedId: "r42", - }); - const read = await readRemoteState(connection); - expect(read?.state.deploys).toEqual([ours]); -}); - test("writeRemoteState does not resurrect intentionally removed deploys on a prune/deploy race", async () => { const connection = fakeConnection(); const kept = { diff --git a/packages/cli/src/commands/sites/api.ts b/packages/cli/src/commands/sites/api.ts index 15abe369..d4be32ba 100644 --- a/packages/cli/src/commands/sites/api.ts +++ b/packages/cli/src/commands/sites/api.ts @@ -108,7 +108,7 @@ export async function readRemoteState( return { state, etag: sha256Hex(raw) }; } -// Write `_bunny/site.json` (returns the new etag). On an `expectedEtag` mismatch a parseable concurrent state is reconciled: deploy records merge (minus any `removedIds` this writer intentionally deleted, so a prune racing a deploy doesn't resurrect pruned records), and the current/previous pointers follow `promotedTo` (last promote wins; a non-promoting writer adopts the concurrent pointers rather than clobber them with its stale read). A concurrent record under `claimedId` with different content aborts instead of merging, and an unparseable conflict aborts rather than overwrite. +// Write `_bunny/site.json` (returns the new etag). On an `expectedEtag` mismatch a parseable concurrent state is reconciled: deploy records merge (minus any `removedIds` this writer intentionally deleted, so a prune racing a deploy doesn't resurrect pruned records), and the current/previous pointers follow `promotedTo` (last promote wins; a non-promoting writer adopts the concurrent pointers rather than clobber them with its stale read). An unparseable conflict aborts rather than overwrite. export async function writeRemoteState( connection: StorageZone, state: RemoteSiteState, @@ -118,8 +118,6 @@ export async function writeRemoteState( promotedTo?: string; /** Deploy IDs this writer intentionally removed (e.g. prune); the conflict merge must not resurrect them from concurrent state. */ removedIds?: readonly string[]; - /** Deploy ID whose files this writer owns (deploy's claim/finalize/promote writes). A concurrent record under it with a different contentHash aborts: `deploys/{id}/` can only hold one artifact, so an ours-win merge would vouch for bytes another writer is scribbling over. Storage has no compare-and-swap, so this is detection, not a lock — but it shrinks the blind window from the whole upload to this read-check-write. */ - claimedId?: string; }, ): Promise { if (expectedEtag) { @@ -132,20 +130,6 @@ export async function writeRemoteState( "Another process may be writing it. Re-run the command.", ); } - if (opts?.claimedId) { - const ourClaim = state.deploys.find((d) => d.id === opts.claimedId); - const theirClaim = remote.deploys.find((d) => d.id === opts.claimedId); - if ( - ourClaim && - theirClaim && - theirClaim.contentHash !== ourClaim.contentHash - ) { - throw new UserError( - `Another deploy is writing ${opts.claimedId} with different content.`, - `Two deploys raced the same ID, so deploys/${opts.claimedId}/ may hold a mix of both. Once the other finishes, re-run this deploy with --force to make this content the deploy, or leave the other writer's.`, - ); - } - } const ours = new Set(state.deploys.map((d) => d.id)); const removed = new Set(opts?.removedIds ?? []); state.deploys = [ @@ -671,5 +655,10 @@ export async function deleteDeployFiles( connection: StorageZone, deployId: string, ): Promise { - await siteFiles.remove(connection, `${deployPrefix(deployId)}/`); + try { + await siteFiles.remove(connection, `${deployPrefix(deployId)}/`); + } catch (err) { + // An absent prefix is already the goal (a fresh ID, or a re-run after a partial delete). + if (!isNotFoundError(err)) throw err; + } } diff --git a/packages/cli/src/commands/sites/constants.ts b/packages/cli/src/commands/sites/constants.ts index e298ac6f..c13f2fa6 100644 --- a/packages/cli/src/commands/sites/constants.ts +++ b/packages/cli/src/commands/sites/constants.ts @@ -31,13 +31,6 @@ export interface DeployRecord { contentHash: string; files: number; bytes: number; - /** - * Set while the deploy's files are being written, cleared once they all - * landed. A record still pending was interrupted (or is being written right - * now), so its prefix cannot be trusted to hold what `contentHash` says: - * deploy re-uploads it rather than no-op'ing, and publish refuses it. - */ - pending?: boolean; } // Source of truth (at `_bunny/site.json`) for a site's resource triple and deploys; `.bunny/site.json` is just a local pointer to it. diff --git a/packages/cli/src/commands/sites/deploy.test.ts b/packages/cli/src/commands/sites/deploy.test.ts index 525192a0..80a8527c 100644 --- a/packages/cli/src/commands/sites/deploy.test.ts +++ b/packages/cli/src/commands/sites/deploy.test.ts @@ -107,15 +107,18 @@ test("a custom id is used exactly as given and never aliases onto another deploy ).toEqual({ deployId: "Release-42", skipUpload: true }); }); +// The handler resolves a content conflict by asking (--force answers yes), so it is reported regardless of force. test("reusing a custom id for different content conflicts instead of overwriting", () => { const existing = deploy("r42", "hash1", "custom"); - const target = resolveDeployTarget({ - deploys: [existing], - identity: identity("r42", "hash2", "custom"), - customId: "r42", - force: false, - }); - expect(target.conflict).toEqual({ record: existing, reason: "content" }); + for (const force of [false, true]) { + const target = resolveDeployTarget({ + deploys: [existing], + identity: identity("r42", "hash2", "custom"), + customId: "r42", + force, + }); + expect(target.conflict).toEqual({ record: existing, reason: "content" }); + } }); // Two deploys whose storage paths differ only by case are indistinguishable to anything that folds case, @@ -137,28 +140,6 @@ test("an id differing only in case is refused, with or without --force", () => { ); }); -test("--force clears the conflict and re-uploads under the same id", () => { - const target = resolveDeployTarget({ - deploys: [deploy("r42", "hash1", "custom")], - identity: identity("r42", "hash2", "custom"), - customId: "r42", - force: true, - }); - expect(target).toEqual({ deployId: "r42", skipUpload: false }); -}); - -// An interrupted upload leaves a pending record; its prefix may hold only part of these bytes, so re-upload rather than no-op onto it. -test("a pending record never satisfies the no-op check", () => { - expect( - resolveDeployTarget({ - deploys: [{ ...deploy("r42", "hash1", "custom"), pending: true }], - identity: identity("r42", "hash1", "custom"), - customId: "r42", - force: false, - }), - ).toEqual({ deployId: "r42", skipUpload: false }); -}); - // Replacing the deploy production serves (or the rollback target) rewrites its prefix while the router reads it, so it is never forceable — custom ID or not. test("replacing the live or rollback deploy's content is refused, even with --force", () => { const live = deploy("r42", "hash1", "custom"); diff --git a/packages/cli/src/commands/sites/deploy.ts b/packages/cli/src/commands/sites/deploy.ts index 8585e95b..5312777f 100644 --- a/packages/cli/src/commands/sites/deploy.ts +++ b/packages/cli/src/commands/sites/deploy.ts @@ -12,8 +12,15 @@ import { errorMessage, UserError } from "../../core/errors.ts"; import { formatBytes } from "../../core/format.ts"; import { normalizeHostname } from "../../core/hostnames/index.ts"; import { logger } from "../../core/logger.ts"; -import { confirm, isInteractive, prompts, withSpinner } from "../../core/ui.ts"; import { + confirm, + isInteractive, + prompts, + requireConfirmable, + withSpinner, +} from "../../core/ui.ts"; +import { + deleteDeployFiles, ensureRouterCurrent, fetchSystemHostname, promoteDeploy, @@ -42,12 +49,7 @@ import { siteOptionBuilder, } from "./interactive.ts"; import { createLinkedSite, promptSiteName } from "./provision.ts"; -import { - collectFiles, - hashFiles, - pruneDeployOrphans, - uploadDeploy, -} from "./uploader.ts"; +import { collectFiles, hashFiles, uploadDeploy } from "./uploader.ts"; interface DeployArgs extends SiteSelectorArgs { dir?: string; @@ -66,14 +68,15 @@ export interface DeployTarget { /** * An existing deploy that blocks this one. * - * `content`: the same ID already holds different bytes; --force replaces it. - * `case`: an ID differing only in case exists. Not forceable, because two + * `content`: the same ID already holds different bytes; the handler asks + * before replacing them (--force answers yes). + * `case`: an ID differing only in case exists. Never replaceable, because two * deploys whose paths differ only by case are indistinguishable to anything * that folds case, and the loser's files would back the winner's rollback. * `live`/`rollback`: the ID holds different bytes AND is the production - * deploy or the rollback target. Not forceable, because replacing it means - * rewriting the very prefix the router serves (or would roll back to) - * file-by-file, and a failure mid-replace strands it on a mix of both. + * deploy or the rollback target. Never replaceable, because a replacement + * empties and rewrites the very prefix the router serves (or would roll + * back to). */ conflict?: { record: DeployRecord; @@ -104,13 +107,10 @@ export function resolveDeployTarget(opts: { const alreadyUploaded = force ? undefined - : deploys.find( - (d) => - // A pending record marks an interrupted (or in-flight) write; its prefix can't be trusted to hold these bytes, so re-upload instead of skipping. - !d.pending && - (customId - ? d.id === customId && d.contentHash === identity.contentHash - : d.contentHash === identity.contentHash), + : deploys.find((d) => + customId + ? d.id === customId && d.contentHash === identity.contentHash + : d.contentHash === identity.contentHash, ); // A skipped deploy reuses the already-uploaded deploy's id; that's where its files live. const deployId = alreadyUploaded?.id ?? identity.id; @@ -130,7 +130,7 @@ export function resolveDeployTarget(opts: { if (!skipUpload) { const existing = deploys.find((d) => d.id === deployId); if (existing && existing.contentHash !== identity.contentHash) { - // Replacing the deploy production serves (or would roll back to) rewrites its prefix while the router reads it, so it is refused outright — before the forceable content conflict, which would otherwise send the caller down a --force dead end. A same-bytes re-upload stays fine: every write is byte-identical. + // Replacing the deploy production serves (or would roll back to) rewrites its prefix while the router reads it, so it is refused outright — checked before the confirmable content conflict, which would otherwise send the caller down a dead end. A same-bytes re-upload stays fine: every write is byte-identical. if (deployId === current || deployId === previous) { return { deployId, @@ -141,7 +141,7 @@ export function resolveDeployTarget(opts: { }, }; } - if (customId && !force) { + if (customId) { return { deployId, skipUpload, @@ -224,7 +224,7 @@ export const sitesDeployCommand = defineCommand({ type: "boolean", default: false, describe: - "Deploy even when the content is unchanged, or replace an existing --deploy-id's content", + "Deploy even when the content is unchanged, and replace an existing --deploy-id's content without asking", }) .option("deploy-id", { type: "string", @@ -380,18 +380,24 @@ export const sitesDeployCommand = defineCommand({ ); } if (target.conflict?.reason === "content") { - throw new UserError( - `Deploy ${customId} already exists for ${state.name} with different content.`, - "Rolling back to that ID would serve these new files instead of the originals. Pick another ID, or pass --force to replace it.", + // Rolling back to the ID would serve the new files instead of the originals, so replacing is opt-in. + requireConfirmable(output, { + force: args.force, + message: `Deploy ${customId} already exists for ${state.name} with different content; replacing it needs a confirmation prompt.`, + hint: "Pick another ID, or re-run with --force to replace it non-interactively.", + }); + const proceed = await confirm( + `Deploy ${customId} already exists for ${state.name} with different content. Replace it?`, + { force: args.force }, ); + if (!proceed) { + logger.log("Cancelled."); + return; + } } const { deployId, skipUpload } = target; const alreadyLive = state.current === deployId; - // Re-uploading onto an existing ID (--force, or a rebuilt artifact under the same git sha) - // leaves any file the new build dropped behind in the prefix, still reachable via the router. - const replacing = - !skipUpload && state.deploys.some((d) => d.id === deployId); // The production URL prefers the custom domain; only fetch the system host when there is none. const systemHost = state.domain @@ -430,7 +436,28 @@ export const sitesDeployCommand = defineCommand({ } if (!skipUpload) { - // The deploy record. A re-deployed ID keeps its slot but gets fresh metadata; the purge on promote drops the old bytes from cache. + // Unless these exact bytes are already recorded under the ID, the upload starts from an empty prefix: emptying first is what keeps a replaced deploy free of files the new content dropped, and also clears half-written leftovers from an interrupted earlier run. + const existing = state.deploys.find((d) => d.id === deployId); + if (existing && existing.contentHash !== identity.contentHash) { + // Drop the record before deleting its files, so no record ever vouches for a prefix mid-rewrite (a concurrent publish re-reads state and refuses an ID without one), and a crashed replacement re-runs as a fresh upload. + state.deploys = state.deploys.filter((d) => d.id !== deployId); + etag = await writeRemoteState(connection, state, etag, { + removedIds: [deployId], + }); + } + if (existing?.contentHash !== identity.contentHash) { + await deleteDeployFiles(connection, deployId); + } + + await withSpinner(`Uploading ${files.length} files...`, (spin) => + uploadDeploy(connection, deployId, files, { + onFileUploaded: (done, total) => { + spin.text = `Uploading ${done}/${total} files (${formatBytes(totalBytes)} total)...`; + }, + }), + ); + + // Record the deploy. A re-deployed ID keeps its slot but gets fresh metadata; the purge on promote drops the old bytes from cache. const record: DeployRecord = { id: deployId, createdAt: new Date().toISOString(), @@ -441,49 +468,11 @@ export const sitesDeployCommand = defineCommand({ files: files.length, bytes: totalBytes, }; - - // Claim the ID in state before touching any object, and finalize only once every file landed: an interrupted or raced write leaves a record that says the prefix can't be trusted, never one vouching for bytes that aren't all there. The claim write also surfaces a concurrent deploy of the same ID (via `claimedId`) before this one starts scribbling over its files. - state.deploys = [ - { ...record, pending: true }, - ...state.deploys.filter((d) => d.id !== deployId), - ]; - etag = await writeRemoteState(connection, state, etag, { - claimedId: deployId, - }); - - try { - await withSpinner(`Uploading ${files.length} files...`, (spin) => - uploadDeploy(connection, deployId, files, { - onFileUploaded: (done, total) => { - spin.text = `Uploading ${done}/${total} files (${formatBytes(totalBytes)} total)...`; - }, - }), - ); - - if (replacing) { - const orphans = await withSpinner("Removing replaced files...", () => - pruneDeployOrphans(connection, deployId, files), - ); - if (orphans.length > 0 && output !== "json") { - logger.dim( - `Removed ${orphans.length} file(s) the new build no longer includes.`, - ); - } - } - } catch (err) { - logger.warn( - `Deploy ${deployId} is marked incomplete; re-run the deploy to finish it.`, - ); - throw err; - } - state.deploys = [ record, ...state.deploys.filter((d) => d.id !== deployId), ]; - etag = await writeRemoteState(connection, state, etag, { - claimedId: deployId, - }); + etag = await writeRemoteState(connection, state, etag); } await withSpinner("Publishing to production...", async () => { @@ -496,7 +485,6 @@ export const sitesDeployCommand = defineCommand({ markCurrent(state, deployId); etag = await writeRemoteState(connection, state, etag, { promotedTo: deployId, - claimedId: deployId, }); }); diff --git a/packages/cli/src/commands/sites/deployments/list.ts b/packages/cli/src/commands/sites/deployments/list.ts index f3e506fd..c5b7860f 100644 --- a/packages/cli/src/commands/sites/deployments/list.ts +++ b/packages/cli/src/commands/sites/deployments/list.ts @@ -80,14 +80,11 @@ export const sitesDeploymentsListCommand = defineCommand({ ["ID", "Status", "Created", "Source", "Files", "Size"], state.deploys.map((d) => [ d.id, - // Incomplete trumps the pointer markers: an interrupted upload is the actionable state, whatever the pointers say. - d.pending - ? "⚠ Incomplete" - : d.id === state.current - ? "● Live" - : d.id === state.previous - ? "○ Previous" - : "○", + d.id === state.current + ? "● Live" + : d.id === state.previous + ? "○ Previous" + : "○", formatDateTime(d.createdAt), deploySource(d), String(d.files), diff --git a/packages/cli/src/commands/sites/deployments/publish.ts b/packages/cli/src/commands/sites/deployments/publish.ts index 2c699187..50e676d9 100644 --- a/packages/cli/src/commands/sites/deployments/publish.ts +++ b/packages/cli/src/commands/sites/deployments/publish.ts @@ -98,14 +98,6 @@ export const sitesDeploymentsPublishCommand = defineCommand({ : "Run `bunny sites deployments list` to see available deploys.", ); } - // A pending record's upload never finished, so its files may be missing or mixed with an earlier deploy's. - if (deploy.pending) { - throw new UserError( - `Deploy ${targetId} never finished uploading and can't be published.`, - "Re-run `bunny sites deploy` for that content to complete it, or remove it with `bunny sites deployments delete`.", - ); - } - if (state.current === targetId) { if (output === "json") { logger.log( @@ -141,7 +133,7 @@ export const sitesDeploymentsPublishCommand = defineCommand({ } await withSpinner("Publishing...", async () => { - // Revalidate on fresh state right before promoting: the confirmation window is long enough for a concurrent deploy to have claimed this ID and started rewriting its files. + // Revalidate on fresh state right before promoting: the confirmation window is long enough for a concurrent replace to have dropped this deploy's record and started rewriting its files. const fresh = await readRemoteState(connection); if (!fresh) { throw new UserError( @@ -150,10 +142,9 @@ export const sitesDeploymentsPublishCommand = defineCommand({ ); } const { state: latest, etag: latestEtag } = fresh; - const record = latest.deploys.find((d) => d.id === targetId); - if (!record || record.pending) { + if (!latest.deploys.some((d) => d.id === targetId)) { throw new UserError( - `Deploy ${targetId} ${record ? "is being rewritten by a concurrent deploy" : `is gone from ${latest.name}`} and can't be published.`, + `Deploy ${targetId} is gone from ${latest.name} (a concurrent replace or delete?) and can't be published.`, "Run `bunny sites deployments list` and retry.", ); } diff --git a/packages/cli/src/commands/sites/uploader.test.ts b/packages/cli/src/commands/sites/uploader.test.ts index 8d61f86f..8d716896 100644 --- a/packages/cli/src/commands/sites/uploader.test.ts +++ b/packages/cli/src/commands/sites/uploader.test.ts @@ -7,18 +7,13 @@ import { siteFiles } from "./api.ts"; import { collectFiles, hashFiles, - pruneDeployOrphans, shouldSkipEntry, uploadDeploy, } from "./uploader.ts"; const realUpload = siteFiles.upload; -const realList = siteFiles.list; -const realRemove = siteFiles.remove; afterEach(() => { siteFiles.upload = realUpload; - siteFiles.list = realList; - siteFiles.remove = realRemove; }); const fakeConnection = {} as StorageZone; @@ -118,65 +113,3 @@ test("uploadDeploy surfaces an error after retries are exhausted", async () => { "permanent", ); }); - -// Stand in for a storage prefix: maps a listed directory to its entries. -function fakeStorage(paths: string[]) { - siteFiles.list = (async (_zone, dir: string) => { - const under = paths.filter((p) => p.startsWith(dir)); - const seen = new Map(); - for (const path of under) { - const rest = path.slice(dir.length); - const slash = rest.indexOf("/"); - seen.set(slash === -1 ? rest : rest.slice(0, slash), slash !== -1); - } - return [...seen].map(([objectName, isDirectory]) => ({ - objectName, - isDirectory, - length: 1, - })); - }) as typeof siteFiles.list; - - const removed: string[] = []; - siteFiles.remove = (async (_zone, path: string) => { - removed.push(path); - }) as typeof siteFiles.remove; - return removed; -} - -const hashed = (path: string) => - ({ path, absPath: `/tmp/${path}`, size: 1, sha256: "ab" }) as const; - -test("pruneDeployOrphans deletes only files the new build dropped", async () => { - const removed = fakeStorage([ - "deploys/r42/index.html", - "deploys/r42/old-page.html", - "deploys/r42/assets/app.js", - "deploys/r42/assets/old.css", - ]); - - const orphans = await pruneDeployOrphans(fakeConnection, "r42", [ - hashed("index.html"), - hashed("assets/app.js"), - ]); - - expect(orphans.sort()).toEqual(["assets/old.css", "old-page.html"]); - expect(removed.sort()).toEqual([ - "deploys/r42/assets/old.css", - "deploys/r42/old-page.html", - ]); -}); - -test("pruneDeployOrphans removes nothing when the build still has every file", async () => { - const removed = fakeStorage([ - "deploys/r42/index.html", - "deploys/r42/assets/app.js", - ]); - - const orphans = await pruneDeployOrphans(fakeConnection, "r42", [ - hashed("index.html"), - hashed("assets/app.js"), - ]); - - expect(orphans).toEqual([]); - expect(removed).toEqual([]); -}); diff --git a/packages/cli/src/commands/sites/uploader.ts b/packages/cli/src/commands/sites/uploader.ts index eca22792..152f633f 100644 --- a/packages/cli/src/commands/sites/uploader.ts +++ b/packages/cli/src/commands/sites/uploader.ts @@ -120,53 +120,3 @@ export async function uploadDeploy( }, ); } - -// Every object under a deploy's prefix, as paths relative to it. -async function listDeployObjects( - connection: StorageZone, - prefix: string, - dir = "", -): Promise { - const entries = await siteFiles.list(connection, `${prefix}/${dir}`); - const paths: string[] = []; - for (const entry of entries) { - const rel = `${dir}${entry.objectName}`; - if (entry.isDirectory) { - paths.push(...(await listDeployObjects(connection, prefix, `${rel}/`))); - } else { - paths.push(rel); - } - } - return paths; -} - -/** - * Delete objects an earlier upload of the same deploy ID left behind. - * - * Re-uploading writes the new files but never removes ones the artifact has - * dropped, so without this a replaced deploy serves a mix of both. Runs after - * the new files are in place, so a live deploy is never missing a file mid-replace. - * Returns the paths removed. - */ -export async function pruneDeployOrphans( - connection: StorageZone, - deployId: string, - files: HashedLocalFile[], -): Promise { - const prefix = deployPrefix(deployId); - const keep = new Set(files.map((file) => file.path)); - const orphans = (await listDeployObjects(connection, prefix)).filter( - (path) => !keep.has(path), - ); - - await mapWithConcurrency( - orphans, - DEFAULT_UPLOAD_CONCURRENCY, - async (path) => { - await withRetries(() => - siteFiles.remove(connection, `${prefix}/${path}`), - ); - }, - ); - return orphans; -} diff --git a/skills/bunny-cli/references/sites.md b/skills/bunny-cli/references/sites.md index d841cc6d..fcff67fb 100644 --- a/skills/bunny-cli/references/sites.md +++ b/skills/bunny-cli/references/sites.md @@ -47,8 +47,7 @@ Content is root-served, so client-side routers (TanStack Router, React Router, V - `--deploy-id ` sets the ID yourself, so a deploy can carry the same identifier as whatever produced it (a release tag, a catalog build, a timestamped artifact) and `deployments list` needs no cross-referencing. The ID is used **exactly as given**, case included: it exists to match your identifier, and it never appears in a client-facing URL (the router builds the origin path from it server-side). IDs become storage paths, so they take letters, digits and `-`, `_` or `.`, 4 to 64 characters, starting and ending alphanumeric: `20260827-1433-r42`, `Catalog_V3`, `v1.2.3`. - Deploy IDs are therefore **case-sensitive**. `publish`/`delete` match exactly and suggest a case variant when one exists, and deploying an ID that differs from an existing one only in case is refused (not even with `--force`), since two storage paths differing only by case are indistinguishable to anything that folds case. - An explicit ID is an assertion about identity, so it is never aliased onto an earlier deploy that happens to share content: each release keeps its own ID and rollback target even when the bytes are unchanged. - - Reusing an ID for **different** content is refused, because rolling back to that ID would then serve the new files instead of the originals. Pass `--force` to replace it deliberately; the replacement also removes files the new content no longer includes. The **live deploy and the rollback target are never replaceable in place** (not even with `--force`): that would rewrite the files the router is serving. Deploy under a new ID, or publish another deploy first. - - A deploy claims its ID in site state before uploading and finalizes it after, so an upload that is interrupted (or raced by a concurrent deploy of the same ID) leaves the record marked **incomplete** (`⚠ Incomplete` in `deployments list`) instead of one vouching for half-written files. An incomplete deploy can't be published; re-run the deploy (with `--force` if its content differs) to finish it, or `deployments delete` it. Two deploys writing the same ID with different content abort with an error when they detect each other, rather than silently mixing files. + - Reusing an ID for **different** content asks before replacing, because rolling back to that ID would then serve the new files instead of the originals (`--force` skips the prompt for CI). A replacement clears the old files first, so nothing stale survives. The **live deploy and the rollback target are never replaceable in place** (not even with `--force`): that would empty and rewrite the files the router is serving. Deploy under a new ID, or publish another deploy first. - The git sha is still recorded alongside a custom ID when the deploy came from a repo, so provenance is not lost; `deployments list` shows it as `custom (git abc12345)`. - Dotfiles and `node_modules` are never uploaded.