refactor(web): migrate weeklyDigestStorage off raw localStorage

[Skords-01]

Summary

Item #6 round-9 follow-up. The web-side StorageReader adapter in apps/web/src/shared/lib/storage/weeklyDigestStorage.ts was the last remaining higher-level wrapper still calling localStorage.getItem inside its own inline try/catch. Replace that with a delegation to the shared safeReadStringLS helper from ./storage so the file no longer needs to live in the sergeant-design/no-raw-local-storage allowlist.

• apps/web/src/shared/lib/storage/weeklyDigestStorage.ts — webStorageReader.getItem now calls safeReadStringLS(key). Same try/catch contract (returns null on failure), one fewer file with raw localStorage access.
• eslint.config.js — drop weeklyDigestStorage.ts from the web no-raw-local-storage allowlist; extend the round-9 migration comment.
• .tech-debt/localstorage-allowlist-budget.json — production budget 14 → 13 (headroom 0). Updated rationale.
• apps/web/src/shared/lib/storage/weeklyDigestStorage.test.ts — keep the existing 3 happy-path scenarios and add 2 hardening cases that mock Storage.prototype.getItem to throw (Safari Private Mode / disabled storage), asserting loadDigest and hasLiveWeeklyDigest collapse to null/false instead of throwing.

Roadmap: docs/diagnostics/2026-05-03-web-deep-dive/00-overview.md Item #6 follow-up (round 9). Burndown plan: docs/diagnostics/2026-05-03-web-deep-dive/02-architecture-and-state.md §2.2 + docs/tech-debt/frontend.md §2.

Governing Skill

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

Playbook

