Skip to content

feat(core): back button event on Android, closes #8142#19

Open
tomerqodo wants to merge 2 commits intoqodo_combined_20260121_qodo_grep_cursor_copilot_1_base_featcore_back_button_event_on_android_closes_8142_pr162from
qodo_combined_20260121_qodo_grep_cursor_copilot_1_head_featcore_back_button_event_on_android_closes_8142_pr162
Open

feat(core): back button event on Android, closes #8142#19
tomerqodo wants to merge 2 commits intoqodo_combined_20260121_qodo_grep_cursor_copilot_1_base_featcore_back_button_event_on_android_closes_8142_pr162from
qodo_combined_20260121_qodo_grep_cursor_copilot_1_head_featcore_back_button_event_on_android_closes_8142_pr162

Conversation

@tomerqodo
Copy link
Copy Markdown

Benchmark PR from qodo-benchmark#162

@qodo-code-review
Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (2) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Unsafe activity cast 📘 Rule violation ⛯ Reliability
Description
AppPlugin force-casts activity to AppCompatActivity when registering the back-press
  callback, which can crash with ClassCastException if a non-AppCompatActivity is ever passed.
• This is an unhandled edge case at a critical input boundary (Android host activity type) and
  provides no graceful fallback or actionable error context.
Code

crates/tauri/mobile/android/src/main/java/app/tauri/AppPlugin.kt[46]

+    (activity as AppCompatActivity).onBackPressedDispatcher.addCallback(activity, callback)
Evidence
The compliance rule requires handling potential failure points and edge cases. The new code
introduces a force-cast (activity as AppCompatActivity) without validation or fallback, which can
throw at runtime and is not handled.

Rule 3: Generic: Robust Error Handling and Edge Case Management
crates/tauri/mobile/android/src/main/java/app/tauri/AppPlugin.kt[27-47]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`AppPlugin` force-casts `activity` to `AppCompatActivity`, which can throw at runtime and is not handled.

## Issue Context
This is a platform integration boundary; any mismatch in the host activity type should not cause an unhandled crash.

## Fix Focus Areas
- crates/tauri/mobile/android/src/main/java/app/tauri/AppPlugin.kt[17-47]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Android build compile error 🐞 Bug ✓ Correctness
Description
• On Android builds, AppPlugin(*handle_ref) attempts to move a non-Copy PluginHandle out of a
  shared reference, which should not compile.
• This is gated by #[cfg(target_os = "android")], so it can slip through non-Android CI while
  still breaking Android builds.
• Fix by moving handle directly into state or cloning it explicitly (no ref+deref).
Code

crates/tauri/src/app/plugin.rs[R138-140]

+        let handle = _api.register_android_plugin("app.tauri", "AppPlugin")?;
+        let handle_ref = &handle;
+        _app.manage(AppPlugin(*handle_ref));
Evidence
PluginHandle is Clone but not Copy; dereferencing &PluginHandle (*handle_ref) requires
Copy (otherwise it’s a move out of a shared ref, which is rejected by Rust). The PR uses
*handle_ref inside the Android-only setup.

crates/tauri/src/app/plugin.rs[135-141]
crates/tauri/src/plugin.rs[123-138]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Android-only setup code attempts to construct `AppPlugin(*handle_ref)` from a `&PluginHandle`, but `PluginHandle` is not `Copy`, so this should fail to compile for Android targets.

### Issue Context
`PluginHandle` implements `Clone` but not `Copy`. Dereferencing a shared reference (`*handle_ref`) requires `Copy`; otherwise it’s a move out of a shared reference.

### Fix Focus Areas
- crates/tauri/src/app/plugin.rs[135-141]
- crates/tauri/src/plugin.rs[123-138]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Listener fallback never triggers 🐞 Bug ✓ Correctness
Description
• The new onBackButtonPress API relies on addPluginListener, which tries register_listener and
  intends to fall back to registerListener.
• That fallback uses synchronous try/catch around a Promise (invoke(...).then(...)), so
  command-not-found rejections won’t be caught and the fallback will never execute.
• On Android, plugin commands are keyed strictly by method name (camelCase), so
  register_listener/remove_listener won’t match registerListener/removeListener; this likely
  breaks back-button subscription and unsubscription.
Code

packages/api/src/app.ts[R267-274]

