🤖 fix: restore ${os} token in release artifact names - #3927
Conversation
electron-builder's default artifact pattern includes ${os}; our override
dropped it, so no published asset carries a mac/win/linux token. Installers
that match release assets by OS (mise/ubi, Homebrew, asdf) reject every
asset as a result.
Also update the arm64 AppImage upload glob, which anchored on the old
"-arm64.AppImage" suffix.
|
@codex review Config-only change to
|
This comment has been minimized.
This comment has been minimized.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7520768a0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review caught four consumers of the old names that my first grep
missed (it required a `release/` prefix and an `xum-` prefix, so bare
`*-x64.dmg` style globs slipped through):
- _desktop-release.yml: `gh release download --pattern` for the Linux
checksum job; an unmatched pattern errors out before checksums publish.
- nightly.yml: REQUIRED_ASSETS sentinel list, which would treat a complete
nightly as incomplete and rebuild it.
- pr.yml: macOS dmg upload globs, which set `if-no-files-found: error`.
- docs/install.mdx (and its generated built-in skill copy): documented
Windows installer filename.
Also address the tautological-test finding: expandArtifactName now routes
through electron-builder's own expandMacro instead of reimplementing the
substitution rules, so it throws ERR_ELECTRON_BUILDER_MACRO_NOT_DEFINED if
${os} ever stops resolving. Verified it does throw when the macro is absent.
Alias-script fixtures updated to realistic post-rename filenames so they
prove mux-* aliasing still works with the new names.
|
@codex review Addressed all four findings in 35c0b30 — thanks, the Linux and macOS consumer catches were real misses. My original grep required both a
Also updated the Re-grepped for residual old-pattern references across workflows, scripts, and docs and found none — but given this round, another look for consumers I've still missed would be welcome. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 participant
Summary
Restores the
${os}token inbuild.{mac,linux,win}.artifactNameso every published release asset carries amac/linux/wintoken in its filename, and updates every in-repo consumer of the old names. Without the token, installers that select release assets by operating system cannot find any xum asset at all.Background
electron-builder's default artifact pattern is
${productName}-${version}-${arch}-${os}.${ext}(app-builder-lib/out/targets/ArchiveTarget.js). Our override exists to get the lowercasexum-slug instead of theXumproductName, but it also dropped-${os}.The result is that v0.28.2 published
mux-0.28.2-arm64.dmg,mux-0.28.2-arm64.zip,mux-0.28.2-x64.exe, andmux-0.28.2-x86_64.AppImage— not one of which says which OS it is for.Asset-matching installers filter on an OS token before anything else, so they reject the entire asset list. Concretely, with
mise:Worse is the silent failure mode:
mise install github:coder/xumreports success. The only asset whose name contains an OS-ish token islatest-mac.yml, so mise installs that 785-byte electron-updater manifest and exits 0. The user gets a tool directory containing one YAML file, no binary, and no error. The same class of failure applies to Homebrew and asdf.Implementation
xum-${version}-${arch}.${ext}→xum-${version}-${arch}-${os}.${ext}for all three platforms.${os}expands tomac/linux/winfromplatform.buildConfigurationKey, and all targets (ArchiveTarget,DmgTarget,NsisTarget,AppImageTarget) route through the sameexpandArtifactNamePattern, so dmg/zip/exe/AppImage all pick it up.The rename has in-repo consumers that hardcode the old suffixes; all are updated here:
_desktop-release.ymlarm64 AppImage uploadrelease/xum-*-arm64.AppImagegh release upload_desktop-release.ymlLinux checksum job--pattern "xum-*-x86_64.AppImage"etc.gh release downloaderrorsno assets match the file pattern, before checksums publishnightly.ymlREQUIRED_ASSETSxum-${RELEASE_VERSION}-arm64.AppImageetc.pr.ymlmacOS dmg uploadsrelease/*-x64.dmg,release/*-arm64.dmgif-no-files-found: errorfails every macOS CI build after packagingdocs/install.mdx(+ generated skill copy)xum-x.x.x-x64.exescripts/create-legacy-mux-artifact-aliases.shneeds no change — it rewrites thexum-/Xum-prefix and passes the remainder through untouched, somux-aliases keep tracking canonical names. Its test fixtures are updated to realistic post-rename filenames so they actually demonstrate that.expandArtifactNameinproductIdentity.test.tsnow calls electron-builder's ownexpandMacrorather than reimplementing the substitution rules, so the test exercises the builder instead of comparing two copies of the same string.Validation
Verified the fix actually solves the problem end-to-end, rather than assuming it would. I published a scratch GitHub release carrying the post-fix asset names (
xum-0.29.0-arm64-mac.zipplus the full set of dmg/blockmap/exe/AppImage/yml siblings, the zip containing only anXum.appbundle) and pointed mise at it:mise strips the
.appwrapper, installsContents/MacOS/xum, and the shim runs. No consumer-side options needed —"github:coder/xum" = "latest"is enough. Two things worth recording from that test:.app-only archive is otherwise a hazard for these installers: without a resolvable executable they abort withIs a directory (os error 21). Our bundle layout happens to resolve cleanly.Confirmed the rewritten test is not tautological —
expandMacrothrows when the macro is absent, so the assertion has teeth:Locally:
bun test src/common/compat src/desktop/updater.test.tsgreen,make typecheckclean,actionlintclean on all three modified workflows. Grepped for residual old-pattern references and found none.What this unblocks for users
After this lands, installing xum's CLI via mise is one config line — no asset patterns, no per-user workarounds:
mise install,mise up, and version pinning all work from there, and the same applies to any installer that matches release assets by OS token.To be precise about scope: this gives users the CLI on PATH, not a Finder/Spotlight-registered application. mise installs under
~/.local/share/mise— a hidden directory macOS does not index — and has no concept of an.appbundle; it unpacks ours to bareContents/MacOS/xum. Anyone wanting the desktop app should still use the dmg. This PR is about making the assets discoverable by tooling, not about making mise a supported install path for the GUI.Today, with no OS token, neither is possible —
github:coder/xumsilently installs a YAML manifest and reports success.Risks
Low, but it touches the release path, so worth being explicit.
Auto-update is unaffected.
src/desktop/updater.tsis stockelectron-updaterwith the GitHub provider and never reconstructs asset filenames from a pattern — it reads them out oflatest-mac.yml, which electron-builder regenerates with the new names. A client on 0.28.2 reads the new manifest and downloads the new filename. I checked for hardcoded name-building specifically because that would have made this a coordinated change instead of a config one.Download links change. Anything pinning an exact asset URL for a future release needs updating; already-published assets are untouched.
The residual risk is a hardcoded asset name outside this repo (website, install script, package manifest) that I can't see from here — worth a second pair of eyes. The first review round showed this rename has more consumers than a grep for the obvious prefix turns up.
Note:
Pixel / Reviewfails on this PR and equally on unrelated PR #3926; this change touches no UI.Generated with
Claude Code• Model:claude-opus-5