Skip to content

Commit 8a438bc

Browse files
fix(sim): report failed, canceled and incomplete runs instead of a false pass
`npm run sim` read a `results` field the simulation-run API doesn't return and counted `status === "pass"` (items are `passed`/`failed`), so every watched run summarised as 0/0 and exited 0, including failing runs. - src/sim-result.ts: strict, pure verdict. Passed only when the run ended, every expected item exists and passed, and each had a required evaluation that was actually scored; otherwise failed or incomplete with a reason. - src/sim.ts: read run items (paginated or bare array, deduped by id), wait for late item results, print the run link from the create response and the failing judges, cancel the run on --timeout (default 20 min) or Ctrl-C. - src/vapi-client.ts: config-free client; run creation is never retried on a 5xx, since the run may already be queued. - src/sim-cmd.ts: exported simCommandRun; exit 0 passed, 1 failed, 2 usage, 3 incomplete; --no-watch prints the link and exits 0. - Docs: run/item shapes and the verdict in docs/learnings/simulations.md, sim rows in README and AGENTS.md, improvements.md #33. Refs TEST-141 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent aa0d3d7 commit 8a438bc

10 files changed

Lines changed: 1164 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: 120 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,78 @@ 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(`--transport must be "voice" or "chat" (got "${v}")`);
8279
}
80+
parsed.transport = v;
8381
} else if (arg === "--iterations") {
84-
parsed.iterations = parseInt(args[++i] ?? "", 10);
82+
parsed.iterations = Number.parseInt(args[++i] ?? "", 10);
8583
if (Number.isNaN(parsed.iterations)) {
86-
console.error("❌ --iterations requires a number");
87-
process.exit(1);
84+
throw new UsageError("--iterations requires a number");
85+
}
86+
} else if (arg === "--timeout") {
87+
parsed.timeoutMinutes = Number(args[++i]);
88+
if (!Number.isFinite(parsed.timeoutMinutes) || parsed.timeoutMinutes <= 0) {
89+
throw new UsageError("--timeout requires a positive number of minutes");
8890
}
8991
} else if (arg === "--no-watch") parsed.watch = false;
9092
else if (arg === "--watch") parsed.watch = true;
91-
else if (arg === "--help" || arg === "-h") {
92-
printUsage();
93-
process.exit(0);
94-
}
93+
else if (arg === "--help" || arg === "-h") parsed.help = true;
94+
else throw new UsageError(`Unknown argument: ${arg}`);
9595
}
96-
9796
return parsed;
9897
}
9998

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

105118
// Disambiguate --target: if the bare value matches a squad name in state
106119
// and not an assistant, treat it as a squad. Explicit --assistant / --squad
107120
// override the heuristic.
108-
let assistant = args.assistant;
109-
let squad = args.squad;
121+
let assistant = parsed.assistant;
122+
let squad = parsed.squad;
110123
if (assistant && !squad) {
111124
const isSquad =
112125
typeof state.squads[assistant] !== "undefined" &&
@@ -120,39 +133,71 @@ async function main(): Promise<void> {
120133
console.log(
121134
"═══════════════════════════════════════════════════════════════",
122135
);
123-
console.log(`🧪 Vapi GitOps Sim Runner — Environment: ${args.env}`);
136+
console.log(`🧪 Vapi GitOps Sim Runner — Environment: ${parsed.env}`);
124137
console.log(` API: ${cfg.baseUrl}`);
125138
console.log(
126139
"═══════════════════════════════════════════════════════════════\n",
127140
);
128141

129142
const selection = resolveSelection(state, {
130-
suite: args.suite,
131-
simulations: args.simulations,
143+
suite: parsed.suite,
144+
simulations: parsed.simulations,
132145
});
133146
const target = resolveTarget(state, { assistant, squad });
134147

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

141170
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);
171+
if (!summary.verdict) {
172+
console.log("▶️ Run started (not watched).");
173+
return 0;
174+
}
175+
if (summary.verdict.status === "passed") {
176+
console.log("✅ Simulation run passed.");
177+
return 0;
148178
}
149-
console.log("✅ Simulation run passed.");
179+
if (summary.verdict.status === "failed") {
180+
console.error(`❌ Simulation run failed: ${summary.verdict.reason}`);
181+
return 1;
182+
}
183+
console.error(`⚠️ Simulation run incomplete: ${summary.verdict.reason}`);
184+
return 3;
150185
}
151186

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

0 commit comments

Comments
 (0)