[codex] Structure mobile notification setting failures#3391
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Co-authored-by: codex <codex@users.noreply.github.com>
There was a problem hiding this comment.
Effect service review: one convention finding on the new tagged errors. The errors otherwise follow conventions well — namespace imports (effect/Effect, effect/Schema), Schema.TaggedErrorClass, separate tags for the distinct read vs. request failures, a real cause preserved via Schema.Defect(), and a message derived from a structural attribute rather than from cause.
Posted via Macroscope — Effect Service Conventions
Co-authored-by: codex <codex@users.noreply.github.com>
ApprovabilityVerdict: Approved This PR adds structured error classes for notification permission and live activity preference failures, replacing generic error catches with typed errors. The changes are purely about error typing/structure with no runtime behavior impact beyond improved error messages. You can customize Macroscope's approvability policy. Learn more. |
Summary
Mobile agent-awareness settings exposed raw
unknownfailures from three native or persistence boundaries: reading notification permission, requesting notification permission, and saving the Live Activity updates setting. Downstream settings commands could not reliably identify which operation failed, and the preference write discarded the requested setting value.This change introduces distinct Schema errors for reading and requesting notification permissions. Both record the iOS platform and preserve the exact native cause. Live Activity preference persistence now has its own Schema error carrying the requested
enabledvalue and exact storage cause. Messages are derived from those fields without reason switches, cause-derived text, or constructor wrappers.No behavior tests were added for the pure error-model refactors; the existing Live Activity preference suite still passes.
Validation
pnpm vp test apps/mobile/src/features/agent-awareness/liveActivityPreferences.test.ts(4 tests)pnpm vp check(passes with existing repository warnings)pnpm vp run typecheckpnpm vp run lint:mobileNote
Low Risk
Pure error-model refactor on settings paths; success paths and UI handling via existing failure squashing are unchanged.
Overview
Replaces raw
unknownfailures on mobile agent-awareness settings with Effect Schema tagged errors, so callers can distinguish which boundary failed and inspect structured fields.Notification permissions (
notificationPermissions.ts):requestAgentNotificationPermissionnow fails withNotificationPermissionReadErrororNotificationPermissionRequestErrorinstead ofunknown, each wrapping the native cause viaSchema.Defect()and a fixed iOS-oriented message.Live Activity preference (
liveActivityPreferences.ts):savePreferencesPatchfailures map toLiveActivityPreferenceSaveError, which carries the requestedenabledvalue plus the storage cause.Runtime behavior of permission checks and preference saves is unchanged; only the error channel typing and failure payloads change.
Reviewed by Cursor Bugbot for commit 0f03667. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Structure mobile notification and live activity preference failures as typed errors
LiveActivityPreferenceSaveErrortagged error class (withenabledflag and original cause) in liveActivityPreferences.tsNotificationPermissionReadErrororNotificationPermissionRequestErrortagged error classes in notificationPermissions.tsunknownerrors with structured Effect error types, making failure handling explicit and type-safe.Macroscope summarized 0f03667.