From be4fd079462d9094148dd317b617702b1637365f Mon Sep 17 00:00:00 2001 From: B <6723574+louisgv@users.noreply.github.com> Date: Fri, 6 Mar 2026 22:17:41 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20add=20schema=20versioning=20to=20history?= =?UTF-8?q?.json=20(v0=20bare=20array=20=E2=86=92=20v1=20wrapped)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #2252 history.json now uses a versioned envelope: { "version": 1, "records": [...] } This creates a migration escape hatch for future SpawnRecord shape changes. loadHistory() transparently reads both v0 (bare array) and v1 formats, automatically migrating v0 files on next write. All write operations now use writeHistory() to stamp the current schema version consistently. Validation uses valibot schemas (VMConnectionSchema, SpawnRecordSchema, HistoryFileV1Schema) so the structure is verified and typed without `as` casts. Updated all affected tests to check data.records instead of data. Agent: issue-fixer Co-Authored-By: Claude Sonnet 4.6 --- .../src/__tests__/cmdrun-happy-path.test.ts | 34 ++--- .../src/__tests__/history-trimming.test.ts | 9 +- packages/cli/src/__tests__/history.test.ts | 123 ++++++++++++++++-- packages/cli/src/history.ts | 101 ++++++++++---- 4 files changed, 211 insertions(+), 56 deletions(-) diff --git a/packages/cli/src/__tests__/cmdrun-happy-path.test.ts b/packages/cli/src/__tests__/cmdrun-happy-path.test.ts index efdb44cdd..b81ff52a6 100644 --- a/packages/cli/src/__tests__/cmdrun-happy-path.test.ts +++ b/packages/cli/src/__tests__/cmdrun-happy-path.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:te import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { join } from "node:path"; +import { HISTORY_SCHEMA_VERSION } from "../history.js"; import { loadManifest } from "../manifest"; import { isString } from "../shared/type-guards"; import { createConsoleMocks, createMockManifest, mockClackPrompts, restoreMocks } from "./test-helpers"; @@ -262,9 +263,10 @@ describe("cmdRun happy-path pipeline", () => { const historyPath = join(historyDir, "history.json"); expect(existsSync(historyPath)).toBe(true); - const records = JSON.parse(readFileSync(historyPath, "utf-8")); - expect(records.length).toBeGreaterThanOrEqual(1); - const record = records[records.length - 1]; + const data = JSON.parse(readFileSync(historyPath, "utf-8")); + expect(data.version).toBe(HISTORY_SCHEMA_VERSION); + expect(data.records.length).toBeGreaterThanOrEqual(1); + const record = data.records[data.records.length - 1]; expect(record.agent).toBe("claude"); expect(record.cloud).toBe("sprite"); expect(record.timestamp).toBeDefined(); @@ -279,8 +281,8 @@ describe("cmdRun happy-path pipeline", () => { await cmdRun("claude", "sprite", "Fix all bugs"); const historyPath = join(historyDir, "history.json"); - const records = JSON.parse(readFileSync(historyPath, "utf-8")); - const record = records[records.length - 1]; + const data = JSON.parse(readFileSync(historyPath, "utf-8")); + const record = data.records[data.records.length - 1]; expect(record.prompt).toBe("Fix all bugs"); }); @@ -293,8 +295,8 @@ describe("cmdRun happy-path pipeline", () => { await cmdRun("claude", "sprite"); const historyPath = join(historyDir, "history.json"); - const records = JSON.parse(readFileSync(historyPath, "utf-8")); - const record = records[records.length - 1]; + const data = JSON.parse(readFileSync(historyPath, "utf-8")); + const record = data.records[data.records.length - 1]; expect(record.prompt).toBeUndefined(); }); @@ -309,8 +311,8 @@ describe("cmdRun happy-path pipeline", () => { const after = new Date().toISOString(); const historyPath = join(historyDir, "history.json"); - const records = JSON.parse(readFileSync(historyPath, "utf-8")); - const record = records[records.length - 1]; + const data = JSON.parse(readFileSync(historyPath, "utf-8")); + const record = data.records[data.records.length - 1]; expect(record.timestamp >= before).toBe(true); expect(record.timestamp <= after).toBe(true); }); @@ -331,9 +333,9 @@ describe("cmdRun happy-path pipeline", () => { const historyPath = join(historyDir, "history.json"); expect(existsSync(historyPath)).toBe(true); - const records = JSON.parse(readFileSync(historyPath, "utf-8")); - expect(records.length).toBeGreaterThanOrEqual(1); - expect(records[records.length - 1].agent).toBe("claude"); + const data = JSON.parse(readFileSync(historyPath, "utf-8")); + expect(data.records.length).toBeGreaterThanOrEqual(1); + expect(data.records[data.records.length - 1].agent).toBe("claude"); }); it("should still execute script when history save fails", async () => { @@ -381,10 +383,10 @@ describe("cmdRun happy-path pipeline", () => { await cmdRun("claude", "sprite"); const historyPath = join(historyDir, "history.json"); - const records = JSON.parse(readFileSync(historyPath, "utf-8")); - expect(records).toHaveLength(2); - expect(records[0].agent).toBe("codex"); - expect(records[1].agent).toBe("claude"); + const data = JSON.parse(readFileSync(historyPath, "utf-8")); + expect(data.records).toHaveLength(2); + expect(data.records[0].agent).toBe("codex"); + expect(data.records[1].agent).toBe("claude"); }); }); diff --git a/packages/cli/src/__tests__/history-trimming.test.ts b/packages/cli/src/__tests__/history-trimming.test.ts index 63326a721..43f752338 100644 --- a/packages/cli/src/__tests__/history-trimming.test.ts +++ b/packages/cli/src/__tests__/history-trimming.test.ts @@ -4,7 +4,7 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test"; import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { join } from "node:path"; -import { filterHistory, loadHistory, saveSpawnRecord } from "../history.js"; +import { filterHistory, HISTORY_SCHEMA_VERSION, loadHistory, saveSpawnRecord } from "../history.js"; /** * Tests for history trimming and boundary behavior. @@ -849,12 +849,13 @@ describe("History Trimming and Boundaries", () => { timestamp: "2026-01-02T00:00:00.000Z", }); - // Read raw file and verify it's valid JSON + // Read raw file and verify it's valid v1 JSON const raw = readFileSync(join(testDir, "history.json"), "utf-8"); expect(() => JSON.parse(raw)).not.toThrow(); const parsed = JSON.parse(raw); - expect(Array.isArray(parsed)).toBe(true); - expect(parsed).toHaveLength(100); + expect(parsed.version).toBe(HISTORY_SCHEMA_VERSION); + expect(Array.isArray(parsed.records)).toBe(true); + expect(parsed.records).toHaveLength(100); }); it("should write pretty-printed JSON with trailing newline after trimming", () => { diff --git a/packages/cli/src/__tests__/history.test.ts b/packages/cli/src/__tests__/history.test.ts index 573a5de4a..d64b1c0fe 100644 --- a/packages/cli/src/__tests__/history.test.ts +++ b/packages/cli/src/__tests__/history.test.ts @@ -4,7 +4,14 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test"; import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { join } from "node:path"; -import { filterHistory, getHistoryPath, getSpawnDir, loadHistory, saveSpawnRecord } from "../history.js"; +import { + filterHistory, + getHistoryPath, + getSpawnDir, + HISTORY_SCHEMA_VERSION, + loadHistory, + saveSpawnRecord, +} from "../history.js"; describe("history", () => { let testDir: string; @@ -185,6 +192,55 @@ describe("history", () => { writeFileSync(join(testDir, "history.json"), ""); expect(loadHistory()).toEqual([]); }); + + it("loads v1 format: { version: 1, records: [...] }", () => { + const records: SpawnRecord[] = [ + { + agent: "claude", + cloud: "sprite", + timestamp: "2026-01-01T00:00:00.000Z", + }, + ]; + writeFileSync( + join(testDir, "history.json"), + JSON.stringify({ + version: 1, + records, + }), + ); + expect(loadHistory()).toEqual(records); + }); + + it("returns empty array for v1 format with unknown version", () => { + const records: SpawnRecord[] = [ + { + agent: "claude", + cloud: "sprite", + timestamp: "2026-01-01T00:00:00.000Z", + }, + ]; + writeFileSync( + join(testDir, "history.json"), + JSON.stringify({ + version: 99, + records, + }), + ); + // Unknown version is not a recognized format; treated as invalid non-array + expect(loadHistory()).toEqual([]); + }); + + it("loads v0 format: bare array (backward compatibility)", () => { + const records: SpawnRecord[] = [ + { + agent: "claude", + cloud: "sprite", + timestamp: "2026-01-01T00:00:00.000Z", + }, + ]; + writeFileSync(join(testDir, "history.json"), JSON.stringify(records)); + expect(loadHistory()).toEqual(records); + }); }); // ── saveSpawnRecord ───────────────────────────────────────────────────── @@ -202,8 +258,9 @@ describe("history", () => { expect(existsSync(join(nestedDir, "history.json"))).toBe(true); const data = JSON.parse(readFileSync(join(nestedDir, "history.json"), "utf-8")); - expect(data).toHaveLength(1); - expect(data[0].agent).toBe("claude"); + expect(data.version).toBe(HISTORY_SCHEMA_VERSION); + expect(data.records).toHaveLength(1); + expect(data.records[0].agent).toBe("claude"); // Clean up rmSync(join(homedir(), ".spawn-test"), { @@ -229,9 +286,10 @@ describe("history", () => { }); const data = JSON.parse(readFileSync(join(testDir, "history.json"), "utf-8")); - expect(data).toHaveLength(2); - expect(data[0].agent).toBe("claude"); - expect(data[1].agent).toBe("codex"); + expect(data.version).toBe(HISTORY_SCHEMA_VERSION); + expect(data.records).toHaveLength(2); + expect(data.records[0].agent).toBe("claude"); + expect(data.records[1].agent).toBe("codex"); }); it("saves record with prompt field", () => { @@ -243,7 +301,7 @@ describe("history", () => { }); const data = JSON.parse(readFileSync(join(testDir, "history.json"), "utf-8")); - expect(data[0].prompt).toBe("Fix all linter errors"); + expect(data.records[0].prompt).toBe("Fix all linter errors"); }); it("saves record without prompt field", () => { @@ -254,7 +312,7 @@ describe("history", () => { }); const data = JSON.parse(readFileSync(join(testDir, "history.json"), "utf-8")); - expect(data[0].prompt).toBeUndefined(); + expect(data.records[0].prompt).toBeUndefined(); }); it("writes pretty-printed JSON with trailing newline", () => { @@ -281,9 +339,47 @@ describe("history", () => { } const data = JSON.parse(readFileSync(join(testDir, "history.json"), "utf-8")); - expect(data).toHaveLength(5); - expect(data[0].agent).toBe("agent-0"); - expect(data[4].agent).toBe("agent-4"); + expect(data.version).toBe(HISTORY_SCHEMA_VERSION); + expect(data.records).toHaveLength(5); + expect(data.records[0].agent).toBe("agent-0"); + expect(data.records[4].agent).toBe("agent-4"); + }); + + it("writes v1 format with version and records fields", () => { + saveSpawnRecord({ + agent: "claude", + cloud: "sprite", + timestamp: "2026-01-01T00:00:00.000Z", + }); + + const data = JSON.parse(readFileSync(join(testDir, "history.json"), "utf-8")); + expect(data.version).toBe(HISTORY_SCHEMA_VERSION); + expect(Array.isArray(data.records)).toBe(true); + }); + + it("migrates v0 bare array to v1 format on next save", () => { + const existing: SpawnRecord[] = [ + { + agent: "claude", + cloud: "sprite", + timestamp: "2026-01-01T00:00:00.000Z", + }, + ]; + // Write v0 bare array + writeFileSync(join(testDir, "history.json"), JSON.stringify(existing)); + + // Trigger a write via saveSpawnRecord + saveSpawnRecord({ + agent: "codex", + cloud: "hetzner", + timestamp: "2026-01-02T00:00:00.000Z", + }); + + const data = JSON.parse(readFileSync(join(testDir, "history.json"), "utf-8")); + expect(data.version).toBe(HISTORY_SCHEMA_VERSION); + expect(data.records).toHaveLength(2); + expect(data.records[0].agent).toBe("claude"); + expect(data.records[1].agent).toBe("codex"); }); it("recovers from corrupted existing history file", () => { @@ -297,8 +393,9 @@ describe("history", () => { // loadHistory returns [] for corrupted files, so saveSpawnRecord starts fresh const data = JSON.parse(readFileSync(join(testDir, "history.json"), "utf-8")); - expect(data).toHaveLength(1); - expect(data[0].agent).toBe("claude"); + expect(data.version).toBe(HISTORY_SCHEMA_VERSION); + expect(data.records).toHaveLength(1); + expect(data.records[0].agent).toBe("claude"); }); }); diff --git a/packages/cli/src/history.ts b/packages/cli/src/history.ts index 2603b22de..eacdbb486 100644 --- a/packages/cli/src/history.ts +++ b/packages/cli/src/history.ts @@ -2,6 +2,7 @@ import { randomUUID } from "node:crypto"; import { existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { isAbsolute, join, resolve } from "node:path"; +import * as v from "valibot"; import { validateConnectionIP, validateLaunchCmd, validateServerIdentifier, validateUsername } from "./security.js"; import { isString } from "./shared/type-guards"; @@ -27,6 +28,38 @@ export interface SpawnRecord { connection?: VMConnection; } +// ── Schema versioning ────────────────────────────────────────────────────── + +export const HISTORY_SCHEMA_VERSION = 1; + +const VMConnectionSchema = v.object({ + ip: v.string(), + user: v.string(), + server_id: v.optional(v.string()), + server_name: v.optional(v.string()), + cloud: v.optional(v.string()), + deleted: v.optional(v.boolean()), + deleted_at: v.optional(v.string()), + launch_cmd: v.optional(v.string()), + metadata: v.optional(v.record(v.string(), v.string())), +}); + +const SpawnRecordSchema = v.object({ + id: v.optional(v.string()), + agent: v.string(), + cloud: v.string(), + timestamp: v.string(), + name: v.optional(v.string()), + prompt: v.optional(v.string()), + connection: v.optional(VMConnectionSchema), +}); + +/** v1 history file format: { version: 1, records: SpawnRecord[] } */ +const HistoryFileV1Schema = v.object({ + version: v.literal(1), + records: v.array(SpawnRecordSchema), +}); + /** Generate a unique spawn ID. */ export function generateSpawnId(): string { return randomUUID(); @@ -69,6 +102,24 @@ export function getConnectionPath(): string { return join(getSpawnDir(), "last-connection.json"); } +/** Write history records to disk in v1 format: { version: 1, records: [...] } */ +function writeHistory(records: SpawnRecord[]): void { + writeFileSync( + getHistoryPath(), + JSON.stringify( + { + version: HISTORY_SCHEMA_VERSION, + records, + }, + null, + 2, + ) + "\n", + { + mode: 0o600, + }, + ); +} + /** Save VM connection info directly into history.json. * Matches by spawnId for exact targeting. Falls back to heuristic matching * for backward compatibility with records that have no id. */ @@ -121,9 +172,7 @@ export function saveVmConnection( } if (merged) { - writeFileSync(getHistoryPath(), JSON.stringify(history, null, 2) + "\n", { - mode: 0o600, - }); + writeHistory(history); } // Also write last-connection.json for backward compatibility @@ -180,9 +229,7 @@ export function saveLaunchCmd(launchCmd: string, spawnId?: string): void { } if (found) { - writeFileSync(getHistoryPath(), JSON.stringify(history, null, 2) + "\n", { - mode: 0o600, - }); + writeHistory(history); } } catch { // non-fatal @@ -207,8 +254,24 @@ export function loadHistory(): SpawnRecord[] { return []; } try { - const data = JSON.parse(readFileSync(path, "utf-8")); - return Array.isArray(data) ? data : []; + const text = readFileSync(path, "utf-8"); + if (!text.trim()) { + return []; + } + const raw: unknown = JSON.parse(text); + + // v1 format: { version: 1, records: [...] } + const v1 = v.safeParse(HistoryFileV1Schema, raw); + if (v1.success) { + return v1.output.records; + } + + // v0 format: bare array (pre-versioning; migrated to v1 on next write) + if (Array.isArray(raw)) { + return raw; + } + + return []; } catch { return []; } @@ -287,9 +350,7 @@ export function saveSpawnRecord(record: SpawnRecord): void { ]); } } - writeFileSync(getHistoryPath(), JSON.stringify(history, null, 2) + "\n", { - mode: 0o600, - }); + writeHistory(history); } export function clearHistory(): number { @@ -325,9 +386,9 @@ function mergeLastConnection(): void { let metadata: Record | undefined; if (entries.metadata && typeof entries.metadata === "object" && !Array.isArray(entries.metadata)) { metadata = {}; - for (const [k, v] of Object.entries(entries.metadata)) { - if (isString(v)) { - metadata[k] = v; + for (const [k, val] of Object.entries(entries.metadata)) { + if (isString(val)) { + metadata[k] = val; } } if (Object.keys(metadata).length === 0) { @@ -391,9 +452,7 @@ function mergeLastConnection(): void { } } if (merged) { - writeFileSync(getHistoryPath(), JSON.stringify(history, null, 2) + "\n", { - mode: 0o600, - }); + writeHistory(history); } // Clean up the connection file after merging @@ -425,9 +484,7 @@ export function removeRecord(record: SpawnRecord): boolean { return false; } history.splice(index, 1); - writeFileSync(getHistoryPath(), JSON.stringify(history, null, 2) + "\n", { - mode: 0o600, - }); + writeHistory(history); return true; } @@ -443,9 +500,7 @@ export function markRecordDeleted(record: SpawnRecord): boolean { } found.connection.deleted = true; found.connection.deleted_at = new Date().toISOString(); - writeFileSync(getHistoryPath(), JSON.stringify(history, null, 2) + "\n", { - mode: 0o600, - }); + writeHistory(history); return true; }