Skip to content

feat(desktop): migrate Nightly to GitHub Releases - #4317

Merged
Astro-Han merged 5 commits into
apache:mainfrom
Astro-Han:feat/desktop-nightly-github-releases
Aug 31, 2026
Merged

feat(desktop): migrate Nightly to GitHub Releases#4317
Astro-Han merged 5 commits into
apache:mainfrom
Astro-Han:feat/desktop-nightly-github-releases

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • publish each Desktop Nightly directly as a versioned GitHub prerelease under its exact v<version> tag, with the macOS and Windows installers, blockmaps, dev-mac.yml, dev.yml, and offline Sigstore bundle
  • switch packaged Nightlies to the GitHub dev update channel so the app continues to detect, download, verify, and install newer Nightlies automatically, including differential blockmap lookup
  • remove the ASF Nightlies compatibility bridge, SSH/rsync transport, cutover marker, duplicate feeds, index, and their tests; existing testers reinstall the newest GitHub prerelease once
  • point both READMEs at GitHub Releases and keep CI artifacts as workflow-internal transport only, not a user distribution channel
  • fail closed on source/tag drift, partial or tampered drafts, unexpected assets, and remote digest mismatches without weakening the formal release path

Verification

  • npm run check:release — 141 tests passed
  • focused Desktop Nightly suite — 18 tests passed
  • node --test apps/desktop/dist/main/__tests__/app-update-attestation.test.js apps/desktop/dist/main/__tests__/app-update-service.test.js — 18 tests passed
  • npm run lint
  • npm run format:check
  • actionlint .github/workflows/desktop-nightly.yml
  • git diff --check
  • Desktop build:main, build:preload, build:overlay, and build:renderer passed on the rebased exact head
  • independent adversarial review found a normal-path packaging mismatch: the Nightly builder generated dev*.yml while the platform wrappers required latest*.yml; both wrappers now select metadata from the resolved build identity and have direct Nightly coverage
  • the Simplify audit confirmed that CI artifacts are transient cross-job transport rather than a second distribution authority; it also removed the redundant post-publication Latest API query

Rollout

  1. Enable GitHub Immutable Releases for apache/maka before distributing Desktop Nightlies.
  2. Set DESKTOP_NIGHTLY_ENABLED to true, dispatch a fresh npm Nightly from main, and verify the Desktop workflow publishes one non-Latest prerelease with the exact tag and nine assets.
  3. Ask existing ASF Nightlies testers to install the newest GitHub prerelease once.
  4. Publish one later fresh Nightly and confirm in-app automatic and differential updates on macOS and Windows.

Each Nightly remains a separate versioned prerelease. Reusing one mutable prerelease would conflict with immutable publication, updater version history, differential blockmaps, and rollback. Retention is intentionally left as a later operator policy rather than adding a second cleanup mechanism in this PR.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented and simplified the migration, added tests and operational documentation, ran the verification, and coordinated independent adversarial review plus a Simplify audit. The commits include the required Generated-by: Codex trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Lint, format, release checks, affected Desktop builds, and affected suites pass. The unchecked item preserves the repository-wide typecheck caveat recorded earlier for unrelated packages/ui sources.

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/XL Over 1000 readable lines label Aug 31, 2026
@Astro-Han
Astro-Han force-pushed the feat/desktop-nightly-github-releases branch from 93be1f8 to 69eaca6 Compare August 31, 2026 08:06
@Astro-Han
Astro-Han force-pushed the feat/desktop-nightly-github-releases branch from 69eaca6 to 6af9173 Compare August 31, 2026 08:53
@Astro-Han
Astro-Han marked this pull request as ready for review August 31, 2026 08:54

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the latest head 6af9173a8. I am not approving: the package check is red and this PR caused it. You said approve only if there is no P0/P1, so I am reporting instead. The cause is small and the fix is two lines — details inline.

First, the part that matters most, and it is good news. This migration removes the rsync/SSH publish path entirely — there is no rsync, no NIGHTLIES_RSYNC_*, and no StrictHostKeyChecking left in desktop-nightly.yml. That resolves the P1 I raised on #4254 (host key verification disabled on the Nightlies transport) by deleting the transport rather than patching it, and it also retires the --delete guard P3 I raised on #4271, #4280 and #4282, since those rsync steps no longer exist. Replacing an unverified SSH upload with GitHub Releases plus per-asset attestation (Attest every GitHub Nightly asset subject, add-attestation) is a materially stronger integrity story than what it replaces. Worth stating plainly: this PR closes my longest-standing open finding.

The blocker is a Windows path-separator bug in two new assertions. The package job fails on "Package the Windows installer and ZIP" with:

expected: /\/dev-mac\.yml$/u
actual:   'D:\a\maka\maka\apps\desktop\release\dev-mac.yml'