+async function onBackButtonPress(
+  handler: (payload: OnBackButtonPressPayload) => void
+): Promise<PluginListener> {
+  return addPluginListener<OnBackButtonPressPayload>(
+    'app',
+    'back-button',
+    handler
+  )
Evidence
The PR adds onBackButtonPress which calls addPluginListener. In core.ts, addPluginListener
uses try/catch around a Promise chain, which won’t catch promise rejection. Android plugin
dispatch maps commands by exact method name (no snake_case normalization) and exposes
registerListener/removeListener.

packages/api/src/app.ts[267-275]
packages/api/src/core.ts[156-200]
crates/tauri/mobile/android/src/main/java/app/tauri/plugin/PluginHandle.kt[138-157]
crates/tauri/mobile/android/src/main/java/app/tauri/plugin/Plugin.kt[153-180]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`addPluginListener` uses `try/catch` around a Promise, so it does not catch asynchronous rejections. This breaks the intended fallback to camelCase commands. `PluginListener.unregister()` also lacks any fallback and always calls `remove_listener`.

### Issue Context
Android plugin dispatch keys commands strictly by method name (`registerListener`/`removeListener`), so snake_case calls can fail.

### Fix Focus Areas
- packages/api/src/core.ts[156-200]
- packages/api/src/app.ts[267-275]
- crates/tauri/mobile/android/src/main/java/app/tauri/plugin/PluginHandle.kt[138-167]
- crates/tauri/mobile/android/src/main/java/app/tauri/plugin/Plugin.kt[153-180]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. Misleading _app/_api names 📘 Rule violation ✓ Correctness
Description
• The .setup(|_app, _api| { ... }) closure uses underscore-prefixed parameter names even though
  they are used, which is misleading and reduces readability.
• The extra handle_ref indirection further obscures intent and makes the code less
  self-documenting.
Code

crates/tauri/src/app/plugin.rs[R135-141]

+    .setup(|_app, _api| {
+      #[cfg(target_os = "android")]
+      {
+        let handle = _api.register_android_plugin("app.tauri", "AppPlugin")?;
+        let handle_ref = &handle;
+        _app.manage(AppPlugin(*handle_ref));
+      }
Evidence
The naming rule requires identifiers to clearly express purpose and avoid misleading names.
Underscore-prefixed parameters conventionally indicate intentionally unused variables, but _app
and _api are actively used; handle_ref adds unnecessary indirection.

Rule 2: Generic: Meaningful Naming and Self-Documenting Code
crates/tauri/src/app/plugin.rs[135-141]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new setup closure uses underscore-prefixed names for variables that are actually used, and adds unnecessary `handle_ref` indirection.

## Issue Context
Clear naming improves maintainability and reduces confusion during review and future changes.

## Fix Focus Areas
- crates/tauri/src/app/plugin.rs[135-141]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



ⓘ The new review experience is currently in Beta. Learn more

Qodo Logo

}
}
}
(activity as AppCompatActivity).onBackPressedDispatcher.addCallback(activity, callback)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Unsafe activity cast 📘 Rule violation ⛯ Reliability

AppPlugin force-casts activity to AppCompatActivity when registering the back-press
  callback, which can crash with ClassCastException if a non-AppCompatActivity is ever passed.
• This is an unhandled edge case at a critical input boundary (Android host activity type) and
  provides no graceful fallback or actionable error context.
Agent prompt
## Issue description
`AppPlugin` force-casts `activity` to `AppCompatActivity`, which can throw at runtime and is not handled.

## Issue Context
This is a platform integration boundary; any mismatch in the host activity type should not cause an unhandled crash.

## Fix Focus Areas
- crates/tauri/mobile/android/src/main/java/app/tauri/AppPlugin.kt[17-47]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +135 to +141
.setup(|_app, _api| {
#[cfg(target_os = "android")]
{
let handle = _api.register_android_plugin("app.tauri", "AppPlugin")?;
let handle_ref = &handle;
_app.manage(AppPlugin(*handle_ref));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Misleading _app/_api names 📘 Rule violation ✓ Correctness

• The .setup(|_app, _api| { ... }) closure uses underscore-prefixed parameter names even though
  they are used, which is misleading and reduces readability.
• The extra handle_ref indirection further obscures intent and makes the code less
  self-documenting.
Agent prompt
## Issue description
The new setup closure uses underscore-prefixed names for variables that are actually used, and adds unnecessary `handle_ref` indirection.

## Issue Context
Clear naming improves maintainability and reduces confusion during review and future changes.

## Fix Focus Areas
- crates/tauri/src/app/plugin.rs[135-141]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +138 to +140
let handle = _api.register_android_plugin("app.tauri", "AppPlugin")?;
let handle_ref = &handle;
_app.manage(AppPlugin(*handle_ref));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

3. Android build compile error 🐞 Bug ✓ Correctness

• On Android builds, AppPlugin(*handle_ref) attempts to move a non-Copy PluginHandle out of a
  shared reference, which should not compile.
• This is gated by #[cfg(target_os = "android")], so it can slip through non-Android CI while
  still breaking Android builds.
• Fix by moving handle directly into state or cloning it explicitly (no ref+deref).
Agent prompt
### Issue description
Android-only setup code attempts to construct `AppPlugin(*handle_ref)` from a `&PluginHandle`, but `PluginHandle` is not `Copy`, so this should fail to compile for Android targets.

### Issue Context
`PluginHandle` implements `Clone` but not `Copy`. Dereferencing a shared reference (`*handle_ref`) requires `Copy`; otherwise it’s a move out of a shared reference.

### Fix Focus Areas
- crates/tauri/src/app/plugin.rs[135-141]
- crates/tauri/src/plugin.rs[123-138]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread packages/api/src/app.ts
Comment on lines +267 to +274
async function onBackButtonPress(
handler: (payload: OnBackButtonPressPayload) => void
): Promise<PluginListener> {
return addPluginListener<OnBackButtonPressPayload>(
'app',
'back-button',
handler
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

4. Listener fallback never triggers 🐞 Bug ✓ Correctness

• The new onBackButtonPress API relies on addPluginListener, which tries register_listener and
  intends to fall back to registerListener.
• That fallback uses synchronous try/catch around a Promise (invoke(...).then(...)), so
  command-not-found rejections won’t be caught and the fallback will never execute.
• On Android, plugin commands are keyed strictly by method name (camelCase), so
  register_listener/remove_listener won’t match registerListener/removeListener; this likely
  breaks back-button subscription and unsubscription.
Agent prompt
### Issue description
`addPluginListener` uses `try/catch` around a Promise, so it does not catch asynchronous rejections. This breaks the intended fallback to camelCase commands. `PluginListener.unregister()` also lacks any fallback and always calls `remove_listener`.

### Issue Context
Android plugin dispatch keys commands strictly by method name (`registerListener`/`removeListener`), so snake_case calls can fail.

### Fix Focus Areas
- packages/api/src/core.ts[156-200]
- packages/api/src/app.ts[267-275]
- crates/tauri/mobile/android/src/main/java/app/tauri/plugin/PluginHandle.kt[138-167]
- crates/tauri/mobile/android/src/main/java/app/tauri/plugin/Plugin.kt[153-180]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant