feat(desktop): ship macOS x64 and Linux packages - #4468
Conversation
Desktop published only macOS arm64 and Windows x64, so Intel Macs and every Linux desktop had no build at all. Both nightly and formal releases now package five targets: macOS arm64 and x64, Windows x64, and Linux x64 and arm64, with Linux shipping an AppImage and a deb. Each architecture builds on a runner of its own because the native Runtime Host peer and the packaged smoke probes are host binaries; cross-building would embed the wrong architecture's addon and produce a package that builds green and dies on launch. That makes one update feed carry several payloads for the first time. macOS puts both architectures in one feed and electron-updater picks its payload out of the `files` list, but each architecture is packaged separately and writes a feed naming only its own zip. Packaging now names the macOS feed after its architecture so the two uploads cannot overwrite each other, and publication merges them back into the single feed clients read. Linux advertises both the AppImage and the deb in one feed, so `verifyDesktopUpdateArtifacts` takes the full payload list instead of a single name, and requires a blockmap for every payload except the fpm targets, which ship without one. `desktop-release-targets.mjs` becomes the one place that knows what a Desktop target produces. Both publication channels read it, so the nightly workflow no longer repeats artifact names in YAML and the provenance step no longer counts subjects against a hardcoded number. Adding a target is now one edit. The macOS automatic-update proof stays on arm64: the updater is one code path across both architectures, and package-macos-autoupdate-next.mjs already enforces that restriction. Generated-by: Claude Code
…builds
The target descriptor asserted three things about electron-builder that were
all false, and nothing checked any of them.
electron-builder ignores the command line's architecture flags for any target
the configuration names: computeArchToTargetNamesMap only falls back to the CLI
when `target.arch` is absent. `mac.target` carried `arch: ['arm64', 'x64']`, so
`--mac --arm64` built both architectures on every macOS runner, and `--linux`
built both on every Linux runner — each embedding the host's Runtime Host peer
and, on macOS, leaving the cross-built DMG unnotarized. The targets move to the
packaging scripts, which now name the target and the architecture together, and
the configuration says nothing about either.
`${arch}` in a Linux artifactName is the packaging ecosystem's spelling, not
Node's: getArtifactArchName rewrites x64 to `x86_64` for an AppImage and to
`amd64` for a deb. The descriptor expected `-linux-x64`, so the x64 runner
would have failed on its own output.
An AppImage has no `<file>.blockmap`: appImageUtil appends its block map to the
AppImage itself, and the client reads it back out from there. The descriptor
listed a sidecar that is never written, and the feed verifier demanded one for
every payload that is not a deb. It now names the two payloads that do have a
sidecar instead of guessing from what does not.
desktop-release-targets.test.mjs drives electron-builder's own resolution
functions against the packaging scripts and the configuration, so the descriptor
can no longer state something electron-builder does not do.
Two further defects on the same seam:
The AppImage and the deb are built from one unpacked tree, and the deb target
writes a `package-type` marker into it that electron-updater reads to choose
DebUpdater over AppImageUpdater. An AppImage carrying that marker updates itself
by installing a deb. Linux packaging now builds the AppImage in a run of its
own, before the deb, and merges the two feeds the runs write; verify-linux.mjs
fails if the marker reaches the AppImage anyway.
Publication never merged the two macOS feeds. Each runner uploads a feed named
after its architecture and `releaseAssets` names only the merged feed, so the
exact-manifest check could not have passed. `mergedFrom` now survives into the
release identity and the publish job merges before it verifies.
Also restores the provenance count guard the previous commit dropped — a
process substitution hides its exit code, so a failing lister left the step
green with nothing verified — binds release.yml's matrix to the descriptor,
and brings the release notes and RELEASE_CHECKLIST.md up to five targets.
Generated-by: Claude Opus 5 via Claude Code
667f9c0 to
786c19e
Compare
ARE404
left a comment
ARE404
left a comment
There was a problem hiding this comment.
Holding approval at head 786c19e4369300b69f8be756b87a27a8d9406eaf: the code review is clean, but this is a release-platform feature whose gate is the package check, and CI is not green here yet (package/audit in progress, label queued). No P0/P1 found in the change.
feat(desktop): ship macOS x64 and Linux packages — a broad, well-structured release-infrastructure expansion. Verified:
- Targets module is authoritative and correct.
desktopReleaseTargetsenumerates macos(arm64,x64), windows-x64, and linux(x64,arm64) with per-runner payloads/feed/advertised/checksums; Linux artifact-arch naming (x86_64/amd64) is recorded and pinned to electron-builder's own mapping by test; mac feeds correctly collapse into onelatest-mac.yml/dev-mac.ymlmerged across both architectures. - electron-builder config fix is a real correctness catch. Removing the per-arch
target/archfrommac/winand making the package.json scripts the single authority avoids the electron-builder quirk where a configuredarchsilently overrides the CLI and would build every architecture on every runner. Newlinuxblock (icon/category/maintainer) is correct — fpm requires a maintainer address. - mac x64 + Linux packaging and update contracts are complete.
package-macos.mjs/package-linux.mjsbuild native Runtime Host per-arch on its own runner (no unsafe cross-build);linuxUpdateMetadataName+mergeDesktopUpdateFeedDocumentscorrectly handle the all-arch-in-one-mac-feed vs per-arch-Linux-feed distinction, with duplicate-payload and version-consistency rejection; blockmap sidecar handling is explicit (AppImage appends inside, fpm has none). - Release identity generalizes cleanly.
product-release-identityderives the must-publish artifact set fromdesktopReleaseTargets(...).checksums(.sha256beside each) instead of a hardcoded per-platform list, covering every platform consistently.
Holding only on CI, which for a packaging feature is the actual proof: bring package (and test) green at this head and I'll approve — the design reads correct and the Windows/mac baseline gate is already green at the head. If package self-reds on a new platform, that's a real finding and I'll flag it precisely.
jackwener
left a comment
jackwener
left a comment
There was a problem hiding this comment.
I reviewed this at 786c19e4369300b69f8be756b87a27a8d9406eaf. No P0, P1, P2, or P3.
Desktop published only macOS arm64 and Windows x64. This branch packages five targets, each on a runner of that architecture: macOS arm64 and x64, Windows x64, and Linux x64 and arm64 (AppImage and deb). The Runtime Host peer is a host binary, and the packaging scripts refuse to cross-build, which is the right cut.
desktop-release-targets.mjs is the one place that names payloads, feeds, advertised files, and checksums. Nightly staging and the formal release both read it. electron-builder config no longer sets target.arch, so the CLI flags actually choose the architecture; a configured arch would have silently built every architecture on every runner and embedded the wrong peer.
The feed contract holds:
- Each macOS runner writes
<channel>-mac-<arch>.ymlso the two uploads cannot overwrite each other. Publication merges them into the single feed clients read.MacUpdater.filterFilesForArchthen picks the zip by architecture from thatfileslist. - Linux builds the AppImage first, moves that feed aside, builds the deb, then merges. That keeps the deb
package-typemarker out of the AppImage (verified on extract). AppImageUpdater and DebUpdater select by extension from the combined list. Neither payload has a sidecar blockmap; the verifier only demands one for.exeand.zip. - Provenance counting writes the attested list to a file first, so a failing process substitution cannot leave the step green.
The PR body says package is blocked on #4461's Windows path.join test. That change is already on main, and this branch does not touch the test, so that is not a live rebase requirement.
Linux packaging, macos-15-intel, the x64 auto-update path, and 16K-page ARM hosts are unproven in this repository. Those are notes, not defects in this diff. This is a feature; I am not merging it.
简体中文
我审的是 786c19e4369300b69f8be756b87a27a8d9406eaf。没有 P0/P1/P2/P3。
五个目标各自在对应架构的 runner 上打包，禁止交叉编译。目标描述是唯一权威；配置里不再写 target.arch。macOS 按架构写 feed，发布时合并；Linux 先打 AppImage 再打 deb，避免 package-type 进 AppImage。PR 正文里写的 #4461 Windows path.join 阻塞已经在 main 上，不是当前变基条件。Linux 首次打包、macos-15-intel、x64 自动更新、16K 页 ARM 都还没在本仓库跑过，那是备注不是缺陷。这是功能，我不合入。
Automated review notice: This comment was posted by an automated review agent operated by WAWQAQ. It is not an independent human review and does not replace one.
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed exact head 786c19e4369300b69f8be756b87a27a8d9406eaf. Result: 2 P1 blockers; I am not approving this head.
This change expands Desktop packaging/publication from macOS arm64 and Windows x64 to five native targets, centralizes release artifact/feed identity, and adds Linux AppImage/deb packaging and verification.
I inspected the complete diff, target descriptors, Nightly/formal workflow matrices, electron-builder target selection, update-feed merging, publication identity, Linux package/verification paths, and current-main integration. A clean install, build:test, full typecheck, focused release/workflow tests, git diff --check, and a clean synthetic merge against current main passed.
The release path itself does not pass: npm run package:linux fails on the first AppImage build. Supplying a temporary executable-name override exposes a second deb metadata failure. After supplying both temporary overrides, I built real x64 artifacts and confirmed that verify:linux accepts an arbitrary text file in place of the deb and emits a checksum for it.
Unable to validate macOS x64 signing/notarization, Linux arm64 packaging, or real update installation from this Linux x64 host. Hosted checks were not terminal at publication, and the current PR-triggered package job is the existing Windows release check rather than either new Linux package job.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
M4n5ter
left a comment
M4n5ter
left a comment
There was a problem hiding this comment.
Requesting changes at exact head 786c19e4369300b69f8be756b87a27a8d9406eaf for two P1 release blockers:
- The new Linux packaging path cannot produce its payloads. A real
npm run package:linuxfails the first AppImage target because the scoped package name becomes the unsafe executable name@makadesktop. Supplying a temporary safe executable name lets the deb target proceed, but it then fails because no project homepage is defined. Both Nightly and formal Linux matrix jobs call this path. - The Linux verifier does not inspect the independently built deb. With a real x64 AppImage and the text
not a deb packageat the expected.debpath,npm run verify:linux -- x64exited successfully and wrote a checksum for the text file. That payload can then be staged and attested as a release asset.
Please define a safe Linux executable name and canonical homepage, exercise the real Linux packaging path in CI, and extract and validate the deb's resources, architecture, update configuration, dependency closure, package metadata, and package-type=deb before issuing its checksum. The existing current-head inline discussions contain the exact anchors and reproduction details. I found no additional P0–P3 issues.
Automated review notice: This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.
ARE404
left a comment
ARE404
left a comment
There was a problem hiding this comment.
Correcting my earlier review comment (which said no P0/P1): two valid P1 blockers have since been demonstrated on this exact head, and I agree they block.
electron-builder.config.mjs— the new Linux targets don't currently build:npm run package:linuxfails at the first electron-builder invocation (executableName … @makadesktop), and after a--config.linux.executableName=Makaprobe the deb step separately fails withPlease specify project homepage. So none of the four new Linux artifacts can be produced.verify-linux.mjs— the deb is only hashed, never validated: replacing the deb withnot a deb packagestill exits 0 and writes a checksum, so a corrupt/structurally-wrong deb could pass the release gate and be published.
Both were reproduced by hqhq1025 (and M4n5ter requested changes for the same two). Please fix these (safe executableName + a homepage/package URL source, and deb payload extraction/validation in verify-linux before its checksum), then re-run — I'll re-review and approve once they're closed and test is green at the head.
Neither Linux target could be built. electron-builder derives the Linux
executable name from the npm package name, and this one is scoped:
`sanitizeFileName('@maka/desktop')` is `@makadesktop`, which a desktop
entry's `Exec=` cannot launch. It has a scoped-package fix for the deb's
package name and none for this. fpm then refuses to run at all without a
project homepage, which neither manifest declared and which cannot be
recovered from the git remote on a fork.
Both are settled before electron-builder packages anything, so the
contract test now drives that resolution instead of a runner discovering
it: it builds a packager per Linux packaging script, requires the
executable name to be one a shell will accept, and calls `checkOptions`
on each target, which is where fpm demands its metadata. The AppImage
target inherits an empty `checkOptions`, so the name assertion is what
covers it. This costs 180ms on any host and fails with the same message
the build would have.
Only Linux resolves an executable name this way and only fpm demands a
homepage, which is why macOS and Windows never showed either defect.
Generated-by: Claude Opus 5 via Claude Code
The verifier opened the AppImage and applied every packaged-resource, update and dependency assertion to it, then handed the deb a checksum without ever reading it. Replacing the deb with a text file left the gate green and produced a checksum for that text. Its comment explained why: both payloads came from one unpacked tree, so proving the contract on the AppImage proved it for both. That stopped being true in this branch. Keeping the deb's `package-type` marker out of the AppImage required splitting packaging into two electron-builder runs, and the second one packs its own tree. Nothing about one payload carries over to the other any more. Both are now extracted and asserted. The deb additionally has to carry `package-type=deb` — the mirror of the AppImage forbidding it, and what sends the packaged updater down DebUpdater — and to declare the architecture its filename claims, so a runner that built the wrong one cannot publish it under the right name. Checksums are issued only after every assertion has accepted the payload. Where the deb keeps its resources is discovered from the extracted tree rather than derived from the install prefix. Deriving what a payload should contain, instead of reading what it does, is how this verifier came to trust a file it had never opened. Generated-by: Claude Opus 5 via Claude Code
Desktop packaging went from two targets to five, but the pull-request checks did not move: `package` is the Windows lane, and nothing built macOS x64 or either Linux package before merge. Both Linux defects fixed in this branch survived review for that reason — no check could see them, because nothing outside a real Linux build ever ran that path. `release-windows-check.yml` already carries this lesson, in its own words: the Windows release path first ran on release day, on main, with a tag reserved. This is the same lane for Linux, and its `paths` list overlaps because the packaging configuration and the manifests decide the identity both platforms need — those files changed here, the Windows lane ran, and it passed, because neither defect was observable on Windows. x64 only. arm64 is built from the same scripts, configuration and descriptor, so this lane would re-prove everything it can reach and nothing that differs — a 16K-page host and its own native peer. The Nightly matrix builds and verifies arm64 every night. This covers what the contract test structurally cannot: fpm, the AppImage runtime, the native peer, and verification passing over real payloads. Generated-by: Claude Opus 5 via Claude Code
Both lanes named their job `package`, so a pull request showed two checks called `package` with no way to tell which platform failed. Generated-by: Claude Opus 5 via Claude Code
The Windows lane pairs its path filter with a nightly cron because the filter is a pre-filter, not an import closure. This lane needs no such pairing: the Nightly matrix already runs `package:linux` and `verify:linux` for both architectures on every npm Nightly publication, which is more often than a daily cron. The Windows cron earns its runner by also exercising the installer lifecycle, automatic update and rollback; this one would only buy a second copy of the same run. Generated-by: Claude Opus 5 via Claude Code
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed exact head 7ef634d80a81d6c91a072591df49daad4d54e401. Verdict: P1 NO-GO (plus one P2).
The two previous P1 findings are fixed: a clean Linux x64 AppImage/deb build now succeeds, and replacing the deb with non-package data makes verify:linux fail. The remaining blocker is the new PR workflow failing the repository-wide CI policy; the required test check is red on this head. I also found a Linux desktop identity mismatch that can break launcher/window association.
Scope checked: complete fix delta, Linux packaging and verification scripts, release/update contracts, workflow policy, generated deb metadata, current-main synthetic merge, existing reviews, and exact-head hosted checks.
Validation: clean npm ci, build:test, full typecheck, 83 focused release/workflow tests, real x64 AppImage + deb packaging, verify:linux, corrupt-deb rejection probe, and clean synthetic-merge build/typecheck all passed. ci-workflow-policy.test.mjs reproduces 3 failures locally, matching the hosted test failure. Windows package was still running at publication time.
Unable to verify: Linux arm64, macOS signing/notarization, real Wayland/desktop-portal behavior, and update installation against a published feed.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
`ci-workflow-policy.test.mjs` rejected the lane on three counts, and it was right on all three. Restores the cron I had removed one commit earlier. I argued the Nightly made it redundant because that matrix runs the same two commands. The policy is enumerated over the whole workflow directory precisely so that reasoning cannot be applied lane by lane: a path filter is a pre-filter, not an import closure, so the lane itself must be reachable without consulting it. The Nightly is a different workflow, gated on a successful npm publication and a repository variable, so it is not this lane's escape from its own filter. Aligns the timeout to 45, the single tier every pull-request lane carries, rather than the 60 I picked with nothing behind it. The first real run packaged and verified in 6m22s. Registers the lane in the pull-request allowlist, which exists because a workflow that nobody remembered to name once kept an unbounded trigger. Generated-by: Claude Opus 5 via Claude Code
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed exact head 7f9b1479ecf2e4b99fdbbbb427e240b5e172685a. APPROVED: no P0/P1 findings.
The previous CI-policy P1 is fixed: the Linux gate is now explicitly allowlisted, uses the shared 45-minute timeout, and has its own scheduled escape from the path filter. The exact-head policy suite passes 35/35 locally, and all hosted checks are terminal green, including test, Windows package, and Linux package-linux.
One non-blocking P2 remains in inline comment 3906229002: the generated Linux desktop entry uses StartupWMClass=Maka while the packaged process reports WM_CLASS=maka; adding desktopName plus linux.syncDesktopName would keep launcher/window identity aligned.
Validation: clean npm ci, build:test, full workspace typecheck, 118 focused release/workflow tests, git diff --check, and the same build/typecheck/focused suite on a clean synthetic merge with current main 5013f904b all passed. The prior real x64 AppImage/deb package and corrupt-deb rejection probes remain applicable because this head changes only the Linux workflow policy and its allowlist test.
Unable to verify independently: Linux arm64, macOS signing/notarization, real Wayland/desktop-portal behavior, and update installation against a published feed.
Review notice: This review was prepared by an automated review agent operated by hqhq1025 and is published at the direction of AstroHan, who has read these findings and is the human accountable for them.
electron-builder warned about this once per Linux target in the packaging log, and named the fix; nobody read the log, because the lane was green. Electron derives its app_id — the window's WM_CLASS — from `desktopName` in the manifest. Absent that field, the desktop entry's `StartupWMClass` falls back to the product name, so the entry advertises `Maka` while the window reports `maka`. They never match, and a desktop environment that cannot link the two shows a generic icon and silently ignores an attempt to pin the app. Every Linux user meets this on first launch. Setting `desktopName` in the manifest and `linux.syncDesktopName` in the configuration derives the entry's filename and the app_id from one string. Generated-by: Claude Opus 5 via Claude Code
Every Linux assertion read files. `verify-macos-dmg.mjs` asserts the executable's architecture with `lipo`, runs a pty probe, smokes the filesystem worker and launches the renderer over CDP; the Windows lane runs its sandbox probe. Linux never ran anything it had built. Each defect this branch collected — the unlaunchable executable name, the unread deb, the unset desktop name — was some check the other two platforms already had and Linux did not. Three assertions close that gap: The Runtime Host peer's ELF `e_machine` is read in both payloads. The packaging scripts refuse to cross-build precisely because a mismatched peer produces a package that installs and dies at launch, and Linux was the only platform where nothing could detect it. `assertElfArchitecture` is exported and covered by `verify-linux-harness.test.mjs`, which builds ELF headers byte by byte and so runs on any host. The deb's `Package:` field must be an installable Debian name. electron-builder derives it from the product name, and `dpkg` rejects a capital letter, so a package nobody could install would otherwise reach the Nightly — which publishes without a human. I could not settle this by reading fpm's sources, and reading sources instead of running them is the mistake this branch keeps repeating, so it is now a question CI answers. The extracted AppImage is launched under `xvfb-run`. Only this can fail on a package that is structurally perfect and still will not start. It is applied to the AppImage alone: extracting it yields the tree its runtime mounts at launch, whereas `dpkg-deb -x` output is not an installation — `dpkg` would still have to set the sandbox helper's setuid bit — so launching that would prove something about a tree no user has. Also drops the lane comment claiming it covered "the AppImage runtime and the native peer", which described none of what the lane did; requires exactly one `resources/app.asar` in the deb rather than taking whichever `find` returned first; and extends the path filter to what the packaging configuration and the verifier actually import, plus the icon catalogue `assertPackagedResources` enumerates to build its own required list. Generated-by: Claude Opus 5 via Claude Code
This branch exists to make `desktop-release-targets.mjs` the one place
that knows what a target produces, and it freed the Nightly's artifact
staging from spelling names in YAML. Then it added three new spellings in
the same diff: macOS went from one architecture to two, so the hardcoded
`-mac-arm64.dmg` became `-mac-${{ matrix.arch }}.dmg` in the signing step,
the verification step, and again in the Nightly. A second authority on the
artifact name, and unlike the descriptor nothing checked this one.
`verify:macos` now takes an architecture and resolves the path from the
descriptor, the way `verify:linux` already did — which also means the
verification fails if the descriptor and the build ever disagree.
The signing and notarization steps discover the DMG instead: a runner
builds one architecture, so exactly one exists, and signing never needed
its name. Finding none or several now fails the step rather than passing
a wrong path to `codesign`.
Two spellings remain, both older than this branch and neither on the
matrix: the arm64-only automatic-update ZIP and the Windows installer.
Generated-by: Claude Opus 5 via Claude Code
`resolveProductReleaseIdentity` rebuilt each published feed as
`{name, advertised, mergedFrom}` — the exact shape `desktopPublishedFeeds`
already returns, field for field. The comment above it described what the
function itself does.
`mergeDesktopUpdateFeedDocuments` and `desktopNightlyPublishedFeeds` were
exported with no consumer outside their own modules, tests included. Two
fewer public contracts to keep.
Generated-by: Claude Opus 5 via Claude Code
Review responseThanks to @hqhq1025 and @M4n5ter for reproducing on a real Linux host, and to @ARE404 for correcting your own earlier assessment once they had. Everything reported is addressed across One correction firstI claimed in this thread that CI was green over the packaging matrix. That was wrong: The defects share one causeI initially diagnosed the two P1s as "derived from reading electron-builder's sources, never executed", and added execution — the new Linux lane. That analysis was too narrow, and this second round proved it. Line up everything reported:
Every one is something the supported platforms already had that Linux did not. The real cause is that I implemented "add a platform" as "write new code", when in this repository it is "make a new platform satisfy the contract the existing ones already meet" — a contract written down in The sharpest instance: the commit that fixed "claims exceeding what was executed" also introduced a lane comment claiming it covered "the AppImage runtime and the native peer" — which described nothing the lane did. That line is gone, and the lane now does what it said. What changed beyond the individual fixes
Pushed back
Still not covered, stated plainlymacOS x64 has no pull-request lane. It shares its scripts, configuration and descriptor with arm64, and the Nightly builds and verifies it. Linux arm64 and
|
The contract test isolates a configuration copy per packager because electron-builder rewrites the object it is handed. structuredClone did that locally on Node 26 and failed on the Node 24 the CI lane runs: "Unable to deserialize cloned data due to invalid or unsupported version". Everything electron-builder reads out of this configuration has to survive serialization anyway, and JSON round-tripping behaves the same on every version. `beforePack` is the one function in it and is carried across by reference. Generated-by: Claude Opus 5 via Claude Code
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed exact head 59cbf4f0d1affc2cfb88dd2d77416830669c380e. Technical GO: no P0-P3 findings on this head.
The previous Linux desktop-identity P2 is fixed: the package now derives the desktop entry and Electron app identity from desktopName: "maka.desktop" with linux.syncDesktopName: true. This head also strengthens the platform contract by validating the Runtime Host ELF architecture in both Linux payloads, launching the extracted AppImage under Xvfb/CDP, checking deb metadata, and resolving macOS DMG paths from the shared target descriptor.
Scope checked: the complete PR diff and latest fix delta; Linux/macOS package scripts and verifiers; release target, update-feed, provenance, and workflow-policy contracts; generated x64 AppImage/deb payloads and desktop metadata; existing review threads; and a clean synthetic merge with current main (6b3f38ddf).
Validation passed: clean install, build:test, full workspace typecheck, 144 focused release/workflow/verifier tests, real Linux x64 AppImage and deb packaging, verify:linux -- x64, git diff --check, and synthetic-merge build/typecheck plus 190 focused tests. All current-head hosted checks are terminal green, including test, package-linux, Windows package, audit, and both Runtime Host owner-platform lanes.
Not independently validated from this Linux x64 host: Linux arm64 packaging, macOS x64 signing/notarization, real Wayland/desktop-portal integration, or installing/updating from a published feed. Older reviews and the old CHANGES_REQUESTED state do not cover this head; human maintainers must decide their disposition and merge readiness.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
ARE404
left a comment
ARE404
left a comment
There was a problem hiding this comment.
Approving at head 59cbf4f0d1 — the two P1s (and the third gate P1) that I agreed block are now fixed in-head, and CI is green at this exact head. No P0/P1.
- Linux targets build (P1#1):
d9b57b3setslinux.executableName(no more scoped-name leak into the path) and the package URL/homepage source;package-linuxCI is green. - Deb is verified, not just hashed (P1#2):
9773ef6verifies the deb payload, deepened by98cc58b(verify Linux as deeply as macOS/Windows); the verifier no longer writes a checksum for an unverified body. - Linux PR gate integrates with the lane policy (P1#3):
7f9b1479holdsrelease-linux-checkto the repository's CI-policy contract (thenot ok 45/46/52policy-suite failures are gone). - Test red is cleared: the
desktop-release-targets.test.mjsfailure was fixed by59cbf4f0(copy builder config through JSON) —testis now green at the exact head.
CI at 59cbf4f0: test✅ package✅ package-linux✅ (new, proves the Linux release path actually packages) audit✅ owner✅. The review threads are substantively resolved by the referenced fix commits (toggles not flipped on GitHub, but the code + CI confirm the substance). Approving on code + green CI at the exact head.
|
All six checks are green on Closing the question I left open above: the deb's One correction to the previous push: the contract test copied the builder configuration with @M4n5ter @ARE404 — re-review requested; the head now meets the condition ARE404 set. |
M4n5ter
left a comment
M4n5ter
left a comment
There was a problem hiding this comment.
English
Reviewed exact head 59cbf4f0d1affc2cfb88dd2d77416830669c380e.
Technical verdict: not ready to merge. I found three blocking correctness issues and one important release-procedure issue. The previously reported Linux packaging, deb verification, CI-policy, and desktop-identity issues are resolved.
-
Blocking — the formal release cannot pass Finalize.
release.ymlmergeslatest-mac-arm64.ymlandlatest-mac-x64.ymlintolatest-mac.ymlonly inside the publish job's temporaryrelease-assetsdirectory, then uploads the publication record but not that normalized artifact set.release-cli-finalize.ymllater downloads the originalrelease-*runner artifacts and checks them against the record. The result is deterministic:missing latest-mac.yml; unexpected latest-mac-arm64.yml, latest-mac-x64.ymlFinalize therefore fails before attestation/publication. The minimal root fix is to upload the merged and verified normalized set as an immutable artifact and have Finalize consume that set, not reconstruct its input from raw runner groups.
-
Blocking — every newly added update target is rejected by the application's provenance gate after download.
apps/desktop/src/main/app-update-attestation.ts:85-92accepts only the macOS arm64 ZIP and Windows x64 EXE. The production update path invokes this verifier for every packaged application, so macOS x64 and Linux x64/arm64 end in a download error even when the feed, payload, and attestation are valid.This cannot be fixed safely with only another
platform/arch -> filenamescalar mapping: Linux may select either AppImage or deb for the same tuple, and electron-updater may select the arm64 ZIP while an x64 process is running under Rosetta. The verifier should validate the actual selected payload identity against the allowed target/package set and its exact attestation subject. Please cover every new target/package plus the Rosetta selection case. -
Blocking — the real Linux PR lane never validates the update feed it generated.
release-linux-check.ymlruns real packaging andverify:linux, butverify-linux.mjsnever reads the merged feed. The merge checks only version agreement and duplicate URLs. A wrong URL,path, size, or SHA-512 therefore leavespackage-linuxgreen and is first rejected during Nightly/formal publication. Synthetic staging fixtures and the descriptor-vs-builder test do not exercise electron-builder's actual feed bytes.verifyLinuxReleaseshould resolve the target feed/advertised payloads from the descriptor and runverifyDesktopUpdateArtifactsagainst the real merged feed before issuing checksums, with negative tests for URL/path/size/digest drift. -
Important — the release checklist's exact-manifest command now reports the wrong set.
.github/RELEASE_CHECKLIST.mdtells operators to compare the Draft withnode scripts/product-release-artifacts.mjs list, butlistprints raw runner groups containing the per-architecture macOS feeds. The Draft and publication record contain the merged feed instead, so that manual step cannot pass as written.listshould reportreleaseAssets(or expose a separate explicit published-set command), and “both Desktop platforms” should be updated to all three platforms.
Non-blocking simplification: the Nightly and formal workflows independently repeat the same five-row target matrix even though the descriptor already owns target/platform/architecture. Consider adding runner selection to the target plan, emitting one matrix projection from the identity jobs, and consuming it with fromJSON(...). This removes two production target authorities while preserving native runners, package/verify boundaries, exact-set checks, feed merging, and provenance.
Validation performed: the exact Finalize set mismatch and the three unsupported new platform/architecture cases were reproduced directly; the installed electron-updater 6.8.9 macOS architecture and Linux package-format selection paths were inspected and exercised; 125/126 focused contract tests passed, with the sole local failure caused by the isolated worktree lacking a prebuilt unrelated Eval dist directory. All hosted checks are green at this head, but none exercises the first two broken handoffs or validates the real Linux feed.
Automated review notice: this review was prepared by an AI review agent and posted at the direction of M4n5ter. It does not replace maintainer judgment.
简体中文
审查的精确 head 是 59cbf4f0d1affc2cfb88dd2d77416830669c380e。
技术结论：目前不适合合并。确认三个阻塞性 correctness 问题和一个重要的发布流程问题。 此前报告的 Linux 无法打包、deb 未验证、CI policy 以及桌面 identity 问题均已解决。
-
阻塞 — 正式发布必然无法通过 Finalize。
release.yml只在 publish job 的临时release-assets目录中将latest-mac-arm64.yml、latest-mac-x64.yml合并成latest-mac.yml，随后仅上传 publication record，没有保存这组规范化产物。release-cli-finalize.yml之后重新下载原始release-*runner artifacts 并与 record 做精确集合校验，结果必然是：missing latest-mac.yml; unexpected latest-mac-arm64.yml, latest-mac-x64.yml因此 Finalize 会在 attestation/publication 之前失败。最小根因修复是把合并并验证后的规范化产物集合上传为 immutable artifact，让 Finalize 直接使用它，不再从原始 runner groups 重建输入。
-
阻塞 — 所有新增更新目标在下载后都会被应用自身的 provenance gate 拒绝。
apps/desktop/src/main/app-update-attestation.ts:85-92仍只允许 macOS arm64 ZIP 和 Windows x64 EXE。所有 packaged app 都会走该 verifier，因此 macOS x64、Linux x64/arm64 即使 feed、payload、attestation 全部正确，也会变成下载错误。这里不能只增加简单的
platform/arch -> filename映射：Linux 同一 tuple 可能选择 AppImage 或 deb；Rosetta 下运行的 x64 进程也可能由 electron-updater 选择 arm64 ZIP。应验证 updater 实际选择的可信 payload identity 是否属于允许的 target/package 集合，并匹配精确 attestation subject；测试应覆盖所有新增 target/package 及 Rosetta 选择路径。 -
阻塞 — 真实 Linux PR lane 从未验证它实际生成的 update feed。
release-linux-check.yml会真实打包并执行verify:linux，但verify-linux.mjs不读取合并后的 feed；merge 只检查版本一致和重复 URL。URL、path、size 或 SHA-512 错误时，package-linux仍然全绿，直到 Nightly/正式发布阶段才首次失败。人工 fixture 和 descriptor-vs-builder 测试都没有验证 electron-builder 实际生成的 feed bytes。verifyLinuxRelease应从 descriptor 解析 target feed 和 advertised payloads，在签发 checksum 前对真实 merged feed 调用verifyDesktopUpdateArtifacts，并加入 URL/path/size/digest 漂移的负测。 -
重要 — Release checklist 的 exact-manifest 命令现在输出的是错误集合。
.github/RELEASE_CHECKLIST.md要求用node scripts/product-release-artifacts.mjs list核对 Draft，但list输出的是含 per-architecture macOS feeds 的原始 runner groups；Draft 和 publication record 使用的是 merged feed，因此该人工步骤按当前文字不可能通过。list应输出releaseAssets（或新增明确的 published-set 命令），并将 “both Desktop platforms” 更新为三个平台。
非阻塞简化建议： Nightly 和正式发布工作流分别重复维护相同的五行 target matrix，而 descriptor 已经拥有 target/platform/architecture。可以把 runner 纳入 target plan，由 identity jobs 输出统一 matrix projection，再通过 fromJSON(...) 消费。这样能删除两套生产 target authority，同时保留原生 runner、package/verify 边界、exact-set、feed merge 和 provenance。
验证情况：已直接复现 Finalize 集合不匹配以及三个新增 platform/architecture 被拒绝；检查并执行了已安装 electron-updater 6.8.9 的 macOS 架构选择和 Linux 包格式选择路径；定向契约测试 125/126 通过，唯一失败是隔离 worktree 缺少与本 PR 无关的 Eval 预构建 dist 目录。当前 head 的 hosted checks 全绿，但没有任何一个覆盖前两个断裂的 handoff，也没有验证真实 Linux feed。
自动化审查说明：本 review 由 AI 审查代理准备，并由 M4n5ter 指示发布；不能替代维护者判断。
The attestation verifier held a hand-written platform/arch to filename table that named only the macOS arm64 ZIP and the Windows installer, so every target this branch adds ended in a download error with a valid feed, payload and attestation. The table was also a second authority for what is built, and could not be completed: electron-updater serves the arm64 ZIP to an x64 build under Rosetta, and one Linux tuple installs either the AppImage or the deb. Identify the download by the name electron-updater gave it, require that name among the feed entries the updater was offered and under the released version, and keep the attestation subject as the trust anchor.
Merging the per-architecture macOS feeds split the set each runner uploads from the set that is published, but the merged result lived only in the publish job's working directory. Finalize rebuilt its input from the raw runner artifacts and failed the exact-set check deterministically (missing latest-mac.yml; unexpected latest-mac-arm64.yml, latest-mac-x64.yml), and would have attested the per-architecture feeds rather than the one users download. The checklist's `list` command printed the same stale set. Upload the verified release-assets directory as a run artifact, have Finalize consume it instead of reconstructing it, and make `list` report the published set.
Linux is the one platform whose feed this repository assembles itself, and the merged bytes were first read at publication. Verify them in the Linux lane against the payloads it just accepted.
|
Thanks @M4n5ter — three of the four reproduced, and two of those are regressions this branch introduced. Addressed in 1. Finalize exact-set mismatch — verified, regression by this branch, fixed in 2. Update verifier rejects every new target — verified, fixed in 3. Linux lane never validates its feed — the fact is right, the parity framing is not; addressed in 4. Checklist Matrix projection — agreed it is real, deferred. Both policy tests currently hold the workflows' literal matrices to the descriptor as an independent cross-check, and Not verifiable here: the artifact handoff in #1 is reasoned from |
Reading a feed goes through the yaml package, so the suite can no longer run in the planner step that has only node builtins.
The harness asserted that a function with no write path wrote nothing, and the attestation suite restated the descriptor's payload names that desktop-release-targets.test.mjs already pins to electron-builder.
…criptor The target descriptor was declared the one authority on what each desktop target builds, advertises and checksums, but only the Linux scripts read it: macOS and Windows packaging still spelled their own payload and feed names, and their verifiers hard-coded which files get a checksum while the descriptor's `checksums` mirrored them by hand. Resolve every name from the descriptor, and let the verifiers take an architecture the way `verify:linux` does.
…atrix target key
The identity's `exe` output and ten inline spellings of the installer
name were the Windows twin of the macOS `dmg` output this branch already
removed; the workflows now discover the installer the way they discover
the DMG, and verify it through the descriptor. The matrix `target` key
was always `${platform}-${arch}`, which both policy tests asserted.
Six scripts repeated the same lookup — manifest version, nightly override, descriptor entry, release directory — and the macOS autoupdate step still spelled its ZIP in YAML. One resolver owns it now, and the runner-label assertions check that each matrix row's runner suits its platform and architecture instead of restating the list.
A checksum is issued only for a payload the verify script opened; the macOS script opens the DMG alone.
|
Follow-up to the round above: after the fixes landed I ran a simplification audit over the whole branch, since 43 files was more than "add three targets" should cost. Six findings, all landed, all six checks green at The theme was the same as the review findings: the target descriptor had been declared the one authority on what each target builds, advertises and checksums, but only the Linux scripts honoured it. So:
Deliberately kept, with the reason written in the code: the Net over the branch: 45 files, +2433 −409. Roughly 40% of the additions are contract tests; four of the six defects the review found are now caught by one of them in milliseconds on any host. |
M4n5ter
left a comment
M4n5ter
left a comment
There was a problem hiding this comment.
English
Reviewed exact head 1c3c796f2dc5b14227f6e52de6224978aec7e1e2.
The four findings from the previous review are resolved: Finalize now consumes the normalized published asset set; update attestation follows the payload selected by electron-updater and verifies its exact signed subject; Linux validates its real merged feed before issuing checksums; and the checklist lists releaseAssets.
One blocking release-path issue remains: the real Linux Nightly and formal Release jobs launch the renderer without a display server.
scripts/verify-linux.mjs:199 starts the packaged Electron renderer. The dedicated PR lane correctly runs xvfb-run -a npm run verify:linux -- x64 at .github/workflows/release-linux-check.yml:128, and its own comment states that Xvfb is required for this renderer smoke. However, .github/workflows/release.yml:294 and .github/workflows/desktop-nightly.yml:199 invoke npm run verify:linux directly on headless Ubuntu runners.
I reproduced the boundary with the installed Electron runtime after clearing DISPLAY and WAYLAND_DISPLAY; it exits immediately with:
Missing X server or $DISPLAY
The platform failed to initialize.
Consequently, both Linux architectures fail verification before their artifacts can be staged or uploaded even though the specialized PR check is green. The minimal fix is to run both production workflow steps under xvfb-run -a as well, and bind all three Linux verification call sites to that requirement in the workflow contract tests.
Correctness verdict: not ready until this path is fixed. Design verdict: acceptable.
Validation on a clean synthetic merge with current main@6c632b133: build:test, full workspace typecheck, git diff --check, and 130 focused release/workflow/updater tests passed. The no-display Electron failure was reproduced directly.
Automated review notice: this re-review was prepared by an AI review agent and posted at the direction of M4n5ter. It does not replace maintainer judgment.
简体中文
复核的精确 head 是 1c3c796f2dc5b14227f6e52de6224978aec7e1e2。
上次审查的四项问题均已修复：Finalize 现在消费规范化后的发布产物集合；更新 attestation 会跟随 electron-updater 实际选中的 payload 并核验其精确签名 subject；Linux 会在签发 checksum 前验证真实 merged feed；checklist 也已改为列出 releaseAssets。
仍有一个阻塞性的发布路径问题：实际 Linux Nightly 和正式 Release job 在没有 display server 的情况下启动 renderer。
scripts/verify-linux.mjs:199 会启动打包后的 Electron renderer。专用 PR lane 在 .github/workflows/release-linux-check.yml:128 正确执行 xvfb-run -a npm run verify:linux -- x64，其注释也明确说明 renderer smoke 需要 Xvfb。但 .github/workflows/release.yml:294 和 .github/workflows/desktop-nightly.yml:199 在 headless Ubuntu runner 上直接调用 npm run verify:linux。
我清除 DISPLAY 和 WAYLAND_DISPLAY 后，用已安装的同一 Electron runtime 直接复现，进程立即退出并报告：
Missing X server or $DISPLAY
The platform failed to initialize.
因此，即使专用 PR check 为绿，Linux 两种架构仍会在产物 stage/upload 之前的验证阶段失败。最小修复是在这两个生产 workflow 中同样使用 xvfb-run -a，并在 workflow contract tests 中约束三处 Linux verification 调用保持一致。
Correctness 结论：修复该路径前尚不适合合并。Design 结论：可接受。
在当前 main@6c632b133 的干净 synthetic merge 上，build:test、全 workspace typecheck、git diff --check 以及 130 项定向 release/workflow/updater 测试均通过；无 display 的 Electron 失败已直接复现。
自动化审查说明：本次复核由 AI 审查代理准备，并由 M4n5ter 指示发布；不能替代维护者判断。
The verifier launches the extracted AppImage's renderer over CDP, and only the pull-request lane ran it under xvfb-run; the release and Nightly matrices called it bare, which fails on a runner without a display. Wrap all three, install xvfb the way ci.yml does, and hold every workflow to it. Alongside, the checks the review exposed as never executed now run over real functions: the Linux harness merges two single-document feeds with mergeDesktopUpdateFeeds before verifying, and a release test drives the publish job's merge and verify over a directory built from every staged group. The macOS verifier asserts the architecture it was asked for, and the three verifiers take the channel from the resolved target instead of re-reading the Nightly environment.
…arget A packaged build never reads the development peer target — it ships its own setup package — but both local callers evaluated it eagerly, and the whitelist has no darwin-x64 because no npm prebuild exists for it. An Intel Mac, which this branch now ships with its own native peer, would throw at first use of local remote access. Read the target only where a development build needs it; remote callers keep choosing their own.
…kage The feed-derived identity accepted any same-version artifact the release attested, including the CLI archive and other platforms' packages. Match the platform and format only — never the architecture, so a Rosetta process still accepts the arm64 ZIP — and read a legacy feed without a files list as offering nothing.
|
Before asking for another look I ran three independent adversarial reviews over the whole branch — one tracing every reader and writer of each invariant this branch changed, one a fresh-eye code review of the full diff, one mapping every changed file to where it actually executes. They found what the earlier rounds should have, and the shape is the one this PR keeps producing: a node changed, one of its edges not. Four commits, P0 — P1 — Intel Mac would throw at first use of local Runtime Host features. Executed now rather than reasoned about: the Linux harness merges two single-document feeds with the real Smaller: Not fixed, with reasons: the contract test deep-requires @M4n5ter — |
M4n5ter
left a comment
M4n5ter
left a comment
There was a problem hiding this comment.
English
LGTM. Re-reviewed exact head fc10401b58; the previous Xvfb finding is fully resolved across the PR, Nightly, and formal Release paths. No blocking issues found.
简体中文
LGTM。已复核精确 head fc10401b58；此前的 Xvfb 问题已在 PR、Nightly 和正式 Release 三条路径中完整修复。未发现阻塞问题。
5 participants
Summary
Desktop published only macOS arm64 and Windows x64, so Intel Macs and every Linux desktop had no build at all. Both the nightly and the formal release now package five targets — macOS arm64 and x64, Windows x64, and Linux x64 and arm64 — with Linux shipping an AppImage and a deb.
Each architecture builds on a runner of its own. The native Runtime Host peer and the packaged smoke probes are host binaries, so cross-building would embed the wrong architecture's addon and produce a package that builds green and dies on launch.
That makes one update feed carry several payloads for the first time, which is where most of the diff goes:
fileslist, but each architecture is packaged separately and writes a feed naming only its own zip. Packaging now names the macOS feed after its architecture so the two uploads cannot overwrite each other in the merged download, and publication merges them back into the single feed clients read.verifyDesktopUpdateArtifactstakes the full payload list instead of a single name, and requires a blockmap for every payload except the fpm targets, which ship without one.scripts/desktop-release-targets.mjsbecomes the one place that knows what a Desktop target produces, parameterised by channel and read by both publication paths. Because of it the nightly workflow no longer repeats artifact names in YAML (it callsdesktop-nightly.mjs stage-target) and the provenance step no longer counts subjects against a hardcoded8. Adding a target is now one edit.Linux inherits the existing non-darwin behaviour with no product change:
capability-snapshot.tsalready reports computer-use and TCC permissions as unsupported off macOS, which is how Windows has always shipped.Verification
node --testover the affected files —ci-test-plan.test.mjs,desktop-nightly*.test.mjs,product-release.test.mjs,product-release-authority.test.mjs: 107 tests, 106 pass.the Eval workspace owns the complete runtime asset declaration, reporting@maka/eval release file is missing: dist. It is environmental, not a regression: a freshly created worktree has no build output, and the same test passes 29/29 in a built checkout ofmain. CI builds before testing.npm run format:checkandbiome lint scripts/ apps/desktop/electron-builder.config.mjsare clean.MacUpdater.filterFilesForArchselects by architecture from thefileslist,getUpdateInfoFileNameapplies an architecture suffix only on Linux, andFpmTargetsetsisWriteUpdateInfoso the deb joins the Linux feed.Review focus
Two things this PR cannot prove locally, both of which the first CI run will:
macos-15-intelis not yet used anywhere in this repository's workflows.One deliberate coverage decision: the macOS end-to-end automatic-update proof runs on arm64 only. The updater is one code path across both architectures and
package-macos-autoupdate-next.mjsalready enforces that restriction, but it does mean the x64 job packages and verifies without exercising an update.Linux arm64 is built and verified on
ubuntu-24.04-arm, whose kernel uses 4K pages. Asahi-class installs use 16K pages, so a green CI run is not evidence that the arm64 AppImage runs there; that wants one manual check after the first published nightly.AI use
Select exactly one:
Tool(s) and scope: Claude Code (Opus) wrote the implementation, the tests, and this description, working from a plan agreed in conversation. The comparison of what other Electron desktop projects publish for Linux, and the electron-updater and electron-builder behaviour cited above, were checked against those projects' released assets and the installed package sources. The affected commit carries a
Generated-bytrailer.Checklist
Does this PR entail a change in behavior?
Review follow-up
Two P1 blockers were reported against
786c19e4and both are fixed here.Neither Linux target could be built. electron-builder derives the Linux executable name from the npm package name, and this one is scoped:
sanitizeFileName('@maka/desktop')is@makadesktop, which a desktop entry'sExec=cannot launch. It carries a scoped-package fix for the deb's package name and none for this one. fpm then refuses to run without a project homepage, which neither manifest declared and which cannot be recovered from the git remote on a fork.The deb was hashed, never read. The verifier opened the AppImage and applied every assertion to it, then issued the deb a checksum without reading it — a text file in its place left the gate green. Its comment explained why: both payloads came from one unpacked tree. That stopped being true in this branch, because keeping the deb's
package-typemarker out of the AppImage required splitting packaging into two electron-builder runs, and the second packs its own tree. Both are now extracted and asserted; the deb must additionally carrypackage-type=deband declare the architecture its filename claims, and checksums are issued only after every assertion has accepted the payload.The two defects share one cause: every claim this branch made about Linux was derived from reading electron-builder's sources, and none of them was ever executed.
release-windows-check.ymlexists because the Windows release path first ran on release day, on main, with a tag reserved — but this branch took packaging from two targets to five without extending that lane, so nothing could observe the Linux path before merge. Itspathslist even covers the two files changed here; the Windows lane ran and passed, because neither defect is observable on Windows.Both layers are now closed. The configuration failures are settled before electron-builder packages anything, so
desktop-release-targets.test.mjsdrives that resolution directly and catches both in 180ms on any host.release-linux-check.ymlcovers what a static test structurally cannot: fpm, the AppImage runtime, the native peer, and verification passing over real payloads.Remaining gap, stated plainly: macOS x64 still has no pull-request lane. It shares its scripts, configuration and descriptor with arm64, and the Nightly matrix builds and verifies it every night.
The earlier "Blocked on" note is obsolete — #4461 merged and this branch is rebased onto it.