From 01f4b67bc1b33a2df0e885e4d7fecf73774af4e6 Mon Sep 17 00:00:00 2001 From: Scott Lowe Date: Thu, 1 Oct 2026 16:32:36 -0700 Subject: [PATCH] feat(check): add the opt-in Vapi checks PR workflow and user docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit .github/workflows/vapi-checks.yml runs `npm run check` on pull requests (opened, synchronize, reopened, ready_for_review) and on manual dispatch, only when the repository variable VAPI_CHECKS_ENABLED is 'true' and a vapi-checks.yml exists — so on the upstream template it is dormant. - Live runs only for same-repository, non-Dependabot PRs and dispatch; forks and Dependabot get a keyless dry run, and secrets are passed only to live runs. Never pull_request_target. - Checks out the head SHA with full history (for --changed-since against origin/) and persist-credentials: false. - --all posts the aggregate `Vapi Evals` status; dispatching one named check doesn't. The run step execs node so GitHub's cancel reaches it, inside a 22-minute budget under a 30-minute job timeout; concurrency cancels a superseded push's runs. - permissions: contents: read, statuses: write. Docs: a README "PR Checks" section (setup from test files to required status, the build-failure table, fork/Dependabot handling, CI orgs, cost and what stays real), the AGENTS.md simulations step, a "Inline PR Checks" section in docs/learnings/simulations.md, and improvements.md #34. Refs TEST-141 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/vapi-checks.yml | 118 +++++++++++++++++++++ AGENTS.md | 2 +- README.md | 163 +++++++++++++++++++++++++++++- docs/learnings/README.md | 2 +- docs/learnings/simulations.md | 45 +++++++++ improvements.md | 49 +++++++++ 6 files changed, 376 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/vapi-checks.yml diff --git a/.github/workflows/vapi-checks.yml b/.github/workflows/vapi-checks.yml new file mode 100644 index 0000000..a63e21b --- /dev/null +++ b/.github/workflows/vapi-checks.yml @@ -0,0 +1,118 @@ +name: Vapi checks + +# Runs the simulation checks in vapi-checks.yml against the PR branch's own +# files: each target is built inline and sent in one simulation run, so +# nothing is deployed. Opt in with the repository variable +# VAPI_CHECKS_ENABLED=true; see "PR checks" in the README. +# +# Never `pull_request_target`: the branch's code runs here, so it must only +# ever get this repository's secrets when the branch is this repository's. + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + workflow_dispatch: + inputs: + check: + description: >- + Check name from vapi-checks.yml. Leave blank to run every check and + update the Vapi Evals status on the dispatched commit. + required: false + type: string + +permissions: + contents: read + # Only to post the `Vapi Evals` commit statuses that link to each run. + statuses: write + +concurrency: + group: vapi-checks-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + vapi-checks: + if: vars.VAPI_CHECKS_ENABLED == 'true' + runs-on: ubuntu-latest + # The run step's budget (22 min) leaves time to cancel runs and report. + timeout-minutes: 30 + env: + HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }} + + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} + fetch-depth: 0 + persist-credentials: false + + - name: Look for vapi-checks.yml + id: config + shell: bash + run: | + if [[ -f vapi-checks.yml ]]; then + echo "present=true" >> "$GITHUB_OUTPUT" + else + echo "No vapi-checks.yml at the repository root; nothing to check." + echo "present=false" >> "$GITHUB_OUTPUT" + fi + + # Live runs only for this repository's own branches (not forks, not + # Dependabot) and for manual dispatch. Everything else builds the + # payloads without a key, and an affected PR gets an error status + # until a maintainer dispatches the check on it. + - name: Choose live or dry run + id: mode + if: steps.config.outputs.present == 'true' + shell: bash + env: + EVENT: ${{ github.event_name }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + ACTOR: ${{ github.actor }} + AUTHOR: ${{ github.event.pull_request.user.login }} + run: | + live=false + if [[ "$EVENT" == "workflow_dispatch" ]]; then + live=true + elif [[ "$HEAD_REPO" == "$GITHUB_REPOSITORY" && "$ACTOR" != "dependabot[bot]" && "$AUTHOR" != "dependabot[bot]" ]]; then + live=true + fi + echo "live=$live" >> "$GITHUB_OUTPUT" + echo "Live run: $live" + + - uses: actions/setup-node@v4 + if: steps.config.outputs.present == 'true' + with: + node-version: 22 + cache: npm + + - run: npm ci + if: steps.config.outputs.present == 'true' + + - name: Run Vapi checks + if: steps.config.outputs.present == 'true' + shell: bash + env: + LIVE: ${{ steps.mode.outputs.live }} + CHECK: ${{ inputs.check }} + BASE_REF: ${{ github.base_ref }} + GITHUB_TOKEN: ${{ github.token }} + VAPI_CHECK_TOKENS: ${{ steps.mode.outputs.live == 'true' && secrets.VAPI_CHECK_TOKENS || '' }} + VAPI_PRIVATE_API_KEY: ${{ steps.mode.outputs.live == 'true' && secrets.VAPI_PRIVATE_API_KEY || '' }} + run: | + set -euo pipefail + args=() + if [[ -n "$CHECK" ]]; then args+=("$CHECK"); else args+=(--all); fi + if [[ "$LIVE" == "true" ]]; then + args+=(--refresh-bindings) + else + args+=(--dry-run) + fi + if [[ "$GITHUB_EVENT_NAME" == "pull_request" ]] && + git rev-parse --verify --quiet "origin/$BASE_REF" > /dev/null; then + args+=(--changed-since "origin/$BASE_REF") + fi + [[ -n "$VAPI_CHECK_TOKENS" ]] || unset VAPI_CHECK_TOKENS + [[ -n "$VAPI_PRIVATE_API_KEY" ]] || unset VAPI_PRIVATE_API_KEY + # exec, so GitHub's cancel signal (a newer push) reaches node, + # which cancels its in-flight simulation runs. + exec node --import tsx src/check-cmd.ts "${args[@]}" --budget-minutes 22 diff --git a/AGENTS.md b/AGENTS.md index 8753eb0..1f31553 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1032,4 +1032,4 @@ When transferring to human: 2. Create scenarios (what the simulated caller says + evaluation criteria) 3. Create simulations (pair personality + scenario) 4. Create suites (batch simulations together) -5. Run via Vapi dashboard or API +5. Run against the deployed resources with `npm run sim`, or against the local files (nothing deployed) with `npm run check` — see "PR Checks" in the README. The PR workflow runs `npm run check` on every affected PR when `VAPI_CHECKS_ENABLED=true` diff --git a/README.md b/README.md index ca1dc00..8d2e165 100644 --- a/README.md +++ b/README.md @@ -456,6 +456,165 @@ from the destination without touching unrelated destination resources. --- +## PR Checks: simulations against your branch + +`npm run check` runs your simulation suites against the **PR branch's own +files** — prompts, tools, handoffs, structured outputs — without deploying +anything. Each check target (an assistant or a squad) is built from +`resources//` and sent inline, with its scenarios and judges, in one +simulation run. Nothing is created in the org, so there is nothing to clean +up, and it works with a single org. + +### 1. Write tests + +Under `resources//simulations/`: + +- `personalities/calm-caller.yml` (or reference a stock personality by ID) +- `scenarios/books-appointment.yml` — instructions, at least one **required + text judge**, and `toolMocks` for the tools this scenario calls: + + ```yaml + name: Books an appointment + instructions: > + You are John Smith calling to book a cleaning next Tuesday. End the call once it's confirmed. + evaluations: + - structuredOutput: + name: booking-confirmed + type: ai + schema: { type: boolean, description: "Did the assistant confirm a booking time?" } + comparator: "=" + value: true + required: true + toolMocks: + - toolName: book_appointment + result: '{"success": true, "time": "Tuesday 10:00"}' + ``` + + Judges may also reference a file with `structuredOutputId: `. Over + chat, yes/no judges (`=` with `value: true`) are the verified shape; don't + use `hooks` or `messages-with-audio` judges with chat. +- `tests/books-appointment-calm.yml`: `{ name, personalityId: calm-caller, scenarioId: books-appointment }` +- `suites/core.yml`: `{ name: Core, simulationIds: [books-appointment-calm] }` + +### 2. Configure the check + +```bash +cp vapi-checks.example.yml vapi-checks.yml +``` + +```yaml +version: 1 +checks: + core: + org: my-org + targets: [squads/main-squad] + suites: [core] +``` + +### 3. Dry run locally (no key, nothing sent) + +```bash +npm run check -- core --dry-run --print-payload +``` + +`tmp/check-payloads/` shows exactly what would be sent. The build fails, +naming the field, when it can't make a safe, faithful payload: + +| Problem | Fix | +| --- | --- | +| A tool that can't be mocked (SMS, MCP, code, integrations), or an `apiRequest` with no `name` | Remove it, name the `apiRequest`, or set `toolMocks: off` with a dedicated CI org | +| A tool referenced by ID, or a `toolRefs` pin, with no file in `resources//tools/` | Pull the tool into gitops | +| A handoff leaving the squad (`dynamic`, another squad, a non-member), or an assistant target that hands off | Make the target a squad of those assistants | +| A legacy `assistantDestinations` entry naming an assistant by ID | Convert it to a handoff tool | +| Tools by ID inside `assistantOverrides`, `membersOverrides` or `targetOverrides` | Put them inline in `tools:append` | +| Tools outside `model.tools` / `model.toolIds` (`model.functions`, reasoner skills, a recording-consent decline tool) | Move them to `model.tools` | +| `model.knowledgeBaseId`, a custom knowledge base, or a knowledge base / `query` tool when the check runs in another org | Use a knowledge-base tool in the same org | +| Personality tools beyond `endCall`-style ones | Keep the personality free of side-effect tools | +| Two tools with the same type and name on one assistant | Rename one | +| Audio judges, scenario hooks, or no required text judge over chat | Add a text judge, or use `transport: voice` | + +Two things to know: + +- **Transfers never happen.** Every `transferCall` becomes a mocked + function, so a scenario that needs a real transfer fails rather than + falsely passing. +- **Handoff names.** The check warns when a prompt mentions an + auto-generated `handoff_to_…` name — those differ between inline and + deployed assistants. Give that handoff an explicit `function.name`. + +### 4. Live run locally + +```bash +npm run check -- core +``` + +Uses `.env.`, prints the run link, and exits 0 passed, 1 failed, 2 +config or build error, 3 incomplete. It uses simulation minutes. + +### 5. Turn on the PR workflow + +In GitHub → Settings → Secrets and variables → Actions: + +- Secret `VAPI_PRIVATE_API_KEY` (single org), or `VAPI_CHECK_TOKENS` = + `{"my-org":""}` (several orgs, or a CI org). +- Variable `VAPI_CHECKS_ENABLED=true`. + +`.github/workflows/vapi-checks.yml` then runs every affected check on each +PR push. It asks for `statuses: write` only to post the direct links. + +### 6. What a PR shows + +- `Vapi Evals`, plus `Vapi Evals / / ` per target. **Details** + opens the run in Vapi. +- A job summary: per-target result, failing judges with expected vs actual, + and any "unmocked tool called" notices. No PR comments. +- PRs that touch neither a check's org, its state, `vapi-checks.yml`, + `promotion.yml`, the engine (`src/**`, `package*.json`), nor the check's + own `paths` skip it, and `Vapi Evals` posts success. +- A newer push cancels the older run. + +### 7. Make it required (after a burn-in) + +Require the **commit status `Vapi Evals`** in branch protection — not the +`vapi-checks` job (fork dry runs succeed) and not the per-target statuses +(PRs that don't touch a check never get them). + +- **Fork PRs** run a dry run without secrets and can't post statuses (the + token is read-only), so a required `Vapi Evals` blocks them. +- **Dependabot PRs** that change `package*.json` post `error`. +- **To unblock either**, a maintainer runs Actions → Vapi checks → Run + workflow on the PR's branch with `check` blank (push a fork's branch into + the repository first). A later PR event on the same commit resets the + status, so dispatch again after that. Running one named check by hand + never changes `Vapi Evals`. + +### Dedicated CI org (optional; recommended with `toolMocks: off`) + +1. `npm run setup -- my-ci-org --resources none`. +2. Create the credentials your agents need there, with the same names as + production. +3. `npm run pull -- my-ci-org --bootstrap --bindings-only`, then commit + `.vapi-state.my-ci-org.json` so the state knows those credentials. The PR + workflow refreshes bindings on every live run. +4. Add `runOrg: my-ci-org` (and `baseUrl` for EU) to the check, and + optionally `bindings:` (same shape as `promotion.yml`). Phone numbers are + omitted by default. +5. Put only the CI org's key in `VAPI_CHECK_TOKENS`. + +### Cost and safety + +- Every affected push starts paid runs; chat transport is the default. +- Tool calls get their scenario mock or an error, and every assistant and + function-tool server points at `https://vapi-gitops-ci.invalid`. +- Still real in the run org: custom LLM, voice and transcriber servers see + the conversation; org-wide and assistant monitors run; `observabilityPlan` + exports transcripts; prompts are stored with the run. A CI org avoids all + of these. +- `.ts` resource files execute during the check, with the workflow's secrets + on same-repository PRs. + +--- + ## How to Use This Repo 1. **Run `npm run setup`** to configure your first org (or `npm run setup -- ` without a terminal) @@ -812,7 +971,8 @@ vapi-gitops/ │ ├── resources.ts # Resource loading (YAML, MD, TS) │ ├── resolver.ts # Reference resolution │ ├── credentials.ts # Credential resolution (name ↔ UUID) -│ └── delete.ts # Deletion & orphan checks +│ ├── delete.ts # Deletion & orphan checks +│ └── check-cmd.ts # Entry point: PR simulation checks (check-*.ts) ├── resources/ │ └── / # One directory per configured org │ ├── assistants/ @@ -831,6 +991,7 @@ vapi-gitops/ │ ├── path-matching.test.ts # Short-form path matching (P0-7 regression suite) │ ├── cleanup-safety.test.ts # --confirm + empty-state gates (P0-4 regression suite) │ └── cli-arg-parsing.test.ts # Bare-id refusal, --confirm pass-through (P0-7) +├── vapi-checks.example.yml # Copy to vapi-checks.yml for PR simulation checks ├── .env. # Private API key per org (gitignored) └── .vapi-state..json # State file per org ``` diff --git a/docs/learnings/README.md b/docs/learnings/README.md index 5b9838b..0ac6214 100644 --- a/docs/learnings/README.md +++ b/docs/learnings/README.md @@ -44,7 +44,7 @@ Gotchas and silent defaults for each resource type: | [assistants.md](assistants.md) | Model defaults, voice, transcriber, firstMessage, outbound modes, voicemailMessage, hooks, idle messages, endpointing, interruption, analysis, artifacts, background sound, server messages, HIPAA, tool resolution | | [squads.md](squads.md) | Name uniqueness, tools:append, assistantDestinations, handoff context, contextEngineeringPlan, VM detection relay pattern, override merge order, `membersOverrides` structuredDataPlan + fullMessageHistory | | [structured-outputs.md](structured-outputs.md) | Schema type gotchas, assistant_ids, default models, target modes, KPI patterns, squad `membersOverrides` vs standalone SOs | -| [simulations.md](simulations.md) | Personalities, evaluation comparators, chat-mode gotcha, missing references, full `/eval/simulation/*` API reference | +| [simulations.md](simulations.md) | Personalities, evaluation comparators, chat-mode gotcha, missing references, inline PR checks (`npm run check`), full `/eval/simulation/*` API reference | | [webhooks.md](webhooks.md) | Default server messages, timeouts, unreachable servers, credential resolution, payload shape | | [voice-providers.md](voice-providers.md) | Per-provider voice block layout (Cartesia vs 11labs vs OpenAI/Azure/Rime/LMNT/Minimax/Neuphonic/SmallestAI) — saves 400s at push time | diff --git a/docs/learnings/simulations.md b/docs/learnings/simulations.md index e94b18b..5578c99 100644 --- a/docs/learnings/simulations.md +++ b/docs/learnings/simulations.md @@ -227,6 +227,51 @@ Base URL: `https://api.vapi.ai` --- +## Inline PR Checks (`npm run check`) + +`npm run check` sends the target and its tests **inline** in one +`POST /eval/simulation/run` (`target.assistant` / `target.squad`, and +`{type: "simulation", name, scenario, personality}` entries), built from the +branch's files. Setup is in the README's "PR Checks" section; these are the +behaviours worth knowing when a check surprises you. + +- **Inline matches stored, with two known differences.** A 2026-10-01 parity + run (TEST-141) scored inline and stored versions of the same squad 15/15 + each, with the same handoff and business-tool sequences. The differences: + - **Generated handoff names:** `handoff_to_` inline vs + `handoff_to_` stored. A prompt or judge that names the generated + function behaves differently — give the handoff an explicit + `function.name`. The check warns when it sees `handoff_to_` in text. + - **Tool order** decides which tool the model reaches for. The runtime + puts `model.tools` first, then `toolIds` in order, then `toolRefs`; the + check builds inline tools in that same order. +- **Tool mocks fail closed.** Function tools are mocked by `function.name`, + `apiRequest` tools by their top-level `name`. A tool the scenario doesn't + mock answers `{"error":"vapi-gitops-ci: is not mocked in this + scenario"}`, and the report lists it as "unmocked tool called". A scenario + mock with `enabled: false` is replaced by that error. +- **Hook-fired tools bypass scenario mocks.** Scenario `toolMocks` apply on + the LLM tool-call path; tools fired from an assistant's `hooks[].do[]` + probably don't consult them. Their safeguard is the dead server: expect an + error result for them in transcripts, not a mock. +- **Servers are replaced, never deleted.** Every assistant and function tool + gets `server: https://vapi-gitops-ci.invalid` (1 s timeout) and + `serverMessages: []`. A *deleted* server falls back to the phone number's + or the org's server URL, which would leak the conversation. +- **Transfers can't happen.** `transferCall` becomes a mocked dead-server + function under the same name. A scenario that needs a real transfer fails. +- **What still reaches real systems** in the run org: custom LLM, voice and + transcriber providers, org-wide and assistant monitors, and + `observabilityPlan` exports. Use a dedicated CI org (`runOrg`) to avoid + them. +- **Over chat**, `messages-with-audio` judges and scenario `hooks` don't + work, and a scenario with no required text judge can't fail; the build + refuses all three. +- **Run items echo the scenario.** `metadata.scenario.toolMocks` includes the + default error mocks whether or not a tool was called; read tool results + from `metadata.call.messages` (`tool_calls` carry the ids and names, + `tool_call_result` the answers by `toolCallId`). + ## Simulations (`/eval/simulation`) ### Create simulation — `POST /eval/simulation` diff --git a/improvements.md b/improvements.md index 7472bd7..bf41714 100644 --- a/improvements.md +++ b/improvements.md @@ -85,6 +85,7 @@ you which stack PR closes the row.** | 31 | Unresolved references handled 3 inconsistent ways, no dangling-ref check | Same authoring mistake, three different failure modes | None | Open | | 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 | **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. @@ -1773,6 +1774,54 @@ None needed once the fix below lands. --- +## 34. No pre-merge simulation signal; simulations only tested what was deployed + +**[RESOLVED 2026-10-01]** + +**Discovered:** 2026-10-01, TEST-141 (and PAL-608, where customers hand-maintain shell workflows for this). + +### Problem + +Nothing told a PR author that a change to an assistant, squad, tool or +structured output broke behaviour before it merged. `npm run sim` runs a +suite against the *deployed* target by ID, so it can only test a change +after `apply`, and it couldn't see the PR branch at all. + +### Current behavior (Verified, before the fix) + +- `src/sim.ts` sent `target: {assistantId | squadId}` and suite IDs: the + platform's stored copies, never the branch's files. +- No workflow ran simulations on pull requests, and nothing posted a commit + status or linked a run. + +### Risk + +Behaviour regressions merged green and reached production through +`apply` or promotion. + +### Current mitigation + +None needed once the fix below lands. + +### Possible fix (landed) + +- `npm run check` (`src/check-*.ts`) builds each `vapi-checks.yml` target and + its tests from the branch's files and sends them inline in one run per + target, so nothing is deployed or left behind. Tools are mocked fail-closed + and servers dead-ended (`src/check-mocks.ts`); the verdict is the strict + `simRunVerdict` from #33. +- `.github/workflows/vapi-checks.yml` (opt-in via `VAPI_CHECKS_ENABLED`) runs + affected checks on each PR and posts `Vapi Evals` commit statuses that link + straight to the run; forks and Dependabot get a keyless dry run. +- Docs: README "PR Checks", `docs/learnings/simulations.md` "Inline PR + Checks". + +### Status + +**RESOLVED 2026-10-01.** + +--- + ## Out of scope (intentionally not improvements) - **State file is identity-only and not git-ignored.** It's intentionally