Skip to content

Commit e66f1e7

Browse files
fix(sim): report failed, canceled and incomplete runs instead of a false pass (#58)
## Value **V.A.L.U.E. tier:** project — PR 2 of 10 for inline simulation PR checks ([TEST-141](https://linear.app/vapi/issue/TEST-141/gitops-run-simulation-suites-against-pr-changes-inline-as-ci-checks)). Stacked on #57. - **Problem:** `npm run sim` reported every run as passed. It read a `results` field the simulation-run API doesn't return and counted `status === "pass"` (items are `passed`/`failed`), so it always summarised 0/0 and exited 0, failing runs included. - **Who it affects:** anyone gating on `npm run sim`, locally or in CI. The PR check (later in this stack) reuses this verdict, so it has to be right. - **What changes:** - A strict verdict (`src/sim-result.ts`). - Item fetching that handles both response shapes and late results. - The run link printed, plus each failing judge with expected vs extracted values. - `--timeout` and Ctrl-C both cancel the run. - Exit codes: 0 passed, 1 failed, 2 usage, 3 incomplete. - A config-free client that never retries run creation on a 5xx, because the run may already be queued. ## Evidence of value **Same stub API, one passed and one failed item:** | | Result | |---|---| | `main`'s `sim.ts` | `{"pass":0,"fail":0}` → exits 0 (false green) | | This branch | `failed — 1 of 1 simulations failed` → exits 1 | **Live, against a test org (chat transport):** | Suite | Exit | Output | |---|---|---| | Designed to fail (judge: "open 24 hours?") | **1** | `✗ … open-24h (expected = true, got false)` — [run](https://dashboard.vapi.ai/simulations/run/9b933e3c-4ea6-4829-bedc-06bc795e6df3) | | Designed to pass (judge: "open 8–5 on Fridays?") | **0** | `passed — 1 of 1 simulations passed` — [run](https://dashboard.vapi.ai/simulations/run/91bdb9d7-0bf5-4d07-87e3-bda9f5d1391b) | The temporary resources were deleted afterwards. ## Testing plan - `tests/sim-result.test.ts` is a verdict table covering: - the old false-green shape (no `results`); - 0 items, a short item list, and a count mismatch; - a failed item, with the failing judge listed; - canceled items; - all required evaluations skipped, and an optional skip alongside a scored required judge; - missing `itemCounts`, and a run that hasn't ended. - `tests/sim-run.test.ts` runs `runSimulation` against a local HTTP stub: - pass, and fail using the bare-array item shape; - late item results; - timeout cancels the run, and an interrupt cancels it with the 400 "already ended" swallowed; - no retry of a 502 on create; - `--no-watch`; - pagination with overlapping pages deduped. - `npm run build` and `npm test` pass (377 tests). - **Not tested:** a live run that's still `running` when it's canceled (cancel was only exercised against the stub), and voice transport (the live runs used chat). Default behaviour change: unknown CLI arguments are now an error (exit 2) instead of being silently ignored. Refs TEST-141 🤖 Generated with [Claude Code](https://claude.com/claude-code)
2 parents d7b9a34 + 663612e commit e66f1e7

10 files changed

Lines changed: 1282 additions & 160 deletions

File tree

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -902,7 +902,7 @@ npm run promote -- --pipeline <name> --from <source> --to <target> --apply # For
902902
npm run validate -- <org> # Lint resources locally (fails fast on schema drift)
903903
npm run audit -- <org> # Read-only drift detector: orphan YAML, state ghosts, content-identical clusters, sibling base-slugs, dashboard orphans, inline model.tools. Exit 1 on findings.
904904
npm run audit -- <org> --type assistants # Scope audit to a single resource type
905-
npm run sim -- <org> --suite <name> --target <name> # Run a simulation suite against an assistant/squad
905+
npm run sim -- <org> --suite <name> --target <name> # Run a simulation suite against an assistant/squad (exit 0 pass, 1 fail, 3 incomplete; --timeout <min>)
906906
npm run rollback -- <org> --to <ISO-timestamp> # Re-apply a snapshot taken before a push
907907
npm run rollback -- <org> --list # List available snapshots
908908

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ Every command works in two modes:
103103
| `npm run cleanup` | ✅ | `npm run cleanup -- <org> [--force --confirm <org>]` | Inspect (default) or delete orphaned remote resources. Destructive run requires `--confirm <org>`. |
104104
| `npm run rollback` | — | `npm run rollback -- <org> --list` or `--to <ISO>` | Restore from a snapshot in `.vapi-state.<org>.snapshots/` (one is written before every push/apply). |
105105
| `npm run call` | ✅ | `npm run call -- <org> -a <name>` or `-s <squad>` | Start an interactive WebSocket call against an assistant or squad. |
106-
| `npm run sim` | — | `npm run sim -- <org> --suite <name> --target <name>` | Run a simulation suite (or specific simulations) against a deployed assistant/squad. |
106+
| `npm run sim` | — | `npm run sim -- <org> --suite <name> --target <name> [--timeout <min>]` | Run a simulation suite (or specific simulations) against a deployed assistant/squad. Prints the run link; exits 0 passed, 1 failed, 3 incomplete (timeout, Ctrl-C, missing results). |
107107
| `npm run migrate` | — | `npm run migrate` | One-time, all orgs at once: slim legacy state files to pure `name → uuid` and seed the per-developer `.vapi-state-hash/` baseline store from the old hashes. Required once after upgrading to the hash-store engine — `pull`/`push`/`apply` refuse legacy-shaped state until it runs. Idempotent. |
108108
| `npm run build` | — | — | Type-check the codebase (`tsc --noEmit`). |
109109
| `npm test` | — | — | Run regression tests (`node:test`). |

‎docs/learnings/simulations.md‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,23 @@ This is the unified executor. A "run" is a batch — it expands into many `runIt
471471
}
472472
```
473473

474+
The **create** response (`CreateSimulationRunResponse`) also carries two fields nothing else returns:
475+
476+
- `url` — the dashboard link to this run. `GET /eval/simulation/run/:id` does **not** return it, so read it from the create response.
477+
- `simulationRunItemIds` — one id per item queued (simulations × iterations), i.e. how many items the run should end with.
478+
479+
A run has **no `results` field**. Pass/fail lives in `itemCounts` and on the items themselves (`GET …/item`, below). Creating a run queues paid work before the response returns, so a 5xx on create may still have started it — don't blindly retry the POST.
480+
481+
### How `npm run sim` decides pass/fail
482+
483+
`src/sim-result.ts` (`simRunVerdict`) reports **passed** only when all of these hold; otherwise **failed** (any failed item) or **incomplete**:
484+
485+
- the run `ended` and has `itemCounts`, with `total` equal to the number of items created (`simulationRunItemIds`) and greater than 0;
486+
- nothing is `queued`, `running` or `canceled`, and `passed === total`;
487+
- every item was fetched, is `passed`, and has at least one **required** evaluation that wasn't skipped (the all-skipped trap from the chat-mode gotcha above).
488+
489+
Exit codes: 0 passed, 1 failed, 2 usage error, 3 incomplete (timeout, Ctrl-C, canceled items, results that never arrived). On timeout or Ctrl-C the run is canceled.
490+
474491
### List runs — `GET /eval/simulation/run`
475492

476493
Query params:
@@ -486,7 +503,7 @@ Query params:
486503
### Get / Cancel run
487504

488505
- `GET /eval/simulation/run/:id` → `SimulationRun`
489-
- `PATCH /eval/simulation/run/:id` → cancels the run **and** all its queued items. No body required.
506+
- `PATCH /eval/simulation/run/:id` → cancels the run **and** all its queued items. No body required. Returns 400 `Run has already ended` for an ended run and 409 when a concurrent cancel wins; both mean "nothing left to cancel".
490507

491508
---
492509

@@ -498,6 +515,8 @@ Run items are system-managed — there's no create/update API for users; they're
498515

499516
Query params: `limit`, `page`, `simulationId`, `runId`, `status` (`queued` | `running` | `evaluating` | `passed` | `failed` | `canceled`).
500517

518+
Response shape depends on the query: **with** `limit` or `page` it's paginated (`{ results, metadata: { totalItems, itemsPerPage, currentPage } }`, `limit` up to 1000); **without** them it's a bare array. Pages are ordered only by creation time and a run's items share it, so OFFSET pages can overlap — dedupe by `id`. Items can also lag the run: a run can be `ended` while an item is `passed` but its `results` aren't written yet.
519+
501520
### Get a run item — `GET /eval/simulation/run/:id/item/:itemId`
502521

503522
**Response** — `SimulationRunItem` is rich; the highlights:

‎improvements.md‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ you which stack PR closes the row.**
8484
| 30 | Tool-linking pass could PATCH a raw assistant slug | Mid-push 400 naming the wrong resource | None | RESOLVED 2026-08-03 (#51) |
8585
| 31 | Unresolved references handled 3 inconsistent ways, no dangling-ref check | Same authoring mistake, three different failure modes | None | Open |
8686
| 32 | Test suite never ran in CI; 20 tests rotted after the hash store | Regression guards for #22/#23 silently stopped running | None | RESOLVED 2026-09-30 (#56) |
87+
| 33 | `npm run sim` reported every run as passed | A failing suite exited 0 — false green | None | RESOLVED 2026-10-01 |
8788

8889
**Active backlog after cleanup:** `#2`, `#6`, `#8`, `#12`, `#20`, `#24–#26`, `#31`, and the open remainder of `#27` (wiring the listing-completeness verdict into push/delete/audit, and moving `cleanup.ts` onto the shared pager). Resolved entries stay in this file as historical incident notes per the maintenance directive; stale superseded backlog rows are not duplicated.
8990

@@ -1721,6 +1722,57 @@ fields.
17211722

17221723
---
17231724

1725+
## 33. `npm run sim` reported every run as passed
1726+
1727+
**[RESOLVED 2026-10-01]**
1728+
1729+
**Discovered:** 2026-10-01, while designing simulation PR checks (TEST-141).
1730+
1731+
### Problem
1732+
1733+
`npm run sim` scored a run by reading `results[]` on the run and counting
1734+
`status === "pass"`. The simulation-run API has no `results` field and
1735+
item statuses are `passed` / `failed`, so every watched run summarised
1736+
as 0 pass / 0 fail and the command exited 0 — including runs whose
1737+
simulations failed.
1738+
1739+
### Current behavior (Verified, before the fix)
1740+
1741+
- `src/sim.ts` `runSimulation` computed pass/fail from `last.results`,
1742+
which `GET /eval/simulation/run/:id` never returns; pass/fail is in
1743+
`itemCounts` and on the run items (`GET /eval/simulation/run/:id/item`).
1744+
- `src/sim-cmd.ts` exited 1 only when `fail > 0`, which could never happen.
1745+
- Polling also stopped on statuses that don't exist (`failed`,
1746+
`completed`), never canceled a timed-out run, and didn't print the run
1747+
link (only the create response carries `url`).
1748+
1749+
### Risk
1750+
1751+
Anyone gating on `npm run sim` (locally or in CI) got a green result for a
1752+
failing suite.
1753+
1754+
### Current mitigation
1755+
1756+
None needed once the fix below lands.
1757+
1758+
### Possible fix (landed)
1759+
1760+
- `src/sim-result.ts` `simRunVerdict`: passed only when the run ended,
1761+
every expected item exists, passed, and had a required evaluation that was
1762+
actually scored; otherwise failed or incomplete with a reason.
1763+
- `src/sim.ts` reads items (paginated or bare-array, deduped by id),
1764+
waits for late item results, prints the run link and failing judges, and
1765+
cancels the run on `--timeout` (default 20 min) or Ctrl-C.
1766+
- `src/vapi-client.ts`: a config-free client that never retries run
1767+
creation on a 5xx (the run may already be queued).
1768+
- Exit codes: 0 passed, 1 failed, 2 usage, 3 incomplete.
1769+
1770+
### Status
1771+
1772+
**RESOLVED 2026-10-01.**
1773+
1774+
---
1775+
17241776
## Out of scope (intentionally not improvements)
17251777

17261778
- **State file is identity-only and not git-ignored.** It's intentionally

‎src/sim-cmd.ts‎

Lines changed: 125 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@
22
//
33
// Wraps `POST /eval/simulation/run` (the simulations API). See AGENTS.md for
44
// usage.
5+
//
6+
// Exit codes: 0 passed, 1 failed, 2 usage/config error, 3 incomplete
7+
// (timed out, interrupted, canceled, or results that never fully arrived).
58

9+
import { resolve } from "node:path";
10+
import { fileURLToPath } from "node:url";
611
import {
712
formatSummary,
813
loadEnvFile,
@@ -12,27 +17,28 @@ import {
1217
runSimulation,
1318
} from "./sim.ts";
1419

15-
function printUsage(): void {
16-
console.error(
17-
[
18-
"Usage:",
19-
" npm run sim -- <org> --suite <suite-name> --target <assistant-or-squad-name>",
20-
" npm run sim -- <org> --simulations <name1>,<name2> --target <assistant-name>",
21-
"",
22-
"Options:",
23-
" --suite <name> Run an entire simulation suite by local resource name",
24-
" --simulations <list> Run one or more simulations by comma-separated local names",
25-
" --target <name> Local assistant or squad name (resolves to UUID via state)",
26-
" --transport voice|chat Transport (default: voice; chat is faster/cheaper)",
27-
" --iterations N Override default iteration count",
28-
" --watch Tail status until completion (default: on)",
29-
"",
30-
"Examples:",
31-
" npm run sim -- my-org --suite booking-tests --target intake-agent",
32-
" npm run sim -- my-org --simulations happy-path,edge-case --target main-agent --transport chat",
33-
].join("\n"),
34-
);
35-
}
20+
const USAGE = [
21+
"Usage:",
22+
" npm run sim -- <org> --suite <suite-name> --target <assistant-or-squad-name>",
23+
" npm run sim -- <org> --simulations <name1>,<name2> --target <assistant-name>",
24+
"",
25+
"Options:",
26+
" --suite <name> Run an entire simulation suite by local resource name",
27+
" --simulations <list> Run one or more simulations by comma-separated local names",
28+
" --target <name> Local assistant or squad name (resolves to UUID via state)",
29+
" --transport voice|chat Transport (default: voice; chat is faster/cheaper)",
30+
" --iterations N Override default iteration count",
31+
" --timeout <minutes> Give up and cancel the run after this long (default: 20)",
32+
" --no-watch Start the run, print its link, and exit 0 without a verdict",
33+
"",
34+
"Exit codes: 0 passed, 1 failed, 2 usage error, 3 incomplete (timeout, interrupt, missing results)",
35+
"",
36+
"Examples:",
37+
" npm run sim -- my-org --suite booking-tests --target intake-agent",
38+
" npm run sim -- my-org --simulations happy-path,edge-case --target main-agent --transport chat",
39+
].join("\n");
40+
41+
class UsageError extends Error {}
3642

3743
interface ParsedArgs {
3844
env: string;
@@ -42,71 +48,83 @@ interface ParsedArgs {
4248
squad?: string;
4349
transport?: "voice" | "chat";
4450
iterations?: number;
51+
timeoutMinutes?: number;
4552
watch: boolean;
53+
help: boolean;
4654
}
4755

48-
function parseArgs(): ParsedArgs {
49-
const args = process.argv.slice(2);
56+
function argsParse(args: string[]): ParsedArgs {
5057
const env = args[0];
51-
if (!env) {
52-
printUsage();
53-
process.exit(1);
58+
if (!env || env === "--help" || env === "-h") {
59+
return { env: env ?? "", watch: true, help: true };
5460
}
5561
const SLUG_RE = /^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/;
56-
if (!SLUG_RE.test(env)) {
57-
console.error(`❌ Invalid org name: ${env}`);
58-
process.exit(1);
59-
}
60-
61-
const parsed: ParsedArgs = { env, watch: true };
62+
if (!SLUG_RE.test(env)) throw new UsageError(`Invalid org name: ${env}`);
6263

64+
const parsed: ParsedArgs = { env, watch: true, help: false };
6365
for (let i = 1; i < args.length; i++) {
6466
const arg = args[i];
6567
if (arg === "--suite") parsed.suite = args[++i];
6668
else if (arg === "--simulations") parsed.simulations = args[++i];
6769
else if (arg === "--target") {
6870
// We don't know yet whether target is an assistant or squad — defer
69-
// resolution to the state lookup. Try assistant first; resolveTarget()
70-
// accepts either argument key, so we set the candidate in `assistant`
71-
// and let `resolveTarget` fall through to `squad` if not found.
72-
// For clarity, we accept --assistant / --squad as explicit alternatives.
71+
// resolution to the state lookup (see simCommandRun).
7372
parsed.assistant = args[++i];
7473
} else if (arg === "--assistant") parsed.assistant = args[++i];
7574
else if (arg === "--squad") parsed.squad = args[++i];
7675
else if (arg === "--transport") {
7776
const v = args[++i];
78-
if (v === "voice" || v === "chat") parsed.transport = v;
79-
else {
80-
console.error(`❌ --transport must be "voice" or "chat" (got "${v}")`);
81-
process.exit(1);
77+
if (v !== "voice" && v !== "chat") {
78+
throw new UsageError(
79+
`--transport must be "voice" or "chat" (got "${v}")`,
80+
);
8281
}
82+
parsed.transport = v;
8383
} else if (arg === "--iterations") {
84-
parsed.iterations = parseInt(args[++i] ?? "", 10);
84+
parsed.iterations = Number.parseInt(args[++i] ?? "", 10);
8585
if (Number.isNaN(parsed.iterations)) {
86-
console.error("❌ --iterations requires a number");
87-
process.exit(1);
86+
throw new UsageError("--iterations requires a number");
87+
}
88+
} else if (arg === "--timeout") {
89+
parsed.timeoutMinutes = Number(args[++i]);
90+
if (
91+
!Number.isFinite(parsed.timeoutMinutes) ||
92+
parsed.timeoutMinutes <= 0
93+
) {
94+
throw new UsageError("--timeout requires a positive number of minutes");
8895
}
8996
} else if (arg === "--no-watch") parsed.watch = false;
9097
else if (arg === "--watch") parsed.watch = true;
91-
else if (arg === "--help" || arg === "-h") {
92-
printUsage();
93-
process.exit(0);
94-
}
98+
else if (arg === "--help" || arg === "-h") parsed.help = true;
99+
else throw new UsageError(`Unknown argument: ${arg}`);
95100
}
96-
97101
return parsed;
98102
}
99103

100-
async function main(): Promise<void> {
101-
const args = parseArgs();
102-
const cfg = loadEnvFile(args.env);
103-
const state = loadStateFile(args.env);
104+
export async function simCommandRun(
105+
args = process.argv.slice(2),
106+
): Promise<number> {
107+
let parsed: ParsedArgs;
108+
try {
109+
parsed = argsParse(args);
110+
} catch (error) {
111+
if (!(error instanceof UsageError)) throw error;
112+
console.error(`❌ ${error.message}\n\n${USAGE}`);
113+
return 2;
114+
}
115+
if (parsed.help) {
116+
console.log(USAGE);
117+
return parsed.env ? 0 : 2;
118+
}
119+
120+
const cfg = loadEnvFile(parsed.env);
121+
const state = loadStateFile(parsed.env);
104122

105123
// Disambiguate --target: if the bare value matches a squad name in state
106124
// and not an assistant, treat it as a squad. Explicit --assistant / --squad
107125
// override the heuristic.
108-
let assistant = args.assistant;
109-
let squad = args.squad;
126+
let assistant = parsed.assistant;
127+
let squad = parsed.squad;
110128
if (assistant && !squad) {
111129
const isSquad =
112130
typeof state.squads[assistant] !== "undefined" &&
@@ -120,39 +138,71 @@ async function main(): Promise<void> {
120138
console.log(
121139
"═══════════════════════════════════════════════════════════════",
122140
);
123-
console.log(`🧪 Vapi GitOps Sim Runner — Environment: ${args.env}`);
141+
console.log(`🧪 Vapi GitOps Sim Runner — Environment: ${parsed.env}`);
124142
console.log(` API: ${cfg.baseUrl}`);
125143
console.log(
126144
"═══════════════════════════════════════════════════════════════\n",
127145
);
128146

129147
const selection = resolveSelection(state, {
130-
suite: args.suite,
131-
simulations: args.simulations,
148+
suite: parsed.suite,
149+
simulations: parsed.simulations,
132150
});
133151
const target = resolveTarget(state, { assistant, squad });
134152

135-
const summary = await runSimulation(cfg, selection, target, {
136-
watch: args.watch,
137-
iterations: args.iterations,
138-
transport: args.transport,
139-
});
153+
// Ctrl-C / SIGTERM cancel the run instead of leaving it running unwatched.
154+
const controller = new AbortController();
155+
const stop = () => controller.abort();
156+
process.on("SIGINT", stop);
157+
process.on("SIGTERM", stop);
158+
let summary;
159+
try {
160+
summary = await runSimulation(cfg, selection, target, {
161+
watch: parsed.watch,
162+
iterations: parsed.iterations,
163+
transport: parsed.transport,
164+
timeoutMs:
165+
parsed.timeoutMinutes === undefined
166+
? undefined
167+
: parsed.timeoutMinutes * 60_000,
168+
signal: controller.signal,
169+
});
170+
} finally {
171+
process.off("SIGINT", stop);
172+
process.off("SIGTERM", stop);
173+
}
140174

141175
console.log(`\n${formatSummary(summary)}\n`);
142-
143-
if (summary.fail > 0) {
144-
console.error(
145-
`❌ Simulation run failed (${summary.fail} fail / ${summary.pass} pass)`,
146-
);
147-
process.exit(1);
176+
if (!summary.verdict) {
177+
console.log("▶️ Run started (not watched).");
178+
return 0;
179+
}
180+
if (summary.verdict.status === "passed") {
181+
console.log("✅ Simulation run passed.");
182+
return 0;
148183
}
149-
console.log("✅ Simulation run passed.");
184+
if (summary.verdict.status === "failed") {
185+
console.error(`❌ Simulation run failed: ${summary.verdict.reason}`);
186+
return 1;
187+
}
188+
console.error(`⚠️ Simulation run incomplete: ${summary.verdict.reason}`);
189+
return 3;
150190
}
151191

152-
main().catch((error) => {
153-
console.error(
154-
"\n❌ Sim failed:",
155-
error instanceof Error ? error.message : error,
192+
const isMainModule =
193+
process.argv[1] !== undefined &&
194+
resolve(process.argv[1]) === fileURLToPath(import.meta.url);
195+
if (isMainModule) {
196+
simCommandRun().then(
197+
(code) => {
198+
process.exitCode = code;
199+
},
200+
(error: unknown) => {
201+
console.error(
202+
"\n❌ Sim failed:",
203+
error instanceof Error ? error.message : error,
204+
);
205+
process.exitCode = 2;
206+
},
156207
);
157-
process.exit(1);
158-
});
208+
}

0 commit comments

Comments
 (0)