diff --git a/apps/api/src/agent-release-service.ts b/apps/api/src/agent-release-service.ts index b658246..5caf244 100644 --- a/apps/api/src/agent-release-service.ts +++ b/apps/api/src/agent-release-service.ts @@ -52,6 +52,14 @@ interface AgentReleaseServiceOptions { const DEFAULT_TTL_MS = 30 * 60 * 1000; const DEFAULT_ERROR_TTL_MS = 5 * 60 * 1000; const DEFAULT_TIMEOUT_MS = 10_000; +// The newest agent release is normally on page 1 (GitHub lists newest-first), but +// a burst of docs/controller tags could push it back. Follow `Link: rel=next` a +// bounded number of pages so we still find it without unbounded paging (R N-3A). +const MAX_RELEASE_PAGES = 5; +// Never buffer an unbounded response body into memory. 100 releases/page of +// GitHub release JSON is well under this; a body larger than this from a +// misconfigured/hostile API URL is rejected rather than read (R N-3B). +const MAX_RELEASE_BODY_BYTES = 8 * 1024 * 1024; export function createAgentReleaseService( options: AgentReleaseServiceOptions = {}, @@ -72,22 +80,42 @@ export function createAgentReleaseService( async function doFetch(): Promise { try { - const response = await fetchImpl(`${apiUrl}/repos/${repo}/releases?per_page=100`, { - headers: { - Accept: "application/vnd.github+json", - "User-Agent": "rakkr-controller", - "X-GitHub-Api-Version": "2022-11-28", - ...(token ? { Authorization: `Bearer ${token}` } : {}), - }, - signal: AbortSignal.timeout(timeoutMs), - }); + const firstUrl = `${apiUrl}/repos/${repo}/releases?per_page=100`; + const origin = new URL(firstUrl).origin; + const entries: unknown[] = []; + let url: string | null = firstUrl; + + for (let page = 0; page < MAX_RELEASE_PAGES && url; page += 1) { + const response = await fetchImpl(url, { + headers: { + Accept: "application/vnd.github+json", + "User-Agent": "rakkr-controller", + "X-GitHub-Api-Version": "2022-11-28", + ...(token ? { Authorization: `Bearer ${token}` } : {}), + }, + signal: AbortSignal.timeout(timeoutMs), + }); + + if (!response.ok) { + throw new Error(`github_releases_${response.status}`); + } + + const parsed = JSON.parse( + await readBoundedText(response, MAX_RELEASE_BODY_BYTES), + ) as unknown; + + if (Array.isArray(parsed)) { + entries.push(...parsed); + } - if (!response.ok) { - throw new Error(`github_releases_${response.status}`); + // Only follow a next link that stays on the same origin — don't chase a + // header pointing at an unrelated host. + const next = parseNextLink(response.headers.get("link")); + + url = next && new URL(next).origin === origin ? next : null; } - const body = (await response.json()) as unknown; - const resolved = resolveLatestAgentRelease(body); + const resolved = resolveLatestAgentRelease(entries); if (resolved) { release = resolved; @@ -160,6 +188,75 @@ export function resolveLatestAgentRelease(body: unknown): AgentRelease | null { return latest; } +// Extracts the `rel="next"` URL from a GitHub `Link` header, or null when there +// is no next page. GitHub emits `; rel="next", ; rel="last"`. +export function parseNextLink(linkHeader: string | null): string | null { + if (!linkHeader) { + return null; + } + + for (const part of linkHeader.split(",")) { + const match = /<([^>]+)>\s*;\s*rel="?next"?/.exec(part); + + if (match) { + return match[1]; + } + } + + return null; +} + +// Reads a response body as text but refuses to buffer more than `maxBytes`, +// checking the declared Content-Length first and then enforcing the cap while +// streaming (chunked responses omit Content-Length). +async function readBoundedText(response: Response, maxBytes: number): Promise { + const declared = Number(response.headers.get("content-length")); + + if (Number.isFinite(declared) && declared > maxBytes) { + throw new Error("github_releases_body_too_large"); + } + + const body = response.body; + + if (!body) { + const text = await response.text(); + + if (byteLength(text) > maxBytes) { + throw new Error("github_releases_body_too_large"); + } + + return text; + } + + const reader = body.getReader(); + const decoder = new TextDecoder(); + let received = 0; + let text = ""; + + for (;;) { + const { done, value } = await reader.read(); + + if (done) { + break; + } + + received += value.byteLength; + + if (received > maxBytes) { + await reader.cancel(); + throw new Error("github_releases_body_too_large"); + } + + text += decoder.decode(value, { stream: true }); + } + + return text + decoder.decode(); +} + +function byteLength(value: string): number { + return new TextEncoder().encode(value).length; +} + let defaultService: AgentReleaseService | undefined; export function agentReleaseService(): AgentReleaseService { diff --git a/apps/api/src/agent-route-helpers.ts b/apps/api/src/agent-route-helpers.ts index 210a430..647a1bf 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/src/agent-routes.ts b/apps/api/src/agent-routes.ts index 6fd1908..b8ee86b 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/src/metrics.ts b/apps/api/src/metrics.ts index 53f4b5d..c15f4b1 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/src/node-action-routes.ts b/apps/api/src/node-action-routes.ts index 44acbcc..20366d6 100644 --- a/apps/api/src/node-action-routes.ts +++ b/apps/api/src/node-action-routes.ts @@ -26,7 +26,10 @@ interface NodeActionState { } const monitorChunkMaxAgeMs = 5000; -const unavailableNodeStatuses = new Set(["offline"]); +// A provisioning node has never sent a heartbeat, so listen/meters/start are as +// unavailable as an offline node — but the reason differs (it is not "offline", +// it has never been online), so callers get an accurate label (audit H1-3). +const unavailableNodeStatuses = new Set(["offline", "provisioning"]); export function registerNodeActionRoutes({ app, @@ -103,6 +106,7 @@ function nodeActions( ) { const basePath = `/api/v1/nodes/${node.id}`; const nodeAvailable = !unavailableNodeStatuses.has(node.status); + const unavailableReason = node.status === "provisioning" ? "node_provisioning" : "node_offline"; return { detail: actionState({ @@ -132,7 +136,7 @@ function nodeActions( permission: "listen:monitor", permissions, ready: nodeAvailable && readiness.listen, - reason: nodeAvailable ? "monitor_source_unavailable" : "node_offline", + reason: nodeAvailable ? "monitor_source_unavailable" : unavailableReason, }), meters: actionState({ href: `${basePath}/meters`, @@ -140,7 +144,7 @@ function nodeActions( permission: "node:read", permissions, ready: nodeAvailable && readiness.meters, - reason: nodeAvailable ? "meter_frame_not_found" : "node_offline", + reason: nodeAvailable ? "meter_frame_not_found" : unavailableReason, }), rotateCredential: actionState({ href: `${basePath}/credentials/rotate`, @@ -156,7 +160,7 @@ function nodeActions( permission: "recording:create", permissions, ready: nodeAvailable, - reason: "node_offline", + reason: unavailableReason, }), }; } diff --git a/apps/api/src/node-routes.ts b/apps/api/src/node-routes.ts index dcf2c21..1af606f 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/src/node-store-updates.ts b/apps/api/src/node-store-updates.ts index afb5084..feddde4 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 7e8ec58..5bde4d0 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/src/schedule-route-helpers.ts b/apps/api/src/schedule-route-helpers.ts index ff68d59..8635580 100644 --- a/apps/api/src/schedule-route-helpers.ts +++ b/apps/api/src/schedule-route-helpers.ts @@ -62,7 +62,9 @@ export function buildSchedule(input: ScheduleInput): ScheduleSummary { tags: uniqueTags(input.tags), timezone: input.timezone, titleTemplate: input.titleTemplate, - uploadPolicyIds: input.uploadPolicyIds, + // Dedup server-side (the client also dedups) so the server is authoritative: + // each id fans a recording out to its own upload queue item (audit R4-1). + uploadPolicyIds: [...new Set(input.uploadPolicyIds)], watchdogPolicyId: input.watchdogPolicyId, }; } @@ -105,6 +107,10 @@ export function sanitizeScheduleUpdate( updates.tags = uniqueTags(input.tags); } + if (input.uploadPolicyIds) { + updates.uploadPolicyIds = [...new Set(input.uploadPolicyIds)]; + } + return updates; } diff --git a/apps/api/src/settings-upload-policy-routes.ts b/apps/api/src/settings-upload-policy-routes.ts index 50f5bff..4ce96a5 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/src/watchdog-node-liveness.ts b/apps/api/src/watchdog-node-liveness.ts index bddbc74..22bc2e1 100644 --- a/apps/api/src/watchdog-node-liveness.ts +++ b/apps/api/src/watchdog-node-liveness.ts @@ -40,31 +40,38 @@ export async function reconcileNodeLivenessEvents({ continue; } - const existing = await activeNodeOfflineEvent(healthEventStore, node.id); + // Isolate each node's reconcile: a store failure (or health-event write + // error) for one node must not abort the sweep and leave every later node + // unreconciled — skip the failing one and keep going (audit R4-2). + try { + const existing = await activeNodeOfflineEvent(healthEventStore, node.id); - if (nodeHeartbeatStale(node, now)) { - results.push( - await writeNodeOfflineEvent({ - auditStore, - existing, - healthEventStore, - node, - now, - }), - ); - continue; - } + if (nodeHeartbeatStale(node, now)) { + results.push( + await writeNodeOfflineEvent({ + auditStore, + existing, + healthEventStore, + node, + now, + }), + ); + continue; + } - if (existing) { - results.push( - await resolveNodeOfflineEvent({ - auditStore, - existing, - healthEventStore, - node, - now, - }), - ); + if (existing) { + results.push( + await resolveNodeOfflineEvent({ + auditStore, + existing, + healthEventStore, + node, + now, + }), + ); + } + } catch { + results.push({ nodeId: node.id, outcome: "skipped", reason: "reconcile_failed" }); } } diff --git a/apps/api/test/agent-heartbeat-routes.test.ts b/apps/api/test/agent-heartbeat-routes.test.ts index 6303e7d..b869852 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), diff --git a/apps/api/test/agent-job-read-routes.test.ts b/apps/api/test/agent-job-read-routes.test.ts index 1438f73..005e9db 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(""); diff --git a/apps/api/test/agent-release-service.test.ts b/apps/api/test/agent-release-service.test.ts index 384f8cb..416de3d 100644 --- a/apps/api/test/agent-release-service.test.ts +++ b/apps/api/test/agent-release-service.test.ts @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import test from "node:test"; -const { createAgentReleaseService, resolveLatestAgentRelease } = +const { createAgentReleaseService, parseNextLink, resolveLatestAgentRelease } = await import("../src/agent-release-service.js"); const releasesPage = (tags: string[]) => @@ -84,6 +84,123 @@ test("stale-while-revalidate serves the old value then refreshes", async () => { assert.equal(fetchCalls, 2); }); +test("parseNextLink extracts the rel=next URL and ignores others", () => { + const header = + '; rel="next", ' + + '; rel="last"'; + + assert.equal( + parseNextLink(header), + "https://api.github.com/repositories/1/releases?per_page=100&page=2", + ); + // A header with only prev/last (no next) yields null. + assert.equal(parseNextLink('; rel="prev"'), null); + assert.equal(parseNextLink(null), null); +}); + +test("doFetch sends the documented GitHub request contract", async () => { + const requests: Array<{ headers: Headers; url: string }> = []; + const service = createAgentReleaseService({ + fetchImpl: async (input, init) => { + requests.push({ + headers: new Headers(init?.headers), + url: typeof input === "string" ? input : String(input), + }); + return new Response(JSON.stringify(releasesPage(["agent-v2026.06.28-1"])), { status: 200 }); + }, + now: () => new Date("2026-07-05T00:00:00.000Z"), + repo: "acme/widgets", + token: "ghp_secret", + }); + + await service.warm(); + + assert.equal(requests.length, 1); + assert.equal(requests[0].url, "https://api.github.com/repos/acme/widgets/releases?per_page=100"); + assert.equal(requests[0].headers.get("accept"), "application/vnd.github+json"); + assert.equal(requests[0].headers.get("user-agent"), "rakkr-controller"); + assert.equal(requests[0].headers.get("x-github-api-version"), "2022-11-28"); + assert.equal(requests[0].headers.get("authorization"), "Bearer ghp_secret"); +}); + +test("doFetch follows Link rel=next to find a newer release on a later page", async () => { + const urls: string[] = []; + const page2 = "https://api.github.com/repos/yashau/Rakkr/releases?per_page=100&page=2"; + const service = createAgentReleaseService({ + fetchImpl: async (input) => { + const url = typeof input === "string" ? input : String(input); + urls.push(url); + + if (url.includes("page=2")) { + // Newest agent release lives on page 2 (page 1 was all docs tags). + return new Response(JSON.stringify(releasesPage(["agent-v2026.07.04-1"])), { status: 200 }); + } + + return new Response(JSON.stringify(releasesPage(["docs-v2026.07.03-1"])), { + headers: { Link: `<${page2}>; rel="next"` }, + status: 200, + }); + }, + now: () => new Date("2026-07-05T00:00:00.000Z"), + }); + + await service.warm(); + + assert.equal(urls.length, 2); + assert.equal(urls[1], page2); + assert.equal(service.snapshot().data?.version, "2026.07.04-1"); +}); + +test("doFetch stops paging at the page cap", async () => { + let fetchCalls = 0; + const service = createAgentReleaseService({ + fetchImpl: async (input) => { + fetchCalls += 1; + const url = typeof input === "string" ? input : String(input); + const page = Number(new URL(url).searchParams.get("page") ?? "1"); + // Every page advertises a next link, so only the cap stops the loop. + return new Response(JSON.stringify(releasesPage(["docs-v2026.07.03-1"])), { + headers: { + Link: `; rel="next"`, + }, + status: 200, + }); + }, + now: () => new Date("2026-07-05T00:00:00.000Z"), + }); + + await service.warm(); + + assert.equal(fetchCalls, 5); +}); + +test("doFetch rejects an over-cap body and keeps the last good value", async () => { + let fail = false; + let clock = new Date("2026-07-05T00:00:00.000Z").getTime(); + const service = createAgentReleaseService({ + fetchImpl: async () => { + if (fail) { + return new Response(JSON.stringify(releasesPage(["agent-v2026.07.04-1"])), { + headers: { "Content-Length": String(64 * 1024 * 1024) }, + status: 200, + }); + } + return new Response(JSON.stringify(releasesPage(["agent-v2026.06.28-1"])), { status: 200 }); + }, + now: () => new Date(clock), + ttlMs: 1000, + }); + + await service.warm(); + assert.equal(service.snapshot().data?.version, "2026.06.28-1"); + + clock += 2000; + fail = true; + await service.warm(); + // The oversized body is rejected; the previous good value is retained. + assert.equal(service.snapshot().data?.version, "2026.06.28-1"); +}); + test("a failed refresh keeps the last good value and backs off", async () => { let clock = new Date("2026-07-05T00:00:00.000Z").getTime(); let fetchCalls = 0; diff --git a/apps/api/test/input-hardening.test.ts b/apps/api/test/input-hardening.test.ts index 1134365..a97254f 100644 --- a/apps/api/test/input-hardening.test.ts +++ b/apps/api/test/input-hardening.test.ts @@ -1,6 +1,11 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { ianaTimeZoneSchema, isoDateTimeSchema, meterFrameSchema } from "@rakkr/shared"; +import { + ianaTimeZoneSchema, + isoDateTimeSchema, + meterFrameSchema, + scheduleInputSchema, +} from "@rakkr/shared"; import { nodeHealthEventSchema } from "../src/agent-route-helpers.js"; import { hashPassword, verifyPassword } from "../src/password.js"; @@ -24,6 +29,25 @@ test("meterFrameSchema caps the levels array (watchdog Math.max spread guard)", assert.equal(meterFrameSchema.safeParse(frame(513)).success, false); }); +test("scheduleInputSchema caps uploadPolicyIds so one recording cannot fan out unbounded", () => { + const base = { + folderTemplate: "recordings/{room}", + name: "Council Capture", + nodeId: "node_1", + recordingProfileId: "profile_voice", + room: "Council Room", + timezone: "UTC", + titleTemplate: "{room}", + watchdogPolicyId: "watchdog_default", + }; + const ids = (count: number) => Array.from({ length: count }, (_unused, index) => `up_${index}`); + + // 32 destinations is already generous for one recording; 33 is rejected so a + // hostile/mistaken payload cannot multiply upload queue work per recording. + assert.equal(scheduleInputSchema.safeParse({ ...base, uploadPolicyIds: ids(32) }).success, true); + assert.equal(scheduleInputSchema.safeParse({ ...base, uploadPolicyIds: ids(33) }).success, false); +}); + test("ianaTimeZoneSchema rejects zones that would throw in Intl.DateTimeFormat", () => { // Valid IANA zones (and UTC) parse. for (const good of [ diff --git a/apps/api/test/metrics.test.ts b/apps/api/test/metrics.test.ts index 0e47bdc..c6fa05b 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/api/test/node-action-routes.test.ts b/apps/api/test/node-action-routes.test.ts index aac728e..bc03dc9 100644 --- a/apps/api/test/node-action-routes.test.ts +++ b/apps/api/test/node-action-routes.test.ts @@ -103,6 +103,30 @@ test("node action summary explains permission and lifecycle blockers", async () assert.equal(body.data.actions.meters.reason, "node_offline"); }); +test("node action summary marks a provisioning node unavailable with an accurate reason", async () => { + // A provisioning node has never sent a heartbeat, so it cannot record/listen/ + // meter — but the reason must say "provisioning", not "offline" (it has never + // been online), so operators are not misled into troubleshooting a downed node. + const recorder = nodeWithInterface({ id: "node_provisioning", status: "provisioning" }); + const app = nodeActionsApp({ + meterFrames: [], + nodes: [recorder], + permissionCalls: [], + user: user(["listen:monitor", "node:read", "recording:create"]), + }); + + const response = await app.request(`/api/v1/nodes/${recorder.id}/actions`); + const body = (await response.json()) as NodeActionsResponse; + + assert.equal(response.status, 200); + assert.equal(body.data.actions.listen.enabled, false); + assert.equal(body.data.actions.listen.reason, "node_provisioning"); + assert.equal(body.data.actions.meters.enabled, false); + assert.equal(body.data.actions.meters.reason, "node_provisioning"); + assert.equal(body.data.actions.startRecording.enabled, false); + assert.equal(body.data.actions.startRecording.reason, "node_provisioning"); +}); + test("node action summary separates listen source from meter readiness", async () => { const recorder = nodeWithInterface({ id: "node_chunk_only" }); const listenMonitorStore = createListenMonitorStore(); diff --git a/apps/api/test/node-routes-helpers.ts b/apps/api/test/node-routes-helpers.ts new file mode 100644 index 0000000..5c74042 --- /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 96dff81..868129c 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; -} 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 0000000..45d3898 --- /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"); +}); diff --git a/apps/api/test/schedule-route-helpers.test.ts b/apps/api/test/schedule-route-helpers.test.ts new file mode 100644 index 0000000..4bbc1cb --- /dev/null +++ b/apps/api/test/schedule-route-helpers.test.ts @@ -0,0 +1,44 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { scheduleInputSchema, scheduleUpdateSchema } from "@rakkr/shared"; +import { buildSchedule, sanitizeScheduleUpdate } from "../src/schedule-route-helpers.js"; + +const baseInput = (overrides: Record = {}) => + scheduleInputSchema.parse({ + folderTemplate: "recordings/{room}", + name: "Council Capture", + nodeId: "node_1", + recordingProfileId: "profile_voice", + room: "Council Room", + timezone: "UTC", + titleTemplate: "{room}", + watchdogPolicyId: "watchdog_default", + ...overrides, + }); + +test("buildSchedule dedups uploadPolicyIds so a recording fans out once per destination", () => { + // A recording fans out to one upload queue item per id; duplicates would + // double the upload work, so the server dedups (client also dedups) — R4-1. + const schedule = buildSchedule(baseInput({ uploadPolicyIds: ["up_a", "up_a", "up_b", "up_b"] })); + + assert.deepEqual(schedule.uploadPolicyIds, ["up_a", "up_b"]); +}); + +test("sanitizeScheduleUpdate dedups uploadPolicyIds on patch", () => { + const before = buildSchedule(baseInput({ uploadPolicyIds: ["up_a"] })); + const update = scheduleUpdateSchema.parse({ uploadPolicyIds: ["up_x", "up_x", "up_y", "up_x"] }); + + const updates = sanitizeScheduleUpdate(update, before); + + assert.deepEqual(updates.uploadPolicyIds, ["up_x", "up_y"]); +}); + +test("sanitizeScheduleUpdate leaves uploadPolicyIds untouched when the field is absent", () => { + const before = buildSchedule(baseInput({ uploadPolicyIds: ["up_a", "up_b"] })); + const update = scheduleUpdateSchema.parse({ name: "Renamed" }); + + const updates = sanitizeScheduleUpdate(update, before); + + assert.equal("uploadPolicyIds" in updates, false); + assert.equal(updates.name, "Renamed"); +}); diff --git a/apps/api/test/settings-controller-routes.test.ts b/apps/api/test/settings-controller-routes.test.ts index 9729571..9ba3e27 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(); diff --git a/apps/api/test/settings-routes.test.ts b/apps/api/test/settings-routes.test.ts index 31fd49c..0589daa 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 b448077..0d7a8bc 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,82 @@ 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); +}); + +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, diff --git a/apps/api/test/watchdog-node-liveness.test.ts b/apps/api/test/watchdog-node-liveness.test.ts new file mode 100644 index 0000000..2099ee6 --- /dev/null +++ b/apps/api/test/watchdog-node-liveness.test.ts @@ -0,0 +1,68 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import type { RecorderNode } from "@rakkr/shared"; + +import { createAuditStore } from "../src/audit-store.js"; +import { createHealthEventStore } from "../src/health-store.js"; +import type { HealthEventStore } from "../src/health-store.js"; +import { reconcileNodeLivenessEvents } from "../src/watchdog-node-liveness.js"; + +function node(id: string, overrides: Partial = {}): RecorderNode { + return { + agentVersion: "0.1.0", + alias: id, + hostname: `${id}.local`, + id, + interfaces: [], + ipAddresses: ["172.22.145.152"], + lastSeenAt: "2026-06-18T12:00:00.000Z", + location: { room: "Council Chamber", site: "Main Office" }, + status: "online", + tags: [], + ...overrides, + }; +} + +test("R4-2: a failing node reconcile is isolated and later nodes still reconcile", async () => { + const auditStore = createAuditStore(""); + const real = createHealthEventStore("", []); + // The first node's health-event lookup throws; the sweep must skip it and go on. + // Delegate every method to the real (class-based) store and intercept only + // `list` so node_bad's lookup fails. + const healthEventStore: HealthEventStore = { + count: (filters) => real.count(filters), + create: (input) => real.create(input), + find: (eventId) => real.find(eventId), + async list(filters) { + if (filters?.nodeId === "node_bad") { + throw new Error("health store unavailable"); + } + + return real.list(filters); + }, + listAll: (filters) => real.listAll(filters), + update: (eventId, update) => real.update(eventId, update), + updateLifecycle: (eventId, update) => real.updateLifecycle(eventId, update), + }; + + // Both nodes are stale (last seen 12:00, now 12:05) so both would normally + // raise an offline alert. + const results = await reconcileNodeLivenessEvents({ + auditStore, + healthEventStore, + nodes: [node("node_bad"), node("node_good")], + now: new Date("2026-06-18T12:05:00.000Z"), + }); + + const bad = results.find((result) => result.nodeId === "node_bad"); + const good = results.find((result) => result.nodeId === "node_good"); + + // Pre-fix the thrown error propagated out of the loop, aborting the whole + // sweep so node_good was never evaluated (offline nodes stayed unflagged). + assert.equal(bad?.outcome, "skipped"); + assert.equal(bad?.reason, "reconcile_failed"); + assert.equal(good?.outcome, "alert_created"); + + const goodEvents = await healthEventStore.list({ nodeId: "node_good" }); + assert.equal(goodEvents.length, 1); +}); diff --git a/apps/web/src/components/node-inventory-dialogs.tsx b/apps/web/src/components/node-inventory-dialogs.tsx index fec1748..dd3ee06 100644 --- a/apps/web/src/components/node-inventory-dialogs.tsx +++ b/apps/web/src/components/node-inventory-dialogs.tsx @@ -102,10 +102,17 @@ export function EnrollNodeDialog() { const { data: token } = await api.mintNodeBootstrapToken(enrollment.node.id); return { node: enrollment.node, token }; }, - onError: () => + onError: () => { toast.error("Enroll failed", { description: "The recorder node could not be enrolled.", - }), + }); + // Enrollment is two calls (enroll node, then mint bootstrap token). If the + // node was created but the token mint failed, it exists server-side — + // refresh the table so it is visible and the operator does not re-enroll a + // duplicate (audit R6-ENROLL-DUP). + void queryClient.invalidateQueries({ queryKey: ["nodes"] }); + void queryClient.invalidateQueries({ queryKey: ["audit-events"] }); + }, onSuccess: ({ node, token }) => { setEnrolled({ alias: node.alias, 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 0000000..62857dc --- /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 2bc9032..d5302ad 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]; diff --git a/apps/web/src/components/recording-profile-settings-card.tsx b/apps/web/src/components/recording-profile-settings-card.tsx index e752b87..c9feed5 100644 --- a/apps/web/src/components/recording-profile-settings-card.tsx +++ b/apps/web/src/components/recording-profile-settings-card.tsx @@ -9,7 +9,7 @@ import { import { Save, Wand2 } from "lucide-react"; import { toast } from "sonner"; -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"; @@ -92,17 +92,13 @@ export function RecordingProfileSettingsCard({ - - - 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, @@ -119,7 +131,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, @@ -225,6 +244,9 @@ export function ScheduleFormDialog({ Select a recorder + {nodeMissing ? ( + {draft.nodeId} (unavailable) + ) : null} {nodes.map((node) => ( {node.alias} / {node.location.room} @@ -586,6 +608,11 @@ export function ScheduleFormDialog({ Node default + {interfaceMissing ? ( + + {draft.captureInterfaceId} (unavailable) + + ) : null} {selectedNode?.interfaces.map((audioInterface) => ( {audioInterfaceLabel(audioInterface)} @@ -707,19 +734,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/components/settings-fields.tsx b/apps/web/src/components/settings-fields.tsx index 1ba6c8f..954bd34 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/settings-upload-policies-section.tsx b/apps/web/src/components/settings-upload-policies-section.tsx index de63c88..53ec185 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/ui/truncate-cell.tsx b/apps/web/src/components/ui/truncate-cell.tsx index ff79fd1..9a37ffd 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 6ac5d64..7b408f1 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,38 +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; -}) { - return ( - - onChange(Number(event.target.value))} - step={step} - type="number" - value={value} - /> - - ); -} - // 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/dashboard-page-helpers.test.ts b/apps/web/src/lib/dashboard-page-helpers.test.ts index c6be492..e0a558b 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 50feffb..5868e97 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/lib/node-page-helpers.test.ts b/apps/web/src/lib/node-page-helpers.test.ts index 7356a09..72413df 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 02e78a2..e455f62 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 0000000..b547a1f --- /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 9a98329..d352560 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/recording-page-helpers.test.ts b/apps/web/src/lib/recording-page-helpers.test.ts index dcff9f9..0ee9064 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 3f844bd..5ce75d9 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 95ece42..61fd209 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"; @@ -13,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(); @@ -40,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"); @@ -136,6 +211,43 @@ 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("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 5f8e83d..fcea3f4 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, @@ -62,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: [], @@ -89,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, @@ -99,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, + ), }; } @@ -124,7 +172,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, }; @@ -157,7 +209,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, }; } @@ -528,3 +580,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 ` 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 { + const trimmed = raw.trim(); + + if (trimmed === "") { + return undefined; + } + + // 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; +} + export function recordingProfileUpdate(profile: RecordingProfile): RecordingProfileUpdate { return { bitrateKbps: profile.bitrateKbps, @@ -20,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 5b934f2..fbaf3cd 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/lib/use-server-pagination.ts b/apps/web/src/lib/use-server-pagination.ts index fcc813e..4bcef75 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 bd5ee75..e5486d7 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 e9e49e9..9c82e02 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/dashboard.tsx b/apps/web/src/pages/dashboard.tsx index 088a4ce..2d422ee 100644 --- a/apps/web/src/pages/dashboard.tsx +++ b/apps/web/src/pages/dashboard.tsx @@ -26,13 +26,14 @@ import { dashboardActiveHealthEvents, dashboardIncidentActions, dashboardPagePermissions, + dashboardReportingNodes, type DashboardIncidentAction, } from "@/lib/dashboard-page-helpers"; 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"; @@ -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; @@ -188,7 +189,7 @@ export function DashboardPage() {
{node.alias} - {node.status} + {nodeStatusLabel(node.status)}
diff --git a/apps/web/src/pages/health.tsx b/apps/web/src/pages/health.tsx index 6fd2ae1..56bdcf0 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 a0b0e87..1f2e5dc 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 3edbe24..5f70731 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"; @@ -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 }), @@ -357,7 +358,7 @@ function nodeColumns({ onToggleSelected, selectedNodeIds, }: NodeColumnOptions): ColumnDef[] { - return [ + const columns: ColumnDef[] = [ { cell: ({ row }) => ( ( - {row.original.status} + {nodeStatusLabel(row.original.status)} ), header: "Status", @@ -455,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/recordings.tsx b/apps/web/src/pages/recordings.tsx index c0c2b3e..7ca4e2f 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} /> ); diff --git a/apps/web/src/pages/room-detail.tsx b/apps/web/src/pages/room-detail.tsx index 59e7cd9..28c7eb4 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 4348865..12fc667 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; @@ -279,13 +284,33 @@ 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). 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 fd91049..2e94e37 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"; @@ -86,6 +87,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()), @@ -292,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); } @@ -307,7 +327,7 @@ export function SchedulesPage() { function closeDialog() { setDialogOpen(false); setEditingId(undefined); - setDraft(defaultDraft(firstNode, schedulingDefaults)); + setDraft(defaultDraft(firstNode, schedulingDefaults, schedulingAvailability())); } function submitSchedule() { diff --git a/crates/recorder-agent/src/inventory.rs b/crates/recorder-agent/src/inventory.rs index 30b3ac9..a7e7c27 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( 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 0000000..f621380 --- /dev/null +++ b/docs/internal/audits/2026-07-08-gap-hunt.md @@ -0,0 +1,476 @@ +# 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 entire audit) | +| Started | 2026-07-08 | +| Completed | 2026-07-08 | +| Runs | 13 — Runs 1–3 dirty; Runs 4–5 clean; Runs 6–8 dirty (`H4-2W`,`R7-IPCAP`,`R8-RETENTION-SELECT`); **Runs 9–13 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 | ✅ **Converged — 5 consecutive clean runs achieved (Runs 9–13)**, against the static base `619b6f10`. 18 findings fixed (1 Critical, several Medium, rest Low), each with red→green proof or a coverage lock; the remaining open items are all catalogued cosmetic/suspected/pre-existing/by-design (none a confirmed functional bug). | +| Gates at close (Run 13) | green — **full `mise run check` exit 0** (582.8s): tsc, API + web node tests, oxlint, oxfmt, `check:loc`, `db:verify` (Drizzle replay 0001–0046), **all baseline verifiers** (rbac/transport/scheduler/settings/recordings/first-reliable/generic-device/watchdog/storage/switcher/node-lifecycle/oidc + ops alerts/grafana/prometheus/observability + helm render), **`rust:check`+`clippy`+`fmt`+`miri`** (137 pass / 0 fail / 28 `cfg(miri)`-ignored), and **`agent:fake-controller-smoke`**. Separately validated during the loop: `node:test-db` (22 DB-backed concurrency/atomicity tests, green). | + +**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 + +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 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 + +| 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` | + +### 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 | +| --- | --- | --- | --- | --- | +| 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` | +| 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` | +| R8-RETENTION-SELECT | 8 | web/schedule-form | **Run-8 web re-sweep.** The schedule form wrapped its recording-profile + watchdog `` from the raw list. A stale/deleted retention id (a prefilled controller-settings default, or an edited schedule whose policy was removed/renamed, or `settings:read` unavailable) made the controlled Select fall back to its placeholder — reading as "unselected" while the draft silently kept and re-saved the id (profile/watchdog showed the id; retention alone went blank — a misleading same-form asymmetry). Routed retention through the same helper (moved to `schedule-draft.ts` as a shared, now-tested export). Red→green helper test; page wiring mirrors the two tested-intent siblings. | `13c81a79` | +| 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 + +| 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) + +| 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 + +> **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 | +| --- | --- | --- | --- | --- | +| 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 | 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. | +| 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. | +| 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. | +| 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`). | +| 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`. | +| 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 `` 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. +- **Run 9 — clean (streak 1).** Adversary re-verified `R8-RETENTION-SELECT` sound + and complete (sub-checks a–d: the helper move preserved behaviour, no other + policy-select desyncs, present/duplicate/empty selection all correct), and a + full-surface **completeness critic** over all 85 changed files found **no new + real defect** (env parsing, controller-settings failover + `keep` merge, contract + bounds, every one of the 18 fix-interactions, and the docs/baselines all verified + consistent). **Zero code/test changes.** One new **suspected/pre-existing** lead + logged, not fixed: `R9-NODEIFACE-SELECT` — the node/interface selects share `R8`'s + UI-honesty class but are pre-existing (not PR-introduced) and bounded (>200 nodes + or a deleted node; API-validated on save), catalogued as a consistency follow-up. + No confirmed functional bug required a change → **clean, streak 1**. +- **Run 10 — clean (streak 2).** Two fresh angles: a dedicated **security-lens** + review (GitHub/bootstrap/SSH/runner tokens never leaked or persisted; authz + correct + no IDOR on every new route; untrusted GitHub JSON shape-validated with + no injection sink; the `update_binary` supply-chain is checksum-verified + operator- + gated, unreachable by a node credential) and a **production-scenario** edge pass + (mixed-version fleet, deleted-default fallback, long-stuck-provisioning growth, + mid-recording-window promotion, and a broken/deleted upload destination — all + degrade gracefully). Both **clean**. One architectural sharp edge investigated — + the schedule store's `update` has no compare-and-set, so multiple API replicas + could double-fire a schedule — but it is **fenced by the documented single-writer + deployment contract** (`replicaCount:1` + `strategy: Recreate` + the store + comment), unreachable in the shipped deployment, so rejected (not a fresh bug). + **Zero code/test changes; no new leads.** → **clean, streak 2**. +- **Run 11 — clean (streak 3).** A fresh-eyes, line-by-line first-principles + re-read of the four densest new files (worst-input traces of the version compare, + release-cache TTL/dedup, liveness threshold + `firstObservedAt` preservation, and + the `keep` merge round-trip) found **no new defect**; and a **test-durability + critic** re-derived all 18 fixes' tests and confirmed each would fail on a revert + (the two route-shadow reproductions re-run empirically; the `keep→??`, nullable- + destination, `>16`-truncate, and `numericInputCommit`/`withSelectedOption` locks + all verified discriminating). **Zero code/test changes.** One marginal note logged: + `R11-CAP-COV` — R3-4's `.max(32)` cap has no `>32`-rejection test (only the client + dedup is tested), but a cap regression is fully mitigated downstream (destination- + dedup fan-out) so it is not a reportable durability gap. No confirmed bug or + actionable coverage gap → **clean, streak 3**. +- **Run 12 — clean (streak 4).** Two fresh angles: a **diff-as-reviewer** pass + reading the raw PR hunks (`git diff 72a1a495^..619b6f10`) for the classic + "changed X but forgot the dependent Y", and a **React effect/query-timing** pass + over the changed components' effects/queries/mutations. The diff-review confirmed + every changed seam is fully wired — the `provisioning` status, the four + controller-settings default columns, the `uploadPolicyIds` `[]` default, the + enroll-form field removal (payload still satisfies the unchanged `.strict()` + server schema), and the agent-release feature all have their dependent callers/ + siblings updated (the classic missed siblings were the already-caught `N1`/`N2`/ + `G1`). The timing review confirmed the one non-trivial changed effect (the + recording-start-panel ref-guarded one-time upload-default seed) and the schedule- + form/watchdog/settings-section reset effects are all correctly scoped. Session + limits killed the first hunter pair and the timing-hunter rerun, so both angles + were also run inline in the main loop (the diff-reviewer rerun did complete and + independently confirmed clean). **Zero code/test changes; no new leads.** → + **clean, streak 4**. +- **Run 13 — clean; convergence achieved (streak 5).** A last-chance broad + adversarial sweep (all four PR features re-traced end-to-end at the line level) + and an agent↔controller wire-contract re-confirmation (heartbeat/inventory/ + bootstrap + the reverse job/command/config/enhancement/bootstrap-response + direction, field-by-field, both ways) both found **nothing new** — every lead + resolved to a catalogued item or unreachable/by-design behaviour. As the final + gate, the **full `mise run check` passed (exit 0, 582.8s)** — tsc, all node + tests, lint, fmt, LOC, `db:verify` (Drizzle replay 0001–0046), every baseline + verifier, `rust:check`/`clippy`/`fmt`/`miri` (137 pass / 0 fail), the ops/helm + render checks, and `agent:fake-controller-smoke` — and **`mise run build`** + passed. **Zero code/test changes; no new leads.** → **clean, streak 5 → ✅ + converged (Runs 9–13).** + +## Convergence summary + +Point-an-agent-at-the-doc audit of the PR #31/#32/#33 surface converged at **5 +consecutive clean runs (Runs 9–13)** after 13 total runs, against the static base +`619b6f10`. **18 findings fixed** with red→green proof (or a verified coverage +lock), headlined by the Critical `G1` (a Hono route-shadow that left the entire +PR #32 update-available feature silently dead in production), the reachable +availability bugs `N4` (a heartbeat could un-promote a live node and suppress its +offline alert) and `R7-IPCAP` (a multi-homed node's over-cap `ipAddresses` desynced +it permanently), the provisioning-reachability class `N1`/`N2`, the stub-upload +removal completion `H3-1`/`H3-2`/`H3-3`, and the UI-honesty fixes `H4-2W`/`R8`. The +remaining open items are all catalogued **cosmetic / suspected / pre-existing / +by-design** (e.g. `R3-1` offline tone, `R3-3` switcher-password affordance, +`R9-NODEIFACE-SELECT`, `N-3A`/`N-3B` release paging, `W1`–`W4` data-table visual +feedback needing a React render harness) — none a confirmed functional bug. The +loop's value showed most in Runs 6–8, where fresh adversarial angles (a catalogue +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 `