chore(self-update): clearer Windows hint, surface xattr failures#23
Merged
Conversation
Detect Windows in detectBinaryName and return a message that points users at install.ps1 instead of the generic "Unsupported platform" fallback, which gives no clue that Windows is actually supported via the install script. Extract the macOS quarantine removal into a small, injectable helper. The previous implementation swallowed every failure silently, leaving the user without any feedback when xattr was missing or errored — and without any hint why Gatekeeper might still block the new binary. Failures now surface as ui.warn() without failing the update, since quarantine removal is best-effort. Add unit tests for the Windows branch, the generic fallback, and the quarantine warn-on-failure path.
- tryRemoveMacosQuarantine now try/catches the injected remover so a
buggy or future remover that throws synchronously still degrades to
a warning instead of failing the update, matching the documented
best-effort contract.
- Add positive test for detectBinaryName("darwin", "x64") — previously
only darwin/arm64 and linux/x64 had positive assertions.
- Replace misleading "non-Error" test (which actually passed an Error)
with two tests that genuinely exercise the synchronous-throw path:
one with a thrown Error, one with a thrown non-Error primitive.
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
1 participant
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.
What
detectBinaryNamenow branches onwin32before falling through to the generic "Unsupported platform" error, returning"Windows is not supported by self-update; reinstall via install.ps1". The previous message gave Windows users no clue that the install script supports their platform.tryRemoveMacosQuarantine(targetPath, deps)helper. Failures (missingxattr, non-zero exit, thrown error) now surface asui.warn(...)instead of being silently swallowed. The update itself still succeeds — quarantine removal is best-effort.Why
Bun.spawnSync(["xattr", ...])previously returned a result that was ignored entirely (no exit-code check), and thetry/catchonly caught throws. On a system withoutxattr, or when the attribute is missing, the user got no feedback at all — and no hint about why Gatekeeper might keep blocking the new binary.For Windows the generic
"Unsupported platform: win32/..."was actively misleading:install.ps1ships exactly this CLI, but nothing in the error pointed there.Design
detectBinaryName(platform, arch)now takes its inputs as parameters instead of readingprocess.*inline. The CLI caller passesprocess.platform/process.arch; tests pass strings. Pure function, deterministic.QuarantineRemover = (path: string) => Result<void>letsdownloadBinaryaccept either the realBun.spawnSyncbased remover (defaultQuarantineRemover) or a fake in tests.downloadBinaryalso takesplatformand awarncallback in its deps bag, so neitherprocess.platformnoruiis referenced inside.selfUpdateCommandwiresdefaultQuarantineRemoverandui.warn. No container change needed — the dep is local to this command.Tests
src/cli/commands/self-update.test.tscovers:detectBinaryName("win32", "x64")returns the Windows-specific message; it is not the generic fallback.detectBinaryName("freebsd", "x64")anddetectBinaryName("linux", "ia32")keep the genericUnsupported platform: …message.detectBinaryName("darwin", "arm64")/("linux", "x64")succeed with the expected binary names.tryRemoveMacosQuarantinedoes not warn on success and does warn (without throwing) on failure, surfacing the underlying error and the manual fix command.Checks
pnpm typecheck && pnpm lint && pnpm test— all green locally (502 pass, 0 fail).Closes Vikunja #43 (id 215)