fix: packaging smoke path, RF reconnect ownership, and CI build-channel stamps - #798
Conversation
Mixing release-warnings into upload-artifact paths changed the LCA so installers nested as release/release/*.exe and broke NSIS smoke downloads.
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR adds CI build metadata stamping and exposes that metadata in bundles and logs. It also introduces a shared RF reconnect controller for MeshCore and Meshtastic, with staged packaging warnings and updated reconnect contract tests. ChangesBuild metadata and packaging
RF reconnect ownership
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CIWorkflow
participant BuildInfoScript
participant MainBuild
participant Application
CIWorkflow->>BuildInfoScript: Set channel and workflow metadata
BuildInfoScript->>MainBuild: Write MESH_CLIENT_BUILD_INFO
MainBuild->>Application: Embed compile-time build stamp
Application->>Application: Add metadata to logs and support bundles
sequenceDiagram
participant MeshCoreOrMeshtastic
participant RfReconnectController
participant ReconnectRuntime
MeshCoreOrMeshtastic->>RfReconnectController: Report link loss
RfReconnectController->>RfReconnectController: Select one owner and coalesce events
RfReconnectController->>ReconnectRuntime: Schedule reconnect attempt
ReconnectRuntime->>RfReconnectController: Begin, settle, or cancel attempt
RfReconnectController->>ReconnectRuntime: Request deferred follow-up when dirty
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Stop connection-lost from scheduling after await disconnect while a reconnect cycle is already active (n7eal dual attempt 2+3). Shared createRfReconnectController owns dirty/generation; TCP device_status no longer double-enters lost; DMG Applications link named explicitly.
Embed GitHub Actions run metadata at package time so support-bundle manifests and startup logs distinguish CI test builds from official releases without changing package.json semver.
rinchen
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
src/main/windows-packaging.contract.test.ts-160-166 (1)
160-166: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAssert staging order and upload coverage.
These assertions only verify that the strings exist somewhere in the workflow. The test still passes if staging moves below the uploads or if every upload omits
release/READ-ME-FIRST-test-build.md. Scope the checks to each platform upload block and verify that staging occurs before the first upload.🤖 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 `@src/main/windows-packaging.contract.test.ts` around lines 160 - 166, Strengthen the assertions in the Windows packaging contract test around the buildWorkflow staging and upload blocks: verify READ-ME-FIRST is staged before the first platform upload, and confirm each platform upload includes release/READ-ME-FIRST-test-build.md while excluding release-warnings/READ-ME-FIRST-test-build.md. Scope the checks to the individual upload blocks rather than matching strings anywhere in the workflow.docs/ci-cd.md-102-109 (1)
102-109: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the documented release order.
.github/workflows/release.yamlinstalls Linux dependencies and runspnpm run rebuildbefore the stamp andBuild and Publishsteps. This section lists the build first, then dependency installation and rebuilding. Reorder the numbered steps, or label them as a phase summary, so maintainers do not follow a sequence that differs from CI.As per path instructions: preserve packaging and build-metadata scope and keep packaging diagnostics aligned with the workflow.
🤖 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 `@docs/ci-cd.md` around lines 102 - 109, Update the numbered release workflow summary in the section containing “Stamp CI build info” so it matches `.github/workflows/release.yaml`: list Linux dependency installation and `pnpm run rebuild` before the build-info stamp and parallel platform builds, while preserving the existing packaging and build-metadata descriptions and diagnostics.Source: Path instructions
🤖 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/ci-write-build-info-env.mjs`:
- Around line 152-156: Replace the console.log call in the build-info status
output with process.stdout.write, preserving the existing message and
conditional channel, runId, and tag formatting while ensuring the output
includes the intended line termination.
In `@src/renderer/runtime/useMeshcoreRuntime.ts`:
- Line 2786: Update the retry-exhaustion branches in
src/renderer/runtime/useMeshcoreRuntime.ts:2786-2786 and
src/renderer/runtime/useMeshtasticRuntime.ts:2159-2159 to call
RfReconnectController.markExhausted() after clearing the legacy reconnect state.
When serial rediscovery begins another cycle, re-enter it through the controller
before scheduling, while preserving shared single-owner scheduling, generation
guards, cancellation, attempt settling, and manual-disconnect suppression. Add a
behavioral regression test covering reconnect after exhaustion.
In `@src/renderer/runtime/useMeshtasticRuntime.ts`:
- Around line 1997-2005: In
src/renderer/runtime/useMeshtasticRuntime.ts:1997-2005, guard
handleConnectionLost so explicit disconnects return before calling
meshtasticRfReconnectRef.current.onLinkLost(), and cancel the controller when
manual disconnect, power suspend, or manual connection replacement abandons a
cycle. In src/renderer/runtime/useMeshcoreRuntime.ts:567-570, apply equivalent
cancellation and teardown synchronization to the MeshCore persistent controller.
Preserve shared single-owner scheduling, generation guards, cancellation,
attempt settling, and manual-disconnect suppression, and add regression coverage
for explicit disconnect and suspend before the scheduled owner runs.
---
Other comments:
In `@docs/ci-cd.md`:
- Around line 102-109: Update the numbered release workflow summary in the
section containing “Stamp CI build info” so it matches
`.github/workflows/release.yaml`: list Linux dependency installation and `pnpm
run rebuild` before the build-info stamp and parallel platform builds, while
preserving the existing packaging and build-metadata descriptions and
diagnostics.
In `@src/main/windows-packaging.contract.test.ts`:
- Around line 160-166: Strengthen the assertions in the Windows packaging
contract test around the buildWorkflow staging and upload blocks: verify
READ-ME-FIRST is staged before the first platform upload, and confirm each
platform upload includes release/READ-ME-FIRST-test-build.md while excluding
release-warnings/READ-ME-FIRST-test-build.md. Scope the checks to the individual
upload blocks rather than matching strings anywhere in the workflow.
🪄 Autofix
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: QUIET
Plan: Pro Plus
Run ID: fac101fb-ed0d-4e7a-8d96-76e1696b4296
📒 Files selected for processing (24)
.github/workflows/build.yaml.github/workflows/release.yamldocs/ci-cd.mddocs/troubleshooting.mdelectron-builder.ymlscripts/ci-write-build-info-env.mjsscripts/ci-write-build-info-env.test.mjsscripts/esbuild-main-build.mjsscripts/test-win-nsis-install.mjssrc/main/log-service.test.tssrc/main/log-service.tssrc/main/support-bundle.test.tssrc/main/support-bundle.tssrc/main/windows-packaging.contract.test.tssrc/renderer/hooks/meshcore/meshcoreConnSideEffects.tssrc/renderer/lib/rfReconnectController.test.tssrc/renderer/lib/rfReconnectController.tssrc/renderer/runtime/loraRfReconnectParity.contract.test.tssrc/renderer/runtime/useMeshcoreRuntime.reconnect.test.tssrc/renderer/runtime/useMeshcoreRuntime.tssrc/renderer/runtime/useMeshtasticRuntime.reconnect-hardening.test.tssrc/renderer/runtime/useMeshtasticRuntime.tssrc/shared/buildInfo.test.tssrc/shared/buildInfo.ts
…docs Use stdout.write for CI build-info status, markExhausted + controller re-entry after serial rediscovery, and cancel/guard on disconnect and suspend so scheduled owners cannot restart after abandon.
1 participant
Summary
READ-ME-FIRST-test-build.mdunderrelease/soupload-artifactLCA stays inrelease/(fixes nestedrelease/release/*.exethat broke x64 NSIS smoke in Build Binaries #31012456717).createRfReconnectControllerso MeshCore/Meshtastic TCP do not schedule a second connection-lost reconnect while a cycle is already active (n7eal dual attempt 2+3); TCPdevice_statusno longer double-enters lost; DMG Applications link named explicitly.scripts/ci-write-build-info-env.mjs+src/shared/buildInfo.ts) so support-bundle manifests and startup logs distinguish CI test builds from official releases without changingpackage.jsonsemver.Commits
fix(ci): keep READ-ME-FIRST under release/ for packaging smokefix: single-owner RF reconnect controller for MeshCore/Meshtastic TCPfeat: stamp test vs release build channel into exports and logsTest plan
pnpm exec vitest run src/main/windows-packaging.contract.test.tsrfReconnectController, MeshCore/Meshtastic reconnect contracts,buildInfo, andci-write-build-info-envrelease/(notrelease/release/)READ-ME-FIRST-test-build.mdat the top of the downloadSummary by CodeRabbit
New Features
Bug Fixes
Documentation