Skip to content

feat(check): fail closed on tools and servers in inline check payloads - #62

Open
scott-lowe-vapi wants to merge 1 commit into
feat/check-inline-payloadfrom
feat/check-fail-closed-mocks
Open

scott-lowe-vapi wants to merge 1 commit into
feat/check-inline-payloadfrom
feat/check-fail-closed-mocks

Conversation

@scott-lowe-vapi

Copy link
Copy Markdown
Contributor

Value

V.A.L.U.E. tier: project — PR 6 of 10 for inline simulation PR checks (TEST-141); this is the safety-critical PR, so it's kept separate for review. Still offline: nothing is sent until PR 7.

  • Problem: a PR check runs the branch's agents in a real org, with real LLM conversations that call tools. Without a policy, a check could send a real SMS, book a real appointment, transfer to a real number, or POST transcripts to a customer's webhook, on every push.
  • Who it affects: gitops users, who need checks safe by default; and the TEST-141 failure condition "any tool call reaching a real server under default settings".
  • What changes: src/check-mocks.ts runs as the last pass of checkPayloadBuild under toolMocks: strict (the default).
Class Types Treatment
No external side effect endCall, dtmf, voicemail, output; query (same org only) Sent as written
Knowledge base in model.toolIds by UUID Same org: kept, with a warning. Cross-org or inline: fails
Handoff handoff Only to a squad member by name, or an inline assistant (walked). dynamic, squad, non-members: fail
Mockable function by function.name; apiRequest by top-level name (url set to the dead host) Scenario mock if present, otherwise {"error":"vapi-gitops-ci: <tool> is not mocked in this scenario"}
Transfer transferCall Rewritten to a mocked dead-server function under its own name, so it can never connect
Everything else sms, sipRequest, code, mcp, bash, computer, textEditor, transferCancel, transferSuccessful, google.*, slack.*, gohighlevel.*, ghl, make, unknown Fails the build, naming the tool
  • Structural rule: any key in the exported TOOL_BEARING_KEYS outside a handled position fails with "unsupported tool position". Examples: model.functions, model.toolRefs, reasoner skills, declineTool, tools:append outside overrides. A future API field that carries tools fails instead of slipping through.
  • Servers are replaced, never deleted, because a deleted server falls back to the phone number's or the org's URL:
    • every assistant (target, members, inline handoff assistants, personalities) gets server: {url: "https://vapi-gitops-ci.invalid", timeoutSeconds: 1} and serverMessages: [];
    • overrides that set a server get the dead one;
    • every function tool gets the dead server, and serverUrl / serverUrlSecret are dropped;
    • scenario webhook hooks get the dead server.
  • Default mocks go in each scenario's toolMocks, never in assistant metadata, because handoffs rebuild the assistant. A user mock with enabled: false is replaced, and a mock naming no tool in the target produces a warning.
  • Also fails:
    • model.knowledgeBaseId, and custom-provider knowledge bases;
    • personality tools beyond the side-effect-free ones;
    • hook transfer actions, and hook toolIds;
    • scenarioId entries, and non-stock personalityIds.
  • Hook-fired tools (assistant hooks[].do[]) probably bypass scenario toolMocks, which apply on the LLM tool-call path. They're classified the same way and get the dead server, which is the real safeguard there. This is documented in the module header and goes into simulations.md in PR 8.
  • Opt-outs:
    • toolMocks: off skips the tool rules, for a dedicated CI org;
    • stripWebhooks: false keeps assistant servers, while tool servers are still replaced under strict mocks.

Evidence of value

Dry run of the TEST-141 parity squad. In a copy of the fixture:

  • every tool and the receptionist got real-looking example.com servers;
  • one scenario dropped its book_appointment mock;
  • the --print-payload output was then inspected:
