From fce8e046ea70ad98432d32de0d94d284c7d3a8b7 Mon Sep 17 00:00:00 2001 From: DongHyeonka Date: Fri, 14 Aug 2026 00:34:09 +0900 Subject: [PATCH] fix: bind Web Push mutations to exact authority WP-01: a CAS receipt is only evidence when it names the expected key and the exact next revision. Write and remove now share one validator, so a stale or arbitrary repository receipt can no longer be packaged as a confirmed control. WP-05: a pre-aborted command records the operation the caller requested instead of always reporting an inspection. WP-06: bounded fan-out is reported honestly. The subscriptionchange client handoff and the notification cleanup both emit countBucket and truncated, and an incomplete cleanup returns { complete: false } and is observed as DEGRADED separately from revoke authority. WP-07: the user-visible native notification effect is tracked through NOT_APPLIED, MAYBE_APPLIED and CONFIRMED phases and surfaced as observation evidence, never as retry authorization. WP-02, WP-03 and WP-04 stay open: they need the V2 wire protocol with server request-shape negotiation, which belongs to the versioned-migration task rather than this correctness pass. The ledger records them as DEFERRED_TO_MIGRATION. Web Push remains NOT_SELECTED and AVAILABLE_NOT_COMPOSED. Co-Authored-By: Claude Opus 5 (1M context) --- docs/operations/adapter-remediation-ledger.md | 14 ++++---- .../web-push/inbound/push-event-adapter.ts | 31 ++++++++++++++---- .../web-push/push-association-fence-store.ts | 20 ++++++++---- .../web-push/push-subscription-adapter.ts | 30 ++++++++++++++--- .../web-push/service-worker-runtime.ts | 19 +++++++++-- src/contracts/web-push.ts | 31 ++++++++++++++++++ tests/unit/service-worker-runtime.test.ts | 32 ++++++++----------- tests/unit/web-push-fence-store.test.ts | 31 ++++++++++++++++++ tests/unit/web-push-worker-runtime.test.ts | 6 ++++ 9 files changed, 170 insertions(+), 44 deletions(-) diff --git a/docs/operations/adapter-remediation-ledger.md b/docs/operations/adapter-remediation-ledger.md index c483c5e..99be2dc 100644 --- a/docs/operations/adapter-remediation-ledger.md +++ b/docs/operations/adapter-remediation-ledger.md @@ -149,13 +149,13 @@ Rollout state starts at `NOT_STARTED`; documented-unimplemented items start at | SW-08 | Composed when capability selected | `corepack pnpm exec vitest run tests/unit/service-worker-runtime.test.ts` | — | `NOT_STARTED` | per-client failure escalation | — | | SW-09 | Composed when capability selected | `corepack pnpm exec vitest run tests/unit/service-worker-runtime.test.ts` | — | `NOT_STARTED` | late install work observed | — | | SW-10 | Protocol V2 migration | `corepack pnpm exec vitest run tests/unit/service-worker-runtime.test.ts` | — | `NOT_STARTED` | V1/V2 mismatch fail-close | — | -| WP-01 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-fence-store.test.ts` | — | `NOT_STARTED` | CAS receipt rejection | — | -| WP-02 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-fence-store.test.ts` | — | `NOT_STARTED` | `RECONCILIATION_REQUIRED` backlog | — | -| WP-03 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-subscription-adapter.test.ts` | — | `NOT_STARTED` | backend receipt mismatch | — | -| WP-04 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-subscription-adapter.test.ts` | — | `NOT_STARTED` | reconcile loop | — | -| WP-05 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-subscription-adapter.test.ts` | — | `NOT_STARTED` | pre-abort observation drift | — | -| WP-06 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-worker-runtime.test.ts` | — | `NOT_STARTED` | truncation reported degraded | — | -| WP-07 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-worker-runtime.test.ts` | — | `NOT_STARTED` | late native effect certainty | — | +| WP-01 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-fence-store.test.ts` | `fix: bind Web Push mutations to exact authority` | `FIXED_NOT_RELEASED` | CAS receipt rejection | Red stale/skipped/huge revision receipts → green; write and remove share one exact-next-revision validator | +| WP-02 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-fence-store.test.ts` | — | `DEFERRED_TO_MIGRATION` | `RECONCILIATION_REQUIRED` backlog | Deferred to the Task 16 versioned-migration PR: `MUTATION_OUTCOME_UNKNOWN` and the `RECONCILIATION_REQUIRED` lifecycle are part of the same wire/data migration as WP-03. | +| WP-03 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-subscription-adapter.test.ts` | — | `DEFERRED_TO_MIGRATION` | backend receipt mismatch | Deferred to the Task 16 versioned-migration PR: the V2 receipt requires server request-shape negotiation before a client rollout. | +| WP-04 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-subscription-adapter.test.ts` | — | `DEFERRED_TO_MIGRATION` | reconcile loop | Deferred with WP-03: `expectedPreviousAssociationEpoch` and `replacedAssociationEpoch` are part of the V2 register contract. | +| WP-05 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-subscription-adapter.test.ts` | `fix: bind Web Push mutations to exact authority` | `FIXED_NOT_RELEASED` | pre-abort observation drift | A pre-aborted command records the requested operation instead of always INSPECT | +| WP-06 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-worker-runtime.test.ts` | `fix: bind Web Push mutations to exact authority` | `FIXED_NOT_RELEASED` | truncation reported degraded | Client handoff and notification cleanup report `countBucket` and `truncated`; an incomplete cleanup returns `{ complete: false }` and is DEGRADED | +| WP-07 | Web Push `NOT_SELECTED` | `corepack pnpm exec vitest run tests/unit/web-push-worker-runtime.test.ts` | `fix: bind Web Push mutations to exact authority` | `FIXED_NOT_RELEASED` | late native effect certainty | Native notification effect is tracked as NOT_APPLIED → MAYBE_APPLIED → CONFIRMED and observed as evidence only | ## Rules for updating this ledger diff --git a/src/adapters/web-push/inbound/push-event-adapter.ts b/src/adapters/web-push/inbound/push-event-adapter.ts index 40ca39f..9b15f5b 100644 --- a/src/adapters/web-push/inbound/push-event-adapter.ts +++ b/src/adapters/web-push/inbound/push-event-adapter.ts @@ -1,4 +1,5 @@ import { + type WebPushNativeEffectCertainty, WEB_PUSH_LIMITS, webPushFailure, webPushSuccess, @@ -148,14 +149,30 @@ export function createPushEventAdapter(dependencies: Readonly<{ ); if (!finalFence.ok) return finalFence; const clickData = clickDataFromHint(decoded.value); + // WP-07. The user-visible effect has its own certainty phase: NOT_APPLIED + // before the native call, MAYBE_APPLIED while the promise is pending and + // CONFIRMED on fulfilment. It is evidence only and never authorizes retry. + let nativeEffect: WebPushNativeEffectCertainty = "NOT_APPLIED"; try { - await dependencies.notifications.showNotification(definition.title, { - body: definition.body, - data: clickData, - requireInteraction: false, - tag, - }); + const shown = dependencies.notifications.showNotification( + definition.title, + { + body: definition.body, + data: clickData, + requireInteraction: false, + tag, + }, + ); + nativeEffect = "MAYBE_APPLIED"; + await shown; + nativeEffect = "CONFIRMED"; if (signal.aborted) { + observeWebPush(dependencies.observer, { + event: "web_push_notification_finished", + outcome: "DEGRADED", + reason: "ABORTED", + nativeEffect, + }); return webPushFailure("ABORTED", "NOTIFICATION_SHOW"); } } catch { @@ -164,12 +181,14 @@ export function createPushEventAdapter(dependencies: Readonly<{ event: "web_push_notification_finished", outcome: "FAILED", reason: failureCode(failed), + nativeEffect, }); return failed; } observeWebPush(dependencies.observer, { event: "web_push_notification_finished", outcome: "SUCCEEDED", + nativeEffect, }); return webPushSuccess(undefined); } diff --git a/src/adapters/web-push/push-association-fence-store.ts b/src/adapters/web-push/push-association-fence-store.ts index c19af32..e5b7d5c 100644 --- a/src/adapters/web-push/push-association-fence-store.ts +++ b/src/adapters/web-push/push-association-fence-store.ts @@ -502,7 +502,7 @@ export function createPushAssociationFenceStore( operation, ); if (!written.ok) return written; - if (!validWriteReceipt(written.value)) { + if (!validWriteReceipt(written.value, expectedRevision)) { return webPushFailure("CONTROL_CORRUPT", operation); } return webPushSuccess( @@ -537,10 +537,7 @@ export function createPushAssociationFenceStore( "CONTROL_PURGE", ); if (!removed.ok) return removed; - if ( - !validWriteReceipt(removed.value) || - removed.value.revision !== expectedRevision + 1 - ) { + if (!validWriteReceipt(removed.value, expectedRevision)) { return webPushFailure("CONTROL_CORRUPT", "CONTROL_PURGE"); } return webPushSuccess(undefined); @@ -693,14 +690,25 @@ function validRepository( ); } +/** + * WP-01. One validator for both write and remove. + * + * A CAS receipt is only evidence when it names the expected key and the exact + * next revision. Accepting any well-typed revision let a stale or arbitrary + * repository receipt be packaged as a confirmed control, after which the whole + * CAS authority is wrong. A replayed receipt must still carry that exact + * revision, since replay means "this command already produced this revision". + */ function validWriteReceipt( value: PushControlWriteReceipt, + expectedRevision: number | null, ): value is PushControlWriteReceipt { return ( Boolean(value) && value.key === CONTROL_KEY && validRevision(value.revision) && - typeof value.replayed === "boolean" + typeof value.replayed === "boolean" && + value.revision === (expectedRevision ?? 0) + 1 ); } diff --git a/src/adapters/web-push/push-subscription-adapter.ts b/src/adapters/web-push/push-subscription-adapter.ts index 33c6320..adda927 100644 --- a/src/adapters/web-push/push-subscription-adapter.ts +++ b/src/adapters/web-push/push-subscription-adapter.ts @@ -1,5 +1,6 @@ import type { WebPushControlPort } from "../../application/ports/out/web-push-control.ts"; import { + webPushCountBucket, WEB_PUSH_LIMITS, samePushAuthority, webPushFailure, @@ -474,7 +475,9 @@ export function createWebPushSubscriptionAdapter( if (closed) return webPushSuccess(unavailable("CLOSED")); if (busy) return webPushSuccess(unavailable("BUSY")); if (signal?.aborted) { - return webPushFailure("ABORTED", "SUBSCRIPTION_INSPECT"); + // WP-05. A pre-aborted command is recorded as the operation the caller + // actually requested, not always as an inspection. + return webPushFailure("ABORTED", failureOperation); } busy = true; const generation = lifecycleGeneration; @@ -879,7 +882,9 @@ export function createWebPushSubscriptionAdapter( await Promise.allSettled([ unsubscribe, associationEpoch === null - ? Promise.resolve(webPushSuccess(undefined)) + ? Promise.resolve( + webPushSuccess(Object.freeze({ complete: true })), + ) : closeOwnedNotifications( associationEpoch, signal, @@ -891,7 +896,8 @@ export function createWebPushSubscriptionAdapter( nativeResult.value; const notificationsClean = notificationResult.status === "fulfilled" && - notificationResult.value.ok; + notificationResult.value.ok && + notificationResult.value.value.complete; return webPushSuccess( nativeClean && notificationsClean, ); @@ -905,11 +911,16 @@ export function createWebPushSubscriptionAdapter( return cleanup.ok && cleanup.value; } + /** + * WP-06. Notification cleanup is bounded best effort and is reported + * separately from revoke authority: an incomplete pass returns + * `{ complete: false }` and is observed as DEGRADED rather than success. + */ async function closeOwnedNotifications( associationEpoch: string, signal: AbortSignal | undefined, generation: number, - ): Promise> { + ): Promise>> { let notifications: readonly OwnedNotificationFacade[]; try { notifications = await dependencies.registration.getNotifications(); @@ -923,6 +934,15 @@ export function createWebPushSubscriptionAdapter( if (stale(signal, generation)) { return webPushFailure("ABORTED", "NOTIFICATION_CLEANUP"); } + const truncated = + notifications.length > WEB_PUSH_LIMITS.notificationCleanupCount; + observeWebPush(dependencies.observer, { + event: "web_push_notification_finished", + outcome: truncated ? "DEGRADED" : "SUCCEEDED", + ...(truncated ? { reason: "LIMIT_EXCEEDED" as const } : {}), + countBucket: webPushCountBucket(notifications.length), + truncated, + }); for (const notification of notifications.slice( 0, WEB_PUSH_LIMITS.notificationCleanupCount, @@ -941,7 +961,7 @@ export function createWebPushSubscriptionAdapter( } } } - return webPushSuccess(undefined); + return webPushSuccess(Object.freeze({ complete: !truncated })); } function stale( diff --git a/src/adapters/web-push/service-worker-runtime.ts b/src/adapters/web-push/service-worker-runtime.ts index d7cac3d..88b0a73 100644 --- a/src/adapters/web-push/service-worker-runtime.ts +++ b/src/adapters/web-push/service-worker-runtime.ts @@ -1,4 +1,5 @@ import { + webPushCountBucket, WEB_PUSH_LIMITS, WEB_PUSH_PROTOCOLS, webPushFailure, @@ -134,6 +135,10 @@ export function createWebPushServiceWorkerRuntime( const facade = functionalEventFacade(event); if (!facade) return; const taskControl = createLinkedAbortController(lifecycle.signal); + // WP-06. Bounded fan-out is policy, but the operator must be able to see + // that only part of the client set was notified. + let observedClientCount = 0; + let truncatedClients = false; const processing = withAbortableDeadline( async (signal) => { let clients: readonly unknown[]; @@ -148,6 +153,9 @@ export function createWebPushServiceWorkerRuntime( if (signal.aborted) { return webPushFailure("ABORTED", "SUBSCRIPTION_RECONCILE"); } + observedClientCount = clients.length; + truncatedClients = + clients.length > WEB_PUSH_LIMITS.clientHandoffCount; try { for (const candidate of clients.slice( 0, @@ -181,8 +189,15 @@ export function createWebPushServiceWorkerRuntime( const lifetime = processing.then((result) => { observeWebPush(dependencies.observer, { event: "web_push_subscription_rotated", - outcome: result.ok ? "SUCCEEDED" : "DEGRADED", - ...(result.ok ? {} : { reason: result.error.code }), + outcome: + result.ok && !truncatedClients ? "SUCCEEDED" : "DEGRADED", + ...(result.ok + ? truncatedClients + ? { reason: "LIMIT_EXCEEDED" as const } + : {} + : { reason: result.error.code }), + countBucket: webPushCountBucket(observedClientCount), + truncated: truncatedClients, }); }); try { diff --git a/src/contracts/web-push.ts b/src/contracts/web-push.ts index 3badc39..88b94e4 100644 --- a/src/contracts/web-push.ts +++ b/src/contracts/web-push.ts @@ -161,10 +161,41 @@ export type WebPushObservationEvent = | "web_push_click_dispatched" | "web_push_association_revoked"; +/** + * WP-06. Bounded fan-out is a deliberate policy, but reporting a truncated pass + * as plain success hid the fact that only part of the set was handled. + */ +export type WebPushCountBucket = + | "0" + | "1_8" + | "9_32" + | "33_64" + | "GT_64"; + +export function webPushCountBucket(count: number): WebPushCountBucket { + if (!Number.isFinite(count) || count <= 0) return "0"; + if (count <= 8) return "1_8"; + if (count <= 32) return "9_32"; + if (count <= 64) return "33_64"; + return "GT_64"; +} + +/** + * WP-07. Certainty of a user-visible native effect. It is evidence only and + * never authorizes a retry. + */ +export type WebPushNativeEffectCertainty = + | "CONFIRMED" + | "NOT_APPLIED" + | "MAYBE_APPLIED"; + export type WebPushObservation = Readonly<{ event: WebPushObservationEvent; outcome: "SUCCEEDED" | "FAILED" | "DEGRADED"; reason?: WebPushFailureCode | WebPushUnavailableReason; + countBucket?: WebPushCountBucket; + truncated?: boolean; + nativeEffect?: WebPushNativeEffectCertainty; }>; export interface WebPushObserver { diff --git a/tests/unit/service-worker-runtime.test.ts b/tests/unit/service-worker-runtime.test.ts index 04bd7aa..6175dbf 100644 --- a/tests/unit/service-worker-runtime.test.ts +++ b/tests/unit/service-worker-runtime.test.ts @@ -193,20 +193,13 @@ describe("service worker static cache authority", () => { const previousMatch = vi.fn( async () => new Response("previous", { status: 200 }), ); + const deleteEntry = async (): Promise => true; + const emptyMatch = async (): Promise => undefined; + const previousCache = { match: previousMatch, delete: deleteEntry }; + const emptyCache = { match: emptyMatch, delete: deleteEntry }; const runtime = staticRuntime({ - open: vi.fn( - async ( - name: string, - ): Promise> => - name === previousCacheName - ? { - match: previousMatch, - delete: async (): Promise => true, - } - : { - match: async (): Promise => undefined, - delete: async (): Promise => true, - }, + open: vi.fn(async (name: string) => + name === previousCacheName ? previousCache : emptyCache, ), keys: vi.fn(async () => [currentCacheName, previousCacheName]), delete: vi.fn(async () => true), @@ -226,13 +219,16 @@ describe("service worker static cache authority", () => { it("deletes an invalid hit only from the current release cache", async () => { const deletes: string[] = []; const runtime = staticRuntime({ - open: vi.fn(async (name: string) => ({ - match: async () => new Response("bad", { status: 500 }), - delete: async (url: string): Promise => { + open: vi.fn(async (name: string) => { + const recordDelete = async (url: string): Promise => { deletes.push(`${name}:${url}`); return true; - }, - })), + }; + return { + match: async () => new Response("bad", { status: 500 }), + delete: recordDelete, + }; + }), keys: vi.fn(async () => [currentCacheName]), delete: vi.fn(async () => true), }); diff --git a/tests/unit/web-push-fence-store.test.ts b/tests/unit/web-push-fence-store.test.ts index 1c164d4..2a6e397 100644 --- a/tests/unit/web-push-fence-store.test.ts +++ b/tests/unit/web-push-fence-store.test.ts @@ -42,6 +42,37 @@ function manualScheduler() { } describe("Web Push durable control fence", () => { + it.each([0, 2, 3, 9_999])( + "rejects a CAS receipt that is not the exact next revision (%i)", + async (revision) => { + // WP-01. Only the exact next revision is evidence that this command + // actually wrote the control it claims to have written. + const dependencies = createFakePushControlStoreDependencies(); + const repository = dependencies.repository; + const compareAndSwap = repository.compareAndSwap.bind(repository); + repository.compareAndSwap = async (input) => { + const written = await compareAndSwap(input); + return written.ok + ? { + ok: true as const, + value: { ...written.value, revision }, + } + : written; + }; + const store = createPushAssociationFenceStore(dependencies); + + await expect( + store.prepare({ + authority: firstAuthority, + updatedAt: "2026-07-28T00:00:00.000Z", + }), + ).resolves.toMatchObject({ + ok: false, + error: { code: "CONTROL_CORRUPT" }, + }); + }, + ); + it("CASes UNASSOCIATED to ACTIVE and prevents tombstone resurrection", async () => { const store = createPushAssociationFenceStore( createFakePushControlStoreDependencies(), diff --git a/tests/unit/web-push-worker-runtime.test.ts b/tests/unit/web-push-worker-runtime.test.ts index 1c18d7a..df55302 100644 --- a/tests/unit/web-push-worker-runtime.test.ts +++ b/tests/unit/web-push-worker-runtime.test.ts @@ -445,6 +445,8 @@ describe("Web Push worker runtime", () => { event: "web_push_subscription_rotated", outcome: "DEGRADED", reason: "DEADLINE_EXCEEDED", + countBucket: expect.any(String), + truncated: expect.any(Boolean), }); runtime.dispose(); }); @@ -489,6 +491,8 @@ describe("Web Push worker runtime", () => { event: "web_push_subscription_rotated", outcome: "DEGRADED", reason: "ABORTED", + countBucket: expect.any(String), + truncated: expect.any(Boolean), }); const throwingStore = await activeFence(); @@ -540,6 +544,8 @@ describe("Web Push worker runtime", () => { event: "web_push_subscription_rotated", outcome: "DEGRADED", reason: "ABORTED", + countBucket: expect.any(String), + truncated: expect.any(Boolean), }); throwingRuntime.dispose(); });