Skip to content

Atavism/linux arm - YAML test#1

Open
ayushjrathod-dd wants to merge 16 commits intomainfrom
atavism/linux-arm
Open

Atavism/linux arm - YAML test#1
ayushjrathod-dd wants to merge 16 commits intomainfrom
atavism/linux-arm

Conversation

@ayushjrathod-dd
Copy link
Copy Markdown

@ayushjrathod-dd ayushjrathod-dd commented Mar 20, 2026

CodeAnt-AI Description

Support Linux ARM64 builds, packaging, and more reliable Linux UI smoke tests

What Changed

  • CI and release flows can build, package, verify, and publish Linux ARM64 artifacts alongside AMD64; release notes and Slack messages now list AMD64 vs ARM64 downloads separately
  • Makefiles and packaging use a selectable Linux target arch so produced .deb/.rpm names include arch suffix and binaries/packages are built from the correct bundle
  • S3 publish and release upload scripts handle ARM64 artifacts, validate architecture, and skip missing artifacts with clear messages
  • Linux UI integration smoke test: gate external IP checks by architecture, retry and detect IP changes without failing the teardown, and always attempt a clean disconnect; CI enforces timeouts and surfaces logs for timed-out or failed UI smoke runs
  • Small test harness and formatting tweaks to reduce flaky failures (shorter timeouts, improved debug output, Flutter install fallback for arm64 CI)

Impact

✅ Linux ARM64 builds published to releases
✅ Fewer Linux CI build and publish failures for mixed-arch runs
✅ Fewer flaky Linux UI smoke test failures

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Summary by CodeRabbit

  • New Features

    • Added support for ARM64 architecture on Linux alongside x86_64, enabling native builds for ARM-based systems.
    • Linux releases now include distinct architecture-specific installer packages for each supported platform.
  • Tests

    • Improved VPN connection smoke tests with enhanced timeout management and error detection for network state transitions.

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Mar 20, 2026

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly expands the Linux build capabilities by enabling multi-architecture support for both amd64 and arm64. It involves comprehensive updates to the build system and continuous integration scripts to handle architecture-specific compilation, packaging, and artifact publishing. Additionally, the changes improve the stability and correctness of VPN integration tests by refining IP change detection and ensuring reliable cleanup.