Check Result
example.com URLs left in the payload 0 (8 dead URLs: 2 member assistants, 3 personality copies, 3 function tools including the tools:append one)
Receptionist server / serverMessages {"url":"https://vapi-gitops-ci.invalid","timeoutSeconds":1} / []
S1 and S3 mocks all 3 tools from the scenario
S2 mocks lookup_patient, check_availability from the scenario; book_appointment = default error mock
Same squad plus an sms tool on the receptionist exit 2: target.squad.members[0].assistant.model.tools[1]: sms tools can't be mocked; remove it, or set toolMocks: off with a dedicated CI org

TOOL_BEARING_KEYS audit against the API's OpenAPI schema (apps/dashboard/src/api/schema.json in the monorepo), listing every property whose schema references a tool DTO or is named like a tool reference:

  • Listed:
    • tools (all model DTOs and TransferAssistantModel);
    • tools:append (AssistantOverrides);
    • toolIds and toolRefs (all model DTOs);
    • declineTool / declineToolId (RecordingConsentPlanVerbal);
    • skills (OpenAIReasoner);
    • assistantDestinations (SquadMemberDTO).
  • Not listed, handled at their only position:
    • tool / toolId (ToolCallHookAction) and function (FunctionCallHookAction) are hook actions;
    • function also appears on handoff DTOs, as the tool's own definition.
  • functions and forwardingPhoneNumber(s) aren't in the current schema, and stay listed as fail-closed.

Tests: npm test goes from 430 to 450 passing; npm run build is clean.

Testing plan

  • tests/check-mocks.test.ts (20 tests):
    • each class in the table, with every listed failing type asserted by name;
    • query and knowledge bases, same org vs cross-org;
    • every TOOL_BEARING_KEYS entry at an unhandled position, plus functions / toolRefs / assistantDestinations placement;
    • parameters / judge schema properties named tools not tripping the rule;
    • each handoff destination kind, with inline handoff assistants walked;
    • assistant and override servers;
    • each hook action kind, and scenario webhooks;
    • personality tools;
    • knowledge-base fields;
    • entry shapes;
    • toolMocks: off and stripWebhooks: false.
  • tests/check-payload.test.ts expectations were updated for the dead servers the policy now adds, and the parity end-to-end test still passes.
  • Not tested:
    • Live runs: that scenario toolMocks intercept every function and apiRequest call is shown for the parity run only, and the full transcript scan is PR 7.
    • The unverified transferCall and integration mock names. They're rewritten or refused rather than relied on.
    • Hook-fired tools bypassing mocks (assumed, and safe either way).

Stacked on #61.

Refs TEST-141

🤖 Generated with Claude Code

A PR check runs the branch's agents in a real org, so nothing it sends may
reach a real server by default. The payload builder's last pass now:

- classifies every tool at a handled position (model.tools, tools:append
  in overrides, hook do[], same-org knowledge bases in model.toolIds):
  endCall/dtmf/voicemail/output are sent as written; function tools by
  function.name and apiRequest by name are mocked; transferCall becomes
  a mocked dead-server function; handoffs pass only to squad members or
  inline assistants; everything else (sms, sipRequest, code, mcp,
  integrations, unknown types) fails the build naming the tool;
- fails any tool-bearing key (TOOL_BEARING_KEYS) outside those positions,
  so a new API field can't slip through unclassified;
- replaces servers with https://vapi-gitops-ci.invalid on every assistant
  and function tool rather than deleting them (a deleted server falls back
  to the phone number's or org's), clears serverMessages, and dead-ends
  scenario webhook hooks;
- adds a default error mock for every mocked tool to each scenario's
  toolMocks, replacing disabled ones, and warns about mocks that match no
  tool;
- fails custom knowledge bases, model.knowledgeBaseId, personality tools
  with side effects, hook transfer actions, and cross-org query or
  knowledge-base tools.

`toolMocks: off` skips the tool rules; `stripWebhooks: false` keeps
assistant servers while tool servers are still replaced.

Refs TEST-141

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants