From 4d090f2c1f89f6a0b7555f09eeea97a6fd7c5f9f Mon Sep 17 00:00:00 2001 From: Guilherme Albert Date: Thu, 17 Sep 2026 11:27:15 -0300 Subject: [PATCH 1/8] docs: design Play update ownership handling --- ...-09-17-play-update-app-not-owned-design.md | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-17-play-update-app-not-owned-design.md diff --git a/docs/superpowers/specs/2026-09-17-play-update-app-not-owned-design.md b/docs/superpowers/specs/2026-09-17-play-update-app-not-owned-design.md new file mode 100644 index 0000000..cb0bd92 --- /dev/null +++ b/docs/superpowers/specs/2026-09-17-play-update-app-not-owned-design.md @@ -0,0 +1,42 @@ +# Play update APP_NOT_OWNED handling + +## Context + +The signed Android `0.1.3` build checks Google Play for an in-app update at startup. When that build is sideloaded, or when the active Play account has never acquired the app, Play Core rejects the check with `Install Error(-10) / ERROR_APP_NOT_OWNED`. The provider already fails open and keeps the app usable, but it reports this expected eligibility outcome to Sentry as a technical exception. + +The internal release workflow also embeds the repository variable `EXPO_PUBLIC_SENTRY_ENVIRONMENT`. That variable is currently `development`, so signed internal releases are mislabeled even though they run with production behavior. + +## Decision + +Treat only Play Core `ERROR_APP_NOT_OWNED` (`-10`) as a non-actionable update eligibility result. The Google Play update provider will return its existing no-update response without reporting the error. Every other SDK loading, update-check, or update-start failure remains reportable and continues to fail open. + +Signed Android releases will use the Sentry environment `production`. Development builds remain excluded from Google Play update checks by the existing `!__DEV__` runtime guard. + +## Boundaries + +- Keep the handling inside `GooglePlayUpdateProvider`; UI and versioning context behavior do not change. +- Recognize the stable native error code from the rejected error text emitted by `expo-in-app-updates`. +- Do not suppress generic Play errors or all errors from sideloaded builds. +- Do not attempt to infer install origin, because the current runtime does not expose a reliable ownership signal before calling Play Core. +- Do not promote the Android release to production. + +## Data flow + +1. The versioning context requests update availability. +2. The provider calls `expo-in-app-updates.checkForUpdate()` on an Android release build. +3. A successful result is mapped as today. +4. `ERROR_APP_NOT_OWNED` maps to the existing no-update result and is not sent to telemetry. +5. Any other rejection maps to no-update and is sent to telemetry once. + +The same classification applies when the provider rechecks availability before starting a flexible or immediate update. + +## Testing + +- Add regression coverage showing that `Install Error(-10)` returns the safe fallback and is not reported. +- Cover both availability checking and update starting so the two SDK call sites cannot regress independently. +- Preserve the existing assertion that an unrelated native failure is reported. +- Run the focused versioning tests, then lint, type checking, the complete Jest suite, and Expo Doctor before integration. + +## Release and observation + +Ship the change through a protected pull request. After merge, update the GitHub Sentry environment variable to `production` and publish the next version only to Google Play internal testing. The Sentry issue can be considered prevented for new builds once a sideloaded build produces no new `APP_NOT_OWNED` event; existing events remain historical. From b9d79ebd09eeaf044e05e7e089bc9935fca5ad2a Mon Sep 17 00:00:00 2001 From: Guilherme Albert Date: Thu, 17 Sep 2026 11:29:36 -0300 Subject: [PATCH 2/8] docs: plan Play update ownership handling --- ...lay-update-app-not-owned-implementation.md | 271 ++++++++++++++++++ 1 file changed, 271 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-17-play-update-app-not-owned-implementation.md diff --git a/docs/superpowers/plans/2026-09-17-play-update-app-not-owned-implementation.md b/docs/superpowers/plans/2026-09-17-play-update-app-not-owned-implementation.md new file mode 100644 index 0000000..99bfdc1 --- /dev/null +++ b/docs/superpowers/plans/2026-09-17-play-update-app-not-owned-implementation.md @@ -0,0 +1,271 @@ +# Play update APP_NOT_OWNED Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Prevent Google Play's expected `ERROR_APP_NOT_OWNED (-10)` eligibility result from creating Sentry alerts while preserving reporting for real update failures and correctly labeling signed releases. + +**Architecture:** Keep classification at the Google Play update provider boundary, where native SDK failures already become the safe no-update fallback. Add one narrow predicate for the stable Play Core error code and use it at both check sites; leave all UI and version-policy behavior unchanged. Configure signed workflow builds as `production` in Sentry through the existing GitHub variable. + +**Tech Stack:** TypeScript, React Native, Expo, `expo-in-app-updates`, Jest, GitHub Actions, Sentry, Google Play internal testing. + +--- + +### Task 1: Add failing regression coverage + +**Files:** +- Modify: `tests/services/versioning/google-play-update-provider.test.ts` + +- [ ] **Step 1: Add a failing test for availability checks** + +Add this test after the supported-runtime test: + +```ts +it("does not report APP_NOT_OWNED while checking availability", async () => { + const sdk = createSdk(); + sdk.checkForUpdate.mockRejectedValue( + new Error( + 'Failed to check for updates: -10: Install Error(-10): The app is not owned by any user on this device.', + ), + ); + const report = jest.fn(); + const provider = new GooglePlayUpdateProvider( + async () => sdk, + () => "android", + () => true, + report, + ); + + await expect(provider.checkAvailability()).resolves.toEqual({ + available: false, + flexibleAllowed: false, + immediateAllowed: false, + storeVersion: null, + updateInProgress: false, + }); + expect(report).not.toHaveBeenCalled(); +}); +``` + +- [ ] **Step 2: Add a failing test for update starts** + +Add this test beside the availability regression: + +```ts +it("does not report APP_NOT_OWNED while starting an update", async () => { + const sdk = createSdk(); + sdk.checkForUpdate.mockRejectedValue( + new Error( + 'Failed to check for updates: -10: Install Error(-10): The app is not owned by any user on this device.', + ), + ); + const report = jest.fn(); + const provider = new GooglePlayUpdateProvider( + async () => sdk, + () => "android", + () => true, + report, + ); + + await expect(provider.startFlexibleUpdate()).resolves.toBe(false); + expect(report).not.toHaveBeenCalled(); + expect(sdk.startUpdate).not.toHaveBeenCalled(); +}); +``` + +- [ ] **Step 3: Run the focused test and verify RED** + +Run: + +```bash +npm test -- --runInBand tests/services/versioning/google-play-update-provider.test.ts +``` + +Expected: both new tests fail because `report` is called once; the pre-existing tests pass. + +- [ ] **Step 4: Commit the regression tests** + +```bash +git add tests/services/versioning/google-play-update-provider.test.ts +git commit -m "test(versioning): cover Play ownership errors" +``` + +### Task 2: Classify the expected Play ownership result + +**Files:** +- Modify: `src/services/versioning/google-play-update.provider.ts` +- Test: `tests/services/versioning/google-play-update-provider.test.ts` + +- [ ] **Step 1: Add the narrow classifier** + +Add this module-level function below `NO_UPDATE`: + +```ts +function isAppNotOwnedError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + return /(?:^|\s)Install Error\(-10\):/.test(message); +} +``` + +The expression intentionally recognizes only Play Core's stable `-10` code as formatted by `expo-in-app-updates`; it does not classify generic ownership wording or other install errors. + +- [ ] **Step 2: Suppress reporting only at the availability boundary** + +Replace the `checkAvailability` catch body with: + +```ts +} catch (error) { + if (!isAppNotOwnedError(error)) this.reportSafely(error); + return NO_UPDATE; +} +``` + +- [ ] **Step 3: Suppress reporting only at the update-start boundary** + +Replace the `startUpdate` catch body with: + +```ts +} catch (error) { + if (!isAppNotOwnedError(error)) this.reportSafely(error); + return false; +} +``` + +- [ ] **Step 4: Run the focused test and verify GREEN** + +Run: + +```bash +npm test -- --runInBand tests/services/versioning/google-play-update-provider.test.ts +``` + +Expected: all provider tests pass, including both `APP_NOT_OWNED` regressions and the existing unrelated-error reporting test. + +- [ ] **Step 5: Commit the implementation** + +```bash +git add src/services/versioning/google-play-update.provider.ts +git commit -m "fix(versioning): ignore Play ownership eligibility" +``` + +### Task 3: Verify the repository + +**Files:** +- Verify: `src/services/versioning/google-play-update.provider.ts` +- Verify: `tests/services/versioning/google-play-update-provider.test.ts` + +- [ ] **Step 1: Run lint** + +```bash +npm run lint +``` + +Expected: exit code 0 with no warnings or errors. + +- [ ] **Step 2: Run type checking** + +```bash +npm run typecheck +``` + +Expected: exit code 0 with no TypeScript errors. + +- [ ] **Step 3: Run the complete Jest suite** + +```bash +npm test -- --runInBand +``` + +Expected: every suite and test passes with zero failures. + +- [ ] **Step 4: Run Expo Doctor** + +```bash +npm run doctor +``` + +Expected: all checks pass. + +- [ ] **Step 5: Check the final diff** + +```bash +git diff origin/main...HEAD --check +git status --short +``` + +Expected: no whitespace errors and no uncommitted implementation files. + +### Task 4: Integrate through branch protection + +**Files:** +- Push commits from `codex/play-update-app-not-owned` + +- [ ] **Step 1: Push the feature branch** + +```bash +git push -u origin codex/play-update-app-not-owned +``` + +Expected: the remote branch is created without modifying `main` directly. + +- [ ] **Step 2: Open a pull request** + +```bash +gh pr create --repo openings-dev/mobile --base main --head codex/play-update-app-not-owned --title "Fix Play update ownership alerts" --body "Treat Google Play APP_NOT_OWNED as an expected eligibility result while preserving telemetry for real failures. Includes regression coverage for availability checks and update starts." +``` + +Expected: GitHub returns a pull request URL. + +- [ ] **Step 3: Wait for required checks** + +```bash +gh pr checks --repo openings-dev/mobile --watch "$(gh pr view --repo openings-dev/mobile --json number --jq .number)" +``` + +Expected: all required checks pass. + +- [ ] **Step 4: Squash merge the pull request** + +```bash +gh pr merge "$(gh pr view --repo openings-dev/mobile --json number --jq .number)" --repo openings-dev/mobile --squash --delete-branch +``` + +Expected: the pull request state becomes `MERGED` and protected `main` contains the fix. + +### Task 5: Correct Sentry classification and release internally + +**Files:** +- External configuration: repository variable `EXPO_PUBLIC_SENTRY_ENVIRONMENT` +- Workflow: `.github/workflows/android-internal.yml` + +- [ ] **Step 1: Set the signed-release Sentry environment** + +```bash +gh variable set EXPO_PUBLIC_SENTRY_ENVIRONMENT --repo openings-dev/mobile --body production +gh variable get EXPO_PUBLIC_SENTRY_ENVIRONMENT --repo openings-dev/mobile +``` + +Expected: the read-back value is exactly `production`. + +- [ ] **Step 2: Dispatch Android `0.1.4 (6)` to internal testing** + +```bash +gh workflow run android-internal.yml --repo openings-dev/mobile --ref main -f version_code=6 -f version_name=0.1.4 -f validate_only=false +``` + +Expected: a new workflow run is queued from the merged `main` commit. + +- [ ] **Step 3: Wait for the entire release workflow** + +```bash +gh run watch "$(gh run list --repo openings-dev/mobile --workflow android-internal.yml --branch main --limit 1 --json databaseId --jq '.[0].databaseId')" --repo openings-dev/mobile --exit-status +``` + +Expected: preflight, signed bundle build, bundle verification, and Google Play internal upload all pass. + +- [ ] **Step 4: Verify the final run independently** + +```bash +gh run view "$(gh run list --repo openings-dev/mobile --workflow android-internal.yml --branch main --limit 1 --json databaseId --jq '.[0].databaseId')" --repo openings-dev/mobile --json status,conclusion,headSha,jobs,url +``` + +Expected: status is `completed`, conclusion is `success`, and `Upload signed bundle to Google Play internal testing` succeeded. Do not promote the release to production. From b28328950f196764f28c200c79d612a03160428f Mon Sep 17 00:00:00 2001 From: Guilherme Albert Date: Thu, 17 Sep 2026 11:30:18 -0300 Subject: [PATCH 3/8] test(versioning): cover Play ownership errors --- .../google-play-update-provider.test.ts | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/services/versioning/google-play-update-provider.test.ts b/tests/services/versioning/google-play-update-provider.test.ts index a2eb61b..da20481 100644 --- a/tests/services/versioning/google-play-update-provider.test.ts +++ b/tests/services/versioning/google-play-update-provider.test.ts @@ -96,6 +96,51 @@ describe("GooglePlayUpdateProvider", () => { expect(loadSdk).not.toHaveBeenCalled(); }); + it("does not report APP_NOT_OWNED while checking availability", async () => { + const sdk = createSdk(); + sdk.checkForUpdate.mockRejectedValue( + new Error( + "Failed to check for updates: -10: Install Error(-10): The app is not owned by any user on this device.", + ), + ); + const report = jest.fn(); + const provider = new GooglePlayUpdateProvider( + async () => sdk, + () => "android", + () => true, + report, + ); + + await expect(provider.checkAvailability()).resolves.toEqual({ + available: false, + flexibleAllowed: false, + immediateAllowed: false, + storeVersion: null, + updateInProgress: false, + }); + expect(report).not.toHaveBeenCalled(); + }); + + it("does not report APP_NOT_OWNED while starting an update", async () => { + const sdk = createSdk(); + sdk.checkForUpdate.mockRejectedValue( + new Error( + "Failed to check for updates: -10: Install Error(-10): The app is not owned by any user on this device.", + ), + ); + const report = jest.fn(); + const provider = new GooglePlayUpdateProvider( + async () => sdk, + () => "android", + () => true, + report, + ); + + await expect(provider.startFlexibleUpdate()).resolves.toBe(false); + expect(report).not.toHaveBeenCalled(); + expect(sdk.startUpdate).not.toHaveBeenCalled(); + }); + it("contains and reports native failures", async () => { const report = jest.fn(); const provider = new GooglePlayUpdateProvider( From 794da35422eb6233b4d291aae3129f2f3c86dd80 Mon Sep 17 00:00:00 2001 From: Guilherme Albert Date: Thu, 17 Sep 2026 11:30:44 -0300 Subject: [PATCH 4/8] fix(versioning): ignore Play ownership eligibility --- src/services/versioning/google-play-update.provider.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/services/versioning/google-play-update.provider.ts b/src/services/versioning/google-play-update.provider.ts index d9bbc77..8fbed90 100644 --- a/src/services/versioning/google-play-update.provider.ts +++ b/src/services/versioning/google-play-update.provider.ts @@ -25,6 +25,11 @@ const NO_UPDATE: GooglePlayUpdateAvailability = { updateInProgress: false, }; +function isAppNotOwnedError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + return /(?:^|\s)Install Error\(-10\):/.test(message); +} + function reportVersioningError(error: Error): void { void import("@/services/telemetry") .then(({ captureTechnicalException }) => { @@ -56,7 +61,7 @@ export class GooglePlayUpdateProvider { updateInProgress: result.updateInProgress === true, }; } catch (error) { - this.reportSafely(error); + if (!isAppNotOwnedError(error)) this.reportSafely(error); return NO_UPDATE; } } @@ -88,7 +93,7 @@ export class GooglePlayUpdateProvider { if (!allowed) return false; return await sdk.startUpdate(mode === "immediate"); } catch (error) { - this.reportSafely(error); + if (!isAppNotOwnedError(error)) this.reportSafely(error); return false; } } From 6ff7eda565d13e9b92f2b133c12e8a61a830b315 Mon Sep 17 00:00:00 2001 From: Guilherme Albert Date: Thu, 17 Sep 2026 11:52:28 -0300 Subject: [PATCH 5/8] docs: design OneSignal initialization ordering --- ...7-onesignal-initialization-order-design.md | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-17-onesignal-initialization-order-design.md diff --git a/docs/superpowers/specs/2026-09-17-onesignal-initialization-order-design.md b/docs/superpowers/specs/2026-09-17-onesignal-initialization-order-design.md new file mode 100644 index 0000000..4349294 --- /dev/null +++ b/docs/superpowers/specs/2026-09-17-onesignal-initialization-order-design.md @@ -0,0 +1,45 @@ +# OneSignal initialization order + +## Context + +Android release `0.1.3 (5)` can crash while mounting notification navigation with `IllegalStateException: Must call 'initWithContext' before use`. The native stack ends at `RNOneSignal.addNotificationClickListener`. + +The application starts notifications at module load, but `startNotifications()` first awaits consent from asynchronous storage. React can mount `NotificationNavigation` during that wait. Its effect calls `OneSignal.Notifications.addEventListener` before `initOneSignal()` has invoked `OneSignal.initialize()`. A JavaScript `try/catch` cannot contain this failure because the React Native bridge invokes the native method asynchronously. + +## Decision + +Make listener registration initialization-aware inside the OneSignal client. Calls made before successful initialization will create pending registrations without touching `OneSignal.Notifications`. Immediately after `OneSignal.initialize()` succeeds, the client will attach every active pending registration. + +This keeps SDK lifecycle rules at the service boundary and protects current and future consumers without coupling screen rendering to notification startup. + +## Listener lifecycle + +Each registration owns one stable native listener reference and an attached flag. + +1. `addNotificationClickListener()` creates the registration. +2. If OneSignal is initialized, it attaches immediately. +3. Otherwise, it remains in a pending set without calling the native notifications API. +4. Successful initialization attaches all still-active registrations. +5. Unsubscribing before initialization removes the pending registration and never calls the native API. +6. Unsubscribing after attachment removes the exact listener reference and makes repeated cleanup a no-op. +7. Test reset clears both initialization state and pending registrations. + +If listener attachment throws synchronously after initialization, the registration remains unattached and the application continues. Notification failures must not block startup or navigation. + +## Alternatives rejected + +- Gating `NotificationNavigation` rendering on initialization would work but would couple presentation to SDK lifecycle and leave other future callers exposed. +- Dropping early listener registrations would avoid the crash but break cold-start notification navigation. +- Relying on the existing `try/catch` is insufficient because the fatal exception occurs later on the native bridge thread. + +## Testing + +- Reproduce registration before initialization and assert that the native listener method is not called. +- Initialize afterward and assert that the pending listener attaches once. +- Unsubscribe before initialization and assert that initialization does not attach it. +- Preserve payload validation and exact-listener cleanup coverage. +- Run notification tests, lint, type checking, the complete Jest suite, and Expo Doctor. + +## Release + +Integrate this change with the pending Play ownership fix through a protected pull request. Correct the Sentry release environment to `production`, then publish Android `0.1.4 (6)` only to Google Play internal testing. Do not promote to production. From 57639e04437dca3efec8d2e1ae2916dffda9abf1 Mon Sep 17 00:00:00 2001 From: Guilherme Albert Date: Thu, 17 Sep 2026 11:54:41 -0300 Subject: [PATCH 6/8] docs: plan OneSignal initialization ordering --- ...nal-initialization-order-implementation.md | 258 ++++++++++++++++++ 1 file changed, 258 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-17-onesignal-initialization-order-implementation.md diff --git a/docs/superpowers/plans/2026-09-17-onesignal-initialization-order-implementation.md b/docs/superpowers/plans/2026-09-17-onesignal-initialization-order-implementation.md new file mode 100644 index 0000000..8df5432 --- /dev/null +++ b/docs/superpowers/plans/2026-09-17-onesignal-initialization-order-implementation.md @@ -0,0 +1,258 @@ +# OneSignal Initialization Order Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Prevent notification click listeners from reaching the native OneSignal SDK before initialization while preserving cold-start click handling and cleanup. + +**Architecture:** `onesignal-client.ts` will own listener lifecycle records and defer native attachment until `initOneSignal()` has invoked `OneSignal.initialize()`. Consumers keep the existing synchronous subscribe/unsubscribe interface; pending subscriptions can be cancelled without touching native APIs. + +**Tech Stack:** TypeScript, React Native, OneSignal React Native SDK 5, Jest. + +--- + +### Task 1: Reproduce the initialization race + +**Files:** +- Modify: `tests/services/notifications/onesignal-client.test.ts` + +- [ ] **Step 1: Add a failing deferred-registration test** + +```ts +it("defers click listener registration until initialization", () => { + const onJob = jest.fn(); + const unsubscribe = addNotificationClickListener(onJob); + expect(mockAddEventListener).not.toHaveBeenCalled(); + + process.env.EXPO_PUBLIC_ONESIGNAL_APP_ID = "app-id"; + expect(initOneSignal("undecided")).toBe(true); + expect(mockAddEventListener).toHaveBeenCalledTimes(1); + + const listener = mockAddEventListener.mock.calls[0]?.[1] as ( + event: unknown, + ) => void; + listener({ + notification: { + additionalData: { + type: "openings.job", + version: 1, + jobId: "gh_1234567890abcdef12345678", + }, + }, + }); + expect(onJob).toHaveBeenCalledWith("gh_1234567890abcdef12345678"); + + unsubscribe(); + expect(mockRemoveEventListener).toHaveBeenCalledWith("click", listener); +}); +``` + +- [ ] **Step 2: Add a failing cancellation test** + +```ts +it("cancels a pending click listener without touching the native SDK", () => { + const unsubscribe = addNotificationClickListener(jest.fn()); + unsubscribe(); + + process.env.EXPO_PUBLIC_ONESIGNAL_APP_ID = "app-id"; + expect(initOneSignal("undecided")).toBe(true); + expect(mockAddEventListener).not.toHaveBeenCalled(); + expect(mockRemoveEventListener).not.toHaveBeenCalled(); +}); +``` + +- [ ] **Step 3: Initialize the existing payload-validation test** + +At the beginning of `accepts only the versioned new-job payload and removes the exact listener`, add: + +```ts +process.env.EXPO_PUBLIC_ONESIGNAL_APP_ID = "app-id"; +initOneSignal("undecided"); +``` + +- [ ] **Step 4: Run the focused suite and verify RED** + +```bash +npm test -- --runInBand tests/services/notifications/onesignal-client.test.ts +``` + +Expected: the two new tests fail because the current client calls the native listener API immediately. + +- [ ] **Step 5: Commit the regression tests** + +```bash +git add tests/services/notifications/onesignal-client.test.ts +git commit -m "test(notifications): reproduce OneSignal startup race" +``` + +### Task 2: Defer native listener attachment + +**Files:** +- Modify: `src/services/notifications/onesignal-client.ts` +- Test: `tests/services/notifications/onesignal-client.test.ts` + +- [ ] **Step 1: Define the listener event and lifecycle record** + +Add below `NotificationClickPayload`: + +```ts +interface NotificationClickEvent { + notification?: { additionalData?: unknown }; +} + +interface NotificationClickRegistration { + active: boolean; + attached: boolean; + listener: (event: NotificationClickEvent) => void; +} +``` + +- [ ] **Step 2: Add the registration set and guarded attachment helper** + +Replace the single initialization state declaration with: + +```ts +let initialized = false; +const clickRegistrations = new Set(); + +function attachNotificationClickListener( + registration: NotificationClickRegistration, +): void { + if (!initialized || !registration.active || registration.attached) return; + try { + OneSignal.Notifications.addEventListener("click", registration.listener); + registration.attached = true; + } catch { + // Listener failures never block application startup. + } +} +``` + +- [ ] **Step 3: Flush pending listeners after initialization** + +Immediately after `initialized = true` inside `initOneSignal`, add: + +```ts +for (const registration of clickRegistrations) { + attachNotificationClickListener(registration); +} +``` + +- [ ] **Step 4: Replace direct listener registration with lifecycle records** + +Replace `addNotificationClickListener` with: + +```ts +export function addNotificationClickListener( + onJob: (jobId: string) => void, +): () => void { + const registration: NotificationClickRegistration = { + active: true, + attached: false, + listener: (event) => { + const payload = parseNotificationClickPayload( + event.notification?.additionalData, + ); + if (payload) onJob(payload.jobId); + }, + }; + clickRegistrations.add(registration); + attachNotificationClickListener(registration); + + return () => { + if (!registration.active) return; + registration.active = false; + clickRegistrations.delete(registration); + if (!registration.attached) return; + try { + OneSignal.Notifications.removeEventListener( + "click", + registration.listener, + ); + } catch { + // Listener cleanup failure must not affect app navigation. + } + }; +} +``` + +- [ ] **Step 5: Clear lifecycle state in the test reset** + +```ts +export function resetOneSignalClientForTests(): void { + initialized = false; + clickRegistrations.clear(); +} +``` + +- [ ] **Step 6: Run the focused suite and verify GREEN** + +```bash +npm test -- --runInBand tests/services/notifications/onesignal-client.test.ts +``` + +Expected: all OneSignal client tests pass and early registration produces zero native calls before initialization. + +- [ ] **Step 7: Commit the fix** + +```bash +git add src/services/notifications/onesignal-client.ts +git commit -m "fix(notifications): defer listeners until OneSignal init" +``` + +### Task 3: Verify and release both fixes + +**Files:** +- Verify: `src/services/notifications/onesignal-client.ts` +- Verify: `src/services/versioning/google-play-update.provider.ts` +- Verify: `tests/services/notifications/onesignal-client.test.ts` +- Verify: `tests/services/versioning/google-play-update-provider.test.ts` + +- [ ] **Step 1: Run all repository checks** + +```bash +npm run lint +npm run typecheck +npm test -- --runInBand +npm run doctor +git diff origin/main...HEAD --check +git status --short +``` + +Expected: lint, type checking, Jest, and Expo Doctor pass; the diff has no whitespace errors; the worktree has no uncommitted implementation files. + +- [ ] **Step 2: Push and open a protected pull request** + +```bash +git push -u origin codex/play-update-app-not-owned +gh pr create --repo openings-dev/mobile --base main --head codex/play-update-app-not-owned --title "Fix Android startup monitoring issues" --body "Prevent expected Play APP_NOT_OWNED alerts and defer OneSignal click listeners until native initialization. Includes regression coverage for both Android startup paths." +``` + +Expected: GitHub returns a pull request URL; direct writes to protected `main` are not used. + +- [ ] **Step 3: Merge only after required checks** + +```bash +gh pr checks --repo openings-dev/mobile --watch "$(gh pr view --repo openings-dev/mobile --json number --jq .number)" +gh pr merge "$(gh pr view --repo openings-dev/mobile --json number --jq .number)" --repo openings-dev/mobile --squash --delete-branch +``` + +Expected: every required check succeeds and the pull request becomes `MERGED`. + +- [ ] **Step 4: Correct Sentry and publish internal version `0.1.4 (6)`** + +```bash +gh variable set EXPO_PUBLIC_SENTRY_ENVIRONMENT --repo openings-dev/mobile --body production +gh variable get EXPO_PUBLIC_SENTRY_ENVIRONMENT --repo openings-dev/mobile +gh workflow run android-internal.yml --repo openings-dev/mobile --ref main -f version_code=6 -f version_name=0.1.4 -f validate_only=false +``` + +Expected: Sentry reads back `production`; the workflow is queued from merged `main`. + +- [ ] **Step 5: Verify the complete internal release** + +```bash +gh run watch "$(gh run list --repo openings-dev/mobile --workflow android-internal.yml --branch main --limit 1 --json databaseId --jq '.[0].databaseId')" --repo openings-dev/mobile --exit-status +gh run view "$(gh run list --repo openings-dev/mobile --workflow android-internal.yml --branch main --limit 1 --json databaseId --jq '.[0].databaseId')" --repo openings-dev/mobile --json status,conclusion,headSha,jobs,url +``` + +Expected: bundle verification and `Upload signed bundle to Google Play internal testing` succeed. Do not promote to production. From 5df391d8207ed2fd702a6cf46e9184fe776fc2e7 Mon Sep 17 00:00:00 2001 From: Guilherme Albert Date: Thu, 17 Sep 2026 11:55:19 -0300 Subject: [PATCH 7/8] test(notifications): reproduce OneSignal startup race --- .../notifications/onesignal-client.test.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/services/notifications/onesignal-client.test.ts b/tests/services/notifications/onesignal-client.test.ts index 29fe7e2..f961d72 100644 --- a/tests/services/notifications/onesignal-client.test.ts +++ b/tests/services/notifications/onesignal-client.test.ts @@ -96,7 +96,46 @@ describe("OneSignal client", () => { expect(mockOptOut).toHaveBeenCalledTimes(1); }); + it("defers click listener registration until initialization", () => { + const onJob = jest.fn(); + const unsubscribe = addNotificationClickListener(onJob); + expect(mockAddEventListener).not.toHaveBeenCalled(); + + process.env.EXPO_PUBLIC_ONESIGNAL_APP_ID = "app-id"; + expect(initOneSignal("undecided")).toBe(true); + expect(mockAddEventListener).toHaveBeenCalledTimes(1); + + const listener = mockAddEventListener.mock.calls[0]?.[1] as ( + event: unknown, + ) => void; + listener({ + notification: { + additionalData: { + type: "openings.job", + version: 1, + jobId: "gh_1234567890abcdef12345678", + }, + }, + }); + expect(onJob).toHaveBeenCalledWith("gh_1234567890abcdef12345678"); + + unsubscribe(); + expect(mockRemoveEventListener).toHaveBeenCalledWith("click", listener); + }); + + it("cancels a pending click listener without touching the native SDK", () => { + const unsubscribe = addNotificationClickListener(jest.fn()); + unsubscribe(); + + process.env.EXPO_PUBLIC_ONESIGNAL_APP_ID = "app-id"; + expect(initOneSignal("undecided")).toBe(true); + expect(mockAddEventListener).not.toHaveBeenCalled(); + expect(mockRemoveEventListener).not.toHaveBeenCalled(); + }); + it("accepts only the versioned new-job payload and removes the exact listener", () => { + process.env.EXPO_PUBLIC_ONESIGNAL_APP_ID = "app-id"; + initOneSignal("undecided"); const onJob = jest.fn(); const unsubscribe = addNotificationClickListener(onJob); const listener = mockAddEventListener.mock.calls[0]?.[1] as (event: unknown) => void; From 40e147cceb9e9a4e39385d440df6a2601c7a601d Mon Sep 17 00:00:00 2001 From: Guilherme Albert Date: Thu, 17 Sep 2026 11:56:14 -0300 Subject: [PATCH 8/8] fix(notifications): defer listeners until OneSignal init --- .../notifications/onesignal-client.ts | 60 +++++++++++++++---- 1 file changed, 50 insertions(+), 10 deletions(-) diff --git a/src/services/notifications/onesignal-client.ts b/src/services/notifications/onesignal-client.ts index b21a650..5d4ed73 100644 --- a/src/services/notifications/onesignal-client.ts +++ b/src/services/notifications/onesignal-client.ts @@ -10,6 +10,16 @@ interface NotificationClickPayload { version: 1; } +interface NotificationClickEvent { + notification?: { additionalData?: unknown }; +} + +interface NotificationClickRegistration { + active: boolean; + attached: boolean; + listener: (event: NotificationClickEvent) => void; +} + function parseNotificationClickPayload(value: unknown): NotificationClickPayload | null { if (value === null || typeof value !== "object" || Array.isArray(value)) return null; const candidate = value as Record; @@ -19,6 +29,19 @@ function parseNotificationClickPayload(value: unknown): NotificationClickPayload } let initialized = false; +const clickRegistrations = new Set(); + +function attachNotificationClickListener( + registration: NotificationClickRegistration, +): void { + if (!initialized || !registration.active || registration.attached) return; + try { + OneSignal.Notifications.addEventListener("click", registration.listener); + registration.attached = true; + } catch { + // Listener failures never block application startup. + } +} export function initOneSignal(consent: NotificationConsentState): boolean { const appId = process.env.EXPO_PUBLIC_ONESIGNAL_APP_ID; @@ -28,6 +51,9 @@ export function initOneSignal(consent: NotificationConsentState): boolean { OneSignal.setConsentRequired(true); OneSignal.initialize(appId); initialized = true; + for (const registration of clickRegistrations) { + attachNotificationClickListener(registration); + } if (consent === "granted") { OneSignal.setConsentGiven(true); } else { @@ -62,19 +88,32 @@ export function denyOneSignalConsent(): void { } } -export function addNotificationClickListener(onJob: (jobId: string) => void): () => void { - const listener = (event: { notification?: { additionalData?: unknown } }): void => { - const payload = parseNotificationClickPayload(event.notification?.additionalData); - if (payload) onJob(payload.jobId); +export function addNotificationClickListener( + onJob: (jobId: string) => void, +): () => void { + const registration: NotificationClickRegistration = { + active: true, + attached: false, + listener: (event) => { + const payload = parseNotificationClickPayload( + event.notification?.additionalData, + ); + if (payload) onJob(payload.jobId); + }, }; - try { - OneSignal.Notifications.addEventListener("click", listener); - } catch { - return () => {}; - } + clickRegistrations.add(registration); + attachNotificationClickListener(registration); + return () => { + if (!registration.active) return; + registration.active = false; + clickRegistrations.delete(registration); + if (!registration.attached) return; try { - OneSignal.Notifications.removeEventListener("click", listener); + OneSignal.Notifications.removeEventListener( + "click", + registration.listener, + ); } catch { // Listener cleanup failure must not affect app navigation. } @@ -83,4 +122,5 @@ export function addNotificationClickListener(onJob: (jobId: string) => void): () export function resetOneSignalClientForTests(): void { initialized = false; + clickRegistrations.clear(); }