fix(android): load embedded server config#608
Conversation
Greptile SummaryThis PR fixes Android config loading so that
Confidence Score: 4/5Changes are logically correct and narrowly scoped to Android; the Linux/desktop path is entirely unaffected. All three changes are small and well-reasoned. get_config_dir() on Android no longer panics and create_config correctly creates and reads config.toml in the app data dir. The port override intentionally discards the port value from the config, which is correct for Android but could surprise future maintainers. No logic regressions on the Linux build path, and the Android-specific code compiles with correct argument count and types. aw-server/src/android/mod.rs is the only file worth a second look since it cannot be exercised by the Linux host build. Important Files Changed
Sequence DiagramsequenceDiagram
participant Android as aw-android JVM
participant JNI as RustInterface (JNI)
participant Dirs as dirs.rs
participant Config as config.rs
Android->>JNI: initialize()
Android->>JNI: setDataDir(appFilesDir)
JNI->>Dirs: set_android_data_dir(path)
Note over Dirs: ANDROID_DATA_DIR = appFilesDir
Android->>JNI: startServer()
JNI->>Config: create_config(false, None)
Config->>Dirs: get_config_dir()
Dirs-->>Config: Ok(ANDROID_DATA_DIR)
Note over Config: resolves to ANDROID_DATA_DIR/config.toml
Config->>Config: create_dir_all(ANDROID_DATA_DIR)
Config->>Config: read/write config.toml (incl. [auth].api_key)
Config-->>JNI: "AWConfig { port, auth, ... }"
JNI->>JNI: "server_config.port = 5600"
JNI->>JNI: build_rocket(server_state, server_config)
Reviews (2): Last reviewed commit: "fix(android): pass missing config_overri..." | Re-trigger Greptile |
| info!("Using server_state:: device_id: {}", server_state.device_id); | ||
|
|
||
| let mut server_config: AWConfig = AWConfig::default(); | ||
| let mut server_config = crate::config::create_config(false); |
There was a problem hiding this comment.
create_config has two parameters (testing: bool, config_override: Option<&Path>); the missing None argument for config_override will cause a compile error on any Android-target build, even though Linux cargo test silently skips this #[cfg(target_os = "android")] block.
| let mut server_config = crate::config::create_config(false); | |
| let mut server_config = crate::config::create_config(false, None); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #608 +/- ##
==========================================
+ Coverage 70.81% 76.95% +6.14%
==========================================
Files 51 62 +11
Lines 2916 4926 +2010
==========================================
+ Hits 2065 3791 +1726
- Misses 851 1135 +284 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
fix(android): added missing Thanks @greptile-apps for catching this. The Linux build never exercises the android cfg block so it was invisible in local testing. |
|
Makes sense — glad that's sorted. The fix is correct: The overall shape of the patch looks good now — Tip: You can customize Greptile's behavior for this repo with |
|
Good catch @greptile-apps — the missing |
|
Thanks for the update! Glad Android CI is green now. The fix sounds straightforward — nothing else to flag on that front. |
|
Android CI is green and the Greptile compile fix is already in the latest push. This PR is ready to merge, but the |
Summary
create_config(false)instead of starting fromAWConfig::default()get_config_dir()return the app data dir so config-backed auth state can actually exist on-deviceWhy
Android auth is still blocked before the
aw-androidWebView handoff step: the embeddedaw-server-rustpath currently bypassesconfig.toml, so[auth].api_keycannot be loaded or persisted on Android at all. This patch fixes that prerequisite and unblocksActivityWatch/aw-android#145.Testing
cargo test -p aw-serverNotes
I could not run an Android-target build in this environment because only the Linux host target is installed here, so the Android-specific validation still needs device/SDK coverage on the
aw-androidside.