Conversation
|
@atavism Any Idea here, Why macos step is skipping without error? |
|
Found the issue. |
|
@atavism Feel free to take this one this sprint, if you have time. |
|
@jigar-f Will do so today! |
|
@atavism this is not even complete. Feel free to take it if you have a time. |
There was a problem hiding this comment.
Pull request overview
This pull request adds environment awareness to the Lantern application, enabling it to distinguish between "production" and "staging" modes across all platforms (Android, iOS, macOS, and via FFI). The environment can be toggled from developer settings on most platforms, with the state persisted locally and communicated to native code at startup.
Changes:
- Added environment detection and propagation from Flutter through native code (iOS/macOS/Android) to the Go core backend
- Updated database schema to persist environment setting in AppSetting entity
- Added UI toggle in developer settings to switch environments (excluded from iOS) with app restart prompt
- Modified macOS to use a shared directory (/Users/Shared/Lantern) instead of user-specific Application Support
Reviewed changes
Copilot reviewed 37 out of 49 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| macos/Shared/FilePath.swift | Changed app support directory to shared location and added environment detection |
| macos/Runner/Handlers/MethodHandler.swift | Updated plans method to accept channel parameter |
| macos/Runner/AppDelegate.swift | Added environment setting to initialization options |
| macos/Podfile.lock | Updated pod checksums from dependency changes |
| lib/lantern/lantern_ffi_service.dart | Added environment parameter to FFI setup call |
| lib/features/home/provider/app_setting_notifier.dart | Implemented setEnvironment method with directory cleanup |
| lib/features/developer/developer_mode.dart | Added environment toggle UI (excluded from iOS) |
| lib/core/utils/storage_utils.dart | Added writeRadianceEnvFile utility method |
| lib/core/services/db/objectbox.g.dart | Generated code for new environment field |
| lib/core/services/db/objectbox-model.json | Updated schema with environment property |
| lib/core/models/entity/app_setting_entity.dart | Added environment field to AppSetting entity |
| lantern-core/utils/common.go | Added Env field to Opts struct |
| lantern-core/ffi/ffi.go | Added _env parameter to setup function |
| lantern-core/core.go | Added logic to set environment before Radiance initialization |
| ios/Shared/FilePath.swift | Added isRadianceEnv method for environment detection |
| ios/Runner/AppDelegate.swift | Added environment setting to initialization options |
| go.mod | Updated radiance dependency and uncommented replace directive |
| go.sum | Updated checksums for radiance dependency |
| android/app/src/main/kotlin/.../utils.kt | Added getRadianceEnv function |
| android/app/src/main/kotlin/.../LanternVpnService.kt | Added environment to VPN service options |
| Makefile | Added echo for RADIANCE_ENV during macOS build |
| .github/workflows/nightly.yml | Removed trailing whitespace |
| .github/workflows/build-macos.yml | Removed trailing whitespace |
Comments suppressed due to low confidence (3)
lib/features/home/provider/app_setting_notifier.dart:149
- The logic for switching to production mode (when isStaging is false) contains redundant file deletion. Lines 146-149 attempt to delete the .radiance_env file, but the entire directory has already been deleted recursively on line 140. The file deletion check and operation at lines 146-149 will never find a file since the directory was just recreated empty on line 144.
go.mod:5 - The replace directive for github.com/getlantern/radiance is uncommented, pointing to a local ../radiance directory. This is typically used for local development and should not be committed to the repository for production releases. This will cause build failures for anyone who doesn't have the radiance repository checked out in the expected relative location. Consider removing this replace directive or commenting it out before merging.
// replace github.com/getlantern/radiance => ../radiance
lib/features/home/provider/app_setting_notifier.dart:150
- The setEnvironment method deletes the entire app directory recursively (line 127 and 140), which will delete all user data including user accounts, settings, VPN configurations, and any other data stored in that directory. This is a destructive operation that goes beyond just switching environments. Users will lose their login session, preferences, and other important data.
Consider either:
- Documenting this behavior clearly in the UI to warn users
- Selectively preserving important user data that shouldn't be lost on environment switch
- Only deleting radiance-specific data that needs to be cleared for environment changes
The current implementation treats environment switching like a full app reset, which may not be the intended behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static func appSupportDir() -> URL { | ||
| let base = FileManager.default | ||
| .urls(for: .applicationSupportDirectory, in: .userDomainMask)[0] | ||
| let dir = URL(fileURLWithPath: "/Users/Shared/Lantern", isDirectory: true) | ||
|
|
||
| // Create directory if it doesn't exist | ||
| try? FileManager.default.createDirectory( | ||
| at: dir, | ||
| withIntermediateDirectories: true, | ||
| attributes: nil) | ||
|
|
||
| let bundleId = FilePath.bundleId | ||
| let dir = base.appendingPathComponent(bundleId, isDirectory: true) | ||
| return dir |
There was a problem hiding this comment.
This change hardcodes the application data directory to "/Users/Shared/Lantern", which is a significant change from the previous implementation that used the user's Application Support directory. This has several implications:
- All users on the system will share the same Lantern data, including VPN settings, user accounts, and environment configuration
- This may cause privacy concerns as one user's data becomes accessible to other users
- The /Users/Shared directory requires appropriate permissions and may not be accessible in all environments
- This breaks data isolation between different user accounts
This appears to be an intentional change based on the PR context, but it's a breaking change that could affect existing users. Consider documenting this change and providing a migration path for existing user data.
| guard let channel: String = self.decodeValue(from: call.arguments, result: result) else { | ||
| return | ||
| } | ||
| self.plans(result: result, channel: channel) |
There was a problem hiding this comment.
Inconsistent formatting: there's a missing space after the comma in the plans method call. Line 48 has "result,channel:" which should be "result, channel:" to match Swift formatting conventions and the rest of the codebase.
|
|
||
| $(MACOS_FRAMEWORK_BUILD): $(GO_SOURCES) | ||
| @echo "Building macOS Framework.." | ||
| echo "RADIANCE_ENV $(RADIANCE_ENV)" |
There was a problem hiding this comment.
The echo statement on line 177 is missing a dollar sign before the variable name. It should be "echo "RADIANCE_ENV $$RADIANCE_ENV"" or "echo "RADIANCE_ENV
| echo "RADIANCE_ENV $(RADIANCE_ENV)" | |
| echo "RADIANCE_ENV $$RADIANCE_ENV" |
| }, | ||
| ), | ||
| ), | ||
| DividerSpace(), |
There was a problem hiding this comment.
There's a DividerSpace on line 82 that will always render, even when "Test Play Purchase" is not shown (on non-Android platforms). This will create an extra divider at the top of the card when the first tile is "Stage Environment". Consider wrapping the DividerSpace in the same conditional as the "Test Play Purchase" tile, or moving it inside that conditional block.
| DividerSpace(), | |
| if (PlatformUtils.isAndroid) | |
| DividerSpace(), |
| Future<void> setEnvironment(bool isStaging) async { | ||
| update(state.copyWith(environment: isStaging ? 'staging' : 'production')); | ||
|
|
||
| final dir = await AppStorageUtils.getAppDirectory(); | ||
|
|
||
| // Delete and recreate the directory | ||
| if (dir.existsSync()) { | ||
| await dir.delete(recursive: true); | ||
| } | ||
| await dir.create(recursive: true); | ||
|
|
||
| // Create .radiance_env file only in staging | ||
| if (isStaging) { | ||
| final file = File('${dir.path}/.radiance_env'); | ||
| await file.create(); | ||
| } | ||
| } |
There was a problem hiding this comment.
There's a semantic mismatch in how the environment is stored vs read. Flutter's setEnvironment stores the string 'staging' or 'production' in the AppSetting model, but the .radiance_env file is created/deleted without any content. The native code (iOS, macOS, Android) only checks for file existence, not the content. This works but is inconsistent with the writeRadianceEnvFile utility which expects to write the environment string to the file. Consider either:
- Writing the environment value ('stage' or 'prod') to the file content, or
- Removing the env parameter from writeRadianceEnvFile since only file existence matters
This inconsistency could lead to confusion in future maintenance.
2 participants
This pull request introduces environment awareness to the application, allowing it to distinguish between "production" and "staging" modes across Android, iOS, and Flutter. The environment can now be toggled from the developer settings, and this state is persisted and communicated to native code at startup. The changes span native code, Flutter models, database schema, and developer UI.
Environment Detection and Propagation:
Added logic to detect the environment (prod/stage) at startup on both Android (
getRadianceEnv()inutils.kt) and iOS (isRadianceEnv()inFilePath.swift), based on the presence of a.radiance_envfile. This value is passed to the core initialization so that the backend can behave accordingly. [1] [2] [3] [4] [5] [6]The
.radiance_envfile is now written from Flutter using a new utility method, ensuring native code can read the correct environment before Flutter is ready.Flutter Model and Database Updates:
environmentfield to theAppSettingentity, updated the ObjectBox schema and code generation files to persist and query this value. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]Developer UI Enhancements:
Added a "Stage Environment" switch to the developer mode screen (except on iOS), allowing toggling between environments. The user is prompted to restart the app for changes to take effect. [1] [2] [3]
Refactored the developer mode UI to group tiles more logically and moved "Reset App" to its own card. [1] [2]
Core and Build System Changes:
Updated the Go core and FFI layer to accept and propagate the environment option, and set the correct environment before initializing Radiance. [1] [2] [3] [4]
Minor: Updated the
radiancedependency version and made a small Makefile change to echo the current environment during macOS framework builds. [1] [2] [3]These changes collectively enable robust environment switching and awareness across the app stack, facilitating easier testing and deployment.