Highlights

  • Multi-architecture Linux Builds: Introduced support for building and packaging Linux applications for both amd64 and arm64 architectures, allowing for more flexible deployment options.
  • CI/CD Pipeline Enhancements: Updated CI scripts to correctly generate architecture-specific release notes, Slack messages, and publish artifacts to S3, streamlining the release process for different Linux targets.
  • Robust VPN Integration Tests: Refactored VPN connection smoke tests to improve IP change verification logic and ensure proper VPN disconnection, enhancing test reliability.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Ignored Files
  • Ignored by pattern: .github/workflows/** (2)
    • .github/workflows/build-linux.yml
    • .github/workflows/release.yml
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Mar 20, 2026
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Mar 20, 2026

Sequence Diagram

This PR adds architecture aware Linux release handling by routing a selected linux arch through metadata, build matrix, packaging, and publishing. It ensures amd64 and arm64 artifacts are built, named, and published consistently.

sequenceDiagram
    participant Maintainer
    participant ReleaseWorkflow
    participant LinuxBuildJob
    participant Makefile
    participant ArtifactStore
    participant PublishScript
    participant Formatter

    Maintainer->>ReleaseWorkflow: Trigger release with platform and linux arch
    ReleaseWorkflow->>LinuxBuildJob: Start linux matrix for selected arch values
    LinuxBuildJob->>Makefile: Build app and service for target arch
    Makefile-->>LinuxBuildJob: Produce arch specific deb and rpm packages
    LinuxBuildJob->>ArtifactStore: Upload linux artifacts with arch suffix
    ReleaseWorkflow->>PublishScript: Publish linux artifacts using linux arch filter
    PublishScript->>ArtifactStore: Copy files to version and latest release paths
    ReleaseWorkflow->>Formatter: Generate release notes and slack text per linux arch
Loading

Generated by CodeAnt AI

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for building and publishing Linux ARM64 packages. The changes are well-structured, modifying the Makefile for multi-architecture builds and updating CI scripts for release notes and artifact publishing. I have one suggestion for the Makefile to improve maintainability by reducing code duplication in a build recipe. The test harness modifications are also a good improvement for test stability.

Comment thread Makefile
Comment on lines +335 to +343
VERSION=$(APP_VERSION) GOARCH=$(LINUX_TARGET_ARCH) LINUX_BUNDLE_SRC="$$BUNDLE_DIR/" SYSTEMD_UNIT_SRC=$(LINUX_SYSTEMD_UNIT_DST) \
LANTERND_SRC=$(LINUX_SERVICE_DST)/$(LINUX_SERVICE_NAME) LANTERND_DST=/usr/sbin/$(LINUX_SERVICE_NAME) \
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p deb -t $(LINUX_INSTALLER_DEB)
VERSION=$(APP_VERSION) SYSTEMD_UNIT_SRC=$(LINUX_SYSTEMD_UNIT_DST) \
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p deb -t $(LINUX_INSTALLER_DEB); \
VERSION=$(APP_VERSION) GOARCH=$(LINUX_TARGET_ARCH) LINUX_BUNDLE_SRC="$$BUNDLE_DIR/" SYSTEMD_UNIT_SRC=$(LINUX_SYSTEMD_UNIT_DST) \
LANTERND_SRC=$(LINUX_SERVICE_DST)/$(LINUX_SERVICE_NAME) LANTERND_DST=/usr/sbin/$(LINUX_SERVICE_NAME) \
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p rpm -t $(LINUX_INSTALLER_RPM)
VERSION=$(APP_VERSION) SYSTEMD_UNIT_SRC=$(LINUX_SYSTEMD_UNIT_DST) \
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p rpm -t $(LINUX_INSTALLER_RPM); \
VERSION=$(APP_VERSION) GOARCH=$(LINUX_TARGET_ARCH) LINUX_BUNDLE_SRC="$$BUNDLE_DIR/" SYSTEMD_UNIT_SRC=$(LINUX_SYSTEMD_UNIT_DST) \
LANTERND_SRC=$(LINUX_SERVICE_DST)/$(LINUX_SERVICE_NAME) LANTERND_DST=/usr/bin/$(LINUX_SERVICE_NAME) \
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p archlinux -t $(LINUX_INSTALLER_ARCH)
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p archlinux -t $(LINUX_INSTALLER_ARCH)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The three nfpm commands are very similar and repeat a long list of environment variables. This repetition can make the recipe harder to read and maintain. You can define a shell variable to hold the common environment variables to reduce duplication and improve clarity.

	NFPM_ENV="VERSION=$(APP_VERSION) GOARCH=$(LINUX_TARGET_ARCH) LINUX_BUNDLE_SRC='$$BUNDLE_DIR/' SYSTEMD_UNIT_SRC=$(LINUX_SYSTEMD_UNIT_DST) LANTERND_SRC=$(LINUX_SERVICE_DST)/$(LINUX_SERVICE_NAME)"; \
	$$NFPM_ENV LANTERND_DST=/usr/sbin/$(LINUX_SERVICE_NAME) \
		nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p deb -t $(LINUX_INSTALLER_DEB); \
	$$NFPM_ENV LANTERND_DST=/usr/sbin/$(LINUX_SERVICE_NAME) \
		nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p rpm -t $(LINUX_INSTALLER_RPM); \
	$$NFPM_ENV LANTERND_DST=/usr/bin/$(LINUX_SERVICE_NAME) \
		nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p archlinux -t $(LINUX_INSTALLER_ARCH)

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Mar 20, 2026

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • IP-Change Flag
    ipChanged is initialized to true before attempting the connect/IP check. If an exception occurs before the IP-change detection runs, the default true can mask a failed IP check and cause the test to pass incorrectly. The flag should default to false and be set to true only after a successful detection.

  • Disconnect Precondition
    _disconnectVpn returns early unless the current state is connected or connecting. If current() returns null (unknown state) the function will skip attempting a disconnect; this may leave the VPN in a connected state. Consider attempting a best-effort disconnect when state is unknown or re-checking the UI state after waiting for the toggle to be available.

  • Artifact discovery
    The upload_artifact candidate directory / filename logic is strict (e.g. arm64 only checks
    lantern-installer-<ext>-arm64). Real build outputs may use different naming conventions
    or extra variants. This may cause valid artifacts to be skipped. Broaden search patterns or
    add robust fallbacks.

  • Bundle & packaging flow
    The linux-release-ci flow selects a bundle dir based on LINUX_TARGET_ARCH and then runs patchelf and nfpm packaging. Verify the mapping from GO arch names (e.g., 'amd64') to bundle dir names (e.g., 'x64') is correct, that the expected binary name exists at "$$BUNDLE_DIR/lantern", and that patchelf/nfpm invocations receive the right variables for multi-arch packaging.

  • Cross-compiler assumption
    New variables define external cross-compilers (LINUX_CC_AMD64, LINUX_CC_ARM64). Confirm CI and developer machines have these toolchains installed, or provide fallbacks/documentation. Also confirm that these CC values are actually used by the build commands in all invoked contexts.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 20, 2026

📝 Walkthrough

Walkthrough

This PR introduces multi-architecture Linux build support (amd64 and arm64) across CI/CD workflows, packaging configuration, and build infrastructure. It parameterizes the build pipeline with selectable target architectures, updates artifact naming and upload logic, and refactors VPN testing to improve reliability.

Changes

Cohort / File(s) Summary
CI/CD Workflow Configuration
.github/workflows/build-linux.yml, .github/workflows/release.yml
Added linux_arch input and matrix-based builds selecting runner/architecture (amd64 on ubuntu-latest, arm64 on ubuntu-24.04-arm). Updated Flutter installation to parse version from .github/flutter-version.yaml and use architecture-specific methods. Modified packaging verification and smoke tests to accept architecture argument and set ENABLE_IP_CHECK based on target architecture. Updated artifact naming to include architecture suffix.
Build System Parameterization
Makefile, linux/packaging/nfpm.yaml
Added LINUX_TARGET_ARCH parameter (default amd64) with architecture-specific package suffixes, service build targets, and bundle directories. Introduced cross-compiler variables (LINUX_CC_AMD64, LINUX_CC_ARM64). Updated linux meta-target to build only the selected architecture and rewired service/staging flow. Modified nfpm to use configurable ${LINUX_BUNDLE_SRC} environment variable instead of fixed path.
CI/CD Scripts
scripts/ci/format.sh, scripts/ci/publish-to-s3.sh, scripts/ci/verify_linux_package.sh
Added LINUX_ARCH environment variable support with validation. Updated artifact discovery and upload logic to handle architecture-specific naming (*-amd64, *-arm64 variants). Modified format.sh to generate architecture-specific download links. Enhanced verify_linux_package.sh to optionally validate package architecture via dpkg-deb.
VPN Integration Testing
integration_test/vpn/connect_smoke_harness.dart
Refactored _assertPublicIpChangesFromBaseline to return boolean instead of directly failing. Added _disconnectVpn helper for conditional VPN state-aware disconnection. Restructured connect/disconnect flow with try/finally to ensure cleanup, and made IP verification failure conditional on baseline capture and enabled checking.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Multi-arch hops across the land,
amd64, arm64, hand in hand,
From Linux builds to packaging schemes,
We compile for all the different dreams!
One target architecture's no longer enough—
The Lantern burns brighter, twice as tough! 🔦✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Atavism/linux arm - YAML test' does not clearly describe the main changes in the pull request, which involve adding ARM64 architecture support to Linux build workflows and build systems. Use a more descriptive title that captures the main objective, such as 'Add ARM64 architecture support for Linux builds' or 'Enable multi-architecture Linux builds (amd64 and arm64)'.
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch atavism/linux-arm
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread Makefile

$(LINUX_LIB_ARM64): $(GO_SOURCES)
CC=aarch64-linux-gnu-gcc GOARCH=arm64 LIB_NAME=$@ make desktop-lib
CC=$(LINUX_CC_ARM64) GOOS=linux GOARCH=arm64 LIB_NAME=$@ $(MAKE) desktop-lib
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The arm64 library build writes output to bin/linux-arm64/... but never creates that directory first. On clean builds this will fail with a "no such file or directory" error when go build -o tries to write the shared library. [resource leak]

Severity Level: Critical 🚨
- ❌ Linux arm64 release build can fail immediately.
- ❌ CI pipeline blocks Linux artifact production.
Suggested change
CC=$(LINUX_CC_ARM64) GOOS=linux GOARCH=arm64 LIB_NAME=$@ $(MAKE) desktop-lib
$(call MKDIR_P,$(dir $@))
Steps of Reproduction ✅
1. Trigger Linux CI build at `.github/workflows/build-linux.yml:155`, which runs `make
linux-release-ci` with `LINUX_TARGET_ARCH=${{ matrix.arch }}` (`:160`), and matrix
includes `arm64` (`:27`).

2. In `Makefile`, `linux-release-ci` (`:318`) depends on `linux` (`:279`), which resolves
to `linux-arm64` and executes target rule `$(LINUX_LIB_ARM64):` at `:269-270`.

3. That rule directly calls `desktop-lib` at `Makefile:270` without creating `$(dir $@)`.
`desktop-lib` runs `go build ... -o $(LIB_NAME)` at `Makefile:177`.

4. On a clean checkout (`actions/checkout` at workflow `:35`), `bin/linux-arm64/` does not
exist, so writing `-o bin/linux-arm64/liblantern.so` fails with missing-directory error.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** Makefile
**Line:** 270:270
**Comment:**
	*Resource Leak: The arm64 library build writes output to `bin/linux-arm64/...` but never creates that directory first. On clean builds this will fail with a "no such file or directory" error when `go build -o` tries to write the shared library.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment thread Makefile

$(LINUX_LIB_AMD64): $(GO_SOURCES)
CC=x86_64-linux-gnu-gcc GOARCH=amd64 LIB_NAME=$@ make desktop-lib
CC=$(LINUX_CC_AMD64) GOOS=linux GOARCH=amd64 LIB_NAME=$@ $(MAKE) desktop-lib
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The amd64 library build has the same missing-directory issue: it writes to bin/linux-amd64/... without ensuring the directory exists first. This breaks clean CI/local runs where that path has not been created yet. [possible bug]

Severity Level: Critical 🚨
- ❌ Linux amd64 release build can fail immediately.
- ❌ Primary CI release workflow becomes unstable.
Suggested change
CC=$(LINUX_CC_AMD64) GOOS=linux GOARCH=amd64 LIB_NAME=$@ $(MAKE) desktop-lib
$(call MKDIR_P,$(dir $@))
Steps of Reproduction ✅
1. Trigger Linux CI build at `.github/workflows/build-linux.yml:155` with
`matrix.arch=amd64` (`:27`, passed to `LINUX_TARGET_ARCH` at `:160`).

2. `linux-release-ci` (`Makefile:318`) depends on `linux` (`:279`), which resolves to
`linux-amd64` and runs rule `$(LINUX_LIB_AMD64):` at `:275-276`.

3. Rule line `Makefile:276` invokes `desktop-lib` directly; `desktop-lib` writes output
via `go build ... -o $(LIB_NAME)` (`Makefile:177`) and does not create parent directories.

4. In clean CI workspaces, `bin/linux-amd64/` is absent, so writing
`bin/linux-amd64/liblantern.so` can fail with "no such file or directory".
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** Makefile
**Line:** 276:276
**Comment:**
	*Possible Bug: The amd64 library build has the same missing-directory issue: it writes to `bin/linux-amd64/...` without ensuring the directory exists first. This breaks clean CI/local runs where that path has not been created yet.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment thread Makefile
Comment on lines +337 to +340
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p deb -t $(LINUX_INSTALLER_DEB); \
VERSION=$(APP_VERSION) GOARCH=$(LINUX_TARGET_ARCH) LINUX_BUNDLE_SRC="$$BUNDLE_DIR/" SYSTEMD_UNIT_SRC=$(LINUX_SYSTEMD_UNIT_DST) \
LANTERND_SRC=$(LINUX_SERVICE_DST)/$(LINUX_SERVICE_NAME) LANTERND_DST=/usr/sbin/$(LINUX_SERVICE_NAME) \
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p rpm -t $(LINUX_INSTALLER_RPM)
VERSION=$(APP_VERSION) SYSTEMD_UNIT_SRC=$(LINUX_SYSTEMD_UNIT_DST) \
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p rpm -t $(LINUX_INSTALLER_RPM); \
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The three packaging commands are chained with ;, so failure in the deb/rpm packaging steps can be silently ignored if the final archlinux command succeeds. This can produce partially broken release artifacts while the target still exits successfully. [logic error]

Severity Level: Major ⚠️
- ❌ RPM artifact can be missing while build reports success.
- ⚠️ Partial packaging failures become harder to detect.
Suggested change
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p deb -t $(LINUX_INSTALLER_DEB); \
VERSION=$(APP_VERSION) GOARCH=$(LINUX_TARGET_ARCH) LINUX_BUNDLE_SRC="$$BUNDLE_DIR/" SYSTEMD_UNIT_SRC=$(LINUX_SYSTEMD_UNIT_DST) \
LANTERND_SRC=$(LINUX_SERVICE_DST)/$(LINUX_SERVICE_NAME) LANTERND_DST=/usr/sbin/$(LINUX_SERVICE_NAME) \
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p rpm -t $(LINUX_INSTALLER_RPM)
VERSION=$(APP_VERSION) SYSTEMD_UNIT_SRC=$(LINUX_SYSTEMD_UNIT_DST) \
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p rpm -t $(LINUX_INSTALLER_RPM); \
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p deb -t $(LINUX_INSTALLER_DEB) && \
VERSION=$(APP_VERSION) GOARCH=$(LINUX_TARGET_ARCH) LINUX_BUNDLE_SRC="$$BUNDLE_DIR/" SYSTEMD_UNIT_SRC=$(LINUX_SYSTEMD_UNIT_DST) \
LANTERND_SRC=$(LINUX_SERVICE_DST)/$(LINUX_SERVICE_NAME) LANTERND_DST=/usr/sbin/$(LINUX_SERVICE_NAME) \
nfpm package -f $(LINUX_PKG_ROOT)/nfpm.yaml -p rpm -t $(LINUX_INSTALLER_RPM) && \
Steps of Reproduction ✅
1. CI executes `make linux-release-ci` at `.github/workflows/build-linux.yml:155`, running
packaging recipe at `Makefile:335-343`.

2. The recipe chains three `nfpm package` commands with `;` (`Makefile:337`, `:340`,
`:343`) in one shell line.

3. `Makefile` does not set global shell fail-fast (`.ONESHELL`/`SHELLFLAGS -e` not
defined; only unrelated local `@set -e` appears in iOS block at `Makefile:555`), so the
shell returns the status of the last command.

4. If `deb` or `rpm` packaging fails but `archlinux` succeeds, this line exits success; CI
can proceed and artifact upload for rpm (`.github/workflows/build-linux.yml:256-257`) may
silently publish incomplete outputs.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** Makefile
**Line:** 337:340
**Comment:**
	*Logic Error: The three packaging commands are chained with `;`, so failure in the deb/rpm packaging steps can be silently ignored if the final archlinux command succeeds. This can produce partially broken release artifacts while the target still exits successfully.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment on lines +192 to +204
if (currentState != VPNStatus.connected &&
currentState != VPNStatus.connecting) {
return;
}

await WidgetWaitUtils.waitForFinder(
tester,
vpnToggle,
timeout: const Duration(seconds: 15),
reason: 'VPN toggle not available for disconnect',
);
await tester.tap(vpnToggle);
await tester.pump(const Duration(milliseconds: 200));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The disconnect helper decides to tap based on an old state snapshot, then waits for the toggle to reappear; if the state changes from connecting to disconnected during that wait, it will tap anyway and accidentally start a new connection. Re-check the state after waiting and only tap when the current stable state is actually connected. [logic error]

Severity Level: Major ⚠️
- ⚠️ VPN smoke cleanup can trigger unintended reconnect.
- ⚠️ Linux/Windows integration tests become flaky.
- ❌ Failure reason can be masked by cleanup timeout.
Suggested change
if (currentState != VPNStatus.connected &&
currentState != VPNStatus.connecting) {
return;
}
await WidgetWaitUtils.waitForFinder(
tester,
vpnToggle,
timeout: const Duration(seconds: 15),
reason: 'VPN toggle not available for disconnect',
);
await tester.tap(vpnToggle);
await tester.pump(const Duration(milliseconds: 200));
if (currentState == VPNStatus.disconnected ||
currentState == VPNStatus.missingPermission ||
currentState == VPNStatus.error) {
return;
}
await WidgetWaitUtils.waitForFinder(
tester,
vpnToggle,
timeout: const Duration(seconds: 15),
reason: 'VPN toggle not available for disconnect',
);
final stableState = await vpnStateFinders.tryWaitFor(
tester,
expected: const [VPNStatus.connected, VPNStatus.disconnected],
timeout: const Duration(seconds: 15),
);
if (stableState == VPNStatus.connected) {
await tester.tap(vpnToggle);
await tester.pump(const Duration(milliseconds: 200));
}
Steps of Reproduction ✅
1. Run either integration entrypoint
`integration_test/vpn/linux_connect_smoke_test.dart:13-15` or
`integration_test/vpn/windows_connect_smoke_test.dart:13-15`, which both call
`runConnectSmokeHarness(...)`.

2. In `integration_test/vpn/connect_smoke_harness.dart:333-359`, the harness always
executes `_disconnectVpn(...)` in the `finally` block after the connect attempt.

3. Trigger a realistic unstable-connect run where the connect phase (`waitFor` at
`connect_smoke_harness.dart:338-343`) is still transitioning or fails to stabilize;
`_disconnectVpn` then snapshots state once at `:191-194` and proceeds.

4. If UI state flips from `connecting` to `disconnected` while waiting for toggle
visibility (`:197-202`), code still taps at `:203`, toggling from disconnected back toward
connect.

5. Observe cleanup becomes non-deterministic: `_disconnectVpn` may start a new connection
and then wait for disconnected at `:206-211`, causing extra timeout/failure noise in smoke
test results.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** integration_test/vpn/connect_smoke_harness.dart
**Line:** 192:204
**Comment:**
	*Logic Error: The disconnect helper decides to tap based on an old state snapshot, then waits for the toggle to reappear; if the state changes from connecting to disconnected during that wait, it will tap anyway and accidentally start a new connection. Re-check the state after waiting and only tap when the current stable state is actually connected.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Comment on lines +149 to 150
upload_artifact "$platform" "$extension" "${arch:-}"
result=$?
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: With set -e, calling the upload function directly will terminate the script immediately on return codes 1 (not found) or 2 (upload failure), so the summary/counting logic never runs. Wrap the call in an if so non-zero statuses can be captured safely instead of triggering errexit. [logic error]

Severity Level: Critical 🚨
- ❌ upload-s3 job fails on first missing artifact.
- ❌ Release flow aborts before aggregated failure accounting.
- ⚠️ Skip/failure summary in lines 159-160 never prints.
Suggested change
upload_artifact "$platform" "$extension" "${arch:-}"
result=$?
if upload_artifact "$platform" "$extension" "${arch:-}"; then
result=0
else
result=$?
fi
Steps of Reproduction ✅
1. Trigger the release pipeline job `upload-s3` in
`.github/workflows/release.yml:362-410`; step `Upload to S3` runs
`./scripts/ci/publish-to-s3.sh` at lines 406-410.

2. In `scripts/ci/publish-to-s3.sh`, the script is in errexit mode (`set -euo pipefail` at
line 3) and calls `upload_artifact` directly inside the artifact loop at line 149.

3. If an artifact file is absent, `upload_artifact()` returns `1` at lines 115-117 (`⊘
Skipping ... not found`), or returns `2` on upload failure via `upload_file()` lines
66-75.

4. Because line 149 is a plain command (not guarded by `if`), Bash exits immediately under
`-e` before `result=$?` (line 150), so counting logic (`case` lines 152-156) and summary
output (lines 159-160) never execute.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** scripts/ci/publish-to-s3.sh
**Line:** 149:150
**Comment:**
	*Logic Error: With `set -e`, calling the upload function directly will terminate the script immediately on return codes `1` (not found) or `2` (upload failure), so the summary/counting logic never runs. Wrap the call in an `if` so non-zero statuses can be captured safely instead of triggering errexit.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Mar 20, 2026

CodeAnt AI finished reviewing your PR.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Makefile`:
- Around line 269-270: The recipe for the LINUX_LIB_ARM64 target currently
invokes the desktop-lib make target without ensuring the output directory
exists; update the LINUX_LIB_ARM64 recipe to create the parent directory for $@
before calling desktop-lib (use the same pattern as the Windows/Darwin targets,
e.g. call the MKDIR_P macro on $(dir $@)) so the nested path (e.g.
bin/linux-arm64/) is created prior to running CC=$(LINUX_CC_ARM64) GOOS=linux
GOARCH=arm64 LIB_NAME=$@ $(MAKE) desktop-lib.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c854437f-e7f5-483c-8681-1e5b5d39e9cc

📥 Commits

Reviewing files that changed from the base of the PR and between c1a0238 and ffba53a.

📒 Files selected for processing (8)
  • .github/workflows/build-linux.yml
  • .github/workflows/release.yml
  • Makefile
  • integration_test/vpn/connect_smoke_harness.dart
  • linux/packaging/nfpm.yaml
  • scripts/ci/format.sh
  • scripts/ci/publish-to-s3.sh
  • scripts/ci/verify_linux_package.sh

Comment thread Makefile
Comment on lines 269 to +270
$(LINUX_LIB_ARM64): $(GO_SOURCES)
CC=aarch64-linux-gnu-gcc GOARCH=arm64 LIB_NAME=$@ make desktop-lib
CC=$(LINUX_CC_ARM64) GOOS=linux GOARCH=arm64 LIB_NAME=$@ $(MAKE) desktop-lib
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "Linux lib target recipes:"
nl -ba Makefile | sed -n '266,281p'

echo
echo "desktop-lib recipe:"
nl -ba Makefile | sed -n '175,181p'

echo
echo "clean recipe:"
nl -ba Makefile | sed -n '618,624p'

Repository: coderabbit-test/lantern

Length of output: 152


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "=== Linux lib target recipes (lines 266-281) ==="
sed -n '266,281p' Makefile | cat -n

echo
echo "=== desktop-lib recipe (lines 175-181) ==="
sed -n '175,181p' Makefile | cat -n

echo
echo "=== clean recipe (lines 618-624) ==="
sed -n '618,624p' Makefile | cat -n

echo
echo "=== Checking for MKDIR_P definition ==="
grep -n "MKDIR_P" Makefile | head -20

Repository: coderabbit-test/lantern

Length of output: 2123


🏁 Script executed:

#!/bin/bash
echo "=== Searching for LINUX_LIB_ARM64 and LINUX_LIB_AMD64 definitions ==="
grep -n "LINUX_LIB_ARM64\|LINUX_LIB_AMD64" Makefile | grep -v "^\s*#" | head -30

echo
echo "=== Searching for BIN_DIR definition ==="
grep -n "^BIN_DIR" Makefile

echo
echo "=== Searching for LINUX_LIB definition ==="
grep -n "^LINUX_LIB" Makefile | head -10

Repository: coderabbit-test/lantern

Length of output: 750


Create the per-arch lib output directories before calling desktop-lib.

These targets write to nested paths like bin/linux-amd64/liblantern.so and bin/linux-arm64/liblantern.so, but neither recipe creates the parent directories. After a clean checkout or after make clean removes bin/*, the nested output paths will be missing before the build even starts, causing the go build step to fail. This pattern is already correctly implemented in other platform targets (Windows and Darwin lib builds use $(call MKDIR_P,$(dir $@))), but is missing here.

🛠️ Proposed fix
 $(LINUX_LIB_ARM64): $(GO_SOURCES)
+	$(call MKDIR_P,$(dir $@))
 	CC=$(LINUX_CC_ARM64) GOOS=linux GOARCH=arm64 LIB_NAME=$@ $(MAKE) desktop-lib

 $(LINUX_LIB_AMD64): $(GO_SOURCES)
+	$(call MKDIR_P,$(dir $@))
 	CC=$(LINUX_CC_AMD64) GOOS=linux GOARCH=amd64 LIB_NAME=$@ $(MAKE) desktop-lib
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Makefile` around lines 269 - 270, The recipe for the LINUX_LIB_ARM64 target
currently invokes the desktop-lib make target without ensuring the output
directory exists; update the LINUX_LIB_ARM64 recipe to create the parent
directory for $@ before calling desktop-lib (use the same pattern as the
Windows/Darwin targets, e.g. call the MKDIR_P macro on $(dir $@)) so the nested
path (e.g. bin/linux-arm64/) is created prior to running CC=$(LINUX_CC_ARM64)
GOOS=linux GOARCH=arm64 LIB_NAME=$@ $(MAKE) desktop-lib.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants