Skip to content

Commit 051792f

Browse files
esokulluesokullu
committed
Preserve handoff ownership across clears
1 parent 2889b7f commit 051792f

9 files changed

Lines changed: 177 additions & 27 deletions

File tree

‎src/chrome/src/background.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2684,7 +2684,10 @@ async function handleMessage(msg, sender) {
26842684
});
26852685

26862686
case 'clear_tab_chat':
2687-
return await tabChatHandoff.clear(msg.tabId || sender.tab?.id);
2687+
return await tabChatHandoff.clear(msg.tabId || sender.tab?.id, {
2688+
ownerId: msg.handoffOwnerId,
2689+
handoffGeneration: msg.handoffGeneration,
2690+
});
26882691

26892692
case 'list_scheduled_jobs': {
26902693
const tabId = msg.tabId || sender.tab?.id || null;

‎src/chrome/src/context-menu-storage.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ export function createContextMenuStorage(getStore) {
261261
});
262262
}
263263

264-
async function reserve(tabId, promptId, claimantId, onReserve, now = Date.now()) {
264+
async function reserve(tabId, promptId, claimantId, onReserve, now) {
265265
const normalizedPromptId = String(promptId || '');
266266
const normalizedClaimantId = String(claimantId || '');
267267
if (!normalizedPromptId || !normalizedClaimantId || typeof onReserve !== 'function') {
@@ -292,7 +292,11 @@ export function createContextMenuStorage(getStore) {
292292
const samePrompt = String(activeClaim?.promptId || '') === normalizedPromptId;
293293
const sameClaimant = String(activeClaim?.claimantId || '') === normalizedClaimantId;
294294
const leaseExpiresAt = Number(activeClaim?.expiresAt || 0);
295-
const activeLease = leaseExpiresAt > Number(now);
295+
const suppliedNow = Number(now);
296+
const validationNow = now == null || !Number.isFinite(suppliedNow)
297+
? Date.now()
298+
: suppliedNow;
299+
const activeLease = leaseExpiresAt > validationNow;
296300
if (!samePrompt || !sameClaimant || !activeLease) {
297301
return {
298302
ok: true,

‎src/chrome/src/ui/sidepanel.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1645,11 +1645,27 @@ function clearCachedTabChat(tabId) {
16451645
persistTimerTabId = null;
16461646
}
16471647
tabChats.delete(tabId);
1648-
tabChatHandoffGenerations.delete(Number(tabId));
16491648
return enqueueTabChatOperation(tabId, async (numericTabId) => {
16501649
tabChats.delete(numericTabId);
16511650
try {
1652-
await sendToBackground('clear_tab_chat', { tabId: numericTabId });
1651+
let handoffGeneration = tabChatHandoffGenerations.get(numericTabId);
1652+
if (!Number.isFinite(handoffGeneration)) {
1653+
const ownership = await sendToBackground('load_tab_chat', {
1654+
tabId: numericTabId,
1655+
waitForHandoff: true,
1656+
handoffOwnerId: tabChatHandoffOwnerId,
1657+
});
1658+
if (ownership?.handoffOwnerId === tabChatHandoffOwnerId
1659+
&& Number.isFinite(Number(ownership?.handoffGeneration))) {
1660+
handoffGeneration = Number(ownership.handoffGeneration);
1661+
tabChatHandoffGenerations.set(numericTabId, handoffGeneration);
1662+
}
1663+
}
1664+
await sendToBackground('clear_tab_chat', {
1665+
tabId: numericTabId,
1666+
handoffOwnerId: tabChatHandoffOwnerId,
1667+
handoffGeneration,
1668+
});
16531669
} catch (e) { /* ignore */ }
16541670
return { ok: true };
16551671
});

‎src/chrome/src/ui/tab-chat-persistence.js‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,10 @@ export function createTabChatHandoffCoordinator(storageArea, {
274274
return enqueue(numericTabId, async (queuedTabId) => {
275275
const normalizedOwnerId = String(ownerId || '');
276276
const normalizedGeneration = Number(handoffGeneration);
277-
if (normalizedOwnerId && Number.isFinite(normalizedGeneration) && normalizedGeneration > 0) {
277+
if (normalizedOwnerId) {
278+
if (!Number.isFinite(normalizedGeneration) || normalizedGeneration <= 0) {
279+
return { ok: true, skipped: true, reason: 'stale-handoff' };
280+
}
278281
const state = await readHandoffState(queuedTabId);
279282
if (!state
280283
|| state.ownerId !== normalizedOwnerId
@@ -339,16 +342,32 @@ export function createTabChatHandoffCoordinator(storageArea, {
339342
});
340343
}
341344

342-
function clear(tabId) {
345+
function clear(tabId, { ownerId = '', handoffGeneration = null } = {}) {
343346
const numericTabId = normalizeTabId(tabId);
344347
if (numericTabId == null) return Promise.resolve({ ok: false, error: 'No tab ID' });
345348
return enqueue(numericTabId, async (queuedTabId) => {
349+
const normalizedOwnerId = String(ownerId || '');
350+
const normalizedGeneration = Number(handoffGeneration);
351+
if (normalizedOwnerId) {
352+
if (!Number.isFinite(normalizedGeneration) || normalizedGeneration <= 0) {
353+
return { ok: true, skipped: true, reason: 'stale-handoff' };
354+
}
355+
const state = await readHandoffState(queuedTabId);
356+
if (!state
357+
|| state.ownerId !== normalizedOwnerId
358+
|| state.generation !== normalizedGeneration) {
359+
return { ok: true, skipped: true, reason: 'stale-handoff' };
360+
}
361+
}
346362
latestHtml.delete(queuedTabId);
347-
await storageArea.remove([
348-
TAB_CHAT_PREFIX + queuedTabId,
349-
TAB_CHAT_HANDOFF_PREFIX + queuedTabId,
350-
]);
351-
return { ok: true };
363+
const keys = [TAB_CHAT_PREFIX + queuedTabId];
364+
if (!normalizedOwnerId) keys.push(TAB_CHAT_HANDOFF_PREFIX + queuedTabId);
365+
await storageArea.remove(keys);
366+
return {
367+
ok: true,
368+
handoffOwnerId: normalizedOwnerId || null,
369+
handoffGeneration: normalizedOwnerId ? normalizedGeneration : null,
370+
};
352371
});
353372
}
354373

‎src/firefox/src/background.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2371,7 +2371,10 @@ async function handleMessage(msg, sender) {
23712371
});
23722372

23732373
case 'clear_tab_chat':
2374-
return await tabChatHandoff.clear(msg.tabId || sender.tab?.id);
2374+
return await tabChatHandoff.clear(msg.tabId || sender.tab?.id, {
2375+
ownerId: msg.handoffOwnerId,
2376+
handoffGeneration: msg.handoffGeneration,
2377+
});
23752378

23762379
case 'list_scheduled_jobs': {
23772380
const tabId = msg.tabId || sender.tab?.id || null;

‎src/firefox/src/context-menu-storage.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ export function createContextMenuStorage(getStore) {
261261
});
262262
}
263263

264-
async function reserve(tabId, promptId, claimantId, onReserve, now = Date.now()) {
264+
async function reserve(tabId, promptId, claimantId, onReserve, now) {
265265
const normalizedPromptId = String(promptId || '');
266266
const normalizedClaimantId = String(claimantId || '');
267267
if (!normalizedPromptId || !normalizedClaimantId || typeof onReserve !== 'function') {
@@ -292,7 +292,11 @@ export function createContextMenuStorage(getStore) {
292292
const samePrompt = String(activeClaim?.promptId || '') === normalizedPromptId;
293293
const sameClaimant = String(activeClaim?.claimantId || '') === normalizedClaimantId;
294294
const leaseExpiresAt = Number(activeClaim?.expiresAt || 0);
295-
const activeLease = leaseExpiresAt > Number(now);
295+
const suppliedNow = Number(now);
296+
const validationNow = now == null || !Number.isFinite(suppliedNow)
297+
? Date.now()
298+
: suppliedNow;
299+
const activeLease = leaseExpiresAt > validationNow;
296300
if (!samePrompt || !sameClaimant || !activeLease) {
297301
return {
298302
ok: true,

‎src/firefox/src/ui/sidepanel.js‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1510,11 +1510,27 @@ function clearCachedTabChat(tabId) {
15101510
persistTimerTabId = null;
15111511
}
15121512
tabChats.delete(tabId);
1513-
tabChatHandoffGenerations.delete(Number(tabId));
15141513
return enqueueTabChatOperation(tabId, async (numericTabId) => {
15151514
tabChats.delete(numericTabId);
15161515
try {
1517-
await sendToBackground('clear_tab_chat', { tabId: numericTabId });
1516+
let handoffGeneration = tabChatHandoffGenerations.get(numericTabId);
1517+
if (!Number.isFinite(handoffGeneration)) {
1518+
const ownership = await sendToBackground('load_tab_chat', {
1519+
tabId: numericTabId,
1520+
waitForHandoff: true,
1521+
handoffOwnerId: tabChatHandoffOwnerId,
1522+
});
1523+
if (ownership?.handoffOwnerId === tabChatHandoffOwnerId
1524+
&& Number.isFinite(Number(ownership?.handoffGeneration))) {
1525+
handoffGeneration = Number(ownership.handoffGeneration);
1526+
tabChatHandoffGenerations.set(numericTabId, handoffGeneration);
1527+
}
1528+
}
1529+
await sendToBackground('clear_tab_chat', {
1530+
tabId: numericTabId,
1531+
handoffOwnerId: tabChatHandoffOwnerId,
1532+
handoffGeneration,
1533+
});
15181534
} catch (e) { /* ignore */ }
15191535
return { ok: true };
15201536
});

‎src/firefox/src/ui/tab-chat-persistence.js‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,10 @@ export function createTabChatHandoffCoordinator(storageArea, {
274274
return enqueue(numericTabId, async (queuedTabId) => {
275275
const normalizedOwnerId = String(ownerId || '');
276276
const normalizedGeneration = Number(handoffGeneration);
277-
if (normalizedOwnerId && Number.isFinite(normalizedGeneration) && normalizedGeneration > 0) {
277+
if (normalizedOwnerId) {
278+
if (!Number.isFinite(normalizedGeneration) || normalizedGeneration <= 0) {
279+
return { ok: true, skipped: true, reason: 'stale-handoff' };
280+
}
278281
const state = await readHandoffState(queuedTabId);
279282
if (!state
280283
|| state.ownerId !== normalizedOwnerId
@@ -339,16 +342,32 @@ export function createTabChatHandoffCoordinator(storageArea, {
339342
});
340343
}
341344

342-
function clear(tabId) {
345+
function clear(tabId, { ownerId = '', handoffGeneration = null } = {}) {
343346
const numericTabId = normalizeTabId(tabId);
344347
if (numericTabId == null) return Promise.resolve({ ok: false, error: 'No tab ID' });
345348
return enqueue(numericTabId, async (queuedTabId) => {
349+
const normalizedOwnerId = String(ownerId || '');
350+
const normalizedGeneration = Number(handoffGeneration);
351+
if (normalizedOwnerId) {
352+
if (!Number.isFinite(normalizedGeneration) || normalizedGeneration <= 0) {
353+
return { ok: true, skipped: true, reason: 'stale-handoff' };
354+
}
355+
const state = await readHandoffState(queuedTabId);
356+
if (!state
357+
|| state.ownerId !== normalizedOwnerId
358+
|| state.generation !== normalizedGeneration) {
359+
return { ok: true, skipped: true, reason: 'stale-handoff' };
360+
}
361+
}
346362
latestHtml.delete(queuedTabId);
347-
await storageArea.remove([
348-
TAB_CHAT_PREFIX + queuedTabId,
349-
TAB_CHAT_HANDOFF_PREFIX + queuedTabId,
350-
]);
351-
return { ok: true };
363+
const keys = [TAB_CHAT_PREFIX + queuedTabId];
364+
if (!normalizedOwnerId) keys.push(TAB_CHAT_HANDOFF_PREFIX + queuedTabId);
365+
await storageArea.remove(keys);
366+
return {
367+
ok: true,
368+
handoffOwnerId: normalizedOwnerId || null,
369+
handoffGeneration: normalizedOwnerId ? normalizedGeneration : null,
370+
};
352371
});
353372
}
354373

‎test/run.js‎

Lines changed: 69 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18177,6 +18177,28 @@ test('tab-chat handoff coordinator orders a returning-panel read behind the outg
1817718177
});
1817818178
assert.equal(staleWrite.skipped, true, `${label}: a late write from the old generation must be rejected`);
1817918179
assert.equal(values[`${persistence.TAB_CHAT_PREFIX}7`], '<div>fresh</div>', `${label}: stale delivery must not overwrite the acknowledged snapshot`);
18180+
18181+
const cleared = await coordinator.clear(7, {
18182+
ownerId: 'returning-panel',
18183+
handoffGeneration: 2,
18184+
});
18185+
assert.equal(cleared.ok, true, `${label}: the visible owner should clear its transcript`);
18186+
assert.equal(values[`${persistence.TAB_CHAT_PREFIX}7`], undefined, `${label}: clear should remove transcript content`);
18187+
assert.deepEqual(
18188+
values[`${persistence.TAB_CHAT_HANDOFF_PREFIX}7`],
18189+
{ ownerId: 'returning-panel', generation: 2 },
18190+
`${label}: clear should preserve the visible document's validated handoff ownership`,
18191+
);
18192+
const unversionedWrite = await coordinator.save(7, '<div>unversioned stale</div>', {
18193+
ownerId: 'returning-panel',
18194+
});
18195+
assert.equal(unversionedWrite.skipped, true, `${label}: owner-tagged writes without a generation must fail closed`);
18196+
const postClearWrite = await coordinator.save(7, '<div>new conversation</div>', {
18197+
ownerId: 'returning-panel',
18198+
handoffGeneration: 2,
18199+
});
18200+
assert.equal(postClearWrite.ok, true, `${label}: the preserved owner should persist the new conversation`);
18201+
assert.equal(values[`${persistence.TAB_CHAT_PREFIX}7`], '<div>new conversation</div>', `${label}: the post-clear transcript should remain current`);
1818018202
}
1818118203
});
1818218204

@@ -18199,7 +18221,8 @@ test('chrome sidepanel serializes tab-chat storage writes with clears and reads'
1819918221
const clearBody = panel.slice(clearStart, panel.indexOf('\n}\n\nasync function renderClearedConversationForTab', clearStart) + 2);
1820018222
assert.match(clearBody, /tabChats\.delete\(tabId\);/, 'chrome: clearing tab chat should delete the cached HTML before queuing storage removal');
1820118223
assert.match(clearBody, /return enqueueTabChatOperation\(tabId, async \(numericTabId\) => \{/, 'chrome: clearing tab chat should be serialized through the queue');
18202-
assert.match(clearBody, /return enqueueTabChatOperation\(tabId, async \(numericTabId\) => \{[\s\S]*?tabChats\.delete\(numericTabId\);[\s\S]*?sendToBackground\('clear_tab_chat', \{ tabId: numericTabId \}\)/, 'chrome: queued clears should delete stale HTML before clearing the shared background state');
18224+
assert.match(clearBody, /return enqueueTabChatOperation\(tabId, async \(numericTabId\) => \{[\s\S]*?tabChats\.delete\(numericTabId\);[\s\S]*?handoffGeneration = tabChatHandoffGenerations\.get\(numericTabId\);[\s\S]*?sendToBackground\('load_tab_chat', \{[\s\S]*?waitForHandoff: true,[\s\S]*?sendToBackground\('clear_tab_chat', \{[\s\S]*?handoffOwnerId: tabChatHandoffOwnerId,[\s\S]*?handoffGeneration,/, 'chrome: queued clears should retain or reacquire ownership before clearing shared state');
18225+
assert.doesNotMatch(clearBody, /tabChatHandoffGenerations\.delete/, 'chrome: clearing content must not discard the visible document handoff generation');
1820318226
});
1820418227

1820518228
test('firefox sidepanel serializes tab-chat storage writes with clears and reads', () => {
@@ -18221,7 +18244,8 @@ test('firefox sidepanel serializes tab-chat storage writes with clears and reads
1822118244
const clearBody = panel.slice(clearStart, panel.indexOf('\n}\n\n// Save current tab', clearStart) + 2);
1822218245
assert.match(clearBody, /tabChats\.delete\(tabId\);/, 'firefox: clearing tab chat should delete the cached HTML before queuing storage removal');
1822318246
assert.match(clearBody, /return enqueueTabChatOperation\(tabId, async \(numericTabId\) => \{/, 'firefox: clearing tab chat should be serialized through the queue');
18224-
assert.match(clearBody, /return enqueueTabChatOperation\(tabId, async \(numericTabId\) => \{[\s\S]*?tabChats\.delete\(numericTabId\);[\s\S]*?sendToBackground\('clear_tab_chat', \{ tabId: numericTabId \}\)/, 'firefox: queued clears should delete stale HTML before clearing the shared background state');
18247+
assert.match(clearBody, /return enqueueTabChatOperation\(tabId, async \(numericTabId\) => \{[\s\S]*?tabChats\.delete\(numericTabId\);[\s\S]*?handoffGeneration = tabChatHandoffGenerations\.get\(numericTabId\);[\s\S]*?sendToBackground\('load_tab_chat', \{[\s\S]*?waitForHandoff: true,[\s\S]*?sendToBackground\('clear_tab_chat', \{[\s\S]*?handoffOwnerId: tabChatHandoffOwnerId,[\s\S]*?handoffGeneration,/, 'firefox: queued clears should retain or reacquire ownership before clearing shared state');
18248+
assert.doesNotMatch(clearBody, /tabChatHandoffGenerations\.delete/, 'firefox: clearing content must not discard the visible document handoff generation');
1822518249
});
1822618250

1822718251
test('chrome sidepanel cancels stale tab-chat persistence when clearing a tab', () => {
@@ -18242,7 +18266,7 @@ test('chrome sidepanel cancels stale tab-chat persistence when clearing a tab',
1824218266
const clearBody = panel.slice(clearStart, panel.indexOf('\n}\n\nasync function renderClearedConversationForTab', clearStart) + 2);
1824318267
assert.match(
1824418268
clearBody,
18245-
/if \(persistTimer && persistTimerTabId === tabId\) \{[\s\S]*?clearTimeout\(persistTimer\);[\s\S]*?persistTimer = null;[\s\S]*?persistTimerTabId = null;[\s\S]*?\}[\s\S]*?tabChats\.delete\(tabId\);[\s\S]*?return enqueueTabChatOperation\(tabId, async \(numericTabId\) => \{[\s\S]*?tabChats\.delete\(numericTabId\);[\s\S]*?sendToBackground\('clear_tab_chat', \{ tabId: numericTabId \}\)/,
18269+
/if \(persistTimer && persistTimerTabId === tabId\) \{[\s\S]*?clearTimeout\(persistTimer\);[\s\S]*?persistTimer = null;[\s\S]*?persistTimerTabId = null;[\s\S]*?\}[\s\S]*?tabChats\.delete\(tabId\);[\s\S]*?return enqueueTabChatOperation\(tabId, async \(numericTabId\) => \{[\s\S]*?tabChats\.delete\(numericTabId\);[\s\S]*?sendToBackground\('clear_tab_chat', \{[\s\S]*?handoffGeneration,/,
1824618270
'chrome: clearing a tab should cancel any pending stale write before removing cached chat',
1824718271
);
1824818272
});
@@ -21905,6 +21929,11 @@ test('context-menu ownership and stale-panel persistence guards are wired in bot
2190521929
/createTabChatHandoffCoordinator\([\s\S]*?case 'persist_tab_chat':[\s\S]*?tabChatHandoff\.save[\s\S]*?case 'load_tab_chat':[\s\S]*?tabChatHandoff\.load/,
2190621930
`${label}: transcript handoff reads and writes should share a background coordinator`,
2190721931
);
21932+
assert.match(
21933+
background,
21934+
/case 'clear_tab_chat':[\s\S]*?tabChatHandoff\.clear\([\s\S]*?ownerId: msg\.handoffOwnerId,[\s\S]*?handoffGeneration: msg\.handoffGeneration/,
21935+
`${label}: New Chat should clear content through the visible document's validated handoff ownership`,
21936+
);
2190821937
assert.match(
2190921938
background,
2191021939
/requestHandoff:[\s\S]*?tab_chat_handoff_request[\s\S]*?ownerId,[\s\S]*?generation,/,
@@ -21966,8 +21995,12 @@ test('context-menu prompt storage enforces a durable expiring lease', async () =
2196621995
['firefox', createContextMenuStorageFx, CONTEXT_MENU_CLAIM_LEASE_MS_FX],
2196721996
]) {
2196821997
const data = new Map();
21998+
let nextSetGate = null;
2196921999
const store = {
2197022000
async set(values) {
22001+
const gate = nextSetGate;
22002+
nextSetGate = null;
22003+
if (gate) await gate.promise;
2197122004
Object.entries(values || {}).forEach(([key, value]) => data.set(key, value));
2197222005
},
2197322006
async get(key) {
@@ -22039,6 +22072,39 @@ test('context-menu prompt storage enforces a durable expiring lease', async () =
2203922072
await storage.clear(prompt.tabId, prompt.id);
2204022073
assert.equal(data.has(storage.key(prompt.tabId)), false, `${label}: accepting the run should clear the durable prompt`);
2204122074
assert.equal(data.has(storage.claimKey(prompt.tabId)), false, `${label}: accepting the run should clear its lease`);
22075+
22076+
const queuedPrompt = {
22077+
id: `${label}-queued-expiry`,
22078+
tabId: 13,
22079+
text: 'Summarize this selection',
22080+
};
22081+
await storage.save(queuedPrompt.tabId, queuedPrompt);
22082+
const queuedClaim = await storage.claim(queuedPrompt.tabId, queuedPrompt.id, 'queued-panel', 1_000);
22083+
const setGate = deferred();
22084+
nextSetGate = setGate;
22085+
const blockingSave = storage.save(queuedPrompt.tabId, queuedPrompt);
22086+
await waitMicrotasks(3);
22087+
22088+
const originalNow = Date.now;
22089+
let queuedReservation = null;
22090+
try {
22091+
Date.now = () => 1_000;
22092+
const reservation = storage.reserve(
22093+
queuedPrompt.tabId,
22094+
queuedPrompt.id,
22095+
'queued-panel',
22096+
() => ({ accepted: true }),
22097+
);
22098+
Date.now = () => queuedClaim.leaseExpiresAt;
22099+
setGate.resolve();
22100+
await blockingSave;
22101+
queuedReservation = await reservation;
22102+
} finally {
22103+
Date.now = originalNow;
22104+
setGate.resolve();
22105+
}
22106+
assert.equal(queuedReservation?.reserved, false, `${label}: queued validation must reject a lease that expired while waiting`);
22107+
assert.equal(queuedReservation?.reason, 'claim-lost', `${label}: queued expiry should require a fresh claim`);
2204222108
}
2204322109
});
2204422110

0 commit comments

Comments
 (0)