Repository navigation
feat(android): ship session service and microphone plugins as a plugin crate - #990
Open
smart-ssh-coder[bot] wants to merge 4 commits into
Open
smart-ssh-coder[bot] wants to merge 4 commits into
smart-ssh-coder[bot] wants to merge 4 commits into
Conversation
…n crate (#888) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
8 tasks
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
8 tasks
There was a problem hiding this comment.
Review
Verdict: approved
Review level: normal
Blocking
None.
Non-blocking
crates/tauri-plugin-android-host/src/lib.rs:1— the doc comment cites "ADR 0153", but the new ADR is 0154 (0153 is the entitlement ADR). Cosmetic.- PR body "Tests" names
plugins_are_named_and_build_on_desktop, but the crate has no test (the Summary says it was dropped). Refactoring only, so no regression test is required.
Verified
- counter-proof: not applicable, because this is a packaging refactoring with no behaviour fix
- edge cases: microphone plugin still registered in the after-unlock chain via
microphone_plugin()(read: lib.rs:1129,1157) → ok; non-Android builds get empty plugins → ok;session_servicehandle missing is handled viatry_state(read: android_service.rs) → ok; allR.*references in the Kotlin files resolve to the library's own package and strings (executed: rg) → ok - callers checked:
voice_input::microphone_pluginremoved (no remaining references),Handle/SessionServiceHandle(2 call sites) → ok - tests run locally:
cargo clippy --locked -p tauri-plugin-android-host -p app-shell --all-targets -- -D warnings→ clean (executed). The crate has no tests; theaarch64-linux-androidcheck was not run locally (toolchain not available), covered by CI instead. - CI: all checks green, including Android debug APK (with the new manifest/Gradle step) and the launch smoke test (executed:
gh pr checks 990) - read: all moved Kotlin files (diff), manifest, strings, build.gradle.kts,
scripts/check-android-apk-manifest.sh, workflow step - files read: 5/5 logic files
- UI: not a UI change
- data: not touched
Timing analysis
- no test files changed
Spec
- visible behaviour changed: none (notification shows the host app's icon, same app icon as before)
- spec searched (terms: Hintergrund, foreground, Mikrofon, android), spec covering it: 0114 §4, which the PR updates
- verdict: spec change present and matches the code
Security checks
- Permissions, service entry and
foregroundServiceTypeunchanged; the service only reflects the session count. The CI step only greps and runs aapt; no new actions, secrets or permissions. No filter/risk/redaction/credential code touched.Threat model: no changeis justified. Assignedsecurity-paths.txtpaths are not touched.
Manual checks pending
- Notification with "Disconnect all" and the app icon after backgrounding with an open session; microphone permission prompt on voice input (Android device or emulator)
Follow-ups
None.
There was a problem hiding this comment.
Review
Verdict: approved
Review level: normal
Blocking
None.
Non-blocking
- Carried over from the previous review: the doc comment in
crates/tauri-plugin-android-host/src/lib.rscites ADR 0153, but the new ADR is 0154. The PR body names a desktop test that was dropped. Both are cosmetic.
Verified
- follow-up round: the only change since the approved commit a8f83d8 is a merge of
develop. The PR's own files (plugin crate, app-shell android files, manifest check script, Spec 0114) have an identical diff against the merge base before and after (executed: git diff + sha1sum). - CI on the new head: all checks green, including Android debug APK, the launch smoke test, and tests on all three OSes (executed:
gh pr checks 990). - ADR number 0154 is unique on the base branch, and no other open PR touches it (executed: ls, gh pr list).
- tests run locally: none. The Rust toolchain could not be installed in this container (permission denied), so I relied on CI.
- counter-proof, UI, data: not applicable (packaging refactoring).
Timing analysis
- no test files changed
Spec
- visible behaviour changed: none
- spec searched earlier (terms: Hintergrund, foreground, Mikrofon, android); covering spec 0114 §4, updated in the PR
- verdict: spec change present and matches the code
Security checks
- Unchanged since the previous review. Permissions and service entry are unchanged, and no security-critical paths are touched.
Manual checks pending
- Notification with "Disconnect all" and the app icon after backgrounding with an open session; microphone permission prompt on voice input (Android device or emulator)
Follow-ups
None.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #888
Summary
The foreground session service and the microphone permission plugin (Kotlin, manifest entries, strings,
lifecycle-process) now ship in the new cratecrates/tauri-plugin-android-hostwith its own Android library module (dev.smartssh.androidhost).app-shelluses its two constructors (session_service(),microphone()); the community app lists the crate as a direct Android dependency and loses the moved files, manifest lines and strings. The notification icon is now the host app's icon at runtime (applicationInfo.icon).Acceptance Criteria
build.rs,android/library, three Kotlin classes, manifest, strings: met (crates/tauri-plugin-android-host).MainActivity.kt; app manifest has no moved entries: met.scripts/check-android-apk-manifest.shruns inandroid-build: met in code; verified in CI on the built APK (jobAndroid debug APKgreen) and locally against a stubbedaaptdump (pass and fail case).tauri.settings.gradlelists the library (CI step): met (same CI step, green).cargo clippy -p app-shell -p tauri-plugin-android-host --all-targets -D warnings: met (the desktop unit test was dropped: a test binary linking Tauri cannot start on the Windows runner);cargo check -p app-shell --target aarch64-linux-androidcompiles.Tests
Desktop unit test
tauri-plugin-android-host/src/lib.rs:plugins_are_named_and_build_on_desktop. No regression test applies (refactoring); no full gate run locally.Assumptions
app-shelldepends on it unconditionally.minSdk26 andcompileSdk36 as in the app. Intent action strings now use the library namespace prefix (internal).CI changes
.github/workflows/community.yml, jobandroid-build, new step after the 16 KB alignment check: grepstauri.settings.gradlefor the library and runsscripts/check-android-apk-manifest.shon each built APK. No new actions, secrets or permissions.Spec
updated 0114 (§4)
Threat model: no change – same permissions and service, only the packaging of the Kotlin code moves.
Manual test steps
Follow-ups
None.
🤖 Generated with Claude Code