From 2e5def34edd0fe360429d2a3590b0094cf752544 Mon Sep 17 00:00:00 2001 From: "Anthony Fu (via agent)" Date: Thu, 13 Aug 2026 04:31:32 +0000 Subject: [PATCH] =?UTF-8?q?fix(hub):=20stop=20no-op=20state=20churn=20?= =?UTF-8?q?=E2=80=94=20docks=20republish,=20message=20re-adds,=20a11y=20sc?= =?UTF-8?q?an=20loop?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the hub-vite example the `devframe:docks` shared state rebroadcast every ~1s with identical content and `hub:messages:add` fired continuously. Root cause: a self-sustaining feedback loop — a11y agent scan → messages.add (summary flips loading→idle + one entry per rule, identical content) → message:updated (no dedupe) → docks shared-state republish (no change detection, fresh array every time) → client re-render → unconditional innerHTML rewrite → DOM mutation → a11y MutationObserver → scan (600ms debounce ≈ 1s period) hub-next was immune only because React reconciliation no-ops identical lists, keeping the MutationObserver quiet. Independent fixes, each of which breaks the cycle on its own: - hub context: hash-guard the `devframe:docks` republish — dock, terminal, and message events publish only when the dock list content actually changed. - messages host: content dedupe in `update()` — an identical re-add emits no `message:updated` and bumps no clock; an identical re-add carrying `autoDelete` still resets the keep-alive timer. - a11y agent: observer/interaction-driven rescans run in the background (no loading→idle summary churn), and the messages reporter skips re-sending entries whose content is unchanged since the last scan. - hub-vite example: `renderList` skips identical innerHTML rewrites so repainting an unchanged list is no longer a DOM mutation. - shared-state client host: applied server updates are no longer reflected back to the server as `server-state:set`/`patch` events the server would just discard (one wasted wire message — a whole HTTP POST over SSE — per server-side state tick, on any transport). Verified live: with the fixes, an idle hub-vite over SSE settles to exactly its intentional 2s drawer poll (all parked 200s) — the 202 message-add storm and docks broadcasts are gone. --- examples/hub-vite/src/client/main.ts | 16 +++++-- .../src/adapters/__tests__/sse-e2e.test.ts | 22 +++++++++- .../devframe/src/client/rpc-shared-state.ts | 28 ++++++++++++ .../hub/src/node/__tests__/context.test.ts | 43 +++++++++++++++++++ .../src/node/__tests__/host-messages.test.ts | 41 ++++++++++++++++++ packages/hub/src/node/context.ts | 23 ++++++++-- packages/hub/src/node/host-messages.ts | 15 +++++-- plugins/a11y/src/inject/index.ts | 12 ++++-- plugins/a11y/src/inject/messages.ts | 18 +++++++- plugins/a11y/tests/inject-messages.test.ts | 27 +++++++++++- 10 files changed, 226 insertions(+), 19 deletions(-) diff --git a/examples/hub-vite/src/client/main.ts b/examples/hub-vite/src/client/main.ts index d99e9496..27e770d0 100644 --- a/examples/hub-vite/src/client/main.ts +++ b/examples/hub-vite/src/client/main.ts @@ -70,12 +70,20 @@ function renderTransportToggle(current: TransportPref) { button.addEventListener('click', () => applyTransportPref(button.dataset.transport as TransportPref)) } +const renderedMarkup = new WeakMap() + function renderList(host: HTMLElement, items: readonly T[], render: (item: T) => string) { - if (!items.length) { - host.innerHTML = '
  • empty
    • ' + const html = items.length + ? items.map(render).join('') + : '
  • empty
    • ' + // Skip identical rewrites: an innerHTML assignment always recreates the + // nodes, which counts as a DOM mutation — and the a11y inspector's in-page + // agent watches the body for mutations to schedule rescans. Repainting an + // unchanged list every poll would keep it scanning forever. + if (renderedMarkup.get(host) === html) return - } - host.innerHTML = items.map(render).join('') + renderedMarkup.set(host, html) + host.innerHTML = html } // Session-lifetime cache of resolved dock-icon SVGs, keyed by the icon id diff --git a/packages/devframe/src/adapters/__tests__/sse-e2e.test.ts b/packages/devframe/src/adapters/__tests__/sse-e2e.test.ts index 77eb19ab..27895b03 100644 --- a/packages/devframe/src/adapters/__tests__/sse-e2e.test.ts +++ b/packages/devframe/src/adapters/__tests__/sse-e2e.test.ts @@ -223,7 +223,7 @@ describe('sse transport e2e — full client', () => { } }) - it('shared state syncs over SSE', async () => { + it('shared state syncs over SSE without echoing server updates back as POSTs', async () => { const { devframe, origin, base, close } = await bootServer('sse-client-state') try { const ctx = await devframe.context @@ -232,19 +232,39 @@ describe('sse transport e2e — full client', () => { }) stubLocation(origin, base) + let postCount = 0 const client = await getDevframeRpcClient({ baseURL: `${origin}${base}`, transport: 'sse', otpParam: false, simpleAuth: false, + sseOptions: { + fetch: (input, init) => { + if (init?.method === 'POST') + postCount++ + return fetch(input, init) + }, + }, }) await client.ensureTrusted(5000) const clientState = await client.sharedState.get<{ count: number }>('sse-test:counter') expect(clientState.value().count).toBe(1) + // Server-side ticks stream down; none of them may reflect back up as + // a `server-state:set` POST — the echo the server would just discard. + const postsBeforeTicks = postCount serverState.mutate(() => ({ count: 2 })) await vi.waitFor(() => expect(clientState.value().count).toBe(2)) + serverState.mutate(() => ({ count: 3 })) + serverState.mutate(() => ({ count: 4 })) + await vi.waitFor(() => expect(clientState.value().count).toBe(4)) + expect(postCount).toBe(postsBeforeTicks) + + // A local mutation still forwards to the server (exactly once). + clientState.mutate(() => ({ count: 5 })) + await vi.waitFor(() => expect(serverState.value().count).toBe(5)) + expect(postCount).toBe(postsBeforeTicks + 1) client.close?.() } finally { diff --git a/packages/devframe/src/client/rpc-shared-state.ts b/packages/devframe/src/client/rpc-shared-state.ts index 3e3ffb87..ad67ea6c 100644 --- a/packages/devframe/src/client/rpc-shared-state.ts +++ b/packages/devframe/src/client/rpc-shared-state.ts @@ -3,6 +3,14 @@ import type { SharedState, SharedStatePatch } from 'devframe/utils/shared-state' import type { DevframeRpcClient } from './rpc' import { createSharedState } from 'devframe/utils/shared-state' +/** + * Upper bound on remembered server-originated syncIds. An update's own + * `updated` event fires synchronously after it is applied, so the set only + * needs to outlive the brief window between applying a server update and + * observing its emission — 100 comfortably covers a burst. + */ +const MAX_REMOTE_SYNC_IDS = 100 + export function createRpcSharedStateClientHost(rpc: DevframeRpcClient): RpcSharedStateHost { const sharedState = new Map>() const stateDisposers = new Map void>() @@ -10,6 +18,21 @@ export function createRpcSharedStateClientHost(rpc: DevframeRpcClient): RpcShare const keyAddedListeners = new Set<(key: string) => void>() const isStaticBackend = rpc.connectionMeta.backend === 'static' + // Server-originated syncIds, so the forwarding listener below can tell a + // local mutation (forward it to the server) from an applied server update + // (already the server's own — forwarding it back would be a pure echo the + // server discards, at the cost of one wire message per update; over the + // SSE transport that's a whole HTTP POST per server-side state tick). + const remoteSyncIds = new Set() + function rememberRemoteSyncId(syncId: string): void { + remoteSyncIds.add(syncId) + if (remoteSyncIds.size > MAX_REMOTE_SYNC_IDS) { + const oldest = remoteSyncIds.values().next().value + if (oldest !== undefined) + remoteSyncIds.delete(oldest) + } + } + function mergeWithInitialValue(key: string, serverState: any): any { const initial = initialValues.get(key) if (initial && typeof initial === 'object' && !Array.isArray(initial) @@ -26,6 +49,7 @@ export function createRpcSharedStateClientHost(rpc: DevframeRpcClient): RpcShare const state = sharedState.get(key) if (!state || state.syncIds.has(syncId)) return + rememberRemoteSyncId(syncId) state.mutate(() => mergeWithInitialValue(key, fullState), syncId) }, }) @@ -37,6 +61,7 @@ export function createRpcSharedStateClientHost(rpc: DevframeRpcClient): RpcShare const state = sharedState.get(key) if (!state || state.syncIds.has(syncId)) return + rememberRemoteSyncId(syncId) state.patch(patches, syncId) }, }) @@ -46,6 +71,9 @@ export function createRpcSharedStateClientHost(rpc: DevframeRpcClient): RpcShare offs.push(state.on('updated', (fullState, patches, syncId) => { if (isStaticBackend) return + // An update the server just sent needs no reflection back to it. + if (remoteSyncIds.has(syncId)) + return if (patches) { rpc.callEvent('devframe:rpc:server-state:patch', key, patches, syncId) } diff --git a/packages/hub/src/node/__tests__/context.test.ts b/packages/hub/src/node/__tests__/context.test.ts index 1b87e723..a80ac274 100644 --- a/packages/hub/src/node/__tests__/context.test.ts +++ b/packages/hub/src/node/__tests__/context.test.ts @@ -29,6 +29,49 @@ describe('createHubContext shared state', () => { }) }) +describe('createHubContext docks state churn', () => { + it('message events republish the docks state only when the dock list changed', async () => { + const context = await createHubContext({ + cwd: process.cwd(), + mode: 'build', // debounceMs = 0 — republishes settle synchronously-ish + host: createHost(), + }) + context.docks.register({ + type: 'iframe', + id: 'devframes_plugin_terminals', + title: 'Terminals', + icon: 'ph:terminal-window-duotone', + url: '/__devframes_plugin_terminals/', + }) + const docks = await context.rpc.sharedState.get('devframe:docks') + // Let the register's debounced publish settle first. + await vi.waitFor(() => expect(docks.value()).toHaveLength(1)) + + let publishes = 0 + const off = docks.on('updated', () => void publishes++) + + // The periodic-producer pattern: message adds/updates that leave the + // dock list untouched must not rebroadcast `devframe:docks`. + await context.messages.add({ id: 'scan', level: 'info', message: 'No issues found' }) + await context.messages.add({ id: 'scan', level: 'info', message: '2 issues found' }) + await context.messages.add({ id: 'scan', level: 'info', message: '3 issues found' }) + await new Promise(resolve => setTimeout(resolve, 30)) + expect(publishes).toBe(0) + + // A real dock change still publishes. + context.docks.register({ + type: 'iframe', + id: 'second', + title: 'Second', + icon: 'ph:cube-duotone', + url: '/__second/', + }) + await vi.waitFor(() => expect(publishes).toBeGreaterThan(0)) + expect(docks.value()).toHaveLength(2) + off() + }) +}) + describe('createHubContext dock activation', () => { it('mirrors an activation into shared state and broadcasts it live', async () => { const context = await createHubContext({ diff --git a/packages/hub/src/node/__tests__/host-messages.test.ts b/packages/hub/src/node/__tests__/host-messages.test.ts index bd2e9c65..c972d08b 100644 --- a/packages/hub/src/node/__tests__/host-messages.test.ts +++ b/packages/hub/src/node/__tests__/host-messages.test.ts @@ -21,6 +21,47 @@ describe('devframeMessagesHost', () => { expect(host.removals.at(-1)?.id).toBe('message:1004') }) + it('dedupes identical re-adds: no update event, no clock tick', async () => { + const host = new DevframeMessagesHost({} as DevframeHubContext) + const updates: string[] = [] + host.events.on('message:updated', entry => void updates.push(entry.id)) + + const input = { id: 'scan', level: 'info' as const, message: 'No issues found', labels: ['a11y'] } + await host.add(input) + const tickAfterAdd = host.lastModified.get('scan') + + // The periodic-producer pattern: the same entry mirrored again and again. + await host.add({ ...input }) + await host.add({ ...input, labels: ['a11y'] }) + expect(updates).toEqual([]) + expect(host.lastModified.get('scan')).toBe(tickAfterAdd) + + // A real change still updates and emits. + await host.add({ ...input, message: '2 issues found' }) + expect(updates).toEqual(['scan']) + expect(host.entries.get('scan')?.message).toBe('2 issues found') + expect(host.lastModified.get('scan')).not.toBe(tickAfterAdd) + }) + + it('an identical re-add carrying autoDelete still resets the keep-alive timer', async () => { + const host = new DevframeMessagesHost({} as DevframeHubContext) + const updates: string[] = [] + host.events.on('message:updated', entry => void updates.push(entry.id)) + + await host.add({ id: 'alive', level: 'info', message: 'still here', autoDelete: 50 }) + // Keep-alive re-adds: content identical, timer restarted each time. + for (let i = 0; i < 3; i++) { + await new Promise(resolve => setTimeout(resolve, 30)) + await host.add({ id: 'alive', level: 'info', message: 'still here', autoDelete: 50 }) + } + // 90ms elapsed — well past the 50ms window — yet the entry survives. + expect(host.entries.has('alive')).toBe(true) + expect(updates).toEqual([]) + // Once the re-adds stop, the timer finally fires. + await new Promise(resolve => setTimeout(resolve, 80)) + expect(host.entries.has('alive')).toBe(false) + }) + it('provides per-level shortcuts that delegate to add()', async () => { const host = new DevframeMessagesHost({} as DevframeHubContext) diff --git a/packages/hub/src/node/context.ts b/packages/hub/src/node/context.ts index 870ffed9..78a6faef 100644 --- a/packages/hub/src/node/context.ts +++ b/packages/hub/src/node/context.ts @@ -6,6 +6,7 @@ import type { DevframeMessageEntry, DevframeMessageEntryInput, DevframeMessagesH import type { DevframeTerminalsHost } from '../types/terminals' import type { InstallDevframeOptions } from './install-devframe' import { createHostContext } from 'devframe/node' +import { hash } from 'devframe/utils/hash' import { debounce } from 'perfect-debounce' import { DevframeCommandsHost as CommandsHostImpl } from './host-commands' import { DevframeDocksHost as DocksHostImpl } from './host-docks' @@ -151,11 +152,25 @@ export async function createHubContext(options: CreateHubContextOptions): Promis const debounceMs = options.mode === 'build' ? 0 : 10 const docksSharedState = await context.rpc.sharedState.get('devframe:docks', { initialValue: [] }) + // The docks state republishes on dock, terminal, *and* message events — + // most of which don't actually change the dock list. Publish only when + // the content really changed, so a chatty subsystem (e.g. a message feed + // being mirrored every scan) can't turn into a stream of identical + // `devframe:docks` broadcasts. + let publishedDocksHash: string | undefined + function publishDocks(): void { + const values = docks.values() + const digest = hash(values) + if (digest === publishedDocksHash) + return + publishedDocksHash = digest + docksSharedState.mutate(() => values) + } const refreshDocks = debounce(() => { - docksSharedState.mutate(() => docks.values()) + publishDocks() }, debounceMs) docks.events.on('dock:entry:updated', refreshDocks) - docksSharedState.mutate(() => docks.values()) + publishDocks() // Cross-iframe dock activation. A dock activation is a discrete user intent // ("go to Terminals now"), so it fires immediately (no debounce, which could @@ -181,7 +196,7 @@ export async function createHubContext(options: CreateHubContextOptions): Promis method: 'devframe:terminals:updated', args: [], }) - docksSharedState.mutate(() => docks.values()) + publishDocks() }, debounceMs) terminals.events.on('terminal:session:updated', broadcastTerminals) @@ -190,7 +205,7 @@ export async function createHubContext(options: CreateHubContextOptions): Promis method: 'devframe:messages:updated', args: [], }) - docksSharedState.mutate(() => docks.values()) + publishDocks() }, debounceMs) messages.events.on('message:added', broadcastMessages) messages.events.on('message:updated', broadcastMessages) diff --git a/packages/hub/src/node/host-messages.ts b/packages/hub/src/node/host-messages.ts index 519018bb..89a6bb32 100644 --- a/packages/hub/src/node/host-messages.ts +++ b/packages/hub/src/node/host-messages.ts @@ -8,6 +8,7 @@ import type { } from '../types/messages' import type { DevframeHubContext } from './context' import { createEventEmitter } from 'devframe/utils/events' +import { hash } from 'devframe/utils/hash' import { nanoid } from 'devframe/utils/nanoid' const MAX_ENTRIES = 1000 @@ -93,9 +94,17 @@ export class DevframeMessagesHost implements DevframeMessagesHostType { timestamp: existing.timestamp, } - this.entries.set(id, updated) - this.lastModified.set(id, this._tick()) - this.events.emit('message:updated', updated) + // Content dedupe: a re-add/update that changes nothing (a periodic + // producer mirroring the same entries — e.g. a scanner re-reporting an + // unchanged result) emits no event, so it can't fan out into broadcast + // and shared-state churn. An identical patch that carries `autoDelete` + // still falls through below to reset the keep-alive timer. + const unchanged = hash(updated) === hash(existing) + if (!unchanged) { + this.entries.set(id, updated) + this.lastModified.set(id, this._tick()) + this.events.emit('message:updated', updated) + } // Reset autoDelete timer if changed if (patch.autoDelete !== undefined) { diff --git a/plugins/a11y/src/inject/index.ts b/plugins/a11y/src/inject/index.ts index b87bdcf6..7932366b 100644 --- a/plugins/a11y/src/inject/index.ts +++ b/plugins/a11y/src/inject/index.ts @@ -126,7 +126,12 @@ function start(context?: A11yAgentContext) { function scheduleScan() { clearTimeout(debounceTimer) - debounceTimer = window.setTimeout(runScan, 600) + // Observer/interaction-driven rescans are background refreshes: they + // update the report silently instead of flipping the messages-feed + // summary through loading → idle — that status churn is itself a page + // mutation (the host re-renders its feed), which would re-trigger the + // observer and turn the scan into a self-sustaining loop. + debounceTimer = window.setTimeout(() => void runScan({ background: true }), 600) } // Interaction-driven rescans, layered on top of the DOM observer. Bound only @@ -176,7 +181,7 @@ function start(context?: A11yAgentContext) { console.groupEnd() } - async function runScan() { + async function runScan(options: { background?: boolean } = {}) { if (scanning) { rescanQueued = true return @@ -184,7 +189,8 @@ function start(context?: A11yAgentContext) { scanning = true activeRoute = location.pathname post({ type: 'a11y:scanning', route: activeRoute }) - reporter?.scanning() + if (!options.background) + reporter?.scanning() // Suspend observation so attribute-stamping during the scan doesn't // retrigger us. observer.disconnect() diff --git a/plugins/a11y/src/inject/messages.ts b/plugins/a11y/src/inject/messages.ts index dae1832b..eb70ffad 100644 --- a/plugins/a11y/src/inject/messages.ts +++ b/plugins/a11y/src/inject/messages.ts @@ -113,9 +113,23 @@ export function createMessagesReporter( let reportedRules = new Set() // Fire-and-forget: the feed is a mirror, never a gate for the scan loop. // Every entry is grouped under the short `a11y` category. - const send = (input: HubMessageInput) => + // + // Re-scans routinely produce the identical report; sending it again would + // be one wire message per entry for zero feed change (and the feed + // re-render it causes on the host page can re-trigger the DOM observer — + // a scan loop). Remember what each entry last carried and send only diffs. + const lastSent = new Map() + const send = (input: HubMessageInput & { id: string }) => { + const digest = JSON.stringify(input) + if (lastSent.get(input.id) === digest) + return + lastSent.set(input.id, digest) void messages.add({ category: MESSAGE_CATEGORY, ...input }).catch(() => {}) - const drop = (id: string) => void messages.remove(id).catch(() => {}) + } + const drop = (id: string) => { + lastSent.delete(id) + void messages.remove(id).catch(() => {}) + } const dockId = () => options.dockId?.() ?? A11Y_DEFAULT_DOCK_ID return { diff --git a/plugins/a11y/tests/inject-messages.test.ts b/plugins/a11y/tests/inject-messages.test.ts index eaa5d585..cf2d5776 100644 --- a/plugins/a11y/tests/inject-messages.test.ts +++ b/plugins/a11y/tests/inject-messages.test.ts @@ -147,8 +147,31 @@ describe('createMessagesReporter', () => { reporter.report(report([violation('label', 'minor')])) expect(removed).toEqual(['devframes:plugin:a11y:rule:image-alt']) - // The surviving rule was re-added (dedup by stable id updates in place). - expect(added.filter(m => m.id === 'devframes:plugin:a11y:rule:label')).toHaveLength(2) + // The surviving rule's content is unchanged — the reporter skips the + // redundant re-send instead of mirroring an identical entry every scan. + expect(added.filter(m => m.id === 'devframes:plugin:a11y:rule:label')).toHaveLength(1) + }) + + it('skips re-sending unchanged entries across scans, but sends real changes', () => { + const { client, added } = createStubMessages() + const reporter = createMessagesReporter(client) + + reporter.report(report([violation('image-alt', 'critical')])) + reporter.report(report([violation('image-alt', 'critical')])) + reporter.report(report([violation('image-alt', 'critical')])) + // One summary + one rule entry, once — identical re-scans add nothing. + expect(added).toHaveLength(2) + + // A dropped rule changes the summary (and removes the rule entry). + reporter.report(report([])) + const summaries = added.filter(m => m.id === 'devframes:plugin:a11y:scan') + expect(summaries).toHaveLength(2) + expect(summaries.at(-1)?.message).toBe('No accessibility issues found') + + // A rule that comes back is sent again (its `lastSent` slot was dropped + // with the removal). + reporter.report(report([violation('image-alt', 'critical')])) + expect(added.filter(m => m.id === 'devframes:plugin:a11y:rule:image-alt')).toHaveLength(2) }) it('settles the summary entry as an error when a scan fails', () => {