From c7aecbffa9f52613e2229542bf20e361ef2dc79d Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Sat, 14 Nov 2026 23:51:59 -0600 Subject: [PATCH] fix(webui): sp1 second review round, a read or file save in flight at a switch keeps nothing (#1873, lane sp1) The second review found two paths the first fix round left open. - The project-file save captures orgEpoch before its POST. When a switch happened meanwhile, it neither paints nor announces the old file's outcome on the new organization's page. An opaqueredirect still signs out. - A live read (reader, get.api), the palette's inbox and tasks reads and the board poll keep their answer in `last` only while the organization they were sent for is still open. A later cached render on the new organization no longer draws the old one's rows. single-pane test 11 holds acme's file save across a switch to beta and asserts nothing is announced: red on the previous bus.js ("Saved as commit cccccccccccc."), green now. The kept-read guard has no browser test. --- packages/webui/src/public/bus.js | 22 +++++++++++----- .../webui/tests/single-pane-browser.test.mjs | 25 +++++++++++++++++++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/packages/webui/src/public/bus.js b/packages/webui/src/public/bus.js index 10a1db894..8d9843552 100644 --- a/packages/webui/src/public/bus.js +++ b/packages/webui/src/public/bus.js @@ -75,13 +75,17 @@ window.MosaicIcons.succeeded(); return body; } + // #1873 SP1: bumped by each in-place organization switch. A write, or a read that keeps its answer, + // captures it before its await and drops the answer when it changed, so nothing the old organization + // answered lands on the new page or in its kept answers. + let orgEpoch = 0; // A reader for one render. `live` asks the server; otherwise only kept answers are used. function reader(live) { const used = []; const get = async (verb, subject) => { const k = subject === undefined ? verb : `${verb}:${subject}`; let r; - if (live) { r = await fetchRead(verb, subject); last.set(k, r); } + if (live) { const epoch = orgEpoch; r = await fetchRead(verb, subject); if (epoch === orgEpoch) last.set(k, r); } else if (!(r = last.get(k))) throw new ReadError('not-read', 'not read before'); used.push(r.at); if (verb === 'messages') { messageCount = r.unread; return r; } @@ -90,7 +94,7 @@ get.api = async path => { const k = `api:${path}`; let r; - if (live) { r = await fetchApi(path); last.set(k, r); } + if (live) { const epoch = orgEpoch; r = await fetchApi(path); if (epoch === orgEpoch) last.set(k, r); } else if (!(r = last.get(k))) throw new ReadError('not-read', 'not read before'); used.push(r.at); return r; @@ -345,9 +349,6 @@ let configRead = Promise.resolve(); // #1873 SP1: the config read an organization switch started, until the next render has waited for it. let switchRead = null; - // #1873 SP1: bumped by each in-place organization switch. A write captures it before its await and - // drops its answer when it changed, so nothing the old organization answered lands on the new page. - let orgEpoch = 0; let canSend = false, canMessage = false, chatSend = false, sendRoles = null, sending = false, receipt = null, replies = []; const REPLIES = 50; // the broker's cap function repliesHtml() { @@ -1539,6 +1540,7 @@ if (!s || !saveable(s)) return; const sent = { project: s.project, path: s.path, base: s.commit, content: s.text }, unknown = 'Outcome unknown: the save may have gone through. Copy your text, then reload the file to check before saving again. Draft kept.'; s.saving = true; s.status = ''; + const epoch = orgEpoch; let res = null, body = null; try { paintDoc(); @@ -1550,6 +1552,9 @@ } else if (body?.sent === false) s.status = `${typeof body.message === 'string' && body.message ? body.message : 'Not saved.'} Draft kept.`; else s.status = unknown; } finally { s.saving = false; } // a throw above must not leave Save off and Reload refused + // #1873 SP1: a switch while this was in flight cleared the drafts; the old file's outcome is not + // painted or announced on the new organization's page. + if (epoch !== orgEpoch) { if (res?.type === 'opaqueredirect') signOut(); return; } paintDoc(); announce(s.status); if (res?.type === 'opaqueredirect') signOut(); @@ -1756,12 +1761,15 @@ // The board page keeps its own title; the Inbox count still follows the bus. if (shown !== null) { shown = null; unmountEditor(); $('bus-view').innerHTML = ''; document.title = 'Dashboard ยท Mosaic Console'; if (navigated) $('main').focus(); } // The held roles (row 95) are read with it. A refusal of either forgets both, as on the bus pages. + const epoch = orgEpoch; const [inbox, agents, messages, tasks, queue, business, projects] = await Promise.allSettled([ fetchRead('inbox'), fetchRead('agents'), fetchRead('messages'), fetchRead('tasks'), fetchApi('queue'), fetchApi('business'), fetch('/api/chat/projects', { cache: 'no-store', redirect: 'manual' }).then(async r => { if (r.type === 'opaqueredirect') { signOut(); throw signedOut(); } const body = await r.json(); if (signInRequired(r, body)) { signOutSso(); throw signedOut(); } if (!r.ok || !Array.isArray(body.rows)) throw new ReadError(body?.error, body?.message, r.status); return body; }) ]); const failed = [inbox, agents, messages, tasks].filter(s => s.status === 'rejected').map(s => s.reason); if (failed.some(err => err.code === 'signed-out')) return; + // #1873 SP1: an answer read for an organization that is no longer open is not kept. + if (epoch !== orgEpoch) return; if (inbox.status === 'fulfilled') last.set('inbox', inbox.value); if (agents.status === 'fulfilled') last.set('agents', agents.value); if (messages.status === 'fulfilled') last.set('messages', messages.value); @@ -1902,7 +1910,9 @@ if (d.open) return; pkBack = document.activeElement; pkAt = 0; $('cmdk-input').value = ''; pkItems(); pkFill(); d.showModal(); $('cmdk-input').focus(); - Promise.allSettled(['inbox', 'tasks'].map(async v => last.set(v, await fetchRead(v)))).then(rs => { + const epoch = orgEpoch; + Promise.allSettled(['inbox', 'tasks'].map(async v => { const r = await fetchRead(v); if (epoch === orgEpoch) last.set(v, r); })).then(rs => { + if (epoch !== orgEpoch) return; if (rs.some(r => r.status === 'rejected' && refusal(r.reason))) { forget(); sections(routeOf().split('?')[0]); } if (d.open) { pkItems(); pkFill(); } }); diff --git a/packages/webui/tests/single-pane-browser.test.mjs b/packages/webui/tests/single-pane-browser.test.mjs index 40e01b623..fe9a1f446 100644 --- a/packages/webui/tests/single-pane-browser.test.mjs +++ b/packages/webui/tests/single-pane-browser.test.mjs @@ -285,3 +285,28 @@ test('a restart started on one organization finishes on that organization after // beta's page draws nothing of acme's chain. assert.equal(await b.evaluate('!!document.querySelector("[data-agent-restart-status=coder]")'), false); }); + +test('a file save still in flight at a switch: its outcome is not announced on the next organization', async t => { + const json = 'new Response(JSON.stringify(x), { status: 200, headers: { "content-type": "application/json" } })'; + // acme's file reads are stubbed; its save POST is held until the test releases it, then answers with a commit. + const init = `window.__posts = []; const original = window.fetch; + window.fetch = async (url, options = {}) => { const s = String(url); let x; + if (/^\\/api\\/orgs\\/[^/]+\\/config$/.test(s)) { const r = await original(url, options), body = await r.json(); x = { ...body, send: true, edit: true }; return ${json}; } + if (s.startsWith('/api/orgs/acme/project-file?')) { x = { path: 'docs/a.md', commit: 'a'.repeat(40), content: '# A\\n', at: new Date().toISOString() }; return ${json}; } + if (options.method === 'POST' && s === '/api/orgs/acme/project-file') { __posts.push(JSON.parse(options.body)); await new Promise(r => { window.__release = r; }); + x = { path: 'docs/a.md', commit: 'c'.repeat(40) }; return ${json}; } + return original(url, options); };`; + const { b, wait } = await setup(t, { path: `/#/orgs/acme/docs/site/${encodeURIComponent('docs/a.md')}`, init }); + await wait('document.querySelector("#doc-form")'); + if (!await b.evaluate('!!document.querySelector("#doc-text")')) await b.evaluate('document.querySelector("#doc-mode-source").click(), true'); + await wait('document.querySelector("#doc-text")?.value === "# A\\n"'); + await b.evaluate('(() => { const a = document.querySelector("#doc-text"); a.value = "# A\\n\\nmore\\n"; a.dispatchEvent(new Event("input", { bubbles: true })); document.querySelector("#doc-form").requestSubmit(); })()'); + await wait('typeof window.__release === "function"'); + assert.equal(await b.evaluate('window.__posts[0]?.base'), 'a'.repeat(40)); + await b.evaluate('location.hash = "#/orgs/beta/organization"'); + await wait('location.hash === "#/orgs/beta/organization" && document.querySelector("[data-org-dashboard] h1")?.textContent === "Beta Works"'); + await b.evaluate('window.__release()'); + await new Promise(r => setTimeout(r, 700)); + assert.equal(await b.evaluate('location.hash'), '#/orgs/beta/organization'); + assert.doesNotMatch(await b.evaluate('document.getElementById("announce")?.textContent ?? ""'), /Saved as commit|Draft kept/); +});