From 4c5514a6ef360922ac437358680263389db4df1d Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 13:30:54 +0500 Subject: [PATCH 01/33] Fix agent-release route shadowed by /nodes/:nodeId (audit G-CRIT) GET /api/v1/nodes/agent-release was registered after GET /api/v1/nodes/:nodeId. The static+param collision forces Hono onto the registration-order-sensitive TrieRouter, so the release route was swallowed by the detail handler (404), silently killing the whole PR #32 update-available feature in production while isolated unit tests stayed green. Register the static route first (mirroring /export); make the release service injectable through registerNodeRoutes; add a full-surface route test (red: 404, green: 200). Extract large node-route test fakes to node-routes-helpers.ts to stay under the LOC guard. Co-Authored-By: Claude Opus 4.8 --- apps/api/src/node-routes.ts | 19 ++- apps/api/test/node-routes-helpers.ts | 142 +++++++++++++++++++++ apps/api/test/node-routes.test.ts | 178 ++++++--------------------- 3 files changed, 197 insertions(+), 142 deletions(-) create mode 100644 apps/api/test/node-routes-helpers.ts diff --git a/apps/api/src/node-routes.ts b/apps/api/src/node-routes.ts index dcf2c216..1af606f8 100644 --- a/apps/api/src/node-routes.ts +++ b/apps/api/src/node-routes.ts @@ -11,6 +11,7 @@ import { } from "@rakkr/shared"; import { registerAgentReleaseRoutes } from "./agent-release-routes.js"; +import type { AgentReleaseService } from "./agent-release-service.js"; import type { AuthResult } from "./auth-service.js"; import { buildMeterFrame, demoMetersEnabled } from "./demo-data.js"; import type { @@ -36,6 +37,7 @@ import type { NodeStore } from "./node-store.js"; import { NodeStoreError } from "./node-store.js"; interface NodeRouteDependencies { + agentReleaseService?: AgentReleaseService; app: Hono; bootstrapStore: NodeBootstrapStore; currentAuth: (c: Context) => AuthResult; @@ -151,6 +153,7 @@ const nodeInterfaceUpdateSchema = z .refine(hasNodeUpdate, "At least one interface field is required"); export function registerNodeRoutes({ + agentReleaseService: releaseService, app, bootstrapStore, canServeWholeNodeMonitor = async () => true, @@ -167,6 +170,18 @@ export function registerNodeRoutes({ scopedNodes, sshCredentialStore, }: NodeRouteDependencies) { + // Register the static `/api/v1/nodes/agent-release` route BEFORE any + // `/api/v1/nodes/:nodeId` route. The node route set mixes a static child + // (`/export`) with a param child (`:nodeId`) at the same trie position, which + // Hono's RegExpRouter cannot represent, so the whole app falls back to the + // registration-order-sensitive TrieRouter. A static route registered AFTER + // `:nodeId` loses the match and gets swallowed by the detail handler (→ 404). + // Keeping this first mirrors how `/export` avoids the collision. + registerAgentReleaseRoutes({ + agentReleaseService: releaseService, + app, + requirePermission, + }); registerNodeInventoryRoutes({ app, currentAuth, @@ -193,10 +208,6 @@ export function registerNodeRoutes({ requirePermission, scopedNodes, }); - registerAgentReleaseRoutes({ - app, - requirePermission, - }); registerNodeSshCredentialRoutes({ app, currentAuth, diff --git a/apps/api/test/node-routes-helpers.ts b/apps/api/test/node-routes-helpers.ts new file mode 100644 index 00000000..5c74042f --- /dev/null +++ b/apps/api/test/node-routes-helpers.ts @@ -0,0 +1,142 @@ +import type { MeterFrame, RecorderNode } from "@rakkr/shared"; +import type { MeterFrameStore } from "../src/meter-store.js"; +import type { NodeInterfaceUpdateInput, NodeStore, NodeUpdateInput } from "../src/node-store.js"; + +// Shared fakes/fixtures for the node route tests. Extracted from +// node-routes.test.ts to keep that file under the 1000-LOC guard (audit Run 1). + +export function memoryMeterFrameStore(frames: MeterFrame[]): MeterFrameStore { + return { + async history(nodeId, limit = frames.length) { + return frames.filter((frame) => frame.nodeId === nodeId).slice(0, limit); + }, + async latest(nodeId) { + return frames.find((frame) => frame.nodeId === nodeId); + }, + async save(frame) { + frames.unshift(frame); + + return { + frame, + receivedAt: new Date().toISOString(), + }; + }, + }; +} + +export function memoryNodeStore(nodes: RecorderNode[]): NodeStore { + return { + async authenticateCredential() { + return undefined; + }, + async enroll() { + throw new Error("not implemented"); + }, + async find(nodeId) { + return nodes.find((candidate) => candidate.id === nodeId); + }, + async heartbeat() { + throw new Error("not implemented"); + }, + async list() { + return nodes; + }, + async rotateCredential() { + throw new Error("not implemented"); + }, + async updateInterface(nodeId: string, interfaceId: string, input: NodeInterfaceUpdateInput) { + const index = nodes.findIndex((candidate) => candidate.id === nodeId); + + if (index < 0) { + return undefined; + } + + const interfaceIndex = nodes[index].interfaces.findIndex( + (candidate) => candidate.id === interfaceId, + ); + + if (interfaceIndex < 0) { + return undefined; + } + + const audioInterface = nodes[index].interfaces[interfaceIndex]; + const channelAliases = new Map( + (input.channels ?? []).map((channel) => [channel.index, channel.alias]), + ); + const interfaces = [...nodes[index].interfaces]; + + interfaces[interfaceIndex] = { + ...audioInterface, + alias: input.alias ?? audioInterface.alias, + channels: audioInterface.channels.map((channel) => ({ + ...channel, + alias: channelAliases.get(channel.index) ?? channel.alias, + })), + hardwarePath: + input.hardwarePath === undefined + ? audioInterface.hardwarePath + : (input.hardwarePath ?? undefined), + sampleRates: input.sampleRates ?? audioInterface.sampleRates, + serialNumber: + input.serialNumber === undefined + ? audioInterface.serialNumber + : (input.serialNumber ?? undefined), + systemName: input.systemName ?? audioInterface.systemName, + systemRef: input.systemRef ?? audioInterface.systemRef, + }; + nodes[index] = { + ...nodes[index], + interfaces, + }; + + return nodes[index]; + }, + async update(nodeId, input: NodeUpdateInput) { + const index = nodes.findIndex((candidate) => candidate.id === nodeId); + + if (index < 0) { + return undefined; + } + + nodes[index] = { + ...nodes[index], + alias: input.alias ?? nodes[index].alias, + hostname: input.hostname ?? nodes[index].hostname, + ipAddresses: input.ipAddresses ?? nodes[index].ipAddresses, + location: { + ...nodes[index].location, + ...input.location, + }, + notes: input.notes === undefined ? nodes[index].notes : (input.notes ?? undefined), + audioDefaults: + input.audioDefaults === undefined ? nodes[index].audioDefaults : input.audioDefaults, + recordingCapacity: input.recordingCapacity ?? nodes[index].recordingCapacity, + tags: input.tags ?? nodes[index].tags, + }; + + return nodes[index]; + }, + }; +} + +export function wavChunk() { + const bytes = Buffer.alloc(48); + + bytes.write("RIFF", 0); + bytes.writeUInt32LE(40, 4); + bytes.write("WAVE", 8); + bytes.write("fmt ", 12); + bytes.writeUInt32LE(16, 16); + bytes.writeUInt16LE(1, 20); + bytes.writeUInt16LE(1, 22); + bytes.writeUInt32LE(16_000, 24); + bytes.writeUInt32LE(32_000, 28); + bytes.writeUInt16LE(2, 32); + bytes.writeUInt16LE(16, 34); + bytes.write("data", 36); + bytes.writeUInt32LE(4, 40); + bytes.writeInt16LE(100, 44); + bytes.writeInt16LE(-100, 46); + + return bytes; +} diff --git a/apps/api/test/node-routes.test.ts b/apps/api/test/node-routes.test.ts index 96dff811..868129ca 100644 --- a/apps/api/test/node-routes.test.ts +++ b/apps/api/test/node-routes.test.ts @@ -3,6 +3,7 @@ import { randomUUID } from "node:crypto"; import test from "node:test"; import { Hono } from "hono"; import type { AuditEvent, CurrentUser, MeterFrame, Permission, RecorderNode } from "@rakkr/shared"; +import type { AgentReleaseService } from "../src/agent-release-service.js"; import type { AuthResult } from "../src/auth-service.js"; import type { AppBindings, @@ -10,8 +11,7 @@ import type { RecordAuditEvent, RequirePermission, } from "../src/http-types.js"; -import type { MeterFrameStore } from "../src/meter-store.js"; -import type { NodeInterfaceUpdateInput, NodeStore, NodeUpdateInput } from "../src/node-store.js"; +import { memoryMeterFrameStore, memoryNodeStore, wavChunk } from "./node-routes-helpers.js"; const { createAuditStore } = await import("../src/audit-store.js"); const { createListenMonitorStore } = await import("../src/listen-monitor-store.js"); @@ -99,6 +99,41 @@ test("node routes deny users without required permissions", async () => { assert.ok(deniedEvents.every((event) => event.actor.id === deniedUser.id)); }); +test("GET /api/v1/nodes/agent-release resolves to the release route, not the :nodeId detail handler", async () => { + const auditStore = createAuditStore(""); + const snapshot = { + checkedAt: "2026-07-01T00:00:00.000Z", + data: { + publishedAt: "2026-06-28T00:00:00.000Z", + tag: "agent-v2026.06.28-1", + url: "https://github.com/yashau/Rakkr/releases/tag/agent-v2026.06.28-1", + version: "2026.06.28-1", + }, + }; + const app = nodeApp({ + agentReleaseService: { snapshot: () => snapshot, warm: async () => {} }, + auditStore, + frames: [], + nodes: [node()], + permissionCalls: [], + }); + + const response = await app.request("/api/v1/nodes/agent-release"); + const body = (await response.json()) as { + checkedAt?: string; + data?: { version?: string }; + error?: string; + }; + + // Before the fix, the static /agent-release path is shadowed by GET + // /api/v1/nodes/:nodeId (registered earlier), so it resolves to the detail + // handler with nodeId="agent-release" and returns 404 "Node not found" — the + // whole update-available feature is silently dead in production. + assert.equal(response.status, 200); + assert.equal(body.error, undefined); + assert.equal(body.data?.version, "2026.06.28-1"); +}); + test("listen start returns a monitor stream URL and audits access", async () => { const auditStore = createAuditStore(""); const permissionCalls: PermissionCall[] = []; @@ -662,6 +697,7 @@ interface PermissionCall { } function nodeApp({ + agentReleaseService, auditStore, canServeWholeNodeMonitor, currentUser = user(), @@ -673,6 +709,7 @@ function nodeApp({ permissionMiddleware, scopedNodeIds, }: { + agentReleaseService?: AgentReleaseService; auditStore: ReturnType; canServeWholeNodeMonitor?: (user: CurrentUser, node: RecorderNode) => Promise; currentUser?: CurrentUser; @@ -687,6 +724,7 @@ function nodeApp({ const app = new Hono(); registerNodeRoutes({ + agentReleaseService, app, canServeWholeNodeMonitor, currentAuth: () => auth(currentUser), @@ -774,120 +812,6 @@ function recordAuditEvent(auditStore: ReturnType): Reco }; } -function memoryMeterFrameStore(frames: MeterFrame[]): MeterFrameStore { - return { - async history(nodeId, limit = frames.length) { - return frames.filter((frame) => frame.nodeId === nodeId).slice(0, limit); - }, - async latest(nodeId) { - return frames.find((frame) => frame.nodeId === nodeId); - }, - async save(frame) { - frames.unshift(frame); - - return { - frame, - receivedAt: new Date().toISOString(), - }; - }, - }; -} - -function memoryNodeStore(nodes: RecorderNode[]): NodeStore { - return { - async authenticateCredential() { - return undefined; - }, - async enroll() { - throw new Error("not implemented"); - }, - async find(nodeId) { - return nodes.find((candidate) => candidate.id === nodeId); - }, - async heartbeat() { - throw new Error("not implemented"); - }, - async list() { - return nodes; - }, - async rotateCredential() { - throw new Error("not implemented"); - }, - async updateInterface(nodeId: string, interfaceId: string, input: NodeInterfaceUpdateInput) { - const index = nodes.findIndex((candidate) => candidate.id === nodeId); - - if (index < 0) { - return undefined; - } - - const interfaceIndex = nodes[index].interfaces.findIndex( - (candidate) => candidate.id === interfaceId, - ); - - if (interfaceIndex < 0) { - return undefined; - } - - const audioInterface = nodes[index].interfaces[interfaceIndex]; - const channelAliases = new Map( - (input.channels ?? []).map((channel) => [channel.index, channel.alias]), - ); - const interfaces = [...nodes[index].interfaces]; - - interfaces[interfaceIndex] = { - ...audioInterface, - alias: input.alias ?? audioInterface.alias, - channels: audioInterface.channels.map((channel) => ({ - ...channel, - alias: channelAliases.get(channel.index) ?? channel.alias, - })), - hardwarePath: - input.hardwarePath === undefined - ? audioInterface.hardwarePath - : (input.hardwarePath ?? undefined), - sampleRates: input.sampleRates ?? audioInterface.sampleRates, - serialNumber: - input.serialNumber === undefined - ? audioInterface.serialNumber - : (input.serialNumber ?? undefined), - systemName: input.systemName ?? audioInterface.systemName, - systemRef: input.systemRef ?? audioInterface.systemRef, - }; - nodes[index] = { - ...nodes[index], - interfaces, - }; - - return nodes[index]; - }, - async update(nodeId, input: NodeUpdateInput) { - const index = nodes.findIndex((candidate) => candidate.id === nodeId); - - if (index < 0) { - return undefined; - } - - nodes[index] = { - ...nodes[index], - alias: input.alias ?? nodes[index].alias, - hostname: input.hostname ?? nodes[index].hostname, - ipAddresses: input.ipAddresses ?? nodes[index].ipAddresses, - location: { - ...nodes[index].location, - ...input.location, - }, - notes: input.notes === undefined ? nodes[index].notes : (input.notes ?? undefined), - audioDefaults: - input.audioDefaults === undefined ? nodes[index].audioDefaults : input.audioDefaults, - recordingCapacity: input.recordingCapacity ?? nodes[index].recordingCapacity, - tags: input.tags ?? nodes[index].tags, - }; - - return nodes[index]; - }, - }; -} - function auth(currentUser = user()): AuthResult { return { user: currentUser }; } @@ -964,25 +888,3 @@ function meterFrame(nodeId = node().id): MeterFrame { nodeId, }; } - -function wavChunk() { - const bytes = Buffer.alloc(48); - - bytes.write("RIFF", 0); - bytes.writeUInt32LE(40, 4); - bytes.write("WAVE", 8); - bytes.write("fmt ", 12); - bytes.writeUInt32LE(16, 16); - bytes.writeUInt16LE(1, 20); - bytes.writeUInt16LE(1, 22); - bytes.writeUInt32LE(16_000, 24); - bytes.writeUInt32LE(32_000, 28); - bytes.writeUInt16LE(2, 32); - bytes.writeUInt16LE(16, 34); - bytes.write("data", 36); - bytes.writeUInt32LE(4, 40); - bytes.writeInt16LE(100, 44); - bytes.writeInt16LE(-100, 46); - - return bytes; -} From 3baa06280f5faddb79ab820d8d3c9d0ce2c4a167 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 13:31:02 +0500 Subject: [PATCH 02/33] Exclude provisioning nodes from reachable counts (audit N1/N2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A never-contacted provisioning node was reported as reachable by both the rakkr_node_online metric gauge and the dashboard Active Nodes count, because each used a naive status !== "offline" that predates the new provisioning state — inflating the reachable count and masking the RakkrNodeOffline alert. Add a shared isNodeReachable predicate (online/ recording/degraded/alerting) and use it in both sites so they cannot diverge again. Red->green via a metrics test (provisioning -> 0) and a dashboardReportingNodes helper test. Co-Authored-By: Claude Opus 4.8 --- apps/api/src/metrics.ts | 19 +++++++------ apps/api/test/metrics.test.ts | 28 +++++++++++++++++++ .../src/lib/dashboard-page-helpers.test.ts | 19 +++++++++++++ apps/web/src/lib/dashboard-page-helpers.ts | 16 ++++++++++- apps/web/src/pages/dashboard.tsx | 3 +- packages/shared/src/index.ts | 12 ++++++++ 6 files changed, 86 insertions(+), 11 deletions(-) diff --git a/apps/api/src/metrics.ts b/apps/api/src/metrics.ts index 53f4b5de..c15f4b14 100644 --- a/apps/api/src/metrics.ts +++ b/apps/api/src/metrics.ts @@ -1,11 +1,12 @@ -import type { - AuditEvent, - HealthEvent, - MeterFrame, - RecorderNode, - RecordingJob, - RecordingSummary, - UploadQueueItem, +import { + isNodeReachable, + type AuditEvent, + type HealthEvent, + type MeterFrame, + type RecorderNode, + type RecordingJob, + type RecordingSummary, + type UploadQueueItem, } from "@rakkr/shared"; import { nodeOfflineEventType, scheduledLowSignalEventType } from "./watchdog-runner.js"; @@ -45,7 +46,7 @@ export function renderPrometheusMetrics(input: PrometheusMetricsInput) { pushHelp(lines, "rakkr_node_online", "Whether a recorder node is reachable."); pushType(lines, "rakkr_node_online", "gauge"); for (const node of input.nodes) { - pushMetric(lines, "rakkr_node_online", nodeLabels(node), node.status === "offline" ? 0 : 1); + pushMetric(lines, "rakkr_node_online", nodeLabels(node), isNodeReachable(node.status) ? 1 : 0); } pushHelp(lines, "rakkr_recording_active", "Active recording jobs by recorder node."); diff --git a/apps/api/test/metrics.test.ts b/apps/api/test/metrics.test.ts index 0e47bdc9..c6fa05be 100644 --- a/apps/api/test/metrics.test.ts +++ b/apps/api/test/metrics.test.ts @@ -179,6 +179,34 @@ test("neutralizes a carriage return in a label value so the scrape stays parseab assert.match(output, /alias="Rack\\rA"/); }); +test("reports a never-contacted provisioning node as not reachable (online=0)", () => { + // A `provisioning` node has never made contact, so `rakkr_node_online` + // ("Whether a recorder node is reachable") must be 0 — a naive + // `status !== "offline"` wrongly reports it as 1, inflating the reachable + // count on the Grafana single-stat and masking the RakkrNodeOffline alert. + const output = renderPrometheusMetrics({ + auditEvents: [], + healthEvents: [], + listenMonitorChunks: [], + meterFrames: [], + nodes: [ + { ...node(), id: "node_provisioning", status: "provisioning" }, + { ...node(), id: "node_live", status: "recording" }, + { ...node(), id: "node_dead", status: "offline" }, + ], + observedAt: new Date("2026-06-18T12:16:00.000Z"), + recordingCacheBytes: {}, + recordingJobs: [], + recordings: [], + startedAt: new Date("2026-06-18T12:00:00.000Z"), + uploadQueueItems: [], + }); + + assert.match(output, /rakkr_node_online\{[^}]*node_id="node_provisioning"[^}]*\} 0/); + assert.match(output, /rakkr_node_online\{[^}]*node_id="node_live"[^}]*\} 1/); + assert.match(output, /rakkr_node_online\{[^}]*node_id="node_dead"[^}]*\} 0/); +}); + function auditEvent( action: string, outcome: AuditEvent["outcome"], diff --git a/apps/web/src/lib/dashboard-page-helpers.test.ts b/apps/web/src/lib/dashboard-page-helpers.test.ts index c6be4925..e0a558b2 100644 --- a/apps/web/src/lib/dashboard-page-helpers.test.ts +++ b/apps/web/src/lib/dashboard-page-helpers.test.ts @@ -7,9 +7,28 @@ import { dashboardActiveHealthEvents, dashboardIncidentActions, dashboardPagePermissions, + dashboardReportingNodes, dashboardSelectedNodeId, } from "./dashboard-page-helpers"; +test("dashboard reporting nodes exclude never-contacted provisioning and offline nodes", () => { + const nodes = [ + { id: "n_prov", status: "provisioning" as const }, + { id: "n_online", status: "online" as const }, + { id: "n_recording", status: "recording" as const }, + { id: "n_degraded", status: "degraded" as const }, + { id: "n_alerting", status: "alerting" as const }, + { id: "n_offline", status: "offline" as const }, + ]; + + // A provisioning node has never reported; it (and offline) must not count as + // "reporting". A naive `status !== "offline"` filter wrongly keeps provisioning. + assert.deepEqual( + dashboardReportingNodes(nodes).map((node) => node.id), + ["n_online", "n_recording", "n_degraded", "n_alerting"], + ); +}); + test("dashboard page reads and meters require node read permission", () => { assert.deepEqual(dashboardPagePermissions(undefined), { canAcknowledgeHealth: false, diff --git a/apps/web/src/lib/dashboard-page-helpers.ts b/apps/web/src/lib/dashboard-page-helpers.ts index 50feffbc..5868e97e 100644 --- a/apps/web/src/lib/dashboard-page-helpers.ts +++ b/apps/web/src/lib/dashboard-page-helpers.ts @@ -1,7 +1,21 @@ -import type { CurrentUser, HealthEvent, RecorderNode, RecordingJob } from "@rakkr/shared"; +import { + isNodeReachable, + type CurrentUser, + type HealthEvent, + type RecorderNode, + type RecordingJob, +} from "@rakkr/shared"; export type DashboardIncidentAction = "acknowledge" | "resolve"; +// Nodes the dashboard counts as "reporting" / lists under Active Nodes. A naive +// `status !== "offline"` wrongly counts a never-contacted `provisioning` node as +// online (audit N2); defer to the shared reachability predicate so this matches +// the /metrics gauge and the node-status badge convention. +export function dashboardReportingNodes>(nodes: T[]): T[] { + return nodes.filter((node) => isNodeReachable(node.status)); +} + export function dashboardPagePermissions(user: CurrentUser | undefined) { const permissions = user?.permissions ?? []; const canRead = permissions.includes("node:read"); diff --git a/apps/web/src/pages/dashboard.tsx b/apps/web/src/pages/dashboard.tsx index 088a4ce0..fa19ad08 100644 --- a/apps/web/src/pages/dashboard.tsx +++ b/apps/web/src/pages/dashboard.tsx @@ -26,6 +26,7 @@ import { dashboardActiveHealthEvents, dashboardIncidentActions, dashboardPagePermissions, + dashboardReportingNodes, type DashboardIncidentAction, } from "@/lib/dashboard-page-helpers"; import { formatDateTime } from "@/lib/dates"; @@ -107,7 +108,7 @@ export function DashboardPage() { const status = statusQuery.data; const nodes = nodesQuery.data?.data ?? []; - const activeNodes = nodes.filter((node) => node.status !== "offline"); + const activeNodes = dashboardReportingNodes(nodes); const activeHealthEvents = dashboardActiveHealthEvents(healthEventsQuery.data?.data ?? []); const activeRecordingJobs = dashboardActiveRecordingJobs(recordingJobsQuery.data?.data ?? []); const nodeAlias = (nodeId: string) => nodes.find((node) => node.id === nodeId)?.alias ?? nodeId; diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 309c97da..81f91829 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -45,6 +45,18 @@ export const nodeStatusSchema = z.enum([ "alerting", ]); +// A node is "reachable" (in contact and reporting) when its status is online, +// recording, degraded, or alerting. "offline" (heartbeat gone stale) and +// "provisioning" (enrolled but never contacted) are NOT reachable. Shared by the +// /metrics `rakkr_node_online` gauge and the dashboard active-node count so the +// two cannot diverge on how a never-contacted provisioning node is treated +// (a naive `status !== "offline"` counts provisioning as online — see audit N1/N2). +export function isNodeReachable(status: NodeStatus): boolean { + return ( + status === "online" || status === "recording" || status === "degraded" || status === "alerting" + ); +} + export const healthEventStatusSchema = z.enum(["open", "acknowledged", "suppressed", "resolved"]); export const recordingSourceSchema = z.enum(["ad_hoc", "schedule"]); From 61987116639eb2e0f50cf7325cab6af81a032e90 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 13:31:11 +0500 Subject: [PATCH 03/33] Prefill scheduling defaults on calendar create (audit S1) The calendar day-cell create path called defaultDraft(firstNode) without the controller settings, so it always fell back to the built-in profile/ policies and ignored the operator's configured scheduling defaults (the schedules list page passes them). Thread the already-fetched controller settings into the create draft, mirroring the list page. Co-Authored-By: Claude Opus 4.8 --- apps/web/src/pages/schedules-calendar.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/web/src/pages/schedules-calendar.tsx b/apps/web/src/pages/schedules-calendar.tsx index 43488656..c81baead 100644 --- a/apps/web/src/pages/schedules-calendar.tsx +++ b/apps/web/src/pages/schedules-calendar.tsx @@ -279,7 +279,10 @@ export function SchedulesCalendarPage() { function openCreate(cell: CalendarDayCell) { setDraft({ - ...defaultDraft(firstNode), + // Prefill the operator's configured scheduling defaults, mirroring the + // schedules list page — the calendar create path previously dropped the + // second arg and always fell back to the built-ins (audit S1). + ...defaultDraft(firstNode, controllerSettingsQuery.data?.data), recurrenceMode: "once", recurrenceStartAt: `${cell.iso}T09:00`, }); From a2383c3fc6cecc2758607c868617721b9bba243a Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 13:31:11 +0500 Subject: [PATCH 04/33] Cover controller-settings default merge/clear on the in-memory store (audit S4) The keep-vs-?? merge (omitted field keeps, explicit null clears a default) was only exercised by a DB-gated test, so a regression of keep() to ?? would clear-silently-broken with the whole in-memory suite still green. Add an in-memory route test that sets two defaults independently, confirms an unrelated PATCH preserves them, and confirms explicit null clears exactly one. Co-Authored-By: Claude Opus 4.8 --- .../test/settings-controller-routes.test.ts | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/apps/api/test/settings-controller-routes.test.ts b/apps/api/test/settings-controller-routes.test.ts index 9729571d..9ba3e271 100644 --- a/apps/api/test/settings-controller-routes.test.ts +++ b/apps/api/test/settings-controller-routes.test.ts @@ -60,6 +60,53 @@ test("controller settings read and update persist and audit", async () => { assert.ok(failures.some((event) => event.action === "settings.controller.update.failed")); }); +test("controller settings merge keeps unrelated defaults and clears one only on explicit null", async () => { + const app = new Hono(); + const auditStore = createAuditStore(""); + const currentUser = viewer(["settings:read", "settings:manage"]); + const controllerSettingsStore = createControllerSettingsStore(); + + registerSettingsControllerRoutes({ + app, + controllerSettingsStore, + currentAuth: () => ({ user: currentUser }), + recordAuditEvent: recordAuditEvent(auditStore), + requirePermission: allowPermission(), + }); + + // Two defaults set in separate single-field PATCHes (the shape the console's + // per-policy "set default" toggle actually sends): each must persist and not + // clobber the other. + await requestJson(app, "/api/v1/settings/controller", "PATCH", { + defaultRecordingProfileId: "profile_hifi", + }); + await requestJson(app, "/api/v1/settings/controller", "PATCH", { + defaultWatchdogPolicyId: "wd_strict", + }); + const afterSet = await readControllerSettings(app); + + // A PATCH of an unrelated field must leave both defaults untouched (keep, + // not reset to the schema default). + await requestJson(app, "/api/v1/settings/controller", "PATCH", { controllerName: "Keep Test" }); + const afterName = await readControllerSettings(app); + + // Explicit null clears exactly that default; an omitted field is preserved. + // A `?? current` merge would treat the clearing null as "keep" and this would + // regress silently — this is the case the `keep` helper exists for. + await requestJson(app, "/api/v1/settings/controller", "PATCH", { + defaultRecordingProfileId: null, + }); + const afterClear = await readControllerSettings(app); + + assert.equal(afterSet.defaultRecordingProfileId, "profile_hifi"); + assert.equal(afterSet.defaultWatchdogPolicyId, "wd_strict"); + assert.equal(afterName.controllerName, "Keep Test"); + assert.equal(afterName.defaultRecordingProfileId, "profile_hifi"); + assert.equal(afterName.defaultWatchdogPolicyId, "wd_strict"); + assert.equal(afterClear.defaultRecordingProfileId, null); + assert.equal(afterClear.defaultWatchdogPolicyId, "wd_strict"); +}); + test("controller settings deny without settings read and manage", async () => { const app = new Hono(); const auditStore = createAuditStore(""); @@ -111,6 +158,21 @@ async function jsonData(app: Hono, routePath: string) { return body.data; } +async function readControllerSettings(app: Hono) { + const response = await app.request("/api/v1/settings/controller"); + const body = (await response.json()) as { + data: { + controllerName: string; + defaultRecordingProfileId: string | null; + defaultWatchdogPolicyId: string | null; + }; + }; + + assert.equal(response.status, 200); + + return body.data; +} + function allowPermission(): RequirePermission { return () => async (_c, next) => { await next(); From 83b2fe7f4bb53a90041f92cc4823995996c384af Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 13:31:19 +0500 Subject: [PATCH 05/33] Lock provisioning/offline-liveness invariant in node-lifecycle baseline (audit N3) The node-lifecycle baseline verifier (shipped to promote node lifecycle to done) never asserted the provisioning-gating invariant PR #31 introduced: its source list omitted node-liveness.ts and watchdog-node-liveness.ts and no phrase/snippet mentioned provisioning or the offline gate. Document the provisioning/offline-liveness contract and extend the verifier to assert the gate source, the isNodeReachable predicate, and the liveness/watchdog test titles. Co-Authored-By: Claude Opus 4.8 --- .../baselines/NODE_LIFECYCLE_BASELINE.md | 21 +++++++++++++++++++ scripts/verify-node-lifecycle-baseline.mjs | 17 +++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/docs/internal/baselines/NODE_LIFECYCLE_BASELINE.md b/docs/internal/baselines/NODE_LIFECYCLE_BASELINE.md index 3db5bc7a..7f3f0cbf 100644 --- a/docs/internal/baselines/NODE_LIFECYCLE_BASELINE.md +++ b/docs/internal/baselines/NODE_LIFECYCLE_BASELINE.md @@ -74,6 +74,27 @@ rejected at every layer: `apps/api/src/agent-release-service.ts`, `apps/api/src/agent-release-routes.ts`, `apps/web/src/pages/nodes.tsx`, `apps/web/src/components/node-lifecycle-menu.tsx`. +## Provisioning And Offline Liveness + +- A newly enrolled node starts **provisioning** ("Awaiting first contact"): it has + never sent a heartbeat, so heartbeat-staleness cannot apply. It is **excluded + from offline alerting** — `reconcileNodeLivenessEvents` skips it with reason + `node_never_provisioned`, and `deriveNodeStatus` keeps it `provisioning` — until + its first heartbeat flips it to a live status. +- For every non-provisioning node, `deriveNodeStatus`/`nodeHeartbeatStale` returns + **offline** when the heartbeat is stale: strictly `ageSeconds > + offlineAfterSeconds` (`RAKKR_NODE_OFFLINE_AFTER_SECONDS`, default 120; a zero + threshold disables derivation). The watchdog opens exactly one critical + `watchdog.node_offline` health event per node (deduped against an already-open + event, filtered by type) and auto-resolves it on recovery. +- "Reachable" for the `/metrics` `rakkr_node_online` gauge and the dashboard's + active-node count is the shared `isNodeReachable` predicate (online / recording / + degraded / alerting). A never-contacted **provisioning** node and an **offline** + node are both *not* reachable, so neither inflates the "reporting" count. +- Evidence: `apps/api/src/node-liveness.ts`, + `apps/api/src/watchdog-node-liveness.ts`, `packages/shared/src/index.ts` + (`isNodeReachable`). + ## Binary Deployment - `update_binary` deploys from a published GitHub release by default: each target diff --git a/scripts/verify-node-lifecycle-baseline.mjs b/scripts/verify-node-lifecycle-baseline.mjs index 0f5e1306..ebef6c7b 100644 --- a/scripts/verify-node-lifecycle-baseline.mjs +++ b/scripts/verify-node-lifecycle-baseline.mjs @@ -6,11 +6,15 @@ const sourceFiles = [ "packages/shared/src/index.ts", "apps/api/src/node-lifecycle.ts", "apps/api/src/node-lifecycle-routes.ts", + "apps/api/src/node-liveness.ts", + "apps/api/src/watchdog-node-liveness.ts", "apps/api/src/agent-release-service.ts", "apps/api/src/agent-release-routes.ts", "apps/api/src/node-routes.ts", "apps/api/test/node-lifecycle.test.ts", "apps/api/test/node-lifecycle-routes.test.ts", + "apps/api/test/node-liveness.test.ts", + "apps/api/test/watchdog-runner.test.ts", "apps/api/test/agent-release-service.test.ts", "apps/api/test/agent-release-routes.test.ts", "apps/api/test/agent-version.test.ts", @@ -43,6 +47,11 @@ const baselinePhrases = [ "serial", "sha256", "never SSHes", + "provisioning", + "Awaiting first contact", + "node_never_provisioned", + "isNodeReachable", + "rakkr_node_online", "mise run nodes:check-lifecycle", ]; const sourceSnippets = [ @@ -76,6 +85,11 @@ const sourceSnippets = [ "api.agentRelease", "Update available", "agentReleaseQuery", + 'if (node.status === "provisioning")', + "node_never_provisioned", + "reconcileNodeLivenessEvents", + "nodeHeartbeatStale", + "isNodeReachable", ]; const testSnippets = [ "node lifecycle route runs allowlisted Ansible action and audits result", @@ -87,6 +101,9 @@ const testSnippets = [ "agent-release route returns the cached snapshot and is gated by node:read", "isAgentUpdateAvailable only fires for a strictly newer real version", "compareAgentVersions orders by date then counter", + "never-provisioned nodes are skipped by the liveness watchdog", + "provisioning nodes never derive offline, however old their enrollment", + "stale nodes derive offline status", ]; const errors = []; From 26e89d28cc7c2dc4a26f0127979d852860fe4174 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 13:33:04 +0500 Subject: [PATCH 06/33] Add 2026-07-08 gap-hunt audit ledger (Run 1) Co-Authored-By: Claude Opus 4.8 --- docs/internal/audits/2026-07-08-gap-hunt.md | 149 ++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 docs/internal/audits/2026-07-08-gap-hunt.md diff --git a/docs/internal/audits/2026-07-08-gap-hunt.md b/docs/internal/audits/2026-07-08-gap-hunt.md new file mode 100644 index 00000000..6c765407 --- /dev/null +++ b/docs/internal/audits/2026-07-08-gap-hunt.md @@ -0,0 +1,149 @@ +# Rakkr — Gap Hunt Findings (provisioning, scheduling defaults, agent-update) + +Adversarial correctness/security audit of the Rakkr monorepo, run as the +repeatable loop in [`docs/contributing/audit-workflow.md`](../../contributing/audit-workflow.md): +independent read-only fan-out hunters plus an adversarial verify pass on every +lead, then red→green proof for each confirmed fix. This audit opened right after +the PR #31 (node/schedule dialogs, scheduling defaults, **provisioning** node +state), PR #32 (recorder-agent **update-available** check; node lifecycle → ✅), +and PR #33 (web-console UI polish) landed on `main`, so that fresh surface is the +primary target. Target: **5 consecutive clean runs.** + +| Field | Content | +| ----- | ------- | +| Branch | `claude/elastic-khorana-78b100` | +| Base | rebased onto `origin/main` `619b6f10` (static for the audit so far) | +| Started | 2026-07-08 | +| Completed | _in progress_ | +| Runs | 1 so far — **streak 0** (Run 1 landed 6 fixes → dirty) | +| Findings closed | 6 (1 Critical, 3 Medium confirmed, 2 Medium coverage) — each red→green except the page-wiring `S1` (see its row) | +| Result | _in progress — Run 1 complete, streak 0_ | +| Gates at close of Run 1 | green — API 625 pass / 19 skip (non-DB), web 145 pass, tsc, oxlint, oxfmt, `check:loc` (all ≤1000), node-lifecycle baseline verifier. DB-gated suites + full `mise run check` (Docker/Postgres) deferred to convergence. | + +**Status legend:** `FIXED` = failing test + fix landed on this branch · `RESOLVED` += closed indirectly by another fix · `CATALOGUED` = confirmed/reproduced, fix +recommended but not applied (blast radius, product call, or hardware/Playwright +verification needed) · `SUSPECTED` = strong lead, not fully substantiated · +`COVERAGE` = no bug, a missing-test gap. + +The freshly-landed core logic is largely sound: the calendar version parse/compare +(`agent-version.ts`) is numeric (no lexicographic trap) and refuses to prompt on +unknown/dev versions; the GitHub release resolver validates shape and skips drafts/ +pre-releases/non-`agent-v` tags; the controller-settings `keep` merge correctly +separates omitted (keep) from explicit-`null` (clear); the liveness threshold math +is strict `age > threshold` with clock-skew guards; and the web permission +boundaries survived the redesign intact (every privileged control is capability- +gated through tested helpers). The confirmed gaps clustered where the **new +`provisioning` state** or the **new endpoint/defaults** met an older code path that +did not learn about them. + +--- + +## Everything fixed + +6 findings. Tags: `G1` (route shadow), `N1`/`N2` (provisioning reachability), +`S1` (calendar defaults), `S4` (merge coverage), `N3` (baseline coverage). + +### Critical + +| ID | Run | Area | Fix | Commit | +| --- | --- | --- | --- | --- | +| G1 | 1 | node-routes/routing | `GET /api/v1/nodes/agent-release` was registered **after** `GET /api/v1/nodes/:nodeId`. The node route set mixes a static child (`/export`) with a param child (`:nodeId`) at one trie position, which Hono's `RegExpRouter` can't represent → the app falls back to the **registration-order-sensitive** `TrieRouter`, and a static route registered after `:nodeId` is swallowed by the detail handler. So every console poll of the release endpoint hit the `:nodeId` handler with `nodeId="agent-release"` → `404 Node not found`; `isAgentUpdateAvailable(current, null)` was then always false and the **entire PR #32 update-available feature was silently dead in production** (while `agent-release-routes.test.ts`, which registers the route in isolation, stayed green). Empirically reproduced through a real Hono app. Fix: register the static release route **before** the inventory routes (mirrors how `/export` already wins); made the release service injectable through `registerNodeRoutes`; added a full-surface route test (red 404 → green 200). Test fakes extracted to `node-routes-helpers.ts` for the LOC guard. | `4c5514a6` | + +### Medium + +| ID | Run | Area | Fix | Commit | +| --- | --- | --- | --- | --- | +| N1 | 1 | metrics/observability | `rakkr_node_online` ("Whether a recorder node is reachable") used `node.status === "offline" ? 0 : 1`, so a never-contacted `provisioning` node reported `1` — inflating `sum(rakkr_node_online)` on the Grafana single-stat and masking the `RakkrNodeOffline` (`== 0`) alert. Introduced a shared `isNodeReachable(status)` predicate (online/recording/degraded/alerting) in `packages/shared` and gauge now uses it. Red→green via a metrics test (provisioning → 0, live → 1, offline → 0). | `3baa0628` | +| N2 | 1 | web/dashboard | The dashboard "Active Nodes" count + list used `nodes.filter(n => n.status !== "offline")`, counting a `provisioning` node as reporting (and the "No nodes online" empty state disagreed with the node-status convention). Routed through a new `dashboardReportingNodes` helper backed by the same `isNodeReachable` predicate as N1, so metric and UI cannot diverge. Red→green via a dashboard-page-helpers test. | `3baa0628` | +| S1 | 1 | web/scheduling | The `/schedules/calendar` day-cell create built its draft with `defaultDraft(firstNode)` — no controller-settings arg — so it always fell back to the built-in profile/policies and ignored the operator's configured scheduling defaults (the list page passes them). Threaded the already-fetched `controllerSettingsQuery.data?.data` into the create draft, mirroring the list page. Page-wiring fix: the `defaultDraft(node, defaults)` behaviour it now uses is covered by `schedule-draft.test.ts`; the page call itself has no component-test seam in the web unit harness (Playwright would be the end-to-end proof), so it lands verified-by-shared-coverage + inspection rather than a dedicated red→green. | `61987116` | +| S4 | 1 | settings/coverage | COVERAGE: the controller-settings `keep`-vs-`??` merge (omitted keeps, explicit `null` clears a default) was exercised only by a DB-gated test, so regressing `keep()` to `?? current` would silently stop clearing defaults with the whole in-memory suite green. Added an in-memory route test (set two defaults independently, unrelated PATCH preserves them, explicit `null` clears exactly one). Verified it goes red under the `keep→??` regression. | `a2383c3f` | +| N3 | 1 | baseline/coverage | COVERAGE: the node-lifecycle baseline verifier (shipped to promote node lifecycle to ✅) never asserted PR #31's provisioning-gating invariant — its `sourceFiles` omitted `node-liveness.ts`/`watchdog-node-liveness.ts` and no phrase/snippet mentioned provisioning or the offline gate. Documented the provisioning/offline-liveness contract in the baseline doc and extended the verifier to assert the gate source (`if (node.status === "provisioning")`, `node_never_provisioned`, `nodeHeartbeatStale`, `isNodeReachable`) plus the liveness/watchdog test titles. Verifier green. | `83b2fe7f` | + +### Also fixed (gate, not a product finding) + +| Item | Run | Fix | Commit | +| ---- | --- | --- | --- | +| LOC budget | 1 | The `G1` full-surface test pushed `apps/api/test/node-routes.test.ts` past 1000 LOC → extracted the large store/fixture fakes (`memoryNodeStore`, `memoryMeterFrameStore`, `wavChunk`) to `apps/api/test/node-routes-helpers.ts`. | `4c5514a6` | + +--- + +## Open work + +### Catalogued / suspected + +| ID | Sev | Kind | Locus | Note | +| --- | --- | --- | --- | --- | +| N4-HEARTBEAT-STATUS | Med-Low | suspected | `apps/api/src/agent-route-helpers.ts`, `node-store-updates.ts` | The heartbeat schema accepts the full `nodeStatusSchema` (incl. `provisioning`/`offline`) and persists the agent-reported status verbatim, with no "first contact promotes provisioning → live" guard. A node that heartbeats `status:"provisioning"` stays gated out of offline detection **forever** (a later-dead node never flagged); one reporting `offline` while alive shows offline. **Not reachable by the shipped Rust agent** (hardcodes `status:"online"` in `inventory.rs`), so it's a latent trust-boundary gap, not a live bug. Fix recipe: on the heartbeat write, coerce a stored `provisioning` node to a live status on first contact and/or restrict the heartbeat schema to live statuses. | +| S3-STALE-DEFAULT-ID | Med-Low | suspected | `apps/web/src/lib/schedule-draft.ts` (`defaultDraft`) | `defaultDraft` prefills `defaults?.defaultRecordingProfileId ?? built-in` with no existence check; the controller-settings columns are intentionally FK-free and may hold a deleted id. The ad-hoc panel (`recording-start-panel.tsx`) guards with a `.some(id === …)` existence check and falls back; `defaultDraft` does not, so a new-schedule form can prefill a dangling id. Bounded — the create API validates existence and 404s (`recording_profile_not_found` etc.), so the operator gets an opaque failed save, not corruption. Not a falsy-coalesce bug (`??`, so null/undefined fall back correctly). Fix: thread the available profile/policy lists into `defaultDraft` and guard each default like the ad-hoc panel. | +| N-3A-RELEASE-PAGING | Low | suspected | `apps/api/src/agent-release-service.ts:75` | The release fetch reads only `/releases?per_page=100` (page 1) and never follows `Link: rel="next"`. Because the repo interleaves `docs-v*`/`controller-v*`/`agent-v*` tags into one list, a burst of >100 non-agent releases could push the newest `agent-v…` off page 1 → a stale/null "latest". Unlikely at current cadence. Fix: follow pagination or filter server-side. | +| N-3B-RELEASE-BODY | Low | suspected | `apps/api/src/agent-release-service.ts:89` | `await response.json()` reads the whole body with no size cap. Only reachable via a misconfigured `RAKKR_GITHUB_API_URL` (operator-controlled), so low severity. Fix: bound the response read. | +| N-COV-FETCH | Low | coverage | `apps/api/test/agent-release-service.test.ts` | The fetch contract is correct but untested: request URL/headers (`User-Agent`, `X-GitHub-Api-Version`, `per_page=100`, `Bearer` only when a token is set), abort/timeout, and a non-`ok` (403 rate-limit) preserving the last-good value. Add assertions to lock the contract. | +| S2-DEAD-EXPORT | Low | coverage | `apps/web/src/lib/scheduling-defaults.ts:52` (`schedulingDefaultsFrom`) | Exported but has zero call sites repo-wide (both real consumers pass raw `ControllerSettings` into `defaultDraft`). A stray helper from PR #31 that also defines a second, divergent shape for "the four defaults". Remove it (or adopt it), not a runtime bug. | +| W1-DEAD-ROW-SELECT | Low | confirmed | `apps/web/src/components/ui/data-table.tsx:176,242,244` | `row.getIsSelected()` is always false — no page wires TanStack row selection (every consumer uses a hand-rolled checkbox column + page-local `selected*Ids`), so the `data-state="selected"` desktop styling and the mobile card `ring` highlight never render. Dead visual-feedback, not a correctness/permission bug. Proof needs a component/Playwright test (no RTL in the web unit harness). Fix: pass a per-row `isSelected` predicate into DataTable, or drive selection through TanStack. | +| W3-EMPTY-CARD-FOOTER | Low | confirmed | `apps/web/src/components/ui/data-table.tsx:229,272-278` + `nodes.tsx` | `nodes.tsx` keeps the actions column always present and returns `null` inside its cell when `!canManage`, so on mobile `DataTableCard` renders an empty bordered actions footer for read-only users. Other pages omit the column entirely (`if (canControl) columns.push(...)`). Playwright-only proof. Fix: make nodes.tsx omit the actions column when `!canManage` (mirror the other pages), or have the card skip an all-null footer. | +| W2-OBSERVER-DEP | Low | suspected | `apps/web/src/components/ui/truncate-cell.tsx:17-38` | `useLayoutEffect` has no dependency array, so every render tears down + recreates the `ResizeObserver` and re-measures. No leak (cleanup disconnects), but continuous churn on dense polling tables. Fix: depend on `[children]`. | +| W4-CARD-TITLE | Low | suspected | `apps/web/src/components/ui/data-table.tsx:233` + jobs/health | `DataTableCard` uses the first non-utility field cell as the card title; in jobs/health that's a `status`/`severity` Badge, so each mobile card's headline is a bare colored word and the identifying name is one row down. UX, not wrong data. Fix: order the identifying column first in those tables, or let a column opt into the card title. | + +### Coverage gaps (no bug — missing tests) + +- `N-COV-FETCH` (above) — GitHub fetch request/abort/rate-limit contract. +- Web `data-table.tsx` / `DataTableCard` have **no** tests at all (no RTL harness); `W1`/`W3`/`W4` are only catchable by Playwright today. + +--- + +## Verified clean (checked, no bug) + +Confirmed correct by direct source review + adversarial verification during Run 1: +the calendar-version parse/compare (`agent-version.ts` — numeric per-component, +`0.0.0-dev`/`agent-v` prefixes rejected as unknown, strict-greater update check, +no panic path); the GitHub release resolver (`resolveLatestAgentRelease` — array/ +entry shape validated, drafts/pre-releases/non-`agent-v` tags skipped, newest by +numeric compare); the agent-release service caching (stale-while-revalidate, error +back-off capped at `ttlMs`, last-good retained, `AbortSignal.timeout`, no SSRF — +URL/repo/token are operator env only, token never returned); the agent-release +route RBAC (`node:read`, audited, no per-node scope needed, no token leak); the +node liveness threshold math (`nodeHeartbeatStale` strict `age > threshold`, +`Math.max(0,…)` clock-skew guard, zero-threshold disables); the watchdog offline +dedupe (per-type 500-row query, single audit on create/resolve, no repeat-audit +spam) and recovery; the controller-settings `keep` merge (omitted vs explicit-null, +both JSON and Postgres stores share it); `weekStartsOn` is an enum (not a 0–6 int) +so out-of-range is impossible; controller-settings create-vs-update schemas have +identical bounds (no drift); the web redesign's permission boundaries (every +privileged control capability-gated through tested helpers; `SetDefaultButton` +gated on `canManage`; `ui-rbac-boundary.test.ts` forbids raw `permissions.includes` +in pages); DataTable pagination math + stable `getRowId`; `status-colors.ts` / +`node-status.ts` tone ladders; migrations 0045 (nullable default columns) and 0046 +(`provisioning` enum value) vs `schema.ts` (no drift). + +--- + +## Run log + +Following [`docs/contributing/audit-workflow.md`](../../contributing/audit-workflow.md). +Target: **5 consecutive clean runs.** A run is *dirty* if it changes any file, a +gate fails, a new confirmed finding surfaces, or `main` advanced with un-audited +commits → the streak resets to 0. Runs are strictly sequential; parallel read-only +hunters run within a run. + +| Run | Date | Focus | New confirmed | Fixed | Clean? | Streak | +| --- | ---- | ----- | ------------- | ----- | ------ | ------ | +| 1 | 2026-07-08 | fresh PR #31/#32/#33 surface: agent-release/version (H1), node liveness/provisioning (H2), scheduling defaults (H3), web-console redesign (H4) — 4 parallel hunters | G1, N1, N2, S1 (+ S4, N3 coverage) | 6 | no | 0 | + +### Notes on the runs + +- **Run 1** targeted the fresh surface from the three PRs that landed since the + prior audit merged, with four read-only fan-out hunters plus main-loop + verification. The headline is `G1`: a Hono static-vs-param route-registration + collision left the entire PR #32 "update available" feature dead in production + while its isolated unit test stayed green — reproduced empirically, then fixed + and locked with a full-surface route test. The `provisioning` state introduced + by PR #31 was correctly gated in the liveness core, but two sibling reachability + sites (`N1` metric, `N2` dashboard) still used the pre-provisioning `!== "offline"` + negation — unified behind a shared `isNodeReachable` predicate. `S1` (calendar + create ignored operator defaults) and two coverage gaps (`S4` merge-clear, + `N3` baseline verifier) rounded out the run. New **suspected/catalogued** leads + logged, not fixed: `N4-HEARTBEAT-STATUS`, `S3-STALE-DEFAULT-ID`, + `N-3A-RELEASE-PAGING`, `N-3B-RELEASE-BODY`, `N-COV-FETCH`, `S2-DEAD-EXPORT`, + `W1-DEAD-ROW-SELECT`, `W2-OBSERVER-DEP`, `W3-EMPTY-CARD-FOOTER`, `W4-CARD-TITLE`. + Dirty (landed 6 fixes) → streak 0. From 663798d162e4f0fc58d6b305867d2e204e3f4270 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 14:19:54 +0500 Subject: [PATCH 07/33] Re-clamp paginated lists when the total shrinks (audit H4-1) useServerPagination only reset offset on filter/page-size change, never when the row total shrank below the current offset. Deleting the last page's rows (bulk delete / retention sweep) stranded the operator on a blank page past the end with only Previous to escape. Add a pure clampedOffset helper (red->green) + a clampToTotal hook method the pages call during render with the server meta.total, mirroring the filter-reset pattern. Wired into all paginated pages. Co-Authored-By: Claude Opus 4.8 --- apps/web/src/lib/server-pagination.test.ts | 16 +++++++++++++++ apps/web/src/lib/server-pagination.ts | 23 ++++++++++++++++++++++ apps/web/src/lib/use-server-pagination.ts | 21 ++++++++++++++++++++ apps/web/src/pages/access.tsx | 1 + apps/web/src/pages/audit.tsx | 1 + apps/web/src/pages/health.tsx | 1 + apps/web/src/pages/jobs.tsx | 1 + apps/web/src/pages/nodes.tsx | 1 + apps/web/src/pages/schedules.tsx | 1 + 9 files changed, 66 insertions(+) diff --git a/apps/web/src/lib/server-pagination.test.ts b/apps/web/src/lib/server-pagination.test.ts index d4a24727..e23beb24 100644 --- a/apps/web/src/lib/server-pagination.test.ts +++ b/apps/web/src/lib/server-pagination.test.ts @@ -2,12 +2,28 @@ import assert from "node:assert/strict"; import test from "node:test"; import { + clampedOffset, currentPageFromOffset, offsetForPage, paginationSummary, shallowFiltersEqual, } from "./server-pagination"; +test("clampedOffset pulls a stranded offset back to the last non-empty page", () => { + // Still-valid offsets are left alone. + assert.equal(clampedOffset(50, 50, 60), 50); // page 2 of 60 has 10 rows + assert.equal(clampedOffset(0, 50, 0), 0); + assert.equal(clampedOffset(0, 50, 120), 0); + // Total shrank to exactly one page → the empty page 2 clamps to page 1. + assert.equal(clampedOffset(50, 50, 50), 0); + assert.equal(clampedOffset(50, 50, 40), 0); + // Total shrank but multiple pages remain → clamp to the new last page. + assert.equal(clampedOffset(100, 50, 90), 50); + // Defensive: negative offset / zero limit. + assert.equal(clampedOffset(-10, 50, 100), 0); + assert.equal(clampedOffset(50, 0, 100), 50); +}); + test("offsetForPage and currentPageFromOffset round-trip", () => { assert.equal(offsetForPage(1, 25), 0); assert.equal(offsetForPage(3, 25), 50); diff --git a/apps/web/src/lib/server-pagination.ts b/apps/web/src/lib/server-pagination.ts index 45a92cf4..042887ba 100644 --- a/apps/web/src/lib/server-pagination.ts +++ b/apps/web/src/lib/server-pagination.ts @@ -17,6 +17,29 @@ export function currentPageFromOffset(offset: number, limit: number): number { return Math.floor(Math.max(offset, 0) / limit) + 1; } +/** + * Clamp an offset back onto the last non-empty page when the total shrinks below + * it — e.g. the rows on the last page are deleted (bulk delete / retention sweep / + * single delete). Without this, a server-paginated list strands the user on an + * empty page past the end with only "Previous" to escape. Returns 0 for an empty + * list or a non-positive offset; leaves a still-valid offset untouched. + */ +export function clampedOffset(offset: number, limit: number, total: number): number { + if (offset <= 0 || limit <= 0) { + return Math.max(offset, 0); + } + + if (total <= 0) { + return 0; + } + + if (offset < total) { + return offset; + } + + return Math.max(Math.ceil(total / limit) - 1, 0) * limit; +} + export interface PaginationSummary { from: number; to: number; diff --git a/apps/web/src/lib/use-server-pagination.ts b/apps/web/src/lib/use-server-pagination.ts index fcc813e8..4bcef75b 100644 --- a/apps/web/src/lib/use-server-pagination.ts +++ b/apps/web/src/lib/use-server-pagination.ts @@ -1,6 +1,7 @@ import { useRef, useState } from "react"; import { + clampedOffset, currentPageFromOffset, defaultPageSize, defaultPageSizes, @@ -24,6 +25,13 @@ export interface ServerPagination { nextPage: () => void; previousPage: () => void; resetToFirstPage: () => void; + /** + * Re-clamp the window onto the last non-empty page when the server's reported + * total shrinks below the current offset (rows on the last page were deleted). + * Call during render with the latest `meta.total`; a no-op when the offset is + * still valid or the total is unknown. + */ + clampToTotal: (total: number | undefined) => void; } /** @@ -57,6 +65,19 @@ export function useServerPagination( } return { + clampToTotal: (total: number | undefined) => { + if (total === undefined) { + return; + } + + // Adjust state during render (same React pattern as the filter reset + // above): a shrunk total pulls a stranded offset back onto the last page. + const clamped = clampedOffset(offset, pageSize, total); + + if (clamped !== offset) { + setOffset(clamped); + } + }, limit: pageSize, nextPage: () => setOffset((current) => current + pageSize), offset, diff --git a/apps/web/src/pages/access.tsx b/apps/web/src/pages/access.tsx index bd5ee75d..e5486d7f 100644 --- a/apps/web/src/pages/access.tsx +++ b/apps/web/src/pages/access.tsx @@ -58,6 +58,7 @@ export function AccessPage() { queryFn: () => api.accessUsers(pagination.query), queryKey: ["access-users", pagination.query], }); + pagination.clampToTotal(usersQuery.data?.meta?.total); const groupsQuery = useQuery({ enabled: permissions.canRead, queryFn: () => api.accessGroups({ limit: 200 }), diff --git a/apps/web/src/pages/audit.tsx b/apps/web/src/pages/audit.tsx index e9e49e90..9c82e027 100644 --- a/apps/web/src/pages/audit.tsx +++ b/apps/web/src/pages/audit.tsx @@ -186,6 +186,7 @@ export function AuditPage() { const events = auditQuery.data?.data ?? []; const meta = auditQuery.data?.meta; + pagination.clampToTotal(meta?.total); const activeFilterChips = auditFilterChips(filters); const updateDraft = (key: keyof AuditFilterDraft, value: string) => setDraft((current) => ({ diff --git a/apps/web/src/pages/health.tsx b/apps/web/src/pages/health.tsx index 6fd2ae17..56bdcf0b 100644 --- a/apps/web/src/pages/health.tsx +++ b/apps/web/src/pages/health.tsx @@ -105,6 +105,7 @@ export function HealthPage() { queryKey: ["health-events", "workbench", pagination.query], refetchInterval: 5000, }); + pagination.clampToTotal(healthQuery.data?.meta?.total); const nodesQuery = useQuery({ enabled: permissions.canReadNodes, queryFn: () => api.nodes(nodePickerFilters()), diff --git a/apps/web/src/pages/jobs.tsx b/apps/web/src/pages/jobs.tsx index a0b0e877..1f2e5dce 100644 --- a/apps/web/src/pages/jobs.tsx +++ b/apps/web/src/pages/jobs.tsx @@ -99,6 +99,7 @@ export function JobsPage() { queryKey: ["recording-jobs", "workbench", pagination.query], refetchInterval: 5000, }); + pagination.clampToTotal(jobsQuery.data?.meta?.total); const nodesQuery = useQuery({ enabled: permissions.canReadNodes, queryFn: () => api.nodes(nodePickerFilters()), diff --git a/apps/web/src/pages/nodes.tsx b/apps/web/src/pages/nodes.tsx index 3edbe24b..9d40a6ea 100644 --- a/apps/web/src/pages/nodes.tsx +++ b/apps/web/src/pages/nodes.tsx @@ -123,6 +123,7 @@ export function NodesPage() { queryKey: ["nodes", pagination.query], refetchInterval: 5000, }); + pagination.clampToTotal(nodesQuery.data?.meta?.total); const healthEventsQuery = useQuery({ enabled: actionPermissions.canReadHealth, queryFn: () => api.healthEvents({ limit: 500 }), diff --git a/apps/web/src/pages/schedules.tsx b/apps/web/src/pages/schedules.tsx index fd91049c..3f74753c 100644 --- a/apps/web/src/pages/schedules.tsx +++ b/apps/web/src/pages/schedules.tsx @@ -86,6 +86,7 @@ export function SchedulesPage() { queryFn: () => api.schedules(pagination.query), queryKey: ["schedules", pagination.query], }); + pagination.clampToTotal(schedulesQuery.data?.meta?.total); const nodesQuery = useQuery({ enabled: actionPermissions.canReadNodes, queryFn: () => api.nodes(nodePickerFilters()), From 0257dbe5fb43103fa425a9c260d8c500b6468258 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 14:20:04 +0500 Subject: [PATCH 08/33] Finish stub-upload-policy removal from the console (audit H3-1, H3-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #31 filtered the test-only stub upload policy from settings/scheduling but missed two paths: (H3-1) the recordings page fed the unfiltered list to the per-recording and bulk Queue Upload dropdowns, so the stub was a selectable — and default-first — real upload target; (H3-2) editing a legacy schedule persisted with the stub silently re-saved it (the form's filtered toggles left no way to clear it). Add a selectableUploadPolicies helper for the recordings action dropdowns (labeling keeps the full list), and strip the stub in scheduleToDraft. Red->green helper + draft tests. (recordings.tsx also carries its pagination clampToTotal wiring, H4-1.) Co-Authored-By: Claude Opus 4.8 --- .../src/lib/recording-page-helpers.test.ts | 15 +++++++++++ apps/web/src/lib/recording-page-helpers.ts | 10 +++++++ apps/web/src/lib/schedule-draft.test.ts | 27 +++++++++++++++++++ apps/web/src/lib/schedule-draft.ts | 7 ++++- apps/web/src/pages/recordings.tsx | 9 +++++-- 5 files changed, 65 insertions(+), 3 deletions(-) diff --git a/apps/web/src/lib/recording-page-helpers.test.ts b/apps/web/src/lib/recording-page-helpers.test.ts index dcff9f9d..0ee9064d 100644 --- a/apps/web/src/lib/recording-page-helpers.test.ts +++ b/apps/web/src/lib/recording-page-helpers.test.ts @@ -9,6 +9,7 @@ import type { ScheduleSummary, UploadPolicy, } from "@rakkr/shared"; +import { defaultStubUploadPolicy } from "@rakkr/shared"; import { auditedUploadActionQueryKeys, @@ -23,6 +24,7 @@ import { recordingFileActionState, recordingPagePermissions, recordingRelationshipBadges, + selectableUploadPolicies, transcriptSnippetsFromText, transcriptSnippetsToText, uploadQueueStatusSummary, @@ -32,6 +34,19 @@ import { waveformPreviewSummary, } from "./recording-page-helpers"; +test("selectableUploadPolicies drops the test-only stub upload policy", () => { + const policies = [ + { id: defaultStubUploadPolicy.id, name: "Stub Upload Queue" }, + { id: "up_smb", name: "SMB Archive" }, + { id: "up_s3", name: "S3 Cold Storage" }, + ]; + + assert.deepEqual( + selectableUploadPolicies(policies).map((policy) => policy.id), + ["up_smb", "up_s3"], + ); +}); + test("G77: recording card cross-reference fetches beyond the default page", () => { // Jobs and upload-queue items are fetched globally and grouped onto each // recording card; the default 50-row page drops a recording's jobs/uploads diff --git a/apps/web/src/lib/recording-page-helpers.ts b/apps/web/src/lib/recording-page-helpers.ts index 3f844bde..5ce75d97 100644 --- a/apps/web/src/lib/recording-page-helpers.ts +++ b/apps/web/src/lib/recording-page-helpers.ts @@ -10,6 +10,7 @@ import type { UploadQueueItem, UploadQueueStatus, } from "@rakkr/shared"; +import { defaultStubUploadPolicy } from "@rakkr/shared"; import type { RecordingFileBlob, @@ -170,6 +171,15 @@ export const recordingSortOrders: Array<{ label: string; value: RecordingSortOrd { label: "Ascending", value: "asc" }, ]; +// The built-in stub upload policy is a test-only discard queue that the +// controller always injects into the policy list. It must never be an +// operator-selectable upload target in the console (audit H3-1); filter it from +// action dropdowns. The full list is still used to *label* any legacy recording +// that references it. +export function selectableUploadPolicies(policies: T[]): T[] { + return policies.filter((policy) => policy.id !== defaultStubUploadPolicy.id); +} + export function transcriptSnippetsFromText(value: string) { const seen = new Set(); const snippets: string[] = []; diff --git a/apps/web/src/lib/schedule-draft.test.ts b/apps/web/src/lib/schedule-draft.test.ts index 95ece426..2ee1c0eb 100644 --- a/apps/web/src/lib/schedule-draft.test.ts +++ b/apps/web/src/lib/schedule-draft.test.ts @@ -4,6 +4,7 @@ import { defaultControllerSettings, defaultKeepControllerCacheRetentionPolicy, defaultScheduledVoiceWatchdogPolicy, + defaultStubUploadPolicy, defaultVoiceRecordingProfile, } from "@rakkr/shared"; @@ -136,6 +137,32 @@ test("schedule backend draft round trips pinned and default values", () => { assert.equal(draft.channelMode, "stereo"); }); +test("scheduleToDraft drops a legacy stub upload policy so it is not silently re-saved", () => { + const legacy = draftToInput({ + ...defaultDraft(), + name: "Legacy Council Capture", + nodeId: "node_legacy", + room: "Council Chamber", + }); + const draft = scheduleToDraft({ + ...legacy, + // draftToInput yields nullable capture fields (ScheduleInput); a + // ScheduleSummary uses undefined for "unset". + captureBackend: undefined, + captureChannelSelection: undefined, + captureInterfaceId: undefined, + channelMode: undefined, + id: "sched_legacy_stub", + nextRunAt: "2026-06-18T09:00:00.000Z", + recurrence: { mode: "manual" }, + tags: [], + // A schedule persisted before the stub-removal still carries the stub id. + uploadPolicyIds: [defaultStubUploadPolicy.id, "up_real"], + }); + + assert.deepEqual(draft.uploadPolicyIds, ["up_real"]); +}); + test("schedule draft pins a sorted channel selection only with an interface", () => { const withInterface = draftToInput({ ...defaultDraft(), diff --git a/apps/web/src/lib/schedule-draft.ts b/apps/web/src/lib/schedule-draft.ts index 5f8e83dd..ee2c57be 100644 --- a/apps/web/src/lib/schedule-draft.ts +++ b/apps/web/src/lib/schedule-draft.ts @@ -1,6 +1,7 @@ import { defaultKeepControllerCacheRetentionPolicy, defaultScheduledVoiceWatchdogPolicy, + defaultStubUploadPolicy, defaultVoiceRecordingProfile, type AuditEvent, type ChannelMode, @@ -124,7 +125,11 @@ export function scheduleToDraft(schedule: ScheduleSummary): ScheduleDraft { tags: schedule.tags.join(", "), timezone: schedule.timezone, titleTemplate: schedule.titleTemplate, - uploadPolicyIds: schedule.uploadPolicyIds, + // Drop the test-only stub from a legacy schedule persisted with it: the form's + // policy toggles filter the stub out, so a retained stub id would be invisible + // yet silently re-saved on every edit (audit H3-2). Loading it out resolves the + // schedule to "no upload", matching the stub-removal intent. + uploadPolicyIds: schedule.uploadPolicyIds.filter((id) => id !== defaultStubUploadPolicy.id), watchdogPolicyId: schedule.watchdogPolicyId, }; diff --git a/apps/web/src/pages/recordings.tsx b/apps/web/src/pages/recordings.tsx index c0c2b3e8..7ca4e2f6 100644 --- a/apps/web/src/pages/recordings.tsx +++ b/apps/web/src/pages/recordings.tsx @@ -59,6 +59,7 @@ import { recordingSortOrders, recordingStatuses, selectClassName, + selectableUploadPolicies, uploadQueueStatusSummary, } from "@/lib/recording-page-helpers"; import { useRecordingPlaybackMutation } from "@/lib/recording-playback"; @@ -94,6 +95,7 @@ export function RecordingsPage() { queryFn: () => api.recordings(pagination.query), queryKey: ["recordings", pagination.query], }); + pagination.clampToTotal(recordingsQuery.data?.meta?.total); const recordingFacetsQuery = useQuery({ enabled: pagePermissions.canReadRecordings, queryFn: api.recordingFacets, @@ -369,6 +371,9 @@ export function RecordingsPage() { const recordingProfiles = recordingProfilesQuery.data?.data ?? []; const schedules = schedulesQuery.data?.data ?? []; const uploadPolicies = uploadPoliciesQuery.data?.data ?? []; + // Operator-selectable upload targets exclude the test-only stub (audit H3-1); + // `uploadPolicies` stays full for labeling any recording that references it. + const assignableUploadPolicies = selectableUploadPolicies(uploadPolicies); // Free-text search is inline in the toolbar; the slide-out chips/count cover // the remaining filters. const advancedFilterChips = recordingFilterChips(recordingFilters).filter( @@ -758,7 +763,7 @@ export function RecordingsPage() { selectedCount={selectedRecordingIds.length} uploadDisabled={bulkEnqueueUploadMutation.isPending} uploadEligibleCount={selectedCachedRecordingIds.length} - uploadPolicies={uploadPolicies} + uploadPolicies={assignableUploadPolicies} visibleCount={recordings.length} /> ) : null} @@ -812,7 +817,7 @@ export function RecordingsPage() { selected={selectedRecordingIdSet.has(recording.id)} stopPending={stopMutation.isPending} uploadItems={uploadItemsByRecording.get(recording.id) ?? []} - uploadPolicies={uploadPolicies} + uploadPolicies={assignableUploadPolicies} uploadPending={enqueueUploadMutation.isPending} /> ); From f5b5f219b6b69ac6e369caf47124953c9b1173f1 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 14:20:12 +0500 Subject: [PATCH 09/33] Stop agent :jobId route from shadowing /recording-jobs/export (audit H1-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The residual instance of the G1 route-shadow class: the node-auth GET /recording-jobs/:jobId (registered before the operator export route) swallowed GET /recording-jobs/export under TrieRouter, answering it with a node-credential 401. The agent handler must stay registered first (so agent job-reads work), so defer the reserved 'export' segment to the downstream operator handler — a job id is never literally 'export'. Red->green in the agent-job-read harness (production registration order). Co-Authored-By: Claude Opus 4.8 --- apps/api/src/agent-routes.ts | 12 +++++++++ apps/api/test/agent-job-read-routes.test.ts | 27 +++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/apps/api/src/agent-routes.ts b/apps/api/src/agent-routes.ts index 6fd1908f..b8ee86bb 100644 --- a/apps/api/src/agent-routes.ts +++ b/apps/api/src/agent-routes.ts @@ -480,6 +480,18 @@ export function registerAgentRoutes({ app.get("/api/v1/recording-jobs/:jobId", async (c, next) => { const jobId = c.req.param("jobId"); + + // `/api/v1/recording-jobs/export` is a static operator route that collides + // with this `:jobId` param route under Hono's TrieRouter (the app falls back + // to TrieRouter because of the nodes static+param collision — see audit G1). + // This node-auth handler is registered first, so without this guard it would + // answer `/export` with a node-credential 401 instead of the operator export + // handler. A real job id is never the literal "export", so defer it downstream. + if (jobId === "export") { + await next(); + return c.res; + } + const token = bearerToken(c.req.header("authorization")); if (!token) diff --git a/apps/api/test/agent-job-read-routes.test.ts b/apps/api/test/agent-job-read-routes.test.ts index 1438f731..005e9db0 100644 --- a/apps/api/test/agent-job-read-routes.test.ts +++ b/apps/api/test/agent-job-read-routes.test.ts @@ -26,6 +26,33 @@ test.after(async () => { await rm(routeRoot, { force: true, recursive: true }); }); +test("agent recording-job :jobId read defers the reserved /export segment to the operator route", async () => { + const app = new Hono(); + const auditStore = createAuditStore(""); + + registerAgentRoutes({ + app, + healthEventStore: createHealthEventStore("", []), + meterFrameStore: memoryMeterFrameStore(), + nodeStore: memoryNodeStore([node()]), + recordAuditEvent: recordAuditEvent(auditStore), + recordingStore: memoryRecordingStore([]), + settingsStore: {} as SettingsStore, + }); + // Operator export route registered AFTER the agent :jobId route, exactly as in + // index.ts (registerAgentRoutes runs before registerRecordingRoutes). Under + // TrieRouter the earlier `:jobId` handler would otherwise swallow `/export`. + app.get("/api/v1/recording-jobs/export", (c) => c.json({ handler: "operator-export" })); + + const response = await app.request("/api/v1/recording-jobs/export"); + const body = (await response.json()) as { error?: string; handler?: string }; + + // Before the guard, the node-auth handler answered /export with a + // node-credential 401 and the operator export handler was never reached. + assert.equal(response.status, 200); + assert.equal(body.handler, "operator-export"); +}); + test("agent recording-job polling and reads audit successes", async () => { const app = new Hono(); const auditStore = createAuditStore(""); From 75a7ca6b4ae9aaf221536111db770f692e7608b1 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 14:20:20 +0500 Subject: [PATCH 10/33] Promote provisioning nodes on heartbeat; block self-un-promotion (audit N4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The heartbeat write persisted the agent-reported status verbatim and the schema accepted 'provisioning'/'offline', so a first heartbeat only promoted a provisioning node because the shipped agent hardcodes 'online' — and any node-credential holder could POST status:'provisioning' to un-promote a live node, which deriveNodeStatus short-circuits, suppressing its offline alert forever. Coerce a heartbeat's provisioning/offline to online in both stores (heartbeatStatus); the controller owns lifecycle state. Red->green transform tests (also closes the H2-2 coverage gap). Co-Authored-By: Claude Opus 4.8 --- apps/api/src/node-store-updates.ts | 14 +++++- apps/api/src/node-store.ts | 9 +++- apps/api/test/node-store-updates.test.ts | 56 ++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 apps/api/test/node-store-updates.test.ts diff --git a/apps/api/src/node-store-updates.ts b/apps/api/src/node-store-updates.ts index afb5084f..feddde48 100644 --- a/apps/api/src/node-store-updates.ts +++ b/apps/api/src/node-store-updates.ts @@ -1,4 +1,4 @@ -import type { AudioInterface, RecorderNode } from "@rakkr/shared"; +import type { AudioInterface, NodeStatus, RecorderNode } from "@rakkr/shared"; import { nonEmptyAudioDefaults } from "./node-metadata.js"; import type { @@ -11,6 +11,16 @@ import type { // paths. Extracted from node-store.ts to keep it under the LOC budget; the type // imports above are erased, so the store <-> updates edge is not a runtime cycle. +// A heartbeat proves the node is in contact right now, so it must never leave +// the node looking never-contacted (`provisioning`) or stale (`offline`) — the +// controller owns the lifecycle state machine, so a first heartbeat promotes a +// provisioning node to live, and a node (or a stale/rolled-back agent) cannot +// self-report itself back out of offline detection (audit N4). Other live +// statuses the agent may report (recording/degraded/alerting) pass through. +export function heartbeatStatus(status: NodeStatus): NodeStatus { + return status === "provisioning" || status === "offline" ? "online" : status; +} + export function updatedNodeHeartbeat(node: RecorderNode, input: NodeHeartbeatInput): RecorderNode { return { ...node, @@ -19,7 +29,7 @@ export function updatedNodeHeartbeat(node: RecorderNode, input: NodeHeartbeatInp ipAddresses: input.ipAddresses, lastSeenAt: new Date().toISOString(), runtime: input.runtime ?? node.runtime, - status: input.status, + status: heartbeatStatus(input.status), }; } diff --git a/apps/api/src/node-store.ts b/apps/api/src/node-store.ts index 7e8ec587..5bde4d05 100644 --- a/apps/api/src/node-store.ts +++ b/apps/api/src/node-store.ts @@ -39,7 +39,12 @@ import { recorderInterfaceToRow, recorderNodeToRow, } from "./node-store-mappers.js"; -import { updatedNode, updatedNodeHeartbeat, updatedNodeInterface } from "./node-store-updates.js"; +import { + heartbeatStatus, + updatedNode, + updatedNodeHeartbeat, + updatedNodeInterface, +} from "./node-store-updates.js"; export interface NodeEnrollmentInput { agentVersion: string; @@ -448,7 +453,7 @@ class PostgresNodeStore implements NodeStore { lastSeenAt: new Date(), metadata: nodeMetadata(row.metadata, nodeRuntimeFromInput(input.runtime, row.metadata)), network: { ipAddresses: input.ipAddresses }, - status: input.status, + status: heartbeatStatus(input.status), updatedAt: new Date(), }) .where(eq(nodeRows.id, nodeId)); diff --git a/apps/api/test/node-store-updates.test.ts b/apps/api/test/node-store-updates.test.ts new file mode 100644 index 00000000..45d38986 --- /dev/null +++ b/apps/api/test/node-store-updates.test.ts @@ -0,0 +1,56 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import type { RecorderNode } from "@rakkr/shared"; + +import { heartbeatStatus, updatedNodeHeartbeat } from "../src/node-store-updates.js"; + +const baseNode: RecorderNode = { + agentVersion: "0.0.0-dev", + alias: "Council Chamber", + hostname: "council-node", + id: "node_council", + interfaces: [], + ipAddresses: ["10.0.0.10"], + lastSeenAt: "2026-06-18T12:00:00.000Z", + location: { room: "Council Chamber", site: "Main Office" }, + status: "provisioning", + tags: [], +}; + +test("heartbeatStatus promotes never-contacted/stale statuses to online", () => { + // A heartbeat proves the node is in contact, so it can never leave the node + // looking never-contacted (provisioning) or stale (offline). + assert.equal(heartbeatStatus("provisioning"), "online"); + assert.equal(heartbeatStatus("offline"), "online"); + // Genuine live statuses pass through unchanged. + assert.equal(heartbeatStatus("online"), "online"); + assert.equal(heartbeatStatus("recording"), "recording"); + assert.equal(heartbeatStatus("degraded"), "degraded"); + assert.equal(heartbeatStatus("alerting"), "alerting"); +}); + +test("first heartbeat promotes a provisioning node to a live status", () => { + const updated = updatedNodeHeartbeat(baseNode, { + agentVersion: "2026.06.28-1", + hostname: baseNode.hostname, + ipAddresses: baseNode.ipAddresses, + status: "online", + }); + + assert.equal(baseNode.status, "provisioning"); + assert.equal(updated.status, "online"); +}); + +test("a heartbeat cannot un-promote a live node back to provisioning", () => { + // A stale/rolled-back/hostile agent reporting `provisioning` must not suppress + // offline detection for a node that is actually alive (audit N4). + const live: RecorderNode = { ...baseNode, status: "online" }; + const updated = updatedNodeHeartbeat(live, { + agentVersion: "2026.06.28-1", + hostname: live.hostname, + ipAddresses: live.ipAddresses, + status: "provisioning", + }); + + assert.equal(updated.status, "online"); +}); From e42fb7ec9424399ab1d32cbcfa530e9006518b51 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 14:20:30 +0500 Subject: [PATCH 11/33] Require a real destination when creating an upload policy (audit H3-3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #31 intended every upload policy to target a real destination, but the create input left destinationId optional and the console eager-created a destination-less policy on New — assignable, and reconciling its recordings to partial (provider_not_configured). Enforce destinationId at the operator create route (the store stays lenient for seeds/tests); client seeds the first destination and disables New until one exists. Red->green route test; fixture updates for the stricter route. Co-Authored-By: Claude Opus 4.8 --- apps/api/src/settings-upload-policy-routes.ts | 12 ++++++ apps/api/test/settings-routes.test.ts | 8 ++++ ...ettings-upload-policy-scope-routes.test.ts | 41 ++++++++++++++++++- .../settings-upload-policies-section.tsx | 25 +++++++++-- .../src/components/upload-policy-panel.tsx | 3 +- packages/shared/src/index.ts | 3 ++ 6 files changed, 86 insertions(+), 6 deletions(-) diff --git a/apps/api/src/settings-upload-policy-routes.ts b/apps/api/src/settings-upload-policy-routes.ts index 50f5bff9..4ce96a53 100644 --- a/apps/api/src/settings-upload-policy-routes.ts +++ b/apps/api/src/settings-upload-policy-routes.ts @@ -51,6 +51,18 @@ export function registerSettingsUploadPolicyRoutes({ return c.json({ error: "Invalid upload policy", issues: body.error.issues }, 400); } + // Every operator-created policy must target a real destination; a + // destination-less policy silently reconciles its recordings to `partial` + // (audit H3-3). The store stays lenient for seeds/tests, so enforce here. + if (!body.data.destinationId) { + await recordSettingsFailure( + c, + "settings.upload_policies.create.failed", + "destination_required", + ); + return c.json({ error: "An upload policy must target a destination" }, 400); + } + const destinationDenied = await destinationReferenceFailure( c, body.data.destinationId, diff --git a/apps/api/test/settings-routes.test.ts b/apps/api/test/settings-routes.test.ts index 31fd49cb..0589daad 100644 --- a/apps/api/test/settings-routes.test.ts +++ b/apps/api/test/settings-routes.test.ts @@ -571,6 +571,13 @@ test("settings manage routes update operational templates and audit snapshots", const primaryTemplateId = `channel_map_ops_${randomUUID()}`; const rollbackTemplateId = `channel_map_rollback_${randomUUID()}`; const uploadPolicyId = `upload-policy-ops-${randomUUID()}`; + const uploadDestination = await uploadDestinationStore.create({ + displayName: "Operations Destination", + enabled: true, + kind: "smb", + smb: { server: "ops.lan", share: "recordings", username: "svc" }, + smbPassword: "s3cr3t", + }); registerSettingsRoutes({ app, @@ -616,6 +623,7 @@ test("settings manage routes update operational templates and audit snapshots", }, ); const uploadCreateResponse = await requestJson(app, "/api/v1/settings/upload-policies", "POST", { + destinationId: uploadDestination.id, enabled: true, id: uploadPolicyId, maxAttempts: 4, diff --git a/apps/api/test/settings-upload-policy-scope-routes.test.ts b/apps/api/test/settings-upload-policy-scope-routes.test.ts index b4480770..68dc6d2d 100644 --- a/apps/api/test/settings-upload-policy-scope-routes.test.ts +++ b/apps/api/test/settings-upload-policy-scope-routes.test.ts @@ -43,6 +43,7 @@ test("upload policy routes honor resource-scope denies", async () => { const auditStore = createAuditStore(""); const currentUser = viewer(); const hiddenPolicy = await createUploadPolicy({ + destinationId: "dest_hidden", enabled: true, maxAttempts: 3, name: `Hidden Upload Policy ${randomUUID()}`, @@ -180,6 +181,15 @@ test("R28: upload policy create with a duplicate id is a 409 and does not overwr const auditStore = createAuditStore(""); const currentUser = viewer(); + const uploadDestinationStore = createUploadDestinationStore(); + const destination = await uploadDestinationStore.create({ + displayName: "Duplicate Test Destination", + enabled: true, + kind: "smb", + smb: { server: "dup.lan", share: "recordings", username: "svc" }, + smbPassword: "s3cr3t", + }); + registerSettingsRoutes({ app, currentAuth: () => ({ user: currentUser }), @@ -187,11 +197,12 @@ test("R28: upload policy create with a duplicate id is a 409 and does not overwr recordAuditEvent: recordAuditEvent(auditStore), requirePermission: denyResourceScope(auditStore, currentUser, () => true), settingsStore: createSettingsStore(), - uploadDestinationStore: createUploadDestinationStore(), + uploadDestinationStore, }); const policyId = `upload_policy_dup_${randomUUID()}`; const first = await requestJson(app, "/api/v1/settings/upload-policies", "POST", { + destinationId: destination.id, enabled: true, id: policyId, maxAttempts: 3, @@ -199,6 +210,7 @@ test("R28: upload policy create with a duplicate id is a 409 and does not overwr trigger: "manual", }); const conflict = await requestJson(app, "/api/v1/settings/upload-policies", "POST", { + destinationId: destination.id, enabled: true, id: policyId, maxAttempts: 9, @@ -218,6 +230,33 @@ test("R28: upload policy create with a duplicate id is a 409 and does not overwr assert.equal(failed.at(-1)?.reason, "upload_policy_exists"); }); +test("H3-3: upload policy create without a destination is rejected", async () => { + const app = new Hono(); + const auditStore = createAuditStore(""); + const currentUser = viewer(); + + registerSettingsRoutes({ + app, + currentAuth: () => ({ user: currentUser }), + hasResourceScope: async () => true, + recordAuditEvent: recordAuditEvent(auditStore), + requirePermission: denyResourceScope(auditStore, currentUser, () => true), + settingsStore: createSettingsStore(), + uploadDestinationStore: createUploadDestinationStore(), + }); + + // Every upload policy must target a real destination; a destination-less + // create would otherwise reconcile its recordings to `partial` (audit H3-3). + const response = await requestJson(app, "/api/v1/settings/upload-policies", "POST", { + enabled: true, + maxAttempts: 3, + name: "Destination-less Policy", + trigger: "manual", + }); + + assert.equal(response.status, 400); +}); + function requestJson( app: Hono, url: string, diff --git a/apps/web/src/components/settings-upload-policies-section.tsx b/apps/web/src/components/settings-upload-policies-section.tsx index de63c885..53ec185e 100644 --- a/apps/web/src/components/settings-upload-policies-section.tsx +++ b/apps/web/src/components/settings-upload-policies-section.tsx @@ -38,8 +38,19 @@ export function SettingsUploadPoliciesSection({ queryFn: api.uploadPolicies, queryKey: ["upload-policies"], }); + const destinationsQuery = useQuery({ + enabled: canRead, + queryFn: api.uploadDestinations, + queryKey: ["upload-destinations"], + }); + // Every policy must target a real destination (audit H3-3): seed a new policy + // with the first configured destination, and don't offer "New" until one + // exists (the create schema now rejects a destination-less policy). + const destinations = destinationsQuery.data?.data ?? []; + const firstDestinationId = destinations[0]?.id; const createMutation = useMutation({ - mutationFn: () => api.createUploadPolicy(defaultUploadPolicyInput()), + mutationFn: (destinationId: string) => + api.createUploadPolicy(defaultUploadPolicyInput(destinationId)), onError: () => toast.error("Create failed", { description: "The upload policy could not be created.", @@ -81,9 +92,15 @@ export function SettingsUploadPoliciesSection({ {policies.length} policies createMutation.mutate()} + disabled={createMutation.isPending || !canManage || !firstDestinationId} + hint={ + !canManage + ? "Requires settings manage" + : firstDestinationId + ? "Create upload policy" + : "Add an upload destination first" + } + onClick={() => firstDestinationId && createMutation.mutate(firstDestinationId)} variant="outline" > diff --git a/apps/web/src/components/upload-policy-panel.tsx b/apps/web/src/components/upload-policy-panel.tsx index 6ac5d642..bb606745 100644 --- a/apps/web/src/components/upload-policy-panel.tsx +++ b/apps/web/src/components/upload-policy-panel.tsx @@ -191,9 +191,10 @@ function policyUpdate(policy: UploadPolicy): UploadPolicyUpdate { }; } -export function defaultUploadPolicyInput(): UploadPolicyInput { +export function defaultUploadPolicyInput(destinationId: string): UploadPolicyInput { return { deleteCacheAfterUpload: false, + destinationId, enabled: true, maxAttempts: 5, name: "New Upload Policy", diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 81f91829..57c5225a 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -810,6 +810,9 @@ export const uploadPolicySchema = z.object({ }); export const uploadPolicyInputSchema = z.object({ deleteCacheAfterUpload: z.boolean().default(false), + // Optional in the shared shape (the store is an internal seeding primitive), + // but the operator create route requires it — every policy must target a real + // destination or its recordings reconcile to `partial` (audit H3-3). destinationId: z.string().trim().min(1).max(160).optional(), enabled: z.boolean().default(true), id: z.string().trim().min(1).max(160).optional(), From 16f631762348673861254f468f9b9bd2dd0cd23d Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 14:20:31 +0500 Subject: [PATCH 12/33] Offer provisioning in the node status filter (audit H1-2) The node inventory status filter omitted 'provisioning', so operators could not filter to the enrolled-but-never-contacted cohort the API fully supports. Add it and lock the dropdown against the full NodeStatus enum. Co-Authored-By: Claude Opus 4.8 --- .../src/components/node-inventory-filters.test.ts | 12 ++++++++++++ apps/web/src/components/node-inventory-filters.tsx | 11 ++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 apps/web/src/components/node-inventory-filters.test.ts diff --git a/apps/web/src/components/node-inventory-filters.test.ts b/apps/web/src/components/node-inventory-filters.test.ts new file mode 100644 index 00000000..62857dcd --- /dev/null +++ b/apps/web/src/components/node-inventory-filters.test.ts @@ -0,0 +1,12 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { nodeStatusSchema } from "@rakkr/shared"; + +import { nodeStatuses } from "./node-inventory-filters"; + +test("node status filter offers every node status, including provisioning", () => { + // The dropdown must enumerate the full NodeStatus enum so no cohort (notably + // the enrolled-but-never-contacted `provisioning` nodes) becomes unfilterable. + assert.deepEqual([...nodeStatuses].sort(), [...nodeStatusSchema.options].sort()); + assert.ok(nodeStatuses.includes("provisioning")); +}); diff --git a/apps/web/src/components/node-inventory-filters.tsx b/apps/web/src/components/node-inventory-filters.tsx index 2bc90325..d5302ad3 100644 --- a/apps/web/src/components/node-inventory-filters.tsx +++ b/apps/web/src/components/node-inventory-filters.tsx @@ -11,7 +11,16 @@ import { SelectValue, } from "@/components/ui/select"; -const nodeStatuses: NodeStatus[] = ["online", "recording", "degraded", "alerting", "offline"]; +// Every node status is filterable, including `provisioning` (enrolled but never +// contacted) so operators can review the just-onboarded cohort (audit H1-2). +export const nodeStatuses: NodeStatus[] = [ + "provisioning", + "online", + "recording", + "degraded", + "alerting", + "offline", +]; const audioBackendFilters = ["alsa", "jack", "pipewire", "unknown"] as const; export type AudioBackendFilter = (typeof audioBackendFilters)[number]; From dd8c991353048f028f28f0b28ec6f948f3e568f9 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 14:23:32 +0500 Subject: [PATCH 13/33] Update gap-hunt ledger: Run 2 (7 fixes, streak 0) Co-Authored-By: Claude Opus 4.8 --- docs/internal/audits/2026-07-08-gap-hunt.md | 71 ++++++++++++++++++--- 1 file changed, 63 insertions(+), 8 deletions(-) diff --git a/docs/internal/audits/2026-07-08-gap-hunt.md b/docs/internal/audits/2026-07-08-gap-hunt.md index 6c765407..c4983bdf 100644 --- a/docs/internal/audits/2026-07-08-gap-hunt.md +++ b/docs/internal/audits/2026-07-08-gap-hunt.md @@ -15,10 +15,10 @@ primary target. Target: **5 consecutive clean runs.** | Base | rebased onto `origin/main` `619b6f10` (static for the audit so far) | | Started | 2026-07-08 | | Completed | _in progress_ | -| Runs | 1 so far — **streak 0** (Run 1 landed 6 fixes → dirty) | -| Findings closed | 6 (1 Critical, 3 Medium confirmed, 2 Medium coverage) — each red→green except the page-wiring `S1` (see its row) | -| Result | _in progress — Run 1 complete, streak 0_ | -| Gates at close of Run 1 | green — API 625 pass / 19 skip (non-DB), web 145 pass, tsc, oxlint, oxfmt, `check:loc` (all ≤1000), node-lifecycle baseline verifier. DB-gated suites + full `mise run check` (Docker/Postgres) deferred to convergence. | +| Runs | 2 so far — **streak 0** (Run 1 landed 6 fixes, Run 2 landed 7 → both dirty) | +| Findings closed | 13 (Run 1: `G1`,`N1`,`N2`,`S1`,`S4`,`N3` · Run 2: `H4-1`,`H3-1`,`H3-2`,`H1-1`,`N4`,`H3-3`,`H1-2`) — each red→green except the page-wiring `S1` (see its row) | +| Result | _in progress — Run 2 complete, streak 0_ | +| Gates at close of Run 2 | green — API 630 pass / 19 skip (non-DB), web 149 pass, tsc, oxlint, oxfmt, `check:loc` (all ≤1000), node-lifecycle baseline verifier. DB-gated suites + full `mise run check` (Docker/Postgres) deferred to convergence. | **Status legend:** `FIXED` = failing test + fix landed on this branch · `RESOLVED` = closed indirectly by another fix · `CATALOGUED` = confirmed/reproduced, fix @@ -41,8 +41,13 @@ did not learn about them. ## Everything fixed -6 findings. Tags: `G1` (route shadow), `N1`/`N2` (provisioning reachability), -`S1` (calendar defaults), `S4` (merge coverage), `N3` (baseline coverage). +13 findings. Run 1: `G1` (route shadow), `N1`/`N2` (provisioning reachability), +`S1` (calendar defaults), `S4` (merge coverage), `N3` (baseline coverage). Run 2: +`H4-1` (pagination re-clamp), `H3-1`/`H3-2` (stub-upload removal), `H1-1` +(recording-jobs/export shadow), `N4` (heartbeat promotion), `H3-3` (upload +destination required), `H1-2` (status filter). Run 2's route-shadow adversary +(`H1-1`) is the residual of the `G1` class — its ground-truth probe of all 369 +real routes found exactly one other shadowed static route. ### Critical @@ -50,6 +55,13 @@ did not learn about them. | --- | --- | --- | --- | --- | | G1 | 1 | node-routes/routing | `GET /api/v1/nodes/agent-release` was registered **after** `GET /api/v1/nodes/:nodeId`. The node route set mixes a static child (`/export`) with a param child (`:nodeId`) at one trie position, which Hono's `RegExpRouter` can't represent → the app falls back to the **registration-order-sensitive** `TrieRouter`, and a static route registered after `:nodeId` is swallowed by the detail handler. So every console poll of the release endpoint hit the `:nodeId` handler with `nodeId="agent-release"` → `404 Node not found`; `isAgentUpdateAvailable(current, null)` was then always false and the **entire PR #32 update-available feature was silently dead in production** (while `agent-release-routes.test.ts`, which registers the route in isolation, stayed green). Empirically reproduced through a real Hono app. Fix: register the static release route **before** the inventory routes (mirrors how `/export` already wins); made the release service injectable through `registerNodeRoutes`; added a full-surface route test (red 404 → green 200). Test fakes extracted to `node-routes-helpers.ts` for the LOC guard. | `4c5514a6` | +### High + +| ID | Run | Area | Fix | Commit | +| --- | --- | --- | --- | --- | +| H4-1 | 2 | web/pagination | `useServerPagination` reset the offset only on a filter/page-size change, never when the row total shrank below it. Deleting the last page's rows (bulk delete, retention sweep, single delete) stranded the operator on a **blank page past the end** with only "Previous" to escape, and the footer read "No results" though rows existed on page 1 — across all 7 paginated pages. Added a pure `clampedOffset(offset, limit, total)` helper (red→green) + a `clampToTotal` hook method each page calls during render with the server `meta.total` (mirroring the existing filter-reset render-time adjust). | `663798d1` | +| H3-1 | 2 | web/uploads | The recordings page never filtered the test-only stub upload policy (the controller always injects it into the policy list), so it appeared as a selectable — and default-first (`uploadPolicies[0]`) — target in the per-recording and bulk "Queue Upload" dropdowns; one click queued a real upload to the discard stub. The three settings/scheduling surfaces filtered it; recordings (a separate `["upload-policies"]` fetch) was missed. Added a `selectableUploadPolicies` helper for the two action dropdowns (the full list still labels legacy stub-tagged recordings). Red→green helper test. | `0257dbe5` | + ### Medium | ID | Run | Area | Fix | Commit | @@ -59,6 +71,16 @@ did not learn about them. | S1 | 1 | web/scheduling | The `/schedules/calendar` day-cell create built its draft with `defaultDraft(firstNode)` — no controller-settings arg — so it always fell back to the built-in profile/policies and ignored the operator's configured scheduling defaults (the list page passes them). Threaded the already-fetched `controllerSettingsQuery.data?.data` into the create draft, mirroring the list page. Page-wiring fix: the `defaultDraft(node, defaults)` behaviour it now uses is covered by `schedule-draft.test.ts`; the page call itself has no component-test seam in the web unit harness (Playwright would be the end-to-end proof), so it lands verified-by-shared-coverage + inspection rather than a dedicated red→green. | `61987116` | | S4 | 1 | settings/coverage | COVERAGE: the controller-settings `keep`-vs-`??` merge (omitted keeps, explicit `null` clears a default) was exercised only by a DB-gated test, so regressing `keep()` to `?? current` would silently stop clearing defaults with the whole in-memory suite green. Added an in-memory route test (set two defaults independently, unrelated PATCH preserves them, explicit `null` clears exactly one). Verified it goes red under the `keep→??` regression. | `a2383c3f` | | N3 | 1 | baseline/coverage | COVERAGE: the node-lifecycle baseline verifier (shipped to promote node lifecycle to ✅) never asserted PR #31's provisioning-gating invariant — its `sourceFiles` omitted `node-liveness.ts`/`watchdog-node-liveness.ts` and no phrase/snippet mentioned provisioning or the offline gate. Documented the provisioning/offline-liveness contract in the baseline doc and extended the verifier to assert the gate source (`if (node.status === "provisioning")`, `node_never_provisioned`, `nodeHeartbeatStale`, `isNodeReachable`) plus the liveness/watchdog test titles. Verifier green. | `83b2fe7f` | +| H1-1 | 2 | agent-routes/routing | The residual of the `G1` route-shadow class, cross-module: the node-auth `GET /api/v1/recording-jobs/:jobId` (registered before the operator export route) swallowed `GET /api/v1/recording-jobs/export` under TrieRouter — answering unauthenticated with a node-credential `401` (operator export "worked" only via the handler's `await next()` fallthrough, fragile). The agent handler must stay registered first (agent job-reads depend on it), so the fix defers the reserved `export` segment (`jobId === "export" → next()`) to the operator handler rather than reordering the security-sensitive `:jobId` fallthrough. Red→green in the agent-job-read harness (production registration order). | `f5b5f219` | +| N4 | 2 | node-store/availability-security | The heartbeat write persisted the agent-reported status verbatim and the schema accepted `provisioning`/`offline`, so first-contact promotion happened **only** because the shipped agent hardcodes `"online"` — and any node-credential holder could POST `status:"provisioning"` to **un-promote a live node**, which `deriveNodeStatus` short-circuits, permanently suppressing its offline alert (reachable, not just latent). Coerce a heartbeat's `provisioning`/`offline` to `online` in both stores (`heartbeatStatus`); the controller owns the lifecycle state machine. Red→green transform tests (also closes the `H2-2` promotion-coverage gap). | `75a7ca6b` | +| H3-2 | 2 | web/uploads | Editing a schedule persisted before the stub-removal loaded the stub id verbatim into the draft, but the form's policy toggles filter the stub out — leaving the operator no control to clear it, so every edit silently re-saved `["upload-policy-stub"]`. `scheduleToDraft` now strips the stub, resolving a legacy schedule to "no upload". Red→green draft test. | `0257dbe5` | +| H3-3 | 2 | uploads/validation | PR #31 intended every upload policy to target a real destination, but the create input left `destinationId` optional and the console eager-created a destination-less policy on "New" — assignable, and reconciling its recordings to `partial` (`provider_not_configured`). Enforced `destinationId` at the operator **create route** (the store stays lenient for seeds/tests, avoiding a broad fixture ripple); the console seeds the first destination and disables "New" until one exists. Red→green route test (create without a destination → `400`). | `e42fb7ec` | + +### Low + +| ID | Run | Area | Fix | Commit | +| --- | --- | --- | --- | --- | +| H1-2 | 2 | web/coverage | The node inventory status filter omitted `provisioning`, so operators could not filter to the enrolled-but-never-contacted cohort the API fully supports (`nodeStatusSchema.optional()`, exact-match filter). Added it and locked the dropdown against the full `NodeStatus` enum. Red→green. | `16f63176` | ### Also fixed (gate, not a product finding) @@ -74,8 +96,12 @@ did not learn about them. | ID | Sev | Kind | Locus | Note | | --- | --- | --- | --- | --- | -| N4-HEARTBEAT-STATUS | Med-Low | suspected | `apps/api/src/agent-route-helpers.ts`, `node-store-updates.ts` | The heartbeat schema accepts the full `nodeStatusSchema` (incl. `provisioning`/`offline`) and persists the agent-reported status verbatim, with no "first contact promotes provisioning → live" guard. A node that heartbeats `status:"provisioning"` stays gated out of offline detection **forever** (a later-dead node never flagged); one reporting `offline` while alive shows offline. **Not reachable by the shipped Rust agent** (hardcodes `status:"online"` in `inventory.rs`), so it's a latent trust-boundary gap, not a live bug. Fix recipe: on the heartbeat write, coerce a stored `provisioning` node to a live status on first contact and/or restrict the heartbeat schema to live statuses. | +| N4-HEARTBEAT-STATUS | ✅ resolved | fixed | `node-store-updates.ts`, `node-store.ts` | **RESOLVED by `N4` (Run 2, `75a7ca6b`):** confirmed reachable (a node-credential holder could POST `status:"provisioning"` to un-promote a live node and suppress its offline alert). `heartbeatStatus` now coerces a heartbeat's `provisioning`/`offline` to `online` in both stores; red→green transform tests added. | +| H4-2-NUMBER-INPUT | Medium | catalogued | recording-profile-settings-card.tsx, upload-policy-panel.tsx, watchdog-policy-card.tsx, retention-policy-panel.tsx (+ enhancement fields) | Cleared numeric inputs submit `0` (`Number("") === 0`), not empty, and the editors are not `
`s (bare `onClick` Save, so the HTML `min`/`max` are inert). A `0`/out-of-range value POSTs, the server Zod schema rejects it (`bitrateKbps` `.positive()`, `highpass.hz` `.min(20)`, `maxAttempts` `.positive().max(100)`, etc.), and the operator sees only a generic "Save failed" toast with no field-level cause. `H4-3` (retention `optionalNumber` lets `0`/`2.5` through, diverging from `.int().positive()`) is the same class. **Deferred** as a focused slice: it spans 5 editor files + a new shared `parseBoundedNumber(value, {min,max,integer})` helper, and the clamp-to-min-vs-keep-last-valid behaviour is a UX call. Bounded impact — the server correctly rejects, so no bad data persists; the defect is a confusing error, not corruption. | | S3-STALE-DEFAULT-ID | Med-Low | suspected | `apps/web/src/lib/schedule-draft.ts` (`defaultDraft`) | `defaultDraft` prefills `defaults?.defaultRecordingProfileId ?? built-in` with no existence check; the controller-settings columns are intentionally FK-free and may hold a deleted id. The ad-hoc panel (`recording-start-panel.tsx`) guards with a `.some(id === …)` existence check and falls back; `defaultDraft` does not, so a new-schedule form can prefill a dangling id. Bounded — the create API validates existence and 404s (`recording_profile_not_found` etc.), so the operator gets an opaque failed save, not corruption. Not a falsy-coalesce bug (`??`, so null/undefined fall back correctly). Fix: thread the available profile/policy lists into `defaultDraft` and guard each default like the ad-hoc panel. | +| W4A-WATCHDOG-DISPLAY-DEFAULT | Low | suspected | `apps/web/src/components/watchdog-policy-card.tsx` (`value={draft.x ?? 0.98}` fields) | The watchdog editor renders a `??`-fallback (e.g. `0.98`) for an optional threshold whose draft value is `undefined`; if the operator never touches the field, `watchdogPolicyUpdate(draft)` JSON-drops the key and the server preserves the existing DB value — so a null-column legacy policy shows "0.98" but saves unchanged. New policies seed from `defaultScheduledVoiceWatchdogPolicy` (all populated), so only legacy/null-column rows are affected, hence suspected. Fix: fold the display fallback into the draft on load (what-you-see-is-what-you-save). | +| H1-3-PROVISIONING-REASON | Low | cosmetic | `apps/api/src/node-action-routes.ts:29` (`unavailableNodeStatuses`) | `unavailableNodeStatuses = new Set(["offline"])` excludes `provisioning`, so a provisioning node's listen/meters action `reason` reads `monitor_source_unavailable`/`meter_frame_not_found` instead of a node-not-ready reason. **Not a bypass** — a provisioning node has never contacted the controller so `readiness.listen`/`.meters` are false and the action stays disabled; only the reason label is inaccurate. Optional: add `"provisioning"` to the set. | +| H2-STATUS-RAW | Low | cosmetic | `apps/web/src/pages/dashboard.tsx:191`, `room-detail.tsx:228` | These render `node.status` raw, so a `provisioning` node shows the literal string "provisioning" rather than the "Awaiting first contact" label used elsewhere (`nodes.tsx`). Cosmetic label inconsistency, no wrong data. | | N-3A-RELEASE-PAGING | Low | suspected | `apps/api/src/agent-release-service.ts:75` | The release fetch reads only `/releases?per_page=100` (page 1) and never follows `Link: rel="next"`. Because the repo interleaves `docs-v*`/`controller-v*`/`agent-v*` tags into one list, a burst of >100 non-agent releases could push the newest `agent-v…` off page 1 → a stale/null "latest". Unlikely at current cadence. Fix: follow pagination or filter server-side. | | N-3B-RELEASE-BODY | Low | suspected | `apps/api/src/agent-release-service.ts:89` | `await response.json()` reads the whole body with no size cap. Only reachable via a misconfigured `RAKKR_GITHUB_API_URL` (operator-controlled), so low severity. Fix: bound the response read. | | N-COV-FETCH | Low | coverage | `apps/api/test/agent-release-service.test.ts` | The fetch contract is correct but untested: request URL/headers (`User-Agent`, `X-GitHub-Api-Version`, `per_page=100`, `Bearer` only when a token is set), abort/timeout, and a non-`ok` (403 rate-limit) preserving the last-good value. Add assertions to lock the contract. | @@ -116,6 +142,18 @@ in pages); DataTable pagination math + stable `getRowId`; `status-colors.ts` / `node-status.ts` tone ladders; migrations 0045 (nullable default columns) and 0046 (`provisioning` enum value) vs `schema.ts` (no drift). +Additionally confirmed clean in **Run 2**: the node enroll→bootstrap→first-contact +path (`node-store-mappers.ts` maps `provisioning` faithfully both directions; +`enroll` sets `provisioning`; bootstrap-token single-use + atomic-consume; +console enroll ↔ API contract match); the `SetDefaultButton`/`useSchedulingDefault` +toggle + `["controller-settings"]` cache/invalidation (null clears round-trip +end-to-end); the ad-hoc `recording-start-panel` default prefill (existence-guarded, +empty-draft start); pagination filter-reset (value-compare) + page-size reset + +`hasNext`/`hasPrevious` server wiring; the settings-section editors' draft reset +between opens (editor unmounts on close, no stale carry-over) and create-vs-edit +shapes; and the full-app route surface (a probe of all 369 routes found `H1-1` the +only residual shadow beyond `G1`). + --- ## Run log @@ -128,7 +166,8 @@ hunters run within a run. | Run | Date | Focus | New confirmed | Fixed | Clean? | Streak | | --- | ---- | ----- | ------------- | ----- | ------ | ------ | -| 1 | 2026-07-08 | fresh PR #31/#32/#33 surface: agent-release/version (H1), node liveness/provisioning (H2), scheduling defaults (H3), web-console redesign (H4) — 4 parallel hunters | G1, N1, N2, S1 (+ S4, N3 coverage) | 6 | no | 0 | +| 1 | 2026-07-08 | fresh PR #31/#32/#33 surface: agent-release/version, node liveness/provisioning, scheduling defaults, web-console redesign — 4 parallel hunters | G1, N1, N2, S1 (+ S4, N3 coverage) | 6 | no | 0 | +| 2 | 2026-07-08 | adversary-on-Run-1-fixes + route/status class sweeps; fresh: enroll/bootstrap/heartbeat, set-default + stub-upload removal, data-table/settings-dialog correctness — 4 parallel hunters | H4-1, H3-1, H3-2, H1-1, N4, H3-3 (+ H1-2 coverage) | 7 | no | 0 | ### Notes on the runs @@ -147,3 +186,19 @@ hunters run within a run. `N-3A-RELEASE-PAGING`, `N-3B-RELEASE-BODY`, `N-COV-FETCH`, `S2-DEAD-EXPORT`, `W1-DEAD-ROW-SELECT`, `W2-OBSERVER-DEP`, `W3-EMPTY-CARD-FOOTER`, `W4-CARD-TITLE`. Dirty (landed 6 fixes) → streak 0. +- **Run 2** turned one hunter adversarially on Run 1's fixes (all three verified + clean via a ground-truth probe of the real app's 369 routes) and three at fresh + ground. The route-shadow class sweep found its **one** residual (`H1-1`, + `/recording-jobs/export`); the `isNodeReachable`/provisioning class sweep found + the sites already unified. Fresh hunters surfaced two HIGH web bugs — `H4-1` + (pagination stranding on a blank page when the total shrinks) and `H3-1` (the + stub upload policy selectable on the recordings page) — plus the confirmation + that `N4` (heartbeat can un-promote a live node to `provisioning`) is **reachable** + (elevated from Run 1's catalogue and fixed), `H3-2` (legacy schedule silently + re-saves the stub), and `H3-3` (destination-less upload policy persists). `H1-2` + added the missing `provisioning` filter option. New **suspected/catalogued** + leads logged, not fixed: `H4-2-NUMBER-INPUT` (+ `H4-3` retention, same class, + Medium — deferred multi-file slice), `W4A-WATCHDOG-DISPLAY-DEFAULT`, + `H1-3-PROVISIONING-REASON`, `H2-STATUS-RAW` (cosmetic). The Run-1 web-cosmetic + catalogue (`W1`–`W4`) and release-robustness leads (`N-3A`/`N-3B`/`N-COV-FETCH`) + and `S2`/`S3` remain open. Dirty (landed 7 fixes) → streak 0. From 38bf6de1e123b09a0238ae111aa699546b3a95d0 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 14:37:53 +0500 Subject: [PATCH 14/33] Cap and dedup schedule uploadPolicyIds (audit R3-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each schedule upload policy id fans a recording out to its own upload queue item, but the list was uncapped and un-deduped — a schedule:manage holder could multiply queue work per recording with a long/duplicate list (the sibling switcher-mappings list already caps at 256). Cap the schedule create/update schemas at 32 and dedup the client draft before submit. Red->green dedup test. Co-Authored-By: Claude Opus 4.8 --- apps/web/src/lib/schedule-draft.test.ts | 11 +++++++++++ apps/web/src/lib/schedule-draft.ts | 2 +- packages/shared/src/index.ts | 6 ++++-- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/apps/web/src/lib/schedule-draft.test.ts b/apps/web/src/lib/schedule-draft.test.ts index 2ee1c0eb..58a04ac5 100644 --- a/apps/web/src/lib/schedule-draft.test.ts +++ b/apps/web/src/lib/schedule-draft.test.ts @@ -163,6 +163,17 @@ test("scheduleToDraft drops a legacy stub upload policy so it is not silently re assert.deepEqual(draft.uploadPolicyIds, ["up_real"]); }); +test("draftToInput dedups uploadPolicyIds so a recording fans out once per destination", () => { + const input = draftToInput({ + ...defaultDraft(), + name: "Dedup Capture", + nodeId: "node_dedup", + uploadPolicyIds: ["up_a", "up_a", "up_b", "up_b"], + }); + + assert.deepEqual(input.uploadPolicyIds, ["up_a", "up_b"]); +}); + test("schedule draft pins a sorted channel selection only with an interface", () => { const withInterface = draftToInput({ ...defaultDraft(), diff --git a/apps/web/src/lib/schedule-draft.ts b/apps/web/src/lib/schedule-draft.ts index ee2c57be..f7367e44 100644 --- a/apps/web/src/lib/schedule-draft.ts +++ b/apps/web/src/lib/schedule-draft.ts @@ -162,7 +162,7 @@ export function draftToInput(draft: ScheduleDraft): ScheduleInput { tags: uniqueTags(draft.tags), timezone: draft.timezone, titleTemplate: draft.titleTemplate, - uploadPolicyIds: draft.uploadPolicyIds, + uploadPolicyIds: [...new Set(draft.uploadPolicyIds)], watchdogPolicyId: draft.watchdogPolicyId, }; } diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 57c5225a..317e8e45 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -565,7 +565,9 @@ export const scheduleInputSchema = z.object({ tags: z.array(z.string().trim().min(1).max(80)).max(64).default([]), timezone: ianaTimeZoneSchema, titleTemplate: z.string().trim().min(1).max(500), - uploadPolicyIds: z.array(z.string().trim().min(1).max(160)).default([]), + // Capped + deduped on write: each recording fans out to one upload queue item + // per id, so an uncapped list multiplies queue work per recording (audit R3-4). + uploadPolicyIds: z.array(z.string().trim().min(1).max(160)).max(32).default([]), watchdogPolicyId: z.string().trim().min(1).max(160), }); export const scheduleUpdateSchema = z @@ -589,7 +591,7 @@ export const scheduleUpdateSchema = z tags: z.array(z.string().trim().min(1).max(80)).max(64).optional(), timezone: ianaTimeZoneSchema.optional(), titleTemplate: z.string().trim().min(1).max(500).optional(), - uploadPolicyIds: z.array(z.string().trim().min(1).max(160)).optional(), + uploadPolicyIds: z.array(z.string().trim().min(1).max(160)).max(32).optional(), watchdogPolicyId: z.string().trim().min(1).max(160).optional(), }) .refine((value) => Object.keys(value).length > 0, "At least one schedule field is required"); From d5b68136a1664a14c0e48ed62f07576973e29a66 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 14:37:54 +0500 Subject: [PATCH 15/33] Lock upload-policy update against clearing the destination (audit H3-3 coverage) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The create route rejects a destination-less policy; add the matching coverage on the update path — an empty destinationId is schema-rejected (400) and an omitted one is preserved — so a future nullable/empty-allowed change to the update schema can't silently reopen the H3-3 hole. Co-Authored-By: Claude Opus 4.8 --- ...ettings-upload-policy-scope-routes.test.ts | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/apps/api/test/settings-upload-policy-scope-routes.test.ts b/apps/api/test/settings-upload-policy-scope-routes.test.ts index 68dc6d2d..0d7a8bc5 100644 --- a/apps/api/test/settings-upload-policy-scope-routes.test.ts +++ b/apps/api/test/settings-upload-policy-scope-routes.test.ts @@ -257,6 +257,55 @@ test("H3-3: upload policy create without a destination is rejected", async () => assert.equal(response.status, 400); }); +test("H3-3: upload policy update cannot clear an existing destination", async () => { + const app = new Hono(); + const auditStore = createAuditStore(""); + const currentUser = viewer(); + const uploadDestinationStore = createUploadDestinationStore(); + const destination = await uploadDestinationStore.create({ + displayName: "Update Test Destination", + enabled: true, + kind: "smb", + smb: { server: "upd.lan", share: "recordings", username: "svc" }, + smbPassword: "s3cr3t", + }); + + registerSettingsRoutes({ + app, + currentAuth: () => ({ user: currentUser }), + hasResourceScope: async () => true, + recordAuditEvent: recordAuditEvent(auditStore), + requirePermission: denyResourceScope(auditStore, currentUser, () => true), + settingsStore: createSettingsStore(), + uploadDestinationStore, + }); + + const policyId = `upload_policy_update_${randomUUID()}`; + const created = await requestJson(app, "/api/v1/settings/upload-policies", "POST", { + destinationId: destination.id, + enabled: true, + id: policyId, + maxAttempts: 3, + name: "Update Target", + trigger: "manual", + }); + // The update schema keeps destinationId non-nullable `.min(1)`, so an empty + // value is rejected and an omitted one is preserved — an existing policy can + // never be made destination-less on the update path (audit H3-3 coverage). + const cleared = await requestJson(app, `/api/v1/settings/upload-policies/${policyId}`, "PATCH", { + destinationId: "", + }); + const renamed = await requestJson(app, `/api/v1/settings/upload-policies/${policyId}`, "PATCH", { + name: "Renamed", + }); + const stored = await findUploadPolicy(policyId); + + assert.equal(created.status, 201); + assert.equal(cleared.status, 400); + assert.equal(renamed.status, 200); + assert.equal(stored?.destinationId, destination.id); +}); + function requestJson( app: Hono, url: string, From 8444b71a3490b583a64a5fffb701684fc27559a0 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 14:39:40 +0500 Subject: [PATCH 16/33] Update gap-hunt ledger: Run 3 (2 changes, streak 0) Co-Authored-By: Claude Opus 4.8 --- docs/internal/audits/2026-07-08-gap-hunt.md | 46 +++++++++++++++++---- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/docs/internal/audits/2026-07-08-gap-hunt.md b/docs/internal/audits/2026-07-08-gap-hunt.md index c4983bdf..bc25816a 100644 --- a/docs/internal/audits/2026-07-08-gap-hunt.md +++ b/docs/internal/audits/2026-07-08-gap-hunt.md @@ -15,10 +15,10 @@ primary target. Target: **5 consecutive clean runs.** | Base | rebased onto `origin/main` `619b6f10` (static for the audit so far) | | Started | 2026-07-08 | | Completed | _in progress_ | -| Runs | 2 so far — **streak 0** (Run 1 landed 6 fixes, Run 2 landed 7 → both dirty) | -| Findings closed | 13 (Run 1: `G1`,`N1`,`N2`,`S1`,`S4`,`N3` · Run 2: `H4-1`,`H3-1`,`H3-2`,`H1-1`,`N4`,`H3-3`,`H1-2`) — each red→green except the page-wiring `S1` (see its row) | -| Result | _in progress — Run 2 complete, streak 0_ | -| Gates at close of Run 2 | green — API 630 pass / 19 skip (non-DB), web 149 pass, tsc, oxlint, oxfmt, `check:loc` (all ≤1000), node-lifecycle baseline verifier. DB-gated suites + full `mise run check` (Docker/Postgres) deferred to convergence. | +| Runs | 3 so far — **streak 0** (Run 1: 6 fixes, Run 2: 7, Run 3: 2 → all dirty) | +| Findings closed | 15 (Run 1: `G1`,`N1`,`N2`,`S1`,`S4`,`N3` · Run 2: `H4-1`,`H3-1`,`H3-2`,`H1-1`,`N4`,`H3-3`,`H1-2` · Run 3: `R3-4`, `H3-3-UPDATE-COV`) — each red→green except the page-wiring `S1` and the coverage locks (see rows) | +| Result | _in progress — Run 3 complete, streak 0_ | +| Gates at close of Run 3 | green — API 631 pass / 19 skip (non-DB), web 150 pass, tsc, oxlint, oxfmt, `check:loc` (all ≤1000), node-lifecycle + storage/recordings/time/switcher/rbac/watchdog/settings/operations/scheduler baseline verifiers. DB-gated suites + full `mise run check` (Docker/Postgres) deferred to convergence. | **Status legend:** `FIXED` = failing test + fix landed on this branch · `RESOLVED` = closed indirectly by another fix · `CATALOGUED` = confirmed/reproduced, fix @@ -41,13 +41,15 @@ did not learn about them. ## Everything fixed -13 findings. Run 1: `G1` (route shadow), `N1`/`N2` (provisioning reachability), +15 findings. Run 1: `G1` (route shadow), `N1`/`N2` (provisioning reachability), `S1` (calendar defaults), `S4` (merge coverage), `N3` (baseline coverage). Run 2: `H4-1` (pagination re-clamp), `H3-1`/`H3-2` (stub-upload removal), `H1-1` (recording-jobs/export shadow), `N4` (heartbeat promotion), `H3-3` (upload -destination required), `H1-2` (status filter). Run 2's route-shadow adversary -(`H1-1`) is the residual of the `G1` class — its ground-truth probe of all 369 -real routes found exactly one other shadowed static route. +destination required), `H1-2` (status filter). Run 3: `R3-4` (uploadPolicyIds +cap/dedup) + a coverage lock. Run 2's route-shadow adversary (`H1-1`) is the +residual of the `G1` class — its ground-truth probe of all 369 real routes found +exactly one other shadowed static route. Run 3's adversary verified all Run-2 +fixes clean (no regressions) and surfaced only LOW items. ### Critical @@ -81,6 +83,8 @@ real routes found exactly one other shadowed static route. | ID | Run | Area | Fix | Commit | | --- | --- | --- | --- | --- | | H1-2 | 2 | web/coverage | The node inventory status filter omitted `provisioning`, so operators could not filter to the enrolled-but-never-contacted cohort the API fully supports (`nodeStatusSchema.optional()`, exact-match filter). Added it and locked the dropdown against the full `NodeStatus` enum. Red→green. | `16f63176` | +| R3-4 | 3 | schedule/hardening | Each schedule upload-policy id fans a recording out to its own upload queue item, but `uploadPolicyIds` was uncapped and un-deduped on write (the sibling switcher-mappings list caps at 256) — a `schedule:manage` holder could multiply queue work per recording. Capped the schedule create/update schemas at 32 (read-back schema left uncapped so pre-existing data still parses) and dedup the client draft before submit. Red→green dedup test. | `38bf6de1` | +| H3-3-UPDATE-COV | 3 | uploads/coverage | COVERAGE: the `H3-3` create-route destination requirement had no matching lock on the update path. Added a test proving an empty `destinationId` PATCH is schema-rejected (`400`) and an omitted one preserves the existing destination — so a future `.nullable()`/empty-allowed change to the update schema can't silently reopen the hole. | `d5b68136` | ### Also fixed (gate, not a product finding) @@ -110,10 +114,18 @@ real routes found exactly one other shadowed static route. | W3-EMPTY-CARD-FOOTER | Low | confirmed | `apps/web/src/components/ui/data-table.tsx:229,272-278` + `nodes.tsx` | `nodes.tsx` keeps the actions column always present and returns `null` inside its cell when `!canManage`, so on mobile `DataTableCard` renders an empty bordered actions footer for read-only users. Other pages omit the column entirely (`if (canControl) columns.push(...)`). Playwright-only proof. Fix: make nodes.tsx omit the actions column when `!canManage` (mirror the other pages), or have the card skip an all-null footer. | | W2-OBSERVER-DEP | Low | suspected | `apps/web/src/components/ui/truncate-cell.tsx:17-38` | `useLayoutEffect` has no dependency array, so every render tears down + recreates the `ResizeObserver` and re-measures. No leak (cleanup disconnects), but continuous churn on dense polling tables. Fix: depend on `[children]`. | | W4-CARD-TITLE | Low | suspected | `apps/web/src/components/ui/data-table.tsx:233` + jobs/health | `DataTableCard` uses the first non-utility field cell as the card title; in jobs/health that's a `status`/`severity` Badge, so each mobile card's headline is a bare colored word and the identifying name is one row down. UX, not wrong data. Fix: order the identifying column first in those tables, or let a column opt into the card title. | +| R3-1-OFFLINE-TONE | Low-Med | product | `apps/web/src/lib/node-status.ts:5-28` (`nodeStatusBadgeClass`) | Every node status has an explicit tone branch except **`offline`**, which falls through to the `neutral` (muted gray) default — so an unreachable node's status pill looks the same as an unknown/nothing state, arguably calmer than `degraded` (`warning`) or `alerting` (`critical`). Likely an omission (the `provisioning` branch was added in PR #31; `offline` was left implicit), but muted-for-offline is a defensible design too, so it's a **product call on the tone** (`warning` vs `critical`), not a clear bug — catalogued for the operator to pick rather than imposing a visible UX change. Fix: add an explicit `offline` branch + a `node-status.test.ts` pinning every enum member. | +| R3-2-TONEFILL-NEUTRAL | Low | by-design | `apps/web/src/lib/status-colors.ts:84-98` (`toneFillClass`) | `toneFillClass` falls through to the sky/info style for **both** `info` and `neutral`, so a `neutral` argument would render as blue "info". **Latent** — the only caller (`quality-timeline.tsx`) never passes `neutral`. Same missing-case-fallthrough class as `R3-1`. Fix if touched: give `neutral` an explicit gray branch or narrow the param type. | +| R3-3-SWITCHER-PW-CLEAR | Low | product | `apps/web/src/components/settings-switchers-section.tsx:666` (`buildUpdate`) | The console cannot clear a stored switcher control-channel password: the edit form never loads the stored secret (`initialDraft` sets `password:""`) and `buildUpdate` omits an empty password (so blank = "keep"), but the server treats `password:""` as "clear" (`switcher-store.ts`). Blank cannot mean both keep and clear, so the fix needs a **UI affordance** (a password "dirty" flag or an explicit "clear password" control) — a product decision, not a one-liner (the naive "always send the empty string" would wipe the password on any unrelated edit). **Current impact is nil**: the only shipped model (AVPro AC-MAX) has `requiresLogin:false` and ignores the password. Deferred pending the affordance design. | +| R3-5-DEFAULT-ID-WRITE | Low | by-design | `apps/api/src/settings-controller-routes.ts` + `controller-settings-store.ts` | The four scheduling-default id columns accept any non-empty string on write with no existence check, so an operator can persist a default pointing at a nonexistent policy. This is **by design** — `schema.ts` documents the columns as intentionally FK-free ("a referenced policy may be deleted, and the forms tolerate a stale id"); the graceful-fallback belongs in the forms (the ad-hoc panel already guards; the schedule form is the catalogued `S3-STALE-DEFAULT-ID`). Folded into `S3`; no separate write-time validation planned. | +| R3-6-BUILDER-COV | Low | coverage | `settings-switchers-section.tsx`, `settings-upload-destinations-section.tsx` (`buildCreate`/`buildUpdate`) | The switcher + upload-destination dialog payload builders are non-exported functions with no unit test, which is why `R3-3` was invisible to the suite. Extract them to a `*-helpers.ts` module and unit-test create-vs-update shape parity (mirrors `schedule-draft.ts`). Deferred with the `R3-3` affordance work. | +| H4-1-CLAMP-HOOK-COV | Medium | coverage | `apps/web/src/lib/use-server-pagination.ts` | The `clampToTotal` **hook** behavior (render-time `setOffset`, filter-reset interaction, stale-`keepPreviousData` total) is untested — only the pure `clampedOffset` is. The adversary verified the behavior is correct (converges in one step, never fights the filter-reset, Next is server-gated), but locking it needs a `renderHook` test, which the web unit harness (`tsx --test`, no `@testing-library/react`/jsdom) cannot run today. Deferred pending a React test-harness addition (would also unblock `W1`/`W3`/`W4`). | ### Coverage gaps (no bug — missing tests) -- `N-COV-FETCH` (above) — GitHub fetch request/abort/rate-limit contract. +- `N-COV-FETCH` — GitHub fetch request/abort/rate-limit contract. +- `R3-6-BUILDER-COV` — switcher/destination dialog builders (private, untested). +- `H4-1-CLAMP-HOOK-COV` — the pagination hook's render-time clamp (needs a React render harness). - Web `data-table.tsx` / `DataTableCard` have **no** tests at all (no RTL harness); `W1`/`W3`/`W4` are only catchable by Playwright today. --- @@ -168,6 +180,7 @@ hunters run within a run. | --- | ---- | ----- | ------------- | ----- | ------ | ------ | | 1 | 2026-07-08 | fresh PR #31/#32/#33 surface: agent-release/version, node liveness/provisioning, scheduling defaults, web-console redesign — 4 parallel hunters | G1, N1, N2, S1 (+ S4, N3 coverage) | 6 | no | 0 | | 2 | 2026-07-08 | adversary-on-Run-1-fixes + route/status class sweeps; fresh: enroll/bootstrap/heartbeat, set-default + stub-upload removal, data-table/settings-dialog correctness — 4 parallel hunters | H4-1, H3-1, H3-2, H1-1, N4, H3-3 (+ H1-2 coverage) | 7 | no | 0 | +| 3 | 2026-07-08 | adversary-on-Run-2-fixes (all verified clean) + fresh: data-table/agent-version render correctness, cross-cutting schema-bounds + switcher/channel-map/destination dialogs — 3 parallel hunters | R3-4 (+ H3-3-update coverage) | 2 | no | 0 | ### Notes on the runs @@ -202,3 +215,18 @@ hunters run within a run. `H1-3-PROVISIONING-REASON`, `H2-STATUS-RAW` (cosmetic). The Run-1 web-cosmetic catalogue (`W1`–`W4`) and release-robustness leads (`N-3A`/`N-3B`/`N-COV-FETCH`) and `S2`/`S3` remain open. Dirty (landed 7 fixes) → streak 0. +- **Run 3** turned one hunter adversarially on Run 2's 7 fixes (all **verified + clean** — no regressions: the pagination clamp converges in one step and never + fights the filter-reset, the `N4` heartbeat coercion can't hide a genuinely + stale node since `deriveNodeStatus` re-derives offline at read time, the stub + removal is complete across every selectable consumer, and the route-shadow class + has only the two known instances) and two at fresh ground (data-table/agent-version + render correctness; cross-cutting schema-bounds + the switcher/channel-map/ + upload-destination dialogs). **No confirmed high/critical bug** surfaced — only + LOW items. Landed `R3-4` (uploadPolicyIds cap+dedup) and a coverage lock on the + `H3-3` update path. New **suspected/catalogued/product** leads logged, not fixed: + `R3-1-OFFLINE-TONE` (product call on the offline status tone), + `R3-2-TONEFILL-NEUTRAL` (latent), `R3-3-SWITCHER-PW-CLEAR` (needs a UI + clear-affordance; nil current impact), `R3-5-DEFAULT-ID-WRITE` (by-design, folded + into `S3`), `R3-6-BUILDER-COV`, `H4-1-CLAMP-HOOK-COV` (needs a React render + harness). Dirty (landed 2 changes) → streak 0. From c2f90f5dae5984a313d9859dbc89cc1e8588cb4b Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 14:51:01 +0500 Subject: [PATCH 17/33] Update gap-hunt ledger: Run 4 clean (streak 1/5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run 4 convergence attempt: adversary-on-Run-3 + completeness critic (all 85 changed files), data-integrity/concurrency, permission-boundary sweeps — zero code changes; db:verify migration replay green. R4-1/R4-2 logged as suspected/mitigated. First clean run. Co-Authored-By: Claude Opus 4.8 --- docs/internal/audits/2026-07-08-gap-hunt.md | 26 ++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/docs/internal/audits/2026-07-08-gap-hunt.md b/docs/internal/audits/2026-07-08-gap-hunt.md index bc25816a..2fad106e 100644 --- a/docs/internal/audits/2026-07-08-gap-hunt.md +++ b/docs/internal/audits/2026-07-08-gap-hunt.md @@ -15,10 +15,10 @@ primary target. Target: **5 consecutive clean runs.** | Base | rebased onto `origin/main` `619b6f10` (static for the audit so far) | | Started | 2026-07-08 | | Completed | _in progress_ | -| Runs | 3 so far — **streak 0** (Run 1: 6 fixes, Run 2: 7, Run 3: 2 → all dirty) | +| Runs | 4 so far — **streak 1** (Runs 1–3 dirty: 6+7+2 fixes; **Run 4 clean**) | | Findings closed | 15 (Run 1: `G1`,`N1`,`N2`,`S1`,`S4`,`N3` · Run 2: `H4-1`,`H3-1`,`H3-2`,`H1-1`,`N4`,`H3-3`,`H1-2` · Run 3: `R3-4`, `H3-3-UPDATE-COV`) — each red→green except the page-wiring `S1` and the coverage locks (see rows) | -| Result | _in progress — Run 3 complete, streak 0_ | -| Gates at close of Run 3 | green — API 631 pass / 19 skip (non-DB), web 150 pass, tsc, oxlint, oxfmt, `check:loc` (all ≤1000), node-lifecycle + storage/recordings/time/switcher/rbac/watchdog/settings/operations/scheduler baseline verifiers. DB-gated suites + full `mise run check` (Docker/Postgres) deferred to convergence. | +| Result | _in progress — Run 4 clean, **streak 1 of 5**_ | +| Gates at close of Run 4 | green — API 631 pass / 19 skip (non-DB), web 150 pass, tsc, oxlint, oxfmt, `check:loc` (all ≤1000), node-lifecycle + storage/recordings/time/switcher/rbac/watchdog/settings/operations/scheduler baselines, **and `db:verify` (Drizzle migration replay 0001–0046 against throwaway Postgres, exit 0 — confirms 0046 `ALTER TYPE ADD VALUE 'provisioning'` replays cleanly)**. Full `mise run check` + DB-gated API suite deferred to the final clean run. | **Status legend:** `FIXED` = failing test + fix landed on this branch · `RESOLVED` = closed indirectly by another fix · `CATALOGUED` = confirmed/reproduced, fix @@ -120,6 +120,8 @@ fixes clean (no regressions) and surfaced only LOW items. | R3-5-DEFAULT-ID-WRITE | Low | by-design | `apps/api/src/settings-controller-routes.ts` + `controller-settings-store.ts` | The four scheduling-default id columns accept any non-empty string on write with no existence check, so an operator can persist a default pointing at a nonexistent policy. This is **by design** — `schema.ts` documents the columns as intentionally FK-free ("a referenced policy may be deleted, and the forms tolerate a stale id"); the graceful-fallback belongs in the forms (the ad-hoc panel already guards; the schedule form is the catalogued `S3-STALE-DEFAULT-ID`). Folded into `S3`; no separate write-time validation planned. | | R3-6-BUILDER-COV | Low | coverage | `settings-switchers-section.tsx`, `settings-upload-destinations-section.tsx` (`buildCreate`/`buildUpdate`) | The switcher + upload-destination dialog payload builders are non-exported functions with no unit test, which is why `R3-3` was invisible to the suite. Extract them to a `*-helpers.ts` module and unit-test create-vs-update shape parity (mirrors `schedule-draft.ts`). Deferred with the `R3-3` affordance work. | | H4-1-CLAMP-HOOK-COV | Medium | coverage | `apps/web/src/lib/use-server-pagination.ts` | The `clampToTotal` **hook** behavior (render-time `setOffset`, filter-reset interaction, stale-`keepPreviousData` total) is untested — only the pure `clampedOffset` is. The adversary verified the behavior is correct (converges in one step, never fights the filter-reset, Next is server-gated), but locking it needs a `renderHook` test, which the web unit harness (`tsx --test`, no `@testing-library/react`/jsdom) cannot run today. Deferred pending a React test-harness addition (would also unblock `W1`/`W3`/`W4`). | +| R4-1-SERVER-DEDUP | Low | suspected (mitigated) | `apps/api/src/schedule-route-helpers.ts` (create/update input builders) | The `R3-4` dedup of `uploadPolicyIds` lives only in the web client (`draftToInput`) + the `.max(32)` cap; the server persists duplicates verbatim (unlike sibling `tags`, deduped via `uniqueTags`). A non-browser API client could store `["x","x"]`. **Fully mitigated downstream** — the upload fan-out (`uploadPoliciesForChunkedRecording`) dedups by destination target, so duplicates collapse to one queue item (no multiplied work); the only residual is a cosmetic duplicate in the stored row / schedule-detail display. Two hunters (data-integrity + completeness critic) judged it a non-defect / pre-existing. Fix if touched: mirror `uniqueTags` server-side (or add `.transform(v => [...new Set(v)])` to the shared schema) so the server is authoritative. | +| R4-2-LIVENESS-ISOLATION | Low | suspected (mitigated) | `apps/api/src/watchdog-node-liveness.ts:34-69` (`reconcileNodeLivenessEvents`) | The per-node reconcile loop has no per-node try/catch, so a throw for one node skips the rest of that tick's nodes. **Mitigated**: `HealthEventStore` create/update/list swallow DB errors and fail over to memory (don't throw), so the only throw source is `auditStore.append`; and the watchdog tick is non-latching (`watchdog-runner` resets `running` in `finally`, wraps in `.catch`), so a throw = one skipped tick, retried next interval — no wedged state. Fix if touched: wrap the per-node body in try/catch → `{ outcome: "skipped", reason: "reconcile_failed" }` and continue (mirror the recording-watchdog families' independence). | ### Coverage gaps (no bug — missing tests) @@ -181,6 +183,7 @@ hunters run within a run. | 1 | 2026-07-08 | fresh PR #31/#32/#33 surface: agent-release/version, node liveness/provisioning, scheduling defaults, web-console redesign — 4 parallel hunters | G1, N1, N2, S1 (+ S4, N3 coverage) | 6 | no | 0 | | 2 | 2026-07-08 | adversary-on-Run-1-fixes + route/status class sweeps; fresh: enroll/bootstrap/heartbeat, set-default + stub-upload removal, data-table/settings-dialog correctness — 4 parallel hunters | H4-1, H3-1, H3-2, H1-1, N4, H3-3 (+ H1-2 coverage) | 7 | no | 0 | | 3 | 2026-07-08 | adversary-on-Run-2-fixes (all verified clean) + fresh: data-table/agent-version render correctness, cross-cutting schema-bounds + switcher/channel-map/destination dialogs — 3 parallel hunters | R3-4 (+ H3-3-update coverage) | 2 | no | 0 | +| 4 | 2026-07-08 | convergence: adversary-on-Run-3-fixes + completeness critic (all 85 changed files enumerated), fresh data-integrity/concurrency, fresh web permission-boundary re-sweep — 3 parallel hunters; `db:verify` migration replay run green | — (R4-1, R4-2 suspected/mitigated, catalogued) | 0 | **yes** | **1** | ### Notes on the runs @@ -230,3 +233,20 @@ hunters run within a run. clear-affordance; nil current impact), `R3-5-DEFAULT-ID-WRITE` (by-design, folded into `S3`), `R3-6-BUILDER-COV`, `H4-1-CLAMP-HOOK-COV` (needs a React render harness). Dirty (landed 2 changes) → streak 0. +- **Run 4 — first clean run (streak 1).** A dedicated convergence attempt: one + hunter adversarially re-verified all three Run-3 fixes (`R3-4`, `H3-3-UPDATE-COV`, + migration 0046 — all clean) and ran a completeness critic enumerating **all 85 + changed files** across the three PRs, confirming every previously-unswept file + now has coverage with no outstanding real bug; a second swept data-integrity/ + concurrency on the new stores/runners/services (agent-release cache dedup, + controller-settings JSON persist atomicity, upload-queue transitions, node + heartbeat locking — all verified guarded); a third re-swept web permission + boundaries across every restructured component + the enroll/bootstrap flow (all + mutating controls correctly gated; no secret leak; no `R27`-style hardcode). I + also ran `db:verify` (Drizzle replay 0001–0046 against a fresh throwaway + Postgres, exit 0). **Zero code/test changes.** The only leads were `R4-1` + (server-side `uploadPolicyIds` dedup — fully mitigated downstream, judged a + non-defect by two hunters) and `R4-2` (liveness per-node error isolation — + suspected, mitigated by store-failover + non-latching tick); both catalogued as + suspected/mitigated, listed here per the clean-run rule. No confirmed functional + bug and no actionable coverage gap required a change → **clean, streak 1**. From ad20d3d4e6c47bcb019e35de097b2355ac2a9513 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 14:59:18 +0500 Subject: [PATCH 18/33] Update gap-hunt ledger: Run 5 clean (streak 2/5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run 5 convergence (fresh angles): end-to-end feature-trace, deploy/ops + docs/baseline consistency, observability/error-surface — all clean, zero code changes. node:test-db 22/22 green. Second consecutive clean run. Co-Authored-By: Claude Opus 4.8 --- docs/internal/audits/2026-07-08-gap-hunt.md | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/docs/internal/audits/2026-07-08-gap-hunt.md b/docs/internal/audits/2026-07-08-gap-hunt.md index 2fad106e..43dfbe14 100644 --- a/docs/internal/audits/2026-07-08-gap-hunt.md +++ b/docs/internal/audits/2026-07-08-gap-hunt.md @@ -15,9 +15,9 @@ primary target. Target: **5 consecutive clean runs.** | Base | rebased onto `origin/main` `619b6f10` (static for the audit so far) | | Started | 2026-07-08 | | Completed | _in progress_ | -| Runs | 4 so far — **streak 1** (Runs 1–3 dirty: 6+7+2 fixes; **Run 4 clean**) | +| Runs | 5 so far — **streak 2** (Runs 1–3 dirty: 6+7+2 fixes; **Runs 4–5 clean**) | | Findings closed | 15 (Run 1: `G1`,`N1`,`N2`,`S1`,`S4`,`N3` · Run 2: `H4-1`,`H3-1`,`H3-2`,`H1-1`,`N4`,`H3-3`,`H1-2` · Run 3: `R3-4`, `H3-3-UPDATE-COV`) — each red→green except the page-wiring `S1` and the coverage locks (see rows) | -| Result | _in progress — Run 4 clean, **streak 1 of 5**_ | +| Result | _in progress — Run 5 clean, **streak 2 of 5**_ | | Gates at close of Run 4 | green — API 631 pass / 19 skip (non-DB), web 150 pass, tsc, oxlint, oxfmt, `check:loc` (all ≤1000), node-lifecycle + storage/recordings/time/switcher/rbac/watchdog/settings/operations/scheduler baselines, **and `db:verify` (Drizzle migration replay 0001–0046 against throwaway Postgres, exit 0 — confirms 0046 `ALTER TYPE ADD VALUE 'provisioning'` replays cleanly)**. Full `mise run check` + DB-gated API suite deferred to the final clean run. | **Status legend:** `FIXED` = failing test + fix landed on this branch · `RESOLVED` @@ -184,6 +184,7 @@ hunters run within a run. | 2 | 2026-07-08 | adversary-on-Run-1-fixes + route/status class sweeps; fresh: enroll/bootstrap/heartbeat, set-default + stub-upload removal, data-table/settings-dialog correctness — 4 parallel hunters | H4-1, H3-1, H3-2, H1-1, N4, H3-3 (+ H1-2 coverage) | 7 | no | 0 | | 3 | 2026-07-08 | adversary-on-Run-2-fixes (all verified clean) + fresh: data-table/agent-version render correctness, cross-cutting schema-bounds + switcher/channel-map/destination dialogs — 3 parallel hunters | R3-4 (+ H3-3-update coverage) | 2 | no | 0 | | 4 | 2026-07-08 | convergence: adversary-on-Run-3-fixes + completeness critic (all 85 changed files enumerated), fresh data-integrity/concurrency, fresh web permission-boundary re-sweep — 3 parallel hunters; `db:verify` migration replay run green | — (R4-1, R4-2 suspected/mitigated, catalogued) | 0 | **yes** | **1** | +| 5 | 2026-07-08 | convergence (fresh angles): end-to-end feature-trace (all 4 PR features through every layer), deploy/ops + docs/baseline consistency, observability/audit/error-surface — 3 parallel hunters; `node:test-db` (22 DB-backed concurrency/atomicity tests) run green | — (no new leads) | 0 | **yes** | **2** | ### Notes on the runs @@ -250,3 +251,17 @@ hunters run within a run. suspected, mitigated by store-failover + non-latching tick); both catalogued as suspected/mitigated, listed here per the clean-run rule. No confirmed functional bug and no actionable coverage gap required a change → **clean, streak 1**. +- **Run 5 — clean (streak 2).** Second convergence pass with three *fresh angles* + (not a replay of Run 4): (a) end-to-end feature tracing — each of the four PR + features (provisioning lifecycle, agent update-available, scheduling defaults, + stub-upload removal) followed through every layer for a cross-layer seam break, + all clean (notably the bare-version-vs-`agent-v`-tag seam and the single→list + `defaultUploadPolicyId` transform); (b) deploy/ops + docs/baseline consistency — + the `update_binary` tag chain, the Node-lifecycle-✅ SoT promotion (backed by the + wired `nodes:check-lifecycle` verifier), doc-vs-behavior, and migration/CI wiring + all consistent; (c) observability/audit/error-surface — metrics gauges correct + for `provisioning`, label escaping complete, audit events + error surfaces clean, + no secret leak. I also ran `node:test-db` (22 DB-backed concurrency/atomicity + tests, all green) against a throwaway Postgres, confirming the `N4` Postgres-store + change and the upload-queue/credential/room-delete races. **Zero code/test + changes; no new leads.** → **clean, streak 2**. From 63f8629351f2a550f5eea7d56b0d2ae077f8172e Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 15:14:05 +0500 Subject: [PATCH 19/33] Stop cleared watchdog numeric fields from persisting 0 (audit H4-2 re-triage) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run-6 re-triage found H4-2 was mis-classified as cosmetic for the watchdog editor: clearing a numeric field yields Number("")===0, and thresholdDbfs (dbfsSchema [-160,24]) and the score thresholds ([0,1]) ACCEPT 0 — so a cleared threshold silently persists 0 and arms an always-fire alert (watchdog-signal fires low-signal for all healthy audio), flooding the operator with spurious critical health events. Add a shared numericInputCommit helper (empty/invalid -> no commit, never 0) + a local text buffer in the shared NumberField (keeps the field editable while typing) so all 16 watchdog numeric fields are fixed at one point. Red->green helper test. Other editors' clear->0 is server-rejected (no persist) and stays the catalogued cosmetic. Co-Authored-By: Claude Opus 4.8 --- .../src/components/watchdog-policy-card.tsx | 29 +++++++++++++++++-- apps/web/src/lib/settings-updates.test.ts | 16 +++++++++- apps/web/src/lib/settings-updates.ts | 16 ++++++++++ 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/apps/web/src/components/watchdog-policy-card.tsx b/apps/web/src/components/watchdog-policy-card.tsx index 4ba377da..034c62ce 100644 --- a/apps/web/src/components/watchdog-policy-card.tsx +++ b/apps/web/src/components/watchdog-policy-card.tsx @@ -22,7 +22,7 @@ import { import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; import { api } from "@/lib/api"; import { watchdogCalibrationActionState } from "@/lib/settings-page-helpers"; -import { watchdogPolicyUpdate } from "@/lib/settings-updates"; +import { numericInputCommit, watchdogPolicyUpdate } from "@/lib/settings-updates"; export function WatchdogPolicyCard({ canManage, @@ -486,16 +486,39 @@ function NumberField({ step?: number; value: number; }) { + // Local text buffer so the field stays clearable/editable, while an empty or + // invalid entry never commits a 0 to the draft (audit H4-2): a cleared watchdog + // threshold (e.g. thresholdDbfs / a score threshold) would otherwise persist 0 + // — a value the server accepts — and arm an always-fire alert. Re-sync from + // `value` only on a genuine external change (not while typing "0."). + const [text, setText] = useState(String(value)); + + useEffect(() => { + setText((current) => (numericInputCommit(current) === value ? current : String(value))); + }, [value]); + return ( onChange(Number(event.target.value))} + onBlur={() => + setText((current) => + numericInputCommit(current) === undefined ? String(value) : current, + ) + } + onChange={(event) => { + setText(event.target.value); + const next = numericInputCommit(event.target.value); + + if (next !== undefined) { + onChange(next); + } + }} step={step} type="number" - value={value} + value={text} /> ); diff --git a/apps/web/src/lib/settings-updates.test.ts b/apps/web/src/lib/settings-updates.test.ts index f64dabd9..3e89bed9 100644 --- a/apps/web/src/lib/settings-updates.test.ts +++ b/apps/web/src/lib/settings-updates.test.ts @@ -2,7 +2,21 @@ import assert from "node:assert/strict"; import test from "node:test"; import type { WatchdogPolicy } from "@rakkr/shared"; -import { watchdogPolicyUpdate } from "./settings-updates"; +import { numericInputCommit, watchdogPolicyUpdate } from "./settings-updates"; + +test("numericInputCommit never coerces an empty/invalid numeric field to 0", () => { + // The bug: a cleared watchdog threshold field yields Number("") === 0, which + // the server accepts (dbfsSchema allows 0, score thresholds allow 0) and arms + // an always-fire alert. An empty/invalid entry must NOT commit a value. + assert.equal(numericInputCommit(""), undefined); + assert.equal(numericInputCommit(" "), undefined); + assert.equal(numericInputCommit("abc"), undefined); + // A deliberately-typed number (including 0) still commits. + assert.equal(numericInputCommit("0"), 0); + assert.equal(numericInputCommit("-18.5"), -18.5); + assert.equal(numericInputCommit("0.97"), 0.97); + assert.equal(numericInputCommit("120"), 120); +}); test("watchdog policy update preserves quality and flatline fields", () => { assert.deepEqual(watchdogPolicyUpdate(watchdogPolicy()), { diff --git a/apps/web/src/lib/settings-updates.ts b/apps/web/src/lib/settings-updates.ts index 774fb930..2cb3048b 100644 --- a/apps/web/src/lib/settings-updates.ts +++ b/apps/web/src/lib/settings-updates.ts @@ -5,6 +5,22 @@ import type { WatchdogPolicyUpdate, } from "@rakkr/shared"; +// The value a numeric change should COMMIT to a draft, or `undefined` to +// leave the committed value unchanged. Returning undefined for an empty/invalid +// field is what stops `Number("") === 0` from silently committing a 0 — e.g. a +// watchdog `thresholdDbfs`/score threshold cleared to `0` passes server +// validation and arms an always-fire alert (audit H4-2). Callers keep a local +// text buffer so the field stays clearable while typing. +export function numericInputCommit(raw: string): number | undefined { + if (raw.trim() === "") { + return undefined; + } + + const parsed = Number(raw); + + return Number.isFinite(parsed) ? parsed : undefined; +} + export function recordingProfileUpdate(profile: RecordingProfile): RecordingProfileUpdate { return { bitrateKbps: profile.bitrateKbps, From e25446ad35319f3ee8cbb002ed6f91d7d98a4a30 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 15:16:08 +0500 Subject: [PATCH 20/33] Update gap-hunt ledger: Run 6 dirty, H4-2 re-triaged + fixed (streak reset 2->0) Catalogue re-triage found H4-2 mis-classified (watchdog thresholdDbfs/score thresholds accept 0 -> persisted always-fire alert); fixed as H4-2W. Other 19 catalogued items confirmed correctly triaged. New suspected: R6-ENROLL-DUP, R6-INSTALL-URL-QUOTE. Co-Authored-By: Claude Opus 4.8 --- docs/internal/audits/2026-07-08-gap-hunt.md | 32 ++++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/docs/internal/audits/2026-07-08-gap-hunt.md b/docs/internal/audits/2026-07-08-gap-hunt.md index 43dfbe14..ac1b9d65 100644 --- a/docs/internal/audits/2026-07-08-gap-hunt.md +++ b/docs/internal/audits/2026-07-08-gap-hunt.md @@ -15,9 +15,9 @@ primary target. Target: **5 consecutive clean runs.** | Base | rebased onto `origin/main` `619b6f10` (static for the audit so far) | | Started | 2026-07-08 | | Completed | _in progress_ | -| Runs | 5 so far — **streak 2** (Runs 1–3 dirty: 6+7+2 fixes; **Runs 4–5 clean**) | -| Findings closed | 15 (Run 1: `G1`,`N1`,`N2`,`S1`,`S4`,`N3` · Run 2: `H4-1`,`H3-1`,`H3-2`,`H1-1`,`N4`,`H3-3`,`H1-2` · Run 3: `R3-4`, `H3-3-UPDATE-COV`) — each red→green except the page-wiring `S1` and the coverage locks (see rows) | -| Result | _in progress — Run 5 clean, **streak 2 of 5**_ | +| Runs | 6 so far — **streak 0** (Runs 1–3 dirty; Runs 4–5 clean → streak 2; **Run 6 dirty** (`H4-2W`) → streak reset) | +| Findings closed | 16 (Run 1: `G1`,`N1`,`N2`,`S1`,`S4`,`N3` · Run 2: `H4-1`,`H3-1`,`H3-2`,`H1-1`,`N4`,`H3-3`,`H1-2` · Run 3: `R3-4`, `H3-3-UPDATE-COV` · Run 6: `H4-2W`) — each red→green except the page-wiring `S1` and the coverage locks | +| Result | _in progress — Run 6 dirty (re-triage caught a mis-classified real bug, fixed), **streak 0 of 5**_ | | Gates at close of Run 4 | green — API 631 pass / 19 skip (non-DB), web 150 pass, tsc, oxlint, oxfmt, `check:loc` (all ≤1000), node-lifecycle + storage/recordings/time/switcher/rbac/watchdog/settings/operations/scheduler baselines, **and `db:verify` (Drizzle migration replay 0001–0046 against throwaway Postgres, exit 0 — confirms 0046 `ALTER TYPE ADD VALUE 'provisioning'` replays cleanly)**. Full `mise run check` + DB-gated API suite deferred to the final clean run. | **Status legend:** `FIXED` = failing test + fix landed on this branch · `RESOLVED` @@ -77,6 +77,7 @@ fixes clean (no regressions) and surfaced only LOW items. | N4 | 2 | node-store/availability-security | The heartbeat write persisted the agent-reported status verbatim and the schema accepted `provisioning`/`offline`, so first-contact promotion happened **only** because the shipped agent hardcodes `"online"` — and any node-credential holder could POST `status:"provisioning"` to **un-promote a live node**, which `deriveNodeStatus` short-circuits, permanently suppressing its offline alert (reachable, not just latent). Coerce a heartbeat's `provisioning`/`offline` to `online` in both stores (`heartbeatStatus`); the controller owns the lifecycle state machine. Red→green transform tests (also closes the `H2-2` promotion-coverage gap). | `75a7ca6b` | | H3-2 | 2 | web/uploads | Editing a schedule persisted before the stub-removal loaded the stub id verbatim into the draft, but the form's policy toggles filter the stub out — leaving the operator no control to clear it, so every edit silently re-saved `["upload-policy-stub"]`. `scheduleToDraft` now strips the stub, resolving a legacy schedule to "no upload". Red→green draft test. | `0257dbe5` | | H3-3 | 2 | uploads/validation | PR #31 intended every upload policy to target a real destination, but the create input left `destinationId` optional and the console eager-created a destination-less policy on "New" — assignable, and reconciling its recordings to `partial` (`provider_not_configured`). Enforced `destinationId` at the operator **create route** (the store stays lenient for seeds/tests, avoiding a broad fixture ripple); the console seeds the first destination and disables "New" until one exists. Red→green route test (create without a destination → `400`). | `e42fb7ec` | +| H4-2W | 6 | web/watchdog-editor | **Run-6 re-triage** found the catalogued `H4-2` was mis-classified as cosmetic for the watchdog editor: clearing a numeric field yields `Number("")===0`, and `thresholdDbfs` (`dbfsSchema` `[-160,24]`) + the score thresholds (`[0,1]`) **accept 0**, so a cleared threshold silently persisted `0` and armed an always-fire alert (`watchdog-signal.ts:296` fires low-signal for all healthy audio) → a flood of spurious `critical` health events (alert fatigue masking real issues). Added a shared `numericInputCommit(raw)` helper (empty/invalid → no commit, never `0`) + a local text buffer in the shared `NumberField` (keeps the field editable while typing; re-syncs from `value` only on a genuine external change), fixing all 16 watchdog numeric fields at one point. Red→green helper test (`numericInputCommit("") === undefined ≠ 0`). The other editors' clear→`0` is server-**rejected** (no persist) and remains the catalogued cosmetic `H4-2`. | `63f86293` | ### Low @@ -101,7 +102,7 @@ fixes clean (no regressions) and surfaced only LOW items. | ID | Sev | Kind | Locus | Note | | --- | --- | --- | --- | --- | | N4-HEARTBEAT-STATUS | ✅ resolved | fixed | `node-store-updates.ts`, `node-store.ts` | **RESOLVED by `N4` (Run 2, `75a7ca6b`):** confirmed reachable (a node-credential holder could POST `status:"provisioning"` to un-promote a live node and suppress its offline alert). `heartbeatStatus` now coerces a heartbeat's `provisioning`/`offline` to `online` in both stores; red→green transform tests added. | -| H4-2-NUMBER-INPUT | Medium | catalogued | recording-profile-settings-card.tsx, upload-policy-panel.tsx, watchdog-policy-card.tsx, retention-policy-panel.tsx (+ enhancement fields) | Cleared numeric inputs submit `0` (`Number("") === 0`), not empty, and the editors are not ``s (bare `onClick` Save, so the HTML `min`/`max` are inert). A `0`/out-of-range value POSTs, the server Zod schema rejects it (`bitrateKbps` `.positive()`, `highpass.hz` `.min(20)`, `maxAttempts` `.positive().max(100)`, etc.), and the operator sees only a generic "Save failed" toast with no field-level cause. `H4-3` (retention `optionalNumber` lets `0`/`2.5` through, diverging from `.int().positive()`) is the same class. **Deferred** as a focused slice: it spans 5 editor files + a new shared `parseBoundedNumber(value, {min,max,integer})` helper, and the clamp-to-min-vs-keep-last-valid behaviour is a UX call. Bounded impact — the server correctly rejects, so no bad data persists; the defect is a confusing error, not corruption. | +| H4-2-NUMBER-INPUT | Low | ⚠ partial (watchdog fixed; rest cosmetic) | recording-profile-settings-card.tsx, upload-policy-panel.tsx, retention-policy-panel.tsx (+ enhancement fields) | Cleared numeric inputs submit `0` (`Number("") === 0`). **The watchdog-editor half — where the server ACCEPTS 0 and persists a harmful always-fire threshold — is RESOLVED by `H4-2W` (Run 6, `63f86293`).** The remainder here is genuinely cosmetic: the recording-profile/upload-policy fields all have `.positive()`/`.min(20)`-style bounds that **reject 0 server-side** (no bad data persists — just a generic "Save failed" toast), and retention's `optionalNumber` clears to `null` (not 0). `H4-3` (retention allows a typed `0` for `minFreeDiskPercent` `.int().min(0)`) is the same cosmetic class. Optional follow-up: apply the new `numericInputCommit` helper to these editors too for consistent UX. | | S3-STALE-DEFAULT-ID | Med-Low | suspected | `apps/web/src/lib/schedule-draft.ts` (`defaultDraft`) | `defaultDraft` prefills `defaults?.defaultRecordingProfileId ?? built-in` with no existence check; the controller-settings columns are intentionally FK-free and may hold a deleted id. The ad-hoc panel (`recording-start-panel.tsx`) guards with a `.some(id === …)` existence check and falls back; `defaultDraft` does not, so a new-schedule form can prefill a dangling id. Bounded — the create API validates existence and 404s (`recording_profile_not_found` etc.), so the operator gets an opaque failed save, not corruption. Not a falsy-coalesce bug (`??`, so null/undefined fall back correctly). Fix: thread the available profile/policy lists into `defaultDraft` and guard each default like the ad-hoc panel. | | W4A-WATCHDOG-DISPLAY-DEFAULT | Low | suspected | `apps/web/src/components/watchdog-policy-card.tsx` (`value={draft.x ?? 0.98}` fields) | The watchdog editor renders a `??`-fallback (e.g. `0.98`) for an optional threshold whose draft value is `undefined`; if the operator never touches the field, `watchdogPolicyUpdate(draft)` JSON-drops the key and the server preserves the existing DB value — so a null-column legacy policy shows "0.98" but saves unchanged. New policies seed from `defaultScheduledVoiceWatchdogPolicy` (all populated), so only legacy/null-column rows are affected, hence suspected. Fix: fold the display fallback into the draft on load (what-you-see-is-what-you-save). | | H1-3-PROVISIONING-REASON | Low | cosmetic | `apps/api/src/node-action-routes.ts:29` (`unavailableNodeStatuses`) | `unavailableNodeStatuses = new Set(["offline"])` excludes `provisioning`, so a provisioning node's listen/meters action `reason` reads `monitor_source_unavailable`/`meter_frame_not_found` instead of a node-not-ready reason. **Not a bypass** — a provisioning node has never contacted the controller so `readiness.listen`/`.meters` are false and the action stays disabled; only the reason label is inaccurate. Optional: add `"provisioning"` to the set. | @@ -122,6 +123,8 @@ fixes clean (no regressions) and surfaced only LOW items. | H4-1-CLAMP-HOOK-COV | Medium | coverage | `apps/web/src/lib/use-server-pagination.ts` | The `clampToTotal` **hook** behavior (render-time `setOffset`, filter-reset interaction, stale-`keepPreviousData` total) is untested — only the pure `clampedOffset` is. The adversary verified the behavior is correct (converges in one step, never fights the filter-reset, Next is server-gated), but locking it needs a `renderHook` test, which the web unit harness (`tsx --test`, no `@testing-library/react`/jsdom) cannot run today. Deferred pending a React test-harness addition (would also unblock `W1`/`W3`/`W4`). | | R4-1-SERVER-DEDUP | Low | suspected (mitigated) | `apps/api/src/schedule-route-helpers.ts` (create/update input builders) | The `R3-4` dedup of `uploadPolicyIds` lives only in the web client (`draftToInput`) + the `.max(32)` cap; the server persists duplicates verbatim (unlike sibling `tags`, deduped via `uniqueTags`). A non-browser API client could store `["x","x"]`. **Fully mitigated downstream** — the upload fan-out (`uploadPoliciesForChunkedRecording`) dedups by destination target, so duplicates collapse to one queue item (no multiplied work); the only residual is a cosmetic duplicate in the stored row / schedule-detail display. Two hunters (data-integrity + completeness critic) judged it a non-defect / pre-existing. Fix if touched: mirror `uniqueTags` server-side (or add `.transform(v => [...new Set(v)])` to the shared schema) so the server is authoritative. | | R4-2-LIVENESS-ISOLATION | Low | suspected (mitigated) | `apps/api/src/watchdog-node-liveness.ts:34-69` (`reconcileNodeLivenessEvents`) | The per-node reconcile loop has no per-node try/catch, so a throw for one node skips the rest of that tick's nodes. **Mitigated**: `HealthEventStore` create/update/list swallow DB errors and fail over to memory (don't throw), so the only throw source is `auditStore.append`; and the watchdog tick is non-latching (`watchdog-runner` resets `running` in `finally`, wraps in `.catch`), so a throw = one skipped tick, retried next interval — no wedged state. Fix if touched: wrap the per-node body in try/catch → `{ outcome: "skipped", reason: "reconcile_failed" }` and continue (mirror the recording-watchdog families' independence). | +| R6-ENROLL-DUP | Low | suspected | `apps/web/src/components/node-inventory-dialogs.tsx` (`EnrollNodeDialog`) | Enroll is a two-step client sequence (`enrollNode` then `mintNodeBootstrapToken`); server `enroll` inserts a fresh-UUID row (not idempotent by alias/hostname). If the mint fails after the node is created, the `["nodes"]` query isn't invalidated on error and a retry creates a **duplicate** provisioning node. Minor UX/idempotency edge (recoverable: the operator can mint a token from the node card later; duplicates are deletable), not data-loss/security. Fix if touched: invalidate `["nodes"]` on error and/or combine enroll+mint into one atomic step, or make enroll idempotent by identity. | +| R6-INSTALL-URL-QUOTE | Low | cosmetic | `apps/web/src/lib/node-page-helpers.ts:146` (`buildAgentInstallCommand`) | The copy-paste install one-liner shell-quotes `site`/`room` but not the operator-set `controllerUrl`. No privilege boundary is crossed (the operator runs the command on their own host with their own URL), so it's a cosmetic quoting inconsistency, not an injection. Fix if touched: quote `controllerUrl` too via the existing `shellQuoteArg`. | ### Coverage gaps (no bug — missing tests) @@ -185,6 +188,7 @@ hunters run within a run. | 3 | 2026-07-08 | adversary-on-Run-2-fixes (all verified clean) + fresh: data-table/agent-version render correctness, cross-cutting schema-bounds + switcher/channel-map/destination dialogs — 3 parallel hunters | R3-4 (+ H3-3-update coverage) | 2 | no | 0 | | 4 | 2026-07-08 | convergence: adversary-on-Run-3-fixes + completeness critic (all 85 changed files enumerated), fresh data-integrity/concurrency, fresh web permission-boundary re-sweep — 3 parallel hunters; `db:verify` migration replay run green | — (R4-1, R4-2 suspected/mitigated, catalogued) | 0 | **yes** | **1** | | 5 | 2026-07-08 | convergence (fresh angles): end-to-end feature-trace (all 4 PR features through every layer), deploy/ops + docs/baseline consistency, observability/audit/error-surface — 3 parallel hunters; `node:test-db` (22 DB-backed concurrency/atomicity tests) run green | — (no new leads) | 0 | **yes** | **2** | +| 6 | 2026-07-08 | convergence (fresh angles): **catalogue re-triage** (adversarially re-verify every open item's severity), PR×existing-code interaction, invariant-based fresh-outsider — 3 parallel hunters | H4-2W (re-triage escalation) | 1 | no | 0 | ### Notes on the runs @@ -265,3 +269,23 @@ hunters run within a run. tests, all green) against a throwaway Postgres, confirming the `N4` Postgres-store change and the upload-queue/credential/room-delete races. **Zero code/test changes; no new leads.** → **clean, streak 2**. +- **Run 6 — dirty; the re-triage adversary earned its keep.** Three fresh-angle + hunters: (a) a **catalogue re-triage** that adversarially re-verified every one + of the ~20 open items' severity/classification; (b) PR×existing-code interaction + (the new `provisioning` state / `[]` uploadPolicyIds vs the pre-PR RBAC/recording/ + retention/watchdog/metrics subsystems — all seams clean); (c) an invariant-based + fresh-outsider (no-crash / UTC / idempotency / fail-closed / bounded — all hold). + 19 of 20 catalogued items were confirmed correctly triaged, but the re-triage + found **`H4-2-NUMBER-INPUT` was mis-classified**: its "server rejects → no bad + data persists" mitigation is FALSE for the watchdog editor — `thresholdDbfs` + (`dbfsSchema` `[-160,24]`) and the score thresholds (`[0,1]`) accept `0`, so a + cleared field silently persisted `0` and armed an always-fire alert flooding the + operator with spurious `critical` events. Fixed as `H4-2W` (shared + `numericInputCommit` + a local text buffer in the shared `NumberField`, + red→green). Because a real confirmed functional bug required a change, the run is + **dirty and the streak resets 2 → 0** — the honest outcome of the re-triage + catching a genuine defect that the earlier runs' clean verdicts had rested on. + New **suspected/cosmetic** leads logged, not fixed: `R6-ENROLL-DUP` (enroll + two-step partial-failure could create a duplicate node) and `R6-INSTALL-URL-QUOTE` + (install one-liner doesn't shell-quote `controllerUrl` — cosmetic, no privilege + boundary). Dirty (landed `H4-2W`) → streak 0. From 3e59bc445097212703049310dc41350cfd05098d Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 15:27:55 +0500 Subject: [PATCH 21/33] Truncate over-cap heartbeat ipAddresses instead of failing closed (audit R7-IPCAP) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent's collect_ip_addresses() reports every address from 'hostname -I' uncapped, but the heartbeat schema caps ipAddresses at .max(16). A multi-homed node (>16 IPs: IPv6 SLAAC/privacy + Docker/libvirt/VLAN bridges) had every heartbeat rejected 400 — and since the agent freezes the IP list at startup, that desynced the node forever, freezing lastSeenAt and flipping the live, recording node offline. A liveness heartbeat must not fail closed over a cosmetic field: preprocess ipAddresses to truncate to the documented cap and accept the heartbeat (keeping the first 16, the primary addresses). Red->green route test (20 IPs -> 202 + truncated, was 400). Agent-side .take(16) noted as defensive follow-up. Co-Authored-By: Claude Opus 4.8 --- apps/api/src/agent-route-helpers.ts | 13 +++++++- apps/api/test/agent-heartbeat-routes.test.ts | 34 ++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/apps/api/src/agent-route-helpers.ts b/apps/api/src/agent-route-helpers.ts index 210a430d..647a1bf7 100644 --- a/apps/api/src/agent-route-helpers.ts +++ b/apps/api/src/agent-route-helpers.ts @@ -43,7 +43,18 @@ export const nodeHeartbeatSchema = z .object({ agentVersion: z.string().trim().min(1).max(80), hostname: z.string().trim().min(1).max(255), - ipAddresses: z.array(z.string().trim().min(1).max(120)).max(16).default([]), + // A heartbeat is liveness-critical, so an over-cap ipAddresses list must not + // fail the whole heartbeat closed and strand the node as "offline". A + // multi-homed host's `hostname -I` can exceed 16 (IPv6 SLAAC/privacy addresses + // + Docker/libvirt/VLAN bridges); truncate to the documented cap and accept + // (the kept 16 are the primary addresses) rather than 400 every heartbeat + // forever and desync the node (audit R7-IPCAP). + ipAddresses: z + .preprocess( + (value) => (Array.isArray(value) ? value.slice(0, 16) : value), + z.array(z.string().trim().min(1).max(120)).max(16), + ) + .default([]), runtime: nodeRuntimeSchema.optional(), status: nodeStatusSchema.default("online"), }) diff --git a/apps/api/test/agent-heartbeat-routes.test.ts b/apps/api/test/agent-heartbeat-routes.test.ts index 6303e7dc..b8698520 100644 --- a/apps/api/test/agent-heartbeat-routes.test.ts +++ b/apps/api/test/agent-heartbeat-routes.test.ts @@ -68,6 +68,40 @@ test("agent heartbeat audits changed and unchanged successes", async () => { assert.ok(audits.every((event) => event.permission === "node:control")); }); +test("agent heartbeat truncates an over-cap ipAddresses list instead of failing closed", async () => { + const app = new Hono(); + const auditStore = createAuditStore(""); + const routeNode = node(); + const nodeStore = memoryNodeStore([routeNode]); + const manyIps = Array.from({ length: 20 }, (_, index) => `10.9.0.${index + 1}`); + + registerAgentRoutes({ + app, + healthEventStore: createHealthEventStore("", []), + meterFrameStore: memoryMeterFrameStore(), + nodeStore, + recordAuditEvent: recordAuditEvent(auditStore), + recordingStore: memoryRecordingStore(), + settingsStore: {} as SettingsStore, + }); + + const response = await postHeartbeat(app, routeNode.id, { + agentVersion: "0.2.0", + hostname: "multi-homed-node", + ipAddresses: manyIps, + status: "online", + }); + const body = (await response.json()) as { data: RecorderNode }; + + // A multi-homed host's `hostname -I` can exceed 16 addresses; the heartbeat is + // liveness-critical, so it must be accepted (not 400'd every tick, which would + // freeze lastSeenAt and flip the live node offline). The list is truncated to + // the documented cap (audit R7-IPCAP). + assert.equal(response.status, 202); + assert.equal(body.data.ipAddresses.length, 16); + assert.deepEqual(body.data.ipAddresses, manyIps.slice(0, 16)); +}); + function postHeartbeat(app: Hono, nodeId: string, heartbeat: NodeHeartbeatInput) { return app.request(`/api/v1/nodes/${nodeId}/heartbeat`, { body: JSON.stringify(heartbeat), From 2c31abe17703c76ad5565bfce391a748775bd39b Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 15:29:14 +0500 Subject: [PATCH 22/33] Update gap-hunt ledger: Run 7 dirty, R7-IPCAP heartbeat desync fixed (streak 0) Rust agent-contract angle found the agent's uncapped ipAddresses vs the heartbeat .max(16) reject -> permanent desync for multi-homed nodes; fixed controller-side (truncate). H4-2W verified sound. New catalogue: R7-IP-AGENT-CAP, R7-NUMCOMMIT-HEX, R7-SEED-LIVENESS. Co-Authored-By: Claude Opus 4.8 --- docs/internal/audits/2026-07-08-gap-hunt.md | 26 ++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/docs/internal/audits/2026-07-08-gap-hunt.md b/docs/internal/audits/2026-07-08-gap-hunt.md index ac1b9d65..a8dfcb9e 100644 --- a/docs/internal/audits/2026-07-08-gap-hunt.md +++ b/docs/internal/audits/2026-07-08-gap-hunt.md @@ -15,9 +15,9 @@ primary target. Target: **5 consecutive clean runs.** | Base | rebased onto `origin/main` `619b6f10` (static for the audit so far) | | Started | 2026-07-08 | | Completed | _in progress_ | -| Runs | 6 so far — **streak 0** (Runs 1–3 dirty; Runs 4–5 clean → streak 2; **Run 6 dirty** (`H4-2W`) → streak reset) | -| Findings closed | 16 (Run 1: `G1`,`N1`,`N2`,`S1`,`S4`,`N3` · Run 2: `H4-1`,`H3-1`,`H3-2`,`H1-1`,`N4`,`H3-3`,`H1-2` · Run 3: `R3-4`, `H3-3-UPDATE-COV` · Run 6: `H4-2W`) — each red→green except the page-wiring `S1` and the coverage locks | -| Result | _in progress — Run 6 dirty (re-triage caught a mis-classified real bug, fixed), **streak 0 of 5**_ | +| Runs | 7 so far — **streak 0** (Runs 1–3 dirty; Runs 4–5 clean → 2; Run 6 dirty (`H4-2W`); **Run 7 dirty** (`R7-IPCAP`)) | +| Findings closed | 17 (Run 1: `G1`,`N1`,`N2`,`S1`,`S4`,`N3` · Run 2: `H4-1`,`H3-1`,`H3-2`,`H1-1`,`N4`,`H3-3`,`H1-2` · Run 3: `R3-4`, `H3-3-UPDATE-COV` · Run 6: `H4-2W` · Run 7: `R7-IPCAP`) — each red→green except the page-wiring `S1` and the coverage locks | +| Result | _in progress — Run 7 dirty (Rust-contract angle caught a heartbeat desync bug, fixed), **streak 0 of 5**_ | | Gates at close of Run 4 | green — API 631 pass / 19 skip (non-DB), web 150 pass, tsc, oxlint, oxfmt, `check:loc` (all ≤1000), node-lifecycle + storage/recordings/time/switcher/rbac/watchdog/settings/operations/scheduler baselines, **and `db:verify` (Drizzle migration replay 0001–0046 against throwaway Postgres, exit 0 — confirms 0046 `ALTER TYPE ADD VALUE 'provisioning'` replays cleanly)**. Full `mise run check` + DB-gated API suite deferred to the final clean run. | **Status legend:** `FIXED` = failing test + fix landed on this branch · `RESOLVED` @@ -77,6 +77,7 @@ fixes clean (no regressions) and surfaced only LOW items. | N4 | 2 | node-store/availability-security | The heartbeat write persisted the agent-reported status verbatim and the schema accepted `provisioning`/`offline`, so first-contact promotion happened **only** because the shipped agent hardcodes `"online"` — and any node-credential holder could POST `status:"provisioning"` to **un-promote a live node**, which `deriveNodeStatus` short-circuits, permanently suppressing its offline alert (reachable, not just latent). Coerce a heartbeat's `provisioning`/`offline` to `online` in both stores (`heartbeatStatus`); the controller owns the lifecycle state machine. Red→green transform tests (also closes the `H2-2` promotion-coverage gap). | `75a7ca6b` | | H3-2 | 2 | web/uploads | Editing a schedule persisted before the stub-removal loaded the stub id verbatim into the draft, but the form's policy toggles filter the stub out — leaving the operator no control to clear it, so every edit silently re-saved `["upload-policy-stub"]`. `scheduleToDraft` now strips the stub, resolving a legacy schedule to "no upload". Red→green draft test. | `0257dbe5` | | H3-3 | 2 | uploads/validation | PR #31 intended every upload policy to target a real destination, but the create input left `destinationId` optional and the console eager-created a destination-less policy on "New" — assignable, and reconciling its recordings to `partial` (`provider_not_configured`). Enforced `destinationId` at the operator **create route** (the store stays lenient for seeds/tests, avoiding a broad fixture ripple); the console seeds the first destination and disables "New" until one exists. Red→green route test (create without a destination → `400`). | `e42fb7ec` | +| R7-IPCAP | 7 | agent-contract/availability | **Run-7 Rust-contract angle.** The agent's `collect_ip_addresses()` (`inventory.rs`) reports every `hostname -I` address uncapped, but `nodeHeartbeatSchema.ipAddresses` capped at `.max(16)` and **rejected** an over-cap list with `400`. A multi-homed node (>16 IPs — IPv6 SLAAC/privacy + Docker/libvirt/VLAN bridges) had every heartbeat 400'd, and since the agent freezes the IP list at startup the node **desynced permanently** — `lastSeenAt` never advanced and the offline watchdog flipped the live, recording node to `offline` (fails closed, worst case). A liveness heartbeat must not fail over a cosmetic field: the schema now `preprocess`-truncates `ipAddresses` to the documented cap and accepts the heartbeat (keeps the first 16, the primary addresses). Red→green route test (20 IPs → `202` + truncated, was `400`). Agent-side `.take(16)` noted as `R7-IP-AGENT-CAP` (defensive follow-up). | `3e59bc44` | | H4-2W | 6 | web/watchdog-editor | **Run-6 re-triage** found the catalogued `H4-2` was mis-classified as cosmetic for the watchdog editor: clearing a numeric field yields `Number("")===0`, and `thresholdDbfs` (`dbfsSchema` `[-160,24]`) + the score thresholds (`[0,1]`) **accept 0**, so a cleared threshold silently persisted `0` and armed an always-fire alert (`watchdog-signal.ts:296` fires low-signal for all healthy audio) → a flood of spurious `critical` health events (alert fatigue masking real issues). Added a shared `numericInputCommit(raw)` helper (empty/invalid → no commit, never `0`) + a local text buffer in the shared `NumberField` (keeps the field editable while typing; re-syncs from `value` only on a genuine external change), fixing all 16 watchdog numeric fields at one point. Red→green helper test (`numericInputCommit("") === undefined ≠ 0`). The other editors' clear→`0` is server-**rejected** (no persist) and remains the catalogued cosmetic `H4-2`. | `63f86293` | ### Low @@ -125,6 +126,9 @@ fixes clean (no regressions) and surfaced only LOW items. | R4-2-LIVENESS-ISOLATION | Low | suspected (mitigated) | `apps/api/src/watchdog-node-liveness.ts:34-69` (`reconcileNodeLivenessEvents`) | The per-node reconcile loop has no per-node try/catch, so a throw for one node skips the rest of that tick's nodes. **Mitigated**: `HealthEventStore` create/update/list swallow DB errors and fail over to memory (don't throw), so the only throw source is `auditStore.append`; and the watchdog tick is non-latching (`watchdog-runner` resets `running` in `finally`, wraps in `.catch`), so a throw = one skipped tick, retried next interval — no wedged state. Fix if touched: wrap the per-node body in try/catch → `{ outcome: "skipped", reason: "reconcile_failed" }` and continue (mirror the recording-watchdog families' independence). | | R6-ENROLL-DUP | Low | suspected | `apps/web/src/components/node-inventory-dialogs.tsx` (`EnrollNodeDialog`) | Enroll is a two-step client sequence (`enrollNode` then `mintNodeBootstrapToken`); server `enroll` inserts a fresh-UUID row (not idempotent by alias/hostname). If the mint fails after the node is created, the `["nodes"]` query isn't invalidated on error and a retry creates a **duplicate** provisioning node. Minor UX/idempotency edge (recoverable: the operator can mint a token from the node card later; duplicates are deletable), not data-loss/security. Fix if touched: invalidate `["nodes"]` on error and/or combine enroll+mint into one atomic step, or make enroll idempotent by identity. | | R6-INSTALL-URL-QUOTE | Low | cosmetic | `apps/web/src/lib/node-page-helpers.ts:146` (`buildAgentInstallCommand`) | The copy-paste install one-liner shell-quotes `site`/`room` but not the operator-set `controllerUrl`. No privilege boundary is crossed (the operator runs the command on their own host with their own URL), so it's a cosmetic quoting inconsistency, not an injection. Fix if touched: quote `controllerUrl` too via the existing `shellQuoteArg`. | +| R7-IP-AGENT-CAP | Low | suspected (defensive) | `crates/recorder-agent/src/inventory.rs:213` (`collect_ip_addresses`) | Defensive follow-up to the `R7-IPCAP` controller fix: the agent should also bound its own `ipAddresses` (append `.take(16)` after the `split_whitespace()`, ideally preferring stable/routable addresses over IPv6 temporary/link-local) so a well-behaved client never sends a payload exceeding the documented cap. The controller truncate already prevents the desync for any client, so this is belt-and-suspenders, not load-bearing. Rust change → defer to the rust/rig harness; red→green via a pure `parse_ip_addresses(stdout)` unit test. | +| R7-NUMCOMMIT-HEX | Low | latent | `apps/web/src/lib/settings-updates.ts` (`numericInputCommit`) | `numericInputCommit` uses `Number(raw)`, which accepts hex/binary/octal (`Number("0x10")===16`). **Unreachable** through `` (the DOM sanitizes non-decimal to `""`), so it is not a live bug — a latent sharp edge only if the helper is reused behind a `type="text"` input. Fix if reuse grows: gate with a decimal-shape regex or document the type-number-only assumption. | +| R7-SEED-LIVENESS | Low | by-design (dev-only) | `apps/api/src/node-store.ts:259` (`SeedOnlyNodeStore.list`) | The in-memory seed store's `list()` does not apply `nodeWithDerivedLiveness` (only the Postgres `list()` derives). In DB-less dev/test mode a seeded node whose heartbeat went stale would keep its stored status in the inventory filter / CSV / `rakkr_node_online`. **No production reachability** (production always sets `DATABASE_URL` → Postgres derives; the only seed uses `status:"online"`). Noted for completeness; fix only if a future feature seeds a stale-timestamped node in DB-less mode. | ### Coverage gaps (no bug — missing tests) @@ -189,6 +193,7 @@ hunters run within a run. | 4 | 2026-07-08 | convergence: adversary-on-Run-3-fixes + completeness critic (all 85 changed files enumerated), fresh data-integrity/concurrency, fresh web permission-boundary re-sweep — 3 parallel hunters; `db:verify` migration replay run green | — (R4-1, R4-2 suspected/mitigated, catalogued) | 0 | **yes** | **1** | | 5 | 2026-07-08 | convergence (fresh angles): end-to-end feature-trace (all 4 PR features through every layer), deploy/ops + docs/baseline consistency, observability/audit/error-surface — 3 parallel hunters; `node:test-db` (22 DB-backed concurrency/atomicity tests) run green | — (no new leads) | 0 | **yes** | **2** | | 6 | 2026-07-08 | convergence (fresh angles): **catalogue re-triage** (adversarially re-verify every open item's severity), PR×existing-code interaction, invariant-based fresh-outsider — 3 parallel hunters | H4-2W (re-triage escalation) | 1 | no | 0 | +| 7 | 2026-07-08 | convergence (fresh angles): hard adversary-on-`H4-2W` (sound), data-type/state-machine decomposition, **Rust agent↔controller contract** — 3 parallel hunters | R7-IPCAP (heartbeat ipAddresses desync) | 1 | no | 0 | ### Notes on the runs @@ -289,3 +294,18 @@ hunters run within a run. two-step partial-failure could create a duplicate node) and `R6-INSTALL-URL-QUOTE` (install one-liner doesn't shell-quote `controllerUrl` — cosmetic, no privilege boundary). Dirty (landed `H4-2W`) → streak 0. +- **Run 7 — dirty; a fresh Rust-contract angle found a real desync.** Three + fresh-angle hunters: (a) a hard adversary on the `H4-2W` fix, which traced all 7 + regression scenarios and confirmed it **sound** (no mid-type clobber, clear+save + commits the last valid value never `0`, converges in one render step); (b) a + data-type/state-machine decomposition (NodeStatus / uploadPolicyIds / AgentRelease + / default-ids all fully handled — only a dev-only seed-store note `R7-SEED-LIVENESS`); + (c) the **agent↔controller wire contract** (agent_version/status/inventory/ + update_binary all consistent), which surfaced `R7-IPCAP`: the agent's uncapped + `hostname -I` list vs the heartbeat schema's `.max(16)` **reject** → a multi-homed + node desyncs permanently (every heartbeat 400s, node flips offline while live). + Fixed controller-side (truncate the liveness heartbeat's `ipAddresses` to the cap + rather than fail closed; red→green). New **suspected/latent/dev-only** leads + logged, not fixed: `R7-IP-AGENT-CAP` (defensive agent-side `.take(16)`, rust rig), + `R7-NUMCOMMIT-HEX` (latent — unreachable via `type=number`), `R7-SEED-LIVENESS` + (dev-only). Dirty (landed `R7-IPCAP`) → streak 0. From 13c81a79be57a5ff454697f78997c6fe6b9d35f3 Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 15:40:49 +0500 Subject: [PATCH 23/33] Keep a stale retention selection visible in the schedule form (audit R8-RETENTION-SELECT) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The schedule form wrapped its recording-profile and watchdog Selects in withSelectedOption (which prepends a synthetic option so a current-but-absent id stays visible), but the retention Select rendered the raw list. A stale/ deleted retention id (a prefilled controller-settings default, or an edited schedule whose policy was removed) made the controlled Select fall back to its placeholder — reading as 'unselected' while the draft silently kept and re-saved the id. Route retention through the same helper (moved to schedule-draft as a shared, now-tested export). Red->green helper test. Co-Authored-By: Claude Opus 4.8 --- .../src/components/schedule-form-dialog.tsx | 26 +++++++----------- apps/web/src/lib/schedule-draft.test.ts | 27 +++++++++++++++++++ apps/web/src/lib/schedule-draft.ts | 19 +++++++++++++ 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/apps/web/src/components/schedule-form-dialog.tsx b/apps/web/src/components/schedule-form-dialog.tsx index ca714d48..a22586d7 100644 --- a/apps/web/src/components/schedule-form-dialog.tsx +++ b/apps/web/src/components/schedule-form-dialog.tsx @@ -49,6 +49,7 @@ import { applyNaturalLanguageSchedule, dayOptions, removeExceptionFromDraft, + withSelectedOption, type ScheduleDraft, } from "@/lib/schedule-draft"; @@ -119,7 +120,14 @@ export function ScheduleFormDialog({ queryFn: () => api.accessGroups(subjectPickerFilters()), queryKey: subjectPickerGroupsQueryKey(), }); - const retentionPolicies = retentionPoliciesQuery.data?.data ?? []; + // Keep the current retention selection visible even if it's a stale/deleted id + // (mirrors recording-profile + watchdog); otherwise the controlled Select falls + // back to its placeholder and reads as unselected while the draft still holds it + // (audit R8-RETENTION-SELECT). + const retentionPolicies = withSelectedOption( + retentionPoliciesQuery.data?.data ?? [], + draft.retentionPolicyId, + ); // The built-in stub is a test-only queue and never appears in the console. const uploadPolicies = (uploadPoliciesQuery.data?.data ?? []).filter( (policy) => policy.id !== defaultStubUploadPolicy.id, @@ -707,19 +715,3 @@ export function ScheduleFormDialog({ function audioInterfaceLabel(audioInterface: AudioInterface) { return `${audioInterface.alias} / ${audioInterface.systemName} / ${audioInterface.backend}`; } - -// Render the fetched profiles/policies as dropdown options, but keep the -// schedule's current selection visible even if it is missing from the list -// (e.g. a renamed template, or settings:read is unavailable to this operator). -function withSelectedOption( - items: Item[], - selectedId: string, -) { - const options = items.map((item) => ({ id: item.id, name: item.name })); - - if (selectedId && !options.some((option) => option.id === selectedId)) { - return [{ id: selectedId, name: selectedId }, ...options]; - } - - return options; -} diff --git a/apps/web/src/lib/schedule-draft.test.ts b/apps/web/src/lib/schedule-draft.test.ts index 58a04ac5..bd7b5565 100644 --- a/apps/web/src/lib/schedule-draft.test.ts +++ b/apps/web/src/lib/schedule-draft.test.ts @@ -14,8 +14,35 @@ import { defaultDraft, draftToInput, scheduleToDraft, + withSelectedOption, } from "./schedule-draft"; +test("withSelectedOption keeps a current-but-absent selection visible", () => { + const items = [ + { id: "ret_keep", name: "Keep cache" }, + { id: "ret_30d", name: "30 days" }, + ]; + + // A selection present in the list is shown once (not duplicated). + assert.deepEqual(withSelectedOption(items, "ret_30d"), [ + { id: "ret_keep", name: "Keep cache" }, + { id: "ret_30d", name: "30 days" }, + ]); + // A stale/deleted selection is prepended so a controlled Select shows it + // instead of falling back to the placeholder (which reads as "unselected" + // while the draft silently keeps and re-saves the id). + assert.deepEqual(withSelectedOption(items, "ret_deleted"), [ + { id: "ret_deleted", name: "ret_deleted" }, + { id: "ret_keep", name: "Keep cache" }, + { id: "ret_30d", name: "30 days" }, + ]); + // No current selection → no synthetic option. + assert.deepEqual(withSelectedOption(items, ""), [ + { id: "ret_keep", name: "Keep cache" }, + { id: "ret_30d", name: "30 days" }, + ]); +}); + test("default draft falls back to built-in profile/policies and no upload when unset", () => { const draft = defaultDraft(); diff --git a/apps/web/src/lib/schedule-draft.ts b/apps/web/src/lib/schedule-draft.ts index f7367e44..55c9e5b7 100644 --- a/apps/web/src/lib/schedule-draft.ts +++ b/apps/web/src/lib/schedule-draft.ts @@ -533,3 +533,22 @@ function exceptionLabel(exception: NonNullable return `Pause ${exception.startDate}-${exception.endDate}`; } + +// Render the fetched profiles/policies as dropdown options, but keep the +// schedule's current selection visible even if it is missing from the list (a +// renamed/deleted template, a stale controller-settings default, or `settings:read` +// unavailable to this operator). Without this, a controlled ``s in `withSelectedOption` (prepends a synthetic option so a current-but-absent id stays visible) but rendered the **retention** `` was the one option-list not wrapped in `withSelectedOption`, so a + stale/deleted retention id rendered blank while the draft re-saved it — fixed by + wiring it through the shared helper (three consecutive runs, 6–8, each closed one + real-but-increasingly-marginal defect in the PR-restructured surface). No new + suspected leads. Dirty (landed `R8-RETENTION-SELECT`) → streak 0. From 4c880821e3c03a56ecd6dc38157c135220f6ee9d Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 15:49:00 +0500 Subject: [PATCH 25/33] Update gap-hunt ledger: Run 9 clean (streak 1/5) Adversary verified R8 sound; completeness critic over all 85 changed files found no new real defect. Zero code changes. R9-NODEIFACE-SELECT catalogued (pre-existing/bounded). First clean run after the 6-8 dirty streak. Co-Authored-By: Claude Opus 4.8 --- docs/internal/audits/2026-07-08-gap-hunt.md | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/internal/audits/2026-07-08-gap-hunt.md b/docs/internal/audits/2026-07-08-gap-hunt.md index 8b0ce11f..bf863f92 100644 --- a/docs/internal/audits/2026-07-08-gap-hunt.md +++ b/docs/internal/audits/2026-07-08-gap-hunt.md @@ -15,9 +15,9 @@ primary target. Target: **5 consecutive clean runs.** | Base | rebased onto `origin/main` `619b6f10` (static for the audit so far) | | Started | 2026-07-08 | | Completed | _in progress_ | -| Runs | 8 so far — **streak 0** (Runs 1–3 dirty; Runs 4–5 clean → 2; Runs 6–8 dirty: `H4-2W`, `R7-IPCAP`, `R8-RETENTION-SELECT`) | +| Runs | 9 so far — **streak 1** (Runs 1–3 dirty; Runs 4–5 clean; Runs 6–8 dirty: `H4-2W`,`R7-IPCAP`,`R8-RETENTION-SELECT`; **Run 9 clean**) | | Findings closed | 18 (Run 1: `G1`,`N1`,`N2`,`S1`,`S4`,`N3` · Run 2: `H4-1`,`H3-1`,`H3-2`,`H1-1`,`N4`,`H3-3`,`H1-2` · Run 3: `R3-4`, `H3-3-UPDATE-COV` · Run 6: `H4-2W` · Run 7: `R7-IPCAP` · Run 8: `R8-RETENTION-SELECT`) — each red→green except the page-wiring items and coverage locks | -| Result | _in progress — Runs 6–8 each caught + fixed one real (increasingly marginal) bug from a fresh angle, **streak 0 of 5**_ | +| Result | _in progress — Run 9 clean, **streak 1 of 5**_ | | Gates at close of Run 4 | green — API 631 pass / 19 skip (non-DB), web 150 pass, tsc, oxlint, oxfmt, `check:loc` (all ≤1000), node-lifecycle + storage/recordings/time/switcher/rbac/watchdog/settings/operations/scheduler baselines, **and `db:verify` (Drizzle migration replay 0001–0046 against throwaway Postgres, exit 0 — confirms 0046 `ALTER TYPE ADD VALUE 'provisioning'` replays cleanly)**. Full `mise run check` + DB-gated API suite deferred to the final clean run. | **Status legend:** `FIXED` = failing test + fix landed on this branch · `RESOLVED` @@ -129,6 +129,7 @@ fixes clean (no regressions) and surfaced only LOW items. | R6-INSTALL-URL-QUOTE | Low | cosmetic | `apps/web/src/lib/node-page-helpers.ts:146` (`buildAgentInstallCommand`) | The copy-paste install one-liner shell-quotes `site`/`room` but not the operator-set `controllerUrl`. No privilege boundary is crossed (the operator runs the command on their own host with their own URL), so it's a cosmetic quoting inconsistency, not an injection. Fix if touched: quote `controllerUrl` too via the existing `shellQuoteArg`. | | R7-IP-AGENT-CAP | Low | suspected (defensive) | `crates/recorder-agent/src/inventory.rs:213` (`collect_ip_addresses`) | Defensive follow-up to the `R7-IPCAP` controller fix: the agent should also bound its own `ipAddresses` (append `.take(16)` after the `split_whitespace()`, ideally preferring stable/routable addresses over IPv6 temporary/link-local) so a well-behaved client never sends a payload exceeding the documented cap. The controller truncate already prevents the desync for any client, so this is belt-and-suspenders, not load-bearing. Rust change → defer to the rust/rig harness; red→green via a pure `parse_ip_addresses(stdout)` unit test. | | R7-NUMCOMMIT-HEX | Low | latent | `apps/web/src/lib/settings-updates.ts` (`numericInputCommit`) | `numericInputCommit` uses `Number(raw)`, which accepts hex/binary/octal (`Number("0x10")===16`). **Unreachable** through `` (the DOM sanitizes non-decimal to `""`), so it is not a live bug — a latent sharp edge only if the helper is reused behind a `type="text"` input. Fix if reuse grows: gate with a decimal-shape regex or document the type-number-only assumption. | +| R9-NODEIFACE-SELECT | Low | suspected (pre-existing) | `apps/web/src/components/schedule-form-dialog.tsx` (node + interface ` - - - setDraft((current) => ({ ...current, bitrateKbps: Number(event.target.value) })) - } - type="number" - value={draft.bitrateKbps} - /> - + setDraft((current) => ({ ...current, bitrateKbps }))} + value={draft.bitrateKbps} + /> - - - updateEnhancement({ - highpass: { ...enhancement.highpass, hz: Number(event.target.value) }, - }) - } - type="number" - value={enhancement.highpass.hz} - /> - + updateEnhancement({ highpass: { ...enhancement.highpass, hz } })} + value={enhancement.highpass.hz} + /> - - - updateEnhancement({ - lowpass: { ...enhancement.lowpass, hz: Number(event.target.value) }, - }) - } - type="number" - value={enhancement.lowpass.hz} - /> - + updateEnhancement({ lowpass: { ...enhancement.lowpass, hz } })} + value={enhancement.lowpass.hz} + /> - - - updateEnhancement({ - loudnorm: { ...enhancement.loudnorm, targetI: Number(event.target.value) }, - }) - } - type="number" - value={enhancement.loudnorm.targetI} - /> - + + updateEnhancement({ loudnorm: { ...enhancement.loudnorm, targetI } }) + } + value={enhancement.loudnorm.targetI} + /> node.id === draft.nodeId); + // Keep a stale node/interface selection visible (like the profile/policy + // selects): if the draft pins a node or interface that is not in the current + // list (deleted, out of scope, or renamed away), the controlled Select would + // otherwise show its placeholder and read as unselected while the draft still + // holds — and silently re-saves — the id (audit R9-NODEIFACE-SELECT). + const nodeMissing = Boolean(draft.nodeId) && !selectedNode; + const interfaceMissing = + Boolean(draft.captureInterfaceId) && + !selectedNode?.interfaces.some( + (audioInterface) => audioInterface.id === draft.captureInterfaceId, + ); const recordingProfilesQuery = useQuery({ enabled: open, queryFn: api.recordingProfiles, @@ -233,6 +244,9 @@ export function ScheduleFormDialog({ Select a recorder + {nodeMissing ? ( + {draft.nodeId} (unavailable) + ) : null} {nodes.map((node) => ( {node.alias} / {node.location.room} @@ -594,6 +608,11 @@ export function ScheduleFormDialog({ Node default + {interfaceMissing ? ( + + {draft.captureInterfaceId} (unavailable) + + ) : null} {selectedNode?.interfaces.map((audioInterface) => ( {audioInterfaceLabel(audioInterface)} diff --git a/apps/web/src/components/settings-fields.tsx b/apps/web/src/components/settings-fields.tsx index 1ba6c8f2..954bd340 100644 --- a/apps/web/src/components/settings-fields.tsx +++ b/apps/web/src/components/settings-fields.tsx @@ -1,7 +1,9 @@ -import { type ReactNode, useId } from "react"; +import { type ReactNode, useEffect, useId, useState } from "react"; import { Checkbox } from "@/components/ui/checkbox"; +import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; +import { numericInputCommit } from "@/lib/settings-updates"; import { cn } from "@/lib/utils"; // Canonical labelled form field used across every settings/policy dialog. Keep @@ -27,6 +29,67 @@ export function Field({ ); } +// Canonical numeric settings field: a labelled numeric input backed by a local +// text buffer so the field stays clearable/editable while typing, but an empty +// or invalid entry never commits a value (Number("") === 0 would silently arm a +// 0 — e.g. a cleared watchdog threshold or a 0 bitrate). Shared across every +// policy/profile dialog so numeric-entry behaviour is identical everywhere +// (audit H4-2/H4-3). Re-sync from `value` only on a genuine external change. +export function NumberField({ + disabled = false, + hint, + label, + max, + min, + onChange, + placeholder, + step, + value, +}: { + disabled?: boolean; + hint?: ReactNode; + label: string; + max?: number; + min?: number; + onChange: (value: number) => void; + placeholder?: string; + step?: number; + value: number; +}) { + const [text, setText] = useState(String(value)); + + useEffect(() => { + setText((current) => (numericInputCommit(current) === value ? current : String(value))); + }, [value]); + + return ( + + + setText((current) => + numericInputCommit(current) === undefined ? String(value) : current, + ) + } + onChange={(event) => { + setText(event.target.value); + const next = numericInputCommit(event.target.value); + + if (next !== undefined) { + onChange(next); + } + }} + placeholder={placeholder} + step={step} + type="number" + value={text} + /> + + ); +} + // Canonical boolean row: a full-width bordered control that reads as one tap // target and highlights when checked. Shared by every policy dialog so toggles // look identical everywhere. diff --git a/apps/web/src/components/ui/truncate-cell.tsx b/apps/web/src/components/ui/truncate-cell.tsx index ff79fd1c..9a37ffd5 100644 --- a/apps/web/src/components/ui/truncate-cell.tsx +++ b/apps/web/src/components/ui/truncate-cell.tsx @@ -35,7 +35,10 @@ export function TruncateCell({ children, className }: { children: ReactNode; cla observer.observe(el); return () => observer.disconnect(); - }); + // Re-measure (and re-subscribe) when the cell content changes; width changes + // are handled by the ResizeObserver. Without this dep the observer was torn + // down and rebuilt on every render (audit W2-OBSERVER-DEP). + }, [children]); const line = (
diff --git a/apps/web/src/components/upload-policy-panel.tsx b/apps/web/src/components/upload-policy-panel.tsx index bb606745..7b408f1a 100644 --- a/apps/web/src/components/upload-policy-panel.tsx +++ b/apps/web/src/components/upload-policy-panel.tsx @@ -4,7 +4,7 @@ import { Save } from "lucide-react"; import { toast } from "sonner"; import type { UploadPolicy, UploadPolicyInput, UploadPolicyUpdate } from "@rakkr/shared"; -import { Field, Toggle } from "@/components/settings-fields"; +import { Field, NumberField, Toggle } from "@/components/settings-fields"; import { Button } from "@/components/ui/button"; import { DialogFooter } from "@/components/ui/dialog"; import { Input } from "@/components/ui/input"; @@ -106,17 +106,13 @@ export function UploadPolicyEditor({ - - - setDraft((current) => ({ ...current, maxAttempts: Number(event.target.value) })) - } - type="number" - value={draft.maxAttempts} - /> - + setDraft((current) => ({ ...current, maxAttempts }))} + value={draft.maxAttempts} + /> withWatchdogDisplayDefaults(policy)); const [calibrationNodeId, setCalibrationNodeId] = useState(nodes[0]?.id ?? ""); const [signalMarginDb, setSignalMarginDb] = useState(8); const calibrationAction = watchdogCalibrationActionState({ @@ -53,7 +53,7 @@ export function WatchdogPolicyCard({ description: "The watchdog policy could not be saved.", }), onSuccess: ({ data }) => { - setDraft(data); + setDraft(withWatchdogDisplayDefaults(data)); toast.success("Watchdog policy saved"); void queryClient.invalidateQueries({ queryKey: ["watchdog-policies"] }); void queryClient.invalidateQueries({ queryKey: ["status"] }); @@ -73,7 +73,7 @@ export function WatchdogPolicyCard({ }), onSuccess: ({ data }) => { if (data.policy) { - setDraft(data.policy); + setDraft(withWatchdogDisplayDefaults(data.policy)); } void queryClient.invalidateQueries({ queryKey: ["watchdog-policies"] }); @@ -82,7 +82,7 @@ export function WatchdogPolicyCard({ }); useEffect(() => { - setDraft(policy); + setDraft(withWatchdogDisplayDefaults(policy)); }, [policy]); useEffect(() => { @@ -469,61 +469,6 @@ export function WatchdogPolicyCard({ ); } -function NumberField({ - disabled, - label, - max, - min, - onChange, - step, - value, -}: { - disabled: boolean; - label: string; - max?: number; - min: number; - onChange: (value: number) => void; - step?: number; - value: number; -}) { - // Local text buffer so the field stays clearable/editable, while an empty or - // invalid entry never commits a 0 to the draft (audit H4-2): a cleared watchdog - // threshold (e.g. thresholdDbfs / a score threshold) would otherwise persist 0 - // — a value the server accepts — and arm an always-fire alert. Re-sync from - // `value` only on a genuine external change (not while typing "0."). - const [text, setText] = useState(String(value)); - - useEffect(() => { - setText((current) => (numericInputCommit(current) === value ? current : String(value))); - }, [value]); - - return ( - - - setText((current) => - numericInputCommit(current) === undefined ? String(value) : current, - ) - } - onChange={(event) => { - setText(event.target.value); - const next = numericInputCommit(event.target.value); - - if (next !== undefined) { - onChange(next); - } - }} - step={step} - type="number" - value={text} - /> - - ); -} - // Starting point for a newly created policy: the built-in scheduled-voice // watchdog with a placeholder name. The server assigns the id; the operator then // tunes thresholds in this editor. diff --git a/apps/web/src/lib/node-page-helpers.test.ts b/apps/web/src/lib/node-page-helpers.test.ts index 7356a09e..72413dfb 100644 --- a/apps/web/src/lib/node-page-helpers.test.ts +++ b/apps/web/src/lib/node-page-helpers.test.ts @@ -73,14 +73,28 @@ test("agent install command mirrors the documented day-0 one-liner", () => { // Pulls the official installer (not a hand-typed inventory) and runs the // single-use token via the documented flags. assert.match(command, /^curl -fsSL https:\/\/rakkr\.org\/agent\.sh \| sudo sh -s --/u); - assert.ok(command.includes("--controller-url https://controller.example:8787")); - assert.ok(command.includes("--bootstrap-token rakkr_bs_abc123")); - assert.ok(command.includes("--node-id node_42")); - // Free-text site/room are POSIX single-quoted so spaces survive `sh -s --`. + // Every interpolated value is POSIX single-quoted so spaces/metacharacters + // survive `sh -s --`. + assert.ok(command.includes("--controller-url 'https://controller.example:8787'")); + assert.ok(command.includes("--bootstrap-token 'rakkr_bs_abc123'")); + assert.ok(command.includes("--node-id 'node_42'")); assert.ok(command.includes("--site 'HQ'")); assert.ok(command.includes("--room 'Studio A'")); }); +test("agent install command quotes a controller URL with shell metacharacters", () => { + const command = buildAgentInstallCommand({ + bootstrapToken: "rakkr_bs_x", + // A hostile/mistaken controller URL must not break out of the pasted line. + controllerUrl: "https://c; rm -rf /", + nodeId: "node_x", + room: "HQ", + site: "HQ", + }); + + assert.ok(command.includes("--controller-url 'https://c; rm -rf /'")); +}); + test("agent install command escapes embedded quotes in room/site names", () => { const command = buildAgentInstallCommand({ bootstrapToken: "rakkr_bs_x", diff --git a/apps/web/src/lib/node-page-helpers.ts b/apps/web/src/lib/node-page-helpers.ts index 02e78a20..e455f624 100644 --- a/apps/web/src/lib/node-page-helpers.ts +++ b/apps/web/src/lib/node-page-helpers.ts @@ -151,11 +151,15 @@ export function buildAgentInstallCommand({ room, site, }: AgentInstallCommandInput): string { + // POSIX single-quote every interpolated value (not just free-text site/room): + // a controller URL with a shell metacharacter (`&`, `;`, `$`, a space) would + // otherwise break the pasted one-liner or inject into the operator's shell + // (audit R6-INSTALL-URL-QUOTE). return [ `curl -fsSL ${installScriptUrl} | sudo sh -s --`, - `--controller-url ${controllerUrl}`, - `--bootstrap-token ${bootstrapToken}`, - `--node-id ${nodeId}`, + `--controller-url ${shellQuoteArg(controllerUrl)}`, + `--bootstrap-token ${shellQuoteArg(bootstrapToken)}`, + `--node-id ${shellQuoteArg(nodeId)}`, `--site ${shellQuoteArg(site)}`, `--room ${shellQuoteArg(room)}`, ].join(" \\\n "); diff --git a/apps/web/src/lib/node-status.test.ts b/apps/web/src/lib/node-status.test.ts new file mode 100644 index 00000000..b547a1fa --- /dev/null +++ b/apps/web/src/lib/node-status.test.ts @@ -0,0 +1,60 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import type { NodeStatus } from "@rakkr/shared"; + +import { nodeStatusBadgeClass, nodeStatusLabel } from "./node-status"; +import { toneBadgeClass, toneFillClass } from "./status-colors"; + +// Pin the tone for every node status so a new/renamed enum member (or a silent +// change to the offline tone) breaks this test rather than shipping the wrong +// colour. Offline reads critical (red) per the operator decision (audit R3-1). +const expectedTone: Record[0]> = { + alerting: "critical", + degraded: "warning", + offline: "critical", + online: "healthy", + provisioning: "info", + recording: "info", +}; + +test("nodeStatusBadgeClass maps every node status to its pinned tone", () => { + for (const [status, tone] of Object.entries(expectedTone) as Array< + [NodeStatus, Parameters[0]] + >) { + assert.equal(nodeStatusBadgeClass(status), toneBadgeClass(tone), status); + } +}); + +test("nodeStatusBadgeClass offline reads as critical, not neutral", () => { + assert.equal(nodeStatusBadgeClass("offline"), toneBadgeClass("critical")); + assert.notEqual(nodeStatusBadgeClass("offline"), toneBadgeClass("neutral")); +}); + +test("an unknown/undefined status falls back to neutral", () => { + assert.equal(nodeStatusBadgeClass(undefined), toneBadgeClass("neutral")); +}); + +test("nodeStatusLabel surfaces a Title Case label, never the raw lowercase token", () => { + const labels: Record = { + alerting: "Alerting", + degraded: "Degraded", + offline: "Offline", + online: "Online", + provisioning: "Provisioning", + recording: "Recording", + }; + + for (const [status, label] of Object.entries(labels) as Array<[NodeStatus, string]>) { + assert.equal(nodeStatusLabel(status), label, status); + // The label must not be the raw machine token (audit H2-STATUS-RAW). + assert.notEqual(nodeStatusLabel(status), status); + } + + assert.equal(nodeStatusLabel(undefined), "Unknown"); +}); + +test("toneFillClass gives neutral its own muted fill, not the sky info fill", () => { + assert.notEqual(toneFillClass("neutral"), toneFillClass("info")); + assert.match(toneFillClass("neutral"), /bg-muted/); + assert.match(toneFillClass("info"), /bg-sky/); +}); diff --git a/apps/web/src/lib/node-status.ts b/apps/web/src/lib/node-status.ts index 9a98329b..d352560f 100644 --- a/apps/web/src/lib/node-status.ts +++ b/apps/web/src/lib/node-status.ts @@ -2,27 +2,43 @@ import type { NodeStatus } from "@rakkr/shared"; import { toneBadgeClass } from "@/lib/status-colors"; -export function nodeStatusBadgeClass(status: NodeStatus | undefined) { - if (status === "online") { - return toneBadgeClass("healthy"); - } - - // Enrolled but awaiting first contact — informational, not an alarm. - if (status === "provisioning") { - return toneBadgeClass("info"); - } +// Human-facing label for a node status. The API/enum values are lowercase +// machine tokens ("provisioning", "alerting"); surface a Title Case label rather +// than the raw token in operator-facing badges (audit H2-STATUS-RAW). +const nodeStatusLabels: Record = { + alerting: "Alerting", + degraded: "Degraded", + offline: "Offline", + online: "Online", + provisioning: "Provisioning", + recording: "Recording", +}; - if (status === "recording") { - return toneBadgeClass("info"); +export function nodeStatusLabel(status: NodeStatus | undefined): string { + if (!status) { + return "Unknown"; } - if (status === "degraded") { - return toneBadgeClass("warning"); - } + return nodeStatusLabels[status] ?? status; +} - if (status === "alerting") { - return toneBadgeClass("critical"); +export function nodeStatusBadgeClass(status: NodeStatus | undefined) { + switch (status) { + case "online": + return toneBadgeClass("healthy"); + // Enrolled but awaiting first contact — informational, not an alarm. + case "provisioning": + case "recording": + return toneBadgeClass("info"); + case "degraded": + return toneBadgeClass("warning"); + // Offline is a hard failure for a recording node: a room that should be + // capturing is not. Operators chose for it to read as critical (red) — the + // same weight as an active alert — not a quiet neutral grey (audit R3-1). + case "offline": + case "alerting": + return toneBadgeClass("critical"); + default: + return toneBadgeClass("neutral"); } - - return toneBadgeClass("neutral"); } diff --git a/apps/web/src/lib/schedule-draft.test.ts b/apps/web/src/lib/schedule-draft.test.ts index bd7b5565..61fd209e 100644 --- a/apps/web/src/lib/schedule-draft.test.ts +++ b/apps/web/src/lib/schedule-draft.test.ts @@ -68,6 +68,53 @@ test("default draft prefers operator-configured scheduling defaults", () => { assert.deepEqual(draft.uploadPolicyIds, ["upload_smb_primary"]); }); +test("default draft drops operator defaults that no longer exist in the available lists", () => { + const settings = { + ...defaultControllerSettings, + defaultRecordingProfileId: "profile_deleted", + defaultRetentionPolicyId: "retention_deleted", + defaultUploadPolicyId: "upload_deleted", + defaultWatchdogPolicyId: "watchdog_deleted", + }; + + // The stored defaults point at policies that have since been deleted; with the + // available lists known, each falls back to its built-in (upload → no upload) + // instead of prefilling a dangling id (audit S3). + const draft = defaultDraft(undefined, settings, { + recordingProfileIds: ["profile_hi_fi"], + retentionPolicyIds: ["retention_30d"], + uploadPolicyIds: ["upload_smb_primary"], + watchdogPolicyIds: ["watchdog_strict"], + }); + + assert.equal(draft.recordingProfileId, defaultVoiceRecordingProfile.id); + assert.equal(draft.retentionPolicyId, defaultKeepControllerCacheRetentionPolicy.id); + assert.equal(draft.watchdogPolicyId, defaultScheduledVoiceWatchdogPolicy.id); + assert.deepEqual(draft.uploadPolicyIds, []); +}); + +test("default draft keeps operator defaults that exist in the available lists", () => { + const settings = { + ...defaultControllerSettings, + defaultRecordingProfileId: "profile_hi_fi", + defaultRetentionPolicyId: "retention_30d", + defaultUploadPolicyId: "upload_smb_primary", + defaultWatchdogPolicyId: "watchdog_strict", + }; + + const draft = defaultDraft(undefined, settings, { + recordingProfileIds: ["profile_hi_fi"], + retentionPolicyIds: ["retention_30d"], + uploadPolicyIds: ["upload_smb_primary"], + watchdogPolicyIds: ["watchdog_strict"], + }); + + assert.equal(draft.recordingProfileId, "profile_hi_fi"); + assert.equal(draft.retentionPolicyId, "retention_30d"); + assert.equal(draft.watchdogPolicyId, "watchdog_strict"); + assert.deepEqual(draft.uploadPolicyIds, ["upload_smb_primary"]); +}); + test("schedule quick phrases produce structured weekly recurrence", () => { const draft = defaultDraft(); const updated = applyNaturalLanguageSchedule(draft, "weekdays 9am to 10:30am"); diff --git a/apps/web/src/lib/schedule-draft.ts b/apps/web/src/lib/schedule-draft.ts index 55c9e5b7..fcea3f4a 100644 --- a/apps/web/src/lib/schedule-draft.ts +++ b/apps/web/src/lib/schedule-draft.ts @@ -63,11 +63,47 @@ export const dayOptions: Array<{ id: ScheduleDayOfWeek; label: string }> = [ ]; const weekdayDays: ScheduleDayOfWeek[] = ["monday", "tuesday", "wednesday", "thursday", "friday"]; +// Available profile/policy ids to validate operator defaults against. When a +// list is omitted (not yet loaded) the corresponding default is trusted as-is; +// when provided, a default id absent from it is treated as deleted (audit S3). +export interface SchedulingDefaultAvailability { + recordingProfileIds?: readonly string[]; + retentionPolicyIds?: readonly string[]; + uploadPolicyIds?: readonly string[]; + watchdogPolicyIds?: readonly string[]; +} + +// Use an operator default id only when it still exists in the available list; a +// since-deleted default falls back to the built-in so a new draft never prefills +// a dangling policy id. An omitted list means "unknown", so trust the default. +function resolveDefaultId( + id: string | null | undefined, + available: readonly string[] | undefined, + fallback: string, +): string { + if (!id || (available && !available.includes(id))) { + return fallback; + } + + return id; +} + // Operator-configured scheduling defaults (from controller settings) win; when // unset each profile/policy falls back to its built-in. Upload has no built-in // fallback — an unset upload default means "no upload" (recording stays in the // controller cache), so the test-only stub never seeds a real schedule. -export function defaultDraft(node?: RecorderNode, defaults?: ControllerSettings): ScheduleDraft { +export function defaultDraft( + node?: RecorderNode, + defaults?: ControllerSettings, + available?: SchedulingDefaultAvailability, +): ScheduleDraft { + const uploadDefault = defaults?.defaultUploadPolicyId ?? null; + const uploadPolicyIds = + uploadDefault && + (!available?.uploadPolicyIds || available.uploadPolicyIds.includes(uploadDefault)) + ? [uploadDefault] + : []; + return { assignedGroupIds: [], assignedUserIds: [], @@ -90,9 +126,16 @@ export function defaultDraft(node?: RecorderNode, defaults?: ControllerSettings) pauseStartDate: "", recurrenceMode: "once", recurrenceStartAt: "", - recordingProfileId: defaults?.defaultRecordingProfileId ?? defaultVoiceRecordingProfile.id, - retentionPolicyId: - defaults?.defaultRetentionPolicyId ?? defaultKeepControllerCacheRetentionPolicy.id, + recordingProfileId: resolveDefaultId( + defaults?.defaultRecordingProfileId, + available?.recordingProfileIds, + defaultVoiceRecordingProfile.id, + ), + retentionPolicyId: resolveDefaultId( + defaults?.defaultRetentionPolicyId, + available?.retentionPolicyIds, + defaultKeepControllerCacheRetentionPolicy.id, + ), room: node?.location.room ?? "", startTime: "09:00", startEarlyMinutes: 0, @@ -100,8 +143,12 @@ export function defaultDraft(node?: RecorderNode, defaults?: ControllerSettings) tags: "voice, scheduled", timezone: fallbackTimezone, titleTemplate: "{{date}}_{{time}}_{{schedule.name}}_{{node.alias}}", - uploadPolicyIds: defaults?.defaultUploadPolicyId ? [defaults.defaultUploadPolicyId] : [], - watchdogPolicyId: defaults?.defaultWatchdogPolicyId ?? defaultScheduledVoiceWatchdogPolicy.id, + uploadPolicyIds, + watchdogPolicyId: resolveDefaultId( + defaults?.defaultWatchdogPolicyId, + available?.watchdogPolicyIds, + defaultScheduledVoiceWatchdogPolicy.id, + ), }; } diff --git a/apps/web/src/lib/scheduling-defaults.ts b/apps/web/src/lib/scheduling-defaults.ts index b376fb5e..ff2f5654 100644 --- a/apps/web/src/lib/scheduling-defaults.ts +++ b/apps/web/src/lib/scheduling-defaults.ts @@ -1,6 +1,5 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { toast } from "sonner"; -import type { ControllerSettings } from "@rakkr/shared"; import { api } from "@/lib/api"; @@ -47,13 +46,3 @@ export function useSchedulingDefault(field: SchedulingDefaultField, enabled = tr mutation.mutate(query.data?.data[field] === policyId ? null : policyId), }; } - -/** The four scheduling defaults, resolved from controller settings for prefill. */ -export function schedulingDefaultsFrom(settings: ControllerSettings | undefined) { - return { - recordingProfileId: settings?.defaultRecordingProfileId ?? null, - retentionPolicyId: settings?.defaultRetentionPolicyId ?? null, - uploadPolicyId: settings?.defaultUploadPolicyId ?? null, - watchdogPolicyId: settings?.defaultWatchdogPolicyId ?? null, - }; -} diff --git a/apps/web/src/lib/settings-updates.test.ts b/apps/web/src/lib/settings-updates.test.ts index 3e89bed9..6491ba52 100644 --- a/apps/web/src/lib/settings-updates.test.ts +++ b/apps/web/src/lib/settings-updates.test.ts @@ -2,7 +2,11 @@ import assert from "node:assert/strict"; import test from "node:test"; import type { WatchdogPolicy } from "@rakkr/shared"; -import { numericInputCommit, watchdogPolicyUpdate } from "./settings-updates"; +import { + numericInputCommit, + watchdogPolicyUpdate, + withWatchdogDisplayDefaults, +} from "./settings-updates"; test("numericInputCommit never coerces an empty/invalid numeric field to 0", () => { // The bug: a cleared watchdog threshold field yields Number("") === 0, which @@ -16,6 +20,62 @@ test("numericInputCommit never coerces an empty/invalid numeric field to 0", () assert.equal(numericInputCommit("-18.5"), -18.5); assert.equal(numericInputCommit("0.97"), 0.97); assert.equal(numericInputCommit("120"), 120); + assert.equal(numericInputCommit(".5"), 0.5); + assert.equal(numericInputCommit("+3"), 3); +}); + +test("numericInputCommit rejects non-decimal shapes Number() would otherwise parse", () => { + // Number("0x1f") === 31, Number("1e3") === 1000, Number("0b101") === 5. None + // are valid for dBFS/score/second fields; commit nothing rather than a + // surprising value (audit R7-NUMCOMMIT-HEX). + assert.equal(numericInputCommit("0x1f"), undefined); + assert.equal(numericInputCommit("0b101"), undefined); + assert.equal(numericInputCommit("0o17"), undefined); + assert.equal(numericInputCommit("1e3"), undefined); + assert.equal(numericInputCommit("Infinity"), undefined); + assert.equal(numericInputCommit("12px"), undefined); +}); + +test("withWatchdogDisplayDefaults fills unset optional fields so the form round-trips", () => { + // A policy with the optional threshold/mode fields unset. The card renders + // each with a `?? fallback`; the fold must persist those same values so a save + // without touching them keeps what the operator saw (audit W4A). + const sparse: WatchdogPolicy = { + activeDuring: "scheduled_recording", + graceSeconds: 0, + id: "watchdog_sparse", + metric: "rms", + minCumulativeSecondsAboveThreshold: 7, + name: "Sparse", + repeatEverySeconds: 900, + severity: "warning", + thresholdDbfs: -45, + windowSeconds: 60, + }; + + const folded = withWatchdogDisplayDefaults(sparse); + + assert.equal(folded.channelCorrelationMode, "off"); + assert.equal(folded.clippingMode, "off"); + assert.equal(folded.flatlineMode, "off"); + assert.equal(folded.qualityAlertMode, "off"); + assert.equal(folded.channelCorrelationThreshold, 0.98); + assert.equal(folded.flatlineThresholdDbfs, -100); + assert.equal(folded.minCumulativeClippingSeconds, 1); + assert.equal(folded.minCumulativeFlatlineSeconds, 10); + assert.equal(folded.broadbandNoiseScoreThreshold, 0.85); + assert.equal(folded.noiseScoreThreshold, 0.9); + assert.equal(folded.humScoreThreshold, 0.8); + assert.equal(folded.staticScoreThreshold, 0.8); + // The two cumulative-seconds fields fall back to the shared baseline. + assert.equal(folded.minCumulativeChannelCorrelationSeconds, 7); + assert.equal(folded.minCumulativeQualitySeconds, 7); +}); + +test("withWatchdogDisplayDefaults leaves already-set fields untouched", () => { + const populated = watchdogPolicy(); + + assert.deepEqual(withWatchdogDisplayDefaults(populated), populated); }); test("watchdog policy update preserves quality and flatline fields", () => { diff --git a/apps/web/src/lib/settings-updates.ts b/apps/web/src/lib/settings-updates.ts index 2cb3048b..91a371ef 100644 --- a/apps/web/src/lib/settings-updates.ts +++ b/apps/web/src/lib/settings-updates.ts @@ -12,11 +12,22 @@ import type { // validation and arms an always-fire alert (audit H4-2). Callers keep a local // text buffer so the field stays clearable while typing. export function numericInputCommit(raw: string): number | undefined { - if (raw.trim() === "") { + const trimmed = raw.trim(); + + if (trimmed === "") { return undefined; } - const parsed = Number(raw); + // Only accept a plain decimal shape (optional sign, digits, optional + // fraction). `Number()` also parses hex/octal/binary ("0x1f") and exponent + // forms, none of which are valid for these operational fields (dBFS, 0–1 + // scores, seconds) and would silently commit a surprising value + // (audit R7-NUMCOMMIT-HEX). + if (!/^[+-]?(\d+(\.\d*)?|\.\d+)$/.test(trimmed)) { + return undefined; + } + + const parsed = Number(trimmed); return Number.isFinite(parsed) ? parsed : undefined; } @@ -36,6 +47,36 @@ export function recordingProfileUpdate(profile: RecordingProfile): RecordingProf }; } +// Normalize a watchdog policy into a fully-populated editor draft: fill every +// optional field the card renders with a `?? fallback` so the value shown in the +// form is exactly what a save persists. Without this, a policy with an unset +// optional field showed the fallback (e.g. a 0.98 correlation threshold) but +// saved it as unset — the displayed value silently did not round-trip +// (audit W4A-WATCHDOG-DISPLAY-DEFAULT). Modes fold to "off" (disabled), numeric +// thresholds to their built-in fallback, and the two cumulative-seconds fields +// to the shared `minCumulativeSecondsAboveThreshold` baseline. +export function withWatchdogDisplayDefaults(policy: WatchdogPolicy): WatchdogPolicy { + return { + ...policy, + broadbandNoiseScoreThreshold: policy.broadbandNoiseScoreThreshold ?? 0.85, + channelCorrelationMode: policy.channelCorrelationMode ?? "off", + channelCorrelationThreshold: policy.channelCorrelationThreshold ?? 0.98, + clippingMode: policy.clippingMode ?? "off", + flatlineMode: policy.flatlineMode ?? "off", + flatlineThresholdDbfs: policy.flatlineThresholdDbfs ?? -100, + humScoreThreshold: policy.humScoreThreshold ?? 0.8, + minCumulativeChannelCorrelationSeconds: + policy.minCumulativeChannelCorrelationSeconds ?? policy.minCumulativeSecondsAboveThreshold, + minCumulativeClippingSeconds: policy.minCumulativeClippingSeconds ?? 1, + minCumulativeFlatlineSeconds: policy.minCumulativeFlatlineSeconds ?? 10, + minCumulativeQualitySeconds: + policy.minCumulativeQualitySeconds ?? policy.minCumulativeSecondsAboveThreshold, + noiseScoreThreshold: policy.noiseScoreThreshold ?? 0.9, + qualityAlertMode: policy.qualityAlertMode ?? "off", + staticScoreThreshold: policy.staticScoreThreshold ?? 0.8, + }; +} + export function watchdogPolicyUpdate(policy: WatchdogPolicy): WatchdogPolicyUpdate { return { activeDuring: policy.activeDuring, diff --git a/apps/web/src/lib/status-colors.ts b/apps/web/src/lib/status-colors.ts index 5b934f28..fbaf3cd0 100644 --- a/apps/web/src/lib/status-colors.ts +++ b/apps/web/src/lib/status-colors.ts @@ -94,5 +94,12 @@ export function toneFillClass(tone: StatusTone): string { return "border-emerald-200 dark:border-emerald-900 bg-emerald-500/70 dark:bg-emerald-950/50 text-emerald-800 dark:text-emerald-200"; } - return "border-sky-200 dark:border-sky-900 bg-sky-400/75 dark:bg-sky-950/50 text-sky-800 dark:text-sky-200"; + if (tone === "info") { + return "border-sky-200 dark:border-sky-900 bg-sky-400/75 dark:bg-sky-950/50 text-sky-800 dark:text-sky-200"; + } + + // Neutral: muted fill matching the other tone variants. Previously neutral + // fell through to the sky "info" fill, mislabeling neutral segments as info + // (audit R3-2). + return "border-border bg-muted text-muted-foreground"; } diff --git a/apps/web/src/pages/dashboard.tsx b/apps/web/src/pages/dashboard.tsx index fa19ad08..2d422eef 100644 --- a/apps/web/src/pages/dashboard.tsx +++ b/apps/web/src/pages/dashboard.tsx @@ -33,7 +33,7 @@ import { formatDateTime } from "@/lib/dates"; import { healthEventTargetLabel, readableHealthEventType } from "@/lib/health-page-helpers"; import { recordingJobStatusClass, recordingJobStopActionState } from "@/lib/jobs-page-helpers"; import { nodePickerFilters } from "@/lib/node-page-helpers"; -import { nodeStatusBadgeClass } from "@/lib/node-status"; +import { nodeStatusBadgeClass, nodeStatusLabel } from "@/lib/node-status"; import { toneBadgeClass, toneTextClass } from "@/lib/status-colors"; import { cn } from "@/lib/utils"; @@ -189,7 +189,7 @@ export function DashboardPage() {
{node.alias} - {node.status} + {nodeStatusLabel(node.status)}
diff --git a/apps/web/src/pages/nodes.tsx b/apps/web/src/pages/nodes.tsx index 9d40a6ea..5f707314 100644 --- a/apps/web/src/pages/nodes.tsx +++ b/apps/web/src/pages/nodes.tsx @@ -67,7 +67,7 @@ import { type NodeFilterKey, type NodeHealthLifecycleAction, } from "@/lib/node-page-helpers"; -import { nodeStatusBadgeClass } from "@/lib/node-status"; +import { nodeStatusBadgeClass, nodeStatusLabel } from "@/lib/node-status"; import { toneBadgeClass } from "@/lib/status-colors"; import { downloadBlob } from "@/lib/recording-page-helpers"; import { defaultPageSize } from "@/lib/server-pagination"; @@ -358,7 +358,7 @@ function nodeColumns({ onToggleSelected, selectedNodeIds, }: NodeColumnOptions): ColumnDef[] { - return [ + const columns: ColumnDef[] = [ { cell: ({ row }) => ( ( - {row.original.status} + {nodeStatusLabel(row.original.status)} ), header: "Status", @@ -456,18 +456,25 @@ function nodeColumns({ header: "IPs / serial", id: "network", }, - { - cell: ({ row }) => - canManage ? ( -
- -
- ) : null, + ]; + + // Only add the Actions column for managers; a read-only operator otherwise + // gets an always-empty column with a dangling "Actions" header (audit + // W3-EMPTY-CARD-FOOTER), mirroring the conditional-column pattern elsewhere. + if (canManage) { + columns.push({ + cell: ({ row }) => ( +
+ +
+ ), header: "Actions", id: "actions", meta: { cellClassName: "text-right", headClassName: "text-right" }, - }, - ]; + }); + } + + return columns; } function NodeDetailRow({ diff --git a/apps/web/src/pages/room-detail.tsx b/apps/web/src/pages/room-detail.tsx index 59e7cd9f..28c7eb4f 100644 --- a/apps/web/src/pages/room-detail.tsx +++ b/apps/web/src/pages/room-detail.tsx @@ -36,7 +36,7 @@ import { toast } from "sonner"; import { api, apiErrorStatus } from "@/lib/api"; import { formatDateTime } from "@/lib/dates"; import { useDocumentTitle } from "@/lib/document-title"; -import { nodeStatusBadgeClass } from "@/lib/node-status"; +import { nodeStatusBadgeClass, nodeStatusLabel } from "@/lib/node-status"; import { roomDraftFromRoom, roomDraftToUpdate, @@ -226,7 +226,7 @@ export function RoomDetailPage({ roomId }: { roomId: string }) { {node.alias} {node.hostname} - {node.status} + {nodeStatusLabel(node.status)}
))} diff --git a/apps/web/src/pages/schedules-calendar.tsx b/apps/web/src/pages/schedules-calendar.tsx index c81baead..12fc6671 100644 --- a/apps/web/src/pages/schedules-calendar.tsx +++ b/apps/web/src/pages/schedules-calendar.tsx @@ -28,7 +28,12 @@ import { import { cn } from "@/lib/utils"; import { nodePickerFilters } from "@/lib/node-page-helpers"; import { schedulePageActionPermissions } from "@/lib/schedule-page-helpers"; -import { defaultDraft, draftToInput, type ScheduleDraft } from "@/lib/schedule-draft"; +import { + defaultDraft, + draftToInput, + type ScheduleDraft, + type SchedulingDefaultAvailability, +} from "@/lib/schedule-draft"; const maxVisibleChips = 3; @@ -281,14 +286,31 @@ export function SchedulesCalendarPage() { setDraft({ // Prefill the operator's configured scheduling defaults, mirroring the // schedules list page — the calendar create path previously dropped the - // second arg and always fell back to the built-ins (audit S1). - ...defaultDraft(firstNode, controllerSettingsQuery.data?.data), + // second arg and always fell back to the built-ins (audit S1). Validate + // them against the dialog's cached lists so a deleted default is not + // prefilled (audit S3); a cold cache trusts the default. + ...defaultDraft(firstNode, controllerSettingsQuery.data?.data, schedulingAvailability()), recurrenceMode: "once", recurrenceStartAt: `${cell.iso}T09:00`, }); setCreateDialogOpen(true); } + function schedulingAvailability(): SchedulingDefaultAvailability { + const cachedIds = (key: string) => { + const cached = queryClient.getQueryData<{ data: Array<{ id: string }> }>([key]); + + return cached ? cached.data.map((item) => item.id) : undefined; + }; + + return { + recordingProfileIds: cachedIds("recording-profiles"), + retentionPolicyIds: cachedIds("retention-policies"), + uploadPolicyIds: cachedIds("upload-policies"), + watchdogPolicyIds: cachedIds("watchdog-policies"), + }; + } + function closeCreate() { setCreateDialogOpen(false); setDraft(undefined); diff --git a/apps/web/src/pages/schedules.tsx b/apps/web/src/pages/schedules.tsx index 3f74753c..2e94e372 100644 --- a/apps/web/src/pages/schedules.tsx +++ b/apps/web/src/pages/schedules.tsx @@ -46,6 +46,7 @@ import { recurrenceSummary, scheduleToDraft, type ScheduleDraft, + type SchedulingDefaultAvailability, } from "@/lib/schedule-draft"; import { defaultPageSize } from "@/lib/server-pagination"; import { useServerPagination } from "@/lib/use-server-pagination"; @@ -293,9 +294,27 @@ export function SchedulesPage() {
); + // Validate operator defaults against the schedule dialog's cached profile/ + // policy lists so a since-deleted default is not prefilled (audit S3). A cold + // cache (lists not yet loaded) yields `undefined`, which trusts the default. + function schedulingAvailability(): SchedulingDefaultAvailability { + const cachedIds = (key: string) => { + const cached = queryClient.getQueryData<{ data: Array<{ id: string }> }>([key]); + + return cached ? cached.data.map((item) => item.id) : undefined; + }; + + return { + recordingProfileIds: cachedIds("recording-profiles"), + retentionPolicyIds: cachedIds("retention-policies"), + uploadPolicyIds: cachedIds("upload-policies"), + watchdogPolicyIds: cachedIds("watchdog-policies"), + }; + } + function openCreate() { setEditingId(undefined); - setDraft(defaultDraft(firstNode, schedulingDefaults)); + setDraft(defaultDraft(firstNode, schedulingDefaults, schedulingAvailability())); setDialogOpen(true); } @@ -308,7 +327,7 @@ export function SchedulesPage() { function closeDialog() { setDialogOpen(false); setEditingId(undefined); - setDraft(defaultDraft(firstNode, schedulingDefaults)); + setDraft(defaultDraft(firstNode, schedulingDefaults, schedulingAvailability())); } function submitSchedule() { From d85277afe64cb0b39bd5acdf363f9715d5c8605f Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 19:48:57 +0500 Subject: [PATCH 32/33] Cap recorder-agent ipAddresses at the documented heartbeat limit R7-IP-AGENT-CAP: refactor collect_ip_addresses to a pure parse_ip_addresses helper bounded to MAX_IP_ADDRESSES (16, matching nodeHeartbeatSchema), so a well-behaved agent never emits a payload the controller must truncate. Pure Rust unit test (cap at 16, normal parse, empty). Co-Authored-By: Claude Opus 4.8 --- crates/recorder-agent/src/inventory.rs | 41 +++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/crates/recorder-agent/src/inventory.rs b/crates/recorder-agent/src/inventory.rs index 30b3ac9c..a7e7c274 100644 --- a/crates/recorder-agent/src/inventory.rs +++ b/crates/recorder-agent/src/inventory.rs @@ -210,6 +210,13 @@ fn discover_proc_asound_interfaces( .unwrap_or_default() } +// The controller caps a heartbeat's `ipAddresses` at 16 (nodeHeartbeatSchema) +// and truncates an over-cap list rather than reject it. A well-behaved agent +// should never emit more than the cap in the first place, so bound the list +// here too (a multi-homed host — IPv6 SLAAC/privacy + Docker/libvirt/VLAN +// bridges — can exceed 16). See audit R7-IP-AGENT-CAP / R7-IPCAP. +const MAX_IP_ADDRESSES: usize = 16; + fn collect_ip_addresses() -> Vec { let Ok(output) = Command::new("hostname").arg("-I").output() else { return Vec::new(); @@ -219,8 +226,16 @@ fn collect_ip_addresses() -> Vec { return Vec::new(); } - String::from_utf8_lossy(&output.stdout) + parse_ip_addresses(&String::from_utf8_lossy(&output.stdout)) +} + +// Pure parser for `hostname -I` output: whitespace-split, bounded to the +// documented cap so the agent never sends a payload the controller would have +// to truncate. +fn parse_ip_addresses(stdout: &str) -> Vec { + stdout .split_whitespace() + .take(MAX_IP_ADDRESSES) .map(str::to_string) .collect() } @@ -645,6 +660,30 @@ where mod tests { use super::*; + #[test] + fn parses_ip_addresses_and_caps_at_the_documented_limit() { + // A normal multi-address host parses to a trimmed list. + assert_eq!( + parse_ip_addresses("192.168.1.10 10.0.0.5 \n"), + vec!["192.168.1.10".to_string(), "10.0.0.5".to_string()], + ); + + // A host with more than the cap (e.g. IPv6 SLAAC/privacy + bridges) is + // bounded to MAX_IP_ADDRESSES so the controller never has to truncate. + let many = (0..40) + .map(|n| format!("10.0.0.{n}")) + .collect::>() + .join(" "); + let parsed = parse_ip_addresses(&many); + + assert_eq!(parsed.len(), MAX_IP_ADDRESSES); + assert_eq!(parsed.first().map(String::as_str), Some("10.0.0.0")); + assert_eq!(parsed.last().map(String::as_str), Some("10.0.0.15")); + + // Empty output yields no addresses. + assert!(parse_ip_addresses(" \n").is_empty()); + } + #[test] fn parses_arecord_capture_devices() { let devices = parse_alsa_capture_devices( From 3b0aa84712170a57e81eeeecd93720141679760e Mon Sep 17 00:00:00 2001 From: yashau Date: Wed, 8 Jul 2026 19:48:57 +0500 Subject: [PATCH 33/33] Record post-convergence catalogued-entry cleanup in the gap-hunt ledger Document the disposition of the 2026-07-08 gap-hunt catalogue after convergence: 21 items fixed (each with a test where a unit seam exists), 6 deferred with rationale (render-harness-gated or product decisions, plus R7-SEED-LIVENESS as won't-fix). Records the two operator product decisions (offline tone -> Critical; switcher password-clear -> defer) and the green full-check gate at close. Co-Authored-By: Claude Opus 4.8 --- docs/internal/audits/2026-07-08-gap-hunt.md | 63 +++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/docs/internal/audits/2026-07-08-gap-hunt.md b/docs/internal/audits/2026-07-08-gap-hunt.md index 27baf4b9..f6213806 100644 --- a/docs/internal/audits/2026-07-08-gap-hunt.md +++ b/docs/internal/audits/2026-07-08-gap-hunt.md @@ -99,6 +99,14 @@ fixes clean (no regressions) and surfaced only LOW items. ## Open work +> **Post-convergence update (2026-07-08):** after convergence the operator asked +> to work through the catalogued backlog. **21 of the open items below are now +> FIXED** (each with a red→green or coverage-lock test where a unit seam exists); +> the rest are **deferred** with rationale. See +> [Post-convergence: catalogued-entry cleanup](#post-convergence-catalogued-entry-cleanup) +> for the full disposition and commits. The table below is the state **at +> convergence** (Run 13); the cleanup section is authoritative for current status. + ### Catalogued / suspected | ID | Sev | Kind | Locus | Note | @@ -411,3 +419,58 @@ loop's value showed most in Runs 6–8, where fresh adversarial angles (a catalo re-triage, a Rust-contract pass, a web re-sweep) each caught a real defect the earlier "clean" verdicts had rested on — exactly the non-determinism the iterate- until-quiet procedure exists to absorb. + +--- + +## Post-convergence: catalogued-entry cleanup + +After convergence the operator asked to work through the catalogued backlog +("now handle any catalogued entries"). None of these were confirmed functional +regressions (that is why the audit converged), but most were legitimate +hardening / consistency / UX-honesty improvements. **21 items fixed, 6 deferred.** +Each fix has a red→green or coverage-lock test where a unit seam exists; the +React-render-only items are verified by tsc + oxlint + build (the web unit +harness has no RTL/jsdom). Two product decisions were made by the operator: +**offline status tone → Critical/red** (`R3-1`) and **switcher password-clear → +defer the UI affordance** (`R3-3`). + +Gate at close of cleanup: **full `mise run check` green** (tsc, API + web node +tests, oxlint, oxfmt, `check:loc`, `db:verify`, all baseline verifiers, +`rust:check`/`clippy`/`fmt`/`miri`, fake-controller-smoke) + `mise run build`. + +### Fixed + +| ID | Sev | Area | Fix | Proof | +| --- | --- | --- | --- | --- | +| R4-1-SERVER-DEDUP | Low | api/schedule | `buildSchedule` + `sanitizeScheduleUpdate` now dedup `uploadPolicyIds` via `[...new Set(...)]` (mirrors `uniqueTags`), so the server is authoritative and a non-browser client can't persist duplicates. | `schedule-route-helpers.test.ts` (create + patch dedup, absent-field untouched) | +| R4-2-LIVENESS-ISOLATION | Low | api/watchdog | Per-node body of `reconcileNodeLivenessEvents` wrapped in try/catch → `{outcome:"skipped",reason:"reconcile_failed"}` and continue, so one node's store/write failure no longer aborts the whole tick. | `watchdog-node-liveness.test.ts` (bad node isolated, later node still alerts) | +| H1-3-PROVISIONING-REASON | Low | api/node-actions | Added `"provisioning"` to `unavailableNodeStatuses`; listen/meters/start now report `node_provisioning` (accurate) instead of `node_offline`/`monitor_source_unavailable`. | `node-action-routes.test.ts` (provisioning node → disabled + `node_provisioning`) | +| N-3A-RELEASE-PAGING | Low | api/agent-release | `doFetch` follows `Link: rel=next` up to `MAX_RELEASE_PAGES` (5), same-origin-only, so the newest `agent-v…` is found even behind a burst of docs/controller tags. | `agent-release-service.test.ts` (finds release on page 2; stops at page cap; `parseNextLink` unit) | +| N-3B-RELEASE-BODY | Low | api/agent-release | Response body read through `readBoundedText` (Content-Length pre-check + streaming byte cap `MAX_RELEASE_BODY_BYTES` = 8 MiB); an over-cap body is rejected and the last-good value retained. | `agent-release-service.test.ts` (over-cap body → keeps last good) | +| N-COV-FETCH | Low | api/coverage | Locked the GitHub request contract: URL, `Accept`/`User-Agent`/`X-GitHub-Api-Version`, `Bearer` only when a token is set. | `agent-release-service.test.ts` (request-contract assertions) | +| R11-CAP-COV | Low | shared/coverage | Added the missing `>32` rejection test for `scheduleInputSchema.uploadPolicyIds` (only the client dedup was covered before). | `input-hardening.test.ts` (32 ok, 33 rejected) | +| R3-1-OFFLINE-TONE | Low-Med | web/status | **Operator decision: Critical.** `nodeStatusBadgeClass` rewritten as an exhaustive switch; `offline` now reads **critical (red)**, the same weight as `alerting`, not a muted neutral. | `node-status.test.ts` (every enum pinned; offline ≠ neutral) | +| R3-2-TONEFILL-NEUTRAL | Low | web/status | `toneFillClass` given an explicit `info` branch + a muted `neutral` fallback (previously `neutral` fell through to the sky "info" fill). | `node-status.test.ts` (neutral fill ≠ info fill) | +| H4-2/H4-3-NUMBER-INPUT | Low | web/settings | Extracted the buffered `NumberField` into the shared `settings-fields.tsx` (single source of truth) and wired it into the recording-profile (bitrate + enhancement Hz/LUFS), upload-policy (attempts), and watchdog cards; retention's `optionalNumber` now routes through `numericInputCommit` (empty/invalid → `null`, never `NaN`). | `settings-updates.test.ts` (helper) + tsc/build (components) | +| R7-NUMCOMMIT-HEX | Low | web/settings | `numericInputCommit` now gates on a decimal-shape regex, so `Number()`'s hex/octal/binary/exponent parsing can't commit a surprising value. | `settings-updates.test.ts` (0x1f/1e3/0b101/… → undefined) | +| R9-NODEIFACE-SELECT | Low | web/schedule-form | Node + interface `