Skip to content

Commit 37e243a

Browse files
esokulluesokullu
committed
Fix sidepanel refresh and claim races
1 parent d01615e commit 37e243a

3 files changed

Lines changed: 128 additions & 57 deletions

File tree

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

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2102,7 +2102,19 @@ async function renderClearedConversationForTab(tabId) {
21022102
let persistTimer = null;
21032103
let persistTimerTabId = null;
21042104
let visibleStateRefreshPending = false;
2105+
let visibleStateRefreshPromise = Promise.resolve();
2106+
let visibleStateRefreshInProgress = false;
2107+
let visibleStateRefreshGeneration = 0;
21052108
let lastVisibleTabChatSnapshot = null;
2109+
2110+
async function waitForVisibleSidePanelStateRefresh() {
2111+
let pendingRefresh;
2112+
do {
2113+
pendingRefresh = visibleStateRefreshPromise;
2114+
await pendingRefresh.catch(() => {});
2115+
} while (pendingRefresh !== visibleStateRefreshPromise);
2116+
}
2117+
21062118
function schedulePersist() {
21072119
if (document.visibilityState === 'hidden') return;
21082120
if (persistTimer) clearTimeout(persistTimer);
@@ -3760,19 +3772,30 @@ async function refreshVisibleSidePanelState() {
37603772
addMessage('system', t('sp.help_message'));
37613773
}
37623774
await restoreActiveRunState(tabId);
3763-
if (document.visibilityState === 'hidden' || !sameTabId(currentTabId, tabId)) return;
3764-
await consumePendingContextMenuPrompt();
3765-
drainQueuedContextMenuPrompts();
3775+
return document.visibilityState !== 'hidden' && sameTabId(currentTabId, tabId);
37663776
}
37673777

37683778
function requestVisibleSidePanelStateRefresh() {
37693779
if (document.visibilityState === 'hidden') return;
37703780
visibleStateRefreshPending = true;
3771-
Promise.resolve().then(() => {
3772-
if (document.visibilityState === 'hidden' || tabSwitchTransitionId != null) return;
3781+
visibleStateRefreshInProgress = true;
3782+
const refreshGeneration = ++visibleStateRefreshGeneration;
3783+
syncSendButtonState();
3784+
visibleStateRefreshPromise = visibleStateRefreshPromise.catch(() => {}).then(async () => {
3785+
if (document.visibilityState === 'hidden' || tabSwitchTransitionId != null) return false;
37733786
visibleStateRefreshPending = false;
3774-
refreshVisibleSidePanelState().catch(() => {});
3787+
return refreshVisibleSidePanelState();
37753788
});
3789+
const refreshPromise = visibleStateRefreshPromise;
3790+
refreshPromise.finally(() => {
3791+
if (refreshGeneration !== visibleStateRefreshGeneration) return;
3792+
visibleStateRefreshInProgress = false;
3793+
syncSendButtonState();
3794+
}).catch(() => {});
3795+
refreshPromise.then((refreshed) => {
3796+
if (!refreshed || refreshPromise !== visibleStateRefreshPromise) return;
3797+
consumePendingContextMenuPrompt().then(() => drainQueuedContextMenuPrompts()).catch(() => {});
3798+
}).catch(() => {});
37763799
}
37773800

37783801
async function restoreActiveRunState(tabId = currentTabId) {
@@ -6253,6 +6276,10 @@ function isOutOfBandSlashDraft(value) {
62536276
function syncSendButtonState() {
62546277
if (!sendBtn) return;
62556278
const draft = normalizeScreenshotCommandText(inputEl?.value || '').trim();
6279+
if (visibleStateRefreshPending || visibleStateRefreshInProgress) {
6280+
sendBtn.disabled = true;
6281+
return;
6282+
}
62566283
if (isAwaitingPlanReviewForTab()) {
62576284
sendBtn.disabled = true;
62586285
return;
@@ -6989,11 +7016,25 @@ async function sendMessage(extraChatParams = {}) {
69897016
: null;
69907017
delete chatExtraParams.sourceGrounding;
69917018
if (sourceGrounding) chatExtraParams.sourceGrounding = sourceGrounding;
7019+
await waitForVisibleSidePanelStateRefresh();
69927020
stopListening();
69937021
let text = inputEl.value.trim();
69947022
if (!text) return;
69957023
const submittedText = text;
69967024
const tabId = currentTabId;
7025+
let contextMenuClaimOwned = Boolean(contextMenuClaim?.promptId && contextMenuClaim?.claimantId);
7026+
const releaseOwnedContextMenuClaim = async () => {
7027+
if (!contextMenuClaimOwned) return;
7028+
contextMenuClaimOwned = false;
7029+
try {
7030+
await sendToBackground('release_context_menu_prompt_claim', {
7031+
tabId,
7032+
promptId: contextMenuClaim.promptId,
7033+
claimantId: contextMenuClaim.claimantId,
7034+
});
7035+
} catch { /* the durable lease still expires if release fails */ }
7036+
onContextMenuClaimRejected?.({ reason: 'panel-hidden', retryAfterMs: 250 });
7037+
};
69977038
if (isConversationClearInProgress(tabId)) return false;
69987039
const permissionSkipContext = permissionSkipCommandContextForDraft(tabId, text);
69997040
const requestId = createRunRequestId(tabId);
@@ -7097,27 +7138,14 @@ async function sendMessage(extraChatParams = {}) {
70977138
&& sameTabId(currentTabId, tabId)
70987139
&& sameTabId(renderedTabId, tabId);
70997140
if (!renderToCurrentTab) {
7141+
await releaseOwnedContextMenuClaim();
71007142
if (text) saveInputDraftForTab(tabId, text);
71017143
setTabProcessing(tabId, false);
71027144
setTabAbortRequested(tabId, false);
71037145
syncSendButtonState();
71047146
return false;
71057147
}
71067148

7107-
let renewedContextMenuClaim = false;
7108-
const releaseRenewedContextMenuClaim = async () => {
7109-
if (!renewedContextMenuClaim) return;
7110-
renewedContextMenuClaim = false;
7111-
try {
7112-
await sendToBackground('release_context_menu_prompt_claim', {
7113-
tabId,
7114-
promptId: contextMenuClaim.promptId,
7115-
claimantId: contextMenuClaim.claimantId,
7116-
});
7117-
} catch { /* the durable lease still expires if release fails */ }
7118-
onContextMenuClaimRejected?.({ reason: 'panel-hidden', retryAfterMs: 250 });
7119-
};
7120-
71217149
if (contextMenuClaim?.promptId && contextMenuClaim?.claimantId) {
71227150
let renewedClaim = null;
71237151
try {
@@ -7129,12 +7157,12 @@ async function sendMessage(extraChatParams = {}) {
71297157
} catch {
71307158
renewedClaim = { claimed: false, reason: 'connection', retryAfterMs: 1_000 };
71317159
}
7132-
renewedContextMenuClaim = renewedClaim?.claimed === true;
7160+
contextMenuClaimOwned = renewedClaim?.claimed === true;
71337161
const claimStillVisible = document.visibilityState !== 'hidden'
71347162
&& sameTabId(currentTabId, tabId)
71357163
&& sameTabId(renderedTabId, tabId);
7136-
if (renewedContextMenuClaim && !claimStillVisible) {
7137-
await releaseRenewedContextMenuClaim();
7164+
if (contextMenuClaimOwned && !claimStillVisible) {
7165+
await releaseOwnedContextMenuClaim();
71387166
setTabProcessing(tabId, false);
71397167
setTabAbortRequested(tabId, false);
71407168
if (sameTabId(currentTabId, tabId)) syncSendButtonState();
@@ -7156,7 +7184,7 @@ async function sendMessage(extraChatParams = {}) {
71567184
&& sameTabId(currentTabId, tabId)
71577185
&& sameTabId(renderedTabId, tabId);
71587186
if (!renderToCurrentTab) {
7159-
await releaseRenewedContextMenuClaim();
7187+
await releaseOwnedContextMenuClaim();
71607188
if (text) saveInputDraftForTab(tabId, text);
71617189
setTabProcessing(tabId, false);
71627190
setTabAbortRequested(tabId, false);

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

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,7 +1522,19 @@ function clearCachedTabChat(tabId) {
15221522
let persistTimer = null;
15231523
let persistTimerTabId = null;
15241524
let visibleStateRefreshPending = false;
1525+
let visibleStateRefreshPromise = Promise.resolve();
1526+
let visibleStateRefreshInProgress = false;
1527+
let visibleStateRefreshGeneration = 0;
15251528
let lastVisibleTabChatSnapshot = null;
1529+
1530+
async function waitForVisibleSidePanelStateRefresh() {
1531+
let pendingRefresh;
1532+
do {
1533+
pendingRefresh = visibleStateRefreshPromise;
1534+
await pendingRefresh.catch(() => {});
1535+
} while (pendingRefresh !== visibleStateRefreshPromise);
1536+
}
1537+
15261538
function schedulePersist() {
15271539
if (document.visibilityState === 'hidden') return;
15281540
if (persistTimer) clearTimeout(persistTimer);
@@ -3608,19 +3620,30 @@ async function refreshVisibleSidePanelState() {
36083620
addMessage('system', t('sp.help_message'));
36093621
}
36103622
await restoreActiveRunState(tabId);
3611-
if (document.visibilityState === 'hidden' || !sameTabId(currentTabId, tabId)) return;
3612-
await consumePendingContextMenuPrompt();
3613-
drainQueuedContextMenuPrompts();
3623+
return document.visibilityState !== 'hidden' && sameTabId(currentTabId, tabId);
36143624
}
36153625

36163626
function requestVisibleSidePanelStateRefresh() {
36173627
if (document.visibilityState === 'hidden') return;
36183628
visibleStateRefreshPending = true;
3619-
Promise.resolve().then(() => {
3620-
if (document.visibilityState === 'hidden' || tabSwitchTransitionId != null) return;
3629+
visibleStateRefreshInProgress = true;
3630+
const refreshGeneration = ++visibleStateRefreshGeneration;
3631+
syncSendButtonState();
3632+
visibleStateRefreshPromise = visibleStateRefreshPromise.catch(() => {}).then(async () => {
3633+
if (document.visibilityState === 'hidden' || tabSwitchTransitionId != null) return false;
36213634
visibleStateRefreshPending = false;
3622-
refreshVisibleSidePanelState().catch(() => {});
3635+
return refreshVisibleSidePanelState();
36233636
});
3637+
const refreshPromise = visibleStateRefreshPromise;
3638+
refreshPromise.finally(() => {
3639+
if (refreshGeneration !== visibleStateRefreshGeneration) return;
3640+
visibleStateRefreshInProgress = false;
3641+
syncSendButtonState();
3642+
}).catch(() => {});
3643+
refreshPromise.then((refreshed) => {
3644+
if (!refreshed || refreshPromise !== visibleStateRefreshPromise) return;
3645+
consumePendingContextMenuPrompt().then(() => drainQueuedContextMenuPrompts()).catch(() => {});
3646+
}).catch(() => {});
36243647
}
36253648

36263649
async function restoreActiveRunState(tabId = currentTabId) {
@@ -6087,6 +6110,10 @@ function isOutOfBandSlashDraft(value) {
60876110
function syncSendButtonState() {
60886111
if (!sendBtn) return;
60896112
const draft = normalizeScreenshotCommandText(inputEl?.value || '').trim();
6113+
if (visibleStateRefreshPending || visibleStateRefreshInProgress) {
6114+
sendBtn.disabled = true;
6115+
return;
6116+
}
60906117
if (isAwaitingPlanReviewForTab()) {
60916118
sendBtn.disabled = true;
60926119
return;
@@ -6725,11 +6752,25 @@ async function sendMessage(extraChatParams = {}) {
67256752
: null;
67266753
delete chatExtraParams.sourceGrounding;
67276754
if (sourceGrounding) chatExtraParams.sourceGrounding = sourceGrounding;
6755+
await waitForVisibleSidePanelStateRefresh();
67286756
stopListening();
67296757
let text = inputEl.value.trim();
67306758
if (!text) return;
67316759
const submittedText = text;
67326760
const tabId = currentTabId;
6761+
let contextMenuClaimOwned = Boolean(contextMenuClaim?.promptId && contextMenuClaim?.claimantId);
6762+
const releaseOwnedContextMenuClaim = async () => {
6763+
if (!contextMenuClaimOwned) return;
6764+
contextMenuClaimOwned = false;
6765+
try {
6766+
await sendToBackground('release_context_menu_prompt_claim', {
6767+
tabId,
6768+
promptId: contextMenuClaim.promptId,
6769+
claimantId: contextMenuClaim.claimantId,
6770+
});
6771+
} catch { /* the durable lease still expires if release fails */ }
6772+
onContextMenuClaimRejected?.({ reason: 'panel-hidden', retryAfterMs: 250 });
6773+
};
67336774
if (isConversationClearInProgress(tabId)) return false;
67346775
const permissionSkipContext = permissionSkipCommandContextForDraft(tabId, text);
67356776
const requestId = createRunRequestId(tabId);
@@ -6828,27 +6869,14 @@ async function sendMessage(extraChatParams = {}) {
68286869
&& sameTabId(currentTabId, tabId)
68296870
&& sameTabId(renderedTabId, tabId);
68306871
if (!renderToCurrentTab) {
6872+
await releaseOwnedContextMenuClaim();
68316873
if (text) saveInputDraftForTab(tabId, text);
68326874
setTabProcessing(tabId, false);
68336875
setTabAbortRequested(tabId, false);
68346876
syncSendButtonState();
68356877
return false;
68366878
}
68376879

6838-
let renewedContextMenuClaim = false;
6839-
const releaseRenewedContextMenuClaim = async () => {
6840-
if (!renewedContextMenuClaim) return;
6841-
renewedContextMenuClaim = false;
6842-
try {
6843-
await sendToBackground('release_context_menu_prompt_claim', {
6844-
tabId,
6845-
promptId: contextMenuClaim.promptId,
6846-
claimantId: contextMenuClaim.claimantId,
6847-
});
6848-
} catch { /* the durable lease still expires if release fails */ }
6849-
onContextMenuClaimRejected?.({ reason: 'panel-hidden', retryAfterMs: 250 });
6850-
};
6851-
68526880
if (contextMenuClaim?.promptId && contextMenuClaim?.claimantId) {
68536881
let renewedClaim = null;
68546882
try {
@@ -6860,12 +6888,12 @@ async function sendMessage(extraChatParams = {}) {
68606888
} catch {
68616889
renewedClaim = { claimed: false, reason: 'connection', retryAfterMs: 1_000 };
68626890
}
6863-
renewedContextMenuClaim = renewedClaim?.claimed === true;
6891+
contextMenuClaimOwned = renewedClaim?.claimed === true;
68646892
const claimStillVisible = document.visibilityState !== 'hidden'
68656893
&& sameTabId(currentTabId, tabId)
68666894
&& sameTabId(renderedTabId, tabId);
6867-
if (renewedContextMenuClaim && !claimStillVisible) {
6868-
await releaseRenewedContextMenuClaim();
6895+
if (contextMenuClaimOwned && !claimStillVisible) {
6896+
await releaseOwnedContextMenuClaim();
68696897
setTabProcessing(tabId, false);
68706898
setTabAbortRequested(tabId, false);
68716899
if (sameTabId(currentTabId, tabId)) syncSendButtonState();
@@ -6887,7 +6915,7 @@ async function sendMessage(extraChatParams = {}) {
68876915
&& sameTabId(currentTabId, tabId)
68886916
&& sameTabId(renderedTabId, tabId);
68896917
if (!renderToCurrentTab) {
6890-
await releaseRenewedContextMenuClaim();
6918+
await releaseOwnedContextMenuClaim();
68916919
if (text) saveInputDraftForTab(tabId, text);
68926920
setTabProcessing(tabId, false);
68936921
setTabAbortRequested(tabId, false);

‎test/run.js‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21824,8 +21824,23 @@ test('context-menu ownership and stale-panel persistence guards are wired in bot
2182421824
);
2182521825
assert.match(
2182621826
panel,
21827-
/async function refreshVisibleSidePanelState\(\) \{[\s\S]*?tabChats\.delete\(tabId\);[\s\S]*?await loadTabChat\(tabId, \{ waitForHandoff: true \}\);[\s\S]*?await consumePendingContextMenuPrompt\(\);/,
21828-
`${label}: a returning panel should wait for the shared handoff before consuming prompts`,
21827+
/let visibleStateRefreshPromise = Promise\.resolve\(\);[\s\S]*?async function waitForVisibleSidePanelStateRefresh\(\) \{[\s\S]*?pendingRefresh = visibleStateRefreshPromise;[\s\S]*?await pendingRefresh\.catch\(\(\) => \{\}\);[\s\S]*?pendingRefresh !== visibleStateRefreshPromise/,
21828+
`${label}: sends should wait until the complete serialized visibility refresh queue settles`,
21829+
);
21830+
assert.match(
21831+
panel,
21832+
/async function refreshVisibleSidePanelState\(\) \{[\s\S]*?tabChats\.delete\(tabId\);[\s\S]*?await loadTabChat\(tabId, \{ waitForHandoff: true \}\);[\s\S]*?await restoreActiveRunState\(tabId\);[\s\S]*?function requestVisibleSidePanelStateRefresh\(\) \{[\s\S]*?visibleStateRefreshPromise = visibleStateRefreshPromise\.catch\(\(\) => \{\}\)\.then\(async \(\) => \{[\s\S]*?return refreshVisibleSidePanelState\(\);[\s\S]*?refreshPromise\.then\(\(refreshed\) => \{[\s\S]*?refreshPromise !== visibleStateRefreshPromise[\s\S]*?consumePendingContextMenuPrompt\(\)/,
21833+
`${label}: a returning panel should serialize the shared handoff before consuming prompts`,
21834+
);
21835+
assert.match(
21836+
panel,
21837+
/function syncSendButtonState\(\) \{[\s\S]*?if \(visibleStateRefreshPending \|\| visibleStateRefreshInProgress\) \{\s*sendBtn\.disabled = true;\s*return;/,
21838+
`${label}: the composer should stay disabled throughout visibility reconciliation`,
21839+
);
21840+
assert.match(
21841+
panel,
21842+
/async function sendMessage\(extraChatParams = \{\}\) \{[\s\S]*?await waitForVisibleSidePanelStateRefresh\(\);\s*stopListening\(\);\s*let text = inputEl\.value\.trim\(\);/,
21843+
`${label}: user sends should not capture or append a turn until visibility reconciliation finishes`,
2182921844
);
2183021845
assert.match(
2183121846
panel,
@@ -21869,8 +21884,8 @@ test('context-menu ownership and stale-panel persistence guards are wired in bot
2186921884
);
2187021885
assert.match(
2187121886
panel,
21872-
/const releaseRenewedContextMenuClaim = async \(\) => \{[\s\S]*?renewedContextMenuClaim = false;[\s\S]*?release_context_menu_prompt_claim[\s\S]*?onContextMenuClaimRejected\?\.\(\{ reason: 'panel-hidden', retryAfterMs: 250 \}\);[\s\S]*?renewedContextMenuClaim = renewedClaim\?\.claimed === true;/,
21873-
`${label}: renewed ownership should have one idempotent release-and-retry path`,
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;/,
21888+
`${label}: initial and renewed ownership should share one idempotent release-and-retry path`,
2187421889
);
2187521890
const sendMessageStart = panel.indexOf('async function sendMessage(extraChatParams = {}) {');
2187621891
const sendMessageEnd = panel.indexOf(
@@ -21880,14 +21895,14 @@ test('context-menu ownership and stale-panel persistence guards are wired in bot
2188021895
assert.notEqual(sendMessageEnd, -1, `${label}: sendMessage boundary should remain inspectable`);
2188121896
const sendMessageBody = panel.slice(sendMessageStart, sendMessageEnd);
2188221897
assert.equal(
21883-
(sendMessageBody.match(/await releaseRenewedContextMenuClaim\(\);/g) || []).length,
21884-
2,
21885-
`${label}: both visibility checks after renewal should release the claim and schedule retry`,
21898+
(sendMessageBody.match(/await releaseOwnedContextMenuClaim\(\);/g) || []).length,
21899+
3,
21900+
`${label}: visibility exits after preflight and renewal should release the claim and schedule retry`,
2188621901
);
2188721902
assert.match(
2188821903
sendMessageBody,
21889-
/renewedContextMenuClaim = renewedClaim\?\.claimed === true;[\s\S]*?const claimStillVisible[\s\S]*?if \(renewedContextMenuClaim && !claimStillVisible\) \{[\s\S]*?await releaseRenewedContextMenuClaim\(\);[\s\S]*?renderToCurrentTab = document\.visibilityState !== 'hidden'[\s\S]*?if \(!renderToCurrentTab\) \{\s*await releaseRenewedContextMenuClaim\(\);/,
21890-
`${label}: the final post-renewal visibility failure should relinquish ownership before returning`,
21904+
/await prepareChatHistoryForTurn\(tabId, modeForSend\);[\s\S]*?if \(!renderToCurrentTab\) \{\s*await releaseOwnedContextMenuClaim\(\);[\s\S]*?contextMenuClaimOwned = renewedClaim\?\.claimed === true;[\s\S]*?const claimStillVisible[\s\S]*?if \(contextMenuClaimOwned && !claimStillVisible\) \{[\s\S]*?await releaseOwnedContextMenuClaim\(\);[\s\S]*?renderToCurrentTab = document\.visibilityState !== 'hidden'[\s\S]*?if \(!renderToCurrentTab\) \{\s*await releaseOwnedContextMenuClaim\(\);/,
21905+
`${label}: every post-preflight visibility failure should relinquish initial or renewed ownership before returning`,
2189121906
);
2189221907
}
2189321908
});

0 commit comments

Comments
 (0)