Skip to content

Commit 4ffb972

Browse files
esokulluesokullu
committed
Retry initial handoff ownership
1 parent 9428470 commit 4ffb972

7 files changed

Lines changed: 119 additions & 37 deletions

File tree

‎src/chrome/src/background.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2648,7 +2648,6 @@ async function handleMessage(msg, sender) {
26482648
tabId,
26492649
msg.promptId,
26502650
msg.claimantId,
2651-
Date.now(),
26522651
() => agent.activeRunState(tabId)?.running || detachedRunStarts.has(tabId),
26532652
);
26542653
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ export function createContextMenuStorage(getStore) {
196196
return { ok: true, prompt: prompt?.text ? prompt : null };
197197
}
198198

199-
async function claim(tabId, promptId, claimantId, now = Date.now(), isRunActive = () => false) {
199+
async function claim(tabId, promptId, claimantId, isRunActive = () => false, now) {
200200
const normalizedPromptId = String(promptId || '');
201201
const normalizedClaimantId = String(claimantId || '');
202202
if (!normalizedPromptId || !normalizedClaimantId) {
@@ -232,7 +232,10 @@ export function createContextMenuStorage(getStore) {
232232
retryAfterMs: 1_000,
233233
};
234234
}
235-
const nowMs = Number.isFinite(Number(now)) ? Number(now) : Date.now();
235+
const suppliedNow = Number(now);
236+
const nowMs = now == null || !Number.isFinite(suppliedNow)
237+
? Date.now()
238+
: suppliedNow;
236239
const samePrompt = String(activeClaim?.promptId || '') === normalizedPromptId;
237240
const activeLease = samePrompt && Number(activeClaim?.expiresAt || 0) > nowMs;
238241
if (activeLease && String(activeClaim?.claimantId || '') !== normalizedClaimantId) {

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

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2136,8 +2136,12 @@ async function waitForVisibleSidePanelStateRefresh() {
21362136
do {
21372137
pendingRefresh = visibleStateRefreshPromise;
21382138
await pendingRefresh.catch(() => {});
2139-
if (tabSwitchTransitionId != null) await waitForTabChatHandoffRetry();
2140-
} while (pendingRefresh !== visibleStateRefreshPromise || tabSwitchTransitionId != null);
2139+
if (tabSwitchTransitionId != null || visibleStateRefreshInProgress) {
2140+
await waitForTabChatHandoffRetry();
2141+
}
2142+
} while (pendingRefresh !== visibleStateRefreshPromise
2143+
|| tabSwitchTransitionId != null
2144+
|| visibleStateRefreshInProgress);
21412145
}
21422146

21432147
function schedulePersist() {
@@ -3622,14 +3626,27 @@ async function init() {
36223626
// Restore prior conversation for this tab (if any) — survives close/reopen.
36233627
const restoreTabId = currentTabId;
36243628
if (restoreTabId != null) {
3625-
const html = await loadTabChat(restoreTabId, { waitForHandoff: true });
3626-
if (html !== TAB_CHAT_LOAD_FAILED && currentTabId === restoreTabId && html) {
3627-
await hydrateRestoredChatHistory(restoreTabId, html);
3628-
if (currentTabId === restoreTabId) {
3629-
messagesEl.innerHTML = html;
3630-
messagesEl.querySelectorAll('[data-bound]').forEach(el => delete el.dataset.bound);
3631-
rebindRestoredMessageControls();
3629+
visibleStateRefreshInProgress = true;
3630+
syncSendButtonState();
3631+
try {
3632+
let html = TAB_CHAT_LOAD_FAILED;
3633+
while (html === TAB_CHAT_LOAD_FAILED && currentTabId === restoreTabId) {
3634+
html = await loadTabChat(restoreTabId, { waitForHandoff: true });
3635+
if (html === TAB_CHAT_LOAD_FAILED && currentTabId === restoreTabId) {
3636+
await waitForTabChatHandoffRetry();
3637+
}
36323638
}
3639+
if (currentTabId === restoreTabId && html && html !== TAB_CHAT_LOAD_FAILED) {
3640+
await hydrateRestoredChatHistory(restoreTabId, html);
3641+
if (currentTabId === restoreTabId) {
3642+
messagesEl.innerHTML = html;
3643+
messagesEl.querySelectorAll('[data-bound]').forEach(el => delete el.dataset.bound);
3644+
rebindRestoredMessageControls();
3645+
}
3646+
}
3647+
} finally {
3648+
visibleStateRefreshInProgress = false;
3649+
syncSendButtonState();
36333650
}
36343651
}
36353652

‎src/firefox/src/background.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2335,7 +2335,6 @@ async function handleMessage(msg, sender) {
23352335
tabId,
23362336
msg.promptId,
23372337
msg.claimantId,
2338-
Date.now(),
23392338
() => agent.activeRunState(tabId)?.running || detachedRunStarts.has(tabId),
23402339
);
23412340
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ export function createContextMenuStorage(getStore) {
196196
return { ok: true, prompt: prompt?.text ? prompt : null };
197197
}
198198

199-
async function claim(tabId, promptId, claimantId, now = Date.now(), isRunActive = () => false) {
199+
async function claim(tabId, promptId, claimantId, isRunActive = () => false, now) {
200200
const normalizedPromptId = String(promptId || '');
201201
const normalizedClaimantId = String(claimantId || '');
202202
if (!normalizedPromptId || !normalizedClaimantId) {
@@ -232,7 +232,10 @@ export function createContextMenuStorage(getStore) {
232232
retryAfterMs: 1_000,
233233
};
234234
}
235-
const nowMs = Number.isFinite(Number(now)) ? Number(now) : Date.now();
235+
const suppliedNow = Number(now);
236+
const nowMs = now == null || !Number.isFinite(suppliedNow)
237+
? Date.now()
238+
: suppliedNow;
236239
const samePrompt = String(activeClaim?.promptId || '') === normalizedPromptId;
237240
const activeLease = samePrompt && Number(activeClaim?.expiresAt || 0) > nowMs;
238241
if (activeLease && String(activeClaim?.claimantId || '') !== normalizedClaimantId) {

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

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1556,8 +1556,12 @@ async function waitForVisibleSidePanelStateRefresh() {
15561556
do {
15571557
pendingRefresh = visibleStateRefreshPromise;
15581558
await pendingRefresh.catch(() => {});
1559-
if (tabSwitchTransitionId != null) await waitForTabChatHandoffRetry();
1560-
} while (pendingRefresh !== visibleStateRefreshPromise || tabSwitchTransitionId != null);
1559+
if (tabSwitchTransitionId != null || visibleStateRefreshInProgress) {
1560+
await waitForTabChatHandoffRetry();
1561+
}
1562+
} while (pendingRefresh !== visibleStateRefreshPromise
1563+
|| tabSwitchTransitionId != null
1564+
|| visibleStateRefreshInProgress);
15611565
}
15621566

15631567
function schedulePersist() {
@@ -3474,14 +3478,27 @@ async function init() {
34743478
// Restore prior conversation for this tab (if any) — survives close/reopen.
34753479
const restoreTabId = currentTabId;
34763480
if (restoreTabId != null) {
3477-
const html = await loadTabChat(restoreTabId, { waitForHandoff: true });
3478-
if (html !== TAB_CHAT_LOAD_FAILED && currentTabId === restoreTabId && html) {
3479-
await hydrateRestoredChatHistory(restoreTabId, html);
3480-
if (currentTabId === restoreTabId) {
3481-
messagesEl.innerHTML = html;
3482-
messagesEl.querySelectorAll('[data-bound]').forEach(el => delete el.dataset.bound);
3483-
rebindRestoredMessageControls();
3481+
visibleStateRefreshInProgress = true;
3482+
syncSendButtonState();
3483+
try {
3484+
let html = TAB_CHAT_LOAD_FAILED;
3485+
while (html === TAB_CHAT_LOAD_FAILED && currentTabId === restoreTabId) {
3486+
html = await loadTabChat(restoreTabId, { waitForHandoff: true });
3487+
if (html === TAB_CHAT_LOAD_FAILED && currentTabId === restoreTabId) {
3488+
await waitForTabChatHandoffRetry();
3489+
}
34843490
}
3491+
if (currentTabId === restoreTabId && html && html !== TAB_CHAT_LOAD_FAILED) {
3492+
await hydrateRestoredChatHistory(restoreTabId, html);
3493+
if (currentTabId === restoreTabId) {
3494+
messagesEl.innerHTML = html;
3495+
messagesEl.querySelectorAll('[data-bound]').forEach(el => delete el.dataset.bound);
3496+
rebindRestoredMessageControls();
3497+
}
3498+
}
3499+
} finally {
3500+
visibleStateRefreshInProgress = false;
3501+
syncSendButtonState();
34853502
}
34863503
}
34873504

‎test/run.js‎

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17814,7 +17814,7 @@ test('sidepanel hydrates restored history ids before fallback records', () => {
1781417814
const initMatch = panel.match(/async function init\(\) \{([\s\S]*?)\n \/\/ Start observing the messages container/);
1781517815
assert.ok(initMatch, `${label}: init restore block missing`);
1781617816
const initBody = initMatch[1];
17817-
const initLoadIdx = initBody.indexOf('const html = await loadTabChat(restoreTabId, { waitForHandoff: true });');
17817+
const initLoadIdx = initBody.indexOf('loadTabChat(restoreTabId, { waitForHandoff: true })');
1781817818
const initHydrateIdx = initBody.indexOf('await hydrateRestoredChatHistory(restoreTabId, html);');
1781917819
const initGuardIdx = initBody.indexOf('if (currentTabId === restoreTabId) {', initHydrateIdx);
1782017820
const initDomIdx = initBody.indexOf('messagesEl.innerHTML = html;', initGuardIdx);
@@ -18307,8 +18307,8 @@ test('sidepanel does not miss startup tab switches before consuming tab-scoped s
1830718307

1830818308
if (label === 'chrome') {
1830918309
const restoreCaptureIdx = body.indexOf('const restoreTabId = currentTabId;');
18310-
const restoreLoadIdx = body.indexOf('const html = await loadTabChat(restoreTabId, { waitForHandoff: true });');
18311-
const restoreGuardIdx = body.indexOf('if (html !== TAB_CHAT_LOAD_FAILED && currentTabId === restoreTabId && html)');
18310+
const restoreLoadIdx = body.indexOf('loadTabChat(restoreTabId, { waitForHandoff: true })');
18311+
const restoreGuardIdx = body.indexOf('if (currentTabId === restoreTabId && html && html !== TAB_CHAT_LOAD_FAILED)');
1831218312
assert.notEqual(restoreCaptureIdx, -1, 'chrome: initial tab-chat restore should capture the target tab');
1831318313
assert.notEqual(restoreLoadIdx, -1, 'chrome: initial tab-chat restore should load the captured tab');
1831418314
assert.notEqual(restoreGuardIdx, -1, 'chrome: initial tab-chat restore should drop failed and stale async results');
@@ -21859,6 +21859,11 @@ test('context-menu ownership and stale-panel persistence guards are wired in bot
2185921859
/case 'claim_context_menu_prompt':[\s\S]*?contextMenuStorage\.claim\([\s\S]*?agent\.activeRunState\(tabId\)\?\.running \|\| detachedRunStarts\.has\(tabId\)/,
2186021860
`${label}: background should reject queued claim operations after another run reserves the tab`,
2186121861
);
21862+
assert.doesNotMatch(
21863+
background,
21864+
/case 'claim_context_menu_prompt':[\s\S]*?contextMenuStorage\.claim\([\s\S]*?msg\.claimantId,\s*Date\.now\(\),/,
21865+
`${label}: claim timestamps must be sampled inside the serialized storage operation`,
21866+
);
2186221867
assert.match(
2186321868
handler,
2186421869
/sendToBackground\('claim_context_menu_prompt', \{[\s\S]*?promptId: payload\.id,[\s\S]*?claimantId,[\s\S]*?\}\)/,
@@ -21886,7 +21891,7 @@ test('context-menu ownership and stale-panel persistence guards are wired in bot
2188621891
);
2188721892
assert.match(
2188821893
panel,
21889-
/let visibleStateRefreshPromise = Promise\.resolve\(\);[\s\S]*?async function waitForVisibleSidePanelStateRefresh\(\) \{[\s\S]*?pendingRefresh = visibleStateRefreshPromise;[\s\S]*?await pendingRefresh\.catch\(\(\) => \{\}\);[\s\S]*?tabSwitchTransitionId != null[\s\S]*?pendingRefresh !== visibleStateRefreshPromise \|\| tabSwitchTransitionId != null/,
21894+
/let visibleStateRefreshPromise = Promise\.resolve\(\);[\s\S]*?async function waitForVisibleSidePanelStateRefresh\(\) \{[\s\S]*?pendingRefresh = visibleStateRefreshPromise;[\s\S]*?await pendingRefresh\.catch\(\(\) => \{\}\);[\s\S]*?tabSwitchTransitionId != null \|\| visibleStateRefreshInProgress[\s\S]*?pendingRefresh !== visibleStateRefreshPromise[\s\S]*?\|\| tabSwitchTransitionId != null[\s\S]*?\|\| visibleStateRefreshInProgress/,
2189021895
`${label}: sends should wait until visibility refreshes and coordinated tab switches settle`,
2189121896
);
2189221897
assert.match(
@@ -21913,8 +21918,8 @@ test('context-menu ownership and stale-panel persistence guards are wired in bot
2191321918
);
2191421919
assert.match(
2191521920
panel,
21916-
/const restoreTabId = currentTabId;[\s\S]*?await loadTabChat\(restoreTabId, \{ waitForHandoff: true \}\);/,
21917-
`${label}: an initially visible panel should also wait for an outgoing document handoff`,
21921+
/const restoreTabId = currentTabId;[\s\S]*?visibleStateRefreshInProgress = true;[\s\S]*?let html = TAB_CHAT_LOAD_FAILED;[\s\S]*?while \(html === TAB_CHAT_LOAD_FAILED && currentTabId === restoreTabId\) \{[\s\S]*?await loadTabChat\(restoreTabId, \{ waitForHandoff: true \}\);[\s\S]*?await waitForTabChatHandoffRetry\(\);[\s\S]*?finally \{[\s\S]*?visibleStateRefreshInProgress = false;/,
21922+
`${label}: an initially visible panel should stay blocked and retry until it acquires handoff ownership`,
2191821923
);
2191921924
assert.match(
2192021925
panel,
@@ -22021,10 +22026,10 @@ test('context-menu prompt storage enforces a durable expiring lease', async () =
2202122026
const prompt = { id: `${label}-lease`, tabId: 12, text: 'Explain this selection' };
2202222027
await storage.save(prompt.tabId, prompt);
2202322028

22024-
const first = await storage.claim(prompt.tabId, prompt.id, 'panel-a', 1_000);
22025-
const duplicateOwner = await storage.claim(prompt.tabId, prompt.id, 'panel-a', 1_001);
22026-
const contender = await storage.claim(prompt.tabId, prompt.id, 'panel-b', 1_002);
22027-
const takeover = await storage.claim(prompt.tabId, prompt.id, 'panel-b', duplicateOwner.leaseExpiresAt);
22029+
const first = await storage.claim(prompt.tabId, prompt.id, 'panel-a', () => false, 1_000);
22030+
const duplicateOwner = await storage.claim(prompt.tabId, prompt.id, 'panel-a', () => false, 1_001);
22031+
const contender = await storage.claim(prompt.tabId, prompt.id, 'panel-b', () => false, 1_002);
22032+
const takeover = await storage.claim(prompt.tabId, prompt.id, 'panel-b', () => false, duplicateOwner.leaseExpiresAt);
2202822033

2202922034
assert.equal(first.claimed, true, `${label}: first panel should acquire the lease`);
2203022035
assert.equal(duplicateOwner.claimed, true, `${label}: lease acquisition should be idempotent for its owner`);
@@ -22062,15 +22067,15 @@ test('context-menu prompt storage enforces a durable expiring lease', async () =
2206222067
assert.equal(released.prompt?.id, prompt.id, `${label}: releasing ownership must retain the durable prompt`);
2206322068
assert.equal(data.has(storage.claimKey(prompt.tabId)), false, `${label}: released ownership should clear only the lease`);
2206422069
assert.equal(data.has(storage.key(prompt.tabId)), true, `${label}: released ownership must not consume the prompt`);
22065-
const reclaimed = await storage.claim(prompt.tabId, prompt.id, 'panel-c', takeover.leaseExpiresAt + 1);
22070+
const reclaimed = await storage.claim(prompt.tabId, prompt.id, 'panel-c', () => false, takeover.leaseExpiresAt + 1);
2206622071
assert.equal(reclaimed.claimed, true, `${label}: a newly visible panel should reclaim immediately after release`);
2206722072

2206822073
const blockedByRun = await storage.claim(
2206922074
prompt.tabId,
2207022075
prompt.id,
2207122076
'panel-d',
22072-
reclaimed.leaseExpiresAt,
2207322077
() => true,
22078+
reclaimed.leaseExpiresAt,
2207422079
);
2207522080
assert.equal(blockedByRun.claimed, false, `${label}: a queued claimant must be rejected once a run is reserved`);
2207622081
assert.equal(blockedByRun.reason, 'run-active', `${label}: run reservation rejection should remain retryable`);
@@ -22080,13 +22085,52 @@ test('context-menu prompt storage enforces a durable expiring lease', async () =
2208022085
assert.equal(data.has(storage.key(prompt.tabId)), false, `${label}: accepting the run should clear the durable prompt`);
2208122086
assert.equal(data.has(storage.claimKey(prompt.tabId)), false, `${label}: accepting the run should clear its lease`);
2208222087

22088+
const delayedClaimPrompt = {
22089+
id: `${label}-queued-claim-clock`,
22090+
tabId: 14,
22091+
text: 'Explain this page',
22092+
};
22093+
await storage.save(delayedClaimPrompt.tabId, delayedClaimPrompt);
22094+
const incumbentClaim = await storage.claim(
22095+
delayedClaimPrompt.tabId,
22096+
delayedClaimPrompt.id,
22097+
'incumbent-panel',
22098+
() => false,
22099+
2_000,
22100+
);
22101+
const claimSetGate = deferred();
22102+
nextSetGate = claimSetGate;
22103+
const claimBlockingSave = storage.save(delayedClaimPrompt.tabId, delayedClaimPrompt);
22104+
await waitMicrotasks(3);
22105+
22106+
const claimOriginalNow = Date.now;
22107+
let delayedClaim = null;
22108+
try {
22109+
Date.now = () => 2_000;
22110+
const queuedTakeover = storage.claim(
22111+
delayedClaimPrompt.tabId,
22112+
delayedClaimPrompt.id,
22113+
'next-panel',
22114+
() => false,
22115+
);
22116+
Date.now = () => incumbentClaim.leaseExpiresAt;
22117+
claimSetGate.resolve();
22118+
await claimBlockingSave;
22119+
delayedClaim = await queuedTakeover;
22120+
} finally {
22121+
Date.now = claimOriginalNow;
22122+
claimSetGate.resolve();
22123+
}
22124+
assert.equal(delayedClaim?.claimed, true, `${label}: queued claims should re-sample time and take over an expired lease`);
22125+
assert.equal(delayedClaim?.leaseExpiresAt, incumbentClaim.leaseExpiresAt + leaseMs, `${label}: a delayed claim should receive a fresh full lease`);
22126+
2208322127
const queuedPrompt = {
2208422128
id: `${label}-queued-expiry`,
2208522129
tabId: 13,
2208622130
text: 'Summarize this selection',
2208722131
};
2208822132
await storage.save(queuedPrompt.tabId, queuedPrompt);
22089-
const queuedClaim = await storage.claim(queuedPrompt.tabId, queuedPrompt.id, 'queued-panel', 1_000);
22133+
const queuedClaim = await storage.claim(queuedPrompt.tabId, queuedPrompt.id, 'queued-panel', () => false, 1_000);
2209022134
const setGate = deferred();
2209122135
nextSetGate = setGate;
2209222136
const blockingSave = storage.save(queuedPrompt.tabId, queuedPrompt);

0 commit comments

Comments
 (0)