From 2c25e512a794d88eda9dd71f7364b7d114b28dfc Mon Sep 17 00:00:00 2001 From: archipelago Date: Fri, 31 Jul 2026 09:00:32 -0400 Subject: [PATCH] fix(02-11): gate three leaked background pollers to activate/deactivate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fleet.vue's useFleetData() (60s telemetry.fleet-status/-alerts poll), Server.vue's FipsNetworkCard.vue (15s fips.status poll), and Web5.vue's Web5Monitoring.vue (30s system.stats poll — redundant with Home.vue's own correctly-gated 10s poll of the same store) all armed their setInterval in onMounted and only disarmed it in onUnmounted/ onBeforeUnmount. That was harmless before 02-04 registered their owning views in KEEP_ALIVE_PATHS (the view was destroyed on every tab-away, so the teardown hook fired every time); once KeepAlive keeps the instance alive, the teardown hook never fires again and the poll ran forever in the background regardless of which dashboard tab was showing. Gated arm/disarm to onActivated/onDeactivated, mirroring Server.vue's own vpnPollInterval fix from 02-04 exactly. Added regression tests to keepAliveLifecycle.test.ts mounting each real component under a synthetic KeepAlive with fake timers; confirmed RED against the pre-fix code (git stash) before confirming GREEN with the fix restored. Full suite (95 files/788 tests), type-check and build all green. keepAliveTabs.test.ts is byte-for-byte unmodified. Co-Authored-By: Claude Opus 5 (1M context) --- .../phases/02-ui-performance/02-FINDINGS.md | 31 +++ .../__tests__/keepAliveLifecycle.test.ts | 198 ++++++++++++++++++ neode-ui/src/views/fleet/useFleetData.ts | 37 +++- neode-ui/src/views/server/FipsNetworkCard.vue | 34 ++- neode-ui/src/views/web5/Web5Monitoring.vue | 37 +++- 5 files changed, 326 insertions(+), 11 deletions(-) diff --git a/.planning/phases/02-ui-performance/02-FINDINGS.md b/.planning/phases/02-ui-performance/02-FINDINGS.md index 4fea8513..363fd0a5 100644 --- a/.planning/phases/02-ui-performance/02-FINDINGS.md +++ b/.planning/phases/02-ui-performance/02-FINDINGS.md @@ -1042,3 +1042,34 @@ onMounted and onActivated" rule (a KeepAlive-wrapped first mount fires both once guard is not needed here since the actions themselves — arming a 15/30/60s interval — are cheap and idempotent, unlike Home/Web5/Mesh/Server's heavier first-load RPC bursts that needed a guard to avoid double-firing). + +### Task 2 outcome — landed + +- **`neode-ui/src/views/fleet/useFleetData.ts`**: `armFleetPoll()`/`disarmFleetPoll()` wrap + `startAutoRefresh()`/`stopAutoRefresh()`, called from `onMounted`+`onActivated`/`onDeactivated` + respectively. `toggleAutoRefresh()` (the user's own Pause/Resume button) is untouched — a + deactivated-then-reactivated Fleet still respects the user's own pause choice, since + `armFleetPoll()` only restarts the timer `if (autoRefresh.value)`. +- **`neode-ui/src/views/server/FipsNetworkCard.vue`**: `armFipsPoll()`/`disarmFipsPoll()` wrap the + 15s `statusRes.refresh()` poll the same way. `useCachedResource`'s own `onActivated` revalidation + (already present, unaffected) still covers the staleness-gated background refresh independently. +- **`neode-ui/src/views/web5/Web5Monitoring.vue`**: `armWeb5MonitoringPoll()`/ + `disarmWeb5MonitoringPoll()` wrap the 30s `homeStatus.refreshSystemStats()` poll the same way. + +Regression tests added to `keepAliveLifecycle.test.ts`'s new `02-11 gap closure` describe block — +mounting each REAL component (Fleet.vue, FipsNetworkCard.vue, Web5Monitoring.vue) inside a +synthetic ``, with fake timers: deactivate -> advance 3x the poll interval -> assert +zero new calls; reactivate -> assert new calls resume. **RED confirmed before GREEN, not assumed**: +ran all three new tests against the pre-fix code (`git stash push` on the three source files, +keeping the new tests) — all three failed exactly as expected (Fleet: 8 calls vs the expected 2; +FipsNetworkCard: 2 vs 1; Web5Monitoring: 4 vs 1 — every failure in the direction of "kept firing +while deactivated"), then `git stash pop` restored the fix and all three passed. Full suite (95 +files / 788 tests), `vue-tsc --noEmit`, and `npm run build` (43.93s, not a cached no-op — the log +shows real transform/collect work and a fresh `sw.js`/precache manifest) all green. +`keepAliveTabs.test.ts` confirmed byte-for-byte unmodified (`git diff --stat` empty) and still +passing (part of the full-suite run). + +**No fix attempted for Discover/AppDetails' own surface-specific cost** (there is none identified — +their navSteps-transit and session-wide-contention explanations above are the standing account) — +Task 3's re-measure is the test of whether removing the three leaked pollers alone is enough to move +their numbers, or whether they need their own follow-up. diff --git a/neode-ui/src/views/dashboard/__tests__/keepAliveLifecycle.test.ts b/neode-ui/src/views/dashboard/__tests__/keepAliveLifecycle.test.ts index 0ec9942d..e796355f 100644 --- a/neode-ui/src/views/dashboard/__tests__/keepAliveLifecycle.test.ts +++ b/neode-ui/src/views/dashboard/__tests__/keepAliveLifecycle.test.ts @@ -21,6 +21,9 @@ import { keepAliveIncludeNames, KEEP_WRAP_PREFIX } from '../dashboardViewWrapper import { rpcClient } from '@/api/rpc-client' import Server from '@/views/Server.vue' import Web5 from '@/views/web5/Web5.vue' +import Fleet from '@/views/Fleet.vue' +import FipsNetworkCard from '@/views/server/FipsNetworkCard.vue' +import Web5Monitoring from '@/views/web5/Web5Monitoring.vue' // Hoisted by vitest — must live at module scope, not inside a test body, or // Server.vue's/Web5.vue's own module-level `import { rpcClient } from @@ -408,6 +411,201 @@ describe('keepAliveLifecycle: real converted view (Server.vue vpnPollInterval)', }) }) +// 02-11 gap closure: three child components/composables inside the +// KeepAlive'd Fleet/Server/Web5 subtrees armed a setInterval poll in +// onMounted and only ever disarmed it in onUnmounted/onBeforeUnmount — a +// no-op before 02-04's KeepAlive registration (the owning view was +// destroyed on every tab-away, so onUnmounted fired every time) and a +// permanent, session-long background-RPC leak afterward, invisible to +// 02-04's own audit because it grepped each top-level view file, not the +// composables/child components it delegates to. These pin the fix: no +// poll activity while deactivated, resumed on reactivation — the exact +// contract `Server.vue vpnPollInterval` above already proves for the view +// files 02-04 did audit directly. +describe('keepAliveLifecycle: 02-11 gap closure — leaked background pollers in un-audited child composables', () => { + beforeEach(() => { + vi.useFakeTimers() + }) + afterEach(() => { + vi.useRealTimers() + }) + + it("Fleet.vue's useFleetData() poll (telemetry.fleet-status/-alerts) is not invoked while deactivated, and resumes on reactivation", async () => { + const Other = defineComponent({ render: () => h('div', 'other') }) + const Host = defineComponent({ + setup() { + const show = ref(true) + return { show } + }, + render() { + return h(KeepAlive, null, () => + this.show ? h(Fleet, { key: 'fleet' }) : h(Other, { key: 'other' }), + ) + }, + }) + + const wrapper = mount(Host, { + global: { + plugins: [createPinia()], + stubs: { + FleetOverviewCards: true, + FleetNodeGrid: true, + FleetAlerts: true, + FleetNodeDetail: true, + FleetContainerMatrix: true, + BackButton: true, + }, + }, + }) + await flushPromises() + + const callMock = vi.mocked(rpcClient.call) + const fleetCallCount = () => + callMock.mock.calls.filter(([arg]) => (arg as { method?: string }).method?.startsWith('telemetry.fleet-')).length + + const callsAtActivation = fleetCallCount() + expect(callsAtActivation).toBeGreaterThan(0) // refreshAll() on the initial mount + + // Deactivate — the 60s poll must not keep firing while off screen. + ;(wrapper.vm as unknown as { show: boolean }).show = false + await wrapper.vm.$nextTick() + vi.advanceTimersByTime(180_000) // 3x the poll interval + await flushPromises() + expect(fleetCallCount()).toBe(callsAtActivation) + + // Reactivate — armFleetPoll() re-arms the interval; no call is expected + // synchronously on activation itself (unlike Server's vpnPollInterval, + // this composable's poll has no "immediate tick" — it only ticks on the + // next 60s boundary), but a tick after reactivation must fire again. + ;(wrapper.vm as unknown as { show: boolean }).show = true + await wrapper.vm.$nextTick() + await flushPromises() + vi.advanceTimersByTime(60_000) + await flushPromises() + expect(fleetCallCount()).toBeGreaterThan(callsAtActivation) + + wrapper.unmount() + }) + + it("FipsNetworkCard.vue's 15s fips.status poll is not invoked while deactivated, and resumes on reactivation", async () => { + const Other = defineComponent({ render: () => h('div', 'other') }) + const Host = defineComponent({ + setup() { + const show = ref(true) + return { show } + }, + render() { + return h(KeepAlive, null, () => + this.show ? h(FipsNetworkCard, { key: 'fips' }) : h(Other, { key: 'other' }), + ) + }, + }) + + const wrapper = mount(Host, { global: { plugins: [createPinia()] } }) + await flushPromises() + + const callMock = vi.mocked(rpcClient.call) + const fipsCallCount = () => + callMock.mock.calls.filter(([arg]) => (arg as { method?: string }).method === 'fips.status').length + + const callsAtActivation = fipsCallCount() + expect(callsAtActivation).toBeGreaterThan(0) // useCachedResource's own immediate fetch on mount + + // Deactivate — the manual 15s poll must not keep calling fips.status + // while off screen, regardless of how long the deactivation lasts. + ;(wrapper.vm as unknown as { show: boolean }).show = false + await wrapper.vm.$nextTick() + vi.advanceTimersByTime(45_000) // 3x the poll interval + await flushPromises() + expect(fipsCallCount()).toBe(callsAtActivation) + + // Reactivate — armFipsPoll's immediate setInterval arm plus + // useCachedResource's own onActivated revalidation (stale past its 15s + // TTL) both contribute; the decisive assertion is simply "more than + // zero new calls after reactivation", not an exact count, since the two + // effects' relative ordering is an implementation detail. + ;(wrapper.vm as unknown as { show: boolean }).show = true + await wrapper.vm.$nextTick() + await flushPromises() + expect(fipsCallCount()).toBeGreaterThan(callsAtActivation) + + wrapper.unmount() + }) + + it("Web5Monitoring.vue's 30s system.stats poll is not invoked while deactivated, and resumes on reactivation", async () => { + const Other = defineComponent({ render: () => h('div', 'other') }) + const Host = defineComponent({ + setup() { + const show = ref(true) + return { show } + }, + render() { + return h(KeepAlive, null, () => + this.show ? h(Web5Monitoring, { key: 'web5mon' }) : h(Other, { key: 'other' }), + ) + }, + }) + + // homeStatus.ts's refreshSystemStats() assigns numeric fields straight + // from the RPC response (e.g. `stats.cpuPercent = res.cpu_usage_percent`) + // with no fallback — the file-wide default `call` mock resolving `{}` + // leaves `cpuPercent` as `undefined`, and the template's `.toFixed(0)` + // throws. Scoped to this test only (restored to the file-wide default in + // reverse order after) so no other test's call-count assertions shift. + const callMock = vi.mocked(rpcClient.call) + const priorImpl = callMock.getMockImplementation() + callMock.mockImplementation(async (arg: unknown) => { + const method = (arg as { method?: string }).method + if (method === 'system.stats') { + return { + cpu_usage_percent: 12, + mem_used_bytes: 1, + mem_total_bytes: 2, + disk_used_bytes: 1, + disk_total_bytes: 2, + uptime_secs: 100, + } + } + return {} + }) + + const wrapper = mount(Host, { + global: { + plugins: [createPinia()], + stubs: { RouterLink: true }, + }, + }) + await flushPromises() + const statsCallCount = () => + callMock.mock.calls.filter(([arg]) => (arg as { method?: string }).method === 'system.stats').length + + const callsAtActivation = statsCallCount() + expect(callsAtActivation).toBeGreaterThan(0) // loadStats() on the initial mount + + // Deactivate — the 30s poll must not keep calling system.stats while + // off screen (previously ran forever once Web5 was first visited). + ;(wrapper.vm as unknown as { show: boolean }).show = false + await wrapper.vm.$nextTick() + vi.advanceTimersByTime(90_000) // 3x the poll interval + await flushPromises() + expect(statsCallCount()).toBe(callsAtActivation) + + // Reactivate — armWeb5MonitoringPoll() calls loadStats() immediately. + ;(wrapper.vm as unknown as { show: boolean }).show = true + await wrapper.vm.$nextTick() + await flushPromises() + expect(statsCallCount()).toBeGreaterThan(callsAtActivation) + + wrapper.unmount() + // Restore the file-wide default so later tests' `call` behavior is + // unaffected — never `mockReset()`, which would also wipe the queued + // implementation the top-level `vi.mock('@/api/rpc-client', ...)` + // factory set up, leaving `call()` return `undefined` instead of a + // resolved promise for every test that runs after this one. + callMock.mockImplementation(priorImpl ?? (async () => ({}))) + }) +}) + describe('keepAliveRoutes: widened registration set (02-04)', () => { it('shouldKeepAlive returns true for every registered main-tab path and false for every detail path, including detail paths whose prefix matches a registered path', () => { expect(shouldKeepAlive({ path: '/dashboard/apps' })).toBe(true) diff --git a/neode-ui/src/views/fleet/useFleetData.ts b/neode-ui/src/views/fleet/useFleetData.ts index 4a2df4f7..b60e3349 100644 --- a/neode-ui/src/views/fleet/useFleetData.ts +++ b/neode-ui/src/views/fleet/useFleetData.ts @@ -1,6 +1,6 @@ /** Composable encapsulating fleet telemetry data fetching, types, and utilities */ -import { ref, computed, onMounted, onUnmounted, watch } from 'vue' +import { ref, computed, onMounted, onUnmounted, onActivated, onDeactivated, watch } from 'vue' import { rpcClient } from '@/api/rpc-client' import type { ChartDataset } from '@/components/LineChart.vue' @@ -480,17 +480,44 @@ export function useFleetData() { // --- Lifecycle --- + // 02-11 gap closure: `startAutoRefresh()`'s 60s poll used to be armed only + // in onMounted and disarmed only in onUnmounted — harmless before Fleet + // joined KEEP_ALIVE_PATHS (02-04), since leaving the tab fully unmounted + // this composable's owning component and onUnmounted fired every time. + // Once Fleet is KeepAlive'd, onUnmounted never fires again after the first + // visit, so the poll ran forever in the background regardless of tab + // visibility — the exact off-screen-drain class 02-04's own audit + // convention exists to prevent, missed here because that audit grepped + // Fleet.vue itself (which has no lifecycle side effects of its own) and + // never extended to this composable it delegates to. Arm/disarm now + // follows activate/deactivate, mirroring Server.vue's vpnPollInterval + // fix from 02-04; `startAutoRefresh()` already clears any existing timer + // first, so calling it from both onMounted and onActivated on a fresh + // KeepAlive-wrapped mount is safe and idempotent. + function armFleetPoll() { + if (autoRefresh.value) startAutoRefresh() + } + function disarmFleetPoll() { + stopAutoRefresh() + } + onMounted(async () => { updateChartWidth() window.addEventListener('resize', updateChartWidth) await refreshAll() - if (autoRefresh.value) { - startAutoRefresh() - } + armFleetPoll() + }) + + onActivated(() => { + armFleetPoll() + }) + + onDeactivated(() => { + disarmFleetPoll() }) onUnmounted(() => { - stopAutoRefresh() + disarmFleetPoll() window.removeEventListener('resize', updateChartWidth) }) diff --git a/neode-ui/src/views/server/FipsNetworkCard.vue b/neode-ui/src/views/server/FipsNetworkCard.vue index b36d5dce..03f3b555 100644 --- a/neode-ui/src/views/server/FipsNetworkCard.vue +++ b/neode-ui/src/views/server/FipsNetworkCard.vue @@ -119,7 +119,7 @@ diff --git a/neode-ui/src/views/web5/Web5Monitoring.vue b/neode-ui/src/views/web5/Web5Monitoring.vue index bd0c7447..3c225041 100644 --- a/neode-ui/src/views/web5/Web5Monitoring.vue +++ b/neode-ui/src/views/web5/Web5Monitoring.vue @@ -72,7 +72,7 @@