From 8bae382a3c88abc688e7405baf9a661ed56a4149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B0=8F=E6=A9=98?= Date: Sat, 25 Apr 2026 09:00:58 +0000 Subject: [PATCH] fix(cli): handle invalid timestamps in workflow commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit formatTs() now guards against null/undefined/NaN/Infinity timestamps, returning '(unknown)' instead of crashing with 'Invalid time value'. Added 18+ unit tests covering edge cases for formatTs, formatRunLine, buildListOutput, buildInspectOutput, and formatThreadRoundBlock. Fixes #139 小橘 --- .../src/__tests__/workflow-cli-e2e.test.ts | 90 +++++++++++++ packages/cli/src/__tests__/workflow.test.ts | 122 +++++++++++++++++- packages/cli/src/commands/workflow.ts | 21 ++- 3 files changed, 229 insertions(+), 4 deletions(-) create mode 100644 packages/cli/src/__tests__/workflow-cli-e2e.test.ts diff --git a/packages/cli/src/__tests__/workflow-cli-e2e.test.ts b/packages/cli/src/__tests__/workflow-cli-e2e.test.ts new file mode 100644 index 0000000..e149cda --- /dev/null +++ b/packages/cli/src/__tests__/workflow-cli-e2e.test.ts @@ -0,0 +1,90 @@ +/** + * Smoke / integration tests for `nerve workflow` citty handlers with a real HOME + * layout and logs.db. `loadDaemonModule` is mocked so tests use workspace + * `@uncaged/nerve-store` directly (no ~/.uncaged-nerve daemon install required). + */ + +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { runCommand } from "citty"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("../workspace-daemon.js", async () => { + const { createLogStore } = await import("@uncaged/nerve-store"); + return { + loadDaemonModule: vi.fn(async () => ({ createLogStore })), + }; +}); + +import { createLogStore } from "@uncaged/nerve-store"; + +import { workflowCommand } from "../commands/workflow.js"; + +describe("nerve workflow CLI (runCommand + temp HOME)", () => { + let prevHome: string | undefined; + let fakeHome: string; + let stdoutSpy: ReturnType | null; + + beforeEach(() => { + stdoutSpy = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + prevHome = process.env.HOME; + fakeHome = mkdtempSync(join(tmpdir(), "nerve-wf-cli-e2e-")); + process.env.HOME = fakeHome; + + const nerveRoot = join(fakeHome, ".uncaged-nerve"); + mkdirSync(join(nerveRoot, "data"), { recursive: true }); + const dbPath = join(nerveRoot, "data", "logs.db"); + const store = createLogStore(dbPath); + + store.upsertWorkflowRun( + { source: "workflow", type: "started", refId: "e2e-run", payload: "{}", timestamp: 5000 }, + { runId: "e2e-run", workflow: "demo", status: "completed", timestamp: 5000, exitCode: 0 }, + ); + store.append({ + source: "workflow", + type: "completed", + refId: "e2e-run", + payload: null, + timestamp: 5001, + }); + store.append({ + source: "workflow", + type: "thread_command_event", + refId: "e2e-run", + payload: JSON.stringify({ type: "step", role: "bot", content: "hello" }), + timestamp: 5100, + }); + store.close(); + }); + + afterEach(() => { + stdoutSpy?.mockRestore(); + stdoutSpy = null; + if (prevHome === undefined) { + process.env.HOME = undefined; + } else { + process.env.HOME = prevHome; + } + rmSync(fakeHome, { recursive: true, force: true }); + }); + + it("workflow runs --all completes without throwing", async () => { + await expect( + runCommand(workflowCommand, { rawArgs: ["runs", "--all"] }), + ).resolves.toBeDefined(); + }); + + it("workflow inspect completes without throwing", async () => { + await expect( + runCommand(workflowCommand, { rawArgs: ["inspect", "e2e-run", "--limit", "10"] }), + ).resolves.toBeDefined(); + }); + + it("workflow thread completes without throwing (role rounds path)", async () => { + await expect( + runCommand(workflowCommand, { rawArgs: ["thread", "e2e-run", "--budget", "50000"] }), + ).resolves.toBeDefined(); + }); +}); diff --git a/packages/cli/src/__tests__/workflow.test.ts b/packages/cli/src/__tests__/workflow.test.ts index 0c0d582..239e3fa 100644 --- a/packages/cli/src/__tests__/workflow.test.ts +++ b/packages/cli/src/__tests__/workflow.test.ts @@ -18,9 +18,11 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import type { LogStore, ThreadRoundRow, WorkflowRun } from "@uncaged/nerve-store"; import { DEFAULT_THREAD_BUDGET_CHARS, + UNKNOWN_TIMESTAMP_LABEL, buildInspectOutput, buildListOutput, buildThreadCommandOutput, + formatRunLine, formatThreadRoundBlock, formatTs, getAllWorkflowRuns, @@ -64,10 +66,75 @@ afterEach(() => { // --------------------------------------------------------------------------- describe("formatTs", () => { - it("returns ISO 8601 string", () => { + it("returns ISO 8601 string for a valid UTC instant", () => { const timestampMs = new Date("2026-01-01T00:00:00.000Z").getTime(); expect(formatTs(timestampMs)).toBe("2026-01-01T00:00:00.000Z"); }); + + it("returns placeholder for null and undefined", () => { + expect(formatTs(null)).toBe(UNKNOWN_TIMESTAMP_LABEL); + expect(formatTs(undefined)).toBe(UNKNOWN_TIMESTAMP_LABEL); + }); + + it("returns placeholder for NaN and non-finite numbers", () => { + expect(formatTs(Number.NaN)).toBe(UNKNOWN_TIMESTAMP_LABEL); + expect(formatTs(Number.POSITIVE_INFINITY)).toBe(UNKNOWN_TIMESTAMP_LABEL); + expect(formatTs(Number.NEGATIVE_INFINITY)).toBe(UNKNOWN_TIMESTAMP_LABEL); + }); + + it("returns placeholder for non-number runtime values", () => { + expect(formatTs("2024" as unknown as number)).toBe(UNKNOWN_TIMESTAMP_LABEL); + expect(formatTs({} as unknown as number)).toBe(UNKNOWN_TIMESTAMP_LABEL); + }); + + it("formats 0 as Unix epoch", () => { + expect(formatTs(0)).toBe("1970-01-01T00:00:00.000Z"); + }); + + it("formats negative finite values as ISO strings", () => { + expect(formatTs(-1)).toBe("1969-12-31T23:59:59.999Z"); + }); +}); + +// --------------------------------------------------------------------------- +// formatRunLine +// --------------------------------------------------------------------------- + +describe("formatRunLine", () => { + it("omits exit_code segment when exitCode is null", () => { + const run: WorkflowRun = { + runId: "r1", + workflow: "wf", + status: "started", + timestamp: 1000, + exitCode: null, + }; + const line = formatRunLine(run); + expect(line).not.toContain("exit_code"); + expect(line).toContain("timestamp=1970-01-01T00:00:01.000Z"); + }); + + it("includes exit_code when set", () => { + const run: WorkflowRun = { + runId: "r1", + workflow: "wf", + status: "failed", + timestamp: 2000, + exitCode: 7, + }; + expect(formatRunLine(run)).toContain("exit_code=7"); + }); + + it("uses unknown timestamp label for bad run timestamps", () => { + const run = { + runId: "r-bad", + workflow: "wf", + status: "completed" as const, + timestamp: Number.NaN, + exitCode: null, + } as WorkflowRun; + expect(formatRunLine(run)).toContain(`timestamp=${UNKNOWN_TIMESTAMP_LABEL}`); + }); }); // --------------------------------------------------------------------------- @@ -150,7 +217,7 @@ describe("buildListOutput", () => { status: WorkflowRun["status"], timestampMs: number, ): WorkflowRun { - return { runId, workflow, status, timestamp: timestampMs }; + return { runId, workflow, status, timestamp: timestampMs, exitCode: null }; } it("returns empty message when no runs and --all=false", () => { @@ -226,6 +293,22 @@ describe("buildListOutput", () => { expect(paginationHint).toContain("1 more"); expect(paginationHint).toContain("--offset 4"); }); + + it("does not throw when a run has null exit_code and invalid timestamp", () => { + const runs: WorkflowRun[] = [ + { + runId: "bad-ts", + workflow: "wf", + status: "completed", + timestamp: null as unknown as number, + exitCode: null, + }, + ]; + const { lines } = buildListOutput(runs, 0, 20, true, null); + const text = lines.join(""); + expect(text).toContain(UNKNOWN_TIMESTAMP_LABEL); + expect(text).not.toContain("exit_code"); + }); }); // --------------------------------------------------------------------------- @@ -238,6 +321,7 @@ describe("buildInspectOutput", () => { workflow: "cleanup", status: "completed", timestamp: 1_700_000_000_000, + exitCode: null, }; it("shows header with run details", () => { @@ -293,6 +377,21 @@ describe("buildInspectOutput", () => { const { paginationHint } = buildInspectOutput(baseRun, logs, 0, 20); expect(paginationHint).toBeNull(); }); + + it("renders unknown labels for bad run and event timestamps without throwing", () => { + const run: WorkflowRun = { + ...baseRun, + timestamp: Number.NaN as unknown as number, + }; + const logs = [{ timestamp: Number.NaN as unknown as number, type: "started", payload: null }]; + const { header, eventLines } = buildInspectOutput(run, logs, 0, 20); + const all = [...header, ...eventLines].join(""); + expect(all).toContain(UNKNOWN_TIMESTAMP_LABEL); + expect(all.match(new RegExp(UNKNOWN_TIMESTAMP_LABEL, "g"))).not.toBeNull(); + expect( + (all.match(new RegExp(UNKNOWN_TIMESTAMP_LABEL, "g")) ?? []).length, + ).toBeGreaterThanOrEqual(2); + }); }); // --------------------------------------------------------------------------- @@ -338,6 +437,7 @@ describe("partitionWorkflowMessage", () => { role: "scanner", content: "ok", meta: { items: [1, 2] }, + timestamp: 1, }); expect(p.roleStr).toBe("scanner"); expect(p.contentBody).toBe("ok"); @@ -371,6 +471,24 @@ describe("formatThreadRoundBlock", () => { expect(text).toContain("score: 0.5"); expect(text).toContain("hi"); }); + + it("uses unknown label when row timestamp is null (defensive)", () => { + const badRow = { + ...row, + timestamp: null as unknown as number, + }; + expect(formatThreadRoundBlock(badRow)).toContain(UNKNOWN_TIMESTAMP_LABEL); + }); + + it("uses unknown label when row timestamp is NaN (defensive)", () => { + const badRow = { ...row, timestamp: Number.NaN }; + expect(formatThreadRoundBlock(badRow)).toContain(UNKNOWN_TIMESTAMP_LABEL); + }); + + it("uses unknown label when row timestamp is undefined (defensive)", () => { + const badRow = { ...row, timestamp: undefined as unknown as number }; + expect(formatThreadRoundBlock(badRow)).toContain(UNKNOWN_TIMESTAMP_LABEL); + }); }); describe("buildThreadCommandOutput", () => { diff --git a/packages/cli/src/commands/workflow.ts b/packages/cli/src/commands/workflow.ts index e8e9209..3b0f040 100644 --- a/packages/cli/src/commands/workflow.ts +++ b/packages/cli/src/commands/workflow.ts @@ -29,8 +29,25 @@ export function getDbPath(): string { return join(getNerveRoot(), "data", "logs.db"); } -export function formatTs(timestampMs: number): string { - return new Date(timestampMs).toISOString(); +/** Human-readable placeholder when a timestamp is missing or not representable as ISO 8601. */ +export const UNKNOWN_TIMESTAMP_LABEL = "(unknown)"; + +/** + * Format epoch milliseconds as UTC ISO 8601, or {@link UNKNOWN_TIMESTAMP_LABEL} when the value + * is nullish, not a finite number, or cannot be converted (defensive against bad DB / test data). + */ +export function formatTs(timestampMs: number | null | undefined): string { + if (timestampMs === null || timestampMs === undefined) { + return UNKNOWN_TIMESTAMP_LABEL; + } + if (typeof timestampMs !== "number" || !Number.isFinite(timestampMs)) { + return UNKNOWN_TIMESTAMP_LABEL; + } + try { + return new Date(timestampMs).toISOString(); + } catch { + return UNKNOWN_TIMESTAMP_LABEL; + } } async function openStore(): Promise { -- 2.43.0