feat(app): install every update in-app, including a Homebrew-managed bundle - #3
Merged
Merged
Conversation
added 2 commits
August 28, 2026 20:04
…bundle A brew-installed GitPic used to be sent to `brew upgrade --cask gitpic`, and the app had to quit *before* brew was spawned. So the tap refresh and the whole download happened with an .accessory app's menu-bar icon already gone: no progress, nothing to cancel, and `upgradeBound = 900` as the only bound on an upgrade that wedged. Everyone else got the opposite ordering — download, verify and stage while the window is open, then quit for two renames — which is the better path by every measure that matters here. Worse, the two could disagree about whether there was anything to do. The tap learns about a release by dispatch with a six-hourly cron behind it, and AGENTS.md records that the dispatch token has expired before. In that window `brew upgrade --cask gitpic` prints "Warning: Not upgrading gitpic, the latest version is already installed" and exits **0** — measured. The script logged that as success and reopened the same build, so the user lost their app for nothing and was offered the identical update again on the next check, with only GitPic-update.log to say why. The argument for the fork was real and is not being waved away: replacing a cask-managed bundle behind brew's back leaves its manifest describing a version that is not on disk. Homebrew has a stanza for exactly that and the cask was missing it. `auto_updates true` is the Cask Cookbook's own definition of this case, and it does not mean brew stops managing gitpic — for an auto_updates cask Homebrew reads the version out of the installed Info.plist and compares *that* against the tap (Cask#auto_updates_bundle_outdated?; HOMEBREW_UPGRADE_AUTO_UPDATES_CASKS defaults on) rather than its own receipt. Verified against this machine's real install: installed_app_info_plist resolves to /Applications/GitPic.app/Contents/Info.plist, and brew's own comparator gives bundle-behind-tap → upgrade, equal → nothing, bundle-ahead-of-tap → nothing. That last row is the window after a self-update, and it is why this cannot cause a downgrade. Without the stanza brew compares the receipt, which goes stale the moment the app self-updates, and reinstalls a version already on disk. **Depends on tarnish233/homebrew-tap@90ea12d**, which added that stanza and `uninstall quit: "dev.gitpic.app"`. It is already pushed, and the order was deliberate: an old app against the new cask still works (it only ever calls brew when the bundle really is behind, which is the row that upgrades), whereas this app against a cask without the stanzas leaves brew and the app both trying to own /Applications/GitPic.app. Deleted rather than disabled: Route.homebrew, BrewOwnership/BrewVerdict/fold/ brewOwnership, ToolDiscovery's locateBrewOutcome/brewCaskApp/brewCaskroom/ firstLine, upgradeAndRelaunch and the writeScript that generated the bash and its watchdog, the 立即更新 button and its confirm alert. probeQueue is renamed stagingQueue because staging is now its only user. Net -692 lines. Two side-effects worth naming. Opening the update sheet no longer pays an 8 s login-shell probe plus a 20 s `brew list --cask` per prefix, so 「正在确认升级方式…」 is now nearly always invisible; route() is a pure function of three facts. And nothing produces `.unavailable(retryable: true)` any more — the probe was its only source — which is documented on the case rather than collapsed, since removing it would change nothing that runs. Tests: the twelve rows that covered the fork go with it. Two of their assertions were worth more than the rows and are kept, generalised: every refusal `route` can mint is now checked to be a Chinese sentence with no option flag in it (the old row pinned that on one string, which is where the bug had been), and the "these three facts and nothing else decide the route" claim is asserted structurally. probeAnswerSeparatesAbsenceFromSilence stays — loginShellProbe survives for locateGitpic, so deleting it would strip covered code. QuitPathContractTests keeps its forbidden-selector list anchored as it was; the brew script was the original motive but SelfUpdateInstall's swap script is still generated shell text. Not verified live: selfInstall has never run against a cask-managed bundle, because route() returned .homebrew before reaching it. check-self-update.sh refuses to touch /Applications by design, so it cannot cover this — it needs a real old cask install driven through the UI.
`stage` attached the disk image once, and any non-zero exit became InstallFailure.image(stderr) — which the sheet renders as 「磁盘映像有问题:<detail>」. That is the right sentence for an unreadable download and the wrong one for a kernel that is temporarily out of attach slots, where the update simply fails and the user has nothing to act on. Measured on a GitHub macos-latest runner, in this repository's own CI: seventeen seconds into the Bundle install suite, after several tests had attached and detached fine, every remaining `hdiutil attach` began returning `hdiutil: attach failed - Resource temporarily unavailable` inside 0.07 s and kept doing so for the rest of the run — twenty failures, none of which reached an assertion. The run before it and a re-run with no code change were both green, so nothing about the images had changed; the machine had stopped accepting attaches. A user's Mac can be in that state too. Three attempts a second apart, the same shape detachMount already uses for the mirror-image problem. **A timeout is not retried.** An attach that timed out may have landed anyway — ChildProcess terminates and then SIGKILLs hdiutil, and an attach the kernel has committed to survives that, which is why the detach `defer` is installed before the attach in the first place. Attaching a second time could mount the same image twice, so a timeout still fails on the first try. **stderr is not pattern-matched to decide what is transient.** That spelling is one of several the kernel and hdiutil could produce, and a list of them is a list to get wrong. The attempt count bounds the cost instead: an image that really is unreadable is now refused about two seconds later. That two seconds is also the only way to observe the retry, since `stage` spawns /usr/bin/hdiutil by absolute path and a transient failure cannot be injected. A file that is not an image fails the same way — non-zero, fast — so refusesGarbage now asserts the refusal took at least two seconds. A lower bound on elapsed time is deterministic in the safe direction: Thread.sleep can overrun, never undershoot. Verified both ways — 2.301 s with the retry, and with the loop cut to one attempt the test fails at 1.14 s, which is the check working. The one test that mounts an image itself rather than through `stage` (undoDetachesTheMount) gets the same tolerance via attachWithRetries, for the same reason and with the same no-retry-on-timeout rule.
Owner
Author
|
补一个提交 起因是这个 PR 的 CI 里 但它暴露的产品问题是真的:
|
added 2 commits
August 28, 2026 20:31
…seen
`stage` had never once run against a cask-managed bundle: `route` returned
.homebrew before reaching it, so the path that now installs over *every* copy
was the one path with no coverage of the layout Homebrew leaves behind. And
check-self-update.sh cannot close it — it refuses to touch /Applications by
design, which is exactly where the cask installs.
Built to what was measured off this machine's real install rather than to what
seemed likely:
/opt/homebrew/bin/gitpic -> /Applications/GitPic.app/Contents/Resources/gitpic
/opt/homebrew/Caskroom/gitpic/0.20.8/GitPic.app -> /Applications/GitPic.app
Both are symlinks to absolute paths, and the swap is two renames inside the
bundle's own directory, so both should come out on the new version. The CLI
upgrading with the app is not a feature anyone wrote; it is a consequence of
that, and it is the promise the cask's `binary` stanza makes to the terminal.
So the bin/gitpic half is asserted by *running* the symlink and reading the
version it prints — "the link still resolves" and "the terminal has the new
CLI" are different claims, and only the second is the one that matters.
The completions the cask also installs are real files rather than symlinks —
measured — which is both why they go stale after a self-update and why there is
nothing here to assert about them.
Verified the assertions bite, not just that they pass: with the swap skipped,
the CLI assertion fails with "still resolves to the old bundle" and the Caskroom
assertion with 0.18.0 != 0.19.0. A first attempt at falsifying it was a no-op —
resolvingSymlinksInPath() on a path containing no symlink returns it unchanged —
which is worth recording as the reason the mutation that did work is the one
described here.
The doc comment states the limit rather than overselling the test: a swap that
replaced the bundle by copying into the same path would *not* be caught, because
a path-based symlink cannot tell that from a rename. What it does catch is the
bundle landing at any other path, and anything that leaves the CLI link on the
old version — the second of which `bundleVersion(of: target)` alone would miss.
Found by grepping the whole tree for the mechanism this branch removed, rather than by reading the files it had already touched — which is how all four survived the change that falsified them. src/commands/update.rs said "`GitPic.app` runs `brew upgrade` instead" as the reason the CLI does not install. Flatly false now, and it was the *stated* reason for a behaviour that is still correct — so the behaviour keeps its rationale and gets the true one: a `gitpic` on PATH can come from the cask, the formula, `cargo install` or a tarball, which want four different upgrade commands and this process could perform at most one. SelfUpdate.swift introduced its no-proxy decision as "the asymmetry with `Updater.upgradeAndRelaunch`", naming a function this branch deleted. There is no asymmetry left — this is the only thing that downloads an update now — but the measurement the decision rests on is untouched and stays. QuitPathContractTests said `prepareToQuit()` has "two callers that do not exit". It has one; the second was the Homebrew path's own GITPIC_APP_DRY_RUN branch. The rule the sentence exists to justify is unchanged, which is worth saying rather than leaving a count that is simply wrong. check-self-update.sh described the action row as costing "a 20 s `brew list --cask`" and the three buttons as "下载并更新 or 立即更新, then 打开发布页 and 稍后". The three-button check is still correct — 下载并更新, 打开发布页, 稍后 — so the script works as written; only its account of why did not survive. Verified with `bash -n` and left otherwise alone. No behaviour changes. cargo fmt/build clean, `bash -n` clean, swift test 267 passed.
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
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.
Homebrew 装的用户点更新时,app 先退出,然后才由脱离进程的脚本去跑
brew upgrade --cask gitpic—— 刷 tap、下载、校验、安装全都发生在 app 已经消失之后,没有进度、不能取消,900s 看门狗是唯一的兜底。手动装的用户走的是相反的顺序(活着时下载校验,退出后两个 rename,秒回)。这个 PR 让所有人都走后者。顺带修掉一个静默失败:tap 落后于 Release 时,
brew upgrade --cask gitpic打印Warning: Not upgrading gitpic, the latest version is already installed并 exit 0(已实测)。脚本把它记成成功、把老版本开回来,用户白丢一次 app,下次还提示同一个更新。依赖:tarnish233/homebrew-tap@90ea12d(已推送)
原来那条分叉的理由是成立的 —— 背着 brew 换 bundle 会让 cask 的 manifest 描述一个不在磁盘上的版本。Homebrew 有一个专门的 stanza,而 cask 里没写:
auto_updates true。它不等于「brew 从此不管」。对 auto_updates cask,Homebrew 读的是装着的 bundle 的 Info.plist 再和 tap 比(
Cask#auto_updates_bundle_outdated?),不是它自己的安装记录。已对本机真实安装验证:最后一行就是自更新之后、tap 还没跟上的那个窗口。
installed_app_info_plist解析到/Applications/GitPic.app/Contents/Info.plist,证明读的是真实 bundle。tap 先推是刻意的:老版本 app 配新 cask 仍然正常(它只在 bundle 真落后时才调 brew,正是「会升」那一行),反过来则会让 brew 和 app 同时争
/Applications/GitPic.app。删掉的(不是禁用)
Route.homebrew、BrewOwnership/BrewVerdict/fold/brewOwnership、ToolDiscovery的locateBrewOutcome/brewCaskApp/brewCaskroom/firstLine、upgradeAndRelaunch和生成 bash + 看门狗的writeScript、「立即更新」按钮及其确认弹窗。probeQueue改名stagingQueue。净 -692 行。两个副作用:开更新 sheet 不再付 8s 登录 shell + 每个 prefix 20s 的
brew list --cask,所以「正在确认升级方式…」基本看不到了;.unavailable(retryable: true)再也没有来源(探测是唯一来源),这一点写在 case 的注释上而不是把字段折掉,因为删掉它不改变任何运行时行为。测试
覆盖那条分叉的 12 个测试随代码一起删。其中两条断言比测试本身更有价值,保留并推广了:现在
route能产生的每一个拒绝都会被检查「是中文句子且不含选项 flag」(旧测试只把它钉在一个字符串上,而 bug 恰好就在那里),以及「只有三个事实决定路由」改成结构性断言。probeAnswerSeparatesAbsenceFromSilence保留 ——loginShellProbe因为locateGitpic还在用,删掉会让活代码失去覆盖。swift test266 passedcargo test245 + 28 passed,cargo fmt --check干净./scripts/build-app.sh通过(app 0.20.8 / 内嵌 CLI 0.20.8 一致),并按 CI 顺序 build 完再跑了一遍 swift testCHANGELOG 和版本号没动 —— 按仓库惯例那是
chore(release): bump to X那个提交写的。还没验证的
selfInstall从来没跑过 cask 管着的 bundle(route在到它之前就先返回.homebrew)。check-self-update.sh按设计拒绝碰/Applications,覆盖不到这个情形 —— 需要装一个旧版 cask 再从真实 UI 里点一次更新。合并前建议手工跑一遍,重点看$(brew --prefix)/bin/gitpic符号链接是否仍解析、gitpic --version是否跟着升。