From cc9117f7296367fe565bfc81bc9019e1e5f25ffd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=90=B4=E6=9D=A8=E5=B8=86?= <39647285+leno23@users.noreply.github.com> Date: Sun, 17 May 2026 07:11:46 +0800 Subject: [PATCH] fix: validate limit edge cases and voicecall numeric flags (#82679) Fix diagnostics/session usage limit handling and voice-call numeric CLI validation. - Treat explicit zero, negative, and non-finite diagnostics/session limits as empty results instead of falling back to defaults. - Reject invalid, non-finite, and fractional voice-call numeric flags. - Add focused tests and a live repro proof for the canonical edge cases. Fixes #82646, #82650, #82651, #82653. Co-authored-by: wuyangfan <1102042793@qq.com> --- CHANGELOG.md | 1 + extensions/voice-call/index.test.ts | 43 +++++++ extensions/voice-call/src/cli.ts | 26 ++++- scripts/repro/limit-edge-case-live-proof.mjs | 116 +++++++++++++++++++ src/infra/session-cost-usage.test.ts | 93 +++++++++++++++ src/infra/session-cost-usage.ts | 10 ++ src/logging/diagnostic-phase.test.ts | 65 +++++++++++ src/logging/diagnostic-phase.ts | 13 ++- 8 files changed, 362 insertions(+), 5 deletions(-) create mode 100644 scripts/repro/limit-edge-case-live-proof.mjs create mode 100644 src/logging/diagnostic-phase.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4df5b28325c..83bbf48b392 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ Docs: https://docs.openclaw.ai - Agents/session_status: resolve implicit no-arg status lookups against the live run session, so `/think` changes report the current thinking level instead of stale sandbox state. Fixes #82669. (#82696) Thanks @leno23. - Discord: keep progress drafts visible for message-tool-only guild replies under the default coding tool profile. Fixes #82747. Thanks @eliranwong. - CLI/setup: order the model/auth provider picker as OpenAI, Anthropic, xAI, Google, then the remaining providers alphabetically. +- Diagnostics/usage/voice-call: treat explicit zero and non-finite limits as empty results and reject invalid voice-call numeric CLI flags. Fixes #82646, #82650, #82651, and #82653. (#82679) Thanks @leno23. - Gateway/diagnostics: add opt-in critical memory pressure stability snapshots with gateway logs, V8 heap, cgroup, active-resource, and redacted large session-file evidence. Fixes #82518. - Doctor/Gateway: avoid treating unrelated macOS LaunchAgents as legacy gateways just because their environment values mention old checkout paths. - Gateway/heartbeat: defer heartbeat runs while the target reply operation is queued or active, preventing heartbeat prompts from interleaving with WebChat responses before the streaming lane starts. Fixes #82722. Thanks @Andy-Xie-1145. diff --git a/extensions/voice-call/index.test.ts b/extensions/voice-call/index.test.ts index 304e9384e15..fe25a2d04bb 100644 --- a/extensions/voice-call/index.test.ts +++ b/extensions/voice-call/index.test.ts @@ -654,6 +654,49 @@ describe("voice-call plugin", () => { expect(String(result.details.error)).toContain("sid required"); }); + it("CLI rejects invalid numeric options", async () => { + const program = new Command(); + await registerVoiceCallCli(program); + + await expect( + program.parseAsync(["voicecall", "expose", "--port", "nope", "--mode", "off"], { + from: "user", + }), + ).rejects.toThrow(/Invalid numeric value for --port/); + await expect( + program.parseAsync(["voicecall", "expose", "--port", "Infinity", "--mode", "off"], { + from: "user", + }), + ).rejects.toThrow(/Invalid numeric value for --port/); + await expect( + program.parseAsync(["voicecall", "expose", "--port", "3334.9", "--mode", "off"], { + from: "user", + }), + ).rejects.toThrow(/Invalid numeric value for --port/); + + const tmpFile = path.join(os.tmpdir(), `voicecall-invalid-${Date.now()}.jsonl`); + fs.writeFileSync(tmpFile, "{}\n", "utf8"); + try { + await expect( + program.parseAsync(["voicecall", "latency", "--file", tmpFile, "--last", "later"], { + from: "user", + }), + ).rejects.toThrow(/Invalid numeric value for --last/); + await expect( + program.parseAsync(["voicecall", "latency", "--file", tmpFile, "--last", "Infinity"], { + from: "user", + }), + ).rejects.toThrow(/Invalid numeric value for --last/); + await expect( + program.parseAsync(["voicecall", "latency", "--file", tmpFile, "--last", "1.5"], { + from: "user", + }), + ).rejects.toThrow(/Invalid numeric value for --last/); + } finally { + fs.unlinkSync(tmpFile); + } + }); + it("CLI latency summarizes turn metrics from JSONL", async () => { const program = new Command(); const tmpFile = path.join(os.tmpdir(), `voicecall-latency-${Date.now()}.jsonl`); diff --git a/extensions/voice-call/src/cli.ts b/extensions/voice-call/src/cli.ts index 0dcf2b3a45d..f3562df9da3 100644 --- a/extensions/voice-call/src/cli.ts +++ b/extensions/voice-call/src/cli.ts @@ -61,6 +61,7 @@ export const __testing = { voiceCallCliDeps.callGatewayFromCli = next ?? callGatewayFromCli; }, isGatewayUnavailableForLocalFallback, + parseVoiceCallIntOption, }; function writeStdoutLine(...values: unknown[]): void { @@ -71,6 +72,19 @@ function writeStdoutJson(value: unknown): void { process.stdout.write(`${JSON.stringify(value, null, 2)}\n`); } +function parseVoiceCallIntOption( + raw: string | undefined, + optionName: string, + opts?: { min?: number }, +): number { + const min = opts?.min ?? 0; + const parsed = Number(raw); + if (!Number.isInteger(parsed) || parsed < min) { + throw new Error(`Invalid numeric value for ${optionName}: ${raw ?? ""}`); + } + return parsed; +} + function isRecord(value: unknown): value is Record { return Boolean(value && typeof value === "object" && !Array.isArray(value)); } @@ -708,8 +722,8 @@ export function registerVoiceCallCli(params: { .option("--poll ", "Poll interval in ms", "250") .action(async (options: { file: string; since?: string; poll?: string }) => { const file = options.file; - const since = Math.max(0, Number(options.since ?? 0)); - const pollMs = Math.max(50, Number(options.poll ?? 250)); + const since = parseVoiceCallIntOption(options.since, "--since", { min: 0 }); + const pollMs = parseVoiceCallIntOption(options.poll, "--poll", { min: 50 }); if (!fs.existsSync(file)) { logger.error(`No log file at ${file}`); @@ -758,7 +772,7 @@ export function registerVoiceCallCli(params: { .option("--last ", "Analyze last N records", "200") .action(async (options: { file: string; last?: string }) => { const file = options.file; - const last = Math.max(1, Number(options.last ?? 200)); + const last = parseVoiceCallIntOption(options.last, "--last", { min: 1 }); if (!fs.existsSync(file)) { throw new Error("No log file at " + file); @@ -805,7 +819,11 @@ export function registerVoiceCallCli(params: { .action( async (options: { mode?: string; port?: string; path?: string; servePath?: string }) => { const mode = resolveMode(options.mode ?? "funnel"); - const servePort = Number(options.port ?? config.serve.port ?? 3334); + const servePort = parseVoiceCallIntOption( + options.port ?? String(config.serve.port ?? 3334), + "--port", + { min: 1 }, + ); const servePath = options.servePath ?? config.serve.path ?? "/voice/webhook"; const tsPath = options.path ?? config.tailscale?.path ?? servePath; diff --git a/scripts/repro/limit-edge-case-live-proof.mjs b/scripts/repro/limit-edge-case-live-proof.mjs new file mode 100644 index 00000000000..ec1ad199847 --- /dev/null +++ b/scripts/repro/limit-edge-case-live-proof.mjs @@ -0,0 +1,116 @@ +#!/usr/bin/env node +import assert from "node:assert/strict"; +/** + * Live repro for limit/CLI numeric fixes (PR #82679). Run: pnpm exec tsx scripts/repro/limit-edge-case-live-proof.mjs + */ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { __testing as voiceCallCliTesting } from "../../extensions/voice-call/src/cli.ts"; +import { loadSessionLogs, loadSessionUsageTimeSeries } from "../../src/infra/session-cost-usage.ts"; +import { + getRecentDiagnosticPhases, + recordDiagnosticPhase, + resetDiagnosticPhasesForTest, +} from "../../src/logging/diagnostic-phase.ts"; + +async function main() { + resetDiagnosticPhasesForTest(); + recordDiagnosticPhase({ + name: "phase-a", + startedAt: 1, + endedAt: 2, + durationMs: 1, + cpuUserMs: 0, + cpuSystemMs: 0, + cpuTotalMs: 0, + cpuCoreRatio: 0, + }); + recordDiagnosticPhase({ + name: "phase-b", + startedAt: 3, + endedAt: 4, + durationMs: 1, + cpuUserMs: 0, + cpuSystemMs: 0, + cpuTotalMs: 0, + cpuCoreRatio: 0, + }); + const zeroPhases = getRecentDiagnosticPhases(0); + assert.equal(zeroPhases.length, 0); + console.log("getRecentDiagnosticPhases(0).length =", zeroPhases.length); + + const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-proof-")); + const sessionFile = path.join(root, "s.jsonl"); + fs.writeFileSync( + sessionFile, + [ + JSON.stringify({ + type: "message", + timestamp: "2026-01-01T00:00:00.000Z", + message: { role: "user", content: "a" }, + }), + JSON.stringify({ + type: "message", + timestamp: "2026-01-01T00:01:00.000Z", + message: { + role: "assistant", + content: "b", + provider: "openai", + model: "gpt-5.5", + usage: { + input: 1, + output: 2, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 3, + cost: { total: 0.001 }, + }, + }, + }), + JSON.stringify({ + type: "message", + timestamp: "2026-01-01T00:02:00.000Z", + message: { + role: "assistant", + content: "c", + provider: "openai", + model: "gpt-5.5", + usage: { + input: 3, + output: 4, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 7, + cost: { total: 0.002 }, + }, + }, + }), + ].join("\n"), + ); + + const logs = await loadSessionLogs({ sessionFile, limit: 0 }); + const series = await loadSessionUsageTimeSeries({ sessionFile, maxPoints: 0 }); + const positiveLogs = await loadSessionLogs({ sessionFile, limit: 10 }); + const positiveSeries = await loadSessionUsageTimeSeries({ sessionFile, maxPoints: 10 }); + assert.equal(logs?.length, 0); + assert.equal(series.points.length, 0); + assert.equal(positiveLogs?.length, 3); + assert.equal(positiveSeries.points.length, 2); + console.log("loadSessionLogs({ limit: 0 }).length =", logs?.length); + console.log( + "loadSessionUsageTimeSeries({ maxPoints: 0 }).points.length =", + series?.points.length, + ); + + try { + voiceCallCliTesting.parseVoiceCallIntOption("nope", "--port", { min: 1 }); + assert.fail("expected invalid voicecall --port value to throw"); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + assert.equal(message, "Invalid numeric value for --port: nope"); + console.log("parseVoiceCallIntOption('nope', '--port') error:", message); + } +} + +await main(); diff --git a/src/infra/session-cost-usage.test.ts b/src/infra/session-cost-usage.test.ts index cc6ffb988fc..5bcfa43485b 100644 --- a/src/infra/session-cost-usage.test.ts +++ b/src/infra/session-cost-usage.test.ts @@ -1962,4 +1962,97 @@ example expect(lastPoint?.cumulativeTokens).toBe(165); expect(lastPoint?.cumulativeCost).toBeCloseTo(0.055, 8); }); + + it("returns empty points for zero, negative, and non-finite maxPoints", async () => { + const root = await makeSessionCostRoot("timeseries-invalid-max-points"); + const sessionsDir = path.join(root, "agents", "main", "sessions"); + await fs.mkdir(sessionsDir, { recursive: true }); + const sessionFile = path.join(sessionsDir, "sess-invalid-max-points.jsonl"); + const entries = [ + { + type: "message", + timestamp: new Date(Date.UTC(2026, 1, 12, 10, 1, 0)).toISOString(), + message: { + role: "assistant", + provider: "openai", + model: "gpt-5.4", + usage: { + input: 1, + output: 2, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 3, + cost: { total: 0.001 }, + }, + }, + }, + { + type: "message", + timestamp: new Date(Date.UTC(2026, 1, 12, 10, 2, 0)).toISOString(), + message: { + role: "assistant", + provider: "openai", + model: "gpt-5.4", + usage: { + input: 2, + output: 4, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 6, + cost: { total: 0.002 }, + }, + }, + }, + ]; + await fs.writeFile( + sessionFile, + entries.map((entry) => JSON.stringify(entry)).join("\n"), + "utf-8", + ); + + await expect(loadSessionUsageTimeSeries({ sessionFile, maxPoints: 0 })).resolves.toEqual({ + sessionId: undefined, + points: [], + }); + await expect(loadSessionUsageTimeSeries({ sessionFile, maxPoints: -1 })).resolves.toEqual({ + sessionId: undefined, + points: [], + }); + await expect( + loadSessionUsageTimeSeries({ sessionFile, maxPoints: Number.NaN }), + ).resolves.toEqual({ sessionId: undefined, points: [] }); + await expect( + loadSessionUsageTimeSeries({ sessionFile, maxPoints: Number.POSITIVE_INFINITY }), + ).resolves.toEqual({ sessionId: undefined, points: [] }); + }); + + it("returns empty logs for zero, negative, and non-finite limits", async () => { + const root = await makeSessionCostRoot("session-logs-invalid-limit"); + const sessionsDir = path.join(root, "agents", "main", "sessions"); + await fs.mkdir(sessionsDir, { recursive: true }); + const sessionFile = path.join(sessionsDir, "sess-invalid-limit.jsonl"); + await fs.writeFile( + sessionFile, + [ + JSON.stringify({ + type: "message", + timestamp: new Date(Date.UTC(2026, 1, 12, 10, 0, 0)).toISOString(), + message: { role: "user", content: "hello" }, + }), + JSON.stringify({ + type: "message", + timestamp: new Date(Date.UTC(2026, 1, 12, 10, 1, 0)).toISOString(), + message: { role: "user", content: "world" }, + }), + ].join("\n"), + "utf-8", + ); + + await expect(loadSessionLogs({ sessionFile, limit: 0 })).resolves.toEqual([]); + await expect(loadSessionLogs({ sessionFile, limit: -1 })).resolves.toEqual([]); + await expect(loadSessionLogs({ sessionFile, limit: Number.NaN })).resolves.toEqual([]); + await expect( + loadSessionLogs({ sessionFile, limit: Number.POSITIVE_INFINITY }), + ).resolves.toEqual([]); + }); }); diff --git a/src/infra/session-cost-usage.ts b/src/infra/session-cost-usage.ts index fbfa3175edb..638ff917145 100644 --- a/src/infra/session-cost-usage.ts +++ b/src/infra/session-cost-usage.ts @@ -1895,6 +1895,11 @@ export async function loadSessionUsageTimeSeries(params: { const sortedPoints = points.toSorted((a, b) => a.timestamp - b.timestamp); // Optionally downsample if too many points + if (params.maxPoints !== undefined && params.maxPoints !== null) { + if (!Number.isFinite(params.maxPoints) || params.maxPoints <= 0) { + return { sessionId: params.sessionId, points: [] }; + } + } const maxPoints = params.maxPoints ?? 100; if (sortedPoints.length > maxPoints) { const step = Math.ceil(sortedPoints.length / maxPoints); @@ -1958,6 +1963,11 @@ export async function loadSessionLogs(params: { } const logs: SessionLogEntry[] = []; + if (params.limit !== undefined && params.limit !== null) { + if (!Number.isFinite(params.limit) || params.limit <= 0) { + return []; + } + } const limit = params.limit ?? 50; for await (const parsed of readJsonlRecords(sessionFile)) { diff --git a/src/logging/diagnostic-phase.test.ts b/src/logging/diagnostic-phase.test.ts new file mode 100644 index 00000000000..23d9cd551af --- /dev/null +++ b/src/logging/diagnostic-phase.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import { + getRecentDiagnosticPhases, + recordDiagnosticPhase, + resetDiagnosticPhasesForTest, +} from "./diagnostic-phase.js"; + +describe("getRecentDiagnosticPhases", () => { + it("returns an empty list for zero, negative, and non-finite limits", () => { + resetDiagnosticPhasesForTest(); + recordDiagnosticPhase({ + name: "phase-a", + startedAt: 1, + endedAt: 2, + durationMs: 1, + cpuUserMs: 0, + cpuSystemMs: 0, + cpuTotalMs: 0, + cpuCoreRatio: 0, + }); + recordDiagnosticPhase({ + name: "phase-b", + startedAt: 3, + endedAt: 4, + durationMs: 1, + cpuUserMs: 0, + cpuSystemMs: 0, + cpuTotalMs: 0, + cpuCoreRatio: 0, + }); + + expect(getRecentDiagnosticPhases(0)).toEqual([]); + expect(getRecentDiagnosticPhases(-1)).toEqual([]); + expect(getRecentDiagnosticPhases(Number.NaN)).toEqual([]); + expect(getRecentDiagnosticPhases(Number.POSITIVE_INFINITY)).toEqual([]); + }); + + it("returns the most recent phases for positive limits", () => { + resetDiagnosticPhasesForTest(); + recordDiagnosticPhase({ + name: "phase-a", + startedAt: 1, + endedAt: 2, + durationMs: 1, + cpuUserMs: 0, + cpuSystemMs: 0, + cpuTotalMs: 0, + cpuCoreRatio: 0, + }); + recordDiagnosticPhase({ + name: "phase-b", + startedAt: 3, + endedAt: 4, + durationMs: 1, + cpuUserMs: 0, + cpuSystemMs: 0, + cpuTotalMs: 0, + cpuCoreRatio: 0, + }); + + const recent = getRecentDiagnosticPhases(1); + expect(recent).toHaveLength(1); + expect(recent[0]?.name).toBe("phase-b"); + }); +}); diff --git a/src/logging/diagnostic-phase.ts b/src/logging/diagnostic-phase.ts index d6b8ddbb1bc..818462fe59a 100644 --- a/src/logging/diagnostic-phase.ts +++ b/src/logging/diagnostic-phase.ts @@ -38,8 +38,19 @@ export function getCurrentDiagnosticPhase(): string | undefined { return activePhaseStack.at(-1)?.name; } +function resolveRecentPhaseLimit(limit: number): number | null { + if (!Number.isFinite(limit) || limit <= 0) { + return null; + } + return Math.floor(limit); +} + export function getRecentDiagnosticPhases(limit = 8): DiagnosticPhaseSnapshot[] { - return recentPhases.slice(-Math.max(0, limit)).map((phase) => Object.assign({}, phase)); + const resolved = resolveRecentPhaseLimit(limit); + if (resolved === null) { + return []; + } + return recentPhases.slice(-resolved).map((phase) => Object.assign({}, phase)); } export function recordDiagnosticPhase(snapshot: DiagnosticPhaseSnapshot): void {