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>
2026-05-18 08:34:46 +00:00 · 2026-05-17 07:11:46 +08:00
parent 99a269f8b4
commit cc9117f729
8 changed files with 362 additions and 5 deletions
--- 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.
--- 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`);
--- 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<string, unknown> {
  return Boolean(value && typeof value === "object" && !Array.isArray(value));
 }
@@ -708,8 +722,8 @@ export function registerVoiceCallCli(params: {
    .option("--poll <ms>", "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 <n>", "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;

--- a/scripts/repro/limit-edge-case-live-proof.mjs
+++ 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();
--- 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([]);
+  });
 });
--- 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)) {
--- a/src/logging/diagnostic-phase.test.ts
+++ 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");
+  });
+});
--- 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 {