From 18abf710ab4eee53e5223eca364cd1dd1a1d432c Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Mon, 24 Aug 2026 13:19:47 +0700 Subject: [PATCH 1/2] feat: a Homebrew tap, because brew owns what it installs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit paddock ships pre-compiled binaries and self-updates. homebrew-core rejects both — a formula must build from source, and "software that updates itself conflicts with Homebrew's version and upgrade management" — and casks are not the escape hatch, since core rejecting a CLI does not make it cask-eligible. So distribution is a personal tap, one command: brew install lntvan166/paddock/paddock Fully qualified, not `brew tap` then `brew install`: since Homebrew 6.0.0 a non-official tap needs explicit trust, and the qualified name grants it for this one formula in a single step. `paddock update` now refuses inside a keg. This is not politeness. The Homebrew prefix is USER-writable, so the rename(2) would have SUCCEEDED, leaving `brew info paddock` reporting a version that is no longer the bytes on disk and the next `brew upgrade` reverting the operator with neither side saying anything. The existing "installed by a package manager" hint only fires when rename FAILS, which under brew it does not — so the guard is proactive, resolves realpath first (brew links /bin at the keg, and execPath may hand back either), and refuses before the download rather than after 83MB. `--check` still reports, because it writes nothing, and names `brew upgrade paddock` rather than the command that now declines. `/Cellar/` is matched as a path SEGMENT, so every prefix is covered — including a custom one, which enumerating the known three would have missed — while `~/Cellars/…` is not a false positive. The formula's checksums come from the release's own SHA256SUMS, so it cannot disagree with the artifacts it points at, and it carries depends_on "herdr": a tap formula may depend on a core one, so brew guarantees the thing paddock is useless without. No version constraint, because the herdr check is directional and core never moves backwards. Co-Authored-By: Claude Opus 5 --- .github/workflows/release.yml | 39 ++++++++++ .gitignore | 5 ++ Makefile | 10 ++- README.md | 13 ++++ docs/decisions.md | 34 +++++++++ docs/gotchas.md | 1 + packaging/homebrew/paddock.rb.tmpl | 74 ++++++++++++++++++ scripts/render-formula.ts | 66 ++++++++++++++++ src/server/update.ts | 69 ++++++++++++++++- tests/homebrew-formula.test.ts | 79 +++++++++++++++++++ tests/release-workflow.test.ts | 31 ++++++++ tests/update.test.ts | 117 ++++++++++++++++++++++++++++- 12 files changed, 533 insertions(+), 5 deletions(-) create mode 100644 packaging/homebrew/paddock.rb.tmpl create mode 100644 scripts/render-formula.ts create mode 100644 tests/homebrew-formula.test.ts diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9a3385b..9beb43f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -78,3 +78,42 @@ jobs: else gh release create "$GITHUB_REF_NAME" --generate-notes --verify-tag out/* fi + + # The tap comes LAST. The formula's urls point at the assets attached + # above, so a tap published any earlier serves a formula whose downloads + # 404 — and had that upload then failed, one advertising a release that + # never existed. + - name: Check out the Homebrew tap + uses: actions/checkout@v4 + with: + repository: lntvan166/homebrew-paddock + # github.token is scoped to THIS repository. Pushing to the tap with + # it fails 403 at the very end of an otherwise green release, so the + # tap gets its own fine-grained token, contents:write on that repo + # and nothing else. + token: ${{ secrets.HOMEBREW_TAP_TOKEN }} + path: tap + + - name: Render and push the formula + run: | + set -eu + VERSION="${GITHUB_REF_NAME#v}" + mkdir -p tap/Formula + bun run scripts/render-formula.ts "$VERSION" out/SHA256SUMS tap/Formula/paddock.rb + cd tap + git add Formula/paddock.rb + # Re-running a tag renders byte-identical output. That is not a + # failure, but `git commit` with nothing staged exits 1 and would + # fail the job on a legitimate re-run — so the no-op is named + # explicitly. Suppressing the exit code instead would hide a real + # failure here too, which is the thing CLAUDE.md forbids. + if git diff --cached --quiet; then + echo "formula already current for $VERSION — nothing to push" + else + # A literal identity, not an address: this repository is public and + # CLAUDE.md forbids committing email addresses. git does not + # require user.email to be one. + git -c user.name=paddock-release -c user.email=paddock-release \ + commit -m "paddock $VERSION" + git push + fi diff --git a/.gitignore b/.gitignore index 9f6a681..2f43dce 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,11 @@ build/ paddock paddock-* +# Release staging: the compiled binaries, their SHA256SUMS, and the rendered +# Homebrew formula (`make formula`). Built by the release workflow in CI and +# reproducible locally; nothing here is source. +out/ + # Generated by scripts/gen-embedded.ts (`make embed`). Vite content-hashes # asset names, so a committed copy would silently drift from the bundle it # claims to describe. diff --git a/Makefile b/Makefile index 5a81a2e..c310cc5 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,7 @@ export GID := $(shell id -g) TAG := $(firstword $(shell git tag --points-at HEAD)) VERSION := $(if $(TAG),$(TAG:v%=%),0.0.0-dev) -.PHONY: dev types icons check check-clean embed build-web test build up down logs restart +.PHONY: dev types icons check check-clean embed build-web test build formula up down logs restart # A real directory target, deliberately NOT in .PHONY: make compares its mtime # against package.json and bun.lock, so this installs on a fresh clone or after @@ -91,6 +91,14 @@ build: check check-clean test --define 'process.env.PADDOCK_VERSION="$(VERSION)"' \ src/server/index.ts --outfile paddock +# Renders the tap formula so it can be eyeballed without cutting a release. +# The release workflow runs the same script against the real SHA256SUMS; point +# this at any sums file: make formula SUMS=/tmp/SHA256SUMS +SUMS ?= out/SHA256SUMS +FORMULA_OUT ?= out/paddock.rb +formula: + bun run scripts/render-formula.ts $(VERSION) $(SUMS) $(FORMULA_OUT) + up: docker compose up -d --build diff --git a/README.md b/README.md index cddc169..c999e8e 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,19 @@ Installs to `~/.local/bin/paddock`, no `sudo`, checksum verified before anything is written · [read it first](https://lntvan166.github.io/paddock/install.sh) · [binaries](https://github.com/lntvan166/paddock/releases) +Or with Homebrew, which pulls in herdr as a dependency: + +```bash +brew install lntvan166/paddock/paddock +``` + +One command — the fully-qualified name taps and trusts this single formula. +Homebrew 6.0.0 requires explicit trust for a non-official tap, so a bare +`brew install paddock` cannot reach a tap; that name belongs to +`homebrew/core`, which paddock does not qualify for (`docs/decisions.md`). +Homebrew then owns the install, so upgrade with `brew upgrade paddock` — +`paddock update` detects the keg and declines rather than desyncing it. + ### herdr version paddock talks to herdr over herdr's own socket protocol. This release is built diff --git a/docs/decisions.md b/docs/decisions.md index 61de5c4..db5738d 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -534,3 +534,37 @@ session does not silently re-litigate them. way a failed key press or reply already is, the affordance is left in place for a retry, and the cursor is left exactly where it was so the retry asks for the same page rather than skipping ahead. + +19. **Homebrew ships from a personal tap, not `homebrew/core`, and `paddock + update` refuses under it.** Core is closed to paddock on two independent + counts, and neither is a matter of effort. Notability: a self-submission + by the repository owner needs 90 forks, 90 watchers or 225 stars + (`Package-Acceptance-Policy.md`). Self-update: *"Software that updates + itself conflicts with Homebrew's version and upgrade management"* + (`Acceptable-Formulae.md`) — which is `paddock update`, exactly. The + obvious escape hatch is closed by name: casks are for pre-built + distributions, and *"Open-source command-line-only software normally + belongs in homebrew/core as a formula built from source… A rejection from + homebrew/core does not by itself make the software eligible for + homebrew/cask."* + + herdr, by contrast, IS a core formula — built from a source tarball with + `rust` and `zig` as build deps, bottled by Homebrew's own CI, at ~32k + stars. That is the template if paddock ever qualifies: source build, no + self-update. It is also why the tap formula carries `depends_on "herdr"` — + a tap formula may depend on a core one, so brew can guarantee the thing + paddock is useless without. No version constraint, because paddock's herdr + check is directional and core never moves backwards. + + The bare name `paddock` is free in both core and cask and is deliberately + left unclaimed elsewhere, so a future core submission can still have it. + Until then the install is `brew install lntvan166/paddock/paddock`: since + Homebrew 6.0.0 a non-official tap needs explicit trust, and the + fully-qualified form grants it for that one formula in a single command. + + Under brew, `paddock update` refuses rather than warning-and-proceeding. + Warning and proceeding would leave `brew info` lying about what is + installed and let the next `brew upgrade` silently revert the operator — + and disabling self-update is a precondition for core anyway, so a clean + refusal is the same direction the project would have to move regardless. + diff --git a/docs/gotchas.md b/docs/gotchas.md index 6d149a2..3e256d7 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -25,6 +25,7 @@ one, recorded here so they are not reintroduced. | A test passes locally and fails in CI with "Attempted to assign to readonly property" | Bun runs every test file in ONE process, and `tests/support/dom.ts` makes globals readonly — so whether `globalThis.window = …` works depends on which file ran first. Adding test files changes that order | Fake a global with `Object.defineProperty`, restore its real descriptor, and put the setup INSIDE the `try` so a partial fake still unwinds | | One test fails and takes an unrelated test in another file with it | Globals faked before the `try`, so a throw skipped the restore and the next DOM file rendered against a two-property `window` | Setup inside the `try`; the restore is what must be unconditional | | `make test` fails about one run in twenty, looking like a timer flake | A test picked its port by arithmetic on `performance.now()` within a range that contains a real listener — paddock's own default port, on the machine of anyone running paddock | `tests/support/port.ts` asks the OS for a free port; a range that "looks unused" is a guess about someone else's machine | +| `brew info paddock` reports a version that is not the bytes on disk | The Homebrew prefix is USER-owned, so `paddock update`'s `rename(2)` over a keg SUCCEEDS. The existing "installed by a package manager" hint only fires when rename FAILS, so nothing was said — and the next `brew upgrade` reverted the operator's update without either side mentioning it | `update` resolves `realpath(selfPath)` and refuses when a `/Cellar/` segment is present, naming `brew upgrade paddock`. Refused BEFORE the download, and matched as a path SEGMENT so `~/Cellars/…` is not a false positive. `--check` still reports, because it writes nothing — and names the brew command, not `paddock update` | ## herdr protocol specifics diff --git a/packaging/homebrew/paddock.rb.tmpl b/packaging/homebrew/paddock.rb.tmpl new file mode 100644 index 0000000..cab6f3c --- /dev/null +++ b/packaging/homebrew/paddock.rb.tmpl @@ -0,0 +1,74 @@ +# Rendered by scripts/render-formula.ts and pushed to the tap by +# .github/workflows/release.yml. The copy in the tap repository is GENERATED — +# edit this template, never that file, or the next release overwrites the fix. +# +# This is a tap formula, not a homebrew-core one. Core requires a build from +# source and rejects software that updates itself; see docs/decisions.md. +class Paddock < Formula + desc "Watch and answer your coding agents from your phone" + homepage "https://github.com/lntvan166/paddock" + version "{{version}}" + license "MIT" + + # paddock reads herdr's own socket protocol and does nothing without it, so + # the dependency is real rather than a convenience. herdr is in + # homebrew-core, and a tap formula may depend on a core formula (the reverse + # is what Homebrew forbids), so brew can guarantee herdr is present instead + # of `paddock doctor` reporting its absence after the install. + # + # No version constraint, deliberately: paddock's herdr check is directional + # (README) — a NEWER herdr is accepted, only an older one is refused. Core + # never moves backwards, so tracking whatever it ships stays correct. + depends_on "herdr" + + on_macos do + on_arm do + url "https://github.com/lntvan166/paddock/releases/download/v{{version}}/paddock-macos-aarch64" + sha256 "{{sha256:paddock-macos-aarch64}}" + end + on_intel do + url "https://github.com/lntvan166/paddock/releases/download/v{{version}}/paddock-macos-x86_64" + sha256 "{{sha256:paddock-macos-x86_64}}" + end + end + + on_linux do + on_arm do + url "https://github.com/lntvan166/paddock/releases/download/v{{version}}/paddock-linux-aarch64" + sha256 "{{sha256:paddock-linux-aarch64}}" + end + on_intel do + url "https://github.com/lntvan166/paddock/releases/download/v{{version}}/paddock-linux-x86_64" + sha256 "{{sha256:paddock-linux-x86_64}}" + end + end + + livecheck do + url :stable + strategy :github_latest + end + + def install + # The release assets are bare binaries, not archives, so Homebrew stages + # each one under its own platform-specific name. Exactly one is present. + asset = Dir["paddock-*"].first + odie "no paddock binary in the staged download" if asset.nil? + bin.install asset => "paddock" + end + + def caveats + <<~CAVEAT + Homebrew owns this install, so `paddock update` will decline and send you + back here. Upgrade with: + brew upgrade paddock + CAVEAT + end + + test do + # The failure this catches has happened here before: a binary that reports + # 0.0.0-dev because a build-time define never reached it. Every test in the + # suite stayed green while every released binary was unupdatable. + ENV["PADDOCK_NO_UPDATE_CHECK"] = "1" + assert_match version.to_s, shell_output("#{bin}/paddock --version") + end +end diff --git a/scripts/render-formula.ts b/scripts/render-formula.ts new file mode 100644 index 0000000..7e5d191 --- /dev/null +++ b/scripts/render-formula.ts @@ -0,0 +1,66 @@ +/** + * Renders `packaging/homebrew/paddock.rb.tmpl` into the Homebrew formula the + * tap repository serves. + * + * The checksums come from the release's own SHA256SUMS rather than being + * recomputed here, so the formula and the published artifacts cannot disagree: + * there is one source for both, and it is the one `install.sh` and + * `paddock update` already verify against. + * + * The template names its own platforms. This renderer resolves whatever + * `{{sha256:}}` placeholders it finds, so adding or dropping a platform + * is a template edit and nothing here changes. + */ + +/** Parses `sha256sum` output: ` `, or ` *` in binary mode. */ +function digests(sums: string): Map { + const out = new Map(); + for (const line of sums.split("\n")) { + const trimmed = line.trim(); + if (!trimmed) continue; + const [digest, ...rest] = trimmed.split(/\s+/); + const asset = rest.join(" ").replace(/^\*/, ""); + if (digest && asset) out.set(asset, digest); + } + return out; +} + +export function renderFormula(tmpl: string, version: string, sums: string): string { + const known = digests(sums); + const missing: string[] = []; + + const out = tmpl + .replaceAll("{{version}}", version) + .replace(/\{\{sha256:([^}]+)\}\}/g, (_match, asset: string) => { + const digest = known.get(asset); + if (!digest) { + // Collected rather than thrown at the first miss, so a release that + // published none of its assets reports all four instead of sending + // whoever is debugging it round the loop once per platform. + missing.push(asset); + return ""; + } + return digest; + }); + + if (missing.length > 0) { + throw new Error( + `render-formula: not listed in SHA256SUMS: ${missing.sort().join(", ")}`, + ); + } + return out; +} + +// Run as a program only when invoked directly, so the tests above can import +// the renderer without it trying to read a release's files. +if (import.meta.main) { + const [version, sumsPath, outPath] = process.argv.slice(2); + if (!version || !sumsPath || !outPath) { + console.error("usage: bun run scripts/render-formula.ts "); + process.exit(2); + } + const tmpl = await Bun.file("packaging/homebrew/paddock.rb.tmpl").text(); + const sums = await Bun.file(sumsPath).text(); + await Bun.write(outPath, renderFormula(tmpl, version.replace(/^v/, ""), sums)); + console.log(`render-formula: wrote ${outPath} for ${version}`); +} diff --git a/src/server/update.ts b/src/server/update.ts index 7f1258a..2759b9c 100644 --- a/src/server/update.ts +++ b/src/server/update.ts @@ -1,4 +1,4 @@ -import { chmod, rename, rm, writeFile } from "node:fs/promises"; +import { chmod, realpath, rename, rm, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; import { say } from "@server/term"; @@ -9,6 +9,25 @@ const REPO = "lntvan166/paddock"; * disagree about which asset to fetch. Windows returns null deliberately: * herdr publishes no Windows build, so there is nothing to connect to. */ +/** + * Whether this binary lives inside a Homebrew keg. + * + * A tap installs the SAME released asset this updater downloads, so the two + * are byte-identical and there is no build-time flag to tell them apart. The + * install path is the only signal, and `/Cellar/` is the one invariant worth + * matching: EVERY Homebrew prefix — /opt/homebrew, /usr/local, Linuxbrew's + * default (a `.linuxbrew` directory inside a home directory), or a custom one + * — puts kegs under /Cellar///. Enumerating the known + * prefixes instead would silently stop recognising a custom one. + * + * Matched as a whole path SEGMENT, not a substring: `~/Cellars/paddock` and + * `~/wine-cellar/bin/paddock` are ordinary paths and must not be mistaken for + * kegs, which is what `includes("/Cellar")` would do. + */ +export function isBrewManaged(path: string): boolean { + return path.split("/").includes("Cellar"); +} + export function assetName(platform: string, arch: string): string | null { const os = platform === "darwin" ? "macos" : platform === "linux" ? "linux" : null; const cpu = arch === "arm64" ? "aarch64" : arch === "x64" ? "x86_64" : null; @@ -128,7 +147,53 @@ export async function runUpdate(o: UpdateOpts): Promise { return 0; } log(`paddock: ${o.current} -> ${latest}`); - if (o.checkOnly) { log("paddock: run `paddock update` to install it"); return 0; } + + // Where the binary REALLY is, before anything decides whether to write it. + // + // process.execPath may hand back the symlink Homebrew leaves in + // /bin rather than the keg it points at, and a guard that inspected + // only the literal string would miss that and overwrite the keg THROUGH the + // link. Resolved here rather than at the top so the common "already current" + // path above costs no syscall. + let resolved = o.selfPath; + try { + resolved = await realpath(o.selfPath); + } catch (e) { + // Not silenced, and not fatal. A path that cannot be resolved is still + // the path a write would target, so the check below remains meaningful + // for the literal — but an operator whose binary path just failed to + // resolve should hear about it rather than find out via a later error. + log(`paddock: could not resolve ${o.selfPath}: ${(e as Error).message}`); + } + const brewed = isBrewManaged(resolved); + + if (o.checkOnly) { + // --check writes nothing, so brew is no reason to refuse it — the in-app + // update banner reads this signal. It must name the command that actually + // works for this install, though: telling a brew user to run `paddock + // update` sends them to the command the next branch refuses. + log( + brewed + ? "paddock: run `brew upgrade paddock` to install it" + : "paddock: run `paddock update` to install it", + ); + return 0; + } + + // Refused BEFORE the download: pulling 83MB to then say no would be a silly + // way to say it. + // + // The Homebrew prefix is user-owned, so the rename(2) below would SUCCEED + // here — leaving `brew info paddock` reporting a version that is no longer + // the bytes on disk, and `brew upgrade` later reverting the operator's + // update without either side saying anything. The "installed by a package + // manager" hint further down only fires when rename FAILS, which under brew + // it does not. + if (brewed) { + log(`paddock: this binary is managed by Homebrew (${resolved})`); + log("paddock: run `brew upgrade paddock` instead"); + return 1; + } const base = `https://github.com/${REPO}/releases/download/v${latest}`; let bytes: Uint8Array; diff --git a/tests/homebrew-formula.test.ts b/tests/homebrew-formula.test.ts new file mode 100644 index 0000000..d0c142c --- /dev/null +++ b/tests/homebrew-formula.test.ts @@ -0,0 +1,79 @@ +import { expect, test } from "bun:test"; +import { renderFormula } from "../scripts/render-formula"; + +const TMPL = await Bun.file("packaging/homebrew/paddock.rb.tmpl").text(); + +/** + * The four assets release.yml publishes. Deliberately repeated here rather + * than imported from update.ts: this is the list a MAC user's `brew install` + * depends on, and a test that derived it from the same source as the code + * would agree with a mistake in that source. + */ +const ASSETS = [ + "paddock-linux-x86_64", + "paddock-linux-aarch64", + "paddock-macos-x86_64", + "paddock-macos-aarch64", +]; + +/** A stand-in SHA256SUMS, in the format `sha256sum` actually writes. */ +const SUMS = ASSETS.map((a, i) => `${String(i + 1).repeat(64)} ${a}`).join("\n") + "\n"; + +test("no placeholder survives into the rendered formula", () => { + // An unsubstituted {{...}} is not a cosmetic bug: `sha256 "{{sha256:...}}"` + // is valid Ruby, so brew would accept the formula and fail every install + // with a checksum mismatch instead of a syntax error. + const out = renderFormula(TMPL, "1.2.3", SUMS); + expect(out).not.toContain("{{"); + expect(out).not.toContain("}}"); +}); + +test("each platform gets its own checksum, not another platform's", () => { + const out = renderFormula(TMPL, "1.2.3", SUMS); + for (const [i, asset] of ASSETS.entries()) { + const digest = String(i + 1).repeat(64); + expect(out).toContain(digest); + // The digest must sit in the same block as the asset it belongs to: a + // renderer that substituted all four correctly but in the wrong order + // would still satisfy a bare `toContain` on each. + const at = out.indexOf(asset); + expect(at).toBeGreaterThan(-1); + const nearby = out.slice(Math.max(0, at - 200), at + 200); + expect(nearby).toContain(digest); + } +}); + +test("the version reaches both the url and the version field", () => { + const out = renderFormula(TMPL, "1.2.3", SUMS); + expect(out).toContain('version "1.2.3"'); + expect(out).toContain("download/v1.2.3/"); +}); + +test("a checksum missing from SHA256SUMS is refused, not left blank", () => { + // A release published without one of its assets must fail the render, not + // ship a formula whose sha256 is the empty string. + const partial = `${"1".repeat(64)} paddock-linux-x86_64\n`; + expect(() => renderFormula(TMPL, "1.2.3", partial)).toThrow(/paddock-linux-aarch64/); +}); + +test("the template covers exactly the four platforms herdr supports", () => { + for (const a of ASSETS) expect(TMPL).toContain(a); + expect(TMPL).not.toContain("windows"); + expect(TMPL).not.toContain("Windows"); +}); + +test("the formula depends on herdr, which is in homebrew-core", () => { + // paddock reads herdr's unix socket and does nothing without it. herdr is + // a core formula, and a tap formula may depend on core, so brew can + // guarantee it is present instead of `paddock doctor` reporting its + // absence after the fact. + expect(TMPL).toContain('depends_on "herdr"'); +}); + +test("the formula smoke-tests the binary it installed", () => { + // Homebrew runs `brew test` in CI and on request. Asserting the version + // catches the failure mode this project has already hit once: a binary + // that reports 0.0.0-dev because a build-time define did not reach it. + expect(TMPL).toContain("test do"); + expect(TMPL).toContain("--version"); +}); diff --git a/tests/release-workflow.test.ts b/tests/release-workflow.test.ts index e838b57..e225be3 100644 --- a/tests/release-workflow.test.ts +++ b/tests/release-workflow.test.ts @@ -51,3 +51,34 @@ test("the compiled artifact's version stamp is smoke-tested before it is publish expect(wf).toContain("out/paddock-linux-x86_64 --version"); expect(wf).toMatch(/if \[ "\$GOT" != "\$VERSION" \]; then/); }); + +test("the Homebrew formula is rendered and pushed to the tap", () => { + expect(wf).toContain("scripts/render-formula.ts"); + expect(wf).toContain("homebrew-paddock"); +}); + +test("the formula is pushed AFTER the release exists, not before", () => { + // The formula's urls point at release assets. Pushing it first publishes a + // tap that 404s for every install until the upload finishes -- and if the + // upload then fails, the tap advertises a release that does not exist. + const attachAt = wf.indexOf("- name: Attach to the release"); + const formulaAt = wf.indexOf("scripts/render-formula.ts"); + expect(attachAt).toBeGreaterThan(-1); + expect(formulaAt).toBeGreaterThan(attachAt); +}); + +test("the tap push uses its own token, because github.token cannot reach another repo", () => { + // GITHUB_TOKEN is scoped to this repository. A push to the tap with it fails + // 403 at the end of an otherwise successful release. + expect(wf).toContain("HOMEBREW_TAP_TOKEN"); +}); + +test("a failed tap push does not silently pass", () => { + // CLAUDE.md forbids swallowed errors, and a release whose tap step failed + // quietly is exactly the silent break that rule exists for: every binary + // published, brew still serving the previous version, nothing red. + const formulaAt = wf.indexOf("scripts/render-formula.ts"); + const tail = wf.slice(formulaAt); + expect(tail).not.toContain("|| true"); + expect(tail).not.toContain("continue-on-error"); +}); diff --git a/tests/update.test.ts b/tests/update.test.ts index 9dc884a..fdeb81f 100644 --- a/tests/update.test.ts +++ b/tests/update.test.ts @@ -1,8 +1,8 @@ import { expect, test } from "bun:test"; -import { chmod, mkdir, mkdtemp, readdir, readFile, rm, stat, writeFile } from "node:fs/promises"; +import { chmod, mkdir, mkdtemp, readdir, readFile, rm, stat, symlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { assetName, isNewer, runUpdate } from "@server/update"; +import { assetName, isBrewManaged, isNewer, runUpdate } from "@server/update"; test("the asset table matches install.sh exactly, so the two cannot disagree", () => { expect(assetName("linux", "x64")).toBe("paddock-linux-x86_64"); @@ -304,3 +304,116 @@ test("update says nothing about restarting when nothing is running", async () => }); expect(said.join("\n")).not.toContain("restart"); }); + +// --- Homebrew-managed installs ------------------------------------------ +// +// A tap installs the SAME released binary this updater downloads, so there is +// no build-time flag to distinguish the two — the artifact is byte-identical. +// The install PATH is the only signal, and `/Cellar/` is the one invariant: +// every Homebrew prefix (/opt/homebrew, /usr/local, Linuxbrew's default under +// a home directory, or a custom one) puts kegs under +// /Cellar///. Home paths are written as /path/to/ here +// because this repository is public and CLAUDE.md forbids absolute home paths; +// what the check actually reads is the segment, not the prefix. + +test("a Cellar path is recognised under every Homebrew prefix", () => { + expect(isBrewManaged("/opt/homebrew/Cellar/paddock/0.8.4/bin/paddock")).toBe(true); + expect(isBrewManaged("/usr/local/Cellar/paddock/0.8.4/bin/paddock")).toBe(true); + expect(isBrewManaged("/path/to/linuxbrew/.linuxbrew/Cellar/paddock/0.8.4/bin/paddock")).toBe(true); + expect(isBrewManaged("/opt/custom-brew/Cellar/paddock/0.8.4/bin/paddock")).toBe(true); +}); + +test("an ordinary install path is not mistaken for a Homebrew keg", () => { + // The installer's default, and the two paths most likely to be a false + // positive: a directory merely NAMED Cellar-something, and a project that + // happens to have the word in it. + expect(isBrewManaged("/path/to/.local/bin/paddock")).toBe(false); + expect(isBrewManaged("/usr/local/bin/paddock")).toBe(false); + expect(isBrewManaged("/path/to/Cellars/paddock")).toBe(false); + expect(isBrewManaged("/path/to/wine-cellar/bin/paddock")).toBe(false); +}); + +/** The real brew layout: a keg under Cellar, symlinked into /bin. */ +async function brewHarness(body: string, sum: string) { + const dir = await mkdtemp(join(tmpdir(), "paddock-brew-")); + const keg = join(dir, "opt", "homebrew", "Cellar", "paddock", "0.8.4", "bin"); + await mkdir(keg, { recursive: true }); + const kegBin = join(keg, "paddock"); + await writeFile(kegBin, "BREW BINARY"); + await chmod(kegBin, 0o755); + const linkDir = join(dir, "opt", "homebrew", "bin"); + await mkdir(linkDir, { recursive: true }); + const link = join(linkDir, "paddock"); + await symlink(kegBin, link); + const h = await harness(body, sum); + return { dir, kegBin, link, fetchImpl: h.fetchImpl }; +} + +test("update refuses to overwrite a Homebrew keg and points at brew", async () => { + // The Homebrew prefix is user-owned, so rename(2) here would SUCCEED -- + // leaving `brew info paddock` reporting a version that is no longer the + // bytes on disk. The existing "installed by a package manager" message + // only fires when rename FAILS, which under brew it does not. + const body = "NEW BINARY"; + const h = await brewHarness(body, await sha(body)); + const said: string[] = []; + const code = await runUpdate({ + selfPath: h.kegBin, platform: "linux", arch: "x64", + current: "0.1.0", fetchImpl: h.fetchImpl, log: (l) => said.push(l), + }); + expect(code).not.toBe(0); + expect(await readFile(h.kegBin, "utf8")).toBe("BREW BINARY"); + expect(said.join("\n")).toContain("brew upgrade paddock"); +}); + +test("update follows the brew symlink before deciding, so /bin is caught too", async () => { + // process.execPath may hand back either the symlink in /bin or the + // resolved keg path. A guard that only inspects the literal string misses + // the former and overwrites the keg through the link. + const body = "NEW BINARY"; + const h = await brewHarness(body, await sha(body)); + const code = await runUpdate({ + selfPath: h.link, platform: "linux", arch: "x64", + current: "0.1.0", fetchImpl: h.fetchImpl, log: () => {}, + }); + expect(code).not.toBe(0); + expect(await readFile(h.kegBin, "utf8")).toBe("BREW BINARY"); +}); + +test("--check still reports a new version under brew, and names brew upgrade", async () => { + // The check writes nothing, so there is no reason to refuse it -- and the + // in-app update banner depends on this signal. It must name the command + // that actually works here, not `paddock update`. + const body = "NEW BINARY"; + const h = await brewHarness(body, await sha(body)); + const said: string[] = []; + const code = await runUpdate({ + selfPath: h.kegBin, platform: "linux", arch: "x64", + current: "0.1.0", checkOnly: true, fetchImpl: h.fetchImpl, + log: (l) => said.push(l), + }); + expect(code).toBe(0); + const text = said.join("\n"); + expect(text).toContain("0.1.0 -> 9.9.9"); + expect(text).toContain("brew upgrade paddock"); + expect(text).not.toContain("run `paddock update`"); +}); + +test("the brew guard does not download the binary before refusing", async () => { + // Refusing after pulling 83MB would be a silly way to say no. The release + // API call is fine -- it is how the version is known -- but the asset must + // never be fetched. + const body = "NEW BINARY"; + const h = await brewHarness(body, await sha(body)); + const asked: string[] = []; + const spy = (async (url: string) => { + asked.push(String(url)); + return h.fetchImpl(url as unknown as Request); + }) as unknown as typeof fetch; + await runUpdate({ + selfPath: h.kegBin, platform: "linux", arch: "x64", + current: "0.1.0", fetchImpl: spy, log: () => {}, + }); + expect(asked.some((u) => u.includes("releases/latest"))).toBe(true); + expect(asked.some((u) => u.endsWith("paddock-linux-x86_64"))).toBe(false); +}); From 5c8509086750a81303759c5a2d1fc54694d8a9dd Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Mon, 24 Aug 2026 13:29:35 +0700 Subject: [PATCH 2/2] fix: a banner that tells a brew user to run the command that declines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ReleaseBanner` printed `paddock update` to everyone. Under Homebrew that command now refuses by design, so the notice named an action that does not work — the same defect as a mislabelled Approve button, which CLAUDE.md rules out for exactly this reason. A control is labelled from what actually owns the install, never guessed. The install's owner is a fact the client cannot derive, so it rides the WS envelope beside `latestKnown`: `managedBy: "homebrew" | null`. A union rather than a boolean because the command differs per manager, so a second packager adds a case instead of a second field. `detectManagedBy` is where the realpath hop lives, extracted rather than inlined at the composition root because a boot-time expression in index.ts cannot be tested — and this one has a branch worth testing: brew leaves a symlink in /bin and `process.execPath` may hand back either that or the keg. Resolved ONCE at boot, as a value rather than a getter, because the path of a running executable cannot change and `update` refuses to swap it. `managedBy` is REQUIRED on `HealthBody`, matching the reasoning already recorded there for `latestKnown`: an optional field lets a later edit drop it silently, and a phone would read the absence as "unmanaged" and print the wrong command again. Making it required turned that into the type error that found both wiring sites. Verified end to end, not only in units: a binary compiled into a Cellar path reports `managedBy: "homebrew"` on /api/health, refuses `update` through both the keg path and the /bin symlink, leaves the binary byte-identical with no temp file behind, and still reports the new version under `--check`. Co-Authored-By: Claude Opus 5 --- src/server/index.ts | 17 ++++++++-- src/server/routes.ts | 12 +++++++- src/server/update.ts | 29 ++++++++++++++++++ src/server/ws/hub.ts | 13 ++++++-- src/shared/types.ts | 28 ++++++++++++++++- src/web/components/App.tsx | 3 +- src/web/components/ReleaseBanner.tsx | 12 ++++++-- src/web/store.ts | 24 ++++++++++++++- tests/action-routes.test.ts | 10 +++--- tests/build-update.test.ts | 2 +- tests/grouping.test.ts | 2 +- tests/hub.test.ts | 23 ++++++++++++++ tests/immutable-cache.test.ts | 2 +- tests/journal-route.test.ts | 2 +- tests/origin-gate.test.ts | 2 +- tests/origin-tunnel.test.ts | 2 +- tests/release-banner.test.tsx | 46 +++++++++++++++++++++++++--- tests/routes.test.ts | 20 +++++++++++- tests/settings-routes.test.ts | 2 +- tests/static.test.ts | 2 +- tests/tunnel-gate-scope.test.ts | 2 +- tests/tunnel-routes.test.ts | 2 +- tests/update.test.ts | 22 ++++++++++++- tests/web-store.test.ts | 16 +++++++++- 24 files changed, 263 insertions(+), 32 deletions(-) diff --git a/src/server/index.ts b/src/server/index.ts index 8ea30c3..10fd7f7 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -32,7 +32,7 @@ import { preflight, tunnelHint } from "@server/tunnel/preflight"; import { runTunnel } from "@server/tunnel/run"; import { VERSION } from "@server/version"; import { runDoctor } from "@server/doctor"; -import { runUpdate } from "@server/update"; +import { detectManagedBy, runUpdate } from "@server/update"; import { noUpdateCheckRequested, scheduleUpdateChecks } from "@server/update-check"; import { say, warn } from "@server/term"; import { BootLog } from "@server/boot-log"; @@ -293,7 +293,19 @@ const updateChecks = scheduleUpdateChecks( }, ); -const hub = new Hub({ build: currentBuildId, latestKnown: () => latestKnown }); +/** + * Which package manager owns this binary, resolved ONCE at boot. + * + * A constant, unlike `latestKnown`: the path of a running executable does not + * change, and `update` refuses inside a keg so it will not be swapped + * underneath either. It rides the WS envelope because the client cannot know + * which upgrade command applies — `paddock update` declines under Homebrew, + * so a banner naming it there would label the notice with an action that + * refuses. + */ +const managedBy = await detectManagedBy(process.execPath); + +const hub = new Hub({ build: currentBuildId, latestKnown: () => latestKnown, managedBy }); const settings = new SettingsStore(defaultConfigDir()); await settings.load(); @@ -576,6 +588,7 @@ const appDeps = { lastNotifyError: notifier.lastError, version: VERSION, latestKnown, + managedBy, herdrProtocol: DEMO ? HERDR_PROTOCOL : herdrProtocol, // Read from the supervisor rather than cached here, for the same reason // herdrConnected reads the stream: a copy can go stale and then lie. diff --git a/src/server/routes.ts b/src/server/routes.ts index b0bae34..fd822f6 100644 --- a/src/server/routes.ts +++ b/src/server/routes.ts @@ -19,7 +19,7 @@ import { EMBEDDED } from "@server/embedded"; import { allowWrite, hostOf, refusalReason } from "@server/origin"; import { warn } from "@server/term"; import type { JournalReader } from "@server/journal/read"; -import { isNavKey, type NotifyTrigger, type SettingsPatch } from "@shared/types"; +import { isNavKey, type ManagedBy, type NotifyTrigger, type SettingsPatch } from "@shared/types"; import { diffScreens, digestOf } from "@shared/screen"; import type { HerdrAgentSession } from "@shared/herdr-api"; @@ -58,6 +58,16 @@ export interface HealthBody { * field from `health()` must be a type error, not a silently missing key. */ latestKnown: string | null; + /** + * The package manager that owns this install, or null for the ordinary case. + * + * Exposed because the UPGRADE COMMAND depends on it: `paddock update` + * refuses inside a Homebrew keg, so anything telling an operator to run it + * there is wrong. Required rather than optional, for the same reason as + * `latestKnown` above — a future edit to `health()` that drops it must be a + * type error, not a silently missing key a phone then reads as "unmanaged". + */ + managedBy: ManagedBy | null; /** * The protocol the LIVE herdr reports, or null before it has answered. * diff --git a/src/server/update.ts b/src/server/update.ts index 2759b9c..1bfcc72 100644 --- a/src/server/update.ts +++ b/src/server/update.ts @@ -1,9 +1,38 @@ import { chmod, realpath, rename, rm, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; +import type { ManagedBy } from "@shared/types"; import { say } from "@server/term"; const REPO = "lntvan166/paddock"; +/** + * Which package manager owns the binary at `execPath`, if any. + * + * Called once at boot rather than per request: the path of a running + * executable does not change, and `update` refuses inside a keg so it will not + * be swapped underneath either. + * + * Resolves the symlink first — Homebrew leaves one in `/bin` pointing + * at the keg, and `process.execPath` may hand back either. A path that cannot + * be resolved is reported as unmanaged rather than thrown: a source checkout's + * execPath is the operator's `bun`, and boot must not die because a binary was + * moved underneath a running process. + */ +export async function detectManagedBy(execPath: string): Promise { + let resolved = execPath; + try { + resolved = await realpath(execPath); + } catch (e) { + // Said out loud, not swallowed. This only fires when the executable cannot + // be resolved at all — a binary moved or deleted underneath a running + // process — which is worth one line at boot. `console.info` rather than a + // thrown error, and the literal path is still checked below, because + // failing to classify the install must not stop the server starting. + console.info(`paddock: could not resolve ${execPath}: ${(e as Error).message}`); + } + return isBrewManaged(resolved) ? "homebrew" : null; +} + /** * The same mapping `install.sh` uses, so the installer and the updater cannot * disagree about which asset to fetch. Windows returns null deliberately: diff --git a/src/server/ws/hub.ts b/src/server/ws/hub.ts index 13f3d40..a31258a 100644 --- a/src/server/ws/hub.ts +++ b/src/server/ws/hub.ts @@ -1,4 +1,4 @@ -import type { Agent, ServerMessage } from "@shared/types"; +import type { Agent, ManagedBy, ServerMessage } from "@shared/types"; export interface HubClient { send(data: string): void; @@ -18,6 +18,12 @@ export interface HubOptions { * value is produced (a GitHub release check, cached on disk), only that * it is read fresh on every snapshot/heartbeat. */ latestKnown?: () => string | null; + /** The package manager owning this install, if any. + * + * A VALUE, not a getter like `build` and `latestKnown`: the path of the + * running executable cannot change while the process runs, and `update` + * refuses inside a keg, so there is nothing to re-read. */ + managedBy?: ManagedBy | null; } /** @@ -41,6 +47,7 @@ export class Hub { */ private readonly build: () => string | null; private readonly latestKnown: () => string | null; + private readonly managedBy: ManagedBy | null; constructor(opts: HubOptions = {}) { this.coalesceMs = opts.coalesceMs ?? 100; @@ -48,6 +55,7 @@ export class Hub { this.now = opts.now ?? Date.now; this.build = opts.build ?? (() => null); this.latestKnown = opts.latestKnown ?? (() => null); + this.managedBy = opts.managedBy ?? null; } get clientCount(): number { @@ -82,6 +90,7 @@ export class Hub { sendHeartbeat(): void { const msg: ServerMessage = { type: "heartbeat", serverTime: this.now(), build: this.build(), latestKnown: this.latestKnown(), + managedBy: this.managedBy, }; for (const client of [...this.clients]) this.sendTo(client, msg); } @@ -97,7 +106,7 @@ export class Hub { sendSnapshot(client: HubClient, hostId: string, agents: Agent[]): void { this.sendTo(client, { type: "snapshot", hostId, agents, serverTime: this.now(), - build: this.build(), latestKnown: this.latestKnown(), + build: this.build(), latestKnown: this.latestKnown(), managedBy: this.managedBy, }); } diff --git a/src/shared/types.ts b/src/shared/types.ts index acfa0d2..f51e9eb 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -262,6 +262,18 @@ export type ServerMessage = * riding it gives eventual consistency for free. */ latestKnown?: string | null; + /** + * The package manager that owns this install, or null for the ordinary + * case (the installer's `~/.local/bin`, a container, a source build). + * + * On the wire because the UPGRADE COMMAND differs and the client cannot + * know which one applies: `paddock update` refuses inside a Homebrew keg + * (see `update.ts`), so a banner naming it would be telling the operator + * to run something that declines. Absent and null are different claims — + * "this frame does not say" versus "nothing owns it" — and the store + * distinguishes them. + */ + managedBy?: ManagedBy | null; } | { type: "delta"; upserted: Agent[]; removedIds: string[]; serverTime: number } /** @@ -280,7 +292,21 @@ export type ServerMessage = * is running stale JavaScript. `index.html` is `no-cache`, which fixes fresh * loads and does nothing for a tab left open on a phone for days. */ - | { type: "heartbeat"; serverTime: number; build?: string | null; latestKnown?: string | null }; + | { + type: "heartbeat"; + serverTime: number; + build?: string | null; + latestKnown?: string | null; + managedBy?: ManagedBy | null; + }; + +/** + * A package manager that owns a paddock install and therefore owns its + * upgrades. One member today; a union rather than a boolean because the + * command to print differs per manager, so a second entry adds a case rather + * than a second field. + */ +export type ManagedBy = "homebrew"; export const SECTION_ORDER = ["needs-you", "working", "idle"] as const; export type Section = (typeof SECTION_ORDER)[number]; diff --git a/src/web/components/App.tsx b/src/web/components/App.tsx index 2dcf8b6..926f1fc 100644 --- a/src/web/components/App.tsx +++ b/src/web/components/App.tsx @@ -18,7 +18,7 @@ import { readPrefs, themeAttr } from "@web/prefs"; export function App() { const { - agents, hostId, connected, lastMessageAt, updateAvailable, latestKnown, connect, + agents, hostId, connected, lastMessageAt, updateAvailable, latestKnown, managedBy, connect, } = useStore(); const [now, setNow] = useState(() => Date.now()); // Expanded by default. Collapsed, idle agents render as chips that carry a @@ -123,6 +123,7 @@ export function App() { {shouldShowRelease(latestKnown, dismissedVersion) && ( { dismissRelease(latestKnown!); setDismissedVersion(latestKnown); diff --git a/src/web/components/ReleaseBanner.tsx b/src/web/components/ReleaseBanner.tsx index 3923750..2b53e8f 100644 --- a/src/web/components/ReleaseBanner.tsx +++ b/src/web/components/ReleaseBanner.tsx @@ -14,12 +14,20 @@ * what it is — and the dismiss control is a real button, not a hover-revealed * affordance, because on a phone there is no hover. */ +import type { ManagedBy } from "@shared/types"; + export function ReleaseBanner({ - version, onDismiss, + version, managedBy, onDismiss, }: { version: string; + managedBy: ManagedBy | null; onDismiss: () => void; }) { + // Named from what actually owns the install, never guessed. `paddock update` + // refuses inside a Homebrew keg (src/server/update.ts), so printing it there + // labels the notice with an action that declines — the same defect as a + // mislabelled Approve button, which CLAUDE.md rules out for the same reason. + const command = managedBy === "homebrew" ? "brew upgrade paddock" : "paddock update"; return (
paddock {version} is available — run{" "} - paddock update on the machine running it + {command} on the machine running it