refactor(web): migrate usePushNotifications off raw localStorage (Item #6 round 8)

[Skords-01]

Summary

Web deep-dive Item #6 (localStorage allowlist burndown) round-8 follow-up. Replaces direct localStorage.{getItem,setItem,removeItem} calls in apps/web/src/shared/hooks/usePushNotifications.ts with safeReadStringLS / safeWriteLS / safeRemoveLS for the hub_push_subscribed cache key (1 read + 2 writes + 4 removes total — covers native Capacitor + web push branches symmetrically).

Drops the production allowlist budget 15 → 14 (headroom 0). The hook is removed from the sergeant-design/no-raw-local-storage allowlist in eslint.config.js; budget rationale in .tech-debt/localstorage-allowlist-budget.json updated to cite this round.

Also adds 3 RTL hardening tests that stub Storage.prototype.{getItem,setItem,removeItem} to throw SecurityError / QuotaExceededError, locking in the no-crash semantics in Safari Private Mode and on full-quota devices (the behavior the safe-helpers already guarantee, now with explicit coverage).

Governing Skill

• Primary skill: sergeant-web-ui
• Secondary skill (if truly needed): n/a

Playbook

• Primary playbook: n/a (small refactor matching the round-7 burndown pattern)
• Why this playbook: tracking initiative is the deep-dive itself (docs/diagnostics/2026-05-03-web-deep-dive/02-architecture-and-state.md §2.2) — pattern proven across 3 prior PRs (refactor(web): migrate perf.ts + useDarkMode.ts off raw localStorage (Item 6 follow-up) #1674, feat(web): migrate useActiveFizrukWorkout off raw localStorage (Item 6 follow-up) #1692).
• If no playbook matched, why: same shape as the prior round-7 burndown PR feat(web): migrate useActiveFizrukWorkout off raw localStorage (Item 6 follow-up) #1692, no new policy or workflow.

Verification

pnpm --filter @sergeant/web test -- --run usePushNotifications   # 11/11 passed (was 8/8)
pnpm --filter @sergeant/web typecheck                            # clean
pnpm lint:localstorage-allowlist                                 # 14/14 (headroom 0)

Additional checks:

• Local smoke / manual validation completed
• Surface-specific checks completed

Docs and Governance

• I updated docs that changed with the behavior, contract, workflow, or rollout.
• I checked whether AGENTS.md needed an update.
• I checked whether a playbook or skill needed an update.
• I checked whether governance docs or review docs needed an update.

Updated docs:

• .tech-debt/localstorage-allowlist-budget.json — production: 15 → 14 + rationale.
• (docs/diagnostics/2026-05-03-web-deep-dive/00-overview.md round-8 summary lands in a separate docs PR after all 4 round-8 follow-ups merge, matching the round-7 sequence.)

Risk and Rollout

• User-visible risk: none. safe*LS helpers are call-compatible with the previous direct access on success; they additionally swallow throws on Safari Private Mode and quota-exceeded paths instead of crashing the hook on mount or mid-subscribe.
• Rollout / deploy order: standard web build; no env / migration / feature flag.
• Backout plan: revert this PR; the previous direct-localStorage shape is byte-equivalent on the happy path.

Hard Rule #15

• I read AGENTS.md before coding.
• Internal docs I touched are in Ukrainian.
• I did not use --no-verify.

Reviewer Notes

The two getItem paths feeding useState initializers were collapsed into a single safeReadStringLS(PUSH_SUB_KEY) === "1" lazy initializer — the prior try/catch returned false on any throw, which is exactly what safeReadStringLS's default fallback produces, so the observable behavior is identical.

---

Summary by cubic

Migrated usePushNotifications off raw localStorage to safeReadStringLS/safeWriteLS/safeRemoveLS for the hub_push_subscribed flag to prevent crashes in Safari Private Mode and quota-full cases. Removed the hook from the sergeant-design/no-raw-local-storage allowlist and reduced the production allowlist budget from 15 to 14; added 3 tests that stub Storage throws to lock in the behavior.

^{Written for commit 3840b3a. Summary will update on new commits.}

Summary by CodeRabbit

• Tests

  • Added test suite for push notifications hook to verify resilience when localStorage operations fail.

• Chores

  • Updated push notifications implementation to use safer localStorage access patterns.
  • Updated configuration files to reflect recent changes.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
sergeant	Ready	Preview, Comment	May 4, 2026 5:38pm

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f11ff6f-e3ba-412c-a0d8-a5ceb4f6d851

📥 Commits

Reviewing files that changed from the base of the PR and between ec1f382 and 3840b3a.

📒 Files selected for processing (4)

• .tech-debt/localstorage-allowlist-budget.json
• apps/web/src/shared/hooks/usePushNotifications.test.tsx
• apps/web/src/shared/hooks/usePushNotifications.ts
• eslint.config.js

---

📝 Walkthrough

Walkthrough

This PR migrates the usePushNotifications hook from direct localStorage access to safe storage helpers (safeReadStringLS, safeWriteLS, safeRemoveLS). The implementation is updated, tests verify error resilience, the ESLint allowlist is updated to enforce safe access going forward, and the localStorage budget count is decremented accordingly.

Changes

localStorage Hardening Migration for usePushNotifications

Layer / File(s)	Summary
Core Implementation apps/web/src/shared/hooks/usePushNotifications.ts	Replaced direct localStorage.getItem, setItem, and removeItem calls with safe helpers. Initialization, subscribe, and unsubscribe paths now use safeReadStringLS, safeWriteLS, and safeRemoveLS respectively.
Tests / Verification apps/web/src/shared/hooks/usePushNotifications.test.tsx	Added localStorage hardening test suite (101 lines) covering error resilience: hook tolerates getItem failures, subscribe() succeeds despite setItem quota errors, and unsubscribe() clears state even when removeItem throws security errors.
ESLint Enforcement eslint.config.js	Removed usePushNotifications.ts from the no-raw-local-storage ignore allowlist so direct storage access is now linted. Added migration documentation comments referencing the follow-up and round-tracking details.
Budget Tracking .tech-debt/localstorage-allowlist-budget.json	Decremented production allowlist budget from 15 to 14 and updated rationale to reference the completed 2026-05-04 migration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• chore(web): migrate modules raw-LS to safeReadLS/safeWriteLS #1350: Performs the same storage helper migration pattern and updates the no-raw-local-storage ESLint allowlist in parallel.
• refactor(web): migrate top-5 files off raw localStorage #1089: Related localStorage safe-access migration from raw localStorage to shared safe helpers with allowlist enforcement updates.
• ci(ci): gate localStorage allowlist growth via burndown budget script #1589: Related localStorage governance changes affecting allowlist budget and ESLint ignore list handling with CI gating.

Suggested labels

size/L

Poem

🐰 With whiskers twitching and paws so fine,
We wrap those storage calls in safe design,
No crashes here when errors appear,
Just gentle helpers, sturdy and clear!
One less item on the allowlist heap,
A migration complete—now we can sleep! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main change: migrating usePushNotifications away from raw localStorage by using safe helper functions, with specific context (Item #6 round 8) included.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch devin/1777904460-item6-localstorage-pushnotif

---

_{Review rate limit: 7/10 reviews remaining, refill in 17 minutes.}

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

[github-actions]

⏱️ CI Pipeline Duration Report

Based on the last 50 successful runs on the default branch.

Overall Pipeline

Metric	Value
p50	6m 26s
p95	7m 55s
p99	9m 3s
Current run	6m 15s
vs p95	-21.1%

Trend (last 20 runs): ▃▃▁▂▃▃▃▂▃▃▂▂▄▃▃▆▅▄█▆

Per-Job Breakdown

Job	p50	p95	p99	Current	vs p95
Accessibility (axe-core)	2m 5s	2m 21s	2m 23s	0s	-100.7%
Commit messages (commitlint)	0s	0s	0s	35s	N/A
Critical-flow E2E (Playwright)	1m 36s	1m 44s	1m 44s	5m 59s	+245.2%
Migration lint (AGENTS rule	0s	0s	0s	8s	N/A
Pipeline duration (p95 trend)	26s	27s	27s	—	—
Secret scan (gitleaks)	8s	11s	11s	7s	-36.4%
Smoke E2E (Playwright)	1m 26s	1m 40s	1m 40s	—	—
Test coverage (vitest)	2m 4s	2m 33s	2m 33s	2m 18s	-9.8%
Workflow lint (actionlint)	7s	7s	7s	6s	-14.3%
check	4m 12s	4m 54s	5m 6s	1m 2s	-78.9%
tsconfig strict guard (PR-1.A)	5s	14s	14s	4s	-71.4%