From 1f982af168107eb6a05b59654d50188921bd23bd Mon Sep 17 00:00:00 2001 From: Scott Lowe Date: Sat, 3 Oct 2026 00:43:37 -0700 Subject: [PATCH] feat(validate): catch broken references and show findings on the PR A reference that names no file and no state entry failed three different ways depending on the field: silently dropped (toolIds, structuredOutputIds), sent raw and rejected mid-push (squad members, hook tools, personalityId, scenarioId), or deferred (improvements.md #31). validate never checked it, so the new CI check couldn't either. - src/validate-refs.ts, run by validate (so by apply and CI) and by push: - dangling-reference (error): a name with no local file and no state entry, across the shared reference walk plus scenario judges' evaluations[].structuredOutputId; - override-tool-by-name (error): toolIds names inside assistantOverrides, membersOverrides or targetOverrides, which push never resolves; - unresolved-credential (warning): a credential name not in state, naming the org's bootstrap pull; - reference-by-uuid (warning): breaks promotion; stock personalities exempt. - validate now also runs reference-to-ignored, as push already did, and reads the committed state file offline. - On GitHub Actions, validate prints each finding as an annotation, so it shows on the file in the PR, warnings included. - The validate header no longer prints an API URL for an offline command. - Docs: a rule table in troubleshooting, the commands row, AGENTS.md (never edit state to make a reference resolve), improvements.md #31 resolved. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 3 +- docs/guides/commands.md | 2 +- docs/guides/troubleshooting.md | 29 +++- improvements.md | 21 ++- src/push.ts | 7 + src/state.ts | 2 +- src/validate-cmd.ts | 47 +++++- src/validate-refs.ts | 226 ++++++++++++++++++++++++++ src/validate.ts | 19 +++ tests/ci-validate-workflow.test.ts | 41 ++++- tests/validate-refs.test.ts | 246 +++++++++++++++++++++++++++++ tests/validate.test.ts | 31 ++++ 12 files changed, 655 insertions(+), 19 deletions(-) create mode 100644 src/validate-refs.ts create mode 100644 tests/validate-refs.test.ts diff --git a/AGENTS.md b/AGENTS.md index e30d589..044cda4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -133,7 +133,8 @@ precisely. 3. **Validate:** `npm run validate -- ` (offline). CI's **Validate resources** check runs it for every org on every PR; if that check fails, run it locally for the org it names and fix the errors. Don't weaken the - check or the workflow to get past it. + check or the workflow to get past it, and never edit the state file to + make a reference resolve: fix the name, or pull. 4. **Build PR checks offline** if `vapi-checks.yml` exists: `npm run check -- --all --dry-run`. Fix anything it reports. 5. **Deploy only with a yes** (safety rule 1): `npm run apply -- `, or diff --git a/docs/guides/commands.md b/docs/guides/commands.md index f5902fe..299b12f 100644 --- a/docs/guides/commands.md +++ b/docs/guides/commands.md @@ -10,7 +10,7 @@ The other commands are direct only. | Command | Usage | What it does | | --- | --- | --- | | `npm run setup` | `npm run setup [-- ]` | Connect an org: creates `.env.` and `resources//`. | -| `npm run validate` | `npm run validate -- ` | Check resource files offline. Run it before every `apply`. | +| `npm run validate` | `npm run validate -- ` | Check resource files offline: API shape rules, and that every reference names a file or a state entry. `apply` runs it first. On GitHub Actions, findings are also shown on the files in the pull request. | | `npm run apply` | `npm run apply -- [types or paths]` | **The default deploy:** pull, merge, then push. See [workflows](workflows.md). | | `npm run pull` | `npm run pull -- [--force] [--bootstrap]` | Sync platform changes down; never overwrites local edits unless `--force`. | | `npm run push` | `npm run push -- [--dry-run] [--strict]` | Push without pulling first. Prefer `apply`. `--strict` aborts before any API call if validation finds an error. | diff --git a/docs/guides/troubleshooting.md b/docs/guides/troubleshooting.md index 030238c..7a73ce1 100644 --- a/docs/guides/troubleshooting.md +++ b/docs/guides/troubleshooting.md @@ -2,7 +2,8 @@ ## "Reference not found" warnings -The referenced resource doesn't exist. Check: +The referenced resource doesn't exist. `npm run validate` reports these as +`dangling-reference` errors before you deploy. Check: 1. File exists in correct folder 2. Filename matches exactly (case-sensitive) @@ -30,7 +31,9 @@ bypassed). ## "Credential with ID not found" errors -The credential UUID doesn't exist in the target org. Fix: +The credential UUID doesn't exist in the target org. `npm run validate` +warns about a credential name that isn't in the state file +(`unresolved-credential`). Fix: 1. Run `npm run pull -- ` to fetch credentials into the state file 2. If the credential doesn't exist, create it in the Vapi dashboard with the same name @@ -92,11 +95,23 @@ findings: npm run validate -- ``` -Each error names the file, field and rule. Plain `push` only warns about -these errors, so a repository that has been deploying with `push` can carry -some from before the check existed; they show up on the next pull request, -whatever it changes. Fix them in that PR or a separate one first. `apply` -refuses to deploy until they're fixed anyway. +Each finding names the file and the rule, and on GitHub it's also shown on +the file in the pull request. Plain `push` only warns about these errors (unless `--strict`), so +a repository that has been deploying with `push` can carry some from before +the check existed; they show up on the next pull request, whatever it +changes. Fix them in that PR or a separate one first. `apply` refuses to +deploy until they're fixed anyway. + +| Rule | Severity | What to do | +| --- | --- | --- | +| `dangling-reference` | error | A reference names no local file and no state entry. Fix the name (it's the file name without extension, including any folder), or run `npm run pull -- ` if the resource was created in the dashboard. Don't add a state entry by hand. | +| `override-tool-by-name` | error | References inside `assistantOverrides`, `membersOverrides` and `targetOverrides` aren't resolved. Put the tool inline in the override's `model.tools`. | +| `reference-to-ignored` | error | The referenced resource matches `.vapi-ignore`, so it's never deployed. Stop ignoring it, or remove the reference. | +| `name-length` | error | Shorten the name to 40 characters or fewer. | +| `voice-provider-schema` | error | Move the setting to where that voice provider expects it; the message says where. | +| `unresolved-credential` | warning | The credential name isn't in the state file. Run `npm run pull -- --bootstrap` and commit the state file, or create the credential in the dashboard first. | +| `reference-by-uuid` | warning | A UUID only exists in one org and breaks promotion. Reference the file by name. Vapi's stock personalities are exempt. | +| `so-assistant-lockstep`, `prompt-duplicate-*`, `max-tokens-floor` | warning | Follow the message; see [structured outputs](../learnings/structured-outputs.md) and [writing prompts](writing-prompts.md). | A folder under `resources/` that isn't a valid org name (lowercase letters, digits and hyphens) fails too. Rename it, or move it out of `resources/`. diff --git a/improvements.md b/improvements.md index dda040a..28f49c2 100644 --- a/improvements.md +++ b/improvements.md @@ -82,7 +82,7 @@ you which stack PR closes the row.** | 28 | Handoff tools 400 on first push into an empty org | Push aborts before the assistant-linking pass runs | None | RESOLVED 2026-08-01 | | 29 | SO linking sent filtered `assistantIds` arrays | Silent unlink of live-but-untracked assistants | None | RESOLVED 2026-08-03 (#51) | | 30 | Tool-linking pass could PATCH a raw assistant slug | Mid-push 400 naming the wrong resource | None | RESOLVED 2026-08-03 (#51) | -| 31 | Unresolved references handled 3 inconsistent ways, no dangling-ref check | Same authoring mistake, three different failure modes | None | Open | +| 31 | Unresolved references handled 3 inconsistent ways, no dangling-ref check | Same authoring mistake, three different failure modes | None | RESOLVED 2026-10-03 (validation) | | 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) | | 33 | `npm run sim` reported every run as passed | A failing suite exited 0 — false green | None | RESOLVED 2026-10-01 | | 34 | No pre-merge simulation signal; simulations only tested what was deployed | A PR that breaks an agent merges green | #33 | RESOLVED 2026-10-01 | @@ -1568,6 +1568,8 @@ the repo and a subsequent push runs. ## 31. Unresolved references are handled three different ways depending on the field, and `validate.ts` has no dangling-reference check +**[RESOLVED 2026-10-03]** by validation; the three runtime behaviours remain. + **Discovered:** while fixing #29 and #30 — those two entries close the loudest and quietest failure modes for their specific fields, but the underlying question ("what happens when a reference resolves to nothing") @@ -1646,9 +1648,24 @@ conditions; it doesn't require picking one runtime behavior (filter vs. defer vs. 400) for every field, since it stops the push before any of those three behaviors gets a chance to run. +### Possible fix (landed) + +`src/validate-refs.ts` reuses the `extractReferencedIds` walk, plus scenario +judges' `evaluations[].structuredOutputId`, and reports an error for any +name that matches no local file and no state entry (`dangling-reference`). +Names matched by `.vapi-ignore` are left to `reference-to-ignored`, which +`npm run validate` now runs too. Alongside it: `override-tool-by-name` +(an error: push never resolves `toolIds` inside overrides), +`unresolved-credential` and `reference-by-uuid` (warnings). `validate` +reads the committed state file, so the check runs offline and in CI, and +`apply` stops on it before its pull. `push` runs the same checks with its +other validators: warnings by default, blocking under `--strict`. + ### Status -**Open.** +**RESOLVED 2026-10-03** by validation. The runtime still filters, defers or +sends an unresolved reference depending on the field, but `validate` and +`apply` stop before any of that runs. --- diff --git a/src/push.ts b/src/push.ts index d5efc93..5bac449 100644 --- a/src/push.ts +++ b/src/push.ts @@ -39,6 +39,7 @@ import { validateNoIgnoredReferences, validateResources, } from "./validate.ts"; +import { validateReferences } from "./validate-refs.ts"; // Map a resource label to its state-file key. Used for snapshotting — // snapshot directories are keyed by the same names the state file uses. @@ -1714,6 +1715,12 @@ async function main(): Promise { // a config that references an ignored resource is a contradiction the // operator should see. ...validateNoIgnoredReferences(loadedResources, loadIgnorePatterns()), + ...validateReferences({ + loaded: loadedResources, + org: VAPI_ENV, + state, + ignorePatterns: loadIgnorePatterns(), + }), ]; if (findings.length > 0) { console.log(summarizeFindings(findings)); diff --git a/src/state.ts b/src/state.ts index a4ef644..502e096 100644 --- a/src/state.ts +++ b/src/state.ts @@ -48,7 +48,7 @@ function migrateSection( // State Management // ───────────────────────────────────────────────────────────────────────────── -function createEmptyState(): StateFile { +export function createEmptyState(): StateFile { return { credentials: {}, assistants: {}, diff --git a/src/validate-cmd.ts b/src/validate-cmd.ts index f6ab527..fc886b3 100644 --- a/src/validate-cmd.ts +++ b/src/validate-cmd.ts @@ -5,19 +5,32 @@ // and prints findings. Exit code 0 if no errors, 1 if any error-severity // finding is present. -import { resolve } from "path"; +import { existsSync } from "fs"; +import { relative, resolve } from "path"; import { fileURLToPath } from "url"; -import { VAPI_BASE_URL, VAPI_ENV } from "./config.ts"; +import { + BASE_DIR, + loadIgnorePatterns, + STATE_FILE_PATH, + VAPI_ENV, +} from "./config.ts"; import { loadResources } from "./resources.ts"; +import { createEmptyState, loadState } from "./state.ts"; import type { LoadedResources } from "./types.ts"; -import { summarizeFindings, validateResources } from "./validate.ts"; +import { + findingAnnotation, + summarizeFindings, + validateNoIgnoredReferences, + validateResources, +} from "./validate.ts"; +import { validateReferences } from "./validate-refs.ts"; async function main(): Promise { console.log( "═══════════════════════════════════════════════════════════════", ); console.log(`🔎 Vapi GitOps Validate - Environment: ${VAPI_ENV}`); - console.log(` API: ${VAPI_BASE_URL}`); + console.log(" Offline: no API calls"); console.log( "═══════════════════════════════════════════════════════════════\n", ); @@ -35,9 +48,33 @@ async function main(): Promise { evals: await loadResources("evals"), }; - const findings = validateResources(resources); + // References resolve through the committed state file, as they do on push. + const stateExists = existsSync(STATE_FILE_PATH); + if (!stateExists) + console.log("📄 No state file yet: references must name local files."); + const state = stateExists ? loadState() : createEmptyState(); + const ignorePatterns = loadIgnorePatterns(); + const findings = [ + ...validateResources(resources), + ...validateNoIgnoredReferences(resources, ignorePatterns), + ...validateReferences({ + loaded: resources, + org: VAPI_ENV, + state, + ignorePatterns, + }), + ]; console.log(`\n${summarizeFindings(findings)}\n`); + if (process.env.GITHUB_ACTIONS === "true") { + for (const finding of findings) { + const file = resources[finding.type].find( + (r) => r.resourceId === finding.resourceId, + )?.filePath; + console.log(findingAnnotation(finding, file && relative(BASE_DIR, file))); + } + } + const errorCount = findings.filter((f) => f.severity === "error").length; if (errorCount > 0) { console.error( diff --git a/src/validate-refs.ts b/src/validate-refs.ts new file mode 100644 index 0000000..00d7cd9 --- /dev/null +++ b/src/validate-refs.ts @@ -0,0 +1,226 @@ +// ───────────────────────────────────────────────────────────────────────────── +// Reference validators — does every reference point at something? +// +// Push resolves a reference by name through the org's state file, after it +// has created any local files. A name that matches neither is dropped from +// some fields, sent raw (and rejected mid-push) in others, and deferred in +// the rest (improvements #31). These checks catch it before any of that runs. +// Config-free: the caller passes the state and the ignore patterns. +// ───────────────────────────────────────────────────────────────────────────── + +import { credentialForwardMap } from "./credentials.ts"; +import { extractReferencedIds } from "./resolver.ts"; +import { FOLDER_MAP, matchesIgnore } from "./resource-parse.ts"; +import type { + LoadedResources, + ResourceFile, + ResourceType, + StateFile, +} from "./types.ts"; +import type { ValidationFinding } from "./validate.ts"; + +const UUID_RE = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +// Vapi's built-in personalities exist in every org, so a UUID is the only +// way to reference them and is portable. +const STOCK_PERSONALITY_RE = /^a0000000-0000-4000-8000-00000000000\d$/; + +const REF_TYPES: Array<{ + refKey: keyof ReturnType; + refType: ResourceType; +}> = [ + { refKey: "tools", refType: "tools" }, + { refKey: "structuredOutputs", refType: "structuredOutputs" }, + { refKey: "assistants", refType: "assistants" }, + { refKey: "personalities", refType: "personalities" }, + { refKey: "scenarios", refType: "scenarios" }, + { refKey: "simulations", refType: "simulations" }, +]; + +const RESOURCE_TYPES: ResourceType[] = [ + "tools", + "structuredOutputs", + "assistants", + "squads", + "personalities", + "scenarios", + "simulations", + "simulationSuites", + "evals", +]; + +// Push never resolves references inside these objects, so a name there +// reaches the API as-is. +const OVERRIDE_KEYS = new Set([ + "assistantOverrides", + "membersOverrides", + "targetOverrides", +]); + +const refClean = (id: string) => id.split("##")[0]?.trim() ?? ""; + +// Every reference push resolves: the shared walk, plus scenario judges' +// `evaluations[].structuredOutputId`, which the resolver handles separately. +function referencesCollect( + data: Record, +): Map { + const extracted = extractReferencedIds(data); + const refs = new Map(); + for (const { refKey, refType } of REF_TYPES) + refs.set(refType, extracted[refKey].map(refClean).filter(Boolean)); + if (Array.isArray(data.evaluations)) { + for (const evaluation of data.evaluations) { + const id = (evaluation as { structuredOutputId?: unknown }) + ?.structuredOutputId; + if (typeof id === "string") + refs.get("structuredOutputs")!.push(refClean(id)); + } + } + return refs; +} + +// `toolIds` entries inside override objects that aren't UUIDs. +function overrideToolNames(value: unknown, inOverride = false): string[] { + if (Array.isArray(value)) + return value.flatMap((item) => overrideToolNames(item, inOverride)); + if (!value || typeof value !== "object") return []; + const names: string[] = []; + for (const [key, child] of Object.entries(value)) { + if (inOverride && key === "toolIds" && Array.isArray(child)) { + for (const id of child) + if (typeof id === "string" && !UUID_RE.test(refClean(id))) + names.push(refClean(id)); + continue; + } + names.push( + ...overrideToolNames(child, inOverride || OVERRIDE_KEYS.has(key)), + ); + } + return names; +} + +// Credential names (`credentialId` / `credentialIds` values that aren't UUIDs). +function credentialNames( + value: unknown, + names = new Set(), +): Set { + if (Array.isArray(value)) { + for (const item of value) credentialNames(item, names); + return names; + } + if (!value || typeof value !== "object") return names; + for (const [key, child] of Object.entries(value)) { + if (key === "credentialId" && typeof child === "string") { + if (!UUID_RE.test(child)) names.add(child); + } else if (key === "credentialIds" && Array.isArray(child)) { + for (const id of child) + if (typeof id === "string" && !UUID_RE.test(id)) names.add(id); + } else { + credentialNames(child, names); + } + } + return names; +} + +function resourceReferencesCheck(args: { + resource: ResourceFile; + type: ResourceType; + local: Map>; + org: string; + state: StateFile; + ignorePatterns: string[]; +}): ValidationFinding[] { + const { resource, type, local, org, state, ignorePatterns } = args; + const findings: ValidationFinding[] = []; + const finding = ( + severity: ValidationFinding["severity"], + rule: string, + message: string, + ) => + findings.push({ + severity, + type, + resourceId: resource.resourceId, + rule, + message, + }); + const data = resource.data as Record; + + for (const [refType, ids] of referencesCollect(data)) { + const folder = FOLDER_MAP[refType]; + for (const id of new Set(ids)) { + if (UUID_RE.test(id)) { + if (refType === "personalities" && STOCK_PERSONALITY_RE.test(id)) + continue; + finding( + "warn", + "reference-by-uuid", + `references ${folder}/${id} by UUID, which only exists in one org ` + + `and breaks promotion; reference the file by name instead`, + ); + continue; + } + // Reported by the reference-to-ignored rule instead. + if (matchesIgnore(folder, id, ignorePatterns)) continue; + if (local.get(refType)!.has(id) || state[refType][id]) continue; + finding( + "error", + "dangling-reference", + `references ${folder}/${id}, but there is no such file and no ` + + `${refType} entry "${id}" in the state file; check the name, or ` + + `pull first if the resource was created in the dashboard`, + ); + } + } + + for (const name of new Set(overrideToolNames(data))) + finding( + "error", + "override-tool-by-name", + `an override lists tool "${name}" in toolIds, but references inside ` + + `overrides aren't resolved, so the API would receive the name; put ` + + `the tool inline in the override's model.tools instead`, + ); + + const credentials = credentialForwardMap(state); + for (const name of credentialNames(data)) + if (!credentials.has(name)) + finding( + "warn", + "unresolved-credential", + `credential "${name}" isn't in the state file; deploys look it up ` + + `with a bootstrap pull and fail if the org has no credential with ` + + `that name. Run \`npm run pull -- ${org} --bootstrap\` and commit ` + + `the state file`, + ); + + return findings; +} + +export function validateReferences(args: { + loaded: LoadedResources; + org: string; + state: StateFile; + ignorePatterns: string[]; +}): ValidationFinding[] { + const { loaded, org, state, ignorePatterns } = args; + const local = new Map>( + RESOURCE_TYPES.map((type) => [ + type, + new Set(loaded[type].map((resource) => resource.resourceId)), + ]), + ); + return RESOURCE_TYPES.flatMap((type) => + loaded[type].flatMap((resource) => + resourceReferencesCheck({ + resource, + type, + local, + org, + state, + ignorePatterns, + }), + ), + ); +} diff --git a/src/validate.ts b/src/validate.ts index 8e0f68e..4e031f1 100644 --- a/src/validate.ts +++ b/src/validate.ts @@ -534,3 +534,22 @@ export function summarizeFindings(findings: ValidationFinding[]): string { for (const f of findings) lines.push(formatFinding(f)); return lines.join("\n"); } + +// Format a finding as a GitHub Actions workflow command, so it shows on the +// file in the pull request rather than only in the job log. `file` is +// relative to the repository root. +export function findingAnnotation(f: ValidationFinding, file?: string): string { + const escapeData = (s: string) => + s.replace(/%/g, "%25").replace(/\r/g, "%0D").replace(/\n/g, "%0A"); + const escapeProperty = (s: string) => + escapeData(s).replace(/:/g, "%3A").replace(/,/g, "%2C"); + const command = f.severity === "error" ? "error" : "warning"; + const properties = [ + ...(file ? [`file=${escapeProperty(file)}`] : []), + `title=${escapeProperty(`${f.rule}: ${f.type}/${f.resourceId}`)}`, + ]; + const where = f.fieldPath ? ` (${f.fieldPath})` : ""; + return `::${command} ${properties.join(",")}::${escapeData( + `${f.type}/${f.resourceId}${where}: ${f.message}`, + )}`; +} diff --git a/tests/ci-validate-workflow.test.ts b/tests/ci-validate-workflow.test.ts index c4b0b93..9cb8bb4 100644 --- a/tests/ci-validate-workflow.test.ts +++ b/tests/ci-validate-workflow.test.ts @@ -40,7 +40,10 @@ const JOB = ( const STEP = JOB.steps.find((s) => s.name === "Validate every org")!; // Run the step in a scratch repository holding the given org folders. -function validateStepRun(orgs: Record void>): { +function validateStepRun( + orgs: Record void>, + extraEnv: Record = {}, +): { code: number | null; output: string; } { @@ -62,7 +65,12 @@ function validateStepRun(orgs: Record void>): { cwd: root, encoding: "utf8", timeout: 60_000, - env: { PATH: process.env.PATH, HOME: process.env.HOME, ...STEP.env }, + env: { + PATH: process.env.PATH, + HOME: process.env.HOME, + ...extraEnv, + ...STEP.env, + }, }); return { code: result.status, output: `${result.stdout}${result.stderr}` }; } finally { @@ -126,6 +134,35 @@ test("validate step fails naming only the invalid org, after checking all of the ); }); +test("on GitHub, a broken reference fails the step and is annotated on its file", () => { + const run = validateStepRun( + { + clinic: (dir) => { + starterCopy(dir); + const squad = join(dir, "squads", "front-desk.yml"); + writeFileSync( + squad, + readFileSync(squad, "utf8").replace( + "assistantId: scheduler", + "assistantId: schedular", + ), + ); + }, + }, + { GITHUB_ACTIONS: "true" }, + ); + assert.deepEqual( + [ + run.code, + run.output.includes( + "::error file=resources/clinic/squads/front-desk.yml,title=dangling-reference%3A squads/front-desk::", + ), + ], + [1, true], + run.output, + ); +}); + test("validate step fails on an org folder that isn't a valid org name", () => { const run = validateStepRun({ Clinic_Prod: starterCopy }); assert.deepEqual( diff --git a/tests/validate-refs.test.ts b/tests/validate-refs.test.ts new file mode 100644 index 0000000..60d67e5 --- /dev/null +++ b/tests/validate-refs.test.ts @@ -0,0 +1,246 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import type { LoadedResources, ResourceType, StateFile } from "../src/types.ts"; +import { validateReferences } from "../src/validate-refs.ts"; + +// Reference checks: every name a file uses must match a local file or a +// state entry, overrides can't name tools, UUIDs warn, and credential names +// must be in state. + +const STOCK = "a0000000-0000-4000-8000-000000000001"; +const UUID = "3f2b1c4d-5e6f-4a1b-8c9d-0e1f2a3b4c5d"; + +function loaded( + files: Partial>>, +): LoadedResources { + const list = (type: ResourceType) => + Object.entries(files[type] ?? {}).map(([resourceId, data]) => ({ + resourceId, + filePath: `/repo/resources/clinic/${type}/${resourceId}.yml`, + data: data as Record, + })); + return { + tools: list("tools"), + structuredOutputs: list("structuredOutputs"), + assistants: list("assistants"), + squads: list("squads"), + personalities: list("personalities"), + scenarios: list("scenarios"), + simulations: list("simulations"), + simulationSuites: list("simulationSuites"), + evals: list("evals"), + }; +} + +function state( + entries: Partial> = {}, +): StateFile { + const section = (ids: string[] = []) => + Object.fromEntries(ids.map((id) => [id, { uuid: UUID }])); + return { + credentials: section(entries.credentials), + assistants: section(entries.assistants), + structuredOutputs: section(entries.structuredOutputs), + tools: section(entries.tools), + squads: section(entries.squads), + personalities: section(entries.personalities), + scenarios: section(entries.scenarios), + simulations: section(entries.simulations), + simulationSuites: section(entries.simulationSuites), + evals: section(entries.evals), + }; +} + +// [rule, resource, the quoted name or path in the message] +function findings(args: { + files: Partial>>; + stateEntries?: Partial>; + ignorePatterns?: string[]; +}): Array<[string, string, string]> { + return validateReferences({ + loaded: loaded(args.files), + org: "clinic", + state: state(args.stateEntries), + ignorePatterns: args.ignorePatterns ?? [], + }).map((f) => [ + `${f.severity}:${f.rule}`, + `${f.type}/${f.resourceId}`, + f.message + .match(/references (\S+?),? |tool "([^"]+)"|credential "([^"]+)"/)! + .slice(1) + .find(Boolean)!, + ]); +} + +test("references to local files and state entries pass", () => { + assert.deepEqual( + findings({ + files: { + tools: { "book-appointment": {} }, + assistants: { + receptionist: { + model: { + toolIds: ["book-appointment ## books it", "lookup-patient"], + }, + }, + }, + }, + stateEntries: { tools: ["lookup-patient"] }, + }), + [], + ); +}); + +test("a name that matches nothing is an error in every reference field", () => { + assert.deepEqual( + findings({ + files: { + assistants: { + receptionist: { + model: { toolIds: ["book-apointment"] }, + artifactPlan: { structuredOutputIds: ["call-sumary"] }, + hooks: [{ do: [{ toolId: "end-call-tool" }] }], + }, + }, + squads: { "front-desk": { members: [{ assistantId: "schedular" }] } }, + scenarios: { + "books-cleaning": { + evaluations: [{ structuredOutputId: "booking-confirmd" }], + }, + }, + simulations: { + "books-cleaning-calm": { + personalityId: "calm-calller", + scenarioId: "books-cleaning", + }, + }, + simulationSuites: { + core: { simulationIds: ["books-cleaning-calm", "gone"] }, + }, + }, + }), + [ + [ + "error:dangling-reference", + "assistants/receptionist", + "tools/book-apointment", + ], + [ + "error:dangling-reference", + "assistants/receptionist", + "tools/end-call-tool", + ], + [ + "error:dangling-reference", + "assistants/receptionist", + "structuredOutputs/call-sumary", + ], + ["error:dangling-reference", "squads/front-desk", "assistants/schedular"], + [ + "error:dangling-reference", + "scenarios/books-cleaning", + "structuredOutputs/booking-confirmd", + ], + [ + "error:dangling-reference", + "simulations/books-cleaning-calm", + "simulations/personalities/calm-calller", + ], + [ + "error:dangling-reference", + "simulationSuites/core", + "simulations/tests/gone", + ], + ], + ); +}); + +test("a reference to an ignored resource is left to the reference-to-ignored rule", () => { + assert.deepEqual( + findings({ + files: { assistants: { receptionist: { toolIds: ["legacy-lookup"] } } }, + ignorePatterns: ["tools/legacy-*"], + }), + [], + ); +}); + +test("UUID references warn, except Vapi's stock personalities", () => { + assert.deepEqual( + findings({ + files: { + assistants: { receptionist: { model: { toolIds: [UUID] } } }, + simulations: { calm: { personalityId: STOCK, scenarioId: UUID } }, + }, + }), + [ + ["warn:reference-by-uuid", "assistants/receptionist", `tools/${UUID}`], + [ + "warn:reference-by-uuid", + "simulations/calm", + `simulations/scenarios/${UUID}`, + ], + ], + ); +}); + +test("a tool named inside an override is an error; a UUID there is not", () => { + assert.deepEqual( + findings({ + files: { + tools: { "book-appointment": {} }, + squads: { + "front-desk": { + members: [ + { + assistantId: "receptionist", + assistantOverrides: { + model: { toolIds: ["book-appointment", UUID] }, + }, + }, + ], + membersOverrides: { model: { toolIds: ["lookup-patient"] } }, + }, + }, + assistants: { receptionist: {} }, + scenarios: { + s: { targetOverrides: { model: { toolIds: ["transfer-tool"] } } }, + }, + }, + }), + [ + ["error:override-tool-by-name", "squads/front-desk", "book-appointment"], + ["error:override-tool-by-name", "squads/front-desk", "lookup-patient"], + ["error:override-tool-by-name", "scenarios/s", "transfer-tool"], + ], + ); +}); + +test("credential names must be in state; UUIDs and known names pass", () => { + assert.deepEqual( + findings({ + files: { + tools: { + lookup: { server: { credentialId: "crm-api" } }, + other: { credentialIds: ["crm-api", "billing-api", UUID] }, + raw: { server: { credentialId: UUID } }, + }, + }, + stateEntries: { credentials: ["crm-api"] }, + }), + [["warn:unresolved-credential", "tools/other", "billing-api"]], + ); +}); + +test("the credential warning names the org's bootstrap pull", () => { + const [finding] = validateReferences({ + loaded: loaded({ tools: { t: { server: { credentialId: "crm-api" } } } }), + org: "clinic", + state: state(), + ignorePatterns: [], + }); + assert.equal( + finding?.message.includes("`npm run pull -- clinic --bootstrap`"), + true, + ); +}); diff --git a/tests/validate.test.ts b/tests/validate.test.ts index 0286202..3181ecf 100644 --- a/tests/validate.test.ts +++ b/tests/validate.test.ts @@ -370,3 +370,34 @@ test("voice-provider-schema: cartesia membersOverrides.voice in squad checked", assert.equal(findings.length, 1); assert.equal(findings[0]!.fieldPath, "membersOverrides.voice.speed"); }); + +const { findingAnnotation } = await import("../src/validate.ts"); + +test("findingAnnotation: errors and warnings become GitHub workflow commands on the file", () => { + assert.deepEqual( + [ + findingAnnotation( + { + severity: "error", + type: "assistants", + resourceId: "front-desk", + rule: "name-length", + message: "100% too long:\nsee docs, then fix", + fieldPath: "name", + }, + "resources/clinic/assistants/front-desk.md", + ), + findingAnnotation({ + severity: "warn", + type: "tools", + resourceId: "a,b", + rule: "reference-by-uuid", + message: "uses a UUID", + }), + ], + [ + "::error file=resources/clinic/assistants/front-desk.md,title=name-length%3A assistants/front-desk::assistants/front-desk (name): 100%25 too long:%0Asee docs, then fix", + "::warning title=reference-by-uuid%3A tools/a%2Cb::tools/a,b: uses a UUID", + ], + ); +});