diff --git a/.pi/orksorksorks/alanvardy-var-1165-sentry-error/conventions.md b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/conventions.md new file mode 100644 index 00000000..703593b7 --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/conventions.md @@ -0,0 +1,76 @@ +# Conventions — SingleThread + +Shared factual appendix for later phases. Dense `file:line` references. + +## Canonical commands + +Run from the worktree root. `fish` shell — compound commands go in `/tmp/x.sh` via `bash /tmp/x.sh`. + +| Purpose | Command | Source | +|---|---|---| +| Build (test-ready) | `make build` → `xcodebuild -scheme SingleThread -destination '$(SIM)' -configuration Debug build-for-testing` | `Makefile:35-36` | +| Watch build | `make watch-build` | `Makefile:38-39` | +| Mac build / test | `make mac-build` / `make mac-test` (`-only-testing:SingleThreadTests`) | `Makefile:41-46` | +| Unit tests only | `make test` → `./scripts/test.sh --unit-only` | `Makefile` `test` target | +| UI tests only | `make ui-test` → `./scripts/test.sh --ui-only` | `Makefile` `ui-test` | +| Full CI gate | `make check` → `./scripts/test.sh` (full) | `Makefile` `check`; run via `run-gate` skill only | +| Target one suite/case (bounded) | `scripts/test-one.sh [seconds]` | `scripts/test-one.sh:10-13` | +| Lint / format | `make lint` / `make format` | `Makefile`; gate uses `swiftlint lint --strict`, `swiftformat --lint` (`scripts/test.sh:228-233`) | +| Periphery | `make periphery` → `periphery scan --skip-build --index-store-path DerivedData/Index.noindex/DataStore --strict` | `scripts/test.sh:278-279` | +| App-Group notify probe | `bash scripts/check-appgroup-notify.sh` | `.pi/skills/ai-sorting/SKILL.md` | + +Full gate pipeline order (`scripts/test.sh`): wrapper check `:132`, deployment-target/floor check `:149`, format+lint `:223-233`, warning-check self-test `:236`, build `:240`, watch build `:270`, periphery `:278`, UI tests `:282`, watch UI tests `:290`, watch unit tests `:320`, macOS unit tests `:328`; `--unit-only` runs macOS-native `SingleThreadTests` (`:343-349`), `--ui-only` builds+UI test (`:357-371`). + +## Destination & simulator conventions + +- Precedence: explicit `SIM=` > this worktree's `.simulator_id` > shared default `iPhone 17` (`Makefile:1-4`; `scripts/test.sh:47-72`). `.simulator_id` for this worktree = `5B8A7B5E-A420-4DC0-87EF-AE12FE148B4B`. +- `resolve_sim_udid` matches the device name anchored at line start to avoid the leftover `Gate iPhone 17` sim (`scripts/test.sh:33-41`); name-only destinations are pinned to a UDID and pre-booted (`:28-72`). +- Watch UI tests need a concrete watch UDID: `WATCH_TEST_SIM='platform=watchOS Simulator,id=…'`; default `Apple Watch Series 11 (46mm)` (`Makefile:11-17`). +- `scripts/test-one.sh` exits non-zero on a zero-match `-only-testing:` (xcodebuild otherwise prints TEST SUCCEEDED and exits 0) and bounds the run (`scripts/test-one.sh:5,9`). +- One `xcodebuild` test process at a time; on `Busy`/`RequestDenied` shut down sims and kill orphaned `xcodebuild`/`xctest` (see `simulator-pairing` skill). + +## Test conventions + +- Unit tests: Swift Testing (`import Testing`, `@Test`), no `test`/`testing` name prefix (SwiftFormat strips it). UI tests: XCTest, keep `test…` names, SwiftFormat-excluded (`SingleThreadUITests`). Watch UI tests are XCTest and NOT excluded. +- Swift Testing single-case form must be suite-level or `case()` — a bare `` matches zero. +- App target enables `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor`; `SingleThreadCore` does not, so its types annotate `@MainActor` explicitly (e.g. `AIReminderRanking.swift:63-64`). +- `SWIFT_TREAT_WARNINGS_AS_ERRORS = YES`; gate fails on any source-located compiler warning; exceptions go to `scripts/xcodebuild-warnings.allow`. +- New test suites must be added to `Makefile` `test` and `scripts/test.sh` when they need explicit `-only-testing` filters. +- Bug fixes must add a unit test reproducing the reported symptom; new feature logic needs happy + sad path coverage. + +## Test-suite inventory (relevant subsystems) + +| Suite (path under `SingleThreadTests/`) | Covers | Gating | +|---|---|---| +| `SkippedReminderSyncServiceTests.swift` | activation, push/receive round-trips, malformed payloads, hooks, relaunch persistence (also `SkipCountSyncTests`, `AISortOptionTravelTests`) | iOS/watchOS seam | +| `AppViewModelSyncWiringTests.swift:22-27` | injects `FakeSession` into the view model | iOS | +| `RescheduleSyncTests.swift`, `EntitlementSyncTests.swift`, `AppLanguageSyncTests.swift`, `EnableActionButtonsSyncTests.swift` | sync variants | iOS/watchOS | +| `ReminderStoreTests.swift`, `ReminderStoreGateTests.swift` | persistence, writes, skip path, gate | Swift Testing | +| `EventStoreChangedObserverTests.swift`, `StaleReminderRecheckerTests.swift` | `.EKEventStoreChanged` observer + rechecker | Swift Testing | +| `EventKitStoringTests.swift` | `EventKitStoring` seam | Swift Testing | +| `AISortCoordinatorTests.swift:9,187` | debounce/dedupe/generation/reconcile; fakes `CannedRanker`…`FailingRanker`; `@MainActor struct` | Swift Testing | +| `AISortFailureBannerTests.swift`, `AISortRulesStoreTests.swift`, `AISortRulesViewTests.swift`, `FoundationModelsReminderRankerTests.swift`, `SortOptionTests.swift` | AI sorting | Swift Testing | +| `AppGroupTests.swift:26-71` | suite name, `defaultsIsAStableInstance` (`:36-42`), object-filtered observer (`:44-64`), round-trip | Swift Testing | +| `PreferenceHolderTests.swift:16,26` | `PreferenceHolder` vs real `AppGroup.defaults`, defer-restore | Swift Testing | +| `SentryConfigurationTests.swift`, `SentryScrubberTests.swift`, `CrashReportingPreferenceTests.swift`, `SettingsViewTests.swift:42-50` | Sentry config/scrub/consent | Swift Testing | +| UI: `SingleThreadUITests/` (`testAccessibilityAudit()`), `SingleThreadWatchUITests/` | a11y audit + end-to-end flows | XCTest; `--seed ''` scene for write flows | +| Watch unit: `SingleThreadWatchTests/` | watch logic | Swift Testing, run via `-only-testing:SingleThreadWatchTests` | + +## Test seams / doubles + +- WatchConnectivity: `SkipSyncSession` protocol (`SingleThreadCore/.../SkippedReminderSyncService.swift:18`), `WCSession` conformance (`:36`), `FakeSession` (`SingleThreadTests/TestFixtures.swift:71-104`). Delegate callbacks are invoked directly on the service. +- EventKit: `EventKitStoring` (`EventKitStoring.swift:8`, `EKEventStore` conformance `:43`), `InMemoryEventStore` (`InMemoryEventStore.swift:13`); `ReminderStore.init` injectable with `loadsReminders: false` (`ReminderStore.swift:23`); shared `sharedTestEventStore`/`makeReminder`/`makeCalendar` builders in `TestFixtures.swift`. +- Persistence timing: `ReminderStoreSettle` injectable (`ReminderStore.swift:12,48-50,627`); `StaleReminderRechecker.Sleep` injectable (`StaleReminderRechecker.swift:27,34,46`). +- AI ranker: `AIReminderRanking: Sendable` (`AIReminderRanking.swift:36`); fakes in `AISortCoordinatorTests.swift:9`; tests inject short debounce and `await`. +- Speech: `TestFakeTranscriber: SpeechTranscribing`; background fetch: `FakeBackgroundFetcher`/`GatedBackgroundFetcher`/`SeededFetcher` + `FetchGate` actor rendezvous (`TestFixtures.swift`). +- Deterministic UI write flows: `--seed ''` backed by `InMemoryEventStore`; watch uses `--ui-testing` / `--ui-testing-live-excluded` (`SingleThreadWatch/WatchAppViewModel.swift:312-327`). +- App-Group tests use isolated `UserDefaults.standard` keys with per-test `UUID` suffixes, not the shared suite. + +## Build / verify gotchas + +- One xcodebuild test process at a time; see `simulator-pairing` skill for `Busy`/`RequestDenied`. +- `make periphery` reads a stale build index after branch switches — clean `DerivedData/` and rerun first (`periphery` skill). +- Xcode 27.0 local and CI agree; split `verify_deployment_target()` before any floor change (`AGENTS.md`). +- Known local-only macOS failures: three `EntitlementStoreTests` (`isEntitledSurvivesStoreRecreation`, `initialRefreshSettlesResolvedFlag`, `hostStoreKitIsClean` canary) — pre-existing, not debug targets. +- Full gate runs ONCE after phases commit via the `run-gate` skill; phase subagents use build + targeted `-only-testing:` only. +- Never `git stash` to baseline a failure (stashes span branches) — use `git show origin/main:` or a throwaway worktree. diff --git a/.pi/orksorksorks/alanvardy-var-1165-sentry-error/design.md b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/design.md new file mode 100644 index 00000000..0984be05 --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/design.md @@ -0,0 +1,191 @@ +# Design Discussion + +## Current State + +Sentry issue 7821993169 is reported as `Fatal App Hang Fully Blocked`, culprit +`SingleThreadApp.$main`, on 1.4+21 / iOS 27.2. The raw event (provided during +grilling, Q2) resolves the question the research left open: the blocked main +thread is **entirely inside system code**, and the only app frames are the app +entry points. + +Blocked chain (top of stack is the block itself, read bottom-up): + +``` +SingleThreadApp.$main → App.main → GSEventRunModal → __CFRunLoopRun + → _dispatch_main_queue_drain → __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + → _UIScreenBacklightLevelDidChangeCallback + → +[UIScreen _currentSystemResolvedBrightnessForScreen:] + → BKSDisplayBrightnessGetCurrent → _BKSServerPortHelper + → bootstrap_look_up → _xpc_interface_routine → _xpc_pipe_routine +``` + +UIKit's own backlight-level-change notification handler performs a synchronous +XPC Mach-port lookup to BackBoardServices; that call stalls, the main thread +blocks, and Sentry's hang tracker fires. Breadcrumbs corroborate: `started` +13:29:37.98, `device.orientation` 13:29:38.05, a 5m07s gap, `device.event` +13:34:45.08, hang 13:34:45.45. `SingleThreadApp.$main` is merely the topmost +*app* frame — which is why the report claimed "no app frames on the hung +thread". **No app-code change can prevent this stall.** + +Supporting facts, source-verified: + +- Pinned sentry-cocoa **9.29.2** (`SingleThread.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved:7`). +- Hang events are built in `SentryHangTrackingIntegration.swift:96-115`: + `mechanism.type == "AppHang"` (`:109`), `exceptions[0].stacktrace = + threads[0].stacktrace` (the main thread, `snapshot = true`, `:111-112`), + and `event.threads` populated with every thread (`:115`) with + `threads[0].isMain == true` (`:113`, `SentryDefaultThreadInspector.swift:98`). +- The human-readable type strings (`"Fatal App Hang Fully Blocked"`, …) live in + `SentryAppHangTypeMapper.swift:8-14`, which is `@_spi(Private)` — **not** + importable via plain `import Sentry`. `"AppHang"` is the stable public marker. +- `beforeSend` runs for **both** the in-process hang and the next-launch fatal + flush, and returning `nil` drops the event + (`SentryClient.m:1156-1161`, `:394-402`). +- iOS/tvOS/visionOS in 9.29.2 use `SentryANRTrackerV2` unconditionally and the + integration already gates on `applicationState == .active` + (`SentryHangTrackingIntegration.swift:93-95`) and excludes background time, so + "the app was backgrounded" is **not** an available explanation. + `enableAppHangTrackingV2` does not exist in 9.29.2 (DOC-ONLY: removed in 9.x). +- `appHangTimeoutInterval` defaults to 2.0 s (`Options.swift:646-660`); + `enableReportNonFullyBlockingAppHangs` is **deprecated** (`Options.swift:448-452`). +- Current wiring: `SentryBootstrap.swift:26,38-39` sets + `enableAppHangTracking = true` and `beforeSend = { SentryScrubber.scrub($0) }`. + `SentryScrubber.scrub(_ event: Event) -> Event?` is pure and already returns an + optional, but **never returns `nil` today** (`SentryScrubber.swift:13-25`). +- Latent, unrelated main-thread defects found en route: `BackgroundImageStore` + does synchronous `Data(contentsOf:)`/`FileManager` work on the MainActor + (`BackgroundImageStore.swift:150,152,188,229,251`); `EKEventStore()` is + constructed at launch (`ReminderStore.swift:24`) and + `refreshSourcesIfNecessary()` runs synchronously on the MainActor + (`ReminderStore.swift:525`). +- There are **no** `DispatchSemaphore`/`.sync`/`NSLock`/`Thread.sleep`/`RunLoop`/ + `while true`/`NSFileCoordinator`/`fatalError` in production source, and the + rechecker (`StaleReminderRechecker.swift:102-138`) and AI coordinator + (`AIReminderRanking.swift:108-165`) are cooperative and coalesced. + +## Desired End State + +1. A "Fatal App Hang Fully Blocked" event whose blocked stack is system-only and + matches the BackBoardServices/backlight signature is **dropped in `beforeSend`** + instead of being reported; every hang that has an app frame is still reported + unchanged. +2. The drop rule is a pure, unit-tested predicate whose fixture is the **real + captured stack from this ticket** — the reproducible regression artifact the + repo's bug-fix policy requires. +3. `BackgroundImageStore`'s synchronous disk I/O no longer runs on the MainActor. +4. The rationale for the filter is recorded **in code** (Q6, revised), so a future + reader cannot mistake it for a swallowed error. + +**Verification:** `make format` + `make lint` clean; +`scripts/test-one.sh SingleThreadTests/SentryScrubberTests` and a new +`BackgroundImageStoreTests` case pass; then the full gate via the `run-gate` +skill. Manual confirmation: a unit-test assertion that the captured frame list is +dropped and an app-frame stack is not. + +## Patterns to Follow + +- **Pure static predicate in an `enum` namespace** — mirrors + `SentryScrubber` (`SentryScrubber.swift:11,13,31`): `enum` + `nonisolated static func`, + no state, directly unit-testable. Extend this type rather than adding a new + service object. +- **Test style for Sentry types** — `SingleThreadTests/SentryScrubberTests.swift` + uses Swift Testing, `import Sentry` (`:1-3`), and constructs SDK objects + directly (`Event()` `:9`, `Exception(value:type:)` `:55-57`). Test names do not + start with `test` (SwiftFormat strips the prefix). +- **Prefer a pure layer over SDK-object construction** — the repo's + injectable-seam habit (`ReminderStoreSettle` `ReminderStore.swift:12,48-50`; + `StaleReminderRechecker.Sleep` `StaleReminderRechecker.swift:27,34,46`) applies + here: put the decision logic in a function over plain values, keep the SDK + adapter thin. +- **MainActor isolation defaults** — the app target sets + `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor`; `SingleThreadCore` does not, so + annotate explicitly there (`AGENTS.md` → Concurrency Model). `SentryScrubber` + is already `nonisolated`. +- **Do NOT follow:** `enableReportNonFullyBlockingAppHangs` (deprecated → + warning → gate failure under `SWIFT_TREAT_WARNINGS_AS_ERRORS = YES`), any + `@_spi(Private)` Sentry symbol, or raising `appHangTimeoutInterval` as a blunt + instrument. + +## Design Decisions + +1. **Deliverable is containment, not a hang fix** — the stall is in + `BKSDisplayBrightnessGetCurrent`/`bootstrap_look_up` via UIKit's own observer; + no app frame participates, so app-code hardening cannot fix it. The ticket's + testable artifact is therefore the event filter. +2. **Primary lever = signature-targeted `beforeSend` filter** (Q7=A) — it + removes the false-alarm class without coarsening detection of real hangs. + Options rejected: raising `appHangTimeoutInterval` (coarsens everything and + still fails past the new threshold), disabling `enableAppHangTracking` (loses + all ANR signal). +3. **Drop predicate = three-condition conjunction** (Q8=A), all of which must + hold: (a) `exceptions` contains `mechanism.type == "AppHang"` (the public, + stable marker; the descriptive strings are SPI), (b) the blocked stack has + **zero app frames** (`frame.inApp != true` / `package != "SingleThread"`, + both usable per `SentryFrame.h:41,94`), and (c) the signature contains the + BackBoardServices/backlight denylist (`BKSDisplayBrightnessGetCurrent`, + `bootstrap_look_up`). Narrowest predicate that still kills this class. +4. **Test surface = two layers** (Q9=A) — a pure + `isSystemStall(frames:hasAppFrames:)`-style function over plain signature + strings, plus a thin `Event → signature` adapter. The fixture is the captured + frame list verbatim. This avoids depending on unverified public initializers + for `Frame`/`Thread`/`Stacktrace` in the test target. +5. **Filter placement = extend `SentryScrubber`** — add the predicate and return + `nil` from `scrub(_ event: Event)` when it matches. The signature already + permits `nil` (`SentryScrubber.swift:13`), and `beforeSend` already routes + through it (`SentryBootstrap.swift:38`), so no bootstrap plumbing changes. + The existing PII-scrubbing branch order must be preserved for non-hang events. +6. **Rationale lives in code** (Q6, revised) — a doc comment on the predicate + naming the issue id, the captured stack, and why it is system-only, plus the + PR body. No `docs/CrashReporting.md` section, no Apple Feedback report. +7. **Bundled hardening = `BackgroundImageStore` only** (Q10=A) — move its + synchronous `Data(contentsOf:)`/`FileManager` work off the MainActor. Bounded + and low-risk; EventKit launch work is explicitly out of scope (see below). +8. **`"AppHang"` is matched as a literal, not via SPI** — `event.exceptions` + is inspected directly; if a future SDK renames the mechanism string the filter + stops matching and events are **reported** rather than lost (fail-open). + +## What We're NOT Doing + +- **Not moving EventKit off the main actor.** `EKEventStore()` construction + (`ReminderStore.swift:24`) and `refreshSourcesIfNecessary()` (`:525`) stay as + they are; relocating them requires changing `EventKitStoring`'s `@MainActor` + isolation and the `ResumptionGate` contract (`ResumptionGate.swift:19-51`) — a + separate ticket with its own tests. +- **Not adding Sentry breadcrumbs/instrumentation.** The Q1(A) "diagnose" half is + unnecessary now that the stack is attributed; the cause is proven, not + hypothesised. +- **Not touching `appHangTimeoutInterval`, `enableAppHangTracking`, or + `enableReportNonFullyBlockingAppHangs`** (the last would break the gate). +- **Not using `@_spi(Private)`** (`SentryAppHangTypeMapper`, `SentryANRType`). +- **Not changing PII-scrubbing behaviour** for any event that isn't a matched + system-stall hang. +- **Not touching the watch target** — the filter is iOS Sentry bootstrap only. +- **Not adding a UI test** — the change is a pure predicate; a UI test would be + slow, flaky, and unable to trigger a BackBoardServices stall. +- **Not adding a docs page or an Apple Feedback report** (Q6, revised). + +## Open Risks + +- **Public initializers unverified.** The report confirmed `Frame.inApp`/ + `.package` exist but not public `Frame()`/`Thread()`/`Stacktrace(frames:)` + initializers. Mitigated by Decision 4's pure-layer split; if the adapter cannot + be unit-tested, the plan must say so and rely on the build for the adapter. +- **Fatal-flush retention inferred.** `beforeSend` provably runs on the fatal + path (`SentryClient.m:394-402`), but that the *persisted* event still carries + `event.threads`/`exceptions[0].stacktrace` at flush time was inferred, not + proven. Verify during planning (plan-phase fact check), not at implementation. +- **Masking risk.** A genuine app hang whose stack happens to have zero app frames + and passes through a backlight frame would be dropped. Bounded by the + three-condition conjunction (Decision 3) and accepted. +- **Denylist coverage.** Only this one system signature is filtered; other + system-stall signatures (CloudKit, StoreKit, cfprefsd) still report as noise. + Adding signatures is a deliberate, reviewed act. +- **`BackgroundImageStore` timing change.** Moving its I/O off the MainActor may + alter when the image is available and interacts with tests that inject + `FakeBackgroundFetcher`/`SeededFetcher`; it needs its own test. +- **Audit-trail tension.** With Q6=C there is no external record; the only + durable rationale is the code comment and PR body. A future maintainer may + still delete the filter as "swallowed errors" — partially mitigated by + Decision 6 and Decision 8's fail-open property. +- **SDK churn.** A sentry-cocoa bump may rename the `"AppHang"` mechanism; the + failure mode is fail-open (noisy), never silent loss. diff --git a/.pi/orksorksorks/alanvardy-var-1165-sentry-error/done.md b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/done.md new file mode 100644 index 00000000..9ebe39eb --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/done.md @@ -0,0 +1,14 @@ +# Done + +- **Branch / head SHA**: `alanvardy-var-1165-sentry-error` @ `65a65dbf` (review fix on top of gated `9cba83c5`); pushed to `origin` with `--force-with-lease`. +- **Mechanical checks**: + - `make format` clean; `make lint --strict` → 0 violations, 233 files. + - Full CI gate (`./scripts/test.sh`, pinned to the worktree iPhone UDID + watchOS 26.5 UDID) at `9cba83c5`: PASSED deployment-target check, xcodebuild-wrapper check, format, SwiftFormat check, SwiftLint, warning-check self-test, iOS build, and watch build (no un-allowlisted warnings). + - Gate **FAILED stage-fatal at Periphery** (`SingleThread/ReminderCardView.swift:74:24: Unused property 'swipeOffset'`); the test/UI stages never ran at that commit. + - Targeted re-verification at `65a65dbf`: `SingleThreadTests/SentryScrubberTests` **15/15**, `SingleThreadTests/BackgroundImageStoreTests` **21/21**, both exit 0 with no zero-match. +- **Review outcome**: one bounded reviewer + parent cross-check — **no blockers**. Applied the two fixes worth doing now: `SentryScrubber.swift` doc now says "the stack contains every image" (code searches the whole frame list, not just the blocked region) and `BackgroundImageStoreTests` renamed `storedImageStillLoadsAndBlocksStaleRefetch` → `storedImageStillLoadsAndBlocksRefetchWhenFresh`. Optional nits **declined**: `appHangStack` multi-exception belt-and-braces (theoretical; events are single-exception) and hoisting the `!isFetching` guard above `loadStoredImage()` (no correctness impact). Accepted residual risk: the coarser BackBoardServices + libxpc image signature can drop a non-backlight system stall traversing both images, bounded by the positional app-frame check (documented in `plan.md`). +- **Periphery failure is main-origin, not this ticket**: `ReminderCardView.swift` is byte-identical to `origin/main` (`ab254d45`), no branch commit touches it, and main's own CI run `39142756669` failed the `lint` job at the same Periphery finding. This is the documented Xcode-27 `$`-projection-only `@State` blind spot. Left unmodified here; it needs its own fix on main. +- **Remaining manual items**: + - The `plan.md` manual confirmations (captured backlight stack is dropped, app-frame stacks are kept, off-main executor runs off the MainActor, no public surface change) are all covered by the green targeted suites; the operator may spot-check assertions directly. + - Post-release: confirm Sentry issue 7821993169 stops reporting once a build containing this filter ships. + - Out of scope: the main-origin `swipeOffset` Periphery red (above), and the plan's deferred EventKit main-thread work. diff --git a/.pi/orksorksorks/alanvardy-var-1165-sentry-error/implement.md b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/implement.md new file mode 100644 index 00000000..911a8ea8 --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/implement.md @@ -0,0 +1,47 @@ +# Implementation Summary + +## Commits + +| Phase | Commit | Description | +|-------|--------|-------------| +| 1 | `5d99584e` | drop captured BackBoardServices stall in beforeSend | +| 2 | `739d7eea` | fail-open guarantee for AppHang signature | +| 3 | `1c5f6bc2` | move BackgroundImageStore disk I/O off the MainActor | + +All three commits pushed to `origin/alanvardy-var-1165-sentry-error`. + +## Automated Checks + +- [x] Phase 1 — `make build` succeeded (`** TEST BUILD SUCCEEDED **`). +- [x] Phase 1 — `scripts/test-one.sh SingleThreadTests/SentryScrubberTests`: `ok: 9 case(s) ran`, exit 0, no zero-match. +- [x] Phase 2 — `make build` succeeded. +- [x] Phase 2 — `scripts/test-one.sh SingleThreadTests/SentryScrubberTests`: `ok: 15 case(s) ran`, exit 0, no zero-match. +- [x] Phase 2 — `swiftformat --lint` and `swiftlint lint --strict` clean on the touched files. +- [x] Phase 3 — `make build` succeeded. +- [x] Phase 3 — `scripts/test-one.sh SingleThreadTests/BackgroundImageStoreTests`: `ok: 21 case(s) ran` (19 pre-existing + 2 new), exit 0, no zero-match. +- [x] Phase 3 — `swiftformat --lint` and `swiftlint lint --strict` clean on the touched files. + +## Manual Verification Items (from the plan) + +Phase 1: +- [ ] Confirm `dropsCapturedBacklightSystemStall` succeeds, i.e. `scrub` returns `nil` for the real captured client-side frame list. +- [ ] Confirm `keepsHangWithAppFrameInBlockedChain` and `keepsNonHangBacklightEvent` both return non-`nil`. + +Phase 2: +- [ ] Confirm every new assertion is `scrub(event) != nil` (the event survives). +- [ ] Confirm no force-unwrap was introduced (SwiftLint bans it outside tests). + +Phase 3: +- [ ] `diskWorkRunsOffMainActor` asserts `false` (the work runs off the MainActor). +- [ ] No public surface changed: grep for `refreshIfNeeded(`/`forceRefresh(` call sites (`ContentViewModel:156`, `BackgroundSettingsView:57`) and confirm they still compile unchanged. + +## Deviations from plan snippets (intent preserved) + +- **Phase 1**: `Stacktrace(frames:)` from the plan is not the sentry-cocoa 9.29.2 Swift name — used `SentryStacktrace(frames:registers:)`. Added a `legacy_objc_type` SwiftLint disable for the SDK-mandated `NSNumber` `Frame.inApp`, matching the existing `SentryBootstrap.swift` pattern. SwiftFormat removed trailing commas the plan's snippets contained. +- **Phase 3**: `BackgroundMetadata` and `LoadedImage` are declared `nonisolated` structs so their `Codable`/`Sendable` conformances work across the `@concurrent` executor under the default MainActor isolation. `LoadedImage.photographerURL` is `String?` (the plan's snippet declared `URL?` but passed a `String?`), converted on assignment via `.flatMap(URL.init)`. `fetchWallpaper` now calls `Self.isDecodableImage(data)`. + +## Notes + +- No new files, no pbxproj change (synchronized file groups), no schema migration, no new test target, no `Makefile`/`scripts/test.sh` filter entries. +- The full CI-identical gate (`./scripts/test.sh`) was intentionally not run by phase workers; it belongs to the review step. +- `DELETEME` remains an unstaged deletion in the worktree (pre-existing, must be removed before merge). Plan artifacts under `.pi/orksorksorks/` remain untracked. diff --git a/.pi/orksorksorks/alanvardy-var-1165-sentry-error/large.md b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/large.md new file mode 100644 index 00000000..e273340e --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/large.md @@ -0,0 +1,20 @@ +# Task + +Investigate and fix a production Sentry "Fatal App Hang Fully Blocked" (issue +7821993169, culprit `SingleThreadApp.$main`) on SingleThread 1.4+21 / iOS 27.2. +The captured stacks contain **no SingleThread app frames on the hung main +thread** — only the Sentry ANR-detection thread (`monitorThreadCache` sleeping) +plus system and WatchConnectivity background threads — so the hang's root cause +is unidentified and must be diagnosed before a fix can be designed. The fix must +be backed by a unit test reproducing the hang condition (per repo policy for bug +fixes). + +## Why LARGE + +Matched triggers: **UNKNOWNS** (the hang's cause is unidentified; the crash +report has no app-code culprit frame, so a spike/repro investigation is needed +before any approach exists) and **CONVENTION_RISK** (the only in-app-adjacent +threads point at WatchConnectivity `WCContentIndex index` and file coordination +on the shared EventKit/App-Group persistence path). The decisive signal is the +missing root cause: with no known culprit and no dictated approach, neither +MEDIUM's M1 ("approach known, 0–2 unknowns") nor SMALL's A–B can hold. diff --git a/.pi/orksorksorks/alanvardy-var-1165-sentry-error/plan.md b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/plan.md new file mode 100644 index 00000000..a13e58fa --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/plan.md @@ -0,0 +1,570 @@ +# Implementation Plan + +## Overview + +Contain Sentry issue 7821993169 (`Fatal App Hang Fully Blocked`, culprit +`SingleThreadApp.$main`) by dropping, in `beforeSend`, AppHang events whose +blocked chain is the system-only BackBoardServices/backlight XPC stall, while +still reporting every hang whose blockage includes app code; and move +`BackgroundImageStore`'s synchronous disk I/O off the MainActor. + +--- + +## Pre-phase validation (plan step) — RESOLVED + +Two facts the design left inferred are now verified against sentry-cocoa 9.29.2 +and the raw event JSON. **Both change the predicate**, so they are recorded here +rather than left as implementation-time surprises. + +1. **`beforeSend` runs on the fatal-flush path** (design open risk, resolved). + `SentryHangTrackingIntegration.captureStoredAppHangEvent` → + `SentrySDKInternal.captureFatalAppHang` → `SentryHub.captureFatalAppHangEvent` + → `captureFatalEvent` → `sendEvent(... isFatalEvent:YES)` → `prepareEvent` + → `beforeSend`. `SentryEvent.serialize` / `SentryFrameCodable` preserve + `exceptions`, `stacktrace`, and `in_app` across the store/read cycle. ✅ +2. **Function names are NOT available at `beforeSend`** (design assumption, + false). App-hang stacks are built by + `SentryDefaultThreadInspector.getCurrentThreadsWithStackTrace()` → + `SentryStacktraceBuilder` → `SentryCrashStackEntryMapper.mapAddress:`, which + sets only `instructionAddress`, `imageAddress`, `package` (= the loaded + binary path) and `inApp`. The only `Frame.function` writer in the SDK is + `SentryFrameCodable` (JSON decode of an already-symbolicated payload); there + is no local symbolication. The design's denylist + (`BKSDisplayBrightnessGetCurrent`, `bootstrap_look_up`) is therefore + **unmatchable client-side**. The signature must be matched on image paths. +3. **The exception stack is not app-frame-free** (design assumption, false). + The raw event's client-side exception stack contains exactly one app frame + (`…/SingleThread.app/SingleThread`, `in_app: true`); server symbolication + later splits it into `main` + `SingleThreadApp.$main`. A "zero app frames" + condition never fires. The correct condition is "no app frame in the + *blocked* (deepest) region". + +**Deviations from `structure.md` (forced by the above), all confined to Phase 1–2:** + +- Denylist is image paths, not function names: + `systemStallSignatureImages = ["BackBoardServices", "libxpc.dylib"]`. +- `isSystemStall` takes the ordered `[StackFrameSignature]` (image + `isApp`) + instead of `(frameSignatures, hasAppFrames)`; the app-frame condition becomes + positional ("no in-app frame at or deeper than the first stall frame"). +- The Phase 2 case `keepsEventWhenAnyFrameIsInApp` is implemented as + `keepsEventWhenAppFrameIsInBlockedChain` (an app frame shallower than the + stall — the real captured entry frame — must NOT prevent the drop). + +The fixture is the raw event's **client-side** frame list (`package` + `in_app`) +verbatim — sentry-cocoa's equivalent of `raw_stacktrace.frames` in the JSON. + +--- + +## Phase 1: Walking skeleton — drop the captured BackBoardServices stall in `beforeSend` + +**Outcome**: the captured event (system-only blocked chain, app frame only at +the bottom) is dropped end to end through the real captured signature → pure +predicate → `SentryScrubber.scrub(_:) == nil`; the same hang with an app frame in +the blocked chain is reported unchanged. + +### Changes + +#### 1. `SingleThread/SentryScrubber.swift` +**Action**: modify + +Add the signature constants, the reduced frame value, the pure predicate, and +the thin `Event` adapter, then short-circuit `scrub(_:)`. + +```swift +enum SentryScrubber { + /// Tag keys that carry app/device metadata only — never reminder content. + nonisolated static let allowedTags: Set = ["environment", "release", "level"] + + /// Binary-image substrings of the synchronous BackBoardServices/backlight XPC + /// stall UIKit performs on the main thread. Sentry issue 7821993169: + /// `BKSDisplayBrightnessGetCurrent` (BackBoardServices) → + /// `bootstrap_look_up` (libxpc). + /// + /// Function names are NOT available in `beforeSend` — sentry-cocoa sets only + /// `package`/`inApp` at capture and symbolicates `function` server-side — so + /// the signature is matched on image paths. + nonisolated static let systemStallSignatureImages: [String] = [ + "BackBoardServices", + "libxpc.dylib", + ] + + /// One frame reduced to the fields `beforeSend` actually receives. + struct StackFrameSignature: Equatable { + /// `Frame.package` — the loaded binary's path. + let image: String + /// `Frame.inApp`, with the app-bundle path as a belt-and-braces fallback. + let isApp: Bool + } + + /// Pure drop decision. Fail-open: any under-specified input returns `false`, + /// so the event is reported rather than silently lost. + /// + /// All three conditions must hold: + /// 1. the mechanism is the public, stable `"AppHang"` marker, + /// 2. the blocked chain contains every image in `systemStallSignatureImages`, + /// 3. no in-app frame sits at or deeper than the first stall frame. + nonisolated static func isSystemStall( + mechanismType: String?, + frames: [StackFrameSignature] + ) -> Bool { + guard mechanismType == "AppHang" else { return false } + guard !frames.isEmpty else { return false } + guard systemStallSignatureImages.allSatisfy({ signature in + frames.contains { $0.image.contains(signature) } + }) else { return false } + guard let firstStall = frames.firstIndex(where: { frame in + systemStallSignatureImages.contains { frame.image.contains($0) } + }) else { return false } + return !frames[firstStall...].contains(where: \.isApp) + } + + /// Reduces a Sentry event to an app-hang signature, or `nil` for any shape it + /// does not recognise (fail-open). + nonisolated static func appHangStack( + in event: Event + ) -> (mechanismType: String, frames: [StackFrameSignature])? { + guard + let exception = event.exceptions?.first(where: { + $0.mechanism?.type == "AppHang" + }), + let mechanismType = exception.mechanism?.type, + let stacktrace = exception.stacktrace, + !stacktrace.frames.isEmpty + else { return nil } + return (mechanismType, stacktrace.frames.map(StackFrameSignature.init)) + } + + /// Allow-list scrubber… When the event is the captured system-stall hang the + /// whole event is dropped; every other event is PII-scrubbed as before. + nonisolated static func scrub(_ event: Event) -> Event? { + if let stack = appHangStack(in: event), + isSystemStall(mechanismType: stack.mechanismType, frames: stack.frames) { + return nil + } + event.user = nil + event.extra = nil + // …existing body unchanged… + return event + } + // …existing breadcrumb scrubber and allowedTags… +} + +extension SentryScrubber.StackFrameSignature { + init(_ frame: Frame) { + let image = frame.package ?? "" + self.image = image + self.isApp = (frame.inApp?.boolValue == true) || image.contains(".app/") + } +} +``` + +No `SentryBootstrap.swift` change: `beforeSend` already routes through +`scrub` (`SentryBootstrap.swift:38`). + +#### 2. `SingleThreadTests/SentryScrubberTests.swift` +**Action**: modify + +Add the captured-fixture helper and the Phase 1 cases. + +```swift +private static let appImage = + "/private/var/containers/Bundle/Application/277152F6-B99C-4FA0-BADB-098D8D06EC13/SingleThread.app/SingleThread" +private static let swiftUI = "/System/Library/Frameworks/SwiftUI.framework/SwiftUI" +private static let uiKit = "/System/Library/PrivateFrameworks/UIKitCore.framework/UIKitCore" +private static let graphicsServices = + "/System/Library/PrivateFrameworks/GraphicsServices.framework/GraphicsServices" +private static let coreFoundation = + "/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation" +private static let libDispatch = "/usr/lib/system/libdispatch.dylib" +private static let backBoard = + "/System/Library/PrivateFrameworks/BackBoardServices.framework/BackBoardServices" +private static let libXPC = "/usr/lib/system/libxpc.dylib" +private static let libObjCMsgSend = "/usr/lib/objc/libobjcMsgSend.dylib" + +/// The raw event's client-side exception stack (`package` + `in_app`), verbatim. +private static let capturedFrames: [(image: String, inApp: Bool)] = [ + ("dyld", false), + (appImage, true), + (swiftUI, false), (swiftUI, false), (swiftUI, false), + (uiKit, false), (graphicsServices, false), + (coreFoundation, false), (coreFoundation, false), (coreFoundation, false), + (libDispatch, false), (libDispatch, false), (libDispatch, false), (libDispatch, false), + (coreFoundation, false), (coreFoundation, false), + (uiKit, false), (uiKit, false), (uiKit, false), + (backBoard, false), (backBoard, false), + (libXPC, false), (libXPC, false), (libXPC, false), (libXPC, false), (libXPC, false), + (libObjCMsgSend, false), +] + +private func hangEvent( + frames: [(image: String, inApp: Bool)], + mechanismType: String = "AppHang" +) -> Event { + let event = Event() + let exception = Exception(value: "App hanging.", type: "App Hang Fully Blocked") + exception.mechanism = Mechanism(type: mechanismType) + exception.stacktrace = Stacktrace(frames: frames.map { frame in + let sentryFrame = Frame() + sentryFrame.package = frame.image + sentryFrame.inApp = NSNumber(value: frame.inApp) + return sentryFrame + }) + event.exceptions = [exception] + return event +} + +@Test +func dropsCapturedBacklightSystemStall() { + #expect(SentryScrubber.scrub(hangEvent(frames: Self.capturedFrames)) == nil) +} + +@Test +func keepsHangWithAppFrameInBlockedChain() { + var frames = Self.capturedFrames + let leaf = frames.count - 1 + frames[leaf] = (Self.appImage, true) // app is the blocked (deepest) frame + #expect(SentryScrubber.scrub(hangEvent(frames: frames)) != nil) +} + +@Test +func keepsNonHangBacklightEvent() { + let event = hangEvent(frames: Self.capturedFrames, mechanismType: "Crash") + event.extra = ["reminderTitle": "Buy milk"] + let scrubbed = SentryScrubber.scrub(event) + #expect(scrubbed != nil) // not dropped + #expect(scrubbed?.extra == nil) // PII scrubbing still runs +} +``` + +### Verification + +#### Automated +- [x] `make build` succeeds. +- [x] `scripts/test-one.sh SingleThreadTests/SentryScrubberTests` passes (suite-level form; a bare case name matches zero under Swift Testing). +- [x] `scripts/test-one.sh SingleThreadTests/SentryScrubberTests` reports no zero-match (the script exits non-zero on a zero-match). + +#### Manual +- [ ] Confirm `dropsCapturedBacklightSystemStall` succeeds, i.e. `scrub` returns `nil` for the real captured client-side frame list. +- [ ] Confirm `keepsHangWithAppFrameInBlockedChain` and `keepsNonHangBacklightEvent` both return non-`nil`. + +--- + +## Phase 2: Fail-open guarantee — SDK churn or odd shapes never lose signal + +**Outcome**: every deviation from the exact filtered shape still reports the +event, so a sentry-cocoa change or a missing/empty stack surfaces as noise, never +silent loss. + +### Changes + +#### 1. `SingleThread/SentryScrubber.swift` +**Action**: review only (the guards written in Phase 1 already implement this) + +Confirm the explicit fail-open returns: +- `appHangStack` returns `nil` when the mechanism is absent/renamed, the + stacktrace is missing, or the frame list is empty. +- `isSystemStall` returns `false` when the mechanism is not `"AppHang"`, the + frame list is empty, or either signature image is absent. +- The positional app check does not misfire when `frame.package` is `nil` + (`image == ""`, `isApp` falls back to `inApp`). + +#### 2. `SingleThreadTests/SentryScrubberTests.swift` +**Action**: modify + +```swift +@Test +func keepsEventWhenMechanismRenamed() { + #expect(SentryScrubber.scrub(hangEvent( + frames: Self.capturedFrames, mechanismType: "AppHangRenamed")) != nil) +} + +@Test +func keepsEventWhenStacktraceMissing() { + let event = Event() + let exception = Exception(value: "App hanging.", type: "App Hang Fully Blocked") + exception.mechanism = Mechanism(type: "AppHang") + event.exceptions = [exception] + #expect(SentryScrubber.scrub(event) != nil) +} + +@Test +func keepsEventWhenFrameListEmpty() { + #expect(SentryScrubber.scrub(hangEvent(frames: [])) != nil) +} + +@Test +func keepsEventWhenDenylistPartiallyMatches() { + // BackBoardServices present, libxpc absent ⇒ not the backlight XPC stall. + let frames = Self.capturedFrames.filter { $0.image != Self.libXPC } + #expect(SentryScrubber.scrub(hangEvent(frames: frames)) != nil) +} + +@Test +func keepsEventWhenAppFrameIsInBlockedChain() { + var frames = Self.capturedFrames + frames.insert((Self.appImage, true), at: frames.count) // app frame at the leaf + #expect(SentryScrubber.scrub(hangEvent(frames: frames)) != nil) +} + +@Test +func keepsEventWhenFramePackageIsMissing() { + let frames = Self.capturedFrames.map { (image: "", inApp: $0.inApp) } + #expect(SentryScrubber.scrub(hangEvent(frames: frames)) != nil) +} +``` + +### Verification + +#### Automated +- [x] `scripts/test-one.sh SingleThreadTests/SentryScrubberTests` passes with the five fail-open negatives added. + +#### Manual +- [ ] Confirm every new assertion is `scrub(event) != nil` (the event survives). +- [ ] Confirm no force-unwrap was introduced (SwiftLint bans it outside tests). + +--- + +## Phase 3: `BackgroundImageStore` disk I/O off the MainActor + +**Outcome**: the wallpaper store's synchronous `Data(contentsOf:)` / +`FileManager` reads and writes no longer execute on the MainActor. + +**Deviation from `structure.md`**: the disk helpers are `@concurrent`, not plain +`nonisolated`. `SWIFT_APPROACHABLE_CONCURRENCY = YES` (set project-wide) makes a +plain `nonisolated async` function run on the **caller's** actor, so +`nonisolated` alone would keep the I/O on the MainActor. `@concurrent` (Swift +6.2+, available in the toolchain here — verified compiling and executing off the +MainActor) is required. + +### Changes + +#### 1. `SingleThread/BackgroundImageStore.swift` +**Action**: modify + +Add the off-main executor and move disk work into it; make +`loadStoredImage()`/`isFresh(maxAge:)`/`persist(imageData:metadata:)` async; keep +`refreshIfNeeded`/`forceRefresh` signatures unchanged. + +```swift + /// Runs synchronous disk work on the concurrent executor, off the MainActor. + /// `@concurrent` is required because `SWIFT_APPROACHABLE_CONCURRENCY` leaves a + /// plain `nonisolated async` function on the caller's actor (here MainActor). + /// Internal so tests can prove the executor leaves the MainActor. + @concurrent + nonisolated static func offMain( + _ work: @Sendable () throws -> T + ) async rethrows -> T { + try work() + } + + /// A decoded, validated wallpaper (plain `Sendable` value, no observable state). + private struct LoadedImage: Sendable { + let data: Data + let photographer: String + let photographerURL: URL? + } + + func loadStoredImage() async { + let metadataURL = self.metadataURL + let imageURL = self.imageURL + let loaded = await Self.offMain { () -> LoadedImage? in + guard + let metadataData = try? Data(contentsOf: metadataURL), + let metadata = try? Self.decodeMetadata(from: metadataData), + let data = try? Data(contentsOf: imageURL), + Self.isDecodableImage(data) + else { return nil } + return LoadedImage( + data: data, + photographer: metadata.photographer, + photographerURL: metadata.photographerURL) + } + guard let loaded else { + imageData = nil + photographer = nil + photographerURL = nil + return + } + imageData = loaded.data + photographer = loaded.photographer + photographerURL = loaded.photographerURL.flatMap(URL.init) + } + + private func isFresh(maxAge: TimeInterval) async -> Bool { + let metadataURL = self.metadataURL + let fetchedAt = await Self.offMain { () -> Date? in + guard + let data = try? Data(contentsOf: metadataURL), + let metadata = try? Self.decodeMetadata(from: data) + else { return nil } + return metadata.fetchedAt + } + guard let fetchedAt else { return false } + return Date().timeIntervalSince(fetchedAt) < maxAge + } + + private func persist(imageData: Data, metadata: BackgroundMetadata) async throws { + let directory = self.directory + let imageURL = self.imageURL + let metadataURL = self.metadataURL + try await Self.offMain { + let encoder = JSONEncoder() + encoder.dateEncodingStrategy = .iso8601 + let metadataData = try encoder.encode(metadata) + try FileManager.default.createDirectory( + at: directory, withIntermediateDirectories: true) + try imageData.write(to: imageURL, options: .atomic) + try metadataData.write(to: metadataURL, options: .atomic) + } + } + + private func decodeMetadata(from data: Data) throws -> BackgroundMetadata { + try Self.decodeMetadata(from: data) // becomes: nonisolated static + } + + /// Now `nonisolated static` so it can run inside `offMain`. + private nonisolated static func decodeMetadata(from data: Data) throws -> BackgroundMetadata { + let decoder = JSONDecoder() + decoder.dateDecodingStrategy = .iso8601 + return try decoder.decode(BackgroundMetadata.self, from: data) + } + + /// Now `nonisolated static` so it can run inside `offMain`. + private nonisolated static func isDecodableImage(_ data: Data) -> Bool { + #if os(iOS) + UIImage(data: data) != nil + #elseif os(macOS) + NSImage(data: data) != nil + #endif + } +``` + +Update the call sites inside the store: + +```swift + func refreshIfNeeded(maxAge: TimeInterval = BackgroundImageStore.defaultMaxAge) async { + await loadStoredImage() + guard !isPinned || imageData == nil else { return } + guard !(await isFresh(maxAge: maxAge)) else { return } + guard !isFetching else { return } + isFetching = true + defer { isFetching = false } + do { + let wallpaper = try await fetchWallpaper(from: Self.endpoint) + guard !isPinned || imageData == nil else { return } + try await commit(wallpaper) + } catch { + Self.logger.error("Background refresh failed: \(error.localizedDescription)") + } + } + + func forceRefresh() async { + // …unchanged guards… + do { + let wallpaper = try await fetchWallpaper(from: Self.randomEndpoint) + try await commit(wallpaper) // was: try commit(wallpaper) + } catch { … } + } + + private func commit(_ wallpaper: FetchedWallpaper) async throws { + let metadata = BackgroundMetadata( + photographer: wallpaper.photographer, + photographerURL: wallpaper.photographerURL?.absoluteString, + fetchedAt: Date()) + try await persist(imageData: wallpaper.data, metadata: metadata) // disk before state + imageData = wallpaper.data + photographer = wallpaper.photographer + photographerURL = wallpaper.photographerURL + } +``` + +Delete the two replaced private declarations (the old synchronous `decodeMetadata` +and `isDecodableImage`) so there is exactly one implementation each. Remove the +now-unused instance `decodeMetadata` wrapper if SwiftFormat/Periphery flags it. +All disk-before-state ordering is preserved. + +#### 2. `SingleThreadTests/BackgroundImageStoreTests.swift` +**Action**: modify + +Await the now-async `loadStoredImage` at the five existing call sites (lines +267, 318, 367, 396, 423): + +```swift + await store.loadStoredImage() +``` + +Add the off-main regression guard and a stored-image parity case: + +```swift + /// Regression guard for the removed synchronous `Data(contentsOf:)` on the + /// MainActor: the executor used for all disk work must leave the MainActor. + @Test + func diskWorkRunsOffMainActor() async { + let ranOnMain = await BackgroundImageStore.offMain { pthread_main_np() != 0 } + #expect(ranOnMain == false) + } + + /// A stored image still loads and a fresh sidecar still short-circuits the + /// network after the disk work moved off-main. + @Test + func storedImageStillLoadsAndBlocksStaleRefetch() async throws { + let fake = FakeBackgroundFetcher() + fake.stubbedData[Self.endpoint] = .success(payloadJSON()) + fake.stubbedData[Self.imageURL] = .success(BackgroundTestFixtures.jpegData) + let (store, _) = makeStore(client: fake) + await store.refreshIfNeeded(maxAge: 3600) + #expect(store.imageData == BackgroundTestFixtures.jpegData) + + await store.loadStoredImage() + #expect(store.photographer == "NEOM") + + let countAfterLoad = fake.requestedURLs.count + await store.refreshIfNeeded() + #expect(fake.requestedURLs.count == countAfterLoad, + "fresh sidecar must skip the network after the off-main move") + } +``` + +### Verification + +#### Automated +- [x] `make build` succeeds. +- [x] `scripts/test-one.sh SingleThreadTests/BackgroundImageStoreTests` passes (all pre-existing cases plus the two new ones). +- [x] `scripts/test-one.sh SingleThreadTests/BackgroundImageStoreTests` exits 0 (no zero-match). + +#### Manual +- [ ] `diskWorkRunsOffMainActor` asserts `false` (the work runs off the MainActor). +- [ ] No public surface changed: grep for `refreshIfNeeded(`/`forceRefresh(` call sites (`ContentViewModel:156`, `BackgroundSettingsView:57`) and confirm they still compile unchanged. + +--- + +## Testing checkpoints + +- After Phase 1: `SingleThreadTests/SentryScrubberTests` green — the captured client-side stack is dropped; a blocked-chain app frame is kept. +- After Phase 2: same suite green with the six fail-open negatives added. +- After Phase 3: `SingleThreadTests/BackgroundImageStoreTests` green (off-main guard + parity) and the pre-existing cases still green. +- Once all three phases commit: run `make format` then `make lint` inline, then the full CI gate **once** via the `run-gate` skill (never `nohup`). No UI test is added — a UI test cannot trigger a BackBoardServices stall, and the change is a pure predicate (design decision). + +## Files touched + +| File | Phase | Action | +|---|---|---| +| `SingleThread/SentryScrubber.swift` | 1, 2 | modify | +| `SingleThreadTests/SentryScrubberTests.swift` | 1, 2 | modify | +| `SingleThread/BackgroundImageStore.swift` | 3 | modify | +| `SingleThreadTests/BackgroundImageStoreTests.swift` | 3 | modify | + +No new files. No pbxproj change (synchronized file groups). No schema migration. +No new test target, so no `Makefile`/`scripts/test.sh` filter entries are needed. + +## Residual risks (accepted) + +- **Image-based signature is coarser than function names.** `BackBoardServices` + + `libxpc.dylib` on an AppHang whose blocked region has no app frame is the + filtered class; a genuine app hang traversing those images *deeper than the + app frame* would be dropped. Bounded by the positional app-frame condition. +- **SDK churn.** If sentry-cocoa stops setting `inApp`/`package` or renames the + `"AppHang"` mechanism, the predicate returns `false` and events are reported + (fail-open noise), never silently lost — the Phase 2 negatives pin this. +- **Other system stalls** (CloudKit, StoreKit, cfprefsd) are not filtered; + adding signatures remains a deliberate, reviewed act. diff --git a/.pi/orksorksorks/alanvardy-var-1165-sentry-error/questions.md b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/questions.md new file mode 100644 index 00000000..403b21a1 --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/questions.md @@ -0,0 +1,32 @@ +# Research Questions + +## Context +Focus on the SingleThread iOS app's main-thread lifecycle: app launch and +`AppViewModel` construction, the WatchConnectivity sync service, the +EventKit/ReminderStore persistence layer and App Group `UserDefaults`, the AI +sorting coordinator, and the app's Sentry crash/hang-reporting setup. Also map +existing unit-test seams and conventions used to exercise these subsystems +without real system services. + +## Questions +1. What synchronous work runs on the main thread during app launch and + `AppViewModel` initialization, and what dependencies does that construction + chain touch (stores, coordinators, sync services, observers)? +2. How does the WatchConnectivity sync service (`SkippedReminderSyncService`) + operate: on which queues/actors its delegate callbacks run, how activation + and payload handling unfold, and how it intersects the main actor? +3. How does the EventKit `ReminderStore` persistence path execute reads, + writes, settle delays, and change observers, and what isolation or + synchronization primitives guard them? +4. How is the AI sorting coordinator triggered and executed — debounce, + observation wiring, ranker invocation, and the actor/queue on which + ranking work runs? +5. How are App Group `UserDefaults` values and their change notifications read + and observed across the app and watch, and what single-instance or + notification-filtering constraints exist? +6. What is the current Sentry integration (bootstrap options, hang/watchdog + tracking, scrubbing) and what in-app diagnostics or breadcrumbs exist around + hangs? +7. What test seams and conventions exist for exercising WatchConnectivity, + EventKit, App Group persistence, and main-actor coordinator behavior in + unit tests, and how are targeted tests run? diff --git a/.pi/orksorksorks/alanvardy-var-1165-sentry-error/research.md b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/research.md new file mode 100644 index 00000000..fa5c6fb1 --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/research.md @@ -0,0 +1,89 @@ +# Research Findings + +## Q1: What synchronous work runs on the main thread during app launch and `AppViewModel` initialization, and what dependencies does that construction chain touch? + +### Findings +- App target sets `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor`; `AppViewModel` is `@MainActor @Observable` (`SingleThread/AppViewModel.swift:14-17`). +- `SingleThreadApp.init()` (`SingleThread/SingleThreadApp.swift:14-18`) runs synchronously on the main thread before any view: `SentryBootstrap.startIfEnabled()` then `State(initialValue: AppViewModel())`. +- `SentryBootstrap.startIfEnabled()` is fully synchronous: reads `CrashReportingPreference().isEnabled`, `SentryConfiguration.make(...)`, then `SentrySDK.start { options in … }` with DSN, breadcrumbs, sampling, crash/hang/watchdog flags, scrubbers (`SingleThread/SentryBootstrap.swift:10-41`). +- `AppViewModel.init` (`SingleThread/AppViewModel.swift:21-64`) does, in order: `makeStore` (line 26), reads `SortOptionStore().load()` (line 28), constructs `AISortCoordinator` (line 29), `registerDefaults()` incl. one-time App-Group migration (line 30, body `:161-179`), `BackgroundImageStore()` (line 32, no I/O in init), iOS `setupSyncService` (line 35), sets `store.onRemindersChanged` (lines 37-45), `onAIRerankRequested` + `setupAIRankingObservation()` (lines 47-51), iOS `setupSyncObservation()`/`setupEntitlementObservation()` (lines 55-57). +- `makeStore` (`SingleThread/AppViewModel.swift:~283-360`) returns `ReminderStore(...)`; `ReminderStore.init` (`SingleThreadCore/.../ReminderStore.swift:23-57`) synchronously constructs `EKEventStore()` plus `SkippedReminderStore`, `SkipCountStore`, `PendingCompletionStore`, `ExcludedListStore`, `DailyCompletionStore`, `CompletionCounterStore` (all App-Group/UserDefaults-backed, several performing a synchronous read) and `EntitlementStore` (whose init spawns an async StoreKit task, `EntitlementStore.swift:17-21`). Init only assigns fields; `store.start()` is NOT called here. +- `ContentView.task` (`SingleThread/ContentView.swift:280-294`) → `ContentViewModel.task` (`SingleThread/ContentViewModel.swift:~143-160`) is the deferred path: `await store.start()`, then build/start `StaleReminderRechecker`, `await backgroundImage.refreshIfNeeded()`. + +## Q2: How does the WatchConnectivity sync service operate — queues/actors, activation, payload handling, main-actor intersection? + +### Findings +- `SkippedReminderSyncService` is a nonisolated `public final class ... NSObject, WCSessionDelegate`, entirely inside `#if os(iOS) || os(watchOS)` (`SingleThreadCore/.../SkippedReminderSyncService.swift:16,48`). It is in `SingleThreadCore`, which does NOT enable default MainActor isolation. +- Seam: protocol `SkipSyncSession` (`:18-35`) abstracts `isReachable`, `activate()`, `updateApplicationContext`, `sendMessage`, `queueUserInfo`; `extension WCSession: SkipSyncSession` (`:36-42`); `FakeSession` test double (`SingleThreadTests/TestFixtures.swift:71-104`). +- `activate()` (`:266-271`) assigns `session.delegate = self` only when `session as? WCSession` succeeds, then calls `activate()`. Real delegate callbacks run on WCSession's own serial delegate queue — the service never hops actors itself. +- Delegate entry points: `didReceiveApplicationContext → apply(context:)` (`:334-337`), `didReceiveMessage → apply(request:)` (`:339-341`), `didReceiveUserInfo → apply(request:)` (`:344-350`), `activationDidCompleteWith` logs errors (`:352-359`), iOS `sessionDidDeactivate` re-activates (`:361-366`). +- ~18 `nonisolated(unsafe) var on…Received` hooks (`:175-262`) are written once from the main actor before `activate()` and read afterward on the delegate queue (documented happens-before, `:161-172`). Main-actor hop happens at the AppViewModel/WatchAppViewModel call sites via `Task { await store?.… }` / `Task { @MainActor in … }`, not in the service. +- Send side `pushAll()` (`:276-333`) builds one latest-wins `updateApplicationContext`; `entitled` read via `MainActor.assumeIsolated` (`:325-330`). `deliver(_:)` (`:430-440`) is the single outgoing chokepoint: `sendMessage` when reachable, else `queueUserInfo`. Receive side `apply(context:)` (`:447-517`) + `applyRemaining(context:)` (`:522-548`) decode key-by-key, persist via injected stores, then invoke handlers. `apply(request:)` (`:412-428`) decodes complete/delete/reschedule. +- No `WCContentIndex`, `transferFile`, or file-transfer usage exists anywhere in the repo (also Q1 locator). +- iOS wiring `AppViewModel.setupSyncService` (`SingleThread/AppViewModel.swift:486-540`): guard `session != nil || (WCSession.isSupported() && !usesInMemoryStore)` (`:489`), handlers assigned before `activate()` (`:504-521`), store hooks wired after (`:523-539`). Watch wiring `WatchAppViewModel.setupSyncService` (`SingleThreadWatch/WatchAppViewModel.swift:190-228`); `WCSession.default` at `:248`; delayed application-context replay for UI tests at `:312-327`. + +## Q3: How does the EventKit `ReminderStore` persistence path execute reads, writes, settle delays, and change observers, and what guards them? + +### Findings +- `ReminderStore` is `@MainActor @Observable public final class` (`SingleThreadCore/.../ReminderStore.swift:14-16`); `EventKitStoring` is `@MainActor` and `EKEventStore` conforms (`EventKitStoring.swift:9-10,63`) — every EventKit call routed through the store is MainActor-isolated. +- `ResumptionGate` (`@unchecked Sendable`, `ResumptionGate.swift:19`) is the one-shot continuation guard; its invariant is that check-and-set happens on MainActor with no `await` between (`:30-33`), and `resumeOnMainActor(_:_:)` is `nonisolated` so EventKit's off-main completion can hop back (`:46-51`). +- `fetchReminders(matching:)` (`ReminderStore.swift:711-721`) bridges the completion-handler API with a `ResumptionGate`; the comment states fetch works off the main thread and the gate keeps setup from blocking UI. +- Reads: `reload(clearSkipped:)` (`:522`) → `refreshSourcesIfNecessary` (iOS, `:526`) → predicate (`:542`) → `await fetchReminders` (`:549`); computes `hasHidden`, filters pending completions, assigns `reminders`/`availableLists`, reconciles skip state, prunes, fires `onRemindersChanged?()` (`:567-587`). Derived state is computed MainActor properties (`:182-243`). +- Writes (iOS branch) all follow save → `await settle()` → `await reload()`: `completeReminder` (`:294`), `undoLastCompletion` (`:353`), `deleteReminder` (`:378`), `addReminder` (`:407`), `rescheduleReminder` (`:438`). `skipCurrentReminder` (`:463`) spawns `Task { await settle(); if applySkipSet(...) { await reload() } }` (`:471-483`) guarded by `skipGeneration`; `skipCurrentReminderImmediately` (`:504`) writes synchronously with no settle. +- Settle seam: `public typealias ReminderStoreSettle = @Sendable () async -> Void` (`:12`), production default `Task.sleep(200 ms)` (`:48-50`), tests inject a no-op. +- Change observer: `EventStoreChangedObserving.onChange(_:)` (`EventStoreChangedObserver.swift:9-11`); `EventStoreChangedObserver.onChange` registers `.EKEventStoreChanged` on `queue: .main` (`:25-28`) and returns a removal closure. `StaleReminderRechecker` (`@MainActor`, `:18`) consumes it; reentrancy guarded by `isReloading`/`reloadPending` with no `await` between the final `reload()` and clearing the flag (`:107-118`). `live(store:)` factory at `:127-141`; lifecycle in `ContentViewModel.task` (`SingleThread/ContentViewModel.swift:144-161`). +- Watch-relayed writes do not touch EventKit: they mutate `reminders`/`pendingCompletions` and fire relay hooks (`ReminderStore.swift:296-312, 379-385, 443-447`). +- No `NSFileCoordinator`/coordinated file I/O exists in the repo (per Q1/Q3 locator). + +## Q4: How is the AI sorting coordinator triggered and executed? + +### Findings +- `AISortCoordinator` is explicit `@MainActor` (`SingleThreadCore/.../AIReminderRanking.swift:63-64`); constructed in `AppViewModel.init` (`SingleThread/AppViewModel.swift:29`) with a 500 ms default debounce (`:23-25`). +- Triggers: rules-text App-Group notification (`AppViewModel.swift:450-457`), `store.onRemindersChanged` (`:39-45`), `store.onAIRerankRequested` → forced refresh (`:47-51`, request at `ReminderStore.swift:264-267`). +- `refreshAIRanking(force:)` (`AppViewModel.swift:418-425`): cancels when sort option ≠ `.ai`, else `aiCoordinator.update(rules: AISortRulesStore().load(), candidates: store.aiCandidates, force: force)`. +- `update(rules:candidates:force:)` (`AIReminderRanking.swift:104`): blank rules or `!ranker.isAvailable` → cancel/reset/`emit([:])` (`:105-113`); digest hash of rules+candidates (`:179-187`); dedupe unless forced (`:121-128`); increments `generation`, cancels `pending`, creates `Task { Task.sleep(for: wait); await ranker.rank(...) }` (`:133-142`). Success guards generation, clears pending, sets `lastCompletedDigest`, `emit(reconcile(...))` (`:138-143`); failure retains prior ranking, leaves digest unset for retry, logs, fires `onRankingFailed` unless unavailable (`:144-151`). `reconcile` drops invented ids/dedupes/appends omitted (`:79-99`). +- The `pending` Task inherits MainActor isolation; only `await ranker.rank` and `Task.sleep` suspend. `emit`/callbacks run on the main actor. No separate queue/actor. +- Production ranker `FoundationModelsReminderRanker` is `nonisolated struct`, `isAvailable` from `SystemLanguageModel.default.availability` (`SingleThread/FoundationModelsReminderRanker.swift:12,22-48`); `rank` builds a `LanguageModelSession` and awaits `session.respond(...)` (`:78-91`), so it runs off the MainActor when awaited. Protocol `AIReminderRanking: Sendable` (`AIReminderRanking.swift:37`). +- Flow: reminder change / rules edit / refresh button → `refreshAIRanking` → `update` (MainActor debounce+dedupe) → `await ranker.rank` (nonisolated) → `reconcile` → `onRankingUpdated` → `store.setAIRanking` → `visibleReminders` applies AI order (`ReminderStore.swift:172-196`). `setAIRanking` deliberately fires no `onRemindersChanged` (`:252-259`). + +## Q5: How are App Group `UserDefaults` values and change notifications read and observed, and what constraints exist? + +### Findings +- `AppGroup.defaults` is a deliberately cached `nonisolated(unsafe) static let` (`SingleThreadCore/.../AppGroup.swift:28-29`); suite `group.app.alanvardy.SingleThread` (`:8`), falling back to `.standard` when the suite is unavailable (`:11-13,19-21`). The doc comment (`:15-26`) states the invariant: `didChangeNotification` posts with the changing instance as `object`, so a computed property would break every `object:`-filtered observer. +- Store wrappers (`BoolPreferenceStore.swift:16`, `SortOption.swift:59`, `AISortRulesStore.swift:9`, `SkipCountStore.swift:26`, `DailyCompletionStore.swift:18`, `PendingCompletionStore.swift:20`, `ExcludedListStore.swift:7`, `CompletionCounterStore.swift:20`) all default `defaults: UserDefaults = AppGroup.defaults`. `BoolPreferenceStore` deliberately avoids `bool(forKey:)` so nil ≠ explicitly-off (`:7-9,26-28`). +- App-side observers, all `object: AppGroup.defaults`, `queue: .main`, then a MainActor hop: `PreferenceHolder` (`PreferenceHolder.swift:13-22`, refresh `:53-67`, token removal `:45-51`), `AppViewModel.setupSyncObservation` → `handlePreferencesChanged()` (value-diff against cached `last*` fields before `pushAll`, `AppViewModel.swift:565-638`), `AppViewModel.setupAIRankingObservation` (`:435-461`). One-time `enableActionButtons` migration from `.standard` (`:129-148`). +- Watch side does NOT observe `didChangeNotification`; state holders read once and rely on explicit sync callbacks (`ShowDateState.swift:14-34`, `ShowListState.swift:31-34`, `ShowRecurrenceState.swift:29`, `ShowAlarmsState.swift:29`, `ShowCompletionGlowState.swift:28`). `ShowEnableActionButtonsState` is the outlier using `AppGroup.defaults` (`:21-26`). +- Divergence noted: the sync service receive side writes show-* prefs through `BoolPreferenceStore` over `AppGroup.defaults` (`SkippedReminderSyncService.swift:495-534`), while watch UI holders read/write `.standard` for the same keys except `enableActionButtons`; these coincide only because `AppGroup.defaults` falls back to `.standard` on watch. +- Tests enforcing the invariants: `SingleThreadTests/AppGroupTests.swift:28-72` (`defaultsIsAStableInstance` at `:36-42`, `objectFilteredObserverSeesAppGroupWrites` at `:44-64`). Runtime regression probe `scripts/check-appgroup-notify.sh`. + +## Q6: What is the current Sentry integration, and what diagnostics/breadcrumbs exist around hangs? + +### Findings +- `SingleThreadApp.swift:15` calls `SentryBootstrap.startIfEnabled()` before `AppViewModel()` (line 16). Guard requires `CrashReportingPreference().isEnabled` AND a non-nil `SentryConfiguration.make(...)` (`SentryBootstrap.swift:11`). +- Options (`SentryBootstrap.swift:17-41`): dsn, environment, `maxBreadcrumbs`, `sendDefaultPii`, `tracesSampleRate`, `enableCrashHandler = true` (`:25`), `enableAppHangTracking = true` (`:26`), `enableWatchdogTerminationTracking = true` (`:27`), `enableAutoSessionTracking = false`; iOS `attachScreenshot/attachViewHierarchy = false` (`:31-34`); macOS `enableUncaughtNSExceptionReporting = true` (`:35-37`); `beforeSend`/`beforeBreadcrumb` routed to `SentryScrubber.scrub` (`:38-39`). `setEnabled(_:)` starts/closes (`:44-48`). +- Hang/watchdog are SDK auto-instrumentation flags only — no app-configured thresholds or callbacks. No `SentrySDK.addBreadcrumb`/`captureMessage`/`captureError` calls exist anywhere (only `start`/`close`). +- `SentryConfiguration` is pure `Equatable, Sendable`, no Sentry import (`:4-19`); `make` returns nil on empty DSN, defaults `maxBreadcrumbs: 20`, `sendDefaultPii: false`, `tracesSampleRate: 0` (`:22-35`). +- `SentryScrubber` is an allow-list scrubber: nils user/extra/request/modules/message/transaction, filters tags to `["environment","release","level"]`, nils each exception `value` but keeps `type` so crashes still group, compacts breadcrumbs through `scrub` (`SentryScrubber.swift:11-33`). +- Consent: `CrashReportingPreference` persists in `UserDefaults.standard` deliberately not App Group (`:4-6,9,20`), absent key → enabled (`:24-26`); binding in `SettingsBindings.swift:219-230`; toggle `SettingsView.swift:155`. +- Only prose source is `docs/CrashReporting.md` (DSN/org setup, dSYM upload; Debug uses `dwarf` so only Release symbolicate). + +## Q7: What test seams and conventions exist, and how are targeted tests run? + +### Findings +- WatchConnectivity seam: `SkipSyncSession` (`SkippedReminderSyncService.swift:18`), `WCSession` conformance (`:36`), `FakeSession` (`SingleThreadTests/TestFixtures.swift:71-104`). Delegate callbacks driven directly on the service. Suites: `SkippedReminderSyncServiceTests.swift`, `SkipCountSyncTests`, `AISortOptionTravelTests`, `AppViewModelSyncWiringTests.swift:22-27`. +- EventKit seam: `EventKitStoring` (`EventKitStoring.swift:8`, conformance `:43`), `InMemoryEventStore` (`InMemoryEventStore.swift:13`), injectable `ReminderStore.init` (`ReminderStore.swift:23`) with `loadsReminders: false` and `makeEmptyReminderStore()`; shared `sharedTestEventStore`/builders in `TestFixtures.swift`. Change observer seam `EventStoreChangedObserving` (`EventStoreChangedObserver.swift:8`). Suites: `EventKitStoringTests`, `EventStoreChangedObserverTests`, `ReminderStoreTests`, `ReminderStoreGateTests`. +- App Group: `AppGroupTests.swift:26-71`; convention is isolated `UserDefaults.standard` keys with per-test `UUID` suffixes rather than the shared suite. +- Main-actor coordinator: ranker seam `AIReminderRanking: Sendable` (`AIReminderRanking.swift:36`); fakes `CannedRanker`, `UnavailableRanker`, `SwitchableRanker`, `ErrorSwitchableRanker`, `NeverCompletingRanker`, `BlockOnceRanker`, `FailingRanker` in `AISortCoordinatorTests.swift:9`; tests inject a short debounce and `await`. Coordinator tests are `@MainActor struct` (`:187`). +- Entry points: `Makefile` `test` → `./scripts/test.sh --unit-only`; `ui-test` → `--ui-only`; `check` → full gate; watch/mac targets via `xcodebuild -only-testing:...`. `scripts/test.sh` runs macOS-native unit tests under `--unit-only` (`:344`) and the full matrix otherwise (`:287-349`); `resolve_sim_udid` precedence at `:28-72`. `scripts/test-one.sh` runs a single bounded suite/case with pinned destination and exits non-zero on a zero-match. + +## Cross-Cutting Observations +- The ticket's cited culprit thread names (`WCContentIndex index`, file coordination) have **no app-code counterpart**: the repo contains no `WCContentIndex`, `transferFile`, `NSFileCoordinator`, or `BGTaskScheduler` usage; the only WatchConnectivity API surface is `updateApplicationContext`/`sendMessage`/`transferUserInfo` (`SkippedReminderSyncService.swift`) with no app frames on the hung main thread. Those frames are therefore system-internal to the captured stack, consistent with the "no app frames" report. +- Main-thread work is concentrated in two places: synchronous construction at `SingleThreadApp.init` / `AppViewModel.init` (Q1), and every `@MainActor` coordinator/observer callback hopping back to the main actor (Q3-Q5). +- App Group `UserDefaults` is the shared wire between phone and watch; its correctness rests on three documented invariants — single cached instance, `object:` filtering, and token lifetime — each with a guarding test. +- EventKit's completion-handler APIs are bridged with `ResumptionGate` + `resumeOnMainActor`; writes are spread out by a 200 ms injectable settle delay before reload. + +## Open Areas +- The actual cause of the reported hang is NOT established by static research: no code path was found that obviously blocks the main thread indefinitely. The hang could be a system-level stall (EventKit/CloudKit/WatchConnectivity internals) not visible in app code. +- The captured Sentry stacks were not provided to the researchers; stack-frame correlation was not possible. No Sentry breadcrumb/attachment diagnostics exist to correlate (Q6). +- Exact WCSession delegate-queue identity and whether a delegate callback can contend with MainActor work were reasoned from structure/comments, not instrumented (Q2). +- Whether the production 200 ms settle or the rechecker loop can overlap/starve under specific conditions was not proven. diff --git a/.pi/orksorksorks/alanvardy-var-1165-sentry-error/structure.md b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/structure.md new file mode 100644 index 00000000..fa60b9aa --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/structure.md @@ -0,0 +1,127 @@ +# Structure Outline + +## Approach + +Contain, don't fix: the reported hang is a system-only stall +(`BKSDisplayBrightnessGetCurrent` → `bootstrap_look_up`) with no app frame, so +the shippable increment is a narrow `beforeSend` filter that drops exactly that +event class and leaves every other hang/crash untouched. The decision lives in a +pure predicate over plain values (repo test-seam style), with a thin `Event` +adapter; a single bundled hardening slice moves `BackgroundImageStore`'s +synchronous disk I/O off the MainActor. + +All work lands on the main ticket branch — no child tickets. + +**Pre-phase validation (plan step, not a slice):** confirm the open risk from +`design.md` — that the fatal-flush persisted event still carries +`event.exceptions[0].stacktrace` / `event.threads` at `beforeSend` time — before +relying on the filter for the next-launch flush. If it does not hold, the slice +still contains the in-process hang and Phase 1's contract is unchanged. + +--- + +## Phase 1: Walking skeleton — drop the captured BackBoardServices stall in `beforeSend` + +**Outcome**: the exact captured "Fatal App Hang Fully Blocked" event (system-only +blocked stack matching the backlight denylist) is dropped instead of reported, +end to end from the real captured signature through the pure predicate to +`SentryScrubber.scrub(_:)` returning `nil`; a hang carrying any app frame is +reported unchanged. Green tests prove both directions. + +**Files**: `SingleThread/SentryScrubber.swift`, +`SingleThreadTests/SentryScrubberTests.swift` (app-target logic + its suite). + +**Key changes**: +- `nonisolated static let systemStallSignatureFrames: Set` — denylist + `["BKSDisplayBrightnessGetCurrent", "bootstrap_look_up"]`. +- `nonisolated static func isSystemStall(mechanismType: String?, frameSignatures: [String], hasAppFrames: Bool) -> Bool` + — pure three-condition conjunction: `mechanismType == "AppHang"` (public, + stable marker) ∧ `!hasAppFrames` ∧ denylist ∩ signatures non-empty. +- `nonisolated static func appHangStack(in event: Event) -> (mechanismType: String, frameSignatures: [String], hasAppFrames: Bool)?` + — thin adapter selecting the `"AppHang"` exception and reading its + `stacktrace.frames` (`inApp == true` **or** `package == "SingleThread"` ⇒ app + frame); returns `nil` for any shape it does not recognize (fail-open). +- `nonisolated static func scrub(_ event: Event) -> Event?` — when + `appHangStack` is non-nil **and** `isSystemStall` is true, return `nil`; + otherwise run the existing PII-scrub branch order unchanged. +- Doc comment on the predicate naming issue `7821993169`, the captured chain, + and why a system-only stack is intentionally filtered (design Decision 6). + +**Contract**: `isSystemStall(mechanismType:frameSignatures:hasAppFrames:)` and +`scrub(_:)`'s nil-on-match behaviour. Later slices/tests depend only on these, +never on the adapter internals. No `SentryBootstrap` change — `beforeSend` +already routes through `scrub` (`SentryBootstrap.swift:38`). + +**Tests**: `dropsCapturedBacklightSystemStall` (fixture = captured frame list +verbatim; happy path), `keepsHangWithAppFrame` (sad path — one `inApp` frame), +`keepsNonHangBacklightEvent` (sad path — same system frames, no `"AppHang"` +mechanism ⇒ PII still scrubbed, not dropped). +**Verify**: `scripts/test-one.sh SingleThreadTests/SentryScrubberTests` passes; +`make build`; manual check = the two assertions above demonstrate drop vs report. + +--- + +## Phase 2: Fail-open guarantee — SDK churn or odd shapes never lose signal + +**Outcome**: every deviation from the exact filtered shape still reports the +event, so a sentry-cocoa rename or a missing/empty stack surfaces as noise, never +as silent loss. This is the correctness hardening of Phase 1's predicate. + +**Files**: `SingleThread/SentryScrubber.swift` (explicit `false`/`nil` guards), +`SingleThreadTests/SentryScrubberTests.swift`. + +**Key changes**: clamp `isSystemStall` to `false` and `appHangStack` to `nil` +for every under-specified input; treat a frame as app-owned if either +`inApp == true` or `package == "SingleThread"` (belt-and-braces per +`SentryFrame.h`). + +**Contract**: unchanged from Phase 1 — only more negative cases are pinned. + +**Tests**: `keepsEventWhenMechanismRenamed`, `keepsEventWhenStacktraceMissing`, +`keepsEventWhenFrameListEmpty`, `keepsEventWhenDenylistPartiallyMatches`, +`keepsEventWhenAnyFrameIsInApp` (all sad paths; assertions are `scrub != nil`). +**Verify**: `scripts/test-one.sh SingleThreadTests/SentryScrubberTests` passes. + +--- + +## Phase 3: `BackgroundImageStore` disk I/O off the MainActor + +**Outcome**: the wallpaper store's synchronous `Data(contentsOf:)` / +`FileManager` reads and writes no longer execute on the MainActor, removing a +real main-thread stall source independent of the Sentry filter. + +**Files**: `SingleThread/BackgroundImageStore.swift`, +`SingleThreadTests/BackgroundImageStoreTests.swift`. + +**Key changes**: +- `loadStoredImage()`, `isFresh(maxAge:)`, `persist(imageData:metadata:)` move + their filesystem work into `nonisolated` helpers awaited from the existing + `async` callers; observable state is assigned back on the MainActor after the + disk work returns (disk-before-state ordering preserved). +- Public surface unchanged: `refreshIfNeeded(maxAge:)` / `forceRefresh()` + signatures and `imageData`/`photographer`/`photographerURL` semantics stay as + today; `BackgroundImageFetching` fakes unaffected. + +**Contract**: `refreshIfNeeded`/`forceRefresh` behaviour and the observable +properties — existing callers (`ContentViewModel:156`, +`BackgroundSettingsView:57`) need no change. + +**Tests**: existing `BackgroundImageStoreTests` must remain green; add +`diskWorkRunsOffMainActor` (sad path = the sync-read regression) plus a +behavior-parity case that a stored image still loads and a fresh sidecar still +short-circuits the network. +**Verify**: `scripts/test-one.sh SingleThreadTests/BackgroundImageStoreTests` +passes; `make build`. + +--- + +## Testing Checkpoints + +- After Phase 1: `SingleThreadTests/SentryScrubberTests` green (captured stack + dropped, app-frame hang kept) before Phase 2 starts. +- After Phase 2: same suite green with the fail-open negatives added. +- After Phase 3: `SingleThreadTests/BackgroundImageStoreTests` green + (off-main assertion + parity) before the gate. +- Once all three phases commit: run `make format` + `make lint` inline, then the + full CI gate **once** via the `run-gate` skill (never `nohup`); no UI test is + added (design decision — a UI test cannot trigger a BackBoardServices stall). diff --git a/.pi/orksorksorks/alanvardy-var-1165-sentry-error/task.md b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/task.md new file mode 100644 index 00000000..74df74d0 --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1165-sentry-error/task.md @@ -0,0 +1,9 @@ +# Task + +Diagnose and fix a production Sentry "Fatal App Hang Fully Blocked" (issue +7821993169, culprit `SingleThreadApp.$main`) on SingleThread 1.4+21 / iOS 27.2. +The captured stacks contain no SingleThread app frames on the hung main thread — +only the Sentry ANR-detection thread plus system and WatchConnectivity +background threads — so the root cause is unidentified and must be established +before a fix can be designed. The fix must ship with a unit test that +reproduces the hang condition, per the repo's bug-fix test policy. diff --git a/SingleThread/BackgroundImageStore.swift b/SingleThread/BackgroundImageStore.swift index 18beb7f0..892e67f7 100644 --- a/SingleThread/BackgroundImageStore.swift +++ b/SingleThread/BackgroundImageStore.swift @@ -29,7 +29,7 @@ extension URLSession: BackgroundImageFetching { /// Sidecar metadata persisted next to the photo bytes. One write covers both /// the displayed photo and the attribution credit, so they can never disagree. -private struct BackgroundMetadata: Codable { +private nonisolated struct BackgroundMetadata: Codable { let photographer: String let photographerURL: String? let fetchedAt: Date @@ -88,6 +88,16 @@ final class BackgroundImageStore { directory.appendingPathComponent("background.json") } + /// Runs synchronous disk work on the concurrent executor, off the MainActor. + /// `@concurrent` is required because `SWIFT_APPROACHABLE_CONCURRENCY` leaves a + /// plain `nonisolated async` function on the caller's actor (here MainActor). + /// Internal so tests can prove the executor leaves the MainActor. + @concurrent + nonisolated static func offMain( + _ work: @Sendable () throws -> T) async rethrows -> T { + try work() + } + /// Loads stored bytes into observable state, then refetches over the /// network when the sidecar is missing/corrupt/stale. Called from /// ContentView's `.task`; rendering is never gated on the network result. @@ -96,9 +106,9 @@ final class BackgroundImageStore { /// Failure convention: persist to disk FIRST, then flip observable state; /// on any error, log and keep prior state. func refreshIfNeeded(maxAge: TimeInterval = BackgroundImageStore.defaultMaxAge) async { - loadStoredImage() + await loadStoredImage() guard !isPinned || imageData == nil else { return } - guard !isFresh(maxAge: maxAge) else { return } + guard await !isFresh(maxAge: maxAge) else { return } guard !isFetching else { return } isFetching = true defer { isFetching = false } @@ -107,7 +117,7 @@ final class BackgroundImageStore { // Re-check after the network suspension: a re-pin while the fetch was // in flight must not commit a new photo over the pinned one. guard !isPinned || imageData == nil else { return } - try commit(wallpaper) + try await commit(wallpaper) } catch { Self.logger.error("Background refresh failed: \(error.localizedDescription)") } @@ -129,7 +139,7 @@ final class BackgroundImageStore { } do { let wallpaper = try await fetchWallpaper(from: Self.randomEndpoint) - try commit(wallpaper) + try await commit(wallpaper) } catch { Self.logger.error("Background force refresh failed: \(error.localizedDescription)") } @@ -146,24 +156,42 @@ final class BackgroundImageStore { } /// Missing/corrupt sidecar ⇒ treated as "no valid stored image". - func loadStoredImage() { - guard let metadataData = try? Data(contentsOf: metadataURL), - let metadata = try? decodeMetadata(from: metadataData), - let data = try? Data(contentsOf: imageURL), - isDecodableImage(data) - else { + func loadStoredImage() async { + let metadataURL = metadataURL + let imageURL = imageURL + let loaded = await Self.offMain { () -> LoadedImage? in + guard + let metadataData = try? Data(contentsOf: metadataURL), + let metadata = try? Self.decodeMetadata(from: metadataData), + let data = try? Data(contentsOf: imageURL), + Self.isDecodableImage(data) + else { return nil } + return LoadedImage( + data: data, + photographer: metadata.photographer, + photographerURL: metadata.photographerURL) + } + guard let loaded else { imageData = nil photographer = nil photographerURL = nil return } - imageData = data - photographer = metadata.photographer - photographerURL = metadata.photographerURL.flatMap(URL.init) + imageData = loaded.data + photographer = loaded.photographer + photographerURL = loaded.photographerURL.flatMap(URL.init) } // MARK: Private + /// A decoded, validated stored wallpaper (plain `Sendable` value, no + /// observable state). + private nonisolated struct LoadedImage: Sendable { + let data: Data + let photographer: String + let photographerURL: String? + } + /// A decoded, validated wallpaper ready to persist. Purely a value — no /// observable state — so callers can re-check pin state after the fetch. private struct FetchedWallpaper { @@ -197,6 +225,20 @@ final class BackgroundImageStore { /// automatic refresh never interleaves with a manual one at their awaits. private var isFetching = false + private nonisolated static func decodeMetadata(from data: Data) throws -> BackgroundMetadata { + let decoder = JSONDecoder() + decoder.dateDecodingStrategy = .iso8601 + return try decoder.decode(BackgroundMetadata.self, from: data) + } + + private nonisolated static func isDecodableImage(_ data: Data) -> Bool { + #if os(iOS) + UIImage(data: data) != nil + #elseif os(macOS) + NSImage(data: data) != nil + #endif + } + /// Fetches the Unsplash payload and photo from the given endpoint and /// validates it as a decodable image. Mutates nothing: the actual network /// I/O suspends off the main thread and returns a plain value. @@ -205,7 +247,7 @@ final class BackgroundImageStore { let decoder = JSONDecoder() let payload = try decoder.decode(UnsplashPayload.self, from: payloadData) let data = try await client.fetchData(from: payload.url) - guard isDecodableImage(data) else { throw URLError(.cannotDecodeContentData) } + guard Self.isDecodableImage(data) else { throw URLError(.cannotDecodeContentData) } return FetchedWallpaper( data: data, photographer: payload.photographer, @@ -214,46 +256,44 @@ final class BackgroundImageStore { /// Persists both files atomically, then flips the observable state. /// Disk before state keeps the displayed photo and attribution consistent. - private func commit(_ wallpaper: FetchedWallpaper) throws { + private func commit(_ wallpaper: FetchedWallpaper) async throws { let metadata = BackgroundMetadata( photographer: wallpaper.photographer, photographerURL: wallpaper.photographerURL?.absoluteString, fetchedAt: Date()) - try persist(imageData: wallpaper.data, metadata: metadata) // disk before state + try await persist(imageData: wallpaper.data, metadata: metadata) // disk before state imageData = wallpaper.data photographer = wallpaper.photographer photographerURL = wallpaper.photographerURL } - private func isFresh(maxAge: TimeInterval) -> Bool { - guard let metadataData = try? Data(contentsOf: metadataURL), - let metadata = try? decodeMetadata(from: metadataData) - else { return false } - return Date().timeIntervalSince(metadata.fetchedAt) < maxAge - } - - private func decodeMetadata(from data: Data) throws -> BackgroundMetadata { - let decoder = JSONDecoder() - decoder.dateDecodingStrategy = .iso8601 - return try decoder.decode(BackgroundMetadata.self, from: data) - } - - private nonisolated func isDecodableImage(_ data: Data) -> Bool { - #if os(iOS) - UIImage(data: data) != nil - #elseif os(macOS) - NSImage(data: data) != nil - #endif + private func isFresh(maxAge: TimeInterval) async -> Bool { + let metadataURL = metadataURL + let fetchedAt = await Self.offMain { () -> Date? in + guard + let data = try? Data(contentsOf: metadataURL), + let metadata = try? Self.decodeMetadata(from: data) + else { return nil } + return metadata.fetchedAt + } + guard let fetchedAt else { return false } + return Date().timeIntervalSince(fetchedAt) < maxAge } /// Creates the directory if needed and writes both files atomically. - private func persist(imageData: Data, metadata: BackgroundMetadata) throws { - try FileManager.default.createDirectory( - at: directory, withIntermediateDirectories: true) - try imageData.write(to: imageURL, options: .atomic) - let encoder = JSONEncoder() - encoder.dateEncodingStrategy = .iso8601 - try encoder.encode(metadata).write(to: metadataURL, options: .atomic) + private func persist(imageData: Data, metadata: BackgroundMetadata) async throws { + let directory = directory + let imageURL = imageURL + let metadataURL = metadataURL + try await Self.offMain { + let encoder = JSONEncoder() + encoder.dateEncodingStrategy = .iso8601 + let metadataData = try encoder.encode(metadata) + try FileManager.default.createDirectory( + at: directory, withIntermediateDirectories: true) + try imageData.write(to: imageURL, options: .atomic) + try metadataData.write(to: metadataURL, options: .atomic) + } } } diff --git a/SingleThread/SentryScrubber.swift b/SingleThread/SentryScrubber.swift index b5df1545..730d0438 100644 --- a/SingleThread/SentryScrubber.swift +++ b/SingleThread/SentryScrubber.swift @@ -7,10 +7,71 @@ import Sentry /// breadcrumb category/level/timestamp). /// Returns `nil` to drop a breadcrumb when its payload is not allow-listed. enum SentryScrubber { + /// One frame reduced to the fields `beforeSend` actually receives. + struct StackFrameSignature: Equatable { + /// `Frame.package` — the loaded binary's path. + let image: String + /// `Frame.inApp`, with the app-bundle path as a belt-and-braces fallback. + let isApp: Bool + } + /// Tag keys that carry app/device metadata only — never reminder content. nonisolated static let allowedTags: Set = ["environment", "release", "level"] + /// Binary-image substrings of the synchronous BackBoardServices/backlight XPC + /// stall UIKit performs on the main thread. Sentry issue 7821993169: + /// `BKSDisplayBrightnessGetCurrent` (BackBoardServices) → + /// `bootstrap_look_up` (libxpc). + /// + /// Function names are NOT available in `beforeSend` — sentry-cocoa sets only + /// `package`/`inApp` at capture and symbolicates `function` server-side — so + /// the signature is matched on image paths. + nonisolated static let systemStallSignatureImages: [String] = [ + "BackBoardServices", + "libxpc.dylib" + ] + + /// Pure drop decision. Fail-open: any under-specified input returns `false`, + /// so the event is reported rather than silently lost. + /// + /// All three conditions must hold: + /// 1. the mechanism is the public, stable `"AppHang"` marker, + /// 2. the stack contains every image in `systemStallSignatureImages`, + /// 3. no in-app frame sits at or deeper than the first stall frame. + nonisolated static func isSystemStall( + mechanismType: String?, + frames: [StackFrameSignature]) -> Bool { + guard mechanismType == "AppHang" else { return false } + guard !frames.isEmpty else { return false } + guard systemStallSignatureImages.allSatisfy({ signature in + frames.contains { $0.image.contains(signature) } + }) else { return false } + guard let firstStall = frames.firstIndex(where: { frame in + systemStallSignatureImages.contains { frame.image.contains($0) } + }) else { return false } + return !frames[firstStall...].contains(where: \.isApp) + } + + /// Reduces a Sentry event to an app-hang signature, or `nil` for any shape it + /// does not recognise (fail-open). + nonisolated static func appHangStack( + in event: Event) -> (mechanismType: String, frames: [StackFrameSignature])? { + guard + let exception = event.exceptions?.first(where: { + $0.mechanism?.type == "AppHang" + }), + let mechanismType = exception.mechanism?.type, + let stacktrace = exception.stacktrace, + !stacktrace.frames.isEmpty + else { return nil } + return (mechanismType, stacktrace.frames.map(StackFrameSignature.init)) + } + nonisolated static func scrub(_ event: Event) -> Event? { + if let stack = appHangStack(in: event), + isSystemStall(mechanismType: stack.mechanismType, frames: stack.frames) { + return nil + } event.user = nil event.extra = nil event.request = nil @@ -34,3 +95,11 @@ enum SentryScrubber { return scrubbed } } + +extension SentryScrubber.StackFrameSignature { + nonisolated init(_ frame: Frame) { + let image = frame.package ?? "" + self.image = image + isApp = (frame.inApp?.boolValue == true) || image.contains(".app/") + } +} diff --git a/SingleThreadTests/BackgroundImageStoreTests.swift b/SingleThreadTests/BackgroundImageStoreTests.swift index f1c86a17..06681500 100644 --- a/SingleThreadTests/BackgroundImageStoreTests.swift +++ b/SingleThreadTests/BackgroundImageStoreTests.swift @@ -1,3 +1,4 @@ +import Darwin import Foundation @testable import SingleThread import Testing @@ -249,6 +250,37 @@ struct BackgroundImageStoreTests { #expect(!store.isRefreshing) } + // MARK: - Off-main disk I/O + + /// Regression guard for the removed synchronous `Data(contentsOf:)` on the + /// MainActor: the executor used for all disk work must leave the MainActor. + @Test + func diskWorkRunsOffMainActor() async { + let ranOnMain = await BackgroundImageStore.offMain { pthread_main_np() != 0 } + #expect(ranOnMain == false) + } + + /// A stored image still loads and a fresh sidecar still short-circuits the + /// network after the disk work moved off-main. + @Test + func storedImageStillLoadsAndBlocksRefetchWhenFresh() async { + let fake = FakeBackgroundFetcher() + fake.stubbedData[Self.endpoint] = .success(payloadJSON()) + fake.stubbedData[Self.imageURL] = .success(BackgroundTestFixtures.jpegData) + let (store, _) = makeStore(client: fake) + await store.refreshIfNeeded(maxAge: 3600) + #expect(store.imageData == BackgroundTestFixtures.jpegData) + + await store.loadStoredImage() + #expect(store.photographer == "NEOM") + + let countAfterLoad = fake.requestedURLs.count + await store.refreshIfNeeded() + #expect( + fake.requestedURLs.count == countAfterLoad, + "fresh sidecar must skip the network after the off-main move") + } + // MARK: - Pin gating @Test @@ -264,7 +296,7 @@ struct BackgroundImageStoreTests { .write(to: store.metadataURL, options: .atomic) try BackgroundTestFixtures.jpegData .write(to: store.imageURL, options: .atomic) - store.loadStoredImage() + await store.loadStoredImage() // Stub the endpoint to confirm it is NOT called. fake.stubbedData[Self.endpoint] = .success(payloadJSON()) @@ -315,7 +347,7 @@ struct BackgroundImageStoreTests { .write(to: store.metadataURL, options: .atomic) try BackgroundTestFixtures.jpegData .write(to: store.imageURL, options: .atomic) - store.loadStoredImage() + await store.loadStoredImage() #expect(store.photographer == "Old") // Pin, then unpin; the unpin fetch is parked at the gate. @@ -364,7 +396,7 @@ struct BackgroundImageStoreTests { .write(to: store.metadataURL, options: .atomic) try BackgroundTestFixtures.jpegData .write(to: store.imageURL, options: .atomic) - store.loadStoredImage() + await store.loadStoredImage() fake.stubbedData[Self.endpoint] = .success(payloadJSON()) fake.stubbedData[Self.imageURL] = .success(BackgroundTestFixtures.jpegData) @@ -393,7 +425,7 @@ struct BackgroundImageStoreTests { .write(to: store.metadataURL, options: .atomic) try BackgroundTestFixtures.jpegData .write(to: store.imageURL, options: .atomic) - store.loadStoredImage() + await store.loadStoredImage() // Stub to confirm endpoint is NOT called. fake.stubbedData[Self.endpoint] = .success(payloadJSON()) @@ -420,7 +452,7 @@ struct BackgroundImageStoreTests { .write(to: store.metadataURL, options: .atomic) try BackgroundTestFixtures.jpegData .write(to: store.imageURL, options: .atomic) - store.loadStoredImage() + await store.loadStoredImage() fake.stubbedData[Self.endpoint] = .success(payloadJSON()) fake.stubbedData[Self.imageURL] = .success(BackgroundTestFixtures.jpegData) diff --git a/SingleThreadTests/SentryScrubberTests.swift b/SingleThreadTests/SentryScrubberTests.swift index b66da88f..84151269 100644 --- a/SingleThreadTests/SentryScrubberTests.swift +++ b/SingleThreadTests/SentryScrubberTests.swift @@ -4,6 +4,8 @@ import Sentry import Testing struct SentryScrubberTests { + // MARK: Internal + @Test func reminderContentIsRemovedFromEvent() { let event = Event() @@ -79,4 +81,121 @@ struct SentryScrubberTests { #expect(scrubbed?.message == nil) #expect(scrubbed?.transaction == nil) } + + // MARK: App-hang system-stall predicate + + @Test + func dropsCapturedBacklightSystemStall() { + #expect(SentryScrubber.scrub(hangEvent(frames: Self.capturedFrames)) == nil) + } + + @Test + func keepsHangWithAppFrameInBlockedChain() { + var frames = Self.capturedFrames + let leaf = frames.count - 1 + frames[leaf] = (Self.appImage, true) // app is the blocked (deepest) frame + #expect(SentryScrubber.scrub(hangEvent(frames: frames)) != nil) + } + + @Test + func keepsNonHangBacklightEvent() { + let event = hangEvent(frames: Self.capturedFrames, mechanismType: "Crash") + event.extra = ["reminderTitle": "Buy milk"] + let scrubbed = SentryScrubber.scrub(event) + #expect(scrubbed != nil) // not dropped + #expect(scrubbed?.extra == nil) // PII scrubbing still runs + } + + // MARK: Fail-open guarantee + + @Test + func keepsEventWhenMechanismRenamed() { + #expect(SentryScrubber.scrub(hangEvent( + frames: Self.capturedFrames, mechanismType: "AppHangRenamed")) != nil) + } + + @Test + func keepsEventWhenStacktraceMissing() { + let event = Event() + let exception = Exception(value: "App hanging.", type: "App Hang Fully Blocked") + exception.mechanism = Mechanism(type: "AppHang") + event.exceptions = [exception] + #expect(SentryScrubber.scrub(event) != nil) + } + + @Test + func keepsEventWhenFrameListEmpty() { + #expect(SentryScrubber.scrub(hangEvent(frames: [])) != nil) + } + + @Test + func keepsEventWhenDenylistPartiallyMatches() { + // BackBoardServices present, libxpc absent ⇒ not the backlight XPC stall. + let frames = Self.capturedFrames.filter { $0.image != Self.libXPC } + #expect(SentryScrubber.scrub(hangEvent(frames: frames)) != nil) + } + + @Test + func keepsEventWhenAppFrameIsInBlockedChain() { + var frames = Self.capturedFrames + frames.insert((Self.appImage, true), at: frames.count) // app frame at the leaf + #expect(SentryScrubber.scrub(hangEvent(frames: frames)) != nil) + } + + @Test + func keepsEventWhenFramePackageIsMissing() { + let frames = Self.capturedFrames.map { (image: "", inApp: $0.inApp) } + #expect(SentryScrubber.scrub(hangEvent(frames: frames)) != nil) + } + + // MARK: Private + + // MARK: Fixtures + + private static let appImage = + "/private/var/containers/Bundle/Application/277152F6-B99C-4FA0-BADB-098D8D06EC13/SingleThread.app/SingleThread" + private static let swiftUI = "/System/Library/Frameworks/SwiftUI.framework/SwiftUI" + private static let uiKit = "/System/Library/PrivateFrameworks/UIKitCore.framework/UIKitCore" + private static let graphicsServices = + "/System/Library/PrivateFrameworks/GraphicsServices.framework/GraphicsServices" + private static let coreFoundation = + "/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation" + private static let libDispatch = "/usr/lib/system/libdispatch.dylib" + private static let backBoard = + "/System/Library/PrivateFrameworks/BackBoardServices.framework/BackBoardServices" + private static let libXPC = "/usr/lib/system/libxpc.dylib" + private static let libObjCMsgSend = "/usr/lib/objc/libobjcMsgSend.dylib" + + /// The raw event's client-side exception stack (`package` + `in_app`), verbatim. + private static let capturedFrames: [(image: String, inApp: Bool)] = [ + ("dyld", false), + (appImage, true), + (swiftUI, false), (swiftUI, false), (swiftUI, false), + (uiKit, false), (graphicsServices, false), + (coreFoundation, false), (coreFoundation, false), (coreFoundation, false), + (libDispatch, false), (libDispatch, false), (libDispatch, false), (libDispatch, false), + (coreFoundation, false), (coreFoundation, false), + (uiKit, false), (uiKit, false), (uiKit, false), + (backBoard, false), (backBoard, false), + (libXPC, false), (libXPC, false), (libXPC, false), (libXPC, false), (libXPC, false), + (libObjCMsgSend, false) + ] + + private func hangEvent( + frames: [(image: String, inApp: Bool)], + mechanismType: String = "AppHang") -> Event { + let event = Event() + let exception = Exception(value: "App hanging.", type: "App Hang Fully Blocked") + exception.mechanism = Mechanism(type: mechanismType) + exception.stacktrace = SentryStacktrace(frames: frames.map { frame in + let sentryFrame = Frame() + sentryFrame.package = frame.image + // Sentry's `Frame.inApp` is SDK-mandated NSNumber. + // swiftlint:disable:next legacy_objc_type + sentryFrame.inApp = NSNumber(value: frame.inApp) + return sentryFrame + }, registers: [:]) + event.exceptions = [exception] + return event + } }