The packaging is correct — the right file is produced in the right place. The assertion is what is wrong: it requires a literal forward slash before the filename, and on a Windows runner the separator is a backslash, so it can never match. Both new assertFile callbacks have the same defect, so both the macOS-wrapper and Windows-wrapper tests fail. It would pass on a Linux-only run, which is presumably why it slipped through.

138 tests pass and only these 2 fail, so nothing else in the change is implicated.

I will re-review as soon as that is green.

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

Comment thread scripts/desktop-nightly.test.mjs

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one P1 and do not recommend merging this head yet.

The Windows release gate is failing because the two new wrapper assertions in scripts/desktop-nightly.test.mjs require POSIX / separators. On the Windows runner, the real paths end in \\dev-mac.yml and \\dev.yml, so both assertions fail even though the package metadata names are correct. This is already captured by the exact-head inline comment on the first assertion; I independently reproduced both failures and am not duplicating the same inline finding. Comparing basename(path) with the expected filename would make the contract platform-neutral.

I found no additional P0-P3 issues in the GitHub prerelease publisher, tag/source binding, exact asset/digest checks, dev update-channel configuration, or downloaded-update attestation path.

Validation completed on 6af9173a84ab11f33a10a7ff25faa028d43cccdc: clean install, full build, full typecheck, release contracts (141/141 on Linux), focused Nightly publisher/update tests, app update service and attestation tests, lint, formatting, diff check, and a clean merge tree against 6e6af952a6335fa904c1da7037ccc6269b36928a. The hosted package check fails with the two Windows path assertions; the hosted test check was still running at final refresh.

I could not execute signed/notarized macOS packaging, a real Nightly publication, or verify the repository's Immutable Releases setting from this environment.

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.

@zhiiw zhiiw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at exact head 6af9173a (verified unchanged at review time).

Not approving: the package check is red on this head. The Windows packaging job runs check:release, and the two new wrapper tests fail there with a POSIX-only path assertion — see the inline [P1]. The same failure reproduces on my Windows machine, so this is deterministic, not flake.

What I verified before the gate:

  • The migration removes the rsync chain cleanly: the SSH transport, feed-advance read, and dual-directory staging are gone; publication is now draft-prerelease → exact-asset verification → publish, with the draft made non-draft only after the complete verified asset set is present (the feed-last equivalent holds).
  • Fail-closed properties are real: tag/source drift rejected via remoteProductTagCommit; a pre-existing draft with unexpected or changed assets is rejected by assertAssetSubset (name+size+digest); the final state must match the local bytes exactly, checked twice (PATCH response and a fresh re-view).
  • The channel split is coherent: packaged Nightlies use the GitHub dev channel (dev-mac.yml/dev.yml), formal releases keep latest* (the wrappers select by buildVersion === manifest.version), and attestation URLs now resolve to the release assets for every channel.
  • Executed on a real Windows machine: the nightly script suites 16/18 — the 2 failures are exactly the inline P1.

Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.

简体中文

不批:本 head 的 package 检查红——Windows 打包作业跑 check:release,两个新 wrapper 测试的 POSIX 路径断言在那里挂(详见行内 P1;我本机 Windows 确定性复现)。闸前已核实:rsync 链整体拆除干净;发布序列为 draft → 精确资产校验 → 转正式(feed-last 语义保持);fail-closed 属性真实(tag/源漂移拒、草稿资产子集校验 name+size+digest、终态双重核验);channel 划分一致(nightly 走 dev 频道、正式发布保留 latest*,wrapper 按 buildVersion===manifest.version 选择)。本机套件 16/18,2 个失败即行内这条。

Comment thread scripts/desktop-nightly.test.mjs Outdated

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

English

Approved at d284fca.

I found no unresolved blocking correctness, security, recovery, or design issues. The previous Windows path-separator failure is correctly fixed with basename(), and the focused Nightly suites pass 18/18.

One non-blocking simplification suggestion: .nightly-input is validated and then copied unchanged into .nightly-stage/release. The workflow could validate, attest, add the bundle, and publish directly from the downloaded directory. This would remove one redundant asset representation and its rm/mkdir/copyFile path without weakening the exact-set, metadata, attestation, or final digest checks.

The publication authority, exact tag/source binding, draft-before-publish sequence, GitHub dev update channel, differential metadata, and downloaded-update attestation all look coherent.

Real signed/notarized publication and two consecutive Nightly updates remain rollout validation tasks; GitHub Immutable Releases must also be enabled before distribution. These do not block approval of this code change.

Automated review notice: This review was produced by an AI-assisted review agent operated by M4n5ter. It does not replace independent human review.

简体中文

已审查最新 head d284fca,批准。

没有发现未解决的 correctness、安全、恢复或设计阻塞问题。此前 Windows 路径分隔符导致的失败已通过 basename() 正确修复,Nightly 定向测试 18/18 通过。

有一个非阻塞的简化建议:.nightly-input 通过验证后,被原样复制到 .nightly-stage/release。可以直接在下载目录完成验证、attestation、加入 bundle 和发布,从而删除一套重复的资产表示以及对应的 rm/mkdir/copyFile 路径,同时保留 exact-set、metadata、attestation 和最终 digest 校验。

