Skip to content

Commit 81d684d

Browse files
committed
Ignore stale question polls after newer review actions
1 parent c0cabe9 commit 81d684d

3 files changed

Lines changed: 19 additions & 2 deletions

File tree

‎docs/implementation/read-only-review.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,5 @@ Answer scrolling: when an asynchronous answer or failure arrives, Conversation f
8282
Conversation width: drag the divider on its left edge to resize between 280 and 480 pixels. Focus the divider and use Left/Right for 20-pixel steps or Home/End for the limits. The width survives collapsing/reopening the pane within the page; reloading uses the layout default. This replaces the hard-to-discover native corner resize grip. The browser regression covers pointer dragging, keyboard limits, code-pane space, and collapse/reopen.
8383

8484
Selection actions now appear in a bordered floating toolbar beside the visible selected lines, with Ask emphasized. The toolbar stays within the code viewport, leaves line numbers available for Shift-click extension, and hides when the selection scrolls out of view. Clear selection removes it. The long-diff browser regression checks proximity, viewport bounds, attachment, scrolling away/back, and clearing; existing range/highlight tests remain required.
85+
86+
PR #11 review round 1 reproduced a stale-poll race: a held question-list response hid a question submitted after the poll began. Review actions and refreshes now advance a generation counter; earlier poll successes and errors are ignored. The browser regression failed with one visible note instead of two before the fix.

‎test/browser/review.spec.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,3 +212,14 @@ test('Clear selection removes native text selection as well as selected lines',a
212212
await page.locator('#code').press('ArrowRight');
213213
await expect(page.getByRole('group',{name:'Selected code'})).toBeHidden();
214214
});
215+
test('ignores question polls started before a newer submission',async({page})=>{
216+
const config=app.service.config;await app.close();app=await startServer(config,0,()=>new Promise(()=>{}));
217+
await page.goto(app.url);await page.getByLabel('Question about this item').fill('First question');await page.getByRole('button',{name:'Ask agent',exact:true}).click();await expect(page.getByText('Agent · Answering…',{exact:true})).toBeVisible();
218+
let release!:()=>void,arrived!:()=>void;const held=new Promise<void>(r=>release=r),captured=new Promise<void>(r=>arrived=r);
219+
await page.route('**/api/questions',async route=>{const response=await route.fetch();arrived();await held;await route.fulfill({response});});
220+
await captured;
221+
await page.getByLabel('Question about this item').fill('Second question');await page.getByRole('button',{name:'Ask agent',exact:true}).click();await expect(page.locator('#notes .note')).toHaveCount(2);
222+
release();await page.waitForResponse(response=>response.url().endsWith('/api/questions'));
223+
await page.waitForTimeout(100);
224+
expect(await page.locator('#notes .note').count()).toBe(2);
225+
});

‎web/public/app.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ let data,
1919
mode = "question",
2020
since = false,
2121
busy = false;
22+
let reviewGeneration = 0;
2223
const drafts = new Map();
2324
const attachments = new Map();
2425
let snippetSelection = null;
@@ -75,6 +76,7 @@ function showFailure(message) {
7576
async function refresh() {
7677
if (busy) return;
7778
busy = true;
79+
reviewGeneration++;
7880
renderAttachment();
7981
$("banner").textContent = "Linking changes to plan items…";
8082
try {
@@ -96,6 +98,7 @@ async function refresh() {
9698
async function act(command) {
9799
if (busy || !data) return false;
98100
busy = true;
101+
reviewGeneration++;
99102
renderAttachment();
100103
try {
101104
rememberDraft();
@@ -597,8 +600,9 @@ setInterval(async()=>{
597600
renderNotes({ follow: true });return;
598601
}
599602
pollingQuestions=true;
600-
try {const response=await api("/api/questions");if(data){data.notes=response.notes;renderNotes({ follow: true });}}
601-
catch { $("saved").textContent="Could not refresh agent answers. Use Refresh to reconnect."; }
603+
const generation = reviewGeneration;
604+
try {const response=await api("/api/questions");if(data && generation === reviewGeneration){data.notes=response.notes;renderNotes({ follow: true });}}
605+
catch { if (generation === reviewGeneration) $("saved").textContent="Could not refresh agent answers. Use Refresh to reconnect."; }
602606
finally {pollingQuestions=false;}
603607
},2000);
604608
$("settings").onclick=async()=>{

0 commit comments

Comments
 (0)