From 0edfab8876c51308f749a476cabe65ba45dd8a01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 20 Sep 2026 19:11:31 +0200 Subject: [PATCH 1/6] fix(glanceable): rate-limit native updates to stop device overheating https://github.com/Kilo-Org/cloud/pull/6323 --- .../src/lib/glanceable/publisher.test.ts | 273 ++++- apps/mobile/src/lib/glanceable/publisher.ts | 127 ++- .../src/lib/glanceable/snapshot-transforms.ts | 61 ++ .../src/glanceable-agents-snapshot.ts | 9 +- .../notifications/src/rpc-schemas.test.ts | 14 + packages/notifications/src/rpc-schemas.ts | 11 + .../src/dos/NotificationChannelDO.ts | 32 +- services/notifications/src/index.ts | 32 +- .../src/lib/glanceable-delivery.test.ts | 88 +- .../src/lib/glanceable-refresh.test.ts | 995 ++++++++++++++++++ .../src/lib/glanceable-refresh.ts | 374 ++++++- .../src/dos/UserConnectionDO.test.ts | 125 ++- .../src/dos/UserConnectionDO.ts | 36 +- .../src/ingest/metadata.test.ts | 57 + .../session-ingest/src/ingest/metadata.ts | 8 + 15 files changed, 2118 insertions(+), 124 deletions(-) create mode 100644 apps/mobile/src/lib/glanceable/snapshot-transforms.ts create mode 100644 services/notifications/src/lib/glanceable-refresh.test.ts diff --git a/apps/mobile/src/lib/glanceable/publisher.test.ts b/apps/mobile/src/lib/glanceable/publisher.test.ts index 9fc746d049..5462d9a02b 100644 --- a/apps/mobile/src/lib/glanceable/publisher.test.ts +++ b/apps/mobile/src/lib/glanceable/publisher.test.ts @@ -3,13 +3,19 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { buildGlanceableSnapshot, + GLANCEABLE_COALESCE_MS, GLANCEABLE_SNAPSHOT_EXPIRY_MS, + GLANCEABLE_STALE_MS, type GlanceableAgentsSnapshot, isStartableGlanceableWork, } from '@kilocode/app-shared/glanceable-agents-snapshot'; import { getTerminalBlankEpoch, writeSignedOutSnapshotAndEnd } from './cleanup'; -import { GlanceablePublisher } from './publisher'; +import { + GLANCEABLE_RENEW_MARGIN_MS, + GlanceablePublisher, + hasSameGlanceableContent, +} from './publisher'; import { type GlanceableSink, type GlanceableSinkContext, @@ -68,6 +74,10 @@ function snapshotFor(sessions: { status: string }[], now: number, revision = 0) }); } +function withTitle(snapshot: GlanceableAgentsSnapshot, newestSessionTitle: string | null = null) { + return { snapshot, newestSessionTitle }; +} + afterEach(() => { vi.useRealTimers(); setSurfaceExtras({ newestSessionTitle: null, actionFeedback: null }); @@ -122,6 +132,20 @@ describe('GlanceablePublisher', () => { publisher.dispose(); }); + it('starts the activity when the first tray write matches the seeded snapshot', () => { + // A restored revision can equal the tray's content, but nothing has raised + // the surface yet, so the first eligible emit must not be skipped. + const { sink, calls } = makeSink(); + const publisher = new GlanceablePublisher({ + sinks: [sink], + now: () => NOW, + initial: snapshotFor([{ status: 'busy' }], NOW, 0), + }); + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + expect(count(calls, 'startOrUpdate')).toBe(1); + publisher.dispose(); + }); + it('counts an unrecognized status as running, matching what the row glyph draws', () => { // One session, unknown status: the shared kind map folds every non-idle, // non-needs-input status into running, and the list row's glyph draws the @@ -158,6 +182,195 @@ describe('GlanceablePublisher', () => { publisher.dispose(); }); + it('emits an approval transition immediately instead of on the coalesce window', () => { + // `question` and `permission` both count as needs-input, so `needsInput` + // stays constant while `needsApproval` (the Approve control gate) changes. + // The approval transition must not wait for the coalesce window. + vi.useFakeTimers(); + const { sink, calls } = makeSink(); + const publisher = new GlanceablePublisher({ sinks: [sink], now: () => NOW, coalesceMs: 1000 }); + publisher.handleSessions([{ status: 'question' }], PUB_CTX); + publisher.handleSessions([{ status: 'permission' }], PUB_CTX); + expect(count(calls, 'startOrUpdate')).toBe(2); + publisher.dispose(); + }); + + it('emits a cleared approval immediately too', () => { + vi.useFakeTimers(); + const { sink, calls } = makeSink(); + const publisher = new GlanceablePublisher({ sinks: [sink], now: () => NOW, coalesceMs: 1000 }); + publisher.handleSessions([{ status: 'permission' }], PUB_CTX); + publisher.handleSessions([{ status: 'question' }], PUB_CTX); + expect(count(calls, 'startOrUpdate')).toBe(2); + publisher.dispose(); + }); + + it('does not redraw the native surfaces for a heartbeat that changes no visible content', () => { + vi.useFakeTimers(); + const { sink, calls } = makeSink(); + const publisher = new GlanceablePublisher({ sinks: [sink], now: () => NOW, coalesceMs: 1000 }); + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + for (let heartbeat = 0; heartbeat < 50; heartbeat += 1) { + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + } + vi.advanceTimersByTime(1000); + // Every heartbeat writes the tray cache, but only the first one changed the + // surface, so the 50 identical writes must not re-render it and must not + // rewrite the widget timeline or Live Activity either. + expect(count(calls, 'startOrUpdate')).toBe(1); + expect(count(calls, 'publish')).toBe(1); + publisher.dispose(); + }); + + it('renews the deadline on an unchanged heartbeat only once the stale window approaches', () => { + // Identical heartbeats must still renew before the published deadline + // lapses: `updatedAt`/`expiresAt`, the widget stale frame, and the Live + // Activity stale date all key off the write, so never renewing falsely + // flags confirmed-current data as stale. Renewing on every heartbeat would + // rewrite the native surface every few seconds, so it waits for the margin. + vi.useFakeTimers(); + let now = NOW; + const { sink, calls } = makeSink(); + const publisher = new GlanceablePublisher({ sinks: [sink], now: () => now, coalesceMs: 1000 }); + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + const first = lastSnapshot(calls, 'publish'); + + // Inside the margin: no write at all, so the heartbeat cannot amplify. + now += GLANCEABLE_RENEW_MARGIN_MS - 1; + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + expect(count(calls, 'publish')).toBe(1); + + // At the margin the local write renews the deadline well before it lapses. + now += 1; + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + const renewed = lastSnapshot(calls, 'publish'); + expect(count(calls, 'publish')).toBe(2); + expect(renewed.revision).toBeGreaterThan(first.revision); + expect(renewed.updatedAt).toBe(new Date(now).toISOString()); + expect(renewed.expiresAt).toBe(new Date(now + GLANCEABLE_SNAPSHOT_EXPIRY_MS).toISOString()); + expect(Date.parse(renewed.updatedAt)).toBeLessThan( + Date.parse(first.updatedAt) + GLANCEABLE_STALE_MS + ); + // The renewal still carries a start/update so a failed or deferred Live + // Activity start is retried while the counts stay stable; it happens at the + // renewal margin, never on every heartbeat. + expect(count(calls, 'startOrUpdate')).toBe(2); + expect(lastSnapshot(calls, 'startOrUpdate').running).toBe(1); + publisher.dispose(); + }); + + it('retries the Live Activity start on an unchanged heartbeat past the renewal margin', () => { + // A start the sink could not raise (transient ActivityKit failure, or a + // start deferred behind a dismissal) must not be stranded: the renewal + // re-emits it while the visible counts are unchanged. + vi.useFakeTimers(); + let now = NOW; + const { sink, calls } = makeSink(); + const publisher = new GlanceablePublisher({ sinks: [sink], now: () => now, coalesceMs: 1000 }); + publisher.handleSessions([{ status: 'permission' }], PUB_CTX); + expect(count(calls, 'startOrUpdate')).toBe(1); + + now += GLANCEABLE_RENEW_MARGIN_MS; + publisher.handleSessions([{ status: 'permission' }], PUB_CTX); + expect(count(calls, 'startOrUpdate')).toBe(2); + publisher.dispose(); + }); + + it('renews at most once per stale margin across many unchanged heartbeats', () => { + // A 10 s heartbeat for 15 minutes is exactly one renewal, not one native + // rewrite per heartbeat (the in-app amplification the heat fix removed). + vi.useFakeTimers(); + let now = NOW; + const { sink, calls } = makeSink(); + const publisher = new GlanceablePublisher({ sinks: [sink], now: () => now, coalesceMs: 1000 }); + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + for (let heartbeat = 0; heartbeat < 90; heartbeat += 1) { + now += 10_000; + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + } + expect(count(calls, 'publish')).toBe(2); + // The single renewal re-emits the start so a failed start is retried; the + // other 89 heartbeats write nothing. + expect(count(calls, 'startOrUpdate')).toBe(2); + publisher.dispose(); + }); + + it('does not republish a pending coalesced frame after an unchanged renewal', () => { + // A content change inside the coalesce window stores a snapshot dated at + // that change. A renewal heartbeat before the timer fires publishes a newer + // frame for the same visible content; if the timer then fired, it would + // republish the older revision/updatedAt and mark the surface stale right + // after the renewal. + vi.useFakeTimers(); + let now = NOW; + const { sink, calls } = makeSink(); + const publisher = new GlanceablePublisher({ sinks: [sink], now: () => now, coalesceMs: 1000 }); + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + + // A counts-only change at t+1 s is inside the window: coalesced, not emitted. + now += 1; + publisher.handleSessions([{ status: 'busy' }, { status: 'busy' }], PUB_CTX); + expect(count(calls, 'startOrUpdate')).toBe(1); + + // A renewal heartbeat at the margin emits the newer frame for the same content. + now += GLANCEABLE_RENEW_MARGIN_MS; + publisher.handleSessions([{ status: 'busy' }, { status: 'busy' }], PUB_CTX); + const renewed = lastSnapshot(calls, 'startOrUpdate'); + expect(count(calls, 'startOrUpdate')).toBe(2); + expect(renewed.updatedAt).toBe(new Date(now).toISOString()); + + // The pending coalesced frame must not fire after the renewal. + vi.advanceTimersByTime(1000); + expect(count(calls, 'startOrUpdate')).toBe(2); + expect(lastSnapshot(calls, 'startOrUpdate').updatedAt).toBe(renewed.updatedAt); + expect(lastSnapshot(calls, 'startOrUpdate').running).toBe(2); + publisher.dispose(); + }); + + it('bounds count churn to one native update per window', () => { + vi.useFakeTimers(); + let now = NOW; + const { sink, calls } = makeSink(); + const publisher = new GlanceablePublisher({ + sinks: [sink], + now: () => now, + coalesceMs: GLANCEABLE_COALESCE_MS, + }); + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + expect(count(calls, 'startOrUpdate')).toBe(1); + for (let second = 1; second <= 5; second += 1) { + vi.advanceTimersByTime(1000); + now += 1000; + publisher.handleSessions( + Array.from({ length: second + 1 }, () => ({ status: 'busy' })), + PUB_CTX + ); + } + // Five once-a-second count changes coalesce into the one window's update, + // not one native re-render per heartbeat. + expect(count(calls, 'startOrUpdate')).toBe(1); + publisher.dispose(); + }); + + it('still redraws when only the newest session title changes', () => { + vi.useFakeTimers(); + const { sink, calls } = makeSink(); + const publisher = new GlanceablePublisher({ sinks: [sink], now: () => NOW, coalesceMs: 1000 }); + publisher.handleSessions( + [{ status: 'busy', title: 'First session', updatedAt: '2026-01-01T00:00:00.000Z' }], + PUB_CTX + ); + publisher.handleSessions( + [{ status: 'busy', title: 'Renamed session', updatedAt: '2026-01-01T00:00:00.000Z' }], + PUB_CTX + ); + vi.advanceTimersByTime(1000); + // The counts are identical, but the widget draws the title, so the rename + // must still reach the native surface. + expect(count(calls, 'startOrUpdate')).toBe(2); + publisher.dispose(); + }); + it('discards an incoming older revision', () => { const { sink, calls } = makeSink(); const publisher = new GlanceablePublisher({ sinks: [sink], now: () => NOW }); @@ -425,6 +638,64 @@ describe('GlanceablePublisher', () => { }); }); +describe('hasSameGlanceableContent', () => { + const busy = (previousRevision: number, now = NOW) => + buildGlanceableSnapshot({ + sessions: [{ status: 'busy' }], + userId: 'u1', + organizationId: null, + now, + previousRevision, + }); + + it('ignores revision and timestamps but compares every visible field', () => { + const first = busy(0); + const nextRevision = busy(1, NOW + 5000); + expect(nextRevision.revision).toBe(2); + expect(hasSameGlanceableContent(withTitle(first), withTitle(nextRevision))).toBe(true); + // The widget draws the newest title, so a title-only change still differs. + expect( + hasSameGlanceableContent(withTitle(first, 'First'), withTitle(nextRevision, 'Renamed')) + ).toBe(false); + + const moreRunning = buildGlanceableSnapshot({ + sessions: [{ status: 'busy' }, { status: 'busy' }], + userId: 'u1', + organizationId: null, + now: NOW + 5000, + previousRevision: 1, + }); + expect(hasSameGlanceableContent(withTitle(first), withTitle(moreRunning))).toBe(false); + }); + + it('compares the approval count, the wait anchor, and the status', () => { + const question = buildGlanceableSnapshot({ + sessions: [{ status: 'question' }], + userId: 'u1', + organizationId: null, + now: NOW, + }); + const permission = buildGlanceableSnapshot({ + sessions: [{ status: 'permission' }], + userId: 'u1', + organizationId: null, + now: NOW, + }); + // Same needsInput total, but only one of the two can be approved. + expect(question.needsInput).toBe(permission.needsInput); + expect(hasSameGlanceableContent(withTitle(question), withTitle(permission))).toBe(false); + expect( + hasSameGlanceableContent(withTitle(question), withTitle({ ...question, status: 'stale' })) + ).toBe(false); + expect( + hasSameGlanceableContent( + withTitle(question), + withTitle({ ...question, needsInputSince: '2026-01-01T00:00:00.000Z' }) + ) + ).toBe(false); + }); +}); + describe('GlanceablePublisher waiting ask', () => { function makePublisher(overrides: { terminalBlankEpoch?: () => number } = {}) { const asks: (WaitingAsk | null)[] = []; diff --git a/apps/mobile/src/lib/glanceable/publisher.ts b/apps/mobile/src/lib/glanceable/publisher.ts index 40e99dfea2..6ac5543972 100644 --- a/apps/mobile/src/lib/glanceable/publisher.ts +++ b/apps/mobile/src/lib/glanceable/publisher.ts @@ -1,16 +1,16 @@ import { buildGlanceableSnapshot, GLANCEABLE_COALESCE_MS, - GLANCEABLE_SNAPSHOT_EXPIRY_MS, + GLANCEABLE_STALE_MS, GLANCEABLE_TERMINAL_MS, type GlanceableAgentsSnapshot, - type GlanceableAgentsSnapshotStatus, isEligibleGlanceableWork, isStartableGlanceableWork, shouldDiscardGlanceableRevision, } from '@kilocode/app-shared/glanceable-agents-snapshot'; import { type NewestSessionRow, newestSessionTitle } from './newest-session'; +import { hasSameGlanceableContent, withStatus } from './snapshot-transforms'; import { getGlanceableDelivery, type GlanceableSink, @@ -20,6 +20,8 @@ import { import { getSurfaceExtras, setSurfaceExtras } from './surface-extras'; import { selectWaitingAsk, type WaitingAsk, type WaitingAskRow } from './waiting-ask'; +export { hasSameGlanceableContent, withStatus }; + /** * Framework-agnostic publisher state machine. Derives one versioned snapshot * from the active-sessions tray cache, coalesces later happy updates, starts @@ -74,33 +76,15 @@ export type GlanceablePublisherOptions = { type TimerHandle = ReturnType; -/** Advance the status and revision without renewing stale data's lifetime. */ -export function withStatus( - snapshot: GlanceableAgentsSnapshot, - status: GlanceableAgentsSnapshotStatus, - now: number -): GlanceableAgentsSnapshot { - if (status === 'stale') { - if (snapshot.status === 'signed_out' || snapshot.status === 'privacy') { - return snapshot; - } - const expired = snapshot.status === 'expired' || now >= Date.parse(snapshot.expiresAt); - return { - ...snapshot, - revision: snapshot.revision + 1, - status: expired ? 'expired' : 'stale', - ...(expired ? { running: 0, needsInput: 0, idle: 0, needsInputSince: null } : {}), - }; - } - const updatedAt = new Date(now).toISOString(); - return { - ...snapshot, - revision: snapshot.revision + 1, - updatedAt, - expiresAt: new Date(now + GLANCEABLE_SNAPSHOT_EXPIRY_MS).toISOString(), - status, - }; -} +/** + * Renew the published deadline once the surface is within this margin of its + * stale frame (`updatedAt + GLANCEABLE_STALE_MS`). The tray heartbeats every + * 10–30 s, so renewing at half the window leaves the heartbeat free to rewrite + * the native surface only once per half window, not once per heartbeat, while + * still refreshing well before the widget stale frame and the Live Activity + * stale date land. + */ +export const GLANCEABLE_RENEW_MARGIN_MS = GLANCEABLE_STALE_MS / 2; export class GlanceablePublisher { private readonly sinks: readonly GlanceableSink[]; @@ -114,6 +98,13 @@ export class GlanceablePublisher { private readonly skipWaitingAskSessionId?: string; private current: GlanceableAgentsSnapshot | null; private activityStarted: boolean; + /** + * `updatedAt` of the snapshot last written to a sink, i.e. the frame the + * native stale deadline keys off. A heartbeat whose visible content did not + * change leaves it alone, so the renewal gate can tell how close that + * deadline is. + */ + private lastPublishedAt: number | null = null; private coalesceTimer: TimerHandle | null = null; private terminalTimer: TimerHandle | null = null; private pendingCoalesced: { @@ -152,10 +143,9 @@ export class GlanceablePublisher { } // The newest session's title never enters the snapshot (privacy contract): // it rides in the surface extras every widget reads on redraw. - setSurfaceExtras({ - ...getSurfaceExtras(), - newestSessionTitle: newestSessionTitle(sessions), - }); + const previousTitle = getSurfaceExtras().newestSessionTitle; + const nextTitle = newestSessionTitle(sessions); + setSurfaceExtras({ ...getSurfaceExtras(), newestSessionTitle: nextTitle }); getGlanceableDelivery().registerScopeTokens(ctx.organizationId, ctx.userId); const now = this.now(); this.applyExpiry(now, ctx); @@ -168,24 +158,81 @@ export class GlanceablePublisher { previousRevision: this.current?.revision ?? 0, }); + // The rows are the only place the session id exists, so the ask is selected + // here, on every heartbeat, before the unchanged-content gate below: that + // gate is about the native surface, and the ask is not part of it. The + // visible content can stay identical — the same counts and wait anchor — + // while the session that waits changes, because neither the session id nor + // the tray order is a snapshot field: a tie on `statusUpdatedAt` resolves to + // the first asking row, and an answered row leaving while another starts + // keeps the counts. Selecting before the gate, not after it, is what keeps + // the Approve/Open target on the row that is actually asking. + this.noteWaitingAsk( + isEligibleGlanceableWork(snapshot) ? selectWaitingAsk(this.askRows(sessions), ctx, now) : null + ); + + // Every heartbeat writes the tray cache, so a write whose visible content + // did not change must not re-render the widget or update the ongoing + // notification / Live Activity. It must still renew the deadline before it + // lapses, because `updatedAt`/`expiresAt`, the widget stale frame, and the + // Live Activity stale date all key off the published write: skipping the + // renewal would falsely flag confirmed-current data as stale, while + // publishing every heartbeat would rewrite the native surface every few + // seconds. So renew only once the published frame approaches its stale + // window, and renew through `emit` rather than `publish`: the start/update + // call is what retries a Live Activity start the sink could not raise (a + // transient ActivityKit failure, or a start deferred behind a dismissal), + // and leaving it out of the renewal would strand that surface until the + // counts next changed. Keep the revision monotonic for the next real emit, + // and leave any pending coalesced emit alone. The first eligible emit + // (nothing started yet) is exempt: it is what raises the surface. + if ( + this.current !== null && + hasSameGlanceableContent( + { snapshot: this.current, newestSessionTitle: previousTitle }, + { snapshot, newestSessionTitle: nextTitle } + ) && + (this.activityStarted || !isEligibleGlanceableWork(snapshot)) + ) { + if ( + isEligibleGlanceableWork(snapshot) && + (this.lastPublishedAt === null || now - this.lastPublishedAt >= GLANCEABLE_RENEW_MARGIN_MS) + ) { + // The renewal frame carries the same visible content as any pending + // coalesced emit but a newer revision, so emitting it supersedes that + // timer: leaving the timer armed would republish the older frame after + // this one and move `lastPublishedAt` backwards, marking the surface + // stale again right after it was renewed. + this.cancelCoalesce(); + this.emit(snapshot, ctx); + } + this.current = snapshot; + return; + } + + // `needsApproval` is optional, so normalize it for the coalesce decision. + const previousNeedsApproval = this.current?.needsApproval ?? 0; + if (isEligibleGlanceableWork(snapshot)) { - // The rows are the only place the session id exists, so the ask is - // selected here, beside the snapshot derived from the same rows. - this.noteWaitingAsk(selectWaitingAsk(this.askRows(sessions), ctx, now)); this.cancelTerminal(); if (!this.activityStarted) { // First eligible emit starts the activity immediately, no coalesce wait. this.emit(snapshot, ctx); this.activityStarted = true; - } else if (snapshot.needsInput !== this.current?.needsInput) { - // Badge changes are actionable and must reach the launcher immediately. + } else if ( + snapshot.needsInput !== this.current?.needsInput || + (snapshot.needsApproval ?? 0) !== previousNeedsApproval + ) { + // Actionable needs-input/approval changes must reach the launcher + // immediately: `needsApproval` gates the Approve control on every + // surface, and a question <-> permission move keeps `needsInput` + // constant while that control appears or disappears. this.cancelCoalesce(); this.emit(snapshot, ctx); } else { this.scheduleCoalesced(snapshot, ctx); } } else { - this.noteWaitingAsk(null); this.cancelCoalesce(); this.publish(snapshot); if (this.activityStarted) { @@ -304,6 +351,7 @@ export class GlanceablePublisher { } private emit(snapshot: GlanceableAgentsSnapshot, ctx: GlanceableSinkContext): void { + this.lastPublishedAt = Date.parse(snapshot.updatedAt); for (const sink of this.sinks) { // Guarded separately: a failing widget timeline write must not skip the // Live Activity start that follows it. @@ -317,6 +365,7 @@ export class GlanceablePublisher { } private publish(snapshot: GlanceableAgentsSnapshot): void { + this.lastPublishedAt = Date.parse(snapshot.updatedAt); for (const sink of this.sinks) { guardSink('publish', () => { sink.publish(snapshot); diff --git a/apps/mobile/src/lib/glanceable/snapshot-transforms.ts b/apps/mobile/src/lib/glanceable/snapshot-transforms.ts new file mode 100644 index 0000000000..2f8b0437d0 --- /dev/null +++ b/apps/mobile/src/lib/glanceable/snapshot-transforms.ts @@ -0,0 +1,61 @@ +import { + GLANCEABLE_SNAPSHOT_EXPIRY_MS, + type GlanceableAgentsSnapshot, + type GlanceableAgentsSnapshotStatus, +} from '@kilocode/app-shared/glanceable-agents-snapshot'; + +/** + * Pure snapshot transforms shared by the publisher and the side effects that + * reason about a previous snapshot (cleanup, view props). Kept out of + * `publisher.ts` so that state machine stays within the module size limit. + */ + +/** Advance the status and revision without renewing stale data's lifetime. */ +export function withStatus( + snapshot: GlanceableAgentsSnapshot, + status: GlanceableAgentsSnapshotStatus, + now: number +): GlanceableAgentsSnapshot { + if (status === 'stale') { + if (snapshot.status === 'signed_out' || snapshot.status === 'privacy') { + return snapshot; + } + const expired = snapshot.status === 'expired' || now >= Date.parse(snapshot.expiresAt); + return { + ...snapshot, + revision: snapshot.revision + 1, + status: expired ? 'expired' : 'stale', + ...(expired ? { running: 0, needsInput: 0, idle: 0, needsInputSince: null } : {}), + }; + } + const updatedAt = new Date(now).toISOString(); + return { + ...snapshot, + revision: snapshot.revision + 1, + updatedAt, + expiresAt: new Date(now + GLANCEABLE_SNAPSHOT_EXPIRY_MS).toISOString(), + status, + }; +} + +/** + * True when two snapshot + newest-title pairs would draw the same native + * surface. Compares the user-visible fields only — `status`, the counts, the + * wait anchor, and the title the widget draws from the surface extras — so a + * tray write whose only difference is `revision`/`updatedAt` does not + * re-render the widget or update the ongoing notification / Live Activity. + */ +export function hasSameGlanceableContent( + a: { snapshot: GlanceableAgentsSnapshot; newestSessionTitle: string | null }, + b: { snapshot: GlanceableAgentsSnapshot; newestSessionTitle: string | null } +): boolean { + return ( + a.snapshot.status === b.snapshot.status && + a.snapshot.running === b.snapshot.running && + a.snapshot.needsInput === b.snapshot.needsInput && + (a.snapshot.needsApproval ?? 0) === (b.snapshot.needsApproval ?? 0) && + a.snapshot.idle === b.snapshot.idle && + a.snapshot.needsInputSince === b.snapshot.needsInputSince && + a.newestSessionTitle === b.newestSessionTitle + ); +} diff --git a/packages/app-shared/src/glanceable-agents-snapshot.ts b/packages/app-shared/src/glanceable-agents-snapshot.ts index 84e495b2b1..be52470838 100644 --- a/packages/app-shared/src/glanceable-agents-snapshot.ts +++ b/packages/app-shared/src/glanceable-agents-snapshot.ts @@ -14,8 +14,13 @@ import { z } from 'zod'; export const GLANCEABLE_SNAPSHOT_SCHEMA_VERSION = 1; /** 8 hours: matches the usual Live Activity lifetime. */ export const GLANCEABLE_SNAPSHOT_EXPIRY_MS = 28_800_000; -/** Later happy updates are coalesced for at most this long. */ -export const GLANCEABLE_COALESCE_MS = 1000; +/** + * Later happy updates are coalesced for at most this long. A tray with many + * running sessions heartbeats every few seconds, and each emit re-renders the + * native surfaces, so a counts-only change may lag by at most one window. An + * actionable needs-input change never waits (the publisher emits it at once). + */ +export const GLANCEABLE_COALESCE_MS = 10_000; /** Terminal empty lasts at most this long before the activity ends. */ export const GLANCEABLE_TERMINAL_MS = 8000; /** diff --git a/packages/notifications/src/rpc-schemas.test.ts b/packages/notifications/src/rpc-schemas.test.ts index b284be7b60..45e998a295 100644 --- a/packages/notifications/src/rpc-schemas.test.ts +++ b/packages/notifications/src/rpc-schemas.test.ts @@ -77,4 +77,18 @@ describe('refreshGlanceableSessionsInputSchema', () => { expect(parsed).not.toHaveProperty('organizationId'); expect(parsed).not.toHaveProperty('running'); }); + + it('passes the approval hint through for the glanceable delivery window', () => { + const parsed = refreshGlanceableSessionsInputSchema.parse({ + userId: 'usr_1', + cliSessionIds: ['ses_1', 'ses_2'], + approvalChangedSessionIds: ['ses_2'], + }); + expect(parsed.approvalChangedSessionIds).toEqual(['ses_2']); + // A refresh with no hint is a normal counts change: no window exemption. + expect( + refreshGlanceableSessionsInputSchema.parse({ userId: 'usr_1', cliSessionIds: ['ses_1'] }) + .approvalChangedSessionIds + ).toBeUndefined(); + }); }); diff --git a/packages/notifications/src/rpc-schemas.ts b/packages/notifications/src/rpc-schemas.ts index 70349602f1..3e43683cbf 100644 --- a/packages/notifications/src/rpc-schemas.ts +++ b/packages/notifications/src/rpc-schemas.ts @@ -176,6 +176,17 @@ export type SendCloudAgentSessionNotificationResult = z.infer< export const refreshGlanceableSessionsInputSchema = z.object({ userId: z.string().min(1), cliSessionIds: z.array(z.string().min(1)).min(1), + /** + * The subset of `cliSessionIds` whose change moved a session into or out of + * the `permission` status. `needsApproval` gates the Approve control on the + * locked/background surfaces, so the delivery window may not defer those + * scopes. The caller is the one that saw the previous status; the server + * cannot read a past status. Naming the sessions lets the server exempt only + * the scope that actually moved: one batch can span the personal scope and + * several organizations, and exempting all of them would wake devices that + * had no approval change. + */ + approvalChangedSessionIds: z.array(z.string().min(1)).optional(), }); export type RefreshGlanceableSessionsParams = z.infer; diff --git a/services/notifications/src/dos/NotificationChannelDO.ts b/services/notifications/src/dos/NotificationChannelDO.ts index e1234e0a61..07f169f26e 100644 --- a/services/notifications/src/dos/NotificationChannelDO.ts +++ b/services/notifications/src/dos/NotificationChannelDO.ts @@ -18,7 +18,11 @@ import { isPushSinkEnabled } from '../lib/push-sink'; import type { ExpoPushMessage, SendResult, TicketTokenPair } from '../lib/expo-push'; import { sendPushNotifications } from '../lib/expo-push'; import { glanceableDeliveryDeps } from '../lib/glanceable-delivery-deps'; -import { refreshGlanceableSnapshot } from '../lib/glanceable-refresh'; +import { + foldPendingGlanceableRefreshDeadline, + flushDueGlanceableRefreshes, + refreshGlanceableSnapshot, +} from '../lib/glanceable-refresh'; import { expoPushExtrasForPushData } from '../lib/push-message-extras'; type ReceiptCheckMessage = { ticketTokenPairs: TicketTokenPair[] }; @@ -69,8 +73,16 @@ export class NotificationChannelDO extends DurableObject { async refreshGlanceableSnapshot(params: { userId: string; organizationId: string | null; + approvalChanged?: boolean; }): Promise { - await refreshGlanceableSnapshot(params, this.ctx.storage, glanceableDeliveryDeps(this.env)); + const { userId, organizationId, approvalChanged } = params; + await refreshGlanceableSnapshot( + { userId, organizationId }, + this.ctx.storage, + glanceableDeliveryDeps(this.env), + Date.now, + { approvalChanged } + ); } async dispatchPush(input: DispatchPushInput): Promise { @@ -470,6 +482,13 @@ export class NotificationChannelDO extends DurableObject { override async alarm(): Promise { const now = Date.now(); + // Deliver any glanceable refresh the rate-limit window deferred. Its + // remaining deadline folds into this sweep's alarm so the trailing + // delivery is not stranded when no idem/rl record outlives it. + const dueGlanceableRefreshAt = await flushDueGlanceableRefreshes( + this.ctx.storage, + glanceableDeliveryDeps(this.env) + ); const idemEntries = await this.ctx.storage.list({ prefix: IDEM_PREFIX }); const expiredIdem: string[] = []; let nextAlarmAt: number | undefined; @@ -478,6 +497,7 @@ export class NotificationChannelDO extends DurableObject { nextAlarmAt = deadline; } }; + if (dueGlanceableRefreshAt !== null) requestAlarmAtOrBefore(dueGlanceableRefreshAt); for (const [key, rec] of idemEntries) { if (rec.stage === 'accepted') { @@ -514,8 +534,12 @@ export class NotificationChannelDO extends DurableObject { const toDelete = [...expiredIdem, ...expiredRl]; if (toDelete.length > 0) await this.ctx.storage.delete(toDelete); - if (nextAlarmAt !== undefined) { - await this.ctx.storage.setAlarm(nextAlarmAt); + // A deferral can land during the awaits above. Fold the pending deadline in + // again rather than trusting the flush's earlier capture, so the final + // setAlarm cannot overwrite it and delay the trailing delivery. + const alarmAt = await foldPendingGlanceableRefreshDeadline(this.ctx.storage, nextAlarmAt); + if (alarmAt !== undefined) { + await this.ctx.storage.setAlarm(alarmAt); } } diff --git a/services/notifications/src/index.ts b/services/notifications/src/index.ts index b20a8d052a..1f0919d16f 100644 --- a/services/notifications/src/index.ts +++ b/services/notifications/src/index.ts @@ -325,7 +325,8 @@ export class NotificationsService extends WorkerEntrypoint { /** Refresh each affected scope without notification preferences or viewer-presence gates. */ async refreshGlanceableSessions(params: RefreshGlanceableSessionsParams): Promise { - const { userId, cliSessionIds } = refreshGlanceableSessionsInputSchema.parse(params); + const { userId, cliSessionIds, approvalChangedSessionIds } = + refreshGlanceableSessionsInputSchema.parse(params); const db = getWorkerDb(this.env.HYPERDRIVE.connectionString); // Read ownership too: an absent row is personal, but a foreign row is not authorized. const rows = await db @@ -337,19 +338,34 @@ export class NotificationsService extends WorkerEntrypoint { .from(cli_sessions_v2) .where(inArray(cli_sessions_v2.session_id, cliSessionIds)); const byId = new Map(rows.map(row => [row.sessionId, row])); - const scopes = new Set(); - for (const sessionId of cliSessionIds) { - const row = byId.get(sessionId); - if (!row) scopes.add(null); - else if (row.userId === userId) scopes.add(row.organizationId); - } + const scopesOf = (sessionIds: readonly string[]): Set => { + const scopes = new Set(); + for (const sessionId of sessionIds) { + const row = byId.get(sessionId); + if (!row) scopes.add(null); + else if (row.userId === userId) scopes.add(row.organizationId); + } + return scopes; + }; + const scopes = scopesOf(cliSessionIds); + // The window exemption reaches only the scope(s) that actually moved into or + // out of `permission`. Forwarding the caller's flag to every scope would skip + // the rate-limit/deferral branch for scopes with no approval change — an extra + // build and device wake, the cost this window exists to prevent. + const approvalScopes = scopesOf(approvalChangedSessionIds ?? []); // Every entrypoint uses the same user DO. The snapshot route still rechecks membership. const stub = this.env.NOTIFICATION_CHANNEL_DO.get( this.env.NOTIFICATION_CHANNEL_DO.idFromName(userId) ); const results = await Promise.allSettled( - [...scopes].map(organizationId => stub.refreshGlanceableSnapshot({ userId, organizationId })) + [...scopes].map(organizationId => + stub.refreshGlanceableSnapshot({ + userId, + organizationId, + approvalChanged: approvalScopes.has(organizationId), + }) + ) ); for (const result of results) { if (result.status === 'rejected') { diff --git a/services/notifications/src/lib/glanceable-delivery.test.ts b/services/notifications/src/lib/glanceable-delivery.test.ts index 6d8d617a5a..8b06b8f20b 100644 --- a/services/notifications/src/lib/glanceable-delivery.test.ts +++ b/services/notifications/src/lib/glanceable-delivery.test.ts @@ -440,6 +440,8 @@ describe('NotificationsService.refreshGlanceableSessions', () => { }); await createService().refreshGlanceableSessions(personalRefresh); current = freshSnapshot({ status: 'empty', running: 0, needsInputSince: null }); + // A newer read only reaches the route once the delivery window has elapsed. + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); deferNext = true; const oldIdle = createService().refreshGlanceableSessions(personalRefresh); await started.promise; @@ -509,6 +511,46 @@ describe('NotificationsService.refreshGlanceableSessions', () => { ]); }); + it('exempts only the scope whose session moved into permission, not every scope in the batch', async () => { + // One heartbeat batch can carry the personal session and an org session at + // once. Only the scope that actually moved into permission may skip the + // delivery window: exempting the org scope too would wake that device for a + // counts-only change, the cost the window exists to prevent. + const { createService, messages } = setupService({ + response: scope => + Response.json( + freshSnapshot({ + scopeKey: `${scope.userId}:${scope.organizationId ?? 'personal'}`, + organizationBound: scope.organizationId !== null, + running: scope.organizationId === null ? 1 : 2, + needsApproval: scope.organizationId === null ? 1 : 0, + }) + ), + }); + // Open the delivery window for both scopes. + await createService().refreshGlanceableSessions({ + userId: 'usr_1', + cliSessionIds: ['personal'], + }); + await createService().refreshGlanceableSessions({ userId: 'usr_1', cliSessionIds: ['org-a'] }); + + // Inside both windows, only the personal session moved into permission. + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:02.000Z')); + await createService().refreshGlanceableSessions({ + userId: 'usr_1', + cliSessionIds: ['personal', 'org-a'], + approvalChangedSessionIds: ['personal'], + }); + + const scopeKeys = messages + .filter(message => message.to === 'ExponentPushToken[ios]') + .map(message => (message.data as { scopeKey: string }).scopeKey); + // The personal scope delivers at once (exempt); the org scope defers to its + // window, so its counts-only change lands on the trailing refresh instead. + expect(scopeKeys.filter(scopeKey => scopeKey === 'usr_1:personal')).toHaveLength(2); + expect(scopeKeys.filter(scopeKey => scopeKey === 'usr_1:org-1')).toHaveLength(1); + }); + it('recovers delivery after snapshot and delivery failures', async () => { let current = freshSnapshot(); let unavailable = false; @@ -517,12 +559,17 @@ describe('NotificationsService.refreshGlanceableSessions', () => { }); await createService().refreshGlanceableSessions(personalRefresh); unavailable = true; + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); await createService().refreshGlanceableSessions(personalRefresh); unavailable = false; vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:10:00.000Z')); current = freshSnapshot({ running: 0, idle: 1 }); vi.mocked(sendPushNotifications).mockRejectedValueOnce(new Error('Expo unavailable')); await createService().refreshGlanceableSessions(personalRefresh); + // The failed send spent its window: an immediate retry would turn every + // change inside GLANCEABLE_DELIVERY_MIN_INTERVAL_MS into another build and + // device wake. The next change recovers once the window elapses. + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:10:20.000Z')); await createService().refreshGlanceableSessions(personalRefresh); expect( messages @@ -539,6 +586,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { const { createService, messages } = setupService({ response: () => Response.json(current) }); await createService().refreshGlanceableSessions(personalRefresh); current = freshSnapshot({ status: 'stale', running: 0, needsInputSince: null }); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); await createService().refreshGlanceableSessions(personalRefresh); vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:10:00.000Z')); current = freshSnapshot({ running: 0, idle: 1 }); @@ -591,13 +639,14 @@ describe('NotificationsService.refreshGlanceableSessions', () => { needsInputSince: null, }); - vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:01.000Z')); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); // Idle work keeps a card alive but never raises one, so the push-to-start // token stays unused until an agent works or asks for input. current = freshSnapshot({ running: 0, idle: 1 }); await createService().refreshGlanceableSessions(personalRefresh); expect(apns.map(({ token, aps }) => [token, aps.event])).toEqual([['old-activity', 'end']]); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:20.000Z')); current = freshSnapshot({ running: 1, idle: 1 }); await createService().refreshGlanceableSessions(personalRefresh); expect(apns.map(({ token, aps }) => [token, aps.event])).toEqual([ @@ -607,7 +656,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { expect(JSON.parse(apns[1].aps['content-state'].props)).toMatchObject({ running: 1, idle: 1, - needsInputSince: '2026-08-27T10:00:01.000Z', + needsInputSince: '2026-08-27T10:00:20.000Z', }); expect([...activityRows.keys()]).toEqual(['scope-token']); }); @@ -622,7 +671,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { // Nothing registers an activity token while the app never runs, so without // the fence every refresh would stack another card on the Lock Screen. for (const second of [0, 1, 2]) { - vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:00.000Z') + second * 1000); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:00.000Z') + second * 10_000); await createService().refreshGlanceableSessions(personalRefresh); } expect(apns.map(({ token, aps }) => [token, aps.event])).toEqual([['scope-token', 'start']]); @@ -645,13 +694,13 @@ describe('NotificationsService.refreshGlanceableSessions', () => { kind: 'ios_activity', updated_at: '2026-08-27 10:00:00+00', }); - vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:01.000Z')); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); current = freshSnapshot({ status: 'empty', running: 0, needsInputSince: null }); await createService().refreshGlanceableSessions(personalRefresh); expect([...activityRows.keys()]).toEqual(['scope-token']); // That end retired the card, so fresh work may raise another one. - vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:02.000Z')); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:20.000Z')); current = freshSnapshot({ running: 1 }); await createService().refreshGlanceableSessions(personalRefresh); expect(apns.map(({ token, aps }) => [token, aps.event])).toEqual([ @@ -691,11 +740,11 @@ describe('NotificationsService.refreshGlanceableSessions', () => { kind: 'ios_activity', updated_at: '2026-08-27 10:00:00+00', }); - vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:01.000Z')); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); await createService().refreshGlanceableSessions(personalRefresh); expect(apns.map(({ aps }) => [aps.event, aps['stale-date']])).toEqual([ ['start', Date.parse('2026-08-27T10:30:00.000Z') / 1000], - ['update', Date.parse('2026-08-27T10:30:01.000Z') / 1000], + ['update', Date.parse('2026-08-27T10:30:10.000Z') / 1000], ]); }); @@ -824,7 +873,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { contentState: toGlanceableContentState(snapshot), }); } - vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:01.000Z')); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:11.000Z')); current = freshSnapshot({ running: 0, needsInput: 1 }); await createService().refreshGlanceableSessions(personalRefresh); expect(liveActivityProps()).toMatchObject([{ running: 0, needsInput: 1 }]); @@ -838,7 +887,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { await ending; } expect(activityRows.get('old-activity')).toEqual(renewedRow); - vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:02.000Z')); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:21.000Z')); current = freshSnapshot({ running: 0, idle: 1 }); await createService().refreshGlanceableSessions(personalRefresh); expect(liveActivityProps()).toMatchObject([ @@ -847,7 +896,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { needsInput: 0, idle: 1, // Forwarded from this refresh's snapshot, not latched at the earlier one. - needsInputSince: '2026-08-27T10:00:02.000Z', + needsInputSince: '2026-08-27T10:00:21.000Z', }, ]); const liveToken = withPushToStart ? 'started-2' : 'live-activity'; @@ -1004,6 +1053,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { expect([...activityRows.keys()]).toEqual(['scope-token', 'activity-token']); configured = true; current = freshSnapshot({ running: 0, needsInput: 1 }); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); await createService().refreshGlanceableSessions(personalRefresh); expect(liveActivityProps()).toMatchObject([{ running: 0, needsInput: 1 }]); expect(apns.map(({ token, aps }) => [token, aps.event])).toEqual([ @@ -1030,7 +1080,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { expect([...activityRows.keys()]).toEqual(['scope-token', 'old-activity']); expect(liveActivityProps()).toEqual([]); - vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:01.000Z')); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); current = freshSnapshot({ running: 0, needsInput: 1 }); await createService().refreshGlanceableSessions(personalRefresh); expect(liveActivityProps()).toMatchObject([{ running: 0, needsInput: 1 }]); @@ -1043,11 +1093,11 @@ describe('NotificationsService.refreshGlanceableSessions', () => { updated_at: '2026-08-27 10:00:01+00', }); } - vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:02.000Z')); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:20.000Z')); current = freshSnapshot({ running: 0, idle: 1 }); await createService().refreshGlanceableSessions(personalRefresh); expect(liveActivityProps()).toMatchObject([ - { running: 0, needsInput: 0, idle: 1, needsInputSince: '2026-08-27T10:00:02.000Z' }, + { running: 0, needsInput: 0, idle: 1, needsInputSince: '2026-08-27T10:00:20.000Z' }, ]); expect(apns.map(({ token, aps }) => [token, aps.event])).toEqual([ ['old-activity', 'end'], @@ -1072,11 +1122,11 @@ describe('NotificationsService.refreshGlanceableSessions', () => { }); await createService().refreshGlanceableSessions(personalRefresh); rejected = false; - vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:01.000Z')); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); current = freshSnapshot({ running: 0, needsInput: 1 }); await createService().refreshGlanceableSessions(personalRefresh); expect(liveActivityProps()).toMatchObject([ - { running: 0, needsInput: 1, needsInputSince: '2026-08-27T10:00:01.000Z' }, + { running: 0, needsInput: 1, needsInputSince: '2026-08-27T10:00:10.000Z' }, ]); expect(apns.map(({ token, aps }) => [token, aps.event])).toEqual([ ['old-activity', 'end'], @@ -1110,7 +1160,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { kind: 'ios_activity', updated_at: '2026-08-27 10:00:01+00', }); - vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:01.000Z')); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); current = freshSnapshot({ running: 0, needsInput: 1 }); await createService().refreshGlanceableSessions(personalRefresh); expect(liveActivityProps()).toMatchObject([{ running: 0, needsInput: 1 }]); @@ -1173,6 +1223,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { release.resolve(); await firstEnd; } + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); current = freshSnapshot({ running: 0, needsInput: 1 }); await createService().refreshGlanceableSessions(personalRefresh); expect(liveActivityProps()).toMatchObject([{ running: 0, needsInput: 1 }]); @@ -1191,6 +1242,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { } } current = freshSnapshot({ running: 0, idle: 1 }); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:20.000Z')); await createService().refreshGlanceableSessions(personalRefresh); expect(liveActivityProps()).toMatchObject([{ running: 0, needsInput: 0, idle: 1 }]); expect(apns.map(({ token, aps }) => [token, aps.event])).toEqual([ @@ -1233,6 +1285,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { await firstEnd; } rejected = false; + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); current = freshSnapshot({ running: 0, needsInput: 1 }); await createService().refreshGlanceableSessions(personalRefresh); expect(liveActivityProps()).toMatchObject([{ running: 0, needsInput: 1 }]); @@ -1282,6 +1335,7 @@ describe('NotificationsService.refreshGlanceableSessions', () => { cliSessionIds: ['org-a'], }); rejected = false; + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); current = freshSnapshot({ running: 0, needsInput: 1 }); await createService().refreshGlanceableSessions({ userId: 'usr_1', @@ -1316,10 +1370,12 @@ describe('NotificationsService.refreshGlanceableSessions', () => { await createService().refreshGlanceableSessions(personalRefresh); expect([...activityRows.keys()]).toEqual(['scope-token', 'old-activity']); rejected = false; + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:10.000Z')); await createService().refreshGlanceableSessions(personalRefresh); expect(liveActivityProps()).toEqual([]); expect([...activityRows.keys()]).toEqual(['scope-token']); current = freshSnapshot({ running: 0, needsInput: 1 }); + vi.mocked(Date.now).mockReturnValue(Date.parse('2026-08-27T10:00:20.000Z')); await createService().refreshGlanceableSessions(personalRefresh); expect(liveActivityProps()).toMatchObject([{ running: 0, needsInput: 1 }]); expect(apns.map(request => request.aps.event)).toEqual(['end', 'end', 'start']); diff --git a/services/notifications/src/lib/glanceable-refresh.test.ts b/services/notifications/src/lib/glanceable-refresh.test.ts new file mode 100644 index 0000000000..f9cbf6fa6d --- /dev/null +++ b/services/notifications/src/lib/glanceable-refresh.test.ts @@ -0,0 +1,995 @@ +import { env, runInDurableObject } from 'cloudflare:test'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import type { ExpoPushMessage } from './expo-push'; +import type { ActiveAgentsGlanceable, GlanceableDeliveryDeps } from './glanceable-delivery'; +import { + foldPendingGlanceableRefreshDeadline, + flushDueGlanceableRefreshes, + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + refreshGlanceableSnapshot, +} from './glanceable-refresh'; + +/** + * In-memory `DurableObjectStorage` for the pure refresh unit tests. The DO-level + * case below runs against the real storage through `runInDurableObject`. + */ +class FakeStorage { + private readonly entries = new Map(); + private alarmTime: number | null = null; + + async get(key: string): Promise { + return this.entries.get(key) as T | undefined; + } + + async put(key: string, value: T): Promise { + this.entries.set(key, value); + } + + async delete(key: string | string[]): Promise { + if (Array.isArray(key)) { + let deleted = false; + for (const entry of key) deleted = this.entries.delete(entry) || deleted; + return deleted; + } + return this.entries.delete(key); + } + + async list(options: { prefix?: string; limit?: number } = {}): Promise> { + const out = new Map(); + for (const [key, value] of this.entries) { + if (options.prefix !== undefined && !key.startsWith(options.prefix)) continue; + out.set(key, value as T); + if (options.limit !== undefined && out.size >= options.limit) break; + } + return out; + } + + async transaction(closure: (txn: FakeStorage) => Promise): Promise { + return closure(this); + } + + async getAlarm(): Promise { + return this.alarmTime; + } + + async setAlarm(scheduledTime: number | Date): Promise { + this.alarmTime = typeof scheduledTime === 'number' ? scheduledTime : scheduledTime.getTime(); + } +} + +function pendingKey(userId: string, organizationId: string | null): string { + return `glanceable-pending:${JSON.stringify([userId, organizationId])}`; +} + +function deliveryKey(userId: string, organizationId: string | null): string { + return `glanceable:${JSON.stringify([userId, organizationId])}:delivery`; +} + +function snapshot(overrides: Partial = {}): ActiveAgentsGlanceable { + return { + type: 'active_agents_glanceable', + schemaVersion: 1, + revision: 1, + scopeKey: 'scope-key', + organizationBound: false, + status: 'happy', + running: 2, + needsInput: 1, + needsApproval: 0, + idle: 0, + updatedAt: '2026-09-18T00:00:00.000Z', + expiresAt: '2026-09-18T08:00:00.000Z', + needsInputSince: null, + ...overrides, + }; +} + +type Harness = { + storage: FakeStorage; + deps: GlanceableDeliveryDeps; + builds: number; + expoSends: ExpoPushMessage[][]; + iosSends: { token: string; event: string }[][]; + setNext: (next: ActiveAgentsGlanceable | null) => void; + failNextBuild: (error: Error) => void; + /** Make the next Expo send throw (a failed transport attempt). */ + failNextExpoPush: (error: Error) => void; + /** Stall the next Expo send so a second refresh can supersede it mid-flight. */ + blockNextExpoPush: () => { started: Promise; release: () => void }; +}; + +function makeHarness(): Harness { + const storage = new FakeStorage(); + let next: ActiveAgentsGlanceable | null = snapshot(); + let buildError: Error | null = null; + let expoError: Error | null = null; + let expoGate: { gate: Promise; started: () => void } | null = null; + + const harness: Harness = { + storage, + builds: 0, + expoSends: [], + iosSends: [], + setNext: value => { + next = value; + }, + failNextBuild: error => { + buildError = error; + }, + failNextExpoPush: error => { + expoError = error; + }, + blockNextExpoPush: () => { + let release: () => void = () => undefined; + let started: () => void = () => undefined; + const gate = new Promise(resolve => { + release = resolve; + }); + const startedPromise = new Promise(resolve => { + started = resolve; + }); + expoGate = { gate, started }; + return { started: startedPromise, release }; + }, + deps: { + buildSnapshot: async () => { + harness.builds += 1; + if (buildError !== null) { + const error = buildError; + buildError = null; + throw error; + } + return next; + }, + listIosActivityTokens: async () => [], + sendIosLiveActivity: async tokens => { + harness.iosSends.push(tokens.map(token => ({ ...token }))); + }, + listIosExpoTokens: async () => [], + listAndroidExpoTokens: async () => [{ token: 'android-token', locale: null }], + hasAndroidOngoingToken: async () => true, + sendExpoPush: async messages => { + harness.expoSends.push(messages); + if (expoError !== null) { + const error = expoError; + expoError = null; + throw error; + } + const gate = expoGate; + if (gate !== null) { + expoGate = null; + gate.started(); + await gate.gate; + } + }, + }, + }; + + return harness; +} + +const asStorage = (storage: FakeStorage) => storage as unknown as DurableObjectStorage; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('refreshGlanceableSnapshot delivery window', () => { + it('coalesces a change inside the window and delivers it from the trailing flush', async () => { + const h = makeHarness(); + const base = Date.parse('2026-09-18T00:00:00.000Z'); + let now = base; + const scope = { userId: 'user-1', organizationId: null }; + + h.setNext(snapshot({ running: 1, needsInput: 0 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(h.builds).toBe(1); + expect(h.expoSends).toHaveLength(1); + + // A change 2s later is inside the 10s window: deferred, not delivered. + now = base + 2_000; + h.setNext(snapshot({ running: 2, needsInput: 1 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(h.builds).toBe(1); + expect(h.expoSends).toHaveLength(1); + expect(await h.storage.get(pendingKey('user-1', null))).toEqual({ + userId: 'user-1', + organizationId: null, + dueAt: base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: base + 2_000, + }); + expect(await h.storage.getAlarm()).toBe(base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS); + + // The trailing flush at the deadline delivers the newer counts. + now = base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS; + await expect( + flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now) + ).resolves.toBeNull(); + expect(h.expoSends).toHaveLength(2); + expect(h.expoSends[1][0].data).toMatchObject({ running: 2, needsInput: 1, idle: 0 }); + expect(await h.storage.get(pendingKey('user-1', null))).toBeUndefined(); + }); + + it('delivers an approval change inside the window instead of deferring it', async () => { + const h = makeHarness(); + const base = Date.parse('2026-09-18T01:00:00.000Z'); + let now = base; + const scope = { userId: 'user-approval', organizationId: null }; + + h.setNext(snapshot({ running: 1, needsInput: 0, needsApproval: 0 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(h.expoSends).toHaveLength(1); + + // A question -> permission move keeps needsInput constant but gates the + // Approve control, which must not wait out the shared window. + now = base + 2_000; + h.setNext(snapshot({ running: 1, needsInput: 1, needsApproval: 1 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now, { + approvalChanged: true, + }); + + expect(h.builds).toBe(2); + expect(h.expoSends).toHaveLength(2); + expect(h.expoSends[1][0].data).toMatchObject({ needsInput: 1, needsApproval: 1 }); + expect(await h.storage.get(pendingKey('user-approval', null))).toBeUndefined(); + // The approval delivery still spends the window, so counts-only churn right + // after it defers instead of waking the device again. + expect(await h.storage.get(deliveryKey('user-approval', null))).toEqual({ + deliveredAt: now, + outcome: 'delivered', + }); + + now = base + 3_000; + h.setNext(snapshot({ running: 2, needsInput: 1, needsApproval: 1 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(h.builds).toBe(2); + expect(h.expoSends).toHaveLength(2); + expect(await h.storage.get(pendingKey('user-approval', null))).toMatchObject({ + dueAt: base + 2_000 + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + }); + }); + + it('re-arms a trailing refresh whose build returns no snapshot', async () => { + const h = makeHarness(); + const now = 70_000_000; + const key = pendingKey('user-null-build', null); + await h.storage.put(key, { userId: 'user-null-build', organizationId: null, dueAt: now - 1 }); + // Production buildSnapshot returns null (it never throws) when the route or + // its credentials fail. Consuming the record here would drop the final + // counts with no alarm left to retry them. + h.setNext(null); + + await expect( + flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now) + ).resolves.toBe(now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS); + expect(h.builds).toBe(1); + expect(h.expoSends).toHaveLength(0); + expect(await h.storage.get(key)).toEqual({ + userId: 'user-null-build', + organizationId: null, + dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: now, + }); + }); + + it('does not re-arm a trailing refresh superseded by a newer delivery', async () => { + const h = makeHarness(); + const base = 80_000_000; + let now = base; + const scope = { userId: 'user-superseded-null', organizationId: null }; + const key = pendingKey('user-superseded-null', null); + // The flush consumes the due record before it runs. + await h.storage.put(key, { + userId: 'user-superseded-null', + organizationId: null, + dueAt: now - 1, + }); + + // The trailing build stalls, so a newer refresh can bump the revision and + // deliver while this fetch is in flight. + const started = Promise.withResolvers(); + const release = Promise.withResolvers(); + let build = 0; + h.deps.buildSnapshot = async () => { + build += 1; + if (build === 1) { + started.resolve(); + await release.promise; + return null; + } + return snapshot({ running: 5 }); + }; + + const trailing = flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now); + await started.promise; + + // The newer refresh owns the revision and delivers; its snapshot already + // covers the deferred change. + now = base + 1_000; + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(h.expoSends).toHaveLength(1); + + release.resolve(); + // The superseded trailing fetch must not re-arm a redundant device wake. + await expect(trailing).resolves.toBeNull(); + expect(await h.storage.get(key)).toBeUndefined(); + }); + + it('re-arms a trailing refresh when a concurrent refresh only moves the revision', async () => { + const h = makeHarness(); + const base = 85_000_000; + let now = base; + const scope = { userId: 'user-superseded-no-delivery', organizationId: null }; + const key = pendingKey('user-superseded-no-delivery', null); + await h.storage.put(key, { + userId: 'user-superseded-no-delivery', + organizationId: null, + dueAt: now - 1, + }); + + // The trailing build stalls; a concurrent refresh bumps the revision while + // it is in flight but its own build returns null, so it delivers nothing + // and re-arms nothing. + const started = Promise.withResolvers(); + const release = Promise.withResolvers(); + let build = 0; + h.deps.buildSnapshot = async () => { + build += 1; + if (build === 1) { + started.resolve(); + await release.promise; + } + return null; + }; + + const trailing = flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now); + await started.promise; + + now = base + 1_000; + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(h.expoSends).toHaveLength(0); + + release.resolve(); + // The revision moved but no delivery landed, so the deferred counts must + // keep a pending record and a deadline instead of being dropped. + await expect(trailing).resolves.toBe(now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS); + expect(await h.storage.get(key)).toEqual({ + userId: 'user-superseded-no-delivery', + organizationId: null, + dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: now, + }); + }); + + it('re-arms a trailing refresh when the concurrent attempt fails at the transport', async () => { + const h = makeHarness(); + const base = 90_000_000; + let now = base; + const scope = { userId: 'user-superseded-failed', organizationId: null }; + const key = pendingKey('user-superseded-failed', null); + await h.storage.put(key, { + userId: 'user-superseded-failed', + organizationId: null, + dueAt: now - 1, + }); + + // The trailing build stalls, so a newer refresh attempts its delivery while + // this fetch is in flight. + const started = Promise.withResolvers(); + const release = Promise.withResolvers(); + let build = 0; + h.deps.buildSnapshot = async () => { + build += 1; + if (build === 1) { + started.resolve(); + await release.promise; + return null; + } + return snapshot({ running: 5 }); + }; + + const trailing = flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now); + await started.promise; + + // The concurrent refresh delivers but the transport rejects: the failure + // branch still writes the delivery record (a spent window) before it + // rethrows, so the record alone cannot tell a landed delivery from a spent + // window. + now = base + 1_000; + h.failNextExpoPush(new Error('transport down')); + await expect( + refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now) + ).rejects.toThrow('transport down'); + expect(h.expoSends).toHaveLength(1); + expect(await h.storage.get(deliveryKey('user-superseded-failed', null))).toEqual({ + deliveredAt: now, + outcome: 'failed', + }); + + release.resolve(); + // No snapshot was delivered, so the deferred counts must keep a pending + // record and a deadline instead of being dropped with no alarm left. + await expect(trailing).resolves.toBe(now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS); + expect(await h.storage.get(key)).toEqual({ + userId: 'user-superseded-failed', + organizationId: null, + dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: now, + }); + }); + + it('re-arms a stale past alarm so the trailing delivery is not stranded', async () => { + const h = makeHarness(); + const base = 60_000_000; + let now = base; + const scope = { userId: 'user-stale-alarm', organizationId: null }; + + // A first delivery opens the window. + h.setNext(snapshot({ running: 1 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + + // A leftover alarm from an earlier schedule sits in the past. A past alarm + // is not a usable schedule: keeping it would strand the deferred change and + // the trailing flush would never deliver the final counts. + await h.storage.setAlarm(base - 60_000); + + now = base + 2_000; + h.setNext(snapshot({ running: 2, needsInput: 1 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(await h.storage.getAlarm()).toBe(base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS); + + now = base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS; + await flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now); + expect(h.expoSends).toHaveLength(2); + expect(h.expoSends[1][0].data).toMatchObject({ running: 2, needsInput: 1 }); + }); + + it('spends the window on a failed delivery and re-arms the deferred counts', async () => { + const h = makeHarness(); + const base = 30_000_000; + let now = base; + const scope = { userId: 'user-failed-send', organizationId: null }; + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + // The first attempt builds and sends, and the transport rejects. + h.setNext(snapshot({ running: 1 })); + h.failNextExpoPush(new Error('The bearer token is invalid.')); + await expect( + refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now) + ).rejects.toThrow('The bearer token is invalid.'); + expect(h.builds).toBe(1); + expect(h.expoSends).toHaveLength(1); + // The failed attempt still opens the window: without it every later change + // would retry the whole build+send at once. + expect(await h.storage.get(deliveryKey('user-failed-send', null))).toEqual({ + deliveredAt: base, + outcome: 'failed', + }); + + // A change inside the window is deferred, not retried immediately. + now = base + 2_000; + h.setNext(snapshot({ running: 2, needsInput: 1 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(h.builds).toBe(1); + expect(h.expoSends).toHaveLength(1); + expect(await h.storage.get(pendingKey('user-failed-send', null))).toEqual({ + userId: 'user-failed-send', + organizationId: null, + dueAt: base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: base + 2_000, + }); + expect(await h.storage.getAlarm()).toBe(base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS); + + // The trailing flush at the deadline rebuilds and re-attempts: its build + // carries the final counts even though the transport still fails, and the + // window re-opens at the trailing attempt instead of unlocking a storm. + now = base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS; + h.failNextExpoPush(new Error('The bearer token is invalid.')); + await expect( + flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now) + ).resolves.toBe(now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS); + expect(h.builds).toBe(2); + expect(h.expoSends).toHaveLength(2); + expect(warnSpy).toHaveBeenCalledWith( + 'Glanceable trailing refresh failed', + expect.objectContaining({ error: 'The bearer token is invalid.' }) + ); + expect(await h.storage.get(deliveryKey('user-failed-send', null))).toEqual({ + deliveredAt: base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + outcome: 'failed', + }); + // A throwing trailing delivery keeps the deferred counts: re-armed for the + // next window instead of dropped with no retry left. + expect(await h.storage.get(pendingKey('user-failed-send', null))).toEqual({ + userId: 'user-failed-send', + organizationId: null, + dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: now, + }); + }); + + it('keeps a pending trailing refresh when a failed attempt opens the window', async () => { + const h = makeHarness(); + const now = 40_000_000; + const dueAt = now + 5_000; + const scope = { userId: 'user-failed-keep', organizationId: null }; + await h.storage.put(pendingKey('user-failed-keep', null), { + userId: 'user-failed-keep', + organizationId: null, + dueAt, + }); + + h.failNextExpoPush(new Error('transport down')); + await expect( + refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now) + ).rejects.toThrow('transport down'); + // The failed attempt opens the window but must not cancel the trailing + // refresh a deferred change is waiting on. + expect(await h.storage.get(deliveryKey('user-failed-keep', null))).toEqual({ + deliveredAt: now, + outcome: 'failed', + }); + expect(await h.storage.get(pendingKey('user-failed-keep', null))).toMatchObject({ dueAt }); + }); + + it('does not let a superseded in-flight delivery cancel a trailing refresh', async () => { + const h = makeHarness(); + const base = 20_000_000; + let now = base; + const scope = { userId: 'user-superseded', organizationId: null }; + + // A first delivery opens the window. + h.setNext(snapshot({ running: 1 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(await h.storage.get(deliveryKey('user-superseded', null))).toEqual({ + deliveredAt: base, + outcome: 'delivered', + }); + + // A refresh at the window edge starts delivering but stalls in transport. + now = base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS; + h.setNext(snapshot({ running: 2 })); + const gate = h.blockNextExpoPush(); + const stalled = refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + await gate.started; + + // A newer change supersedes it and delivers, opening a fresh window. + now = base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS + 500; + h.setNext(snapshot({ running: 3 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + + // A change inside that new window is deferred to the trailing alarm. + now = base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS + 1_000; + h.setNext(snapshot({ running: 4 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + const dueAt = base + 2 * GLANCEABLE_DELIVERY_MIN_INTERVAL_MS + 500; + const deferredAt = base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS + 1_000; + expect(await h.storage.get(pendingKey('user-superseded', null))).toEqual({ + userId: 'user-superseded', + organizationId: null, + dueAt, + deferredAt, + }); + + // The stalled, superseded delivery must neither drop the trailing refresh + // the newer change recorded while it was in flight nor open a window. + gate.release(); + await stalled; + expect(await h.storage.get(pendingKey('user-superseded', null))).toEqual({ + userId: 'user-superseded', + organizationId: null, + dueAt, + deferredAt, + }); + expect(await h.storage.get(deliveryKey('user-superseded', null))).toEqual({ + deliveredAt: base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS + 500, + outcome: 'delivered', + }); + }); + + it('keeps a counts-only deferral written while an approval-exempt delivery is in flight', async () => { + const h = makeHarness(); + const base = 50_000_000; + let now = base; + const scope = { userId: 'user-approval-race', organizationId: null }; + + // A first delivery opens the window. + h.setNext(snapshot({ running: 1, needsInput: 0 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + + // An approval change inside the window starts delivering (exempt) but + // stalls in transport. + now = base + 2_000; + h.setNext(snapshot({ running: 1, needsInput: 1, needsApproval: 1 })); + const gate = h.blockNextExpoPush(); + const stalled = refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now, { + approvalChanged: true, + }); + await gate.started; + + // A counts-only change lands while the exempt delivery is in flight. The + // window is still open, so it defers to the alarm; the revision is not + // advanced by a deferral. + now = base + 2_500; + h.setNext(snapshot({ running: 2, needsInput: 1, needsApproval: 1 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + const dueAt = base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS; + expect(await h.storage.get(pendingKey('user-approval-race', null))).toMatchObject({ dueAt }); + + // The exempt delivery completing must not discard the deferral that landed + // while it was in flight. + gate.release(); + await stalled; + expect(await h.storage.get(pendingKey('user-approval-race', null))).toMatchObject({ dueAt }); + // The deferral keeps the alarm that will deliver it. + expect(await h.storage.getAlarm()).toBe(dueAt); + + // The trailing flush still delivers the final counts. The exempt delivery + // started the next window when it completed, so the deferred refresh waits + // out that window before it lands. + now = base + 2_500 + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS; + await flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now); + expect(h.expoSends).toHaveLength(3); + expect(h.expoSends[2][0].data).toMatchObject({ + running: 2, + needsInput: 1, + needsApproval: 1, + }); + }); + + it('keeps a newer deferral even when an older one in the same window shares its dueAt', async () => { + const h = makeHarness(); + const base = 55_000_000; + let now = base; + const scope = { userId: 'user-approval-collision', organizationId: null }; + + // A first delivery opens the window. + h.setNext(snapshot({ running: 1, needsInput: 0 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + + // A counts-only change defers to the window end. + now = base + 1_000; + h.setNext(snapshot({ running: 2, needsInput: 0 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + const dueAt = base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS; + + // An approval change inside the window starts delivering (exempt) and + // stalls; its snapshot already covers the first deferral. + now = base + 2_000; + h.setNext(snapshot({ running: 2, needsInput: 1, needsApproval: 1 })); + const gate = h.blockNextExpoPush(); + const stalled = refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now, { + approvalChanged: true, + }); + await gate.started; + + // A second counts-only change defers during the delivery. It carries the + // same `dueAt` as the first deferral, so only its write time tells them + // apart: the delivery superseded the first, not this one. + now = base + 2_500; + h.setNext(snapshot({ running: 3, needsInput: 1, needsApproval: 1 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + + gate.release(); + await stalled; + expect(await h.storage.get(pendingKey('user-approval-collision', null))).toEqual({ + userId: 'user-approval-collision', + organizationId: null, + dueAt, + deferredAt: base + 2_500, + }); + + // The trailing flush still delivers the final counts. + now = base + 2_500 + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS; + await flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now); + expect(h.expoSends).toHaveLength(3); + expect(h.expoSends[2][0].data).toMatchObject({ + running: 3, + needsInput: 1, + needsApproval: 1, + }); + }); + + it('leaves a pending record that is not due and returns its deadline', async () => { + const h = makeHarness(); + const now = 1_000_000; + const dueAt = now + 5_000; + await h.storage.put(pendingKey('user-later', null), { + userId: 'user-later', + organizationId: null, + dueAt, + }); + + await expect( + flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now) + ).resolves.toBe(dueAt); + expect(await h.storage.get(pendingKey('user-later', null))).toBeDefined(); + expect(h.builds).toBe(0); + }); + + it('rate-limits each scope independently', async () => { + const h = makeHarness(); + const base = 5_000_000; + let now = base; + + h.setNext(snapshot({ running: 1 })); + await refreshGlanceableSnapshot( + { userId: 'user-1', organizationId: 'org-1' }, + asStorage(h.storage), + h.deps, + () => now + ); + + // A different scope is outside the first scope's window and delivers at once. + now = base + 2_000; + h.setNext(snapshot({ running: 2 })); + await refreshGlanceableSnapshot( + { userId: 'user-1', organizationId: 'org-2' }, + asStorage(h.storage), + h.deps, + () => now + ); + + expect(h.expoSends).toHaveLength(2); + expect(await h.storage.get(pendingKey('user-1', 'org-2'))).toBeUndefined(); + }); + + it('logs one build and one delivery per window, the trailing flush carrying the final counts', async () => { + const h = makeHarness(); + const base = 15_000_000; + let now = base; + const scope = { userId: 'user-log', organizationId: null }; + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + + // The first change builds and delivers immediately (trailing: false). + h.setNext(snapshot({ running: 1, needsInput: 0, idle: 0 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(logSpy).toHaveBeenCalledWith({ + event: 'glanceable_snapshot_build', + scope: ['user-log', null], + revision: 1, + trailing: false, + status: 'happy', + running: 1, + needsInput: 0, + idle: 0, + needsApproval: 0, + }); + expect(logSpy).toHaveBeenCalledWith({ + event: 'glanceable_delivery', + scope: ['user-log', null], + revision: 1, + trailing: false, + status: 'happy', + running: 1, + needsInput: 0, + idle: 0, + needsApproval: 0, + }); + + // A change inside the window leaves no further evidence: it defers. + logSpy.mockClear(); + now = base + 2_000; + h.setNext(snapshot({ running: 2, needsInput: 1, idle: 3 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(logSpy).not.toHaveBeenCalled(); + + // The trailing flush logs its build and delivery with the final counts. + now = base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS; + await flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now); + expect(logSpy).toHaveBeenCalledWith({ + event: 'glanceable_snapshot_build', + scope: ['user-log', null], + revision: 2, + trailing: true, + status: 'happy', + running: 2, + needsInput: 1, + idle: 3, + needsApproval: 0, + }); + expect(logSpy).toHaveBeenCalledWith({ + event: 'glanceable_delivery', + scope: ['user-log', null], + revision: 2, + trailing: true, + status: 'happy', + running: 2, + needsInput: 1, + idle: 3, + needsApproval: 0, + }); + }); + + it('does not log a delivery when the attempt is superseded mid-flight', async () => { + const h = makeHarness(); + const base = 25_000_000; + let now = base; + const scope = { userId: 'user-log-superseded', organizationId: null }; + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + + // A first delivery opens the window. + h.setNext(snapshot({ running: 1 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + logSpy.mockClear(); + + // A refresh at the window edge stalls in transport; a newer change + // supersedes it. The superseded attempt logs its committed build (the POST + // happened) but must not log a delivery. + now = base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS; + h.setNext(snapshot({ running: 2 })); + const gate = h.blockNextExpoPush(); + const stalled = refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + await gate.started; + now = base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS + 500; + h.setNext(snapshot({ running: 3 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + const delivered = logSpy.mock.calls + .map(call => call[0] as { event?: string }) + .filter(call => call?.event !== undefined); + expect(delivered).toEqual([ + expect.objectContaining({ event: 'glanceable_snapshot_build', revision: 2 }), + expect.objectContaining({ event: 'glanceable_snapshot_build', revision: 3 }), + expect.objectContaining({ event: 'glanceable_delivery', revision: 3 }), + ]); + + gate.release(); + await stalled; + logSpy.mockClear(); + const afterStall = logSpy.mock.calls + .map(call => call[0] as { event?: string }) + .filter(call => call?.event !== undefined); + expect(afterStall).toEqual([]); + }); + + it('does not consume the window when the snapshot build fails', async () => { + const h = makeHarness(); + const base = 9_000_000; + let now = base; + const scope = { userId: 'user-fail', organizationId: null }; + + h.setNext(null); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(h.expoSends).toHaveLength(0); + expect(await h.storage.get(pendingKey('user-fail', null))).toBeUndefined(); + + // The next change retries immediately because no delivery was recorded. + now = base + 2_000; + h.setNext(snapshot({ running: 3 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(h.expoSends).toHaveLength(1); + expect(await h.storage.get(deliveryKey('user-fail', null))).toEqual({ + deliveredAt: now, + outcome: 'delivered', + }); + expect(await h.storage.get(pendingKey('user-fail', null))).toBeUndefined(); + }); + + it('re-arms a throwing build during the flush so the deferred counts retry', async () => { + const h = makeHarness(); + const now = 12_000_000; + const key = pendingKey('user-throw', null); + await h.storage.put(key, { userId: 'user-throw', organizationId: null, dueAt: now - 1 }); + h.failNextBuild(new Error('route down')); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + await expect( + flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now) + ).resolves.toBe(now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS); + expect(warnSpy).toHaveBeenCalledWith( + 'Glanceable trailing refresh failed', + expect.objectContaining({ error: 'route down' }) + ); + // The record was consumed before the refresh; the rejected build must put it + // back with a fresh deadline or the deferred counts are gone for good. + expect(await h.storage.get(key)).toEqual({ + userId: 'user-throw', + organizationId: null, + dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: now, + }); + expect(h.builds).toBe(1); + }); + + it('keeps a deferral written while a throwing trailing refresh ran', async () => { + const h = makeHarness(); + const now = 13_000_000; + const key = pendingKey('user-throw-race', null); + await h.storage.put(key, { userId: 'user-throw-race', organizationId: null, dueAt: now - 1 }); + // The refresh defers a newer change (the key is reclaimed with a later + // deadline) and only then rejects. The re-arm must not overwrite that + // newer record with an older deadline. + h.deps.buildSnapshot = async () => { + await h.storage.put(key, { + userId: 'user-throw-race', + organizationId: null, + dueAt: now + 3_000, + deferredAt: now + 1, + }); + throw new Error('route down'); + }; + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + await expect( + flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now) + ).resolves.toBe(now + 3_000); + expect(await h.storage.get(key)).toEqual({ + userId: 'user-throw-race', + organizationId: null, + dueAt: now + 3_000, + deferredAt: now + 1, + }); + }); +}); + +describe('NotificationChannelDO alarm glanceable flush', () => { + it('re-arms a due record whose build returns no snapshot and keeps the later deadline', async () => { + const id = env.NOTIFICATION_CHANNEL_DO.idFromName('user-glanceable-alarm'); + const stub = env.NOTIFICATION_CHANNEL_DO.get(id); + const now = Date.now(); + const dueAt = now - 1_000; + const laterDueAt = now + 30_000; + + await runInDurableObject(stub, async (_instance, state) => { + await state.storage.put('glanceable-pending:["user-glanceable-alarm",null]', { + userId: 'user-glanceable-alarm', + organizationId: null, + dueAt, + }); + await state.storage.put('glanceable-pending:["user-glanceable-alarm","org-1"]', { + userId: 'user-glanceable-alarm', + organizationId: 'org-1', + dueAt: laterDueAt, + }); + }); + + await runInDurableObject(stub, async instance => { + await (instance as unknown as { alarm: () => Promise }).alarm(); + }); + + const result = await runInDurableObject(stub, async (_instance, state) => ({ + due: await state.storage.get<{ dueAt: number }>( + 'glanceable-pending:["user-glanceable-alarm",null]' + ), + later: await state.storage.get<{ dueAt: number }>( + 'glanceable-pending:["user-glanceable-alarm","org-1"]' + ), + alarm: await state.storage.getAlarm(), + })); + + // The test env has no internal secret, so the trailing build returns null + // and the record re-arms for the next window instead of being dropped. + const rearmedDueAt = result.due?.dueAt ?? 0; + expect(result.due).toMatchObject({ userId: 'user-glanceable-alarm', organizationId: null }); + expect(rearmedDueAt).toBeGreaterThan(now); + expect(rearmedDueAt).toBeLessThan(laterDueAt); + expect(result.later).toMatchObject({ dueAt: laterDueAt }); + // The alarm takes the earliest remaining deadline. + expect(result.alarm).toBe(rearmedDueAt); + }); + + it('folds a deferral that lands after the sweep chose its alarm', async () => { + // The sweep picks the alarm from its idem/rl records, then awaits before it + // sets it. A pending refresh written in that window owns the earlier + // deadline and must win, or the trailing delivery is delayed. + const storage = new FakeStorage(); + await storage.put(pendingKey('user-fold', null), { + userId: 'user-fold', + organizationId: null, + dueAt: 5_000, + }); + + await expect(foldPendingGlanceableRefreshDeadline(asStorage(storage), 9_000)).resolves.toBe( + 5_000 + ); + // An earlier sweep candidate still wins over the pending deadline. + await expect(foldPendingGlanceableRefreshDeadline(asStorage(storage), 3_000)).resolves.toBe( + 3_000 + ); + // A sweep with no other deadline adopts the pending one. + await expect(foldPendingGlanceableRefreshDeadline(asStorage(storage), undefined)).resolves.toBe( + 5_000 + ); + }); +}); diff --git a/services/notifications/src/lib/glanceable-refresh.ts b/services/notifications/src/lib/glanceable-refresh.ts index 4c9e197ba3..a3b775a4fc 100644 --- a/services/notifications/src/lib/glanceable-refresh.ts +++ b/services/notifications/src/lib/glanceable-refresh.ts @@ -3,6 +3,13 @@ import { z } from 'zod'; import { deliverGlanceableSnapshot, type GlanceableDeliveryDeps } from './glanceable-delivery'; +/** + * At most one aggregate device wake per account scope per window. A change + * inside the window leaves a trailing refresh for the DO alarm, so the final + * counts still land. See #6112: these surfaces must never add device wakeups. + */ +export const GLANCEABLE_DELIVERY_MIN_INTERVAL_MS = 10_000; + const scopeSchema = z.object({ userId: z.string().min(1), organizationId: z.string().min(1).nullable(), @@ -19,14 +26,99 @@ const snapshotTimestampsSchema = refreshStateSchema .pick({ updatedAt: true }) .extend({ expiresAt: z.string().datetime(), needsInputSince: z.string().datetime().nullable() }); +/** + * The last device wake for a scope, used to rate-limit aggregate delivery. A + * failed attempt writes the same record with `outcome: 'failed'` — it still + * spends the window, but it delivered no snapshot, so a trailing refresh must + * not read it as a landed delivery that supersedes its deferred change. + * Optional for records written before this field existed: those only ever came + * from a successful delivery. + */ +const deliveryStateSchema = z.object({ + deliveredAt: z.number(), + outcome: z.enum(['delivered', 'failed']).optional(), +}); + +/** A refresh deferred until the delivery window elapses. */ +const pendingRefreshSchema = z.object({ + userId: z.string().min(1), + organizationId: z.string().min(1).nullable(), + dueAt: z.number(), + // When the deferral was written. A delivery can tell a record it superseded + // from one that landed while it was in flight only by write time: both carry + // the same `dueAt` when they defer inside the same window. Optional for + // records written before this field existed. + deferredAt: z.number().optional(), +}); +type PendingGlanceableRefresh = z.infer; + +const PENDING_PREFIX = 'glanceable-pending:'; + +function pendingKey(scope: { userId: string; organizationId: string | null }): string { + return `${PENDING_PREFIX}${JSON.stringify([scope.userId, scope.organizationId])}`; +} + +/** + * Whether a pending record read after a delivery is the same one that was + * already stored when the delivery started. A deferral written while the + * delivery was in flight carries a later `deferredAt`, so the delivery must not + * cancel it: its counts are not in the delivered snapshot. + */ +function isSameDeferral( + before: PendingGlanceableRefresh | undefined, + after: PendingGlanceableRefresh | undefined +): boolean { + if (before === undefined || after === undefined) return before === after; + return before.dueAt === after.dueAt && before.deferredAt === after.deferredAt; +} + /** The user DO owns these records; no ordering or interval state lives in a Worker instance. */ export async function refreshGlanceableSnapshot( params: { userId: string; organizationId: string | null }, storage: DurableObjectStorage, - deps: GlanceableDeliveryDeps + deps: GlanceableDeliveryDeps, + nowMs: () => number = Date.now, + options: { trailing?: boolean; approvalChanged?: boolean } = {} ): Promise { const scope = scopeSchema.parse(params); const key = `glanceable:${JSON.stringify([scope.userId, scope.organizationId])}`; + const deliveryKey = `${key}:delivery`; + // Rate-limit the device wake per scope. A change inside the window is + // deferred to the alarm rather than dropping it, so the final counts land. + // `needsApproval` is exempt: it gates the Approve control, which must appear + // and clear at once on the locked/background surfaces, exactly as the + // in-app publisher emits a question <-> permission move without waiting. + const delivery = deliveryStateSchema.optional().parse(await storage.get(deliveryKey)); + if ( + options.approvalChanged !== true && + delivery !== undefined && + nowMs() - delivery.deliveredAt < GLANCEABLE_DELIVERY_MIN_INTERVAL_MS + ) { + const now = nowMs(); + const dueAt = delivery.deliveredAt + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS; + await storage.put(pendingKey(scope), { + userId: scope.userId, + organizationId: scope.organizationId, + dueAt, + deferredAt: now, + }); + const currentAlarm = await storage.getAlarm(); + // Only keep an alarm that will actually fire and reschedule before `dueAt`. + // A past alarm is not a usable schedule — it may be a stale record left by a + // restart — so a deferral must (re)arm at `dueAt` or the trailing delivery + // that lands the final counts is never delivered. + if (currentAlarm === null || currentAlarm <= now || dueAt < currentAlarm) { + await storage.setAlarm(dueAt); + } + return; + } + // The delivery that follows supersedes any deferral already stored: its + // snapshot is built after that change. A deferral written while it is in + // flight is not superseded, so remember the one that existed at the start and + // keep a newer one when the delivery completes. + const pendingBeforeDelivery = pendingRefreshSchema + .optional() + .parse(await storage.get(pendingKey(scope))); // Row renewal or temporary absence cannot prove that the native token is live. const iosEndPrefix = (token: string) => `glanceable-ios-end:${JSON.stringify(token)}:`; // A card raised by push-to-start carries no update token until the app runs @@ -36,7 +128,7 @@ export async function refreshGlanceableSnapshot( const iosStartKey = (token: string) => `${iosStartPrefix}${JSON.stringify(token)}`; const request = await storage.transaction(async tx => { const previous = refreshStateSchema.optional().parse(await tx.get(key)); - const now = Date.now(); + const now = nowMs(); const next = { revision: (previous?.revision ?? 0) + 1, updatedAt: new Date( @@ -54,7 +146,40 @@ export async function refreshGlanceableSnapshot( const snapshot = await deps.buildSnapshot(scope.userId, scope.organizationId); // Only the authoritative happy/empty result can change an eligible interval. - if (snapshot === null || (snapshot.status !== 'happy' && snapshot.status !== 'empty')) return; + if (snapshot === null || (snapshot.status !== 'happy' && snapshot.status !== 'empty')) { + // A trailing refresh owes the deferred change its final counts. Production + // `buildSnapshot` returns null (it never throws) when the route or its + // credentials fail, and the flush has already consumed the pending record, + // so re-arm the next window instead of dropping the change with no alarm. + if (options.trailing === true) { + // `buildSnapshot` was awaited after the revision bump, so a concurrent + // refresh for this scope can deliver while this fetch is in flight. That + // delivery already covers the change; re-arming here would leave a record + // its `isSameDeferral` check keeps and the alarm would later fire a + // redundant build+send. Skip the re-arm only when such a delivery actually + // landed, told by the outcome in the record it wrote: the failure branch + // writes the same record with `outcome: 'failed'` after spending the + // window, and treating that as a landed delivery would drop this deferred + // change with no pending record and no alarm left to retry it. A record + // without an outcome predates the field and only ever meant a delivery. + const landed = deliveryStateSchema.optional().parse(await storage.get(deliveryKey)); + if ( + landed !== undefined && + landed.outcome !== 'failed' && + landed.deliveredAt !== delivery?.deliveredAt + ) { + return; + } + const now = nowMs(); + await storage.put(pendingKey(scope), { + userId: scope.userId, + organizationId: scope.organizationId, + dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: now, + }); + } + return; + } // The shared wire schema accepts strings; validate the dates before delivery. snapshotTimestampsSchema.parse(snapshot); @@ -74,54 +199,207 @@ export async function refreshGlanceableSnapshot( }); if (committed === null) return; + // Content-free success evidence for the one-build-per-window invariant + // (§4.15 rules: identifiers and aggregate counts, never session content). + // Without this line a passing delivery window leaves no trace in the + // notifications log and the per-scope window cannot be audited. + console.log({ + event: 'glanceable_snapshot_build', + scope: [scope.userId, scope.organizationId], + revision: request.revision, + trailing: options.trailing === true, + status: committed.status, + running: committed.running, + needsInput: committed.needsInput, + idle: committed.idle, + needsApproval: committed.needsApproval ?? 0, + }); + const eligible = committed.running + committed.needsInput + committed.idle > 0; - await deliverGlanceableSnapshot(scope, { - ...deps, - buildSnapshot: async () => committed, - apnsTimestampSeconds: request.apnsTimestampSeconds, - isCurrent: async () => { - const current = refreshStateSchema.parse(await storage.get(key)); - return current.revision === request.revision; - }, - listIosActivityTokens: async (userId, organizationId) => { - const tokens = await deps.listIosActivityTokens(userId, organizationId); - const current = refreshStateSchema.parse(await storage.get(key)); - if (current.revision !== request.revision) return []; - const withoutFencedStarts = await dropFencedStarts(tokens, storage, { - prefix: iosStartPrefix, - key: iosStartKey, - }); - // Empty work can retry ends. Eligible work excludes every accepted or uncertain end. - if (!eligible) return withoutFencedStarts; - const retiring = await Promise.all( - withoutFencedStarts.map(async ({ token, kind }) => - kind === 'ios_activity' - ? (await storage.list({ prefix: iosEndPrefix(token), limit: 1 })).size > 0 - : false - ) + try { + await deliverGlanceableSnapshot(scope, { + ...deps, + buildSnapshot: async () => committed, + apnsTimestampSeconds: request.apnsTimestampSeconds, + isCurrent: async () => { + const current = refreshStateSchema.parse(await storage.get(key)); + return current.revision === request.revision; + }, + listIosActivityTokens: async (userId, organizationId) => { + const tokens = await deps.listIosActivityTokens(userId, organizationId); + const current = refreshStateSchema.parse(await storage.get(key)); + if (current.revision !== request.revision) return []; + const withoutFencedStarts = await dropFencedStarts(tokens, storage, { + prefix: iosStartPrefix, + key: iosStartKey, + }); + // Empty work can retry ends. Eligible work excludes every accepted or uncertain end. + if (!eligible) return withoutFencedStarts; + const retiring = await Promise.all( + withoutFencedStarts.map(async ({ token, kind }) => + kind === 'ios_activity' + ? (await storage.list({ prefix: iosEndPrefix(token), limit: 1 })).size > 0 + : false + ) + ); + return withoutFencedStarts.filter((_, index) => !retiring[index]); + }, + onIosStarted: async token => { + // Hold the fence for the whole maximum life of the card it raised. An + // orphan card cannot be ended remotely, so a second one would simply sit + // beside it until ActivityKit dismisses them both. + await storage.put(iosStartKey(token), Date.now() + GLANCEABLE_SNAPSHOT_EXPIRY_MS); + }, + beforeIosEnd: async token => { + return storage.transaction(async tx => { + const current = refreshStateSchema.parse(await tx.get(key)); + if (current.revision !== request.revision) return false; + // Each revision sends at most one end per token. Keep its obligation separate. + await tx.put(`${iosEndPrefix(token)}${key}:${request.revision}`, true); + return true; + }); + }, + onIosEndRejected: async token => { + // A delayed rejection releases only its attempt, not another pending or accepted end. + await storage.delete(`${iosEndPrefix(token)}${key}:${request.revision}`); + }, + }); + } catch (error) { + // A failed attempt still spent the window: the send reached for every + // device (or the transport is down for all of them), and without a + // recorded window every later change inside GLANCEABLE_DELIVERY_MIN_INTERVAL_MS + // retries the whole build+send at once — a failing transport turns each + // session flip into another burst of builds and device wakes. One attempt + // per window per scope, and the trailing refresh still carries the final + // counts at the window end. Re-check the revision first so a superseded + // attempt opens no window (same rule as the delivered path below), and + // keep any pending trailing refresh so the deferred change still lands. + // The error propagates: the entrypoints' existing failure logs stay the + // per-attempt evidence. + const current = refreshStateSchema.optional().parse(await storage.get(key)); + if (current?.revision === request.revision) { + await storage.put(deliveryKey, { deliveredAt: nowMs(), outcome: 'failed' }); + } + throw error; + } + + // A superseded delivery sent nothing: a newer revision already owns the + // surface, so opening a window or cancelling its trailing refresh here would + // drop the change that superseded this one. Re-check the revision first. + const current = refreshStateSchema.optional().parse(await storage.get(key)); + if (current?.revision !== request.revision) return; + + // A delivered snapshot starts the next window and cancels the trailing + // refresh it superseded. A deferral written while this delivery was in flight + // (an approval-exempt delivery can run inside an open window) is not in the + // snapshot, so keep it: its counts must still land on the trailing alarm. + await storage.put(deliveryKey, { deliveredAt: nowMs(), outcome: 'delivered' }); + const pendingAfterDelivery = pendingRefreshSchema + .optional() + .parse(await storage.get(pendingKey(scope))); + if (isSameDeferral(pendingBeforeDelivery, pendingAfterDelivery)) { + await storage.delete(pendingKey(scope)); + } + // One delivery event per window per scope; the trailing flush's line carries + // the final counts so a deferred burst settles on them. + console.log({ + event: 'glanceable_delivery', + scope: [scope.userId, scope.organizationId], + revision: request.revision, + trailing: options.trailing === true, + status: committed.status, + running: committed.running, + needsInput: committed.needsInput, + idle: committed.idle, + needsApproval: committed.needsApproval ?? 0, + }); +} + +/** + * Deliver every deferred refresh whose window has elapsed, then report the + * earliest deadline still pending so the caller can reschedule its alarm. + * + * A scope that throws must not abort the sweep or the DO's idem GC, so each + * refresh is isolated. The pending record is consumed before the refresh runs, + * so a refresh that throws (a rejected build or a rejected transport) re-arms + * the record here: a build that returns no snapshot re-arms itself, and a + * record written while the sweep runs is caught by the second list. Either way + * the deferred change keeps a deadline instead of being dropped. + */ +export async function flushDueGlanceableRefreshes( + storage: DurableObjectStorage, + deps: GlanceableDeliveryDeps, + nowMs: () => number = Date.now +): Promise { + const pending = await storage.list({ prefix: PENDING_PREFIX }); + for (const [key, record] of pending) { + if (record.dueAt > nowMs()) continue; + await storage.delete(key); + try { + await refreshGlanceableSnapshot( + { userId: record.userId, organizationId: record.organizationId }, + storage, + deps, + nowMs, + { trailing: true } ); - return withoutFencedStarts.filter((_, index) => !retiring[index]); - }, - onIosStarted: async token => { - // Hold the fence for the whole maximum life of the card it raised. An - // orphan card cannot be ended remotely, so a second one would simply sit - // beside it until ActivityKit dismisses them both. - await storage.put(iosStartKey(token), Date.now() + GLANCEABLE_SNAPSHOT_EXPIRY_MS); - }, - beforeIosEnd: async token => { - return storage.transaction(async tx => { - const current = refreshStateSchema.parse(await tx.get(key)); - if (current.revision !== request.revision) return false; - // Each revision sends at most one end per token. Keep its obligation separate. - await tx.put(`${iosEndPrefix(token)}${key}:${request.revision}`, true); - return true; + } catch (error) { + console.warn('Glanceable trailing refresh failed', { + scope: [record.userId, record.organizationId], + error: error instanceof Error ? error.message : String(error), }); - }, - onIosEndRejected: async token => { - // A delayed rejection releases only its attempt, not another pending or accepted end. - await storage.delete(`${iosEndPrefix(token)}${key}:${request.revision}`); - }, - }); + // The refresh threw before it could deliver or re-arm itself: the build + // rejected (a network/DNS failure on the snapshot route) or the transport + // rejected. The pending record was consumed above, so re-arm the next + // window or the deferred counts are dropped with no alarm left to retry + // them. A record written while the refresh ran already owns the key and a + // later deadline; only an empty slot is re-armed. The next window, not + // now, so a persistently failing route is retried once per window instead + // of spinning the alarm. + if ((await storage.get(key)) === undefined) { + const now = nowMs(); + await storage.put(key, { + userId: record.userId, + organizationId: record.organizationId, + dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: now, + }); + } + } + } + + // Re-list so a record written during the sweep is not stranded. + return earliestPendingGlanceableRefresh(storage); +} + +/** + * Earliest `dueAt` among the pending refreshes still stored, or null when none + * remains. The alarm sweep re-reads this after its awaits so a deferral that + * landed mid-sweep is not overwritten by the alarm it schedules. + */ +export async function earliestPendingGlanceableRefresh( + storage: DurableObjectStorage +): Promise { + const remaining = await storage.list({ prefix: PENDING_PREFIX }); + let earliest: number | null = null; + for (const [, record] of remaining) { + if (earliest === null || record.dueAt < earliest) earliest = record.dueAt; + } + return earliest; +} + +/** + * Fold the earliest pending glanceable deadline into the alarm the sweep chose. + * The sweep awaits between choosing `candidate` and setting it, so a deferral + * that landed in between must not be overwritten by the later `candidate`. + */ +export async function foldPendingGlanceableRefreshDeadline( + storage: DurableObjectStorage, + candidate: number | undefined +): Promise { + const pending = await earliestPendingGlanceableRefresh(storage); + if (pending === null) return candidate; + return candidate === undefined || pending < candidate ? pending : candidate; } /** diff --git a/services/session-ingest/src/dos/UserConnectionDO.test.ts b/services/session-ingest/src/dos/UserConnectionDO.test.ts index 7507c864f6..481cc106e1 100644 --- a/services/session-ingest/src/dos/UserConnectionDO.test.ts +++ b/services/session-ingest/src/dos/UserConnectionDO.test.ts @@ -6,6 +6,7 @@ import { import { getWorkerDb } from '@kilocode/db/client'; import { drizzle } from 'drizzle-orm/pg-proxy'; import { NotificationChannelDO, NotificationsService } from '../../../notifications/src/index'; +import { GLANCEABLE_DELIVERY_MIN_INTERVAL_MS } from '../../../notifications/src/lib/glanceable-refresh'; import { sendPushNotifications, type ExpoPushMessage, @@ -84,9 +85,10 @@ function createMockWs(tags: string[] = [], attachment?: unknown): MockWS { return ws; } -/** In-memory Map-backed KV fake for ctx.storage (put/get/delete/list). */ +/** In-memory Map-backed KV fake for ctx.storage (put/get/delete/list/alarm). */ function makeStorageFake() { const store = new Map(); + let alarmTime: number | null = null; return { store, kv: { @@ -98,7 +100,9 @@ function makeStorageFake() { list: (opts?: { prefix?: string }) => new Map([...store].filter(([key]) => key.startsWith(opts?.prefix ?? ''))), }, - deleteAlarm: vi.fn(async () => undefined), + deleteAlarm: vi.fn(async () => { + alarmTime = null; + }), put: vi.fn(async (key: string, value: unknown) => { store.set(key, value); }), @@ -118,7 +122,12 @@ function makeStorageFake() { } return result; }), - setAlarm: vi.fn(), + // The glanceable deferral reads the current alarm before re-arming it, so + // the fake must model the alarm rather than return `undefined`. + getAlarm: vi.fn(async () => alarmTime), + setAlarm: vi.fn(async (scheduledTime: number | Date) => { + alarmTime = typeof scheduledTime === 'number' ? scheduledTime : scheduledTime.getTime(); + }), }; } @@ -168,6 +177,24 @@ async function flushAsync(): Promise { }); } +/** + * The aggregate delivery coordinator wakes a device at most once per account + * scope per `GLANCEABLE_DELIVERY_MIN_INTERVAL_MS`, deferring a change inside the + * window to the Durable Object alarm. The glanceable cases below assert the + * connection DO's own per-status-change trigger, so step the wall clock past + * the window between heartbeats. The window itself is covered by + * `services/notifications/src/lib/glanceable-refresh.test.ts`. + */ +function useDeliveryWindowClock(): { tick: () => void } { + let now = Date.now(); + vi.spyOn(Date, 'now').mockImplementation(() => now); + return { + tick: () => { + now += GLANCEABLE_DELIVERY_MIN_INTERVAL_MS + 1_000; + }, + }; +} + function makeSession( id: string, status = 'busy', @@ -216,6 +243,10 @@ function setup(env: Partial = {}) { return { doInstance, ctx, mockCtx }; } +function pendingGlanceableKey(userId: string, organizationId: string | null): string { + return `glanceable-pending:${JSON.stringify([userId, organizationId])}`; +} + function setupGlanceableDelivery(foreignSessionIds: string[] = []) { const messages: ExpoPushMessage[] = []; vi.mocked(getWorkerDb).mockReturnValue( @@ -269,7 +300,7 @@ function setupGlanceableDelivery(foreignSessionIds: string[] = []) { }) ); }); - return { ...result, env, messages }; + return { ...result, env, messages, channelStorage: storage }; } function connectWebSocket(doInstance: UserConnectionDO, connectionId: string): MockWS { @@ -576,7 +607,9 @@ describe('UserConnectionDO', () => { it('delivers rowless personal busy, retry, attention-clear, and idle heartbeats through the real coordinator', async () => { const { doInstance, mockCtx, messages } = setupGlanceableDelivery(); const cliWs = addCliSocket(mockCtx, 'cli-1', [], undefined, 'usr_1'); + const clock = useDeliveryWindowClock(); for (const status of ['busy', 'retry', 'question', 'busy', 'idle']) { + clock.tick(); sendHeartbeat(doInstance, cliWs, [makeSession('s1', status)]); await flushAsync(); } @@ -602,6 +635,46 @@ describe('UserConnectionDO', () => { expect(messages.every(message => message.data?.organizationBound === false)).toBe(true); }); + it('delivers a question -> permission move inside the delivery window', async () => { + const { doInstance, mockCtx, messages } = setupGlanceableDelivery(); + const cliWs = addCliSocket(mockCtx, 'cli-1', [], undefined, 'usr_1'); + const clock = useDeliveryWindowClock(); + clock.tick(); + sendHeartbeat(doInstance, cliWs, [makeSession('s1', 'question')]); + await flushAsync(); + expect(messages).toHaveLength(1); + // No clock tick: still inside the delivery window. The Approve control + // gates nothing here except this window, so the move must not wait it out. + sendHeartbeat(doInstance, cliWs, [makeSession('s1', 'permission')]); + await flushAsync(); + expect(messages.map(message => message.data)).toMatchObject([ + { needsInput: 1, needsApproval: 0 }, + { needsInput: 1, needsApproval: 1 }, + ]); + }); + + it('defers a counts-only move inside the delivery window and arms the trailing alarm', async () => { + const { doInstance, mockCtx, messages, channelStorage } = setupGlanceableDelivery(); + const cliWs = addCliSocket(mockCtx, 'cli-1', [], undefined, 'usr_1'); + const clock = useDeliveryWindowClock(); + clock.tick(); + sendHeartbeat(doInstance, cliWs, [makeSession('s1')]); + await flushAsync(); + expect(messages).toHaveLength(1); + sendHeartbeat(doInstance, cliWs, [makeSession('s1', 'idle')]); + await flushAsync(); + // Deferred to the trailing alarm, not delivered on the spot. + expect(messages).toHaveLength(1); + // The deferral is stored and the alarm is armed at its deadline. Without + // both, the trailing delivery that lands the final counts never runs and + // the deferral is a silent drop. + const pending = (await channelStorage.get(pendingGlanceableKey('usr_1', null))) as + | { dueAt: number } + | undefined; + expect(pending).toMatchObject({ userId: 'usr_1', organizationId: null }); + expect(channelStorage.setAlarm).toHaveBeenCalledWith(pending?.dueAt); + }); + it('does not authorize a foreign-owned row from a real authenticated heartbeat', async () => { const { doInstance, mockCtx, messages } = setupGlanceableDelivery(['foreign']); const cliWs = addCliSocket(mockCtx, 'cli-1', [], undefined, 'usr_1'); @@ -614,18 +687,23 @@ describe('UserConnectionDO', () => { it('resends only when a reorder, rename, or child attention changes the roots', async () => { const { doInstance, mockCtx, messages } = setupGlanceableDelivery(); const cliWs = addCliSocket(mockCtx, 'cli-1', [], undefined, 'usr_1'); + const clock = useDeliveryWindowClock(); + clock.tick(); sendHeartbeat(doInstance, cliWs, [makeSession('s1'), makeSession('s2', 'retry')]); await flushAsync(); // A reorder and a rename leave every root status unchanged: no resend. + clock.tick(); sendHeartbeat(doInstance, cliWs, [makeSession('s2', 'retry', 'Renamed'), makeSession('s1')]); await flushAsync(); // A child raise hoists NEEDS INPUT onto its root, so the counts change. + clock.tick(); sendHeartbeat(doInstance, cliWs, [ makeSession('s1'), makeSession('s2', 'retry'), makeSession('child', 'question', 'Child', 's1'), ]); await flushAsync(); + clock.tick(); sendHeartbeat(doInstance, cliWs, [ makeSession('s1'), makeSession('s2', 'retry'), @@ -664,8 +742,11 @@ describe('UserConnectionDO', () => { it('delivers an empty aggregate when a root disappears from the heartbeat', async () => { const { doInstance, mockCtx, messages } = setupGlanceableDelivery(); const cliWs = addCliSocket(mockCtx, 'cli-1', [], undefined, 'usr_1'); + const clock = useDeliveryWindowClock(); + clock.tick(); sendHeartbeat(doInstance, cliWs, [makeSession('s1')]); await flushAsync(); + clock.tick(); sendHeartbeat(doInstance, cliWs, []); await flushAsync(); expect(messages.map(message => message.data)).toMatchObject([ @@ -679,10 +760,16 @@ describe('UserConnectionDO', () => { async listed => { const { doInstance, mockCtx, ctx, messages } = setupGlanceableDelivery(); const cliWs = addCliSocket(mockCtx, 'cli-1', [], undefined, 'usr_1'); + const clock = useDeliveryWindowClock(); + clock.tick(); sendHeartbeat(doInstance, cliWs, [makeSession('s1', 'question')]); await flushAsync(); messages.length = 0; if (!listed) mockCtx.removeSocket(cliWs); + // The disconnect's empty aggregate is a counts-only change and the + // preceding heartbeat consumed the delivery window, so step the clock + // past it; otherwise the refresh is deferred to the alarm. + clock.tick(); await disconnectCli(doInstance, cliWs); await flushAsync(); // The raise is held, not cleared: no delegate write happens on the @@ -695,6 +782,36 @@ describe('UserConnectionDO', () => { } ); + it('names the owning root when a disconnecting CLI owned a permission subagent', async () => { + // A subagent raise carries `permission` on the child row and is only + // hoisted onto its root for display. The disconnect caller names root ids + // in `cliSessionIds`, so naming the child id in `approvalChangedSessionIds` + // would be unknown to the server's batch query, which resolves it to the + // personal scope — the org scope whose permission cleared would lose the + // delivery-window exemption and the Approve control would lag a window. + const { doInstance, mockCtx, env } = setupGlanceableDelivery(); + const service = env.NOTIFICATIONS as unknown as NotificationsService; + const refreshParams: unknown[] = []; + const spy = async (params: unknown) => { + refreshParams.push(params); + }; + service.refreshGlanceableSessions = spy as never; + const cliWs = addCliSocket(mockCtx, 'cli-1', [], undefined, 'usr_1'); + sendHeartbeat(doInstance, cliWs, [ + makeSession('root'), + makeSession('child', 'permission', 'Child', 'root'), + ]); + await flushAsync(); + refreshParams.length = 0; // the heartbeat's own refresh is a different case + await disconnectCli(doInstance, cliWs); + await flushAsync(); + // The named ids must stay a subset of `cliSessionIds`: the root owns the + // child's raise, so it is the root's scope that actually moved. + expect(refreshParams).toEqual([ + { userId: 'usr_1', cliSessionIds: ['root'], approvalChangedSessionIds: ['root'] }, + ]); + }); + it.each(['cli-1', 'cli-2'])( 'does not send a stale close after replacement by %s', async replacementId => { diff --git a/services/session-ingest/src/dos/UserConnectionDO.ts b/services/session-ingest/src/dos/UserConnectionDO.ts index 304e604402..02a51f02c3 100644 --- a/services/session-ingest/src/dos/UserConnectionDO.ts +++ b/services/session-ingest/src/dos/UserConnectionDO.ts @@ -762,16 +762,32 @@ export class UserConnectionDO extends DurableObject { if (attachment.kiloUserId) { const changedSessionIds = new Set(); + // `needsApproval` gates the Approve control on the locked surfaces, so a + // move into or out of `permission` must bypass the delivery window. Only + // the sessions that moved are named: this batch aggregates every + // connection, so it can span the personal scope and several orgs, and the + // exemption must not reach a scope that had no approval change. + const approvalChangedSessionIds = new Set(); for (const session of this.aggregateSessions()) { - if (previousStatuses.get(session.id) !== session.status) changedSessionIds.add(session.id); + const previous = previousStatuses.get(session.id); + if (previous !== session.status) { + changedSessionIds.add(session.id); + if (previous === 'permission' || session.status === 'permission') { + approvalChangedSessionIds.add(session.id); + } + } previousStatuses.delete(session.id); } - for (const sessionId of previousStatuses.keys()) changedSessionIds.add(sessionId); + for (const [sessionId, previous] of previousStatuses) { + changedSessionIds.add(sessionId); + if (previous === 'permission') approvalChangedSessionIds.add(sessionId); + } if (changedSessionIds.size > 0) { this.ctx.waitUntil( refreshGlanceableSessions(this.env, { userId: attachment.kiloUserId, cliSessionIds: [...changedSessionIds], + approvalChangedSessionIds: [...approvalChangedSessionIds], }) ); } @@ -1916,10 +1932,26 @@ export class UserConnectionDO extends DurableObject { .filter(session => !session.parentSessionId && ownedSessions.has(session.id)) .map(session => session.id); if (attachment.kiloUserId && rootSessionIds.length > 0) { + // A subagent raise carries `permission` on the child row and is only + // hoisted onto its root for display, so the scope the Approve control + // moves in is the root's. Name the owning root: an id outside + // `cliSessionIds` is unknown to the server's batch query, which resolves + // it to the personal scope and leaves the org scope whose permission + // cleared stuck behind the delivery window. + const permissionRootIds = new Set( + sessions + .filter(session => ownedSessions.has(session.id) && session.status === 'permission') + .map(session => session.parentSessionId ?? session.id) + ); this.ctx.waitUntil( refreshGlanceableSessions(this.env, { userId: attachment.kiloUserId, cliSessionIds: rootSessionIds, + // A disconnecting CLI drops a permission wait to `retry`, which + // clears the Approve control on the locked surfaces. The in-memory + // status is the last one the CLI reported; the attention reset above + // writes the DB but does not touch this list. + approvalChangedSessionIds: rootSessionIds.filter(id => permissionRootIds.has(id)), }) ); } diff --git a/services/session-ingest/src/ingest/metadata.test.ts b/services/session-ingest/src/ingest/metadata.test.ts index d38186cc44..fbdea8bcc4 100644 --- a/services/session-ingest/src/ingest/metadata.test.ts +++ b/services/session-ingest/src/ingest/metadata.test.ts @@ -293,10 +293,12 @@ function createApplyMetadataDb(options: ApplyMetadataDbOptions = {}) { function metadataDelivery(db: ReturnType) { const messages: ExpoPushMessage[] = []; const tasks: Promise[] = []; + const refreshParams: RefreshGlanceableSessionsParams[] = []; const env = { HYPERDRIVE: { connectionString: 'postgres://unused' }, NOTIFICATIONS: { async refreshGlanceableSessions(params: RefreshGlanceableSessionsParams) { + refreshParams.push(params); if (params.userId !== 'usr_1' || !params.cliSessionIds.includes('ses_1')) return; const row = db.readCommittedSession(); await deliverGlanceableSnapshot( @@ -329,6 +331,7 @@ function metadataDelivery(db: ReturnType) { env, messages, tasks, + refreshParams, ctx: { waitUntil: (task: Promise) => { tasks.push(task); @@ -614,6 +617,60 @@ describe('applyMetadataChanges', () => { } ); + it('marks a permission move as approval-relevant for the delivery window', async () => { + const db = createApplyMetadataDb({ initialStatus: 'question' }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + const delivery = metadataDelivery(db); + await applyMetadataChanges( + delivery.env as never, + 'usr_1', + 'ses_1', + new Map([['status', 'permission']]), + delivery.ctx + ); + await Promise.all(delivery.tasks); + expect(delivery.refreshParams).toEqual([ + { userId: 'usr_1', cliSessionIds: ['ses_1'], approvalChangedSessionIds: ['ses_1'] }, + ]); + }); + + it('marks a cleared permission wait as approval-relevant for the delivery window', async () => { + // `permission -> busy` leaves the `session.status === 'permission'` clause + // false, so only `previousStatus === 'permission'` can exempt the clearing + // move: the Approve control must disappear as promptly as it appears. + const db = createApplyMetadataDb({ initialStatus: 'permission' }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + const delivery = metadataDelivery(db); + await applyMetadataChanges( + delivery.env as never, + 'usr_1', + 'ses_1', + new Map([['status', 'busy']]), + delivery.ctx + ); + await Promise.all(delivery.tasks); + expect(delivery.refreshParams).toEqual([ + { userId: 'usr_1', cliSessionIds: ['ses_1'], approvalChangedSessionIds: ['ses_1'] }, + ]); + }); + + it('does not exempt a counts-only status move from the delivery window', async () => { + const db = createApplyMetadataDb({ initialStatus: 'idle' }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + const delivery = metadataDelivery(db); + await applyMetadataChanges( + delivery.env as never, + 'usr_1', + 'ses_1', + new Map([['status', 'busy']]), + delivery.ctx + ); + await Promise.all(delivery.tasks); + expect(delivery.refreshParams).toEqual([ + { userId: 'usr_1', cliSessionIds: ['ses_1'], approvalChangedSessionIds: [] }, + ]); + }); + it('keeps committed ingestion successful when aggregate transport fails', async () => { const db = createApplyMetadataDb(); vi.mocked(getWorkerDb).mockReturnValue(db as never); diff --git a/services/session-ingest/src/ingest/metadata.ts b/services/session-ingest/src/ingest/metadata.ts index 4810e78bf6..209482c547 100644 --- a/services/session-ingest/src/ingest/metadata.ts +++ b/services/session-ingest/src/ingest/metadata.ts @@ -445,6 +445,14 @@ export async function applyMetadataChanges( const delivery = refreshGlanceableSessions(env, { userId: kiloUserId, cliSessionIds: [sessionId], + // A permission wait appearing or clearing gates the Approve control on + // the locked/background surfaces, so it must not wait for the shared + // delivery window. This caller is the one that saw the previous status. + approvalChangedSessionIds: + notification.previousStatus === 'permission' || + notification.session.status === 'permission' + ? [sessionId] + : [], }); if (ctx) ctx.waitUntil(delivery); else await delivery; From bc68be1f53dac37fe75040027ed5679d83f566e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 20 Sep 2026 18:52:47 +0000 Subject: [PATCH 2/6] fix: kwf-fix-ci-fix-d386 patch delivery --- .../src/lib/glanceable-refresh.test.ts | 103 ++++++++++++++++++ .../src/lib/glanceable-refresh.ts | 80 +++++++++----- 2 files changed, 157 insertions(+), 26 deletions(-) diff --git a/services/notifications/src/lib/glanceable-refresh.test.ts b/services/notifications/src/lib/glanceable-refresh.test.ts index f9cbf6fa6d..6e69160120 100644 --- a/services/notifications/src/lib/glanceable-refresh.test.ts +++ b/services/notifications/src/lib/glanceable-refresh.test.ts @@ -419,6 +419,109 @@ describe('refreshGlanceableSnapshot delivery window', () => { }); }); + it('re-arms a superseded trailing refresh whose own build succeeded', async () => { + const h = makeHarness(); + const base = 95_000_000; + let now = base; + const scope = { userId: 'user-superseded-built', organizationId: null }; + const key = pendingKey('user-superseded-built', null); + await h.storage.put(key, { + userId: 'user-superseded-built', + organizationId: null, + dueAt: now - 1, + }); + + // The trailing build stalls, so a newer refresh bumps the revision while it + // is in flight. That refresh's own build returns no snapshot, so it delivers + // nothing and records nothing: only the trailing refresh can keep the + // deferred counts alive. + const started = Promise.withResolvers(); + const release = Promise.withResolvers(); + let build = 0; + h.deps.buildSnapshot = async () => { + build += 1; + if (build === 1) { + started.resolve(); + await release.promise; + return snapshot({ running: 5 }); + } + return null; + }; + + const trailing = flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now); + await started.promise; + + now = base + 1_000; + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(h.expoSends).toHaveLength(0); + + release.resolve(); + // The newer revision owns the surface, so the trailing fetch must not + // deliver; the deferred counts still need a pending record and a deadline + // instead of being dropped with no alarm left to retry them. + await expect(trailing).resolves.toBe(now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS); + expect(await h.storage.get(key)).toEqual({ + userId: 'user-superseded-built', + organizationId: null, + dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: now, + }); + }); + + it('re-arms a superseded trailing refresh when the concurrent attempt failed at the transport', async () => { + const h = makeHarness(); + const base = 96_000_000; + let now = base; + const scope = { userId: 'user-superseded-built-failed', organizationId: null }; + const key = pendingKey('user-superseded-built-failed', null); + await h.storage.put(key, { + userId: 'user-superseded-built-failed', + organizationId: null, + dueAt: now - 1, + }); + + // The trailing build stalls, so a newer refresh supersedes it and attempts + // its delivery. The transport rejects, and that failure still writes the + // delivery record: the record alone cannot tell a landed delivery from a + // spent window. + const started = Promise.withResolvers(); + const release = Promise.withResolvers(); + let build = 0; + h.deps.buildSnapshot = async () => { + build += 1; + if (build === 1) { + started.resolve(); + await release.promise; + return snapshot({ running: 5 }); + } + return snapshot({ running: 6 }); + }; + + const trailing = flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now); + await started.promise; + + now = base + 1_000; + h.failNextExpoPush(new Error('transport down')); + await expect( + refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now) + ).rejects.toThrow('transport down'); + expect(await h.storage.get(deliveryKey('user-superseded-built-failed', null))).toEqual({ + deliveredAt: now, + outcome: 'failed', + }); + + release.resolve(); + // No snapshot was delivered, so the deferred counts must keep a pending + // record and a deadline instead of being dropped with no alarm left. + await expect(trailing).resolves.toBe(now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS); + expect(await h.storage.get(key)).toEqual({ + userId: 'user-superseded-built-failed', + organizationId: null, + dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: now, + }); + }); + it('re-arms a stale past alarm so the trailing delivery is not stranded', async () => { const h = makeHarness(); const base = 60_000_000; diff --git a/services/notifications/src/lib/glanceable-refresh.ts b/services/notifications/src/lib/glanceable-refresh.ts index a3b775a4fc..52069d9048 100644 --- a/services/notifications/src/lib/glanceable-refresh.ts +++ b/services/notifications/src/lib/glanceable-refresh.ts @@ -51,6 +51,7 @@ const pendingRefreshSchema = z.object({ deferredAt: z.number().optional(), }); type PendingGlanceableRefresh = z.infer; +type DeliveryState = z.infer; const PENDING_PREFIX = 'glanceable-pending:'; @@ -72,6 +73,45 @@ function isSameDeferral( return before.dueAt === after.dueAt && before.deferredAt === after.deferredAt; } +/** + * Re-arm the deferred change a trailing refresh owes when nothing else carried + * it, so the flush's consumed pending record is replaced instead of dropped. + * + * `buildSnapshot` is awaited after the revision bump, so a concurrent refresh + * for this scope can deliver while the fetch is in flight. That delivery + * already covers the change; re-arming would leave a record its + * `isSameDeferral` check keeps and the alarm would later fire a redundant + * build+send. Skip the re-arm only when such a delivery actually landed, told + * by the outcome in the record it wrote: the failure branch writes the same + * record with `outcome: 'failed'` after spending the window, and treating that + * as a landed delivery would drop the deferred change with no pending record + * and no alarm left to retry it. A record without an outcome predates the field + * and only ever meant a delivery. + */ +async function rearmTrailingRefresh( + scope: { userId: string; organizationId: string | null }, + storage: DurableObjectStorage, + deliveryKey: string, + deliveryAtStart: DeliveryState | undefined, + nowMs: () => number +): Promise { + const landed = deliveryStateSchema.optional().parse(await storage.get(deliveryKey)); + if ( + landed !== undefined && + landed.outcome !== 'failed' && + landed.deliveredAt !== deliveryAtStart?.deliveredAt + ) { + return; + } + const now = nowMs(); + await storage.put(pendingKey(scope), { + userId: scope.userId, + organizationId: scope.organizationId, + dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: now, + }); +} + /** The user DO owns these records; no ordering or interval state lives in a Worker instance. */ export async function refreshGlanceableSnapshot( params: { userId: string; organizationId: string | null }, @@ -152,31 +192,7 @@ export async function refreshGlanceableSnapshot( // credentials fail, and the flush has already consumed the pending record, // so re-arm the next window instead of dropping the change with no alarm. if (options.trailing === true) { - // `buildSnapshot` was awaited after the revision bump, so a concurrent - // refresh for this scope can deliver while this fetch is in flight. That - // delivery already covers the change; re-arming here would leave a record - // its `isSameDeferral` check keeps and the alarm would later fire a - // redundant build+send. Skip the re-arm only when such a delivery actually - // landed, told by the outcome in the record it wrote: the failure branch - // writes the same record with `outcome: 'failed'` after spending the - // window, and treating that as a landed delivery would drop this deferred - // change with no pending record and no alarm left to retry it. A record - // without an outcome predates the field and only ever meant a delivery. - const landed = deliveryStateSchema.optional().parse(await storage.get(deliveryKey)); - if ( - landed !== undefined && - landed.outcome !== 'failed' && - landed.deliveredAt !== delivery?.deliveredAt - ) { - return; - } - const now = nowMs(); - await storage.put(pendingKey(scope), { - userId: scope.userId, - organizationId: scope.organizationId, - dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, - deferredAt: now, - }); + await rearmTrailingRefresh(scope, storage, deliveryKey, delivery, nowMs); } return; } @@ -197,7 +213,19 @@ export async function refreshGlanceableSnapshot( ).toISOString(), }; }); - if (committed === null) return; + if (committed === null) { + // A concurrent refresh bumped the revision while this build was in flight, + // so the newer revision owns the surface and this attempt must not deliver. + // It can still have delivered nothing — its own build returns no snapshot + // when the route fails, and a failed attempt only records a spent window — + // while the sweep has already consumed the pending record. Re-arm the + // deferred change unless a delivery actually landed, exactly as the + // no-snapshot branch above does. + if (options.trailing === true) { + await rearmTrailingRefresh(scope, storage, deliveryKey, delivery, nowMs); + } + return; + } // Content-free success evidence for the one-build-per-window invariant // (§4.15 rules: identifiers and aggregate counts, never session content). From d7fff568a84fcc56739fd7345864bd69b20b3ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 20 Sep 2026 21:53:43 +0000 Subject: [PATCH 3/6] fix: kwf-fix-ci-fix-60d9 patch delivery --- .../src/dos/NotificationChannelDO.ts | 9 +- .../src/lib/glanceable-refresh.test.ts | 147 +++++++++++++++++- .../src/lib/glanceable-refresh.ts | 134 +++++++++++----- 3 files changed, 248 insertions(+), 42 deletions(-) diff --git a/services/notifications/src/dos/NotificationChannelDO.ts b/services/notifications/src/dos/NotificationChannelDO.ts index 07f169f26e..11111db635 100644 --- a/services/notifications/src/dos/NotificationChannelDO.ts +++ b/services/notifications/src/dos/NotificationChannelDO.ts @@ -20,7 +20,7 @@ import { sendPushNotifications } from '../lib/expo-push'; import { glanceableDeliveryDeps } from '../lib/glanceable-delivery-deps'; import { foldPendingGlanceableRefreshDeadline, - flushDueGlanceableRefreshes, + flushDueGlanceableRefreshesSafely, refreshGlanceableSnapshot, } from '../lib/glanceable-refresh'; import { expoPushExtrasForPushData } from '../lib/push-message-extras'; @@ -484,8 +484,11 @@ export class NotificationChannelDO extends DurableObject { const now = Date.now(); // Deliver any glanceable refresh the rate-limit window deferred. Its // remaining deadline folds into this sweep's alarm so the trailing - // delivery is not stranded when no idem/rl record outlives it. - const dueGlanceableRefreshAt = await flushDueGlanceableRefreshes( + // delivery is not stranded when no idem/rl record outlives it. The flush runs + // safely: a failure inside it must not skip the idem/rate-limit GC below, + // which is storage reclamation. The fold re-reads the pending deadlines, so + // a failed flush still reschedules the trailing delivery. + const dueGlanceableRefreshAt = await flushDueGlanceableRefreshesSafely( this.ctx.storage, glanceableDeliveryDeps(this.env) ); diff --git a/services/notifications/src/lib/glanceable-refresh.test.ts b/services/notifications/src/lib/glanceable-refresh.test.ts index 6e69160120..394c1baab8 100644 --- a/services/notifications/src/lib/glanceable-refresh.test.ts +++ b/services/notifications/src/lib/glanceable-refresh.test.ts @@ -6,6 +6,7 @@ import type { ActiveAgentsGlanceable, GlanceableDeliveryDeps } from './glanceabl import { foldPendingGlanceableRefreshDeadline, flushDueGlanceableRefreshes, + flushDueGlanceableRefreshesSafely, GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, refreshGlanceableSnapshot, } from './glanceable-refresh'; @@ -58,6 +59,18 @@ class FakeStorage { } } +/** Fails `list` for one prefix, as a transient storage error would. */ +class FailingListStorage extends FakeStorage { + constructor(private readonly failingPrefix: string) { + super(); + } + + override async list(options: { prefix?: string; limit?: number } = {}): Promise> { + if (options.prefix === this.failingPrefix) throw new Error('storage unavailable'); + return super.list(options); + } +} + function pendingKey(userId: string, organizationId: string | null): string { return `glanceable-pending:${JSON.stringify([userId, organizationId])}`; } @@ -316,6 +329,56 @@ describe('refreshGlanceableSnapshot delivery window', () => { expect(await h.storage.get(key)).toBeUndefined(); }); + it('cancels a re-armed trailing refresh when the superseding delivery lands afterwards', async () => { + const h = makeHarness(); + const base = 82_500_000; + let now = base; + const scope = { userId: 'user-rearm-race', organizationId: null }; + const key = pendingKey('user-rearm-race', null); + // The flush consumes this due record; its write time is what the re-arm + // must preserve so the later delivery still sees the change as covered. + await h.storage.put(key, { + userId: 'user-rearm-race', + organizationId: null, + dueAt: now - 1, + deferredAt: now - 2, + }); + + // The trailing build stalls so a concurrent refresh can bump the revision + // and reach its transport before the trailing fetch re-arms. + const started = Promise.withResolvers(); + const releaseBuild = Promise.withResolvers(); + let build = 0; + h.deps.buildSnapshot = async () => { + build += 1; + if (build === 1) { + started.resolve(); + await releaseBuild.promise; + return null; + } + return snapshot({ running: 5 }); + }; + + const trailing = flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now); + await started.promise; + + // The superseding delivery is in flight — revision bumped, delivery record + // not yet written — when the trailing fetch re-arms. + now = base + 1_000; + const gate = h.blockNextExpoPush(); + const delivery = refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + await gate.started; + releaseBuild.resolve(); + await trailing; + expect(await h.storage.get(key)).toMatchObject({ deferredAt: base - 2 }); + + // Its snapshot already covers the deferred change, so the re-arm must not + // survive the delivery as a redundant device wake. + gate.release(); + await delivery; + expect(await h.storage.get(key)).toBeUndefined(); + }); + it('re-arms a trailing refresh when a concurrent refresh only moves the revision', async () => { const h = makeHarness(); const base = 85_000_000; @@ -603,12 +666,14 @@ describe('refreshGlanceableSnapshot delivery window', () => { outcome: 'failed', }); // A throwing trailing delivery keeps the deferred counts: re-armed for the - // next window instead of dropped with no retry left. + // next window instead of dropped with no retry left. The re-arm keeps the + // deferral's original write time so a delivery that lands later still + // recognises it as covered. expect(await h.storage.get(pendingKey('user-failed-send', null))).toEqual({ userId: 'user-failed-send', organizationId: null, dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, - deferredAt: now, + deferredAt: base + 2_000, }); }); @@ -1024,6 +1089,29 @@ describe('refreshGlanceableSnapshot delivery window', () => { deferredAt: now + 1, }); }); + + it('contains a failing flush sweep so the caller can still run its own sweep', async () => { + const h = makeHarness(); + const now = 1_000; + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + const storage = new FailingListStorage('glanceable-pending:'); + await storage.put(pendingKey('user-sweep', null), { + userId: 'user-sweep', + organizationId: null, + dueAt: now, + }); + + // The sweep's own storage read rejects. The guard must absorb it so the + // caller's GC pass still runs instead of the alarm aborting here. + await expect( + flushDueGlanceableRefreshesSafely(asStorage(storage), h.deps, () => now) + ).resolves.toBeNull(); + expect(warnSpy).toHaveBeenCalledWith( + 'Glanceable trailing refresh sweep failed', + expect.objectContaining({ error: 'storage unavailable' }) + ); + expect(h.builds).toBe(0); + }); }); describe('NotificationChannelDO alarm glanceable flush', () => { @@ -1095,4 +1183,59 @@ describe('NotificationChannelDO alarm glanceable flush', () => { 5_000 ); }); + + it('runs the idem/rate-limit GC even when the glanceable flush fails', async () => { + const id = env.NOTIFICATION_CHANNEL_DO.idFromName('user-glanceable-gc'); + const stub = env.NOTIFICATION_CHANNEL_DO.get(id); + const now = Date.now(); + const dueAt = now + 30_000; + + await runInDurableObject(stub, async (_instance, state) => { + await state.storage.put('idem:expired', { stage: 'delivered', ts: now - 2 * 60 * 60 * 1000 }); + await state.storage.put('rl:expired', { expiresAt: now - 1_000, timestamps: [] }); + await state.storage.put('glanceable-pending:["user-glanceable-gc",null]', { + userId: 'user-glanceable-gc', + organizationId: null, + dueAt, + }); + }); + + await runInDurableObject(stub, async (instance, state) => { + // The flush's first pending-prefix read rejects; the guard must absorb it + // so the GC below still runs. Later reads (the fold) succeed. + const mutable = state.storage as unknown as { + list: (options?: { prefix?: string }) => Promise; + }; + const originalList = state.storage.list.bind(state.storage); + let pendingLists = 0; + mutable.list = (options?: { prefix?: string }) => { + if (options?.prefix === 'glanceable-pending:' && ++pendingLists === 1) { + return Promise.reject(new Error('storage unavailable')); + } + return originalList(options); + }; + try { + await (instance as unknown as { alarm: () => Promise }).alarm(); + } finally { + delete (mutable as { list?: unknown }).list; + } + }); + + const result = await runInDurableObject(stub, async (_instance, state) => ({ + idem: await state.storage.get('idem:expired'), + rl: await state.storage.get('rl:expired'), + pending: await state.storage.get<{ dueAt: number }>( + 'glanceable-pending:["user-glanceable-gc",null]' + ), + alarm: await state.storage.getAlarm(), + })); + + // A failing flush no longer skips the sweep's storage reclamation. + expect(result.idem).toBeUndefined(); + expect(result.rl).toBeUndefined(); + // The deferred counts are not dropped: the record and its deadline survive + // the failed flush, so the trailing delivery is rescheduled. + expect(result.pending?.dueAt).toBe(dueAt); + expect(result.alarm).toBe(dueAt); + }); }); diff --git a/services/notifications/src/lib/glanceable-refresh.ts b/services/notifications/src/lib/glanceable-refresh.ts index 52069d9048..e44c592320 100644 --- a/services/notifications/src/lib/glanceable-refresh.ts +++ b/services/notifications/src/lib/glanceable-refresh.ts @@ -79,36 +79,55 @@ function isSameDeferral( * * `buildSnapshot` is awaited after the revision bump, so a concurrent refresh * for this scope can deliver while the fetch is in flight. That delivery - * already covers the change; re-arming would leave a record its - * `isSameDeferral` check keeps and the alarm would later fire a redundant - * build+send. Skip the re-arm only when such a delivery actually landed, told - * by the outcome in the record it wrote: the failure branch writes the same - * record with `outcome: 'failed'` after spending the window, and treating that - * as a landed delivery would drop the deferred change with no pending record - * and no alarm left to retry it. A record without an outcome predates the field - * and only ever meant a delivery. + * already covers the change; re-arming would leave a record the alarm later + * fires a redundant build+send for. Two things keep that from happening: + * + * - The delivery record is read and the re-arm is written in one transaction, + * the same slot the superseding delivery writes. If the delivery commits + * first this read sees it and skips; if the re-arm commits first the + * delivery's own landing transaction sees the re-arm and removes it. A plain + * read-then-write pair would let the delivery land in between and leave a + * record behind. + * - The re-arm keeps the deferral's original write time in `deferredAt`. The + * landing transaction cancels a pending record whose change predates the + * delivery's revision, so a re-arm written while the delivery was in flight + * is still recognised as superseded. Without the preserved time its own + * `deferredAt` would look newer than the snapshot and survive. + * + * Skip the re-arm when a delivery actually landed, told by the outcome in the + * record it wrote: the failure branch writes the same record with + * `outcome: 'failed'` after spending the window, and treating that as a landed + * delivery would drop the deferred change with no pending record and no alarm + * left to retry it. A record without an outcome predates the field and only + * ever meant a delivery. A pending record already in the slot is a newer + * deferral this flush did not consume; leave its deadline alone. */ async function rearmTrailingRefresh( scope: { userId: string; organizationId: string | null }, storage: DurableObjectStorage, deliveryKey: string, deliveryAtStart: DeliveryState | undefined, + deferredChangeAt: number | undefined, nowMs: () => number ): Promise { - const landed = deliveryStateSchema.optional().parse(await storage.get(deliveryKey)); - if ( - landed !== undefined && - landed.outcome !== 'failed' && - landed.deliveredAt !== deliveryAtStart?.deliveredAt - ) { - return; - } + const pendingK = pendingKey(scope); const now = nowMs(); - await storage.put(pendingKey(scope), { - userId: scope.userId, - organizationId: scope.organizationId, - dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, - deferredAt: now, + await storage.transaction(async tx => { + const landed = deliveryStateSchema.optional().parse(await tx.get(deliveryKey)); + if ( + landed !== undefined && + landed.outcome !== 'failed' && + landed.deliveredAt !== deliveryAtStart?.deliveredAt + ) { + return; + } + if ((await tx.get(pendingK)) !== undefined) return; + await tx.put(pendingK, { + userId: scope.userId, + organizationId: scope.organizationId, + dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: deferredChangeAt ?? now, + }); }); } @@ -118,7 +137,7 @@ export async function refreshGlanceableSnapshot( storage: DurableObjectStorage, deps: GlanceableDeliveryDeps, nowMs: () => number = Date.now, - options: { trailing?: boolean; approvalChanged?: boolean } = {} + options: { trailing?: boolean; approvalChanged?: boolean; deferredAt?: number } = {} ): Promise { const scope = scopeSchema.parse(params); const key = `glanceable:${JSON.stringify([scope.userId, scope.organizationId])}`; @@ -192,7 +211,7 @@ export async function refreshGlanceableSnapshot( // credentials fail, and the flush has already consumed the pending record, // so re-arm the next window instead of dropping the change with no alarm. if (options.trailing === true) { - await rearmTrailingRefresh(scope, storage, deliveryKey, delivery, nowMs); + await rearmTrailingRefresh(scope, storage, deliveryKey, delivery, options.deferredAt, nowMs); } return; } @@ -222,7 +241,7 @@ export async function refreshGlanceableSnapshot( // deferred change unless a delivery actually landed, exactly as the // no-snapshot branch above does. if (options.trailing === true) { - await rearmTrailingRefresh(scope, storage, deliveryKey, delivery, nowMs); + await rearmTrailingRefresh(scope, storage, deliveryKey, delivery, options.deferredAt, nowMs); } return; } @@ -318,16 +337,31 @@ export async function refreshGlanceableSnapshot( if (current?.revision !== request.revision) return; // A delivered snapshot starts the next window and cancels the trailing - // refresh it superseded. A deferral written while this delivery was in flight - // (an approval-exempt delivery can run inside an open window) is not in the - // snapshot, so keep it: its counts must still land on the trailing alarm. - await storage.put(deliveryKey, { deliveredAt: nowMs(), outcome: 'delivered' }); - const pendingAfterDelivery = pendingRefreshSchema - .optional() - .parse(await storage.get(pendingKey(scope))); - if (isSameDeferral(pendingBeforeDelivery, pendingAfterDelivery)) { - await storage.delete(pendingKey(scope)); - } + // refresh it superseded. The record write and the cancel are one transaction + // so the re-arm in `rearmTrailingRefresh` cannot interleave: whichever + // commits second sees the other and either skips or removes the record, so a + // re-arm can never survive as a redundant device wake. + const deliveredAt = nowMs(); + const deliveredRevisionAt = Date.parse(request.updatedAt); + await storage.transaction(async tx => { + await tx.put(deliveryKey, { deliveredAt, outcome: 'delivered' }); + const pendingAfterDelivery = pendingRefreshSchema + .optional() + .parse(await tx.get(pendingKey(scope))); + if (pendingAfterDelivery === undefined) return; + // A deferral whose change predates this delivery's revision is in the + // snapshot, whether it was the one this delivery superseded or a re-arm + // written while the delivery was in flight (a re-arm carries the original + // deferral's write time). A deferral written after the revision (an + // approval-exempt delivery can run inside an open window) is not in the + // snapshot, so keep it: its counts must still land on the trailing alarm. + const supersededByRevision = + pendingAfterDelivery.deferredAt !== undefined && + pendingAfterDelivery.deferredAt <= deliveredRevisionAt; + if (isSameDeferral(pendingBeforeDelivery, pendingAfterDelivery) || supersededByRevision) { + await tx.delete(pendingKey(scope)); + } + }); // One delivery event per window per scope; the trailing flush's line carries // the final counts so a deferred burst settles on them. console.log({ @@ -369,7 +403,7 @@ export async function flushDueGlanceableRefreshes( storage, deps, nowMs, - { trailing: true } + { trailing: true, deferredAt: record.deferredAt } ); } catch (error) { console.warn('Glanceable trailing refresh failed', { @@ -383,14 +417,17 @@ export async function flushDueGlanceableRefreshes( // them. A record written while the refresh ran already owns the key and a // later deadline; only an empty slot is re-armed. The next window, not // now, so a persistently failing route is retried once per window instead - // of spinning the alarm. + // of spinning the alarm. Keep the consumed record's write time: a delivery + // that lands later covers that change and cancels this record, while a + // fresh `now` would look newer than the delivery's snapshot and survive as + // a redundant wake. if ((await storage.get(key)) === undefined) { const now = nowMs(); await storage.put(key, { userId: record.userId, organizationId: record.organizationId, dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, - deferredAt: now, + deferredAt: record.deferredAt ?? now, }); } } @@ -400,6 +437,29 @@ export async function flushDueGlanceableRefreshes( return earliestPendingGlanceableRefresh(storage); } +/** + * `flushDueGlanceableRefreshes` with its own failures contained. The + * NotificationChannelDO alarm runs the flush before its idem/rate-limit GC, and + * that GC is storage reclamation that must not be skipped by a transient + * glanceable failure (a rejected `list`, a bug in the sweep). The alarm re-reads + * the pending deadlines after the GC, so a swallowed failure still reschedules + * the trailing delivery instead of stranding it. + */ +export async function flushDueGlanceableRefreshesSafely( + storage: DurableObjectStorage, + deps: GlanceableDeliveryDeps, + nowMs: () => number = Date.now +): Promise { + try { + return await flushDueGlanceableRefreshes(storage, deps, nowMs); + } catch (error) { + console.warn('Glanceable trailing refresh sweep failed', { + error: error instanceof Error ? error.message : String(error), + }); + return null; + } +} + /** * Earliest `dueAt` among the pending refreshes still stored, or null when none * remains. The alarm sweep re-reads this after its awaits so a deferral that From 52a8712daef83788bd600d7fc75cbc198da7a3cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 20 Sep 2026 21:57:16 +0000 Subject: [PATCH 4/6] style: apply the repo formatter --- services/notifications/src/lib/glanceable-refresh.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/notifications/src/lib/glanceable-refresh.test.ts b/services/notifications/src/lib/glanceable-refresh.test.ts index 394c1baab8..f650d5ced0 100644 --- a/services/notifications/src/lib/glanceable-refresh.test.ts +++ b/services/notifications/src/lib/glanceable-refresh.test.ts @@ -65,7 +65,9 @@ class FailingListStorage extends FakeStorage { super(); } - override async list(options: { prefix?: string; limit?: number } = {}): Promise> { + override async list( + options: { prefix?: string; limit?: number } = {} + ): Promise> { if (options.prefix === this.failingPrefix) throw new Error('storage unavailable'); return super.list(options); } From be9678c3cab4ecfcc9b79ea000f20f468012e9ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Mon, 21 Sep 2026 01:29:19 +0000 Subject: [PATCH 5/6] fix: kwf-fix-review-0c0f patch delivery --- .../src/lib/glanceable/publisher.test.ts | 106 ++++++++++++++ apps/mobile/src/lib/glanceable/publisher.ts | 118 ++++++++++++---- .../src/lib/glanceable/sink-registry.ts | 48 ++++++- .../src/lib/glanceable-refresh.test.ts | 129 +++++++++++++++++- .../src/lib/glanceable-refresh.ts | 121 ++++++++++++---- .../src/dos/UserConnectionDO.ts | 15 +- .../src/ingest/metadata.test.ts | 43 ++++++ .../session-ingest/src/ingest/metadata.ts | 21 ++- 8 files changed, 535 insertions(+), 66 deletions(-) diff --git a/apps/mobile/src/lib/glanceable/publisher.test.ts b/apps/mobile/src/lib/glanceable/publisher.test.ts index 5462d9a02b..e70a0719cb 100644 --- a/apps/mobile/src/lib/glanceable/publisher.test.ts +++ b/apps/mobile/src/lib/glanceable/publisher.test.ts @@ -13,7 +13,9 @@ import { import { getTerminalBlankEpoch, writeSignedOutSnapshotAndEnd } from './cleanup'; import { GLANCEABLE_RENEW_MARGIN_MS, + GLANCEABLE_RENEW_RETRY_MAX_MS, GlanceablePublisher, + glanceableRenewRetryDelayMs, hasSameGlanceableContent, } from './publisher'; import { @@ -295,6 +297,110 @@ describe('GlanceablePublisher', () => { publisher.dispose(); }); + it('spaces a rejected renewal retry with a doubling backoff instead of every heartbeat', () => { + // The wait doubles from one coalesce window to the five-minute ceiling, so a + // transient failure is retried promptly and a permanently broken surface is + // attempted at a bounded rate. + expect(glanceableRenewRetryDelayMs(0)).toBe(GLANCEABLE_COALESCE_MS); + expect(glanceableRenewRetryDelayMs(1)).toBe(GLANCEABLE_COALESCE_MS); + expect(glanceableRenewRetryDelayMs(2)).toBe(2 * GLANCEABLE_COALESCE_MS); + expect(glanceableRenewRetryDelayMs(20)).toBe(GLANCEABLE_RENEW_RETRY_MAX_MS); + + // A sink that rejects every write never advances the published deadline, so + // without a bound the unchanged-content renewal would re-emit on every + // heartbeat. + vi.useFakeTimers(); + let now = NOW; + let attempts = 0; + const sink: GlanceableSink = { + publish() { + // This case observes only the start/update attempts. + }, + startOrUpdate() { + attempts += 1; + if (attempts > 1) { + throw new Error('ActivityKit start failed'); + } + }, + endImmediate() { + // The counts never go empty here. + }, + }; + const publisher = new GlanceablePublisher({ sinks: [sink], now: () => now, coalesceMs: 1000 }); + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + expect(attempts).toBe(1); + + // The renewal at the margin is rejected and spends the first backoff: the + // next attempt waits one coalesce window, not a full renewal margin. + now += GLANCEABLE_RENEW_MARGIN_MS; + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + expect(attempts).toBe(2); + + // 179 more 10 s heartbeats span another 30 minutes. The retries space out + // 10 s, 20 s, 40 s ... to the five-minute ceiling, so the whole span takes + // eleven attempts rather than one per heartbeat. + for (let heartbeat = 0; heartbeat < 179; heartbeat += 1) { + now += 10_000; + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + } + expect(attempts).toBe(11); + publisher.dispose(); + }); + + it('backs off a rejected first write instead of letting every heartbeat through', () => { + // The restart reconciliation lets the first heartbeat write through even + // when nothing was published yet. If that first write is rejected, the + // latch must not keep letting every heartbeat through: the renewal backoff + // spaces the retries, exactly as it does once a frame has landed. + vi.useFakeTimers(); + let now = NOW; + let attempts = 0; + const sink: GlanceableSink = { + publish() { + // This case observes only the start/update attempts. + }, + startOrUpdate() { + attempts += 1; + throw new Error('ActivityKit start failed'); + }, + endImmediate() { + // The counts never go empty here. + }, + }; + const publisher = new GlanceablePublisher({ sinks: [sink], now: () => now, coalesceMs: 1000 }); + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + expect(attempts).toBe(1); + + // Inside the first backoff a heartbeat must not re-emit. + now += 1000; + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + expect(attempts).toBe(1); + + // Once the backoff elapses the renewal retries. + now += GLANCEABLE_COALESCE_MS - 1000; + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + expect(attempts).toBe(2); + publisher.dispose(); + }); + + it('renders the first heartbeat after a restart even when the persisted snapshot matches', () => { + // The native surfaces outlive the JS process. A death inside the 8 s + // terminal window leaves the ongoing card in the shade while the persisted + // snapshot is already empty, so an unchanged empty heartbeat must still + // reach the sinks: that is where the Android sink dismisses the orphan + // (`!notificationActive -> endNotification`). Suppressing it strands the + // card until the counts next change. + const { sink, calls } = makeSink(); + const restored = snapshotFor([], NOW - 60_000, 7); + const publisher = new GlanceablePublisher({ sinks: [sink], now: () => NOW, initial: restored }); + + publisher.handleSessions([], PUB_CTX); + + expect(count(calls, 'publish')).toBe(1); + expect(lastSnapshot(calls, 'publish').running).toBe(0); + publisher.dispose(); + }); + it('does not republish a pending coalesced frame after an unchanged renewal', () => { // A content change inside the coalesce window stores a snapshot dated at // that change. A renewal heartbeat before the timer fires publishes a newer diff --git a/apps/mobile/src/lib/glanceable/publisher.ts b/apps/mobile/src/lib/glanceable/publisher.ts index 6ac5543972..bfdedf28fd 100644 --- a/apps/mobile/src/lib/glanceable/publisher.ts +++ b/apps/mobile/src/lib/glanceable/publisher.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines -- one publisher state machine: derive, gate, renew, coalesce, and terminal end */ import { buildGlanceableSnapshot, GLANCEABLE_COALESCE_MS, @@ -16,6 +17,7 @@ import { type GlanceableSink, type GlanceableSinkContext, guardSink, + writeGlanceableFrame, } from './sink-registry'; import { getSurfaceExtras, setSurfaceExtras } from './surface-extras'; import { selectWaitingAsk, type WaitingAsk, type WaitingAskRow } from './waiting-ask'; @@ -86,6 +88,27 @@ type TimerHandle = ReturnType; */ export const GLANCEABLE_RENEW_MARGIN_MS = GLANCEABLE_STALE_MS / 2; +/** + * Ceiling for the renewal retry backoff. A sink that rejects every write never + * advances the published deadline, so without a bound the unchanged-content + * renewal would re-emit on every heartbeat — the in-app amplification the heat + * fix removed. The wait doubles from the coalesce window to this ceiling, so a + * transient ActivityKit failure is still retried within seconds while a + * permanently broken surface is attempted at most once per five minutes. + */ +export const GLANCEABLE_RENEW_RETRY_MAX_MS = 5 * 60_000; + +/** + * Wait before the next renewal attempt, from the number of consecutive writes + * whose sinks rejected them: one coalesce window for the first, then doubling + * up to the ceiling. A landed write clears the count, so a surface that + * recovers is back to the normal renewal cadence at once. + */ +export function glanceableRenewRetryDelayMs(failures: number): number { + const delay = GLANCEABLE_COALESCE_MS * 2 ** Math.max(0, failures - 1); + return Math.min(delay, GLANCEABLE_RENEW_RETRY_MAX_MS); +} + export class GlanceablePublisher { private readonly sinks: readonly GlanceableSink[]; private readonly now: () => number; @@ -99,12 +122,26 @@ export class GlanceablePublisher { private current: GlanceableAgentsSnapshot | null; private activityStarted: boolean; /** - * `updatedAt` of the snapshot last written to a sink, i.e. the frame the - * native stale deadline keys off. A heartbeat whose visible content did not - * change leaves it alone, so the renewal gate can tell how close that - * deadline is. + * `updatedAt` of the frame the native surface accepted, i.e. the one its stale + * deadline keys off. A heartbeat whose visible content did not change leaves + * it alone, so the renewal gate can tell how close that deadline is. A + * rejected write leaves it alone too, so the frame the surface actually holds + * is still the one measured. */ private lastPublishedAt: number | null = null; + /** + * Consecutive writes whose sinks rejected them. The renewal retry spaces + * itself by this count, so a sink that throws on every call cannot make the + * heartbeat re-emit every few seconds. A landed write clears it. + */ + private writeFailures = 0; + /** + * When the last write was attempted, accepted or not. The renewal gate + * anchors the backoff here, so a rejected renewal spends its wait instead of + * the heartbeat re-emitting every time. Non-null is also the "a write was + * attempted in this process" latch the restart reconciliation reads. + */ + private lastWriteAttemptAt: number | null = null; private coalesceTimer: TimerHandle | null = null; private terminalTimer: TimerHandle | null = null; private pendingCoalesced: { @@ -183,10 +220,20 @@ export class GlanceablePublisher { // call is what retries a Live Activity start the sink could not raise (a // transient ActivityKit failure, or a start deferred behind a dismissal), // and leaving it out of the renewal would strand that surface until the - // counts next changed. Keep the revision monotonic for the next real emit, - // and leave any pending coalesced emit alone. The first eligible emit + // count next changed. Keep the revision monotonic for the next real emit, + // and leave any pending coalesced emit alone. A renewal the sinks rejected is + // retried on a doubling backoff rather than on every heartbeat, so a + // permanently broken surface cannot re-emit forever. The first eligible emit // (nothing started yet) is exempt: it is what raises the surface. + // + // Nothing attempted yet in this process is exempt too. The native surfaces + // outlive the JS process: after a restart inside the 8 s terminal window the + // persisted snapshot is already empty, so an unchanged empty heartbeat would + // return here and the Android sink's `!notificationActive → endNotification` + // dismissal would never run, leaving the orphaned ongoing card in the shade. + // Let the first write through so each sink reconciles the surface it finds. if ( + this.lastWriteAttemptAt !== null && this.current !== null && hasSameGlanceableContent( { snapshot: this.current, newestSessionTitle: previousTitle }, @@ -194,10 +241,7 @@ export class GlanceablePublisher { ) && (this.activityStarted || !isEligibleGlanceableWork(snapshot)) ) { - if ( - isEligibleGlanceableWork(snapshot) && - (this.lastPublishedAt === null || now - this.lastPublishedAt >= GLANCEABLE_RENEW_MARGIN_MS) - ) { + if (isEligibleGlanceableWork(snapshot) && this.isRenewalDue(now)) { // The renewal frame carries the same visible content as any pending // coalesced emit but a newer revision, so emitting it supersedes that // timer: leaving the timer armed would republish the older frame after @@ -350,27 +394,49 @@ export class GlanceablePublisher { return skip === undefined ? sessions : sessions.filter(row => row.id !== skip); } - private emit(snapshot: GlanceableAgentsSnapshot, ctx: GlanceableSinkContext): void { - this.lastPublishedAt = Date.parse(snapshot.updatedAt); - for (const sink of this.sinks) { - // Guarded separately: a failing widget timeline write must not skip the - // Live Activity start that follows it. - guardSink('emit_publish', () => { - sink.publish(snapshot); - }); - guardSink('emit_start_or_update', () => { - sink.startOrUpdate(snapshot, ctx); - }); + /** + * Whether an unchanged heartbeat should renew the native surface. The last + * accepted frame must be at or past the renewal margin (or none was ever + * accepted), and the backoff from a rejected write must have elapsed. The + * backoff is what keeps a sink that rejects every write from re-emitting on + * every heartbeat: the rejected attempt spends the wait, which doubles to a + * ceiling, instead of the published deadline staying frozen and re-arming the + * renewal each heartbeat. + */ + private isRenewalDue(now: number): boolean { + if (this.lastPublishedAt !== null && now - this.lastPublishedAt < GLANCEABLE_RENEW_MARGIN_MS) { + return false; } + return ( + this.lastWriteAttemptAt === null || + now - this.lastWriteAttemptAt >= glanceableRenewRetryDelayMs(this.writeFailures) + ); + } + + private emit(snapshot: GlanceableAgentsSnapshot, ctx: GlanceableSinkContext): void { + this.writeFrame(snapshot, ctx); } private publish(snapshot: GlanceableAgentsSnapshot): void { - this.lastPublishedAt = Date.parse(snapshot.updatedAt); - for (const sink of this.sinks) { - guardSink('publish', () => { - sink.publish(snapshot); - }); + this.writeFrame(snapshot, null); + } + + /** + * Write one frame through the sinks and record the outcome. The published + * deadline is the frame the native surface accepted, so it advances only when + * every sink write landed. A rejected write is not dropped: it raises the + * failure count, which holds the renewal gate on its backoff so the frame is + * retried while the counts stay stable, instead of the heartbeat re-emitting + * every few seconds. + */ + private writeFrame(snapshot: GlanceableAgentsSnapshot, ctx: GlanceableSinkContext | null): void { + this.lastWriteAttemptAt = this.now(); + if (writeGlanceableFrame(this.sinks, snapshot, ctx)) { + this.lastPublishedAt = Date.parse(snapshot.updatedAt); + this.writeFailures = 0; + return; } + this.writeFailures += 1; } private scheduleCoalesced(snapshot: GlanceableAgentsSnapshot, ctx: GlanceableSinkContext): void { diff --git a/apps/mobile/src/lib/glanceable/sink-registry.ts b/apps/mobile/src/lib/glanceable/sink-registry.ts index 5b9c15dda6..c5ea326e7d 100644 --- a/apps/mobile/src/lib/glanceable/sink-registry.ts +++ b/apps/mobile/src/lib/glanceable/sink-registry.ts @@ -53,17 +53,22 @@ function reportSinkFailure(operation: string, error: unknown): void { } /** - * Run one sink operation and swallow its failure. A native surface must never - * throw into the auth transition, the org switch, or the in-app publisher: a - * throwing WidgetKit or ActivityKit host function there would abort a sign-in - * or kill the publisher effect. The background push path deliberately does NOT - * use this — a native failure must reject so the OS retries the push. + * Run one sink operation and swallow its failure, reporting whether the + * operation completed. A native surface must never throw into the auth + * transition, the org switch, or the in-app publisher: a throwing WidgetKit or + * ActivityKit host function there would abort a sign-in or kill the publisher + * effect. The background push path deliberately does NOT use this — a native + * failure must reject so the OS retries the push. Callers that track the frame + * the native surface last accepted (the publisher's renewal gate) use the + * result to tell a write that landed from one that did not. */ -export function guardSink(operation: string, run: () => void): void { +export function guardSink(operation: string, run: () => void): boolean { try { run(); + return true; } catch (error) { reportSinkFailure(operation, error); + return false; } } @@ -78,6 +83,37 @@ export function forEachSink(operation: string, run: (sink: GlanceableSink) => vo } } +/** + * Write one frame through each supplied sink, reporting whether every write + * landed. A context selects the publisher's emit shape (`emit_publish` plus + * `emit_start_or_update`); without one, only the plain `publish` write runs. + * Each operation is guarded on its own, so a rejected WidgetKit or ActivityKit + * call never skips the operation after it or the other sinks. The publisher + * advances its published deadline only when this returns true and spaces the + * next renewal by a backoff: a rejected renewal is retried without waiting a + * full renewal margin and without re-emitting on every heartbeat. + */ +export function writeGlanceableFrame( + targets: readonly GlanceableSink[], + snapshot: GlanceableAgentsSnapshot, + ctx: GlanceableSinkContext | null +): boolean { + let ok = true; + for (const sink of targets) { + const writePublish = () => { + sink.publish(snapshot); + }; + ok = guardSink(ctx === null ? 'publish' : 'emit_publish', writePublish) && ok; + if (ctx !== null) { + const writeStart = () => { + sink.startOrUpdate(snapshot, ctx); + }; + ok = guardSink('emit_start_or_update', writeStart) && ok; + } + } + return ok; +} + /** * Activity-token registrar, set by a later token slice. No-op by default. * `unregisterTokens` reports only the tokens whose unregister failed, so diff --git a/services/notifications/src/lib/glanceable-refresh.test.ts b/services/notifications/src/lib/glanceable-refresh.test.ts index f650d5ced0..d63a76dd70 100644 --- a/services/notifications/src/lib/glanceable-refresh.test.ts +++ b/services/notifications/src/lib/glanceable-refresh.test.ts @@ -8,6 +8,7 @@ import { flushDueGlanceableRefreshes, flushDueGlanceableRefreshesSafely, GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + GLANCEABLE_REFRESH_RETRY_MAX_MS, refreshGlanceableSnapshot, } from './glanceable-refresh'; @@ -285,6 +286,7 @@ describe('refreshGlanceableSnapshot delivery window', () => { organizationId: null, dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, deferredAt: now, + attempts: 1, }); }); @@ -424,6 +426,7 @@ describe('refreshGlanceableSnapshot delivery window', () => { organizationId: null, dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, deferredAt: now, + attempts: 1, }); }); @@ -481,6 +484,7 @@ describe('refreshGlanceableSnapshot delivery window', () => { organizationId: null, dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, deferredAt: now, + attempts: 1, }); }); @@ -530,6 +534,7 @@ describe('refreshGlanceableSnapshot delivery window', () => { organizationId: null, dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, deferredAt: now, + attempts: 1, }); }); @@ -584,6 +589,7 @@ describe('refreshGlanceableSnapshot delivery window', () => { organizationId: null, dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, deferredAt: now, + attempts: 1, }); }); @@ -676,6 +682,7 @@ describe('refreshGlanceableSnapshot delivery window', () => { organizationId: null, dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, deferredAt: base + 2_000, + attempts: 1, }); }); @@ -1058,10 +1065,115 @@ describe('refreshGlanceableSnapshot delivery window', () => { organizationId: null, dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, deferredAt: now, + attempts: 1, }); expect(h.builds).toBe(1); }); + it('does not re-arm when a delivery landed while the throwing refresh ran', async () => { + const h = makeHarness(); + const now = 12_500_000; + const key = pendingKey('user-throw-delivered', null); + await h.storage.put(key, { + userId: 'user-throw-delivered', + organizationId: null, + dueAt: now - 1, + }); + // A concurrent approval-exempt delivery lands while this refresh is in + // flight, then the refresh's build throws. The re-arm reads the delivery + // record and the slot in one transaction, so it must see the landed + // delivery and skip: a plain get-then-put would leave a record behind that + // later fires a redundant device wake. + h.deps.buildSnapshot = async () => { + await h.storage.put(deliveryKey('user-throw-delivered', null), { + deliveredAt: now, + outcome: 'delivered', + }); + throw new Error('route down'); + }; + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + await expect( + flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now) + ).resolves.toBeNull(); + expect(await h.storage.get(key)).toBeUndefined(); + }); + + it('escalates the trailing re-arm backoff and caps it', async () => { + const h = makeHarness(); + const now = 14_000_000; + const key = pendingKey('user-backoff', null); + // Four consecutive failed attempts: the fifth waits sixteen windows (the + // doubling from one window), and the twenty-first is clamped to the ceiling + // so a permanently failing route cannot keep rebuilding every window + // forever. + await h.storage.put(key, { + userId: 'user-backoff', + organizationId: null, + dueAt: now - 1, + deferredAt: now - 500_000, + attempts: 4, + }); + h.setNext(null); + + await expect( + flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now) + ).resolves.toBe(now + 16 * GLANCEABLE_DELIVERY_MIN_INTERVAL_MS); + expect(await h.storage.get(key)).toMatchObject({ + deferredAt: now - 500_000, + attempts: 5, + dueAt: now + 16 * GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + }); + + await h.storage.put(key, { + userId: 'user-backoff', + organizationId: null, + dueAt: now - 1, + deferredAt: now - 500_000, + attempts: 20, + }); + await expect( + flushDueGlanceableRefreshes(asStorage(h.storage), h.deps, () => now) + ).resolves.toBe(now + GLANCEABLE_REFRESH_RETRY_MAX_MS); + expect(await h.storage.get(key)).toMatchObject({ + attempts: 21, + dueAt: now + GLANCEABLE_REFRESH_RETRY_MAX_MS, + }); + }); + + it('resets the retry backoff when a newer change defers inside the window', async () => { + const h = makeHarness(); + const base = 15_000_000; + let now = base; + const scope = { userId: 'user-backoff-reset', organizationId: null }; + const key = pendingKey('user-backoff-reset', null); + + // A delivery opens the window and a later heartbeat re-arms a retried + // record, so the slot carries a backed-off count. + h.setNext(snapshot({ running: 1 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + await h.storage.put(key, { + userId: 'user-backoff-reset', + organizationId: null, + dueAt: now, + deferredAt: now - 400_000, + attempts: 7, + }); + + // A fresh change inside the window is a new deferral, not a retry: it gets + // the window's normal deadline and drops the failed-attempt count so the + // next failure backs off from one window again. + now = base + 2_000; + h.setNext(snapshot({ running: 2 })); + await refreshGlanceableSnapshot(scope, asStorage(h.storage), h.deps, () => now); + expect(await h.storage.get(key)).toEqual({ + userId: 'user-backoff-reset', + organizationId: null, + dueAt: base + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + deferredAt: now, + }); + }); + it('keeps a deferral written while a throwing trailing refresh ran', async () => { const h = makeHarness(); const now = 13_000_000; @@ -1191,6 +1303,10 @@ describe('NotificationChannelDO alarm glanceable flush', () => { const stub = env.NOTIFICATION_CHANNEL_DO.get(id); const now = Date.now(); const dueAt = now + 30_000; + // Proves the flush actually failed rather than quietly succeeding: a + // successful sweep skips the not-yet-due record and folds the same `dueAt`, + // so the GC and deadline assertions alone cannot tell the two apart. + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); await runInDurableObject(stub, async (_instance, state) => { await state.storage.put('idem:expired', { stage: 'delivered', ts: now - 2 * 60 * 60 * 1000 }); @@ -1202,7 +1318,7 @@ describe('NotificationChannelDO alarm glanceable flush', () => { }); }); - await runInDurableObject(stub, async (instance, state) => { + const rejections = await runInDurableObject(stub, async (instance, state) => { // The flush's first pending-prefix read rejects; the guard must absorb it // so the GC below still runs. Later reads (the fold) succeed. const mutable = state.storage as unknown as { @@ -1210,8 +1326,10 @@ describe('NotificationChannelDO alarm glanceable flush', () => { }; const originalList = state.storage.list.bind(state.storage); let pendingLists = 0; + let rejected = 0; mutable.list = (options?: { prefix?: string }) => { if (options?.prefix === 'glanceable-pending:' && ++pendingLists === 1) { + rejected += 1; return Promise.reject(new Error('storage unavailable')); } return originalList(options); @@ -1221,6 +1339,7 @@ describe('NotificationChannelDO alarm glanceable flush', () => { } finally { delete (mutable as { list?: unknown }).list; } + return rejected; }); const result = await runInDurableObject(stub, async (_instance, state) => ({ @@ -1232,6 +1351,14 @@ describe('NotificationChannelDO alarm glanceable flush', () => { alarm: await state.storage.getAlarm(), })); + // The sweep's own read was rejected and the guard absorbed it with the + // content-free warning; without these two the test cannot tell a failed + // flush from a successful one. + expect(rejections).toBe(1); + expect(warnSpy).toHaveBeenCalledWith( + 'Glanceable trailing refresh sweep failed', + expect.objectContaining({ error: 'storage unavailable' }) + ); // A failing flush no longer skips the sweep's storage reclamation. expect(result.idem).toBeUndefined(); expect(result.rl).toBeUndefined(); diff --git a/services/notifications/src/lib/glanceable-refresh.ts b/services/notifications/src/lib/glanceable-refresh.ts index e44c592320..d2a8283a09 100644 --- a/services/notifications/src/lib/glanceable-refresh.ts +++ b/services/notifications/src/lib/glanceable-refresh.ts @@ -10,6 +10,26 @@ import { deliverGlanceableSnapshot, type GlanceableDeliveryDeps } from './glance */ export const GLANCEABLE_DELIVERY_MIN_INTERVAL_MS = 10_000; +/** + * Ceiling for the trailing-refresh retry backoff. A permanently failing build + * or transport re-arms its pending record instead of dropping the deferred + * counts, so without a ceiling it would rebuild and resend every window + * forever. Backing off to at most one attempt per five minutes bounds that + * loop while still landing the counts once the route recovers. + */ +export const GLANCEABLE_REFRESH_RETRY_MAX_MS = 5 * 60_000; + +/** + * Wait before the next trailing attempt, from the number of consecutive + * attempts that failed to deliver a snapshot: one window for the first, then + * doubling up to the ceiling. A landed delivery or a fresh deferral drops the + * count, so a route that recovers is back to the normal cadence at once. + */ +export function glanceableRetryDelayMs(attempts: number): number { + const delay = GLANCEABLE_DELIVERY_MIN_INTERVAL_MS * 2 ** Math.max(0, attempts - 1); + return Math.min(delay, GLANCEABLE_REFRESH_RETRY_MAX_MS); +} + const scopeSchema = z.object({ userId: z.string().min(1), organizationId: z.string().min(1).nullable(), @@ -49,6 +69,11 @@ const pendingRefreshSchema = z.object({ // the same `dueAt` when they defer inside the same window. Optional for // records written before this field existed. deferredAt: z.number().optional(), + // Consecutive trailing attempts that failed to deliver this change. Each + // re-arm increments it so a permanently failing build or transport backs off + // instead of re-arming every window forever; a new deferral or a landed + // delivery clears it. Optional for records written before this field existed. + attempts: z.number().int().nonnegative().optional(), }); type PendingGlanceableRefresh = z.infer; type DeliveryState = z.infer; @@ -59,6 +84,11 @@ function pendingKey(scope: { userId: string; organizationId: string | null }): s return `${PENDING_PREFIX}${JSON.stringify([scope.userId, scope.organizationId])}`; } +/** The delivery record's key, so the sweep can read it before it runs a refresh. */ +function deliveryStateKey(scope: { userId: string; organizationId: string | null }): string { + return `glanceable:${JSON.stringify([scope.userId, scope.organizationId])}:delivery`; +} + /** * Whether a pending record read after a delivery is the same one that was * already stored when the delivery started. A deferral written while the @@ -101,6 +131,10 @@ function isSameDeferral( * left to retry it. A record without an outcome predates the field and only * ever meant a delivery. A pending record already in the slot is a newer * deferral this flush did not consume; leave its deadline alone. + * + * Each re-arm increments the consumed record's `attempts` and spaces the + * deadline by `glanceableRetryDelayMs`, so a build or transport that keeps + * failing retries at a bounded rate instead of every window forever. */ async function rearmTrailingRefresh( scope: { userId: string; organizationId: string | null }, @@ -108,10 +142,12 @@ async function rearmTrailingRefresh( deliveryKey: string, deliveryAtStart: DeliveryState | undefined, deferredChangeAt: number | undefined, + retryAttempts: number | undefined, nowMs: () => number ): Promise { const pendingK = pendingKey(scope); const now = nowMs(); + const attempts = (retryAttempts ?? 0) + 1; await storage.transaction(async tx => { const landed = deliveryStateSchema.optional().parse(await tx.get(deliveryKey)); if ( @@ -125,8 +161,9 @@ async function rearmTrailingRefresh( await tx.put(pendingK, { userId: scope.userId, organizationId: scope.organizationId, - dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, + dueAt: now + glanceableRetryDelayMs(attempts), deferredAt: deferredChangeAt ?? now, + attempts, }); }); } @@ -137,11 +174,16 @@ export async function refreshGlanceableSnapshot( storage: DurableObjectStorage, deps: GlanceableDeliveryDeps, nowMs: () => number = Date.now, - options: { trailing?: boolean; approvalChanged?: boolean; deferredAt?: number } = {} + options: { + trailing?: boolean; + approvalChanged?: boolean; + deferredAt?: number; + attempts?: number; + } = {} ): Promise { const scope = scopeSchema.parse(params); const key = `glanceable:${JSON.stringify([scope.userId, scope.organizationId])}`; - const deliveryKey = `${key}:delivery`; + const deliveryKey = deliveryStateKey(scope); // Rate-limit the device wake per scope. A change inside the window is // deferred to the alarm rather than dropping it, so the final counts land. // `needsApproval` is exempt: it gates the Approve control, which must appear @@ -211,7 +253,15 @@ export async function refreshGlanceableSnapshot( // credentials fail, and the flush has already consumed the pending record, // so re-arm the next window instead of dropping the change with no alarm. if (options.trailing === true) { - await rearmTrailingRefresh(scope, storage, deliveryKey, delivery, options.deferredAt, nowMs); + await rearmTrailingRefresh( + scope, + storage, + deliveryKey, + delivery, + options.deferredAt, + options.attempts, + nowMs + ); } return; } @@ -241,7 +291,15 @@ export async function refreshGlanceableSnapshot( // deferred change unless a delivery actually landed, exactly as the // no-snapshot branch above does. if (options.trailing === true) { - await rearmTrailingRefresh(scope, storage, deliveryKey, delivery, options.deferredAt, nowMs); + await rearmTrailingRefresh( + scope, + storage, + deliveryKey, + delivery, + options.deferredAt, + options.attempts, + nowMs + ); } return; } @@ -396,15 +454,19 @@ export async function flushDueGlanceableRefreshes( const pending = await storage.list({ prefix: PENDING_PREFIX }); for (const [key, record] of pending) { if (record.dueAt > nowMs()) continue; + const scope = { userId: record.userId, organizationId: record.organizationId }; + const deliveryKey = deliveryStateKey(scope); + // The re-arm below tells a delivery that landed while the refresh ran apart + // from one already recorded by comparing this read with the record's at + // re-arm time, so it must happen before the refresh starts. + const deliveryAtStart = deliveryStateSchema.optional().parse(await storage.get(deliveryKey)); await storage.delete(key); try { - await refreshGlanceableSnapshot( - { userId: record.userId, organizationId: record.organizationId }, - storage, - deps, - nowMs, - { trailing: true, deferredAt: record.deferredAt } - ); + await refreshGlanceableSnapshot(scope, storage, deps, nowMs, { + trailing: true, + deferredAt: record.deferredAt, + attempts: record.attempts, + }); } catch (error) { console.warn('Glanceable trailing refresh failed', { scope: [record.userId, record.organizationId], @@ -414,22 +476,25 @@ export async function flushDueGlanceableRefreshes( // rejected (a network/DNS failure on the snapshot route) or the transport // rejected. The pending record was consumed above, so re-arm the next // window or the deferred counts are dropped with no alarm left to retry - // them. A record written while the refresh ran already owns the key and a - // later deadline; only an empty slot is re-armed. The next window, not - // now, so a persistently failing route is retried once per window instead - // of spinning the alarm. Keep the consumed record's write time: a delivery - // that lands later covers that change and cancels this record, while a - // fresh `now` would look newer than the delivery's snapshot and survive as - // a redundant wake. - if ((await storage.get(key)) === undefined) { - const now = nowMs(); - await storage.put(key, { - userId: record.userId, - organizationId: record.organizationId, - dueAt: now + GLANCEABLE_DELIVERY_MIN_INTERVAL_MS, - deferredAt: record.deferredAt ?? now, - }); - } + // them. Re-arm through the same transaction-hardened helper the trailing + // refresh uses, not a plain get-then-put: a delivery landing between the + // slot check and the write would leave a record behind that later fires a + // redundant device wake. A record written while the refresh ran already + // owns the key and a later deadline; only an empty slot is re-armed. Keep + // the consumed record's write time: a delivery that lands later covers + // that change and cancels this record, while a fresh `now` would look + // newer than the delivery's snapshot and survive as a redundant wake. The + // re-arm escalates the consumed record's backoff so a permanently failing + // route is retried at a bounded rate, not once per window forever. + await rearmTrailingRefresh( + scope, + storage, + deliveryKey, + deliveryAtStart, + record.deferredAt, + record.attempts, + nowMs + ); } } diff --git a/services/session-ingest/src/dos/UserConnectionDO.ts b/services/session-ingest/src/dos/UserConnectionDO.ts index 02a51f02c3..38806af2d8 100644 --- a/services/session-ingest/src/dos/UserConnectionDO.ts +++ b/services/session-ingest/src/dos/UserConnectionDO.ts @@ -1947,10 +1947,17 @@ export class UserConnectionDO extends DurableObject { refreshGlanceableSessions(this.env, { userId: attachment.kiloUserId, cliSessionIds: rootSessionIds, - // A disconnecting CLI drops a permission wait to `retry`, which - // clears the Approve control on the locked surfaces. The in-memory - // status is the last one the CLI reported; the attention reset above - // writes the DB but does not touch this list. + // A disconnecting CLI leaves the live aggregate, so the snapshot the + // locked surfaces build no longer carries its permission: the Approve + // control clears now rather than behind the delivery window. The + // attention reset above does not write the stored status — it holds + // the clear for the CLI absence window — so this exemption covers the + // aggregate drop, and the deferred write fires its own exemption in + // `resetAttentionStatusOnCliDisconnect` for a session that stays + // snapshot-visible (a cloud agent merged from Postgres, not the live + // list). Only the scopes whose roots moved are named: this batch + // aggregates every connection, so a request-level flag would exempt + // scopes that had no approval change. approvalChangedSessionIds: rootSessionIds.filter(id => permissionRootIds.has(id)), }) ); diff --git a/services/session-ingest/src/ingest/metadata.test.ts b/services/session-ingest/src/ingest/metadata.test.ts index fbdea8bcc4..c7dd078bd6 100644 --- a/services/session-ingest/src/ingest/metadata.test.ts +++ b/services/session-ingest/src/ingest/metadata.test.ts @@ -415,6 +415,49 @@ describe('resetAttentionStatusOnCliDisconnect', () => { ); }); + it('asks for an approval-exempt refresh when a permission wait clears', async () => { + const db = createTransactionDb({ initialStatus: 'permission' }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + const refreshParams: RefreshGlanceableSessionsParams[] = []; + const env = { + HYPERDRIVE: { connectionString: 'postgres://unused' }, + NOTIFICATIONS: { + async refreshGlanceableSessions(params: RefreshGlanceableSessionsParams) { + refreshParams.push(params); + }, + }, + } as never; + + await resetAttentionStatusOnCliDisconnect(env, 'usr_1', 'ses_1'); + + // The deferred `permission -> retry` write is what actually clears the + // stored attention, so it — not the socket close ten minutes earlier — must + // get the window exemption that makes the Approve control disappear. + expect(refreshParams).toEqual([ + { userId: 'usr_1', cliSessionIds: ['ses_1'], approvalChangedSessionIds: ['ses_1'] }, + ]); + }); + + it('does not exempt a cleared question from the delivery window', async () => { + const db = createTransactionDb({ initialStatus: 'question' }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + const refreshParams: RefreshGlanceableSessionsParams[] = []; + const env = { + HYPERDRIVE: { connectionString: 'postgres://unused' }, + NOTIFICATIONS: { + async refreshGlanceableSessions(params: RefreshGlanceableSessionsParams) { + refreshParams.push(params); + }, + }, + } as never; + + await resetAttentionStatusOnCliDisconnect(env, 'usr_1', 'ses_1'); + + // A question is not an approval: it keeps counting as needs-input after the + // clear, so no wake is worth spending the window on. + expect(refreshParams).toEqual([]); + }); + it.each(['busy', 'idle', 'retry', null] as const)( 'no-ops without write or notify when stored status is %s', async status => { diff --git a/services/session-ingest/src/ingest/metadata.ts b/services/session-ingest/src/ingest/metadata.ts index 209482c547..634b1707ee 100644 --- a/services/session-ingest/src/ingest/metadata.ts +++ b/services/session-ingest/src/ingest/metadata.ts @@ -484,7 +484,9 @@ export async function flushPartialMetadataChanges( * Only rows currently in `question`/`permission` are updated (to `retry`). Uses a * conditional write so concurrent non-attention updates are not overwritten. Emits * `session.status.updated` via the metadata path only — never enters the ingest - * completion pipeline, so no "Task completed" push can fire. + * completion pipeline, so no "Task completed" push can fire. A cleared + * `permission` also asks for an approval-exempt glanceable refresh: the Approve + * control must disappear here, ten minutes after the CLI that held it left. */ export async function resetAttentionStatusOnCliDisconnect( env: Env, @@ -573,4 +575,21 @@ export async function resetAttentionStatusOnCliDisconnect( }, ctx ); + + if (notification.previousStatus === 'permission') { + // This is the deferred half of a CLI disconnect: the live session left the + // aggregate when the socket closed, but a session that stays + // snapshot-visible (a cloud agent, whose row is merged from Postgres rather + // than the live list) kept showing the permission the whole absence window. + // The Approve control gates on `permission`, so clearing it must not wait + // for the shared delivery window — the same exemption an in-band + // `permission -> busy` move gets in `applyMetadataChanges`. + const delivery = refreshGlanceableSessions(env, { + userId: kiloUserId, + cliSessionIds: [sessionId], + approvalChangedSessionIds: [sessionId], + }); + if (ctx) ctx.waitUntil(delivery); + else await delivery; + } } From aecddf529c21cb9e67a502e68dd01f6e9b94f058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Mon, 21 Sep 2026 02:33:39 +0000 Subject: [PATCH 6/6] fix: kwf-fix-review-e20c patch delivery --- .../src/lib/glanceable/publisher.test.ts | 48 +++++++++++++++++++ apps/mobile/src/lib/glanceable/publisher.ts | 22 ++++++--- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/apps/mobile/src/lib/glanceable/publisher.test.ts b/apps/mobile/src/lib/glanceable/publisher.test.ts index e70a0719cb..fceb499ecd 100644 --- a/apps/mobile/src/lib/glanceable/publisher.test.ts +++ b/apps/mobile/src/lib/glanceable/publisher.test.ts @@ -433,6 +433,54 @@ describe('GlanceablePublisher', () => { publisher.dispose(); }); + it('retries a rejected counts change on the failure backoff, not the renewal margin', () => { + // A coalesced counts change is emitted regardless of the renewal gate, so a + // sink that rejects it leaves the surface on the previous counts. The last + // accepted frame is only seconds old, so waiting for the renewal margin + // alone would hold the change back for up to 15 minutes; the rejected frame + // must instead retry once the failure backoff elapses. + vi.useFakeTimers(); + let now = NOW; + const calls: SinkCall[] = []; + let writes = 0; + const sink: GlanceableSink = { + publish(snapshot) { + calls.push({ type: 'publish', snapshot }); + }, + startOrUpdate(snapshot, ctx) { + writes += 1; + calls.push({ type: 'startOrUpdate', snapshot, ctx }); + if (writes === 2) { + throw new Error('ActivityKit update failed'); + } + }, + endImmediate() { + // The counts never go empty here. + }, + }; + const publisher = new GlanceablePublisher({ sinks: [sink], now: () => now, coalesceMs: 1000 }); + publisher.handleSessions([{ status: 'busy' }], PUB_CTX); + expect(count(calls, 'startOrUpdate')).toBe(1); + + // A counts-only change is coalesced, then emitted when the window elapses. + now += 1; + publisher.handleSessions([{ status: 'busy' }, { status: 'busy' }], PUB_CTX); + expect(count(calls, 'startOrUpdate')).toBe(1); + + now += 1000; + vi.advanceTimersByTime(1000); + expect(count(calls, 'startOrUpdate')).toBe(2); + + // The accepted frame is only ~1 s old, but the change was rejected, so the + // next heartbeat after the backoff retries it rather than holding it until + // the surface's stale window approaches. + now += GLANCEABLE_COALESCE_MS; + publisher.handleSessions([{ status: 'busy' }, { status: 'busy' }], PUB_CTX); + expect(count(calls, 'startOrUpdate')).toBe(3); + expect(lastSnapshot(calls, 'startOrUpdate').running).toBe(2); + publisher.dispose(); + }); + it('bounds count churn to one native update per window', () => { vi.useFakeTimers(); let now = NOW; diff --git a/apps/mobile/src/lib/glanceable/publisher.ts b/apps/mobile/src/lib/glanceable/publisher.ts index bfdedf28fd..13ffaf737a 100644 --- a/apps/mobile/src/lib/glanceable/publisher.ts +++ b/apps/mobile/src/lib/glanceable/publisher.ts @@ -395,16 +395,24 @@ export class GlanceablePublisher { } /** - * Whether an unchanged heartbeat should renew the native surface. The last + * Whether an unchanged heartbeat should retry the native surface. The last * accepted frame must be at or past the renewal margin (or none was ever - * accepted), and the backoff from a rejected write must have elapsed. The - * backoff is what keeps a sink that rejects every write from re-emitting on - * every heartbeat: the rejected attempt spends the wait, which doubles to a - * ceiling, instead of the published deadline staying frozen and re-arming the - * renewal each heartbeat. + * accepted), and the backoff from a rejected write must have elapsed. A + * rejected write leaves the surface on an older frame than the one the last + * emit carried, so it skips the margin check and retries on the backoff alone: + * the wait anchors at the rejected attempt, not at the older accepted frame, + * so a change the sinks rejected cannot be held back until that frame nears + * its stale window. The backoff is what keeps a sink that rejects every write + * from re-emitting on every heartbeat: the rejected attempt spends the wait, + * which doubles to a ceiling, instead of the published deadline staying frozen + * and re-arming the renewal each heartbeat. */ private isRenewalDue(now: number): boolean { - if (this.lastPublishedAt !== null && now - this.lastPublishedAt < GLANCEABLE_RENEW_MARGIN_MS) { + if ( + this.writeFailures === 0 && + this.lastPublishedAt !== null && + now - this.lastPublishedAt < GLANCEABLE_RENEW_MARGIN_MS + ) { return false; } return (