发布 authority、精确 tag/source 绑定、先 draft 后公开、GitHub dev 更新通道、差分更新 metadata 和下载后 attestation 验证均保持一致。

真实签名/notarization 发布和连续两个 Nightly 的自动/差分更新仍需在 rollout 阶段验证;正式分发前也必须开启 GitHub Immutable Releases。这些不阻塞本次代码审批。

自动化审查说明: 本审查由 M4n5ter 操作的 AI 辅助审查代理生成,不能替代独立人工审查。

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at head d284fca86. The P1 is resolved — approving. All six checks are terminal green, including the package job on Windows that was failing.

The fix is correct and does not trade strength for portability, which is the thing I check first when a red test turns green:

- if (path.endsWith('.yml')) assert.match(path, /\/dev-mac\.yml$/u);
+ if (path.endsWith('.yml')) assert.equal(basename(path), 'dev-mac.yml');

basename removes the path-syntax dependency entirely, and exact equality on the filename is stricter than the suffix match it replaces — the old regex would have accepted weird-dev-mac.yml, this will not. No assertion was deleted, no test skipped, and the commit touches three lines in one test file and nothing else. Both callbacks were fixed the same way.

The rest of the change is unchanged from 6af9173a8, so my earlier review stands. Restating the part that matters: this migration removes the rsync/SSH publish path entirely, which resolves the #4254 P1 about disabled host key verification by deleting the transport rather than patching it, and retires the --delete guard P3 I raised on #4271, #4280 and #4282 along with it. GitHub Releases with per-asset attestation is a stronger integrity story than what it replaces.

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

@zhiiw zhiiw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at the current head d284fca8 (verified unchanged at review time; all checks completed/success on this head, including the previously red package at 22m18s and test at 24m34s).

The delta from my earlier review (6af9173a) is one test-only commit (+3/−3): both wrapper assertions now compare basename(path) instead of a forward-slash-anchored regex — exactly the platform-neutral fix, no production change. The previously failing suite passes on the same Windows machine that failed before (8/8), and the hosted Windows packaging job is green again.

The earlier verification stands unchanged: the rsync chain is cleanly removed, publication remains draft → exact-asset verification → publish with fail-closed tag/asset/digest checks, and the dev/latest channel split is coherent.


Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.

简体中文

增量只有一个 test-only 提交(+3/−3):两处断言改用 basename 比较,正是行内 P1 建议的修法,生产代码未动。先前失败的套件在之前失败的同一台 Windows 机器上 8/8 过;本 head 的 package(之前红)与 test 全绿。此前对迁移主体的核实结论全部保留。

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head d284fca8664aa6fb69e091788fc9968a0c1279cf. The previous P1 is fixed: both Nightly wrapper assertions now compare basename(path) with dev-mac.yml / dev.yml, so the contract is independent of platform path separators. The fix delta is limited to that test file, and the current Windows package job now passes.

I found no remaining P0-P3 issues in the complete current change. The GitHub prerelease publisher still binds the exact tag and source commit, validates the exact remote asset set and digests before publishing, keeps Nightly packages on the dev update channel, and verifies downloaded-update attestation from GitHub Release assets.

Validation on this head: clean install; build:test; full production build; full workspace typecheck; complete release gate 141/141; focused Nightly suites 18/18; full lint, format, ASF-header, and diff checks; clean merge tree against current main; and all current hosted checks terminal green. I could not independently execute signed/notarized macOS packaging, perform a real Nightly publication, or verify the repository Immutable Releases setting.

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.

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at exact head d284fca8664aa6fb69e091788fc9968a0c1279cf. I found no P0–P3 issue in the current source.

The previous Windows-only release-test failure is closed by comparing basename() with dev-mac.yml / dev.yml; all exact-head hosted checks are now green, including the complete Windows packaging and update job. The Nightly publisher binds the exact npm version, source commit, protected tag, nine release assets, and remote SHA-256 records before one draft-to-prerelease mutation. Packaged Nightlies use the GitHub dev channel and verify downloaded bytes against the Nightly workflow's Sigstore identity.

The migration also removes the SSH/rsync credentials and transport, cutover marker, mutable dual-feed advancement, and Apache Nightlies compatibility path without introducing another distribution authority. Both prior review threads are resolved, and the PR is mergeable and clean against current main.


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.

@Astro-Han
Astro-Han merged commit c54092e into apache:main Aug 31, 2026
6 checks passed
Astro-Han added a commit that referenced this pull request Sep 1, 2026
`check:release` conflicted on its single line: main added
`scripts/desktop-nightly-release.test.mjs` (#4317) and
`scripts/qualify-released-cli-state-root.test.mjs`, while this branch removed
the `npm run check:asf-npm` chain. Both suites stay; the duplicate chain
stays removed.

Generated-by: Claude Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Over 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants