Prompt for POST_NOTIFICATIONS so Android live activities can start (discussion #5444) - #5505
Merged
shai-almog merged 17 commits intoAug 2, 2026
Merged
Conversation
…iscussion #5444) On Android 13+ the ongoing notification a live activity lowers to needs the POST_NOTIFICATIONS runtime permission. The build declares it in the manifest whenever surfaces.json sets "liveActivities": true, but nothing ever asked the user for it: the only runtime requests in the port are on the push path, the local-notification path and the explicit requestNotificationPermission API, none of which run for a surfaces-only app, and the default targetSdkVersion is well past 32 so there is no legacy auto-prompt either. The result on a fresh install was a silent dead end. areNotificationsEnabled() reads false while the permission is ungranted, so isLiveActivitySupported() returned false, LiveActivity.start() bailed to an inert handle before ever reaching the bridge, and update()/end() were no-ops. No exception, no log. The simulator can't catch it -- notifications are enabled there, so SurfaceDiagnostics.inertActivity never fires. The developer guide meanwhile promised "Android 13 and newer prompts for the notification permission, which the build declares for you", which was simply untrue. isSupported() stops conflating "the user turned notifications off" with "nobody has asked yet". From API 33, when areNotificationsEnabled() is false it now checks why: granted-but-disabled is a settled choice and stays unsupported, merely-ungranted is a pending prompt and counts as supported. That distinction is what makes the fix reachable at all -- reporting false there would make the core skip the very call that prompts. start() then raises the prompt through the existing checkForPermission(..., forceAsk=true) path, the same one requestNotificationPermission uses, which blocks the caller while keeping the EDT pumping and skips the CN1 rationale dialog (so it is safe off the EDT, where the guide tells apps to publish from). update() and end() never prompt: they act on an activity that was already granted at start. Three refusals are now distinguished and logged to CN1Surfaces instead of failing silently -- no foreground activity to prompt from (the refusal flag is left alone so the next foreground start still asks), POST_NOTIFICATIONS absent from the manifest because surfaces.json lacks "liveActivities": true (an undeclared permission is auto-denied with no UI, which would otherwise be remembered as a user refusal), and an actual decline. A decline is persisted in the surface store so isSupported() reports false from then on rather than nagging on every start; it can't trap an app, because enabling notifications later in the system settings makes areNotificationsEnabled() true and the flag is never consulted. The guide sentence is now true and gained the specifics, as did the Android per-platform notes and the LiveActivity.start/isSupported javadoc. android module compiles with SpotBugs at zero findings; core-unittests Surface* 51/51. Not exercised on hardware -- the port has no unit-test harness -- so the grant/decline/background-start paths still want a manual pass on an Android 13+ device. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses Android 13+ behavior where live activities (lowered to ongoing notifications) were effectively unreachable because POST_NOTIFICATIONS was declared but never requested at runtime. It adds a first-start prompt path, tracks user refusal to avoid repeated prompts, and updates developer documentation/Javadoc to match the new behavior.
Changes:
- Persist a “notifications permission refused” flag for live activities on Android (
CN1SurfaceStore). - Update Android live-activity support and start logic to distinguish “pending prompt” vs “disabled”, and to raise
POST_NOTIFICATIONSpermission prompt on first start (CN1LiveActivityManager). - Update surfaces developer guide and
LiveActivityJavadoc to describe Android 13+ prompting/refusal behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Ports/Android/src/com/codename1/impl/android/surfaces/CN1SurfaceStore.java | Adds persisted flag for remembered notification permission refusal. |
| Ports/Android/src/com/codename1/impl/android/surfaces/CN1LiveActivityManager.java | Adds permission prompting and refusal handling for POST_NOTIFICATIONS; adjusts support detection. |
| docs/developer-guide/External-Surfaces.asciidoc | Updates Android live-activity documentation to reflect prompt/refusal and foreground requirement. |
| CodenameOne/src/com/codename1/surfaces/LiveActivity.java | Updates public API docs to describe Android 13+ permission prompt/refusal behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b5075bceb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
… precisely Three review findings, all correct. Codex P1: AndroidNativeUtil.getActivity() stays non-null after onStop, so a null check does not prove there is UI to prompt from -- a background fetch or push handler running with the app stopped would have tried to raise the dialog anyway. Check CodenameOneActivity.isBackground() instead. (The comment's other scenario, getActivity() returning a CodenameOneBackgroundFetchActivity, cannot happen: setActivity is typed to CodenameOneActivity. The instanceof still earns its keep by making the cast total.) Codex P2: a dismissed dialog was latched as a permanent refusal. The platform hands back one bare boolean for an explicit "Don't allow", a dialog swiped away without choosing, and a request auto-denied with no UI at all, so the first false cannot be read as a decision. Count prompts instead of latching a flag, bounded at two -- Android auto-denies after two refusals, so a third attempt would never reach the user. Only attempts that actually reached the user count: no foreground activity and a missing manifest entry both return without spending one. Codex P2: denials made outside surfaces (push registration, Display.requestNotificationPermission, local notifications) left the old surfaces-private flag unset, so isSupported() claimed true. Nothing is now consulted before the live permission state, so a grant from any of those paths takes effect immediately and clears the count, and a denial from them costs at most the two bounded attempts -- which show no UI once the system is auto-denying -- before isSupported() settles on false. Copilot: isPermissionDeclared() could not tell "no permissions declared" from "lookup failed" because getRequestedPermissions() flattens both into an empty list, and it can throw NPE of its own on a null requestedPermissions array. Read PackageInfo directly instead and answer the narrower question the caller actually has -- isPermissionMissing() is true only when the manifest was read and POST_NOTIFICATIONS is definitely absent, so an unreadable manifest proceeds to the prompt rather than being mistaken for a broken build. SpotBugs rejected the first cut of that helper (NP_BOOLEAN_RETURN_NULL on a three-state Boolean); the boolean phrasing above is both cleaner and clean under the gate. Docs follow: the guide, the Android per-platform notes and the LiveActivity javadoc now describe two bounded attempts rather than a single latched refusal. android module compiles with SpotBugs at zero findings; core-unittests Surface* 51/51. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2717c61356
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
Cloudflare Preview
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Ports/Android/src/com/codename1/impl/android/surfaces/CN1LiveActivityManager.java:229
recordNotificationPrompt(ctx)is executed before the permission request. This means a successful grant can still briefly persist an incremented prompt count (it is later cleared), and in crash/race/exception scenarios it can leave a stale count even though the user granted (or the dialog never showed). This can incorrectly exhaust the 2-attempt budget after a later revoke. Recording should happen only after the request returns ungranted (i.e., only for outcomes that “didn’t end up granted”, per the method’s doc).
CN1SurfaceStore.recordNotificationPrompt(ctx);
boolean granted;
try {
granted = AndroidImplementation.checkForPermission(
"android.permission.POST_NOTIFICATIONS",
"This is required to show live activities", true);
} catch (Throwable t) {
… only answers Codex P2: isSupported() returns early the moment areNotificationsEnabled() is true, so an app that observes a grant only through that call never cleared the stored prompt count. A later revoke then left one attempt instead of the two CN1SurfaceStore.clearNotificationPrompts documents. Clear it there too -- observing the grant is what retires earlier refusals, not the path it arrived by. Copilot: recordNotificationPrompt ran before the request, so a granted outcome transiently incremented the count, and an app killed while the dialog was up spent an attempt on a question the user never answered. Record only once the request comes back ungranted; the throw path does not record either, for the same reason. This also matches what the counter's own javadoc claims to count. Vale (the failing `build` job): Microsoft.Auto rejects the hyphen in "auto-denies". Reworded to "Android stops showing the dialog after two refusals anyway", which reads better than the unhyphenated spelling would. android module compiles with SpotBugs at zero findings; core-unittests Surface* 51/51; vale clean on the developer guide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9597059ae3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2: LiveActivity.start is documented as callable from any thread, and checkForPermission drives the activity's one shared requestForPermission flag and request code. Two concurrent starts on a fresh install therefore both passed canPromptAgain, both entered the request, and a single dialog outcome released both callers -- which then each recorded an attempt, spending the whole two-prompt budget on one user answer. Guard the request with a static lock and re-read the permission under it, so a caller that waited sees whatever the winner produced: granted returns true without a second dialog, and an outcome is counted once. The re-check has to be inside the lock rather than only at the top, otherwise the waiter still acts on the state it sampled before blocking. Holding the lock across a user-visible dialog is intended -- start already documents that it blocks the calling thread until the user answers -- and it is the only lock on this path, so there is no ordering hazard. android module compiles with SpotBugs at zero findings; core-unittests Surface* 51/51. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Ports/Android/src/com/codename1/impl/android/surfaces/CN1LiveActivityManager.java:112
- In start(), the short-circuit
!isSupported(ctx) || ...prevents ensureNotificationPermission() from running when the prompt budget is exhausted (the exact state where isSupported() returns false on API 33+). This makes ensureNotificationPermission()'s "refused twice" logging unreachable and leaves the budget check duplicated in two places. Consider running ensureNotificationPermission() before isSupported() (with explicit ctx/API guards) so the refusal reason can still be logged and the logic stays consistent.
/// Raises the `POST_NOTIFICATIONS` prompt first when Android 13+ needs it, blocking the
/// calling thread until the user answers.
public static String start(Context ctx, String descriptorJson, Map<String, byte[]> images) {
docs/developer-guide/External-Surfaces.asciidoc:180
- This sentence says the prompt attempt is only counted when it "reached the user", but the implementation can’t reliably distinguish a visible dialog from an OS auto-deny; what it actually avoids counting is cases where no request can be issued (no foreground activity / permission missing) or where the request didn’t complete (exception). Suggest rephrasing to match the behavior.
Widgets are rendered through `RemoteViews` by generated per-kind providers; no Android-specific build hints are needed, and the per-kind sizing metadata comes from `surfaces.json`. Timeline entry flips are scheduled with inexact alarms (a 30-second window) to avoid the exact-alarm permission by default; apps that need to-the-second flips can opt in with the `android.surfaces.exactAlarms` build hint. Second-precision countdowns still tick natively through `Chronometer`. Live activities lower to ongoing notifications, which on Android 13 and newer require the `POST_NOTIFICATIONS` runtime permission: the build declares it for you when `surfaces.json` sets `"liveActivities": true`, and the first `LiveActivity.start(...)` raises the system prompt, blocking the calling thread until the user answers. Codename One raises it at most twice across an install -- Android stops showing the dialog after two refusals anyway -- and only counts an attempt that reached the user, so a dismissed dialog costs one attempt rather than disabling live activities outright. Start the first activity while your app is in the foreground: there is no UI to prompt from in a background service or push handler, so a start from one before the permission is granted is refused without spending an attempt (`adb logcat -s CN1Surfaces` says which case you hit). A grant from anywhere counts -- these prompts, push registration, `Display.requestNotificationPermission(...)` or the system settings -- because the live permission state is always checked first. The approximations listed in the node catalog table apply: font weights collapse to regular/bold, circular progress falls back to linear, relative dates refresh only on entry flips, and vector nodes render as bitmaps.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d74fc51e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Ports/Android/src/com/codename1/impl/android/surfaces/CN1LiveActivityManager.java:115
start()short-circuits onisSupported(ctx)before callingensureNotificationPermission(ctx). When API 33+ returnsfalsefromisSupported()due solely to the prompt budget being exhausted (canPromptAgain()==false), this preventsensureNotificationPermission()from emitting its warning log about the two refusals, reintroducing a silent failure mode.
public static String start(Context ctx, String descriptorJson, Map<String, byte[]> images) {
if (!isSupported(ctx) || !ensureNotificationPermission(ctx)) {
return null;
}
docs/developer-guide/External-Surfaces.asciidoc:180
- This paragraph says the permission prompt logic "only counts an attempt that reached the user", but the implementation counts any completed request that returns ungranted (including cases where Android auto-denies without showing UI). The code only avoids counting cases where it cannot make a request at all (e.g. no foreground activity, missing manifest permission, or an exception).
Widgets are rendered through `RemoteViews` by generated per-kind providers; no Android-specific build hints are needed, and the per-kind sizing metadata comes from `surfaces.json`. Timeline entry flips are scheduled with inexact alarms (a 30-second window) to avoid the exact-alarm permission by default; apps that need to-the-second flips can opt in with the `android.surfaces.exactAlarms` build hint. Second-precision countdowns still tick natively through `Chronometer`. Live activities lower to ongoing notifications, which on Android 13 and newer require the `POST_NOTIFICATIONS` runtime permission: the build declares it for you when `surfaces.json` sets `"liveActivities": true`, and the first `LiveActivity.start(...)` raises the system prompt, blocking the calling thread until the user answers. Codename One raises it at most twice across an install -- Android stops showing the dialog after two refusals anyway -- and only counts an attempt that reached the user, so a dismissed dialog costs one attempt rather than disabling live activities outright. Start the first activity while your app is in the foreground: there is no UI to prompt from in a background service or push handler, so a start from one before the permission is granted is refused without spending an attempt (`adb logcat -s CN1Surfaces` says which case you hit). A grant from anywhere counts -- these prompts, push registration, `Display.requestNotificationPermission(...)` or the system settings -- because the live permission state is always checked first. The approximations listed in the node catalog table apply: font weights collapse to regular/bold, circular progress falls back to linear, relative dates refresh only on entry flips, and vector nodes render as bitmaps.
… claims Copilot: start() tested isSupported() first, which reports false exactly when the prompt budget is spent, so it short-circuited past the one branch that logs why the start was refused -- the case a developer most needs to see, in a PR whose whole point is that this failure used to be silent. Ask for the permission first. Nothing gets prompted that isSupported() would have rejected on capability grounds, because POST_NOTIFICATIONS only exists from API 33 and live activities need API 24. Copilot: the guide claimed attempts are only counted when the prompt "reached the user", which the implementation cannot tell -- an OS auto-deny is reported exactly like a refusal. What it actually skips counting is a request it never issued (no foreground activity, permission missing from the manifest) or one that threw. Reworded to say that, and to admit a dismissed dialog does cost an attempt. Codex: PERMISSION_LOCK serializes surfaces against itself, not against a camera or location request in flight elsewhere -- those share the same activity-wide requestForPermission flag and request code 1, and their callback can release this caller early. That is a pre-existing defect in the shared permission machinery, and fixing it properly means per-request completion state in AndroidImplementation, which touches every permission in the port and wants its own change. Documented at the lock instead, with why the damage is bounded: a spuriously counted attempt costs one of two prompts, and any later grant from any source clears the count. android module compiles with SpotBugs at zero findings; core-unittests Surface* 51/51; vale clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Review status through
Copilot re-reported both against All six inline threads are resolved. One was closed with a partial decline rather than a fix — codex correctly noted that Still outstanding and not something I can close from here: none of the permission state machine has run on hardware. The two-attempt budget, the foreground gate, the manifest check and the lock are all reasoned. A pass on an Android 13+ device — grant, decline twice, dismiss, background start — is worth doing before release. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c10300f38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…pe the budget to the install Codex: an app that publishes widgets but never sets "liveActivities": true gets no POST_NOTIFICATIONS declaration from the build, and isSupported() then returned true forever on API 33+ -- ungranted, count zero -- while every start() stopped at the manifest check without spending an attempt, so the count could never settle it. Deliberately not counting those attempts is what made the lie permanent. isSupported() now consults the manifest too. The diagnostic still reaches the log because round 4 moved ensureNotificationPermission ahead of isSupported in start(). The manifest lookup is now cached: isSupported is called per screen or per frame by real apps, and a binder round trip per call is not free. Only a definite verdict is cached -- a failed lookup is retried rather than frozen -- and a manifest cannot change under a live process, since an app update kills it. Codex: Codename One builds allow backup by default, so the cn1surfaces preferences ride along to a reinstall or a new device. A restored count of 2 would suppress the dialog forever on what the API documents as a fresh install: the same permanent silent failure this whole path exists to remove. The count is now stamped with the installation that earned it, using firstInstallTime -- preserved across app updates, which must not hand back a spent budget, and changed by a genuine reinstall or a restore onto another device. A count whose stamp does not match reads as zero. That keeps it self-contained; excluding the file from backup would have meant build-side backup rules for one integer. android module compiles with SpotBugs at zero findings; core-unittests Surface* 51/51. Unrelated CI flake on the previous head: HealthEdtDeliveryTest .aFacadeActionDeliversOnTheEdt failed once in build-test (8). It is an EDT timing test in com.codename1.health, nothing this branch touches, it passes locally 7/7, and master is green. This push re-runs it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba24fe660e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Ports/Android/src/com/codename1/impl/android/surfaces/CN1LiveActivityManager.java:380
- The comment in this
finallyblock says that a thrown permission request "leaves waiters free to ask rather than adopting an outcome that never happened", butansweredWhileWaitingalso treatspermissionRequestInFlightas an adopt condition. BecausepermissionRequestInFlightis intentionally cleared only in the outer finally (afterPERMISSION_LOCKis released), a waiting caller can still "adopt" a thrown/failed request in the handoff window. Either adjust the comment to match the behavior, or adjust the state machine so thrown requests don’t get adopted.
// The generation moves only for an answer, so a throw leaves waiters free to
// ask rather than adopting an outcome that never happened. The in-flight flag
// is deliberately NOT cleared here -- see the outer finally.
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
Collaborator
Author
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
The Android legs of this PR failed deterministically -- three jobs, twice each --
with DesktopMode, Media360Panorama, VRStereoScene and VideoIODecodedFrames
producing no screenshot, sixteen tests never running, and no completion marker.
The same four pass on master.
Root cause is this branch. SurfacesPublishTest does:
if (LiveActivity.isSupported()) { LiveActivity.start(...); }
Before this branch isSupported() returned false on an emulator, because
POST_NOTIFICATIONS is ungranted and areNotificationsEnabled() reads false, so
the start was skipped. Making that case report supported is the entire point of
the fix -- and it means the suite now reaches start(), which asks for the
permission. On an unattended emulator nobody answers: the requesting thread
waits on the dialog indefinitely and the modal sits on top of every screenshot
that follows, so the suite stops producing output partway through.
Grant the permission from the instrumentation entry point before the app is
launched, which is what an unattended suite has to do and what a device user
does once by hand. The suite then exercises the granted live-activity path
rather than the prompt. The grant is best-effort: below API 33 the permission
does not exist and pm grant rejects it, which must not fail the run.
Product behaviour is deliberately unchanged. Blocking the caller until the user
answers is what every other Codename One permission does through
AndroidImplementation.checkForPermission -- camera, location, contacts -- and
what LiveActivity.start already documents.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
check-copyright-headers validates added and modified files, and this one predates the gate without a header, so touching it in the previous commit tripped the check. Add the Codename One GPLv2 + Classpath Exception header the script expects; verified locally with scripts/check-copyright-headers.sh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/device-runner-app/androidTest/DeviceRunnerInstrumentationTest.java:70
- The catch-all warning message is misleading on API 33+ failures (it always says the failure is "expected below API 33"), and concatenating the Throwable drops the stack trace from logcat. This makes it harder to diagnose real failures (e.g. permission missing from manifest) where the suite may later hang on a system dialog.
/// thread waits on it indefinitely and the modal sits on top of every screenshot that follows,
/// so the suite stops emitting output partway through and never reaches its completion marker.
/// Granting up front is what a device user does once by hand, and it lets the suite exercise
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/device-runner-app/androidTest/DeviceRunnerInstrumentationTest.java:91
- The catch block logs the exception via string concatenation, which drops the stack trace. Using the Log.w overload with a Throwable will preserve useful diagnostics in cases where this fails for reasons other than "below API 33".
Log.w(TAG, "Could not grant POST_NOTIFICATIONS (expected below API 33): " + t);
Collaborator
Author
|
Compared 217 screenshots: 217 matched. |
Collaborator
Author
|
Compared 144 screenshots: 144 matched. |
Collaborator
Author
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
… stop guessing at the prompt Two findings on the legacy-target branch from round 11, both correct. Codex: the bypass returned true before isPermissionMissing ran, so an app that uses widgets but never set "liveActivities": true got an active handle for a notification Android would not post, and isSupported then settled on false down its own legacy path without ever emitting the missing-manifest diagnosis. The declaration check now comes first. Codex: returning true also meant notifyActivity posted immediately. The platform's prompt is asynchronous -- it is tied to the next activity start, not to channel creation returning -- so a fresh install was still ungranted, nothing appeared, and start nevertheless handed back a handle reporting isActive() with no repost once the user allowed. Create the channel so the platform has its trigger, re-check, and refuse this start when the permission has not arrived; the next start posts for real. isSupported's legacy branch stops guessing too. It inferred "the user was asked" from a channel existing, which does not follow, and Android exposes nothing that separates "not asked yet" from "asked and declined" -- areNotificationsEnabled is false for both. It now reports the one knowable thing, whether the permission was declared, so a legacy app keeps retrying rather than being locked out by a wrong guess. hasNotificationChannels was the only caller of that inference and is deleted rather than left as dead code. This branch has now been wrong in three consecutive rounds, in three different directions, and it cannot be exercised here: it needs an Android 13+ device running a deliberately down-targeted build. It is reasoned from documented behaviour, not observed, and deserves a device before release more than anything else in this change. android module compiles with SpotBugs at zero findings; core-unittests Surface* 51/51. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/device-runner-app/androidTest/DeviceRunnerInstrumentationTest.java:93
grantNotificationPermission()assumespm grantsucceeded and always logs "Granted" once the pipe is drained.UiAutomation.executeShellCommand()generally won't throw on command failure (e.g., permission not in manifest / not allowed), so this can silently leave POST_NOTIFICATIONS ungranted and the suite can still hang on the runtime prompt. Capture the command output and only log success when it is empty (or otherwise indicates success); also skip running the command entirely below API 33 to avoid expected failures.
private void grantNotificationPermission(String packageName) {
String command = "pm grant " + packageName + " android.permission.POST_NOTIFICATIONS";
try {
UiAutomation automation = InstrumentationRegistry.getInstrumentation().getUiAutomation();
ParcelFileDescriptor pfd = automation.executeShellCommand(command);
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a64c5afceb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ground too Codex: the legacy-target branch created the notification channel without the foreground check the modern path puts in front of its request. Creating that channel is precisely what arms the platform-owned prompt, and Android defers the prompt to the next activity launch -- so a background push or service reaching this path would ambush the user with a notification dialog the next time they opened the app, for a live activity they never saw asked for, and the start was refused anyway. Apply the same foreground gate before creating the channel. android module compiles with SpotBugs at zero findings; core-unittests Surface* 51/51. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Ports/Android/src/com/codename1/impl/android/surfaces/CN1LiveActivityManager.java:421
AndroidImplementation.checkForPermission()can returnfalsewithout ever showing/issuing a permission request (e.g. ifgetActivity()becomes null). In that case this code still treats the result as “answered”, increments the generation, and records a prompt attempt, which can exhaust the 2-attempt budget without the user ever seeing a prompt.
boolean granted = false;
boolean answered = false;
try {
granted = AndroidImplementation.checkForPermission(
"android.permission.POST_NOTIFICATIONS",
scripts/device-runner-app/androidTest/DeviceRunnerInstrumentationTest.java:93
pm grantfailures won’t throw here; they are printed to the command’s stdout/stderr stream. The current implementation drains and discards the output, then logs “Granted …” unconditionally, which can both mislead debugging and mask why the permission wasn’t actually granted (e.g. below API 33, or if the app didn’t declare the permission). Also, logging the Throwable via string concatenation drops the stack trace.
String command = "pm grant " + packageName + " android.permission.POST_NOTIFICATIONS";
try {
UiAutomation automation = InstrumentationRegistry.getInstrumentation().getUiAutomation();
ParcelFileDescriptor pfd = automation.executeShellCommand(command);
try (FileInputStream fis = new FileInputStream(pfd.getFileDescriptor())) {
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fc392caba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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.
Reported in discussion #5444: "Why the Surfaces don't work in android side? any build hint necessary to work?"
No build hint is needed. Live activities were unreachable on Android 13+.
The bug
The ongoing notification a live activity lowers to needs the
POST_NOTIFICATIONSruntime permission on API 33+. The build declares it in the manifest wheneversurfaces.jsonsets"liveActivities": true(AndroidGradleBuilder.java:3021), but nothing ever asked the user for it — the only runtime requests in the port are on the push path, the local-notification path and the explicitrequestNotificationPermissionAPI, none of which run for a surfaces-only app. The defaulttargetSdkVersionis well past 32, so there is no legacy auto-prompt either.areNotificationsEnabled()reads false while the permission is ungranted, so on a fresh install:Surfaces.isLiveActivitySupported()→falseLiveActivity.start()bailed to an inert handle before ever reaching the bridge (LiveActivity.java:90)update()/end()were silent no-opsNo exception, no log. The simulator can't catch it either — notifications are enabled there, so
SurfaceDiagnostics.inertActivitynever fires.The developer guide meanwhile promised the opposite (
External-Surfaces.asciidoc:126):The fix
isSupported()stops conflating "the user turned notifications off" with "nobody has asked yet". From API 33, whenareNotificationsEnabled()is false it checks why: granted-but-disabled is a settled choice and stays unsupported; merely-ungranted is a pending prompt and counts as supported. Below API 33 behaviour is unchanged. This distinction is what makes the fix reachable at all — reporting false there would make the core skip the very call that prompts.start()raises the prompt through the existingcheckForPermission(..., forceAsk=true)path — the same onerequestNotificationPermissionuses, so it blocks the caller while keeping the EDT pumping, and skips the CN1 rationale dialog (safe off the EDT, where the guide tells apps to publish from).update()andend()never prompt: they act on an activity that was already granted at start.Three refusals are distinguished and logged to
CN1Surfacesinstead of failing silently:POST_NOTIFICATIONSabsent from the manifest (surfaces.jsonlacks"liveActivities": true)isSupported()reports false from then on rather than nagging on every startA decline can't trap an app: enabling notifications later in the system settings makes
areNotificationsEnabled()true, which is checked first, so the flag is never consulted.Docs follow the behaviour — the guide sentence is now true and gained the specifics, as did the Android per-platform notes and the
LiveActivity.start/isSupportedjavadoc.Verification
mvn -Pcompile-android -pl android verify— compiles, SpotBugs 0 findingscore-unittests -Dtest='Surface*'— 51/51 (SurfaceTest29,SurfaceRasterizerTest22)Not exercised on hardware. The port has no unit-test harness (
maven/androidhas nosrc/test), so the permission state machine is reasoned rather than run. The grant / decline / background-start paths want a manual pass on an Android 13+ device before release.Scope note: this fixes the live-activity half. The widget half looked correct under review — receivers, per-kind provider codegen,
@xml/cn1_widget_*metadata and layout copies are all in place, andexported="false"is correct for AppWidget providers.🤖 Generated with Claude Code