diff --git a/improvements.md b/improvements.md index 5cc60ad..44606c4 100644 --- a/improvements.md +++ b/improvements.md @@ -1709,6 +1709,16 @@ so `npm run build` never typechecks them and the compiler could not have caught these stale fixture shapes. Including them surfaces 37 existing type errors today; widening `include` is a separate change. +**Follow-up 2026-10-01:** `tsconfig.json` now includes `tests/`, so +`npm run build` and CI type-check the tests too. The 37 errors were all +fixture drift: state entries still carrying `lastPulledHash` / +`lastPushedHash` or bare-string values, an untyped `emptyLoaded()` fixture, +and one real gap — the `reconcile-state-key` harness never passed the +required `formatError`, so any test reaching that error path would have +thrown a `TypeError` instead of exercising it. Fixtures now use the `{ uuid }` +shape and mark "which copy won" with distinct UUIDs instead of removed hash +fields. + --- ## Out of scope (intentionally not improvements) diff --git a/src/sim.ts b/src/sim.ts index 9715753..c63a4e6 100644 --- a/src/sim.ts +++ b/src/sim.ts @@ -9,6 +9,7 @@ import { existsSync, readFileSync } from "fs"; import { dirname, join } from "path"; import { fileURLToPath } from "url"; import type { StateFile } from "./types.ts"; +import { userAgentGet } from "./user-agent.ts"; const __dirname = dirname(fileURLToPath(import.meta.url)); const BASE_DIR = join(__dirname, ".."); @@ -219,6 +220,7 @@ async function fetchJson( headers: { Authorization: `Bearer ${cfg.token}`, "Content-Type": "application/json", + "User-Agent": userAgentGet("sim"), }, ...(body ? { body: JSON.stringify(body) } : {}), }); diff --git a/src/user-agent.ts b/src/user-agent.ts new file mode 100644 index 0000000..39a383b --- /dev/null +++ b/src/user-agent.ts @@ -0,0 +1,39 @@ +// User-Agent for the API requests this tool makes, so simulation runs started +// from gitops can be told apart in the platform's analytics. +// +// Config-free on purpose (like api-key.ts): importing config.ts would parse +// argv and exit, which breaks importing this from sim.ts and tests. + +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const PACKAGE_JSON_PATH = join( + dirname(fileURLToPath(import.meta.url)), + "..", + "package.json", +); + +function packageVersionRead(): string { + try { + const parsed: unknown = JSON.parse( + readFileSync(PACKAGE_JSON_PATH, "utf-8"), + ); + if ( + parsed && + typeof parsed === "object" && + "version" in parsed && + typeof parsed.version === "string" + ) { + return parsed.version; + } + } catch { + // Fall through: a missing or unreadable package.json must never block + // an API request. + } + return "unknown"; +} + +export function userAgentGet(product: "sim" | "check"): string { + return `vapi-gitops-${product}/${packageVersionRead()}`; +} diff --git a/tests/credentials.test.ts b/tests/credentials.test.ts index 109e7d6..09c85d7 100644 --- a/tests/credentials.test.ts +++ b/tests/credentials.test.ts @@ -1,6 +1,10 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { replaceCredentialRefs } from "../src/credentials.ts"; +import { + credentialForwardMap as forwardMap, + credentialReverseMap as reverseMap, + replaceCredentialRefs, +} from "../src/credentials.ts"; import type { StateFile } from "../src/types.ts"; // Regression tests for P0-1. @@ -14,9 +18,12 @@ import type { StateFile } from "../src/types.ts"; // API then rejects on POST/PATCH. These tests lock in the scoped semantics // (only swap at exactly `credentialId` / `credentialIds` keys). +// Takes `name → uuid` for brevity and builds the `{ uuid }` state shape. function makeState(creds: Record): StateFile { return { - credentials: creds, + credentials: Object.fromEntries( + Object.entries(creds).map(([name, uuid]) => [name, { uuid }]), + ), assistants: {}, structuredOutputs: {}, tools: {}, @@ -29,22 +36,6 @@ function makeState(creds: Record): StateFile { }; } -function reverseMap(state: StateFile): Map { - const m = new Map(); - for (const [name, uuid] of Object.entries(state.credentials)) { - m.set(uuid, name); - } - return m; -} - -function forwardMap(state: StateFile): Map { - const m = new Map(); - for (const [name, uuid] of Object.entries(state.credentials)) { - m.set(name, uuid); - } - return m; -} - test("replaceCredentialRefs swaps at credentialId keys", () => { const state = makeState({ "roofr-server-credential": "11111111-1111-1111-1111-111111111111", diff --git a/tests/recanonicalize.test.ts b/tests/recanonicalize.test.ts index 479513f..e9283e4 100644 --- a/tests/recanonicalize.test.ts +++ b/tests/recanonicalize.test.ts @@ -189,16 +189,11 @@ test("recanonicalize: AUTO-RESOLVES when canonical slug claims the SAME UUID (du // SAME uuid_A. This is not a twin — it's one resource aliased twice. // Safe action: drop the UUID-suffixed key (canonical wins). Reported // as a rekey, not a conflict. + const canonical = makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"); const state = makeStateFile({ squads: { - foo: { - uuid: "aaaaaaaa-0000-0000-0000-000000000000", - lastPulledHash: "canonical-hash", - }, - "foo-aaaaaaaa": { - uuid: "aaaaaaaa-0000-0000-0000-000000000000", - lastPulledHash: "stale-hash", - }, + foo: canonical, + "foo-aaaaaaaa": makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"), }, }); const report = recanonicalizeStateKeys({ @@ -214,7 +209,7 @@ test("recanonicalize: AUTO-RESOLVES when canonical slug claims the SAME UUID (du // Canonical entry survives unchanged (its metadata is presumed // authoritative — we discard the stale alias, not merge metadata). assert.deepEqual(Object.keys(state.squads), ["foo"]); - assert.equal(state.squads["foo"]!.lastPulledHash, "canonical-hash"); + assert.equal(state.squads["foo"], canonical); }); test("recanonicalize: refuses when canonical local file is missing (would create phantom state mapping)", () => { diff --git a/tests/reconcile-state-key.test.ts b/tests/reconcile-state-key.test.ts index aefca7a..c1953ff 100644 --- a/tests/reconcile-state-key.test.ts +++ b/tests/reconcile-state-key.test.ts @@ -142,6 +142,8 @@ async function runReconcile(opts: RunOpts): Promise { return existing ?? `uuid-${r.resourceId}-created`; }, vapiEnv: "test-env", + formatError: (resourceId, error) => + `${resourceId}: ${error instanceof Error ? error.message : String(error)}`, }); } diff --git a/tests/sim.test.ts b/tests/sim.test.ts index 7414f96..9b6ff5f 100644 --- a/tests/sim.test.ts +++ b/tests/sim.test.ts @@ -8,8 +8,12 @@ import type { StateFile } from "../src/types.ts"; // against `POST /eval/simulation/run` is integration territory and is // covered manually against a sandbox org. -function makeState(overrides: Partial = {}): StateFile { - return { +// Overrides take `name → uuid` for brevity and become the `{ uuid }` state +// shape. +function makeState( + overrides: Partial>> = {}, +): StateFile { + const state: StateFile = { credentials: {}, assistants: {}, structuredOutputs: {}, @@ -20,8 +24,13 @@ function makeState(overrides: Partial = {}): StateFile { simulations: {}, simulationSuites: {}, evals: {}, - ...overrides, }; + for (const [section, entries] of Object.entries(overrides)) { + state[section as keyof StateFile] = Object.fromEntries( + Object.entries(entries).map(([name, uuid]) => [name, { uuid }]), + ); + } + return state; } test("resolveTarget: resolves assistant by local name to UUID", () => { @@ -110,27 +119,12 @@ test("resolveSelection: rejects both suite and simulations simultaneously", () = ); }); -test("resolveTarget: handles forward-compat ResourceState shape (Stack F)", () => { - // Stack F migrates state values from `string` to `{uuid: string, ...}`. - // The resolver must accept both shapes so this stack lands cleanly - // before F or after. - const state = { - credentials: {}, - assistants: { - "future-agent": { - uuid: "uuid-future", - lastPulledHash: "abc123", - } as unknown as string, - }, - structuredOutputs: {}, - tools: {}, - squads: {}, - personalities: {}, - scenarios: {}, - simulations: {}, - simulationSuites: {}, - evals: {}, - } as StateFile; - const target = resolveTarget(state, { assistant: "future-agent" }); - assert.equal(target.id, "uuid-future"); +test("resolveTarget: still accepts a legacy bare-string state value", () => { + // `loadStateFile` reads the state JSON without migrating it, so a legacy + // file can still hold `name → "uuid"`. The resolver accepts both shapes. + const state = makeState(); + (state.assistants as Record)["legacy-agent"] = + "uuid-legacy"; + const target = resolveTarget(state, { assistant: "legacy-agent" }); + assert.equal(target.id, "uuid-legacy"); }); diff --git a/tests/state-merge.test.ts b/tests/state-merge.test.ts index 5cf2f30..f3dd643 100644 --- a/tests/state-merge.test.ts +++ b/tests/state-merge.test.ts @@ -37,46 +37,43 @@ function emptyTouched(): TouchedSets { }; } +// State entries hold only `{ uuid }`, so these tests mark which copy +// `mergeScoped` kept with distinct UUIDs: `-disk` for the on-disk entry, +// `-mem` for the in-memory one. + test("mergeScoped: untouched entries copied from on-disk state", () => { const onDisk = emptyState(); - onDisk.assistants["unrelated-1"] = { uuid: "u-1", lastPulledHash: "h-1" }; - onDisk.assistants["unrelated-2"] = { uuid: "u-2", lastPulledHash: "h-2" }; + onDisk.assistants["unrelated-1"] = { uuid: "u-1-disk" }; + onDisk.assistants["unrelated-2"] = { uuid: "u-2-disk" }; const inMemory = emptyState(); - // In-memory state has unrelated-1 with a different hash (drift) and a - // newly-touched assistant. mergeScoped should copy unrelated-1 from disk - // (untouched), and only take touched-agent from in-memory. - inMemory.assistants["unrelated-1"] = { uuid: "u-1", lastPulledHash: "h-X" }; - inMemory.assistants["touched-agent"] = { - uuid: "u-3", - lastPushedHash: "fresh", - }; + // In-memory state has a drifted unrelated-1 and a newly-touched + // assistant. mergeScoped should copy unrelated-1 from disk (untouched), + // and only take touched-agent from in-memory. + inMemory.assistants["unrelated-1"] = { uuid: "u-1-mem" }; + inMemory.assistants["touched-agent"] = { uuid: "u-3-mem" }; const touched = emptyTouched(); touched.assistants.add("touched-agent"); const merged = mergeScoped(onDisk, inMemory, touched); - assert.equal(merged.assistants["unrelated-1"]!.lastPulledHash, "h-1"); - assert.equal(merged.assistants["unrelated-2"]!.lastPulledHash, "h-2"); - assert.equal(merged.assistants["touched-agent"]!.lastPushedHash, "fresh"); + assert.equal(merged.assistants["unrelated-1"]!.uuid, "u-1-disk"); + assert.equal(merged.assistants["unrelated-2"]!.uuid, "u-2-disk"); + assert.equal(merged.assistants["touched-agent"]!.uuid, "u-3-mem"); }); test("mergeScoped: touched entries take in-memory version", () => { const onDisk = emptyState(); - onDisk.assistants["agent-a"] = { uuid: "u-1", lastPulledHash: "old" }; + onDisk.assistants["agent-a"] = { uuid: "u-1-disk" }; const inMemory = emptyState(); - inMemory.assistants["agent-a"] = { - uuid: "u-1", - lastPulledHash: "old", - lastPushedHash: "new", - }; + inMemory.assistants["agent-a"] = { uuid: "u-1-mem" }; const touched = emptyTouched(); touched.assistants.add("agent-a"); const merged = mergeScoped(onDisk, inMemory, touched); - assert.equal(merged.assistants["agent-a"]!.lastPushedHash, "new"); + assert.equal(merged.assistants["agent-a"]!.uuid, "u-1-mem"); }); test("mergeScoped: credentials always refreshed from in-memory", () => { @@ -111,26 +108,20 @@ test("mergeScoped: empty touched preserves all on-disk state", () => { test("mergeScoped: cross-section isolation (touched assistants do NOT affect tools section)", () => { const onDisk = emptyState(); - onDisk.tools["unrelated-tool"] = { - uuid: "u-tool", - lastPulledHash: "tool-hash", - }; - onDisk.assistants["agent-a"] = { uuid: "u-old" }; + onDisk.tools["unrelated-tool"] = { uuid: "u-tool-disk" }; + onDisk.assistants["agent-a"] = { uuid: "u-agent-disk" }; const inMemory = emptyState(); - inMemory.assistants["agent-a"] = { uuid: "u-old", lastPushedHash: "fresh" }; + inMemory.assistants["agent-a"] = { uuid: "u-agent-mem" }; // In-memory has an unrelated drift in tools section that should NOT bleed in - inMemory.tools["unrelated-tool"] = { - uuid: "u-tool", - lastPulledHash: "drifted", - }; + inMemory.tools["unrelated-tool"] = { uuid: "u-tool-mem" }; const touched = emptyTouched(); touched.assistants.add("agent-a"); // ONLY assistants touched const merged = mergeScoped(onDisk, inMemory, touched); // tools section preserved from disk - assert.equal(merged.tools["unrelated-tool"]!.lastPulledHash, "tool-hash"); + assert.equal(merged.tools["unrelated-tool"]!.uuid, "u-tool-disk"); // assistants section: touched entry takes in-memory - assert.equal(merged.assistants["agent-a"]!.lastPushedHash, "fresh"); + assert.equal(merged.assistants["agent-a"]!.uuid, "u-agent-mem"); }); diff --git a/tests/user-agent.test.ts b/tests/user-agent.test.ts new file mode 100644 index 0000000..5ba18f8 --- /dev/null +++ b/tests/user-agent.test.ts @@ -0,0 +1,57 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; +import test from "node:test"; +import { runSimulation } from "../src/sim.ts"; +import { userAgentGet } from "../src/user-agent.ts"; + +// The User-Agent is how gitops-started simulation runs are counted in the +// platform's analytics (`user_agent` on the run-started event), so its +// format is a contract worth pinning. + +const packageJsonPath = new URL("../package.json", import.meta.url); +const packageVersion = ( + JSON.parse(readFileSync(packageJsonPath, "utf-8")) as { version: string } +).version; + +test("userAgentGet: names the product and the package version", () => { + assert.equal(userAgentGet("sim"), `vapi-gitops-sim/${packageVersion}`); + assert.equal(userAgentGet("check"), `vapi-gitops-check/${packageVersion}`); +}); + +test("runSimulation: sends the sim User-Agent on run create", async () => { + const seen: { method?: string; url?: string; userAgent?: string } = {}; + const server = createServer((req, res) => { + seen.method = req.method; + seen.url = req.url; + seen.userAgent = req.headers["user-agent"]; + res.writeHead(201, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ id: "run-1", status: "queued" })); + }); + await new Promise((resolve) => server.listen(0, resolve)); + const { port } = server.address() as AddressInfo; + const log = console.log; + console.log = () => {}; + try { + await runSimulation( + { + env: "test-org", + token: "test-token", + baseUrl: `http://127.0.0.1:${port}`, + }, + { + entries: [{ type: "simulationSuite", simulationSuiteId: "suite-1" }], + label: "suite test", + }, + { type: "assistant", id: "assistant-1", resourceName: "a" }, + { watch: false }, + ); + } finally { + console.log = log; + await new Promise((resolve) => server.close(() => resolve())); + } + assert.equal(seen.method, "POST"); + assert.equal(seen.url, "/eval/simulation/run"); + assert.equal(seen.userAgent, `vapi-gitops-sim/${packageVersion}`); +}); diff --git a/tests/vapi-ignore-push.test.ts b/tests/vapi-ignore-push.test.ts index 1b768f5..79df1e2 100644 --- a/tests/vapi-ignore-push.test.ts +++ b/tests/vapi-ignore-push.test.ts @@ -644,7 +644,18 @@ test("findOrphanedResources: an explicit file scope cannot delete sibling orphan // for any squad/assistant that references an ignored assistant id. // ───────────────────────────────────────────────────────────────────────────── -function emptyLoaded() { +function emptyLoaded(): Record< + | "tools" + | "structuredOutputs" + | "assistants" + | "squads" + | "personalities" + | "scenarios" + | "simulations" + | "simulationSuites" + | "evals", + { resourceId: string; filePath: string; data: Record }[] +> { return { tools: [], structuredOutputs: [], diff --git a/tsconfig.json b/tsconfig.json index 754f639..dccfd9c 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -23,5 +23,7 @@ "noUnusedLocals": false, "noUnusedParameters": false }, - "include": ["src/**/*"] + // tests/ is included so `npm run build` (and CI) type-checks the tests, + // not only src/. tsx runs tests without type-checking. + "include": ["src/**/*", "tests/**/*"] }