From 6b84fe136e8b081e301337fcbb189b33546647d0 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Fri, 6 Nov 2026 09:35:25 -0600 Subject: [PATCH] fix(webui): order same-key files settles by commit, not start --- BUILD-LOG.md | 2 + docs/SESSIONS.md | 1 + packages/webui/src/public/files.js | 35 +++++---- .../webui/tests/w15-files-browser.test.mjs | 76 +++++++++++++++++++ 4 files changed, 101 insertions(+), 13 deletions(-) diff --git a/BUILD-LOG.md b/BUILD-LOG.md index 2154f6243..769c4c6d3 100644 --- a/BUILD-LOG.md +++ b/BUILD-LOG.md @@ -6312,3 +6312,5 @@ R5 quote fallback room (#797 D3, 2026-11-06, MVP3 lead): r4 review P2 found that 2026-11-06 UTC | #797 write_record JSON object compatibility — implementation and verification | Before: JSON-encoded requirements and decisions were refused and project-record-invalid detail was lost before the agent. After: strict object parsing preserves object-form bytes, and only record.write project-record-invalid details reach the tool result; docs remain Markdown. Exact-base negative control: seven behavioral failures. Node 26 serial gates: touched file 7/7, project 50/50, harness 316/316, bus excluding project-files 1642/1642, CLI 1204 pass and one federation acceptance skip reproduced on base. An initial candidate focused run failed one test due to a duplicate code prefix; corrected and rerun green. Evidence and handoff: /mnt/storage/scratch/mosaic-lead/mvp3/write-record-object-evidence/ and /mnt/storage/scratch/mosaic-lead/mvp3/write-record-object-report.md. Linked-worktree queue refused; local candidate for independent review, no push or deployment. 2026-11-06 UTC | #797 Files polish — implementation and author verification | Before: caller and Files module each rendered a heading, and request redraws replaced settled content with a loading message. After: callers retain the single heading; cloned DOM content is cached per deduplicated request list or validated thread key, fenced against older mounts and cleared at sign-out. Exact-base negative control: 8 behavioral failures, 3 passes. Node 26 serial authorized browser gate: 91 pass, 0 fail, 0 cancelled, 0 skipped. An initial detached-container test timed out because its fixture awaited its own held request; the same timeout reproduced on base and the fixture was corrected. All evidence and limits: /mnt/storage/scratch/mosaic-lead/mvp3/files-polish-evidence/ and /mnt/storage/scratch/mosaic-lead/mvp3/files-polish-report.md. Local implementation candidate for independent review; no push, merge or deployment. + +2026-11-06 UTC | #797 Files polish r2 — review r1 F1/F2 ordering fix | Before (86383a9d1): the per-key sequence fenced only cache writes, so an older same-key read in another live container painted stale files after a newer mount settled (F1), and a pending newer mount kept a completed result out of the cache, so a remount flashed "Reading files…" (F2). After: each mount takes a start order; a settle paints and caches only if no later-started mount for the key has committed, otherwise it shows the newer cached result; a pending mount never blocks an earlier completion. Reviewer probes ported as two tests; on 86383a9d1 they fail 2/13 for behavior, on the fix w15-files-browser 13/13 and the touched caller browser files 93/0/0/0 (Node 26 serial). Evidence: /mnt/storage/scratch/mosaic-lead/mvp3/files-polish-r2-evidence/. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index 9ccc77965..bb3661ae5 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -2103,3 +2103,4 @@ are never rewritten or removed; corrections are new entries. 2026-11-06 | mvp3-lead-797 (Claude, #797 lane) | project archive r9 WebUI fix on 4c85d3784 for the r8 review P1 | A same-thread refresh that resolves archived or unknown withdraws request-panel and unguarded thread writes before the session read. r8 negative fails for behavior; reviewer probe 4/0; focused 12/12; whole WebUI 661/0/0/0. Evidence: /mnt/storage/scratch/mosaic-lead/mvp3/project-archive-r9-evidence/. Local candidate for independent review; no push, merge or deployment. 2026-11-06 UTC | Terra implementation sub-agent | #797 write_record JSON object compatibility on fix/write-record-object from 13f81354e | Strict parsed JSON objects and scoped agent-visible validation details; exact-base seven-test negative, Node 26 serial gates and base-reproduced CLI skip recorded in /mnt/storage/scratch/mosaic-lead/mvp3/write-record-object-report.md. Queue refused linked worktree; local commit for independent review, no push or deployment. 2026-11-06 UTC | Codex implementation sub-agent | #797 Files heading and remount polish on files-polish from 4cc3d05b3 | Single caller heading and fenced sign-out-cleared DOM cache; Node 26 authorized browser gates 91/0/0/0, exact-base behavioral controls and fixture correction recorded in /mnt/storage/scratch/mosaic-lead/mvp3/files-polish-report.md. Queue refused linked worktree; canonical queue reported nothing for this seat. One local candidate for independent review; no push, merge or deployment. +2026-11-06 UTC | mvp3-lead-797 (Claude, #797 lane) | files-polish r2 on 86383a9d1 for Sol r1 F1/F2 | Per-key commit order fences live paints and caches the newest completed result; reviewer probes ported; r1 negative 11/2, w15-files 13/13, caller browser gate 93/0/0/0. Evidence: /mnt/storage/scratch/mosaic-lead/mvp3/files-polish-r2-evidence/. diff --git a/packages/webui/src/public/files.js b/packages/webui/src/public/files.js index 365e2beb6..bf4953542 100644 --- a/packages/webui/src/public/files.js +++ b/packages/webui/src/public/files.js @@ -4,9 +4,11 @@ const id = value => typeof value === 'string' && /^[A-Za-z0-9][A-Za-z0-9._-]{0,159}$/.test(value); const generations = new WeakMap(); const cache = new Map(); - const latest = new Map(); + // Per key: the start order of the newest mount whose result has been shown and cached. + const committed = new Map(); + let started = 0; let signedOut = false; - document.addEventListener('mosaic-signed-out', () => { signedOut = true; cache.clear(); latest.clear(); document.querySelectorAll('.mosaic-files').forEach(el => el.replaceChildren()); }); + document.addEventListener('mosaic-signed-out', () => { signedOut = true; cache.clear(); committed.clear(); document.querySelectorAll('.mosaic-files').forEach(el => el.replaceChildren()); }); function element(tag, content, className) { const node = document.createElement(tag); if (content !== undefined) node.textContent = content; @@ -37,21 +39,28 @@ container.classList.add('mosaic-files'); const ids = Array.isArray(requestIds) ? [...new Set(requestIds.filter(id))] : []; const cacheKey = typeof key === 'string' && key.length > 0 && key.length <= 320 ? `key:${key}` : `ids:${JSON.stringify(ids)}`; - const sequence = {}; - latest.set(cacheKey, sequence); + const order = ++started; const previous = !signedOut && !document.body.classList.contains('signed-out') && cache.get(cacheKey); container.replaceChildren(...(previous ? previous.map(node => node.cloneNode(true)) : [element('p', 'Reading files…')])); - function remember() { - if (!signedOut && !document.body.classList.contains('signed-out') && generations.get(container) === generation && latest.get(cacheKey) === sequence) { - cache.set(cacheKey, [...container.childNodes].map(node => node.cloneNode(true))); + // A settled read paints and caches only if no later-started mount for this key has already + // committed; otherwise this container shows that newer result. A pending later mount never + // blocks an earlier completion. Callers keep their own fences before calling this. + function commit(nodes) { + if ((committed.get(cacheKey) || 0) > order) { + const newer = cache.get(cacheKey); + if (newer) container.replaceChildren(...newer.map(node => node.cloneNode(true))); + return; } + container.replaceChildren(...nodes); + if (signedOut || document.body.classList.contains('signed-out')) return; + committed.set(cacheKey, order); + cache.set(cacheKey, nodes.map(node => node.cloneNode(true))); } if (ids.length) { const config = await read('/api/config'); if (signedOut || generations.get(container) !== generation) return; if (config?.requests !== true) { - container.replaceChildren(element('p', 'Files are unavailable.')); - remember(); + commit([element('p', 'Files are unavailable.')]); return; } } @@ -61,8 +70,8 @@ return { request, base, state, workspace }; })); if (signedOut || generations.get(container) !== generation || document.body.classList.contains('signed-out')) return; - container.replaceChildren(); - if (!ids.length) { container.append(element('p', 'No request files are available.')); remember(); return; } + if (!ids.length) { commit([element('p', 'No request files are available.')]); return; } + const nodes = []; for (const { request, base, state, workspace } of results) { const section = element('section', undefined, 'mosaic-files-request'); section.append(element('h4', `Request ${request}`), element('h5', 'Reviewed delivery')); @@ -98,9 +107,9 @@ } if (workspace.truncated) draft.append(element('p', 'The draft list was truncated.')); } - container.append(section); + nodes.push(section); } - remember(); + commit(nodes); } window.MosaicFiles = Object.freeze({ mount }); })(); diff --git a/packages/webui/tests/w15-files-browser.test.mjs b/packages/webui/tests/w15-files-browser.test.mjs index 7f66466d3..3ea57a7ec 100644 --- a/packages/webui/tests/w15-files-browser.test.mjs +++ b/packages/webui/tests/w15-files-browser.test.mjs @@ -228,3 +228,79 @@ test('Files keeps settled read errors visible on refresh', async t => { assert.doesNotMatch(text, /old\.html|old-draft\.txt/); assert.deepEqual(errors, []); }); + +// Review r1 F1/F2: per-key ordering across live containers (from the reviewer probes). +async function orderFixture(t) { + const fixture = await filesFixture(t); + await fixture.b.navigate(fixture.f.base + '/#/'); + await fixture.b.evaluate(`(() => { + const section = document.createElement('section'); + section.innerHTML = '

Files

'; + document.querySelector('#main').append(section); + window.reviewState = name => ({ version: 1, revision: 1, phase: 'complete', request: 'request-1', delivery: 'delivery-1', delivery_evidence: {id:'delivery-1', request:'request-1', artifact:{path:'output/' + name}} }); + window.fixtureState = window.reviewState('seed.html'); + window.fixtureWorkspace = {available:true, entries:[{name:'seed-draft.txt'}], truncated:false}; + window.reviewFetch = window.fetch; + window.reviewMount = (name, options = {}) => MosaicFiles.mount({container:document.querySelector('#review-' + name), requestIds:['request-1'], ...options}); + })()`); + return fixture; +} + +test('Files two live same-key mounts reject a late older on-screen settlement', async t => { + const { b, errors } = await orderFixture(t); + await b.evaluate("reviewMount('a')"); + await b.evaluate(`(() => { + window.fixtureState = reviewState('stale.html'); + window.fixtureWorkspace.entries = [{name:'stale-draft.txt'}]; + let ready; + window.reviewOldSnapshot = new Promise(resolve => ready = resolve); + let first = true; + window.fetch = async (url, init) => { + if (String(url) === '/api/requests/request-1' && first) { + first = false; + const capturedResponse = await window.reviewFetch(url, init); + await new Promise(resolve => { window.reviewReleaseOld = resolve; ready(); }); + return capturedResponse; + } + return window.reviewFetch(url, init); + }; + window.reviewOld = reviewMount('a'); + })()`); + await b.evaluate('window.reviewOldSnapshot'); + await b.evaluate(`(async () => { + window.fixtureState = reviewState('newest.html'); + window.fixtureWorkspace.entries = [{name:'newest-draft.txt'}]; + await reviewMount('b'); + window.reviewReleaseOld(); await window.reviewOld; + window.fetch = async (url, init) => { if(String(url) === '/api/config') await new Promise(resolve => window.reviewReleaseNext = resolve); return window.reviewFetch(url, init); }; + window.reviewNext = reviewMount('c'); + })()`); + const actual = await b.evaluate(`Object.fromEntries(['a','b','c'].map(id => [id, { connected:document.querySelector('#review-' + id).isConnected, text:document.querySelector('#review-' + id).textContent }]))`); + await b.evaluate('(async () => { reviewReleaseNext(); await reviewNext; })()'); + assert.deepEqual(errors, []); + assert.match(actual.b.text, /newest\.html/); + assert.match(actual.c.text, /newest\.html/, 'the shared cache correctly retains the newer mount'); + assert.doesNotMatch(actual.a.text, /stale\.html|stale-draft\.txt/, 'the superseded live mount must not paint an older result'); +}); + +test('Files a new container for the same key keeps a completed result while a later read is pending', async t => { + const { b, errors } = await orderFixture(t); + await b.evaluate(`(() => { + window.fetch = async (url, init) => { if(String(url) === '/api/config') await new Promise(resolve => window.reviewReleaseFirst = resolve); return window.reviewFetch(url, init); }; + window.reviewFirst = reviewMount('a'); + })()`); + await b.evaluate(`(() => { + window.fetch = async (url, init) => { if(String(url) === '/api/config') await new Promise(resolve => window.reviewReleaseSecond = resolve); return window.reviewFetch(url, init); }; + window.reviewSecond = reviewMount('b'); + })()`); + await b.evaluate('(async () => { reviewReleaseFirst(); await reviewFirst; })()'); + await b.evaluate(`(() => { + window.fetch = async (url, init) => { if(String(url) === '/api/config') await new Promise(resolve => window.reviewReleaseThird = resolve); return window.reviewFetch(url, init); }; + window.reviewThird = reviewMount('c'); + })()`); + const actual = await b.evaluate(`({completed:document.querySelector('#review-a').textContent, newMount:document.querySelector('#review-c').textContent})`); + await b.evaluate('(async () => { reviewReleaseSecond(); await reviewSecond; reviewReleaseThird(); await reviewThird; })()'); + assert.deepEqual(errors, []); + assert.match(actual.completed, /seed\.html/); + assert.match(actual.newMount, /seed\.html/, 'a completed result exists for this request key'); +});