Package meshtasticd for Windows as an MSI - #11289
Conversation
⚡ Try this PR in the Web Flasher
Warning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (31)
Build artifacts expire on 2026-08-29. Updated for |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughWindows builds now package MSI and Winget artifacts, validate release manifests, and support Portduino Windows service lifecycle reporting. Portduino also adds configuration-check CLI handling, while native build linker and source filters are updated. ChangesWindows packaging and service delivery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WindowsBuild
participant WiX
participant WingetBuilder
participant ArtifactStore
WindowsBuild->>WiX: Install WiX 5.0.2
WindowsBuild->>WingetBuilder: Build MSI and manifests with ReleaseTag
WingetBuilder->>WiX: Invoke wix build
WingetBuilder->>WindowsBuild: Return MSI and manifest paths
WindowsBuild->>ArtifactStore: Upload MSI and manifests
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Adds a --service flag connecting meshtasticd to the Service Control Manager, a WiX MSI installing it as an auto-start LocalSystem service with config in %ProgramData%\Meshtastic, and a CI step attaching the MSI to releases.
dfeeefe to
cd01592
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
bin/build-winget-package.ps1 (1)
92-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
New-DeterministicGuidPSScriptAnalyzer hint is a false positive; consider a test instead.The
PSUseShouldProcessForStateChangingFunctionswarning doesn't apply here — the function only computes an in-memory GUID and mutates no system state, soShouldProcess/-WhatIfsupport isn't meaningful. Given this routine's byte-order manipulation is intricate and directly determines whether major-upgradeProductCodes stay stable across rebuilds, a small Pester test with a known input/output pair would give more confidence than manual review alone.🤖 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 `@bin/build-winget-package.ps1` around lines 92 - 115, Add a focused Pester test for New-DeterministicGuid using a fixed namespace and name with a known expected GUID, covering the RFC 4122 byte-order and version/variant transformations. Do not add ShouldProcess or -WhatIf support, since the function only computes and returns an in-memory value.Source: Linters/SAST tools
.github/workflows/package_winget.yml (1)
36-40: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
Select-Object -First 1silently picks an arbitrary MSI if more than one exists.Fine today since only one architecture is published per release, but will silently mis-package if a second architecture MSI is ever attached to the same release tag.
🤖 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/package_winget.yml around lines 36 - 40, Update the “Generate manifests for the published MSI” step to validate that exactly one MSI exists under release/download before assigning $msi; fail the workflow with a clear error when zero or multiple files are found, and pass the single validated MSI to build-winget-package.ps1 without using Select-Object -First 1..github/workflows/build_windows_bin.yml (1)
145-163: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDirect template expansion into
run:(zizmor template-injection).Line 154 expands
${{ steps.version.outputs.long }}directly into the PowerShellrun:body. Lower risk here since the value is derived from repo build metadata rather than raw user input, but the standard hardening is to pass it viaenv:and reference$env:VARinstead. See the consolidated comment covering this pattern across files.🤖 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_windows_bin.yml around lines 145 - 163, The Build the MSI and winget manifests step directly interpolates steps.version.outputs.long into its PowerShell run script. Pass the version through the step’s env configuration and reference the environment variable in the build-winget-package.ps1 invocation, preserving the existing v-prefixed ReleaseTag value.Source: Linters/SAST tools
.github/workflows/main_matrix.yml (1)
531-538: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDirect template expansion into
run:(zizmor template-injection).Same pattern as flagged elsewhere:
${{ needs.version.outputs.long }}is expanded directly into thegh release uploadcommand body. See the consolidated comment for the shared fix.🤖 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/main_matrix.yml around lines 531 - 538, Update the “Add sources to GitHub Release” step so needs.version.outputs.long and the other version outputs are passed through environment variables or step inputs before the shell runs, rather than interpolated directly in the run script. Use those shell-safe variables in each gh release upload command while preserving the existing release tag and asset paths.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 @.github/workflows/package_winget.yml:
- Around line 27-40: Route all GitHub Actions expression values through step
environment variables before using them in run bodies: in
.github/workflows/package_winget.yml lines 27-40, bind inputs.release_tag and
inputs.version and reference them as PowerShell $env variables in both steps; in
.github/workflows/build_windows_bin.yml lines 145-163, bind
steps.version.outputs.long and use the corresponding $env variable; in
.github/workflows/main_matrix.yml lines 531-538, bind needs.version.outputs.long
and use that variable for the gh release upload invocation and shared sibling
lines. Preserve the existing command behavior while removing direct ${{ }}
interpolation from shell scripts.
In `@src/platform/portduino/windows/WindowsService.cpp`:
- Around line 20-39: Synchronize concurrent status reporting in reportStatus,
which is called by both serviceMain and controlHandler. Add the mutex include
and guard the shared checkPoint and serviceStatus updates through
SetServiceStatus with a mutex, ensuring each status snapshot and checkpoint
increment is serialized without changing the existing state behavior.
---
Nitpick comments:
In @.github/workflows/build_windows_bin.yml:
- Around line 145-163: The Build the MSI and winget manifests step directly
interpolates steps.version.outputs.long into its PowerShell run script. Pass the
version through the step’s env configuration and reference the environment
variable in the build-winget-package.ps1 invocation, preserving the existing
v-prefixed ReleaseTag value.
In @.github/workflows/main_matrix.yml:
- Around line 531-538: Update the “Add sources to GitHub Release” step so
needs.version.outputs.long and the other version outputs are passed through
environment variables or step inputs before the shell runs, rather than
interpolated directly in the run script. Use those shell-safe variables in each
gh release upload command while preserving the existing release tag and asset
paths.
In @.github/workflows/package_winget.yml:
- Around line 36-40: Update the “Generate manifests for the published MSI” step
to validate that exactly one MSI exists under release/download before assigning
$msi; fail the workflow with a clear error when zero or multiple files are
found, and pass the single validated MSI to build-winget-package.ps1 without
using Select-Object -First 1.
In `@bin/build-winget-package.ps1`:
- Around line 92-115: Add a focused Pester test for New-DeterministicGuid using
a fixed namespace and name with a known expected GUID, covering the RFC 4122
byte-order and version/variant transformations. Do not add ShouldProcess or
-WhatIf support, since the function only computes and returns an in-memory
value.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: aa1bafcd-069e-46ff-b52c-6bedb5a91c5a
📒 Files selected for processing (10)
.github/workflows/build_windows_bin.yml.github/workflows/main_matrix.yml.github/workflows/package_winget.ymlbin/build-winget-package.ps1packaging/windows/meshtasticd.wxssrc/main.cppsrc/platform/portduino/PortduinoGlue.cppsrc/platform/portduino/windows/WindowsService.cppsrc/platform/portduino/windows/WindowsService.hvariants/native/portduino/platformio.ini
caveman99
added
the
triaged
Bind workflow expressions to env vars in run: bodies, and build the service status per call with an atomic checkpoint.
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 `@src/platform/portduino/windows/WindowsService.cpp`:
- Around line 21-42: Update reportStatus to serialize concurrent status updates
and track when shutdown begins, ensuring that once SERVICE_STOP_PENDING is
reported, later startup or running reports are suppressed. Preserve stop-related
reporting and existing checkpoint behavior while preventing startup polling from
overwriting the shutdown state.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d2dabb70-38f0-4b18-b297-dd9685b4f198
📒 Files selected for processing (6)
.github/workflows/build_windows_bin.yml.github/workflows/main_matrix.yml.github/workflows/package_winget.ymlbin/build-winget-package.ps1packaging/windows/meshtasticd.wxssrc/platform/portduino/windows/WindowsService.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- bin/build-winget-package.ps1
- .github/workflows/package_winget.yml
Latch the stop under a mutex so a startup report cannot walk the state back. Ignore the new workflows in semgrep and checkov, as main_matrix already is.
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 @.trunk/trunk.yaml:
- Around line 112-116: Replace the broad Checkov file-level ignore for
package_winget.yml in the Trunk configuration with a targeted inline suppression
for checkov/CKV_GHA_7, using the existing workflow’s suppression convention.
Remove the linters: [checkov] path entry so other Checkov findings remain
enabled.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d3ab6a34-33f7-4f0f-9c32-41d2fd6c09f8
📒 Files selected for processing (3)
.semgrepignore.trunk/trunk.yamlsrc/platform/portduino/windows/WindowsService.cpp
| # CKV_GHA_7 wants workflow_dispatch inputs empty, but the manual winget validation run | ||
| # needs a tag and version. Ignored at file level to keep the workflow suppression-free. | ||
| - linters: [checkov] | ||
| paths: | ||
| - .github/workflows/package_winget.yml |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository files =="
git ls-files | rg '(^|/)(\.trunk/trunk\.yaml|package_winget\.yml)$|trunk|checkov' || true
echo
echo "== trunk.yaml around referenced lines =="
if [ -f .trunk/trunk.yaml ]; then
nl -ba .trunk/trunk.yaml | sed -n '90,130p'
else
echo ".trunk/trunk.yaml not found"
fi
echo
echo "== workflow referenced file existence/size =="
if [ -f .github/workflows/package_winget.yml ]; then
wc -l .github/workflows/package_winget.yml
nl -ba .github/workflows/package_winget.yml | sed -n '1,220p'
else
echo ".github/workflows/package_winget.yml not found"
fi
echo
echo "== checkov skip/suppression/docs references in repo =="
rg -n "ckv_gha_7|CKV_GHA_7|\.checkov|checkov skip|checkov-suppress|linters:\s*\[checkov\]" .trunk .github . 2>/dev/null || trueRepository: meshtastic/firmware
Length of output: 747
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant files =="
for f in .trunk/trunk.yaml .github/workflows/package_winget.yml .github/workflows/trunk_annotate_pr.yml .github/workflows/trunk_check.yml; do
if [ -f "$f" ]; then
echo "--- $f ($(wc -l < "$f") lines) ---"
sed -n '1,220p' "$f"
else
echo "--- $f not found ---"
fi
done
echo
echo "== search suppression/checkov references =="
grep -RInEi "ckv_gha_7|CKV_GHA_7|\.checkov|checkov skip|checkov-suppress|linters:\s*\[checkov\]|suppress-checkov|skip_checkov" .trunk .github . 2>/dev/null || trueRepository: meshtastic/firmware
Length of output: 9509
🌐 Web query:
Trunk linter ignore syntax file level all linters path ignored all rules linters [ALL]
💡 Result:
To ignore all rules for all linters in specific files within your Trunk configuration, you should use the ignore key in your .trunk/trunk.yaml file [1][2]. Under the lint section, you can define a list of ignored paths by specifying linters: [ALL] [1][3]. This tells Trunk to skip all linting checks for the files matching the provided glob patterns [1][2]. Example configuration in .trunk/trunk.yaml: yaml lint: ignore: - linters: [ALL] paths: - path/to/ignore/ - another/specific/file.ext Key points regarding this configuration: - The paths are relative to the root of your repository [1][2]. - You can use glob patterns to match multiple files or directories [1][2]. - If you need to include specific files within an ignored directory, you can use the ! prefix for an inverse ignore (e.g., - !path/to/ignore/important_file.ext) [1][2]. - Trunk is also git-aware and will automatically ignore files that are defined in your .gitignore [1][2]. If you only need to suppress all issues for a specific linter in a single file via code comments, you can use // trunk-ignore-all(<linter-name>) at the top of that file [1][4][2]. However, for a broad, rule-independent exclusion of all linters for specific paths, the trunk.yaml configuration above is the standard and recommended approach [1][3].
Citations:
- 1: https://docs.trunk.io/code-quality/overview/linters/ignoring-issues-and-files
- 2: https://docs.trunk.io/code-quality/overview/linters/ignoring-issues-and-files.md
- 3: https://docs.trunk.io/code-quality/overview/debugging
- 4: https://github.com/trunk-io/plugins/tree/main/linters/ruff
🌐 Web query:
Trunk code quality ignore all rules specific linter specific file trunk.yaml linters: [lint-name]
💡 Result:
To ignore specific linters for specific files or paths in Trunk, you should use the ignore key within your.trunk/trunk.yaml configuration file [1][2]. The ignore configuration allows you to define a list of rules, where each entry specifies the linters to ignore and the associated file paths [1][3]. Example configuration in.trunk/trunk.yaml: lint: ignore: - linters: [eslint] paths: - path/to/specific/file.ts - path/to/directory/ - linters: [ALL] paths: - src/generated/ Key details: - linters: You can provide a list of specific linter names (e.g., [eslint, black]) or use the special [ALL] tag to ignore all linters for the specified paths [1][2]. - paths: These are glob patterns relative to the root of your repository [1][3]. You can use! at the start of a path to create an inverse ignore (e.g., to exclude a subdirectory within an ignored path) [1][3]. If you only need to ignore issues on specific lines within a file, you can use inline comment directives instead of modifying the YAML config [1][4]. For example: // trunk-ignore(linter-name) const unusedVariable = 42; Or to ignore all issues on a specific line: // trunk-ignore const unusedVariable = 42; To ignore all issues in an entire file via comments, you can add this to the top of the file: // trunk-ignore-all(linter-name) Trunk is also git-aware and will ignore files listed in your.gitignore by default [1][3].
Citations:
- 1: https://docs.trunk.io/code-quality/overview/linters/ignoring-issues-and-files
- 2: https://docs.trunk.io/code-quality/overview/cli/getting-started/configuration.md
- 3: https://docs.trunk.io/code-quality/overview/linters/ignoring-issues-and-files.md
- 4: https://docs.trunk.io/code-quality/overview/deal-with-existing-issues
🌐 Web query:
Trunk config ignore multiple linter-specific entries paths can have multiple rules single linter path
💡 Result:
In Trunk, you can ignore specific files and paths for one or more linters by using the lint.ignore key within your .trunk/trunk.yaml configuration file [1][2]. Each entry in the lint.ignore list requires two keys: linters and paths [1][3]. linters: A list of linter names (e.g., [eslint, black]) or the special [ALL] tag to apply the ignore rule to every enabled linter [1][3]. paths: A list of glob patterns relative to the repository root [1][3]. You can define multiple ignore entries to handle different requirements, such as ignoring specific files for one linter while ignoring different paths for all linters [2]. Trunk also supports inverse ignores using the ! prefix, which ensures that files matching that glob will not be ignored even if they are caught by other ignore patterns [1][3]. Example configuration: lint: ignore: # Ignore generated files for all linters - linters: [ALL] paths: - src/generated/ - test/test_data # Ignore a specific file only for eslint - linters: [eslint] paths: - c/run.ts # Inverse ignore: ignore all files in a directory except specific ones - linters: [ALL] paths: - src/generated/ -!src/generated//*.foo Trunk is also git-aware and automatically ignores files specified in your.gitignore file by default [1][3].
Citations:
- 1: https://docs.trunk.io/code-quality/overview/linters/ignoring-issues-and-files
- 2: https://docs.trunk.io/code-quality/overview/getting-started/configuration
- 3: https://docs.trunk.io/code-quality/overview/linters/ignoring-issues-and-files.md
🌐 Web query:
Trunk trunk-ignore-checkov-specific-rule inline suppression
💡 Result:
To suppress a specific Checkov rule inline when using Trunk, you should use the Trunk-specific ignore syntax rather than the native Checkov comment syntax [1][2]. Trunk provides a standardized inline suppression mechanism that applies to all supported linters, including Checkov [1]. You can suppress a specific Checkov rule on a given line by adding the following comment: // trunk-ignore(checkov/CHECK_ID) Replace CHECK_ID with the specific Checkov rule ID (e.g., CKV_AWS_123) [1]. For example, to suppress the rule CKV_AWS_123: # trunk-ignore(checkov/CKV_AWS_123) resource "aws_s3_bucket" "example" {... } If you wish to suppress all Checkov issues on a specific line, you can omit the rule ID [1]: # trunk-ignore(checkov) resource "aws_s3_bucket" "example" {... } While native Checkov allows inline suppression using checkov:skip=<check_id> [3][4], using the Trunk-native trunk-ignore syntax is the recommended approach within the Trunk ecosystem as it ensures Trunk is aware of the suppression and can manage it consistently across your project [1].
Citations:
- 1: https://docs.trunk.io/code-quality/overview/linters/ignoring-issues-and-files.md
- 2: https://docs.trunk.io/code-quality/overview/deal-with-existing-issues
- 3: https://www.checkov.io/2.Basics/Suppressing%20and%20Skipping%20Policies.html
- 4: https://github.com/bridgecrewio/checkov/blob/main/docs/2.Basics/Suppressing%20and%20Skipping%20Policies.md
Scope the file-level Checkov ignore to CKV_GHA_7.
linters: [checkov] silences every Checkov rule for package_winget.yml, but this only exists to allow non-empty workflow_dispatch inputs. Add the targeted inline suppression used elsewhere, e.g. # trunk-ignore(checkov/CKV_GHA_7), and remove the broad file-level ignore so future workflow security findings remain visible.
🤖 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 @.trunk/trunk.yaml around lines 112 - 116, Replace the broad Checkov
file-level ignore for package_winget.yml in the Trunk configuration with a
targeted inline suppression for checkov/CKV_GHA_7, using the existing workflow’s
suppression convention. Remove the linters: [checkov] path entry so other
Checkov findings remain enabled.
Resolve the newest release inside the job instead of taking workflow_dispatch inputs, so CKV_GHA_7 no longer fires and checkov stays active on the file.
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 @.github/workflows/package_winget.yml:
- Around line 25-30: Update the MSI selection and packaging flow around the
`$msi` and `$version` assignments so it explicitly selects the x64 asset
expected by `build-winget-package.ps1`'s default `Architecture`, or extracts the
selected filename architecture and passes it through to that script. Ensure the
generated manifest architecture always matches the downloaded MSI URL and hash,
rather than allowing arbitrary arm64 selection.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c8907831-5177-43da-8a69-11ba47ade8ff
📒 Files selected for processing (1)
.github/workflows/package_winget.yml
Parse it from the asset name instead of defaulting to x64, and fail on a multi-arch release rather than validating one at random.
| Writes the MSI and the three winget manifests into -OutputDirectory. The installer manifest | ||
| pins the MSI's SHA256, so the file must be published unchanged at -InstallerUrl. | ||
|
|
||
| WiX is required, pinned below v6, which refuses to build without accepting the OSMF EULA: |
There was a problem hiding this comment.
Seeing WiX triggers a lot of dayjob PTSD 😅
There was a problem hiding this comment.
I think the native service control interface is the real win here ;-)
|
LGTM overall. We may consider doing an arch specific build for arm64. It's fine for now because Meshtastic is not resource intensive and it should run fine through the x64 emulation layer. |
Release attachment moves to the matrix rework in #11151. The MSI is still built and uploaded as a CI artifact.
69a6543 to
9e68447
Compare
As far as I can see, we actually *can't* ship an arm64 build without switching msys2 toolchains (from gcc to clang/llvm)
|
# Conflicts: # src/platform/portduino/PortduinoGlue.cpp
3 participants
Installs the Windows build as a background service instead of shipping a bare binary.
--serviceconnects meshtasticd to the Service Control Manager. Dispatcher runs on its own thread fromparse_opt(), so portduino'smain()is unchanged. Stop setsshutdownAtMsecand reuses the existingPower::shutdown()save-and-exit path.%ProgramData%\Meshtastic\config.yaml,NeverOverwriteandPermanentso an edited one survives upgrade and uninstall.Vital="no".bin/build-winget-package.ps1builds the MSI and emits the winget manifests.WiX is pinned below v6, which refuses to build without accepting the Open Source Maintenance Fee EULA.
Not included, deliberately:
main_matrix.ymlchanges. Attaching the MSI to releases waits for the matrix rework in Actions: Surgical CI - Only run PRs upon actual change #11151, alongside MacOS.package_winget.ymlvalidates manifests against a published release but does not submit them. Publishing to microsoft/winget-pkgs stays manual for now.Verified:
pio run -e native-windows, MSI service/config rows by decompile,winget validate, trunk.Service lifecycle not exercised end to end, which needs an elevated msiexec and attached hardware.
Summary by CodeRabbit
New Features
Bug Fixes