Skip to content

Commit aa0d3d7

Browse files
ci: type-check tests/ and send a User-Agent on sim API requests
- tsconfig.json now includes tests/, so `npm run build` (already run by CI on every PR) type-checks the tests. tsx runs them without type-checking, which is how 37 type errors accumulated unnoticed. - Fix those 37 errors. All were fixture drift after the hash-store migration (state entries carrying lastPulledHash/lastPushedHash or bare-string values, an untyped emptyLoaded fixture), plus one real gap: the reconcile-state-key harness never passed the required formatError, so any test reaching that error path would have thrown. - New src/user-agent.ts; `npm run sim` sends `User-Agent: vapi-gitops-sim/<version>`, so gitops-started simulation runs can be counted in the platform's run-started analytics. Refs TEST-141 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 69c7e83 commit aa0d3d7

11 files changed

Lines changed: 181 additions & 87 deletions

‎improvements.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1709,6 +1709,16 @@ so `npm run build` never typechecks them and the compiler could not have
17091709
caught these stale fixture shapes. Including them surfaces 37 existing type
17101710
errors today; widening `include` is a separate change.
17111711

1712+
**Follow-up 2026-10-01:** `tsconfig.json` now includes `tests/`, so
1713+
`npm run build` and CI type-check the tests too. The 37 errors were all
1714+
fixture drift: state entries still carrying `lastPulledHash` /
1715+
`lastPushedHash` or bare-string values, an untyped `emptyLoaded()` fixture,
1716+
and one real gap — the `reconcile-state-key` harness never passed the
1717+
required `formatError`, so any test reaching that error path would have
1718+
thrown a `TypeError` instead of exercising it. Fixtures now use the `{ uuid }`
1719+
shape and mark "which copy won" with distinct UUIDs instead of removed hash
1720+
fields.
1721+
17121722
---
17131723

17141724
## Out of scope (intentionally not improvements)

‎src/sim.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { existsSync, readFileSync } from "fs";
99
import { dirname, join } from "path";
1010
import { fileURLToPath } from "url";
1111
import type { StateFile } from "./types.ts";
12+
import { userAgentGet } from "./user-agent.ts";
1213

1314
const __dirname = dirname(fileURLToPath(import.meta.url));
1415
const BASE_DIR = join(__dirname, "..");
@@ -219,6 +220,7 @@ async function fetchJson(
219220
headers: {
220221
Authorization: `Bearer ${cfg.token}`,
221222
"Content-Type": "application/json",
223+
"User-Agent": userAgentGet("sim"),
222224
},
223225
...(body ? { body: JSON.stringify(body) } : {}),
224226
});

‎src/user-agent.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// User-Agent for the API requests this tool makes, so simulation runs started
2+
// from gitops can be told apart in the platform's analytics.
3+
//
4+
// Config-free on purpose (like api-key.ts): importing config.ts would parse
5+
// argv and exit, which breaks importing this from sim.ts and tests.
6+
7+
import { readFileSync } from "node:fs";
8+
import { dirname, join } from "node:path";
9+
import { fileURLToPath } from "node:url";
10+
11+
const PACKAGE_JSON_PATH = join(
12+
dirname(fileURLToPath(import.meta.url)),
13+
"..",
14+
"package.json",
15+
);
16+
17+
function packageVersionRead(): string {
18+
try {
19+
const parsed: unknown = JSON.parse(
20+
readFileSync(PACKAGE_JSON_PATH, "utf-8"),
21+
);
22+
if (
23+
parsed &&
24+
typeof parsed === "object" &&
25+
"version" in parsed &&
26+
typeof parsed.version === "string"
27+
) {
28+
return parsed.version;
29+
}
30+
} catch {
31+
// Fall through: a missing or unreadable package.json must never block
32+
// an API request.
33+
}
34+
return "unknown";
35+
}
36+
37+
export function userAgentGet(product: "sim" | "check"): string {
38+
return `vapi-gitops-${product}/${packageVersionRead()}`;
39+
}

‎tests/credentials.test.ts‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import assert from "node:assert/strict";
22
import test from "node:test";
3-
import { replaceCredentialRefs } from "../src/credentials.ts";
3+
import {
4+
credentialForwardMap as forwardMap,
5+
credentialReverseMap as reverseMap,
6+
replaceCredentialRefs,
7+
} from "../src/credentials.ts";
48
import type { StateFile } from "../src/types.ts";
59

610
// Regression tests for P0-1.
@@ -14,9 +18,12 @@ import type { StateFile } from "../src/types.ts";
1418
// API then rejects on POST/PATCH. These tests lock in the scoped semantics
1519
// (only swap at exactly `credentialId` / `credentialIds` keys).
1620

