diff --git a/core/http/react-ui/src/addons/registry.jsx b/core/http/react-ui/src/addons/registry.jsx index c4f71f51ef..d6fdb01df6 100644 --- a/core/http/react-ui/src/addons/registry.jsx +++ b/core/http/react-ui/src/addons/registry.jsx @@ -1,5 +1,5 @@ /* eslint-disable no-unused-vars -- components used only inside JSX look unused to this config, which has no eslint-plugin-react */ -import { useCallback, useEffect, useMemo, useRef, useState, useSyncExternalStore } from 'react' +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState, useSyncExternalStore } from 'react' import { useLocation, useNavigate } from 'react-router-dom' import { useTranslation } from 'react-i18next' import { useAuth } from '../context/AuthContext' @@ -63,10 +63,17 @@ export function AddonRegistryProvider({ children, api = addonsApi }) { // The opaque auth epoch also changes on same-user refresh. Invalidate reads // and pending Home dispatch synchronously, before effect cleanup can run. const actor = `${user?.id || ''}:${allowed}:${isAdmin}:${sessionEpoch}` - requests.setScope(actor) const authority = useRef({ actor, allowed, list: EMPTY }) - if (authority.current.actor !== actor) authority.current = { actor, allowed, list: EMPTY } - authority.current.allowed = allowed + // Render only reads the ref: a render that React discards (concurrent or + // StrictMode) must not change the scope of live requests. A render for a new + // actor shows an empty list at once, and the layout effect below commits + // the new scope before the browser paints and before any passive effect. + const viewed = authority.current.actor === actor ? authority.current.list : EMPTY + useLayoutEffect(() => { + requests.setScope(actor) + if (authority.current.actor !== actor) authority.current = { actor, allowed, list: EMPTY } + authority.current.allowed = allowed + }, [requests, actor, allowed]) const [consumers, setConsumers] = useState(0) const [layers, setLayers] = useState([]) const navigateRef = useRef(navigate) @@ -261,9 +268,9 @@ export function AddonRegistryProvider({ children, api = addonsApi }) { // only because the answer is not in yet. actor, known: serverOn !== null, - status: authority.current.list.status, - apps: authority.current.list.apps, - error: authority.current.list.error, + status: viewed.status, + apps: viewed.apps, + error: viewed.error, instances, allowed, refresh, @@ -283,7 +290,7 @@ export function AddonRegistryProvider({ children, api = addonsApi }) { // them leaves the host with no entries, never with a broken render. navigationItems: guarded(() => buildNavigation(list.apps, { language: i18n.language })), chatActions: guarded(() => availableChatActions(instances, list.apps, { hasFeature: () => allowed })), - }), [actor, serverOn, list, instances, allowed, refresh, navigateHomeCommand, navigateModelAction, ingest, acquire, manager, i18n.language, runAction, pushLayer, presentApproval]) + }), [actor, serverOn, list, viewed, instances, allowed, refresh, navigateHomeCommand, navigateModelAction, ingest, acquire, manager, i18n.language, runAction, pushLayer, presentApproval]) const boundaryLabels = { title: t('shell.error.title'), text: t('shell.error.text'), retryLabel: t('shell.error.retry') } return ( diff --git a/core/http/react-ui/tests/unit/addonhomecommands.test.mjs b/core/http/react-ui/tests/unit/addonhomecommands.test.mjs index 6ecd2c5a2c..3b7c7fe304 100644 --- a/core/http/react-ui/tests/unit/addonhomecommands.test.mjs +++ b/core/http/react-ui/tests/unit/addonhomecommands.test.mjs @@ -210,7 +210,9 @@ async function registrySessionHarness() { render(); h.flush() await new Promise(resolve => setImmediate(resolve)) render(); h.flush(); render() - return { h, render, reads, paths, revalidated, refreshSession(user = auth.user) { auth = {...auth,user,sessionEpoch:auth.sessionEpoch + 1}; return render() } } + return { h, render, reads, paths, revalidated, + refreshSession(user = auth.user) { auth = {...auth,user,sessionEpoch:auth.sessionEpoch + 1}; return render() }, + discardRefresh() { const live = auth; auth = {...auth,sessionEpoch:auth.sessionEpoch + 1}; try { return h.renderDiscarded({api}) } finally { auth = live } } } } test('Home addons: actual registry rejects same-user refresh late poll before effect cleanup', async () => { @@ -275,3 +277,18 @@ for (const accountSwitch of [false, true]) test(`Apps registry: late list cannot assert.equal(render().apps[0].id, 'paint') assert.equal(r.render().apps[0].id, 'paint') }) + +test('Home addons: a discarded render does not change the scope of live requests', async () => { + const r = await registrySessionHarness() + const value = r.render() + value.acquire(); r.render(); r.h.flush() + const poll = r.reads[0] + const shown = r.discardRefresh() // React renders a new session epoch and throws the render away. + assert.deepEqual(shown.apps, [], 'the discarded render itself never shows the old list') + assert.equal(poll.signal.aborted, false, 'the live request keeps its scope') + poll.resolve([app()]) + await new Promise(resolve => setImmediate(resolve)) + assert.equal(r.revalidated.length, 1, 'the live read still publishes') + r.refreshSession() // The same epoch committed for real moves the scope at once. + assert.equal(r.reads.length, 1) +}) diff --git a/core/http/react-ui/tests/unit/chatcommands-lifecycle.mjs b/core/http/react-ui/tests/unit/chatcommands-lifecycle.mjs index 507072aceb..892932459d 100644 --- a/core/http/react-ui/tests/unit/chatcommands-lifecycle.mjs +++ b/core/http/react-ui/tests/unit/chatcommands-lifecycle.mjs @@ -6,6 +6,9 @@ export function hookHarness(file, name, imports, transform = source => source) { const slots = [] let cursor = 0 let pending = new Map() + // Layout effects run at commit, before the render returns to its caller and + // before any passive effect. A discarded render never commits them. + let layout = new Map() const same = (a, b) => a && b && a.length === b.length && a.every((v, i) => Object.is(v, b[i])) const hooks = { useState(initial) { @@ -24,6 +27,13 @@ export function hookHarness(file, name, imports, transform = source => source) { slots[i] = { deps, cleanup: fn() } }) }, + useLayoutEffect(fn, deps) { + const i = cursor++ + if (!same(slots[i]?.deps, deps)) layout.set(i, () => { + slots[i]?.cleanup?.() + slots[i] = { deps, cleanup: fn() } + }) + }, useMemo(fn, deps) { const i = cursor++ if (!same(slots[i]?.deps, deps)) slots[i] = { deps, value: fn() } @@ -37,7 +47,14 @@ export function hookHarness(file, name, imports, transform = source => source) { const scope = { ...hooks, ...imports } const hook = new Function(...Object.keys(scope), `${source}; return ${name}`)(...Object.values(scope)) return { - render(...args) { cursor = 0; return hook(...args) }, + render(...args) { + cursor = 0 + const out = hook(...args) + const effects = layout; layout = new Map(); effects.forEach(fn => fn()) + return out + }, + // A render that React throws away: nothing is committed, so no layout effect runs. + renderDiscarded(...args) { cursor = 0; const out = hook(...args); layout = new Map(); return out }, unmount() { pending.clear(); slots.forEach(slot => slot?.cleanup?.()) }, flush() { const effects = pending; pending = new Map(); effects.forEach(fn => fn()) }, } diff --git a/core/http/react-ui/tests/unit/hostsettings.test.mjs b/core/http/react-ui/tests/unit/hostsettings.test.mjs index 1ce9d73949..54bb9d852c 100644 --- a/core/http/react-ui/tests/unit/hostsettings.test.mjs +++ b/core/http/react-ui/tests/unit/hostsettings.test.mjs @@ -168,7 +168,7 @@ test('malformed reload never publishes a partial snapshot and successful read re // Load the real JSX form and network-policy block. Only routing, translation // and the unrelated egress telemetry read are stubbed; owner reads/writes use // the real client and dispatch serializer against a strict fetch boundary. -test('mounted HostControlsForm confirms private access locally and sends only the owner policy patch', async () => { +test('mounted HostControlsForm asks for the private access phrase and sends it to the owner with the policy patch', async () => { const server = await createServer({ configFile: false, ssr: { noExternal: ['react-i18next'] }, @@ -205,7 +205,7 @@ test('mounted HostControlsForm confirms private access locally and sends only th } if (body.action === 'host-settings-read') return reply(dto(revision, { chats_egress_allow_private: allowPrivate })) assert.equal(body.action, 'host-settings-update') - const unknown = Object.keys(body.arguments.patch).filter(key => key !== 'chats_egress_allow_private') + const unknown = Object.keys(body.arguments.patch).filter(key => !['chats_egress_allow_private', 'chats_egress_private_confirm'].includes(key)) if (unknown.length) { rejected.push(...unknown) return new Response(JSON.stringify({ payload: { error: 'unknown_policy_field' } })) @@ -233,7 +233,8 @@ test('mounted HostControlsForm confirms private access locally and sends only th await act(async () => apply().click()) assert.deepEqual(rejected, [], 'strict owner rejected unknown policy fields') assert.deepEqual(calls.map(c => c.body.action), ['host-settings-read', 'host-settings-update', 'host-settings-read']) - assert.deepEqual(calls[1].body.arguments, { expectedRevision: 7, patch: { chats_egress_allow_private: true } }) + // The owner checks the phrase too, so a direct call cannot skip it. + assert.deepEqual(calls[1].body.arguments, { expectedRevision: 7, patch: { chats_egress_allow_private: true, chats_egress_private_confirm: EGRESS_PRIVATE_PHRASE } }) assert.ok(calls.every(c => c.url !== '/api/settings')) assert.match(document.body.textContent, /revision 8/) assert.equal(document.querySelector('[data-testid="settings-pending"]'), null)