feat(update): one-click macOS auto-updater + channel-aware mcpproxy update CLI (spec 092) - #958
Merged
Conversation
Deploying mcpproxy-docs with
|
| Latest commit: |
663d52c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f83e92eb.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://092-auto-updater.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…idance-only, enclosure-signature wording, RC pipeline parity Related #957 - FR-001a: launch provenance must survive tray restarts (today every pre-existing core reads as external) - FR-002: external-core restart = explicit user consent or instructions - FR-020: self-update only on positively identified tarball channel; unknown stays guidance-only (detector never returns tarball today — positive marker is a prerequisite) - FR-011: enclosure-level signature + bundle-identity codesign policy - FR-014: prerelease pipeline must gain feed/manifest parity for RC channel - FR-015: explicit hot-reloadable update policy contract governing all tray-side checks - FR-017: single-source-of-truth menu state machine (feed owns one-click; legacy check is browser-guidance only) - FR-021/022: defined confirmation point, backup/rollback, symlink/permission semantics; downgrade needs explicit version + --force - FR-006: SemVer 2.0 precedence (rc.10 > rc.2) - SC-001 scoped by ownership/acceptance; FR-030 premise corrected
…(unknown guidance-only; downgrade needs explicit version + --force) Related #957
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 31258349320 --repo smart-mcp-proxy/mcpproxy-go
|
Related #957 The tray already stamps MCPPROXY_LAUNCHED_BY=tray on every core it spawns (CoreProcessManager.swift, cmd/mcpproxy-tray), and the macOS PKG postinstall stamps installer — but the core never reported it back, so ownership lived only in the launching tray's memory. A newer tray attaching to a core started by an older tray classified it as external and could not supersede it: exactly the #957 upgrade scenario (FR-001a). ## Changes - internal/launch: capture MCPPROXY_LAUNCHED_BY once at process start and normalize it to "tray" / "installer" / "" (strict — an unrecognized marker never authorizes killing a process) - GET /api/v1/info always carries launched_by (contracts.InfoResponse, oas, generated frontend contracts, docs/api/rest-api.md) - mcpproxy status renders "Launched by:" when the core asserted a marker and stays quiet otherwise; JSON keeps it omitempty ## Testing - go test -race ./internal/launch/... ./internal/httpapi/... ./cmd/mcpproxy/... - golangci-lint v2 clean on the touched packages
…able Related #957 ChannelTarball existed since Spec 079 but detect() could never return it: the release matrix stamped no build marker and no heuristic produces "tarball". A positively identified tarball install is the precondition for `mcpproxy update` to ever self-replace a binary (FR-020), so without it the CLI self-update path is unreachable by construction. ## Changes - release.yml builds a SEPARATE core binary for the .tar.gz/.zip archive, stamped with updatecheck.buildChannel=tarball, signed alongside the others on macOS, and verified post-build via `go version -m`. The shared matrix binary stays unstamped so the DMG/PKG bundle and the .deb/.rpm packages keep detecting their own channel. - The tarball marker is deliberately WEAK in detect(): Homebrew installs the very same .tar.gz into its Cellar, so positive heuristics (brew prefix, container, package-manager path, .app bundle) override it and it only applies where detection would otherwise answer "unknown". Every other marker keeps absolute precedence. - UpdateCommand(tarball) / PrereleaseUpdateCommand(tarball) => "mcpproxy update" (FR-024 for the tarball channel only; DMG guidance is unchanged until the tray updater ships). - prerelease.yml documents why RC archives stay unstamped for now: self-update needs checksums.txt + cosign bundle, which that pipeline does not publish yet (FR-014). - docs/features/version-updates.md: weak-marker rule and the updated command matrix. ## Testing - go test -race ./internal/updatecheck/... (incl. a precedence table proving a stamped binary under a brew prefix / in a container / in an .app bundle / under dpkg ownership never resolves to tarball) - golangci-lint v2 clean; both workflows re-parsed as YAML
Related #957 `mcpproxy update` branches on the already-detected install channel (Spec 092 US3/FR-020..FR-023): package-manager installs get the exact upgrade command, Docker/Windows-installer get guidance, a macOS app bundle is delegated to the tray, and only a positively identified tarball install (or `unknown` plus an explicit `--self` assertion) is ever self-replaced. ## Changes - cmd/mcpproxy/update_cmd.go: the command, its decision table and the -o json/yaml report. --check reports current/latest/channel with no side effects and is also the output when nothing is newer. A development build is reported as such instead of "already up to date". - cmd/mcpproxy/update_apply.go: checksum manifest parsing, sha256 verification, archive extraction, and the atomic swap — stage inside the target directory, preserve mode, rename target -> .old, rename new -> target, run `<new> --version` and only then drop the backup; restore on any failure. Symlinked launchers have their destination replaced, never the symlink. - Refusals: app-bundle and staged-copy paths (checked independently of GOOS), non-writable targets (error names path + owner, never suggests sudo), and a downgrade without both --version and --force. - internal/updatecheck: GetReleaseByTag + an overridable API base so an exact tag can be requested and tests can run against httptest. ## Signature verification — deliberate compromise (flagged) FR-021 asks for offline signature verification. The sha256 check against the release's checksums.txt is unconditional, and the cosign bundle over checksums.txt is verified with the identity pinned to this repo's release workflow — but via a locally installed cosign binary, not in-process. sigstore-go v1.3.0 would add 72 linked modules (+16 MB to a hello-world binary) and force a go.mod Go-directive bump; the decision report lists that dependency as an open maintainer decision. Missing cosign therefore ABORTS the update by default; --allow-unverified-signature is the explicit, loudly warned opt-out. ## Testing - go test -race ./cmd/mcpproxy/... ./internal/updatecheck/... — 130 test cases across the channel decision table, the downgrade/force matrix, bundle-path refusal, non-writable targets, and a real download->verify->swap against an httptest release server (happy path, tampered artifact, verify-failure restore, missing/failing signature, unlisted artifact) - golangci-lint v2 (.github/.golangci.yml) on ./... — 0 issues - go build -tags server ./cmd/mcpproxy + go test -tags server ./internal/serveredition/... -race
Related #957 The step runs under `bash -eo pipefail`, where `go version -m … | grep -q …` can report the pipeline as failed when grep exits on the first match and go takes SIGPIPE — turning the guard into a flaky release blocker. Capture the build settings into a variable and match with `case` instead.
Related #957 FR-002 needs a stop mechanism for a core the tray only ATTACHED to: it holds no Process handle for one, and the core exposes no shutdown endpoint, so without a pid the consent action could only ever print instructions. Paired with the launched_by provenance added in e186f57 this is what lets a newer tray actually supersede a core an older tray started. ## Changes - contracts.InfoResponse.PID + the `pid` key in the handler payload, indirected through `var pidFn = os.Getpid` as the test seam - regenerated frontend/src/types/contracts.ts (generator edited, not the output), oas/swagger.yaml + oas/docs.go via `make swagger` - docs/api/rest-api.md field table and example ## Testing - go test -race ./internal/httpapi/... ./internal/contracts/... — ok - golangci-lint v2 (.github/.golangci.yml) — 0 issues
Related #957 FR-006. The tray's only version comparison sorted prerelease identifiers as whole strings, so 1.0.0-rc.10 ranked BELOW 1.0.0-rc.2. On the update path that offers an RC user a downgrade as an "update"; the supersede logic Phase 0 builds on top of it would have killed a newer core to start an older one. Extracted into a shared type so both callers get the same ordering and it has tests of its own. ## Changes - new `SemanticVersion`: SemVer 2.0 parse + §11 precedence (numeric identifiers numerically, numeric < alphanumeric, prerelease < release, larger identifier set wins), tolerating a leading "v" and a two-part core, discarding build metadata per §10 - malformed input returns nil — "no decision" — instead of 0; only the update-nudge adapter (`UpdateService.compareSemver`) collapses that to 0, where "not greater" already means "no update" ## Testing - swift test --filter SemanticVersionTests: 13 tests, 0 failures
Related #957 FR-001/FR-001a/FR-002/FR-005. After an upgrade the new tray finds the OLD core still running: the previous tray spawned it, that tray is gone, and nothing ever stopped it. The tray attached and served the old version indefinitely — the reported bug. Ownership was in-memory in the launching tray, so every pre-existing core classified as external and could never be superseded. The core now reports durable provenance (launched_by) and its pid, which is what makes the decision possible at all. ## Changes - `CoreSupersede.decide`: pure verdict — restartManaged (we hold the Process), stopAndRespawn (tray provenance + usable pid), askForConsent (user-launched, or tray-launched with no pid), or none with a logged reason. `installer` counts as tray provenance because launchCore deliberately preserves that marker on the core it spawns. - `CoreProcessIdentity`: re-checks that a pid is an mcpproxy process immediately before signalling it — pids are recycled, and the core may have died between the info read and the signal. - `BundledCore.respawnVersion`: asks the bundled binary (`version -o json`, a pure print — no config, no BBolt lock), falling back to CFBundleShortVersionString. Nil for dev builds and for a MCPPROXY_CORE_PATH override, where a restart would re-launch the same binary in a circle. - `CoreProcessManager` evaluates after every connect (attach, launch, reconnect) — the only points a version report arrives — and publishes `AppState.staleCorePrompt` for the consent case. One attempt per session; shutdown retracts the offer. - InfoResponse gains optional launched_by/pid (optional because a pre-092 core omits them, and an old core is the whole subject). ## Testing - swift test --filter CoreSupersede…|CoreProcessIdentity…|BundledCore…: 21 tests, 0 failures - swift test --filter CoreSupersedeAttachTests: 8 tests, 0 failures (real attach path against a Unix-socket stub core) - swift test --filter ModelsTests: 97 tests, 0 failures
Related #957 FR-003 and the menu half of FR-002. A drag-install replaces /Applications/MCPProxy.app underneath the running process: macOS neither notifies nor restarts it, so the old code — and the old core it manages — keeps running until the user works out that they have to quit it. ## Changes - `BundleUpdateWatcher` reads CFBundleShortVersionString from the Info.plist ON DISK on every check. Not `Bundle(path:)`: Foundation caches bundles by path and would keep answering with the pre-upgrade dictionary, which is exactly the state being detected. Offers only on a strictly newer, SemVer-comparable version — equal, older and unparseable all stay silent. - Checked at launch, on every `applicationDidBecomeActive`, and on a 5-minute timer. - Menu: "MCPProxy was updated to vY — Relaunch" (stops the core, `open -n` the new bundle, then terminates — `-n` because plain `open` activates this stale process, the reported symptom) and "Old core vX running — Restart into vY" (FR-002 consent; with no pid to act on it presents instructions instead of failing silently). ## Testing - swift test --filter BundleUpdateWatcherTests: 7 tests, 0 failures (incl. rewriting a fixture bundle's plist mid-test) - swift test --filter SupersedeMenuTests: 6 tests, 0 failures (real controller + real rebuildMenu through the existing menu-host seam)
…e new one Related #957 FR-004. `open -a` ACTIVATES a running instance instead of starting one, so the postinstall step handed the user the OLD, just-overwritten app — still serving from the old core — and called the upgrade done. Quitting first is what makes the launch a launch. ## Changes - politeness ladder: `osascript … to quit` by bundle id, 5s wait, SIGTERM (3s), SIGKILL. The app routes SIGTERM through its normal quit path, so even the fallback stops the managed core. - the quit runs through the same `launchctl asuser … env -i` wrapper as the launch (factored into `run_as_user`) — an AppleScript sent from root's context cannot reach the user's app. - pkill matches `MCPProxy.app/Contents/MacOS/MCPProxy`, not the bare name: a looser pattern also matches the core (whose orderly shutdown belongs to the tray) and this script's own command line. - a survivor never fails the install; the new tray's stale-core supersede handles it. ## Testing - bash -n and shellcheck: clean - verified the pgrep/pkill pattern against a fixture process living at .../MCPProxy.app/Contents/MacOS/MCPProxy — matched, and cleared
Related #957 FR-030, with the analysis it asks for recorded in the file header. Findings: nothing in THIS tray resolves the staged copy at ~/Library/Application Support/mcpproxy/bin/mcpproxy ahead of the bundled core — resolveBinary() checks the bundle first, so that branch is reachable only for a build with no bundled core, where there is nothing to shadow. The legacy Go tray does prefer it, but re-stages it from its own bundle, so it tracks. The /usr/local/bin symlink points at the bundled binary. A user's PATH is theirs. The residual hazard is the file itself: a stale executable anything can still run. So it is REFRESHED, never removed, and only when provable — regular file (a symlink is deliberate wiring), answers `version -o json` so it really is an mcpproxy binary, and that version is strictly older by SemVer. Anything unprovable is a logged no-op. ## Changes - `StagedCoreBinary.decide` (pure) + `refreshIfStale` (copy to a sibling temp, preserve the existing mode, rename over the target — a legacy tray may be executing that image, and a rename leaves it on its own inode) - called detached at startup next to the symlink setup - FR-030 note on resolveBinary()'s step 3 explaining the ordering ## Testing - swift test --filter StagedCoreBinary: 13 tests, 0 failures — decision table (8) plus real file-system cases (5): stale copy swapped with its mode preserved and no temp left behind, current copy untouched, unidentifiable file neither refreshed nor deleted, absent path not created, symlink left pointing where it pointed
Related #957 FR-002. The automatic branch tore the connection down and only then discovered it could not identify the pid — leaving the user with an error and no core, where a moment earlier they had a working (if old) one. The unidentifiable-pid case is not a failure, it is the "no safe stop mechanism" case the requirement answers with instructions. ## Changes - check the pid's identity BEFORE dropping the connection; on refusal downgrade the offer to the pid-less instructions prompt and leave the connection alone. `stopCore` keeps its own re-check immediately before the signal (pids are recycled). ## Testing - swift test --filter CoreSupersede: 21 tests, 0 failures (the refusal test now asserts .connected + a pid-less prompt)
Related #957 FR-015 asks for the update policy to be an *explicit* contract the tray can read, "not inferred from missing data". Today the only signal is the presence of the `update` object — and `Checker.GetVersionInfo()` returns nil BOTH when checking is disabled and when no check has produced a result yet. A tray that must decide "may I run a Sparkle feed check at all?" cannot tell those apart, so it either nudges after the operator disabled updates or stays silent when it should not. ## Changes - `internal/updatecheck/policy.go`: `Policy{enabled, channel, nudges_suppressed}` plus `Checker.Policy()` (computed live, so a `SetConfig` hot-reload and both environment overrides are visible on the very next read) and `UnavailablePolicy()` for runtimes constructed without a checker. - `update_policy` in `GET /api/v1/info`, always present with all three fields (no omitempty — an all-zero policy is exactly the disabled case that must not vanish). Wired `Runtime.UpdatePolicy` → `Server.UpdatePolicy` → `httpapi.ServerController`. - `internal/contracts`, `cmd/generate-types` (+ regenerated `frontend/src/types/contracts.ts`), `make swagger`, `docs/api/rest-api.md`. `enabled` governs AUTOMATIC checks only; a user-initiated "Check for Updates" stays available, which is what FR-015 requires. ## Testing - `internal/updatecheck/policy_test.go`: config/env precedence matrix, CI nudge suppression, unavailable-checker policy (CI pinned via t.Setenv). - `internal/httpapi/info_update_policy_test.go`: field always present for four policies, including the all-zero one. - go test -race ./internal/updatecheck/... ./internal/httpapi/... ./internal/contracts/... → ok - golangci-lint v2 → 0 issues; server edition build + tests ok.
Related #957 Sparkle 2.9.3 has been declared in Package.swift, dynamically linked, bundled into Contents/Frameworks and code-signed by both build scripts since Spec 037 — and never imported by a single Swift file. `UpdateService.checkWithSparkle()` was a `NSClassFromString` stub that fell straight through to a raw GitHub API call. FIRST, the one unproven assumption in the decision report: `import Sparkle` DOES resolve under plain `swift build` with no Xcode project. Verified by compiling `#if !canImport(Sparkle) #error(...) #endif` against the package — the error does not fire, so the whole framework path below is really compiled, not silently `#if`-ed away. ## Changes - `Services/FeedUpdater.swift` — `FeedUpdating` / `FeedUpdaterObserver` protocols plus the real `SparkleFeedUpdater`: * programmatic `SPUStandardUpdaterController(startingUpdater: false, …)` and an explicit `try updater.start()`. Deliberately NOT `startingUpdater: true`: that variant logs a misconfiguration and puts a "contact the developer" alert in front of the user a few seconds later — which is exactly what every build shipped before the CI appcast job would do, since Info.plist still carries `SPARKLE_PUBLIC_KEY_PLACEHOLDER`. A start failure now degrades to the browser path with a reason (FR-016/FR-017); * gentle reminders (FR-010): `supportsGentleScheduledUpdateReminders = true`, `standardUserDriverShouldHandleShowingScheduledUpdate` → false so a scheduled check never throws a window at a menu-bar user, and `standardUserDriverWillHandleShowingUpdate` feeds the published state; * `allowedChannelsForUpdater:` from the policy — empty set (Sparkle's "default channel only") for stable, `["beta"]` for RC (FR-014); * FR-012: the core is stopped in `updater(_:willInstallUpdate:)` — "called immediately before installing", i.e. the last moment the OLD bundle is still on disk. `shouldPostponeRelaunchForUpdate:` and `updaterWillRelaunchApplication:` both run AFTER the swap; the latter is kept as an idempotent second stop, the former is not used. - `Services/ManagedCoreStop.swift` — the synchronous SIGTERM → bounded wait → SIGKILL ladder. Sparkle's pre-install hooks are synchronous main-thread callbacks, and `CoreProcessManager.shutdown()` hops to the main actor, so blocking on it would deadlock rather than stop anything. Phase 0's `CoreProcessIdentity` re-check still gates the signal. - `Services/UpdatePolicy.swift` — FR-015 resolution: tray `MCPPROXY_DISABLE_AUTO_UPDATE`, CI suppression, and the core's new `update_policy`. A user-initiated "Check for Updates" bypasses all of it. - `Services/UpdateInstallability.swift` — FR-016: translocation, read-only volume and unwritable parent, each with an explanation AND a fallback. - `Services/UpdateMenuState.swift` — FR-017: one owner. Feed offer wins for the same or lower legacy version; equal versions dedupe; a legacy version the feed does not carry renders as browser guidance, never as a one-click action; a blocked app never gets a one-click item at all. - `UpdateService.swift` rewritten around those pieces; `MCPProxyApp.swift` renders `updateService.menuEntries`, installs the pre-update core stop, and routes the launch/hourly checks through the policy-gated entry point (they used to call the user-initiated one). - `API/Models.swift`, `Core/CoreProcessManager.swift`, `State/AppState.swift` — carry `update_policy` from `/api/v1/info` to the service on every connect. - `scripts/build-swift-app.sh` — stamp `SUPublicEDKey` / `SUFeedURL` from `SPARKLE_PUBLIC_ED_KEY` / `SPARKLE_FEED_URL` when present; without them the bundle keeps the placeholder and one-click updates stay off, which is the correct failure direction. ## Testing - `swift build` → Build complete (only the two pre-existing warnings). - `cd native/macos/MCPProxy && swift test` → 911 tests, 1 failure — the known environmental `AppLifecycleTests.testTheSharedJournalNeverWritesToTheRealInstanceRootUnderTests` (this machine's live tray owns ~/.mcpproxy/tray-lifecycle.jsonl). - New: UpdatePolicyTests 13 · UpdateMenuStateTests 14 · UpdateInstallabilityTests 6 · ManagedCoreStopTests 8 · UpdateServiceFeedTests 16 — 0 failures. The last suite drives the REAL `AppController.rebuildMenu()`, so a renamed menu slot fails there. - Delegate spelling verified against the Sparkle headers AND by the compiler: deliberately misspelling one method produces "nearly matches optional requirement"; the committed code produces zero such warnings. ## NOTES - FR-018 (Homebrew cask `auto_updates true`) lives in the tap repo (smart-mcp-proxy/homebrew-mcpproxy) and is out of scope for this repository. It must be set before the first Sparkle-capable release, or `brew upgrade` will fight the in-app updater. - No live upgrade QA: a real one-click run needs a notarized older build and a published appcast, neither of which exists yet.
Related #957 A Sparkle appcast has no architecture selector, and the release pipeline builds one app bundle per architecture — each carrying a per-arch core in Contents/Resources/bin — both reporting the same CFBundleShortVersionString. A single merged feed would therefore hand an Intel user the Apple-Silicon build, and the failure would arrive as a crash after a "successful" update. Until a universal enclosure exists (open decision #4 in the decision report), the pipeline publishes one feed per architecture and the tray asks for its own. ## Changes - `Services/SparkleFeedURL.swift` — pure rewrite of the configured `SUFeedURL`: only the exact default file name `appcast.xml` becomes `appcast-<arch>.xml`. Any other name is used verbatim, so an operator already serving a merged or universal feed is not second-guessed. - `SparkleFeedUpdater.feedURLString(for:)` applies it, returning nil (= "use Info.plist") when the rewrite is a no-op. - The file names are half of a contract with `.github/workflows/release.yml`, which generates exactly those names; the test file says so. ## Testing - `SparkleFeedURLTests` — 5 tests, 0 failures (default rewrite per arch, nested paths, operator-supplied names left alone, query strings preserved). - `swift build` → Build complete, no "nearly matches optional requirement" warning, i.e. the delegate method really binds.
Related #957 FR-013/FR-014: the one-click updater needs two things this pipeline never produced — a notarized, stapled, symlink-preserving archive of the .app, and a signed update feed pointing at it. (The existing macOS assets do not qualify: the bare DMG is signed but NOT notarized, and only the PKG-wrapping `-installer.dmg` is notarized + stapled. Neither is an .app archive.) ## release.yml (stable) - **Build Sparkle enclosure (macOS)**, inserted AFTER the existing notarization step and before keychain cleanup. The signing step order is untouched — the bundle was already signed nested-first; this step only archives, notarizes, staples and verifies it. * `ditto -c -k --sequesterRsrc --keepParent` — the only archiver Apple documents for this. `zip` drops symlinks and xattrs, which breaks the bundle's signature seal and surfaces on the user's machine as `Killed: 9`. * The .app is notarized and stapled SEPARATELY from the PKG: a stapled ticket lives inside the bundle it was stapled to, and the copy extracted from this zip is not the copy inside the PKG. Unstapled, the first launch after every update would wait on Apple — or fail offline. * Post-condition proven, not assumed: the archive is extracted again and run through `codesign --verify --deep --strict`, `stapler validate` and `spctl`. * Named `*.app.zip`, so the release job's existing "copy archives" glob picks it up and it lands in `checksums.txt` with everything else. - **sparkle-appcast** job, `needs: [release]` — deliberately after publication, because `generate_appcast` bakes absolute download URLs into the feed and the first client to read it would otherwise 404. Runs Sparkle 2.9.3's `generate_appcast` (version pinned to Package.resolved) with the private key on **stdin** (`--ed-key-file -`), so it never touches disk or `ps`. Fails the job if a generated feed lacks `sparkle:edSignature`. - ONE FEED PER ARCHITECTURE (`appcast-arm64.xml` / `appcast-amd64.xml`): an appcast has no arch selector and both bundles carry the same version, so a merged feed would offer Intel users the arm64 build. The tray-side half of this contract is `Services/SparkleFeedURL.swift`. - `SPARKLE_PUBLIC_ED_KEY` / `SPARKLE_FEED_URL` passed to the Swift app build so `scripts/build-swift-app.sh` can stamp `SUPublicEDKey` / `SUFeedURL`. ## prerelease.yml (RC) Same enclosure step and the same appcast job with `--channel beta`, gated on a TAG (this workflow also runs on `next` pushes, which have no release to attach a feed to). The job asserts the `<sparkle:channel>beta</sparkle:channel>` tag is present — that tag is the only thing keeping stable users from being offered an RC, since Sparkle offers a tagged item only to clients that request the channel. ## Fork / no-key behaviour Every new step no-ops with a `::notice::` when `SPARKLE_ED_PRIVATE_KEY` is absent, and both artifact uploads use `if-no-files-found: ignore`. A build without `SPARKLE_ED_PUBLIC_KEY` keeps `SPARKLE_PUBLIC_KEY_PLACEHOLDER`, which makes the updater refuse to start — no key, no one-click, never an unverified install. ## Testing - `python3 -c "import yaml; yaml.safe_load(...)"` on both files → parse. - Every new `run:` block extracted and run through `bash -n` and `shellcheck -S warning` → clean. - `actionlint` on both files reports nothing new (the pre-existing `matrix.edition` notices come from the commented-out server matrix rows). - The workflows themselves cannot be executed here; the first real tag is the first true test. Riskiest untested lines: the `notarytool` submission of the .app zip and the `generate_appcast` invocation. ## NOTES - FR-018 (`auto_updates true` on the Homebrew cask) lives in the tap repo, smart-mcp-proxy/homebrew-mcpproxy — out of scope here, and required before the first Sparkle-capable release. - TODO (maintainer decision #1): the shipped Info.plist points at https://mcpproxy.app/appcast.xml, served by the WEBSITE repo, which this pipeline cannot publish to. Feeds are attached to the release and exported as the `sparkle-appcast` / `sparkle-appcast-beta` artifacts for it to consume. - STILL MISSING from FR-014: the RC pipeline publishes no checksums.txt and no cosign bundle, so RC tarballs stay guidance-only for `mcpproxy update`.
Related #957 New `docs/features/auto-update.md`: the per-channel behaviour matrix (CLI and tray side by side), the one-click flow and its two independent verifications, why the core is stopped before the swap rather than after, what happens when an in-place update is impossible, the FR-017 "one item, one owner" rule, every kill switch, the `update_policy` contract, how the RC/beta channel keeps stable users away from prereleases, and the release infrastructure (enclosure, per-arch feeds, the three signing key/variable names, and the fork/no-key behaviour). Linked from `docs/configuration.md`'s env-var table — which also gains the `CI` row that was previously only documented for the core — and from `docs/features/version-updates.md`, which covers *noticing* an update rather than applying one. The unresolved feed-hosting decision is called out in an admonition rather than buried: the shipped Info.plist points at https://mcpproxy.app/appcast.xml, which only the website repo can serve, and the interim GitHub `releases/latest/download/...` URL works for stable but structurally cannot serve the beta channel.
Related #957 Two follow-ups to the auto-update doc: - `website/sidebars.js` is an EXPLICIT sidebar, not filesystem-generated, so a new page under docs/features/ is built and routed but invisible in navigation until it is listed. Added next to Version Updates. - `docs/configuration/environment-variables.md` is a second env-var surface (alongside the table in `docs/configuration.md`); its Auto-Update section never mentioned the CI rule and had no pointer to the updater itself. Both surfaces now say the same thing and link to the same page.
Related #957 Both new `sparkle-appcast` jobs deliberately skip `actions/checkout` — they only need the enclosure artifacts. But `gh` resolves the target repository from the current directory's git remote, and it does NOT read `GITHUB_REPOSITORY`; the only env var it consults is `GH_REPO`. So the final `gh release upload "${GITHUB_REF_NAME}" appcast-out/*.xml` would have failed with "could not determine what repo to use" on the first real tag — after generating and signing the feeds, so the failure would land at the very last step of a release. Every other `gh release upload` in both workflows lives in a job that ran `actions/checkout`, which is why this is the only pair that needed it. ## Changes - `GH_REPO: ${{ github.repository }}` in the appcast step's env, in release.yml and prerelease.yml, with a note explaining why a checkout is not the fix. ## Testing - Both workflows re-parse with `yaml.safe_load`; asserted the resolved env block of `jobs.sparkle-appcast` now carries GH_REPO in each file.
Related #957 FR-020 allows `--self` to turn the guidance-only `unknown` channel into a self-update, but attaches "with a clear warning" to that override — and the override shipped without one. The requirement is not cosmetic: `unknown` is precisely the set of installs mcpproxy could NOT identify, which includes AUR, MacPorts and Nix-like layouts that are writable and still owned by a package manager whose bookkeeping the swap silently invalidates. A user who types --self after reading the guidance line deserves to be told what they just claimed. A positively identified tarball install stays silent — there is nothing uncertain about it to warn over — and the warning goes to stderr so `-o json` stays machine-parseable. ## Changes - `updateRunner.run`: emit the warning on the self-update branch only when the effective channel is `unknown`. ## Testing - New `TestUpdateCommand_SelfOverrideWarnsOnUnknownChannel`: 3 cases (unknown+--self warns, tarball silent, tarball+--self silent), each also asserting the self-update branch really ran and that stdout's JSON report never contains the warning. - `go test -race ./cmd/mcpproxy/... ./internal/updatecheck/... ./internal/httpapi/...` → 3/3 packages ok.
… plist Related #957 Two leftovers from the tarball channel becoming command-carrying (FR-020). `UpdateBanner.vue` still listed `tarball` among the channels with no safe update command, in both template comments and the guidance switch. The rendered output was already correct — every branch keys off the presence of `update_command`, never the channel name — so this is comment-only and changes no markup, no class, and no computed result. `build-macos-tray.sh`'s from-scratch Info.plist (the branch that runs only when the source Info.plist is missing) emitted `SUFeedURL` with no `SUPublicEDKey`. A feed URL with no pinned key is the one bundle shape FR-011 must never produce; the checked-in plist and `build-swift-app.sh` both carry the placeholder, and this branch should not be the one that diverges. ## Changes - `frontend/src/components/UpdateBanner.vue`: comments only. - `native/macos/MCPProxy/scripts/build-macos-tray.sh`: add the `SUPublicEDKey` placeholder to the fallback plist. ## Testing - `bash -n` + `shellcheck -S warning` on build-macos-tray.sh: clean apart from the pre-existing SC2155 at line 25. - Scanned every HTML comment in UpdateBanner.vue for an embedded `--` (which would break the comment) — none.
Member
Author
Implementation pushed — Phases 0–2 implemented20 commits on top of the spec. Phase 0 — the #957 fix (ships regardless of Sparkle keys)
Phase 1 — Sparkle one-click
Phase 2 —
Local verification
Blocked on maintainer (documented in spec/report): generate Sparkle EdDSA keys (secrets), appcast hosting decision (feed URL), Homebrew cask Related #957 🤖 Generated with Claude Code |
… check exact Related #957 Two defects in the mechanical half of `mcpproxy update --self` (FR-021). The pair of renames that swaps the binary is not atomic, and the retry path made an interrupted swap unrecoverable: it removed a leftover `<target>.old` before ever looking at the target, so a crash in the window where the target path is empty and `.old` holds the only copy of the binary was turned into a destroyed install by the next attempt. The post-swap probe matched the reported version as a substring, so a binary announcing 0.54.10 satisfied a request for 0.54.1 — the wrong release would have passed verification and had its predecessor deleted. ## Changes - `restoreInterruptedSwap` runs first: target missing + backup present moves the backup back; target missing with no backup is a refusal, not a silent install over nothing. A leftover backup is only removed once the target is known to be present. - `reportsVersion` compares whole whitespace-delimited tokens (tolerating a `v` prefix and surrounding punctuation) instead of a substring. ## Testing - go test ./cmd/mcpproxy/ — interrupted-swap recovery, interrupted swap with no staged binary, missing target and backup, 0.54.10 vs 0.54.1
Related #957 Two gaps in the comparison FR-006 makes every supersede and update decision depend on. Leading zeros in a numeric prerelease identifier were accepted, so "rc.01" and "rc.1" were two spellings of one version and one of the two orderings they produced had to be wrong. §9 forbids them; FR-006 says a malformed version is a no-decision, not a guess. Numeric identifiers were coerced through `Int`, which returns nil past Int64. Two identifiers that both overflowed fell through to the alphanumeric branch and were compared as ASCII, ordering "10000000000000000000000" below "9999999999999999999999" — a downgrade offered as an update. ## Changes - Prerelease identifiers are validated with `numericMayHaveLeadingZeros: false`; build metadata keeps the §10 exemption - `compareNumericIdentifiers` compares digit strings by length then lexicographically, with no upper bound ## Testing - swift test — leading-zero rejection, a lone "0" still valid, build metadata unaffected, 22- and 23-digit identifiers both directions
Related #957 FR-017 allows one update item at a time. When the feed offered a version and the legacy GitHub check advertised a NEWER one, the resolver rendered both: a one-click item and a browser item, side by side, asking the user to choose between an install the tray can perform and a download it cannot verify. The feed now owns the slot whenever it has an offer, whatever the legacy check found. What the feed offers is real and installable in one click, and the next scheduled check picks up the newer version. The legacy result is only rendered when the feed has nothing. ## Changes - Collapse the two-source switch to "feed if present, else legacy guidance" - Drop the now-unreachable comparison branches ## Testing - swift test — the full feed × legacy × blocked matrix asserts at most one offer and that the feed owns it
…confirmed Related #957 Five interlocking defects in the tray half of the updater. They share one shape: something was assumed — a policy, a feed name, a dead process — where nothing had been established. **The launch check ran before the policy arrived.** `/api/v1/info` carries the core version and the update policy, and Combine delivers `@Published` subscribers from `willSet` — so assigning `version` first ran the launch check under the previous (permissive-by-default) policy, checking for updates for a user who had switched them off. The default is now restrictive and the two assignments are ordered. A core that predates 092 is a different case and stays permissive: the tray stamps `CoreUpdatePolicy.legacyDefault` at the point it has actually talked to one, which is what keeps "an old core said nothing" apart from "we have not asked yet" (FR-015). **RC clients read the stable feed.** `prerelease.yml` publishes `appcast-beta-<arch>.xml`; the rewrite only ever produced `appcast-<arch>.xml`. An RC user was pointed at a feed that by FR-014's design never carries an RC, so the beta channel could not offer anything at all. **SIGKILL was reported as success.** `ManagedCoreStop` returned `.killed` the instant it sent the signal. Sparkle used that answer to decide the bundle could be replaced, so a core that outlived SIGKILL kept serving from a deleted inode — issue #957 exactly. The stop now confirms the exit, and a stop that ends in a question mark POSTPONES the installation through `shouldPostponeRelaunchForUpdate:untilInvokingBlock:` (verified against Sparkle 2.9.3: it runs at the top of `installWithToolAndRelaunch:`, before the installer is contacted) rather than being logged and ignored. **SIGKILL trusted a five-second-old identity check.** Both kill ladders proved the pid was an mcpproxy process, waited out the SIGTERM grace period, then killed whatever held the pid by then. The identity is re-proven immediately before the signal. **One click was two clicks with a download in between.** `automatically- DownloadsUpdates` now follows the same switch as scheduled checks, so the bytes are verified and on disk before the menu item is touched. The residual Sparkle confirmation cannot be removed through the public API — `SPUUpdater` has no "install what you are holding" method — so the honest count is one click on our item plus one on Sparkle's install prompt, and that is written down where the next reader will look. ## Changes - `EffectiveUpdatePolicy.awaitingCore`; `CoreUpdatePolicy.legacyDefault` - `coreUpdatePolicy` assigned before `version` in `connectToCore` - `SparkleFeedURL.archSpecific(_:arch:channel:)` + `fileName(channel:arch:)` - `ManagedCoreStopOutcome.failed` and `.coreIsDown`; bounded post-SIGKILL wait - `feedUpdaterWillInstallUpdate()` returns whether the core is down; Sparkle postpones when it is not - Identity re-check before SIGKILL in `ManagedCoreStop` and `stopCore` - `automaticallyDownloadsUpdates`, `updateIsReadyToInstall` ## Testing - swift test — 87 tests over the policy precedence, both channels × both arches, the post-SIGKILL confirmation, pid recycling across the grace period, and the install veto
Related #957 postinstall runs as root, where `pgrep -f` and `pkill -f` match every user's processes. Installing an upgrade for one account therefore quit — and, two rungs down the ladder, SIGKILLed — the MCPProxy tray of every logged-in user on the machine, including sessions nobody was upgrading. ## Changes - Resolve `-U <console uid>` once (the same uid `run_as_user` already hops to) and pass it to every pgrep/pkill; empty when there is no real user, which is the CI-imaging case where there is nobody else's session to protect - A plain string rather than an array: macOS bash 3.2 errors on an empty array expansion under `set -u` ## Testing - bash -n packaging/macos/postinstall.sh
Related #957 FR-014 requires RC builds to be covered by a signed checksum manifest, and the prerelease pipeline published neither checksums.txt nor a cosign bundle. That made every RC uninstallable by `mcpproxy update`: the command refuses an artifact it cannot verify, so a user who opted into the prerelease channel had no update path at all — the CLI would find the RC and then decline it. ## Changes - `Generate checksums` in the prerelease `release` job, mirroring release.yml including the self-reference guard - `Sign checksums (keyless)` + bundle upload, guarded on an OIDC token being available so a fork run publishes unsigned rather than failing - `id-token: write` on the job (release.yml grants it workflow-wide) - `cosignIdentityRegexp` accepts prerelease.yml as well as release.yml; both are pinned to `@refs/tags/v`, so this widens the trusted set by nothing a tag push could not already do - Corrected the two comments that described the missing artifacts as a standing gap ## Testing - python3 -c 'yaml.safe_load(...)' on both workflow files - go build ./... ; go test ./cmd/mcpproxy/...
Related #957 The feed URL baked into every bundle (https://mcpproxy.app/appcast.xml) is served by nobody: the release pipeline attaches the feeds to the GitHub release and exports them as workflow artifacts, and the website repository would have to publish them. That is a maintainer decision, not something this branch can make — but it was recorded only as a warning box halfway down the page, which is not enough for someone trying to work out why nobody gets one-click updates. An explicit activation checklist replaces it, naming the exact files the website repo must serve, at the exact URLs the tray derives, and what the serving side must not do to them. The graceful side was already correct and is now pinned by tests: a feed 404 reads as "the feed has nothing" and hands the menu to the browser-download path, rather than surfacing as an error that eats the offer. ## Changes - "Activation checklist" section: five steps, the artifact→URL table, the three serving constraints, and the interim stable-only GitHub URL - Banner at the top pointing at it - Brought the page in line with this round's behaviour changes: honest two-click count, the postpone-on-failed-stop rule, one item always, the per-channel feed table, and the restrictive pre-policy default - Troubleshooting entries for the feed 404 and the postponed install ## Testing - swift test — feed failure falls back to browser guidance, and a failure after a withdrawal leaves no stranded one-click item
Dumbris
changed the title
Related #957 The recovery added in 20caffa closed one crash window and left the other open. A crash AFTER the staged->target rename but BEFORE verification leaves an unverified binary at the target and the last known-good one at .old — and on disk that is indistinguishable from a completed update. Recovery saw a target and returned; the swap then deleted .old, and if that attempt's verification failed, the rollback restored the unverified binary. The known-good one was already gone. Nothing about the file contents can settle it, so the swap now records that it is in progress. A sentinel written next to the target before the first rename and removed only after verification passes makes the two states tell themselves apart, and no path removes the backup while it exists. With the sentinel present, target and backup between them say how far the interrupted swap got: neither -> nothing to recover from; refuse backup only -> crashed between the renames; restore it target only -> crashed before the first rename; the target is original target + backup -> crashed after the second; the target was never verified Only the last needs a judgement, and it is made by running the caller's verification against what is at the target. It proves itself or the known-good binary goes back. With no verifier available, the known-good binary wins. ## Changes - `swapSentinelSuffix` + `writeSwapSentinel` (fsynced content; the directory entry is not, which is the same guarantee the renames already had) - `recoverInterruptedSwap` replaces `restoreInterruptedSwap`, covering all four states plus the no-sentinel case left by a build that predates it - `restoreKnownGood` — os.Rename over the target is what discards the unverified binary - The sentinel is removed on every path the function returns by, so only a crash leaves one ## Testing - go test -race ./cmd/mcpproxy/ — verified interrupted swap cleans up, an unverifiable one is replaced, a retry that cannot install ends on the known-good binary, no verifier prefers the known-good, crash-before-first- rename is a no-op, and the sentinel is present while the new binary is unverified
Related #957 Enabling automaticallyDownloadsUpdates for the pre-download in 6914bb6 also switched Sparkle from SPUScheduledUpdateDriver to SPUAutomaticUpdateDriver — the driver is chosen on that flag alone (SPUUpdater.m:621) — and that driver arms a silent install-on-quit. It hands the update to the external installer tool, which replaces the bundle once this process exits, on a path that never calls installWithToolAndRelaunch: and therefore never reaches the postpone hook that refuses an unconfirmed core stop. The fallback the on-quit path does reach is `void`; it called the stop and dropped the answer. `willInstallUpdateOnQuit:` is NOT a veto, so intercepting it does not fix this. Its own documentation: "In either case Sparkle will always attempt to install the update when the app terminates." Nor does the terminate path make it safe — applicationWillTerminate sends the core a SIGTERM and does not wait, so the installer can still run against a live core. What is provably safe is not arming it: with automaticallyDownloadsUpdates false, SPUAutomaticUpdateDriver is never constructed and every install goes through the postpone hook, which can refuse. The cost is that FR-010's click downloads before it installs — a slower click, in exchange for an install that can always be called off. ## Changes - `EffectiveUpdatePolicy.automaticDownloadsAllowed` — always false, given a name and a test because SparkleFeedUpdater cannot be instantiated outside a real .app bundle. Assigned unconditionally: the flag is backed by a user default Sparkle's own permission prompt can write, so leaving it alone is not the same as it being off - `willInstallUpdateOnQuit:` implemented as a tripwire — it cannot refuse, so it logs and surfaces the situation instead of pretending to have handled it - The two `void` hooks report a failed stop instead of discarding it - Docs: honest click count and why the pre-download is refused ## Testing - swift test — every policy (permissive, awaiting-core, legacy, rc, disabled, CI, kill switch) refuses pre-downloading
…he website Related #957 The docs site's docusaurus include-whitelist serves only configuration/**, development/**, etc.; root-level docs/prerelease-builds.md has no site route, so /prerelease-builds broke the site build (onBrokenLinks: throw).
…lock Related #957 Two concurrent 'mcpproxy update' runs could read each other's mid-swap sentinel state, misclassify it, and delete the only known-good backup. The whole recover-and-swap sequence now runs under an exclusive non-blocking flock (LockFileEx on Windows) on <target>.update-lock; the second invocation fails fast with 'another update is already in progress'. The lock file is never unlinked (unlink+relock races two holders onto different inodes); the kernel drops the lock with the process, so a crash cannot wedge future updates.
Member
Author
Cross-model review status: 5/5 rounds used — 1 finding outstandingCodex reviewed this PR through 5 fix→re-review rounds (2 on the spec, 3 on the implementation; 15 code findings fixed and re-verified, including crash-atomic swap recovery, kill-switch init race, PID-reuse guards, and install-on-quit hardening). The final round left one narrow finding, documented per the review-cap policy:
macOS/Windows are unaffected (no /proc/self/exe rename-following). Awaiting maintainer direction on whether to continue past the round cap. Related #957 🤖 Generated with Claude Code |
Related #957 The per-target lock did not serialize what it was supposed to. Its key was the resolved executable path, and on Linux os.Executable() reads /proc/self/exe, which names the running INODE rather than the path it was launched from. A second `mcpproxy update` that resolves its own path AFTER the first has renamed mcpproxy to mcpproxy.old therefore sees itself as mcpproxy.old: it locks mcpproxy.old.update-lock, contends with nobody, and proceeds to swap mcpproxy.old — moving the first process's only known-good backup aside. Two "exclusive" holders on two different files. macOS and Windows report the launch path and do not alias this way, so the fix is pure path arithmetic with no /proc reads and behaves identically everywhere. Both layers are addressed, because they answer different questions. The LOCK KEY is canonicalized: mcpproxy.old, mcpproxy.updating, mcpproxy.update-lock and the staged .mcpproxy.new-<pid> all resolve to mcpproxy, so however each process spells the path they contend on one file. Mapping an unrelated user file named foo.old onto foo's lock costs nothing worse than two updates taking turns. Deciding which file to REPLACE gets the opposite treatment: it refuses. Two things produce a target called mcpproxy.old — a concurrent update that just renamed the binary aside, where we must touch nothing, and a user who kept a copy under that name and ran it, where rewriting mcpproxy instead would silently update the wrong binary. Nothing on disk reliably separates them and one of the answers is destructive, so neither is guessed at; the error names both readings and says what to do about each. ## Changes - `canonicalUpdateTarget` / `stripUpdateArtifactSuffix`, applied repeatedly (a swap of an aliased path yields names like mcpproxy.old.old) and leaving non-aliased paths untouched rather than silently Clean-ing them - `acquireUpdateLock` keys on the canonical path - `refuseAliasedUpdateTarget` gates `applyNewBinary`, and `selfUpdate` too so a concurrent update is caught before ~90 MB is downloaded on its behalf - `backupSuffix` named, so the suffix list cannot drift from the swap ## Testing - go test -race ./cmd/mcpproxy/ — canonicalization table (incl. directories, double suffixes, staged names, near-misses), x and x.old contending on one lock with no aliased lock file created, and an aliased swap refused with the other update's backup left untouched - go build ./... ; GOOS=linux and GOOS=windows builds of ./cmd/mcpproxy
Member
Author
|
✅ Cross-model review complete — CLEAN. The Linux lock-aliasing finding was fixed in Related #957 🤖 Generated with Claude Code |
Related #957 After uploading the signed appcasts as release assets, release.yml and prerelease.yml dispatch publish-appcast at the website repo (same MARKETING_SITE_DISPATCH_TOKEN pattern as the marketing version bump); its receiving workflow downloads the feeds from the public release, verifies signatures, and commits them into public/ for Cloudflare Pages. Non-blocking and skipped when no appcast was generated; manual backfill stays possible via the site workflow's workflow_dispatch. Website side: smart-mcp-proxy/mcpproxy.app-website#4.
…sal, N+1 canary Related #957 Detection failures degrade to the legacy browser nudge (structurally never 'stuck forever'); install failures are the class to guard. First real test is an rc.1->rc.2 one-click on the beta channel with the full production pipeline before any stable ships; the concurrency release follows within days as the first OTA payload to minimize the exposure window if release N's updater is broken.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Spec for the auto-updater decided in the #957 investigation (Option A of the decision report, included under
docs/research/):postinstall.shquit-before-launch. Ships independently.mcpproxy updateCLI: channel-aware (uv/deno pattern) — package-manager one-liners for brew/deb/rpm, guidance for docker/windows, tray delegation for DMG, cosign-verified atomic self-update for tarball installs only.Related #957
Review notes
Spec only — no code. Key research findings it rests on (adversarially verified): Sparkle 2.9.3 is already SPM-declared, linked, bundled and signed into MCPProxy.app but never imported; Info.plist already points at a nonexistent appcast URL with a placeholder EdDSA key; the bare per-arch DMG is signed but not notarized; #957's root cause is that no upgrade path stops the old processes.
Open maintainer decisions are listed in the report (appcast hosting URL, externally-attached core policy, per-arch vs universal enclosure, CLI verification dependency).
🤖 Generated with Claude Code