21+
// Takes `name → uuid` for brevity and builds the `{ uuid }` state shape.
1722
function makeState(creds: Record<string, string>): StateFile {
1823
return {
19-
credentials: creds,
24+
credentials: Object.fromEntries(
25+
Object.entries(creds).map(([name, uuid]) => [name, { uuid }]),
26+
),
2027
assistants: {},
2128
structuredOutputs: {},
2229
tools: {},
@@ -29,22 +36,6 @@ function makeState(creds: Record<string, string>): StateFile {
2936
};
3037
}
3138

32-
function reverseMap(state: StateFile): Map<string, string> {
33-
const m = new Map<string, string>();
34-
for (const [name, uuid] of Object.entries(state.credentials)) {
35-
m.set(uuid, name);
36-
}
37-
return m;
38-
}
39-
40-
function forwardMap(state: StateFile): Map<string, string> {
41-
const m = new Map<string, string>();
42-
for (const [name, uuid] of Object.entries(state.credentials)) {
43-
m.set(name, uuid);
44-
}
45-
return m;
46-
}
47-
4839
test("replaceCredentialRefs swaps at credentialId keys", () => {
4940
const state = makeState({
5041
"roofr-server-credential": "11111111-1111-1111-1111-111111111111",

‎tests/recanonicalize.test.ts‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -189,16 +189,11 @@ test("recanonicalize: AUTO-RESOLVES when canonical slug claims the SAME UUID (du
189189
// SAME uuid_A. This is not a twin — it's one resource aliased twice.
190190
// Safe action: drop the UUID-suffixed key (canonical wins). Reported
191191
// as a rekey, not a conflict.
192+
const canonical = makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000");
192193
const state = makeStateFile({
193194
squads: {
194-
foo: {
195-
uuid: "aaaaaaaa-0000-0000-0000-000000000000",
196-
lastPulledHash: "canonical-hash",
197-
},
198-
"foo-aaaaaaaa": {
199-
uuid: "aaaaaaaa-0000-0000-0000-000000000000",
200-
lastPulledHash: "stale-hash",
201-
},
195+
foo: canonical,
196+
"foo-aaaaaaaa": makeStateEntry("aaaaaaaa-0000-0000-0000-000000000000"),
202197
},
203198
});
204199
const report = recanonicalizeStateKeys({
@@ -214,7 +209,7 @@ test("recanonicalize: AUTO-RESOLVES when canonical slug claims the SAME UUID (du
214209
// Canonical entry survives unchanged (its metadata is presumed
215210
// authoritative — we discard the stale alias, not merge metadata).
216211
assert.deepEqual(Object.keys(state.squads), ["foo"]);
217-
assert.equal(state.squads["foo"]!.lastPulledHash, "canonical-hash");
212+
assert.equal(state.squads["foo"], canonical);
218213
});
219214

220215
test("recanonicalize: refuses when canonical local file is missing (would create phantom state mapping)", () => {

‎tests/reconcile-state-key.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,8 @@ async function runReconcile(opts: RunOpts): Promise<void> {
142142
return existing ?? `uuid-${r.resourceId}-created`;
143143
},
144144
vapiEnv: "test-env",
145+
formatError: (resourceId, error) =>
146+
`${resourceId}: ${error instanceof Error ? error.message : String(error)}`,
145147
});
146148
}
147149

‎tests/sim.test.ts‎

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,12 @@ import type { StateFile } from "../src/types.ts";
88
// against `POST /eval/simulation/run` is integration territory and is
99
// covered manually against a sandbox org.
1010

11-
function makeState(overrides: Partial<StateFile> = {}): StateFile {
12-
return {
11+
// Overrides take `name → uuid` for brevity and become the `{ uuid }` state
12+
// shape.
13+
function makeState(
14+
overrides: Partial<Record<keyof StateFile, Record<string, string>>> = {},
15+
): StateFile {
16+
const state: StateFile = {
1317
credentials: {},
1418
assistants: {},
1519
structuredOutputs: {},
@@ -20,8 +24,13 @@ function makeState(overrides: Partial<StateFile> = {}): StateFile {
2024
simulations: {},
2125
simulationSuites: {},
2226
evals: {},
23-
...overrides,
2427
};
28+
for (const [section, entries] of Object.entries(overrides)) {
29+
state[section as keyof StateFile] = Object.fromEntries(
30+
Object.entries(entries).map(([name, uuid]) => [name, { uuid }]),
31+
);
32+
}
33+
return state;
2534
}
2635

2736
test("resolveTarget: resolves assistant by local name to UUID", () => {
@@ -110,27 +119,12 @@ test("resolveSelection: rejects both suite and simulations simultaneously", () =
110119
);
111120
});
112121

113-
test("resolveTarget: handles forward-compat ResourceState shape (Stack F)", () => {
114-
// Stack F migrates state values from `string` to `{uuid: string, ...}`.
115-
// The resolver must accept both shapes so this stack lands cleanly
116-
// before F or after.
117-
const state = {
118-
credentials: {},
119-
assistants: {
120-
"future-agent": {
121-
uuid: "uuid-future",
122-
lastPulledHash: "abc123",
123-
} as unknown as string,
124-
},
125-
structuredOutputs: {},
126-
tools: {},
127-
squads: {},
128-
personalities: {},
129-
scenarios: {},
130-
simulations: {},
131-
simulationSuites: {},
132-
evals: {},
133-
} as StateFile;
134-
const target = resolveTarget(state, { assistant: "future-agent" });
135-
assert.equal(target.id, "uuid-future");
122+
test("resolveTarget: still accepts a legacy bare-string state value", () => {
123+
// `loadStateFile` reads the state JSON without migrating it, so a legacy
124+
// file can still hold `name → "uuid"`. The resolver accepts both shapes.
125+
const state = makeState();
126+
(state.assistants as Record<string, unknown>)["legacy-agent"] =
127+
"uuid-legacy";
128+
const target = resolveTarget(state, { assistant: "legacy-agent" });
129+
assert.equal(target.id, "uuid-legacy");
136130
});

‎tests/state-merge.test.ts‎

Lines changed: 23 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -37,46 +37,43 @@ function emptyTouched(): TouchedSets {
3737
};
3838
}
3939

40+
// State entries hold only `{ uuid }`, so these tests mark which copy
41+
// `mergeScoped` kept with distinct UUIDs: `-disk` for the on-disk entry,
42+
// `-mem` for the in-memory one.
43+
4044
test("mergeScoped: untouched entries copied from on-disk state", () => {
4145
const onDisk = emptyState();
42-
onDisk.assistants["unrelated-1"] = { uuid: "u-1", lastPulledHash: "h-1" };
43-
onDisk.assistants["unrelated-2"] = { uuid: "u-2", lastPulledHash: "h-2" };
46+
onDisk.assistants["unrelated-1"] = { uuid: "u-1-disk" };
47+
onDisk.assistants["unrelated-2"] = { uuid: "u-2-disk" };
4448

4549
const inMemory = emptyState();
46-
// In-memory state has unrelated-1 with a different hash (drift) and a
47-
// newly-touched assistant. mergeScoped should copy unrelated-1 from disk
48-
// (untouched), and only take touched-agent from in-memory.
49-
inMemory.assistants["unrelated-1"] = { uuid: "u-1", lastPulledHash: "h-X" };
50-
inMemory.assistants["touched-agent"] = {
51-
uuid: "u-3",
52-
lastPushedHash: "fresh",
53-
};
50+
// In-memory state has a drifted unrelated-1 and a newly-touched
51+
// assistant. mergeScoped should copy unrelated-1 from disk (untouched),
52+
// and only take touched-agent from in-memory.
53+
inMemory.assistants["unrelated-1"] = { uuid: "u-1-mem" };
54+
inMemory.assistants["touched-agent"] = { uuid: "u-3-mem" };
5455

5556
const touched = emptyTouched();
5657
touched.assistants.add("touched-agent");
5758

5859
const merged = mergeScoped(onDisk, inMemory, touched);
59-
assert.equal(merged.assistants["unrelated-1"]!.lastPulledHash, "h-1");
60-
assert.equal(merged.assistants["unrelated-2"]!.lastPulledHash, "h-2");
61-
assert.equal(merged.assistants["touched-agent"]!.lastPushedHash, "fresh");
60+
assert.equal(merged.assistants["unrelated-1"]!.uuid, "u-1-disk");
61+
assert.equal(merged.assistants["unrelated-2"]!.uuid, "u-2-disk");
62+
assert.equal(merged.assistants["touched-agent"]!.uuid, "u-3-mem");
6263
});
6364

6465
test("mergeScoped: touched entries take in-memory version", () => {
6566
const onDisk = emptyState();
66-
onDisk.assistants["agent-a"] = { uuid: "u-1", lastPulledHash: "old" };
67+
onDisk.assistants["agent-a"] = { uuid: "u-1-disk" };
6768

6869
const inMemory = emptyState();
69-
inMemory.assistants["agent-a"] = {
70-
uuid: "u-1",
71-
lastPulledHash: "old",
72-
lastPushedHash: "new",
73-
};
70+
inMemory.assistants["agent-a"] = { uuid: "u-1-mem" };
7471

7572
const touched = emptyTouched();
7673
touched.assistants.add("agent-a");
7774

7875
const merged = mergeScoped(onDisk, inMemory, touched);
79-
assert.equal(merged.assistants["agent-a"]!.lastPushedHash, "new");
76+
assert.equal(merged.assistants["agent-a"]!.uuid, "u-1-mem");
8077
});
8178

8279
test("mergeScoped: credentials always refreshed from in-memory", () => {
@@ -111,26 +108,20 @@ test("mergeScoped: empty touched preserves all on-disk state", () => {
111108

112109
test("mergeScoped: cross-section isolation (touched assistants do NOT affect tools section)", () => {
113110
const onDisk = emptyState();
114-
onDisk.tools["unrelated-tool"] = {
115-
uuid: "u-tool",
116-
lastPulledHash: "tool-hash",
117-
};
118-
onDisk.assistants["agent-a"] = { uuid: "u-old" };
111+
onDisk.tools["unrelated-tool"] = { uuid: "u-tool-disk" };
112+
onDisk.assistants["agent-a"] = { uuid: "u-agent-disk" };
119113

120114
const inMemory = emptyState();
121-
inMemory.assistants["agent-a"] = { uuid: "u-old", lastPushedHash: "fresh" };
115+
inMemory.assistants["agent-a"] = { uuid: "u-agent-mem" };
122116
// In-memory has an unrelated drift in tools section that should NOT bleed in
123-
inMemory.tools["unrelated-tool"] = {
124-
uuid: "u-tool",
125-
lastPulledHash: "drifted",
126-
};
117+
inMemory.tools["unrelated-tool"] = { uuid: "u-tool-mem" };
127118

128119
const touched = emptyTouched();
129120
touched.assistants.add("agent-a"); // ONLY assistants touched
130121

131122
const merged = mergeScoped(onDisk, inMemory, touched);
132123
// tools section preserved from disk
133-
assert.equal(merged.tools["unrelated-tool"]!.lastPulledHash, "tool-hash");
124+
assert.equal(merged.tools["unrelated-tool"]!.uuid, "u-tool-disk");
134125
// assistants section: touched entry takes in-memory
135-
assert.equal(merged.assistants["agent-a"]!.lastPushedHash, "fresh");
126+
assert.equal(merged.assistants["agent-a"]!.uuid, "u-agent-mem");
136127
});

‎tests/user-agent.test.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import assert from "node:assert/strict";
2+
import { readFileSync } from "node:fs";
3+
import { createServer } from "node:http";
4+
import type { AddressInfo } from "node:net";
5+
import test from "node:test";
6+
import { runSimulation } from "../src/sim.ts";
7+
import { userAgentGet } from "../src/user-agent.ts";
8+
9+
// The User-Agent is how gitops-started simulation runs are counted in the
10+
// platform's analytics (`user_agent` on the run-started event), so its
11+
// format is a contract worth pinning.
12+
13+
const packageJsonPath = new URL("../package.json", import.meta.url);
14+
const packageVersion = (
15+
JSON.parse(readFileSync(packageJsonPath, "utf-8")) as { version: string }
16+
).version;
17+
18+
test("userAgentGet: names the product and the package version", () => {
19+
assert.equal(userAgentGet("sim"), `vapi-gitops-sim/${packageVersion}`);
20+
assert.equal(userAgentGet("check"), `vapi-gitops-check/${packageVersion}`);
21+
});
22+
23+
test("runSimulation: sends the sim User-Agent on run create", async () => {
24+
const seen: { method?: string; url?: string; userAgent?: string } = {};
25+
const server = createServer((req, res) => {
26+
seen.method = req.method;
27+
seen.url = req.url;
28+
seen.userAgent = req.headers["user-agent"];
29+
res.writeHead(201, { "Content-Type": "application/json" });
30+
res.end(JSON.stringify({ id: "run-1", status: "queued" }));
31+
});
32+
await new Promise<void>((resolve) => server.listen(0, resolve));
33+
const { port } = server.address() as AddressInfo;
34+
const log = console.log;
35+
console.log = () => {};
36+
try {
37+
await runSimulation(
38+
{
39+
env: "test-org",
40+
token: "test-token",
41+
baseUrl: `http://127.0.0.1:${port}`,
42+
},
43+
{
44+
entries: [{ type: "simulationSuite", simulationSuiteId: "suite-1" }],
45+
label: "suite test",
46+
},
47+
{ type: "assistant", id: "assistant-1", resourceName: "a" },
48+
{ watch: false },
49+
);
50+
} finally {
51+
console.log = log;
52+
await new Promise<void>((resolve) => server.close(() => resolve()));
53+
}
54+
assert.equal(seen.method, "POST");
55+
assert.equal(seen.url, "/eval/simulation/run");
56+
assert.equal(seen.userAgent, `vapi-gitops-sim/${packageVersion}`);
57+
});

0 commit comments

Comments
 (0)