fix: stop notification loss, credentialed cache reuse, and clipboard clobber (#11230)
* fix: stop notification loss, credentialed cache reuse, and clipboard clobber Mobile catch-up (#8591): fetchMissed swallowed the RPC failure while deliverLive kept advancing and persisting lastDeliveredSeq, so the next successful catch-up asked from above the abandoned range and the desktop cut it. Sessions are module-scope, so an unchanged epoch never resets it. Quarantine the watermark at the last contiguously-delivered seq and hold it there until some later catch-up actually drains — not just one retry. A batch cut short by a teardown quarantines at the last event it settled. Jira attachment cache: currentEpoch summed two independent counters, so a site at siteEpoch 1 read the same value before and after a global clear. The mid-flight guard passed and re-inserted credentialed image bytes that "disconnect all" had just purged — resident for the process lifetime since pruneExpired has no timer. One monotonic ticker, compared by max. Web copy fallback: the handler registered in the capture phase, so xterm's bubble-phase listener overwrote text/plain with the terminal selection afterwards; served was already true, so the copy reported success. Every Orca copy affordance over plain HTTP (Copy Pane ID, Copy Path, commit SHA, PR URL) pasted the terminal selection. Bubble phase with stopImmediatePropagation. Covers the secure-context retry branch too, which shares the same helper. * fix: roll back the persisted watermark on catch-up failure; cover stopImmediatePropagation Adversarial review of a98d7f4d5d found two gaps. 1. The quarantine clamped only writes made AFTER the failure. getMissedSince waits up to 30s, so a live event routinely persists a higher seq while the request is still outstanding; that value stayed on disk, and the next launch read it back and resumed past the abandoned range -- the original bug, reached through a restart. quarantineCatchUpWatermark now re-persists the clamped seq, so the stored value never outlives the gap it guards. 2. web-clipboard-copy-terminal-selection's second test registered its "late" document handler BEFORE the fallback's, so it lost on registration order alone and stopImmediatePropagation was never exercised -- the test passed with that line deleted. Bubbling reaches the document before the window, so a window-level listener is what actually requires it. * fix(mobile): mark a notification seen only once its show lands A pre-marked seen key made a rejected show unrecoverable: the next catch-up re-fetched the seq and the dedup guard dropped it, and the first later event to drain the batch lifted the quarantine past it. Also contains the rejection so it does not escape the un-awaited 'ready'/live handlers as an unhandled rejection. Co-authored-by: Orca <help@stably.ai> * test(web-clipboard): pin stopImmediatePropagation with a same-target handler Both existing cases passed with plain stopPropagation, and with the listener back in the capture phase — neither half of the fix was actually pinned. The window-level clobber is on a different target, so stopPropagation suppresses it too. Registering the clobber on the document, ordered after the fallback's own listener, is the only shape stopPropagation cannot cover. Addresses the review comment posted after the last commit. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
GitHub
parent
681c4ba458
commit
3a67186623
|
|
@ -15,9 +15,12 @@ import {
|
|||
} from './local-notification-scheduling'
|
||||
import {
|
||||
adoptNotificationEpoch,
|
||||
catchUpWatermarkSeq,
|
||||
enqueueHostDelivery,
|
||||
getHostNotificationSession,
|
||||
quarantineCatchUpWatermark,
|
||||
releaseQueuedShowNotificationId,
|
||||
resolveCatchUpQuarantine,
|
||||
saveWatermark,
|
||||
seedWatermarkFromStorage,
|
||||
seenKeyForEvent,
|
||||
|
|
@ -68,7 +71,10 @@ export function subscribeToDesktopNotifications(client: RpcClient, hostId: strin
|
|||
releaseQueuedShowNotificationId(session, event.notificationId)
|
||||
}
|
||||
}
|
||||
})
|
||||
// Why swallowed: the caller is an un-awaited handler, so a rejected show would
|
||||
// surface as an unhandled rejection (a RN redbox) instead of being retried by
|
||||
// the next catch-up — which is now possible, since `seen` is marked after the show.
|
||||
}).catch(() => {})
|
||||
}
|
||||
|
||||
async function deliverLive(
|
||||
|
|
@ -76,87 +82,127 @@ export function subscribeToDesktopNotifications(client: RpcClient, hostId: strin
|
|||
event: NotificationEvent | DismissNotificationEvent
|
||||
): Promise<void> {
|
||||
adoptNotificationEpoch(session, hostId, event.notificationEpoch)
|
||||
// Why (#8129): mark seen on the live path too, so a later replay of an already-pushed id dedups instead of double-pushing.
|
||||
const key = seenKeyForEvent(event)
|
||||
if (key) {
|
||||
session.seen.add(key)
|
||||
}
|
||||
const epochAtDelivery = session.lastDeliveredEpoch
|
||||
if (type === 'notification') {
|
||||
await showLocalNotification(event as NotificationEvent, hostId)
|
||||
} else {
|
||||
await dismissLocalNotification(event as DismissNotificationEvent, hostId)
|
||||
}
|
||||
// Why after the await, exactly like the watermark below: `seen` asserts this event
|
||||
// reached the user (#8129). Marked before, a rejected show leaves the key behind and
|
||||
// every later replay is dropped as a duplicate — loss the quarantine cannot recover,
|
||||
// since the first event to drain a batch lifts it past the one never shown.
|
||||
const key = seenKeyForEvent(event)
|
||||
// A mid-flight epoch adoption already cleared the counter lifetime this key indexes.
|
||||
if (key && session.lastDeliveredEpoch === epochAtDelivery) {
|
||||
session.seen.add(key)
|
||||
}
|
||||
// Why after the await (#8591): the watermark is a promise that everything up
|
||||
// to this seq has been shown. Advancing it before the local notification lands
|
||||
// means a process death in between silently drops it — the next launch asks the
|
||||
// desktop for seq greater than one the user never saw.
|
||||
if (event.notificationSeq != null && event.notificationSeq > session.lastDeliveredSeq) {
|
||||
session.lastDeliveredSeq = event.notificationSeq
|
||||
// Persisted as a pair: a seq is only trustworthy alongside the epoch it indexes.
|
||||
// Why clamped: while a failed catch-up's range is still unrecovered, persisting
|
||||
// the live seq would let the next catch-up ask from above the gap and the desktop
|
||||
// would cut it. resolveCatchUpQuarantine writes the held-back value on success.
|
||||
void saveWatermark(hostId, {
|
||||
seq: session.lastDeliveredSeq,
|
||||
seq: catchUpWatermarkSeq(session),
|
||||
epoch: session.lastDeliveredEpoch
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Claimed inline rather than via queueDelivery: the batch is already one queue
|
||||
// entry, and re-enqueueing per item is what let a live event cut in.
|
||||
async function deliverMissedEvent(
|
||||
event: NotificationEvent | DismissNotificationEvent
|
||||
): Promise<void> {
|
||||
// No pre-marking here either: deliverLive marks the key once the show lands.
|
||||
const key = seenKeyForEvent(event)
|
||||
if (key && session.seen.has(key)) {
|
||||
return
|
||||
}
|
||||
if (event.type === 'notification') {
|
||||
if (!shouldQueueShowForNotificationId(session, event.notificationId)) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
await deliverLive('notification', event)
|
||||
} finally {
|
||||
releaseQueuedShowNotificationId(session, event.notificationId)
|
||||
}
|
||||
return
|
||||
}
|
||||
if (event.type === 'dismiss') {
|
||||
await deliverLive('dismiss', event)
|
||||
}
|
||||
}
|
||||
|
||||
// Why: desktop cuts by seq > lastSeenSeq, so re-fetching from the watermark is idempotent (session.seen guards residual overlap).
|
||||
async function fetchMissed(): Promise<void> {
|
||||
if (disposed) {
|
||||
return
|
||||
}
|
||||
// Captured before the request: everything at or below it is known delivered, so
|
||||
// it is the floor the watermark falls back to if this catch-up never completes.
|
||||
const askFrom = catchUpWatermarkSeq(session)
|
||||
const missed = await client
|
||||
.sendRequest('notifications.getMissedSince', {
|
||||
lastSeenSeq: session.lastDeliveredSeq,
|
||||
lastSeenSeq: askFrom,
|
||||
// Why: sending the epoch lets the desktop reject a watermark from a counter
|
||||
// it no longer has and return the whole retained buffer instead of nothing.
|
||||
...(session.lastDeliveredEpoch != null ? { epoch: session.lastDeliveredEpoch } : {})
|
||||
})
|
||||
.then((response) => {
|
||||
if (!response.ok) {
|
||||
return []
|
||||
return null
|
||||
}
|
||||
const result = response.result as { notifications?: unknown[]; epoch?: string } | undefined
|
||||
adoptNotificationEpoch(session, hostId, result?.epoch)
|
||||
return Array.isArray(result?.notifications) ? result.notifications : []
|
||||
})
|
||||
.catch(() => [])
|
||||
.catch(() => null)
|
||||
if (missed == null) {
|
||||
// Why quarantine rather than retry: the range this catch-up abandoned stays
|
||||
// unrecovered until SOME later one succeeds, and a live seq persisting past it
|
||||
// meanwhile would make the desktop cut it forever.
|
||||
quarantineCatchUpWatermark(session, hostId, askFrom)
|
||||
return
|
||||
}
|
||||
// Why the whole batch is ONE queue entry (#8591): awaiting per event returns to
|
||||
// the event loop between replays, so a live seq 11 slots into the chain between
|
||||
// seq 6 and 7 and persists a watermark past a notification still unshown. Why the
|
||||
// request stays OUTSIDE the queue: sendRequest waits up to 30s, and holding the
|
||||
// chain for that would stall live delivery on a slow link.
|
||||
await enqueueHostDelivery(session, async () => {
|
||||
for (const raw of missed) {
|
||||
// Re-checked per event: the batch can start before a teardown and still be
|
||||
// draining after it, and a torn-down host must stop pushing.
|
||||
if (disposed) {
|
||||
return
|
||||
}
|
||||
const event = raw as NotificationEvent | DismissNotificationEvent
|
||||
const key = seenKeyForEvent(event)
|
||||
if (key && session.seen.has(key)) {
|
||||
continue
|
||||
}
|
||||
if (key) {
|
||||
session.seen.add(key)
|
||||
}
|
||||
// Claimed inline rather than via queueDelivery: the batch is already one
|
||||
// queue entry, and re-enqueueing per item is what let a live event cut in.
|
||||
if (event.type === 'notification') {
|
||||
if (!shouldQueueShowForNotificationId(session, event.notificationId)) {
|
||||
continue
|
||||
// Advances only past events this batch settled, so a teardown or a failing show
|
||||
// quarantines the true contiguous point instead of the range it never reached.
|
||||
let contiguousSeq = askFrom
|
||||
let drained = false
|
||||
try {
|
||||
for (const raw of missed) {
|
||||
// Re-checked per event: the batch can start before a teardown and still be
|
||||
// draining after it, and a torn-down host must stop pushing.
|
||||
if (disposed) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
await deliverLive('notification', event)
|
||||
} finally {
|
||||
releaseQueuedShowNotificationId(session, event.notificationId)
|
||||
}
|
||||
} else if (event.type === 'dismiss') {
|
||||
await deliverLive('dismiss', event)
|
||||
const event = raw as NotificationEvent | DismissNotificationEvent
|
||||
await deliverMissedEvent(event)
|
||||
contiguousSeq = event.notificationSeq ?? contiguousSeq
|
||||
}
|
||||
drained = true
|
||||
} finally {
|
||||
if (drained) {
|
||||
resolveCatchUpQuarantine(session, hostId)
|
||||
} else {
|
||||
quarantineCatchUpWatermark(session, hostId, contiguousSeq)
|
||||
}
|
||||
}
|
||||
})
|
||||
// Why swallowed here: the `finally` above already recorded the contiguous point,
|
||||
// and the only caller is an un-awaited 'ready' continuation — letting a failed
|
||||
// show escape turns every one into an unhandled rejection (a RN redbox).
|
||||
}).catch(() => {})
|
||||
}
|
||||
|
||||
seedWatermarkFromStorage(session, hostId)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,316 @@
|
|||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import * as Notifications from 'expo-notifications'
|
||||
import { subscribeToDesktopNotifications } from './mobile-notifications'
|
||||
import { resetHostNotificationSessionsForTests } from './notification-reconnect-catchup'
|
||||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import { loadPushNotificationsEnabled } from '../storage/preferences'
|
||||
|
||||
vi.mock('expo-notifications', () => ({
|
||||
AndroidImportance: { HIGH: 'high' },
|
||||
setNotificationChannelAsync: vi.fn(),
|
||||
getPermissionsAsync: vi.fn(),
|
||||
requestPermissionsAsync: vi.fn(),
|
||||
scheduleNotificationAsync: vi.fn(),
|
||||
dismissNotificationAsync: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('react-native', () => ({
|
||||
Platform: { OS: 'ios', Version: 18 }
|
||||
}))
|
||||
|
||||
const WATERMARK_KEY = 'orca:mobileNotificationsWatermark:host-1'
|
||||
const storage = new Map<string, string>()
|
||||
|
||||
vi.mock('@react-native-async-storage/async-storage', () => ({
|
||||
default: {
|
||||
getItem: vi.fn(async (key: string) => storage.get(key) ?? null),
|
||||
setItem: vi.fn(async (key: string, value: string) => {
|
||||
storage.set(key, value)
|
||||
})
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('../storage/preferences', () => ({
|
||||
loadPushNotificationsEnabled: vi.fn()
|
||||
}))
|
||||
|
||||
function flushAsync(): Promise<void> {
|
||||
return new Promise((resolve) => {
|
||||
setTimeout(resolve, 10)
|
||||
})
|
||||
}
|
||||
|
||||
function persistedSeq(): number {
|
||||
return (JSON.parse(storage.get(WATERMARK_KEY) ?? '{}') as { seq?: number }).seq ?? 0
|
||||
}
|
||||
|
||||
type MissedOutcome =
|
||||
| { kind: 'reject' }
|
||||
| { kind: 'notOk' }
|
||||
| { kind: 'ok'; notifications: unknown[] }
|
||||
// Rejects only once `settle()` is called, so a live event can land mid-request.
|
||||
| { kind: 'heldReject' }
|
||||
|
||||
function makeHostClient() {
|
||||
let onData: ((data: unknown) => void) | null = null
|
||||
const askedFrom: number[] = []
|
||||
let outcome: MissedOutcome = { kind: 'ok', notifications: [] }
|
||||
let releaseHeld: (() => void) | null = null
|
||||
const client = {
|
||||
subscribe: vi.fn((_m: string, _p: unknown, cb: (data: unknown) => void) => {
|
||||
onData = cb
|
||||
return vi.fn(() => {
|
||||
onData = null
|
||||
})
|
||||
}),
|
||||
getState: vi.fn(() => 'connected'),
|
||||
sendRequest: vi.fn(async (method: string, params: unknown = {}) => {
|
||||
if (method !== 'notifications.getMissedSince') {
|
||||
return { ok: true, result: undefined } as never
|
||||
}
|
||||
askedFrom.push((params as { lastSeenSeq: number }).lastSeenSeq)
|
||||
if (outcome.kind === 'heldReject') {
|
||||
await new Promise<void>((resolve) => {
|
||||
releaseHeld = resolve
|
||||
})
|
||||
throw new Error('socket closed')
|
||||
}
|
||||
if (outcome.kind === 'reject') {
|
||||
throw new Error('socket closed')
|
||||
}
|
||||
if (outcome.kind === 'notOk') {
|
||||
return { ok: false, error: { message: 'timeout' } } as never
|
||||
}
|
||||
return { ok: true, result: { notifications: outcome.notifications } } as never
|
||||
})
|
||||
}
|
||||
return {
|
||||
client: client as unknown as RpcClient,
|
||||
get onData() {
|
||||
return onData
|
||||
},
|
||||
askedFrom,
|
||||
setOutcome(next: MissedOutcome) {
|
||||
outcome = next
|
||||
},
|
||||
settleHeld() {
|
||||
releaseHeld?.()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function notification(seq: number) {
|
||||
return {
|
||||
type: 'notification',
|
||||
title: `m${seq}`,
|
||||
body: 'b',
|
||||
notificationId: `agent:${seq}`,
|
||||
notificationSeq: seq
|
||||
}
|
||||
}
|
||||
|
||||
describe('#8591 catch-up failure quarantines the watermark', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
storage.clear()
|
||||
resetHostNotificationSessionsForTests()
|
||||
vi.mocked(loadPushNotificationsEnabled).mockResolvedValue(true)
|
||||
vi.mocked(Notifications.getPermissionsAsync).mockResolvedValue({
|
||||
status: 'granted',
|
||||
canAskAgain: true
|
||||
} as never)
|
||||
vi.mocked(Notifications.scheduleNotificationAsync).mockResolvedValue('sched-1')
|
||||
vi.mocked(Notifications.dismissNotificationAsync).mockResolvedValue(undefined)
|
||||
})
|
||||
|
||||
it('keeps asking from the abandoned range until a catch-up actually succeeds', async () => {
|
||||
// The phone was offline while seqs 6-7 dispatched. The catch-up that would have
|
||||
// replayed them dies (socket close / timeout / ok:false), and live traffic keeps
|
||||
// flowing. If a live seq is allowed to persist past 6-7, the desktop cuts by
|
||||
// `seq > lastSeenSeq` on the next catch-up and they are gone for good — and the
|
||||
// window stays open until some catch-up succeeds, not for one round trip.
|
||||
storage.set(WATERMARK_KEY, JSON.stringify({ seq: 5, epoch: 'epoch-1' }))
|
||||
const host = makeHostClient()
|
||||
host.setOutcome({ kind: 'reject' })
|
||||
|
||||
subscribeToDesktopNotifications(host.client, 'host-1')
|
||||
host.onData?.({ type: 'ready', subscriptionId: 'sub-1', epoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
expect(host.askedFrom).toEqual([5])
|
||||
|
||||
host.onData?.({ ...notification(11), notificationEpoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
expect(persistedSeq()).toBe(5)
|
||||
|
||||
// Second catch-up also fails; the gap is still open.
|
||||
host.setOutcome({ kind: 'notOk' })
|
||||
host.onData?.({ type: 'ready', subscriptionId: 'sub-1', epoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
host.onData?.({ ...notification(12), notificationEpoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
expect(host.askedFrom).toEqual([5, 5])
|
||||
expect(persistedSeq()).toBe(5)
|
||||
|
||||
// Third succeeds and replays the abandoned range.
|
||||
host.setOutcome({ kind: 'ok', notifications: [notification(6), notification(7)] })
|
||||
host.onData?.({ type: 'ready', subscriptionId: 'sub-1', epoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
|
||||
expect(host.askedFrom).toEqual([5, 5, 5])
|
||||
const titles = vi
|
||||
.mocked(Notifications.scheduleNotificationAsync)
|
||||
.mock.calls.map((call) => (call[0] as { content: { title: string } }).content.title)
|
||||
// Exact, not arrayContaining: a duplicate here is the double-push `seen` prevents.
|
||||
// m11/m12 are the live events that kept flowing while the gap stayed open.
|
||||
expect(titles).toEqual(['m11', 'm12', 'm6', 'm7'])
|
||||
|
||||
// Only now may the watermark move past the recovered range.
|
||||
expect(persistedSeq()).toBe(12)
|
||||
host.onData?.({ type: 'ready', subscriptionId: 'sub-1', epoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
expect(host.askedFrom).toEqual([5, 5, 5, 12])
|
||||
})
|
||||
|
||||
it('rolls back a watermark a live event stored while the catch-up was in flight', async () => {
|
||||
// getMissedSince waits up to 30s, so live traffic routinely persists during it.
|
||||
// Clamping only writes made AFTER the failure leaves that higher seq on disk, and
|
||||
// the next launch reads it back and resumes past the range this catch-up abandoned.
|
||||
storage.set(WATERMARK_KEY, JSON.stringify({ seq: 5, epoch: 'epoch-1' }))
|
||||
const host = makeHostClient()
|
||||
host.setOutcome({ kind: 'heldReject' })
|
||||
|
||||
subscribeToDesktopNotifications(host.client, 'host-1')
|
||||
host.onData?.({ type: 'ready', subscriptionId: 'sub-1', epoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
expect(host.askedFrom).toEqual([5])
|
||||
|
||||
host.onData?.({ ...notification(11), notificationEpoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
expect(persistedSeq()).toBe(11)
|
||||
|
||||
host.settleHeld()
|
||||
await flushAsync()
|
||||
expect(persistedSeq()).toBe(5)
|
||||
})
|
||||
|
||||
it('quarantines at the last replayed seq when a teardown cuts the batch short', async () => {
|
||||
// The batch can start before a teardown and still be draining after it, so the
|
||||
// events past the interruption were never shown. A live seq arriving on the next
|
||||
// connection must not persist over them.
|
||||
storage.set(WATERMARK_KEY, JSON.stringify({ seq: 5, epoch: 'epoch-1' }))
|
||||
const host = makeHostClient()
|
||||
host.setOutcome({
|
||||
kind: 'ok',
|
||||
notifications: [notification(6), notification(7), notification(8)]
|
||||
})
|
||||
|
||||
let unsubscribe: (() => void) | null = null
|
||||
vi.mocked(Notifications.scheduleNotificationAsync).mockImplementation(async (request) => {
|
||||
if ((request as { content: { title: string } }).content.title === 'm6') {
|
||||
unsubscribe?.()
|
||||
}
|
||||
return 'sched-1'
|
||||
})
|
||||
|
||||
unsubscribe = subscribeToDesktopNotifications(host.client, 'host-1')
|
||||
host.onData?.({ type: 'ready', subscriptionId: 'sub-1', epoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
|
||||
const titles = vi
|
||||
.mocked(Notifications.scheduleNotificationAsync)
|
||||
.mock.calls.map((call) => (call[0] as { content: { title: string } }).content.title)
|
||||
expect(titles).toEqual(['m6'])
|
||||
|
||||
// A fresh subscription on the same module-scope session takes a live seq 20 before
|
||||
// its own catch-up, then resumes from 6 rather than from 20.
|
||||
vi.mocked(Notifications.scheduleNotificationAsync).mockResolvedValue('sched-1')
|
||||
const host2 = makeHostClient()
|
||||
host2.setOutcome({ kind: 'ok', notifications: [notification(7), notification(8)] })
|
||||
subscribeToDesktopNotifications(host2.client, 'host-1')
|
||||
host2.onData?.({ ...notification(20), notificationEpoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
expect(persistedSeq()).toBe(6)
|
||||
|
||||
host2.onData?.({ type: 'ready', subscriptionId: 'sub-2', epoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
|
||||
expect(host2.askedFrom).toEqual([6])
|
||||
expect(
|
||||
vi
|
||||
.mocked(Notifications.scheduleNotificationAsync)
|
||||
.mock.calls.map((call) => (call[0] as { content: { title: string } }).content.title)
|
||||
).toEqual(['m6', 'm20', 'm7', 'm8'])
|
||||
expect(persistedSeq()).toBe(20)
|
||||
})
|
||||
|
||||
it('re-shows a replay whose show threw, instead of dropping it as already seen', async () => {
|
||||
// The quarantine only holds the RANGE. If the failing event is also marked seen,
|
||||
// the next catch-up re-fetches it and the dedup guard drops it — the banner is
|
||||
// never shown, and the first later event to drain the batch lifts the quarantine
|
||||
// past it. Silent loss with the watermark looking healthy.
|
||||
storage.set(WATERMARK_KEY, JSON.stringify({ seq: 5, epoch: 'epoch-1' }))
|
||||
const host = makeHostClient()
|
||||
host.setOutcome({ kind: 'ok', notifications: [notification(6), notification(7)] })
|
||||
|
||||
let failNext = true
|
||||
vi.mocked(Notifications.scheduleNotificationAsync).mockImplementation(async (request) => {
|
||||
const title = (request as { content: { title: string } }).content.title
|
||||
if (title === 'm6' && failNext) {
|
||||
failNext = false
|
||||
throw new Error('scheduling rejected')
|
||||
}
|
||||
return 'sched-1'
|
||||
})
|
||||
|
||||
subscribeToDesktopNotifications(host.client, 'host-1')
|
||||
host.onData?.({ type: 'ready', subscriptionId: 'sub-1', epoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
expect(persistedSeq()).toBe(5)
|
||||
|
||||
host.onData?.({ type: 'ready', subscriptionId: 'sub-1', epoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
|
||||
const titles = vi
|
||||
.mocked(Notifications.scheduleNotificationAsync)
|
||||
.mock.calls.map((call) => (call[0] as { content: { title: string } }).content.title)
|
||||
expect(titles).toEqual(['m6', 'm6', 'm7'])
|
||||
expect(host.askedFrom).toEqual([5, 5])
|
||||
expect(persistedSeq()).toBe(7)
|
||||
})
|
||||
|
||||
it('re-shows a live event whose show threw, instead of dropping it as already seen', async () => {
|
||||
// The same hole without any catch-up failing: the live path marks seen before the
|
||||
// show, so a rejected show leaves the key behind while the watermark stays put.
|
||||
// The next catch-up dutifully re-fetches the seq and the guard eats it.
|
||||
storage.set(WATERMARK_KEY, JSON.stringify({ seq: 5, epoch: 'epoch-1' }))
|
||||
const host = makeHostClient()
|
||||
host.setOutcome({ kind: 'ok', notifications: [] })
|
||||
|
||||
let failNext = true
|
||||
vi.mocked(Notifications.scheduleNotificationAsync).mockImplementation(async () => {
|
||||
if (failNext) {
|
||||
failNext = false
|
||||
throw new Error('scheduling rejected')
|
||||
}
|
||||
return 'sched-1'
|
||||
})
|
||||
|
||||
subscribeToDesktopNotifications(host.client, 'host-1')
|
||||
host.onData?.({ type: 'ready', subscriptionId: 'sub-1', epoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
|
||||
host.onData?.({ ...notification(6), notificationEpoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
expect(persistedSeq()).toBe(5)
|
||||
|
||||
host.setOutcome({ kind: 'ok', notifications: [notification(6)] })
|
||||
host.onData?.({ type: 'ready', subscriptionId: 'sub-1', epoch: 'epoch-1' })
|
||||
await flushAsync()
|
||||
|
||||
const titles = vi
|
||||
.mocked(Notifications.scheduleNotificationAsync)
|
||||
.mock.calls.map((call) => (call[0] as { content: { title: string } }).content.title)
|
||||
expect(titles).toEqual(['m6', 'm6'])
|
||||
expect(persistedSeq()).toBe(6)
|
||||
})
|
||||
})
|
||||
|
|
@ -135,6 +135,9 @@ export type HostNotificationSession = {
|
|||
// Counter lifetime lastDeliveredSeq belongs to; null until one is known. A
|
||||
// mismatch on reconnect means the desktop restarted and the watermark is void.
|
||||
lastDeliveredEpoch: string | null
|
||||
// Highest seq known delivered CONTIGUOUSLY, frozen here while a catch-up is
|
||||
// outstanding; null when none has failed. See quarantineCatchUpWatermark.
|
||||
catchUpQuarantineSeq: number | null
|
||||
seen: ReturnType<typeof createSeenNotificationGuard>
|
||||
// False only until the host's first subscription reaches 'ready' — a true cold open.
|
||||
connectedBefore: boolean
|
||||
|
|
@ -161,6 +164,7 @@ export function getHostNotificationSession(hostId: string): HostNotificationSess
|
|||
session = {
|
||||
lastDeliveredSeq: 0,
|
||||
lastDeliveredEpoch: null,
|
||||
catchUpQuarantineSeq: null,
|
||||
seen: createSeenNotificationGuard(),
|
||||
connectedBefore: false,
|
||||
hadStoredWatermark: false,
|
||||
|
|
@ -239,6 +243,55 @@ export function resetHostNotificationSessionsForTests(): void {
|
|||
sessionsByHost.clear()
|
||||
}
|
||||
|
||||
/**
|
||||
* Freeze the catch-up watermark at the last seq known delivered contiguously,
|
||||
* after a catch-up that did not complete.
|
||||
*
|
||||
* Why: live delivery advances lastDeliveredSeq unconditionally, so an abandoned
|
||||
* catch-up otherwise lets the NEXT one ask from above the range it gave up on —
|
||||
* the desktop cuts by seq, so those notifications are never replayed and are
|
||||
* gone. Lowest wins: an earlier failure's gap is still open.
|
||||
*/
|
||||
export function quarantineCatchUpWatermark(
|
||||
session: HostNotificationSession,
|
||||
hostId: string,
|
||||
contiguousSeq: number
|
||||
): void {
|
||||
session.catchUpQuarantineSeq =
|
||||
session.catchUpQuarantineSeq == null
|
||||
? contiguousSeq
|
||||
: Math.min(session.catchUpQuarantineSeq, contiguousSeq)
|
||||
// Why re-persist: a live event delivered while the catch-up was still in flight
|
||||
// already stored a seq above the gap. Clamping only later writes would leave that
|
||||
// value on disk, so a restart still resumes past the abandoned range.
|
||||
void saveWatermark(hostId, {
|
||||
seq: catchUpWatermarkSeq(session),
|
||||
epoch: session.lastDeliveredEpoch
|
||||
})
|
||||
}
|
||||
|
||||
/** Lift the quarantine once a catch-up completes, persisting what it held back. */
|
||||
export function resolveCatchUpQuarantine(session: HostNotificationSession, hostId: string): void {
|
||||
if (session.catchUpQuarantineSeq == null) {
|
||||
return
|
||||
}
|
||||
session.catchUpQuarantineSeq = null
|
||||
void saveWatermark(hostId, {
|
||||
seq: session.lastDeliveredSeq,
|
||||
epoch: session.lastDeliveredEpoch
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* The seq a catch-up may ask from and the highest seq safe to persist — the live
|
||||
* watermark, clamped to any open gap.
|
||||
*/
|
||||
export function catchUpWatermarkSeq(session: HostNotificationSession): number {
|
||||
return session.catchUpQuarantineSeq == null
|
||||
? session.lastDeliveredSeq
|
||||
: Math.min(session.catchUpQuarantineSeq, session.lastDeliveredSeq)
|
||||
}
|
||||
|
||||
// Why (#8591): the desktop's seq counter restarts at 0 every launch, so a watermark
|
||||
// from a previous lifetime indexes a counter that no longer exists. Comparing it
|
||||
// against the fresh counter makes `lastSeenSeq >= seq` true for everything and
|
||||
|
|
@ -262,6 +315,8 @@ export function adoptNotificationEpoch(
|
|||
// counter re-issues those same low seqs, so a stale `seq:1` would silently drop
|
||||
// the new counter's first bell. The dedup window belongs to one counter lifetime.
|
||||
session.seen.clear()
|
||||
// The quarantined gap indexed the dead counter; the watermark it guarded is gone too.
|
||||
session.catchUpQuarantineSeq = null
|
||||
session.lastDeliveredEpoch = epoch
|
||||
void saveWatermark(hostId, { seq: 0, epoch })
|
||||
}
|
||||
|
|
|
|||
|
|
@ -58,4 +58,40 @@ describe('attachment image cache', () => {
|
|||
expect(await p3).toBe('data:image/png;base64,OK==')
|
||||
expect(_getAttachmentImageCacheSize()).toBe(1)
|
||||
})
|
||||
|
||||
it('does not repopulate after "disconnect all" when the site was cleared before', async () => {
|
||||
// Summed epochs read the same before a global clear (1 + 0) and after it (0 + 1).
|
||||
clearAttachmentImagesForSite('site-a')
|
||||
|
||||
let resolveLoad: (value: { dataUrl: string; byteSize: number } | null) => void = () => {}
|
||||
const inFlight = loadAttachmentDataUrlWithCache({
|
||||
siteId: 'site-a',
|
||||
attachmentId: '1',
|
||||
load: () =>
|
||||
new Promise<{ dataUrl: string; byteSize: number } | null>((resolve) => {
|
||||
resolveLoad = resolve
|
||||
})
|
||||
})
|
||||
|
||||
clearAttachmentImagesForSite()
|
||||
resolveLoad({ dataUrl: 'data:image/png;base64,SECRET==', byteSize: 4 })
|
||||
|
||||
// The waiter still gets its bytes; nothing survives in the cache.
|
||||
expect(await inFlight).toBe('data:image/png;base64,SECRET==')
|
||||
expect(getCachedAttachmentDataUrl('site-a', '1')).toBeNull()
|
||||
expect(_getAttachmentImageCacheSize()).toBe(0)
|
||||
})
|
||||
|
||||
it('still caches a load that spans no clear at all', async () => {
|
||||
clearAttachmentImagesForSite('site-a')
|
||||
|
||||
const dataUrl = await loadAttachmentDataUrlWithCache({
|
||||
siteId: 'site-a',
|
||||
attachmentId: '1',
|
||||
load: async () => ({ dataUrl: 'data:image/png;base64,OK==', byteSize: 2 })
|
||||
})
|
||||
|
||||
expect(dataUrl).toBe('data:image/png;base64,OK==')
|
||||
expect(getCachedAttachmentDataUrl('site-a', '1')).toBe('data:image/png;base64,OK==')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -15,15 +15,23 @@ type CacheEntry = {
|
|||
const cache = new Map<string, CacheEntry>()
|
||||
const inFlight = new Map<string, Promise<string | null>>()
|
||||
// Why: mid-flight downloads must not repopulate cache after disconnect/clearToken.
|
||||
let cacheEpoch = 0
|
||||
// Why ONE ticker across both scopes: summing separate counters lets distinct clear
|
||||
// states collide, passing the guard and re-inserting credentialed bytes.
|
||||
let epochTicker = 0
|
||||
let globalEpoch = 0
|
||||
const siteEpoch = new Map<string, number>()
|
||||
|
||||
function cacheKey(siteId: string, attachmentId: string): string {
|
||||
return `${siteId}::${attachmentId}`
|
||||
}
|
||||
|
||||
function nextEpoch(): number {
|
||||
epochTicker += 1
|
||||
return epochTicker
|
||||
}
|
||||
|
||||
function currentEpoch(siteId: string): number {
|
||||
return (siteEpoch.get(siteId) ?? 0) + cacheEpoch
|
||||
return Math.max(globalEpoch, siteEpoch.get(siteId) ?? 0)
|
||||
}
|
||||
|
||||
function pruneExpired(now = Date.now()): void {
|
||||
|
|
@ -137,11 +145,11 @@ export function clearAttachmentImagesForSite(siteId?: string): void {
|
|||
if (siteId == null || siteId === '') {
|
||||
cache.clear()
|
||||
inFlight.clear()
|
||||
cacheEpoch += 1
|
||||
globalEpoch = nextEpoch()
|
||||
siteEpoch.clear()
|
||||
return
|
||||
}
|
||||
siteEpoch.set(siteId, (siteEpoch.get(siteId) ?? 0) + 1)
|
||||
siteEpoch.set(siteId, nextEpoch())
|
||||
const prefix = `${siteId}::`
|
||||
for (const key of cache.keys()) {
|
||||
if (key.startsWith(prefix)) {
|
||||
|
|
@ -159,7 +167,8 @@ export function clearAttachmentImagesForSite(siteId?: string): void {
|
|||
export function _resetAttachmentImageCache(): void {
|
||||
cache.clear()
|
||||
inFlight.clear()
|
||||
cacheEpoch = 0
|
||||
epochTicker = 0
|
||||
globalEpoch = 0
|
||||
siteEpoch.clear()
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ type FakeDocOptions = {
|
|||
function createFakeDocument(options?: FakeDocOptions) {
|
||||
const listeners: ((event: unknown) => void)[] = []
|
||||
const clipboardData = { setData: vi.fn() }
|
||||
const stopImmediatePropagation = vi.fn()
|
||||
const createElement = vi.fn()
|
||||
const appendChild = vi.fn()
|
||||
const execCommand = vi.fn((command: string) => {
|
||||
|
|
@ -24,7 +25,8 @@ function createFakeDocument(options?: FakeDocOptions) {
|
|||
for (const listener of listeners.slice()) {
|
||||
listener({
|
||||
clipboardData: (options?.withClipboardData ?? true) ? clipboardData : undefined,
|
||||
preventDefault: vi.fn()
|
||||
preventDefault: vi.fn(),
|
||||
stopImmediatePropagation
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -50,7 +52,15 @@ function createFakeDocument(options?: FakeDocOptions) {
|
|||
body: { appendChild }
|
||||
} as unknown as Document
|
||||
|
||||
return { doc, clipboardData, createElement, appendChild, execCommand, listeners }
|
||||
return {
|
||||
doc,
|
||||
clipboardData,
|
||||
stopImmediatePropagation,
|
||||
createElement,
|
||||
appendChild,
|
||||
execCommand,
|
||||
listeners
|
||||
}
|
||||
}
|
||||
|
||||
describe('copyClipboardTextViaExecCommand', () => {
|
||||
|
|
|
|||
|
|
@ -11,17 +11,19 @@ export function copyClipboardTextViaExecCommand(text: string, doc: Document = do
|
|||
return
|
||||
}
|
||||
event.clipboardData.setData('text/plain', text)
|
||||
// Why bubble + stop: xterm's listener on terminal.element overwrites text/plain,
|
||||
// and preventDefault alone does not stop it or any later window-level handler.
|
||||
event.stopImmediatePropagation()
|
||||
event.preventDefault()
|
||||
served = true
|
||||
}
|
||||
// Why capture: run before any app-level copy handler so the terminal text wins.
|
||||
doc.addEventListener('copy', onCopy, true)
|
||||
doc.addEventListener('copy', onCopy)
|
||||
try {
|
||||
// Chromium can return true even when no handler supplied clipboard data.
|
||||
return doc.execCommand('copy') === true && served
|
||||
} catch {
|
||||
return false
|
||||
} finally {
|
||||
doc.removeEventListener('copy', onCopy, true)
|
||||
doc.removeEventListener('copy', onCopy)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,113 @@
|
|||
// @vitest-environment happy-dom
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { copyClipboardTextViaExecCommand } from './web-clipboard-copy-fallback'
|
||||
|
||||
// Why a real DOM: only genuine capture/bubble propagation reproduces the ordering bug.
|
||||
|
||||
type ClipboardDataStub = {
|
||||
setData: (format: string, value: string) => void
|
||||
getData: (format: string) => string
|
||||
}
|
||||
|
||||
function createClipboardDataStub(): ClipboardDataStub {
|
||||
const store = new Map<string, string>()
|
||||
return {
|
||||
setData(format, value) {
|
||||
store.set(format, value)
|
||||
},
|
||||
getData(format) {
|
||||
return store.get(format) ?? ''
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Stands in for xterm's copyHandler: bubble-phase, overwrites text/plain. */
|
||||
function mountTerminalWithSelection(selectionText: string): HTMLElement {
|
||||
const terminalElement = document.createElement('div')
|
||||
document.body.appendChild(terminalElement)
|
||||
terminalElement.addEventListener('copy', (event) => {
|
||||
const clipboardData = (event as unknown as { clipboardData?: ClipboardDataStub }).clipboardData
|
||||
clipboardData?.setData('text/plain', selectionText)
|
||||
event.preventDefault()
|
||||
})
|
||||
return terminalElement
|
||||
}
|
||||
|
||||
/** Stands in for execCommand('copy'): dispatches from the DOM selection's anchor. */
|
||||
function stubExecCommand(
|
||||
source: HTMLElement,
|
||||
clipboardData: ClipboardDataStub,
|
||||
// Runs once the fallback has registered, so a handler added here is ordered after it.
|
||||
beforeDispatch?: () => void
|
||||
): void {
|
||||
;(document as unknown as { execCommand: (command: string) => boolean }).execCommand = (
|
||||
command
|
||||
) => {
|
||||
if (command !== 'copy') {
|
||||
return false
|
||||
}
|
||||
beforeDispatch?.()
|
||||
const event = new Event('copy', { bubbles: true, cancelable: true })
|
||||
Object.defineProperty(event, 'clipboardData', { value: clipboardData })
|
||||
source.dispatchEvent(event)
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
describe('web copy fallback vs. the terminal selection', () => {
|
||||
afterEach(() => {
|
||||
document.body.innerHTML = ''
|
||||
})
|
||||
|
||||
it('copies the requested text even when the selection anchor is inside a terminal', () => {
|
||||
// Copy Path / Copy Pane ID leave the selection in the terminal, so the copy event
|
||||
// dispatches from there and a capture-phase write loses to xterm's bubble handler.
|
||||
const terminalElement = mountTerminalWithSelection('rm -rf ./secret-dir')
|
||||
const clipboardData = createClipboardDataStub()
|
||||
stubExecCommand(terminalElement, clipboardData)
|
||||
|
||||
expect(copyClipboardTextViaExecCommand('/Users/me/repo/src/index.ts', document)).toBe(true)
|
||||
expect(clipboardData.getData('text/plain')).toBe('/Users/me/repo/src/index.ts')
|
||||
})
|
||||
|
||||
it('wins over a later document-level copy handler', () => {
|
||||
// Same target as the fallback's own listener, so only stopImmediatePropagation
|
||||
// suppresses it — stopPropagation would still let it run and clobber text/plain.
|
||||
const source = document.createElement('div')
|
||||
document.body.appendChild(source)
|
||||
const clipboardData = createClipboardDataStub()
|
||||
const clobber = (event: Event): void => {
|
||||
const data = (event as unknown as { clipboardData?: ClipboardDataStub }).clipboardData
|
||||
data?.setData('text/plain', 'later document handler')
|
||||
}
|
||||
stubExecCommand(source, clipboardData, () => document.addEventListener('copy', clobber))
|
||||
|
||||
try {
|
||||
expect(copyClipboardTextViaExecCommand('pane-42', document)).toBe(true)
|
||||
expect(clipboardData.getData('text/plain')).toBe('pane-42')
|
||||
} finally {
|
||||
document.removeEventListener('copy', clobber)
|
||||
}
|
||||
})
|
||||
|
||||
it('wins over a copy handler that still runs after the document', () => {
|
||||
// Bubbling hits the document before the window, so only stopImmediatePropagation
|
||||
// protects against a window-level listener.
|
||||
const source = document.createElement('div')
|
||||
document.body.appendChild(source)
|
||||
const clipboardData = createClipboardDataStub()
|
||||
stubExecCommand(source, clipboardData)
|
||||
const clobber = (event: Event): void => {
|
||||
const data = (event as unknown as { clipboardData?: ClipboardDataStub }).clipboardData
|
||||
data?.setData('text/plain', 'later window handler')
|
||||
}
|
||||
window.addEventListener('copy', clobber)
|
||||
|
||||
try {
|
||||
expect(copyClipboardTextViaExecCommand('pane-42', document)).toBe(true)
|
||||
expect(clipboardData.getData('text/plain')).toBe('pane-42')
|
||||
} finally {
|
||||
window.removeEventListener('copy', clobber)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
@ -70,7 +70,11 @@ function installExecCommandClipboardDocument(execCommandResult = true): {
|
|||
const execCommand = vi.fn((command: string) => {
|
||||
if (command === 'copy') {
|
||||
for (const listener of listeners.slice()) {
|
||||
listener({ clipboardData: { setData }, preventDefault: vi.fn() })
|
||||
listener({
|
||||
clipboardData: { setData },
|
||||
preventDefault: vi.fn(),
|
||||
stopImmediatePropagation: vi.fn()
|
||||
})
|
||||
}
|
||||
}
|
||||
return execCommandResult
|
||||
|
|
|
|||
Loading…
Reference in New Issue