From 8706b2b765790e4da314e58dbd479c2fef664d30 Mon Sep 17 00:00:00 2001 From: Brad Groux <3053586+BradGroux@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:18:29 -0500 Subject: [PATCH] fix: classify memory pressure against real limits (#1316) --- .../__tests__/memory-health-feature.test.ts | 176 ++++++++++++++++++ .../__tests__/system-health-service.test.ts | 14 +- server/src/routes/health.ts | 44 ++--- server/src/services/system-health-service.ts | 20 +- server/src/utils/memory-pressure.ts | 72 +++++++ shared/src/types/system-health.types.ts | 21 +++ .../__tests__/system-health-banner.test.tsx | 69 +++++++ web/src/hooks/useSystemHealth.ts | 2 + 8 files changed, 381 insertions(+), 37 deletions(-) create mode 100644 server/src/__tests__/memory-health-feature.test.ts create mode 100644 server/src/utils/memory-pressure.ts create mode 100644 web/src/__tests__/system-health-banner.test.tsx diff --git a/server/src/__tests__/memory-health-feature.test.ts b/server/src/__tests__/memory-health-feature.test.ts new file mode 100644 index 00000000..74b3ce53 --- /dev/null +++ b/server/src/__tests__/memory-health-feature.test.ts @@ -0,0 +1,176 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import express from 'express'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import request from 'supertest'; +import type { MemoryPressureDiagnostics } from '@veritas-kanban/shared'; + +const { mockGetMemoryPressure, mockRegistry, mockMetrics } = vi.hoisted(() => ({ + mockGetMemoryPressure: vi.fn(), + mockRegistry: { list: vi.fn() }, + mockMetrics: { getRunMetrics: vi.fn() }, +})); + +vi.mock('../utils/memory-pressure.js', async (importOriginal) => ({ + ...(await importOriginal()), + getMemoryPressure: mockGetMemoryPressure, +})); + +vi.mock('../services/agent-registry-service.js', () => ({ + getAgentRegistryService: () => mockRegistry, +})); + +vi.mock('../services/metrics/index.js', () => ({ + getMetricsService: () => mockMetrics, +})); + +import { classifyMemoryPressure } from '../utils/memory-pressure.js'; +import { healthRouter } from '../routes/health.js'; +import { systemHealthRouter } from '../routes/system-health.js'; +import { errorHandler } from '../middleware/error-handler.js'; + +const healthyPressure = classifyMemoryPressure({ + heapUsedBytes: 95, + heapAllocatedBytes: 100, + heapLimitBytes: 1_000, + rssBytes: 100, + externalBytes: 5, + effectiveMemoryLimitBytes: 2_000, + effectiveMemoryLimitSource: 'process-constrained', +}); + +const warningPressure = classifyMemoryPressure({ + heapUsedBytes: 950, + heapAllocatedBytes: 975, + heapLimitBytes: 1_000, + rssBytes: 100, + externalBytes: 5, + effectiveMemoryLimitBytes: 2_000, + effectiveMemoryLimitSource: 'process-constrained', +}); + +describe('memory health feature', () => { + let app: express.Express; + let dataDir: string; + const originalEnv = { + dataDir: process.env.DATA_DIR, + storage: process.env.VERITAS_STORAGE, + adminKey: process.env.VERITAS_ADMIN_KEY, + authEnabled: process.env.VERITAS_AUTH_ENABLED, + nodeEnv: process.env.NODE_ENV, + }; + + beforeEach(async () => { + dataDir = await fs.mkdtemp(path.join(os.tmpdir(), 'memory-health-feature-')); + const runtimeDir = path.join(dataDir, '.veritas-kanban'); + await fs.mkdir(runtimeDir, { recursive: true }); + await fs.writeFile(path.join(runtimeDir, 'tasks.json'), '[]'); + + process.env.DATA_DIR = dataDir; + process.env.VERITAS_STORAGE = 'file'; + process.env.VERITAS_ADMIN_KEY = 'test-admin-key-for-memory-health-feature'; + process.env.VERITAS_AUTH_ENABLED = 'true'; + process.env.NODE_ENV = 'development'; + + mockRegistry.list.mockReturnValue([]); + mockMetrics.getRunMetrics.mockResolvedValue({ + runs: 0, + successRate: 1, + failures: 0, + errors: 0, + }); + mockGetMemoryPressure.mockReturnValue(healthyPressure); + + app = express(); + app.use(express.json()); + app.use('/api/v1/system/health', systemHealthRouter); + app.use('/health', healthRouter); + app.use(errorHandler); + }); + + afterEach(async () => { + vi.clearAllMocks(); + await fs.rm(dataDir, { recursive: true, force: true }); + + restoreEnv('DATA_DIR', originalEnv.dataDir); + restoreEnv('VERITAS_STORAGE', originalEnv.storage); + restoreEnv('VERITAS_ADMIN_KEY', originalEnv.adminKey); + restoreEnv('VERITAS_AUTH_ENABLED', originalEnv.authEnabled); + restoreEnv('NODE_ENV', originalEnv.nodeEnv); + }); + + it('keeps normal V8 allocation healthy across the API and probe contracts', async () => { + expect(healthyPressure).toMatchObject({ + status: 'ok', + reason: 'within-limits', + threshold: 0.9, + }); + + const [system, ready, deep] = await Promise.all([ + request(app).get('/api/v1/system/health'), + request(app).get('/health/ready'), + request(app).get('/health/deep').set('X-API-Key', 'test-admin-key-for-memory-health-feature'), + ]); + + expect(system.body).toMatchObject({ + status: 'stable', + signals: { + system: { + status: 'ok', + memory: true, + memoryPressure: healthyPressure, + }, + }, + }); + expect(ready.body).toMatchObject({ + status: 'ok', + checks: { memory: 'ok' }, + memoryPressure: healthyPressure, + }); + expect(deep.body).toMatchObject({ + status: 'ok', + checks: { memory: 'ok' }, + memoryPressure: healthyPressure, + }); + }); + + it('reports real pressure consistently without weakening one-warning severity', async () => { + mockGetMemoryPressure.mockReturnValue(warningPressure satisfies MemoryPressureDiagnostics); + + const [system, ready, deep] = await Promise.all([ + request(app).get('/api/v1/system/health'), + request(app).get('/health/ready'), + request(app).get('/health/deep').set('X-API-Key', 'test-admin-key-for-memory-health-feature'), + ]); + + expect(warningPressure).toMatchObject({ + status: 'warn', + reason: 'v8-heap-limit', + metric: 'heapUsed', + usedBytes: 950, + limitBytes: 1_000, + utilization: 0.95, + }); + expect(system.body).toMatchObject({ + status: 'reviewing', + signals: { system: { status: 'warn', memory: false, memoryPressure: warningPressure } }, + }); + expect(ready.status).toBe(200); + expect(ready.body).toMatchObject({ + status: 'degraded', + checks: { memory: 'warn' }, + memoryPressure: warningPressure, + }); + expect(deep.body).toMatchObject({ + status: 'degraded', + checks: { memory: 'warn' }, + memoryPressure: warningPressure, + }); + }); +}); + +function restoreEnv(name: string, value: string | undefined): void { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; +} diff --git a/server/src/__tests__/system-health-service.test.ts b/server/src/__tests__/system-health-service.test.ts index 73a3da11..4fd004c9 100644 --- a/server/src/__tests__/system-health-service.test.ts +++ b/server/src/__tests__/system-health-service.test.ts @@ -25,7 +25,7 @@ describe('SystemHealthService', () => { cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tmpDir); memSpy = vi .spyOn(process, 'memoryUsage') - .mockReturnValue({ heapUsed: 50, heapTotal: 100 } as any); + .mockReturnValue({ heapUsed: 50, heapTotal: 100, rss: 100, external: 0, arrayBuffers: 0 }); mockList.mockReturnValue([]); mockGetRunMetrics.mockResolvedValue({ runs: 0, successRate: 1, failures: 0, errors: 0 }); process.env.DATA_DIR = 'data'; @@ -53,14 +53,20 @@ describe('SystemHealthService', () => { expect(status.signals.agents).toMatchObject({ status: 'ok', total: 0, online: 0, offline: 0 }); }); - it('degrades level for warnings, offline agents, and low success rate', async () => { - memSpy.mockReturnValue({ heapUsed: 95, heapTotal: 100 } as any); + it('keeps allocated-heap utilization healthy while degrading for other warnings', async () => { + memSpy.mockReturnValue({ + heapUsed: 95, + heapTotal: 100, + rss: 100, + external: 0, + arrayBuffers: 0, + }); mockList.mockReturnValue([{ status: 'online' }, { status: 'offline' }]); mockGetRunMetrics.mockResolvedValue({ runs: 10, successRate: 0.75, failures: 2, errors: 0 }); const mod = await import('../services/system-health-service.js'); const status = await new mod.SystemHealthService().getStatus(); - expect(status.signals.system.status).toBe('warn'); + expect(status.signals.system.status).toBe('ok'); expect(status.signals.agents.status).toBe('warn'); expect(status.signals.operations.status).toBe('warn'); expect(status.status).toBe('drifting'); diff --git a/server/src/routes/health.ts b/server/src/routes/health.ts index 195bb347..c322b59f 100644 --- a/server/src/routes/health.ts +++ b/server/src/routes/health.ts @@ -23,6 +23,7 @@ import { } from '../services/dependency-circuit-runtime.js'; import { getSqliteStorageDiagnostics } from '../storage/sqlite/database.js'; import type { WebSocketServer } from 'ws'; +import { getMemoryPressure } from '../utils/memory-pressure.js'; import { getRuntimeDir } from '../utils/paths.js'; const log = createLogger('health'); @@ -111,22 +112,6 @@ async function checkDisk(): Promise<'ok' | 'fail'> { } } -/** - * Check that memory usage is below 90% of heap. - */ -function checkMemory(): 'ok' | 'warn' { - const mem = process.memoryUsage(); - const usedPercent = mem.heapUsed / mem.heapTotal; - if (usedPercent > 0.9) { - log.warn( - { heapUsed: mem.heapUsed, heapTotal: mem.heapTotal, usedPercent }, - 'Memory usage high' - ); - return 'warn'; - } - return 'ok'; -} - /** * Check that tasks.json is readable and valid JSON. */ @@ -213,9 +198,14 @@ healthRouter.get('/ready', async (_req, res) => { checkDisk(), checkTasksFile(), ]); - const memory = checkMemory(); + const memoryPressure = getMemoryPressure(); + const memory = memoryPressure.status; const sqlite = getSqliteStorageDiagnostics(); + if (memory === 'warn') { + log.warn({ memoryPressure }, 'Memory pressure high'); + } + // Storage encompasses both the directory check and the tasks file check const storageStatus = storage === 'fail' || tasksFile === 'fail' ? 'fail' : 'ok'; @@ -232,12 +222,13 @@ healthRouter.get('/ready', async (_req, res) => { checks.storage === 'fail' || checks.disk === 'fail' || ('sqlite' in checks && checks.sqlite === 'fail'); - const status = hasCriticalFailure ? 'degraded' : 'ok'; + const status = hasCriticalFailure || memory === 'warn' ? 'degraded' : 'ok'; const httpStatus = hasCriticalFailure ? 503 : 200; res.status(httpStatus).json({ status, checks, + memoryPressure, timestamp: new Date().toISOString(), }); } catch (err) { @@ -264,8 +255,8 @@ async function buildDeepHealthPayload() { checkDisk(), checkTasksFile(), ]); - const memory = checkMemory(); - const memUsage = process.memoryUsage(); + const memoryPressure = getMemoryPressure(); + const memory = memoryPressure.status; const storageStatus = storage === 'fail' || tasksFile === 'fail' ? 'fail' : 'ok'; const sqlite = getSqliteStorageDiagnostics(); @@ -323,6 +314,7 @@ async function buildDeepHealthPayload() { dependencyCircuitSummary.open > 0 || dependencyCircuitSummary.halfOpen > 0 || dependencyCircuitError !== undefined || + memory === 'warn' || (process.env.VERITAS_STORAGE === 'sqlite' && sqlite?.healthPosture !== 'healthy') ? 'degraded' : 'ok', @@ -333,11 +325,15 @@ async function buildDeepHealthPayload() { }, uptime: process.uptime(), version, + memoryPressure, memory: { - heapUsed: memUsage.heapUsed, - heapTotal: memUsage.heapTotal, - rss: memUsage.rss, - external: memUsage.external, + heapUsed: memoryPressure.sample.heapUsedBytes, + heapTotal: memoryPressure.sample.heapAllocatedBytes, + heapSizeLimit: memoryPressure.sample.heapLimitBytes, + rss: memoryPressure.sample.rssBytes, + external: memoryPressure.sample.externalBytes, + effectiveMemoryLimit: memoryPressure.sample.effectiveMemoryLimitBytes, + effectiveMemoryLimitSource: memoryPressure.sample.effectiveMemoryLimitSource, }, wsConnections, circuitBreakers, diff --git a/server/src/services/system-health-service.ts b/server/src/services/system-health-service.ts index 724cd761..27ea8260 100644 --- a/server/src/services/system-health-service.ts +++ b/server/src/services/system-health-service.ts @@ -11,11 +11,13 @@ */ import { createLogger } from '../lib/logger.js'; import { checkRuntimeDiskSpace, checkRuntimeStorageAccess } from '../storage/runtime-health.js'; +import { getMemoryPressure } from '../utils/memory-pressure.js'; import { getAgentRegistryService } from './agent-registry-service.js'; import { getMetricsService } from './metrics/index.js'; import type { HealthLevel, HealthStatus, + MemoryPressureDiagnostics, SystemSignal, AgentSignal, OperationsSignal, @@ -25,18 +27,18 @@ const log = createLogger('system-health-service'); // ─── Helpers ────────────────────────────────────────────────── -function checkMemory(): boolean { - const mem = process.memoryUsage(); - return mem.heapUsed / mem.heapTotal <= 0.9; -} - // ─── Signal builders ────────────────────────────────────────── -function buildSystemSignal(storage: boolean, disk: boolean, memory: boolean): SystemSignal { +function buildSystemSignal( + storage: boolean, + disk: boolean, + memoryPressure: MemoryPressureDiagnostics +): SystemSignal { + const memory = memoryPressure.status === 'ok'; const anyFail = !storage || !disk; const anyWarn = !memory; const status: 'ok' | 'warn' | 'fail' = anyFail ? 'fail' : anyWarn ? 'warn' : 'ok'; - return { status, storage, disk, memory }; + return { status, storage, disk, memory, memoryPressure }; } function buildAgentSignal(): AgentSignal { @@ -133,9 +135,9 @@ export class SystemHealthService { checkRuntimeStorageAccess(), checkRuntimeDiskSpace(), ]); - const memory = checkMemory(); + const memoryPressure = getMemoryPressure(); - const system = buildSystemSignal(storage, disk, memory); + const system = buildSystemSignal(storage, disk, memoryPressure); const agents = buildAgentSignal(); const operations = await buildOperationsSignal(); diff --git a/server/src/utils/memory-pressure.ts b/server/src/utils/memory-pressure.ts new file mode 100644 index 00000000..6aae567a --- /dev/null +++ b/server/src/utils/memory-pressure.ts @@ -0,0 +1,72 @@ +import os from 'node:os'; +import v8 from 'node:v8'; +import type { MemoryPressureDiagnostics } from '@veritas-kanban/shared'; + +/** + * Warn only when the process is using at least 90% of an actual capacity + * boundary. V8's currently allocated heap (`heapTotal`) is intentionally not + * a boundary because it normally grows and shrinks around garbage collection. + */ +export const MEMORY_PRESSURE_THRESHOLD = 0.9; + +export interface MemoryPressureSample { + heapUsedBytes: number; + heapAllocatedBytes: number; + heapLimitBytes: number; + rssBytes: number; + externalBytes: number; + effectiveMemoryLimitBytes: number; + effectiveMemoryLimitSource: 'process-constrained' | 'host-total'; +} + +function ratio(usedBytes: number, limitBytes: number): number { + return limitBytes > 0 ? usedBytes / limitBytes : 0; +} + +export function classifyMemoryPressure(sample: MemoryPressureSample): MemoryPressureDiagnostics { + const heapUtilization = ratio(sample.heapUsedBytes, sample.heapLimitBytes); + const rssUtilization = ratio(sample.rssBytes, sample.effectiveMemoryLimitBytes); + const useHeapMetric = heapUtilization >= rssUtilization; + const metric = useHeapMetric ? 'heapUsed' : 'rss'; + const usedBytes = useHeapMetric ? sample.heapUsedBytes : sample.rssBytes; + const limitBytes = useHeapMetric ? sample.heapLimitBytes : sample.effectiveMemoryLimitBytes; + const utilization = useHeapMetric ? heapUtilization : rssUtilization; + + return { + status: utilization >= MEMORY_PRESSURE_THRESHOLD ? 'warn' : 'ok', + reason: + utilization < MEMORY_PRESSURE_THRESHOLD + ? 'within-limits' + : metric === 'heapUsed' + ? 'v8-heap-limit' + : 'rss-memory-limit', + metric, + usedBytes, + limitBytes, + utilization, + threshold: MEMORY_PRESSURE_THRESHOLD, + sample, + }; +} + +export function collectMemoryPressureSample(): MemoryPressureSample { + const memory = process.memoryUsage(); + const heapLimitBytes = v8.getHeapStatistics().heap_size_limit; + const hostTotalBytes = os.totalmem(); + const constrainedBytes = process.constrainedMemory(); + const hasTighterProcessLimit = constrainedBytes > 0 && constrainedBytes <= hostTotalBytes; + + return { + heapUsedBytes: memory.heapUsed, + heapAllocatedBytes: memory.heapTotal, + heapLimitBytes, + rssBytes: memory.rss, + externalBytes: memory.external, + effectiveMemoryLimitBytes: hasTighterProcessLimit ? constrainedBytes : hostTotalBytes, + effectiveMemoryLimitSource: hasTighterProcessLimit ? 'process-constrained' : 'host-total', + }; +} + +export function getMemoryPressure(): MemoryPressureDiagnostics { + return classifyMemoryPressure(collectMemoryPressureSample()); +} diff --git a/shared/src/types/system-health.types.ts b/shared/src/types/system-health.types.ts index a4f4d0f6..84ce5462 100644 --- a/shared/src/types/system-health.types.ts +++ b/shared/src/types/system-health.types.ts @@ -23,6 +23,26 @@ export type HealthLevel = 'stable' | 'reviewing' | 'drifting' | 'elevated' | 'al // inline types that already exist in the server route. export type OverallStatus = HealthLevel; +/** Non-sensitive evidence used to classify process memory pressure. */ +export interface MemoryPressureDiagnostics { + status: 'ok' | 'warn'; + reason: 'within-limits' | 'v8-heap-limit' | 'rss-memory-limit'; + metric: 'heapUsed' | 'rss'; + usedBytes: number; + limitBytes: number; + utilization: number; + threshold: number; + sample: { + heapUsedBytes: number; + heapAllocatedBytes: number; + heapLimitBytes: number; + rssBytes: number; + externalBytes: number; + effectiveMemoryLimitBytes: number; + effectiveMemoryLimitSource: 'process-constrained' | 'host-total'; + }; +} + // ─── Individual Signals ──────────────────────────────────────── /** Infrastructure / system-level signal */ @@ -31,6 +51,7 @@ export interface SystemSignal { storage: boolean; disk: boolean; memory: boolean; + memoryPressure: MemoryPressureDiagnostics; } /** Agent registry signal */ diff --git a/web/src/__tests__/system-health-banner.test.tsx b/web/src/__tests__/system-health-banner.test.tsx new file mode 100644 index 00000000..c4e3c96e --- /dev/null +++ b/web/src/__tests__/system-health-banner.test.tsx @@ -0,0 +1,69 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { SystemHealthBar } from '@/components/layout/SystemHealthBar'; + +const mockUseSystemHealth = vi.fn(); + +vi.mock('@/hooks/useSystemHealth', () => ({ + useSystemHealth: () => mockUseSystemHealth(), +})); + +const baseSignals = { + agents: { status: 'ok', total: 0, online: 0, offline: 0 }, + operations: { status: 'ok', recentRuns: 0, successRate: 100, failedRuns: 0 }, +}; + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe('system health banner memory status', () => { + it('renders Stable and Memory: OK for normal V8 allocation', () => { + mockUseSystemHealth.mockReturnValue( + hookResult({ + status: 'stable', + signals: { + ...baseSignals, + system: { status: 'ok', storage: true, disk: true, memory: true }, + }, + }) + ); + + render(); + expect(screen.getByText('Stable')).toBeTruthy(); + + fireEvent.click(screen.getByRole('button', { name: /System health: Stable/ })); + expect(screen.getByText('Memory: OK')).toBeTruthy(); + expect(screen.queryByText('Memory: High')).toBeNull(); + }); + + it('renders the real memory warning detail', () => { + mockUseSystemHealth.mockReturnValue( + hookResult({ + status: 'reviewing', + signals: { + ...baseSignals, + system: { status: 'warn', storage: true, disk: true, memory: false }, + }, + }) + ); + + render(); + expect(screen.getByText('Reviewing')).toBeTruthy(); + + fireEvent.click(screen.getByRole('button', { name: /System health: Reviewing/ })); + expect(screen.getByText('Memory: High')).toBeTruthy(); + }); +}); + +function hookResult(data: unknown) { + return { + data, + isLoading: false, + isError: false, + isFetching: false, + refetch: vi.fn(), + error: null, + }; +} diff --git a/web/src/hooks/useSystemHealth.ts b/web/src/hooks/useSystemHealth.ts index beba9f2e..bc88bc38 100644 --- a/web/src/hooks/useSystemHealth.ts +++ b/web/src/hooks/useSystemHealth.ts @@ -1,4 +1,5 @@ import { useQuery } from '@tanstack/react-query'; +import type { MemoryPressureDiagnostics } from '@veritas-kanban/shared'; import { useWebSocketStatus } from '@/contexts/WebSocketContext'; import { apiFetch } from '@/lib/api/helpers'; @@ -11,6 +12,7 @@ export interface SystemSignal { storage: boolean; disk: boolean; memory: boolean; + memoryPressure: MemoryPressureDiagnostics; } export interface AgentSignal {