From 534712e50fd5f9fd72aca1e9b1d1e61fb2fda8fd Mon Sep 17 00:00:00 2001 From: arkon Date: Sun, 14 Jun 2026 07:33:52 +0200 Subject: [PATCH] fix(usage): address code-review findings in plan-usage telemetry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the plan-usage chip feature (commits since 1.0.0) surfaced several issues; this fixes all confirmed findings: - HIGH: applyStatusLineConfig clobbered a user's hand-authored statusLine on the enable path (the isOurs guard only protected disable). Now bails out when an existing statusLine isn't ours, on both the enable and disable paths. - MED: StatusTelemetrySchema used z.optional() (rejects null) on Claude's undocumented statusline fields — a single stray null 400'd the entire POST and silently killed the chip's data feed. Switched the modeled fields to .nullish(). - MED: dropping the Token Count / Show Cost header toggles left their features reading settings.showTokenCount/showCost, but saveAppSettings rebuilds settings fresh from the DOM, dropping those keys and resetting them to defaults on every save (re-enabling the token chip with no UI to turn it off). Preserve the prior stored preference. - telemetrySignature keyed on contextUsedPercentage (never displayed) and the raw unrounded %, churning a redundant SSE broadcast + localStorage write + identical chip re-render on every assistant message. Now keys on the rounded displayed window values only. - Plan-usage chip flashed hidden on load (no server-side reveal): renderIndexHtml now strips header-plan-usage--hidden when enabled, matching btn-multimonitor; fixes the FOUC and makes the "server renders initial state" comments accurate. - Serialize all settings.local.json read-modify-write writers in hooks-config via a shared per-path mutex (previously lock-free; concurrent session-create + settings-toggle on the same repo could lose writes). - Hardened the chip's innerHTML against any future string field; removed the dead _latestPlanUsage field; clamped ctx% in the footer formatter; corrected the session-create comment (the path is add-only by design — a per-repo settings file is shared by sibling sessions). - Tests: new test/routes/status-telemetry-routes.test.ts (route behavior, dedup, null-tolerance) + NaN/Infinity/fractional and signature-churn unit tests; made server-index-title.test.ts deterministic against the ambient settings.json. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/hooks-config.ts | 217 +++++++++++--------- src/usage-telemetry.ts | 14 +- src/web/public/app.js | 16 +- src/web/public/session-ui.js | 8 +- src/web/public/settings-ui.js | 13 +- src/web/schemas.ts | 30 ++- src/web/server.ts | 8 + test/routes/status-telemetry-routes.test.ts | 102 +++++++++ test/server-index-title.test.ts | 21 +- test/usage-telemetry.test.ts | 41 ++++ 10 files changed, 352 insertions(+), 118 deletions(-) create mode 100644 test/routes/status-telemetry-routes.test.ts diff --git a/src/hooks-config.ts b/src/hooks-config.ts index d0a7d026..4b7adfc3 100644 --- a/src/hooks-config.ts +++ b/src/hooks-config.ts @@ -31,6 +31,30 @@ import { join } from 'node:path'; import type { HookEventType } from './types.js'; import { HOOK_TIMEOUT_MS } from './config/auth-config.js'; +/** + * Serializes read-modify-write access to a `settings.local.json` path. Every + * writer in this module (hooks, env, model, statusLine) shares this map, so + * concurrent updates to the SAME file — e.g. session-create writing hooks/model + * while an App-Settings toggle injects the statusLine into the same repo — can't + * lose each other's changes through interleaved read-then-write. Per-path chains + * are independent; the map self-prunes when a path's chain goes idle. + */ +const settingsWriteLocks = new Map>(); +function withSettingsLock(path: string, fn: () => Promise): Promise { + const prev = settingsWriteLocks.get(path) ?? Promise.resolve(); + const run = prev.then(fn, fn); // run after the prior writer, regardless of its outcome + // Tail never rejects, so a failed write doesn't poison subsequent writers. + const tail = run.then( + () => {}, + () => {} + ); + settingsWriteLocks.set(path, tail); + void tail.then(() => { + if (settingsWriteLocks.get(path) === tail) settingsWriteLocks.delete(path); + }); + return run; +} + /** * Generates the hooks section for .claude/settings.local.json * @@ -103,29 +127,31 @@ export async function stripCaseEnvKeys(casePath: string, keysToRemove: readonly if (keysToRemove.length === 0) return; const settingsPath = join(casePath, '.claude', 'settings.local.json'); - if (!existsSync(settingsPath)) return; + await withSettingsLock(settingsPath, async () => { + if (!existsSync(settingsPath)) return; - let existing: Record; - try { - existing = JSON.parse(await readFile(settingsPath, 'utf-8')); - } catch { - return; // Malformed — don't rewrite it - } - - const env = existing.env as Record | undefined; - if (!env) return; - - let changed = false; - for (const key of keysToRemove) { - if (key in env) { - delete env[key]; - changed = true; + let existing: Record; + try { + existing = JSON.parse(await readFile(settingsPath, 'utf-8')); + } catch { + return; // Malformed — don't rewrite it } - } - if (!changed) return; - existing.env = env; - await writeFile(settingsPath, JSON.stringify(existing, null, 2) + '\n'); + const env = existing.env as Record | undefined; + if (!env) return; + + let changed = false; + for (const key of keysToRemove) { + if (key in env) { + delete env[key]; + changed = true; + } + } + if (!changed) return; + + existing.env = env; + await writeFile(settingsPath, JSON.stringify(existing, null, 2) + '\n'); + }); } /** @@ -134,30 +160,31 @@ export async function stripCaseEnvKeys(casePath: string, keysToRemove: readonly */ export async function updateCaseEnvVars(casePath: string, envVars: Record): Promise { const claudeDir = join(casePath, '.claude'); - if (!existsSync(claudeDir)) { - await mkdir(claudeDir, { recursive: true }); - } - const settingsPath = join(claudeDir, 'settings.local.json'); - let existing: Record = {}; - - try { - existing = JSON.parse(await readFile(settingsPath, 'utf-8')); - } catch { - existing = {}; - } - - const currentEnv = (existing.env as Record) || {}; - for (const [key, value] of Object.entries(envVars)) { - if (value) { - currentEnv[key] = value; - } else { - delete currentEnv[key]; + await withSettingsLock(settingsPath, async () => { + if (!existsSync(claudeDir)) { + await mkdir(claudeDir, { recursive: true }); } - } - existing.env = currentEnv; - await writeFile(settingsPath, JSON.stringify(existing, null, 2) + '\n'); + let existing: Record = {}; + try { + existing = JSON.parse(await readFile(settingsPath, 'utf-8')); + } catch { + existing = {}; + } + + const currentEnv = (existing.env as Record) || {}; + for (const [key, value] of Object.entries(envVars)) { + if (value) { + currentEnv[key] = value; + } else { + delete currentEnv[key]; + } + } + existing.env = currentEnv; + + await writeFile(settingsPath, JSON.stringify(existing, null, 2) + '\n'); + }); } /** @@ -166,26 +193,27 @@ export async function updateCaseEnvVars(casePath: string, envVars: Record { const claudeDir = join(casePath, '.claude'); - if (!existsSync(claudeDir)) { - await mkdir(claudeDir, { recursive: true }); - } - const settingsPath = join(claudeDir, 'settings.local.json'); - let existing: Record = {}; + await withSettingsLock(settingsPath, async () => { + if (!existsSync(claudeDir)) { + await mkdir(claudeDir, { recursive: true }); + } - try { - existing = JSON.parse(await readFile(settingsPath, 'utf-8')); - } catch { - existing = {}; - } + let existing: Record = {}; + try { + existing = JSON.parse(await readFile(settingsPath, 'utf-8')); + } catch { + existing = {}; + } - if (model) { - existing.model = model; - } else { - delete existing.model; - } + if (model) { + existing.model = model; + } else { + delete existing.model; + } - await writeFile(settingsPath, JSON.stringify(existing, null, 2) + '\n'); + await writeFile(settingsPath, JSON.stringify(existing, null, 2) + '\n'); + }); } /** @@ -194,24 +222,25 @@ export async function updateCaseModel(casePath: string, model: string | null): P */ export async function writeHooksConfig(casePath: string): Promise { const claudeDir = join(casePath, '.claude'); - if (!existsSync(claudeDir)) { - await mkdir(claudeDir, { recursive: true }); - } - const settingsPath = join(claudeDir, 'settings.local.json'); - let existing: Record = {}; + await withSettingsLock(settingsPath, async () => { + if (!existsSync(claudeDir)) { + await mkdir(claudeDir, { recursive: true }); + } - try { - existing = JSON.parse(await readFile(settingsPath, 'utf-8')); - } catch { - // If file is malformed or doesn't exist, start fresh - existing = {}; - } + let existing: Record = {}; + try { + existing = JSON.parse(await readFile(settingsPath, 'utf-8')); + } catch { + // If file is malformed or doesn't exist, start fresh + existing = {}; + } - const hooksConfig = generateHooksConfig(); - const merged = { ...existing, ...hooksConfig }; + const hooksConfig = generateHooksConfig(); + const merged = { ...existing, ...hooksConfig }; - await writeFile(settingsPath, JSON.stringify(merged, null, 2) + '\n'); + await writeFile(settingsPath, JSON.stringify(merged, null, 2) + '\n'); + }); } /** Unique marker identifying Codeman's own statusLine command (vs a user's). */ @@ -243,34 +272,38 @@ export function generateStatusLineCommand(): string { * Add or remove Codeman's plan-usage statusLine exporter in * `.claude/settings.local.json`. Only ever touches a statusLine that is OURS * (command targets `/api/status-telemetry`), so a user's hand-authored - * statusLine is never removed. Callers gate on Claude mode + a Codeman-managed - * case path. Merges, preserving all other keys (hooks, env, model). + * statusLine is never removed OR overwritten — on both the enable and disable + * paths we bail out when an existing statusLine isn't ours. Callers gate on + * Claude mode. Merges, preserving all other keys (hooks, env, model). */ export async function applyStatusLineConfig(casePath: string, enabled: boolean): Promise { const claudeDir = join(casePath, '.claude'); const settingsPath = join(claudeDir, 'settings.local.json'); - let existing: Record = {}; - if (existsSync(settingsPath)) { - try { - existing = JSON.parse(await readFile(settingsPath, 'utf-8')); - } catch { - return; // Malformed — don't rewrite it + await withSettingsLock(settingsPath, async () => { + let existing: Record = {}; + if (existsSync(settingsPath)) { + try { + existing = JSON.parse(await readFile(settingsPath, 'utf-8')); + } catch { + return; // Malformed — don't rewrite it + } } - } - const current = existing.statusLine as { command?: unknown } | undefined; - const isOurs = !!current && typeof current.command === 'string' && current.command.includes(STATUSLINE_MARKER); + const current = existing.statusLine as { command?: unknown } | undefined; + const isOurs = !!current && typeof current.command === 'string' && current.command.includes(STATUSLINE_MARKER); - if (enabled) { - const desired = generateStatusLineCommand(); - if (isOurs && current?.command === desired) return; // already current — skip rewrite - if (!existsSync(claudeDir)) await mkdir(claudeDir, { recursive: true }); - existing.statusLine = { type: 'command', command: desired }; // add, or update an out-of-date ours - } else { - if (!isOurs) return; // nothing of ours to remove (leave a user's own statusLine alone) - delete existing.statusLine; - } + if (enabled) { + const desired = generateStatusLineCommand(); + if (isOurs && current?.command === desired) return; // already current — skip rewrite + if (current && !isOurs) return; // user has their OWN statusLine — never clobber it + if (!existsSync(claudeDir)) await mkdir(claudeDir, { recursive: true }); + existing.statusLine = { type: 'command', command: desired }; // add, or update an out-of-date ours + } else { + if (!isOurs) return; // nothing of ours to remove (leave a user's own statusLine alone) + delete existing.statusLine; + } - await writeFile(settingsPath, JSON.stringify(existing, null, 2) + '\n'); + await writeFile(settingsPath, JSON.stringify(existing, null, 2) + '\n'); + }); } diff --git a/src/usage-telemetry.ts b/src/usage-telemetry.ts index 88a013dd..b549147b 100644 --- a/src/usage-telemetry.ts +++ b/src/usage-telemetry.ts @@ -141,20 +141,26 @@ export function formatSessionStatusText(s: SessionStatus | null): string { if (s.inputTokens != null) tok.push(`in:${withCommas(s.inputTokens)}`); if (s.outputTokens != null) tok.push(`out:${withCommas(s.outputTokens)}`); if (tok.length) groups.push(tok.join(' ')); - if (s.contextUsedPercentage != null) groups.push(`ctx:${Math.round(s.contextUsedPercentage)}%`); + if (s.contextUsedPercentage != null) groups.push(`ctx:${Math.round(clampPct(s.contextUsedPercentage))}%`); return groups.length ? groups.join(' ') : 'codeman'; } /** * Stable signature for change-detection — the statusline fires on every * assistant message, so the route only rebroadcasts when this value changes. + * + * Keys on EXACTLY the values the header chip displays: the two windows' ROUNDED + * percentages (the chip renders `Math.round`) + their reset times. Deliberately + * excludes contextUsedPercentage / costUsd / modelDisplayName — none are shown + * in the chip, and contextUsedPercentage in particular drifts on every assistant + * message, which would defeat the dedup and fan out a redundant SSE broadcast + + * localStorage write + identical chip re-render each time. */ export function telemetrySignature(t: StatusTelemetry): string { return JSON.stringify([ - t.fiveHour?.usedPercentage ?? null, + t.fiveHour ? Math.round(t.fiveHour.usedPercentage) : null, t.fiveHour?.resetAt ?? null, - t.sevenDay?.usedPercentage ?? null, + t.sevenDay ? Math.round(t.sevenDay.usedPercentage) : null, t.sevenDay?.resetAt ?? null, - t.contextUsedPercentage ?? null, ]); } diff --git a/src/web/public/app.js b/src/web/public/app.js index 904111a7..1fa1a538 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -1817,7 +1817,6 @@ class CodemanApp { // Claude plan usage limits (5-hour + weekly) — account-global, so the latest // sample from any session drives the shared header chip. _onSessionStatusTelemetry(data) { - this._latestPlanUsage = data; this.updatePlanUsageChip(data); // Persist last-known so the chip shows immediately on the next page load / // SSE reconnect, instead of staying blank until a session next renders. @@ -1848,10 +1847,17 @@ class CodemanApp { if (five === null && seven === null) return; // Per-window color by how much is used up: green < 60%, yellow 60–84%, red ≥ 85%. const colorClass = (p) => (p >= 85 ? 'pu-red' : p >= 60 ? 'pu-yellow' : 'pu-green'); - const seg = (label, p) => - p === null - ? '' - : `${label}${p}%`; + // innerHTML here is XSS-safe ONLY because every interpolated value is a + // coerced finite number and the labels/classes are fixed literals. If a + // string field (e.g. modelDisplayName, which the route also broadcasts) is + // ever shown in this chip, render it via textContent — never interpolate an + // untrusted string into this template. + const seg = (label, p) => { + if (p === null) return ''; + const n = Math.round(Number(p)); + if (!Number.isFinite(n)) return ''; + return `${label}${n}%`; + }; chip.innerHTML = [seg('5h', five), seg('7d', seven)].filter(Boolean).join('·'); const resetStr = (w) => (w && w.resetAt ? new Date(w.resetAt).toLocaleString() : '—'); chip.title = diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index d3cfae0c..1602948d 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -370,8 +370,12 @@ Object.assign(CodemanApp.prototype, { ...(hasEnvOverrides ? { envOverrides } : {}), ...(effort ? { effort } : {}), ...(modelOverride !== undefined ? { modelOverride } : {}), - // Plan-usage statusLine exporter (App Settings → Display). Always - // sent so toggling the setting off removes our exporter on next create. + // Plan-usage statusLine exporter (App Settings → Display). The server + // ADDS our exporter on create when true; when false it intentionally + // leaves any existing exporter in place (a per-repo settings.local.json + // is shared by sibling sessions, so create-with-false must not yank it + // — see the comment in session-routes create). Disabling the setting + // removes it via the App Settings toggle path (system-routes), not here. statusLineTelemetry: globalSettings.showPlanUsageLimits === true, }) }).then(r => r.json()) diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 784646d2..8800ddaf 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -1351,7 +1351,8 @@ Object.assign(CodemanApp.prototype, { async saveAppSettings() { // Gesture overlay is injected at page render (server-side), so a change to it // only takes effect on reload — remember the prior value to decide below. - const _prevGestureEnabled = (this.loadAppSettingsFromStorage().gestureControlEnabled ?? false) === true; + const _prev = this.loadAppSettingsFromStorage(); + const _prevGestureEnabled = (_prev.gestureControlEnabled ?? false) === true; const settings = { defaultClaudeMdPath: document.getElementById('appSettingsClaudeMdPath').value.trim(), defaultWorkingDir: document.getElementById('appSettingsDefaultDir').value.trim(), @@ -1394,6 +1395,16 @@ Object.assign(CodemanApp.prototype, { }, }; + // The "Token Count" / "Show Cost ($)" header toggles were removed from the + // UI, but their features still read settings.showTokenCount / settings.showCost + // (applyHeaderVisibilitySettings, the header cost render). saveAppSettings + // rebuilds `settings` fresh from the DOM (a full replacement, not a merge), so + // without this these keys would be DROPPED on every save and fall back to their + // defaults — silently re-enabling the token chip for anyone who'd turned it off, + // with no UI left to turn it back off. Preserve the prior stored preference. + if (_prev.showTokenCount !== undefined) settings.showTokenCount = _prev.showTokenCount; + if (_prev.showCost !== undefined) settings.showCost = _prev.showCost; + // Save to localStorage this.saveAppSettingsToStorage(settings); this._updateLocalEchoState(); diff --git a/src/web/schemas.ts b/src/web/schemas.ts index fb4ef27a..1c54ed96 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -195,12 +195,20 @@ export const ResizeSchema = z.object({ * subset Codeman displays; unknown keys (session_id, transcript_path, cwd, …) * are stripped by z.object. Auth-exempt like /api/hook-event. */ +// NOTE: every modeled field is `.nullish()` (not `.optional()`) on purpose. +// Claude's statusline blob is officially shipped but undocumented in exact +// shape, and `z.optional()` REJECTS an explicit `null` (accepts only +// `undefined`) — a single stray `null` (e.g. `cost:{total_cost_usd:null}`) +// would 400 the ENTIRE POST before the deliberately-tolerant parser +// (usage-telemetry.ts, which only acts on `typeof === 'number'/'string'`) ever +// runs, silently killing the chip's data feed. `.nullish()` keeps the schema +// gate as forgiving as the parser it guards. const RateLimitWindowSchema = z .object({ - used_percentage: z.number().optional(), - resets_at: z.number().optional(), + used_percentage: z.number().nullish(), + resets_at: z.number().nullish(), }) - .optional(); + .nullish(); export const StatusTelemetrySchema = z.object({ sessionId: z.string().min(1).max(100), @@ -211,18 +219,18 @@ export const StatusTelemetrySchema = z.object({ five_hour: RateLimitWindowSchema, seven_day: RateLimitWindowSchema, }) - .optional(), + .nullish(), context_window: z .object({ - used_percentage: z.number().optional(), - total_input_tokens: z.number().optional(), - total_output_tokens: z.number().optional(), + used_percentage: z.number().nullish(), + total_input_tokens: z.number().nullish(), + total_output_tokens: z.number().nullish(), }) - .optional(), - cost: z.object({ total_cost_usd: z.number().optional() }).optional(), - model: z.object({ display_name: z.string().max(100).optional() }).optional(), + .nullish(), + cost: z.object({ total_cost_usd: z.number().nullish() }).nullish(), + model: z.object({ display_name: z.string().max(100).nullish() }).nullish(), }) - .optional(), + .nullish(), }); // ========== Case Routes ========== diff --git a/src/web/server.ts b/src/web/server.ts index d20ad147..ad45fa98 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -1124,6 +1124,14 @@ export class WebServer extends EventEmitter { if (settings.showMultiMonitorButton === true) { html = html.replace(' btn-multimonitor--hidden', ''); } + // Plan-usage chip: same pattern. Ships with `header-plan-usage--hidden` (App + // Settings → Display → "Plan Usage Limits", default off); strip it server-side + // when enabled so the chip doesn't flash hidden on every reload before the + // client's applyHeaderVisibilitySettings runs. showPlanUsageLimits is a synced + // (non-display) setting, so readSettings sees the persisted value here. + if (settings.showPlanUsageLimits === true) { + html = html.replace(' header-plan-usage--hidden', ''); + } // Detached single-session ("solo") window: inject the target session id so // the client can enter solo mode even if a (network-first) service worker // later serves a cached shell. The client primarily detects solo mode from diff --git a/test/routes/status-telemetry-routes.test.ts b/test/routes/status-telemetry-routes.test.ts new file mode 100644 index 00000000..c6d15cb5 --- /dev/null +++ b/test/routes/status-telemetry-routes.test.ts @@ -0,0 +1,102 @@ +/** + * Route tests for POST /api/status-telemetry — the Claude statusline exporter + * endpoint that feeds the header "Plan Usage Limits" chip. + * + * Covers: broadcast on real telemetry + footer print-through, unknown-session + * skip, per-session change-detection (dedup), rebroadcast on a displayed change, + * NO rebroadcast on context-only drift, null-tolerance of Claude's undocumented + * fields (the .nullish() schema — the project's recurring .optional()/null trap), + * and 400 on a malformed body. + */ +import { describe, it, expect, beforeEach } from 'vitest'; +import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js'; +import { registerStatusTelemetryRoutes } from '../../src/web/routes/status-telemetry-routes.js'; +import { SessionStatusTelemetry } from '../../src/web/sse-events.js'; + +const SID = 'test-session-1'; // default id created by createMockRouteContext + +const REAL = { + rate_limits: { + five_hour: { used_percentage: 15, resets_at: 1781409000 }, + seven_day: { used_percentage: 34, resets_at: 1781827200 }, + }, + context_window: { used_percentage: 56, total_input_tokens: 562411, total_output_tokens: 1188 }, + cost: { total_cost_usd: 0.0415 }, + model: { display_name: 'Opus 4.8 (1M context)' }, +}; + +describe('POST /api/status-telemetry', () => { + let h: RouteTestHarness; + + beforeEach(async () => { + h = await createRouteTestHarness(registerStatusTelemetryRoutes, { sessionId: SID }); + }); + + const post = (body: unknown) => h.app.inject({ method: 'POST', url: '/api/status-telemetry', payload: body }); + + it('broadcasts plan-usage telemetry and returns the session-status footer', async () => { + const res = await post({ sessionId: SID, data: REAL }); + expect(res.statusCode).toBe(200); + expect(res.headers['content-type']).toContain('text/plain'); + expect(res.body).toBe('Opus 4.8 (1M context) in:562,411 out:1,188 ctx:56%'); + expect(h.ctx.broadcast).toHaveBeenCalledTimes(1); + const [event, payload] = h.ctx.broadcast.mock.calls[0]; + expect(event).toBe(SessionStatusTelemetry); + expect(payload).toMatchObject({ + sessionId: SID, + fiveHour: { usedPercentage: 15 }, + sevenDay: { usedPercentage: 34 }, + }); + }); + + it('does not broadcast for an unknown session; returns the brand footer', async () => { + const res = await post({ sessionId: 'does-not-exist', data: REAL }); + expect(res.statusCode).toBe(200); + expect(res.body).toBe('codeman'); + expect(h.ctx.broadcast).not.toHaveBeenCalled(); + }); + + it('dedups identical telemetry — rebroadcasts only once', async () => { + await post({ sessionId: SID, data: REAL }); + await post({ sessionId: SID, data: REAL }); + expect(h.ctx.broadcast).toHaveBeenCalledTimes(1); + }); + + it('rebroadcasts when a displayed window percentage changes', async () => { + await post({ sessionId: SID, data: REAL }); + const moved = { + ...REAL, + rate_limits: { ...REAL.rate_limits, five_hour: { used_percentage: 16, resets_at: 1781409000 } }, + }; + await post({ sessionId: SID, data: moved }); + expect(h.ctx.broadcast).toHaveBeenCalledTimes(2); + }); + + it('does NOT rebroadcast on context-only drift (the chip never shows context %)', async () => { + await post({ sessionId: SID, data: REAL }); + await post({ sessionId: SID, data: { ...REAL, context_window: { ...REAL.context_window, used_percentage: 91 } } }); + expect(h.ctx.broadcast).toHaveBeenCalledTimes(1); + }); + + it("tolerates null in Claude's undocumented fields (no 400) and ignores them", async () => { + const res = await post({ + sessionId: SID, + data: { + rate_limits: { five_hour: { used_percentage: 20, resets_at: 1781409000 }, seven_day: null }, + cost: { total_cost_usd: null }, + model: { display_name: null }, + context_window: { used_percentage: null, total_input_tokens: null, total_output_tokens: null }, + }, + }); + expect(res.statusCode).toBe(200); + expect(h.ctx.broadcast).toHaveBeenCalledTimes(1); + const [, payload] = h.ctx.broadcast.mock.calls[0]; + expect(payload).toMatchObject({ fiveHour: { usedPercentage: 20 } }); + expect(payload.sevenDay).toBeUndefined(); + }); + + it('rejects a malformed body (missing sessionId) with 400', async () => { + const res = await post({ data: REAL }); + expect(res.statusCode).toBe(400); + }); +}); diff --git a/test/server-index-title.test.ts b/test/server-index-title.test.ts index 76cd4f02..5318056a 100644 --- a/test/server-index-title.test.ts +++ b/test/server-index-title.test.ts @@ -21,11 +21,11 @@ * Port: N/A (no server start) */ -import { describe, it, expect } from 'vitest'; -import { readFileSync } from 'node:fs'; +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { readFileSync, mkdtempSync } from 'node:fs'; import { join, dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { hostname as osHostname } from 'node:os'; +import { hostname as osHostname, tmpdir } from 'node:os'; import { WebServer } from '../src/web/server.js'; const __dirname = dirname(fileURLToPath(import.meta.url)); @@ -40,6 +40,21 @@ async function render(host?: string): Promise { } describe('WebServer index.html templating (#82)', () => { + // renderIndexHtml reads the ambient settings.json (for the gesture bundle and + // the header-toggle marker-class strips, e.g. showPlanUsageLimits / + // showMultiMonitorButton). Point it at an empty data dir so this test is + // deterministic regardless of the developer's real settings — otherwise an + // enabled toggle would strip a marker class and break the byte-identical + // assertion below. getDataDir() reads CODEMAN_DATA_DIR fresh per call. + const _prevDataDir = process.env.CODEMAN_DATA_DIR; + beforeAll(() => { + process.env.CODEMAN_DATA_DIR = mkdtempSync(join(tmpdir(), 'codeman-title-test-')); + }); + afterAll(() => { + if (_prevDataDir === undefined) delete process.env.CODEMAN_DATA_DIR; + else process.env.CODEMAN_DATA_DIR = _prevDataDir; + }); + it('substitutes the bare <title>Codeman with codeman:', async () => { const html = await render('laptop'); expect(html).toContain('codeman:laptop'); diff --git a/test/usage-telemetry.test.ts b/test/usage-telemetry.test.ts index 185154a4..ac018f2c 100644 --- a/test/usage-telemetry.test.ts +++ b/test/usage-telemetry.test.ts @@ -68,6 +68,24 @@ describe('parseStatusTelemetry', () => { it('ignores a zero/negative reset timestamp', () => { expect(parseStatusTelemetry({ rate_limits: { five_hour: { used_percentage: 10, resets_at: 0 } } })).toBeNull(); }); + + it('keeps a NaN percentage as 0 and drops a window with a non-finite reset', () => { + const t = parseStatusTelemetry({ + rate_limits: { + five_hour: { used_percentage: NaN, resets_at: 1781409000 }, + seven_day: { used_percentage: 40, resets_at: Infinity }, + }, + }); + expect(t!.fiveHour).toEqual({ usedPercentage: 0, resetAt: 1781409000 * 1000 }); + expect(t!.sevenDay).toBeUndefined(); + }); + + it('rounds a fractional resets_at to whole milliseconds', () => { + const t = parseStatusTelemetry({ + rate_limits: { five_hour: { used_percentage: 10, resets_at: 1781409000.7 } }, + }); + expect(t!.fiveHour?.resetAt).toBe(Math.round(1781409000.7 * 1000)); + }); }); describe('parseSessionStatus', () => { @@ -118,4 +136,27 @@ describe('telemetrySignature', () => { })!; expect(telemetrySignature(moved)).not.toBe(telemetrySignature(a)); }); + + it('ignores contextUsedPercentage (not displayed) so it does not churn each message', () => { + const base = { rate_limits: { five_hour: { used_percentage: 15, resets_at: 1781409000 } } }; + const a = parseStatusTelemetry({ ...base, context_window: { used_percentage: 56 } })!; + const b = parseStatusTelemetry({ ...base, context_window: { used_percentage: 91 } })!; + expect(telemetrySignature(a)).toBe(telemetrySignature(b)); + }); + + it('keys on the ROUNDED window percentage (matches the chip) — sub-integer drift is ignored', () => { + const sig = (p: number) => + telemetrySignature( + parseStatusTelemetry({ rate_limits: { five_hour: { used_percentage: p, resets_at: 1781409000 } } })! + ); + expect(sig(15.1)).toBe(sig(15.4)); // both render as 15% + expect(sig(15.1)).not.toBe(sig(15.6)); // 15% vs 16% + }); + + it('excludes cost/model (not shown in the chip) from the signature', () => { + const base = { rate_limits: { five_hour: { used_percentage: 15, resets_at: 1781409000 } } }; + const a = parseStatusTelemetry({ ...base, cost: { total_cost_usd: 0.01 }, model: { display_name: 'A' } })!; + const b = parseStatusTelemetry({ ...base, cost: { total_cost_usd: 9.99 }, model: { display_name: 'B' } })!; + expect(telemetrySignature(a)).toBe(telemetrySignature(b)); + }); });