Skip to content

fix: stop the updater installing a disk image over the app - #142

Merged
Paulkm2006 merged 2 commits into
mainfrom
fix/updater-asset-selection
Aug 10, 2026
Merged

fix: stop the updater installing a disk image over the app#142
Paulkm2006 merged 2 commits into
mainfrom
fix/updater-asset-selection

Conversation

@yujiezhang-ops

Copy link
Copy Markdown
Collaborator

Fixes #139.

The failure

A v0.4.0 install that accepted the update to v0.5.0 could no longer launch. /Applications/OneAgent.app had become a 5.2MB regular file:

$ file /Applications/OneAgent.app
/Applications/OneAgent.app: zlib compressed data

That size matches OneAgent-darwin-arm64.dmg exactly.

Why

Two upstream behaviours combine, and our asset naming decides whether they fire.

DefaultAssetMatcher takes the first asset whose name contains both the platform and the architecture. The GitHub API returns assets alphabetically, so OneAgent-darwin-arm64.dmg precedes OneAgent-darwin-arm64.zip. It skips .sig, checksums and -installer., but nothing restricts it to formats the updater can unpack.

detectArchive then recognises only .zip, .tar.gz and .tgz. A .dmg falls through to archiveNone, whose contract is "not an archive, hand it to the helper unchanged" — so the helper moved the disk image onto the bundle path.

The helper logged success, and it was not wrong on its own terms: replaceTarget moved a file, and macOS launch shells out to open, which exits 0 for a .dmg because mounting a disk image is a valid open. Both rollback points are conditioned on those two steps failing, so neither fired.

The upstream comment at updater.go:315 describes this exact hazard for .zip — "without this step the helper would replace /Applications/MyApp.app (a directory) with the downloaded .zip (a file)" — and .dmg was simply not on the list.

Only reachable once the app is installed somewhere writable. Four earlier attempts ran from the mounted dmg, where App Translocation put the bundle on a read-only volume and the backup step failed, aborting the update.

The change

ExtractableAssetMatcher (internal/binding/update_asset.go) filters to extractable suffixes, then delegates to DefaultAssetMatcher, mapping the index back. Filtering first rather than reimplementing keeps the upstream sidecar, installer and architecture-alias handling (x86_64, aarch64) intact.

DownloadAndInstall rejects an uninstallable staged artifact. DownloadedPath is set after extraction, so a container suffix surviving there means extraction did not happen. This check must precede the restart offer — the swap runs after the process exits, and there is no interface left to report a failure then. Defensive on purpose: it holds even if upstream matching changes again.

UPDATE_NOT_INSTALLABLE is a new error code. INTERNAL_ERROR maps to no copy by design (failureCopy.test.ts:48 pins that), and this failure needs to say something specific: retrying re-downloads the same asset, so the only way forward is a manual download. It shares exit code 10 since existing numeric codes are pinned by tests.

failureLine joins message and hint for single-line surfaces. The task centre was dropping .hint, which here is the actionable half — "The downloaded update cannot be installed" alone names no next step.

Tests

  • The matcher picks the .zip for all four published platform/arch pairs, driven by v0.5.0's real 9-asset list in API order
  • A companion test asserts DefaultAssetMatcher still picks the .dmg, so the regression case stays honest; it skips rather than fails if upstream changes, pointing at the filter possibly being redundant
  • Filtering does not reintroduce sidecars or installers
  • DownloadAndInstall rejects .dmg / .zip / .msi / .AppImage and accepts .app, a bare binary, and an unreported path; the error is non-retryable and the artifact name stays in the private cause, not the user-facing message
  • AppUpdater shows both the message and the manual-download hint in the task centre

Each layer was verified load-bearing by neutering it and confirming the tests fail.

Verification

go test ./..., go test -race ./internal/binding ./internal/errors, go vet ./..., go build -tags wails ./cmd/oneagent-desktop all pass; frontend 334 tests pass and pnpm run build succeeds; check-docs.py clean.

internal/process/process.go shows up under gofmt -l but is untouched by this branch and unmodified in git status — pre-existing.

Not covered here

Windows was reasoned about but not verified on hardware: the default matcher already skips -installer.exe, and the test covers both Windows pairs selecting the .zip.

🤖 Generated with Claude Code

The GitHub provider's default matcher takes the first asset whose name
contains both the platform and the architecture, and the API returns
assets alphabetically -- so OneAgent-darwin-arm64.dmg won over the
sibling .zip. detectArchive only recognises .zip and .tar.gz, so the
.dmg was treated as "not an archive, pass it through" and the helper
moved the disk image onto /Applications/OneAgent.app. The app could no
longer launch, and the helper reported success: it swapped a file, and
`open` on a .dmg exits 0.

ExtractableAssetMatcher filters to formats the updater unpacks before
deferring to the upstream matcher, so its sidecar and architecture
handling is unchanged.

DownloadAndInstall now also rejects a staged artifact that still carries
a container suffix. That check has to happen before the user is offered
a restart: the swap runs after this process exits, so there is no
interface left to report it. UPDATE_NOT_INSTALLABLE is a new code rather
than InternalError because the only way forward is a manual download and
retrying re-downloads the same asset.

The task centre renders a single line and was dropping the hint, which
for this failure is the half that names a next step, so failureLine
joins the two through the translation table.

Refs #139

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@yujiezhang-ops
yujiezhang-ops requested a review from a team August 10, 2026 06:59
@Paulkm2006
Paulkm2006 merged commit 9bae73e into main Aug 10, 2026
4 checks passed
@Paulkm2006
Paulkm2006 deleted the fix/updater-asset-selection branch August 10, 2026 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P0:自更新会选中 .dmg 并用它覆盖 .app,导致安装被破坏

2 participants