From 7783fcd0cea3e589b05473eececf8e6ec2b2babf Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sat, 24 Oct 2026 14:15:22 +0200 Subject: [PATCH] fix: apply the review items for #692 at merge host-sensors: never await the PowerShell RAM probe on a read; keep the resolved total and show the memory in use alone until it answers (test with a held PowerShell) test: rename the Windows sampler test that said the GPU counters are added "for good" test: make host-sensors.test.ts's @fileoverview describe what that file covers host-sensors: make the nvidiaSkipUntil JSDoc describe the back-off deadline docs: Settings Reference says the host readings live inside System Stats and nothing is fetched while it is off Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/wiki/Settings-Reference.md | 3 +- src/host-sensors.ts | 34 +++++++++--------- test/host-sensors-readers.test.ts | 57 +++++++++++++++++++++++++++++-- test/host-sensors.test.ts | 9 +++-- 4 files changed, 81 insertions(+), 22 deletions(-) diff --git a/docs/wiki/Settings-Reference.md b/docs/wiki/Settings-Reference.md index 612e5fd1..8e2fb62f 100644 --- a/docs/wiki/Settings-Reference.md +++ b/docs/wiki/Settings-Reference.md @@ -66,7 +66,8 @@ Project Insights, File Browser, Subagents, Approvals Inbox, Read My Mind, Ultrac Ultracode Windows, Cron. **Host readings** (opt-in, both off by default and per device): **GPU Stats** adds three items after CPU and MEM: GPU load, graphics memory and the GPU clock, -and **CPU MHz** adds the live CPU clock. An item appears only when the +and **CPU MHz** adds the live CPU clock. Both sets of items live inside **System Stats**, so they show only +while that chip is on, and nothing is fetched while it is off. An item appears only when the host reports it, so a machine with no readable GPU shows nothing extra. Sources: `nvidia-smi` or the amdgpu sysfs files for the GPU; inside WSL, the Windows performance counters through `typeperf.exe`, which cover any vendor (an Intel GPU gives load and memory in use, but Windows publishes no clock for it, and its memory diff --git a/src/host-sensors.ts b/src/host-sensors.ts index c2128de3..a972be07 100644 --- a/src/host-sensors.ts +++ b/src/host-sensors.ts @@ -194,28 +194,29 @@ function inWsl(): boolean { return wsl; } -let windowsRamMB: Promise | undefined; +let windowsRamMB: number | undefined; +let ramProbe: Promise | undefined; let ramRetryAt = 0; /** - * Windows' installed RAM, asked once (PowerShell takes about a second) and remembered when it answers. A - * failure is not retried on every GPU poll (8 s timeout each): it waits READER_BACKOFF_MS first. + * Windows' installed RAM in MB once PowerShell has answered, else undefined. The probe (about a second, 8 s + * timeout) is started here but never awaited: the stats route answers CPU and MEM only after the sensors, so + * the GPU reading carries the memory in use alone until the answer lands, and the total from then on. A + * failure is not retried on every GPU poll: it waits READER_BACKOFF_MS first. */ -function readWindowsRamMB(): Promise { - if (!windowsRamMB && Date.now() < ramRetryAt) return Promise.resolve(undefined); - windowsRamMB ??= run( +function windowsRamMBIfKnown(): number | undefined { + if (windowsRamMB !== undefined || ramProbe || Date.now() < ramRetryAt) return windowsRamMB; + ramProbe = run( POWERSHELL, ['-NoProfile', '-NonInteractive', '-Command', '(Get-CimInstance Win32_ComputerSystem).TotalPhysicalMemory'], 8000 ).then((out) => { const bytes = out ? finite(Number(out.trim())) : undefined; - if (!bytes || bytes <= 0) { - windowsRamMB = undefined; - ramRetryAt = Date.now() + READER_BACKOFF_MS; - } - return bytes && bytes > 0 ? Math.round(bytes / MB) : undefined; + if (bytes && bytes > 0) windowsRamMB = Math.round(bytes / MB); + else ramRetryAt = Date.now() + READER_BACKOFF_MS; + ramProbe = undefined; }); - return windowsRamMB; + return undefined; } /** What the sampler needs from a child process; `ChildProcess` fits, and tests pass a fake. */ @@ -423,7 +424,7 @@ async function readAmdGpu(): Promise { return undefined; } -/** NVIDIA, then AMD: the readers that need nothing from Windows. */ +/** nvidia-smi is not run again before this time: it failed or timed out, and waits READER_BACKOFF_MS. */ let nvidiaSkipUntil = 0; /** NVIDIA, then AMD. A host without a working nvidia-smi is not asked again on every poll. */ @@ -441,12 +442,13 @@ async function readNativeGpu(): Promise { } /** A GPU reading from the Windows counters (WSL only): load and memory in use, but no clock. */ -async function gpuFromWindows(windows: WindowsCounters | undefined): Promise { +function gpuFromWindows(windows: WindowsCounters | undefined): GpuReading | undefined { if (!windows?.gpu) return undefined; const { percent, memUsedMB, dedicated } = windows.gpu; const reading: GpuReading = { percent, memUsedMB }; - // An integrated GPU draws its memory from system RAM, and Windows lets it use up to half of it. - const ram = dedicated ? undefined : await readWindowsRamMB(); + // An integrated GPU draws its memory from system RAM, and Windows lets it use up to half of it. Until + // Windows has said how much RAM it has, the reading is the memory in use alone (the header shows no ring). + const ram = dedicated ? undefined : windowsRamMBIfKnown(); if (ram) { reading.memTotalMB = Math.round(ram / 2); reading.memPercent = clampPercent((memUsedMB / reading.memTotalMB) * 100); diff --git a/test/host-sensors-readers.test.ts b/test/host-sensors-readers.test.ts index 8030a77b..ed1fb79c 100644 --- a/test/host-sensors-readers.test.ts +++ b/test/host-sensors-readers.test.ts @@ -1,8 +1,8 @@ /** * The order and the back-offs of readHostSensors, with `child_process` and the sysfs reads mocked so no * hardware is needed: native GPU readers go first (a machine with a working nvidia-smi never starts the - * Windows GPU counters), a failing reader is left alone for a minute, a cold Windows sampler is not - * waited for, and the GPU counters leave the sampler again once nothing needs them. + * Windows GPU counters), a failing reader is left alone for a minute, neither a cold Windows sampler nor + * the PowerShell RAM probe is waited for, and the GPU counters leave the sampler again once nothing needs them. */ import { EventEmitter } from 'node:events'; import { PassThrough } from 'node:stream'; @@ -12,6 +12,9 @@ type ExecCall = { cmd: string; args: string[] }; const state = vi.hoisted(() => ({ nvidia: '' as string | Error, powershell: '' as string | Error, + /** When set, PowerShell does not answer until the test calls the callbacks kept here. */ + holdPowershell: false, + heldPowershell: [] as Array<() => void>, execCalls: [] as Array<{ cmd: string; args: string[] }>, spawnCalls: [] as Array<{ cmd: string; args: string[]; stdout: unknown }>, })); @@ -19,6 +22,14 @@ const state = vi.hoisted(() => ({ vi.mock('node:child_process', () => ({ execFile: (cmd: string, args: string[], _opts: unknown, cb: (err: Error | null, stdout: string) => void) => { state.execCalls.push({ cmd, args }); + if (state.holdPowershell && cmd.endsWith('powershell.exe')) { + state.heldPowershell.push(() => { + const held = state.powershell; + if (held instanceof Error) cb(held, ''); + else cb(null, held); + }); + return; + } const answer = cmd === 'nvidia-smi' ? state.nvidia : cmd.endsWith('powershell.exe') ? state.powershell : ''; if (answer instanceof Error) cb(answer, ''); else cb(null, answer); @@ -56,6 +67,8 @@ const tick = () => new Promise((r) => setImmediate(r)); beforeEach(() => { state.nvidia = NVIDIA; state.powershell = new Error('powershell failed'); + state.holdPowershell = false; + state.heldPowershell = []; state.execCalls = []; state.spawnCalls = []; process.env.WSL_INTEROP = '1'; @@ -138,6 +151,46 @@ describe('a failing reader is left alone for a minute', () => { }); }); +describe('the PowerShell RAM probe', () => { + it('is never waited for: the memory in use shows at once, and the total once Windows has answered', async () => { + state.nvidia = new Error('no nvidia'); + state.holdPowershell = true; + const { readHostSensors, stopHostSensors } = await load(); + await readHostSensors({ gpu: true }); // starts the sampler + (state.spawnCalls[0].stdout as PassThrough).write(HEADER + ROW); + await tick(); + vi.setSystemTime(Date.now() + 2500); + // PowerShell has not answered (and never will unless the test lets it): the read must not wait for it + const first = await Promise.race([ + readHostSensors({ gpu: true }), + new Promise<'waited'>((r) => setTimeout(() => r('waited'), 1000)), + ]); + expect(first).not.toBe('waited'); + expect((first as { gpu?: object }).gpu).toEqual({ percent: 20, memUsedMB: 1024 }); + expect(counted('powershell.exe')).toBe(1); + // a poll while the probe is still running starts no second PowerShell + vi.setSystemTime(Date.now() + 2500); + (state.spawnCalls[0].stdout as PassThrough).write(ROW); + await tick(); + expect((await readHostSensors({ gpu: true })).gpu?.memTotalMB).toBeUndefined(); + expect(counted('powershell.exe')).toBe(1); + // Windows answers 16 GiB: an integrated GPU may use half of it + state.powershell = '17179869184\r\n'; + state.heldPowershell.forEach((answer) => answer()); + await tick(); + for (let i = 0; i < 2; i++) { + vi.setSystemTime(Date.now() + 2500); + (state.spawnCalls[0].stdout as PassThrough).write(ROW); + await tick(); + const out = await readHostSensors({ gpu: true }); + expect(out.gpu).toEqual({ percent: 20, memUsedMB: 1024, memTotalMB: 8192, memPercent: 13 }); + } + // the answer is kept: PowerShell is not asked again + expect(counted('powershell.exe')).toBe(1); + stopHostSensors(); + }); +}); + describe('the GPU counters do not stay in the sampler for good', () => { it('holds them for a while after the last read that needed them, then drops them', async () => { const { createWindowsSource, windowsCounterList } = await load(); diff --git a/test/host-sensors.test.ts b/test/host-sensors.test.ts index e6733e58..5bcdacba 100644 --- a/test/host-sensors.test.ts +++ b/test/host-sensors.test.ts @@ -1,6 +1,9 @@ /** - * @fileoverview The pure parsers behind the opt-in header readings (GPU, CPU clock), the route's - * opt-in query handling, and the header display's defaults. No hardware needed. + * @fileoverview The pure parsers behind the opt-in header readings (nvidia-smi, the amdgpu clock table, + * cpufreq and /proc/cpuinfo, typeperf), readHostSensors reading nothing unless a reading is asked for, the + * typeperf counter stream's life cycle (first sample, recycling, idle stop, back-off after a failed start, + * no wait on a cold sampler by default, a late row from a replaced sampler), and the Windows sampler that + * every browser shares. No hardware needed. */ import { EventEmitter } from 'node:events'; import { PassThrough } from 'node:stream'; @@ -233,7 +236,7 @@ describe('the Windows sampler shared by every browser', () => { expect(children[0].killed).toBe(false); }); - it('adds the GPU counters once, for good, on a machine with no native GPU reader', async () => { + it('adds the GPU counters with one restart on a machine with no native GPU reader, and keeps them while needed', async () => { const { children, source } = rig(); void source.read(); expect(children[0].counters).toEqual(windowsCounterList(false));