• Primary playbook: n/a (rolling Item Claude/review project structure l2 ke1 #6 burndown — no dedicated playbook)
• Why this playbook: n/a
• If no playbook matched, why: Item Claude/review project structure l2 ke1 #6 burndown is a single-file localStorage migration that consistently follows the safeRead*LS/safeWriteLS pattern documented in docs/tech-debt/frontend.md §2 and the diagnostic; previous rounds (5–8) shipped without a dedicated playbook for the same reason.

Verification

pnpm --filter @sergeant/db-schema build
pnpm --filter @sergeant/web exec eslint apps/web/src/shared/lib/storage/weeklyDigestStorage.ts
pnpm --filter @sergeant/web exec tsc -p tsconfig.json --noEmit
pnpm --filter @sergeant/web exec vitest run src/shared/lib/storage/weeklyDigestStorage.test.ts
pnpm lint:localstorage-allowlist

Result:

• ESLint: 0 errors / 0 warnings.
• tsc --noEmit: clean.
• Vitest: Test Files 1 passed (1) | Tests 5 passed (5).
• lint:localstorage-allowlist: 13/13 (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:

• The diagnostic overview round-9 summary entry will be added in a follow-up docs PR alongside the other three round-9 items (Storybook expansion, finyk-domain noUncheckedIndexedAccess, WaitlistForm useApiForm migration), to keep that single roadmap-touching commit isolated.

Risk and Rollout

• User-visible risk: zero. The new webStorageReader.getItem returns the exact same shape (string | null) and silently catches failures the same way the previous implementation did; @sergeant/shared loadDigest/hasLiveWeeklyDigest are unchanged.
• Rollout / deploy order: Vercel auto-deploys on merge; no migration / env change.
• Backout plan: revert this single commit. The allowlist entry can be re-added if needed without server-side coordination.

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 new tests stub Storage.prototype.getItem directly via Object.defineProperty and restore the descriptor in afterEach; this matches the pattern used elsewhere in the repo for Safari Private Mode coverage and keeps the existing happy-path tests untouched.

---

Summary by cubic

Migrated the web weeklyDigestStorage adapter off raw localStorage to use safeReadStringLS, keeping the same null-on-failure behavior and improving resilience. Removed the file from the sergeant-design/no-raw-local-storage allowlist and lowered the production budget to 13.

• Refactors
  • apps/web/src/shared/lib/storage/weeklyDigestStorage.ts: webStorageReader.getItem now calls safeReadStringLS(key); same string | null contract.
  • apps/web/src/shared/lib/storage/weeklyDigestStorage.test.ts: added two resilience tests where Storage.getItem throws; loadDigest returns null, hasLiveWeeklyDigest returns false.
  • eslint.config.js: dropped weeklyDigestStorage.ts from the allowlist and documented the round-9 change.
  • .tech-debt/localstorage-allowlist-budget.json: updated rationale; production count 14 → 13.

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

Summary by CodeRabbit

• Bug Fixes

  • Enhanced resilience of weekly digest functionality with improved error handling for scenarios where local storage is unavailable or inaccessible, including Safari Private Mode and disabled storage configurations.

• Chores

  • Reduced technical debt through migration of storage access patterns and corresponding configuration updates.

[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 8:26pm

[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: 1e552319-70e8-4fc5-913b-b862bb65fce5

📥 Commits

Reviewing files that changed from the base of the PR and between 6591813 and abce050.

📒 Files selected for processing (4)

• .tech-debt/localstorage-allowlist-budget.json
• apps/web/src/shared/lib/storage/weeklyDigestStorage.test.ts
• apps/web/src/shared/lib/storage/weeklyDigestStorage.ts
• eslint.config.js

---

📝 Walkthrough

Walkthrough

The PR migrates weeklyDigestStorage.ts from direct localStorage.getItem access to the safeReadStringLS safe wrapper, adds resilience tests for storage access errors, removes its ESLint allowlist exemption, and decrements the tech-debt budget from 14 to 13.

Changes

Weekly Digest Storage Safe Wrapper Migration

Layer / File(s)	Summary
Core Implementation apps/web/src/shared/lib/storage/weeklyDigestStorage.ts	webStorageReader.getItem now delegates to safeReadStringLS(key) instead of inline try/catch on localStorage.getItem, collapsing access failures to null.
Test Coverage apps/web/src/shared/lib/storage/weeklyDigestStorage.test.ts	New resilience test block stubs Storage.prototype.getItem to throw DOMException and generic errors, verifying loadDigest and hasLiveWeeklyDigest return safe defaults without throwing.
Linting & Config eslint.config.js	Removed weeklyDigestStorage.ts from the no-raw-local-storage allowlist; added comment noting migration to safeReadStringLS.
Tech Debt Tracking .tech-debt/localstorage-allowlist-budget.json	Decremented production budget from 14 to 13; updated rationale to reference this round-9 migration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Skords-01/Sergeant#1344: Same migration pattern—replacing localStorage.getItem with safeReadStringLS and adjusting ESLint allowlists.
• Skords-01/Sergeant#1350: Migrates web modules away from direct localStorage to safe helpers and updates tech-debt tracking.
• Skords-01/Sergeant#1589: Modifies the same budget JSON and allowlist exemptions for storage migrations.

Suggested labels

size/M

Poem

🐰 A rabbit hops through storage code,
No more raw reads on the road,
Safe wrappers catch the error's croak,
Tech debt fades like morning smoke,
One less exemption—let's revoke!

🚥 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 pull request title clearly and accurately summarizes the primary change: migrating weeklyDigestStorage to use safeReadStringLS instead of raw localStorage access.
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/1777925774-item6-weekly-digest-storage-ls

---

_{Review rate limit: 1/10 review remaining, refill in 48 minutes and 38 seconds.}

_{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	66m 54s
vs p95	+745.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	46s	N/A
Critical-flow E2E (Playwright)	1m 36s	1m 44s	1m 44s	6m 16s	+261.5%
Migration lint (AGENTS rule	0s	0s	0s	10s	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 8s	-16.3%
Workflow lint (actionlint)	7s	7s	7s	6s	-14.3%
check	4m 12s	4m 54s	5m 6s	57s	-80.6%
tsconfig strict guard (PR-1.A)	5s	14s	14s	6s	-57.1%

⚠️ Warning: Current run (66m 54s) exceeds p95 + 20% threshold (9m 30s). Consider reviewing slow jobs.