diff --git a/.planning/STATE.md b/.planning/STATE.md index c09b3a8..c8115c7 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -189,6 +189,7 @@ Recent decisions affecting current work: | 260610-k1z | Persist OIDC session cookie (AUTH-02) — @hono/oidc-auth 1.8.3 sets a session-scoped `oidc-auth` cookie (no maxAge) so it died on PWA/browser close → re-login almost every return (both devices). Added persistSessionCookie middleware re-issuing the cookie with maxAge(=OIDC_AUTH_EXPIRES)+SameSite=Lax, ONLY when a valid session exists (no resurrection guard). NOT an Authelia/refresh issue. 14 auth tests pass | 2026-06-10 | 8343fad | Verified | [260610-k1z-persist-oidc-session-cookie-with-maxage-](./quick/260610-k1z-persist-oidc-session-cookie-with-maxage-/) | | 260610-ka9 | Fix silent Android push (Phase 5 UAT Test 4) — SW showNotification had only {body,tag,data} → Android Chromium/Edge showed them silently. Added icon/badge/renotify:true/vibrate; generalized re-enable instructions to Chrome-or-Edge. iOS unaffected. Build emits sw.js with renotify; 187 pwa tests pass | 2026-06-10 | c864fc4 | Verified | [260610-ka9-fix-silent-android-push-notifications-en](./quick/260610-ka9-fix-silent-android-push-notifications-en/) | | 260611-ozt | Split publish job into standalone .gitea/workflows/publish.yml (on: push→main only, no redundant event-guard if:; MILESTONE env moved with it) and strip it + the push trigger from ci.yml — kills the orphaned `CI / publish (pull_request)` pending status (phase-8 code-review WR-01). name:CI + fast-checks/api/harness job ids held stable so the required branch-protection contexts stay valid. Documented the release model in README "Publishing / Releases" + publish.yml header. Both YAML validated (yq) | 2026-06-11 | 92353e1 | | [260611-ozt-split-publish-job-into-standalone-gitea-](./quick/260611-ozt-split-publish-job-into-standalone-gitea-/) | +| 260611-tfc | Fix WR-01 (13-REVIEW): apps/pwa/src/sw.ts notificationclick openWindow fallback was unreachable when client.focus() rejects (window closed between matchAll/focus) or client.navigate() resolves null — chained a navigate-result check + a .catch, both falling through to self.clients.openWindow(url). lint/format:check/typecheck green, build emits sw.js, 191/191 pwa tests | 2026-06-12 | af78ccc | Verified | [260611-tfc-fix-wr-01-sw-ts-notificationclick-openwi](./quick/260611-tfc-fix-wr-01-sw-ts-notificationclick-openwi/) | ## Deferred Items diff --git a/.planning/quick/260611-tfc-fix-wr-01-sw-ts-notificationclick-openwi/260611-tfc-PLAN.md b/.planning/quick/260611-tfc-fix-wr-01-sw-ts-notificationclick-openwi/260611-tfc-PLAN.md new file mode 100644 index 0000000..09e357d --- /dev/null +++ b/.planning/quick/260611-tfc-fix-wr-01-sw-ts-notificationclick-openwi/260611-tfc-PLAN.md @@ -0,0 +1,138 @@ +--- +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" +--- + + +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. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/STATE.md +@apps/pwa/src/sw.ts +@.planning/phases/13-real-lint-gate-eslint/13-REVIEW.md + + + + + + Task 1: Make the openWindow fallback reachable in the notificationclick handler + apps/pwa/src/sw.ts + + 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 `/// ` and `declare const self: + ServiceWorkerGlobalScope;` at the top of the file — no new casts or eslint-disable comments + should be required for the fix. + + + 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 + + + - `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. + + + + + + +- `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. + + + +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`. + + + +Create `.planning/quick/260611-tfc-fix-wr-01-sw-ts-notificationclick-openwi/260611-tfc-SUMMARY.md` when done. + diff --git a/.planning/quick/260611-tfc-fix-wr-01-sw-ts-notificationclick-openwi/260611-tfc-SUMMARY.md b/.planning/quick/260611-tfc-fix-wr-01-sw-ts-notificationclick-openwi/260611-tfc-SUMMARY.md new file mode 100644 index 0000000..7daf14e --- /dev/null +++ b/.planning/quick/260611-tfc-fix-wr-01-sw-ts-notificationclick-openwi/260611-tfc-SUMMARY.md @@ -0,0 +1,77 @@ +--- +phase: quick-260611-tfc +plan: "01" +subsystem: pwa-sw +tags: [service-worker, push-notifications, bug-fix, wr-01] +dependency_graph: + requires: [] + provides: [reachable-openWindow-fallback-notificationclick] + affects: [apps/pwa/src/sw.ts] +tech_stack: + added: [] + patterns: [promise-chain-catch-fallback] +key_files: + created: [] + modified: + - apps/pwa/src/sw.ts +decisions: + - "Canonical fix shape: .then(navigated => openWindow if null) + .catch(() => openWindow) on the focus/navigate chain — keeps the returned promise intact for no-floating-promises" +metrics: + duration: ~6 min + completed: "2026-06-12" +--- + +# Quick Task 260611-tfc: Fix WR-01 sw.ts notificationclick openWindow Fallback + +**One-liner:** notificationclick handler extended with `.then(navigated===null → openWindow)` + `.catch(→ openWindow)` so the new-window fallback is reachable on both focus rejection and null navigate. + +## What Was Done + +Fixed WR-01 from `.planning/phases/13-real-lint-gate-eslint/13-REVIEW.md`. + +The focusable-client `return` inside the `for (const client of clientList)` loop was: + +```ts +return client.focus().then(() => client.navigate(url)); +``` + +Two failure modes made the `openWindow` fallback unreachable: +1. `navigate()` resolving `null` (browser rejected the navigation) — the chain resolved successfully, so the post-loop fallback was never reached. +2. `focus()` or `navigate()` rejecting (window closed between `matchAll` and `focus`) — the rejection propagated into `event.waitUntil`, marking the click as handled without opening a new window. + +Applied the canonical replacement per the review: + +```ts +return 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); + }); +``` + +All surrounding behaviour preserved: `event.notification.close()`, `url` extraction + eslint-disable comments, `matchAll` options, and the post-loop no-focusable-client fallback. + +## Verification — Exit Codes + +| Command | Exit | +|---------|------| +| `pnpm lint` | 0 | +| `pnpm typecheck` | 0 | +| `pnpm --filter @familysync/pwa test` | 0 (191/191 pass) | +| `pnpm --filter @familysync/pwa build` | 0 (dist/sw.js emitted) | + +## Deviations from Plan + +None — plan executed exactly as written. + +## Self-Check: PASSED + +- `apps/pwa/src/sw.ts` modified: confirmed +- Commit `af78ccc` exists: confirmed +- `git diff --diff-filter=D HEAD~1 HEAD`: no deletions +- Only `apps/pwa/src/sw.ts` changed (`git diff --stat` = 1 file, 10 insertions, 1 deletion)