Skip to content

[codex] Structure mobile notification navigation failures#3359

Merged
juliusmarminge merged 2 commits into
mainfrom
codex/mobile-notification-errors
Jun 20, 2026
Merged

[codex] Structure mobile notification navigation failures#3359
juliusmarminge merged 2 commits into
mainfrom
codex/mobile-notification-errors

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • replace the swallowed initial-notification promise chain with a focused consumer that reports read, route, and clear failures
  • attach the notification identifier where available and preserve each exact underlying cause
  • keep notification navigation best-effort and avoid clearing a response that failed to route

Validation

  • vp test apps/mobile/src/features/agent-awareness/notificationNavigation.test.ts (9 tests)
  • vp check (passes with 20 pre-existing warnings)
  • vp run typecheck
  • vp run lint:mobile

Overlap


Note

Low Risk
Localized mobile notification bootstrap refactor with clearer error logging; behavior stays best-effort aside from not clearing after a routing failure.

Overview
Replaces the silent getLastNotificationResponseAsync → route → clearLastNotificationResponseAsync chain in useAgentNotificationNavigation with consumeLastAgentNotificationResponse, which runs read → route → clear and logs structured failures instead of swallowing errors.

Adds NotificationNavigationError (Effect tagged error) with operation (read | route | clear), optional notificationId, and the original cause. Routing errors skip clear so a failed navigation can be retried; read/route/clear failures are console.error’d with that shape.

Tests cover which step failed, route-before-clear on clear failure, and no-clear on route failure.

Reviewed by Cursor Bugbot for commit 66a873e. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add structured error logging for mobile notification navigation failures

  • Introduces NotificationNavigationError in notificationResponseConsumer.ts, a tagged error with an operation field ('read', 'route', or 'clear') and optional notificationId.
  • Adds consumeLastAgentNotificationResponse, which orchestrates reading, routing, and clearing the last notification response, logging a NotificationNavigationError to console.error at each failure point.
  • Route failures skip the clear step; clear failures are logged after routing completes successfully.
  • Refactors useAgentNotificationNavigation to delegate to consumeLastAgentNotificationResponse instead of an inline then/catch chain that swallowed errors.
  • Behavioral Change: notification navigation errors are now logged to console.error with structured context instead of being silently swallowed.

Macroscope summarized 66a873e.

Co-authored-by: codex <codex@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 22e6d83e-55b0-4b70-99fb-c6aae4c9378d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/mobile-notification-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Jun 20, 2026
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This PR extracts notification response handling into a separate module and adds structured error logging. The changes are a straightforward refactor with comprehensive tests - the only behavioral difference is improved error observability instead of silent failure swallowing.

You can customize Macroscope's approvability policy. Learn more.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
@macroscopeapp macroscopeapp Bot dismissed their stale review June 20, 2026 20:13

Dismissing prior approval to re-evaluate 66a873e

@juliusmarminge juliusmarminge merged commit d7718e8 into main Jun 20, 2026
16 checks passed
@juliusmarminge juliusmarminge deleted the codex/mobile-notification-errors branch June 20, 2026 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant