139 lines
7.3 KiB
Markdown
139 lines
7.3 KiB
Markdown
---
|
|
phase: quick-260611-tfc
|
|
plan: 01
|
|
type: execute
|
|
wave: 1
|
|
depends_on: []
|
|
files_modified:
|
|
- apps/pwa/src/sw.ts
|
|
autonomous: true
|
|
requirements: [WR-01]
|
|
|
|
must_haves:
|
|
truths:
|
|
- "When a notification is tapped and a focusable window exists, the SW focuses it and navigates it to the deep-link URL"
|
|
- "When client.navigate() resolves null (browser rejected navigation), the SW opens a new window at the deep-link URL"
|
|
- "When client.focus() rejects (window closed between matchAll and focus), the SW falls through and opens a new window at the deep-link URL"
|
|
- "When no focusable window exists, the existing openWindow fallback still fires"
|
|
artifacts:
|
|
- path: "apps/pwa/src/sw.ts"
|
|
provides: "notificationclick handler with reachable openWindow fallback on focus rejection and null navigate"
|
|
contains: ".catch("
|
|
key_links:
|
|
- from: "apps/pwa/src/sw.ts notificationclick handler"
|
|
to: "self.clients.openWindow"
|
|
via: "fallback on navigate()===null and on focus()/navigate() rejection"
|
|
pattern: "self\\.clients\\.openWindow"
|
|
---
|
|
|
|
<objective>
|
|
Fix WR-01 from `.planning/phases/13-real-lint-gate-eslint/13-REVIEW.md`: the `notificationclick`
|
|
handler in `apps/pwa/src/sw.ts` has an unreachable `openWindow` fallback. The current
|
|
`client.focus().then(() => client.navigate(url))` chain (1) silently drops the case where
|
|
`navigate()` resolves to `null` (browser rejected the navigation), and (2) lets a `focus()`
|
|
rejection propagate into the `event.waitUntil` chain — counting the click as handled so the
|
|
new-window fallback never runs. Net result: on the focusable-client path, a window that closed
|
|
between `matchAll` and `focus`, or a rejected navigation, leaves the user with neither
|
|
navigation nor a new window.
|
|
|
|
Purpose: Guarantee D-14 deep-link behaviour holds on every notification tap, including the
|
|
window-closed and navigation-rejected edge cases — important on iOS where push reliability is
|
|
load-bearing.
|
|
|
|
Output: Edited `apps/pwa/src/sw.ts` notificationclick handler; single atomic commit.
|
|
</objective>
|
|
|
|
<execution_context>
|
|
@$HOME/.claude/gsd-core/workflows/execute-plan.md
|
|
@$HOME/.claude/gsd-core/templates/summary.md
|
|
</execution_context>
|
|
|
|
<context>
|
|
@.planning/STATE.md
|
|
@apps/pwa/src/sw.ts
|
|
@.planning/phases/13-real-lint-gate-eslint/13-REVIEW.md
|
|
</context>
|
|
|
|
<tasks>
|
|
|
|
<task type="auto">
|
|
<name>Task 1: Make the openWindow fallback reachable in the notificationclick handler</name>
|
|
<files>apps/pwa/src/sw.ts</files>
|
|
<action>
|
|
First Read `apps/pwa/src/sw.ts` to confirm the current notificationclick handler text
|
|
(the `event.waitUntil(self.clients.matchAll(...).then((clientList) => { for (const client
|
|
of clientList) { if ('focus' in client) { return client.focus().then(() =>
|
|
client.navigate(url)); } } ... })` block, around lines 167-182). Match the real variable
|
|
names and the existing `'focus' in client` guard exactly — do not rename anything.
|
|
|
|
Apply the WR-01 fix per `.planning/phases/13-real-lint-gate-eslint/13-REVIEW.md`. Inside the
|
|
`for (const client of clientList)` loop, replace the focusable-client `return` so the
|
|
chain (a) checks the `navigate()` result and opens a new window when it resolves `null`,
|
|
and (b) carries a `.catch()` that opens a new window when `focus()` or `navigate()` rejects.
|
|
Both new-window branches must be guarded by `if (self.clients.openWindow)` and pass the same
|
|
`url`. The chain stays a single `return`ed promise so it is correctly returned (not floating)
|
|
inside the `.then((clientList) => ...)` callback. The reviewer's exact replacement is the
|
|
canonical shape (`client.focus().then(() => client.navigate(url)).then((navigated) => { if
|
|
(navigated === null && self.clients.openWindow) return self.clients.openWindow(url); })
|
|
.catch(() => { if (self.clients.openWindow) return self.clients.openWindow(url); })`).
|
|
|
|
Preserve ALL other behaviour: the `event.notification.close()` call, the `url` extraction +
|
|
its `typeof` guard and existing `eslint-disable-next-line` comments (lines ~160-165), the
|
|
`matchAll({ type: 'window', includeUncontrolled: true })` options, and the final
|
|
`if (self.clients.openWindow) { return self.clients.openWindow(url); }` no-focusable-client
|
|
fallback after the loop. Touch only the focusable-client `return` expression.
|
|
|
|
Scope discipline: do NOT address IN-01 (eslint.config.js spike.ts) or IN-02 (redundant
|
|
`as string` cast) from the same review — out of scope for WR-01. Do NOT run a repo-wide
|
|
Prettier pass. If the edited lines need formatting, run
|
|
`pnpm exec prettier --write apps/pwa/src/sw.ts` on that single file only and confirm the
|
|
diff is limited to the handler.
|
|
|
|
The new code must satisfy the Phase 13 type-aware ESLint gate: the returned promise chain is
|
|
correctly returned (no `@typescript-eslint/no-floating-promises`), and no
|
|
`@typescript-eslint/no-misused-promises` is introduced. `client.focus()`, `client.navigate()`,
|
|
and `self.clients.openWindow()` are all typed by the WebWorker/ServiceWorker lib already
|
|
referenced via `/// <reference lib="webworker" />` and `declare const self:
|
|
ServiceWorkerGlobalScope;` at the top of the file — no new casts or eslint-disable comments
|
|
should be required for the fix.
|
|
</action>
|
|
<verify>
|
|
<automated>cd /home/luc/Projects/familysync && pnpm lint && pnpm typecheck && pnpm --filter @familysync/pwa test && pnpm --filter @familysync/pwa build && grep -q "self.clients.openWindow" apps/pwa/src/sw.ts && grep -q "\.catch(" apps/pwa/src/sw.ts</automated>
|
|
</verify>
|
|
<done>
|
|
- `pnpm lint` exits 0 — no new no-floating-promises / no-misused-promises violations from the
|
|
edited chain (the Phase 13 type-aware ESLint gate stays green).
|
|
- `pnpm typecheck` exits 0 — sw.ts type-checks against the WebWorker/ServiceWorker lib.
|
|
- `pnpm --filter @familysync/pwa test` passes — the existing PWA vitest suite stays green
|
|
(no `sw.test.ts` exercises notificationclick; the SW runs only in the browser/build, so no
|
|
new unit test is added — verification is lint + typecheck + build + suite-green).
|
|
- `pnpm --filter @familysync/pwa build` succeeds — `tsc && vite build` emits `sw.js` via
|
|
injectManifest with the fixed handler.
|
|
- The notificationclick handler's focusable-client path returns a promise that: navigates on
|
|
success, opens a new window when `navigate()` resolves `null`, and opens a new window when
|
|
`focus()`/`navigate()` rejects — all guarded by `self.clients.openWindow`. The post-loop
|
|
no-focusable-client fallback and all surrounding behaviour are unchanged.
|
|
</done>
|
|
</task>
|
|
|
|
</tasks>
|
|
|
|
<verification>
|
|
- `pnpm lint` exits 0 (type-aware ESLint gate from Phase 13).
|
|
- `pnpm typecheck` exits 0.
|
|
- `pnpm --filter @familysync/pwa test` passes.
|
|
- `pnpm --filter @familysync/pwa build` succeeds (emits sw.js).
|
|
- `git diff --stat` shows only `apps/pwa/src/sw.ts` changed.
|
|
</verification>
|
|
|
|
<success_criteria>
|
|
The `notificationclick` openWindow fallback is reachable on both failure modes (focus rejection
|
|
and null navigate), the deep-link happy path is preserved, all four gate commands pass, and the
|
|
change lands as a single atomic commit on branch `gsd/phase-13-real-lint-gate-eslint` touching
|
|
only `apps/pwa/src/sw.ts`.
|
|
</success_criteria>
|
|
|
|
<output>
|
|
Create `.planning/quick/260611-tfc-fix-wr-01-sw-ts-notificationclick-openwi/260611-tfc-SUMMARY.md` when done.
|
|
</output>
|