ci: publish cross-platform portable differential updates - #2439
Conversation
Walkthrough新增 Windows-only 手动桌面构建入口，并扩展可复用工作流以支持动态平台矩阵、签名控制、Portable 更新资产、Nightly/稳定版校验及本地发布脚本喵。 Changes桌面构建与发布
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsRepository analysis: Couldn't refresh Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile Summary这个 PR 将桌面构建工作流改造为可复用的
Confidence Score: 4/5The local build script will crash on first adoption if The workflow restructuring and signing pipeline are well-covered by contract tests and the publish script CDN verification logic is solid. The one defect is Get-PreviousManifest in the local build script: it calls gh release download --pattern via Invoke-Checked, which aborts on zero matches, meaning the very first stable build will fail with a cryptic gh error whenever a maintainer passes -PreviousReleaseTag for a release that predates portable manifest support. Files Needing Attention: scripts/build-desktop-release.ps1 — specifically the Get-PreviousManifest function and its handling of gh release download when the previous release has no matching manifest assets.
|
| Filename | Overview |
|---|---|
| .github/workflows/build-desktop-windows.yml | New Windows-only workflow_dispatch entry point that delegates to build-desktop.yml with windows_only: true and secrets: inherit; straightforward and correct. |
| .github/workflows/build-desktop.yml | Major refactor to a reusable workflow_call with parameterized matrix (via fromJSON toggle), portable update pipeline, and differential manifest signing; one inline ${{ github.sha }} remains in the Windows nightly step against the new env-var pattern. |
| .github/workflows/sync-update-release.yml | New validation-only workflow triggered on stable release publication; checks all required portable assets and signatures without touching the update service or CDN. |
| scripts/build-desktop-release.ps1 | New local stable-release build script; Get-PreviousManifest uses Invoke-Checked gh release download --pattern which throws when no asset matches, unlike the CI workflow's graceful "full package only" fallback; all other logic (env cleanup, signing, staged-output guard) is correct. |
| scripts/publish-desktop-release-assets.ps1 | New CDN upload and mirror registration script; correctly validates local-vs-remote asset parity, enforces immutability on existing OSS objects via SHA-256 comparison, and verifies CDN bytes before mirror registration. |
| tests/unit/test_desktop_windows_workflow_contract.py | New contract tests covering signing input handling, matrix selection, env-var injection pattern, Windows-only nightly branching, and stable release validation; all referenced job/step names align with the current workflow YAML. |
| .gitignore | Adds !scripts/build-desktop-release.ps1 to un-ignore the new script whose basename matches the existing build* rule; publish-desktop-release-assets.ps1 is unaffected and correctly tracked without a negation. |
| docs/deployment/manual-desktop-release.md | New deployment guide documenting the full manual stable-release flow; accurately reflects the two-script model and correctly notes that sync-update-release.yml only validates assets. |
Sequence Diagram
sequenceDiagram
participant WD as workflow_dispatch / schedule
participant WW as build-desktop-windows.yml
participant BD as build-desktop.yml
participant NJ as nightly job
participant GH as GitHub Release
WD->>WW: trigger (windows_only)
WW->>BD: "workflow_call (windows_only=true)"
BD->>BD: version job
BD->>BD: build-python (win only matrix)
BD->>BD: build-electron (win only matrix)
BD->>BD: Download previous manifests
BD->>BD: create-portable-update.js
BD->>NJ: artifact upload
NJ->>NJ: Organize release files
NJ->>NJ: Sign Portable manifests (Ed25519)
NJ->>GH: gh release upload --clobber
Note over BD,GH: Full cross-platform tag build
WD->>BD: "push v* tag"
BD->>BD: All platforms (4 matrix entries each)
BD->>NJ: artifacts (all platforms)
NJ->>NJ: Sign all manifests
NJ->>GH: gh release create (full nightly)
Note over GH: Stable release publishing
GH->>GH: Maintainer publishes stable release
GH->>GH: sync-update-release.yml validates assets
Comments Outside Diff (1)
-
scripts/build-desktop-release.ps1, line 824-827 (link)Get-PreviousManifestcrashes instead of falling back on missing assetsInvoke-Checked gh release download ... --pattern "..."exits non-zero when no assets match the pattern. This causes the entire script to throw a cryptic error and abort. The first time a maintainer runs this script with-PreviousReleaseTagpointing at a release that predates portable manifest support, the script will crash instead of gracefully producing a full package — exactly the scenario that will occur on first adoption.The CI workflow handles this correctly: it uses
gh apito test for the URL first, and only downloads when the asset exists, otherwise printing "full package only". The local script has no equivalent guard inGet-PreviousManifest.
Reviews (15): Last reviewed commit: "fix: harden portable release publication" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: 3c53853b6e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/build-desktop.yml (1)
819-825: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winmacOS/Linux 也走包装脚本喵
npx electron-builder会跳过scripts/build-electron-distribution.js里的 Electron 版本、package-lock和 Wayland 补丁清单校验；这里建议和 Windows/Portable 一样改用该脚本，避免产出不一致的分发包喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-desktop.yml around lines 819 - 825, Update the “Build Electron app (macOS/Linux)” step to invoke scripts/build-electron-distribution.js instead of npx electron-builder, while preserving the existing matrix.electron_args, working directory, publish behavior, and environment variables so all platforms use the same validation and packaging flow.Source: Linked repositories
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/build-desktop.yml:
- Around line 819-825: Update the “Build Electron app (macOS/Linux)” step to
invoke scripts/build-electron-distribution.js instead of npx electron-builder,
while preserving the existing matrix.electron_args, working directory, publish
behavior, and environment variables so all platforms use the same validation and
packaging flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 10168a2c-b25b-48c0-9ea8-b799b2e9b332
📒 Files selected for processing (4)
.github/workflows/build-desktop-windows.yml.github/workflows/build-desktop.ymlmain_logic/asr_client/workers/openai.pytests/unit/test_desktop_windows_workflow_contract.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
|
Addressed the CodeRabbit macOS/Linux packaging nit in fb8034b: the normal distribution step now uses scripts/build-electron-distribution.js with the matrix platform and architecture arguments, so Electron/package-lock/Wayland manifest validation is shared with the Portable builds. Combined ASR + workflow contract tests: 36 passed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/test_desktop_windows_workflow_contract.py`:
- Around line 34-49: 更新
test_reusable_build_honors_signing_inputs_and_distribution_wrapper，使其解析 workflow
YAML，并按步骤名称提取对应的 mapping，而不是在完整文本中匹配字符串。将 Windows-only nightly 上传步骤的 if 与 run
联合断言，并将签名构建步骤的 if、run 和 env 中的 WIN_CSC_LINK、WIN_CSC_KEY_PASSWORD
联合断言，确保这些配置属于同一步骤且不会被注释或其它步骤中的字符串误通过。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c522809a-7704-41b9-9e1a-14401c51ca92
📒 Files selected for processing (2)
.github/workflows/build-desktop.ymltests/unit/test_desktop_windows_workflow_contract.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-desktop.yml
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0527ff020c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # Stable tags publish every self-managed Portable update channel. If a release was | ||
| # created manually first, upload --clobber updates its assets in place. | ||
| publish-stable-portable: | ||
| if: (github.repository == 'Project-N-E-K-O/N.E.K.O' || inputs.allow_fork_build) && startsWith(github.ref, 'refs/tags/v') && needs.build-electron.result == 'success' |
There was a problem hiding this comment.
Restrict this condition to stable semantic versions before publishing: a tag such as the advertised v1.0.0-beta.1 also satisfies refs/tags/v*, so the job creates a non-prerelease GitHub release and then posts that beta version to /stable/sync. When the update service is configured, stable-channel users can therefore be offered a prerelease build.
Useful? React with 👍 / 👎.
| run: | | ||
| set -euo pipefail | ||
| mkdir -p previous-portable | ||
| RELEASE_ENDPOINT="repos/${GITHUB_REPOSITORY}/releases/latest" |
There was a problem hiding this comment.
Resolve the latest release preceding the tag rather than querying releases/latest unconditionally. If the tag's GitHub Release was created manually before this workflow—as the publication code explicitly supports—or this workflow is rerun after publishing, that endpoint can return the current release; the first case produces no delta and the second compares against the current manifest instead of the version users are upgrading from, defeating differential updates for that release.
Useful? React with 👍 / 👎.
|
|
||
| jobs: | ||
| build-windows: | ||
| uses: ./.github/workflows/build-desktop.yml |
There was a problem hiding this comment.
Grant contents: write to this calling job or workflow. When the repository uses a read-only default GITHUB_TOKEN, this wrapper passes only that permission to the reusable workflow, so its Windows-nightly release upload fails even though the called job requests write access; GitHub's reusable-workflow documentation states that permissions “can be only downgraded (not elevated) by the called workflow.”
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build-desktop-release.ps1`:
- Around line 194-197: Update the build setup around $portableUpdateDirectory
and the later dist asset collection to clear the entire dist directory before
starting a new build, while retaining the existing existence check and
preservation guidance for dist/portable-update. Ensure stale artifacts cannot be
selected by the Where-Object asset-copy logic, and keep the cleanup scoped to
the pre-build phase.
- Around line 9-13: After determining buildPlatform in the release script,
validate Architecture against the platform’s actual build behavior: only allow
the supported architecture value for Windows/Linux, and reject unsupported
combinations before invoking build-electron-distribution.js or
create-portable-update.js. Ensure create-portable-update.js receives an
architecture value that matches the produced artifact, while preserving the
existing macOS architecture handling.
- Around line 186-192: 在 Linux 分支中更新对 `$package.build.linux` 的检查，改用
`$package.build.PSObject.Properties.Match('linux')` 等属性枚举方式判断属性是否存在，避免
StrictMode 下直接访问缺失属性抛出异常；保留现有缺失时添加 linux 配置及设置 maintainer 的行为。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d3221761-4035-4199-b3eb-96e77ea07eb6
📒 Files selected for processing (5)
.github/workflows/build-desktop.yml.github/workflows/sync-update-release.yml.gitignoredocs/deployment/manual-desktop-release.mdscripts/build-desktop-release.ps1
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
💤 Files with no reviewable changes (1)
- .github/workflows/build-desktop.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/test_desktop_windows_workflow_contract.py`:
- Around line 135-145: Strengthen
test_published_stable_release_is_the_only_update_service_sync_trigger by
asserting the workflow condition explicitly requires draft and prerelease to be
false, not merely that those fields are referenced. Extend the “Validate stable
release assets” assertions to cover the complete release matrix: Windows, Linux
x64 AppImage, ordinary Linux, macOS x64, and macOS arm64 manifest .sig
filenames.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 171f2e34-303c-4919-9035-2dcd7f7b55a9
📒 Files selected for processing (3)
.github/workflows/build-desktop.ymlscripts/build-desktop-release.ps1tests/unit/test_desktop_windows_workflow_contract.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
198caf8 to
a72c78e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/build-desktop.yml (1)
1038-1057: 🩺 Stability & Availability | 🔵 Trivial签名密钥缺失就直接炸掉整条 Nightly，记得提前把 secret 配好喵。
PORTABLE_UPDATE_MANIFEST_ED25519_PRIVATE_KEY现在是硬性必需项，没配就exit 1，会让 Nightly（含 windows_only 重跑）整体失败，不只是跳过签名这一步喵。这符合 linked repo 文档"客户端强制要求签名 manifest"的约定，只是提醒一下：合并前请确认这个仓库 secret 已经配置好，否则第一次跑 Nightly 就会全军覆没喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-desktop.yml around lines 1038 - 1057, Configure the PORTABLE_UPDATE_MANIFEST_ED25519_PRIVATE_KEY repository secret before merging so the “Sign Portable manifests” workflow step can run successfully. Keep the existing required-secret validation and failure behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-desktop.yml:
- Around line 820-838: Update the downstream Nightly release configuration to
match the Windows --dir build output: replace the N.E.K.O.exe (Portable) release
text with the generated N.E.K.O_<version>_win.zip Portable package name, and
remove obsolete *.exe upload patterns. Locate the relevant release description
and artifact upload configuration near the existing Windows build steps,
preserving other platform artifacts and messaging.
- Around line 1117-1133: Update the “Create or update Windows nightly release”
step to derive the release notes from inputs.skip_signing instead of hardcoding
“Unsigned.” When creating a new nightly release, use wording that accurately
identifies signed versus unsigned builds while preserving the existing version
and release flow.
- Around line 104-105: Update the version handling in the workflow step
containing the VERSION assignment so inputs.version is passed through the step’s
env configuration and referenced via a shell environment variable, rather than
interpolated directly into the run script. Preserve the existing empty-value
check and VERSION assignment behavior while preventing input contents from being
parsed as shell syntax.
---
Nitpick comments:
In @.github/workflows/build-desktop.yml:
- Around line 1038-1057: Configure the
PORTABLE_UPDATE_MANIFEST_ED25519_PRIVATE_KEY repository secret before merging so
the “Sign Portable manifests” workflow step can run successfully. Keep the
existing required-secret validation and failure behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 667d98e6-a05c-41bf-9985-9315aa967a8f
📒 Files selected for processing (6)
.github/workflows/build-desktop-windows.yml.github/workflows/build-desktop.yml.github/workflows/sync-update-release.yml.gitignoredocs/deployment/manual-desktop-release.mdscripts/build-desktop-release.ps1
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- docs/deployment/manual-desktop-release.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/test_desktop_windows_workflow_contract.py`:
- Around line 105-112: 补强 windows_nightly["run"] 的断言，验证 SIGNING_NOTE 会根据
REQUESTED_SKIP_SIGNING 的值分支设置为 Signed 或
Unsigned，而不是仅检查两段赋值文本同时存在。确保断言覆盖跳过签名和正常签名两种路径，并保留现有发布说明使用 SIGNING_NOTE 的校验。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3067faca-5c71-4bae-8486-14763c1baccd
📒 Files selected for processing (2)
.github/workflows/build-desktop.ymltests/unit/test_desktop_windows_workflow_contract.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-desktop.yml
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/sync-update-release.yml (1)
35-51: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win稳定版校验也必须要求 Portable 差分资产喵
当前缺少
N.E.K.O_${VERSION}_linux_x64_appimage_delta.bin仍会通过校验，但 AppImage 更新通道依赖该.bin差分包喵。
.github/workflows/sync-update-release.yml#L35-L51: 将 AppImage.bindelta（以及该发布契约要求的其余差分资产）加入required喵。tests/unit/test_desktop_windows_workflow_contract.py#L188-L195: 为同一组差分资产增加断言，防止校验清单再次缩减喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/sync-update-release.yml around lines 35 - 51, Extend the required asset list in .github/workflows/sync-update-release.yml at lines 35-51 to include the AppImage .bin delta and all other Portable delta assets required by the release contract. Update tests/unit/test_desktop_windows_workflow_contract.py at lines 188-195 to assert the same complete delta-asset set, including N.E.K.O_${VERSION}_linux_x64_appimage_delta.bin, so the validation list cannot regress.Source: Linked repositories
🧹 Nitpick comments (1)
scripts/publish-desktop-release-assets.ps1 (1)
106-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win改用命名参数调用
Invoke-Checked喵PSScriptAnalyzer 已标记这两处定位参数调用喵。请显式使用
-FilePath和-Arguments，避免辅助函数签名调整后出现参数绑定歧义喵Also applies to: 112-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/publish-desktop-release-assets.ps1` at line 106, Update both Invoke-Checked calls in the asset upload flow to use explicit -FilePath and -Arguments named parameters instead of positional arguments, preserving the existing ossutil command, asset path, object URL, and metadata arguments.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/publish-desktop-release-assets.ps1`:
- Around line 86-114: 在发布校验流程中补充每个资产的 SHA-256（或等价远端摘要）内容比对，不能只比较名称和 CDN
首字节可达性；同时调整 `Invoke-Checked ossutil 'cp'` 的强制覆盖行为，禁止覆盖已有 immutable
对象，或在确需覆盖时显式执行缓存失效并确认 CDN 返回的新内容，确保同版本资产修正不会继续提供旧字节后仍注册元数据。
- Around line 74-94: 在发布资产校验流程中补充 manifest 与 Ed25519 .sig 的配对验证：对每个 manifest
要求存在对应签名文件，并拒绝没有对应 manifest 的孤立 .sig 文件；将此校验放在上传前、现有 $assets 与 $remoteAssetNames
集合比较之前，并沿用现有 throw 方式报告缺失或孤立资产。
- Line 121: 为发布脚本中的 Invoke-RestMethod 管理 API 注册请求设置有限的超时时间（例如 30
秒），并增加幂等重试机制；确保请求失败或超时后按既定次数重试，避免上传完成后的注册步骤无限阻塞。
- Around line 24-25: 修正发布脚本中 $Channel 与 latest 校验逻辑的不一致：在负责比较 $Tag 与 GitHub
/releases/latest 的校验流程中按 $Channel 分支处理，nightly 不应执行 stable 的 latest 比较；如果脚本仅支持
stable，则同步移除 ValidateSet、默认值及预发布标签相关输入契约中的 nightly 支持。
---
Outside diff comments:
In @.github/workflows/sync-update-release.yml:
- Around line 35-51: Extend the required asset list in
.github/workflows/sync-update-release.yml at lines 35-51 to include the AppImage
.bin delta and all other Portable delta assets required by the release contract.
Update tests/unit/test_desktop_windows_workflow_contract.py at lines 188-195 to
assert the same complete delta-asset set, including
N.E.K.O_${VERSION}_linux_x64_appimage_delta.bin, so the validation list cannot
regress.
---
Nitpick comments:
In `@scripts/publish-desktop-release-assets.ps1`:
- Line 106: Update both Invoke-Checked calls in the asset upload flow to use
explicit -FilePath and -Arguments named parameters instead of positional
arguments, preserving the existing ossutil command, asset path, object URL, and
metadata arguments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 41810e2d-b18f-4900-a812-d3e7b40dff8b
📒 Files selected for processing (4)
.github/workflows/sync-update-release.ymldocs/deployment/manual-desktop-release.mdscripts/publish-desktop-release-assets.ps1tests/unit/test_desktop_windows_workflow_contract.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/deployment/manual-desktop-release.md
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/publish-desktop-release-assets.ps1`:
- Around line 78-83: Update the catch block’s response detection in the retry
logic to inspect $_.Exception.PSObject.Properties['Response'] before accessing
the response value, avoiding direct $_.Exception.Response access under
StrictMode. Preserve the existing status-code extraction and retryable
conditions when the Response property exists.
In `@tests/unit/test_desktop_windows_workflow_contract.py`:
- Around line 216-226: Strengthen the assertions in the desktop Windows workflow
contract test to verify the publish path actually invokes the hash validation,
OSS object-existence check, and timeout configuration, in the required execution
order, rather than merely containing their definitions or command text. Replace
the narrow single-quote ValidateSet exclusion with a check that rejects the
equivalent validation in either quote style, or use PowerShell AST/Pester
assertions for these behavioral contracts.
- Around line 119-123: Strengthen the nightly asset assertions in the
full_nightly contract test so excluding the portable executable is validated in
the actual staging directory or upload manifest, not merely by checking that the
literal filename is absent from full_nightly["run"]. Preserve the existing
REQUESTED_SKIP_SIGNING and SIGNING_NOTE checks, and retain the allow-list
assertion for N.E.K.O_*_win.zip while ensuring N.E.K.O.exe cannot be selected
through wildcards or variables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1bdf80ab-7a6a-4846-ac02-7fe4a1baf921
📒 Files selected for processing (4)
.github/workflows/build-desktop.ymldocs/deployment/manual-desktop-release.mdscripts/publish-desktop-release-assets.ps1tests/unit/test_desktop_windows_workflow_contract.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/deployment/manual-desktop-release.md
- .github/workflows/build-desktop.yml
| catch { | ||
| $statusCode = $null | ||
| if ($_.Exception.Response) { | ||
| $statusCode = [int]$_.Exception.Response.StatusCode | ||
| } | ||
| $retryable = $null -eq $statusCode -or $statusCode -eq 408 -or $statusCode -eq 429 -or $statusCode -ge 500 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline/size =="
wc -l scripts/publish-desktop-release-assets.ps1 || true
rg -n "Set-StrictMode|function Invoke-UpdateMirrorSync|Invoke-UpdateMirrorSync|\.Response|TaskCanceled|catch" scripts/publish-desktop-release-assets.ps1 || true
echo
echo "== relevant section =="
sed -n '30,115p' scripts/publish-desktop-release-assets.ps1 | cat -nRepository: Project-N-E-K-O/N.E.K.O
Length of output: 1925
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
echo "== PowerShell strict property access probe =="
if command -v pwsh >/dev/null 2>&1; then
pwsh -NoProfile -Command '
Set-StrictMode -Version Latest
try {
$value = $null
try {
$null = (new-object Object).Response
} catch {
$value = $_.Exception.GetType().FullName
}
$value
} catch {
"Outer: $($_.Exception.GetType().FullName): $($_.Exception.Message)"
}
'
else
echo "pwsh not available"
fi
Length of output: 327
🌐 Web query:
PowerShell StrictMode PropertyNotFoundException accessing non-existent property returns null throws only StrictMode
💡 Result:
In PowerShell, the behavior when accessing a non-existent property depends on whether Set-StrictMode is enabled and the version configured [1][2]. Standard Behavior When Set-StrictMode is off (or set to version 1.0), attempting to access a non-existent property on an object returns $null without raising an error [3][2]. This is the default behavior in PowerShell, intended to allow for flexible, ad-hoc scripting [2]. Strict Mode Behavior When Set-StrictMode is set to version 2.0 or higher (including 'Latest'), PowerShell prohibits references to non-existent properties [1][4]. In this state, attempting to access a non-existent property causes a terminating error, typically manifesting as a PropertyNotFoundException [3][5][6]. Key details: - Version 1.0: Only restricts uninitialized variables (excluding those in strings) [1][2]. Accessing a non-existent property still returns $null [3][2]. - Version 2.0 and above: Prohibits references to uninitialized variables and non-existent properties, and enforces strict function call syntax [1][2][4]. - Scope: Set-StrictMode affects only the current scope (and its children) where it is defined, not the entire engine [2][6]. If you encounter this exception in your scripts, it indicates that StrictMode has been enabled (likely via Set-StrictMode -Version 2.0 or higher) in that scope [5][7]. If the goal is to safely check for a property's existence, you can use cmdlets like Get-Member to verify the property before access, rather than relying on the default $null return value [7]. [1][7]
Citations:
- 1: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/set-strictmode?view=powershell-7.4
- 2: https://devblogs.microsoft.com/powershell/strict-mode-v1-debug-tool-v1-v2-best-practice/
- 3: https://github.com/MicrosoftDocs/PowerShell-Docs/blob/main/reference/7.7/Microsoft.PowerShell.Core/Set-StrictMode.md
- 4: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/set-strictmode?view=powershell-7.5
- 5: https://stackoverflow.com/questions/53481174/accessing-non-existent-property-of-a-hashtable-behaves-differently-in-and-out-of
- 6: https://learn.microsoft.com/en-us/dotnet/api/microsoft.powershell.commands.setstrictmodecommand?view=powershellsdk-7.6.0
- 7: https://stackoverflow.com/questions/18901681/why-dont-errors-get-returned-when-calling-properties-that-dont-exist
🌐 Web query:
PowerShell PSObject Properties indexer accessing missing property StrictMode
💡 Result:
In PowerShell, accessing a non-existent property on an object normally returns $null [1][2][3]. However, when StrictMode (version 2.0 or higher) is enabled, referencing a non-existent property generates a terminating PropertyNotFoundException [2][3][4]. To safely access properties or check for their existence while StrictMode is enabled, you can use the PSObject.Properties collection [1][5]. Recommended Approaches: 1. Using PSObject.Properties for Existence Checks The PSObject.Properties collection provides a safe way to check if a property exists without triggering a StrictMode error [1][5]. Example: if ($myObject.PSObject.Properties['PropertyName']) { Write-Host "Property exists and is not null" } 2. Using Member Access with Null-Coalescing or Null-Conditional Operators If you are using PowerShell 7+, you can use the null-conditional operator (?.) to safely access properties [6]. This allows you to attempt to access a property and return $null if the object or the property does not exist, avoiding the StrictMode error. Example: $value = $myObject?.PropertyName 3. Checking via Get-Member You can also use Get-Member to verify a property's existence before accessing it [1][5]. Example: if ($myObject | Get-Member -Name 'PropertyName' -MemberType Properties) { $value = $myObject.PropertyName } Why StrictMode affects this: Set-StrictMode is designed to enforce best practices by prohibiting references to uninitialized variables and non-existent object properties [2][7]. When version 2.0 or higher (including 'Latest') is active, the engine explicitly converts what would otherwise be a silent $null return into a terminating error to help developers catch typos and logic errors early [3][8][4].
Citations:
- 1: https://forums.powershell.org/t/testing-for-property-existance/2300
- 2: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/set-strictmode?view=powershell-7.6
- 3: https://github.com/MicrosoftDocs/PowerShell-Docs/blob/main/reference/7.6/Microsoft.PowerShell.Core/Set-StrictMode.md
- 4: https://devblogs.microsoft.com/scripting/enforce-better-script-practices-by-using-set-strictmode/
- 5: https://stackoverflow.com/questions/26997511/how-can-you-test-if-an-object-has-a-specific-property
- 6: Check if a property exists on an object in a strict mode PowerShell/PowerShell#10875
- 7: https://devblogs.microsoft.com/powershell/strict-mode-v1-debug-tool-v1-v2-best-practice/
- 8: https://learn.microsoft.com/en-us/dotnet/api/microsoft.powershell.commands.setstrictmodecommand?view=powershellsdk-7.6.0
用 PSObject.Properties['Response'] 避开严格模式下的属性访问爆炸喵。
脚本开启了 Set-StrictMode -Version Latest，当异常没有 Response 属性时，$_ .Exception.Response 会先抛 PropertyNotFoundException，导致重试判断逻辑跳过喵。像建议的用法那样先通过属性集合探测是否存在再取值，可避免在 catch 里二次抛异常喵。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/publish-desktop-release-assets.ps1` around lines 78 - 83, Update the
catch block’s response detection in the retry logic to inspect
$_.Exception.PSObject.Properties['Response'] before accessing the response
value, avoiding direct $_.Exception.Response access under StrictMode. Preserve
the existing status-code extraction and retryable conditions when the Response
property exists.
| full_nightly = nightly_steps["Create nightly release"] | ||
| assert full_nightly["env"]["REQUESTED_SKIP_SIGNING"] == "${{ inputs.skip_signing }}" | ||
| assert 'SIGNING_NOTE="unsigned"' in full_nightly["run"] | ||
| assert "N.E.K.O_*_win.zip" in full_nightly["run"] | ||
| assert "N.E.K.O.exe" not in full_nightly["run"] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
补强 nightly 资产排除断言，避免通配符绕过喵
Line 123 只验证 N.E.K.O.exe 这个字面量不在脚本文本中；如果发布命令通过 release/* 或变量上传，单文件 Portable 可执行文件仍可能被上传，而测试照样通过喵。请断言实际暂存目录或上传清单明确排除该文件，并保留对 N.E.K.O_*_win.zip 的允许列表校验喵。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/test_desktop_windows_workflow_contract.py` around lines 119 - 123,
Strengthen the nightly asset assertions in the full_nightly contract test so
excluding the portable executable is validated in the actual staging directory
or upload manifest, not merely by checking that the literal filename is absent
from full_nightly["run"]. Preserve the existing REQUESTED_SKIP_SIGNING and
SIGNING_NOTE checks, and retain the allow-list assertion for N.E.K.O_*_win.zip
while ensuring N.E.K.O.exe cannot be selected through wildcards or variables.
Source: Linked repositories
| assert "function Test-OssObjectExists" in script | ||
| assert "function Get-Sha256" in script | ||
| assert "Get-FileHash -LiteralPath $Path -Algorithm SHA256" in script | ||
| assert "Portable manifest is missing its signature asset" in script | ||
| assert "Portable signature has no matching manifest" in script | ||
| assert "Refusing to overwrite immutable OSS object" in script | ||
| assert "--max-time', '1800'" in script | ||
| assert "Invoke-UpdateMirrorSync" in script | ||
| assert "-TimeoutSec 30" in script | ||
| assert "ValidateSet('stable', 'nightly')" not in script | ||
| assert "/stable/sync" in script |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
验证发布脚本的真实调用关系，而不是只搜字符串喵
Lines 216-224 只检查函数名和命令片段存在，无法证明哈希校验、对象存在性检查及超时参数实际被发布路径调用；Line 225 也只排除一种特定引号写法，等价的双引号写法仍会误通过喵。请改为校验调用点、参数和执行顺序，或使用 PowerShell AST/Pester 做行为级契约断言喵。Based on learnings：测试断言应至少与所声明验证的不变量同等强度喵。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/test_desktop_windows_workflow_contract.py` around lines 216 - 226,
Strengthen the assertions in the desktop Windows workflow contract test to
verify the publish path actually invokes the hash validation, OSS
object-existence check, and timeout configuration, in the required execution
order, rather than merely containing their definitions or command text. Replace
the narrow single-quote ValidateSet exclusion with a check that rejects the
equivalent validation in either quote style, or use PowerShell AST/Pester
assertions for these behavioral contracts.
Source: Learnings
2 participants
改动概述 / Summary
v*tag 构建后统一发布各平台 Portable 资产；Windows-only nightly 重跑只覆盖 Windows 资产，不删除其他平台文件。.links元数据导致的签名识别问题，并调整 OpenAI ASR worker 的异步任务清理以避免 Nuitka 运行时崩溃。回归报告 / Regression Report
main_logic/asr_client/workers/openai.py的 worker 清理阶段改为显式收集实际存在的 sender/receiver task，取消未完成任务后统一gather。None，逐个取消未完成 task 并加入列表，最后等待所有实际创建的 task 完成清理。closed事件发送。已运行完整tests/unit/test_asr_workers.py覆盖正常关闭、异常和取消路径。不拆分理由 / Why Not Split
不适用。计入上限的既有文件不超过 20 个；新增 workflow 与测试文件不计入该上限。
测试 / Testing
.\.venv\Scripts\python.exe -m pytest -q tests/unit/test_asr_workers.py tests/unit/test_desktop_windows_workflow_contract.py -p no:cacheprovider：36 passed。git diff --check：通过。Summary by CodeRabbit