Skip to content

Commit d27bcb3

Browse files
esokulluesokullu
committed
Handle handoff and claim retry races
1 parent 37e243a commit d27bcb3

5 files changed

Lines changed: 113 additions & 26 deletions

File tree

‎src/chrome/src/ui/context-menu-prompts.js‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,15 @@ export function createContextMenuPromptHandler({
135135

136136
const currentTabId = getCurrentTabId();
137137
const clearPayload = { tabId: payload.tabId ?? currentTabId, promptId: payload.id };
138+
const releasePromptClaim = async () => {
139+
try {
140+
await sendToBackground('release_context_menu_prompt_claim', {
141+
tabId: clearPayload.tabId,
142+
promptId: payload.id,
143+
claimantId,
144+
});
145+
} catch { /* the durable lease still expires if release fails */ }
146+
};
138147
let claimResult = null;
139148
try {
140149
claimResult = await sendToBackground('claim_context_menu_prompt', {
@@ -148,13 +157,7 @@ export function createContextMenuPromptHandler({
148157
claimResult = { claimed: false, reason: 'connection', retryAfterMs: 1_000 };
149158
}
150159
if (claimResult?.claimed && !getIsDocumentVisible()) {
151-
try {
152-
await sendToBackground('release_context_menu_prompt_claim', {
153-
tabId: clearPayload.tabId,
154-
promptId: payload.id,
155-
claimantId,
156-
});
157-
} catch { /* the durable lease still expires if release fails */ }
160+
await releasePromptClaim();
158161
claimResult = { claimed: false, reason: 'panel-hidden', retryAfterMs: 250 };
159162
}
160163
if (!claimResult?.claimed || !getIsDocumentVisible()) {
@@ -166,6 +169,15 @@ export function createContextMenuPromptHandler({
166169
drainQueuedContextMenuPrompts();
167170
return;
168171
}
172+
const promptTabStillActive = getCurrentTabId() != null
173+
&& contextMenuPromptMatchesCurrentTab(payload);
174+
if (getIsProcessing() || !promptTabStillActive) {
175+
await releasePromptClaim();
176+
runningContextMenuPromptId = null;
177+
queuedContextMenuPrompts.push(payload);
178+
drainQueuedContextMenuPrompts();
179+
return;
180+
}
169181

170182
if (getAgentMode() !== 'ask') setMode('ask');
171183
getInputEl().value = payload.text;

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,6 +1537,7 @@ function isSuccessfulAskCompletion(mode, response) {
15371537
// Also mirrored to chrome.storage.session keyed `tabChat:<tabId>` so the
15381538
// conversation survives the side panel being closed and reopened.
15391539
const tabChats = new Map();
1540+
const TAB_CHAT_LOAD_FAILED = Symbol('tab-chat-load-failed');
15401541
const tabChatOperations = new Map();
15411542
const tabChatHandoffGenerations = new Map();
15421543
const tabChatHandoffOwnerId = `sidepanel-chat-${
@@ -1586,7 +1587,9 @@ async function loadTabChat(tabId, { waitForHandoff = false } = {}) {
15861587
tabChats.delete(queuedTabId);
15871588
return null;
15881589
});
1589-
} catch (e) { /* ignore */ }
1590+
} catch (e) {
1591+
if (waitForHandoff) return TAB_CHAT_LOAD_FAILED;
1592+
}
15901593
return null;
15911594
}
15921595

@@ -3598,7 +3601,7 @@ async function init() {
35983601
const restoreTabId = currentTabId;
35993602
if (restoreTabId != null) {
36003603
const html = await loadTabChat(restoreTabId, { waitForHandoff: true });
3601-
if (currentTabId === restoreTabId && html) {
3604+
if (html !== TAB_CHAT_LOAD_FAILED && currentTabId === restoreTabId && html) {
36023605
await hydrateRestoredChatHistory(restoreTabId, html);
36033606
if (currentTabId === restoreTabId) {
36043607
messagesEl.innerHTML = html;
@@ -3759,6 +3762,7 @@ async function refreshVisibleSidePanelState() {
37593762
// from the shared session copy before that document is allowed to persist.
37603763
tabChats.delete(tabId);
37613764
const html = await loadTabChat(tabId, { waitForHandoff: true });
3765+
if (html === TAB_CHAT_LOAD_FAILED) return false;
37623766
if (document.visibilityState === 'hidden' || !sameTabId(currentTabId, tabId)) return;
37633767
renderedTabId = tabId;
37643768
if (html && html !== messagesEl.innerHTML) {
@@ -7023,7 +7027,9 @@ async function sendMessage(extraChatParams = {}) {
70237027
const submittedText = text;
70247028
const tabId = currentTabId;
70257029
let contextMenuClaimOwned = Boolean(contextMenuClaim?.promptId && contextMenuClaim?.claimantId);
7026-
const releaseOwnedContextMenuClaim = async () => {
7030+
const releaseOwnedContextMenuClaim = async (
7031+
rejection = { reason: 'panel-hidden', retryAfterMs: 250 },
7032+
) => {
70277033
if (!contextMenuClaimOwned) return;
70287034
contextMenuClaimOwned = false;
70297035
try {
@@ -7033,7 +7039,7 @@ async function sendMessage(extraChatParams = {}) {
70337039
claimantId: contextMenuClaim.claimantId,
70347040
});
70357041
} catch { /* the durable lease still expires if release fails */ }
7036-
onContextMenuClaimRejected?.({ reason: 'panel-hidden', retryAfterMs: 250 });
7042+
onContextMenuClaimRejected?.(rejection);
70377043
};
70387044
if (isConversationClearInProgress(tabId)) return false;
70397045
const permissionSkipContext = permissionSkipCommandContextForDraft(tabId, text);
@@ -7049,6 +7055,10 @@ async function sendMessage(extraChatParams = {}) {
70497055
return false;
70507056
}
70517057
if (isProcessing) {
7058+
if (contextMenuClaimOwned) {
7059+
await releaseOwnedContextMenuClaim({ reason: 'run-active', retryAfterMs: 1_000 });
7060+
return false;
7061+
}
70527062
if (isOutOfBandSlashDraft(text)) {
70537063
resetComposerHistoryNavigation(tabId);
70547064
saveInputDraftForTab(tabId, '');

‎src/firefox/src/ui/context-menu-prompts.js‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,15 @@ export function createContextMenuPromptHandler({
135135

136136
const currentTabId = getCurrentTabId();
137137
const clearPayload = { tabId: payload.tabId ?? currentTabId, promptId: payload.id };
138+
const releasePromptClaim = async () => {
139+
try {
140+
await sendToBackground('release_context_menu_prompt_claim', {
141+
tabId: clearPayload.tabId,
142+
promptId: payload.id,
143+
claimantId,
144+
});
145+
} catch { /* the durable lease still expires if release fails */ }
146+
};
138147
let claimResult = null;
139148
try {
140149
claimResult = await sendToBackground('claim_context_menu_prompt', {
@@ -148,13 +157,7 @@ export function createContextMenuPromptHandler({
148157
claimResult = { claimed: false, reason: 'connection', retryAfterMs: 1_000 };
149158
}
150159
if (claimResult?.claimed && !getIsDocumentVisible()) {
151-
try {
152-
await sendToBackground('release_context_menu_prompt_claim', {
153-
tabId: clearPayload.tabId,
154-
promptId: payload.id,
155-
claimantId,
156-
});
157-
} catch { /* the durable lease still expires if release fails */ }
160+
await releasePromptClaim();
158161
claimResult = { claimed: false, reason: 'panel-hidden', retryAfterMs: 250 };
159162
}
160163
if (!claimResult?.claimed || !getIsDocumentVisible()) {
@@ -166,6 +169,15 @@ export function createContextMenuPromptHandler({
166169
drainQueuedContextMenuPrompts();
167170
return;
168171
}
172+
const promptTabStillActive = getCurrentTabId() != null
173+
&& contextMenuPromptMatchesCurrentTab(payload);
174+
if (getIsProcessing() || !promptTabStillActive) {
175+
await releasePromptClaim();
176+
runningContextMenuPromptId = null;
177+
queuedContextMenuPrompts.push(payload);
178+
drainQueuedContextMenuPrompts();
179+
return;
180+
}
169181

170182
if (getAgentMode() !== 'ask') setMode('ask');
171183
getInputEl().value = payload.text;

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1402,6 +1402,7 @@ function updateActWarning() {
14021402
// Also mirrored to browser.storage.session keyed `tabChat:<tabId>` so the
14031403
// conversation survives the sidebar being closed and reopened.
14041404
const tabChats = new Map();
1405+
const TAB_CHAT_LOAD_FAILED = Symbol('tab-chat-load-failed');
14051406
const tabChatOperations = new Map();
14061407
const tabChatHandoffGenerations = new Map();
14071408
const tabChatHandoffOwnerId = `sidepanel-chat-${
@@ -1451,7 +1452,9 @@ async function loadTabChat(tabId, { waitForHandoff = false } = {}) {
14511452
tabChats.delete(queuedTabId);
14521453
return null;
14531454
});
1454-
} catch (e) { /* ignore */ }
1455+
} catch (e) {
1456+
if (waitForHandoff) return TAB_CHAT_LOAD_FAILED;
1457+
}
14551458
return null;
14561459
}
14571460

@@ -3450,7 +3453,7 @@ async function init() {
34503453
const restoreTabId = currentTabId;
34513454
if (restoreTabId != null) {
34523455
const html = await loadTabChat(restoreTabId, { waitForHandoff: true });
3453-
if (currentTabId === restoreTabId && html) {
3456+
if (html !== TAB_CHAT_LOAD_FAILED && currentTabId === restoreTabId && html) {
34543457
await hydrateRestoredChatHistory(restoreTabId, html);
34553458
if (currentTabId === restoreTabId) {
34563459
messagesEl.innerHTML = html;
@@ -3607,6 +3610,7 @@ async function refreshVisibleSidePanelState() {
36073610
// becomes eligible to persist this tab again.
36083611
tabChats.delete(tabId);
36093612
const html = await loadTabChat(tabId, { waitForHandoff: true });
3613+
if (html === TAB_CHAT_LOAD_FAILED) return false;
36103614
if (document.visibilityState === 'hidden' || !sameTabId(currentTabId, tabId)) return;
36113615
renderedTabId = tabId;
36123616
if (html && html !== messagesEl.innerHTML) {
@@ -6759,7 +6763,9 @@ async function sendMessage(extraChatParams = {}) {
67596763
const submittedText = text;
67606764
const tabId = currentTabId;
67616765
let contextMenuClaimOwned = Boolean(contextMenuClaim?.promptId && contextMenuClaim?.claimantId);
6762-
const releaseOwnedContextMenuClaim = async () => {
6766+
const releaseOwnedContextMenuClaim = async (
6767+
rejection = { reason: 'panel-hidden', retryAfterMs: 250 },
6768+
) => {
67636769
if (!contextMenuClaimOwned) return;
67646770
contextMenuClaimOwned = false;
67656771
try {
@@ -6769,7 +6775,7 @@ async function sendMessage(extraChatParams = {}) {
67696775
claimantId: contextMenuClaim.claimantId,
67706776
});
67716777
} catch { /* the durable lease still expires if release fails */ }
6772-
onContextMenuClaimRejected?.({ reason: 'panel-hidden', retryAfterMs: 250 });
6778+
onContextMenuClaimRejected?.(rejection);
67736779
};
67746780
if (isConversationClearInProgress(tabId)) return false;
67756781
const permissionSkipContext = permissionSkipCommandContextForDraft(tabId, text);
@@ -6785,6 +6791,10 @@ async function sendMessage(extraChatParams = {}) {
67856791
return false;
67866792
}
67876793
if (isProcessing) {
6794+
if (contextMenuClaimOwned) {
6795+
await releaseOwnedContextMenuClaim({ reason: 'run-active', retryAfterMs: 1_000 });
6796+
return false;
6797+
}
67886798
if (isOutOfBandSlashDraft(text)) {
67896799
resetComposerHistoryNavigation(tabId);
67906800
saveInputDraftForTab(tabId, '');

‎test/run.js‎

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18190,6 +18190,8 @@ test('chrome sidepanel serializes tab-chat storage writes with clears and reads'
1819018190
assert.match(loadBody, /const numericTabId = Number\(tabId\);[\s\S]*?if \(!Number\.isFinite\(numericTabId\)\) return null;/, 'chrome: tab-chat restore should normalize tab ids before checking the cache');
1819118191
assert.match(loadBody, /if \(!waitForHandoff && !tabChatOperations\.has\(numericTabId\) && tabChats\.has\(numericTabId\)\)/, 'chrome: coordinated handoff restores should bypass stale document-local HTML');
1819218192
assert.match(loadBody, /return await enqueueTabChatOperation\(numericTabId, async \(queuedTabId\) => \{[\s\S]*?sendToBackground\('load_tab_chat', \{[\s\S]*?waitForHandoff,[\s\S]*?\}\);/, 'chrome: tab-chat restore should read through the shared background queue');
18193+
assert.match(loadBody, /catch \(e\) \{\s*if \(waitForHandoff\) return TAB_CHAT_LOAD_FAILED;\s*\}[\s\S]*?return null;/, 'chrome: coordinated load failures should remain distinct from successful empty restores');
18194+
assert.match(panel, /const html = await loadTabChat\(tabId, \{ waitForHandoff: true \}\);\s*if \(html === TAB_CHAT_LOAD_FAILED\) return false;[\s\S]*?messagesEl\.innerHTML = '';/, 'chrome: a failed visibility handoff must preserve the current transcript DOM');
1819318195
assert.match(panel, /const payload = \{[\s\S]*?handoffOwnerId: tabChatHandoffOwnerId,[\s\S]*?handoffGeneration[\s\S]*?return enqueueTabChatOperation\(tabId, async \(numericTabId\) => \{[\s\S]*?sendToBackground\('persist_tab_chat', payload\);/, 'chrome: visible tab-chat persistence should carry its owner generation through the shared background queue');
1819418196
assert.match(panel, /document\.visibilityState === 'hidden' && allowHidden[\s\S]*?sendToBackground\('persist_tab_chat', payload\);/, 'chrome: hidden handoff must bypass the document-local queue and enter the shared queue immediately');
1819518197
const clearStart = panel.indexOf('function clearCachedTabChat(tabId) {');
@@ -18210,6 +18212,8 @@ test('firefox sidepanel serializes tab-chat storage writes with clears and reads
1821018212
assert.match(loadBody, /const numericTabId = Number\(tabId\);[\s\S]*?if \(!Number\.isFinite\(numericTabId\)\) return null;/, 'firefox: tab-chat restore should normalize tab ids before checking the cache');
1821118213
assert.match(loadBody, /if \(!waitForHandoff && !tabChatOperations\.has\(numericTabId\) && tabChats\.has\(numericTabId\)\)/, 'firefox: coordinated handoff restores should bypass stale document-local HTML');
1821218214
assert.match(loadBody, /return await enqueueTabChatOperation\(numericTabId, async \(queuedTabId\) => \{[\s\S]*?sendToBackground\('load_tab_chat', \{[\s\S]*?waitForHandoff,[\s\S]*?\}\);/, 'firefox: tab-chat restore should read through the shared background queue');
18215+
assert.match(loadBody, /catch \(e\) \{\s*if \(waitForHandoff\) return TAB_CHAT_LOAD_FAILED;\s*\}[\s\S]*?return null;/, 'firefox: coordinated load failures should remain distinct from successful empty restores');
18216+
assert.match(panel, /const html = await loadTabChat\(tabId, \{ waitForHandoff: true \}\);\s*if \(html === TAB_CHAT_LOAD_FAILED\) return false;[\s\S]*?messagesEl\.innerHTML = '';/, 'firefox: a failed visibility handoff must preserve the current transcript DOM');
1821318217
assert.match(panel, /const payload = \{[\s\S]*?handoffOwnerId: tabChatHandoffOwnerId,[\s\S]*?handoffGeneration[\s\S]*?return enqueueTabChatOperation\(tabId, async \(numericTabId\) => \{[\s\S]*?sendToBackground\('persist_tab_chat', payload\);/, 'firefox: visible tab-chat persistence should carry its owner generation through the shared background queue');
1821418218
assert.match(panel, /document\.visibilityState === 'hidden' && allowHidden[\s\S]*?sendToBackground\('persist_tab_chat', payload\);/, 'firefox: hidden handoff must bypass the document-local queue and enter the shared queue immediately');
1821518219
const clearStart = panel.indexOf('function clearCachedTabChat(tabId) {');
@@ -18280,10 +18284,10 @@ test('sidepanel does not miss startup tab switches before consuming tab-scoped s
1828018284
if (label === 'chrome') {
1828118285
const restoreCaptureIdx = body.indexOf('const restoreTabId = currentTabId;');
1828218286
const restoreLoadIdx = body.indexOf('const html = await loadTabChat(restoreTabId, { waitForHandoff: true });');
18283-
const restoreGuardIdx = body.indexOf('if (currentTabId === restoreTabId && html)');
18287+
const restoreGuardIdx = body.indexOf('if (html !== TAB_CHAT_LOAD_FAILED && currentTabId === restoreTabId && html)');
1828418288
assert.notEqual(restoreCaptureIdx, -1, 'chrome: initial tab-chat restore should capture the target tab');
1828518289
assert.notEqual(restoreLoadIdx, -1, 'chrome: initial tab-chat restore should load the captured tab');
18286-
assert.notEqual(restoreGuardIdx, -1, 'chrome: initial tab-chat restore should drop stale async results');
18290+
assert.notEqual(restoreGuardIdx, -1, 'chrome: initial tab-chat restore should drop failed and stale async results');
1828718291
assert.equal(listenerIdx < restoreCaptureIdx && restoreCaptureIdx < restoreLoadIdx && restoreLoadIdx < restoreGuardIdx, true, 'chrome: initial restore must be guarded after listener-driven tab changes');
1828818292
}
1828918293
}
@@ -21707,6 +21711,40 @@ test('context-menu prompts release an initial claim acquired after the panel bec
2170721711
}
2170821712
});
2170921713

21714+
test('context-menu prompts release and requeue when another run starts during claim acquisition', async () => {
21715+
for (const [label, createHandler] of [
21716+
['chrome', createContextMenuPromptHandlerCh],
21717+
['firefox', createContextMenuPromptHandlerFx],
21718+
]) {
21719+
const prompt = { id: `${label}-busy-during-claim`, tabId: 10, text: 'Explain this selection' };
21720+
const claimGate = deferred();
21721+
const h = createContextMenuPromptHarness(createHandler, prompt, async () => true, {
21722+
claimPrompt: async (_params, attempt) => (
21723+
attempt === 1 ? claimGate.promise : { ok: true, claimed: true }
21724+
),
21725+
});
21726+
21727+
h.handler.acceptContextMenuPrompt(prompt);
21728+
await waitMicrotasks(3);
21729+
h.setProcessing(true);
21730+
claimGate.resolve({ ok: true, claimed: true });
21731+
await waitMicrotasks(8);
21732+
21733+
assert.equal(h.sends.length, 0, `${label}: a prompt must not enter the ordinary composer queue after a run starts`);
21734+
assert.equal(h.releases.length, 1, `${label}: the prompt lease should be released while the active run owns the tab`);
21735+
assert.equal(h.input.value, '', `${label}: the claimed prompt should not replace the user's busy composer draft`);
21736+
21737+
h.setProcessing(false);
21738+
h.handler.drainQueuedContextMenuPrompts();
21739+
await waitMicrotasks(8);
21740+
21741+
assert.equal(h.claims.length, 2, `${label}: the context-menu queue should reclaim after the active run settles`);
21742+
assert.equal(h.sends.length, 1, `${label}: the full prompt payload should submit exactly once after reclaim`);
21743+
assert.equal(h.sends[0].extra.contextMenuClaim.promptId, prompt.id, `${label}: the deferred send must preserve ownership metadata`);
21744+
assert.equal(h.sends[0].extra.contextMenuClear.promptId, prompt.id, `${label}: the deferred send must still clear durable prompt storage`);
21745+
}
21746+
});
21747+
2171021748
test('context-menu prompt lease recovery retries after an abandoned owner expires', async () => {
2171121749
for (const [label, createHandler] of [
2171221750
['chrome', createContextMenuPromptHandlerCh],
@@ -21809,7 +21847,7 @@ test('context-menu ownership and stale-panel persistence guards are wired in bot
2180921847
);
2181021848
assert.match(
2181121849
handler,
21812-
/claimResult\?\.claimed && !getIsDocumentVisible\(\)[\s\S]*?release_context_menu_prompt_claim[\s\S]*?reason: 'panel-hidden'/,
21850+
/const releasePromptClaim = async \(\) => \{[\s\S]*?release_context_menu_prompt_claim[\s\S]*?claimResult\?\.claimed && !getIsDocumentVisible\(\)[\s\S]*?await releasePromptClaim\(\);[\s\S]*?reason: 'panel-hidden'/,
2181321851
`${label}: an initial claim that resolves after hiding should be released for the visible panel`,
2181421852
);
2181521853
assert.match(
@@ -21884,9 +21922,14 @@ test('context-menu ownership and stale-panel persistence guards are wired in bot
2188421922
);
2188521923
assert.match(
2188621924
panel,
21887-
/let contextMenuClaimOwned = Boolean\(contextMenuClaim\?\.promptId && contextMenuClaim\?\.claimantId\);[\s\S]*?const releaseOwnedContextMenuClaim = async \(\) => \{[\s\S]*?contextMenuClaimOwned = false;[\s\S]*?release_context_menu_prompt_claim[\s\S]*?onContextMenuClaimRejected\?\.\(\{ reason: 'panel-hidden', retryAfterMs: 250 \}\);[\s\S]*?contextMenuClaimOwned = renewedClaim\?\.claimed === true;/,
21925+
/let contextMenuClaimOwned = Boolean\(contextMenuClaim\?\.promptId && contextMenuClaim\?\.claimantId\);[\s\S]*?const releaseOwnedContextMenuClaim = async \([\s\S]*?reason: 'panel-hidden', retryAfterMs: 250[\s\S]*?contextMenuClaimOwned = false;[\s\S]*?release_context_menu_prompt_claim[\s\S]*?onContextMenuClaimRejected\?\.\(rejection\);[\s\S]*?contextMenuClaimOwned = renewedClaim\?\.claimed === true;/,
2188821926
`${label}: initial and renewed ownership should share one idempotent release-and-retry path`,
2188921927
);
21928+
assert.match(
21929+
panel,
21930+
/if \(isProcessing\) \{\s*if \(contextMenuClaimOwned\) \{\s*await releaseOwnedContextMenuClaim\(\{ reason: 'run-active', retryAfterMs: 1_000 \}\);\s*return false;/,
21931+
`${label}: a context-menu claim must never fall through to the ordinary busy composer queue`,
21932+
);
2189021933
const sendMessageStart = panel.indexOf('async function sendMessage(extraChatParams = {}) {');
2189121934
const sendMessageEnd = panel.indexOf(
2189221935
label === 'chrome' ? '\nfunction formatRecordTimer(' : '\nfunction ensureCurrentRunAssistant(',

0 commit comments

Comments
 (0)