mirror of
https://github.com/mudler/LocalAI.git
synced 2026-11-05 20:13:23 -05:00
fix(ui): do not change the request scope while rendering
AddonRegistryProvider called requests.setScope(actor) and replaced authority.current in the render body. React may start a render and throw it away (concurrent rendering, StrictMode), so a discarded render for a new session epoch aborted live reads and reset the list of the actor that is still on screen. Render now only reads the ref: for a new actor it shows an empty list at once. A layout effect commits the new scope and the ref. It runs at commit, before the browser paints and before any passive effect, so no read of the old actor can publish in between. The hook harness of the unit tests gains useLayoutEffect and a discarded-render mode, and a test shows that a discarded render leaves a live read alone. The private-access form test expects the phrase in the owner patch, as the owner now checks it. Assisted-by: Claude Code:claude-sonnet-5-5 Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
This commit is contained in:
4 files changed
+55
-13
No files matched your search
@@ -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 (
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
@@ -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()) },
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in new issue
Block a user